refactor: migrate to RemoteOAuthStorage for web OAuth handling

[BobDickinson]

Closes #1548

Summary

Wire the v2 web client through shared file-backed OAuth storage instead of tab-local sessionStorage.

• Web OAuth runtime state now uses RemoteOAuthStorage → GET/POST /api/storage/oauth → ~/.mcp-inspector/storage/oauth.json (same on-disk file as CLI/TUI when using the default local backend).
• New web-local accessor getWebRemoteOAuthStorage() (clients/web/src/lib/remoteOAuthStorage.ts) memoizes one in-memory RemoteOAuthStorage per { origin, authToken } so connect, EMA IdP session, and per-server “clear stored OAuth” all mutate the same Zustand view.
• core/ unchanged — no Zustand removal, no async storage interface churn (Remove Zustand file-store wrappers; auto-convert to plain JSON on read #1549 / Web: migrate auth store to shared /store API (RemoteOAuthStorage parity with TUI/CLI) #1592-style work stays out of scope).

Why

Before	After
BrowserOAuthStorage → sessionStorage (tab-local)	RemoteOAuthStorage → oauth.json (shared with CLI/TUI)
IdP session / tokens lost on refresh or cross-tab	Survive refresh; callback no longer depends on same-tab sessionStorage
Separate singleton (getBrowserOAuthStorage)	Web singleton (getWebRemoteOAuthStorage) over remote adapter

Call chain

App.tsx / environmentFactory.ts
  → getWebRemoteOAuthStorage(authToken)
  → RemoteOAuthStorage (core/auth/remote/storage-remote.ts)
  → /api/storage/oauth (Hono backend)
  → ~/.mcp-inspector/storage/oauth.json

authToken is forwarded on storage HTTP calls as x-mcp-remote-auth when the backend requires it (same as transport/fetch/logger).

---

Changes

New

File	Purpose
clients/web/src/lib/remoteOAuthStorage.ts	Memoized getRemoteOAuthStorage / getWebRemoteOAuthStorage
clients/web/src/lib/remoteOAuthStorage.test.ts	Cache key, origin, window guard, cache reset
clients/web/src/lib/environmentFactory.test.ts	Wiring: shared storage, navigation, redirect provider, fetch wrapper

Modified (web)

File	Change
clients/web/src/lib/environmentFactory.ts	getWebRemoteOAuthStorage(authToken) instead of getBrowserOAuthStorage(); doc comment
clients/web/src/App.tsx	webOAuthStorage useMemo; EMA + clear OAuth use shared store; callback comment
clients/web/src/utils/clearServerOAuthState.ts	Accept oauthStorage: OAuthStorage; active client narrowed to Pick<InspectorClient, "clearOAuthTokens">
clients/web/vite.config.ts	Add src/lib/** to coverage include

Tests / coverage

File	Change
clearServerOAuthState.test.ts	Isolated BrowserOAuthStorage instances; typed client stub (no as never)
storage-browser.test.ts	getBrowserOAuthStorage singleton branch (coverage for core helper still used elsewhere)

Spec

File	Change
specification/v2_auth_ema.md	Web on shared oauth.json; mark #1548 follow-up done
specification/v2_auth_mid_session.md	Client matrix + multi-tab OAuth storage note

---

Out of scope (follow-ups)

• Remove Zustand file-store wrappers; auto-convert to plain JSON on read #1549 — Remove Zustand from OAuth persistence; plain JSON blob read/write.
• navigator.locks — Cross-tab single-flight on silent refresh (noted in spec).

---

Review notes

• Singleton cache is intentional: each RemoteOAuthStorage instance owns a separate in-memory Zustand store; memoization matches the old getBrowserOAuthStorage() pattern.
• clearServerOAuthState plumbs oauthStorage from App so unit tests stay isolated without stubbing window/remote fetch; parameter types narrowed to what the helper actually calls.
• useMemo([], getAuthToken()) matches existing sessionStorageAdapter assumption: API token is stable for the page lifetime (injected __INSPECTOR_API_TOKEN__ on load).

[BobDickinson]

@claude review

[claude]

Claude finished @BobDickinson's task in 3m 21s —— View job

---

Code Review — PR #1609 (migrate web OAuth to RemoteOAuthStorage)

• Read the PR diff against origin/v2/main
• Review new remoteOAuthStorage.ts + tests
• Review environmentFactory.ts, App.tsx, clearServerOAuthState.ts changes
• Check RemoteOAuthStorage core adapter semantics
• Post review feedback

Nice, well-scoped refactor — the plumbing of oauthStorage into clearServerOAuthState and the typed test stubs (dropping as never) are clean improvements, and keeping core/ untouched is the right call. My main feedback is one architectural risk worth verifying before merge.

---

🔴 Main concern: sync reads/writes now race async hydration

The old BrowserOAuthStorage was backed by sessionStorage, whose getItem is synchronous — so Zustand's persist middleware hydrated the store synchronously during createStore. Every subsequent read (getCodeVerifier, getClientInformation, …) was guaranteed to see the persisted state.

RemoteOAuthStorage is backed by an async adapter (core/storage/adapters/remote-storage.ts — getItem is async + HTTP GET). Zustand hydrates such stores asynchronously in the background; createOAuthStore (core/auth/store.ts) has no hydration barrier. But the reads in OAuthStorageBase are still synchronous against in-memory state:

// core/auth/providers.ts:242
codeVerifier(): string {
  const verifier = this.storage.getCodeVerifier(this.serverUrl); // reads get().servers[url] in memory
  if (!verifier) throw new Error("No code verifier saved for session");
  return verifier;
}

Two failure modes fall out of this:

1. Callback code-exchange can throw. On the /oauth/callback fresh page load, webOAuthStorage is created at App mount and the effect proceeds as soon as servers.length > 0 (App.tsx:2118). That gate is a separate backend fetch — nothing awaits the /api/storage/oauth hydration GET. If servers hydrate first, resumeAfterOAuth → codeVerifier() reads an empty store and throws "No code verifier saved for session", dropping the user into the re-auth banner. This is precisely the scenario the PR aims to make more robust, so it'd be good to prove it can't regress.

2. Writes before hydration can clobber oauth.json. setServerState/clearServerState call Zustand set(), which persists the full in-memory servers map. If a write (e.g. saveCodeVerifier on connect, or clear() from "clear stored OAuth") lands before hydration merges the on-disk blob, the persisted state is { servers: { [thisUrl]: … } } — silently wiping every other server's tokens that existed on disk. With tab-local sessionStorage this couldn't happen (sync hydration + per-tab scope); with a shared file it can.

Both are timing-dependent, so they may pass in a quick manual test and bite intermittently. Worth confirming there's a hydration await on these paths, e.g. gate the callback effect (and ideally the first connect/clear) on persist's onFinishHydration / hasHydrated, or expose a whenHydrated() on the storage that resumeAfterOAuth awaits. Investigate this →

---

🟡 Minor

• getRemoteOAuthStorage is a single-slot cache, not a per-key map (remoteOAuthStorage.ts:7). The doc comment and PR body say "memoizes one instance per { origin, authToken }", but cached holds only the most recent key — alternating two keys would thrash and hand out fresh Zustand stores (divergent in-memory state). In practice only one {origin, token} is live per page, so this is harmless today; consider a Map if you want the comment to hold literally, or tighten the wording to "memoizes the last requested key."
• resetWebRemoteOAuthStorageCacheForTests ships in the production bundle. It's @internal-tagged so this is acceptable, just flagging it's reachable at runtime.
• Cross-tab sharing is load-time only. Because persist hydrates once at store creation, a tab reads oauth.json only on load — live cross-tab propagation isn't there (a second tab won't see a first tab's new token until it reloads). That matches the "survive refresh" framing and the navigator.locks follow-up note, so no change needed; just making sure the "shared with CLI/TUI" claim is understood as load-time consistency.

✅ Looks good

• clearServerOAuthState now takes oauthStorage + narrows inspectorClient to Pick<…, "clearOAuthTokens"> — good testability and least-privilege typing.
• useMemo([], getAuthToken()) is consistent with the existing token-is-stable-for-page-lifetime assumption (getAuthToken is module-level at App.tsx:214).
• Test coverage additions are solid and the vite.config.ts src/lib/** include keeps the gate honest.
  • v2/1548-web-remote-oauth-storage

[BobDickinson]

PR review response — async OAuth storage / callback race

Re: review feedback on RemoteOAuthStorage + async persistence

The latest commit (a2409148 — fix(auth): await OAuth storage load() to fix remote OAuth races) addresses the main concern about RemoteOAuthStorage + Zustand persist loading over HTTP while callers still treated storage as immediately ready.

1. Callback reads PKCE before persisted state is loaded

/oauth/callback now calls await webOAuthStorage.load() before resumeAfterOAuth, so the code exchange does not read getCodeVerifier() against an empty in-memory store while the GET to /api/storage/oauth is still in flight.

2. Writes can clobber oauth.json with an empty snapshot

OAuthStorage now has an explicit load(): Promise<void>. OAuthStorageBase implements it (waits until the backing adapter has read persisted state). All mutating methods — save*, clear*, etc. — call ensureLoaded() first, so we do not persist before the initial load completes.

3. Code exchange path

completeOAuthFlow in oauthManager also awaits storage.load() before reading the verifier, so the same race is covered outside the web callback path (CLI/TUI/EMA).

4. API shape

Public name is load(), not Zustand “hydration” terminology. Sync getters (getCodeVerifier, getScope, …) are unchanged; callers that depend on persisted values must await load() first (or use the async mutators, which load internally). clearOAuthTokens / related clears are async end-to-end where needed (e.g. TUI await clearOAuthTokens()).

5. Tests

• storage-remote.test.ts: regression test that load() waits on a delayed GET before sync reads see persisted state.
• Storage/integration tests updated to await async saves/clears and to mock load on stubs.

Out of scope for this commit (as discussed)

• Removing Zustand (Remove Zustand file-store wrappers; auto-convert to plain JSON on read #1549) — still deferred.
• Full root npm run coverage for mid-session CLI/TUI gaps — follow-up PR.

[BobDickinson]

@claude review

[claude]

Claude finished @BobDickinson's task in 2m 57s —— View job

---

Re-review after the load() fix (a2409148)

• Read the full PR diff against origin/v2/main
• Verify the load() hydration barrier addresses the earlier race concerns
• Review new/changed core auth storage code
• Post review

The load() barrier is the right shape and resolves both earlier race concerns:

• Callback code-exchange — App.tsx:2194 now await webOAuthStorage.load() before resumeAfterOAuth, and oauthManager.completeOAuthFlow (oauthManager.ts:285) awaits requireStorage().load() before reading the verifier. So the sync getCodeVerifier() in providers.ts:243 can no longer read an empty store. ✅
• Write-before-hydration clobber — every mutator (save*/clear*) now await ensureLoaded() before set(), so the on-disk servers map is merged in before any partial write persists. ✅
• The sync getters that stay unguarded (getScope/getServerMetadata/getClientRegistrationKind in connection-state.ts:90-97) are fine because computeConnectionState awaits getTokens() first (which calls ensureLoaded), so hydration is done before the sync reads. Good.

