test: cover the App-level shared-key clear suppression (#2217)

Copilot: `oauthClearKey`'s unit tests prove the derivation, but a
regression that dropped the `has` check, added or deleted the wrong key,
or invoked the clear twice would still have passed.

Adds an App.test.tsx case that connects entry A, holds its clear open on
a deferred `clearOAuthTokens`, then drives a clear for entry B — a
different catalog id against the same URL — and asserts the underlying
clear ran exactly once, and once more after the first settles, so the
key is proven released rather than leaking a permanently dead control.
Verified to fail against the id-keyed guard.

Two supporting changes it needs:

- an `open-settings-b` control on the mocked InspectorView, the only way
  to reach a settings modal for a server other than the active one;
- `@inspector/core/mcp/remote/index.js` becomes a partial mock.
  `getWebProxiedFetch` reaches for `createRemoteFetch` there, and the
  bare stub threw a missing-export error before the clear under test ever
  ran — the failure surfaced as the generic "Could not clear the stored
  OAuth state" toast.

The case carries an explicit 20s timeout and `delay: null`: it drives
three full modal interaction sequences against the whole App tree, which
runs past the 5s default when the suite is under load.

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.test.tsx

@@ -244,7 +244,13 @@ vi.mock("@inspector/core/mcp/state/stderrLogState.js", () => ({
   }),
 }));
 
-vi.mock("@inspector/core/mcp/remote/index.js", () => ({
+vi.mock("@inspector/core/mcp/remote/index.js", async (importOriginal) => ({
+  // Partial: `getWebProxiedFetch` (the OAuth clear's revocation fetch, #2144)
+  // reaches for `createRemoteFetch` from this module, and a bare stub would
+  // throw a missing-export error before the clear under test ever ran.
+  ...(await importOriginal<
+    typeof import("@inspector/core/mcp/remote/index.js")
+  >()),
   RemoteInspectorClientStorage: vi.fn(function () {
     return { saveSession: vi.fn() };
   }),
@@ -601,6 +607,11 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({
       <button onClick={() => props.servers.onServerSettings("A")}>
         open-settings
       </button>
+      {/* A second settings target, so a test can drive a clear against an
+          entry other than the active one (#2217). */}
+      <button onClick={() => props.servers.onServerSettings("B")}>
+        open-settings-b
+      </button>
       {/* The real server grid (and its Add / Edit controls) lives inside this
           mocked view, so the config modal is only reachable through these
           callbacks — and the highlight batch only observable through this prop. */}
@@ -4214,3 +4225,119 @@ describe("App MCP App listed-resource metadata wiring (#2055)", () => {
     }
   });
 });
+
+// #2217 — persisted OAuth state is keyed by server URL and `serverList`
+// enforces no URL uniqueness, so two catalog entries can share one credential
+// blob, one grant and one revocation. `runClear`'s in-flight guard used to
+// dedupe by catalog id, which cannot see that: clearing both entries while the
+// first revocation was still out ran concurrent store writes, concurrent RFC
+// 7009 requests and two contradictory teardown toasts — the exact race the
+// guard exists to prevent (Copilot, on this PR).
+describe("App dedupes concurrent OAuth clears that share a storage key (#2217)", () => {
+  const SHARED_URL = "https://shared.example/mcp";
+  const sharedEntry = (id: string, name: string): ServerEntry => ({
+    id,
+    name,
+    config: { type: "streamable-http", url: SHARED_URL },
+    connection: { status: "disconnected" },
+  });
+
+  let previousUseServers: typeof useServers | undefined;
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+    clientInstances.length = 0;
+    previousUseServers = vi.mocked(useServers).getMockImplementation();
+    vi.mocked(useInspectorClient).mockReturnValue(DEFAULT_USE_INSPECTOR_CLIENT);
+    vi.mocked(useServers).mockReturnValue({
+      servers: [sharedEntry("A", "PlotRocket"), sharedEntry("B", "Same URL")],
+      loading: false,
+      error: undefined,
+      refresh: vi.fn().mockResolvedValue(undefined),
+      addServer: addServerSpy,
+      updateServer: updateServerSpy,
+      updateServerSettings: updateServerSettingsSpy,
+      removeServer: vi.fn(),
+    } as unknown as ReturnType<typeof useServers>);
+  });
+
+  afterEach(() => {
+    if (previousUseServers) {
+      vi.mocked(useServers).mockImplementation(previousUseServers);
+    }
+  });
+
+  /** Open the settings modal for `which` and press its OAuth-section clear. */
+  async function clearFromSettings(
+    user: ReturnType<typeof userEvent.setup>,
+    which: "open-settings" | "open-settings-b",
+  ): Promise<void> {
+    await user.click(screen.getByText(which));
+    // The control lives in an accordion section. The modal stays mounted
+    // across a target switch, so the section may already be open from a
+    // previous call — toggling it again would close it.
+    const section = await screen.findByRole("button", {
+      name: "OAuth Settings",
+    });
+    if (section.getAttribute("aria-expanded") !== "true") {
+      await user.click(section);
+    }
+    await user.click(
+      await screen.findByRole("button", { name: "Clear stored OAuth state" }),
+    );
+  }
+
+  // Three full modal interaction sequences against the whole App tree, so this
+  // one runs long enough to trip the 5s default when the suite is under load.
+  it(
+    "suppresses a second clear for another entry with the same URL, and allows one after it settles",
+    { timeout: 20000 },
+    async () => {
+      // `delay: null` drops userEvent's inter-event waits, which dominate here.
+      const user = userEvent.setup({ delay: null });
+      renderWithMantine(<App />);
+
+      await user.click(screen.getByText("connect"));
+      await waitFor(() => expect(clientInstances).toHaveLength(1));
+      const client = clientInstances[0] as unknown as {
+        clearOAuthTokens: ReturnType<typeof vi.fn>;
+      };
+
+      // Hold the first clear open, as a pending RFC 7009 request would.
+      let settle: (v: { status: string; reason: string }) => void = () => {};
+      client.clearOAuthTokens.mockImplementation(
+        () =>
+          new Promise((resolve) => {
+            settle = resolve as typeof settle;
+          }),
+      );
+
+      await clearFromSettings(user, "open-settings");
+      await waitFor(() =>
+        expect(client.clearOAuthTokens).toHaveBeenCalledTimes(1),
+      );
+
+      // Entry B: a different catalog id, the same OAuth storage key. Both clears
+      // route through the live client precisely because they share that key, so
+      // an id-keyed guard would let this second one straight through.
+      await clearFromSettings(user, "open-settings-b");
+      expect(client.clearOAuthTokens).toHaveBeenCalledTimes(1);
+
+      // Suppression is for the duration of the in-flight clear only — the key
+      // must be released when it settles, or the control is dead for the rest of
+      // the session.
+      await act(async () => {
+        settle({ status: "skipped", reason: "no_endpoint" });
+        await Promise.resolve();
+      });
+      client.clearOAuthTokens.mockResolvedValue({
+        status: "skipped",
+        reason: "no_endpoint",
+      });
+      await clearFromSettings(user, "open-settings-b");
+      await waitFor(() =>
+        expect(client.clearOAuthTokens).toHaveBeenCalledTimes(2),
+      );
+    },
+  );
+});