From 4288091f63cdffc21cfeaea4290c734056188808 Mon Sep 17 00:00:00 2001 From: zhao-oai Date: Tue, 18 Nov 2025 17:40:22 -0800 Subject: [PATCH 1/6] update credit status details (#6862) --- .../src/models/credit_status_details.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/codex-rs/codex-backend-openapi-models/src/models/credit_status_details.rs b/codex-rs/codex-backend-openapi-models/src/models/credit_status_details.rs index d444a7a8d2..b62b88d715 100644 --- a/codex-rs/codex-backend-openapi-models/src/models/credit_status_details.rs +++ b/codex-rs/codex-backend-openapi-models/src/models/credit_status_details.rs @@ -12,6 +12,8 @@ use serde::Serialize; #[derive(Clone, Default, Debug, PartialEq, Serialize, Deserialize)] pub struct CreditStatusDetails { + #[serde(rename = "has_credits")] + pub has_credits: bool, #[serde(rename = "unlimited")] pub unlimited: bool, #[serde( @@ -38,8 +40,9 @@ pub struct CreditStatusDetails { } impl CreditStatusDetails { - pub fn new(unlimited: bool) -> CreditStatusDetails { + pub fn new(has_credits: bool, unlimited: bool) -> CreditStatusDetails { CreditStatusDetails { + has_credits, unlimited, balance: None, approx_local_messages: None, From b395dc1be6d00cfb855279f39d6df215611e411b Mon Sep 17 00:00:00 2001 From: Celia Chen Date: Tue, 18 Nov 2025 17:55:24 -0800 Subject: [PATCH 2/6] [app-server] introduce `turn/completed` v2 event (#6800) similar to logic in `codex/codex-rs/exec/src/event_processor_with_jsonl_output.rs`. translation of v1 -> v2 events: `codex/event/task_complete` -> `turn/completed` `codex/event/turn_aborted` -> `turn/completed` with `interrupted` status `codex/event/error` -> `turn/completed` with `error` status this PR also makes `items` field in `Turn` optional. For now, we only populate it when we resume a thread, and leave it as None for all other places until we properly rewrite core to keep track of items. tested using the codex app server client. example new event: ``` < { < "method": "turn/completed", < "params": { < "turn": { < "id": "0", < "items": [], < "status": "interrupted" < } < } < } ``` --- .../app-server-protocol/src/protocol/v2.rs | 12 +- codex-rs/app-server-test-client/src/main.rs | 4 +- .../app-server/src/bespoke_event_handling.rs | 306 ++++++++++++++++++ .../app-server/src/codex_message_processor.rs | 22 +- .../tests/suite/v2/turn_interrupt.rs | 16 +- .../app-server/tests/suite/v2/turn_start.rs | 24 +- 6 files changed, 362 insertions(+), 22 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index f4a6d636aa..05e0e3560e 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -519,9 +519,11 @@ pub struct AccountUpdatedNotification { #[ts(export_to = "v2/")] pub struct Turn { pub id: String, + /// This is currently only populated for resumed threads. + /// TODO: properly populate items for all turns. pub items: Vec, + #[serde(flatten)] pub status: TurnStatus, - pub error: Option, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] @@ -532,12 +534,12 @@ pub struct TurnError { } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] -#[serde(rename_all = "camelCase")] -#[ts(export_to = "v2/")] +#[serde(tag = "status", rename_all = "camelCase")] +#[ts(tag = "status", export_to = "v2/")] pub enum TurnStatus { Completed, Interrupted, - Failed, + Failed { error: TurnError }, InProgress, } @@ -853,8 +855,6 @@ pub struct Usage { #[ts(export_to = "v2/")] pub struct TurnCompletedNotification { pub turn: Turn, - // TODO: should usage be stored on the Turn object, and we return that instead? - pub usage: Usage, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] diff --git a/codex-rs/app-server-test-client/src/main.rs b/codex-rs/app-server-test-client/src/main.rs index 8513ec0321..6130b2d511 100644 --- a/codex-rs/app-server-test-client/src/main.rs +++ b/codex-rs/app-server-test-client/src/main.rs @@ -46,6 +46,7 @@ use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::TurnStartResponse; +use codex_app_server_protocol::TurnStatus; use codex_app_server_protocol::UserInput as V2UserInput; use codex_protocol::ConversationId; use codex_protocol::protocol::Event; @@ -509,10 +510,9 @@ impl CodexClient { ServerNotification::TurnCompleted(payload) => { if payload.turn.id == turn_id { println!("\n< turn/completed notification: {:?}", payload.turn.status); - if let Some(error) = payload.turn.error { + if let TurnStatus::Failed { error } = &payload.turn.status { println!("[turn error] {}", error.message); } - println!("< usage: {:?}", payload.usage); break; } } diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index ce4f7590e2..b4fdbe3e03 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -1,5 +1,7 @@ use crate::codex_message_processor::ApiVersion; use crate::codex_message_processor::PendingInterrupts; +use crate::codex_message_processor::TurnSummary; +use crate::codex_message_processor::TurnSummaryStore; use crate::outgoing_message::OutgoingMessageSender; use codex_app_server_protocol::AccountRateLimitsUpdatedNotification; use codex_app_server_protocol::AgentMessageDeltaNotification; @@ -26,7 +28,11 @@ use codex_app_server_protocol::SandboxCommandAssessment as V2SandboxCommandAsses use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ServerRequestPayload; use codex_app_server_protocol::ThreadItem; +use codex_app_server_protocol::Turn; +use codex_app_server_protocol::TurnCompletedNotification; +use codex_app_server_protocol::TurnError; use codex_app_server_protocol::TurnInterruptResponse; +use codex_app_server_protocol::TurnStatus; use codex_core::CodexConversation; use codex_core::parse_command::shlex_join; use codex_core::protocol::ApplyPatchApprovalRequestEvent; @@ -54,10 +60,14 @@ pub(crate) async fn apply_bespoke_event_handling( conversation: Arc, outgoing: Arc, pending_interrupts: PendingInterrupts, + turn_summary_store: TurnSummaryStore, api_version: ApiVersion, ) { let Event { id: event_id, msg } = event; match msg { + EventMsg::TaskComplete(_ev) => { + handle_turn_complete(conversation_id, event_id, &outgoing, &turn_summary_store).await; + } EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent { call_id, changes, @@ -191,6 +201,9 @@ pub(crate) async fn apply_bespoke_event_handling( .await; } } + EventMsg::Error(ev) => { + handle_error(conversation_id, ev.message, &turn_summary_store).await; + } EventMsg::EnteredReviewMode(review_request) => { let notification = ItemStartedNotification { item: ThreadItem::CodeReview { @@ -326,12 +339,79 @@ pub(crate) async fn apply_bespoke_event_handling( } } } + + handle_turn_interrupted(conversation_id, event_id, &outgoing, &turn_summary_store) + .await; } _ => {} } } +async fn emit_turn_completed_with_status( + event_id: String, + status: TurnStatus, + outgoing: &OutgoingMessageSender, +) { + let notification = TurnCompletedNotification { + turn: Turn { + id: event_id, + items: vec![], + status, + }, + }; + outgoing + .send_server_notification(ServerNotification::TurnCompleted(notification)) + .await; +} + +async fn find_and_remove_turn_summary( + conversation_id: ConversationId, + turn_summary_store: &TurnSummaryStore, +) -> TurnSummary { + let mut map = turn_summary_store.lock().await; + map.remove(&conversation_id).unwrap_or_default() +} + +async fn handle_turn_complete( + conversation_id: ConversationId, + event_id: String, + outgoing: &OutgoingMessageSender, + turn_summary_store: &TurnSummaryStore, +) { + let turn_summary = find_and_remove_turn_summary(conversation_id, turn_summary_store).await; + + let status = if let Some(message) = turn_summary.last_error_message { + TurnStatus::Failed { + error: TurnError { message }, + } + } else { + TurnStatus::Completed + }; + + emit_turn_completed_with_status(event_id, status, outgoing).await; +} + +async fn handle_turn_interrupted( + conversation_id: ConversationId, + event_id: String, + outgoing: &OutgoingMessageSender, + turn_summary_store: &TurnSummaryStore, +) { + find_and_remove_turn_summary(conversation_id, turn_summary_store).await; + + emit_turn_completed_with_status(event_id, TurnStatus::Interrupted, outgoing).await; +} + +async fn handle_error( + conversation_id: ConversationId, + message: String, + turn_summary_store: &TurnSummaryStore, +) { + let mut map = turn_summary_store.lock().await; + map.entry(conversation_id).or_default().last_error_message = Some(message); +} + async fn on_patch_approval_response( event_id: String, receiver: oneshot::Receiver, @@ -536,13 +616,140 @@ async fn construct_mcp_tool_call_end_notification( #[cfg(test)] mod tests { use super::*; + use crate::CHANNEL_CAPACITY; + use crate::outgoing_message::OutgoingMessage; + use crate::outgoing_message::OutgoingMessageSender; + use anyhow::Result; + use anyhow::anyhow; + use anyhow::bail; use codex_core::protocol::McpInvocation; use mcp_types::CallToolResult; use mcp_types::ContentBlock; use mcp_types::TextContent; use pretty_assertions::assert_eq; use serde_json::Value as JsonValue; + use std::collections::HashMap; use std::time::Duration; + use tokio::sync::Mutex; + use tokio::sync::mpsc; + + fn new_turn_summary_store() -> TurnSummaryStore { + Arc::new(Mutex::new(HashMap::new())) + } + + #[tokio::test] + async fn test_handle_error_records_message() -> Result<()> { + let conversation_id = ConversationId::new(); + let turn_summary_store = new_turn_summary_store(); + + handle_error(conversation_id, "boom".to_string(), &turn_summary_store).await; + + let turn_summary = find_and_remove_turn_summary(conversation_id, &turn_summary_store).await; + assert_eq!(turn_summary.last_error_message, Some("boom".to_string())); + Ok(()) + } + + #[tokio::test] + async fn test_handle_turn_complete_emits_completed_without_error() -> Result<()> { + let conversation_id = ConversationId::new(); + let event_id = "complete1".to_string(); + let (tx, mut rx) = mpsc::channel(CHANNEL_CAPACITY); + let outgoing = Arc::new(OutgoingMessageSender::new(tx)); + let turn_summary_store = new_turn_summary_store(); + + handle_turn_complete( + conversation_id, + event_id.clone(), + &outgoing, + &turn_summary_store, + ) + .await; + + let msg = rx + .recv() + .await + .ok_or_else(|| anyhow!("should send one notification"))?; + match msg { + OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => { + assert_eq!(n.turn.id, event_id); + assert_eq!(n.turn.status, TurnStatus::Completed); + } + other => bail!("unexpected message: {other:?}"), + } + assert!(rx.try_recv().is_err(), "no extra messages expected"); + Ok(()) + } + + #[tokio::test] + async fn test_handle_turn_interrupted_emits_interrupted_with_error() -> Result<()> { + let conversation_id = ConversationId::new(); + let event_id = "interrupt1".to_string(); + let turn_summary_store = new_turn_summary_store(); + handle_error(conversation_id, "oops".to_string(), &turn_summary_store).await; + let (tx, mut rx) = mpsc::channel(CHANNEL_CAPACITY); + let outgoing = Arc::new(OutgoingMessageSender::new(tx)); + + handle_turn_interrupted( + conversation_id, + event_id.clone(), + &outgoing, + &turn_summary_store, + ) + .await; + + let msg = rx + .recv() + .await + .ok_or_else(|| anyhow!("should send one notification"))?; + match msg { + OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => { + assert_eq!(n.turn.id, event_id); + assert_eq!(n.turn.status, TurnStatus::Interrupted); + } + other => bail!("unexpected message: {other:?}"), + } + assert!(rx.try_recv().is_err(), "no extra messages expected"); + Ok(()) + } + + #[tokio::test] + async fn test_handle_turn_complete_emits_failed_with_error() -> Result<()> { + let conversation_id = ConversationId::new(); + let event_id = "complete_err1".to_string(); + let turn_summary_store = new_turn_summary_store(); + handle_error(conversation_id, "bad".to_string(), &turn_summary_store).await; + let (tx, mut rx) = mpsc::channel(CHANNEL_CAPACITY); + let outgoing = Arc::new(OutgoingMessageSender::new(tx)); + + handle_turn_complete( + conversation_id, + event_id.clone(), + &outgoing, + &turn_summary_store, + ) + .await; + + let msg = rx + .recv() + .await + .ok_or_else(|| anyhow!("should send one notification"))?; + match msg { + OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => { + assert_eq!(n.turn.id, event_id); + assert_eq!( + n.turn.status, + TurnStatus::Failed { + error: TurnError { + message: "bad".to_string(), + } + } + ); + } + other => bail!("unexpected message: {other:?}"), + } + assert!(rx.try_recv().is_err(), "no extra messages expected"); + Ok(()) + } #[tokio::test] async fn test_construct_mcp_tool_call_begin_notification_with_args() { @@ -572,6 +779,105 @@ mod tests { assert_eq!(notification, expected); } + #[tokio::test] + async fn test_handle_turn_complete_emits_error_multiple_turns() -> Result<()> { + // Conversation A will have two turns; Conversation B will have one turn. + let conversation_a = ConversationId::new(); + let conversation_b = ConversationId::new(); + let turn_summary_store = new_turn_summary_store(); + + let (tx, mut rx) = mpsc::channel(CHANNEL_CAPACITY); + let outgoing = Arc::new(OutgoingMessageSender::new(tx)); + + // Turn 1 on conversation A + let a_turn1 = "a_turn1".to_string(); + handle_error(conversation_a, "a1".to_string(), &turn_summary_store).await; + handle_turn_complete( + conversation_a, + a_turn1.clone(), + &outgoing, + &turn_summary_store, + ) + .await; + + // Turn 1 on conversation B + let b_turn1 = "b_turn1".to_string(); + handle_error(conversation_b, "b1".to_string(), &turn_summary_store).await; + handle_turn_complete( + conversation_b, + b_turn1.clone(), + &outgoing, + &turn_summary_store, + ) + .await; + + // Turn 2 on conversation A + let a_turn2 = "a_turn2".to_string(); + handle_turn_complete( + conversation_a, + a_turn2.clone(), + &outgoing, + &turn_summary_store, + ) + .await; + + // Verify: A turn 1 + let msg = rx + .recv() + .await + .ok_or_else(|| anyhow!("should send first notification"))?; + match msg { + OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => { + assert_eq!(n.turn.id, a_turn1); + assert_eq!( + n.turn.status, + TurnStatus::Failed { + error: TurnError { + message: "a1".to_string(), + } + } + ); + } + other => bail!("unexpected message: {other:?}"), + } + + // Verify: B turn 1 + let msg = rx + .recv() + .await + .ok_or_else(|| anyhow!("should send second notification"))?; + match msg { + OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => { + assert_eq!(n.turn.id, b_turn1); + assert_eq!( + n.turn.status, + TurnStatus::Failed { + error: TurnError { + message: "b1".to_string(), + } + } + ); + } + other => bail!("unexpected message: {other:?}"), + } + + // Verify: A turn 2 + let msg = rx + .recv() + .await + .ok_or_else(|| anyhow!("should send third notification"))?; + match msg { + OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => { + assert_eq!(n.turn.id, a_turn2); + assert_eq!(n.turn.status, TurnStatus::Completed); + } + other => bail!("unexpected message: {other:?}"), + } + + assert!(rx.try_recv().is_err(), "no extra messages expected"); + Ok(()) + } + #[tokio::test] async fn test_construct_mcp_tool_call_begin_notification_without_args() { let begin_event = McpToolCallBeginEvent { diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 0172458074..6191fdf4a7 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -154,6 +154,14 @@ use uuid::Uuid; type PendingInterruptQueue = Vec<(RequestId, ApiVersion)>; pub(crate) type PendingInterrupts = Arc>>; +/// Per-conversation accumulation of the latest states e.g. error message while a turn runs. +#[derive(Default, Clone)] +pub(crate) struct TurnSummary { + pub(crate) last_error_message: Option, +} + +pub(crate) type TurnSummaryStore = Arc>>; + // Duration before a ChatGPT login attempt is abandoned. const LOGIN_CHATGPT_TIMEOUT: Duration = Duration::from_secs(10 * 60); struct ActiveLogin { @@ -178,6 +186,7 @@ pub(crate) struct CodexMessageProcessor { active_login: Arc>>, // Queue of pending interrupt requests per conversation. We reply when TurnAborted arrives. pending_interrupts: PendingInterrupts, + turn_summary_store: TurnSummaryStore, pending_fuzzy_searches: Arc>>>, feedback: CodexFeedback, } @@ -230,6 +239,7 @@ impl CodexMessageProcessor { conversation_listeners: HashMap::new(), active_login: Arc::new(Mutex::new(None)), pending_interrupts: Arc::new(Mutex::new(HashMap::new())), + turn_summary_store: Arc::new(Mutex::new(HashMap::new())), pending_fuzzy_searches: Arc::new(Mutex::new(HashMap::new())), feedback, } @@ -2363,9 +2373,6 @@ impl CodexMessageProcessor { } }; - // Keep a copy of v2 inputs for the notification payload. - let v2_inputs_for_notif = params.input.clone(); - // Map v2 input items to core input items. let mapped_items: Vec = params .input @@ -2405,12 +2412,8 @@ impl CodexMessageProcessor { Ok(turn_id) => { let turn = Turn { id: turn_id.clone(), - items: vec![ThreadItem::UserMessage { - id: turn_id, - content: v2_inputs_for_notif, - }], + items: vec![], status: TurnStatus::InProgress, - error: None, }; let response = TurnStartResponse { turn: turn.clone() }; @@ -2471,7 +2474,6 @@ impl CodexMessageProcessor { id: turn_id.clone(), items, status: TurnStatus::InProgress, - error: None, }; let response = TurnStartResponse { turn: turn.clone() }; self.outgoing.send_response(request_id, response).await; @@ -2591,6 +2593,7 @@ impl CodexMessageProcessor { let outgoing_for_task = self.outgoing.clone(); let pending_interrupts = self.pending_interrupts.clone(); + let turn_summary_store = self.turn_summary_store.clone(); let api_version_for_task = api_version; tokio::spawn(async move { loop { @@ -2647,6 +2650,7 @@ impl CodexMessageProcessor { conversation.clone(), outgoing_for_task.clone(), pending_interrupts.clone(), + turn_summary_store.clone(), api_version_for_task, ) .await; diff --git a/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs b/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs index d1deb60801..aa140c9e7f 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs @@ -5,14 +5,17 @@ use app_test_support::McpProcess; use app_test_support::create_mock_chat_completions_server; use app_test_support::create_shell_sse_response; use app_test_support::to_response; +use codex_app_server_protocol::JSONRPCNotification; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; +use codex_app_server_protocol::TurnCompletedNotification; use codex_app_server_protocol::TurnInterruptParams; use codex_app_server_protocol::TurnInterruptResponse; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::TurnStartResponse; +use codex_app_server_protocol::TurnStatus; use codex_app_server_protocol::UserInput as V2UserInput; use tempfile::TempDir; use tokio::time::timeout; @@ -99,7 +102,18 @@ async fn turn_interrupt_aborts_running_turn() -> Result<()> { .await??; let _resp: TurnInterruptResponse = to_response::(interrupt_resp)?; - // No fields to assert on; successful deserialization confirms proper response shape. + let completed_notif: JSONRPCNotification = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + let completed: TurnCompletedNotification = serde_json::from_value( + completed_notif + .params + .expect("turn/completed params must be present"), + )?; + assert_eq!(completed.turn.status, TurnStatus::Interrupted); + Ok(()) } diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index 433c7b4486..753e33c243 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -14,9 +14,11 @@ use codex_app_server_protocol::ServerRequest; use codex_app_server_protocol::ThreadItem; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; +use codex_app_server_protocol::TurnCompletedNotification; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::TurnStartedNotification; +use codex_app_server_protocol::TurnStatus; use codex_app_server_protocol::UserInput as V2UserInput; use codex_core::protocol_config_types::ReasoningEffort; use codex_core::protocol_config_types::ReasoningSummary; @@ -118,13 +120,17 @@ async fn turn_start_emits_notifications_and_accepts_model_override() -> Result<( ) .await??; - // And we should ultimately get a task_complete without having to add a - // legacy conversation listener explicitly (auto-attached by thread/start). - let _task_complete: JSONRPCNotification = timeout( + let completed_notif: JSONRPCNotification = timeout( DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_notification_message("codex/event/task_complete"), + mcp.read_stream_until_notification_message("turn/completed"), ) .await??; + let completed: TurnCompletedNotification = serde_json::from_value( + completed_notif + .params + .expect("turn/completed params must be present"), + )?; + assert_eq!(completed.turn.status, TurnStatus::Completed); Ok(()) } @@ -274,6 +280,11 @@ async fn turn_start_exec_approval_toggle_v2() -> Result<()> { mcp.read_stream_until_notification_message("codex/event/task_complete"), ) .await??; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; // Second turn with approval_policy=never should not elicit approval let second_turn_id = mcp @@ -302,6 +313,11 @@ async fn turn_start_exec_approval_toggle_v2() -> Result<()> { mcp.read_stream_until_notification_message("codex/event/task_complete"), ) .await??; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; Ok(()) } From cac0a6a29d1060877c588ba266835c06f4c45e3c Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Tue, 18 Nov 2025 18:21:57 -0800 Subject: [PATCH 3/6] Remote compaction on by-default (#6866) --- codex-rs/core/src/features.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index 5f579defd4..a3457e363c 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -313,7 +313,7 @@ pub const FEATURES: &[FeatureSpec] = &[ id: Feature::RemoteCompaction, key: "remote_compaction", stage: Stage::Experimental, - default_enabled: false, + default_enabled: true, }, FeatureSpec { id: Feature::ParallelToolCalls, From 7508e4fd2d0161a3c7558391f7c0ce5fe92224f5 Mon Sep 17 00:00:00 2001 From: ae Date: Tue, 18 Nov 2025 21:02:04 -0800 Subject: [PATCH 4/6] chore: update windows sandbox docs (#6872) --- docs/sandbox.md | 41 ++++++++++++++++---------------- docs/windows_sandbox_security.md | 4 +++- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/docs/sandbox.md b/docs/sandbox.md index 674ecc4838..94f9c8280c 100644 --- a/docs/sandbox.md +++ b/docs/sandbox.md @@ -4,21 +4,12 @@ What Codex is allowed to do is governed by a combination of **sandbox modes** (w ### Approval policies -Codex starts conservatively. Until you explicitly tell it a workspace is trusted, the CLI defaults to **read-only sandboxing** with the `read-only` approval preset. Codex can inspect files and answer questions, but every edit or command requires approval. +Codex starts conservatively. Until you explicitly tell it a working directory is trusted, the CLI defaults to **read-only**. Codex can inspect files and answer questions, but every edit or command requires approval. -When you mark a workspace as trusted (for example via the onboarding prompt or `/approvals` → “Trust this directory”), Codex upgrades the default preset to **Auto**: sandboxed writes inside the workspace with `AskForApproval::OnRequest`. Codex only interrupts you when it needs to leave the workspace or rerun something outside the sandbox. +When you mark a working directory as trusted (for example via the onboarding prompt or `/approvals` → “Trust this directory”), Codex upgrades the default preset to **Agent**, which allows writes inside the workspace. Codex only interrupts you when it needs to leave the workspace or rerun something outside the sandbox. Note that the workspace includes the working directory plus temporary directories like `/tmp`. Use `/status` to confirm the exact writable roots. If you want maximum guardrails for a trusted repo, switch back to Read Only from the `/approvals` picker. If you truly need hands-off automation, use `Full Access`—but be deliberate, because that skips both the sandbox and approvals. -#### Defaults and recommendations - -- Every session starts in a sandbox. Until a repo is trusted, Codex enforces read-only access and will prompt before any write or command. -- Marking a repo as trusted switches the default preset to Auto (`workspace-write` + `ask-for-approval on-request`) so Codex can keep iterating locally without nagging you. -- The workspace always includes the current directory plus temporary directories like `/tmp`. Use `/status` to confirm the exact writable roots. -- You can override the defaults from the command line at any time: - - `codex --sandbox read-only --ask-for-approval on-request` - - `codex --sandbox workspace-write --ask-for-approval on-request` - ### Can I run without ANY approvals? Yes, you can disable all approval prompts with `--ask-for-approval never`. This option works with all `--sandbox` modes, so you still have full control over Codex's level of autonomy. It will make its best attempt with whatever constraints you provide. @@ -63,22 +54,32 @@ approval_policy = "never" sandbox_mode = "read-only" ``` -### Sandbox mechanics by platform {#platform-sandboxing-details} +### Sandbox mechanics by platform The mechanism Codex uses to enforce the sandbox policy depends on your OS: -- **macOS 12+** uses **Apple Seatbelt**. Codex invokes `sandbox-exec` with a profile that corresponds to the selected `--sandbox` mode, constraining filesystem and network access at the OS level. -- **Linux** combines **Landlock** and **seccomp** APIs to approximate the same guarantees. Kernel support is required; older kernels may not expose the necessary features. -- **Windows (experimental)**: - - Launches commands inside a restricted token derived from an AppContainer profile. - - Grants only specifically requested filesystem capabilities by attaching capability SIDs to that profile. - - Disables outbound network access by overriding proxy-related environment variables and inserting stub executables for common network tools. +#### macOS 12+ -Windows sandbox support remains highly experimental. It cannot prevent file writes, deletions, or creations in any directory where the Everyone SID already has write permissions (for example, world-writable folders). +Uses **Apple Seatbelt**. Codex invokes `sandbox-exec` with a profile that corresponds to the selected `--sandbox` mode, constraining filesystem and network access at the OS level. + +#### Linux + +Combines **Landlock** and **seccomp** APIs to approximate the same guarantees. Kernel support is required; older kernels may not expose the necessary features. In containerized Linux environments (for example Docker), sandboxing may not work when the host or container configuration does not expose Landlock/seccomp. In those cases, configure the container to provide the isolation you need and run Codex with `--sandbox danger-full-access` (or the shorthand `--dangerously-bypass-approvals-and-sandbox`) inside that container. -### Experimenting with the Codex Sandbox +#### Windows + +Windows sandbox support remains experimental. How it works: + +- Launches commands inside a restricted token derived from an AppContainer profile. +- Grants only specifically requested filesystem capabilities by attaching capability SIDs to that profile. +- Disables outbound network access by overriding proxy-related environment variables and inserting stub executables for common network tools. + +Its primary limitation is that it cannot prevent file writes, deletions, or creations in any directory where the Everyone SID already has write permissions (for example, world-writable folders). +See more discussion and limitations at [Windows Sandbox Security Details](./windows_sandbox_security.md). + +## Experimenting with the Codex Sandbox To test how commands behave under Codex's sandbox, use the CLI helpers: diff --git a/docs/windows_sandbox_security.md b/docs/windows_sandbox_security.md index 3dab0449d1..e8008964f9 100644 --- a/docs/windows_sandbox_security.md +++ b/docs/windows_sandbox_security.md @@ -1,4 +1,6 @@ -# Windows Sandbox Status +# Windows Sandbox Security Details + +For overall context on sandboxing in Codex, see [sandbox.md](./sandbox.md). ## Implementation Overview From a75321a64c990275ed4368bf26a5334c9ddfa0a7 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 18 Nov 2025 21:18:43 -0800 Subject: [PATCH 5/6] fix: add more fields to ThreadStartResponse and ThreadResumeResponse (#6847) This adds the following fields to `ThreadStartResponse` and `ThreadResumeResponse`: ```rust pub model: String, pub model_provider: String, pub cwd: PathBuf, pub approval_policy: AskForApproval, pub sandbox: SandboxPolicy, pub reasoning_effort: Option, ``` This is important because these fields are optional in `ThreadStartParams` and `ThreadResumeParams`, so the caller needs to be able to determine what values were ultimately used to start/resume the conversation. (Though note that any of these could be changed later between turns in the conversation.) Though to get this information reliably, it must be read from the internal `SessionConfiguredEvent` that is created in response to the start of a conversation. Because `SessionConfiguredEvent` (as defined in `codex-rs/protocol/src/protocol.rs`) did not have all of these fields, a number of them had to be added as part of this PR. Because `SessionConfiguredEvent` is referenced in many tests, test instances of `SessionConfiguredEvent` had to be updated, as well, which is why this PR touches so many files. --- codex-rs/Cargo.lock | 1 + .../app-server-protocol/src/protocol/v2.rs | 12 +++++++ .../app-server/src/codex_message_processor.rs | 33 +++++++++++++++++-- codex-rs/app-server/tests/suite/v2/review.rs | 2 +- .../tests/suite/v2/thread_archive.rs | 2 +- .../tests/suite/v2/thread_resume.rs | 25 ++++++++------ .../app-server/tests/suite/v2/thread_start.rs | 8 +++-- .../tests/suite/v2/turn_interrupt.rs | 2 +- .../app-server/tests/suite/v2/turn_start.rs | 8 ++--- codex-rs/core/src/codex.rs | 5 ++- .../src/event_processor_with_human_output.rs | 6 +--- .../tests/event_processor_with_json_output.rs | 6 ++++ codex-rs/mcp-server/src/outgoing_message.rs | 22 +++++++++++-- codex-rs/protocol/Cargo.toml | 3 +- codex-rs/protocol/src/protocol.rs | 25 +++++++++++++- codex-rs/tui/src/app.rs | 6 ++++ codex-rs/tui/src/chatwidget/tests.rs | 4 +++ codex-rs/tui/src/history_cell.rs | 6 +--- 18 files changed, 139 insertions(+), 37 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index c756c30ab9..1f7934bc45 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1420,6 +1420,7 @@ dependencies = [ "icu_provider", "mcp-types", "mime_guess", + "pretty_assertions", "schemars 0.8.22", "serde", "serde_json", diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 05e0e3560e..67b072e669 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -402,6 +402,12 @@ pub struct ThreadStartParams { #[ts(export_to = "v2/")] pub struct ThreadStartResponse { pub thread: Thread, + pub model: String, + pub model_provider: String, + pub cwd: PathBuf, + pub approval_policy: AskForApproval, + pub sandbox: SandboxPolicy, + pub reasoning_effort: Option, } #[derive(Serialize, Deserialize, Debug, Default, Clone, PartialEq, JsonSchema, TS)] @@ -444,6 +450,12 @@ pub struct ThreadResumeParams { #[ts(export_to = "v2/")] pub struct ThreadResumeResponse { pub thread: Thread, + pub model: String, + pub model_provider: String, + pub cwd: PathBuf, + pub approval_policy: AskForApproval, + pub sandbox: SandboxPolicy, + pub reasoning_effort: Option, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 6191fdf4a7..3cb0d5b569 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -118,6 +118,7 @@ use codex_core::parse_cursor; use codex_core::protocol::EventMsg; use codex_core::protocol::Op; use codex_core::protocol::ReviewRequest; +use codex_core::protocol::SessionConfiguredEvent; use codex_core::read_head_for_summary; use codex_feedback::CodexFeedback; use codex_login::ServerOptions as LoginServerOptions; @@ -1313,8 +1314,12 @@ impl CodexMessageProcessor { match self.conversation_manager.new_conversation(config).await { Ok(new_conv) => { - let conversation_id = new_conv.conversation_id; - let rollout_path = new_conv.session_configured.rollout_path.clone(); + let NewConversation { + conversation_id, + session_configured, + .. + } = new_conv; + let rollout_path = session_configured.rollout_path.clone(); let fallback_provider = self.config.model_provider_id.as_str(); // A bit hacky, but the summary contains a lot of useful information for the thread @@ -1339,8 +1344,22 @@ impl CodexMessageProcessor { } }; + let SessionConfiguredEvent { + model, + model_provider_id, + cwd, + approval_policy, + sandbox_policy, + .. + } = session_configured; let response = ThreadStartResponse { thread: thread.clone(), + model, + model_provider: model_provider_id, + cwd, + approval_policy: approval_policy.into(), + sandbox: sandbox_policy.into(), + reasoning_effort: session_configured.reasoning_effort, }; // Auto-attach a conversation listener when starting a thread. @@ -1653,7 +1672,15 @@ impl CodexMessageProcessor { return; } }; - let response = ThreadResumeResponse { thread }; + let response = ThreadResumeResponse { + thread, + model: session_configured.model, + model_provider: session_configured.model_provider_id, + cwd: session_configured.cwd, + approval_policy: session_configured.approval_policy.into(), + sandbox: session_configured.sandbox_policy.into(), + reasoning_effort: session_configured.reasoning_effort, + }; self.outgoing.send_response(request_id, response).await; } Err(err) => { diff --git a/codex-rs/app-server/tests/suite/v2/review.rs b/codex-rs/app-server/tests/suite/v2/review.rs index 194ed6ae96..cdb3acd088 100644 --- a/codex-rs/app-server/tests/suite/v2/review.rs +++ b/codex-rs/app-server/tests/suite/v2/review.rs @@ -251,7 +251,7 @@ async fn start_default_thread(mcp: &mut McpProcess) -> Result { mcp.read_stream_until_response_message(RequestId::Integer(thread_req)), ) .await??; - let ThreadStartResponse { thread } = to_response::(thread_resp)?; + let ThreadStartResponse { thread, .. } = to_response::(thread_resp)?; Ok(thread.id) } diff --git a/codex-rs/app-server/tests/suite/v2/thread_archive.rs b/codex-rs/app-server/tests/suite/v2/thread_archive.rs index 083f3da901..88891af77d 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_archive.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_archive.rs @@ -35,7 +35,7 @@ async fn thread_archive_moves_rollout_into_archived_directory() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(start_id)), ) .await??; - let ThreadStartResponse { thread } = to_response::(start_resp)?; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; assert!(!thread.id.is_empty()); // Locate the rollout path recorded for this thread id. diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index bda2d14172..89020c8876 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -36,7 +36,7 @@ async fn thread_resume_returns_original_thread() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(start_id)), ) .await??; - let ThreadStartResponse { thread } = to_response::(start_resp)?; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; // Resume it via v2 API. let resume_id = mcp @@ -50,8 +50,9 @@ async fn thread_resume_returns_original_thread() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(resume_id)), ) .await??; - let ThreadResumeResponse { thread: resumed } = - to_response::(resume_resp)?; + let ThreadResumeResponse { + thread: resumed, .. + } = to_response::(resume_resp)?; assert_eq!(resumed, thread); Ok(()) @@ -77,7 +78,7 @@ async fn thread_resume_prefers_path_over_thread_id() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(start_id)), ) .await??; - let ThreadStartResponse { thread } = to_response::(start_resp)?; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; let thread_path = thread.path.clone(); let resume_id = mcp @@ -93,8 +94,9 @@ async fn thread_resume_prefers_path_over_thread_id() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(resume_id)), ) .await??; - let ThreadResumeResponse { thread: resumed } = - to_response::(resume_resp)?; + let ThreadResumeResponse { + thread: resumed, .. + } = to_response::(resume_resp)?; assert_eq!(resumed, thread); Ok(()) @@ -121,7 +123,7 @@ async fn thread_resume_supports_history_and_overrides() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(start_id)), ) .await??; - let ThreadStartResponse { thread } = to_response::(start_resp)?; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; let history_text = "Hello from history"; let history = vec![ResponseItem::Message { @@ -147,10 +149,13 @@ async fn thread_resume_supports_history_and_overrides() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(resume_id)), ) .await??; - let ThreadResumeResponse { thread: resumed } = - to_response::(resume_resp)?; + let ThreadResumeResponse { + thread: resumed, + model_provider, + .. + } = to_response::(resume_resp)?; assert!(!resumed.id.is_empty()); - assert_eq!(resumed.model_provider, "mock_provider"); + assert_eq!(model_provider, "mock_provider"); assert_eq!(resumed.preview, history_text); Ok(()) diff --git a/codex-rs/app-server/tests/suite/v2/thread_start.rs b/codex-rs/app-server/tests/suite/v2/thread_start.rs index a5e4c0d487..ad0949ba29 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -40,13 +40,17 @@ async fn thread_start_creates_thread_and_emits_started() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(req_id)), ) .await??; - let ThreadStartResponse { thread } = to_response::(resp)?; + let ThreadStartResponse { + thread, + model_provider, + .. + } = to_response::(resp)?; assert!(!thread.id.is_empty(), "thread id should not be empty"); assert!( thread.preview.is_empty(), "new threads should start with an empty preview" ); - assert_eq!(thread.model_provider, "mock_provider"); + assert_eq!(model_provider, "mock_provider"); assert!( thread.created_at > 0, "created_at should be a positive UNIX timestamp" diff --git a/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs b/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs index aa140c9e7f..beafec9cfc 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs @@ -65,7 +65,7 @@ async fn turn_interrupt_aborts_running_turn() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(thread_req)), ) .await??; - let ThreadStartResponse { thread } = to_response::(thread_resp)?; + let ThreadStartResponse { thread, .. } = to_response::(thread_resp)?; // Start a turn that triggers a long-running command. let turn_req = mcp diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index 753e33c243..8c4cf4e677 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -59,7 +59,7 @@ async fn turn_start_emits_notifications_and_accepts_model_override() -> Result<( mcp.read_stream_until_response_message(RequestId::Integer(thread_req)), ) .await??; - let ThreadStartResponse { thread } = to_response::(thread_resp)?; + let ThreadStartResponse { thread, .. } = to_response::(thread_resp)?; // Start a turn with only input and thread_id set (no overrides). let turn_req = mcp @@ -163,7 +163,7 @@ async fn turn_start_accepts_local_image_input() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(thread_req)), ) .await??; - let ThreadStartResponse { thread } = to_response::(thread_resp)?; + let ThreadStartResponse { thread, .. } = to_response::(thread_resp)?; let image_path = codex_home.path().join("image.png"); // No need to actually write the file; we just exercise the input path. @@ -239,7 +239,7 @@ async fn turn_start_exec_approval_toggle_v2() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(start_id)), ) .await??; - let ThreadStartResponse { thread } = to_response::(start_resp)?; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; // turn/start — expect CommandExecutionRequestApproval request from server let first_turn_id = mcp @@ -378,7 +378,7 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(start_id)), ) .await??; - let ThreadStartResponse { thread } = to_response::(start_resp)?; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; // first turn with workspace-write sandbox and first_cwd let first_turn = mcp diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 5f45fc25b6..7cf90ad1e3 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -293,7 +293,6 @@ impl TurnContext { } } -#[allow(dead_code)] #[derive(Clone)] pub(crate) struct SessionConfiguration { /// Provider identifier ("openai", "openrouter", ...). @@ -581,6 +580,10 @@ impl Session { msg: EventMsg::SessionConfigured(SessionConfiguredEvent { session_id: conversation_id, model: session_configuration.model.clone(), + model_provider_id: config.model_provider_id.clone(), + approval_policy: session_configuration.approval_policy, + sandbox_policy: session_configuration.sandbox_policy.clone(), + cwd: session_configuration.cwd.clone(), reasoning_effort: session_configuration.model_reasoning_effort, history_log_id, history_entry_count, 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 8c7bb6881e..2d550fea46 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -480,11 +480,7 @@ impl EventProcessor for EventProcessorWithHumanOutput { let SessionConfiguredEvent { session_id: conversation_id, model, - reasoning_effort: _, - history_log_id: _, - history_entry_count: _, - initial_messages: _, - rollout_path: _, + .. } = session_configured_event; ts_msg!( 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 7a6245ae77..5053f61921 100644 --- a/codex-rs/exec/tests/event_processor_with_json_output.rs +++ b/codex-rs/exec/tests/event_processor_with_json_output.rs @@ -1,5 +1,6 @@ use codex_core::protocol::AgentMessageEvent; use codex_core::protocol::AgentReasoningEvent; +use codex_core::protocol::AskForApproval; use codex_core::protocol::ErrorEvent; use codex_core::protocol::Event; use codex_core::protocol::EventMsg; @@ -12,6 +13,7 @@ use codex_core::protocol::McpToolCallBeginEvent; use codex_core::protocol::McpToolCallEndEvent; use codex_core::protocol::PatchApplyBeginEvent; use codex_core::protocol::PatchApplyEndEvent; +use codex_core::protocol::SandboxPolicy; use codex_core::protocol::SessionConfiguredEvent; use codex_core::protocol::WarningEvent; use codex_core::protocol::WebSearchEndEvent; @@ -72,6 +74,10 @@ fn session_configured_produces_thread_started_event() { EventMsg::SessionConfigured(SessionConfiguredEvent { session_id, model: "codex-mini-latest".to_string(), + model_provider_id: "test-provider".to_string(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::ReadOnly, + cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, history_log_id: 0, history_entry_count: 0, diff --git a/codex-rs/mcp-server/src/outgoing_message.rs b/codex-rs/mcp-server/src/outgoing_message.rs index 4b6782d8de..9e9d079306 100644 --- a/codex-rs/mcp-server/src/outgoing_message.rs +++ b/codex-rs/mcp-server/src/outgoing_message.rs @@ -231,8 +231,12 @@ pub(crate) struct OutgoingError { #[cfg(test)] mod tests { + use std::path::PathBuf; + use anyhow::Result; + use codex_core::protocol::AskForApproval; use codex_core::protocol::EventMsg; + use codex_core::protocol::SandboxPolicy; use codex_core::protocol::SessionConfiguredEvent; use codex_protocol::ConversationId; use codex_protocol::config_types::ReasoningEffort; @@ -254,6 +258,10 @@ mod tests { msg: EventMsg::SessionConfigured(SessionConfiguredEvent { session_id: conversation_id, model: "gpt-4o".to_string(), + model_provider_id: "test-provider".to_string(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::ReadOnly, + cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffort::default()), history_log_id: 1, history_entry_count: 1000, @@ -289,6 +297,10 @@ mod tests { let session_configured_event = SessionConfiguredEvent { session_id: conversation_id, model: "gpt-4o".to_string(), + model_provider_id: "test-provider".to_string(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::ReadOnly, + cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffort::default()), history_log_id: 1, history_entry_count: 1000, @@ -318,12 +330,18 @@ mod tests { }, "id": "1", "msg": { + "type": "session_configured", "session_id": session_configured_event.session_id, - "model": session_configured_event.model, + "model": "gpt-4o", + "model_provider_id": "test-provider", + "approval_policy": "never", + "sandbox_policy": { + "type": "read-only" + }, + "cwd": "/home/user/project", "reasoning_effort": session_configured_event.reasoning_effort, "history_log_id": session_configured_event.history_log_id, "history_entry_count": session_configured_event.history_entry_count, - "type": "session_configured", "rollout_path": rollout_file.path().to_path_buf(), } }); diff --git a/codex-rs/protocol/Cargo.toml b/codex-rs/protocol/Cargo.toml index d3ec3af08d..00ed100e08 100644 --- a/codex-rs/protocol/Cargo.toml +++ b/codex-rs/protocol/Cargo.toml @@ -20,10 +20,10 @@ icu_locale_core = { workspace = true } icu_provider = { workspace = true, features = ["sync"] } mcp-types = { workspace = true } mime_guess = { workspace = true } +schemars = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } serde_with = { workspace = true, features = ["macros", "base64"] } -schemars = { workspace = true } strum = { workspace = true } strum_macros = { workspace = true } sys-locale = { workspace = true } @@ -37,6 +37,7 @@ uuid = { workspace = true, features = ["serde", "v7", "v4"] } [dev-dependencies] anyhow = { workspace = true } +pretty_assertions = { workspace = true } tempfile = { workspace = true } [package.metadata.cargo-shear] diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 934b6ad286..e3bc76199a 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1469,7 +1469,7 @@ pub struct ListCustomPromptsResponseEvent { pub custom_prompts: Vec, } -#[derive(Debug, Default, Clone, Deserialize, Serialize, JsonSchema, TS)] +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct SessionConfiguredEvent { /// Name left as session_id instead of conversation_id for backwards compatibility. pub session_id: ConversationId, @@ -1477,6 +1477,18 @@ pub struct SessionConfiguredEvent { /// Tell the client what model is being queried. pub model: String, + pub model_provider_id: String, + + /// When to escalate for approval for execution + pub approval_policy: AskForApproval, + + /// How to sandbox commands executed in the system + pub sandbox_policy: SandboxPolicy, + + /// Working directory that should be treated as the *root* of the + /// session. + pub cwd: PathBuf, + /// The effort the model is putting into reasoning about the user's request. #[serde(skip_serializing_if = "Option::is_none")] pub reasoning_effort: Option, @@ -1562,6 +1574,7 @@ mod tests { use crate::items::UserMessageItem; use crate::items::WebSearchItem; use anyhow::Result; + use pretty_assertions::assert_eq; use serde_json::json; use tempfile::NamedTempFile; @@ -1606,6 +1619,10 @@ mod tests { msg: EventMsg::SessionConfigured(SessionConfiguredEvent { session_id: conversation_id, model: "codex-mini-latest".to_string(), + model_provider_id: "openai".to_string(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::ReadOnly, + cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffortConfig::default()), history_log_id: 0, history_entry_count: 0, @@ -1620,6 +1637,12 @@ mod tests { "type": "session_configured", "session_id": "67e55044-10b1-426f-9247-bb680e5fe0c8", "model": "codex-mini-latest", + "model_provider_id": "openai", + "approval_policy": "never", + "sandbox_policy": { + "type": "read-only" + }, + "cwd": "/home/user/project", "reasoning_effort": "medium", "history_log_id": 0, "history_entry_count": 0, diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 718c33af3c..32c466db57 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -934,6 +934,8 @@ mod tests { use codex_core::AuthManager; use codex_core::CodexAuth; use codex_core::ConversationManager; + use codex_core::protocol::AskForApproval; + use codex_core::protocol::SandboxPolicy; use codex_core::protocol::SessionConfiguredEvent; use codex_protocol::ConversationId; use ratatui::prelude::Line; @@ -1043,6 +1045,10 @@ mod tests { let event = SessionConfiguredEvent { session_id: ConversationId::new(), model: "gpt-test".to_string(), + model_provider_id: "test-provider".to_string(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::ReadOnly, + cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, history_log_id: 0, history_entry_count: 0, diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index d6d8f0248d..bad8f24c30 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -93,6 +93,10 @@ fn resumed_initial_messages_render_history() { let configured = codex_core::protocol::SessionConfiguredEvent { session_id: conversation_id, model: "test-model".to_string(), + model_provider_id: "test-provider".to_string(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::ReadOnly, + cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffortConfig::default()), history_log_id: 0, history_entry_count: 0, diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index b026170f70..f479f3933e 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -584,11 +584,7 @@ pub(crate) fn new_session_info( let SessionConfiguredEvent { model, reasoning_effort, - session_id: _, - history_log_id: _, - history_entry_count: _, - initial_messages: _, - rollout_path: _, + .. } = event; SessionInfoCell(if is_first_event { // Header box rendered as history (so it appears at the very top) From 4c82f9a609bd504ad63fd354a943e1f6cd806fd3 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 19 Nov 2025 00:37:11 -0800 Subject: [PATCH 6/6] fix: prepare ExecPolicy in exec-server for execpolicy2 cutover --- codex-rs/Cargo.lock | 1 + codex-rs/Cargo.toml | 1 + codex-rs/exec-server/Cargo.toml | 1 + codex-rs/exec-server/src/posix.rs | 23 +++++++--- .../exec-server/src/posix/escalate_client.rs | 3 ++ .../src/posix/escalate_protocol.rs | 2 + .../exec-server/src/posix/escalate_server.rs | 45 ++++++++++++++++--- 7 files changed, 66 insertions(+), 10 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 1f7934bc45..f355462374 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1189,6 +1189,7 @@ dependencies = [ "anyhow", "clap", "codex-core", + "codex-execpolicy2", "libc", "path-absolutize", "pretty_assertions", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 2e88aba713..b0cf1e70fb 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -67,6 +67,7 @@ codex-chatgpt = { path = "chatgpt" } codex-common = { path = "common" } codex-core = { path = "core" } codex-exec = { path = "exec" } +codex-execpolicy2 = { path = "execpolicy2" } codex-feedback = { path = "feedback" } codex-file-search = { path = "file-search" } codex-git = { path = "utils/git" } diff --git a/codex-rs/exec-server/Cargo.toml b/codex-rs/exec-server/Cargo.toml index 24ee460f50..6c211d3953 100644 --- a/codex-rs/exec-server/Cargo.toml +++ b/codex-rs/exec-server/Cargo.toml @@ -14,6 +14,7 @@ workspace = true anyhow = { workspace = true } clap = { workspace = true, features = ["derive"] } codex-core = { workspace = true } +codex-execpolicy2 = { workspace = true } libc = { workspace = true } path-absolutize = { workspace = true } rmcp = { workspace = true, default-features = false, features = [ diff --git a/codex-rs/exec-server/src/posix.rs b/codex-rs/exec-server/src/posix.rs index 179fc7a6fc..d9820a50c4 100644 --- a/codex-rs/exec-server/src/posix.rs +++ b/codex-rs/exec-server/src/posix.rs @@ -59,11 +59,12 @@ use std::path::Path; use clap::Parser; use clap::Subcommand; +use codex_execpolicy2::Decision; use tracing_subscriber::EnvFilter; use tracing_subscriber::{self}; -use crate::posix::escalate_protocol::EscalateAction; use crate::posix::escalate_server::EscalateServer; +use crate::posix::escalate_server::ExecPolicyOutcome; mod escalate_client; mod escalate_protocol; @@ -71,16 +72,28 @@ mod escalate_server; mod mcp; mod socket; -fn dummy_exec_policy(file: &Path, argv: &[String], _workdir: &Path) -> EscalateAction { +fn dummy_exec_policy(file: &Path, argv: &[String], _workdir: &Path) -> ExecPolicyOutcome { // TODO: execpolicy - if file == Path::new("/opt/homebrew/bin/gh") + if file.ends_with("/git") { + ExecPolicyOutcome { + decision: Decision::Prompt, + run_with_escalated_permissions: false, + } + } else if file == Path::new("/opt/homebrew/bin/gh") && let [_, arg1, arg2, ..] = argv && arg1 == "issue" && arg2 == "list" { - return EscalateAction::Escalate; + ExecPolicyOutcome { + decision: Decision::Allow, + run_with_escalated_permissions: true, + } + } else { + ExecPolicyOutcome { + decision: Decision::Allow, + run_with_escalated_permissions: false, + } } - EscalateAction::Run } #[derive(Parser)] diff --git a/codex-rs/exec-server/src/posix/escalate_client.rs b/codex-rs/exec-server/src/posix/escalate_client.rs index 2add1dade2..becf91944f 100644 --- a/codex-rs/exec-server/src/posix/escalate_client.rs +++ b/codex-rs/exec-server/src/posix/escalate_client.rs @@ -98,5 +98,8 @@ pub(crate) async fn run(file: String, argv: Vec) -> anyhow::Result Err(err.into()) } + EscalateAction::Deny => { + todo!("handle EscalateAction::Deny") + } } } diff --git a/codex-rs/exec-server/src/posix/escalate_protocol.rs b/codex-rs/exec-server/src/posix/escalate_protocol.rs index 09d33fc982..cb6f174388 100644 --- a/codex-rs/exec-server/src/posix/escalate_protocol.rs +++ b/codex-rs/exec-server/src/posix/escalate_protocol.rs @@ -34,6 +34,8 @@ pub(super) enum EscalateAction { Run, /// The command should be escalated to the server for execution. Escalate, + /// The command should be denied. + Deny, } /// The client sends this to the server to forward its open FDs. diff --git a/codex-rs/exec-server/src/posix/escalate_server.rs b/codex-rs/exec-server/src/posix/escalate_server.rs index 6d058ff0f0..1068874f45 100644 --- a/codex-rs/exec-server/src/posix/escalate_server.rs +++ b/codex-rs/exec-server/src/posix/escalate_server.rs @@ -12,6 +12,7 @@ use codex_core::exec::SandboxType; use codex_core::exec::process_exec_tool_call; use codex_core::get_platform_sandbox; use codex_core::protocol::SandboxPolicy; +use codex_execpolicy2::Decision as ExecPolicyDecision; use tokio::process::Command; use crate::posix::escalate_protocol::BASH_EXEC_WRAPPER_ENV_VAR; @@ -30,7 +31,13 @@ use crate::posix::socket::AsyncSocket; /// `argv` is the argv, including the program name (`argv[0]`). /// `workdir` is the absolute, canonical path to the working directory in which to execute the /// command. -pub(crate) type ExecPolicy = fn(file: &Path, argv: &[String], workdir: &Path) -> EscalateAction; +pub(crate) type ExecPolicy = fn(file: &Path, argv: &[String], workdir: &Path) -> ExecPolicyOutcome; + +#[derive(Debug)] +pub(crate) struct ExecPolicyOutcome { + pub(crate) decision: ExecPolicyDecision, + pub(crate) run_with_escalated_permissions: bool, +} pub(crate) struct EscalateServer { bash_path: PathBuf, @@ -132,8 +139,27 @@ async fn handle_escalate_session_with_policy( } = socket.receive::().await?; let file = PathBuf::from(&file).absolutize()?.into_owned(); let workdir = PathBuf::from(&workdir).absolutize()?.into_owned(); - let action = policy(file.as_path(), &argv, &workdir); - tracing::debug!("decided {action:?} for {file:?} {argv:?} {workdir:?}"); + let outcome = policy(file.as_path(), &argv, &workdir); + + tracing::debug!("decided {outcome:?} for {file:?} {argv:?} {workdir:?}"); + let ExecPolicyOutcome { + decision, + run_with_escalated_permissions, + } = outcome; + let action = match decision { + ExecPolicyDecision::Allow => { + if run_with_escalated_permissions { + EscalateAction::Escalate + } else { + EscalateAction::Run + } + } + ExecPolicyDecision::Prompt => { + todo!("prompting not yet implemented") + } + ExecPolicyDecision::Forbidden => EscalateAction::Deny, + }; + match action { EscalateAction::Run => { socket @@ -195,6 +221,9 @@ async fn handle_escalate_session_with_policy( }) .await?; } + EscalateAction::Deny => { + todo!("denying not yet implemented"); + } } Ok(()) } @@ -211,7 +240,10 @@ mod tests { let (server, client) = AsyncSocket::pair()?; let server_task = tokio::spawn(handle_escalate_session_with_policy( server, - |_file, _argv, _workdir| EscalateAction::Run, + |_file, _argv, _workdir| ExecPolicyOutcome { + decision: ExecPolicyDecision::Allow, + run_with_escalated_permissions: false, + }, )); client @@ -238,7 +270,10 @@ mod tests { let (server, client) = AsyncSocket::pair()?; let server_task = tokio::spawn(handle_escalate_session_with_policy( server, - |_file, _argv, _workdir| EscalateAction::Escalate, + |_file, _argv, _workdir| ExecPolicyOutcome { + decision: ExecPolicyDecision::Allow, + run_with_escalated_permissions: true, + }, )); client