diff --git a/.github/workflows/cla.yml b/.github/workflows/cla.yml deleted file mode 100644 index bab34d036..000000000 --- a/.github/workflows/cla.yml +++ /dev/null @@ -1,49 +0,0 @@ -name: CLA Assistant -on: - issue_comment: - types: [created] - pull_request_target: - types: [opened, closed, synchronize] - -permissions: - actions: write - contents: write - pull-requests: write - statuses: write - -jobs: - cla: - # Only run the CLA assistant for the canonical openai repo so forks are not blocked - # and contributors who signed previously do not receive duplicate CLA notifications. - if: ${{ github.repository_owner == 'openai' }} - runs-on: ubuntu-latest - steps: - - uses: contributor-assistant/github-action@v2.6.1 - # Run on close only if the PR was merged. This will lock the PR to preserve - # the CLA agreement. We don't want to lock PRs that have been closed without - # merging because the contributor may want to respond with additional comments. - # This action has a "lock-pullrequest-aftermerge" option that can be set to false, - # but that would unconditionally skip locking even in cases where the PR was merged. - if: | - ( - github.event_name == 'pull_request_target' && - ( - github.event.action == 'opened' || - github.event.action == 'synchronize' || - (github.event.action == 'closed' && github.event.pull_request.merged == true) - ) - ) || - ( - github.event_name == 'issue_comment' && - ( - github.event.comment.body == 'recheck' || - github.event.comment.body == 'I have read the CLA Document and I hereby sign the CLA' - ) - ) - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - with: - path-to-document: https://github.com/openai/codex/blob/main/docs/CLA.md - path-to-signatures: signatures/cla.json - branch: cla-signatures - allowlist: codex,dependabot,dependabot[bot],github-actions[bot] diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index 640471cef..0d7772d5b 100644 --- a/.github/workflows/rust-release.yml +++ b/.github/workflows/rust-release.yml @@ -323,6 +323,26 @@ jobs: - name: Checkout repository uses: actions/checkout@v6 + - name: Generate release notes from tag commit message + id: release_notes + shell: bash + run: | + set -euo pipefail + + # On tag pushes, GITHUB_SHA may be a tag object for annotated tags; + # peel it to the underlying commit. + commit="$(git rev-parse "${GITHUB_SHA}^{commit}")" + notes_path="${RUNNER_TEMP}/release-notes.md" + + # Use the commit message for the commit the tag points at (not the + # annotated tag message). + git log -1 --format=%B "${commit}" > "${notes_path}" + # Ensure trailing newline so GitHub's markdown renderer doesn't + # occasionally run the last line into subsequent content. + echo >> "${notes_path}" + + echo "path=${notes_path}" >> "${GITHUB_OUTPUT}" + - uses: actions/download-artifact@v7 with: path: dist @@ -395,6 +415,7 @@ jobs: with: name: ${{ steps.release_name.outputs.name }} tag_name: ${{ github.ref_name }} + body_path: ${{ steps.release_notes.outputs.path }} files: dist/** # Mark as prerelease only when the version has a suffix after x.y.z # (e.g. -alpha, -beta). Otherwise publish a normal release. diff --git a/announcement_tip.toml b/announcement_tip.toml new file mode 100644 index 000000000..3ad4a7659 --- /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 735b8dc88..a08d58af2 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", ] @@ -1867,7 +1867,7 @@ dependencies = [ name = "codex-utils-cache" version = "0.0.0" dependencies = [ - "lru 0.16.2", + "lru 0.16.3", "sha1", "tokio", ] @@ -2782,7 +2782,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]] @@ -2890,7 +2890,7 @@ checksum = "0ce92ff622d6dadf7349484f42c93271a0d49b7cc4d466a936405bacbe10aa78" dependencies = [ "cfg-if", "rustix 1.0.8", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -3831,7 +3831,7 @@ checksum = "e04d7f318608d35d4b61ddd75cbdaee86b023ebe2bd5a66ee0915f0bf93095a9" dependencies = [ "hermit-abi", "libc", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -4152,9 +4152,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", ] @@ -5332,7 +5332,7 @@ dependencies = [ "once_cell", "socket2 0.6.1", "tracing", - "windows-sys 0.52.0", + "windows-sys 0.60.2", ] [[package]] @@ -5460,7 +5460,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", @@ -5711,7 +5711,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.4.15", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -5724,7 +5724,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.9.4", - "windows-sys 0.52.0", + "windows-sys 0.60.2", ] [[package]] @@ -8012,7 +8012,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 f42ed30bf..b7c016e30 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 83fa53b99..e9e71140a 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 ba1e6261c..6fa6dfabb 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 13a654535..dedf51ea1 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 b66c59d55..526558250 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 267d7577e..b8f7de90a 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 ad0455a05..c9b78fe8c 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 5930fe435..1cadd5e4f 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; } @@ -1103,7 +1112,7 @@ impl CodexMessageProcessor { } async fn get_user_saved_config(&self, request_id: RequestId) { - let service = ConfigService::new(self.config.codex_home.clone(), Vec::new()); + let service = ConfigService::new_with_defaults(self.config.codex_home.clone()); let user_saved_config: UserSavedConfig = match service.load_user_saved_config().await { Ok(config) => config, Err(err) => { @@ -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/config_api.rs b/codex-rs/app-server/src/config_api.rs index 98e0f108e..b06de3f1e 100644 --- a/codex-rs/app-server/src/config_api.rs +++ b/codex-rs/app-server/src/config_api.rs @@ -9,6 +9,7 @@ use codex_app_server_protocol::ConfigWriteResponse; use codex_app_server_protocol::JSONRPCErrorError; use codex_core::config::ConfigService; use codex_core::config::ConfigServiceError; +use codex_core::config_loader::LoaderOverrides; use serde_json::json; use std::path::PathBuf; use toml::Value as TomlValue; @@ -19,9 +20,13 @@ pub(crate) struct ConfigApi { } impl ConfigApi { - pub(crate) fn new(codex_home: PathBuf, cli_overrides: Vec<(String, TomlValue)>) -> Self { + pub(crate) fn new( + codex_home: PathBuf, + cli_overrides: Vec<(String, TomlValue)>, + loader_overrides: LoaderOverrides, + ) -> Self { Self { - service: ConfigService::new(codex_home, cli_overrides), + service: ConfigService::new(codex_home, cli_overrides, loader_overrides), } } diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 224e0da10..68663a991 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -1,7 +1,8 @@ #![deny(clippy::print_stdout, clippy::print_stderr)] use codex_common::CliConfigOverrides; -use codex_core::config::Config; +use codex_core::config::ConfigBuilder; +use codex_core::config_loader::LoaderOverrides; use std::io::ErrorKind; use std::io::Result as IoResult; use std::path::PathBuf; @@ -42,6 +43,7 @@ const CHANNEL_CAPACITY: usize = 128; pub async fn run_main( codex_linux_sandbox_exe: Option, cli_config_overrides: CliConfigOverrides, + loader_overrides: LoaderOverrides, ) -> IoResult<()> { // Set up channels. let (incoming_tx, mut incoming_rx) = mpsc::channel::(CHANNEL_CAPACITY); @@ -78,7 +80,11 @@ pub async fn run_main( format!("error parsing -c overrides: {e}"), ) })?; - let config = Config::load_with_cli_overrides(cli_kv_overrides.clone()) + let loader_overrides_for_config_api = loader_overrides.clone(); + let config = ConfigBuilder::default() + .cli_overrides(cli_kv_overrides.clone()) + .loader_overrides(loader_overrides) + .build() .await .map_err(|e| { std::io::Error::new(ErrorKind::InvalidData, format!("error loading config: {e}")) @@ -120,11 +126,13 @@ pub async fn run_main( let processor_handle = tokio::spawn({ let outgoing_message_sender = OutgoingMessageSender::new(outgoing_tx); let cli_overrides: Vec<(String, TomlValue)> = cli_kv_overrides.clone(); + let loader_overrides = loader_overrides_for_config_api; let mut processor = MessageProcessor::new( outgoing_message_sender, codex_linux_sandbox_exe, std::sync::Arc::new(config), cli_overrides, + loader_overrides, feedback.clone(), ); async move { diff --git a/codex-rs/app-server/src/main.rs b/codex-rs/app-server/src/main.rs index 689ec0877..be57311e8 100644 --- a/codex-rs/app-server/src/main.rs +++ b/codex-rs/app-server/src/main.rs @@ -1,10 +1,42 @@ use codex_app_server::run_main; use codex_arg0::arg0_dispatch_or_else; use codex_common::CliConfigOverrides; +use codex_core::config_loader::LoaderOverrides; +use std::path::PathBuf; + +// Debug-only test hook: lets integration tests point the server at a temporary +// managed config file without writing to /etc. +const MANAGED_CONFIG_PATH_ENV_VAR: &str = "CODEX_APP_SERVER_MANAGED_CONFIG_PATH"; fn main() -> anyhow::Result<()> { arg0_dispatch_or_else(|codex_linux_sandbox_exe| async move { - run_main(codex_linux_sandbox_exe, CliConfigOverrides::default()).await?; + let managed_config_path = managed_config_path_from_debug_env(); + let loader_overrides = LoaderOverrides { + managed_config_path, + ..Default::default() + }; + + run_main( + codex_linux_sandbox_exe, + CliConfigOverrides::default(), + loader_overrides, + ) + .await?; Ok(()) }) } + +fn managed_config_path_from_debug_env() -> Option { + #[cfg(debug_assertions)] + { + if let Ok(value) = std::env::var(MANAGED_CONFIG_PATH_ENV_VAR) { + return if value.is_empty() { + None + } else { + Some(PathBuf::from(value)) + }; + } + } + + None +} diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 6a6cf5edb..be57ad397 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -20,6 +20,7 @@ use codex_app_server_protocol::RequestId; use codex_core::AuthManager; use codex_core::ConversationManager; use codex_core::config::Config; +use codex_core::config_loader::LoaderOverrides; use codex_core::default_client::USER_AGENT_SUFFIX; use codex_core::default_client::get_codex_user_agent; use codex_feedback::CodexFeedback; @@ -41,6 +42,7 @@ impl MessageProcessor { codex_linux_sandbox_exe: Option, config: Arc, cli_overrides: Vec<(String, TomlValue)>, + loader_overrides: LoaderOverrides, feedback: CodexFeedback, ) -> Self { let outgoing = Arc::new(outgoing); @@ -62,7 +64,7 @@ impl MessageProcessor { cli_overrides.clone(), feedback, ); - let config_api = ConfigApi::new(config.codex_home.clone(), cli_overrides); + let config_api = ConfigApi::new(config.codex_home.clone(), cli_overrides, loader_overrides); Self { outgoing, diff --git a/codex-rs/app-server/src/models.rs b/codex-rs/app-server/src/models.rs index 214116035..29a9c9963 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 e4b3c8a09..0817fe07e 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; @@ -318,6 +319,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/config_rpc.rs b/codex-rs/app-server/tests/suite/v2/config_rpc.rs index c0be58f50..18311d324 100644 --- a/codex-rs/app-server/tests/suite/v2/config_rpc.rs +++ b/codex-rs/app-server/tests/suite/v2/config_rpc.rs @@ -184,7 +184,10 @@ writable_roots = [{}] let mut mcp = McpProcess::new_with_env( codex_home.path(), - &[("CODEX_MANAGED_CONFIG_PATH", Some(&managed_path_str))], + &[( + "CODEX_APP_SERVER_MANAGED_CONFIG_PATH", + Some(&managed_path_str), + )], ) .await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; diff --git a/codex-rs/app-server/tests/suite/v2/mod.rs b/codex-rs/app-server/tests/suite/v2/mod.rs index f23f792d3..1ef00c693 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 e9fe70dbe..4cdb30620 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 000000000..f3313c759 --- /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/apply-patch/src/parser.rs b/codex-rs/apply-patch/src/parser.rs index 768c89ad7..8785b3851 100644 --- a/codex-rs/apply-patch/src/parser.rs +++ b/codex-rs/apply-patch/src/parser.rs @@ -227,11 +227,14 @@ fn check_start_and_end_lines_strict( first_line: Option<&&str>, last_line: Option<&&str>, ) -> Result<(), ParseError> { + let first_line = first_line.map(|line| line.trim()); + let last_line = last_line.map(|line| line.trim()); + match (first_line, last_line) { - (Some(&first), Some(&last)) if first == BEGIN_PATCH_MARKER && last == END_PATCH_MARKER => { + (Some(first), Some(last)) if first == BEGIN_PATCH_MARKER && last == END_PATCH_MARKER => { Ok(()) } - (Some(&first), _) if first != BEGIN_PATCH_MARKER => Err(InvalidPatchError(String::from( + (Some(first), _) if first != BEGIN_PATCH_MARKER => Err(InvalidPatchError(String::from( "The first line of the patch must be '*** Begin Patch'", ))), _ => Err(InvalidPatchError(String::from( @@ -444,6 +447,25 @@ fn test_parse_patch() { "The last line of the patch must be '*** End Patch'".to_string() )) ); + + assert_eq!( + parse_patch_text( + concat!( + "*** Begin Patch", + " ", + "\n*** Add File: foo\n+hi\n", + " ", + "*** End Patch" + ), + ParseMode::Strict + ) + .unwrap() + .hunks, + vec![AddFile { + path: PathBuf::from("foo"), + contents: "hi\n".to_string() + }] + ); assert_eq!( parse_patch_text( "*** Begin Patch\n\ diff --git a/codex-rs/apply-patch/tests/fixtures/scenarios/020_delete_file_success/expected/keep.txt b/codex-rs/apply-patch/tests/fixtures/scenarios/020_delete_file_success/expected/keep.txt new file mode 100644 index 000000000..2fa992c0b --- /dev/null +++ b/codex-rs/apply-patch/tests/fixtures/scenarios/020_delete_file_success/expected/keep.txt @@ -0,0 +1 @@ +keep diff --git a/codex-rs/apply-patch/tests/fixtures/scenarios/020_delete_file_success/input/keep.txt b/codex-rs/apply-patch/tests/fixtures/scenarios/020_delete_file_success/input/keep.txt new file mode 100644 index 000000000..2fa992c0b --- /dev/null +++ b/codex-rs/apply-patch/tests/fixtures/scenarios/020_delete_file_success/input/keep.txt @@ -0,0 +1 @@ +keep diff --git a/codex-rs/apply-patch/tests/fixtures/scenarios/020_delete_file_success/input/obsolete.txt b/codex-rs/apply-patch/tests/fixtures/scenarios/020_delete_file_success/input/obsolete.txt new file mode 100644 index 000000000..6e263abce --- /dev/null +++ b/codex-rs/apply-patch/tests/fixtures/scenarios/020_delete_file_success/input/obsolete.txt @@ -0,0 +1 @@ +obsolete diff --git a/codex-rs/apply-patch/tests/fixtures/scenarios/020_delete_file_success/patch.txt b/codex-rs/apply-patch/tests/fixtures/scenarios/020_delete_file_success/patch.txt new file mode 100644 index 000000000..5978f7388 --- /dev/null +++ b/codex-rs/apply-patch/tests/fixtures/scenarios/020_delete_file_success/patch.txt @@ -0,0 +1,3 @@ +*** Begin Patch +*** Delete File: obsolete.txt +*** End Patch diff --git a/codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/expected/file.txt b/codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/expected/file.txt new file mode 100644 index 000000000..f719efd43 --- /dev/null +++ b/codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/expected/file.txt @@ -0,0 +1 @@ +two diff --git a/codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/input/file.txt b/codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/input/file.txt new file mode 100644 index 000000000..5626abf0f --- /dev/null +++ b/codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/input/file.txt @@ -0,0 +1 @@ +one diff --git a/codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/patch.txt b/codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/patch.txt new file mode 100644 index 000000000..3d2a1dbe5 --- /dev/null +++ b/codex-rs/apply-patch/tests/fixtures/scenarios/020_whitespace_padded_patch_marker_lines/patch.txt @@ -0,0 +1,6 @@ +*** Begin Patch +*** Update File: file.txt +@@ +-one ++two + *** End Patch diff --git a/codex-rs/apply-patch/tests/fixtures/scenarios/021_update_file_deletion_only/expected/lines.txt b/codex-rs/apply-patch/tests/fixtures/scenarios/021_update_file_deletion_only/expected/lines.txt new file mode 100644 index 000000000..8129d305c --- /dev/null +++ b/codex-rs/apply-patch/tests/fixtures/scenarios/021_update_file_deletion_only/expected/lines.txt @@ -0,0 +1,2 @@ +line1 +line3 diff --git a/codex-rs/apply-patch/tests/fixtures/scenarios/021_update_file_deletion_only/input/lines.txt b/codex-rs/apply-patch/tests/fixtures/scenarios/021_update_file_deletion_only/input/lines.txt new file mode 100644 index 000000000..83db48f84 --- /dev/null +++ b/codex-rs/apply-patch/tests/fixtures/scenarios/021_update_file_deletion_only/input/lines.txt @@ -0,0 +1,3 @@ +line1 +line2 +line3 diff --git a/codex-rs/apply-patch/tests/fixtures/scenarios/021_update_file_deletion_only/patch.txt b/codex-rs/apply-patch/tests/fixtures/scenarios/021_update_file_deletion_only/patch.txt new file mode 100644 index 000000000..860c6c9a9 --- /dev/null +++ b/codex-rs/apply-patch/tests/fixtures/scenarios/021_update_file_deletion_only/patch.txt @@ -0,0 +1,7 @@ +*** Begin Patch +*** Update File: lines.txt +@@ + line1 +-line2 + line3 +*** End Patch diff --git a/codex-rs/apply-patch/tests/fixtures/scenarios/022_update_file_end_of_file_marker/expected/tail.txt b/codex-rs/apply-patch/tests/fixtures/scenarios/022_update_file_end_of_file_marker/expected/tail.txt new file mode 100644 index 000000000..87463f92d --- /dev/null +++ b/codex-rs/apply-patch/tests/fixtures/scenarios/022_update_file_end_of_file_marker/expected/tail.txt @@ -0,0 +1,2 @@ +first +second updated diff --git a/codex-rs/apply-patch/tests/fixtures/scenarios/022_update_file_end_of_file_marker/input/tail.txt b/codex-rs/apply-patch/tests/fixtures/scenarios/022_update_file_end_of_file_marker/input/tail.txt new file mode 100644 index 000000000..66a52ee7a --- /dev/null +++ b/codex-rs/apply-patch/tests/fixtures/scenarios/022_update_file_end_of_file_marker/input/tail.txt @@ -0,0 +1,2 @@ +first +second diff --git a/codex-rs/apply-patch/tests/fixtures/scenarios/022_update_file_end_of_file_marker/patch.txt b/codex-rs/apply-patch/tests/fixtures/scenarios/022_update_file_end_of_file_marker/patch.txt new file mode 100644 index 000000000..8b16b5bd9 --- /dev/null +++ b/codex-rs/apply-patch/tests/fixtures/scenarios/022_update_file_end_of_file_marker/patch.txt @@ -0,0 +1,8 @@ +*** Begin Patch +*** Update File: tail.txt +@@ + first +-second ++second updated +*** End of File +*** End Patch diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index ffdff690c..a17a83a0f 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -503,7 +503,12 @@ async fn cli_main(codex_linux_sandbox_exe: Option) -> anyhow::Result<() } Some(Subcommand::AppServer(app_server_cli)) => match app_server_cli.subcommand { None => { - codex_app_server::run_main(codex_linux_sandbox_exe, root_config_overrides).await?; + codex_app_server::run_main( + codex_linux_sandbox_exe, + root_config_overrides, + codex_core::config_loader::LoaderOverrides::default(), + ) + .await?; } Some(AppServerSubcommand::GenerateTs(gen_cli)) => { codex_app_server_protocol::generate_ts( diff --git a/codex-rs/core/src/agent/control.rs b/codex-rs/core/src/agent/control.rs new file mode 100644 index 000000000..468f580a2 --- /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 000000000..d6348b38b --- /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 000000000..f5345a291 --- /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 822a5b28b..7bea33040 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 64036195c..c721bbd05 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; @@ -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={:?}", @@ -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,6 +2086,46 @@ 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 @@ -2132,6 +2223,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, @@ -2959,6 +3051,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"); @@ -3192,6 +3409,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, @@ -3236,7 +3491,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(), @@ -3282,6 +3539,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( @@ -3298,6 +3556,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), @@ -3323,7 +3582,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(), @@ -3369,6 +3630,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( @@ -3385,6 +3647,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), diff --git a/codex-rs/core/src/codex_conversation.rs b/codex-rs/core/src/codex_conversation.rs index 5bb9c97c5..723a4cf4f 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 e767abf4a..d7458f1ac 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 8eac13fd2..035794f4a 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 900706490..e1c45c1f1 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/service.rs b/codex-rs/core/src/config/service.rs index 211a12fa0..d65ed10a0 100644 --- a/codex-rs/core/src/config/service.rs +++ b/codex-rs/core/src/config/service.rs @@ -106,16 +106,7 @@ pub struct ConfigService { } impl ConfigService { - pub fn new(codex_home: PathBuf, cli_overrides: Vec<(String, TomlValue)>) -> Self { - Self { - codex_home, - cli_overrides, - loader_overrides: LoaderOverrides::default(), - } - } - - #[cfg(test)] - fn with_overrides( + pub fn new( codex_home: PathBuf, cli_overrides: Vec<(String, TomlValue)>, loader_overrides: LoaderOverrides, @@ -127,6 +118,14 @@ impl ConfigService { } } + pub fn new_with_defaults(codex_home: PathBuf) -> Self { + Self { + codex_home, + cli_overrides: Vec::new(), + loader_overrides: LoaderOverrides::default(), + } + } + pub async fn read( &self, params: ConfigReadParams, @@ -707,7 +706,7 @@ unified_exec = true "#; std::fs::write(tmp.path().join(CONFIG_TOML_FILE), original)?; - let service = ConfigService::new(tmp.path().to_path_buf(), vec![]); + let service = ConfigService::new_with_defaults(tmp.path().to_path_buf()); service .write_value(ConfigValueWriteParams { file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), @@ -748,7 +747,7 @@ remote_compaction = true std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); let managed_file = AbsolutePathBuf::try_from(managed_path.clone()).expect("managed file"); - let service = ConfigService::with_overrides( + let service = ConfigService::new( tmp.path().to_path_buf(), vec![], LoaderOverrides { @@ -829,7 +828,7 @@ remote_compaction = true std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); let managed_file = AbsolutePathBuf::try_from(managed_path.clone()).expect("managed file"); - let service = ConfigService::with_overrides( + let service = ConfigService::new( tmp.path().to_path_buf(), vec![], LoaderOverrides { @@ -881,7 +880,7 @@ remote_compaction = true let user_path = tmp.path().join(CONFIG_TOML_FILE); std::fs::write(&user_path, "model = \"user\"").unwrap(); - let service = ConfigService::new(tmp.path().to_path_buf(), vec![]); + let service = ConfigService::new_with_defaults(tmp.path().to_path_buf()); let error = service .write_value(ConfigValueWriteParams { file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), @@ -904,7 +903,7 @@ remote_compaction = true let tmp = tempdir().expect("tempdir"); std::fs::write(tmp.path().join(CONFIG_TOML_FILE), "").unwrap(); - let service = ConfigService::new(tmp.path().to_path_buf(), vec![]); + let service = ConfigService::new_with_defaults(tmp.path().to_path_buf()); service .write_value(ConfigValueWriteParams { file_path: None, @@ -932,7 +931,7 @@ remote_compaction = true let managed_path = tmp.path().join("managed_config.toml"); std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); - let service = ConfigService::with_overrides( + let service = ConfigService::new( tmp.path().to_path_buf(), vec![], LoaderOverrides { @@ -980,7 +979,7 @@ remote_compaction = true TomlValue::String("session".to_string()), )]; - let service = ConfigService::with_overrides( + let service = ConfigService::new( tmp.path().to_path_buf(), cli_overrides, LoaderOverrides { @@ -1026,7 +1025,7 @@ remote_compaction = true std::fs::write(&managed_path, "approval_policy = \"never\"").unwrap(); let managed_file = AbsolutePathBuf::try_from(managed_path.clone()).expect("managed file"); - let service = ConfigService::with_overrides( + let service = ConfigService::new( tmp.path().to_path_buf(), vec![], LoaderOverrides { @@ -1085,7 +1084,7 @@ alpha = "a" std::fs::write(&path, base)?; - let service = ConfigService::new(tmp.path().to_path_buf(), vec![]); + let service = ConfigService::new_with_defaults(tmp.path().to_path_buf()); service .write_value(ConfigValueWriteParams { file_path: Some(path.display().to_string()), diff --git a/codex-rs/core/src/config/types.rs b/codex-rs/core/src/config/types.rs index 49a2bc804..c90d38e74 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/config_loader/layer_io.rs b/codex-rs/core/src/config_loader/layer_io.rs index 84a29a611..0ece69b47 100644 --- a/codex-rs/core/src/config_loader/layer_io.rs +++ b/codex-rs/core/src/config_loader/layer_io.rs @@ -93,12 +93,8 @@ pub(super) async fn read_config_from_path( } } -/// Return the default managed config path (honoring `CODEX_MANAGED_CONFIG_PATH`). +/// Return the default managed config path. pub(super) fn managed_config_default_path(codex_home: &Path) -> PathBuf { - if let Ok(path) = std::env::var("CODEX_MANAGED_CONFIG_PATH") { - return PathBuf::from(path); - } - #[cfg(unix)] { let _ = codex_home; diff --git a/codex-rs/core/src/context_manager/history.rs b/codex-rs/core/src/context_manager/history.rs index c18ad7df8..e52561c9f 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 d121b7dc6..1fb47e1e6 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 5093e03c6..cf0f0106d 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,100 +94,56 @@ 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 async fn new_conversation(&self, config: Config) -> CodexResult { - self.spawn_conversation( - config, - self.auth_manager.clone(), - self.models_manager.clone(), - ) - .await - } - - async fn spawn_conversation( - &self, - config: Config, - auth_manager: Arc, - models_manager: Arc, - ) -> CodexResult { - let CodexSpawnOk { - codex, - conversation_id, - } = Codex::spawn( - config, - auth_manager, - models_manager, - self.skills_manager.clone(), - InitialHistory::New, - self.session_source.clone(), - ) - .await?; - self.finalize_spawn(codex, conversation_id).await + pub fn get_models_manager(&self) -> Arc { + self.state.models_manager.clone() } - async fn finalize_spawn( - &self, - 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 { - id, - msg: EventMsg::SessionConfigured(session_configured), - } if id == INITIAL_SUBMIT_ID => session_configured, - _ => { - return Err(CodexErr::SessionConfiguredNotFirstEvent); - } - }; - - let conversation = Arc::new(CodexConversation::new( - codex, - session_configured.rollout_path.clone(), - )); - self.conversations - .write() - .await - .insert(conversation_id, conversation.clone()); - - Ok(NewConversation { - conversation_id, - conversation, - session_configured, - }) + 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> { - let conversations = self.conversations.read().await; - conversations - .get(&conversation_id) - .cloned() - .ok_or_else(|| CodexErr::ConversationNotFound(conversation_id)) + 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( @@ -196,100 +163,151 @@ impl ConversationManager { 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 + 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. + /// 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) + 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. + /// 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); + self.state + .spawn_conversation( + config, + history, + Arc::clone(&self.state.auth_manager), + self.agent_control(), + ) + .await + } - // Spawn a new conversation with the computed initial history. - let auth_manager = self.auth_manager.clone(); + 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, + InitialHistory::New, + Arc::clone(&self.auth_manager), + agent_control, + ) + .await + } + + pub(crate) async fn spawn_conversation( + &self, + config: Config, + initial_history: InitialHistory, + auth_manager: Arc, + agent_control: AgentControl, + ) -> CodexResult { let CodexSpawnOk { codex, conversation_id, } = Codex::spawn( config, auth_manager, - self.models_manager.clone(), - self.skills_manager.clone(), - history, + 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 } - pub async fn list_models(&self, config: &Config) -> Vec { - self.models_manager.list_models(config).await - } + async fn finalize_spawn( + &self, + codex: Codex, + conversation_id: ConversationId, + ) -> CodexResult { + let event = codex.next_event().await?; + let session_configured = match event { + Event { + id, + msg: EventMsg::SessionConfigured(session_configured), + } if id == INITIAL_SUBMIT_ID => session_configured, + _ => { + return Err(CodexErr::SessionConfiguredNotFirstEvent); + } + }; - pub fn get_models_manager(&self) -> Arc { - self.models_manager.clone() + let conversation = Arc::new(CodexConversation::new( + codex, + session_configured.rollout_path.clone(), + )); + self.conversations + .write() + .await + .insert(conversation_id, conversation.clone()); + + Ok(NewConversation { + conversation_id, + conversation, + session_configured, + }) } } /// 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 3b22bfc3f..99b0522a1 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, diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 238646ed8..8f76dce63 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 060f4a5c2..29be88735 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 0a7e78578..080c44433 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 647dcf1d5..9920bb1dc 100644 --- a/codex-rs/core/src/rollout/mod.rs +++ b/codex-rs/core/src/rollout/mod.rs @@ -14,6 +14,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 07c8af114..6c02ad094 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 000000000..b8127f034 --- /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 63b66647e..8520dbb25 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -2,6 +2,7 @@ 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; @@ -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/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 0fcfd3270..71daf4968 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -1,7 +1,6 @@ use crate::function_tool::FunctionCallError; use crate::is_safe_command::is_known_safe_command; use crate::protocol::EventMsg; -use crate::protocol::ExecCommandSource; use crate::protocol::TerminalInteractionEvent; use crate::sandboxing::SandboxPermissions; use crate::shell::Shell; @@ -9,9 +8,6 @@ use crate::shell::get_shell_by_model_provided_path; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; -use crate::tools::events::ToolEmitter; -use crate::tools::events::ToolEventCtx; -use crate::tools::events::ToolEventStage; use crate::tools::handlers::apply_patch::intercept_apply_patch; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; @@ -172,20 +168,6 @@ impl ToolHandler for UnifiedExecHandler { return Ok(output); } - let event_ctx = ToolEventCtx::new( - context.session.as_ref(), - context.turn.as_ref(), - &context.call_id, - None, - ); - let emitter = ToolEmitter::unified_exec( - &command, - cwd.clone(), - ExecCommandSource::UnifiedExecStartup, - Some(process_id.clone()), - ); - emitter.emit(event_ctx, ToolEventStage::Begin).await; - manager .exec_command( ExecCommandRequest { diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index a8543cd3f..51eb07b83 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(), } @@ -1113,8 +1116,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 { @@ -1157,7 +1167,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, } } @@ -1235,7 +1245,7 @@ mod tests { ToolSpec::Function(ResponsesApiTool { parameters, .. }) => { strip_descriptions_schema(parameters); } - ToolSpec::Freeform(_) | ToolSpec::LocalShell {} | ToolSpec::WebSearch {} => {} + ToolSpec::Freeform(_) | ToolSpec::LocalShell {} | ToolSpec::WebSearch { .. } => {} } } @@ -1279,7 +1289,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); @@ -1312,6 +1324,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 53b6b7c77..22aca62aa 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::session::UnifiedExecSession; 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); @@ -40,7 +39,7 @@ const UNIFIED_EXEC_OUTPUT_DELTA_MAX_BYTES: usize = 8192; pub(crate) fn start_streaming_output( session: &UnifiedExecSession, context: &UnifiedExecContext, - transcript: Arc>, + transcript: Arc>, ) { let mut receiver = session.output_receiver(); let output_drained = session.output_drained_notify(); @@ -112,7 +111,7 @@ pub(crate) fn spawn_exit_watcher( command: Vec, cwd: PathBuf, process_id: String, - transcript: Arc>, + transcript: Arc>, started_at: Instant, ) { let exit_token = session.cancellation_token(); @@ -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/head_tail_buffer.rs b/codex-rs/core/src/unified_exec/head_tail_buffer.rs new file mode 100644 index 000000000..852446604 --- /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 2cb30e5aa..c88238fe5 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -37,6 +37,7 @@ use crate::sandboxing::SandboxPermissions; mod async_watcher; mod errors; +mod head_tail_buffer; mod session; mod session_manager; @@ -53,24 +54,6 @@ pub(crate) const MAX_UNIFIED_EXEC_SESSIONS: 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); - } - } -} - pub(crate) struct UnifiedExecContext { pub session: Arc, pub turn: Arc, @@ -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)] diff --git a/codex-rs/core/src/unified_exec/session.rs b/codex-rs/core/src/unified_exec/session.rs index 4973a1a64..0e08c5990 100644 --- a/codex-rs/core/src/unified_exec/session.rs +++ b/codex-rs/core/src/unified_exec/session.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, @@ -90,7 +46,7 @@ impl UnifiedExecSession { 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()); @@ -163,7 +119,7 @@ impl UnifiedExecSession { 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 { diff --git a/codex-rs/core/src/unified_exec/session_manager.rs b/codex-rs/core/src/unified_exec/session_manager.rs index 867479d21..1d524c23d 100644 --- a/codex-rs/core/src/unified_exec/session_manager.rs +++ b/codex-rs/core/src/unified_exec/session_manager.rs @@ -16,8 +16,12 @@ use crate::codex::TurnContext; use crate::exec_env::create_env; use crate::protocol::BackgroundEventEvent; use crate::protocol::EventMsg; +use crate::protocol::ExecCommandSource; use crate::sandboxing::ExecEnv; use crate::sandboxing::SandboxPermissions; +use crate::tools::events::ToolEmitter; +use crate::tools::events::ToolEventCtx; +use crate::tools::events::ToolEventStage; use crate::tools::orchestrator::ToolOrchestrator; use crate::tools::runtimes::unified_exec::UnifiedExecRequest as UnifiedExecToolRequest; use crate::tools::runtimes::unified_exec::UnifiedExecRuntime; @@ -25,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_SESSIONS; +use crate::unified_exec::SessionEntry; +use crate::unified_exec::SessionStore; +use crate::unified_exec::UnifiedExecContext; +use crate::unified_exec::UnifiedExecError; +use crate::unified_exec::UnifiedExecResponse; +use crate::unified_exec::UnifiedExecSessionManager; +use crate::unified_exec::WARNING_UNIFIED_EXEC_SESSIONS; +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::resolve_max_tokens; +use crate::unified_exec::session::OutputBuffer; +use crate::unified_exec::session::OutputHandles; +use crate::unified_exec::session::UnifiedExecSession; const UNIFIED_EXEC_ENV: [(&str, &str); 9] = [ ("NO_COLOR", "1"), @@ -140,14 +143,22 @@ impl UnifiedExecSessionManager { } }; - // Do not stream ExecCommandOutputDelta events during the initial poll. - // - // The initial exec_command call has a `yield_time_ms` poll window. If - // the command exits within that window (early-exit), we should emit - // only ExecCommandBegin/ExecCommandEnd, not output deltas. This keeps - // event semantics deterministic: we only start streaming deltas once we - // know the session will outlive the initial poll. - 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(), + &context.call_id, + None, + ); + let emitter = ToolEmitter::unified_exec( + &request.command, + cwd.clone(), + ExecCommandSource::UnifiedExecStartup, + Some(request.process_id.clone()), + ); + emitter.emit(event_ctx, ToolEventStage::Begin).await; + + start_streaming_output(&session, 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,11 +182,6 @@ impl UnifiedExecSessionManager { .await; let wall_time = Instant::now().saturating_duration_since(start); - { - let mut guard = transcript.lock().await; - guard.append(&collected); - } - 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(); @@ -204,7 +210,6 @@ impl UnifiedExecSessionManager { self.release_process_id(&request.process_id).await; session.check_for_sandbox_denial_with_text(&text).await?; } else { - start_streaming_output(&session, context, Arc::clone(&transcript)); // Long‑lived command: persist the session so write_stdin can reuse // it, and register a background watcher that will emit // ExecCommandEnd when the PTY eventually exits (even if no further @@ -403,7 +408,7 @@ impl UnifiedExecSessionManager { cwd: PathBuf, started_at: Instant, process_id: String, - transcript: Arc>, + transcript: Arc>, ) { let entry = SessionEntry { session: Arc::clone(&session), @@ -545,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()); } diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index a081a6bf1..7ae3ea159 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 565b978fa..30593a8f6 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 63784bd40..e5b809ca3 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 201d61c9e..94641f1a4 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/web_search_cached.rs b/codex-rs/core/tests/suite/web_search_cached.rs new file mode 100644 index 000000000..fa8e303d8 --- /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/deny.toml b/codex-rs/deny.toml index f9d411f84..8ca3d92f9 100644 --- a/codex-rs/deny.toml +++ b/codex-rs/deny.toml @@ -73,6 +73,7 @@ ignore = [ { id = "RUSTSEC-2024-0388", reason = "derivative is unmaintained; pulled in via starlark v0.13.0 used by execpolicy/cli/core; no fixed release yet" }, { id = "RUSTSEC-2025-0057", reason = "fxhash is unmaintained; pulled in via starlark_map/starlark v0.13.0 used by execpolicy/cli/core; no fixed release yet" }, { id = "RUSTSEC-2024-0436", reason = "paste is unmaintained; pulled in via ratatui/rmcp/starlark used by tui/execpolicy; no fixed release yet" }, + { id = "RUSTSEC-2026-0002", reason = "lru <0.16.3 is marked unsound; ratatui 0.29.x currently depends on lru 0.12.x (no compatible upgrade path yet)" }, ] # If this is true, then cargo deny will use the git executable to fetch advisory database. # If this is false, then it uses a built-in git library. diff --git a/codex-rs/exec-server/tests/common/lib.rs b/codex-rs/exec-server/tests/common/lib.rs index a5a6a3cac..0679ad55d 100644 --- a/codex-rs/exec-server/tests/common/lib.rs +++ b/codex-rs/exec-server/tests/common/lib.rs @@ -25,15 +25,13 @@ use std::sync::Arc; use std::sync::Mutex; use tokio::process::Command; -pub async fn create_transport

