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
10 changes: 9 additions & 1 deletion app/modules/proxy/_service/http_bridge/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2770,7 +2770,15 @@ def _http_bridge_should_attempt_local_previous_response_recovery(exc: ProxyRespo
error = payload.get("error")
if not isinstance(error, dict):
return False
code = error.get("code")
code_value = error.get("code")
raw_code = code_value.strip() if isinstance(code_value, str) and code_value.strip() else None
type_value = error.get("type")
error_type = type_value.strip() if isinstance(type_value, str) and type_value.strip() else None
# Normalize like the websocket rewrite path (#1818): upstream frames may
# carry the classifiable code only in ``type`` (or omit both code and
# param on the terse previous-response rejection), and a raw read would
# misclassify them into the ambiguous transport class below (issue #1830).
code = _normalize_error_code(raw_code, error_type)
if code in {
"bridge_owner_unreachable",
"bridge_previous_response_not_found",
Expand Down
38 changes: 37 additions & 1 deletion app/modules/proxy/_service/http_bridge/request_submit.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@
from app.modules.proxy._service.http_bridge.quarantine import (
_record_http_bridge_quarantine_wedged_pending,
)
from app.modules.proxy._service.http_bridge.retry_circuit import (
_http_bridge_anchor_poison_detail,
)
from app.modules.proxy._service.http_bridge.service_stubs import (
_call_with_supported_optional_kwargs,
_classify_upstream_close,
Expand Down Expand Up @@ -124,6 +127,9 @@
_websocket_auth_failure_requires_reauth,
_websocket_request_text_is_account_neutral_fresh_replay,
)
from app.modules.proxy._service.http_bridge.upstream_events import (
_abandon_durable_http_bridge_continuity,
)
from app.modules.proxy._service.observability import (
_hash_identifier as _hash_identifier,
)
Expand Down Expand Up @@ -2846,11 +2852,41 @@ async def _retire_stale_pending_http_bridge_session(
# 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_for_attempt_selection(
consecutive_failures = await self._record_http_bridge_retry_circuit_failure_for_attempt_selection(
session,
detail=retry_circuit_detail or detail,
selection=retry_circuit_attempt_selection,
)
poison_detail = _http_bridge_anchor_poison_detail(retry_circuit_detail or detail)
if (
poison_detail is not None
and consecutive_failures is not None
and consecutive_failures
>= _service_get_settings().http_responses_session_bridge_anchor_poison_failure_threshold
):
# Consecutive eventless failures on one bridge key are
# same-anchor failures (the anchor only advances on a
# completed response, which resets the circuit). Clear the
# poisoned durable anchor while this session still owns the
# lease so the next attempt is not re-anchored into the same
# failure. Without this, only the admission-waiter reader
# path could ever poison an anchor, and an anchored session
# failing without waiters cooled down forever (issue #1830).
durable_cleared = await _abandon_durable_http_bridge_continuity(self, session, detail=poison_detail)
if not durable_cleared and session.durable_session_id is not None:
# Keep failed waiterless clears visible in the same
# poison-clear telemetry the admission-waiter path emits;
# the next threshold failure re-attempts the clear.
_log_http_bridge_event(
"durable_anchor_poison_clear_failed",
session.key,
account_id=session.account.id,
model=session.request_model,
pending_count=retired_request_count,
detail=poison_detail,
cache_key_family=session.key.affinity_kind,
model_class=_extract_model_class(session.request_model) if session.request_model else None,
)
session.closed = True
async with self._http_bridge_lock:
# Bounded close may return while resource finalization is still
Expand Down
18 changes: 18 additions & 0 deletions app/modules/proxy/_service/http_bridge/retry_circuit.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,24 @@
"missing_response_created_timeout": "stream_idle_timeout",
"response_create_gate_timeout_stuck_pending": "stream_idle_timeout",
}
_HTTP_BRIDGE_ANCHOR_POISON_DETAILS = {
"stream_idle_timeout": "repeated_zero_event_idle_timeout",
"stream_incomplete": "repeated_zero_event_stream_incomplete",
}


def _http_bridge_anchor_poison_detail(detail: str | None) -> str | None:
"""Map an eventless retry-circuit failure class to its anchor-poison detail.

Consecutive eventless failures on one bridge key are same-anchor failures:
the durable anchor only advances on a completed response, which resets the
circuit. Both ambiguous transport classes therefore count toward anchor
poison (issue #1830); ``clean_close`` never does.
"""
if detail is None:
return None
aliased = _HTTP_BRIDGE_RETRY_CIRCUIT_DETAIL_ALIASES.get(detail, detail)
return _HTTP_BRIDGE_ANCHOR_POISON_DETAILS.get(aliased)


@dataclass(slots=True)
Expand Down
25 changes: 16 additions & 9 deletions app/modules/proxy/_service/http_bridge/upstream_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@
_record_http_bridge_quarantine_eventless_timeout,
_record_http_bridge_quarantine_wedged_pending,
)
from app.modules.proxy._service.http_bridge.retry_circuit import (
_http_bridge_anchor_poison_detail,
)
from app.modules.proxy._service.http_bridge.service_stubs import (
_assign_websocket_response_id,
_await_cancelled_task,
Expand Down Expand Up @@ -963,6 +966,8 @@ async def _clear_durable_http_bridge_response_anchor(
async def _abandon_durable_http_bridge_continuity(
service: Any,
session: "_HTTPBridgeSession",
*,
detail: str = "repeated_zero_event_idle_timeout",
) -> bool:
"""Clear durable continuity before retiring a repeatedly poisoned bridge.

Expand Down Expand Up @@ -999,7 +1004,7 @@ async def _abandon_durable_http_bridge_continuity(
session.key,
account_id=session.account.id,
model=session.request_model,
detail="repeated_zero_event_idle_timeout",
detail=detail,
cache_key_family=session.key.affinity_kind,
model_class=_extract_model_class(session.request_model) if session.request_model else None,
)
Expand Down Expand Up @@ -1159,7 +1164,7 @@ async def _fail_http_bridge_reader_and_maybe_retire(
),
)
finally:
poison_after_deferred_failures = False
poison_detail: str | None = None
if session.admission_waiter_count > 0 and not force_retire:
retry_circuit_detail = None
if close_classification == "clean":
Expand All @@ -1179,19 +1184,21 @@ async def _fail_http_bridge_reader_and_maybe_retire(
detail=retry_circuit_detail,
selection=retry_circuit_attempt_selection,
)
poison_after_deferred_failures = bool(
retry_circuit_detail == "stream_idle_timeout"
poison_candidate_detail = _http_bridge_anchor_poison_detail(retry_circuit_detail)
if (
poison_candidate_detail is not None
and observed_response_events == 0
and consecutive_failures is not None
and consecutive_failures
>= _service_get_settings().http_responses_session_bridge_anchor_poison_failure_threshold
)
if poison_after_deferred_failures:
durable_cleared = await _abandon_durable_http_bridge_continuity(self, session)
):
poison_detail = poison_candidate_detail
if poison_detail is not None:
durable_cleared = await _abandon_durable_http_bridge_continuity(self, session, detail=poison_detail)
if durable_cleared:
await self._retire_stale_pending_http_bridge_session(
session,
detail="repeated_zero_event_idle_timeout",
detail=poison_detail,
response_events_seen=observed_response_events,
**retry_circuit_attempt_kwargs,
)
Expand All @@ -1203,7 +1210,7 @@ async def _fail_http_bridge_reader_and_maybe_retire(
account_id=session.account.id,
model=session.request_model,
pending_count=session.admission_waiter_count,
detail="repeated_zero_event_idle_timeout",
detail=poison_detail,
cache_key_family=session.key.affinity_kind,
model_class=_extract_model_class(session.request_model) if session.request_model else None,
)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-08-20
29 changes: 29 additions & 0 deletions openspec/changes/classify-bridge-recovery-error-frames/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Classify Bridge Recovery Error Frames

## Why

The HTTP responses session bridge can wedge a session permanently (issue #1830). After one genuine mid-turn interruption the bridge rebinds to its stored durable anchor and re-injects it on every attempt. When upstream rejects that anchor with a classifiable previous-response error, two gaps keep the session unrecoverable:

1. The bridge-local recovery gate reads raw error codes without the normalization the WebSocket path gained in the `classify-invalid-previous-response-id` change (#1818): a frame that carries its classifiable code only in `type`, or the terse parameterless ``Invalid `previous_response_id`.`` shape, falls through to the ambiguous-transport class instead of previous-response recovery.
2. Anchor poisoning only counts `stream_idle_timeout` failures, and only on the reader path when admission waiters exist. The wedge observed in production fails eventlessly with `stream_incomplete` (the bridge's masked form of an upstream previous-response rejection), so the retry circuit opens and cools down forever while `http_responses_session_bridge_anchor_poison_failure_threshold` never fires. Operators had to wipe the `http_bridge_*` tables to free sessions.

## What Changes

- Route the bridge-local previous-response recovery gate through the same error-code normalization as the WebSocket rewrite path (code falls back to `type`; the terse parameterless invalid-previous-response shape classifies as a continuity miss).
- Count both ambiguous eventless transport classes — `stream_incomplete` and `stream_idle_timeout` (with its aliased diagnostics) — toward anchor poison, so consecutive same-anchor failures self-heal even when the frame is genuinely unclassifiable. `clean_close` still never poisons.
- Evaluate anchor poison at the shared retirement boundary as well, so a wedged anchored session that fails without admission waiters also clears its poisoned durable anchor once the threshold is reached.

## Capabilities

### New Capabilities

None.

### Modified Capabilities

- `responses-api-compat`: Normalize error frames at the bridge-local recovery gate and widen anchor poisoning to all consecutive eventless same-anchor failures, including the waiterless retirement path.

## Impact

- HTTP bridge recovery gate (`app/modules/proxy/_service/http_bridge/helpers.py`), anchor-poison accounting (`app/modules/proxy/_service/http_bridge/upstream_events.py`, `app/modules/proxy/_service/http_bridge/request_submit.py`, `app/modules/proxy/_service/http_bridge/retry_circuit.py`).
- No API, schema, migration, dependency, configuration, or dashboard changes; the existing poison threshold setting and its default of seven are unchanged.
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# responses-api-compat Delta

## ADDED Requirements

### Requirement: Bridge-local previous-response recovery classifies normalized error frames

When the HTTP bridge evaluates whether a failed anchored request may enter bridge-local previous-response recovery, it MUST classify the error frame with the same normalization as the WebSocket rewrite path: a missing or empty `code` MUST fall back to the error `type` before classification, and the parameterless ``Invalid `previous_response_id`.`` invalid-request shape MUST classify as a previous-response continuity miss. A classifiable previous-response rejection MUST route into previous-response recovery and MUST NOT be treated as an ambiguous transport failure that only feeds the retry-circuit cooldown.

#### Scenario: Terse parameterless rejection enters local recovery

- **GIVEN** an anchored HTTP bridge request fails with `type = "invalid_request_error"`, no `code`, no `param`, and the message ``Invalid `previous_response_id`.``
- **WHEN** the bridge evaluates bridge-local previous-response recovery for that failure
- **THEN** the failure classifies as a previous-response continuity miss
- **AND** the bridge attempts previous-response recovery instead of the ambiguous-transport path

#### Scenario: Code carried only in the error type classifies

- **GIVEN** an anchored HTTP bridge request fails with no `code` and `type = "previous_response_not_found"`
- **WHEN** the bridge evaluates bridge-local previous-response recovery for that failure
- **THEN** the failure enters previous-response recovery instead of the ambiguous-transport class

#### Scenario: Unrelated errors keep their classification

- **WHEN** a failed anchored request carries an error whose normalized code, param, and message do not match a previous-response continuity miss
- **THEN** the bridge MUST NOT classify it as a previous-response continuity miss

## MODIFIED Requirements

### Requirement: Repeated zero-event idle failures poison dead anchors

For hard HTTP bridge keys, repeated zero-event failures MUST use the existing durable retry-circuit counter to identify an anchor that should no longer remain addressable; the counter resets on a completed response, so a run of consecutive failures proves the anchor never advanced. Both ambiguous eventless transport classes — `stream_idle_timeout` (including its aliased diagnostics) and `stream_incomplete` — MUST be able to trigger anchor poisoning at the threshold; a `clean_close` outcome MUST NOT itself trigger anchor poisoning. When an eligible eventless failure reaches the configured poison threshold for the same hard bridge key, the proxy MUST abandon durable continuity for that session and retire the bridge even when admission waiters exist, and the shared retirement boundary MUST clear the poisoned durable anchor even when no admission waiter exists, while the session still owns its durable lease. If the clear cannot be confirmed on the waiterless retirement path, the proxy MUST re-attempt it when a later eligible eventless failure at or above the threshold retires the session. The default threshold MUST be no greater than seven failures.

#### Scenario: Admission waiters cannot defer anchor poisoning forever

- **GIVEN** a hard durable bridge key has admission waiters
- **AND** repeated zero-event idle failures for that same key reach the poison
threshold
- **WHEN** the reader failure path would normally defer retirement for the
admission waiter
- **THEN** the proxy clears the durable continuity anchors
- **AND** retires the session despite the admission waiter
- **AND** the next attach starts from fresh durable state rather than the
poisoned previous-response anchor

#### Scenario: Repeated eventless stream_incomplete failures poison the anchor

- **GIVEN** a hard durable bridge key has a stored durable anchor
- **AND** every anchored attempt fails eventlessly with `stream_incomplete` (for example a masked upstream previous-response rejection)
- **WHEN** consecutive failures for that key reach the poison threshold
- **THEN** the proxy clears the durable continuity anchors under the session's owner epoch
- **AND** the next attach starts from fresh durable state instead of looping through retry-circuit cooldown

#### Scenario: Waiterless retirement poisons the anchor at the threshold

- **GIVEN** a hard durable bridge key fails eventlessly with no admission waiters
- **WHEN** the shared retirement boundary records the eventless failure that reaches the poison threshold
- **THEN** the proxy clears the durable continuity anchors before releasing the durable lease

#### Scenario: Failed waiterless clear is re-attempted on the next threshold failure

- **GIVEN** the waiterless retirement path reached the poison threshold but the durable continuity clear could not be confirmed
- **WHEN** the next eligible eventless failure for the same key retires the session
- **THEN** the proxy re-attempts the durable continuity clear under the new session's owner epoch

#### Scenario: Clean closes never trigger anchor poisoning

- **WHEN** a `clean_close` retry-circuit outcome is recorded for a hard bridge key, at any consecutive-failure count
- **THEN** that outcome does not clear the durable continuity anchors

#### Scenario: Lease liveness comparison is timezone-safe
- **GIVEN** a durable bridge session whose `lease_expires_at` was read from a `timestamptz` column (offset-aware) on PostgreSQL
- **WHEN** the dead-owner classifier evaluates lease liveness against the application's naive-UTC clock
- **THEN** both timestamps MUST be normalized to naive UTC before comparison
- **AND** the anchored-lookup path MUST NOT raise on mixed-awareness datetimes
21 changes: 21 additions & 0 deletions openspec/changes/classify-bridge-recovery-error-frames/tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Tasks

## 1. Regression Coverage

- [x] 1.1 Add gate regressions for the terse parameterless ``Invalid `previous_response_id`.`` frame and a frame carrying the classifiable code only in `type`, verifying both misclassify (no recovery) before the fix.
- [x] 1.2 Add anchor-poison regressions: consecutive eventless `stream_incomplete` reader failures with an admission waiter, and consecutive eventless failures through the shared retirement boundary without waiters, verifying neither poisons the anchor before the fix.

## 2. Classifier Routing

- [x] 2.1 Normalize the error code (falling back to `type`) in the bridge-local previous-response recovery gate before all classification checks, matching the WebSocket rewrite path from `classify-invalid-previous-response-id`.

## 3. Anchor Poison Counting

- [x] 3.1 Map both ambiguous eventless retry-circuit classes (`stream_incomplete`, `stream_idle_timeout` and its aliases) to anchor-poison details; keep `clean_close` excluded.
- [x] 3.2 Widen the deferred reader-path poison branch to both classes and thread the poison detail into the poisoned-anchor observability events.
- [x] 3.3 Evaluate the poison threshold at the shared retirement boundary and clear the poisoned durable anchor while the session still owns its durable lease.

## 4. Verification

- [x] 4.1 Run the touched bridge unit and integration suites, ruff, and type checks.
- [x] 4.2 Run strict OpenSpec validation for this change and review the final diff for unrelated changes.
Loading
Loading