diff --git a/AGENTS.md b/AGENTS.md index 4f7bb68b6..99cade549 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -59,6 +59,8 @@ v2 is **not** an npm workspace — each client under `clients/*` keeps its own ` After installing, `npm run build` builds all clients. The launcher scripts (`npm run web` / `web:dev`) run the built launcher, so build first; for day-to-day web iteration use `cd clients/web && npm run dev`. +For automated MCP App review (CLI-first probe + one-shot web render), see [docs/mcp-app-review.md](docs/mcp-app-review.md). + ## Repository & Project Board - **Repo**: https://github.com/modelcontextprotocol/inspector.git diff --git a/clients/cli/__tests__/app-info.test.ts b/clients/cli/__tests__/app-info.test.ts new file mode 100644 index 000000000..c1ce65456 --- /dev/null +++ b/clients/cli/__tests__/app-info.test.ts @@ -0,0 +1,154 @@ +import { describe, it, expect } from "vitest"; +import { runCli } from "./helpers/cli-runner.js"; +import { getTestMcpServerCommand } from "@modelcontextprotocol/inspector-test-server"; + +/** + * The default test server has no MCP App tools, so this exercises the + * negative path: `--app-info` prints `{"hasApp":false,...}` on stdout and + * exits 2. The positive path is unit-tested at the `extractAppInfo` level + * (`clients/web/src/test/core/apps.test.ts`) and end-to-end via the + * `mcpAppDemo` fixture. + */ +describe("--app-info", () => { + it("rejects --app-info on a method other than tools/call or tools/list", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "resources/list", + "--app-info", + ]); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain( + "--app-info requires --method tools/call (with --tool-name) or --method tools/list", + ); + }); + + it("emits NDJSON (one app-info line per tool) on --method tools/list --app-info", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/list", + "--app-info", + ]); + expect(result.exitCode).toBe(0); + const lines = result.stdout.trim().split("\n"); + expect(lines.length).toBeGreaterThan(1); + const infos = lines.map( + (l) => JSON.parse(l) as { hasApp: boolean; toolName: string }, + ); + // Every line is a valid app-info object with a toolName. + expect(infos.every((i) => typeof i.toolName === "string")).toBe(true); + // The fixture's `mcp_app_demo` is the (only) App tool. + const demo = infos.find((i) => i.toolName === "mcp_app_demo"); + expect(demo?.hasApp).toBe(true); + expect(infos.filter((i) => i.hasApp).length).toBe(1); + }); + + it("emits {hasApp:false} as one JSON line and exits 2 for a non-App tool", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/call", + "--tool-name", + "echo", + "--app-info", + ]); + expect(result.exitCode).toBe(2); + const line = result.stdout.trim().split("\n")[0]; + const info = JSON.parse(line) as { hasApp: boolean; toolName: string }; + expect(info).toEqual({ hasApp: false, toolName: "echo" }); + expect(result.stderr).toContain("has no MCP App UI resource"); + }); + + it("exits 5 (TOOL_ERROR, code:tool_not_found) when the tool is not found — distinct from the no-app exit-2", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/call", + "--tool-name", + "no-such-tool", + "--app-info", + ]); + expect(result.exitCode).toBe(5); + expect(result.stdout).toBe(""); + const envelope = JSON.parse(result.stderr.trim()) as { + error: { code: string }; + }; + expect(envelope.error.code).toBe("tool_not_found"); + }); + + it("emits the resource-side csp/permissions and exits 0 for an App tool", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/call", + "--tool-name", + "mcp_app_demo", + "--app-info", + ]); + expect(result.exitCode).toBe(0); + const info = JSON.parse(result.stdout.trim().split("\n")[0]) as { + hasApp: boolean; + toolName: string; + resourceUri: string; + csp: unknown; + permissions: unknown; + prefersBorder: boolean; + resourceMimeType: string; + }; + expect(info.hasApp).toBe(true); + expect(info.toolName).toBe("mcp_app_demo"); + expect(info.resourceUri).toBe("ui://demo/widget.html"); + expect(info.csp).toEqual({ connectDomains: [], resourceDomains: [] }); + expect(info.permissions).toEqual({ clipboard: false }); + expect(info.prefersBorder).toBe(true); + expect(info.resourceMimeType).toBe("text/html"); + }); + + it("does not collect app-info on a plain text-mode tools/call (use --format json or --app-info)", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/call", + "--tool-name", + "mcp_app_demo", + "--tool-arg", + "title=hello", + ]); + expect(result.exitCode).toBe(0); + expect(result.stdout).not.toContain("--- MCP App Info ---"); + expect(result.stdout).not.toContain("hasApp"); + // stdout is now a single JSON value (the tool result) so `| jq` works. + expect(() => JSON.parse(result.stdout)).not.toThrow(); + }); + + it("does not invoke the tool when --app-info is set", async () => { + // get_sum requires numeric a/b args; without --app-info this would fail + // with a tool error. With --app-info the tool is never called, so the + // only error is the no-app exit-2. + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--method", + "tools/call", + "--tool-name", + "get_sum", + "--app-info", + ]); + expect(result.exitCode).toBe(2); + expect(result.output).not.toContain("isError"); + }); +}); diff --git a/clients/cli/__tests__/error-handler.test.ts b/clients/cli/__tests__/error-handler.test.ts index 79d0eab8d..d9682a741 100644 --- a/clients/cli/__tests__/error-handler.test.ts +++ b/clients/cli/__tests__/error-handler.test.ts @@ -1,5 +1,11 @@ import { describe, it, expect, vi, afterEach } from "vitest"; -import { handleError } from "../src/error-handler.js"; +import { + CliExitCodeError, + EXIT_CODES, + classifyError, + formatErrorOutput, + handleError, +} from "../src/error-handler.js"; /** * `handleError` is the binary's last-resort error sink (wired up in @@ -13,33 +19,142 @@ describe("handleError", () => { vi.restoreAllMocks(); }); - it("logs an Error's message and exits with code 1", () => { - const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + it("emits a JSON error envelope on stderr and exits with the classified code", () => { + const writeSpy = vi + .spyOn(process.stderr, "write") + .mockImplementation((() => true) as never); const exitSpy = vi .spyOn(process, "exit") .mockImplementation((() => undefined) as never); handleError(new Error("boom")); - expect(errorSpy).toHaveBeenCalledWith("boom"); - expect(exitSpy).toHaveBeenCalledWith(1); + expect(exitSpy).toHaveBeenCalledWith(EXIT_CODES.USAGE); + const written = writeSpy.mock.calls[0]![0] as string; + const parsed = JSON.parse(written) as { + error: { code: string; message: string }; + }; + expect(parsed.error.code).toBe("error"); + expect(parsed.error.message).toBe("boom"); }); - it("logs a string error verbatim", () => { - const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - vi.spyOn(process, "exit").mockImplementation((() => undefined) as never); + it("uses a CliExitCodeError's exitCode and envelope code", () => { + vi.spyOn(process.stderr, "write").mockImplementation((() => true) as never); + const exitSpy = vi + .spyOn(process, "exit") + .mockImplementation((() => undefined) as never); - handleError("plain failure"); + handleError( + new CliExitCodeError(EXIT_CODES.NO_APP, "no app", { code: "no_app" }), + ); - expect(errorSpy).toHaveBeenCalledWith("plain failure"); + expect(exitSpy).toHaveBeenCalledWith(EXIT_CODES.NO_APP); }); +}); - it("falls back to 'Unknown error' for non-Error, non-string values", () => { - const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - vi.spyOn(process, "exit").mockImplementation((() => undefined) as never); +describe("classifyError", () => { + it("classifies a 401 status as AUTH_REQUIRED", () => { + const err = Object.assign(new Error("Unauthorized"), { status: 401 }); + const { exitCode, envelope } = classifyError(err, { + url: "https://x.example/mcp", + }); + expect(exitCode).toBe(EXIT_CODES.AUTH_REQUIRED); + expect(envelope.code).toBe("auth_required"); + expect(envelope.status).toBe(401); + expect(envelope.url).toBe("https://x.example/mcp"); + }); + + it("classifies a WWW-Authenticate message as AUTH_REQUIRED without a status", () => { + const { exitCode } = classifyError( + new Error("Dynamic client registration failed: WWW-Authenticate Bearer"), + ); + expect(exitCode).toBe(EXIT_CODES.AUTH_REQUIRED); + }); + + it("classifies ENOTFOUND / fetch failed as UNREACHABLE", () => { + const err = new Error("fetch failed"); + (err as { cause?: unknown }).cause = new Error( + "getaddrinfo ENOTFOUND no.such.host", + ); + const { exitCode, envelope } = classifyError(err); + expect(exitCode).toBe(EXIT_CODES.UNREACHABLE); + expect(envelope.cause).toContain("ENOTFOUND"); + }); + + it("classifies a connection-refused cause as UNREACHABLE", () => { + const err = new Error("connect ECONNREFUSED 127.0.0.1:1"); + expect(classifyError(err).exitCode).toBe(EXIT_CODES.UNREACHABLE); + }); + + it("falls back to USAGE for an unrecognized Error", () => { + expect(classifyError(new Error("something else")).exitCode).toBe( + EXIT_CODES.USAGE, + ); + }); - handleError({ unexpected: true }); + it("handles a string error", () => { + const { exitCode, envelope } = classifyError("plain failure"); + expect(exitCode).toBe(EXIT_CODES.USAGE); + expect(envelope.message).toBe("plain failure"); + }); + + it("handles a non-Error, non-string value", () => { + const { envelope } = classifyError({ unexpected: true }); + expect(envelope.message).toBe("Unknown error"); + }); + + it("preserves a CliExitCodeError's explicit envelope code over the default", () => { + const { envelope } = classifyError( + new CliExitCodeError(EXIT_CODES.TOOL_ERROR, "x", { + code: "tool_not_found", + }), + ); + expect(envelope.code).toBe("tool_not_found"); + }); + + it("derives a default envelope code for a bare CliExitCodeError", () => { + expect( + classifyError(new CliExitCodeError(EXIT_CODES.UNREACHABLE, "x")).envelope + .code, + ).toBe("unreachable"); + expect( + classifyError(new CliExitCodeError(EXIT_CODES.NO_APP, "x")).envelope.code, + ).toBe("no_app"); + expect( + classifyError(new CliExitCodeError(EXIT_CODES.AUTH_REQUIRED, "x")) + .envelope.code, + ).toBe("auth_required"); + expect( + classifyError(new CliExitCodeError(EXIT_CODES.TOOL_ERROR, "x")).envelope + .code, + ).toBe("tool_error"); + expect( + classifyError(new CliExitCodeError(EXIT_CODES.USAGE, "x")).envelope.code, + ).toBe("error"); + }); + + it("captures a non-Error cause as a string", () => { + const err = new Error("outer"); + (err as { cause?: unknown }).cause = { reason: "blocked" }; + const { envelope } = classifyError(err); + expect(envelope.cause).toBe("[object Object]"); + }); + + it("reads a numeric SDK-style .code as an HTTP status", () => { + const err = Object.assign(new Error("forbidden"), { code: 403 }); + const { exitCode, envelope } = classifyError(err); + expect(exitCode).toBe(EXIT_CODES.AUTH_REQUIRED); + expect(envelope.status).toBe(403); + }); +}); - expect(errorSpy).toHaveBeenCalledWith("Unknown error"); +describe("formatErrorOutput", () => { + it("emits one JSON line on stderr ending in a newline", () => { + const { stderr, exitCode } = formatErrorOutput(new Error("nope")); + expect(stderr.endsWith("\n")).toBe(true); + expect(stderr.split("\n").length).toBe(2); + const parsed = JSON.parse(stderr) as { error: { message: string } }; + expect(parsed.error.message).toBe("nope"); + expect(exitCode).toBe(EXIT_CODES.USAGE); }); }); diff --git a/clients/cli/__tests__/format-and-args.test.ts b/clients/cli/__tests__/format-and-args.test.ts new file mode 100644 index 000000000..c36737a85 --- /dev/null +++ b/clients/cli/__tests__/format-and-args.test.ts @@ -0,0 +1,226 @@ +import { describe, it, expect } from "vitest"; +import { getTestMcpServerCommand } from "@modelcontextprotocol/inspector-test-server"; +import { runCli } from "./helpers/cli-runner.js"; +import { + expectCliFailure, + expectCliSuccess, + expectOutputContains, +} from "./helpers/assertions.js"; + +describe("--method initialize", () => { + it("connects and emits the cached InitializeResult fields", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--cli", + "--method", + "initialize", + ]); + expectCliSuccess(result); + const json = JSON.parse(result.stdout) as { + serverInfo: { name: string }; + protocolVersion: string; + capabilities: Record; + }; + expect(json.serverInfo.name).toBeTruthy(); + expect(typeof json.protocolVersion).toBe("string"); + expect(typeof json.capabilities).toBe("object"); + }); +}); + +describe("--format json", () => { + it("wraps tools/list output in a single {result} envelope", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--cli", + "--method", + "tools/list", + "--format", + "json", + ]); + expectCliSuccess(result); + expect(result.stdout.trim().split("\n").length).toBe(1); + const env = JSON.parse(result.stdout) as { result: { tools: unknown[] } }; + expect(Array.isArray(env.result.tools)).toBe(true); + }); + + it("emits {result, appInfo} as one JSON object for an App tool (no banner)", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--cli", + "--method", + "tools/call", + "--tool-name", + "mcp_app_demo", + "--tool-arg", + "title=hello", + "--format", + "json", + ]); + expectCliSuccess(result); + expect(result.stdout).not.toContain("--- MCP App Info ---"); + const env = JSON.parse(result.stdout) as { + result: unknown; + appInfo: { hasApp: boolean; resourceUri: string }; + }; + expect(env.appInfo.hasApp).toBe(true); + expect(env.appInfo.resourceUri).toBe("ui://demo/widget.html"); + }); + + it("wraps --app-info output under {appInfo}", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--cli", + "--method", + "tools/call", + "--tool-name", + "echo", + "--app-info", + "--format", + "json", + ]); + expect(result.exitCode).toBe(2); + const env = JSON.parse(result.stdout.trim()) as { + appInfo: { hasApp: boolean }; + }; + expect(env.appInfo.hasApp).toBe(false); + }); + + it("rejects an unknown --format value", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--cli", + "--method", + "tools/list", + "--format", + "yaml", + ]); + expectCliFailure(result); + expectOutputContains(result, "--format must be 'text' or 'json'"); + }); +}); + +describe("--tool-args-json", () => { + it("passes the JSON object verbatim (string values stay strings)", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--cli", + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-args-json", + JSON.stringify({ message: "012" }), + ]); + expectCliSuccess(result); + // echo returns its args; with --tool-arg this would have coerced to the + // number 12, but --tool-args-json preserves the string. + expect(result.stdout).toContain("012"); + }); + + it("rejects --tool-args-json combined with --tool-arg", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--cli", + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-arg", + "x=1", + "--tool-args-json", + "{}", + ]); + expectCliFailure(result); + expectOutputContains(result, "cannot be combined with --tool-arg"); + }); + + it("rejects malformed JSON", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--cli", + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-args-json", + "{not json}", + ]); + expectCliFailure(result); + expectOutputContains(result, "not valid JSON"); + }); + + it("rejects a non-object value", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--cli", + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-args-json", + "[1,2]", + ]); + expectCliFailure(result); + expectOutputContains(result, "must be a JSON object"); + }); +}); + +describe("MCP_CATALOG_PATH with ad-hoc --server-url", () => { + it("does not conflict with an ad-hoc target (env catalog is ignored)", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli( + [command, ...args, "--cli", "--method", "tools/list"], + { env: { MCP_CATALOG_PATH: "/no/such/catalog.json" } }, + ); + expectCliSuccess(result); + }); +}); + +describe("--connect-timeout", () => { + it("accepts a numeric value and connects within it", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--cli", + "--connect-timeout", + "5000", + "--method", + "tools/list", + ]); + expectCliSuccess(result); + }); + + it("rejects a negative value", async () => { + const { command, args } = getTestMcpServerCommand(); + const result = await runCli([ + command, + ...args, + "--cli", + "--connect-timeout", + "-1", + "--method", + "tools/list", + ]); + expectCliFailure(result); + expectOutputContains(result, "non-negative number"); + }); +}); diff --git a/clients/cli/__tests__/helpers/assertions.ts b/clients/cli/__tests__/helpers/assertions.ts index 3631c00bf..03da5f7e3 100644 --- a/clients/cli/__tests__/helpers/assertions.ts +++ b/clients/cli/__tests__/helpers/assertions.ts @@ -43,15 +43,6 @@ export function expectValidJson(result: CliResult) { return JSON.parse(result.stdout); } -/** - * Assert that output contains JSON with error flag - */ -export function expectJsonError(result: CliResult) { - const json = expectValidJson(result); - expect(json.isError).toBe(true); - return json; -} - /** * Assert that output contains expected JSON structure */ diff --git a/clients/cli/__tests__/helpers/cli-runner.ts b/clients/cli/__tests__/helpers/cli-runner.ts index 38b401d14..d5c8abe66 100644 --- a/clients/cli/__tests__/helpers/cli-runner.ts +++ b/clients/cli/__tests__/helpers/cli-runner.ts @@ -1,4 +1,5 @@ import { runCli as invokeCli } from "../../src/cli.js"; +import { formatErrorOutput } from "../../src/error-handler.js"; export interface CliResult { exitCode: number | null; @@ -52,7 +53,8 @@ function captureWrite(append: (text: string) => void) { * (`exitCode` / `stdout` / `stderr` / `output`) so every existing test and the * shared assertion helpers keep working unchanged: * - stdout/stderr are captured by temporarily patching `process.std*.write` - * (and `console.error`/`console.warn`, which commander and error paths use). + * (which also captures `console.error`/`console.warn`, since those route + * through `process.stderr.write`). * - A thrown error (bad args, failed connect, unsupported method) maps to * `exitCode: 1` with the message appended to stderr — mirroring how the real * binary (`index.ts`) routes errors through `handleError` → `process.exit(1)`. @@ -77,8 +79,6 @@ export async function runCli( const originalStdoutWrite = process.stdout.write; const originalStderrWrite = process.stderr.write; - const originalConsoleError = console.error; - const originalConsoleWarn = console.warn; // Snapshot and apply env overrides; `undefined` records keys that did not // exist so they can be deleted (not set to the string "undefined") on restore. @@ -93,15 +93,12 @@ export async function runCli( process.stdout.write = captureWrite((text) => { stdout += text; }) as typeof process.stdout.write; + // `console.error` / `console.warn` route through `process.stderr.write`, so + // patching the underlying stream captures them too — no separate console + // override is needed (the previous explicit overrides were redundant). process.stderr.write = captureWrite((text) => { stderr += text; }) as typeof process.stderr.write; - console.error = (...parts: unknown[]) => { - stderr += parts.map(String).join(" ") + "\n"; - }; - console.warn = (...parts: unknown[]) => { - stderr += parts.map(String).join(" ") + "\n"; - }; // `runCli` reads `process.argv.slice(2)`, so prepend two placeholder entries // ([node, script]) to mirror how the binary is launched. @@ -127,14 +124,15 @@ export async function runCli( try { await Promise.race([invokeCli(argv), timeout]); } catch (error) { - exitCode = 1; - stderr += (error instanceof Error ? error.message : String(error)) + "\n"; + // Mirror the binary's `handleError` exactly so tests observe the same exit + // code and stderr (one JSON envelope line) the real CLI would emit. + const out = formatErrorOutput(error); + exitCode = out.exitCode; + stderr += out.stderr; } finally { if (timer) clearTimeout(timer); process.stdout.write = originalStdoutWrite; process.stderr.write = originalStderrWrite; - console.error = originalConsoleError; - console.warn = originalConsoleWarn; for (const [key, value] of Object.entries(envBackup)) { if (value === undefined) delete process.env[key]; else process.env[key] = value; diff --git a/clients/cli/__tests__/stored-auth.test.ts b/clients/cli/__tests__/stored-auth.test.ts new file mode 100644 index 000000000..eaf319b2e --- /dev/null +++ b/clients/cli/__tests__/stored-auth.test.ts @@ -0,0 +1,373 @@ +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import { mkdtempSync, writeFileSync, rmSync } from "node:fs"; +import { join, dirname } from "node:path"; +import { tmpdir } from "node:os"; +import { runCli } from "./helpers/cli-runner.js"; +import { expectCliFailure, expectCliSuccess } from "./helpers/assertions.js"; +import { normalizeServerUrl } from "../src/cli.js"; +import { + createTestServerHttp, + createEchoTool, + createTestServerInfo, +} from "@modelcontextprotocol/inspector-test-server"; + +/** + * Writes a Zustand-persist–shaped oauth.json fixture (the same blob the web + * inspector's RemoteOAuthStorage POSTs to /api/storage/oauth) into a temp dir + * and returns the file path. The CLI's NodeOAuthStorage reads this format via + * the same `createOAuthStore` factory. + */ +function writeOAuthFixture(servers: Record): string { + const dir = mkdtempSync(join(tmpdir(), "inspector-cli-stored-auth-")); + const file = join(dir, "oauth.json"); + writeFileSync( + file, + JSON.stringify({ state: { servers }, version: 0 }), + "utf8", + ); + return file; +} + +describe("--use-stored-auth", () => { + let server: ReturnType; + let serverUrl: string; + let fixturePath: string; + const TOKEN = "stored-access-token-abc"; + + beforeAll(async () => { + server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + tools: [createEchoTool()], + }); + await server.start(); + serverUrl = server.url; + fixturePath = writeOAuthFixture({ + [serverUrl]: { tokens: { access_token: TOKEN, token_type: "Bearer" } }, + }); + }); + + afterAll(async () => { + await server.stop(); + rmSync(fixturePath, { force: true }); + }); + + it("injects the stored token as Authorization: Bearer on the outgoing request", async () => { + const result = await runCli( + [ + "--cli", + "--transport", + "http", + "--server-url", + serverUrl, + "--use-stored-auth", + "--method", + "tools/list", + ], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: fixturePath } }, + ); + expectCliSuccess(result); + const recorded = server.getRecordedRequests(); + expect(recorded.length).toBeGreaterThan(0); + const last = recorded[recorded.length - 1]!; + expect(last.method).toBe("tools/list"); + // Express normalises header names to lowercase. + expect(last.headers?.authorization).toBe(`Bearer ${TOKEN}`); + }); + + it("merges with --header (explicit headers + stored auth coexist)", async () => { + const result = await runCli( + [ + "--cli", + "--transport", + "http", + "--server-url", + serverUrl, + "--use-stored-auth", + "--header", + "X-Trace: abc", + "--method", + "tools/list", + ], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: fixturePath } }, + ); + expectCliSuccess(result); + const last = server.getRecordedRequests().at(-1)!; + expect(last.headers?.authorization).toBe(`Bearer ${TOKEN}`); + expect(last.headers?.["x-trace"]).toBe("abc"); + }); + + it("errors clearly when --server-url is missing", async () => { + const result = await runCli( + ["--cli", "--use-stored-auth", "--method", "tools/list"], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: fixturePath } }, + ); + expectCliFailure(result); + expect(result.stderr).toContain("--use-stored-auth requires --server-url"); + }); + + it("errors with exit 3 (AUTH_REQUIRED) and lists stored keys when no token matches", async () => { + const other = writeOAuthFixture({ + "https://other.example/mcp": { + tokens: { access_token: "x", token_type: "Bearer" }, + }, + }); + try { + const result = await runCli( + [ + "--cli", + "--transport", + "http", + "--server-url", + serverUrl, + "--use-stored-auth", + "--method", + "tools/list", + ], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: other } }, + ); + expect(result.exitCode).toBe(3); + const env = JSON.parse(result.stderr.trim()) as { + error: { code: string; message: string }; + }; + expect(env.error.code).toBe("no_stored_token"); + expect(env.error.message).toContain("No stored OAuth token"); + expect(env.error.message).toContain("https://other.example/mcp"); + } finally { + rmSync(other, { force: true }); + } + }); + + it("matches a stored key even when --server-url differs by URL normalisation", async () => { + // Store under the normalised form (lowercased host); pass the upper-cased + // variant on the command line. The lookup should still find it. + const normalised = normalizeServerUrl( + serverUrl.replace("127.0.0.1", "127.0.0.1"), + ); + const upper = serverUrl.replace("http://", "HTTP://"); + const fixture = writeOAuthFixture({ + [normalised]: { + tokens: { access_token: TOKEN, token_type: "Bearer" }, + }, + }); + try { + const result = await runCli( + [ + "--cli", + "--transport", + "http", + "--server-url", + upper, + "--use-stored-auth", + "--method", + "tools/list", + ], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: fixture } }, + ); + expectCliSuccess(result); + } finally { + rmSync(fixture, { force: true }); + } + }); + + it("honours MCP_STORAGE_DIR when MCP_INSPECTOR_OAUTH_STATE_PATH is unset", async () => { + const dir = dirname(fixturePath); + const result = await runCli( + [ + "--cli", + "--transport", + "http", + "--server-url", + serverUrl, + "--use-stored-auth", + "--method", + "tools/list", + ], + { env: { MCP_STORAGE_DIR: dir } }, + ); + expectCliSuccess(result); + const last = server.getRecordedRequests().at(-1)!; + expect(last.headers?.authorization).toBe(`Bearer ${TOKEN}`); + }); +}); + +describe("--list-stored-auth", () => { + it("prints the stored server URLs and the resolved state path", async () => { + const fixture = writeOAuthFixture({ + "https://a.example/mcp": { + tokens: { access_token: "t1", token_type: "Bearer" }, + }, + "https://b.example/mcp": { tokens: {} }, + }); + try { + const result = await runCli(["--cli", "--list-stored-auth"], { + env: { MCP_INSPECTOR_OAUTH_STATE_PATH: fixture }, + }); + expectCliSuccess(result); + const out = JSON.parse(result.stdout) as { + oauthStatePath: string; + storedServerUrls: string[]; + }; + expect(out.oauthStatePath).toBe(fixture); + expect(out.storedServerUrls).toEqual(["https://a.example/mcp"]); + } finally { + rmSync(fixture, { force: true }); + } + }); + + it("emits an empty list when the state file is absent", async () => { + const result = await runCli(["--cli", "--list-stored-auth"], { + env: { MCP_INSPECTOR_OAUTH_STATE_PATH: "/no/such/file.json" }, + }); + expectCliSuccess(result); + const out = JSON.parse(result.stdout) as { storedServerUrls: string[] }; + expect(out.storedServerUrls).toEqual([]); + }); +}); + +describe("--print-handoff", () => { + it("emits a JSON handoff block with deepLink, port-forward command, and the resolved state path", async () => { + const result = await runCli( + ["--cli", "--print-handoff", "--server-url", "https://x.example/mcp"], + { + env: { + MCP_INSPECTOR_API_TOKEN: "tok123", + CLIENT_PORT: "16274", + MCP_SANDBOX_PORT: "16275", + MCP_STORAGE_DIR: "/tmp/inspector-storage", + }, + }, + ); + expectCliSuccess(result); + const out = JSON.parse(result.stdout) as { + serverUrl: string; + deepLink: string; + portForwardCmd: string; + oauthStatePath: string; + apiToken: string; + }; + expect(out.serverUrl).toBe("https://x.example/mcp"); + expect(out.deepLink).toContain("autoConnect=tok123"); + expect(out.deepLink).toContain("serverUrl=https%3A%2F%2Fx.example%2Fmcp"); + expect(out.portForwardCmd).toContain("--tcp 16274:16274"); + expect(out.portForwardCmd).toContain("--tcp 16275:16275"); + expect(out.oauthStatePath).toBe("/tmp/inspector-storage/oauth.json"); + expect(out.apiToken).toBe("tok123"); + }); + + it("includes a note when MCP_INSPECTOR_API_TOKEN is unset", async () => { + const result = await runCli( + ["--cli", "--print-handoff", "--server-url", "https://x.example/mcp"], + { env: {} }, + ); + expectCliSuccess(result); + const out = JSON.parse(result.stdout) as { + apiToken: string | null; + note?: string; + }; + expect(out.apiToken).toBeNull(); + expect(out.note).toContain("MCP_INSPECTOR_API_TOKEN is not set"); + }); + + it("requires --server-url", async () => { + const result = await runCli(["--cli", "--print-handoff"]); + expectCliFailure(result); + expect(result.stderr).toContain("--print-handoff requires --server-url"); + }); +}); + +describe("--wait-for-auth", () => { + let server: ReturnType; + let serverUrl: string; + + beforeAll(async () => { + server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + tools: [createEchoTool()], + }); + await server.start(); + serverUrl = server.url; + }); + + afterAll(async () => { + await server.stop(); + }); + + it("polls until the token appears, then proceeds with it", async () => { + const dir = mkdtempSync(join(tmpdir(), "inspector-cli-wait-")); + const file = join(dir, "oauth.json"); + // Write the token after a short delay so the first poll misses. + setTimeout(() => { + writeFileSync( + file, + JSON.stringify({ + state: { + servers: { + [normalizeServerUrl(serverUrl)]: { + tokens: { access_token: "waited-tok", token_type: "Bearer" }, + }, + }, + }, + version: 0, + }), + "utf8", + ); + }, 200); + try { + const result = await runCli( + [ + "--cli", + "--transport", + "http", + "--server-url", + serverUrl, + "--wait-for-auth", + "5", + "--method", + "tools/list", + ], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: file } }, + ); + expectCliSuccess(result); + const last = server.getRecordedRequests().at(-1)!; + expect(last.headers?.authorization).toBe("Bearer waited-tok"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("times out with exit 3 (AUTH_REQUIRED) when no token appears", async () => { + const result = await runCli( + [ + "--cli", + "--transport", + "http", + "--server-url", + serverUrl, + "--wait-for-auth", + "1", + "--method", + "tools/list", + ], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: "/no/such/file.json" } }, + ); + expect(result.exitCode).toBe(3); + const env = JSON.parse(result.stderr.trim()) as { + error: { code: string }; + }; + expect(env.error.code).toBe("auth_wait_timeout"); + }); + + it("rejects a non-positive timeout", async () => { + const result = await runCli([ + "--cli", + "--server-url", + "https://x.example/mcp", + "--wait-for-auth", + "0", + "--method", + "tools/list", + ]); + expectCliFailure(result); + expect(result.stderr).toContain("positive number of seconds"); + }); +}); diff --git a/clients/cli/__tests__/tools.test.ts b/clients/cli/__tests__/tools.test.ts index 3a7ea6a17..c67a72f04 100644 --- a/clients/cli/__tests__/tools.test.ts +++ b/clients/cli/__tests__/tools.test.ts @@ -4,7 +4,6 @@ import { expectCliSuccess, expectCliFailure, expectValidJson, - expectJsonError, } from "./helpers/assertions.js"; import { getTestMcpServerCommand } from "@modelcontextprotocol/inspector-test-server"; @@ -378,8 +377,14 @@ describe("Tool Tests", () => { "message=test", ]); - // CLI returns exit code 0 but includes isError: true in JSON (server returns error) - expectJsonError(result); + // Tool-not-found is now a hard failure (exit 5, code:tool_not_found) so a + // typo/rename is distinguishable from a tool that ran and returned + // isError:true. + expect(result.exitCode).toBe(5); + const envelope = JSON.parse(result.stderr.trim()) as { + error: { code: string }; + }; + expect(envelope.error.code).toBe("tool_not_found"); }); it("should fail when tool name is missing", async () => { diff --git a/clients/cli/package-lock.json b/clients/cli/package-lock.json index 7923015d6..90f6d16f9 100644 --- a/clients/cli/package-lock.json +++ b/clients/cli/package-lock.json @@ -14,6 +14,7 @@ "atomically": "^2.1.1", "commander": "^13.1.0", "pino": "^9.14.0", + "undici": "^8.5.0", "zod": "^4.3.6", "zustand": "^5.0.13" }, @@ -1035,9 +1036,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1055,9 +1053,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -1075,9 +1070,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1095,9 +1087,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1115,9 +1104,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -1135,9 +1121,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -3677,9 +3660,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -3701,9 +3681,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -3725,9 +3702,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -3749,9 +3723,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -5188,6 +5159,15 @@ "dev": true, "license": "MIT" }, + "node_modules/undici": { + "version": "8.5.0", + "resolved": "https://registry.npmjs.org/undici/-/undici-8.5.0.tgz", + "integrity": "sha512-xamtWoB1EshgjpmlXd7GGm2VfdDtw1+rD8uhry8pSNW3If6S8E0m2T2+orSKeZXEn/aPJMviCpDBA65WJt8zhg==", + "license": "MIT", + "engines": { + "node": ">=22.19.0" + } + }, "node_modules/undici-types": { "version": "7.18.2", "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.18.2.tgz", diff --git a/clients/cli/package.json b/clients/cli/package.json index 0175b69a7..28416abc1 100644 --- a/clients/cli/package.json +++ b/clients/cli/package.json @@ -37,6 +37,7 @@ "atomically": "^2.1.1", "commander": "^13.1.0", "pino": "^9.14.0", + "undici": "^8.5.0", "zod": "^4.3.6", "zustand": "^5.0.13" }, diff --git a/clients/cli/src/cli.ts b/clients/cli/src/cli.ts index 6539da8d2..b886e81ba 100644 --- a/clients/cli/src/cli.ts +++ b/clients/cli/src/cli.ts @@ -7,6 +7,10 @@ import type { InspectorServerSettings, MCPServerConfig, } from "@inspector/core/mcp/types.js"; +import { + DEFAULT_MAX_FETCH_REQUESTS, + DEFAULT_TASK_TTL_MS, +} from "@inspector/core/mcp/types.js"; import { InspectorClient } from "@inspector/core/mcp/index.js"; import { ManagedToolsState, @@ -22,6 +26,9 @@ import { parseHeaderPair, } from "@inspector/core/mcp/node/index.js"; import type { JsonValue } from "@inspector/core/mcp/index.js"; +import { extractAppInfo } from "@inspector/core/mcp/apps.js"; +import type { AppInfo } from "@inspector/core/mcp/apps.js"; +import { CliExitCodeError, EXIT_CODES } from "./error-handler.js"; import { LoggingLevelSchema, type LoggingLevel, @@ -31,6 +38,8 @@ export const validLogLevels: LoggingLevel[] = Object.values( LoggingLevelSchema.enum, ); +type OutputFormat = "text" | "json"; + type MethodArgs = { method?: string; promptName?: string; @@ -41,8 +50,18 @@ type MethodArgs = { toolArg?: Record; toolMeta?: Record; metadata?: Record; + appInfo?: boolean; + format?: OutputFormat; }; +/** + * Default connect timeout (ms) for ad-hoc server invocations. Without this an + * unreachable server (e.g. a partner edge that drops the SYN) hangs the CLI + * indefinitely; the value is generous enough for cold-start OAuth discovery + * round-trips while still failing fast on a black-holed host. + */ +export const DEFAULT_CONNECT_TIMEOUT_MS = 15000; + async function callMethod( serverConfig: MCPServerConfig, serverSettings: InspectorServerSettings | undefined, @@ -84,6 +103,7 @@ async function callMethod( await inspectorClient.connect(); let result: McpResponse; + let appInfo: CliAppInfo | undefined; if (args.method === "tools/list" || args.method === "tools/call") { managedToolsState = new ManagedToolsState(inspectorClient); @@ -108,7 +128,21 @@ async function callMethod( } if (args.method === "tools/list") { - result = { tools: managedToolsState!.getTools() }; + const tools = managedToolsState!.getTools(); + if (args.appInfo) { + // NDJSON: one app-info line per tool, all on a single connection. A + // caller that wants only the App tools can `| jq -c 'select(.hasApp)'`. + for (const tool of tools) { + const info = await collectAppInfo( + inspectorClient, + tool, + args.metadata, + ); + await awaitableLog(JSON.stringify(info) + "\n"); + } + return; + } + result = { tools }; } else if (args.method === "tools/call") { if (!args.toolName) { throw new Error( @@ -120,34 +154,48 @@ async function callMethod( .getTools() .find((t) => t.name === args.toolName); if (!tool) { - result = { - content: [ - { - type: "text" as const, - text: `Tool '${args.toolName}' not found.`, - }, - ], - isError: true, - }; - } else { - const invocation = await inspectorClient.callTool( - tool, - args.toolArg || {}, - args.metadata, - args.toolMeta, + // Distinct from `isError:true` and (for --app-info) from "tool has no + // app": the named tool does not exist on the server. Exit TOOL_ERROR + // with `code: "tool_not_found"` so a caller can tell a typo/rename + // apart from a real tool failure or a no-app probe result. + throw new CliExitCodeError( + EXIT_CODES.TOOL_ERROR, + `Tool '${args.toolName}' not found on server.`, + { code: "tool_not_found" }, ); - if (invocation.result !== null) { - result = invocation.result; + } else { + // Only collect app-info when the caller asked for it (`--app-info` or + // `--format json`); a plain text-mode `tools/call` shouldn't fail just + // because the tool's `_meta.ui.resourceUri` is malformed or its + // resource is unreadable. + if (args.appInfo || args.format === "json") { + appInfo = await collectAppInfo(inspectorClient, tool, args.metadata); + } + if (args.appInfo) { + // --app-info: probe-only — emit the app metadata and skip the tool + // call entirely. The result is the AppInfo block; the no-app exit + // code is handled after disconnect below. + result = { ...appInfo }; } else { - result = { - content: [ - { - type: "text" as const, - text: invocation.error || "Tool call failed", - }, - ], - isError: true, - }; + const invocation = await inspectorClient.callTool( + tool, + args.toolArg || {}, + args.metadata, + args.toolMeta, + ); + if (invocation.result !== null) { + result = invocation.result; + } else { + result = { + content: [ + { + type: "text" as const, + text: invocation.error || "Tool call failed", + }, + ], + isError: true, + }; + } } } } else if (args.method === "resources/list") { @@ -186,6 +234,16 @@ async function callMethod( args.metadata, ); result = invocation.result; + } else if (args.method === "initialize") { + // Connect-only probe: emit the cached InitializeResult fields so a + // caller can read serverInfo / protocolVersion / capabilities / + // instructions without picking a list method. + result = { + serverInfo: inspectorClient.getServerInfo(), + protocolVersion: inspectorClient.getProtocolVersion(), + capabilities: inspectorClient.getCapabilities(), + instructions: inspectorClient.getInstructions(), + }; } else if (args.method === "logging/setLevel") { if (!args.logLevel) { throw new Error( @@ -197,11 +255,11 @@ async function callMethod( result = {}; } else { throw new Error( - `Unsupported method: ${args.method}. Supported methods include: tools/list, tools/call, resources/list, resources/read, resources/templates/list, prompts/list, prompts/get, logging/setLevel`, + `Unsupported method: ${args.method}. Supported methods include: initialize, tools/list, tools/call, resources/list, resources/read, resources/templates/list, prompts/list, prompts/get, logging/setLevel`, ); } - await awaitableLog(JSON.stringify(result, null, 2)); + await emitResult(result, appInfo, args); } finally { managedToolsState?.destroy(); managedResourcesState?.destroy(); @@ -211,6 +269,245 @@ async function callMethod( } } +/** + * Write the method result (and any app-info) to stdout, honouring `--format` + * and `--app-info`, then map `isError`/no-app outcomes onto the exit-code map. + * Extracted from `callMethod` so the format/exit handling is in one place. + */ +async function emitResult( + result: McpResponse, + appInfo: CliAppInfo | undefined, + args: MethodArgs, +): Promise { + const json = args.format === "json"; + + if (args.appInfo) { + const info: CliAppInfo = appInfo ?? { + hasApp: false, + toolName: args.toolName ?? "", + }; + // Single-line JSON either way; --format json wraps it under an `appInfo` + // key so the envelope shape is uniform with the non-probe path. + await awaitableLog(JSON.stringify(json ? { appInfo: info } : info) + "\n"); + if (!info.hasApp) { + throw new CliExitCodeError( + EXIT_CODES.NO_APP, + `Tool '${args.toolName}' has no MCP App UI resource (_meta.ui.resourceUri).`, + ); + } + return; + } + + if (json) { + // One JSON object on stdout — `result` plus, when present, `appInfo` as a + // sibling key. No `--- MCP App Info ---` banner, so `| jq` works for App + // tools as well as plain ones. + const envelope: Record = { result }; + if (appInfo?.hasApp) envelope.appInfo = appInfo; + await awaitableLog(JSON.stringify(envelope) + "\n"); + } else { + // Text mode emits the result only; app-info is not collected on this path + // (use `--format json` or `--app-info` to get it). + await awaitableLog(JSON.stringify(result, null, 2) + "\n"); + } + + // A tool that returned `isError:true` (or whose call failed) is still + // printed above so the caller sees the payload, but the process exits + // TOOL_ERROR so `&&` chains don't proceed on a failed call. + if ((result as { isError?: unknown }).isError === true) { + throw new CliExitCodeError( + EXIT_CODES.TOOL_ERROR, + `Tool '${args.toolName}' returned isError:true.`, + { code: "tool_is_error" }, + ); + } +} + +/** + * {@link AppInfo} plus a CLI-only `resourceError` so a `resources/read` failure + * during the probe is reported instead of being silently swallowed (which would + * make "no CSP declared" indistinguishable from "resource unreadable"). + */ +type CliAppInfo = AppInfo & { resourceError?: string }; + +/** + * Build the CLI's app-info for a tool: extract the tool-side `_meta.ui` and, + * when the tool advertises a UI resource, follow it with a `resources/read` so + * the resource-side csp/permissions/domain are included. A read failure is + * tolerated — the tool-side info is still returned with `resourceError` set, + * since "tool says it has an app but the resource is unreadable" is itself a + * useful probe result. + */ +async function collectAppInfo( + client: InspectorClient, + tool: Parameters[0], + metadata: Record | undefined, +): Promise { + const base = extractAppInfo(tool); + if (!base.hasApp || base.resourceUri === undefined) return base; + try { + const read = await client.readResource(base.resourceUri, metadata); + return extractAppInfo(tool, read.result); + } catch (e) { + return { + ...base, + resourceError: e instanceof Error ? e.message : String(e), + }; + } +} + +/** + * Canonicalise a server URL the same way the web inspector does before storing + * OAuth state (`new URL().href` lowercases the host, normalises the scheme, + * and adds a trailing `/` for bare-origin URLs). The CLI must look up by the + * same key the web side wrote, so a trailing-slash or case mismatch doesn't + * miss a token that's sitting one key over. + * + * TODO: dedupe with `core/auth/store.ts` once that module exports the same + * normaliser. + */ +export function normalizeServerUrl(serverUrl: string): string { + try { + return new URL(serverUrl).href; + } catch { + return serverUrl; + } +} + +/** + * Resolve the path to the OAuth state file. Precedence: + * 1. `MCP_INSPECTOR_OAUTH_STATE_PATH` (deprecated; kept for tests/scripts that + * already pin to a fixture file) + * 2. `/oauth.json` — `MCP_STORAGE_DIR` is what the web + * backend honours, so setting it once points both sides at the same file + * 3. `~/.mcp-inspector/storage/oauth.json` (the default) + */ +export function resolveOAuthStatePath(): string { + const explicit = process.env.MCP_INSPECTOR_OAUTH_STATE_PATH; + if (explicit) return explicit; + const dir = process.env.MCP_STORAGE_DIR; + if (dir) return join(dir, "oauth.json"); + // Fall through to NodeOAuthStorage's own default; the function in + // `core/auth/node/storage-node.ts` has the same `~/.mcp-inspector/storage` + // resolution. Returning an explicit path here lets `--list-stored-auth` and + // `--print-handoff` report the actual file location. + const home = process.env.HOME || process.env.USERPROFILE || "."; + return join(home, ".mcp-inspector", "storage", "oauth.json"); +} + +/** Shape of the Zustand-persist blob the OAuth store writes to disk. */ +type PersistedOAuthBlob = { + state?: { + servers?: Record; + }; +}; + +/** + * Read the OAuth state file directly (bypassing the Zustand store cache) so + * each call sees the current on-disk state. Returns the `servers` map, or an + * empty object when the file is absent or unreadable. + */ +async function readOAuthServers( + statePath: string, +): Promise["servers"]>> { + const { readFile } = await import("node:fs/promises"); + try { + const text = await readFile(statePath, "utf8"); + const blob = JSON.parse(text) as PersistedOAuthBlob; + return blob.state?.servers ?? {}; + } catch { + return {}; + } +} + +/** + * Poll the OAuth state file until a token for `serverUrl` appears (or the + * timeout elapses). Used by `--wait-for-auth` so an automated caller can hand + * off to a human for the OAuth dance and resume once the token lands. The + * lookup is normalised, so a trailing-slash mismatch between the URL the human + * opened and the one the agent passed still resolves. + */ +async function waitForStoredToken( + serverUrl: string, + statePath: string, + timeoutSec: number, +): Promise { + const key = normalizeServerUrl(serverUrl); + const deadline = Date.now() + timeoutSec * 1000; + for (;;) { + const servers = await readOAuthServers(statePath); + const token = + servers[key]?.tokens?.access_token ?? + servers[serverUrl]?.tokens?.access_token; + if (token) return token; + if (Date.now() >= deadline) { + const stored = Object.keys(servers); + throw new CliExitCodeError( + EXIT_CODES.AUTH_REQUIRED, + `--wait-for-auth timed out after ${timeoutSec}s; no stored OAuth token for ${key} in ${statePath}.` + + (stored.length > 0 + ? ` Stored keys: ${stored.join(", ")}.` + : " No tokens stored yet."), + { code: "auth_wait_timeout", url: serverUrl }, + ); + } + await new Promise((r) => setTimeout(r, 500)); + } +} + +/** + * Build the JSON `--print-handoff` emits: everything an automated caller needs + * to relay to a human so they can complete OAuth via port-forward and have the + * token land where the CLI will find it. + */ +function buildHandoff(serverUrl: string, statePath: string): McpResponse { + const host = process.env.HOST || "127.0.0.1"; + const clientPort = process.env.CLIENT_PORT || "6274"; + const sandboxPort = process.env.MCP_SANDBOX_PORT || "6275"; + const apiToken = process.env.MCP_INSPECTOR_API_TOKEN; + const params = new URLSearchParams({ + serverUrl, + transport: "http", + }); + if (apiToken) params.set("autoConnect", apiToken); + return { + serverUrl: normalizeServerUrl(serverUrl), + deepLink: `http://${host}:${clientPort}/?${params.toString()}`, + portForwardCmd: `coder port-forward --tcp ${clientPort}:${clientPort} --tcp ${sandboxPort}:${sandboxPort}`, + oauthStatePath: statePath, + apiToken: apiToken ?? null, + note: + apiToken === undefined + ? "MCP_INSPECTOR_API_TOKEN is not set; deep-link autoConnect gate will reject — launch the web inspector with a known token first." + : undefined, + }; +} + +/** + * Apply a connection timeout to a resolved server's settings, building a + * minimal {@link InspectorServerSettings} when none came from the file. Ad-hoc + * invocations get {@link DEFAULT_CONNECT_TIMEOUT_MS} so a black-holed host + * fails fast; catalog/config invocations keep their file-level timeout unless + * `--connect-timeout` is passed explicitly. + */ +function withConnectTimeout( + settings: InspectorServerSettings | undefined, + connectionTimeout: number | undefined, +): InspectorServerSettings | undefined { + if (connectionTimeout === undefined) return settings; + if (settings) return { ...settings, connectionTimeout }; + return { + headers: [], + metadata: [], + connectionTimeout, + requestTimeout: 0, + taskTtl: DEFAULT_TASK_TTL_MS, + maxFetchRequests: DEFAULT_MAX_FETCH_REQUESTS, + autoRefreshOnListChanged: false, + roots: [], + }; +} + function parseKeyValuePair( value: string, previous: Record = {}, @@ -235,11 +532,18 @@ function parseKeyValuePair( return { ...previous, [key as string]: parsedValue }; } -function parseArgs(argv?: string[]): { - serverConfig: MCPServerConfig; - serverSettings: InspectorServerSettings | undefined; - methodArgs: MethodArgs & { method: string }; -} { +type ParseResult = + | { + shortCircuit?: undefined; + serverConfig: MCPServerConfig; + serverSettings: InspectorServerSettings | undefined; + methodArgs: MethodArgs & { method: string }; + } + // Short-circuit modes (`--list-stored-auth`, `--print-handoff`) do their own + // output and need no server connection; runCli returns immediately. + | { shortCircuit: true }; + +async function parseArgs(argv?: string[]): Promise { const program = new Command(); // On a parse/usage ERROR (exitCode !== 0), throw the CommanderError instead // of letting commander call process.exit(). The binary entry (index.ts) still @@ -359,6 +663,60 @@ function parseArgs(argv?: string[]): { "Tool-specific metadata as key=value pairs (for tools/call method only)", parseKeyValuePair, {}, + ) + .option( + "--app-info", + "Probe the tool's MCP App UI metadata (resourceUri, csp, permissions, domain) and emit it as one JSON line; exit 2 when the tool has no app. Use with --method tools/call --tool-name ; the tool itself is not invoked.", + ) + .option( + "--connect-timeout ", + `Connection timeout in ms (default ${DEFAULT_CONNECT_TIMEOUT_MS} for ad-hoc --server-url / target invocations; 0 = no timeout).`, + (v: string) => { + const n = Number(v); + if (!Number.isFinite(n) || n < 0) { + throw new Error(`--connect-timeout must be a non-negative number.`); + } + return n; + }, + ) + .option( + "--format ", + "Output format: text (default; pretty-printed) or json (one JSON object on stdout, no banners).", + (v: string): OutputFormat => { + if (v !== "text" && v !== "json") { + throw new Error(`--format must be 'text' or 'json'.`); + } + return v; + }, + ) + .option( + "--tool-args-json ", + 'Tool arguments as a single JSON object (e.g. \'{"zip":"10001"}\'). Values are passed verbatim — no key=value coercion. Mutually exclusive with --tool-arg.', + ) + .option( + "--use-stored-auth", + "Read the OAuth access token for --server-url from the OAuth state file (written by the web inspector) and inject it as Authorization: Bearer.", + ) + .option( + "--wait-for-auth ", + "Poll the OAuth state file until a token for --server-url appears (or the timeout elapses), then proceed as if --use-stored-auth were set. Use after handing off to a human to complete OAuth via port-forward.", + (v: string) => { + const n = Number(v); + if (!Number.isFinite(n) || n <= 0) { + throw new Error( + `--wait-for-auth must be a positive number of seconds.`, + ); + } + return n; + }, + ) + .option( + "--list-stored-auth", + "Print the server URLs that have a stored OAuth token (one JSON array on stdout) and exit. No server connection is made.", + ) + .option( + "--print-handoff", + "Print a JSON handoff block (deepLink, portForwardCmd, oauthStatePath, apiToken) for --server-url and exit. No server connection is made.", ); program.parse(preArgs); @@ -381,12 +739,49 @@ function parseArgs(argv?: string[]): { transport?: "sse" | "http" | "stdio"; serverUrl?: string; header?: Record; + appInfo?: boolean; + useStoredAuth?: boolean; + connectTimeout?: number; + format?: OutputFormat; + toolArgsJson?: string; + waitForAuth?: number; + listStoredAuth?: boolean; + printHandoff?: boolean; }; + const oauthStatePath = resolveOAuthStatePath(); + + // Short-circuit modes that need no server connection. + if (options.listStoredAuth) { + const servers = await readOAuthServers(oauthStatePath); + const withToken = Object.entries(servers) + .filter(([, v]) => Boolean(v.tokens?.access_token)) + .map(([k]) => k); + await awaitableLog( + JSON.stringify({ oauthStatePath, storedServerUrls: withToken }) + "\n", + ); + return { shortCircuit: true }; + } + if (options.printHandoff) { + if (!options.serverUrl) { + throw new Error("--print-handoff requires --server-url"); + } + await awaitableLog( + JSON.stringify(buildHandoff(options.serverUrl, oauthStatePath)) + "\n", + ); + return { shortCircuit: true }; + } + + // Honour MCP_CATALOG_PATH only when no ad-hoc target is given. Applying it + // unconditionally meant a homespace that exports the env var could never run + // `--server-url …` (serverSourceConflict rejects catalog + ad-hoc). + const adHoc = + targetArgs.length > 0 || + Boolean(options.transport) || + Boolean(options.serverUrl?.trim()); + const envCatalog = adHoc ? undefined : process.env.MCP_CATALOG_PATH; const serverOptions = { - // `?.trim() ||` (not `??`) so an explicit empty `--catalog ""` still falls - // back to MCP_CATALOG_PATH — keeps CLI and TUI flag resolution identical. - catalogPath: options.catalog?.trim() || process.env.MCP_CATALOG_PATH, + catalogPath: options.catalog?.trim() || envCatalog, configPath: options.config?.trim() || undefined, target: targetArgs.length > 0 ? targetArgs : undefined, transport: options.transport, @@ -398,13 +793,55 @@ function parseArgs(argv?: string[]): { headers: options.header, }; + if (options.waitForAuth !== undefined || options.useStoredAuth) { + if (!options.serverUrl) { + throw new Error( + `${options.waitForAuth !== undefined ? "--wait-for-auth" : "--use-stored-auth"} requires --server-url`, + ); + } + // Read the OAuth state file directly so the lookup is normalised the same + // way the web inspector wrote it (`new URL().href`), and so `--wait-for- + // auth` sees fresh on-disk state on each poll. Header injection is the + // prototype path — wiring NodeOAuthStorage into the SDK auth provider (so + // refresh works) is a follow-up. + let token: string | undefined; + if (options.waitForAuth !== undefined) { + token = await waitForStoredToken( + options.serverUrl, + oauthStatePath, + options.waitForAuth, + ); + } else { + const servers = await readOAuthServers(oauthStatePath); + const key = normalizeServerUrl(options.serverUrl); + token = + servers[key]?.tokens?.access_token ?? + servers[options.serverUrl]?.tokens?.access_token; + if (!token) { + const stored = Object.keys(servers); + throw new CliExitCodeError( + EXIT_CODES.AUTH_REQUIRED, + `No stored OAuth token for ${key} in ${oauthStatePath}. Complete the OAuth flow in the web inspector first.` + + (stored.length > 0 ? ` Stored keys: ${stored.join(", ")}.` : ""), + { code: "no_stored_token", url: options.serverUrl }, + ); + } + } + serverOptions.headers = { + ...(serverOptions.headers ?? {}), + Authorization: `Bearer ${token}`, + }; + } + // Shared with the TUI: resolves the catalog/config source (or ad-hoc target), // enforces the conflict matrix, and lifts disk headers/timeouts/OAuth into // per-server settings. `--server` selects one when the file has several. const entries = loadServerEntries(serverOptions); - const { config: serverConfig, settings: serverSettings } = selectServerEntry( - entries, - options.server, + const selected = selectServerEntry(entries, options.server); + const serverConfig = selected.config; + const serverSettings = withConnectTimeout( + selected.settings, + options.connectTimeout ?? (adHoc ? DEFAULT_CONNECT_TIMEOUT_MS : undefined), ); if (!options.method) { @@ -413,10 +850,47 @@ function parseArgs(argv?: string[]): { ); } + if ( + options.appInfo && + options.method !== "tools/call" && + options.method !== "tools/list" + ) { + throw new Error( + "--app-info requires --method tools/call (with --tool-name) or --method tools/list.", + ); + } + + // --tool-args-json passes arguments verbatim with no key=value coercion (so + // `"012"` stays a string and nested objects work without shell escaping). + let toolArg = options.toolArg; + if (options.toolArgsJson !== undefined) { + if (toolArg && Object.keys(toolArg).length > 0) { + throw new Error( + "--tool-args-json cannot be combined with --tool-arg; pick one.", + ); + } + let parsed: unknown; + try { + parsed = JSON.parse(options.toolArgsJson); + } catch (e) { + throw new Error( + `--tool-args-json is not valid JSON: ${(e as Error).message}`, + ); + } + if ( + parsed === null || + typeof parsed !== "object" || + Array.isArray(parsed) + ) { + throw new Error("--tool-args-json must be a JSON object."); + } + toolArg = parsed as Record; + } + const methodArgs: MethodArgs & { method: string } = { method: options.method, toolName: options.toolName, - toolArg: options.toolArg, + toolArg, uri: options.uri, promptName: options.promptName, promptArgs: options.promptArgs, @@ -437,6 +911,8 @@ function parseArgs(argv?: string[]): { ]), ) : undefined, + appInfo: options.appInfo === true, + format: options.format, }; return { @@ -447,8 +923,11 @@ function parseArgs(argv?: string[]): { } export async function runCli(argv?: string[]): Promise { - const { serverConfig, serverSettings, methodArgs } = parseArgs( - argv ?? process.argv, + const parsed = await parseArgs(argv ?? process.argv); + if (parsed.shortCircuit) return; + await callMethod( + parsed.serverConfig, + parsed.serverSettings, + parsed.methodArgs, ); - await callMethod(serverConfig, serverSettings, methodArgs); } diff --git a/clients/cli/src/error-handler.ts b/clients/cli/src/error-handler.ts index 972577453..b6506e9a8 100644 --- a/clients/cli/src/error-handler.ts +++ b/clients/cli/src/error-handler.ts @@ -1,20 +1,206 @@ -function formatError(error: unknown): string { - let message: string; - - if (error instanceof Error) { - message = error.message; - } else if (typeof error === "string") { - message = error; - } else { - message = "Unknown error"; +/** + * Exit-code map. Non-zero codes let an automated caller (CI, an agent) branch + * on the failure class without regex-scraping stderr: + * + * - 0: success + * - 1: usage / unexpected error (the catch-all; same as before this map) + * - 2: `--app-info` probe found no MCP App on the tool + * - 3: server requires authentication (401 / WWW-Authenticate / OAuth) + * - 4: server unreachable (DNS, connect refused, timeout, fetch failure) + * - 5: tool error (`tools/call` returned `isError:true`, or tool not found) + */ +export const EXIT_CODES = { + OK: 0, + USAGE: 1, + NO_APP: 2, + AUTH_REQUIRED: 3, + UNREACHABLE: 4, + TOOL_ERROR: 5, +} as const; + +/** Machine-readable error envelope written as one JSON line on stderr. */ +export interface ErrorEnvelope { + /** Stable identifier for the failure class (e.g. "auth_required"). */ + code: string; + /** Human-readable message. */ + message: string; + /** The underlying error's `cause` (e.g. undici's ENOTFOUND), if any. */ + cause?: string; + /** HTTP status when the failure was an HTTP-level response. */ + status?: number; + /** The server URL the failure was against, when known. */ + url?: string; +} + +/** + * Thrown by the CLI to request a specific non-zero exit code without routing + * through the generic error path. {@link formatErrorOutput} reads `exitCode` + * and `envelope`; the in-process test runner does the same so tests observe + * the real code and stderr. + */ +export class CliExitCodeError extends Error { + constructor( + public readonly exitCode: number, + message: string, + public readonly envelope?: Partial, + ) { + super(message); + this.name = "CliExitCodeError"; } +} - return message; +/** Read an `Error.cause` chain into a single readable string, if present. */ +function causeOf(error: unknown): string | undefined { + if (!(error instanceof Error)) return undefined; + const cause = (error as { cause?: unknown }).cause; + if (cause === undefined || cause === null) return undefined; + if (cause instanceof Error) { + const nested = causeOf(cause); + return nested ? `${cause.message}: ${nested}` : cause.message; + } + return String(cause); } -export function handleError(error: unknown): never { - const errorMessage = formatError(error); - console.error(errorMessage); +/** Best-effort HTTP status from common error shapes (undici, MCP SDK, fetch). */ +function statusOf(error: unknown): number | undefined { + if (error == null || typeof error !== "object") return undefined; + const e = error as { + status?: unknown; + code?: unknown; + response?: { status?: unknown }; + }; + if (typeof e.status === "number") return e.status; + if (typeof e.response?.status === "number") return e.response.status; + // SDK SSEError exposes `.code` as the HTTP status; numeric only (avoid + // mistaking node error codes like "ENOTFOUND" for a status). + if (typeof e.code === "number") return e.code; + return undefined; +} + +const UNREACHABLE_PATTERN = + /ENOTFOUND|ECONNREFUSED|ECONNRESET|EAI_AGAIN|ETIMEDOUT|fetch failed|getaddrinfo|connect timed out|aborted/i; + +/** + * Classify an arbitrary error into an exit code and envelope. Used both by the + * binary's {@link handleError} and by callers that want to throw a + * {@link CliExitCodeError} with the right code up front. + */ +export function classifyError( + error: unknown, + context?: { url?: string }, +): { exitCode: number; envelope: ErrorEnvelope } { + const message = + error instanceof Error + ? error.message + : typeof error === "string" + ? error + : "Unknown error"; + const cause = causeOf(error); + const status = statusOf(error); + const url = context?.url; - process.exit(1); + // A pre-classified error carries its own exit code; fill in any envelope + // fields the thrower didn't supply. + if (error instanceof CliExitCodeError) { + return { + exitCode: error.exitCode, + envelope: { + code: error.envelope?.code ?? codeForExit(error.exitCode), + message, + ...(cause !== undefined && { cause }), + ...(error.envelope?.status !== undefined && { + status: error.envelope.status, + }), + ...((error.envelope?.url ?? url) !== undefined && { + url: error.envelope?.url ?? url, + }), + }, + }; + } + + // 401 / OAuth-required → AUTH_REQUIRED so the caller can kick the auth flow. + if ( + status === 401 || + status === 403 || + /WWW-Authenticate|Unauthorized|invalid_token|OAuth/i.test( + message + " " + (cause ?? ""), + ) + ) { + return { + exitCode: EXIT_CODES.AUTH_REQUIRED, + envelope: { + code: "auth_required", + message, + ...(cause !== undefined && { cause }), + ...(status !== undefined && { status }), + ...(url !== undefined && { url }), + }, + }; + } + + // Network-layer failure → UNREACHABLE so the caller can retry via a proxy. + if (UNREACHABLE_PATTERN.test(message + " " + (cause ?? ""))) { + return { + exitCode: EXIT_CODES.UNREACHABLE, + envelope: { + code: "unreachable", + message, + ...(cause !== undefined && { cause }), + ...(status !== undefined && { status }), + ...(url !== undefined && { url }), + }, + }; + } + + return { + exitCode: EXIT_CODES.USAGE, + envelope: { + code: "error", + message, + ...(cause !== undefined && { cause }), + ...(status !== undefined && { status }), + ...(url !== undefined && { url }), + }, + }; +} + +function codeForExit(exitCode: number): string { + switch (exitCode) { + case EXIT_CODES.NO_APP: + return "no_app"; + case EXIT_CODES.AUTH_REQUIRED: + return "auth_required"; + case EXIT_CODES.UNREACHABLE: + return "unreachable"; + case EXIT_CODES.TOOL_ERROR: + return "tool_error"; + default: + return "error"; + } +} + +/** + * Single source of truth for mapping a thrown error to the CLI's exit code and + * stderr text. The binary entry ({@link handleError}) and the in-process test + * runner both call this, so tests observe exactly what the binary would emit. + * + * stderr is one JSON line `{"error":{...}}` so a caller can `2>&1 | tail -1 | + * jq .error`; the message is inside the envelope so nothing is lost vs. the + * previous bare-message behaviour. + */ +export function formatErrorOutput( + error: unknown, + context?: { url?: string }, +): { exitCode: number; stderr: string } { + const { exitCode, envelope } = classifyError(error, context); + return { + exitCode, + stderr: JSON.stringify({ error: envelope }) + "\n", + }; +} + +export function handleError(error: unknown): never { + const { exitCode, stderr } = formatErrorOutput(error); + process.stderr.write(stderr); + process.exit(exitCode); } diff --git a/clients/web/server/sandbox-controller.ts b/clients/web/server/sandbox-controller.ts index 16ae5d3c0..c499c67f0 100644 --- a/clients/web/server/sandbox-controller.ts +++ b/clients/web/server/sandbox-controller.ts @@ -47,6 +47,18 @@ export function createSandboxController( let server: Server | null = null; let sandboxUrl: string | null = null; + // Defense-in-depth for the proxy page itself. Only `frame-ancestors` is set + // here — fetch directives (`default-src`, `connect-src`, etc.) are + // deliberately omitted because a `srcdoc` iframe clones its embedder's CSP + // policy container: any fetch directive on this header would be inherited by + // the inner app document and, since multiple CSPs intersect, would override + // the per-app `connect-src`/`img-src` allowlists the host bakes into the + // wrapped HTML (see src/lib/sandbox-csp.ts). The opaque-origin sandbox on + // the inner frame is the structural boundary; `frame-ancestors` ensures the + // proxy can only be embedded by the local inspector itself. + const SANDBOX_PROXY_CSP = + "frame-ancestors http://127.0.0.1:* http://localhost:*"; + let sandboxHtml: string; try { const sandboxHtmlPath = join(__dirname, "../static/sandbox_proxy.html"); @@ -88,6 +100,7 @@ export function createSandboxController( "Content-Type": "text/html; charset=utf-8", "Cache-Control": "no-store, no-cache, must-revalidate", Pragma: "no-cache", + "Content-Security-Policy": SANDBOX_PROXY_CSP, }); res.end(sandboxHtml); }); diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 544cd5e47..145446e66 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -88,6 +88,12 @@ import type { import type { GetPromptState } from "./components/screens/PromptsScreen/PromptsScreen"; import type { ReadResourceState } from "./components/screens/ResourcesScreen/ResourcesScreen"; import type { TaskProgress } from "./components/groups/TaskCard/TaskCard"; +import { + parseDeepLink, + deepLinkConfigEquals, + deepLinkParseStatus, +} from "./utils/deepLink"; +import type { DeepLink, DeepLinkParseStatus } from "./utils/deepLink"; import { EMPTY_TOOLS_UI, EMPTY_PROMPTS_UI, @@ -497,6 +503,33 @@ function App() { authToken: getAuthToken(), }); + // Mirror of `servers` for callbacks that must always read the LATEST list + // even when invoked from a closure captured on an earlier render. The + // deep-link auto-connect IIFE awaits `updateServer` then calls + // `onToggleConnection` from the same closure; without this ref the connect + // would resolve `servers.find(id)` against the pre-update array and build a + // client from the stale config. + const serversRef = useRef(servers); + useEffect(() => { + serversRef.current = servers; + }, [servers]); + + // Last connection-level error message, surfaced as `data-error-message` on + // the InspectorView header so an automated driver can read *why* a connect + // failed without scraping a transient toast. Cleared on the next connect + // attempt and on successful connection. + const [connectErrorMessage, setConnectErrorMessage] = useState< + string | undefined + >(undefined); + // Named writer so the lint rule that flags direct setState-in-effect + // (react-hooks/set-state-in-effect) is satisfied — the OAuth-callback and + // deep-link effects are run-once via ref guards, so the loop concern that + // rule protects against does not apply, but a named writer keeps the intent + // explicit and gives a single place to extend (e.g. telemetry) later. + const recordConnectError = useCallback((message: string) => { + setConnectErrorMessage(message); + }, []); + // CRUD-modal state. `configModal` drives Add / Edit / Clone via a single // shared form modal; `removeTarget` drives the remove-confirmation modal. const [configModal, setConfigModal] = useState<{ @@ -559,6 +592,19 @@ function App() { const invocation = await inspectorClient.readResource(uri); return invocation.result; }, + // The bridge's sandboxready handler reads + posts the UI resource + // inside a detached async block; without this hook a 404 / malformed + // resource is console.error-only and the user stares at a blank + // frame. Surface it as a toast. The renderer separately drives + // `data-app-status` so an automated driver can time out on + // never-reaching-"ready" and read the toast via screenshot. + onResourceError: (err) => { + notifications.show({ + title: "App resource failed to load", + message: err.message, + color: "red", + }); + }, }), [inspectorClient], ); @@ -635,6 +681,22 @@ function App() { // render; this ref ensures the token exchange fires exactly once per load. const oauthCallbackHandledRef = useRef(false); + // Deep-link parameters parsed once from the initial URL. Security gating + // (auth-token match, http(s)-only serverUrl) happens inside `parseDeepLink`, + // so a `DeepLink` value here is already validated. The parse status is + // surfaced as `data-deeplink` so an automated driver can tell "no deep link" + // from "deep link present but rejected" — both leave `data-status` idle. + const [deepLink, deepLinkStatus] = useMemo< + [DeepLink | undefined, DeepLinkParseStatus] + >(() => { + if (typeof window === "undefined") return [undefined, "none"]; + const search = window.location.search; + const parsed = parseDeepLink(search, getAuthToken()); + return [parsed, deepLinkParseStatus(search, parsed)]; + }, []); + const deepLinkEnsureRef = useRef(false); + const deepLinkConnectRef = useRef(false); + // Tracks which progress streams currently have a live toast, so each new tick // updates the existing toast instead of stacking a fresh one. Entries are // removed when their toast closes (auto-dismiss or user). A ref (not state) @@ -1376,8 +1438,9 @@ function App() { // `onToggleConnection` unloaded the previous one), so all React state is // reset and we recover the initiating server from sessionStorage. We wait for // `servers` to hydrate before acting; the ref guard keeps the exchange to a - // single run. The persisted PKCE verifier + DCR client info live in - // `BrowserOAuthStorage` and survive the redirect, so `completeOAuthFlow` + // single run. The persisted PKCE verifier + DCR client info live in the + // backend-backed `RemoteOAuthStorage` (`~/.mcp-inspector/storage/oauth.json`) + // and survive the redirect, so `completeOAuthFlow` // exchanges the code without needing the original in-memory state machine. useEffect(() => { if (typeof window === "undefined") return; @@ -1393,10 +1456,22 @@ function App() { // session key the pre-redirect page saved the fetch log under, so the // rebuilt client can restore those `auth` entries. Read it before the // URL is cleared below. + // + // CSRF binding: the inspector relies on PKCE (the SDK saves/reads the + // `code_verifier`, so an attacker-supplied authorization code cannot be + // exchanged without our verifier) and on `pendingId` from sessionStorage + // (only this origin can have set it, so a callback we never initiated is + // rejected below). The `state` value itself is not stored before + // redirect — `provider.state()` mints a fresh one inside the SDK call — + // so a byte-for-byte comparison is not possible here. We DO require that + // a returned `state` parses to our `{mode}:{authId}` shape; a callback + // carrying a `state` that does not parse is treated as a mismatch and + // rejected. Persisting and comparing the exact `state` value is a + // follow-up that needs a `saveState` hook on the OAuth provider. const stateParam = new URLSearchParams(window.location.search).get("state"); - const sessionId = stateParam - ? (parseOAuthState(stateParam)?.authId ?? undefined) - : undefined; + const parsedState = stateParam ? parseOAuthState(stateParam) : null; + const stateRejected = stateParam !== null && parsedState === null; + const sessionId = parsedState?.authId ?? undefined; const pendingId = window.sessionStorage.getItem(OAUTH_PENDING_SERVER_KEY) ?? undefined; window.sessionStorage.removeItem(OAUTH_PENDING_SERVER_KEY); @@ -1405,33 +1480,54 @@ function App() { // the (now single-use) authorization code through the exchange again. window.history.replaceState({}, "", "/"); - if (!params.successful) { - notifications.show({ - title: "OAuth authorization failed", - message: generateOAuthErrorDescription(params), - color: "red", - }); - return; - } + // The remainder runs inside an async block so error reporting (which + // writes to React state for the `data-error-message` attribute) is + // deferred past the synchronous effect body — the lint rule against + // setState-in-effect is correct in general, but this is a ref-guarded + // run-once effect. + void (async () => { + if (stateRejected) { + const message = + "OAuth callback carried an unrecognized state parameter; rejecting to prevent a cross-site request. Please try connecting again."; + recordConnectError(message); + notifications.show({ + title: "OAuth callback rejected", + message, + color: "red", + }); + return; + } - // By design, the pending id and URL are cleared above before this lookup: - // if the server was deleted/renamed (e.g. in another tab) mid-flow, there's - // nothing to resume against, so we surface the error and require a fresh - // Connect rather than leaving stale callback state lying around. - const server = pendingId - ? servers.find((s) => s.id === pendingId) - : undefined; - if (!server) { - notifications.show({ - title: "OAuth callback could not be matched", - message: - "Could not determine which server started the OAuth flow. Please try connecting again.", - color: "red", - }); - return; - } + if (!params.successful) { + const message = generateOAuthErrorDescription(params); + recordConnectError(message); + notifications.show({ + title: "OAuth authorization failed", + message, + color: "red", + }); + return; + } + + // By design, the pending id and URL are cleared above before this lookup: + // if the server was deleted/renamed (e.g. in another tab) mid-flow, + // there's nothing to resume against, so we surface the error and require + // a fresh Connect rather than leaving stale callback state lying around. + const server = pendingId + ? servers.find((s) => s.id === pendingId) + : undefined; + if (!server) { + const message = + "Could not determine which server started the OAuth flow. Please try connecting again."; + recordConnectError(message); + notifications.show({ + title: "OAuth callback could not be matched", + message, + color: "red", + }); + return; + } - void (async () => { const client = setupClientForServer(server, sessionId); setActiveServerId(server.id); // Two distinct failure modes get distinct toasts. A token-exchange @@ -1446,6 +1542,7 @@ function App() { await client.completeOAuthFlow(params.code); } catch (err) { const message = err instanceof Error ? err.message : String(err); + recordConnectError(message); notifications.show({ title: `OAuth token exchange failed for "${server.name}"`, message: `${message}\n\nPlease try connecting again.`, @@ -1459,6 +1556,7 @@ function App() { } catch (err) { connectStartRef.current = undefined; const message = err instanceof Error ? err.message : String(err); + recordConnectError(message); notifications.show({ title: `Failed to connect to "${server.name}"`, message, @@ -1466,7 +1564,7 @@ function App() { }); } })(); - }, [servers, setupClientForServer]); + }, [servers, setupClientForServer, recordConnectError]); const onToggleConnection = useCallback( async (id: string) => { @@ -1480,9 +1578,15 @@ function App() { return; } - const target = servers.find((s) => s.id === id); + // Read from the ref so a caller that already awaited an + // addServer/updateServer in the same async tick (e.g. the deep-link + // auto-connect IIFE) sees the freshly-mutated list, not the stale array + // captured by this callback's closure. + const target = serversRef.current.find((s) => s.id === id); if (!target) return; + setConnectErrorMessage(undefined); + // Always rebuild the InspectorClient on a (re)connect so the latest // `target.settings` (headers, metadata, timeouts, OAuth credentials) // are picked up. Reusing the previous client object would freeze the @@ -1523,6 +1627,7 @@ function App() { window.sessionStorage.removeItem(OAUTH_PENDING_SERVER_KEY); const message = authErr instanceof Error ? authErr.message : String(authErr); + setConnectErrorMessage(message); notifications.show({ title: `OAuth authorization failed for "${target.name}"`, message, @@ -1536,6 +1641,7 @@ function App() { // instead of the ConnectionToggle silently reverting to // "disconnected". const message = err instanceof Error ? err.message : String(err); + setConnectErrorMessage(message); notifications.show({ title: `Failed to connect to "${target.name}"`, message, @@ -1543,13 +1649,7 @@ function App() { }); } }, - [ - activeServerId, - connectionStatus, - inspectorClient, - servers, - setupClientForServer, - ], + [activeServerId, connectionStatus, inspectorClient, setupClientForServer], ); const onDisconnect = useCallback(async () => { @@ -1557,6 +1657,60 @@ function App() { await inspectorClient.disconnect(); }, [inspectorClient]); + // Deep-link auto-connect (closes #1183 for the URL-driven case). Split into + // `useServers` hydrates asynchronously (initial `servers` is `[]`), so this + // effect runs in two phases: first render attempts a one-shot `addServer` + // (a 409 means the row already exists on disk and hydration will surface it); + // a later render where `servers` actually contains the deep-link row then + // updates the URL if it has changed and connects. Connecting in the same + // closure as `addServer` would resolve against a stale (empty) `servers` + // and silently no-op. The OAuth callback path takes precedence; a deep link + // on `/oauth/callback` would be a misconfiguration, and the callback handler + // clears the URL. + useEffect(() => { + if (!deepLink) return; + if (window.location.pathname === OAUTH_CALLBACK_PATH) return; + + const existing = servers.find((s) => s.id === deepLink.serverId); + if (!existing) { + if (deepLinkEnsureRef.current) return; + deepLinkEnsureRef.current = true; + void addServer(deepLink.serverId, deepLink.serverConfig).catch(() => { + // 409 = already on disk; hydration will surface it on a later render. + }); + return; + } + + if (deepLinkConnectRef.current) return; + deepLinkConnectRef.current = true; + void (async () => { + if (!deepLinkConfigEquals(existing.config, deepLink.serverConfig)) { + await updateServer( + deepLink.serverId, + deepLink.serverId, + deepLink.serverConfig, + ); + } + if (activeServerId !== deepLink.serverId) { + await onToggleConnection(deepLink.serverId); + } + })().catch((err) => { + // Surface deep-link automation failures (updateServer 5xx, connect + // throw) on the machine-readable error attribute instead of dropping + // them. The toast still fires from inside `onToggleConnection` for the + // common cases; this catch covers the rest. + const message = err instanceof Error ? err.message : String(err); + setConnectErrorMessage(message); + }); + }, [ + deepLink, + servers, + activeServerId, + addServer, + updateServer, + onToggleConnection, + ]); + // --- Action handlers that route directly to the InspectorClient. --- const onCallTool = useCallback( @@ -2358,10 +2512,13 @@ function App() { return ( <> ; + sendToolInputPartial: ReturnType; sendToolResult: ReturnType; sendToolCancelled: ReturnType; + setHostContext: ReturnType; + sendHostContextChange: ReturnType; teardownResource: ReturnType; close: ReturnType; addEventListener: ReturnType; removeEventListener: ReturnType; + onrequestdisplaymode?: (params: { + mode: "inline" | "fullscreen" | "pip"; + }) => Promise<{ mode: "inline" | "fullscreen" | "pip" }>; /** Test helper: dispatch a bridge event (e.g. "initialized") to listeners. */ emit: (event: string, payload?: unknown) => void; } @@ -32,8 +38,11 @@ function createMockBridge(): MockBridge { const listeners: Record void)[]> = {}; return { sendToolInput: vi.fn().mockResolvedValue(undefined), + sendToolInputPartial: vi.fn().mockResolvedValue(undefined), sendToolResult: vi.fn().mockResolvedValue(undefined), sendToolCancelled: vi.fn().mockResolvedValue(undefined), + setHostContext: vi.fn(), + sendHostContextChange: vi.fn().mockResolvedValue(undefined), teardownResource: vi.fn().mockResolvedValue({}), close: vi.fn().mockResolvedValue(undefined), addEventListener: vi.fn((event: string, handler: (p: unknown) => void) => { @@ -210,6 +219,155 @@ describe("AppRenderer", () => { }); }); + it("replays partialInputs in order before the complete tool-input on initialize", async () => { + const bridge = createMockBridge(); + const ref = createRef(); + renderWithMantine( + asBridge(bridge)} + partialInputs={[{ city: "N" }, { city: "New" }]} + />, + ); + await flushAsync(); + await act(async () => { + await ref.current?.sendToolInput({ city: "New York" }); + bridge.emit("initialized"); + }); + expect(bridge.sendToolInputPartial).toHaveBeenCalledTimes(2); + expect(bridge.sendToolInputPartial).toHaveBeenNthCalledWith(1, { + arguments: { city: "N" }, + }); + expect(bridge.sendToolInputPartial).toHaveBeenNthCalledWith(2, { + arguments: { city: "New" }, + }); + expect( + bridge.sendToolInputPartial.mock.invocationCallOrder[1], + ).toBeLessThan(bridge.sendToolInput.mock.invocationCallOrder[0]); + }); + + it("sends no tool-input-partial when partialInputs is omitted", async () => { + const bridge = createMockBridge(); + const ref = createRef(); + renderWithMantine( + asBridge(bridge)} + />, + ); + await flushAsync(); + await act(async () => { + await ref.current?.sendToolInput({ city: "NYC" }); + bridge.emit("initialized"); + }); + expect(bridge.sendToolInputPartial).not.toHaveBeenCalled(); + }); + + it("forwards view size-changed notifications to onSizeChange", async () => { + const bridge = createMockBridge(); + const onSizeChange = vi.fn(); + renderWithMantine( + asBridge(bridge)} + onSizeChange={onSizeChange} + />, + ); + await flushAsync(); + await act(async () => { + bridge.emit("sizechange", { width: 480, height: 600 }); + }); + expect(onSizeChange).toHaveBeenCalledWith({ width: 480, height: 600 }); + }); + + it("does not throw on size-changed when no onSizeChange is provided", async () => { + const bridge = createMockBridge(); + renderWithMantine( + asBridge(bridge)} + />, + ); + await flushAsync(); + await act(async () => { + bridge.emit("sizechange", { height: 320 }); + }); + expect(screen.getByTitle("Cohort App")).toBeInTheDocument(); + }); + + it("routes ui/request-display-mode to onRequestDisplayMode and returns the applied mode", async () => { + const bridge = createMockBridge(); + const onRequestDisplayMode = vi + .fn<(m: "inline" | "fullscreen" | "pip") => "inline" | "fullscreen">() + .mockReturnValue("fullscreen"); + renderWithMantine( + asBridge(bridge)} + displayMode="inline" + onRequestDisplayMode={onRequestDisplayMode} + />, + ); + await flushAsync(); + await expect( + bridge.onrequestdisplaymode?.({ mode: "fullscreen" }), + ).resolves.toEqual({ mode: "fullscreen" }); + expect(onRequestDisplayMode).toHaveBeenCalledWith("fullscreen"); + }); + + it("declines ui/request-display-mode by returning the current mode when no handler is provided", async () => { + const bridge = createMockBridge(); + renderWithMantine( + asBridge(bridge)} + displayMode="inline" + />, + ); + await flushAsync(); + await expect( + bridge.onrequestdisplaymode?.({ mode: "pip" }), + ).resolves.toEqual({ mode: "inline" }); + }); + + it("pushes a displayMode change to the running view via host-context-changed", async () => { + const bridge = createMockBridge(); + // Stable factory identity so the rerender reuses the live bridge instead of + // rebuilding (which would reset `initialized` and gate the push). + const factory: BridgeFactory = () => asBridge(bridge); + const { rerender } = renderWithMantine( + , + ); + await flushAsync(); + await act(async () => bridge.emit("initialized")); + bridge.sendHostContextChange.mockClear(); + rerender( + , + ); + await flushAsync(); + expect(bridge.sendHostContextChange).toHaveBeenCalledWith({ + displayMode: "fullscreen", + }); + }); + it("forwards sendToolCancelled through the bridge", async () => { const bridge = createMockBridge(); const ref = createRef(); @@ -230,6 +388,191 @@ describe("AppRenderer", () => { }); }); + // The host theme lives on ``. MantineProvider + // writes its own scheme there on mount, so these tests manipulate the + // attribute AFTER render to model a user-driven theme flip on an open app. + it("pushes a live theme flip to the running bridge via host-context-changed", async () => { + const bridge = createMockBridge(); + renderWithMantine( + asBridge(bridge)} + />, + ); + await flushAsync(); + // Ignore any seeding from Mantine's own mount-time write — assert only the + // flip we trigger below. + bridge.sendHostContextChange.mockClear(); + + await act(async () => { + document.documentElement.setAttribute( + "data-mantine-color-scheme", + "dark", + ); + // MutationObserver callbacks are delivered on a microtask. + await Promise.resolve(); + }); + expect(bridge.sendHostContextChange).toHaveBeenCalledWith( + expect.objectContaining({ theme: "dark" }), + ); + }); + + it("stops observing theme changes after the renderer unmounts", async () => { + const bridge = createMockBridge(); + const { unmount } = renderWithMantine( + asBridge(bridge)} + />, + ); + await flushAsync(); + await act(async () => { + unmount(); + await Promise.resolve(); + await Promise.resolve(); + }); + bridge.sendHostContextChange.mockClear(); + + await act(async () => { + document.documentElement.setAttribute( + "data-mantine-color-scheme", + "dark", + ); + await Promise.resolve(); + }); + expect(bridge.sendHostContextChange).not.toHaveBeenCalled(); + }); + + describe("containerDimensions ResizeObserver", () => { + // Stub ResizeObserver so tests can drive the callback directly: capture + // the callback per observed element + the constructed instance so an + // observed iframe's getBoundingClientRect can be patched before each fire. + let resizeCallback: (() => void) | undefined; + let observedIframe: HTMLIFrameElement | undefined; + let originalResizeObserver: typeof ResizeObserver | undefined; + + beforeEach(() => { + resizeCallback = undefined; + observedIframe = undefined; + originalResizeObserver = globalThis.ResizeObserver; + globalThis.ResizeObserver = class { + constructor(cb: () => void) { + resizeCallback = cb; + } + observe(el: Element) { + observedIframe = el as HTMLIFrameElement; + } + unobserve() {} + disconnect() { + resizeCallback = undefined; + } + } as unknown as typeof ResizeObserver; + }); + afterEach(() => { + if (originalResizeObserver) { + globalThis.ResizeObserver = originalResizeObserver; + } else { + delete (globalThis as { ResizeObserver?: unknown }).ResizeObserver; + } + }); + + function setObservedSize(width: number, height: number) { + if (!observedIframe) throw new Error("no iframe observed"); + vi.spyOn(observedIframe, "getBoundingClientRect").mockReturnValue({ + width, + height, + top: 0, + left: 0, + right: width, + bottom: height, + x: 0, + y: 0, + toJSON: () => ({}), + } as DOMRect); + } + + it("does not push containerDimensions before the view is initialized", async () => { + const bridge = createMockBridge(); + renderWithMantine( + asBridge(bridge)} + />, + ); + await flushAsync(); + bridge.sendHostContextChange.mockClear(); + setObservedSize(640, 480); + await act(async () => resizeCallback?.()); + expect(bridge.sendHostContextChange).not.toHaveBeenCalled(); + }); + + it("pushes containerDimensions on resize once initialized; skips a 0×0 box and a value-equal repeat", async () => { + const bridge = createMockBridge(); + renderWithMantine( + asBridge(bridge)} + />, + ); + await flushAsync(); + await act(async () => bridge.emit("initialized")); + bridge.sendHostContextChange.mockClear(); + + setObservedSize(640, 480); + await act(async () => resizeCallback?.()); + expect(bridge.sendHostContextChange).toHaveBeenCalledWith({ + containerDimensions: { width: 640, height: 480 }, + }); + + bridge.sendHostContextChange.mockClear(); + setObservedSize(640, 480); + await act(async () => resizeCallback?.()); + expect(bridge.sendHostContextChange).not.toHaveBeenCalled(); + + setObservedSize(0, 0); + await act(async () => resizeCallback?.()); + expect(bridge.sendHostContextChange).not.toHaveBeenCalled(); + }); + + it("observes the host-supplied containerRef element instead of the iframe when provided", async () => { + const bridge = createMockBridge(); + const container = document.createElement("div"); + renderWithMantine( + asBridge(bridge)} + containerRef={{ current: container }} + />, + ); + await flushAsync(); + expect(observedIframe).toBe(container); + }); + + it("disconnects the ResizeObserver on unmount", async () => { + const bridge = createMockBridge(); + const { unmount } = renderWithMantine( + asBridge(bridge)} + />, + ); + await flushAsync(); + await act(async () => bridge.emit("initialized")); + await act(async () => { + unmount(); + await Promise.resolve(); + await Promise.resolve(); + }); + expect(resizeCallback).toBeUndefined(); + }); + }); + it("builds a single bridge and does not dispose it under StrictMode double-invoke", async () => { // React StrictMode runs effects setup→cleanup→setup in dev. The bridge // (a stateful handshake) must survive that as ONE instance — rebuilding it @@ -450,6 +793,94 @@ describe("AppRenderer", () => { expect(screen.getByTitle("Cohort App")).toBeInTheDocument(); }); + describe("onAppStatusChange", () => { + it("fires loading on build, then ready when the view signals initialized", async () => { + const bridge = createMockBridge(); + const onAppStatusChange = vi.fn(); + renderWithMantine( + asBridge(bridge)} + onAppStatusChange={onAppStatusChange} + />, + ); + await flushAsync(); + expect(onAppStatusChange).toHaveBeenCalledWith("loading"); + expect(onAppStatusChange).not.toHaveBeenCalledWith("ready"); + await act(async () => bridge.emit("initialized")); + expect(onAppStatusChange).toHaveBeenLastCalledWith("ready"); + expect(onAppStatusChange).not.toHaveBeenCalledWith("error"); + }); + + it("fires loading then error when the bridge factory throws synchronously", async () => { + const onAppStatusChange = vi.fn(); + const factory: BridgeFactory = () => { + throw new Error("boom"); + }; + renderWithMantine( + , + ); + await flushAsync(); + expect(onAppStatusChange.mock.calls).toEqual([["loading"], ["error"]]); + }); + + it("fires loading then error when the bridge factory rejects", async () => { + const onAppStatusChange = vi.fn(); + const factory: BridgeFactory = () => Promise.reject(new Error("nope")); + renderWithMantine( + , + ); + await flushAsync(); + expect(onAppStatusChange.mock.calls).toEqual([["loading"], ["error"]]); + }); + + it("fires loading again on a real rebuild and not on the StrictMode reuse path", async () => { + const bridge = createMockBridge(); + const onAppStatusChange = vi.fn(); + const { rerender } = renderWithMantine( + + asBridge(bridge)} + onAppStatusChange={onAppStatusChange} + /> + , + ); + await flushAsync(); + // StrictMode's setup→cleanup→setup reuses the bridge; only ONE loading. + expect( + onAppStatusChange.mock.calls.filter(([s]) => s === "loading"), + ).toHaveLength(1); + await act(async () => bridge.emit("initialized")); + onAppStatusChange.mockClear(); + // Changing an input forces a real rebuild → loading fires again. + rerender( + + asBridge(bridge)} + onAppStatusChange={onAppStatusChange} + /> + , + ); + await flushAsync(); + expect(onAppStatusChange).toHaveBeenCalledWith("loading"); + }); + }); + it("rebuilds the bridge when sandboxPath changes", async () => { const first = createMockBridge(); const second = createMockBridge(); diff --git a/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx b/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx index da23751ff..5d7dfea59 100644 --- a/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx +++ b/clients/web/src/components/elements/AppRenderer/AppRenderer.tsx @@ -5,9 +5,24 @@ import { useImperativeHandle, useRef, type Ref, + type RefObject, } from "react"; -import type { AppBridge } from "@modelcontextprotocol/ext-apps/app-bridge"; -import type { CallToolResult, Tool } from "@modelcontextprotocol/sdk/types.js"; +import type { + AppBridge, + AppBridgeEventMap, + McpUiDisplayMode, + McpUiMessageRequest, +} from "@modelcontextprotocol/ext-apps/app-bridge"; +import type { + CallToolResult, + LoggingMessageNotification, + Tool, +} from "@modelcontextprotocol/sdk/types.js"; +import { + currentStyles, + currentTheme, + measureContainerDimensions, +} from "./hostContext"; /** * Constructs the `AppBridge` for a freshly mounted sandbox iframe. Wrap with @@ -27,11 +42,74 @@ export interface AppRendererHandle { teardown(): Promise; } +/** + * High-level lifecycle of a running app, surfaced so a host (or an automated + * driver polling a `data-app-status` attribute) can wait for the right moment: + * `loading` while the bridge is being built and the view's `ui/initialize` + * handshake is in flight; `ready` once the view has fired + * `notifications/initialized`; `error` when the bridge factory throws or + * rejects (no live view to wait on). + */ +export type AppRendererStatus = "loading" | "ready" | "error"; + export interface AppRendererProps { sandboxPath: string; tool: Tool; bridgeFactory: BridgeFactory; onError?: (err: Error) => void; + /** + * Reports the renderer's high-level lifecycle (see {@link AppRendererStatus}). + * Fires `loading` at the start of every (re)build, `ready` when the view + * signals `initialized`, and `error` on a factory throw/rejection. + */ + onAppStatusChange?: (status: AppRendererStatus) => void; + /** + * Called when the running view reports a new rendered content size via + * `ui/notifications/size-changed` (typically driven by its `ResizeObserver`). + * Width and height (px) are both optional. The host uses this to resize the + * iframe's container so the widget is neither clipped nor padded with dead + * space. + */ + onSizeChange?: (size: AppBridgeEventMap["sizechange"]) => void; + /** + * Current host display mode for the app frame. Pushed to the running view + * via `host-context-changed` whenever it changes (e.g. Maximize/Restore), so + * an app can adapt its layout to inline vs fullscreen. + */ + displayMode?: McpUiDisplayMode; + /** + * Handles a view-originated `ui/request-display-mode`. Return the mode the + * host actually applied — the spec lets the host decline an unsupported mode + * by returning its current one. + */ + onRequestDisplayMode?: (requested: McpUiDisplayMode) => McpUiDisplayMode; + /** + * Called when the running view submits a user-role message via + * `ui/message`. The renderer returns the spec-required empty result on + * the host's behalf, so the callback is fire-and-forget. + */ + onMessage?: (params: McpUiMessageRequest["params"]) => void; + /** + * Called for each MCP log notification (`notifications/message`) the + * running view emits. Backs the advertised `logging` host capability. + */ + onLog?: (params: LoggingMessageNotification["params"]) => void; + /** + * Ordered tool-input fragments to replay via + * `ui/notifications/tool-input-partial` BEFORE the complete `tool-input`, + * exercising widgets that render progressively. Captured at bridge-build + * time (see `pendingPartialsRef`) so prop churn never rebuilds the iframe. + * Nothing is sent when omitted/empty. + */ + partialInputs?: Record[]; + /** + * The host-controlled box the app renders within, used to derive + * `hostContext.containerDimensions`. This MUST be an element whose size is + * driven by the host's layout (window resize, sidebar toggle, maximize) and + * NOT by the view's own `size-changed` reports — otherwise the two signals + * couple into a feedback loop. Falls back to the iframe element when omitted. + */ + containerRef?: RefObject; ref?: Ref; } @@ -79,11 +157,20 @@ export function AppRenderer({ tool, bridgeFactory, onError, + onAppStatusChange, + onSizeChange, + displayMode, + onRequestDisplayMode, + onMessage, + onLog, + partialInputs, + containerRef, ref, }: AppRendererProps) { const iframeRef = useRef(null); const bridgeRef = useRef(null); const initializedRef = useRef(false); + const pendingPartialsRef = useRef[]>([]); const pendingInputRef = useRef | null>(null); const pendingResultRef = useRef(null); const teardownStartedRef = useRef(false); @@ -98,8 +185,22 @@ export function AppRenderer({ tool: Tool; } | null>(null); const onErrorRef = useRef(onError); + const onAppStatusChangeRef = useRef(onAppStatusChange); + const onSizeChangeRef = useRef(onSizeChange); + const displayModeRef = useRef(displayMode); + const onRequestDisplayModeRef = useRef(onRequestDisplayMode); + const onMessageRef = useRef(onMessage); + const onLogRef = useRef(onLog); + const partialInputsRef = useRef(partialInputs); useEffect(() => { onErrorRef.current = onError; + onAppStatusChangeRef.current = onAppStatusChange; + onSizeChangeRef.current = onSizeChange; + displayModeRef.current = displayMode; + onRequestDisplayModeRef.current = onRequestDisplayMode; + onMessageRef.current = onMessage; + onLogRef.current = onLog; + partialInputsRef.current = partialInputs; }); // Flush buffered tool input/result to the view, but only once the bridge @@ -111,6 +212,12 @@ export function AppRenderer({ const flushPending = useCallback(() => { const bridge = bridgeRef.current; if (!bridge || !initializedRef.current) return; + // Partial-input fragments first, in staged order, BEFORE the complete + // tool-input — the spec requires partials to precede the final input. + for (const args of pendingPartialsRef.current) { + void bridge.sendToolInputPartial({ arguments: args }); + } + pendingPartialsRef.current = []; if (pendingInputRef.current !== null) { const args = pendingInputRef.current; pendingInputRef.current = null; @@ -142,6 +249,7 @@ export function AppRenderer({ bridgeRef.current = null; initializedRef.current = false; lastDepsRef.current = null; + pendingPartialsRef.current = []; if (bridge) void disposeBridge(bridge); }); }, []); @@ -182,11 +290,18 @@ export function AppRenderer({ const buildId = ++buildIdRef.current; teardownStartedRef.current = false; initializedRef.current = false; + onAppStatusChangeRef.current?.("loading"); + // Snapshot the staged partial-input fragments for THIS bridge build (read + // via the ref so the prop is not a dep — adding/removing fragments must not + // rebuild the iframe). The StrictMode reuse path above returned before + // reaching here, so a reused bridge keeps the queue it was built with. + pendingPartialsRef.current = [...(partialInputsRef.current ?? [])]; let pending: Promise; try { pending = Promise.resolve(bridgeFactory(iframe, tool)); } catch (err) { + onAppStatusChangeRef.current?.("error"); onErrorRef.current?.(toError(err)); return scheduleDispose; } @@ -203,16 +318,135 @@ export function AppRenderer({ // drives), so the view's `initialized` signal is never missed. bridge.addEventListener("initialized", () => { initializedRef.current = true; + onAppStatusChangeRef.current?.("ready"); + // The factory already seeded theme/styles/displayMode into the + // handshake hostContext; the observers below cover any subsequent + // changes. Only containerDimensions can plausibly differ between + // bridge construction and initialization (layout settles), so push + // that one field now via the SDK's partial-change notification. + const container = containerRef?.current ?? iframeRef.current; + const containerDimensions = container + ? measureContainerDimensions(container) + : undefined; + if (containerDimensions) { + void bridge.sendHostContextChange({ containerDimensions }); + } flushPending(); }); + // Forward the view's content-size reports (ui/notifications/size-changed) + // so the host can resize the iframe container to fit the rendered widget. + bridge.addEventListener("sizechange", (size) => { + onSizeChangeRef.current?.(size); + }); + // Forward the view's MCP log notifications so the host can honor the + // advertised `logging` capability instead of dropping them. + bridge.addEventListener("loggingmessage", (params) => { + onLogRef.current?.(params); + }); + // Handle ui/request-display-mode: let the host (AppsScreen) decide what + // mode to actually apply and return that. With no handler the request is + // declined by returning the current host-side mode. + bridge.onrequestdisplaymode = async ({ mode }) => { + const handler = onRequestDisplayModeRef.current; + const applied = handler + ? handler(mode) + : (displayModeRef.current ?? "inline"); + return { mode: applied }; + }; + // Handle ui/message: surface the submitted content and return the + // spec-required empty result (no conversation content) to avoid + // information leakage. With no handler the submission is declined. + bridge.onmessage = async (params) => { + const handler = onMessageRef.current; + if (!handler) return { isError: true }; + handler(params); + return {}; + }; flushPending(); }) .catch((err) => { - if (buildIdRef.current === buildId) onErrorRef.current?.(toError(err)); + if (buildIdRef.current !== buildId) return; + onAppStatusChangeRef.current?.("error"); + onErrorRef.current?.(toError(err)); }); return scheduleDispose; - }, [bridgeFactory, sandboxPath, tool, flushPending, scheduleDispose]); + }, [ + bridgeFactory, + sandboxPath, + tool, + containerRef, + flushPending, + scheduleDispose, + ]); + + // Push live host-context changes to the running view as discrete partial + // updates via AppBridge.sendHostContextChange (the SDK's + // ui/notifications/host-context-changed sender). Each effect observes one + // host signal and sends only the field(s) it owns, so the view receives the + // spec's "only changed fields" partials without any host-side snapshot + // bookkeeping. Reading `bridgeRef.current` at callback time (not capturing a + // bridge) means the observers always target the live bridge, even though it + // resolves asynchronously after these effects run. + + // Theme + styles: Mantine writes the resolved scheme to + // ``; observe that attribute and forward + // changes through the live bridge. + useEffect(() => { + if ( + typeof MutationObserver === "undefined" || + typeof document === "undefined" + ) { + return; + } + const observer = new MutationObserver(() => { + const styles = currentStyles(); + void bridgeRef.current?.sendHostContextChange({ + theme: currentTheme(), + ...(styles ? { styles } : {}), + }); + }); + observer.observe(document.documentElement, { + attributes: true, + attributeFilter: ["data-mantine-color-scheme"], + }); + return () => observer.disconnect(); + }, []); + + // Container size: observes the host-controlled container (or the iframe as a + // fallback) — NOT an element whose height is driven by the view's own + // size-changed reports, which would couple the two signals into a feedback + // loop. Gated on the view's `initialized` signal so the notification only + // fires once the handshake is complete; a 0×0 (not-yet-laid-out) measurement + // and a value-equal repeat are both skipped. + useEffect(() => { + const target = containerRef?.current ?? iframeRef.current; + if (typeof ResizeObserver === "undefined" || !target) return; + let last: { width: number; height: number } | undefined; + const observer = new ResizeObserver(() => { + if (!initializedRef.current) return; + const next = measureContainerDimensions(target); + if (!next) return; + if (last && last.width === next.width && last.height === next.height) { + return; + } + last = next; + void bridgeRef.current?.sendHostContextChange({ + containerDimensions: next, + }); + }); + observer.observe(target); + return () => observer.disconnect(); + }, [containerRef]); + + // Display mode: pushes whenever the prop changes (Maximize/Restore, or after + // we honored a ui/request-display-mode). Gated on `initialized` for the same + // reason as the other host-context pushes. + useEffect(() => { + if (displayMode === undefined) return; + if (!initializedRef.current) return; + void bridgeRef.current?.sendHostContextChange({ displayMode }); + }, [displayMode]); useImperativeHandle( ref, diff --git a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts index 54dd6020a..8f211ea12 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.test.ts @@ -6,7 +6,7 @@ * end-to-end iframe round-trip is covered by the AppsScreen Storybook play test. */ -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import type { Tool, ReadResourceResult, @@ -23,6 +23,9 @@ interface MockBridge { sendSandboxResourceReady: ReturnType; connect: ReturnType; onopenlink?: (params: { url: string }) => Promise<{ isError?: boolean }>; + ondownloadfile?: (params: { + contents: unknown[]; + }) => Promise<{ isError?: boolean }>; emit: (event: string, payload?: unknown) => void; } @@ -36,6 +39,9 @@ vi.mock("@modelcontextprotocol/ext-apps/app-bridge", () => { sendSandboxResourceReady = vi.fn().mockResolvedValue(undefined); connect = vi.fn().mockResolvedValue(undefined); onopenlink?: (params: { url: string }) => Promise<{ isError?: boolean }>; + ondownloadfile?: (params: { + contents: unknown[]; + }) => Promise<{ isError?: boolean }>; emit = (event: string, payload?: unknown) => { (this.listeners[event] ?? []).forEach((h) => h(payload)); }; @@ -62,6 +68,7 @@ vi.mock("@modelcontextprotocol/ext-apps/app-bridge", () => { }); import { createAppBridgeFactory } from "./createAppBridgeFactory"; +import { measureContainerDimensions } from "./hostContext"; const tool: Tool = { name: "weather_app", @@ -133,19 +140,77 @@ describe("createAppBridgeFactory", () => { const bridge = bridgeInstances[0]; expect(bridge.ctorArgs[0]).toBe(fakeClient); expect(bridge.ctorArgs[1]).toMatchObject({ name: "MCP Inspector" }); - expect(bridge.ctorArgs[2]).toMatchObject({ serverTools: {} }); - expect(bridge.ctorArgs[3]).toEqual({ hostContext: { theme: "dark" } }); + expect(bridge.ctorArgs[2]).toMatchObject({ + serverTools: {}, + downloadFile: {}, + }); + expect(bridge.ctorArgs[3]).toMatchObject({ + hostContext: { + theme: "dark", + displayMode: "inline", + availableDisplayModes: ["inline", "fullscreen"], + }, + }); expect(bridge.connect).toHaveBeenCalledTimes(1); } finally { document.documentElement.removeAttribute("data-mantine-color-scheme"); } }); - it("on sandboxready, reads the UI resource and pushes html + meta to the sandbox", async () => { + it("seeds hostContext.styles from the resolved host design tokens", async () => { + // The host resolves spec style keys from its own CSS variables via + // getComputedStyle. Stub it so the mapped variables resolve to values + // (happy-dom returns "" for custom properties otherwise). + const resolved: Record = { + "--mantine-color-body": "#1a1b1e", + "--mantine-color-text": "#c1c2c5", + "--mantine-font-family": "Inter, sans-serif", + "--mantine-radius-md": "0.5rem", + }; + const getComputedStyle = vi + .spyOn(window, "getComputedStyle") + .mockReturnValue({ + getPropertyValue: (prop: string) => resolved[prop] ?? "", + } as unknown as CSSStyleDeclaration); + try { + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource: vi.fn().mockResolvedValue(uiResource("

