Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 41 additions & 1 deletion app/modules/proxy/_service/http_bridge/request_submit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 8 additions & 0 deletions app/modules/proxy/_service/http_bridge/upstream_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Loading
Loading