Guard Plugins page reloads against a stale tenant's late fetch

apps/web/src/pages/plugins-page.tsx

@@ -24,7 +24,7 @@ import {
 import type { ResolvedPlugin } from "@workbench/connections/plugins";
 import { listPluginsForTenant } from "@workbench/connections/plugins";
 import { Plus, SquaresFour, Warning } from "@corbits/icons";
-import { useCallback, useEffect, useState } from "react";
+import { useCallback, useEffect, useRef, useState } from "react";
 
 import { useBench } from "../bench-context";
 import { SKILLS_PATH_PREFIX } from "../path-ids";
@@ -92,33 +92,65 @@ export function PluginsRoute({
     setConnectDeepLinkNotFound(false);
   }, []);
 
+  const [pluginsReloadKey, setPluginsReloadKey] = useState(0);
+  const [skillsReloadKey, setSkillsReloadKey] = useState(0);
+
   const reloadPlugins = useCallback(() => {
-    if (selectedTenantId === null) return;
-    listPluginsForTenant(selectedTenantId)
-      .then((plugins) => setPluginsState({ status: "ready", plugins }))
-      .catch((cause: unknown) =>
-        setPluginsState({ status: "error", message: messageOf(cause) }),
-      );
-  }, [selectedTenantId]);
+    setPluginsReloadKey((key) => key + 1);
+  }, []);
 
   const reloadSkills = useCallback(() => {
-    if (selectedTenantId === null) return;
-    listSkills(selectedTenantId)
-      .then((skills) => setSkillsState({ status: "ready", skills }))
-      .catch((cause: unknown) =>
-        setSkillsState({ status: "error", message: messageOf(cause) }),
-      );
-  }, [selectedTenantId]);
+    setSkillsReloadKey((key) => key + 1);
+  }, []);
+
+  // Guarded by `cancelled` (same pattern as settings-ui's `people-section`)
+  // so a tenant switch mid-flight can't have the previous tenant's late
+  // response overwrite the newly selected tenant's state. The loading
+  // skeleton only shows for a tenant this page hasn't fetched yet — an
+  // imperative reload (a connect/disconnect's `onChanged`, the error
+  // screen's Retry) keeps whatever is already on screen and swaps in the
+  // fresh data once it lands, the same way it worked before cancellation
+  // was added.
+  const pluginsLoadedTenantRef = useRef<string | null>(null);
+  const skillsLoadedTenantRef = useRef<string | null>(null);
 
   useEffect(() => {
-    setPluginsState({ status: "loading" });
-    reloadPlugins();
-  }, [reloadPlugins]);
+    if (selectedTenantId === null) return;
+    let cancelled = false;
+    const isTenantChange = pluginsLoadedTenantRef.current !== selectedTenantId;
+    pluginsLoadedTenantRef.current = selectedTenantId;
+    if (isTenantChange) setPluginsState({ status: "loading" });
+    listPluginsForTenant(selectedTenantId)
+      .then((plugins) => {
+        if (!cancelled) setPluginsState({ status: "ready", plugins });
+      })
+      .catch((cause: unknown) => {
+        if (!cancelled)
+          setPluginsState({ status: "error", message: messageOf(cause) });
+      });
+    return () => {
+      cancelled = true;
+    };
+  }, [selectedTenantId, pluginsReloadKey]);
 
   useEffect(() => {
-    setSkillsState({ status: "loading" });
-    reloadSkills();
-  }, [reloadSkills]);
+    if (selectedTenantId === null) return;
+    let cancelled = false;
+    const isTenantChange = skillsLoadedTenantRef.current !== selectedTenantId;
+    skillsLoadedTenantRef.current = selectedTenantId;
+    if (isTenantChange) setSkillsState({ status: "loading" });
+    listSkills(selectedTenantId)
+      .then((skills) => {
+        if (!cancelled) setSkillsState({ status: "ready", skills });
+      })
+      .catch((cause: unknown) => {
+        if (!cancelled)
+          setSkillsState({ status: "error", message: messageOf(cause) });
+      });
+    return () => {
+      cancelled = true;
+    };
+  }, [selectedTenantId, skillsReloadKey]);
 
   // The shell banner's "Fix it" deep link (CL-6092): once the gallery has
   // loaded, pick up any pending provider id and open its connect panel —

apps/web/test/plugins-page.test.tsx

@@ -6,11 +6,13 @@
 // data into that component correctly.
 
 import { afterEach, describe, expect, test } from "bun:test";
-import { act } from "react";
+import { act, useState } from "react";
+import type { ReactNode } from "react";
 import { createRoot } from "react-dom/client";
 import type { Root } from "react-dom/client";
 
