Skip to content
Open
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
27 changes: 25 additions & 2 deletions app/modules/proxy/_service/http_bridge/retry_circuit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-08-24
17 changes: 17 additions & 0 deletions openspec/changes/normalize-expired-retry-cooldown/proposal.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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
15 changes: 15 additions & 0 deletions openspec/changes/normalize-expired-retry-cooldown/tasks.md
Original file line number Diff line number Diff line change
@@ -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`.
91 changes: 91 additions & 0 deletions tests/unit/test_proxy_http_bridge.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
Expand Down
Loading