From cc8c293378c8dae5eee5fb11f915a99197cbdc17 Mon Sep 17 00:00:00 2001 From: Dylan Hurd Date: Tue, 10 Feb 2026 19:22:23 -0800 Subject: [PATCH 1/4] fix(exec-policy) No empty command lists (#11397) ## Summary This should rarely, if ever, happen in practice. But regardless, we should never provide an empty list of `commands` to ExecPolicy. This PR is almost entirely adding test around these cases. ## Testing - [x] Adds a bunch of unit tests for this --- codex-rs/core/src/exec_policy.rs | 74 ++++++- codex-rs/core/tests/suite/exec_policy.rs | 264 +++++++++++++++++++++++ codex-rs/shell-command/src/bash.rs | 15 ++ 3 files changed, 352 insertions(+), 1 deletion(-) diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 514b4f0af5..710794a78d 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -345,7 +345,9 @@ fn default_policy_path(codex_home: &Path) -> PathBuf { } fn commands_for_exec_policy(command: &[String]) -> (Vec>, bool) { - if let Some(commands) = parse_shell_lc_plain_commands(command) { + if let Some(commands) = parse_shell_lc_plain_commands(command) + && !commands.is_empty() + { return (commands, false); } @@ -814,6 +816,24 @@ prefix_rule(pattern=["rm"], decision="forbidden") ); } + #[test] + fn commands_for_exec_policy_falls_back_for_empty_shell_script() { + let command = vec!["bash".to_string(), "-lc".to_string(), "".to_string()]; + + assert_eq!(commands_for_exec_policy(&command), (vec![command], false)); + } + + #[test] + fn commands_for_exec_policy_falls_back_for_whitespace_shell_script() { + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + " \n\t ".to_string(), + ]; + + assert_eq!(commands_for_exec_policy(&command), (vec![command], false)); + } + #[tokio::test] async fn evaluates_heredoc_script_against_prefix_rules() { let policy_src = r#"prefix_rule(pattern=["python3"], decision="allow")"#; @@ -1023,6 +1043,58 @@ prefix_rule( ); } + #[tokio::test] + async fn empty_bash_lc_script_falls_back_to_original_command() { + let command = vec!["bash".to_string(), "-lc".to_string(), "".to_string()]; + + let manager = ExecPolicyManager::default(); + let requirement = manager + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command: &command, + approval_policy: AskForApproval::UnlessTrusted, + sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_permissions: SandboxPermissions::UseDefault, + prefix_rule: None, + }) + .await; + + assert_eq!( + requirement, + ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)), + } + ); + } + + #[tokio::test] + async fn whitespace_bash_lc_script_falls_back_to_original_command() { + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + " \n\t ".to_string(), + ]; + + let manager = ExecPolicyManager::default(); + let requirement = manager + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command: &command, + approval_policy: AskForApproval::UnlessTrusted, + sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_permissions: SandboxPermissions::UseDefault, + prefix_rule: None, + }) + .await; + + assert_eq!( + requirement, + ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)), + } + ); + } + #[tokio::test] async fn request_rule_uses_prefix_rule() { let command = vec![ diff --git a/codex-rs/core/tests/suite/exec_policy.rs b/codex-rs/core/tests/suite/exec_policy.rs index 835d940cde..4922fcf9c8 100644 --- a/codex-rs/core/tests/suite/exec_policy.rs +++ b/codex-rs/core/tests/suite/exec_policy.rs @@ -1,11 +1,15 @@ #![allow(clippy::unwrap_used, clippy::expect_used)] use anyhow::Result; +use codex_core::features::Feature; use codex_core::protocol::AskForApproval; use codex_core::protocol::EventMsg; use codex_core::protocol::Op; use codex_core::protocol::SandboxPolicy; +use codex_protocol::config_types::CollaborationMode; +use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::ReasoningSummary; +use codex_protocol::config_types::Settings; use codex_protocol::user_input::UserInput; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; @@ -16,9 +20,59 @@ use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; +use serde_json::Value; use serde_json::json; use std::fs; +fn collaboration_mode_for_model(model: String) -> CollaborationMode { + CollaborationMode { + mode: ModeKind::Default, + settings: Settings { + model, + reasoning_effort: None, + developer_instructions: Some("exercise approvals in collaboration mode".to_string()), + }, + } +} + +async fn submit_user_turn( + test: &core_test_support::test_codex::TestCodex, + prompt: &str, + approval_policy: AskForApproval, + sandbox_policy: SandboxPolicy, + collaboration_mode: Option, +) -> Result<()> { + let session_model = test.session_configured.model.clone(); + test.codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: prompt.into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + cwd: test.cwd_path().to_path_buf(), + approval_policy, + sandbox_policy, + model: session_model, + effort: None, + summary: ReasoningSummary::Auto, + collaboration_mode, + personality: None, + }) + .await?; + Ok(()) +} + +fn assert_no_matched_rules_invariant(output_item: &Value) { + let Some(output) = output_item.get("output").and_then(Value::as_str) else { + panic!("function_call_output should include string output payload: {output_item:?}"); + }; + assert!( + !output.contains("invariant failed: matched_rules must be non-empty"), + "unexpected invariant panic surfaced in output: {output}" + ); +} + #[tokio::test] async fn execpolicy_blocks_shell_invocation() -> Result<()> { // TODO execpolicy doesn't parse powershell commands yet @@ -107,3 +161,213 @@ async fn execpolicy_blocks_shell_invocation() -> Result<()> { Ok(()) } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn shell_command_empty_script_with_collaboration_mode_does_not_panic() -> Result<()> { + let server = start_mock_server().await; + let mut builder = test_codex().with_model("gpt-5").with_config(|config| { + config.features.enable(Feature::CollaborationModes); + }); + let test = builder.build(&server).await?; + let call_id = "shell-empty-script-collab"; + let args = json!({ + "command": "", + "timeout_ms": 1_000, + }); + + mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-empty-shell-1"), + ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?), + ev_completed("resp-empty-shell-1"), + ]), + ) + .await; + let results_mock = mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-empty-shell-1", "done"), + ev_completed("resp-empty-shell-2"), + ]), + ) + .await; + + let collaboration_mode = collaboration_mode_for_model(test.session_configured.model.clone()); + submit_user_turn( + &test, + "run an empty shell command", + AskForApproval::OnRequest, + SandboxPolicy::DangerFullAccess, + Some(collaboration_mode), + ) + .await?; + + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + let output_item = results_mock.single_request().function_call_output(call_id); + assert_no_matched_rules_invariant(&output_item); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn unified_exec_empty_script_with_collaboration_mode_does_not_panic() -> Result<()> { + let server = start_mock_server().await; + let mut builder = test_codex().with_model("gpt-5").with_config(|config| { + config.features.enable(Feature::UnifiedExec); + config.features.enable(Feature::CollaborationModes); + }); + let test = builder.build(&server).await?; + let call_id = "unified-exec-empty-script-collab"; + let args = json!({ + "cmd": "", + "yield_time_ms": 1_000, + }); + + mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-empty-unified-1"), + ev_function_call(call_id, "exec_command", &serde_json::to_string(&args)?), + ev_completed("resp-empty-unified-1"), + ]), + ) + .await; + let results_mock = mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-empty-unified-1", "done"), + ev_completed("resp-empty-unified-2"), + ]), + ) + .await; + + let collaboration_mode = collaboration_mode_for_model(test.session_configured.model.clone()); + submit_user_turn( + &test, + "run empty unified exec command", + AskForApproval::OnRequest, + SandboxPolicy::DangerFullAccess, + Some(collaboration_mode), + ) + .await?; + + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + let output_item = results_mock.single_request().function_call_output(call_id); + assert_no_matched_rules_invariant(&output_item); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn shell_command_whitespace_script_with_collaboration_mode_does_not_panic() -> Result<()> { + let server = start_mock_server().await; + let mut builder = test_codex().with_model("gpt-5").with_config(|config| { + config.features.enable(Feature::CollaborationModes); + }); + let test = builder.build(&server).await?; + let call_id = "shell-whitespace-script-collab"; + let args = json!({ + "command": " \n\t ", + "timeout_ms": 1_000, + }); + + mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-whitespace-shell-1"), + ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?), + ev_completed("resp-whitespace-shell-1"), + ]), + ) + .await; + let results_mock = mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-whitespace-shell-1", "done"), + ev_completed("resp-whitespace-shell-2"), + ]), + ) + .await; + + let collaboration_mode = collaboration_mode_for_model(test.session_configured.model.clone()); + submit_user_turn( + &test, + "run whitespace shell command", + AskForApproval::OnRequest, + SandboxPolicy::DangerFullAccess, + Some(collaboration_mode), + ) + .await?; + + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + let output_item = results_mock.single_request().function_call_output(call_id); + assert_no_matched_rules_invariant(&output_item); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn unified_exec_whitespace_script_with_collaboration_mode_does_not_panic() -> Result<()> { + let server = start_mock_server().await; + let mut builder = test_codex().with_model("gpt-5").with_config(|config| { + config.features.enable(Feature::UnifiedExec); + config.features.enable(Feature::CollaborationModes); + }); + let test = builder.build(&server).await?; + let call_id = "unified-exec-whitespace-script-collab"; + let args = json!({ + "cmd": " \n \t", + "yield_time_ms": 1_000, + }); + + mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-whitespace-unified-1"), + ev_function_call(call_id, "exec_command", &serde_json::to_string(&args)?), + ev_completed("resp-whitespace-unified-1"), + ]), + ) + .await; + let results_mock = mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-whitespace-unified-1", "done"), + ev_completed("resp-whitespace-unified-2"), + ]), + ) + .await; + + let collaboration_mode = collaboration_mode_for_model(test.session_configured.model.clone()); + submit_user_turn( + &test, + "run whitespace unified exec command", + AskForApproval::OnRequest, + SandboxPolicy::DangerFullAccess, + Some(collaboration_mode), + ) + .await?; + + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + let output_item = results_mock.single_request().function_call_output(call_id); + assert_no_matched_rules_invariant(&output_item); + + Ok(()) +} diff --git a/codex-rs/shell-command/src/bash.rs b/codex-rs/shell-command/src/bash.rs index a8eb37a927..47573621a8 100644 --- a/codex-rs/shell-command/src/bash.rs +++ b/codex-rs/shell-command/src/bash.rs @@ -431,6 +431,21 @@ mod tests { assert!(parse_seq("ls &&").is_none()); } + #[test] + fn rejects_empty_command_position_with_leading_operator() { + assert!(parse_seq("&& ls").is_none()); + } + + #[test] + fn rejects_empty_command_position_with_double_separator() { + assert!(parse_seq("ls ;; pwd").is_none()); + } + + #[test] + fn rejects_empty_command_position_with_empty_pipeline_segment() { + assert!(parse_seq("ls | | wc").is_none()); + } + #[test] fn parse_zsh_lc_plain_commands() { let command = vec!["zsh".to_string(), "-lc".to_string(), "ls".to_string()]; From 4473147985c300031f0aa28865cb83a613bd0225 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Tue, 10 Feb 2026 19:38:08 -0800 Subject: [PATCH 2/4] Do not resend output items in incremental websockets connections (#11383) In the incremental websocket output items are already part of the context, no need to send them again and duplicate. --- codex-rs/Cargo.lock | 360 +++++++++++++----- codex-rs/core/src/client.rs | 127 +++--- .../core/tests/suite/client_websockets.rs | 39 +- 3 files changed, 365 insertions(+), 161 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 4e2877a2c8..36ee957471 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -430,9 +430,9 @@ dependencies = [ [[package]] name = "arc-swap" -version = "1.8.0" +version = "1.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "51d03449bb8ca2cc2ef70869af31463d1ae5ccc8fa3e334b307203fbf815207e" +checksum = "9ded5f9a03ac8f24d1b8a25101ee812cd32cdc8c50a4c50237de2c4915850e73" dependencies = [ "rustversion", ] @@ -1227,9 +1227,9 @@ dependencies = [ [[package]] name = "clap" -version = "4.5.56" +version = "4.5.57" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a75ca66430e33a14957acc24c5077b503e7d374151b2b4b3a10c83b4ceb4be0e" +checksum = "6899ea499e3fb9305a65d5ebf6e3d2248c5fab291f300ad0a704fbe142eae31a" dependencies = [ "clap_builder", "clap_derive", @@ -1237,9 +1237,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.5.56" +version = "4.5.57" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "793207c7fa6300a0608d1080b858e5fdbe713cdc1c8db9fb17777d8a13e63df0" +checksum = "7b12c8b680195a62a8364d16b8447b01b6c2c8f9aaf68bee653be34d4245e238" dependencies = [ "anstream", "anstyle", @@ -1374,7 +1374,7 @@ dependencies = [ "tempfile", "time", "tokio", - "toml 0.9.11+spec-1.1.0", + "toml 0.9.12+spec-1.1.0", "tracing", "tracing-subscriber", "uuid", @@ -1534,7 +1534,7 @@ dependencies = [ "supports-color 3.0.2", "tempfile", "tokio", - "toml 0.9.11+spec-1.1.0", + "toml 0.9.12+spec-1.1.0", "tracing", ] @@ -1575,7 +1575,7 @@ dependencies = [ "serde_json", "tempfile", "tokio", - "toml 0.9.11+spec-1.1.0", + "toml 0.9.12+spec-1.1.0", "tracing", ] @@ -1635,7 +1635,7 @@ dependencies = [ "codex-utils-absolute-path", "pretty_assertions", "serde", - "toml 0.9.11+spec-1.1.0", + "toml 0.9.12+spec-1.1.0", ] [[package]] @@ -1728,8 +1728,8 @@ dependencies = [ "tokio", "tokio-tungstenite", "tokio-util", - "toml 0.9.11+spec-1.1.0", - "toml_edit 0.24.0+spec-1.1.0", + "toml 0.9.12+spec-1.1.0", + "toml_edit 0.24.1+spec-1.1.0", "tracing", "tracing-subscriber", "tracing-test", @@ -2304,7 +2304,7 @@ dependencies = [ "tokio", "tokio-stream", "tokio-util", - "toml 0.9.11+spec-1.1.0", + "toml 0.9.12+spec-1.1.0", "tracing", "tracing-appender", "tracing-subscriber", @@ -2380,7 +2380,7 @@ version = "0.0.0" dependencies = [ "pretty_assertions", "serde_json", - "toml 0.9.11+spec-1.1.0", + "toml 0.9.12+spec-1.1.0", ] [[package]] @@ -3318,9 +3318,9 @@ dependencies = [ [[package]] name = "ena" -version = "0.14.3" +version = "0.14.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3d248bdd43ce613d87415282f69b9bb99d947d290b10962dd6c56233312c2ad5" +checksum = "eabffdaee24bd1bf95c5ef7cec31260444317e72ea56c4c91750e8b7ee58d5f1" dependencies = [ "log", ] @@ -3640,9 +3640,9 @@ checksum = "1d674e81391d1e1ab681a28d99df07927c6d4aa5b027d7da16ba32d1d21ecd99" [[package]] name = "flate2" -version = "1.1.8" +version = "1.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b375d6465b98090a5f25b1c7703f3859783755aa9a80433b36e0379a3ec2f369" +checksum = "843fba2746e448b37e26a819579957415c8cef339bf08564fe8b7ddbd959573c" dependencies = [ "crc32fast", "libz-sys", @@ -3975,6 +3975,19 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "getrandom" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "139ef39800118c7683f2fd3c98c1b23c09ae076556b435f8e9064ae108aaeeec" +dependencies = [ + "cfg-if", + "libc", + "r-efi", + "wasip2", + "wasip3", +] + [[package]] name = "gif" version = "0.14.1" @@ -4001,7 +4014,7 @@ dependencies = [ "bstr", "log", "regex-automata", - "regex-syntax 0.8.8", + "regex-syntax 0.8.9", ] [[package]] @@ -4307,7 +4320,7 @@ dependencies = [ "tokio", "tokio-rustls", "tower-service", - "webpki-roots 1.0.5", + "webpki-roots 1.0.6", ] [[package]] @@ -4341,14 +4354,13 @@ dependencies = [ [[package]] name = "hyper-util" -version = "0.1.19" +version = "0.1.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "727805d60e7938b76b826a6ef209eb70eaa1812794f9424d4a4e2d740662df5f" +checksum = "96547c2556ec9d12fb1578c4eaf448b04993e7fb79cbaad930a656880a6bdfa0" dependencies = [ "base64 0.22.1", "bytes", "futures-channel", - "futures-core", "futures-util", "http 1.4.0", "http-body", @@ -4581,6 +4593,12 @@ dependencies = [ "zerovec", ] +[[package]] +name = "id-arena" +version = "2.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3d3067d79b975e8844ca9eb072e16b31c3c1c36928edf9c6789548c524d0d954" + [[package]] name = "ident_case" version = "1.0.1" @@ -4891,9 +4909,9 @@ checksum = "92ecc6618181def0457392ccd0ee51198e065e016d1d527a7ac1b6dc7c1f09d2" [[package]] name = "jiff" -version = "0.2.18" +version = "0.2.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e67e8da4c49d6d9909fe03361f9b620f58898859f5c7aded68351e85e71ecf50" +checksum = "d89a5b5e10d5a9ad6e5d1f4bd58225f655d6fe9767575a5e8ac5a6fe64e04495" dependencies = [ "jiff-static", "log", @@ -4904,9 +4922,9 @@ dependencies = [ [[package]] name = "jiff-static" -version = "0.2.18" +version = "0.2.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e0c84ee7f197eca9a86c6fd6cb771e55eb991632f15f2bc3ca6ec838929e6e78" +checksum = "ff7a39c8862fc1369215ccf0a8f12dd4598c7f6484704359f0351bd617034dbf" dependencies = [ "proc-macro2", "quote", @@ -5051,10 +5069,16 @@ dependencies = [ ] [[package]] -name = "libc" -version = "0.2.180" +name = "leb128fmt" +version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bcc35a38544a891a5f7c865aca548a982ccb3b8650a5b06d0fd33a10283c56fc" +checksum = "09edd9e8b54e49e587e4f6295a7d29c3ea94d469cb40ab8ca70b288248a81db2" + +[[package]] +name = "libc" +version = "0.2.181" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "459427e2af2b9c839b132acb702a1c654d95e10f8c326bfc2ad11310e458b1c5" [[package]] name = "libdbus-sys" @@ -5314,9 +5338,9 @@ checksum = "ae960838283323069879657ca3de837e9f7bbb4c7bf6ea7f1b290d5e9476d2e0" [[package]] name = "memchr" -version = "2.7.6" +version = "2.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f52b00d39961fc5b2736ea853c9cc86238e165017a493d1d5c8eac6bdc4cc273" +checksum = "f8ca58f447f06ed17d5fc4043ce1b10dd205e060fb3ce5b979b8ed8e59ff3f79" [[package]] name = "memoffset" @@ -6474,6 +6498,16 @@ dependencies = [ "yansi", ] +[[package]] +name = "prettyplease" +version = "0.2.37" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "479ca8adacdd7ce8f1fb39ce9ecccbfe93a3f1344b3d0d97f20bc0196208f62b" +dependencies = [ + "proc-macro2", + "syn 2.0.114", +] + [[package]] name = "proc-macro-crate" version = "3.4.0" @@ -6530,16 +6564,16 @@ dependencies = [ [[package]] name = "proptest" -version = "1.9.0" +version = "1.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bee689443a2bd0a16ab0348b52ee43e3b2d1b1f931c8aa5c9f8de4c86fbe8c40" +checksum = "37566cb3fdacef14c0737f9546df7cfeadbfbc9fef10991038bf5015d0c80532" dependencies = [ "bitflags 2.10.0", "num-traits", "rand 0.9.2", "rand_chacha 0.9.0", "rand_xorshift", - "regex-syntax 0.8.8", + "regex-syntax 0.8.9", "unarray", ] @@ -6568,9 +6602,9 @@ dependencies = [ [[package]] name = "psl" -version = "2.1.184" +version = "2.1.188" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "81dc6a90669f481b41cae3005c68efa36bef275b95aa9123a7af7f1c68c6e5b2" +checksum = "b033d75bca9da25cfdcd9528de22ed7870d1695b9e1c3ce55b7127a4a2b16fac" dependencies = [ "psl-types", ] @@ -6978,7 +7012,7 @@ dependencies = [ "rustls-pki-types", "tokio", "tokio-rustls", - "webpki-roots 1.0.5", + "webpki-roots 1.0.6", "x509-parser", ] @@ -7224,25 +7258,25 @@ dependencies = [ "aho-corasick", "memchr", "regex-automata", - "regex-syntax 0.8.8", + "regex-syntax 0.8.9", ] [[package]] name = "regex-automata" -version = "0.4.13" +version = "0.4.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5276caf25ac86c8d810222b3dbb938e512c55c6831a10f3e6ed1c93b84041f1c" +checksum = "6e1dd4122fc1595e8162618945476892eefca7b88c52820e74af6262213cae8f" dependencies = [ "aho-corasick", "memchr", - "regex-syntax 0.8.8", + "regex-syntax 0.8.9", ] [[package]] name = "regex-lite" -version = "0.1.8" +version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8d942b98df5e658f56f20d592c7f868833fe38115e65c33003d8cd224b0155da" +checksum = "cab834c73d247e67f4fae452806d17d3c7501756d98c8808d7c9c7aa7d18f973" [[package]] name = "regex-syntax" @@ -7252,9 +7286,9 @@ checksum = "f162c6dd7b008981e4d40210aca20b4bd0f9b60ca9271061b07f78537722f2e1" [[package]] name = "regex-syntax" -version = "0.8.8" +version = "0.8.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7a2d987857b319362043e95f5353c0535c1f58eec5336fdfcf626430af7def58" +checksum = "a96887878f22d7bad8a3b6dc5b7440e0ada9a245242924394987b21cf2210a4c" [[package]] name = "reqwest" @@ -7302,7 +7336,7 @@ dependencies = [ "wasm-bindgen-futures", "wasm-streams", "web-sys", - "webpki-roots 1.0.5", + "webpki-roots 1.0.6", ] [[package]] @@ -7585,9 +7619,9 @@ dependencies = [ [[package]] name = "ryu" -version = "1.0.22" +version = "1.0.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a50f4cf475b65d88e057964e0e9bb1f0aa9bbb2036dc65c64596b42932536984" +checksum = "9774ba4a74de5f7b1c1451ed6cd5285a32eddb5cccb8cc655a4e50009e06477f" [[package]] name = "salsa20" @@ -7854,10 +7888,11 @@ checksum = "d767eb0aabc880b29956c35734170f26ed551a859dbd361d140cdbeca61ab1e2" [[package]] name = "sentry" -version = "0.46.1" +version = "0.46.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2f925d575b468e88b079faf590a8dd0c9c99e2ec29e9bab663ceb8b45056312f" +checksum = "d92d893ba7469d361a6958522fa440e4e2bc8bf4c5803cd1bf40b9af63f8f9a8" dependencies = [ + "cfg_aliases 0.2.1", "httpdate", "native-tls", "reqwest", @@ -7874,9 +7909,9 @@ dependencies = [ [[package]] name = "sentry-actix" -version = "0.46.1" +version = "0.46.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "18bac0f6b8621fa0f85e298901e51161205788322e1a995e3764329020368058" +checksum = "56cb150fd6b55b3023714a3aaa1e3bdadfd44f164efc54fad69efc69aac36887" dependencies = [ "actix-http", "actix-web", @@ -7887,9 +7922,9 @@ dependencies = [ [[package]] name = "sentry-backtrace" -version = "0.46.1" +version = "0.46.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6cb1ef7534f583af20452b1b1bf610a60ed9c8dd2d8485e7bd064efc556a78fb" +checksum = "5f8784d0a27b5cd4b5f75769ffc84f0b7580e3c35e1af9cd83cb90b612d769cc" dependencies = [ "backtrace", "regex", @@ -7898,9 +7933,9 @@ dependencies = [ [[package]] name = "sentry-contexts" -version = "0.46.1" +version = "0.46.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ebd6be899d9938390b6d1ec71e2f53bd9e57b6a9d8b1d5b049e5c364e7da9078" +checksum = "0e5eb42f4cd4f9fdfec9e3b07b25a4c9769df83d218a7e846658984d5948ad3e" dependencies = [ "hostname", "libc", @@ -7912,9 +7947,9 @@ dependencies = [ [[package]] name = "sentry-core" -version = "0.46.1" +version = "0.46.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "26ab054c34b87f96c3e4701bea1888317cde30cc7e4a6136d2c48454ab96661c" +checksum = "b0b1e7ca40f965db239da279bf278d87b7407469b98835f27f0c8e59ed189b06" dependencies = [ "rand 0.9.2", "sentry-types", @@ -7925,9 +7960,9 @@ dependencies = [ [[package]] name = "sentry-debug-images" -version = "0.46.1" +version = "0.46.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5637ec550dc6f8c49a711537950722d3fc4baa6fd433c371912104eaff31e2a5" +checksum = "002561e49ea3a9de316e2efadc40fae553921b8ff41448f02ea85fd135a778d6" dependencies = [ "findshlibs", "sentry-core", @@ -7935,9 +7970,9 @@ dependencies = [ [[package]] name = "sentry-panic" -version = "0.46.1" +version = "0.46.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3f02c7162f7b69b8de872b439d4696dc1d65f80b13ddd3c3831723def4756b63" +checksum = "8906f8be87aea5ac7ef937323fb655d66607427f61007b99b7cb3504dc5a156c" dependencies = [ "sentry-backtrace", "sentry-core", @@ -7945,9 +7980,9 @@ dependencies = [ [[package]] name = "sentry-tracing" -version = "0.46.1" +version = "0.46.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e1dd47df349a80025819f3d25c3d2f751df705d49c65a4cdc0f130f700972a48" +checksum = "5b07eefe04486316c57aba08ab53dd44753c25102d1d3fe05775cc93a13262d9" dependencies = [ "bitflags 2.10.0", "sentry-backtrace", @@ -7958,9 +7993,9 @@ dependencies = [ [[package]] name = "sentry-types" -version = "0.46.1" +version = "0.46.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eecbd63e9d15a26a40675ed180d376fcb434635d2e33de1c24003f61e3e2230d" +checksum = "567711f01f86a842057e1fc17779eba33a336004227e1a1e7e6cc2599e22e259" dependencies = [ "debugid", "hex", @@ -8844,9 +8879,9 @@ dependencies = [ [[package]] name = "system-configuration" -version = "0.6.1" +version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3c879d448e9d986b661742763247d3693ed13609438cf3d006f51f5368a5ba6b" +checksum = "a13f3d0daba03132c0aa9767f98351b3488edc2c100cda2d2ec2b04f3d8d3c8b" dependencies = [ "bitflags 2.10.0", "core-foundation 0.9.4", @@ -8871,12 +8906,12 @@ checksum = "7b2093cf4c8eb1e67749a6762251bc9cd836b6fc171623bd0a9d324d37af2417" [[package]] name = "tempfile" -version = "3.24.0" +version = "3.25.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "655da9c7eb6305c55742045d5a8d2037996d61d8de95806335c7c86ce0f82e9c" +checksum = "0136791f7c95b1f6dd99f9cc786b91bb81c3800b639b3478e561ddb7be95e5f1" dependencies = [ "fastrand", - "getrandom 0.3.4", + "getrandom 0.4.1", "once_cell", "rustix 1.1.3", "windows-sys 0.61.2", @@ -9261,9 +9296,9 @@ dependencies = [ [[package]] name = "toml" -version = "0.9.11+spec-1.1.0" +version = "0.9.12+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f3afc9a848309fe1aaffaed6e1546a7a14de1f935dc9d89d32afd9a44bab7c46" +checksum = "cf92845e79fc2e2def6a5d828f0801e29a2f8acc037becc5ab08595c7d5e9863" dependencies = [ "indexmap 2.13.0", "serde_core", @@ -9297,9 +9332,9 @@ dependencies = [ [[package]] name = "toml_edit" -version = "0.24.0+spec-1.1.0" +version = "0.24.1+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8c740b185920170a6d9191122cafef7010bd6270a3824594bff6784c04d7f09e" +checksum = "01f2eadbbc6b377a847be05f60791ef1058d9f696ecb51d2c07fe911d8569d8e" dependencies = [ "indexmap 2.13.0", "toml_datetime", @@ -9310,9 +9345,9 @@ dependencies = [ [[package]] name = "toml_parser" -version = "1.0.6+spec-1.1.0" +version = "1.0.7+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a3198b4b0a8e11f09dd03e133c0280504d0801269e9afa46362ffde1cbeebf44" +checksum = "247eaa3197818b831697600aadf81514e577e0cba5eab10f7e064e78ae154df1" dependencies = [ "winnow", ] @@ -9513,9 +9548,9 @@ dependencies = [ [[package]] name = "tracing-test" -version = "0.2.5" +version = "0.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "557b891436fe0d5e0e363427fc7f217abf9ccd510d5136549847bdcbcd011d68" +checksum = "19a4c448db514d4f24c5ddb9f73f2ee71bfb24c526cf0c570ba142d1119e0051" dependencies = [ "tracing-core", "tracing-subscriber", @@ -9524,9 +9559,9 @@ dependencies = [ [[package]] name = "tracing-test-macro" -version = "0.2.5" +version = "0.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "04659ddb06c87d233c566112c1c9c5b9e98256d9af50ec3bc9c8327f873a7568" +checksum = "ad06847b7afb65c7866a36664b75c40b895e318cea4f71299f013fb22965329d" dependencies = [ "quote", "syn 2.0.114", @@ -9540,7 +9575,7 @@ checksum = "78f873475d258561b06f1c595d93308a7ed124d9977cb26b148c2084a4a3cc87" dependencies = [ "cc", "regex", - "regex-syntax 0.8.8", + "regex-syntax 0.8.9", "serde_json", "streaming-iterator", "tree-sitter-language", @@ -9709,9 +9744,9 @@ checksum = "5c1cb5db39152898a79168971543b1cb5020dff7fe43c8dc468b0885f5e29df5" [[package]] name = "unicode-ident" -version = "1.0.22" +version = "1.0.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9312f7c4f6ff9069b165498234ce8be658059c6728633667c526e27dc2cf1df5" +checksum = "537dd038a89878be9b64dd4bd1b260315c1bb94f4d784956b81e27a088d9a09e" [[package]] name = "unicode-linebreak" @@ -9799,9 +9834,9 @@ checksum = "8ecb6da28b8a351d773b68d5825ac39017e680750f980f3a1a85cd8dd28a47c1" [[package]] name = "ureq" -version = "3.1.4" +version = "3.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d39cb1dbab692d82a977c0392ffac19e188bd9186a9f32806f0aaa859d75585a" +checksum = "fdc97a28575b85cfedf2a7e7d3cc64b3e11bd8ac766666318003abbacc7a21fc" dependencies = [ "base64 0.22.1", "der", @@ -9958,6 +9993,15 @@ dependencies = [ "wit-bindgen", ] +[[package]] +name = "wasip3" +version = "0.4.0+wasi-0.3.0-rc-2026-01-06" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5428f8bf88ea5ddc08faddef2ac4a67e390b88186c703ce6dbd955e1c145aca5" +dependencies = [ + "wit-bindgen", +] + [[package]] name = "wasite" version = "0.1.0" @@ -10023,6 +10067,28 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "wasm-encoder" +version = "0.244.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "990065f2fe63003fe337b932cfb5e3b80e0b4d0f5ff650e6985b1048f62c8319" +dependencies = [ + "leb128fmt", + "wasmparser", +] + +[[package]] +name = "wasm-metadata" +version = "0.244.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bb0e353e6a2fbdc176932bbaab493762eb1255a7900fe0fea1a2f96c296cc909" +dependencies = [ + "anyhow", + "indexmap 2.13.0", + "wasm-encoder", + "wasmparser", +] + [[package]] name = "wasm-streams" version = "0.4.2" @@ -10036,6 +10102,18 @@ dependencies = [ "web-sys", ] +[[package]] +name = "wasmparser" +version = "0.244.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47b807c72e1bac69382b3a6fb3dbe8ea4c0ed87ff5629b8685ae6b9a611028fe" +dependencies = [ + "bitflags 2.10.0", + "hashbrown 0.15.5", + "indexmap 2.13.0", + "semver", +] + [[package]] name = "wayland-backend" version = "0.3.12" @@ -10128,9 +10206,9 @@ dependencies = [ [[package]] name = "webbrowser" -version = "1.0.6" +version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "00f1243ef785213e3a32fa0396093424a3a6ea566f9948497e5a2309261a4c97" +checksum = "3f00bb839c1cf1e3036066614cbdcd035ecf215206691ea646aa3c60a24f68f2" dependencies = [ "core-foundation 0.10.1", "jni", @@ -10144,9 +10222,9 @@ dependencies = [ [[package]] name = "webpki-root-certs" -version = "1.0.5" +version = "1.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "36a29fc0408b113f68cf32637857ab740edfafdf460c326cd2afaa2d84cc05dc" +checksum = "804f18a4ac2676ffb4e8b5b5fa9ae38af06df08162314f96a68d2a363e21a8ca" dependencies = [ "rustls-pki-types", ] @@ -10157,14 +10235,14 @@ version = "0.26.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "521bc38abb08001b01866da9f51eb7c5d647a19260e00054a8c7fd5f9e57f7a9" dependencies = [ - "webpki-roots 1.0.5", + "webpki-roots 1.0.6", ] [[package]] name = "webpki-roots" -version = "1.0.5" +version = "1.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "12bed680863276c63889429bfd6cab3b99943659923822de1c8a39c49e4d722c" +checksum = "22cfaf3c063993ff62e73cb4311efde4db1efb31ab78a3e5c457939ad5cc0bed" dependencies = [ "rustls-pki-types", ] @@ -10807,6 +10885,88 @@ name = "wit-bindgen" version = "0.51.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d7249219f66ced02969388cf2bb044a09756a083d0fab1e566056b04d9fbcaa5" +dependencies = [ + "wit-bindgen-rust-macro", +] + +[[package]] +name = "wit-bindgen-core" +version = "0.51.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ea61de684c3ea68cb082b7a88508a8b27fcc8b797d738bfc99a82facf1d752dc" +dependencies = [ + "anyhow", + "heck", + "wit-parser", +] + +[[package]] +name = "wit-bindgen-rust" +version = "0.51.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7c566e0f4b284dd6561c786d9cb0142da491f46a9fbed79ea69cdad5db17f21" +dependencies = [ + "anyhow", + "heck", + "indexmap 2.13.0", + "prettyplease", + "syn 2.0.114", + "wasm-metadata", + "wit-bindgen-core", + "wit-component", +] + +[[package]] +name = "wit-bindgen-rust-macro" +version = "0.51.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c0f9bfd77e6a48eccf51359e3ae77140a7f50b1e2ebfe62422d8afdaffab17a" +dependencies = [ + "anyhow", + "prettyplease", + "proc-macro2", + "quote", + "syn 2.0.114", + "wit-bindgen-core", + "wit-bindgen-rust", +] + +[[package]] +name = "wit-component" +version = "0.244.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d66ea20e9553b30172b5e831994e35fbde2d165325bec84fc43dbf6f4eb9cb2" +dependencies = [ + "anyhow", + "bitflags 2.10.0", + "indexmap 2.13.0", + "log", + "serde", + "serde_derive", + "serde_json", + "wasm-encoder", + "wasm-metadata", + "wasmparser", + "wit-parser", +] + +[[package]] +name = "wit-parser" +version = "0.244.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ecc8ac4bc1dc3381b7f59c34f00b67e18f910c2c0f50015669dde7def656a736" +dependencies = [ + "anyhow", + "id-arena", + "indexmap 2.13.0", + "log", + "semver", + "serde", + "serde_derive", + "serde_json", + "unicode-xid", + "wasmparser", +] [[package]] name = "wl-clipboard-rs" @@ -11001,18 +11161,18 @@ dependencies = [ [[package]] name = "zerocopy" -version = "0.8.37" +version = "0.8.39" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7456cf00f0685ad319c5b1693f291a650eaf345e941d082fc4e03df8a03996ac" +checksum = "db6d35d663eadb6c932438e763b262fe1a70987f9ae936e60158176d710cae4a" dependencies = [ "zerocopy-derive", ] [[package]] name = "zerocopy-derive" -version = "0.8.37" +version = "0.8.39" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1328722bbf2115db7e19d69ebcc15e795719e2d66b60827c6a69a117365e37a0" +checksum = "4122cd3169e94605190e77839c9a40d40ed048d305bfdc146e7df40ab0f3e517" dependencies = [ "proc-macro2", "quote", @@ -11126,9 +11286,9 @@ dependencies = [ [[package]] name = "zmij" -version = "1.0.19" +version = "1.0.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3ff05f8caa9038894637571ae6b9e29466c1f4f829d26c9b28f869a29cbe3445" +checksum = "4de98dfa5d5b7fef4ee834d0073d560c9ca7b6c46a71d058c48db7960f8cfaf7" [[package]] name = "zopfli" diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 0999c3291d..ffb74315a0 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -167,8 +167,7 @@ pub struct ModelClientSession { client: ModelClient, connection: Option, websocket_last_request: Option, - websocket_last_response_id: Option, - websocket_last_response_id_rx: Option>, + websocket_last_response_rx: Option>, /// Turn state for sticky routing. /// /// This is an `OnceLock` that stores the turn state value received from the server @@ -182,6 +181,12 @@ pub struct ModelClientSession { turn_state: Arc>, } +#[derive(Debug, Clone)] +struct LastResponse { + response_id: String, + items_added: Vec, +} + enum WebsocketStreamOutcome { Stream(ResponseStream), FallbackToHttp, @@ -231,8 +236,7 @@ impl ModelClient { client: self.clone(), connection: None, websocket_last_request: None, - websocket_last_response_id: None, - websocket_last_response_id_rx: None, + websocket_last_response_rx: None, turn_state: Arc::new(OnceLock::new()), } } @@ -531,10 +535,15 @@ impl ModelClientSession { } } - fn get_incremental_items(&self, request: &ResponsesApiRequest) -> Option> { + fn get_incremental_items( + &self, + request: &ResponsesApiRequest, + last_response: Option<&LastResponse>, + ) -> Option> { // Checks whether the current request is an incremental append to the previous request. // We only append when non-input request fields are unchanged and `input` is a strict - // extension of the previous input. + // extension of the previous known input. Server-returned output items are treated as part + // of the baseline so we do not resend them. let previous_request = self.websocket_last_request.as_ref()?; let mut previous_without_input = previous_request.clone(); previous_without_input.input.clear(); @@ -544,38 +553,29 @@ impl ModelClientSession { return None; } - let previous_len = previous_request.input.len(); - if previous_len > 0 - && request.input.starts_with(&previous_request.input) - && previous_len < request.input.len() + let mut baseline = previous_request.input.clone(); + if let Some(last_response) = last_response { + baseline.extend(last_response.items_added.clone()); + } + + let baseline_len = baseline.len(); + if baseline_len > 0 + && request.input.starts_with(&baseline) + && baseline_len < request.input.len() { - Some(request.input[previous_len..].to_vec()) + Some(request.input[baseline_len..].to_vec()) } else { None } } - fn refresh_websocket_last_response_id(&mut self) { - if let Some(mut receiver) = self.websocket_last_response_id_rx.take() { - match receiver.try_recv() { - Ok(response_id) if !response_id.is_empty() => { - self.websocket_last_response_id = Some(response_id); - } - Ok(_) | Err(TryRecvError::Closed) => { - self.websocket_last_response_id = None; - } - Err(TryRecvError::Empty) => { - self.websocket_last_response_id_rx = Some(receiver); - } - } - } - } - - fn websocket_previous_response_id(&mut self) -> Option { - self.refresh_websocket_last_response_id(); - self.websocket_last_response_id - .clone() - .filter(|id| !id.is_empty()) + fn get_last_response(&mut self) -> Option { + self.websocket_last_response_rx + .take() + .and_then(|mut receiver| match receiver.try_recv() { + Ok(last_response) => Some(last_response), + Err(TryRecvError::Closed) | Err(TryRecvError::Empty) => None, + }) } fn prepare_websocket_request( @@ -583,11 +583,15 @@ impl ModelClientSession { payload: ResponseCreateWsRequest, request: &ResponsesApiRequest, ) -> ResponsesWsRequest { + let last_response = self.get_last_response(); let responses_websockets_v2_enabled = self.client.responses_websockets_v2_enabled(); - let incremental_items = self.get_incremental_items(request); + let incremental_items = self.get_incremental_items(request, last_response.as_ref()); if let Some(append_items) = incremental_items { if responses_websockets_v2_enabled - && let Some(previous_response_id) = self.websocket_previous_response_id() + && let Some(previous_response_id) = last_response + .as_ref() + .map(|last_response| last_response.response_id.clone()) + .filter(|id| !id.is_empty()) { let payload = ResponseCreateWsRequest { previous_response_id: Some(previous_response_id), @@ -660,8 +664,7 @@ impl ModelClientSession { if needs_new { self.websocket_last_request = None; - self.websocket_last_response_id = None; - self.websocket_last_response_id_rx = None; + self.websocket_last_response_rx = None; let turn_state = options .turn_state .clone() @@ -716,7 +719,8 @@ impl ModelClientSession { self.client.state.provider.stream_idle_timeout(), ) .map_err(map_api_error)?; - return Ok(map_response_stream(stream, otel_manager.clone())); + let (stream, _last_request_rx) = map_response_stream(stream, otel_manager.clone()); + return Ok(stream); } let auth_manager = self.client.state.auth_manager.clone(); @@ -747,7 +751,8 @@ impl ModelClientSession { match stream_result { Ok(stream) => { - return Ok(map_response_stream(stream, otel_manager.clone())); + let (stream, _) = map_response_stream(stream, otel_manager.clone()); + return Ok(stream); } Err(ApiError::Transport( unauthorized_transport @ TransportError::Http { status, .. }, @@ -829,22 +834,11 @@ impl ModelClientSession { .await .map_err(map_api_error)?; self.websocket_last_request = Some(request); - let (last_response_id_sender, last_response_id_receiver) = oneshot::channel(); - self.websocket_last_response_id_rx = Some(last_response_id_receiver); - let mut last_response_id_sender = Some(last_response_id_sender); - let stream_result = stream_result.inspect(move |event| { - if let Ok(ResponseEvent::Completed { response_id, .. }) = event - && !response_id.is_empty() - && let Some(sender) = last_response_id_sender.take() - { - let _ = sender.send(response_id.clone()); - } - }); + let (stream, last_request_rx) = + map_response_stream(stream_result, otel_manager.clone()); + self.websocket_last_response_rx = Some(last_request_rx); - return Ok(WebsocketStreamOutcome::Stream(map_response_stream( - stream_result, - otel_manager.clone(), - ))); + return Ok(WebsocketStreamOutcome::Stream(stream)); } } @@ -942,6 +936,7 @@ impl ModelClientSession { self.connection = None; self.websocket_last_request = None; + self.websocket_last_response_rx = None; } activated } @@ -986,7 +981,10 @@ fn build_responses_headers( headers } -fn map_response_stream(api_stream: S, otel_manager: OtelManager) -> ResponseStream +fn map_response_stream( + api_stream: S, + otel_manager: OtelManager, +) -> (ResponseStream, oneshot::Receiver) where S: futures::Stream> + Unpin @@ -994,12 +992,25 @@ where + 'static, { let (tx_event, rx_event) = mpsc::channel::>(1600); + let (tx_last_response, rx_last_response) = oneshot::channel::(); tokio::spawn(async move { let mut logged_error = false; + let mut tx_last_response = Some(tx_last_response); + let mut items_added: Vec = Vec::new(); let mut api_stream = api_stream; while let Some(event) = api_stream.next().await { match event { + Ok(ResponseEvent::OutputItemDone(item)) => { + items_added.push(item.clone()); + if tx_event + .send(Ok(ResponseEvent::OutputItemDone(item))) + .await + .is_err() + { + return; + } + } Ok(ResponseEvent::Completed { response_id, token_usage, @@ -1013,6 +1024,12 @@ where usage.total_tokens, ); } + if let Some(sender) = tx_last_response.take() { + let _ = sender.send(LastResponse { + response_id: response_id.clone(), + items_added: std::mem::take(&mut items_added), + }); + } if tx_event .send(Ok(ResponseEvent::Completed { response_id, @@ -1043,7 +1060,7 @@ where } }); - ResponseStream { rx_event } + (ResponseStream { rx_event }, rx_last_response) } /// Handles a 401 response by optionally refreshing ChatGPT tokens once. diff --git a/codex-rs/core/tests/suite/client_websockets.rs b/codex-rs/core/tests/suite/client_websockets.rs index c4f63f8440..18527ce713 100755 --- a/codex-rs/core/tests/suite/client_websockets.rs +++ b/codex-rs/core/tests/suite/client_websockets.rs @@ -29,6 +29,7 @@ use codex_protocol::user_input::UserInput; use core_test_support::load_default_config_for_test; use core_test_support::responses::WebSocketConnectionConfig; use core_test_support::responses::WebSocketTestServer; +use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_response_created; use core_test_support::responses::start_websocket_server; @@ -570,7 +571,11 @@ async fn responses_websocket_appends_on_prefix() { skip_if_no_network!(); let server = start_websocket_server(vec![vec![ - vec![ev_response_created("resp-1"), ev_completed("resp-1")], + vec![ + ev_response_created("resp-1"), + ev_assistant_message("msg-1", "assistant output"), + ev_completed("resp-1"), + ], vec![ev_response_created("resp-2"), ev_completed("resp-2")], ]]) .await; @@ -578,7 +583,11 @@ async fn responses_websocket_appends_on_prefix() { let harness = websocket_harness(&server).await; let mut client_session = harness.client.new_session(); let prompt_one = prompt_with_input(vec![message_item("hello")]); - let prompt_two = prompt_with_input(vec![message_item("hello"), message_item("second")]); + let prompt_two = prompt_with_input(vec![ + message_item("hello"), + assistant_message_item("msg-1", "assistant output"), + message_item("second"), + ]); stream_until_complete(&mut client_session, &harness, &prompt_one).await; stream_until_complete(&mut client_session, &harness, &prompt_two).await; @@ -594,7 +603,7 @@ async fn responses_websocket_appends_on_prefix() { assert_eq!(first["input"].as_array().map(Vec::len), Some(1)); let expected_append = serde_json::json!({ "type": "response.append", - "input": serde_json::to_value(&prompt_two.input[1..]).expect("serialize append items"), + "input": serde_json::to_value(&prompt_two.input[2..]).expect("serialize append items"), }); assert_eq!(second, expected_append); @@ -675,7 +684,11 @@ async fn responses_websocket_v2_creates_with_previous_response_id_on_prefix() { skip_if_no_network!(); let server = start_websocket_server(vec![vec![ - vec![ev_response_created("resp-1"), ev_completed("resp-1")], + vec![ + ev_response_created("resp-1"), + ev_assistant_message("msg-1", "assistant output"), + ev_completed("resp-1"), + ], vec![ev_response_created("resp-2"), ev_completed("resp-2")], ]]) .await; @@ -683,7 +696,11 @@ async fn responses_websocket_v2_creates_with_previous_response_id_on_prefix() { let harness = websocket_harness_with_v2(&server, true).await; let mut session = harness.client.new_session(); let prompt_one = prompt_with_input(vec![message_item("hello")]); - let prompt_two = prompt_with_input(vec![message_item("hello"), message_item("second")]); + let prompt_two = prompt_with_input(vec![ + message_item("hello"), + assistant_message_item("msg-1", "assistant output"), + message_item("second"), + ]); stream_until_complete(&mut session, &harness, &prompt_one).await; stream_until_complete(&mut session, &harness, &prompt_two).await; @@ -698,7 +715,7 @@ async fn responses_websocket_v2_creates_with_previous_response_id_on_prefix() { assert_eq!(second["previous_response_id"].as_str(), Some("resp-1")); assert_eq!( second["input"], - serde_json::to_value(&prompt_two.input[1..]).unwrap() + serde_json::to_value(&prompt_two.input[2..]).unwrap() ); server.shutdown().await; @@ -876,6 +893,16 @@ fn message_item(text: &str) -> ResponseItem { } } +fn assistant_message_item(id: &str, text: &str) -> ResponseItem { + ResponseItem::Message { + id: Some(id.to_string()), + role: "assistant".into(), + content: vec![ContentItem::OutputText { text: text.into() }], + end_turn: None, + phase: None, + } +} + fn prompt_with_input(input: Vec) -> Prompt { let mut prompt = Prompt::default(); prompt.input = input; From 641d5268fa581bbdff401e5e8e8cc346976b92fd Mon Sep 17 00:00:00 2001 From: Celia Chen Date: Tue, 10 Feb 2026 19:56:01 -0800 Subject: [PATCH 3/4] chore: persist turn_id in rollout session and make turn_id uuid based (#11246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem: 1. turn id is constructed in-memory; 2. on resuming threads, turn_id might not be unique; 3. client cannot no the boundary of a turn from rollout files easily. This PR does three things: 1. persist `task_started` and `task_complete` events; 1. persist `turn_id` in rollout turn events; 5. generate turn_id as unique uuids instead of incrementing it in memory. This helps us resolve the issue of clients wanting to have unique turn ids for resuming a thread, and knowing the boundry of each turn in rollout files. example debug logs ``` 2026-02-11T00:32:10.746876Z DEBUG codex_app_server_protocol::protocol::thread_history: built turn from rollout items turn_index=8 turn=Turn { id: "019c4a07-d809-74c3-bc4b-fd9618487b4b", items: [UserMessage { id: "item-24", content: [Text { text: "hi", text_elements: [] }] }, AgentMessage { id: "item-25", text: "Hi. I’m in the workspace with your current changes loaded and ready. Send the next task and I’ll execute it end-to-end." }], status: Completed, error: None } 2026-02-11T00:32:10.746888Z DEBUG codex_app_server_protocol::protocol::thread_history: built turn from rollout items turn_index=9 turn=Turn { id: "019c4a18-1004-76c0-a0fb-a77610f6a9b8", items: [UserMessage { id: "item-26", content: [Text { text: "hello", text_elements: [] }] }, AgentMessage { id: "item-27", text: "Hello. Ready for the next change in `codex-rs`; I can continue from the current in-progress diff or start a new task." }], status: Completed, error: None } 2026-02-11T00:32:10.746899Z DEBUG codex_app_server_protocol::protocol::thread_history: built turn from rollout items turn_index=10 turn=Turn { id: "019c4a19-41f0-7db0-ad78-74f1503baeb8", items: [UserMessage { id: "item-28", content: [Text { text: "hello", text_elements: [] }] }, AgentMessage { id: "item-29", text: "Hello. Send the specific change you want in `codex-rs`, and I’ll implement it and run the required checks." }], status: Completed, error: None } ``` backward compatibility: if you try to resume an old session without task_started and task_complete event populated, the following happens: - If you resume and do nothing: those reconstructed historical IDs can differ next time you resume. - If you resume and send a new turn: the new turn gets a fresh UUID from live submission flow and is persisted, so that new turn’s ID is stable on later resumes. I think this behavior is fine, because we only care about deterministic turn id once a turn is triggered. --- .../schema/json/EventMsg.json | 28 ++ .../schema/json/ServerNotification.json | 14 + .../codex_app_server_protocol.schemas.json | 14 + .../json/v1/ForkConversationResponse.json | 14 + .../json/v1/ResumeConversationResponse.json | 14 + .../v1/SessionConfiguredNotification.json | 14 + .../schema/typescript/TurnAbortedEvent.ts | 2 +- .../schema/typescript/TurnCompleteEvent.ts | 2 +- .../schema/typescript/TurnStartedEvent.ts | 2 +- .../src/protocol/thread_history.rs | 304 ++++++++++++++---- codex-rs/app-server-test-client/src/lib.rs | 64 ++++ .../app-server/src/bespoke_event_handling.rs | 10 +- .../app-server/src/codex_message_processor.rs | 70 ++-- .../app-server/tests/suite/send_message.rs | 1 + codex-rs/core/src/agent/control.rs | 3 + codex-rs/core/src/codex.rs | 10 +- codex-rs/core/src/codex_delegate.rs | 5 +- codex-rs/core/src/compact.rs | 2 + codex-rs/core/src/compact_remote.rs | 1 + codex-rs/core/src/rollout/policy.rs | 6 +- codex-rs/core/src/tasks/mod.rs | 10 +- codex-rs/core/src/tasks/user_shell.rs | 1 + codex-rs/core/tests/suite/resume.rs | 14 + codex-rs/core/tests/suite/resume_warning.rs | 1 + .../src/event_processor_with_human_output.rs | 4 +- .../src/event_processor_with_jsonl_output.rs | 1 + .../tests/event_processor_with_json_output.rs | 5 + codex-rs/mcp-server/src/codex_tool_runner.rs | 4 +- codex-rs/protocol/src/protocol.rs | 23 ++ codex-rs/tui/src/chatwidget.rs | 6 +- codex-rs/tui/src/chatwidget/tests.rs | 35 ++ codex-rs/tui/src/lib.rs | 1 + 32 files changed, 558 insertions(+), 127 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/EventMsg.json b/codex-rs/app-server-protocol/schema/json/EventMsg.json index 5991401b84..7151f0d6b6 100644 --- a/codex-rs/app-server-protocol/schema/json/EventMsg.json +++ b/codex-rs/app-server-protocol/schema/json/EventMsg.json @@ -560,6 +560,9 @@ "null" ] }, + "turn_id": { + "type": "string" + }, "type": { "enum": [ "task_started" @@ -569,6 +572,7 @@ } }, "required": [ + "turn_id", "type" ], "title": "TaskStartedEventMsg", @@ -583,6 +587,9 @@ "null" ] }, + "turn_id": { + "type": "string" + }, "type": { "enum": [ "task_complete" @@ -592,6 +599,7 @@ } }, "required": [ + "turn_id", "type" ], "title": "TaskCompleteEventMsg", @@ -2129,6 +2137,12 @@ "reason": { "$ref": "#/definitions/TurnAbortReason" }, + "turn_id": { + "type": [ + "string", + "null" + ] + }, "type": { "enum": [ "turn_aborted" @@ -5303,6 +5317,9 @@ "null" ] }, + "turn_id": { + "type": "string" + }, "type": { "enum": [ "task_started" @@ -5312,6 +5329,7 @@ } }, "required": [ + "turn_id", "type" ], "title": "TaskStartedEventMsg", @@ -5326,6 +5344,9 @@ "null" ] }, + "turn_id": { + "type": "string" + }, "type": { "enum": [ "task_complete" @@ -5335,6 +5356,7 @@ } }, "required": [ + "turn_id", "type" ], "title": "TaskCompleteEventMsg", @@ -6872,6 +6894,12 @@ "reason": { "$ref": "#/definitions/TurnAbortReason" }, + "turn_id": { + "type": [ + "string", + "null" + ] + }, "type": { "enum": [ "turn_aborted" diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index 69c6c2cb9b..39fba5b5ac 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -1204,6 +1204,9 @@ "null" ] }, + "turn_id": { + "type": "string" + }, "type": { "enum": [ "task_started" @@ -1213,6 +1216,7 @@ } }, "required": [ + "turn_id", "type" ], "title": "TaskStartedEventMsg", @@ -1227,6 +1231,9 @@ "null" ] }, + "turn_id": { + "type": "string" + }, "type": { "enum": [ "task_complete" @@ -1236,6 +1243,7 @@ } }, "required": [ + "turn_id", "type" ], "title": "TaskCompleteEventMsg", @@ -2773,6 +2781,12 @@ "reason": { "$ref": "#/definitions/TurnAbortReason" }, + "turn_id": { + "type": [ + "string", + "null" + ] + }, "type": { "enum": [ "turn_aborted" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 88fb5ddbdb..633c485e2a 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -2573,6 +2573,9 @@ "null" ] }, + "turn_id": { + "type": "string" + }, "type": { "enum": [ "task_started" @@ -2582,6 +2585,7 @@ } }, "required": [ + "turn_id", "type" ], "title": "TaskStartedEventMsg", @@ -2596,6 +2600,9 @@ "null" ] }, + "turn_id": { + "type": "string" + }, "type": { "enum": [ "task_complete" @@ -2605,6 +2612,7 @@ } }, "required": [ + "turn_id", "type" ], "title": "TaskCompleteEventMsg", @@ -4142,6 +4150,12 @@ "reason": { "$ref": "#/definitions/TurnAbortReason" }, + "turn_id": { + "type": [ + "string", + "null" + ] + }, "type": { "enum": [ "turn_aborted" diff --git a/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json b/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json index 90949d56f2..027caee1ca 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json @@ -560,6 +560,9 @@ "null" ] }, + "turn_id": { + "type": "string" + }, "type": { "enum": [ "task_started" @@ -569,6 +572,7 @@ } }, "required": [ + "turn_id", "type" ], "title": "TaskStartedEventMsg", @@ -583,6 +587,9 @@ "null" ] }, + "turn_id": { + "type": "string" + }, "type": { "enum": [ "task_complete" @@ -592,6 +599,7 @@ } }, "required": [ + "turn_id", "type" ], "title": "TaskCompleteEventMsg", @@ -2129,6 +2137,12 @@ "reason": { "$ref": "#/definitions/TurnAbortReason" }, + "turn_id": { + "type": [ + "string", + "null" + ] + }, "type": { "enum": [ "turn_aborted" diff --git a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json index b25d2dbed3..e6f3b145a2 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json @@ -560,6 +560,9 @@ "null" ] }, + "turn_id": { + "type": "string" + }, "type": { "enum": [ "task_started" @@ -569,6 +572,7 @@ } }, "required": [ + "turn_id", "type" ], "title": "TaskStartedEventMsg", @@ -583,6 +587,9 @@ "null" ] }, + "turn_id": { + "type": "string" + }, "type": { "enum": [ "task_complete" @@ -592,6 +599,7 @@ } }, "required": [ + "turn_id", "type" ], "title": "TaskCompleteEventMsg", @@ -2129,6 +2137,12 @@ "reason": { "$ref": "#/definitions/TurnAbortReason" }, + "turn_id": { + "type": [ + "string", + "null" + ] + }, "type": { "enum": [ "turn_aborted" diff --git a/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json b/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json index fa9e144c48..1b717eefc4 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json @@ -560,6 +560,9 @@ "null" ] }, + "turn_id": { + "type": "string" + }, "type": { "enum": [ "task_started" @@ -569,6 +572,7 @@ } }, "required": [ + "turn_id", "type" ], "title": "TaskStartedEventMsg", @@ -583,6 +587,9 @@ "null" ] }, + "turn_id": { + "type": "string" + }, "type": { "enum": [ "task_complete" @@ -592,6 +599,7 @@ } }, "required": [ + "turn_id", "type" ], "title": "TaskCompleteEventMsg", @@ -2129,6 +2137,12 @@ "reason": { "$ref": "#/definitions/TurnAbortReason" }, + "turn_id": { + "type": [ + "string", + "null" + ] + }, "type": { "enum": [ "turn_aborted" diff --git a/codex-rs/app-server-protocol/schema/typescript/TurnAbortedEvent.ts b/codex-rs/app-server-protocol/schema/typescript/TurnAbortedEvent.ts index eb0bf24c18..0b4e9075b3 100644 --- a/codex-rs/app-server-protocol/schema/typescript/TurnAbortedEvent.ts +++ b/codex-rs/app-server-protocol/schema/typescript/TurnAbortedEvent.ts @@ -3,4 +3,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { TurnAbortReason } from "./TurnAbortReason"; -export type TurnAbortedEvent = { reason: TurnAbortReason, }; +export type TurnAbortedEvent = { turn_id: string | null, reason: TurnAbortReason, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/TurnCompleteEvent.ts b/codex-rs/app-server-protocol/schema/typescript/TurnCompleteEvent.ts index ab271ba9e3..6987d59f98 100644 --- a/codex-rs/app-server-protocol/schema/typescript/TurnCompleteEvent.ts +++ b/codex-rs/app-server-protocol/schema/typescript/TurnCompleteEvent.ts @@ -2,4 +2,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -export type TurnCompleteEvent = { last_agent_message: string | null, }; +export type TurnCompleteEvent = { turn_id: string, last_agent_message: string | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/TurnStartedEvent.ts b/codex-rs/app-server-protocol/schema/typescript/TurnStartedEvent.ts index 91598aa789..14c0d76707 100644 --- a/codex-rs/app-server-protocol/schema/typescript/TurnStartedEvent.ts +++ b/codex-rs/app-server-protocol/schema/typescript/TurnStartedEvent.ts @@ -3,4 +3,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { ModeKind } from "./ModeKind"; -export type TurnStartedEvent = { model_context_window: bigint | null, collaboration_mode_kind: ModeKind, }; +export type TurnStartedEvent = { turn_id: string, model_context_window: bigint | null, collaboration_mode_kind: ModeKind, }; 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 39efe47655..231ee99b2b 100644 --- a/codex-rs/app-server-protocol/src/protocol/thread_history.rs +++ b/codex-rs/app-server-protocol/src/protocol/thread_history.rs @@ -5,21 +5,25 @@ use crate::protocol::v2::TurnStatus; use crate::protocol::v2::UserInput; use codex_protocol::protocol::AgentReasoningEvent; use codex_protocol::protocol::AgentReasoningRawContentEvent; +use codex_protocol::protocol::CompactedItem; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ItemCompletedEvent; +use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::ThreadRolledBackEvent; use codex_protocol::protocol::TurnAbortedEvent; +use codex_protocol::protocol::TurnCompleteEvent; +use codex_protocol::protocol::TurnStartedEvent; use codex_protocol::protocol::UserMessageEvent; +use uuid::Uuid; -/// Convert persisted [`EventMsg`] entries into a sequence of [`Turn`] values. +/// Convert persisted [`RolloutItem`] entries into a sequence of [`Turn`] values. /// -/// The purpose of this is to convert the EventMsgs persisted in a rollout file -/// into a sequence of Turns and ThreadItems, which allows the client to render -/// the historical messages when resuming a thread. -pub fn build_turns_from_event_msgs(events: &[EventMsg]) -> Vec { +/// When available, this uses `TurnContext.turn_id` as the canonical turn id so +/// resumed/rebuilt thread history preserves the original turn identifiers. +pub fn build_turns_from_rollout_items(items: &[RolloutItem]) -> Vec { let mut builder = ThreadHistoryBuilder::new(); - for event in events { - builder.handle_event(event); + for item in items { + builder.handle_rollout_item(item); } builder.finish() } @@ -27,7 +31,6 @@ pub fn build_turns_from_event_msgs(events: &[EventMsg]) -> Vec { struct ThreadHistoryBuilder { turns: Vec, current_turn: Option, - next_turn_index: i64, next_item_index: i64, } @@ -36,7 +39,6 @@ impl ThreadHistoryBuilder { Self { turns: Vec::new(), current_turn: None, - next_turn_index: 1, next_item_index: 1, } } @@ -63,13 +65,36 @@ impl ThreadHistoryBuilder { EventMsg::ThreadRolledBack(payload) => self.handle_thread_rollback(payload), EventMsg::UndoCompleted(_) => {} EventMsg::TurnAborted(payload) => self.handle_turn_aborted(payload), + EventMsg::TurnStarted(payload) => self.handle_turn_started(payload), + EventMsg::TurnComplete(payload) => self.handle_turn_complete(payload), _ => {} } } + fn handle_rollout_item(&mut self, item: &RolloutItem) { + match item { + RolloutItem::EventMsg(event) => self.handle_event(event), + RolloutItem::Compacted(payload) => self.handle_compacted(payload), + RolloutItem::TurnContext(_) + | RolloutItem::SessionMeta(_) + | RolloutItem::ResponseItem(_) => {} + } + } + fn handle_user_message(&mut self, payload: &UserMessageEvent) { - self.finish_current_turn(); - let mut turn = self.new_turn(); + // User messages should stay in explicitly opened turns. For backward + // compatibility with older streams that did not open turns explicitly, + // close any implicit/inactive turn and start a fresh one for this input. + if let Some(turn) = self.current_turn.as_ref() + && !turn.opened_explicitly + && !(turn.saw_compaction && turn.items.is_empty()) + { + self.finish_current_turn(); + } + let mut turn = self + .current_turn + .take() + .unwrap_or_else(|| self.new_turn(None)); let id = self.next_item_id(); let content = self.build_user_inputs(payload); turn.items.push(ThreadItem::UserMessage { id, content }); @@ -147,6 +172,30 @@ impl ThreadHistoryBuilder { turn.status = TurnStatus::Interrupted; } + fn handle_turn_started(&mut self, payload: &TurnStartedEvent) { + self.finish_current_turn(); + self.current_turn = Some( + self.new_turn(Some(payload.turn_id.clone())) + .opened_explicitly(), + ); + } + + fn handle_turn_complete(&mut self, _payload: &TurnCompleteEvent) { + if let Some(current_turn) = self.current_turn.as_mut() { + current_turn.status = TurnStatus::Completed; + self.finish_current_turn(); + } + } + + /// Marks the current turn as containing a persisted compaction marker. + /// + /// This keeps compaction-only legacy turns from being dropped by + /// `finish_current_turn` when they have no renderable items and were not + /// explicitly opened. + fn handle_compacted(&mut self, _payload: &CompactedItem) { + self.ensure_turn().saw_compaction = true; + } + fn handle_thread_rollback(&mut self, payload: &ThreadRolledBackEvent) { self.finish_current_turn(); @@ -157,34 +206,33 @@ impl ThreadHistoryBuilder { 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() { + if turn.items.is_empty() && !turn.opened_explicitly && !turn.saw_compaction { return; } self.turns.push(turn.into()); } } - fn new_turn(&mut self) -> PendingTurn { + fn new_turn(&mut self, id: Option) -> PendingTurn { PendingTurn { - id: self.next_turn_id(), + id: id.unwrap_or_else(|| Uuid::now_v7().to_string()), items: Vec::new(), error: None, status: TurnStatus::Completed, + opened_explicitly: false, + saw_compaction: false, } } fn ensure_turn(&mut self) -> &mut PendingTurn { if self.current_turn.is_none() { - let turn = self.new_turn(); + let turn = self.new_turn(None); return self.current_turn.insert(turn); } @@ -195,12 +243,6 @@ impl ThreadHistoryBuilder { unreachable!("current turn must exist after initialization"); } - fn next_turn_id(&mut self) -> String { - let id = format!("turn-{}", self.next_turn_index); - self.next_turn_index += 1; - id - } - fn next_item_id(&mut self) -> String { let id = format!("item-{}", self.next_item_index); self.next_item_index += 1; @@ -237,6 +279,19 @@ struct PendingTurn { items: Vec, error: Option, status: TurnStatus, + /// True when this turn originated from an explicit `turn_started`/`turn_complete` + /// boundary, so we preserve it even if it has no renderable items. + opened_explicitly: bool, + /// True when this turn includes a persisted `RolloutItem::Compacted`, which + /// should keep the turn from being dropped even without normal items. + saw_compaction: bool, +} + +impl PendingTurn { + fn opened_explicitly(mut self) -> Self { + self.opened_explicitly = true; + self + } } impl From for Turn { @@ -256,11 +311,15 @@ mod tests { use codex_protocol::protocol::AgentMessageEvent; use codex_protocol::protocol::AgentReasoningEvent; use codex_protocol::protocol::AgentReasoningRawContentEvent; + use codex_protocol::protocol::CompactedItem; use codex_protocol::protocol::ThreadRolledBackEvent; use codex_protocol::protocol::TurnAbortReason; use codex_protocol::protocol::TurnAbortedEvent; + use codex_protocol::protocol::TurnCompleteEvent; + use codex_protocol::protocol::TurnStartedEvent; use codex_protocol::protocol::UserMessageEvent; use pretty_assertions::assert_eq; + use uuid::Uuid; #[test] fn builds_multiple_turns_with_reasoning_items() { @@ -291,11 +350,15 @@ mod tests { }), ]; - let turns = build_turns_from_event_msgs(&events); + let items = events + .into_iter() + .map(RolloutItem::EventMsg) + .collect::>(); + let turns = build_turns_from_rollout_items(&items); assert_eq!(turns.len(), 2); let first = &turns[0]; - assert_eq!(first.id, "turn-1"); + assert!(Uuid::parse_str(&first.id).is_ok()); assert_eq!(first.status, TurnStatus::Completed); assert_eq!(first.items.len(), 3); assert_eq!( @@ -330,7 +393,8 @@ mod tests { ); let second = &turns[1]; - assert_eq!(second.id, "turn-2"); + assert!(Uuid::parse_str(&second.id).is_ok()); + assert_ne!(first.id, second.id); assert_eq!(second.items.len(), 2); assert_eq!( second.items[0], @@ -374,7 +438,11 @@ mod tests { }), ]; - let turns = build_turns_from_event_msgs(&events); + let items = events + .into_iter() + .map(RolloutItem::EventMsg) + .collect::>(); + let turns = build_turns_from_rollout_items(&items); assert_eq!(turns.len(), 1); let turn = &turns[0]; assert_eq!(turn.items.len(), 4); @@ -410,6 +478,7 @@ mod tests { message: "Working...".into(), }), EventMsg::TurnAborted(TurnAbortedEvent { + turn_id: Some("turn-1".into()), reason: TurnAbortReason::Replaced, }), EventMsg::UserMessage(UserMessageEvent { @@ -423,7 +492,11 @@ mod tests { }), ]; - let turns = build_turns_from_event_msgs(&events); + let items = events + .into_iter() + .map(RolloutItem::EventMsg) + .collect::>(); + let turns = build_turns_from_rollout_items(&items); assert_eq!(turns.len(), 2); let first_turn = &turns[0]; @@ -502,46 +575,49 @@ mod tests { }), ]; - 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(), - text_elements: Vec::new(), - }], - }, - 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(), - text_elements: Vec::new(), - }], - }, - ThreadItem::AgentMessage { - id: "item-4".into(), - text: "A3".into(), - }, - ], - }, - ]; - assert_eq!(turns, expected); + let items = events + .into_iter() + .map(RolloutItem::EventMsg) + .collect::>(); + let turns = build_turns_from_rollout_items(&items); + assert_eq!(turns.len(), 2); + assert!(Uuid::parse_str(&turns[0].id).is_ok()); + assert!(Uuid::parse_str(&turns[1].id).is_ok()); + assert_ne!(turns[0].id, turns[1].id); + assert_eq!(turns[0].status, TurnStatus::Completed); + assert_eq!(turns[1].status, TurnStatus::Completed); + assert_eq!( + turns[0].items, + vec![ + ThreadItem::UserMessage { + id: "item-1".into(), + content: vec![UserInput::Text { + text: "First".into(), + text_elements: Vec::new(), + }], + }, + ThreadItem::AgentMessage { + id: "item-2".into(), + text: "A1".into(), + }, + ] + ); + assert_eq!( + turns[1].items, + vec![ + ThreadItem::UserMessage { + id: "item-3".into(), + content: vec![UserInput::Text { + text: "Third".into(), + text_elements: Vec::new(), + }], + }, + ThreadItem::AgentMessage { + id: "item-4".into(), + text: "A3".into(), + }, + ] + ); } #[test] @@ -568,7 +644,95 @@ mod tests { EventMsg::ThreadRolledBack(ThreadRolledBackEvent { num_turns: 99 }), ]; - let turns = build_turns_from_event_msgs(&events); + let items = events + .into_iter() + .map(RolloutItem::EventMsg) + .collect::>(); + let turns = build_turns_from_rollout_items(&items); assert_eq!(turns, Vec::::new()); } + + #[test] + fn uses_explicit_turn_boundaries_for_mid_turn_steering() { + let events = vec![ + EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-a".into(), + model_context_window: None, + collaboration_mode_kind: Default::default(), + }), + EventMsg::UserMessage(UserMessageEvent { + message: "Start".into(), + images: None, + text_elements: Vec::new(), + local_images: Vec::new(), + }), + EventMsg::UserMessage(UserMessageEvent { + message: "Steer".into(), + images: None, + text_elements: Vec::new(), + local_images: Vec::new(), + }), + EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: "turn-a".into(), + last_agent_message: None, + }), + ]; + + let items = events + .into_iter() + .map(RolloutItem::EventMsg) + .collect::>(); + let turns = build_turns_from_rollout_items(&items); + assert_eq!(turns.len(), 1); + assert_eq!(turns[0].id, "turn-a"); + assert_eq!( + turns[0].items, + vec![ + ThreadItem::UserMessage { + id: "item-1".into(), + content: vec![UserInput::Text { + text: "Start".into(), + text_elements: Vec::new(), + }], + }, + ThreadItem::UserMessage { + id: "item-2".into(), + content: vec![UserInput::Text { + text: "Steer".into(), + text_elements: Vec::new(), + }], + }, + ] + ); + } + + #[test] + fn preserves_compaction_only_turn() { + let items = vec![ + RolloutItem::EventMsg(EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-compact".into(), + model_context_window: None, + collaboration_mode_kind: Default::default(), + })), + RolloutItem::Compacted(CompactedItem { + message: String::new(), + replacement_history: None, + }), + RolloutItem::EventMsg(EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: "turn-compact".into(), + last_agent_message: None, + })), + ]; + + let turns = build_turns_from_rollout_items(&items); + assert_eq!( + turns, + vec![Turn { + id: "turn-compact".into(), + status: TurnStatus::Completed, + error: None, + items: Vec::new(), + }] + ); + } } diff --git a/codex-rs/app-server-test-client/src/lib.rs b/codex-rs/app-server-test-client/src/lib.rs index 415f45e4a1..4c41525434 100644 --- a/codex-rs/app-server-test-client/src/lib.rs +++ b/codex-rs/app-server-test-client/src/lib.rs @@ -52,6 +52,8 @@ use codex_app_server_protocol::SendUserMessageParams; use codex_app_server_protocol::SendUserMessageResponse; use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ServerRequest; +use codex_app_server_protocol::ThreadResumeParams; +use codex_app_server_protocol::ThreadResumeResponse; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; use codex_app_server_protocol::TurnStartParams; @@ -112,6 +114,13 @@ enum CliCommand { /// User message to send to Codex. user_message: String, }, + /// Resume a V2 thread by id, then send a user message. + ResumeMessageV2 { + /// Existing thread id to resume. + thread_id: String, + /// User message to send to Codex. + user_message: String, + }, /// Start a V2 turn that elicits an ExecCommand approval. #[command(name = "trigger-cmd-approval")] TriggerCmdApproval { @@ -161,6 +170,16 @@ pub fn run() -> Result<()> { CliCommand::SendMessageV2 { user_message } => { send_message_v2(&codex_bin, &config_overrides, user_message, &dynamic_tools) } + CliCommand::ResumeMessageV2 { + thread_id, + user_message, + } => resume_message_v2( + &codex_bin, + &config_overrides, + thread_id, + user_message, + &dynamic_tools, + ), CliCommand::TriggerCmdApproval { user_message } => { trigger_cmd_approval(&codex_bin, &config_overrides, user_message, &dynamic_tools) } @@ -233,6 +252,41 @@ pub fn send_message_v2( ) } +fn resume_message_v2( + codex_bin: &Path, + config_overrides: &[String], + thread_id: String, + user_message: String, + dynamic_tools: &Option>, +) -> Result<()> { + ensure_dynamic_tools_unused(dynamic_tools, "resume-message-v2")?; + + let mut client = CodexClient::spawn(codex_bin, config_overrides)?; + + let initialize = client.initialize()?; + println!("< initialize response: {initialize:?}"); + + let resume_response = client.thread_resume(ThreadResumeParams { + thread_id, + ..Default::default() + })?; + println!("< thread/resume response: {resume_response:?}"); + + let turn_response = client.turn_start(TurnStartParams { + thread_id: resume_response.thread.id.clone(), + input: vec![V2UserInput::Text { + text: user_message, + text_elements: Vec::new(), + }], + ..Default::default() + })?; + println!("< turn/start response: {turn_response:?}"); + + client.stream_turn(&resume_response.thread.id, &turn_response.turn.id)?; + + Ok(()) +} + fn trigger_cmd_approval( codex_bin: &Path, config_overrides: &[String], @@ -592,6 +646,16 @@ impl CodexClient { self.send_request(request, request_id, "thread/start") } + fn thread_resume(&mut self, params: ThreadResumeParams) -> Result { + let request_id = self.request_id(); + let request = ClientRequest::ThreadResume { + request_id: request_id.clone(), + params, + }; + + self.send_request(request, request_id, "thread/resume") + } + fn turn_start(&mut self, params: TurnStartParams) -> Result { let request_id = self.request_id(); let request = ClientRequest::TurnStart { diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 73647b1c08..4f0c526f7d 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -3,7 +3,7 @@ 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_rollout_items_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; @@ -69,7 +69,7 @@ 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_app_server_protocol::build_turns_from_rollout_items; use codex_core::CodexThread; use codex_core::parse_command::shlex_join; use codex_core::protocol::ApplyPatchApprovalRequestEvent; @@ -1101,9 +1101,9 @@ pub(crate) async fn apply_bespoke_event_handling( { 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); + match read_rollout_items_from_rollout(rollout_path.as_path()).await { + Ok(items) => { + thread.turns = build_turns_from_rollout_items(&items); ThreadRollbackResponse { thread } } Err(err) => { diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index e8b921980f..61507d79d8 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -147,7 +147,7 @@ use codex_app_server_protocol::TurnSteerResponse; use codex_app_server_protocol::UserInfoResponse; use codex_app_server_protocol::UserInput as V2UserInput; use codex_app_server_protocol::UserSavedConfig; -use codex_app_server_protocol::build_turns_from_event_msgs; +use codex_app_server_protocol::build_turns_from_rollout_items; use codex_backend_client::Client as BackendClient; use codex_chatgpt::connectors; use codex_cloud_requirements::cloud_requirements_loader; @@ -2445,9 +2445,9 @@ impl CodexMessageProcessor { }; if include_turns && let Some(rollout_path) = rollout_path.as_ref() { - match read_event_msgs_from_rollout(rollout_path).await { - Ok(events) => { - thread.turns = build_turns_from_event_msgs(&events); + match read_rollout_items_from_rollout(rollout_path).await { + Ok(items) => { + thread.turns = build_turns_from_rollout_items(&items); } Err(err) if err.kind() == std::io::ErrorKind::NotFound => { self.send_invalid_request_error( @@ -2639,11 +2639,7 @@ impl CodexMessageProcessor { session_configured, .. }) => { - let SessionConfiguredEvent { - rollout_path, - initial_messages, - .. - } = session_configured; + let SessionConfiguredEvent { rollout_path, .. } = session_configured; let Some(rollout_path) = rollout_path else { self.send_internal_error( request_id, @@ -2683,9 +2679,22 @@ impl CodexMessageProcessor { return; } }; - thread.turns = initial_messages - .as_deref() - .map_or_else(Vec::new, build_turns_from_event_msgs); + match read_rollout_items_from_rollout(rollout_path.as_path()).await { + Ok(items) => { + thread.turns = build_turns_from_rollout_items(&items); + } + Err(err) => { + self.send_internal_error( + request_id, + format!( + "failed to load rollout `{}` for thread {thread_id}: {err}", + rollout_path.display() + ), + ) + .await; + return; + } + } let response = ThreadResumeResponse { thread, @@ -2847,11 +2856,7 @@ impl CodexMessageProcessor { } }; - let SessionConfiguredEvent { - rollout_path, - initial_messages, - .. - } = session_configured; + let SessionConfiguredEvent { rollout_path, .. } = session_configured; let Some(rollout_path) = rollout_path else { self.send_internal_error( request_id, @@ -2891,9 +2896,22 @@ impl CodexMessageProcessor { return; } }; - thread.turns = initial_messages - .as_deref() - .map_or_else(Vec::new, build_turns_from_event_msgs); + match read_rollout_items_from_rollout(rollout_path.as_path()).await { + Ok(items) => { + thread.turns = build_turns_from_rollout_items(&items); + } + Err(err) => { + self.send_internal_error( + request_id, + format!( + "failed to load rollout `{}` for thread {thread_id}: {err}", + rollout_path.display() + ), + ) + .await; + return; + } + } let response = ThreadForkResponse { thread: thread.clone(), @@ -5779,22 +5797,16 @@ pub(crate) async fn read_summary_from_rollout( }) } -pub(crate) async fn read_event_msgs_from_rollout( +pub(crate) async fn read_rollout_items_from_rollout( path: &Path, -) -> std::io::Result> { +) -> 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()) + Ok(items) } fn extract_conversation_summary( diff --git a/codex-rs/app-server/tests/suite/send_message.rs b/codex-rs/app-server/tests/suite/send_message.rs index ecb742aff0..66032da20b 100644 --- a/codex-rs/app-server/tests/suite/send_message.rs +++ b/codex-rs/app-server/tests/suite/send_message.rs @@ -560,6 +560,7 @@ fn append_rollout_turn_context(path: &Path, timestamp: &str, model: &str) -> std let line = RolloutLine { timestamp: timestamp.to_string(), item: RolloutItem::TurnContext(TurnContextItem { + turn_id: None, cwd: PathBuf::from("/"), approval_policy: AskForApproval::Never, sandbox_policy: SandboxPolicy::DangerFullAccess, diff --git a/codex-rs/core/src/agent/control.rs b/codex-rs/core/src/agent/control.rs index 9334a0764d..c2dc3b8f88 100644 --- a/codex-rs/core/src/agent/control.rs +++ b/codex-rs/core/src/agent/control.rs @@ -268,6 +268,7 @@ mod tests { #[tokio::test] async fn on_event_updates_status_from_task_started() { let status = agent_status_from_event(&EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), model_context_window: None, collaboration_mode_kind: ModeKind::Default, })); @@ -277,6 +278,7 @@ mod tests { #[tokio::test] async fn on_event_updates_status_from_task_complete() { let status = agent_status_from_event(&EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: "turn-1".to_string(), last_agent_message: Some("done".to_string()), })); let expected = AgentStatus::Completed(Some("done".to_string())); @@ -297,6 +299,7 @@ mod tests { #[tokio::test] async fn on_event_updates_status_from_turn_aborted() { let status = agent_status_from_event(&EventMsg::TurnAborted(TurnAbortedEvent { + turn_id: Some("turn-1".to_string()), reason: TurnAbortReason::Interrupted, })); diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 7ee094b566..b64eedbe4a 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -104,6 +104,7 @@ use tracing::instrument; use tracing::trace; use tracing::trace_span; use tracing::warn; +use uuid::Uuid; use crate::ModelProviderInfo; use crate::client::ModelClient; @@ -248,7 +249,6 @@ use codex_utils_readiness::ReadinessFlag; /// The high-level interface to the Codex system. /// It operates as a queue pair where you send submissions and receive events. pub struct Codex { - pub(crate) next_id: AtomicU64, pub(crate) tx_sub: Sender, pub(crate) rx_event: Receiver, // Last known status of the agent. @@ -429,7 +429,6 @@ impl Codex { submission_loop(Arc::clone(&session), config, rx_sub).instrument(session_loop_span), ); let codex = Codex { - next_id: AtomicU64::new(0), tx_sub, rx_event, agent_status: agent_status_rx, @@ -446,10 +445,7 @@ impl Codex { /// Submit the `op` wrapped in a `Submission` with a unique ID. pub async fn submit(&self, op: Op) -> CodexResult { - let id = self - .next_id - .fetch_add(1, std::sync::atomic::Ordering::SeqCst) - .to_string(); + let id = Uuid::now_v7().to_string(); let sub = Submission { id: id.clone(), op }; self.submit_with_id(sub).await?; Ok(id) @@ -3869,6 +3865,7 @@ pub(crate) async fn run_turn( let total_usage_tokens = sess.get_total_token_usage().await; let event = EventMsg::TurnStarted(TurnStartedEvent { + turn_id: turn_context.sub_id.clone(), model_context_window: turn_context.model_context_window(), collaboration_mode_kind: turn_context.collaboration_mode.mode, }); @@ -4822,6 +4819,7 @@ async fn try_run_sampling_request( ) -> CodexResult { let collaboration_mode = sess.current_collaboration_mode().await; let rollout_item = RolloutItem::TurnContext(TurnContextItem { + turn_id: Some(turn_context.sub_id.clone()), cwd: turn_context.cwd.clone(), approval_policy: turn_context.approval_policy, sandbox_policy: turn_context.sandbox_policy.clone(), diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index b9af1dd5f3..504b876bbf 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -1,6 +1,5 @@ use std::collections::HashMap; use std::sync::Arc; -use std::sync::atomic::AtomicU64; use async_channel::Receiver; use async_channel::Sender; @@ -90,7 +89,6 @@ pub(crate) async fn run_codex_thread_interactive( }); Ok(Codex { - next_id: AtomicU64::new(0), tx_sub: tx_ops, rx_event: rx_sub, agent_status: codex.agent_status.clone(), @@ -166,7 +164,6 @@ pub(crate) async fn run_codex_thread_one_shot( drop(rx_closed); Ok(Codex { - next_id: AtomicU64::new(0), rx_event: rx_bridge, tx_sub: tx_closed, agent_status, @@ -470,7 +467,6 @@ mod tests { let (_agent_status_tx, agent_status) = watch::channel(AgentStatus::PendingInit); let (session, ctx, _rx_evt) = crate::codex::make_session_and_context_with_rx().await; let codex = Arc::new(Codex { - next_id: AtomicU64::new(0), tx_sub, rx_event: rx_events, agent_status, @@ -482,6 +478,7 @@ mod tests { .send(Event { id: "full".to_string(), msg: EventMsg::TurnAborted(TurnAbortedEvent { + turn_id: Some("turn-1".to_string()), reason: TurnAbortReason::Interrupted, }), }) diff --git a/codex-rs/core/src/compact.rs b/codex-rs/core/src/compact.rs index ed8abfc546..f7de93dfe5 100644 --- a/codex-rs/core/src/compact.rs +++ b/codex-rs/core/src/compact.rs @@ -57,6 +57,7 @@ pub(crate) async fn run_compact_task( input: Vec, ) -> CodexResult<()> { let start_event = EventMsg::TurnStarted(TurnStartedEvent { + turn_id: turn_context.sub_id.clone(), model_context_window: turn_context.model_context_window(), collaboration_mode_kind: turn_context.collaboration_mode.mode, }); @@ -95,6 +96,7 @@ async fn run_compact_task_inner( // session config before this write occurs. let collaboration_mode = sess.current_collaboration_mode().await; let rollout_item = RolloutItem::TurnContext(TurnContextItem { + turn_id: Some(turn_context.sub_id.clone()), cwd: turn_context.cwd.clone(), approval_policy: turn_context.approval_policy, sandbox_policy: turn_context.sandbox_policy.clone(), diff --git a/codex-rs/core/src/compact_remote.rs b/codex-rs/core/src/compact_remote.rs index 6af43dddfd..4fe07560b9 100644 --- a/codex-rs/core/src/compact_remote.rs +++ b/codex-rs/core/src/compact_remote.rs @@ -34,6 +34,7 @@ pub(crate) async fn run_remote_compact_task( turn_context: Arc, ) -> CodexResult<()> { let start_event = EventMsg::TurnStarted(TurnStartedEvent { + turn_id: turn_context.sub_id.clone(), model_context_window: turn_context.model_context_window(), collaboration_mode_kind: turn_context.collaboration_mode.mode, }); diff --git a/codex-rs/core/src/rollout/policy.rs b/codex-rs/core/src/rollout/policy.rs index 5043c9ddf5..edc3b941c3 100644 --- a/codex-rs/core/src/rollout/policy.rs +++ b/codex-rs/core/src/rollout/policy.rs @@ -47,7 +47,9 @@ pub(crate) fn should_persist_event_msg(ev: &EventMsg) -> bool { | EventMsg::ExitedReviewMode(_) | EventMsg::ThreadRolledBack(_) | EventMsg::UndoCompleted(_) - | EventMsg::TurnAborted(_) => true, + | EventMsg::TurnAborted(_) + | EventMsg::TurnStarted(_) + | EventMsg::TurnComplete(_) => true, EventMsg::ItemCompleted(event) => { // Plan items are derived from streaming tags and are not part of the // raw ResponseItem history, so we persist their completion to replay @@ -56,8 +58,6 @@ pub(crate) fn should_persist_event_msg(ev: &EventMsg) -> bool { } EventMsg::Error(_) | EventMsg::Warning(_) - | EventMsg::TurnStarted(_) - | EventMsg::TurnComplete(_) | EventMsg::AgentMessageDelta(_) | EventMsg::AgentReasoningDelta(_) | EventMsg::AgentReasoningRawContentDelta(_) diff --git a/codex-rs/core/src/tasks/mod.rs b/codex-rs/core/src/tasks/mod.rs index cbb6cd4b37..9a3a4756ab 100644 --- a/codex-rs/core/src/tasks/mod.rs +++ b/codex-rs/core/src/tasks/mod.rs @@ -214,7 +214,10 @@ impl Session { self.record_conversation_items(turn_context.as_ref(), &pending_response_items) .await; } - let event = EventMsg::TurnComplete(TurnCompleteEvent { last_agent_message }); + let event = EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: turn_context.sub_id.clone(), + last_agent_message, + }); self.send_event(turn_context.as_ref(), event).await; } @@ -290,7 +293,10 @@ impl Session { self.flush_rollout().await; } - let event = EventMsg::TurnAborted(TurnAbortedEvent { reason }); + let event = EventMsg::TurnAborted(TurnAbortedEvent { + turn_id: Some(task.turn_context.sub_id.clone()), + reason, + }); self.send_event(task.turn_context.as_ref(), event).await; } } diff --git a/codex-rs/core/src/tasks/user_shell.rs b/codex-rs/core/src/tasks/user_shell.rs index b0c505b64a..ef6c7180a9 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -101,6 +101,7 @@ pub(crate) async fn execute_user_shell_command( // emitted TurnStarted, so emitting another TurnStarted here would create // duplicate turn lifecycle events and confuse clients. let event = EventMsg::TurnStarted(TurnStartedEvent { + turn_id: turn_context.sub_id.clone(), model_context_window: turn_context.model_context_window(), collaboration_mode_kind: turn_context.collaboration_mode.mode, }); diff --git a/codex-rs/core/tests/suite/resume.rs b/codex-rs/core/tests/suite/resume.rs index c0bdcd4fe2..261ec58089 100644 --- a/codex-rs/core/tests/suite/resume.rs +++ b/codex-rs/core/tests/suite/resume.rs @@ -64,14 +64,21 @@ async fn resume_includes_initial_messages_from_rollout_events() -> Result<()> { .expect("expected initial messages to be present for resumed session"); match initial_messages.as_slice() { [ + EventMsg::TurnStarted(started), EventMsg::UserMessage(first_user), EventMsg::TokenCount(_), EventMsg::AgentMessage(assistant_message), EventMsg::TokenCount(_), + EventMsg::TurnComplete(completed), ] => { assert_eq!(first_user.message, "Record some messages"); assert_eq!(first_user.text_elements, text_elements); assert_eq!(assistant_message.message, "Completed first turn"); + assert_eq!(completed.turn_id, started.turn_id); + assert_eq!( + completed.last_agent_message.as_deref(), + Some("Completed first turn") + ); } other => panic!("unexpected initial messages after resume: {other:#?}"), } @@ -123,17 +130,24 @@ async fn resume_includes_initial_messages_from_reasoning_events() -> Result<()> .expect("expected initial messages to be present for resumed session"); match initial_messages.as_slice() { [ + EventMsg::TurnStarted(started), EventMsg::UserMessage(first_user), EventMsg::TokenCount(_), EventMsg::AgentReasoning(reasoning), EventMsg::AgentReasoningRawContent(raw), EventMsg::AgentMessage(assistant_message), EventMsg::TokenCount(_), + EventMsg::TurnComplete(completed), ] => { assert_eq!(first_user.message, "Record reasoning messages"); assert_eq!(reasoning.text, "Summarized step"); assert_eq!(raw.text, "raw detail"); assert_eq!(assistant_message.message, "Completed reasoning turn"); + assert_eq!(completed.turn_id, started.turn_id); + assert_eq!( + completed.last_agent_message.as_deref(), + Some("Completed reasoning turn") + ); } other => panic!("unexpected initial messages after resume: {other:#?}"), } diff --git a/codex-rs/core/tests/suite/resume_warning.rs b/codex-rs/core/tests/suite/resume_warning.rs index 5e5fc6d74a..a91013bedb 100644 --- a/codex-rs/core/tests/suite/resume_warning.rs +++ b/codex-rs/core/tests/suite/resume_warning.rs @@ -22,6 +22,7 @@ fn resume_history( rollout_path: &std::path::Path, ) -> InitialHistory { let turn_ctx = TurnContextItem { + turn_id: None, cwd: config.cwd.clone(), approval_policy: config.approval_policy.value(), sandbox_policy: config.sandbox_policy.get().clone(), 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 6b154b1799..f9edc36627 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -264,7 +264,9 @@ impl EventProcessor for EventProcessorWithHumanOutput { "auto-cancelling (not supported in exec mode)".style(self.dimmed) ); } - EventMsg::TurnComplete(TurnCompleteEvent { last_agent_message }) => { + EventMsg::TurnComplete(TurnCompleteEvent { + last_agent_message, .. + }) => { let last_message = last_agent_message .as_deref() .or(self.last_proposed_plan.as_deref()); diff --git a/codex-rs/exec/src/event_processor_with_jsonl_output.rs b/codex-rs/exec/src/event_processor_with_jsonl_output.rs index 9675651ef7..6cefcbd940 100644 --- a/codex-rs/exec/src/event_processor_with_jsonl_output.rs +++ b/codex-rs/exec/src/event_processor_with_jsonl_output.rs @@ -862,6 +862,7 @@ impl EventProcessor for EventProcessorWithJsonOutput { match msg { protocol::EventMsg::TurnComplete(protocol::TurnCompleteEvent { last_agent_message, + .. }) => { if let Some(output_file) = self.last_message_path.as_deref() { let last_message = last_agent_message diff --git a/codex-rs/exec/tests/event_processor_with_json_output.rs b/codex-rs/exec/tests/event_processor_with_json_output.rs index f349e4835f..4e669d232e 100644 --- a/codex-rs/exec/tests/event_processor_with_json_output.rs +++ b/codex-rs/exec/tests/event_processor_with_json_output.rs @@ -117,6 +117,7 @@ fn task_started_produces_turn_started_event() { let out = ep.collect_thread_events(&event( "t1", EventMsg::TurnStarted(codex_core::protocol::TurnStartedEvent { + turn_id: "turn-1".to_string(), model_context_window: Some(32_000), collaboration_mode_kind: ModeKind::Default, }), @@ -310,6 +311,7 @@ fn plan_update_emits_todo_list_started_updated_and_completed() { let complete = event( "p3", EventMsg::TurnComplete(codex_core::protocol::TurnCompleteEvent { + turn_id: "turn-1".to_string(), last_agent_message: None, }), ); @@ -677,6 +679,7 @@ fn plan_update_after_complete_starts_new_todo_list_with_new_id() { let complete = event( "t2", EventMsg::TurnComplete(codex_core::protocol::TurnCompleteEvent { + turn_id: "turn-1".to_string(), last_agent_message: None, }), ); @@ -829,6 +832,7 @@ fn error_followed_by_task_complete_produces_turn_failed() { let complete_event = event( "e2", EventMsg::TurnComplete(codex_core::protocol::TurnCompleteEvent { + turn_id: "turn-1".to_string(), last_agent_message: None, }), ); @@ -1276,6 +1280,7 @@ fn task_complete_produces_turn_completed_with_usage() { let complete_event = event( "e2", EventMsg::TurnComplete(codex_core::protocol::TurnCompleteEvent { + turn_id: "turn-1".to_string(), last_agent_message: Some("done".to_string()), }), ); diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index 6f6d2ec782..6c5b3c25a2 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -279,7 +279,9 @@ async fn run_codex_tool_session_inner( .await; continue; } - EventMsg::TurnComplete(TurnCompleteEvent { last_agent_message }) => { + EventMsg::TurnComplete(TurnCompleteEvent { + last_agent_message, .. + }) => { let text = match last_agent_message { Some(msg) => msg, None => "".to_string(), diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 2368a39ba3..a56f55ba9a 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1152,11 +1152,13 @@ pub struct ContextCompactedEvent; #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct TurnCompleteEvent { + pub turn_id: String, pub last_agent_message: Option, } #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct TurnStartedEvent { + pub turn_id: String, // TODO(aibrahim): make this not optional pub model_context_window: Option, #[serde(default)] @@ -1740,6 +1742,8 @@ impl From for ResponseItem { #[derive(Serialize, Deserialize, Clone, Debug, JsonSchema, TS)] pub struct TurnContextItem { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub turn_id: Option, pub cwd: PathBuf, pub approval_policy: AskForApproval, pub sandbox_policy: SandboxPolicy, @@ -2402,6 +2406,7 @@ pub struct Chunk { #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct TurnAbortedEvent { + pub turn_id: Option, pub reason: TurnAbortReason, } @@ -2690,6 +2695,24 @@ mod tests { Ok(()) } + #[test] + fn turn_aborted_event_deserializes_without_turn_id() -> Result<()> { + let event: EventMsg = serde_json::from_value(json!({ + "type": "turn_aborted", + "reason": "interrupted", + }))?; + + match event { + EventMsg::TurnAborted(TurnAbortedEvent { turn_id, reason }) => { + assert_eq!(turn_id, None); + assert_eq!(reason, TurnAbortReason::Interrupted); + } + _ => panic!("expected turn_aborted event"), + } + + Ok(()) + } + /// Serialize Event to verify that its JSON representation has the expected /// amount of nesting. #[test] diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 9920e8d1b5..18c4b9b162 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -3929,9 +3929,9 @@ impl ChatWidget { } EventMsg::AgentReasoningSectionBreak(_) => self.on_reasoning_section_break(), EventMsg::TurnStarted(_) => self.on_task_started(), - EventMsg::TurnComplete(TurnCompleteEvent { last_agent_message }) => { - self.on_task_complete(last_agent_message, from_replay) - } + EventMsg::TurnComplete(TurnCompleteEvent { + last_agent_message, .. + }) => self.on_task_complete(last_agent_message, from_replay), EventMsg::TokenCount(ev) => { self.set_token_info(ev.info); self.on_rate_limit_snapshot(ev.rate_limits); diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 280db452f4..6fdefb36c4 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -589,6 +589,7 @@ async fn interrupted_turn_restores_queued_messages_with_images_and_elements() { chat.handle_codex_event(Event { id: "interrupt".into(), msg: EventMsg::TurnAborted(codex_core::protocol::TurnAbortedEvent { + turn_id: Some("turn-1".to_string()), reason: TurnAbortReason::Interrupted, }), }); @@ -652,6 +653,7 @@ async fn interrupted_turn_restore_keeps_active_mode_for_resubmission() { chat.handle_codex_event(Event { id: "interrupt".into(), msg: EventMsg::TurnAborted(codex_core::protocol::TurnAbortedEvent { + turn_id: Some("turn-1".to_string()), reason: TurnAbortReason::Interrupted, }), }); @@ -1627,6 +1629,7 @@ async fn plan_implementation_popup_skips_replayed_turn_complete() { chat.set_collaboration_mask(plan_mask); chat.replay_initial_messages(vec![EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: "turn-1".to_string(), last_agent_message: Some("Plan details".to_string()), })]); @@ -1651,6 +1654,7 @@ async fn plan_implementation_popup_shows_once_when_replay_precedes_live_turn_com chat.on_plan_item_completed("- Step 1\n- Step 2\n".to_string()); chat.replay_initial_messages(vec![EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: "turn-1".to_string(), last_agent_message: Some("Plan details".to_string()), })]); let replay_popup = render_bottom_popup(&chat, 80); @@ -1662,6 +1666,7 @@ async fn plan_implementation_popup_shows_once_when_replay_precedes_live_turn_com chat.handle_codex_event(Event { id: "live-turn-complete-1".to_string(), msg: EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: "turn-1".to_string(), last_agent_message: Some("Plan details".to_string()), }), }); @@ -1682,6 +1687,7 @@ async fn plan_implementation_popup_shows_once_when_replay_precedes_live_turn_com chat.handle_codex_event(Event { id: "live-turn-complete-2".to_string(), msg: EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: "turn-1".to_string(), last_agent_message: Some("Plan details".to_string()), }), }); @@ -2646,6 +2652,7 @@ async fn unified_exec_wait_after_final_agent_message_snapshot() { chat.handle_codex_event(Event { id: "turn-1".into(), msg: EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), model_context_window: None, collaboration_mode_kind: ModeKind::Default, }), @@ -2663,6 +2670,7 @@ async fn unified_exec_wait_after_final_agent_message_snapshot() { chat.handle_codex_event(Event { id: "turn-1".into(), msg: EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: "turn-1".to_string(), last_agent_message: Some("Final response.".into()), }), }); @@ -2681,6 +2689,7 @@ async fn unified_exec_wait_before_streamed_agent_message_snapshot() { chat.handle_codex_event(Event { id: "turn-1".into(), msg: EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), model_context_window: None, collaboration_mode_kind: ModeKind::Default, }), @@ -2703,6 +2712,7 @@ async fn unified_exec_wait_before_streamed_agent_message_snapshot() { chat.handle_codex_event(Event { id: "turn-1".into(), msg: EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: "turn-1".to_string(), last_agent_message: None, }), }); @@ -2755,6 +2765,7 @@ async fn unified_exec_waiting_multiple_empty_snapshots() { chat.handle_codex_event(Event { id: "turn-wait-1".into(), msg: EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: "turn-1".to_string(), last_agent_message: None, }), }); @@ -2806,6 +2817,7 @@ async fn unified_exec_non_empty_then_empty_snapshots() { chat.handle_codex_event(Event { id: "turn-wait-3".into(), msg: EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: "turn-1".to_string(), last_agent_message: None, }), }); @@ -3543,6 +3555,7 @@ async fn interrupt_exec_marks_failed_snapshot() { chat.handle_codex_event(Event { id: "call-int".into(), msg: EventMsg::TurnAborted(codex_core::protocol::TurnAbortedEvent { + turn_id: Some("turn-1".to_string()), reason: TurnAbortReason::Interrupted, }), }); @@ -3568,6 +3581,7 @@ async fn interrupted_turn_error_message_snapshot() { chat.handle_codex_event(Event { id: "task-1".into(), msg: EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), model_context_window: None, collaboration_mode_kind: ModeKind::Default, }), @@ -3577,6 +3591,7 @@ async fn interrupted_turn_error_message_snapshot() { chat.handle_codex_event(Event { id: "task-1".into(), msg: EventMsg::TurnAborted(codex_core::protocol::TurnAbortedEvent { + turn_id: Some("turn-1".to_string()), reason: TurnAbortReason::Interrupted, }), }); @@ -4640,6 +4655,7 @@ async fn interrupt_restores_queued_messages_into_composer() { chat.handle_codex_event(Event { id: "turn-1".into(), msg: EventMsg::TurnAborted(codex_core::protocol::TurnAbortedEvent { + turn_id: Some("turn-1".to_string()), reason: TurnAbortReason::Interrupted, }), }); @@ -4678,6 +4694,7 @@ async fn interrupt_prepends_queued_messages_before_existing_composer_text() { chat.handle_codex_event(Event { id: "turn-1".into(), msg: EventMsg::TurnAborted(codex_core::protocol::TurnAbortedEvent { + turn_id: Some("turn-1".to_string()), reason: TurnAbortReason::Interrupted, }), }); @@ -4706,6 +4723,7 @@ async fn interrupt_clears_unified_exec_processes() { chat.handle_codex_event(Event { id: "turn-1".into(), msg: EventMsg::TurnAborted(codex_core::protocol::TurnAbortedEvent { + turn_id: Some("turn-1".to_string()), reason: TurnAbortReason::Interrupted, }), }); @@ -4726,6 +4744,7 @@ async fn review_ended_keeps_unified_exec_processes() { chat.handle_codex_event(Event { id: "turn-1".into(), msg: EventMsg::TurnAborted(codex_core::protocol::TurnAbortedEvent { + turn_id: Some("turn-1".to_string()), reason: TurnAbortReason::ReviewEnded, }), }); @@ -4758,6 +4777,7 @@ async fn interrupt_clears_unified_exec_wait_streak_snapshot() { chat.handle_codex_event(Event { id: "turn-1".into(), msg: EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), model_context_window: None, collaboration_mode_kind: ModeKind::Default, }), @@ -4769,6 +4789,7 @@ async fn interrupt_clears_unified_exec_wait_streak_snapshot() { chat.handle_codex_event(Event { id: "turn-1".into(), msg: EventMsg::TurnAborted(codex_core::protocol::TurnAbortedEvent { + turn_id: Some("turn-1".to_string()), reason: TurnAbortReason::Interrupted, }), }); @@ -4795,6 +4816,7 @@ async fn turn_complete_keeps_unified_exec_processes() { chat.handle_codex_event(Event { id: "turn-1".into(), msg: EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: "turn-1".to_string(), last_agent_message: None, }), }); @@ -4848,6 +4870,7 @@ async fn ui_snapshots_small_heights_task_running() { chat.handle_codex_event(Event { id: "task-1".into(), msg: EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), model_context_window: None, collaboration_mode_kind: ModeKind::Default, }), @@ -4880,6 +4903,7 @@ async fn status_widget_and_approval_modal_snapshot() { chat.handle_codex_event(Event { id: "task-1".into(), msg: EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), model_context_window: None, collaboration_mode_kind: ModeKind::Default, }), @@ -4933,6 +4957,7 @@ async fn status_widget_active_snapshot() { chat.handle_codex_event(Event { id: "task-1".into(), msg: EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), model_context_window: None, collaboration_mode_kind: ModeKind::Default, }), @@ -4983,6 +5008,7 @@ async fn mcp_startup_complete_does_not_clear_running_task() { chat.handle_codex_event(Event { id: "task-1".into(), msg: EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), model_context_window: None, collaboration_mode_kind: ModeKind::Default, }), @@ -5615,6 +5641,7 @@ async fn status_line_branch_refreshes_after_turn_complete() { chat.handle_codex_event(Event { id: "turn-1".into(), msg: EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: "turn-1".to_string(), last_agent_message: None, }), }); @@ -5632,6 +5659,7 @@ async fn status_line_branch_refreshes_after_interrupt() { chat.handle_codex_event(Event { id: "turn-1".into(), msg: EventMsg::TurnAborted(codex_core::protocol::TurnAbortedEvent { + turn_id: Some("turn-1".to_string()), reason: TurnAbortReason::Interrupted, }), }); @@ -5645,6 +5673,7 @@ async fn stream_recovery_restores_previous_status_header() { chat.handle_codex_event(Event { id: "task".into(), msg: EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), model_context_window: None, collaboration_mode_kind: ModeKind::Default, }), @@ -5727,6 +5756,7 @@ async fn multiple_agent_messages_in_single_turn_emit_multiple_headers() { chat.handle_codex_event(Event { id: "s1".into(), msg: EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), model_context_window: None, collaboration_mode_kind: ModeKind::Default, }), @@ -5752,6 +5782,7 @@ async fn multiple_agent_messages_in_single_turn_emit_multiple_headers() { chat.handle_codex_event(Event { id: "s1".into(), msg: EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: "turn-1".to_string(), last_agent_message: None, }), }); @@ -5922,6 +5953,7 @@ async fn chatwidget_exec_and_status_layout_vt100_snapshot() { chat.handle_codex_event(Event { id: "t1".into(), msg: EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), model_context_window: None, collaboration_mode_kind: ModeKind::Default, }), @@ -5970,6 +6002,7 @@ async fn chatwidget_markdown_code_blocks_vt100_snapshot() { chat.handle_codex_event(Event { id: "t1".into(), msg: EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), model_context_window: None, collaboration_mode_kind: ModeKind::Default, }), @@ -6042,6 +6075,7 @@ printf 'fenced within fenced\n' chat.handle_codex_event(Event { id: "t1".into(), msg: EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: "turn-1".to_string(), last_agent_message: None, }), }); @@ -6060,6 +6094,7 @@ async fn chatwidget_tall() { chat.handle_codex_event(Event { id: "t1".into(), msg: EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".to_string(), model_context_window: None, collaboration_mode_kind: ModeKind::Default, }), diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index f457881ed5..ac82c1d7e1 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -1004,6 +1004,7 @@ mod tests { .clone() .unwrap_or_else(|| "gpt-5.1".to_string()); TurnContextItem { + turn_id: None, cwd, approval_policy: config.approval_policy.value(), sandbox_policy: config.sandbox_policy.get().clone(), From 0794a9e8d4771731189ab13168add005c765f146 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 10 Feb 2026 20:03:22 -0800 Subject: [PATCH 4/4] Make sandbox read access configurable with ReadOnlyAccess ## What This change introduces a new `ReadOnlyAccess` model and threads it through sandbox policy consumers so read access is explicit instead of implicit. - Added `ReadOnlyAccess` to protocol: - `Restricted { include_platform_defaults, readable_roots }` - `FullAccess` - Changed `SandboxPolicy` shape: - `ReadOnly` is now `ReadOnly { access: ReadOnlyAccess }` - `WorkspaceWrite` now carries `read_only_access: ReadOnlyAccess` - Kept existing behavior for now by defaulting to `ReadOnlyAccess::FullAccess` in constructors and current config/app-server mappings. - Added helper methods to compute effective readable roots (including optional platform defaults + cwd) and to detect full read access. - Updated seatbelt policy generation to honor restricted read roots by emitting scoped `(allow file-read* ...)` entries when full read access is not granted. - Updated Linux backends (`bwrap`, legacy landlock path) to fail closed with an explicit `UnsupportedOperation` when restricted read access is requested but not yet implemented there. - Updated Windows sandbox backends (standard, elevated, and runner paths) to fail closed in the same way for restricted read access. - Updated all call sites/tests/pattern matches for the new structured variants and regenerated app-server protocol schema/types. ## Why The previous `SandboxPolicy::ReadOnly` implied full-disk read access and left no way to express a narrower read surface. This refactor establishes the policy model needed to support user-configurable read restrictions in a follow-up without changing current runtime behavior. It also ensures we do not silently ignore future restricted-read policies on platform backends that do not support them yet. Failing closed keeps sandbox semantics predictable and avoids accidental over-permission. ## Compatibility and rollout notes - Existing behavior is preserved by default (`FullAccess`). - Existing config/app-server flows continue to serialize/deserialize cleanly. - New schema artifacts are included to keep generated protocol outputs in sync. ## Validation - `just fmt` - `just fix -p codex-protocol -p codex-core -p codex-linux-sandbox -p codex-windows-sandbox -p codex-app-server-protocol` - `cargo check -p codex-windows-sandbox` - Targeted crate/test runs were executed during development for protocol/core/ sandbox-related crates. --- codex-rs/Cargo.lock | 53 ------ codex-rs/Cargo.toml | 1 - .../schema/json/ClientRequest.json | 69 ++++++- .../schema/json/EventMsg.json | 69 ++++++- .../schema/json/ServerNotification.json | 69 ++++++- .../codex_app_server_protocol.schemas.json | 69 ++++++- .../json/v1/ExecOneOffCommandParams.json | 69 ++++++- .../json/v1/ForkConversationResponse.json | 69 ++++++- .../json/v1/ResumeConversationResponse.json | 69 ++++++- .../schema/json/v1/SendUserTurnParams.json | 69 ++++++- .../v1/SessionConfiguredNotification.json | 69 ++++++- .../schema/typescript/ReadOnlyAccess.ts | 19 ++ .../schema/typescript/SandboxPolicy.ts | 11 +- .../schema/typescript/index.ts | 1 + .../app-server-protocol/src/protocol/v2.rs | 9 +- .../suite/codex_message_processor_flow.rs | 1 + codex-rs/common/src/approval_presets.rs | 2 +- codex-rs/common/src/sandbox_summary.rs | 4 +- codex-rs/core/Cargo.toml | 1 - codex-rs/core/src/config/mod.rs | 35 ++-- .../src/config_loader/config_requirements.rs | 11 +- codex-rs/core/src/config_loader/tests.rs | 3 +- codex-rs/core/src/connectors.rs | 2 +- codex-rs/core/src/exec.rs | 8 +- codex-rs/core/src/exec_policy.rs | 30 ++-- codex-rs/core/src/landlock.rs | 4 +- codex-rs/core/src/mcp/mod.rs | 2 +- codex-rs/core/src/memories/prompts.rs | 47 +---- codex-rs/core/src/rollout/metadata.rs | 2 +- codex-rs/core/src/safety.rs | 4 +- codex-rs/core/src/seatbelt.rs | 53 +++++- codex-rs/core/src/tools/registry.rs | 2 +- codex-rs/core/src/tools/sandboxing.rs | 5 +- codex-rs/core/tests/suite/apply_patch_cli.rs | 2 + codex-rs/core/tests/suite/approvals.rs | 34 ++-- codex-rs/core/tests/suite/codex_delegate.rs | 4 +- codex-rs/core/tests/suite/model_switching.rs | 12 +- .../core/tests/suite/permissions_messages.rs | 1 + codex-rs/core/tests/suite/personality.rs | 28 +-- codex-rs/core/tests/suite/prompt_caching.rs | 2 + codex-rs/core/tests/suite/rmcp_client.rs | 12 +- codex-rs/core/tests/suite/seatbelt.rs | 8 +- codex-rs/core/tests/suite/tools.rs | 2 +- codex-rs/core/tests/suite/truncation.rs | 6 +- codex-rs/core/tests/suite/unified_exec.rs | 4 +- codex-rs/core/tests/suite/web_search.rs | 29 +-- codex-rs/exec-server/src/posix/mcp.rs | 2 +- codex-rs/exec-server/tests/common/lib.rs | 3 +- .../tests/event_processor_with_json_output.rs | 2 +- codex-rs/exec/tests/suite/sandbox.rs | 8 +- codex-rs/linux-sandbox/src/bwrap.rs | 7 + codex-rs/linux-sandbox/src/landlock.rs | 14 +- codex-rs/linux-sandbox/src/linux_run_main.rs | 6 +- .../linux-sandbox/tests/suite/landlock.rs | 1 + codex-rs/mcp-server/src/outgoing_message.rs | 6 +- codex-rs/protocol/src/models.rs | 3 +- codex-rs/protocol/src/protocol.rs | 168 +++++++++++++++++- codex-rs/state/src/model/thread_metadata.rs | 2 +- codex-rs/state/src/runtime.rs | 2 +- codex-rs/tui/src/additional_dirs.rs | 6 +- codex-rs/tui/src/app.rs | 18 +- codex-rs/tui/src/chatwidget.rs | 23 +-- codex-rs/tui/src/chatwidget/tests.rs | 11 +- codex-rs/tui/src/debug_config.rs | 2 +- codex-rs/tui/src/status/card.rs | 2 +- codex-rs/tui/src/status/tests.rs | 1 + codex-rs/windows-sandbox-rs/src/allow.rs | 5 + codex-rs/windows-sandbox-rs/src/audit.rs | 2 +- .../src/command_runner_win.rs | 9 +- .../windows-sandbox-rs/src/elevated_impl.rs | 10 +- codex-rs/windows-sandbox-rs/src/lib.rs | 12 +- codex-rs/windows-sandbox-rs/src/policy.rs | 7 +- 72 files changed, 1122 insertions(+), 285 deletions(-) create mode 100644 codex-rs/app-server-protocol/schema/typescript/ReadOnlyAccess.ts diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 36ee957471..afb3b04ea5 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -458,58 +458,6 @@ dependencies = [ "term", ] -[[package]] -name = "askama" -version = "0.15.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "08e1676b346cadfec169374f949d7490fd80a24193d37d2afce0c047cf695e57" -dependencies = [ - "askama_macros", - "itoa", - "percent-encoding", - "serde", - "serde_json", -] - -[[package]] -name = "askama_derive" -version = "0.15.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7661ff56517787343f376f75db037426facd7c8d3049cef8911f1e75016f3a37" -dependencies = [ - "askama_parser", - "basic-toml", - "memchr", - "proc-macro2", - "quote", - "rustc-hash 2.1.1", - "serde", - "serde_derive", - "syn 2.0.114", -] - -[[package]] -name = "askama_macros" -version = "0.15.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "713ee4dbfd1eb719c2dab859465b01fa1d21cb566684614a713a6b7a99a4e47b" -dependencies = [ - "askama_derive", -] - -[[package]] -name = "askama_parser" -version = "0.15.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1d62d674238a526418b30c0def480d5beadb9d8964e7f38d635b03bf639c704c" -dependencies = [ - "rustc-hash 2.1.1", - "serde", - "serde_derive", - "unicode-ident", - "winnow", -] - [[package]] name = "asn1-rs" version = "0.7.1" @@ -1644,7 +1592,6 @@ version = "0.0.0" dependencies = [ "anyhow", "arc-swap", - "askama", "assert_cmd", "assert_matches", "async-channel", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index e7b30353f2..09ca74487f 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -134,7 +134,6 @@ assert_matches = "1.5.0" async-channel = "2.3.1" async-stream = "0.3.6" async-trait = "0.1.89" -askama = "0.15.4" axum = { version = "0.8", default-features = false } base64 = "0.22.1" bm25 = "2.3.2" diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index de746e7a78..4a29e8d145 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -1243,6 +1243,57 @@ ], "type": "string" }, + "ReadOnlyAccess": { + "description": "Determines how read-only file access is granted inside a restricted sandbox.", + "oneOf": [ + { + "description": "Restrict reads to an explicit set of roots.\n\nWhen `include_platform_defaults` is `true`, platform defaults required for basic execution are included in addition to `readable_roots`.", + "properties": { + "include_platform_defaults": { + "default": true, + "description": "Include built-in platform read roots required for basic process execution.", + "type": "boolean" + }, + "readable_roots": { + "description": "Additional absolute roots that should be readable.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "type": { + "enum": [ + "restricted" + ], + "title": "RestrictedReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "RestrictedReadOnlyAccess", + "type": "object" + }, + { + "description": "Allow unrestricted file reads.", + "properties": { + "type": { + "enum": [ + "full-access" + ], + "title": "FullAccessReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "FullAccessReadOnlyAccess", + "type": "object" + } + ] + }, "ReasoningEffort": { "description": "See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning", "enum": [ @@ -2018,8 +2069,16 @@ "type": "object" }, { - "description": "Read-only access to the entire file-system.", + "description": "Read-only access configuration.", "properties": { + "access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "read-only" @@ -2078,6 +2137,14 @@ "description": "When set to `true`, outbound network access is allowed. `false` by default.", "type": "boolean" }, + "read_only_access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "workspace-write" diff --git a/codex-rs/app-server-protocol/schema/json/EventMsg.json b/codex-rs/app-server-protocol/schema/json/EventMsg.json index 7151f0d6b6..44e8f309b4 100644 --- a/codex-rs/app-server-protocol/schema/json/EventMsg.json +++ b/codex-rs/app-server-protocol/schema/json/EventMsg.json @@ -3532,6 +3532,57 @@ ], "type": "object" }, + "ReadOnlyAccess": { + "description": "Determines how read-only file access is granted inside a restricted sandbox.", + "oneOf": [ + { + "description": "Restrict reads to an explicit set of roots.\n\nWhen `include_platform_defaults` is `true`, platform defaults required for basic execution are included in addition to `readable_roots`.", + "properties": { + "include_platform_defaults": { + "default": true, + "description": "Include built-in platform read roots required for basic process execution.", + "type": "boolean" + }, + "readable_roots": { + "description": "Additional absolute roots that should be readable.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "type": { + "enum": [ + "restricted" + ], + "title": "RestrictedReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "RestrictedReadOnlyAccess", + "type": "object" + }, + { + "description": "Allow unrestricted file reads.", + "properties": { + "type": { + "enum": [ + "full-access" + ], + "title": "FullAccessReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "FullAccessReadOnlyAccess", + "type": "object" + } + ] + }, "ReasoningEffort": { "description": "See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning", "enum": [ @@ -4369,8 +4420,16 @@ "type": "object" }, { - "description": "Read-only access to the entire file-system.", + "description": "Read-only access configuration.", "properties": { + "access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "read-only" @@ -4429,6 +4488,14 @@ "description": "When set to `true`, outbound network access is allowed. `false` by default.", "type": "boolean" }, + "read_only_access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "workspace-write" diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index 39fba5b5ac..39c11a8112 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -4558,6 +4558,57 @@ ], "type": "object" }, + "ReadOnlyAccess": { + "description": "Determines how read-only file access is granted inside a restricted sandbox.", + "oneOf": [ + { + "description": "Restrict reads to an explicit set of roots.\n\nWhen `include_platform_defaults` is `true`, platform defaults required for basic execution are included in addition to `readable_roots`.", + "properties": { + "include_platform_defaults": { + "default": true, + "description": "Include built-in platform read roots required for basic process execution.", + "type": "boolean" + }, + "readable_roots": { + "description": "Additional absolute roots that should be readable.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "type": { + "enum": [ + "restricted" + ], + "title": "RestrictedReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "RestrictedReadOnlyAccess", + "type": "object" + }, + { + "description": "Allow unrestricted file reads.", + "properties": { + "type": { + "enum": [ + "full-access" + ], + "title": "FullAccessReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "FullAccessReadOnlyAccess", + "type": "object" + } + ] + }, "ReasoningEffort": { "description": "See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning", "enum": [ @@ -5475,8 +5526,16 @@ "type": "object" }, { - "description": "Read-only access to the entire file-system.", + "description": "Read-only access configuration.", "properties": { + "access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "read-only" @@ -5535,6 +5594,14 @@ "description": "When set to `true`, outbound network access is allowed. `false` by default.", "type": "boolean" }, + "read_only_access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "workspace-write" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 633c485e2a..4b1b4d768e 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -6617,6 +6617,57 @@ ], "type": "object" }, + "ReadOnlyAccess": { + "description": "Determines how read-only file access is granted inside a restricted sandbox.", + "oneOf": [ + { + "description": "Restrict reads to an explicit set of roots.\n\nWhen `include_platform_defaults` is `true`, platform defaults required for basic execution are included in addition to `readable_roots`.", + "properties": { + "include_platform_defaults": { + "default": true, + "description": "Include built-in platform read roots required for basic process execution.", + "type": "boolean" + }, + "readable_roots": { + "description": "Additional absolute roots that should be readable.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "type": { + "enum": [ + "restricted" + ], + "title": "RestrictedReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "RestrictedReadOnlyAccess", + "type": "object" + }, + { + "description": "Allow unrestricted file reads.", + "properties": { + "type": { + "enum": [ + "full-access" + ], + "title": "FullAccessReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "FullAccessReadOnlyAccess", + "type": "object" + } + ] + }, "ReasoningEffort": { "description": "See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning", "enum": [ @@ -7630,8 +7681,16 @@ "type": "object" }, { - "description": "Read-only access to the entire file-system.", + "description": "Read-only access configuration.", "properties": { + "access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "read-only" @@ -7690,6 +7749,14 @@ "description": "When set to `true`, outbound network access is allowed. `false` by default.", "type": "boolean" }, + "read_only_access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "workspace-write" diff --git a/codex-rs/app-server-protocol/schema/json/v1/ExecOneOffCommandParams.json b/codex-rs/app-server-protocol/schema/json/v1/ExecOneOffCommandParams.json index a325704be4..ec5b789352 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ExecOneOffCommandParams.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ExecOneOffCommandParams.json @@ -13,6 +13,57 @@ ], "type": "string" }, + "ReadOnlyAccess": { + "description": "Determines how read-only file access is granted inside a restricted sandbox.", + "oneOf": [ + { + "description": "Restrict reads to an explicit set of roots.\n\nWhen `include_platform_defaults` is `true`, platform defaults required for basic execution are included in addition to `readable_roots`.", + "properties": { + "include_platform_defaults": { + "default": true, + "description": "Include built-in platform read roots required for basic process execution.", + "type": "boolean" + }, + "readable_roots": { + "description": "Additional absolute roots that should be readable.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "type": { + "enum": [ + "restricted" + ], + "title": "RestrictedReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "RestrictedReadOnlyAccess", + "type": "object" + }, + { + "description": "Allow unrestricted file reads.", + "properties": { + "type": { + "enum": [ + "full-access" + ], + "title": "FullAccessReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "FullAccessReadOnlyAccess", + "type": "object" + } + ] + }, "SandboxPolicy": { "description": "Determines execution restrictions for model shell commands.", "oneOf": [ @@ -34,8 +85,16 @@ "type": "object" }, { - "description": "Read-only access to the entire file-system.", + "description": "Read-only access configuration.", "properties": { + "access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "read-only" @@ -94,6 +153,14 @@ "description": "When set to `true`, outbound network access is allowed. `false` by default.", "type": "boolean" }, + "read_only_access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "workspace-write" diff --git a/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json b/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json index 027caee1ca..051802b26e 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json @@ -3532,6 +3532,57 @@ ], "type": "object" }, + "ReadOnlyAccess": { + "description": "Determines how read-only file access is granted inside a restricted sandbox.", + "oneOf": [ + { + "description": "Restrict reads to an explicit set of roots.\n\nWhen `include_platform_defaults` is `true`, platform defaults required for basic execution are included in addition to `readable_roots`.", + "properties": { + "include_platform_defaults": { + "default": true, + "description": "Include built-in platform read roots required for basic process execution.", + "type": "boolean" + }, + "readable_roots": { + "description": "Additional absolute roots that should be readable.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "type": { + "enum": [ + "restricted" + ], + "title": "RestrictedReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "RestrictedReadOnlyAccess", + "type": "object" + }, + { + "description": "Allow unrestricted file reads.", + "properties": { + "type": { + "enum": [ + "full-access" + ], + "title": "FullAccessReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "FullAccessReadOnlyAccess", + "type": "object" + } + ] + }, "ReasoningEffort": { "description": "See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning", "enum": [ @@ -4369,8 +4420,16 @@ "type": "object" }, { - "description": "Read-only access to the entire file-system.", + "description": "Read-only access configuration.", "properties": { + "access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "read-only" @@ -4429,6 +4488,14 @@ "description": "When set to `true`, outbound network access is allowed. `false` by default.", "type": "boolean" }, + "read_only_access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "workspace-write" diff --git a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json index e6f3b145a2..684352b0f5 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json @@ -3532,6 +3532,57 @@ ], "type": "object" }, + "ReadOnlyAccess": { + "description": "Determines how read-only file access is granted inside a restricted sandbox.", + "oneOf": [ + { + "description": "Restrict reads to an explicit set of roots.\n\nWhen `include_platform_defaults` is `true`, platform defaults required for basic execution are included in addition to `readable_roots`.", + "properties": { + "include_platform_defaults": { + "default": true, + "description": "Include built-in platform read roots required for basic process execution.", + "type": "boolean" + }, + "readable_roots": { + "description": "Additional absolute roots that should be readable.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "type": { + "enum": [ + "restricted" + ], + "title": "RestrictedReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "RestrictedReadOnlyAccess", + "type": "object" + }, + { + "description": "Allow unrestricted file reads.", + "properties": { + "type": { + "enum": [ + "full-access" + ], + "title": "FullAccessReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "FullAccessReadOnlyAccess", + "type": "object" + } + ] + }, "ReasoningEffort": { "description": "See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning", "enum": [ @@ -4369,8 +4420,16 @@ "type": "object" }, { - "description": "Read-only access to the entire file-system.", + "description": "Read-only access configuration.", "properties": { + "access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "read-only" @@ -4429,6 +4488,14 @@ "description": "When set to `true`, outbound network access is allowed. `false` by default.", "type": "boolean" }, + "read_only_access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "workspace-write" diff --git a/codex-rs/app-server-protocol/schema/json/v1/SendUserTurnParams.json b/codex-rs/app-server-protocol/schema/json/v1/SendUserTurnParams.json index d56ae933bd..b2fc248663 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/SendUserTurnParams.json +++ b/codex-rs/app-server-protocol/schema/json/v1/SendUserTurnParams.json @@ -142,6 +142,57 @@ ], "type": "string" }, + "ReadOnlyAccess": { + "description": "Determines how read-only file access is granted inside a restricted sandbox.", + "oneOf": [ + { + "description": "Restrict reads to an explicit set of roots.\n\nWhen `include_platform_defaults` is `true`, platform defaults required for basic execution are included in addition to `readable_roots`.", + "properties": { + "include_platform_defaults": { + "default": true, + "description": "Include built-in platform read roots required for basic process execution.", + "type": "boolean" + }, + "readable_roots": { + "description": "Additional absolute roots that should be readable.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "type": { + "enum": [ + "restricted" + ], + "title": "RestrictedReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "RestrictedReadOnlyAccess", + "type": "object" + }, + { + "description": "Allow unrestricted file reads.", + "properties": { + "type": { + "enum": [ + "full-access" + ], + "title": "FullAccessReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "FullAccessReadOnlyAccess", + "type": "object" + } + ] + }, "ReasoningEffort": { "description": "See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning", "enum": [ @@ -195,8 +246,16 @@ "type": "object" }, { - "description": "Read-only access to the entire file-system.", + "description": "Read-only access configuration.", "properties": { + "access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "read-only" @@ -255,6 +314,14 @@ "description": "When set to `true`, outbound network access is allowed. `false` by default.", "type": "boolean" }, + "read_only_access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "workspace-write" diff --git a/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json b/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json index 1b717eefc4..98853da617 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json @@ -3532,6 +3532,57 @@ ], "type": "object" }, + "ReadOnlyAccess": { + "description": "Determines how read-only file access is granted inside a restricted sandbox.", + "oneOf": [ + { + "description": "Restrict reads to an explicit set of roots.\n\nWhen `include_platform_defaults` is `true`, platform defaults required for basic execution are included in addition to `readable_roots`.", + "properties": { + "include_platform_defaults": { + "default": true, + "description": "Include built-in platform read roots required for basic process execution.", + "type": "boolean" + }, + "readable_roots": { + "description": "Additional absolute roots that should be readable.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "type": { + "enum": [ + "restricted" + ], + "title": "RestrictedReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "RestrictedReadOnlyAccess", + "type": "object" + }, + { + "description": "Allow unrestricted file reads.", + "properties": { + "type": { + "enum": [ + "full-access" + ], + "title": "FullAccessReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "FullAccessReadOnlyAccess", + "type": "object" + } + ] + }, "ReasoningEffort": { "description": "See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning", "enum": [ @@ -4369,8 +4420,16 @@ "type": "object" }, { - "description": "Read-only access to the entire file-system.", + "description": "Read-only access configuration.", "properties": { + "access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "read-only" @@ -4429,6 +4488,14 @@ "description": "When set to `true`, outbound network access is allowed. `false` by default.", "type": "boolean" }, + "read_only_access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "workspace-write" diff --git a/codex-rs/app-server-protocol/schema/typescript/ReadOnlyAccess.ts b/codex-rs/app-server-protocol/schema/typescript/ReadOnlyAccess.ts new file mode 100644 index 0000000000..c01bdd37c6 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/ReadOnlyAccess.ts @@ -0,0 +1,19 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { AbsolutePathBuf } from "./AbsolutePathBuf"; + +/** + * Determines how read-only file access is granted inside a restricted + * sandbox. + */ +export type ReadOnlyAccess = { "type": "restricted", +/** + * Include built-in platform read roots required for basic process + * execution. + */ +include_platform_defaults: boolean, +/** + * Additional absolute roots that should be readable. + */ +readable_roots?: Array, } | { "type": "full-access" }; diff --git a/codex-rs/app-server-protocol/schema/typescript/SandboxPolicy.ts b/codex-rs/app-server-protocol/schema/typescript/SandboxPolicy.ts index 103a6863f4..743ad22229 100644 --- a/codex-rs/app-server-protocol/schema/typescript/SandboxPolicy.ts +++ b/codex-rs/app-server-protocol/schema/typescript/SandboxPolicy.ts @@ -3,11 +3,16 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { AbsolutePathBuf } from "./AbsolutePathBuf"; import type { NetworkAccess } from "./NetworkAccess"; +import type { ReadOnlyAccess } from "./ReadOnlyAccess"; /** * Determines execution restrictions for model shell commands. */ -export type SandboxPolicy = { "type": "danger-full-access" } | { "type": "read-only" } | { "type": "external-sandbox", +export type SandboxPolicy = { "type": "danger-full-access" } | { "type": "read-only", +/** + * Read access granted while running under this policy. + */ +access?: ReadOnlyAccess, } | { "type": "external-sandbox", /** * Whether the external sandbox permits outbound network traffic. */ @@ -17,6 +22,10 @@ network_access: NetworkAccess, } | { "type": "workspace-write", * writable from within the sandbox. */ writable_roots?: Array, +/** + * Read access granted while running under this policy. + */ +read_only_access?: ReadOnlyAccess, /** * When set to `true`, outbound network access is allowed. `false` by * default. diff --git a/codex-rs/app-server-protocol/schema/typescript/index.ts b/codex-rs/app-server-protocol/schema/typescript/index.ts index 895936de2b..03675b3e10 100644 --- a/codex-rs/app-server-protocol/schema/typescript/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/index.ts @@ -137,6 +137,7 @@ export type { Profile } from "./Profile"; export type { RateLimitSnapshot } from "./RateLimitSnapshot"; export type { RateLimitWindow } from "./RateLimitWindow"; export type { RawResponseItemEvent } from "./RawResponseItemEvent"; +export type { ReadOnlyAccess } from "./ReadOnlyAccess"; export type { ReasoningContentDeltaEvent } from "./ReasoningContentDeltaEvent"; export type { ReasoningEffort } from "./ReasoningEffort"; export type { ReasoningItem } from "./ReasoningItem"; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 679dc4ab23..37004e8081 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -32,6 +32,7 @@ use codex_protocol::protocol::CreditsSnapshot as CoreCreditsSnapshot; use codex_protocol::protocol::NetworkAccess as CoreNetworkAccess; use codex_protocol::protocol::RateLimitSnapshot as CoreRateLimitSnapshot; use codex_protocol::protocol::RateLimitWindow as CoreRateLimitWindow; +use codex_protocol::protocol::ReadOnlyAccess as CoreReadOnlyAccess; use codex_protocol::protocol::SessionSource as CoreSessionSource; use codex_protocol::protocol::SkillDependencies as CoreSkillDependencies; use codex_protocol::protocol::SkillErrorInfo as CoreSkillErrorInfo; @@ -680,7 +681,9 @@ impl SandboxPolicy { SandboxPolicy::DangerFullAccess => { codex_protocol::protocol::SandboxPolicy::DangerFullAccess } - SandboxPolicy::ReadOnly => codex_protocol::protocol::SandboxPolicy::ReadOnly, + SandboxPolicy::ReadOnly => codex_protocol::protocol::SandboxPolicy::ReadOnly { + access: CoreReadOnlyAccess::FullAccess, + }, SandboxPolicy::ExternalSandbox { network_access } => { codex_protocol::protocol::SandboxPolicy::ExternalSandbox { network_access: match network_access { @@ -696,6 +699,7 @@ impl SandboxPolicy { exclude_slash_tmp, } => codex_protocol::protocol::SandboxPolicy::WorkspaceWrite { writable_roots: writable_roots.clone(), + read_only_access: CoreReadOnlyAccess::FullAccess, network_access: *network_access, exclude_tmpdir_env_var: *exclude_tmpdir_env_var, exclude_slash_tmp: *exclude_slash_tmp, @@ -710,7 +714,7 @@ impl From for SandboxPolicy { codex_protocol::protocol::SandboxPolicy::DangerFullAccess => { SandboxPolicy::DangerFullAccess } - codex_protocol::protocol::SandboxPolicy::ReadOnly => SandboxPolicy::ReadOnly, + codex_protocol::protocol::SandboxPolicy::ReadOnly { .. } => SandboxPolicy::ReadOnly, codex_protocol::protocol::SandboxPolicy::ExternalSandbox { network_access } => { SandboxPolicy::ExternalSandbox { network_access: match network_access { @@ -721,6 +725,7 @@ impl From for SandboxPolicy { } codex_protocol::protocol::SandboxPolicy::WorkspaceWrite { writable_roots, + read_only_access: _, network_access, exclude_tmpdir_env_var, exclude_slash_tmp, diff --git a/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs b/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs index 2debbda653..863a7bfc4f 100644 --- a/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs +++ b/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs @@ -444,6 +444,7 @@ async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() -> Result<( approval_policy: AskForApproval::Never, sandbox_policy: SandboxPolicy::WorkspaceWrite { writable_roots: vec![first_cwd.try_into()?], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/common/src/approval_presets.rs b/codex-rs/common/src/approval_presets.rs index cec67d258d..9e66e39e08 100644 --- a/codex-rs/common/src/approval_presets.rs +++ b/codex-rs/common/src/approval_presets.rs @@ -26,7 +26,7 @@ pub fn builtin_approval_presets() -> Vec { label: "Read Only", description: "Codex can read files in the current workspace. Approval is required to edit files or access the internet.", approval: AskForApproval::OnRequest, - sandbox: SandboxPolicy::ReadOnly, + sandbox: SandboxPolicy::new_read_only_policy(), }, ApprovalPreset { id: "auto", diff --git a/codex-rs/common/src/sandbox_summary.rs b/codex-rs/common/src/sandbox_summary.rs index 45520b11a0..53851d86b7 100644 --- a/codex-rs/common/src/sandbox_summary.rs +++ b/codex-rs/common/src/sandbox_summary.rs @@ -4,7 +4,7 @@ use codex_core::protocol::SandboxPolicy; pub fn summarize_sandbox_policy(sandbox_policy: &SandboxPolicy) -> String { match sandbox_policy { SandboxPolicy::DangerFullAccess => "danger-full-access".to_string(), - SandboxPolicy::ReadOnly => "read-only".to_string(), + SandboxPolicy::ReadOnly { .. } => "read-only".to_string(), SandboxPolicy::ExternalSandbox { network_access } => { let mut summary = "external-sandbox".to_string(); if matches!(network_access, NetworkAccess::Enabled) { @@ -17,6 +17,7 @@ pub fn summarize_sandbox_policy(sandbox_policy: &SandboxPolicy) -> String { network_access, exclude_tmpdir_env_var, exclude_slash_tmp, + read_only_access: _, } => { let mut summary = "workspace-write".to_string(); @@ -71,6 +72,7 @@ mod tests { let writable_root = AbsolutePathBuf::try_from(root).unwrap(); let summary = summarize_sandbox_policy(&SandboxPolicy::WorkspaceWrite { writable_roots: vec![writable_root.clone()], + read_only_access: Default::default(), network_access: true, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 26e7f17cc8..b68d1c144f 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -22,7 +22,6 @@ anyhow = { workspace = true } arc-swap = "1.8.0" async-channel = { workspace = true } async-trait = { workspace = true } -askama = { workspace = true } base64 = { workspace = true } bm25 = { workspace = true } chardetng = { workspace = true } diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index a923b0ef78..1407436d25 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -43,6 +43,7 @@ use crate::model_provider_info::built_in_model_providers; use crate::project_doc::DEFAULT_PROJECT_DOC_FILENAME; use crate::project_doc::LOCAL_PROJECT_DOC_FILENAME; use crate::protocol::AskForApproval; +use crate::protocol::ReadOnlyAccess; use crate::protocol::SandboxPolicy; use crate::windows_sandbox::WindowsSandboxLevelExt; use codex_app_server_protocol::Tools; @@ -1183,6 +1184,7 @@ impl ConfigToml { exclude_slash_tmp, }) => SandboxPolicy::WorkspaceWrite { writable_roots: writable_roots.clone(), + read_only_access: ReadOnlyAccess::FullAccess, network_access: *network_access, exclude_tmpdir_env_var: *exclude_tmpdir_env_var, exclude_slash_tmp: *exclude_slash_tmp, @@ -2105,7 +2107,7 @@ network_access = true # This should be ignored. assert_eq!( resolution, SandboxPolicyResolution { - policy: SandboxPolicy::ReadOnly, + policy: SandboxPolicy::new_read_only_policy(), forced_auto_mode_downgraded_on_windows: false, } ); @@ -2139,7 +2141,7 @@ exclude_slash_tmp = true assert_eq!( resolution, SandboxPolicyResolution { - policy: SandboxPolicy::ReadOnly, + policy: SandboxPolicy::new_read_only_policy(), forced_auto_mode_downgraded_on_windows: true, } ); @@ -2149,6 +2151,7 @@ exclude_slash_tmp = true SandboxPolicyResolution { policy: SandboxPolicy::WorkspaceWrite { writable_roots: vec![writable_root.clone()], + read_only_access: ReadOnlyAccess::FullAccess, network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -2189,7 +2192,7 @@ trust_level = "trusted" assert_eq!( resolution, SandboxPolicyResolution { - policy: SandboxPolicy::ReadOnly, + policy: SandboxPolicy::new_read_only_policy(), forced_auto_mode_downgraded_on_windows: true, } ); @@ -2199,6 +2202,7 @@ trust_level = "trusted" SandboxPolicyResolution { policy: SandboxPolicy::WorkspaceWrite { writable_roots: vec![writable_root], + read_only_access: ReadOnlyAccess::FullAccess, network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -2371,7 +2375,7 @@ trust_level = "trusted" "expected workspace-write request to be downgraded on Windows" ); match config.sandbox_policy.get() { - &SandboxPolicy::ReadOnly => {} + SandboxPolicy::ReadOnly { .. } => {} other => panic!("expected read-only policy on Windows, got {other:?}"), } } else { @@ -2517,7 +2521,10 @@ trust_level = "trusted" #[test] fn web_search_mode_for_turn_uses_preference_for_read_only() { let web_search_mode = Constrained::allow_any(WebSearchMode::Cached); - let mode = resolve_web_search_mode_for_turn(&web_search_mode, &SandboxPolicy::ReadOnly); + let mode = resolve_web_search_mode_for_turn( + &web_search_mode, + &SandboxPolicy::new_read_only_policy(), + ); assert_eq!(mode, WebSearchMode::Cached); } @@ -2700,7 +2707,7 @@ profile = "project" if cfg!(target_os = "windows") { assert!(matches!( config.sandbox_policy.get(), - SandboxPolicy::ReadOnly + SandboxPolicy::ReadOnly { .. } )); assert!(config.forced_auto_mode_downgraded_on_windows); } else { @@ -4672,7 +4679,7 @@ trust_level = "untrusted" // Verify that untrusted projects get WorkspaceWrite (or ReadOnly on Windows due to downgrade) if cfg!(target_os = "windows") { assert!( - matches!(resolution.policy, SandboxPolicy::ReadOnly), + matches!(resolution.policy, SandboxPolicy::ReadOnly { .. }), "Expected ReadOnly on Windows, got {:?}", resolution.policy ); @@ -4768,7 +4775,7 @@ trust_level = "untrusted" assert_eq!( resolution, SandboxPolicyResolution { - policy: SandboxPolicy::ReadOnly, + policy: SandboxPolicy::new_read_only_policy(), forced_auto_mode_downgraded_on_windows: true, } ); @@ -4924,7 +4931,7 @@ mcp_oauth_callback_port = 5678 // Verify that untrusted projects still get WorkspaceWrite sandbox (or ReadOnly on Windows) if cfg!(target_os = "windows") { assert!( - matches!(config.sandbox_policy.get(), SandboxPolicy::ReadOnly), + matches!(config.sandbox_policy.get(), SandboxPolicy::ReadOnly { .. }), "Expected ReadOnly on Windows" ); } else { @@ -4958,7 +4965,10 @@ mcp_oauth_callback_port = 5678 .build() .await?; - assert_eq!(*config.sandbox_policy.get(), SandboxPolicy::ReadOnly); + assert_eq!( + *config.sandbox_policy.get(), + SandboxPolicy::new_read_only_policy() + ); Ok(()) } @@ -4992,7 +5002,10 @@ mcp_oauth_callback_port = 5678 )) .build() .await?; - assert_eq!(*config.sandbox_policy.get(), SandboxPolicy::ReadOnly); + assert_eq!( + *config.sandbox_policy.get(), + SandboxPolicy::new_read_only_policy() + ); Ok(()) } diff --git a/codex-rs/core/src/config_loader/config_requirements.rs b/codex-rs/core/src/config_loader/config_requirements.rs index 6ba68afebe..7e5ca96d00 100644 --- a/codex-rs/core/src/config_loader/config_requirements.rs +++ b/codex-rs/core/src/config_loader/config_requirements.rs @@ -94,7 +94,7 @@ impl Default for ConfigRequirements { None, ), sandbox_policy: ConstrainedWithSource::new( - Constrained::allow_any(SandboxPolicy::ReadOnly), + Constrained::allow_any(SandboxPolicy::new_read_only_policy()), None, ), web_search_mode: ConstrainedWithSource::new( @@ -421,7 +421,7 @@ impl TryFrom for ConfigRequirements { // the other variants (WorkspaceWrite, ExternalSandbox) require // additional parameters. Ultimately, we should expand the config // format to allow specifying those parameters. - let default_sandbox_policy = SandboxPolicy::ReadOnly; + let default_sandbox_policy = SandboxPolicy::new_read_only_policy(); let sandbox_policy = match allowed_sandbox_modes { Some(Sourced { value: modes, @@ -439,7 +439,7 @@ impl TryFrom for ConfigRequirements { let requirement_source_for_error = requirement_source.clone(); let constrained = Constrained::new(default_sandbox_policy, move |candidate| { let mode = match candidate { - SandboxPolicy::ReadOnly => SandboxModeRequirement::ReadOnly, + SandboxPolicy::ReadOnly { .. } => SandboxModeRequirement::ReadOnly, SandboxPolicy::WorkspaceWrite { .. } => { SandboxModeRequirement::WorkspaceWrite } @@ -873,7 +873,7 @@ mod tests { assert!( requirements .sandbox_policy - .can_set(&SandboxPolicy::ReadOnly) + .can_set(&SandboxPolicy::new_read_only_policy()) .is_ok() ); @@ -892,7 +892,7 @@ mod tests { assert!( requirements .sandbox_policy - .can_set(&SandboxPolicy::ReadOnly) + .can_set(&SandboxPolicy::new_read_only_policy()) .is_ok() ); assert!( @@ -900,6 +900,7 @@ mod tests { .sandbox_policy .can_set(&SandboxPolicy::WorkspaceWrite { writable_roots: vec![AbsolutePathBuf::from_absolute_path(root)?], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/core/src/config_loader/tests.rs b/codex-rs/core/src/config_loader/tests.rs index 0a6f67d85b..a47546dac7 100644 --- a/codex-rs/core/src/config_loader/tests.rs +++ b/codex-rs/core/src/config_loader/tests.rs @@ -397,7 +397,7 @@ allowed_sandbox_modes = ["read-only"] ); assert_eq!( *state.requirements().sandbox_policy.get(), - SandboxPolicy::ReadOnly + SandboxPolicy::new_read_only_policy() ); assert!( state @@ -412,6 +412,7 @@ allowed_sandbox_modes = ["read-only"] .sandbox_policy .can_set(&SandboxPolicy::WorkspaceWrite { writable_roots: Vec::new(), + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index b451d21b57..1769446b66 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -80,7 +80,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options( let cancel_token = CancellationToken::new(); let sandbox_state = SandboxState { - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), sandbox_cwd: env::current_dir().unwrap_or_else(|_| PathBuf::from("/")), use_linux_sandbox_bwrap: config.features.enabled(Feature::UseLinuxSandboxBwrap), diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index e627ebec06..398a767ce0 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -1094,7 +1094,13 @@ mod tests { arg0: None, }; - let output = exec(params, SandboxType::None, &SandboxPolicy::ReadOnly, None).await?; + let output = exec( + params, + SandboxType::None, + &SandboxPolicy::new_read_only_policy(), + None, + ) + .await?; assert!(output.timed_out); let stdout = output.stdout.from_utf8_lossy().text; diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 710794a78d..ba77774c37 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -290,7 +290,7 @@ pub fn render_decision_for_unmatched_command( // On Windows, ReadOnly sandbox is not a real sandbox, so special-case it // here. let runtime_sandbox_provides_safety = - cfg!(windows) && matches!(sandbox_policy, SandboxPolicy::ReadOnly); + cfg!(windows) && matches!(sandbox_policy, SandboxPolicy::ReadOnly { .. }); // If the command is flagged as dangerous or we have no sandbox protection, // we should never allow it to run without user approval. @@ -325,7 +325,7 @@ pub fn render_decision_for_unmatched_command( // command has not been flagged as dangerous. Decision::Allow } - SandboxPolicy::ReadOnly | SandboxPolicy::WorkspaceWrite { .. } => { + SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. } => { // In restricted sandboxes (ReadOnly/WorkspaceWrite), do not prompt for // non‑escalated, non‑dangerous commands — let the sandbox enforce // restrictions (e.g., block network/write) without a user prompt. @@ -852,7 +852,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, }) @@ -879,7 +879,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, }) @@ -907,7 +907,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: Some(requested_prefix.clone()), }) @@ -1028,7 +1028,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, }) @@ -1110,7 +1110,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::RequireEscalated, prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]), }) @@ -1221,7 +1221,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, }) @@ -1278,7 +1278,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, }) @@ -1316,7 +1316,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, }) @@ -1339,7 +1339,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, }) @@ -1369,7 +1369,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, }) @@ -1458,7 +1458,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &sneaky_command, approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: permissions, prefix_rule: None, }) @@ -1481,7 +1481,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &dangerous_command, approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: permissions, prefix_rule: None, }) @@ -1500,7 +1500,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &dangerous_command, approval_policy: AskForApproval::Never, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: permissions, prefix_rule: None, }) diff --git a/codex-rs/core/src/landlock.rs b/codex-rs/core/src/landlock.rs index 51a4661938..65b2a68073 100644 --- a/codex-rs/core/src/landlock.rs +++ b/codex-rs/core/src/landlock.rs @@ -112,7 +112,7 @@ mod tests { fn bwrap_flags_are_feature_gated() { let command = vec!["/bin/true".to_string()]; let cwd = Path::new("/tmp"); - let policy = SandboxPolicy::ReadOnly; + let policy = SandboxPolicy::new_read_only_policy(); let with_bwrap = create_linux_sandbox_command_args(command.clone(), &policy, cwd, true, false); @@ -132,7 +132,7 @@ mod tests { fn proxy_flag_is_included_when_requested() { let command = vec!["/bin/true".to_string()]; let cwd = Path::new("/tmp"); - let policy = SandboxPolicy::ReadOnly; + let policy = SandboxPolicy::new_read_only_policy(); let args = create_linux_sandbox_command_args(command, &policy, cwd, true, true); assert_eq!( diff --git a/codex-rs/core/src/mcp/mod.rs b/codex-rs/core/src/mcp/mod.rs index 8034ed13a2..0ed0d221d2 100644 --- a/codex-rs/core/src/mcp/mod.rs +++ b/codex-rs/core/src/mcp/mod.rs @@ -165,7 +165,7 @@ pub async fn collect_mcp_snapshot(config: &Config) -> McpListToolsResponseEvent // Use ReadOnly sandbox policy for MCP snapshot collection (safest default) let sandbox_state = SandboxState { - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), sandbox_cwd: env::current_dir().unwrap_or_else(|_| PathBuf::from("/")), use_linux_sandbox_bwrap: config.features.enabled(Feature::UseLinuxSandboxBwrap), diff --git a/codex-rs/core/src/memories/prompts.rs b/codex-rs/core/src/memories/prompts.rs index c3faf1287c..e553f598de 100644 --- a/codex-rs/core/src/memories/prompts.rs +++ b/codex-rs/core/src/memories/prompts.rs @@ -1,4 +1,3 @@ -use askama::Template; use std::path::Path; use tracing::warn; @@ -6,36 +5,14 @@ use super::text::prefix_at_char_boundary; use super::text::suffix_at_char_boundary; const MAX_ROLLOUT_BYTES_FOR_PROMPT: usize = 1_000_000; - -#[derive(Template)] -#[template(path = "memories/consolidation.md", escape = "none")] -struct ConsolidationPromptTemplate<'a> { - memory_root: &'a str, -} - -#[derive(Template)] -#[template(path = "memories/stage_one_input.md", escape = "none")] -struct StageOneInputTemplate<'a> { - rollout_path: &'a str, - rollout_contents: &'a str, -} +const CONSOLIDATION_PROMPT_TEMPLATE: &str = + include_str!("../../templates/memories/consolidation.md"); +const STAGE_ONE_INPUT_TEMPLATE: &str = include_str!("../../templates/memories/stage_one_input.md"); /// Builds the consolidation subagent prompt for a specific memory root. -/// -/// Falls back to a simple string replacement if Askama rendering fails. pub(super) fn build_consolidation_prompt(memory_root: &Path) -> String { let memory_root = memory_root.display().to_string(); - let template = ConsolidationPromptTemplate { - memory_root: &memory_root, - }; - match template.render() { - Ok(prompt) => prompt, - Err(err) => { - warn!("failed to render memories consolidation prompt template: {err}"); - include_str!("../../templates/memories/consolidation.md") - .replace("{{ memory_root }}", &memory_root) - } - } + CONSOLIDATION_PROMPT_TEMPLATE.replace("{{ memory_root }}", &memory_root) } /// Builds the stage-1 user message containing rollout metadata and content. @@ -53,19 +30,9 @@ pub(super) fn build_stage_one_input_message(rollout_path: &Path, rollout_content } let rollout_path = rollout_path.display().to_string(); - let template = StageOneInputTemplate { - rollout_path: &rollout_path, - rollout_contents: &rollout_contents, - }; - match template.render() { - Ok(prompt) => prompt, - Err(err) => { - warn!("failed to render memories stage-one input template: {err}"); - include_str!("../../templates/memories/stage_one_input.md") - .replace("{{ rollout_path }}", &rollout_path) - .replace("{{ rollout_contents }}", &rollout_contents) - } - } + STAGE_ONE_INPUT_TEMPLATE + .replace("{{ rollout_path }}", &rollout_path) + .replace("{{ rollout_contents }}", &rollout_contents) } fn truncate_rollout_for_prompt(input: &str) -> (String, bool) { diff --git a/codex-rs/core/src/rollout/metadata.rs b/codex-rs/core/src/rollout/metadata.rs index 5bfdc363ae..7298877751 100644 --- a/codex-rs/core/src/rollout/metadata.rs +++ b/codex-rs/core/src/rollout/metadata.rs @@ -49,7 +49,7 @@ pub(crate) fn builder_from_session_meta( builder.model_provider = session_meta.meta.model_provider.clone(); builder.cwd = session_meta.meta.cwd.clone(); builder.cli_version = Some(session_meta.meta.cli_version.clone()); - builder.sandbox_policy = SandboxPolicy::ReadOnly; + builder.sandbox_policy = SandboxPolicy::new_read_only_policy(); builder.approval_mode = AskForApproval::OnRequest; if let Some(git) = session_meta.git.as_ref() { builder.git_sha = git.commit_hash.clone(); diff --git a/codex-rs/core/src/safety.rs b/codex-rs/core/src/safety.rs index 47a12e029e..b0d359b7e1 100644 --- a/codex-rs/core/src/safety.rs +++ b/codex-rs/core/src/safety.rs @@ -108,7 +108,7 @@ fn is_write_patch_constrained_to_writable_paths( ) -> bool { // Early‑exit if there are no declared writable roots. let writable_roots = match sandbox_policy { - SandboxPolicy::ReadOnly => { + SandboxPolicy::ReadOnly { .. } => { return false; } SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => { @@ -195,6 +195,7 @@ mod tests { // only `cwd` is writable by default. let policy_workspace_only = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -216,6 +217,7 @@ mod tests { // outside write should be permitted. let policy_with_parent = SandboxPolicy::WorkspaceWrite { writable_roots: vec![AbsolutePathBuf::try_from(parent).unwrap()], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, diff --git a/codex-rs/core/src/seatbelt.rs b/codex-rs/core/src/seatbelt.rs index e2061b52dc..acd4638b0d 100644 --- a/codex-rs/core/src/seatbelt.rs +++ b/codex-rs/core/src/seatbelt.rs @@ -237,10 +237,40 @@ pub(crate) fn create_seatbelt_command_args( } }; - let file_read_policy = if sandbox_policy.has_full_disk_read_access() { - "; allow read-only file operations\n(allow file-read*)" + let (file_read_policy, file_read_dir_params) = if sandbox_policy.has_full_disk_read_access() { + ( + "; allow read-only file operations\n(allow file-read*)".to_string(), + Vec::new(), + ) } else { - "" + let mut readable_roots_policies: Vec = Vec::new(); + let mut file_read_params = Vec::new(); + for (index, root) in sandbox_policy + .get_readable_roots_with_cwd(sandbox_policy_cwd) + .into_iter() + .enumerate() + { + // Canonicalize to avoid mismatches like /var vs /private/var on macOS. + let canonical_root = root + .as_path() + .canonicalize() + .unwrap_or_else(|_| root.to_path_buf()); + let root_param = format!("READABLE_ROOT_{index}"); + file_read_params.push((root_param.clone(), canonical_root)); + readable_roots_policies.push(format!("(subpath (param \"{root_param}\"))")); + } + + if readable_roots_policies.is_empty() { + ("".to_string(), Vec::new()) + } else { + ( + format!( + "; allow read-only file operations\n(allow file-read*\n{}\n)", + readable_roots_policies.join(" ") + ), + file_read_params, + ) + } }; let proxy = proxy_policy_inputs(network); @@ -250,7 +280,12 @@ pub(crate) fn create_seatbelt_command_args( "{MACOS_SEATBELT_BASE_POLICY}\n{file_read_policy}\n{file_write_policy}\n{network_policy}" ); - let dir_params = [file_write_dir_params, macos_dir_params()].concat(); + let dir_params = [ + file_read_dir_params, + file_write_dir_params, + macos_dir_params(), + ] + .concat(); let mut seatbelt_args: Vec = vec!["-p".to_string(), full_policy]; let definition_args = dir_params @@ -317,7 +352,7 @@ mod tests { #[test] fn create_seatbelt_args_routes_network_through_proxy_ports() { let policy = dynamic_network_policy( - &SandboxPolicy::ReadOnly, + &SandboxPolicy::new_read_only_policy(), false, &ProxyPolicyInputs { ports: vec![43128, 48081], @@ -351,7 +386,7 @@ mod tests { #[test] fn create_seatbelt_args_allows_local_binding_when_explicitly_enabled() { let policy = dynamic_network_policy( - &SandboxPolicy::ReadOnly, + &SandboxPolicy::new_read_only_policy(), false, &ProxyPolicyInputs { ports: vec![43128], @@ -383,6 +418,7 @@ mod tests { let policy = dynamic_network_policy( &SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: true, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, @@ -410,6 +446,7 @@ mod tests { let policy = dynamic_network_policy( &SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: true, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, @@ -430,6 +467,7 @@ mod tests { let policy = dynamic_network_policy( &SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: true, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, @@ -479,6 +517,7 @@ mod tests { .into_iter() .map(|p| p.try_into().unwrap()) .collect(), + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -664,6 +703,7 @@ mod tests { let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![worktree_root.try_into().expect("worktree_root is absolute")], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -748,6 +788,7 @@ mod tests { // `.codex` checks are done properly for cwd. let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 3dca006d35..12fcc7e844 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -252,7 +252,7 @@ fn unsupported_tool_call_message(payload: &ToolPayload, tool_name: &str) -> Stri fn sandbox_policy_tag(policy: &SandboxPolicy) -> &'static str { match policy { - SandboxPolicy::ReadOnly => "read-only", + SandboxPolicy::ReadOnly { .. } => "read-only", SandboxPolicy::WorkspaceWrite { .. } => "workspace-write", SandboxPolicy::DangerFullAccess => "danger-full-access", SandboxPolicy::ExternalSandbox { .. } => "external-sandbox", diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 56e54e62f1..3671760844 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -326,7 +326,10 @@ mod tests { #[test] fn restricted_sandbox_requires_exec_approval_on_request() { assert_eq!( - default_exec_approval_requirement(AskForApproval::OnRequest, &SandboxPolicy::ReadOnly), + default_exec_approval_requirement( + AskForApproval::OnRequest, + &SandboxPolicy::new_read_only_policy() + ), ExecApprovalRequirement::NeedsApproval { reason: None, proposed_execpolicy_amendment: None, diff --git a/codex-rs/core/tests/suite/apply_patch_cli.rs b/codex-rs/core/tests/suite/apply_patch_cli.rs index ccfa9fe654..112f242bc0 100644 --- a/codex-rs/core/tests/suite/apply_patch_cli.rs +++ b/codex-rs/core/tests/suite/apply_patch_cli.rs @@ -578,6 +578,7 @@ async fn apply_patch_cli_rejects_path_traversal_outside_workspace( let sandbox_policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -634,6 +635,7 @@ async fn apply_patch_cli_rejects_move_path_traversal_outside_workspace( let sandbox_policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index ccf8760c8d..3b5e115ef8 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -628,6 +628,7 @@ fn scenarios() -> Vec { let workspace_write = |network_access| SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, @@ -841,7 +842,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_on_request_requires_approval", approval_policy: OnRequest, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::WriteFile { target: TargetPath::Workspace("ro_on_request.txt"), content: "read-only-approval", @@ -861,7 +862,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_on_request_requires_approval_gpt_5_1_no_exit", approval_policy: OnRequest, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::WriteFile { target: TargetPath::Workspace("ro_on_request_5_1.txt"), content: "read-only-approval", @@ -881,7 +882,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "trusted_command_on_request_read_only_runs_without_prompt", approval_policy: OnRequest, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::RunCommand { command: "echo trusted-read-only", }, @@ -896,7 +897,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "trusted_command_on_request_read_only_runs_without_prompt_gpt_5_1_no_exit", approval_policy: OnRequest, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::RunCommand { command: "echo trusted-read-only", }, @@ -911,7 +912,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_on_request_blocks_network", approval_policy: OnRequest, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::FetchUrl { endpoint: "/ro/network-blocked", response_body: "should-not-see", @@ -925,7 +926,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_on_request_denied_blocks_execution", approval_policy: OnRequest, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::WriteFile { target: TargetPath::Workspace("ro_on_request_denied.txt"), content: "should-not-write", @@ -946,7 +947,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_on_failure_escalates_after_sandbox_error", approval_policy: OnFailure, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::WriteFile { target: TargetPath::Workspace("ro_on_failure.txt"), content: "read-only-on-failure", @@ -967,7 +968,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_on_failure_escalates_after_sandbox_error_gpt_5_1_no_exit", approval_policy: OnFailure, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::WriteFile { target: TargetPath::Workspace("ro_on_failure_5_1.txt"), content: "read-only-on-failure", @@ -987,7 +988,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_on_request_network_escalates_when_approved", approval_policy: OnRequest, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::FetchUrl { endpoint: "/ro/network-approved", response_body: "read-only-network-ok", @@ -1006,7 +1007,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_on_request_network_escalates_when_approved_gpt_5_1_no_exit", approval_policy: OnRequest, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::FetchUrl { endpoint: "/ro/network-approved", response_body: "read-only-network-ok", @@ -1178,7 +1179,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_unless_trusted_requires_approval", approval_policy: UnlessTrusted, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::WriteFile { target: TargetPath::Workspace("ro_unless_trusted.txt"), content: "read-only-unless-trusted", @@ -1198,7 +1199,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_unless_trusted_requires_approval_gpt_5_1_no_exit", approval_policy: UnlessTrusted, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::WriteFile { target: TargetPath::Workspace("ro_unless_trusted_5_1.txt"), content: "read-only-unless-trusted", @@ -1218,7 +1219,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_never_reports_sandbox_failure", approval_policy: Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::WriteFile { target: TargetPath::Workspace("ro_never.txt"), content: "read-only-never", @@ -1242,7 +1243,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "trusted_command_never_runs_without_prompt", approval_policy: Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::RunCommand { command: "echo trusted-never", }, @@ -1407,7 +1408,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "unified exec on request escalated requires approval", approval_policy: OnRequest, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::RunUnifiedExecCommand { command: "python3 -c 'print('\"'\"'escalated unified exec'\"'\"')'", justification: Some(DEFAULT_UNIFIED_EXEC_JUSTIFICATION), @@ -1574,6 +1575,7 @@ async fn approving_apply_patch_for_session_skips_future_prompts_for_same_file() let approval_policy = AskForApproval::OnRequest; let sandbox_policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, @@ -1687,7 +1689,7 @@ async fn approving_apply_patch_for_session_skips_future_prompts_for_same_file() async fn approving_execpolicy_amendment_persists_policy_and_skips_future_prompts() -> Result<()> { let server = start_mock_server().await; let approval_policy = AskForApproval::UnlessTrusted; - let sandbox_policy = SandboxPolicy::ReadOnly; + let sandbox_policy = SandboxPolicy::new_read_only_policy(); let sandbox_policy_for_config = sandbox_policy.clone(); let mut builder = test_codex().with_config(move |config| { config.approval_policy = Constrained::allow_any(approval_policy); diff --git a/codex-rs/core/tests/suite/codex_delegate.rs b/codex-rs/core/tests/suite/codex_delegate.rs index ea759ba5f4..36fa8b4c41 100644 --- a/codex-rs/core/tests/suite/codex_delegate.rs +++ b/codex-rs/core/tests/suite/codex_delegate.rs @@ -63,7 +63,7 @@ async fn codex_delegate_forwards_exec_approval_and_proceeds_on_approval() { // routes ExecApprovalRequest via the parent. let mut builder = test_codex().with_model("gpt-5.1").with_config(|config| { config.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); - config.sandbox_policy = Constrained::allow_any(SandboxPolicy::ReadOnly); + config.sandbox_policy = Constrained::allow_any(SandboxPolicy::new_read_only_policy()); }); let test = builder.build(&server).await.expect("build test codex"); @@ -144,7 +144,7 @@ async fn codex_delegate_forwards_patch_approval_and_proceeds_on_decision() { let mut builder = test_codex().with_model("gpt-5.1").with_config(|config| { config.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); // Use a restricted sandbox so patch approval is required - config.sandbox_policy = Constrained::allow_any(SandboxPolicy::ReadOnly); + config.sandbox_policy = Constrained::allow_any(SandboxPolicy::new_read_only_policy()); config.include_apply_patch_tool = true; }); let test = builder.build(&server).await.expect("build test codex"); diff --git a/codex-rs/core/tests/suite/model_switching.rs b/codex-rs/core/tests/suite/model_switching.rs index fddd3d282d..1c06d40b98 100644 --- a/codex-rs/core/tests/suite/model_switching.rs +++ b/codex-rs/core/tests/suite/model_switching.rs @@ -53,7 +53,7 @@ async fn model_change_appends_model_instructions_developer_message() -> Result<( final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -86,7 +86,7 @@ async fn model_change_appends_model_instructions_developer_message() -> Result<( final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: next_model.to_string(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -141,7 +141,7 @@ async fn model_and_personality_change_only_appends_model_instructions() -> Resul final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -174,7 +174,7 @@ async fn model_and_personality_change_only_appends_model_instructions() -> Resul final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: next_model.to_string(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -289,7 +289,7 @@ async fn model_change_from_image_to_text_strips_prior_image_content() -> Result< final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: image_model_slug.to_string(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -308,7 +308,7 @@ async fn model_change_from_image_to_text_strips_prior_image_content() -> Result< final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: text_model_slug.to_string(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, diff --git a/codex-rs/core/tests/suite/permissions_messages.rs b/codex-rs/core/tests/suite/permissions_messages.rs index fc1f0fa0b6..5bb92114f6 100644 --- a/codex-rs/core/tests/suite/permissions_messages.rs +++ b/codex-rs/core/tests/suite/permissions_messages.rs @@ -457,6 +457,7 @@ async fn permissions_message_includes_writable_roots() -> Result<()> { let writable_root = AbsolutePathBuf::try_from(writable.path())?; let sandbox_policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![writable_root], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/core/tests/suite/personality.rs b/codex-rs/core/tests/suite/personality.rs index 5673e317f6..224cedca43 100644 --- a/codex-rs/core/tests/suite/personality.rs +++ b/codex-rs/core/tests/suite/personality.rs @@ -94,7 +94,7 @@ async fn user_turn_personality_none_does_not_add_update_message() -> anyhow::Res final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -141,7 +141,7 @@ async fn config_personality_some_sets_instructions_template() -> anyhow::Result< final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -195,7 +195,7 @@ async fn config_personality_none_sends_no_personality() -> anyhow::Result<()> { final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -255,7 +255,7 @@ async fn default_personality_is_pragmatic_without_config_toml() -> anyhow::Resul final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -303,7 +303,7 @@ async fn user_turn_personality_some_adds_update_message() -> anyhow::Result<()> final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -337,7 +337,7 @@ async fn user_turn_personality_some_adds_update_message() -> anyhow::Result<()> final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -400,7 +400,7 @@ async fn user_turn_personality_same_value_does_not_add_update_message() -> anyho final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -434,7 +434,7 @@ async fn user_turn_personality_same_value_does_not_add_update_message() -> anyho final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -506,7 +506,7 @@ async fn user_turn_personality_skips_if_feature_disabled() -> anyhow::Result<()> final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -540,7 +540,7 @@ async fn user_turn_personality_skips_if_feature_disabled() -> anyhow::Result<()> final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -652,7 +652,7 @@ async fn ignores_remote_personality_if_remote_models_disabled() -> anyhow::Resul final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: remote_slug.to_string(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -769,7 +769,7 @@ async fn remote_model_friendly_personality_instructions_with_feature() -> anyhow final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: remote_slug.to_string(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -884,7 +884,7 @@ async fn user_turn_personality_remote_model_template_includes_update_message() - final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: remote_slug.to_string(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -918,7 +918,7 @@ async fn user_turn_personality_remote_model_template_includes_update_message() - final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: remote_slug.to_string(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index e1ef097e20..64a9b55f8b 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -375,6 +375,7 @@ async fn overrides_turn_context_but_keeps_cached_prefix_and_key_constant() -> an let writable = TempDir::new().unwrap(); let new_policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![writable.path().try_into().unwrap()], + read_only_access: Default::default(), network_access: true, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -618,6 +619,7 @@ async fn per_turn_overrides_keep_cached_prefix_and_key_constant() -> anyhow::Res let writable = TempDir::new().unwrap(); let new_policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![AbsolutePathBuf::try_from(writable.path()).unwrap()], + read_only_access: Default::default(), network_access: true, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index a1bf72b103..46df9ca17e 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -126,7 +126,7 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> { final_output_json_schema: None, cwd: fixture.cwd.path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: session_model, effort: None, summary: ReasoningSummary::Auto, @@ -293,7 +293,7 @@ async fn stdio_image_responses_round_trip() -> anyhow::Result<()> { final_output_json_schema: None, cwd: fixture.cwd.path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: session_model, effort: None, summary: ReasoningSummary::Auto, @@ -491,7 +491,7 @@ async fn stdio_image_responses_are_sanitized_for_text_only_model() -> anyhow::Re final_output_json_schema: None, cwd: fixture.cwd.path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: text_only_model_slug.to_string(), effort: None, summary: ReasoningSummary::Auto, @@ -603,7 +603,7 @@ async fn stdio_server_propagates_whitelisted_env_vars() -> anyhow::Result<()> { final_output_json_schema: None, cwd: fixture.cwd.path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: session_model, effort: None, summary: ReasoningSummary::Auto, @@ -762,7 +762,7 @@ async fn streamable_http_tool_call_round_trip() -> anyhow::Result<()> { final_output_json_schema: None, cwd: fixture.cwd.path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: session_model, effort: None, summary: ReasoningSummary::Auto, @@ -953,7 +953,7 @@ async fn streamable_http_with_oauth_round_trip() -> anyhow::Result<()> { final_output_json_schema: None, cwd: fixture.cwd.path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: session_model, effort: None, summary: ReasoningSummary::Auto, diff --git a/codex-rs/core/tests/suite/seatbelt.rs b/codex-rs/core/tests/suite/seatbelt.rs index 614e87367a..89674494a9 100644 --- a/codex-rs/core/tests/suite/seatbelt.rs +++ b/codex-rs/core/tests/suite/seatbelt.rs @@ -77,6 +77,7 @@ async fn if_parent_of_repo_is_writable_then_dot_git_folder_is_writable() { let test_scenario = create_test_scenario(&tmp); let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![test_scenario.repo_parent.as_path().try_into().unwrap()], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -103,6 +104,7 @@ async fn if_git_repo_is_writable_root_then_dot_git_folder_is_read_only() { let test_scenario = create_test_scenario(&tmp); let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![test_scenario.repo_root.as_path().try_into().unwrap()], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -145,7 +147,7 @@ async fn danger_full_access_allows_all_writes() { async fn read_only_forbids_all_writes() { let tmp = TempDir::new().expect("should be able to create temp dir"); let test_scenario = create_test_scenario(&tmp); - let policy = SandboxPolicy::ReadOnly; + let policy = SandboxPolicy::new_read_only_policy(); test_scenario .run_test( @@ -171,7 +173,7 @@ async fn openpty_works_under_seatbelt() { return; } - let policy = SandboxPolicy::ReadOnly; + let policy = SandboxPolicy::new_read_only_policy(); let command_cwd = std::env::current_dir().expect("getcwd"); let sandbox_cwd = command_cwd.clone(); @@ -229,7 +231,7 @@ async fn java_home_finds_runtime_under_seatbelt() { return; } - let policy = SandboxPolicy::ReadOnly; + let policy = SandboxPolicy::new_read_only_policy(); let command_cwd = std::env::current_dir().expect("getcwd"); let sandbox_cwd = command_cwd.clone(); diff --git a/codex-rs/core/tests/suite/tools.rs b/codex-rs/core/tests/suite/tools.rs index 7efa8bb28e..a828071ea5 100644 --- a/codex-rs/core/tests/suite/tools.rs +++ b/codex-rs/core/tests/suite/tools.rs @@ -230,7 +230,7 @@ async fn sandbox_denied_shell_returns_original_output() -> Result<()> { fixture .submit_turn_with_policy( "run a command that should be denied by the read-only sandbox", - SandboxPolicy::ReadOnly, + SandboxPolicy::new_read_only_policy(), ) .await?; diff --git a/codex-rs/core/tests/suite/truncation.rs b/codex-rs/core/tests/suite/truncation.rs index b36fef9eac..9475a9c510 100644 --- a/codex-rs/core/tests/suite/truncation.rs +++ b/codex-rs/core/tests/suite/truncation.rs @@ -384,7 +384,7 @@ async fn mcp_tool_call_output_exceeds_limit_truncated_for_model() -> Result<()> fixture .submit_turn_with_policy( "call the rmcp echo tool with a very large message", - SandboxPolicy::ReadOnly, + SandboxPolicy::new_read_only_policy(), ) .await?; @@ -485,7 +485,7 @@ async fn mcp_image_output_preserves_image_and_no_text_summary() -> Result<()> { final_output_json_schema: None, cwd: fixture.cwd.path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: session_model, effort: None, summary: ReasoningSummary::Auto, @@ -742,7 +742,7 @@ async fn mcp_tool_call_output_not_truncated_with_custom_limit() -> Result<()> { fixture .submit_turn_with_policy( "call the rmcp echo tool with a very large message", - SandboxPolicy::ReadOnly, + SandboxPolicy::new_read_only_policy(), ) .await?; diff --git a/codex-rs/core/tests/suite/unified_exec.rs b/codex-rs/core/tests/suite/unified_exec.rs index c7b21413c5..34c5bd9617 100644 --- a/codex-rs/core/tests/suite/unified_exec.rs +++ b/codex-rs/core/tests/suite/unified_exec.rs @@ -2540,7 +2540,7 @@ async fn unified_exec_runs_under_sandbox() -> Result<()> { cwd: cwd.path().to_path_buf(), approval_policy: AskForApproval::Never, // Important! - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: session_model, effort: None, summary: ReasoningSummary::Auto, @@ -2644,7 +2644,7 @@ async fn unified_exec_python_prompt_under_seatbelt() -> Result<()> { final_output_json_schema: None, cwd: cwd.path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: session_model, effort: None, summary: ReasoningSummary::Auto, diff --git a/codex-rs/core/tests/suite/web_search.rs b/codex-rs/core/tests/suite/web_search.rs index 65e8aedbed..8909e01cf4 100644 --- a/codex-rs/core/tests/suite/web_search.rs +++ b/codex-rs/core/tests/suite/web_search.rs @@ -44,9 +44,12 @@ async fn web_search_mode_cached_sets_external_web_access_false() { .await .expect("create test Codex conversation"); - test.submit_turn_with_policy("hello cached web search", SandboxPolicy::ReadOnly) - .await - .expect("submit turn"); + test.submit_turn_with_policy( + "hello cached web search", + SandboxPolicy::new_read_only_policy(), + ) + .await + .expect("submit turn"); let body = resp_mock.single_request().body_json(); let tool = find_web_search_tool(&body); @@ -82,9 +85,12 @@ async fn web_search_mode_takes_precedence_over_legacy_flags() { .await .expect("create test Codex conversation"); - test.submit_turn_with_policy("hello cached+live flags", SandboxPolicy::ReadOnly) - .await - .expect("submit turn"); + test.submit_turn_with_policy( + "hello cached+live flags", + SandboxPolicy::new_read_only_policy(), + ) + .await + .expect("submit turn"); let body = resp_mock.single_request().body_json(); let tool = find_web_search_tool(&body); @@ -121,9 +127,12 @@ async fn web_search_mode_defaults_to_cached_when_features_disabled() { .await .expect("create test Codex conversation"); - test.submit_turn_with_policy("hello default cached web search", SandboxPolicy::ReadOnly) - .await - .expect("submit turn"); + test.submit_turn_with_policy( + "hello default cached web search", + SandboxPolicy::new_read_only_policy(), + ) + .await + .expect("submit turn"); let body = resp_mock.single_request().body_json(); let tool = find_web_search_tool(&body); @@ -169,7 +178,7 @@ async fn web_search_mode_updates_between_turns_with_sandbox_policy() { .await .expect("create test Codex conversation"); - test.submit_turn_with_policy("hello cached", SandboxPolicy::ReadOnly) + test.submit_turn_with_policy("hello cached", SandboxPolicy::new_read_only_policy()) .await .expect("submit first turn"); test.submit_turn_with_policy("hello live", SandboxPolicy::DangerFullAccess) diff --git a/codex-rs/exec-server/src/posix/mcp.rs b/codex-rs/exec-server/src/posix/mcp.rs index 4cfce9ff89..7e8d0ccdff 100644 --- a/codex-rs/exec-server/src/posix/mcp.rs +++ b/codex-rs/exec-server/src/posix/mcp.rs @@ -123,7 +123,7 @@ impl ExecTool { .await .clone() .unwrap_or_else(|| SandboxState { - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), codex_linux_sandbox_exe: None, sandbox_cwd: PathBuf::from(¶ms.workdir), use_linux_sandbox_bwrap: false, diff --git a/codex-rs/exec-server/tests/common/lib.rs b/codex-rs/exec-server/tests/common/lib.rs index a51a589adf..430eb89953 100644 --- a/codex-rs/exec-server/tests/common/lib.rs +++ b/codex-rs/exec-server/tests/common/lib.rs @@ -88,7 +88,7 @@ where S: Service + ClientHandler, { let sandbox_state = SandboxState { - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), codex_linux_sandbox_exe, sandbox_cwd: sandbox_cwd.as_ref().to_path_buf(), use_linux_sandbox_bwrap: false, @@ -110,6 +110,7 @@ where // Note that sandbox_cwd will already be included as a writable root // when the sandbox policy is expanded. writable_roots: vec![], + read_only_access: Default::default(), network_access: false, // Disable writes to temp dir because this is a test, so // writable_folder is likely also under /tmp and we want to be diff --git a/codex-rs/exec/tests/event_processor_with_json_output.rs b/codex-rs/exec/tests/event_processor_with_json_output.rs index 4e669d232e..e058830413 100644 --- a/codex-rs/exec/tests/event_processor_with_json_output.rs +++ b/codex-rs/exec/tests/event_processor_with_json_output.rs @@ -92,7 +92,7 @@ fn session_configured_produces_thread_started_event() { model: "codex-mini-latest".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, history_log_id: 0, diff --git a/codex-rs/exec/tests/suite/sandbox.rs b/codex-rs/exec/tests/suite/sandbox.rs index 45de9c2236..ce2e79eac2 100644 --- a/codex-rs/exec/tests/suite/sandbox.rs +++ b/codex-rs/exec/tests/suite/sandbox.rs @@ -69,7 +69,7 @@ async fn spawn_command_under_sandbox( async fn linux_sandbox_test_env() -> Option> { let command_cwd = std::env::current_dir().ok()?; let sandbox_cwd = command_cwd.clone(); - let policy = SandboxPolicy::ReadOnly; + let policy = SandboxPolicy::new_read_only_policy(); if can_apply_linux_sandbox_policy(&policy, &command_cwd, sandbox_cwd.as_path(), HashMap::new()) .await @@ -134,6 +134,7 @@ async fn python_multiprocessing_lock_works_under_sandbox() { let policy = SandboxPolicy::WorkspaceWrite { writable_roots, + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, @@ -194,7 +195,7 @@ async fn python_getpwuid_works_under_sandbox() { return; } - let policy = SandboxPolicy::ReadOnly; + let policy = SandboxPolicy::new_read_only_policy(); let command_cwd = std::env::current_dir().expect("should be able to get current dir"); let sandbox_cwd = command_cwd.clone(); @@ -247,6 +248,7 @@ async fn sandbox_distinguishes_command_and_policy_cwds() { // is under a writable root. let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -387,7 +389,7 @@ fn unix_sock_body() { async fn allow_unix_socketpair_recvfrom() { run_code_under_sandbox( "allow_unix_socketpair_recvfrom", - &SandboxPolicy::ReadOnly, + &SandboxPolicy::new_read_only_policy(), || async { unix_sock_body() }, ) .await diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 1a835fd5e0..7ff7acf5ef 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -141,6 +141,13 @@ fn create_bwrap_flags( /// 4. `--dev-bind /dev/null /dev/null` preserves the common sink even under a /// read-only root. fn create_filesystem_args(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Result> { + if !sandbox_policy.has_full_disk_read_access() { + return Err(CodexErr::UnsupportedOperation( + "Restricted read-only access is not yet supported by the Linux bubblewrap backend." + .to_string(), + )); + } + let writable_roots = sandbox_policy.get_writable_roots_with_cwd(cwd); ensure_mount_targets_exist(&writable_roots)?; diff --git a/codex-rs/linux-sandbox/src/landlock.rs b/codex-rs/linux-sandbox/src/landlock.rs index d86fafd297..8d568dba0a 100644 --- a/codex-rs/linux-sandbox/src/landlock.rs +++ b/codex-rs/linux-sandbox/src/landlock.rs @@ -62,6 +62,13 @@ pub(crate) fn apply_sandbox_policy_to_current_thread( } if apply_landlock_fs && !sandbox_policy.has_full_disk_write_access() { + if !sandbox_policy.has_full_disk_read_access() { + return Err(CodexErr::UnsupportedOperation( + "Restricted read-only access is not supported by the legacy Linux Landlock filesystem backend." + .to_string(), + )); + } + let writable_roots = sandbox_policy .get_writable_roots_with_cwd(cwd) .into_iter() @@ -70,9 +77,6 @@ pub(crate) fn apply_sandbox_policy_to_current_thread( install_filesystem_landlock_rules_on_current_thread(writable_roots)?; } - // TODO(ragona): Add appropriate restrictions if - // `sandbox_policy.has_full_disk_read_access()` is `false`. - Ok(()) } @@ -222,11 +226,11 @@ mod tests { #[test] fn restricted_network_policy_always_installs_seccomp() { assert!(should_install_network_seccomp( - &SandboxPolicy::ReadOnly, + &SandboxPolicy::new_read_only_policy(), false )); assert!(should_install_network_seccomp( - &SandboxPolicy::ReadOnly, + &SandboxPolicy::new_read_only_policy(), true )); } diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 2978efae2a..1df9a54438 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -391,7 +391,7 @@ mod tests { fn inserts_bwrap_argv0_before_command_separator() { let argv = build_bwrap_argv( vec!["/bin/true".to_string()], - &SandboxPolicy::ReadOnly, + &SandboxPolicy::new_read_only_policy(), Path::new("/"), BwrapOptions { mount_proc: true, @@ -425,7 +425,7 @@ mod tests { fn inserts_unshare_net_when_network_isolation_requested() { let argv = build_bwrap_argv( vec!["/bin/true".to_string()], - &SandboxPolicy::ReadOnly, + &SandboxPolicy::new_read_only_policy(), Path::new("/"), BwrapOptions { mount_proc: true, @@ -439,7 +439,7 @@ mod tests { fn inserts_unshare_net_when_proxy_only_network_mode_requested() { let argv = build_bwrap_argv( vec!["/bin/true".to_string()], - &SandboxPolicy::ReadOnly, + &SandboxPolicy::new_read_only_policy(), Path::new("/"), BwrapOptions { mount_proc: true, diff --git a/codex-rs/linux-sandbox/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index 6623a3f09f..84fa1d2c3e 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -87,6 +87,7 @@ async fn run_cmd_result_with_writable_roots( .iter() .map(|p| AbsolutePathBuf::try_from(p.as_path()).unwrap()) .collect(), + read_only_access: Default::default(), network_access: false, // Exclude tmp-related folders from writable roots because we need a // folder that is writable by tests but that we intentionally disallow diff --git a/codex-rs/mcp-server/src/outgoing_message.rs b/codex-rs/mcp-server/src/outgoing_message.rs index 1bf98534b0..3f83e1990e 100644 --- a/codex-rs/mcp-server/src/outgoing_message.rs +++ b/codex-rs/mcp-server/src/outgoing_message.rs @@ -301,7 +301,7 @@ mod tests { model: "gpt-4o".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffort::default()), history_log_id: 1, @@ -343,7 +343,7 @@ mod tests { model: "gpt-4o".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffort::default()), history_log_id: 1, @@ -409,7 +409,7 @@ mod tests { model: "gpt-4o".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffort::default()), history_log_id: 1, diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 7f33199702..b5ce0f2b2c 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -312,7 +312,7 @@ impl DeveloperInstructions { let (sandbox_mode, writable_roots) = match sandbox_policy { SandboxPolicy::DangerFullAccess => (SandboxMode::DangerFullAccess, None), - SandboxPolicy::ReadOnly => (SandboxMode::ReadOnly, None), + SandboxPolicy::ReadOnly { .. } => (SandboxMode::ReadOnly, None), SandboxPolicy::ExternalSandbox { .. } => (SandboxMode::DangerFullAccess, None), SandboxPolicy::WorkspaceWrite { .. } => { let roots = sandbox_policy.get_writable_roots_with_cwd(cwd); @@ -1223,6 +1223,7 @@ mod tests { fn builds_permissions_from_policy() { let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: true, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index a56f55ba9a..904c5bdbf7 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -4,6 +4,7 @@ //! between user and agent. use std::collections::HashMap; +use std::collections::HashSet; use std::ffi::OsStr; use std::fmt; use std::path::Path; @@ -382,6 +383,107 @@ impl NetworkAccess { } } +fn default_include_platform_defaults() -> bool { + true +} + +/// Determines how read-only file access is granted inside a restricted +/// sandbox. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Display, Default, JsonSchema, TS)] +#[strum(serialize_all = "kebab-case")] +#[serde(tag = "type", rename_all = "kebab-case")] +#[ts(tag = "type")] +pub enum ReadOnlyAccess { + /// Restrict reads to an explicit set of roots. + /// + /// When `include_platform_defaults` is `true`, platform defaults required + /// for basic execution are included in addition to `readable_roots`. + Restricted { + /// Include built-in platform read roots required for basic process + /// execution. + #[serde(default = "default_include_platform_defaults")] + include_platform_defaults: bool, + /// Additional absolute roots that should be readable. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + readable_roots: Vec, + }, + + /// Allow unrestricted file reads. + #[default] + FullAccess, +} + +impl ReadOnlyAccess { + pub fn has_full_disk_read_access(&self) -> bool { + matches!(self, ReadOnlyAccess::FullAccess) + } + + /// Returns the readable roots for restricted read access. + /// + /// For [`ReadOnlyAccess::FullAccess`], returns an empty list because + /// callers should grant blanket read access instead. + pub fn get_readable_roots_with_cwd(&self, cwd: &Path) -> Vec { + let mut roots: Vec = match self { + ReadOnlyAccess::FullAccess => return Vec::new(), + ReadOnlyAccess::Restricted { + include_platform_defaults, + readable_roots, + } => { + let mut roots = readable_roots.clone(); + if *include_platform_defaults { + #[cfg(target_os = "macos")] + for platform_path in [ + "/bin", "/dev", "/etc", "/Library", "/private", "/sbin", "/System", "/tmp", + "/usr", + ] { + #[allow(clippy::expect_used)] + roots.push( + AbsolutePathBuf::from_absolute_path(platform_path) + .expect("platform defaults should be absolute"), + ); + } + + #[cfg(target_os = "linux")] + for platform_path in ["/bin", "/dev", "/etc", "/lib", "/lib64", "/tmp", "/usr"] + { + #[allow(clippy::expect_used)] + roots.push( + AbsolutePathBuf::from_absolute_path(platform_path) + .expect("platform defaults should be absolute"), + ); + } + + #[cfg(target_os = "windows")] + for platform_path in [ + r"C:\Windows", + r"C:\Program Files", + r"C:\Program Files (x86)", + r"C:\ProgramData", + ] { + #[allow(clippy::expect_used)] + roots.push( + AbsolutePathBuf::from_absolute_path(platform_path) + .expect("platform defaults should be absolute"), + ); + } + + match AbsolutePathBuf::from_absolute_path(cwd) { + Ok(cwd_root) => roots.push(cwd_root), + Err(err) => { + error!("Ignoring invalid cwd {cwd:?} for sandbox readable root: {err}"); + } + } + } + roots + } + }; + + let mut seen = HashSet::new(); + roots.retain(|root| seen.insert(root.to_path_buf())); + roots + } +} + /// Determines execution restrictions for model shell commands. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Display, JsonSchema, TS)] #[strum(serialize_all = "kebab-case")] @@ -391,9 +493,16 @@ pub enum SandboxPolicy { #[serde(rename = "danger-full-access")] DangerFullAccess, - /// Read-only access to the entire file-system. + /// Read-only access configuration. #[serde(rename = "read-only")] - ReadOnly, + ReadOnly { + /// Read access granted while running under this policy. + #[serde( + default, + skip_serializing_if = "ReadOnlyAccess::has_full_disk_read_access" + )] + access: ReadOnlyAccess, + }, /// Indicates the process is already in an external sandbox. Allows full /// disk access while honoring the provided network setting. @@ -413,6 +522,13 @@ pub enum SandboxPolicy { #[serde(default, skip_serializing_if = "Vec::is_empty")] writable_roots: Vec, + /// Read access granted while running under this policy. + #[serde( + default, + skip_serializing_if = "ReadOnlyAccess::has_full_disk_read_access" + )] + read_only_access: ReadOnlyAccess, + /// When set to `true`, outbound network access is allowed. `false` by /// default. #[serde(default)] @@ -473,7 +589,9 @@ impl FromStr for SandboxPolicy { impl SandboxPolicy { /// Returns a policy with read-only disk access and no network. pub fn new_read_only_policy() -> Self { - SandboxPolicy::ReadOnly + SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::FullAccess, + } } /// Returns a policy that can read the entire disk, but can only write to @@ -482,22 +600,29 @@ impl SandboxPolicy { pub fn new_workspace_write_policy() -> Self { SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: ReadOnlyAccess::FullAccess, network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, } } - /// Always returns `true`; restricting read access is not supported. pub fn has_full_disk_read_access(&self) -> bool { - true + match self { + SandboxPolicy::DangerFullAccess => true, + SandboxPolicy::ExternalSandbox { .. } => true, + SandboxPolicy::ReadOnly { access } => access.has_full_disk_read_access(), + SandboxPolicy::WorkspaceWrite { + read_only_access, .. + } => read_only_access.has_full_disk_read_access(), + } } pub fn has_full_disk_write_access(&self) -> bool { match self { SandboxPolicy::DangerFullAccess => true, SandboxPolicy::ExternalSandbox { .. } => true, - SandboxPolicy::ReadOnly => false, + SandboxPolicy::ReadOnly { .. } => false, SandboxPolicy::WorkspaceWrite { .. } => false, } } @@ -506,11 +631,35 @@ impl SandboxPolicy { match self { SandboxPolicy::DangerFullAccess => true, SandboxPolicy::ExternalSandbox { network_access } => network_access.is_enabled(), - SandboxPolicy::ReadOnly => false, + SandboxPolicy::ReadOnly { .. } => false, SandboxPolicy::WorkspaceWrite { network_access, .. } => *network_access, } } + /// Returns the list of readable roots (tailored to the current working + /// directory) when read access is restricted. + /// + /// For policies with full read access, this returns an empty list because + /// callers should grant blanket reads. + pub fn get_readable_roots_with_cwd(&self, cwd: &Path) -> Vec { + let mut roots = match self { + SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => Vec::new(), + SandboxPolicy::ReadOnly { access } => access.get_readable_roots_with_cwd(cwd), + SandboxPolicy::WorkspaceWrite { + writable_roots, + read_only_access, + .. + } => { + let mut roots = read_only_access.get_readable_roots_with_cwd(cwd); + roots.extend(writable_roots.clone()); + roots + } + }; + let mut seen = HashSet::new(); + roots.retain(|root| seen.insert(root.to_path_buf())); + roots + } + /// Returns the list of writable roots (tailored to the current working /// directory) together with subpaths that should remain read‑only under /// each writable root. @@ -518,9 +667,10 @@ impl SandboxPolicy { match self { SandboxPolicy::DangerFullAccess => Vec::new(), SandboxPolicy::ExternalSandbox { .. } => Vec::new(), - SandboxPolicy::ReadOnly => Vec::new(), + SandboxPolicy::ReadOnly { .. } => Vec::new(), SandboxPolicy::WorkspaceWrite { writable_roots, + read_only_access: _, exclude_tmpdir_env_var, exclude_slash_tmp, network_access: _, @@ -2728,7 +2878,7 @@ mod tests { model: "codex-mini-latest".to_string(), model_provider_id: "openai".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffortConfig::default()), history_log_id: 0, diff --git a/codex-rs/state/src/model/thread_metadata.rs b/codex-rs/state/src/model/thread_metadata.rs index 2577ead502..d29bf07606 100644 --- a/codex-rs/state/src/model/thread_metadata.rs +++ b/codex-rs/state/src/model/thread_metadata.rs @@ -138,7 +138,7 @@ impl ThreadMetadataBuilder { model_provider: None, cwd: PathBuf::new(), cli_version: None, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), approval_mode: AskForApproval::OnRequest, archived_at: None, git_sha: None, diff --git a/codex-rs/state/src/runtime.rs b/codex-rs/state/src/runtime.rs index dcb472022b..ea9470d96e 100644 --- a/codex-rs/state/src/runtime.rs +++ b/codex-rs/state/src/runtime.rs @@ -1998,7 +1998,7 @@ WHERE kind = 'memory_stage1' cwd, cli_version: "0.0.0".to_string(), title: String::new(), - sandbox_policy: crate::extract::enum_to_string(&SandboxPolicy::ReadOnly), + sandbox_policy: crate::extract::enum_to_string(&SandboxPolicy::new_read_only_policy()), approval_mode: crate::extract::enum_to_string(&AskForApproval::OnRequest), tokens_used: 0, first_user_message: Some("hello".to_string()), diff --git a/codex-rs/tui/src/additional_dirs.rs b/codex-rs/tui/src/additional_dirs.rs index 54746c1705..98724aa161 100644 --- a/codex-rs/tui/src/additional_dirs.rs +++ b/codex-rs/tui/src/additional_dirs.rs @@ -16,7 +16,7 @@ pub fn add_dir_warning_message( SandboxPolicy::WorkspaceWrite { .. } | SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => None, - SandboxPolicy::ReadOnly => Some(format_warning(additional_dirs)), + SandboxPolicy::ReadOnly { .. } => Some(format_warning(additional_dirs)), } } @@ -64,7 +64,7 @@ mod tests { #[test] fn warns_for_read_only() { - let sandbox = SandboxPolicy::ReadOnly; + let sandbox = SandboxPolicy::new_read_only_policy(); let dirs = vec![PathBuf::from("relative"), PathBuf::from("/abs")]; let message = add_dir_warning_message(&dirs, &sandbox) .expect("expected warning for read-only sandbox"); @@ -76,7 +76,7 @@ mod tests { #[test] fn returns_none_when_no_additional_dirs() { - let sandbox = SandboxPolicy::ReadOnly; + let sandbox = SandboxPolicy::new_read_only_policy(); let dirs: Vec = Vec::new(); assert_eq!(add_dir_warning_message(&dirs, &sandbox), None); } diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index d30b747ebc..8fce522562 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -1156,7 +1156,7 @@ impl App { && matches!( app.config.sandbox_policy.get(), codex_core::protocol::SandboxPolicy::WorkspaceWrite { .. } - | codex_core::protocol::SandboxPolicy::ReadOnly + | codex_core::protocol::SandboxPolicy::ReadOnly { .. } ) && !app .config @@ -1987,7 +1987,7 @@ impl App { let policy_is_workspace_write_or_ro = matches!( &policy, codex_core::protocol::SandboxPolicy::WorkspaceWrite { .. } - | codex_core::protocol::SandboxPolicy::ReadOnly + | codex_core::protocol::SandboxPolicy::ReadOnly { .. } ); if let Err(err) = self.config.sandbox_policy.set(policy.clone()) { @@ -1997,9 +1997,11 @@ impl App { return Ok(AppRunControl::Continue); } #[cfg(target_os = "windows")] - if !matches!(&policy, codex_core::protocol::SandboxPolicy::ReadOnly) - || WindowsSandboxLevel::from_config(&self.config) - != WindowsSandboxLevel::Disabled + if !matches!( + &policy, + codex_core::protocol::SandboxPolicy::ReadOnly { .. } + ) || WindowsSandboxLevel::from_config(&self.config) + != WindowsSandboxLevel::Disabled { self.config.forced_auto_mode_downgraded_on_windows = false; } @@ -3053,7 +3055,7 @@ mod tests { model: "gpt-test".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, history_log_id: 0, @@ -3108,7 +3110,7 @@ mod tests { model: "gpt-test".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, history_log_id: 0, @@ -3360,7 +3362,7 @@ mod tests { model: "gpt-test".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, history_log_id: 0, diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 18c4b9b162..92e9338217 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -5414,15 +5414,16 @@ impl ChatWidget { } matches!( (&preset.sandbox, current_sandbox), - (SandboxPolicy::ReadOnly, SandboxPolicy::ReadOnly) - | ( - SandboxPolicy::DangerFullAccess, - SandboxPolicy::DangerFullAccess - ) - | ( - SandboxPolicy::WorkspaceWrite { .. }, - SandboxPolicy::WorkspaceWrite { .. } - ) + ( + SandboxPolicy::ReadOnly { .. }, + SandboxPolicy::ReadOnly { .. } + ) | ( + SandboxPolicy::DangerFullAccess, + SandboxPolicy::DangerFullAccess + ) | ( + SandboxPolicy::WorkspaceWrite { .. }, + SandboxPolicy::WorkspaceWrite { .. } + ) ) } @@ -5543,7 +5544,7 @@ impl ChatWidget { let mut header_children: Vec> = Vec::new(); let describe_policy = |policy: &SandboxPolicy| match policy { SandboxPolicy::WorkspaceWrite { .. } => "Agent mode", - SandboxPolicy::ReadOnly => "Read-Only mode", + SandboxPolicy::ReadOnly { .. } => "Read-Only mode", _ => "Agent mode", }; let mode_label = preset @@ -5955,7 +5956,7 @@ impl ChatWidget { /// Set the sandbox policy in the widget's config copy. pub(crate) fn set_sandbox_policy(&mut self, policy: SandboxPolicy) -> ConstraintResult<()> { #[cfg(target_os = "windows")] - let should_clear_downgrade = !matches!(&policy, SandboxPolicy::ReadOnly) + let should_clear_downgrade = !matches!(&policy, SandboxPolicy::ReadOnly { .. }) || WindowsSandboxLevel::from_config(&self.config) != WindowsSandboxLevel::Disabled; self.config.sandbox_policy.set(policy)?; diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 6fdefb36c4..0e6a6e49ac 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -149,7 +149,7 @@ async fn resumed_initial_messages_render_history() { model: "test-model".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffortConfig::default()), history_log_id: 0, @@ -217,7 +217,7 @@ async fn replayed_user_message_preserves_text_elements_and_local_images() { model: "test-model".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffortConfig::default()), history_log_id: 0, @@ -335,7 +335,7 @@ async fn submission_preserves_text_elements_and_local_images() { model: "test-model".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffortConfig::default()), history_log_id: 0, @@ -415,7 +415,7 @@ async fn submission_prefers_selected_duplicate_skill_path() { model: "test-model".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffortConfig::default()), history_log_id: 0, @@ -3005,7 +3005,7 @@ async fn plan_slash_command_with_args_submits_prompt_in_plan_mode() { model: "test-model".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffortConfig::default()), history_log_id: 0, @@ -4068,6 +4068,7 @@ async fn preset_matching_ignores_extra_writable_roots() { .expect("auto preset exists"); let current_sandbox = SandboxPolicy::WorkspaceWrite { writable_roots: vec![AbsolutePathBuf::try_from("C:\\extra").unwrap()], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/tui/src/debug_config.rs b/codex-rs/tui/src/debug_config.rs index 681d9c3a9a..80e97d5dd8 100644 --- a/codex-rs/tui/src/debug_config.rs +++ b/codex-rs/tui/src/debug_config.rs @@ -403,7 +403,7 @@ mod tests { Some(RequirementSource::CloudRequirements), ); requirements.sandbox_policy = ConstrainedWithSource::new( - Constrained::allow_any(SandboxPolicy::ReadOnly), + Constrained::allow_any(SandboxPolicy::new_read_only_policy()), Some(RequirementSource::SystemRequirementsToml { file: requirements_file.clone(), }), diff --git a/codex-rs/tui/src/status/card.rs b/codex-rs/tui/src/status/card.rs index 23005274db..61421976f5 100644 --- a/codex-rs/tui/src/status/card.rs +++ b/codex-rs/tui/src/status/card.rs @@ -157,7 +157,7 @@ impl StatusHistoryCell { .unwrap_or_else(|| "".to_string()); let sandbox = match config.sandbox_policy.get() { SandboxPolicy::DangerFullAccess => "danger-full-access".to_string(), - SandboxPolicy::ReadOnly => "read-only".to_string(), + SandboxPolicy::ReadOnly { .. } => "read-only".to_string(), SandboxPolicy::WorkspaceWrite { .. } => "workspace-write".to_string(), SandboxPolicy::ExternalSandbox { network_access } => { if matches!(network_access, NetworkAccess::Enabled) { diff --git a/codex-rs/tui/src/status/tests.rs b/codex-rs/tui/src/status/tests.rs index c8844904ab..c9bd2c6e67 100644 --- a/codex-rs/tui/src/status/tests.rs +++ b/codex-rs/tui/src/status/tests.rs @@ -100,6 +100,7 @@ async fn status_snapshot_includes_reasoning_details() { .sandbox_policy .set(SandboxPolicy::WorkspaceWrite { writable_roots: Vec::new(), + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/windows-sandbox-rs/src/allow.rs b/codex-rs/windows-sandbox-rs/src/allow.rs index 83d72f7e55..d2a2f94915 100644 --- a/codex-rs/windows-sandbox-rs/src/allow.rs +++ b/codex-rs/windows-sandbox-rs/src/allow.rs @@ -108,6 +108,7 @@ mod tests { let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![AbsolutePathBuf::try_from(extra_root.as_path()).unwrap()], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, @@ -134,6 +135,7 @@ mod tests { let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: false, @@ -161,6 +163,7 @@ mod tests { let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: false, @@ -188,6 +191,7 @@ mod tests { let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: false, @@ -213,6 +217,7 @@ mod tests { let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: false, diff --git a/codex-rs/windows-sandbox-rs/src/audit.rs b/codex-rs/windows-sandbox-rs/src/audit.rs index 1af9f1b919..2aefb7a3fd 100644 --- a/codex-rs/windows-sandbox-rs/src/audit.rs +++ b/codex-rs/windows-sandbox-rs/src/audit.rs @@ -262,7 +262,7 @@ pub fn apply_capability_denies_for_world_writable( } (sid, roots) } - SandboxPolicy::ReadOnly => ( + SandboxPolicy::ReadOnly { .. } => ( unsafe { convert_string_sid_to_sid(&caps.readonly) }.ok_or_else(|| { anyhow!("ConvertStringSidToSidW failed for readonly capability") })?, diff --git a/codex-rs/windows-sandbox-rs/src/command_runner_win.rs b/codex-rs/windows-sandbox-rs/src/command_runner_win.rs index 9ff61de932..da949f8778 100644 --- a/codex-rs/windows-sandbox-rs/src/command_runner_win.rs +++ b/codex-rs/windows-sandbox-rs/src/command_runner_win.rs @@ -114,6 +114,11 @@ pub fn main() -> Result<()> { ); let policy = parse_policy(&req.policy_json_or_preset).context("parse policy_json_or_preset")?; + if !policy.has_full_disk_read_access() { + anyhow::bail!( + "Restricted read-only access is not yet supported by the Windows sandbox backend" + ); + } let mut cap_psids: Vec<*mut c_void> = Vec::new(); for sid in &req.cap_sids { let Some(psid) = (unsafe { convert_string_sid_to_sid(sid) }) else { @@ -129,7 +134,9 @@ pub fn main() -> Result<()> { let base = unsafe { get_current_token_for_restriction()? }; let token_res: Result = unsafe { match &policy { - SandboxPolicy::ReadOnly => create_readonly_token_with_caps_from(base, &cap_psids), + SandboxPolicy::ReadOnly { .. } => { + create_readonly_token_with_caps_from(base, &cap_psids) + } SandboxPolicy::WorkspaceWrite { .. } => { create_workspace_write_token_with_caps_from(base, &cap_psids) } diff --git a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs index ecd5732c09..8f8de37e0e 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs @@ -238,9 +238,14 @@ mod windows_impl { ) { anyhow::bail!("DangerFullAccess and ExternalSandbox are not supported for sandboxing") } + if !policy.has_full_disk_read_access() { + anyhow::bail!( + "Restricted read-only access is not yet supported by the Windows sandbox backend" + ); + } let caps = load_or_create_cap_sids(codex_home)?; let (psid_to_use, cap_sids) = match &policy { - SandboxPolicy::ReadOnly => ( + SandboxPolicy::ReadOnly { .. } => ( unsafe { convert_string_sid_to_sid(&caps.readonly).unwrap() }, vec![caps.readonly.clone()], ), @@ -469,6 +474,7 @@ mod windows_impl { fn workspace_policy(network_access: bool) -> SandboxPolicy { SandboxPolicy::WorkspaceWrite { writable_roots: Vec::new(), + read_only_access: Default::default(), network_access, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, @@ -487,7 +493,7 @@ mod windows_impl { #[test] fn applies_network_block_for_read_only() { - assert!(!SandboxPolicy::ReadOnly.has_full_network_access()); + assert!(!SandboxPolicy::new_read_only_policy().has_full_network_access()); } } } diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 4a97efaaf8..3716b65f59 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -258,10 +258,15 @@ mod windows_impl { ) { anyhow::bail!("DangerFullAccess and ExternalSandbox are not supported for sandboxing") } + if !policy.has_full_disk_read_access() { + anyhow::bail!( + "Restricted read-only access is not yet supported by the Windows sandbox backend" + ); + } let caps = load_or_create_cap_sids(codex_home)?; let (h_token, psid_generic, psid_workspace): (HANDLE, *mut c_void, Option<*mut c_void>) = unsafe { match &policy { - SandboxPolicy::ReadOnly => { + SandboxPolicy::ReadOnly { .. } => { let psid = convert_string_sid_to_sid(&caps.readonly).unwrap(); let (h, _) = super::token::create_readonly_token_with_cap(psid)?; (h, psid, None) @@ -510,6 +515,7 @@ mod windows_impl { fn workspace_policy(network_access: bool) -> SandboxPolicy { SandboxPolicy::WorkspaceWrite { writable_roots: Vec::new(), + read_only_access: Default::default(), network_access, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, @@ -528,7 +534,9 @@ mod windows_impl { #[test] fn applies_network_block_for_read_only() { - assert!(should_apply_network_block(&SandboxPolicy::ReadOnly)); + assert!(should_apply_network_block( + &SandboxPolicy::new_read_only_policy() + )); } } } diff --git a/codex-rs/windows-sandbox-rs/src/policy.rs b/codex-rs/windows-sandbox-rs/src/policy.rs index 64fc56052f..3cee37cdf7 100644 --- a/codex-rs/windows-sandbox-rs/src/policy.rs +++ b/codex-rs/windows-sandbox-rs/src/policy.rs @@ -3,7 +3,7 @@ pub use codex_protocol::protocol::SandboxPolicy; pub fn parse_policy(value: &str) -> Result { match value { - "read-only" => Ok(SandboxPolicy::ReadOnly), + "read-only" => Ok(SandboxPolicy::new_read_only_policy()), "workspace-write" => Ok(SandboxPolicy::new_workspace_write_policy()), "danger-full-access" | "external-sandbox" => anyhow::bail!( "DangerFullAccess and ExternalSandbox are not supported for sandboxing" @@ -52,6 +52,9 @@ mod tests { #[test] fn parses_read_only_policy() { - assert_eq!(parse_policy("read-only").unwrap(), SandboxPolicy::ReadOnly); + assert_eq!( + parse_policy("read-only").unwrap(), + SandboxPolicy::new_read_only_policy() + ); } }