-import { BenchProvider } from "../src/bench-context";
+import type { BenchState } from "../src/bench-context";
+import { BenchContext, BenchProvider } from "../src/bench-context";
 import { NavigationProvider } from "../src/navigation";
 import { PluginsRoute } from "../src/pages/plugins-page";
 import {
@@ -412,4 +414,250 @@ describe("PluginsRoute", () => {
 
     expect(navigated).toContain("/skills/summarize");
   });
+
+  // CL-7138: a fast bench switch used to let the previous tenant's
+  // in-flight fetch resolve after the new tenant's, overwriting its data.
+  test("a fast bench switch never lets the previous tenant's late fetch overwrite the new one's data", async () => {
+    function deferredResponse() {
+      let resolve: (response: Response) => void = () => undefined;
+      const promise = new Promise<Response>((res) => {
+        resolve = res;
+      });
+      return { promise, resolve };
+    }
+
+    const skillsDeferred: Record<
+      string,
+      ReturnType<typeof deferredResponse>
+    > = {
+      tnt_a: deferredResponse(),
+      tnt_b: deferredResponse(),
+    };
+
+    globalThis.fetch = ((input: RequestInfo | URL) => {
+      const path = typeof input === "string" ? input : String(input);
+      if (path.includes("/credentials/resolve/"))
+        return Promise.resolve(json(null, 404));
+      if (path.includes("/connections/provider-health"))
+        return Promise.resolve(
+          json({ providers: {}, connectedProviderCount: 0 }),
+        );
+      const skillsMatch = /\/api\/tenants\/(tnt_[ab])\/skills/.exec(path);
+      if (skillsMatch) {
+        const tenantId = skillsMatch[1] as string;
+        return (
+          skillsDeferred[tenantId]?.promise ??
+          Promise.resolve(json({ skills: [] }))
+        );
+      }
+      return Promise.resolve(json({ data: [], nextCursor: null }));
+    }) as typeof fetch;
+
+    let selectTenant: (tenantId: string) => void = () => undefined;
+
+    function BenchHarness({ children }: { readonly children: ReactNode }) {
+      const [tenantId, setTenantId] = useState("tnt_a");
+      selectTenant = setTenantId;
+      const value: BenchState = {
+        memberships: { kind: "loading" },
+        selectedTenantId: tenantId,
+        selectedPrincipalId: "prn_1",
+        selectTenant: setTenantId,
+        onBenchCreated: () => undefined,
+      };
+      return (
+        <BenchContext.Provider value={value}>{children}</BenchContext.Provider>
+      );
+    }
+
+    container = document.createElement("div");
+    document.body.appendChild(container);
+    root = createRoot(container);
+    await act(async () => {
+      root?.render(
+        <TestQueryProvider>
+          <NavigationProvider navigate={() => undefined}>
+            <BenchHarness>
+              <ProviderHealthProvider>
+                <PluginsRoute path="/plugins" navigate={() => undefined} />
+              </ProviderHealthProvider>
+            </BenchHarness>
+          </NavigationProvider>
+        </TestQueryProvider>,
+      );
+    });
+    for (let i = 0; i < 5; i++) {
+      await act(async () => {
+        await new Promise((resolve) => setTimeout(resolve, 0));
+      });
+    }
+
+    act(() => {
+      selectTenant("tnt_b");
+    });
+    for (let i = 0; i < 5; i++) {
+      await act(async () => {
+        await new Promise((resolve) => setTimeout(resolve, 0));
+      });
+    }
+
+    skillsDeferred.tnt_b?.resolve(
+      json({
+        skills: [
+          {
+            assetId: "skill_b",
+            name: "beta-only",
+            description: "Tenant B's skill.",
+            scope: "tenant",
+            creatorPrincipalId: "prn_1",
+            updatedAtIso: "2026-01-01T00:00:00.000Z",
+          },
+        ],
+      }),
+    );
+    for (let i = 0; i < 10; i++) {
+      await act(async () => {
+        await new Promise((resolve) => setTimeout(resolve, 0));
+      });
+    }
+
+    skillsDeferred.tnt_a?.resolve(
+      json({
+        skills: [
+          {
+            assetId: "skill_a",
+            name: "alpha-only",
+            description: "Tenant A's skill.",
+            scope: "tenant",
+            creatorPrincipalId: "prn_1",
+            updatedAtIso: "2026-01-01T00:00:00.000Z",
+          },
+        ],
+      }),
+    );
+    for (let i = 0; i < 10; i++) {
+      await act(async () => {
+        await new Promise((resolve) => setTimeout(resolve, 0));
+      });
+    }
+
+    const skillsTab = [...container.querySelectorAll("button")].find(
+      (button) => button.textContent?.includes("Skills") === true,
+    );
+    act(() => {
+      skillsTab?.dispatchEvent(new MouseEvent("click", { bubbles: true }));
+    });
+
+    expect(container.textContent).toContain("beta-only");
+    expect(container.textContent).not.toContain("alpha-only");
+  });
+
+  // CL-7138: cancellation must not turn every imperative reload (a
+  // connect/disconnect panel's `onChanged`, the error screen's Retry) into
+  // a full teardown to the loading skeleton — only a genuine tenant change
+  // should do that. This drives the real disconnect flow end to end.
+  test("disconnecting a plugin reloads without tearing the gallery down to the loading skeleton", async () => {
+    let deferCredentialResolves = false;
+    const deferredResolvers: (() => void)[] = [];
+    const githubCredential = {
+      id: "cred_1",
+      tenantId: "tnt_1",
+      providerId: "prov_1",
+      principalId: null,
+      oauthClientId: null,
+      name: "GitHub",
+      type: "api_key",
+      description: null,
+      scopes: [],
+      expiresAt: null,
+      status: "active",
+      metadata: {},
+      createdAt: "2026-01-01T00:00:00.000Z",
+      updatedAt: "2026-01-01T00:00:00.000Z",
+    };
+
+    globalThis.fetch = ((input: RequestInfo | URL, init?: RequestInit) => {
+      const path = typeof input === "string" ? input : String(input);
+      const method = (init?.method ?? "GET").toUpperCase();
+      if (method === "DELETE" && path.includes("/credentials/cred_1"))
+        return Promise.resolve(new Response(null, { status: 204 }));
+      if (path.includes("/api/me/principals"))
+        return Promise.resolve(json(membership));
+      if (path.includes("/api/workbench-tenancies/kinds"))
+        return Promise.resolve(json({ workbenchTenantIds: [] }));
+      if (path.includes("/credentials/resolve/GitHub")) {
+        if (deferCredentialResolves) {
+          return new Promise<Response>((resolve) => {
+            deferredResolvers.push(() => resolve(json(null, 404)));
+          });
+        }
+        return Promise.resolve(json(githubCredential));
+      }
+      if (path.includes("/connections/provider-health"))
+        return Promise.resolve(
+          json({ providers: {}, connectedProviderCount: 1 }),
+        );
+      if (path.includes("/credentials/resolve/"))
+        return Promise.resolve(json(null, 404));
+      if (path.includes("/api/tenants/tnt_1/skills"))
+        return Promise.resolve(json({ skills: [] }));
+      return Promise.resolve(json({ data: [], nextCursor: null }));
+    }) as typeof fetch;
+
+    const el = await mount();
+    expect(el.textContent).toContain("Connected here");
+
+    const manageButton = el.querySelector<HTMLButtonElement>(
+      'button[aria-label="Manage GitHub"]',
+    );
+    await act(async () => {
+      manageButton?.click();
+    });
+    for (let i = 0; i < 5; i++) {
+      await act(async () => {
+        await new Promise((resolve) => setTimeout(resolve, 0));
+      });
+    }
+
+    const disconnectButton = () =>
+      [...document.body.querySelectorAll("button")].find(
+        (button) => button.textContent?.includes("Disconnect") === true,
+      );
+    expect(disconnectButton()).not.toBeUndefined();
+
+    // From here on, the reload the disconnect triggers must not resolve
+    // until we say so — gives us a window to inspect the mid-reload DOM.
+    deferCredentialResolves = true;
+
+    // First click arms the confirm button, second click fires the delete.
+    await act(async () => {
+      disconnectButton()?.dispatchEvent(
+        new MouseEvent("click", { bubbles: true }),
+      );
+    });
+    await act(async () => {
+      disconnectButton()?.dispatchEvent(
+        new MouseEvent("click", { bubbles: true }),
+      );
+    });
+    for (let i = 0; i < 5; i++) {
+      await act(async () => {
+        await new Promise((resolve) => setTimeout(resolve, 0));
+      });
+    }
+
+    // The delete resolved and `onChanged` fired `reloadPlugins`; its fetch
+    // is now the deferred one above, still pending.
+    expect(el.textContent).not.toContain("Loading plugins…");
+    expect(el.textContent).toContain("GitHub");
+
+    deferredResolvers.forEach((resolve) => resolve());
+    for (let i = 0; i < 10; i++) {
+      await act(async () => {
+        await new Promise((resolve) => setTimeout(resolve, 0));
+      });
+    }
+
+    expect(el.textContent).not.toContain("Connected here");
+  });
 });