diff --git a/app/modules/proxy/_service/http_bridge/retry_circuit.py b/app/modules/proxy/_service/http_bridge/retry_circuit.py index e2642de2da..3223953d8c 100644 --- a/app/modules/proxy/_service/http_bridge/retry_circuit.py +++ b/app/modules/proxy/_service/http_bridge/retry_circuit.py @@ -258,7 +258,16 @@ async def _load_http_bridge_retry_circuit(self: Any, session: _HTTPBridgeSession return True cooldown_remaining = max(0.0, persisted.cooldown_until_epoch - now_epoch) - persisted_cooldown_until = now_monotonic + cooldown_remaining + # ``cooldown_until`` is a monotonic deadline whose zero means "this key + # is not cooling down". A durable row whose cooldown already elapsed -- + # or which never had one, because ``persist_retry_circuit`` writes + # ``now_wall`` for a below-threshold failure count -- must load as that + # zero. Loading it as ``now_monotonic`` instead makes it simultaneously + # non-zero and expired, which is exactly the condition + # ``_http_bridge_precreated_retry_allowed`` reads as "a cooldown just + # ended", so it burns a half-open probe lease on a key that was never + # cooling down and suppresses every other request for that lease. + persisted_cooldown_until = now_monotonic + cooldown_remaining if cooldown_remaining > 0.0 else 0.0 async with self._http_bridge_retry_circuit_lock: self._http_bridge_retry_circuit_persisted_keys.add(session.key) state = self._http_bridge_retry_circuits.get(session.key) @@ -269,10 +278,24 @@ async def _load_http_bridge_retry_circuit(self: Any, session: _HTTPBridgeSession if persisted.updated_at_epoch > state.persisted_updated_at_epoch and not local_failure_is_newer: state.consecutive_failures = max(0, persisted.consecutive_failures) state.cooldown_until = persisted_cooldown_until + if persisted_cooldown_until <= 0.0: + state.half_open_until = 0.0 state.last_detail = persisted.last_detail else: state.consecutive_failures = max(state.consecutive_failures, max(0, persisted.consecutive_failures)) - state.cooldown_until = max(state.cooldown_until, persisted_cooldown_until) + if ( + not local_failure_is_newer + and persisted.updated_at_epoch >= state.persisted_updated_at_epoch + and persisted_cooldown_until <= 0.0 + ): + # An equal-version durable reload can observe the same + # row after its cooldown elapsed. Clear the old local + # monotonic deadline instead of turning that expiry into + # a half-open probe. + state.cooldown_until = 0.0 + state.half_open_until = 0.0 + else: + state.cooldown_until = max(state.cooldown_until, persisted_cooldown_until) if local_failure_is_newer: state.last_detail = state.last_detail or persisted.last_detail else: diff --git a/openspec/changes/normalize-expired-retry-cooldown/.openspec.yaml b/openspec/changes/normalize-expired-retry-cooldown/.openspec.yaml new file mode 100644 index 0000000000..4102db8a47 --- /dev/null +++ b/openspec/changes/normalize-expired-retry-cooldown/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-24 diff --git a/openspec/changes/normalize-expired-retry-cooldown/proposal.md b/openspec/changes/normalize-expired-retry-cooldown/proposal.md new file mode 100644 index 0000000000..7336bef2d2 --- /dev/null +++ b/openspec/changes/normalize-expired-retry-cooldown/proposal.md @@ -0,0 +1,17 @@ +# Normalize expired durable retry cooldowns + +## Summary + +An expired durable retry-circuit cooldown must reload as an open circuit with +no cooldown, not as a newly ended cooldown that consumes a half-open probe. + +## What Changes + +- Map an elapsed or absent durable cooldown to the zero in-memory deadline. +- Preserve future cooldown deadlines and all existing threshold, persistence, + ownership, and backoff behavior. + +## Impact + +This is an internal retry-circuit admission fix. It adds no setting, schema, +wire-format, or operator action. diff --git a/openspec/changes/normalize-expired-retry-cooldown/specs/responses-api-compat/spec.md b/openspec/changes/normalize-expired-retry-cooldown/specs/responses-api-compat/spec.md new file mode 100644 index 0000000000..06b9d744d8 --- /dev/null +++ b/openspec/changes/normalize-expired-retry-cooldown/specs/responses-api-compat/spec.md @@ -0,0 +1,27 @@ +# responses-api-compat Delta + +## ADDED Requirements + +### Requirement: Expired durable retry cooldowns do not consume a half-open lease + +When a durable retry-circuit row has an absent or elapsed cooldown, the proxy +MUST load the in-memory cooldown deadline as zero. It MUST NOT interpret that +row as a cooldown that just ended, consume an exclusive half-open probe lease, +or suppress subsequent requests solely because the persisted cooldown elapsed. +Future cooldown deadlines MUST remain represented as a positive in-memory +deadline, with thresholds, backoff, persistence, ownership, and retry +classification unchanged. + +#### Scenario: Elapsed cooldown reloads open without a probe lease + +- **GIVEN** a hard-affinity durable retry row whose cooldown deadline is in the past +- **WHEN** retry admission loads the row +- **THEN** the in-memory cooldown deadline is zero +- **AND** the half-open lease is zero +- **AND** repeated admission checks remain allowed + +#### Scenario: Future cooldown remains enforced + +- **GIVEN** a hard-affinity durable retry row whose cooldown deadline is in the future +- **WHEN** retry admission loads the row +- **THEN** the positive cooldown remains enforced until it expires diff --git a/openspec/changes/normalize-expired-retry-cooldown/tasks.md b/openspec/changes/normalize-expired-retry-cooldown/tasks.md new file mode 100644 index 0000000000..395fedb0c7 --- /dev/null +++ b/openspec/changes/normalize-expired-retry-cooldown/tasks.md @@ -0,0 +1,15 @@ +## 1. Specification + +- [x] Add the expired-cooldown and future-cooldown requirements. +- [x] Validate the change in strict mode. + +## 2. Implementation + +- [x] Normalize non-positive durable cooldown remaining time to the zero + sentinel while preserving future deadlines. +- [x] Add regression coverage proving an elapsed row does not burn a + half-open probe lease. + +## 3. Verification + +- [x] Run focused retry-circuit tests, Ruff, and `git diff --check`. diff --git a/tests/unit/test_proxy_http_bridge.py b/tests/unit/test_proxy_http_bridge.py index a8b8fd5628..bdf1b0ed58 100644 --- a/tests/unit/test_proxy_http_bridge.py +++ b/tests/unit/test_proxy_http_bridge.py @@ -28314,6 +28314,97 @@ async def test_http_bridge_retry_circuit_counts_stuck_gate_timeout() -> None: assert state.cooldown_until > time.monotonic() +@pytest.mark.asyncio +async def test_http_bridge_retry_circuit_elapsed_durable_cooldown_does_not_burn_half_open_probe() -> None: + """An already-elapsed durable cooldown is not a cooldown that just ended.""" + service = proxy_service.ProxyService(cast(Any, nullcontext())) + hard_session = _make_bridge_session(key_value="bridge-circuit-elapsed-cooldown") + # ``persist_retry_circuit`` writes ``now_wall`` when the failure count is + # below the threshold, and a real cooldown simply elapses. Both leave a row + # whose ``cooldown_until_epoch`` is in the past. + service._durable_bridge = SimpleNamespace( + lookup_retry_circuit=AsyncMock( + return_value=SimpleNamespace( + consecutive_failures=2, + cooldown_until_epoch=time.time() - 120.0, + last_detail="stream_incomplete", + updated_at_epoch=time.time(), + ) + ), + persist_retry_circuit=AsyncMock(), + clear_retry_circuit=AsyncMock(), + ) + + assert await service._http_bridge_precreated_retry_allowed(hard_session) is True + state = cast(Any, service)._http_bridge_retry_circuits[hard_session.key] + assert state.cooldown_until == 0.0 + assert state.half_open_until == 0.0 + assert await service._http_bridge_precreated_retry_cooldown_seconds(hard_session) == 0.0 + # The decisive part: a key that was never cooling down must not lock every + # other request out behind a probe lease it never needed. + assert await service._http_bridge_precreated_retry_allowed(hard_session) is True + assert await service._http_bridge_precreated_retry_allowed(hard_session) is True + + +@pytest.mark.asyncio +async def test_http_bridge_retry_circuit_expiry_clears_loaded_local_deadline() -> None: + """A cooldown that expires after loading does not create a half-open lease.""" + service = proxy_service.ProxyService(cast(Any, nullcontext())) + hard_session = _make_bridge_session(key_value="bridge-circuit-expiry-transition") + persisted = SimpleNamespace( + consecutive_failures=2, + cooldown_until_epoch=time.time() + 60.0, + last_detail="stream_incomplete", + updated_at_epoch=time.time(), + ) + service._durable_bridge = SimpleNamespace( + lookup_retry_circuit=AsyncMock(return_value=persisted), + persist_retry_circuit=AsyncMock(), + clear_retry_circuit=AsyncMock(), + ) + + assert await service._http_bridge_precreated_retry_allowed(hard_session) is False + state = cast(Any, service)._http_bridge_retry_circuits[hard_session.key] + assert state.cooldown_until > time.monotonic() + + persisted.cooldown_until_epoch = time.time() - 120.0 + assert await service._http_bridge_precreated_retry_allowed(hard_session) is True + assert state.cooldown_until == 0.0 + assert state.half_open_until == 0.0 + assert await service._http_bridge_precreated_retry_allowed(hard_session) is True + + +@pytest.mark.asyncio +async def test_http_bridge_retry_circuit_expiry_clears_lookup_failure_probe() -> None: + """An expired row clears a probe leased during a transient lookup failure.""" + service = proxy_service.ProxyService(cast(Any, nullcontext())) + hard_session = _make_bridge_session(key_value="bridge-circuit-expiry-lookup-failure") + persisted = SimpleNamespace( + consecutive_failures=2, + cooldown_until_epoch=time.time() + 0.1, + last_detail="stream_incomplete", + updated_at_epoch=time.time(), + ) + lookup_retry_circuit = AsyncMock(side_effect=[persisted, RuntimeError("temporary lookup failure"), persisted]) + service._durable_bridge = SimpleNamespace( + lookup_retry_circuit=lookup_retry_circuit, + persist_retry_circuit=AsyncMock(), + clear_retry_circuit=AsyncMock(), + ) + + assert await service._http_bridge_precreated_retry_allowed(hard_session) is False + state = cast(Any, service)._http_bridge_retry_circuits[hard_session.key] + persisted.cooldown_until_epoch = time.time() - 120.0 + await anyio.sleep(0.15) + + assert await service._http_bridge_precreated_retry_allowed(hard_session) is True + assert state.half_open_until > time.monotonic() + assert await service._http_bridge_precreated_retry_allowed(hard_session) is True + assert state.cooldown_until == 0.0 + assert state.half_open_until == 0.0 + assert await service._http_bridge_precreated_retry_allowed(hard_session) is True + + @pytest.mark.asyncio async def test_http_bridge_submit_suppresses_hard_key_during_retry_cooldown() -> None: service = proxy_service.ProxyService(cast(Any, nullcontext()))