diff --git a/app/modules/proxy/_service/http_bridge/anchor_poison.py b/app/modules/proxy/_service/http_bridge/anchor_poison.py new file mode 100644 index 0000000000..f3d83871a6 --- /dev/null +++ b/app/modules/proxy/_service/http_bridge/anchor_poison.py @@ -0,0 +1,88 @@ +from __future__ import annotations + +import logging +from typing import Any + +from app.modules.proxy._service.http_bridge.helpers import _log_http_bridge_event +from app.modules.proxy._service.http_bridge.service_stubs import _service_get_settings +from app.modules.proxy.affinity import _extract_model_class + +logger = logging.getLogger(__name__) + + +def _log_durable_anchor_poison_clear_failed(session: Any, detail: str) -> None: + if session.durable_session_id is None: + return + _log_http_bridge_event( + "durable_anchor_poison_clear_failed", + session.key, + account_id=session.account.id, + model=session.request_model, + pending_count=len(session.pending_requests), + detail=detail, + cache_key_family=session.key.affinity_kind, + model_class=_extract_model_class(session.request_model) if session.request_model else None, + ) + + +async def _abandon_durable_http_bridge_continuity( + service: Any, + session: Any, + *, + detail: str = "repeated_zero_event_idle_timeout", + settle_circuit: bool = False, +) -> bool: + """Clear durable continuity while the failed session still owns its row. + + The write is fenced by the session owner epoch. A failed or fenced clear + is deliberately reported as ``False`` so callers keep the retry circuit + open and do not treat cooldown expiry as proof that the poisoned anchor is + gone. + """ + if session.durable_session_id is None or session.durable_owner_epoch is None: + session.anchor_poison_clear_failed = True + return False + try: + cleared = await service._durable_bridge.rebind_session_account( + session_id=session.durable_session_id, + api_key_id=session.key.api_key_id, + instance_id=_service_get_settings().http_responses_session_bridge_instance_id, + owner_epoch=session.durable_owner_epoch, + account_id=session.account.id, + clear_continuity=True, + ) + except Exception: + session.anchor_poison_clear_failed = True + _log_durable_anchor_poison_clear_failed(session, detail) + logger.warning("Failed to abandon poisoned HTTP bridge continuity", exc_info=True) + return False + if not cleared: + session.anchor_poison_clear_failed = True + _log_durable_anchor_poison_clear_failed(session, detail) + logger.warning( + "Durable bridge continuity clear was fenced before poisoned anchor retirement", + extra={ + "session_id": session.durable_session_id, + "account_id": session.account.id, + }, + ) + return False + + session.anchor_poison_cleared = True + session.anchor_poison_clear_failed = False + _log_http_bridge_event( + "durable_anchor_poisoned", + session.key, + account_id=session.account.id, + model=session.request_model, + detail=detail, + cache_key_family=session.key.affinity_kind, + model_class=_extract_model_class(session.request_model) if session.request_model else None, + ) + # The anchor is gone, so the circuit can be settled as the cause-removal + # step. Keep this after the fenced continuity write: if circuit cleanup + # itself is unavailable, the durable poison row remains conservative and a + # later healthy response can still clear it. + if settle_circuit: + await service._clear_http_bridge_retry_circuit(session, settle_unfenced=True) + return True diff --git a/app/modules/proxy/_service/http_bridge/quarantine.py b/app/modules/proxy/_service/http_bridge/quarantine.py index b1e66157e7..f654eb8980 100644 --- a/app/modules/proxy/_service/http_bridge/quarantine.py +++ b/app/modules/proxy/_service/http_bridge/quarantine.py @@ -206,6 +206,11 @@ def _clear_http_bridge_quarantine( """A completed response disproves the current and recovery-origin wedges.""" registry = _http_bridge_quarantine_registry(service) session.quarantined = False + # A completed response establishes a new healthy continuity anchor. Any + # poison-settlement result from the previous lifecycle must not suppress + # future anchor handling on this session. + session.anchor_poison_cleared = False + session.anchor_poison_clear_failed = False keys = (session.key,) if additional_key is None or additional_key == session.key else (session.key, additional_key) for key in keys: entry = registry.pop(key, None) diff --git a/app/modules/proxy/_service/http_bridge/request_submit.py b/app/modules/proxy/_service/http_bridge/request_submit.py index 263cc98370..6f6e0dc274 100644 --- a/app/modules/proxy/_service/http_bridge/request_submit.py +++ b/app/modules/proxy/_service/http_bridge/request_submit.py @@ -78,7 +78,6 @@ from app.modules.proxy._service.http_bridge.helpers import ( _await_task_deferring_cancellation, _build_http_bridge_prewarm_text, - _http_bridge_abandonment_strands_requests, _http_bridge_durable_lease_ttl_seconds, _http_bridge_is_previous_response_owner_unavailable, _http_bridge_key_strength, @@ -96,9 +95,6 @@ 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, @@ -128,9 +124,6 @@ _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, ) @@ -2927,46 +2920,11 @@ 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: - consecutive_failures = await self._record_http_bridge_retry_circuit_failure_for_attempt_selection( + 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, - settle_circuit=_http_bridge_abandonment_strands_requests(retired_request_states), - ) - 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 diff --git a/app/modules/proxy/_service/http_bridge/retry_circuit.py b/app/modules/proxy/_service/http_bridge/retry_circuit.py index 332a0c9fa0..a25f68e8d8 100644 --- a/app/modules/proxy/_service/http_bridge/retry_circuit.py +++ b/app/modules/proxy/_service/http_bridge/retry_circuit.py @@ -9,6 +9,9 @@ import anyio from app.core.metrics.prometheus import PROMETHEUS_AVAILABLE, http_bridge_retry_circuit_total +from app.modules.proxy._service.http_bridge.anchor_poison import ( + _abandon_durable_http_bridge_continuity, +) from app.modules.proxy._service.http_bridge.quarantine import ( _HTTP_BRIDGE_QUARANTINE_POISONED_ANCHOR_REASON, _quarantine_http_bridge_session, @@ -80,6 +83,8 @@ class _HTTPBridgeRetryCircuitState: last_failure_monotonic: float = 0.0 last_durable_load_monotonic: float = 0.0 half_open_until: float = 0.0 + anchor_poison_clear_in_flight: bool = False + anchor_poison_clear_retry_after: float = 0.0 def _initialize_http_bridge_retry_circuit(service: Any, reset_transient_cache: Any = None) -> None: @@ -159,6 +164,38 @@ async def _http_bridge_retry_circuit_generation_for_key( local_cooldown_until, ) + async def _http_bridge_retry_circuit_anchor_poisoned_for_key( + self: Any, + key: _HTTPBridgeSessionKey, + ) -> bool: + """Return whether a durable poison-class circuit still needs clearing. + + ``last_detail`` plus the threshold is the durable marker. A successful + fenced anchor settlement clears the circuit row, while a failed clear + leaves this marker in place so a restarted replica cannot re-inject the + old anchor after the in-memory quarantine expires. + """ + try: + persisted = await self._durable_bridge.lookup_retry_circuit( + session_key_kind=key.affinity_kind, + session_key_value=key.affinity_key, + api_key_id=key.api_key_id, + ) + except Exception: + return False + if persisted is not None: + return bool( + persisted.consecutive_failures >= _HTTP_BRIDGE_RETRY_CIRCUIT_FAILURE_THRESHOLD + and _http_bridge_anchor_poison_detail(persisted.last_detail) is not None + ) + async with self._http_bridge_retry_circuit_lock: + state = self._http_bridge_retry_circuits.get(key) + return bool( + state is not None + and state.consecutive_failures >= _HTTP_BRIDGE_RETRY_CIRCUIT_FAILURE_THRESHOLD + and _http_bridge_anchor_poison_detail(state.last_detail) is not None + ) + async def _http_bridge_retry_circuit_generation_is_not_newer( self: Any, *, @@ -647,6 +684,7 @@ async def _record_http_bridge_retry_circuit_failure( poison_class_failure = _http_bridge_anchor_poison_detail(detail) is not None quarantine_poisoned_anchor = False quarantine_cooldown_remaining = 0.0 + anchor_clear_required = False async with self._http_bridge_retry_circuit_lock: if scoped_attempt is not None and scoped_attempt.retry_circuit_failure_recorded: duplicate_attempt = scoped_attempt @@ -678,6 +716,13 @@ async def _record_http_bridge_retry_circuit_failure( if poison_class_failure: quarantine_poisoned_anchor = True quarantine_cooldown_remaining = max(0.0, state.cooldown_until - now) + if ( + not getattr(session, "anchor_poison_cleared", False) + and not state.anchor_poison_clear_in_flight + and now >= state.anchor_poison_clear_retry_after + ): + state.anchor_poison_clear_in_flight = True + anchor_clear_required = True if PROMETHEUS_AVAILABLE and http_bridge_retry_circuit_total is not None: http_bridge_retry_circuit_total.labels(outcome="opened").inc() logger.warning( @@ -705,6 +750,38 @@ async def _record_http_bridge_retry_circuit_failure( ) try: await self._persist_http_bridge_retry_circuit(session, state) + if poison_class_failure and not anchor_clear_required: + # A durable conflict can merge another replica's strike into + # this state while persistence is in flight. Re-evaluate the + # threshold after the merge so the clear cannot be skipped. + async with self._http_bridge_retry_circuit_lock: + if ( + self._http_bridge_retry_circuits.get(session.key) is state + and state.consecutive_failures >= threshold + and not getattr(session, "anchor_poison_cleared", False) + and not state.anchor_poison_clear_in_flight + and time.monotonic() >= state.anchor_poison_clear_retry_after + ): + state.anchor_poison_clear_in_flight = True + anchor_clear_required = True + if anchor_clear_required: + poison_detail = _http_bridge_anchor_poison_detail(detail) + durable_anchor_cleared = False + if poison_detail is not None: + durable_anchor_cleared = await _abandon_durable_http_bridge_continuity( + self, + session, + detail=poison_detail, + settle_circuit=True, + ) + async with self._http_bridge_retry_circuit_lock: + if self._http_bridge_retry_circuits.get(session.key) is state: + state.anchor_poison_clear_in_flight = False + if not durable_anchor_cleared: + state.anchor_poison_clear_retry_after = max( + state.anchor_poison_clear_retry_after, + state.cooldown_until, + ) merged_quarantine_cooldown_remaining = 0.0 async with self._http_bridge_retry_circuit_lock: if self._http_bridge_retry_circuits.get(session.key) is state: diff --git a/app/modules/proxy/_service/http_bridge/streaming.py b/app/modules/proxy/_service/http_bridge/streaming.py index 81d12bd530..cb17ae7a10 100644 --- a/app/modules/proxy/_service/http_bridge/streaming.py +++ b/app/modules/proxy/_service/http_bridge/streaming.py @@ -1353,6 +1353,7 @@ async def release_unowned_bridge_lifecycle( durable_full_resend_fresh_bridge_proof: _VerifiedDurableFullResend | None = None force_local_recovery_creation = False payload_looks_like_full_resend = _http_bridge_payload_looks_like_full_resend(payload) + anchor_poison_circuit_unresolved = False # Set when the quarantine check below suppresses the durable-anchor # injection for a full-resend payload; the session hydration and the # session-level anchor injection further down must honor it so the @@ -1514,6 +1515,27 @@ def classify_durable_full_resend( durable_lookup.canonical_key, bridge_session_key.api_key_id, ) + anchor_poison_circuit_unresolved = await self._http_bridge_retry_circuit_anchor_poisoned_for_key( + bridge_session_key + ) + if anchor_poison_circuit_unresolved and not payload_looks_like_full_resend: + _record_continuity_fail_closed( + surface="http_bridge", + reason="poisoned_anchor_unresolved", + previous_response_id=payload.previous_response_id, + session_id=bridge_session_key.affinity_key, + upstream_error_code="retry_circuit_poisoned_anchor", + ) + raise ProxyResponseError( + 503, + openai_error( + "upstream_request_timeout", + "The previous response anchor is temporarily unavailable while bridge recovery completes.", + ), + retryable_same_contract=True, + failure_phase="upstream", + failure_detail="retry_circuit_poisoned_anchor", + ) live_local_session_exists = await self._http_bridge_has_live_local_session( key=bridge_session_key, incoming_turn_state=incoming_turn_state_header, @@ -1530,7 +1552,9 @@ def classify_durable_full_resend( and durable_lookup.latest_response_id is not None and (not payload_looks_like_full_resend or durable_anchor_trimmable) ) - if payload_looks_like_full_resend and _http_bridge_session_key_quarantined(self, bridge_session_key): + if payload_looks_like_full_resend and ( + _http_bridge_session_key_quarantined(self, bridge_session_key) or anchor_poison_circuit_unresolved + ): # The previous attach on this key proved silent/wedged # (#1534). The client's own payload already carries the full # conversation, so send it unanchored on the fresh path diff --git a/app/modules/proxy/_service/http_bridge/upstream_events.py b/app/modules/proxy/_service/http_bridge/upstream_events.py index 452cc85a16..e6517c51b6 100644 --- a/app/modules/proxy/_service/http_bridge/upstream_events.py +++ b/app/modules/proxy/_service/http_bridge/upstream_events.py @@ -61,7 +61,6 @@ from app.modules.proxy._service.http_bridge.helpers import ( _HTTP_BRIDGE_MISSING_RESPONSE_CREATED_TIMEOUT_DETAIL, _await_task_deferring_cancellation, - _http_bridge_abandonment_strands_requests, _http_bridge_continuity_bound_without_safe_replay, _http_bridge_durable_lease_ttl_seconds, _http_bridge_eventless_precreated_deadline, @@ -966,62 +965,6 @@ 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", - settle_circuit: bool = False, -) -> bool: - """Clear durable continuity before retiring a repeatedly poisoned bridge. - - ``rebind_session_account(clear_continuity=True)`` is an existing fenced - write that clears the durable response/turn anchor and its alias rows while - this worker still owns the session. The ordinary retirement path then - closes the row and removes the process-local registrations. - """ - if session.durable_session_id is None or session.durable_owner_epoch is None: - return False - try: - cleared = await service._durable_bridge.rebind_session_account( - session_id=session.durable_session_id, - api_key_id=session.key.api_key_id, - instance_id=_service_get_settings().http_responses_session_bridge_instance_id, - owner_epoch=session.durable_owner_epoch, - account_id=session.account.id, - clear_continuity=True, - ) - except Exception: - logger.warning("Failed to abandon poisoned HTTP bridge continuity", exc_info=True) - return False - if not cleared: - logger.warning( - "Durable bridge continuity clear was fenced before poisoned anchor retirement", - extra={ - "session_id": session.durable_session_id, - "account_id": session.account.id, - }, - ) - return False - _log_http_bridge_event( - "durable_anchor_poisoned", - session.key, - account_id=session.account.id, - model=session.request_model, - detail=detail, - cache_key_family=session.key.affinity_kind, - model_class=_extract_model_class(session.request_model) if session.request_model else None, - ) - # Retirement and close funnels may reach this helper while a verified - # stale-anchor replay is still in flight. Those callers opt in only when - # every covered request is stranded; a replay that still holds a safe - # anchor-free body needs the circuit generation fence until dispatch. - if not settle_circuit: - return True - await service._clear_http_bridge_retry_circuit(session, settle_unfenced=True) - return True - - class _HTTPBridgeUpstreamEventsMixin: async def _fail_http_bridge_reader_and_maybe_retire( self: Any, @@ -1200,18 +1143,11 @@ async def _fail_http_bridge_reader_and_maybe_retire( 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 + and consecutive_failures >= _HTTP_BRIDGE_RETRY_CIRCUIT_FAILURE_THRESHOLD ): poison_detail = poison_candidate_detail if poison_detail is not None: - durable_cleared = await _abandon_durable_http_bridge_continuity( - self, - session, - detail=poison_detail, - settle_circuit=_http_bridge_abandonment_strands_requests(pending_request_states), - ) - if durable_cleared: + if getattr(session, "anchor_poison_cleared", False): await self._retire_stale_pending_http_bridge_session( session, detail=poison_detail, @@ -1219,17 +1155,6 @@ async def _fail_http_bridge_reader_and_maybe_retire( **retry_circuit_attempt_kwargs, ) force_retire = True - else: - _log_http_bridge_event( - "durable_anchor_poison_clear_failed", - session.key, - account_id=session.account.id, - model=session.request_model, - pending_count=session.admission_waiter_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, - ) else: _log_http_bridge_event( "retire_deferred_for_admission_waiter", @@ -2039,7 +1964,6 @@ async def _process_parsed_http_bridge_upstream_event( ) grouped_retry_detail = "stream_incomplete" if is_previous_response_not_found_event else grouped_error_reason grouped_terminal_events = [] - grouped_terminal_strike_failures: list[int] = [] for grouped_request_state in grouped_previous_response_request_states: grouped_request_state.error_http_status_override = 502 ( @@ -2064,14 +1988,12 @@ async def _process_parsed_http_bridge_upstream_event( ) ) if grouped_request_state.response_event_count == 0: - grouped_failure_count = await self._record_http_bridge_retry_circuit_failure( + await self._record_http_bridge_retry_circuit_failure( session, detail=grouped_retry_detail, attempt=grouped_request_state.response_create_attempt, terminal_pre_response_frame=True, ) - if grouped_failure_count is not None: - grouped_terminal_strike_failures.append(grouped_failure_count) append_terminal_batch = getattr( getattr(self, "_http_bridge_operation_event_batcher", None), @@ -2219,16 +2141,6 @@ async def persist_grouped_terminal_events() -> Exception | None: raise grouped_cancellation if grouped_error is not None: raise grouped_error - if any( - failures >= _HTTP_BRIDGE_RETRY_CIRCUIT_FAILURE_THRESHOLD - for failures in grouped_terminal_strike_failures - ): - await _abandon_durable_http_bridge_continuity( - self, - session, - detail="repeated_zero_event_stream_incomplete", - settle_circuit=True, - ) return if len(grouped_previous_response_request_states) == 1 and terminal_request_state is None: @@ -2986,8 +2898,6 @@ async def persist_grouped_terminal_events() -> Exception | None: if retried: return - terminal_strike_failures: int | None = None - terminal_poison_detail: str | None = None if settlement_event_type in {"response.failed", "response.incomplete", "error"}: terminal_error = ( settlement_event.error @@ -3011,13 +2921,12 @@ async def persist_grouped_terminal_events() -> Exception | None: and terminal_request_state.response_event_count == 0 and _http_bridge_continuity_bound_without_safe_replay(terminal_request_state) ): - terminal_strike_failures = await self._record_http_bridge_retry_circuit_failure( + await self._record_http_bridge_retry_circuit_failure( session, detail=terminal_retry_detail or "stream_incomplete", attempt=terminal_request_state.response_create_attempt, terminal_pre_response_frame=True, ) - terminal_poison_detail = _http_bridge_anchor_poison_detail(terminal_retry_detail) matched_event_queue = ( completed_event_queue @@ -3117,21 +3026,6 @@ async def persist_grouped_terminal_events() -> Exception | None: # awaited recovery work before it rechecks this scope. completed_delivery_scope.terminal_enqueued = True - if ( - terminal_poison_detail is not None - and terminal_strike_failures is not None - and terminal_strike_failures >= _HTTP_BRIDGE_RETRY_CIRCUIT_FAILURE_THRESHOLD - ): - # Terminal frames settle without entering the retirement funnel. - # Clear the durable anchor after publishing the failure so a client - # resend is covered by the quarantine already armed by the strike. - await _abandon_durable_http_bridge_continuity( - self, - session, - detail=terminal_poison_detail, - settle_circuit=True, - ) - if settlement_event_type in {"response.failed", "response.incomplete", "error"}: error_code = None if settlement_event_type == "error": diff --git a/app/modules/proxy/_service/support.py b/app/modules/proxy/_service/support.py index bcd5a1637a..aa018afb56 100644 --- a/app/modules/proxy/_service/support.py +++ b/app/modules/proxy/_service/support.py @@ -1254,6 +1254,11 @@ class _HTTPBridgeSession: # timeouts). A quarantined session must never be selected for reuse or # re-attach; later requests take the fresh session/no-anchor path. quarantined: bool = False + # Durable anchor-poison settlement is owned by the retry circuit. These + # flags bridge the asynchronous fenced clear result back to reader and + # retirement paths without making either path count a second threshold. + anchor_poison_cleared: bool = False + anchor_poison_clear_failed: bool = False # Set while a reader handoff is replacing the socket. Idle pruning must # retain the registered session during this short transition even though # ``closed`` is fail-closed for normal request reuse. diff --git a/openspec/changes/unify-eventless-anchor-poison-settlement/design.md b/openspec/changes/unify-eventless-anchor-poison-settlement/design.md new file mode 100644 index 0000000000..330b5707ac --- /dev/null +++ b/openspec/changes/unify-eventless-anchor-poison-settlement/design.md @@ -0,0 +1,17 @@ +# Design + +The retry circuit owns the repeated eventless failure transition. When a hard +affinity key reaches the circuit threshold for a poison-class failure, it must +record one durable `OPEN` generation and obtain a fenced anchor-clear result +before allowing a recovery probe. Reader and terminal paths call the same +attempt-scoped recorder, so their ordering cannot change the outcome. + +If the clear is fenced out or unavailable, the key remains quarantined and the +durable circuit generation remains open. Anchor injection is denied while that +generation is unresolved; the next recovery attempt retries the fenced clear or +uses an explicitly unanchored full replay. Quarantine may be cleared only by a +completed response after the durable anchor has been confirmed absent. + +The implementation must preserve single-settlement ownership: one physical +response-create attempt contributes at most one strike, and a successful +`response.completed` resets the local and durable circuit state. diff --git a/openspec/changes/unify-eventless-anchor-poison-settlement/proposal.md b/openspec/changes/unify-eventless-anchor-poison-settlement/proposal.md new file mode 100644 index 0000000000..d384b0b638 --- /dev/null +++ b/openspec/changes/unify-eventless-anchor-poison-settlement/proposal.md @@ -0,0 +1,35 @@ +# Unify eventless anchor-poison settlement + +Source sidechat: `01a04ac6-27a1-74b3-88e0-33b4022fc1ed` + +Recovered from: `d1f4fa4ac3929e29bb531716a1964c39a5839793` + +## Why + +The HTTP bridge currently has separate reader-retirement and terminal-frame +paths for eventless failures. The retry circuit opens after two failures, but +the reader path still waits for the independent seven-failure anchor-poison +threshold. A reader failure followed by a terminal +`previous_response_not_found`, or the reverse order, can therefore open and +expire quarantine while the durable anchor remains available for reinjection. + +## What changes + +- Route every request-affecting, eventless, no-safe-replay failure through one + attempt-scoped settlement decision, regardless of whether the reader or + terminal path observes it first. +- Make the circuit-open transition atomically require durable anchor + abandonment and quarantine. A failed or fenced anchor clear keeps the key + fail-closed and prevents quarantine expiry from restoring the anchor. +- Remove the independent seven-strike reader-only poison decision; the circuit + threshold is the sole threshold for a repeated eventless poisoned anchor. +- Fence stale session writers and make poison settlement idempotent so a + concurrent reader/terminal callback cannot double-count or re-persist the + cleared anchor. + +## Impact + +The behavior is default-on and scoped to hard-affinity HTTP bridge sessions. +Successful completed responses still reset the circuit and quarantine. Clean +close and midstream/eventful failures remain excluded from eventless anchor +poisoning. No operator threshold tuning or migration is required. diff --git a/openspec/changes/unify-eventless-anchor-poison-settlement/tasks.md b/openspec/changes/unify-eventless-anchor-poison-settlement/tasks.md new file mode 100644 index 0000000000..14760529e0 --- /dev/null +++ b/openspec/changes/unify-eventless-anchor-poison-settlement/tasks.md @@ -0,0 +1,26 @@ +# Tasks + +## Specification + +- [x] Add requirements for mixed reader/terminal eventless ordering, atomic + circuit-open anchor settlement, and fail-closed clear failures. +- [x] Add scenarios for concurrent duplicate settlement and cooldown expiry. + +## Implementation + +- [x] Centralize poison settlement at the retry-circuit threshold. +- [x] Remove the reader-only seven-strike clear gate. +- [x] Fence anchor injection and quarantine expiry until durable clear succeeds. + +## Regression coverage + +- [x] Cover reader→terminal and terminal→reader `previous_response_not_found` + sequences. +- [x] Cover clear failure/fencing, duplicate callbacks, completed-response + reset, and clean-close exclusion. +- [x] Cover cooldown expiry with no stale anchor reinjection. + +## Verification + +- [x] Run bridge unit/integration tests, lint, type checks, and strict OpenSpec + validation. diff --git a/openspec/specs/responses-api-compat/spec.md b/openspec/specs/responses-api-compat/spec.md index 1e5472c160..5bdae5d33d 100644 --- a/openspec/specs/responses-api-compat/spec.md +++ b/openspec/specs/responses-api-compat/spec.md @@ -3963,12 +3963,23 @@ current and the failure is ordinary transient upstream silence. ### Requirement: Repeated zero-event idle failures poison dead anchors -For hard HTTP bridge keys, repeated zero-event idle failures MUST use the -existing durable retry-circuit counter to identify an anchor that should no -longer remain addressable. When consecutive failures for the same hard bridge -key reach the configured poison threshold, the proxy MUST abandon durable -continuity for that session and retire the bridge even when admission waiters -exist. The default threshold MUST be no greater than seven failures. +For hard HTTP bridge keys, every request-affecting eventless failure with no +safe replay MUST use the durable retry-circuit counter, regardless of whether +the reader, terminal-frame, stale-gate, or grouped-terminal path observes it +first. One physical response-create attempt MUST contribute at most one strike. +When consecutive failures for the same hard bridge key reach the retry-circuit +open threshold (two by default), the proxy MUST persist the open generation, +fenced-clear the durable continuity anchor, and quarantine the key before any +cooldown probe can reuse it. The independent seven-failure reader threshold +MUST NOT participate in anchor poisoning. + +If the fenced anchor clear fails, is unavailable, or is rejected by a newer +owner, the durable poison marker MUST remain open. Cooldown expiry or process +restart MUST NOT permit the old anchor to be injected again. A full-context +request MAY proceed as an explicitly unanchored replay; a delta-only request +MUST fail closed until the anchor is cleared or a completed response proves a +new healthy continuity state. A successful ``response.completed`` MUST clear +the poison marker and quarantine. #### Scenario: Admission waiters cannot defer anchor poisoning forever @@ -3982,6 +3993,40 @@ exist. The default threshold MUST be no greater than seven failures. - **AND** the next attach starts from fresh durable state rather than the poisoned previous-response anchor +#### Scenario: Reader and terminal failures share one poison threshold + +- **GIVEN** a hard bridge key has one eventless reader failure +- **WHEN** the next request receives an eventless terminal + `previous_response_not_found` failure before `response.created` +- **THEN** the second failure opens the same durable retry circuit +- **AND** the fenced continuity clear is attempted exactly once for that + circuit generation +- **AND** the reader-only seven-failure threshold is not required + +#### Scenario: Terminal and reader failures share one poison threshold + +- **GIVEN** a hard bridge key has one eventless terminal failure +- **WHEN** the next request fails eventlessly in the reader path +- **THEN** the second failure opens the same durable retry circuit +- **AND** the durable anchor is not re-injected after cooldown + +#### Scenario: Poison clear failure remains fail-closed after cooldown + +- **GIVEN** the fenced durable anchor clear fails or is rejected by a newer + owner when the circuit opens +- **WHEN** the cooldown expires or a new replica handles the next request +- **THEN** the durable poison marker remains observable +- **AND** a full-context request is sent without a proxy-injected anchor +- **AND** a delta-only request fails closed instead of replaying the old anchor + +#### Scenario: Duplicate callbacks settle one physical attempt once + +- **GIVEN** reader and terminal callbacks race for the same response-create + attempt +- **WHEN** both callbacks classify the outcome as eventless +- **THEN** the circuit records one strike +- **AND** durable anchor clear and poison telemetry are not duplicated + #### 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 diff --git a/tests/unit/test_proxy_http_bridge.py b/tests/unit/test_proxy_http_bridge.py index c676c7e522..ecd1ea5dc6 100644 --- a/tests/unit/test_proxy_http_bridge.py +++ b/tests/unit/test_proxy_http_bridge.py @@ -29343,13 +29343,13 @@ async def test_http_bridge_repeated_zero_event_idle_timeouts_poison_anchor_with_ monkeypatch.setattr(service, "_fail_pending_websocket_requests", fail_pending) monkeypatch.setattr(service, "_retire_stale_pending_http_bridge_session", retire) - for failure_number in range(1, 8): + for failure_number in range(1, 3): retired = await service._fail_http_bridge_reader_and_maybe_retire( session, error_code="stream_idle_timeout", error_message="idle timeout", ) - assert retired is (failure_number == 7) + assert retired is (failure_number == 2) durable_bridge.rebind_session_account.assert_awaited_once_with( session_id="durable-anchor-poison", @@ -29394,13 +29394,13 @@ async def test_http_bridge_repeated_zero_event_stream_incompletes_poison_anchor_ monkeypatch.setattr(service, "_fail_pending_websocket_requests", fail_pending) monkeypatch.setattr(service, "_retire_stale_pending_http_bridge_session", retire) - for failure_number in range(1, 8): + for failure_number in range(1, 3): retired = await service._fail_http_bridge_reader_and_maybe_retire( session, error_code="stream_incomplete", error_message="Upstream websocket closed before response.completed", ) - assert retired is (failure_number == 7) + assert retired is (failure_number == 2) durable_bridge.rebind_session_account.assert_awaited_once_with( session_id="durable-anchor-poison-stream-incomplete", @@ -29434,7 +29434,7 @@ async def test_http_bridge_retire_stale_pending_poisons_anchor_after_repeated_ev service._durable_bridge = durable_bridge monkeypatch.setattr(service, "_close_http_bridge_session_bounded", AsyncMock()) - for _failure_number in range(7): + for _failure_number in range(2): session = _make_bridge_session( key_value="bridge-anchor-poison-retire", pending_requests=deque([_make_eventless_http_bridge_owner()]), @@ -29462,9 +29462,9 @@ async def test_http_bridge_retire_stale_pending_reattempts_failed_poison_clear( caplog: pytest.LogCaptureFixture, monkeypatch: pytest.MonkeyPatch, ) -> None: - # A clear that cannot be confirmed must not lose the self-heal: the next - # eligible eventless failure at or above the threshold re-attempts it, and - # each failed clear stays visible in the poison-clear telemetry. + # A clear that cannot be confirmed must not lose the self-heal. The circuit + # keeps the key fail-closed through its cooldown, so an immediate burst of + # later failures does not hammer the fenced durable row. service = proxy_service.ProxyService(cast(Any, nullcontext())) durable_bridge = SimpleNamespace( lookup_retry_circuit=AsyncMock(return_value=None), @@ -29475,7 +29475,7 @@ async def test_http_bridge_retire_stale_pending_reattempts_failed_poison_clear( monkeypatch.setattr(service, "_close_http_bridge_session_bounded", AsyncMock()) with caplog.at_level(logging.INFO): - for _failure_number in range(8): + for _failure_number in range(4): session = _make_bridge_session( key_value="bridge-anchor-poison-clear-retry", pending_requests=deque([_make_eventless_http_bridge_owner()]), @@ -29488,8 +29488,8 @@ async def test_http_bridge_retire_stale_pending_reattempts_failed_poison_clear( detail="stream_incomplete", ) - assert durable_bridge.rebind_session_account.await_count == 2 - assert caplog.text.count("event=durable_anchor_poison_clear_failed") == 2 + assert durable_bridge.rebind_session_account.await_count == 1 + assert caplog.text.count("event=durable_anchor_poison_clear_failed") == 1 @pytest.mark.asyncio @@ -32405,6 +32405,33 @@ def fake_prepare( assert captured["previous_response_id"] is None +@pytest.mark.asyncio +@pytest.mark.parametrize("detail", ["stream_incomplete", "stream_idle_timeout"]) +async def test_durable_poison_marker_blocks_anchor_after_process_restart(detail: str) -> None: + """A durable open poison row outlives the in-memory quarantine registry.""" + service = proxy_service.ProxyService(cast(Any, nullcontext())) + key = proxy_service._HTTPBridgeSessionKey("session_header", "durable-poison-restart", None) + snapshot = SimpleNamespace( + consecutive_failures=2, + last_detail=detail, + cooldown_until_epoch=time.time() - 1, + updated_at_epoch=time.time(), + admission_generation=4, + ) + lookup = AsyncMock(return_value=snapshot) + service._durable_bridge = SimpleNamespace(lookup_retry_circuit=lookup) + + assert await service._http_bridge_retry_circuit_anchor_poisoned_for_key(key) is True + lookup.assert_awaited_once_with( + session_key_kind="session_header", + session_key_value="durable-poison-restart", + api_key_id=None, + ) + + snapshot.last_detail = "clean_close" + assert await service._http_bridge_retry_circuit_anchor_poisoned_for_key(key) is False + + def _make_terminal_error_bridge_fixture( *, request_id: str, @@ -32576,3 +32603,84 @@ async def test_two_eventless_terminal_errors_open_circuit_and_clear_anchor() -> clear_retry_circuit.assert_awaited_once() assert session.key not in cast(Any, service)._http_bridge_retry_circuits assert http_bridge_quarantine_module._http_bridge_session_key_quarantined(service, session.key) is True + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("first_detail", "second_detail"), + [ + ("stream_idle_timeout", "stream_incomplete"), + ("stream_incomplete", "stream_idle_timeout"), + ], +) +async def test_eventless_reader_terminal_order_uses_one_anchor_poison_settlement( + first_detail: str, + second_detail: str, +) -> None: + """Reader/terminal ordering must not reintroduce the seven-strike gap.""" + service = proxy_service.ProxyService(cast(Any, nullcontext())) + session = _make_bridge_session(key_value=f"mixed-poison-{first_detail}") + session.durable_session_id = "durable-mixed-poison" + session.durable_owner_epoch = 9 + rebind = AsyncMock(return_value=True) + clear_retry = AsyncMock(return_value=None) + service._durable_bridge = SimpleNamespace( + lookup_retry_circuit=AsyncMock(return_value=None), + persist_retry_circuit=AsyncMock(return_value=None), + rebind_session_account=rebind, + clear_retry_circuit=clear_retry, + ) + + first_attempt = proxy_support_module._HTTPBridgeResponseCreateAttempt(ordinal=1) + second_attempt = proxy_support_module._HTTPBridgeResponseCreateAttempt(ordinal=2) + assert ( + await service._record_http_bridge_retry_circuit_failure( + session, + detail=first_detail, + attempt=first_attempt, + ) + == 1 + ) + assert ( + await service._record_http_bridge_retry_circuit_failure( + session, + detail=second_detail, + attempt=second_attempt, + ) + == 2 + ) + + rebind.assert_awaited_once() + clear_retry.assert_awaited_once() + assert session.anchor_poison_cleared is True + assert session.key not in cast(Any, service)._http_bridge_retry_circuits + + +@pytest.mark.asyncio +async def test_eventless_duplicate_reader_and_terminal_callbacks_count_one_attempt_once() -> None: + service = proxy_service.ProxyService(cast(Any, nullcontext())) + session = _make_bridge_session(key_value="duplicate-eventless-attempt") + persist_retry = AsyncMock(return_value=None) + service._durable_bridge = SimpleNamespace( + lookup_retry_circuit=AsyncMock(return_value=None), + persist_retry_circuit=persist_retry, + ) + attempt = proxy_support_module._HTTPBridgeResponseCreateAttempt(ordinal=1) + + counts = await asyncio.gather( + service._record_http_bridge_retry_circuit_failure( + session, + detail="stream_idle_timeout", + attempt=attempt, + ), + service._record_http_bridge_retry_circuit_failure( + session, + detail="stream_incomplete", + attempt=attempt, + terminal_pre_response_frame=True, + ), + ) + + assert counts == [1, 1] + assert persist_retry.await_count == 1 + assert attempt.retry_circuit_failure_recorded is True