diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 2e61e1125..8faaebfdd 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -107,9 +107,7 @@ import { useResourceSubscriptions } from "@inspector/core/react/useResourceSubsc import { useMessageLog } from "@inspector/core/react/useMessageLog.js"; import { useFetchRequestLog } from "@inspector/core/react/useFetchRequestLog.js"; import { useStderrLog } from "@inspector/core/react/useStderrLog.js"; -import { useSandboxUrl } from "@inspector/core/react/useSandboxUrl.js"; -import { useServerListWritable } from "@inspector/core/react/useServerListWritable.js"; -import { useInspectorVersion } from "@inspector/core/react/useInspectorVersion.js"; +import { useInitialConfig } from "@inspector/core/react/useInitialConfig.js"; import { usePendingClientRequests } from "@inspector/core/react/usePendingClientRequests.js"; import { InspectorView } from "./components/views/InspectorView/InspectorView"; import type { @@ -697,19 +695,16 @@ function App() { const appRendererRef = useRef(null); const configBaseUrl = typeof window !== "undefined" ? window.location.origin : "http://localhost"; - const { sandboxUrl } = useSandboxUrl({ - baseUrl: configBaseUrl, - authToken: getAuthToken(), - }); - // Read-only sessions (launched with `--config` or an ad-hoc server) hide - // catalog CRUD; the default catalog and `--catalog` stay writable. - const { writable: serverListWritable } = useServerListWritable({ - baseUrl: configBaseUrl, - authToken: getAuthToken(), - }); - // The Inspector version (root package.json), shown in the lower-right corner. - // The browser can't read it off disk, so the backend sends it via /api/config. - const { version: inspectorVersion } = useInspectorVersion({ + // One `GET /api/config` fetch recovers every static payload field the app + // reads: the MCP Apps `sandboxUrl`, the session's `writable` flag (read-only + // `--config` / ad-hoc sessions hide catalog CRUD; the default catalog and + // `--catalog` stay writable), and the Inspector `version` shown in the + // lower-right corner (the browser can't read it off disk). + const { + sandboxUrl, + writable: serverListWritable, + version: inspectorVersion, + } = useInitialConfig({ baseUrl: configBaseUrl, authToken: getAuthToken(), }); diff --git a/clients/web/src/test/core/react/useSandboxUrl.test.tsx b/clients/web/src/test/core/react/useInitialConfig.test.tsx similarity index 52% rename from clients/web/src/test/core/react/useSandboxUrl.test.tsx rename to clients/web/src/test/core/react/useInitialConfig.test.tsx index 152a5f3e1..3d246e881 100644 --- a/clients/web/src/test/core/react/useSandboxUrl.test.tsx +++ b/clients/web/src/test/core/react/useInitialConfig.test.tsx @@ -1,12 +1,15 @@ /** - * Tests for useSandboxUrl — runs in happy-dom under the unit project. Uses a + * Tests for useInitialConfig — runs in happy-dom under the unit project. Uses a * controlled fake `fetch` so each branch (present / absent / non-string / HTTP - * error / network throw) and the auth header are asserted directly. + * error / network throw / post-unmount guards) and the auth header are asserted + * directly. This is the single hook that replaced useSandboxUrl / + * useServerListWritable / useInspectorVersion (#1643), so it covers each field's + * branch matrix in one place. */ import { describe, it, expect, vi } from "vitest"; import { renderHook, waitFor } from "@testing-library/react"; -import { useSandboxUrl } from "@inspector/core/react/useSandboxUrl"; +import { useInitialConfig } from "@inspector/core/react/useInitialConfig"; function jsonResponse(body: unknown, ok = true): Response { return { @@ -16,30 +19,40 @@ function jsonResponse(body: unknown, ok = true): Response { } as unknown as Response; } -describe("useSandboxUrl", () => { - it("starts loading, then resolves the sandboxUrl from the config payload", async () => { - const fetchFn = vi - .fn() - .mockResolvedValue( - jsonResponse({ sandboxUrl: "http://localhost:6299/sandbox" }), - ); +describe("useInitialConfig", () => { + it("starts loading, then resolves all three fields from one config payload", async () => { + const fetchFn = vi.fn().mockResolvedValue( + jsonResponse({ + version: "2.0.0", + sandboxUrl: "http://localhost:6299/sandbox", + writable: false, + }), + ); const { result } = renderHook(() => - useSandboxUrl({ baseUrl: "http://test.local", fetchFn }), + useInitialConfig({ baseUrl: "http://test.local", fetchFn }), ); + // Initial (pre-fetch) state: version/sandboxUrl undefined, writable defaults + // true, loading true. expect(result.current.loading).toBe(true); + expect(result.current.version).toBeUndefined(); expect(result.current.sandboxUrl).toBeUndefined(); + expect(result.current.writable).toBe(true); await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.version).toBe("2.0.0"); expect(result.current.sandboxUrl).toBe("http://localhost:6299/sandbox"); + expect(result.current.writable).toBe(false); + // One static payload, one request. + expect(fetchFn).toHaveBeenCalledTimes(1); }); it("sends the bearer auth header when a token is provided", async () => { const fetchFn = vi.fn().mockResolvedValue(jsonResponse({})); renderHook(() => - useSandboxUrl({ + useInitialConfig({ baseUrl: "http://test.local/", authToken: "secret-token", fetchFn, @@ -57,77 +70,119 @@ describe("useSandboxUrl", () => { it("omits the auth header when no token is provided", async () => { const fetchFn = vi.fn().mockResolvedValue(jsonResponse({})); - renderHook(() => useSandboxUrl({ baseUrl: "http://test.local", fetchFn })); + renderHook(() => + useInitialConfig({ baseUrl: "http://test.local", fetchFn }), + ); await waitFor(() => expect(fetchFn).toHaveBeenCalled()); const [, init] = fetchFn.mock.calls[0]; expect(init.headers["x-mcp-remote-auth"]).toBeUndefined(); }); - it("leaves sandboxUrl undefined when the payload omits it", async () => { + it("applies each field's default when the payload omits it", async () => { const fetchFn = vi .fn() .mockResolvedValue(jsonResponse({ defaultEnvironment: {} })); const { result } = renderHook(() => - useSandboxUrl({ baseUrl: "http://test.local", fetchFn }), + useInitialConfig({ baseUrl: "http://test.local", fetchFn }), ); await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.version).toBeUndefined(); expect(result.current.sandboxUrl).toBeUndefined(); + // Missing writable (legacy backend) stays writable. + expect(result.current.writable).toBe(true); }); - it("leaves sandboxUrl undefined when the field is not a usable string", async () => { - const fetchFn = vi.fn().mockResolvedValue(jsonResponse({ sandboxUrl: "" })); + it("leaves version/sandboxUrl undefined when the fields are not usable strings", async () => { + const fetchFn = vi + .fn() + .mockResolvedValue(jsonResponse({ version: "", sandboxUrl: "" })); const { result } = renderHook(() => - useSandboxUrl({ baseUrl: "http://test.local", fetchFn }), + useInitialConfig({ baseUrl: "http://test.local", fetchFn }), ); await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.version).toBeUndefined(); expect(result.current.sandboxUrl).toBeUndefined(); }); - it("leaves sandboxUrl undefined on a non-ok response", async () => { + // Only an explicit `false` flips the list read-only. A nonconforming backend + // could send a falsy-but-not-false value (null / 0 / a string); each is + // `!== false`, so each must leave the list writable. + it.each([null, 0, "no"])( + "keeps writable true for writable=%j (falsy but not false)", + async (value) => { + const fetchFn = vi + .fn() + .mockResolvedValue(jsonResponse({ writable: value })); + + const { result } = renderHook(() => + useInitialConfig({ baseUrl: "http://test.local", fetchFn }), + ); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.writable).toBe(true); + }, + ); + + it("applies defaults on a non-ok response", async () => { const fetchFn = vi .fn() - .mockResolvedValue(jsonResponse({ sandboxUrl: "x" }, false)); + .mockResolvedValue( + jsonResponse( + { version: "9.9.9", sandboxUrl: "http://x/sb", writable: false }, + false, + ), + ); const { result } = renderHook(() => - useSandboxUrl({ baseUrl: "http://test.local", fetchFn }), + useInitialConfig({ baseUrl: "http://test.local", fetchFn }), ); await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.version).toBeUndefined(); expect(result.current.sandboxUrl).toBeUndefined(); + expect(result.current.writable).toBe(true); }); - it("leaves sandboxUrl undefined when the fetch throws", async () => { + it("applies defaults when the fetch throws", async () => { const fetchFn = vi.fn().mockRejectedValue(new Error("network down")); const { result } = renderHook(() => - useSandboxUrl({ baseUrl: "http://test.local", fetchFn }), + useInitialConfig({ baseUrl: "http://test.local", fetchFn }), ); await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.version).toBeUndefined(); expect(result.current.sandboxUrl).toBeUndefined(); + expect(result.current.writable).toBe(true); }); it("falls back to globalThis.fetch when no fetchFn is provided", async () => { - const globalFetch = vi - .fn() - .mockResolvedValue(jsonResponse({ sandboxUrl: "http://global/sb" })); + const globalFetch = vi.fn().mockResolvedValue( + jsonResponse({ + version: "3.1.4", + sandboxUrl: "http://global/sb", + writable: false, + }), + ); const original = globalThis.fetch; globalThis.fetch = globalFetch as unknown as typeof fetch; try { const { result } = renderHook(() => - useSandboxUrl({ baseUrl: "http://test.local" }), + useInitialConfig({ baseUrl: "http://test.local" }), ); await waitFor(() => expect(result.current.loading).toBe(false)); expect(globalFetch).toHaveBeenCalledWith( "http://test.local/api/config", expect.objectContaining({ method: "GET" }), ); + expect(result.current.version).toBe("3.1.4"); expect(result.current.sandboxUrl).toBe("http://global/sb"); + expect(result.current.writable).toBe(false); } finally { globalThis.fetch = original; } @@ -145,19 +200,25 @@ describe("useSandboxUrl", () => { ); const { result, unmount } = renderHook(() => - useSandboxUrl({ baseUrl: "http://test.local", fetchFn }), + useInitialConfig({ baseUrl: "http://test.local", fetchFn }), ); expect(result.current.loading).toBe(true); unmount(); // Resolve after unmount — the post-fetch isCancelled() guard returns early. - resolveFetch?.(jsonResponse({ sandboxUrl: "http://late/sb" })); + resolveFetch?.( + jsonResponse({ version: "2.0.0", sandboxUrl: "http://late/sb" }), + ); // Let the microtask queue drain so the continuation runs. await Promise.resolve(); await Promise.resolve(); - // No assertion error / React warning means the guards held; the last - // observed value stayed at its initial undefined. + // This test exists to exercise the post-unmount `isCancelled()` guard + // branches; it can't detect their removal (React 18 dropped the + // setState-after-unmount warning, and `result.current` is frozen at the last + // render), so it only asserts the fields stayed at their initial values. + expect(result.current.version).toBeUndefined(); expect(result.current.sandboxUrl).toBeUndefined(); + expect(result.current.writable).toBe(true); }); it("drops a response whose json resolves after unmount", async () => { @@ -175,16 +236,17 @@ describe("useSandboxUrl", () => { const fetchFn = vi.fn().mockResolvedValue(res); const { result, unmount } = renderHook(() => - useSandboxUrl({ baseUrl: "http://test.local", fetchFn }), + useInitialConfig({ baseUrl: "http://test.local", fetchFn }), ); // Let the fetch resolve so we're parked awaiting json(). await waitFor(() => expect(fetchFn).toHaveBeenCalled()); await Promise.resolve(); unmount(); - resolveJson?.({ sandboxUrl: "http://late/sb" }); + resolveJson?.({ version: "2.0.0", writable: false }); await Promise.resolve(); await Promise.resolve(); - expect(result.current.sandboxUrl).toBeUndefined(); + expect(result.current.version).toBeUndefined(); + expect(result.current.writable).toBe(true); }); }); diff --git a/clients/web/src/test/core/react/useInspectorVersion.test.tsx b/clients/web/src/test/core/react/useInspectorVersion.test.tsx deleted file mode 100644 index 4d1f84927..000000000 --- a/clients/web/src/test/core/react/useInspectorVersion.test.tsx +++ /dev/null @@ -1,181 +0,0 @@ -/** - * Tests for useInspectorVersion — runs in happy-dom under the unit project. Uses - * a controlled fake `fetch` so each branch (present / absent / non-string / HTTP - * error / network throw / post-unmount guards) and the auth header are asserted - * directly. Mirrors useSandboxUrl's test (same `/api/config` fetch shape). - */ - -import { describe, it, expect, vi } from "vitest"; -import { renderHook, waitFor } from "@testing-library/react"; -import { useInspectorVersion } from "@inspector/core/react/useInspectorVersion"; - -function jsonResponse(body: unknown, ok = true): Response { - return { - ok, - status: ok ? 200 : 401, - json: async () => body, - } as unknown as Response; -} - -describe("useInspectorVersion", () => { - it("starts loading, then resolves the version from the config payload", async () => { - const fetchFn = vi - .fn() - .mockResolvedValue(jsonResponse({ version: "2.0.0" })); - - const { result } = renderHook(() => - useInspectorVersion({ baseUrl: "http://test.local", fetchFn }), - ); - - expect(result.current.loading).toBe(true); - expect(result.current.version).toBeUndefined(); - - await waitFor(() => expect(result.current.loading).toBe(false)); - expect(result.current.version).toBe("2.0.0"); - }); - - it("sends the bearer auth header when a token is provided", async () => { - const fetchFn = vi.fn().mockResolvedValue(jsonResponse({})); - - renderHook(() => - useInspectorVersion({ - baseUrl: "http://test.local/", - authToken: "secret-token", - fetchFn, - }), - ); - - await waitFor(() => expect(fetchFn).toHaveBeenCalled()); - const [url, init] = fetchFn.mock.calls[0]; - // Trailing slash on baseUrl is normalized away. - expect(url).toBe("http://test.local/api/config"); - expect(init.method).toBe("GET"); - expect(init.headers["x-mcp-remote-auth"]).toBe("Bearer secret-token"); - }); - - it("omits the auth header when no token is provided", async () => { - const fetchFn = vi.fn().mockResolvedValue(jsonResponse({})); - - renderHook(() => - useInspectorVersion({ baseUrl: "http://test.local", fetchFn }), - ); - - await waitFor(() => expect(fetchFn).toHaveBeenCalled()); - const [, init] = fetchFn.mock.calls[0]; - expect(init.headers["x-mcp-remote-auth"]).toBeUndefined(); - }); - - it("leaves version undefined when the payload omits it", async () => { - const fetchFn = vi - .fn() - .mockResolvedValue(jsonResponse({ defaultEnvironment: {} })); - - const { result } = renderHook(() => - useInspectorVersion({ baseUrl: "http://test.local", fetchFn }), - ); - - await waitFor(() => expect(result.current.loading).toBe(false)); - expect(result.current.version).toBeUndefined(); - }); - - it("leaves version undefined when the field is not a usable string", async () => { - const fetchFn = vi.fn().mockResolvedValue(jsonResponse({ version: "" })); - - const { result } = renderHook(() => - useInspectorVersion({ baseUrl: "http://test.local", fetchFn }), - ); - - await waitFor(() => expect(result.current.loading).toBe(false)); - expect(result.current.version).toBeUndefined(); - }); - - it("leaves version undefined on a non-ok response", async () => { - const fetchFn = vi - .fn() - .mockResolvedValue(jsonResponse({ version: "9.9.9" }, false)); - - const { result } = renderHook(() => - useInspectorVersion({ baseUrl: "http://test.local", fetchFn }), - ); - - await waitFor(() => expect(result.current.loading).toBe(false)); - expect(result.current.version).toBeUndefined(); - }); - - it("leaves version undefined when the fetch throws", async () => { - const fetchFn = vi.fn().mockRejectedValue(new Error("network down")); - - const { result } = renderHook(() => - useInspectorVersion({ baseUrl: "http://test.local", fetchFn }), - ); - - await waitFor(() => expect(result.current.loading).toBe(false)); - expect(result.current.version).toBeUndefined(); - }); - - it("falls back to globalThis.fetch when no fetchFn is provided", async () => { - const globalFetch = vi - .fn() - .mockResolvedValue(jsonResponse({ version: "3.1.4" })); - const original = globalThis.fetch; - globalThis.fetch = globalFetch as unknown as typeof fetch; - try { - const { result } = renderHook(() => - useInspectorVersion({ baseUrl: "http://test.local" }), - ); - await waitFor(() => expect(result.current.loading).toBe(false)); - expect(globalFetch).toHaveBeenCalledWith( - "http://test.local/api/config", - expect.objectContaining({ method: "GET" }), - ); - expect(result.current.version).toBe("3.1.4"); - } finally { - globalThis.fetch = original; - } - }); - - it("drops a response that resolves after unmount (no state update)", async () => { - let resolveFetch: ((r: Response) => void) | undefined; - const fetchFn = vi.fn().mockReturnValue( - new Promise((r) => { - resolveFetch = r; - }), - ); - - const { result, unmount } = renderHook(() => - useInspectorVersion({ baseUrl: "http://test.local", fetchFn }), - ); - expect(result.current.loading).toBe(true); - - unmount(); - resolveFetch?.(jsonResponse({ version: "2.0.0" })); - await Promise.resolve(); - await Promise.resolve(); - expect(result.current.version).toBeUndefined(); - }); - - it("drops a response whose json resolves after unmount", async () => { - let resolveJson: ((v: unknown) => void) | undefined; - const res = { - ok: true, - status: 200, - json: () => - new Promise((r) => { - resolveJson = r; - }), - } as unknown as Response; - const fetchFn = vi.fn().mockResolvedValue(res); - - const { result, unmount } = renderHook(() => - useInspectorVersion({ baseUrl: "http://test.local", fetchFn }), - ); - await waitFor(() => expect(fetchFn).toHaveBeenCalled()); - await Promise.resolve(); - - unmount(); - resolveJson?.({ version: "2.0.0" }); - await Promise.resolve(); - await Promise.resolve(); - expect(result.current.version).toBeUndefined(); - }); -}); diff --git a/clients/web/src/test/core/react/useServerListWritable.test.tsx b/clients/web/src/test/core/react/useServerListWritable.test.tsx deleted file mode 100644 index 68eb5aebd..000000000 --- a/clients/web/src/test/core/react/useServerListWritable.test.tsx +++ /dev/null @@ -1,177 +0,0 @@ -/** - * Tests for useServerListWritable — runs in happy-dom under the unit project. - * Uses a controlled fake `fetch` so each branch (writable / read-only / absent - * field / HTTP error / network throw) and the auth header are asserted directly. - */ - -import { describe, it, expect, vi } from "vitest"; -import { renderHook, waitFor } from "@testing-library/react"; -import { useServerListWritable } from "@inspector/core/react/useServerListWritable"; - -function jsonResponse(body: unknown, ok = true): Response { - return { - ok, - status: ok ? 200 : 401, - json: async () => body, - } as unknown as Response; -} - -describe("useServerListWritable", () => { - it("defaults to writable while loading, then keeps true for a writable session", async () => { - const fetchFn = vi.fn().mockResolvedValue(jsonResponse({ writable: true })); - - const { result } = renderHook(() => - useServerListWritable({ baseUrl: "http://test.local", fetchFn }), - ); - - expect(result.current.loading).toBe(true); - expect(result.current.writable).toBe(true); - - await waitFor(() => expect(result.current.loading).toBe(false)); - expect(result.current.writable).toBe(true); - }); - - it("resolves writable:false for a read-only session", async () => { - const fetchFn = vi - .fn() - .mockResolvedValue(jsonResponse({ writable: false })); - - const { result } = renderHook(() => - useServerListWritable({ baseUrl: "http://test.local", fetchFn }), - ); - - await waitFor(() => expect(result.current.loading).toBe(false)); - expect(result.current.writable).toBe(false); - }); - - it("stays writable when the field is absent (legacy backend)", async () => { - const fetchFn = vi.fn().mockResolvedValue(jsonResponse({})); - - const { result } = renderHook(() => - useServerListWritable({ baseUrl: "http://test.local", fetchFn }), - ); - - await waitFor(() => expect(result.current.loading).toBe(false)); - expect(result.current.writable).toBe(true); - }); - - it("sends the bearer auth header when a token is provided", async () => { - const fetchFn = vi - .fn() - .mockResolvedValue(jsonResponse({ writable: false })); - - renderHook(() => - useServerListWritable({ - baseUrl: "http://test.local/", - authToken: "secret-token", - fetchFn, - }), - ); - - await waitFor(() => expect(fetchFn).toHaveBeenCalled()); - const [url, init] = fetchFn.mock.calls[0] as [string, RequestInit]; - // Trailing slash on baseUrl is trimmed. - expect(url).toBe("http://test.local/api/config"); - expect((init.headers as Record)["x-mcp-remote-auth"]).toBe( - "Bearer secret-token", - ); - }); - - it("stays writable on an HTTP error", async () => { - const fetchFn = vi - .fn() - .mockResolvedValue(jsonResponse({ writable: false }, false)); - - const { result } = renderHook(() => - useServerListWritable({ baseUrl: "http://test.local", fetchFn }), - ); - - // An !ok response is ignored (the body is never read), so the writable - // default survives even though loading still settles via `finally`. - await waitFor(() => expect(result.current.loading).toBe(false)); - expect(result.current.writable).toBe(true); - }); - - it("stays writable when fetch throws", async () => { - const fetchFn = vi.fn().mockRejectedValue(new Error("network down")); - - const { result } = renderHook(() => - useServerListWritable({ baseUrl: "http://test.local", fetchFn }), - ); - - await waitFor(() => expect(result.current.loading).toBe(false)); - expect(result.current.writable).toBe(true); - }); - - it("falls back to globalThis.fetch when no fetchFn is provided", async () => { - const globalFetch = vi - .fn() - .mockResolvedValue(jsonResponse({ writable: false })); - const original = globalThis.fetch; - globalThis.fetch = globalFetch as unknown as typeof fetch; - try { - const { result } = renderHook(() => - useServerListWritable({ baseUrl: "http://test.local" }), - ); - await waitFor(() => expect(result.current.loading).toBe(false)); - expect(globalFetch).toHaveBeenCalledWith( - "http://test.local/api/config", - expect.objectContaining({ method: "GET" }), - ); - expect(result.current.writable).toBe(false); - } finally { - globalThis.fetch = original; - } - }); - - it("drops a response that resolves after unmount (no state update)", async () => { - // Gate the fetch so it is still in flight at unmount; the isCancelled() - // guards (after fetch and in finally) short-circuit so no setState runs on - // the dead component and `loading` is never flipped. - let resolveFetch: ((r: Response) => void) | undefined; - const fetchFn = vi.fn().mockReturnValue( - new Promise((r) => { - resolveFetch = r; - }), - ); - - const { result, unmount } = renderHook(() => - useServerListWritable({ baseUrl: "http://test.local", fetchFn }), - ); - expect(result.current.loading).toBe(true); - - unmount(); - resolveFetch?.(jsonResponse({ writable: false })); - await Promise.resolve(); - await Promise.resolve(); - // Guards held: the post-unmount value stayed at the writable default. - expect(result.current.writable).toBe(true); - }); - - it("drops a response whose json resolves after unmount", async () => { - // Fetch resolves before unmount but json() resolves after, exercising the - // second isCancelled() guard (post-json). - let resolveJson: ((v: unknown) => void) | undefined; - const res = { - ok: true, - status: 200, - json: () => - new Promise((r) => { - resolveJson = r; - }), - } as unknown as Response; - const fetchFn = vi.fn().mockResolvedValue(res); - - const { result, unmount } = renderHook(() => - useServerListWritable({ baseUrl: "http://test.local", fetchFn }), - ); - await waitFor(() => expect(fetchFn).toHaveBeenCalled()); - await Promise.resolve(); - - unmount(); - resolveJson?.({ writable: false }); - await Promise.resolve(); - await Promise.resolve(); - expect(result.current.writable).toBe(true); - }); -}); diff --git a/clients/web/src/test/setup.ts b/clients/web/src/test/setup.ts index 871993ff1..25a226a4e 100644 --- a/clients/web/src/test/setup.ts +++ b/clients/web/src/test/setup.ts @@ -34,7 +34,7 @@ Object.defineProperty(window, "localStorage", { }); // Benign default `fetch`. Several components hit the backend on mount — e.g. -// the app reads `GET /api/config` via `useSandboxUrl` / `useServerListWritable`. +// the app reads `GET /api/config` via `useInitialConfig`. // Under happy-dom (no server) those real requests 404 and log alarming // `GET .../api/config 404 (Not Found)` lines that make a green run look broken. // Returning an empty 200 keeps such *incidental* calls quiet. Tests that care diff --git a/core/react/useInitialConfig.ts b/core/react/useInitialConfig.ts new file mode 100644 index 000000000..884b23705 --- /dev/null +++ b/core/react/useInitialConfig.ts @@ -0,0 +1,122 @@ +/** + * React hook over the `GET /api/config` endpoint — the single fetch of the + * static `InitialConfigPayload` the dev/prod web backends serve alongside the + * SPA (see `core/mcp/remote/node/server.ts`). It exposes the three fields the + * web app reads off that payload: + * + * - `sandboxUrl` — the MCP Apps sandbox proxy URL. The backend mounts + * `sandbox_proxy.html` on a separate controller port and advertises the URL + * here; the Apps screen embeds it as the trusted outer iframe. `undefined` + * until the fetch resolves, and whenever the backend omits it (legacy backend, + * or a build without the sandbox controller) — callers treat that as "Apps + * unavailable" rather than a blank iframe. + * - `writable` — whether the session's server list is a writable catalog or a + * read-only session (`--config` / ad-hoc `--server-url`). Defaults to `true` + * until the fetch resolves and whenever the field is absent (a legacy backend + * predating the flag), so the default catalog keeps full CRUD. + * - `version` — the Inspector version the backend reads from the root + * `package.json` (the browser can't read it off disk). `undefined` until the + * fetch resolves, and whenever the backend omits it (legacy backend) — the UI + * renders nothing then. + * + * This consolidates the three former single-field hooks (`useSandboxUrl`, + * `useServerListWritable`, `useInspectorVersion`), each of which fetched the + * same static payload separately, into one request (#1643). + * + * Fetches on mount, and re-fetches if `baseUrl` or `authToken` changes (rare — + * effectively a full reload; the GET is idempotent). A response that resolves + * after unmount or a re-fetch is dropped rather than overwriting current state. + */ + +import { useCallback, useEffect, useState } from "react"; + +export interface UseInitialConfigOptions { + /** Base URL of the remote server (typically `window.location.origin`). */ + baseUrl: string; + /** Optional auth token for the `x-mcp-remote-auth` header. */ + authToken?: string; + /** Fetch function to use (default: globalThis.fetch). Useful in tests. */ + fetchFn?: typeof fetch; +} + +export interface UseInitialConfigResult { + /** The Inspector version, or undefined when unavailable / not yet loaded. */ + version: string | undefined; + /** The sandbox proxy URL, or undefined when unavailable / not yet loaded. */ + sandboxUrl: string | undefined; + /** Whether the server list is writable (catalog) or read-only (session). */ + writable: boolean; + /** True while the initial fetch is in flight. */ + loading: boolean; +} + +/** Minimal shape we read from the `/api/config` payload. */ +interface ConfigPayload { + version?: unknown; + sandboxUrl?: unknown; + writable?: unknown; +} + +/** Coerce a payload field to a usable non-empty string, else undefined. */ +function usableString(value: unknown): string | undefined { + return typeof value === "string" && value ? value : undefined; +} + +export function useInitialConfig( + opts: UseInitialConfigOptions, +): UseInitialConfigResult { + const { baseUrl, authToken, fetchFn } = opts; + const doFetch = fetchFn ?? globalThis.fetch; + const base = baseUrl.replace(/\/$/, ""); + + const [version, setVersion] = useState(undefined); + const [sandboxUrl, setSandboxUrl] = useState(undefined); + // Default writable so the common (catalog) case shows CRUD immediately and a + // legacy backend that omits the field keeps working. + const [writable, setWritable] = useState(true); + const [loading, setLoading] = useState(true); + + // `isCancelled` lets the effect drop a response that resolves after unmount or + // after a re-run (baseUrl/authToken change), so a stale payload can't overwrite + // current state. React 18 no longer warns on setState-after-unmount — it's a + // silent no-op — so the guard is about correctness, not the warning. + const load = useCallback( + async (isCancelled: () => boolean): Promise => { + const headers: Record = {}; + if (authToken) headers["x-mcp-remote-auth"] = `Bearer ${authToken}`; + try { + const res = await doFetch(`${base}/api/config`, { + method: "GET", + headers, + }); + if (isCancelled() || !res.ok) return; + const body = (await res.json()) as ConfigPayload; + if (isCancelled()) return; + // Tolerate missing/non-usable fields — a legacy backend that omits any + // of them leaves that value at its "unavailable" default rather than + // showing a bogus value. + setVersion(usableString(body.version)); + setSandboxUrl(usableString(body.sandboxUrl)); + // Only an explicit `false` makes the list read-only; a missing field + // (legacy backend) stays writable. + setWritable(body.writable !== false); + } catch { + // Network error / aborted fetch: leave every field at its default + // (version/sandboxUrl undefined, writable true). + } finally { + if (!isCancelled()) setLoading(false); + } + }, + [base, authToken, doFetch], + ); + + useEffect(() => { + let cancelled = false; + void load(() => cancelled); + return () => { + cancelled = true; + }; + }, [load]); + + return { version, sandboxUrl, writable, loading }; +} diff --git a/core/react/useInspectorVersion.ts b/core/react/useInspectorVersion.ts deleted file mode 100644 index 81300bfc6..000000000 --- a/core/react/useInspectorVersion.ts +++ /dev/null @@ -1,83 +0,0 @@ -/** - * React hook over the `GET /api/config` endpoint that recovers the Inspector - * `version` the backend reads from the root `package.json`. The browser can't - * read the filesystem the way the CLI/TUI do, so the version is delivered on the - * config payload (see `webServerConfigToInitialPayload`) and surfaced here. - * - * Mirrors {@link useSandboxUrl} / {@link useServerListWritable} (same - * `/api/config` source, same fetch shape). `version` is `undefined` until the - * fetch resolves, and stays `undefined` if the backend omits it (a legacy - * backend that predates the field) — the UI simply renders nothing then. - */ - -import { useCallback, useEffect, useState } from "react"; - -export interface UseInspectorVersionOptions { - /** Base URL of the remote server (typically `window.location.origin`). */ - baseUrl: string; - /** Optional auth token for the `x-mcp-remote-auth` header. */ - authToken?: string; - /** Fetch function to use (default: globalThis.fetch). Useful in tests. */ - fetchFn?: typeof fetch; -} - -export interface UseInspectorVersionResult { - /** The Inspector version, or undefined when unavailable / not yet loaded. */ - version: string | undefined; - /** True while the initial fetch is in flight. */ - loading: boolean; -} - -/** Minimal shape we read from the `/api/config` payload. */ -interface ConfigPayload { - version?: unknown; -} - -export function useInspectorVersion( - opts: UseInspectorVersionOptions, -): UseInspectorVersionResult { - const { baseUrl, authToken, fetchFn } = opts; - const doFetch = fetchFn ?? globalThis.fetch; - const base = baseUrl.replace(/\/$/, ""); - - const [version, setVersion] = useState(undefined); - const [loading, setLoading] = useState(true); - - const load = useCallback( - async (isCancelled: () => boolean): Promise => { - const headers: Record = {}; - if (authToken) headers["x-mcp-remote-auth"] = `Bearer ${authToken}`; - try { - const res = await doFetch(`${base}/api/config`, { - method: "GET", - headers, - }); - if (isCancelled() || !res.ok) return; - const body = (await res.json()) as ConfigPayload; - if (isCancelled()) return; - // Tolerate a missing/non-string field — a legacy backend that omits it - // leaves the version hidden rather than showing a bogus value. - setVersion( - typeof body.version === "string" && body.version - ? body.version - : undefined, - ); - } catch { - // Network error / aborted fetch: leave version undefined (hidden). - } finally { - if (!isCancelled()) setLoading(false); - } - }, - [base, authToken, doFetch], - ); - - useEffect(() => { - let cancelled = false; - void load(() => cancelled); - return () => { - cancelled = true; - }; - }, [load]); - - return { version, loading }; -} diff --git a/core/react/useSandboxUrl.ts b/core/react/useSandboxUrl.ts deleted file mode 100644 index 95d06807b..000000000 --- a/core/react/useSandboxUrl.ts +++ /dev/null @@ -1,88 +0,0 @@ -/** - * React hook over the `GET /api/config` endpoint, used to recover the MCP Apps - * sandbox URL the backend serves alongside the SPA. The dev/prod web backends - * mount `sandbox_proxy.html` on a separate controller port and advertise the - * resulting URL as `sandboxUrl` on the config payload (see - * `clients/web/server/vite-hono-plugin.ts`). The Apps screen embeds that URL as - * the trusted outer iframe of the double-iframe sandbox; without it MCP Apps - * cannot run, so the screen renders an unavailable state instead. - * - * Fetches on mount, and re-fetches if `baseUrl` or `authToken` changes (rare — - * effectively a full reload; the GET is idempotent). `sandboxUrl` is `undefined` - * until the fetch resolves and stays `undefined` if the backend omits it (legacy - * backend, or a build without the sandbox controller) — callers should treat - * that as "Apps unavailable" rather than falling back to a blank iframe. - */ - -import { useCallback, useEffect, useState } from "react"; - -export interface UseSandboxUrlOptions { - /** Base URL of the remote server (typically `window.location.origin`). */ - baseUrl: string; - /** Optional auth token for the `x-mcp-remote-auth` header. */ - authToken?: string; - /** Fetch function to use (default: globalThis.fetch). Useful in tests. */ - fetchFn?: typeof fetch; -} - -export interface UseSandboxUrlResult { - /** The sandbox proxy URL, or undefined when unavailable / not yet loaded. */ - sandboxUrl: string | undefined; - /** True while the initial fetch is in flight. */ - loading: boolean; -} - -/** Minimal shape we read from the `/api/config` payload. */ -interface ConfigPayload { - sandboxUrl?: unknown; -} - -export function useSandboxUrl(opts: UseSandboxUrlOptions): UseSandboxUrlResult { - const { baseUrl, authToken, fetchFn } = opts; - const doFetch = fetchFn ?? globalThis.fetch; - const base = baseUrl.replace(/\/$/, ""); - - const [sandboxUrl, setSandboxUrl] = useState(undefined); - const [loading, setLoading] = useState(true); - - // `isCancelled` lets the effect drop a response that resolves after unmount, - // avoiding a setState on a dead component (React 18 warns, doesn't throw). - const load = useCallback( - async (isCancelled: () => boolean): Promise => { - const headers: Record = {}; - if (authToken) headers["x-mcp-remote-auth"] = `Bearer ${authToken}`; - try { - const res = await doFetch(`${base}/api/config`, { - method: "GET", - headers, - }); - if (isCancelled() || !res.ok) return; - const body = (await res.json()) as ConfigPayload; - if (isCancelled()) return; - // Tolerate a missing/non-string field — anything but a usable URL means - // "unavailable", which the Apps screen surfaces as its own empty state. - setSandboxUrl( - typeof body.sandboxUrl === "string" && body.sandboxUrl - ? body.sandboxUrl - : undefined, - ); - } catch { - // Network error / aborted fetch: leave sandboxUrl undefined. The Apps - // screen degrades to its unavailable state rather than a blank iframe. - } finally { - if (!isCancelled()) setLoading(false); - } - }, - [base, authToken, doFetch], - ); - - useEffect(() => { - let cancelled = false; - void load(() => cancelled); - return () => { - cancelled = true; - }; - }, [load]); - - return { sandboxUrl, loading }; -} diff --git a/core/react/useServerListWritable.ts b/core/react/useServerListWritable.ts deleted file mode 100644 index 949430b21..000000000 --- a/core/react/useServerListWritable.ts +++ /dev/null @@ -1,82 +0,0 @@ -/** - * React hook over the `GET /api/config` endpoint that recovers the session's - * `writable` flag. The backend sets `writable: false` when the web UI was - * launched against a read-only server list — a `--config` session file or an - * ad-hoc `--server-url` / command target — so the UI can hide catalog CRUD - * (add / edit / remove / reorder / settings-save) it would only get a 403 for. - * - * Mirrors {@link useSandboxUrl} (same `/api/config` source, same fetch shape). - * Defaults to `true` until the fetch resolves and whenever the field is absent - * (a legacy backend that predates the flag), so the default catalog and - * `--catalog` keep full CRUD without any UI change. - */ - -import { useCallback, useEffect, useState } from "react"; - -export interface UseServerListWritableOptions { - /** Base URL of the remote server (typically `window.location.origin`). */ - baseUrl: string; - /** Optional auth token for the `x-mcp-remote-auth` header. */ - authToken?: string; - /** Fetch function to use (default: globalThis.fetch). Useful in tests. */ - fetchFn?: typeof fetch; -} - -export interface UseServerListWritableResult { - /** Whether the server list is writable (catalog) or read-only (session). */ - writable: boolean; - /** True while the initial fetch is in flight. */ - loading: boolean; -} - -/** Minimal shape we read from the `/api/config` payload. */ -interface ConfigPayload { - writable?: unknown; -} - -export function useServerListWritable( - opts: UseServerListWritableOptions, -): UseServerListWritableResult { - const { baseUrl, authToken, fetchFn } = opts; - const doFetch = fetchFn ?? globalThis.fetch; - const base = baseUrl.replace(/\/$/, ""); - - // Default writable so the common (catalog) case shows CRUD immediately and a - // legacy backend that omits the field keeps working. - const [writable, setWritable] = useState(true); - const [loading, setLoading] = useState(true); - - const load = useCallback( - async (isCancelled: () => boolean): Promise => { - const headers: Record = {}; - if (authToken) headers["x-mcp-remote-auth"] = `Bearer ${authToken}`; - try { - const res = await doFetch(`${base}/api/config`, { - method: "GET", - headers, - }); - if (isCancelled() || !res.ok) return; - const body = (await res.json()) as ConfigPayload; - if (isCancelled()) return; - // Only an explicit `false` makes the list read-only; a missing field - // (legacy backend) stays writable. - setWritable(body.writable !== false); - } catch { - // Network error / aborted fetch: leave writable at its default (true). - } finally { - if (!isCancelled()) setLoading(false); - } - }, - [base, authToken, doFetch], - ); - - useEffect(() => { - let cancelled = false; - void load(() => cancelled); - return () => { - cancelled = true; - }; - }, [load]); - - return { writable, loading }; -}