mirror of
https://github.com/openai/codex.git
synced 2026-11-10 21:00:47 +00:00
Stop retrying explicit invalid-workspace errors (#60475)
## Why HTTP 403 responses with `invalid_workspace_selected` were treated as unexpected status errors, allowing retries and compaction fallback to another model after the server rejected the selected workspace. ## What changed Recognize the code in `detail.code` or `error.code`, falling back to the `x-error-json` header when the body has no code. Classify these 403 responses as `InvalidWorkspaceSelected` and disable retries and compaction model fallback while preserving the response details and HTTP status. Other 403 responses retain their existing retry behavior. ## Testing Add coverage for body and header error codes, code precedence, HTTP and WebSocket retry behavior, and local and remote compaction during a model downshift. GitOrigin-RevId: f44a3bee5ef06e601909839868ab99cea6dd7d03
This commit is contained in:
@@ -261,7 +261,7 @@ fn map_api_error_details(err: ApiError) -> CodexErr {
|
||||
request_id: extract_request_tracking_id(headers.as_ref()),
|
||||
})
|
||||
} else {
|
||||
CodexErr::UnexpectedStatus(UnexpectedResponseError {
|
||||
let response = UnexpectedResponseError {
|
||||
status,
|
||||
user_message: api_error_user_message(status, &body_text),
|
||||
body: body_text,
|
||||
@@ -273,7 +273,19 @@ fn map_api_error_details(err: ApiError) -> CodexErr {
|
||||
X_OPENAI_AUTHORIZATION_ERROR_HEADER,
|
||||
),
|
||||
identity_error_code: extract_x_error_json_code(headers.as_ref()),
|
||||
})
|
||||
};
|
||||
let body_code = parsed_body.as_ref().and_then(|body| {
|
||||
["/detail/code", "/error/code"]
|
||||
.into_iter()
|
||||
.find_map(|path| body.pointer(path)?.as_str())
|
||||
});
|
||||
let code = body_code.or(response.identity_error_code.as_deref());
|
||||
match (status, code) {
|
||||
(http::StatusCode::FORBIDDEN, Some("invalid_workspace_selected")) => {
|
||||
CodexErrorDetails::InvalidWorkspaceSelected(response).into()
|
||||
}
|
||||
_ => CodexErr::UnexpectedStatus(response),
|
||||
}
|
||||
}
|
||||
}
|
||||
TransportError::RetryLimit => CodexErr::RetryLimit(RetryLimitReachedError {
|
||||
|
||||
@@ -804,6 +804,59 @@ fn map_api_error_ignores_unparseable_rate_limit_reached_type_headers() {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn only_an_explicit_workspace_403_stops_retries() {
|
||||
use http::StatusCode;
|
||||
let provider = r#"{"error":{"code":"invalid_workspace_selected"}}"#;
|
||||
let detail = r#"{"detail":{"code":"invalid_workspace_selected"}}"#;
|
||||
let other = r#"{"detail":{"code":"route_rate_limited"}}"#;
|
||||
let workspace_header =
|
||||
http::HeaderValue::from_str(&base64::engine::general_purpose::STANDARD.encode(provider))
|
||||
.unwrap();
|
||||
for (status, body, with_header, terminal) in [
|
||||
(StatusCode::FORBIDDEN, detail, false, true),
|
||||
(StatusCode::FORBIDDEN, provider, false, true),
|
||||
(StatusCode::FORBIDDEN, "", true, true),
|
||||
(StatusCode::FORBIDDEN, "", false, false),
|
||||
(
|
||||
StatusCode::FORBIDDEN,
|
||||
"invalid_workspace_selected",
|
||||
false,
|
||||
false,
|
||||
),
|
||||
(StatusCode::FORBIDDEN, other, true, false),
|
||||
(StatusCode::UNAUTHORIZED, detail, true, false),
|
||||
] {
|
||||
let mut headers = HeaderMap::new();
|
||||
headers.insert(REQUEST_ID_HEADER, http::HeaderValue::from_static("req-403"));
|
||||
if with_header {
|
||||
headers.insert(X_ERROR_JSON_HEADER, workspace_header.clone());
|
||||
}
|
||||
let err = map_api_error(ApiError::Transport(TransportError::Http {
|
||||
status,
|
||||
headers: Some(headers),
|
||||
body: Some(body.to_owned()),
|
||||
url: None,
|
||||
retry_after: None,
|
||||
}));
|
||||
assert_eq!(
|
||||
matches!(
|
||||
err.details(),
|
||||
CodexErrorDetails::InvalidWorkspaceSelected(_)
|
||||
),
|
||||
terminal,
|
||||
"{err}"
|
||||
);
|
||||
assert_eq!(
|
||||
err.retry_delay(/*retry_count*/ 1).is_none(),
|
||||
terminal,
|
||||
"{err}"
|
||||
);
|
||||
assert_eq!(err.http_status_code_value(), Some(status.as_u16()));
|
||||
assert!(err.to_string().contains("req-403"));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn map_api_error_extracts_identity_auth_details_from_headers() {
|
||||
let mut headers = HeaderMap::new();
|
||||
|
||||
@@ -17,6 +17,7 @@ pub(crate) fn should_retry_with_current_model(
|
||||
CodexErrorDetails::TurnAborted
|
||||
| CodexErrorDetails::Interrupted
|
||||
| CodexErrorDetails::SessionBudgetExceeded
|
||||
| CodexErrorDetails::InvalidWorkspaceSelected(_)
|
||||
| CodexErrorDetails::ResponseIncomplete(_)
|
||||
) || (previous.model_info().slug == current.model_info().slug
|
||||
&& previous.cyber_access_program == current.cyber_access_program)
|
||||
|
||||
@@ -1140,6 +1140,63 @@ async fn reasoning_effort_override_remote_v2_compaction_resets_pinned_effort(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case::test_case(non_openai_model_provider; "local")]
|
||||
#[test_case::test_case(openai_model_provider; "remote")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn workspace_denial_stops_compaction_on_model_downshift(
|
||||
provider: fn(&MockServer) -> ModelProviderInfo,
|
||||
) -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
let server = start_mock_server().await;
|
||||
let previous = "gpt-5.4";
|
||||
let selected = "gpt-5.2";
|
||||
let model_provider = provider(&server);
|
||||
let test = test_codex()
|
||||
.with_model(previous)
|
||||
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
|
||||
.with_config(move |config| {
|
||||
config.model_provider = model_provider;
|
||||
config.model_provider.stream_max_retries = Some(2);
|
||||
config.model_catalog = Some(ModelsResponse {
|
||||
models: vec![
|
||||
model_info_with_context_window(previous, /*context_window*/ 273_000),
|
||||
model_info_with_context_window(selected, /*context_window*/ 125_000),
|
||||
],
|
||||
});
|
||||
set_test_compact_prompt(config);
|
||||
})
|
||||
.build_with_auto_env(&server)
|
||||
.await?;
|
||||
mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_assistant_message("message", "First reply"),
|
||||
ev_completed_with_tokens("first", /*total_tokens*/ 120_000),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
test.submit_turn("first turn").await?;
|
||||
|
||||
let denied = responses::mount_response_handler(&server, |_| {
|
||||
wiremock::ResponseTemplate::new(403)
|
||||
.set_body_json(json!({ "detail": { "code": "invalid_workspace_selected" } }))
|
||||
})
|
||||
.await;
|
||||
test.codex
|
||||
.start_or_steer_turn(disabled_permission_user_turn(
|
||||
"continue on the smaller model",
|
||||
test.cwd.path().to_path_buf(),
|
||||
selected.to_string(),
|
||||
))
|
||||
.await?;
|
||||
wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::TurnComplete(_))
|
||||
})
|
||||
.await;
|
||||
assert_eq!(denied.single_request().body_json()["model"], previous);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case::test_case(json!({"input_tokens": 120_000, "output_tokens": 30_000, "total_tokens": 150_000}); "reported_usage")]
|
||||
#[test_case::test_case(Value::Null; "missing_usage")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
|
||||
@@ -127,6 +127,49 @@ async fn websocket_fallback_switches_to_http_after_retries_exhausted() -> Result
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case::test_case(true, Some("invalid_workspace_selected"), 3, 0; "workspace denial on websocket")]
|
||||
#[test_case::test_case(false, Some("invalid_workspace_selected"), 0, 2; "workspace denial on http")]
|
||||
#[test_case::test_case(true, None, 3, 4; "bare websocket and http denials")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn forbidden_responses_retry_only_without_workspace_denial(
|
||||
supports_websockets: bool,
|
||||
code: Option<&str>,
|
||||
expected_upgrades: u64,
|
||||
expected_posts: u64,
|
||||
) -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let mut denial = ResponseTemplate::new(403);
|
||||
if let Some(code) = code {
|
||||
denial = denial.set_body_json(serde_json::json!({"detail": {"code": code}}));
|
||||
}
|
||||
for (verb, count) in [("GET", expected_upgrades), ("POST", expected_posts)] {
|
||||
Mock::given(method(verb))
|
||||
.and(path_regex(".*/responses$"))
|
||||
.respond_with(denial.clone())
|
||||
.expect(count)
|
||||
.mount(&server)
|
||||
.await;
|
||||
}
|
||||
|
||||
let test = test_codex()
|
||||
.with_config(move |config| {
|
||||
config.model_provider.supports_websockets = supports_websockets;
|
||||
config.model_provider.stream_max_retries = Some(1);
|
||||
config.model_provider.request_max_retries = Some(1);
|
||||
})
|
||||
.build_with_auto_env(&server)
|
||||
.await?;
|
||||
|
||||
// A WebSocket-enabled session also attempts a startup prewarm. A typed denial
|
||||
// should leave the next turn eligible for its own WebSocket attempt.
|
||||
test.submit_turn("first").await?;
|
||||
test.submit_turn("second").await?;
|
||||
server.verify().await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn websocket_fallback_hides_first_websocket_retry_stream_error() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -148,6 +148,9 @@ pub enum CodexErrorDetails {
|
||||
/// surface a polite FunctionCallOutput back to the model instead of crashing the CLI.
|
||||
#[error("interrupted (Ctrl-C). Something went wrong? Hit `/feedback` to report the issue.")]
|
||||
Interrupted,
|
||||
/// The server rejected the selected workspace.
|
||||
#[error("{0}")]
|
||||
InvalidWorkspaceSelected(UnexpectedResponseError),
|
||||
/// Unexpected HTTP status code.
|
||||
#[error("{0}")]
|
||||
UnexpectedStatus(UnexpectedResponseError),
|
||||
@@ -466,6 +469,7 @@ impl CodexErr {
|
||||
| CodexErrorDetails::FlexUnavailable
|
||||
| CodexErrorDetails::CyberPolicy { .. }
|
||||
| CodexErrorDetails::BioPolicy { .. }
|
||||
| CodexErrorDetails::InvalidWorkspaceSelected(_)
|
||||
| CodexErrorDetails::MisalignmentPolicyViolation { .. } => None,
|
||||
CodexErrorDetails::ServerOverloaded
|
||||
| CodexErrorDetails::PolicyCapacityUnavailable
|
||||
@@ -539,11 +543,11 @@ impl CodexErr {
|
||||
CodexErrorDetails::RetryLimit(_) => CodexErrorInfo::ResponseTooManyFailedAttempts {
|
||||
http_status_code: self.http_status_code_value(),
|
||||
},
|
||||
CodexErrorDetails::ConnectionFailed(_) | CodexErrorDetails::UnexpectedStatus(_) => {
|
||||
CodexErrorInfo::HttpConnectionFailed {
|
||||
http_status_code: self.http_status_code_value(),
|
||||
}
|
||||
}
|
||||
CodexErrorDetails::ConnectionFailed(_)
|
||||
| CodexErrorDetails::InvalidWorkspaceSelected(_)
|
||||
| CodexErrorDetails::UnexpectedStatus(_) => CodexErrorInfo::HttpConnectionFailed {
|
||||
http_status_code: self.http_status_code_value(),
|
||||
},
|
||||
CodexErrorDetails::ResponseStreamFailed(_) => {
|
||||
CodexErrorInfo::ResponseStreamConnectionFailed {
|
||||
http_status_code: self.http_status_code_value(),
|
||||
@@ -586,7 +590,8 @@ impl CodexErr {
|
||||
CodexErrorDetails::FlexUnavailable => Some(StatusCode::TOO_MANY_REQUESTS),
|
||||
CodexErrorDetails::ModelNotFound { status, .. } => *status,
|
||||
CodexErrorDetails::RetryLimit(err) => Some(err.status),
|
||||
CodexErrorDetails::UnexpectedStatus(err) => Some(err.status),
|
||||
CodexErrorDetails::InvalidWorkspaceSelected(err)
|
||||
| CodexErrorDetails::UnexpectedStatus(err) => Some(err.status),
|
||||
CodexErrorDetails::ConnectionFailed(err) => err.source.status(),
|
||||
CodexErrorDetails::ResponseStreamFailed(err) => err.source.status(),
|
||||
_ => None,
|
||||
|
||||
Reference in New Issue
Block a user