From 18cefba922ab17fda8783ac7c61c9b96798439cc Mon Sep 17 00:00:00 2001 From: jif-oai Date: Wed, 20 May 2026 11:56:00 +0200 Subject: [PATCH] Add timeout for remote compaction requests (#23451) ## Why Remote compaction currently sends a unary `POST /responses/compact` and waits for the full response before replacing history or emitting the completed `ContextCompaction` item. Unlike normal `/responses` streaming requests, this unary compact request had no timeout boundary. If the backend accepts the request and then stalls before returning a body, the existing request retry policy never sees a transport error, so the compact turn can remain stuck after the started item with no completion or actionable error. That matches the reported hang shape in issues such as #18363, where logs show `responses/compact` was posted but no corresponding compact completion followed. A bounded request timeout gives the existing retry policy a concrete timeout error to retry instead of letting the user sit indefinitely on automatic context compaction. ## What - Add a request timeout to legacy `/responses/compact` calls. - Size that timeout from the provider stream idle timeout with a conservative multiplier, so the default compact attempt gets 20 minutes rather than the 5 minute stream idle window. - Map API transport timeouts to a request timeout error instead of the child-process timeout message. ## Testing - Not run (per request; CI will cover). --- codex-rs/codex-api/src/api_bridge.rs | 2 +- codex-rs/codex-api/src/endpoint/compact.rs | 15 +++++++++++++-- codex-rs/core/src/client.rs | 9 ++++++++- codex-rs/protocol/src/error.rs | 3 +++ 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/codex-rs/codex-api/src/api_bridge.rs b/codex-rs/codex-api/src/api_bridge.rs index 4f37c2ff9..401dfab3a 100644 --- a/codex-rs/codex-api/src/api_bridge.rs +++ b/codex-rs/codex-api/src/api_bridge.rs @@ -123,7 +123,7 @@ pub fn map_api_error(err: ApiError) -> CodexErr { status: http::StatusCode::INTERNAL_SERVER_ERROR, request_id: None, }), - TransportError::Timeout => CodexErr::Timeout, + TransportError::Timeout => CodexErr::RequestTimeout, TransportError::Network(msg) | TransportError::Build(msg) => { CodexErr::Stream(msg, None) } diff --git a/codex-rs/codex-api/src/endpoint/compact.rs b/codex-rs/codex-api/src/endpoint/compact.rs index 336e958d4..a3da85484 100644 --- a/codex-rs/codex-api/src/endpoint/compact.rs +++ b/codex-rs/codex-api/src/endpoint/compact.rs @@ -11,6 +11,7 @@ use http::Method; use serde::Deserialize; use serde_json::to_value; use std::sync::Arc; +use std::time::Duration; pub struct CompactClient { session: EndpointSession, @@ -37,10 +38,19 @@ impl CompactClient { &self, body: serde_json::Value, extra_headers: HeaderMap, + request_timeout: Duration, ) -> Result, ApiError> { let resp = self .session - .execute(Method::POST, Self::path(), extra_headers, Some(body)) + .execute_with( + Method::POST, + Self::path(), + extra_headers, + Some(body), + |req| { + req.timeout = Some(request_timeout); + }, + ) .await?; let parsed: CompactHistoryResponse = serde_json::from_slice(&resp.body).map_err(|e| ApiError::Stream(e.to_string()))?; @@ -51,10 +61,11 @@ impl CompactClient { &self, input: &CompactionInput<'_>, extra_headers: HeaderMap, + request_timeout: Duration, ) -> Result, ApiError> { let body = to_value(input) .map_err(|e| ApiError::Stream(format!("failed to encode compaction input: {e}")))?; - self.compact(body, extra_headers).await + self.compact(body, extra_headers, request_timeout).await } } diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index f604a6345..e3569e6d1 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -146,6 +146,9 @@ const X_CODEX_WS_STREAM_REQUEST_START_MS_CLIENT_METADATA_KEY: &str = const RESPONSES_WEBSOCKETS_V2_BETA_HEADER_VALUE: &str = "responses_websockets=2026-02-06"; const RESPONSES_ENDPOINT: &str = "/responses"; const RESPONSES_COMPACT_ENDPOINT: &str = "/responses/compact"; +// `/responses/compact` is unary, so the timeout covers the full response rather than one idle +// period between stream events. +const COMPACT_REQUEST_TIMEOUT_IDLE_MULTIPLIER: u32 = 4; const MEMORIES_SUMMARIZE_ENDPOINT: &str = "/memories/trace_summarize"; #[cfg(test)] pub(crate) const WEBSOCKET_CONNECT_TIMEOUT: Duration = @@ -502,12 +505,16 @@ impl ModelClient { if let Some(header_value) = self.generate_attestation_header_for().await { extra_headers.insert(X_OAI_ATTESTATION_HEADER, header_value); } + let compact_request_timeout = client_setup + .api_provider + .stream_idle_timeout + .saturating_mul(COMPACT_REQUEST_TIMEOUT_IDLE_MULTIPLIER); let client = ApiCompactClient::new(transport, client_setup.api_provider, client_setup.api_auth) .with_telemetry(Some(request_telemetry)); let trace_attempt = compaction_trace.start_attempt(&payload); let result = client - .compact_input(&payload, extra_headers) + .compact_input(&payload, extra_headers, compact_request_timeout) .await .map_err(map_api_error); trace_attempt.record_result(result.as_deref()); diff --git a/codex-rs/protocol/src/error.rs b/codex-rs/protocol/src/error.rs index 207fd94ca..ef9c86cad 100644 --- a/codex-rs/protocol/src/error.rs +++ b/codex-rs/protocol/src/error.rs @@ -89,6 +89,8 @@ pub enum CodexErr { /// Returned by run_command_stream when the spawned child process timed out (10s). #[error("timeout waiting for child process to exit")] Timeout, + #[error("request timed out")] + RequestTimeout, /// Returned by run_command_stream when the child could not be spawned (its stdout/stderr pipes /// could not be captured). Analogous to the previous `CodexError::Spawn` variant. #[error("spawn failed: child stdout/stderr not captured")] @@ -192,6 +194,7 @@ impl CodexErr { | CodexErr::CyberPolicy { .. } => false, CodexErr::Stream(..) | CodexErr::Timeout + | CodexErr::RequestTimeout | CodexErr::UnexpectedStatus(_) | CodexErr::ResponseStreamFailed(_) | CodexErr::ConnectionFailed(_)