mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
Generalize approval timeout messaging
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
4
codex-rs/core/src/approval_review.rs
Normal file
4
codex-rs/core/src/approval_review.rs
Normal file
@@ -0,0 +1,4 @@
|
||||
pub(crate) const APPROVAL_REVIEW_TIMEOUT_MESSAGE: &str = concat!(
|
||||
"Automatic approval review timed out before it could finish evaluating this action. ",
|
||||
"The action was not intentionally rejected, and a retry may succeed.",
|
||||
);
|
||||
@@ -25,7 +25,6 @@ pub(crate) use approval_request::GuardianApprovalRequest;
|
||||
pub(crate) use approval_request::GuardianMcpAnnotations;
|
||||
pub(crate) use approval_request::guardian_approval_request_to_json;
|
||||
pub(crate) use review::GUARDIAN_REJECTION_MESSAGE;
|
||||
pub(crate) use review::GUARDIAN_TIMEOUT_MESSAGE;
|
||||
pub(crate) use review::GuardianApprovalDecision;
|
||||
pub(crate) use review::is_guardian_reviewer_source;
|
||||
pub(crate) use review::review_approval_request;
|
||||
|
||||
@@ -11,6 +11,7 @@ use codex_protocol::protocol::SubAgentSource;
|
||||
use codex_protocol::protocol::WarningEvent;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
|
||||
use crate::approval_review::APPROVAL_REVIEW_TIMEOUT_MESSAGE;
|
||||
use crate::codex::Session;
|
||||
use crate::codex::TurnContext;
|
||||
|
||||
@@ -37,11 +38,6 @@ pub(crate) const GUARDIAN_REJECTION_MESSAGE: &str = concat!(
|
||||
"Otherwise, stop and request user input.",
|
||||
);
|
||||
|
||||
pub(crate) const GUARDIAN_TIMEOUT_MESSAGE: &str = concat!(
|
||||
"Automatic approval review timed out before it could finish evaluating this action. ",
|
||||
"The action was not intentionally rejected, and a retry may succeed.",
|
||||
);
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub(crate) enum GuardianApprovalDecision {
|
||||
Approved,
|
||||
@@ -181,7 +177,7 @@ async fn run_guardian_review(
|
||||
format!("Automatic approval review failed: {err}"),
|
||||
),
|
||||
GuardianReviewOutcome::TimedOut => {
|
||||
let warning_event_message = GUARDIAN_TIMEOUT_MESSAGE.to_string();
|
||||
let warning_event_message = APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string();
|
||||
session
|
||||
.send_event(
|
||||
turn.as_ref(),
|
||||
|
||||
@@ -8,6 +8,7 @@
|
||||
mod analytics_client;
|
||||
pub mod api_bridge;
|
||||
mod apply_patch;
|
||||
mod approval_review;
|
||||
mod apps;
|
||||
mod arc_monitor;
|
||||
pub use codex_login as auth;
|
||||
|
||||
@@ -11,6 +11,7 @@ use tracing::error;
|
||||
use crate::analytics_client::AppInvocation;
|
||||
use crate::analytics_client::InvocationType;
|
||||
use crate::analytics_client::build_track_events_context;
|
||||
use crate::approval_review::APPROVAL_REVIEW_TIMEOUT_MESSAGE;
|
||||
use crate::arc_monitor::ArcMonitorOutcome;
|
||||
use crate::arc_monitor::monitor_action;
|
||||
use crate::codex::Session;
|
||||
@@ -19,7 +20,6 @@ use crate::config::edit::ConfigEdit;
|
||||
use crate::config::edit::ConfigEditsBuilder;
|
||||
use crate::config::types::AppToolApproval;
|
||||
use crate::connectors;
|
||||
use crate::guardian::GUARDIAN_TIMEOUT_MESSAGE;
|
||||
use crate::guardian::GuardianApprovalDecision;
|
||||
use crate::guardian::GuardianApprovalRequest;
|
||||
use crate::guardian::GuardianMcpAnnotations;
|
||||
@@ -768,7 +768,7 @@ fn mcp_tool_approval_decision_from_guardian(
|
||||
McpToolApprovalDecision::Decline
|
||||
}
|
||||
GuardianApprovalDecision::TimedOut => {
|
||||
McpToolApprovalDecision::TimedOut(GUARDIAN_TIMEOUT_MESSAGE.to_string())
|
||||
McpToolApprovalDecision::TimedOut(APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string())
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1200,7 +1200,7 @@ fn parse_mcp_tool_approval_response(
|
||||
.iter()
|
||||
.any(|answer| answer == MCP_TOOL_APPROVAL_TIMED_OUT_SYNTHETIC)
|
||||
{
|
||||
McpToolApprovalDecision::TimedOut(GUARDIAN_TIMEOUT_MESSAGE.to_string())
|
||||
McpToolApprovalDecision::TimedOut(APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string())
|
||||
} else if answers
|
||||
.iter()
|
||||
.any(|answer| answer == MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION)
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use super::*;
|
||||
use crate::approval_review::APPROVAL_REVIEW_TIMEOUT_MESSAGE;
|
||||
use crate::codex::make_session_and_context;
|
||||
use crate::config::ApprovalsReviewer;
|
||||
use crate::config::ConfigToml;
|
||||
@@ -6,7 +7,6 @@ use crate::config::types::AppConfig;
|
||||
use crate::config::types::AppToolConfig;
|
||||
use crate::config::types::AppToolsConfig;
|
||||
use crate::config::types::AppsConfigToml;
|
||||
use crate::guardian::GUARDIAN_TIMEOUT_MESSAGE;
|
||||
use crate::guardian::GuardianApprovalDecision;
|
||||
use codex_config::CONFIG_TOML_FILE;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
@@ -708,7 +708,7 @@ fn guardian_review_decision_maps_to_mcp_tool_decision() {
|
||||
);
|
||||
assert_eq!(
|
||||
mcp_tool_approval_decision_from_guardian(GuardianApprovalDecision::TimedOut),
|
||||
McpToolApprovalDecision::TimedOut(GUARDIAN_TIMEOUT_MESSAGE.to_string())
|
||||
McpToolApprovalDecision::TimedOut(APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string())
|
||||
);
|
||||
}
|
||||
|
||||
@@ -863,7 +863,7 @@ fn synthetic_timeout_request_user_input_response_stays_timed_out() {
|
||||
|
||||
assert_eq!(
|
||||
response,
|
||||
McpToolApprovalDecision::TimedOut(GUARDIAN_TIMEOUT_MESSAGE.to_string())
|
||||
McpToolApprovalDecision::TimedOut(APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string())
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
use crate::approval_review::APPROVAL_REVIEW_TIMEOUT_MESSAGE;
|
||||
use crate::codex::Session;
|
||||
use crate::guardian::GUARDIAN_REJECTION_MESSAGE;
|
||||
use crate::guardian::GUARDIAN_TIMEOUT_MESSAGE;
|
||||
use crate::guardian::GuardianApprovalDecision;
|
||||
use crate::guardian::GuardianApprovalRequest;
|
||||
use crate::guardian::review_approval_request;
|
||||
@@ -393,7 +393,7 @@ impl NetworkApprovalService {
|
||||
self.record_call_outcome(
|
||||
&owner_call.registration_id,
|
||||
NetworkApprovalOutcome::TimedOutByReviewer(
|
||||
GUARDIAN_TIMEOUT_MESSAGE.to_string(),
|
||||
APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string(),
|
||||
),
|
||||
)
|
||||
.await;
|
||||
@@ -520,7 +520,7 @@ impl NetworkApprovalService {
|
||||
self.record_call_outcome(
|
||||
&owner_call.registration_id,
|
||||
NetworkApprovalOutcome::TimedOutByReviewer(
|
||||
GUARDIAN_TIMEOUT_MESSAGE.to_string(),
|
||||
APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string(),
|
||||
),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -6,11 +6,11 @@ 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::approval_review::APPROVAL_REVIEW_TIMEOUT_MESSAGE;
|
||||
use crate::error::CodexErr;
|
||||
use crate::error::SandboxErr;
|
||||
use crate::exec::ExecToolCallOutput;
|
||||
use crate::guardian::GUARDIAN_REJECTION_MESSAGE;
|
||||
use crate::guardian::GUARDIAN_TIMEOUT_MESSAGE;
|
||||
use crate::guardian::routes_approval_to_guardian;
|
||||
use crate::network_policy_decision::network_approval_context_from_payload;
|
||||
use crate::tools::network_approval::DeferredNetworkApproval;
|
||||
@@ -163,7 +163,9 @@ impl ToolOrchestrator {
|
||||
|
||||
match approval_outcome {
|
||||
ApprovalOutcome::TimedOut => {
|
||||
return Err(ToolError::Message(GUARDIAN_TIMEOUT_MESSAGE.to_string()));
|
||||
return Err(ToolError::Message(
|
||||
APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string(),
|
||||
));
|
||||
}
|
||||
ApprovalOutcome::Decision {
|
||||
decision: ReviewDecision::Denied | ReviewDecision::Abort,
|
||||
@@ -322,7 +324,9 @@ impl ToolOrchestrator {
|
||||
|
||||
match approval_outcome {
|
||||
ApprovalOutcome::TimedOut => {
|
||||
return Err(ToolError::Message(GUARDIAN_TIMEOUT_MESSAGE.to_string()));
|
||||
return Err(ToolError::Message(
|
||||
APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string(),
|
||||
));
|
||||
}
|
||||
ApprovalOutcome::Decision {
|
||||
decision: ReviewDecision::Denied | ReviewDecision::Abort,
|
||||
|
||||
@@ -1,11 +1,11 @@
|
||||
use super::ShellRequest;
|
||||
use crate::approval_review::APPROVAL_REVIEW_TIMEOUT_MESSAGE;
|
||||
use crate::error::CodexErr;
|
||||
use crate::error::SandboxErr;
|
||||
use crate::exec::ExecCapturePolicy;
|
||||
use crate::exec::ExecExpiration;
|
||||
use crate::exec::ExecToolCallOutput;
|
||||
use crate::exec::is_likely_sandbox_denied;
|
||||
use crate::guardian::GUARDIAN_TIMEOUT_MESSAGE;
|
||||
use crate::guardian::GuardianApprovalRequest;
|
||||
use crate::guardian::review_approval_request;
|
||||
use crate::guardian::routes_approval_to_guardian;
|
||||
@@ -551,9 +551,9 @@ impl CoreShellActionProvider {
|
||||
)
|
||||
.await?
|
||||
{
|
||||
ApprovalOutcome::TimedOut => {
|
||||
EscalationDecision::deny(Some(GUARDIAN_TIMEOUT_MESSAGE.to_string()))
|
||||
}
|
||||
ApprovalOutcome::TimedOut => EscalationDecision::deny(Some(
|
||||
APPROVAL_REVIEW_TIMEOUT_MESSAGE.to_string(),
|
||||
)),
|
||||
ApprovalOutcome::Decision {
|
||||
decision:
|
||||
ReviewDecision::Approved
|
||||
|
||||
Reference in New Issue
Block a user