hi

")), + }); + await factory(makeIframe(), tool); + const options = bridgeInstances[0].ctorArgs[3] as { + hostContext?: { styles?: { variables?: Record } }; + }; + expect(options.hostContext?.styles?.variables).toEqual({ + "--color-background-primary": "#1a1b1e", + "--color-text-primary": "#c1c2c5", + "--font-sans": "Inter, sans-serif", + "--border-radius-md": "0.5rem", + }); + } finally { + getComputedStyle.mockRestore(); + } + }); + + it("omits hostContext.styles when no host token resolves", async () => { + // happy-dom resolves custom properties to "" — nothing maps, so the host + // advertises no styles object rather than an empty one. + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource: vi.fn().mockResolvedValue(uiResource("

hi

")), + }); + await factory(makeIframe(), tool); + const options = bridgeInstances[0].ctorArgs[3] as { + hostContext?: { styles?: unknown }; + }; + expect(options.hostContext?.styles).toBeUndefined(); + }); + + it("on sandboxready, reads the UI resource, wraps it with the host CSP shell, and pushes it to the sandbox", async () => { const readResource = vi.fn().mockResolvedValue( uiResource("

weather

", { permissions: { geolocation: {} }, - csp: { connectSrc: ["https://api.example.com"] }, + csp: { connectDomains: ["https://api.example.com"] }, }), ); const factory = createAppBridgeFactory({ @@ -159,11 +224,141 @@ describe("createAppBridgeFactory", () => { await flush(); expect(readResource).toHaveBeenCalledWith("ui://weather/app.html"); - expect(bridge.sendSandboxResourceReady).toHaveBeenCalledWith({ - html: "

