Skip to content
Merged
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
8 changes: 8 additions & 0 deletions app/modules/proxy/_service/http_bridge/request_submit.py
Original file line number Diff line number Diff line change
Expand Up @@ -949,6 +949,14 @@ async def _submit_http_bridge_request_with_handoff(
1,
math.ceil(await self._http_bridge_precreated_retry_cooldown_seconds(session)),
)
# The session may have been created before this late admission
# check. Do not leave an unsubmitted socket reusable during the
# cooldown: mark it for retirement before surfacing the 503 and
# let the normal bounded drain path handle registry detach,
# aliases, leases, and close ownership.
session.upstream_control.reconnect_requested = True
session.upstream_control.retire_after_drain = True
await self._retire_http_bridge_after_drain_if_ready(session)
_log_http_bridge_event(
"submit_retry_circuit_suppressed",
session.key,
Expand Down
8 changes: 8 additions & 0 deletions app/modules/proxy/_service/http_bridge/streaming.py
Original file line number Diff line number Diff line change
Expand Up @@ -3873,6 +3873,14 @@ async def startup_continuity_cooldown_terminal_event() -> str | None:
request_state.request_id,
retry_cooldown_seconds,
)
# Session creation precedes this startup admission check. Mark
# the opened bridge as retiring before returning the terminal
# cooldown result so no later request can reuse its socket. The
# bounded helper closes immediately when idle and otherwise waits
# for existing owners to drain.
session.upstream_control.reconnect_requested = True
session.upstream_control.retire_after_drain = True
await self._retire_http_bridge_after_drain_if_ready(session)
# This path returns before the request is submitted, so the normal
# detach/finally cleanup cannot settle an API-key reservation.
# Release it before handing the synthetic terminal event to the
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Context

The bridge opens or reuses a session before the request-specific pre-created
retry decision. That ordering is required for continuity and account routing,
but it means a cooldown decision can arrive after a WebSocket has already been
registered. Returning an error alone is insufficient: the session remains in
the registry and may be selected by a later request.

The existing `_retire_http_bridge_after_drain_if_ready` helper is the correct
lifecycle owner. Setting both control flags makes `_http_bridge_session_reusable_for_request`
reject new work immediately. The helper closes only when there is no visible
pending request, queue count, unanchored reservation, or competing close, and
otherwise leaves the session marked for retirement until the remaining owner
settles it.

The startup terminal path is equivalent to late suppression from a lifecycle
perspective: it has an already-created session but no upstream `response.create`
dispatch. It must use the same flags and helper so both paths are idempotent
and share registry detachment, alias cleanup, lease settlement, and bounded
socket close behavior.

Replay exceptions remain intentionally unchanged. A proof-gated full resend or
an operation-fenced continuity replay is an explicitly authorized dispatch;
those paths must not be retired by the generic cooldown suppression branch.
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Retire Cooldown-Suppressed HTTP Bridge Sessions

## Why

Issue #1943 exposes a lifecycle leak in the HTTP Responses bridge. A request
can create and register an upstream WebSocket before the late retry-circuit
admission check runs. When hard-key cooldown suppresses that request, the proxy
returns the expected 503 but leaves the newly created session reusable. The
orphaned socket can then be selected by a later half-open probe and keep the
key in a repeated failure loop.

The same leak exists in the startup pre-submit cooldown path used for
continuity-bound requests: it returns a synthetic 503 before submit without
retiring the session that was already opened.

## What Changes

- Mark a session as `reconnect_requested` and `retire_after_drain` whenever a
hard-key cooldown rejects a request before upstream dispatch.
- Invoke the existing bounded drain-retirement helper immediately so an idle
session is detached and closed; sessions with other pending work remain
non-reusable until that work drains.
- Cover both late submit suppression and startup pre-submit terminal paths.
- Preserve proof-gated and operation-fenced replay bypasses, durable retry
circuit state, reservation settlement, and the existing 503 envelope.

## Impact

- Cooldown-suppressed bridge sessions cannot be reused for later requests.
- Newly opened but never-dispatched WebSockets are closed and removed from the
registry when no other work owns the session.
- Shared sessions with pending work drain through the existing lifecycle and
are not force-closed or double-settled.
- No new setting, schema, migration, retry threshold, or cooldown duration is
introduced.

## Source

- GitHub issue: Soju06/codex-lb#1943
- Feasibility analysis: Fable5 response `msg_l4vLQ0i8RzQiFjCGE2W7cOiE`
- Source sidechat: 01a04718-7322-72d0-b4b3-8f5bb4581157
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
## MODIFIED Requirements

