From 4f456681067d8a8ab1dc1cf5e2a4fec9d3a11227 Mon Sep 17 00:00:00 2001 From: Celia Chen Date: Wed, 25 Feb 2026 17:02:42 -0800 Subject: [PATCH 01/54] Revert "Add skill approval event/response (#12633)" (#12811) This reverts commit https://github.com/openai/codex/pull/12633. We no longer need this PR, because we favor sending normal exec command approval server request with `additional_permissions` of skill permissions instead --- .../schema/json/EventMsg.json | 48 -------- .../schema/json/ServerRequest.json | 39 ------- .../json/SkillRequestApprovalParams.json | 17 --- .../json/SkillRequestApprovalResponse.json | 22 ---- .../codex_app_server_protocol.schemas.json | 85 -------------- .../schema/typescript/EventMsg.ts | 3 +- .../schema/typescript/ServerRequest.ts | 3 +- .../typescript/SkillRequestApprovalEvent.ts | 5 - .../schema/typescript/index.ts | 1 - .../typescript/v2/SkillApprovalDecision.ts | 5 - .../v2/SkillRequestApprovalParams.ts | 5 - .../v2/SkillRequestApprovalResponse.ts | 6 - .../schema/typescript/v2/index.ts | 3 - .../src/protocol/common.rs | 5 - .../app-server-protocol/src/protocol/v2.rs | 23 ---- codex-rs/app-server-test-client/src/lib.rs | 22 ---- .../app-server/src/bespoke_event_handling.rs | 35 ------ codex-rs/app-server/tests/suite/v2/mod.rs | 1 - .../tests/suite/v2/skill_approval.rs | 109 ------------------ codex-rs/core/config.schema.json | 6 - codex-rs/core/src/codex.rs | 82 ------------- codex-rs/core/src/features.rs | 8 -- codex-rs/core/src/rollout/policy.rs | 1 - codex-rs/core/src/state/turn.rs | 18 --- codex-rs/core/tests/suite/mod.rs | 1 - codex-rs/core/tests/suite/skill_approval.rs | 11 -- .../src/event_processor_with_human_output.rs | 3 +- codex-rs/mcp-server/src/codex_tool_runner.rs | 1 - codex-rs/protocol/src/lib.rs | 1 - codex-rs/protocol/src/protocol.rs | 12 -- codex-rs/protocol/src/skill_approval.rs | 15 --- codex-rs/tui/src/chatwidget.rs | 3 +- 32 files changed, 4 insertions(+), 595 deletions(-) delete mode 100644 codex-rs/app-server-protocol/schema/json/SkillRequestApprovalParams.json delete mode 100644 codex-rs/app-server-protocol/schema/json/SkillRequestApprovalResponse.json delete mode 100644 codex-rs/app-server-protocol/schema/typescript/SkillRequestApprovalEvent.ts delete mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/SkillApprovalDecision.ts delete mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/SkillRequestApprovalParams.ts delete mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/SkillRequestApprovalResponse.ts delete mode 100644 codex-rs/app-server/tests/suite/v2/skill_approval.rs delete mode 100644 codex-rs/protocol/src/skill_approval.rs diff --git a/codex-rs/app-server-protocol/schema/json/EventMsg.json b/codex-rs/app-server-protocol/schema/json/EventMsg.json index 583a98c1d6..51064047ee 100644 --- a/codex-rs/app-server-protocol/schema/json/EventMsg.json +++ b/codex-rs/app-server-protocol/schema/json/EventMsg.json @@ -1884,30 +1884,6 @@ "title": "DynamicToolCallResponseEventMsg", "type": "object" }, - { - "properties": { - "item_id": { - "type": "string" - }, - "skill_name": { - "type": "string" - }, - "type": { - "enum": [ - "skill_request_approval" - ], - "title": "SkillRequestApprovalEventMsgType", - "type": "string" - } - }, - "required": [ - "item_id", - "skill_name", - "type" - ], - "title": "SkillRequestApprovalEventMsg", - "type": "object" - }, { "properties": { "id": { @@ -7374,30 +7350,6 @@ "title": "DynamicToolCallResponseEventMsg", "type": "object" }, - { - "properties": { - "item_id": { - "type": "string" - }, - "skill_name": { - "type": "string" - }, - "type": { - "enum": [ - "skill_request_approval" - ], - "title": "SkillRequestApprovalEventMsgType", - "type": "string" - } - }, - "required": [ - "item_id", - "skill_name", - "type" - ], - "title": "SkillRequestApprovalEventMsg", - "type": "object" - }, { "properties": { "id": { diff --git a/codex-rs/app-server-protocol/schema/json/ServerRequest.json b/codex-rs/app-server-protocol/schema/json/ServerRequest.json index ba94b889cf..e9cbc2ac1b 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ServerRequest.json @@ -722,21 +722,6 @@ } ] }, - "SkillRequestApprovalParams": { - "properties": { - "itemId": { - "type": "string" - }, - "skillName": { - "type": "string" - } - }, - "required": [ - "itemId", - "skillName" - ], - "type": "object" - }, "ThreadId": { "type": "string" }, @@ -898,30 +883,6 @@ "title": "Item/tool/requestUserInputRequest", "type": "object" }, - { - "properties": { - "id": { - "$ref": "#/definitions/RequestId" - }, - "method": { - "enum": [ - "skill/requestApproval" - ], - "title": "Skill/requestApprovalRequestMethod", - "type": "string" - }, - "params": { - "$ref": "#/definitions/SkillRequestApprovalParams" - } - }, - "required": [ - "id", - "method", - "params" - ], - "title": "Skill/requestApprovalRequest", - "type": "object" - }, { "description": "Execute a dynamic tool call on the client.", "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/SkillRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/SkillRequestApprovalParams.json deleted file mode 100644 index eebe6570b2..0000000000 --- a/codex-rs/app-server-protocol/schema/json/SkillRequestApprovalParams.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "$schema": "http://json-schema.org/draft-07/schema#", - "properties": { - "itemId": { - "type": "string" - }, - "skillName": { - "type": "string" - } - }, - "required": [ - "itemId", - "skillName" - ], - "title": "SkillRequestApprovalParams", - "type": "object" -} \ No newline at end of file diff --git a/codex-rs/app-server-protocol/schema/json/SkillRequestApprovalResponse.json b/codex-rs/app-server-protocol/schema/json/SkillRequestApprovalResponse.json deleted file mode 100644 index 0bea163ccb..0000000000 --- a/codex-rs/app-server-protocol/schema/json/SkillRequestApprovalResponse.json +++ /dev/null @@ -1,22 +0,0 @@ -{ - "$schema": "http://json-schema.org/draft-07/schema#", - "definitions": { - "SkillApprovalDecision": { - "enum": [ - "approve", - "decline" - ], - "type": "string" - } - }, - "properties": { - "decision": { - "$ref": "#/definitions/SkillApprovalDecision" - } - }, - "required": [ - "decision" - ], - "title": "SkillRequestApprovalResponse", - "type": "object" -} \ No newline at end of file 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 3da788229c..52a33c4c02 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 @@ -3052,30 +3052,6 @@ "title": "DynamicToolCallResponseEventMsg", "type": "object" }, - { - "properties": { - "item_id": { - "type": "string" - }, - "skill_name": { - "type": "string" - }, - "type": { - "enum": [ - "skill_request_approval" - ], - "title": "SkillRequestApprovalEventMsgType", - "type": "string" - } - }, - "required": [ - "item_id", - "skill_name", - "type" - ], - "title": "SkillRequestApprovalEventMsg", - "type": "object" - }, { "properties": { "id": { @@ -6732,30 +6708,6 @@ "title": "Item/tool/requestUserInputRequest", "type": "object" }, - { - "properties": { - "id": { - "$ref": "#/definitions/RequestId" - }, - "method": { - "enum": [ - "skill/requestApproval" - ], - "title": "Skill/requestApprovalRequestMethod", - "type": "string" - }, - "params": { - "$ref": "#/definitions/SkillRequestApprovalParams" - } - }, - "required": [ - "id", - "method", - "params" - ], - "title": "Skill/requestApprovalRequest", - "type": "object" - }, { "description": "Execute a dynamic tool call on the client.", "properties": { @@ -6877,43 +6829,6 @@ ], "type": "object" }, - "SkillApprovalDecision": { - "enum": [ - "approve", - "decline" - ], - "type": "string" - }, - "SkillRequestApprovalParams": { - "$schema": "http://json-schema.org/draft-07/schema#", - "properties": { - "itemId": { - "type": "string" - }, - "skillName": { - "type": "string" - } - }, - "required": [ - "itemId", - "skillName" - ], - "title": "SkillRequestApprovalParams", - "type": "object" - }, - "SkillRequestApprovalResponse": { - "$schema": "http://json-schema.org/draft-07/schema#", - "properties": { - "decision": { - "$ref": "#/definitions/SkillApprovalDecision" - } - }, - "required": [ - "decision" - ], - "title": "SkillRequestApprovalResponse", - "type": "object" - }, "StepStatus": { "enum": [ "pending", diff --git a/codex-rs/app-server-protocol/schema/typescript/EventMsg.ts b/codex-rs/app-server-protocol/schema/typescript/EventMsg.ts index f1e528d7db..5ccbe1e697 100644 --- a/codex-rs/app-server-protocol/schema/typescript/EventMsg.ts +++ b/codex-rs/app-server-protocol/schema/typescript/EventMsg.ts @@ -57,7 +57,6 @@ import type { RemoteSkillDownloadedEvent } from "./RemoteSkillDownloadedEvent"; import type { RequestUserInputEvent } from "./RequestUserInputEvent"; import type { ReviewRequest } from "./ReviewRequest"; import type { SessionConfiguredEvent } from "./SessionConfiguredEvent"; -import type { SkillRequestApprovalEvent } from "./SkillRequestApprovalEvent"; import type { StreamErrorEvent } from "./StreamErrorEvent"; import type { TerminalInteractionEvent } from "./TerminalInteractionEvent"; import type { ThreadNameUpdatedEvent } from "./ThreadNameUpdatedEvent"; @@ -80,4 +79,4 @@ import type { WebSearchEndEvent } from "./WebSearchEndEvent"; * Response event from the agent * NOTE: Make sure none of these values have optional types, as it will mess up the extension code-gen. */ -export type EventMsg = { "type": "error" } & ErrorEvent | { "type": "warning" } & WarningEvent | { "type": "realtime_conversation_started" } & RealtimeConversationStartedEvent | { "type": "realtime_conversation_realtime" } & RealtimeConversationRealtimeEvent | { "type": "realtime_conversation_closed" } & RealtimeConversationClosedEvent | { "type": "model_reroute" } & ModelRerouteEvent | { "type": "context_compacted" } & ContextCompactedEvent | { "type": "thread_rolled_back" } & ThreadRolledBackEvent | { "type": "task_started" } & TurnStartedEvent | { "type": "task_complete" } & TurnCompleteEvent | { "type": "token_count" } & TokenCountEvent | { "type": "agent_message" } & AgentMessageEvent | { "type": "user_message" } & UserMessageEvent | { "type": "agent_message_delta" } & AgentMessageDeltaEvent | { "type": "agent_reasoning" } & AgentReasoningEvent | { "type": "agent_reasoning_delta" } & AgentReasoningDeltaEvent | { "type": "agent_reasoning_raw_content" } & AgentReasoningRawContentEvent | { "type": "agent_reasoning_raw_content_delta" } & AgentReasoningRawContentDeltaEvent | { "type": "agent_reasoning_section_break" } & AgentReasoningSectionBreakEvent | { "type": "session_configured" } & SessionConfiguredEvent | { "type": "thread_name_updated" } & ThreadNameUpdatedEvent | { "type": "mcp_startup_update" } & McpStartupUpdateEvent | { "type": "mcp_startup_complete" } & McpStartupCompleteEvent | { "type": "mcp_tool_call_begin" } & McpToolCallBeginEvent | { "type": "mcp_tool_call_end" } & McpToolCallEndEvent | { "type": "web_search_begin" } & WebSearchBeginEvent | { "type": "web_search_end" } & WebSearchEndEvent | { "type": "exec_command_begin" } & ExecCommandBeginEvent | { "type": "exec_command_output_delta" } & ExecCommandOutputDeltaEvent | { "type": "terminal_interaction" } & TerminalInteractionEvent | { "type": "exec_command_end" } & ExecCommandEndEvent | { "type": "view_image_tool_call" } & ViewImageToolCallEvent | { "type": "exec_approval_request" } & ExecApprovalRequestEvent | { "type": "request_user_input" } & RequestUserInputEvent | { "type": "dynamic_tool_call_request" } & DynamicToolCallRequest | { "type": "dynamic_tool_call_response" } & DynamicToolCallResponseEvent | { "type": "skill_request_approval" } & SkillRequestApprovalEvent | { "type": "elicitation_request" } & ElicitationRequestEvent | { "type": "apply_patch_approval_request" } & ApplyPatchApprovalRequestEvent | { "type": "deprecation_notice" } & DeprecationNoticeEvent | { "type": "background_event" } & BackgroundEventEvent | { "type": "undo_started" } & UndoStartedEvent | { "type": "undo_completed" } & UndoCompletedEvent | { "type": "stream_error" } & StreamErrorEvent | { "type": "patch_apply_begin" } & PatchApplyBeginEvent | { "type": "patch_apply_end" } & PatchApplyEndEvent | { "type": "turn_diff" } & TurnDiffEvent | { "type": "get_history_entry_response" } & GetHistoryEntryResponseEvent | { "type": "mcp_list_tools_response" } & McpListToolsResponseEvent | { "type": "list_custom_prompts_response" } & ListCustomPromptsResponseEvent | { "type": "list_skills_response" } & ListSkillsResponseEvent | { "type": "list_remote_skills_response" } & ListRemoteSkillsResponseEvent | { "type": "remote_skill_downloaded" } & RemoteSkillDownloadedEvent | { "type": "skills_update_available" } | { "type": "plan_update" } & UpdatePlanArgs | { "type": "turn_aborted" } & TurnAbortedEvent | { "type": "shutdown_complete" } | { "type": "entered_review_mode" } & ReviewRequest | { "type": "exited_review_mode" } & ExitedReviewModeEvent | { "type": "raw_response_item" } & RawResponseItemEvent | { "type": "item_started" } & ItemStartedEvent | { "type": "item_completed" } & ItemCompletedEvent | { "type": "agent_message_content_delta" } & AgentMessageContentDeltaEvent | { "type": "plan_delta" } & PlanDeltaEvent | { "type": "reasoning_content_delta" } & ReasoningContentDeltaEvent | { "type": "reasoning_raw_content_delta" } & ReasoningRawContentDeltaEvent | { "type": "collab_agent_spawn_begin" } & CollabAgentSpawnBeginEvent | { "type": "collab_agent_spawn_end" } & CollabAgentSpawnEndEvent | { "type": "collab_agent_interaction_begin" } & CollabAgentInteractionBeginEvent | { "type": "collab_agent_interaction_end" } & CollabAgentInteractionEndEvent | { "type": "collab_waiting_begin" } & CollabWaitingBeginEvent | { "type": "collab_waiting_end" } & CollabWaitingEndEvent | { "type": "collab_close_begin" } & CollabCloseBeginEvent | { "type": "collab_close_end" } & CollabCloseEndEvent | { "type": "collab_resume_begin" } & CollabResumeBeginEvent | { "type": "collab_resume_end" } & CollabResumeEndEvent; +export type EventMsg = { "type": "error" } & ErrorEvent | { "type": "warning" } & WarningEvent | { "type": "realtime_conversation_started" } & RealtimeConversationStartedEvent | { "type": "realtime_conversation_realtime" } & RealtimeConversationRealtimeEvent | { "type": "realtime_conversation_closed" } & RealtimeConversationClosedEvent | { "type": "model_reroute" } & ModelRerouteEvent | { "type": "context_compacted" } & ContextCompactedEvent | { "type": "thread_rolled_back" } & ThreadRolledBackEvent | { "type": "task_started" } & TurnStartedEvent | { "type": "task_complete" } & TurnCompleteEvent | { "type": "token_count" } & TokenCountEvent | { "type": "agent_message" } & AgentMessageEvent | { "type": "user_message" } & UserMessageEvent | { "type": "agent_message_delta" } & AgentMessageDeltaEvent | { "type": "agent_reasoning" } & AgentReasoningEvent | { "type": "agent_reasoning_delta" } & AgentReasoningDeltaEvent | { "type": "agent_reasoning_raw_content" } & AgentReasoningRawContentEvent | { "type": "agent_reasoning_raw_content_delta" } & AgentReasoningRawContentDeltaEvent | { "type": "agent_reasoning_section_break" } & AgentReasoningSectionBreakEvent | { "type": "session_configured" } & SessionConfiguredEvent | { "type": "thread_name_updated" } & ThreadNameUpdatedEvent | { "type": "mcp_startup_update" } & McpStartupUpdateEvent | { "type": "mcp_startup_complete" } & McpStartupCompleteEvent | { "type": "mcp_tool_call_begin" } & McpToolCallBeginEvent | { "type": "mcp_tool_call_end" } & McpToolCallEndEvent | { "type": "web_search_begin" } & WebSearchBeginEvent | { "type": "web_search_end" } & WebSearchEndEvent | { "type": "exec_command_begin" } & ExecCommandBeginEvent | { "type": "exec_command_output_delta" } & ExecCommandOutputDeltaEvent | { "type": "terminal_interaction" } & TerminalInteractionEvent | { "type": "exec_command_end" } & ExecCommandEndEvent | { "type": "view_image_tool_call" } & ViewImageToolCallEvent | { "type": "exec_approval_request" } & ExecApprovalRequestEvent | { "type": "request_user_input" } & RequestUserInputEvent | { "type": "dynamic_tool_call_request" } & DynamicToolCallRequest | { "type": "dynamic_tool_call_response" } & DynamicToolCallResponseEvent | { "type": "elicitation_request" } & ElicitationRequestEvent | { "type": "apply_patch_approval_request" } & ApplyPatchApprovalRequestEvent | { "type": "deprecation_notice" } & DeprecationNoticeEvent | { "type": "background_event" } & BackgroundEventEvent | { "type": "undo_started" } & UndoStartedEvent | { "type": "undo_completed" } & UndoCompletedEvent | { "type": "stream_error" } & StreamErrorEvent | { "type": "patch_apply_begin" } & PatchApplyBeginEvent | { "type": "patch_apply_end" } & PatchApplyEndEvent | { "type": "turn_diff" } & TurnDiffEvent | { "type": "get_history_entry_response" } & GetHistoryEntryResponseEvent | { "type": "mcp_list_tools_response" } & McpListToolsResponseEvent | { "type": "list_custom_prompts_response" } & ListCustomPromptsResponseEvent | { "type": "list_skills_response" } & ListSkillsResponseEvent | { "type": "list_remote_skills_response" } & ListRemoteSkillsResponseEvent | { "type": "remote_skill_downloaded" } & RemoteSkillDownloadedEvent | { "type": "skills_update_available" } | { "type": "plan_update" } & UpdatePlanArgs | { "type": "turn_aborted" } & TurnAbortedEvent | { "type": "shutdown_complete" } | { "type": "entered_review_mode" } & ReviewRequest | { "type": "exited_review_mode" } & ExitedReviewModeEvent | { "type": "raw_response_item" } & RawResponseItemEvent | { "type": "item_started" } & ItemStartedEvent | { "type": "item_completed" } & ItemCompletedEvent | { "type": "agent_message_content_delta" } & AgentMessageContentDeltaEvent | { "type": "plan_delta" } & PlanDeltaEvent | { "type": "reasoning_content_delta" } & ReasoningContentDeltaEvent | { "type": "reasoning_raw_content_delta" } & ReasoningRawContentDeltaEvent | { "type": "collab_agent_spawn_begin" } & CollabAgentSpawnBeginEvent | { "type": "collab_agent_spawn_end" } & CollabAgentSpawnEndEvent | { "type": "collab_agent_interaction_begin" } & CollabAgentInteractionBeginEvent | { "type": "collab_agent_interaction_end" } & CollabAgentInteractionEndEvent | { "type": "collab_waiting_begin" } & CollabWaitingBeginEvent | { "type": "collab_waiting_end" } & CollabWaitingEndEvent | { "type": "collab_close_begin" } & CollabCloseBeginEvent | { "type": "collab_close_end" } & CollabCloseEndEvent | { "type": "collab_resume_begin" } & CollabResumeBeginEvent | { "type": "collab_resume_end" } & CollabResumeEndEvent; diff --git a/codex-rs/app-server-protocol/schema/typescript/ServerRequest.ts b/codex-rs/app-server-protocol/schema/typescript/ServerRequest.ts index 1f1bc2c77e..17c66959aa 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ServerRequest.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ServerRequest.ts @@ -8,10 +8,9 @@ import type { ChatgptAuthTokensRefreshParams } from "./v2/ChatgptAuthTokensRefre import type { CommandExecutionRequestApprovalParams } from "./v2/CommandExecutionRequestApprovalParams"; import type { DynamicToolCallParams } from "./v2/DynamicToolCallParams"; import type { FileChangeRequestApprovalParams } from "./v2/FileChangeRequestApprovalParams"; -import type { SkillRequestApprovalParams } from "./v2/SkillRequestApprovalParams"; import type { ToolRequestUserInputParams } from "./v2/ToolRequestUserInputParams"; /** * Request initiated from the server and sent to the client. */ -export type ServerRequest = { "method": "item/commandExecution/requestApproval", id: RequestId, params: CommandExecutionRequestApprovalParams, } | { "method": "item/fileChange/requestApproval", id: RequestId, params: FileChangeRequestApprovalParams, } | { "method": "item/tool/requestUserInput", id: RequestId, params: ToolRequestUserInputParams, } | { "method": "skill/requestApproval", id: RequestId, params: SkillRequestApprovalParams, } | { "method": "item/tool/call", id: RequestId, params: DynamicToolCallParams, } | { "method": "account/chatgptAuthTokens/refresh", id: RequestId, params: ChatgptAuthTokensRefreshParams, } | { "method": "applyPatchApproval", id: RequestId, params: ApplyPatchApprovalParams, } | { "method": "execCommandApproval", id: RequestId, params: ExecCommandApprovalParams, }; +export type ServerRequest = { "method": "item/commandExecution/requestApproval", id: RequestId, params: CommandExecutionRequestApprovalParams, } | { "method": "item/fileChange/requestApproval", id: RequestId, params: FileChangeRequestApprovalParams, } | { "method": "item/tool/requestUserInput", id: RequestId, params: ToolRequestUserInputParams, } | { "method": "item/tool/call", id: RequestId, params: DynamicToolCallParams, } | { "method": "account/chatgptAuthTokens/refresh", id: RequestId, params: ChatgptAuthTokensRefreshParams, } | { "method": "applyPatchApproval", id: RequestId, params: ApplyPatchApprovalParams, } | { "method": "execCommandApproval", id: RequestId, params: ExecCommandApprovalParams, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/SkillRequestApprovalEvent.ts b/codex-rs/app-server-protocol/schema/typescript/SkillRequestApprovalEvent.ts deleted file mode 100644 index 6699eadac3..0000000000 --- a/codex-rs/app-server-protocol/schema/typescript/SkillRequestApprovalEvent.ts +++ /dev/null @@ -1,5 +0,0 @@ -// 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. - -export type SkillRequestApprovalEvent = { item_id: string, skill_name: string, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/index.ts b/codex-rs/app-server-protocol/schema/typescript/index.ts index a954eecd28..bc5b0297b8 100644 --- a/codex-rs/app-server-protocol/schema/typescript/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/index.ts @@ -209,7 +209,6 @@ export type { SkillDependencies } from "./SkillDependencies"; export type { SkillErrorInfo } from "./SkillErrorInfo"; export type { SkillInterface } from "./SkillInterface"; export type { SkillMetadata } from "./SkillMetadata"; -export type { SkillRequestApprovalEvent } from "./SkillRequestApprovalEvent"; export type { SkillScope } from "./SkillScope"; export type { SkillToolDependency } from "./SkillToolDependency"; export type { SkillsListEntry } from "./SkillsListEntry"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/SkillApprovalDecision.ts b/codex-rs/app-server-protocol/schema/typescript/v2/SkillApprovalDecision.ts deleted file mode 100644 index 6773b0f1e0..0000000000 --- a/codex-rs/app-server-protocol/schema/typescript/v2/SkillApprovalDecision.ts +++ /dev/null @@ -1,5 +0,0 @@ -// 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. - -export type SkillApprovalDecision = "approve" | "decline"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/SkillRequestApprovalParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/SkillRequestApprovalParams.ts deleted file mode 100644 index 3f70fea595..0000000000 --- a/codex-rs/app-server-protocol/schema/typescript/v2/SkillRequestApprovalParams.ts +++ /dev/null @@ -1,5 +0,0 @@ -// 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. - -export type SkillRequestApprovalParams = { itemId: string, skillName: string, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/SkillRequestApprovalResponse.ts b/codex-rs/app-server-protocol/schema/typescript/v2/SkillRequestApprovalResponse.ts deleted file mode 100644 index 9ce454ff4f..0000000000 --- a/codex-rs/app-server-protocol/schema/typescript/v2/SkillRequestApprovalResponse.ts +++ /dev/null @@ -1,6 +0,0 @@ -// 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. -import type { SkillApprovalDecision } from "./SkillApprovalDecision"; - -export type SkillRequestApprovalResponse = { decision: SkillApprovalDecision, }; 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 19594723db..4c638c7d30 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -141,13 +141,10 @@ export type { SandboxMode } from "./SandboxMode"; export type { SandboxPolicy } from "./SandboxPolicy"; export type { SandboxWorkspaceWrite } from "./SandboxWorkspaceWrite"; export type { SessionSource } from "./SessionSource"; -export type { SkillApprovalDecision } from "./SkillApprovalDecision"; export type { SkillDependencies } from "./SkillDependencies"; export type { SkillErrorInfo } from "./SkillErrorInfo"; export type { SkillInterface } from "./SkillInterface"; export type { SkillMetadata } from "./SkillMetadata"; -export type { SkillRequestApprovalParams } from "./SkillRequestApprovalParams"; -export type { SkillRequestApprovalResponse } from "./SkillRequestApprovalResponse"; export type { SkillScope } from "./SkillScope"; export type { SkillToolDependency } from "./SkillToolDependency"; export type { SkillsConfigWriteParams } from "./SkillsConfigWriteParams"; diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 357341cc4b..df969e26cd 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -715,11 +715,6 @@ server_request_definitions! { response: v2::ToolRequestUserInputResponse, }, - SkillRequestApproval => "skill/requestApproval" { - params: v2::SkillRequestApprovalParams, - response: v2::SkillRequestApprovalResponse, - }, - /// Execute a dynamic tool call on the client. DynamicToolCall => "item/tool/call" { params: v2::DynamicToolCallParams, diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index d563287fa8..62bb0d8991 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -3842,29 +3842,6 @@ pub struct FileChangeRequestApprovalResponse { pub decision: FileChangeApprovalDecision, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS, ExperimentalApi)] -#[serde(rename_all = "camelCase")] -#[ts(export_to = "v2/")] -pub struct SkillRequestApprovalParams { - pub item_id: String, - pub skill_name: String, -} - -#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)] -#[serde(rename_all = "camelCase")] -#[ts(export_to = "v2/")] -pub enum SkillApprovalDecision { - Approve, - Decline, -} - -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] -#[serde(rename_all = "camelCase")] -#[ts(export_to = "v2/")] -pub struct SkillRequestApprovalResponse { - pub decision: SkillApprovalDecision, -} - #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] diff --git a/codex-rs/app-server-test-client/src/lib.rs b/codex-rs/app-server-test-client/src/lib.rs index a9225b0426..a66f6bf664 100644 --- a/codex-rs/app-server-test-client/src/lib.rs +++ b/codex-rs/app-server-test-client/src/lib.rs @@ -57,9 +57,6 @@ use codex_app_server_protocol::SendUserMessageParams; use codex_app_server_protocol::SendUserMessageResponse; use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ServerRequest; -use codex_app_server_protocol::SkillApprovalDecision; -use codex_app_server_protocol::SkillRequestApprovalParams; -use codex_app_server_protocol::SkillRequestApprovalResponse; use codex_app_server_protocol::ThreadItem; use codex_app_server_protocol::ThreadListParams; use codex_app_server_protocol::ThreadListResponse; @@ -1511,9 +1508,6 @@ impl CodexClient { ServerRequest::FileChangeRequestApproval { request_id, params } => { self.approve_file_change_request(request_id, params)?; } - ServerRequest::SkillRequestApproval { request_id, params } => { - self.approve_skill_request(request_id, params)?; - } other => { bail!("received unsupported server request: {other:?}"); } @@ -1593,22 +1587,6 @@ impl CodexClient { Ok(()) } - fn approve_skill_request( - &mut self, - request_id: RequestId, - params: SkillRequestApprovalParams, - ) -> Result<()> { - println!( - "\n< skill approval requested for item {}, skill {}", - params.item_id, params.skill_name - ); - let response = SkillRequestApprovalResponse { - decision: SkillApprovalDecision::Approve, - }; - self.send_server_request_response(request_id, &response)?; - Ok(()) - } - fn approve_file_change_request( &mut self, request_id: RequestId, diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 683fca361d..fc8c109711 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -58,9 +58,6 @@ use codex_app_server_protocol::ReasoningSummaryTextDeltaNotification; use codex_app_server_protocol::ReasoningTextDeltaNotification; use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ServerRequestPayload; -use codex_app_server_protocol::SkillApprovalDecision as V2SkillApprovalDecision; -use codex_app_server_protocol::SkillRequestApprovalParams; -use codex_app_server_protocol::SkillRequestApprovalResponse; use codex_app_server_protocol::TerminalInteractionNotification; use codex_app_server_protocol::ThreadItem; use codex_app_server_protocol::ThreadNameUpdatedNotification; @@ -111,7 +108,6 @@ use codex_protocol::protocol::TokenCountEvent; use codex_protocol::protocol::TurnDiffEvent; use codex_protocol::request_user_input::RequestUserInputAnswer as CoreRequestUserInputAnswer; use codex_protocol::request_user_input::RequestUserInputResponse as CoreRequestUserInputResponse; -use codex_protocol::skill_approval::SkillApprovalResponse as CoreSkillApprovalResponse; use codex_shell_command::parse_command::shlex_join; use std::collections::HashMap; use std::convert::TryFrom; @@ -518,37 +514,6 @@ pub(crate) async fn apply_bespoke_event_handling( } } } - EventMsg::SkillRequestApproval(request) => { - if matches!(api_version, ApiVersion::V2) { - let item_id = request.item_id; - let skill_name = request.skill_name; - let params = SkillRequestApprovalParams { - item_id: item_id.clone(), - skill_name, - }; - let rx = outgoing - .send_request(ServerRequestPayload::SkillRequestApproval(params)) - .await; - tokio::spawn(async move { - let approved = match rx.await { - Ok(Ok(value)) => { - serde_json::from_value::(value) - .map(|response| { - matches!(response.decision, V2SkillApprovalDecision::Approve) - }) - .unwrap_or(false) - } - _ => false, - }; - let _ = conversation - .submit(Op::SkillApproval { - id: item_id, - response: CoreSkillApprovalResponse { approved }, - }) - .await; - }); - } - } EventMsg::DynamicToolCallRequest(request) => { if matches!(api_version, ApiVersion::V2) { let call_id = request.call_id; diff --git a/codex-rs/app-server/tests/suite/v2/mod.rs b/codex-rs/app-server/tests/suite/v2/mod.rs index 2444130978..cb3acb633d 100644 --- a/codex-rs/app-server/tests/suite/v2/mod.rs +++ b/codex-rs/app-server/tests/suite/v2/mod.rs @@ -19,7 +19,6 @@ mod realtime_conversation; mod request_user_input; mod review; mod safety_check_downgrade; -mod skill_approval; mod skills_list; mod thread_archive; mod thread_fork; diff --git a/codex-rs/app-server/tests/suite/v2/skill_approval.rs b/codex-rs/app-server/tests/suite/v2/skill_approval.rs deleted file mode 100644 index 945a545a8d..0000000000 --- a/codex-rs/app-server/tests/suite/v2/skill_approval.rs +++ /dev/null @@ -1,109 +0,0 @@ -use anyhow::Result; -use app_test_support::McpProcess; -use app_test_support::create_final_assistant_message_sse_response; -use app_test_support::create_mock_responses_server_sequence; -use app_test_support::to_response; -use codex_app_server_protocol::JSONRPCResponse; -use codex_app_server_protocol::RequestId; -use codex_app_server_protocol::ServerRequest; -use codex_app_server_protocol::ThreadStartParams; -use codex_app_server_protocol::ThreadStartResponse; -use codex_app_server_protocol::TurnStartParams; -use codex_app_server_protocol::TurnStartResponse; -use codex_app_server_protocol::UserInput as V2UserInput; -use tokio::time::timeout; - -const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); - -#[tokio::test(flavor = "multi_thread", worker_threads = 4)] -async fn skill_request_approval_round_trip() -> Result<()> { - let codex_home = tempfile::TempDir::new()?; - let server = - create_mock_responses_server_sequence(vec![create_final_assistant_message_sse_response( - "done", - )?]) - .await; - create_config_toml(codex_home.path(), &server.uri())?; - - let mut mcp = McpProcess::new(codex_home.path()).await?; - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - - let thread_start_id = mcp - .send_thread_start_request(ThreadStartParams { - model: Some("mock-model".to_string()), - ..Default::default() - }) - .await?; - let thread_start_resp: JSONRPCResponse = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(thread_start_id)), - ) - .await??; - let ThreadStartResponse { thread, .. } = to_response(thread_start_resp)?; - - let turn_start_id = mcp - .send_turn_start_request(TurnStartParams { - thread_id: thread.id.clone(), - input: vec![V2UserInput::Text { - text: "ask something".to_string(), - text_elements: Vec::new(), - }], - model: Some("mock-model".to_string()), - ..Default::default() - }) - .await?; - let turn_start_resp: JSONRPCResponse = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(turn_start_id)), - ) - .await??; - let TurnStartResponse { turn, .. } = to_response(turn_start_resp)?; - - let server_req = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_request_message(), - ) - .await??; - let ServerRequest::SkillRequestApproval { request_id, params } = server_req else { - panic!("expected SkillRequestApproval request, got: {server_req:?}"); - }; - - assert_eq!(params.item_id, turn.id); - assert_eq!(params.skill_name, "test-skill"); - - mcp.send_response(request_id, serde_json::json!({ "decision": "approve" })) - .await?; - - timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_notification_message("turn/completed"), - ) - .await??; - - Ok(()) -} - -fn create_config_toml(codex_home: &std::path::Path, server_uri: &str) -> std::io::Result<()> { - let config_toml = codex_home.join("config.toml"); - std::fs::write( - config_toml, - format!( - r#" -model = "mock-model" -approval_policy = "never" -sandbox_mode = "read-only" - -model_provider = "mock_provider" - -[features] -skill_approval = true - -[model_providers.mock_provider] -name = "Mock provider for test" -base_url = "{server_uri}/v1" -request_max_retries = 0 -stream_max_retries = 0 -"# - ), - ) -} diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 6c4c78fdf1..e1f0fa5147 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -409,9 +409,6 @@ "shell_zsh_fork": { "type": "boolean" }, - "skill_approval": { - "type": "boolean" - }, "skill_env_var_dependency_prompt": { "type": "boolean" }, @@ -1705,9 +1702,6 @@ "shell_zsh_fork": { "type": "boolean" }, - "skill_approval": { - "type": "boolean" - }, "skill_env_var_dependency_prompt": { "type": "boolean" }, diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index a63aa24148..26e6b339e6 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -94,7 +94,6 @@ use codex_protocol::protocol::TurnContextNetworkItem; use codex_protocol::protocol::TurnStartedEvent; use codex_protocol::request_user_input::RequestUserInputArgs; use codex_protocol::request_user_input::RequestUserInputResponse; -use codex_protocol::skill_approval::SkillApprovalResponse; use codex_rmcp_client::ElicitationResponse; use codex_rmcp_client::OAuthCredentialsStoreMode; use codex_utils_stream_parser::AssistantTextChunk; @@ -2723,40 +2722,6 @@ impl Session { rx_response.await.ok() } - pub async fn request_skill_approval( - &self, - turn_context: &TurnContext, - item_id: String, - skill_name: String, - ) -> Option { - let (tx_response, rx_response) = oneshot::channel(); - let prev_entry = { - let mut active = self.active_turn.lock().await; - match active.as_mut() { - Some(at) => { - let mut ts = at.turn_state.lock().await; - ts.insert_pending_skill_approval(item_id.clone(), tx_response) - } - None => None, - } - }; - if prev_entry.is_some() { - warn!("Overwriting existing pending skill approval for item_id: {item_id}"); - } - - self.send_event( - turn_context, - EventMsg::SkillRequestApproval( - codex_protocol::skill_approval::SkillRequestApprovalEvent { - item_id, - skill_name, - }, - ), - ) - .await; - rx_response.await.ok() - } - pub async fn notify_user_input_response( &self, sub_id: &str, @@ -2803,31 +2768,6 @@ impl Session { } } - pub async fn notify_skill_approval_response( - &self, - item_id: &str, - response: SkillApprovalResponse, - ) { - let entry = { - let mut active = self.active_turn.lock().await; - match active.as_mut() { - Some(at) => { - let mut ts = at.turn_state.lock().await; - ts.remove_pending_skill_approval(item_id) - } - None => None, - } - }; - match entry { - Some(tx_response) => { - tx_response.send(response).ok(); - } - None => { - warn!("No pending skill approval found for item_id: {item_id}"); - } - } - } - pub async fn notify_approval(&self, approval_id: &str, decision: ReviewDecision) { let entry = { let mut active = self.active_turn.lock().await; @@ -3731,9 +3671,6 @@ async fn submission_loop(sess: Arc, config: Arc, rx_sub: Receiv Op::DynamicToolResponse { id, response } => { handlers::dynamic_tool_response(&sess, id, response).await; } - Op::SkillApproval { id, response } => { - handlers::skill_approval_response(&sess, id, response).await; - } Op::AddToHistory { text } => { handlers::add_to_history(&sess, &config, text).await; } @@ -3857,7 +3794,6 @@ mod handlers { use codex_protocol::protocol::TurnAbortReason; use codex_protocol::protocol::WarningEvent; use codex_protocol::request_user_input::RequestUserInputResponse; - use codex_protocol::skill_approval::SkillApprovalResponse; use crate::context_manager::is_user_turn_boundary; use codex_protocol::config_types::CollaborationMode; @@ -4098,14 +4034,6 @@ mod handlers { sess.notify_dynamic_tool_response(&id, response).await; } - pub async fn skill_approval_response( - sess: &Arc, - id: String, - response: SkillApprovalResponse, - ) { - sess.notify_skill_approval_response(&id, response).await; - } - pub async fn add_to_history(sess: &Arc, config: &Arc, text: String) { let id = sess.conversation_id; let config = Arc::clone(config); @@ -4819,15 +4747,6 @@ pub(crate) async fn run_turn( collaboration_mode_kind: turn_context.collaboration_mode.mode, }); sess.send_event(&turn_context, event).await; - if turn_context.config.features.enabled(Feature::SkillApproval) { - let _ = sess - .request_skill_approval( - turn_context.as_ref(), - turn_context.sub_id.clone(), - "test-skill".to_string(), - ) - .await; - } // TODO(ccunningham): Pre-turn compaction runs before context updates and the // new user message are recorded. Estimate pending incoming items (context // diffs/full reinjection + user input) and trigger compaction preemptively @@ -5856,7 +5775,6 @@ fn realtime_text_for_event(msg: &EventMsg) -> Option { | EventMsg::RequestUserInput(_) | EventMsg::DynamicToolCallRequest(_) | EventMsg::DynamicToolCallResponse(_) - | EventMsg::SkillRequestApproval(_) | EventMsg::ElicitationRequest(_) | EventMsg::ApplyPatchApprovalRequest(_) | EventMsg::DeprecationNotice(_) diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index cc65ad572f..a4dd81c640 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -133,8 +133,6 @@ pub enum Feature { SkillMcpDependencyInstall, /// Prompt for missing skill env var dependencies. SkillEnvVarDependencyPrompt, - /// Emit skill approval test prompts/events. - SkillApproval, /// Steer feature flag - when enabled, Enter submits immediately instead of queuing. /// Kept for config backward compatibility; behavior is always steer-enabled. Steer, @@ -630,12 +628,6 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::UnderDevelopment, default_enabled: false, }, - FeatureSpec { - id: Feature::SkillApproval, - key: "skill_approval", - stage: Stage::UnderDevelopment, - default_enabled: false, - }, FeatureSpec { id: Feature::Steer, key: "steer", diff --git a/codex-rs/core/src/rollout/policy.rs b/codex-rs/core/src/rollout/policy.rs index a1e77ba4fc..4c76bc54f6 100644 --- a/codex-rs/core/src/rollout/policy.rs +++ b/codex-rs/core/src/rollout/policy.rs @@ -141,7 +141,6 @@ fn event_msg_persistence_mode(ev: &EventMsg) -> Option { | EventMsg::TerminalInteraction(_) | EventMsg::ExecCommandOutputDelta(_) | EventMsg::ExecApprovalRequest(_) - | EventMsg::SkillRequestApproval(_) | EventMsg::RequestUserInput(_) | EventMsg::ElicitationRequest(_) | EventMsg::ApplyPatchApprovalRequest(_) diff --git a/codex-rs/core/src/state/turn.rs b/codex-rs/core/src/state/turn.rs index 3301740866..ccc50d066b 100644 --- a/codex-rs/core/src/state/turn.rs +++ b/codex-rs/core/src/state/turn.rs @@ -11,7 +11,6 @@ use tokio_util::task::AbortOnDropHandle; use codex_protocol::dynamic_tools::DynamicToolResponse; use codex_protocol::models::ResponseInputItem; use codex_protocol::request_user_input::RequestUserInputResponse; -use codex_protocol::skill_approval::SkillApprovalResponse; use tokio::sync::oneshot; use crate::codex::TurnContext; @@ -72,7 +71,6 @@ impl ActiveTurn { pub(crate) struct TurnState { pending_approvals: HashMap>, pending_user_input: HashMap>, - pending_skill_approvals: HashMap>, pending_dynamic_tools: HashMap>, pending_input: Vec, } @@ -96,7 +94,6 @@ impl TurnState { pub(crate) fn clear_pending(&mut self) { self.pending_approvals.clear(); self.pending_user_input.clear(); - self.pending_skill_approvals.clear(); self.pending_dynamic_tools.clear(); self.pending_input.clear(); } @@ -124,21 +121,6 @@ impl TurnState { self.pending_dynamic_tools.insert(key, tx) } - pub(crate) fn insert_pending_skill_approval( - &mut self, - key: String, - tx: oneshot::Sender, - ) -> Option> { - self.pending_skill_approvals.insert(key, tx) - } - - pub(crate) fn remove_pending_skill_approval( - &mut self, - key: &str, - ) -> Option> { - self.pending_skill_approvals.remove(key) - } - pub(crate) fn remove_pending_dynamic_tool( &mut self, key: &str, diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 4cfe955b55..1509428f38 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -114,7 +114,6 @@ mod seatbelt; mod shell_command; mod shell_serialization; mod shell_snapshot; -mod skill_approval; mod skills; mod sqlite_state; mod stream_error_allows_next_turn; diff --git a/codex-rs/core/tests/suite/skill_approval.rs b/codex-rs/core/tests/suite/skill_approval.rs index 8abd8f5924..891f483d02 100644 --- a/codex-rs/core/tests/suite/skill_approval.rs +++ b/codex-rs/core/tests/suite/skill_approval.rs @@ -64,17 +64,6 @@ async fn submit_turn_with_policies( Ok(()) } -async fn wait_for_turn_complete_without_skill_approval(test: &TestCodex) { - wait_for_event_match(test.codex.as_ref(), |event| match event { - EventMsg::SkillRequestApproval(request) => { - panic!("unexpected skill approval request: {request:?}"); - } - EventMsg::TurnComplete(_) => Some(()), - _ => None, - }) - .await; -} - fn write_skill_with_shell_script(home: &Path, name: &str, script_name: &str) -> Result { use std::os::unix::fs::PermissionsExt; diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index eac514a8f1..a3a29852b3 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -854,8 +854,7 @@ impl EventProcessor for EventProcessorWithHumanOutput { | EventMsg::RealtimeConversationRealtime(_) | EventMsg::RealtimeConversationClosed(_) | EventMsg::DynamicToolCallRequest(_) - | EventMsg::DynamicToolCallResponse(_) - | EventMsg::SkillRequestApproval(_) => {} + | EventMsg::DynamicToolCallResponse(_) => {} } CodexStatus::Running } diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index 805ac0e6ea..cb204fa12b 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -364,7 +364,6 @@ async fn run_codex_tool_session_inner( | EventMsg::RequestUserInput(_) | EventMsg::DynamicToolCallRequest(_) | EventMsg::DynamicToolCallResponse(_) - | EventMsg::SkillRequestApproval(_) | EventMsg::ContextCompacted(_) | EventMsg::ModelReroute(_) | EventMsg::ThreadRolledBack(_) diff --git a/codex-rs/protocol/src/lib.rs b/codex-rs/protocol/src/lib.rs index c942a717d7..5841b1187e 100644 --- a/codex-rs/protocol/src/lib.rs +++ b/codex-rs/protocol/src/lib.rs @@ -15,5 +15,4 @@ pub mod parse_command; pub mod plan_tool; pub mod protocol; pub mod request_user_input; -pub mod skill_approval; pub mod user_input; diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 0e2e86f8ca..15d85c9c73 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -41,7 +41,6 @@ use crate::openai_models::ReasoningEffort as ReasoningEffortConfig; use crate::parse_command::ParsedCommand; use crate::plan_tool::UpdatePlanArgs; use crate::request_user_input::RequestUserInputResponse; -use crate::skill_approval::SkillApprovalResponse; use crate::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; use schemars::JsonSchema; @@ -62,7 +61,6 @@ pub use crate::approvals::NetworkApprovalProtocol; pub use crate::approvals::NetworkPolicyAmendment; pub use crate::approvals::NetworkPolicyRuleAction; pub use crate::request_user_input::RequestUserInputEvent; -pub use crate::skill_approval::SkillRequestApprovalEvent; /// Open/close tags for special user-input blocks. Used across crates to avoid /// duplicated hardcoded strings. @@ -296,14 +294,6 @@ pub enum Op { response: DynamicToolResponse, }, - /// Resolve a skill approval request. - SkillApproval { - /// Item id for the in-flight request. - id: String, - /// User decision. - response: SkillApprovalResponse, - }, - /// Append an entry to the persistent cross-session message history. /// /// Note the entry is not guaranteed to be logged if the user has @@ -1056,8 +1046,6 @@ pub enum EventMsg { DynamicToolCallResponse(DynamicToolCallResponseEvent), - SkillRequestApproval(SkillRequestApprovalEvent), - ElicitationRequest(ElicitationRequestEvent), ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent), diff --git a/codex-rs/protocol/src/skill_approval.rs b/codex-rs/protocol/src/skill_approval.rs deleted file mode 100644 index 152e248186..0000000000 --- a/codex-rs/protocol/src/skill_approval.rs +++ /dev/null @@ -1,15 +0,0 @@ -use schemars::JsonSchema; -use serde::Deserialize; -use serde::Serialize; -use ts_rs::TS; - -#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] -pub struct SkillRequestApprovalEvent { - pub item_id: String, - pub skill_name: String, -} - -#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] -pub struct SkillApprovalResponse { - pub approved: bool, -} diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index a6c7b1fef2..47ad7aad58 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -4455,8 +4455,7 @@ impl ChatWidget { | EventMsg::ReasoningContentDelta(_) | EventMsg::ReasoningRawContentDelta(_) | EventMsg::DynamicToolCallRequest(_) - | EventMsg::DynamicToolCallResponse(_) - | EventMsg::SkillRequestApproval(_) => {} + | EventMsg::DynamicToolCallResponse(_) => {} EventMsg::RealtimeConversationStarted(ev) => { if !from_replay { self.on_realtime_conversation_started(ev); From 14116ade8d8eb8066cb84b65b63f640a3cdd72e7 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 25 Feb 2026 17:10:46 -0800 Subject: [PATCH 02/54] 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 37380f5988..84842fd194 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 2122ad6521..4e2106bbbc 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 04e914a390..0b7986fba9 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 51064047ee..c7bf908748 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 ac15f23658..baedafa403 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 e9cbc2ac1b..ff810643d4 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 52a33c4c02..62850442fc 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 5144dab6e6..cc13400900 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 70cb8e1521..8fb6375e69 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 df969e26cd..3af2976106 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 62bb0d8991..6dba059ebe 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 a66f6bf664..27203c6012 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 505abb0457..20b2dcf2c7 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 fc8c109711..0bc2f1118f 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 8c36df9822..6ac3e0d466 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 26e6b339e6..6566f08486 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 c1fb31ebd1..0777f8e3b2 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 ca31ec4d4c..aeef59833d 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 80d3b22ebb..101d9309fd 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 3b1a0c9067..5bcf809e21 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 4b0c53d123..f4d823bf0e 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 891f483d02..a3cfd4d7fd 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 cb204fa12b..e624fe2b1b 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 f742dc6328..98fe0280e3 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 15d85c9c73..1928f1beea 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 62d459fc6f..6ed8670e07 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 3d30914781..d82d014827 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 fd58baef46..2ea3c10e9d 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 17d885d5f8..e774e9c420 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 47ad7aad58..a4063ed3d6 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 00d4a1df91..48de3e3681 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 { From 40ab71a9852d36745542a7c9367cc67af6fb2df9 Mon Sep 17 00:00:00 2001 From: Curtis 'Fjord' Hawthorne Date: Wed, 25 Feb 2026 17:14:51 -0800 Subject: [PATCH 03/54] Disable js_repl when Node is incompatible at startup (#12824) ## Summary - validate `js_repl` Node compatibility during session startup when the experiment is enabled - if Node is missing or too old, disable `js_repl` and `js_repl_tools_only` for the session before tools and instructions are built - surface that startup disablement to users through the existing startup warning flow instead of only logging it - reuse the same compatibility check in js_repl kernel startup so startup gating and runtime behavior stay aligned - add a regression test that verifies the warning is emitted and that the first advertised tool list omits `js_repl` and `js_repl_reset` when Node is incompatible ## Why Today `js_repl` can be advertised based only on the feature flag, then fail later when the kernel starts. That makes the available tool list inaccurate at the start of a conversation, and users do not get a clear explanation for why the tool is unavailable. This change makes tool availability reflect real startup checks, keeps the advertised tool set stable for the lifetime of the session, and gives users a visible warning when `js_repl` is disabled. ## Testing - `just fmt` - `cargo test -p codex-core --test all js_repl_is_not_advertised_when_startup_node_is_incompatible` --- codex-rs/core/src/codex.rs | 13 ++++ codex-rs/core/src/tools/js_repl/mod.rs | 13 +++- codex-rs/core/tests/suite/js_repl.rs | 104 +++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 4 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 6566f08486..b2145afaed 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -251,6 +251,7 @@ use crate::tools::ToolRouter; use crate::tools::context::SharedTurnDiffTracker; use crate::tools::handlers::SEARCH_TOOL_BM25_TOOL_NAME; use crate::tools::js_repl::JsReplHandle; +use crate::tools::js_repl::resolve_compatible_node; use crate::tools::network_approval::NetworkApprovalService; use crate::tools::network_approval::build_blocked_request_observer; use crate::tools::network_approval::build_network_policy_decider; @@ -341,6 +342,18 @@ impl Codex { config.features.disable(Feature::Collab); } + if config.features.enabled(Feature::JsRepl) + && let Err(err) = resolve_compatible_node(config.js_repl_node_path.as_deref()).await + { + let message = format!( + "Disabled `js_repl` for this session because the configured Node runtime is unavailable or incompatible. {err}" + ); + warn!("{message}"); + config.features.disable(Feature::JsRepl); + config.features.disable(Feature::JsReplToolsOnly); + config.startup_warnings.push(message); + } + let allowed_skills_for_implicit_invocation = loaded_skills.allowed_skills_for_implicit_invocation(); let user_instructions = diff --git a/codex-rs/core/src/tools/js_repl/mod.rs b/codex-rs/core/src/tools/js_repl/mod.rs index 066cb8258c..aaa1ab29d1 100644 --- a/codex-rs/core/src/tools/js_repl/mod.rs +++ b/codex-rs/core/src/tools/js_repl/mod.rs @@ -556,10 +556,7 @@ impl JsReplManager { turn: Arc, thread_id: Option, ) -> Result { - let node_path = resolve_node(self.node_path.as_deref()).ok_or_else(|| { - "Node runtime not found; install Node or set CODEX_JS_REPL_NODE_PATH".to_string() - })?; - ensure_node_version(&node_path).await?; + let node_path = resolve_compatible_node(self.node_path.as_deref()).await?; let kernel_path = self .write_kernel_script() @@ -1321,6 +1318,14 @@ async fn ensure_node_version(node_path: &Path) -> Result<(), String> { Ok(()) } +pub(crate) async fn resolve_compatible_node(config_path: Option<&Path>) -> Result { + let node_path = resolve_node(config_path).ok_or_else(|| { + "Node runtime not found; install Node or set CODEX_JS_REPL_NODE_PATH".to_string() + })?; + ensure_node_version(&node_path).await?; + Ok(node_path) +} + pub(crate) fn resolve_node(config_path: Option<&Path>) -> Option { if let Some(path) = std::env::var_os("CODEX_JS_REPL_NODE_PATH") { let p = PathBuf::from(path); diff --git a/codex-rs/core/tests/suite/js_repl.rs b/codex-rs/core/tests/suite/js_repl.rs index ba2b52fd6d..31dbe6e60a 100644 --- a/codex-rs/core/tests/suite/js_repl.rs +++ b/codex-rs/core/tests/suite/js_repl.rs @@ -2,6 +2,7 @@ use anyhow::Result; use codex_core::features::Feature; +use codex_protocol::protocol::EventMsg; use core_test_support::responses; use core_test_support::responses::ResponseMock; use core_test_support::responses::ResponsesRequest; @@ -12,6 +13,12 @@ use core_test_support::responses::ev_response_created; use core_test_support::responses::sse; use core_test_support::skip_if_no_network; use core_test_support::test_codex::test_codex; +use core_test_support::wait_for_event_match; +use std::fs; +#[cfg(unix)] +use std::os::unix::fs::PermissionsExt; +use std::path::Path; +use tempfile::tempdir; use wiremock::MockServer; fn custom_tool_output_text_and_success( @@ -24,6 +31,45 @@ fn custom_tool_output_text_and_success( (output.unwrap_or_default(), success) } +fn tool_names(body: &serde_json::Value) -> Vec { + body["tools"] + .as_array() + .expect("tools array should be present") + .iter() + .map(|tool| { + tool.get("name") + .and_then(|value| value.as_str()) + .or_else(|| tool.get("type").and_then(|value| value.as_str())) + .expect("tool should have a name or type") + .to_string() + }) + .collect() +} + +fn write_too_old_node_script(dir: &Path) -> Result { + #[cfg(windows)] + { + let path = dir.join("old-node.cmd"); + fs::write(&path, "@echo off\r\necho v0.0.1\r\n")?; + Ok(path) + } + + #[cfg(unix)] + { + let path = dir.join("old-node.sh"); + fs::write(&path, "#!/bin/sh\necho v0.0.1\n")?; + let mut permissions = fs::metadata(&path)?.permissions(); + permissions.set_mode(0o755); + fs::set_permissions(&path, permissions)?; + Ok(path) + } + + #[cfg(not(any(unix, windows)))] + { + anyhow::bail!("unsupported platform for js_repl test fixture"); + } +} + async fn run_js_repl_turn( server: &MockServer, prompt: &str, @@ -54,6 +100,64 @@ async fn run_js_repl_turn( Ok(second_mock) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn js_repl_is_not_advertised_when_startup_node_is_incompatible() -> Result<()> { + skip_if_no_network!(Ok(())); + if std::env::var_os("CODEX_JS_REPL_NODE_PATH").is_some() { + return Ok(()); + } + + let server = responses::start_mock_server().await; + let temp = tempdir()?; + let old_node = write_too_old_node_script(temp.path())?; + + let mut builder = test_codex().with_config(move |config| { + config.features.enable(Feature::JsRepl); + config.js_repl_node_path = Some(old_node); + }); + let test = builder.build(&server).await?; + let warning = wait_for_event_match(&test.codex, |event| match event { + EventMsg::Warning(ev) if ev.message.contains("Disabled `js_repl` for this session") => { + Some(ev.message.clone()) + } + _ => None, + }) + .await; + assert!( + warning.contains("Node runtime"), + "warning should explain the Node compatibility issue: {warning}" + ); + + let request_mock = responses::mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-1", "done"), + ev_completed("resp-1"), + ]), + ) + .await; + + test.submit_turn("hello").await?; + + let body = request_mock.single_request().body_json(); + let tools = tool_names(&body); + assert!( + !tools.iter().any(|tool| tool == "js_repl"), + "js_repl should be omitted when startup validation fails: {tools:?}" + ); + assert!( + !tools.iter().any(|tool| tool == "js_repl_reset"), + "js_repl_reset should be omitted when startup validation fails: {tools:?}" + ); + let instructions = body["instructions"].as_str().unwrap_or_default(); + assert!( + !instructions.contains("## JavaScript REPL (Node)"), + "startup instructions should not mention js_repl when it is disabled: {instructions}" + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn js_repl_persists_top_level_bindings_and_supports_tla() -> Result<()> { skip_if_no_network!(Ok(())); From 2c1f2254278f61232bb98fb14fe51c113516abe9 Mon Sep 17 00:00:00 2001 From: xl-openai Date: Wed, 25 Feb 2026 18:15:27 -0800 Subject: [PATCH 04/54] Clarify device auth login hint (#12813) Mention device auth for remote login --- codex-rs/cli/src/login.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/cli/src/login.rs b/codex-rs/cli/src/login.rs index 42622f25d3..f2fd5a3bdc 100644 --- a/codex-rs/cli/src/login.rs +++ b/codex-rs/cli/src/login.rs @@ -22,7 +22,7 @@ const LOGIN_SUCCESS_MESSAGE: &str = "Successfully logged in"; fn print_login_server_start(actual_port: u16, auth_url: &str) { eprintln!( - "Starting local login server on http://localhost:{actual_port}.\nIf your browser did not open, navigate to this URL to authenticate:\n\n{auth_url}" + "Starting local login server on http://localhost:{actual_port}.\nIf your browser did not open, navigate to this URL to authenticate:\n\n{auth_url}\n\nOn a remote or headless machine? Use `codex login --device-auth` instead." ); } From ea621ae1520f49901aafca5ddfefbc64785f784e Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Wed, 25 Feb 2026 18:18:00 -0800 Subject: [PATCH 05/54] Try fixing windows pipeline (#12848) # External (non-OpenAI) Pull Request Requirements Before opening this Pull Request, please read the dedicated "Contributing" markdown file or your PR may be closed: https://github.com/openai/codex/blob/main/docs/contributing.md If your PR conforms to our contribution guidelines, replace this text with a detailed and high quality description of your changes. Include a link to a bug report or enhancement request. --- .github/workflows/rust-release-windows.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/rust-release-windows.yml b/.github/workflows/rust-release-windows.yml index 85556bc8c9..589aa98719 100644 --- a/.github/workflows/rust-release-windows.yml +++ b/.github/workflows/rust-release-windows.yml @@ -88,6 +88,8 @@ jobs: - name: Cargo build (Windows binaries) shell: bash + env: + RUSTFLAGS: ${{ matrix.target == 'aarch64-pc-windows-msvc' && '-C link-arg=/Gy' || '' }} run: | cargo build --target ${{ matrix.target }} --release --timings ${{ matrix.build_args }} From b65205fb3d9ca9d8dbc3bf626a104fbbc7b3ac33 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Wed, 25 Feb 2026 19:12:19 -0800 Subject: [PATCH 06/54] Attempt 2 to fix release (#12856) --- .github/workflows/rust-release-windows.yml | 2 -- codex-rs/.cargo/config.toml | 8 +++++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/rust-release-windows.yml b/.github/workflows/rust-release-windows.yml index 589aa98719..85556bc8c9 100644 --- a/.github/workflows/rust-release-windows.yml +++ b/.github/workflows/rust-release-windows.yml @@ -88,8 +88,6 @@ jobs: - name: Cargo build (Windows binaries) shell: bash - env: - RUSTFLAGS: ${{ matrix.target == 'aarch64-pc-windows-msvc' && '-C link-arg=/Gy' || '' }} run: | cargo build --target ${{ matrix.target }} --release --timings ${{ matrix.build_args }} diff --git a/codex-rs/.cargo/config.toml b/codex-rs/.cargo/config.toml index f5107cf73e..7adeec500b 100644 --- a/codex-rs/.cargo/config.toml +++ b/codex-rs/.cargo/config.toml @@ -1,5 +1,11 @@ [target.'cfg(all(windows, target_env = "msvc"))'] -rustflags = ["-C", "link-arg=/STACK:8388608"] +rustflags = [ + "-C", + "link-arg=/STACK:8388608", + "-C", + "llvm-args=-function-sections", +] + [target.'cfg(all(windows, target_env = "gnu"))'] rustflags = ["-C", "link-arg=-Wl,--stack,8388608"] From 8cdee988f99c9caf2b15f39b22e044171cce2486 Mon Sep 17 00:00:00 2001 From: xl-openai Date: Wed, 25 Feb 2026 19:55:28 -0800 Subject: [PATCH 07/54] Skip system skills for extra roots (#12744) When extra roots is set do not load system skills. --- codex-rs/core/src/skills/manager.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/codex-rs/core/src/skills/manager.rs b/codex-rs/core/src/skills/manager.rs index fa00fac41f..7d2c6b89fd 100644 --- a/codex-rs/core/src/skills/manager.rs +++ b/codex-rs/core/src/skills/manager.rs @@ -130,6 +130,12 @@ impl SkillsManager { }), ); let mut outcome = load_skills_from_roots(roots); + if !extra_user_roots.is_empty() { + // When extra user roots are provided, skip system skills before caching the result. + outcome + .skills + .retain(|skill| skill.scope != SkillScope::System); + } outcome.disabled_paths = disabled_paths_from_stack(&config_layer_stack); let (by_scripts_dir, by_doc_path) = build_implicit_skill_path_indexes(outcome.allowed_skills_for_implicit_invocation()); From 7326c097e3f66eb38c86b4ac470f7296ca8e277c Mon Sep 17 00:00:00 2001 From: Curtis 'Fjord' Hawthorne Date: Wed, 25 Feb 2026 20:09:30 -0800 Subject: [PATCH 08/54] Reduce js_repl Node version requirement to 22.22.0 (#12857) ## Summary Lower the `js_repl` minimum Node version from `24.13.1` to `22.22.0`. This updates the enforced minimum in `codex-rs/node-version.txt` and the corresponding user-facing `/experimental` description for the JavaScript REPL feature. ## Rationale The previous `24.13.1` floor was stricter than necessary for `js_repl`. I validated the REPL kernel behavior under Node `22.22.0` still works. ## Why `22.22.0` `22.22.0` is a current, widely packaged Node 22 release across common developer environments and distros, including Homebrew `node@22`, Fedora `nodejs22`, Arch `nodejs-lts-jod`, and Debian testing. That makes it a better exact floor than guessing at an older `22.x` patch we have not validated. `22.x` is also a maintenance branch that will be supported through April 2027, where the previous maintenance branch of `20.x` is only supported through April of this year. ## Changes - Update `codex-rs/node-version.txt` from `24.13.1` to `22.22.0` - Update the `/experimental` JavaScript REPL description to say `Requires Node >= v22.22.0 installed.` --- codex-rs/core/src/features.rs | 2 +- codex-rs/node-version.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index a4dd81c640..8f8178d201 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -463,7 +463,7 @@ pub const FEATURES: &[FeatureSpec] = &[ key: "js_repl", stage: Stage::Experimental { name: "JavaScript REPL", - menu_description: "Enable a persistent Node-backed JavaScript REPL for interactive website debugging and other inline JavaScript execution capabilities. Requires Node >= v24.13.1 installed.", + menu_description: "Enable a persistent Node-backed JavaScript REPL for interactive website debugging and other inline JavaScript execution capabilities. Requires Node >= v22.22.0 installed.", announcement: "NEW: JavaScript REPL is now available in /experimental. Enable it, then start a new chat or restart Codex to use it.", }, default_enabled: false, diff --git a/codex-rs/node-version.txt b/codex-rs/node-version.txt index 32f8c50de0..85e502778f 100644 --- a/codex-rs/node-version.txt +++ b/codex-rs/node-version.txt @@ -1 +1 @@ -24.13.1 +22.22.0 From cbbf302f5f7eac9ca38b0310dffb228f084d13a0 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Wed, 25 Feb 2026 20:59:07 -0800 Subject: [PATCH 09/54] Fix release build take (#12865) --- codex-rs/.cargo/config.toml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/codex-rs/.cargo/config.toml b/codex-rs/.cargo/config.toml index 7adeec500b..5d5eb8fd6f 100644 --- a/codex-rs/.cargo/config.toml +++ b/codex-rs/.cargo/config.toml @@ -1,11 +1,11 @@ [target.'cfg(all(windows, target_env = "msvc"))'] -rustflags = [ - "-C", - "link-arg=/STACK:8388608", - "-C", - "llvm-args=-function-sections", -] +rustflags = ["-C", "link-arg=/STACK:8388608"] +# MSVC emits a warning about code that may trip "Cortex-A53 MPCore processor bug #843419" (see +# https://developer.arm.com/documentation/epm048406/latest) which is sometimes emitted by LLVM. +# Since Arm64 Windows 10+ isn't supported on that processor, it's safe to disable the warning. +[target.aarch64-pc-windows-msvc] +rustflags = ["-C", "link-arg=/STACK:8388608", "-C", "link-arg=/arm64hazardfree"] [target.'cfg(all(windows, target_env = "gnu"))'] rustflags = ["-C", "link-arg=-Wl,--stack,8388608"] From 9a96b6f5091be1424f542965c00ae62dd29be991 Mon Sep 17 00:00:00 2001 From: pash-openai Date: Wed, 25 Feb 2026 21:28:37 -0800 Subject: [PATCH 10/54] Hide local file link destinations in TUI markdown (#12705) ## Summary - hide appended destinations for local path-style markdown links in the TUI renderer - keep web links rendering with their visible destination and style link labels consistently - add markdown renderer tests and a snapshot for the new file-link output ## Testing - just fmt - cargo test -p codex-tui image --- codex-rs/tui/src/markdown_render.rs | 41 +++++++++++++++--- codex-rs/tui/src/markdown_render_tests.rs | 42 ++++++++++++++++++- ...s__markdown_render_file_link_snapshot.snap | 5 +++ 3 files changed, 82 insertions(+), 6 deletions(-) create mode 100644 codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__markdown_render_file_link_snapshot.snap diff --git a/codex-rs/tui/src/markdown_render.rs b/codex-rs/tui/src/markdown_render.rs index ba8b47d5ed..a7e80acae8 100644 --- a/codex-rs/tui/src/markdown_render.rs +++ b/codex-rs/tui/src/markdown_render.rs @@ -85,6 +85,30 @@ pub(crate) fn render_markdown_text_with_width(input: &str, width: Option) w.text } +#[derive(Clone, Debug)] +struct LinkState { + destination: String, + show_destination: bool, +} + +fn should_render_link_destination(dest_url: &str) -> bool { + !is_local_path_like_link(dest_url) +} + +fn is_local_path_like_link(dest_url: &str) -> bool { + dest_url.starts_with("file://") + || dest_url.starts_with('/') + || dest_url.starts_with("~/") + || dest_url.starts_with("./") + || dest_url.starts_with("../") + || dest_url.starts_with("\\\\") + || matches!( + dest_url.as_bytes(), + [drive, b':', separator, ..] + if drive.is_ascii_alphabetic() && matches!(separator, b'/' | b'\\') + ) +} + struct Writer<'a, I> where I: Iterator>, @@ -95,7 +119,7 @@ where inline_styles: Vec