weather

", - permissions: { geolocation: {} }, - csp: { connectSrc: ["https://api.example.com"] }, + expect(bridge.sendSandboxResourceReady).toHaveBeenCalledOnce(); + const sent = bridge.sendSandboxResourceReady.mock.calls[0][0] as { + html: string; + permissions: unknown; + }; + expect(sent.permissions).toEqual({ geolocation: {} }); + // The host wraps the app HTML in a fixed shell whose first child is + // the CSP meta — the proxy writes this verbatim. + expect(sent.html.startsWith("

weather

"); + }); + + it("filters unsafe csp sources before wrapping and echoes only the approved ones", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const readResource = vi.fn().mockResolvedValue( + uiResource("

app

", { + csp: { + connectDomains: ["https://ok.com", "https://x.com; script-src *"], + }, + }), + ); + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource, + }); + await factory(makeIframe(), tool); + const bridge = bridgeInstances[0]; + + bridge.emit("sandboxready"); + await flush(); + + const sent = bridge.sendSandboxResourceReady.mock.calls[0][0] as { + html: string; + }; + expect(sent.html).toContain("connect-src https://ok.com"); + expect(sent.html).not.toContain("script-src *"); + expect(bridge.ctorArgs[2]).toMatchObject({ + sandbox: { csp: { connectDomains: ["https://ok.com"] } }, + }); + warn.mockRestore(); + }); + + it("echoes the approved csp + permissions in hostCapabilities after sandboxready", async () => { + const readResource = vi.fn().mockResolvedValue( + uiResource("

