diff --git a/apps/web/src/pages/plugins-page.tsx b/apps/web/src/pages/plugins-page.tsx index bc7bad089..31979f871 100644 --- a/apps/web/src/pages/plugins-page.tsx +++ b/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(null); + const skillsLoadedTenantRef = useRef(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 — diff --git a/apps/web/test/plugins-page.test.tsx b/apps/web/test/plugins-page.test.tsx index 1f92dc530..e47281b92 100644 --- a/apps/web/test/plugins-page.test.tsx +++ b/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((res) => { + resolve = res; + }); + return { promise, resolve }; + } + + const skillsDeferred: Record< + string, + ReturnType + > = { + 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 ( + {children} + ); + } + + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + await act(async () => { + root?.render( + + undefined}> + + + undefined} /> + + + + , + ); + }); + 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((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( + '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"); + }); });