diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 077a0cf1..c02dc763 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -2191,7 +2191,15 @@ function App() { } void (async () => { - await webOAuthStorage.load(); + try { + await webOAuthStorage.load(); + } catch (err) { + connectStartRef.current = undefined; + queueMicrotask(() => { + showReAuthBanner(server.id, err instanceof Error ? err : String(err)); + }); + return; + } const client = setupClientForServer(server, sessionId); setActiveServerId(server.id); try { diff --git a/clients/web/src/test/core/auth/storage-remote.test.ts b/clients/web/src/test/core/auth/storage-remote.test.ts index 04175845..be8536a5 100644 --- a/clients/web/src/test/core/auth/storage-remote.test.ts +++ b/clients/web/src/test/core/auth/storage-remote.test.ts @@ -1,5 +1,9 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { RemoteOAuthStorage } from "@inspector/core/auth/remote/storage-remote.js"; +import { + type OAuthStore, + waitForOAuthStorePersistLoad, +} from "@inspector/core/auth/store.js"; const NOOP_FETCH = vi.fn( async () => @@ -121,6 +125,12 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => { expect(s).toBeInstanceOf(RemoteOAuthStorage); }); + it("waitForOAuthStorePersistLoad resolves for an unregistered store handle", async () => { + await expect( + waitForOAuthStorePersistLoad({} as OAuthStore), + ).resolves.toBeUndefined(); + }); + it("load() waits for remote GET before sync reads see persisted state", async () => { const persisted = { state: { @@ -155,4 +165,20 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => { "persisted-verifier", ); }); + + it("load() rejects when remote GET fails instead of hanging", async () => { + const failingFetch = vi.fn( + async () => new Response("server error", { status: 500 }), + ) as unknown as typeof fetch; + + const failingStorage = new RemoteOAuthStorage({ + baseUrl: "http://remote.example", + storeId: `fail-${Math.random().toString(36).slice(2)}`, + fetchFn: failingFetch, + }); + + await expect(failingStorage.load()).rejects.toThrow( + "Failed to read store: 500", + ); + }); }); diff --git a/core/auth/oauth-storage.ts b/core/auth/oauth-storage.ts index 0332adf7..64c29534 100644 --- a/core/auth/oauth-storage.ts +++ b/core/auth/oauth-storage.ts @@ -8,7 +8,11 @@ import { OAuthTokensSchema, } from "@modelcontextprotocol/sdk/shared/auth.js"; import type { OAuthStorage } from "./storage.js"; -import { type createOAuthStore, type ServerOAuthState } from "./store.js"; +import { + type OAuthStore, + type ServerOAuthState, + waitForOAuthStorePersistLoad, +} from "./store.js"; import type { IdpSessionState, OAuthClientRegistrationKind, @@ -16,10 +20,9 @@ import type { SaveTokensOptions, } from "./storage.js"; -type OAuthStoreWithPersist = ReturnType & { +type OAuthStoreWithPersist = OAuthStore & { persist: { hasHydrated: () => boolean; - onFinishHydration: (fn: () => void) => void; }; }; @@ -32,19 +35,18 @@ export class OAuthStorageBase implements OAuthStorage { private readonly store: OAuthStoreWithPersist; private loadedPromise: Promise | undefined; - constructor(store: ReturnType) { + constructor(store: OAuthStore) { this.store = store as OAuthStoreWithPersist; } load(): Promise { if (!this.loadedPromise) { - this.loadedPromise = new Promise((resolve) => { + this.loadedPromise = (async () => { if (this.store.persist.hasHydrated()) { - resolve(); return; } - this.store.persist.onFinishHydration(() => resolve()); - }); + await waitForOAuthStorePersistLoad(this.store); + })(); } return this.loadedPromise; } diff --git a/core/auth/store.ts b/core/auth/store.ts index a54bfb30..52d966c3 100644 --- a/core/auth/store.ts +++ b/core/auth/store.ts @@ -43,6 +43,23 @@ export interface OAuthStoreState { clearEnterpriseManagedResourceServers: () => void; } +export type OAuthStore = ReturnType; + +const persistLoadByStore = new WeakMap>(); + +/** + * Wait until the store's initial persist read has finished (success or failure). + * Used by {@link OAuthStorageBase.load}; resolves on success, rejects when the + * storage adapter's getItem fails (Zustand does not fire onFinishHydration on error). + */ +export function waitForOAuthStorePersistLoad(store: OAuthStore): Promise { + const promise = persistLoadByStore.get(store); + if (!promise) { + return Promise.resolve(); + } + return promise; +} + /** * Creates a Zustand store for OAuth state with the given storage adapter. * The storage adapter handles persistence (file, remote HTTP, sessionStorage, etc.). @@ -53,7 +70,29 @@ export interface OAuthStoreState { export function createOAuthStore( storage: ReturnType, ) { - return createStore()( + let resolvePersistLoad!: () => void; + let rejectPersistLoad!: (error: unknown) => void; + let persistLoadSettled = false; + + const persistLoadPromise = new Promise((resolve, reject) => { + resolvePersistLoad = resolve; + rejectPersistLoad = reject; + }); + + const settlePersistLoad = (error?: unknown) => { + if (persistLoadSettled) { + /* v8 ignore next -- idempotent if persist signals completion more than once */ + return; + } + persistLoadSettled = true; + if (error) { + rejectPersistLoad(error); + return; + } + resolvePersistLoad(); + }; + + const store = createStore()( persist( (set, get) => ({ servers: {}, @@ -118,7 +157,13 @@ export function createOAuthStore( { name: "mcp-inspector-oauth", storage, + onRehydrateStorage: () => (_state, error) => { + settlePersistLoad(error); + }, }, ), ); + + persistLoadByStore.set(store, persistLoadPromise); + return store; }