weather

", { + permissions: { geolocation: {} }, + csp: { connectDomains: ["https://api.example.com"] }, + }), + ); + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource, + }); + await factory(makeIframe(), tool); + const bridge = bridgeInstances[0]; + + bridge.emit("sandboxready"); + await flush(); + + // The capabilities object passed to the bridge constructor is mutated in + // place, so the ui/initialize handshake (which reads it lazily) echoes the + // approved sandbox config back to the view. + expect(bridge.ctorArgs[2]).toMatchObject({ + sandbox: { + permissions: { geolocation: {} }, + csp: { connectDomains: ["https://api.example.com"] }, + }, + }); + }); + + it("advertises an empty csp object when the resource declares no csp", async () => { + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource: vi.fn().mockResolvedValue(uiResource("

hi

")), + }); + await factory(makeIframe(), tool); + const bridge = bridgeInstances[0]; + + bridge.emit("sandboxready"); + await flush(); + + const sandbox = ( + bridge.ctorArgs[2] as { + sandbox?: { csp?: unknown; permissions?: unknown }; + } + ).sandbox; + expect(sandbox?.csp).toEqual({}); + expect(sandbox?.permissions).toBeUndefined(); + }); + + it("does not echo a sandbox capability before sandboxready or on read failure", async () => { + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource: vi.fn().mockRejectedValue(new Error("read boom")), + }); + await factory(makeIframe(), tool); + const bridge = bridgeInstances[0]; + + // Nothing read yet → no sandbox echo. + expect( + (bridge.ctorArgs[2] as { sandbox?: unknown }).sandbox, + ).toBeUndefined(); + + bridge.emit("sandboxready"); + await flush(); + + // Read failed → still no sandbox echo. + expect( + (bridge.ctorArgs[2] as { sandbox?: unknown }).sandbox, + ).toBeUndefined(); + errSpy.mockRestore(); + }); + + it("logs and reports a UI-resource read failure via onResourceError", async () => { + const errSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const onResourceError = vi.fn(); + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource: vi.fn().mockRejectedValue(new Error("read boom")), + onResourceError, }); + await factory(makeIframe(), tool); + bridgeInstances[0].emit("sandboxready"); + await flush(); + expect(errSpy).toHaveBeenCalledWith( + expect.stringContaining("failed to load UI resource"), + expect.any(Error), + ); + expect(onResourceError).toHaveBeenCalledWith( + expect.objectContaining({ message: "read boom" }), + ); + errSpy.mockRestore(); }); it("does not push when the tool has no UI resource uri", async () => { @@ -223,7 +418,7 @@ describe("createAppBridgeFactory", () => { bridge.onopenlink!({ url: "https://example.com" }), ).resolves.toEqual({ isError: false }); expect(open).toHaveBeenCalledWith( - "https://example.com", + "https://example.com/", "_blank", "noopener,noreferrer", ); @@ -234,4 +429,331 @@ describe("createAppBridgeFactory", () => { open.mockRestore(); }); + + describe("ondownloadfile", () => { + // happy-dom does not implement window.confirm, so stub it (rather than + // spyOn an absent function). Returns the installed mock for assertions. + function stubConfirm(approved: boolean): ReturnType { + const confirm = vi.fn().mockReturnValue(approved); + vi.stubGlobal("confirm", confirm); + return confirm; + } + + async function buildBridge(): Promise { + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource: vi.fn().mockResolvedValue(uiResource("

