Skip to content

feat: add global auth-failure circuit breaker to coordinate polling on 401 - #720

Merged
birme merged 2 commits into
mainfrom
feature-writer/597-auth-circuit-breaker
Oct 2, 2026
Merged

birme merged 2 commits into
mainfrom
feature-writer/597-auth-circuit-breaker

Conversation

@birme

@birme birme commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Closes #597

Summary

  • Add a new framework-agnostic singleton src/api/auth-circuit-breaker.ts implementing a three-state machine (closed → reauthing → closed on success / open on failure).
  • Route every 401 through handle-fetch-request.ts, a single chokepoint that reports to the breaker: the first 401 trips it to reauthing while concurrent 401s are deduplicated so exactly one coordinated reauth runs.
  • Add use-reauth.tsx, a hook mounted in App.tsx, to configure the breaker with the reauth runner and error handling.
  • Make polling hooks respect the breaker — use-line-polling.ts, use-heartbeat.ts, use-fetch-production-list.ts, and use-websocket-reconnect.ts pause while reauthing, resume with reset failure counters on success, and stop when the breaker trips open.

Test plan

  • Tests pass (npm test in both repos)
  • TypeScript compiles (npm run typecheck)
  • Lint clean (npm run lint)
  • New auth-circuit-breaker.test.ts covers the 10 state-machine cases
  • Updated heartbeat and production-list hook tests pass; full suite reports 304 tests passing

🤖 Generated with Claude Code

…ling on 401

Intercept 401s at the single fetch chokepoint (handleFetchRequest) and route
them through a framework-agnostic circuit breaker that coordinates the whole
app's reaction: pause all polling, run a single reauth, then resume on success
or stop and surface an error on failure.

- Add src/api/auth-circuit-breaker.ts: closed/reauthing/open state machine with
  subscribe/report401/configure/reset. Dedupes concurrent 401s into one reauth.
- handleFetchRequest reports every 401 to the breaker.
- use-reauth.tsx: new useAuthCircuitBreaker hook wires the real reauth call and
  error surfacing (redirect/suppress/dispatch) into the breaker; mounted in App.
- use-line-polling, use-heartbeat: gate each loop on the breaker, waiting for
  resume (counters reset) or stopping when tripped.
- use-fetch-production-list: skip fetches while the breaker is non-active and
  defer 401 handling to the breaker instead of an independent reauth.
- use-websocket-reconnect: hold off reconnecting while the breaker is non-active.
- Tests for the breaker, heartbeat pause/resume/stop, and updated fetch-list 401
  expectations.

Resolves #597

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@birme

birme commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

code-reviewer verdict: NEEDS CHANGES (automated self-review; recorded as a marker because GitHub blocks state-bearing self-review when author and reviewer are the same account).

Blocking

  • The circuit breaker's open state is terminal (only reset() on unmount exits it) and, for suppressed statuses (500/404/405), silent. A transient 500 (expected when the initial OSC token expires) or a stray pre-config 401 now permanently stops all app-wide polling — including the active-call use-heartbeat — with no error surfaced. When heartbeats stop mid-call the manager reclaims the SMB session, silently killing a live call. This is a resilience/UX regression from the previous self-healing per-hook logic. Needs a half-open/backoff retry, or keep polling alive for suppressed/transient cases, or at minimum surface a visible "connection lost" state when the breaker goes terminally open.

Warnings

  • A 401 that resolves before configure() commits trips the breaker open with a swallowed error (no runner/errorReporter yet). Add a defensive guard/queue.
  • No test coverage for the new gate() logic in use-line-polling or the use-websocket-reconnect breaker integration; the use-fetch-production-list tests only assert reauth isn't called, not that fetches pause/resume with breaker state.

Suggestions

  • Guard the failure branch with if (state === "reauthing") like the success branch, so a post-reset() failing reauth can't re-open the breaker.
  • Note in use-heartbeat that the breaker is now the primary 401 coordinator (its own failure401Count path now overlaps).

The core dedup guarantee (exactly one coordinated reauth under concurrent 401s) is correct, there's no import cycle, and subscription cleanup is sound — the issue is specifically the terminal/silent open state.

…ing forever

Address code-review (NEEDS CHANGES) on the global auth-failure circuit breaker.

Blocking: the `open` state was terminal (only reset() on unmount exited it) and
silent for suppressed statuses (500/404/405). A transient 500 (OSC token refresh)
or a stray pre-config 401 permanently froze ALL app-wide polling — including the
active-call heartbeat — which let the manager reclaim the live SMB session and
silently kill the call. The breaker is now non-terminal: on failure it schedules
a half-open retry on an exponential backoff (2s..30s) and keeps retrying until
reauth succeeds, at which point every polling loop resumes. The stall is also made
visible — a non-fatal, auto-dismissing "reconnecting" warning is surfaced for the
previously-silent suppressed cases, and cleared via an onRecover callback when a
retry recovers.

- use-heartbeat / use-line-polling: wait for resume whenever the breaker is not
  active (reauthing OR open) instead of stopping dead on open, so polling resumes
  once the breaker recovers.

Warnings:
- Defensive guard: a 401 that arrives before configure() wires in the runner no
  longer trips the breaker open with a swallowed error — it is remembered and
  replayed once configure() runs, keeping polling alive meanwhile.
- Added test coverage: half-open recovery, gate() pause/resume in use-line-polling,
  use-websocket-reconnect breaker integration, use-fetch-production-list pause/resume,
  and heartbeat resume-after-open.

Suggestions:
- Guard the reauth failure branch with a reauthing-state check (mirrors the success
  branch) so a reauth in flight when reset() ran cannot re-open the breaker.
- Note in use-heartbeat that the breaker is now the primary 401 coordinator.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@birme

birme commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

code-reviewer verdict: LGTM (automated self-review; recorded as a marker because GitHub blocks state-bearing self-review when author and reviewer are the same account).

The prior blocking defect — the circuit breaker's open state being terminal and silent, which could permanently stop all polling (including the active-call heartbeat) on a transient 500 / stray 401 — is genuinely fixed and verified: open is non-terminal (half-open retry on 2s→30s exponential backoff, cycling back to closed on the first successful reauth), the failure is now surfaced via a visible auto-dismissing WARNING that onRecover clears, the heartbeat/line-polling hooks resume on open, a pre-configure() 401 is queued and replayed, and the concurrent-401 dedup guarantee still holds with no import cycle and sound timer/subscription cleanup. typecheck, lint and the full suite (39 files / 319 tests, incl. ~12 new breaker/gate tests) pass.

Non-blocking follow-ups for later (not gating merge): persistent 404/405 reauth-impossible backends keep polling paused (consider treating definitive 404/405 as resume-and-stay-closed); add a DEV guard mirroring useSetupTokenRefresh; dispatch the warning once per closed→open edge rather than every retry; have onRecover also clear a non-suppressed ERROR banner.

@birme
birme merged commit 0843e75 into main Oct 2, 2026
6 checks passed
@birme
birme deleted the feature-writer/597-auth-circuit-breaker branch October 2, 2026 16:59
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.

Add global auth-failure circuit breaker to coordinate all polling on 401

2 participants