Repository navigation
fix(route-proxy): known-100% seating gate — exclude pairs with a future quota reset (#5295) - #5296
Conversation
![developing-today[bot]](https://s.lequ.pw/_assets/images/gravatars/gravatar-user-420.png){kind=avatar}
…re quota reset (#5295) Implements #4370 acceptance (c) from triage task 8950. Demotion only reorders a pair after >=3 quota failures in 4h, so a leg whose binding window was ALREADY known exhausted (e.g. a monthly-walled leg) stayed a first-choice candidate until it killed three more sessions. - knownExhaustedPairs(now): the latest quota-bearing ledger failure for a pair whose detail carries a future reset (Retry-After-derived retryAfterMs from classifyHttp, or an absolute resetsAt/resetAt/retryAt field), not cleared by a later success. 40-day lookback covers monthly resets; the ledger is the only source — no new network dependency. - applyKnownExhaustedGate runs right after applyDemotion, following the #3228 credential-less exclusion pattern. Fail-safe: if the gate would empty the candidate list, the demoted order is kept (an exhausted leg as last resort beats no route). Decisions are logged to stderr, the proxy diagnostics stream. - The gate clock is nowMs(), so OPENCODE_ROUTE_PROXY_NOW_MS pins it for tests and incident replay. - Tests (node --test, 31/31 pass): exclusion while a live pair exists (ledger extract asserted: the t-ordered walk records exactly one attempt, leafB, beside the seeded leafA quota row), return after reset, all-exhausted bypass, and success-clears-gate.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 21 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f0bef24e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const wantStream = Boolean(body.stream); | ||
| let candidates = resolveRoute(cfg, routeId, hooks.variantIn ?? null); | ||
| candidates = applyDemotion(candidates, demotedPairs()); | ||
| candidates = applyKnownExhaustedGate(candidates); |
There was a problem hiding this comment.
When a route contains a known-exhausted credentialed leaf and a non-exhausted leaf without credentials, this gate removes the credentialed leaf because another candidate survives. The subsequent hasCredential filter then removes the remaining leaf, making the request fail with no credentialed members instead of retaining the exhausted leaf as the intended last resort. Apply credential filtering before this gate, or evaluate the gate's empty-list fallback against credentialed candidates.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e1a9504: the gate fail-safe is now evaluated against credentialed survivors — applyKnownExhaustedGate takes cfg and bypasses (keeping the demoted order) unless at least one kept candidate has a credential, so the later #3228 filter can no longer empty the route out from under the gate. New regression test: "gate bypassed when the only survivor has no credential" (leafA exhausted + leafNoCred survivor → LEAF-A still serves, bypass logged). Full suite 33/33.
| } | ||
| if (!q || typeof q !== "object") return null; | ||
| if (Number.isFinite(q.retryAfterMs) && q.retryAfterMs > 0) | ||
| return timeStarted + q.retryAfterMs; |
There was a problem hiding this comment.
When a leaf takes time to return its 429, adding its retryAfterMs to the request's start time shortens the exclusion by the entire upstream response latency. For example, a request that takes 30 seconds to return Retry-After: 30 is immediately eligible again, even though the provider requested another 30 seconds of backoff. runCompletion records time_ended when the response arrives; select and use that timestamp for relative retry delays so slow quota responses do not bypass the new seating gate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e1a9504: relative resets now anchor to the attempts time_ended (quota response receipt) — knownExhaustedPairs selects time_ended and quotaResetMs adds retryAfterMs to it. New pinned-clock regression test: a 429 that took 90s to arrive with Retry-After 60s is still excluded 30s after receipt, where start-anchoring would have re-eligible it immediately. Full suite 33/33.
…ck, receipt-anchored retry delay - The gate fail-safe is now evaluated against credentialed survivors: the #3228 credential-less filter runs after the gate, so a kept list whose only members lack credentials would still have emptied the route and failed the request instead of retaining the exhausted leaf as last resort. New test: gate bypassed when the only survivor has no credential. - Relative quota resets are anchored to the attempt's time_ended (when the 429 was received), not time_started, so a slow quota response no longer consumes its own Retry-After backoff. New pinned-clock test with a 90s response time.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1a9504126
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const successAt = latestSuccess.get(key); | ||
| if (successAt !== undefined && successAt > r.time_started) continue; |
There was a problem hiding this comment.
When requests to the same pair overlap, request-start ordering can clear a quota failure using a success that occurred before that failure was received. For example, if a request starts at t0 and returns a 429 at t3, while another starts at t1 and succeeds at t2, this comparison ignores the fresh quota failure because t1 > t0. The pair then remains eligible throughout its promised backoff despite having no success since the quota response. Use completion timestamps consistently when selecting the latest quota failure, aggregating successes, and deciding whether a success clears the gate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in the completion-ordering commit (see latest push): knownExhaustedPairs now selects the latest quota failure by time_ended, aggregates successes by MAX(time_ended), and clears the gate only when a success completed after the quota response was received. New pinned-clock regression test: an overlapping success that started after the failing request but completed a minute before its 429 arrived does not clear the gate (LEAF-B serves, leafA untouched). Full suite 34/34.
… completion time Overlapping requests made start-time ordering unsound: a success that started after a failing request but completed before its 429 arrived would clear the gate. knownExhaustedPairs now selects the latest quota failure by time_ended, aggregates successes by MAX(time_ended), and clears only when a success completed after the quota response was received. New pinned-clock regression test for the overlap case. Suite 34/34.
Fixes #5295
Implements #4370 acceptance (c) (triage task 8950; queue task 9239): a seating-time gate so sessions are not seated on a leg whose binding quota window is already known to be at 100%.
Problem
applyDemotionis reactive (needs ≥3 quota failures inside 4h in the route_attempt ledger) and reorder-only — demoted pairs sink but stay candidates. A leg known exhausted before any failure (the steward/quota state said 100% with a future reset; on 2026-11-01 a monthly-walled cline-pass leg killed 11 sessions in a day) remained a normal first-choice candidate until it had killed three attempts.Change (
bin/opencode-route-proxy.mjsonly)knownExhaustedPairs(now)reads the route_attempt ledger — the only source, no new network dependency in the hot path: a (provider, model) pair is known-exhausted when its latest quota-bearing failure carries a future reset — the Retry-After-derivedretryAfterMsthatclassifyHttpalready records for 429s, or an absoluteresetsAt/resetAt/retryAtfield — and no success has been recorded for the pair since that failure. A quota record with no reset (e.g. a barepaymentRequired) cannot gate; demotion remains its handling. 40-day lookback covers monthly resets.applyKnownExhaustedGateruns immediately afterapplyDemotion, following the opencode-route-proxy: opendesign leaves sit in pools but the provider's credential is inactive — walks burn attempts and surface a misleading auth error #3228 credential-less exclusion pattern. Fail-safe: if exclusion would empty the candidate list, the demoted order is kept — a known-exhausted leg as last resort beats no route. Gate decisions (exclusions with their reset instant, and bypasses) are logged to stderr, the proxy's diagnostics stream.nowMs(), soOPENCODE_ROUTE_PROXY_NOW_MSpins it for tests and incident replay. Demotion thresholds are unchanged (out of scope per the packet), as are the in-stream shedding (task 8932) and per-leg transport timeouts (companion fleet session deaths rotated legs: provider.transport timeouts on the gpt-6-luna leg and cline-pass leg wall/rate-limit deaths are not tracked by #3325 #4370(b)).Tests (
node --test bin/opencode-route-proxy.test.mjs— 31/31 pass)Four new tests, all with the clock pinned via
OPENCODE_ROUTE_PROXY_NOW_MS:leafA.hits == 0, and the ledger extract for routet-orderedcontains exactly one attempt (leafB) beside the seeded leafA quota row; the exclusion is in the proxy diagnostics log. One failure suffices — demotion's ≥3 is not required.Existing suite (windows, provider routes, credential-less skip, deadlines) unchanged and green.
Agent note: queue task 9239, lane code-c (repo-worker-code). Laptop worktree
~/git/fleet/9239; no sandbox builds.Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.