x

")), + }); + await factory(makeIframe(), tool); + return bridgeInstances[0]; + } + + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it("downloads an inline text resource after confirmation", async () => { + vi.useFakeTimers(); + const confirm = stubConfirm(true); + const createUrl = vi + .spyOn(URL, "createObjectURL") + .mockReturnValue("blob:fake"); + const revokeUrl = vi + .spyOn(URL, "revokeObjectURL") + .mockImplementation(() => undefined); + const click = vi + .spyOn(HTMLAnchorElement.prototype, "click") + .mockImplementation(() => undefined); + + const bridge = await buildBridge(); + await expect( + bridge.ondownloadfile!({ + contents: [ + { + type: "resource", + resource: { + uri: "file:///report.csv", + mimeType: "text/csv", + text: "a,b\n1,2", + }, + }, + ], + }), + ).resolves.toEqual({ isError: false }); + + expect(confirm).toHaveBeenCalledTimes(1); + expect(createUrl).toHaveBeenCalledTimes(1); + expect(click).toHaveBeenCalledTimes(1); + const clickedAnchor = click.mock.instances[0] as HTMLAnchorElement; + expect(clickedAnchor.download).toBe("report.csv"); + // Revoke is deferred so the browser can read the blob before it's freed. + expect(revokeUrl).not.toHaveBeenCalled(); + vi.runAllTimers(); + expect(revokeUrl).toHaveBeenCalledWith("blob:fake"); + vi.useRealTimers(); + }); + + it("falls back to a 'download' filename when the URI has no usable tail", async () => { + vi.useFakeTimers(); + stubConfirm(true); + vi.spyOn(URL, "createObjectURL").mockReturnValue("blob:fake"); + vi.spyOn(URL, "revokeObjectURL").mockImplementation(() => undefined); + const click = vi + .spyOn(HTMLAnchorElement.prototype, "click") + .mockImplementation(() => undefined); + const bridge = await buildBridge(); + await bridge.ondownloadfile!({ + contents: [ + { + type: "resource", + resource: { + uri: "file:///path/", + mimeType: "text/plain", + text: "", + }, + }, + ], + }); + expect((click.mock.instances[0] as HTMLAnchorElement).download).toBe( + "download", + ); + vi.runAllTimers(); + vi.useRealTimers(); + }); + + it("warns and reports partial success when some items in a batch are skipped", async () => { + stubConfirm(true); + vi.spyOn(URL, "createObjectURL").mockReturnValue("blob:fake"); + vi.spyOn(URL, "revokeObjectURL").mockImplementation(() => undefined); + vi.spyOn(HTMLAnchorElement.prototype, "click").mockImplementation( + () => undefined, + ); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const bridge = await buildBridge(); + await expect( + bridge.ondownloadfile!({ + contents: [ + { + type: "resource", + resource: { uri: "file:///ok.txt", text: "ok" }, + }, + { type: "resource_link", uri: "javascript:alert(1)" }, + ], + }), + ).resolves.toEqual({ isError: false }); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining("1 of 2 download item(s) skipped"), + expect.arrayContaining(["javascript:alert(1)"]), + ); + warn.mockRestore(); + }); + + it("decodes a base64 blob resource", async () => { + stubConfirm(true); + const createUrl = vi + .spyOn(URL, "createObjectURL") + .mockReturnValue("blob:fake"); + vi.spyOn(URL, "revokeObjectURL").mockImplementation(() => undefined); + vi.spyOn(HTMLAnchorElement.prototype, "click").mockImplementation( + () => undefined, + ); + + const bridge = await buildBridge(); + await expect( + bridge.ondownloadfile!({ + contents: [ + { + type: "resource", + resource: { + uri: "file:///logo.png", + mimeType: "image/png", + blob: btoa("PNGDATA"), + }, + }, + ], + }), + ).resolves.toEqual({ isError: false }); + + const blob = createUrl.mock.calls[0][0] as Blob; + expect(blob.type).toBe("image/png"); + expect(await blob.text()).toBe("PNGDATA"); + }); + + it("opens a resource link in a new tab", async () => { + stubConfirm(true); + const open = vi + .spyOn(window, "open") + .mockImplementation(() => null as unknown as Window); + + const bridge = await buildBridge(); + await expect( + bridge.ondownloadfile!({ + contents: [ + { type: "resource_link", uri: "https://example.com/a.pdf" }, + ], + }), + ).resolves.toEqual({ isError: false }); + + expect(open).toHaveBeenCalledWith( + "https://example.com/a.pdf", + "_blank", + "noopener,noreferrer", + ); + }); + + it("returns isError when the user declines the confirmation", async () => { + stubConfirm(false); + const createUrl = vi.spyOn(URL, "createObjectURL"); + + const bridge = await buildBridge(); + await expect( + bridge.ondownloadfile!({ + contents: [ + { + type: "resource", + resource: { uri: "file:///x.txt", text: "x" }, + }, + ], + }), + ).resolves.toEqual({ isError: true }); + expect(createUrl).not.toHaveBeenCalled(); + }); + + it("returns isError for an empty contents array without confirming", async () => { + const confirm = stubConfirm(true); + const bridge = await buildBridge(); + await expect(bridge.ondownloadfile!({ contents: [] })).resolves.toEqual({ + isError: true, + }); + expect(confirm).not.toHaveBeenCalled(); + }); + + it("returns isError when a download throws", async () => { + stubConfirm(true); + vi.spyOn(URL, "createObjectURL").mockImplementation(() => { + throw new Error("boom"); + }); + + const bridge = await buildBridge(); + await expect( + bridge.ondownloadfile!({ + contents: [ + { + type: "resource", + resource: { uri: "file:///x.txt", text: "x" }, + }, + ], + }), + ).resolves.toEqual({ isError: true }); + }); + + it("rejects a non-http(s) resource_link without opening it", async () => { + stubConfirm(true); + vi.spyOn(console, "warn").mockImplementation(() => {}); + const open = vi + .spyOn(window, "open") + .mockImplementation(() => null as never); + const bridge = await buildBridge(); + for (const uri of [ + "javascript:alert(1)", + "data:text/html,", + "file:///etc/passwd", + "not a url", + ]) { + await expect( + bridge.ondownloadfile!({ + contents: [{ type: "resource_link", uri }], + }), + ).resolves.toEqual({ isError: true }); + } + expect(open).not.toHaveBeenCalled(); + }); + + it("sanitizes the confirmation summary so server-supplied labels cannot inject newlines", async () => { + const confirm = stubConfirm(false); + const bridge = await buildBridge(); + await bridge.ondownloadfile!({ + contents: [ + { + type: "resource_link", + uri: "https://example.com/a\n\nThis is safe, click OK", + }, + ], + }); + const prompt = confirm.mock.calls[0][0] as string; + // Only the framing newlines around the (single) summary entry remain; + // the embedded ones from the URI have been stripped to spaces. + expect(prompt).not.toContain("\n\nThis is safe"); + expect(prompt).toContain("This is safe, click OK"); + }); + + it("strips bidi-override and zero-width format characters from the confirmation summary", async () => { + const RLO = "\u{202E}"; + const ZWSP = "\u{200B}"; + const ZWJ = "\u{200D}"; + const confirm = stubConfirm(false); + const bridge = await buildBridge(); + await bridge.ondownloadfile!({ + contents: [ + { + type: "resource_link", + uri: `https://example.com/${RLO}gpj.exe${ZWSP}${ZWJ}`, + }, + ], + }); + const prompt = confirm.mock.calls[0][0] as string; + expect(prompt).not.toContain(RLO); + expect(prompt).not.toContain(ZWSP); + expect(prompt).not.toContain(ZWJ); + expect(prompt).toContain("gpj.exe"); + }); + }); +}); + +describe("measureContainerDimensions", () => { + function elementWithRect(width: number, height: number): HTMLElement { + return { + getBoundingClientRect: () => ({ width, height }) as unknown as DOMRect, + } as unknown as HTMLElement; + } + + it("returns the iframe box as fixed width/height in whole pixels", () => { + expect(measureContainerDimensions(elementWithRect(640.4, 480.6))).toEqual({ + width: 640, + height: 481, + }); + }); + + it("returns undefined for an unmeasured (0×0) box", () => { + expect(measureContainerDimensions(elementWithRect(0, 0))).toBeUndefined(); + expect(measureContainerDimensions(elementWithRect(640, 0))).toBeUndefined(); + }); + + it("returns undefined when getBoundingClientRect is unavailable", () => { + expect( + measureContainerDimensions({} as unknown as HTMLElement), + ).toBeUndefined(); + }); + + it("seeds containerDimensions into the bridge's initial hostContext", async () => { + const measured = { + contentWindow: {} as Window, + getBoundingClientRect: () => + ({ width: 320, height: 240 }) as unknown as DOMRect, + } as unknown as HTMLIFrameElement; + const factory = createAppBridgeFactory({ + getClient: () => fakeClient, + readResource: vi.fn().mockResolvedValue(uiResource("

