From 7bafdada8beaad9325ed69218f743f058e3598ab Mon Sep 17 00:00:00 2001 From: "Adam Perry @ OpenAI" Date: Thu, 23 Jul 2026 19:24:15 +0000 Subject: [PATCH] Separate Codex error details from retry metadata (#34996) ## What changed - Wrap `CodexErrorDetails` and an optional retry delay in `CodexErr`, allowing any mapped error to preserve server-provided retry timing. - Generate the payload-free `CodexErrKind` classification alongside the error details and reuse it for analytics. - Update error handling sites to inspect `CodexErr::details()` while preserving existing display, debug, protocol mapping, and retryability behavior. ## Testing - Add coverage for legacy debug formatting, error-specific retryability, and retry-delay propagation through API error mapping. GitOrigin-RevId: d3ab8a305f2a2ee21d0c0a8c8c388b06dda9c59a --- codex-rs/analytics/src/facts.rs | 91 +---- .../src/request_processors/request_errors.rs | 7 +- .../request_processors/thread_processor.rs | 41 ++- .../src/request_processors/turn_processor.rs | 7 +- codex-rs/codex-api/src/api_bridge.rs | 23 +- codex-rs/codex-api/src/api_bridge_tests.rs | 61 ++-- codex-rs/core/src/agent/agent_resolver.rs | 9 +- codex-rs/core/src/agent/control.rs | 8 +- codex-rs/core/src/agent/control/execution.rs | 5 +- .../core/src/agent/control/execution_tests.rs | 6 +- codex-rs/core/src/agent/control/legacy.rs | 22 +- codex-rs/core/src/agent/control/residency.rs | 5 +- .../core/src/agent/control/residency_tests.rs | 26 +- codex-rs/core/src/agent/control_tests.rs | 45 ++- codex-rs/core/src/agent/registry.rs | 5 +- codex-rs/core/src/agent/registry_tests.rs | 19 +- codex-rs/core/src/codex_delegate_tests.rs | 6 +- codex-rs/core/src/compact.rs | 22 +- codex-rs/core/src/compact_model_fallback.rs | 17 +- codex-rs/core/src/compact_remote_v2.rs | 4 +- codex-rs/core/src/responses_retry.rs | 7 +- codex-rs/core/src/responses_retry_tests.rs | 5 +- codex-rs/core/src/session/mod.rs | 3 +- codex-rs/core/src/session/turn.rs | 45 ++- codex-rs/core/src/tasks/compact.rs | 6 +- codex-rs/core/src/tasks/mod.rs | 6 +- .../src/thread_rollout_truncation_tests.rs | 13 +- codex-rs/core/src/tools/events.rs | 60 ++-- .../handlers/multi_agents/close_agent.rs | 6 +- .../src/tools/handlers/multi_agents/wait.rs | 4 +- .../src/tools/handlers/multi_agents_common.rs | 21 +- .../multi_agents_v2/interrupt_agent.rs | 12 +- codex-rs/core/src/tools/orchestrator.rs | 55 +-- .../core/src/unified_exec/process_manager.rs | 24 +- .../linux-sandbox/tests/suite/landlock.rs | 45 ++- .../src/amazon_bedrock/error.rs | 11 +- .../src/amazon_bedrock/error_tests.rs | 8 +- codex-rs/model-provider/src/auth.rs | 11 +- codex-rs/protocol/src/error.rs | 337 ++++++++++++++---- codex-rs/protocol/src/error_tests.rs | 58 +++ 40 files changed, 736 insertions(+), 430 deletions(-) diff --git a/codex-rs/analytics/src/facts.rs b/codex-rs/analytics/src/facts.rs index 9f6a4a9fcc0a..41df723a372a 100644 --- a/codex-rs/analytics/src/facts.rs +++ b/codex-rs/analytics/src/facts.rs @@ -16,6 +16,7 @@ use codex_protocol::config_types::Personality; use codex_protocol::config_types::ReasoningSummary; use codex_protocol::config_types::ServiceTier; use codex_protocol::error::CodexErr; +pub use codex_protocol::error::CodexErrKind; use codex_protocol::models::PermissionProfile; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::protocol::AskForApproval; @@ -139,48 +140,6 @@ impl TurnCodexErrorFact { } } -#[derive(Clone, Copy, Debug, Serialize)] -#[serde(rename_all = "snake_case")] -pub enum CodexErrKind { - TurnAborted, - SessionBudgetExceeded, - Stream, - ContextWindowExceeded, - ThreadNotFound, - AgentLimitReached, - SessionConfiguredNotFirstEvent, - Timeout, - RequestTimeout, - Spawn, - Interrupted, - UnexpectedStatus, - InvalidRequest, - InvalidImageRequest, - UsageLimitReached, - ServerOverloaded, - CyberPolicy, - ResponseStreamFailed, - ConnectionFailed, - QuotaExceeded, - UsageNotIncluded, - InternalServerError, - RetryLimit, - InternalAgentDied, - Sandbox, - LandlockSandboxExecutableNotProvided, - UnsupportedOperation, - RefreshTokenFailed, - Fatal, - Io, - Json, - #[cfg(target_os = "linux")] - LandlockRuleset, - #[cfg(target_os = "linux")] - LandlockPathFd, - TokioJoin, - EnvVar, -} - #[derive(Clone)] pub(crate) struct TurnCodexError { pub(crate) kind: CodexErrKind, @@ -196,54 +155,6 @@ impl TurnCodexError { } } -impl From<&CodexErr> for CodexErrKind { - fn from(error: &CodexErr) -> Self { - match error { - CodexErr::TurnAborted => CodexErrKind::TurnAborted, - CodexErr::SessionBudgetExceeded => CodexErrKind::SessionBudgetExceeded, - CodexErr::Stream(..) => CodexErrKind::Stream, - CodexErr::ContextWindowExceeded => CodexErrKind::ContextWindowExceeded, - CodexErr::ThreadNotFound(_) => CodexErrKind::ThreadNotFound, - CodexErr::AgentLimitReached { .. } => CodexErrKind::AgentLimitReached, - CodexErr::SessionConfiguredNotFirstEvent => { - CodexErrKind::SessionConfiguredNotFirstEvent - } - CodexErr::Timeout => CodexErrKind::Timeout, - CodexErr::RequestTimeout => CodexErrKind::RequestTimeout, - CodexErr::Spawn => CodexErrKind::Spawn, - CodexErr::Interrupted => CodexErrKind::Interrupted, - CodexErr::UnexpectedStatus(_) => CodexErrKind::UnexpectedStatus, - CodexErr::InvalidRequest(_) => CodexErrKind::InvalidRequest, - CodexErr::InvalidImageRequest() => CodexErrKind::InvalidImageRequest, - CodexErr::UsageLimitReached(_) => CodexErrKind::UsageLimitReached, - CodexErr::ServerOverloaded => CodexErrKind::ServerOverloaded, - CodexErr::CyberPolicy { .. } => CodexErrKind::CyberPolicy, - CodexErr::ResponseStreamFailed(_) => CodexErrKind::ResponseStreamFailed, - CodexErr::ConnectionFailed(_) => CodexErrKind::ConnectionFailed, - CodexErr::QuotaExceeded => CodexErrKind::QuotaExceeded, - CodexErr::UsageNotIncluded => CodexErrKind::UsageNotIncluded, - CodexErr::InternalServerError => CodexErrKind::InternalServerError, - CodexErr::RetryLimit(_) => CodexErrKind::RetryLimit, - CodexErr::InternalAgentDied => CodexErrKind::InternalAgentDied, - CodexErr::Sandbox(_) => CodexErrKind::Sandbox, - CodexErr::LandlockSandboxExecutableNotProvided => { - CodexErrKind::LandlockSandboxExecutableNotProvided - } - CodexErr::UnsupportedOperation(_) => CodexErrKind::UnsupportedOperation, - CodexErr::RefreshTokenFailed(_) => CodexErrKind::RefreshTokenFailed, - CodexErr::Fatal(_) => CodexErrKind::Fatal, - CodexErr::Io(_) => CodexErrKind::Io, - CodexErr::Json(_) => CodexErrKind::Json, - #[cfg(target_os = "linux")] - CodexErr::LandlockRuleset(_) => CodexErrKind::LandlockRuleset, - #[cfg(target_os = "linux")] - CodexErr::LandlockPathFd(_) => CodexErrKind::LandlockPathFd, - CodexErr::TokioJoin(_) => CodexErrKind::TokioJoin, - CodexErr::EnvVar(_) => CodexErrKind::EnvVar, - } - } -} - #[derive(Clone, Copy, Debug, Serialize)] #[serde(rename_all = "snake_case")] pub enum TurnStatus { diff --git a/codex-rs/app-server/src/request_processors/request_errors.rs b/codex-rs/app-server/src/request_processors/request_errors.rs index 48bae43afa41..9d342c4b32a3 100644 --- a/codex-rs/app-server/src/request_processors/request_errors.rs +++ b/codex-rs/app-server/src/request_processors/request_errors.rs @@ -1,8 +1,9 @@ use super::*; +use codex_protocol::error::CodexErrorDetails; pub(super) fn environment_selection_error(err: CodexErr) -> JSONRPCErrorError { - match err { - CodexErr::InvalidRequest(message) => invalid_request(message), - err => internal_error(format!("failed to validate environment selections: {err}")), + match err.details() { + CodexErrorDetails::InvalidRequest(message) => invalid_request(message.clone()), + _ => internal_error(format!("failed to validate environment selections: {err}")), } } diff --git a/codex-rs/app-server/src/request_processors/thread_processor.rs b/codex-rs/app-server/src/request_processors/thread_processor.rs index 5d33dad6b3d3..8861a9f049c7 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -5,6 +5,7 @@ use crate::error_code::method_not_found; use codex_app_server_protocol::SelectedCapabilityRoot; use codex_extension_api::ExtensionDataInit; use codex_protocol::config_types::MultiAgentMode; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS; use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE; use codex_protocol::protocol::ThreadHistoryMode; @@ -1253,10 +1254,12 @@ impl ThreadRequestProcessor { thread_start.dynamic_tool_count = dynamic_tool_count, )) .await - .map_err(|err| match err { - CodexErr::InvalidRequest(message) => invalid_request(message), - CodexErr::UnsupportedOperation(message) => method_not_found(message), - err => internal_error(format!("error creating thread: {err}")), + .map_err(|err| match err.details() { + CodexErrorDetails::InvalidRequest(message) => invalid_request(message.clone()), + CodexErrorDetails::UnsupportedOperation(message) => { + method_not_found(message.clone()) + } + _ => internal_error(format!("error creating thread: {err}")), })?; let session_telemetry = thread.session_telemetry(); session_telemetry.record_startup_phase( @@ -1553,9 +1556,9 @@ impl ThreadRequestProcessor { let count = thread .decrement_out_of_band_elicitation_count() .await - .map_err(|err| match err { - CodexErr::InvalidRequest(message) => invalid_request(message), - err => internal_error(format!( + .map_err(|err| match err.details() { + CodexErrorDetails::InvalidRequest(message) => invalid_request(message.clone()), + _ => internal_error(format!( "failed to decrement out-of-band elicitation counter: {err}" )), })?; @@ -3399,9 +3402,9 @@ impl ThreadRequestProcessor { .await; } Err(err) => { - let error = match err { - CodexErr::InvalidRequest(message) => invalid_request(message), - err => internal_error(format!("error resuming thread: {err}")), + let error = match err.details() { + CodexErrorDetails::InvalidRequest(message) => invalid_request(message.clone()), + _ => internal_error(format!("error resuming thread: {err}")), }; self.outgoing.send_error(request_id, error).await; } @@ -4129,12 +4132,12 @@ impl ThreadRequestProcessor { supports_openai_form_elicitation, ) .await - .map_err(|err| match err { - CodexErr::Io(_) | CodexErr::Json(_) => { + .map_err(|err| match err.details() { + CodexErrorDetails::Io(_) | CodexErrorDetails::Json(_) => { invalid_request(format!("failed to load thread {source_thread_id}: {err}")) } - CodexErr::InvalidRequest(message) => invalid_request(message), - err => internal_error(format!("error forking thread: {err}")), + CodexErrorDetails::InvalidRequest(message) => invalid_request(message.clone()), + _ => internal_error(format!("error forking thread: {err}")), })?; Self::set_app_server_client_info( @@ -4932,13 +4935,13 @@ fn conversation_summary_rollout_path_read_error( } pub(super) fn core_thread_write_error(operation: &str, err: CodexErr) -> JSONRPCErrorError { - match err { - CodexErr::ThreadNotFound(thread_id) => { + match err.details() { + CodexErrorDetails::ThreadNotFound(thread_id) => { invalid_request(format!("thread not found: {thread_id}")) } - CodexErr::InvalidRequest(message) => invalid_request(message), - CodexErr::UnsupportedOperation(message) => method_not_found(message), - err => internal_error(format!("failed to {operation}: {err}")), + CodexErrorDetails::InvalidRequest(message) => invalid_request(message.clone()), + CodexErrorDetails::UnsupportedOperation(message) => method_not_found(message.clone()), + _ => internal_error(format!("failed to {operation}: {err}")), } } diff --git a/codex-rs/app-server/src/request_processors/turn_processor.rs b/codex-rs/app-server/src/request_processors/turn_processor.rs index 2477a6c991f6..eee0b0ae277a 100644 --- a/codex-rs/app-server/src/request_processors/turn_processor.rs +++ b/codex-rs/app-server/src/request_processors/turn_processor.rs @@ -2,6 +2,7 @@ use super::*; use codex_agent_extension::AgentInvocation; use codex_agent_extension::AgentRun; use codex_agent_extension::AgentRunner; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::models::ContentItem; use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::PermissionProfile; @@ -890,9 +891,9 @@ impl TurnRequestProcessor { thread .inject_response_items(items) .await - .map_err(|err| match err { - CodexErr::InvalidRequest(message) => invalid_request(message), - err => internal_error(format!("failed to inject response items: {err}")), + .map_err(|err| match err.details() { + CodexErrorDetails::InvalidRequest(message) => invalid_request(message.clone()), + _ => internal_error(format!("failed to inject response items: {err}")), })?; Ok(ThreadInjectItemsResponse {}) } diff --git a/codex-rs/codex-api/src/api_bridge.rs b/codex-rs/codex-api/src/api_bridge.rs index a56488b6ea66..c825ab03c5fc 100644 --- a/codex-rs/codex-api/src/api_bridge.rs +++ b/codex-rs/codex-api/src/api_bridge.rs @@ -8,6 +8,7 @@ use chrono::DateTime; use chrono::Utc; use codex_protocol::auth::PlanType; use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::error::RetryLimitReachedError; use codex_protocol::error::UnexpectedResponseError; use codex_protocol::error::UsageLimitReachedError; @@ -20,8 +21,14 @@ pub fn map_api_error(err: ApiError) -> CodexErr { ApiError::ContextWindowExceeded => CodexErr::ContextWindowExceeded, ApiError::QuotaExceeded => CodexErr::QuotaExceeded, ApiError::UsageNotIncluded => CodexErr::UsageNotIncluded, - ApiError::Retryable { message, delay } => CodexErr::Stream(message, delay), - ApiError::Stream(msg) => CodexErr::Stream(msg, None), + ApiError::Retryable { message, delay } => { + let error = CodexErr::Stream(message); + match delay { + Some(delay) => error.with_retry_delay(delay), + None => error, + } + } + ApiError::Stream(msg) => CodexErr::Stream(msg), ApiError::ServerOverloaded => CodexErr::ServerOverloaded, ApiError::Api { status, message } => { let user_message = api_error_user_message(status, &message); @@ -37,7 +44,9 @@ pub fn map_api_error(err: ApiError) -> CodexErr { }) } ApiError::InvalidRequest { message } => CodexErr::InvalidRequest(message), - ApiError::CyberPolicy { message } => CodexErr::CyberPolicy { message }, + ApiError::CyberPolicy { message } => { + CodexErr::new(CodexErrorDetails::CyberPolicy { message }) + } ApiError::Transport(transport) => match transport { TransportError::Http { status, @@ -72,7 +81,7 @@ pub fn map_api_error(err: ApiError) -> CodexErr { .filter(|message| !message.trim().is_empty()) .map(str::to_string) .unwrap_or_else(|| CYBER_POLICY_FALLBACK_MESSAGE.to_string()); - CodexErr::CyberPolicy { message } + CodexErr::new(CodexErrorDetails::CyberPolicy { message }) } else if body_text .contains("The image data you provided does not represent a valid image") { @@ -139,11 +148,9 @@ pub fn map_api_error(err: ApiError) -> CodexErr { request_id: None, }), TransportError::Timeout => CodexErr::RequestTimeout, - TransportError::Network(msg) | TransportError::Build(msg) => { - CodexErr::Stream(msg, None) - } + TransportError::Network(msg) | TransportError::Build(msg) => CodexErr::Stream(msg), }, - ApiError::RateLimit(msg) => CodexErr::Stream(msg, None), + ApiError::RateLimit(msg) => CodexErr::Stream(msg), } } diff --git a/codex-rs/codex-api/src/api_bridge_tests.rs b/codex-rs/codex-api/src/api_bridge_tests.rs index 4e531cbbfafd..391b4f5ea850 100644 --- a/codex-rs/codex-api/src/api_bridge_tests.rs +++ b/codex-rs/codex-api/src/api_bridge_tests.rs @@ -6,7 +6,22 @@ use pretty_assertions::assert_eq; #[test] fn map_api_error_maps_server_overloaded() { let err = map_api_error(ApiError::ServerOverloaded); - assert!(matches!(err, CodexErr::ServerOverloaded)); + assert!(matches!(err.details(), CodexErrorDetails::ServerOverloaded)); +} + +#[test] +fn map_api_error_preserves_retry_delay() { + let retry_delay = std::time::Duration::from_secs(17); + let err = map_api_error(ApiError::Retryable { + message: "retry later".to_string(), + delay: Some(retry_delay), + }); + + assert!(matches!( + err.details(), + CodexErrorDetails::Stream(message) if message == "retry later" + )); + assert_eq!(err.retry_delay(), Some(retry_delay)); } #[test] @@ -24,7 +39,7 @@ fn map_api_error_maps_server_overloaded_from_503_body() { body: Some(body), })); - assert!(matches!(err, CodexErr::ServerOverloaded)); + assert!(matches!(err.details(), CodexErrorDetails::ServerOverloaded)); } #[test] @@ -40,8 +55,8 @@ fn map_api_error_maps_cloudflare_blocked_response_to_user_message() { ), })); - let CodexErr::UnexpectedStatus(err) = err else { - panic!("expected CodexErr::UnexpectedStatus, got {err:?}"); + let CodexErrorDetails::UnexpectedStatus(err) = err.details() else { + panic!("expected CodexErrorDetails::UnexpectedStatus, got {err:?}"); }; assert_eq!( err.user_message.as_deref(), @@ -73,8 +88,8 @@ fn map_api_error_maps_cyber_policy_from_400_body() { body: Some(body), })); - let CodexErr::CyberPolicy { message } = err else { - panic!("expected CodexErr::CyberPolicy, got {err:?}"); + let CodexErrorDetails::CyberPolicy { message } = err.details() else { + panic!("expected CodexErrorDetails::CyberPolicy, got {err:?}"); }; assert_eq!( message, @@ -101,8 +116,8 @@ fn map_api_error_maps_wrapped_websocket_cyber_policy_from_400_body() { body: Some(body), })); - let CodexErr::CyberPolicy { message } = err else { - panic!("expected CodexErr::CyberPolicy, got {err:?}"); + let CodexErrorDetails::CyberPolicy { message } = err.details() else { + panic!("expected CodexErrorDetails::CyberPolicy, got {err:?}"); }; assert_eq!(message, "This websocket request was flagged."); } @@ -122,8 +137,8 @@ fn map_api_error_uses_cyber_policy_fallback_for_missing_message() { body: Some(body), })); - let CodexErr::CyberPolicy { message } = err else { - panic!("expected CodexErr::CyberPolicy, got {err:?}"); + let CodexErrorDetails::CyberPolicy { message } = err.details() else { + panic!("expected CodexErrorDetails::CyberPolicy, got {err:?}"); }; assert_eq!( message, @@ -147,10 +162,10 @@ fn map_api_error_keeps_unknown_400_errors_generic() { body: Some(body.clone()), })); - let CodexErr::InvalidRequest(message) = err else { - panic!("expected CodexErr::InvalidRequest, got {err:?}"); + let CodexErrorDetails::InvalidRequest(message) = err.details() else { + panic!("expected CodexErrorDetails::InvalidRequest, got {err:?}"); }; - assert_eq!(message, body); + assert_eq!(message, &body); } #[test] @@ -178,8 +193,8 @@ fn map_api_error_maps_usage_limit_limit_name_header() { body: Some(body), })); - let CodexErr::UsageLimitReached(usage_limit) = err else { - panic!("expected CodexErr::UsageLimitReached, got {err:?}"); + let CodexErrorDetails::UsageLimitReached(usage_limit) = err.details() else { + panic!("expected CodexErrorDetails::UsageLimitReached, got {err:?}"); }; assert_eq!( usage_limit @@ -211,8 +226,8 @@ fn map_api_error_does_not_fallback_limit_name_to_limit_id() { body: Some(body), })); - let CodexErr::UsageLimitReached(usage_limit) = err else { - panic!("expected CodexErr::UsageLimitReached, got {err:?}"); + let CodexErrorDetails::UsageLimitReached(usage_limit) = err.details() else { + panic!("expected CodexErrorDetails::UsageLimitReached, got {err:?}"); }; assert_eq!( usage_limit @@ -260,8 +275,8 @@ fn map_api_error_copies_rate_limit_reached_type_to_usage_limit_snapshot() { body: Some(body), })); - let CodexErr::UsageLimitReached(usage_limit) = err else { - panic!("expected CodexErr::UsageLimitReached, got {err:?}"); + let CodexErrorDetails::UsageLimitReached(usage_limit) = err.details() else { + panic!("expected CodexErrorDetails::UsageLimitReached, got {err:?}"); }; assert_eq!( usage_limit.rate_limit_reached_type, @@ -311,8 +326,8 @@ fn map_api_error_ignores_unparseable_rate_limit_reached_type_headers() { body: Some(body), })); - let CodexErr::UsageLimitReached(usage_limit) = err else { - panic!("expected CodexErr::UsageLimitReached, got {err:?}"); + let CodexErrorDetails::UsageLimitReached(usage_limit) = err.details() else { + panic!("expected CodexErrorDetails::UsageLimitReached, got {err:?}"); }; assert_eq!(usage_limit.rate_limit_reached_type, None); } @@ -341,8 +356,8 @@ fn map_api_error_extracts_identity_auth_details_from_headers() { body: Some(r#"{"detail":"Unauthorized"}"#.to_string()), })); - let CodexErr::UnexpectedStatus(err) = err else { - panic!("expected CodexErr::UnexpectedStatus, got {err:?}"); + let CodexErrorDetails::UnexpectedStatus(err) = err.details() else { + panic!("expected CodexErrorDetails::UnexpectedStatus, got {err:?}"); }; assert_eq!(err.request_id.as_deref(), Some("req-401")); assert_eq!(err.cf_ray.as_deref(), Some("ray-401")); diff --git a/codex-rs/core/src/agent/agent_resolver.rs b/codex-rs/core/src/agent/agent_resolver.rs index fff2d7afd6ae..76a2c4812283 100644 --- a/codex-rs/core/src/agent/agent_resolver.rs +++ b/codex-rs/core/src/agent/agent_resolver.rs @@ -2,6 +2,7 @@ use crate::function_tool::FunctionCallError; use crate::session::session::Session; use crate::session::turn_context::TurnContext; use codex_protocol::ThreadId; +use codex_protocol::error::CodexErrorDetails; use std::sync::Arc; /// Resolves a single tool-facing agent target to a thread id. @@ -20,11 +21,11 @@ pub(crate) async fn resolve_agent_target( .agent_control .resolve_agent_reference(session.thread_id, &turn.session_source, target) .await - .map_err(|err| match err { - codex_protocol::error::CodexErr::UnsupportedOperation(message) => { - FunctionCallError::RespondToModel(message) + .map_err(|err| match err.details() { + CodexErrorDetails::UnsupportedOperation(message) => { + FunctionCallError::RespondToModel(message.clone()) } - other => FunctionCallError::RespondToModel(other.to_string()), + _ => FunctionCallError::RespondToModel(err.to_string()), }) } diff --git a/codex-rs/core/src/agent/control.rs b/codex-rs/core/src/agent/control.rs index 749c3f3f4b0e..74a8bc224b88 100644 --- a/codex-rs/core/src/agent/control.rs +++ b/codex-rs/core/src/agent/control.rs @@ -22,6 +22,7 @@ use codex_protocol::AgentPath; use codex_protocol::SessionId; use codex_protocol::ThreadId; use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::error::Result as CodexResult; use codex_protocol::models::ContentItem; use codex_protocol::models::MessagePhase; @@ -241,7 +242,10 @@ impl AgentControl { state: &Arc, result: CodexResult, ) -> CodexResult { - if matches!(result, Err(CodexErr::InternalAgentDied)) { + if result + .as_ref() + .is_err_and(|err| matches!(err.details(), CodexErrorDetails::InternalAgentDied)) + { let _ = state.remove_thread(&agent_id).await; self.forget_v2_residency(agent_id); self.state.release_spawned_thread(agent_id); @@ -278,7 +282,7 @@ impl AgentControl { pub(crate) fn ensure_agent_known(&self, agent_id: ThreadId) -> CodexResult { self.state .agent_metadata_for_thread(agent_id) - .ok_or(CodexErr::ThreadNotFound(agent_id)) + .ok_or_else(|| CodexErr::ThreadNotFound(agent_id)) } pub(crate) async fn list_live_agent_subtree_thread_ids( diff --git a/codex-rs/core/src/agent/control/execution.rs b/codex-rs/core/src/agent/control/execution.rs index 2c4fc76f6fd1..fe10868dbb11 100644 --- a/codex-rs/core/src/agent/control/execution.rs +++ b/codex-rs/core/src/agent/control/execution.rs @@ -1,6 +1,7 @@ use super::AgentControl; use codex_protocol::ThreadId; use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::error::Result as CodexResult; use codex_protocol::protocol::MultiAgentVersion; use codex_protocol::protocol::Op; @@ -68,7 +69,9 @@ impl AgentControl { if self.agent_execution_limiter.has_capacity() { Ok(()) } else { - Err(CodexErr::AgentLimitReached { max_threads }) + Err(CodexErr::new(CodexErrorDetails::AgentLimitReached { + max_threads, + })) } } diff --git a/codex-rs/core/src/agent/control/execution_tests.rs b/codex-rs/core/src/agent/control/execution_tests.rs index 9cad08f95128..8ad8ddcf9f18 100644 --- a/codex-rs/core/src/agent/control/execution_tests.rs +++ b/codex-rs/core/src/agent/control/execution_tests.rs @@ -1,5 +1,5 @@ use crate::agent::AgentControl; -use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::protocol::MultiAgentVersion; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; @@ -29,10 +29,10 @@ fn execution_guards_count_active_v2_subagent_turns() { let Err(err) = control.ensure_execution_capacity(MultiAgentVersion::V2, &source) else { panic!("second active turn should exceed the derived non-root cap"); }; - let CodexErr::AgentLimitReached { max_threads } = err else { + let CodexErrorDetails::AgentLimitReached { max_threads } = err.details() else { panic!("expected AgentLimitReached"); }; - assert_eq!(max_threads, 1); + assert_eq!(*max_threads, 1); drop(first); control diff --git a/codex-rs/core/src/agent/control/legacy.rs b/codex-rs/core/src/agent/control/legacy.rs index 9264833cbb57..3fb1717fa600 100644 --- a/codex-rs/core/src/agent/control/legacy.rs +++ b/codex-rs/core/src/agent/control/legacy.rs @@ -1,4 +1,5 @@ use super::*; +use codex_protocol::error::CodexErrorDetails; impl AgentControl { /// Submit a shutdown request for a live agent without marking it explicitly closed in @@ -43,7 +44,9 @@ impl AgentControl { warn!("failed to persist thread-spawn edge status for {agent_id}: {err}"); } } - Err(CodexErr::ThreadNotFound(_)) if known_agent => { + Err(err) + if known_agent && matches!(err.details(), CodexErrorDetails::ThreadNotFound(_)) => + { if let Some(agent_graph_store) = state.agent_graph_store() && let Err(err) = agent_graph_store .set_thread_spawn_edge_status( @@ -57,13 +60,19 @@ impl AgentControl { ))); } } - Err(CodexErr::ThreadNotFound(_)) => {} + Err(err) if matches!(err.details(), CodexErrorDetails::ThreadNotFound(_)) => {} Err(err) => { warn!("failed to inspect agent before close {agent_id}: {err}"); } } match Box::pin(self.shutdown_agent_tree(agent_id)).await { - Err(CodexErr::ThreadNotFound(_)) | Err(CodexErr::InternalAgentDied) if known_agent => { + Err(err) + if known_agent + && matches!( + err.details(), + CodexErrorDetails::ThreadNotFound(_) | CodexErrorDetails::InternalAgentDied + ) => + { Ok(String::new()) } result => result, @@ -76,7 +85,12 @@ impl AgentControl { let result = self.shutdown_live_agent(agent_id).await; for descendant_id in descendant_ids { match self.shutdown_live_agent(descendant_id).await { - Ok(_) | Err(CodexErr::ThreadNotFound(_)) | Err(CodexErr::InternalAgentDied) => {} + Ok(_) => {} + Err(err) + if matches!( + err.details(), + CodexErrorDetails::ThreadNotFound(_) | CodexErrorDetails::InternalAgentDied + ) => {} Err(err) => return Err(err), } } diff --git a/codex-rs/core/src/agent/control/residency.rs b/codex-rs/core/src/agent/control/residency.rs index 0f069a8e7f61..99fa05112aa2 100644 --- a/codex-rs/core/src/agent/control/residency.rs +++ b/codex-rs/core/src/agent/control/residency.rs @@ -5,6 +5,7 @@ use crate::config::Config; use crate::thread_manager::ThreadManagerState; use codex_protocol::ThreadId; use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::error::Result as CodexResult; use codex_protocol::protocol::MultiAgentVersion; use codex_protocol::protocol::SessionSource; @@ -94,9 +95,9 @@ impl V2Residency { .try_unload_one_resident(manager, protected_thread_id) .await { - return Err(CodexErr::AgentLimitReached { + return Err(CodexErr::new(CodexErrorDetails::AgentLimitReached { max_threads: capacity, - }); + })); } } } diff --git a/codex-rs/core/src/agent/control/residency_tests.rs b/codex-rs/core/src/agent/control/residency_tests.rs index cc51e69da79a..75afdd45cdde 100644 --- a/codex-rs/core/src/agent/control/residency_tests.rs +++ b/codex-rs/core/src/agent/control/residency_tests.rs @@ -8,7 +8,7 @@ use crate::thread_manager::ThreadManagerState; use codex_features::Feature; use codex_login::CodexAuth; use codex_protocol::ThreadId; -use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; @@ -54,8 +54,10 @@ async fn residency_slot_reservation_unloads_oldest_idle_v2_agent() { .await .expect("second resident slot should evict the first idle agent"); match manager.get_thread(first.thread_id).await { - Err(CodexErr::ThreadNotFound(thread_id)) => assert_eq!(thread_id, first.thread_id), - Err(err) => panic!("expected evicted thread to be missing, got {err:?}"), + Err(err) => match err.details() { + CodexErrorDetails::ThreadNotFound(thread_id) => assert_eq!(*thread_id, first.thread_id), + _ => panic!("expected evicted thread to be missing, got {err:?}"), + }, Ok(_) => panic!("expected evicted thread to be missing"), } let second = spawn_v2_subagent(&control, &state, config, root.thread_id, "worker-2").await; @@ -100,8 +102,10 @@ async fn interrupted_v2_agent_is_lost_after_residency_eviction() { .await .expect("second resident slot should evict the first interrupted idle agent"); match manager.get_thread(first.thread_id).await { - Err(CodexErr::ThreadNotFound(thread_id)) => assert_eq!(thread_id, first.thread_id), - Err(err) => panic!("expected evicted thread to be missing, got {err:?}"), + Err(err) => match err.details() { + CodexErrorDetails::ThreadNotFound(thread_id) => assert_eq!(*thread_id, first.thread_id), + _ => panic!("expected evicted thread to be missing, got {err:?}"), + }, Ok(_) => panic!("expected evicted thread to be missing"), } let second = @@ -113,16 +117,18 @@ async fn interrupted_v2_agent_is_lost_after_residency_eviction() { .ensure_v2_agent_loaded(config, first.thread_id) .await .expect_err("evicted interrupted agent should stay lost"); - match err { - CodexErr::ThreadNotFound(thread_id) => assert_eq!(thread_id, first.thread_id), - err => panic!("expected ThreadNotFound, got {err:?}"), + match err.details() { + CodexErrorDetails::ThreadNotFound(thread_id) => assert_eq!(*thread_id, first.thread_id), + _ => panic!("expected ThreadNotFound, got {err:?}"), } assert!(manager.get_thread(root.thread_id).await.is_ok()); assert!(manager.get_thread(second.thread_id).await.is_ok()); match manager.get_thread(first.thread_id).await { - Err(CodexErr::ThreadNotFound(thread_id)) => assert_eq!(thread_id, first.thread_id), - Err(err) => panic!("expected evicted thread to be missing, got {err:?}"), + Err(err) => match err.details() { + CodexErrorDetails::ThreadNotFound(thread_id) => assert_eq!(*thread_id, first.thread_id), + _ => panic!("expected evicted thread to be missing, got {err:?}"), + }, Ok(_) => panic!("expected evicted thread to be missing"), } } diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index 260b95a483e0..ab14e8dbebbe 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -26,6 +26,7 @@ use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::items::TurnItem; use codex_protocol::items::UserMessageItem; use codex_protocol::models::ContentItem; @@ -350,8 +351,10 @@ async fn wait_for_live_thread_spawn_children( async fn assert_thread_not_loaded(manager: &ThreadManager, thread_id: ThreadId) { match manager.get_thread(thread_id).await { - Err(CodexErr::ThreadNotFound(id)) => assert_eq!(id, thread_id), - Err(err) => panic!("expected ThreadNotFound, got {err:?}"), + Err(err) => match err.details() { + CodexErrorDetails::ThreadNotFound(id) => assert_eq!(*id, thread_id), + _ => panic!("expected ThreadNotFound, got {err:?}"), + }, Ok(_) => panic!("expected thread not to be loaded"), } } @@ -483,7 +486,10 @@ async fn send_input_errors_when_thread_missing() { ) .await .expect_err("send_input should fail for missing thread"); - assert_matches!(err, CodexErr::ThreadNotFound(id) if id == thread_id); + assert_matches!( + err.details(), + CodexErrorDetails::ThreadNotFound(id) if *id == thread_id + ); } #[tokio::test] @@ -510,7 +516,10 @@ async fn subscribe_status_errors_for_missing_thread() { .subscribe_status(thread_id) .await .expect_err("subscribe_status should fail for missing thread"); - assert_matches!(err, CodexErr::ThreadNotFound(id) if id == thread_id); + assert_matches!( + err.details(), + CodexErrorDetails::ThreadNotFound(id) if *id == thread_id + ); } #[tokio::test] @@ -689,8 +698,10 @@ async fn ensure_v2_agent_loaded_reloads_registered_unloaded_agent() { .is_some() ); match harness.manager.get_thread(spawned_agent.thread_id).await { - Err(CodexErr::ThreadNotFound(id)) => assert_eq!(id, spawned_agent.thread_id), - Err(err) => panic!("expected ThreadNotFound, got {err:?}"), + Err(err) => match err.details() { + CodexErrorDetails::ThreadNotFound(id) => assert_eq!(*id, spawned_agent.thread_id), + _ => panic!("expected ThreadNotFound, got {err:?}"), + }, Ok(_) => panic!("expected thread to be removed"), } @@ -1978,13 +1989,13 @@ async fn spawn_agent_respects_legacy_max_threads_alias() { ) .await .expect_err("spawn_agent should respect max threads"); - let CodexErr::AgentLimitReached { + let CodexErrorDetails::AgentLimitReached { max_threads: seen_max_threads, - } = err + } = err.details() else { - panic!("expected CodexErr::AgentLimitReached"); + panic!("expected AgentLimitReached"); }; - assert_eq!(seen_max_threads, max_threads); + assert_eq!(*seen_max_threads, max_threads); let _ = control .shutdown_live_agent(first_agent_id) @@ -2069,10 +2080,10 @@ async fn spawn_agent_limit_shared_across_clones() { ) .await .expect_err("spawn_agent should respect shared guard"); - let CodexErr::AgentLimitReached { max_threads } = err else { - panic!("expected CodexErr::AgentLimitReached"); + let CodexErrorDetails::AgentLimitReached { max_threads } = err.details() else { + panic!("expected AgentLimitReached"); }; - assert_eq!(max_threads, 1); + assert_eq!(*max_threads, 1); let _ = control .shutdown_live_agent(first_agent_id) @@ -2122,13 +2133,13 @@ async fn resume_agent_respects_max_threads_limit() { .resume_agent_from_rollout(config, resumable_id, SessionSource::Exec) .await .expect_err("resume should respect max threads"); - let CodexErr::AgentLimitReached { + let CodexErrorDetails::AgentLimitReached { max_threads: seen_max_threads, - } = err + } = err.details() else { - panic!("expected CodexErr::AgentLimitReached"); + panic!("expected AgentLimitReached"); }; - assert_eq!(seen_max_threads, max_threads); + assert_eq!(*seen_max_threads, max_threads); let _ = control .shutdown_live_agent(active_id) diff --git a/codex-rs/core/src/agent/registry.rs b/codex-rs/core/src/agent/registry.rs index eac2f14ae481..36b8cdf862f1 100644 --- a/codex-rs/core/src/agent/registry.rs +++ b/codex-rs/core/src/agent/registry.rs @@ -1,6 +1,7 @@ use codex_protocol::AgentPath; use codex_protocol::ThreadId; use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::error::Result; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; @@ -82,7 +83,9 @@ impl AgentRegistry { ) -> Result { if let Some(max_threads) = max_threads { if !self.try_increment_spawned(max_threads) { - return Err(CodexErr::AgentLimitReached { max_threads }); + return Err(CodexErr::new(CodexErrorDetails::AgentLimitReached { + max_threads, + })); } } else { self.total_count.fetch_add(1, Ordering::AcqRel); diff --git a/codex-rs/core/src/agent/registry_tests.rs b/codex-rs/core/src/agent/registry_tests.rs index fc172fb336df..160489d2d121 100644 --- a/codex-rs/core/src/agent/registry_tests.rs +++ b/codex-rs/core/src/agent/registry_tests.rs @@ -1,5 +1,6 @@ use super::*; use codex_protocol::AgentPath; +use codex_protocol::error::CodexErrorDetails; use pretty_assertions::assert_eq; use std::collections::HashSet; @@ -91,10 +92,10 @@ fn commit_holds_slot_until_release() { Ok(_) => panic!("limit should be enforced"), Err(err) => err, }; - let CodexErr::AgentLimitReached { max_threads } = err else { - panic!("expected CodexErr::AgentLimitReached"); + let CodexErrorDetails::AgentLimitReached { max_threads } = err.details() else { + panic!("expected AgentLimitReached"); }; - assert_eq!(max_threads, 1); + assert_eq!(*max_threads, 1); registry.release_spawned_thread(thread_id); let reservation = registry @@ -116,10 +117,10 @@ fn release_ignores_unknown_thread_id() { Ok(_) => panic!("limit should still be enforced"), Err(err) => err, }; - let CodexErr::AgentLimitReached { max_threads } = err else { - panic!("expected CodexErr::AgentLimitReached"); + let CodexErrorDetails::AgentLimitReached { max_threads } = err.details() else { + panic!("expected AgentLimitReached"); }; - assert_eq!(max_threads, 1); + assert_eq!(*max_threads, 1); registry.release_spawned_thread(thread_id); let reservation = registry @@ -147,10 +148,10 @@ fn release_is_idempotent_for_registered_threads() { Ok(_) => panic!("limit should still be enforced"), Err(err) => err, }; - let CodexErr::AgentLimitReached { max_threads } = err else { - panic!("expected CodexErr::AgentLimitReached"); + let CodexErrorDetails::AgentLimitReached { max_threads } = err.details() else { + panic!("expected AgentLimitReached"); }; - assert_eq!(max_threads, 1); + assert_eq!(*max_threads, 1); registry.release_spawned_thread(second_id); let reservation = registry diff --git a/codex-rs/core/src/codex_delegate_tests.rs b/codex-rs/core/src/codex_delegate_tests.rs index c74a01c584c4..3a8f1e4589e2 100644 --- a/codex-rs/core/src/codex_delegate_tests.rs +++ b/codex-rs/core/src/codex_delegate_tests.rs @@ -5,6 +5,7 @@ use crate::mcp_tool_call::MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX; use async_channel::bounded; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_protocol::config_types::ApprovalsReviewer; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::models::NetworkPermissions; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::AgentStatus; @@ -216,7 +217,10 @@ async fn run_codex_thread_interactive_respects_pre_cancelled_spawn() { .await .expect("cancelled delegate spawn should not hang"); - assert!(matches!(result, Err(CodexErr::TurnAborted))); + assert!(matches!( + result, + Err(err) if matches!(err.details(), CodexErrorDetails::TurnAborted) + )); } #[tokio::test] diff --git a/codex-rs/core/src/compact.rs b/codex-rs/core/src/compact.rs index 783b6bdb0528..71abe5acc369 100644 --- a/codex-rs/core/src/compact.rs +++ b/codex-rs/core/src/compact.rs @@ -29,6 +29,7 @@ use codex_analytics::CompactionStrategy; use codex_analytics::CompactionTrigger; use codex_analytics::now_unix_seconds; use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::error::Result as CodexResult; use codex_protocol::items::ContextCompactionItem; use codex_protocol::items::TurnItem; @@ -291,16 +292,21 @@ async fn run_compact_task_inner_impl( Ok(()) => { break; } - Err(err @ (CodexErr::Interrupted | CodexErr::TurnAborted)) => { + Err(err) + if matches!( + err.details(), + CodexErrorDetails::Interrupted | CodexErrorDetails::TurnAborted + ) => + { return Err(err); } - Err(e @ CodexErr::SessionBudgetExceeded) => { + Err(e) if matches!(e.details(), CodexErrorDetails::SessionBudgetExceeded) => { sess.track_turn_codex_error(turn_context.as_ref(), &e); let event = EventMsg::Error(e.to_error_event(/*message_prefix*/ None)); sess.send_event(&turn_context, event).await; return Err(e); } - Err(e @ CodexErr::ContextWindowExceeded) => { + Err(e) if matches!(e.details(), CodexErrorDetails::ContextWindowExceeded) => { if turn_input_len > 1 { // Trim from the beginning to preserve cache (prefix-based) and keep recent messages intact. error!( @@ -479,7 +485,14 @@ impl CompactionAnalyticsAttempt { pub(crate) fn compaction_status_from_result(result: &CodexResult) -> CompactionStatus { match result { Ok(_) => CompactionStatus::Completed, - Err(CodexErr::Interrupted | CodexErr::TurnAborted) => CompactionStatus::Interrupted, + Err(err) + if matches!( + err.details(), + CodexErrorDetails::Interrupted | CodexErrorDetails::TurnAborted + ) => + { + CompactionStatus::Interrupted + } Err(_) => CompactionStatus::Failed, } } @@ -697,7 +710,6 @@ async fn drain_to_completed( let Some(event) = maybe_event else { return Err(CodexErr::Stream( "stream closed before response.completed".into(), - None, )); }; match event { diff --git a/codex-rs/core/src/compact_model_fallback.rs b/codex-rs/core/src/compact_model_fallback.rs index 3cc797a1d976..3c4d1f24ea24 100644 --- a/codex-rs/core/src/compact_model_fallback.rs +++ b/codex-rs/core/src/compact_model_fallback.rs @@ -2,19 +2,20 @@ use codex_analytics::CompactionImplementation; use codex_analytics::CompactionReason; use codex_otel::SessionTelemetry; use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use tracing::warn; /// Retries failures that may be model-specific and succeed with a different model. pub(crate) fn should_retry_with_current_model(error: &CodexErr) -> bool { matches!( - error, - CodexErr::InvalidRequest(_) - | CodexErr::UnexpectedStatus(_) - | CodexErr::ContextWindowExceeded - | CodexErr::UsageLimitReached(_) - | CodexErr::ServerOverloaded - | CodexErr::InternalServerError - | CodexErr::RetryLimit(_) + error.details(), + CodexErrorDetails::InvalidRequest(_) + | CodexErrorDetails::UnexpectedStatus(_) + | CodexErrorDetails::ContextWindowExceeded + | CodexErrorDetails::UsageLimitReached(_) + | CodexErrorDetails::ServerOverloaded + | CodexErrorDetails::InternalServerError + | CodexErrorDetails::RetryLimit(_) ) } diff --git a/codex-rs/core/src/compact_remote_v2.rs b/codex-rs/core/src/compact_remote_v2.rs index 0bcc85132cfb..034dd10e2acf 100644 --- a/codex-rs/core/src/compact_remote_v2.rs +++ b/codex-rs/core/src/compact_remote_v2.rs @@ -29,6 +29,7 @@ use codex_analytics::CompactionPhase; use codex_analytics::CompactionReason; use codex_analytics::CompactionTrigger; use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::error::Result as CodexResult; use codex_protocol::items::ContextCompactionItem; use codex_protocol::items::TurnItem; @@ -185,7 +186,7 @@ async fn run_remote_compact_task_inner( .await; match result { Ok(()) => Ok(()), - Err(err @ CodexErr::TurnAborted) => Err(err), + Err(err) if matches!(err.details(), CodexErrorDetails::TurnAborted) => Err(err), Err(err) => { sess.track_turn_codex_error(turn_context, &err); let event = EventMsg::Error( @@ -413,7 +414,6 @@ async fn collect_compaction_output( if !saw_completed { return Err(CodexErr::Stream( "remote compaction v2 stream closed before response.completed".to_string(), - None, )); } diff --git a/codex-rs/core/src/responses_retry.rs b/codex-rs/core/src/responses_retry.rs index 2af97820f011..dee517c9854b 100644 --- a/codex-rs/core/src/responses_retry.rs +++ b/codex-rs/core/src/responses_retry.rs @@ -48,12 +48,7 @@ pub(crate) async fn handle_retryable_response_stream_error( if *retries < max_retries { *retries += 1; let retry_count = *retries; - let delay = match &err { - CodexErr::Stream(_, requested_delay) => { - requested_delay.unwrap_or_else(|| backoff(retry_count)) - } - _ => backoff(retry_count), - }; + let delay = err.retry_delay().unwrap_or_else(|| backoff(retry_count)); log_retry(request, turn_context, &err, retry_count, max_retries, delay); // In release builds, hide the first websocket retry notification to reduce noisy diff --git a/codex-rs/core/src/responses_retry_tests.rs b/codex-rs/core/src/responses_retry_tests.rs index 94ad2e46d0f0..b6465ded37cb 100644 --- a/codex-rs/core/src/responses_retry_tests.rs +++ b/codex-rs/core/src/responses_retry_tests.rs @@ -20,10 +20,7 @@ async fn sampling_retry_logs_stream_error_context() { log_retry( ResponsesStreamRequest::Sampling, &turn_context, - &CodexErr::Stream( - "websocket closed by server before response.completed".to_string(), - None, - ), + &CodexErr::Stream("websocket closed by server before response.completed".to_string()), /*retries*/ 2, /*max_retries*/ 5, Duration::from_secs(1), diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 5a42655e0946..54ed25788980 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -199,6 +199,7 @@ use codex_config::types::McpServerConfig; use codex_config::types::OAuthCredentialsStoreMode; use codex_model_provider_info::ModelProviderInfo; use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::error::Result as CodexResult; #[cfg(test)] use codex_protocol::exec_output::StreamOutput; @@ -819,7 +820,7 @@ impl SessionIo { let session_loop_termination = self.session_loop_termination.clone(); match self.submit(Op::Shutdown).await { Ok(_) => {} - Err(CodexErr::InternalAgentDied) => {} + Err(err) if matches!(err.details(), CodexErrorDetails::InternalAgentDied) => {} Err(err) => return Err(err), } session_loop_termination.await; diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 3bd3bcc2d146..a425affb19d1 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -87,6 +87,7 @@ use codex_protocol::config_types::AutoCompactTokenLimitScope; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::ServiceTier; use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::error::Result as CodexResult; use codex_protocol::items::PlanItem; use codex_protocol::items::TurnItem; @@ -168,7 +169,7 @@ pub(crate) async fn run_turn( ) .await { - if matches!(err, CodexErr::TurnAborted) { + if matches!(err.details(), CodexErrorDetails::TurnAborted) { run_hooks_and_record_inputs(&sess, &turn_context, &input).await; return Err(err); } @@ -185,7 +186,7 @@ pub(crate) async fn run_turn( .await { Ok(step_context) => step_context, - Err(err @ CodexErr::TurnAborted) => { + Err(err) if matches!(err.details(), CodexErrorDetails::TurnAborted) => { run_hooks_and_record_inputs(&sess, &turn_context, &input).await; return Err(err); } @@ -405,7 +406,7 @@ pub(crate) async fn run_turn( ) .await { - if matches!(err, CodexErr::TurnAborted) { + if matches!(err.details(), CodexErrorDetails::TurnAborted) { return Err(err); } let error = err.to_codex_protocol_error(); @@ -473,10 +474,15 @@ pub(crate) async fn run_turn( } continue; } - Err(err @ CodexErr::TurnAborted) => { + Err(err) if matches!(err.details(), CodexErrorDetails::TurnAborted) => { return Err(err); } - Err(codex_error @ CodexErr::InvalidImageRequest()) => { + Err(codex_error) + if matches!( + codex_error.details(), + CodexErrorDetails::InvalidImageRequest() + ) => + { sess.track_turn_codex_error(turn_context.as_ref(), &codex_error); let error = CodexErrorInfo::BadRequest; sess.emit_turn_error_lifecycle(turn_context.as_ref(), error.clone()) @@ -1222,18 +1228,20 @@ async fn run_sampling_request( Ok(output) => { return Ok((output, original_input.unwrap_or(prompt.input))); } - Err(CodexErr::ContextWindowExceeded) => { - sess.set_total_tokens_full(&turn_context).await; - return Err(CodexErr::ContextWindowExceeded); - } - Err(CodexErr::UsageLimitReached(e)) => { - let rate_limits = e.rate_limits.clone(); - if let Some(rate_limits) = rate_limits { - sess.update_rate_limits(&turn_context, *rate_limits).await; + Err(err) => match err.details() { + CodexErrorDetails::ContextWindowExceeded => { + sess.set_total_tokens_full(&turn_context).await; + return Err(err); } - return Err(CodexErr::UsageLimitReached(e)); - } - Err(err) => err, + CodexErrorDetails::UsageLimitReached(e) => { + let rate_limits = e.rate_limits.clone(); + if let Some(rate_limits) = rate_limits { + sess.update_rate_limits(&turn_context, *rate_limits).await; + } + return Err(err); + } + _ => err, + }, }; if original_input.is_none() { @@ -2074,7 +2082,9 @@ async fn try_run_sampling_request( .await { Ok(event) => event, - Err(codex_async_utils::CancelErr::Cancelled) => break Err(CodexErr::TurnAborted), + Err(codex_async_utils::CancelErr::Cancelled) => { + break Err(CodexErr::TurnAborted); + } }; let event = match event { @@ -2083,7 +2093,6 @@ async fn try_run_sampling_request( None => { break Err(CodexErr::Stream( "stream closed before response.completed".into(), - None, )); } }; diff --git a/codex-rs/core/src/tasks/compact.rs b/codex-rs/core/src/tasks/compact.rs index eba04f1d1065..def729b2ace8 100644 --- a/codex-rs/core/src/tasks/compact.rs +++ b/codex-rs/core/src/tasks/compact.rs @@ -8,7 +8,7 @@ use crate::session::TurnInput; use crate::session::turn_context::TurnContext; use crate::state::TaskKind; use codex_features::Feature; -use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::user_input::UserInput; use tokio_util::sync::CancellationToken; @@ -76,7 +76,9 @@ impl SessionTask for CompactTask { }]; crate::compact::run_compact_task(session.clone(), ctx, input).await }; - if let Err(err @ CodexErr::TurnAborted) = result { + if let Err(err) = result + && matches!(err.details(), CodexErrorDetails::TurnAborted) + { return Err(err); } Ok(None) diff --git a/codex-rs/core/src/tasks/mod.rs b/codex-rs/core/src/tasks/mod.rs index d931e8bd6575..54e561de4fac 100644 --- a/codex-rs/core/src/tasks/mod.rs +++ b/codex-rs/core/src/tasks/mod.rs @@ -54,7 +54,7 @@ use codex_protocol::protocol::TurnCompleteEvent; use codex_protocol::protocol::WarningEvent; use codex_features::Feature; -use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::error::Result as CodexResult; use codex_protocol::models::ContentItem; pub(crate) use compact::CompactTask; @@ -584,7 +584,9 @@ impl Session { ) { let (last_agent_message, abort_reason) = match task_result { Ok(last_agent_message) => (last_agent_message, None), - Err(CodexErr::TurnAborted) => (None, Some(TurnAbortReason::Interrupted)), + Err(err) if matches!(err.details(), CodexErrorDetails::TurnAborted) => { + (None, Some(TurnAbortReason::Interrupted)) + } Err(err) => { warn!(%err, "session task returned an unexpected error"); (None, None) diff --git a/codex-rs/core/src/thread_rollout_truncation_tests.rs b/codex-rs/core/src/thread_rollout_truncation_tests.rs index de2b8bf9e71a..8391f967572a 100644 --- a/codex-rs/core/src/thread_rollout_truncation_tests.rs +++ b/codex-rs/core/src/thread_rollout_truncation_tests.rs @@ -3,6 +3,7 @@ use crate::session::tests::build_world_state_from_turn_context; use crate::session::tests::make_session_and_context; use codex_protocol::AgentPath; use codex_protocol::ResponseItemId; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::models::ContentItem; use codex_protocol::models::ReasoningItemReasoningSummary; use codex_protocol::protocol::InterAgentCommunication; @@ -169,8 +170,8 @@ fn truncate_rollout_after_turn_id_rejects_rolled_back_turn() { .expect_err("rolled-back turn should not be a fork anchor"); assert!(matches!( - err, - CodexErr::InvalidRequest(message) + err.details(), + CodexErrorDetails::InvalidRequest(message) if message == "lastTurnId 'turn-2' was not found in the source thread" )); } @@ -188,8 +189,8 @@ fn truncate_rollout_after_turn_id_rejects_synthetic_legacy_turn_id() { .expect_err("synthetic turn should not be a fork anchor"); assert!(matches!( - err, - CodexErr::InvalidRequest(message) + err.details(), + CodexErrorDetails::InvalidRequest(message) if message == "lastTurnId 'rollout-0' is not a persisted canonical turn in the source thread" )); @@ -203,8 +204,8 @@ fn truncate_rollout_after_turn_id_rejects_in_progress_turn() { .expect_err("in-progress turn should not be a fork anchor"); assert!(matches!( - err, - CodexErr::InvalidRequest(message) + err.details(), + CodexErrorDetails::InvalidRequest(message) if message == "lastTurnId 'turn-1' identifies an in-progress turn" )); } diff --git a/codex-rs/core/src/tools/events.rs b/codex-rs/core/src/tools/events.rs index d67ee115e1f8..d1794b527078 100644 --- a/codex-rs/core/src/tools/events.rs +++ b/codex-rs/core/src/tools/events.rs @@ -4,7 +4,7 @@ use crate::session::turn_context::TurnContext; use crate::tools::context::SharedTurnDiffTracker; use crate::tools::sandboxing::ToolError; use codex_apply_patch::AppliedPatchDelta; -use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::error::SandboxErr; use codex_protocol::exec_output::ExecToolCallOutput; use codex_protocol::items::CommandExecutionItem; @@ -382,33 +382,37 @@ impl ToolEmitter { }; (event, result) } - Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Timeout { output }))) => { - let response = self.format_exec_output_for_model(&output, ctx); - let event = ToolEventStage::Failure(ToolEventFailure::Output(*output)); - let result = Err(FunctionCallError::RespondToModel(response)); - (event, result) - } - Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output, .. }))) => { - let response = self.format_exec_output_for_model(&output, ctx); - // apply_patch can be denied after it has already committed a - // known prefix. Reuse the output-bearing path so the visible - // item still fails while the turn diff consumes that prefix. - let event = match (self, applied_patch_delta) { - (Self::ApplyPatch { .. }, Some(delta)) => ToolEventStage::Success { - output: *output, - applied_patch_delta: Some(delta), - }, - _ => ToolEventStage::Failure(ToolEventFailure::Output(*output)), - }; - let result = Err(FunctionCallError::RespondToModel(response)); - (event, result) - } - Err(ToolError::Codex(err)) => { - let message = format!("execution error: {err:?}"); - let event = ToolEventStage::Failure(ToolEventFailure::Message(message.clone())); - let result = Err(FunctionCallError::RespondToModel(message)); - (event, result) - } + Err(ToolError::Codex(err)) => match err.details() { + CodexErrorDetails::Sandbox(SandboxErr::Timeout { output }) => { + let output = output.as_ref().clone(); + let response = self.format_exec_output_for_model(&output, ctx); + let event = ToolEventStage::Failure(ToolEventFailure::Output(output)); + let result = Err(FunctionCallError::RespondToModel(response)); + (event, result) + } + CodexErrorDetails::Sandbox(SandboxErr::Denied { output, .. }) => { + let output = output.as_ref().clone(); + let response = self.format_exec_output_for_model(&output, ctx); + // apply_patch can be denied after it has already committed a + // known prefix. Reuse the output-bearing path so the visible + // item still fails while the turn diff consumes that prefix. + let event = match (self, applied_patch_delta) { + (Self::ApplyPatch { .. }, Some(delta)) => ToolEventStage::Success { + output, + applied_patch_delta: Some(delta), + }, + _ => ToolEventStage::Failure(ToolEventFailure::Output(output)), + }; + let result = Err(FunctionCallError::RespondToModel(response)); + (event, result) + } + _ => { + let message = format!("execution error: {err:?}"); + let event = ToolEventStage::Failure(ToolEventFailure::Message(message.clone())); + let result = Err(FunctionCallError::RespondToModel(message)); + (event, result) + } + }, Err(ToolError::Rejected(msg)) => { // Normalize common rejection messages for exec tools so tests and // users see a clear, consistent phrase. diff --git a/codex-rs/core/src/tools/handlers/multi_agents/close_agent.rs b/codex-rs/core/src/tools/handlers/multi_agents/close_agent.rs index d905cf4b9426..2ce3dd0c675d 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/close_agent.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/close_agent.rs @@ -1,6 +1,6 @@ use super::*; use crate::tools::handlers::multi_agents_spec::create_close_agent_tool_v1; -use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_tools::ToolSpec; pub(crate) struct Handler; @@ -66,7 +66,9 @@ async fn handle_close_agent( .await { Ok(mut status_rx) => status_rx.borrow_and_update().clone(), - Err(CodexErr::ThreadNotFound(_)) if known_agent => { + Err(err) + if known_agent && matches!(err.details(), CodexErrorDetails::ThreadNotFound(_)) => + { session.services.agent_control.get_status(agent_id).await } Err(err) => { diff --git a/codex-rs/core/src/tools/handlers/multi_agents/wait.rs b/codex-rs/core/src/tools/handlers/multi_agents/wait.rs index 75fe4f0f9fa7..1ccad426eb62 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/wait.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/wait.rs @@ -3,7 +3,7 @@ use crate::agent::status::is_final; use crate::session::session::Session; use crate::tools::handlers::multi_agents_spec::WaitAgentTimeoutOptions; use crate::tools::handlers::multi_agents_spec::create_wait_agent_tool_v1; -use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_tools::ToolSpec; use futures::FutureExt; use futures::StreamExt; @@ -125,7 +125,7 @@ impl Handler { } status_rxs.push((*id, rx)); } - Err(CodexErr::ThreadNotFound(_)) => { + Err(err) if matches!(err.details(), CodexErrorDetails::ThreadNotFound(_)) => { initial_final_statuses.push((*id, AgentStatus::NotFound)); } Err(err) => { diff --git a/codex-rs/core/src/tools/handlers/multi_agents_common.rs b/codex-rs/core/src/tools/handlers/multi_agents_common.rs index 352fc5ba5ac4..9bb7f76f9de0 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_common.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_common.rs @@ -12,6 +12,7 @@ use codex_models_manager::manager::RefreshStrategy; use codex_protocol::AgentPath; use codex_protocol::ThreadId; use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::models::BaseInstructions; use codex_protocol::models::ResponseInputItem; use codex_protocol::openai_models::ModelPreset; @@ -80,27 +81,29 @@ where } pub(crate) fn collab_spawn_error(err: CodexErr) -> FunctionCallError { - match err { - CodexErr::UnsupportedOperation(message) if message == "thread manager dropped" => { + match err.details() { + CodexErrorDetails::UnsupportedOperation(message) if message == "thread manager dropped" => { FunctionCallError::RespondToModel("collab manager unavailable".to_string()) } - CodexErr::UnsupportedOperation(message) => FunctionCallError::RespondToModel(message), - err => FunctionCallError::RespondToModel(format!("collab spawn failed: {err}")), + CodexErrorDetails::UnsupportedOperation(message) => { + FunctionCallError::RespondToModel(message.clone()) + } + _ => FunctionCallError::RespondToModel(format!("collab spawn failed: {err}")), } } pub(crate) fn collab_agent_error(agent_id: ThreadId, err: CodexErr) -> FunctionCallError { - match err { - CodexErr::ThreadNotFound(id) => { + match err.details() { + CodexErrorDetails::ThreadNotFound(id) => { FunctionCallError::RespondToModel(format!("agent with id {id} not found")) } - CodexErr::InternalAgentDied => { + CodexErrorDetails::InternalAgentDied => { FunctionCallError::RespondToModel(format!("agent with id {agent_id} is closed")) } - CodexErr::UnsupportedOperation(_) => { + CodexErrorDetails::UnsupportedOperation(_) => { FunctionCallError::RespondToModel("collab manager unavailable".to_string()) } - err => FunctionCallError::RespondToModel(format!("collab tool failed: {err}")), + _ => FunctionCallError::RespondToModel(format!("collab tool failed: {err}")), } } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/interrupt_agent.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/interrupt_agent.rs index 00eb149ca166..a418dec4a865 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/interrupt_agent.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/interrupt_agent.rs @@ -1,6 +1,6 @@ use super::*; use crate::tools::handlers::multi_agents_spec::create_interrupt_agent_tool_v2; -use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_tools::ToolSpec; pub(crate) struct Handler; @@ -66,7 +66,15 @@ async fn handle_interrupt_agent( .interrupt_agent(agent_id) .await { - Ok(_) | Err(CodexErr::ThreadNotFound(_)) | Err(CodexErr::InternalAgentDied) => Ok(()), + Ok(_) => Ok(()), + Err(err) + if matches!( + err.details(), + CodexErrorDetails::ThreadNotFound(_) | CodexErrorDetails::InternalAgentDied + ) => + { + Ok(()) + } Err(err) => Err(collab_agent_error(agent_id, err)), }; result?; diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 810b0eb9b6a4..b154e1a1408c 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -27,7 +27,7 @@ use crate::tools::sandboxing::default_exec_approval_requirement; use crate::tools::sandboxing::sandbox_override_for_first_attempt; use crate::tools::sandboxing::unsandboxed_execution_allowed; use codex_otel::ToolDecisionSource; -use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::error::SandboxErr; use codex_protocol::exec_output::ExecToolCallOutput; use codex_protocol::protocol::AskForApproval; @@ -298,10 +298,24 @@ impl ToolOrchestrator { deferred_network_approval: first_deferred_network_approval, }) } - Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { - output, - network_policy_decision, - }))) => { + Err(ToolError::Codex(err)) => { + let CodexErrorDetails::Sandbox(SandboxErr::Denied { + output, + network_policy_decision, + }) = err.details() + else { + let err = ToolError::Codex(err); + if let Some(outcome) = sandbox_outcome_from_tool_error(&err) { + otel.sandbox_outcome( + &otel_tn, + otel_ci, + outcome, + initial_duration, + /*escalated_duration*/ None, + ); + } + return Err(err); + }; let network_approval_context = if managed_network_active { network_policy_decision .as_ref() @@ -317,10 +331,7 @@ impl ToolOrchestrator { initial_duration, /*escalated_duration*/ None, ); - return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { - output, - network_policy_decision, - }))); + return Err(ToolError::Codex(err)); } if !tool.escalate_on_failure() { otel.sandbox_outcome( @@ -330,10 +341,7 @@ impl ToolOrchestrator { initial_duration, /*escalated_duration*/ None, ); - return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { - output, - network_policy_decision, - }))); + return Err(ToolError::Codex(err)); } let unsandboxed_allowed = unsandboxed_execution_allowed(&file_system_sandbox_policy); @@ -359,10 +367,7 @@ impl ToolOrchestrator { initial_duration, /*escalated_duration*/ None, ); - return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { - output, - network_policy_decision, - }))); + return Err(ToolError::Codex(err)); } } if !unsandboxed_allowed && network_approval_context.is_none() { @@ -373,10 +378,7 @@ impl ToolOrchestrator { initial_duration, /*escalated_duration*/ None, ); - return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { - output, - network_policy_decision, - }))); + return Err(ToolError::Codex(err)); } let retry_reason = if let Some(network_approval_context) = network_approval_context.as_ref() { @@ -514,10 +516,13 @@ impl ToolOrchestrator { fn sandbox_outcome_from_tool_error(err: &ToolError) -> Option<&'static str> { match err { - ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { .. })) => Some("denied"), - ToolError::Codex(CodexErr::Sandbox(SandboxErr::Timeout { .. })) => Some("timed_out"), - ToolError::Codex(CodexErr::Sandbox(SandboxErr::Signal(_))) => Some("signal"), - ToolError::Rejected(_) | ToolError::Codex(_) => None, + ToolError::Codex(err) => match err.details() { + CodexErrorDetails::Sandbox(SandboxErr::Denied { .. }) => Some("denied"), + CodexErrorDetails::Sandbox(SandboxErr::Timeout { .. }) => Some("timed_out"), + CodexErrorDetails::Sandbox(SandboxErr::Signal(_)) => Some("signal"), + _ => None, + }, + ToolError::Rejected(_) => None, } } diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 96c324adc011..a8c3f7b3aa0d 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -61,6 +61,7 @@ use crate::unified_exec::process::UnifiedExecProcess; use codex_network_proxy::NetworkProxy; use codex_protocol::config_types::ShellEnvironmentPolicy; use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::error::SandboxErr; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ExecCommandSource; @@ -1203,16 +1204,19 @@ impl UnifiedExecProcessManager { .await .map(|result| (result.output, result.deferred_network_approval)) .map_err(|err| match err { - ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output, .. })) => { - let output = *output; - let message = if output.aggregated_output.text.is_empty() { - let exit_code = output.exit_code; - format!("Process exited with code {exit_code}") - } else { - output.aggregated_output.text.clone() - }; - UnifiedExecError::sandbox_denied(message, output) - } + ToolError::Codex(err) => match err.details() { + CodexErrorDetails::Sandbox(SandboxErr::Denied { output, .. }) => { + let output = output.as_ref().clone(); + let message = if output.aggregated_output.text.is_empty() { + let exit_code = output.exit_code; + format!("Process exited with code {exit_code}") + } else { + output.aggregated_output.text.clone() + }; + UnifiedExecError::sandbox_denied(message, output) + } + _ => UnifiedExecError::create_process(format!("{err:?}")), + }, other => UnifiedExecError::create_process(format!("{other:?}")), }) } diff --git a/codex-rs/linux-sandbox/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index cd82781a1442..5d4ed8bd01a0 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -7,7 +7,7 @@ use codex_core::exec_env::create_env; use codex_core::sandboxing::SandboxPermissions; use codex_protocol::config_types::ShellEnvironmentPolicy; use codex_protocol::config_types::WindowsSandboxLevel; -use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use codex_protocol::error::Result; use codex_protocol::error::SandboxErr; use codex_protocol::models::PermissionProfile; @@ -218,13 +218,15 @@ async fn should_skip_bwrap_tests() -> bool { .await { Ok(output) => is_bwrap_unavailable_output(&output), - Err(CodexErr::Sandbox(SandboxErr::Denied { output, .. })) => { - is_bwrap_unavailable_output(&output) - } - // Probe timeouts are not actionable for the bwrap-specific assertions below; - // skip rather than fail the whole suite. - Err(CodexErr::Sandbox(SandboxErr::Timeout { .. })) => true, - Err(err) => panic!("bwrap availability probe failed unexpectedly: {err:?}"), + Err(err) => match err.details() { + CodexErrorDetails::Sandbox(SandboxErr::Denied { output, .. }) => { + is_bwrap_unavailable_output(output) + } + // Probe timeouts are not actionable for the bwrap-specific assertions below; + // skip rather than fail the whole suite. + CodexErrorDetails::Sandbox(SandboxErr::Timeout { .. }) => true, + details => panic!("bwrap availability probe failed unexpectedly: {details:?}"), + }, } } @@ -237,8 +239,12 @@ fn expect_denied( assert_ne!(output.exit_code, 0, "{context}: expected nonzero exit code"); output } - Err(CodexErr::Sandbox(SandboxErr::Denied { output, .. })) => *output, - Err(err) => panic!("{context}: {err:?}"), + Err(err) => match err.details() { + CodexErrorDetails::Sandbox(SandboxErr::Denied { output, .. }) => { + output.as_ref().clone() + } + details => panic!("{context}: {details:?}"), + }, } } @@ -456,10 +462,12 @@ async fn assert_network_blocked(cmd: &[&str]) { let output = match result { Ok(output) => output, - Err(CodexErr::Sandbox(SandboxErr::Denied { output, .. })) => *output, - _ => { - panic!("expected sandbox denied error, got: {result:?}"); - } + Err(err) => match err.details() { + CodexErrorDetails::Sandbox(SandboxErr::Denied { output, .. }) => { + output.as_ref().clone() + } + details => panic!("expected sandbox denied error, got: {details:?}"), + }, }; dbg!(&output.stderr.text); @@ -612,8 +620,13 @@ async fn sandbox_reports_codex_symlink_build_failure_without_panicking() { ) .await { - Err(CodexErr::Sandbox(SandboxErr::Denied { output, .. })) => *output, - result => panic!(".codex symlink build failure should deny: {result:?}"), + Err(err) => match err.details() { + CodexErrorDetails::Sandbox(SandboxErr::Denied { output, .. }) => { + output.as_ref().clone() + } + details => panic!(".codex symlink build failure should deny: {details:?}"), + }, + Ok(output) => panic!(".codex symlink build failure should deny: {output:?}"), }; assert_eq!(output.exit_code, 1); diff --git a/codex-rs/model-provider/src/amazon_bedrock/error.rs b/codex-rs/model-provider/src/amazon_bedrock/error.rs index 1e489352b983..51428be7f5a9 100644 --- a/codex-rs/model-provider/src/amazon_bedrock/error.rs +++ b/codex-rs/model-provider/src/amazon_bedrock/error.rs @@ -1,5 +1,6 @@ use codex_api::ApiError; use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use http::StatusCode; pub(super) const BEDROCK_EXPIRED_SIGNATURE_MESSAGE: &str = concat!( @@ -9,12 +10,18 @@ pub(super) const BEDROCK_EXPIRED_SIGNATURE_MESSAGE: &str = concat!( ); pub(super) fn map_api_error(error: ApiError) -> CodexErr { - let mut error = codex_api::map_api_error(error); - if let CodexErr::UnexpectedStatus(response) = &mut error + let error = codex_api::map_api_error(error); + if let CodexErrorDetails::UnexpectedStatus(response) = error.details() && response.status == StatusCode::UNAUTHORIZED && response.body.contains("Signature expired:") { + let mut response = response.clone(); response.user_message = Some(BEDROCK_EXPIRED_SIGNATURE_MESSAGE.to_string()); + let mapped_error = CodexErr::new(CodexErrorDetails::UnexpectedStatus(response)); + return match error.retry_delay() { + Some(retry_delay) => mapped_error.with_retry_delay(retry_delay), + None => mapped_error, + }; } error } diff --git a/codex-rs/model-provider/src/amazon_bedrock/error_tests.rs b/codex-rs/model-provider/src/amazon_bedrock/error_tests.rs index 50291c3eec5a..053697907a50 100644 --- a/codex-rs/model-provider/src/amazon_bedrock/error_tests.rs +++ b/codex-rs/model-provider/src/amazon_bedrock/error_tests.rs @@ -1,6 +1,6 @@ use codex_api::ApiError; use codex_api::TransportError; -use codex_protocol::error::CodexErr; +use codex_protocol::error::CodexErrorDetails; use http::HeaderMap; use http::HeaderValue; use http::StatusCode; @@ -29,7 +29,7 @@ fn expired_signature_has_actionable_guidance() { "Signature expired: 20260609T133205Z is now earlier than 20260614T062525Z", )); - let CodexErr::UnexpectedStatus(response) = &error else { + let CodexErrorDetails::UnexpectedStatus(response) = error.details() else { panic!("expected unexpected status error, got {error:?}"); }; assert_eq!( @@ -51,7 +51,7 @@ fn other_unauthorized_errors_remain_generic() { "The security token included in the request is invalid", )); - let CodexErr::UnexpectedStatus(response) = &error else { + let CodexErrorDetails::UnexpectedStatus(response) = error.details() else { panic!("expected unexpected status error, got {error:?}"); }; assert_eq!(response.user_message, None); @@ -70,7 +70,7 @@ fn signature_errors_with_other_statuses_remain_generic() { "Signature expired: old is now earlier than new", )); - let CodexErr::UnexpectedStatus(response) = &error else { + let CodexErrorDetails::UnexpectedStatus(response) = error.details() else { panic!("expected unexpected status error, got {error:?}"); }; assert_eq!(response.user_message, None); diff --git a/codex-rs/model-provider/src/auth.rs b/codex-rs/model-provider/src/auth.rs index f8bbea38d890..85d01ae216f8 100644 --- a/codex-rs/model-provider/src/auth.rs +++ b/codex-rs/model-provider/src/auth.rs @@ -323,6 +323,7 @@ mod tests { use codex_model_provider_info::WireApi; use codex_model_provider_info::create_oss_provider_with_base_url; use codex_protocol::account::PlanType; + use codex_protocol::error::CodexErrorDetails; use http::header::AUTHORIZATION; use pretty_assertions::assert_eq; use serde_json::json; @@ -471,10 +472,12 @@ mod tests { }); match resolve_provider_auth(Some(&auth), &provider) { - Err(CodexErr::UnsupportedOperation(message)) => { - assert_eq!(message, BEDROCK_API_KEY_UNSUPPORTED_MESSAGE); - } - Err(err) => panic!("unexpected auth error: {err:?}"), + Err(err) => match err.details() { + CodexErrorDetails::UnsupportedOperation(message) => { + assert_eq!(message, BEDROCK_API_KEY_UNSUPPORTED_MESSAGE); + } + details => panic!("unexpected auth error: {details:?}"), + }, Ok(_) => panic!("Bedrock API key auth should be rejected"), } } diff --git a/codex-rs/protocol/src/error.rs b/codex-rs/protocol/src/error.rs index a96ebd07d697..0cf660b096e4 100644 --- a/codex-rs/protocol/src/error.rs +++ b/codex-rs/protocol/src/error.rs @@ -19,8 +19,10 @@ use codex_utils_string::truncate_middle_chars; use codex_utils_string::truncate_middle_with_token_budget; use reqwest::StatusCode; use serde_json; +use std::fmt; use std::io; use std::time::Duration; +use strum_macros::EnumDiscriminants; use thiserror::Error; use tokio::task::JoinError; @@ -64,8 +66,18 @@ pub enum SandboxErr { LandlockRestrict, } -#[derive(Error, Debug)] -pub enum CodexErr { +pub struct CodexErr { + details: CodexErrorDetails, + retry_delay: Option, +} + +/// The semantic category and diagnostic payload for a [`CodexErr`]. +#[derive(Error, Debug, EnumDiscriminants)] +#[strum_discriminants(name(CodexErrKind))] +#[strum_discriminants(derive(serde::Serialize))] +#[strum_discriminants(serde(rename_all = "snake_case"))] +#[strum_discriminants(doc = "The payload-free semantic category used for analytics.")] +pub enum CodexErrorDetails { #[error("turn aborted. Something went wrong? Hit `/feedback` to report the issue.")] TurnAborted, @@ -76,10 +88,8 @@ pub enum CodexErr { /// handshake has succeeded but **before** it finished emitting `response.completed`. /// /// The Session loop treats this as a transient error and will automatically retry the turn. - /// - /// Optionally includes the requested delay before retrying the turn. #[error("stream disconnected before completion: {0}")] - Stream(String, Option), + Stream(String), #[error( "Codex ran out of room in the model's context window. Start a new thread or clear earlier history before retrying." )] @@ -166,87 +176,270 @@ pub enum CodexErr { EnvVar(EnvVarError), } +impl From<&CodexErr> for CodexErrKind { + fn from(error: &CodexErr) -> Self { + error.details().into() + } +} + +impl fmt::Debug for CodexErr { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match &self.details { + CodexErrorDetails::Stream(message) => formatter + .debug_tuple("Stream") + .field(message) + .field(&self.retry_delay) + .finish(), + details => fmt::Debug::fmt(details, formatter), + } + } +} + +impl fmt::Display for CodexErr { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&self.details, formatter) + } +} + +impl std::error::Error for CodexErr { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + self.details.source() + } +} + +impl From for CodexErr { + fn from(details: CodexErrorDetails) -> Self { + Self { + details, + retry_delay: None, + } + } +} + impl From for CodexErr { + fn from(error: CancelErr) -> Self { + CodexErrorDetails::from(error).into() + } +} + +impl From for CodexErr { + fn from(error: SandboxErr) -> Self { + CodexErrorDetails::from(error).into() + } +} + +impl From for CodexErr { + fn from(error: io::Error) -> Self { + CodexErrorDetails::from(error).into() + } +} + +impl From for CodexErr { + fn from(error: serde_json::Error) -> Self { + CodexErrorDetails::from(error).into() + } +} + +impl From for CodexErr { + fn from(error: JoinError) -> Self { + CodexErrorDetails::from(error).into() + } +} + +#[cfg(target_os = "linux")] +impl From for CodexErr { + fn from(error: landlock::RulesetError) -> Self { + CodexErrorDetails::from(error).into() + } +} + +#[cfg(target_os = "linux")] +impl From for CodexErr { + fn from(error: landlock::PathFdError) -> Self { + CodexErrorDetails::from(error).into() + } +} + +impl From for CodexErrorDetails { fn from(_: CancelErr) -> Self { - CodexErr::TurnAborted + CodexErrorDetails::TurnAborted } } +// TODO(anp): Remove this compatibility macro once callers construct +// `CodexErrorDetails` directly. +macro_rules! codex_err_unit_constructors { + ($($variant:ident),* $(,)?) => { + $( + #[doc(hidden)] + #[allow(non_upper_case_globals)] + pub const $variant: Self = Self { + details: CodexErrorDetails::$variant, + retry_delay: None, + }; + )* + }; +} + +// TODO(anp): Remove this compatibility macro once callers construct +// `CodexErrorDetails` directly. +macro_rules! codex_err_tuple_constructors { + ($($(#[$attr:meta])* $variant:ident($value:ident: $value_type:ty)),* $(,)?) => { + $( + $(#[$attr])* + #[doc(hidden)] + #[allow(non_snake_case)] + pub fn $variant($value: $value_type) -> Self { + CodexErrorDetails::$variant($value).into() + } + )* + }; +} + impl CodexErr { + codex_err_unit_constructors!( + TurnAborted, + SessionBudgetExceeded, + ContextWindowExceeded, + SessionConfiguredNotFirstEvent, + Timeout, + RequestTimeout, + Spawn, + Interrupted, + ServerOverloaded, + QuotaExceeded, + UsageNotIncluded, + InternalServerError, + InternalAgentDied, + LandlockSandboxExecutableNotProvided, + ); + + codex_err_tuple_constructors!( + Stream(message: String), + ThreadNotFound(thread_id: ThreadId), + UnexpectedStatus(error: UnexpectedResponseError), + InvalidRequest(message: String), + UsageLimitReached(error: UsageLimitReachedError), + ResponseStreamFailed(error: ResponseStreamFailed), + ConnectionFailed(error: ConnectionFailedError), + RetryLimit(error: RetryLimitReachedError), + Sandbox(error: SandboxErr), + UnsupportedOperation(message: String), + RefreshTokenFailed(error: RefreshTokenFailedError), + Fatal(message: String), + Io(error: io::Error), + Json(error: serde_json::Error), + #[cfg(target_os = "linux")] + LandlockRuleset(error: landlock::RulesetError), + #[cfg(target_os = "linux")] + LandlockPathFd(error: landlock::PathFdError), + TokioJoin(error: JoinError), + EnvVar(error: EnvVarError), + ); + + // TODO(anp): Remove this compatibility constructor once callers construct + // `CodexErrorDetails` directly. + #[doc(hidden)] + #[allow(non_snake_case)] + pub fn InvalidImageRequest() -> Self { + CodexErrorDetails::InvalidImageRequest().into() + } + + /// Creates an error with no server-provided retry delay. + pub fn new(details: CodexErrorDetails) -> Self { + details.into() + } + + /// Returns the semantic failure and its diagnostic payload. + pub fn details(&self) -> &CodexErrorDetails { + &self.details + } + pub fn is_retryable(&self) -> bool { - match self { - CodexErr::TurnAborted - | CodexErr::SessionBudgetExceeded - | CodexErr::Interrupted - | CodexErr::EnvVar(_) - | CodexErr::Fatal(_) - | CodexErr::UsageNotIncluded - | CodexErr::QuotaExceeded - | CodexErr::InvalidImageRequest() - | CodexErr::InvalidRequest(_) - | CodexErr::RefreshTokenFailed(_) - | CodexErr::UnsupportedOperation(_) - | CodexErr::Sandbox(_) - | CodexErr::LandlockSandboxExecutableNotProvided - | CodexErr::RetryLimit(_) - | CodexErr::ContextWindowExceeded - | CodexErr::ThreadNotFound(_) - | CodexErr::AgentLimitReached { .. } - | CodexErr::Spawn - | CodexErr::SessionConfiguredNotFirstEvent - | CodexErr::UsageLimitReached(_) - | CodexErr::ServerOverloaded - | CodexErr::CyberPolicy { .. } => false, - CodexErr::Stream(..) - | CodexErr::Timeout - | CodexErr::RequestTimeout - | CodexErr::UnexpectedStatus(_) - | CodexErr::ResponseStreamFailed(_) - | CodexErr::ConnectionFailed(_) - | CodexErr::InternalServerError - | CodexErr::InternalAgentDied - | CodexErr::Io(_) - | CodexErr::Json(_) - | CodexErr::TokioJoin(_) => true, + match self.details() { + CodexErrorDetails::TurnAborted + | CodexErrorDetails::SessionBudgetExceeded + | CodexErrorDetails::Interrupted + | CodexErrorDetails::EnvVar(_) + | CodexErrorDetails::Fatal(_) + | CodexErrorDetails::UsageNotIncluded + | CodexErrorDetails::QuotaExceeded + | CodexErrorDetails::InvalidImageRequest() + | CodexErrorDetails::InvalidRequest(_) + | CodexErrorDetails::RefreshTokenFailed(_) + | CodexErrorDetails::UnsupportedOperation(_) + | CodexErrorDetails::Sandbox(_) + | CodexErrorDetails::LandlockSandboxExecutableNotProvided + | CodexErrorDetails::RetryLimit(_) + | CodexErrorDetails::ContextWindowExceeded + | CodexErrorDetails::ThreadNotFound(_) + | CodexErrorDetails::AgentLimitReached { .. } + | CodexErrorDetails::Spawn + | CodexErrorDetails::SessionConfiguredNotFirstEvent + | CodexErrorDetails::UsageLimitReached(_) + | CodexErrorDetails::ServerOverloaded + | CodexErrorDetails::CyberPolicy { .. } => false, + CodexErrorDetails::Stream(..) + | CodexErrorDetails::Timeout + | CodexErrorDetails::RequestTimeout + | CodexErrorDetails::UnexpectedStatus(_) + | CodexErrorDetails::ResponseStreamFailed(_) + | CodexErrorDetails::ConnectionFailed(_) + | CodexErrorDetails::InternalServerError + | CodexErrorDetails::InternalAgentDied + | CodexErrorDetails::Io(_) + | CodexErrorDetails::Json(_) + | CodexErrorDetails::TokioJoin(_) => true, #[cfg(target_os = "linux")] - CodexErr::LandlockRuleset(_) | CodexErr::LandlockPathFd(_) => false, + CodexErrorDetails::LandlockRuleset(_) | CodexErrorDetails::LandlockPathFd(_) => false, } } + pub fn retry_delay(&self) -> Option { + self.retry_delay + } + + pub fn with_retry_delay(mut self, retry_delay: Duration) -> Self { + self.retry_delay = Some(retry_delay); + self + } + /// Minimal shim so that existing `e.downcast_ref::()` checks continue to compile /// after replacing `anyhow::Error` in the return signature. This mirrors the behavior of - /// `anyhow::Error::downcast_ref` but works directly on our concrete enum. + /// `anyhow::Error::downcast_ref` but works directly on our concrete error type. pub fn downcast_ref(&self) -> Option<&T> { (self as &dyn std::any::Any).downcast_ref::() } /// Translate core error to client-facing protocol error. pub fn to_codex_protocol_error(&self) -> CodexErrorInfo { - match self { - CodexErr::ContextWindowExceeded => CodexErrorInfo::ContextWindowExceeded, - CodexErr::SessionBudgetExceeded => CodexErrorInfo::SessionBudgetExceeded, - CodexErr::UsageLimitReached(_) - | CodexErr::QuotaExceeded - | CodexErr::UsageNotIncluded => CodexErrorInfo::UsageLimitExceeded, - CodexErr::ServerOverloaded => CodexErrorInfo::ServerOverloaded, - CodexErr::CyberPolicy { .. } => CodexErrorInfo::CyberPolicy, - CodexErr::RetryLimit(_) => CodexErrorInfo::ResponseTooManyFailedAttempts { - http_status_code: self.http_status_code_value(), - }, - CodexErr::ConnectionFailed(_) => CodexErrorInfo::HttpConnectionFailed { + match &self.details { + CodexErrorDetails::ContextWindowExceeded => CodexErrorInfo::ContextWindowExceeded, + CodexErrorDetails::SessionBudgetExceeded => CodexErrorInfo::SessionBudgetExceeded, + CodexErrorDetails::UsageLimitReached(_) + | CodexErrorDetails::QuotaExceeded + | CodexErrorDetails::UsageNotIncluded => CodexErrorInfo::UsageLimitExceeded, + CodexErrorDetails::ServerOverloaded => CodexErrorInfo::ServerOverloaded, + CodexErrorDetails::CyberPolicy { .. } => CodexErrorInfo::CyberPolicy, + CodexErrorDetails::RetryLimit(_) => CodexErrorInfo::ResponseTooManyFailedAttempts { http_status_code: self.http_status_code_value(), }, - CodexErr::ResponseStreamFailed(_) => CodexErrorInfo::ResponseStreamConnectionFailed { + CodexErrorDetails::ConnectionFailed(_) => CodexErrorInfo::HttpConnectionFailed { http_status_code: self.http_status_code_value(), }, - CodexErr::RefreshTokenFailed(_) => CodexErrorInfo::Unauthorized, - CodexErr::SessionConfiguredNotFirstEvent - | CodexErr::InternalServerError - | CodexErr::InternalAgentDied => CodexErrorInfo::InternalServerError, - CodexErr::UnsupportedOperation(_) - | CodexErr::ThreadNotFound(_) - | CodexErr::AgentLimitReached { .. } => CodexErrorInfo::BadRequest, - CodexErr::Sandbox(_) => CodexErrorInfo::SandboxError, + CodexErrorDetails::ResponseStreamFailed(_) => { + CodexErrorInfo::ResponseStreamConnectionFailed { + http_status_code: self.http_status_code_value(), + } + } + CodexErrorDetails::RefreshTokenFailed(_) => CodexErrorInfo::Unauthorized, + CodexErrorDetails::SessionConfiguredNotFirstEvent + | CodexErrorDetails::InternalServerError + | CodexErrorDetails::InternalAgentDied => CodexErrorInfo::InternalServerError, + CodexErrorDetails::UnsupportedOperation(_) + | CodexErrorDetails::ThreadNotFound(_) + | CodexErrorDetails::AgentLimitReached { .. } => CodexErrorInfo::BadRequest, + CodexErrorDetails::Sandbox(_) => CodexErrorInfo::SandboxError, _ => CodexErrorInfo::Other, } } @@ -264,11 +457,11 @@ impl CodexErr { } pub fn http_status_code_value(&self) -> Option { - let http_status_code = match self { - CodexErr::RetryLimit(err) => Some(err.status), - CodexErr::UnexpectedStatus(err) => Some(err.status), - CodexErr::ConnectionFailed(err) => err.source.status(), - CodexErr::ResponseStreamFailed(err) => err.source.status(), + let http_status_code = match &self.details { + CodexErrorDetails::RetryLimit(err) => Some(err.status), + CodexErrorDetails::UnexpectedStatus(err) => Some(err.status), + CodexErrorDetails::ConnectionFailed(err) => err.source.status(), + CodexErrorDetails::ResponseStreamFailed(err) => err.source.status(), _ => None, }; http_status_code.as_ref().map(StatusCode::as_u16) @@ -306,7 +499,7 @@ impl std::fmt::Display for ResponseStreamFailed { } } -#[derive(Debug)] +#[derive(Clone, Debug)] pub struct UnexpectedResponseError { pub status: StatusCode, pub body: String, @@ -606,8 +799,8 @@ impl std::fmt::Display for EnvVarError { } pub fn get_error_message_ui(e: &CodexErr) -> String { - let message = match e { - CodexErr::Sandbox(SandboxErr::Denied { output, .. }) => { + let message = match e.details() { + CodexErrorDetails::Sandbox(SandboxErr::Denied { output, .. }) => { let aggregated = output.aggregated_output.text.trim(); if !aggregated.is_empty() { output.aggregated_output.text.clone() @@ -626,7 +819,7 @@ pub fn get_error_message_ui(e: &CodexErr) -> String { } } // Timeouts are not sandbox errors from a UX perspective; present them plainly. - CodexErr::Sandbox(SandboxErr::Timeout { output }) => { + CodexErrorDetails::Sandbox(SandboxErr::Timeout { output }) => { format!( "error: command timed out after {} ms", output.duration.as_millis() diff --git a/codex-rs/protocol/src/error_tests.rs b/codex-rs/protocol/src/error_tests.rs index b6fd84cbbbfa..22ccf0a327eb 100644 --- a/codex-rs/protocol/src/error_tests.rs +++ b/codex-rs/protocol/src/error_tests.rs @@ -11,6 +11,64 @@ use reqwest::Response; use reqwest::ResponseBuilderExt; use reqwest::StatusCode; use reqwest::Url; +use std::time::Duration; + +#[test] +fn codex_err_debug_preserves_legacy_shape() { + let actual = [ + CodexErr::Timeout, + CodexErr::Stream("disconnected".to_string()), + CodexErr::Stream("retry later".to_string()).with_retry_delay(Duration::from_secs(2)), + CodexErr::InternalServerError.with_retry_delay(Duration::from_secs(3)), + ] + .map(|err| format!("{err:?}")); + + assert_eq!( + actual, + [ + "Timeout".to_string(), + "Stream(\"disconnected\", None)".to_string(), + "Stream(\"retry later\", Some(2s))".to_string(), + "InternalServerError".to_string(), + ] + ); +} + +#[test] +fn retryability_preserves_error_details_distinctions() { + let errors = [ + (CodexErr::ServerOverloaded, false), + ( + CodexErr::RetryLimit(RetryLimitReachedError { + status: StatusCode::TOO_MANY_REQUESTS, + request_id: None, + }), + false, + ), + ( + CodexErr::UnexpectedStatus(UnexpectedResponseError { + status: StatusCode::TOO_MANY_REQUESTS, + body: String::new(), + user_message: None, + url: None, + cf_ray: None, + request_id: None, + identity_authorization_error: None, + identity_error_code: None, + }), + true, + ), + (CodexErr::InternalServerError, true), + ]; + + for (err, expected) in errors { + assert_eq!( + err.is_retryable(), + expected, + "unexpected retryability for {err:?}" + ); + } +} fn rate_limit_snapshot() -> RateLimitSnapshot { let primary_reset_at = Utc