( - codex_home: P, - dotslash_cache: P, -) -> anyhow::Result +pub async fn create_transport

(codex_home: P) -> anyhow::Result where P: AsRef, { let mcp_executable = codex_utils_cargo_bin::cargo_bin("codex-exec-mcp-server")?; let execve_wrapper = codex_utils_cargo_bin::cargo_bin("codex-execve-wrapper")?; + // `bash` requires a special lookup when running under Buck because it is a // _resource_ rather than a binary target. let bash = if let Some(root) = codex_utils_cargo_bin::buck_project_root()? { @@ -47,11 +45,14 @@ where // Need to ensure the artifact associated with the bash DotSlash file is // available before it is run in a read-only sandbox. + // + // Note we intentionally *do not* set `DOTSLASH_CACHE`: tests should use the + // user's configured cache (or DotSlash default) so that `cargo nextest` + // doesn't re-download the tarball for each test case. let status = Command::new("dotslash") .arg("--") .arg("fetch") .arg(bash.clone()) - .env("DOTSLASH_CACHE", dotslash_cache.as_ref()) .status() .await?; assert!(status.success(), "dotslash fetch failed: {status:?}"); @@ -60,7 +61,6 @@ where cmd.arg("--bash").arg(bash); cmd.arg("--execve").arg(&execve_wrapper); cmd.env("CODEX_HOME", codex_home.as_ref()); - cmd.env("DOTSLASH_CACHE", dotslash_cache.as_ref()); // Important: pipe stdio so rmcp can speak JSON-RPC over stdin/stdout cmd.stdin(Stdio::piped()); diff --git a/codex-rs/exec-server/tests/suite/accept_elicitation.rs b/codex-rs/exec-server/tests/suite/accept_elicitation.rs index 1e0ea32e3..de89b1332 100644 --- a/codex-rs/exec-server/tests/suite/accept_elicitation.rs +++ b/codex-rs/exec-server/tests/suite/accept_elicitation.rs @@ -44,14 +44,12 @@ prefix_rule( match = [ "git init ." ], -) + ) "#, codex_home.as_ref(), ) .await?; - let dotslash_cache_temp_dir = TempDir::new()?; - let dotslash_cache = dotslash_cache_temp_dir.path(); - let transport = create_transport(codex_home.as_ref(), dotslash_cache).await?; + let transport = create_transport(codex_home.as_ref()).await?; // Create an MCP client that approves expected elicitation messages. let project_root = TempDir::new()?; diff --git a/codex-rs/exec-server/tests/suite/list_tools.rs b/codex-rs/exec-server/tests/suite/list_tools.rs index 4a28dec5b..0d7e20db2 100644 --- a/codex-rs/exec-server/tests/suite/list_tools.rs +++ b/codex-rs/exec-server/tests/suite/list_tools.rs @@ -22,9 +22,7 @@ async fn list_tools() -> Result<()> { policy_dir.join("default.rules"), r#"prefix_rule(pattern=["ls"], decision="prompt")"#, )?; - let dotslash_cache_temp_dir = TempDir::new()?; - let dotslash_cache = dotslash_cache_temp_dir.path(); - let transport = create_transport(codex_home.path(), dotslash_cache).await?; + let transport = create_transport(codex_home.path()).await?; let service = ().serve(transport).await?; let tools = service.list_tools(Default::default()).await?.tools; 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 40afab7c9..ba6d99c5a 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 16d38da45..26fdc0732 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.", - "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.", + "sandbox": { + "description": "Sandbox mode: `read-only`, `workspace-write`, or `danger-full-access`.", + "enum": [ + "read-only", + "workspace-write", + "danger-full-access" + ], "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 b50d0c764..629f4cc49 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 bd6ef0614..02f20fc2b 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 e0af51e7e..d76a5b403 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 d42861eb1..0923d41b5 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 d6d01045b..0d5fe4406 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -1650,6 +1650,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(); @@ -4109,6 +4119,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/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 140ff0bbc..25d29e92b 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -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_sessions.clear(); + self.sync_unified_exec_footer(); if reason != TurnAbortReason::ReviewEnded { self.add_to_history(history_cell::new_error_event( @@ -2113,6 +2115,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(_) @@ -2308,7 +2311,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() } @@ -2424,6 +2427,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 000000000..3c256fe92 --- /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 000000000..56dff7b5f --- /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 a0ff8d42e..f7af95b32 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; @@ -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_sessions() { + 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_sessions.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_sessions.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/diff_render.rs b/codex-rs/tui/src/diff_render.rs index ff0fb509b..505ebc8ed 100644 --- a/codex-rs/tui/src/diff_render.rs +++ b/codex-rs/tui/src/diff_render.rs @@ -299,7 +299,18 @@ fn render_change(change: &FileChange, out: &mut Vec>, width: usi } } +/// Format a path for display relative to the current working directory when +/// possible, keeping output stable in jj/no-`.git` workspaces (e.g. image +/// tool calls should show `example.png` instead of an absolute path). pub(crate) fn display_path_for(path: &Path, cwd: &Path) -> String { + if path.is_relative() { + return path.display().to_string(); + } + + if let Ok(stripped) = path.strip_prefix(cwd) { + return stripped.display().to_string(); + } + let path_in_same_repo = match (get_git_repo_root(cwd), get_git_repo_root(path)) { (Some(cwd_repo), Some(path_repo)) => cwd_repo == path_repo, _ => false, @@ -420,6 +431,7 @@ fn style_del() -> Style { mod tests { use super::*; use insta::assert_snapshot; + use pretty_assertions::assert_eq; use ratatui::Terminal; use ratatui::backend::TestBackend; use ratatui::text::Text; @@ -459,6 +471,26 @@ mod tests { assert_snapshot!(name, text); } + #[test] + fn display_path_prefers_cwd_without_git_repo() { + let cwd = if cfg!(windows) { + PathBuf::from(r"C:\workspace\codex") + } else { + PathBuf::from("/workspace/codex") + }; + let path = cwd.join("tui").join("example.png"); + + let rendered = display_path_for(&path, &cwd); + + assert_eq!( + rendered, + PathBuf::from("tui") + .join("example.png") + .display() + .to_string() + ); + } + #[test] fn ui_snapshot_wrap_behavior_insert() { // Narrow width to force wrapping within our diff line rendering diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 8bfd9493e..c9c801e14 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -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 } } } diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index ef76cc091..ccdb19eff 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -100,7 +100,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( @@ -357,6 +356,8 @@ async fn run_ratatui_app( ) -> color_eyre::Result { install_color_eyre_once()?; + 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 000000000..9016aebea --- /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 8a3a1c1e9..402a18ba2 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/docs/streaming_wrapping_design.md b/codex-rs/tui2/docs/streaming_wrapping_design.md index 28937c377..f7af4cccc 100644 --- a/codex-rs/tui2/docs/streaming_wrapping_design.md +++ b/codex-rs/tui2/docs/streaming_wrapping_design.md @@ -1,85 +1,169 @@ -# Streaming Markdown Wrapping & Animation – TUI2 Notes - -This document mirrors the original `tui/streaming_wrapping_design.md` and -captures how the same concerns apply to the new `tui2` crate. It exists so that -future viewport and streaming work in TUI2 can rely on the same context without -having to cross‑reference the legacy TUI implementation. - -At a high level, the design constraints are the same: - -- Streaming agent responses are rendered incrementally, with an animation loop - that reveals content over time. -- Non‑streaming history cells are rendered width‑agnostically and wrapped only - at display time, so they reflow correctly when the terminal is resized. -- Streaming content should eventually follow the same “wrap on display” model so - the transcript reflows consistently across width changes, without regressing - animation or markdown semantics. - -## 1. Where streaming is implemented in TUI2 - -TUI2 keeps the streaming pipeline conceptually aligned with the legacy TUI but -in a separate crate: - -- `tui2/src/markdown_stream.rs` implements the markdown streaming collector and - animation controller for agent deltas. -- `tui2/src/chatwidget.rs` integrates streamed content into the transcript via - `HistoryCell` implementations. -- `tui2/src/history_cell.rs` provides the concrete history cell types used by - the inline transcript and overlays. -- `tui2/src/wrapping.rs` contains the shared text wrapping utilities used by - both streaming and non‑streaming render paths: - - `RtOptions` describes viewport‑aware wrapping (width, indents, algorithm). - - `word_wrap_line`, `word_wrap_lines`, and `word_wrap_lines_borrowed` provide - span‑aware wrapping that preserves markdown styling and emoji width. - -As in the original TUI, the key tension is between: - -- **Pre‑wrapping streamed content at commit time** (simpler animation, but - baked‑in splits that don’t reflow), and -- **Deferring wrapping to render time** (better reflow, but requires a more - sophisticated streaming cell model or recomputation on each frame). - -## 2. Current behavior and limitations - -TUI2 is intentionally conservative for now: - -- Streaming responses use the same markdown streaming and wrapping utilities as - the legacy TUI, with width decisions made near the streaming collector. -- The transcript viewport (`App::render_transcript_cells` in - `tui2/src/app.rs`) always uses `word_wrap_lines_borrowed` against the - current `Rect` width, so: - - Non‑streaming cells reflow naturally on resize. - - Streamed cells respect whatever wrapping was applied when their lines were - constructed, and may not fully “un‑wrap” if that work happened at a fixed - width earlier in the pipeline. - -This means TUI2 shares the same fundamental limitation documented in the -original design note: streamed paragraphs can retain historical wrap decisions -made at the time they were streamed, even if the viewport later grows wider. - -## 3. Design directions (forward‑looking) - -The options outlined in the legacy document apply here as well: - -1. **Keep the current behavior but clarify tests and documentation.** - - Ensure tests in `tui2/src/markdown_stream.rs`, `tui2/src/markdown_render.rs`, - `tui2/src/history_cell.rs`, and `tui2/src/wrapping.rs` encode the current - expectations around streaming, wrapping, and emoji / markdown styling. -2. **Move towards width‑agnostic streaming cells.** - - Introduce a dedicated streaming history cell that stores the raw markdown - buffer and lets `HistoryCell::display_lines(width)` perform both markdown - rendering and wrapping based on the current viewport width. - - Keep the commit animation logic expressed in terms of “logical” positions - (e.g., number of tokens or lines committed) rather than pre‑wrapped visual - lines at a fixed width. -3. **Hybrid “visual line count” model.** - - Track committed visual lines as a scalar and re‑render the streamed prefix - at the current width, revealing only the first `N` visual lines on each - animation tick. - -TUI2 does not yet implement these refactors; it intentionally stays close to -the legacy behavior while the viewport work (scrolling, selection, exit -transcripts) is being ported. This document exists to make that trade‑off -explicit for TUI2 and to provide a natural home for any TUI2‑specific streaming -wrapping notes as the design evolves. +# Streaming Wrapping Reflow (tui2) +This document describes a correctness bug in `codex-rs/tui2` and the chosen fix: +while streaming assistant markdown, soft-wrap decisions were effectively persisted as hard line +breaks, so resizing the viewport could not reflow prose. + +## Goal + +- Resizing the viewport reflows transcript prose (including streaming assistant output). +- Width-derived breaks are always treated as *soft wraps* (not logical newlines). +- Copy/paste continues to treat soft wraps as joinable (via joiners), and hard breaks as newlines. + +Non-goals: + +- Reflowing terminal scrollback that has already been printed. +- Reflowing content that is intentionally treated as preformatted (e.g., code blocks, raw stdout). + +## Background: where reflow happens in tui2 + +TUI2 renders the transcript as a list of `HistoryCell`s: + +1. A cell stores width-agnostic content (string, diff, logical lines, etc.). +2. At draw time (and on resize), `transcript_render` asks each cell for lines at the *current* + width (ideally via `HistoryCell::transcript_lines_with_joiners(width)`). +3. `TranscriptViewCache` caches the wrapped visual lines keyed by width; a width change triggers a + rebuild. + +This only works if cells do *not* persist width-derived wrapping inside their stored state. + +## The bug: soft wraps became hard breaks during streaming + +Ratatui represents multi-line content as `Vec`. If we split a paragraph into multiple `Line`s +because the viewport is narrow, that split is indistinguishable from an explicit newline unless we +also carry metadata describing which breaks were “soft”. + +Streaming assistant output used to generate already-wrapped `Line`s and store them inside the +history cell. Later, when the viewport became wider, the transcript renderer could not “un-split” +those baked lines — they looked like hard breaks. + +## Chosen solution (A, F1): stream logical markdown lines; wrap in the cell at render-time + +User choice recap: + +- **A**: Keep append-only streaming (new history cell per commit tick), but make the streamed data + width-agnostic. +- **F1**: Make the agent message cell responsible for wrapping-to-width so transcript-level wrapping + can be a no-op for it. + +### Key idea: separate markdown parsing from wrapping + +We introduce a width-agnostic “logical markdown line” representation that preserves the metadata +needed to wrap correctly later: + +- `codex-rs/tui2/src/markdown_render.rs` + - `MarkdownLogicalLine { content, initial_indent, subsequent_indent, line_style, is_preformatted }` + - `render_markdown_logical_lines(input: &str) -> Vec` + +This keeps: + +- hard breaks (paragraph/list boundaries, explicit newlines), +- markdown indentation rules for wraps (list markers, nested lists, blockquotes), +- preformatted runs (code blocks) stable. + +### Updated streaming pipeline + +- `codex-rs/tui2/src/markdown_stream.rs` + - `MarkdownStreamCollector` is newline-gated (no change), but now commits + `Vec` instead of already-wrapped `Vec`. + - Width is removed from the collector; wrapping is not performed during streaming. + +- `codex-rs/tui2/src/streaming/controller.rs` + - Emits `AgentMessageCell::new_logical(...)` containing logical lines. + +- `codex-rs/tui2/src/history_cell.rs` + - `AgentMessageCell` stores `Vec`. + - `HistoryCell::transcript_lines_with_joiners(width)` wraps each logical line at the current + width using `word_wrap_line_with_joiners` and composes indents as: + - transcript gutter prefix (`• ` / ` `), plus + - markdown-provided initial/subsequent indents. + - Preformatted logical lines are rendered without wrapping. + +Result: on resize, the transcript cache rebuilds against the new width and the agent output reflows +correctly because the stored content contains no baked soft wraps. + +## Overlay deferral fix (D): defer cells, not rendered lines + +When an overlay (transcript/static) is active, TUI2 is in alt screen and the normal terminal buffer +is not visible. Historically, `tui2` attempted to queue “history to print” for the normal buffer by +deferring *rendered lines*, which baked the then-current width. + +User choice recap: + +- **D**: Store deferred *cells* and render them at overlay close time. + +Implementation: + +- `codex-rs/tui2/src/app.rs` + - `deferred_history_cells: Vec>` (replaces `deferred_history_lines`). + - `AppEvent::InsertHistoryCell` pushes cells into the deferral list when `overlay.is_some()`. + +- `codex-rs/tui2/src/app_backtrack.rs` + - `close_transcript_overlay` renders deferred cells at the *current* width when closing the + overlay, then queues the resulting lines for the normal terminal buffer. + +Note: as of today, `Tui::insert_history_lines` queues lines but `Tui::draw` does not flush them into +the terminal (see `codex-rs/tui2/src/tui.rs`). This section is therefore best read as “behavior we +want when/if scrollback printing is re-enabled”, not a guarantee that content is printed during the +main TUI loop. For the current intended behavior around printing, see +`codex-rs/tui2/docs/tui_viewport_and_history.md`. + +## Tests (G2) + +User choice recap: + +- **G2**: Add resize reflow tests + snapshot coverage. + +Added coverage: + +- `codex-rs/tui2/src/history_cell.rs` + - `agent_message_cell_reflows_streamed_prose_on_resize` + - `agent_message_cell_reflows_streamed_prose_vt100_snapshot` + +These assert that a streamed agent cell produces fewer visual lines at wider widths and provide +snapshots showing reflow for list items and blockquotes. + +## Audit: other `HistoryCell`s and width-baked paths + +This section answers “what else might behave like this?” up front. + +### History cells + +- `AgentMessageCell` (`codex-rs/tui2/src/history_cell.rs`): **was affected**; now stores logical + markdown lines and wraps at render time. +- `UserHistoryCell` (`codex-rs/tui2/src/history_cell.rs`): wraps at render time from stored `String` + using `word_wrap_lines_with_joiners` (reflowable). +- `ReasoningSummaryCell` (`codex-rs/tui2/src/history_cell.rs`): renders from stored `String` on each + call; it does call `append_markdown(..., Some(width))`, but that wrapping is recomputed per width + (reflowable). +- `PrefixedWrappedHistoryCell` (`codex-rs/tui2/src/history_cell.rs`): wraps at render time and + returns joiners (reflowable). +- `PlainHistoryCell` (`codex-rs/tui2/src/history_cell.rs`): stores `Vec` and returns it + unchanged (not reflowable by design; used for already-structured/preformatted output). + +Rule of thumb: any cell that stores already-wrapped `Vec` for prose is a candidate for the +same bug; cells that store source text or logical lines and compute wrapping inside +`display_lines(width)` are safe. + +### Width-baked output outside the transcript model + +Even with the streaming fix, some paths are inherently width-baked: + +- Printed transcript after exit (`codex-rs/tui2/src/app.rs`): `AppExitInfo.session_lines` is rendered + once using the final width and then printed; it cannot reflow afterward. +- Optional scrollback insertion helper (`codex-rs/tui2/src/insert_history.rs`): once ANSI is written + to the terminal, that output cannot be reflowed later. This helper is currently used for + deterministic ANSI emission (`write_spans`) and tests; it is not wired into the main TUI draw + loop. +- Static overlays (`codex-rs/tui2/src/pager_overlay.rs`): reflow depends on whether callers provided + width-agnostic input; pre-split `Vec` cannot be “un-split” within the overlay. + +## Deferred / follow-ups + +The fix above is sufficient to unblock correct reflow on resize. Remaining choices can be deferred: + +- Streaming granularity: one logical line can wrap into multiple visual lines, so “commit tick” + updates can appear in larger chunks than before. If this becomes a UX issue, we can add a render- + time “progressive reveal” layer without reintroducing width baking. +- Expand logical-line rendering to other markdown-ish cells if needed (e.g., unify `append_markdown` + usage), but only if we find a concrete reflow bug beyond `AgentMessageCell`. diff --git a/codex-rs/tui2/src/app.rs b/codex-rs/tui2/src/app.rs index 677d73d71..49bb005a7 100644 --- a/codex-rs/tui2/src/app.rs +++ b/codex-rs/tui2/src/app.rs @@ -344,10 +344,22 @@ pub(crate) struct App { transcript_copy_action: TranscriptCopyAction, transcript_scrollbar_ui: TranscriptScrollbarUi, - // Pager overlay state (Transcript or Static like Diff) + // Pager overlay state (Transcript or Static like Diff). pub(crate) overlay: Option, - pub(crate) deferred_history_lines: Vec>, - has_emitted_history_lines: bool, + /// History cells received while an overlay is active. + /// + /// While in an alt-screen overlay, the normal terminal buffer is not visible. + /// Instead we queue the incoming cells here and, on overlay close, render them at the *current* + /// width and queue them in one batch via `Tui::insert_history_lines`. + /// + /// This matters for correctness if/when scrollback printing is enabled: if we deferred + /// already-rendered `Vec`, we'd bake viewport-width wrapping based on the width at the + /// time the cell arrived (which may differ from the width when the overlay closes). + pub(crate) deferred_history_cells: Vec>, + /// True once at least one history cell has been inserted into terminal scrollback. + /// + /// Used to decide whether to insert an extra blank separator line when flushing deferred cells. + pub(crate) has_emitted_history_lines: bool, pub(crate) enhanced_keys_supported: bool, @@ -511,7 +523,7 @@ impl App { transcript_copy_action: TranscriptCopyAction::default(), transcript_scrollbar_ui: TranscriptScrollbarUi::default(), overlay: None, - deferred_history_lines: Vec::new(), + deferred_history_cells: Vec::new(), has_emitted_history_lines: false, commit_anim_running: Arc::new(AtomicBool::new(false)), scroll_config, @@ -1449,21 +1461,8 @@ impl App { tui.frame_requester().schedule_frame(); } self.transcript_cells.push(cell.clone()); - let mut display = cell.display_lines(tui.terminal.last_known_screen_size.width); - if !display.is_empty() { - // Only insert a separating blank line for new cells that are not - // part of an ongoing stream. Streaming continuations should not - // accrue extra blank lines between chunks. - if !cell.is_stream_continuation() { - if self.has_emitted_history_lines { - display.insert(0, Line::from("")); - } else { - self.has_emitted_history_lines = true; - } - } - if self.overlay.is_some() { - self.deferred_history_lines.extend(display); - } + if self.overlay.is_some() { + self.deferred_history_cells.push(cell); } } AppEvent::StartCommitAnimation => { @@ -2089,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; @@ -2135,7 +2137,7 @@ mod tests { transcript_copy_action: TranscriptCopyAction::default(), transcript_scrollbar_ui: TranscriptScrollbarUi::default(), overlay: None, - deferred_history_lines: Vec::new(), + deferred_history_cells: Vec::new(), has_emitted_history_lines: false, enhanced_keys_supported: false, commit_anim_running: Arc::new(AtomicBool::new(false)), @@ -2188,7 +2190,7 @@ mod tests { transcript_copy_action: TranscriptCopyAction::default(), transcript_scrollbar_ui: TranscriptScrollbarUi::default(), overlay: None, - deferred_history_lines: Vec::new(), + deferred_history_cells: Vec::new(), has_emitted_history_lines: false, enhanced_keys_supported: false, commit_anim_running: Arc::new(AtomicBool::new(false)), @@ -2209,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(); @@ -2244,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/app_backtrack.rs b/codex-rs/tui2/src/app_backtrack.rs index ce5dff2ed..01cb64fe5 100644 --- a/codex-rs/tui2/src/app_backtrack.rs +++ b/codex-rs/tui2/src/app_backtrack.rs @@ -123,12 +123,42 @@ impl App { } /// Close transcript overlay and restore normal UI. + /// + /// Any history emitted while the overlay was open is flushed to the normal-buffer queue here. + /// + /// Importantly, we defer *cells* (not rendered lines) so we can render them against the current + /// width on close and avoid baking width-derived wrapping based on an earlier viewport size. + /// (This matters if/when scrollback printing is enabled; `Tui::insert_history_lines` currently + /// queues lines without printing them during the main draw loop.) pub(crate) fn close_transcript_overlay(&mut self, tui: &mut tui::Tui) { let _ = tui.leave_alt_screen(); let was_backtrack = self.backtrack.overlay_preview_active; - if !self.deferred_history_lines.is_empty() { - let lines = std::mem::take(&mut self.deferred_history_lines); - tui.insert_history_lines(lines); + if !self.deferred_history_cells.is_empty() { + let cells = std::mem::take(&mut self.deferred_history_cells); + let width = tui.terminal.last_known_screen_size.width; + let mut lines: Vec> = Vec::new(); + for cell in cells { + let mut display = cell.display_lines(width); + if display.is_empty() { + continue; + } + + // Only insert a separating blank line for new cells that are not part of an + // ongoing stream. Streaming continuations should not accrue extra blank lines + // between chunks. + if !cell.is_stream_continuation() { + if self.has_emitted_history_lines { + display.insert(0, ratatui::text::Line::from("")); + } else { + self.has_emitted_history_lines = true; + } + } + + lines.extend(display); + } + if !lines.is_empty() { + tui.insert_history_lines(lines); + } } self.overlay = None; self.backtrack.overlay_preview_active = false; diff --git a/codex-rs/tui2/src/bottom_pane/approval_overlay.rs b/codex-rs/tui2/src/bottom_pane/approval_overlay.rs index d42861eb1..0923d41b5 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/chatwidget.rs b/codex-rs/tui2/src/chatwidget.rs index b661894a1..200d66cf3 100644 --- a/codex-rs/tui2/src/chatwidget.rs +++ b/codex-rs/tui2/src/chatwidget.rs @@ -1023,9 +1023,9 @@ impl ChatWidget { self.needs_final_message_separator = false; needs_redraw = true; } - self.stream_controller = Some(StreamController::new( - self.last_rendered_width.get().map(|w| w.saturating_sub(2)), - )); + // Streaming must not capture the current viewport width: width-derived wraps are + // applied later, at render time, so the transcript can reflow on resize. + self.stream_controller = Some(StreamController::new()); } if let Some(controller) = self.stream_controller.as_mut() && controller.push(&delta) @@ -1921,6 +1921,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 +2106,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 +2222,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 000000000..962a38ebd --- /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 000000000..a03d43490 --- /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 8b216812d..cbece2dba 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/diff_render.rs b/codex-rs/tui2/src/diff_render.rs index 1bda0e53d..b92f24349 100644 --- a/codex-rs/tui2/src/diff_render.rs +++ b/codex-rs/tui2/src/diff_render.rs @@ -299,7 +299,18 @@ fn render_change(change: &FileChange, out: &mut Vec>, width: usi } } +/// Format a path for display relative to the current working directory when +/// possible, keeping output stable in jj/no-`.git` workspaces (e.g. image +/// tool calls should show `example.png` instead of an absolute path). pub(crate) fn display_path_for(path: &Path, cwd: &Path) -> String { + if path.is_relative() { + return path.display().to_string(); + } + + if let Ok(stripped) = path.strip_prefix(cwd) { + return stripped.display().to_string(); + } + // Prefer a stable, user-local relative path when the file is under the current working // directory. This keeps output deterministic in jj-only repos (no `.git`) and matches user // expectations for "files in this project". @@ -431,6 +442,7 @@ fn style_del() -> Style { mod tests { use super::*; use insta::assert_snapshot; + use pretty_assertions::assert_eq; use ratatui::Terminal; use ratatui::backend::TestBackend; use ratatui::text::Text; @@ -470,6 +482,26 @@ mod tests { assert_snapshot!(name, text); } + #[test] + fn display_path_prefers_cwd_without_git_repo() { + let cwd = if cfg!(windows) { + PathBuf::from(r"C:\workspace\codex") + } else { + PathBuf::from("/workspace/codex") + }; + let path = cwd.join("tui").join("example.png"); + + let rendered = display_path_for(&path, &cwd); + + assert_eq!( + rendered, + PathBuf::from("tui") + .join("example.png") + .display() + .to_string() + ); + } + #[test] fn ui_snapshot_wrap_behavior_insert() { // Narrow width to force wrapping within our diff line rendering diff --git a/codex-rs/tui2/src/history_cell.rs b/codex-rs/tui2/src/history_cell.rs index 7696a3875..3124d0fc3 100644 --- a/codex-rs/tui2/src/history_cell.rs +++ b/codex-rs/tui2/src/history_cell.rs @@ -119,6 +119,19 @@ pub(crate) trait HistoryCell: std::fmt::Debug + Send + Sync + Any { /// Most cells can use the default implementation (no joiners), but cells that apply wrapping /// should override this and return joiners derived from the same wrapping operation so /// clipboard reconstruction can distinguish hard breaks from soft wraps. + /// + /// `joiner_before[i]` describes the boundary *between* `lines[i - 1]` and `lines[i]`: + /// + /// - `None` means "hard break": copy inserts a newline between the two lines. + /// - `Some(joiner)` means "soft wrap continuation": copy inserts `joiner` and continues on the + /// same logical line. + /// + /// Example (one logical line wrapped across two visual lines): + /// + /// - `lines = ["• Hello", " world"]` + /// - `joiner_before = [None, Some(\" \")]` + /// + /// Copy should produce `"Hello world"` (no hard newline). fn transcript_lines_with_joiners(&self, width: u16) -> TranscriptLinesWithJoiners { let lines = self.transcript_lines(width); TranscriptLinesWithJoiners { @@ -313,14 +326,64 @@ impl HistoryCell for ReasoningSummaryCell { #[derive(Debug)] pub(crate) struct AgentMessageCell { - lines: Vec>, + /// Width-agnostic logical markdown lines for this chunk. + /// + /// These are produced either: + /// - by streaming (`markdown_stream` → `markdown_render::render_markdown_logical_lines`), or + /// - by legacy/non-streaming callers that pass pre-rendered `Vec` via [`Self::new`]. + /// + /// Importantly, this stores *logical* lines, not already-wrapped visual lines, so the transcript + /// can reflow on resize. + logical_lines: Vec, + /// Whether this cell should render the leading transcript bullet (`• `). + /// + /// Streaming emits multiple immutable `AgentMessageCell`s per assistant message; only the first + /// chunk shows the bullet. Continuations use a two-space gutter. is_first_line: bool, } impl AgentMessageCell { + /// Construct an agent message cell from already-rendered `Line`s. + /// + /// This is primarily used by non-streaming paths. The lines are treated as already "logical" + /// lines (no additional markdown indentation metadata is available), and wrapping is still + /// performed at render time so the transcript can reflow on resize. pub(crate) fn new(lines: Vec>, is_first_line: bool) -> Self { Self { - lines, + logical_lines: lines + .into_iter() + .map(|line| { + let is_preformatted = line.style.fg == Some(ratatui::style::Color::Cyan); + let line_style = line.style; + let content = Line { + style: Style::default(), + alignment: line.alignment, + spans: line.spans, + }; + crate::markdown_render::MarkdownLogicalLine { + content, + initial_indent: Line::default(), + subsequent_indent: Line::default(), + line_style, + is_preformatted, + } + }) + .collect(), + is_first_line, + } + } + + /// Construct an agent message cell from markdown logical lines. + /// + /// This is the preferred streaming constructor: it preserves markdown indentation rules (list + /// markers, nested list continuation indent, blockquote prefix, etc.) so wrapping can be + /// performed correctly at render time for the current viewport width. + pub(crate) fn new_logical( + logical_lines: Vec, + is_first_line: bool, + ) -> Self { + Self { + logical_lines, is_first_line, } } @@ -331,43 +394,98 @@ impl HistoryCell for AgentMessageCell { self.transcript_lines_with_joiners(width).lines } + /// Render wrapped transcript lines plus soft-wrap joiners. + /// + /// This is where width-dependent wrapping happens for streaming agent output. The cell composes + /// indentation as: + /// + /// - transcript gutter (`• ` or ` `), plus + /// - markdown-provided indent/prefix spans (`initial_indent` / `subsequent_indent`) + /// + /// The wrapping algorithm returns a `joiner_before` vector so copy/paste can treat soft wraps + /// as joinable (no hard newline) while preserving exact whitespace at wrap boundaries. fn transcript_lines_with_joiners(&self, width: u16) -> TranscriptLinesWithJoiners { - use ratatui::style::Color; + if width == 0 { + return TranscriptLinesWithJoiners { + lines: Vec::new(), + joiner_before: Vec::new(), + }; + } let mut out_lines: Vec> = Vec::new(); let mut joiner_before: Vec> = Vec::new(); - let mut is_first_output_line = true; - for line in &self.lines { - let is_code_block_line = line.style.fg == Some(Color::Cyan); - let initial_indent: Line<'static> = if is_first_output_line && self.is_first_line { + // `at_cell_start` tracks whether we're about to emit the first *visual* line of this cell. + // Only the first chunk of a streamed message gets the `• ` gutter; continuations use ` `. + let mut at_cell_start = true; + for logical in &self.logical_lines { + let gutter_first_visual_line: Line<'static> = if at_cell_start && self.is_first_line { "• ".dim().into() } else { " ".into() }; - let subsequent_indent: Line<'static> = " ".into(); + let gutter_continuation: Line<'static> = " ".into(); + + // Compose the transcript gutter with markdown-provided indentation: + // + // - `gutter_*` is the transcript-level prefix (`• ` / ` `). + // - `initial_indent` / `subsequent_indent` come from markdown structure (blockquote + // prefix, list marker indentation, nested list continuation indentation, etc.). + // + // We apply these indents during wrapping so: + // - the UI renders with correct continuation indentation, and + // - soft-wrap joiners stay aligned with the exact whitespace the wrapper skipped. + let compose_indent = + |gutter: &Line<'static>, md_indent: &Line<'static>| -> Line<'static> { + let mut spans = gutter.spans.clone(); + spans.extend(md_indent.spans.iter().cloned()); + Line::from(spans) + }; - if is_code_block_line { - let mut spans = initial_indent.spans; - spans.extend(line.spans.iter().cloned()); - out_lines.push(Line::from(spans).style(line.style)); + // Preformatted lines are rendered as a single visual line (no wrapping). + // This preserves code-block whitespace and keeps code copy behavior stable. + if logical.is_preformatted { + let mut spans = gutter_first_visual_line.spans.clone(); + spans.extend(logical.initial_indent.spans.iter().cloned()); + spans.extend(logical.content.spans.iter().cloned()); + out_lines.push(Line::from(spans).style(logical.line_style)); joiner_before.push(None); - is_first_output_line = false; + at_cell_start = false; continue; } + // Prose path: wrap to current width and capture joiners. + // + // `word_wrap_line_with_joiners` guarantees: + // - `wrapped.len() == wrapped_joiners.len()` + // - `wrapped_joiners[0] == None` (first visual segment of a logical line is a hard break) + // - subsequent entries are `Some(joiner)` (soft-wrap continuations). let opts = RtOptions::new(width as usize) - .initial_indent(initial_indent) - .subsequent_indent(subsequent_indent.clone()); + .initial_indent(compose_indent( + &gutter_first_visual_line, + &logical.initial_indent, + )) + .subsequent_indent(compose_indent( + &gutter_continuation, + &logical.subsequent_indent, + )); + let (wrapped, wrapped_joiners) = - crate::wrapping::word_wrap_line_with_joiners(line, opts); - for (l, j) in wrapped.into_iter().zip(wrapped_joiners) { - out_lines.push(line_to_static(&l)); - joiner_before.push(j); - is_first_output_line = false; + crate::wrapping::word_wrap_line_with_joiners(&logical.content, opts); + for (visual, joiner) in wrapped.into_iter().zip(wrapped_joiners) { + out_lines.push(line_to_static(&visual).style(logical.line_style)); + joiner_before.push(joiner); + at_cell_start = false; } } + debug_assert_eq!(out_lines.len(), joiner_before.len()); + debug_assert!( + joiner_before + .first() + .is_none_or(std::option::Option::is_none) + ); + TranscriptLinesWithJoiners { lines: out_lines, joiner_before, @@ -699,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 } } } @@ -1674,6 +1792,131 @@ mod tests { render_lines(&cell.transcript_lines(u16::MAX)) } + /// Remove a single leading markdown blockquote marker (`> `) from `line`. + /// + /// This is a test-only normalization helper. + /// + /// In the rendered transcript, blockquote indentation is represented as literal `> ` spans in + /// the line prefix. For wrapped blockquote prose, those prefix spans can appear on every visual + /// line (including soft-wrap continuations). When we want to compare the *logical* joined text + /// across different widths, we strip the repeated marker on continuation lines so the + /// comparison doesn't fail due to prefix duplication. + fn strip_leading_blockquote_marker(line: &str) -> String { + let mut out = String::with_capacity(line.len()); + let mut seen_non_space = false; + let mut removed = false; + let mut chars = line.chars().peekable(); + while let Some(ch) = chars.next() { + if !seen_non_space { + if ch == ' ' { + out.push(ch); + continue; + } + seen_non_space = true; + if ch == '>' && !removed { + removed = true; + if matches!(chars.peek(), Some(' ')) { + chars.next(); + } + continue; + } + } + out.push(ch); + } + out + } + + /// Normalize rendered transcript output into a width-insensitive "logical text" string. + /// + /// This is used by resize/reflow tests: + /// + /// - Joiners tell us which visual line breaks are soft wraps (`Some(joiner)`) vs hard breaks + /// (`None`). + /// - For soft-wrap continuation lines, we strip repeated blockquote markers so we can compare + /// the underlying prose independent of prefix repetition. + /// - Finally, we collapse whitespace so wrapping differences (line breaks vs spaces) do not + /// affect equality. + fn normalize_rendered_text_with_joiners(tr: &TranscriptLinesWithJoiners) -> String { + let mut rendered = render_lines(&tr.lines); + for (line, joiner) in rendered.iter_mut().zip(&tr.joiner_before) { + if joiner.is_some() { + *line = strip_leading_blockquote_marker(line); + } + } + rendered + .join("\n") + .split_whitespace() + .collect::>() + .join(" ") + } + + #[test] + fn agent_message_cell_reflows_streamed_prose_on_resize() { + let md = concat!( + "- This is a long list item that should reflow when the viewport width changes. ", + "The old streaming implementation used to bake soft wraps into hard line breaks.\n", + "> A blockquote line that is also long enough to wrap and should reflow cleanly.\n", + ); + let logical_lines = crate::markdown_stream::simulate_stream_markdown_for_tests(&[md], true); + let cell = AgentMessageCell::new_logical(logical_lines, true); + + let narrow = cell.transcript_lines_with_joiners(28); + let wide = cell.transcript_lines_with_joiners(80); + + assert!( + narrow.lines.len() > wide.lines.len(), + "expected fewer visual lines at wider width; narrow={} wide={}", + narrow.lines.len(), + wide.lines.len() + ); + assert_eq!( + normalize_rendered_text_with_joiners(&narrow), + normalize_rendered_text_with_joiners(&wide) + ); + + let snapshot = format!( + "narrow:\n{}\n\nwide:\n{}", + render_lines(&narrow.lines).join("\n"), + render_lines(&wide.lines).join("\n") + ); + insta::assert_snapshot!("agent_message_cell_reflow_on_resize", snapshot); + } + + #[test] + fn agent_message_cell_reflows_streamed_prose_vt100_snapshot() { + use crate::test_backend::VT100Backend; + + let md = concat!( + "- This is a long list item that should reflow when the viewport width changes.\n", + "> A blockquote that also reflows across widths.\n", + ); + let logical_lines = crate::markdown_stream::simulate_stream_markdown_for_tests(&[md], true); + let cell = AgentMessageCell::new_logical(logical_lines, true); + + let render = |width, height| -> String { + let backend = VT100Backend::new(width, height); + let mut terminal = ratatui::Terminal::new(backend).expect("terminal"); + terminal + .draw(|f| { + let area = f.area(); + let lines = cell.display_lines(area.width); + Paragraph::new(Text::from(lines)) + .wrap(Wrap { trim: false }) + .render(area, f.buffer_mut()); + }) + .expect("draw"); + terminal.backend().vt100().screen().contents() + }; + + let narrow = render(30, 12); + let wide = render(70, 12); + + insta::assert_snapshot!( + "agent_message_cell_reflow_on_resize_vt100", + format!("narrow:\n{narrow}\n\nwide:\n{wide}") + ); + } + #[tokio::test] async fn mcp_tools_output_masks_sensitive_values() { let mut config = test_config().await; diff --git a/codex-rs/tui2/src/insert_history.rs b/codex-rs/tui2/src/insert_history.rs index 86a32c16c..1b236e8ee 100644 --- a/codex-rs/tui2/src/insert_history.rs +++ b/codex-rs/tui2/src/insert_history.rs @@ -1,9 +1,13 @@ -//! Render `ratatui` transcript lines into terminal scrollback. +//! Render `ratatui` transcript lines into terminal output (scrollback) and/or deterministic ANSI. //! //! `insert_history_lines` is responsible for inserting rendered transcript lines //! *above* the TUI viewport by emitting ANSI control sequences through the //! terminal backend writer. //! +//! Note: the current `tui2` main draw loop does not call `insert_history_lines` (see +//! `codex-rs/tui2/src/tui.rs`). This module is still used for deterministic ANSI emission via +//! `write_spans` (e.g., "print after exit" flows) and for tests. +//! //! ## Why we use crossterm style commands //! //! `write_spans` is also used by non-terminal callers (e.g. diff --git a/codex-rs/tui2/src/lib.rs b/codex-rs/tui2/src/lib.rs index 8cd3fde13..c8106e087 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/markdown_render.rs b/codex-rs/tui2/src/markdown_render.rs index 22a234326..c285ca41a 100644 --- a/codex-rs/tui2/src/markdown_render.rs +++ b/codex-rs/tui2/src/markdown_render.rs @@ -1,3 +1,37 @@ +//! Markdown rendering for `tui2`. +//! +//! This module has two related but intentionally distinct responsibilities: +//! +//! 1. **Parse Markdown into styled text** (for display). +//! 2. **Preserve width-agnostic structure for reflow** (for streaming + resize). +//! +//! ## Why logical lines exist +//! +//! TUI2 supports viewport resize reflow and copy/paste that treats soft-wrapped prose as a single +//! logical line. If we apply wrapping while rendering and store the resulting `Vec`, those +//! width-derived breaks become indistinguishable from hard newlines and cannot be "unwrapped" when +//! the viewport gets wider. +//! +//! To avoid baking width, streaming uses [`MarkdownLogicalLine`] output: +//! +//! - `content` holds the styled spans for a single *logical* line (a hard break boundary). +//! - `initial_indent` / `subsequent_indent` encode markdown-aware indentation rules for wraps +//! (list markers, nested lists, blockquotes, etc.). +//! - `line_style` captures line-level styling (e.g., blockquote green) that must apply to all +//! wrapped segments. +//! - `is_preformatted` marks runs that should not be wrapped like prose (e.g., fenced code). +//! +//! History cells can then wrap `content` at the *current* width, applying indents appropriately and +//! returning soft-wrap joiners for correct copy/paste. +//! +//! ## Outputs +//! +//! - [`render_markdown_text_with_width`]: emits a `Text` suitable for immediate display and may +//! apply wrapping if a width is provided. +//! - [`render_markdown_logical_lines`]: emits width-agnostic logical lines (no wrapping). +//! +//! The underlying `Writer` can emit either (or both) depending on call site needs. + use crate::render::line_utils::line_to_static; use crate::wrapping::RtOptions; use crate::wrapping::word_wrap_line; @@ -14,6 +48,31 @@ use ratatui::text::Line; use ratatui::text::Span; use ratatui::text::Text; +/// A single width-agnostic markdown "logical line" plus the metadata required to wrap it later. +/// +/// A logical line is a hard-break boundary produced by markdown parsing (explicit newlines, +/// paragraph boundaries, list item boundaries, etc.). It is not a viewport-derived wrap segment. +/// +/// Wrapping is performed later (typically in `HistoryCell::transcript_lines_with_joiners(width)`), +/// where a cell can: +/// +/// - prepend a transcript gutter prefix (`• ` / ` `), +/// - prepend markdown-specific indents (`initial_indent` / `subsequent_indent`), and +/// - wrap `content` to the current width while producing joiners for copy/paste. +#[derive(Clone, Debug)] +pub(crate) struct MarkdownLogicalLine { + /// The raw content for this logical line (does not include markdown prefix/indent spans). + pub(crate) content: Line<'static>, + /// Prefix/indent spans to apply to the first visual line when wrapping. + pub(crate) initial_indent: Line<'static>, + /// Prefix/indent spans to apply to wrapped continuation lines. + pub(crate) subsequent_indent: Line<'static>, + /// Line-level style to apply to all wrapped segments. + pub(crate) line_style: Style, + /// True when this line is preformatted and should not be wrapped like prose. + pub(crate) is_preformatted: bool, +} + struct MarkdownStyles { h1: Style, h2: Style, @@ -56,8 +115,12 @@ impl Default for MarkdownStyles { #[derive(Clone, Debug)] struct IndentContext { + /// Prefix spans to apply for this nesting level (e.g., blockquote `> `, list indentation). prefix: Vec>, + /// Optional list marker spans (e.g., `- ` or `1. `) that apply only to the first visual line of + /// a list item. marker: Option>>, + /// True if this context represents a list indentation level. is_list: bool, } @@ -75,21 +138,45 @@ pub fn render_markdown_text(input: &str) -> Text<'static> { render_markdown_text_with_width(input, None) } +/// Render markdown into a ratatui `Text`, optionally wrapping to a specific width. +/// +/// This is primarily used for non-streaming rendering where storing width-derived wrapping is +/// acceptable or where the caller immediately consumes the output. pub(crate) fn render_markdown_text_with_width(input: &str, width: Option) -> Text<'static> { let mut options = Options::empty(); options.insert(Options::ENABLE_STRIKETHROUGH); let parser = Parser::new_ext(input, options); - let mut w = Writer::new(parser, width); + let mut w = Writer::new(parser, width, true, false); w.run(); w.text } +/// Render markdown into width-agnostic logical lines (no wrapping). +/// +/// This is used by streaming so that the transcript can reflow on resize: wrapping is deferred to +/// the history cell at render time. +pub(crate) fn render_markdown_logical_lines(input: &str) -> Vec { + let mut options = Options::empty(); + options.insert(Options::ENABLE_STRIKETHROUGH); + let parser = Parser::new_ext(input, options); + let mut w = Writer::new(parser, None, false, true); + w.run(); + w.logical_lines +} + +/// A markdown event sink that builds either: +/// - a wrapped `Text` (`emit_text = true`), and/or +/// - width-agnostic [`MarkdownLogicalLine`]s (`emit_logical_lines = true`). +/// +/// The writer tracks markdown structure (paragraphs, lists, blockquotes, code blocks) and builds up +/// a "current logical line". `flush_current_line` commits it to the selected output(s). struct Writer<'a, I> where I: Iterator>, { iter: I, text: Text<'static>, + logical_lines: Vec, styles: MarkdownStyles, inline_styles: Vec