fix(auth): persist OAuth pre-redirect Network log across the redirect (#1384)

clients/web/src/App.tsx

@@ -34,8 +34,10 @@ import { StderrLogState } from "@inspector/core/mcp/state/stderrLogState.js";
 import type { RedirectUrlProvider } from "@inspector/core/auth/index.js";
 import {
   parseOAuthCallbackParams,
+  parseOAuthState,
   generateOAuthErrorDescription,
 } from "@inspector/core/auth/index.js";
+import { RemoteInspectorClientStorage } from "@inspector/core/mcp/remote/index.js";
 import { useInspectorClient } from "@inspector/core/react/useInspectorClient.js";
 import { useServers } from "@inspector/core/react/useServers.js";
 import { useSettingsDraft } from "@inspector/core/react/useSettingsDraft.js";
@@ -450,11 +452,68 @@ function App() {
     [messages],
   );
 
+  // Backend-backed session storage used to carry the fetch (Network) log
+  // across the OAuth full-page redirect. The auth handshake's first half —
+  // protected-resource + auth-server discovery and Dynamic Client
+  // Registration — happens on the pre-redirect page; without persisting it
+  // those `auth` entries would vanish when the browser navigates to the
+  // authorization server. `FetchRequestLogState` saves to this on the
+  // client's `saveSession` event (fired in `onBeforeOAuthRedirect`) keyed by
+  // the OAuth authId, and restores from it when rebuilt on `/oauth/callback`.
+  // Created once; `getAuthToken()` is stable for the page's lifetime.
+  const sessionStorageAdapter = useMemo(
+    () =>
+      new RemoteInspectorClientStorage({
+        baseUrl:
+          typeof window !== "undefined"
+            ? window.location.origin
+            : "http://localhost",
+        authToken: getAuthToken(),
+      }),
+    [],
+  );
+
+  // Always points at the live `FetchRequestLogState` so the synchronous
+  // pre-redirect hook below can read the current Network log without being
+  // rebound every time the active server (and its log state) changes.
+  const fetchLogRef = useRef<FetchRequestLogState | null>(null);
+
+  // Flush the pre-redirect Network log to backend storage, keyed by the OAuth
+  // authId carried in the authorization URL's `state`. Runs synchronously from
+  // `BrowserNavigation` right before `window.location.href`, so the keepalive
+  // POST it kicks off outlives the unloading page. The `/oauth/callback`
+  // rebuild restores these entries via `FetchRequestLogState`'s `sessionId`.
+  // Stable identity: it reads mutable refs, so it never needs to be rebuilt.
+  const onBeforeOAuthRedirect = useCallback(
+    (authorizationUrl: URL) => {
+      const stateParam = authorizationUrl.searchParams.get("state");
+      const authId = stateParam
+        ? (parseOAuthState(stateParam)?.authId ?? undefined)
+        : undefined;
+      if (!authId) return;
+      const fetchRequests = fetchLogRef.current?.getFetchRequests() ?? [];
+      if (fetchRequests.length === 0) return;
+      const now = Date.now();
+      // Fire-and-forget: the keepalive request inside `saveSession` is
+      // dispatched synchronously here, before navigation commits.
+      void sessionStorageAdapter
+        .saveSession(authId, {
+          fetchRequests,
+          createdAt: now,
+          updatedAt: now,
+        })
+        .catch(() => {
+          // Best-effort; losing the pre-redirect log is non-fatal.
+        });
+    },
+    [sessionStorageAdapter],
+  );
+
   // Wire up + tear down per active server. Called by `onToggleConnection`
   // when the user switches targets. Returns the new client so the toggle
   // can call `connect()` against it before React re-renders.
   const setupClientForServer = useCallback(
-    (server: ServerEntry): InspectorClient => {
+    (server: ServerEntry, sessionId?: string): InspectorClient => {
       // Tear down the previous session's managers — each destroy()
       // unsubscribes from the old client's events. Skipped on the first
       // call (initial values are null).
@@ -471,6 +530,7 @@ function App() {
       const { environment } = createWebEnvironment(
         getAuthToken(),
         redirectUrlProvider,
+        onBeforeOAuthRedirect,
       );
       // The settings node persisted in mcp.json for this server — distinct
       // from the InspectorClient options we're about to derive from it.
@@ -519,6 +579,10 @@ function App() {
           }),
         ...(oauth && { oauth }),
         ...(savedSettings && { serverSettings: savedSettings }),
+        // Set on the `/oauth/callback` rebuild so the client's `saveSession`
+        // events (and any later persistence) key off the same OAuth authId
+        // the pre-redirect page saved under.
+        ...(sessionId && { sessionId }),
       });
 
       setInspectorClient(client);
@@ -538,7 +602,18 @@ function App() {
         new ResourceSubscriptionsState(client, nextResourcesState),
       );
       setMessageLogState(new MessageLogState(client));
-      setFetchRequestLogState(new FetchRequestLogState(client));
+      // Wire session storage so the fetch log survives the OAuth redirect.
+      // When `sessionId` is supplied (the `/oauth/callback` rebuild) the prior
+      // page's `auth` entries are restored on construction; the actual save is
+      // driven synchronously from `onBeforeOAuthRedirect` above (keyed by the
+      // same authId). Keep `fetchLogRef` pointed at this instance so that hook
+      // reads the current log.
+      const nextFetchLog = new FetchRequestLogState(client, {
+        sessionStorage: sessionStorageAdapter,
+        ...(sessionId && { sessionId }),
+      });
+      fetchLogRef.current = nextFetchLog;
+      setFetchRequestLogState(nextFetchLog);
       setStderrLogState(new StderrLogState(client));
 
       return client;
@@ -553,6 +628,8 @@ function App() {
       messageLogState,
       fetchRequestLogState,
       stderrLogState,
+      sessionStorageAdapter,
+      onBeforeOAuthRedirect,
     ],
   );
 
@@ -574,6 +651,14 @@ function App() {
     oauthCallbackHandledRef.current = true;
 
     const params = parseOAuthCallbackParams(window.location.search);
+    // The OAuth `state` round-trips `{mode}:{authId}`; the authId is the
+    // session key the pre-redirect page saved the fetch log under, so the
+    // rebuilt client can restore those `auth` entries. Read it before the
+    // URL is cleared below.
+    const stateParam = new URLSearchParams(window.location.search).get("state");
+    const sessionId = stateParam
+      ? (parseOAuthState(stateParam)?.authId ?? undefined)
+      : undefined;
     const pendingId =
       window.sessionStorage.getItem(OAUTH_PENDING_SERVER_KEY) ?? undefined;
     window.sessionStorage.removeItem(OAUTH_PENDING_SERVER_KEY);
@@ -609,7 +694,7 @@ function App() {
     }
 
     void (async () => {
-      const client = setupClientForServer(server);
+      const client = setupClientForServer(server, sessionId);
       setActiveServerId(server.id);
       // Two distinct failure modes get distinct toasts. A token-exchange
       // failure means OAuth did NOT complete — and since the single-use code

clients/web/src/lib/environmentFactory.ts

@@ -30,10 +30,16 @@ export interface WebEnvironmentResult {
  * `authToken` is read from a higher level (currently unused in this app since
  * v2 has no auth-token UI yet, but kept in the signature so the wiring is
  * ready when token plumbing lands).
+ *
+ * `onBeforeOAuthRedirect` runs synchronously immediately before the OAuth
+ * full-page redirect (see `BrowserNavigation`). The app uses it to flush the
+ * pre-redirect Network log to backend storage so the auth handshake's first
+ * half (discovery + Dynamic Client Registration) survives the navigation.
  */
 export function createWebEnvironment(
   authToken: string | undefined,
   redirectUrlProvider: RedirectUrlProvider,
+  onBeforeOAuthRedirect?: (authorizationUrl: URL) => void,
 ): WebEnvironmentResult {
   const baseUrl = `${window.location.protocol}//${window.location.host}`;
 
@@ -62,7 +68,7 @@ export function createWebEnvironment(
     logger,
     oauth: {
       storage: new BrowserOAuthStorage(),
-      navigation: new BrowserNavigation(),
+      navigation: new BrowserNavigation(undefined, onBeforeOAuthRedirect),
       redirectUrlProvider,
     },
   };

clients/web/src/test/core/auth/providers.test.ts

@@ -79,6 +79,39 @@ describe("OAuthNavigation", () => {
         "BrowserNavigation requires browser environment",
       );
     });
+
+    it("runs beforeNavigate synchronously BEFORE assigning location.href", () => {
+      // The pre-redirect persistence relies on this ordering: the hook must
+      // observe the still-current document (location.href not yet reassigned)
+      // so a keepalive request it fires outlives the navigation.
+      const order: string[] = [];
+      const authUrl = new URL("http://example.com/authorize?state=normal:abc");
+      const navigation = new BrowserNavigation(undefined, (url) => {
+        order.push("before");
+        // At hook time the redirect has not happened yet.
+        expect((global as GlobalWithWindow).window!.location.href).toBe(
+          "http://localhost:5173",
+        );
+        expect(url.toString()).toBe(authUrl.toString());
+      });
+
+      navigation.navigateToAuthorization(authUrl);
+      order.push("after");
+
+      expect(order).toEqual(["before", "after"]);
+      expect((global as GlobalWithWindow).window!.location.href).toBe(
+        authUrl.toString(),
+      );
+    });
+
+    it("still navigates when no beforeNavigate hook is provided", () => {
+      const navigation = new BrowserNavigation();
+      const authUrl = new URL("http://example.com/authorize");
+      navigation.navigateToAuthorization(authUrl);
+      expect((global as GlobalWithWindow).window!.location.href).toBe(
+        authUrl.toString(),
+      );
+    });
   });
 
   describe("BrowserOAuthClientProvider", () => {

clients/web/src/test/core/mcp/remote/sessionStorage.test.ts

@@ -160,4 +160,28 @@ describe("RemoteInspectorClientStorage", () => {
       ],
     ).toBeUndefined();
   });
+
+  it("defaults to a wrapper around globalThis.fetch (not the bare reference)", async () => {
+    // Regression guard: the default must call `globalThis.fetch` with the
+    // global as receiver. Assigning the bare reference and invoking it as
+    // `this.fetchFn(...)` throws "Illegal invocation" — which callers swallow
+    // via `.catch(() => {})`, so it would silently break all save/load.
+    const spy = vi
+      .spyOn(globalThis, "fetch")
+      .mockResolvedValue(new Response(null, { status: 204 }));
+    try {
+      const storage = new RemoteInspectorClientStorage({
+        baseUrl: "http://remote.example/",
+      });
+      await expect(
+        storage.saveSession("sid", sampleState()),
+      ).resolves.toBeUndefined();
+      expect(spy).toHaveBeenCalledTimes(1);
+      expect(spy.mock.calls[0]?.[0]).toBe(
+        "http://remote.example/api/storage/inspector-session-sid",
+      );
+    } finally {
+      spy.mockRestore();
+    }
+  });
 });

clients/web/src/test/core/mcp/state/fetchRequestLogState.test.ts

@@ -202,6 +202,73 @@ describe("FetchRequestLogState", () => {
     expect(hydrated.getFetchRequests().map((e) => e.id)).toEqual(["r2", "r3"]);
   });
 
+  it("merges restored entries ahead of live ones, deduping by id, when load resolves after a live append", async () => {
+    // Mirrors the `/oauth/callback` race: the resuming connect appends live
+    // entries while the persisted pre-redirect log is still loading. The
+    // restored (older) entries must land in front without clobbering the live
+    // ones, and an id present in both must not duplicate.
+    let resolveLoad:
+      | ((s: InspectorClientSessionState | undefined) => void)
+      | undefined;
+    const storage: InspectorClientStorage = {
+      async saveSession() {},
+      loadSession: () =>
+        new Promise((resolve) => {
+          resolveLoad = resolve;
+        }),
+      async deleteSession() {},
+    };
+    const hydrated = new FetchRequestLogState(client, {
+      sessionStorage: storage,
+      sessionId: "sess-merge",
+    });
+    // Live entries arrive before the load resolves.
+    client.dispatchTypedEvent("fetchRequest", entry("token"));
+    client.dispatchTypedEvent("fetchRequest", entry("transport"));
+    // Restored set includes the two pre-redirect entries plus a duplicate of
+    // a live one ("token").
+    resolveLoad?.({
+      fetchRequests: [entry("discovery"), entry("register"), entry("token")],
+      createdAt: 0,
+      updatedAt: 0,
+    });
+    await Promise.resolve();
+    await Promise.resolve();
+    expect(hydrated.getFetchRequests().map((e) => e.id)).toEqual([
+      "discovery",
+      "register",
+      "token",
+      "transport",
+    ]);
+    hydrated.destroy();
+  });
+
+  it("hydrate is a no-op when restored entries are all already present", async () => {
+    const storage = makeStorage({
+      "sess-dup": {
+        fetchRequests: [entry("a")],
+        createdAt: 0,
+        updatedAt: 0,
+      },
+    });
+    const hydrated = new FetchRequestLogState(client, {
+      sessionStorage: storage,
+      sessionId: "sess-dup",
+    });
+    const changes: FetchRequestEntry[][] = [];
+    hydrated.addEventListener("fetchRequestsChange", (e) =>
+      changes.push(e.detail),
+    );
+    client.dispatchTypedEvent("fetchRequest", entry("a"));
+    changes.length = 0; // ignore the live-append dispatch
+    await Promise.resolve();
+    await Promise.resolve();
+    // The only restored entry ("a") is already present → no merge, no event.
+    expect(hydrated.getFetchRequests().map((e) => e.id)).toEqual(["a"]);
+    expect(changes).toEqual([]);
+    hydrated.destroy();
+  });
+
   it("ignores hydration when destroy() happens first", async () => {
     let resolveLoad:
       | ((s: InspectorClientSessionState | undefined) => void)

core/auth/browser/providers.ts

@@ -9,15 +9,29 @@ export type { OAuthNavigationCallback } from "../providers.js";
 
 /**
  * Browser navigation handler
- * Redirects the browser window to the authorization URL, optionally invokes an
- * extra callback.
+ * Redirects the browser window to the authorization URL.
+ *
+ * `beforeNavigate` runs **synchronously, immediately before** the
+ * `window.location.href` assignment. It is the last point at which code can
+ * run on the soon-to-unload document, so consumers that need to flush state
+ * across the OAuth redirect (e.g. persisting the Network log via a `keepalive`
+ * request) must do it here — a request dispatched synchronously before
+ * navigation survives the unload, whereas one fired from a later microtask
+ * (after `auth()` returns) is dropped when the document tears down.
+ *
+ * `callback` runs after navigation is requested; kept for backwards
+ * compatibility with callers that only need a post-redirect notification.
  */
 export class BrowserNavigation extends CallbackNavigation {
-  constructor(callback?: OAuthNavigationCallback) {
+  constructor(
+    callback?: OAuthNavigationCallback,
+    beforeNavigate?: (authorizationUrl: URL) => void,
+  ) {
     super((url) => {
       if (typeof window === "undefined") {
         throw new Error("BrowserNavigation requires browser environment");
       }
+      beforeNavigate?.(url);
       window.location.href = url.href;
       return callback?.(url);
     });

core/mcp/remote/sessionStorage.ts

@@ -31,7 +31,12 @@ export class RemoteInspectorClientStorage implements InspectorClientStorage {
   constructor(options: RemoteInspectorClientStorageOptions) {
     this.baseUrl = options.baseUrl.replace(/\/$/, "");
     this.authToken = options.authToken;
-    this.fetchFn = options.fetchFn ?? globalThis.fetch;
+    // Default to a wrapper rather than `globalThis.fetch` directly: invoking
+    // the bare reference as `this.fetchFn(...)` re-binds `this` to this
+    // instance, which native `fetch` rejects with "Illegal invocation". The
+    // wrapper preserves the global receiver. (Same gotcha handled in
+    // `environmentFactory.ts` for the transport/auth fetchers.)
+    this.fetchFn = options.fetchFn ?? ((...args) => globalThis.fetch(...args));
   }
 
   private getStoreId(sessionId: string): string {
@@ -69,6 +74,17 @@ export class RemoteInspectorClientStorage implements InspectorClientStorage {
       method: "POST",
       headers,
       body: JSON.stringify(serializedState),
+      // `saveSession` runs as the OAuth full-page redirect is being
+      // scheduled, so a normal fetch would be cancelled mid-flight and the
+      // pre-redirect network log (discovery + DCR) would never reach disk.
+      // `keepalive` lets the request outlive the unloading document.
+      // Caveat: keepalive caps the body at 64KB. The pre-redirect-log payload
+      // is small, but this method is also reachable from
+      // `FetchRequestLogState`'s `saveSession` listener with the full session
+      // log — a long session could exceed the cap and be dropped silently.
+      // Acceptable here: the persisted log is a best-effort convenience, not
+      // load-bearing state.
+      keepalive: true,
     });
 
     if (!res.ok) {