### 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.

When hard-key retry-circuit cooldown suppresses a request after its bridge
session has been created but before `response.create` is dispatched, the proxy
MUST mark that session `reconnect_requested` and `retire_after_drain`, then
MUST invoke the bounded drain-retirement path before returning the suppression
error. The session MUST become ineligible for new reuse immediately. If other
pending work or an unanchored handoff still owns the session, closure MAY wait
for that work to drain, but the retirement marker MUST remain set and the
session MUST NOT accept new requests. This applies to both late submit
suppression and startup pre-submit cooldown terminal handling.

Proof-gated full-resend replay and operation-fenced continuity replay remain
eligible bypasses and MUST NOT be retired by this suppression requirement.

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

#### Scenario: late cooldown suppression retires the newly created session

- **GIVEN** a hard-key request has created or selected an HTTP bridge session
- **AND** the retry circuit is still in cooldown when late pre-created
admission runs
- **WHEN** the request is suppressed before `response.create` is dispatched
- **THEN** the proxy returns the existing HTTP 503 cooldown error
- **AND** marks the session for reconnect and retirement after drain
- **AND** invokes bounded retirement
- **AND** does not send `response.create` upstream
- **AND** the session is not reusable for a later request

#### Scenario: startup cooldown terminal handling retires its session

- **GIVEN** a hard continuity-bound request has an already-created bridge
session but no safe replay bypass
- **AND** the retry circuit is in cooldown before the startup submit attempt
- **WHEN** startup terminal handling returns the cooldown failure
- **THEN** the existing 503 or synthetic `stream_idle_timeout` envelope is
preserved
- **AND** the session is marked for reconnect and retirement after drain
- **AND** bounded retirement is invoked
- **AND** submit is not attempted

#### Scenario: cooldown replay bypass does not retire the session

- **GIVEN** a hard-key request is in cooldown
- **AND** proof-gated or operation-fenced continuity replay is allowed
- **WHEN** the retry decision runs
- **THEN** the request remains eligible for that authorized replay
- **AND** the generic cooldown suppression retirement is not triggered

#### Scenario: shared pending work drains before close

- **GIVEN** cooldown suppression marks a session that owns other visible
pending work or an unanchored handoff
- **WHEN** bounded retirement runs
- **THEN** it does not force-close or double-settle the active work
- **AND** new requests cannot reuse the session
- **AND** the existing owner settlement path may close it after drain
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Tasks

## Specification

- [x] Add the `responses-api-compat` delta for retiring sessions rejected by
hard-key retry-circuit cooldown before upstream dispatch.
- [x] Record the late-submit and startup pre-submit scenarios, including the
replay bypass and shared-pending-work safeguards.

## Implementation

- [x] Mark and retire the session in the late submit suppression branch.
- [x] Mark and retire the session in the startup continuity cooldown terminal
branch.
- [x] Keep proof-gated and operation-fenced bypasses unchanged.

## Regression coverage

- [x] Assert late suppression returns the same 503, marks the session retiring,
invokes the bounded retire helper, and never sends upstream.
- [x] Assert startup pre-submit suppression marks the session retiring, invokes
the helper, preserves the 503 envelope, and never submits.
- [x] Assert the replay bypass path does not retire the session.

## Verification

- [x] Run focused and full HTTP bridge unit/integration tests and Ruff on
changed files.
- [x] Run strict OpenSpec validation and inspect the final diff/status.

Validation note: the change passes strict OpenSpec validation. The repository's
non-strict full spec scan reports one pre-existing `model-source-routing`
failure; the affected `responses-api-compat` and `proxy-admission-control`
specs pass.
73 changes: 73 additions & 0 deletions tests/unit/test_proxy_http_bridge.py
Original file line number Diff line number Diff line change
Expand Up @@ -6408,6 +6408,7 @@ async def test_http_bridge_previous_response_anchor_bypasses_hard_turn_cooldown_
cooldown = AsyncMock(return_value=30.0)
submit = AsyncMock(side_effect=RuntimeError("submitted without cooldown wait"))
sleep = AsyncMock()
retire = AsyncMock(return_value=False)

monkeypatch.setattr(
proxy_service,
Expand All @@ -6418,6 +6419,7 @@ async def test_http_bridge_previous_response_anchor_bypasses_hard_turn_cooldown_
)
monkeypatch.setattr(service, "_http_bridge_precreated_retry_cooldown_seconds", cooldown)
monkeypatch.setattr(service, "_submit_http_bridge_request", submit)
monkeypatch.setattr(service, "_retire_http_bridge_after_drain_if_ready", retire)
monkeypatch.setattr(http_bridge_streaming_module.asyncio, "sleep", sleep)