app

")), + }); + await factory(measured, tool); + const bridge = bridgeInstances[bridgeInstances.length - 1]; + const opts = bridge.ctorArgs[3] as { + hostContext?: { containerDimensions?: unknown }; + }; + expect(opts.hostContext?.containerDimensions).toEqual({ + width: 320, + height: 240, + }); + }); }); diff --git a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts index 079d26650..acbd3f76b 100644 --- a/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts +++ b/clients/web/src/components/elements/AppRenderer/createAppBridgeFactory.ts @@ -4,14 +4,28 @@ import { getToolUiResourceUri, } from "@modelcontextprotocol/ext-apps/app-bridge"; import type { + McpUiDisplayMode, McpUiHostCapabilities, McpUiResourceMeta, } from "@modelcontextprotocol/ext-apps/app-bridge"; import type { Client } from "@modelcontextprotocol/sdk/client/index.js"; import type { + EmbeddedResource, Implementation, ReadResourceResult, + ResourceLink, } from "@modelcontextprotocol/sdk/types.js"; +import { + approveCspSources, + buildSandboxCspPolicy, + wrapSandboxedHtml, +} from "../../../lib/sandbox-csp"; +import { + downloadBlob, + fileNameFromUri, + isHttpUrl, +} from "../../../lib/downloadFile"; +import { snapshotHostContext } from "./hostContext"; import type { BridgeFactory } from "./AppRenderer"; /** @@ -27,52 +41,52 @@ export const HOST_INFO: Implementation = { * Capabilities the inspector host offers a running MCP App. Constructed WITH an * MCP client (see {@link createAppBridgeFactory}), so the bridge auto-forwards * tools/resources/prompts to the view; we only declare the host-side features - * we actually back: external links (handled below), tool/resource list-change - * forwarding, and logging passthrough. + * we actually back: external links and file downloads (both handled below), + * tool/resource list-change forwarding, logging passthrough, and accepting + * view-originated messages. */ export const HOST_CAPABILITIES: McpUiHostCapabilities = { openLinks: {}, + downloadFile: {}, serverTools: { listChanged: true }, serverResources: { listChanged: true }, logging: {}, + // Accept view-originated user messages (ui/message). The inspector has no + // chat loop to continue, so it surfaces each submission in a log rather than + // adding it to a conversation — enough to verify a widget's send path. The + // handler and display live in AppsScreen (which owns the running-app UI); + // here we only advertise the content modalities the inspector can render. + message: { + text: {}, + image: {}, + audio: {}, + resource: {}, + resourceLink: {}, + }, }; +/** + * Display modes the inspector host supports. AppsScreen renders an app either + * inline within its layout card or maximized to fill the screen, so only those + * two are advertised — `pip` is declined (the spec lets a host return its + * current mode for an unsupported request). + */ +export const HOST_AVAILABLE_DISPLAY_MODES: readonly McpUiDisplayMode[] = [ + "inline", + "fullscreen", +]; + export interface AppBridgeFactoryDeps { /** The connected SDK client to back the bridge, or null when disconnected. */ getClient: () => Client | null; /** Reads a UI resource (resources/read) and returns the SDK result. */ readResource: (uri: string) => Promise; -} - -/** - * Resolve the host theme from the DOM at bridge-build time. Mantine writes the - * resolved color scheme to ``. Reading it here - * (rather than capturing React state in the factory deps) keeps the factory's - * identity stable across theme toggles — the AppRenderer treats a new factory - * identity as "rebuild the bridge", which would reload a running app's iframe on - * every theme flip. The theme is read once per bridge build (the value at open - * time); pushing live theme updates to an already-open app would need an - * AppBridge.setHostContext follow-up. - * - * The attribute is only ever `"light"` or `"dark"` — Mantine resolves - * `defaultColorScheme="auto"` to the system value before paint and never writes - * `"auto"` here, so no `auto` branch is needed. The matchMedia fallback only - * covers the attribute being absent (e.g. a hydration race). - */ -function currentTheme(): "light" | "dark" { - if (typeof document !== "undefined") { - const attr = document.documentElement.getAttribute( - "data-mantine-color-scheme", - ); - if (attr === "dark" || attr === "light") return attr; - } - if ( - typeof window !== "undefined" && - window.matchMedia?.("(prefers-color-scheme: dark)").matches - ) { - return "dark"; - } - return "light"; + /** + * Called when reading or posting the UI resource fails after the sandbox + * proxy is ready. Without this the user is left staring at a blank-but-live + * frame; the error is also always console.error'd. + */ + onResourceError?: (err: Error) => void; } /** First text content block of a UI resource, plus its `_meta` (sandbox hints). */ @@ -92,6 +106,64 @@ function extractHtmlAndMeta(result: ReadResourceResult): { throw new Error("UI resource has no text (HTML) content"); } +/** + * Decode a base64-encoded blob resource into bytes for download. Allocates the + * backing store explicitly so the return type is `Uint8Array` + * (Blob accepts `ArrayBufferView`, not the wider + * `ArrayBufferLike`). + */ +function base64ToBytes(b64: string): Uint8Array { + const binary = atob(b64); + const bytes = new Uint8Array(new ArrayBuffer(binary.length)); + for (let i = 0; i < binary.length; i++) bytes[i] = binary.charCodeAt(i); + return bytes; +} + +/** + * Strip control characters and clamp length so a server-supplied filename or + * URI cannot forge additional lines in the confirmation prompt or push the + * real summary off-screen. + */ +function sanitizeDownloadLabel(label: string): string { + // Cc = control chars (newlines, escape, etc.); Cf = format chars (bidi + // overrides, zero-width joiners, BOM) — both can spoof or reflow the prompt. + const cleaned = label.replace(/[\p{Cc}\p{Cf}]+/gu, " ").trim(); + return cleaned.length > 80 ? cleaned.slice(0, 77) + "..." : cleaned; +} + +/** Human-readable label for a download item, shown in the confirmation prompt. */ +function describeDownloadItem(item: EmbeddedResource | ResourceLink): string { + if (item.type === "resource_link") return item.uri; + return fileNameFromUri(item.resource.uri); +} + +/** + * Trigger a browser download for a single MCP resource item. Inline + * {@link EmbeddedResource}s (text or base64 blob) are written via + * {@link downloadBlob}. A {@link ResourceLink} is *opened* in a new tab — + * the inspector does not fetch the URL to disk itself, since the link may + * require auth or content negotiation the browser can supply but we cannot. + * Returns false when the item carries nothing downloadable or its URI is + * rejected by the http(s)-only allowlist. + */ +function downloadResourceItem(item: EmbeddedResource | ResourceLink): boolean { + if (item.type === "resource_link") { + const parsed = isHttpUrl(item.uri); + if (!parsed) return false; + window.open(parsed.href, "_blank", "noopener,noreferrer"); + return true; + } + const resource = item.resource; + const blob = + "blob" in resource + ? new Blob([base64ToBytes(resource.blob)], { + type: resource.mimeType ?? "application/octet-stream", + }) + : new Blob([resource.text], { type: resource.mimeType ?? "text/plain" }); + downloadBlob(fileNameFromUri(resource.uri), blob); + return true; +} + /** * Builds the {@link BridgeFactory} the AppRenderer uses to bring a sandbox * iframe to life. For each mounted iframe + tool it: @@ -99,9 +171,13 @@ function extractHtmlAndMeta(result: ReadResourceResult): { * 1. constructs a host-side {@link AppBridge} over the SDK client (so the view * can call tools/resources/prompts directly), * 2. on the sandbox proxy's `sandboxready` signal, reads the tool's UI - * resource and pushes its HTML + sandbox/permissions into the inner iframe, + * resource and pushes its HTML + sandbox/permissions/CSP into the inner + * iframe, echoing the applied sandbox config back via hostCapabilities, * 3. handles `openLinks` by opening http(s) URLs in a new tab, - * 4. connects a {@link PostMessageTransport} to the iframe and returns the + * 4. handles `downloadFile` by confirming with the user, then writing each + * embedded resource to disk via an object-URL anchor (resource links are + * opened in a new tab), + * 5. connects a {@link PostMessageTransport} to the iframe and returns the * live bridge. * * Host-initiated tool input/result are pushed separately through the renderer's @@ -123,15 +199,21 @@ export function createAppBridgeFactory( throw new Error("Cannot render MCP App: sandbox iframe has no window."); } - const bridge = new AppBridge(client, HOST_INFO, HOST_CAPABILITIES, { - hostContext: { theme: currentTheme() }, + // Per-app copy so the approved-sandbox echo (set on sandboxready below) never + // mutates the shared HOST_CAPABILITIES constant — each app may declare its own + // csp/permissions. + const hostCapabilities: McpUiHostCapabilities = { ...HOST_CAPABILITIES }; + const bridge = new AppBridge(client, HOST_INFO, hostCapabilities, { + hostContext: snapshotHostContext(iframe, HOST_AVAILABLE_DISPLAY_MODES), }); // The double-iframe proxy posts `sandboxready` once it can receive content. // Read the tool's UI resource and hand its HTML (plus any sandbox/permission - // hints from the resource _meta) to the inner sandboxed iframe. Swallow - // failures: a read error leaves an empty (but live) app frame rather than - // tearing down the bridge mid-handshake. + // hints from the resource _meta) to the inner sandboxed iframe. A failure + // here is the case a developer most needs surfaced (their app's resource is + // erroring or malformed) — log it and report it via deps.onResourceError so + // the host can show something better than a blank frame. The bridge stays + // live so a retry path remains possible. bridge.addEventListener("sandboxready", () => { void (async () => { try { @@ -139,23 +221,79 @@ export function createAppBridgeFactory( if (!uri) return; const result = await deps.readResource(uri); const { html, meta } = extractHtmlAndMeta(result); + // Build the per-app CSP host-side: filter the requested sources to + // ones the host accepts, render the policy string, and wrap the + // app's HTML in a fixed shell whose first child is the CSP + // . The proxy writes that document verbatim — it never parses + // the untrusted bytes — so the policy is guaranteed to apply before + // any app content loads. The approved (post-filter) csp is what we + // echo back via hostCapabilities.sandbox so the view sees what was + // granted, not what it asked for. Set before sendSandboxResourceReady: + // the view only sends ui/initialize once it has the HTML, so the + // bridge reflects this in the initialize result. + const approvedCsp = approveCspSources(meta?.csp); + hostCapabilities.sandbox = { + permissions: meta?.permissions, + csp: approvedCsp, + }; await bridge.sendSandboxResourceReady({ - html, + html: wrapSandboxedHtml(html, buildSandboxCspPolicy(approvedCsp)), permissions: meta?.permissions, - csp: meta?.csp, }); - } catch { - /* read/post failed (or bridge closed) — leave the frame empty */ + } catch (err) { + const error = err instanceof Error ? err : new Error(String(err)); + console.error( + "[mcp-app] failed to load UI resource into sandbox:", + error, + ); + deps.onResourceError?.(error); } })(); }); bridge.onopenlink = async ({ url }) => { - if (/^https?:\/\//i.test(url)) { - window.open(url, "_blank", "noopener,noreferrer"); - return { isError: false }; + const parsed = isHttpUrl(url); + if (!parsed) return { isError: true }; + window.open(parsed.href, "_blank", "noopener,noreferrer"); + return { isError: false }; + }; + + // The view asks the host to save MCP resource contents to disk (sandboxed + // iframes can't download directly). Confirm with the user first — the spec + // requires a host-mediated confirmation — then write each item out. A + // declined prompt, an empty payload, or a thrown error all return isError. + bridge.ondownloadfile = async ({ contents }) => { + if (!Array.isArray(contents) || contents.length === 0) { + return { isError: true }; + } + const summary = contents + .map((item) => sanitizeDownloadLabel(describeDownloadItem(item))) + .join("\n"); + const approved = window.confirm( + `This MCP App wants to download ${contents.length} file(s):\n\n${summary}`, + ); + if (!approved) return { isError: true }; + let succeeded = 0; + const skipped: string[] = []; + for (const item of contents) { + try { + if (downloadResourceItem(item)) { + succeeded++; + } else { + skipped.push(describeDownloadItem(item)); + } + } catch (err) { + skipped.push(describeDownloadItem(item)); + console.error("[mcp-app] download item failed:", err); + } + } + if (skipped.length > 0) { + console.warn( + `[mcp-app] ${skipped.length} of ${contents.length} download item(s) skipped:`, + skipped, + ); } - return { isError: true }; + return { isError: succeeded === 0 }; }; const transport = new PostMessageTransport(targetWindow, targetWindow); diff --git a/clients/web/src/components/elements/AppRenderer/hostContext.ts b/clients/web/src/components/elements/AppRenderer/hostContext.ts new file mode 100644 index 000000000..9ea6f8f08 --- /dev/null +++ b/clients/web/src/components/elements/AppRenderer/hostContext.ts @@ -0,0 +1,156 @@ +import type { + McpUiDisplayMode, + McpUiHostContext, + McpUiHostStyles, + McpUiStyles, + McpUiStyleVariableKey, +} from "@modelcontextprotocol/ext-apps/app-bridge"; + +/** + * Resolve the host theme from the DOM. Mantine writes the resolved color + * scheme to ``. Reading it here (rather than + * capturing React state) keeps the bridge factory's identity stable across + * theme toggles — the renderer treats a new factory identity as "rebuild the + * bridge", which would reload a running app's iframe on every theme flip. + * + * The attribute is only ever `"light"` or `"dark"` — Mantine resolves + * `defaultColorScheme="auto"` to the system value before paint and never + * writes `"auto"` here, so no `auto` branch is needed. The matchMedia + * fallback only covers the attribute being absent (e.g. a hydration race). + */ +export function currentTheme(): "light" | "dark" { + if (typeof document !== "undefined") { + const attr = document.documentElement.getAttribute( + "data-mantine-color-scheme", + ); + if (attr === "dark" || attr === "light") return attr; + } + if ( + typeof window !== "undefined" && + window.matchMedia?.("(prefers-color-scheme: dark)").matches + ) { + return "dark"; + } + return "light"; +} + +/** + * Maps the spec's host-style variable keys ({@link McpUiStyleVariableKey}) to + * the inspector's underlying CSS custom properties. The inspector themes itself + * with Mantine, so each spec token resolves to the matching Mantine design-token + * variable (or an `--inspector-*` token layered on top of one). Only a curated + * subset of the ~90 spec keys is mapped — the ones the inspector has a + * meaningful equivalent for; the rest are omitted, which the spec allows (hosts + * may provide any subset). + */ +const STYLE_VARIABLE_SOURCES: Partial> = { + "--color-background-primary": "--mantine-color-body", + "--color-background-secondary": "--inspector-surface-card", + "--color-background-tertiary": "--inspector-surface-subtle", + "--color-text-primary": "--mantine-color-text", + "--color-text-secondary": "--inspector-text-secondary", + "--color-text-inverse": "--inspector-text-inverse", + "--color-text-info": "--inspector-log-info", + "--color-text-danger": "--inspector-log-error", + "--color-text-success": "--inspector-status-connected", + "--color-text-warning": "--inspector-log-warning", + "--color-border-primary": "--inspector-border-default", + "--color-border-secondary": "--inspector-border-subtle", + "--font-sans": "--mantine-font-family", + "--font-mono": "--mantine-font-family-monospace", + "--font-text-xs-size": "--mantine-font-size-xs", + "--font-text-sm-size": "--mantine-font-size-sm", + "--font-text-md-size": "--mantine-font-size-md", + "--font-text-lg-size": "--mantine-font-size-lg", + "--border-radius-xs": "--mantine-radius-xs", + "--border-radius-sm": "--mantine-radius-sm", + "--border-radius-md": "--mantine-radius-md", + "--border-radius-lg": "--mantine-radius-lg", + "--border-radius-xl": "--mantine-radius-xl", + "--shadow-sm": "--mantine-shadow-sm", + "--shadow-md": "--mantine-shadow-md", + "--shadow-lg": "--mantine-shadow-lg", +}; + +const STYLE_VARIABLE_ENTRIES = Object.entries(STYLE_VARIABLE_SOURCES) as [ + McpUiStyleVariableKey, + string, +][]; + +/** + * Resolve the inspector's design tokens into a {@link McpUiHostStyles} for + * hostContext, so style-aware apps can theme themselves from the host instead + * of falling back to their own defaults. Reads the computed value of each + * mapped CSS variable from the document root — which reflects the active + * Mantine color scheme — and keeps only the ones that resolve to a non-empty + * value. Returns undefined when nothing resolves (e.g. a non-DOM/test + * environment) so we never advertise an empty styles object. + */ +export function currentStyles(): McpUiHostStyles | undefined { + if (typeof document === "undefined" || typeof window === "undefined") { + return undefined; + } + const computed = window.getComputedStyle(document.documentElement); + const variables: McpUiStyles = {} as McpUiStyles; + let resolved = false; + for (const [specKey, sourceVar] of STYLE_VARIABLE_ENTRIES) { + const value = computed.getPropertyValue(sourceVar).trim(); + if (value) { + variables[specKey] = value; + resolved = true; + } + } + return resolved ? { variables } : undefined; +} + +/** + * Spec shape for `hostContext.containerDimensions`. Derived from + * {@link McpUiHostContext} so the seed and live-push paths share one source of + * truth and stay in lockstep with the spec types. + */ +export type ContainerDimensions = NonNullable< + McpUiHostContext["containerDimensions"] +>; + +/** + * Measure the host container an app renders into and return its concrete + * `{ width, height }` (whole pixels). Returns undefined when the element has + * no layout box yet (0×0 — e.g. before the iframe is attached, or in a + * non-DOM/test environment) so a meaningless size is never seeded into + * hostContext. The return type is the concrete pair rather than the spec's + * {@link ContainerDimensions} union so callers can compare both fields. + */ +export function measureContainerDimensions( + element: HTMLElement, +): { width: number; height: number } | undefined { + if (typeof element.getBoundingClientRect !== "function") return undefined; + const rect = element.getBoundingClientRect(); + const width = Math.round(rect.width); + const height = Math.round(rect.height); + if (width <= 0 || height <= 0) return undefined; + return { width, height }; +} + +/** + * Read the live host UI state into a {@link McpUiHostContext} for the bridge + * handshake — the single place that decides which fields the inspector seeds. + * Optional fields are omitted (not set undefined) so the SDK's diff stays + * accurate; subsequent live changes are pushed by the renderer's observers as + * partial `host-context-changed` notifications. + */ +export function snapshotHostContext( + container: HTMLElement | null, + availableDisplayModes: readonly McpUiDisplayMode[], +): McpUiHostContext { + const styles = currentStyles(); + const containerDimensions = container + ? measureContainerDimensions(container) + : undefined; + return { + theme: currentTheme(), + displayMode: "inline", + availableDisplayModes: [...availableDisplayModes], + ...(styles ? { styles } : {}), + ...(containerDimensions ? { containerDimensions } : {}), + }; +} diff --git a/clients/web/src/components/groups/AppDetailPanel/AppDetailPanel.test.tsx b/clients/web/src/components/groups/AppDetailPanel/AppDetailPanel.test.tsx index 9ba090b88..269a14037 100644 --- a/clients/web/src/components/groups/AppDetailPanel/AppDetailPanel.test.tsx +++ b/clients/web/src/components/groups/AppDetailPanel/AppDetailPanel.test.tsx @@ -149,4 +149,10 @@ describe("AppDetailPanel", () => { await user.click(screen.getByRole("button", { name: /open app/i })); expect(onOpenApp).toHaveBeenCalledTimes(1); }); + + it("exposes the Open App button via the open-app testid", () => { + renderWithMantine(); + const button = screen.getByTestId("open-app"); + expect(button).toBe(screen.getByRole("button", { name: /open app/i })); + }); }); diff --git a/clients/web/src/components/groups/AppDetailPanel/AppDetailPanel.tsx b/clients/web/src/components/groups/AppDetailPanel/AppDetailPanel.tsx index 40e56a6ce..09c25a1f4 100644 --- a/clients/web/src/components/groups/AppDetailPanel/AppDetailPanel.tsx +++ b/clients/web/src/components/groups/AppDetailPanel/AppDetailPanel.tsx @@ -71,6 +71,7 @@ export function AppDetailPanel({ onClick={onOpenApp} disabled={disabled} loading={isOpening} + data-testid="open-app" > Open App diff --git a/clients/web/src/components/screens/AppsScreen/AppsScreen.test.tsx b/clients/web/src/components/screens/AppsScreen/AppsScreen.test.tsx index 9157918b4..ed10eb150 100644 --- a/clients/web/src/components/screens/AppsScreen/AppsScreen.test.tsx +++ b/clients/web/src/components/screens/AppsScreen/AppsScreen.test.tsx @@ -1,7 +1,8 @@ import { createRef, useState } from "react"; +import { act } from "@testing-library/react"; import { describe, it, expect, vi } from "vitest"; import userEvent from "@testing-library/user-event"; -import type { Tool } from "@modelcontextprotocol/sdk/types.js"; +import type { ContentBlock, Tool } from "@modelcontextprotocol/sdk/types.js"; import type { AppBridge } from "@modelcontextprotocol/ext-apps/app-bridge"; import { renderWithMantine, @@ -59,6 +60,63 @@ const okBridgeFactory: BridgeFactory = () => close: async () => {}, }) as unknown as AppBridge; +// A bridge factory whose bridge supports the addEventListener/emit surface the +// AppRenderer wires up, so a test can drive a bridge event (sizechange, +// loggingmessage) through to the screen's handling. `emit` dispatches to the +// captured listeners; `bridges` exposes each built bridge so tests can also +// drive the per-bridge handlers (onmessage, onrequestdisplaymode). +function createEventBridgeFactory(): { + factory: BridgeFactory; + bridges: AppBridge[]; + emit: (event: string, payload?: unknown) => void; +} { + const listeners: Record void)[]> = {}; + const bridges: AppBridge[] = []; + const factory: BridgeFactory = () => { + const bridge = { + sendToolInput: async () => {}, + sendToolInputPartial: async () => {}, + sendToolResult: async () => {}, + sendToolCancelled: async () => {}, + sendHostContextChange: async () => {}, + teardownResource: async () => ({}), + close: async () => {}, + addEventListener: (event: string, handler: (p: unknown) => void) => { + (listeners[event] ??= []).push(handler); + }, + removeEventListener: () => {}, + } as unknown as AppBridge; + bridges.push(bridge); + return bridge; + }; + return { + factory, + bridges, + emit: (event, payload) => + (listeners[event] ?? []).forEach((h) => h(payload)), + }; +} + +// Invoke the onmessage handler the screen attached to the latest bridge, +// mimicking a ui/message request from the running view. +async function sendUiMessage( + bridges: AppBridge[], + content: ContentBlock[], +): Promise> { + const onmessage = bridges.at(-1)?.onmessage as unknown as + | ((params: { + role: "user"; + content: ContentBlock[]; + }) => Promise>) + | undefined; + if (!onmessage) throw new Error("no onmessage handler attached"); + let result: Record = {}; + await act(async () => { + result = await onmessage({ role: "user", content }); + }); + return result; +} + function buildProps(overrides: Partial = {}): AppsScreenProps { return { tools: [fieldedApp, noFieldsApp, cohortApp] as Tool[], @@ -147,6 +205,31 @@ describe("AppsScreen", () => { expect((onError.mock.calls[0][0] as Error).message).toContain( "no connected MCP client", ); + // The error is also captured locally and surfaced visibly + as a data + // attribute, so an automated driver can read *why* without scraping a toast. + const card = screen.getByTestId("apps-form"); + expect(card.getAttribute("data-app-status")).toBe("error"); + expect(card.getAttribute("data-app-error")).toBe("no connected MCP client"); + expect(screen.getByTestId("apps-error")).toBeInTheDocument(); + expect(screen.getByText("App failed to load")).toBeInTheDocument(); + expect(screen.getByText("no connected MCP client")).toBeInTheDocument(); + }); + + it("clears the app error when the app is closed", async () => { + const user = userEvent.setup(); + const throwingFactory: BridgeFactory = () => { + throw new Error("transient"); + }; + renderWithMantine(); + await user.click(screen.getByText("Ops Dashboard")); + await vi.waitFor(() => + expect(screen.getByTestId("apps-error")).toBeInTheDocument(), + ); + await user.click(screen.getByLabelText("Close")); + expect(screen.queryByTestId("apps-error")).not.toBeInTheDocument(); + expect( + screen.getByTestId("apps-form").getAttribute("data-app-error"), + ).toBeNull(); }); it("filters the list via the search input", async () => { @@ -249,6 +332,31 @@ describe("AppsScreen", () => { expect(screen.getByText("MCP Apps (3)")).toBeInTheDocument(); }); + it("sizes the renderer frame to the view-reported height", async () => { + const user = userEvent.setup(); + const { factory, emit } = createEventBridgeFactory(); + renderWithMantine(); + // Auto-launches the no-fields app, mounting the renderer and registering + // its sizechange listener once the bridge resolves. + await user.click(screen.getByText("Ops Dashboard")); + const iframe = screen.getByTitle("Ops Dashboard"); + const frame = iframe.parentElement as HTMLElement; + // Until the view reports a size, the frame flex-grows to fill the card. + expect(frame.style.flexGrow).toBe("1"); + + await act(async () => { + // Let the synchronous bridge factory's promise chain settle so the + // sizechange listener is registered before we emit. + await Promise.resolve(); + await Promise.resolve(); + emit("sizechange", { height: 600 }); + }); + // A reported height switches the frame to its content size: it stops + // flex-growing and takes the explicit `h` (applied as a calc() the test + // env's CSS parser elides, so the observable signal is the flex change). + expect(frame.style.flexGrow).toBe("0"); + }); + it("calls onCloseApp and clears selection on Close", async () => { const user = userEvent.setup(); const onCloseApp = vi.fn(); @@ -353,4 +461,168 @@ describe("AppsScreen", () => { ).not.toBeInTheDocument(); expect(screen.getByText("Select an app to view details")).toBeVisible(); }); + + it("surfaces ui/message content from the view in the message log", async () => { + const user = userEvent.setup(); + const { factory, bridges } = createEventBridgeFactory(); + renderWithMantine(); + // The no-fields app auto-launches on selection, mounting the renderer, + // whose effect invokes the (message-wrapped) factory and attaches the + // onmessage handler the test drives below. + await user.click(screen.getByText("Ops Dashboard")); + await vi.waitFor(() => + expect(bridges.at(-1)?.onmessage).toBeTypeOf("function"), + ); + await sendUiMessage(bridges, [ + { type: "text", text: "hello from the app" }, + ]); + expect(screen.getByText(/Messages from app \(1\)/)).toBeInTheDocument(); + expect(screen.getByText(/hello from the app/)).toBeInTheDocument(); + expect(screen.getByTestId("apps-messages")).toBeInTheDocument(); + }); + + it("returns an empty ui/message result (no conversation content leak)", async () => { + const user = userEvent.setup(); + const { factory, bridges } = createEventBridgeFactory(); + renderWithMantine(); + await user.click(screen.getByText("Ops Dashboard")); + await vi.waitFor(() => + expect(bridges.at(-1)?.onmessage).toBeTypeOf("function"), + ); + const result = await sendUiMessage(bridges, [ + { type: "text", text: "secret" }, + ]); + expect(result).toEqual({}); + }); + + it("clears the message log when the app is closed", async () => { + const user = userEvent.setup(); + const { factory, bridges } = createEventBridgeFactory(); + renderWithMantine(); + await user.click(screen.getByText("Ops Dashboard")); + await vi.waitFor(() => + expect(bridges.at(-1)?.onmessage).toBeTypeOf("function"), + ); + await sendUiMessage(bridges, [ + { type: "text", text: "hello from the app" }, + ]); + expect(screen.getByText(/Messages from app/)).toBeInTheDocument(); + await user.click(screen.getByLabelText("Close")); + expect(screen.queryByText(/Messages from app/)).not.toBeInTheDocument(); + }); + + it("surfaces app log notifications in a (default-expanded) collapsible log panel with a working Clear", async () => { + const user = userEvent.setup(); + const { factory, emit } = createEventBridgeFactory(); + renderWithMantine(); + await user.click(screen.getByText("Ops Dashboard")); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + expect(screen.queryByText(/App logs/)).not.toBeInTheDocument(); + await act(async () => { + emit("loggingmessage", { level: "warning", data: "disk almost full" }); + emit("loggingmessage", { + level: "error", + logger: "render", + data: { code: 500 }, + }); + }); + const toggle = screen.getByRole("button", { name: /App logs \(2\)/ }); + expect(toggle).toBeInTheDocument(); + expect(toggle.getAttribute("aria-expanded")).toBe("true"); + // Default-expanded: entries are visible without clicking the toggle, so an + // automated driver reading `[data-testid="apps-logs"]` innerText sees them. + expect(screen.getByText("disk almost full")).toBeInTheDocument(); + expect(screen.getByText("render")).toBeInTheDocument(); + expect(screen.getByText('{"code":500}')).toBeInTheDocument(); + expect(screen.getByTestId("apps-logs")).toBeInTheDocument(); + // Collapsing still works. + await user.click(toggle); + expect(toggle.getAttribute("aria-expanded")).toBe("false"); + await user.click(screen.getByRole("button", { name: "Clear" })); + expect(screen.queryByText(/App logs/)).not.toBeInTheDocument(); + }); + + describe("data-app-status", () => { + function getStatus(): string | null { + return screen.getByTestId("apps-form").getAttribute("data-app-status"); + } + + it("is idle when no app is running", () => { + renderWithMantine( + , + ); + expect(getStatus()).toBe("idle"); + }); + + it("transitions idle → loading → ready around the view's initialized signal", async () => { + const user = userEvent.setup(); + const { factory, emit } = createEventBridgeFactory(); + renderWithMantine(); + expect(getStatus()).toBe("idle"); + await user.click(screen.getByText("Ops Dashboard")); + // Auto-launch mounts the renderer and starts the bridge build. + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + expect(getStatus()).toBe("loading"); + await act(async () => emit("initialized")); + expect(getStatus()).toBe("ready"); + }); + + it("reports error when the bridge factory rejects", async () => { + const user = userEvent.setup(); + const failingFactory: BridgeFactory = () => + Promise.reject(new Error("connect refused")); + renderWithMantine( + , + ); + await user.click(screen.getByText("Ops Dashboard")); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + expect(getStatus()).toBe("error"); + }); + + it("resets to idle when the running app is closed", async () => { + const user = userEvent.setup(); + const { factory, emit } = createEventBridgeFactory(); + renderWithMantine(); + await user.click(screen.getByText("Ops Dashboard")); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + await act(async () => emit("initialized")); + expect(getStatus()).toBe("ready"); + await user.click(screen.getByLabelText("Close")); + expect(getStatus()).toBe("idle"); + }); + }); + + it("stages partial-input snapshots from the form and clears them", async () => { + const user = userEvent.setup(); + renderWithMantine(); + // No-fields apps don't show the control. + await user.click(screen.getByText("Ops Dashboard")); + expect( + screen.queryByRole("button", { name: "Stage partial input" }), + ).not.toBeInTheDocument(); + // Fielded apps do. + await user.click(screen.getByText("Weather Widget")); + const stage = screen.getByRole("button", { name: "Stage partial input" }); + expect(stage).toBeInTheDocument(); + expect(screen.queryByText(/staged/)).not.toBeInTheDocument(); + await user.click(stage); + await user.click(stage); + expect(screen.getByText("2 staged")).toBeInTheDocument(); + await user.click(screen.getByRole("button", { name: "Clear staged" })); + expect(screen.queryByText(/staged/)).not.toBeInTheDocument(); + }); }); diff --git a/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx b/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx index 127f43c67..1b66280b7 100644 --- a/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx +++ b/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx @@ -1,13 +1,18 @@ -import { useState, type Ref } from "react"; +import { useRef, useState, type Ref } from "react"; import { ActionIcon, Button, Card, + Code, + Collapse, Flex, Group, Image, + Paper, + ScrollArea, Stack, Text, + Title, Tooltip, } from "@mantine/core"; import { @@ -16,14 +21,23 @@ import { MdFullscreen, MdFullscreenExit, } from "react-icons/md"; -import type { Tool } from "@modelcontextprotocol/sdk/types.js"; +import type { + ContentBlock, + LoggingMessageNotification, + Tool, +} from "@modelcontextprotocol/sdk/types.js"; +import type { McpUiDisplayMode } from "@modelcontextprotocol/ext-apps/app-bridge"; import { AppRenderer, type AppRendererHandle, + type AppRendererStatus, type BridgeFactory, } from "../../elements/AppRenderer/AppRenderer"; +import { HOST_AVAILABLE_DISPLAY_MODES } from "../../elements/AppRenderer/createAppBridgeFactory"; import { AppDetailPanel } from "../../groups/AppDetailPanel/AppDetailPanel"; import { AppControls } from "../../groups/AppControls/AppControls"; +import { ContentViewer } from "../../elements/ContentViewer/ContentViewer"; +import { LogLevelBadge } from "../../elements/LogLevelBadge/LogLevelBadge"; import { hasInputFields, resolveDisplayLabel } from "../../../utils/toolUtils"; import { collectSchemaDefaults } from "../../../utils/jsonUtils"; @@ -94,7 +108,7 @@ const EmptyState = Text.withProps({ py: "xl", }); -const HeaderRow = Group.withProps({ +const SpacedRow = Group.withProps({ justify: "space-between", wrap: "nowrap", gap: "sm", @@ -127,7 +141,32 @@ const HeaderActions = Group.withProps({ wrap: "nowrap", }); -const RendererFrame = Stack.withProps({ +const HeaderActionIcon = ActionIcon.withProps({ + variant: "subtle", +}); + +const CompactSubtleButton = Button.withProps({ + variant: "subtle", + size: "compact-xs", +}); + +const BackToInputButton = Button.withProps({ + variant: "subtle", + size: "sm", + leftSection: , +}); + +const StagePartialButton = Button.withProps({ + variant: "default", + size: "compact-xs", +}); + +// The host-controlled box the running app sits within. Its size is driven by +// the host's layout (window resize, sidebar toggle, maximize) and NOT by the +// view's reported content height — that drives the inner RendererFrame — so +// the renderer's containerDimensions observer can measure this element without +// coupling host→view container size to view→host size-changed. +const RendererContainer = Stack.withProps({ flex: 1, miw: 0, mih: 0, @@ -139,6 +178,116 @@ const ContentStack = Stack.withProps({ h: "100%", }); +// Pinned panel below the running app (used by both the message log and the +// app-log panel). `0 0 auto` keeps it at its content height (capped by the +// inner scroll's `mah`) so it never squeezes out the iframe above it. +const PinnedPanel = Stack.withProps({ + gap: "xs", + flex: "0 0 auto", + mih: 0, +}); + +const LogScroll = ScrollArea.withProps({ + mah: 200, + type: "auto", + scrollbars: "y", + offsetScrollbars: true, +}); + +const MessageLogStack = Stack.withProps({ + gap: "sm", +}); + +const MessageItem = Paper.withProps({ + p: "md", + radius: "md", + withBorder: true, +}); + +const MessageItemStack = Stack.withProps({ + gap: "xs", +}); + +const MonoCaption = Text.withProps({ + size: "xs", + c: "dimmed", + ff: "monospace", +}); + +const AppLogList = Stack.withProps({ + gap: "xs", +}); + +const AppLogRow = Group.withProps({ + gap: "sm", + wrap: "nowrap", + align: "flex-start", +}); + +const AppLogData = Code.withProps({ + block: true, + fz: "xs", +}); + +const PartialStageControls = Group.withProps({ + gap: "sm", + wrap: "nowrap", + align: "center", + flex: "0 0 auto", +}); + +const PartialStageCount = Text.withProps({ + size: "xs", + c: "dimmed", +}); + +const AppErrorPanel = Paper.withProps({ + p: "md", + radius: "md", + withBorder: true, + c: "var(--inspector-log-error)", +}); + +const AppErrorTitle = Text.withProps({ + fw: 600, + size: "sm", +}); + +const AppErrorMessage = Text.withProps({ + size: "sm", + ff: "monospace", +}); + +/** Render a log payload as a string for display. */ +function formatLogData(data: unknown): string { + if (typeof data === "string") return data; + try { + return JSON.stringify(data); + } catch { + return String(data); + } +} + +// A user-role message submitted by the running view through ui/message. The +// inspector has no conversation to append to, so it just records the content +// blocks for display. Shape matches McpUiMessageRequest["params"]. +interface AppMessage { + role: "user"; + content: ContentBlock[]; +} + +/** + * One MCP `notifications/message` log entry from the running app, with the + * payload stringified once at capture time so the render path can use it + * directly. `id` is a stable React key. + */ +interface AppLogEntry { + id: number; + level: LoggingMessageNotification["params"]["level"]; + logger?: string; + text: string; +} + export function AppsScreen({ tools, listChanged, @@ -156,16 +305,122 @@ export function AppsScreen({ const { selectedAppName, formValues, search } = ui; const [running, setRunning] = useState(false); const [maximized, setMaximized] = useState(false); + const rendererContainerRef = useRef(null); + const nextLogIdRef = useRef(0); + // Height (px) the running view last reported via ui/notifications/size-changed. + // Undefined until the view reports (or after it's torn down), in which case + // the iframe fills the available card space as before. Local to the screen + // like `running`/`maximized`: it's tied to the live iframe, not persisted. + const [appHeight, setAppHeight] = useState(undefined); + // Messages the running view has pushed via ui/message. The inspector has no + // chat loop, so they're collected here and shown in a log below the app + // rather than continuing a conversation. Local to the screen like `running`: + // tied to the live bridge, cleared when the open ends or the app changes. + const [messages, setMessages] = useState([]); + // Standard MCP log notifications (notifications/message) the running app + // emits. The host advertises the `logging` capability; without surfacing + // these here they'd be silently dropped by the bridge. Same lifecycle as + // `messages`: tied to the live bridge, cleared on open/close/switch. + const [appLogs, setAppLogs] = useState([]); + // Expanded by default so an automated driver reading + // `[data-testid="apps-logs"]` `innerText` sees the entries without an extra + // click. The user can still collapse it for the rest of the run. + const [appLogsExpanded, setAppLogsExpanded] = useState(true); + // High-level renderer lifecycle, surfaced as `data-app-status` on the + // `apps-form` card so an automated driver can poll + // `[data-app-status="ready"]` instead of racing the iframe selector. + const [appStatus, setAppStatus] = useState( + "idle", + ); + // The error that put the renderer into status="error" (factory throw, no + // connected client, resource read failure routed via the same `onError` + // chain). Shown in place of the blank iframe and surfaced as + // `data-app-error` on the `apps-form` card so an automated driver can read + // *why* the open failed without screenshotting a toast. + const [appError, setAppError] = useState(undefined); + // Snapshots of the input form captured via "Stage partial input". On Open + // App they're passed to AppRenderer as `partialInputs` and replayed via + // ui/notifications/tool-input-partial before the complete tool-input, so a + // widget's progressive-render path can be exercised. Cleared on switch/close. + const [partialStages, setPartialStages] = useState[]>( + [], + ); + + // Clear every piece of "tied to the live bridge" local state. Called on + // select/open/close/back so a new run starts clean. `keepPartials` is set + // for `handleOpen`, where the staged fragments are about to be consumed by + // the renderer and should not be cleared first. + function resetSessionState(opts?: { keepPartials?: boolean }) { + setAppHeight(undefined); + setMessages([]); + setAppLogs([]); + setAppLogsExpanded(true); + setAppStatus("idle"); + setAppError(undefined); + setMaximized(false); + if (!opts?.keepPartials) setPartialStages([]); + } + + // Capture the error locally (so it can be shown in the card and surfaced as + // `data-app-error`) and forward it to the parent's onError. The renderer + // already drives `data-app-status="error"` via onAppStatusChange; this adds + // the *reason* alongside it. + function handleAppError(err: Error) { + setAppError(err); + onError?.(err); + } - const selectedTool = selectedAppName - ? tools.find((t) => t.name === selectedAppName) - : undefined; + const selectedTool = tools.find((t) => t.name === selectedAppName); const selectedHasFields = selectedTool ? hasInputFields(selectedTool) : false; + // The running view reports its rendered content height via + // ui/notifications/size-changed; honor it so the iframe is neither clipped + // nor surrounded by dead space. Width is left at the host-controlled + // container width. The value is clamped to the available space by the + // renderer frame's `mah` below, and ignored while maximized (the app fills + // the screen instead). + function handleSizeChange(size: { width?: number; height?: number }) { + if (size.height != null) setAppHeight(size.height); + } + + function handleMessage(params: { role: "user"; content: ContentBlock[] }) { + setMessages((prev) => [...prev, params]); + } + + function handleLog(params: LoggingMessageNotification["params"]) { + setAppLogs((prev) => [ + ...prev, + { + id: nextLogIdRef.current++, + level: params.level, + logger: params.logger, + text: formatLogData(params.data), + }, + ]); + } + + // The app's display mode is derived from the existing maximized toggle. + // Passed to AppRenderer so the running view receives it via + // host-context-changed; the Maximize/Restore button below keeps toggling + // `maximized`, which now flows out as a protocol event. + const displayMode: McpUiDisplayMode = maximized ? "fullscreen" : "inline"; + + // Handle a view-originated ui/request-display-mode. Only modes the inspector + // advertises in `availableDisplayModes` are honored — an unsupported request + // (e.g. "pip") is declined by returning the current mode, per spec. + function handleRequestDisplayMode( + requested: McpUiDisplayMode, + ): McpUiDisplayMode { + if (!HOST_AVAILABLE_DISPLAY_MODES.includes(requested)) return displayMode; + setMaximized(requested === "fullscreen"); + return requested; + } + function handleSelect(name: string) { if (name === selectedAppName) return; const next = tools.find((t) => t.name === name); if (!next) return; + resetSessionState(); // Seed schema defaults so default-only fields are sent on Open App (parity // with the form's resolveValue display, which onChange doesn't capture). onUiChange({ @@ -173,7 +428,6 @@ export function AppsScreen({ selectedAppName: name, formValues: collectSchemaDefaults(next.inputSchema), }); - setMaximized(false); onSelectApp(name); // No-input apps auto-launch on selection so the user lands directly in // the running view; apps with fields wait for the explicit Open App click. @@ -187,20 +441,25 @@ export function AppsScreen({ function handleOpen() { if (!selectedTool) return; + resetSessionState({ keepPartials: true }); setRunning(true); onOpenApp(selectedTool.name, formValues); } + function handleStagePartialInput() { + setPartialStages((prev) => [...prev, { ...formValues }]); + } + function handleClose() { setRunning(false); + resetSessionState(); onUiChange({ ...ui, selectedAppName: undefined, formValues: {} }); - setMaximized(false); onCloseApp(); } function handleBackToInput() { setRunning(false); - setMaximized(false); + resetSessionState(); } // No sandbox proxy URL means the host can't embed the trusted outer iframe @@ -218,6 +477,10 @@ export function AppsScreen({ ); } + // While maximized the app fills the screen, so the view-reported height is + // ignored; otherwise we honor it (clamped to the card by the frame's `mah`). + const contentHeight = maximized ? undefined : appHeight; + return ( {!maximized && ( @@ -236,10 +499,16 @@ export function AppsScreen({ )} - + {selectedTool ? ( - + {selectedTool.icons?.[0]?.src && ( @@ -250,19 +519,13 @@ export function AppsScreen({ {running && selectedHasFields && ( - + )} {running && ( - setMaximized((m) => !m)} aria-label={maximized ? "Restore" : "Maximize"} > @@ -271,34 +534,53 @@ export function AppsScreen({ ) : ( )} - + )} - + - + - + {running ? ( - - {/* Keying by name forces the renderer to remount when the - selected app changes, ensuring a fresh bridge and iframe - rather than reusing the previous app's transport. */} - - + // RendererContainer is the host-controlled box (its size only + // changes with host layout); the inner Stack is sized by the + // view's reported content height, capped at the container. + + + {/* Keying by name forces the renderer to remount when the + selected app changes, ensuring a fresh bridge and iframe + rather than reusing the previous app's transport. */} + + + {appError && ( + + App failed to load + {appError.message} + + )} + ) : ( // `isOpening` is always false here because `handleOpen` // synchronously flips `running` to true, swapping in the @@ -307,15 +589,91 @@ export function AppsScreen({ // standalone use (the `Opening` story) and for Phase 3 // wiring, where a managed-state hook can hold the panel // in a pending state across an awaited `tools/call`. - - onUiChange({ ...ui, formValues: values }) - } - onOpenApp={handleOpen} - /> + <> + {selectedHasFields && ( + + + Stage partial input + + {partialStages.length > 0 && ( + <> + + {partialStages.length} staged + + setPartialStages([])} + > + Clear staged + + + )} + + )} + + onUiChange({ ...ui, formValues: values }) + } + onOpenApp={handleOpen} + /> + + )} + {running && messages.length > 0 && ( + + Messages from app ({messages.length}) + + + {messages.map((message, index) => ( + + + + [{index}] role: {message.role} + + {message.content.map((block, blockIndex) => ( + + ))} + + + ))} + + + + )} + {running && appLogs.length > 0 && ( + + + setAppLogsExpanded((e) => !e)} + aria-expanded={appLogsExpanded} + > + App logs ({appLogs.length}) + + setAppLogs([])}> + Clear + + + + + + {appLogs.map((entry) => ( + + + {entry.logger && ( + {entry.logger} + )} + {entry.text} + + ))} + + + + )} ) : ( diff --git a/clients/web/src/components/views/InspectorView/InspectorView.test.tsx b/clients/web/src/components/views/InspectorView/InspectorView.test.tsx index 3f26dd661..875a8f834 100644 --- a/clients/web/src/components/views/InspectorView/InspectorView.test.tsx +++ b/clients/web/src/components/views/InspectorView/InspectorView.test.tsx @@ -192,6 +192,22 @@ describe("InspectorView", () => { ).toBeInTheDocument(); }); + it("surfaces connectErrorMessage and deepLinkStatus as data-* attributes on the connection-status header", () => { + renderWithMantine( + , + ); + const header = screen.getByTestId("connection-status"); + expect(header.dataset.status).toBe("error"); + expect(header.dataset.errorMessage).toBe("fetch failed: ENOTFOUND"); + expect(header.dataset.deeplink).toBe("rejected"); + }); + it("renders the server card from the input list", () => { renderWithMantine( , diff --git a/clients/web/src/components/views/InspectorView/InspectorView.tsx b/clients/web/src/components/views/InspectorView/InspectorView.tsx index d0ce045b2..40966052c 100644 --- a/clients/web/src/components/views/InspectorView/InspectorView.tsx +++ b/clients/web/src/components/views/InspectorView/InspectorView.tsx @@ -1,4 +1,12 @@ -import { useMemo, useState, type ReactNode, type Ref } from "react"; +import { + useEffect, + useMemo, + useRef, + useState, + type ReactNode, + type Ref, +} from "react"; +import type { DeepLink, DeepLinkParseStatus } from "../../../utils/deepLink"; import { AppShell, Box, Stack, Transition } from "@mantine/core"; import { useLocalStorage } from "@mantine/hooks"; import type { @@ -19,6 +27,7 @@ import type { ServerEntry, } from "@inspector/core/mcp/types.js"; import { isAppTool } from "@inspector/core/mcp/apps.js"; +import { collectSchemaDefaults } from "../../../utils/jsonUtils"; import { ViewHeader } from "../../groups/ViewHeader/ViewHeader"; import { ServerListScreen } from "../../screens/ServerListScreen/ServerListScreen"; import { @@ -160,9 +169,12 @@ const ScreenStageContainer = Stack.withProps({ // expanded sections) is reset on tab switch. function ScreenStage({ active, + testId, children, }: { active: boolean; + /** Stable hook for automated drivers; only present in the DOM while mounted. */ + testId?: string; children: ReactNode; }) { return ( @@ -176,7 +188,14 @@ function ScreenStage({ {(styles) => ( // `style={styles}` is the runtime transition state from Mantine's // Transition API — these are interpolated values, not static styling. - + {children} )} @@ -185,6 +204,21 @@ function ScreenStage({ } export interface InspectorViewProps { + /** + * Validated deep-link parameters from the page URL. When present and + * `openApp` is set, this component switches to the Apps tab and pre-selects + * that app (with `appArgs` as the form values) once the connection is up and + * the app list contains it. The connect itself is driven by the parent. + */ + deepLink?: DeepLink; + /** + * Outcome of parsing the initial-URL deep link, surfaced as `data-deeplink` + * on the `connection-status` testid. Distinguishes "no deep link" from + * "rejected" (token mismatch / bad serverUrl) — both leave `data-status` + * idle, so an automated driver otherwise cannot tell them apart. + */ + deepLinkStatus?: DeepLinkParseStatus; + // Server list (static config; runtime connection state comes from the // separate fields below and is merged into each card by this component). servers: ServerEntry[]; @@ -198,6 +232,13 @@ export interface InspectorViewProps { // Connection state — driven by the parent via `useInspectorClient`. activeServer?: string; connectionStatus: ConnectionStatus; + /** + * Last connection-level error message (handshake failure, OAuth start + * failure, deep-link automation failure). Surfaced as `data-error-message` + * on the header's `connection-status` testid so an automated driver can read + * *why* a connect failed without scraping a transient toast. + */ + connectErrorMessage?: string; initializeResult?: InitializeResult; latencyMs?: number; @@ -354,10 +395,13 @@ export interface InspectorViewProps { } export function InspectorView({ + deepLink, + deepLinkStatus, servers: serversInput, serverListWritable = true, activeServer, connectionStatus, + connectErrorMessage, initializeResult, latencyMs, tools, @@ -448,8 +492,13 @@ export function InspectorView({ }: InspectorViewProps) { // UI-only state. Connection state, primitive lists, and all action // dispatching live in the parent; this component only owns navigation - // (which tab is visible) and a couple of view-local toggles. - const [selectedTab, setSelectedTab] = useState(SERVERS_TAB); + // (which tab is visible) and a couple of view-local toggles. A deep link + // with `openApp` seeds the selection at "Apps" — the `availableTabs` clamp + // below shows Servers until the connection is up and the Apps tab becomes + // available, at which point the seeded selection takes effect. + const [selectedTab, setSelectedTab] = useState( + deepLink?.openApp ? "Apps" : SERVERS_TAB, + ); const [logsSort, setLogsSort] = useSortDirection("logs"); const [historySort, setHistorySort] = useSortDirection("history"); @@ -537,6 +586,50 @@ export function InspectorView({ ? selectedTab : SERVERS_TAB; + // Deep-link auto-open: once connected and the requested app appears in the + // app-tools list, pre-select it with the supplied form values (the tab + // itself is seeded above). AppsScreen owns the running/iframe state + // internally, so the remaining "click Open" is one explicit user (or + // automated driver) action away. + const deepLinkOpenAppRef = useRef(false); + useEffect(() => { + if (!deepLink?.openApp) return; + if (deepLinkOpenAppRef.current) return; + if (connectionStatus !== "connected") return; + if (!availableTabs.includes("Apps")) return; + const target = appTools.find((t) => t.name === deepLink.openApp); + if (!target) return; + deepLinkOpenAppRef.current = true; + // Seed with the schema's defaults THEN overlay the deep-link's appArgs. + // Without the defaults, a required field that the form would display with + // its default value is absent from `formValues`, the schema-form's + // validity check fails, and the Open App button is silently disabled — + // an automated driver's click then no-ops and the iframe-wait spins + // forever. + const formValues = { + ...collectSchemaDefaults(target.inputSchema), + ...deepLink.appArgs, + }; + onAppsUiChange({ + ...appsUi, + selectedAppName: target.name, + formValues, + }); + onSelectApp(target.name); + // `deepLink.autoOpen` is parsed and exposed for AppsScreen to consume + // (the screen owns its `running` state and the iframe mount); calling + // `onOpenApp` here would fire the tool call without mounting the + // renderer, so the open is left to the screen layer. + }, [ + deepLink, + connectionStatus, + availableTabs, + appTools, + appsUi, + onAppsUiChange, + onSelectApp, + ]); + // Merge the parent's `serversInput` (static config) with the runtime // connection state owned by the parent — only the active server reflects // the live status; the rest render as `disconnected`. Handshake errors @@ -575,7 +668,12 @@ export function InspectorView({ // Main-slot height clamp + overflow:hidden keep that scroll on the inner // ScrollArea regions only. - + {connectionStatus === "connected" && initializeResult ? ( - + URL.revokeObjectURL(url), 0); + } +} + +/** Download an in-memory JSON string as `filename`. */ +export function downloadJsonFile(filename: string, json: string): void { + downloadBlob(filename, new Blob([json], { type: "application/json" })); +} + +/** + * Derive a safe suggested filename from a resource URI: the last path segment, + * stripped of control/format characters, path separators, and characters + * disallowed in filenames on common platforms. Falls back to `"download"` when + * nothing usable remains. + */ +export function fileNameFromUri(uri: string): string { + const tail = uri.split(/[\\/]/).pop() ?? ""; + const safe = tail + .replace(/[\p{Cc}\p{Cf}]+/gu, "") + .replace(/[\\/:*?"<>|]+/g, "_") + .trim(); + return safe.length > 0 ? safe.slice(0, 255) : "download"; +} + +/** + * Parse `url` and return it only if its scheme is `http:` or `https:`; + * otherwise null. Shared http(s)-only allowlist for opening or downloading + * server-supplied URLs. + */ +export function isHttpUrl(url: string): URL | null { + try { + const parsed = new URL(url); + return parsed.protocol === "https:" || parsed.protocol === "http:" + ? parsed + : null; + } catch { + return null; } } diff --git a/clients/web/src/lib/environmentFactory.test.ts b/clients/web/src/lib/environmentFactory.test.ts new file mode 100644 index 000000000..492ad8414 --- /dev/null +++ b/clients/web/src/lib/environmentFactory.test.ts @@ -0,0 +1,36 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { createWebEnvironment } from "./environmentFactory"; +import { RemoteOAuthStorage } from "@inspector/core/auth/remote/index.js"; + +describe("createWebEnvironment", () => { + beforeEach(() => { + // RemoteOAuthStorage's persist adapter issues a hydration GET on + // construction; stub global fetch so the test doesn't depend on a backend. + vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response(JSON.stringify({}), { status: 200 }), + ); + }); + + it("uses RemoteOAuthStorage so OAuth state lands on the backend", () => { + const { environment } = createWebEnvironment( + "test-auth-token", + { getRedirectUrl: () => "http://localhost/callback" }, + undefined, + ); + expect(environment.oauth?.storage).toBeInstanceOf(RemoteOAuthStorage); + }); + + it("returns the same RemoteOAuthStorage for the same {baseUrl, authToken}", () => { + const redirect = { getRedirectUrl: () => "http://localhost/callback" }; + const a = createWebEnvironment("token-1", redirect, undefined); + const b = createWebEnvironment("token-1", redirect, undefined); + expect(a.environment.oauth?.storage).toBe(b.environment.oauth?.storage); + }); + + it("returns a distinct RemoteOAuthStorage when the authToken differs", () => { + const redirect = { getRedirectUrl: () => "http://localhost/callback" }; + const a = createWebEnvironment("token-A", redirect, undefined); + const b = createWebEnvironment("token-B", redirect, undefined); + expect(a.environment.oauth?.storage).not.toBe(b.environment.oauth?.storage); + }); +}); diff --git a/clients/web/src/lib/environmentFactory.ts b/clients/web/src/lib/environmentFactory.ts index bba83fc32..a48387a01 100644 Binary files a/clients/web/src/lib/environmentFactory.ts and b/clients/web/src/lib/environmentFactory.ts differ diff --git a/clients/web/src/lib/sandbox-csp.test.ts b/clients/web/src/lib/sandbox-csp.test.ts new file mode 100644 index 000000000..8beee9781 --- /dev/null +++ b/clients/web/src/lib/sandbox-csp.test.ts @@ -0,0 +1,164 @@ +import { describe, expect, it, vi } from "vitest"; +import { + SAFE_CSP_SOURCE, + approveCspSources, + buildSandboxCspPolicy, + escapeHtmlAttr, + wrapSandboxedHtml, +} from "./sandbox-csp"; + +describe("SAFE_CSP_SOURCE", () => { + it.each([ + "*", + "https://api.example.com", + "https://api.example.com:8443", + "https://api.example.com/path/seg", + "wss://realtime.service.com", + "*.example.com", + "https://*.cloudflare.com", + "data:", + "blob:", + "example.com", + ])("accepts %s", (s) => { + expect(SAFE_CSP_SOURCE.test(s)).toBe(true); + }); + + it.each([ + "https://a.com; script-src *", + 'https://a.com" onload=', + "https://a.com>", + "app`; + const wrapped = wrapSandboxedHtml(evil, "default-src 'none'"); + // The first in the output is OURS; the untrusted token only + // appears after . + const ourHead = wrapped.indexOf(""); + const body = wrapped.indexOf(""); + const evilHead = wrapped.indexOf("", ourHead + 1); + expect(ourHead).toBeLessThan(body); + expect(evilHead).toBeGreaterThan(body); + }); + + it("does not introduce a host-authored