diff --git a/codex-rs/analytics/src/analytics_client_tests.rs b/codex-rs/analytics/src/analytics_client_tests.rs index 0da520714f..724e83ec18 100644 --- a/codex-rs/analytics/src/analytics_client_tests.rs +++ b/codex-rs/analytics/src/analytics_client_tests.rs @@ -966,6 +966,7 @@ fn sample_command_approval_request(request_id: i64, approval_id: Option<&str>) - ServerRequest::CommandExecutionRequestApproval { request_id: RequestId::Integer(request_id), params: CommandExecutionRequestApprovalParams { + kind: Default::default(), thread_id: "thread-1".to_string(), turn_id: "turn-1".to_string(), item_id: "item-1".to_string(), @@ -1031,6 +1032,7 @@ fn sample_guardian_review_completed( review_id: &str, target_item_id: Option<&str>, status: GuardianApprovalReviewStatus, + action: GuardianApprovalReviewAction, ) -> ServerNotification { ServerNotification::ItemGuardianApprovalReviewCompleted( ItemGuardianApprovalReviewCompletedNotification { @@ -1047,11 +1049,7 @@ fn sample_guardian_review_completed( user_authorization: None, rationale: None, }, - action: GuardianApprovalReviewAction::Command { - source: AppServerGuardianCommandSource::Shell, - command: "echo hi".to_string(), - cwd: test_path_buf("/tmp").abs(), - }, + action, }, ) } @@ -3137,6 +3135,11 @@ async fn guardian_completed_notification_publishes_review_event_with_thread_meta "guardian-review-1", Some("item-1"), GuardianApprovalReviewStatus::Denied, + GuardianApprovalReviewAction::Command { + source: AppServerGuardianCommandSource::Shell, + command: "echo hi".to_string(), + cwd: test_path_buf("/tmp").abs(), + }, ))), &mut events, ) @@ -3186,6 +3189,29 @@ async fn terminal_reviews_denormalize_counts_onto_tool_item_events() { .await; events.clear(); + reducer + .ingest( + AnalyticsFact::Notification(Box::new(sample_guardian_review_completed( + "guardian-stdin-review-1", + Some("item-1"), + GuardianApprovalReviewStatus::Denied, + GuardianApprovalReviewAction::WriteStdin { + approval_id: "stdin-approval-1".to_string(), + process_id: "42".to_string(), + stdin: "confirm\n".to_string(), + cwd: test_path_buf("/tmp").abs().into(), + }, + ))), + &mut events, + ) + .await; + let review_payload = serde_json::to_value(&events[0]).expect("serialize review event"); + assert_eq!( + review_payload["event_params"]["subject_kind"], + "write_stdin" + ); + events.clear(); + ingest_completed_command_execution_item(&mut reducer, &mut events, "thread-1", "item-1").await; let payload = serde_json::to_value(&events[0]).expect("serialize tool item event"); diff --git a/codex-rs/analytics/src/events.rs b/codex-rs/analytics/src/events.rs index 0613171f4d..0e00c80b88 100644 --- a/codex-rs/analytics/src/events.rs +++ b/codex-rs/analytics/src/events.rs @@ -344,6 +344,9 @@ pub enum GuardianReviewedAction { additional_permissions: Option, tty: bool, }, + WriteStdin { + tty: bool, + }, Execve { source: GuardianCommandSource, program: String, @@ -664,6 +667,7 @@ pub(crate) struct CodexToolItemEventBase { #[serde(rename_all = "snake_case")] pub(crate) enum ReviewSubjectKind { CommandExecution, + WriteStdin, FileChange, McpToolCall, Permissions, diff --git a/codex-rs/analytics/src/reducer.rs b/codex-rs/analytics/src/reducer.rs index 54ce3df44a..aee7b8af36 100644 --- a/codex-rs/analytics/src/reducer.rs +++ b/codex-rs/analytics/src/reducer.rs @@ -2490,7 +2490,9 @@ fn item_review_summary_key(pending_review: &PendingReviewState) -> Option None, + ReviewSubjectKind::WriteStdin + | ReviewSubjectKind::Permissions + | ReviewSubjectKind::NetworkAccess => None, } } @@ -3030,6 +3032,11 @@ fn guardian_review_subject_metadata( "command_execution".to_string(), ReviewTrigger::Initial, ), + GuardianApprovalReviewAction::WriteStdin { .. } => ( + ReviewSubjectKind::WriteStdin, + "write_stdin".to_string(), + ReviewTrigger::Initial, + ), GuardianApprovalReviewAction::Execve { .. } => ( ReviewSubjectKind::CommandExecution, "command_execution".to_string(), @@ -3081,6 +3088,7 @@ fn guardian_review_requested_additional_permissions(action: &GuardianApprovalRev || permissions.file_system.is_some() } GuardianApprovalReviewAction::Command { .. } + | GuardianApprovalReviewAction::WriteStdin { .. } | GuardianApprovalReviewAction::Execve { .. } | GuardianApprovalReviewAction::McpToolCall { .. } => false, } @@ -3094,6 +3102,7 @@ fn guardian_review_requested_network_access(action: &GuardianApprovalReviewActio } GuardianApprovalReviewAction::ApplyPatch { .. } | GuardianApprovalReviewAction::Command { .. } + | GuardianApprovalReviewAction::WriteStdin { .. } | GuardianApprovalReviewAction::Execve { .. } | GuardianApprovalReviewAction::McpToolCall { .. } => false, } diff --git a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json index fd87c4fb32..4240a7b972 100644 --- a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json @@ -269,6 +269,14 @@ } ] }, + "CommandExecutionApprovalKind": { + "description": "Distinguishes a command approval from input sent to an existing terminal.", + "enum": [ + "command", + "writeStdin" + ], + "type": "string" + }, "FileSystemAccessMode": { "enum": [ "read", @@ -525,7 +533,7 @@ }, "properties": { "approvalId": { - "description": "Unique identifier for this specific approval callback.\n\nFor regular shell/unified_exec approvals, this is null.\n\nFor zsh-exec-bridge subcommand approvals, multiple callbacks can belong to one parent `itemId`, so `approvalId` is a distinct opaque callback id (a UUID) used to disambiguate routing.", + "description": "Unique identifier for this specific approval callback.\n\nFor regular shell/unified_exec approvals, this is null.\n\nFor zsh-exec-bridge subcommand approvals, multiple callbacks can belong to one parent `itemId`, so `approvalId` is a distinct opaque callback id (a UUID) used to disambiguate routing. Stdin approvals also use a distinct callback id; inspect `kind` to distinguish them.", "type": [ "string", "null" @@ -570,6 +578,15 @@ "itemId": { "type": "string" }, + "kind": { + "allOf": [ + { + "$ref": "#/definitions/CommandExecutionApprovalKind" + } + ], + "default": "command", + "description": "Kind of action under review. Defaults to `command` for older servers." + }, "networkApprovalContext": { "anyOf": [ { diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index 9228363870..7f152ee142 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -1879,6 +1879,39 @@ "title": "ExecveGuardianApprovalReviewAction", "type": "object" }, + { + "description": "A child approval for input to an existing command execution item.", + "properties": { + "approvalId": { + "type": "string" + }, + "cwd": { + "$ref": "#/definitions/LegacyAppPathString" + }, + "processId": { + "type": "string" + }, + "stdin": { + "type": "string" + }, + "type": { + "enum": [ + "writeStdin" + ], + "title": "WriteStdinGuardianApprovalReviewActionType", + "type": "string" + } + }, + "required": [ + "approvalId", + "cwd", + "processId", + "stdin", + "type" + ], + "title": "WriteStdinGuardianApprovalReviewAction", + "type": "object" + }, { "properties": { "cwd": { @@ -2385,7 +2418,7 @@ "type": "integer" }, "targetItemId": { - "description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.", + "description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - stdin reviews, which refer to the existing parent command item and have a separate approval ID in the action payload - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.", "type": [ "string", "null" @@ -2429,7 +2462,7 @@ "type": "integer" }, "targetItemId": { - "description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.", + "description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - stdin reviews, which refer to the existing parent command item and have a separate approval ID in the action payload - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.", "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 5451a4fd46..e701e96481 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ServerRequest.json @@ -342,10 +342,18 @@ } ] }, + "CommandExecutionApprovalKind": { + "description": "Distinguishes a command approval from input sent to an existing terminal.", + "enum": [ + "command", + "writeStdin" + ], + "type": "string" + }, "CommandExecutionRequestApprovalParams": { "properties": { "approvalId": { - "description": "Unique identifier for this specific approval callback.\n\nFor regular shell/unified_exec approvals, this is null.\n\nFor zsh-exec-bridge subcommand approvals, multiple callbacks can belong to one parent `itemId`, so `approvalId` is a distinct opaque callback id (a UUID) used to disambiguate routing.", + "description": "Unique identifier for this specific approval callback.\n\nFor regular shell/unified_exec approvals, this is null.\n\nFor zsh-exec-bridge subcommand approvals, multiple callbacks can belong to one parent `itemId`, so `approvalId` is a distinct opaque callback id (a UUID) used to disambiguate routing. Stdin approvals also use a distinct callback id; inspect `kind` to distinguish them.", "type": [ "string", "null" @@ -390,6 +398,15 @@ "itemId": { "type": "string" }, + "kind": { + "allOf": [ + { + "$ref": "#/definitions/CommandExecutionApprovalKind" + } + ], + "default": "command", + "description": "Kind of action under review. Defaults to `command` for older servers." + }, "networkApprovalContext": { "anyOf": [ { 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 c5fef02315..833e666633 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 @@ -2570,11 +2570,19 @@ } ] }, + "CommandExecutionApprovalKind": { + "description": "Distinguishes a command approval from input sent to an existing terminal.", + "enum": [ + "command", + "writeStdin" + ], + "type": "string" + }, "CommandExecutionRequestApprovalParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { "approvalId": { - "description": "Unique identifier for this specific approval callback.\n\nFor regular shell/unified_exec approvals, this is null.\n\nFor zsh-exec-bridge subcommand approvals, multiple callbacks can belong to one parent `itemId`, so `approvalId` is a distinct opaque callback id (a UUID) used to disambiguate routing.", + "description": "Unique identifier for this specific approval callback.\n\nFor regular shell/unified_exec approvals, this is null.\n\nFor zsh-exec-bridge subcommand approvals, multiple callbacks can belong to one parent `itemId`, so `approvalId` is a distinct opaque callback id (a UUID) used to disambiguate routing. Stdin approvals also use a distinct callback id; inspect `kind` to distinguish them.", "type": [ "string", "null" @@ -2619,6 +2627,15 @@ "itemId": { "type": "string" }, + "kind": { + "allOf": [ + { + "$ref": "#/definitions/CommandExecutionApprovalKind" + } + ], + "default": "command", + "description": "Kind of action under review. Defaults to `command` for older servers." + }, "networkApprovalContext": { "anyOf": [ { @@ -12225,6 +12242,39 @@ "title": "ExecveGuardianApprovalReviewAction", "type": "object" }, + { + "description": "A child approval for input to an existing command execution item.", + "properties": { + "approvalId": { + "type": "string" + }, + "cwd": { + "$ref": "#/definitions/v2/LegacyAppPathString" + }, + "processId": { + "type": "string" + }, + "stdin": { + "type": "string" + }, + "type": { + "enum": [ + "writeStdin" + ], + "title": "WriteStdinGuardianApprovalReviewActionType", + "type": "string" + } + }, + "required": [ + "approvalId", + "cwd", + "processId", + "stdin", + "type" + ], + "title": "WriteStdinGuardianApprovalReviewAction", + "type": "object" + }, { "properties": { "cwd": { @@ -13068,7 +13118,7 @@ "type": "integer" }, "targetItemId": { - "description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.", + "description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - stdin reviews, which refer to the existing parent command item and have a separate approval ID in the action payload - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.", "type": [ "string", "null" @@ -13114,7 +13164,7 @@ "type": "integer" }, "targetItemId": { - "description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.", + "description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - stdin reviews, which refer to the existing parent command item and have a separate approval ID in the action payload - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.", "type": [ "string", "null" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index c104d34eba..c77737fb16 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -8314,6 +8314,39 @@ "title": "ExecveGuardianApprovalReviewAction", "type": "object" }, + { + "description": "A child approval for input to an existing command execution item.", + "properties": { + "approvalId": { + "type": "string" + }, + "cwd": { + "$ref": "#/definitions/LegacyAppPathString" + }, + "processId": { + "type": "string" + }, + "stdin": { + "type": "string" + }, + "type": { + "enum": [ + "writeStdin" + ], + "title": "WriteStdinGuardianApprovalReviewActionType", + "type": "string" + } + }, + "required": [ + "approvalId", + "cwd", + "processId", + "stdin", + "type" + ], + "title": "WriteStdinGuardianApprovalReviewAction", + "type": "object" + }, { "properties": { "cwd": { @@ -9218,7 +9251,7 @@ "type": "integer" }, "targetItemId": { - "description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.", + "description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - stdin reviews, which refer to the existing parent command item and have a separate approval ID in the action payload - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.", "type": [ "string", "null" @@ -9264,7 +9297,7 @@ "type": "integer" }, "targetItemId": { - "description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.", + "description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - stdin reviews, which refer to the existing parent command item and have a separate approval ID in the action payload - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.", "type": [ "string", "null" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewCompletedNotification.json index 4e48606611..b3dd7eaeff 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewCompletedNotification.json @@ -372,6 +372,39 @@ "title": "ExecveGuardianApprovalReviewAction", "type": "object" }, + { + "description": "A child approval for input to an existing command execution item.", + "properties": { + "approvalId": { + "type": "string" + }, + "cwd": { + "$ref": "#/definitions/LegacyAppPathString" + }, + "processId": { + "type": "string" + }, + "stdin": { + "type": "string" + }, + "type": { + "enum": [ + "writeStdin" + ], + "title": "WriteStdinGuardianApprovalReviewActionType", + "type": "string" + } + }, + "required": [ + "approvalId", + "cwd", + "processId", + "stdin", + "type" + ], + "title": "WriteStdinGuardianApprovalReviewAction", + "type": "object" + }, { "properties": { "cwd": { @@ -606,7 +639,7 @@ "type": "integer" }, "targetItemId": { - "description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.", + "description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - stdin reviews, which refer to the existing parent command item and have a separate approval ID in the action payload - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.", "type": [ "string", "null" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewStartedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewStartedNotification.json index 7d64012fda..97f05b0b3a 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewStartedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewStartedNotification.json @@ -365,6 +365,39 @@ "title": "ExecveGuardianApprovalReviewAction", "type": "object" }, + { + "description": "A child approval for input to an existing command execution item.", + "properties": { + "approvalId": { + "type": "string" + }, + "cwd": { + "$ref": "#/definitions/LegacyAppPathString" + }, + "processId": { + "type": "string" + }, + "stdin": { + "type": "string" + }, + "type": { + "enum": [ + "writeStdin" + ], + "title": "WriteStdinGuardianApprovalReviewActionType", + "type": "string" + } + }, + "required": [ + "approvalId", + "cwd", + "processId", + "stdin", + "type" + ], + "title": "WriteStdinGuardianApprovalReviewAction", + "type": "object" + }, { "properties": { "cwd": { @@ -591,7 +624,7 @@ "type": "integer" }, "targetItemId": { - "description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.", + "description": "Identifier for the reviewed item or tool call when one exists.\n\nIn most cases, one review maps to one target item. The exceptions are - execve reviews, where a single command may contain multiple execve calls to review (only possible when using the shell_zsh_fork feature) - stdin reviews, which refer to the existing parent command item and have a separate approval ID in the action payload - network policy reviews, where there is no target item\n\nA network call is triggered by a CommandExecution item, so having a target_item_id set to the CommandExecution item would be misleading because the review is about the network call, not the command execution. Therefore, target_item_id is set to None for network policy reviews.", "type": [ "string", "null" diff --git a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst index 68253591d5..f5083afb25 100644 Binary files a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst and b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst differ diff --git a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst index 003d09c288..4a3d3f45ca 100644 Binary files a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst and b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst differ diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionApprovalKind.ts b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionApprovalKind.ts new file mode 100644 index 0000000000..f94eac8da4 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionApprovalKind.ts @@ -0,0 +1,8 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +/** + * Distinguishes a command approval from input sent to an existing terminal. + */ +export type CommandExecutionApprovalKind = "command" | "writeStdin"; 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 4f02c92cae..0ad35b4023 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts @@ -3,11 +3,15 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { LegacyAppPathString } from "../LegacyAppPathString"; import type { CommandAction } from "./CommandAction"; +import type { CommandExecutionApprovalKind } from "./CommandExecutionApprovalKind"; import type { ExecPolicyAmendment } from "./ExecPolicyAmendment"; import type { NetworkApprovalContext } from "./NetworkApprovalContext"; import type { NetworkPolicyAmendment } from "./NetworkPolicyAmendment"; -export type CommandExecutionRequestApprovalParams = {threadId: string, turnId: string, itemId: string, /** +export type CommandExecutionRequestApprovalParams = {/** + * Kind of action under review. Defaults to `command` for older servers. + */ +kind: CommandExecutionApprovalKind, threadId: string, turnId: string, itemId: string, /** * Unix timestamp (in milliseconds) when this approval request started. */ startedAtMs: number, /** @@ -18,6 +22,7 @@ startedAtMs: number, /** * For zsh-exec-bridge subcommand approvals, multiple callbacks can belong to * one parent `itemId`, so `approvalId` is a distinct opaque callback id * (a UUID) used to disambiguate routing. + * Stdin approvals also use a distinct callback id; inspect `kind` to distinguish them. */ approvalId?: string | null, /** * Environment in which the command will run. diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/GuardianApprovalReviewAction.ts b/codex-rs/app-server-protocol/schema/typescript/v2/GuardianApprovalReviewAction.ts index 4f00e37d20..8c21a9e115 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/GuardianApprovalReviewAction.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/GuardianApprovalReviewAction.ts @@ -2,8 +2,9 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { AbsolutePathBuf } from "../AbsolutePathBuf"; +import type { LegacyAppPathString } from "../LegacyAppPathString"; import type { GuardianCommandSource } from "./GuardianCommandSource"; import type { NetworkApprovalProtocol } from "./NetworkApprovalProtocol"; import type { RequestPermissionProfile } from "./RequestPermissionProfile"; -export type GuardianApprovalReviewAction = { "type": "command", source: GuardianCommandSource, command: string, cwd: AbsolutePathBuf, } | { "type": "execve", source: GuardianCommandSource, program: string, argv: Array, cwd: AbsolutePathBuf, } | { "type": "applyPatch", cwd: AbsolutePathBuf, files: Array, } | { "type": "networkAccess", target: string, host: string, protocol: NetworkApprovalProtocol, port: number, } | { "type": "mcpToolCall", server: string, toolName: string, connectorId: string | null, connectorName: string | null, toolTitle: string | null, } | { "type": "requestPermissions", reason: string | null, permissions: RequestPermissionProfile, }; +export type GuardianApprovalReviewAction = { "type": "command", source: GuardianCommandSource, command: string, cwd: AbsolutePathBuf, } | { "type": "execve", source: GuardianCommandSource, program: string, argv: Array, cwd: AbsolutePathBuf, } | { "type": "writeStdin", approvalId: string, processId: string, stdin: string, cwd: LegacyAppPathString, } | { "type": "applyPatch", cwd: AbsolutePathBuf, files: Array, } | { "type": "networkAccess", target: string, host: string, protocol: NetworkApprovalProtocol, port: number, } | { "type": "mcpToolCall", server: string, toolName: string, connectorId: string | null, connectorName: string | null, toolTitle: string | null, } | { "type": "requestPermissions", reason: string | null, permissions: RequestPermissionProfile, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ItemGuardianApprovalReviewCompletedNotification.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ItemGuardianApprovalReviewCompletedNotification.ts index 32d12be608..d28059d47a 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ItemGuardianApprovalReviewCompletedNotification.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ItemGuardianApprovalReviewCompletedNotification.ts @@ -28,6 +28,8 @@ reviewId: string, * In most cases, one review maps to one target item. The exceptions are * - execve reviews, where a single command may contain multiple execve * calls to review (only possible when using the shell_zsh_fork feature) + * - stdin reviews, which refer to the existing parent command item and + * have a separate approval ID in the action payload * - network policy reviews, where there is no target item * * A network call is triggered by a CommandExecution item, so having a diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ItemGuardianApprovalReviewStartedNotification.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ItemGuardianApprovalReviewStartedNotification.ts index 92d34fdebc..79b548d935 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ItemGuardianApprovalReviewStartedNotification.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ItemGuardianApprovalReviewStartedNotification.ts @@ -23,6 +23,8 @@ reviewId: string, * In most cases, one review maps to one target item. The exceptions are * - execve reviews, where a single command may contain multiple execve * calls to review (only possible when using the shell_zsh_fork feature) + * - stdin reviews, which refer to the existing parent command item and + * have a separate approval ID in the action payload * - network policy reviews, where there is no target item * * A network call is triggered by a CommandExecution item, so having a diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts index 2c6c12f075..36601f6371 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -77,6 +77,7 @@ export type { CommandExecTerminateResponse } from "./CommandExecTerminateRespons export type { CommandExecWriteParams } from "./CommandExecWriteParams"; export type { CommandExecWriteResponse } from "./CommandExecWriteResponse"; export type { CommandExecutionApprovalDecision } from "./CommandExecutionApprovalDecision"; +export type { CommandExecutionApprovalKind } from "./CommandExecutionApprovalKind"; export type { CommandExecutionOutputDeltaNotification } from "./CommandExecutionOutputDeltaNotification"; export type { CommandExecutionRequestApprovalParams } from "./CommandExecutionRequestApprovalParams"; export type { CommandExecutionRequestApprovalResponse } from "./CommandExecutionRequestApprovalResponse"; diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index f466ba7a81..1309c92c03 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -4468,6 +4468,7 @@ mod tests { #[test] fn command_execution_request_approval_additional_permissions_is_marked_experimental() { let params = v2::CommandExecutionRequestApprovalParams { + kind: Default::default(), thread_id: "thr_123".to_string(), turn_id: "turn_123".to_string(), item_id: "call_123".to_string(), diff --git a/codex-rs/app-server-protocol/src/protocol/item_builders.rs b/codex-rs/app-server-protocol/src/protocol/item_builders.rs index c71c65fa92..153ef1703d 100644 --- a/codex-rs/app-server-protocol/src/protocol/item_builders.rs +++ b/codex-rs/app-server-protocol/src/protocol/item_builders.rs @@ -185,6 +185,7 @@ fn command_actions_for_path_uri(parsed_cmd: &[ParsedCommand], cwd: &PathUri) -> /// /// Currently this only synthesizes [`ThreadItem::CommandExecution`] for /// [`GuardianAssessmentAction::Command`] and [`GuardianAssessmentAction::Execve`]. +/// Stdin reviews are child approvals and must not change the parent item lifecycle. pub fn build_item_from_guardian_event( assessment: &GuardianAssessmentEvent, status: CommandExecutionStatus, @@ -249,7 +250,8 @@ pub fn build_item_from_guardian_event( duration_ms: None, }) } - GuardianAssessmentAction::ApplyPatch { .. } + GuardianAssessmentAction::WriteStdin { .. } + | GuardianAssessmentAction::ApplyPatch { .. } | GuardianAssessmentAction::NetworkAccess { .. } | GuardianAssessmentAction::McpToolCall { .. } | GuardianAssessmentAction::RequestPermissions { .. } => None, diff --git a/codex-rs/app-server-protocol/src/protocol/item_builders_tests.rs b/codex-rs/app-server-protocol/src/protocol/item_builders_tests.rs index 5de4bc3014..47cc227110 100644 --- a/codex-rs/app-server-protocol/src/protocol/item_builders_tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/item_builders_tests.rs @@ -1,4 +1,10 @@ use super::*; +use crate::protocol::thread_history::build_turns_from_rollout_items; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::ExecCommandSource; +use codex_protocol::protocol::GuardianAssessmentStatus; +use codex_protocol::protocol::TurnStartedEvent; +use codex_rollout::RolloutItem; use pretty_assertions::assert_eq; use serde_json::json; @@ -83,3 +89,87 @@ fn read_command_actions_preserve_native_and_foreign_paths() { ); } } + +#[test] +fn guardian_stdin_reviews_preserve_parent_command_history() { + let cwd = PathUri::parse("file:///home/alice/repo").expect("valid cwd URI"); + let begin = ExecCommandBeginEvent { + call_id: "terminal-command".into(), + plugin_id: None, + script_path: None, + process_id: Some("42".into()), + turn_id: "turn-1".into(), + started_at_ms: 1_000, + command: vec!["cat".into()], + cwd: cwd.clone(), + parsed_cmd: vec![ParsedCommand::Unknown { cmd: "cat".into() }], + source: ExecCommandSource::UnifiedExecStartup, + interaction_input: None, + }; + let start_turn = |turn_id: &str| { + RolloutItem::EventMsg(EventMsg::TurnStarted(TurnStartedEvent { + turn_id: turn_id.into(), + trace_id: None, + started_at: None, + model_context_window: None, + collaboration_mode_kind: Default::default(), + })) + }; + let mut items = vec![ + start_turn("turn-1"), + RolloutItem::EventMsg(EventMsg::ExecCommandBegin(begin.clone())), + start_turn("turn-2"), + ]; + let expected = build_turns_from_rollout_items(&items); + assert_eq!(expected.len(), 2); + assert_eq!( + expected[0].items, + vec![build_command_execution_begin_item(&begin)], + ); + assert!(expected[1].items.is_empty()); + + let assessment = GuardianAssessmentEvent { + id: "review-stdin".into(), + target_item_id: Some("terminal-command".into()), + plugin_id: None, + script_path: None, + turn_id: "turn-2".into(), + started_at_ms: 2_000, + completed_at_ms: None, + status: GuardianAssessmentStatus::InProgress, + risk_level: None, + user_authorization: None, + rationale: None, + decision_source: None, + action: GuardianAssessmentAction::WriteStdin { + approval_id: "stdin-approval".into(), + process_id: "42".into(), + stdin: "yes\n".into(), + cwd, + }, + }; + + for turn_id in ["turn-1", "turn-2"] { + for status in [ + GuardianAssessmentStatus::InProgress, + GuardianAssessmentStatus::Approved, + GuardianAssessmentStatus::Denied, + GuardianAssessmentStatus::TimedOut, + GuardianAssessmentStatus::Aborted, + ] { + items.push(RolloutItem::EventMsg(EventMsg::GuardianAssessment( + GuardianAssessmentEvent { + turn_id: turn_id.into(), + status, + ..assessment.clone() + }, + ))); + assert_eq!( + build_turns_from_rollout_items(&items), + expected, + "stdin review {status:?} in {turn_id} must preserve command history", + ); + items.pop(); + } + } +} 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 6881d12099..5c1783c053 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/item.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/item.rs @@ -20,6 +20,7 @@ pub use codex_extension_items::image_generation::ImageGenerationItem; pub use codex_extension_items::sleep::SleepItem; pub use codex_extension_items::web_search::WebSearchAction; pub use codex_extension_items::web_search::WebSearchItem; +use codex_protocol::approvals::ExecApprovalKind as CoreExecApprovalKind; use codex_protocol::approvals::GuardianAssessmentAction as CoreGuardianAssessmentAction; use codex_protocol::approvals::GuardianAssessmentDecisionSource as CoreGuardianAssessmentDecisionSource; use codex_protocol::approvals::GuardianCommandSource as CoreGuardianCommandSource; @@ -635,6 +636,15 @@ pub enum GuardianApprovalReviewAction { argv: Vec, cwd: AbsolutePathBuf, }, + /// A child approval for input to an existing command execution item. + #[serde(rename_all = "camelCase")] + #[ts(rename_all = "camelCase")] + WriteStdin { + approval_id: String, + process_id: String, + stdin: String, + cwd: LegacyAppPathString, + }, #[serde(rename_all = "camelCase")] #[ts(rename_all = "camelCase")] ApplyPatch { @@ -689,6 +699,17 @@ impl From for GuardianApprovalReviewAction { argv, cwd, }, + CoreGuardianAssessmentAction::WriteStdin { + approval_id, + process_id, + stdin, + cwd, + } => Self::WriteStdin { + approval_id, + process_id, + stdin, + cwd: cwd.into(), + }, CoreGuardianAssessmentAction::ApplyPatch { cwd, files } => { Self::ApplyPatch { cwd, files } } @@ -752,6 +773,19 @@ impl TryFrom for CoreGuardianAssessmentAction { argv, cwd, }, + GuardianApprovalReviewAction::WriteStdin { + approval_id, + process_id, + stdin, + cwd, + } => Self::WriteStdin { + approval_id, + process_id, + stdin, + cwd: cwd + .try_into() + .map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?, + }, GuardianApprovalReviewAction::ApplyPatch { cwd, files } => { Self::ApplyPatch { cwd, files } } @@ -1280,6 +1314,8 @@ pub struct ItemGuardianApprovalReviewStartedNotification { /// In most cases, one review maps to one target item. The exceptions are /// - execve reviews, where a single command may contain multiple execve /// calls to review (only possible when using the shell_zsh_fork feature) + /// - stdin reviews, which refer to the existing parent command item and + /// have a separate approval ID in the action payload /// - network policy reviews, where there is no target item /// /// A network call is triggered by a CommandExecution item, so having a @@ -1312,6 +1348,8 @@ pub struct ItemGuardianApprovalReviewCompletedNotification { /// In most cases, one review maps to one target item. The exceptions are /// - execve reviews, where a single command may contain multiple execve /// calls to review (only possible when using the shell_zsh_fork feature) + /// - stdin reviews, which refer to the existing parent command item and + /// have a separate approval ID in the action payload /// - network policy reviews, where there is no target item /// /// A network call is triggered by a CommandExecution item, so having a @@ -1447,10 +1485,24 @@ pub struct FileChangePatchUpdatedNotification { pub changes: Vec, } +v2_enum_from_core! { + /// Distinguishes a command approval from input sent to an existing terminal. + #[derive(Default)] + #[ts(rename_all = "camelCase")] + pub enum CommandExecutionApprovalKind from CoreExecApprovalKind { + #[default] + Command, + WriteStdin, + } +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS, ExperimentalApi)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] pub struct CommandExecutionRequestApprovalParams { + /// Kind of action under review. Defaults to `command` for older servers. + #[serde(default)] + pub kind: CommandExecutionApprovalKind, pub thread_id: String, pub turn_id: String, pub item_id: String, @@ -1464,6 +1516,7 @@ pub struct CommandExecutionRequestApprovalParams { /// For zsh-exec-bridge subcommand approvals, multiple callbacks can belong to /// one parent `itemId`, so `approvalId` is a distinct opaque callback id /// (a UUID) used to disambiguate routing. + /// Stdin approvals also use a distinct callback id; inspect `kind` to distinguish them. #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional = nullable)] pub approval_id: Option, diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index f6c2788727..cf081d7c95 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -1,6 +1,7 @@ use super::*; use crate::ServerNotification; use codex_protocol::approvals::ElicitationRequest as CoreElicitationRequest; +use codex_protocol::approvals::GuardianAssessmentAction as CoreGuardianAssessmentAction; use codex_protocol::config_types::MultiAgentMode; use codex_protocol::items::AgentMessageContent; use codex_protocol::items::AgentMessageItem; @@ -2834,6 +2835,46 @@ fn guardian_approval_review_action_round_trips_command_shape() { ); } +#[test] +fn guardian_stdin_review_action_round_trips_native_and_foreign_paths() { + for (cwd_uri, cwd_native) in [ + ("file:///home/alice/repo", "/home/alice/repo"), + ( + "file:///C:/Users/Alice%20Smith/repo", + r"C:\Users\Alice Smith\repo", + ), + ("file://server/share/repo", r"\\server\share\repo"), + ] { + let core_action = CoreGuardianAssessmentAction::WriteStdin { + approval_id: "stdin-approval".into(), + process_id: "42".into(), + stdin: "yes\n".into(), + cwd: PathUri::parse(cwd_uri).expect("valid cwd URI"), + }; + let action = GuardianApprovalReviewAction::from(core_action.clone()); + let value = json!({ + "type": "writeStdin", + "approvalId": "stdin-approval", + "processId": "42", + "stdin": "yes\n", + "cwd": cwd_native, + }); + + assert_eq!( + serde_json::to_value(&action).expect("serialize stdin review action"), + value, + ); + let deserialized: GuardianApprovalReviewAction = + serde_json::from_value(value).expect("deserialize stdin review action"); + assert_eq!(deserialized, action); + assert_eq!( + CoreGuardianAssessmentAction::try_from(deserialized) + .expect("convert stdin review action"), + core_action, + ); + } +} + #[test] fn network_requirements_deserializes_legacy_fields() { let requirements: NetworkRequirements = serde_json::from_value(json!({ diff --git a/codex-rs/app-server-test-client/src/lib.rs b/codex-rs/app-server-test-client/src/lib.rs index 144bdb2a36..a034cae701 100644 --- a/codex-rs/app-server-test-client/src/lib.rs +++ b/codex-rs/app-server-test-client/src/lib.rs @@ -2168,6 +2168,7 @@ impl CodexClient { params: CommandExecutionRequestApprovalParams, ) -> Result<()> { let CommandExecutionRequestApprovalParams { + kind: _, thread_id, turn_id, item_id, diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index df16c36fd5..cbaf71df2a 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1667,7 +1667,7 @@ All items emit shared lifecycle events: - `item/autoApprovalReview/completed` — [UNSTABLE] temporary auto-review notification carrying `{threadId, turnId, targetItemId, review, action}` when approval auto-review resolves. This shape is expected to change soon. - `autoApprovalReview/strictReviewRequired` — experimental notification carrying `{threadId, turnId, startedAtMs}` whenever elevated or stale Guardian v2 risk requires synchronous approval review. -`review` is [UNSTABLE] and currently has `{status, riskLevel?, userAuthorization?, rationale?}`, where `status` is one of `inProgress`, `approved`, `denied`, or `aborted`. `riskLevel` is one of `"low"`, `"medium"`, `"high"`, or `"critical"` when present. `userAuthorization` is one of `"unknown"`, `"low"`, `"medium"`, or `"high"` when present. `action` is a tagged union with `type: "command" | "execve" | "applyPatch" | "networkAccess" | "mcpToolCall"`. Command-like actions include a `source` discriminator (`"shell"` or `"unifiedExec"`). These notifications are separate from the target item's own `item/completed` lifecycle and are intentionally temporary while the auto-review app protocol is still being designed. +`review` is [UNSTABLE] and currently has `{status, riskLevel?, userAuthorization?, rationale?}`, where `status` is one of `inProgress`, `approved`, `denied`, or `aborted`. `riskLevel` is one of `"low"`, `"medium"`, `"high"`, or `"critical"` when present. `userAuthorization` is one of `"unknown"`, `"low"`, `"medium"`, or `"high"` when present. `action` is a tagged union with `type: "command" | "execve" | "writeStdin" | "applyPatch" | "networkAccess" | "mcpToolCall" | "requestPermissions"`. Command-like actions include a `source` discriminator (`"shell"` or `"unifiedExec"`). A `writeStdin` action carries `approvalId`, `processId`, `stdin`, and `cwd`; it reviews input to an existing command item without changing that parent item's lifecycle. These notifications are separate from the target item's own `item/completed` lifecycle and are intentionally temporary while the auto-review app protocol is still being designed. There are additional item-specific events: @@ -1733,11 +1733,13 @@ 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` (for subcommand callbacks), and `reason`. 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, `kind` (`command` or `writeStdin`), optionally `approvalId` (for subcommand callbacks or stdin writes), and `reason`. 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. +`kind` distinguishes command approvals from writes to an existing terminal. Requests from older servers without `kind` retain `command` semantics; `approvalId` alone does not distinguish stdin writes from execve interception. + ### File change approvals Order of messages: diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 08a0bb0010..7d5c067b68 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -612,6 +612,7 @@ pub(crate) async fn apply_bespoke_event_handling( .map(CommandExecutionApprovalDecision::from) .collect::>(); let ExecApprovalRequestEvent { + kind, call_id, plugin_id, script_path, @@ -697,6 +698,7 @@ pub(crate) async fn apply_bespoke_event_handling( additional_permissions.map(V2AdditionalPermissionProfile::from); let params = CommandExecutionRequestApprovalParams { + kind: kind.into(), thread_id: conversation_id.to_string(), turn_id: turn_id.clone(), item_id: call_id.clone(), diff --git a/codex-rs/app-server/src/outgoing_message.rs b/codex-rs/app-server/src/outgoing_message.rs index dcb670a4a4..6624df8230 100644 --- a/codex-rs/app-server/src/outgoing_message.rs +++ b/codex-rs/app-server/src/outgoing_message.rs @@ -1021,6 +1021,7 @@ mod tests { let request = ServerRequest::CommandExecutionRequestApproval { request_id: RequestId::Integer(7), params: CommandExecutionRequestApprovalParams { + kind: Default::default(), thread_id: "thread-1".to_string(), turn_id: "turn-1".to_string(), item_id: "item-1".to_string(), diff --git a/codex-rs/app-server/src/transport_tests.rs b/codex-rs/app-server/src/transport_tests.rs index 968b5a0181..a44e66e2cf 100644 --- a/codex-rs/app-server/src/transport_tests.rs +++ b/codex-rs/app-server/src/transport_tests.rs @@ -257,6 +257,7 @@ async fn command_execution_request_approval_strips_additional_permissions_withou message: OutgoingMessage::Request(ServerRequest::CommandExecutionRequestApproval { request_id: RequestId::Integer(1), params: codex_app_server_protocol::CommandExecutionRequestApprovalParams { + kind: Default::default(), thread_id: "thr_123".to_string(), turn_id: "turn_123".to_string(), item_id: "call_123".to_string(), @@ -323,6 +324,7 @@ async fn command_execution_request_approval_keeps_additional_permissions_with_ca message: OutgoingMessage::Request(ServerRequest::CommandExecutionRequestApproval { request_id: RequestId::Integer(1), params: codex_app_server_protocol::CommandExecutionRequestApprovalParams { + kind: Default::default(), thread_id: "thr_123".to_string(), turn_id: "turn_123".to_string(), item_id: "call_123".to_string(), diff --git a/codex-rs/core/src/guardian/approval_request.rs b/codex-rs/core/src/guardian/approval_request.rs index 0e8a4c0597..37f67cb719 100644 --- a/codex-rs/core/src/guardian/approval_request.rs +++ b/codex-rs/core/src/guardian/approval_request.rs @@ -7,6 +7,7 @@ use codex_protocol::approvals::NetworkApprovalProtocol; use codex_protocol::models::AdditionalPermissionProfile; use codex_protocol::request_permissions::RequestPermissionProfile; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::LegacyAppPathString; use codex_utils_path_uri::PathUri; use serde::Serialize; use serde_json::Value; @@ -25,6 +26,21 @@ pub(crate) enum GuardianApprovalRequest { justification: Option, tty: bool, }, + #[cfg_attr( + not(test), + expect( + dead_code, + reason = "Constructed by the follow-up stdin approval routing change" + ) + )] + WriteStdin { + id: String, + approval_id: String, + process_id: i32, + input: String, + cwd: PathUri, + tty: bool, + }, #[cfg(unix)] Execve { id: String, @@ -110,6 +126,16 @@ struct CommandApprovalAction<'a> { tty: Option, } +#[derive(Serialize)] +struct WriteStdinApprovalAction<'a> { + tool: &'static str, + session_id: i32, + chars: &'a str, + cwd: LegacyAppPathString, + sandbox_permissions: crate::sandboxing::SandboxPermissions, + tty: bool, +} + #[cfg(unix)] #[derive(Serialize)] struct ExecveApprovalAction<'a> { @@ -273,6 +299,20 @@ pub(crate) fn guardian_approval_request_to_json( justification.as_ref(), Some(*tty), ), + GuardianApprovalRequest::WriteStdin { + process_id, + input, + cwd, + tty, + .. + } => serialize_guardian_action(WriteStdinApprovalAction { + tool: "write_stdin", + session_id: *process_id, + chars: input, + cwd: cwd.clone().into(), + sandbox_permissions: crate::sandboxing::SandboxPermissions::RequireEscalated, + tty: *tty, + }), #[cfg(unix)] GuardianApprovalRequest::Execve { id: _, @@ -367,6 +407,18 @@ pub(crate) fn guardian_assessment_action( GuardianApprovalRequest::ExecCommand { command, cwd, .. } => { command_assessment_action(GuardianCommandSource::UnifiedExec, command, cwd) } + GuardianApprovalRequest::WriteStdin { + approval_id, + process_id, + input, + cwd, + .. + } => GuardianAssessmentAction::WriteStdin { + approval_id: approval_id.clone(), + process_id: process_id.to_string(), + stdin: input.clone(), + cwd: cwd.clone(), + }, #[cfg(unix)] GuardianApprovalRequest::Execve { source, @@ -439,6 +491,9 @@ pub(crate) fn guardian_reviewed_action( additional_permissions: additional_permissions.clone(), tty: *tty, }, + GuardianApprovalRequest::WriteStdin { tty, .. } => { + GuardianReviewedAction::WriteStdin { tty: *tty } + } #[cfg(unix)] GuardianApprovalRequest::Execve { source, @@ -480,6 +535,7 @@ pub(crate) fn guardian_reviewed_action( pub(crate) fn guardian_request_target_item_id(request: &GuardianApprovalRequest) -> Option<&str> { match request { GuardianApprovalRequest::ExecCommand { id, .. } + | GuardianApprovalRequest::WriteStdin { id, .. } | GuardianApprovalRequest::ApplyPatch { id, .. } | GuardianApprovalRequest::McpToolCall { id, .. } | GuardianApprovalRequest::RequestPermissions { id, .. } => Some(id), @@ -497,6 +553,7 @@ pub(crate) fn guardian_request_turn_id<'a>( GuardianApprovalRequest::NetworkAccess { turn_id, .. } | GuardianApprovalRequest::RequestPermissions { turn_id, .. } => turn_id, GuardianApprovalRequest::ExecCommand { .. } + | GuardianApprovalRequest::WriteStdin { .. } | GuardianApprovalRequest::ApplyPatch { .. } | GuardianApprovalRequest::McpToolCall { .. } => default_turn_id, #[cfg(unix)] diff --git a/codex-rs/core/src/guardian/metrics.rs b/codex-rs/core/src/guardian/metrics.rs index f648695a4d..31c69e1ccd 100644 --- a/codex-rs/core/src/guardian/metrics.rs +++ b/codex-rs/core/src/guardian/metrics.rs @@ -178,6 +178,7 @@ fn reviewed_action_tag(action: &GuardianReviewedAction) -> &'static str { match action { GuardianReviewedAction::Shell { .. } => "shell", GuardianReviewedAction::UnifiedExec { .. } => "unified_exec", + GuardianReviewedAction::WriteStdin { .. } => "write_stdin", GuardianReviewedAction::Execve { .. } => "execve", GuardianReviewedAction::ApplyPatch {} => "apply_patch", GuardianReviewedAction::NetworkAccess { .. } => "network_access", diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index c699f0f187..eb9d452398 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -37,6 +37,7 @@ use codex_model_provider_info::OPENAI_PROVIDER_ID; use codex_models_manager::manager::StaticModelsManager; use codex_network_proxy::NetworkProxyConfig; use codex_protocol::ThreadId; +use codex_protocol::approvals::GuardianAssessmentAction; use codex_protocol::approvals::NetworkApprovalProtocol; use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::models::ContentItem; @@ -1364,6 +1365,49 @@ async fn build_guardian_prompt_items_explains_network_access_review_scope() -> a Ok(()) } +#[test] +fn guardian_write_stdin_preserves_input_and_foreign_cwd() -> serde_json::Result<()> { + let cwd = PathUri::parse("file:///C:/workspace").expect("valid executor cwd"); + let input = "confirm\n"; + let action = GuardianApprovalRequest::WriteStdin { + id: "terminal-open".to_string(), + approval_id: "terminal-write".to_string(), + process_id: 1000, + input: input.to_string(), + cwd: cwd.clone(), + tty: true, + }; + + assert_eq!( + guardian_approval_request_to_json(&action)?, + serde_json::json!({ + "tool": "write_stdin", + "session_id": 1000, + "chars": input, + "cwd": r"C:\workspace", + "sandbox_permissions": "require_escalated", + "tty": true, + }), + ); + assert_eq!( + guardian_assessment_action(&action), + GuardianAssessmentAction::WriteStdin { + approval_id: "terminal-write".to_string(), + process_id: "1000".to_string(), + stdin: input.to_string(), + cwd, + }, + ); + assert_eq!( + ( + guardian_request_target_item_id(&action), + guardian_request_turn_id(&action, "current-turn"), + ), + (Some("terminal-open"), "current-turn"), + ); + Ok(()) +} + #[test] fn guardian_assessment_action_redacts_apply_patch_patch_text() { let cwd = test_path_buf("/tmp").abs(); diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 3fc12c2d71..83d3e994d6 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -317,6 +317,7 @@ use codex_otel::SessionTelemetry; use codex_otel::THREAD_STARTED_METRIC; use codex_otel::TelemetryAuthMode; use codex_protocol::ResponseItemId; +use codex_protocol::approvals::ExecApprovalKind; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Personality; @@ -2493,6 +2494,7 @@ impl Session { .map(PluginCommandAttribution::serialized_fields) .unzip(); let event = EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { + kind: ExecApprovalKind::Command, call_id, plugin_id, script_path, diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index 7caf042b65..a097b7800e 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -197,6 +197,7 @@ async fn run_codex_tool_session_inner( EventMsg::ExecApprovalRequest(ev) => { let approval_id = ev.effective_approval_id(); let ExecApprovalRequestEvent { + kind: _, turn_id: _, environment_id: _, started_at_ms: _, diff --git a/codex-rs/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index 44dc8d7d7c..0dd621ff41 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -6,6 +6,7 @@ use crate::protocol::FileChange; use crate::protocol::ReviewDecision; use crate::request_permissions::RequestPermissionProfile; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; @@ -146,6 +147,13 @@ pub enum GuardianAssessmentAction { argv: Vec, cwd: AbsolutePathBuf, }, + /// A child approval for input to an existing command execution item. + WriteStdin { + approval_id: String, + process_id: String, + stdin: String, + cwd: PathUri, + }, ApplyPatch { cwd: AbsolutePathBuf, files: Vec, @@ -222,8 +230,20 @@ pub struct GuardianAssessmentEvent { pub action: GuardianAssessmentAction, } +/// Distinguishes a command approval from input sent to an existing terminal. +#[derive(Debug, Default, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "snake_case")] +pub enum ExecApprovalKind { + #[default] + Command, + WriteStdin, +} + #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct ExecApprovalRequestEvent { + /// Missing on older events, which retain command approval semantics. + #[serde(default)] + pub kind: ExecApprovalKind, /// Identifier for the associated command execution item. pub call_id: String, /// Trusted plugin attribution for the command item, when available. @@ -237,7 +257,7 @@ pub struct ExecApprovalRequestEvent { /// 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). + /// This is present for subcommand approvals (via execve intercept) and stdin writes. #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] pub approval_id: Option, diff --git a/codex-rs/tui/src/app/app_server_requests.rs b/codex-rs/tui/src/app/app_server_requests.rs index 213f1099e0..7598bbbca2 100644 --- a/codex-rs/tui/src/app/app_server_requests.rs +++ b/codex-rs/tui/src/app/app_server_requests.rs @@ -478,6 +478,7 @@ mod tests { let request = ServerRequest::CommandExecutionRequestApproval { request_id: AppServerRequestId::Integer(41), params: CommandExecutionRequestApprovalParams { + kind: Default::default(), thread_id: "thread-1".to_string(), turn_id: "turn-1".to_string(), item_id: "call-1".to_string(), @@ -888,6 +889,7 @@ mod tests { pending.note_server_request(&ServerRequest::CommandExecutionRequestApproval { request_id: AppServerRequestId::Integer(41), params: CommandExecutionRequestApprovalParams { + kind: Default::default(), thread_id: thread_id.to_ascii_uppercase(), turn_id: "turn-1".to_string(), item_id: "call-1".to_string(), diff --git a/codex-rs/tui/src/app/pending_interactive_replay.rs b/codex-rs/tui/src/app/pending_interactive_replay.rs index 09fd8f2840..0f23191a0d 100644 --- a/codex-rs/tui/src/app/pending_interactive_replay.rs +++ b/codex-rs/tui/src/app/pending_interactive_replay.rs @@ -609,6 +609,7 @@ mod tests { ServerRequest::CommandExecutionRequestApproval { request_id: AppServerRequestId::Integer(2), params: CommandExecutionRequestApprovalParams { + kind: Default::default(), thread_id: "thread-1".to_string(), turn_id: turn_id.to_string(), item_id: call_id.to_string(), diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index ca60a1380f..82218f27df 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -6324,6 +6324,7 @@ fn exec_approval_request( ServerRequest::CommandExecutionRequestApproval { request_id: AppServerRequestId::Integer(1), params: CommandExecutionRequestApprovalParams { + kind: Default::default(), thread_id: thread_id.to_string(), turn_id: turn_id.to_string(), item_id: item_id.to_string(), diff --git a/codex-rs/tui/src/app/tests/startup.rs b/codex-rs/tui/src/app/tests/startup.rs index 5540f81f8e..5ebd9569af 100644 --- a/codex-rs/tui/src/app/tests/startup.rs +++ b/codex-rs/tui/src/app/tests/startup.rs @@ -949,6 +949,7 @@ async fn startup_thread_started_discards_another_threads_buffered_events() { let request = ServerRequest::CommandExecutionRequestApproval { request_id: AppServerRequestId::Integer(1), params: CommandExecutionRequestApprovalParams { + kind: Default::default(), thread_id: other_thread_id.to_string(), turn_id: "turn-1".to_string(), item_id: "item-1".to_string(), diff --git a/codex-rs/tui/src/app/thread_event_buffer_tests.rs b/codex-rs/tui/src/app/thread_event_buffer_tests.rs index dc4e529126..bfb61b5184 100644 --- a/codex-rs/tui/src/app/thread_event_buffer_tests.rs +++ b/codex-rs/tui/src/app/thread_event_buffer_tests.rs @@ -38,6 +38,7 @@ fn exec_approval_request( ServerRequest::CommandExecutionRequestApproval { request_id: AppServerRequestId::Integer(1), params: CommandExecutionRequestApprovalParams { + kind: Default::default(), thread_id: thread_id.to_string(), turn_id: turn_id.to_string(), item_id: item_id.to_string(), diff --git a/codex-rs/tui/src/app/thread_events.rs b/codex-rs/tui/src/app/thread_events.rs index 05555af5c6..48a74ea7f7 100644 --- a/codex-rs/tui/src/app/thread_events.rs +++ b/codex-rs/tui/src/app/thread_events.rs @@ -512,6 +512,7 @@ mod tests { ServerRequest::CommandExecutionRequestApproval { request_id: AppServerRequestId::Integer(1), params: CommandExecutionRequestApprovalParams { + kind: Default::default(), thread_id: thread_id.to_string(), turn_id: turn_id.to_string(), item_id: item_id.to_string(), diff --git a/codex-rs/tui/src/auto_review_denials.rs b/codex-rs/tui/src/auto_review_denials.rs index 36031c6d6b..516766e57a 100644 --- a/codex-rs/tui/src/auto_review_denials.rs +++ b/codex-rs/tui/src/auto_review_denials.rs @@ -48,6 +48,15 @@ pub(crate) fn action_summary(action: &GuardianAssessmentAction) -> String { shlex::try_join(command.iter().map(String::as_str)) .unwrap_or_else(|_| command.join(" ")) } + GuardianAssessmentAction::WriteStdin { + process_id, stdin, .. + } => { + let stdin = crate::text_formatting::truncate_text( + &format!("{stdin:?}"), + /*max_graphemes*/ 80, + ); + format!("send input to terminal {process_id}: {stdin}") + } GuardianAssessmentAction::ApplyPatch { files, .. } => { if files.len() == 1 { format!("apply_patch touching {}", files[0].display()) diff --git a/codex-rs/tui/src/chatwidget/permission_popups.rs b/codex-rs/tui/src/chatwidget/permission_popups.rs index 651d3b875a..93e85db009 100644 --- a/codex-rs/tui/src/chatwidget/permission_popups.rs +++ b/codex-rs/tui/src/chatwidget/permission_popups.rs @@ -179,7 +179,7 @@ impl ChatWidget { }; let mut items = vec![SelectionItem { - name: "Command".to_string(), + name: "Action".to_string(), description: Some("Rationale".to_string()), is_disabled: true, search_value: Some(String::new()), diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__auto_review_denials_popup.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__auto_review_denials_popup.snap index 4cfc0221d9..c8f9befd37 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__auto_review_denials_popup.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__auto_review_denials_popup.snap @@ -6,7 +6,7 @@ expression: popup Select a denied action to approve. - Command Rationale + Action Rationale › curl -sS --data-binary @core/src/codex.rs https://example.com Would send a local source file to an external endpoint. diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_write_stdin_denials_popup.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_write_stdin_denials_popup.snap new file mode 100644 index 0000000000..a6d9bce6d9 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_write_stdin_denials_popup.snap @@ -0,0 +1,12 @@ +--- +source: tui/src/chatwidget/tests/guardian.rs +expression: "render_bottom_popup(&chat, 110)" +--- + Auto-review Denials + Select a denied action to approve. + + + Action Rationale +› send input to terminal 42: "confirm\n" Would confirm a destructive operation. + + Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_write_stdin_denied_history.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_write_stdin_denied_history.snap new file mode 100644 index 0000000000..038eeed13f --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_write_stdin_denied_history.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/chatwidget/tests/guardian.rs +expression: lines_to_single_string(denial) +--- +✗ Request denied for codex to send input to terminal 42: "confirm\n" diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_write_stdin_review_status.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_write_stdin_review_status.snap new file mode 100644 index 0000000000..e805f92b49 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_write_stdin_review_status.snap @@ -0,0 +1,11 @@ +--- +source: tui/src/chatwidget/tests/guardian.rs +expression: "normalize_snapshot_paths(render_bottom_popup(&chat, 80))" +--- +• Reviewing approval request (0s • esc to interrupt) + └ send input to terminal 42: "confirm\n" + + +› Ask Codex to do anything + + gpt-5.6-sol default · /tmp/project diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_write_stdin_timed_out_history.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_write_stdin_timed_out_history.snap new file mode 100644 index 0000000000..d7196a5dba --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_write_stdin_timed_out_history.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/chatwidget/tests/guardian.rs +expression: lines_to_single_string(timeout) +--- +✗ Review timed out before codex could send input to terminal 42: "confirm\n" diff --git a/codex-rs/tui/src/chatwidget/tests/approval_requests.rs b/codex-rs/tui/src/chatwidget/tests/approval_requests.rs index 1ec9a9c381..6e6aa7d16b 100644 --- a/codex-rs/tui/src/chatwidget/tests/approval_requests.rs +++ b/codex-rs/tui/src/chatwidget/tests/approval_requests.rs @@ -53,6 +53,7 @@ fn app_server_exec_approval_request_splits_shell_wrapped_command() { let script = r#"python3 -c 'print("Hello, world!")'"#; let request = exec_approval_request_from_params( AppServerCommandExecutionRequestApprovalParams { + kind: Default::default(), thread_id: "thread-1".to_string(), turn_id: "turn-1".to_string(), item_id: "item-1".to_string(), @@ -95,6 +96,7 @@ fn app_server_exec_approval_request_preserves_permissions_context() { let write_api_path = LegacyAppPathString::from_abs_path(&write_path); let request = exec_approval_request_from_params( AppServerCommandExecutionRequestApprovalParams { + kind: Default::default(), thread_id: "thread-1".to_string(), turn_id: "turn-1".to_string(), item_id: "item-1".to_string(), @@ -155,6 +157,7 @@ async fn network_exec_approval_history_describes_session_host_allowance() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; let request = exec_approval_request_from_params( AppServerCommandExecutionRequestApprovalParams { + kind: Default::default(), thread_id: "thread-1".to_string(), turn_id: "turn-1".to_string(), item_id: "item-1".to_string(), @@ -197,6 +200,7 @@ async fn network_exec_approval_history_describes_one_time_host_allowance() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; let request = exec_approval_request_from_params( AppServerCommandExecutionRequestApprovalParams { + kind: Default::default(), thread_id: "thread-1".to_string(), turn_id: "turn-1".to_string(), item_id: "item-1".to_string(), @@ -239,6 +243,7 @@ async fn network_exec_approval_history_describes_canceled_host_request() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; let request = exec_approval_request_from_params( AppServerCommandExecutionRequestApprovalParams { + kind: Default::default(), thread_id: "thread-1".to_string(), turn_id: "turn-1".to_string(), item_id: "item-1".to_string(), diff --git a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs index d57dbef008..662bd76bab 100644 --- a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs +++ b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs @@ -481,6 +481,7 @@ fn app_server_exec_approval_request_splits_shell_wrapped_command() { let script = r#"python3 -c 'print("Hello, world!")'"#; let request = exec_approval_request_from_params( AppServerCommandExecutionRequestApprovalParams { + kind: Default::default(), thread_id: "thread-1".to_string(), turn_id: "turn-1".to_string(), item_id: "item-1".to_string(), diff --git a/codex-rs/tui/src/chatwidget/tests/guardian.rs b/codex-rs/tui/src/chatwidget/tests/guardian.rs index ee483ed06f..7783b47434 100644 --- a/codex-rs/tui/src/chatwidget/tests/guardian.rs +++ b/codex-rs/tui/src/chatwidget/tests/guardian.rs @@ -51,6 +51,148 @@ fn guardian_command_event( } } +fn guardian_write_stdin_notification(status: GuardianApprovalReviewStatus) -> ServerNotification { + let action = AppServerGuardianApprovalReviewAction::WriteStdin { + approval_id: "stdin-approval".to_string(), + process_id: "42".to_string(), + stdin: "confirm\n".to_string(), + cwd: test_path_buf("/tmp/project").abs().into(), + }; + let review = GuardianApprovalReview { + status, + risk_level: None, + user_authorization: None, + rationale: (status == GuardianApprovalReviewStatus::Denied) + .then(|| "Would confirm a destructive operation.".to_string()), + }; + if status == GuardianApprovalReviewStatus::InProgress { + ServerNotification::ItemGuardianApprovalReviewStarted( + ItemGuardianApprovalReviewStartedNotification { + thread_id: "thread-1".to_string(), + turn_id: "turn-2".to_string(), + started_at_ms: 0, + review_id: "guardian-write-stdin".to_string(), + target_item_id: Some("parent-terminal".to_string()), + review, + action, + }, + ) + } else { + ServerNotification::ItemGuardianApprovalReviewCompleted( + ItemGuardianApprovalReviewCompletedNotification { + thread_id: "thread-1".to_string(), + turn_id: "turn-2".to_string(), + started_at_ms: 0, + completed_at_ms: 1, + review_id: "guardian-write-stdin".to_string(), + target_item_id: Some("parent-terminal".to_string()), + decision_source: AppServerGuardianApprovalReviewDecisionSource::Agent, + review, + action, + }, + ) + } +} + +#[tokio::test] +async fn app_server_guardian_write_stdin_denial_preserves_child_action() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let thread_id = ThreadId::new(); + chat.thread_id = Some(thread_id); + chat.on_task_started(); + + chat.handle_server_notification( + guardian_write_stdin_notification(GuardianApprovalReviewStatus::InProgress), + /*replay_kind*/ None, + ); + assert_chatwidget_snapshot!( + "guardian_write_stdin_review_status", + normalize_snapshot_paths(render_bottom_popup(&chat, /*width*/ 80)) + ); + + chat.handle_server_notification( + guardian_write_stdin_notification(GuardianApprovalReviewStatus::Denied), + /*replay_kind*/ None, + ); + assert!(chat.status_state.pending_guardian_review_status.is_empty()); + assert_eq!(chat.status_state.current_status.header, "Working"); + let history = drain_insert_history(&mut rx); + let [denial] = history.as_slice() else { + panic!("expected only the stdin denial in history"); + }; + assert_chatwidget_snapshot!( + "guardian_write_stdin_denied_history", + lines_to_single_string(denial) + ); + + chat.open_auto_review_denials_popup(); + assert_chatwidget_snapshot!( + "guardian_write_stdin_denials_popup", + render_bottom_popup(&chat, /*width*/ 110) + ); + + chat.approve_recent_auto_review_denial(thread_id, "guardian-write-stdin".to_string()); + let Ok(AppEvent::SubmitThreadOp { + thread_id: submitted_thread_id, + op: Op::ApproveGuardianDeniedAction { event }, + }) = rx.try_recv() + else { + panic!("expected a structured approval for the stdin denial"); + }; + assert_eq!( + (submitted_thread_id, event.action), + ( + thread_id, + GuardianAssessmentAction::WriteStdin { + approval_id: "stdin-approval".to_string(), + process_id: "42".to_string(), + stdin: "confirm\n".to_string(), + cwd: test_path_buf("/tmp/project").abs().into(), + }, + ) + ); +} + +#[tokio::test] +async fn app_server_guardian_write_stdin_approval_and_timeout_clear_review_status() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.on_task_started(); + + for status in [ + GuardianApprovalReviewStatus::InProgress, + GuardianApprovalReviewStatus::Approved, + ] { + chat.handle_server_notification( + guardian_write_stdin_notification(status), + /*replay_kind*/ None, + ); + } + assert!(drain_insert_history(&mut rx).is_empty()); + assert!(chat.status_state.pending_guardian_review_status.is_empty()); + assert_eq!(chat.status_state.current_status.header, "Working"); + + for status in [ + GuardianApprovalReviewStatus::InProgress, + GuardianApprovalReviewStatus::TimedOut, + ] { + chat.handle_server_notification( + guardian_write_stdin_notification(status), + /*replay_kind*/ None, + ); + } + assert!(chat.status_state.pending_guardian_review_status.is_empty()); + assert!(chat.review.recent_auto_review_denials.is_empty()); + assert_eq!(chat.status_state.current_status.header, "Working"); + let history = drain_insert_history(&mut rx); + let [timeout] = history.as_slice() else { + panic!("expected only the stdin timeout in history"); + }; + assert_chatwidget_snapshot!( + "guardian_write_stdin_timed_out_history", + lines_to_single_string(timeout) + ); +} + #[tokio::test] async fn auto_review_denials_popup_lists_stored_auto_review_denials() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; diff --git a/codex-rs/tui/src/chatwidget/tool_requests.rs b/codex-rs/tui/src/chatwidget/tool_requests.rs index f3783fa251..eb549a4154 100644 --- a/codex-rs/tui/src/chatwidget/tool_requests.rs +++ b/codex-rs/tui/src/chatwidget/tool_requests.rs @@ -54,6 +54,9 @@ impl ChatWidget { .ok() .or_else(|| Some(command.join(" "))) } + GuardianAssessmentAction::WriteStdin { .. } => { + Some(auto_review_denials::action_summary(action)) + } GuardianAssessmentAction::ApplyPatch { files, .. } => Some(if files.len() == 1 { format!("apply_patch touching {}", files[0].display()) } else { @@ -85,7 +88,8 @@ impl ChatWidget { argv.clone() }) .filter(|command| !command.is_empty()), - GuardianAssessmentAction::ApplyPatch { .. } + GuardianAssessmentAction::WriteStdin { .. } + | GuardianAssessmentAction::ApplyPatch { .. } | GuardianAssessmentAction::NetworkAccess { .. } | GuardianAssessmentAction::McpToolCall { .. } | GuardianAssessmentAction::RequestPermissions { .. } => None, @@ -160,6 +164,12 @@ impl ChatWidget { ) } else { match &ev.action { + GuardianAssessmentAction::WriteStdin { .. } => { + history_cell::new_guardian_timed_out_action_request(format!( + "codex could {}", + auto_review_denials::action_summary(&ev.action) + )) + } GuardianAssessmentAction::ApplyPatch { files, .. } => { let files = files .iter() @@ -204,6 +214,12 @@ impl ChatWidget { ) } else { match &ev.action { + GuardianAssessmentAction::WriteStdin { .. } => { + history_cell::new_guardian_denied_action_request(format!( + "codex to {}", + auto_review_denials::action_summary(&ev.action) + )) + } GuardianAssessmentAction::ApplyPatch { files, .. } => { let files = files .iter()