mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Preserve thread context when retrying or editing turns (#33211)
## What changed - Add experimental `beforeTurnId` support to `thread/fork` for copying history strictly before a turn, including an in-progress turn. - Use before-turn forks for TUI backtracking and safety retries so the replacement prompt retains thread context instead of recreating the branch from the preceding turn or a new thread. - Add experimental `deferGoalContinuation` support so safety-retry forks can inherit an active goal while waiting for the replacement turn before automatic continuation resumes. - Hide duplicated nested-review prompts when replaying history or selecting a prompt to edit. ## Testing - Cover before-turn truncation, fork validation, active-goal inheritance, TUI safety retries, backtracking, and nested-review replay. GitOrigin-RevId: 4915d85ae1929a0b8c40f3ed7dca29c7a5bdcdf3
This commit is contained in:
@@ -501,6 +501,12 @@ pub struct ThreadForkParams {
|
||||
#[ts(optional = nullable)]
|
||||
pub last_turn_id: Option<String>,
|
||||
|
||||
/// Optional turn id to fork before, excluding that turn and all later turns.
|
||||
/// Cannot be combined with `last_turn_id`.
|
||||
#[experimental("thread/fork.beforeTurnId")]
|
||||
#[ts(optional = nullable)]
|
||||
pub before_turn_id: Option<String>,
|
||||
|
||||
/// [UNSTABLE] Specify the rollout path to fork from.
|
||||
/// If specified, the thread_id param will be ignored.
|
||||
#[experimental("thread/fork.path")]
|
||||
@@ -561,6 +567,12 @@ pub struct ThreadForkParams {
|
||||
#[experimental("thread/fork.excludeTurns")]
|
||||
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
|
||||
pub exclude_turns: bool,
|
||||
/// When true, carry the source thread's current goal into the fork without
|
||||
/// starting its initial automatic continuation. The next explicit turn owns
|
||||
/// the goal lifecycle, and normal automatic continuation resumes after it.
|
||||
#[experimental("thread/fork.deferGoalContinuation")]
|
||||
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
|
||||
pub defer_goal_continuation: bool,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS, ExperimentalApi)]
|
||||
|
||||
@@ -139,7 +139,7 @@ Example with notification opt-out:
|
||||
|
||||
- `thread/start` — create a new thread; emits `thread/started` (including the current `thread.status`) and auto-subscribes you to turn/item events for that thread. When the request includes a `cwd` and the resolved sandbox is `workspace-write` or full access, app-server also marks that project as trusted in the user `config.toml`. Pass `sessionStartSource: "clear"` when starting a replacement thread after clearing the current session so `SessionStart` hooks receive `source: "clear"` instead of the default `"startup"`. Experimental `allowProviderModelFallback` lets providers backed by an authoritative static model catalog replace an unavailable requested `model` with the catalog default; dynamic or cached catalogs preserve the requested model. Experimental `runtimeWorkspaceRoots` supplies the runtime workspace roots used when app-server creates default environment selections; paths must be absolute. For permissions, prefer experimental `permissions` profile selection by id; the legacy `sandbox` shorthand is still accepted but cannot be combined with `permissions`. Deprecated experimental `multiAgentMode` is ignored; use Ultra reasoning effort for proactive multi-agent behavior. Experimental `environments` selects the sticky execution environments for turns on the thread; omit it to use the server default, pass `[]` to disable environments, or pass explicit environment ids with per-environment `cwd` and optional environment-native `runtimeWorkspaceRoots`. Explicit environments ignore the top-level roots; omitted per-environment roots default to that environment's `cwd`, while an empty list explicitly selects no roots. Experimental `selectedCapabilityRoots` selects environment-owned plugin or standalone-skill roots using environment-native absolute paths. Skills found below those roots are listed and read through the owning environment. Stdio MCP servers declared by selected plugins are started in that environment, and HTTP MCP connections use that environment's HTTP client.
|
||||
- `thread/resume` — reopen an existing thread by id so subsequent `turn/start` calls append to it. Accepts the same permission override rules as `thread/start`.
|
||||
- `thread/fork` — fork an existing thread into a new thread id by copying the stored history; pass an optional `lastTurnId` to copy history only through that turn, inclusive, and drop later turns from the fork. An in-progress `lastTurnId` is rejected. If `lastTurnId` is null while the source thread is mid-turn, the fork records the same interruption marker as `turn/interrupt` instead of inheriting an unmarked partial turn suffix. The returned `thread.forkedFromId` points at the source thread when known. Accepts `ephemeral: true` for an in-memory temporary fork, emits `thread/started` (including the current `thread.status`), and auto-subscribes you to turn/item events for the new thread. Experimental clients can pass `excludeTurns: true` when they plan to page fork history via `thread/turns/list` instead of receiving the full turn array immediately. Accepts the same permission override rules as `thread/start`.
|
||||
- `thread/fork` — fork an existing thread into a new thread id by copying the stored history; pass an optional `lastTurnId` to copy history only through that turn, inclusive, and drop later turns from the fork. An in-progress `lastTurnId` boundary is rejected. Experimental `beforeTurnId` instead copies history strictly before the referenced turn, including when that turn is in progress, and cannot be combined with `lastTurnId`. If both boundaries are null while the source thread is mid-turn, the fork records the same interruption marker as `turn/interrupt` instead of inheriting an unmarked partial turn suffix. The returned `thread.forkedFromId` points at the source thread when known. Accepts `ephemeral: true` for an in-memory temporary fork, emits `thread/started` (including the current `thread.status`), and auto-subscribes you to turn/item events for the new thread. Experimental clients can pass `excludeTurns: true` when they plan to page fork history via `thread/turns/list` instead of receiving the full turn array immediately, or `deferGoalContinuation: true` to carry the source thread's current goal into the fork and run an explicit turn before automatic continuation resumes. Deferred goal continuation is persisted until that turn starts and cannot be combined with `ephemeral: true`. Accepts the same permission override rules as `thread/start`.
|
||||
- `thread/start`, `thread/resume`, and `thread/fork` responses include the legacy `sandbox` compatibility projection. `instructionSources` lists loaded instruction files using each source environment's native absolute path syntax, including files loaded from remote environments. Experimental clients can read `runtimeWorkspaceRoots` for the thread-scoped runtime roots and `activePermissionProfile` for the named or implicit built-in profile identity/provenance when known. Their deprecated experimental `multiAgentMode` field, and the corresponding thread setting, always report `explicitRequestOnly`; Ultra reasoning effort is the source of proactive multi-agent behavior.
|
||||
- `thread/list` — page through stored threads; supports cursor-based pagination and optional `modelProviders`, `sourceKinds`, `archived`, `cwd`, and `searchTerm` filters. Experimental clients can use `parentThreadId` for direct spawned children or `ancestorThreadId` for spawned descendants at any depth; the two filters are mutually exclusive. Review and Guardian threads are not included because they do not participate in that spawn-edge lifecycle. Each returned `thread` includes `status` (`ThreadStatus`), defaulting to `notLoaded` when the thread is not currently loaded. Subagent threads also include `parentThreadId` when the immediate parent is known.
|
||||
- `thread/loaded/list` — list the thread ids currently loaded in memory.
|
||||
|
||||
@@ -343,6 +343,7 @@ use codex_core::path_utils;
|
||||
use codex_core::read_head_for_summary;
|
||||
use codex_core::sandboxing::SandboxPermissions;
|
||||
use codex_core::truncate_rollout_after_turn_id;
|
||||
use codex_core::truncate_rollout_before_turn_id;
|
||||
use codex_core::windows_sandbox::WindowsSandboxLevelExt;
|
||||
use codex_core::windows_sandbox::WindowsSandboxSetupMode as CoreWindowsSandboxSetupMode;
|
||||
use codex_core::windows_sandbox::WindowsSandboxSetupRequest;
|
||||
@@ -523,6 +524,7 @@ mod plugins;
|
||||
mod process_exec_processor;
|
||||
mod remote_control_processor;
|
||||
mod search;
|
||||
mod thread_fork_goal;
|
||||
mod thread_processor;
|
||||
mod token_usage_replay;
|
||||
mod turn_processor;
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::protocol::validate_thread_goal_objective;
|
||||
use codex_state::StateRuntime;
|
||||
|
||||
pub(super) async fn inherit_thread_goal_snapshot(
|
||||
state_db: &StateRuntime,
|
||||
source_thread_id: ThreadId,
|
||||
target_thread_id: ThreadId,
|
||||
) -> anyhow::Result<bool> {
|
||||
let Some(mut goal) = state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(source_thread_id)
|
||||
.await?
|
||||
else {
|
||||
return Ok(false);
|
||||
};
|
||||
if let Err(err) = validate_thread_goal_objective(&goal.objective) {
|
||||
tracing::warn!(%source_thread_id, "skipping invalid inherited thread goal: {err}");
|
||||
return Ok(false);
|
||||
}
|
||||
|
||||
goal.thread_id = target_thread_id;
|
||||
state_db
|
||||
.thread_goals()
|
||||
.replace_thread_goal_snapshot(&goal)
|
||||
.await?;
|
||||
Ok(true)
|
||||
}
|
||||
@@ -94,6 +94,26 @@ impl ThreadGoalRequestProcessor {
|
||||
(emit_thread_goal_update, thread_goal_state_db)
|
||||
}
|
||||
|
||||
pub(crate) async fn restore_inherited_goal_runtime(&self, thread_id: ThreadId) {
|
||||
if let Err(err) = self
|
||||
.goal_service
|
||||
.restore_thread_runtime_after_resume(thread_id)
|
||||
.await
|
||||
{
|
||||
warn!("failed to restore inherited goal runtime for {thread_id}: {err}");
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn flush_goal_progress_for_fork(
|
||||
&self,
|
||||
thread_id: ThreadId,
|
||||
) -> Result<(), String> {
|
||||
self.goal_service
|
||||
.flush_thread_goal_progress_for_fork(thread_id)
|
||||
.await
|
||||
.map_err(|err| err.to_string())
|
||||
}
|
||||
|
||||
async fn thread_goal_set_inner(
|
||||
&self,
|
||||
request_id: ConnectionRequestId,
|
||||
@@ -283,7 +303,7 @@ impl ThreadGoalRequestProcessor {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn emit_thread_goal_snapshot(&self, thread_id: ThreadId) {
|
||||
pub(crate) async fn emit_thread_goal_snapshot(&self, thread_id: ThreadId) {
|
||||
let state_db = match self.state_db_for_materialized_thread(thread_id).await {
|
||||
Ok(state_db) => state_db,
|
||||
Err(err) => {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use super::thread_fork_goal::inherit_thread_goal_snapshot;
|
||||
use super::*;
|
||||
use crate::error_code::method_not_found;
|
||||
use codex_app_server_protocol::SelectedCapabilityRoot;
|
||||
@@ -3549,6 +3550,7 @@ impl ThreadRequestProcessor {
|
||||
let ThreadForkParams {
|
||||
thread_id,
|
||||
last_turn_id,
|
||||
before_turn_id,
|
||||
path,
|
||||
model,
|
||||
model_provider,
|
||||
@@ -3565,6 +3567,7 @@ impl ThreadRequestProcessor {
|
||||
ephemeral,
|
||||
thread_source,
|
||||
exclude_turns,
|
||||
defer_goal_continuation,
|
||||
} = params;
|
||||
let include_turns = !exclude_turns;
|
||||
if sandbox.is_some() && permissions.is_some() {
|
||||
@@ -3582,6 +3585,16 @@ impl ThreadRequestProcessor {
|
||||
if matches!(source_thread.history_mode, ThreadHistoryMode::Paginated) {
|
||||
return Err(method_not_found("paginated_threads is not supported yet"));
|
||||
}
|
||||
if last_turn_id.is_some() && before_turn_id.is_some() {
|
||||
return Err(invalid_request(
|
||||
"`beforeTurnId` cannot be combined with `lastTurnId`",
|
||||
));
|
||||
}
|
||||
if ephemeral && defer_goal_continuation {
|
||||
return Err(invalid_request(
|
||||
"`deferGoalContinuation` cannot be combined with `ephemeral`",
|
||||
));
|
||||
}
|
||||
let mut source_thread = self
|
||||
.read_stored_thread_for_resume(&thread_id, path.as_ref(), /*include_history*/ true)
|
||||
.await?;
|
||||
@@ -3599,13 +3612,17 @@ impl ThreadRequestProcessor {
|
||||
"thread {source_thread_id} did not include persisted history"
|
||||
))
|
||||
})?;
|
||||
let history_items = if let Some(last_turn_id) = last_turn_id.as_deref() {
|
||||
Arc::new(
|
||||
let history_items = match (last_turn_id.as_deref(), before_turn_id.as_deref()) {
|
||||
(Some(last_turn_id), None) => Arc::new(
|
||||
truncate_rollout_after_turn_id(&history_items, last_turn_id)
|
||||
.map_err(|err| core_thread_write_error("truncate thread for fork", err))?,
|
||||
)
|
||||
} else {
|
||||
Arc::new(history_items)
|
||||
),
|
||||
(None, Some(before_turn_id)) => Arc::new(
|
||||
truncate_rollout_before_turn_id(&history_items, before_turn_id)
|
||||
.map_err(|err| core_thread_write_error("truncate thread for fork", err))?,
|
||||
),
|
||||
(None, None) => Arc::new(history_items),
|
||||
(Some(_), Some(_)) => unreachable!("fork boundaries are mutually exclusive"),
|
||||
};
|
||||
let history_cwd = Some(source_thread.cwd.clone());
|
||||
|
||||
@@ -3653,6 +3670,7 @@ impl ThreadRequestProcessor {
|
||||
.load_for_cwd(request_overrides, typesafe_overrides, history_cwd)
|
||||
.await
|
||||
.map_err(|err| config_load_error(&err))?;
|
||||
let goals_enabled = config.features.enabled(Feature::Goals);
|
||||
|
||||
let fallback_model_provider = config.model_provider_id.clone();
|
||||
|
||||
@@ -3705,6 +3723,33 @@ impl ThreadRequestProcessor {
|
||||
.await
|
||||
.map_err(|err| core_thread_write_error("inherit source thread name", err))?;
|
||||
}
|
||||
let inherited_goal = if defer_goal_continuation
|
||||
&& session_configured.rollout_path.is_some()
|
||||
&& goals_enabled
|
||||
{
|
||||
if let Some(state_db) = forked_thread.state_db().or_else(|| self.state_db.clone()) {
|
||||
self.thread_goal_processor
|
||||
.flush_goal_progress_for_fork(source_thread_id)
|
||||
.await
|
||||
.map_err(|err| {
|
||||
internal_error(format!("failed to flush source thread goal: {err}"))
|
||||
})?;
|
||||
inherit_thread_goal_snapshot(&state_db, source_thread_id, thread_id)
|
||||
.await
|
||||
.map_err(|err| {
|
||||
internal_error(format!("failed to inherit source thread goal: {err}"))
|
||||
})?
|
||||
} else {
|
||||
false
|
||||
}
|
||||
} else {
|
||||
false
|
||||
};
|
||||
if inherited_goal {
|
||||
self.thread_goal_processor
|
||||
.restore_inherited_goal_runtime(thread_id)
|
||||
.await;
|
||||
}
|
||||
|
||||
let instruction_sources = forked_thread.legacy_instruction_sources().await;
|
||||
|
||||
@@ -3822,6 +3867,11 @@ impl ThreadRequestProcessor {
|
||||
self.outgoing
|
||||
.send_server_notification(ServerNotification::ThreadStarted(notif))
|
||||
.await;
|
||||
if inherited_goal {
|
||||
self.thread_goal_processor
|
||||
.emit_thread_goal_snapshot(thread_id)
|
||||
.await;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -4,6 +4,8 @@ use app_test_support::TestAppServer;
|
||||
use app_test_support::create_fake_rollout;
|
||||
use app_test_support::create_fake_rollout_with_token_usage;
|
||||
use app_test_support::create_mock_responses_server_repeating_assistant;
|
||||
use app_test_support::create_mock_responses_server_sequence_unchecked;
|
||||
use app_test_support::rollout_path;
|
||||
use app_test_support::to_response;
|
||||
use app_test_support::write_chatgpt_auth;
|
||||
use codex_app_server_protocol::JSONRPCError;
|
||||
@@ -17,6 +19,8 @@ use codex_app_server_protocol::ThreadForkResponse;
|
||||
use codex_app_server_protocol::ThreadItem;
|
||||
use codex_app_server_protocol::ThreadListParams;
|
||||
use codex_app_server_protocol::ThreadListResponse;
|
||||
use codex_app_server_protocol::ThreadReadParams;
|
||||
use codex_app_server_protocol::ThreadReadResponse;
|
||||
use codex_app_server_protocol::ThreadResumeParams;
|
||||
use codex_app_server_protocol::ThreadSource;
|
||||
use codex_app_server_protocol::ThreadStartParams;
|
||||
@@ -31,11 +35,16 @@ use codex_app_server_protocol::UserInput;
|
||||
use codex_config::types::AuthCredentialsStoreMode;
|
||||
use codex_login::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::MultiAgentVersion;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::TurnStartedEvent;
|
||||
use codex_protocol::protocol::UserMessageEvent;
|
||||
use codex_rollout::append_rollout_item_to_path;
|
||||
use codex_rollout::append_thread_name;
|
||||
use codex_rollout::read_session_meta_line;
|
||||
use codex_state::StateRuntime;
|
||||
use core_test_support::responses;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
@@ -370,6 +379,276 @@ async fn thread_fork_at_last_turn_id_keeps_only_terminal_prefix() -> Result<()>
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_fork_defers_inherited_active_goal_until_next_turn() -> Result<()> {
|
||||
let server = create_mock_responses_server_sequence_unchecked(vec![
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("first-source-turn"),
|
||||
responses::ev_completed("first-source-turn"),
|
||||
]),
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("second-source-turn"),
|
||||
responses::ev_completed("second-source-turn"),
|
||||
]),
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("explicit-fork-turn"),
|
||||
responses::ev_completed_with_tokens("explicit-fork-turn", /*total_tokens*/ 20),
|
||||
]),
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("goal-continuation"),
|
||||
responses::ev_completed_with_tokens("goal-continuation", /*total_tokens*/ 100),
|
||||
]),
|
||||
])
|
||||
.await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri())?;
|
||||
let config_path = codex_home.path().join("config.toml");
|
||||
let config = std::fs::read_to_string(&config_path)?;
|
||||
std::fs::write(
|
||||
&config_path,
|
||||
format!("{config}\n[features]\ngoals = true\n"),
|
||||
)?;
|
||||
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_managed_config()
|
||||
.build()
|
||||
.await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let start_id = mcp
|
||||
.send_thread_start_request_with_auto_env(ThreadStartParams::default())
|
||||
.await?;
|
||||
let start_response: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(start_id)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadStartResponse {
|
||||
thread: source_thread,
|
||||
..
|
||||
} = to_response::<ThreadStartResponse>(start_response)?;
|
||||
let source_thread_id = ThreadId::from_string(&source_thread.id)?;
|
||||
|
||||
let mut turn_ids = Vec::new();
|
||||
for text in ["first", "second"] {
|
||||
let completed = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.start_turn_and_wait_for_completion(TurnStartParams {
|
||||
thread_id: source_thread.id.clone(),
|
||||
input: vec![UserInput::Text {
|
||||
text: text.to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
..Default::default()
|
||||
}),
|
||||
)
|
||||
.await??;
|
||||
turn_ids.push(completed.turn.id);
|
||||
}
|
||||
mcp.clear_message_buffer();
|
||||
|
||||
let state_db =
|
||||
StateRuntime::init(codex_home.path().to_path_buf(), "mock_provider".into()).await?;
|
||||
let source_goal = state_db
|
||||
.thread_goals()
|
||||
.replace_thread_goal(
|
||||
source_thread_id,
|
||||
"continue after the retry",
|
||||
codex_state::ThreadGoalStatus::Active,
|
||||
/*token_budget*/ Some(150),
|
||||
)
|
||||
.await?;
|
||||
state_db
|
||||
.thread_goals()
|
||||
.account_thread_goal_usage(
|
||||
source_thread_id,
|
||||
/*time_delta_seconds*/ 11,
|
||||
/*token_delta*/ 37,
|
||||
codex_state::GoalAccountingMode::ActiveOnly,
|
||||
Some(source_goal.goal_id.as_str()),
|
||||
)
|
||||
.await?;
|
||||
let source_goal = state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(source_thread_id)
|
||||
.await?
|
||||
.expect("source goal");
|
||||
|
||||
let ordinary_fork_id = mcp
|
||||
.send_thread_fork_request(ThreadForkParams {
|
||||
thread_id: source_thread.id.clone(),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let ordinary_fork_response: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(ordinary_fork_id)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadForkResponse {
|
||||
thread: ordinary_fork,
|
||||
..
|
||||
} = to_response::<ThreadForkResponse>(ordinary_fork_response)?;
|
||||
assert_eq!(
|
||||
state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(ThreadId::from_string(&ordinary_fork.id)?)
|
||||
.await?,
|
||||
None
|
||||
);
|
||||
|
||||
let mut forked_threads = Vec::new();
|
||||
for (last_turn_id, before_turn_id, expected_turn_count) in [
|
||||
(None, None, 2),
|
||||
(Some(turn_ids[0].clone()), None, 1),
|
||||
(None, Some(turn_ids[0].clone()), 0),
|
||||
] {
|
||||
let fork_id = mcp
|
||||
.send_thread_fork_request(ThreadForkParams {
|
||||
thread_id: source_thread.id.clone(),
|
||||
last_turn_id,
|
||||
before_turn_id,
|
||||
defer_goal_continuation: true,
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let fork_response: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(fork_id)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadForkResponse {
|
||||
thread: forked_thread,
|
||||
..
|
||||
} = to_response::<ThreadForkResponse>(fork_response)?;
|
||||
let forked_thread_id = ThreadId::from_string(&forked_thread.id)?;
|
||||
assert_eq!(forked_thread.turns.len(), expected_turn_count);
|
||||
let mut expected_goal = source_goal.clone();
|
||||
expected_goal.thread_id = forked_thread_id;
|
||||
assert_eq!(
|
||||
state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(forked_thread_id)
|
||||
.await?,
|
||||
Some(expected_goal)
|
||||
);
|
||||
assert!(
|
||||
state_db
|
||||
.thread_goals()
|
||||
.has_thread_goal_continuation_deferral(forked_thread_id)
|
||||
.await?
|
||||
);
|
||||
forked_threads.push(forked_thread);
|
||||
}
|
||||
|
||||
assert_eq!(
|
||||
state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(source_thread_id)
|
||||
.await?,
|
||||
Some(source_goal.clone())
|
||||
);
|
||||
assert!(
|
||||
!mcp.pending_notification_methods()
|
||||
.iter()
|
||||
.any(|method| method == "turn/started"),
|
||||
"deferred goal should not start a turn while forking"
|
||||
);
|
||||
assert_eq!(
|
||||
server
|
||||
.received_requests()
|
||||
.await
|
||||
.expect("wiremock requests")
|
||||
.iter()
|
||||
.filter(|request| request.url.path().ends_with("/responses"))
|
||||
.count(),
|
||||
2,
|
||||
"deferred goal should not issue a model request while forking"
|
||||
);
|
||||
|
||||
let forked_thread = forked_threads.pop().expect("empty-prefix fork");
|
||||
let forked_thread_id = ThreadId::from_string(&forked_thread.id)?;
|
||||
drop(mcp);
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_managed_config()
|
||||
.build()
|
||||
.await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
let resume_id = mcp
|
||||
.send_thread_resume_request(ThreadResumeParams {
|
||||
thread_id: forked_thread.id.clone(),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(resume_id)),
|
||||
)
|
||||
.await??;
|
||||
assert!(
|
||||
!mcp.pending_notification_methods()
|
||||
.iter()
|
||||
.any(|method| method == "turn/started"),
|
||||
"deferred goal should remain deferred after app-server restart"
|
||||
);
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.start_turn_and_wait_for_completion(TurnStartParams {
|
||||
thread_id: forked_thread.id,
|
||||
input: vec![UserInput::Text {
|
||||
text: "retry the interrupted prompt".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
..Default::default()
|
||||
}),
|
||||
)
|
||||
.await??;
|
||||
|
||||
assert!(
|
||||
!state_db
|
||||
.thread_goals()
|
||||
.has_thread_goal_continuation_deferral(forked_thread_id)
|
||||
.await?,
|
||||
"first explicit turn should consume the deferred-goal marker"
|
||||
);
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("turn/started"),
|
||||
)
|
||||
.await??;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("turn/completed"),
|
||||
)
|
||||
.await??;
|
||||
|
||||
let forked_goal = state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(forked_thread_id)
|
||||
.await?
|
||||
.expect("forked goal");
|
||||
assert_eq!(forked_goal.goal_id, source_goal.goal_id);
|
||||
assert_eq!(forked_goal.objective, source_goal.objective);
|
||||
assert_eq!(forked_goal.token_budget, Some(150));
|
||||
assert_eq!(forked_goal.tokens_used, 157);
|
||||
assert!(forked_goal.time_used_seconds >= source_goal.time_used_seconds);
|
||||
assert_eq!(
|
||||
forked_goal.status,
|
||||
codex_state::ThreadGoalStatus::BudgetLimited
|
||||
);
|
||||
assert_eq!(
|
||||
state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(source_thread_id)
|
||||
.await?,
|
||||
Some(source_goal)
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_fork_inherits_explicit_source_name_from_session_index() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
@@ -473,6 +752,90 @@ async fn thread_fork_can_load_source_by_path() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_fork_can_cut_before_unfinished_stored_turn() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri())?;
|
||||
|
||||
let filename_ts = "2025-01-05T12-00-00";
|
||||
let conversation_id = create_fake_rollout(
|
||||
codex_home.path(),
|
||||
filename_ts,
|
||||
"2025-01-05T12:00:00Z",
|
||||
"Saved user message",
|
||||
Some("mock_provider"),
|
||||
/*git_info*/ None,
|
||||
)?;
|
||||
let source_path = rollout_path(codex_home.path(), filename_ts, &conversation_id);
|
||||
let unfinished_turn_id = "unfinished-turn";
|
||||
append_rollout_item_to_path(
|
||||
&source_path,
|
||||
&RolloutItem::EventMsg(EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: unfinished_turn_id.to_string(),
|
||||
trace_id: None,
|
||||
started_at: None,
|
||||
model_context_window: None,
|
||||
collaboration_mode_kind: Default::default(),
|
||||
})),
|
||||
)
|
||||
.await?;
|
||||
append_rollout_item_to_path(
|
||||
&source_path,
|
||||
&RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent {
|
||||
message: "Unfinished user message".to_string(),
|
||||
..Default::default()
|
||||
})),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_auto_env()
|
||||
.build()
|
||||
.await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let read_id = mcp
|
||||
.send_thread_read_request(ThreadReadParams {
|
||||
thread_id: conversation_id.clone(),
|
||||
include_turns: true,
|
||||
})
|
||||
.await?;
|
||||
let read_response: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(read_id)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadReadResponse {
|
||||
thread: source_thread,
|
||||
} = to_response::<ThreadReadResponse>(read_response)?;
|
||||
assert_eq!(source_thread.turns.len(), 2);
|
||||
assert_eq!(source_thread.turns[1].id, unfinished_turn_id);
|
||||
assert_eq!(source_thread.turns[1].status, TurnStatus::Interrupted);
|
||||
|
||||
let fork_id = mcp
|
||||
.send_thread_fork_request(ThreadForkParams {
|
||||
thread_id: conversation_id,
|
||||
before_turn_id: Some(unfinished_turn_id.to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let fork_response: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(fork_id)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadForkResponse {
|
||||
thread: forked_thread,
|
||||
..
|
||||
} = to_response::<ThreadForkResponse>(fork_response)?;
|
||||
assert_eq!(forked_thread.turns.len(), 1);
|
||||
assert_eq!(forked_thread.preview, "Saved user message");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_fork_emits_restored_token_usage_before_next_turn() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
@@ -1052,6 +1415,58 @@ async fn thread_fork_ephemeral_remains_pathless_and_omits_listing() -> Result<()
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_fork_rejects_incompatible_boundaries_and_ephemeral_goal_deferral() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri())?;
|
||||
let thread_id = create_fake_rollout(
|
||||
codex_home.path(),
|
||||
"2025-01-05T12-00-00",
|
||||
"2025-01-05T12:00:00Z",
|
||||
"Saved user message",
|
||||
Some("mock_provider"),
|
||||
/*git_info*/ None,
|
||||
)?;
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.without_auto_env()
|
||||
.build()
|
||||
.await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
for (params, expected_message) in [
|
||||
(
|
||||
ThreadForkParams {
|
||||
thread_id: thread_id.clone(),
|
||||
last_turn_id: Some("turn-1".to_string()),
|
||||
before_turn_id: Some("turn-2".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
"`beforeTurnId` cannot be combined with `lastTurnId`",
|
||||
),
|
||||
(
|
||||
ThreadForkParams {
|
||||
thread_id: thread_id.clone(),
|
||||
ephemeral: true,
|
||||
defer_goal_continuation: true,
|
||||
..Default::default()
|
||||
},
|
||||
"`deferGoalContinuation` cannot be combined with `ephemeral`",
|
||||
),
|
||||
] {
|
||||
let fork_id = mcp.send_thread_fork_request(params).await?;
|
||||
let error = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_error_message(RequestId::Integer(fork_id)),
|
||||
)
|
||||
.await??;
|
||||
assert_eq!(error.error.message, expected_message);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn pathless_ephemeral_thread_rejects_codex_home_path_after_reload() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
|
||||
@@ -148,6 +148,7 @@ pub use state_db_bridge::StateDbHandle;
|
||||
pub use state_db_bridge::init_state_db;
|
||||
mod thread_rollout_truncation;
|
||||
pub use thread_rollout_truncation::truncate_rollout_after_turn_id;
|
||||
pub use thread_rollout_truncation::truncate_rollout_before_turn_id;
|
||||
mod tools;
|
||||
pub(crate) mod turn_diff_tracker;
|
||||
mod turn_metadata;
|
||||
|
||||
@@ -205,6 +205,35 @@ pub fn truncate_rollout_after_turn_id(
|
||||
Ok(items[..cut_index].to_vec())
|
||||
}
|
||||
|
||||
/// Return a rollout prefix ending immediately before the requested persisted turn.
|
||||
pub fn truncate_rollout_before_turn_id(
|
||||
items: &[RolloutItem],
|
||||
before_turn_id: &str,
|
||||
) -> CodexResult<Vec<RolloutItem>> {
|
||||
let turns = build_turns_from_rollout_items(items);
|
||||
if !turns.iter().any(|turn| turn.id == before_turn_id) {
|
||||
return Err(CodexErr::InvalidRequest(format!(
|
||||
"beforeTurnId '{before_turn_id}' was not found in the source thread"
|
||||
)));
|
||||
}
|
||||
|
||||
let cut_index = items
|
||||
.iter()
|
||||
.position(|item| {
|
||||
matches!(
|
||||
item,
|
||||
RolloutItem::EventMsg(EventMsg::TurnStarted(event))
|
||||
if event.turn_id == before_turn_id
|
||||
)
|
||||
})
|
||||
.ok_or_else(|| {
|
||||
CodexErr::InvalidRequest(format!(
|
||||
"beforeTurnId '{before_turn_id}' is not a persisted canonical turn in the source thread"
|
||||
))
|
||||
})?;
|
||||
Ok(items[..cut_index].to_vec())
|
||||
}
|
||||
|
||||
/// Return a suffix of `items` that keeps the last `n_from_end` fork turns.
|
||||
///
|
||||
/// If fewer than or equal to `n_from_end` fork turns exist, this keeps from the first fork-turn
|
||||
|
||||
@@ -112,6 +112,45 @@ fn truncates_rollout_after_terminal_canonical_turn_id() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn truncates_rollout_before_terminal_canonical_turn_id() {
|
||||
let rollout = vec![
|
||||
turn_started("turn-1"),
|
||||
turn_completed("turn-1"),
|
||||
turn_started("turn-2"),
|
||||
turn_completed("turn-2"),
|
||||
];
|
||||
|
||||
let truncated =
|
||||
truncate_rollout_before_turn_id(&rollout, "turn-2").expect("truncate before turn-2");
|
||||
assert_eq!(
|
||||
serde_json::to_value(&truncated).unwrap(),
|
||||
serde_json::to_value(&rollout[..2]).unwrap()
|
||||
);
|
||||
assert!(
|
||||
truncate_rollout_before_turn_id(&rollout, "turn-1")
|
||||
.expect("truncate before turn-1")
|
||||
.is_empty()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn truncates_rollout_before_in_progress_canonical_turn_id() {
|
||||
let rollout = vec![
|
||||
turn_started("turn-1"),
|
||||
turn_completed("turn-1"),
|
||||
turn_started("turn-2"),
|
||||
];
|
||||
|
||||
let truncated = truncate_rollout_before_turn_id(&rollout, "turn-2")
|
||||
.expect("truncate before in-progress turn-2");
|
||||
|
||||
assert_eq!(
|
||||
serde_json::to_value(&truncated).unwrap(),
|
||||
serde_json::to_value(&rollout[..2]).unwrap()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn truncate_rollout_after_turn_id_rejects_rolled_back_turn() {
|
||||
let rollout = vec![
|
||||
|
||||
@@ -93,6 +93,40 @@ impl GoalService {
|
||||
Self::default()
|
||||
}
|
||||
|
||||
/// Restores persisted goal state into the registered runtime for `thread_id`.
|
||||
pub async fn restore_thread_runtime_after_resume(
|
||||
&self,
|
||||
thread_id: ThreadId,
|
||||
) -> Result<(), GoalServiceError> {
|
||||
let runtime = self.runtime_for_thread(thread_id).ok_or_else(|| {
|
||||
GoalServiceError::Internal(format!(
|
||||
"goal runtime is unavailable for thread {thread_id}"
|
||||
))
|
||||
})?;
|
||||
runtime
|
||||
.restore_after_resume()
|
||||
.await
|
||||
.map_err(GoalServiceError::Internal)
|
||||
}
|
||||
|
||||
/// Flushes any in-flight goal accounting before a fork copies the source goal snapshot.
|
||||
pub async fn flush_thread_goal_progress_for_fork(
|
||||
&self,
|
||||
thread_id: ThreadId,
|
||||
) -> Result<(), GoalServiceError> {
|
||||
let Some(runtime) = self.runtime_for_thread(thread_id) else {
|
||||
return Ok(());
|
||||
};
|
||||
let _goal_state_permit = runtime
|
||||
.goal_state_permit()
|
||||
.await
|
||||
.map_err(GoalServiceError::Internal)?;
|
||||
runtime
|
||||
.prepare_external_goal_mutation()
|
||||
.await
|
||||
.map_err(GoalServiceError::Internal)
|
||||
}
|
||||
|
||||
pub async fn get_thread_goal(
|
||||
&self,
|
||||
state_db: &codex_state::StateRuntime,
|
||||
|
||||
@@ -207,6 +207,15 @@ where
|
||||
return;
|
||||
}
|
||||
|
||||
if let Err(err) = self
|
||||
.state_dbs
|
||||
.thread_goals()
|
||||
.clear_thread_goal_continuation_deferral(runtime.thread_id())
|
||||
.await
|
||||
{
|
||||
tracing::warn!("failed to clear deferred goal continuation: {err}");
|
||||
}
|
||||
|
||||
let accounting = runtime.accounting_state();
|
||||
accounting.start_turn(
|
||||
input.turn_id,
|
||||
|
||||
@@ -365,6 +365,17 @@ impl GoalRuntimeHandle {
|
||||
// change the goal after we read it but before the continuation launches.
|
||||
let _goal_state_permit = self.goal_state_permit().await?;
|
||||
|
||||
if self
|
||||
.inner
|
||||
.state_dbs
|
||||
.thread_goals()
|
||||
.has_thread_goal_continuation_deferral(self.thread_id())
|
||||
.await
|
||||
.map_err(|err| err.to_string())?
|
||||
{
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let Some(thread_manager) = self.inner.thread_manager.upgrade() else {
|
||||
tracing::debug!("skipping goal continuation because thread manager is unavailable");
|
||||
return Ok(());
|
||||
|
||||
@@ -0,0 +1,3 @@
|
||||
CREATE TABLE thread_goal_continuation_deferrals (
|
||||
thread_id TEXT PRIMARY KEY NOT NULL REFERENCES thread_goals(thread_id) ON DELETE CASCADE
|
||||
);
|
||||
@@ -65,6 +65,94 @@ WHERE thread_id = ?
|
||||
row.map(|row| thread_goal_from_row(&row)).transpose()
|
||||
}
|
||||
|
||||
pub async fn replace_thread_goal_snapshot(
|
||||
&self,
|
||||
goal: &crate::ThreadGoal,
|
||||
) -> anyhow::Result<()> {
|
||||
let mut transaction = self.pool.begin().await?;
|
||||
sqlx::query(
|
||||
r#"
|
||||
INSERT INTO thread_goals (
|
||||
thread_id,
|
||||
goal_id,
|
||||
objective,
|
||||
status,
|
||||
token_budget,
|
||||
tokens_used,
|
||||
time_used_seconds,
|
||||
created_at_ms,
|
||||
updated_at_ms
|
||||
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
ON CONFLICT(thread_id) DO UPDATE SET
|
||||
goal_id = excluded.goal_id,
|
||||
objective = excluded.objective,
|
||||
status = excluded.status,
|
||||
token_budget = excluded.token_budget,
|
||||
tokens_used = excluded.tokens_used,
|
||||
time_used_seconds = excluded.time_used_seconds,
|
||||
created_at_ms = excluded.created_at_ms,
|
||||
updated_at_ms = excluded.updated_at_ms
|
||||
"#,
|
||||
)
|
||||
.bind(goal.thread_id.to_string())
|
||||
.bind(&goal.goal_id)
|
||||
.bind(&goal.objective)
|
||||
.bind(goal.status.as_str())
|
||||
.bind(goal.token_budget)
|
||||
.bind(goal.tokens_used)
|
||||
.bind(goal.time_used_seconds)
|
||||
.bind(datetime_to_epoch_millis(goal.created_at))
|
||||
.bind(datetime_to_epoch_millis(goal.updated_at))
|
||||
.execute(&mut *transaction)
|
||||
.await?;
|
||||
|
||||
sqlx::query(
|
||||
r#"
|
||||
INSERT INTO thread_goal_continuation_deferrals (thread_id)
|
||||
VALUES (?)
|
||||
ON CONFLICT(thread_id) DO NOTHING
|
||||
"#,
|
||||
)
|
||||
.bind(goal.thread_id.to_string())
|
||||
.execute(&mut *transaction)
|
||||
.await?;
|
||||
|
||||
transaction.commit().await?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub async fn has_thread_goal_continuation_deferral(
|
||||
&self,
|
||||
thread_id: ThreadId,
|
||||
) -> anyhow::Result<bool> {
|
||||
sqlx::query_scalar(
|
||||
r#"
|
||||
SELECT EXISTS(
|
||||
SELECT 1
|
||||
FROM thread_goal_continuation_deferrals
|
||||
WHERE thread_id = ?
|
||||
)
|
||||
"#,
|
||||
)
|
||||
.bind(thread_id.to_string())
|
||||
.fetch_one(self.pool.as_ref())
|
||||
.await
|
||||
.map_err(Into::into)
|
||||
}
|
||||
|
||||
pub async fn clear_thread_goal_continuation_deferral(
|
||||
&self,
|
||||
thread_id: ThreadId,
|
||||
) -> anyhow::Result<()> {
|
||||
sqlx::query("DELETE FROM thread_goal_continuation_deferrals WHERE thread_id = ?")
|
||||
.bind(thread_id.to_string())
|
||||
.execute(self.pool.as_ref())
|
||||
.await?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub async fn replace_thread_goal(
|
||||
&self,
|
||||
thread_id: ThreadId,
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
use super::resize_reflow::trailing_run_start;
|
||||
use super::session_lifecycle::ThreadAttachPresentation;
|
||||
use super::*;
|
||||
use crate::app_server_session::ForkGoalContinuation;
|
||||
use crate::config_update::format_config_error;
|
||||
use crate::external_agent_config_migration_flow::ExternalAgentConfigMigrationFlowOutcome;
|
||||
#[cfg(target_os = "windows")]
|
||||
@@ -249,14 +250,20 @@ impl App {
|
||||
.thread_read(thread_id, /*include_turns*/ true)
|
||||
.await
|
||||
{
|
||||
Ok(thread) => match crate::app_backtrack::backtrack_fork_last_turn_id(
|
||||
Ok(thread) => match crate::app_backtrack::backtrack_fork_before_turn_id(
|
||||
&thread.turns,
|
||||
nth_user_message,
|
||||
&mut prompt,
|
||||
) {
|
||||
Ok(Some(last_turn_id)) => {
|
||||
Ok(Some(before_turn_id)) => {
|
||||
app_server
|
||||
.fork_thread_after(config.clone(), thread_id, last_turn_id)
|
||||
.fork_thread_at(
|
||||
config.clone(),
|
||||
thread_id,
|
||||
/*last_turn_id*/ None,
|
||||
/*before_turn_id*/ Some(before_turn_id),
|
||||
ForkGoalContinuation::StartIfIdle,
|
||||
)
|
||||
.await
|
||||
}
|
||||
Ok(None) => {
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
use super::session_lifecycle::ThreadAttachPresentation;
|
||||
use super::*;
|
||||
use crate::app_server_session::ForkGoalContinuation;
|
||||
use crate::chatwidget::ThreadInputState;
|
||||
use crate::chatwidget::ThreadInputStateRestoreMode;
|
||||
use crate::chatwidget::UserMessage;
|
||||
@@ -15,11 +16,6 @@ pub(super) struct SafetyBufferedRetry {
|
||||
pub(super) prompt: UserMessage,
|
||||
}
|
||||
|
||||
enum SafetyRetryForkPoint {
|
||||
AfterTurn(String),
|
||||
NewThread,
|
||||
}
|
||||
|
||||
impl App {
|
||||
pub(super) async fn retry_safety_buffered_turn(
|
||||
&mut self,
|
||||
@@ -90,13 +86,10 @@ impl App {
|
||||
return;
|
||||
}
|
||||
};
|
||||
let fork_point = match safety_retry_fork_point(&thread.turns, &turn_id) {
|
||||
Ok(fork_point) => fork_point,
|
||||
Err(err) => {
|
||||
self.fail_safety_buffered_branch(input_state, prompt, err);
|
||||
return;
|
||||
}
|
||||
};
|
||||
if let Err(err) = safety_retry_fork_point(&thread.turns, &turn_id) {
|
||||
self.fail_safety_buffered_branch(input_state, prompt, err);
|
||||
return;
|
||||
}
|
||||
items.extend(
|
||||
thread
|
||||
.turns
|
||||
@@ -117,22 +110,18 @@ impl App {
|
||||
.chain(content.iter().cloned())
|
||||
}),
|
||||
);
|
||||
let retry_display = ChatWidget::user_message_display_from_inputs(items);
|
||||
|
||||
self.config = retry_config.clone();
|
||||
let started = match fork_point {
|
||||
SafetyRetryForkPoint::AfterTurn(last_turn_id) => {
|
||||
app_server
|
||||
.fork_thread_after(retry_config, thread_id, last_turn_id)
|
||||
.await
|
||||
}
|
||||
SafetyRetryForkPoint::NewThread => {
|
||||
app_server
|
||||
.start_thread_with_session_start_source(
|
||||
&retry_config,
|
||||
/*session_start_source*/ None,
|
||||
)
|
||||
.await
|
||||
}
|
||||
};
|
||||
let started = app_server
|
||||
.fork_thread_at(
|
||||
retry_config,
|
||||
thread_id,
|
||||
/*last_turn_id*/ None,
|
||||
/*before_turn_id*/ Some(turn_id),
|
||||
ForkGoalContinuation::DeferUntilNextTurn,
|
||||
)
|
||||
.await;
|
||||
let started = match started {
|
||||
Ok(started) => started,
|
||||
Err(err) => {
|
||||
@@ -174,7 +163,7 @@ impl App {
|
||||
return;
|
||||
}
|
||||
self.chat_widget
|
||||
.commit_safety_buffered_retry_submission(prompt);
|
||||
.commit_safety_buffered_retry_submission(retry_display);
|
||||
}
|
||||
|
||||
fn fail_safety_buffered_branch(
|
||||
@@ -196,7 +185,7 @@ impl App {
|
||||
}
|
||||
}
|
||||
|
||||
fn safety_retry_fork_point(turns: &[Turn], turn_id: &str) -> Result<SafetyRetryForkPoint> {
|
||||
fn safety_retry_fork_point(turns: &[Turn], turn_id: &str) -> Result<()> {
|
||||
let Some(turn_index) = turns.iter().position(|turn| turn.id == turn_id) else {
|
||||
return Err(color_eyre::eyre::eyre!(
|
||||
"interrupted turn {turn_id} is missing from the source thread"
|
||||
@@ -214,7 +203,7 @@ fn safety_retry_fork_point(turns: &[Turn], turn_id: &str) -> Result<SafetyRetryF
|
||||
}
|
||||
|
||||
let Some(previous_turn) = turns[..turn_index].last() else {
|
||||
return Ok(SafetyRetryForkPoint::NewThread);
|
||||
return Ok(());
|
||||
};
|
||||
if previous_turn.status == TurnStatus::InProgress {
|
||||
return Err(color_eyre::eyre::eyre!(
|
||||
@@ -223,7 +212,7 @@ fn safety_retry_fork_point(turns: &[Turn], turn_id: &str) -> Result<SafetyRetryF
|
||||
));
|
||||
}
|
||||
|
||||
Ok(SafetyRetryForkPoint::AfterTurn(previous_turn.id.clone()))
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -7,6 +7,7 @@ use codex_app_server_protocol::ModelSafetyBufferingUpdatedNotification;
|
||||
use core_test_support::responses;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_completed_with_tokens;
|
||||
use core_test_support::responses::ev_response_created;
|
||||
use core_test_support::streaming_sse::StreamingSseChunk;
|
||||
use core_test_support::streaming_sse::StreamingSseServer;
|
||||
@@ -22,6 +23,14 @@ const PREVIOUS_PROMPT: &str = "Establish context";
|
||||
const RETRY_PROMPT: &str = "Handle the safety-buffered request";
|
||||
const COMMITTED_STEER: &str = "Keep the accepted steer";
|
||||
const UNSENT_DRAFT: &str = "Keep this unsent draft";
|
||||
const RETRY_GOAL: &str = "Preserve this goal across the retry";
|
||||
|
||||
#[derive(Clone, Copy, PartialEq, Eq)]
|
||||
enum SafetyRetryScenario {
|
||||
Once,
|
||||
RetryTwice,
|
||||
InterruptedPrevious,
|
||||
}
|
||||
|
||||
fn response_chunks(response_id: &str) -> Vec<StreamingSseChunk> {
|
||||
[
|
||||
@@ -37,7 +46,10 @@ fn response_chunks(response_id: &str) -> Vec<StreamingSseChunk> {
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn gated_response_chunks(response_id: &str) -> (Vec<StreamingSseChunk>, oneshot::Sender<()>) {
|
||||
fn gated_response_chunks(
|
||||
response_id: &str,
|
||||
completed: Value,
|
||||
) -> (Vec<StreamingSseChunk>, oneshot::Sender<()>) {
|
||||
let (release_tx, release_rx) = oneshot::channel();
|
||||
(
|
||||
vec![
|
||||
@@ -47,7 +59,7 @@ fn gated_response_chunks(response_id: &str) -> (Vec<StreamingSseChunk>, oneshot:
|
||||
},
|
||||
StreamingSseChunk {
|
||||
gate: Some(release_rx),
|
||||
body: responses::sse(vec![ev_completed(response_id)]),
|
||||
body: responses::sse(vec![completed]),
|
||||
},
|
||||
],
|
||||
release_tx,
|
||||
@@ -191,19 +203,45 @@ async fn run_safety_retry(
|
||||
previous_prompt: Option<&str>,
|
||||
failing_draft: Option<&str>,
|
||||
committed_steer: Option<&str>,
|
||||
scenario: SafetyRetryScenario,
|
||||
) -> Result<()> {
|
||||
let (active_chunks, release_active_response) = gated_response_chunks("active-response");
|
||||
let (active_chunks, release_active_response) = gated_response_chunks(
|
||||
"active-response",
|
||||
ev_completed_with_tokens("active-response", /*total_tokens*/ 100),
|
||||
);
|
||||
let mut release_active_response = Some(release_active_response);
|
||||
let (steered_chunks, release_steered_response) = gated_response_chunks("steered-response");
|
||||
let (steered_chunks, release_steered_response) =
|
||||
gated_response_chunks("steered-response", ev_completed("steered-response"));
|
||||
let (previous_chunks, release_previous_response) =
|
||||
gated_response_chunks("previous-response", ev_completed("previous-response"));
|
||||
let (retry_chunks, release_retry_response) =
|
||||
gated_response_chunks("retry-response", ev_completed("retry-response"));
|
||||
let mut response_sequences = Vec::new();
|
||||
if previous_prompt.is_some() {
|
||||
response_sequences.push(response_chunks("previous-response"));
|
||||
if scenario == SafetyRetryScenario::InterruptedPrevious {
|
||||
response_sequences.push(previous_chunks);
|
||||
} else {
|
||||
response_sequences.push(response_chunks("previous-response"));
|
||||
}
|
||||
}
|
||||
response_sequences.push(active_chunks);
|
||||
if committed_steer.is_some() {
|
||||
response_sequences.push(steered_chunks);
|
||||
}
|
||||
response_sequences.push(response_chunks("retry-response"));
|
||||
if scenario == SafetyRetryScenario::RetryTwice {
|
||||
response_sequences.push(retry_chunks);
|
||||
response_sequences.push(response_chunks("second-retry-response"));
|
||||
} else {
|
||||
response_sequences.push(response_chunks("retry-response"));
|
||||
}
|
||||
response_sequences.push(vec![StreamingSseChunk {
|
||||
gate: None,
|
||||
body: responses::sse(vec![
|
||||
ev_response_created("goal-continuation-response"),
|
||||
ev_assistant_message("goal-continuation-message", "done"),
|
||||
ev_completed_with_tokens("goal-continuation-response", /*total_tokens*/ 1_000),
|
||||
]),
|
||||
}]);
|
||||
let expected_request_count = response_sequences.len();
|
||||
let (server, _completions) = start_streaming_sse_server(response_sequences).await;
|
||||
|
||||
@@ -222,6 +260,9 @@ base_url = "{}/v1"
|
||||
wire_api = "responses"
|
||||
request_max_retries = 0
|
||||
stream_max_retries = 0
|
||||
|
||||
[features]
|
||||
goals = true
|
||||
"#,
|
||||
server.uri()
|
||||
),
|
||||
@@ -237,6 +278,10 @@ stream_max_retries = 0
|
||||
stream_max_retries: Some(0),
|
||||
..ModelProviderInfo::default()
|
||||
};
|
||||
app.config
|
||||
.features
|
||||
.enable(Feature::Goals)
|
||||
.expect("test config should allow goals");
|
||||
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(&app.config)).await?;
|
||||
@@ -257,9 +302,59 @@ stream_max_retries = 0
|
||||
let previous_turn = next_user_turn_event(&mut app_event_rx);
|
||||
app.submit_thread_op(&mut app_server, source_thread_id, previous_turn)
|
||||
.await?;
|
||||
wait_for_turn_completed(&mut app, &mut app_server, source_thread_id).await;
|
||||
if scenario == SafetyRetryScenario::InterruptedPrevious {
|
||||
let previous_turn_id =
|
||||
next_turn_started(&mut app, &mut app_server, source_thread_id).await;
|
||||
drive_until_request_count(
|
||||
&mut app,
|
||||
&mut app_server,
|
||||
&server,
|
||||
/*expected_request_count*/ 1,
|
||||
)
|
||||
.await;
|
||||
app_server
|
||||
.turn_interrupt(source_thread_id, previous_turn_id)
|
||||
.await?;
|
||||
} else {
|
||||
wait_for_turn_completed(&mut app, &mut app_server, source_thread_id).await;
|
||||
}
|
||||
}
|
||||
|
||||
let state_db = codex_state::StateRuntime::init(
|
||||
app.config.sqlite_home.clone(),
|
||||
app.config.model_provider_id.clone(),
|
||||
)
|
||||
.await
|
||||
.expect("state db should initialize");
|
||||
state_db
|
||||
.thread_goals()
|
||||
.replace_thread_goal(
|
||||
source_thread_id,
|
||||
RETRY_GOAL,
|
||||
codex_state::ThreadGoalStatus::Active,
|
||||
/*token_budget*/ Some(1_000),
|
||||
)
|
||||
.await
|
||||
.expect("source goal should be stored");
|
||||
let source_goal_id = state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(source_thread_id)
|
||||
.await
|
||||
.expect("source goal should be readable")
|
||||
.expect("source goal")
|
||||
.goal_id;
|
||||
state_db
|
||||
.thread_goals()
|
||||
.account_thread_goal_usage(
|
||||
source_thread_id,
|
||||
/*time_delta_seconds*/ 12,
|
||||
/*token_delta*/ 50,
|
||||
codex_state::GoalAccountingMode::ActiveOrStopped,
|
||||
Some(source_goal_id.as_str()),
|
||||
)
|
||||
.await
|
||||
.expect("source goal usage should be recorded");
|
||||
|
||||
submit_prompt(&mut app, RETRY_PROMPT);
|
||||
let active_turn = next_user_turn_event(&mut app_event_rx);
|
||||
app.submit_thread_op(&mut app_server, source_thread_id, active_turn.clone())
|
||||
@@ -341,10 +436,12 @@ stream_max_retries = 0
|
||||
},
|
||||
))
|
||||
.await;
|
||||
|
||||
assert_eq!(app.primary_thread_id, Some(primary_thread_id));
|
||||
assert_eq!(app.active_thread_id, Some(source_thread_id));
|
||||
assert_eq!(app.chat_widget.thread_id(), Some(source_thread_id));
|
||||
app.primary_thread_id = Some(source_thread_id);
|
||||
while app_event_rx.try_recv().is_ok() {}
|
||||
|
||||
Box::pin(app.retry_safety_buffered_turn(
|
||||
&mut tui,
|
||||
@@ -359,6 +456,69 @@ stream_max_retries = 0
|
||||
))
|
||||
.await;
|
||||
|
||||
let first_retry_thread_id = app.chat_widget.thread_id().expect("first retry thread id");
|
||||
if scenario == SafetyRetryScenario::RetryTwice {
|
||||
let first_retry_turn_id =
|
||||
next_turn_started(&mut app, &mut app_server, first_retry_thread_id).await;
|
||||
drive_until_request_count(
|
||||
&mut app,
|
||||
&mut app_server,
|
||||
&server,
|
||||
/*expected_request_count*/ 2,
|
||||
)
|
||||
.await;
|
||||
app.handle_app_server_event(
|
||||
&app_server,
|
||||
AppServerEvent::ServerNotification(ServerNotification::ModelSafetyBufferingUpdated(
|
||||
ModelSafetyBufferingUpdatedNotification {
|
||||
thread_id: first_retry_thread_id.to_string(),
|
||||
turn_id: first_retry_turn_id.clone(),
|
||||
model: FASTER_MODEL.to_string(),
|
||||
use_cases: Vec::new(),
|
||||
reasons: Vec::new(),
|
||||
show_buffering_ui: true,
|
||||
faster_model: Some(FASTER_MODEL.to_string()),
|
||||
},
|
||||
)),
|
||||
)
|
||||
.await;
|
||||
drain_active_thread_events(&mut app);
|
||||
assert!(
|
||||
app.chat_widget
|
||||
.can_retry_safety_buffered_turn(&first_retry_turn_id)
|
||||
);
|
||||
app.chat_widget
|
||||
.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
let second_retry = loop {
|
||||
match app_event_rx.try_recv() {
|
||||
Ok(AppEvent::RetrySafetyBufferedTurn {
|
||||
thread_id,
|
||||
turn_id,
|
||||
model,
|
||||
turn,
|
||||
prompt,
|
||||
}) => {
|
||||
break SafetyBufferedRetry {
|
||||
thread_id,
|
||||
turn_id,
|
||||
model,
|
||||
turn,
|
||||
prompt,
|
||||
};
|
||||
}
|
||||
Ok(_) => continue,
|
||||
Err(err) => panic!("expected second safety-buffering retry event: {err}"),
|
||||
}
|
||||
};
|
||||
let AppCommand::UserTurn { items, .. } = &second_retry.turn else {
|
||||
panic!("second safety-buffering retry should retain the user turn");
|
||||
};
|
||||
assert!(items.iter().any(
|
||||
|item| matches!(item, AppServerUserInput::Text { text, .. } if text == RETRY_PROMPT)
|
||||
));
|
||||
Box::pin(app.retry_safety_buffered_turn(&mut tui, &mut app_server, second_retry)).await;
|
||||
}
|
||||
|
||||
if let Some(draft) = failing_draft {
|
||||
assert_eq!(
|
||||
app.chat_widget.composer_text_with_pending(),
|
||||
@@ -383,7 +543,27 @@ stream_max_retries = 0
|
||||
));
|
||||
}
|
||||
}
|
||||
assert!(!replayed_history.contains("Conversation interrupted"));
|
||||
assert_eq!(
|
||||
replayed_history.contains("Conversation interrupted"),
|
||||
scenario == SafetyRetryScenario::InterruptedPrevious
|
||||
);
|
||||
let forked_history = replayed_history
|
||||
.rsplit_once("Thread forked from")
|
||||
.expect("safety retry should render fork lineage")
|
||||
.1;
|
||||
assert_eq!(
|
||||
forked_history.matches(RETRY_PROMPT).count(),
|
||||
1,
|
||||
"the fork should render the retried prompt once: {forked_history}"
|
||||
);
|
||||
if committed_steer.is_some() {
|
||||
let rendered_retry = forked_history
|
||||
.lines()
|
||||
.skip_while(|line| !line.contains(RETRY_PROMPT))
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
insta::assert_snapshot!("safety_retry_committed_steer_history", rendered_retry);
|
||||
}
|
||||
|
||||
let retry_thread_id = app.chat_widget.thread_id().expect("retry thread id");
|
||||
let source = app_server
|
||||
@@ -397,10 +577,17 @@ stream_max_retries = 0
|
||||
source.turns.last().map(|turn| &turn.status),
|
||||
Some(&TurnStatus::Interrupted)
|
||||
);
|
||||
let expected_forked_from_id = previous_prompt.map(|_| source_thread_id.to_string());
|
||||
assert_eq!(
|
||||
retry.forked_from_id.as_deref(),
|
||||
expected_forked_from_id.as_deref()
|
||||
Some(
|
||||
if scenario == SafetyRetryScenario::RetryTwice {
|
||||
first_retry_thread_id
|
||||
} else {
|
||||
source_thread_id
|
||||
}
|
||||
.to_string()
|
||||
)
|
||||
.as_deref()
|
||||
);
|
||||
let expected_retry_prompt = match committed_steer {
|
||||
Some(committed_steer) => format!("{RETRY_PROMPT}\n{committed_steer}"),
|
||||
@@ -416,14 +603,34 @@ stream_max_retries = 0
|
||||
assert_eq!(user_message_count(&retry, previous_prompt), 1);
|
||||
}
|
||||
|
||||
let source_goal = app_server
|
||||
.thread_goal_get(source_thread_id)
|
||||
.await?
|
||||
.goal
|
||||
.expect("source goal");
|
||||
let retry_goal = app_server
|
||||
.thread_goal_get(retry_thread_id)
|
||||
.await?
|
||||
.goal
|
||||
.expect("retry goal");
|
||||
let expected_source_tokens = if committed_steer.is_some() { 150 } else { 50 };
|
||||
assert_eq!(source_goal.objective, RETRY_GOAL);
|
||||
assert_eq!(source_goal.tokens_used, expected_source_tokens);
|
||||
assert_eq!(source_goal.time_used_seconds, 12);
|
||||
assert_eq!(retry_goal.objective, RETRY_GOAL);
|
||||
assert!(retry_goal.tokens_used >= expected_source_tokens);
|
||||
assert!(retry_goal.time_used_seconds >= 12);
|
||||
|
||||
let request_bodies = server
|
||||
.requests()
|
||||
.await
|
||||
.iter()
|
||||
.map(|request| serde_json::from_slice::<Value>(request))
|
||||
.collect::<serde_json::Result<Vec<_>>>()?;
|
||||
let retry_request_index =
|
||||
usize::from(previous_prompt.is_some()) + usize::from(committed_steer.is_some()) + 1;
|
||||
let retry_request = request_bodies
|
||||
.last()
|
||||
.get(retry_request_index)
|
||||
.expect("retry should issue a Responses API request");
|
||||
assert_eq!(retry_request["model"].as_str(), Some(FASTER_MODEL));
|
||||
assert_eq!(retry_request["reasoning"]["effort"].as_str(), Some("low"));
|
||||
@@ -441,11 +648,47 @@ stream_max_retries = 0
|
||||
};
|
||||
expected_prompts.extend(committed_steer.map(str::to_string));
|
||||
assert_eq!(relevant_prompts, expected_prompts);
|
||||
if previous_prompt.is_none() {
|
||||
assert!(
|
||||
!user_input_texts(retry_request)
|
||||
.iter()
|
||||
.any(|text| text.contains("<turn_aborted>")),
|
||||
"first-turn safety retry should not inherit an interruption marker"
|
||||
);
|
||||
}
|
||||
let goal_continuation_request_index = if scenario == SafetyRetryScenario::RetryTwice {
|
||||
let second_retry_request = request_bodies
|
||||
.get(retry_request_index + 1)
|
||||
.expect("second retry should issue a Responses API request");
|
||||
assert!(
|
||||
user_input_texts(second_retry_request)
|
||||
.iter()
|
||||
.any(|text| text == RETRY_PROMPT),
|
||||
"second safety retry should submit the original prompt"
|
||||
);
|
||||
assert!(
|
||||
!user_input_texts(second_retry_request)
|
||||
.iter()
|
||||
.any(|text| text.contains("<turn_aborted>")),
|
||||
"second safety retry should not inherit an interruption marker"
|
||||
);
|
||||
retry_request_index + 2
|
||||
} else {
|
||||
retry_request_index + 1
|
||||
};
|
||||
assert!(
|
||||
user_input_texts(&request_bodies[goal_continuation_request_index])
|
||||
.iter()
|
||||
.any(|text| text.contains(RETRY_GOAL)),
|
||||
"inherited goal continuation should resume after the explicit retry"
|
||||
);
|
||||
|
||||
if let Some(release_active_response) = release_active_response.take() {
|
||||
let _ = release_active_response.send(());
|
||||
}
|
||||
let _ = release_steered_response.send(());
|
||||
let _ = release_previous_response.send(());
|
||||
let _ = release_retry_response.send(());
|
||||
app_server.shutdown().await?;
|
||||
server.shutdown().await;
|
||||
Ok(())
|
||||
@@ -457,6 +700,7 @@ async fn safety_retry_forks_after_the_previous_turn_and_uses_faster_settings() -
|
||||
Some(PREVIOUS_PROMPT),
|
||||
/*failing_draft*/ None,
|
||||
/*committed_steer*/ None,
|
||||
SafetyRetryScenario::Once,
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -467,14 +711,40 @@ async fn safety_retry_preserves_a_committed_steer_from_the_interrupted_turn() ->
|
||||
Some(PREVIOUS_PROMPT),
|
||||
/*failing_draft*/ None,
|
||||
Some(COMMITTED_STEER),
|
||||
SafetyRetryScenario::Once,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn safety_retry_starts_new_thread_for_first_turn_without_duplicating_prompt() -> Result<()> {
|
||||
async fn safety_retry_forks_first_turn_and_continues_without_duplicating_prompt() -> Result<()> {
|
||||
run_safety_retry(
|
||||
/*previous_prompt*/ None, /*failing_draft*/ None, /*committed_steer*/ None,
|
||||
/*previous_prompt*/ None,
|
||||
/*failing_draft*/ None,
|
||||
/*committed_steer*/ None,
|
||||
SafetyRetryScenario::Once,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn safety_retry_can_retry_a_first_turn_a_second_time() -> Result<()> {
|
||||
run_safety_retry(
|
||||
/*previous_prompt*/ None,
|
||||
/*failing_draft*/ None,
|
||||
/*committed_steer*/ None,
|
||||
SafetyRetryScenario::RetryTwice,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn safety_retry_replays_older_interruption_notices() -> Result<()> {
|
||||
run_safety_retry(
|
||||
Some(PREVIOUS_PROMPT),
|
||||
/*failing_draft*/ None,
|
||||
/*committed_steer*/ None,
|
||||
SafetyRetryScenario::InterruptedPrevious,
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -485,6 +755,7 @@ async fn safety_retry_branch_failure_preserves_unsent_draft() -> Result<()> {
|
||||
Some(PREVIOUS_PROMPT),
|
||||
Some(UNSENT_DRAFT),
|
||||
/*committed_steer*/ None,
|
||||
SafetyRetryScenario::Once,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
+7
@@ -0,0 +1,7 @@
|
||||
---
|
||||
source: tui/src/app/tests/safety_buffering.rs
|
||||
expression: rendered_retry
|
||||
---
|
||||
› Handle the safety-buffered request
|
||||
Keep the accepted steer
|
||||
• done
|
||||
@@ -4,8 +4,8 @@
|
||||
//! mediates a key rendering boundary for the transcript overlay.
|
||||
//!
|
||||
//! Overall goal: keep the main chat view and the transcript overlay in sync while allowing users
|
||||
//! to edit an earlier prompt on a source-preserving branch. Confirming a selection forks through
|
||||
//! the immediately preceding turn and restores the selected prompt in the new composer.
|
||||
//! to edit an earlier prompt on a source-preserving branch. Confirming a selection forks before
|
||||
//! the selected turn and restores its prompt in the new composer.
|
||||
//!
|
||||
//! Backtrack operates as a small state machine:
|
||||
//! - The first `Esc` in the main view "primes" the feature and captures a base thread id.
|
||||
@@ -463,14 +463,14 @@ impl App {
|
||||
}
|
||||
}
|
||||
|
||||
/// Find the persisted turn immediately before a selected transcript prompt.
|
||||
/// Find the persisted turn that contains a selected transcript prompt.
|
||||
///
|
||||
/// Replay hides review prompts and other display-empty inputs, so the selected ordinal must be
|
||||
/// resolved against the same visible projection before restoring its canonical mention bindings.
|
||||
///
|
||||
/// A turn can contain multiple user messages when it was steered. Only its initial prompt can be
|
||||
/// reopened independently because app-server cannot fork in the middle of a turn.
|
||||
pub(crate) fn backtrack_fork_last_turn_id(
|
||||
pub(crate) fn backtrack_fork_before_turn_id(
|
||||
turns: &[Turn],
|
||||
nth_user_message: usize,
|
||||
prompt: &mut UserMessage,
|
||||
@@ -478,6 +478,10 @@ pub(crate) fn backtrack_fork_last_turn_id(
|
||||
let mut visible_user_messages_seen = 0_usize;
|
||||
let mut review_mode = false;
|
||||
for (turn_index, turn) in turns.iter().enumerate() {
|
||||
let hidden_nested_review_turn = turn_index
|
||||
.checked_sub(/*rhs*/ 1)
|
||||
.and_then(|index| turns.get(index))
|
||||
.is_some_and(|previous| is_hidden_nested_review_turn(previous, turn));
|
||||
let mut user_messages_in_turn = 0_usize;
|
||||
for item in &turn.items {
|
||||
let content = match item {
|
||||
@@ -499,6 +503,9 @@ pub(crate) fn backtrack_fork_last_turn_id(
|
||||
}
|
||||
|
||||
let display = ChatWidget::user_message_display_from_inputs(content);
|
||||
if hidden_nested_review_turn {
|
||||
continue;
|
||||
}
|
||||
if display.message.trim().is_empty()
|
||||
&& display.text_elements.is_empty()
|
||||
&& display.local_images.is_empty()
|
||||
@@ -529,16 +536,44 @@ pub(crate) fn backtrack_fork_last_turn_id(
|
||||
}
|
||||
prompt.mention_bindings = mention_bindings_from_user_inputs(content, &display.message);
|
||||
|
||||
return Ok(turn_index
|
||||
.checked_sub(1)
|
||||
.and_then(|index| turns.get(index))
|
||||
.map(|turn| turn.id.clone()));
|
||||
return Ok((turn_index > 0).then(|| turn.id.clone()));
|
||||
}
|
||||
}
|
||||
|
||||
bail!("the selected prompt was not found in the persisted thread")
|
||||
}
|
||||
|
||||
/// Returns whether a turn is the reconstructed inline-review child with duplicated prompt inputs.
|
||||
pub(crate) fn is_hidden_nested_review_turn(previous: &Turn, turn: &Turn) -> bool {
|
||||
if previous.status != TurnStatus::Completed
|
||||
|| turn.status != TurnStatus::Interrupted
|
||||
|| turn.completed_at.is_some()
|
||||
|| !previous
|
||||
.items
|
||||
.iter()
|
||||
.any(|item| matches!(item, ThreadItem::EnteredReviewMode { .. }))
|
||||
|| !previous
|
||||
.items
|
||||
.iter()
|
||||
.any(|item| matches!(item, ThreadItem::ExitedReviewMode { .. }))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
let mut user_messages = turn.items.iter().filter_map(|item| match item {
|
||||
ThreadItem::UserMessage { content, .. } => Some(content),
|
||||
_ => None,
|
||||
});
|
||||
matches!(
|
||||
(
|
||||
user_messages.next(),
|
||||
user_messages.next(),
|
||||
user_messages.next(),
|
||||
),
|
||||
(Some(first), Some(second), None) if first == second
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn user_count(cells: &[Arc<dyn crate::history_cell::HistoryCell>]) -> usize {
|
||||
user_positions_iter(cells).count()
|
||||
}
|
||||
@@ -660,7 +695,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn backtrack_fork_last_turn_id_resolves_first_and_later_prompts() {
|
||||
fn backtrack_fork_before_turn_id_resolves_first_and_later_prompts() {
|
||||
let turns = vec![
|
||||
turn("turn-1", TurnStatus::Completed, /*user_messages*/ 1),
|
||||
turn(
|
||||
@@ -672,7 +707,7 @@ mod tests {
|
||||
];
|
||||
|
||||
assert_eq!(
|
||||
backtrack_fork_last_turn_id(
|
||||
backtrack_fork_before_turn_id(
|
||||
&turns,
|
||||
/*nth_user_message*/ 0,
|
||||
&mut prompt("turn-1-prompt-0"),
|
||||
@@ -681,25 +716,25 @@ mod tests {
|
||||
None
|
||||
);
|
||||
assert_eq!(
|
||||
backtrack_fork_last_turn_id(
|
||||
backtrack_fork_before_turn_id(
|
||||
&turns,
|
||||
/*nth_user_message*/ 1,
|
||||
&mut prompt("turn-2-prompt-0"),
|
||||
)
|
||||
.expect("later prompt should resolve"),
|
||||
Some("turn-compaction".to_string())
|
||||
Some("turn-2".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn backtrack_fork_last_turn_id_rejects_mid_turn_steers() {
|
||||
fn backtrack_fork_before_turn_id_rejects_mid_turn_steers() {
|
||||
let turns = vec![turn(
|
||||
"turn-1",
|
||||
TurnStatus::Completed,
|
||||
/*user_messages*/ 2,
|
||||
)];
|
||||
|
||||
let error = backtrack_fork_last_turn_id(
|
||||
let error = backtrack_fork_before_turn_id(
|
||||
&turns,
|
||||
/*nth_user_message*/ 1,
|
||||
&mut prompt("turn-1-prompt-1"),
|
||||
@@ -713,7 +748,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn backtrack_fork_last_turn_id_rejects_in_progress_and_missing_prompts() {
|
||||
fn backtrack_fork_before_turn_id_rejects_in_progress_and_missing_prompts() {
|
||||
let turns = vec![turn(
|
||||
"turn-1",
|
||||
TurnStatus::InProgress,
|
||||
@@ -721,7 +756,7 @@ mod tests {
|
||||
)];
|
||||
|
||||
assert_eq!(
|
||||
backtrack_fork_last_turn_id(
|
||||
backtrack_fork_before_turn_id(
|
||||
&turns,
|
||||
/*nth_user_message*/ 0,
|
||||
&mut prompt("turn-1-prompt-0"),
|
||||
@@ -731,7 +766,7 @@ mod tests {
|
||||
"the selected prompt belongs to a turn that is still in progress"
|
||||
);
|
||||
assert_eq!(
|
||||
backtrack_fork_last_turn_id(
|
||||
backtrack_fork_before_turn_id(
|
||||
&turns,
|
||||
/*nth_user_message*/ 1,
|
||||
&mut prompt("missing prompt"),
|
||||
@@ -747,7 +782,7 @@ mod tests {
|
||||
/*user_messages*/ 1,
|
||||
)];
|
||||
assert_eq!(
|
||||
backtrack_fork_last_turn_id(
|
||||
backtrack_fork_before_turn_id(
|
||||
&completed_turns,
|
||||
/*nth_user_message*/ 0,
|
||||
&mut prompt("different prompt"),
|
||||
@@ -759,7 +794,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn backtrack_fork_last_turn_id_skips_hidden_review_prompts() {
|
||||
fn backtrack_fork_before_turn_id_skips_hidden_review_prompts() {
|
||||
let mut review_turn = turn(
|
||||
"turn-review",
|
||||
TurnStatus::Completed,
|
||||
@@ -783,18 +818,87 @@ mod tests {
|
||||
];
|
||||
|
||||
assert_eq!(
|
||||
backtrack_fork_last_turn_id(
|
||||
backtrack_fork_before_turn_id(
|
||||
&turns,
|
||||
/*nth_user_message*/ 1,
|
||||
&mut prompt("turn-2-prompt-0"),
|
||||
)
|
||||
.expect("the visible prompt after review should resolve"),
|
||||
Some("turn-review".to_string())
|
||||
Some("turn-2".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn backtrack_fork_last_turn_id_restores_canonical_mention_bindings() {
|
||||
fn backtrack_fork_before_turn_id_skips_hidden_nested_review_prompts() {
|
||||
let review_hint = "current changes";
|
||||
let review_prompt =
|
||||
"Review the current code changes (staged, unstaged, and untracked files).";
|
||||
let review_turn = Turn {
|
||||
items: vec![
|
||||
ThreadItem::EnteredReviewMode {
|
||||
id: "review-start".to_string(),
|
||||
review: review_hint.to_string(),
|
||||
},
|
||||
ThreadItem::ExitedReviewMode {
|
||||
id: "review-end".to_string(),
|
||||
review: "review complete".to_string(),
|
||||
},
|
||||
],
|
||||
..turn(
|
||||
"turn-review",
|
||||
TurnStatus::Completed,
|
||||
/*user_messages*/ 0,
|
||||
)
|
||||
};
|
||||
let review_child_turn = Turn {
|
||||
items: (0..2)
|
||||
.map(|index| ThreadItem::UserMessage {
|
||||
id: format!("review-prompt-{index}"),
|
||||
client_id: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: review_prompt.to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
})
|
||||
.collect(),
|
||||
..turn(
|
||||
"turn-review-child",
|
||||
TurnStatus::Interrupted,
|
||||
/*user_messages*/ 0,
|
||||
)
|
||||
};
|
||||
let interrupted_steered_turn = Turn {
|
||||
items: review_child_turn.items.clone(),
|
||||
completed_at: Some(1),
|
||||
..turn(
|
||||
"turn-interrupted-steer",
|
||||
TurnStatus::Interrupted,
|
||||
/*user_messages*/ 0,
|
||||
)
|
||||
};
|
||||
assert!(!is_hidden_nested_review_turn(
|
||||
&review_turn,
|
||||
&interrupted_steered_turn,
|
||||
));
|
||||
let turns = vec![
|
||||
review_turn,
|
||||
review_child_turn,
|
||||
turn("turn-2", TurnStatus::Completed, /*user_messages*/ 1),
|
||||
];
|
||||
|
||||
assert_eq!(
|
||||
backtrack_fork_before_turn_id(
|
||||
&turns,
|
||||
/*nth_user_message*/ 0,
|
||||
&mut prompt("turn-2-prompt-0"),
|
||||
)
|
||||
.expect("the visible prompt after a nested review should resolve"),
|
||||
Some("turn-2".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn backtrack_fork_before_turn_id_restores_canonical_mention_bindings() {
|
||||
let mut selected_turn = turn("turn-2", TurnStatus::Completed, /*user_messages*/ 1);
|
||||
selected_turn.items = vec![ThreadItem::UserMessage {
|
||||
id: "selected-prompt".to_string(),
|
||||
@@ -825,9 +929,13 @@ mod tests {
|
||||
let mut selected_prompt = prompt("use $skill @sample $google-calendar");
|
||||
|
||||
assert_eq!(
|
||||
backtrack_fork_last_turn_id(&turns, /*nth_user_message*/ 1, &mut selected_prompt,)
|
||||
.expect("the selected prompt should resolve"),
|
||||
Some("turn-1".to_string())
|
||||
backtrack_fork_before_turn_id(
|
||||
&turns,
|
||||
/*nth_user_message*/ 1,
|
||||
&mut selected_prompt,
|
||||
)
|
||||
.expect("the selected prompt should resolve"),
|
||||
Some("turn-2".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
selected_prompt.mention_bindings,
|
||||
|
||||
@@ -138,6 +138,12 @@ pub(crate) const EXTERNAL_AGENT_CONFIG_IMPORT_IN_PROGRESS_MESSAGE: &str =
|
||||
"A previous Claude Code import is still running. Wait for it to finish before importing again.";
|
||||
const THREAD_SETTINGS_UPDATE_METHOD: &str = "thread/settings/update";
|
||||
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
pub(crate) enum ForkGoalContinuation {
|
||||
StartIfIdle,
|
||||
DeferUntilNextTurn,
|
||||
}
|
||||
|
||||
fn bootstrap_request_error(context: &'static str, err: TypedRequestError) -> color_eyre::Report {
|
||||
color_eyre::eyre::eyre!("{context}: {err}")
|
||||
}
|
||||
@@ -530,25 +536,23 @@ impl AppServerSession {
|
||||
config: Config,
|
||||
thread_id: ThreadId,
|
||||
) -> Result<AppServerStartedThread> {
|
||||
self.fork_thread_at(config, thread_id, /*last_turn_id*/ None)
|
||||
.await
|
||||
self.fork_thread_at(
|
||||
config,
|
||||
thread_id,
|
||||
/*last_turn_id*/ None,
|
||||
/*before_turn_id*/ None,
|
||||
ForkGoalContinuation::StartIfIdle,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
pub(crate) async fn fork_thread_after(
|
||||
&mut self,
|
||||
config: Config,
|
||||
thread_id: ThreadId,
|
||||
last_turn_id: String,
|
||||
) -> Result<AppServerStartedThread> {
|
||||
self.fork_thread_at(config, thread_id, /*last_turn_id*/ Some(last_turn_id))
|
||||
.await
|
||||
}
|
||||
|
||||
async fn fork_thread_at(
|
||||
pub(crate) async fn fork_thread_at(
|
||||
&mut self,
|
||||
config: Config,
|
||||
thread_id: ThreadId,
|
||||
last_turn_id: Option<String>,
|
||||
before_turn_id: Option<String>,
|
||||
goal_continuation: ForkGoalContinuation,
|
||||
) -> Result<AppServerStartedThread> {
|
||||
let request_id = self.next_request_id();
|
||||
let session_config = self.session_config_with_effective_service_tier(&config);
|
||||
@@ -558,6 +562,9 @@ impl AppServerSession {
|
||||
request_id,
|
||||
params: ThreadForkParams {
|
||||
last_turn_id,
|
||||
before_turn_id,
|
||||
defer_goal_continuation: goal_continuation
|
||||
== ForkGoalContinuation::DeferUntilNextTurn,
|
||||
..thread_fork_params_from_config(
|
||||
session_config,
|
||||
thread_id,
|
||||
|
||||
@@ -12,7 +12,12 @@ impl ChatWidget {
|
||||
/// avoid triggering side effects. Event ids are passed as `None` to
|
||||
/// distinguish replayed events from live ones.
|
||||
pub(crate) fn replay_thread_turns(&mut self, turns: Vec<Turn>, replay_kind: ReplayKind) {
|
||||
for turn in turns {
|
||||
let hidden_nested_review_turns = std::iter::once(/*value*/ false)
|
||||
.chain(turns.windows(/*size*/ 2).map(|turns| {
|
||||
crate::app_backtrack::is_hidden_nested_review_turn(&turns[0], &turns[1])
|
||||
}))
|
||||
.collect::<Vec<_>>();
|
||||
for (turn, hidden_nested_review_turn) in turns.into_iter().zip(hidden_nested_review_turns) {
|
||||
let Turn {
|
||||
id: turn_id,
|
||||
items_view: _,
|
||||
@@ -29,8 +34,16 @@ impl ChatWidget {
|
||||
self.on_task_started();
|
||||
}
|
||||
for item in items {
|
||||
if hidden_nested_review_turn && matches!(item, ThreadItem::UserMessage { .. }) {
|
||||
continue;
|
||||
}
|
||||
self.replay_thread_item(item, turn_id.clone(), replay_kind);
|
||||
}
|
||||
let status = if hidden_nested_review_turn {
|
||||
TurnStatus::Completed
|
||||
} else {
|
||||
status
|
||||
};
|
||||
if matches!(
|
||||
status,
|
||||
TurnStatus::Completed | TurnStatus::Interrupted | TurnStatus::Failed
|
||||
|
||||
@@ -70,9 +70,7 @@ impl ChatWidget {
|
||||
self.input_queue.user_turn_pending_start = true;
|
||||
}
|
||||
|
||||
pub(crate) fn commit_safety_buffered_retry_submission(&mut self, prompt: UserMessage) {
|
||||
let display =
|
||||
user_message_display_for_history(prompt, &UserMessageHistoryRecord::UserMessageText);
|
||||
pub(crate) fn commit_safety_buffered_retry_submission(&mut self, display: UserMessageDisplay) {
|
||||
self.on_user_message_display(display);
|
||||
}
|
||||
|
||||
|
||||
@@ -226,6 +226,89 @@ async fn replayed_review_prompt_does_not_seed_composer_history() {
|
||||
assert_eq!(chat.bottom_pane.composer_text(), "");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn replayed_nested_review_prompts_do_not_render_or_seed_composer_history() {
|
||||
let (mut chat, mut rx, _ops) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
let review_hint = "current changes";
|
||||
let review_prompt = "Review the current code changes (staged, unstaged, and untracked files).";
|
||||
let user_message = |id: &str, text: &str| AppServerThreadItem::UserMessage {
|
||||
id: id.to_string(),
|
||||
client_id: None,
|
||||
content: vec![AppServerUserInput::Text {
|
||||
text: text.to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
};
|
||||
let review_marker = |turn_id: &str| AppServerTurn {
|
||||
items: vec![
|
||||
AppServerThreadItem::EnteredReviewMode {
|
||||
id: format!("{turn_id}-start"),
|
||||
review: review_hint.to_string(),
|
||||
},
|
||||
AppServerThreadItem::ExitedReviewMode {
|
||||
id: format!("{turn_id}-end"),
|
||||
review: "review complete".to_string(),
|
||||
},
|
||||
],
|
||||
..app_server_turn(
|
||||
turn_id,
|
||||
AppServerTurnStatus::Completed,
|
||||
/*duration_ms*/ None,
|
||||
/*error*/ None,
|
||||
)
|
||||
};
|
||||
|
||||
chat.replay_thread_turns(
|
||||
vec![
|
||||
review_marker("turn-review-before-steer"),
|
||||
AppServerTurn {
|
||||
items: vec![
|
||||
user_message("interrupted-prompt", review_hint),
|
||||
user_message("interrupted-steer", review_hint),
|
||||
],
|
||||
completed_at: Some(1),
|
||||
..app_server_turn(
|
||||
"turn-interrupted",
|
||||
AppServerTurnStatus::Interrupted,
|
||||
/*duration_ms*/ None,
|
||||
/*error*/ None,
|
||||
)
|
||||
},
|
||||
review_marker("turn-review"),
|
||||
AppServerTurn {
|
||||
items: vec![
|
||||
user_message("review-prompt-1", review_prompt),
|
||||
user_message("review-prompt-2", review_prompt),
|
||||
AppServerThreadItem::AgentMessage {
|
||||
id: "review-result".to_string(),
|
||||
text: "review result is retained".to_string(),
|
||||
phase: Some(MessagePhase::FinalAnswer),
|
||||
memory_citation: None,
|
||||
},
|
||||
],
|
||||
..app_server_turn(
|
||||
"turn-review-child",
|
||||
AppServerTurnStatus::Interrupted,
|
||||
/*duration_ms*/ None,
|
||||
/*error*/ None,
|
||||
)
|
||||
},
|
||||
],
|
||||
ReplayKind::ResumeInitialMessages,
|
||||
);
|
||||
|
||||
let rendered = drain_insert_history(&mut rx)
|
||||
.into_iter()
|
||||
.map(|lines| lines_to_single_string(&lines))
|
||||
.collect::<String>();
|
||||
assert!(!rendered.contains(review_prompt));
|
||||
assert_eq!(rendered.matches(review_hint).count(), 4);
|
||||
insta::assert_snapshot!("replayed_nested_review_prompts", rendered);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE));
|
||||
assert_eq!(chat.bottom_pane.composer_text(), review_hint);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn replayed_user_message_preserves_text_elements_and_local_images() {
|
||||
let (mut chat, mut rx, _ops) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
+23
@@ -0,0 +1,23 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/history_replay.rs
|
||||
expression: rendered
|
||||
---
|
||||
>> Code review started: current changes <<
|
||||
|
||||
<< Code review finished >>
|
||||
|
||||
|
||||
› current changes
|
||||
|
||||
|
||||
|
||||
› current changes
|
||||
|
||||
|
||||
■ Conversation interrupted - tell the model what to do differently. Something went wrong? Hit `/feedback` to report the issue.
|
||||
|
||||
>> Code review started: current changes <<
|
||||
|
||||
<< Code review finished >>
|
||||
|
||||
• review result is retained
|
||||
Reference in New Issue
Block a user