diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 1f7934bc45..6ace22c766 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -849,6 +849,7 @@ dependencies = [ "codex-login", "codex-protocol", "codex-utils-json-to-toml", + "codex-windows-sandbox", "core_test_support", "mcp-types", "opentelemetry-appender-tracing", diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 23dcd29e35..088c2b4c3a 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -494,6 +494,9 @@ server_notification_definitions! { ReasoningSummaryPartAdded => "item/reasoning/summaryPartAdded" (v2::ReasoningSummaryPartAddedNotification), ReasoningTextDelta => "item/reasoning/textDelta" (v2::ReasoningTextDeltaNotification), + /// Notifies the user of world-writable directories on Windows, which cannot be protected by the sandbox. + WindowsWorldWritableWarning => "windows/worldWritableWarning" (v2::WindowsWorldWritableWarningNotification), + #[serde(rename = "account/login/completed")] #[ts(rename = "account/login/completed")] #[strum(serialize = "account/login/completed")] diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 67b072e669..fa1037dd14 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -934,6 +934,15 @@ pub struct McpToolCallProgressNotification { pub message: String, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct WindowsWorldWritableWarningNotification { + pub sample_paths: Vec, + pub extra_count: usize, + pub failed_scan: bool, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index 96f64afdf5..84eb68c66d 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -40,6 +40,7 @@ tracing = { workspace = true, features = ["log"] } tracing-subscriber = { workspace = true, features = ["env-filter", "fmt"] } opentelemetry-appender-tracing = { workspace = true } uuid = { workspace = true, features = ["serde", "v7"] } +codex-windows-sandbox.workspace = true [dev-dependencies] app_test_support = { workspace = true } diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 3cb0d5b569..1ccc9b1293 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -111,6 +111,7 @@ use codex_core::config_loader::load_config_as_toml; use codex_core::default_client::get_codex_user_agent; use codex_core::exec::ExecParams; use codex_core::exec_env::create_env; +use codex_core::features::Feature; use codex_core::find_conversation_path_by_id_str; use codex_core::get_platform_sandbox; use codex_core::git_info::git_diff_to_remote; @@ -1249,7 +1250,17 @@ impl CodexMessageProcessor { ..Default::default() }; - let config = match derive_config_from_params(overrides, cli_overrides).await { + // Persist windows sandbox feature. + // TODO: persist default config in general. + let mut cli_overrides = cli_overrides.unwrap_or_default(); + if cfg!(target_os = "windows") && self.config.features.enabled(Feature::WindowsSandbox) { + cli_overrides.insert( + "features.enable_experimental_windows_sandbox".to_string(), + serde_json::json!(true), + ); + } + + let config = match derive_config_from_params(overrides, Some(cli_overrides)).await { Ok(config) => config, Err(err) => { let error = JSONRPCErrorError { diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index a97b037be0..55f857351a 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -6,17 +6,19 @@ use crate::outgoing_message::OutgoingMessageSender; use codex_app_server_protocol::ClientInfo; use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::InitializeResponse; - use codex_app_server_protocol::JSONRPCError; use codex_app_server_protocol::JSONRPCErrorError; use codex_app_server_protocol::JSONRPCNotification; use codex_app_server_protocol::JSONRPCRequest; use codex_app_server_protocol::JSONRPCResponse; +use codex_app_server_protocol::ServerNotification; +use codex_app_server_protocol::WindowsWorldWritableWarningNotification; use codex_core::AuthManager; use codex_core::ConversationManager; use codex_core::config::Config; use codex_core::default_client::USER_AGENT_SUFFIX; use codex_core::default_client::get_codex_user_agent; +use codex_core::features::Feature; use codex_feedback::CodexFeedback; use codex_protocol::protocol::SessionSource; use std::sync::Arc; @@ -24,6 +26,7 @@ use std::sync::Arc; pub(crate) struct MessageProcessor { outgoing: Arc, codex_message_processor: CodexMessageProcessor, + config: Arc, initialized: bool, } @@ -51,13 +54,14 @@ impl MessageProcessor { conversation_manager, outgoing.clone(), codex_linux_sandbox_exe, - config, + config.clone(), feedback, ); Self { outgoing, codex_message_processor, + config, initialized: false, } } @@ -118,6 +122,8 @@ impl MessageProcessor { self.outgoing.send_response(request_id, response).await; self.initialized = true; + self.handle_windows_world_writable_warning().await; + return; } } @@ -156,4 +162,47 @@ impl MessageProcessor { pub(crate) fn process_error(&mut self, err: JSONRPCError) { tracing::error!("<- error: {:?}", err); } + + /// On Windows, when using the experimental sandbox, we need to warn the user about world-writable directories. + async fn handle_windows_world_writable_warning(&self) { + if !cfg!(windows) { + return; + } + + if !self.config.features.enabled(Feature::WindowsSandbox) { + return; + } + + if !matches!( + self.config.sandbox_policy, + codex_protocol::protocol::SandboxPolicy::WorkspaceWrite { .. } + | codex_protocol::protocol::SandboxPolicy::ReadOnly + ) { + return; + } + + if self + .config + .notices + .hide_world_writable_warning + .unwrap_or(false) + { + return; + } + + // This function is stubbed out to return None on non-Windows platforms + if let Some((sample_paths, extra_count, failed_scan)) = + codex_windows_sandbox::world_writable_warning_details(self.config.codex_home.as_path()) + { + self.outgoing + .send_server_notification(ServerNotification::WindowsWorldWritableWarning( + WindowsWorldWritableWarningNotification { + sample_paths, + extra_count, + failed_scan, + }, + )) + .await; + } + } } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index c2f2fbf5d2..e5b4e4c316 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1085,11 +1085,14 @@ impl Session { self.send_token_count_event(turn_context).await; } - pub(crate) async fn override_last_token_usage_estimate( - &self, - turn_context: &TurnContext, - estimated_total_tokens: i64, - ) { + pub(crate) async fn recompute_token_usage(&self, turn_context: &TurnContext) { + let Some(estimated_total_tokens) = self + .clone_history() + .await + .estimate_token_count(turn_context) + else { + return; + }; { let mut state = self.state.lock().await; let mut info = state.token_info().unwrap_or(TokenUsageInfo { @@ -1332,7 +1335,10 @@ impl Session { } async fn submission_loop(sess: Arc, config: Arc, rx_sub: Receiver) { - let mut previous_context: Option> = None; + // Seed with context in case there is an OverrideTurnContext first. + let mut previous_context: Option> = + Some(sess.new_turn(SessionSettingsUpdate::default()).await); + // To break out of this loop, send Op::Shutdown. while let Ok(sub) = rx_sub.recv().await { debug!(?sub, "Submission"); @@ -1982,12 +1988,14 @@ async fn run_turn( let mut base_instructions = turn_context.base_instructions.clone(); if parallel_tool_calls { static INSTRUCTIONS: &str = include_str!("../templates/parallel/instructions.md"); - static INSERTION_SPOT: &str = "## Editing constraints"; - base_instructions - .as_mut() - .map(|base| base.replace(INSERTION_SPOT, INSTRUCTIONS)); + if let Some(family) = + find_family_for_model(&sess.state.lock().await.session_configuration.model) + { + let mut new_instructions = base_instructions.unwrap_or(family.base_instructions); + new_instructions.push_str(INSTRUCTIONS); + base_instructions = Some(new_instructions); + } } - let prompt = Prompt { input, tools: router.specs(), diff --git a/codex-rs/core/src/compact.rs b/codex-rs/core/src/compact.rs index b5ece90892..8c38f93937 100644 --- a/codex-rs/core/src/compact.rs +++ b/codex-rs/core/src/compact.rs @@ -171,15 +171,7 @@ async fn run_compact_task_inner( .collect(); new_history.extend(ghost_snapshots); sess.replace_history(new_history).await; - - if let Some(estimated_tokens) = sess - .clone_history() - .await - .estimate_token_count(&turn_context) - { - sess.override_last_token_usage_estimate(&turn_context, estimated_tokens) - .await; - } + sess.recompute_token_usage(&turn_context).await; let rollout_item = RolloutItem::Compacted(CompactedItem { message: summary_text.clone(), diff --git a/codex-rs/core/src/compact_remote.rs b/codex-rs/core/src/compact_remote.rs index 51c35baf37..0d2e0f138d 100644 --- a/codex-rs/core/src/compact_remote.rs +++ b/codex-rs/core/src/compact_remote.rs @@ -66,15 +66,7 @@ async fn run_remote_compact_task_inner_impl( new_history.extend(ghost_snapshots); } sess.replace_history(new_history.clone()).await; - - if let Some(estimated_tokens) = sess - .clone_history() - .await - .estimate_token_count(turn_context.as_ref()) - { - sess.override_last_token_usage_estimate(turn_context.as_ref(), estimated_tokens) - .await; - } + sess.recompute_token_usage(turn_context).await; let compacted_item = CompactedItem { message: String::new(), diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index a3457e363c..1a7fa21b45 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -260,6 +260,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::Stable, default_enabled: true, }, + FeatureSpec { + id: Feature::ViewImageTool, + key: "view_image_tool", + stage: Stage::Stable, + default_enabled: true, + }, // Unstable features. FeatureSpec { id: Feature::UnifiedExec, @@ -285,12 +291,6 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::Beta, default_enabled: false, }, - FeatureSpec { - id: Feature::ViewImageTool, - key: "view_image_tool", - stage: Stage::Stable, - default_enabled: true, - }, FeatureSpec { id: Feature::WebSearchRequest, key: "web_search_request", diff --git a/codex-rs/core/templates/parallel/instructions.md b/codex-rs/core/templates/parallel/instructions.md index d690501af7..292d585e45 100644 --- a/codex-rs/core/templates/parallel/instructions.md +++ b/codex-rs/core/templates/parallel/instructions.md @@ -1,3 +1,4 @@ + ## Exploration and reading files - **Think first.** Before any tool call, decide ALL files/resources you will need. @@ -10,5 +11,3 @@ * Always maximize parallelism. Never read files one-by-one unless logically unavoidable. * This concern every read/list/search operations including, but not only, `cat`, `rg`, `sed`, `ls`, `git show`, `nl`, `wc`, ... * Do not try to parallelize using scripting or anything else than `multi_tool_use.parallel`. - -## Editing constraints \ No newline at end of file diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index e3bd642386..55e8f66416 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -3,6 +3,7 @@ use codex_core::features::Feature; use codex_core::model_family::find_family_for_model; use codex_core::protocol::AskForApproval; +use codex_core::protocol::ENVIRONMENT_CONTEXT_OPEN_TAG; use codex_core::protocol::EventMsg; use codex_core::protocol::Op; use codex_core::protocol::SandboxPolicy; @@ -372,6 +373,89 @@ async fn overrides_turn_context_but_keeps_cached_prefix_and_key_constant() -> an Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn override_before_first_turn_emits_environment_context() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let req = mount_sse_once(&server, sse_completed("resp-1")).await; + + let TestCodex { codex, .. } = test_codex().build(&server).await?; + + codex + .submit(Op::OverrideTurnContext { + cwd: None, + approval_policy: Some(AskForApproval::Never), + sandbox_policy: None, + model: None, + effort: None, + summary: None, + }) + .await?; + + codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "first message".into(), + }], + }) + .await?; + + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; + + let body = req.single_request().body_json(); + let input = body["input"] + .as_array() + .expect("input array must be present"); + assert!( + !input.is_empty(), + "expected at least environment context and user message" + ); + + let env_msg = &input[1]; + let env_text = env_msg["content"][0]["text"] + .as_str() + .expect("environment context text"); + assert!( + env_text.starts_with(ENVIRONMENT_CONTEXT_OPEN_TAG), + "second entry should be environment context, got: {env_text}" + ); + assert!( + env_text.contains("never"), + "environment context should reflect overridden approval policy: {env_text}" + ); + + let env_count = input + .iter() + .filter(|msg| { + msg["content"] + .as_array() + .and_then(|content| { + content.iter().find(|item| { + item["type"].as_str() == Some("input_text") + && item["text"] + .as_str() + .map(|text| text.starts_with(ENVIRONMENT_CONTEXT_OPEN_TAG)) + .unwrap_or(false) + }) + }) + .is_some() + }) + .count(); + assert_eq!( + env_count, 2, + "environment context should appear exactly twice, found {env_count}" + ); + + let user_msg = &input[2]; + let user_text = user_msg["content"][0]["text"] + .as_str() + .expect("user message text"); + assert_eq!(user_text, "first message"); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn per_turn_overrides_keep_cached_prefix_and_key_constant() -> anyhow::Result<()> { skip_if_no_network!(Ok(()));