diff --git a/app/modules/proxy/_service/http_bridge/request_submit.py b/app/modules/proxy/_service/http_bridge/request_submit.py index a84ea4aec6..7b5aa3bcbd 100644 --- a/app/modules/proxy/_service/http_bridge/request_submit.py +++ b/app/modules/proxy/_service/http_bridge/request_submit.py @@ -1880,16 +1880,56 @@ async def _retire_stale_pending_http_bridge_session( detail: str, retry_circuit_detail: str | None = None, response_events_seen: int | None = None, + retired_request_count: int | None = None, ) -> None: async with session.pending_lock: retired_request_states = list(session.pending_requests) + if retired_request_count is None: + retired_request_count = sum( + 1 + for request_state in retired_request_states + if _http_bridge_request_counts_against_queue(request_state) + ) + if response_events_seen is None: + # Direct retirement must derive event evidence from the same + # locked ownership snapshot as the pending count. Otherwise an + # eventful stale-gate owner looks eventless merely because its + # caller omitted this optional handoff, creating a false + # circuit strike. Explicit values remain authoritative for + # reader-failure callers whose pending deque was already + # drained before entering this shared boundary. + response_events_seen = max( + ( + max( + request_state.response_event_count, + int( + request_state.response_id is not None + or request_state.latency_response_created_ms is not None + or request_state.downstream_visible + ), + ) + for request_state in retired_request_states + ), + default=0, + ) # Direct retirement (for example the all-stale stuck-gate path, where # the wedged reattach is the only pending request) cancels the reader # and fails the pendings without passing the partial-cleanup hook or # the reader-failure funnel, so evaluate the wedge shape (#1534) here # too; recording is idempotent for callers that already quarantined. _record_http_bridge_quarantine_wedged_pending(self, session, retired_request_states) - if response_events_seen is None or response_events_seen == 0: + # This circuit measures failed request lifecycles, not upstream socket + # churn. ``response_events_seen == 0`` is also true when an idle reader + # closes with an empty pending deque. Charging that idle close creates a + # phantom first strike, so one later response-create timeout opens the + # nominally "repeated" 60-second cooldown and interrupts the client. + # Keep the ownership proof at this shared retirement boundary unless a + # caller already claimed and drained the deque. The reader-failure + # funnel must pass its pre-drain count because terminal notification + # deliberately empties ``pending_requests`` before retirement. Without + # that handoff, genuine pre-response failures disappear from circuit + # accounting while idle closes and request failures look identical. + if retired_request_count > 0 and response_events_seen == 0: await self._record_http_bridge_retry_circuit_failure( session, detail=retry_circuit_detail or detail, diff --git a/app/modules/proxy/_service/http_bridge/upstream_events.py b/app/modules/proxy/_service/http_bridge/upstream_events.py index e6992cde06..b84a3c9430 100644 --- a/app/modules/proxy/_service/http_bridge/upstream_events.py +++ b/app/modules/proxy/_service/http_bridge/upstream_events.py @@ -858,12 +858,20 @@ async def _fail_http_bridge_reader_and_maybe_retire( detail=error_code, retry_circuit_detail="clean_close", response_events_seen=observed_response_events, + retired_request_count=failed_pending_count, ) else: await self._retire_stale_pending_http_bridge_session( session, detail=retire_detail or error_code, response_events_seen=observed_response_events, + # ``_fail_pending_websocket_requests`` has already + # claimed and drained these states. Carry the count + # sampled under ``pending_lock`` across that ownership + # transfer so normal reader failures still consume one + # strike. The deferred/poison branch records its own + # strike above and intentionally does not pass it. + retired_request_count=failed_pending_count, ) return force_retire or session.admission_waiter_count == 0 diff --git a/openspec/changes/recover-repeated-clean-close/.openspec.yaml b/openspec/changes/archive/2026-08-10-recover-repeated-clean-close/.openspec.yaml similarity index 100% rename from openspec/changes/recover-repeated-clean-close/.openspec.yaml rename to openspec/changes/archive/2026-08-10-recover-repeated-clean-close/.openspec.yaml diff --git a/openspec/changes/archive/2026-08-10-recover-repeated-clean-close/design.md b/openspec/changes/archive/2026-08-10-recover-repeated-clean-close/design.md new file mode 100644 index 0000000000..3922d38c33 --- /dev/null +++ b/openspec/changes/archive/2026-08-10-recover-repeated-clean-close/design.md @@ -0,0 +1,116 @@ +## Context + +The HTTP Responses bridge multiplexes downstream requests over a reusable +upstream WebSocket. Recovery can be initiated either by the upstream reader or +by the downstream HTTP stream watchdog, so socket replacement, reader +ownership, pending-request settlement, and retry-circuit accounting cross +several asynchronous lifecycle boundaries. See `proposal.md` for motivation +and `specs/responses-api-compat/spec.md` for the normative contract. + +Hard-affinity retry circuits are durable across replicas. Their evidence must +therefore describe a client-affecting request lifecycle, not merely a socket +lifecycle event, because idle socket retirement is normal bridge maintenance. + +## Goals / Non-Goals + +**Goals:** + +- Transfer reader ownership atomically when a downstream watchdog replaces the + upstream socket. +- Bound pre-visible recovery so it completes before the downstream client + deadline without permitting duplicate visible work. +- Count only request-affecting, pre-response bridge failures toward the durable + hard-key circuit. +- Preserve circuit state across replicas while bounding process-local and + durable stale state. + +**Non-Goals:** + +- Replay work after any response event has become visible. +- Replay delivery-ambiguous liveness failures or continuity-sensitive payloads. +- Suppress a cooldown after two genuine consecutive eventless request + failures. +- Change the Codex client's WebSocket-to-HTTP fallback policy. + +## Decisions + +### Treat the reader and socket as one generation + +When recovery originates outside the reader, the bridge cancels and awaits the +old reader before locally closing its socket, keeps the shared session live +during replacement, and starts exactly one reader for the new socket. The old +reader's finalizer is generation-guarded so it cannot retire pending work that +has moved to the replacement. + +Allowing old and new readers to overlap was rejected because a local close can +wake the old reader after the pending deque has already been transferred. A +simple `closed` flag was also rejected because it cannot distinguish the +superseded socket generation from the shared session lifetime. + +### Keep pre-visible replay bounded and ahead of the client deadline + +The bridge permits one additional clean-close replay only after the existing +first replay, only before any response event, and with bounded jitter. Silent +pre-response recovery starts after no more than six default ten-second +keepalive intervals, leaving headroom before a 120-second client deadline. + +An unbounded reconnect loop was rejected because it can duplicate requests, +hide deterministic input rejection, and outlive the downstream caller. + +### Derive circuit evidence from an owned request lifecycle + +Retirement advances the circuit only when the retiring session still owns a +pending request and that lifecycle has observed zero response events. The +eligibility snapshot is taken while lifecycle ownership is known; an idle +session with no pending request remains visible in diagnostics but is neutral +to the circuit. A request that emitted any event is excluded because the +pre-response circuit cannot safely characterize a midstream failure. + +Counting every socket retirement was rejected because routine idle churn +creates phantom first strikes. Counting only error labels was rejected because +the same transport label can describe idle maintenance, pre-response failure, +or midstream loss. + +### Persist hard-key circuits and merge conservatively + +Circuit rows are scoped by hard-affinity kind, key, and API-key scope. Conflict +updates cannot shorten an existing cooldown, retry decisions refresh durable +state, success clears state, and stale local/durable entries expire. Durable +lookup failures degrade to local state with diagnostics rather than failing the +request. + +Process-local-only state was rejected because another replica could continue +replay during an open cooldown. Treating persistence failure as terminal was +rejected because the circuit is protective metadata, not request continuity +state. + +### Judge stuck gates from upstream activity + +The watchdog uses elapsed upstream inactivity plus the absence of a response +identifier or `response.created` latency. A prior continuity anchor receives a +bounded second threshold, not an indefinite exemption. Admission flags alone +were rejected because they can remain ambiguous while the upstream socket is +silent. + +## Risks / Trade-offs + +- [A replacement is also silent] -> The extra replay remains hard-capped and + the request reaches terminal or circuit handling. +- [Reader cancellation races with pruning] -> Session handoff state keeps the + shared lifecycle live until replacement ownership is established. +- [Concurrent replicas record failures] -> Durable merge semantics preserve + the longest applicable cooldown. +- [A genuine failure occurs after an idle close] -> The idle close contributes + no strike, so the genuine failure is correctly treated as the first one. +- [Database ancestry was stamped before a merge edge existed] -> A separate + forward-only repair reconnects the request-usage rollup history without + rewriting deployed migrations. + +## Migration Plan + +Apply the forward-only database revisions, deploy the revision-labelled image, +and verify bridge create/reuse, timeout, and retry-circuit diagnostics. Health +verification must confirm the expected image revision and current schema. +Rollback is an image replacement; the prior version can ignore the additional +runtime behavior while the durable circuit table and repair revision remain +forward-compatible. diff --git a/openspec/changes/recover-repeated-clean-close/proposal.md b/openspec/changes/archive/2026-08-10-recover-repeated-clean-close/proposal.md similarity index 81% rename from openspec/changes/recover-repeated-clean-close/proposal.md rename to openspec/changes/archive/2026-08-10-recover-repeated-clean-close/proposal.md index 04d3093d7e..45ecce0254 100644 --- a/openspec/changes/recover-repeated-clean-close/proposal.md +++ b/openspec/changes/archive/2026-08-10-recover-repeated-clean-close/proposal.md @@ -9,6 +9,11 @@ upstream failure and retires work already moved to the replacement socket. Together these behaviors make a transient handoff issue visible as a reconnect loop and require the Codex client to be restarted. +Post-deploy evidence exposed a related accounting gap: retiring an idle bridge +with no pending request still records a retry-circuit failure. The next real +pre-response timeout can therefore open the repeated-failure cooldown after +only one client-affecting failure. + ## What Changes - Permit one additional pre-visible replay when the replacement upstream @@ -31,6 +36,9 @@ loop and require the Codex client to be restarted. response creation, rather than admission flags alone. Give requests with a prior continuity anchor a bounded two-threshold grace period, and emit diagnostic state when the watchdog skips a candidate. +- Count retirement failures only when the bridge still owns a pending request + that has not emitted a response event; idle no-pending closes remain visible + in lifecycle diagnostics but do not consume retry-circuit strikes. ## Impact @@ -39,6 +47,8 @@ loop and require the Codex client to be restarted. - The retry remains bounded and does not create an unbounded replay loop. - Reader ownership follows the active socket across idle recovery, preventing locally generated close frames from being counted as upstream instability. +- Idle upstream connection churn no longer turns one later request timeout into + an immediate sixty-second hard-key cooldown. - Adds the `http_bridge_retry_circuits` durable table and migration so retry cooldown state survives cross-replica clean-close and incomplete-stream failures. diff --git a/openspec/changes/recover-repeated-clean-close/specs/responses-api-compat/spec.md b/openspec/changes/archive/2026-08-10-recover-repeated-clean-close/specs/responses-api-compat/spec.md similarity index 69% rename from openspec/changes/recover-repeated-clean-close/specs/responses-api-compat/spec.md rename to openspec/changes/archive/2026-08-10-recover-repeated-clean-close/specs/responses-api-compat/spec.md index d825873a65..61b83d8285 100644 --- a/openspec/changes/recover-repeated-clean-close/specs/responses-api-compat/spec.md +++ b/openspec/changes/archive/2026-08-10-recover-repeated-clean-close/specs/responses-api-compat/spec.md @@ -41,6 +41,12 @@ being considered stale. When the watchdog skips a candidate, it MUST emit a low-cardinality diagnostic containing the session-closed state, candidate count, and pending-state verdicts. +#### Scenario: clean close before response.created is not retried + +- **WHEN** the initial upstream HTTP responses bridge closes with `close_code = 1000` before any `response.*` event for the pending request +- **THEN** the proxy returns HTTP 502 with `error.code = "upstream_rejected_input"` +- **AND** does not transparently replay the pre-created request + #### Scenario: clean close before response output receives one bounded additional replay - **GIVEN** an HTTP bridge request has no surfaced `response.*` events @@ -115,6 +121,13 @@ when no API key is present). The proxy MUST record only the documented pre-response failure classes (`stream_incomplete`, `clean_close`, and `stream_idle_timeout`). +A bridge retirement MUST record one of those failures only when the retiring +session still owns at least one pending request and no response event has been +observed for that request lifecycle. Retiring an idle upstream bridge with no +pending request MUST NOT advance the circuit or cause a later request to be +treated as a repeated failure. A pending request that has already emitted a +response event MUST remain excluded from this pre-response circuit. + The default circuit MUST open after two consecutive recorded failures. Once open, it MUST suppress pre-created replay until the persisted cooldown expires, using exponential backoff from sixty seconds up to ten minutes. Clean-close @@ -139,6 +152,25 @@ record the failure for observability. Rows older than one hour MUST be treated as expired and removed. A successful terminal response MUST clear the local and durable circuit state. +#### Scenario: idle bridge retirement does not consume a circuit strike + +- **GIVEN** a hard-affinity HTTP bridge has no pending requests +- **WHEN** its upstream WebSocket closes and the idle bridge is retired +- **THEN** the retry-circuit failure count for that key remains unchanged +- **AND** a later request is not placed in cooldown because of the idle close + +#### Scenario: eventless pending retirement consumes exactly one strike + +- **GIVEN** a hard-affinity HTTP bridge owns a pending request with no observed response event +- **WHEN** the bridge retires because the upstream fails before acknowledging the request +- **THEN** the retry circuit records exactly one failure for that request lifecycle + +#### Scenario: midstream retirement does not consume a pre-response strike + +- **GIVEN** a hard-affinity HTTP bridge owns a pending request with an observed response event +- **WHEN** the bridge retires before completion +- **THEN** the pre-response retry-circuit failure count remains unchanged + #### Scenario: the second hard-key failure opens a durable circuit - **GIVEN** a hard-affinity key has one recorded pre-response failure @@ -174,14 +206,45 @@ When an upstream websocket closes while one or more streamed response requests are pending and have not reached a terminal event, the proxy MUST record a transient upstream error for the account before signaling failure for those pending requests, except when the close carries a classified process-wide -network failure, is a clean close (`close_code = 1000`) before any -`response.*` event, or carries the classified per-socket -`upstream_keepalive_timeout` transport error. Clean pre-response closes and -keepalive timeouts MUST remain account-neutral while using the bounded retry -and retry-circuit handling above. A classified process-wide network failure -MUST remain account neutral and use its network error code. For other closes, -the proxy MUST surface -`stream_incomplete` to affected pending requests. +network failure or upstream WebSocket liveness timeout, is a clean close +(`close_code = 1000`) before any `response.*` event, or carries the classified +per-socket `upstream_keepalive_timeout` transport error. Clean pre-response +closes, keepalive timeouts, process-wide network failures, and liveness +timeouts MUST remain account-neutral and use their classified error and bounded +retry or retry-circuit handling. For other closes, the proxy MUST surface +`stream_incomplete` to affected pending requests except when a direct Responses +WebSocket request has already successfully emitted a finite integer +`sequence_number`. For that sequenced direct-WebSocket case, the proxy MUST +record the request outcome as `stream_incomplete` without emitting a synthetic +terminal frame under the active response id, then MUST close the downstream +WebSocket with code 1011. + +#### Scenario: websocket closes before pending responses complete + +- **GIVEN** a streamed response request is pending on an upstream websocket +- **AND** the direct downstream response has not emitted a numeric sequence, or the request uses another transport +- **WHEN** the websocket closes before a terminal response event is observed +- **AND** the close does not carry a classified process-wide network failure or upstream WebSocket liveness timeout +- **THEN** the pending request fails with `stream_incomplete` +- **AND** the account receives a transient upstream failure signal for routing + +#### Scenario: sequenced direct websocket closes before completion + +- **GIVEN** a direct Responses WebSocket request has successfully emitted a finite integer `sequence_number` +- **WHEN** the upstream websocket closes before a terminal response event is observed +- **AND** the close does not carry a classified process-wide network failure or upstream WebSocket liveness timeout +- **THEN** the request is recorded as failed with `stream_incomplete` +- **AND** no synthetic terminal frame is emitted under the active response id +- **AND** the downstream WebSocket closes with code 1011 +- **AND** the account receives a transient upstream failure signal for routing + +#### Scenario: websocket liveness timeout remains account neutral + +- **GIVEN** a streamed response request is pending on an upstream websocket +- **WHEN** its transport reports `upstream_websocket_liveness_timeout` +- **THEN** the pending request fails with that classified error code +- **AND** the account receives no failure-health signal +- **AND** the request is not transparently replayed #### Scenario: clean pre-response close does not penalize the account diff --git a/openspec/changes/recover-repeated-clean-close/tasks.md b/openspec/changes/archive/2026-08-10-recover-repeated-clean-close/tasks.md similarity index 66% rename from openspec/changes/recover-repeated-clean-close/tasks.md rename to openspec/changes/archive/2026-08-10-recover-repeated-clean-close/tasks.md index aa0246098d..7e1be1873a 100644 --- a/openspec/changes/recover-repeated-clean-close/tasks.md +++ b/openspec/changes/archive/2026-08-10-recover-repeated-clean-close/tasks.md @@ -12,3 +12,11 @@ - [x] Add a forward-only repair for databases stamped before request-usage rollups were connected to the merge head. - [x] Validate the OpenSpec change and run the focused and full test suites. - [x] Build and deploy the validated image, then verify production health and logs. + +## Post-deploy regression: idle retirement accounting + +- [x] Require an owned eventless pending request before retirement advances the retry circuit. +- [x] Add lifecycle coverage proving idle no-pending retirement is neutral and eventless pending retirement records exactly one strike. +- [x] Add routed coverage proving an idle close plus one real timeout does not open the repeated-failure cooldown. +- [x] Run focused bridge suites, lint/type/architecture checks, and strict OpenSpec validation. +- [x] Build and deploy the revised image, then verify health and retry-circuit diagnostics. diff --git a/openspec/specs/responses-api-compat/context.md b/openspec/specs/responses-api-compat/context.md index 9a62f360da..583fc9b4a8 100644 --- a/openspec/specs/responses-api-compat/context.md +++ b/openspec/specs/responses-api-compat/context.md @@ -35,6 +35,7 @@ See `openspec/specs/responses-api-compat/spec.md` for normative requirements. - Upstream Responses WebSockets use transport ping/pong control frames to detect a black-holed connection without confusing valid application-event silence with an idle turn. Direct and routed connections reuse `proxy_downstream_websocket_idle_timeout_seconds` for this zero-config liveness budget. - A post-send liveness timeout is delivery-ambiguous. It remains account-neutral, is never transparently replayed, and retires the affected upstream socket so a client retry opens a fresh route without risking duplicated model work or tool side effects. - HTTP bridge settlement ownership is explicit: `closed` rejects new work but does not imply that a submitter owns existing siblings. Only a liveness-failed send claims whole-deque settlement under the lifecycle lock; otherwise the reader remains responsible for settling pending requests when the transport dies. +- Hard-affinity retry-circuit evidence is request-lifecycle evidence: retirement counts only while the bridge still owns an eventless pending request. Idle no-pending retirement remains observable but neutral, so routine socket churn cannot manufacture the first strike for a later real timeout. ## Fast Mode and Service Tiers @@ -118,6 +119,7 @@ when upstream reports a different actual tier. - **Codex websocket stale previous-response anchors:** Direct backend Codex websocket stale-anchor failures are surfaced as `response.failed` / `codex_previous_response_stale` without the raw upstream code or missing `resp_...` id; OpenAI-compatible `/v1/responses` websocket clients continue to receive generic `stream_incomplete` masking. - **Websocket handshake forbidden/not-found:** Auto transport now fails loud on `403` / `404` instead of silently hiding the websocket regression behind HTTP fallback. - **Upstream websocket stops answering pings:** Pending direct-WebSocket and HTTP-bridge work fails with `upstream_websocket_liveness_timeout`; the account remains healthy and the request is not replayed because upstream acceptance is unknown. +- **Repeated eventless bridge failures:** Two consecutive request-affecting pre-response failures can open the hard-key cooldown. A successful terminal response clears the state; an idle close followed by one real timeout remains only one strike. - **Invalid request payloads:** Return 4xx with `invalid_request_error`. ## Error Envelope Mapping (Reference) @@ -150,6 +152,11 @@ Cursor-style model alias request: This forwards upstream as `model: "gpt-5.4-mini"` with `reasoning.effort: "high"`. +Retry-circuit accounting example: an idle bridge closes with `pending=0`, then +the next request times out before `response.created`. The idle close is logged +but contributes no failure; the timeout is the first strike. Only another +consecutive eventless pending failure may open the repeated-failure cooldown. + ## Known Client Integrations (Reference) Third-party agents that consume the `/v1` Responses surface documented by this @@ -179,5 +186,6 @@ OpenSpec change first. - When tracing compact incidents, confirm that request logs and upstream logs show direct `/codex/responses/compact` usage without surrogate `/codex/responses` fallback. - Post-deploy: monitor `no_accounts`, `stream_incomplete`, and `upstream_unavailable`. - Post-deploy: monitor `upstream_websocket_liveness_timeout`; recurring failures indicate a host route, VPN, proxy, or intermediary that black-holes established WebSockets. +- Post-deploy: correlate retry-circuit `opened`, `half_open`, and `reset` events with bridge `pending` and `response_events_seen` diagnostics. An idle `pending=0` retirement must not precede an immediate two-failure cooldown. - Post-deploy: monitor `codex_previous_response_stale` on `/backend-api/codex/responses`; recurring spikes mean clients are still relying on stale upstream anchors and should perform the documented full-context retry without `previous_response_id`. - Websocket/Codex CLI tier verification runbook: `openspec/specs/responses-api-compat/ops.md` diff --git a/openspec/specs/responses-api-compat/spec.md b/openspec/specs/responses-api-compat/spec.md index 972e710a50..907414edea 100644 --- a/openspec/specs/responses-api-compat/spec.md +++ b/openspec/specs/responses-api-compat/spec.md @@ -67,14 +67,204 @@ When `upstream_stream_transport` is `"auto"` and the serialized request payload ### Requirement: Clean upstream close before any response event fails fast -When the HTTP responses bridge observes an upstream websocket close with `close_code = 1000` before any `response.*` event has been surfaced for the pending request, the proxy MUST classify the close as rejected input, surface HTTP 502 `upstream_rejected_input`, and MUST NOT trigger `retry_precreated` or `retry_fresh_upstream`. +When the HTTP Responses bridge observes an upstream WebSocket close with +`close_code = 1000` before any `response.*` event has been surfaced for the +pending request, the proxy MUST preserve its existing pre-visible replay +guards. If the request has already used exactly one eligible pre-visible +replay and the replacement upstream WebSocket also closes cleanly before any +response event, the proxy MAY perform exactly one additional replay. The +additional replay MUST be hard-capped at one per request, and the configured +maximum MUST NOT raise that cap. + +The proxy MUST NOT replay after downstream-visible output, after a terminal +response event, or when continuity-sensitive request state makes replay unsafe. +Before the additional replay, the proxy MAY sleep for bounded configured +jitter. The proxy MUST emit a dedicated low-cardinality diagnostic event for +the additional replay. + +When a downstream HTTP stream task initiates pre-response recovery while the +upstream reader is blocked on the superseded socket, the proxy MUST cancel and +await that reader before locally closing the socket. It MUST then start exactly +one reader for the replacement socket. A close caused by replacing the socket +MUST NOT be recorded as an upstream clean-close failure, MUST NOT increment the +retry circuit, and MUST NOT retire pending work moved to the replacement. The +cancelled reader's socket-generation finalizer MUST NOT leave the shared session +marked closed while the replacement socket is being selected or opened, so idle +pruning MUST NOT evict the handoff in progress. + +The default pre-response idle-recovery window MUST leave bounded headroom +before the downstream client's request timeout. With the default ten-second +keepalive interval, the proxy MUST initiate eligible recovery after no more +than six silent intervals so replacement connection and first output can occur +before a 120-second client deadline. + +The stuck pre-response watchdog MUST judge staleness using elapsed time since +the last upstream activity and the absence of a response identifier or +`response.created` latency, not admission flags alone. A request with a prior +continuity anchor MUST receive at most two retire-thresholds of grace before +being considered stale. When the watchdog skips a candidate, it MUST emit a +low-cardinality diagnostic containing the session-closed state, candidate +count, and pending-state verdicts. #### Scenario: clean close before response.created is not retried -- **WHEN** upstream closes the HTTP responses bridge with `close_code = 1000` before any `response.*` event for the pending request +- **WHEN** the initial upstream HTTP responses bridge closes with `close_code = 1000` before any `response.*` event for the pending request - **THEN** the proxy returns HTTP 502 with `error.code = "upstream_rejected_input"` - **AND** does not transparently replay the pre-created request +#### Scenario: clean close before response output receives one bounded additional replay + +- **GIVEN** an HTTP bridge request has no surfaced `response.*` events +- **AND** its first pre-visible replay has already been used +- **WHEN** the replacement upstream WebSocket closes with code `1000` +- **THEN** the proxy performs one additional pre-visible replay +- **AND** the request replay count increases by one +- **AND** the proxy emits a `retry_precreated_clean_close` diagnostic event + +#### Scenario: repeated clean closes do not create an unbounded replay loop + +- **GIVEN** the additional clean-close replay has already been used +- **WHEN** another upstream WebSocket closes cleanly before response output +- **THEN** the proxy does not replay the request again +- **AND** the existing terminal or circuit handling is used + +#### Scenario: visible output still prevents clean-close replay + +- **GIVEN** the pending request has surfaced any response event downstream +- **WHEN** the upstream WebSocket closes with code `1000` +- **THEN** the proxy does not replay the request + +#### Scenario: clean-close retry jitter is bounded + +- **GIVEN** clean-close retry jitter is configured +- **WHEN** the additional clean-close replay is scheduled +- **THEN** the delay is no greater than the configured jitter maximum +- **AND** the hard replay cap remains one regardless of the configured value + +#### Scenario: downstream idle recovery transfers reader ownership + +- **GIVEN** the upstream reader is blocked on the current bridge socket +- **AND** the downstream HTTP stream task initiates eligible pre-response recovery +- **WHEN** the bridge replaces the upstream socket +- **THEN** the old reader is cancelled and awaited before its socket is closed +- **AND** the shared session remains live while the replacement socket opens +- **AND** idle pruning retains the registered session while the handoff is in progress +- **AND** exactly one reader owns the replacement socket +- **AND** the local close does not open or increment the retry circuit +- **AND** pending work remains attached to the replacement session + +#### Scenario: silent pre-response recovery precedes the client timeout + +- **GIVEN** the upstream has produced no response event +- **AND** the default ten-second keepalive interval is active +- **WHEN** six silent intervals elapse +- **THEN** the proxy initiates eligible pre-response recovery +- **AND** at least sixty seconds remain before a 120-second client request timeout + +#### Scenario: anchored stuck-gate grace is bounded + +- **GIVEN** a pending HTTP bridge request has a prior continuity anchor +- **AND** no response identifier or `response.created` latency has been recorded +- **WHEN** less than two retire thresholds have elapsed since the gate began waiting +- **THEN** the watchdog does not classify the request as stale +- **WHEN** two retire thresholds elapse without upstream activity +- **THEN** the watchdog may classify the request as stale + +#### Scenario: upstream activity resolves admission-flag ambiguity + +- **GIVEN** a pending request has not acquired the response-created gate +- **AND** upstream activity has not produced a response identifier or `response.created` +- **WHEN** the staleness threshold elapses +- **THEN** the watchdog classifies the request as stale +- **AND** emits pending-state verdict inputs when it skips a watchdog pass + +### Requirement: Durable retry-circuit state protects repeated hard-affinity failures + +For a hard-affinity bridge key, the proxy MUST scope retry-circuit state by +affinity kind, affinity key, and API-key scope (using a stable anonymous scope +when no API key is present). The proxy MUST record only the documented +pre-response failure classes (`stream_incomplete`, `clean_close`, and +`stream_idle_timeout`). + +A bridge retirement MUST record one of those failures only when the retiring +session still owns at least one pending request and no response event has been +observed for that request lifecycle. Retiring an idle upstream bridge with no +pending request MUST NOT advance the circuit or cause a later request to be +treated as a repeated failure. A pending request that has already emitted a +response event MUST remain excluded from this pre-response circuit. + +The default circuit MUST open after two consecutive recorded failures. Once +open, it MUST suppress pre-created replay until the persisted cooldown expires, +using exponential backoff from sixty seconds up to ten minutes. Clean-close +failures MUST cap their cooldown at thirty seconds. The proxy MUST persist +failure count, cooldown deadline, last failure detail, and update time in the +`http_bridge_retry_circuits` table and MUST merge conflict updates so concurrent +replicas cannot shorten an existing cooldown. + +The clean-close retry jitter maximum MUST be read from the +`http_responses_session_bridge_clean_close_retry_jitter_max_seconds` runtime +setting and MUST be bounded to the inclusive range 0–30 seconds. + +The proxy MUST evict process-local circuit entries and their loaded/persisted +markers after one hour without use, independently of durable-row cleanup, so +one-shot hard-affinity keys cannot grow the worker's memory without bound. + +Before every hard-affinity retry decision, the proxy MUST refresh the durable +row so a cooldown opened by another replica is observed even when this process +has already loaded the key. A durable lookup or persistence failure MUST NOT +crash the request; the proxy MUST continue using available local state and +record the failure for observability. Rows older than one hour MUST be treated +as expired and removed. A successful terminal response MUST clear the local +and durable circuit state. + +#### Scenario: idle bridge retirement does not consume a circuit strike + +- **GIVEN** a hard-affinity HTTP bridge has no pending requests +- **WHEN** its upstream WebSocket closes and the idle bridge is retired +- **THEN** the retry-circuit failure count for that key remains unchanged +- **AND** a later request is not placed in cooldown because of the idle close + +#### Scenario: eventless pending retirement consumes exactly one strike + +- **GIVEN** a hard-affinity HTTP bridge owns a pending request with no observed response event +- **WHEN** the bridge retires because the upstream fails before acknowledging the request +- **THEN** the retry circuit records exactly one failure for that request lifecycle + +#### Scenario: midstream retirement does not consume a pre-response strike + +- **GIVEN** a hard-affinity HTTP bridge owns a pending request with an observed response event +- **WHEN** the bridge retires before completion +- **THEN** the pre-response retry-circuit failure count remains unchanged + +#### Scenario: the second hard-key failure opens a durable circuit + +- **GIVEN** a hard-affinity key has one recorded pre-response failure +- **WHEN** a second eligible failure is recorded +- **THEN** the proxy opens the retry circuit +- **AND** persists at least two consecutive failures and a cooldown deadline +- **AND** subsequent pre-created replay is suppressed until that deadline + +#### Scenario: retry decisions observe a cooldown opened by another replica + +- **GIVEN** this replica previously looked up a hard-affinity key with no row +- **AND** another replica persists an open cooldown for that same key and API-key scope +- **WHEN** this replica evaluates the next pre-created retry +- **THEN** it refreshes durable state before deciding +- **AND** suppresses the retry for the persisted cooldown + +#### Scenario: circuit state remains isolated by key and API-key scope + +- **GIVEN** one hard-affinity key has an open circuit +- **WHEN** a different affinity key or API-key scope evaluates a retry +- **THEN** that request is not suppressed by the first key's circuit + +#### Scenario: durable circuit lookup failure does not fail the request + +- **GIVEN** durable retry-circuit lookup or persistence is unavailable +- **WHEN** the proxy evaluates or records a retry-circuit event +- **THEN** the request continues using any available local circuit state +- **AND** the failure is logged and exposed through retry-circuit observability + ### Requirement: Long Codex websocket turns tolerate extended upstream silence The default compact request budget MUST be at least 180 seconds, and the default upstream stream idle timeout MUST be at least 600 seconds, so long-running Codex turns can survive expensive compaction or tool execution without a local proxy watchdog ending the turn prematurely. @@ -128,7 +318,22 @@ The proxy MUST configure direct and routed upstream Responses WebSocket transpor - **AND** the submitter cancellation is preserved after settlement completes ### Requirement: Upstream websocket drops penalize affected accounts -When an upstream websocket closes while one or more streamed response requests are pending and have not reached a terminal event, the proxy MUST record a transient upstream error for the account before signaling failure for those pending requests, except when the close carries a classified process-wide network failure or upstream WebSocket liveness timeout. A classified process-wide network failure or upstream WebSocket liveness timeout MUST remain account neutral and use its classified error code. For other closes, the proxy MUST surface `stream_incomplete` to affected pending requests except when a direct Responses WebSocket request has already successfully emitted a finite integer `sequence_number`. For that sequenced direct-WebSocket case, the proxy MUST record the request outcome as `stream_incomplete` without emitting a synthetic terminal frame under the active response id, then MUST close the downstream WebSocket with code 1011. +When an upstream websocket closes while one or more streamed response requests +are pending and have not reached a terminal event, the proxy MUST record a +transient upstream error for the account before signaling failure for those +pending requests, except when the close carries a classified process-wide +network failure or upstream WebSocket liveness timeout, is a clean close +(`close_code = 1000`) before any `response.*` event, or carries the classified +per-socket `upstream_keepalive_timeout` transport error. Clean pre-response +closes, keepalive timeouts, process-wide network failures, and liveness +timeouts MUST remain account-neutral and use their classified error and bounded +retry or retry-circuit handling. For other closes, the proxy MUST surface +`stream_incomplete` to affected pending requests except when a direct Responses +WebSocket request has already successfully emitted a finite integer +`sequence_number`. For that sequenced direct-WebSocket case, the proxy MUST +record the request outcome as `stream_incomplete` without emitting a synthetic +terminal frame under the active response id, then MUST close the downstream +WebSocket with code 1011. #### Scenario: websocket closes before pending responses complete @@ -157,6 +362,13 @@ When an upstream websocket closes while one or more streamed response requests a - **AND** the account receives no failure-health signal - **AND** the request is not transparently replayed +#### Scenario: clean pre-response close does not penalize the account + +- **GIVEN** a hard-affinity HTTP bridge request is pending with no surfaced response event +- **WHEN** the upstream websocket closes cleanly before response output +- **THEN** the proxy records the clean-close retry-circuit outcome +- **AND** the selected account is not penalized + ### Requirement: Single HTTP bridge previous-response misses recover or fail closed When an HTTP bridge session receives an anonymous upstream `previous_response_not_found` error for a single pending follow-up request, the service MUST treat the error as an internal continuity-loss signal. It MUST either recover through the existing previous-response rebind path or rewrite the error to a retryable continuity failure instead of forwarding the raw upstream invalid-request error. diff --git a/tests/integration/test_http_responses_bridge.py b/tests/integration/test_http_responses_bridge.py index 2007188cdc..375e028496 100644 --- a/tests/integration/test_http_responses_bridge.py +++ b/tests/integration/test_http_responses_bridge.py @@ -37,6 +37,7 @@ from app.modules.proxy._service.http_bridge import quarantine as http_bridge_quarantine_module from app.modules.proxy._service.http_bridge import streaming as http_bridge_streaming_module from app.modules.proxy._service.http_bridge.helpers import ( + _make_http_bridge_session_header_fallback_key, _release_http_bridge_unanchored_handoff, _reserve_http_bridge_unanchored_handoff, ) @@ -12660,6 +12661,89 @@ async def fake_connect_responses_websocket( record_retry_circuit_failure.assert_not_awaited() +@pytest.mark.asyncio +async def test_backend_responses_http_bridge_idle_retirement_does_not_open_retry_circuit_on_next_failure( + async_client, + app_instance, + monkeypatch, +): + _install_bridge_settings(monkeypatch, enabled=True) + account_id = await _import_account( + async_client, + "acc_backend_idle_retirement_circuit", + "backend-idle-retirement-circuit@example.com", + ) + account = await _get_account(account_id) + upstream = _FakeBridgeUpstreamWebSocket("resp_idle_retirement_circuit") + + async def fake_select_account_with_budget(self, deadline, **kwargs): + del self, deadline, kwargs + return AccountSelection(account=account, error_message=None, error_code=None) + + async def fake_ensure_fresh_with_budget(self, target, *, force=False, timeout_seconds): + del self, force, timeout_seconds + return target + + async def fake_connect_responses_websocket( + headers, + access_token, + account_id_header, + *, + base_url=None, + session=None, + ): + del headers, access_token, account_id_header, base_url, session + return upstream + + monkeypatch.setattr(proxy_module.ProxyService, "_select_account_with_budget", fake_select_account_with_budget) + monkeypatch.setattr(proxy_module.ProxyService, "_ensure_fresh_with_budget", fake_ensure_fresh_with_budget) + monkeypatch.setattr(proxy_module, "connect_responses_websocket", fake_connect_responses_websocket) + + session_id = "backend-idle-retirement-circuit-session" + prompt_cache_key = "backend-idle-retirement-circuit-thread" + headers = {"session_id": session_id} + bridge_key = _make_http_bridge_session_header_fallback_key( + headers=headers, + api_key=None, + explicit_prompt_cache_key=prompt_cache_key, + ) + assert bridge_key is not None + service = get_proxy_service_for_app(app_instance) + + # Reproduce the live ordering without waiting for production-scale + # watchdogs: an idle no-pending retirement, then one genuine pre-response + # request failure on the same hard key. Only the latter may be a strike. + idle_session = _make_dummy_bridge_session(bridge_key) + await service._retire_stale_pending_http_bridge_session( + idle_session, + detail="stream_incomplete", + response_events_seen=0, + ) + failed_request_session = _make_dummy_bridge_session(bridge_key) + failures = await service._record_http_bridge_retry_circuit_failure( + failed_request_session, + detail="missing_response_created_timeout", + ) + assert failures == 1 + assert await service._http_bridge_precreated_retry_allowed(failed_request_session) is True + + events = await _collect_sse_events( + async_client, + "/backend-api/codex/responses", + json_body={ + "model": "gpt-5.1", + "instructions": "Return exactly OK.", + "input": "continue after one real timeout", + "prompt_cache_key": prompt_cache_key, + "stream": True, + }, + headers=headers, + ) + + _assert_created_text_delta_completed(events) + assert events[-1]["response"]["id"] == "resp_idle_retirement_circuit_1" + + @pytest.mark.asyncio async def test_retry_http_bridge_precreated_request_releases_pending_lock_before_reconnect(app_instance, monkeypatch): service = get_proxy_service_for_app(app_instance) diff --git a/tests/unit/test_proxy_http_bridge.py b/tests/unit/test_proxy_http_bridge.py index 4dd6baae6d..44bcdeb2c5 100644 --- a/tests/unit/test_proxy_http_bridge.py +++ b/tests/unit/test_proxy_http_bridge.py @@ -21963,6 +21963,7 @@ async def test_http_bridge_liveness_timeout_is_neutral_not_replayed_and_forces_r session, detail=UPSTREAM_WEBSOCKET_LIVENESS_TIMEOUT_CODE, response_events_seen=0, + retired_request_count=1, ) assert session.queued_request_count == 0 assert session.closed is True @@ -22113,6 +22114,7 @@ async def controlled_fail_reader( session, detail=UPSTREAM_WEBSOCKET_LIVENESS_TIMEOUT_CODE, response_events_seen=0, + retired_request_count=2, ) @@ -22235,6 +22237,7 @@ def pending_sibling(request_id: str) -> proxy_service._WebSocketRequestState: session, detail=UPSTREAM_WEBSOCKET_LIVENESS_TIMEOUT_CODE, response_events_seen=0, + retired_request_count=2, ) @@ -22354,7 +22357,12 @@ async def test_http_bridge_clean_close_before_response_does_not_penalize_account assert fail_pending.await_args is not None assert fail_pending.await_args.kwargs["penalize_account"] is False - retire.assert_awaited_once_with(session, detail="stream_incomplete", response_events_seen=0) + retire.assert_awaited_once_with( + session, + detail="stream_incomplete", + response_events_seen=0, + retired_request_count=0, + ) @pytest.mark.asyncio @@ -22526,6 +22534,103 @@ async def test_retire_stale_pending_http_bridge_session_unregisters_aliases_and_ close.assert_awaited_once() +@pytest.mark.asyncio +async def test_http_bridge_idle_retirement_does_not_record_retry_circuit_failure( + monkeypatch: pytest.MonkeyPatch, +) -> None: + service = proxy_service.ProxyService(cast(Any, nullcontext())) + session = _make_bridge_session(key_value="bridge-idle-retire") + record_failure = AsyncMock() + close = AsyncMock() + monkeypatch.setattr(service, "_record_http_bridge_retry_circuit_failure", record_failure) + monkeypatch.setattr(service, "_close_http_bridge_session_bounded", close) + + await service._retire_stale_pending_http_bridge_session( + session, + detail="stream_incomplete", + response_events_seen=0, + ) + + record_failure.assert_not_awaited() + close.assert_awaited_once_with(session, reason="retire_stale_pending") + + +@pytest.mark.asyncio +async def test_http_bridge_eventless_pending_retirement_records_one_retry_circuit_failure( + monkeypatch: pytest.MonkeyPatch, +) -> None: + service = proxy_service.ProxyService(cast(Any, nullcontext())) + owner = _make_eventless_http_bridge_owner(request_id="req-eventless-retire") + session = _make_bridge_session( + key_value="bridge-eventless-retire", + pending_requests=deque([owner]), + queued_request_count=1, + ) + record_failure = AsyncMock() + monkeypatch.setattr(service, "_record_http_bridge_retry_circuit_failure", record_failure) + monkeypatch.setattr(service, "_close_http_bridge_session_bounded", AsyncMock()) + + await service._retire_stale_pending_http_bridge_session( + session, + detail="missing_response_created_timeout", + response_events_seen=0, + ) + + record_failure.assert_awaited_once_with(session, detail="missing_response_created_timeout") + + +@pytest.mark.asyncio +async def test_http_bridge_direct_retirement_derives_observed_response_events( + monkeypatch: pytest.MonkeyPatch, +) -> None: + service = proxy_service.ProxyService(cast(Any, nullcontext())) + owner = _make_eventless_http_bridge_owner(request_id="req-eventful-direct-retire") + owner.response_event_count = 1 + session = _make_bridge_session( + key_value="bridge-eventful-direct-retire", + pending_requests=deque([owner]), + queued_request_count=1, + ) + record_failure = AsyncMock() + monkeypatch.setattr(service, "_record_http_bridge_retry_circuit_failure", record_failure) + monkeypatch.setattr(service, "_close_http_bridge_session_bounded", AsyncMock()) + + await service._retire_stale_pending_http_bridge_session( + session, + detail="stuck_response_create_gate", + ) + + record_failure.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_http_bridge_reader_failure_preserves_pre_drain_request_for_retry_circuit( + monkeypatch: pytest.MonkeyPatch, +) -> None: + service = proxy_service.ProxyService(cast(Any, nullcontext())) + owner = _make_eventless_http_bridge_owner(request_id="req-reader-failure-retire") + session = _make_bridge_session( + key_value="bridge-reader-failure-retire", + pending_requests=deque([owner]), + queued_request_count=1, + ) + record_failure = AsyncMock() + monkeypatch.setattr(service, "_record_http_bridge_retry_circuit_failure", record_failure) + monkeypatch.setattr(service, "_close_http_bridge_session_bounded", AsyncMock()) + + retired = await service._fail_http_bridge_reader_and_maybe_retire( + session, + error_code="stream_incomplete", + error_message="upstream closed before response.completed", + penalize_account=False, + response_events_seen=0, + ) + + assert retired is True + assert not session.pending_requests + record_failure.assert_awaited_once_with(session, detail="stream_incomplete") + + @pytest.mark.asyncio async def test_http_bridge_retirement_does_not_record_midstream_retry_circuit_failure( monkeypatch: pytest.MonkeyPatch, @@ -23467,7 +23572,12 @@ async def test_http_bridge_eventless_timeout_force_retires_with_admission_waiter assert retired is True assert session.closed is True - retire.assert_awaited_once_with(session, detail="missing_response_created_timeout", response_events_seen=0) + retire.assert_awaited_once_with( + session, + detail="missing_response_created_timeout", + response_events_seen=0, + retired_request_count=0, + ) fail_pending_await_args = fail_pending.await_args assert fail_pending_await_args is not None assert fail_pending_await_args.kwargs["penalize_account"] is False @@ -23491,7 +23601,12 @@ async def test_http_bridge_reader_failure_retires_without_waiters_when_notificat error_message="closed", ) - retire.assert_awaited_once_with(session, detail="stream_incomplete", response_events_seen=0) + retire.assert_awaited_once_with( + session, + detail="stream_incomplete", + response_events_seen=0, + retired_request_count=0, + ) @pytest.mark.asyncio