From 7b496c78de6e17ee42a296a651a6ba4c58e61c3f Mon Sep 17 00:00:00 2001 From: Roy Han Date: Mon, 23 Mar 2026 09:56:35 -0700 Subject: [PATCH] Revert "Merge branch 'rhan/approval-source' into rhan/escalations-approvals-sandbox" This reverts commit 2ad79b06f091eba9dd4f973edd458338d7cf41e4, reversing changes made to 7a2c92bf890e82aacbc4bbfcae3b6c80f10da4c7. --- .../schema/json/ClientRequest.json | 19 -- .../codex_app_server_protocol.schemas.json | 19 -- .../codex_app_server_protocol.v2.schemas.json | 19 -- .../RawResponseItemCompletedNotification.json | 19 -- .../schema/json/v2/ThreadResumeParams.json | 19 -- .../typescript/ApprovalSourceMetadata.ts | 5 - .../schema/typescript/ResponseItemMetadata.ts | 3 +- .../schema/typescript/index.ts | 1 - codex-rs/core/src/codex.rs | 17 +- codex-rs/core/src/codex_tests.rs | 240 ++---------------- codex-rs/core/src/mcp_tool_call.rs | 7 - codex-rs/core/src/state/turn.rs | 9 +- .../core/src/tools/runtimes/apply_patch.rs | 14 +- codex-rs/core/src/tools/runtimes/shell.rs | 16 +- .../tools/runtimes/shell/unix_escalation.rs | 33 +-- .../core/src/tools/runtimes/unified_exec.rs | 16 +- codex-rs/protocol/src/models.rs | 17 -- 17 files changed, 32 insertions(+), 441 deletions(-) delete mode 100644 codex-rs/app-server-protocol/schema/typescript/ApprovalSourceMetadata.ts diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index c015d664c5..93d08f7cb7 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -5,15 +5,6 @@ "description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.", "type": "string" }, - "ApprovalSourceMetadata": { - "enum": [ - "guardian", - "user", - "policy", - "unknown" - ], - "type": "string" - }, "ApprovalsReviewer": { "description": "Configures who approval requests are routed to for review. Examples include sandbox escapes, blocked network access, MCP approval prompts, and ARC escalations. Defaults to `user`. `guardian_subagent` uses a carefully prompted subagent to gather relevant context and apply a risk-based decision framework before approving or denying the request.", "enum": [ @@ -2033,16 +2024,6 @@ }, "ResponseItemMetadata": { "properties": { - "approval_source": { - "anyOf": [ - { - "$ref": "#/definitions/ApprovalSourceMetadata" - }, - { - "type": "null" - } - ] - }, "is_tool_call_escalated": { "type": [ "boolean", 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 c604ea661d..5708304428 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 @@ -5296,15 +5296,6 @@ "AppToolsConfig": { "type": "object" }, - "ApprovalSourceMetadata": { - "enum": [ - "guardian", - "user", - "policy", - "unknown" - ], - "type": "string" - }, "ApprovalsReviewer": { "description": "Configures who approval requests are routed to for review. Examples include sandbox escapes, blocked network access, MCP approval prompts, and ARC escalations. Defaults to `user`. `guardian_subagent` uses a carefully prompted subagent to gather relevant context and apply a risk-based decision framework before approving or denying the request.", "enum": [ @@ -10756,16 +10747,6 @@ }, "ResponseItemMetadata": { "properties": { - "approval_source": { - "anyOf": [ - { - "$ref": "#/definitions/v2/ApprovalSourceMetadata" - }, - { - "type": "null" - } - ] - }, "is_tool_call_escalated": { "type": [ "boolean", diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 0d9c191839..f39fd1055c 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -536,15 +536,6 @@ "AppToolsConfig": { "type": "object" }, - "ApprovalSourceMetadata": { - "enum": [ - "guardian", - "user", - "policy", - "unknown" - ], - "type": "string" - }, "ApprovalsReviewer": { "description": "Configures who approval requests are routed to for review. Examples include sandbox escapes, blocked network access, MCP approval prompts, and ARC escalations. Defaults to `user`. `guardian_subagent` uses a carefully prompted subagent to gather relevant context and apply a risk-based decision framework before approving or denying the request.", "enum": [ @@ -7524,16 +7515,6 @@ }, "ResponseItemMetadata": { "properties": { - "approval_source": { - "anyOf": [ - { - "$ref": "#/definitions/ApprovalSourceMetadata" - }, - { - "type": "null" - } - ] - }, "is_tool_call_escalated": { "type": [ "boolean", diff --git a/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json index c5f8ec4b5f..adee557c4b 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json @@ -1,15 +1,6 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { - "ApprovalSourceMetadata": { - "enum": [ - "guardian", - "user", - "policy", - "unknown" - ], - "type": "string" - }, "ContentItem": { "oneOf": [ { @@ -881,16 +872,6 @@ }, "ResponseItemMetadata": { "properties": { - "approval_source": { - "anyOf": [ - { - "$ref": "#/definitions/ApprovalSourceMetadata" - }, - { - "type": "null" - } - ] - }, "is_tool_call_escalated": { "type": [ "boolean", diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json index d73afba5ad..226ec47f75 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json @@ -1,15 +1,6 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { - "ApprovalSourceMetadata": { - "enum": [ - "guardian", - "user", - "policy", - "unknown" - ], - "type": "string" - }, "ApprovalsReviewer": { "description": "Configures who approval requests are routed to for review. Examples include sandbox escapes, blocked network access, MCP approval prompts, and ARC escalations. Defaults to `user`. `guardian_subagent` uses a carefully prompted subagent to gather relevant context and apply a risk-based decision framework before approving or denying the request.", "enum": [ @@ -947,16 +938,6 @@ }, "ResponseItemMetadata": { "properties": { - "approval_source": { - "anyOf": [ - { - "$ref": "#/definitions/ApprovalSourceMetadata" - }, - { - "type": "null" - } - ] - }, "is_tool_call_escalated": { "type": [ "boolean", diff --git a/codex-rs/app-server-protocol/schema/typescript/ApprovalSourceMetadata.ts b/codex-rs/app-server-protocol/schema/typescript/ApprovalSourceMetadata.ts deleted file mode 100644 index 37da210732..0000000000 --- a/codex-rs/app-server-protocol/schema/typescript/ApprovalSourceMetadata.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 ApprovalSourceMetadata = "guardian" | "user" | "policy" | "unknown"; diff --git a/codex-rs/app-server-protocol/schema/typescript/ResponseItemMetadata.ts b/codex-rs/app-server-protocol/schema/typescript/ResponseItemMetadata.ts index 52a7bc1185..deb3c845bd 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ResponseItemMetadata.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ResponseItemMetadata.ts @@ -1,7 +1,6 @@ // 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 { ApprovalSourceMetadata } from "./ApprovalSourceMetadata"; import type { ReviewDecisionMetadata } from "./ReviewDecisionMetadata"; import type { SandboxPolicyMetadata } from "./SandboxPolicyMetadata"; import type { UserMessageType } from "./UserMessageType"; @@ -10,4 +9,4 @@ export type ResponseItemMetadata = { user_message_type?: UserMessageType, /** * Client-visible UUID generated by Codex for this item. */ -uuid?: string, sandbox_policy?: SandboxPolicyMetadata, is_tool_call_escalated?: boolean, review_decision?: ReviewDecisionMetadata, approval_source?: ApprovalSourceMetadata, }; +uuid?: string, sandbox_policy?: SandboxPolicyMetadata, is_tool_call_escalated?: boolean, review_decision?: ReviewDecisionMetadata, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/index.ts b/codex-rs/app-server-protocol/schema/typescript/index.ts index e3124672e4..a963406a5c 100644 --- a/codex-rs/app-server-protocol/schema/typescript/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/index.ts @@ -3,7 +3,6 @@ export type { AbsolutePathBuf } from "./AbsolutePathBuf"; export type { ApplyPatchApprovalParams } from "./ApplyPatchApprovalParams"; export type { ApplyPatchApprovalResponse } from "./ApplyPatchApprovalResponse"; -export type { ApprovalSourceMetadata } from "./ApprovalSourceMetadata"; export type { AuthMode } from "./AuthMode"; export type { ClientInfo } from "./ClientInfo"; export type { ClientNotification } from "./ClientNotification"; diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 5d9a107a65..cd11b99b6e 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -88,7 +88,6 @@ use codex_protocol::items::TurnItem; use codex_protocol::items::UserMessageItem; use codex_protocol::items::build_hook_prompt_message; use codex_protocol::mcp::CallToolResult; -use codex_protocol::models::ApprovalSourceMetadata; use codex_protocol::models::BaseInstructions; use codex_protocol::models::PermissionProfile; use codex_protocol::models::ResponseItemMessageMetadata; @@ -210,7 +209,6 @@ use crate::file_watcher::FileWatcher; use crate::file_watcher::FileWatcherEvent; use crate::git_info::get_git_repo_root; use crate::guardian::GuardianReviewSessionManager; -use crate::guardian::routes_approval_to_guardian; use crate::hook_runtime::PendingInputHookDisposition; use crate::hook_runtime::inspect_pending_input; use crate::hook_runtime::record_additional_contexts; @@ -1054,13 +1052,6 @@ fn user_message_metadata_patch( } } -fn approval_source_for_turn(turn_context: &TurnContext) -> ApprovalSourceMetadata { - if routes_approval_to_guardian(turn_context) { - ApprovalSourceMetadata::Guardian - } else { - ApprovalSourceMetadata::User - } -} fn response_item_tool_call_id(item: &ResponseItem) -> Option<&str> { match item { ResponseItem::LocalShellCall { @@ -1144,12 +1135,10 @@ fn stamp_tool_approval_metadata_with_snapshot( Some(outcome) => { metadata.is_tool_call_escalated = Some(true); metadata.review_decision = outcome.review_decision; - metadata.approval_source = Some(outcome.approval_source); } None if !has_pending_approval => { metadata.is_tool_call_escalated = Some(false); metadata.review_decision = None; - metadata.approval_source = None; } None => { return stamp_tool_metadata_on_response_item(response_item, metadata); @@ -3028,7 +3017,6 @@ impl Session { effective_approval_id.clone(), PendingApprovalMetadata { call_id: call_id.clone(), - approval_source: approval_source_for_turn(turn_context), }, ); ts.insert_pending_approval(effective_approval_id.clone(), tx_approve) @@ -3100,7 +3088,6 @@ impl Session { approval_id.clone(), PendingApprovalMetadata { call_id: call_id.clone(), - approval_source: approval_source_for_turn(turn_context), }, ); ts.insert_pending_approval(approval_id.clone(), tx_approve) @@ -3398,16 +3385,16 @@ impl Session { .remove_pending_approval_call_id(approval_id) .unwrap_or_else(|| PendingApprovalMetadata { call_id: approval_id.to_string(), - approval_source: ApprovalSourceMetadata::Unknown, }); drop(ts); drop(active); self.record_call_approval_outcome( pending_approval.call_id, - ApprovalOutcomeMetadata::reviewed(decision, pending_approval.approval_source), + ApprovalOutcomeMetadata::reviewed(decision), ) .await; } + pub(crate) async fn record_call_approval_outcome( &self, call_id: String, diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index 7b657fda44..7854e3222b 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -4418,100 +4418,52 @@ async fn task_finish_emits_prompt_queued_metadata_for_injected_user_input_when_f #[test] fn review_decision_metadata_mapping_is_stable() { assert_eq!( - ApprovalOutcomeMetadata::reviewed( - &ReviewDecision::Approved, - codex_protocol::models::ApprovalSourceMetadata::User, - ) - .review_decision, + ApprovalOutcomeMetadata::reviewed(&ReviewDecision::Approved).review_decision, Some(codex_protocol::models::ReviewDecisionMetadata::Approved) ); assert_eq!( - ApprovalOutcomeMetadata::reviewed( - &ReviewDecision::Denied, - codex_protocol::models::ApprovalSourceMetadata::User, - ) - .review_decision, + ApprovalOutcomeMetadata::reviewed(&ReviewDecision::Denied).review_decision, Some(codex_protocol::models::ReviewDecisionMetadata::Denied) ); assert_eq!( - ApprovalOutcomeMetadata::reviewed( - &ReviewDecision::Abort, - codex_protocol::models::ApprovalSourceMetadata::User, - ) - .review_decision, + ApprovalOutcomeMetadata::reviewed(&ReviewDecision::Abort).review_decision, Some(codex_protocol::models::ReviewDecisionMetadata::Abort) ); assert_eq!( - ApprovalOutcomeMetadata::reviewed( - &ReviewDecision::ApprovedForSession, - codex_protocol::models::ApprovalSourceMetadata::User, - ) - .review_decision, + ApprovalOutcomeMetadata::reviewed(&ReviewDecision::ApprovedForSession).review_decision, Some(codex_protocol::models::ReviewDecisionMetadata::ApprovedForSession) ); assert_eq!( - ApprovalOutcomeMetadata::reviewed( - &ReviewDecision::ApprovedExecpolicyAmendment { - proposed_execpolicy_amendment: codex_protocol::approvals::ExecPolicyAmendment::new( - vec!["echo".to_string(), "hi".to_string()], - ), - }, - codex_protocol::models::ApprovalSourceMetadata::User, - ) + ApprovalOutcomeMetadata::reviewed(&ReviewDecision::ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment: codex_protocol::approvals::ExecPolicyAmendment::new( + vec!["echo".to_string(), "hi".to_string()], + ), + }) .review_decision, Some(codex_protocol::models::ReviewDecisionMetadata::ApprovedWithAmendment) ); assert_eq!( - ApprovalOutcomeMetadata::reviewed( - &ReviewDecision::NetworkPolicyAmendment { - network_policy_amendment: codex_protocol::approvals::NetworkPolicyAmendment { - host: "example.com".to_string(), - action: NetworkPolicyRuleAction::Allow, - }, + ApprovalOutcomeMetadata::reviewed(&ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment: codex_protocol::approvals::NetworkPolicyAmendment { + host: "example.com".to_string(), + action: codex_protocol::protocol::NetworkPolicyRuleAction::Allow, }, - codex_protocol::models::ApprovalSourceMetadata::User, - ) + }) .review_decision, Some(codex_protocol::models::ReviewDecisionMetadata::ApprovedWithNetworkPolicyAllow) ); assert_eq!( - ApprovalOutcomeMetadata::reviewed( - &ReviewDecision::NetworkPolicyAmendment { - network_policy_amendment: codex_protocol::approvals::NetworkPolicyAmendment { - host: "example.com".to_string(), - action: NetworkPolicyRuleAction::Deny, - }, + ApprovalOutcomeMetadata::reviewed(&ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment: codex_protocol::approvals::NetworkPolicyAmendment { + host: "example.com".to_string(), + action: codex_protocol::protocol::NetworkPolicyRuleAction::Deny, }, - codex_protocol::models::ApprovalSourceMetadata::User, - ) + }) .review_decision, Some(codex_protocol::models::ReviewDecisionMetadata::DeniedWithNetworkPolicyDeny) ); } -#[tokio::test] -async fn approval_source_metadata_mapping_is_stable() { - let (_session, mut turn_context) = make_session_and_context().await; - - assert_eq!( - approval_source_for_turn(&turn_context), - codex_protocol::models::ApprovalSourceMetadata::User - ); - - turn_context - .approval_policy - .set(crate::protocol::AskForApproval::OnRequest) - .expect("test setup should allow updating approval policy"); - Arc::get_mut(&mut turn_context.config) - .expect("single turn config ref") - .approvals_reviewer = codex_protocol::config_types::ApprovalsReviewer::GuardianSubagent; - - assert_eq!( - approval_source_for_turn(&turn_context), - codex_protocol::models::ApprovalSourceMetadata::Guardian - ); -} - #[test] fn sandbox_policy_metadata_mapping_is_stable() { assert_eq!( @@ -4575,7 +4527,6 @@ async fn assert_next_emitted_function_call_metadata( expected_sandbox_policy: codex_protocol::models::SandboxPolicyMetadata, expected_escalated: bool, expected_review_decision: Option, - expected_approval_source: Option, ) { let event = tokio::time::timeout(std::time::Duration::from_secs(2), rx.recv()) .await @@ -4591,7 +4542,6 @@ async fn assert_next_emitted_function_call_metadata( .. } if metadata.is_tool_call_escalated == Some(expected_escalated) && metadata.review_decision == expected_review_decision - && metadata.approval_source == expected_approval_source && metadata.sandbox_policy == Some(expected_sandbox_policy) ) )); @@ -4603,10 +4553,7 @@ async fn tool_call_metadata_stamps_escalated_review_decision_when_feature_enable sess.record_call_approval_outcome( "call-1".to_string(), - ApprovalOutcomeMetadata::reviewed( - &ReviewDecision::Denied, - codex_protocol::models::ApprovalSourceMetadata::User, - ), + ApprovalOutcomeMetadata::reviewed(&ReviewDecision::Denied), ) .await; sess.record_response_item_and_emit_turn_item(tc.as_ref(), function_call_item("call-1")) @@ -4616,7 +4563,6 @@ async fn tool_call_metadata_stamps_escalated_review_decision_when_feature_enable expected_sandbox_policy, true, Some(codex_protocol::models::ReviewDecisionMetadata::Denied), - Some(codex_protocol::models::ApprovalSourceMetadata::User), ) .await; } @@ -4627,62 +4573,7 @@ async fn tool_call_metadata_stamps_non_escalated_false_when_feature_enabled() { sess.record_response_item_and_emit_turn_item(tc.as_ref(), function_call_item("call-2")) .await; - assert_next_emitted_function_call_metadata(&rx, expected_sandbox_policy, false, None, None) - .await; -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn tool_call_metadata_stamps_guardian_direct_review_when_feature_enabled() { - let (sess, tc, rx, expected_sandbox_policy) = setup_tool_call_metadata_runtime_test().await; - - sess.record_call_approval_outcome( - "call-guardian-runtime-1".to_string(), - ApprovalOutcomeMetadata::reviewed( - &ReviewDecision::Denied, - codex_protocol::models::ApprovalSourceMetadata::Guardian, - ), - ) - .await; - sess.record_response_item_and_emit_turn_item( - tc.as_ref(), - function_call_item("call-guardian-runtime-1"), - ) - .await; - assert_next_emitted_function_call_metadata( - &rx, - expected_sandbox_policy, - true, - Some(codex_protocol::models::ReviewDecisionMetadata::Denied), - Some(codex_protocol::models::ApprovalSourceMetadata::Guardian), - ) - .await; -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn tool_call_metadata_stamps_policy_source_without_review_decision_when_feature_enabled() { - let (sess, tc, rx, expected_sandbox_policy) = setup_tool_call_metadata_runtime_test().await; - - sess.record_call_approval_outcome( - "call-policy-runtime-1".to_string(), - ApprovalOutcomeMetadata { - review_decision: None, - approval_source: codex_protocol::models::ApprovalSourceMetadata::Policy, - }, - ) - .await; - sess.record_response_item_and_emit_turn_item( - tc.as_ref(), - function_call_item("call-policy-runtime-1"), - ) - .await; - assert_next_emitted_function_call_metadata( - &rx, - expected_sandbox_policy, - true, - None, - Some(codex_protocol::models::ApprovalSourceMetadata::Policy), - ) - .await; + assert_next_emitted_function_call_metadata(&rx, expected_sandbox_policy, false, None).await; } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -4738,7 +4629,6 @@ async fn handle_output_item_done_stamps_tool_call_metadata_when_feature_enabled( } if call_id == "call-stream-1" && metadata.is_tool_call_escalated == Some(false) && metadata.review_decision.is_none() - && metadata.approval_source.is_none() && metadata.sandbox_policy == Some(expected_sandbox_policy.clone()) ) })); @@ -4781,10 +4671,7 @@ async fn tool_call_metadata_can_be_restamped_after_approval_outcome() { .await; sess.record_call_approval_outcome( "call-restamp-1".to_string(), - ApprovalOutcomeMetadata::reviewed( - &ReviewDecision::Denied, - codex_protocol::models::ApprovalSourceMetadata::User, - ), + ApprovalOutcomeMetadata::reviewed(&ReviewDecision::Denied), ) .await; @@ -4814,89 +4701,6 @@ async fn tool_call_metadata_can_be_restamped_after_approval_outcome() { } if metadata.is_tool_call_escalated == Some(true) && metadata.review_decision == Some(codex_protocol::models::ReviewDecisionMetadata::Denied) - && metadata.approval_source - == Some(codex_protocol::models::ApprovalSourceMetadata::User) - && metadata.sandbox_policy == Some(expected_sandbox_policy) - )); -} - -#[tokio::test] -async fn tool_call_metadata_snapshot_stamps_guardian_approval_source() { - let (_session, turn_context_raw) = make_session_and_context().await; - let turn_context = Arc::new(turn_context_raw); - let expected_sandbox_policy = sandbox_policy_to_metadata(turn_context.sandbox_policy.get()); - let item = ResponseItem::FunctionCall { - id: None, - name: "shell".to_string(), - namespace: None, - arguments: "{}".to_string(), - call_id: "call-guardian-1".to_string(), - metadata: None, - }; - let snapshot = ToolApprovalMetadataSnapshot { - approval_outcomes_by_call_id: HashMap::from([( - "call-guardian-1".to_string(), - ApprovalOutcomeMetadata { - review_decision: Some(codex_protocol::models::ReviewDecisionMetadata::Denied), - approval_source: codex_protocol::models::ApprovalSourceMetadata::Guardian, - }, - )]), - pending_approval_call_ids: HashSet::new(), - }; - - let stamped = - stamp_tool_approval_metadata_with_snapshot(turn_context.as_ref(), item, Some(&snapshot)); - - assert!(matches!( - stamped, - ResponseItem::FunctionCall { - metadata: Some(metadata), - .. - } if metadata.is_tool_call_escalated == Some(true) - && metadata.review_decision - == Some(codex_protocol::models::ReviewDecisionMetadata::Denied) - && metadata.approval_source - == Some(codex_protocol::models::ApprovalSourceMetadata::Guardian) - && metadata.sandbox_policy == Some(expected_sandbox_policy) - )); -} - -#[tokio::test] -async fn tool_call_metadata_snapshot_stamps_policy_approval_source_without_review_decision() { - let (_session, turn_context_raw) = make_session_and_context().await; - let turn_context = Arc::new(turn_context_raw); - let expected_sandbox_policy = sandbox_policy_to_metadata(turn_context.sandbox_policy.get()); - let item = ResponseItem::FunctionCall { - id: None, - name: "shell".to_string(), - namespace: None, - arguments: "{}".to_string(), - call_id: "call-policy-1".to_string(), - metadata: None, - }; - let snapshot = ToolApprovalMetadataSnapshot { - approval_outcomes_by_call_id: HashMap::from([( - "call-policy-1".to_string(), - ApprovalOutcomeMetadata { - review_decision: None, - approval_source: codex_protocol::models::ApprovalSourceMetadata::Policy, - }, - )]), - pending_approval_call_ids: HashSet::new(), - }; - - let stamped = - stamp_tool_approval_metadata_with_snapshot(turn_context.as_ref(), item, Some(&snapshot)); - - assert!(matches!( - stamped, - ResponseItem::FunctionCall { - metadata: Some(metadata), - .. - } if metadata.is_tool_call_escalated == Some(true) - && metadata.review_decision.is_none() - && metadata.approval_source - == Some(codex_protocol::models::ApprovalSourceMetadata::Policy) && metadata.sandbox_policy == Some(expected_sandbox_policy) )); } diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 105f85eefd..3e7f0cb84f 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -31,11 +31,9 @@ use crate::protocol::EventMsg; use crate::protocol::McpInvocation; use crate::protocol::McpToolCallBeginEvent; use crate::protocol::McpToolCallEndEvent; -use crate::state::ApprovalOutcomeMetadata; use crate::state_db; use codex_features::Feature; use codex_protocol::mcp::CallToolResult; -use codex_protocol::models::ApprovalSourceMetadata; use codex_protocol::openai_models::InputModality; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ReviewDecision; @@ -547,11 +545,6 @@ async fn maybe_request_mcp_tool_approval( monitor_reason.clone(), ) .await; - sess.record_call_approval_outcome( - call_id.to_string(), - ApprovalOutcomeMetadata::reviewed(&decision, ApprovalSourceMetadata::Guardian), - ) - .await; let decision = mcp_tool_approval_decision_from_guardian(decision); apply_mcp_tool_approval_decision( sess, diff --git a/codex-rs/core/src/state/turn.rs b/codex-rs/core/src/state/turn.rs index 388ee43efc..1e2b7a24bb 100644 --- a/codex-rs/core/src/state/turn.rs +++ b/codex-rs/core/src/state/turn.rs @@ -10,7 +10,6 @@ use tokio_util::sync::CancellationToken; use tokio_util::task::AbortOnDropHandle; use codex_protocol::dynamic_tools::DynamicToolResponse; -use codex_protocol::models::ApprovalSourceMetadata; use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ReviewDecisionMetadata; use codex_protocol::request_permissions::RequestPermissionsResponse; @@ -62,20 +61,15 @@ pub(crate) struct RunningTask { #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) struct PendingApprovalMetadata { pub(crate) call_id: String, - pub(crate) approval_source: ApprovalSourceMetadata, } #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) struct ApprovalOutcomeMetadata { pub(crate) review_decision: Option, - pub(crate) approval_source: ApprovalSourceMetadata, } impl ApprovalOutcomeMetadata { - pub(crate) fn reviewed( - decision: &ReviewDecision, - approval_source: ApprovalSourceMetadata, - ) -> Self { + pub(crate) fn reviewed(decision: &ReviewDecision) -> Self { let review_decision = match decision { ReviewDecision::Approved => ReviewDecisionMetadata::Approved, ReviewDecision::ApprovedExecpolicyAmendment { .. } => { @@ -97,7 +91,6 @@ impl ApprovalOutcomeMetadata { }; Self { review_decision: Some(review_decision), - approval_source, } } } diff --git a/codex-rs/core/src/tools/runtimes/apply_patch.rs b/codex-rs/core/src/tools/runtimes/apply_patch.rs index 7cceaccca8..f1e9912bc5 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch.rs @@ -12,7 +12,6 @@ use crate::guardian::routes_approval_to_guardian; use crate::sandboxing::CommandSpec; use crate::sandboxing::SandboxPermissions; use crate::sandboxing::execute_env; -use crate::state::ApprovalOutcomeMetadata; use crate::tools::sandboxing::Approvable; use crate::tools::sandboxing::ApprovalCtx; use crate::tools::sandboxing::ExecApprovalRequirement; @@ -25,7 +24,6 @@ use crate::tools::sandboxing::ToolRuntime; use crate::tools::sandboxing::with_cached_approval; use codex_apply_patch::ApplyPatchAction; use codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1; -use codex_protocol::models::ApprovalSourceMetadata; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::FileChange; @@ -144,17 +142,7 @@ impl Approvable for ApplyPatchRuntime { Box::pin(async move { if routes_approval_to_guardian(turn) { let action = ApplyPatchRuntime::build_guardian_review_request(req, ctx.call_id); - let decision = review_approval_request(session, turn, action, retry_reason).await; - session - .record_call_approval_outcome( - call_id.clone(), - ApprovalOutcomeMetadata::reviewed( - &decision, - ApprovalSourceMetadata::Guardian, - ), - ) - .await; - return decision; + return review_approval_request(session, turn, action, retry_reason).await; } if req.permissions_preapproved && retry_reason.is_none() { return ReviewDecision::Approved; diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index 00fa223870..18afb20bc5 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -17,7 +17,6 @@ use crate::powershell::prefix_powershell_script_with_utf8; use crate::sandboxing::SandboxPermissions; use crate::sandboxing::execute_env; use crate::shell::ShellType; -use crate::state::ApprovalOutcomeMetadata; use crate::tools::network_approval::NetworkApprovalMode; use crate::tools::network_approval::NetworkApprovalSpec; use crate::tools::runtimes::build_command_spec; @@ -36,7 +35,6 @@ use crate::tools::sandboxing::sandbox_override_for_first_attempt; use crate::tools::sandboxing::with_cached_approval; use codex_features::Feature; use codex_network_proxy::NetworkProxy; -use codex_protocol::models::ApprovalSourceMetadata; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::ReviewDecision; use futures::future::BoxFuture; @@ -155,11 +153,11 @@ impl Approvable for ShellRuntime { let call_id = ctx.call_id.to_string(); Box::pin(async move { if routes_approval_to_guardian(turn) { - let decision = review_approval_request( + return review_approval_request( session, turn, GuardianApprovalRequest::Shell { - id: call_id.clone(), + id: call_id, command, cwd, sandbox_permissions: req.sandbox_permissions, @@ -169,16 +167,6 @@ impl Approvable for ShellRuntime { retry_reason, ) .await; - session - .record_call_approval_outcome( - call_id.clone(), - ApprovalOutcomeMetadata::reviewed( - &decision, - ApprovalSourceMetadata::Guardian, - ), - ) - .await; - return decision; } with_cached_approval(&session.services, "shell", keys, move || async move { let available_decisions = None; 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 eca9cb5629..948018dae6 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -13,7 +13,6 @@ use crate::sandboxing::ExecRequest; use crate::sandboxing::SandboxPermissions; use crate::shell::ShellType; use crate::skills::SkillMetadata; -use crate::state::ApprovalOutcomeMetadata; use crate::tools::runtimes::ExecveSessionApproval; use crate::tools::runtimes::build_command_spec; use crate::tools::sandboxing::SandboxAttempt; @@ -27,7 +26,6 @@ use codex_execpolicy::Policy; use codex_execpolicy::RuleMatch; use codex_features::Feature; use codex_protocol::config_types::WindowsSandboxLevel; -use codex_protocol::models::ApprovalSourceMetadata; use codex_protocol::models::MacOsSeatbeltProfileExtensions; use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemSandboxPolicy; @@ -428,7 +426,7 @@ impl CoreShellActionProvider { Ok(stopwatch .pause_for(async move { if routes_approval_to_guardian(&turn) { - let decision = review_approval_request( + return review_approval_request( &session, &turn, GuardianApprovalRequest::Execve { @@ -442,16 +440,6 @@ impl CoreShellActionProvider { /*retry_reason*/ None, ) .await; - session - .record_call_approval_outcome( - call_id.clone(), - ApprovalOutcomeMetadata::reviewed( - &decision, - ApprovalSourceMetadata::Guardian, - ), - ) - .await; - return decision; } let available_decisions = vec![ Some(ReviewDecision::Approved), @@ -534,31 +522,12 @@ impl CoreShellActionProvider { ) -> anyhow::Result { let action = match decision { Decision::Forbidden => { - self.session - .record_call_approval_outcome( - self.call_id.clone(), - ApprovalOutcomeMetadata { - review_decision: None, - approval_source: codex_protocol::models::ApprovalSourceMetadata::Policy, - }, - ) - .await; EscalationDecision::deny(Some("Execution forbidden by policy".to_string())) } Decision::Prompt => { if execve_prompt_is_rejected_by_policy(self.approval_policy, &decision_source) .is_some() { - self.session - .record_call_approval_outcome( - self.call_id.clone(), - ApprovalOutcomeMetadata { - review_decision: None, - approval_source: - codex_protocol::models::ApprovalSourceMetadata::Policy, - }, - ) - .await; EscalationDecision::deny(Some("Execution forbidden by policy".to_string())) } else { match self diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 23356410be..0b64092156 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -14,7 +14,6 @@ use crate::guardian::routes_approval_to_guardian; use crate::powershell::prefix_powershell_script_with_utf8; use crate::sandboxing::SandboxPermissions; use crate::shell::ShellType; -use crate::state::ApprovalOutcomeMetadata; use crate::tools::network_approval::NetworkApprovalMode; use crate::tools::network_approval::NetworkApprovalSpec; use crate::tools::runtimes::build_command_spec; @@ -39,7 +38,6 @@ use crate::unified_exec::UnifiedExecProcess; use crate::unified_exec::UnifiedExecProcessManager; use codex_features::Feature; use codex_network_proxy::NetworkProxy; -use codex_protocol::models::ApprovalSourceMetadata; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::ReviewDecision; use futures::future::BoxFuture; @@ -123,11 +121,11 @@ impl Approvable for UnifiedExecRuntime<'_> { let reason = retry_reason.clone().or_else(|| req.justification.clone()); Box::pin(async move { if routes_approval_to_guardian(turn) { - let decision = review_approval_request( + return review_approval_request( session, turn, GuardianApprovalRequest::ExecCommand { - id: call_id.clone(), + id: call_id, command, cwd, sandbox_permissions: req.sandbox_permissions, @@ -138,16 +136,6 @@ impl Approvable for UnifiedExecRuntime<'_> { retry_reason, ) .await; - session - .record_call_approval_outcome( - call_id.clone(), - ApprovalOutcomeMetadata::reviewed( - &decision, - ApprovalSourceMetadata::Guardian, - ), - ) - .await; - return decision; } with_cached_approval(&session.services, "unified_exec", keys, || async move { let available_decisions = None; diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index d9fd95f9bf..6b15a4009c 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -291,16 +291,6 @@ pub enum SandboxPolicyMetadata { FullAccess, } -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema, TS)] -#[serde(rename_all = "snake_case")] -#[ts(rename_all = "snake_case")] -pub enum ApprovalSourceMetadata { - Guardian, - User, - Policy, - Unknown, -} - #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq, JsonSchema, TS)] pub struct ResponseItemMetadata { #[serde(default, skip_serializing_if = "Option::is_none")] @@ -319,9 +309,6 @@ pub struct ResponseItemMetadata { #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] pub review_decision: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - #[ts(optional)] - pub approval_source: Option, } impl ResponseItemMetadata { @@ -329,7 +316,6 @@ impl ResponseItemMetadata { self.sandbox_policy.is_none() && self.is_tool_call_escalated.is_none() && self.review_decision.is_none() - && self.approval_source.is_none() } pub fn merge_from(&mut self, other: Self) { @@ -342,9 +328,6 @@ impl ResponseItemMetadata { if other.review_decision.is_some() { self.review_decision = other.review_decision; } - if other.approval_source.is_some() { - self.approval_source = other.approval_source; - } } }