fix(subscriptions): a rate-limited turn completes on another subscription (#2638)

[dolho]

A Workspace message to an agent whose Claude subscription was rate-limited failed outright — "The agent has reached its usage limit and can't respond right now. Please try again later." — the message was lost to a FAILED execution, and the client was told the failure was not retryable.

Every SUB-003 mechanism was working: switch on the first 429 (#441), re-issue once (#792), rank by cached headroom (#2409). This closes the four gaps between them.

The four gaps

1+2 — the 2h skip-list is now overridable, per candidate, on evidence

list_viable_alternative_subscriptions drops any subscription with any failure event in a flat 2h window. That window is a proxy for "the provider is still refusing it", and on a two-subscription install a single stale event is the difference between a completed turn and a user watching their message die. #2320's own evidence was exactly this: "no viable alternative — the whole pool was exhausted", with an alternative sitting right there.

subscription_headroom_service.recovery_verdict overrides the exclusion on positive evidence only:

verdict	evidence
serving_now	a FRESH reading says the provider is not refusing this token — ground truth about now beats an inference from past failures (#447's rule), and it is the same evidence rank_subscriptions already trusts in the other direction when it drops a refusing candidate
window_reset	no fresh reading, but a BLOCKED window's own reset instant has elapsed and predates the failure
None	everything else

Three properties keep #444's ping-pong closed:

• Absence of evidence readmits nothing. bug(SUB-003): auto-switch ping-pongs between rate-limited subscriptions due to clear_rate_limit_events destroying detection signal #444 was caused by forgetting a failure; nothing here forgets one.
• A fresh refusal does not fall through to the instant arm — the strongest signal available points the other way.
• The failure must PREDATE the reset. Without that ordering a subscription that 429'd a minute after its window rolled over — i.e. one that is exhausted again — would be readmitted on a reset it had already consumed.

Instants are read from an aged snapshot on the asymmetry this codebase already states (#447/#2396): a utilisation number decays, an instant does not.

The fail-open ranking branch readmits nobody by construction — it is precisely the branch where the evidence could not be read, so the skip-list stands.

3 — the switch can happen BEFORE the first dispatch

SUB-003 was purely reactive, so the first message after a subscription hits its wall always burned a failed attempt — and on the Workspace that attempt is a person watching their message fail. Everything needed to avoid it was already known at dispatch time; nothing was reading it.

ensure_serviceable_subscription switches when the assigned subscription is already known not to serve: a fresh provider refusal, or a 429 in the platform's own 2h window. It uses the 429-only DISPLAY predicate (#2352) deliberately — an auth failure is a credential problem a different subscription may share, and quota exhaustion is the case a switch actually fixes.

Contracts: it never raises and never blocks; it records no failure event (nothing failed, and a synthetic one would poison the very skip-list that decides where the agent may move next); with no alternative it dispatches anyway (the provider's answer is better evidence than ours); and it performs the same _perform_auto_switch, so one activity, one notification and one hot-reload happen whichever path fired — AC #6.

1 last resort — the platform API key

When the switcher declines and a key is configured, fallback_to_api_key clears the subscription assignment, sets use_platform_api_key and restarts (the reload endpoint pushes an OAuth token; the change needed here is the opposite one, which lifecycle's auth block already derives from DB state — including #2114's shadowing guard). It clears rather than remembering-and-restoring: a hidden "go back at reset" would be a second invisible scheduler competing with the operator's own assignment.

Setting subscription_api_key_fallback, default ON, GET/PUT /api/subscriptions/settings/api-key-fallback, rendered in Settings → Subscriptions. The GET also returns key_configured: with the setting on and no key stored the fallback is enabled and inert, and a control showing only "on" would describe a remedy that cannot run. The read fails open — the failure it guards is a user's turn dying with a usable key sitting in settings.

4 — the client is told what changed

TaskExecutionResult.subscription_switch carries the switch (in-memory, like dispatched_async), stamped at execute_task's return sites. The portal's AUTH/BILLING branch consults it before refusing:

• switched → 503, category="auth_switched", retryable=True, naming the new subscription. bug(workspace): failed turns are invisible to the client — fast failures misreported as "lost track", backend error discarded, Retry suppressed where it is safe #2320's retryable=False rested on "re-sending re-fails", which holds only while nothing changed underneath — and a switch is exactly something changing underneath.
• not switched → still 502, but the body names the earliest reset instant the sampler already caches. "Please try again later" is true and nearly useless: the person cannot tell whether later means ten minutes or two days. Degrades to the original sentence when the instant is unknown — a fabricated time would be worse than a vague one.

One defect the tests found in the fix itself

Worth calling out because reading the diff would not have caught it. A window_reset candidate's reading carries a blocked flag describing the window that just rolled over, and rank_subscriptions drops a blocked candidate as refused — so the readmission was inert in exactly the case it exists for. The end-to-end test failed, not the unit table. Those readings are now handed to the ranker as UNKNOWN: the number is stale and the flag is about a quota that no longer exists.

Acceptance criteria

• A Workspace message on a rate-limited subscription completes on another subscription when any registered one can serve.
• Alternative selection uses the cached provider reset times instead of the flat 2h skip-list; the ping-pong regressions still hold (test_subscription_auto_switch_pingpong.py, 30 passed, unchanged).
• A known-refused assigned subscription is switched before the first dispatch — no failed attempt recorded.
• No subscription can serve → API-key fallback behind a Settings toggle (default on, honest key_configured); otherwise the error names the earliest known reset.
• The outcome carried to the Workspace includes the switch, and a turn that failed after one is reported retryable with the new subscription named.
• Every switch path writes the same activity + notification (all three converge on _perform_auto_switch).
• Gap 5 filed as bug(subscriptions): a pull-dispatched terminal never triggers SUB-003 — the switch hook is missing on apply_task_result #2643.

Verification

• tests/unit/test_2638_subscription_switch_on_turn.py — 37 passed. The verdict as a pure table (including the failure-after-reset ordering and the fail-closed unreadable-instant cases), readmission through the real selector, the fail-open path readmitting nobody, the pre-dispatch contracts, earliest_known_reset, the fallback's setting semantics, and three end-to-end turns through the real execute_task + real switcher: 429 → completes on a never-failed alternative, 429 → completes on a readmitted one, and the honest negative (nothing to switch to ⇒ still FAILED).
• test_2409_headroom_ranked_switch.py 87, plus test_447 / test_792 / test_2352 / ping-pong — 296 passed, byte-identical to dev across four pytest-randomly seeds.
• Frontend — 2385 passed (106 files).

Not an integration test against a live instance: a real 429 cannot be provoked from a provider on demand, so the seam actually under test — refusal in, completed turn plus switch out — is exercised where it can be deterministic.

Two things worth a reviewer's eye

• test_2409's test_only_survivors_are_ever_read was renamed to test_only_survivors_are_ranked and re-pointed at rank_subscriptions. The selector now also reads the skip-list's complement to decide readmission, so the old name asserted something that is deliberately no longer true; the property it protected (only survivors reach the ranker) is asserted directly. Its fixture also defaults the two new db reads to empty — a bare MagicMock is not iterable, and the resulting TypeError would have degraded the whole ranker to the load-balance fallback while every test still read as if it were exercising it.
• The new Settings toggle copies the file's existing raw-gray toggle classes rather than semantic tokens, for visual consistency with the three toggles beside it. SubscriptionsPanel.vue is not in raw-color-baseline.json, so nothing gates it — stating it rather than leaving it to be found.

Fixes #2638
Related to #2643

🤖 Generated with Claude Code

https://claude.ai/code/session_01Bd71qsYbFodvofba8P69eP

[vybe]

merge-train 2026-09-09: not on this train. AC 1 — the turn completing on another subscription — is genuinely fixed and E2E-tested. Two findings on the rest, both verified against the source.

1. The client-facing half is inert for the 429 path that produced the bug — src/backend/services/task_execution_service.py:2260-2263

error_code = None
if agent_status_code == 503:
    error_code = TaskExecutionErrorCode.AUTH

A Claude subscription usage limit surfaces from the agent as 429, not 503. Workspace turns are triggered_by="public", which is not async-eligible, so they take the sync path into _handle_http_error, where a 429 leaves error_code = None. TaskExecutionErrorCode.BILLING is never assigned anywhere in src/backend — I grepped every .py file at head; it appears only in the enum definition, in comments, and in the portal's gate tuple. The gate at client_portal/service.py:2722 is if code in ("AUTH", "BILLING"): with no substring fallback, so neither the auth_switched arm nor _usage_limit_detail runs. The turn still lands as generic agent_error.

So AC 4/5 — "the client is told what changed", which is what the title advertises — does not fire for the symptom in the title. It fires only when the failure happens to surface as 503.

2. The switch-off predicate re-opens the #447 OR — src/backend/services/subscription_auto_switch.py::_assigned_subscription_is_refused

A fresh reading saying not refusing does not stop the fall-through to is_subscription_rate_limited, a raw COUNT(*) > 0 over 2h. That is fresh_refusing OR db_predicate, the shape resolve_rate_limited_now exists to replace. The readmit side (recovery_verdict) gives the fresh reading priority, so the two directions now disagree: an agent whose subscription has a ≤2h-old 429 event but which the provider is demonstrably serving gets evacuated on every dispatch, with a hot-reload and a high-priority notification each time. With two such subscriptions it flaps turn after turn. The PR's "absence of evidence readmits nothing" defence guards the readmit door only.

Smaller things worth folding in: _with_switch is missing at two of six execute_task return sites (BackendAgentCallBudgetExhausted and the generic except Exception, both reachable after a pre-dispatch switch); a turn can now perform two switches because the pre-dispatch path never sets subscription_switch_attempted; and the E2E negative asserts status == FAILED but never error_code, which is the one assertion that would have caught (1).

The CodeQL alert is a false positive and you handled it correctly by fixing rather than dismissing — alert #351 is fixed on the merge ref. The +8 raw-gray baseline growth is argued in its own commit and is acceptable as written. Body says Related to #2638 — please make it Fixes #2638.

Rides the next train once fixed.

[dolho]

All four addressed in 16aa6037. Body now says Fixes #2638.

1. The 429 path. You were right and it was worse than "inert": TaskExecutionErrorCode.BILLING had no assignment site anywhere in the backend — enum, comments, the portal's own gate tuple, never set. _handle_http_error now classifies 429 → BILLING, so the auth_switched arm and _usage_limit_detail fire on exactly the path a Workspace turn takes. Downstream checked rather than assumed: the dispatch breaker counts auth only (#526 D10) so a quota 429 still cannot trip it, and the #1085 governor does count billing — which is what it was written for ("a fleet-wide Claude-API 429 storm"), has never been reachable from the sync path until now, and is behind a default-OFF flag.

2. The #447 OR. _assigned_subscription_is_refused is three-state now: a fresh reading ends the question in both directions, and only the absence of a usable reading falls through to the 2h event. An unreadable snapshot still falls through rather than clearing, so a Redis blip cannot silently disable the arm. The test I care most about is test_it_agrees_with_recovery_verdict_on_the_same_reading — it feeds one reading to both doors and asserts they agree, because checking each in isolation is how the two came apart.

3. One remediation per turn. The pre-dispatch arm sets subscription_switch_attempted. Worth noting explicitly since it is a second-order effect: the except handler reads the same flag, so after a pre-dispatch switch a subsequent 429 also records no second failure event — which is the rule already stated at that flag's other read site ("a second switch would burn another rate-limit event and churn to a third never-used subscription"), now actually true for both paths.

4. _with_switch. Both missing sites wrapped. Guarded by an AST test over execute_task's return nodes rather than a behavioural test of today's six, since the failure mode is a return site added later; the two legitimate bare returns (admission_denied, breaker_denied, both before any dispatch) are named in the allowlist so a third has to be argued for.

Each fix fails its test against the pre-fix source — verified by reverting them one at a time, not by inspection. 1008 passed, 2 skipped across every subscription / task-execution / portal / headroom test in the suite.

subscription-auto-switch.md records all four, including why BILLING never existed and why the two doors must be read from one predicate.

[dolho]

Self re-review — found one defect in my own fix, now fixed in b1d0ca5f.

Making _assigned_subscription_is_refused three-state was the right correction, but I got the bound wrong, and the bound matters as much as the direction here.

The reading came from cached_headroom_readings([subscription_id]) with no max_age_seconds — so the default, which is the selection bound MAX_READING_AGE_SECONDS (max(2*3600, 2*SAMPLE_INTERVAL), so ≥ 2h). That default is calibrated for ranking candidates, where a stale reading beats none. This call does a different job: it decides whether a provider verdict may overrule the 2h event predicate — and a reading as old as the window it overrules cannot.

Concretely: a two-hour-old "serving" snapshot could suppress a five-minute-old 429 event and pin an agent on a subscription that is refusing it right now. #447's rule is "a probe is ground truth about now"; I applied it to a probe that is no longer about now.

The bound it should have asked for was already stated twice in the file it reads from:

• _headroom_indicates_healthy — the display-side twin making the identical judgement — gates on age > FRESHNESS_SECONDS (30 min);
• REFUSAL_FRESHNESS_SECONDS = FRESHNESS_SECONDS, whose comment is exactly this rule for the mirror case: "a refusal is trusted exactly as long as the LIMIT badge trusts one, then it is merely unknown."

So max_age_seconds=FRESHNESS_SECONDS. This tightens the refusing arm as well — deliberate and safe: a stale refusal now falls through to the event predicate rather than evacuating on its own.

On the tests, because the first version could not have caught this. _with_reading returned the reading regardless of what bound the caller asked for, which makes the distinction structurally untestable — the same trap the E2E harness in this file already documents for the readmission path ("a fake that ignores max_age_seconds hands the selector a fresh refusal"). The fixture now models the age gate, and there are three cases: a stale serving verdict falling through to the event predicate, a display-fresh one still winning, and the bound asserted as the argument — because omitting it is the bug, and a behavioural test alone would go green again the day the default changes. Both new cases red against the unbounded call; verified by reverting the one argument.

49 passed. The rest of the earlier review (429 → BILLING, the one-remediation budget, _with_switch at every return site) re-read clean.

[dolho]

Second note from the same pass — this PR no longer merges cleanly, and one of its stated boundaries is now false.

$ git merge origin/dev
CONFLICT (content): src/backend/services/subscription_auto_switch.py
CONFLICT (content): docs/memory/feature-flows/subscription-auto-switch.md

The conflicting change is 294782b4 — #2651 (fixing #2643), already on dev: "a pull-dispatched terminal triggers SUB-003". It adds a spawned-switch path to the same module, with its own strong-ref set and a no-running-loop branch.

That matters beyond the textual conflict, because this PR's Not covered section says:

Pull-dispatched terminals still never trigger SUB-003 — pull_coordination_service.apply_task_result has no switch hook … Filed as #2643; inert today because no agent is piloted onto the pull path.

That was true when written and is not true now. Rebasing has to do two things, and only the first is mechanical:

1. resolve the code conflict, and check the interaction rather than assuming there is none — dev's pull path now calls into the switcher, and this PR changed the switcher's refusal predicate and the one-remediation budget. Specifically: the pull sink's spawned switch does not carry an _AttemptState, so subscription_switch_attempted cannot bound it, and the pre-dispatch arm this PR added is on the push path only. Worth stating explicitly in the doc which budget applies on which path rather than leaving it to be inferred;
2. delete the Not covered claim, or the merged doc asserts a gap that dev already closed.

Not a defect in the change itself — but it is exactly the kind of stale-boundary claim the flow docs are read for, so it should not land as written.

[github-actions]

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

[github-actions]

⚠️ Live-instance suite skipped — merge conflict against dev.

Resolve by merging dev locally and pushing the result; the next nightly re-tests.

[vybe]

merge-train 2026-09-10 (evening run): not on this train. Everything the 09-09 ejection asked for is verified fixed by execution, not by reading the description: 429 → BILLING on the sync path a Workspace turn takes (test_with_nothing_to_switch_to_the_turn_still_fails drives the real execute_task), the portal auth_switched arm fires for both the switched and the API-key-fallback case, the breaker still counts auth only, the governor's billing count stays behind its default-OFF flag, and the #2651 pull-sink hook is coherent with the changed predicate. Credit for all of that.

❌ The blocker — readmission re-opens the ping-pong the PR says it keeps closed

src/backend/services/subscription_auto_switch.py _readmit_recovered, and subscription_headroom_service.py recovery_verdict.

fresh = headroom.cached_headroom_readings(ids)          # no max_age → MAX_READING_AGE_SECONDS (≥ 2h)
...
if fresh is not None and not fresh.refusing:
    return RECOVERY_SERVING_NOW                          # no "reading postdates the failure" check

b1d0ca5f bounded the evacuation door at FRESHNESS_SECONDS with the rationale "a reading as old as the window it overrules cannot overrule it". The readmission door still reads at the selection bound, and its serving_now arm has none of the ordering the window_reset arm right below it has. Reproduced by feeding one snapshot to both doors on the merged tree:

reading age=6000s, 429 event 5 min ago
  readmission → 'serving_now'   evacuation on the SAME subscription → 'recent_rate_limit'
reading age=600s, 429 event 2 min ago
  readmission → 'serving_now'   evacuation → None

Case 2 is the default configuration and the scenario that motivated the PR: two subscriptions both at the wall, each with a pre-wall "ok" snapshot (the normal state for up to one refresh interval after a wall). Pre-PR: A 429 → switch to B → B 429 → honest FAILED, no churn. Post-PR: every user turn readmits the other subscription on its pre-failure reading → hot-reload + notification + failure event + FAILED + a retryable=True "send that again" that cannot succeed, flapping A↔B until a probe records rate_limited.

Fix needs your intent, which is why it is not a train patch: bound fresh at FRESHNESS_SECONDS and require the reading to postdate last_failure_at for serving_now (mirroring window_reset), or drop serving_now readmission and keep window_reset only.

Why CI is green

tests/unit/test_subscription_auto_switch_pingpong.py installs a headroom_stub ModuleType without RECOVERY_INSTANT_MAX_AGE_SECONDS / recovery_verdict. On the merged tree all four service-level selector calls log [#2409] headroom ranking unavailable (AttributeError …) and take the fail-open branch, so "the ping-pong regressions still hold, 30 passed" never reaches ranking or readmission. It is the same silent inertness you fixed in test_2409's fixture. Please give this suite the same treatment; it is the one that should have caught the above.

Mechanical, ride the same push

• docs/memory/feature-flows/subscription-auto-switch.md:340-342 still says pull-dispatched terminals never trigger SUB-003, four lines above the #2643 section that documents the hook; docs/memory/requirements/security.md:171 repeats it.
• src/frontend/raw-color-baseline.json conflicts with dev only in the notes block (_2662_* vs "2638"); keep both. SubscriptionsPanel.vue 165 → 173 is named and matches the scanner on the merged tree.

Non-blocking, for your judgement: fallback_to_api_key runs with auto_switch_subscriptions OFF (the pre-dispatch arm honours the setting, the more invasive remediation does not); and after a fallback restart the #792 retry most likely lands as a transport error, so the API-key case reaches the user as a generic 502 rather than the auth_switched copy.

Rides the next train once the readmission bound is decided.

[dolho]

Addressed the 2026-09-10 ejection — pushed 3ca6a48 + a dev merge (1e0176f).

Blocker — readmission ordering. Took option 1 (keep serving_now, make it honest):

• recovery_verdict: the serving arm readmits only when the reading postdates last_failure_at (reading instant = now − age_seconds), mirroring the window_reset arm; no failure instant, or an unparseable one, readmits nothing on that arm (fail closed). A pre-failure "ok" falls through to the instant arm, which orders itself.
• _readmit_recovered: the fresh read is bounded at FRESHNESS_SECONDS — the same bound the evacuation door uses — not the ≥2h selection bound.
• Both repro cases pinned at the pure verdict and through the selector (test_a_serving_reading_taken_BEFORE_the_failure_readmits_nothing, test_a_pre_wall_reading_does_not_readmit_at_the_selector), plus the unorderable-failure case. The "doors agree" invariant is restated with the failure predating the reading, and the one permitted disagreement is pinned in the direction that cannot flap (both doors shut).

Inert suite. test_subscription_auto_switch_pingpong.py's headroom stub now carries FRESHNESS_SECONDS, RECOVERY_*, recovery_verdict, and a name-parity assertion over every headroom.<name> the selector reads — so it fails rather than taking the fail-open branch. Verified: 0 [#2409] headroom ranking unavailable lines in its run now.

Mechanical. subscription-auto-switch.md "Not covered" + requirements/security.md:171 no longer claim pull terminals never trigger SUB-003; raw-color-baseline.json merged with both notes blocks kept (ratchet green on the merged tree).

Left as-is (judgement items): fallback_to_api_key still runs with auto_switch_subscriptions OFF, and the API-key case still reaches the user as a 502 rather than the auth_switched copy — both real, both separate from the readmission bug; happy to file them if you want them tracked.

332 subscription-suite tests green on the merged tree.

[vybe]

merge-train 2026-09-11: on this train. The 2026-09-10 blocker is fixed and I proved it by mutation rather than by reading: replacing if read_at > failed_at with if True in recovery_verdict reds four tests (test_a_serving_reading_taken_BEFORE_the_failure_readmits_nothing, test_a_pre_failure_serving_reading_still_falls_through_to_the_reset_arm, TestReadmission::test_a_pre_wall_reading_does_not_readmit_at_the_selector, test_a_pre_failure_reading_closes_the_readmit_door_without_opening_evacuation); failing open on a missing last_failure_at reds one more; and 429 → BILLING is driven through the real execute_task. 171/171 in the PR's suites, no schema change, the subscription_api_key_fallback key passes the ent#435 sink guard. I added the ui label (frontend touched; e2e already ran green by path).

One same-class finding, non-blocking — it's copy-only and a fix wants its own test, so it's yours as a follow-up rather than a fourth ejection:

earliest_known_reset can render an already-passed reset instant. src/backend/services/subscription_auto_switch.py:561-590 takes min() over blocked-window resets_at from a snapshot up to 7 days old with no check that the instant is still in the future, and client_portal/service.py:484-520 _usage_limit_detail renders it. Verified by execution: a 4 h-old snapshot with a reset 3 h ago renders "Its quota resets at 13:06 UTC on 11 Sep" at 16:06 UTC — on exactly the branch that fires when the subscription is refusing now. A parse_iso_timestamp(i) > now filter with the "try again later" fallback closes it. Realistic mainly with ambient refresh off or a sampler outage.

Smaller notes: tests/unit/test_2320_portal_failed_turn_visibility.py:327 is the closed-set guard whose docstring says its job is to name a new retryable category for a turn that ran — this PR adds exactly that (auth_switched, retryable=True) and the guard stays green only because _Result never carries subscription_switch; a TERMINAL_SITES row + the expected set would make it bite. The cancelled-branch comment in service.py ("only retryable verdicts are the ones where nothing reached the agent") is now stale. And the flow doc still doesn't state that the #2643 pull-sink switch carries no _AttemptState — the item your own 09-10 note said to state.

[vybe]

merge-train: batch validated on train/20260911-1635 (#2729)