mirror of
https://github.com/openai/codex.git
synced 2026-10-28 18:58:46 +00:00
Revert "Merge branch 'rhan/approval-source' into rhan/escalations-approvals-sandbox"
This reverts commit2ad79b06f0, reversing changes made to7a2c92bf89.
This commit is contained in:
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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";
|
||||
@@ -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, };
|
||||
|
||||
@@ -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";
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<codex_protocol::models::ReviewDecisionMetadata>,
|
||||
expected_approval_source: Option<codex_protocol::models::ApprovalSourceMetadata>,
|
||||
) {
|
||||
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)
|
||||
));
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<ReviewDecisionMetadata>,
|
||||
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,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<ApplyPatchRequest> 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;
|
||||
|
||||
@@ -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<ShellRequest> 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<ShellRequest> 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;
|
||||
|
||||
@@ -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<EscalationDecision> {
|
||||
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
|
||||
|
||||
@@ -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<UnifiedExecRequest> 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<UnifiedExecRequest> 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;
|
||||
|
||||
@@ -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<ReviewDecisionMetadata>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub approval_source: Option<ApprovalSourceMetadata>,
|
||||
}
|
||||
|
||||
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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user