From 14116ade8d8eb8066cb84b65b63f640a3cdd72e7 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 25 Feb 2026 17:10:46 -0800 Subject: [PATCH] feat: include available decisions in command approval requests (#12758) Command-approval clients currently infer which choices to show from side-channel fields like `networkApprovalContext`, `proposedExecpolicyAmendment`, and `additionalPermissions`. That makes the request shape harder to evolve, and it forces each client to replicate the server's heuristics instead of receiving the exact decision list for the prompt. This PR introduces a mapping between `CommandExecutionApprovalDecision` and `codex_protocol::protocol::ReviewDecision`: ```rust impl From for CommandExecutionApprovalDecision { fn from(value: CoreReviewDecision) -> Self { match value { CoreReviewDecision::Approved => Self::Accept, CoreReviewDecision::ApprovedExecpolicyAmendment { proposed_execpolicy_amendment, } => Self::AcceptWithExecpolicyAmendment { execpolicy_amendment: proposed_execpolicy_amendment.into(), }, CoreReviewDecision::ApprovedForSession => Self::AcceptForSession, CoreReviewDecision::NetworkPolicyAmendment { network_policy_amendment, } => Self::ApplyNetworkPolicyAmendment { network_policy_amendment: network_policy_amendment.into(), }, CoreReviewDecision::Abort => Self::Cancel, CoreReviewDecision::Denied => Self::Decline, } } } ``` And updates `CommandExecutionRequestApprovalParams` to have a new field: ```rust available_decisions: Option> ``` when, if specified, should make it easier for clients to display an appropriate list of options in the UI. This makes it possible for `CoreShellActionProvider::prompt()` in `unix_escalation.rs` to specify the `Vec` directly, adding support for `ApprovedForSession` when approving a skill script, which was previously missing in the TUI. Note this results in a significant change to `exec_options()` in `approval_overlay.rs`, as the displayed options are now derived from `available_decisions: &[ReviewDecision]`. ## What Changed - Add `available_decisions` to [`ExecApprovalRequestEvent`](https://github.com/openai/codex/blob/de00e932dd9801de0a4faac0519162099753f331/codex-rs/protocol/src/approvals.rs#L111-L175), including helpers to derive the legacy default choices when older senders omit the field. - Map `codex_protocol::protocol::ReviewDecision` to app-server `CommandExecutionApprovalDecision` and expose the ordered list as experimental `availableDecisions` in [`CommandExecutionRequestApprovalParams`](https://github.com/openai/codex/blob/de00e932dd9801de0a4faac0519162099753f331/codex-rs/app-server-protocol/src/protocol/v2.rs#L3798-L3807). - Thread optional `available_decisions` through the core approval path so Unix shell escalation can explicitly request `ApprovedForSession` for session-scoped approvals instead of relying on client heuristics. [`unix_escalation.rs`](https://github.com/openai/codex/blob/de00e932dd9801de0a4faac0519162099753f331/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs#L194-L214) - Update the TUI approval overlay to build its buttons from the ordered decision list, while preserving the legacy fallback when `available_decisions` is missing. - Update the app-server README, test client output, and generated schema artifacts to document and surface the new field. ## Testing - Add `approval_overlay.rs` coverage for explicit decision lists, including the generic `ApprovedForSession` path and network approval options. - Update `chatwidget/tests.rs` and app-server protocol tests to populate the new optional field and keep older event shapes working. ## Developers Docs - If we document `item/commandExecution/requestApproval` on [developers.openai.com/codex](https://developers.openai.com/codex), add experimental `availableDecisions` as the preferred source of approval choices and note that older servers may omit it. --- .../json/ApplyPatchApprovalResponse.json | 2 +- ...CommandExecutionRequestApprovalParams.json | 79 +++ ...mmandExecutionRequestApprovalResponse.json | 2 +- .../schema/json/EventMsg.json | 100 ++++ .../json/ExecCommandApprovalResponse.json | 2 +- .../schema/json/ServerRequest.json | 79 +++ .../codex_app_server_protocol.schemas.json | 14 +- .../typescript/ExecApprovalRequestEvent.ts | 10 +- .../CommandExecutionRequestApprovalParams.ts | 7 +- .../src/protocol/common.rs | 1 + .../app-server-protocol/src/protocol/v2.rs | 30 +- codex-rs/app-server-test-client/src/lib.rs | 4 + codex-rs/app-server/README.md | 2 +- .../app-server/src/bespoke_event_handling.rs | 6 + codex-rs/app-server/src/transport.rs | 2 + codex-rs/core/src/codex.rs | 20 +- codex-rs/core/src/codex_delegate.rs | 2 + codex-rs/core/src/tools/network_approval.rs | 2 + codex-rs/core/src/tools/runtimes/shell.rs | 2 + .../tools/runtimes/shell/unix_escalation.rs | 17 + .../core/src/tools/runtimes/unified_exec.rs | 2 + codex-rs/core/tests/suite/skill_approval.rs | 18 +- codex-rs/mcp-server/src/codex_tool_runner.rs | 1 + codex-rs/protocol/src/approvals.rs | 57 ++ codex-rs/protocol/src/protocol.rs | 4 +- codex-rs/tui/src/app.rs | 1 + .../tui/src/app/pending_interactive_replay.rs | 3 + .../tui/src/bottom_pane/approval_overlay.rs | 494 +++++++++--------- codex-rs/tui/src/bottom_pane/mod.rs | 6 +- codex-rs/tui/src/chatwidget.rs | 4 +- codex-rs/tui/src/chatwidget/tests.rs | 8 + 31 files changed, 695 insertions(+), 286 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/ApplyPatchApprovalResponse.json b/codex-rs/app-server-protocol/schema/json/ApplyPatchApprovalResponse.json index 37380f598..84842fd19 100644 --- a/codex-rs/app-server-protocol/schema/json/ApplyPatchApprovalResponse.json +++ b/codex-rs/app-server-protocol/schema/json/ApplyPatchApprovalResponse.json @@ -59,7 +59,7 @@ "type": "object" }, { - "description": "User has approved this command and wants to automatically approve any future identical instances (`command` and `cwd` match exactly) for the remainder of the session.", + "description": "User has approved this request and wants future prompts in the same session-scoped approval cache to be automatically approved for the remainder of the session.", "enum": [ "approved_for_session" ], diff --git a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json index 2122ad652..4e2106bbb 100644 --- a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json @@ -202,6 +202,85 @@ } ] }, + "CommandExecutionApprovalDecision": { + "oneOf": [ + { + "description": "User approved the command.", + "enum": [ + "accept" + ], + "type": "string" + }, + { + "description": "User approved the command and future prompts in the same session-scoped approval cache should run without prompting.", + "enum": [ + "acceptForSession" + ], + "type": "string" + }, + { + "additionalProperties": false, + "description": "User approved the command, and wants to apply the proposed execpolicy amendment so future matching commands can run without prompting.", + "properties": { + "acceptWithExecpolicyAmendment": { + "properties": { + "execpolicy_amendment": { + "items": { + "type": "string" + }, + "type": "array" + } + }, + "required": [ + "execpolicy_amendment" + ], + "type": "object" + } + }, + "required": [ + "acceptWithExecpolicyAmendment" + ], + "title": "AcceptWithExecpolicyAmendmentCommandExecutionApprovalDecision", + "type": "object" + }, + { + "additionalProperties": false, + "description": "User chose a persistent network policy rule (allow/deny) for this host.", + "properties": { + "applyNetworkPolicyAmendment": { + "properties": { + "network_policy_amendment": { + "$ref": "#/definitions/NetworkPolicyAmendment" + } + }, + "required": [ + "network_policy_amendment" + ], + "type": "object" + } + }, + "required": [ + "applyNetworkPolicyAmendment" + ], + "title": "ApplyNetworkPolicyAmendmentCommandExecutionApprovalDecision", + "type": "object" + }, + { + "description": "User denied the command. The agent will continue the turn.", + "enum": [ + "decline" + ], + "type": "string" + }, + { + "description": "User denied the command. The turn will also be immediately interrupted.", + "enum": [ + "cancel" + ], + "type": "string" + } + ] + }, "MacOsAutomationValue": { "anyOf": [ { diff --git a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalResponse.json b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalResponse.json index 04e914a39..0b7986fba 100644 --- a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalResponse.json +++ b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalResponse.json @@ -11,7 +11,7 @@ "type": "string" }, { - "description": "User approved the command and future identical commands should run without prompting.", + "description": "User approved the command and future prompts in the same session-scoped approval cache should run without prompting.", "enum": [ "acceptForSession" ], diff --git a/codex-rs/app-server-protocol/schema/json/EventMsg.json b/codex-rs/app-server-protocol/schema/json/EventMsg.json index 51064047e..c7bf90874 100644 --- a/codex-rs/app-server-protocol/schema/json/EventMsg.json +++ b/codex-rs/app-server-protocol/schema/json/EventMsg.json @@ -1675,6 +1675,16 @@ "null" ] }, + "available_decisions": { + "description": "Ordered list of decisions the client may present for this prompt.\n\nWhen absent, clients should derive the legacy default set from the other fields on this request.", + "items": { + "$ref": "#/definitions/ReviewDecision" + }, + "type": [ + "array", + "null" + ] + }, "call_id": { "description": "Identifier for the associated command execution item.", "type": "string" @@ -4971,6 +4981,86 @@ ], "type": "object" }, + "ReviewDecision": { + "description": "User's decision in response to an ExecApprovalRequest.", + "oneOf": [ + { + "description": "User has approved this command and the agent should execute it.", + "enum": [ + "approved" + ], + "type": "string" + }, + { + "additionalProperties": false, + "description": "User has approved this command and wants to apply the proposed execpolicy amendment so future matching commands are permitted.", + "properties": { + "approved_execpolicy_amendment": { + "properties": { + "proposed_execpolicy_amendment": { + "items": { + "type": "string" + }, + "type": "array" + } + }, + "required": [ + "proposed_execpolicy_amendment" + ], + "type": "object" + } + }, + "required": [ + "approved_execpolicy_amendment" + ], + "title": "ApprovedExecpolicyAmendmentReviewDecision", + "type": "object" + }, + { + "description": "User has approved this request and wants future prompts in the same session-scoped approval cache to be automatically approved for the remainder of the session.", + "enum": [ + "approved_for_session" + ], + "type": "string" + }, + { + "additionalProperties": false, + "description": "User chose to persist a network policy rule (allow/deny) for future requests to the same host.", + "properties": { + "network_policy_amendment": { + "properties": { + "network_policy_amendment": { + "$ref": "#/definitions/NetworkPolicyAmendment" + } + }, + "required": [ + "network_policy_amendment" + ], + "type": "object" + } + }, + "required": [ + "network_policy_amendment" + ], + "title": "NetworkPolicyAmendmentReviewDecision", + "type": "object" + }, + { + "description": "User has denied this command and the agent should not execute it, but it should continue the session and try something else.", + "enum": [ + "denied" + ], + "type": "string" + }, + { + "description": "User has denied this command and the agent should not do anything until the user's next command.", + "enum": [ + "abort" + ], + "type": "string" + } + ] + }, "ReviewFinding": { "description": "A single review finding describing an observed issue or recommendation.", "properties": { @@ -7141,6 +7231,16 @@ "null" ] }, + "available_decisions": { + "description": "Ordered list of decisions the client may present for this prompt.\n\nWhen absent, clients should derive the legacy default set from the other fields on this request.", + "items": { + "$ref": "#/definitions/ReviewDecision" + }, + "type": [ + "array", + "null" + ] + }, "call_id": { "description": "Identifier for the associated command execution item.", "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalResponse.json b/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalResponse.json index ac15f2365..baedafa40 100644 --- a/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalResponse.json +++ b/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalResponse.json @@ -59,7 +59,7 @@ "type": "object" }, { - "description": "User has approved this command and wants to automatically approve any future identical instances (`command` and `cwd` match exactly) for the remainder of the session.", + "description": "User has approved this request and wants future prompts in the same session-scoped approval cache to be automatically approved for the remainder of the session.", "enum": [ "approved_for_session" ], diff --git a/codex-rs/app-server-protocol/schema/json/ServerRequest.json b/codex-rs/app-server-protocol/schema/json/ServerRequest.json index e9cbc2ac1..ff810643d 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ServerRequest.json @@ -268,6 +268,85 @@ } ] }, + "CommandExecutionApprovalDecision": { + "oneOf": [ + { + "description": "User approved the command.", + "enum": [ + "accept" + ], + "type": "string" + }, + { + "description": "User approved the command and future prompts in the same session-scoped approval cache should run without prompting.", + "enum": [ + "acceptForSession" + ], + "type": "string" + }, + { + "additionalProperties": false, + "description": "User approved the command, and wants to apply the proposed execpolicy amendment so future matching commands can run without prompting.", + "properties": { + "acceptWithExecpolicyAmendment": { + "properties": { + "execpolicy_amendment": { + "items": { + "type": "string" + }, + "type": "array" + } + }, + "required": [ + "execpolicy_amendment" + ], + "type": "object" + } + }, + "required": [ + "acceptWithExecpolicyAmendment" + ], + "title": "AcceptWithExecpolicyAmendmentCommandExecutionApprovalDecision", + "type": "object" + }, + { + "additionalProperties": false, + "description": "User chose a persistent network policy rule (allow/deny) for this host.", + "properties": { + "applyNetworkPolicyAmendment": { + "properties": { + "network_policy_amendment": { + "$ref": "#/definitions/NetworkPolicyAmendment" + } + }, + "required": [ + "network_policy_amendment" + ], + "type": "object" + } + }, + "required": [ + "applyNetworkPolicyAmendment" + ], + "title": "ApplyNetworkPolicyAmendmentCommandExecutionApprovalDecision", + "type": "object" + }, + { + "description": "User denied the command. The agent will continue the turn.", + "enum": [ + "decline" + ], + "type": "string" + }, + { + "description": "User denied the command. The turn will also be immediately interrupted.", + "enum": [ + "cancel" + ], + "type": "string" + } + ] + }, "CommandExecutionRequestApprovalParams": { "properties": { "approvalId": { 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 52a33c4c0..62850442f 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 @@ -1442,7 +1442,7 @@ "type": "string" }, { - "description": "User approved the command and future identical commands should run without prompting.", + "description": "User approved the command and future prompts in the same session-scoped approval cache should run without prompting.", "enum": [ "acceptForSession" ], @@ -2843,6 +2843,16 @@ "null" ] }, + "available_decisions": { + "description": "Ordered list of decisions the client may present for this prompt.\n\nWhen absent, clients should derive the legacy default set from the other fields on this request.", + "items": { + "$ref": "#/definitions/ReviewDecision" + }, + "type": [ + "array", + "null" + ] + }, "call_id": { "description": "Identifier for the associated command execution item.", "type": "string" @@ -5676,7 +5686,7 @@ "type": "object" }, { - "description": "User has approved this command and wants to automatically approve any future identical instances (`command` and `cwd` match exactly) for the remainder of the session.", + "description": "User has approved this request and wants future prompts in the same session-scoped approval cache to be automatically approved for the remainder of the session.", "enum": [ "approved_for_session" ], diff --git a/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts b/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts index 5144dab6e..cc1340090 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts @@ -6,6 +6,7 @@ import type { NetworkApprovalContext } from "./NetworkApprovalContext"; import type { NetworkPolicyAmendment } from "./NetworkPolicyAmendment"; import type { ParsedCommand } from "./ParsedCommand"; import type { PermissionProfile } from "./PermissionProfile"; +import type { ReviewDecision } from "./ReviewDecision"; export type ExecApprovalRequestEvent = { /** @@ -51,4 +52,11 @@ proposed_network_policy_amendments?: Array, /** * Optional additional filesystem permissions requested for this command. */ -additional_permissions?: PermissionProfile, parsed_cmd: Array, }; +additional_permissions?: PermissionProfile, +/** + * Ordered list of decisions the client may present for this prompt. + * + * When absent, clients should derive the legacy default set from the + * other fields on this request. + */ +available_decisions?: Array, parsed_cmd: Array, }; 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 70cb8e152..8fb6375e6 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts @@ -3,6 +3,7 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { AdditionalPermissionProfile } from "./AdditionalPermissionProfile"; import type { CommandAction } from "./CommandAction"; +import type { CommandExecutionApprovalDecision } from "./CommandExecutionApprovalDecision"; import type { ExecPolicyAmendment } from "./ExecPolicyAmendment"; import type { NetworkApprovalContext } from "./NetworkApprovalContext"; import type { NetworkPolicyAmendment } from "./NetworkPolicyAmendment"; @@ -49,4 +50,8 @@ proposedExecpolicyAmendment?: ExecPolicyAmendment | null, /** * Optional proposed network policy amendments (allow/deny host) for future requests. */ -proposedNetworkPolicyAmendments?: Array | null, }; +proposedNetworkPolicyAmendments?: Array | null, +/** + * Ordered list of decisions the client may present for this prompt. + */ +availableDecisions?: Array | null, }; diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index df969e26c..3af297610 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -1540,6 +1540,7 @@ mod tests { }), proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, + available_decisions: None, }; let reason = crate::experimental_api::ExperimentalApi::experimental_reason(¶ms); assert_eq!( diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 62bb0d899..6dba059eb 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -49,6 +49,7 @@ use codex_protocol::protocol::RateLimitWindow as CoreRateLimitWindow; use codex_protocol::protocol::ReadOnlyAccess as CoreReadOnlyAccess; use codex_protocol::protocol::RealtimeAudioFrame as CoreRealtimeAudioFrame; use codex_protocol::protocol::RejectConfig as CoreRejectConfig; +use codex_protocol::protocol::ReviewDecision as CoreReviewDecision; use codex_protocol::protocol::SessionSource as CoreSessionSource; use codex_protocol::protocol::SkillDependencies as CoreSkillDependencies; use codex_protocol::protocol::SkillErrorInfo as CoreSkillErrorInfo; @@ -741,7 +742,8 @@ pub struct ConfigEdit { pub enum CommandExecutionApprovalDecision { /// User approved the command. Accept, - /// User approved the command and future identical commands should run without prompting. + /// User approved the command and future prompts in the same session-scoped + /// approval cache should run without prompting. AcceptForSession, /// User approved the command, and wants to apply the proposed execpolicy amendment so future /// matching commands can run without prompting. @@ -758,6 +760,27 @@ pub enum CommandExecutionApprovalDecision { Cancel, } +impl From for CommandExecutionApprovalDecision { + fn from(value: CoreReviewDecision) -> Self { + match value { + CoreReviewDecision::Approved => Self::Accept, + CoreReviewDecision::ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment, + } => Self::AcceptWithExecpolicyAmendment { + execpolicy_amendment: proposed_execpolicy_amendment.into(), + }, + CoreReviewDecision::ApprovedForSession => Self::AcceptForSession, + CoreReviewDecision::NetworkPolicyAmendment { + network_policy_amendment, + } => Self::ApplyNetworkPolicyAmendment { + network_policy_amendment: network_policy_amendment.into(), + }, + CoreReviewDecision::Abort => Self::Cancel, + CoreReviewDecision::Denied => Self::Decline, + } + } +} + v2_enum_from_core! { pub enum NetworkApprovalProtocol from CoreNetworkApprovalProtocol { Http, @@ -3802,6 +3825,11 @@ pub struct CommandExecutionRequestApprovalParams { #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional = nullable)] pub proposed_network_policy_amendments: Option>, + /// Ordered list of decisions the client may present for this prompt. + #[experimental("item/commandExecution/requestApproval.availableDecisions")] + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional = nullable)] + pub available_decisions: Option>, } impl CommandExecutionRequestApprovalParams { diff --git a/codex-rs/app-server-test-client/src/lib.rs b/codex-rs/app-server-test-client/src/lib.rs index a66f6bf66..27203c601 100644 --- a/codex-rs/app-server-test-client/src/lib.rs +++ b/codex-rs/app-server-test-client/src/lib.rs @@ -1534,6 +1534,7 @@ impl CodexClient { additional_permissions, proposed_execpolicy_amendment, proposed_network_policy_amendments, + available_decisions, } = params; println!( @@ -1548,6 +1549,9 @@ impl CodexClient { if let Some(network_approval_context) = network_approval_context.as_ref() { println!("< network approval context: {network_approval_context:?}"); } + if let Some(available_decisions) = available_decisions.as_ref() { + println!("< available decisions: {available_decisions:?}"); + } if let Some(command) = command.as_deref() { println!("< command: {command}"); } diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 505abb045..20b2dcf2c 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -710,7 +710,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`, optionally `approvalId` (for subcommand callbacks), and `reason`. For normal command approvals, it 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. 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`. +2. `item/commandExecution/requestApproval` (request) — carries the same `itemId`, `threadId`, `turnId`, optionally `approvalId` (for subcommand callbacks), and `reason`. For normal command approvals, it 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. 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. `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 fc8c10971..0bc2f1118 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -332,6 +332,11 @@ pub(crate) async fn apply_bespoke_event_handling( .note_permission_requested(&conversation_id.to_string()) .await; let approval_id_for_op = ev.effective_approval_id(); + let available_decisions = ev + .effective_available_decisions() + .into_iter() + .map(CommandExecutionApprovalDecision::from) + .collect::>(); let ExecApprovalRequestEvent { call_id, approval_id, @@ -428,6 +433,7 @@ pub(crate) async fn apply_bespoke_event_handling( additional_permissions, proposed_execpolicy_amendment: proposed_execpolicy_amendment_v2, proposed_network_policy_amendments: proposed_network_policy_amendments_v2, + available_decisions: Some(available_decisions), }; let rx = outgoing .send_request(ServerRequestPayload::CommandExecutionRequestApproval( diff --git a/codex-rs/app-server/src/transport.rs b/codex-rs/app-server/src/transport.rs index 8c36df982..6ac3e0d46 100644 --- a/codex-rs/app-server/src/transport.rs +++ b/codex-rs/app-server/src/transport.rs @@ -986,6 +986,7 @@ mod tests { ), proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, + available_decisions: None, }, }), }, @@ -1047,6 +1048,7 @@ mod tests { ), proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, + available_decisions: None, }, }), }, diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 26e6b339e..6566f0848 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -2589,9 +2589,13 @@ impl Session { /// Emit an exec approval request event and await the user's decision. /// - /// The request is keyed by `call_id` + `approval_id` so matching responses are delivered - /// to the correct in-flight turn. If the task is aborted, this returns the - /// default `ReviewDecision` (`Denied`). + /// The request is keyed by `call_id` + `approval_id` so matching responses + /// are delivered to the correct in-flight turn. If the task is aborted, + /// this returns the default `ReviewDecision` (`Denied`). + /// + /// Note that if `available_decisions` is `None`, then the other fields will + /// be used to derive the available decisions via + /// [ExecApprovalRequestEvent::default_available_decisions]. #[allow(clippy::too_many_arguments)] pub async fn request_command_approval( &self, @@ -2604,6 +2608,7 @@ impl Session { network_approval_context: Option, proposed_execpolicy_amendment: Option, additional_permissions: Option, + available_decisions: Option>, ) -> ReviewDecision { // command-level approvals use `call_id`. // `approval_id` is only present for subcommand callbacks (execve intercept) @@ -2637,6 +2642,14 @@ 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(), + ) + }); let event = EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { call_id, approval_id, @@ -2648,6 +2661,7 @@ impl Session { proposed_execpolicy_amendment, proposed_network_policy_amendments, additional_permissions, + available_decisions: Some(available_decisions), parsed_cmd, }); self.send_event(turn_context, event).await; diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index c1fb31ebd..0777f8e3b 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -322,6 +322,7 @@ async fn handle_exec_approval( network_approval_context, proposed_execpolicy_amendment, additional_permissions, + available_decisions, .. } = event; // Race approval with cancellation and timeout to avoid hangs. @@ -335,6 +336,7 @@ async fn handle_exec_approval( network_approval_context, proposed_execpolicy_amendment, additional_permissions, + available_decisions, ); let decision = await_approval_with_cancel( approval_fut, diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index ca31ec4d4..aeef59833 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -331,6 +331,7 @@ impl NetworkApprovalService { protocol, }; + let available_decisions = None; let approval_decision = session .request_command_approval( turn_context.as_ref(), @@ -342,6 +343,7 @@ impl NetworkApprovalService { Some(network_approval_context.clone()), None, None, + available_decisions, ) .await; diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index 80d3b22eb..101d9309f 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -150,6 +150,7 @@ impl Approvable for ShellRuntime { let call_id = ctx.call_id.to_string(); Box::pin(async move { with_cached_approval(&session.services, "shell", keys, move || async move { + let available_decisions = None; session .request_command_approval( turn, @@ -163,6 +164,7 @@ impl Approvable for ShellRuntime { .proposed_execpolicy_amendment() .cloned(), req.additional_permissions.clone(), + available_decisions, ) .await }) diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 3b1a0c906..5bcf809e2 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -189,6 +189,7 @@ impl CoreShellActionProvider { workdir: &AbsolutePathBuf, stopwatch: &Stopwatch, additional_permissions: Option, + decision_source: &DecisionSource, ) -> anyhow::Result { let command = join_program_and_argv(program, argv); let workdir = workdir.to_path_buf(); @@ -198,6 +199,20 @@ impl CoreShellActionProvider { let approval_id = Some(Uuid::new_v4().to_string()); Ok(stopwatch .pause_for(async move { + let available_decisions = vec![ + Some(ReviewDecision::Approved), + // Currently, ApprovedForSession is only honored for skills, + // so only offer it for skill script approvals. + if matches!(decision_source, DecisionSource::SkillScript { .. }) { + Some(ReviewDecision::ApprovedForSession) + } else { + None + }, + Some(ReviewDecision::Abort), + ] + .into_iter() + .flatten() + .collect(); session .request_command_approval( &turn, @@ -209,6 +224,7 @@ impl CoreShellActionProvider { None, None, additional_permissions, + Some(available_decisions), ) .await }) @@ -273,6 +289,7 @@ impl CoreShellActionProvider { workdir, &self.stopwatch, additional_permissions, + &decision_source, ) .await? { diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 4b0c53d12..f4d823bf0 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -111,6 +111,7 @@ impl Approvable for UnifiedExecRuntime<'_> { .or_else(|| req.justification.clone()); Box::pin(async move { with_cached_approval(&session.services, "unified_exec", keys, || async move { + let available_decisions = None; session .request_command_approval( turn, @@ -124,6 +125,7 @@ impl Approvable for UnifiedExecRuntime<'_> { .proposed_execpolicy_amendment() .cloned(), req.additional_permissions.clone(), + available_decisions, ) .await }) diff --git a/codex-rs/core/tests/suite/skill_approval.rs b/codex-rs/core/tests/suite/skill_approval.rs index 891f483d0..a3cfd4d7f 100644 --- a/codex-rs/core/tests/suite/skill_approval.rs +++ b/codex-rs/core/tests/suite/skill_approval.rs @@ -223,6 +223,14 @@ permissions: }; assert_eq!(approval.call_id, tool_call_id); assert_eq!(approval.command, vec![script_path_str.clone()]); + assert_eq!( + approval.available_decisions, + Some(vec![ + ReviewDecision::Approved, + ReviewDecision::ApprovedForSession, + ReviewDecision::Abort, + ]) + ); assert_eq!( approval.additional_permissions, Some(PermissionProfile { @@ -238,7 +246,7 @@ permissions: .submit(Op::ExecApproval { id: approval.effective_approval_id(), turn_id: None, - decision: ReviewDecision::Approved, + decision: ReviewDecision::Denied, }) .await?; @@ -253,12 +261,8 @@ permissions: .function_call_output(tool_call_id); let output = call_output["output"].as_str().unwrap_or_default(); assert!( - output.contains("zsh-fork-stdout"), - "expected stdout marker in function_call_output: {output:?}" - ); - assert!( - output.contains("zsh-fork-stderr"), - "expected stderr marker in function_call_output: {output:?}" + output.contains("Execution denied: User denied execution"), + "expected rejection marker in function_call_output: {output:?}" ); Ok(()) diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index cb204fa12..e624fe2b1 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -227,6 +227,7 @@ async fn run_codex_tool_session_inner( parsed_cmd, network_approval_context: _, additional_permissions: _, + available_decisions: _, } = ev; handle_exec_approval_request( command, diff --git a/codex-rs/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index f742dc632..98fe0280e 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -5,6 +5,7 @@ use crate::mcp::RequestId; use crate::models::PermissionProfile; use crate::parse_command::ParsedCommand; use crate::protocol::FileChange; +use crate::protocol::ReviewDecision; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; @@ -107,6 +108,13 @@ pub struct ExecApprovalRequestEvent { #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] pub additional_permissions: Option, + /// Ordered list of decisions the client may present for this prompt. + /// + /// When absent, clients should derive the legacy default set from the + /// other fields on this request. + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub available_decisions: Option>, pub parsed_cmd: Vec, } @@ -116,6 +124,55 @@ impl ExecApprovalRequestEvent { .clone() .unwrap_or_else(|| self.call_id.clone()) } + + pub fn effective_available_decisions(&self) -> Vec { + // available_decisions is a new field that may not be populated by older + // senders, so we fall back to the legacy logic if it's not present. + match &self.available_decisions { + Some(decisions) => decisions.clone(), + None => Self::default_available_decisions( + self.network_approval_context.as_ref(), + self.proposed_execpolicy_amendment.as_ref(), + self.proposed_network_policy_amendments.as_deref(), + self.additional_permissions.as_ref(), + ), + } + } + + pub fn default_available_decisions( + network_approval_context: Option<&NetworkApprovalContext>, + proposed_execpolicy_amendment: Option<&ExecPolicyAmendment>, + proposed_network_policy_amendments: Option<&[NetworkPolicyAmendment]>, + additional_permissions: Option<&PermissionProfile>, + ) -> Vec { + if network_approval_context.is_some() { + let mut decisions = vec![ReviewDecision::Approved, ReviewDecision::ApprovedForSession]; + if let Some(amendment) = proposed_network_policy_amendments.and_then(|amendments| { + amendments + .iter() + .find(|amendment| amendment.action == NetworkPolicyRuleAction::Allow) + }) { + decisions.push(ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment: amendment.clone(), + }); + } + decisions.push(ReviewDecision::Abort); + return decisions; + } + + if additional_permissions.is_some() { + return vec![ReviewDecision::Approved, ReviewDecision::Abort]; + } + + let mut decisions = vec![ReviewDecision::Approved]; + if let Some(prefix) = proposed_execpolicy_amendment { + decisions.push(ReviewDecision::ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment: prefix.clone(), + }); + } + decisions.push(ReviewDecision::Abort); + decisions + } } #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 15d85c9c7..1928f1bee 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -2777,8 +2777,8 @@ pub enum ReviewDecision { proposed_execpolicy_amendment: ExecPolicyAmendment, }, - /// User has approved this command and wants to automatically approve any - /// future identical instances (`command` and `cwd` match exactly) for the + /// User has approved this request and wants future prompts in the same + /// session-scoped approval cache to be automatically approved for the /// remainder of the session. ApprovedForSession, diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 62d459fc6..6ed8670e0 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -3569,6 +3569,7 @@ mod tests { proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: Vec::new(), }, ), diff --git a/codex-rs/tui/src/app/pending_interactive_replay.rs b/codex-rs/tui/src/app/pending_interactive_replay.rs index 3d3091478..d82d01482 100644 --- a/codex-rs/tui/src/app/pending_interactive_replay.rs +++ b/codex-rs/tui/src/app/pending_interactive_replay.rs @@ -402,6 +402,7 @@ mod tests { proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: Vec::new(), }, ), @@ -544,6 +545,7 @@ mod tests { proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: Vec::new(), }, ), @@ -622,6 +624,7 @@ mod tests { proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: Vec::new(), }, ), diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index fd58baef4..2ea3c10e9 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -20,10 +20,8 @@ use codex_core::features::Features; use codex_protocol::mcp::RequestId; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::ElicitationAction; -use codex_protocol::protocol::ExecPolicyAmendment; use codex_protocol::protocol::FileChange; use codex_protocol::protocol::NetworkApprovalContext; -use codex_protocol::protocol::NetworkPolicyAmendment; use codex_protocol::protocol::NetworkPolicyRuleAction; use codex_protocol::protocol::Op; use codex_protocol::protocol::ReviewDecision; @@ -46,9 +44,8 @@ pub(crate) enum ApprovalRequest { id: String, command: Vec, reason: Option, + available_decisions: Vec, network_approval_context: Option, - proposed_execpolicy_amendment: Option, - proposed_network_policy_amendments: Option>, additional_permissions: Option, }, ApplyPatch { @@ -67,7 +64,6 @@ pub(crate) enum ApprovalRequest { /// Modal overlay asking the user to approve or deny one or more requests. pub(crate) struct ApprovalOverlay { current_request: Option, - current_variant: Option, queue: Vec, app_event_tx: AppEventSender, list: ListSelectionView, @@ -81,7 +77,6 @@ impl ApprovalOverlay { pub fn new(request: ApprovalRequest, app_event_tx: AppEventSender, features: Features) -> Self { let mut view = Self { current_request: None, - current_variant: None, queue: Vec::new(), app_event_tx: app_event_tx.clone(), list: ListSelectionView::new(Default::default(), app_event_tx), @@ -99,31 +94,28 @@ impl ApprovalOverlay { } fn set_current(&mut self, request: ApprovalRequest) { - self.current_request = Some(request.clone()); - let ApprovalRequestState { variant, header } = ApprovalRequestState::from(request); - self.current_variant = Some(variant.clone()); self.current_complete = false; - let (options, params) = Self::build_options(variant, header, &self.features); + let header = build_header(&request); + let (options, params) = Self::build_options(&request, header, &self.features); + self.current_request = Some(request); self.options = options; self.list = ListSelectionView::new(params, self.app_event_tx.clone()); } fn build_options( - variant: ApprovalVariant, + request: &ApprovalRequest, header: Box, _features: &Features, ) -> (Vec, SelectionViewParams) { - let (options, title) = match &variant { - ApprovalVariant::Exec { + let (options, title) = match request { + ApprovalRequest::Exec { + available_decisions, network_approval_context, - proposed_execpolicy_amendment, - proposed_network_policy_amendments, additional_permissions, .. } => ( exec_options( - proposed_execpolicy_amendment.clone(), - proposed_network_policy_amendments.clone(), + available_decisions, network_approval_context.as_ref(), additional_permissions.as_ref(), ), @@ -137,11 +129,11 @@ impl ApprovalOverlay { }, ), ), - ApprovalVariant::ApplyPatch { .. } => ( + ApprovalRequest::ApplyPatch { .. } => ( patch_options(), "Would you like to make the following edits?".to_string(), ), - ApprovalVariant::McpElicitation { server_name, .. } => ( + ApprovalRequest::McpElicitation { server_name, .. } => ( elicitation_options(), format!("{server_name} needs your approval."), ), @@ -188,18 +180,19 @@ impl ApprovalOverlay { let Some(option) = self.options.get(actual_idx) else { return; }; - if let Some(variant) = self.current_variant.as_ref() { - match (variant, &option.decision) { - (ApprovalVariant::Exec { id, command, .. }, ApprovalDecision::Review(decision)) => { + if let Some(request) = self.current_request.as_ref() { + match (request, &option.decision) { + (ApprovalRequest::Exec { id, command, .. }, ApprovalDecision::Review(decision)) => { self.handle_exec_decision(id, command, decision.clone()); } - (ApprovalVariant::ApplyPatch { id, .. }, ApprovalDecision::Review(decision)) => { + (ApprovalRequest::ApplyPatch { id, .. }, ApprovalDecision::Review(decision)) => { self.handle_patch_decision(id, decision.clone()); } ( - ApprovalVariant::McpElicitation { + ApprovalRequest::McpElicitation { server_name, request_id, + .. }, ApprovalDecision::McpElicitation(decision), ) => { @@ -300,18 +293,19 @@ impl BottomPaneView for ApprovalOverlay { return CancellationEvent::Handled; } if !self.current_complete - && let Some(variant) = self.current_variant.as_ref() + && let Some(request) = self.current_request.as_ref() { - match &variant { - ApprovalVariant::Exec { id, command, .. } => { + match request { + ApprovalRequest::Exec { id, command, .. } => { self.handle_exec_decision(id, command, ReviewDecision::Abort); } - ApprovalVariant::ApplyPatch { id, .. } => { + ApprovalRequest::ApplyPatch { id, .. } => { self.handle_patch_decision(id, ReviewDecision::Abort); } - ApprovalVariant::McpElicitation { + ApprovalRequest::McpElicitation { server_name, request_id, + .. } => { self.handle_elicitation_decision( server_name, @@ -353,122 +347,77 @@ impl Renderable for ApprovalOverlay { } } -struct ApprovalRequestState { - variant: ApprovalVariant, - header: Box, -} - -impl From for ApprovalRequestState { - fn from(value: ApprovalRequest) -> Self { - match value { - ApprovalRequest::Exec { - id, - command, - reason, - network_approval_context, - proposed_execpolicy_amendment, - proposed_network_policy_amendments, - additional_permissions, - } => { - let mut header: Vec> = Vec::new(); - if let Some(reason) = reason { - header.push(Line::from(vec!["Reason: ".into(), reason.italic()])); - header.push(Line::from("")); - } - if let Some(ref additional_permissions) = additional_permissions - && let Some(rule_line) = - format_additional_permissions_rule(additional_permissions) - { - header.push(Line::from(vec![ - "Permission rule: ".into(), - rule_line.cyan(), - ])); - header.push(Line::from("")); - } - let full_cmd = strip_bash_lc_and_escape(&command); - let mut full_cmd_lines = highlight_bash_to_lines(&full_cmd); - if let Some(first) = full_cmd_lines.first_mut() { - first.spans.insert(0, Span::from("$ ")); - } - if network_approval_context.is_none() { - header.extend(full_cmd_lines); - } - Self { - variant: ApprovalVariant::Exec { - id, - command, - network_approval_context, - proposed_execpolicy_amendment, - proposed_network_policy_amendments, - additional_permissions, - }, - header: Box::new(Paragraph::new(header).wrap(Wrap { trim: false })), - } +fn build_header(request: &ApprovalRequest) -> Box { + match request { + ApprovalRequest::Exec { + reason, + command, + network_approval_context, + additional_permissions, + .. + } => { + let mut header: Vec> = Vec::new(); + if let Some(reason) = reason { + header.push(Line::from(vec!["Reason: ".into(), reason.clone().italic()])); + header.push(Line::from("")); } - ApprovalRequest::ApplyPatch { - id, - reason, - cwd, - changes, - } => { - let mut header: Vec> = Vec::new(); - if let Some(reason) = reason - && !reason.is_empty() - { - header.push(Box::new( - Paragraph::new(Line::from_iter(["Reason: ".into(), reason.italic()])) - .wrap(Wrap { trim: false }), - )); - header.push(Box::new(Line::from(""))); - } - header.push(DiffSummary::new(changes, cwd).into()); - Self { - variant: ApprovalVariant::ApplyPatch { id }, - header: Box::new(ColumnRenderable::with(header)), - } + if let Some(additional_permissions) = additional_permissions + && let Some(rule_line) = format_additional_permissions_rule(additional_permissions) + { + header.push(Line::from(vec![ + "Permission rule: ".into(), + rule_line.cyan(), + ])); + header.push(Line::from("")); } - ApprovalRequest::McpElicitation { - server_name, - request_id, - message, - } => { - let header = Paragraph::new(vec![ - Line::from(vec!["Server: ".into(), server_name.clone().bold()]), - Line::from(""), - Line::from(message), - ]) - .wrap(Wrap { trim: false }); - Self { - variant: ApprovalVariant::McpElicitation { - server_name, - request_id, - }, - header: Box::new(header), - } + let full_cmd = strip_bash_lc_and_escape(command); + let mut full_cmd_lines = highlight_bash_to_lines(&full_cmd); + if let Some(first) = full_cmd_lines.first_mut() { + first.spans.insert(0, Span::from("$ ")); } + if network_approval_context.is_none() { + header.extend(full_cmd_lines); + } + Box::new(Paragraph::new(header).wrap(Wrap { trim: false })) + } + ApprovalRequest::ApplyPatch { + reason, + cwd, + changes, + .. + } => { + let mut header: Vec> = Vec::new(); + if let Some(reason) = reason + && !reason.is_empty() + { + header.push(Box::new( + Paragraph::new(Line::from_iter([ + "Reason: ".into(), + reason.clone().italic(), + ])) + .wrap(Wrap { trim: false }), + )); + header.push(Box::new(Line::from(""))); + } + header.push(DiffSummary::new(changes.clone(), cwd.clone()).into()); + Box::new(ColumnRenderable::with(header)) + } + ApprovalRequest::McpElicitation { + server_name, + message, + .. + } => { + let header = Paragraph::new(vec![ + Line::from(vec!["Server: ".into(), server_name.clone().bold()]), + Line::from(""), + Line::from(message.clone()), + ]) + .wrap(Wrap { trim: false }); + Box::new(header) } } } -#[derive(Clone)] -enum ApprovalVariant { - Exec { - id: String, - command: Vec, - network_approval_context: Option, - proposed_execpolicy_amendment: Option, - proposed_network_policy_amendments: Option>, - additional_permissions: Option, - }, - ApplyPatch { - id: String, - }, - McpElicitation { - server_name: String, - request_id: RequestId, - }, -} - #[derive(Clone)] enum ApprovalDecision { Review(ReviewDecision), @@ -492,100 +441,93 @@ impl ApprovalOption { } fn exec_options( - proposed_execpolicy_amendment: Option, - proposed_network_policy_amendments: Option>, + available_decisions: &[ReviewDecision], network_approval_context: Option<&NetworkApprovalContext>, additional_permissions: Option<&PermissionProfile>, ) -> Vec { - if network_approval_context.is_some() { - let mut options = vec![ - ApprovalOption { - label: "Yes, just this once".to_string(), + available_decisions + .iter() + .filter_map(|decision| match decision { + ReviewDecision::Approved => Some(ApprovalOption { + label: if network_approval_context.is_some() { + "Yes, just this once".to_string() + } else { + "Yes, proceed".to_string() + }, decision: ApprovalDecision::Review(ReviewDecision::Approved), display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))], - }, - ApprovalOption { - label: "Yes, and allow this host for this conversation".to_string(), + }), + ReviewDecision::ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment, + } => { + let rendered_prefix = + strip_bash_lc_and_escape(proposed_execpolicy_amendment.command()); + if rendered_prefix.contains('\n') || rendered_prefix.contains('\r') { + return None; + } + + Some(ApprovalOption { + label: format!( + "Yes, and don't ask again for commands that start with `{rendered_prefix}`" + ), + decision: ApprovalDecision::Review( + ReviewDecision::ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment: proposed_execpolicy_amendment.clone(), + }, + ), + display_shortcut: None, + additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))], + }) + } + ReviewDecision::ApprovedForSession => Some(ApprovalOption { + label: if network_approval_context.is_some() { + "Yes, and allow this host for this conversation".to_string() + } else if additional_permissions.is_some() { + "Yes, and allow these permissions for this session".to_string() + } else { + "Yes, and don't ask again for this command in this session".to_string() + }, decision: ApprovalDecision::Review(ReviewDecision::ApprovedForSession), display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))], - }, - ]; - for amendment in proposed_network_policy_amendments.unwrap_or_default() { - let (label, shortcut) = match amendment.action { - NetworkPolicyRuleAction::Allow => ( - "Yes, and allow this host in the future".to_string(), - KeyCode::Char('p'), - ), - NetworkPolicyRuleAction::Deny => continue, - }; - options.push(ApprovalOption { - label, - decision: ApprovalDecision::Review(ReviewDecision::NetworkPolicyAmendment { - network_policy_amendment: amendment, - }), + }), + ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment, + } => { + let (label, shortcut) = match network_policy_amendment.action { + NetworkPolicyRuleAction::Allow => ( + "Yes, and allow this host in the future".to_string(), + KeyCode::Char('p'), + ), + NetworkPolicyRuleAction::Deny => ( + "No, and block this host in the future".to_string(), + KeyCode::Char('d'), + ), + }; + Some(ApprovalOption { + label, + decision: ApprovalDecision::Review(ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment: network_policy_amendment.clone(), + }), + display_shortcut: None, + additional_shortcuts: vec![key_hint::plain(shortcut)], + }) + } + ReviewDecision::Denied => Some(ApprovalOption { + label: "No, continue without running it".to_string(), + decision: ApprovalDecision::Review(ReviewDecision::Denied), display_shortcut: None, - additional_shortcuts: vec![key_hint::plain(shortcut)], - }); - } - options.push(ApprovalOption { - label: "No, and tell Codex what to do differently".to_string(), - decision: ApprovalDecision::Review(ReviewDecision::Abort), - display_shortcut: Some(key_hint::plain(KeyCode::Esc)), - additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))], - }); - return options; - } - - if additional_permissions.is_some() { - return vec![ - ApprovalOption { - label: "Yes, proceed".to_string(), - decision: ApprovalDecision::Review(ReviewDecision::Approved), - display_shortcut: None, - additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))], - }, - ApprovalOption { + additional_shortcuts: vec![key_hint::plain(KeyCode::Char('d'))], + }), + ReviewDecision::Abort => Some(ApprovalOption { label: "No, and tell Codex what to do differently".to_string(), decision: ApprovalDecision::Review(ReviewDecision::Abort), display_shortcut: Some(key_hint::plain(KeyCode::Esc)), additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))], - }, - ]; - } - - vec![ApprovalOption { - label: "Yes, proceed".to_string(), - decision: ApprovalDecision::Review(ReviewDecision::Approved), - display_shortcut: None, - additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))], - }] - .into_iter() - .chain(proposed_execpolicy_amendment.and_then(|prefix| { - let rendered_prefix = strip_bash_lc_and_escape(prefix.command()); - if rendered_prefix.contains('\n') || rendered_prefix.contains('\r') { - return None; - } - - Some(ApprovalOption { - label: format!( - "Yes, and don't ask again for commands that start with `{rendered_prefix}`" - ), - decision: ApprovalDecision::Review(ReviewDecision::ApprovedExecpolicyAmendment { - proposed_execpolicy_amendment: prefix, }), - display_shortcut: None, - additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))], }) - })) - .chain([ApprovalOption { - label: "No, and tell Codex what to do differently".to_string(), - decision: ApprovalDecision::Review(ReviewDecision::Abort), - display_shortcut: Some(key_hint::plain(KeyCode::Esc)), - additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))], - }]) - .collect() + .collect() } fn format_additional_permissions_rule( @@ -669,7 +611,9 @@ mod tests { use super::*; use crate::app_event::AppEvent; use codex_protocol::models::FileSystemPermissions; + use codex_protocol::protocol::ExecPolicyAmendment; use codex_protocol::protocol::NetworkApprovalProtocol; + use codex_protocol::protocol::NetworkPolicyAmendment; use insta::assert_snapshot; use pretty_assertions::assert_eq; use tokio::sync::mpsc::unbounded_channel; @@ -695,9 +639,8 @@ mod tests { id: "test".to_string(), command: vec!["echo".to_string(), "hi".to_string()], reason: Some("reason".to_string()), + available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort], network_approval_context: None, - proposed_execpolicy_amendment: None, - proposed_network_policy_amendments: None, additional_permissions: None, } } @@ -740,11 +683,16 @@ mod tests { id: "test".to_string(), command: vec!["echo".to_string()], reason: None, + available_decisions: vec![ + ReviewDecision::Approved, + ReviewDecision::ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment: ExecPolicyAmendment::new(vec![ + "echo".to_string(), + ]), + }, + ReviewDecision::Abort, + ], network_approval_context: None, - proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ - "echo".to_string(), - ])), - proposed_network_policy_amendments: None, additional_permissions: None, }, tx, @@ -781,21 +729,21 @@ mod tests { id: "test".to_string(), command: vec!["curl".to_string(), "https://example.com".to_string()], reason: None, + available_decisions: vec![ + ReviewDecision::Approved, + ReviewDecision::ApprovedForSession, + ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment: NetworkPolicyAmendment { + host: "example.com".to_string(), + action: NetworkPolicyRuleAction::Allow, + }, + }, + ReviewDecision::Abort, + ], network_approval_context: Some(NetworkApprovalContext { host: "example.com".to_string(), protocol: NetworkApprovalProtocol::Https, }), - proposed_execpolicy_amendment: None, - proposed_network_policy_amendments: Some(vec![ - NetworkPolicyAmendment { - host: "example.com".to_string(), - action: NetworkPolicyRuleAction::Allow, - }, - NetworkPolicyAmendment { - host: "example.com".to_string(), - action: NetworkPolicyRuleAction::Deny, - }, - ]), additional_permissions: None, }, tx, @@ -818,9 +766,8 @@ mod tests { id: "test".into(), command, reason: None, + available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort], network_approval_context: None, - proposed_execpolicy_amendment: None, - proposed_network_policy_amendments: None, additional_permissions: None, }; @@ -850,17 +797,17 @@ mod tests { protocol: NetworkApprovalProtocol::Https, }; let options = exec_options( - Some(ExecPolicyAmendment::new(vec!["curl".to_string()])), - Some(vec![ - NetworkPolicyAmendment { - host: "example.com".to_string(), - action: NetworkPolicyRuleAction::Allow, + &[ + ReviewDecision::Approved, + ReviewDecision::ApprovedForSession, + ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment: NetworkPolicyAmendment { + host: "example.com".to_string(), + action: NetworkPolicyRuleAction::Allow, + }, }, - NetworkPolicyAmendment { - host: "example.com".to_string(), - action: NetworkPolicyRuleAction::Deny, - }, - ]), + ReviewDecision::Abort, + ], Some(&network_context), None, ); @@ -877,6 +824,29 @@ mod tests { ); } + #[test] + fn generic_exec_options_can_offer_allow_for_session() { + let options = exec_options( + &[ + ReviewDecision::Approved, + ReviewDecision::ApprovedForSession, + ReviewDecision::Abort, + ], + None, + None, + ); + + let labels: Vec = options.into_iter().map(|option| option.label).collect(); + assert_eq!( + labels, + vec![ + "Yes, proceed".to_string(), + "Yes, and don't ask again for this command in this session".to_string(), + "No, and tell Codex what to do differently".to_string(), + ] + ); + } + #[test] fn additional_permissions_exec_options_hide_execpolicy_amendment() { let additional_permissions = PermissionProfile { @@ -886,7 +856,11 @@ mod tests { }), ..Default::default() }; - let options = exec_options(None, None, None, Some(&additional_permissions)); + let options = exec_options( + &[ReviewDecision::Approved, ReviewDecision::Abort], + None, + Some(&additional_permissions), + ); let labels: Vec = options.into_iter().map(|option| option.label).collect(); assert_eq!( @@ -906,9 +880,8 @@ mod tests { id: "test".into(), command: vec!["cat".into(), "/tmp/readme.txt".into()], reason: None, + available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort], network_approval_context: None, - proposed_execpolicy_amendment: None, - proposed_network_policy_amendments: None, additional_permissions: Some(PermissionProfile { file_system: Some(FileSystemPermissions { read: Some(vec![PathBuf::from("/tmp/readme.txt")]), @@ -946,9 +919,8 @@ mod tests { id: "test".into(), command: vec!["cat".into(), "/tmp/readme.txt".into()], reason: Some("need filesystem access".into()), + available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort], network_approval_context: None, - proposed_execpolicy_amendment: None, - proposed_network_policy_amendments: None, additional_permissions: Some(PermissionProfile { file_system: Some(FileSystemPermissions { read: Some(vec![PathBuf::from("/tmp/readme.txt")]), @@ -973,21 +945,21 @@ mod tests { id: "test".into(), command: vec!["curl".into(), "https://example.com".into()], reason: Some("network request blocked".into()), + available_decisions: vec![ + ReviewDecision::Approved, + ReviewDecision::ApprovedForSession, + ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment: NetworkPolicyAmendment { + host: "example.com".to_string(), + action: NetworkPolicyRuleAction::Allow, + }, + }, + ReviewDecision::Abort, + ], network_approval_context: Some(NetworkApprovalContext { host: "example.com".to_string(), protocol: NetworkApprovalProtocol::Https, }), - proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec!["curl".into()])), - proposed_network_policy_amendments: Some(vec![ - NetworkPolicyAmendment { - host: "example.com".to_string(), - action: NetworkPolicyRuleAction::Allow, - }, - NetworkPolicyAmendment { - host: "example.com".to_string(), - action: NetworkPolicyRuleAction::Deny, - }, - ]), additional_permissions: None, }; diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 17d885d5f..e774e9c42 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -1113,9 +1113,11 @@ mod tests { id: "1".to_string(), command: vec!["echo".into(), "ok".into()], reason: None, + available_decisions: vec![ + codex_protocol::protocol::ReviewDecision::Approved, + codex_protocol::protocol::ReviewDecision::Abort, + ], network_approval_context: None, - proposed_execpolicy_amendment: None, - proposed_network_policy_amendments: None, additional_permissions: None, } } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 47ad7aad5..a4063ed3d 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2568,13 +2568,13 @@ impl ChatWidget { .unwrap_or_else(|_| ev.command.join(" ")); self.notify(Notification::ExecApprovalRequested { command }); + let available_decisions = ev.effective_available_decisions(); let request = ApprovalRequest::Exec { id: ev.effective_approval_id(), command: ev.command, reason: ev.reason, + available_decisions, network_approval_context: ev.network_approval_context, - proposed_execpolicy_amendment: ev.proposed_execpolicy_amendment, - proposed_network_policy_amendments: ev.proposed_network_policy_amendments, additional_permissions: ev.additional_permissions, }; self.bottom_pane diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 00d4a1df9..48de3e368 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -2790,6 +2790,7 @@ async fn exec_approval_emits_proposed_command_and_decision_history() { proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -2839,6 +2840,7 @@ async fn exec_approval_uses_approval_id_when_present() { proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: vec![], }), }); @@ -2875,6 +2877,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -2929,6 +2932,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { proposed_execpolicy_amendment: None, proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -6748,6 +6752,7 @@ async fn approval_modal_exec_snapshot() -> anyhow::Result<()> { ])), proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -6808,6 +6813,7 @@ async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> { ])), proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -6855,6 +6861,7 @@ async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot() proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)), proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -7221,6 +7228,7 @@ async fn status_widget_and_approval_modal_snapshot() { ])), proposed_network_policy_amendments: None, additional_permissions: None, + available_decisions: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event {