Skip to content

fix(claude-usage): validate session key on save + honest, category-specific errors - #218

Merged
aterrylu merged 2 commits into
mainfrom
terry/claude-usage-error-ux
Jun 19, 2026
Merged

fix(claude-usage): validate session key on save + honest, category-specific errors#218
aterrylu merged 2 commits into
mainfrom
terry/claude-usage-error-ux

Conversation

@aterrylu

Copy link
Copy Markdown
Owner

Problem

The Claude Usage plugin trapped users in a reconfigure loop. Terry's report: "it says error; when I configure the sessionKey it errors again and tells me to reconfigure, but it is the correct session ID."

Root-causing the code surfaced two distinct defects that combine into the loop:

  1. Save reported false success. SetupPanel.handleSave only checked the PUT /api/settings response — but that endpoint just writes the key to disk and returns 200 for any well-formed string. It showed "Saved!" and auto-closed without ever validating the key against claude.ai.
  2. The error panel hardcoded the cause. It always rendered "Session cookie may have expired or is invalid" + a "Reconfigure" button for every error — even when the backend had produced a specific message for a rate-limit or outage. So when the failure wasn't the credential, re-entering a correct key changed nothing → loop.
flowchart TD
    A["Status bar: 'err'"] --> B[ErrorPanel]
    B --> C["Hardcoded:<br/>'cookie expired or invalid'"]
    C --> D["Reconfigure → SetupPanel"]
    D --> E["Paste correct key → Save"]
    E --> F["PUT settings = disk write → 200"]
    F --> G["'Saved!' + auto-close"]
    G --> H["refetch → real validation"]
    H -->|"non-credential failure<br/>(rate-limit / outage / no_org)"| A
    style C fill:#ea6c7322
    style G fill:#ea6c7322
Loading

Root cause (confirmed against real claude.ai)

A bad/expired key does not get a 401. claude.ai's bootstrap API returns 200 with no account (logged-out treatment) → resolves to no_org. Live QA logged exactly account=false, memberships=undefined. So Terry's key was being treated as logged-out (stale), while the UI both lied ("Saved!") and misdiagnosed ("cookie expired → reconfigure").

Solution

Layer Change
scanner.ts Typed errorKind (unauthorized / no_org / rate_limited / unavailable) on the response; specific, reassuring messages (transient → "your key is fine"). Cache fingerprinted by session key so a key change is never served the old key's data. Bootstrap logs its response shape (never the cookie) on no_org.
saveAndValidate.ts (new) Persists the key then validates it against claude.ai before reporting success. Returns ok / invalid / unreachable; a 5xx surfaces the server's detail as transient, not "unreachable".
SetupPanel 4-phase machine: "Save & verify" → "Checking…" → "Connected ✓" or a specific inline failure that keeps the panel open. Input disabled mid-flight; success timer cleaned up on unmount; stale error cleared on re-edit.
ErrorPanel Branches the remedy on errorKind: credential → Reconfigure; transient → "Retry now" + "not a key problem."
Status bar Transient shows amber "delayed", not red "err".

Testing

  • 30 new unit tests (16 scanner incl. cache-fingerprint race + errorKind; 7 saveAndValidate; +7 prior).
  • Real-spawn browser QA — booted server + dashboard (isolated config), drove all four states live:
    • Credential error → "Session key problem" + Reconfigure
    • Validate-on-save → bad key shows the specific failure, no false "Saved!"/"Connected"
    • Success → status bar renders 5h/7d %
    • Transient → "delayed" + "Usage temporarily unavailable" + "Retry now" + "not a key problem"
    • 0 console errors through the full flow.
  • Full suites green: 461 server + 216 dashboard, tsc --build clean, biome 0 errors.

Risks

  • Credential handling — session key stays redacted / type=password, never logged; the new bootstrap diagnostic logs only response shape, never the cookie.
  • Two save surfaces (SetupPanel + in-panel CredentialsSection) share the one saveAndValidate helper, so behavior can't drift.
  • no_org is classified as a credential error; live verification confirmed this is the real bad-key path (account absent). The new shape-log makes a future contract drift debuggable.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BNCGtrKPo66XyqvNFBinzX

…ecific errors

