From 9091ab403df5c789893aed02455094ed329b72d8 Mon Sep 17 00:00:00 2001 From: Chris Bookholt Date: Thu, 2 Jul 2026 20:09:58 -0700 Subject: [PATCH] Preserve repeated approval client integrity --- ...CommandExecutionRequestApprovalParams.json | 2 +- .../schema/json/ServerRequest.json | 2 +- .../codex_app_server_protocol.schemas.json | 2 +- .../CommandExecutionRequestApprovalParams.ts | 10 +- .../src/protocol/v2/item.rs | 10 +- codex-rs/app-server/README.md | 2 +- .../app-server/src/bespoke_event_handling.rs | 473 ++++++------------ codex-rs/app-server/src/thread_state.rs | 10 +- .../tests/suite/v2/turn_start_zsh_fork.rs | 29 ++ codex-rs/core/src/session/mod.rs | 22 +- codex-rs/core/src/session/tests.rs | 85 ++++ codex-rs/protocol/src/approvals.rs | 33 +- codex-rs/tui/src/app.rs | 8 + codex-rs/tui/src/app/tests.rs | 43 ++ codex-rs/tui/src/app/thread_routing.rs | 1 + codex-rs/tui/src/approval_events.rs | 4 + .../src/chatwidget/tests/approval_requests.rs | 6 +- .../tui/src/chatwidget/tests/exec_flow.rs | 54 +- .../src/chatwidget/tests/status_and_layout.rs | 2 +- 19 files changed, 433 insertions(+), 365 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json index 293f3823d4..84dc4a8cf4 100644 --- a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json @@ -521,7 +521,7 @@ }, "properties": { "approvalId": { - "description": "Unique identifier for this specific approval callback.\n\nFor a command item with only one approval callback, this is absent.\n\nWhen multiple callbacks can belong to one parent `itemId` (for example, a zsh-exec-bridge subcommand or an unsandboxed retry), `approvalId` is a distinct opaque callback id (a UUID) used to disambiguate routing.", + "description": "Opaque identifier for this specific approval callback, when present.\n\nUse the JSON-RPC request ID for the response. When tracking callback UI or state outside that envelope, use this instead of `itemId` when it is present. Absence is a legacy/cacheable compatibility path.", "type": [ "string", "null" diff --git a/codex-rs/app-server-protocol/schema/json/ServerRequest.json b/codex-rs/app-server-protocol/schema/json/ServerRequest.json index 749c428674..5cf3c61711 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ServerRequest.json @@ -345,7 +345,7 @@ "CommandExecutionRequestApprovalParams": { "properties": { "approvalId": { - "description": "Unique identifier for this specific approval callback.\n\nFor a command item with only one approval callback, this is absent.\n\nWhen multiple callbacks can belong to one parent `itemId` (for example, a zsh-exec-bridge subcommand or an unsandboxed retry), `approvalId` is a distinct opaque callback id (a UUID) used to disambiguate routing.", + "description": "Opaque identifier for this specific approval callback, when present.\n\nUse the JSON-RPC request ID for the response. When tracking callback UI or state outside that envelope, use this instead of `itemId` when it is present. Absence is a legacy/cacheable compatibility path.", "type": [ "string", "null" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index ea3e6b38ae..0dfb9a8de5 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -2375,7 +2375,7 @@ "$schema": "http://json-schema.org/draft-07/schema#", "properties": { "approvalId": { - "description": "Unique identifier for this specific approval callback.\n\nFor a command item with only one approval callback, this is absent.\n\nWhen multiple callbacks can belong to one parent `itemId` (for example, a zsh-exec-bridge subcommand or an unsandboxed retry), `approvalId` is a distinct opaque callback id (a UUID) used to disambiguate routing.", + "description": "Opaque identifier for this specific approval callback, when present.\n\nUse the JSON-RPC request ID for the response. When tracking callback UI or state outside that envelope, use this instead of `itemId` when it is present. Absence is a legacy/cacheable compatibility path.", "type": [ "string", "null" diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts index fddee8a816..8a77e5ffe0 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts @@ -11,13 +11,11 @@ export type CommandExecutionRequestApprovalParams = {threadId: string, turnId: s * Unix timestamp (in milliseconds) when this approval request started. */ startedAtMs: number, /** - * Unique identifier for this specific approval callback. + * Opaque identifier for this specific approval callback, when present. * - * For a command item with only one approval callback, this is absent. - * - * When multiple callbacks can belong to one parent `itemId` (for example, - * a zsh-exec-bridge subcommand or an unsandboxed retry), `approvalId` is a - * distinct opaque callback id (a UUID) used to disambiguate routing. + * Use the JSON-RPC request ID for the response. When tracking callback UI + * or state outside that envelope, use this instead of `itemId` when it is + * present. Absence is a legacy/cacheable compatibility path. */ approvalId?: string | null, /** * Environment in which the command will run. diff --git a/codex-rs/app-server-protocol/src/protocol/v2/item.rs b/codex-rs/app-server-protocol/src/protocol/v2/item.rs index c6c6b88980..3157aee0b2 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/item.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/item.rs @@ -1444,13 +1444,11 @@ pub struct CommandExecutionRequestApprovalParams { /// Unix timestamp (in milliseconds) when this approval request started. #[ts(type = "number")] pub started_at_ms: i64, - /// Unique identifier for this specific approval callback. + /// Opaque identifier for this specific approval callback, when present. /// - /// For a command item with only one approval callback, this is absent. - /// - /// When multiple callbacks can belong to one parent `itemId` (for example, - /// a zsh-exec-bridge subcommand or an unsandboxed retry), `approvalId` is a - /// distinct opaque callback id (a UUID) used to disambiguate routing. + /// Use the JSON-RPC request ID for the response. When tracking callback UI + /// or state outside that envelope, use this instead of `itemId` when it is + /// present. Absence is a legacy/cacheable compatibility path. #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional = nullable)] pub approval_id: Option, diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index a3909d5447..9eb7ae1598 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1453,7 +1453,7 @@ Certain actions (shell commands or modifying files) may require explicit user ap Order of messages: 1. `item/started` — shows the pending `commandExecution` item with `command`, `cwd`, and other fields so you can render the proposed action. -2. `item/commandExecution/requestApproval` (request) — carries the same `itemId`, `threadId`, `turnId`, the nullable `environmentId` where the command will run, optionally `approvalId`, and `reason`. `itemId` remains the stable command-item identity. `approvalId` is a distinct opaque callback identity when one command can prompt more than once, such as an execve/subcommand callback or an unsandboxed retry; it is unique for each such callback and must not be rendered as a separate command item. A follow-up callback may reuse an active item without another `item/started`; if the prior attempt already completed, the server can restart the same `itemId` before requesting retry approval. New shell and unified-exec approvals set `environmentId`; older events that do not provide one are exposed as `null`. For normal command approvals, the request also includes `command`, `cwd`, and `commandActions` for friendly display. When `initialize.params.capabilities.experimentalApi = true`, it may also include experimental `additionalPermissions` describing requested per-command sandbox access; any filesystem paths in that payload are absolute on the wire, and network access is represented as `additionalPermissions.network.enabled`. For network-only approvals, those command fields may be omitted and `networkApprovalContext` is provided instead. Optional persistence hints may also be included via `proposedExecpolicyAmendment` and `proposedNetworkPolicyAmendments`. Clients can prefer `availableDecisions` when present to render the exact set of choices the server wants to expose, while still falling back to the older heuristics if it is omitted. +2. `item/commandExecution/requestApproval` (request) — carries the same `itemId`, `threadId`, `turnId`, the nullable `environmentId` where the command will run, optionally `approvalId`, and `reason`. `itemId` remains the stable command-item identity. Use the server-initiated JSON-RPC request ID for the response. When tracking approval UI or state outside that envelope, `approvalId`, when present, is the opaque identity of this callback and distinguishes it from other callbacks on the same `itemId`; compatibility flows may omit it. A follow-up callback may reuse an active item without another `item/started`; if the prior attempt already completed, the server can restart the same `itemId` before requesting retry approval. New shell and unified-exec approvals set `environmentId`; older events that do not provide one are exposed as `null`. For normal command approvals, the request also includes `command`, `cwd`, and `commandActions` for friendly display. When `initialize.params.capabilities.experimentalApi = true`, it may also include experimental `additionalPermissions` describing requested per-command sandbox access; any filesystem paths in that payload are absolute on the wire, and network access is represented as `additionalPermissions.network.enabled`. For network-only approvals, those command fields may be omitted and `networkApprovalContext` is provided instead. Optional persistence hints may also be included via `proposedExecpolicyAmendment` and `proposedNetworkPolicyAmendments`. Clients can prefer `availableDecisions` when present to render the exact set of choices the server wants to expose, while still falling back to the older heuristics if it is omitted. 3. Client response — for example `{ "decision": "accept" }`, `{ "decision": "acceptForSession" }`, `{ "decision": { "acceptWithExecpolicyAmendment": { "execpolicy_amendment": [...] } } }`, `{ "decision": { "applyNetworkPolicyAmendment": { "network_policy_amendment": { "host": "example.com", "action": "allow" } } } }`, `{ "decision": "decline" }`, or `{ "decision": "cancel" }`. 4. `serverRequest/resolved` — `{ threadId, requestId }` confirms the pending request has been resolved or cleared, including lifecycle cleanup on turn start/complete/interrupt. 5. `item/completed` — final `commandExecution` item with `status: "completed" | "failed" | "declined"` and execution output. Render this as the authoritative result. diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 52d5b3eb70..aab6f61ea9 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -6,6 +6,7 @@ use crate::request_processors::populate_thread_turns_from_history; use crate::request_processors::thread_from_stored_thread; use crate::request_processors::thread_settings_from_core_snapshot; use crate::server_request_error::is_turn_transition_server_request_error; +use crate::thread_state::CommandExecutionItemMetadata; use crate::thread_state::ThreadState; use crate::thread_state::TurnSummary; use crate::thread_state::resolve_server_request_on_thread_listener; @@ -82,6 +83,7 @@ use codex_app_server_protocol::TurnPlanUpdatedNotification; use codex_app_server_protocol::TurnStartedNotification; use codex_app_server_protocol::TurnStatus; use codex_app_server_protocol::WarningNotification; +use codex_app_server_protocol::build_command_execution_begin_item; use codex_app_server_protocol::build_item_from_guardian_event; use codex_app_server_protocol::guardian_auto_approval_review_notification; use codex_app_server_protocol::item_event_to_server_notification; @@ -128,12 +130,7 @@ enum CommandExecutionApprovalPresentation { Command(CommandExecutionCompletionItem), } -#[derive(Debug, PartialEq)] -struct CommandExecutionCompletionItem { - command: String, - cwd: LegacyAppPathString, - command_actions: Vec, -} +type CommandExecutionCompletionItem = CommandExecutionItemMetadata; #[allow(clippy::too_many_arguments)] pub(crate) async fn apply_bespoke_event_handling( @@ -305,17 +302,14 @@ pub(crate) async fn apply_bespoke_event_handling( | codex_protocol::protocol::GuardianAssessmentStatus::Approved => None, }; if let Some(completion_status) = completion_status - && let Some((target_item_id, completion_item)) = pending_command_execution + && let Some((target_item_id, _)) = pending_command_execution { complete_command_execution_item( &conversation_id, assessment_turn_id, target_item_id, - completion_item.command, - completion_item.cwd, /*process_id*/ None, CommandExecutionSource::Agent, - completion_item.command_actions, completion_status, &outgoing, &thread_state, @@ -1123,13 +1117,25 @@ pub(crate) async fn apply_bespoke_event_handling( return; } let item_id = exec_command_begin_event.call_id.clone(); - let first_start = { - let mut state = thread_state.lock().await; - state - .turn_summary - .command_execution_started - .insert(item_id.clone()) + let ThreadItem::CommandExecution { + command, + cwd, + command_actions, + .. + } = build_command_execution_begin_item(&exec_command_begin_event) + else { + unreachable!("command begin must build a command execution item") }; + let first_start = note_started_command_execution_item( + &thread_state, + item_id, + CommandExecutionCompletionItem { + command, + cwd, + command_actions, + }, + ) + .await; if first_start { let notification = item_event_to_server_notification( EventMsg::ExecCommandBegin(exec_command_begin_event), @@ -1149,7 +1155,9 @@ pub(crate) async fn apply_bespoke_event_handling( } EventMsg::ExecCommandEnd(exec_command_end_event) => { let call_id = exec_command_end_event.call_id.clone(); - let should_emit = remove_started_command_execution_item(&thread_state, &call_id).await; + let should_emit = remove_started_command_execution_item(&thread_state, &call_id) + .await + .is_some(); if matches!( exec_command_end_event.source, codex_protocol::protocol::ExecCommandSource::UnifiedExecInteraction @@ -1387,13 +1395,16 @@ async fn start_command_execution_item( outgoing: &ThreadScopedOutgoingMessageSender, thread_state: &Arc>, ) -> bool { - let first_start = { - let mut state = thread_state.lock().await; - state - .turn_summary - .command_execution_started - .insert(item_id.clone()) - }; + let first_start = note_started_command_execution_item( + thread_state, + item_id.clone(), + CommandExecutionCompletionItem { + command: command.clone(), + cwd: cwd.clone(), + command_actions: command_actions.clone(), + }, + ) + .await; if first_start { let notification = ItemStartedNotification { thread_id: conversation_id.to_string(), @@ -1424,28 +1435,25 @@ async fn complete_command_execution_item( conversation_id: &ThreadId, turn_id: String, item_id: String, - command: String, - cwd: LegacyAppPathString, process_id: Option, source: CommandExecutionSource, - command_actions: Vec, status: CommandExecutionStatus, outgoing: &ThreadScopedOutgoingMessageSender, thread_state: &Arc>, ) { - let should_emit = remove_started_command_execution_item(thread_state, &item_id).await; - if !should_emit { + let Some(completion_item) = remove_started_command_execution_item(thread_state, &item_id).await + else { return; - } + }; let item = ThreadItem::CommandExecution { id: item_id, - command, - cwd, + command: completion_item.command, + cwd: completion_item.cwd, process_id, source, status, - command_actions, + command_actions: completion_item.command_actions, aggregated_output: None, exit_code: None, duration_ms: None, @@ -1461,10 +1469,26 @@ async fn complete_command_execution_item( .await; } +async fn note_started_command_execution_item( + thread_state: &Arc>, + item_id: String, + completion_item: CommandExecutionCompletionItem, +) -> bool { + let mut state = thread_state.lock().await; + if let std::collections::hash_map::Entry::Vacant(entry) = + state.turn_summary.command_execution_started.entry(item_id) + { + entry.insert(completion_item); + true + } else { + false + } +} + async fn remove_started_command_execution_item( thread_state: &Arc>, item_id: &str, -) -> bool { +) -> Option { thread_state .lock() .await @@ -2163,17 +2187,14 @@ async fn on_command_execution_request_approval_response( if let Some(status) = completion_status && !suppress_repeated_prompt_completion_item - && let Some(completion_item) = completion_item + && completion_item.is_some() { complete_command_execution_item( &conversation_id, event_turn_id.clone(), item_id.clone(), - completion_item.command, - completion_item.cwd, /*process_id*/ None, CommandExecutionSource::Agent, - completion_item.command_actions, status, &outgoing, &thread_state, @@ -2224,7 +2245,6 @@ mod tests { use codex_app_server_protocol::AutoReviewDecisionSource; use codex_app_server_protocol::GuardianApprovalReviewStatus; use codex_app_server_protocol::JSONRPCErrorError; - use codex_app_server_protocol::ServerRequest; use codex_app_server_protocol::TurnPlanStepStatus; use codex_login::CodexAuth; use codex_protocol::AgentPath; @@ -2387,100 +2407,6 @@ mod tests { } } - struct CommandApprovalTestContext { - _codex_home: TempDir, - conversation_id: ThreadId, - conversation: Arc, - thread_manager: Arc, - outgoing: ThreadScopedOutgoingMessageSender, - raw_outgoing: Arc, - thread_state: Arc>, - thread_watch_manager: ThreadWatchManager, - rx: mpsc::Receiver, - } - - impl CommandApprovalTestContext { - async fn new() -> Result { - let codex_home = TempDir::new()?; - let config = load_default_config_for_test(&codex_home).await; - let thread_manager = Arc::new( - codex_core::test_support::thread_manager_with_models_provider_and_home( - CodexAuth::create_dummy_chatgpt_auth_for_testing(), - config.model_provider.clone(), - config.codex_home.to_path_buf(), - Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), - ), - ); - let codex_core::NewThread { - thread_id: conversation_id, - thread: conversation, - .. - } = thread_manager.start_thread(config).await?; - let (tx, rx) = mpsc::channel(CHANNEL_CAPACITY); - let raw_outgoing = Arc::new(OutgoingMessageSender::new( - tx, - codex_analytics::AnalyticsEventsClient::disabled(), - )); - let outgoing = ThreadScopedOutgoingMessageSender::new( - Arc::clone(&raw_outgoing), - vec![ConnectionId(1)], - conversation_id, - ); - Ok(Self { - _codex_home: codex_home, - conversation_id, - conversation, - thread_manager, - outgoing, - raw_outgoing, - thread_state: new_thread_state(), - thread_watch_manager: ThreadWatchManager::new(), - rx, - }) - } - - async fn apply(&self, event: Event) { - apply_bespoke_event_handling( - event, - self.conversation_id, - Arc::clone(&self.conversation), - Arc::clone(&self.thread_manager), - self.outgoing.clone(), - Arc::clone(&self.thread_state), - self.thread_watch_manager.clone(), - Arc::new(tokio::sync::Semaphore::new(/*permits*/ 1)), - "test-provider".to_string(), - ) - .await; - } - } - - fn exec_approval_event( - call_id: &str, - approval_id: Option<&str>, - reason: Option<&str>, - ) -> Event { - Event { - id: "turn-1".to_string(), - msg: EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { - call_id: call_id.to_string(), - approval_id: approval_id.map(str::to_string), - turn_id: "turn-1".to_string(), - environment_id: Some("local".to_string()), - started_at_ms: 1_000, - command: vec!["printf".to_string(), "hi".to_string()], - cwd: test_path_buf("/tmp").abs(), - reason: reason.map(str::to_string), - network_approval_context: None, - proposed_execpolicy_amendment: None, - proposed_network_policy_amendments: None, - additional_permissions: None, - available_decisions: Some(vec![ReviewDecision::Approved, ReviewDecision::Abort]), - parsed_cmd: Vec::new(), - }), - } - } - fn guardian_command_assessment( id: &str, turn_id: &str, @@ -2705,189 +2631,120 @@ mod tests { } #[tokio::test] - async fn repeated_approval_dispatch_preserves_item_and_callback_ids() -> Result<()> { - let mut context = CommandApprovalTestContext::new().await?; - context - .apply(exec_approval_event("cmd-1", None, Some("initial"))) - .await; - - let started = recv_broadcast_message(&mut context.rx).await?; - let OutgoingMessage::AppServerNotification(ServerNotification::ItemStarted(started)) = - started - else { - bail!("expected item/started"); - }; - assert!(matches!( - started.item, - ThreadItem::CommandExecution { ref id, .. } if id == "cmd-1" - )); - - let initial_request = recv_broadcast_message(&mut context.rx).await?; - let OutgoingMessage::Request(ServerRequest::CommandExecutionRequestApproval { - request_id, - params, - }) = initial_request - else { - bail!("expected initial command approval request"); - }; - assert_eq!(params.item_id, "cmd-1"); - assert_eq!(params.approval_id, None); - context - .raw_outgoing - .notify_client_response( - request_id, - serde_json::to_value(CommandExecutionRequestApprovalResponse { - decision: CommandExecutionApprovalDecision::Accept, - })?, - ) - .await; - - context - .apply(exec_approval_event( - "cmd-1", - Some("retry-1"), - Some("sandbox denied"), - )) - .await; - let retry_request = recv_broadcast_message(&mut context.rx).await?; - let OutgoingMessage::Request(ServerRequest::CommandExecutionRequestApproval { - request_id, - params, - }) = retry_request - else { - bail!("retry must reuse the active item without another item/started"); - }; - assert_eq!(params.item_id, "cmd-1"); - assert_eq!(params.approval_id.as_deref(), Some("retry-1")); - assert_eq!(params.reason.as_deref(), Some("sandbox denied")); - assert_eq!( - params.available_decisions, - Some(vec![ - CommandExecutionApprovalDecision::Accept, - CommandExecutionApprovalDecision::Cancel, - ]) + async fn repeated_start_retains_parent_metadata_for_completion() -> Result<()> { + let conversation_id = ThreadId::new(); + let thread_state = new_thread_state(); + let (tx, mut rx) = mpsc::channel(CHANNEL_CAPACITY); + let outgoing = ThreadScopedOutgoingMessageSender::new( + Arc::new(OutgoingMessageSender::new( + tx, + codex_analytics::AnalyticsEventsClient::disabled(), + )), + vec![ConnectionId(1)], + conversation_id, ); - context - .raw_outgoing - .notify_client_response( - request_id, - serde_json::to_value(CommandExecutionRequestApprovalResponse { - decision: CommandExecutionApprovalDecision::Decline, - })?, + let parent = command_execution_completion_item("parent-command"); + let child = CommandExecutionCompletionItem { + command: "child-command".to_string(), + cwd: test_path_buf("/child").abs().into(), + command_actions: vec![], + }; + + assert!( + start_command_execution_item( + &conversation_id, + "turn-1".to_string(), + "cmd-1".to_string(), + parent.command.clone(), + parent.cwd.clone(), + parent.command_actions.clone(), + CommandExecutionSource::Agent, + &outgoing, + &thread_state, ) - .await; - let completed = recv_broadcast_message(&mut context.rx).await?; - assert!(matches!( - completed, - OutgoingMessage::AppServerNotification(ServerNotification::ItemCompleted(payload)) - if matches!(payload.item, ThreadItem::CommandExecution { ref id, status: CommandExecutionStatus::Declined, .. } if id == "cmd-1") - )); + .await + ); + let _started = recv_broadcast_message(&mut rx).await?; + assert!( + !start_command_execution_item( + &conversation_id, + "turn-1".to_string(), + "cmd-1".to_string(), + child.command, + child.cwd, + child.command_actions, + CommandExecutionSource::Agent, + &outgoing, + &thread_state, + ) + .await + ); + complete_command_execution_item( + &conversation_id, + "turn-1".to_string(), + "cmd-1".to_string(), + /*process_id*/ None, + CommandExecutionSource::Agent, + CommandExecutionStatus::Declined, + &outgoing, + &thread_state, + ) + .await; + + let OutgoingMessage::AppServerNotification(ServerNotification::ItemCompleted(payload)) = + recv_broadcast_message(&mut rx).await? + else { + bail!("expected item/completed"); + }; + let ThreadItem::CommandExecution { + id, + command, + cwd, + command_actions, + status, + .. + } = payload.item + else { + bail!("expected command execution item"); + }; + assert_eq!( + (id, command, cwd, command_actions, status), + ( + "cmd-1".to_string(), + parent.command, + parent.cwd, + parent.command_actions, + CommandExecutionStatus::Declined, + ) + ); + assert!( + remove_started_command_execution_item(&thread_state, "cmd-1") + .await + .is_none() + ); Ok(()) } - #[tokio::test] - async fn repeated_approval_response_lifecycle_truth_table() -> Result<()> { - let mut context = CommandApprovalTestContext::new().await?; + #[test] + fn repeated_approval_response_lifecycle_truth_table() { let cases = [ - (false, true, CommandExecutionApprovalDecision::Decline, true), - (true, true, CommandExecutionApprovalDecision::Decline, true), - (true, true, CommandExecutionApprovalDecision::Cancel, true), - (true, true, CommandExecutionApprovalDecision::Accept, false), - ( - true, - false, - CommandExecutionApprovalDecision::Decline, - false, - ), - (true, false, CommandExecutionApprovalDecision::Cancel, true), + (None, false, false, false, false), + (Some("callback"), false, false, false, true), + (Some("callback"), false, false, true, false), + (Some("callback"), false, true, false, false), + (Some("callback"), true, false, false, false), ]; - - for (index, (has_active_item, is_sandbox_retry, decision, completes_on_response)) in - cases.into_iter().enumerate() - { - let item_id = format!("cmd-{index}"); - let completion_item = command_execution_completion_item("printf hi"); - if has_active_item { - assert!( - start_command_execution_item( - &context.conversation_id, - "turn-1".to_string(), - item_id.clone(), - completion_item.command.clone(), - completion_item.cwd.clone(), - completion_item.command_actions.clone(), - CommandExecutionSource::Agent, - &context.outgoing, - &context.thread_state, - ) - .await - ); - let _initial_started = recv_broadcast_message(&mut context.rx).await?; - } - let started_now = start_command_execution_item( - &context.conversation_id, - "turn-1".to_string(), - item_id.clone(), - completion_item.command.clone(), - completion_item.cwd.clone(), - completion_item.command_actions.clone(), - CommandExecutionSource::Agent, - &context.outgoing, - &context.thread_state, - ) - .await; - assert_eq!(started_now, !has_active_item); - if started_now { - let _callback_started = recv_broadcast_message(&mut context.rx).await?; - } - - let (response_tx, response_rx) = oneshot::channel(); - response_tx - .send(Ok(serde_json::to_value( - CommandExecutionRequestApprovalResponse { decision }, - )?)) - .expect("response receiver should remain open"); - let permission_guard = context - .thread_watch_manager - .note_permission_requested(&context.conversation_id.to_string()) - .await; - on_command_execution_request_approval_response( - "turn-1".to_string(), - context.conversation_id, - Some(format!("callback-{index}")), - item_id.clone(), - Some(completion_item), - started_now, - is_sandbox_retry, - RequestId::Integer(index as i64), - response_rx, - Arc::clone(&context.conversation), - context.outgoing.clone(), - Arc::clone(&context.thread_state), - permission_guard, - ) - .await; - - if completes_on_response { - let completed = recv_broadcast_message(&mut context.rx).await?; - assert!(matches!( - completed, - OutgoingMessage::AppServerNotification(ServerNotification::ItemCompleted(payload)) - if matches!(payload.item, ThreadItem::CommandExecution { ref id, status: CommandExecutionStatus::Declined, .. } if id == &item_id) - )); - assert!( - !remove_started_command_execution_item(&context.thread_state, &item_id).await, - "a late runtime end must not complete {item_id} twice" - ); - } else { - assert!(context.rx.try_recv().is_err()); - assert!( - remove_started_command_execution_item(&context.thread_state, &item_id).await, - "the active parent retains completion ownership for {item_id}" - ); - } + for (approval_id, started_now, is_sandbox_retry, cancel_requested, expected) in cases { + assert_eq!( + suppress_repeated_prompt_completion_item( + approval_id, + started_now, + is_sandbox_retry, + cancel_requested, + ), + expected + ); } - Ok(()) } #[tokio::test] diff --git a/codex-rs/app-server/src/thread_state.rs b/codex-rs/app-server/src/thread_state.rs index fec786373c..66d1290906 100644 --- a/codex-rs/app-server/src/thread_state.rs +++ b/codex-rs/app-server/src/thread_state.rs @@ -1,5 +1,6 @@ use crate::outgoing_message::ConnectionId; use crate::outgoing_message::ConnectionRequestId; +use codex_app_server_protocol::CommandAction; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::ThreadGoal; use codex_app_server_protocol::ThreadHistoryBuilder; @@ -29,6 +30,13 @@ use tracing::error; type PendingInterruptQueue = Vec; +#[derive(Clone, Debug, PartialEq)] +pub(crate) struct CommandExecutionItemMetadata { + pub(crate) command: String, + pub(crate) cwd: LegacyAppPathString, + pub(crate) command_actions: Vec, +} + pub(crate) struct PendingThreadResumeRequest { pub(crate) request_id: ConnectionRequestId, pub(crate) history_items: Vec, @@ -70,7 +78,7 @@ pub(crate) enum ThreadListenerCommand { #[derive(Default, Clone)] pub(crate) struct TurnSummary { pub(crate) started_at: Option, - pub(crate) command_execution_started: HashSet, + pub(crate) command_execution_started: HashMap, pub(crate) last_error: Option, } diff --git a/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs b/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs index c6f8f0e198..a70e61be09 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs @@ -640,6 +640,27 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2() assert_eq!(approved_subcommand_strings.len(), 2); assert!(approved_subcommand_strings[0].contains(&first_file.display().to_string())); assert!(approved_subcommand_strings[1].contains(&second_file.display().to_string())); + let parent_started_metadata = timeout(DEFAULT_READ_TIMEOUT, async { + loop { + let started_notif = mcp + .read_stream_until_notification_message("item/started") + .await?; + let started: ItemStartedNotification = + serde_json::from_value(started_notif.params.expect("item/started params"))?; + if let ThreadItem::CommandExecution { + id, + command, + cwd, + command_actions, + .. + } = started.item + && id == "call-zsh-fork-subcommand-decline" + { + return Ok::<_, anyhow::Error>((command, cwd, command_actions)); + } + } + }) + .await??; let parent_completed_command_execution = timeout(DEFAULT_READ_TIMEOUT, async { loop { let completed_notif = mcp @@ -664,6 +685,9 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2() Ok(Ok(parent_completed_command_execution)) => { let ThreadItem::CommandExecution { id, + command, + cwd, + command_actions, status, aggregated_output, .. @@ -673,6 +697,11 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2() }; assert_eq!(id, "call-zsh-fork-subcommand-decline"); assert_eq!(status, CommandExecutionStatus::Declined); + assert_eq!( + (command, cwd, command_actions), + parent_started_metadata, + "parent completion metadata was replaced by a child callback" + ); if let Some(output) = aggregated_output.as_deref() { assert!( output == "exec command rejected by user" diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index be4ec3f359..69719710c3 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -2131,9 +2131,9 @@ impl Session { /// cleared before a response arrives, treat it as an abort so interrupted /// turns do not continue on a synthetic denial. /// - /// Note that if `available_decisions` is `None`, then the other fields will - /// be used to derive the available decisions via - /// [ExecApprovalRequestEvent::default_available_decisions]. + /// If `available_decisions` is `None`, callback-scoped requests with an + /// `approval_id` offer only approve or abort. Other requests derive their + /// decisions via [ExecApprovalRequestEvent::default_available_decisions]. #[allow(clippy::too_many_arguments)] #[expect( clippy::await_holding_invalid_type, @@ -2169,12 +2169,16 @@ impl Session { ] }); let available_decisions = available_decisions.unwrap_or_else(|| { - ExecApprovalRequestEvent::default_available_decisions( - network_approval_context.as_ref(), - proposed_execpolicy_amendment.as_ref(), - proposed_network_policy_amendments.as_deref(), - additional_permissions.as_ref(), - ) + if approval_id.is_some() { + vec![ReviewDecision::Approved, ReviewDecision::Abort] + } else { + ExecApprovalRequestEvent::default_available_decisions( + network_approval_context.as_ref(), + proposed_execpolicy_amendment.as_ref(), + proposed_network_policy_amendments.as_deref(), + additional_permissions.as_ref(), + ) + } }); let accepted_decisions = accepted_command_approval_decisions( available_decisions.clone(), diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 2b7c441f8a..9bcc262891 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -793,6 +793,91 @@ fn accepted_command_decisions_add_only_exact_proposed_network_denials() { })); } +#[tokio::test] +async fn callback_without_explicit_decisions_is_one_shot() { + let (session, turn_context, rx_events) = make_session_and_context_with_rx().await; + *session.active_turn.lock().await = Some(ActiveTurn::default()); + let request = tokio::spawn({ + let session = Arc::clone(&session); + let turn_context = Arc::clone(&turn_context); + async move { + session + .request_command_approval( + turn_context.as_ref(), + "command-item".to_string(), + Some("callback".to_string()), + /*environment_id*/ None, + vec!["echo".to_string(), "hi".to_string()], + test_path_buf("/tmp").abs(), + /*reason*/ None, + /*network_approval_context*/ None, + /*proposed_execpolicy_amendment*/ None, + /*additional_permissions*/ None, + /*available_decisions*/ None, + ) + .await + } + }); + let event = tokio::time::timeout(std::time::Duration::from_secs(1), rx_events.recv()) + .await + .expect("approval event timed out") + .expect("approval event missing"); + let EventMsg::ExecApprovalRequest(event) = event.msg else { + panic!("expected exec approval event"); + }; + assert_eq!( + event.available_decisions, + Some(vec![ReviewDecision::Approved, ReviewDecision::Abort]) + ); + session + .notify_exec_approval("callback", ReviewDecision::Abort) + .await; + assert_eq!( + request.await.expect("approval request task"), + ReviewDecision::Abort + ); + + let explicit_request = tokio::spawn({ + let session = Arc::clone(&session); + let turn_context = Arc::clone(&turn_context); + async move { + session + .request_command_approval( + turn_context.as_ref(), + "command-item".to_string(), + Some("explicit-callback".to_string()), + /*environment_id*/ None, + vec!["echo".to_string(), "hi".to_string()], + test_path_buf("/tmp").abs(), + /*reason*/ None, + /*network_approval_context*/ None, + /*proposed_execpolicy_amendment*/ None, + /*additional_permissions*/ None, + Some(vec![ReviewDecision::ApprovedForSession]), + ) + .await + } + }); + let event = tokio::time::timeout(std::time::Duration::from_secs(1), rx_events.recv()) + .await + .expect("explicit approval event timed out") + .expect("explicit approval event missing"); + let EventMsg::ExecApprovalRequest(event) = event.msg else { + panic!("expected exec approval event"); + }; + assert_eq!( + event.available_decisions, + Some(vec![ReviewDecision::ApprovedForSession]) + ); + session + .notify_exec_approval("explicit-callback", ReviewDecision::ApprovedForSession) + .await; + assert_eq!( + explicit_request.await.expect("explicit approval task"), + ReviewDecision::ApprovedForSession + ); +} + #[derive(Clone, Copy, Debug)] enum DuplicateApprovalRequestKind { Exec, diff --git a/codex-rs/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index a1fff871b5..fc7f9a5a9d 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -220,8 +220,8 @@ pub struct ExecApprovalRequestEvent { pub call_id: String, /// Identifier for this specific approval callback. /// - /// When absent, the approval is for the command item itself (`call_id`). - /// This is present for subcommand approvals (via execve intercept). + /// When present, responses must use this ID instead of the command-item + /// `call_id`. Absence preserves legacy/cacheable approval routing. #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] pub approval_id: Option, @@ -286,6 +286,9 @@ impl ExecApprovalRequestEvent { // senders, so we fall back to the legacy logic if it's not present. match &self.available_decisions { Some(decisions) => decisions.clone(), + None if self.approval_id.is_some() => { + vec![ReviewDecision::Approved, ReviewDecision::Abort] + } None => Self::default_available_decisions( self.network_approval_context.as_ref(), self.proposed_execpolicy_amendment.as_ref(), @@ -472,6 +475,32 @@ mod tests { ); } + #[test] + fn callback_without_explicit_decisions_is_one_shot() { + let value = serde_json::json!({ + "call_id": "command-item", + "approval_id": "callback", + "turn_id": "turn", + "started_at_ms": 0, + "command": ["echo", "hi"], + "cwd": test_path_buf("/tmp"), + "reason": null, + "parsed_cmd": [], + }); + let mut event: ExecApprovalRequestEvent = + serde_json::from_value(value).expect("callback event"); + assert_eq!( + event.effective_available_decisions(), + vec![ReviewDecision::Approved, ReviewDecision::Abort] + ); + + event.available_decisions = Some(vec![ReviewDecision::ApprovedForSession]); + assert_eq!( + event.effective_available_decisions(), + vec![ReviewDecision::ApprovedForSession] + ); + } + #[test] fn guardian_assessment_action_deserializes_command_shape() { let action: GuardianAssessmentAction = serde_json::from_value(serde_json::json!({ diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 9684285f39..fb98c57bc5 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -303,6 +303,7 @@ fn collab_receiver_is_not_found( } fn default_exec_approval_decisions( + approval_id: Option<&str>, network_approval_context: Option<&codex_app_server_protocol::NetworkApprovalContext>, proposed_execpolicy_amendment: Option<&codex_app_server_protocol::ExecPolicyAmendment>, proposed_network_policy_amendments: Option< @@ -313,6 +314,13 @@ fn default_exec_approval_decisions( use codex_app_server_protocol::CommandExecutionApprovalDecision; use codex_app_server_protocol::NetworkPolicyRuleAction; + if approval_id.is_some() { + return vec![ + CommandExecutionApprovalDecision::Accept, + CommandExecutionApprovalDecision::Cancel, + ]; + } + if network_approval_context.is_some() { let mut decisions = vec![ CommandExecutionApprovalDecision::Accept, diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index d8d63575bb..35d611ca6d 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -2524,6 +2524,49 @@ async fn inactive_thread_exec_approval_preserves_context() { ); } +#[tokio::test] +async fn callback_exec_approval_falls_back_to_one_shot_decisions() -> std::io::Result<()> { + use codex_app_server_protocol::CommandExecutionApprovalDecision as Decision; + let app = make_test_app().await; + let thread_id = ThreadId::new(); + let request = exec_approval_request( + thread_id, + "turn-approval", + "call-approval", + Some("callback"), + ); + let Some(ThreadInteractiveRequest::Approval(ApprovalRequest::Exec { + available_decisions, + .. + })) = app + .interactive_request_for_thread_request(thread_id, &request) + .await? + else { + panic!("expected exec approval request"); + }; + assert_eq!( + available_decisions, + vec![Decision::Accept, Decision::Cancel] + ); + + let mut request = request; + let ServerRequest::CommandExecutionRequestApproval { params, .. } = &mut request else { + unreachable!(); + }; + params.available_decisions = Some(vec![Decision::AcceptForSession]); + let Some(ThreadInteractiveRequest::Approval(ApprovalRequest::Exec { + available_decisions, + .. + })) = app + .interactive_request_for_thread_request(thread_id, &request) + .await? + else { + panic!("expected exec approval request"); + }; + assert_eq!(available_decisions, vec![Decision::AcceptForSession]); + Ok(()) +} + #[tokio::test] async fn inactive_thread_exec_approval_splits_shell_wrapped_command() { let app = make_test_app().await; diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index b149fd53f3..40e5e02bb5 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -241,6 +241,7 @@ impl App { reason: params.reason.clone(), available_decisions: params.available_decisions.clone().unwrap_or_else(|| { default_exec_approval_decisions( + params.approval_id.as_deref(), network_approval_context.as_ref(), proposed_execpolicy_amendment.as_ref(), proposed_network_policy_amendments.as_deref(), diff --git a/codex-rs/tui/src/approval_events.rs b/codex-rs/tui/src/approval_events.rs index bd61a7cd51..f56d86ea3d 100644 --- a/codex-rs/tui/src/approval_events.rs +++ b/codex-rs/tui/src/approval_events.rs @@ -53,6 +53,10 @@ impl ExecApprovalRequestEvent { pub(crate) fn effective_available_decisions(&self) -> Vec { match &self.available_decisions { Some(decisions) => decisions.clone(), + None if self.approval_id.is_some() => vec![ + CommandExecutionApprovalDecision::Accept, + CommandExecutionApprovalDecision::Cancel, + ], None => Self::default_available_decisions( self.network_approval_context.as_ref(), self.proposed_execpolicy_amendment.as_ref(), diff --git a/codex-rs/tui/src/chatwidget/tests/approval_requests.rs b/codex-rs/tui/src/chatwidget/tests/approval_requests.rs index 1ec9a9c381..29b2cb80b5 100644 --- a/codex-rs/tui/src/chatwidget/tests/approval_requests.rs +++ b/codex-rs/tui/src/chatwidget/tests/approval_requests.rs @@ -11,7 +11,7 @@ async fn exec_approval_emits_proposed_command_and_decision_history() { // Trigger an exec approval request with a short, single-line command. let ev = ExecApprovalRequestEvent { call_id: "call-short".into(), - approval_id: Some("call-short".into()), + approval_id: None, turn_id: "turn-short".into(), environment_id: Some("remote".to_string()), command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], @@ -377,7 +377,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { let ev_multi = ExecApprovalRequestEvent { call_id: "call-multi".into(), - approval_id: Some("call-multi".into()), + approval_id: None, turn_id: "turn-multi".into(), environment_id: None, command: vec!["bash".into(), "-lc".into(), "echo line1\necho line2".into()], @@ -429,7 +429,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { let long = format!("echo {}", "a".repeat(200)); let ev_long = ExecApprovalRequestEvent { call_id: "call-long".into(), - approval_id: Some("call-long".into()), + approval_id: None, turn_id: "turn-long".into(), environment_id: None, command: vec!["bash".into(), "-lc".into(), long], diff --git a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs index 5a6e51fb60..f4e5212173 100644 --- a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs +++ b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs @@ -8,7 +8,7 @@ async fn exec_approval_emits_proposed_command_and_decision_history() { // Trigger an exec approval request with a short, single-line command let ev = ExecApprovalRequestEvent { call_id: "call-short".into(), - approval_id: Some("call-short".into()), + approval_id: None, turn_id: "turn-short".into(), environment_id: None, command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], @@ -88,26 +88,30 @@ fn app_server_exec_approval_request_splits_shell_wrapped_command() { async fn exec_approval_uses_approval_id_when_present() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; - handle_exec_approval_request( - &mut chat, - "sub-short", - ExecApprovalRequestEvent { - call_id: "call-parent".into(), - approval_id: Some("approval-subcommand".into()), - turn_id: "turn-short".into(), - environment_id: None, - command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], - cwd: AbsolutePathBuf::current_dir().expect("current dir"), - reason: Some( - "this is a test reason such as one that would be produced by the model".into(), - ), - network_approval_context: None, - proposed_execpolicy_amendment: None, - proposed_network_policy_amendments: None, - additional_permissions: None, - available_decisions: None, - }, + let event = ExecApprovalRequestEvent { + call_id: "call-parent".into(), + approval_id: Some("approval-subcommand".into()), + turn_id: "turn-short".into(), + environment_id: None, + command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], + cwd: AbsolutePathBuf::current_dir().expect("current dir"), + reason: Some( + "this is a test reason such as one that would be produced by the model".into(), + ), + network_approval_context: None, + proposed_execpolicy_amendment: None, + proposed_network_policy_amendments: None, + additional_permissions: None, + available_decisions: None, + }; + assert_eq!( + event.effective_available_decisions(), + vec![ + codex_app_server_protocol::CommandExecutionApprovalDecision::Accept, + codex_app_server_protocol::CommandExecutionApprovalDecision::Cancel, + ] ); + handle_exec_approval_request(&mut chat, "sub-short", event); chat.handle_key_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE)); @@ -137,7 +141,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { // Multiline command: modal should show full command, history records decision only let ev_multi = ExecApprovalRequestEvent { call_id: "call-multi".into(), - approval_id: Some("call-multi".into()), + approval_id: None, turn_id: "turn-multi".into(), environment_id: None, command: vec!["bash".into(), "-lc".into(), "echo line1\necho line2".into()], @@ -191,7 +195,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { let long = format!("echo {}", "a".repeat(200)); let ev_long = ExecApprovalRequestEvent { call_id: "call-long".into(), - approval_id: Some("call-long".into()), + approval_id: None, turn_id: "turn-long".into(), environment_id: None, command: vec!["bash".into(), "-lc".into(), long], @@ -1116,7 +1120,7 @@ async fn approval_modal_exec_snapshot() -> anyhow::Result<()> { // Inject an exec approval request to display the approval modal. let ev = ExecApprovalRequestEvent { call_id: "call-approve-cmd".into(), - approval_id: Some("call-approve-cmd".into()), + approval_id: None, turn_id: "turn-approve-cmd".into(), environment_id: None, command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], @@ -1174,7 +1178,7 @@ async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> { let ev = ExecApprovalRequestEvent { call_id: "call-approve-cmd-noreason".into(), - approval_id: Some("call-approve-cmd-noreason".into()), + approval_id: None, turn_id: "turn-approve-cmd-noreason".into(), environment_id: None, command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], @@ -1221,7 +1225,7 @@ async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot() let command = vec!["bash".into(), "-lc".into(), script]; let ev = ExecApprovalRequestEvent { call_id: "call-approve-cmd-multiline-trunc".into(), - approval_id: Some("call-approve-cmd-multiline-trunc".into()), + approval_id: None, turn_id: "turn-approve-cmd-multiline-trunc".into(), environment_id: None, command: command.clone(), diff --git a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs index 156348d42c..bd11c703a0 100644 --- a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs +++ b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs @@ -2044,7 +2044,7 @@ async fn status_widget_and_approval_modal_snapshot() { // Now show an approval modal (e.g. exec approval). let ev = ExecApprovalRequestEvent { call_id: "call-approve-exec".into(), - approval_id: Some("call-approve-exec".into()), + approval_id: None, turn_id: "turn-approve-exec".into(), environment_id: None, command: vec!["echo".into(), "hello world".into()],