diff --git a/clients/web/src/test/core/inspectorClient.test.ts b/clients/web/src/test/core/inspectorClient.test.ts index 7a163e762..08ed3028d 100644 --- a/clients/web/src/test/core/inspectorClient.test.ts +++ b/clients/web/src/test/core/inspectorClient.test.ts @@ -39,6 +39,9 @@ import { createSamplingTaskTool, createProgressTaskTool, createTaskTool, + createAddResourceTool, + createAddToolTool, + createAddPromptTool, } from "@modelcontextprotocol/inspector-test-server"; import type { MessageEntry, @@ -4013,4 +4016,591 @@ describe("InspectorClient", () => { expect(parsed.input).toBe("E2E elicitation input"); }); }); + + describe("Coverage backfill (#1310)", () => { + describe("guard methods without server / OAuth", () => { + it("returns no-op defaults from OAuth getters when oauthManager is unset", async () => { + const c = new InspectorClient( + { + type: "stdio", + command: serverCommand.command, + args: serverCommand.args, + }, + { environment: { transport: createTransportNode } }, + ); + await expect(c.getOAuthTokens()).resolves.toBeUndefined(); + await expect(c.isOAuthAuthorized()).resolves.toBe(false); + expect(c.getOAuthStep()).toBeUndefined(); + expect(c.getOAuthState()).toBeUndefined(); + // clearOAuthTokens is a no-op when there is no manager + expect(() => c.clearOAuthTokens()).not.toThrow(); + }); + + it("setOAuthConfig throws when oauthManager is unset", () => { + const c = new InspectorClient( + { + type: "stdio", + command: serverCommand.command, + args: serverCommand.args, + }, + { environment: { transport: createTransportNode } }, + ); + expect(() => c.setOAuthConfig({ clientId: "x" })).toThrow( + /OAuth config must be set at creation/, + ); + }); + + it("authenticate / runGuidedAuth / proceedOAuthStep throw via ensureOAuthManager when oauthManager is unset", async () => { + const c = new InspectorClient( + { + type: "stdio", + command: serverCommand.command, + args: serverCommand.args, + }, + { environment: { transport: createTransportNode } }, + ); + await expect(c.authenticate()).rejects.toThrow(/OAuth not configured/); + await expect(c.runGuidedAuth()).rejects.toThrow(/OAuth not configured/); + await expect(c.proceedOAuthStep()).rejects.toThrow( + /OAuth not configured/, + ); + }); + + it("simple session/roots/subscription accessors return empty defaults before connect", () => { + const c = new InspectorClient( + { + type: "stdio", + command: serverCommand.command, + args: serverCommand.args, + }, + { environment: { transport: createTransportNode } }, + ); + expect(c.getSessionId()).toBeUndefined(); + // saveSession is a no-op when there is no sessionId + expect(() => c.saveSession()).not.toThrow(); + c.setSessionId("session-123"); + expect(c.getSessionId()).toBe("session-123"); + + expect(c.getSubscribedResources()).toEqual([]); + expect(c.isSubscribedToResource("file:///nope")).toBe(false); + // No server capabilities loaded yet -> subscriptions unsupported + expect(c.supportsResourceSubscriptions()).toBe(false); + // No instructions loaded before connect + expect(c.getInstructions()).toBeUndefined(); + // Roots default to [] when undefined + expect(c.getRoots()).toEqual([]); + }); + + it("subscribe / unsubscribe error when server doesn't advertise subscribe capability", async () => { + // Stdio test server doesn't advertise resources.subscribe by default + server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + resources: createNumberedResources(2), + }); + await server.start(); + client = new InspectorClient( + { type: "streamable-http", url: server.url }, + { environment: { transport: createTransportNode } }, + ); + await client.connect(); + + await expect( + client.subscribeToResource("test://resource_1"), + ).rejects.toThrow(/does not support resource subscriptions/); + // unsubscribe path doesn't guard on capability; it calls the SDK which + // will reject because the server has no unsubscribe handler. + await expect( + client.unsubscribeFromResource("test://resource_1"), + ).rejects.toThrow( + /Failed to unsubscribe to resource|Failed to unsubscribe from resource|Method not found/, + ); + }); + }); + + describe("subscribe / unsubscribe happy path", () => { + it("subscribes, reports state, then unsubscribes, when server advertises subscribe", async () => { + server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + resources: createNumberedResources(2), + subscriptions: true, + }); + await server.start(); + client = new InspectorClient( + { type: "streamable-http", url: server.url }, + { environment: { transport: createTransportNode } }, + ); + await client.connect(); + + expect(client.supportsResourceSubscriptions()).toBe(true); + + await client.subscribeToResource("test://resource_1"); + expect(client.getSubscribedResources()).toContain("test://resource_1"); + expect(client.isSubscribedToResource("test://resource_1")).toBe(true); + + await client.unsubscribeFromResource("test://resource_1"); + expect(client.getSubscribedResources()).not.toContain( + "test://resource_1", + ); + expect(client.isSubscribedToResource("test://resource_1")).toBe(false); + }); + }); + + describe("getPrompt + readResourceFromTemplate", () => { + beforeEach(async () => { + server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + resourceTemplates: [createFileResourceTemplate()], + prompts: [createArgsPrompt()], + }); + await server.start(); + client = new InspectorClient( + { type: "streamable-http", url: server.url }, + { environment: { transport: createTransportNode } }, + ); + await client.connect(); + }); + + it("getPrompt fetches and returns an invocation, dispatching no extra error", async () => { + const invocation = await client!.getPrompt("args_prompt", { + city: "Hartford", + state: "Connecticut", + }); + expect(invocation.name).toBe("args_prompt"); + expect(invocation.params).toEqual({ + city: "Hartford", + state: "Connecticut", + }); + expect(invocation.result).toBeDefined(); + expect(invocation.timestamp).toBeInstanceOf(Date); + }); + + it("readResourceFromTemplate expands and reads the resource", async () => { + const invocation = await client!.readResourceFromTemplate( + "file:///{path}", + { path: "report.txt" }, + ); + expect(invocation.uriTemplate).toBe("file:///{path}"); + expect(invocation.expandedUri).toBe("file:///report.txt"); + expect(invocation.params).toEqual({ path: "report.txt" }); + expect(invocation.result).toBeDefined(); + }); + + it("readResourceFromTemplate throws when expansion fails", async () => { + // Pass a syntactically invalid template; UriTemplate ctor throws + await expect( + client!.readResourceFromTemplate("file:///{unclosed", { + unclosed: "x", + }), + ).rejects.toThrow(/Failed to expand URI template/); + }); + }); + + describe("capability detection after connect", () => { + it("round-trips listChanged + subscribe flags via getCapabilities()", async () => { + // The handler-registration arrows in InspectorClient fire during + // connect only when the matching server capability is advertised. + // Exercise all four conditional branches in one connect by enabling + // tools/resources/prompts listChanged + resource subscriptions. + // The resources/prompts arrays are required for the test server to + // actually emit those capability blocks (an empty list omits the + // capability rather than advertising an empty one). + server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + tools: [createEchoTool()], + resources: createNumberedResources(1), + prompts: [createArgsPrompt()], + listChanged: { tools: true, resources: true, prompts: true }, + subscriptions: true, + }); + await server.start(); + client = new InspectorClient( + { type: "streamable-http", url: server.url }, + { environment: { transport: createTransportNode } }, + ); + await client.connect(); + + const caps = client.getCapabilities(); + expect(caps?.tools?.listChanged).toBe(true); + expect(caps?.resources?.listChanged).toBe(true); + expect(caps?.prompts?.listChanged).toBe(true); + expect(caps?.resources?.subscribe).toBe(true); + }); + }); + + describe("setLoggingLevel guards", () => { + it("throws when the server does not advertise logging support", async () => { + server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + tools: [createEchoTool()], + }); + await server.start(); + client = new InspectorClient( + { type: "streamable-http", url: server.url }, + { environment: { transport: createTransportNode } }, + ); + await client.connect(); + await expect(client.setLoggingLevel("info")).rejects.toThrow( + /Server does not support logging/, + ); + }); + }); + + describe("getAppRendererClient", () => { + it("returns null before connect, and a proxy after connect that forwards setNotificationHandler", async () => { + server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + tools: [createEchoTool()], + }); + await server.start(); + const c = new InspectorClient( + { type: "streamable-http", url: server.url }, + { environment: { transport: createTransportNode } }, + ); + // Disconnected => null + expect(c.getAppRendererClient()).toBeNull(); + + client = c; + await c.connect(); + + const proxy1 = c.getAppRendererClient(); + expect(proxy1).not.toBeNull(); + // Second call returns the cached proxy + expect(c.getAppRendererClient()).toBe(proxy1); + // setNotificationHandler on the proxy delegates to the underlying client + expect( + typeof (proxy1 as unknown as { setNotificationHandler?: unknown }) + .setNotificationHandler, + ).toBe("function"); + }); + }); + + describe("list_changed + resourceUpdated notifications", () => { + it("dispatches toolsListChanged when the server emits notifications/tools/list_changed", async () => { + server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + tools: [createAddToolTool()], + listChanged: { tools: true }, + }); + await server.start(); + client = new InspectorClient( + { type: "streamable-http", url: server.url }, + { environment: { transport: createTransportNode } }, + ); + await client.connect(); + + const fired = waitForEvent(client, "toolsListChanged", { + timeout: 5000, + }); + const addToolTool = await getTool(client, "add_tool"); + await client.callTool(addToolTool, { + name: "newly_added", + description: "added at runtime", + }); + await fired; + }); + + it("dispatches resourcesListChanged when the server emits resources/list_changed", async () => { + server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + tools: [createAddResourceTool()], + resources: createNumberedResources(1), + listChanged: { resources: true }, + }); + await server.start(); + client = new InspectorClient( + { type: "streamable-http", url: server.url }, + { environment: { transport: createTransportNode } }, + ); + await client.connect(); + + const fired = waitForEvent(client, "resourcesListChanged", { + timeout: 5000, + }); + const addResourceTool = await getTool(client, "add_resource"); + await client.callTool(addResourceTool, { + uri: "res://new", + name: "new", + text: "hi", + }); + await fired; + }); + + it("dispatches promptsListChanged when the server emits prompts/list_changed", async () => { + server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + tools: [createAddPromptTool()], + prompts: [createArgsPrompt()], + listChanged: { prompts: true }, + }); + await server.start(); + client = new InspectorClient( + { type: "streamable-http", url: server.url }, + { environment: { transport: createTransportNode } }, + ); + await client.connect(); + + const fired = waitForEvent(client, "promptsListChanged", { + timeout: 5000, + }); + const addPromptTool = await getTool(client, "add_prompt"); + await client.callTool(addPromptTool, { + name: "new_prompt", + description: "added at runtime", + promptString: "hello", + }); + await fired; + }); + }); + + describe("misc tiny branches", () => { + it("connect() is a no-op when status is already 'connected'", async () => { + server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + tools: [createEchoTool()], + }); + await server.start(); + client = new InspectorClient( + { type: "streamable-http", url: server.url }, + { environment: { transport: createTransportNode } }, + ); + await client.connect(); + expect(client.getStatus()).toBe("connected"); + // Second connect() should hit the early-return branch + await client.connect(); + expect(client.getStatus()).toBe("connected"); + }); + + it("connect() throws 'Client not initialized' when this.client is null", async () => { + const c = new InspectorClient( + { + type: "stdio", + command: serverCommand.command, + args: serverCommand.args, + }, + { environment: { transport: createTransportNode } }, + ); + (c as unknown as { client: unknown }).client = null; + await expect(c.connect()).rejects.toThrow(/Client not initialized/); + }); + + it("getTaskCapabilities() returns undefined when the server has no tasks capability", async () => { + server = createTestServerHttp({ + serverInfo: createTestServerInfo(), + tools: [createEchoTool()], + }); + await server.start(); + client = new InspectorClient( + { type: "streamable-http", url: server.url }, + { environment: { transport: createTransportNode } }, + ); + await client.connect(); + expect(client.getTaskCapabilities()).toBeUndefined(); + }); + + it("elicit form mode adds form to elicitation capability (constructor branch)", () => { + const c = new InspectorClient( + { + type: "stdio", + command: serverCommand.command, + args: serverCommand.args, + }, + { + environment: { transport: createTransportNode }, + // Hits the `this.elicit.form` branch in the constructor's + // elicitationCap.form assignment block + elicit: { form: true }, + }, + ); + expect(c.getStatus()).toBe("disconnected"); + }); + + it("receiver-task internals: TTL cleanup and cancel terminate via private surface", async () => { + // Drive the private createReceiverTask + cancelReceiverTask + TTL-cleanup + // paths by reaching into the instance. These are server-driven in + // practice (tasks/cancel from server), but the existing receiver-task + // e2e tests don't exercise the cancel path; this is the focused unit + // pass the issue suggested. + const c = new InspectorClient( + { + type: "stdio", + command: serverCommand.command, + args: serverCommand.args, + }, + { environment: { transport: createTransportNode } }, + ); + const internal = c as unknown as { + createReceiverTask: (opts: { + ttl?: number; + initialStatus: "input_required" | "working"; + statusMessage?: string; + }) => { + task: { taskId: string; status: string }; + payloadPromise: Promise; + }; + cancelReceiverTask: (taskId: string) => { + taskId: string; + status: string; + }; + listReceiverTasks: () => Array<{ taskId: string; status: string }>; + getReceiverTask: (taskId: string) => unknown; + getReceiverTaskPayload: (taskId: string) => Promise; + receiverTaskRecords: Map; + }; + + // Short TTL so the cleanup setTimeout fires in-test + const record = internal.createReceiverTask({ + ttl: 50, + initialStatus: "working", + statusMessage: "running", + }); + // Pre-attach a catch so cancel's reject doesn't surface as unhandled + const payloadResult = record.payloadPromise.catch( + (e) => (e as Error).message, + ); + expect(record.task.taskId).toBeDefined(); + // listReceiverTasks contains the new task + const list = internal.listReceiverTasks(); + expect(list.some((t) => t.taskId === record.task.taskId)).toBe(true); + expect(internal.getReceiverTask(record.task.taskId)).toBeDefined(); + + // getReceiverTaskPayload on an unknown id throws InvalidParams + await expect( + internal.getReceiverTaskPayload("does-not-exist"), + ).rejects.toThrow(/Unknown taskId/); + + // Cancel before TTL fires + const cancelled = internal.cancelReceiverTask(record.task.taskId); + expect(cancelled.status).toBe("cancelled"); + await expect(payloadResult).resolves.toBe("Task cancelled"); + // Cancel again — record is in terminal state, returns existing task + const reCancel = internal.cancelReceiverTask(record.task.taskId); + expect(reCancel.status).toBe("cancelled"); + + // cancelReceiverTask on an unknown id throws InvalidParams + expect(() => internal.cancelReceiverTask("nope")).toThrow( + /Unknown taskId/, + ); + + // Drive the TTL-cleanup path: create a record with very short ttl and + // let setTimeout fire — receiverTaskRecords drops the entry. + const ttlRecord = internal.createReceiverTask({ + ttl: 20, + initialStatus: "working", + statusMessage: "running", + }); + await new Promise((r) => setTimeout(r, 80)); + expect(internal.receiverTaskRecords.has(ttlRecord.task.taskId)).toBe( + false, + ); + }); + }); + + describe("defensive guards when client is uninitialized", () => { + // These guards exist as TypeScript narrows for the `Client | null` field + // even though the constructor always assigns it. Force the field to null + // via a private-field cast so we can exercise the throw branches once, + // rather than sprinkling `if` guards through tests for each method. + // + // NOTE: keep this list in sync with the `if (!this.client) throw …` + // sites in core/mcp/inspectorClient.ts. If a guard is removed during a + // future refactor (e.g. when the field becomes `Client` instead of + // `Client | null`) this test will silently under-cover rather than + // fail — the matching call below should be removed at the same time. + function nullify(c: InspectorClient): void { + (c as unknown as { client: unknown }).client = null; + } + + it("public methods that require a client throw or short-circuit when client is null", async () => { + const c = new InspectorClient( + { + type: "stdio", + command: serverCommand.command, + args: serverCommand.args, + }, + { environment: { transport: createTransportNode } }, + ); + nullify(c); + + const expectThrow = async ( + fn: () => Promise, + msg = "Client is not connected", + ) => { + await expect(fn()).rejects.toThrow(msg); + }; + + // listing / pagination + await expectThrow(() => c.listTools()); + await expectThrow(() => c.listResources()); + await expectThrow(() => c.listResourceTemplates()); + await expectThrow(() => c.listPrompts()); + + // single-item fetch + await expectThrow(() => + c.callTool({ name: "x" } as unknown as Tool, {}), + ); + await expectThrow(() => c.readResource("res://x")); + await expectThrow(() => + c.readResourceFromTemplate("file:///{p}", { p: "x" }), + ); + await expectThrow(() => c.getPrompt("x")); + + // logging guard fires the "not connected" branch first + await expectThrow(() => c.setLoggingLevel("info")); + + // roots + subscriptions + await expectThrow(() => c.setRoots([])); + await expectThrow(() => c.subscribeToResource("res://x")); + await expectThrow(() => c.unsubscribeFromResource("res://x")); + + // requestor task ops + await expectThrow(() => c.getRequestorTask("t")); + await expectThrow(() => c.getRequestorTaskResult("t")); + await expectThrow(() => c.cancelRequestorTask("t")); + await expectThrow(() => c.listRequestorTasks()); + + // ping + await expectThrow(() => c.ping(), "Client not initialized"); + + // callToolStream: throws synchronously? Actually returns Promise from + // the iterator's first .next() — call and assert rejection. + await expectThrow(() => + c.callToolStream({ name: "x" } as unknown as Tool, {}), + ); + + // getCompletions short-circuits to { values: [] } rather than throwing + await expect( + c.getCompletions({ type: "ref/prompt", name: "x" }, "arg", "val"), + ).resolves.toEqual({ values: [] }); + }); + }); + + describe("OAuth + stdio transport", () => { + it("authenticate() throws because stdio transports have no server URL", async () => { + const c = new InspectorClient( + { + type: "stdio", + command: serverCommand.command, + args: serverCommand.args, + }, + { + environment: { + transport: createTransportNode, + oauth: { + // Minimal stubs — providers won't ever be reached because + // getServerUrl throws first. + storage: {} as never, + navigation: {} as never, + redirectUrlProvider: {} as never, + }, + }, + }, + ); + // ensureOAuthManager hits the manager (since oauth env was supplied) + // and the manager calls getServerUrl() which throws for stdio. + await expect(c.authenticate()).rejects.toThrow( + /OAuth is only supported for HTTP-based transports/, + ); + }); + }); + }); }); diff --git a/clients/web/src/test/core/remote-tokenAuthProvider.test.ts b/clients/web/src/test/core/remote-tokenAuthProvider.test.ts new file mode 100644 index 000000000..ff522cdcc --- /dev/null +++ b/clients/web/src/test/core/remote-tokenAuthProvider.test.ts @@ -0,0 +1,50 @@ +import { describe, it, expect } from "vitest"; +import { createTokenAuthProvider } from "@inspector/core/mcp/remote/node/tokenAuthProvider.js"; + +describe("createTokenAuthProvider", () => { + it("returns undefined when tokens are not provided", () => { + expect(createTokenAuthProvider(undefined)).toBeUndefined(); + }); + + it("returns a provider whose tokens() resolves with the supplied tokens", async () => { + const tokens = { access_token: "abc", token_type: "Bearer" }; + const provider = createTokenAuthProvider(tokens); + expect(provider).toBeDefined(); + await expect(provider!.tokens()).resolves.toEqual(tokens); + }); + + it("exposes no-op stubs for the auxiliary OAuthClientProvider methods", async () => { + const provider = createTokenAuthProvider({ + access_token: "abc", + token_type: "Bearer", + }); + expect(provider).toBeDefined(); + // The aux methods are no-op stubs that satisfy the OAuthClientProvider + // surface; the underlying object type widens via the `as unknown as` + // cast in the source, so we narrow here for the test assertions. + const p = provider! as unknown as { + clientInformation: () => Promise; + saveTokens: (t: { + access_token: string; + token_type: string; + }) => Promise; + codeVerifier: () => string | undefined; + saveCodeVerifier: (v: string) => Promise; + clear: () => void; + redirectToAuthorization: (url: URL) => void; + state: () => string; + }; + + await expect(p.clientInformation()).resolves.toBeUndefined(); + await expect( + p.saveTokens({ access_token: "noop", token_type: "Bearer" }), + ).resolves.toBeUndefined(); + expect(p.codeVerifier()).toBeUndefined(); + await expect(p.saveCodeVerifier("v")).resolves.toBeUndefined(); + expect(() => p.clear()).not.toThrow(); + expect(() => + p.redirectToAuthorization(new URL("https://example.com/")), + ).not.toThrow(); + expect(p.state()).toBe(""); + }); +}); diff --git a/clients/web/src/test/core/remote-transport.test.ts b/clients/web/src/test/core/remote-transport.test.ts index 4da591803..82d3bd85c 100644 --- a/clients/web/src/test/core/remote-transport.test.ts +++ b/clients/web/src/test/core/remote-transport.test.ts @@ -4,7 +4,7 @@ */ import { describe, it, expect, beforeEach, afterEach } from "vitest"; -import { mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { serve } from "@hono/node-server"; @@ -1231,4 +1231,277 @@ describe("Remote transport e2e", () => { expect(res.status).not.toBe(403); }); }); + + describe("additional coverage paths", () => { + it("/api/mcp/connect returns 500 when createTransportNode throws", async () => { + const { baseUrl, server, authToken } = await startRemoteServer(0); + remoteServer = server; + // Invalid URL causes new URL() in createTransportNode to throw synchronously + const res = await fetch(`${baseUrl}/api/mcp/connect`, { + method: "POST", + headers: { + "Content-Type": "application/json", + "x-mcp-remote-auth": `Bearer ${authToken}`, + }, + body: JSON.stringify({ + config: { type: "sse" as const, url: "not a url" }, + }), + }); + expect(res.status).toBe(500); + expect((await res.json()).error).toMatch(/Failed to create transport:/); + }); + + it("/api/mcp/send returns 500 when transport is dead", async () => { + mcpHttpServer = createTestServerHttp({ + serverInfo: createTestServerInfo(), + tools: [createEchoTool()], + serverType: "sse", + }); + await mcpHttpServer.start(); + + const { baseUrl, server, authToken } = await startRemoteServer(0); + remoteServer = server; + + // Connect to a live MCP server so we get a valid sessionId + const connectRes = await fetch(`${baseUrl}/api/mcp/connect`, { + method: "POST", + headers: { + "Content-Type": "application/json", + "x-mcp-remote-auth": `Bearer ${authToken}`, + }, + body: JSON.stringify({ + config: { type: "sse" as const, url: mcpHttpServer.url }, + }), + }); + expect(connectRes.status).toBe(200); + const { sessionId } = (await connectRes.json()) as { sessionId: string }; + + // Kill the upstream so the transport closes/errors and is marked dead + await mcpHttpServer.stop(); + mcpHttpServer = null; + + // Give the transport a moment to notice the closed upstream. The + // assertion below is intentionally permissive (status >= 500) so the + // test passes whether the SDK transport marks itself dead (short- + // circuit at /api/mcp/send) or the send call surfaces the failure + // directly. Either path covers a 5xx branch in send(). + await new Promise((r) => setTimeout(r, 250)); + + const sendRes = await fetch(`${baseUrl}/api/mcp/send`, { + method: "POST", + headers: { + "Content-Type": "application/json", + "x-mcp-remote-auth": `Bearer ${authToken}`, + }, + body: JSON.stringify({ + sessionId, + message: { jsonrpc: "2.0", id: 99, method: "ping" }, + }), + }); + // Either the dead-transport short-circuit (500) or a downstream send + // failure (500) — either way, the path is covered. + expect(sendRes.status).toBeGreaterThanOrEqual(500); + }); + + it("/api/log accepts non-JSON body silently via the catch fallback", async () => { + const { baseUrl, server, authToken } = await startRemoteServer(0); + remoteServer = server; + const res = await fetch(`${baseUrl}/api/log`, { + method: "POST", + headers: { + "Content-Type": "application/json", + "x-mcp-remote-auth": `Bearer ${authToken}`, + }, + body: "not json at all", + }); + // The catch arrow swallows the parse error and uses {} — handler still 200s. + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ ok: true }); + }); + + it("/api/log ignores events whose level label is not a logger method", async () => { + const tmp = mkdtempSync(join(tmpdir(), "inspector-log-unknown-level-")); + const logFile = join(tmp, "app.log"); + try { + const logger = pino({ level: "info" }, pino.destination(logFile)); + const { baseUrl, server, authToken } = await startRemoteServer(0, { + logger, + }); + remoteServer = server; + const res = await fetch(`${baseUrl}/api/log`, { + method: "POST", + headers: { + "Content-Type": "application/json", + "x-mcp-remote-auth": `Bearer ${authToken}`, + }, + body: JSON.stringify({ + level: { label: "totally-not-a-real-level" }, + messages: ["should be discarded"], + }), + }); + expect(res.status).toBe(200); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } + }); + + it("/api/log forwards events that have no messages", async () => { + const tmp = mkdtempSync(join(tmpdir(), "inspector-log-no-messages-")); + const logFile = join(tmp, "app.log"); + try { + const logger = pino({ level: "info" }, pino.destination(logFile)); + const { baseUrl, server, authToken } = await startRemoteServer(0, { + logger, + }); + remoteServer = server; + const res = await fetch(`${baseUrl}/api/log`, { + method: "POST", + headers: { + "Content-Type": "application/json", + "x-mcp-remote-auth": `Bearer ${authToken}`, + }, + body: JSON.stringify({ + level: { label: "info" }, + bindings: [{ source: "test" }], + messages: [], + }), + }); + expect(res.status).toBe(200); + // Flush + read back to confirm the bindings were forwarded + await new Promise((r) => setTimeout(r, 100)); + const contents = readFileSync(logFile, "utf-8"); + expect(contents).toContain('"source":"test"'); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } + }); + + it("/api/log forwards events whose first message is a string", async () => { + const tmp = mkdtempSync(join(tmpdir(), "inspector-log-string-first-")); + const logFile = join(tmp, "app.log"); + try { + const logger = pino({ level: "info" }, pino.destination(logFile)); + const { baseUrl, server, authToken } = await startRemoteServer(0, { + logger, + }); + remoteServer = server; + const res = await fetch(`${baseUrl}/api/log`, { + method: "POST", + headers: { + "Content-Type": "application/json", + "x-mcp-remote-auth": `Bearer ${authToken}`, + }, + body: JSON.stringify({ + level: { label: "info" }, + bindings: [{ tag: "string-first" }], + messages: ["hello world"], + }), + }); + expect(res.status).toBe(200); + await new Promise((r) => setTimeout(r, 100)); + const contents = readFileSync(logFile, "utf-8"); + expect(contents).toContain('"msg":"hello world"'); + expect(contents).toContain('"tag":"string-first"'); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } + }); + + it("/api/fetch returns 400 when url is missing", async () => { + const { baseUrl, server, authToken } = await startRemoteServer(0); + remoteServer = server; + const res = await fetch(`${baseUrl}/api/fetch`, { + method: "POST", + headers: { + "Content-Type": "application/json", + "x-mcp-remote-auth": `Bearer ${authToken}`, + }, + body: JSON.stringify({}), + }); + expect(res.status).toBe(400); + expect((await res.json()).error).toBe("Missing url"); + }); + + it("/api/fetch returns 500 when the upstream fetch throws", async () => { + const { baseUrl, server, authToken } = await startRemoteServer(0); + remoteServer = server; + // Use a clearly-unroutable URL so global fetch rejects synchronously-ish + const res = await fetch(`${baseUrl}/api/fetch`, { + method: "POST", + headers: { + "Content-Type": "application/json", + "x-mcp-remote-auth": `Bearer ${authToken}`, + }, + body: JSON.stringify({ + url: "http://127.0.0.1:1/does-not-exist", + }), + }); + expect(res.status).toBe(500); + expect((await res.json()).error).toBeDefined(); + }); + + it("/api/storage POST returns 400 for invalid storeId", async () => { + const tmp = mkdtempSync(join(tmpdir(), "inspector-storage-bad-id-")); + try { + const { baseUrl, server, authToken } = await startRemoteServer(0, { + storageDir: tmp, + }); + remoteServer = server; + const res = await fetch(`${baseUrl}/api/storage/has.dots`, { + method: "POST", + headers: { + "Content-Type": "application/json", + "x-mcp-remote-auth": `Bearer ${authToken}`, + }, + body: JSON.stringify({ k: "v" }), + }); + expect(res.status).toBe(400); + expect((await res.json()).error).toBe("Invalid storeId"); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } + }); + + it("/api/storage POST returns 400 for invalid JSON body", async () => { + const tmp = mkdtempSync(join(tmpdir(), "inspector-storage-bad-json-")); + try { + const { baseUrl, server, authToken } = await startRemoteServer(0, { + storageDir: tmp, + }); + remoteServer = server; + const res = await fetch(`${baseUrl}/api/storage/teststore`, { + method: "POST", + headers: { + "Content-Type": "application/json", + "x-mcp-remote-auth": `Bearer ${authToken}`, + }, + body: "not json", + }); + expect(res.status).toBe(400); + expect((await res.json()).error).toBe("Invalid JSON body"); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } + }); + + it("/api/storage GET returns 500 when the on-disk store is unparseable", async () => { + const tmp = mkdtempSync(join(tmpdir(), "inspector-storage-corrupt-")); + try { + // Write a corrupted file at the storeId path so parseStore() throws + writeFileSync(join(tmp, "teststore.json"), "{ not json"); + const { baseUrl, server, authToken } = await startRemoteServer(0, { + storageDir: tmp, + }); + remoteServer = server; + const res = await fetch(`${baseUrl}/api/storage/teststore`, { + method: "GET", + headers: { "x-mcp-remote-auth": `Bearer ${authToken}` }, + }); + expect(res.status).toBe(500); + expect((await res.json()).error).toMatch(/Failed to read store:/); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } + }); + }); }); diff --git a/clients/web/vite.config.ts b/clients/web/vite.config.ts index e9b21c633..f03ae22f4 100644 --- a/clients/web/vite.config.ts +++ b/clients/web/vite.config.ts @@ -138,18 +138,6 @@ export default defineConfig({ path.join(repoRoot, 'core/mcp/remote/types.ts'), // .d.ts files are declaration-only. path.join(repoRoot, '**/*.d.ts'), - // TODO(#1310): coverage debt on two large v1.5-ported files that the - // existing v1.5 tests don't fully exercise. #1307 brought the - // integration suite back online and lifted all other v1.5 coverage - // gates; #1310 tracks filling these two: - // - inspectorClient.ts (2184 LOC) is at 73% lines despite 4005 LOC - // of integration tests; remaining gaps are scattered across OAuth, - // subscription, and error-path code (~150 lines, 107 ranges). - // - remote/node/server.ts is at 88% lines / 78% functions; gaps are - // in transport-failure / 401-propagation error paths and the - // private createTokenAuthProvider helper. - path.join(repoRoot, 'core/mcp/inspectorClient.ts'), - path.join(repoRoot, 'core/mcp/remote/node/server.ts'), // inspectorClientEventTarget.ts is types + a single empty-body class // (extends TypedEventTarget). v8/istanbul records 0 statements for it // today. TODO(#1243): drop this exclusion once the class gains real diff --git a/core/mcp/remote/node/server.ts b/core/mcp/remote/node/server.ts index d0cf0fc6a..b4ae6e625 100644 --- a/core/mcp/remote/node/server.ts +++ b/core/mcp/remote/node/server.ts @@ -23,8 +23,7 @@ import { createTransportNode } from "../../node/transport.js"; import type { RemoteConnectRequest, RemoteSendRequest } from "../types.js"; import type { MCPServerConfig } from "../../types.js"; import { RemoteSession } from "./remote-session.js"; -import type { OAuthClientProvider } from "@modelcontextprotocol/sdk/client/auth.js"; -import type { OAuthTokens } from "@modelcontextprotocol/sdk/shared/auth.js"; +import { createTokenAuthProvider } from "./tokenAuthProvider.js"; import { API_SERVER_ENV_VARS } from "../constants.js"; /** @@ -185,44 +184,6 @@ function createAuthMiddleware(authToken: string) { }; } -/** - * Simple OAuth client provider that just returns tokens. - * Used by remote server to inject Bearer tokens into transport requests. - */ -function createTokenAuthProvider( - tokens: RemoteConnectRequest["oauthTokens"], -): OAuthClientProvider | undefined { - if (!tokens) return undefined; - - return { - async tokens(): Promise { - return tokens as OAuthTokens; - }, - // Other methods not needed for transport Bearer token injection - async clientInformation() { - return undefined; - }, - async saveTokens() { - // No-op - }, - codeVerifier() { - return undefined; - }, - async saveCodeVerifier() { - // No-op - }, - clear() { - // No-op - }, - redirectToAuthorization() { - // No-op - }, - state() { - return ""; - }, - } as unknown as OAuthClientProvider; -} - function forwardLogEvent( logger: pino.Logger, logEvent: Partial, diff --git a/core/mcp/remote/node/tokenAuthProvider.ts b/core/mcp/remote/node/tokenAuthProvider.ts new file mode 100644 index 000000000..aea91c24a --- /dev/null +++ b/core/mcp/remote/node/tokenAuthProvider.ts @@ -0,0 +1,43 @@ +import type { OAuthClientProvider } from "@modelcontextprotocol/sdk/client/auth.js"; +import type { OAuthTokens } from "@modelcontextprotocol/sdk/shared/auth.js"; +import type { RemoteConnectRequest } from "../types.js"; + +/** + * Simple OAuth client provider that just returns tokens. + * Used by the remote server to inject Bearer tokens into transport requests. + * The other OAuthClientProvider methods are required by the interface but never + * exercised in the remote-server path (the SDK only calls `tokens()` for Bearer + * injection); they are kept as no-op stubs. + */ +export function createTokenAuthProvider( + tokens: RemoteConnectRequest["oauthTokens"], +): OAuthClientProvider | undefined { + if (!tokens) return undefined; + + return { + async tokens(): Promise { + return tokens as OAuthTokens; + }, + async clientInformation() { + return undefined; + }, + async saveTokens() { + // No-op + }, + codeVerifier() { + return undefined; + }, + async saveCodeVerifier() { + // No-op + }, + clear() { + // No-op + }, + redirectToAuthorization() { + // No-op + }, + state() { + return ""; + }, + } as unknown as OAuthClientProvider; +}