Nicely done — the async plumbing through clearOAuthTokens/clearEmaIdpSession/clearServerOAuthState and the TUI await is consistent.

---

🔴 One new failure mode introduced by load(): a failed hydration hangs forever

OAuthStorageBase.load() (core/auth/oauth-storage.ts:39-50) resolves only via persist.hasHydrated() or persist.onFinishHydration():

load(): Promise<void> {
  if (!this.loadedPromise) {
    this.loadedPromise = new Promise((resolve) => {
      if (this.store.persist.hasHydrated()) { resolve(); return; }
      this.store.persist.onFinishHydration(() => resolve());
    });
  }
  return this.loadedPromise;
}

In Zustand v5's persist middleware, if the storage adapter's getItem rejects, the hydrate chain lands in .catch, which calls onRehydrateStorage(undefined, e) but does not set hasHydrated = true and does not fire the finishHydration listeners. So on a rejected hydration, hasHydrated() stays false forever and onFinishHydration never fires → load() never settles.

The remote adapter rejects on any non-OK, non-404 response (remote-storage.ts:41-45: throw new Error("Failed to read store: " + status)) — e.g. a 500 from the backend, a 401 if the token is rejected, or a network-level fetch rejection. When that happens:

• await webOAuthStorage.load() in the callback effect (App.tsx:2194) hangs — the try/catch around resumeAfterOAuth never runs, so no re-auth banner, no error, the flow just stalls silently.
• completeOAuthFlow (oauthManager.ts:285) hangs on the same await.
• Every mutator (ensureLoaded) hangs too, so connect / clear-OAuth stall with no feedback.

