openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-03 19:53:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

Preserve timeout approval outcomes

Browse Source 
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
Charles Cunningham
2026-03-24 16:16:14 -07:00
co-authored by Codex
parent 28f9b6549e
commit 734db45cb1
34 changed files with 574 additions and 243 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/src/bespoke_event_handling.rs
+10 -6
View File
@@ -2213,7 +2213,9 @@ async fn on_patch_approval_response(
if let Err(submit_err) = codex
.submit(Op::PatchApproval {
id: call_id.clone(),
decision: ReviewDecision::Denied,
outcome: codex_protocol::protocol::ApprovalOutcome::from(
ReviewDecision::Denied,
),
})
.await
{
@@ -2226,7 +2228,9 @@ async fn on_patch_approval_response(
if let Err(submit_err) = codex
.submit(Op::PatchApproval {
id: call_id.clone(),
decision: ReviewDecision::Denied,
outcome: codex_protocol::protocol::ApprovalOutcome::from(
ReviewDecision::Denied,
),
})
.await
{
@@ -2247,7 +2251,7 @@ async fn on_patch_approval_response(
if let Err(err) = codex
.submit(Op::PatchApproval {
id: call_id,
decision: response.decision,
outcome: codex_protocol::protocol::ApprovalOutcome::from(response.decision),
})
.await
{
@@ -2290,7 +2294,7 @@ async fn on_exec_approval_response(
.submit(Op::ExecApproval {
id: call_id,
turn_id: Some(turn_id),
decision: response.decision,
outcome: codex_protocol::protocol::ApprovalOutcome::from(response.decision),
})
.await
{
@@ -2612,7 +2616,7 @@ async fn on_file_change_request_approval_response(
if let Err(err) = codex
.submit(Op::PatchApproval {
id: item_id,
decision,
outcome: codex_protocol::protocol::ApprovalOutcome::from(decision),
})
.await
{
@@ -2736,7 +2740,7 @@ async fn on_command_execution_request_approval_response(
.submit(Op::ExecApproval {
id: approval_id.unwrap_or_else(|| item_id.clone()),
turn_id: Some(event_turn_id),
decision,
outcome: codex_protocol::protocol::ApprovalOutcome::from(decision),
})
.await
{
codex-rs/core/src/codex.rs
+29 -18
View File
@@ -265,6 +265,7 @@ use crate::project_doc::get_user_instructions;
use crate::protocol::AgentMessageContentDeltaEvent;
use crate::protocol::AgentReasoningSectionBreakEvent;
use crate::protocol::ApplyPatchApprovalRequestEvent;
use crate::protocol::ApprovalOutcome;
use crate::protocol::AskForApproval;
use crate::protocol::BackgroundEventEvent;
use crate::protocol::CompactedItem;
@@ -2889,7 +2890,7 @@ impl Session {
additional_permissions: Option<PermissionProfile>,
skill_metadata: Option<ExecApprovalRequestSkillMetadata>,
available_decisions: Option<Vec<ReviewDecision>>,
) -> ReviewDecision {
) -> ApprovalOutcome {
// command-level approvals use `call_id`.
// `approval_id` is only present for subcommand callbacks (execve intercept)
let effective_approval_id = approval_id.clone().unwrap_or_else(|| call_id.clone());
@@ -2946,7 +2947,9 @@ impl Session {
parsed_cmd,
});
self.send_event(turn_context, event).await;
rx_approve.await.unwrap_or(ReviewDecision::Abort)
rx_approve
.await
.unwrap_or_else(|_| ApprovalOutcome::from(ReviewDecision::Abort))
}
pub async fn request_patch_approval(
@@ -2956,7 +2959,7 @@ impl Session {
changes: HashMap<PathBuf, FileChange>,
reason: Option<String>,
grant_root: Option<PathBuf>,
) -> oneshot::Receiver<ReviewDecision> {
) -> oneshot::Receiver<ApprovalOutcome> {
// Add the tx_approve callback to the map before sending the request.
let (tx_approve, rx_approve) = oneshot::channel();
let approval_id = call_id.clone();
@@ -3250,7 +3253,7 @@ impl Session {
}
}
pub async fn notify_approval(&self, approval_id: &str, decision: ReviewDecision) {
pub async fn notify_approval(&self, approval_id: &str, outcome: ApprovalOutcome) {
let entry = {
let mut active = self.active_turn.lock().await;
match active.as_mut() {
@@ -3263,7 +3266,7 @@ impl Session {
};
match entry {
Some(tx_approve) => {
tx_approve.send(decision).ok();
tx_approve.send(outcome).ok();
}
None => {
warn!("No pending approval found for call_id: {approval_id}");
@@ -4339,13 +4342,13 @@ async fn submission_loop(sess: Arc<Session>, config: Arc<Config>, rx_sub: Receiv
Op::ExecApproval {
id: approval_id,
turn_id,
decision,
outcome,
} => {
handlers::exec_approval(&sess, approval_id, turn_id, decision).await;
handlers::exec_approval(&sess, approval_id, turn_id, outcome).await;
false
}
Op::PatchApproval { id, decision } => {
handlers::patch_approval(&sess, id, decision).await;
Op::PatchApproval { id, outcome } => {
handlers::patch_approval(&sess, id, outcome).await;
false
}
Op::UserInputAnswer { id, response } => {
@@ -4511,6 +4514,7 @@ mod handlers {
use crate::tasks::UserShellCommandTask;
use crate::tasks::execute_user_shell_command;
use codex_protocol::custom_prompts::CustomPrompt;
use codex_protocol::protocol::ApprovalOutcome;
use codex_protocol::protocol::CodexErrorInfo;
use codex_protocol::protocol::ErrorEvent;
use codex_protocol::protocol::Event;
@@ -4755,12 +4759,15 @@ mod handlers {
sess: &Arc<Session>,
approval_id: String,
turn_id: Option<String>,
decision: ReviewDecision,
outcome: ApprovalOutcome,
) {
let event_turn_id = turn_id.unwrap_or_else(|| approval_id.clone());
if let ReviewDecision::ApprovedExecpolicyAmendment {
proposed_execpolicy_amendment,
} = &decision
if let ApprovalOutcome::Decision {
decision:
ReviewDecision::ApprovedExecpolicyAmendment {
proposed_execpolicy_amendment,
},
} = &outcome
{
match sess
.persist_execpolicy_amendment(proposed_execpolicy_amendment)
@@ -4785,17 +4792,21 @@ mod handlers {
}
}
}
match decision {
ReviewDecision::Abort => {
match outcome {
ApprovalOutcome::Decision {
decision: ReviewDecision::Abort,
} => {
sess.interrupt_task().await;
}
other => sess.notify_approval(&approval_id, other).await,
}
}
pub async fn patch_approval(sess: &Arc<Session>, id: String, decision: ReviewDecision) {
match decision {
ReviewDecision::Abort => {
pub async fn patch_approval(sess: &Arc<Session>, id: String, outcome: ApprovalOutcome) {
match outcome {
ApprovalOutcome::Decision {
decision: ReviewDecision::Abort,
} => {
sess.interrupt_task().await;
}
other => sess.notify_approval(&id, other).await,
codex-rs/core/src/codex_delegate.rs
+54 -29
View File
@@ -5,6 +5,7 @@ use async_channel::Receiver;
use async_channel::Sender;
use codex_async_utils::OrCancelExt;
use codex_protocol::protocol::ApplyPatchApprovalRequestEvent;
use codex_protocol::protocol::ApprovalOutcome;
use codex_protocol::protocol::Event;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::ExecApprovalRequestEvent;
@@ -45,6 +46,7 @@ use crate::guardian::routes_approval_to_guardian;
use crate::mcp_tool_call::MCP_TOOL_APPROVAL_ACCEPT;
use crate::mcp_tool_call::MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION;
use crate::mcp_tool_call::MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC;
use crate::mcp_tool_call::MCP_TOOL_APPROVAL_TIMED_OUT_SYNTHETIC;
use crate::mcp_tool_call::build_guardian_mcp_tool_review_request;
use crate::mcp_tool_call::is_mcp_tool_approval_question_id;
use crate::mcp_tool_call::lookup_mcp_tool_metadata;
@@ -437,7 +439,7 @@ async fn handle_exec_approval(
available_decisions,
..
} = event;
let decision = if routes_approval_to_guardian(parent_ctx) {
let outcome = if routes_approval_to_guardian(parent_ctx) {
let review_cancel = cancel_token.child_token();
let review_rx = spawn_guardian_review(
Arc::clone(parent_session),
@@ -458,7 +460,11 @@ async fn handle_exec_approval(
review_cancel.clone(),
);
await_approval_with_cancel(
async move { review_rx.await.unwrap_or_default() },
async move {
review_rx
.await
.unwrap_or_else(|_| ApprovalOutcome::from(ReviewDecision::Denied))
},
parent_session,
&approval_id_for_op,
cancel_token,
@@ -492,7 +498,7 @@ async fn handle_exec_approval(
.submit(Op::ExecApproval {
id: approval_id_for_op,
turn_id: Some(turn_id),
decision,
outcome,
})
.await;
}
@@ -514,7 +520,7 @@ async fn handle_patch_approval(
..
} = event;
let approval_id = call_id.clone();
let guardian_decision = if routes_approval_to_guardian(parent_ctx) {
let guardian_outcome = if routes_approval_to_guardian(parent_ctx) {
let change_count = changes.len();
let maybe_files = changes
.keys()
@@ -564,7 +570,11 @@ async fn handle_patch_approval(
);
Some(
await_approval_with_cancel(
async move { review_rx.await.unwrap_or_default() },
async move {
review_rx
.await
.unwrap_or_else(|_| ApprovalOutcome::from(ReviewDecision::Denied))
},
parent_session,
&approval_id,
cancel_token,
@@ -578,14 +588,18 @@ async fn handle_patch_approval(
} else {
None
};
let decision = if let Some(decision) = guardian_decision {
decision
let outcome = if let Some(outcome) = guardian_outcome {
outcome
} else {
let decision_rx = parent_session
.request_patch_approval(parent_ctx, call_id, changes, reason, grant_root)
.await;
await_approval_with_cancel(
async move { decision_rx.await.unwrap_or_default() },
async move {
decision_rx
.await
.unwrap_or_else(|_| ApprovalOutcome::from(ReviewDecision::Abort))
},
parent_session,
&approval_id,
cancel_token,
@@ -596,7 +610,7 @@ async fn handle_patch_approval(
let _ = codex
.submit(Op::PatchApproval {
id: approval_id,
decision,
outcome,
})
.await;
}
@@ -680,16 +694,22 @@ async fn maybe_auto_review_mcp_request_user_input(
/*retry_reason*/ None,
review_cancel.clone(),
);
let decision = await_approval_with_cancel(
async move { review_rx.await.unwrap_or_default() },
let outcome = await_approval_with_cancel(
async move {
review_rx
.await
.unwrap_or_else(|_| ApprovalOutcome::from(ReviewDecision::Denied))
},
parent_session,
&event.call_id,
cancel_token,
Some(&review_cancel),
)
.await;
let selected_label = match decision {
ReviewDecision::ApprovedForSession => question
let selected_label = match outcome {
ApprovalOutcome::Decision {
decision: ReviewDecision::ApprovedForSession,
} => question
.options
.as_ref()
.and_then(|options| {
@@ -699,12 +719,16 @@ async fn maybe_auto_review_mcp_request_user_input(
})
.map(|option| option.label.clone())
.unwrap_or_else(|| MCP_TOOL_APPROVAL_ACCEPT.to_string()),
ReviewDecision::Approved
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
| ReviewDecision::NetworkPolicyAmendment { .. } => MCP_TOOL_APPROVAL_ACCEPT.to_string(),
ReviewDecision::Denied | ReviewDecision::Abort => {
MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC.to_string()
}
ApprovalOutcome::Decision {
decision:
ReviewDecision::Approved
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
| ReviewDecision::NetworkPolicyAmendment { .. },
} => MCP_TOOL_APPROVAL_ACCEPT.to_string(),
ApprovalOutcome::TimedOut => MCP_TOOL_APPROVAL_TIMED_OUT_SYNTHETIC.to_string(),
ApprovalOutcome::Decision {
decision: ReviewDecision::Denied | ReviewDecision::Abort,
} => MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC.to_string(),
};
Some(RequestUserInputResponse {
answers: HashMap::from([(
@@ -722,14 +746,14 @@ fn spawn_guardian_review(
request: GuardianApprovalRequest,
retry_reason: Option<String>,
cancel_token: CancellationToken,
) -> oneshot::Receiver<ReviewDecision> {
) -> oneshot::Receiver<ApprovalOutcome> {
let (tx, rx) = oneshot::channel();
std::thread::spawn(move || {
let Ok(runtime) = tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
else {
let _ = tx.send(ReviewDecision::Denied);
let _ = tx.send(ApprovalOutcome::from(ReviewDecision::Denied));
return;
};
let decision = runtime.block_on(review_approval_request_with_cancel(
@@ -739,7 +763,7 @@ fn spawn_guardian_review(
retry_reason,
cancel_token,
));
let _ = tx.send(decision.into_review_decision());
let _ = tx.send(decision.into());
});
rx
}
@@ -822,16 +846,16 @@ where
}
}
/// Await an approval decision, aborting on cancellation.
/// Await an approval outcome, aborting on cancellation.
async fn await_approval_with_cancel<F>(
fut: F,
parent_session: &Session,
approval_id: &str,
cancel_token: &CancellationToken,
review_cancel_token: Option<&CancellationToken>,
) -> codex_protocol::protocol::ReviewDecision
) -> ApprovalOutcome
where
F: core::future::Future<Output = codex_protocol::protocol::ReviewDecision>,
F: core::future::Future<Output = ApprovalOutcome>,
{
tokio::select! {
biased;
@@ -839,13 +863,14 @@ where
if let Some(review_cancel_token) = review_cancel_token {
review_cancel_token.cancel();
}
let abort = ApprovalOutcome::from(ReviewDecision::Abort);
parent_session
.notify_approval(approval_id, codex_protocol::protocol::ReviewDecision::Abort)
.notify_approval(approval_id, abort.clone())
.await;
codex_protocol::protocol::ReviewDecision::Abort
abort
}
decision = fut => {
decision
outcome = fut => {
outcome
}
}
}
codex-rs/core/src/codex_delegate_tests.rs
+87 -1
View File
@@ -6,6 +6,7 @@ use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::models::NetworkPermissions;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::AgentStatus;
use codex_protocol::protocol::ApprovalOutcome;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::ExecApprovalRequestEvent;
@@ -342,7 +343,92 @@ async fn handle_exec_approval_uses_call_id_for_guardian_review_and_approval_id_f
Op::ExecApproval {
id: "callback-approval-1".to_string(),
turn_id: Some("child-turn-1".to_string()),
decision: ReviewDecision::Abort,
outcome: ApprovalOutcome::from(ReviewDecision::Abort),
}
);
}
#[tokio::test]
async fn handle_exec_approval_preserves_timed_out_outcome_for_reply() {
let (parent_session, parent_ctx, rx_events) =
crate::codex::make_session_and_context_with_rx().await;
*parent_session.active_turn.lock().await = Some(crate::state::ActiveTurn::default());
let (tx_sub, rx_sub) = bounded(SUBMISSION_CHANNEL_CAPACITY);
let (_tx_events, rx_events_child) = bounded(SUBMISSION_CHANNEL_CAPACITY);
let (_agent_status_tx, agent_status) = watch::channel(AgentStatus::PendingInit);
let codex = Arc::new(Codex {
tx_sub,
rx_event: rx_events_child,
agent_status,
session: Arc::clone(&parent_session),
session_loop_termination: completed_session_loop_termination(),
});
let cancel_token = CancellationToken::new();
let handle = tokio::spawn({
let codex = Arc::clone(&codex);
let parent_session = Arc::clone(&parent_session);
let parent_ctx = Arc::clone(&parent_ctx);
let cancel_token = cancel_token.clone();
async move {
handle_exec_approval(
codex.as_ref(),
"child-turn-1".to_string(),
&parent_session,
&parent_ctx,
ExecApprovalRequestEvent {
call_id: "command-item-1".to_string(),
approval_id: Some("callback-approval-1".to_string()),
turn_id: "child-turn-1".to_string(),
command: vec!["rm".to_string(), "-rf".to_string(), "tmp".to_string()],
cwd: PathBuf::from("/tmp"),
reason: Some("unsafe subcommand".to_string()),
network_approval_context: None,
proposed_execpolicy_amendment: None,
proposed_network_policy_amendments: None,
additional_permissions: None,
skill_metadata: None,
available_decisions: Some(vec![
ReviewDecision::Approved,
ReviewDecision::Abort,
]),
parsed_cmd: Vec::new(),
},
&cancel_token,
)
.await;
}
});
let request_event = timeout(Duration::from_secs(2), rx_events.recv())
.await
.expect("timed out waiting for exec approval request")
.expect("exec approval request missing");
let EventMsg::ExecApprovalRequest(request) = request_event.msg else {
panic!("expected ExecApprovalRequest event");
};
assert_eq!(request.effective_approval_id(), "callback-approval-1");
parent_session
.notify_approval("callback-approval-1", ApprovalOutcome::TimedOut)
.await;
timeout(Duration::from_secs(2), handle)
.await
.expect("handle_exec_approval hung")
.expect("handle_exec_approval join error");
let submission = timeout(Duration::from_secs(2), rx_sub.recv())
.await
.expect("exec approval response timed out")
.expect("exec approval response missing");
assert_eq!(
submission.op,
Op::ExecApproval {
id: "callback-approval-1".to_string(),
turn_id: Some("child-turn-1".to_string()),
outcome: ApprovalOutcome::TimedOut,
}
);
}
codex-rs/core/src/guardian/review.rs
+10
View File
@@ -1,6 +1,7 @@
use std::sync::Arc;
use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::protocol::ApprovalOutcome;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::GuardianAssessmentEvent;
@@ -62,6 +63,15 @@ impl GuardianApprovalDecision {
}
}
impl From<GuardianApprovalDecision> for ApprovalOutcome {
fn from(value: GuardianApprovalDecision) -> Self {
match value {
GuardianApprovalDecision::TimedOut => Self::TimedOut,
decision => Self::from(decision.into_review_decision()),
}
}
}
#[derive(Debug)]
pub(super) enum GuardianReviewOutcome {
Completed(anyhow::Result<GuardianAssessment>),
codex-rs/core/src/mcp_tool_call.rs
+6
View File
@@ -455,6 +455,7 @@ pub(crate) const MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION: &str = "Allow for this se
// real "Decline" answer, so this lets guardian denials round-trip distinctly from user cancel.
// This is not a user-facing option.
pub(crate) const MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC: &str = "__codex_mcp_decline__";
pub(crate) const MCP_TOOL_APPROVAL_TIMED_OUT_SYNTHETIC: &str = "__codex_mcp_timed_out__";
const MCP_TOOL_APPROVAL_ACCEPT_AND_REMEMBER: &str = "Allow and don't ask me again";
const MCP_TOOL_APPROVAL_CANCEL: &str = "Cancel";
const MCP_TOOL_APPROVAL_KIND_KEY: &str = "codex_approval_kind";
@@ -1195,6 +1196,11 @@ fn parse_mcp_tool_approval_response(
.any(|answer| answer == MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC)
{
McpToolApprovalDecision::Decline
} else if answers
.iter()
.any(|answer| answer == MCP_TOOL_APPROVAL_TIMED_OUT_SYNTHETIC)
{
McpToolApprovalDecision::TimedOut(GUARDIAN_TIMEOUT_MESSAGE.to_string())
} else if answers
.iter()
.any(|answer| answer == MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION)
codex-rs/core/src/mcp_tool_call_tests.rs
+20
View File
@@ -847,6 +847,26 @@ fn synthetic_decline_request_user_input_response_stays_decline() {
assert_eq!(response, McpToolApprovalDecision::Decline);
}
#[test]
fn synthetic_timeout_request_user_input_response_stays_timed_out() {
let response = parse_mcp_tool_approval_response(
Some(RequestUserInputResponse {
answers: HashMap::from([(
"approval".to_string(),
RequestUserInputAnswer {
answers: vec![MCP_TOOL_APPROVAL_TIMED_OUT_SYNTHETIC.to_string()],
},
)]),
}),
"approval",
);
assert_eq!(
response,
McpToolApprovalDecision::TimedOut(GUARDIAN_TIMEOUT_MESSAGE.to_string())
);
}
#[test]
fn accepted_elicitation_response_uses_always_persist_meta() {
let response = parse_mcp_tool_approval_elicitation_response(
codex-rs/core/src/state/turn.rs
+5 -5
View File
@@ -18,7 +18,7 @@ use rmcp::model::RequestId;
use tokio::sync::oneshot;
use crate::codex::TurnContext;
use crate::protocol::ReviewDecision;
use crate::protocol::ApprovalOutcome;
use crate::protocol::TokenUsage;
use crate::tasks::SessionTask;
use codex_protocol::models::PermissionProfile;
@@ -75,7 +75,7 @@ impl ActiveTurn {
/// Mutable state for a single turn.
#[derive(Default)]
pub(crate) struct TurnState {
pending_approvals: HashMap<String, oneshot::Sender<ReviewDecision>>,
pending_approvals: HashMap<String, oneshot::Sender<ApprovalOutcome>>,
pending_request_permissions: HashMap<String, oneshot::Sender<RequestPermissionsResponse>>,
pending_user_input: HashMap<String, oneshot::Sender<RequestUserInputResponse>>,
pending_elicitations: HashMap<(String, RequestId), oneshot::Sender<ElicitationResponse>>,
@@ -90,15 +90,15 @@ impl TurnState {
pub(crate) fn insert_pending_approval(
&mut self,
key: String,
tx: oneshot::Sender<ReviewDecision>,
) -> Option<oneshot::Sender<ReviewDecision>> {
tx: oneshot::Sender<ApprovalOutcome>,
) -> Option<oneshot::Sender<ApprovalOutcome>> {
self.pending_approvals.insert(key, tx)
}
pub(crate) fn remove_pending_approval(
&mut self,
key: &str,
) -> Option<oneshot::Sender<ReviewDecision>> {
) -> Option<oneshot::Sender<ApprovalOutcome>> {
self.pending_approvals.remove(key)
}
codex-rs/core/src/tools/network_approval.rs
+28 -7
View File
@@ -17,6 +17,7 @@ use codex_network_proxy::NetworkProxy;
use codex_protocol::approvals::NetworkApprovalContext;
use codex_protocol::approvals::NetworkApprovalProtocol;
use codex_protocol::approvals::NetworkPolicyRuleAction;
use codex_protocol::protocol::ApprovalOutcome;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::Event;
use codex_protocol::protocol::EventMsg;
@@ -420,12 +421,18 @@ impl NetworkApprovalService {
)
.await
{
ReviewDecision::Approved | ReviewDecision::ApprovedExecpolicyAmendment { .. } => {
PendingApprovalDecision::AllowOnce
}
ReviewDecision::ApprovedForSession => PendingApprovalDecision::AllowForSession,
ReviewDecision::NetworkPolicyAmendment {
network_policy_amendment,
ApprovalOutcome::Decision {
decision:
ReviewDecision::Approved | ReviewDecision::ApprovedExecpolicyAmendment { .. },
} => PendingApprovalDecision::AllowOnce,
ApprovalOutcome::Decision {
decision: ReviewDecision::ApprovedForSession,
} => PendingApprovalDecision::AllowForSession,
ApprovalOutcome::Decision {
decision:
ReviewDecision::NetworkPolicyAmendment {
network_policy_amendment,
},
} => match network_policy_amendment.action {
NetworkPolicyRuleAction::Allow => {
match session
@@ -496,7 +503,9 @@ impl NetworkApprovalService {
PendingApprovalDecision::Deny
}
},
ReviewDecision::Denied | ReviewDecision::Abort => {
ApprovalOutcome::Decision {
decision: ReviewDecision::Denied | ReviewDecision::Abort,
} => {
if let Some(owner_call) = owner_call.as_ref() {
self.record_call_outcome(
&owner_call.registration_id,
@@ -506,6 +515,18 @@ impl NetworkApprovalService {
}
PendingApprovalDecision::Deny
}
ApprovalOutcome::TimedOut => {
if let Some(owner_call) = owner_call.as_ref() {
self.record_call_outcome(
&owner_call.registration_id,
NetworkApprovalOutcome::TimedOutByReviewer(
GUARDIAN_TIMEOUT_MESSAGE.to_string(),
),
)
.await;
}
PendingApprovalDecision::Deny
}
}
};
codex-rs/core/src/tools/orchestrator.rs
+39 -27
View File
@@ -19,7 +19,6 @@ use crate::tools::network_approval::begin_network_approval;
use crate::tools::network_approval::finish_deferred_network_approval;
use crate::tools::network_approval::finish_immediate_network_approval;
use crate::tools::sandboxing::ApprovalCtx;
use crate::tools::sandboxing::ApprovalOutcome;
use crate::tools::sandboxing::ExecApprovalRequirement;
use crate::tools::sandboxing::SandboxAttempt;
use crate::tools::sandboxing::SandboxOverride;
@@ -28,6 +27,7 @@ use crate::tools::sandboxing::ToolError;
use crate::tools::sandboxing::ToolRuntime;
use crate::tools::sandboxing::default_exec_approval_requirement;
use codex_otel::ToolDecisionSource;
use codex_protocol::protocol::ApprovalOutcome;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::NetworkPolicyRuleAction;
use codex_protocol::protocol::ReviewDecision;
@@ -135,7 +135,12 @@ impl ToolOrchestrator {
});
match requirement {
ExecApprovalRequirement::Skip { .. } => {
otel.tool_decision(otel_tn, otel_ci, &ReviewDecision::Approved, otel_cfg);
otel.tool_decision(
otel_tn,
otel_ci,
&ApprovalOutcome::from(ReviewDecision::Approved),
otel_cfg,
);
}
ExecApprovalRequirement::Forbidden { reason } => {
return Err(ToolError::Rejected(reason));
@@ -154,26 +159,29 @@ impl ToolOrchestrator {
} else {
otel_user.clone()
};
let decision = approval_outcome.decision_for_telemetry();
otel.tool_decision(otel_tn, otel_ci, &decision, otel_source);
otel.tool_decision(otel_tn, otel_ci, &approval_outcome, otel_source);
match approval_outcome {
ApprovalOutcome::TimedOut => {
return Err(ToolError::Message(GUARDIAN_TIMEOUT_MESSAGE.to_string()));
}
ApprovalOutcome::Decision(ReviewDecision::Denied)
| ApprovalOutcome::Decision(ReviewDecision::Abort) => {
ApprovalOutcome::Decision {
decision: ReviewDecision::Denied | ReviewDecision::Abort,
} => {
return Err(approval_denial_tool_error(turn_ctx));
}
ApprovalOutcome::Decision(
ReviewDecision::Approved
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
| ReviewDecision::ApprovedForSession,
) => {}
ApprovalOutcome::Decision(ReviewDecision::NetworkPolicyAmendment {
network_policy_amendment,
}) => match network_policy_amendment.action {
ApprovalOutcome::Decision {
decision:
ReviewDecision::Approved
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
| ReviewDecision::ApprovedForSession,
} => {}
ApprovalOutcome::Decision {
decision:
ReviewDecision::NetworkPolicyAmendment {
network_policy_amendment,
},
} => match network_policy_amendment.action {
NetworkPolicyRuleAction::Allow => {}
NetworkPolicyRuleAction::Deny => {
return Err(ToolError::Rejected("rejected by user".to_string()));
@@ -310,25 +318,29 @@ impl ToolOrchestrator {
} else {
otel_user
};
let decision = approval_outcome.decision_for_telemetry();
otel.tool_decision(otel_tn, otel_ci, &decision, otel_source);
otel.tool_decision(otel_tn, otel_ci, &approval_outcome, otel_source);
match approval_outcome {
ApprovalOutcome::TimedOut => {
return Err(ToolError::Message(GUARDIAN_TIMEOUT_MESSAGE.to_string()));
}
ApprovalOutcome::Decision(ReviewDecision::Denied)
| ApprovalOutcome::Decision(ReviewDecision::Abort) => {
ApprovalOutcome::Decision {
decision: ReviewDecision::Denied | ReviewDecision::Abort,
} => {
return Err(approval_denial_tool_error(turn_ctx));
}
ApprovalOutcome::Decision(
ReviewDecision::Approved
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
| ReviewDecision::ApprovedForSession,
) => {}
ApprovalOutcome::Decision(ReviewDecision::NetworkPolicyAmendment {
network_policy_amendment,
}) => match network_policy_amendment.action {
ApprovalOutcome::Decision {
decision:
ReviewDecision::Approved
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
| ReviewDecision::ApprovedForSession,
} => {}
ApprovalOutcome::Decision {
decision:
ReviewDecision::NetworkPolicyAmendment {
network_policy_amendment,
},
} => match network_policy_amendment.action {
NetworkPolicyRuleAction::Allow => {}
NetworkPolicyRuleAction::Deny => {
return Err(ToolError::Rejected("rejected by user".to_string()));
codex-rs/core/src/tools/runtimes/apply_patch.rs
+8 -5
View File
@@ -14,7 +14,6 @@ use crate::sandboxing::SandboxPermissions;
use crate::sandboxing::execute_env;
use crate::tools::sandboxing::Approvable;
use crate::tools::sandboxing::ApprovalCtx;
use crate::tools::sandboxing::ApprovalOutcome;
use crate::tools::sandboxing::ExecApprovalRequirement;
use crate::tools::sandboxing::SandboxAttempt;
use crate::tools::sandboxing::Sandboxable;
@@ -25,6 +24,7 @@ use crate::tools::sandboxing::with_cached_approval;
use codex_apply_patch::ApplyPatchAction;
use codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::ApprovalOutcome;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::FileChange;
use codex_protocol::protocol::ReviewDecision;
@@ -144,7 +144,7 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
.into();
}
if req.permissions_preapproved && retry_reason.is_none() {
return ApprovalOutcome::Decision(ReviewDecision::Approved);
return ApprovalOutcome::from(ReviewDecision::Approved);
}
if let Some(reason) = retry_reason {
let rx_approve = session
@@ -156,7 +156,9 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
/*grant_root*/ None,
)
.await;
return ApprovalOutcome::Decision(rx_approve.await.unwrap_or_default());
return rx_approve
.await
.unwrap_or_else(|_| ApprovalOutcome::from(ReviewDecision::Abort));
}
with_cached_approval(
@@ -169,11 +171,12 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
turn, call_id, changes, /*reason*/ None, /*grant_root*/ None,
)
.await;
rx_approve.await.unwrap_or_default()
rx_approve
.await
.unwrap_or_else(|_| ApprovalOutcome::from(ReviewDecision::Abort))
},
)
.await
.into()
})
}
codex-rs/core/src/tools/runtimes/shell.rs
+1 -2
View File
@@ -25,7 +25,6 @@ use crate::tools::runtimes::build_sandbox_command;
use crate::tools::runtimes::maybe_wrap_shell_lc_with_snapshot;
use crate::tools::sandboxing::Approvable;
use crate::tools::sandboxing::ApprovalCtx;
use crate::tools::sandboxing::ApprovalOutcome;
use crate::tools::sandboxing::ExecApprovalRequirement;
use crate::tools::sandboxing::SandboxAttempt;
use crate::tools::sandboxing::SandboxOverride;
@@ -37,6 +36,7 @@ use crate::tools::sandboxing::sandbox_override_for_first_attempt;
use crate::tools::sandboxing::with_cached_approval;
use codex_network_proxy::NetworkProxy;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::ApprovalOutcome;
use codex_sandboxing::SandboxablePreference;
use futures::future::BoxFuture;
use std::collections::HashMap;
@@ -191,7 +191,6 @@ impl Approvable<ShellRequest> for ShellRuntime {
.await
})
.await
.into()
})
}
codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs
+21 -16
View File
@@ -16,7 +16,6 @@ use crate::shell::ShellType;
use crate::skills::SkillMetadata;
use crate::tools::runtimes::ExecveSessionApproval;
use crate::tools::runtimes::build_sandbox_command;
use crate::tools::sandboxing::ApprovalOutcome;
use crate::tools::sandboxing::SandboxAttempt;
use crate::tools::sandboxing::ToolCtx;
use crate::tools::sandboxing::ToolError;
@@ -31,6 +30,7 @@ use codex_protocol::models::MacOsSeatbeltProfileExtensions;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::ApprovalOutcome;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ExecApprovalRequestSkillMetadata;
use codex_protocol::protocol::NetworkPolicyRuleAction;
@@ -488,7 +488,6 @@ impl CoreShellActionProvider {
Some(available_decisions),
)
.await
.into()
})
.await)
}
@@ -555,17 +554,20 @@ impl CoreShellActionProvider {
ApprovalOutcome::TimedOut => {
EscalationDecision::deny(Some(GUARDIAN_TIMEOUT_MESSAGE.to_string()))
}
ApprovalOutcome::Decision(
ReviewDecision::Approved
| ReviewDecision::ApprovedExecpolicyAmendment { .. },
) => {
ApprovalOutcome::Decision {
decision:
ReviewDecision::Approved
| ReviewDecision::ApprovedExecpolicyAmendment { .. },
} => {
if needs_escalation {
EscalationDecision::escalate(escalation_execution.clone())
} else {
EscalationDecision::run()
}
}
ApprovalOutcome::Decision(ReviewDecision::ApprovedForSession) => {
ApprovalOutcome::Decision {
decision: ReviewDecision::ApprovedForSession,
} => {
// Currently, we only add session approvals for
// skill scripts because we are storing only the
// `program` whereas prefix rules may be restricted by a longer prefix.
@@ -592,9 +594,12 @@ impl CoreShellActionProvider {
EscalationDecision::run()
}
}
ApprovalOutcome::Decision(ReviewDecision::NetworkPolicyAmendment {
network_policy_amendment,
}) => match network_policy_amendment.action {
ApprovalOutcome::Decision {
decision:
ReviewDecision::NetworkPolicyAmendment {
network_policy_amendment,
},
} => match network_policy_amendment.action {
NetworkPolicyRuleAction::Allow => {
if needs_escalation {
EscalationDecision::escalate(escalation_execution.clone())
@@ -606,12 +611,12 @@ impl CoreShellActionProvider {
EscalationDecision::deny(Some("User denied execution".to_string()))
}
},
ApprovalOutcome::Decision(ReviewDecision::Denied) => {
EscalationDecision::deny(Some("User denied execution".to_string()))
}
ApprovalOutcome::Decision(ReviewDecision::Abort) => {
EscalationDecision::deny(Some("User cancelled execution".to_string()))
}
ApprovalOutcome::Decision {
decision: ReviewDecision::Denied,
} => EscalationDecision::deny(Some("User denied execution".to_string())),
ApprovalOutcome::Decision {
decision: ReviewDecision::Abort,
} => EscalationDecision::deny(Some("User cancelled execution".to_string())),
}
}
}
codex-rs/core/src/tools/runtimes/unified_exec.rs
+1 -2
View File
@@ -23,7 +23,6 @@ use crate::tools::runtimes::maybe_wrap_shell_lc_with_snapshot;
use crate::tools::runtimes::shell::zsh_fork_backend;
use crate::tools::sandboxing::Approvable;
use crate::tools::sandboxing::ApprovalCtx;
use crate::tools::sandboxing::ApprovalOutcome;
use crate::tools::sandboxing::ExecApprovalRequirement;
use crate::tools::sandboxing::SandboxAttempt;
use crate::tools::sandboxing::SandboxOverride;
@@ -40,6 +39,7 @@ use crate::unified_exec::UnifiedExecProcess;
use crate::unified_exec::UnifiedExecProcessManager;
use codex_network_proxy::NetworkProxy;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::ApprovalOutcome;
use codex_sandboxing::SandboxablePreference;
use futures::future::BoxFuture;
use std::collections::HashMap;
@@ -160,7 +160,6 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
.await
})
.await
.into()
})
}
codex-rs/core/src/tools/sandboxing.rs
+13 -38
View File
@@ -7,7 +7,6 @@
use crate::codex::Session;
use crate::codex::TurnContext;
use crate::error::CodexErr;
use crate::guardian::GuardianApprovalDecision;
#[cfg(test)]
use crate::protocol::SandboxPolicy;
use crate::sandboxing::ExecOptions;
@@ -20,6 +19,7 @@ use codex_protocol::approvals::NetworkApprovalContext;
use codex_protocol::permissions::FileSystemSandboxKind;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::ApprovalOutcome;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ReviewDecision;
use codex_sandboxing::SandboxCommand;
@@ -74,11 +74,11 @@ pub(crate) async fn with_cached_approval<K, F, Fut>(
tool_name: &str,
keys: Vec<K>,
fetch: F,
) -> ReviewDecision
) -> ApprovalOutcome
where
K: Serialize,
F: FnOnce() -> Fut,
Fut: Future<Output = ReviewDecision>,
Fut: Future<Output = ApprovalOutcome>,
{
// To be defensive here, don't bother with checking the cache if keys are empty.
if keys.is_empty() {
@@ -92,58 +92,33 @@ where
};
if already_approved {
return ReviewDecision::ApprovedForSession;
return ApprovalOutcome::from(ReviewDecision::ApprovedForSession);
}
let decision = fetch().await;
let outcome = fetch().await;
services.session_telemetry.counter(
"codex.approval.requested",
/*inc*/ 1,
&[
("tool", tool_name),
("approved", decision.to_opaque_string()),
("approved", outcome.to_opaque_string()),
],
);
if matches!(decision, ReviewDecision::ApprovedForSession) {
if matches!(
outcome,
ApprovalOutcome::Decision {
decision: ReviewDecision::ApprovedForSession,
}
) {
let mut store = services.tool_approvals.lock().await;
for key in keys {
store.put(key, ReviewDecision::ApprovedForSession);
}
}
decision
}
#[derive(Debug)]
pub(crate) enum ApprovalOutcome {
Decision(ReviewDecision),
TimedOut,
}
impl ApprovalOutcome {
pub(crate) fn decision_for_telemetry(&self) -> ReviewDecision {
match self {
Self::Decision(decision) => decision.clone(),
Self::TimedOut => ReviewDecision::Denied,
}
}
}
impl From<ReviewDecision> for ApprovalOutcome {
fn from(value: ReviewDecision) -> Self {
Self::Decision(value)
}
}
impl From<GuardianApprovalDecision> for ApprovalOutcome {
fn from(value: GuardianApprovalDecision) -> Self {
match value {
GuardianApprovalDecision::TimedOut => Self::TimedOut,
decision => Self::Decision(decision.into_review_decision()),
}
}
outcome
}
#[derive(Clone)]
codex-rs/core/tests/suite/approvals.rs
+15 -14
View File
@@ -15,6 +15,7 @@ use codex_protocol::approvals::NetworkApprovalProtocol;
use codex_protocol::approvals::NetworkPolicyAmendment;
use codex_protocol::approvals::NetworkPolicyRuleAction;
use codex_protocol::protocol::ApplyPatchApprovalRequestEvent;
use codex_protocol::protocol::ApprovalOutcome;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::ExecApprovalRequestEvent;
@@ -1715,7 +1716,7 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> {
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: decision.clone(),
outcome: ApprovalOutcome::from(decision.clone()),
})
.await?;
wait_for_completion(&test).await;
@@ -1736,7 +1737,7 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> {
test.codex
.submit(Op::PatchApproval {
id: approval.call_id,
decision: decision.clone(),
outcome: ApprovalOutcome::from(decision.clone()),
})
.await?;
wait_for_completion(&test).await;
@@ -1819,7 +1820,7 @@ async fn approving_apply_patch_for_session_skips_future_prompts_for_same_file()
test.codex
.submit(Op::PatchApproval {
id: approval.call_id,
decision: ReviewDecision::ApprovedForSession,
outcome: ApprovalOutcome::from(ReviewDecision::ApprovedForSession),
})
.await?;
wait_for_completion(&test).await;
@@ -1939,9 +1940,9 @@ async fn approving_execpolicy_amendment_persists_policy_and_skips_future_prompts
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::ApprovedExecpolicyAmendment {
outcome: ApprovalOutcome::from(ReviewDecision::ApprovedExecpolicyAmendment {
proposed_execpolicy_amendment: expected_execpolicy_amendment.clone(),
},
}),
})
.await?;
wait_for_completion(&test).await;
@@ -2178,9 +2179,9 @@ async fn spawned_subagent_execpolicy_amendment_propagates_to_parent_session() ->
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::ApprovedExecpolicyAmendment {
outcome: ApprovalOutcome::from(ReviewDecision::ApprovedExecpolicyAmendment {
proposed_execpolicy_amendment: expected_execpolicy_amendment,
},
}),
})
.await?;
@@ -2403,9 +2404,9 @@ async fn approving_fallback_rule_for_compound_command_works() -> Result<()> {
.submit(Op::ExecApproval {
id: approval_id,
turn_id: None,
decision: ReviewDecision::ApprovedExecpolicyAmendment {
outcome: ApprovalOutcome::from(ReviewDecision::ApprovedExecpolicyAmendment {
proposed_execpolicy_amendment: amendment.clone(),
},
}),
})
.await?;
wait_for_completion(&test).await;
@@ -2599,7 +2600,7 @@ allow_local_binding = true
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
outcome: ApprovalOutcome::from(ReviewDecision::Approved),
})
.await?;
}
@@ -2637,9 +2638,9 @@ allow_local_binding = true
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::NetworkPolicyAmendment {
outcome: ApprovalOutcome::from(ReviewDecision::NetworkPolicyAmendment {
network_policy_amendment: deny_network_amendment.clone(),
},
}),
})
.await?;
wait_for_completion(&test).await;
@@ -2742,7 +2743,7 @@ allow_local_binding = true
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
outcome: ApprovalOutcome::from(ReviewDecision::Approved),
})
.await?;
}
@@ -2826,7 +2827,7 @@ async fn compound_command_with_one_safe_command_still_requires_approval() -> Res
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Denied,
outcome: ApprovalOutcome::from(ReviewDecision::Denied),
})
.await?;
wait_for_completion(&test).await;
codex-rs/core/tests/suite/codex_delegate.rs
+3 -2
View File
@@ -1,5 +1,6 @@
use codex_core::config::Constrained;
use codex_core::sandboxing::SandboxPermissions;
use codex_protocol::protocol::ApprovalOutcome;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::Op;
@@ -102,7 +103,7 @@ async fn codex_delegate_forwards_exec_approval_and_proceeds_on_approval() {
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
outcome: ApprovalOutcome::from(ReviewDecision::Approved),
})
.await
.expect("submit exec approval");
@@ -181,7 +182,7 @@ async fn codex_delegate_forwards_patch_approval_and_proceeds_on_decision() {
test.codex
.submit(Op::PatchApproval {
id: approval.call_id,
decision: ReviewDecision::Denied,
outcome: ApprovalOutcome::from(ReviewDecision::Denied),
})
.await
.expect("submit patch approval");
codex-rs/core/tests/suite/otel.rs
+9 -8
View File
@@ -1,5 +1,6 @@
use codex_core::config::Constrained;
use codex_features::Feature;
use codex_protocol::protocol::ApprovalOutcome;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::Op;
@@ -766,7 +767,7 @@ async fn handle_response_item_records_tool_result_for_custom_tool_call() {
.await
.unwrap();
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TokenCount(_))).await;
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
logs_assert(|lines: &[&str]| {
let line = lines
@@ -839,7 +840,7 @@ async fn handle_response_item_records_tool_result_for_function_call() {
.await
.unwrap();
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TokenCount(_))).await;
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
logs_assert(|lines: &[&str]| {
let line = lines
@@ -1159,7 +1160,7 @@ async fn handle_container_exec_user_approved_records_tool_decision() {
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
outcome: ApprovalOutcome::from(ReviewDecision::Approved),
})
.await
.unwrap();
@@ -1225,7 +1226,7 @@ async fn handle_container_exec_user_approved_for_session_records_tool_decision()
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::ApprovedForSession,
outcome: ApprovalOutcome::from(ReviewDecision::ApprovedForSession),
})
.await
.unwrap();
@@ -1291,7 +1292,7 @@ async fn handle_sandbox_error_user_approves_retry_records_tool_decision() {
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
outcome: ApprovalOutcome::from(ReviewDecision::Approved),
})
.await
.unwrap();
@@ -1357,7 +1358,7 @@ async fn handle_container_exec_user_denies_records_tool_decision() {
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Denied,
outcome: ApprovalOutcome::from(ReviewDecision::Denied),
})
.await
.unwrap();
@@ -1423,7 +1424,7 @@ async fn handle_sandbox_error_user_approves_for_session_records_tool_decision()
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::ApprovedForSession,
outcome: ApprovalOutcome::from(ReviewDecision::ApprovedForSession),
})
.await
.unwrap();
@@ -1490,7 +1491,7 @@ async fn handle_sandbox_error_user_denies_records_tool_decision() {
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Denied,
outcome: ApprovalOutcome::from(ReviewDecision::Denied),
})
.await
.unwrap();
codex-rs/core/tests/suite/request_permissions.rs
+13 -12
View File
@@ -6,6 +6,7 @@ use codex_core::sandboxing::SandboxPermissions;
use codex_features::Feature;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::ApprovalOutcome;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::ExecApprovalRequestEvent;
@@ -378,7 +379,7 @@ async fn with_additional_permissions_requires_approval_under_on_request() -> Res
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
outcome: ApprovalOutcome::from(ReviewDecision::Approved),
})
.await?;
wait_for_completion(&test).await;
@@ -566,7 +567,7 @@ async fn relative_additional_permissions_resolve_against_tool_workdir() -> Resul
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
outcome: ApprovalOutcome::from(ReviewDecision::Approved),
})
.await?;
wait_for_completion(&test).await;
@@ -660,7 +661,7 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_cwd
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
outcome: ApprovalOutcome::from(ReviewDecision::Approved),
})
.await?;
wait_for_completion(&test).await;
@@ -761,7 +762,7 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_tmp
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
outcome: ApprovalOutcome::from(ReviewDecision::Approved),
})
.await?;
wait_for_completion(&test).await;
@@ -869,7 +870,7 @@ async fn workspace_write_with_additional_permissions_can_write_outside_cwd() ->
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
outcome: ApprovalOutcome::from(ReviewDecision::Approved),
})
.await?;
wait_for_completion(&test).await;
@@ -971,7 +972,7 @@ async fn with_additional_permissions_denied_approval_blocks_execution() -> Resul
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Denied,
outcome: ApprovalOutcome::from(ReviewDecision::Denied),
})
.await?;
wait_for_completion(&test).await;
@@ -1100,7 +1101,7 @@ async fn request_permissions_grants_apply_to_later_exec_command_calls() -> Resul
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
outcome: ApprovalOutcome::from(ReviewDecision::Approved),
})
.await?;
wait_for_completion(&test).await;
@@ -1210,7 +1211,7 @@ async fn request_permissions_preapprove_explicit_exec_permissions_outside_on_req
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
outcome: ApprovalOutcome::from(ReviewDecision::Approved),
})
.await?;
wait_for_completion(&test).await;
@@ -1323,7 +1324,7 @@ async fn request_permissions_grants_apply_to_later_shell_command_calls() -> Resu
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
outcome: ApprovalOutcome::from(ReviewDecision::Approved),
})
.await?;
wait_for_completion(&test).await;
@@ -1432,7 +1433,7 @@ async fn request_permissions_grants_apply_to_later_shell_command_calls_without_i
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
outcome: ApprovalOutcome::from(ReviewDecision::Approved),
})
.await?;
wait_for_completion(&test).await;
@@ -1601,7 +1602,7 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions()
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
outcome: ApprovalOutcome::from(ReviewDecision::Approved),
})
.await?;
wait_for_completion(&test).await;
@@ -1846,7 +1847,7 @@ async fn request_permissions_session_grants_carry_across_turns() -> Result<()> {
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
outcome: ApprovalOutcome::from(ReviewDecision::Approved),
})
.await?;
wait_for_completion(&test).await;
codex-rs/core/tests/suite/request_permissions_tool.rs
+2 -1
View File
@@ -5,6 +5,7 @@ use anyhow::Result;
use codex_core::config::Constrained;
use codex_features::Feature;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::protocol::ApprovalOutcome;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::Op;
@@ -277,7 +278,7 @@ async fn approved_folder_write_request_permissions_unblocks_later_exec_without_s
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Approved,
outcome: ApprovalOutcome::from(ReviewDecision::Approved),
})
.await?;
wait_for_event(&test.codex, |event| {
codex-rs/core/tests/suite/skill_approval.rs
+5 -4
View File
@@ -4,6 +4,7 @@
use anyhow::Result;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::ApprovalOutcome;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::ExecApprovalRequestEvent;
@@ -257,7 +258,7 @@ permissions:
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Denied,
outcome: ApprovalOutcome::from(ReviewDecision::Denied),
})
.await?;
@@ -351,7 +352,7 @@ permissions:
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Denied,
outcome: ApprovalOutcome::from(ReviewDecision::Denied),
})
.await?;
@@ -446,7 +447,7 @@ permissions:
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Denied,
outcome: ApprovalOutcome::from(ReviewDecision::Denied),
})
.await?;
@@ -895,7 +896,7 @@ async fn shell_zsh_fork_skill_session_approval_enforces_skill_permissions() -> R
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::ApprovedForSession,
outcome: ApprovalOutcome::from(ReviewDecision::ApprovedForSession),
})
.await?;
codex-rs/mcp-server/src/exec_approval.rs
+1 -1
View File
@@ -138,7 +138,7 @@ async fn on_exec_approval_response(
.submit(Op::ExecApproval {
id: approval_id,
turn_id: Some(event_id),
decision: response.decision,
outcome: codex_protocol::protocol::ApprovalOutcome::from(response.decision),
})
.await
{
codex-rs/mcp-server/src/patch_approval.rs
+4 -2
View File
@@ -113,7 +113,9 @@ pub(crate) async fn on_patch_approval_response(
if let Err(submit_err) = codex
.submit(Op::PatchApproval {
id: approval_id.clone(),
decision: ReviewDecision::Denied,
outcome: codex_protocol::protocol::ApprovalOutcome::from(
ReviewDecision::Denied,
),
})
.await
{
@@ -133,7 +135,7 @@ pub(crate) async fn on_patch_approval_response(
if let Err(err) = codex
.submit(Op::PatchApproval {
id: approval_id,
decision: response.decision,
outcome: codex_protocol::protocol::ApprovalOutcome::from(response.decision),
})
.await
{
codex-rs/otel/src/events/session_telemetry.rs
+3 -3
View File
@@ -35,8 +35,8 @@ use codex_protocol::ThreadId;
use codex_protocol::config_types::ReasoningSummary;
use codex_protocol::models::ResponseItem;
use codex_protocol::openai_models::ReasoningEffort;
use codex_protocol::protocol::ApprovalOutcome;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::protocol::SessionSource;
use codex_protocol::user_input::UserInput;
@@ -865,7 +865,7 @@ impl SessionTelemetry {
&self,
tool_name: &str,
call_id: &str,
decision: &ReviewDecision,
outcome: &ApprovalOutcome,
source: ToolDecisionSource,
) {
log_event!(
@@ -873,7 +873,7 @@ impl SessionTelemetry {
event.name = "codex.tool_decision",
tool_name = %tool_name,
call_id = %call_id,
decision = %decision.clone().to_string().to_lowercase(),
decision = %outcome.to_opaque_string(),
source = %source.to_string(),
);
}
codex-rs/otel/tests/suite/otel_export_routing_policy.rs
+48
View File
@@ -2,6 +2,7 @@ use codex_otel::AuthEnvTelemetryMetadata;
use codex_otel::OtelProvider;
use codex_otel::SessionTelemetry;
use codex_otel::TelemetryAuthMode;
use codex_otel::ToolDecisionSource;
use opentelemetry::KeyValue;
use opentelemetry::logs::AnyValue;
use opentelemetry::trace::TracerProvider as _;
@@ -20,6 +21,7 @@ use tracing_subscriber::layer::SubscriberExt;
use codex_protocol::ThreadId;
use codex_protocol::config_types::ReasoningSummary;
use codex_protocol::protocol::ApprovalOutcome;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::protocol::SessionSource;
@@ -200,6 +202,52 @@ fn otel_export_routing_policy_routes_user_prompt_log_and_trace_events() {
assert!(!prompt_trace_attrs.contains_key("user.account_id"));
}
#[test]
fn otel_export_routing_policy_logs_tool_timeout_decision_as_timed_out() {
let log_exporter = InMemoryLogExporter::default();
let logger_provider = SdkLoggerProvider::builder()
.with_simple_exporter(log_exporter.clone())
.build();
let subscriber = tracing_subscriber::registry().with(
opentelemetry_appender_tracing::layer::OpenTelemetryTracingBridge::new(&logger_provider)
.with_filter(filter_fn(OtelProvider::log_export_filter)),
);
tracing::subscriber::with_default(subscriber, || {
tracing::callsite::rebuild_interest_cache();
let manager = SessionTelemetry::new(
ThreadId::new(),
"gpt-5.1",
"gpt-5.1",
Some("account-id".to_string()),
None,
Some(TelemetryAuthMode::ApiKey),
"codex_exec".to_string(),
true,
"tty".to_string(),
SessionSource::Cli,
);
manager.tool_decision(
"local_shell",
"call-timeout",
&ApprovalOutcome::TimedOut,
ToolDecisionSource::AutomatedReviewer,
);
});
logger_provider.force_flush().expect("flush logs");
let logs = log_exporter.get_emitted_logs().expect("log export");
let tool_log = find_log_by_event_name(&logs, "codex.tool_decision");
let tool_log_attrs = log_attributes(&tool_log.record);
assert_eq!(
tool_log_attrs.get("decision").map(String::as_str),
Some("timed_out")
);
}
#[test]
fn otel_export_routing_policy_routes_tool_result_log_and_trace_events() {
let log_exporter = InMemoryLogExporter::default();
codex-rs/protocol/src/protocol.rs
+43 -4
View File
@@ -373,16 +373,16 @@ pub enum Op {
/// Turn id associated with the approval event, when available.
#[serde(default, skip_serializing_if = "Option::is_none")]
turn_id: Option<String>,
/// The user's decision in response to the request.
decision: ReviewDecision,
/// The outcome of resolving the request.
outcome: ApprovalOutcome,
},
/// Approve a code patch
PatchApproval {
/// The id of the submission we are approving
id: String,
/// The user's decision in response to the request.
decision: ReviewDecision,
/// The outcome of resolving the request.
outcome: ApprovalOutcome,
},
/// Resolve an MCP elicitation request.
@@ -3290,6 +3290,45 @@ impl ReviewDecision {
}
}
/// Result of resolving an approval request.
///
/// `ReviewDecision` represents the generic approval choices available to a
/// user or policy surface. Some approval flows, such as guardian review, can
/// also end without an intentional rejection. Those cases are represented as a
/// richer outcome here so timeout information can survive hops through session
/// state and delegated subagents.
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema)]
#[serde(tag = "type", rename_all = "snake_case")]
pub enum ApprovalOutcome {
Decision { decision: ReviewDecision },
TimedOut,
}
impl ApprovalOutcome {
/// Returns an opaque version of the outcome without PII. We can't use an
/// ignored flag on `serde` because the serialization is required by some
/// surfaces.
pub fn to_opaque_string(&self) -> &'static str {
match self {
Self::Decision { decision } => decision.to_opaque_string(),
Self::TimedOut => "timed_out",
}
}
pub fn into_decision(self) -> Option<ReviewDecision> {
match self {
Self::Decision { decision } => Some(decision),
Self::TimedOut => None,
}
}
}
impl From<ReviewDecision> for ApprovalOutcome {
fn from(decision: ReviewDecision) -> Self {
Self::Decision { decision }
}
}
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, JsonSchema, TS)]
#[serde(tag = "type", rename_all = "snake_case")]
#[ts(tag = "type")]
codex-rs/tui/src/app/pending_interactive_replay.rs
+9 -3
View File
@@ -463,7 +463,9 @@ mod tests {
store.note_outbound_op(&Op::ExecApproval {
id: "approval-1".to_string(),
turn_id: Some("turn-1".to_string()),
decision: codex_protocol::protocol::ReviewDecision::Approved,
outcome: codex_protocol::protocol::ApprovalOutcome::from(
codex_protocol::protocol::ReviewDecision::Approved,
),
});
let snapshot = store.snapshot();
@@ -570,7 +572,9 @@ mod tests {
store.note_outbound_op(&Op::PatchApproval {
id: "call-1".to_string(),
decision: codex_protocol::protocol::ReviewDecision::Approved,
outcome: codex_protocol::protocol::ApprovalOutcome::from(
codex_protocol::protocol::ReviewDecision::Approved,
),
});
let snapshot = store.snapshot();
@@ -699,7 +703,9 @@ mod tests {
store.note_outbound_op(&Op::ExecApproval {
id: "call-1".to_string(),
turn_id: Some("turn-1".to_string()),
decision: codex_protocol::protocol::ReviewDecision::Approved,
outcome: codex_protocol::protocol::ApprovalOutcome::from(
codex_protocol::protocol::ReviewDecision::Approved,
),
});
assert_eq!(store.has_pending_thread_approvals(), false);
codex-rs/tui/src/bottom_pane/approval_overlay.rs
+12 -4
View File
@@ -269,7 +269,7 @@ impl ApprovalOverlay {
op: Op::ExecApproval {
id: id.to_string(),
turn_id: None,
decision,
outcome: codex_protocol::protocol::ApprovalOutcome::from(decision),
},
});
}
@@ -331,7 +331,7 @@ impl ApprovalOverlay {
thread_id,
op: Op::PatchApproval {
id: id.to_string(),
decision,
outcome: codex_protocol::protocol::ApprovalOutcome::from(decision),
},
});
}
@@ -1093,7 +1093,11 @@ mod tests {
let mut saw_op = false;
while let Ok(ev) = rx.try_recv() {
if let AppEvent::SubmitThreadOp {
op: Op::ExecApproval { decision, .. },
op:
Op::ExecApproval {
outcome: codex_protocol::protocol::ApprovalOutcome::Decision { decision },
..
},
..
} = ev
{
@@ -1543,7 +1547,11 @@ mod tests {
let mut decision = None;
while let Ok(ev) = rx.try_recv() {
if let AppEvent::SubmitThreadOp {
op: Op::ExecApproval { decision: d, .. },
op:
Op::ExecApproval {
outcome: codex_protocol::protocol::ApprovalOutcome::Decision { decision: d },
..
},
..
} = ev
{
codex-rs/tui/src/chatwidget/tests.rs
+15 -3
View File
@@ -3520,7 +3520,12 @@ async fn exec_approval_uses_approval_id_when_present() {
let mut found = false;
while let Ok(app_ev) = rx.try_recv() {
if let AppEvent::SubmitThreadOp {
op: Op::ExecApproval { id, decision, .. },
op:
Op::ExecApproval {
id,
outcome: codex_protocol::protocol::ApprovalOutcome::Decision { decision },
..
},
..
} = app_ev
{
@@ -10983,7 +10988,11 @@ async fn apply_patch_approval_sends_op_with_call_id() {
let mut found = false;
while let Ok(app_ev) = rx.try_recv() {
if let AppEvent::SubmitThreadOp {
op: Op::PatchApproval { id, decision },
op:
Op::PatchApproval {
id,
outcome: codex_protocol::protocol::ApprovalOutcome::Decision { decision },
},
..
} = app_ev
{
@@ -11034,7 +11043,10 @@ async fn apply_patch_full_flow_integration_like() {
.try_recv()
.expect("expected op forwarded to codex channel");
match forwarded {
Op::PatchApproval { id, decision } => {
Op::PatchApproval {
id,
outcome: codex_protocol::protocol::ApprovalOutcome::Decision { decision },
} => {
assert_eq!(id, "call-1");
assert_matches!(decision, codex_protocol::protocol::ReviewDecision::Approved);
}
codex-rs/tui_app_server/src/app/app_server_requests.rs
+21 -11
View File
@@ -119,14 +119,18 @@ impl PendingAppServerRequests {
{
let op: AppCommand = op.into();
let resolution = match op.view() {
AppCommandView::ExecApproval { id, decision, .. } => self
AppCommandView::ExecApproval { id, outcome, .. } => self
.exec_approvals
.remove(id)
.map(|request_id| {
let decision = outcome.clone().into_decision().ok_or_else(|| {
"cannot serialize timed-out exec approval as an app-server decision"
.to_string()
})?;
Ok::<AppServerRequestResolution, String>(AppServerRequestResolution {
request_id,
result: serde_json::to_value(CommandExecutionRequestApprovalResponse {
decision: decision.clone().into(),
decision: decision.into(),
})
.map_err(|err| {
format!("failed to serialize command execution approval response: {err}")
@@ -134,14 +138,18 @@ impl PendingAppServerRequests {
})
})
.transpose()?,
AppCommandView::PatchApproval { id, decision } => self
AppCommandView::PatchApproval { id, outcome } => self
.file_change_approvals
.remove(id)
.map(|request_id| {
let decision = outcome.clone().into_decision().ok_or_else(|| {
"cannot serialize timed-out patch approval as an app-server decision"
.to_string()
})?;
Ok::<AppServerRequestResolution, String>(AppServerRequestResolution {
request_id,
result: serde_json::to_value(FileChangeRequestApprovalResponse {
decision: file_change_decision(decision)?,
decision: file_change_decision(&decision)?,
})
.map_err(|err| {
format!("failed to serialize file change approval response: {err}")
@@ -331,7 +339,7 @@ mod tests {
.take_resolution(&Op::ExecApproval {
id: "approval-1".to_string(),
turn_id: None,
decision: ReviewDecision::Approved,
outcome: codex_protocol::protocol::ApprovalOutcome::from(ReviewDecision::Approved),
})
.expect("resolution should serialize")
.expect("request should be pending");
@@ -537,12 +545,14 @@ mod tests {
let error = pending
.take_resolution(&Op::PatchApproval {
id: "patch-1".to_string(),
decision: ReviewDecision::ApprovedExecpolicyAmendment {
proposed_execpolicy_amendment: ExecPolicyAmendment::new(vec![
"echo".to_string(),
"hi".to_string(),
]),
},
outcome: codex_protocol::protocol::ApprovalOutcome::from(
ReviewDecision::ApprovedExecpolicyAmendment {
proposed_execpolicy_amendment: ExecPolicyAmendment::new(vec![
"echo".to_string(),
"hi".to_string(),
]),
},
),
})
.expect_err("invalid patch decision should fail");
codex-rs/tui_app_server/src/app/pending_interactive_replay.rs
+3 -3
View File
@@ -760,7 +760,7 @@ mod tests {
store.note_outbound_op(&Op::ExecApproval {
id: "approval-1".to_string(),
turn_id: Some("turn-1".to_string()),
decision: ReviewDecision::Approved,
outcome: codex_protocol::protocol::ApprovalOutcome::from(ReviewDecision::Approved),
});
let snapshot = store.snapshot();
@@ -847,7 +847,7 @@ mod tests {
store.note_outbound_op(&Op::PatchApproval {
id: "call-1".to_string(),
decision: ReviewDecision::Approved,
outcome: codex_protocol::protocol::ApprovalOutcome::from(ReviewDecision::Approved),
});
let snapshot = store.snapshot();
@@ -911,7 +911,7 @@ mod tests {
store.note_outbound_op(&Op::ExecApproval {
id: "call-1".to_string(),
turn_id: Some("turn-1".to_string()),
decision: ReviewDecision::Approved,
outcome: codex_protocol::protocol::ApprovalOutcome::from(ReviewDecision::Approved),
});
assert_eq!(store.has_pending_thread_approvals(), false);
codex-rs/tui_app_server/src/app_command.rs
+11 -7
View File
@@ -9,6 +9,7 @@ use codex_protocol::config_types::ServiceTier;
use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::mcp::RequestId as McpRequestId;
use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig;
use codex_protocol::protocol::ApprovalOutcome;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ConversationAudioParams;
use codex_protocol::protocol::ConversationStartParams;
@@ -68,11 +69,11 @@ pub(crate) enum AppCommandView<'a> {
ExecApproval {
id: &'a str,
turn_id: &'a Option<String>,
decision: &'a ReviewDecision,
outcome: &'a ApprovalOutcome,
},
PatchApproval {
id: &'a str,
decision: &'a ReviewDecision,
outcome: &'a ApprovalOutcome,
},
ResolveElicitation {
server_name: &'a str,
@@ -209,12 +210,15 @@ impl AppCommand {
Self(Op::ExecApproval {
id,
turn_id,
decision,
outcome: ApprovalOutcome::from(decision),
})
}
pub(crate) fn patch_approval(id: String, decision: ReviewDecision) -> Self {
Self(Op::PatchApproval { id, decision })
Self(Op::PatchApproval {
id,
outcome: ApprovalOutcome::from(decision),
})
}
pub(crate) fn resolve_elicitation(
@@ -356,13 +360,13 @@ impl AppCommand {
Op::ExecApproval {
id,
turn_id,
decision,
outcome,
} => AppCommandView::ExecApproval {
id,
turn_id,
decision,
outcome,
},
Op::PatchApproval { id, decision } => AppCommandView::PatchApproval { id, decision },
Op::PatchApproval { id, outcome } => AppCommandView::PatchApproval { id, outcome },
Op::ResolveElicitation {
server_name,
request_id,
codex-rs/tui_app_server/src/bottom_pane/approval_overlay.rs
+10 -2
View File
@@ -1079,7 +1079,11 @@ mod tests {
let mut saw_op = false;
while let Ok(ev) = rx.try_recv() {
if let AppEvent::SubmitThreadOp {
op: Op::ExecApproval { decision, .. },
op:
Op::ExecApproval {
outcome: codex_protocol::protocol::ApprovalOutcome::Decision { decision },
..
},
..
} = ev
{
@@ -1529,7 +1533,11 @@ mod tests {
let mut decision = None;
while let Ok(ev) = rx.try_recv() {
if let AppEvent::SubmitThreadOp {
op: Op::ExecApproval { decision: d, .. },
op:
Op::ExecApproval {
outcome: codex_protocol::protocol::ApprovalOutcome::Decision { decision: d },
..
},
..
} = ev
{
codex-rs/tui_app_server/src/chatwidget/tests.rs
+15 -3
View File
@@ -3529,7 +3529,12 @@ async fn exec_approval_uses_approval_id_when_present() {
let mut found = false;
while let Ok(app_ev) = rx.try_recv() {
if let AppEvent::SubmitThreadOp {
op: Op::ExecApproval { id, decision, .. },
op:
Op::ExecApproval {
id,
outcome: codex_protocol::protocol::ApprovalOutcome::Decision { decision },
..
},
..
} = app_ev
{
@@ -11647,7 +11652,11 @@ async fn apply_patch_approval_sends_op_with_call_id() {
let mut found = false;
while let Ok(app_ev) = rx.try_recv() {
if let AppEvent::SubmitThreadOp {
op: Op::PatchApproval { id, decision },
op:
Op::PatchApproval {
id,
outcome: codex_protocol::protocol::ApprovalOutcome::Decision { decision },
},
..
} = app_ev
{
@@ -11698,7 +11707,10 @@ async fn apply_patch_full_flow_integration_like() {
.try_recv()
.expect("expected op forwarded to codex channel");
match forwarded {
Op::PatchApproval { id, decision } => {
Op::PatchApproval {
id,
outcome: codex_protocol::protocol::ApprovalOutcome::Decision { decision },
} => {
assert_eq!(id, "call-1");
assert_matches!(decision, codex_protocol::protocol::ReviewDecision::Approved);
}