The Claude Usage plugin collapsed every failure into "error → reconfigure",
trapping users in a loop: pasting a correct key showed "Saved!" and then
errored again telling them to reconfigure, even when the real problem wasn't
the credential.

Root cause (confirmed against real claude.ai): two distinct defects.
1. The setup panel reported success on the settings PUT alone — a disk write
   that returns 200 for ANY string. It never validated the key, so a key that
   didn't work still showed "Saved!".
2. The error panel hardcoded "Session cookie may have expired or is invalid"
   and only ever offered "Reconfigure", discarding the backend's specific,
   already-distinguished failure messages. A rate-limit or outage thus told
   the user to re-enter a key that was never the problem.

Fix:
- scanner.ts now tags each failure with a typed `errorKind`
  (unauthorized | no_org | rate_limited | unavailable) and returns specific,
  reassuring messages (transient failures say "your key is fine").
- New saveAndValidate.ts client helper persists the key AND verifies it
  against claude.ai before reporting success — no more false "Saved!".
- SetupPanel is a 4-phase machine (input → working → success → failed):
  "Save & verify" → "Checking…" → "Connected ✓" or a specific inline failure
  that keeps the panel open instead of blind-closing.
- ErrorPanel branches the remedy on the failure category: credential errors
  offer "Reconfigure"; transient errors offer "Retry now" and state it's not
  a key problem. Status bar shows amber "delayed" (not red "err") for transient.
- Cache is fingerprinted by session key so a key change can never be served
  the previous key's data, even if a concurrent poll repopulates the cache
  between the settings write and the validation read.
- Bootstrap logs its response shape (never the cookie) when no org resolves,
  so an expired cookie vs. a contract change is tellable apart from logs.

Verified: 30 new unit tests, real-spawn browser QA across all four states
(credential error, validate-on-save, success, transient), full suites green
(461 server, 216 dashboard), tsc + biome clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BNCGtrKPo66XyqvNFBinzX
@aterrylu
aterrylu enabled auto-merge (squash) June 19, 2026 23:45
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BNCGtrKPo66XyqvNFBinzX
return () => {
if (closeTimer.current) clearTimeout(closeTimer.current);
};
}, []);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Suggestion: the useEffect cleanup only clears closeTimer.current if it is already populated at unmount. But the timer is scheduled AFTER await saveAndValidate(...) resolves — so a click-outside during the validation flight unmounts the panel with closeTimer.current === null, the cleanup runs and does nothing, then the awaited promise resolves and setTimeout(() => onClose(), 1600) is scheduled on the now-unmounted component. The setState calls before it are silent no-ops in React 18, but the timer survives. If the user re-opens the panel within ~1.6s (by clicking the status-bar icon again), that orphan timer will call onClose() and dismiss it from under them. Narrow race — happy path is unaffected; the only consequence is a surprise close. Easiest fix is a mounted ref (or AbortController) checked between the await and the setTimeout. Not blocking.

@nox-0x nox-0x left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving — the loop fix is well-targeted: typed errorKind in the scanner, save-then-validate on the client, and a UI that branches the remedy on the failure category instead of always pushing reconfigure. Cache fingerprinting by session-key hash genuinely closes the post-save race for the data and last-good buffers. Tests cover all four errorKinds plus the cache-fingerprint races.

Two non-blocking observations:

  • (Inline) closeTimer cleanup races with the validation await — if the user clicks outside during the in-flight saveAndValidate, the cleanup runs while the ref is still null and the orphan setTimeout scheduled afterwards survives. Narrow window, surprise-close consequence only.

  • cachedOrgId in the scanner is NOT fingerprinted alongside cached/lastGood. In normal flow this is fine because settings.ts:109 calls invalidateCache() on PUT, but the comment claim about "correctness local to the scanner rather than dependent on callers remembering to call invalidateCache()" only applies to the data caches — a concurrent poll racing the settings PUT can still repopulate cachedOrgId with the old key’s org. Self-healing (the resulting 401 clears cachedOrgId) so worth at most a follow-up.

@aterrylu
aterrylu merged commit e711630 into main Jun 19, 2026
10 checks passed
@aterrylu
aterrylu deleted the terry/claude-usage-error-ux branch June 19, 2026 23:51
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.

2 participants