fix: key the in-flight clear guard on the OAuth storage key too (#2217)

Copilot: `runClear`'s double-click guard deduped by `server.id`, which is
the same identity mismatch this PR fixes, one layer up. Two catalog
entries against one URL share one credential blob, one grant and one
revocation, so an id-keyed guard let exactly the race it exists to
prevent through between them — concurrent store writes, concurrent RFC
7009 requests, and two contradictory teardown toasts.

The guard now keys on `oauthClearKey`, a pure helper that resolves the
OAuth storage key and falls back to the entry id for a config that has
none (stdio), where there is no shared state to collide over. The id
fallback keeps such entries distinct from each other rather than
collapsing them onto one key, and both forms are prefixed so a
user-supplied id can never impersonate a URL.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VGbw6TezNWnXZSCSp7sqb4
Signed-off-by: cliffhall <cliff@futurescale.com>

clients/web/src/App.tsx

@@ -115,6 +115,7 @@ import { AuthRecoveryRequiredError } from "@inspector/core/auth/challenge.js";
 import { getAuthToken } from "./lib/authToken";
 import { messagesToLogEntries } from "./lib/protocolReplay";
 import { EMPTY_SETTINGS } from "./utils/serverSettingsDefaults";
+import { oauthClearKey } from "./utils/oauthClearKey";
 import {
   bodyDroppedToastId,
   CLIENT_CONFIG_LOAD_ERROR_NOTIFICATION_ID,
@@ -1350,10 +1351,10 @@ function App() {
   const settingsModalIsStdio = settingsModalServerType === "stdio";
 
   /**
-   * Servers whose clear is in flight (#2144). Keyed by id, not a single flag:
-   * the callback explicitly supports clearing a server other than the active
-   * one, so a global lock would silently drop B's click while A's revocation
-   * was still out. See `runClear`.
+   * Servers whose clear is in flight (#2144). Keyed per server, not a single
+   * flag: the callback explicitly supports clearing a server other than the
+   * active one, so a global lock would silently drop B's click while A's
+   * revocation was still out. See `runClear`.
    */
   const clearOAuthInFlightRef = useRef<Set<string>>(new Set());
 
@@ -1375,11 +1376,18 @@ function App() {
       // — and with revocation taking up to five seconds, that means concurrent
       // RFC 7009 requests, concurrent store writes, and two contradictory
       // toasts. Keyed by server so a *different* server's clear is unaffected.
-      if (clearOAuthInFlightRef.current.has(server.id)) return;
-      clearOAuthInFlightRef.current.add(server.id);
+      //
+      // "Different server" is the OAuth storage key, not the catalog id
+      // (#2217, Copilot): two entries against one URL share one blob, one
+      // grant and one revocation, so an id-keyed guard lets exactly the race
+      // above through between them. `oauthClearKey` falls back to the id for a
+      // config with no OAuth URL, which has no shared state to collide over.
+      const inFlightKey = oauthClearKey(server.config, server.id);
+      if (clearOAuthInFlightRef.current.has(inFlightKey)) return;
+      clearOAuthInFlightRef.current.add(inFlightKey);
       clearServerOAuthAndDisconnect(server)
         .finally(() => {
-          clearOAuthInFlightRef.current.delete(server.id);
+          clearOAuthInFlightRef.current.delete(inFlightKey);
         })
         .catch((err: unknown) => {
           notifications.show({

clients/web/src/utils/oauthClearKey.test.ts

@@ -0,0 +1,38 @@
+import { describe, expect, it } from "vitest";
+import { oauthClearKey } from "./oauthClearKey";
+
+describe("oauthClearKey", () => {
+  // #2217 — the whole point: two entries against one URL share one OAuth blob,
+  // so they must share one clear identity even though their ids differ.
+  it("keys two entries with the same URL identically", () => {
+    const config = {
+      type: "streamable-http",
+      url: "https://mcp.example/mcp",
+    } as const;
+    expect(oauthClearKey(config, "a")).toBe(oauthClearKey(config, "b"));
+  });
+
+  it("separates entries pointing at different URLs", () => {
+    expect(
+      oauthClearKey({ type: "sse", url: "https://one.example/mcp" }, "a"),
+    ).not.toBe(
+      oauthClearKey({ type: "sse", url: "https://two.example/mcp" }, "a"),
+    );
+  });
+
+  it("falls back to the entry id when there is no OAuth server URL", () => {
+    const stdio = { type: "stdio", command: "node" } as const;
+    expect(oauthClearKey(stdio, "a")).toBe("id:a");
+    expect(oauthClearKey(stdio, "a")).not.toBe(oauthClearKey(stdio, "b"));
+  });
+
+  // The prefix is what keeps a stdio entry named "url:…" from colliding with a
+  // URL-keyed one — an id is user-supplied and a URL is not a reserved shape.
+  it("cannot collide a URL key with an id key", () => {
+    expect(
+      oauthClearKey({ type: "stdio", command: "node" }, "url:https://x/mcp"),
+    ).not.toBe(
+      oauthClearKey({ type: "sse", url: "https://x/mcp" }, "anything"),
+    );
+  });
+});

clients/web/src/utils/oauthClearKey.ts

@@ -0,0 +1,23 @@
+import { getOAuthServerUrl } from "@inspector/core/mcp/config.js";
+import type { MCPServerConfig } from "@inspector/core/mcp/types.js";
+
+/**
+ * The identity a *clear* of stored OAuth state acts on.
+ *
+ * Persisted OAuth state is keyed by the server URL, and `core/mcp/serverList`
+ * enforces no URL uniqueness — so two catalog entries with distinct ids can
+ * point at one URL and therefore share one credential blob, one live grant and
+ * one revocation. Anything that dedupes, locks or compares *clears* has to key
+ * on that shared identity rather than on the entry id, which cannot see it
+ * (#2217).
+ *
+ * A config with no OAuth server URL (stdio, and anything else
+ * `getOAuthServerUrl` declines) has no shared blob to collide over, so it falls
+ * back to the entry id — which keeps every such entry distinct from every
+ * other, rather than collapsing them all onto one shared key.
+ */
+export function oauthClearKey(config: MCPServerConfig, id: string): string {
+  const url = getOAuthServerUrl(config);
+  // Prefixed so a URL-keyed entry can never collide with an id-keyed one.
+  return url !== undefined ? `url:${url}` : `id:${id}`;
+}