diff --git a/announcement_tip.toml b/announcement_tip.toml new file mode 100644 index 0000000000..3ad4a76590 --- /dev/null +++ b/announcement_tip.toml @@ -0,0 +1,16 @@ +# Example announcement tips for Codex TUI. +# Each [[announcements]] entry is evaluated in order; the last matching one is shown. +# Dates are UTC, formatted as YYYY-MM-DD. The from_date is inclusive and the to_date is exclusive. +# version_regex matches against the CLI version (env!("CARGO_PKG_VERSION")); omit to apply to all versions. +# target_app specify which app should display the announcement (cli, vsce, ...). + +[[announcements]] +content = "Welcome to Codex! Check out the new onboarding flow." +from_date = "2024-10-01" +to_date = "2024-10-15" +target_app = "cli" + +[[announcements]] +content = "This is a test announcement" +version_regex = "^0\\.0\\.0$" +to_date = "2026-01-10" diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 90acb1e772..bf68ff043f 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -360,7 +360,7 @@ dependencies = [ "objc2-foundation", "parking_lot", "percent-encoding", - "windows-sys 0.52.0", + "windows-sys 0.60.2", "wl-clipboard-rs", "x11rb", ] @@ -1866,7 +1866,7 @@ dependencies = [ name = "codex-utils-cache" version = "0.0.0" dependencies = [ - "lru 0.16.2", + "lru 0.16.3", "sha1", "tokio", ] @@ -2781,7 +2781,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "778e2ac28f6c47af28e4907f13ffd1e1ddbd400980a9abd7c8df189bf578a5ad" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.60.2", ] [[package]] @@ -2889,7 +2889,7 @@ checksum = "0ce92ff622d6dadf7349484f42c93271a0d49b7cc4d466a936405bacbe10aa78" dependencies = [ "cfg-if", "rustix 1.0.8", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -3830,7 +3830,7 @@ checksum = "e04d7f318608d35d4b61ddd75cbdaee86b023ebe2bd5a66ee0915f0bf93095a9" dependencies = [ "hermit-abi", "libc", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -4151,9 +4151,9 @@ dependencies = [ [[package]] name = "lru" -version = "0.16.2" +version = "0.16.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "96051b46fc183dc9cd4a223960ef37b9af631b55191852a8274bfef064cda20f" +checksum = "a1dc47f592c06f33f8e3aea9591776ec7c9f9e4124778ff8a3c3b87159f7e593" dependencies = [ "hashbrown 0.16.0", ] @@ -5331,7 +5331,7 @@ dependencies = [ "once_cell", "socket2 0.6.1", "tracing", - "windows-sys 0.52.0", + "windows-sys 0.60.2", ] [[package]] @@ -5459,7 +5459,7 @@ dependencies = [ "indoc", "itertools 0.14.0", "kasuari", - "lru 0.16.2", + "lru 0.16.3", "strum 0.27.2", "thiserror 2.0.17", "unicode-segmentation", @@ -5710,7 +5710,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.4.15", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -5723,7 +5723,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.9.4", - "windows-sys 0.52.0", + "windows-sys 0.60.2", ] [[package]] @@ -8011,7 +8011,7 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb" dependencies = [ - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 645d5aa210..2d0b5eec08 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -152,7 +152,7 @@ landlock = "0.4.4" lazy_static = "1" libc = "0.2.177" log = "0.4" -lru = "0.16.2" +lru = "0.16.3" maplit = "1.0.2" mime_guess = "2.0.5" multimap = "0.10.0" diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 83fa53b997..e9e71140ad 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -113,6 +113,10 @@ client_request_definitions! { params: v2::ThreadArchiveParams, response: v2::ThreadArchiveResponse, }, + ThreadRollback => "thread/rollback" { + params: v2::ThreadRollbackParams, + response: v2::ThreadRollbackResponse, + }, ThreadList => "thread/list" { params: v2::ThreadListParams, response: v2::ThreadListResponse, diff --git a/codex-rs/app-server-protocol/src/protocol/thread_history.rs b/codex-rs/app-server-protocol/src/protocol/thread_history.rs index ba1e6261cc..6fa6dfabbd 100644 --- a/codex-rs/app-server-protocol/src/protocol/thread_history.rs +++ b/codex-rs/app-server-protocol/src/protocol/thread_history.rs @@ -6,6 +6,7 @@ use crate::protocol::v2::UserInput; use codex_protocol::protocol::AgentReasoningEvent; use codex_protocol::protocol::AgentReasoningRawContentEvent; use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::ThreadRolledBackEvent; use codex_protocol::protocol::TurnAbortedEvent; use codex_protocol::protocol::UserMessageEvent; @@ -57,6 +58,7 @@ impl ThreadHistoryBuilder { EventMsg::TokenCount(_) => {} EventMsg::EnteredReviewMode(_) => {} EventMsg::ExitedReviewMode(_) => {} + EventMsg::ThreadRolledBack(payload) => self.handle_thread_rollback(payload), EventMsg::UndoCompleted(_) => {} EventMsg::TurnAborted(payload) => self.handle_turn_aborted(payload), _ => {} @@ -130,6 +132,23 @@ impl ThreadHistoryBuilder { turn.status = TurnStatus::Interrupted; } + fn handle_thread_rollback(&mut self, payload: &ThreadRolledBackEvent) { + self.finish_current_turn(); + + let n = usize::try_from(payload.num_turns).unwrap_or(usize::MAX); + if n >= self.turns.len() { + self.turns.clear(); + } else { + self.turns.truncate(self.turns.len().saturating_sub(n)); + } + + // Re-number subsequent synthetic ids so the pruned history is consistent. + self.next_turn_index = + i64::try_from(self.turns.len().saturating_add(1)).unwrap_or(i64::MAX); + let item_count: usize = self.turns.iter().map(|t| t.items.len()).sum(); + self.next_item_index = i64::try_from(item_count.saturating_add(1)).unwrap_or(i64::MAX); + } + fn finish_current_turn(&mut self) { if let Some(turn) = self.current_turn.take() { if turn.items.is_empty() { @@ -213,6 +232,7 @@ mod tests { use codex_protocol::protocol::AgentMessageEvent; use codex_protocol::protocol::AgentReasoningEvent; use codex_protocol::protocol::AgentReasoningRawContentEvent; + use codex_protocol::protocol::ThreadRolledBackEvent; use codex_protocol::protocol::TurnAbortReason; use codex_protocol::protocol::TurnAbortedEvent; use codex_protocol::protocol::UserMessageEvent; @@ -410,4 +430,95 @@ mod tests { } ); } + + #[test] + fn drops_last_turns_on_thread_rollback() { + let events = vec![ + EventMsg::UserMessage(UserMessageEvent { + message: "First".into(), + images: None, + }), + EventMsg::AgentMessage(AgentMessageEvent { + message: "A1".into(), + }), + EventMsg::UserMessage(UserMessageEvent { + message: "Second".into(), + images: None, + }), + EventMsg::AgentMessage(AgentMessageEvent { + message: "A2".into(), + }), + EventMsg::ThreadRolledBack(ThreadRolledBackEvent { num_turns: 1 }), + EventMsg::UserMessage(UserMessageEvent { + message: "Third".into(), + images: None, + }), + EventMsg::AgentMessage(AgentMessageEvent { + message: "A3".into(), + }), + ]; + + let turns = build_turns_from_event_msgs(&events); + let expected = vec![ + Turn { + id: "turn-1".into(), + status: TurnStatus::Completed, + error: None, + items: vec![ + ThreadItem::UserMessage { + id: "item-1".into(), + content: vec![UserInput::Text { + text: "First".into(), + }], + }, + ThreadItem::AgentMessage { + id: "item-2".into(), + text: "A1".into(), + }, + ], + }, + Turn { + id: "turn-2".into(), + status: TurnStatus::Completed, + error: None, + items: vec![ + ThreadItem::UserMessage { + id: "item-3".into(), + content: vec![UserInput::Text { + text: "Third".into(), + }], + }, + ThreadItem::AgentMessage { + id: "item-4".into(), + text: "A3".into(), + }, + ], + }, + ]; + assert_eq!(turns, expected); + } + + #[test] + fn thread_rollback_clears_all_turns_when_num_turns_exceeds_history() { + let events = vec![ + EventMsg::UserMessage(UserMessageEvent { + message: "One".into(), + images: None, + }), + EventMsg::AgentMessage(AgentMessageEvent { + message: "A1".into(), + }), + EventMsg::UserMessage(UserMessageEvent { + message: "Two".into(), + images: None, + }), + EventMsg::AgentMessage(AgentMessageEvent { + message: "A2".into(), + }), + EventMsg::ThreadRolledBack(ThreadRolledBackEvent { num_turns: 99 }), + ]; + + let turns = build_turns_from_event_msgs(&events); + assert_eq!(turns, Vec::::new()); + } } diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 13a6545359..dedf51ea1b 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -89,6 +89,7 @@ pub enum CodexErrorInfo { InternalServerError, Unauthorized, BadRequest, + ThreadRollbackFailed, SandboxError, /// The response SSE stream disconnected in the middle of a turn before completion. ResponseStreamDisconnected { @@ -119,6 +120,7 @@ impl From for CodexErrorInfo { CoreCodexErrorInfo::InternalServerError => CodexErrorInfo::InternalServerError, CoreCodexErrorInfo::Unauthorized => CodexErrorInfo::Unauthorized, CoreCodexErrorInfo::BadRequest => CodexErrorInfo::BadRequest, + CoreCodexErrorInfo::ThreadRollbackFailed => CodexErrorInfo::ThreadRollbackFailed, CoreCodexErrorInfo::SandboxError => CodexErrorInfo::SandboxError, CoreCodexErrorInfo::ResponseStreamDisconnected { http_status_code } => { CodexErrorInfo::ResponseStreamDisconnected { http_status_code } @@ -330,6 +332,15 @@ pub struct ProfileV2 { pub additional: HashMap, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "snake_case")] +#[ts(export_to = "v2/")] +pub struct AnalyticsConfig { + pub enabled: Option, + #[serde(default, flatten)] + pub additional: HashMap, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "snake_case")] #[ts(export_to = "v2/")] @@ -354,6 +365,7 @@ pub struct Config { pub model_reasoning_effort: Option, pub model_reasoning_summary: Option, pub model_verbosity: Option, + pub analytics: Option, #[serde(default, flatten)] pub additional: HashMap, } @@ -1045,6 +1057,30 @@ pub struct ThreadArchiveParams { #[ts(export_to = "v2/")] pub struct ThreadArchiveResponse {} +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ThreadRollbackParams { + pub thread_id: String, + /// The number of turns to drop from the end of the thread. Must be >= 1. + /// + /// This only modifies the thread's history and does not revert local file changes + /// that have been made by the agent. Clients are responsible for reverting these changes. + pub num_turns: u32, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ThreadRollbackResponse { + /// The updated thread after applying the rollback, with `turns` populated. + /// + /// The ThreadItems stored in each Turn are lossy since we explicitly do not + /// persist all agent interactions, such as command executions. This is the same + /// behavior as `thread/resume`. + pub thread: Thread, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] @@ -1183,7 +1219,7 @@ pub struct Thread { pub source: SessionSource, /// Optional Git metadata captured when the thread was created. pub git_info: Option, - /// Only populated on a `thread/resume` response. + /// Only populated on `thread/resume` and `thread/rollback` responses. /// For all other responses and notifications returning a Thread, /// the turns field will be an empty list. pub turns: Vec, diff --git a/codex-rs/app-server-test-client/src/main.rs b/codex-rs/app-server-test-client/src/main.rs index b66c59d55a..526558250c 100644 --- a/codex-rs/app-server-test-client/src/main.rs +++ b/codex-rs/app-server-test-client/src/main.rs @@ -13,6 +13,7 @@ use std::time::Duration; use anyhow::Context; use anyhow::Result; use anyhow::bail; +use clap::ArgAction; use clap::Parser; use clap::Subcommand; use codex_app_server_protocol::AddConversationListenerParams; @@ -35,6 +36,8 @@ use codex_app_server_protocol::JSONRPCRequest; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::LoginChatGptCompleteNotification; use codex_app_server_protocol::LoginChatGptResponse; +use codex_app_server_protocol::ModelListParams; +use codex_app_server_protocol::ModelListResponse; use codex_app_server_protocol::NewConversationParams; use codex_app_server_protocol::NewConversationResponse; use codex_app_server_protocol::RequestId; @@ -65,6 +68,19 @@ struct Cli { #[arg(long, env = "CODEX_BIN", default_value = "codex")] codex_bin: String, + /// Forwarded to the `codex` CLI as `--config key=value`. Repeatable. + /// + /// Example: + /// `--config 'model_providers.mock.base_url="http://localhost:4010/v2"'` + #[arg( + short = 'c', + long = "config", + value_name = "key=value", + action = ArgAction::Append, + global = true + )] + config_overrides: Vec, + #[command(subcommand)] command: CliCommand, } @@ -113,32 +129,49 @@ enum CliCommand { TestLogin, /// Fetch the current account rate limits from the Codex app-server. GetAccountRateLimits, + /// List the available models from the Codex app-server. + #[command(name = "model-list")] + ModelList, } fn main() -> Result<()> { - let Cli { codex_bin, command } = Cli::parse(); + let Cli { + codex_bin, + config_overrides, + command, + } = Cli::parse(); match command { - CliCommand::SendMessage { user_message } => send_message(codex_bin, user_message), - CliCommand::SendMessageV2 { user_message } => send_message_v2(codex_bin, user_message), + CliCommand::SendMessage { user_message } => { + send_message(&codex_bin, &config_overrides, user_message) + } + CliCommand::SendMessageV2 { user_message } => { + send_message_v2(&codex_bin, &config_overrides, user_message) + } CliCommand::TriggerCmdApproval { user_message } => { - trigger_cmd_approval(codex_bin, user_message) + trigger_cmd_approval(&codex_bin, &config_overrides, user_message) } CliCommand::TriggerPatchApproval { user_message } => { - trigger_patch_approval(codex_bin, user_message) + trigger_patch_approval(&codex_bin, &config_overrides, user_message) } - CliCommand::NoTriggerCmdApproval => no_trigger_cmd_approval(codex_bin), + CliCommand::NoTriggerCmdApproval => no_trigger_cmd_approval(&codex_bin, &config_overrides), CliCommand::SendFollowUpV2 { first_message, follow_up_message, - } => send_follow_up_v2(codex_bin, first_message, follow_up_message), - CliCommand::TestLogin => test_login(codex_bin), - CliCommand::GetAccountRateLimits => get_account_rate_limits(codex_bin), + } => send_follow_up_v2( + &codex_bin, + &config_overrides, + first_message, + follow_up_message, + ), + CliCommand::TestLogin => test_login(&codex_bin, &config_overrides), + CliCommand::GetAccountRateLimits => get_account_rate_limits(&codex_bin, &config_overrides), + CliCommand::ModelList => model_list(&codex_bin, &config_overrides), } } -fn send_message(codex_bin: String, user_message: String) -> Result<()> { - let mut client = CodexClient::spawn(codex_bin)?; +fn send_message(codex_bin: &str, config_overrides: &[String], user_message: String) -> Result<()> { + let mut client = CodexClient::spawn(codex_bin, config_overrides)?; let initialize = client.initialize()?; println!("< initialize response: {initialize:?}"); @@ -159,46 +192,61 @@ fn send_message(codex_bin: String, user_message: String) -> Result<()> { Ok(()) } -fn send_message_v2(codex_bin: String, user_message: String) -> Result<()> { - send_message_v2_with_policies(codex_bin, user_message, None, None) +fn send_message_v2( + codex_bin: &str, + config_overrides: &[String], + user_message: String, +) -> Result<()> { + send_message_v2_with_policies(codex_bin, config_overrides, user_message, None, None) } -fn trigger_cmd_approval(codex_bin: String, user_message: Option) -> Result<()> { +fn trigger_cmd_approval( + codex_bin: &str, + config_overrides: &[String], + user_message: Option, +) -> Result<()> { let default_prompt = "Run `touch /tmp/should-trigger-approval` so I can confirm the file exists."; let message = user_message.unwrap_or_else(|| default_prompt.to_string()); send_message_v2_with_policies( codex_bin, + config_overrides, message, Some(AskForApproval::OnRequest), Some(SandboxPolicy::ReadOnly), ) } -fn trigger_patch_approval(codex_bin: String, user_message: Option) -> Result<()> { +fn trigger_patch_approval( + codex_bin: &str, + config_overrides: &[String], + user_message: Option, +) -> Result<()> { let default_prompt = "Create a file named APPROVAL_DEMO.txt containing a short hello message using apply_patch."; let message = user_message.unwrap_or_else(|| default_prompt.to_string()); send_message_v2_with_policies( codex_bin, + config_overrides, message, Some(AskForApproval::OnRequest), Some(SandboxPolicy::ReadOnly), ) } -fn no_trigger_cmd_approval(codex_bin: String) -> Result<()> { +fn no_trigger_cmd_approval(codex_bin: &str, config_overrides: &[String]) -> Result<()> { let prompt = "Run `touch should_not_trigger_approval.txt`"; - send_message_v2_with_policies(codex_bin, prompt.to_string(), None, None) + send_message_v2_with_policies(codex_bin, config_overrides, prompt.to_string(), None, None) } fn send_message_v2_with_policies( - codex_bin: String, + codex_bin: &str, + config_overrides: &[String], user_message: String, approval_policy: Option, sandbox_policy: Option, ) -> Result<()> { - let mut client = CodexClient::spawn(codex_bin)?; + let mut client = CodexClient::spawn(codex_bin, config_overrides)?; let initialize = client.initialize()?; println!("< initialize response: {initialize:?}"); @@ -222,11 +270,12 @@ fn send_message_v2_with_policies( } fn send_follow_up_v2( - codex_bin: String, + codex_bin: &str, + config_overrides: &[String], first_message: String, follow_up_message: String, ) -> Result<()> { - let mut client = CodexClient::spawn(codex_bin)?; + let mut client = CodexClient::spawn(codex_bin, config_overrides)?; let initialize = client.initialize()?; println!("< initialize response: {initialize:?}"); @@ -259,8 +308,8 @@ fn send_follow_up_v2( Ok(()) } -fn test_login(codex_bin: String) -> Result<()> { - let mut client = CodexClient::spawn(codex_bin)?; +fn test_login(codex_bin: &str, config_overrides: &[String]) -> Result<()> { + let mut client = CodexClient::spawn(codex_bin, config_overrides)?; let initialize = client.initialize()?; println!("< initialize response: {initialize:?}"); @@ -289,8 +338,8 @@ fn test_login(codex_bin: String) -> Result<()> { } } -fn get_account_rate_limits(codex_bin: String) -> Result<()> { - let mut client = CodexClient::spawn(codex_bin)?; +fn get_account_rate_limits(codex_bin: &str, config_overrides: &[String]) -> Result<()> { + let mut client = CodexClient::spawn(codex_bin, config_overrides)?; let initialize = client.initialize()?; println!("< initialize response: {initialize:?}"); @@ -301,6 +350,18 @@ fn get_account_rate_limits(codex_bin: String) -> Result<()> { Ok(()) } +fn model_list(codex_bin: &str, config_overrides: &[String]) -> Result<()> { + let mut client = CodexClient::spawn(codex_bin, config_overrides)?; + + let initialize = client.initialize()?; + println!("< initialize response: {initialize:?}"); + + let response = client.model_list(ModelListParams::default())?; + println!("< model/list response: {response:?}"); + + Ok(()) +} + struct CodexClient { child: Child, stdin: Option, @@ -309,8 +370,12 @@ struct CodexClient { } impl CodexClient { - fn spawn(codex_bin: String) -> Result { - let mut codex_app_server = Command::new(&codex_bin) + fn spawn(codex_bin: &str, config_overrides: &[String]) -> Result { + let mut cmd = Command::new(codex_bin); + for override_kv in config_overrides { + cmd.arg("--config").arg(override_kv); + } + let mut codex_app_server = cmd .arg("app-server") .stdin(Stdio::piped()) .stdout(Stdio::piped()) @@ -452,6 +517,16 @@ impl CodexClient { self.send_request(request, request_id, "account/rateLimits/read") } + fn model_list(&mut self, params: ModelListParams) -> Result { + let request_id = self.request_id(); + let request = ClientRequest::ModelList { + request_id: request_id.clone(), + params, + }; + + self.send_request(request, request_id, "model/list") + } + fn stream_conversation(&mut self, conversation_id: &ConversationId) -> Result<()> { loop { let notification = self.next_notification()?; diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 267d7577e5..b8f7de90ab 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -72,6 +72,7 @@ Example (from OpenAI's official VSCode extension): - `thread/resume` — reopen an existing thread by id so subsequent `turn/start` calls append to it. - `thread/list` — page through stored rollouts; supports cursor-based pagination and optional `modelProviders` filtering. - `thread/archive` — move a thread’s rollout file into the archived directory; returns `{}` on success. +- `thread/rollback` — drop the last N turns from the agent’s in-memory context and persist a rollback marker in the rollout so future resumes see the pruned history; returns the updated `thread` (with `turns` populated) on success. - `turn/start` — add user input to a thread and begin Codex generation; responds with the initial `turn` object and streams `turn/started`, `item/*`, and `turn/completed` notifications. - `turn/interrupt` — request cancellation of an in-flight turn by `(thread_id, turn_id)`; success is an empty `{}` response and the turn finishes with `status: "interrupted"`. - `review/start` — kick off Codex’s automated reviewer for a thread; responds like `turn/start` and emits `item/started`/`item/completed` notifications with `enteredReviewMode` and `exitedReviewMode` items, plus a final assistant `agentMessage` containing the review. diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index ad0455a058..c9b78fe8c8 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -1,7 +1,13 @@ use crate::codex_message_processor::ApiVersion; use crate::codex_message_processor::PendingInterrupts; +use crate::codex_message_processor::PendingRollbacks; use crate::codex_message_processor::TurnSummary; use crate::codex_message_processor::TurnSummaryStore; +use crate::codex_message_processor::read_event_msgs_from_rollout; +use crate::codex_message_processor::read_summary_from_rollout; +use crate::codex_message_processor::summary_to_thread; +use crate::error_code::INTERNAL_ERROR_CODE; +use crate::error_code::INVALID_REQUEST_ERROR_CODE; use crate::outgoing_message::OutgoingMessageSender; use codex_app_server_protocol::AccountRateLimitsUpdatedNotification; use codex_app_server_protocol::AgentMessageDeltaNotification; @@ -27,6 +33,7 @@ use codex_app_server_protocol::FileUpdateChange; use codex_app_server_protocol::InterruptConversationResponse; use codex_app_server_protocol::ItemCompletedNotification; use codex_app_server_protocol::ItemStartedNotification; +use codex_app_server_protocol::JSONRPCErrorError; use codex_app_server_protocol::McpToolCallError; use codex_app_server_protocol::McpToolCallResult; use codex_app_server_protocol::McpToolCallStatus; @@ -40,6 +47,7 @@ use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ServerRequestPayload; use codex_app_server_protocol::TerminalInteractionNotification; use codex_app_server_protocol::ThreadItem; +use codex_app_server_protocol::ThreadRollbackResponse; use codex_app_server_protocol::ThreadTokenUsage; use codex_app_server_protocol::ThreadTokenUsageUpdatedNotification; use codex_app_server_protocol::Turn; @@ -50,9 +58,11 @@ use codex_app_server_protocol::TurnInterruptResponse; use codex_app_server_protocol::TurnPlanStep; use codex_app_server_protocol::TurnPlanUpdatedNotification; use codex_app_server_protocol::TurnStatus; +use codex_app_server_protocol::build_turns_from_event_msgs; use codex_core::CodexConversation; use codex_core::parse_command::shlex_join; use codex_core::protocol::ApplyPatchApprovalRequestEvent; +use codex_core::protocol::CodexErrorInfo as CoreCodexErrorInfo; use codex_core::protocol::Event; use codex_core::protocol::EventMsg; use codex_core::protocol::ExecApprovalRequestEvent; @@ -78,14 +88,17 @@ use tracing::error; type JsonValue = serde_json::Value; +#[allow(clippy::too_many_arguments)] pub(crate) async fn apply_bespoke_event_handling( event: Event, conversation_id: ConversationId, conversation: Arc, outgoing: Arc, pending_interrupts: PendingInterrupts, + pending_rollbacks: PendingRollbacks, turn_summary_store: TurnSummaryStore, api_version: ApiVersion, + fallback_model_provider: String, ) { let Event { id: event_turn_id, @@ -337,6 +350,26 @@ pub(crate) async fn apply_bespoke_event_handling( .await; } EventMsg::Error(ev) => { + let message = ev.message.clone(); + let codex_error_info = ev.codex_error_info.clone(); + + // If this error belongs to an in-flight `thread/rollback` request, fail that request + // (and clear pending state) so subsequent rollbacks are unblocked. + // + // Don't send a notification for this error. + if matches!( + codex_error_info, + Some(CoreCodexErrorInfo::ThreadRollbackFailed) + ) { + return handle_thread_rollback_failed( + conversation_id, + message, + &pending_rollbacks, + &outgoing, + ) + .await; + }; + let turn_error = TurnError { message: ev.message, codex_error_info: ev.codex_error_info.map(V2CodexErrorInfo::from), @@ -345,7 +378,7 @@ pub(crate) async fn apply_bespoke_event_handling( handle_error(conversation_id, turn_error.clone(), &turn_summary_store).await; outgoing .send_server_notification(ServerNotification::Error(ErrorNotification { - error: turn_error, + error: turn_error.clone(), will_retry: false, thread_id: conversation_id.to_string(), turn_id: event_turn_id.clone(), @@ -690,6 +723,58 @@ pub(crate) async fn apply_bespoke_event_handling( ) .await; } + EventMsg::ThreadRolledBack(_rollback_event) => { + let pending = { + let mut map = pending_rollbacks.lock().await; + map.remove(&conversation_id) + }; + + if let Some(request_id) = pending { + let rollout_path = conversation.rollout_path(); + let response = match read_summary_from_rollout( + rollout_path.as_path(), + fallback_model_provider.as_str(), + ) + .await + { + Ok(summary) => { + let mut thread = summary_to_thread(summary); + match read_event_msgs_from_rollout(rollout_path.as_path()).await { + Ok(events) => { + thread.turns = build_turns_from_event_msgs(&events); + ThreadRollbackResponse { thread } + } + Err(err) => { + let error = JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + message: format!( + "failed to load rollout `{}`: {err}", + rollout_path.display() + ), + data: None, + }; + outgoing.send_error(request_id, error).await; + return; + } + } + } + Err(err) => { + let error = JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + message: format!( + "failed to load rollout `{}`: {err}", + rollout_path.display() + ), + data: None, + }; + outgoing.send_error(request_id, error).await; + return; + } + }; + + outgoing.send_response(request_id, response).await; + } + } EventMsg::TurnDiff(turn_diff_event) => { handle_turn_diff( conversation_id, @@ -906,6 +991,31 @@ async fn handle_turn_interrupted( .await; } +async fn handle_thread_rollback_failed( + conversation_id: ConversationId, + message: String, + pending_rollbacks: &PendingRollbacks, + outgoing: &OutgoingMessageSender, +) { + let pending_rollback = { + let mut map = pending_rollbacks.lock().await; + map.remove(&conversation_id) + }; + + if let Some(request_id) = pending_rollback { + outgoing + .send_error( + request_id, + JSONRPCErrorError { + code: INVALID_REQUEST_ERROR_CODE, + message: message.clone(), + data: None, + }, + ) + .await; + } +} + async fn handle_token_count_event( conversation_id: ConversationId, turn_id: String, diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index c1166d9508..1cadd5e4f0 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -91,6 +91,7 @@ use codex_app_server_protocol::ThreadListParams; use codex_app_server_protocol::ThreadListResponse; use codex_app_server_protocol::ThreadResumeParams; use codex_app_server_protocol::ThreadResumeResponse; +use codex_app_server_protocol::ThreadRollbackParams; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; use codex_app_server_protocol::ThreadStartedNotification; @@ -178,6 +179,8 @@ use uuid::Uuid; type PendingInterruptQueue = Vec<(RequestId, ApiVersion)>; pub(crate) type PendingInterrupts = Arc>>; +pub(crate) type PendingRollbacks = Arc>>; + /// Per-conversation accumulation of the latest states e.g. error message while a turn runs. #[derive(Default, Clone)] pub(crate) struct TurnSummary { @@ -220,6 +223,8 @@ pub(crate) struct CodexMessageProcessor { active_login: Arc>>, // Queue of pending interrupt requests per conversation. We reply when TurnAborted arrives. pending_interrupts: PendingInterrupts, + // Queue of pending rollback requests per conversation. We reply when ThreadRollback arrives. + pending_rollbacks: PendingRollbacks, turn_summary_store: TurnSummaryStore, pending_fuzzy_searches: Arc>>>, feedback: CodexFeedback, @@ -275,6 +280,7 @@ impl CodexMessageProcessor { conversation_listeners: HashMap::new(), active_login: Arc::new(Mutex::new(None)), pending_interrupts: Arc::new(Mutex::new(HashMap::new())), + pending_rollbacks: Arc::new(Mutex::new(HashMap::new())), turn_summary_store: Arc::new(Mutex::new(HashMap::new())), pending_fuzzy_searches: Arc::new(Mutex::new(HashMap::new())), feedback, @@ -365,6 +371,9 @@ impl CodexMessageProcessor { ClientRequest::ThreadArchive { request_id, params } => { self.thread_archive(request_id, params).await; } + ClientRequest::ThreadRollback { request_id, params } => { + self.thread_rollback(request_id, params).await; + } ClientRequest::ThreadList { request_id, params } => { self.thread_list(request_id, params).await; } @@ -1246,14 +1255,14 @@ impl CodexMessageProcessor { cwd, approval_policy, sandbox: sandbox_mode, - config: cli_overrides, + config: request_overrides, base_instructions, developer_instructions, compact_prompt, include_apply_patch_tool, } = params; - let overrides = ConfigOverrides { + let typesafe_overrides = ConfigOverrides { model, config_profile: profile, cwd: cwd.clone().map(PathBuf::from), @@ -1270,15 +1279,21 @@ impl CodexMessageProcessor { // Persist windows sandbox feature. // TODO: persist default config in general. - let mut cli_overrides = cli_overrides.unwrap_or_default(); + let mut request_overrides = request_overrides.unwrap_or_default(); if cfg!(windows) && self.config.features.enabled(Feature::WindowsSandbox) { - cli_overrides.insert( + request_overrides.insert( "features.experimental_windows_sandbox".to_string(), serde_json::json!(true), ); } - let config = match derive_config_from_params(overrides, Some(cli_overrides)).await { + let config = match derive_config_from_params( + &self.cli_overrides, + Some(request_overrides), + typesafe_overrides, + ) + .await + { Ok(config) => config, Err(err) => { let error = JSONRPCErrorError { @@ -1318,7 +1333,7 @@ impl CodexMessageProcessor { } async fn thread_start(&mut self, request_id: RequestId, params: ThreadStartParams) { - let overrides = self.build_thread_config_overrides( + let typesafe_overrides = self.build_thread_config_overrides( params.model, params.model_provider, params.cwd, @@ -1328,18 +1343,21 @@ impl CodexMessageProcessor { params.developer_instructions, ); - let config = match derive_config_from_params(overrides, params.config).await { - Ok(config) => config, - Err(err) => { - let error = JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: format!("error deriving config: {err}"), - data: None, - }; - self.outgoing.send_error(request_id, error).await; - return; - } - }; + let config = + match derive_config_from_params(&self.cli_overrides, params.config, typesafe_overrides) + .await + { + Ok(config) => config, + Err(err) => { + let error = JSONRPCErrorError { + code: INVALID_REQUEST_ERROR_CODE, + message: format!("error deriving config: {err}"), + data: None, + }; + self.outgoing.send_error(request_id, error).await; + return; + } + }; match self.conversation_manager.new_conversation(config).await { Ok(new_conv) => { @@ -1506,6 +1524,52 @@ impl CodexMessageProcessor { } } + async fn thread_rollback(&mut self, request_id: RequestId, params: ThreadRollbackParams) { + let ThreadRollbackParams { + thread_id, + num_turns, + } = params; + + if num_turns == 0 { + self.send_invalid_request_error(request_id, "numTurns must be >= 1".to_string()) + .await; + return; + } + + let (conversation_id, conversation) = + match self.conversation_from_thread_id(&thread_id).await { + Ok(v) => v, + Err(error) => { + self.outgoing.send_error(request_id, error).await; + return; + } + }; + + { + let mut map = self.pending_rollbacks.lock().await; + if map.contains_key(&conversation_id) { + self.send_invalid_request_error( + request_id, + "rollback already in progress for this thread".to_string(), + ) + .await; + return; + } + + map.insert(conversation_id, request_id.clone()); + } + + if let Err(err) = conversation.submit(Op::ThreadRollback { num_turns }).await { + // No ThreadRollback event will arrive if an error occurs. + // Clean up and reply immediately. + let mut map = self.pending_rollbacks.lock().await; + map.remove(&conversation_id); + + self.send_internal_error(request_id, format!("failed to start rollback: {err}")) + .await; + } + } + async fn thread_list(&self, request_id: RequestId, params: ThreadListParams) { let ThreadListParams { cursor, @@ -1543,7 +1607,7 @@ impl CodexMessageProcessor { cwd, approval_policy, sandbox, - config: cli_overrides, + config: request_overrides, base_instructions, developer_instructions, } = params; @@ -1553,12 +1617,12 @@ impl CodexMessageProcessor { || cwd.is_some() || approval_policy.is_some() || sandbox.is_some() - || cli_overrides.is_some() + || request_overrides.is_some() || base_instructions.is_some() || developer_instructions.is_some(); let config = if overrides_requested { - let overrides = self.build_thread_config_overrides( + let typesafe_overrides = self.build_thread_config_overrides( model, model_provider, cwd, @@ -1567,7 +1631,13 @@ impl CodexMessageProcessor { base_instructions, developer_instructions, ); - match derive_config_from_params(overrides, cli_overrides).await { + match derive_config_from_params( + &self.cli_overrides, + request_overrides, + typesafe_overrides, + ) + .await + { Ok(config) => config, Err(err) => { let error = JSONRPCErrorError { @@ -2197,7 +2267,7 @@ impl CodexMessageProcessor { cwd, approval_policy, sandbox: sandbox_mode, - config: cli_overrides, + config: request_overrides, base_instructions, developer_instructions, compact_prompt, @@ -2205,15 +2275,15 @@ impl CodexMessageProcessor { } = overrides; // Persist windows sandbox feature. - let mut cli_overrides = cli_overrides.unwrap_or_default(); + let mut request_overrides = request_overrides.unwrap_or_default(); if cfg!(windows) && self.config.features.enabled(Feature::WindowsSandbox) { - cli_overrides.insert( + request_overrides.insert( "features.experimental_windows_sandbox".to_string(), serde_json::json!(true), ); } - let overrides = ConfigOverrides { + let typesafe_overrides = ConfigOverrides { model, config_profile: profile, cwd: cwd.map(PathBuf::from), @@ -2228,7 +2298,12 @@ impl CodexMessageProcessor { ..Default::default() }; - derive_config_from_params(overrides, Some(cli_overrides)).await + derive_config_from_params( + &self.cli_overrides, + Some(request_overrides), + typesafe_overrides, + ) + .await } None => Ok(self.config.as_ref().clone()), }; @@ -3095,8 +3170,10 @@ impl CodexMessageProcessor { let outgoing_for_task = self.outgoing.clone(); let pending_interrupts = self.pending_interrupts.clone(); + let pending_rollbacks = self.pending_rollbacks.clone(); let turn_summary_store = self.turn_summary_store.clone(); let api_version_for_task = api_version; + let fallback_model_provider = self.config.model_provider_id.clone(); tokio::spawn(async move { loop { tokio::select! { @@ -3152,8 +3229,10 @@ impl CodexMessageProcessor { conversation.clone(), outgoing_for_task.clone(), pending_interrupts.clone(), + pending_rollbacks.clone(), turn_summary_store.clone(), api_version_for_task, + fallback_model_provider.clone(), ) .await; } @@ -3341,20 +3420,37 @@ fn errors_to_info( .collect() } +/// Derive the effective [`Config`] by layering three override sources. +/// +/// Precedence (lowest to highest): +/// - `cli_overrides`: process-wide startup `--config` flags. +/// - `request_overrides`: per-request dotted-path overrides (`params.config`), converted JSON->TOML. +/// - `typesafe_overrides`: Request objects such as `NewConversationParams` and +/// `ThreadStartParams` support a limited set of _explicit_ config overrides, so +/// `typesafe_overrides` is a `ConfigOverrides` derived from the respective request object. +/// Because the overrides are defined explicitly in the `*Params`, this takes priority over +/// the more general "bag of config options" provided by `cli_overrides` and `request_overrides`. async fn derive_config_from_params( - overrides: ConfigOverrides, - cli_overrides: Option>, + cli_overrides: &[(String, TomlValue)], + request_overrides: Option>, + typesafe_overrides: ConfigOverrides, ) -> std::io::Result { - let cli_overrides = cli_overrides - .unwrap_or_default() - .into_iter() - .map(|(k, v)| (k, json_to_toml(v))) - .collect(); + let merged_cli_overrides = cli_overrides + .iter() + .cloned() + .chain( + request_overrides + .unwrap_or_default() + .into_iter() + .map(|(k, v)| (k, json_to_toml(v))), + ) + .collect::>(); - Config::load_with_cli_overrides_and_harness_overrides(cli_overrides, overrides).await + Config::load_with_cli_overrides_and_harness_overrides(merged_cli_overrides, typesafe_overrides) + .await } -async fn read_summary_from_rollout( +pub(crate) async fn read_summary_from_rollout( path: &Path, fallback_provider: &str, ) -> std::io::Result { @@ -3413,6 +3509,24 @@ async fn read_summary_from_rollout( }) } +pub(crate) async fn read_event_msgs_from_rollout( + path: &Path, +) -> std::io::Result> { + let items = match RolloutRecorder::get_rollout_history(path).await? { + InitialHistory::New => Vec::new(), + InitialHistory::Forked(items) => items, + InitialHistory::Resumed(resumed) => resumed.history, + }; + + Ok(items + .into_iter() + .filter_map(|item| match item { + RolloutItem::EventMsg(event) => Some(event), + _ => None, + }) + .collect()) +} + fn extract_conversation_summary( path: PathBuf, head: &[serde_json::Value], @@ -3474,7 +3588,7 @@ fn parse_datetime(timestamp: Option<&str>) -> Option> { }) } -fn summary_to_thread(summary: ConversationSummary) -> Thread { +pub(crate) fn summary_to_thread(summary: ConversationSummary) -> Thread { let ConversationSummary { conversation_id, path, diff --git a/codex-rs/app-server/src/models.rs b/codex-rs/app-server/src/models.rs index 2141160354..29a9c99636 100644 --- a/codex-rs/app-server/src/models.rs +++ b/codex-rs/app-server/src/models.rs @@ -15,6 +15,7 @@ pub async fn supported_models( .list_models(config) .await .into_iter() + .filter(|preset| preset.show_in_picker) .map(model_from_preset) .collect() } diff --git a/codex-rs/app-server/tests/common/mcp_process.rs b/codex-rs/app-server/tests/common/mcp_process.rs index 98b2cabaaa..026b01ebba 100644 --- a/codex-rs/app-server/tests/common/mcp_process.rs +++ b/codex-rs/app-server/tests/common/mcp_process.rs @@ -45,6 +45,7 @@ use codex_app_server_protocol::SetDefaultModelParams; use codex_app_server_protocol::ThreadArchiveParams; use codex_app_server_protocol::ThreadListParams; use codex_app_server_protocol::ThreadResumeParams; +use codex_app_server_protocol::ThreadRollbackParams; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::TurnInterruptParams; use codex_app_server_protocol::TurnStartParams; @@ -316,6 +317,15 @@ impl McpProcess { self.send_request("thread/archive", params).await } + /// Send a `thread/rollback` JSON-RPC request. + pub async fn send_thread_rollback_request( + &mut self, + params: ThreadRollbackParams, + ) -> anyhow::Result { + let params = Some(serde_json::to_value(params)?); + self.send_request("thread/rollback", params).await + } + /// Send a `thread/list` JSON-RPC request. pub async fn send_thread_list_request( &mut self, diff --git a/codex-rs/app-server/tests/suite/v2/mod.rs b/codex-rs/app-server/tests/suite/v2/mod.rs index f23f792d39..1ef00c6939 100644 --- a/codex-rs/app-server/tests/suite/v2/mod.rs +++ b/codex-rs/app-server/tests/suite/v2/mod.rs @@ -7,6 +7,7 @@ mod review; mod thread_archive; mod thread_list; mod thread_resume; +mod thread_rollback; mod thread_start; mod turn_interrupt; mod turn_start; diff --git a/codex-rs/app-server/tests/suite/v2/model_list.rs b/codex-rs/app-server/tests/suite/v2/model_list.rs index e9fe70dbee..4cdb306205 100644 --- a/codex-rs/app-server/tests/suite/v2/model_list.rs +++ b/codex-rs/app-server/tests/suite/v2/model_list.rs @@ -74,7 +74,7 @@ async fn list_models_returns_all_models_with_large_limit() -> Result<()> { }, ReasoningEffortOption { reasoning_effort: ReasoningEffort::XHigh, - description: "Extra high reasoning for complex problems".to_string(), + description: "Extra high reasoning depth for complex problems".to_string(), }, ], default_reasoning_effort: ReasoningEffort::Medium, diff --git a/codex-rs/app-server/tests/suite/v2/thread_rollback.rs b/codex-rs/app-server/tests/suite/v2/thread_rollback.rs new file mode 100644 index 0000000000..f3313c759e --- /dev/null +++ b/codex-rs/app-server/tests/suite/v2/thread_rollback.rs @@ -0,0 +1,177 @@ +use anyhow::Result; +use app_test_support::McpProcess; +use app_test_support::create_final_assistant_message_sse_response; +use app_test_support::create_mock_chat_completions_server_unchecked; +use app_test_support::to_response; +use codex_app_server_protocol::JSONRPCResponse; +use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::ThreadItem; +use codex_app_server_protocol::ThreadResumeParams; +use codex_app_server_protocol::ThreadResumeResponse; +use codex_app_server_protocol::ThreadRollbackParams; +use codex_app_server_protocol::ThreadRollbackResponse; +use codex_app_server_protocol::ThreadStartParams; +use codex_app_server_protocol::ThreadStartResponse; +use codex_app_server_protocol::TurnStartParams; +use codex_app_server_protocol::UserInput as V2UserInput; +use pretty_assertions::assert_eq; +use tempfile::TempDir; +use tokio::time::timeout; + +const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); + +#[tokio::test] +async fn thread_rollback_drops_last_turns_and_persists_to_rollout() -> Result<()> { + // Three Codex turns hit the mock model (session start + two turn/start calls). + let responses = vec![ + create_final_assistant_message_sse_response("Done")?, + create_final_assistant_message_sse_response("Done")?, + create_final_assistant_message_sse_response("Done")?, + ]; + let server = create_mock_chat_completions_server_unchecked(responses).await; + + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri())?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + // Start a thread. + let start_id = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("mock-model".to_string()), + ..Default::default() + }) + .await?; + let start_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(start_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; + + // Two turns. + let first_text = "First"; + let turn1_id = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![V2UserInput::Text { + text: first_text.to_string(), + }], + ..Default::default() + }) + .await?; + let _turn1_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn1_id)), + ) + .await??; + let _completed1 = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + let turn2_id = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![V2UserInput::Text { + text: "Second".to_string(), + }], + ..Default::default() + }) + .await?; + let _turn2_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn2_id)), + ) + .await??; + let _completed2 = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + // Roll back the last turn. + let rollback_id = mcp + .send_thread_rollback_request(ThreadRollbackParams { + thread_id: thread.id.clone(), + num_turns: 1, + }) + .await?; + let rollback_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(rollback_id)), + ) + .await??; + let ThreadRollbackResponse { + thread: rolled_back_thread, + } = to_response::(rollback_resp)?; + + assert_eq!(rolled_back_thread.turns.len(), 1); + assert_eq!(rolled_back_thread.turns[0].items.len(), 2); + match &rolled_back_thread.turns[0].items[0] { + ThreadItem::UserMessage { content, .. } => { + assert_eq!( + content, + &vec![V2UserInput::Text { + text: first_text.to_string() + }] + ); + } + other => panic!("expected user message item, got {other:?}"), + } + + // Resume and confirm the history is pruned. + let resume_id = mcp + .send_thread_resume_request(ThreadResumeParams { + thread_id: thread.id, + ..Default::default() + }) + .await?; + let resume_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(resume_id)), + ) + .await??; + let ThreadResumeResponse { thread, .. } = to_response::(resume_resp)?; + + assert_eq!(thread.turns.len(), 1); + assert_eq!(thread.turns[0].items.len(), 2); + match &thread.turns[0].items[0] { + ThreadItem::UserMessage { content, .. } => { + assert_eq!( + content, + &vec![V2UserInput::Text { + text: first_text.to_string() + }] + ); + } + other => panic!("expected user message item, got {other:?}"), + } + + Ok(()) +} + +fn create_config_toml(codex_home: &std::path::Path, server_uri: &str) -> std::io::Result<()> { + let config_toml = codex_home.join("config.toml"); + std::fs::write( + config_toml, + format!( + r#" +model = "mock-model" +approval_policy = "never" +sandbox_mode = "read-only" + +model_provider = "mock_provider" + +[model_providers.mock_provider] +name = "Mock provider for test" +base_url = "{server_uri}/v1" +wire_api = "chat" +request_max_retries = 0 +stream_max_retries = 0 +"# + ), + ) +} diff --git a/codex-rs/core/src/agent/control.rs b/codex-rs/core/src/agent/control.rs new file mode 100644 index 0000000000..468f580a21 --- /dev/null +++ b/codex-rs/core/src/agent/control.rs @@ -0,0 +1,189 @@ +use crate::CodexConversation; +use crate::agent::AgentStatus; +use crate::conversation_manager::ConversationManagerState; +use crate::error::CodexErr; +use crate::error::Result as CodexResult; +use codex_protocol::ConversationId; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::Op; +use codex_protocol::user_input::UserInput; +use std::sync::Arc; +use std::sync::Weak; + +/// Control-plane handle for multi-agent operations. +/// `AgentControl` is held by each session (via `SessionServices`). It provides capability to +/// spawn new agents and the inter-agent communication layer. +#[derive(Clone, Default)] +pub(crate) struct AgentControl { + /// Weak handle back to the global conversation registry/state. + /// This is `Weak` to avoid reference cycles and shadow persistence of the form + /// `ConversationManagerState -> CodexConversation -> Session -> SessionServices -> ConversationManagerState`. + manager: Weak, +} + +impl AgentControl { + /// Construct a new `AgentControl` that can spawn/message agents via the given manager state. + pub(crate) fn new(manager: Weak) -> Self { + Self { manager } + } + + #[allow(dead_code)] // Used by upcoming multi-agent tooling. + /// Spawn a new agent conversation and submit the initial prompt. + /// + /// If `headless` is true, a background drain task is spawned to prevent unbounded event growth + /// of the channel queue when there is no client actively reading the conversation events. + pub(crate) async fn spawn_agent( + &self, + config: crate::config::Config, + prompt: String, + headless: bool, + ) -> CodexResult { + let state = self.upgrade()?; + let new_conversation = state.spawn_new_conversation(config, self.clone()).await?; + + if headless { + spawn_headless_drain(Arc::clone(&new_conversation.conversation)); + } + + self.send_prompt(new_conversation.conversation_id, prompt) + .await?; + + Ok(new_conversation.conversation_id) + } + + #[allow(dead_code)] // Used by upcoming multi-agent tooling. + /// Send a `user` prompt to an existing agent conversation. + pub(crate) async fn send_prompt( + &self, + agent_id: ConversationId, + prompt: String, + ) -> CodexResult { + let state = self.upgrade()?; + state + .send_op( + agent_id, + Op::UserInput { + items: vec![UserInput::Text { text: prompt }], + final_output_json_schema: None, + }, + ) + .await + } + + #[allow(dead_code)] // Used by upcoming multi-agent tooling. + /// Fetch the last known status for `agent_id`, returning `NotFound` when unavailable. + pub(crate) async fn get_status(&self, agent_id: ConversationId) -> AgentStatus { + let Ok(state) = self.upgrade() else { + // No agent available if upgrade fails. + return AgentStatus::NotFound; + }; + let Ok(conversation) = state.get_conversation(agent_id).await else { + return AgentStatus::NotFound; + }; + conversation.agent_status().await + } + + fn upgrade(&self) -> CodexResult> { + self.manager.upgrade().ok_or_else(|| { + CodexErr::UnsupportedOperation("conversation manager dropped".to_string()) + }) + } +} + +/// When an agent is spawned "headless" (no UI/view attached), there may be no consumer polling +/// `CodexConversation::next_event()`. The underlying event channel is unbounded, so the producer can +/// accumulate events indefinitely. This drain task prevents that memory growth by polling and +/// discarding events until shutdown. +fn spawn_headless_drain(conversation: Arc) { + tokio::spawn(async move { + loop { + match conversation.next_event().await { + Ok(event) => { + if matches!(event.msg, EventMsg::ShutdownComplete) { + break; + } + } + Err(err) => { + tracing::warn!("failed to receive event from agent: {err:?}"); + break; + } + } + } + }); +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::agent::agent_status_from_event; + use codex_protocol::protocol::ErrorEvent; + use codex_protocol::protocol::TaskCompleteEvent; + use codex_protocol::protocol::TaskStartedEvent; + use codex_protocol::protocol::TurnAbortReason; + use codex_protocol::protocol::TurnAbortedEvent; + use pretty_assertions::assert_eq; + + #[tokio::test] + async fn send_prompt_errors_when_manager_dropped() { + let control = AgentControl::default(); + let err = control + .send_prompt(ConversationId::new(), "hello".to_string()) + .await + .expect_err("send_prompt should fail without a manager"); + assert_eq!( + err.to_string(), + "unsupported operation: conversation manager dropped" + ); + } + + #[tokio::test] + async fn get_status_returns_not_found_without_manager() { + let control = AgentControl::default(); + let got = control.get_status(ConversationId::new()).await; + assert_eq!(got, AgentStatus::NotFound); + } + + #[tokio::test] + async fn on_event_updates_status_from_task_started() { + let status = agent_status_from_event(&EventMsg::TaskStarted(TaskStartedEvent { + model_context_window: None, + })); + assert_eq!(status, Some(AgentStatus::Running)); + } + + #[tokio::test] + async fn on_event_updates_status_from_task_complete() { + let status = agent_status_from_event(&EventMsg::TaskComplete(TaskCompleteEvent { + last_agent_message: Some("done".to_string()), + })); + let expected = AgentStatus::Completed(Some("done".to_string())); + assert_eq!(status, Some(expected)); + } + + #[tokio::test] + async fn on_event_updates_status_from_error() { + let status = agent_status_from_event(&EventMsg::Error(ErrorEvent { + message: "boom".to_string(), + codex_error_info: None, + })); + + let expected = AgentStatus::Errored("boom".to_string()); + assert_eq!(status, Some(expected)); + } + + #[tokio::test] + async fn on_event_updates_status_from_turn_aborted() { + let status = agent_status_from_event(&EventMsg::TurnAborted(TurnAbortedEvent { + reason: TurnAbortReason::Interrupted, + })); + + let expected = AgentStatus::Errored("Interrupted".to_string()); + assert_eq!(status, Some(expected)); + } + + #[tokio::test] + async fn on_event_updates_status_from_shutdown_complete() { + let status = agent_status_from_event(&EventMsg::ShutdownComplete); + assert_eq!(status, Some(AgentStatus::Shutdown)); + } +} diff --git a/codex-rs/core/src/agent/mod.rs b/codex-rs/core/src/agent/mod.rs new file mode 100644 index 0000000000..d6348b38b3 --- /dev/null +++ b/codex-rs/core/src/agent/mod.rs @@ -0,0 +1,6 @@ +pub(crate) mod control; +pub(crate) mod status; + +pub(crate) use codex_protocol::protocol::AgentStatus; +pub(crate) use control::AgentControl; +pub(crate) use status::agent_status_from_event; diff --git a/codex-rs/core/src/agent/status.rs b/codex-rs/core/src/agent/status.rs new file mode 100644 index 0000000000..f5345a2910 --- /dev/null +++ b/codex-rs/core/src/agent/status.rs @@ -0,0 +1,15 @@ +use codex_protocol::protocol::AgentStatus; +use codex_protocol::protocol::EventMsg; + +/// Derive the next agent status from a single emitted event. +/// Returns `None` when the event does not affect status tracking. +pub(crate) fn agent_status_from_event(msg: &EventMsg) -> Option { + match msg { + EventMsg::TaskStarted(_) => Some(AgentStatus::Running), + EventMsg::TaskComplete(ev) => Some(AgentStatus::Completed(ev.last_agent_message.clone())), + EventMsg::TurnAborted(ev) => Some(AgentStatus::Errored(format!("{:?}", ev.reason))), + EventMsg::Error(ev) => Some(AgentStatus::Errored(ev.message.clone())), + EventMsg::ShutdownComplete => Some(AgentStatus::Shutdown), + _ => None, + } +} diff --git a/codex-rs/core/src/client_common.rs b/codex-rs/core/src/client_common.rs index 913bb22321..6972ff7cec 100644 --- a/codex-rs/core/src/client_common.rs +++ b/codex-rs/core/src/client_common.rs @@ -195,8 +195,13 @@ pub(crate) mod tools { LocalShell {}, // TODO: Understand why we get an error on web_search although the API docs say it's supported. // https://platform.openai.com/docs/guides/tools-web-search?api-mode=responses#:~:text=%7B%20type%3A%20%22web_search%22%20%7D%2C + // The `external_web_access` field determines whether the web search is over cached or live content. + // https://platform.openai.com/docs/guides/tools-web-search#live-internet-access #[serde(rename = "web_search")] - WebSearch {}, + WebSearch { + #[serde(skip_serializing_if = "Option::is_none")] + external_web_access: Option, + }, #[serde(rename = "custom")] Freeform(FreeformTool), } @@ -206,7 +211,7 @@ pub(crate) mod tools { match self { ToolSpec::Function(tool) => tool.name.as_str(), ToolSpec::LocalShell {} => "local_shell", - ToolSpec::WebSearch {} => "web_search", + ToolSpec::WebSearch { .. } => "web_search", ToolSpec::Freeform(tool) => tool.name.as_str(), } } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index bb4a45a75d..a4ea288e0d 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -8,6 +8,9 @@ use std::sync::atomic::Ordering; use crate::AuthManager; use crate::SandboxState; +use crate::agent::AgentControl; +use crate::agent::AgentStatus; +use crate::agent::agent_status_from_event; use crate::client_common::REVIEW_PROMPT; use crate::compact; use crate::compact::run_inline_auto_compact_task; @@ -143,7 +146,7 @@ use crate::tools::sandboxing::ApprovalStore; use crate::tools::spec::ToolsConfig; use crate::tools::spec::ToolsConfigParams; use crate::turn_diff_tracker::TurnDiffTracker; -use crate::unified_exec::UnifiedExecSessionManager; +use crate::unified_exec::UnifiedExecProcessManager; use crate::user_instructions::DeveloperInstructions; use crate::user_instructions::UserInstructions; use crate::user_notification::UserNotification; @@ -167,6 +170,8 @@ pub struct Codex { pub(crate) next_id: AtomicU64, pub(crate) tx_sub: Sender, pub(crate) rx_event: Receiver, + // Last known status of the agent. + pub(crate) agent_status: Arc>, } /// Wrapper returned by [`Codex::spawn`] containing the spawned [`Codex`], @@ -207,13 +212,14 @@ fn maybe_push_chat_wire_api_deprecation( impl Codex { /// Spawn a new [`Codex`] and initialize the session. - pub async fn spawn( + pub(crate) async fn spawn( config: Config, auth_manager: Arc, models_manager: Arc, skills_manager: Arc, conversation_history: InitialHistory, session_source: SessionSource, + agent_control: AgentControl, ) -> CodexResult { let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY); let (tx_event, rx_event) = async_channel::unbounded(); @@ -273,6 +279,7 @@ impl Codex { // Generate a unique ID for the lifetime of this Codex session. let session_source_clone = session_configuration.session_source.clone(); + let agent_status = Arc::new(RwLock::new(AgentStatus::PendingInit)); let session = Session::new( session_configuration, @@ -281,9 +288,11 @@ impl Codex { models_manager.clone(), exec_policy, tx_event.clone(), + Arc::clone(&agent_status), conversation_history, session_source_clone, skills_manager, + agent_control, ) .await .map_err(|e| { @@ -298,6 +307,7 @@ impl Codex { next_id: AtomicU64::new(0), tx_sub, rx_event, + agent_status, }; Ok(CodexSpawnOk { @@ -335,6 +345,11 @@ impl Codex { .map_err(|_| CodexErr::InternalAgentDied)?; Ok(event) } + + pub(crate) async fn agent_status(&self) -> AgentStatus { + let status = self.agent_status.read().await; + status.clone() + } } /// Context for an initialized model agent @@ -343,6 +358,7 @@ impl Codex { pub(crate) struct Session { conversation_id: ConversationId, tx_event: Sender, + agent_status: Arc>, state: Mutex, /// The set of enabled features should be invariant for the lifetime of the /// session. @@ -546,9 +562,11 @@ impl Session { models_manager: Arc, exec_policy: ExecPolicyManager, tx_event: Sender, + agent_status: Arc>, initial_history: InitialHistory, session_source: SessionSource, skills_manager: Arc, + agent_control: AgentControl, ) -> anyhow::Result> { debug!( "Configuring session: model={}; provider={:?}", @@ -659,7 +677,7 @@ impl Session { let services = SessionServices { mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::default())), mcp_startup_cancellation_token: CancellationToken::new(), - unified_exec_manager: UnifiedExecSessionManager::default(), + unified_exec_manager: UnifiedExecProcessManager::default(), notifier: UserNotifier::new(config.notify.clone()), rollout: Mutex::new(Some(rollout_recorder)), user_shell: Arc::new(default_shell), @@ -670,11 +688,13 @@ impl Session { models_manager: Arc::clone(&models_manager), tool_approvals: Mutex::new(ApprovalStore::default()), skills_manager, + agent_control, }; let sess = Arc::new(Session { conversation_id, tx_event: tx_event.clone(), + agent_status: Arc::clone(&agent_status), state: Mutex::new(state), features: config.features.clone(), active_turn: Mutex::new(None), @@ -995,6 +1015,11 @@ impl Session { } pub(crate) async fn send_event_raw(&self, event: Event) { + // Record the last known agent status. + if let Some(status) = agent_status_from_event(&event.msg) { + let mut guard = self.agent_status.write().await; + *guard = status; + } // Persist the event into rollout (recorder filters as needed) let rollout_items = vec![RolloutItem::EventMsg(event.msg.clone())]; self.persist_rollout_items(&rollout_items).await; @@ -1003,6 +1028,25 @@ impl Session { } } + /// Persist the event to the rollout file, flush it, and only then deliver it to clients. + /// + /// Most events can be delivered immediately after queueing the rollout write, but some + /// clients (e.g. app-server thread/rollback) re-read the rollout file synchronously on + /// receipt of the event and depend on the marker already being visible on disk. + pub(crate) async fn send_event_raw_flushed(&self, event: Event) { + // Record the last known agent status. + if let Some(status) = agent_status_from_event(&event.msg) { + let mut guard = self.agent_status.write().await; + *guard = status; + } + self.persist_rollout_items(&[RolloutItem::EventMsg(event.msg.clone())]) + .await; + self.flush_rollout().await; + if let Err(e) = self.tx_event.send(event).await { + error!("failed to send tool call event: {e}"); + } + } + pub(crate) async fn emit_turn_item_started(&self, turn_context: &TurnContext, item: &TurnItem) { self.send_event( turn_context, @@ -1220,6 +1264,9 @@ impl Session { history.replace(rebuilt); } } + RolloutItem::EventMsg(EventMsg::ThreadRolledBack(rollback)) => { + history.drop_last_n_user_turns(rollback.num_turns); + } _ => {} } } @@ -1659,6 +1706,9 @@ async fn submission_loop(sess: Arc, config: Arc, rx_sub: Receiv Op::Compact => { handlers::compact(&sess, sub.id.clone()).await; } + Op::ThreadRollback { num_turns } => { + handlers::thread_rollback(&sess, sub.id.clone(), num_turns).await; + } Op::RunUserShellCommand { command } => { handlers::run_user_shell_command( &sess, @@ -1716,6 +1766,7 @@ mod handlers { use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::ReviewRequest; use codex_protocol::protocol::SkillsListEntry; + use codex_protocol::protocol::ThreadRolledBackEvent; use codex_protocol::protocol::TurnAbortReason; use codex_protocol::protocol::WarningEvent; @@ -2035,11 +2086,51 @@ mod handlers { .await; } + pub async fn thread_rollback(sess: &Arc, sub_id: String, num_turns: u32) { + if num_turns == 0 { + sess.send_event_raw(Event { + id: sub_id, + msg: EventMsg::Error(ErrorEvent { + message: "num_turns must be >= 1".to_string(), + codex_error_info: Some(CodexErrorInfo::ThreadRollbackFailed), + }), + }) + .await; + return; + } + + let has_active_turn = { sess.active_turn.lock().await.is_some() }; + if has_active_turn { + sess.send_event_raw(Event { + id: sub_id, + msg: EventMsg::Error(ErrorEvent { + message: "Cannot rollback while a turn is in progress.".to_string(), + codex_error_info: Some(CodexErrorInfo::ThreadRollbackFailed), + }), + }) + .await; + return; + } + + let turn_context = sess.new_default_turn_with_sub_id(sub_id).await; + + let mut history = sess.clone_history().await; + history.drop_last_n_user_turns(num_turns); + sess.replace_history(history.get_history()).await; + sess.recompute_token_usage(turn_context.as_ref()).await; + + sess.send_event_raw_flushed(Event { + id: turn_context.sub_id.clone(), + msg: EventMsg::ThreadRolledBack(ThreadRolledBackEvent { num_turns }), + }) + .await; + } + pub async fn shutdown(sess: &Arc, sub_id: String) -> bool { sess.abort_all_tasks(TurnAbortReason::Interrupted).await; sess.services .unified_exec_manager - .terminate_all_sessions() + .terminate_all_processes() .await; info!("Shutting down Codex instance"); @@ -2121,6 +2212,7 @@ async fn spawn_review_thread( let mut review_features = sess.features.clone(); review_features .disable(crate::features::Feature::WebSearchRequest) + .disable(crate::features::Feature::WebSearchCached) .disable(crate::features::Feature::ViewImageTool); let tools_config = ToolsConfig::new(&ToolsConfigParams { model_family: &review_model_family, @@ -2948,6 +3040,131 @@ mod tests { assert_eq!(expected, actual); } + #[tokio::test] + async fn thread_rollback_drops_last_turn_from_history() { + let (sess, tc, rx) = make_session_and_context_with_rx().await; + + let initial_context = sess.build_initial_context(tc.as_ref()); + sess.record_into_history(&initial_context, tc.as_ref()) + .await; + + let turn_1 = vec![ + ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "turn 1 user".to_string(), + }], + }, + ResponseItem::Message { + id: None, + role: "assistant".to_string(), + content: vec![ContentItem::OutputText { + text: "turn 1 assistant".to_string(), + }], + }, + ]; + sess.record_into_history(&turn_1, tc.as_ref()).await; + + let turn_2 = vec![ + ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "turn 2 user".to_string(), + }], + }, + ResponseItem::Message { + id: None, + role: "assistant".to_string(), + content: vec![ContentItem::OutputText { + text: "turn 2 assistant".to_string(), + }], + }, + ]; + sess.record_into_history(&turn_2, tc.as_ref()).await; + + handlers::thread_rollback(&sess, "sub-1".to_string(), 1).await; + + let rollback_event = wait_for_thread_rolled_back(&rx).await; + assert_eq!(rollback_event.num_turns, 1); + + let mut expected = Vec::new(); + expected.extend(initial_context); + expected.extend(turn_1); + + let actual = sess.clone_history().await.get_history(); + assert_eq!(expected, actual); + } + + #[tokio::test] + async fn thread_rollback_clears_history_when_num_turns_exceeds_existing_turns() { + let (sess, tc, rx) = make_session_and_context_with_rx().await; + + let initial_context = sess.build_initial_context(tc.as_ref()); + sess.record_into_history(&initial_context, tc.as_ref()) + .await; + + let turn_1 = vec![ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "turn 1 user".to_string(), + }], + }]; + sess.record_into_history(&turn_1, tc.as_ref()).await; + + handlers::thread_rollback(&sess, "sub-1".to_string(), 99).await; + + let rollback_event = wait_for_thread_rolled_back(&rx).await; + assert_eq!(rollback_event.num_turns, 99); + + let actual = sess.clone_history().await.get_history(); + assert_eq!(initial_context, actual); + } + + #[tokio::test] + async fn thread_rollback_fails_when_turn_in_progress() { + let (sess, tc, rx) = make_session_and_context_with_rx().await; + + let initial_context = sess.build_initial_context(tc.as_ref()); + sess.record_into_history(&initial_context, tc.as_ref()) + .await; + + *sess.active_turn.lock().await = Some(crate::state::ActiveTurn::default()); + handlers::thread_rollback(&sess, "sub-1".to_string(), 1).await; + + let error_event = wait_for_thread_rollback_failed(&rx).await; + assert_eq!( + error_event.codex_error_info, + Some(CodexErrorInfo::ThreadRollbackFailed) + ); + + let actual = sess.clone_history().await.get_history(); + assert_eq!(initial_context, actual); + } + + #[tokio::test] + async fn thread_rollback_fails_when_num_turns_is_zero() { + let (sess, tc, rx) = make_session_and_context_with_rx().await; + + let initial_context = sess.build_initial_context(tc.as_ref()); + sess.record_into_history(&initial_context, tc.as_ref()) + .await; + + handlers::thread_rollback(&sess, "sub-1".to_string(), 0).await; + + let error_event = wait_for_thread_rollback_failed(&rx).await; + assert_eq!(error_event.message, "num_turns must be >= 1"); + assert_eq!( + error_event.codex_error_info, + Some(CodexErrorInfo::ThreadRollbackFailed) + ); + + let actual = sess.clone_history().await.get_history(); + assert_eq!(initial_context, actual); + } + #[tokio::test] async fn set_rate_limits_retains_previous_credits() { let codex_home = tempfile::tempdir().expect("create temp dir"); @@ -3181,6 +3398,44 @@ mod tests { assert_eq!(expected, got); } + async fn wait_for_thread_rolled_back( + rx: &async_channel::Receiver, + ) -> crate::protocol::ThreadRolledBackEvent { + let deadline = StdDuration::from_secs(2); + let start = std::time::Instant::now(); + loop { + let remaining = deadline.saturating_sub(start.elapsed()); + let evt = tokio::time::timeout(remaining, rx.recv()) + .await + .expect("timeout waiting for event") + .expect("event"); + match evt.msg { + EventMsg::ThreadRolledBack(payload) => return payload, + _ => continue, + } + } + } + + async fn wait_for_thread_rollback_failed(rx: &async_channel::Receiver) -> ErrorEvent { + let deadline = StdDuration::from_secs(2); + let start = std::time::Instant::now(); + loop { + let remaining = deadline.saturating_sub(start.elapsed()); + let evt = tokio::time::timeout(remaining, rx.recv()) + .await + .expect("timeout waiting for event") + .expect("event"); + match evt.msg { + EventMsg::Error(payload) + if payload.codex_error_info == Some(CodexErrorInfo::ThreadRollbackFailed) => + { + return payload; + } + _ => continue, + } + } + } + fn text_block(s: &str) -> ContentBlock { ContentBlock::TextContent(TextContent { annotations: None, @@ -3225,7 +3480,9 @@ mod tests { let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key")); let models_manager = Arc::new(ModelsManager::new(auth_manager.clone())); + let agent_control = AgentControl::default(); let exec_policy = ExecPolicyManager::default(); + let agent_status = Arc::new(RwLock::new(AgentStatus::PendingInit)); let model = ModelsManager::get_model_offline(config.model.as_deref()); let session_configuration = SessionConfiguration { provider: config.model_provider.clone(), @@ -3260,7 +3517,7 @@ mod tests { let services = SessionServices { mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::default())), mcp_startup_cancellation_token: CancellationToken::new(), - unified_exec_manager: UnifiedExecSessionManager::default(), + unified_exec_manager: UnifiedExecProcessManager::default(), notifier: UserNotifier::new(None), rollout: Mutex::new(None), user_shell: Arc::new(default_user_shell()), @@ -3271,6 +3528,7 @@ mod tests { models_manager: Arc::clone(&models_manager), tool_approvals: Mutex::new(ApprovalStore::default()), skills_manager, + agent_control, }; let turn_context = Session::make_turn_context( @@ -3287,6 +3545,7 @@ mod tests { let session = Session { conversation_id, tx_event, + agent_status: Arc::clone(&agent_status), state: Mutex::new(state), features: config.features.clone(), active_turn: Mutex::new(None), @@ -3312,7 +3571,9 @@ mod tests { let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key")); let models_manager = Arc::new(ModelsManager::new(auth_manager.clone())); + let agent_control = AgentControl::default(); let exec_policy = ExecPolicyManager::default(); + let agent_status = Arc::new(RwLock::new(AgentStatus::PendingInit)); let model = ModelsManager::get_model_offline(config.model.as_deref()); let session_configuration = SessionConfiguration { provider: config.model_provider.clone(), @@ -3347,7 +3608,7 @@ mod tests { let services = SessionServices { mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::default())), mcp_startup_cancellation_token: CancellationToken::new(), - unified_exec_manager: UnifiedExecSessionManager::default(), + unified_exec_manager: UnifiedExecProcessManager::default(), notifier: UserNotifier::new(None), rollout: Mutex::new(None), user_shell: Arc::new(default_user_shell()), @@ -3358,6 +3619,7 @@ mod tests { models_manager: Arc::clone(&models_manager), tool_approvals: Mutex::new(ApprovalStore::default()), skills_manager, + agent_control, }; let turn_context = Arc::new(Session::make_turn_context( @@ -3374,6 +3636,7 @@ mod tests { let session = Arc::new(Session { conversation_id, tx_event, + agent_status: Arc::clone(&agent_status), state: Mutex::new(state), features: config.features.clone(), active_turn: Mutex::new(None), @@ -3392,7 +3655,7 @@ mod tests { session.features = features; session - .record_model_warning("too many unified exec sessions", &turn_context) + .record_model_warning("too many unified exec processes", &turn_context) .await; let mut history = session.clone_history().await; @@ -3405,7 +3668,7 @@ mod tests { assert_eq!( content, &vec![ContentItem::InputText { - text: "Warning: too many unified exec sessions".to_string(), + text: "Warning: too many unified exec processes".to_string(), }] ); } diff --git a/codex-rs/core/src/codex_conversation.rs b/codex-rs/core/src/codex_conversation.rs index 5bb9c97c5b..723a4cf4f0 100644 --- a/codex-rs/core/src/codex_conversation.rs +++ b/codex-rs/core/src/codex_conversation.rs @@ -1,3 +1,4 @@ +use crate::agent::AgentStatus; use crate::codex::Codex; use crate::error::Result as CodexResult; use crate::protocol::Event; @@ -33,6 +34,10 @@ impl CodexConversation { self.codex.next_event().await } + pub async fn agent_status(&self) -> AgentStatus { + self.codex.agent_status().await + } + pub fn rollout_path(&self) -> PathBuf { self.rollout_path.clone() } diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index e767abf4ad..d7458f1acb 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -52,6 +52,7 @@ pub(crate) async fn run_codex_conversation_interactive( Arc::clone(&parent_session.services.skills_manager), initial_history.unwrap_or(InitialHistory::New), SessionSource::SubAgent(SubAgentSource::Review), + parent_session.services.agent_control.clone(), ) .await?; let codex = Arc::new(codex); @@ -86,6 +87,7 @@ pub(crate) async fn run_codex_conversation_interactive( next_id: AtomicU64::new(0), tx_sub: tx_ops, rx_event: rx_sub, + agent_status: Arc::clone(&codex.agent_status), }) } @@ -127,6 +129,7 @@ pub(crate) async fn run_codex_conversation_one_shot( // Bridge events so we can observe completion and shut down automatically. let (tx_bridge, rx_bridge) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY); let ops_tx = io.tx_sub.clone(); + let agent_status = Arc::clone(&io.agent_status); let io_for_bridge = io; tokio::spawn(async move { while let Ok(event) = io_for_bridge.next_event().await { @@ -158,6 +161,7 @@ pub(crate) async fn run_codex_conversation_one_shot( next_id: AtomicU64::new(0), rx_event: rx_bridge, tx_sub: tx_closed, + agent_status, }) } @@ -372,6 +376,7 @@ mod tests { next_id: AtomicU64::new(0), tx_sub, rx_event: rx_events, + agent_status: Default::default(), }); let (session, ctx, _rx_evt) = crate::codex::make_session_and_context_with_rx().await; diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 8eac13fd2e..035794f4aa 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -353,6 +353,10 @@ pub struct Config { /// or placeholder replacement will occur for fast keypress bursts. pub disable_paste_burst: bool, + /// When `false`, disables analytics across Codex product surfaces in this machine. + /// Defaults to `true`. + pub analytics: bool, + /// OTEL configuration (exporter type, endpoint, headers, etc.). pub otel: crate::config::types::OtelConfig, } @@ -813,6 +817,10 @@ pub struct ConfigToml { /// or placeholder replacement will occur for fast keypress bursts. pub disable_paste_burst: Option, + /// When `false`, disables analytics across Codex product surfaces in this machine. + /// Defaults to `true`. + pub analytics: Option, + /// OTEL configuration. pub otel: Option, @@ -1390,6 +1398,12 @@ impl Config { notices: cfg.notice.unwrap_or_default(), check_for_update_on_startup, disable_paste_burst: cfg.disable_paste_burst.unwrap_or(false), + analytics: config_profile + .analytics + .as_ref() + .and_then(|a| a.enabled) + .or(cfg.analytics.as_ref().and_then(|a| a.enabled)) + .unwrap_or(true), tui_notifications: cfg .tui .as_ref() @@ -3039,6 +3053,9 @@ approval_policy = "untrusted" # `ConfigOverrides`. profile = "gpt3" +[analytics] +enabled = true + [model_providers.openai-chat-completions] name = "OpenAI using Chat Completions" base_url = "https://api.openai.com/v1" @@ -3064,6 +3081,9 @@ model = "o3" model_provider = "openai" approval_policy = "on-failure" +[profiles.zdr.analytics] +enabled = false + [profiles.gpt5] model = "gpt-5.1" model_provider = "openai" @@ -3204,6 +3224,7 @@ model_verbosity = "high" tui_notifications: Default::default(), animations: true, show_tooltips: true, + analytics: true, tui_scroll_events_per_tick: None, tui_scroll_wheel_lines: None, tui_scroll_trackpad_lines: None, @@ -3287,6 +3308,7 @@ model_verbosity = "high" tui_notifications: Default::default(), animations: true, show_tooltips: true, + analytics: true, tui_scroll_events_per_tick: None, tui_scroll_wheel_lines: None, tui_scroll_trackpad_lines: None, @@ -3385,6 +3407,7 @@ model_verbosity = "high" tui_notifications: Default::default(), animations: true, show_tooltips: true, + analytics: false, tui_scroll_events_per_tick: None, tui_scroll_wheel_lines: None, tui_scroll_trackpad_lines: None, @@ -3469,6 +3492,7 @@ model_verbosity = "high" tui_notifications: Default::default(), animations: true, show_tooltips: true, + analytics: true, tui_scroll_events_per_tick: None, tui_scroll_wheel_lines: None, tui_scroll_trackpad_lines: None, diff --git a/codex-rs/core/src/config/profile.rs b/codex-rs/core/src/config/profile.rs index 9007064905..e1c45c1f16 100644 --- a/codex-rs/core/src/config/profile.rs +++ b/codex-rs/core/src/config/profile.rs @@ -29,6 +29,7 @@ pub struct ConfigProfile { pub experimental_use_freeform_apply_patch: Option, pub tools_web_search: Option, pub tools_view_image: Option, + pub analytics: Option, /// Optional feature toggles scoped to this profile. #[serde(default)] pub features: Option, diff --git a/codex-rs/core/src/config/types.rs b/codex-rs/core/src/config/types.rs index 3aa72d5ce5..6c421d4608 100644 --- a/codex-rs/core/src/config/types.rs +++ b/codex-rs/core/src/config/types.rs @@ -273,6 +273,15 @@ pub enum HistoryPersistence { None, } +// ===== Analytics configuration ===== + +/// Analytics settings loaded from config.toml. Fields are optional so we can apply defaults. +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default)] +pub struct AnalyticsConfigToml { + /// When `false`, disables analytics across Codex product surfaces in this profile. + pub enabled: Option, +} + // ===== OTEL configuration ===== #[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] diff --git a/codex-rs/core/src/context_manager/history.rs b/codex-rs/core/src/context_manager/history.rs index c18ad7df8e..e52561c9fe 100644 --- a/codex-rs/core/src/context_manager/history.rs +++ b/codex-rs/core/src/context_manager/history.rs @@ -5,6 +5,9 @@ use crate::truncate::approx_token_count; use crate::truncate::approx_tokens_from_byte_count; use crate::truncate::truncate_function_output_items_with_policy; use crate::truncate::truncate_text; +use crate::user_instructions::SkillInstructions; +use crate::user_instructions::UserInstructions; +use crate::user_shell_command::is_user_shell_command_text; use codex_protocol::models::ContentItem; use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::FunctionCallOutputPayload; @@ -152,6 +155,39 @@ impl ContextManager { } } + /// Drop the last `num_turns` user turns from this history. + /// + /// "User turns" are identified as `ResponseItem::Message` entries whose role is `"user"`. + /// + /// This mirrors thread-rollback semantics: + /// - `num_turns == 0` is a no-op + /// - if there are no user turns, this is a no-op + /// - if `num_turns` exceeds the number of user turns, all user turns are dropped while + /// preserving any items that occurred before the first user message. + pub(crate) fn drop_last_n_user_turns(&mut self, num_turns: u32) { + if num_turns == 0 { + return; + } + + // Keep behavior consistent with call sites that previously operated on `get_history()`: + // normalize first (call/output invariants), then truncate based on the normalized view. + let snapshot = self.get_history(); + let user_positions = user_message_positions(&snapshot); + let Some(&first_user_idx) = user_positions.first() else { + self.replace(snapshot); + return; + }; + + let n_from_end = usize::try_from(num_turns).unwrap_or(usize::MAX); + let cut_idx = if n_from_end >= user_positions.len() { + first_user_idx + } else { + user_positions[user_positions.len() - n_from_end] + }; + + self.replace(snapshot[..cut_idx].to_vec()); + } + pub(crate) fn update_token_info( &mut self, usage: &TokenUsage, @@ -291,6 +327,56 @@ fn estimate_reasoning_length(encoded_len: usize) -> usize { .saturating_sub(650) } +fn is_session_prefix(text: &str) -> bool { + let trimmed = text.trim_start(); + let lowered = trimmed.to_ascii_lowercase(); + lowered.starts_with("") +} + +fn is_user_turn_boundary(item: &ResponseItem) -> bool { + let ResponseItem::Message { role, content, .. } = item else { + return false; + }; + + if role != "user" { + return false; + } + + if UserInstructions::is_user_instructions(content) + || SkillInstructions::is_skill_instructions(content) + { + return false; + } + + for content_item in content { + match content_item { + ContentItem::InputText { text } => { + if is_session_prefix(text) || is_user_shell_command_text(text) { + return false; + } + } + ContentItem::OutputText { text } => { + if is_session_prefix(text) { + return false; + } + } + ContentItem::InputImage { .. } => {} + } + } + + true +} + +fn user_message_positions(items: &[ResponseItem]) -> Vec { + let mut positions = Vec::new(); + for (idx, item) in items.iter().enumerate() { + if is_user_turn_boundary(item) { + positions.push(idx); + } + } + positions +} + #[cfg(test)] #[path = "history_tests.rs"] mod tests; diff --git a/codex-rs/core/src/context_manager/history_tests.rs b/codex-rs/core/src/context_manager/history_tests.rs index d121b7dc63..1fb47e1e66 100644 --- a/codex-rs/core/src/context_manager/history_tests.rs +++ b/codex-rs/core/src/context_manager/history_tests.rs @@ -43,6 +43,16 @@ fn user_msg(text: &str) -> ResponseItem { } } +fn user_input_text_msg(text: &str) -> ResponseItem { + ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: text.to_string(), + }], + } +} + fn reasoning_msg(text: &str) -> ResponseItem { ResponseItem::Reasoning { id: String::new(), @@ -227,6 +237,127 @@ fn remove_first_item_handles_local_shell_pair() { assert_eq!(h.contents(), vec![]); } +#[test] +fn drop_last_n_user_turns_preserves_prefix() { + let items = vec![ + assistant_msg("session prefix item"), + user_msg("u1"), + assistant_msg("a1"), + user_msg("u2"), + assistant_msg("a2"), + ]; + + let mut history = create_history_with_items(items); + history.drop_last_n_user_turns(1); + assert_eq!( + history.get_history(), + vec![ + assistant_msg("session prefix item"), + user_msg("u1"), + assistant_msg("a1"), + ] + ); + + let mut history = create_history_with_items(vec![ + assistant_msg("session prefix item"), + user_msg("u1"), + assistant_msg("a1"), + user_msg("u2"), + assistant_msg("a2"), + ]); + history.drop_last_n_user_turns(99); + assert_eq!( + history.get_history(), + vec![assistant_msg("session prefix item")] + ); +} + +#[test] +fn drop_last_n_user_turns_ignores_session_prefix_user_messages() { + let items = vec![ + user_input_text_msg("ctx"), + user_input_text_msg("do the thing"), + user_input_text_msg( + "# AGENTS.md instructions for test_directory\n\n\ntest_text\n", + ), + user_input_text_msg( + "\ndemo\nskills/demo/SKILL.md\nbody\n", + ), + user_input_text_msg("echo 42"), + user_input_text_msg("turn 1 user"), + assistant_msg("turn 1 assistant"), + user_input_text_msg("turn 2 user"), + assistant_msg("turn 2 assistant"), + ]; + + let mut history = create_history_with_items(items); + history.drop_last_n_user_turns(1); + + let expected_prefix_and_first_turn = vec![ + user_input_text_msg("ctx"), + user_input_text_msg("do the thing"), + user_input_text_msg( + "# AGENTS.md instructions for test_directory\n\n\ntest_text\n", + ), + user_input_text_msg( + "\ndemo\nskills/demo/SKILL.md\nbody\n", + ), + user_input_text_msg("echo 42"), + user_input_text_msg("turn 1 user"), + assistant_msg("turn 1 assistant"), + ]; + + assert_eq!(history.get_history(), expected_prefix_and_first_turn); + + let expected_prefix_only = vec![ + user_input_text_msg("ctx"), + user_input_text_msg("do the thing"), + user_input_text_msg( + "# AGENTS.md instructions for test_directory\n\n\ntest_text\n", + ), + user_input_text_msg( + "\ndemo\nskills/demo/SKILL.md\nbody\n", + ), + user_input_text_msg("echo 42"), + ]; + + let mut history = create_history_with_items(vec![ + user_input_text_msg("ctx"), + user_input_text_msg("do the thing"), + user_input_text_msg( + "# AGENTS.md instructions for test_directory\n\n\ntest_text\n", + ), + user_input_text_msg( + "\ndemo\nskills/demo/SKILL.md\nbody\n", + ), + user_input_text_msg("echo 42"), + user_input_text_msg("turn 1 user"), + assistant_msg("turn 1 assistant"), + user_input_text_msg("turn 2 user"), + assistant_msg("turn 2 assistant"), + ]); + history.drop_last_n_user_turns(2); + assert_eq!(history.get_history(), expected_prefix_only); + + let mut history = create_history_with_items(vec![ + user_input_text_msg("ctx"), + user_input_text_msg("do the thing"), + user_input_text_msg( + "# AGENTS.md instructions for test_directory\n\n\ntest_text\n", + ), + user_input_text_msg( + "\ndemo\nskills/demo/SKILL.md\nbody\n", + ), + user_input_text_msg("echo 42"), + user_input_text_msg("turn 1 user"), + assistant_msg("turn 1 assistant"), + user_input_text_msg("turn 2 user"), + assistant_msg("turn 2 assistant"), + ]); + history.drop_last_n_user_turns(3); + assert_eq!(history.get_history(), expected_prefix_only); +} + #[test] fn remove_first_item_handles_custom_tool_pair() { let items = vec![ diff --git a/codex-rs/core/src/conversation_manager.rs b/codex-rs/core/src/conversation_manager.rs index 5093e03c60..cf0f0106db 100644 --- a/codex-rs/core/src/conversation_manager.rs +++ b/codex-rs/core/src/conversation_manager.rs @@ -3,6 +3,7 @@ use crate::AuthManager; use crate::CodexAuth; #[cfg(any(test, feature = "test-support"))] use crate::ModelProviderInfo; +use crate::agent::AgentControl; use crate::codex::Codex; use crate::codex::CodexSpawnOk; use crate::codex::INITIAL_SUBMIT_ID; @@ -15,12 +16,12 @@ use crate::protocol::Event; use crate::protocol::EventMsg; use crate::protocol::SessionConfiguredEvent; use crate::rollout::RolloutRecorder; +use crate::rollout::truncation; use crate::skills::SkillsManager; use codex_protocol::ConversationId; -use codex_protocol::items::TurnItem; -use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::ModelPreset; use codex_protocol::protocol::InitialHistory; +use codex_protocol::protocol::Op; use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::SessionSource; use std::collections::HashMap; @@ -41,24 +42,34 @@ pub struct NewConversation { /// [`ConversationManager`] is responsible for creating conversations and /// maintaining them in memory. pub struct ConversationManager { + state: Arc, + #[cfg(any(test, feature = "test-support"))] + _test_codex_home_guard: Option, +} + +/// Shared, `Arc`-owned state for [`ConversationManager`]. This `Arc` is required to have a single +/// `Arc` reference that can be downgraded to by `AgentControl` while preventing every single +/// function to require an `Arc<&Self>`. +pub(crate) struct ConversationManagerState { conversations: Arc>>>, auth_manager: Arc, models_manager: Arc, skills_manager: Arc, session_source: SessionSource, - #[cfg(any(test, feature = "test-support"))] - _test_codex_home_guard: Option, } impl ConversationManager { pub fn new(auth_manager: Arc, session_source: SessionSource) -> Self { - let skills_manager = Arc::new(SkillsManager::new(auth_manager.codex_home().to_path_buf())); Self { - conversations: Arc::new(RwLock::new(HashMap::new())), - auth_manager: auth_manager.clone(), - session_source, - models_manager: Arc::new(ModelsManager::new(auth_manager)), - skills_manager, + state: Arc::new(ConversationManagerState { + conversations: Arc::new(RwLock::new(HashMap::new())), + models_manager: Arc::new(ModelsManager::new(auth_manager.clone())), + skills_manager: Arc::new(SkillsManager::new( + auth_manager.codex_home().to_path_buf(), + )), + auth_manager, + session_source, + }), #[cfg(any(test, feature = "test-support"))] _test_codex_home_guard: None, } @@ -83,40 +94,165 @@ impl ConversationManager { provider: ModelProviderInfo, codex_home: PathBuf, ) -> Self { - let auth_manager = crate::AuthManager::from_auth_for_testing_with_home(auth, codex_home); - let skills_manager = Arc::new(SkillsManager::new(auth_manager.codex_home().to_path_buf())); + let auth_manager = AuthManager::from_auth_for_testing_with_home(auth, codex_home); Self { - conversations: Arc::new(RwLock::new(HashMap::new())), - auth_manager: auth_manager.clone(), - session_source: SessionSource::Exec, - models_manager: Arc::new(ModelsManager::with_provider(auth_manager, provider)), - skills_manager, + state: Arc::new(ConversationManagerState { + conversations: Arc::new(RwLock::new(HashMap::new())), + models_manager: Arc::new(ModelsManager::with_provider( + auth_manager.clone(), + provider, + )), + skills_manager: Arc::new(SkillsManager::new( + auth_manager.codex_home().to_path_buf(), + )), + auth_manager, + session_source: SessionSource::Exec, + }), _test_codex_home_guard: None, } } pub fn session_source(&self) -> SessionSource { - self.session_source.clone() + self.state.session_source.clone() } pub fn skills_manager(&self) -> Arc { - self.skills_manager.clone() + self.state.skills_manager.clone() + } + + pub fn get_models_manager(&self) -> Arc { + self.state.models_manager.clone() + } + + pub async fn list_models(&self, config: &Config) -> Vec { + self.state.models_manager.list_models(config).await + } + + pub async fn get_conversation( + &self, + conversation_id: ConversationId, + ) -> CodexResult> { + self.state.get_conversation(conversation_id).await } pub async fn new_conversation(&self, config: Config) -> CodexResult { + self.state + .spawn_conversation( + config, + InitialHistory::New, + Arc::clone(&self.state.auth_manager), + self.agent_control(), + ) + .await + } + + pub async fn resume_conversation_from_rollout( + &self, + config: Config, + rollout_path: PathBuf, + auth_manager: Arc, + ) -> CodexResult { + let initial_history = RolloutRecorder::get_rollout_history(&rollout_path).await?; + self.resume_conversation_with_history(config, initial_history, auth_manager) + .await + } + + pub async fn resume_conversation_with_history( + &self, + config: Config, + initial_history: InitialHistory, + auth_manager: Arc, + ) -> CodexResult { + self.state + .spawn_conversation(config, initial_history, auth_manager, self.agent_control()) + .await + } + + /// Removes the conversation from the manager's internal map, though the conversation is stored + /// as `Arc`, it is possible that other references to it exist elsewhere. + /// Returns the conversation if the conversation was found and removed. + pub async fn remove_conversation( + &self, + conversation_id: &ConversationId, + ) -> Option> { + self.state + .conversations + .write() + .await + .remove(conversation_id) + } + + /// Fork an existing conversation by taking messages up to the given position (not including + /// the message at the given position) and starting a new conversation with identical + /// configuration (unless overridden by the caller's `config`). The new conversation will have + /// a fresh id. + pub async fn fork_conversation( + &self, + nth_user_message: usize, + config: Config, + path: PathBuf, + ) -> CodexResult { + let history = RolloutRecorder::get_rollout_history(&path).await?; + let history = truncate_before_nth_user_message(history, nth_user_message); + self.state + .spawn_conversation( + config, + history, + Arc::clone(&self.state.auth_manager), + self.agent_control(), + ) + .await + } + + fn agent_control(&self) -> AgentControl { + AgentControl::new(Arc::downgrade(&self.state)) + } +} + +impl ConversationManagerState { + pub(crate) async fn get_conversation( + &self, + conversation_id: ConversationId, + ) -> CodexResult> { + let conversations = self.conversations.read().await; + conversations + .get(&conversation_id) + .cloned() + .ok_or_else(|| CodexErr::ConversationNotFound(conversation_id)) + } + + pub(crate) async fn send_op( + &self, + conversation_id: ConversationId, + op: Op, + ) -> CodexResult { + self.get_conversation(conversation_id) + .await? + .submit(op) + .await + } + + #[allow(dead_code)] // Used by upcoming multi-agent tooling. + pub(crate) async fn spawn_new_conversation( + &self, + config: Config, + agent_control: AgentControl, + ) -> CodexResult { self.spawn_conversation( config, - self.auth_manager.clone(), - self.models_manager.clone(), + InitialHistory::New, + Arc::clone(&self.auth_manager), + agent_control, ) .await } - async fn spawn_conversation( + pub(crate) async fn spawn_conversation( &self, config: Config, + initial_history: InitialHistory, auth_manager: Arc, - models_manager: Arc, + agent_control: AgentControl, ) -> CodexResult { let CodexSpawnOk { codex, @@ -124,10 +260,11 @@ impl ConversationManager { } = Codex::spawn( config, auth_manager, - models_manager, - self.skills_manager.clone(), - InitialHistory::New, + Arc::clone(&self.models_manager), + Arc::clone(&self.skills_manager), + initial_history, self.session_source.clone(), + agent_control, ) .await?; self.finalize_spawn(codex, conversation_id).await @@ -138,9 +275,6 @@ impl ConversationManager { codex: Codex, conversation_id: ConversationId, ) -> CodexResult { - // The first event must be `SessionInitialized`. Validate and forward it - // to the caller so that they can display it in the conversation - // history. let event = codex.next_event().await?; let session_configured = match event { Event { @@ -167,129 +301,13 @@ impl ConversationManager { session_configured, }) } - - pub async fn get_conversation( - &self, - conversation_id: ConversationId, - ) -> CodexResult> { - let conversations = self.conversations.read().await; - conversations - .get(&conversation_id) - .cloned() - .ok_or_else(|| CodexErr::ConversationNotFound(conversation_id)) - } - - pub async fn resume_conversation_from_rollout( - &self, - config: Config, - rollout_path: PathBuf, - auth_manager: Arc, - ) -> CodexResult { - let initial_history = RolloutRecorder::get_rollout_history(&rollout_path).await?; - self.resume_conversation_with_history(config, initial_history, auth_manager) - .await - } - - pub async fn resume_conversation_with_history( - &self, - config: Config, - initial_history: InitialHistory, - auth_manager: Arc, - ) -> CodexResult { - let CodexSpawnOk { - codex, - conversation_id, - } = Codex::spawn( - config, - auth_manager, - self.models_manager.clone(), - self.skills_manager.clone(), - initial_history, - self.session_source.clone(), - ) - .await?; - self.finalize_spawn(codex, conversation_id).await - } - - /// Removes the conversation from the manager's internal map, though the - /// conversation is stored as `Arc`, it is possible that - /// other references to it exist elsewhere. Returns the conversation if the - /// conversation was found and removed. - pub async fn remove_conversation( - &self, - conversation_id: &ConversationId, - ) -> Option> { - self.conversations.write().await.remove(conversation_id) - } - - /// Fork an existing conversation by taking messages up to the given position - /// (not including the message at the given position) and starting a new - /// conversation with identical configuration (unless overridden by the - /// caller's `config`). The new conversation will have a fresh id. - pub async fn fork_conversation( - &self, - nth_user_message: usize, - config: Config, - path: PathBuf, - ) -> CodexResult { - // Compute the prefix up to the cut point. - let history = RolloutRecorder::get_rollout_history(&path).await?; - let history = truncate_before_nth_user_message(history, nth_user_message); - - // Spawn a new conversation with the computed initial history. - let auth_manager = self.auth_manager.clone(); - let CodexSpawnOk { - codex, - conversation_id, - } = Codex::spawn( - config, - auth_manager, - self.models_manager.clone(), - self.skills_manager.clone(), - history, - self.session_source.clone(), - ) - .await?; - - self.finalize_spawn(codex, conversation_id).await - } - - pub async fn list_models(&self, config: &Config) -> Vec { - self.models_manager.list_models(config).await - } - - pub fn get_models_manager(&self) -> Arc { - self.models_manager.clone() - } } /// Return a prefix of `items` obtained by cutting strictly before the nth user message /// (0-based) and all items that follow it. fn truncate_before_nth_user_message(history: InitialHistory, n: usize) -> InitialHistory { - // Work directly on rollout items, and cut the vector at the nth user message input. let items: Vec = history.get_rollout_items(); - - // Find indices of user message inputs in rollout order. - let mut user_positions: Vec = Vec::new(); - for (idx, item) in items.iter().enumerate() { - if let RolloutItem::ResponseItem(item @ ResponseItem::Message { .. }) = item - && matches!( - crate::event_mapping::parse_turn_item(item), - Some(TurnItem::UserMessage(_)) - ) - { - user_positions.push(idx); - } - } - - // If fewer than or equal to n user messages exist, treat as empty (out of range). - if user_positions.len() <= n { - return InitialHistory::New; - } - - // Cut strictly before the nth user message (do not keep the nth itself). - let cut_idx = user_positions[n]; - let rolled: Vec = items.into_iter().take(cut_idx).collect(); + let rolled = truncation::truncate_rollout_before_nth_user_message_from_start(&items, n); if rolled.is_empty() { InitialHistory::New @@ -345,14 +363,13 @@ mod tests { }, ResponseItem::FunctionCall { id: None, + call_id: "c1".to_string(), name: "tool".to_string(), arguments: "{}".to_string(), - call_id: "c1".to_string(), }, assistant_msg("a4"), ]; - // Wrap as InitialHistory::Forked with response items only. let initial: Vec = items .iter() .cloned() diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index 3b22bfc3f4..2c1dcc9582 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -72,8 +72,11 @@ pub enum Feature { UnifiedExec, /// Include the freeform apply_patch tool. ApplyPatchFreeform, - /// Allow the model to request web searches. + /// Allow the model to request web searches that fetch live content. WebSearchRequest, + /// Allow the model to request web searches that fetch cached content. + /// Takes precedence over `WebSearchRequest`. + WebSearchCached, /// Gate the execpolicy enforcement for shell/unified exec. ExecPolicy, /// Enable Windows sandbox (restricted token) on Windows. @@ -330,6 +333,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::Stable, default_enabled: false, }, + FeatureSpec { + id: Feature::WebSearchCached, + key: "web_search_cached", + stage: Stage::Experimental, + default_enabled: false, + }, // Beta program. Rendered in the `/experimental` menu for users. FeatureSpec { id: Feature::UnifiedExec, @@ -337,7 +346,7 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::Beta { name: "Background terminal", menu_description: "Run long-running terminal commands in the background.", - announcement: "NEW! Try Background terminals for long running processes. Enable in /experimental!", + announcement: "NEW! Try Background terminals for long-running commands. Enable in /experimental!", }, default_enabled: false, }, diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 8794484083..b9f2645b13 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -15,6 +15,7 @@ pub mod codex; mod codex_conversation; mod compact_remote; pub use codex_conversation::CodexConversation; +mod agent; mod codex_delegate; mod command_safety; pub mod config; diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index 060f4a5c27..29be88735b 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -149,14 +149,14 @@ impl ModelsManager { // if codex-auto-balanced exists & signed in with chatgpt mode, return it, otherwise return the default model let auth_mode = self.auth_manager.get_auth_mode(); let remote_models = self.remote_models(config).await; - if auth_mode == Some(AuthMode::ChatGPT) - && self + if auth_mode == Some(AuthMode::ChatGPT) { + let has_auto_balanced = self .build_available_models(remote_models) .iter() - .any(|m| m.model == CODEX_AUTO_BALANCED_MODEL) - { - return CODEX_AUTO_BALANCED_MODEL.to_string(); - } else if auth_mode == Some(AuthMode::ChatGPT) { + .any(|model| model.model == CODEX_AUTO_BALANCED_MODEL && model.show_in_picker); + if has_auto_balanced { + return CODEX_AUTO_BALANCED_MODEL.to_string(); + } return OPENAI_DEFAULT_CHATGPT_MODEL.to_string(); } OPENAI_DEFAULT_API_MODEL.to_string() @@ -247,10 +247,15 @@ impl ModelsManager { merged_presets = self.filter_visible_models(merged_presets); let has_default = merged_presets.iter().any(|preset| preset.is_default); - if let Some(default) = merged_presets.first_mut() - && !has_default - { - default.is_default = true; + if !has_default { + if let Some(default) = merged_presets + .iter_mut() + .find(|preset| preset.show_in_picker) + { + default.is_default = true; + } else if let Some(default) = merged_presets.first_mut() { + default.is_default = true; + } } merged_presets @@ -260,7 +265,7 @@ impl ModelsManager { let chatgpt_mode = self.auth_manager.get_auth_mode() == Some(AuthMode::ChatGPT); models .into_iter() - .filter(|model| model.show_in_picker && (chatgpt_mode || model.supported_in_api)) + .filter(|model| chatgpt_mode || model.supported_in_api) .collect() } @@ -654,12 +659,13 @@ mod tests { let hidden_model = remote_model_with_visibility("hidden", "Hidden", 0, "hide"); let visible_model = remote_model_with_visibility("visible", "Visible", 1, "list"); - let mut expected = ModelPreset::from(visible_model.clone()); - expected.is_default = true; + let expected_hidden = ModelPreset::from(hidden_model.clone()); + let mut expected_visible = ModelPreset::from(visible_model.clone()); + expected_visible.is_default = true; let available = manager.build_available_models(vec![hidden_model, visible_model]); - assert_eq!(available, vec![expected]); + assert_eq!(available, vec![expected_hidden, expected_visible]); } #[test] diff --git a/codex-rs/core/src/models_manager/model_presets.rs b/codex-rs/core/src/models_manager/model_presets.rs index 0a7e785784..080c44433b 100644 --- a/codex-rs/core/src/models_manager/model_presets.rs +++ b/codex-rs/core/src/models_manager/model_presets.rs @@ -112,7 +112,7 @@ static PRESETS: Lazy> = Lazy::new(|| { }, ReasoningEffortPreset { effort: ReasoningEffort::XHigh, - description: "Extra high reasoning for complex problems".to_string(), + description: "Extra high reasoning depth for complex problems".to_string(), }, ], is_default: false, @@ -170,7 +170,7 @@ static PRESETS: Lazy> = Lazy::new(|| { }, ReasoningEffortPreset { effort: ReasoningEffort::XHigh, - description: "Extra high reasoning for complex problems".to_string(), + description: "Extra high reasoning depth for complex problems".to_string(), }, ], is_default: false, @@ -322,11 +322,7 @@ fn gpt_52_codex_upgrade() -> ModelUpgrade { } pub(super) fn builtin_model_presets(_auth_mode: Option) -> Vec { - PRESETS - .iter() - .filter(|preset| preset.show_in_picker) - .cloned() - .collect() + PRESETS.iter().cloned().collect() } #[cfg(any(test, feature = "test-support"))] diff --git a/codex-rs/core/src/rollout/mod.rs b/codex-rs/core/src/rollout/mod.rs index 540d204be3..d7e24602fd 100644 --- a/codex-rs/core/src/rollout/mod.rs +++ b/codex-rs/core/src/rollout/mod.rs @@ -11,6 +11,7 @@ pub(crate) mod error; pub mod list; pub(crate) mod policy; pub mod recorder; +pub(crate) mod truncation; pub use codex_protocol::protocol::SessionMeta; pub(crate) use error::map_session_init_error; diff --git a/codex-rs/core/src/rollout/policy.rs b/codex-rs/core/src/rollout/policy.rs index 07c8af1144..6c02ad0942 100644 --- a/codex-rs/core/src/rollout/policy.rs +++ b/codex-rs/core/src/rollout/policy.rs @@ -45,6 +45,7 @@ pub(crate) fn should_persist_event_msg(ev: &EventMsg) -> bool { | EventMsg::ContextCompacted(_) | EventMsg::EnteredReviewMode(_) | EventMsg::ExitedReviewMode(_) + | EventMsg::ThreadRolledBack(_) | EventMsg::UndoCompleted(_) | EventMsg::TurnAborted(_) => true, EventMsg::Error(_) diff --git a/codex-rs/core/src/rollout/truncation.rs b/codex-rs/core/src/rollout/truncation.rs new file mode 100644 index 0000000000..b8127f0345 --- /dev/null +++ b/codex-rs/core/src/rollout/truncation.rs @@ -0,0 +1,195 @@ +//! Helpers for truncating rollouts based on "user turn" boundaries. +//! +//! In core, "user turns" are detected by scanning `ResponseItem::Message` items and +//! interpreting them via `event_mapping::parse_turn_item(...)`. + +use crate::event_mapping; +use codex_protocol::items::TurnItem; +use codex_protocol::models::ResponseItem; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::RolloutItem; + +/// Return the indices of user message boundaries in a rollout. +/// +/// A user message boundary is a `RolloutItem::ResponseItem(ResponseItem::Message { .. })` +/// whose parsed turn item is `TurnItem::UserMessage`. +/// +/// Rollouts can contain `ThreadRolledBack` markers. Those markers indicate that the +/// last N user turns were removed from the effective thread history; we apply them here so +/// indexing uses the post-rollback history rather than the raw stream. +pub(crate) fn user_message_positions_in_rollout(items: &[RolloutItem]) -> Vec { + let mut user_positions = Vec::new(); + for (idx, item) in items.iter().enumerate() { + match item { + RolloutItem::ResponseItem(item @ ResponseItem::Message { .. }) + if matches!( + event_mapping::parse_turn_item(item), + Some(TurnItem::UserMessage(_)) + ) => + { + user_positions.push(idx); + } + RolloutItem::EventMsg(EventMsg::ThreadRolledBack(rollback)) => { + let num_turns = usize::try_from(rollback.num_turns).unwrap_or(usize::MAX); + let new_len = user_positions.len().saturating_sub(num_turns); + user_positions.truncate(new_len); + } + _ => {} + } + } + user_positions +} + +/// Return a prefix of `items` obtained by cutting strictly before the nth user message. +/// +/// The boundary index is 0-based from the start of `items` (so `n_from_start = 0` returns +/// a prefix that excludes the first user message and everything after it). +/// +/// If fewer than or equal to `n_from_start` user messages exist, this returns an empty +/// vector (out of range). +pub(crate) fn truncate_rollout_before_nth_user_message_from_start( + items: &[RolloutItem], + n_from_start: usize, +) -> Vec { + let user_positions = user_message_positions_in_rollout(items); + + // If fewer than or equal to n user messages exist, treat as empty (out of range). + if user_positions.len() <= n_from_start { + return Vec::new(); + } + + // Cut strictly before the nth user message (do not keep the nth itself). + let cut_idx = user_positions[n_from_start]; + items[..cut_idx].to_vec() +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::codex::make_session_and_context; + use assert_matches::assert_matches; + use codex_protocol::models::ContentItem; + use codex_protocol::models::ReasoningItemReasoningSummary; + use codex_protocol::protocol::ThreadRolledBackEvent; + use pretty_assertions::assert_eq; + + fn user_msg(text: &str) -> ResponseItem { + ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::OutputText { + text: text.to_string(), + }], + } + } + + fn assistant_msg(text: &str) -> ResponseItem { + ResponseItem::Message { + id: None, + role: "assistant".to_string(), + content: vec![ContentItem::OutputText { + text: text.to_string(), + }], + } + } + + #[test] + fn truncates_rollout_from_start_before_nth_user_only() { + let items = [ + user_msg("u1"), + assistant_msg("a1"), + assistant_msg("a2"), + user_msg("u2"), + assistant_msg("a3"), + ResponseItem::Reasoning { + id: "r1".to_string(), + summary: vec![ReasoningItemReasoningSummary::SummaryText { + text: "s".to_string(), + }], + content: None, + encrypted_content: None, + }, + ResponseItem::FunctionCall { + id: None, + name: "tool".to_string(), + arguments: "{}".to_string(), + call_id: "c1".to_string(), + }, + assistant_msg("a4"), + ]; + + let rollout: Vec = items + .iter() + .cloned() + .map(RolloutItem::ResponseItem) + .collect(); + + let truncated = truncate_rollout_before_nth_user_message_from_start(&rollout, 1); + let expected = vec![ + RolloutItem::ResponseItem(items[0].clone()), + RolloutItem::ResponseItem(items[1].clone()), + RolloutItem::ResponseItem(items[2].clone()), + ]; + assert_eq!( + serde_json::to_value(&truncated).unwrap(), + serde_json::to_value(&expected).unwrap() + ); + + let truncated2 = truncate_rollout_before_nth_user_message_from_start(&rollout, 2); + assert_matches!(truncated2.as_slice(), []); + } + + #[test] + fn truncates_rollout_from_start_applies_thread_rollback_markers() { + let rollout_items = vec![ + RolloutItem::ResponseItem(user_msg("u1")), + RolloutItem::ResponseItem(assistant_msg("a1")), + RolloutItem::ResponseItem(user_msg("u2")), + RolloutItem::ResponseItem(assistant_msg("a2")), + RolloutItem::EventMsg(EventMsg::ThreadRolledBack(ThreadRolledBackEvent { + num_turns: 1, + })), + RolloutItem::ResponseItem(user_msg("u3")), + RolloutItem::ResponseItem(assistant_msg("a3")), + RolloutItem::ResponseItem(user_msg("u4")), + RolloutItem::ResponseItem(assistant_msg("a4")), + ]; + + // Effective user history after applying rollback(1) is: u1, u3, u4. + // So n_from_start=2 should cut before u4 (not u3). + let truncated = truncate_rollout_before_nth_user_message_from_start(&rollout_items, 2); + let expected = rollout_items[..7].to_vec(); + assert_eq!( + serde_json::to_value(&truncated).unwrap(), + serde_json::to_value(&expected).unwrap() + ); + } + + #[tokio::test] + async fn ignores_session_prefix_messages_when_truncating_rollout_from_start() { + let (session, turn_context) = make_session_and_context().await; + let mut items = session.build_initial_context(&turn_context); + items.push(user_msg("feature request")); + items.push(assistant_msg("ack")); + items.push(user_msg("second question")); + items.push(assistant_msg("answer")); + + let rollout_items: Vec = items + .iter() + .cloned() + .map(RolloutItem::ResponseItem) + .collect(); + + let truncated = truncate_rollout_before_nth_user_message_from_start(&rollout_items, 1); + let expected: Vec = vec![ + RolloutItem::ResponseItem(items[0].clone()), + RolloutItem::ResponseItem(items[1].clone()), + RolloutItem::ResponseItem(items[2].clone()), + ]; + + assert_eq!( + serde_json::to_value(&truncated).unwrap(), + serde_json::to_value(&expected).unwrap() + ); + } +} diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index 63b66647e5..942df35c02 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -2,12 +2,13 @@ use std::sync::Arc; use crate::AuthManager; use crate::RolloutRecorder; +use crate::agent::AgentControl; use crate::exec_policy::ExecPolicyManager; use crate::mcp_connection_manager::McpConnectionManager; use crate::models_manager::manager::ModelsManager; use crate::skills::SkillsManager; use crate::tools::sandboxing::ApprovalStore; -use crate::unified_exec::UnifiedExecSessionManager; +use crate::unified_exec::UnifiedExecProcessManager; use crate::user_notification::UserNotifier; use codex_otel::otel_manager::OtelManager; use tokio::sync::Mutex; @@ -17,7 +18,7 @@ use tokio_util::sync::CancellationToken; pub(crate) struct SessionServices { pub(crate) mcp_connection_manager: Arc>, pub(crate) mcp_startup_cancellation_token: CancellationToken, - pub(crate) unified_exec_manager: UnifiedExecSessionManager, + pub(crate) unified_exec_manager: UnifiedExecProcessManager, pub(crate) notifier: UserNotifier, pub(crate) rollout: Mutex>, pub(crate) user_shell: Arc, @@ -28,4 +29,5 @@ pub(crate) struct SessionServices { pub(crate) otel_manager: OtelManager, pub(crate) tool_approvals: Mutex, pub(crate) skills_manager: Arc, + pub(crate) agent_control: AgentControl, } diff --git a/codex-rs/core/src/tasks/mod.rs b/codex-rs/core/src/tasks/mod.rs index 4601f9d244..a817d51cd6 100644 --- a/codex-rs/core/src/tasks/mod.rs +++ b/codex-rs/core/src/tasks/mod.rs @@ -159,7 +159,7 @@ impl Session { for task in self.take_all_running_tasks().await { self.handle_task_abort(task, reason.clone()).await; } - self.close_unified_exec_sessions().await; + self.close_unified_exec_processes().await; } pub async fn on_task_finished( @@ -168,7 +168,7 @@ impl Session { last_agent_message: Option, ) { let mut active = self.active_turn.lock().await; - let should_close_sessions = if let Some(at) = active.as_mut() + let should_close_processes = if let Some(at) = active.as_mut() && at.remove_task(&turn_context.sub_id) { *active = None; @@ -177,8 +177,8 @@ impl Session { false }; drop(active); - if should_close_sessions { - self.close_unified_exec_sessions().await; + if should_close_processes { + self.close_unified_exec_processes().await; } let event = EventMsg::TaskComplete(TaskCompleteEvent { last_agent_message }); self.send_event(turn_context.as_ref(), event).await; @@ -203,10 +203,10 @@ impl Session { } } - async fn close_unified_exec_sessions(&self) { + async fn close_unified_exec_processes(&self) { self.services .unified_exec_manager - .terminate_all_sessions() + .terminate_all_processes() .await; } diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index bda8f985a7..8ae6b1a8de 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -13,8 +13,8 @@ use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; use crate::unified_exec::ExecCommandRequest; use crate::unified_exec::UnifiedExecContext; +use crate::unified_exec::UnifiedExecProcessManager; use crate::unified_exec::UnifiedExecResponse; -use crate::unified_exec::UnifiedExecSessionManager; use crate::unified_exec::WriteStdinRequest; use async_trait::async_trait; use serde::Deserialize; @@ -112,7 +112,7 @@ impl ToolHandler for UnifiedExecHandler { } }; - let manager: &UnifiedExecSessionManager = &session.services.unified_exec_manager; + let manager: &UnifiedExecProcessManager = &session.services.unified_exec_manager; let context = UnifiedExecContext::new(session.clone(), turn.clone(), call_id.clone()); let response = match tool_name.as_str() { diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index da1f518e8f..d6362eeb1b 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -2,7 +2,7 @@ Runtime: unified exec Handles approval + sandbox orchestration for unified exec requests, delegating to -the session manager to spawn PTYs once an ExecEnv is prepared. +the process manager to spawn PTYs once an ExecEnv is prepared. */ use crate::error::CodexErr; use crate::error::SandboxErr; @@ -25,8 +25,8 @@ use crate::tools::sandboxing::ToolError; use crate::tools::sandboxing::ToolRuntime; use crate::tools::sandboxing::with_cached_approval; use crate::unified_exec::UnifiedExecError; -use crate::unified_exec::UnifiedExecSession; -use crate::unified_exec::UnifiedExecSessionManager; +use crate::unified_exec::UnifiedExecProcess; +use crate::unified_exec::UnifiedExecProcessManager; use codex_protocol::protocol::ReviewDecision; use futures::future::BoxFuture; use std::collections::HashMap; @@ -50,7 +50,7 @@ pub struct UnifiedExecApprovalKey { } pub struct UnifiedExecRuntime<'a> { - manager: &'a UnifiedExecSessionManager, + manager: &'a UnifiedExecProcessManager, } impl UnifiedExecRequest { @@ -74,7 +74,7 @@ impl UnifiedExecRequest { } impl<'a> UnifiedExecRuntime<'a> { - pub fn new(manager: &'a UnifiedExecSessionManager) -> Self { + pub fn new(manager: &'a UnifiedExecProcessManager) -> Self { Self { manager } } } @@ -158,13 +158,13 @@ impl Approvable for UnifiedExecRuntime<'_> { } } -impl<'a> ToolRuntime for UnifiedExecRuntime<'a> { +impl<'a> ToolRuntime for UnifiedExecRuntime<'a> { async fn run( &mut self, req: &UnifiedExecRequest, attempt: &SandboxAttempt<'_>, ctx: &ToolCtx<'_>, - ) -> Result { + ) -> Result { let base_command = &req.command; let session_shell = ctx.session.user_shell(); let command = maybe_wrap_shell_lc_with_snapshot(base_command, session_shell.as_ref()); diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 0ac91755c2..75756831be 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -21,6 +21,7 @@ pub(crate) struct ToolsConfig { pub shell_type: ConfigShellToolType, pub apply_patch_tool_type: Option, pub web_search_request: bool, + pub web_search_cached: bool, pub include_view_image_tool: bool, pub experimental_supported_tools: Vec, } @@ -38,6 +39,7 @@ impl ToolsConfig { } = params; let include_apply_patch_tool = features.enabled(Feature::ApplyPatchFreeform); let include_web_search_request = features.enabled(Feature::WebSearchRequest); + let include_web_search_cached = features.enabled(Feature::WebSearchCached); let include_view_image_tool = features.enabled(Feature::ViewImageTool); let shell_type = if !features.enabled(Feature::ShellTool) { @@ -69,6 +71,7 @@ impl ToolsConfig { shell_type, apply_patch_tool_type, web_search_request: include_web_search_request, + web_search_cached: include_web_search_cached, include_view_image_tool, experimental_supported_tools: model_family.experimental_supported_tools.clone(), } @@ -1093,8 +1096,15 @@ pub(crate) fn build_specs( builder.register_handler("test_sync_tool", test_sync_handler); } - if config.web_search_request { - builder.push_spec(ToolSpec::WebSearch {}); + // Prefer web_search_cached flag over web_search_request + if config.web_search_cached { + builder.push_spec(ToolSpec::WebSearch { + external_web_access: Some(false), + }); + } else if config.web_search_request { + builder.push_spec(ToolSpec::WebSearch { + external_web_access: Some(true), + }); } if config.include_view_image_tool { @@ -1137,7 +1147,7 @@ mod tests { match tool { ToolSpec::Function(ResponsesApiTool { name, .. }) => name, ToolSpec::LocalShell {} => "local_shell", - ToolSpec::WebSearch {} => "web_search", + ToolSpec::WebSearch { .. } => "web_search", ToolSpec::Freeform(FreeformTool { name, .. }) => name, } } @@ -1215,7 +1225,7 @@ mod tests { ToolSpec::Function(ResponsesApiTool { parameters, .. }) => { strip_descriptions_schema(parameters); } - ToolSpec::Freeform(_) | ToolSpec::LocalShell {} | ToolSpec::WebSearch {} => {} + ToolSpec::Freeform(_) | ToolSpec::LocalShell {} | ToolSpec::WebSearch { .. } => {} } } @@ -1259,7 +1269,9 @@ mod tests { create_read_mcp_resource_tool(), PLAN_TOOL.clone(), create_apply_patch_freeform_tool(), - ToolSpec::WebSearch {}, + ToolSpec::WebSearch { + external_web_access: Some(true), + }, create_view_image_tool(), ] { expected.insert(tool_name(&spec).to_string(), spec); @@ -1292,6 +1304,51 @@ mod tests { assert_eq!(&tool_names, &expected_tools,); } + #[test] + fn web_search_cached_sets_external_web_access_false() { + let config = test_config(); + let model_family = ModelsManager::construct_model_family_offline("gpt-5-codex", &config); + let mut features = Features::with_defaults(); + features.enable(Feature::WebSearchCached); + + let tools_config = ToolsConfig::new(&ToolsConfigParams { + model_family: &model_family, + features: &features, + }); + let (tools, _) = build_specs(&tools_config, None).build(); + + let tool = find_tool(&tools, "web_search"); + assert_eq!( + tool.spec, + ToolSpec::WebSearch { + external_web_access: Some(false), + } + ); + } + + #[test] + fn web_search_cached_takes_precedence_over_web_search_request() { + let config = test_config(); + let model_family = ModelsManager::construct_model_family_offline("gpt-5-codex", &config); + let mut features = Features::with_defaults(); + features.enable(Feature::WebSearchCached); + features.enable(Feature::WebSearchRequest); + + let tools_config = ToolsConfig::new(&ToolsConfigParams { + model_family: &model_family, + features: &features, + }); + let (tools, _) = build_specs(&tools_config, None).build(); + + let tool = find_tool(&tools, "web_search"); + assert_eq!( + tool.spec, + ToolSpec::WebSearch { + external_web_access: Some(false), + } + ); + } + #[test] fn test_build_specs_gpt5_codex_default() { assert_model_tools( diff --git a/codex-rs/core/src/unified_exec/async_watcher.rs b/codex-rs/core/src/unified_exec/async_watcher.rs index 53b6b7c77d..1fbb1e8f6d 100644 --- a/codex-rs/core/src/unified_exec/async_watcher.rs +++ b/codex-rs/core/src/unified_exec/async_watcher.rs @@ -7,6 +7,8 @@ use tokio::time::Duration; use tokio::time::Instant; use tokio::time::Sleep; +use super::UnifiedExecContext; +use super::process::UnifiedExecProcess; use crate::codex::Session; use crate::codex::TurnContext; use crate::exec::ExecToolCallOutput; @@ -19,10 +21,7 @@ use crate::protocol::ExecOutputStream; use crate::tools::events::ToolEmitter; use crate::tools::events::ToolEventCtx; use crate::tools::events::ToolEventStage; - -use super::CommandTranscript; -use super::UnifiedExecContext; -use super::session::UnifiedExecSession; +use crate::unified_exec::head_tail_buffer::HeadTailBuffer; pub(crate) const TRAILING_OUTPUT_GRACE: Duration = Duration::from_millis(100); @@ -38,13 +37,13 @@ const UNIFIED_EXEC_OUTPUT_DELTA_MAX_BYTES: usize = 8192; /// shared transcript, and emits ExecCommandOutputDelta events on UTF‑8 /// boundaries. pub(crate) fn start_streaming_output( - session: &UnifiedExecSession, + process: &UnifiedExecProcess, context: &UnifiedExecContext, - transcript: Arc>, + transcript: Arc>, ) { - let mut receiver = session.output_receiver(); - let output_drained = session.output_drained_notify(); - let exit_token = session.cancellation_token(); + let mut receiver = process.output_receiver(); + let output_drained = process.output_drained_notify(); + let exit_token = process.cancellation_token(); let session_ref = Arc::clone(&context.session); let turn_ref = Arc::clone(&context.turn); @@ -105,24 +104,24 @@ pub(crate) fn start_streaming_output( /// single ExecCommandEnd event with the aggregated transcript. #[allow(clippy::too_many_arguments)] pub(crate) fn spawn_exit_watcher( - session: Arc, + process: Arc, session_ref: Arc, turn_ref: Arc, call_id: String, command: Vec, cwd: PathBuf, process_id: String, - transcript: Arc>, + transcript: Arc>, started_at: Instant, ) { - let exit_token = session.cancellation_token(); - let output_drained = session.output_drained_notify(); + let exit_token = process.cancellation_token(); + let output_drained = process.output_drained_notify(); tokio::spawn(async move { exit_token.cancelled().await; output_drained.notified().await; - let exit_code = session.exit_code().unwrap_or(-1); + let exit_code = process.exit_code().unwrap_or(-1); let duration = Instant::now().saturating_duration_since(started_at); emit_exec_end_for_unified_exec( session_ref, @@ -142,7 +141,7 @@ pub(crate) fn spawn_exit_watcher( async fn process_chunk( pending: &mut Vec, - transcript: &Arc>, + transcript: &Arc>, call_id: &str, session_ref: &Arc, turn_ref: &Arc, @@ -153,7 +152,7 @@ async fn process_chunk( while let Some(prefix) = split_valid_utf8_prefix(pending) { { let mut guard = transcript.lock().await; - guard.append(&prefix); + guard.push_chunk(prefix.to_vec()); } if *emitted_deltas >= MAX_EXEC_OUTPUT_DELTAS_PER_CALL { @@ -183,7 +182,7 @@ pub(crate) async fn emit_exec_end_for_unified_exec( command: Vec, cwd: PathBuf, process_id: Option, - transcript: Arc>, + transcript: Arc>, fallback_output: String, exit_code: i32, duration: Duration, @@ -240,15 +239,15 @@ fn split_valid_utf8_prefix_with_max(buffer: &mut Vec, max_bytes: usize) -> O } async fn resolve_aggregated_output( - transcript: &Arc>, + transcript: &Arc>, fallback: String, ) -> String { let guard = transcript.lock().await; - if guard.data.is_empty() { + if guard.retained_bytes() == 0 { return fallback; } - String::from_utf8_lossy(&guard.data).to_string() + String::from_utf8_lossy(&guard.to_bytes()).to_string() } #[cfg(test)] diff --git a/codex-rs/core/src/unified_exec/errors.rs b/codex-rs/core/src/unified_exec/errors.rs index 02031f22fa..d8df389252 100644 --- a/codex-rs/core/src/unified_exec/errors.rs +++ b/codex-rs/core/src/unified_exec/errors.rs @@ -3,11 +3,11 @@ use thiserror::Error; #[derive(Debug, Error)] pub(crate) enum UnifiedExecError { - #[error("Failed to create unified exec session: {message}")] - CreateSession { message: String }, - // Called "session" in the model's training. - #[error("Unknown session id {process_id}")] - UnknownSessionId { process_id: String }, + #[error("Failed to create unified exec process: {message}")] + CreateProcess { message: String }, + // The model is trained on `session_id`, but internally we track a `process_id`. + #[error("Unknown process id {process_id}")] + UnknownProcessId { process_id: String }, #[error("failed to write to stdin")] WriteToStdin, #[error("missing command line for unified exec request")] @@ -20,8 +20,8 @@ pub(crate) enum UnifiedExecError { } impl UnifiedExecError { - pub(crate) fn create_session(message: String) -> Self { - Self::CreateSession { message } + pub(crate) fn create_process(message: String) -> Self { + Self::CreateProcess { message } } pub(crate) fn sandbox_denied(message: String, output: ExecToolCallOutput) -> Self { diff --git a/codex-rs/core/src/unified_exec/head_tail_buffer.rs b/codex-rs/core/src/unified_exec/head_tail_buffer.rs new file mode 100644 index 0000000000..8524466048 --- /dev/null +++ b/codex-rs/core/src/unified_exec/head_tail_buffer.rs @@ -0,0 +1,272 @@ +use crate::unified_exec::UNIFIED_EXEC_OUTPUT_MAX_BYTES; +use std::collections::VecDeque; + +/// A capped buffer that preserves a stable prefix ("head") and suffix ("tail"), +/// dropping the middle once it exceeds the configured maximum. The buffer is +/// symmetric meaning 50% of the capacity is allocated to the head and 50% is +/// allocated to the tail. +#[derive(Debug)] +pub(crate) struct HeadTailBuffer { + max_bytes: usize, + head_budget: usize, + tail_budget: usize, + head: VecDeque>, + tail: VecDeque>, + head_bytes: usize, + tail_bytes: usize, + omitted_bytes: usize, +} + +impl Default for HeadTailBuffer { + fn default() -> Self { + Self::new(UNIFIED_EXEC_OUTPUT_MAX_BYTES) + } +} + +impl HeadTailBuffer { + /// Create a new buffer that retains at most `max_bytes` of output. + /// + /// The retained output is split across a prefix ("head") and suffix ("tail") + /// budget, dropping bytes from the middle once the limit is exceeded. + pub(crate) fn new(max_bytes: usize) -> Self { + let head_budget = max_bytes / 2; + let tail_budget = max_bytes.saturating_sub(head_budget); + Self { + max_bytes, + head_budget, + tail_budget, + head: VecDeque::new(), + tail: VecDeque::new(), + head_bytes: 0, + tail_bytes: 0, + omitted_bytes: 0, + } + } + + // Used for tests. + #[allow(dead_code)] + /// Total bytes currently retained by the buffer (head + tail). + pub(crate) fn retained_bytes(&self) -> usize { + self.head_bytes.saturating_add(self.tail_bytes) + } + + // Used for tests. + #[allow(dead_code)] + /// Total bytes that were dropped from the middle due to the size cap. + pub(crate) fn omitted_bytes(&self) -> usize { + self.omitted_bytes + } + + /// Append a chunk of bytes to the buffer. + /// + /// Bytes are first added to the head until the head budget is full; any + /// remaining bytes are added to the tail, with older tail bytes being + /// dropped to preserve the tail budget. + pub(crate) fn push_chunk(&mut self, chunk: Vec) { + if self.max_bytes == 0 { + self.omitted_bytes = self.omitted_bytes.saturating_add(chunk.len()); + return; + } + + // Fill the head budget first, then keep a capped tail. + if self.head_bytes < self.head_budget { + let remaining_head = self.head_budget.saturating_sub(self.head_bytes); + if chunk.len() <= remaining_head { + self.head_bytes = self.head_bytes.saturating_add(chunk.len()); + self.head.push_back(chunk); + return; + } + + // Split the chunk: part goes to head, remainder goes to tail. + let (head_part, tail_part) = chunk.split_at(remaining_head); + if !head_part.is_empty() { + self.head_bytes = self.head_bytes.saturating_add(head_part.len()); + self.head.push_back(head_part.to_vec()); + } + self.push_to_tail(tail_part.to_vec()); + return; + } + + self.push_to_tail(chunk); + } + + /// Snapshot the retained output as a list of chunks. + /// + /// The returned chunks are ordered as: head chunks first, then tail chunks. + /// Omitted bytes are not represented in the snapshot. + pub(crate) fn snapshot_chunks(&self) -> Vec> { + let mut out = Vec::new(); + out.extend(self.head.iter().cloned()); + out.extend(self.tail.iter().cloned()); + out + } + + /// Return the retained output as a single byte vector. + /// + /// The output is formed by concatenating head chunks, then tail chunks. + /// Omitted bytes are not represented in the returned value. + pub(crate) fn to_bytes(&self) -> Vec { + let mut out = Vec::with_capacity(self.retained_bytes()); + for chunk in self.head.iter() { + out.extend_from_slice(chunk); + } + for chunk in self.tail.iter() { + out.extend_from_slice(chunk); + } + out + } + + /// Drain all retained chunks from the buffer and reset its state. + /// + /// The drained chunks are returned in head-then-tail order. Omitted bytes + /// are discarded along with the retained content. + pub(crate) fn drain_chunks(&mut self) -> Vec> { + let mut out: Vec> = self.head.drain(..).collect(); + out.extend(self.tail.drain(..)); + self.head_bytes = 0; + self.tail_bytes = 0; + self.omitted_bytes = 0; + out + } + + fn push_to_tail(&mut self, chunk: Vec) { + if self.tail_budget == 0 { + self.omitted_bytes = self.omitted_bytes.saturating_add(chunk.len()); + return; + } + + if chunk.len() >= self.tail_budget { + // This single chunk is larger than the whole tail budget. Keep only the last + // tail_budget bytes and drop everything else. + let start = chunk.len().saturating_sub(self.tail_budget); + let kept = chunk[start..].to_vec(); + let dropped = chunk.len().saturating_sub(kept.len()); + self.omitted_bytes = self + .omitted_bytes + .saturating_add(self.tail_bytes) + .saturating_add(dropped); + self.tail.clear(); + self.tail_bytes = kept.len(); + self.tail.push_back(kept); + return; + } + + self.tail_bytes = self.tail_bytes.saturating_add(chunk.len()); + self.tail.push_back(chunk); + self.trim_tail_to_budget(); + } + + fn trim_tail_to_budget(&mut self) { + let mut excess = self.tail_bytes.saturating_sub(self.tail_budget); + while excess > 0 { + match self.tail.front_mut() { + Some(front) if excess >= front.len() => { + excess -= front.len(); + self.tail_bytes = self.tail_bytes.saturating_sub(front.len()); + self.omitted_bytes = self.omitted_bytes.saturating_add(front.len()); + self.tail.pop_front(); + } + Some(front) => { + front.drain(..excess); + self.tail_bytes = self.tail_bytes.saturating_sub(excess); + self.omitted_bytes = self.omitted_bytes.saturating_add(excess); + break; + } + None => break, + } + } + } +} + +#[cfg(test)] +mod tests { + use super::HeadTailBuffer; + + use pretty_assertions::assert_eq; + + #[test] + fn keeps_prefix_and_suffix_when_over_budget() { + let mut buf = HeadTailBuffer::new(10); + + buf.push_chunk(b"0123456789".to_vec()); + assert_eq!(buf.omitted_bytes(), 0); + + // Exceeds max by 2; we should keep head+tail and omit the middle. + buf.push_chunk(b"ab".to_vec()); + assert!(buf.omitted_bytes() > 0); + + let rendered = String::from_utf8_lossy(&buf.to_bytes()).to_string(); + assert!(rendered.starts_with("01234")); + assert!(rendered.ends_with("89ab")); + } + + #[test] + fn max_bytes_zero_drops_everything() { + let mut buf = HeadTailBuffer::new(0); + buf.push_chunk(b"abc".to_vec()); + + assert_eq!(buf.retained_bytes(), 0); + assert_eq!(buf.omitted_bytes(), 3); + assert_eq!(buf.to_bytes(), b"".to_vec()); + assert_eq!(buf.snapshot_chunks(), Vec::>::new()); + } + + #[test] + fn head_budget_zero_keeps_only_last_byte_in_tail() { + let mut buf = HeadTailBuffer::new(1); + buf.push_chunk(b"abc".to_vec()); + + assert_eq!(buf.retained_bytes(), 1); + assert_eq!(buf.omitted_bytes(), 2); + assert_eq!(buf.to_bytes(), b"c".to_vec()); + } + + #[test] + fn draining_resets_state() { + let mut buf = HeadTailBuffer::new(10); + buf.push_chunk(b"0123456789".to_vec()); + buf.push_chunk(b"ab".to_vec()); + + let drained = buf.drain_chunks(); + assert!(!drained.is_empty()); + + assert_eq!(buf.retained_bytes(), 0); + assert_eq!(buf.omitted_bytes(), 0); + assert_eq!(buf.to_bytes(), b"".to_vec()); + } + + #[test] + fn chunk_larger_than_tail_budget_keeps_only_tail_end() { + let mut buf = HeadTailBuffer::new(10); + buf.push_chunk(b"0123456789".to_vec()); + + // Tail budget is 5 bytes. This chunk should replace the tail and keep only its last 5 bytes. + buf.push_chunk(b"ABCDEFGHIJK".to_vec()); + + let out = String::from_utf8_lossy(&buf.to_bytes()).to_string(); + assert!(out.starts_with("01234")); + assert!(out.ends_with("GHIJK")); + assert!(buf.omitted_bytes() > 0); + } + + #[test] + fn fills_head_then_tail_across_multiple_chunks() { + let mut buf = HeadTailBuffer::new(10); + + // Fill the 5-byte head budget across multiple chunks. + buf.push_chunk(b"01".to_vec()); + buf.push_chunk(b"234".to_vec()); + assert_eq!(buf.to_bytes(), b"01234".to_vec()); + + // Then fill the 5-byte tail budget. + buf.push_chunk(b"567".to_vec()); + buf.push_chunk(b"89".to_vec()); + assert_eq!(buf.to_bytes(), b"0123456789".to_vec()); + assert_eq!(buf.omitted_bytes(), 0); + + // One more byte causes the tail to drop its oldest byte. + buf.push_chunk(b"a".to_vec()); + assert_eq!(buf.to_bytes(), b"012346789a".to_vec()); + assert_eq!(buf.omitted_bytes(), 1); + } +} diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index 2cb30e5aa3..ae10054079 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -1,7 +1,7 @@ -//! Unified Exec: interactive PTY execution orchestrated with approvals + sandboxing. +//! Unified Exec: interactive process execution orchestrated with approvals + sandboxing. //! //! Responsibilities -//! - Manages interactive PTY sessions (create, reuse, buffer output with caps). +//! - Manages interactive processes (create, reuse, buffer output with caps). //! - Uses the shared ToolOrchestrator to handle approval, sandbox selection, and //! retry semantics in a single, descriptive flow. //! - Spawns the PTY from a sandbox‑transformed `ExecEnv`; on sandbox denial, @@ -9,17 +9,17 @@ //! - Uses the shared `is_likely_sandbox_denied` heuristic to keep denial messages //! consistent with other exec paths. //! -//! Flow at a glance (open session) +//! Flow at a glance (open process) //! 1) Build a small request `{ command, cwd }`. //! 2) Orchestrator: approval (bypass/cache/prompt) → select sandbox → run. //! 3) Runtime: transform `CommandSpec` → `ExecEnv` → spawn PTY. //! 4) If denial, orchestrator retries with `SandboxType::None`. -//! 5) Session is returned with streaming output + metadata. +//! 5) Process handle is returned with streaming output + metadata. //! -//! This keeps policy logic and user interaction centralized while the PTY/session +//! This keeps policy logic and user interaction centralized while the PTY/process //! concerns remain isolated here. The implementation is split between: -//! - `session.rs`: PTY session lifecycle + output buffering. -//! - `session_manager.rs`: orchestration (approvals, sandboxing, reuse) and request handling. +//! - `process.rs`: PTY process lifecycle + output buffering. +//! - `process_manager.rs`: orchestration (approvals, sandboxing, reuse) and request handling. use std::collections::HashMap; use std::collections::HashSet; @@ -37,39 +37,22 @@ use crate::sandboxing::SandboxPermissions; mod async_watcher; mod errors; -mod session; -mod session_manager; +mod head_tail_buffer; +mod process; +mod process_manager; pub(crate) use errors::UnifiedExecError; -pub(crate) use session::UnifiedExecSession; +pub(crate) use process::UnifiedExecProcess; pub(crate) const MIN_YIELD_TIME_MS: u64 = 250; pub(crate) const MAX_YIELD_TIME_MS: u64 = 30_000; pub(crate) const DEFAULT_MAX_OUTPUT_TOKENS: usize = 10_000; pub(crate) const UNIFIED_EXEC_OUTPUT_MAX_BYTES: usize = 1024 * 1024; // 1 MiB pub(crate) const UNIFIED_EXEC_OUTPUT_MAX_TOKENS: usize = UNIFIED_EXEC_OUTPUT_MAX_BYTES / 4; -pub(crate) const MAX_UNIFIED_EXEC_SESSIONS: usize = 64; +pub(crate) const MAX_UNIFIED_EXEC_PROCESSES: usize = 64; -// Send a warning message to the models when it reaches this number of sessions. -pub(crate) const WARNING_UNIFIED_EXEC_SESSIONS: usize = 60; - -#[derive(Debug, Default)] -pub(crate) struct CommandTranscript { - pub data: Vec, -} - -impl CommandTranscript { - pub fn append(&mut self, bytes: &[u8]) { - self.data.extend_from_slice(bytes); - if self.data.len() > UNIFIED_EXEC_OUTPUT_MAX_BYTES { - let excess = self - .data - .len() - .saturating_sub(UNIFIED_EXEC_OUTPUT_MAX_BYTES); - self.data.drain(..excess); - } - } -} +// Send a warning message to the models when it reaches this number of processes. +pub(crate) const WARNING_UNIFIED_EXEC_PROCESSES: usize = 60; pub(crate) struct UnifiedExecContext { pub session: Arc, @@ -121,32 +104,32 @@ pub(crate) struct UnifiedExecResponse { } #[derive(Default)] -pub(crate) struct SessionStore { - sessions: HashMap, - reserved_sessions_id: HashSet, +pub(crate) struct ProcessStore { + processes: HashMap, + reserved_process_ids: HashSet, } -impl SessionStore { - fn remove(&mut self, session_id: &str) -> Option { - self.reserved_sessions_id.remove(session_id); - self.sessions.remove(session_id) +impl ProcessStore { + fn remove(&mut self, process_id: &str) -> Option { + self.reserved_process_ids.remove(process_id); + self.processes.remove(process_id) } } -pub(crate) struct UnifiedExecSessionManager { - session_store: Mutex, +pub(crate) struct UnifiedExecProcessManager { + process_store: Mutex, } -impl Default for UnifiedExecSessionManager { +impl Default for UnifiedExecProcessManager { fn default() -> Self { Self { - session_store: Mutex::new(SessionStore::default()), + process_store: Mutex::new(ProcessStore::default()), } } } -struct SessionEntry { - session: Arc, +struct ProcessEntry { + process: Arc, session_ref: Arc, turn_ref: Arc, call_id: String, @@ -173,6 +156,7 @@ pub(crate) fn generate_chunk_id() -> String { #[cfg(test)] #[cfg(unix)] mod tests { + use super::head_tail_buffer::HeadTailBuffer; use super::*; use crate::codex::Session; use crate::codex::TurnContext; @@ -185,8 +169,6 @@ mod tests { use std::sync::Arc; use tokio::time::Duration; - use super::session::OutputBufferState; - async fn test_session_and_turn() -> (Arc, Arc) { let (session, mut turn) = make_session_and_context().await; turn.approval_policy = AskForApproval::Never; @@ -245,21 +227,36 @@ mod tests { } #[test] - fn push_chunk_trims_only_excess_bytes() { - let mut buffer = OutputBufferState::default(); + fn push_chunk_preserves_prefix_and_suffix() { + let mut buffer = HeadTailBuffer::default(); buffer.push_chunk(vec![b'a'; UNIFIED_EXEC_OUTPUT_MAX_BYTES]); buffer.push_chunk(vec![b'b']); buffer.push_chunk(vec![b'c']); - assert_eq!(buffer.total_bytes, UNIFIED_EXEC_OUTPUT_MAX_BYTES); - let snapshot = buffer.snapshot(); - assert_eq!(snapshot.len(), 3); + assert_eq!(buffer.retained_bytes(), UNIFIED_EXEC_OUTPUT_MAX_BYTES); + let snapshot = buffer.snapshot_chunks(); + + let first = snapshot.first().expect("expected at least one chunk"); + assert_eq!(first.first(), Some(&b'a')); + assert!(snapshot.iter().any(|chunk| chunk.as_slice() == b"b")); assert_eq!( - snapshot.first().unwrap().len(), - UNIFIED_EXEC_OUTPUT_MAX_BYTES - 2 + snapshot + .last() + .expect("expected at least one chunk") + .as_slice(), + b"c" ); - assert_eq!(snapshot.get(2).unwrap(), &vec![b'c']); - assert_eq!(snapshot.get(1).unwrap(), &vec![b'b']); + } + + #[test] + fn head_tail_buffer_default_preserves_prefix_and_suffix() { + let mut buffer = HeadTailBuffer::default(); + buffer.push_chunk(vec![b'a'; UNIFIED_EXEC_OUTPUT_MAX_BYTES]); + buffer.push_chunk(b"bc".to_vec()); + + let rendered = buffer.to_bytes(); + assert_eq!(rendered.first(), Some(&b'a')); + assert!(rendered.ends_with(b"bc")); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -424,10 +421,10 @@ mod tests { session .services .unified_exec_manager - .session_store + .process_store .lock() .await - .sessions + .processes .is_empty() ); @@ -435,7 +432,7 @@ mod tests { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn reusing_completed_session_returns_unknown_session() -> anyhow::Result<()> { + async fn reusing_completed_process_returns_unknown_process() -> anyhow::Result<()> { skip_if_sandbox!(Ok(())); let (session, turn) = test_session_and_turn().await; @@ -453,23 +450,23 @@ mod tests { let err = write_stdin(&session, process_id, "", 100) .await - .expect_err("expected unknown session error"); + .expect_err("expected unknown process error"); match err { - UnifiedExecError::UnknownSessionId { process_id: err_id } => { + UnifiedExecError::UnknownProcessId { process_id: err_id } => { assert_eq!(err_id, process_id, "process id should match request"); } - other => panic!("expected UnknownSessionId, got {other:?}"), + other => panic!("expected UnknownProcessId, got {other:?}"), } assert!( session .services .unified_exec_manager - .session_store + .process_store .lock() .await - .sessions + .processes .is_empty() ); diff --git a/codex-rs/core/src/unified_exec/session.rs b/codex-rs/core/src/unified_exec/process.rs similarity index 75% rename from codex-rs/core/src/unified_exec/session.rs rename to codex-rs/core/src/unified_exec/process.rs index 4973a1a641..d52d1a6a9d 100644 --- a/codex-rs/core/src/unified_exec/session.rs +++ b/codex-rs/core/src/unified_exec/process.rs @@ -1,6 +1,5 @@ #![allow(clippy::module_inception)] -use std::collections::VecDeque; use std::sync::Arc; use tokio::sync::Mutex; use tokio::sync::Notify; @@ -19,54 +18,11 @@ use crate::truncate::formatted_truncate_text; use codex_utils_pty::ExecCommandSession; use codex_utils_pty::SpawnedPty; -use super::UNIFIED_EXEC_OUTPUT_MAX_BYTES; use super::UNIFIED_EXEC_OUTPUT_MAX_TOKENS; use super::UnifiedExecError; +use super::head_tail_buffer::HeadTailBuffer; -#[derive(Debug, Default)] -pub(crate) struct OutputBufferState { - chunks: VecDeque>, - pub(crate) total_bytes: usize, -} - -impl OutputBufferState { - pub(super) fn push_chunk(&mut self, chunk: Vec) { - self.total_bytes = self.total_bytes.saturating_add(chunk.len()); - self.chunks.push_back(chunk); - - let mut excess = self - .total_bytes - .saturating_sub(UNIFIED_EXEC_OUTPUT_MAX_BYTES); - - while excess > 0 { - match self.chunks.front_mut() { - Some(front) if excess >= front.len() => { - excess -= front.len(); - self.total_bytes = self.total_bytes.saturating_sub(front.len()); - self.chunks.pop_front(); - } - Some(front) => { - front.drain(..excess); - self.total_bytes = self.total_bytes.saturating_sub(excess); - break; - } - None => break, - } - } - } - - pub(super) fn drain(&mut self) -> Vec> { - let drained: Vec> = self.chunks.drain(..).collect(); - self.total_bytes = 0; - drained - } - - pub(super) fn snapshot(&self) -> Vec> { - self.chunks.iter().cloned().collect() - } -} - -pub(crate) type OutputBuffer = Arc>; +pub(crate) type OutputBuffer = Arc>; pub(crate) struct OutputHandles { pub(crate) output_buffer: OutputBuffer, pub(crate) output_notify: Arc, @@ -74,8 +30,8 @@ pub(crate) struct OutputHandles { } #[derive(Debug)] -pub(crate) struct UnifiedExecSession { - session: ExecCommandSession, +pub(crate) struct UnifiedExecProcess { + process_handle: ExecCommandSession, output_buffer: OutputBuffer, output_notify: Arc, cancellation_token: CancellationToken, @@ -84,13 +40,13 @@ pub(crate) struct UnifiedExecSession { sandbox_type: SandboxType, } -impl UnifiedExecSession { +impl UnifiedExecProcess { pub(super) fn new( - session: ExecCommandSession, + process_handle: ExecCommandSession, initial_output_rx: tokio::sync::broadcast::Receiver>, sandbox_type: SandboxType, ) -> Self { - let output_buffer = Arc::new(Mutex::new(OutputBufferState::default())); + let output_buffer = Arc::new(Mutex::new(HeadTailBuffer::default())); let output_notify = Arc::new(Notify::new()); let cancellation_token = CancellationToken::new(); let output_drained = Arc::new(Notify::new()); @@ -113,7 +69,7 @@ impl UnifiedExecSession { }); Self { - session, + process_handle, output_buffer, output_notify, cancellation_token, @@ -124,7 +80,7 @@ impl UnifiedExecSession { } pub(super) fn writer_sender(&self) -> mpsc::Sender> { - self.session.writer_sender() + self.process_handle.writer_sender() } pub(super) fn output_handles(&self) -> OutputHandles { @@ -136,7 +92,7 @@ impl UnifiedExecSession { } pub(super) fn output_receiver(&self) -> tokio::sync::broadcast::Receiver> { - self.session.output_receiver() + self.process_handle.output_receiver() } pub(super) fn cancellation_token(&self) -> CancellationToken { @@ -148,22 +104,22 @@ impl UnifiedExecSession { } pub(super) fn has_exited(&self) -> bool { - self.session.has_exited() + self.process_handle.has_exited() } pub(super) fn exit_code(&self) -> Option { - self.session.exit_code() + self.process_handle.exit_code() } pub(super) fn terminate(&self) { - self.session.terminate(); + self.process_handle.terminate(); self.cancellation_token.cancel(); self.output_task.abort(); } async fn snapshot_output(&self) -> Vec> { let guard = self.output_buffer.lock().await; - guard.snapshot() + guard.snapshot_chunks() } pub(crate) fn sandbox_type(&self) -> SandboxType { @@ -208,7 +164,7 @@ impl UnifiedExecSession { TruncationPolicy::Tokens(UNIFIED_EXEC_OUTPUT_MAX_TOKENS), ); let message = if snippet.is_empty() { - format!("Session exited with code {exit_code}") + format!("Process exited with code {exit_code}") } else { snippet }; @@ -222,11 +178,11 @@ impl UnifiedExecSession { sandbox_type: SandboxType, ) -> Result { let SpawnedPty { - session, + session: process_handle, output_rx, mut exit_rx, } = spawned; - let managed = Self::new(session, output_rx, sandbox_type); + let managed = Self::new(process_handle, output_rx, sandbox_type); let exit_ready = matches!(exit_rx.try_recv(), Ok(_) | Err(TryRecvError::Closed)); @@ -261,7 +217,7 @@ impl UnifiedExecSession { } } -impl Drop for UnifiedExecSession { +impl Drop for UnifiedExecProcess { fn drop(&mut self) { self.terminate(); } diff --git a/codex-rs/core/src/unified_exec/session_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs similarity index 79% rename from codex-rs/core/src/unified_exec/session_manager.rs rename to codex-rs/core/src/unified_exec/process_manager.rs index df473959d2..2e80156114 100644 --- a/codex-rs/core/src/unified_exec/session_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -29,27 +29,26 @@ use crate::tools::sandboxing::ToolCtx; use crate::truncate::TruncationPolicy; use crate::truncate::approx_token_count; use crate::truncate::formatted_truncate_text; - -use super::CommandTranscript; -use super::ExecCommandRequest; -use super::MAX_UNIFIED_EXEC_SESSIONS; -use super::SessionEntry; -use super::SessionStore; -use super::UnifiedExecContext; -use super::UnifiedExecError; -use super::UnifiedExecResponse; -use super::UnifiedExecSessionManager; -use super::WARNING_UNIFIED_EXEC_SESSIONS; -use super::WriteStdinRequest; -use super::async_watcher::emit_exec_end_for_unified_exec; -use super::async_watcher::spawn_exit_watcher; -use super::async_watcher::start_streaming_output; -use super::clamp_yield_time; -use super::generate_chunk_id; -use super::resolve_max_tokens; -use super::session::OutputBuffer; -use super::session::OutputHandles; -use super::session::UnifiedExecSession; +use crate::unified_exec::ExecCommandRequest; +use crate::unified_exec::MAX_UNIFIED_EXEC_PROCESSES; +use crate::unified_exec::ProcessEntry; +use crate::unified_exec::ProcessStore; +use crate::unified_exec::UnifiedExecContext; +use crate::unified_exec::UnifiedExecError; +use crate::unified_exec::UnifiedExecProcessManager; +use crate::unified_exec::UnifiedExecResponse; +use crate::unified_exec::WARNING_UNIFIED_EXEC_PROCESSES; +use crate::unified_exec::WriteStdinRequest; +use crate::unified_exec::async_watcher::emit_exec_end_for_unified_exec; +use crate::unified_exec::async_watcher::spawn_exit_watcher; +use crate::unified_exec::async_watcher::start_streaming_output; +use crate::unified_exec::clamp_yield_time; +use crate::unified_exec::generate_chunk_id; +use crate::unified_exec::head_tail_buffer::HeadTailBuffer; +use crate::unified_exec::process::OutputBuffer; +use crate::unified_exec::process::OutputHandles; +use crate::unified_exec::process::UnifiedExecProcess; +use crate::unified_exec::resolve_max_tokens; const UNIFIED_EXEC_ENV: [(&str, &str); 9] = [ ("NO_COLOR", "1"), @@ -70,7 +69,7 @@ fn apply_unified_exec_env(mut env: HashMap) -> HashMap>, output_buffer: OutputBuffer, output_notify: Arc, @@ -81,10 +80,10 @@ struct PreparedSessionHandles { process_id: String, } -impl UnifiedExecSessionManager { +impl UnifiedExecProcessManager { pub(crate) async fn allocate_process_id(&self) -> String { loop { - let mut store = self.session_store.lock().await; + let mut store = self.process_store.lock().await; let process_id = if !cfg!(test) && !cfg!(feature = "deterministic_process_ids") { // production mode → random @@ -92,7 +91,7 @@ impl UnifiedExecSessionManager { } else { // test or deterministic mode let next = store - .reserved_sessions_id + .reserved_process_ids .iter() .filter_map(|s| s.parse::().ok()) .max() @@ -102,17 +101,17 @@ impl UnifiedExecSessionManager { next.to_string() }; - if store.reserved_sessions_id.contains(&process_id) { + if store.reserved_process_ids.contains(&process_id) { continue; } - store.reserved_sessions_id.insert(process_id.clone()); + store.reserved_process_ids.insert(process_id.clone()); return process_id; } } pub(crate) async fn release_process_id(&self, process_id: &str) { - let mut store = self.session_store.lock().await; + let mut store = self.process_store.lock().await; store.remove(process_id); } @@ -126,7 +125,7 @@ impl UnifiedExecSessionManager { .clone() .unwrap_or_else(|| context.turn.cwd.clone()); - let session = self + let process = self .open_session_with_sandbox( &request.command, cwd.clone(), @@ -136,15 +135,15 @@ impl UnifiedExecSessionManager { ) .await; - let session = match session { - Ok(session) => Arc::new(session), + let process = match process { + Ok(process) => Arc::new(process), Err(err) => { self.release_process_id(&request.process_id).await; return Err(err); } }; - let transcript = Arc::new(tokio::sync::Mutex::new(CommandTranscript::default())); + let transcript = Arc::new(tokio::sync::Mutex::new(HeadTailBuffer::default())); let event_ctx = ToolEventCtx::new( context.session.as_ref(), context.turn.as_ref(), @@ -158,7 +157,8 @@ impl UnifiedExecSessionManager { Some(request.process_id.clone()), ); emitter.emit(event_ctx, ToolEventStage::Begin).await; - start_streaming_output(&session, context, Arc::clone(&transcript)); + + start_streaming_output(&process, context, Arc::clone(&transcript)); let max_tokens = resolve_max_tokens(request.max_output_tokens); let yield_time_ms = clamp_yield_time(request.yield_time_ms); @@ -171,7 +171,7 @@ impl UnifiedExecSessionManager { output_buffer, output_notify, cancellation_token, - } = session.output_handles(); + } = process.output_handles(); let deadline = start + Duration::from_millis(yield_time_ms); let collected = Self::collect_output_until_deadline( &output_buffer, @@ -184,8 +184,8 @@ impl UnifiedExecSessionManager { let text = String::from_utf8_lossy(&collected).to_string(); let output = formatted_truncate_text(&text, TruncationPolicy::Tokens(max_tokens)); - let exit_code = session.exit_code(); - let has_exited = session.has_exited() || exit_code.is_some(); + let exit_code = process.exit_code(); + let has_exited = process.has_exited() || exit_code.is_some(); let chunk_id = generate_chunk_id(); let process_id = request.process_id.clone(); if has_exited { @@ -208,14 +208,14 @@ impl UnifiedExecSessionManager { .await; self.release_process_id(&request.process_id).await; - session.check_for_sandbox_denial_with_text(&text).await?; + process.check_for_sandbox_denial_with_text(&text).await?; } else { - // Long‑lived command: persist the session so write_stdin can reuse + // Long‑lived command: persist the process so write_stdin can reuse // it, and register a background watcher that will emit // ExecCommandEnd when the PTY eventually exits (even if no further // tool calls are made). - self.store_session( - Arc::clone(&session), + self.store_process( + Arc::clone(&process), context, &request.command, cwd.clone(), @@ -254,7 +254,7 @@ impl UnifiedExecSessionManager { ) -> Result { let process_id = request.process_id.to_string(); - let PreparedSessionHandles { + let PreparedProcessHandles { writer_tx, output_buffer, output_notify, @@ -264,7 +264,7 @@ impl UnifiedExecSessionManager { command: session_command, process_id, .. - } = self.prepare_session_handles(process_id.as_str()).await?; + } = self.prepare_process_handles(process_id.as_str()).await?; if !request.input.is_empty() { Self::send_input(&writer_tx, request.input.as_bytes()).await?; @@ -291,23 +291,23 @@ impl UnifiedExecSessionManager { let original_token_count = approx_token_count(&text); let chunk_id = generate_chunk_id(); - // After polling, refresh_session_state tells us whether the PTY is + // After polling, refresh_process_state tells us whether the PTY is // still alive or has exited and been removed from the store; we thread // that through so the handler can tag TerminalInteraction with an // appropriate process_id and exit_code. - let status = self.refresh_session_state(process_id.as_str()).await; + let status = self.refresh_process_state(process_id.as_str()).await; let (process_id, exit_code, event_call_id) = match status { - SessionStatus::Alive { + ProcessStatus::Alive { exit_code, call_id, process_id, } => (Some(process_id), exit_code, call_id), - SessionStatus::Exited { exit_code, entry } => { + ProcessStatus::Exited { exit_code, entry } => { let call_id = entry.call_id.clone(); (None, exit_code, call_id) } - SessionStatus::Unknown => { - return Err(UnifiedExecError::UnknownSessionId { + ProcessStatus::Unknown => { + return Err(UnifiedExecError::UnknownProcessId { process_id: request.process_id.to_string(), }); } @@ -332,25 +332,25 @@ impl UnifiedExecSessionManager { Ok(response) } - async fn refresh_session_state(&self, process_id: &str) -> SessionStatus { - let mut store = self.session_store.lock().await; - let Some(entry) = store.sessions.get(process_id) else { - return SessionStatus::Unknown; + async fn refresh_process_state(&self, process_id: &str) -> ProcessStatus { + let mut store = self.process_store.lock().await; + let Some(entry) = store.processes.get(process_id) else { + return ProcessStatus::Unknown; }; - let exit_code = entry.session.exit_code(); + let exit_code = entry.process.exit_code(); let process_id = entry.process_id.clone(); - if entry.session.has_exited() { + if entry.process.has_exited() { let Some(entry) = store.remove(&process_id) else { - return SessionStatus::Unknown; + return ProcessStatus::Unknown; }; - SessionStatus::Exited { + ProcessStatus::Exited { exit_code, entry: Box::new(entry), } } else { - SessionStatus::Alive { + ProcessStatus::Alive { exit_code, call_id: entry.call_id.clone(), process_id, @@ -358,16 +358,16 @@ impl UnifiedExecSessionManager { } } - async fn prepare_session_handles( + async fn prepare_process_handles( &self, process_id: &str, - ) -> Result { - let mut store = self.session_store.lock().await; + ) -> Result { + let mut store = self.process_store.lock().await; let entry = store - .sessions + .processes .get_mut(process_id) - .ok_or(UnifiedExecError::UnknownSessionId { + .ok_or(UnifiedExecError::UnknownProcessId { process_id: process_id.to_string(), })?; entry.last_used = Instant::now(); @@ -375,10 +375,10 @@ impl UnifiedExecSessionManager { output_buffer, output_notify, cancellation_token, - } = entry.session.output_handles(); + } = entry.process.output_handles(); - Ok(PreparedSessionHandles { - writer_tx: entry.session.writer_sender(), + Ok(PreparedProcessHandles { + writer_tx: entry.process.writer_sender(), output_buffer, output_notify, cancellation_token, @@ -400,18 +400,18 @@ impl UnifiedExecSessionManager { } #[allow(clippy::too_many_arguments)] - async fn store_session( + async fn store_process( &self, - session: Arc, + process: Arc, context: &UnifiedExecContext, command: &[String], cwd: PathBuf, started_at: Instant, process_id: String, - transcript: Arc>, + transcript: Arc>, ) { - let entry = SessionEntry { - session: Arc::clone(&session), + let entry = ProcessEntry { + process: Arc::clone(&process), session_ref: Arc::clone(&context.session), turn_ref: Arc::clone(&context.turn), call_id: context.call_id.clone(), @@ -419,25 +419,25 @@ impl UnifiedExecSessionManager { command: command.to_vec(), last_used: started_at, }; - let number_sessions = { - let mut store = self.session_store.lock().await; - Self::prune_sessions_if_needed(&mut store); - store.sessions.insert(process_id.clone(), entry); - store.sessions.len() + let number_processes = { + let mut store = self.process_store.lock().await; + Self::prune_processes_if_needed(&mut store); + store.processes.insert(process_id.clone(), entry); + store.processes.len() }; - if number_sessions >= WARNING_UNIFIED_EXEC_SESSIONS { + if number_processes >= WARNING_UNIFIED_EXEC_PROCESSES { context .session .record_model_warning( - format!("The maximum number of unified exec sessions you can keep open is {WARNING_UNIFIED_EXEC_SESSIONS} and you currently have {number_sessions} sessions open. Reuse older sessions or close them to prevent automatic pruning of old session"), + format!("The maximum number of unified exec processes you can keep open is {WARNING_UNIFIED_EXEC_PROCESSES} and you currently have {number_processes} processes open. Reuse older processes or close them to prevent automatic pruning of old processes"), &context.turn ) .await; }; spawn_exit_watcher( - Arc::clone(&session), + Arc::clone(&process), Arc::clone(&context.session), Arc::clone(&context.turn), context.call_id.clone(), @@ -471,7 +471,7 @@ impl UnifiedExecSessionManager { pub(crate) async fn open_session_with_exec_env( &self, env: &ExecEnv, - ) -> Result { + ) -> Result { let (program, args) = env .command .split_first() @@ -485,8 +485,8 @@ impl UnifiedExecSessionManager { &env.arg0, ) .await - .map_err(|err| UnifiedExecError::create_session(err.to_string()))?; - UnifiedExecSession::from_spawned(spawned, env.sandbox).await + .map_err(|err| UnifiedExecError::create_process(err.to_string()))?; + UnifiedExecProcess::from_spawned(spawned, env.sandbox).await } pub(super) async fn open_session_with_sandbox( @@ -496,7 +496,7 @@ impl UnifiedExecSessionManager { sandbox_permissions: SandboxPermissions, justification: Option, context: &UnifiedExecContext, - ) -> Result { + ) -> Result { let env = apply_unified_exec_env(create_env(&context.turn.shell_environment_policy)); let features = context.session.features(); let mut orchestrator = ToolOrchestrator::new(); @@ -536,7 +536,7 @@ impl UnifiedExecSessionManager { context.turn.approval_policy, ) .await - .map_err(|e| UnifiedExecError::create_session(format!("{e:?}"))) + .map_err(|e| UnifiedExecError::create_process(format!("{e:?}"))) } pub(super) async fn collect_output_until_deadline( @@ -550,11 +550,11 @@ impl UnifiedExecSessionManager { let mut collected: Vec = Vec::with_capacity(4096); let mut exit_signal_received = cancellation_token.is_cancelled(); loop { - let drained_chunks; + let drained_chunks: Vec>; let mut wait_for_output = None; { let mut guard = output_buffer.lock().await; - drained_chunks = guard.drain(); + drained_chunks = guard.drain_chunks(); if drained_chunks.is_empty() { wait_for_output = Some(output_notify.notified()); } @@ -600,20 +600,20 @@ impl UnifiedExecSessionManager { collected } - fn prune_sessions_if_needed(store: &mut SessionStore) -> bool { - if store.sessions.len() < MAX_UNIFIED_EXEC_SESSIONS { + fn prune_processes_if_needed(store: &mut ProcessStore) -> bool { + if store.processes.len() < MAX_UNIFIED_EXEC_PROCESSES { return false; } let meta: Vec<(String, Instant, bool)> = store - .sessions + .processes .iter() - .map(|(id, entry)| (id.clone(), entry.last_used, entry.session.has_exited())) + .map(|(id, entry)| (id.clone(), entry.last_used, entry.process.has_exited())) .collect(); - if let Some(session_id) = Self::session_id_to_prune_from_meta(&meta) { - if let Some(entry) = store.remove(&session_id) { - entry.session.terminate(); + if let Some(process_id) = Self::process_id_to_prune_from_meta(&meta) { + if let Some(entry) = store.remove(&process_id) { + entry.process.terminate(); } return true; } @@ -622,7 +622,7 @@ impl UnifiedExecSessionManager { } // Centralized pruning policy so we can easily swap strategies later. - fn session_id_to_prune_from_meta(meta: &[(String, Instant, bool)]) -> Option { + fn process_id_to_prune_from_meta(meta: &[(String, Instant, bool)]) -> Option { if meta.is_empty() { return None; } @@ -650,22 +650,25 @@ impl UnifiedExecSessionManager { .map(|(process_id, _, _)| process_id) } - pub(crate) async fn terminate_all_sessions(&self) { - let entries: Vec = { - let mut sessions = self.session_store.lock().await; - let entries: Vec = - sessions.sessions.drain().map(|(_, entry)| entry).collect(); - sessions.reserved_sessions_id.clear(); + pub(crate) async fn terminate_all_processes(&self) { + let entries: Vec = { + let mut processes = self.process_store.lock().await; + let entries: Vec = processes + .processes + .drain() + .map(|(_, entry)| entry) + .collect(); + processes.reserved_process_ids.clear(); entries }; for entry in entries { - entry.session.terminate(); + entry.process.terminate(); } } } -enum SessionStatus { +enum ProcessStatus { Alive { exit_code: Option, call_id: String, @@ -673,7 +676,7 @@ enum SessionStatus { }, Exited { exit_code: Option, - entry: Box, + entry: Box, }, Unknown, } @@ -716,7 +719,7 @@ mod tests { } #[test] - fn pruning_prefers_exited_sessions_outside_recently_used() { + fn pruning_prefers_exited_processes_outside_recently_used() { let now = Instant::now(); let id = |n: i32| n.to_string(); let meta = vec![ @@ -732,7 +735,7 @@ mod tests { (id(10), now - Duration::from_secs(13), false), ]; - let candidate = UnifiedExecSessionManager::session_id_to_prune_from_meta(&meta); + let candidate = UnifiedExecProcessManager::process_id_to_prune_from_meta(&meta); assert_eq!(candidate, Some(id(2))); } @@ -754,13 +757,13 @@ mod tests { (id(10), now - Duration::from_secs(13), false), ]; - let candidate = UnifiedExecSessionManager::session_id_to_prune_from_meta(&meta); + let candidate = UnifiedExecProcessManager::process_id_to_prune_from_meta(&meta); assert_eq!(candidate, Some(id(1))); } #[test] - fn pruning_protects_recent_sessions_even_if_exited() { + fn pruning_protects_recent_processes_even_if_exited() { let now = Instant::now(); let id = |n: i32| n.to_string(); let meta = vec![ @@ -776,7 +779,7 @@ mod tests { (id(10), now - Duration::from_secs(13), true), ]; - let candidate = UnifiedExecSessionManager::session_id_to_prune_from_meta(&meta); + let candidate = UnifiedExecProcessManager::process_id_to_prune_from_meta(&meta); // (10) is exited but among the last 8; we should drop the LRU outside that set. assert_eq!(candidate, Some(id(1))); diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index a081a6bf12..7ae3ea159f 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -143,6 +143,7 @@ impl TestCodexBuilder { config.model_provider.clone(), config.codex_home.clone(), ); + let conversation_manager = Arc::new(conversation_manager); let new_conversation = match resume_from { Some(path) => { @@ -164,7 +165,7 @@ impl TestCodexBuilder { config, codex: new_conversation.conversation, session_configured: new_conversation.session_configured, - conversation_manager: Arc::new(conversation_manager), + conversation_manager, }) } diff --git a/codex-rs/core/tests/suite/list_models.rs b/codex-rs/core/tests/suite/list_models.rs index 565b978faa..30593a8f60 100644 --- a/codex-rs/core/tests/suite/list_models.rs +++ b/codex-rs/core/tests/suite/list_models.rs @@ -42,7 +42,18 @@ async fn list_models_returns_chatgpt_models() -> Result<()> { } fn expected_models_for_api_key() -> Vec { - vec![gpt_5_1_codex_max(), gpt_5_1_codex_mini(), gpt_5_2()] + vec![ + gpt_5_1_codex_max(), + gpt_5_1_codex_mini(), + gpt_5_2(), + bengalfox(), + boomslang(), + gpt_5_codex(), + gpt_5_codex_mini(), + gpt_5_1_codex(), + gpt_5(), + gpt_5_1(), + ] } fn expected_models_for_chatgpt() -> Vec { @@ -53,6 +64,13 @@ fn expected_models_for_chatgpt() -> Vec { gpt_5_1_codex_max, gpt_5_1_codex_mini(), gpt_5_2(), + bengalfox(), + boomslang(), + gpt_5_codex(), + gpt_5_codex_mini(), + gpt_5_1_codex(), + gpt_5(), + gpt_5_1(), ] } @@ -168,7 +186,7 @@ fn gpt_5_2() -> ModelPreset { ), effort( ReasoningEffort::XHigh, - "Extra high reasoning for complex problems", + "Extra high reasoning depth for complex problems", ), ], is_default: false, @@ -178,6 +196,210 @@ fn gpt_5_2() -> ModelPreset { } } +fn bengalfox() -> ModelPreset { + ModelPreset { + id: "bengalfox".to_string(), + model: "bengalfox".to_string(), + display_name: "bengalfox".to_string(), + description: "bengalfox".to_string(), + default_reasoning_effort: ReasoningEffort::Medium, + supported_reasoning_efforts: vec![ + effort( + ReasoningEffort::Low, + "Fast responses with lighter reasoning", + ), + effort( + ReasoningEffort::Medium, + "Balances speed and reasoning depth for everyday tasks", + ), + effort( + ReasoningEffort::High, + "Greater reasoning depth for complex problems", + ), + effort( + ReasoningEffort::XHigh, + "Extra high reasoning depth for complex problems", + ), + ], + is_default: false, + upgrade: None, + show_in_picker: false, + supported_in_api: true, + } +} + +fn boomslang() -> ModelPreset { + ModelPreset { + id: "boomslang".to_string(), + model: "boomslang".to_string(), + display_name: "boomslang".to_string(), + description: "boomslang".to_string(), + default_reasoning_effort: ReasoningEffort::Medium, + supported_reasoning_efforts: vec![ + effort( + ReasoningEffort::Low, + "Balances speed with some reasoning; useful for straightforward queries and short explanations", + ), + effort( + ReasoningEffort::Medium, + "Provides a solid balance of reasoning depth and latency for general-purpose tasks", + ), + effort( + ReasoningEffort::High, + "Maximizes reasoning depth for complex or ambiguous problems", + ), + effort( + ReasoningEffort::XHigh, + "Extra high reasoning depth for complex problems", + ), + ], + is_default: false, + upgrade: None, + show_in_picker: false, + supported_in_api: true, + } +} + +fn gpt_5_codex() -> ModelPreset { + ModelPreset { + id: "gpt-5-codex".to_string(), + model: "gpt-5-codex".to_string(), + display_name: "gpt-5-codex".to_string(), + description: "Optimized for codex.".to_string(), + default_reasoning_effort: ReasoningEffort::Medium, + supported_reasoning_efforts: vec![ + effort( + ReasoningEffort::Low, + "Fastest responses with limited reasoning", + ), + effort( + ReasoningEffort::Medium, + "Dynamically adjusts reasoning based on the task", + ), + effort( + ReasoningEffort::High, + "Maximizes reasoning depth for complex or ambiguous problems", + ), + ], + is_default: false, + upgrade: Some(gpt52_codex_upgrade()), + show_in_picker: false, + supported_in_api: true, + } +} + +fn gpt_5_codex_mini() -> ModelPreset { + ModelPreset { + id: "gpt-5-codex-mini".to_string(), + model: "gpt-5-codex-mini".to_string(), + display_name: "gpt-5-codex-mini".to_string(), + description: "Optimized for codex. Cheaper, faster, but less capable.".to_string(), + default_reasoning_effort: ReasoningEffort::Medium, + supported_reasoning_efforts: vec![ + effort( + ReasoningEffort::Medium, + "Dynamically adjusts reasoning based on the task", + ), + effort( + ReasoningEffort::High, + "Maximizes reasoning depth for complex or ambiguous problems", + ), + ], + is_default: false, + upgrade: Some(gpt52_codex_upgrade()), + show_in_picker: false, + supported_in_api: true, + } +} + +fn gpt_5_1_codex() -> ModelPreset { + ModelPreset { + id: "gpt-5.1-codex".to_string(), + model: "gpt-5.1-codex".to_string(), + display_name: "gpt-5.1-codex".to_string(), + description: "Optimized for codex.".to_string(), + default_reasoning_effort: ReasoningEffort::Medium, + supported_reasoning_efforts: vec![ + effort( + ReasoningEffort::Low, + "Fastest responses with limited reasoning", + ), + effort( + ReasoningEffort::Medium, + "Dynamically adjusts reasoning based on the task", + ), + effort( + ReasoningEffort::High, + "Maximizes reasoning depth for complex or ambiguous problems", + ), + ], + is_default: false, + upgrade: Some(gpt52_codex_upgrade()), + show_in_picker: false, + supported_in_api: true, + } +} + +fn gpt_5() -> ModelPreset { + ModelPreset { + id: "gpt-5".to_string(), + model: "gpt-5".to_string(), + display_name: "gpt-5".to_string(), + description: "Broad world knowledge with strong general reasoning.".to_string(), + default_reasoning_effort: ReasoningEffort::Medium, + supported_reasoning_efforts: vec![ + effort( + ReasoningEffort::Minimal, + "Fastest responses with little reasoning", + ), + effort( + ReasoningEffort::Low, + "Balances speed with some reasoning; useful for straightforward queries and short explanations", + ), + effort( + ReasoningEffort::Medium, + "Provides a solid balance of reasoning depth and latency for general-purpose tasks", + ), + effort( + ReasoningEffort::High, + "Maximizes reasoning depth for complex or ambiguous problems", + ), + ], + is_default: false, + upgrade: Some(gpt52_codex_upgrade()), + show_in_picker: false, + supported_in_api: true, + } +} + +fn gpt_5_1() -> ModelPreset { + ModelPreset { + id: "gpt-5.1".to_string(), + model: "gpt-5.1".to_string(), + display_name: "gpt-5.1".to_string(), + description: "Broad world knowledge with strong general reasoning.".to_string(), + default_reasoning_effort: ReasoningEffort::Medium, + supported_reasoning_efforts: vec![ + effort( + ReasoningEffort::Low, + "Balances speed with some reasoning; useful for straightforward queries and short explanations", + ), + effort( + ReasoningEffort::Medium, + "Provides a solid balance of reasoning depth and latency for general-purpose tasks", + ), + effort( + ReasoningEffort::High, + "Maximizes reasoning depth for complex or ambiguous problems", + ), + ], + is_default: false, + upgrade: Some(gpt52_codex_upgrade()), + show_in_picker: false, + supported_in_api: true, + } +} + fn gpt52_codex_upgrade() -> codex_protocol::openai_models::ModelUpgrade { codex_protocol::openai_models::ModelUpgrade { id: "gpt-5.2-codex".to_string(), diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 63784bd403..e5b809ca38 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -65,3 +65,4 @@ mod unified_exec; mod user_notification; mod user_shell_cmd; mod view_image; +mod web_search_cached; diff --git a/codex-rs/core/tests/suite/remote_models.rs b/codex-rs/core/tests/suite/remote_models.rs index 201d61c9e0..94641f1a4c 100644 --- a/codex-rs/core/tests/suite/remote_models.rs +++ b/codex-rs/core/tests/suite/remote_models.rs @@ -381,12 +381,11 @@ async fn remote_models_hide_picker_only_models() -> Result<()> { assert_eq!(selected, "gpt-5.2-codex"); let available = manager.list_models(&config).await; - assert!( - available - .iter() - .all(|model| model.model != "codex-auto-balanced"), - "hidden models should not appear in the picker list" - ); + let hidden = available + .iter() + .find(|model| model.model == "codex-auto-balanced") + .expect("hidden remote model should be listed"); + assert!(!hidden.show_in_picker, "hidden models should remain hidden"); Ok(()) } @@ -442,7 +441,8 @@ where mutate_config(&mut config); - let conversation_manager = Arc::new(ConversationManager::with_models_provider(auth, provider)); + let conversation_manager = ConversationManager::with_models_provider(auth, provider); + let conversation_manager = Arc::new(conversation_manager); let new_conversation = conversation_manager .new_conversation(config.clone()) diff --git a/codex-rs/core/tests/suite/tool_parallelism.rs b/codex-rs/core/tests/suite/tool_parallelism.rs index 5a5b443209..71a6031fe8 100644 --- a/codex-rs/core/tests/suite/tool_parallelism.rs +++ b/codex-rs/core/tests/suite/tool_parallelism.rs @@ -301,8 +301,11 @@ async fn shell_tools_start_before_response_completed_when_stream_delayed() -> an "perl -MTime::HiRes -e 'print int(Time::HiRes::time()*1000), \"\\n\"' >> \"{}\"", output_path.display() ); + // Use a non-login shell to avoid slow, user-specific shell init (e.g. zsh profiles) + // from making this timing-based test flaky. let args = json!({ "command": command, + "login": false, "timeout_ms": 1_000, }); diff --git a/codex-rs/core/tests/suite/unified_exec.rs b/codex-rs/core/tests/suite/unified_exec.rs index 2ca62a602f..66319e21c2 100644 --- a/codex-rs/core/tests/suite/unified_exec.rs +++ b/codex-rs/core/tests/suite/unified_exec.rs @@ -1689,7 +1689,7 @@ async fn unified_exec_closes_long_running_session_at_turn_end() -> Result<()> { codex .submit(Op::UserTurn { items: vec![UserInput::Text { - text: "close unified exec sessions on turn end".into(), + text: "close unified exec processes on turn end".into(), }], final_output_json_schema: None, cwd: cwd.path().to_path_buf(), @@ -1710,7 +1710,7 @@ async fn unified_exec_closes_long_running_session_at_turn_end() -> Result<()> { let begin_process_id = begin_event .process_id .clone() - .expect("expected process_id for long-running unified exec session"); + .expect("expected process_id for long-running unified exec process"); let pid = wait_for_pid_file(&pid_path).await?; assert!( @@ -2560,7 +2560,7 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> { let prune_start = requests .iter() .find_map(|req| req.function_call_output_text(prune_call_id)) - .expect("missing initial prune session output"); + .expect("missing initial prune process output"); let prune_start_output = parse_unified_exec_output(&prune_start)?; assert!(prune_start_output.process_id.is_some()); assert!(prune_start_output.exit_code.is_none()); @@ -2573,7 +2573,7 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> { assert!(keep_write_output.process_id.is_some()); assert!( keep_write_output.output.contains("still alive"), - "expected cat session to echo input, got {:?}", + "expected cat process to echo input, got {:?}", keep_write_output.output ); @@ -2582,7 +2582,7 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> { .find_map(|req| req.function_call_output_text(probe_call_id)) .expect("missing probe output"); assert!( - pruned_probe.contains("UnknownSessionId") || pruned_probe.contains("Unknown process id"), + pruned_probe.contains("UnknownProcessId") || pruned_probe.contains("Unknown process id"), "expected probe to fail after pruning, got {pruned_probe:?}" ); diff --git a/codex-rs/core/tests/suite/web_search_cached.rs b/codex-rs/core/tests/suite/web_search_cached.rs new file mode 100644 index 0000000000..fa8e303d80 --- /dev/null +++ b/codex-rs/core/tests/suite/web_search_cached.rs @@ -0,0 +1,87 @@ +#![allow(clippy::unwrap_used)] + +use codex_core::features::Feature; +use core_test_support::load_sse_fixture_with_id; +use core_test_support::responses; +use core_test_support::responses::start_mock_server; +use core_test_support::skip_if_no_network; +use core_test_support::test_codex::test_codex; +use pretty_assertions::assert_eq; +use serde_json::Value; + +fn sse_completed(id: &str) -> String { + load_sse_fixture_with_id("tests/fixtures/completed_template.json", id) +} + +#[allow(clippy::expect_used)] +fn find_web_search_tool(body: &Value) -> &Value { + body["tools"] + .as_array() + .expect("request body should include tools array") + .iter() + .find(|tool| tool.get("type").and_then(Value::as_str) == Some("web_search")) + .expect("tools should include a web_search tool") +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn web_search_cached_sets_external_web_access_false_in_request_body() { + skip_if_no_network!(); + + let server = start_mock_server().await; + let sse = sse_completed("resp-1"); + let resp_mock = responses::mount_sse_once(&server, sse).await; + + let mut builder = test_codex() + .with_model("gpt-5-codex") + .with_config(|config| { + config.features.enable(Feature::WebSearchCached); + }); + let test = builder + .build(&server) + .await + .expect("create test Codex conversation"); + + test.submit_turn("hello cached web search") + .await + .expect("submit turn"); + + let body = resp_mock.single_request().body_json(); + let tool = find_web_search_tool(&body); + assert_eq!( + tool.get("external_web_access").and_then(Value::as_bool), + Some(false), + "web_search_cached should force external_web_access=false" + ); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn web_search_cached_takes_precedence_over_web_search_request_in_request_body() { + skip_if_no_network!(); + + let server = start_mock_server().await; + let sse = sse_completed("resp-1"); + let resp_mock = responses::mount_sse_once(&server, sse).await; + + let mut builder = test_codex() + .with_model("gpt-5-codex") + .with_config(|config| { + config.features.enable(Feature::WebSearchRequest); + config.features.enable(Feature::WebSearchCached); + }); + let test = builder + .build(&server) + .await + .expect("create test Codex conversation"); + + test.submit_turn("hello cached+live flags") + .await + .expect("submit turn"); + + let body = resp_mock.single_request().body_json(); + let tool = find_web_search_tool(&body); + assert_eq!( + tool.get("external_web_access").and_then(Value::as_bool), + Some(false), + "web_search_cached should win over web_search_request" + ); +} diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index 40afab7c9c..ba6d99c5a8 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -595,7 +595,8 @@ impl EventProcessor for EventProcessorWithHumanOutput { | EventMsg::ReasoningRawContentDelta(_) | EventMsg::SkillsUpdateAvailable | EventMsg::UndoCompleted(_) - | EventMsg::UndoStarted(_) => {} + | EventMsg::UndoStarted(_) + | EventMsg::ThreadRolledBack(_) => {} } CodexStatus::Running } diff --git a/codex-rs/mcp-server/src/codex_tool_config.rs b/codex-rs/mcp-server/src/codex_tool_config.rs index 16d38da45e..26fdc07320 100644 --- a/codex-rs/mcp-server/src/codex_tool_config.rs +++ b/codex-rs/mcp-server/src/codex_tool_config.rs @@ -21,7 +21,7 @@ pub struct CodexToolCallParam { /// The *initial user prompt* to start the Codex conversation. pub prompt: String, - /// Optional override for the model name (e.g. "o3", "o4-mini"). + /// Optional override for the model name (e.g. 'gpt-5.2', 'gpt-5.2-codex'). #[serde(default, skip_serializing_if = "Option::is_none")] pub model: Option, @@ -245,11 +245,8 @@ mod tests { let tool = create_tool_for_codex_tool_call_param(); let tool_json = serde_json::to_value(&tool).expect("tool serializes"); let expected_tool_json = serde_json::json!({ - "name": "codex", - "title": "Codex", "description": "Run a Codex session. Accepts configuration parameters matching the Codex Config struct.", "inputSchema": { - "type": "object", "properties": { "approval-policy": { "description": "Approval policy for shell commands generated by the model: `untrusted`, `on-failure`, `on-request`, `never`.", @@ -261,26 +258,29 @@ mod tests { ], "type": "string" }, - "sandbox": { - "description": "Sandbox mode: `read-only`, `workspace-write`, or `danger-full-access`.", - "enum": [ - "read-only", - "workspace-write", - "danger-full-access" - ], + "base-instructions": { + "description": "The set of instructions to use instead of the default ones.", + "type": "string" + }, + "compact-prompt": { + "description": "Prompt used when compacting the conversation.", "type": "string" }, "config": { - "description": "Individual config settings that will override what is in CODEX_HOME/config.toml.", "additionalProperties": true, + "description": "Individual config settings that will override what is in CODEX_HOME/config.toml.", "type": "object" }, "cwd": { "description": "Working directory for the session. If relative, it is resolved against the server process's current working directory.", "type": "string" }, + "developer-instructions": { + "description": "Developer instructions that should be injected as a developer role message.", + "type": "string" + }, "model": { - "description": "Optional override for the model name (e.g. \"o3\", \"o4-mini\").", + "description": "Optional override for the model name (e.g. 'gpt-5.2', 'gpt-5.2-codex').", "type": "string" }, "profile": { @@ -291,23 +291,23 @@ mod tests { "description": "The *initial user prompt* to start the Codex conversation.", "type": "string" }, - "base-instructions": { - "description": "The set of instructions to use instead of the default ones.", + "sandbox": { + "description": "Sandbox mode: `read-only`, `workspace-write`, or `danger-full-access`.", + "enum": [ + "read-only", + "workspace-write", + "danger-full-access" + ], "type": "string" - }, - "developer-instructions": { - "description": "Developer instructions that should be injected as a developer role message.", - "type": "string" - }, - "compact-prompt": { - "description": "Prompt used when compacting the conversation.", - "type": "string" - }, + } }, "required": [ "prompt" - ] - } + ], + "type": "object" + }, + "name": "codex", + "title": "Codex" }); assert_eq!(expected_tool_json, tool_json); } diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index b50d0c7640..629f4cc493 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -311,6 +311,7 @@ async fn run_codex_tool_session_inner( | EventMsg::UndoCompleted(_) | EventMsg::ExitedReviewMode(_) | EventMsg::ContextCompacted(_) + | EventMsg::ThreadRolledBack(_) | EventMsg::DeprecationNotice(_) => { // For now, we do not do anything extra for these // events. Note that diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index f9369de9c5..4dbd7902c5 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -210,6 +210,12 @@ pub enum Op { /// Request Codex to undo a turn (turn are stacked so it is the same effect as CMD + Z). Undo, + /// Request Codex to drop the last N user turns from in-memory context. + /// + /// This does not attempt to revert local filesystem changes. Clients are + /// responsible for undoing any edits on disk. + ThreadRollback { num_turns: u32 }, + /// Request a code review from the agent. Review { review_request: ReviewRequest }, @@ -541,6 +547,9 @@ pub enum EventMsg { /// Conversation history was compacted (either automatically or manually). ContextCompacted(ContextCompactedEvent), + /// Conversation history was rolled back by dropping the last N user turns. + ThreadRolledBack(ThreadRolledBackEvent), + /// Agent has started a task TaskStarted(TaskStartedEvent), @@ -672,6 +681,26 @@ pub enum EventMsg { ReasoningRawContentDelta(ReasoningRawContentDeltaEvent), } +/// Agent lifecycle status, derived from emitted events. +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS, Default)] +#[serde(rename_all = "snake_case")] +#[ts(rename_all = "snake_case")] +pub enum AgentStatus { + /// Agent is waiting for initialization. + #[default] + PendingInit, + /// Agent is currently running. + Running, + /// Agent is done. Contains the final assistant message. + Completed(Option), + /// Agent encountered an error. + Errored(String), + /// Agent has been shutdowned. + Shutdown, + /// Agent is not found. + NotFound, +} + /// Codex errors that we expose to clients. #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "snake_case")] @@ -698,6 +727,7 @@ pub enum CodexErrorInfo { ResponseTooManyFailedAttempts { http_status_code: Option, }, + ThreadRollbackFailed, Other, } @@ -1598,6 +1628,12 @@ pub struct UndoCompletedEvent { pub message: Option, } +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] +pub struct ThreadRolledBackEvent { + /// Number of user turns that were removed from context. + pub num_turns: u32, +} + #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct StreamErrorEvent { pub message: String, diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index e0af51e7eb..d76a5b4039 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -1334,16 +1334,19 @@ mod tests { use codex_core::AuthManager; use codex_core::CodexAuth; use codex_core::ConversationManager; + use codex_core::config::ConfigBuilder; use codex_core::protocol::AskForApproval; use codex_core::protocol::Event; use codex_core::protocol::EventMsg; use codex_core::protocol::SandboxPolicy; use codex_core::protocol::SessionConfiguredEvent; use codex_protocol::ConversationId; + use insta::assert_snapshot; use ratatui::prelude::Line; use std::path::PathBuf; use std::sync::Arc; use std::sync::atomic::AtomicBool; + use tempfile::tempdir; async fn make_test_app() -> App { let (chat_widget, app_event_tx, _rx, _op_rx) = make_chatwidget_manual_with_sender().await; @@ -1427,6 +1430,24 @@ mod tests { codex_core::models_manager::model_presets::all_model_presets().clone() } + fn model_migration_copy_to_plain_text( + copy: &crate::model_migration::ModelMigrationCopy, + ) -> String { + let mut s = String::new(); + for span in ©.heading { + s.push_str(&span.content); + } + s.push('\n'); + s.push('\n'); + for line in ©.content { + for span in &line.spans { + s.push_str(&span.content); + } + s.push('\n'); + } + s + } + #[tokio::test] async fn model_migration_prompt_only_shows_for_deprecated_models() { let seen = BTreeMap::new(); @@ -1508,6 +1529,59 @@ mod tests { assert!(target_preset_for_upgrade(&available, "missing-target").is_none()); } + #[tokio::test] + async fn model_migration_prompt_shows_for_hidden_model() { + let codex_home = tempdir().expect("temp codex home"); + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .build() + .await + .expect("config"); + + let available_models = all_model_presets(); + let current = available_models + .iter() + .find(|preset| preset.model == "gpt-5.1-codex") + .cloned() + .expect("gpt-5.1-codex preset present"); + assert!( + !current.show_in_picker, + "expected gpt-5.1-codex to be hidden from picker for this test" + ); + + let upgrade = current.upgrade.as_ref().expect("upgrade configured"); + assert!( + should_show_model_migration_prompt( + ¤t.model, + &upgrade.id, + &config.notices.model_migrations, + &available_models, + ), + "expected migration prompt to be eligible for hidden model" + ); + + let target = target_preset_for_upgrade(&available_models, &upgrade.id) + .expect("upgrade target present"); + let target_description = + (!target.description.is_empty()).then(|| target.description.clone()); + let can_opt_out = true; + let copy = migration_copy_for_models( + ¤t.model, + &upgrade.id, + upgrade.model_link.clone(), + upgrade.upgrade_copy.clone(), + target.display_name.clone(), + target_description, + can_opt_out, + ); + + // Snapshot the copy we would show; rendering is covered by model_migration snapshots. + assert_snapshot!( + "model_migration_prompt_shows_for_hidden_model", + model_migration_copy_to_plain_text(©) + ); + } + #[tokio::test] async fn update_reasoning_effort_updates_config() { let mut app = make_test_app().await; diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index d42861eb1d..0923d41b5f 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -461,9 +461,13 @@ fn exec_options( .chain( proposed_execpolicy_amendment .filter(|_| features.enabled(Feature::ExecPolicy)) - .map(|prefix| { + .and_then(|prefix| { let rendered_prefix = strip_bash_lc_and_escape(prefix.command()); - ApprovalOption { + if rendered_prefix.contains('\n') || rendered_prefix.contains('\r') { + return None; + } + + Some(ApprovalOption { label: format!( "Yes, and don't ask again for commands that start with `{rendered_prefix}`" ), @@ -474,7 +478,7 @@ fn exec_options( ), display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))], - } + }) }), ) .chain([ApprovalOption { diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index d6d01045b4..6d47739c1b 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -73,6 +73,7 @@ const LARGE_PASTE_CHAR_THRESHOLD: usize = 1000; pub enum InputResult { Submitted(String), Command(SlashCommand), + CommandWithArgs(SlashCommand, String), None, } @@ -1274,6 +1275,18 @@ impl ChatComposer { } } + if !input_starts_with_space + && let Some((name, rest)) = parse_slash_name(&text) + && !rest.is_empty() + && !name.contains('/') + && let Some((_n, cmd)) = built_in_slash_commands() + .into_iter() + .find(|(command_name, _)| *command_name == name) + && cmd == SlashCommand::Review + { + return (InputResult::CommandWithArgs(cmd, rest.to_string()), true); + } + let expanded_prompt = match expand_custom_prompt(&text, &self.custom_prompts) { Ok(expanded) => expanded, Err(err) => { @@ -1650,6 +1663,16 @@ impl ChatComposer { fn sync_popups(&mut self) { let file_token = Self::current_at_token(&self.textarea); + let browsing_history = self + .history + .should_handle_navigation(self.textarea.text(), self.textarea.cursor()); + // When browsing input history (shell-style Up/Down recall), skip all popup + // synchronization so nothing steals focus from continued history navigation. + if browsing_history { + self.active_popup = ActivePopup::None; + return; + } + let skill_token = self.current_skill_token(); let allow_command_popup = file_token.is_none() && skill_token.is_none(); @@ -2831,6 +2854,9 @@ mod tests { InputResult::Command(cmd) => { assert_eq!(cmd.command(), "init"); } + InputResult::CommandWithArgs(_, _) => { + panic!("expected command dispatch without args for '/init'") + } InputResult::Submitted(text) => { panic!("expected command dispatch, but composer submitted literal text: {text}") } @@ -2839,6 +2865,44 @@ mod tests { assert!(composer.textarea.is_empty(), "composer should be cleared"); } + #[test] + fn slash_review_with_args_dispatches_command_with_args() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + type_chars_humanlike(&mut composer, &['/', 'r', 'e', 'v', 'i', 'e', 'w', ' ']); + type_chars_humanlike(&mut composer, &['f', 'i', 'x', ' ', 't', 'h', 'i', 's']); + + let (result, _needs_redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + + match result { + InputResult::CommandWithArgs(cmd, args) => { + assert_eq!(cmd, SlashCommand::Review); + assert_eq!(args, "fix this"); + } + InputResult::Command(cmd) => { + panic!("expected args for '/review', got bare command: {cmd:?}") + } + InputResult::Submitted(text) => { + panic!("expected command dispatch, got literal submit: {text}") + } + InputResult::None => panic!("expected CommandWithArgs result for '/review'"), + } + assert!(composer.textarea.is_empty(), "composer should be cleared"); + } + #[test] fn extract_args_supports_quoted_paths_single_arg() { let args = extract_positional_args_for_prompt_line( @@ -2904,6 +2968,9 @@ mod tests { composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); match result { InputResult::Command(cmd) => assert_eq!(cmd.command(), "diff"), + InputResult::CommandWithArgs(_, _) => { + panic!("expected command dispatch without args for '/diff'") + } InputResult::Submitted(text) => { panic!("expected command dispatch after Tab completion, got literal submit: {text}") } @@ -2937,6 +3004,9 @@ mod tests { InputResult::Command(cmd) => { assert_eq!(cmd.command(), "mention"); } + InputResult::CommandWithArgs(_, _) => { + panic!("expected command dispatch without args for '/mention'") + } InputResult::Submitted(text) => { panic!("expected command dispatch, but composer submitted literal text: {text}") } @@ -4109,6 +4179,59 @@ mod tests { assert_eq!(result, InputResult::None); } + #[test] + fn history_navigation_takes_priority_over_popups() { + use codex_protocol::protocol::SkillScope; + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + use tokio::sync::mpsc::unbounded_channel; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + composer.set_skill_mentions(Some(vec![SkillMetadata { + name: "codex-cli-release-notes".to_string(), + description: "example".to_string(), + short_description: None, + path: PathBuf::from("skills/codex-cli-release-notes/SKILL.md"), + scope: SkillScope::Repo, + }])); + + // Seed local history; the newest entry triggers the skills popup. + composer.history.record_local_submission("older"); + composer + .history + .record_local_submission("$codex-cli-release-notes"); + + // First Up recalls "$...", but we should not open the skills popup while browsing history. + let (result, _redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE)); + assert_eq!(result, InputResult::None); + assert_eq!(composer.textarea.text(), "$codex-cli-release-notes"); + assert!( + matches!(composer.active_popup, ActivePopup::None), + "expected no skills popup while browsing history" + ); + + // Second Up should navigate history again (no popup should interfere). + let (result, _redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE)); + assert_eq!(result, InputResult::None); + assert_eq!(composer.textarea.text(), "older"); + assert!( + matches!(composer.active_popup, ActivePopup::None), + "expected popup to be dismissed after history navigation" + ); + } + #[test] fn slash_popup_activated_for_bare_slash_and_valid_prefixes() { // use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 7634f699aa..329d8ec805 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -416,8 +416,8 @@ impl BottomPane { self.request_redraw(); } - pub(crate) fn set_unified_exec_sessions(&mut self, sessions: Vec) { - if self.unified_exec_footer.set_sessions(sessions) { + pub(crate) fn set_unified_exec_processes(&mut self, processes: Vec) { + if self.unified_exec_footer.set_processes(processes) { self.request_redraw(); } } diff --git a/codex-rs/tui/src/bottom_pane/unified_exec_footer.rs b/codex-rs/tui/src/bottom_pane/unified_exec_footer.rs index a0ea58bed1..736ce01114 100644 --- a/codex-rs/tui/src/bottom_pane/unified_exec_footer.rs +++ b/codex-rs/tui/src/bottom_pane/unified_exec_footer.rs @@ -8,34 +8,34 @@ use crate::live_wrap::take_prefix_by_width; use crate::render::renderable::Renderable; pub(crate) struct UnifiedExecFooter { - sessions: Vec, + processes: Vec, } impl UnifiedExecFooter { pub(crate) fn new() -> Self { Self { - sessions: Vec::new(), + processes: Vec::new(), } } - pub(crate) fn set_sessions(&mut self, sessions: Vec) -> bool { - if self.sessions == sessions { + pub(crate) fn set_processes(&mut self, processes: Vec) -> bool { + if self.processes == processes { return false; } - self.sessions = sessions; + self.processes = processes; true } pub(crate) fn is_empty(&self) -> bool { - self.sessions.is_empty() + self.processes.is_empty() } fn render_lines(&self, width: u16) -> Vec> { - if self.sessions.is_empty() || width < 4 { + if self.processes.is_empty() || width < 4 { return Vec::new(); } - let count = self.sessions.len(); + let count = self.processes.len(); let plural = if count == 1 { "" } else { "s" }; let message = format!(" {count} background terminal{plural} running · /ps to view"); let (truncated, _, _) = take_prefix_by_width(&message, width as usize); @@ -72,7 +72,7 @@ mod tests { #[test] fn render_more_sessions() { let mut footer = UnifiedExecFooter::new(); - footer.set_sessions(vec!["rg \"foo\" src".to_string()]); + footer.set_processes(vec!["rg \"foo\" src".to_string()]); let width = 50; let height = footer.desired_height(width); let mut buf = Buffer::empty(Rect::new(0, 0, width, height)); @@ -83,7 +83,7 @@ mod tests { #[test] fn render_many_sessions() { let mut footer = UnifiedExecFooter::new(); - footer.set_sessions((0..123).map(|idx| format!("cmd {idx}")).collect()); + footer.set_processes((0..123).map(|idx| format!("cmd {idx}")).collect()); let width = 50; let height = footer.desired_height(width); let mut buf = Buffer::empty(Rect::new(0, 0, width, height)); diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 41ab0087ea..0894fada63 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -154,7 +154,7 @@ struct RunningCommand { source: ExecCommandSource, } -struct UnifiedExecSessionSummary { +struct UnifiedExecProcessSummary { key: String, command_display: String, } @@ -332,7 +332,7 @@ pub(crate) struct ChatWidget { suppressed_exec_calls: HashSet, last_unified_wait: Option, task_complete_pending: bool, - unified_exec_sessions: Vec, + unified_exec_processes: Vec, mcp_startup_status: Option>, // Queue of interruptive UI events deferred during an active write cycle interrupts: InterruptManager, @@ -801,6 +801,8 @@ impl ChatWidget { fn on_interrupted_turn(&mut self, reason: TurnAbortReason) { // Finalize, log a gentle prompt, and clear running state. self.finalize_turn(); + self.unified_exec_processes.clear(); + self.sync_unified_exec_footer(); if reason != TurnAbortReason::ReviewEnded { self.add_to_history(history_cell::new_error_event( @@ -866,7 +868,7 @@ impl ChatWidget { fn on_exec_command_begin(&mut self, ev: ExecCommandBeginEvent) { self.flush_answer_stream_with_separator(); if is_unified_exec_source(ev.source) { - self.track_unified_exec_session_begin(&ev); + self.track_unified_exec_process_begin(&ev); if !is_standard_tool_call(&ev.parsed_cmd) { return; } @@ -885,10 +887,10 @@ impl ChatWidget { fn on_terminal_interaction(&mut self, ev: TerminalInteractionEvent) { self.flush_answer_stream_with_separator(); let command_display = self - .unified_exec_sessions + .unified_exec_processes .iter() - .find(|session| session.key == ev.process_id) - .map(|session| session.command_display.clone()); + .find(|process| process.key == ev.process_id) + .map(|process| process.command_display.clone()); if ev.stdin.is_empty() { // Empty stdin means we are still waiting on background output; keep a live shimmer cell. if let Some(wait_cell) = self.active_cell.as_mut().and_then(|cell| { @@ -896,7 +898,7 @@ impl ChatWidget { .downcast_mut::() }) && wait_cell.matches(command_display.as_deref()) { - // Same session still waiting; update command display if it shows up late. + // Same process still waiting; update command display if it shows up late. wait_cell.update_command_display(command_display); self.request_redraw(); return; @@ -965,7 +967,7 @@ impl ChatWidget { fn on_exec_command_end(&mut self, ev: ExecCommandEndEvent) { if is_unified_exec_source(ev.source) { - self.track_unified_exec_session_end(&ev); + self.track_unified_exec_process_end(&ev); if !self.bottom_pane.is_task_running() { return; } @@ -974,20 +976,20 @@ impl ChatWidget { self.defer_or_handle(|q| q.push_exec_end(ev), |s| s.handle_exec_end_now(ev2)); } - fn track_unified_exec_session_begin(&mut self, ev: &ExecCommandBeginEvent) { + fn track_unified_exec_process_begin(&mut self, ev: &ExecCommandBeginEvent) { if ev.source != ExecCommandSource::UnifiedExecStartup { return; } let key = ev.process_id.clone().unwrap_or(ev.call_id.to_string()); let command_display = strip_bash_lc_and_escape(&ev.command); if let Some(existing) = self - .unified_exec_sessions + .unified_exec_processes .iter_mut() - .find(|session| session.key == key) + .find(|process| process.key == key) { existing.command_display = command_display; } else { - self.unified_exec_sessions.push(UnifiedExecSessionSummary { + self.unified_exec_processes.push(UnifiedExecProcessSummary { key, command_display, }); @@ -995,23 +997,23 @@ impl ChatWidget { self.sync_unified_exec_footer(); } - fn track_unified_exec_session_end(&mut self, ev: &ExecCommandEndEvent) { + fn track_unified_exec_process_end(&mut self, ev: &ExecCommandEndEvent) { let key = ev.process_id.clone().unwrap_or(ev.call_id.to_string()); - let before = self.unified_exec_sessions.len(); - self.unified_exec_sessions - .retain(|session| session.key != key); - if self.unified_exec_sessions.len() != before { + let before = self.unified_exec_processes.len(); + self.unified_exec_processes + .retain(|process| process.key != key); + if self.unified_exec_processes.len() != before { self.sync_unified_exec_footer(); } } fn sync_unified_exec_footer(&mut self) { - let sessions = self - .unified_exec_sessions + let processes = self + .unified_exec_processes .iter() - .map(|session| session.command_display.clone()) + .map(|process| process.command_display.clone()) .collect(); - self.bottom_pane.set_unified_exec_sessions(sessions); + self.bottom_pane.set_unified_exec_processes(processes); } fn on_mcp_tool_call_begin(&mut self, ev: McpToolCallBeginEvent) { @@ -1457,7 +1459,7 @@ impl ChatWidget { suppressed_exec_calls: HashSet::new(), last_unified_wait: None, task_complete_pending: false, - unified_exec_sessions: Vec::new(), + unified_exec_processes: Vec::new(), mcp_startup_status: None, interrupts: InterruptManager::new(), reasoning_buffer: String::new(), @@ -1543,7 +1545,7 @@ impl ChatWidget { suppressed_exec_calls: HashSet::new(), last_unified_wait: None, task_complete_pending: false, - unified_exec_sessions: Vec::new(), + unified_exec_processes: Vec::new(), mcp_startup_status: None, interrupts: InterruptManager::new(), reasoning_buffer: String::new(), @@ -1639,6 +1641,9 @@ impl ChatWidget { InputResult::Command(cmd) => { self.dispatch_command(cmd); } + InputResult::CommandWithArgs(cmd, args) => { + self.dispatch_command_with_args(cmd, args); + } InputResult::None => {} } } @@ -1835,6 +1840,33 @@ impl ChatWidget { } } + fn dispatch_command_with_args(&mut self, cmd: SlashCommand, args: String) { + if !cmd.available_during_task() && self.bottom_pane.is_task_running() { + let message = format!( + "'/{}' is disabled while a task is in progress.", + cmd.command() + ); + self.add_to_history(history_cell::new_error_event(message)); + self.request_redraw(); + return; + } + + let trimmed = args.trim(); + match cmd { + SlashCommand::Review if !trimmed.is_empty() => { + self.submit_op(Op::Review { + review_request: ReviewRequest { + target: ReviewTarget::Custom { + instructions: trimmed.to_string(), + }, + user_facing_hint: None, + }, + }); + } + _ => self.dispatch_command(cmd), + } + } + pub(crate) fn handle_paste(&mut self, text: String) { self.bottom_pane.handle_paste(text); } @@ -2112,6 +2144,7 @@ impl ChatWidget { } EventMsg::ExitedReviewMode(review) => self.on_exited_review_mode(review), EventMsg::ContextCompacted(_) => self.on_agent_message("Context compacted".to_owned()), + EventMsg::ThreadRolledBack(_) => {} EventMsg::RawResponseItem(_) | EventMsg::ItemStarted(_) | EventMsg::ItemCompleted(_) @@ -2262,12 +2295,12 @@ impl ChatWidget { } pub(crate) fn add_ps_output(&mut self) { - let sessions = self - .unified_exec_sessions + let processes = self + .unified_exec_processes .iter() - .map(|session| session.command_display.clone()) + .map(|process| process.command_display.clone()) .collect(); - self.add_to_history(history_cell::new_unified_exec_sessions_output(sessions)); + self.add_to_history(history_cell::new_unified_exec_processes_output(processes)); } fn stop_rate_limit_poller(&mut self) { @@ -2307,7 +2340,7 @@ impl ChatWidget { let models = self.models_manager.try_list_models(&self.config).ok()?; models .iter() - .find(|preset| preset.model == NUDGE_MODEL_SLUG) + .find(|preset| preset.show_in_picker && preset.model == NUDGE_MODEL_SLUG) .cloned() } @@ -2423,6 +2456,14 @@ impl ChatWidget { return; } }; + self.open_model_popup_with_presets(presets); + } + + pub(crate) fn open_model_popup_with_presets(&mut self, presets: Vec) { + let presets: Vec = presets + .into_iter() + .filter(|preset| preset.show_in_picker) + .collect(); let current_label = presets .iter() diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap new file mode 100644 index 0000000000..3c256fe923 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap @@ -0,0 +1,16 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: contents +--- + + + Would you like to run the following command? + + $ python - <<'PY' + print('hello') + PY + +› 1. Yes, proceed (y) + 2. No, and tell Codex what to do differently (esc) + + Press enter to confirm or esc to cancel diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__model_picker_filters_hidden_models.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__model_picker_filters_hidden_models.snap new file mode 100644 index 0000000000..56dff7b5f0 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__model_picker_filters_hidden_models.snap @@ -0,0 +1,11 @@ +--- +source: tui/src/chatwidget/tests.rs +assertion_line: 1989 +expression: popup +--- + Select Model and Effort + Access legacy models by running codex -m or in your config.toml + +› 1. test-visible-model (current) test-visible-model description + + Press enter to select reasoning effort, or esc to dismiss. diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index a0ff8d42e9..c5b0bfd55e 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -11,6 +11,7 @@ use codex_core::config::Config; use codex_core::config::ConfigBuilder; use codex_core::config::Constrained; use codex_core::config::ConstraintError; +use codex_core::features::Feature; use codex_core::models_manager::manager::ModelsManager; use codex_core::protocol::AgentMessageDeltaEvent; use codex_core::protocol::AgentMessageEvent; @@ -387,7 +388,7 @@ async fn make_chatwidget_manual( suppressed_exec_calls: HashSet::new(), last_unified_wait: None, task_complete_pending: false, - unified_exec_sessions: Vec::new(), + unified_exec_processes: Vec::new(), mcp_startup_status: None, interrupts: InterruptManager::new(), reasoning_buffer: String::new(), @@ -1961,6 +1962,41 @@ async fn model_selection_popup_snapshot() { assert_snapshot!("model_selection_popup", popup); } +#[tokio::test] +async fn model_picker_hides_show_in_picker_false_models_from_cache() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("test-visible-model")).await; + let preset = |slug: &str, show_in_picker: bool| ModelPreset { + id: slug.to_string(), + model: slug.to_string(), + display_name: slug.to_string(), + description: format!("{slug} description"), + default_reasoning_effort: ReasoningEffortConfig::Medium, + supported_reasoning_efforts: vec![ReasoningEffortPreset { + effort: ReasoningEffortConfig::Medium, + description: "medium".to_string(), + }], + is_default: false, + upgrade: None, + show_in_picker, + supported_in_api: true, + }; + + chat.open_model_popup_with_presets(vec![ + preset("test-visible-model", true), + preset("test-hidden-model", false), + ]); + let popup = render_bottom_popup(&chat, 80); + assert_snapshot!("model_picker_filters_hidden_models", popup); + assert!( + popup.contains("test-visible-model"), + "expected visible model to appear in picker:\n{popup}" + ); + assert!( + !popup.contains("test-hidden-model"), + "expected hidden model to be excluded from picker:\n{popup}" + ); +} + #[tokio::test] async fn approvals_selection_popup_snapshot() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; @@ -2465,6 +2501,48 @@ async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> { Ok(()) } +// Snapshot test: approval modal with a proposed execpolicy prefix that is multi-line; +// we should not offer adding it to execpolicy. +#[tokio::test] +async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot() +-> anyhow::Result<()> { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; + chat.config.approval_policy.set(AskForApproval::OnRequest)?; + + let script = "python - <<'PY'\nprint('hello')\nPY".to_string(); + let command = vec!["bash".into(), "-lc".into(), script]; + let ev = ExecApprovalRequestEvent { + call_id: "call-approve-cmd-multiline-trunc".into(), + turn_id: "turn-approve-cmd-multiline-trunc".into(), + command: command.clone(), + cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + reason: None, + proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)), + parsed_cmd: vec![], + }; + chat.handle_codex_event(Event { + id: "sub-approve-multiline-trunc".into(), + msg: EventMsg::ExecApprovalRequest(ev), + }); + + let width = 100; + let height = chat.desired_height(width); + let mut terminal = + ratatui::Terminal::new(VT100Backend::new(width, height)).expect("create terminal"); + terminal.set_viewport_area(Rect::new(0, 0, width, height)); + terminal + .draw(|f| chat.render(f.area(), f.buffer_mut())) + .expect("draw approval modal (multiline prefix)"); + let contents = terminal.backend().vt100().screen().contents(); + assert!(!contents.contains("don't ask again")); + assert_snapshot!( + "approval_modal_exec_multiline_prefix_no_execpolicy", + contents + ); + + Ok(()) +} + // Snapshot test: patch approval modal #[tokio::test] async fn approval_modal_patch_snapshot() -> anyhow::Result<()> { @@ -2580,6 +2658,26 @@ async fn interrupt_prepends_queued_messages_before_existing_composer_text() { let _ = drain_insert_history(&mut rx); } +#[tokio::test] +async fn interrupt_clears_unified_exec_processes() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + + begin_unified_exec_startup(&mut chat, "call-1", "process-1", "sleep 5"); + begin_unified_exec_startup(&mut chat, "call-2", "process-2", "sleep 6"); + assert_eq!(chat.unified_exec_processes.len(), 2); + + chat.handle_codex_event(Event { + id: "turn-1".into(), + msg: EventMsg::TurnAborted(codex_core::protocol::TurnAbortedEvent { + reason: TurnAbortReason::Interrupted, + }), + }); + + assert!(chat.unified_exec_processes.is_empty()); + + let _ = drain_insert_history(&mut rx); +} + // Snapshot test: ChatWidget at very small heights (idle) // Ensures overall layout behaves when terminal height is extremely constrained. #[tokio::test] diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index b487c6aeff..4e16306d13 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -517,29 +517,29 @@ pub(crate) fn new_unified_exec_wait_live( } #[derive(Debug)] -struct UnifiedExecSessionsCell { - sessions: Vec, +struct UnifiedExecProcessesCell { + processes: Vec, } -impl UnifiedExecSessionsCell { - fn new(sessions: Vec) -> Self { - Self { sessions } +impl UnifiedExecProcessesCell { + fn new(processes: Vec) -> Self { + Self { processes } } } -impl HistoryCell for UnifiedExecSessionsCell { +impl HistoryCell for UnifiedExecProcessesCell { fn display_lines(&self, width: u16) -> Vec> { if width == 0 { return Vec::new(); } let wrap_width = width as usize; - let max_sessions = 16usize; + let max_processes = 16usize; let mut out: Vec> = Vec::new(); out.push(vec!["Background terminals".bold()].into()); out.push("".into()); - if self.sessions.is_empty() { + if self.processes.is_empty() { out.push(" • No background terminals running.".italic().into()); return out; } @@ -549,8 +549,8 @@ impl HistoryCell for UnifiedExecSessionsCell { let truncation_suffix = " [...]"; let truncation_suffix_width = UnicodeWidthStr::width(truncation_suffix); let mut shown = 0usize; - for command in &self.sessions { - if shown >= max_sessions { + for command in &self.processes { + if shown >= max_processes { break; } let (snippet, snippet_truncated) = { @@ -590,7 +590,7 @@ impl HistoryCell for UnifiedExecSessionsCell { shown += 1; } - let remaining = self.sessions.len().saturating_sub(shown); + let remaining = self.processes.len().saturating_sub(shown); if remaining > 0 { let more_text = format!("... and {remaining} more running"); if wrap_width <= prefix_width { @@ -610,9 +610,9 @@ impl HistoryCell for UnifiedExecSessionsCell { } } -pub(crate) fn new_unified_exec_sessions_output(sessions: Vec) -> CompositeHistoryCell { +pub(crate) fn new_unified_exec_processes_output(processes: Vec) -> CompositeHistoryCell { let command = PlainHistoryCell::new(vec!["/ps".magenta().into()]); - let summary = UnifiedExecSessionsCell::new(sessions); + let summary = UnifiedExecProcessesCell::new(processes); CompositeHistoryCell::new(vec![Box::new(command), Box::new(summary)]) } @@ -816,11 +816,11 @@ pub(crate) fn padded_emoji(emoji: &str) -> String { #[derive(Debug)] struct TooltipHistoryCell { - tip: &'static str, + tip: String, } impl TooltipHistoryCell { - fn new(tip: &'static str) -> Self { + fn new(tip: String) -> Self { Self { tip } } } @@ -1819,14 +1819,14 @@ mod tests { #[test] fn ps_output_empty_snapshot() { - let cell = new_unified_exec_sessions_output(Vec::new()); + let cell = new_unified_exec_processes_output(Vec::new()); let rendered = render_lines(&cell.display_lines(60)).join("\n"); insta::assert_snapshot!(rendered); } #[test] fn ps_output_multiline_snapshot() { - let cell = new_unified_exec_sessions_output(vec![ + let cell = new_unified_exec_processes_output(vec![ "echo hello\nand then some extra text".to_string(), "rg \"foo\" src".to_string(), ]); @@ -1836,7 +1836,7 @@ mod tests { #[test] fn ps_output_long_command_snapshot() { - let cell = new_unified_exec_sessions_output(vec![String::from( + let cell = new_unified_exec_processes_output(vec![String::from( "rg \"foo\" src --glob '**/*.rs' --max-count 1000 --no-ignore --hidden --follow --glob '!target/**'", )]); let rendered = render_lines(&cell.display_lines(36)).join("\n"); @@ -1845,8 +1845,9 @@ mod tests { #[test] fn ps_output_many_sessions_snapshot() { - let cell = - new_unified_exec_sessions_output((0..20).map(|idx| format!("command {idx}")).collect()); + let cell = new_unified_exec_processes_output( + (0..20).map(|idx| format!("command {idx}")).collect(), + ); let rendered = render_lines(&cell.display_lines(32)).join("\n"); insta::assert_snapshot!(rendered); } diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 6b784affce..4eb487f1da 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -97,7 +97,6 @@ pub use markdown_render::render_markdown_text; pub use public_widgets::composer_input::ComposerAction; pub use public_widgets::composer_input::ComposerInput; use std::io::Write as _; - // (tests access modules directly within the crate) pub async fn run_main( @@ -344,6 +343,8 @@ async fn run_ratatui_app( ) -> color_eyre::Result { color_eyre::install()?; + tooltips::announcement::prewarm(); + // Forward panic reports through tracing so they appear in the UI status // line, but do not swallow the default/color-eyre panic handler. // Chain to the previous hook so users still get a rich panic report diff --git a/codex-rs/tui/src/snapshots/codex_tui__app__tests__model_migration_prompt_shows_for_hidden_model.snap b/codex-rs/tui/src/snapshots/codex_tui__app__tests__model_migration_prompt_shows_for_hidden_model.snap new file mode 100644 index 0000000000..9016aebea8 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__app__tests__model_migration_prompt_shows_for_hidden_model.snap @@ -0,0 +1,10 @@ +--- +source: tui/src/app.rs +assertion_line: 1579 +expression: model_migration_copy_to_plain_text(©) +--- +Codex just got an upgrade. Introducing gpt-5.2-codex. + +Codex is now powered by gpt-5.2-codex, our latest frontier agentic coding model. It is smarter and faster than its predecessors and capable of long-running project-scale work. Learn more about gpt-5.2-codex at https://openai.com/index/introducing-gpt-5-2-codex + +You can continue using gpt-5.1-codex if you prefer. diff --git a/codex-rs/tui/src/tooltips.rs b/codex-rs/tui/src/tooltips.rs index 8a3a1c1e96..402a18ba2b 100644 --- a/codex-rs/tui/src/tooltips.rs +++ b/codex-rs/tui/src/tooltips.rs @@ -2,15 +2,10 @@ use codex_core::features::FEATURES; use lazy_static::lazy_static; use rand::Rng; +const ANNOUNCEMENT_TIP_URL: &str = + "https://raw.githubusercontent.com/openai/codex/main/announcement_tip.toml"; const RAW_TOOLTIPS: &str = include_str!("../tooltips.txt"); -fn beta_tooltips() -> Vec<&'static str> { - FEATURES - .iter() - .filter_map(|spec| spec.stage.beta_announcement()) - .collect() -} - lazy_static! { static ref TOOLTIPS: Vec<&'static str> = RAW_TOOLTIPS .lines() @@ -25,9 +20,20 @@ lazy_static! { }; } -pub(crate) fn random_tooltip() -> Option<&'static str> { +fn beta_tooltips() -> Vec<&'static str> { + FEATURES + .iter() + .filter_map(|spec| spec.stage.beta_announcement()) + .collect() +} + +/// Pick a random tooltip to show to the user when starting Codex. +pub(crate) fn random_tooltip() -> Option { + if let Some(announcement) = announcement::fetch_announcement_tip() { + return Some(announcement); + } let mut rng = rand::rng(); - pick_tooltip(&mut rng) + pick_tooltip(&mut rng).map(str::to_string) } fn pick_tooltip(rng: &mut R) -> Option<&'static str> { @@ -40,9 +46,149 @@ fn pick_tooltip(rng: &mut R) -> Option<&'static str> { } } +pub(crate) mod announcement { + use crate::tooltips::ANNOUNCEMENT_TIP_URL; + use crate::version::CODEX_CLI_VERSION; + use chrono::NaiveDate; + use chrono::Utc; + use regex_lite::Regex; + use serde::Deserialize; + use std::sync::OnceLock; + use std::thread; + use std::time::Duration; + + static ANNOUNCEMENT_TIP: OnceLock> = OnceLock::new(); + + /// Prewarm the cache of the announcement tip. + pub(crate) fn prewarm() { + let _ = thread::spawn(|| ANNOUNCEMENT_TIP.get_or_init(init_announcement_tip_in_thread)); + } + + /// Fetch the announcement tip, return None if the prewarm is not done yet. + pub(crate) fn fetch_announcement_tip() -> Option { + ANNOUNCEMENT_TIP + .get() + .cloned() + .flatten() + .and_then(|raw| parse_announcement_tip_toml(&raw)) + } + + #[derive(Debug, Deserialize)] + struct AnnouncementTipRaw { + content: String, + from_date: Option, + to_date: Option, + version_regex: Option, + target_app: Option, + } + + #[derive(Debug, Deserialize)] + struct AnnouncementTipDocument { + announcements: Vec, + } + + #[derive(Debug)] + struct AnnouncementTip { + content: String, + from_date: Option, + to_date: Option, + version_regex: Option, + target_app: String, + } + + fn init_announcement_tip_in_thread() -> Option { + thread::spawn(blocking_init_announcement_tip) + .join() + .ok() + .flatten() + } + + fn blocking_init_announcement_tip() -> Option { + let response = reqwest::blocking::Client::new() + .get(ANNOUNCEMENT_TIP_URL) + .timeout(Duration::from_millis(2000)) + .send() + .ok()?; + response.error_for_status().ok()?.text().ok() + } + + pub(crate) fn parse_announcement_tip_toml(text: &str) -> Option { + let announcements = toml::from_str::(text) + .map(|doc| doc.announcements) + .or_else(|_| toml::from_str::>(text)) + .ok()?; + + let mut latest_match = None; + let today = Utc::now().date_naive(); + for raw in announcements { + let Some(tip) = AnnouncementTip::from_raw(raw) else { + continue; + }; + if tip.version_matches(CODEX_CLI_VERSION) + && tip.date_matches(today) + && tip.target_app == "cli" + { + latest_match = Some(tip.content); + } + } + latest_match + } + + impl AnnouncementTip { + fn from_raw(raw: AnnouncementTipRaw) -> Option { + let content = raw.content.trim(); + if content.is_empty() { + return None; + } + + let from_date = match raw.from_date { + Some(date) => Some(NaiveDate::parse_from_str(&date, "%Y-%m-%d").ok()?), + None => None, + }; + let to_date = match raw.to_date { + Some(date) => Some(NaiveDate::parse_from_str(&date, "%Y-%m-%d").ok()?), + None => None, + }; + let version_regex = match raw.version_regex { + Some(pattern) => Some(Regex::new(&pattern).ok()?), + None => None, + }; + + Some(Self { + content: content.to_string(), + from_date, + to_date, + version_regex, + target_app: raw.target_app.unwrap_or("cli".to_string()).to_lowercase(), + }) + } + + fn version_matches(&self, version: &str) -> bool { + self.version_regex + .as_ref() + .is_none_or(|regex| regex.is_match(version)) + } + + fn date_matches(&self, today: NaiveDate) -> bool { + if let Some(from) = self.from_date + && today < from + { + return false; + } + if let Some(to) = self.to_date + && today >= to + { + return false; + } + true + } + } +} + #[cfg(test)] mod tests { use super::*; + use crate::tooltips::announcement::parse_announcement_tip_toml; use rand::SeedableRng; use rand::rngs::StdRng; @@ -62,4 +208,104 @@ mod tests { let mut rng = StdRng::seed_from_u64(7); assert_eq!(expected, pick_tooltip(&mut rng)); } + + #[test] + fn announcement_tip_toml_picks_last_matching() { + let toml = r#" +[[announcements]] +content = "first" +from_date = "2000-01-01" + +[[announcements]] +content = "latest match" +version_regex = ".*" +target_app = "cli" + +[[announcements]] +content = "should not match" +to_date = "2000-01-01" + "#; + + assert_eq!( + Some("latest match".to_string()), + parse_announcement_tip_toml(toml) + ); + + let toml = r#" +[[announcements]] +content = "first" +from_date = "2000-01-01" +target_app = "cli" + +[[announcements]] +content = "latest match" +version_regex = ".*" + +[[announcements]] +content = "should not match" +to_date = "2000-01-01" + "#; + + assert_eq!( + Some("latest match".to_string()), + parse_announcement_tip_toml(toml) + ); + } + + #[test] + fn announcement_tip_toml_picks_no_match() { + let toml = r#" +[[announcements]] +content = "first" +from_date = "2000-01-01" +to_date = "2000-01-05" + +[[announcements]] +content = "latest match" +version_regex = "invalid_version_name" + +[[announcements]] +content = "should not match either " +target_app = "vsce" + "#; + + assert_eq!(None, parse_announcement_tip_toml(toml)); + } + + #[test] + fn announcement_tip_toml_bad_deserialization() { + let toml = r#" +[[announcements]] +content = 123 +from_date = "2000-01-01" + "#; + + assert_eq!(None, parse_announcement_tip_toml(toml)); + } + + #[test] + fn announcement_tip_toml_parse_comments() { + let toml = r#" +# Example announcement tips for Codex TUI. +# Each [[announcements]] entry is evaluated in order; the last matching one is shown. +# Dates are UTC, formatted as YYYY-MM-DD. The from_date is inclusive and the to_date is exclusive. +# version_regex matches against the CLI version (env!("CARGO_PKG_VERSION")); omit to apply to all versions. +# target_app specify which app should display the announcement (cli, vsce, ...). + +[[announcements]] +content = "Welcome to Codex! Check out the new onboarding flow." +from_date = "2024-10-01" +to_date = "2024-10-15" +target_app = "cli" +version_regex = "^0\\.0\\.0$" + +[[announcements]] +content = "This is a test announcement" + "#; + + assert_eq!( + Some("This is a test announcement".to_string()), + parse_announcement_tip_toml(toml) + ); + } } diff --git a/codex-rs/tui2/src/app.rs b/codex-rs/tui2/src/app.rs index 1f7cad5774..49bb005a7a 100644 --- a/codex-rs/tui2/src/app.rs +++ b/codex-rs/tui2/src/app.rs @@ -2088,17 +2088,20 @@ mod tests { use codex_core::AuthManager; use codex_core::CodexAuth; use codex_core::ConversationManager; + use codex_core::config::ConfigBuilder; use codex_core::protocol::AskForApproval; use codex_core::protocol::Event; use codex_core::protocol::EventMsg; use codex_core::protocol::SandboxPolicy; use codex_core::protocol::SessionConfiguredEvent; use codex_protocol::ConversationId; + use insta::assert_snapshot; use pretty_assertions::assert_eq; use ratatui::prelude::Line; use std::path::PathBuf; use std::sync::Arc; use std::sync::atomic::AtomicBool; + use tempfile::tempdir; async fn make_test_app() -> App { let (chat_widget, app_event_tx, _rx, _op_rx) = make_chatwidget_manual_with_sender().await; @@ -2208,6 +2211,24 @@ mod tests { codex_core::models_manager::model_presets::all_model_presets().clone() } + fn model_migration_copy_to_plain_text( + copy: &crate::model_migration::ModelMigrationCopy, + ) -> String { + let mut s = String::new(); + for span in ©.heading { + s.push_str(&span.content); + } + s.push('\n'); + s.push('\n'); + for line in ©.content { + for span in &line.spans { + s.push_str(&span.content); + } + s.push('\n'); + } + s + } + #[tokio::test] async fn model_migration_prompt_only_shows_for_deprecated_models() { let seen = BTreeMap::new(); @@ -2243,6 +2264,59 @@ mod tests { )); } + #[tokio::test] + async fn model_migration_prompt_shows_for_hidden_model() { + let codex_home = tempdir().expect("temp codex home"); + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .build() + .await + .expect("config"); + + let available_models = all_model_presets(); + let current = available_models + .iter() + .find(|preset| preset.model == "gpt-5.1-codex") + .cloned() + .expect("gpt-5.1-codex preset present"); + assert!( + !current.show_in_picker, + "expected gpt-5.1-codex to be hidden from picker for this test" + ); + + let upgrade = current.upgrade.as_ref().expect("upgrade configured"); + assert!( + should_show_model_migration_prompt( + ¤t.model, + &upgrade.id, + &config.notices.model_migrations, + &available_models, + ), + "expected migration prompt to be eligible for hidden model" + ); + + let target = available_models + .iter() + .find(|preset| preset.model == upgrade.id) + .cloned() + .expect("upgrade target present"); + let target_description = + (!target.description.is_empty()).then(|| target.description.clone()); + let can_opt_out = true; + let copy = migration_copy_for_models( + ¤t.model, + &upgrade.id, + target.display_name, + target_description, + can_opt_out, + ); + + assert_snapshot!( + "model_migration_prompt_shows_for_hidden_model", + model_migration_copy_to_plain_text(©) + ); + } + #[tokio::test] async fn transcript_selection_copy_includes_offscreen_lines() { let mut app = make_test_app().await; diff --git a/codex-rs/tui2/src/bottom_pane/approval_overlay.rs b/codex-rs/tui2/src/bottom_pane/approval_overlay.rs index d42861eb1d..0923d41b5f 100644 --- a/codex-rs/tui2/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui2/src/bottom_pane/approval_overlay.rs @@ -461,9 +461,13 @@ fn exec_options( .chain( proposed_execpolicy_amendment .filter(|_| features.enabled(Feature::ExecPolicy)) - .map(|prefix| { + .and_then(|prefix| { let rendered_prefix = strip_bash_lc_and_escape(prefix.command()); - ApprovalOption { + if rendered_prefix.contains('\n') || rendered_prefix.contains('\r') { + return None; + } + + Some(ApprovalOption { label: format!( "Yes, and don't ask again for commands that start with `{rendered_prefix}`" ), @@ -474,7 +478,7 @@ fn exec_options( ), display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))], - } + }) }), ) .chain([ApprovalOption { diff --git a/codex-rs/tui2/src/bottom_pane/chat_composer.rs b/codex-rs/tui2/src/bottom_pane/chat_composer.rs index 0073173fdc..e136b81971 100644 --- a/codex-rs/tui2/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui2/src/bottom_pane/chat_composer.rs @@ -76,6 +76,7 @@ const LARGE_PASTE_CHAR_THRESHOLD: usize = 1000; pub enum InputResult { Submitted(String), Command(SlashCommand), + CommandWithArgs(SlashCommand, String), None, } @@ -1191,6 +1192,18 @@ impl ChatComposer { } } + if !input_starts_with_space + && let Some((name, rest)) = parse_slash_name(&text) + && !rest.is_empty() + && !name.contains('/') + && let Some((_n, cmd)) = built_in_slash_commands() + .into_iter() + .find(|(command_name, _)| *command_name == name) + && cmd == SlashCommand::Review + { + return (InputResult::CommandWithArgs(cmd, rest.to_string()), true); + } + let expanded_prompt = match expand_custom_prompt(&text, &self.custom_prompts) { Ok(expanded) => expanded, Err(err) => { @@ -2754,6 +2767,9 @@ mod tests { InputResult::Command(cmd) => { assert_eq!(cmd.command(), "init"); } + InputResult::CommandWithArgs(_, _) => { + panic!("expected command dispatch without args for '/init'") + } InputResult::Submitted(text) => { panic!("expected command dispatch, but composer submitted literal text: {text}") } @@ -2762,6 +2778,44 @@ mod tests { assert!(composer.textarea.is_empty(), "composer should be cleared"); } + #[test] + fn slash_review_with_args_dispatches_command_with_args() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + type_chars_humanlike(&mut composer, &['/', 'r', 'e', 'v', 'i', 'e', 'w', ' ']); + type_chars_humanlike(&mut composer, &['f', 'i', 'x', ' ', 't', 'h', 'i', 's']); + + let (result, _needs_redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + + match result { + InputResult::CommandWithArgs(cmd, args) => { + assert_eq!(cmd, SlashCommand::Review); + assert_eq!(args, "fix this"); + } + InputResult::Command(cmd) => { + panic!("expected args for '/review', got bare command: {cmd:?}") + } + InputResult::Submitted(text) => { + panic!("expected command dispatch, got literal submit: {text}") + } + InputResult::None => panic!("expected CommandWithArgs result for '/review'"), + } + assert!(composer.textarea.is_empty(), "composer should be cleared"); + } + #[test] fn extract_args_supports_quoted_paths_single_arg() { let args = extract_positional_args_for_prompt_line( @@ -2827,6 +2881,9 @@ mod tests { composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); match result { InputResult::Command(cmd) => assert_eq!(cmd.command(), "diff"), + InputResult::CommandWithArgs(_, _) => { + panic!("expected command dispatch without args for '/diff'") + } InputResult::Submitted(text) => { panic!("expected command dispatch after Tab completion, got literal submit: {text}") } @@ -2860,6 +2917,9 @@ mod tests { InputResult::Command(cmd) => { assert_eq!(cmd.command(), "mention"); } + InputResult::CommandWithArgs(_, _) => { + panic!("expected command dispatch without args for '/mention'") + } InputResult::Submitted(text) => { panic!("expected command dispatch, but composer submitted literal text: {text}") } diff --git a/codex-rs/tui2/src/chatwidget.rs b/codex-rs/tui2/src/chatwidget.rs index ab59467871..053cd85307 100644 --- a/codex-rs/tui2/src/chatwidget.rs +++ b/codex-rs/tui2/src/chatwidget.rs @@ -1500,6 +1500,9 @@ impl ChatWidget { InputResult::Command(cmd) => { self.dispatch_command(cmd); } + InputResult::CommandWithArgs(cmd, args) => { + self.dispatch_command_with_args(cmd, args); + } InputResult::None => {} } } @@ -1665,6 +1668,33 @@ impl ChatWidget { } } + fn dispatch_command_with_args(&mut self, cmd: SlashCommand, args: String) { + if !cmd.available_during_task() && self.bottom_pane.is_task_running() { + let message = format!( + "'/{}' is disabled while a task is in progress.", + cmd.command() + ); + self.add_to_history(history_cell::new_error_event(message)); + self.request_redraw(); + return; + } + + let trimmed = args.trim(); + match cmd { + SlashCommand::Review if !trimmed.is_empty() => { + self.submit_op(Op::Review { + review_request: ReviewRequest { + target: ReviewTarget::Custom { + instructions: trimmed.to_string(), + }, + user_facing_hint: None, + }, + }); + } + _ => self.dispatch_command(cmd), + } + } + pub(crate) fn handle_paste(&mut self, text: String) { self.bottom_pane.handle_paste(text); } @@ -1921,6 +1951,7 @@ impl ChatWidget { EventMsg::ExitedReviewMode(review) => self.on_exited_review_mode(review), EventMsg::ContextCompacted(_) => self.on_agent_message("Context compacted".to_owned()), EventMsg::RawResponseItem(_) + | EventMsg::ThreadRolledBack(_) | EventMsg::ItemStarted(_) | EventMsg::ItemCompleted(_) | EventMsg::AgentMessageContentDelta(_) @@ -2105,7 +2136,7 @@ impl ChatWidget { let models = self.models_manager.try_list_models(&self.config).ok()?; models .iter() - .find(|preset| preset.model == NUDGE_MODEL_SLUG) + .find(|preset| preset.show_in_picker && preset.model == NUDGE_MODEL_SLUG) .cloned() } @@ -2221,6 +2252,14 @@ impl ChatWidget { return; } }; + self.open_model_popup_with_presets(presets); + } + + pub(crate) fn open_model_popup_with_presets(&mut self, presets: Vec) { + let presets: Vec = presets + .into_iter() + .filter(|preset| preset.show_in_picker) + .collect(); let current_label = presets .iter() diff --git a/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap b/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap new file mode 100644 index 0000000000..962a38ebde --- /dev/null +++ b/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap @@ -0,0 +1,16 @@ +--- +source: tui2/src/chatwidget/tests.rs +expression: contents +--- + + + Would you like to run the following command? + + $ python - <<'PY' + print('hello') + PY + +› 1. Yes, proceed (y) + 2. No, and tell Codex what to do differently (esc) + + Press enter to confirm or esc to cancel diff --git a/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__model_picker_filters_hidden_models.snap b/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__model_picker_filters_hidden_models.snap new file mode 100644 index 0000000000..a03d434905 --- /dev/null +++ b/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__model_picker_filters_hidden_models.snap @@ -0,0 +1,11 @@ +--- +source: tui2/src/chatwidget/tests.rs +assertion_line: 1758 +expression: popup +--- + Select Model and Effort + Access legacy models by running codex -m or in your config.toml + +› 1. test-visible-model (current) test-visible-model description + + Press enter to select reasoning effort, or esc to dismiss. diff --git a/codex-rs/tui2/src/chatwidget/tests.rs b/codex-rs/tui2/src/chatwidget/tests.rs index 8b216812df..cbece2dbae 100644 --- a/codex-rs/tui2/src/chatwidget/tests.rs +++ b/codex-rs/tui2/src/chatwidget/tests.rs @@ -1731,6 +1731,41 @@ async fn model_selection_popup_snapshot() { assert_snapshot!("model_selection_popup", popup); } +#[tokio::test] +async fn model_picker_hides_show_in_picker_false_models_from_cache() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("test-visible-model")).await; + let preset = |slug: &str, show_in_picker: bool| ModelPreset { + id: slug.to_string(), + model: slug.to_string(), + display_name: slug.to_string(), + description: format!("{slug} description"), + default_reasoning_effort: ReasoningEffortConfig::Medium, + supported_reasoning_efforts: vec![ReasoningEffortPreset { + effort: ReasoningEffortConfig::Medium, + description: "medium".to_string(), + }], + is_default: false, + upgrade: None, + show_in_picker, + supported_in_api: true, + }; + + chat.open_model_popup_with_presets(vec![ + preset("test-visible-model", true), + preset("test-hidden-model", false), + ]); + let popup = render_bottom_popup(&chat, 80); + assert_snapshot!("model_picker_filters_hidden_models", popup); + assert!( + popup.contains("test-visible-model"), + "expected visible model to appear in picker:\n{popup}" + ); + assert!( + !popup.contains("test-hidden-model"), + "expected hidden model to be excluded from picker:\n{popup}" + ); +} + #[tokio::test] async fn approvals_selection_popup_snapshot() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; @@ -2123,6 +2158,45 @@ async fn approval_modal_exec_without_reason_snapshot() { ); } +// Snapshot test: approval modal with a proposed execpolicy prefix that is multi-line; +// we should not offer adding it to execpolicy. +#[tokio::test] +async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; + chat.config.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); + + let script = "python - <<'PY'\nprint('hello')\nPY".to_string(); + let command = vec!["bash".into(), "-lc".into(), script]; + let ev = ExecApprovalRequestEvent { + call_id: "call-approve-cmd-multiline-trunc".into(), + turn_id: "turn-approve-cmd-multiline-trunc".into(), + command: command.clone(), + cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + reason: None, + proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)), + parsed_cmd: vec![], + }; + chat.handle_codex_event(Event { + id: "sub-approve-multiline-trunc".into(), + msg: EventMsg::ExecApprovalRequest(ev), + }); + + let width = 100; + let height = chat.desired_height(width); + let mut terminal = + ratatui::Terminal::new(VT100Backend::new(width, height)).expect("create terminal"); + terminal.set_viewport_area(Rect::new(0, 0, width, height)); + terminal + .draw(|f| chat.render(f.area(), f.buffer_mut())) + .expect("draw approval modal (multiline prefix)"); + let contents = terminal.backend().vt100().screen().contents(); + assert!(!contents.contains("don't ask again")); + assert_snapshot!( + "approval_modal_exec_multiline_prefix_no_execpolicy", + contents + ); +} + // Snapshot test: patch approval modal #[tokio::test] async fn approval_modal_patch_snapshot() { diff --git a/codex-rs/tui2/src/history_cell.rs b/codex-rs/tui2/src/history_cell.rs index 4d96b389bc..3124d0fc31 100644 --- a/codex-rs/tui2/src/history_cell.rs +++ b/codex-rs/tui2/src/history_cell.rs @@ -817,11 +817,11 @@ pub(crate) fn padded_emoji(emoji: &str) -> String { #[derive(Debug)] struct TooltipHistoryCell { - tip: &'static str, + tip: String, } impl TooltipHistoryCell { - fn new(tip: &'static str) -> Self { + fn new(tip: String) -> Self { Self { tip } } } diff --git a/codex-rs/tui2/src/lib.rs b/codex-rs/tui2/src/lib.rs index 8cd3fde133..c8106e0877 100644 --- a/codex-rs/tui2/src/lib.rs +++ b/codex-rs/tui2/src/lib.rs @@ -361,6 +361,8 @@ async fn run_ratatui_app( ) -> color_eyre::Result { color_eyre::install()?; + tooltips::announcement::prewarm(); + // Forward panic reports through tracing so they appear in the UI status // line, but do not swallow the default/color-eyre panic handler. // Chain to the previous hook so users still get a rich panic report diff --git a/codex-rs/tui2/src/snapshots/codex_tui2__app__tests__model_migration_prompt_shows_for_hidden_model.snap b/codex-rs/tui2/src/snapshots/codex_tui2__app__tests__model_migration_prompt_shows_for_hidden_model.snap new file mode 100644 index 0000000000..acfdedc100 --- /dev/null +++ b/codex-rs/tui2/src/snapshots/codex_tui2__app__tests__model_migration_prompt_shows_for_hidden_model.snap @@ -0,0 +1,12 @@ +--- +source: tui2/src/app.rs +assertion_line: 2314 +expression: model_migration_copy_to_plain_text(©) +--- +Try gpt-5.2-codex + +We recommend switching from gpt-5.1-codex to gpt-5.2-codex. + +Latest frontier agentic coding model. + +You can continue using gpt-5.1-codex if you prefer. diff --git a/codex-rs/tui2/src/snapshots/codex_tui2__transcript_copy_ui__tests__copy_pill_clears_background.snap b/codex-rs/tui2/src/snapshots/codex_tui2__transcript_copy_ui__tests__copy_pill_clears_background.snap new file mode 100644 index 0000000000..a2c3be0cf3 --- /dev/null +++ b/codex-rs/tui2/src/snapshots/codex_tui2__transcript_copy_ui__tests__copy_pill_clears_background.snap @@ -0,0 +1,7 @@ +--- +source: tui2/src/transcript_copy_ui.rs +expression: rendered +--- +xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx +xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx +xxxxxxxxxx ⧉ copy ctrl + shift + c xxxxx diff --git a/codex-rs/tui2/src/tooltips.rs b/codex-rs/tui2/src/tooltips.rs index eb419c2ea3..402a18ba2b 100644 --- a/codex-rs/tui2/src/tooltips.rs +++ b/codex-rs/tui2/src/tooltips.rs @@ -1,6 +1,9 @@ +use codex_core::features::FEATURES; use lazy_static::lazy_static; use rand::Rng; +const ANNOUNCEMENT_TIP_URL: &str = + "https://raw.githubusercontent.com/openai/codex/main/announcement_tip.toml"; const RAW_TOOLTIPS: &str = include_str!("../tooltips.txt"); lazy_static! { @@ -9,24 +12,183 @@ lazy_static! { .map(str::trim) .filter(|line| !line.is_empty() && !line.starts_with('#')) .collect(); + static ref ALL_TOOLTIPS: Vec<&'static str> = { + let mut tips = Vec::new(); + tips.extend(TOOLTIPS.iter().copied()); + tips.extend(beta_tooltips()); + tips + }; } -pub(crate) fn random_tooltip() -> Option<&'static str> { +fn beta_tooltips() -> Vec<&'static str> { + FEATURES + .iter() + .filter_map(|spec| spec.stage.beta_announcement()) + .collect() +} + +/// Pick a random tooltip to show to the user when starting Codex. +pub(crate) fn random_tooltip() -> Option { + if let Some(announcement) = announcement::fetch_announcement_tip() { + return Some(announcement); + } let mut rng = rand::rng(); - pick_tooltip(&mut rng) + pick_tooltip(&mut rng).map(str::to_string) } fn pick_tooltip(rng: &mut R) -> Option<&'static str> { - if TOOLTIPS.is_empty() { + if ALL_TOOLTIPS.is_empty() { None } else { - TOOLTIPS.get(rng.random_range(0..TOOLTIPS.len())).copied() + ALL_TOOLTIPS + .get(rng.random_range(0..ALL_TOOLTIPS.len())) + .copied() + } +} + +pub(crate) mod announcement { + use crate::tooltips::ANNOUNCEMENT_TIP_URL; + use crate::version::CODEX_CLI_VERSION; + use chrono::NaiveDate; + use chrono::Utc; + use regex_lite::Regex; + use serde::Deserialize; + use std::sync::OnceLock; + use std::thread; + use std::time::Duration; + + static ANNOUNCEMENT_TIP: OnceLock> = OnceLock::new(); + + /// Prewarm the cache of the announcement tip. + pub(crate) fn prewarm() { + let _ = thread::spawn(|| ANNOUNCEMENT_TIP.get_or_init(init_announcement_tip_in_thread)); + } + + /// Fetch the announcement tip, return None if the prewarm is not done yet. + pub(crate) fn fetch_announcement_tip() -> Option { + ANNOUNCEMENT_TIP + .get() + .cloned() + .flatten() + .and_then(|raw| parse_announcement_tip_toml(&raw)) + } + + #[derive(Debug, Deserialize)] + struct AnnouncementTipRaw { + content: String, + from_date: Option, + to_date: Option, + version_regex: Option, + target_app: Option, + } + + #[derive(Debug, Deserialize)] + struct AnnouncementTipDocument { + announcements: Vec, + } + + #[derive(Debug)] + struct AnnouncementTip { + content: String, + from_date: Option, + to_date: Option, + version_regex: Option, + target_app: String, + } + + fn init_announcement_tip_in_thread() -> Option { + thread::spawn(blocking_init_announcement_tip) + .join() + .ok() + .flatten() + } + + fn blocking_init_announcement_tip() -> Option { + let response = reqwest::blocking::Client::new() + .get(ANNOUNCEMENT_TIP_URL) + .timeout(Duration::from_millis(2000)) + .send() + .ok()?; + response.error_for_status().ok()?.text().ok() + } + + pub(crate) fn parse_announcement_tip_toml(text: &str) -> Option { + let announcements = toml::from_str::(text) + .map(|doc| doc.announcements) + .or_else(|_| toml::from_str::>(text)) + .ok()?; + + let mut latest_match = None; + let today = Utc::now().date_naive(); + for raw in announcements { + let Some(tip) = AnnouncementTip::from_raw(raw) else { + continue; + }; + if tip.version_matches(CODEX_CLI_VERSION) + && tip.date_matches(today) + && tip.target_app == "cli" + { + latest_match = Some(tip.content); + } + } + latest_match + } + + impl AnnouncementTip { + fn from_raw(raw: AnnouncementTipRaw) -> Option { + let content = raw.content.trim(); + if content.is_empty() { + return None; + } + + let from_date = match raw.from_date { + Some(date) => Some(NaiveDate::parse_from_str(&date, "%Y-%m-%d").ok()?), + None => None, + }; + let to_date = match raw.to_date { + Some(date) => Some(NaiveDate::parse_from_str(&date, "%Y-%m-%d").ok()?), + None => None, + }; + let version_regex = match raw.version_regex { + Some(pattern) => Some(Regex::new(&pattern).ok()?), + None => None, + }; + + Some(Self { + content: content.to_string(), + from_date, + to_date, + version_regex, + target_app: raw.target_app.unwrap_or("cli".to_string()).to_lowercase(), + }) + } + + fn version_matches(&self, version: &str) -> bool { + self.version_regex + .as_ref() + .is_none_or(|regex| regex.is_match(version)) + } + + fn date_matches(&self, today: NaiveDate) -> bool { + if let Some(from) = self.from_date + && today < from + { + return false; + } + if let Some(to) = self.to_date + && today >= to + { + return false; + } + true + } } } #[cfg(test)] mod tests { use super::*; + use crate::tooltips::announcement::parse_announcement_tip_toml; use rand::SeedableRng; use rand::rngs::StdRng; @@ -46,4 +208,104 @@ mod tests { let mut rng = StdRng::seed_from_u64(7); assert_eq!(expected, pick_tooltip(&mut rng)); } + + #[test] + fn announcement_tip_toml_picks_last_matching() { + let toml = r#" +[[announcements]] +content = "first" +from_date = "2000-01-01" + +[[announcements]] +content = "latest match" +version_regex = ".*" +target_app = "cli" + +[[announcements]] +content = "should not match" +to_date = "2000-01-01" + "#; + + assert_eq!( + Some("latest match".to_string()), + parse_announcement_tip_toml(toml) + ); + + let toml = r#" +[[announcements]] +content = "first" +from_date = "2000-01-01" +target_app = "cli" + +[[announcements]] +content = "latest match" +version_regex = ".*" + +[[announcements]] +content = "should not match" +to_date = "2000-01-01" + "#; + + assert_eq!( + Some("latest match".to_string()), + parse_announcement_tip_toml(toml) + ); + } + + #[test] + fn announcement_tip_toml_picks_no_match() { + let toml = r#" +[[announcements]] +content = "first" +from_date = "2000-01-01" +to_date = "2000-01-05" + +[[announcements]] +content = "latest match" +version_regex = "invalid_version_name" + +[[announcements]] +content = "should not match either " +target_app = "vsce" + "#; + + assert_eq!(None, parse_announcement_tip_toml(toml)); + } + + #[test] + fn announcement_tip_toml_bad_deserialization() { + let toml = r#" +[[announcements]] +content = 123 +from_date = "2000-01-01" + "#; + + assert_eq!(None, parse_announcement_tip_toml(toml)); + } + + #[test] + fn announcement_tip_toml_parse_comments() { + let toml = r#" +# Example announcement tips for Codex TUI. +# Each [[announcements]] entry is evaluated in order; the last matching one is shown. +# Dates are UTC, formatted as YYYY-MM-DD. The from_date is inclusive and the to_date is exclusive. +# version_regex matches against the CLI version (env!("CARGO_PKG_VERSION")); omit to apply to all versions. +# target_app specify which app should display the announcement (cli, vsce, ...). + +[[announcements]] +content = "Welcome to Codex! Check out the new onboarding flow." +from_date = "2024-10-01" +to_date = "2024-10-15" +target_app = "cli" +version_regex = "^0\\.0\\.0$" + +[[announcements]] +content = "This is a test announcement" + "#; + + assert_eq!( + Some("This is a test announcement".to_string()), + parse_announcement_tip_toml(toml) + ); + } } diff --git a/codex-rs/tui2/src/transcript_copy_ui.rs b/codex-rs/tui2/src/transcript_copy_ui.rs index 6ec76a1de9..9bc9e8d664 100644 --- a/codex-rs/tui2/src/transcript_copy_ui.rs +++ b/codex-rs/tui2/src/transcript_copy_ui.rs @@ -287,6 +287,13 @@ impl TranscriptCopyUi { let icon_style = base_style.add_modifier(Modifier::BOLD).fg(Color::LightCyan); let bold_style = base_style.add_modifier(Modifier::BOLD); + // Clear background so underlying text doesn't bleed through the pill. + for position in pill_area.positions() { + let cell = &mut buf[position]; + cell.set_symbol(" "); + cell.set_style(base_style); + } + let mut spans: Vec> = vec![ Span::styled(" ", base_style), Span::styled("⧉", icon_style), @@ -305,6 +312,7 @@ impl TranscriptCopyUi { #[cfg(test)] mod tests { use super::*; + use insta::assert_snapshot; use ratatui::buffer::Buffer; fn buf_to_string(buf: &Buffer, area: Rect) -> String { @@ -359,4 +367,21 @@ mod tests { assert_eq!(rect.y, 1); assert!(ui.hit_test(rect.x, rect.y)); } + + #[test] + fn copy_pill_clears_background() { + let area = Rect::new(0, 0, 40, 3); + let mut buf = Buffer::empty(area); + for y in 0..area.height { + for x in 0..area.width { + buf[(x, y)].set_symbol("x"); + } + } + + let mut ui = TranscriptCopyUi::new_with_shortcut(CopySelectionShortcut::CtrlShiftC); + ui.render_copy_pill(area, &mut buf, (1, 2), (1, 6), 0, 3); + + let rendered = buf_to_string(&buf, area); + assert_snapshot!("copy_pill_clears_background", rendered); + } } diff --git a/codex-rs/utils/git/src/apply.rs b/codex-rs/utils/git/src/apply.rs index c9e8503242..b9adb82e6b 100644 --- a/codex-rs/utils/git/src/apply.rs +++ b/codex-rs/utils/git/src/apply.rs @@ -192,28 +192,130 @@ fn render_command_for_log(cwd: &Path, git_cfg: &[String], args: &[String]) -> St /// Collect every path referenced by the diff headers inside `diff --git` sections. pub fn extract_paths_from_patch(diff_text: &str) -> Vec { - static RE: Lazy = Lazy::new(|| { - Regex::new(r"(?m)^diff --git a/(.*?) b/(.*)$") - .unwrap_or_else(|e| panic!("invalid regex: {e}")) - }); let mut set = std::collections::BTreeSet::new(); - for caps in RE.captures_iter(diff_text) { - if let Some(a) = caps.get(1).map(|m| m.as_str()) - && a != "/dev/null" - && !a.trim().is_empty() - { - set.insert(a.to_string()); + for raw_line in diff_text.lines() { + let line = raw_line.trim(); + let Some(rest) = line.strip_prefix("diff --git ") else { + continue; + }; + let Some((a, b)) = parse_diff_git_paths(rest) else { + continue; + }; + if let Some(a) = normalize_diff_path(&a, "a/") { + set.insert(a); } - if let Some(b) = caps.get(2).map(|m| m.as_str()) - && b != "/dev/null" - && !b.trim().is_empty() - { - set.insert(b.to_string()); + if let Some(b) = normalize_diff_path(&b, "b/") { + set.insert(b); } } set.into_iter().collect() } +fn parse_diff_git_paths(line: &str) -> Option<(String, String)> { + let mut chars = line.chars().peekable(); + let first = read_diff_git_token(&mut chars)?; + let second = read_diff_git_token(&mut chars)?; + Some((first, second)) +} + +fn read_diff_git_token(chars: &mut std::iter::Peekable>) -> Option { + while matches!(chars.peek(), Some(c) if c.is_whitespace()) { + chars.next(); + } + let quote = match chars.peek().copied() { + Some('"') | Some('\'') => chars.next(), + _ => None, + }; + let mut out = String::new(); + while let Some(c) = chars.next() { + if let Some(q) = quote { + if c == q { + break; + } + if c == '\\' { + out.push('\\'); + if let Some(next) = chars.next() { + out.push(next); + } + continue; + } + } else if c.is_whitespace() { + break; + } + out.push(c); + } + if out.is_empty() && quote.is_none() { + None + } else { + Some(match quote { + Some(_) => unescape_c_string(&out), + None => out, + }) + } +} + +fn normalize_diff_path(raw: &str, prefix: &str) -> Option { + let trimmed = raw.trim(); + if trimmed.is_empty() { + return None; + } + if trimmed == "/dev/null" || trimmed == format!("{prefix}dev/null") { + return None; + } + let trimmed = trimmed.strip_prefix(prefix).unwrap_or(trimmed); + if trimmed.is_empty() { + return None; + } + Some(trimmed.to_string()) +} + +fn unescape_c_string(input: &str) -> String { + let mut out = String::with_capacity(input.len()); + let mut chars = input.chars().peekable(); + while let Some(c) = chars.next() { + if c != '\\' { + out.push(c); + continue; + } + let Some(next) = chars.next() else { + out.push('\\'); + break; + }; + match next { + 'n' => out.push('\n'), + 'r' => out.push('\r'), + 't' => out.push('\t'), + 'b' => out.push('\u{0008}'), + 'f' => out.push('\u{000C}'), + 'a' => out.push('\u{0007}'), + 'v' => out.push('\u{000B}'), + '\\' => out.push('\\'), + '"' => out.push('"'), + '\'' => out.push('\''), + '0'..='7' => { + let mut value = next.to_digit(8).unwrap_or(0); + for _ in 0..2 { + match chars.peek() { + Some('0'..='7') => { + if let Some(digit) = chars.next() { + value = value * 8 + digit.to_digit(8).unwrap_or(0); + } else { + break; + } + } + _ => break, + } + } + if let Some(ch) = std::char::from_u32(value) { + out.push(ch); + } + } + other => out.push(other), + } + } + out +} + /// Stage only the files that actually exist on disk for the given diff. pub fn stage_paths(git_root: &Path, diff: &str) -> io::Result<()> { let paths = extract_paths_from_patch(diff); @@ -266,12 +368,12 @@ pub fn parse_git_apply_output( let first = trimmed.chars().next().unwrap_or('\0'); let last = trimmed.chars().last().unwrap_or('\0'); let unquoted = if (first == '"' || first == '\'') && last == first && trimmed.len() >= 2 { - &trimmed[1..trimmed.len() - 1] + unescape_c_string(&trimmed[1..trimmed.len() - 1]) } else { - trimmed + trimmed.to_string() }; if !unquoted.is_empty() { - set.insert(unquoted.to_string()); + set.insert(unquoted); } } @@ -531,6 +633,36 @@ mod tests { .replace("\r\n", "\n") } + #[test] + fn extract_paths_handles_quoted_headers() { + let diff = "diff --git \"a/hello world.txt\" \"b/hello world.txt\"\nnew file mode 100644\n--- /dev/null\n+++ b/hello world.txt\n@@ -0,0 +1 @@\n+hi\n"; + let paths = extract_paths_from_patch(diff); + assert_eq!(paths, vec!["hello world.txt".to_string()]); + } + + #[test] + fn extract_paths_ignores_dev_null_header() { + let diff = "diff --git a/dev/null b/ok.txt\nnew file mode 100644\n--- /dev/null\n+++ b/ok.txt\n@@ -0,0 +1 @@\n+hi\n"; + let paths = extract_paths_from_patch(diff); + assert_eq!(paths, vec!["ok.txt".to_string()]); + } + + #[test] + fn extract_paths_unescapes_c_style_in_quoted_headers() { + let diff = "diff --git \"a/hello\\tworld.txt\" \"b/hello\\tworld.txt\"\nnew file mode 100644\n--- /dev/null\n+++ b/hello\tworld.txt\n@@ -0,0 +1 @@\n+hi\n"; + let paths = extract_paths_from_patch(diff); + assert_eq!(paths, vec!["hello\tworld.txt".to_string()]); + } + + #[test] + fn parse_output_unescapes_quoted_paths() { + let stderr = "error: patch failed: \"hello\\tworld.txt\":1\n"; + let (applied, skipped, conflicted) = parse_git_apply_output("", stderr); + assert_eq!(applied, Vec::::new()); + assert_eq!(conflicted, Vec::::new()); + assert_eq!(skipped, vec!["hello\tworld.txt".to_string()]); + } + #[test] fn apply_add_success() { let _g = env_lock().lock().unwrap(); diff --git a/justfile b/justfile index 43ebae74aa..ef150fb266 100644 --- a/justfile +++ b/justfile @@ -25,7 +25,7 @@ app-server-test-client *args: # format code fmt: - cargo fmt -- --config imports_granularity=Item + cargo fmt -- --config imports_granularity=Item 2>/dev/null fix *args: cargo clippy --fix --all-features --tests --allow-dirty "$@"