diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 46d155be74..2dec69ac49 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -468,7 +468,6 @@ impl Session { tools_config: ToolsConfig::new(&ToolsConfigParams { model_family: &config.model_family, approval_policy, - sandbox_policy: sandbox_policy.clone(), include_plan_tool: config.include_plan_tool, include_apply_patch_tool: config.include_apply_patch_tool, include_web_search_request: config.tools_web_search_request, @@ -1148,7 +1147,6 @@ async fn submission_loop( let tools_config = ToolsConfig::new(&ToolsConfigParams { model_family: &effective_family, approval_policy: new_approval_policy, - sandbox_policy: new_sandbox_policy.clone(), include_plan_tool: config.include_plan_tool, include_apply_patch_tool: config.include_apply_patch_tool, include_web_search_request: config.tools_web_search_request, @@ -1186,26 +1184,18 @@ async fn submission_loop( { warn!("failed to persist overrides: {e:#}"); } - - if cwd.is_some() || approval_policy.is_some() || sandbox_policy.is_some() { - sess.record_conversation_items(&[ResponseItem::from(EnvironmentContext::new( - cwd, - approval_policy, - sandbox_policy, - // Shell is not configurable from turn to turn - None, - ))]) - .await; - } } Op::UserInput { items } => { - // attempt to inject input into current task - if let Err(items) = sess.inject_input(items) { - // no current task, spawn a new one - let task = - AgentTask::spawn(sess.clone(), Arc::clone(&turn_context), sub.id, items); - sess.set_task(task); - } + submit_user_input( + turn_context.cwd.clone(), + turn_context.approval_policy, + turn_context.sandbox_policy.clone(), + &sess, + &turn_context, + sub.id.clone(), + items, + ) + .await; } Op::UserTurn { items, @@ -1250,7 +1240,6 @@ async fn submission_loop( tools_config: ToolsConfig::new(&ToolsConfigParams { model_family: &model_family, approval_policy, - sandbox_policy: sandbox_policy.clone(), include_plan_tool: config.include_plan_tool, include_apply_patch_tool: config.include_apply_patch_tool, include_web_search_request: config.tools_web_search_request, @@ -1267,11 +1256,16 @@ async fn submission_loop( shell_environment_policy: turn_context.shell_environment_policy.clone(), cwd, }; - // TODO: record the new environment context in the conversation history - // no current task, spawn a new one with the per‑turn context - let task = - AgentTask::spawn(sess.clone(), Arc::new(fresh_turn_context), sub.id, items); - sess.set_task(task); + submit_user_input( + fresh_turn_context.cwd.clone(), + fresh_turn_context.approval_policy, + fresh_turn_context.sandbox_policy.clone(), + &sess, + &Arc::new(fresh_turn_context), + sub.id.clone(), + items, + ) + .await; } } Op::ExecApproval { id, decision } => match decision { @@ -2833,6 +2827,30 @@ async fn handle_sandbox_error( } } +async fn submit_user_input( + cwd: PathBuf, + approval_policy: AskForApproval, + sandbox_policy: SandboxPolicy, + sess: &Arc, + turn_context: &Arc, + sub_id: String, + items: Vec, +) { + sess.record_conversation_items(&[ResponseItem::from(EnvironmentContext::new( + Some(cwd), + Some(approval_policy), + Some(sandbox_policy), + // Shell is not configurable from turn to turn + None, + ))]) + .await; + if let Err(items) = sess.inject_input(items) { + // no current task, spawn a new one + let task = AgentTask::spawn(Arc::clone(sess), Arc::clone(turn_context), sub_id, items); + sess.set_task(task); + } +} + fn format_exec_output_str(exec_output: &ExecToolCallOutput) -> String { let ExecToolCallOutput { aggregated_output, .. diff --git a/codex-rs/core/src/openai_tools.rs b/codex-rs/core/src/openai_tools.rs index ec56fdfd2c..6dd55fe399 100644 --- a/codex-rs/core/src/openai_tools.rs +++ b/codex-rs/core/src/openai_tools.rs @@ -8,7 +8,6 @@ use std::collections::HashMap; use crate::model_family::ModelFamily; use crate::plan_tool::PLAN_TOOL; use crate::protocol::AskForApproval; -use crate::protocol::SandboxPolicy; use crate::tool_apply_patch::ApplyPatchToolType; use crate::tool_apply_patch::create_apply_patch_freeform_tool; use crate::tool_apply_patch::create_apply_patch_json_tool; @@ -58,7 +57,7 @@ pub(crate) enum OpenAiTool { #[derive(Debug, Clone)] pub enum ConfigShellToolType { DefaultShell, - ShellWithRequest { sandbox_policy: SandboxPolicy }, + ShellWithRequest, LocalShell, StreamableShell, } @@ -76,7 +75,6 @@ pub(crate) struct ToolsConfig { pub(crate) struct ToolsConfigParams<'a> { pub(crate) model_family: &'a ModelFamily, pub(crate) approval_policy: AskForApproval, - pub(crate) sandbox_policy: SandboxPolicy, pub(crate) include_plan_tool: bool, pub(crate) include_apply_patch_tool: bool, pub(crate) include_web_search_request: bool, @@ -90,7 +88,6 @@ impl ToolsConfig { let ToolsConfigParams { model_family, approval_policy, - sandbox_policy, include_plan_tool, include_apply_patch_tool, include_web_search_request, @@ -106,9 +103,7 @@ impl ToolsConfig { ConfigShellToolType::DefaultShell }; if matches!(approval_policy, AskForApproval::OnRequest) && !use_streamable_shell_tool { - shell_type = ConfigShellToolType::ShellWithRequest { - sandbox_policy: sandbox_policy.clone(), - } + shell_type = ConfigShellToolType::ShellWithRequest; } let apply_patch_tool_type = match model_family.apply_patch_tool_type { @@ -251,7 +246,9 @@ fn create_unified_exec_tool() -> OpenAiTool { }) } -fn create_shell_tool_for_sandbox(sandbox_policy: &SandboxPolicy) -> OpenAiTool { +const SHELL_TOOL_DESCRIPTION: &str = r#"Runs a shell command and returns its output"#; + +fn create_shell_tool_for_sandbox() -> OpenAiTool { let mut properties = BTreeMap::new(); properties.insert( "command".to_string(), @@ -263,79 +260,29 @@ fn create_shell_tool_for_sandbox(sandbox_policy: &SandboxPolicy) -> OpenAiTool { properties.insert( "workdir".to_string(), JsonSchema::String { - description: Some("The working directory to execute the command in".to_string()), + description: Some("Working directory to execute the command in.".to_string()), }, ); properties.insert( "timeout_ms".to_string(), JsonSchema::Number { - description: Some("The timeout for the command in milliseconds".to_string()), + description: Some("Timeout for the command in milliseconds.".to_string()), }, ); - - if matches!(sandbox_policy, SandboxPolicy::WorkspaceWrite { .. }) { - properties.insert( + properties.insert( "with_escalated_permissions".to_string(), JsonSchema::Boolean { - description: Some("Whether to request escalated permissions. Set to true if command needs to be run without sandbox restrictions".to_string()), + description: Some("Request escalated permissions, only for when a command would otherwise be blocked by the sandbox.".to_string()), }, ); - properties.insert( + properties.insert( "justification".to_string(), JsonSchema::String { - description: Some("Only set if with_escalated_permissions is true. 1-sentence explanation of why we want to run this command.".to_string()), + description: Some("Required if and only if with_escalated_permissions == true. One sentence explaining why escalation is needed (e.g., write outside CWD, network fetch, git commit).".to_string()), }, ); - } - let description = match sandbox_policy { - SandboxPolicy::WorkspaceWrite { - network_access, - .. - } => { - let network_line = if !network_access { - "\n - Commands that require network access" - } else { - "" - }; - - format!( - r#" -The shell tool is used to execute shell commands. -- When invoking the shell tool, your call will be running in a sandbox, and some shell commands will require escalated privileges: - - Types of actions that require escalated privileges: - - Writing files other than those in the writable roots (see the environment context for the allowed directories){network_line} - - Examples of commands that require escalated privileges: - - git commit - - npm install or pnpm install - - cargo build - - cargo test -- When invoking a command that will require escalated privileges: - - Provide the with_escalated_permissions parameter with the boolean value true - - Include a short, 1 sentence explanation for why we need to run with_escalated_permissions in the justification parameter."#, - ) - } - SandboxPolicy::DangerFullAccess => { - "Runs a shell command and returns its output.".to_string() - } - SandboxPolicy::ReadOnly => { - r#" -The shell tool is used to execute shell commands. -- When invoking the shell tool, your call will be running in a sandbox, and some shell commands (including apply_patch) will require escalated permissions: - - Types of actions that require escalated privileges: - - Writing files - - Applying patches - - Examples of commands that require escalated privileges: - - apply_patch - - git commit - - npm install or pnpm install - - cargo build - - cargo test -- When invoking a command that will require escalated privileges: - - Provide the with_escalated_permissions parameter with the boolean value true - - Include a short, 1 sentence explanation for why we need to run with_escalated_permissions in the justification parameter"#.to_string() - } - }; + let description = SHELL_TOOL_DESCRIPTION.to_string(); OpenAiTool::Function(ResponsesApiTool { name: "shell".to_string(), @@ -348,7 +295,6 @@ The shell tool is used to execute shell commands. }, }) } - fn create_view_image_tool() -> OpenAiTool { // Support only local filesystem path. let mut properties = BTreeMap::new(); @@ -589,8 +535,8 @@ pub(crate) fn get_openai_tools( ConfigShellToolType::DefaultShell => { tools.push(create_shell_tool()); } - ConfigShellToolType::ShellWithRequest { sandbox_policy } => { - tools.push(create_shell_tool_for_sandbox(sandbox_policy)); + ConfigShellToolType::ShellWithRequest => { + tools.push(create_shell_tool_for_sandbox()); } ConfigShellToolType::LocalShell => { tools.push(OpenAiTool::LocalShell {}); @@ -686,7 +632,6 @@ mod tests { let config = ToolsConfig::new(&ToolsConfigParams { model_family: &model_family, approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, include_plan_tool: true, include_apply_patch_tool: false, include_web_search_request: true, @@ -708,7 +653,6 @@ mod tests { let config = ToolsConfig::new(&ToolsConfigParams { model_family: &model_family, approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, include_plan_tool: true, include_apply_patch_tool: false, include_web_search_request: true, @@ -730,7 +674,6 @@ mod tests { let config = ToolsConfig::new(&ToolsConfigParams { model_family: &model_family, approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, include_plan_tool: false, include_apply_patch_tool: false, include_web_search_request: true, @@ -836,7 +779,6 @@ mod tests { let config = ToolsConfig::new(&ToolsConfigParams { model_family: &model_family, approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, include_plan_tool: false, include_apply_patch_tool: false, include_web_search_request: false, @@ -914,7 +856,6 @@ mod tests { let config = ToolsConfig::new(&ToolsConfigParams { model_family: &model_family, approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, include_plan_tool: false, include_apply_patch_tool: false, include_web_search_request: true, @@ -977,7 +918,6 @@ mod tests { let config = ToolsConfig::new(&ToolsConfigParams { model_family: &model_family, approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, include_plan_tool: false, include_apply_patch_tool: false, include_web_search_request: true, @@ -1035,7 +975,6 @@ mod tests { let config = ToolsConfig::new(&ToolsConfigParams { model_family: &model_family, approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, include_plan_tool: false, include_apply_patch_tool: false, include_web_search_request: true, @@ -1096,7 +1035,6 @@ mod tests { let config = ToolsConfig::new(&ToolsConfigParams { model_family: &model_family, approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, include_plan_tool: false, include_apply_patch_tool: false, include_web_search_request: true, @@ -1150,13 +1088,7 @@ mod tests { #[test] fn test_shell_tool_for_sandbox_workspace_write() { - let sandbox_policy = SandboxPolicy::WorkspaceWrite { - writable_roots: vec!["workspace".into()], - network_access: false, - exclude_tmpdir_env_var: false, - exclude_slash_tmp: false, - }; - let tool = super::create_shell_tool_for_sandbox(&sandbox_policy); + let tool = super::create_shell_tool_for_sandbox(); let OpenAiTool::Function(ResponsesApiTool { description, name, .. }) = &tool @@ -1165,26 +1097,13 @@ mod tests { }; assert_eq!(name, "shell"); - let expected = r#" -The shell tool is used to execute shell commands. -- When invoking the shell tool, your call will be running in a sandbox, and some shell commands will require escalated privileges: - - Types of actions that require escalated privileges: - - Writing files other than those in the writable roots (see the environment context for the allowed directories) - - Commands that require network access - - Examples of commands that require escalated privileges: - - git commit - - npm install or pnpm install - - cargo build - - cargo test -- When invoking a command that will require escalated privileges: - - Provide the with_escalated_permissions parameter with the boolean value true - - Include a short, 1 sentence explanation for why we need to run with_escalated_permissions in the justification parameter."#; + let expected = super::SHELL_TOOL_DESCRIPTION; assert_eq!(description, expected); } #[test] fn test_shell_tool_for_sandbox_readonly() { - let tool = super::create_shell_tool_for_sandbox(&SandboxPolicy::ReadOnly); + let tool = super::create_shell_tool_for_sandbox(); let OpenAiTool::Function(ResponsesApiTool { description, name, .. }) = &tool @@ -1193,27 +1112,13 @@ The shell tool is used to execute shell commands. }; assert_eq!(name, "shell"); - let expected = r#" -The shell tool is used to execute shell commands. -- When invoking the shell tool, your call will be running in a sandbox, and some shell commands (including apply_patch) will require escalated permissions: - - Types of actions that require escalated privileges: - - Writing files - - Applying patches - - Examples of commands that require escalated privileges: - - apply_patch - - git commit - - npm install or pnpm install - - cargo build - - cargo test -- When invoking a command that will require escalated privileges: - - Provide the with_escalated_permissions parameter with the boolean value true - - Include a short, 1 sentence explanation for why we need to run with_escalated_permissions in the justification parameter"#; + let expected = super::SHELL_TOOL_DESCRIPTION; assert_eq!(description, expected); } #[test] fn test_shell_tool_for_sandbox_danger_full_access() { - let tool = super::create_shell_tool_for_sandbox(&SandboxPolicy::DangerFullAccess); + let tool = super::create_shell_tool_for_sandbox(); let OpenAiTool::Function(ResponsesApiTool { description, name, .. }) = &tool @@ -1222,6 +1127,7 @@ The shell tool is used to execute shell commands. }; assert_eq!(name, "shell"); - assert_eq!(description, "Runs a shell command and returns its output."); + let expected = super::SHELL_TOOL_DESCRIPTION; + assert_eq!(description, expected); } } diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 992679df7f..69a53b560f 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -276,7 +276,25 @@ async fn resume_includes_initial_messages_and_sends_prior_items() { "content": [{ "type": "input_text", "text": "hello" }] } ]); - assert_eq!(request_body["input"], expected_input); + let input_array = request_body + .get("input") + .and_then(|v| v.as_array()) + .cloned() + .expect("input array in request body"); + let filtered: Vec = input_array + .into_iter() + .filter(|item| { + let text = item + .get("content") + .and_then(|c| c.as_array()) + .and_then(|a| a.first()) + .and_then(|o| o.get("text")) + .and_then(|t| t.as_str()) + .unwrap_or(""); + !text.contains("") + }) + .collect(); + assert_eq!(serde_json::json!(filtered), expected_input); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -874,34 +892,6 @@ async fn history_dedupes_streamed_and_final_messages_across_turns() { assert_eq!(requests.len(), 3, "expected 3 requests (one per turn)"); // Replace full-array compare with tail-only raw JSON compare using a single hard-coded value. - let r3_tail_expected = json!([ - { - "type": "message", - "role": "user", - "content": [{"type":"input_text","text":"U1"}] - }, - { - "type": "message", - "role": "assistant", - "content": [{"type":"output_text","text":"Hey there!\n"}] - }, - { - "type": "message", - "role": "user", - "content": [{"type":"input_text","text":"U2"}] - }, - { - "type": "message", - "role": "assistant", - "content": [{"type":"output_text","text":"Hey there!\n"}] - }, - { - "type": "message", - "role": "user", - "content": [{"type":"input_text","text":"U3"}] - } - ]); - let r3_input_array = requests[2] .body_json::() .unwrap() @@ -909,12 +899,60 @@ async fn history_dedupes_streamed_and_final_messages_across_turns() { .and_then(|v| v.as_array()) .cloned() .expect("r3 missing input array"); - // skipping earlier context and developer messages - let tail_len = r3_tail_expected.as_array().unwrap().len(); - let actual_tail = &r3_input_array[r3_input_array.len() - tail_len..]; + // We only assert on the last 5 items of the input history for request 3. + // With per-turn environment context injected, the last 5 should be: + // [env_ctx, U2, assistant("Hey there!\n"), env_ctx, U3] + let actual_tail = &r3_input_array[r3_input_array.len() - 5..]; + + // env_ctx 1 + assert_eq!(actual_tail[0]["type"], serde_json::json!("message")); + assert_eq!(actual_tail[0]["role"], serde_json::json!("user")); + let env_text_1 = &actual_tail[0]["content"][0]["text"]; + assert!( + env_text_1 + .as_str() + .expect("env text should be string") + .contains("") + ); + + // U2 assert_eq!( - serde_json::Value::Array(actual_tail.to_vec()), - r3_tail_expected, - "request 3 tail mismatch", + actual_tail[1], + serde_json::json!({ + "type": "message", + "role": "user", + "content": [ { "type": "input_text", "text": "U2" } ] + }) + ); + + // assistant response + assert_eq!( + actual_tail[2], + serde_json::json!({ + "type": "message", + "role": "assistant", + "content": [ { "type": "output_text", "text": "Hey there!\n" } ] + }) + ); + + // env_ctx 2 + assert_eq!(actual_tail[3]["type"], serde_json::json!("message")); + assert_eq!(actual_tail[3]["role"], serde_json::json!("user")); + let env_text_2 = &actual_tail[3]["content"][0]["text"]; + assert!( + env_text_2 + .as_str() + .expect("env text should be string") + .contains("") + ); + + // U3 + assert_eq!( + actual_tail[4], + serde_json::json!({ + "type": "message", + "role": "user", + "content": [ { "type": "input_text", "text": "U3" } ] + }) ); } diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index 5ac3da6003..ff0296a97b 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -271,7 +271,7 @@ async fn prefixes_context_and_instructions_once_and_consistently_across_requests let shell = default_user_shell().await; - let expected_env_text = format!( + let expected_env_text_init = format!( r#" {} on-request @@ -284,13 +284,28 @@ async fn prefixes_context_and_instructions_once_and_consistently_across_requests None => String::new(), } ); + // Per-turn environment context omits the shell tag. + let expected_env_text_turn = format!( + r#" + {} + on-request + read-only + restricted +"#, + cwd.path().to_string_lossy(), + ); let expected_ui_text = "\n\nbe consistent and helpful\n\n"; - let expected_env_msg = serde_json::json!({ + let expected_env_msg_init = serde_json::json!({ "type": "message", "role": "user", - "content": [ { "type": "input_text", "text": expected_env_text } ] + "content": [ { "type": "input_text", "text": expected_env_text_init } ] + }); + let expected_env_msg_turn = serde_json::json!({ + "type": "message", + "role": "user", + "content": [ { "type": "input_text", "text": expected_env_text_turn } ] }); let expected_ui_msg = serde_json::json!({ "type": "message", @@ -306,7 +321,12 @@ async fn prefixes_context_and_instructions_once_and_consistently_across_requests let body1 = requests[0].body_json::().unwrap(); assert_eq!( body1["input"], - serde_json::json!([expected_ui_msg, expected_env_msg, expected_user_message_1]) + serde_json::json!([ + expected_ui_msg, + expected_env_msg_init, + expected_env_msg_turn, + expected_user_message_1 + ]) ); let expected_user_message_2 = serde_json::json!({ @@ -318,7 +338,7 @@ async fn prefixes_context_and_instructions_once_and_consistently_across_requests let expected_body2 = serde_json::json!( [ body1["input"].as_array().unwrap().as_slice(), - [expected_user_message_2].as_slice(), + [expected_env_msg_turn, expected_user_message_2].as_slice(), ] .concat() ); @@ -546,10 +566,24 @@ async fn per_turn_overrides_keep_cached_prefix_and_key_constant() { "role": "user", "content": [ { "type": "input_text", "text": "hello 2" } ] }); + let expected_env_text_2 = format!( + r#" + {} + never + workspace-write + enabled +"#, + new_cwd.path().to_string_lossy() + ); + let expected_env_msg_2 = serde_json::json!({ + "type": "message", + "role": "user", + "content": [ { "type": "input_text", "text": expected_env_text_2 } ] + }); let expected_body2 = serde_json::json!( [ body1["input"].as_array().unwrap().as_slice(), - [expected_user_message_2].as_slice(), + [expected_env_msg_2, expected_user_message_2].as_slice(), ] .concat() );