From f7e14103197968855b7e462ab70992df84db9540 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B4=AA=E6=B3=BD=E9=91=AB?= Date: Fri, 28 Aug 2026 17:35:17 +0800 Subject: [PATCH] fix(proxy): retire cooldown-suppressed HTTP bridge sessions Prevent hard-key retry-circuit cooldown paths from leaving newly opened, unsubmitted bridge sessions reusable. Preserve authorized replay bypasses and the existing cooldown response envelope. Fixes #1943 Source sidechat: 01a04718-7322-72d0-b4b3-8f5bb4581157 Recovered from: 2268f8caf1fe9d74a8734bd3f9cd8bd5152b5d3f (cherry picked from commit 2a1ce9962b7daccd335f1f10fc2595f6ea9ab702) --- .../_service/http_bridge/request_submit.py | 8 + .../proxy/_service/http_bridge/streaming.py | 8 + .../context.md | 24 +++ .../proposal.md | 41 +++++ .../specs/responses-api-compat/spec.md | 142 ++++++++++++++++++ .../tasks.md | 34 +++++ tests/unit/test_proxy_http_bridge.py | 73 +++++++++ 7 files changed, 330 insertions(+) create mode 100644 openspec/changes/retire-cooldown-suppressed-http-bridge-session/context.md create mode 100644 openspec/changes/retire-cooldown-suppressed-http-bridge-session/proposal.md create mode 100644 openspec/changes/retire-cooldown-suppressed-http-bridge-session/specs/responses-api-compat/spec.md create mode 100644 openspec/changes/retire-cooldown-suppressed-http-bridge-session/tasks.md diff --git a/app/modules/proxy/_service/http_bridge/request_submit.py b/app/modules/proxy/_service/http_bridge/request_submit.py index 3e9906aabe..842f5cd4fd 100644 --- a/app/modules/proxy/_service/http_bridge/request_submit.py +++ b/app/modules/proxy/_service/http_bridge/request_submit.py @@ -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, diff --git a/app/modules/proxy/_service/http_bridge/streaming.py b/app/modules/proxy/_service/http_bridge/streaming.py index ce78e9dd4f..90170b136e 100644 --- a/app/modules/proxy/_service/http_bridge/streaming.py +++ b/app/modules/proxy/_service/http_bridge/streaming.py @@ -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 diff --git a/openspec/changes/retire-cooldown-suppressed-http-bridge-session/context.md b/openspec/changes/retire-cooldown-suppressed-http-bridge-session/context.md new file mode 100644 index 0000000000..3ff91c0535 --- /dev/null +++ b/openspec/changes/retire-cooldown-suppressed-http-bridge-session/context.md @@ -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. diff --git a/openspec/changes/retire-cooldown-suppressed-http-bridge-session/proposal.md b/openspec/changes/retire-cooldown-suppressed-http-bridge-session/proposal.md new file mode 100644 index 0000000000..f9634b3e86 --- /dev/null +++ b/openspec/changes/retire-cooldown-suppressed-http-bridge-session/proposal.md @@ -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 diff --git a/openspec/changes/retire-cooldown-suppressed-http-bridge-session/specs/responses-api-compat/spec.md b/openspec/changes/retire-cooldown-suppressed-http-bridge-session/specs/responses-api-compat/spec.md new file mode 100644 index 0000000000..41a2ae059e --- /dev/null +++ b/openspec/changes/retire-cooldown-suppressed-http-bridge-session/specs/responses-api-compat/spec.md @@ -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 diff --git a/openspec/changes/retire-cooldown-suppressed-http-bridge-session/tasks.md b/openspec/changes/retire-cooldown-suppressed-http-bridge-session/tasks.md new file mode 100644 index 0000000000..2b4857e359 --- /dev/null +++ b/openspec/changes/retire-cooldown-suppressed-http-bridge-session/tasks.md @@ -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. diff --git a/tests/unit/test_proxy_http_bridge.py b/tests/unit/test_proxy_http_bridge.py index c3813cb0bc..143425a14b 100644 --- a/tests/unit/test_proxy_http_bridge.py +++ b/tests/unit/test_proxy_http_bridge.py @@ -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, @@ -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"): @@ -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 @@ -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", @@ -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: @@ -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 @@ -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,