mirror of
https://github.com/openai/codex.git
synced 2026-10-25 18:28:38 +00:00
Add deterministic Guardian review mode
This commit is contained in:
@@ -1983,6 +1983,7 @@ async fn guardian_review_event_ingests_custom_fact_with_optional_target_item() {
|
||||
risk_level: None,
|
||||
user_authorization: None,
|
||||
outcome: None,
|
||||
decision_source: None,
|
||||
guardian_thread_id: None,
|
||||
guardian_session_kind: None,
|
||||
guardian_model: None,
|
||||
|
||||
@@ -30,6 +30,7 @@ use codex_plugin::PluginTelemetryMetadata;
|
||||
use codex_protocol::approvals::NetworkApprovalProtocol;
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
use codex_protocol::models::SandboxPermissions;
|
||||
use codex_protocol::protocol::GuardianAssessmentDecisionSource;
|
||||
use codex_protocol::protocol::GuardianAssessmentOutcome;
|
||||
use codex_protocol::protocol::GuardianCommandSource;
|
||||
use codex_protocol::protocol::GuardianRiskLevel;
|
||||
@@ -270,6 +271,7 @@ pub struct GuardianReviewEventParams {
|
||||
pub risk_level: Option<GuardianRiskLevel>,
|
||||
pub user_authorization: Option<GuardianUserAuthorization>,
|
||||
pub outcome: Option<GuardianAssessmentOutcome>,
|
||||
pub decision_source: Option<GuardianAssessmentDecisionSource>,
|
||||
pub guardian_thread_id: Option<String>,
|
||||
pub guardian_session_kind: Option<GuardianReviewSessionKind>,
|
||||
pub guardian_model: Option<String>,
|
||||
@@ -343,6 +345,7 @@ impl GuardianReviewTrackContext {
|
||||
risk_level: result.risk_level,
|
||||
user_authorization: result.user_authorization,
|
||||
outcome: result.outcome,
|
||||
decision_source: result.decision_source,
|
||||
guardian_thread_id: result.guardian_thread_id,
|
||||
guardian_session_kind: result.guardian_session_kind,
|
||||
guardian_model: result.guardian_model,
|
||||
@@ -379,6 +382,7 @@ pub struct GuardianReviewAnalyticsResult {
|
||||
pub risk_level: Option<GuardianRiskLevel>,
|
||||
pub user_authorization: Option<GuardianUserAuthorization>,
|
||||
pub outcome: Option<GuardianAssessmentOutcome>,
|
||||
pub decision_source: Option<GuardianAssessmentDecisionSource>,
|
||||
pub guardian_thread_id: Option<String>,
|
||||
pub guardian_session_kind: Option<GuardianReviewSessionKind>,
|
||||
pub guardian_model: Option<String>,
|
||||
@@ -399,6 +403,7 @@ impl GuardianReviewAnalyticsResult {
|
||||
risk_level: None,
|
||||
user_authorization: None,
|
||||
outcome: None,
|
||||
decision_source: None,
|
||||
guardian_thread_id: None,
|
||||
guardian_session_kind: None,
|
||||
guardian_model: None,
|
||||
|
||||
@@ -545,7 +545,8 @@
|
||||
"AutoReviewDecisionSource": {
|
||||
"description": "[UNSTABLE] Source that produced a terminal approval auto-review decision.",
|
||||
"enum": [
|
||||
"agent"
|
||||
"agent",
|
||||
"deterministic"
|
||||
],
|
||||
"type": "string"
|
||||
},
|
||||
|
||||
+2
-1
@@ -6777,7 +6777,8 @@
|
||||
"AutoReviewDecisionSource": {
|
||||
"description": "[UNSTABLE] Source that produced a terminal approval auto-review decision.",
|
||||
"enum": [
|
||||
"agent"
|
||||
"agent",
|
||||
"deterministic"
|
||||
],
|
||||
"type": "string"
|
||||
},
|
||||
|
||||
+2
-1
@@ -1046,7 +1046,8 @@
|
||||
"AutoReviewDecisionSource": {
|
||||
"description": "[UNSTABLE] Source that produced a terminal approval auto-review decision.",
|
||||
"enum": [
|
||||
"agent"
|
||||
"agent",
|
||||
"deterministic"
|
||||
],
|
||||
"type": "string"
|
||||
},
|
||||
|
||||
Generated
+2
-1
@@ -61,7 +61,8 @@
|
||||
"AutoReviewDecisionSource": {
|
||||
"description": "[UNSTABLE] Source that produced a terminal approval auto-review decision.",
|
||||
"enum": [
|
||||
"agent"
|
||||
"agent",
|
||||
"deterministic"
|
||||
],
|
||||
"type": "string"
|
||||
},
|
||||
|
||||
+1
-1
@@ -5,4 +5,4 @@
|
||||
/**
|
||||
* [UNSTABLE] Source that produced a terminal approval auto-review decision.
|
||||
*/
|
||||
export type AutoReviewDecisionSource = "agent";
|
||||
export type AutoReviewDecisionSource = "agent" | "deterministic";
|
||||
|
||||
@@ -425,12 +425,14 @@ pub enum GuardianApprovalReviewStatus {
|
||||
/// [UNSTABLE] Source that produced a terminal approval auto-review decision.
|
||||
pub enum AutoReviewDecisionSource {
|
||||
Agent,
|
||||
Deterministic,
|
||||
}
|
||||
|
||||
impl From<CoreGuardianAssessmentDecisionSource> for AutoReviewDecisionSource {
|
||||
fn from(value: CoreGuardianAssessmentDecisionSource) -> Self {
|
||||
match value {
|
||||
CoreGuardianAssessmentDecisionSource::Agent => Self::Agent,
|
||||
CoreGuardianAssessmentDecisionSource::Deterministic => Self::Deterministic,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1328,7 +1328,7 @@ All items emit shared lifecycle events:
|
||||
- `item/autoApprovalReview/started` — [UNSTABLE] temporary auto-review notification carrying `{threadId, turnId, targetItemId, review, action}` when approval auto-review begins. This shape is expected to change soon.
|
||||
- `item/autoApprovalReview/completed` — [UNSTABLE] temporary auto-review notification carrying `{threadId, turnId, targetItemId, review, action}` when approval auto-review resolves. This shape is expected to change soon.
|
||||
|
||||
`review` is [UNSTABLE] and currently has `{status, riskLevel?, userAuthorization?, rationale?}`, where `status` is one of `inProgress`, `approved`, `denied`, or `aborted`. `riskLevel` is one of `"low"`, `"medium"`, `"high"`, or `"critical"` when present. `userAuthorization` is one of `"unknown"`, `"low"`, `"medium"`, or `"high"` when present. `action` is a tagged union with `type: "command" | "execve" | "applyPatch" | "networkAccess" | "mcpToolCall"`. Command-like actions include a `source` discriminator (`"shell"` or `"unifiedExec"`). These notifications are separate from the target item's own `item/completed` lifecycle and are intentionally temporary while the auto-review app protocol is still being designed.
|
||||
`review` is [UNSTABLE] and currently has `{status, riskLevel?, userAuthorization?, rationale?}`, where `status` is one of `inProgress`, `approved`, `denied`, or `aborted`. `riskLevel` is one of `"low"`, `"medium"`, `"high"`, or `"critical"` when present. `userAuthorization` is one of `"unknown"`, `"low"`, `"medium"`, or `"high"` when present. Completed notifications also carry `decisionSource`, currently `"agent"` or `"deterministic"`. `action` is a tagged union with `type: "command" | "execve" | "applyPatch" | "networkAccess" | "mcpToolCall"`. Command-like actions include a `source` discriminator (`"shell"` or `"unifiedExec"`). These notifications are separate from the target item's own `item/completed` lifecycle and are intentionally temporary while the auto-review app protocol is still being designed.
|
||||
|
||||
There are additional item-specific events:
|
||||
|
||||
|
||||
@@ -13,6 +13,7 @@ use codex_otel::GUARDIAN_REVIEW_TOKEN_USAGE_METRIC;
|
||||
use codex_otel::GUARDIAN_REVIEW_TTFT_DURATION_METRIC;
|
||||
use codex_otel::SessionTelemetry;
|
||||
use codex_otel::sanitize_metric_tag_value;
|
||||
use codex_protocol::protocol::GuardianAssessmentDecisionSource;
|
||||
use codex_protocol::protocol::GuardianAssessmentOutcome;
|
||||
use codex_protocol::protocol::GuardianRiskLevel;
|
||||
use codex_protocol::protocol::GuardianUserAuthorization;
|
||||
@@ -115,6 +116,10 @@ fn guardian_review_metric_tags(
|
||||
user_authorization_tag(result.user_authorization).to_string(),
|
||||
),
|
||||
("outcome", outcome_tag(result.outcome).to_string()),
|
||||
(
|
||||
"decision_source",
|
||||
decision_source_tag(result.decision_source).to_string(),
|
||||
),
|
||||
(
|
||||
"guardian_model",
|
||||
result
|
||||
@@ -231,6 +236,14 @@ fn outcome_tag(outcome: Option<GuardianAssessmentOutcome>) -> &'static str {
|
||||
}
|
||||
}
|
||||
|
||||
fn decision_source_tag(decision_source: Option<GuardianAssessmentDecisionSource>) -> &'static str {
|
||||
match decision_source {
|
||||
Some(GuardianAssessmentDecisionSource::Agent) => "agent",
|
||||
Some(GuardianAssessmentDecisionSource::Deterministic) => "deterministic",
|
||||
None => "none",
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -340,6 +353,7 @@ mod tests {
|
||||
risk_level: Some(GuardianRiskLevel::Low),
|
||||
user_authorization: Some(GuardianUserAuthorization::High),
|
||||
outcome: Some(GuardianAssessmentOutcome::Allow),
|
||||
decision_source: Some(GuardianAssessmentDecisionSource::Agent),
|
||||
guardian_session_kind: Some(GuardianReviewSessionKind::TrunkReused),
|
||||
guardian_model: Some("gpt-5.4 guardian".to_string()),
|
||||
guardian_reasoning_effort: Some("low".to_string()),
|
||||
@@ -382,6 +396,7 @@ mod tests {
|
||||
"delegated_subagent".to_string()
|
||||
),
|
||||
("decision".to_string(), "approved".to_string()),
|
||||
("decision_source".to_string(), "agent".to_string()),
|
||||
("failure_reason".to_string(), "none".to_string()),
|
||||
("guardian_model".to_string(), "gpt-5.4_guardian".to_string()),
|
||||
("guardian_reasoning_effort".to_string(), "low".to_string()),
|
||||
|
||||
@@ -29,6 +29,7 @@ pub(crate) use approval_request::GuardianMcpAnnotations;
|
||||
pub(crate) use approval_request::GuardianNetworkAccessTrigger;
|
||||
#[cfg(test)]
|
||||
pub(crate) use approval_request::guardian_approval_request_to_json;
|
||||
pub(crate) use review::GuardianReviewMode;
|
||||
pub(crate) use review::guardian_rejection_message;
|
||||
pub(crate) use review::guardian_timeout_message;
|
||||
pub(crate) use review::is_guardian_reviewer_source;
|
||||
@@ -66,6 +67,7 @@ pub(crate) struct GuardianAssessment {
|
||||
pub(crate) user_authorization: codex_protocol::protocol::GuardianUserAuthorization,
|
||||
pub(crate) outcome: GuardianAssessmentOutcome,
|
||||
pub(crate) rationale: String,
|
||||
pub(crate) source: GuardianAssessmentDecisionSource,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
|
||||
@@ -619,6 +619,7 @@ pub(crate) fn parse_guardian_assessment(text: Option<&str>) -> anyhow::Result<Gu
|
||||
.unwrap_or(GuardianUserAuthorization::Unknown),
|
||||
outcome,
|
||||
rationale,
|
||||
source: codex_protocol::protocol::GuardianAssessmentDecisionSource::Agent,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -81,7 +81,8 @@ pub(crate) async fn guardian_rejection_message(session: &Session, review_id: &st
|
||||
source: GuardianAssessmentDecisionSource::Agent,
|
||||
});
|
||||
match rejection.source {
|
||||
GuardianAssessmentDecisionSource::Agent => format!(
|
||||
GuardianAssessmentDecisionSource::Agent
|
||||
| GuardianAssessmentDecisionSource::Deterministic => format!(
|
||||
"This action was rejected due to unacceptable risk.\nReason: {}\n{}",
|
||||
rejection.rationale.trim(),
|
||||
GUARDIAN_REJECTION_INSTRUCTIONS
|
||||
@@ -99,6 +100,20 @@ pub(super) enum GuardianReviewOutcome {
|
||||
Error(GuardianReviewError),
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub(crate) enum GuardianReviewMode {
|
||||
Agent,
|
||||
#[cfg_attr(not(test), allow(dead_code))]
|
||||
Deterministic(GuardianAssessment),
|
||||
}
|
||||
|
||||
struct GuardianReviewExecution {
|
||||
retry_reason: Option<String>,
|
||||
review_mode: GuardianReviewMode,
|
||||
approval_request_source: GuardianApprovalRequestSource,
|
||||
external_cancel: Option<CancellationToken>,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub(super) enum GuardianReviewError {
|
||||
PromptBuild {
|
||||
@@ -277,10 +292,14 @@ async fn run_guardian_review(
|
||||
turn: Arc<TurnContext>,
|
||||
review_id: String,
|
||||
request: GuardianApprovalRequest,
|
||||
retry_reason: Option<String>,
|
||||
approval_request_source: GuardianApprovalRequestSource,
|
||||
external_cancel: Option<CancellationToken>,
|
||||
execution: GuardianReviewExecution,
|
||||
) -> ReviewDecision {
|
||||
let GuardianReviewExecution {
|
||||
retry_reason,
|
||||
review_mode,
|
||||
approval_request_source,
|
||||
external_cancel,
|
||||
} = execution;
|
||||
let target_item_id = guardian_request_target_item_id(&request).map(str::to_string);
|
||||
let assessment_turn_id = guardian_request_turn_id(&request, &turn.sub_id).to_string();
|
||||
let action_summary = guardian_assessment_action(&request);
|
||||
@@ -354,18 +373,26 @@ async fn run_guardian_review(
|
||||
return ReviewDecision::Abort;
|
||||
}
|
||||
|
||||
let schema = guardian_output_schema();
|
||||
let terminal_action = action_summary.clone();
|
||||
let (outcome, analytics_result) = Box::pin(run_guardian_review_session_with_retry(
|
||||
session.clone(),
|
||||
turn.clone(),
|
||||
request,
|
||||
retry_reason.clone(),
|
||||
schema,
|
||||
external_cancel,
|
||||
GUARDIAN_REVIEW_MAX_ATTEMPTS,
|
||||
))
|
||||
.await;
|
||||
let (outcome, analytics_result) = match review_mode {
|
||||
GuardianReviewMode::Deterministic(assessment) => (
|
||||
GuardianReviewOutcome::Completed(assessment),
|
||||
GuardianReviewAnalyticsResult::without_session(),
|
||||
),
|
||||
GuardianReviewMode::Agent => {
|
||||
let schema = guardian_output_schema();
|
||||
Box::pin(run_guardian_review_session_with_retry(
|
||||
session.clone(),
|
||||
turn.clone(),
|
||||
request,
|
||||
retry_reason.clone(),
|
||||
schema,
|
||||
external_cancel,
|
||||
GUARDIAN_REVIEW_MAX_ATTEMPTS,
|
||||
))
|
||||
.await
|
||||
}
|
||||
};
|
||||
|
||||
let completed_at_ms = now_unix_timestamp_ms();
|
||||
let (assessment, count_denial_for_circuit_breaker) = match outcome {
|
||||
@@ -391,6 +418,7 @@ async fn run_guardian_review(
|
||||
risk_level: Some(assessment.risk_level),
|
||||
user_authorization: Some(assessment.user_authorization),
|
||||
outcome: Some(assessment.outcome),
|
||||
decision_source: Some(assessment.source),
|
||||
..analytics_result
|
||||
},
|
||||
completed_at_ms.try_into().unwrap_or_default(),
|
||||
@@ -512,6 +540,7 @@ async fn run_guardian_review(
|
||||
user_authorization: GuardianUserAuthorization::Unknown,
|
||||
outcome: GuardianAssessmentOutcome::Deny,
|
||||
rationale,
|
||||
source: GuardianAssessmentDecisionSource::Agent,
|
||||
},
|
||||
false,
|
||||
)
|
||||
@@ -553,7 +582,7 @@ async fn run_guardian_review(
|
||||
} else {
|
||||
let rejection = GuardianRejection {
|
||||
rationale: assessment.rationale.clone(),
|
||||
source: GuardianAssessmentDecisionSource::Agent,
|
||||
source: assessment.source,
|
||||
};
|
||||
rationales.insert(review_id.clone(), rejection);
|
||||
}
|
||||
@@ -571,7 +600,7 @@ async fn run_guardian_review(
|
||||
risk_level: Some(assessment.risk_level),
|
||||
user_authorization: Some(assessment.user_authorization),
|
||||
rationale: Some(assessment.rationale.clone()),
|
||||
decision_source: Some(GuardianAssessmentDecisionSource::Agent),
|
||||
decision_source: Some(assessment.source),
|
||||
action: terminal_action,
|
||||
}),
|
||||
)
|
||||
@@ -596,6 +625,7 @@ pub(crate) async fn review_approval_request(
|
||||
turn: &Arc<TurnContext>,
|
||||
review_id: String,
|
||||
request: GuardianApprovalRequest,
|
||||
review_mode: GuardianReviewMode,
|
||||
retry_reason: Option<String>,
|
||||
) -> ReviewDecision {
|
||||
// Box the delegated review future so callers do not inline the entire
|
||||
@@ -605,9 +635,12 @@ pub(crate) async fn review_approval_request(
|
||||
Arc::clone(turn),
|
||||
review_id,
|
||||
request,
|
||||
retry_reason,
|
||||
GuardianApprovalRequestSource::MainTurn,
|
||||
/*external_cancel*/ None,
|
||||
GuardianReviewExecution {
|
||||
retry_reason,
|
||||
review_mode,
|
||||
approval_request_source: GuardianApprovalRequestSource::MainTurn,
|
||||
external_cancel: None,
|
||||
},
|
||||
))
|
||||
.await
|
||||
}
|
||||
@@ -626,9 +659,12 @@ pub(crate) async fn review_approval_request_with_cancel(
|
||||
Arc::clone(turn),
|
||||
review_id,
|
||||
request,
|
||||
retry_reason,
|
||||
approval_request_source,
|
||||
Some(cancel_token),
|
||||
GuardianReviewExecution {
|
||||
retry_reason,
|
||||
review_mode: GuardianReviewMode::Agent,
|
||||
approval_request_source,
|
||||
external_cancel: Some(cancel_token),
|
||||
},
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -948,6 +984,7 @@ mod review_tests {
|
||||
user_authorization: GuardianUserAuthorization::Unknown,
|
||||
outcome: GuardianAssessmentOutcome::Deny,
|
||||
rationale: "deny".to_string(),
|
||||
source: GuardianAssessmentDecisionSource::Agent,
|
||||
};
|
||||
let transient_error_info = [
|
||||
CodexErrorInfo::ServerOverloaded,
|
||||
|
||||
@@ -42,6 +42,7 @@ use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::Event;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::GranularApprovalConfig;
|
||||
use codex_protocol::protocol::GuardianAssessmentDecisionSource;
|
||||
use codex_protocol::protocol::GuardianAssessmentStatus;
|
||||
use codex_protocol::protocol::GuardianRiskLevel;
|
||||
use codex_protocol::protocol::GuardianUserAuthorization;
|
||||
@@ -1120,6 +1121,51 @@ async fn cancelled_guardian_review_emits_terminal_abort_without_warning() {
|
||||
assert!(warnings.is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn deterministic_guardian_review_emits_terminal_assessment_without_model_review() {
|
||||
let (session, turn, rx) = crate::session::tests::make_session_and_context_with_rx().await;
|
||||
|
||||
let decision = review_approval_request(
|
||||
&session,
|
||||
&turn,
|
||||
"review-deterministic-guardian".to_string(),
|
||||
GuardianApprovalRequest::ApplyPatch {
|
||||
id: "patch-1".to_string(),
|
||||
cwd: test_path_buf("/tmp").abs(),
|
||||
files: vec![test_path_buf("/tmp/guardian.txt").abs()],
|
||||
patch: "*** Begin Patch\n*** Update File: guardian.txt\n@@\n+hello\n*** End Patch"
|
||||
.to_string(),
|
||||
},
|
||||
GuardianReviewMode::Deterministic(GuardianAssessment {
|
||||
risk_level: GuardianRiskLevel::Low,
|
||||
user_authorization: GuardianUserAuthorization::High,
|
||||
outcome: GuardianAssessmentOutcome::Allow,
|
||||
rationale: "Exact deterministic policy allowed this action.".to_string(),
|
||||
source: GuardianAssessmentDecisionSource::Deterministic,
|
||||
}),
|
||||
/*retry_reason*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(decision, ReviewDecision::Approved);
|
||||
|
||||
let mut assessments = Vec::new();
|
||||
while let Ok(event) = rx.try_recv() {
|
||||
if let EventMsg::GuardianAssessment(event) = event.msg {
|
||||
assessments.push(event);
|
||||
}
|
||||
}
|
||||
|
||||
assert_eq!(assessments.len(), 2);
|
||||
assert_eq!(assessments[0].status, GuardianAssessmentStatus::InProgress);
|
||||
assert_eq!(assessments[0].decision_source, None);
|
||||
assert_eq!(assessments[1].status, GuardianAssessmentStatus::Approved);
|
||||
assert_eq!(
|
||||
assessments[1].decision_source,
|
||||
Some(GuardianAssessmentDecisionSource::Deterministic)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn guardian_timeout_message_distinguishes_timeout_from_policy_denial() {
|
||||
let message = guardian_timeout_message();
|
||||
@@ -1279,6 +1325,7 @@ fn parse_guardian_assessment_extracts_embedded_json() {
|
||||
user_authorization: GuardianUserAuthorization::Low,
|
||||
outcome: GuardianAssessmentOutcome::Allow,
|
||||
rationale: "ok".to_string(),
|
||||
source: GuardianAssessmentDecisionSource::Agent,
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -1295,6 +1342,7 @@ fn parse_guardian_assessment_treats_bare_allow_as_low_risk() {
|
||||
user_authorization: GuardianUserAuthorization::Unknown,
|
||||
outcome: GuardianAssessmentOutcome::Allow,
|
||||
rationale: "Auto-review returned a low-risk allow decision.".to_string(),
|
||||
source: GuardianAssessmentDecisionSource::Agent,
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -1311,6 +1359,7 @@ fn parse_guardian_assessment_treats_bare_deny_as_high_risk() {
|
||||
user_authorization: GuardianUserAuthorization::Unknown,
|
||||
outcome: GuardianAssessmentOutcome::Deny,
|
||||
rationale: "Auto-review returned a deny decision without a rationale.".to_string(),
|
||||
source: GuardianAssessmentDecisionSource::Agent,
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -2077,6 +2126,7 @@ async fn guardian_review_surfaces_responses_api_errors_in_rejection_reason() ->
|
||||
additional_permissions: None,
|
||||
justification: Some("Need to push the reviewed docs fix.".to_string()),
|
||||
},
|
||||
GuardianReviewMode::Agent,
|
||||
/*retry_reason*/ None,
|
||||
)
|
||||
.await;
|
||||
@@ -2210,6 +2260,7 @@ async fn guardian_review_does_not_retry_missing_assessment_payload() -> anyhow::
|
||||
&turn,
|
||||
"review-missing-assessment".to_string(),
|
||||
guardian_shell_request("shell-missing-assessment"),
|
||||
GuardianReviewMode::Agent,
|
||||
/*retry_reason*/ None,
|
||||
)
|
||||
.await;
|
||||
@@ -2314,6 +2365,7 @@ async fn guardian_review_exhausts_three_failures_with_one_terminal_event() -> an
|
||||
&turn,
|
||||
"review-exhausted-retry".to_string(),
|
||||
guardian_shell_request("shell-exhausted-retry"),
|
||||
GuardianReviewMode::Agent,
|
||||
/*retry_reason*/ None,
|
||||
)
|
||||
.await;
|
||||
@@ -2365,6 +2417,7 @@ async fn guardian_review_does_not_retry_valid_denial() -> anyhow::Result<()> {
|
||||
&turn,
|
||||
"review-valid-denial".to_string(),
|
||||
guardian_shell_request("shell-valid-denial"),
|
||||
GuardianReviewMode::Agent,
|
||||
/*retry_reason*/ None,
|
||||
)
|
||||
.await;
|
||||
@@ -2468,6 +2521,7 @@ async fn guardian_ephemeral_retry_preserves_parallel_trunk_and_fork_history() ->
|
||||
&turn,
|
||||
"review-shell-guardian-1".to_string(),
|
||||
initial_request,
|
||||
GuardianReviewMode::Agent,
|
||||
/*retry_reason*/ None
|
||||
)
|
||||
.await,
|
||||
@@ -2522,6 +2576,7 @@ async fn guardian_ephemeral_retry_preserves_parallel_trunk_and_fork_history() ->
|
||||
&turn_for_second,
|
||||
"review-shell-guardian-2".to_string(),
|
||||
second_request,
|
||||
GuardianReviewMode::Agent,
|
||||
Some("trunk follow-up".to_string()),
|
||||
)
|
||||
.await
|
||||
@@ -2569,6 +2624,7 @@ async fn guardian_ephemeral_retry_preserves_parallel_trunk_and_fork_history() ->
|
||||
&turn,
|
||||
"review-shell-guardian-3".to_string(),
|
||||
third_request,
|
||||
GuardianReviewMode::Agent,
|
||||
Some("parallel follow-up".to_string()),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -9,6 +9,7 @@ use crate::config::edit::ConfigEditsBuilder;
|
||||
use crate::connectors;
|
||||
use crate::guardian::GuardianApprovalRequest;
|
||||
use crate::guardian::GuardianMcpAnnotations;
|
||||
use crate::guardian::GuardianReviewMode;
|
||||
use crate::guardian::guardian_rejection_message;
|
||||
use crate::guardian::guardian_timeout_message;
|
||||
use crate::guardian::new_guardian_review_id;
|
||||
@@ -1213,6 +1214,7 @@ async fn maybe_request_mcp_tool_approval(
|
||||
turn_context,
|
||||
review_id.clone(),
|
||||
build_guardian_mcp_tool_review_request(call_id, invocation, metadata),
|
||||
GuardianReviewMode::Agent,
|
||||
/*retry_reason*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -466,6 +466,7 @@ async fn review_guardian_mcp_elicitation(
|
||||
&turn_context,
|
||||
review_id.clone(),
|
||||
guardian_request,
|
||||
crate::guardian::GuardianReviewMode::Agent,
|
||||
/*retry_reason*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use crate::guardian::GuardianApprovalRequest;
|
||||
use crate::guardian::GuardianNetworkAccessTrigger;
|
||||
use crate::guardian::GuardianReviewMode;
|
||||
use crate::guardian::guardian_rejection_message;
|
||||
use crate::guardian::guardian_timeout_message;
|
||||
use crate::guardian::new_guardian_review_id;
|
||||
@@ -514,6 +515,7 @@ impl NetworkApprovalService {
|
||||
port: key.port,
|
||||
trigger: owner_call.as_ref().map(|call| call.trigger.clone()),
|
||||
},
|
||||
GuardianReviewMode::Agent,
|
||||
Some(policy_denial_message.clone()),
|
||||
)
|
||||
.await
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
//! sandboxing enforced by the explicit filesystem sandbox context.
|
||||
use crate::exec::is_likely_sandbox_denied;
|
||||
use crate::guardian::GuardianApprovalRequest;
|
||||
use crate::guardian::GuardianReviewMode;
|
||||
use crate::guardian::review_approval_request;
|
||||
use crate::session::turn_context::TurnEnvironment;
|
||||
use crate::tools::hook_names::HookToolName;
|
||||
@@ -144,8 +145,15 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
|
||||
Box::pin(async move {
|
||||
if let Some(review_id) = guardian_review_id {
|
||||
let action = ApplyPatchRuntime::build_guardian_review_request(req, ctx.call_id);
|
||||
return review_approval_request(session, turn, review_id, action, retry_reason)
|
||||
.await;
|
||||
return review_approval_request(
|
||||
session,
|
||||
turn,
|
||||
review_id,
|
||||
action,
|
||||
GuardianReviewMode::Agent,
|
||||
retry_reason,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
if req.permissions_preapproved && retry_reason.is_none() {
|
||||
return ReviewDecision::Approved;
|
||||
|
||||
@@ -12,6 +12,7 @@ use crate::command_canonicalization::canonicalize_command_for_approval;
|
||||
use crate::exec::ExecCapturePolicy;
|
||||
use crate::guardian::GuardianApprovalRequest;
|
||||
use crate::guardian::GuardianNetworkAccessTrigger;
|
||||
use crate::guardian::GuardianReviewMode;
|
||||
use crate::guardian::review_approval_request;
|
||||
use crate::sandboxing::ExecOptions;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
@@ -160,6 +161,7 @@ impl Approvable<ShellRequest> for ShellRuntime {
|
||||
additional_permissions: req.additional_permissions.clone(),
|
||||
justification: req.justification.clone(),
|
||||
},
|
||||
GuardianReviewMode::Agent,
|
||||
retry_reason,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -4,6 +4,7 @@ use crate::exec::ExecExpiration;
|
||||
use crate::exec::cancel_when_either;
|
||||
use crate::exec::is_likely_sandbox_denied;
|
||||
use crate::guardian::GuardianApprovalRequest;
|
||||
use crate::guardian::GuardianReviewMode;
|
||||
use crate::guardian::guardian_rejection_message;
|
||||
use crate::guardian::guardian_timeout_message;
|
||||
use crate::guardian::new_guardian_review_id;
|
||||
@@ -472,6 +473,7 @@ impl CoreShellActionProvider {
|
||||
cwd: workdir.clone(),
|
||||
additional_permissions,
|
||||
},
|
||||
GuardianReviewMode::Agent,
|
||||
/*retry_reason*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -9,6 +9,7 @@ use crate::exec::ExecCapturePolicy;
|
||||
use crate::exec::ExecExpiration;
|
||||
use crate::guardian::GuardianApprovalRequest;
|
||||
use crate::guardian::GuardianNetworkAccessTrigger;
|
||||
use crate::guardian::GuardianReviewMode;
|
||||
use crate::guardian::review_approval_request;
|
||||
use crate::sandboxing::ExecOptions;
|
||||
use crate::sandboxing::ExecServerEnvConfig;
|
||||
@@ -173,6 +174,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
justification: req.justification.clone(),
|
||||
tty: req.tty,
|
||||
},
|
||||
GuardianReviewMode::Agent,
|
||||
retry_reason,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -122,6 +122,7 @@ pub enum GuardianAssessmentStatus {
|
||||
#[serde(rename_all = "snake_case")]
|
||||
pub enum GuardianAssessmentDecisionSource {
|
||||
Agent,
|
||||
Deterministic,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)]
|
||||
|
||||
@@ -127,6 +127,9 @@ impl ChatWidget {
|
||||
codex_app_server_protocol::AutoReviewDecisionSource::Agent => {
|
||||
GuardianAssessmentDecisionSource::Agent
|
||||
}
|
||||
codex_app_server_protocol::AutoReviewDecisionSource::Deterministic => {
|
||||
GuardianAssessmentDecisionSource::Deterministic
|
||||
}
|
||||
}),
|
||||
action: action.into(),
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user