fix(retry-fallback): restoring the primary waits for its own account hold (#3347)

Merged
CO
6d ago
+289-9

Fixes #3347
Part of code-yeongyu/senpi#4090

Summary

After a usage-limit fallback, the session returns to its primary only once the primary's own account hold has ended. Before this, it returned when the shared circuit closed (at most 30 minutes). With a longer named reset, it then hit the same spent account and fell back again at every circuit window.

Root cause

  • packages/coding-agent/src/core/retry-fallback/controller.ts maybeRestorePrimary (lines 137-138 on main): the guard asks UnusableEntries.isUnusable about the model the session is on (the stranded clause), never about the model it restores to. Recovery was decided by originalRecovered alone.
  • originalRecovered follows the circuit when the breaker governs the entry, and the circuit caps an open at DEFAULT_CIRCUIT_MAX_COOLDOWN_MS (30 min, circuit.ts:22). UnusableEntries holds the spent account (account:<provider>) for the reset the error named, on the session cooldowns (unusable.ts:23-31).

The fix adds this.unusable.isUnusable(model) to the restore guard. The stranded-fallback exception (senpi#2376) still wins. A hold with no parsed reset uses the default cooldown, so those entries keep their usual half-open cadence (#2446). Only a reset the account named itself is waited out.

Login / account change drops the hold (review fold)

A review reproduced that a weekly limit kept the session on the fallback until the 48 h cap even after the user followed the refusal's own advice (/login, add an account): nothing cleared the session's hold.

  • RetryFallbackController.noteProviderAccountsChanged(provider) clears that provider's account: hold, its recorded error and the exhausted: entries of its models (UnusableEntries.clearProvider).
  • followProviderAccountChanges(controller) feeds it every accounts_changed event; AgentSession follows from construction and stops on dispose().
  • A successful login now emits accounts_changed for its provider on both login paths: ModelRuntime.login (interactive) and the RPC login_start handler, which used to emit only for Anthropic Subscription.
  • Tests (same file): drops the hold when the primary's provider accounts change (a login or an added account) (restores at 31 min after the event) and keeps the hold when another provider's accounts change. RED at 6ed1d4cd51: both fail with TypeError: followProviderAccountChanges is not a function.
  • GREEN at 4a96d5548a: CI=1 bun run test -- test/suite/regressions/3347-restore-waits-for-primary-hold.test.ts test/suite/rpc-auth-and-connection-handler.test.ts test/suite/regressions/3591-account-order-storage.test.ts: 3 files, 28 tests passed. bun check (no type errors), the changelog gate and git diff --check pass.

RED -> GREEN (original fix)

  • RED (commit fccce7dd09, test only): test/suite/regressions/3347-restore-waits-for-primary-hold.test.ts wires the controller with the shared circuit at its shipped 30-minute ceiling and a synthetic ChatGPT usage-limit error naming a 120-minute reset. No accounts and no model calls; the clock is injected. On main: keeps the fallback while the primary's named usage-limit reset is still ahead fails with AssertionError: expected true to be false (the primary is restored at 31 minutes). The 121-minute restore case passes before and after.
  • GREEN: cd packages/coding-agent && CI=1 bun run test -- test/suite/regressions/3347-restore-waits-for-primary-hold.test.ts test/suite/retry-fallback-dead-target.test.ts test/suite/retry-fallback-circuit-breaker.test.ts test/suite/retry-fallback-circuit-reset-window.test.ts test/suite/retry-fallback-engine.test.ts: 5 files, 52 tests passed.
  • Gates at the pushed head: bunx oxfmt --check (both files), bun check --no-script --all -p tsconfig.json (no type errors), git diff --check origin/main...HEAD.