mirror of
https://github.com/openai/codex.git
synced 2026-11-13 21:30:47 +00:00
guardian patch and test update
This commit is contained in:
@@ -3421,9 +3421,23 @@ impl Session {
|
||||
});
|
||||
drop(ts);
|
||||
drop(active);
|
||||
self.record_direct_approval_outcome(
|
||||
&pending_approval.call_id,
|
||||
decision,
|
||||
pending_approval.approval_source,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
pub async fn record_direct_approval_outcome(
|
||||
&self,
|
||||
call_id: &str,
|
||||
decision: &ReviewDecision,
|
||||
approval_source: ApprovalSourceMetadata,
|
||||
) {
|
||||
self.record_call_approval_outcome(
|
||||
pending_approval.call_id,
|
||||
ApprovalOutcomeMetadata::reviewed(decision, pending_approval.approval_source),
|
||||
call_id.to_string(),
|
||||
ApprovalOutcomeMetadata::reviewed(decision, approval_source),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
@@ -4609,27 +4609,45 @@ async fn tool_call_metadata_stamps_non_escalated_false_when_feature_enabled() {
|
||||
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,
|
||||
),
|
||||
sess.record_direct_approval_outcome(
|
||||
"call-guardian-runtime-1",
|
||||
&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),
|
||||
ResponseItem::FunctionCall {
|
||||
id: None,
|
||||
name: "shell".to_string(),
|
||||
namespace: None,
|
||||
arguments: "{}".to_string(),
|
||||
call_id: "call-guardian-runtime-1".to_string(),
|
||||
metadata: None,
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
let event = tokio::time::timeout(std::time::Duration::from_secs(2), rx.recv())
|
||||
.await
|
||||
.expect("expected raw response item event")
|
||||
.expect("channel open");
|
||||
assert!(matches!(
|
||||
event.msg,
|
||||
EventMsg::RawResponseItem(ref ev)
|
||||
if matches!(
|
||||
&ev.item,
|
||||
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(flavor = "multi_thread", worker_threads = 2)]
|
||||
@@ -4746,12 +4764,10 @@ 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,
|
||||
),
|
||||
sess.record_direct_approval_outcome(
|
||||
"call-restamp-1",
|
||||
&ReviewDecision::Denied,
|
||||
codex_protocol::models::ApprovalSourceMetadata::User,
|
||||
)
|
||||
.await;
|
||||
|
||||
|
||||
@@ -32,7 +32,6 @@ 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_protocol::mcp::CallToolResult;
|
||||
use codex_protocol::models::ApprovalSourceMetadata;
|
||||
@@ -534,11 +533,8 @@ 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;
|
||||
sess.record_direct_approval_outcome(call_id, &decision, ApprovalSourceMetadata::Guardian)
|
||||
.await;
|
||||
let decision = mcp_tool_approval_decision_from_guardian(decision);
|
||||
apply_mcp_tool_approval_decision(
|
||||
sess,
|
||||
|
||||
@@ -11,7 +11,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;
|
||||
@@ -144,12 +143,10 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
|
||||
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,
|
||||
),
|
||||
.record_direct_approval_outcome(
|
||||
&call_id,
|
||||
&decision,
|
||||
ApprovalSourceMetadata::Guardian,
|
||||
)
|
||||
.await;
|
||||
return decision;
|
||||
|
||||
@@ -18,7 +18,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;
|
||||
@@ -170,12 +169,10 @@ impl Approvable<ShellRequest> for ShellRuntime {
|
||||
)
|
||||
.await;
|
||||
session
|
||||
.record_call_approval_outcome(
|
||||
call_id.clone(),
|
||||
ApprovalOutcomeMetadata::reviewed(
|
||||
&decision,
|
||||
ApprovalSourceMetadata::Guardian,
|
||||
),
|
||||
.record_direct_approval_outcome(
|
||||
&call_id,
|
||||
&decision,
|
||||
ApprovalSourceMetadata::Guardian,
|
||||
)
|
||||
.await;
|
||||
return decision;
|
||||
|
||||
@@ -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;
|
||||
@@ -441,12 +440,10 @@ impl CoreShellActionProvider {
|
||||
)
|
||||
.await;
|
||||
session
|
||||
.record_call_approval_outcome(
|
||||
call_id.clone(),
|
||||
ApprovalOutcomeMetadata::reviewed(
|
||||
&decision,
|
||||
ApprovalSourceMetadata::Guardian,
|
||||
),
|
||||
.record_direct_approval_outcome(
|
||||
&call_id,
|
||||
&decision,
|
||||
ApprovalSourceMetadata::Guardian,
|
||||
)
|
||||
.await;
|
||||
return decision;
|
||||
|
||||
@@ -15,7 +15,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;
|
||||
@@ -139,12 +138,10 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
)
|
||||
.await;
|
||||
session
|
||||
.record_call_approval_outcome(
|
||||
call_id.clone(),
|
||||
ApprovalOutcomeMetadata::reviewed(
|
||||
&decision,
|
||||
ApprovalSourceMetadata::Guardian,
|
||||
),
|
||||
.record_direct_approval_outcome(
|
||||
&call_id,
|
||||
&decision,
|
||||
ApprovalSourceMetadata::Guardian,
|
||||
)
|
||||
.await;
|
||||
return decision;
|
||||
|
||||
Reference in New Issue
Block a user