With the old sessionStorage backing this was impossible (sync getItem, never rejects). Moving to an HTTP-backed adapter makes it reachable, and the symptom (a permanent silent hang) is arguably worse than the pre-fix throw, which at least surfaced the banner.

Suggested fix: settle load() on hydration error as well as success. The clean route is to capture completion in createOAuthStore via the onRehydrateStorage option (it receives (state, error) on both paths) and expose that to OAuthStorageBase, so load() resolves (or rejects, if you'd rather surface the banner) when hydration fails. A minimal alternative is to add onRehydrateStorage: () => (_state, _error) => {/* mark loaded */} and have load() await that signal instead of relying solely on onFinishHydration.

Fix this →

---

🟡 Minor (carried over / still applicable)

• Add a test for the failed-hydration path. storage-remote.test.ts now covers the delayed GET (load waits, then sync reads see state), which is great — but not the rejected GET. A test asserting load() settles (however you decide it should behave) when the GET returns 500 would lock in the fix above.
• getRemoteOAuthStorage is still a single-slot cache (remoteOAuthStorage.ts:8), not a per-key Map as the doc comment implies. Harmless today (one {origin, token} per page); either switch to a Map or soften the comment to "last requested key."

✅ Looks good

• load() memoization via loadedPromise (single hydration wait, idempotent) is clean.
• The interface change to OAuthStorage (adding load(), making clears Promise) is coherent and the call sites were updated consistently across web/TUI/core/EMA.
• The computeConnectionState sync-getter ordering (async getTokens first) is correct — good that hydration is forced before the sync reads.
  • branch v2/1548-web-remote-oauth-storage

[BobDickinson]

PR comment — settle load() on persist read failure

Addresses Claude re-review (post-a2409148): load() hung forever when remote GET rejected, because Zustand persist only fires onFinishHydration on success, not when the storage adapter throws.

Fix

• createOAuthStore: onRehydrateStorage settles a per-store promise on both success and error; load() rejects when hydration fails (no silent hang, no mutating against an unresolved read).
• OAuthStorageBase.load(): awaits that signal instead of relying solely on onFinishHydration.
• App.tsx OAuth callback: try/catch around await webOAuthStorage.load() so a failed GET surfaces the re-auth banner instead of stalling the effect.
• Test: load() rejects when remote GET fails instead of hanging in storage-remote.test.ts.

Note on store.ts shape

The WeakMap / settle promise wiring is Zustand persist glue — necessary until #1549 replaces persist with explicit blob load/save. Not the long-term design.

Validation

• npm run ci — green
• clients/web npm run test:coverage — green

[BobDickinson]

@claude review

[claude]

Claude finished @BobDickinson's task in 2m 6s —— View job

---

Re-review after 512f45d3 (reject load() on persist read failure)

• Read the latest diff vs origin/v2/main
• Verify the failed-hydration hang fix (load() now rejects)
• Check the callback / mutator error paths surface correctly
• Re-check carried-over minor items

The failed-hydration concern from the last round is resolved correctly. createOAuthStore now wires onRehydrateStorage: () => (_state, error) => settlePersistLoad(error), settlePersistLoad rejects a per-store promise on error, and load() awaits it via waitForOAuthStorePersistLoad. So a 500/401/network failure on the /api/storage/oauth GET now makes load() reject instead of hanging:

• Callback (App.tsx:2194) — try { await webOAuthStorage.load() } catch clears connectStartRef and shows the re-auth banner. ✅ (Good catch on resetting the ref guard so a retry isn't blocked.)
• Code exchange (oauthManager.ts:285) — await this.requireStorage().load() is inside the existing try in completeOAuthFlow, so a rejection routes to the normal error handling. ✅
• Mutators — ensureLoaded() now rejects rather than hanging; connect/clear surface an error instead of stalling silently. ✅
• Regression test load() rejects when remote GET fails instead of hanging (storage-remote.test.ts) locks in the 500 path. ✅

Nice work — the settlePersistLoad idempotency guard and the hasHydrated() short-circuit in load() (so the success path never awaits a second time) are both correct.

---

🟡 One thing worth considering: eager reject → possible unhandled rejection

persistLoadPromise is created eagerly in createOAuthStore and stored in the WeakMap, but the only thing that ever attaches a handler to it is a call to load()/waitForOAuthStorePersistLoad. If the hydration GET rejects before any consumer calls load(), that promise rejects with no .catch attached → an unhandled promise rejection (console warning in the browser; in Node/tests it can trip unhandledRejection).

In the main web flow this is usually masked: computeConnectionState → getTokens() → ensureLoaded() attaches a handler in a microtask/effect, which runs well before an HTTP fetch can settle. But it's timing-dependent, and any store that's created and then never has load() called on it (e.g. an idle/short-lived store, or certain EMA-disabled paths) would emit the warning on a backend error.

If you want to be defensive, attach a no-op catch at creation so the rejection is always considered handled, while real awaiters still see it:

persistLoadPromise.catch(() => {}); // keep the eager promise from going "unhandled"
persistLoadByStore.set(store, persistLoadPromise);

Not blocking — just cheap insurance now that this promise can reject. Consider this →

🟡 Minor (still open from earlier)

• getRemoteOAuthStorage is still a single-slot cache (remoteOAuthStorage.ts:8), while the doc comment says "Memoized by { baseUrl, authToken }". Harmless today (one {origin, token} per page), but the comment over-promises — either use a Map or soften the wording to "the last requested key."

✅ Looks good

• The load() barrier + reject-on-error is a coherent, minimal shape for the async adapter, and the Promise-returning clear*/clearOAuthTokens/clearIdpSession conversions were plumbed through web/TUI/core/EMA consistently (spot-checked providers.clear, oauthManager.clearOAuthTokens, clearServerOAuthState).
• The computeConnectionState sync-getter ordering (async getTokens first, which forces ensureLoaded) remains correct.
• Test updates to await the now-async mutators are thorough, and the delayed-GET + failed-GET tests together cover the load-barrier behavior well.

Overall: the correctness concern is closed; the remaining note is a defensive polish, not a blocker.
• branch v2/1548-web-remote-oauth-storage

[BobDickinson]

Regarding the "eager rejection" recommendation - that entire class of problem goes away with the removal of Zustand storage, which will be a fast follow. I'm happy to go with what we have now so we can get to that work.

[cliffhall]

🤖 Automated review — Claude review loop

Ran a multi-angle review of this diff (line-by-line, removed-behavior, cross-file tracing, plus reuse/simplification/altitude/conventions) with an adversarial verify pass, then a second confirmation pass. The refactor is solid — OAuthStorage going async with a lazy load() barrier is wired through carefully and every now-async clear* call site is correctly awaited. Findings below.

Confirmed issues

1. logout() fires an async call as a floating promise with no .catch — core/react/useEmaIdpLoginState.ts:43 (severity: medium)
This PR rewrote logout to void clearEmaIdpSession(storage, normalizedIssuer).then(() => setLoginState("none")). With web now on RemoteOAuthStorage, clearEmaIdpSession does real network I/O (load() GET + three POSTs to /api/storage/oauth) and can reject. On failure: the rejection is unhandled (surfaces as unhandledrejection), and because the .then never runs, setLoginState("none") is skipped — the UI stays stuck showing the IdP as signed-in with no error surfaced. Flagged independently by two finder angles.

2. Eager persistLoadPromise can reject unhandled — core/auth/store.ts (severity: low–medium)
createOAuthStore builds persistLoadPromise eagerly and captures its reject. Zustand persist kicks off async hydration at store construction — before any consumer calls OAuthStorageBase.load(). On the web path the store is created in a useMemo, so an initial GET /api/storage/oauth failure with no auto-connect pending rejects a promise nobody is awaiting yet → unhandledrejection in the browser (and process-level unhandledRejection for Node's NodeOAuthStorage on a file-read failure before first use).

3. Inconsistent fetch-mock typing — clients/web/src/test/core/auth/storage-remote.test.ts (severity: nit)
The two new mocks use vi.fn(…) as unknown as typeof fetch (a double-cast through unknown), while the sibling test added in this same PR (environmentFactory.test.ts) correctly uses the typed vi.fn<typeof fetch>() — which needs no cast and keeps the mock type-checked. Two ways to do the same thing in one changeset.

4. Doc comment overstates the cache — clients/web/src/lib/remoteOAuthStorage.ts (severity: nit)
The comment says "Memoized by { baseUrl, authToken }" (implying a keyed map), but cached is a single slot — a call with a different key evicts the previous instance. Harmless in practice (stable origin + page-lifetime token), but the documented contract diverges from the implementation.

Reviewed and accepted (no change recommended)

• load() caches a rejection → storage instance is dead until page reload. Fail-fast is intentional (there's an explicit "rejects instead of hanging" test, and App.tsx catches the rejection to show a re-auth banner). A true retry needs store-level re-hydrate support — worth a follow-up, but out of scope here.
• startIdpOidcAuthorization swapped discoverIdpMetadata → resolveIdpMetadata (cache-first). Deliberate and consistent: start persists the metadata and complete reads it back; the token_endpoint presence check gates cache validity. Only stale cached metadata (IdP endpoint rotation) would bite, and that's a pre-existing property of persisting metadata.
• ensureLoaded/loadedPromise + persistLoadByStore overlap. Three representations of "has hydrated" is a little redundant, but restructuring the hydration plumbing is higher-risk than the value; leaving as-is.

Not a problem (checked)

• Every other void → Promise<void> call site (clearOAuthTokens, storage.clear, clearIdpSession, clearServerOAuthState, BaseOAuthClientProvider.clear, tui App.tsx) is correctly awaited.
• BaseOAuthClientProvider.clear() returning a Promise does not violate the SDK OAuthClientProvider interface (which declares no clear; the credential-invalidation hook is the optional invalidateCredentials?(): void | Promise<void>, which BaseOAuthClientProvider doesn't implement).
• The sync getters that did not gain ensureLoaded (getCodeVerifier, getScope, getServerMetadata, getClientRegistrationKind) are, on every traced flow, preceded by an awaited async read / explicit load() barrier — so PKCE-verifier-loss didn't reproduce (though nothing structurally enforces the ordering).

Fixes for issues 1–4 to follow in a second comment.

🤖 Generated with Claude Code

[cliffhall]

🤖 Fixes for the review findings

Prepared fixes for all four findings from the review above (commit ba48e06f, ready to push to this branch). Each is small and scoped; the two accepted-with-rationale items are left as-is.

1. logout() unhandled rejection — core/react/useEmaIdpLoginState.ts

-    void clearEmaIdpSession(storage, normalizedIssuer).then(() => {
-      setLoginState("none");
-    });
+    void clearEmaIdpSession(storage, normalizedIssuer)
+      .then(() => {
+        setLoginState("none");
+      })
+      .catch(() => {
+        // Clearing the persisted IdP session failed (e.g. the storage backend
+        // is unreachable). Swallow the rejection so it does not surface as an
+        // unhandled promise, and leave loginState unchanged so the UI keeps
+        // reflecting the still-present session rather than falsely showing
+        // signed-out.
+      });

Added a test (logout swallows a clear failure without an unhandled rejection and keeps state) that registers an unhandledrejection listener, drives the reject path, and asserts state stays logged_in with nothing unhandled.

2. Eager persistLoadPromise — core/auth/store.ts

   const persistLoadPromise = new Promise<void>((resolve, reject) => {
     resolvePersistLoad = resolve;
     rejectPersistLoad = reject;
   });
+  // Zustand kicks off async hydration as soon as the store is created, before
+  // any consumer calls `OAuthStorageBase.load()`. If that initial read fails
+  // and no one is awaiting yet, `persistLoadPromise` would reject with no
+  // handler and surface as an unhandled rejection. Attach a no-op handler so
+  // the promise is always "handled"; real awaiters still observe the rejection
+  // through the reference returned by `waitForOAuthStorePersistLoad`.
+  void persistLoadPromise.catch(() => {});

Attaching a .catch marks the original promise as handled without consuming the rejection — waitForOAuthStorePersistLoad hands out the same reference, so OAuthStorageBase.load() still rejects (verified by the existing "load() rejects when remote GET fails" test).

3. Fetch-mock typing — clients/web/src/test/core/auth/storage-remote.test.ts
Replaced the two vi.fn(…) as unknown as typeof fetch double-casts with vi.fn<typeof fetch>(…), matching the pattern this PR already uses in environmentFactory.test.ts.

4. Doc comment — clients/web/src/lib/remoteOAuthStorage.ts
Reworded the "Memoized by { baseUrl, authToken }" comment to describe the single-slot cache-of-one accurately (repeat calls with the same key share the instance; a different key replaces it; sufficient because the web app's origin + page-lifetime token are stable).

Verification

• Web: typecheck ✓ · 2721 unit tests ✓ · lint ✓ · format ✓ · coverage gate exit 0 (every touched file 100% except store.ts at 91.66% branch, above the ≥90 gate; the gate also runs the web integration project).
• TUI 247 ✓ · CLI 121 ✓ (both consume the now-async core).

🤖 Generated with Claude Code

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 2m 0s —— View job

---

Code Review — PR #1609 (re-review after ba48e06f)

• Read the latest diff vs origin/v2/main
• Verify fixes for findings 1–4 (ba48e06f)
• Check for any new/remaining issues
• Post review

All four findings from the last automated pass are addressed correctly. Verified each against the current tree:

1. logout() unhandled rejection — core/react/useEmaIdpLoginState.ts:43 ✅
The void clearEmaIdpSession(...) chain now has a .catch() that swallows the rejection and intentionally leaves loginState unchanged (so the UI keeps reflecting the still-present session rather than falsely showing signed-out). This is the correct call — a failed clear should not claim success. Grepping for other floating void clear* calls turned up only this one site, and it's now guarded.

2. Eager persistLoadPromise unhandled rejection — core/auth/store.ts:87 ✅
void persistLoadPromise.catch(() => {}) at creation marks the eager promise as handled without consuming the rejection. Real awaiters still observe it, because waitForOAuthStorePersistLoad hands out the same reference (persistLoadByStore.set(store, persistLoadPromise) at :174), and OAuthStorageBase.load() awaits that reference — so the failed-GET reject test still holds. Correct fix for the reported failure mode.

3. Fetch-mock typing — storage-remote.test.ts:145,170 ✅
Both new mocks (delayedFetch, failingFetch) now use vi.fn<typeof fetch>(), matching the pattern in environmentFactory.test.ts. (The pre-existing module-level NOOP_FETCH at :8 still uses as unknown as typeof fetch — that one predates this PR and wasn't in scope of the finding, so no concern.)

4. Doc comment — remoteOAuthStorage.ts:16 ✅
Reworded to describe the cache-of-one accurately ("A call with a different key replaces the cached instance … sufficient because the web app uses a stable origin + page-lifetime API token").

Spot-checks on the surrounding wiring (still good)

• Callback barrier (App.tsx:2195) — await webOAuthStorage.load() is inside a try/catch that resets connectStartRef and shows the re-auth banner on failure, so a rejected hydration surfaces instead of stalling. ✅
• Mutators — every save*/clear* in OAuthStorageBase awaits ensureLoaded() before set(), so no partial write can clobber the on-disk servers map before the initial read merges. ✅
• clearServerOAuthAndDisconnect (App.tsx:3285) awaits clearServerOAuthState, which awaits either clearOAuthTokens() or oauthStorage.clear() — no floating promise. ✅
• Regression tests for both the delayed GET (load() waits) and failed GET (load() rejects) are present in storage-remote.test.ts.

Accepted-with-rationale (unchanged, agreed)

• load() caches a rejection → instance dead until reload. Fail-fast is intentional and covered by the banner path + test; a true re-hydrate is a follow-up.
• Eager-promise / Zustand persist glue is transitional until Remove Zustand file-store wrappers; auto-convert to plain JSON on read #1549 removes Zustand from OAuth persistence.

Verdict

The correctness concerns raised across all prior rounds are closed, and the four polish fixes in ba48e06f are implemented as described. Nothing new or blocking on my end — this looks ready to merge, with #1549 (drop Zustand) as the natural fast-follow that dissolves the remaining transitional glue.
• branch v2/1548-web-remote-oauth-storage

[cliffhall]

Thanks for the re-review — agreed on all points. Confirming alignment:

• All four fixes in ba48e06f verified correct, no new or remaining issues raised.
• The two accepted-with-rationale items (load() fail-fast + the transitional Zustand persist glue) are acknowledged as follow-ups, with Remove Zustand file-store wrappers; auto-convert to plain JSON on read #1549 (drop Zustand from OAuth persistence) dissolving the remaining glue.

No actionable comments this round, so no further changes. 🟢

🤖 Generated with Claude Code

[cliffhall]

✅ Merged v2/main + smoke-verified

Merged the latest v2/main (bdd2edc1) into this branch and ran the full smoke suite plus a targeted end-to-end check of the OAuth-storage wiring this PR introduces.

Merge

• v2/main merged cleanly — no conflicts. The only incoming changes were CLI/TUI mid-session-auth test files (Mid-session authorization (web, TUI, CLI) #1608/test: cover v2/main mid-session-auth code to the ≥90% per-file gate #1611 coverage), which don't overlap this PR's core/auth + web changes.

Smoke suite — all green

npm run build succeeded for all clients, then npm run smoke:

smoke:launcher OK — --help shows launcher usage; --cli dispatches to the CLI; --tui dispatches to the TUI
smoke:cli      OK — tools/list over stdio via --catalog; default-catalog seed; read-only --config error;
                    --catalog/--config conflict; unknown --server error; multi-server selection; --header merge
smoke:tui      OK — rendered "MCP Servers" panel from --catalog within timeout
smoke:web      OK — GET / => 200 with injected __INSPECTOR_API_TOKEN__ at http://127.0.0.1:6299

End-to-end proof of the OAuth-storage change

Booted the prod web server and exercised the exact path this PR rewires web onto (RemoteOAuthStorage → GET/POST /api/storage/oauth → ~/.mcp-inspector/storage/oauth.json), replacing tab-local sessionStorage:

Check	Result
GET / (SPA)	HTTP 200
GET /api/storage/oauth without token	HTTP 401 (auth guard enforced)
GET /api/storage/oauth with x-mcp-remote-auth: Bearer …	HTTP 200 — returns the { state: { servers, idpSessions }, version } envelope read from disk
POST /api/storage/oauth (token blob)	{"ok":true} HTTP 200
GET back	persisted value returned ✔
On-disk oauth.json	blob written to ~/.mcp-inspector/storage/oauth.json ✔

This confirms the core behavioral change end-to-end: web OAuth state is now file-backed and shared with CLI/TUI, and survives a page refresh instead of living in the tab's sessionStorage.

---

🤖 Generated with Claude Code