with pytest.raises(RuntimeError, match="submitted without cooldown wait"):
Expand All @@ -6434,6 +6436,9 @@ async def test_http_bridge_previous_response_anchor_bypasses_hard_turn_cooldown_
cooldown.assert_not_awaited()
sleep.assert_not_awaited()
submit.assert_awaited_once()
retire.assert_not_awaited()
assert session.upstream_control.reconnect_requested is False
assert session.upstream_control.retire_after_drain is False


@pytest.mark.asyncio
Expand All @@ -6456,6 +6461,7 @@ async def test_http_bridge_one_shot_hard_turn_without_durable_fence_fails_closed
)
submit = AsyncMock()
sleep = AsyncMock()
retire = AsyncMock(return_value=False)
monkeypatch.setattr(
proxy_service,
"get_settings",
Expand All @@ -6465,6 +6471,7 @@ async def test_http_bridge_one_shot_hard_turn_without_durable_fence_fails_closed
)
monkeypatch.setattr(service, "_http_bridge_precreated_retry_cooldown_seconds", AsyncMock(return_value=30.0))
monkeypatch.setattr(service, "_submit_http_bridge_request", submit)
monkeypatch.setattr(service, "_retire_http_bridge_after_drain_if_ready", retire)
monkeypatch.setattr(http_bridge_streaming_module.asyncio, "sleep", sleep)

with pytest.raises(ProxyResponseError) as exc_info:
Expand All @@ -6482,6 +6489,18 @@ async def test_http_bridge_one_shot_hard_turn_without_durable_fence_fails_closed
assert exc_info.value.payload["error"]["code"] == "upstream_request_timeout"
submit.assert_not_awaited()
sleep.assert_not_awaited()
assert session.upstream_control.reconnect_requested is True
assert session.upstream_control.retire_after_drain is True
retire.assert_awaited_once_with(session)
assert (
http_bridge_helpers_module._http_bridge_session_reusable_for_request(
session=session,
key=session.key,
incoming_turn_state=None,
previous_response_id=None,
)
is False
)


@pytest.mark.asyncio
Expand Down Expand Up @@ -28937,6 +28956,60 @@ async def test_http_bridge_submit_suppresses_hard_key_during_retry_cooldown() ->
assert hard_session.queued_request_count == 0


@pytest.mark.asyncio
async def test_http_bridge_submit_cooldown_suppression_retires_session_before_send() -> None:
service = proxy_service.ProxyService(cast(Any, nullcontext()))
hard_session = _make_bridge_session(key_value="bridge-submit-cooldown-retires")
now = time.monotonic()
cast(Any, service)._http_bridge_retry_circuits[hard_session.key] = (
http_bridge_retry_circuit_module._HTTPBridgeRetryCircuitState(
consecutive_failures=2,
cooldown_until=now + 60.0,
last_detail="stream_idle_timeout",
last_touched_monotonic=now,
)
)
service._durable_bridge = SimpleNamespace(lookup_retry_circuit=AsyncMock(return_value=None))
retire = AsyncMock(return_value=False)
service._retire_http_bridge_after_drain_if_ready = retire
request_state = proxy_service._WebSocketRequestState(
request_id="req-submit-cooldown-retires",
model="gpt-5.6-luna",
service_tier=None,
reasoning_effort=None,
api_key_reservation=None,
started_at=time.monotonic(),
transport="http",
awaiting_response_created=True,
request_text='{"type":"response.create","input":"hello"}',
)

with pytest.raises(ProxyResponseError) as exc_info:
await service._submit_http_bridge_request_with_handoff(
hard_session,
request_state=request_state,
text_data=request_state.request_text or "",
queue_limit=8,
request_scope_id="scope-submit-cooldown-retires",
owned_unanchored_handoff=False,
)

assert exc_info.value.status_code == 503
assert exc_info.value.payload["error"]["code"] == "upstream_request_timeout"
assert hard_session.upstream_control.reconnect_requested is True
assert hard_session.upstream_control.retire_after_drain is True
retire.assert_awaited_once_with(hard_session)
assert (
http_bridge_helpers_module._http_bridge_session_reusable_for_request(
session=hard_session,
key=hard_session.key,
incoming_turn_state=None,
previous_response_id=None,
)
is False
)


@pytest.mark.asyncio
async def test_http_bridge_retry_circuit_ignores_soft_affinity_and_other_failures(
monkeypatch: pytest.MonkeyPatch,
Expand Down
Loading