Skip to content

fix(proxy): clear expired retry cooldown transitions - #1908

Open
JustYannicc wants to merge 4 commits into
Soju06:mainfrom
JustYannicc:fix/half-open-cooldown-beta4
Open

fix(proxy): clear expired retry cooldown transitions#1908
JustYannicc wants to merge 4 commits into
Soju06:mainfrom
JustYannicc:fix/half-open-cooldown-beta4

Conversation

@JustYannicc

@JustYannicc JustYannicc commented Aug 24, 2026

Copy link
Copy Markdown

Problem

A durable retry-circuit cooldown that expired after being loaded could leave a stale positive monotonic deadline in local state. Admission interpreted that expiry as a half-open transition and consumed the exclusive probe lease, suppressing later requests. A transient lookup failure could also create that probe lease; a subsequent same-version expired row cleared only the cooldown and left the lease active.

What this fixes

Expired durable cooldowns now clear equal-version stale local cooldown and half-open deadlines when no newer local failure exists. Newer local failures and newer persisted state remain authoritative.

What is now possible

After a persisted cooldown expires, including recovery from a transient lookup failure, ordinary retry admission remains open without creating or retaining a half-open probe lease. Future cooldowns continue to be enforced.

Tests

  • 37 retry-circuit tests passed
  • Ruff check passed
  • Ruff format check passed
  • ty check passed
  • Proxy architecture checks passed
  • Strict OpenSpec validation passed: normalize-expired-retry-cooldown
  • git diff --check passed

Dependencies

Based directly on beta.4 upstream main b311aea. No dependency on PR #1867; the branches are independent. This PR changes only retry-circuit state merge logic and its regression tests.

A persisted retry-circuit row with an elapsed (or absent) cooldown currently reloads its deadline as a non-zero monotonic timestamp in the past. The admission check interprets that state as a cooldown that just ended, consumes the exclusive half-open lease, and suppresses subsequent requests for the lease duration even though no cooldown remains. Normalize non-positive remaining durable cooldowns to the zero sentinel while preserving future deadlines. Add a regression proving elapsed rows do not burn a lease; thresholds, backoff, persistence, and ownership behavior remain unchanged.
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6a9026a6-70c2-4946-84dd-f428862e5d13

📥 Commits

Reviewing files that changed from the base of the PR and between b311aea and 32cf3aa.

📒 Files selected for processing (6)
  • app/modules/proxy/_service/http_bridge/retry_circuit.py
  • openspec/changes/normalize-expired-retry-cooldown/.openspec.yaml
  • openspec/changes/normalize-expired-retry-cooldown/proposal.md
  • openspec/changes/normalize-expired-retry-cooldown/specs/responses-api-compat/spec.md
  • openspec/changes/normalize-expired-retry-cooldown/tasks.md
  • tests/unit/test_proxy_http_bridge.py

Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The retry circuit now restores elapsed or absent durable cooldowns as an open state with no cooldown or half-open lease. Equal-version reloads clear stale local deadlines. Specifications and unit tests document and verify the behavior.

Changes

Retry cooldown normalization

Layer / File(s) Summary
Cooldown normalization contract
openspec/changes/normalize-expired-retry-cooldown/*
The specification defines zero cooldown handling for absent or elapsed durable cooldowns. Future cooldowns remain enforced.
Retry-circuit state reconciliation
app/modules/proxy/_service/http_bridge/retry_circuit.py
State loading maps elapsed cooldowns to 0.0. Equal-or-newer records without active cooldowns clear stale local deadlines.
Cooldown expiry regression coverage
tests/unit/test_proxy_http_bridge.py
Tests verify expired cooldown restoration, expiry cleanup, repeated admission, and the absence of unnecessary half-open leases.

Estimated code review effort: 2 (Simple) | ~15 minutes

Merge Risk: ⚪ Minimal · up to 32cf3

This localized change clears expired retry cooldown state while preserving newer failures and persisted state; the supplied checks pass, and no actionable merge-blocking risk remains beyond normal review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: clearing expired retry-circuit cooldown transitions in the proxy.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@JustYannicc

Copy link
Copy Markdown
Author

Follow-up on the expiry-transition review: commit ed8ee12 also clears a stale half-open lease when an equal-or-newer expired durable snapshot arrives after a transient lookup failure. Added regression coverage for future row -> lookup failure/lease -> same-version expired row. The branch is based directly on beta.4 main b311aea; no #1867 dependency. Affected gates pass: 37 retry-circuit tests, Ruff, format, ty, architecture, diff check, and strict scoped OpenSpec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant