diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 05243196e..529c28b2b 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -652,11 +652,28 @@ export namespace Config { url: z.string().describe("URL of the remote MCP server"), enabled: z.boolean().optional().describe("Enable or disable the MCP server on startup"), headers: z.record(z.string(), z.string()).optional().describe("Headers to send with the request"), + // altimate_change start — dynamic header values produced by an argv command + // (run via execFile, not a shell — no shell interpolation unless the user + // explicitly invokes one like `sh -c`), resolved on each (re)connect so + // callers can refresh expiring bearer tokens without restarting the session + // (e.g. `az account get-access-token`). + headersCommand: z + .record(z.string(), z.array(z.string()).nonempty()) + .optional() + .describe( + "Headers whose values are produced by running a command (argv form: [cmd, ...args]). " + + "stdout is trimmed and used as the header value. Resolved on every connect so tokens " + + "with short TTLs (e.g. Microsoft Entra ID bearer tokens for Fabric) refresh automatically. " + + "Values from headersCommand override matching keys in `headers`.", + ), + // altimate_change end oauth: z .union([McpOAuth, z.literal(false)]) .optional() .describe( - "OAuth authentication configuration for the MCP server. Set to false to disable OAuth auto-detection.", + "OAuth authentication configuration for the MCP server. Set to false to disable OAuth auto-detection. " + + "When `headers.Authorization` or `headersCommand.Authorization` is set and `oauth` is not specified, " + + "OAuth is disabled automatically so a static bearer token isn't overridden by a competing OAuth flow.", ), timeout: z .number() @@ -1451,7 +1468,22 @@ export namespace Config { servers[name] = transformed } else if (entry.url && typeof entry.url === "string") { const transformed: Record = { type: "remote", url: entry.url } + // Copy `headers` / `headersCommand` through as-is — including malformed + // array shapes. The downstream `Info.safeParse` validates `mcp` against + // the McpRemote schema, and `z.record(...)` rejects an array with an + // actionable `invalid_type` error. Stripping arrays here would instead + // drop the field silently and connect a header-less server with no + // feedback to the user. See #791 / #792. if (entry.headers && typeof entry.headers === "object") transformed.headers = entry.headers + // altimate_change start — preserve fields that the original normalizer dropped + // silently. Without these passes, a user-supplied `oauth: false` or + // `headersCommand` would be reconstructed-away, leaving the runtime + // believing the config was bare. See #791 / #792. + if (entry.headersCommand && typeof entry.headersCommand === "object") { + transformed.headersCommand = entry.headersCommand + } + if (entry.oauth !== undefined) transformed.oauth = entry.oauth + // altimate_change end if (typeof entry.timeout === "number") transformed.timeout = entry.timeout if (typeof entry.enabled === "boolean") transformed.enabled = entry.enabled if (typeof entry.updatedAt === "string") transformed.updatedAt = entry.updatedAt diff --git a/packages/opencode/src/mcp/index.ts b/packages/opencode/src/mcp/index.ts index 1e4d72f37..ef6e7770a 100644 --- a/packages/opencode/src/mcp/index.ts +++ b/packages/opencode/src/mcp/index.ts @@ -13,6 +13,12 @@ import { Config } from "../config/config" import { Log } from "../util/log" import { NamedError } from "@opencode-ai/util/error" import z from "zod/v4" +// altimate_change start — needed to resolve `headersCommand` for remote MCP +// servers that require bearer tokens with short TTLs (Microsoft Fabric, etc.) +import { execFile } from "node:child_process" +import { promisify } from "node:util" +const execFileAsync = promisify(execFile) +// altimate_change end import { Instance } from "../project/instance" import { Installation } from "../installation" // altimate_change start — persist enabled flag @@ -125,6 +131,165 @@ export namespace MCP { const toolListCache = new Map() // altimate_change end + // altimate_change start — Microsoft Fabric Core MCP returns `null` for + // `tool.annotations.{readOnlyHint,destructiveHint,idempotentHint,openWorldHint}`, + // which the SDK's `ListToolsResultSchema` (z.boolean().optional()) rejects via + // Zod, blocking listTools() entirely. We accept `null` as "hint absent" by + // calling `client.request()` with a permissive schema in place of the SDK's + // strict one. See https://github.com/AltimateAI/altimate-code/issues/792. + const LenientToolAnnotationsSchema = z.looseObject({ + title: z.string().optional(), + readOnlyHint: z.boolean().nullable().optional(), + destructiveHint: z.boolean().nullable().optional(), + idempotentHint: z.boolean().nullable().optional(), + openWorldHint: z.boolean().nullable().optional(), + }) + + const LenientToolSchema = z.looseObject({ + name: z.string(), + title: z.string().optional(), + description: z.string().optional(), + inputSchema: z.any(), + outputSchema: z.any().optional(), + annotations: LenientToolAnnotationsSchema.optional(), + _meta: z.record(z.string(), z.unknown()).optional(), + }) + + const LenientListToolsResultSchema = z.looseObject({ + tools: z.array(LenientToolSchema), + nextCursor: z.string().optional(), + _meta: z.record(z.string(), z.unknown()).optional(), + }) + + function isSchemaError(err: unknown): boolean { + // Narrowly match Zod validation errors only. We key off the error *name* + // (not `instanceof z.ZodError`) because the MCP SDK may throw a ZodError + // from a different zod copy (it depends on `zod@^3.25 || ^4.0`), which + // would fail a cross-realm `instanceof`. In practice the SDK's z4-mini path + // names the error `$ZodError` (the primary case); the public ZodError + // subclass names it `ZodError`. We require an `issues` *array* (rather than + // `"issues" in err`) to avoid a throwing getter and to reject unrelated + // errors that merely carry an `issues` property of some other type. + if (typeof err !== "object" || err === null) return false + const name = (err as { name?: string }).name ?? (err as { constructor?: { name?: string } }).constructor?.name + return (name === "ZodError" || name === "$ZodError") && Array.isArray((err as { issues?: unknown }).issues) + } + + // Upper bound on tools/list pages we will follow, guarding against a server + // that returns a repeating or never-terminating `nextCursor`. + const MAX_TOOL_LIST_PAGES = 100 + + /** + * Fetches a server's complete tool list, following `nextCursor` pagination so + * tools beyond the first page are not dropped. Calls the SDK's strict + * `listTools()` first; on a Zod schema-validation failure (e.g. server emits + * non-spec values like `null` annotation hints), switches to `client.request()` + * with a permissive schema for the remaining pages. This keeps the fast path + * unchanged for compliant servers while letting non-compliant ones (Microsoft + * Fabric, etc.) still register all of their tools. See #986. + */ + async function listToolsLenient(client: MCPClient): Promise<{ tools: MCPToolDef[] }> { + const tools: MCPToolDef[] = [] + let cursor: string | undefined = undefined + let lenient = false + for (let page = 0; page < MAX_TOOL_LIST_PAGES; page++) { + let result: { tools: MCPToolDef[]; nextCursor?: string } + if (!lenient) { + try { + result = await client.listTools(cursor ? { cursor } : {}) + } catch (err) { + if (!isSchemaError(err)) throw err + log.info("listTools strict schema rejected response, retrying with lenient schema", { + error: err instanceof Error ? err.message.slice(0, 200) : String(err).slice(0, 200), + }) + // Re-fetch the same cursor via the permissive schema, and stay lenient + // for any remaining pages (a non-compliant server rarely complies later). + lenient = true + page-- + continue + } + } else { + result = (await client.request( + { method: "tools/list", params: cursor ? { cursor } : {} }, + LenientListToolsResultSchema as any, + )) as { tools: MCPToolDef[]; nextCursor?: string } + } + tools.push(...result.tools) + if (!result.nextCursor) return { tools } + cursor = result.nextCursor + } + log.warn("listTools pagination exceeded page cap, returning partial tool list", { + pages: MAX_TOOL_LIST_PAGES, + tools: tools.length, + }) + return { tools } + } + + /** @internal — exported only for unit tests. Prefer using `tools()` in production code. */ + export const _testing = { + LenientListToolsResultSchema, + isSchemaError, + listToolsLenient: (client: { + listTools: (params?: any) => Promise<{ tools: any[]; nextCursor?: string }> + request: (...args: any[]) => Promise + }) => listToolsLenient(client as unknown as MCPClient), + resolveHeadersCommand: (spec: Record | undefined, key = "test") => + resolveHeadersCommand(spec, key), + hasAuthorizationHeader, + } + // altimate_change end + + // altimate_change start — resolve dynamic header values from argv commands + // (e.g. `az account get-access-token`). Each value is an argv array run via + // execFile (no shell) so values aren't subject to shell injection. Re-runs on + // every connect so expiring bearer tokens refresh without manual config edits. + // See https://github.com/AltimateAI/altimate-code/issues/791. + async function resolveHeadersCommand( + spec: Record | undefined, + serverKey: string, + ): Promise> { + if (!spec) return {} + const out: Record = {} + for (const [name, argv] of Object.entries(spec)) { + if (!Array.isArray(argv) || argv.length === 0) { + throw new Error(`headersCommand[${name}] must be a non-empty argv array`) + } + const [cmd, ...args] = argv + let stdout = "" + try { + stdout = ( + await execFileAsync(cmd, args, { + encoding: "utf-8", + maxBuffer: 1024 * 1024, + timeout: 30_000, + }) + ).stdout + } catch (err) { + // Wrap with the header key so `mcp list` points to the exact failing + // command (ENOENT, timeout, non-zero exit) rather than a bare error. + // On a non-zero exit the actionable reason lives on `err.stderr` (e.g. + // `az`'s "run 'az login'"), which `err.message` omits — append it. + const e = err as { message?: string; stderr?: string } + const stderr = typeof e.stderr === "string" ? e.stderr.trim().slice(0, 500) : "" + const base = err instanceof Error ? err.message : String(err) + const message = stderr ? `${base}: ${stderr}` : base + throw new Error(`headersCommand[${name}] failed: ${message}`) + } + const value = stdout.trim() + if (!value) { + throw new Error(`headersCommand[${name}] produced empty output`) + } + log.info("resolved dynamic header", { server: serverKey, header: name }) + out[name] = value + } + return out + } + + function hasAuthorizationHeader(headers: Record): boolean { + return Object.keys(headers).some((k) => k.toLowerCase() === "authorization") + } + // altimate_change end + // Register notification handlers for MCP client function registerNotificationHandlers(client: MCPClient, serverName: string) { client.setNotificationHandler(ToolListChangedNotificationSchema, async () => { @@ -368,8 +533,36 @@ export namespace MCP { let connectedTransport: "stdio" | "sse" | "streamable-http" | undefined = undefined if (mcp.type === "remote") { - // OAuth is enabled by default for remote servers unless explicitly disabled with oauth: false - const oauthDisabled = mcp.oauth === false + // altimate_change start — resolve dynamic headers (e.g. bearer tokens + // produced by `az account get-access-token`) before constructing + // transports. Failure to resolve aborts the connect attempt. The thrown + // message already names the failing header (`headersCommand[] failed: + // ...`), so the user sees exactly which command broke in `mcp list`. + let dynamicHeaders: Record = {} + try { + dynamicHeaders = await resolveHeadersCommand(mcp.headersCommand, key) + } catch (err) { + const message = err instanceof Error ? err.message : String(err) + log.error("headersCommand resolution failed", { key, error: message }) + return { + mcpClient: undefined, + status: { status: "failed" as const, error: message }, + } + } + const mergedHeaders: Record = { ...(mcp.headers ?? {}), ...dynamicHeaders } + // altimate_change end + + // altimate_change start — OAuth is enabled by default for remote servers, + // BUT if the user provided an explicit Authorization header (statically or + // via headersCommand) and didn't ask for OAuth, skip OAuth so the bearer + // header isn't pre-empted by an OAuth flow that fails (e.g. Microsoft + // Entra ID rejects RFC 7591 dynamic client registration). See #792. + const oauthExplicitlyDisabled = mcp.oauth === false + const oauthExplicitlyConfigured = typeof mcp.oauth === "object" + const oauthDisabled = + oauthExplicitlyDisabled || + (!oauthExplicitlyConfigured && hasAuthorizationHeader(mergedHeaders)) + // altimate_change end const oauthConfig = typeof mcp.oauth === "object" ? mcp.oauth : undefined let authProvider: McpOAuthProvider | undefined @@ -391,22 +584,25 @@ export namespace MCP { ) } + // altimate_change start — pass merged (static + dynamic) headers to transports + const requestInit = Object.keys(mergedHeaders).length > 0 ? { headers: mergedHeaders } : undefined const transports: Array<{ name: string; transport: TransportWithAuth }> = [ { name: "StreamableHTTP", transport: new StreamableHTTPClientTransport(new URL(mcp.url), { authProvider, - requestInit: mcp.headers ? { headers: mcp.headers } : undefined, + requestInit, }), }, { name: "SSE", transport: new SSEClientTransport(new URL(mcp.url), { authProvider, - requestInit: mcp.headers ? { headers: mcp.headers } : undefined, + requestInit, }), }, ] + // altimate_change end let lastError: Error | undefined const connectTimeout = mcp.timeout ?? DEFAULT_TIMEOUT @@ -433,8 +629,10 @@ export namespace MCP { duration_ms: Date.now() - connectStart, }) // altimate_change start — bridge merge: prefetch tool list synchronously - // for cache so MCP.tools() doesn't re-call listTools. - const toolsList = await client.listTools().catch(() => undefined) + // for cache so MCP.tools() doesn't re-call listTools. Use lenient + // schema so servers that emit `null` annotation hints (e.g. Fabric) + // don't trip Zod validation. See #792. + const toolsList = await listToolsLenient(client).catch(() => undefined) if (toolsList) toolListCache.set(key, toolsList.tools) // altimate_change end // Census: collect resource counts (fire-and-forget, never block connect) @@ -571,8 +769,9 @@ export namespace MCP { duration_ms: Date.now() - localConnectStart, }) // altimate_change start — bridge merge: prefetch tool list synchronously - // for cache so MCP.tools() doesn't re-call listTools. - const toolsListSync = await client.listTools().catch(() => undefined) + // for cache so MCP.tools() doesn't re-call listTools. Use lenient schema + // so non-compliant annotation hints (`null`) don't fail validation. + const toolsListSync = await listToolsLenient(client).catch(() => undefined) if (toolsListSync) toolListCache.set(key, toolsListSync.tools) // altimate_change end // Census: collect resource counts (fire-and-forget, never block connect) @@ -639,7 +838,7 @@ export namespace MCP { const cachedTools = toolListCache.get(key) const result = cachedTools ? { tools: cachedTools } - : await withTimeout(mcpClient.listTools(), mcp.timeout ?? DEFAULT_TIMEOUT).catch((err) => { + : await withTimeout(listToolsLenient(mcpClient), mcp.timeout ?? DEFAULT_TIMEOUT).catch((err) => { log.error("failed to get tools from client", { key, error: err }) return undefined }) @@ -836,7 +1035,7 @@ export namespace MCP { if (cached) { return { clientName, client, toolsResult: { tools: cached } } } - const toolsResult = await client.listTools().catch((e) => { + const toolsResult = await listToolsLenient(client).catch((e) => { log.error("failed to get tools", { clientName, error: e.message }) const failedStatus = { status: "failed" as const, diff --git a/packages/opencode/test/mcp/headers.test.ts b/packages/opencode/test/mcp/headers.test.ts index 8c488d4c4..8ce78dd2b 100644 --- a/packages/opencode/test/mcp/headers.test.ts +++ b/packages/opencode/test/mcp/headers.test.ts @@ -47,7 +47,7 @@ const { MCP } = await import("../../src/mcp/index") const { Instance } = await import("../../src/project/instance") const { tmpdir } = await import("../fixture/fixture") -test("headers are passed to transports when oauth is enabled (default)", async () => { +test("headers are passed to transports when oauth is enabled (default, no Authorization header)", async () => { await using tmp = await tmpdir({ init: async (dir) => { await Bun.write( @@ -59,8 +59,8 @@ test("headers are passed to transports when oauth is enabled (default)", async ( type: "remote", url: "https://example.com/mcp", headers: { - Authorization: "Bearer test-token", "X-Custom-Header": "custom-value", + "X-Trace-Id": "trace-1", }, }, }, @@ -77,8 +77,8 @@ test("headers are passed to transports when oauth is enabled (default)", async ( type: "remote", url: "https://example.com/mcp", headers: { - Authorization: "Bearer test-token", "X-Custom-Header": "custom-value", + "X-Trace-Id": "trace-1", }, }).catch(() => {}) @@ -88,15 +88,97 @@ test("headers are passed to transports when oauth is enabled (default)", async ( for (const call of transportCalls) { expect(call.options.requestInit).toBeDefined() expect(call.options.requestInit?.headers).toEqual({ - Authorization: "Bearer test-token", "X-Custom-Header": "custom-value", + "X-Trace-Id": "trace-1", + }) + // OAuth should be enabled by default when no Authorization header is provided. + expect(call.options.authProvider).toBeDefined() + } + }, + }) +}) + +// altimate_change start — covers the OAuth auto-disable behavior added for +// https://github.com/AltimateAI/altimate-code/issues/792. When the user +// supplies an explicit Authorization header (statically or via headersCommand), +// the OAuth provider is not attached, so a failing OAuth flow (e.g. Microsoft +// Entra ID rejecting RFC 7591 dynamic client registration) cannot pre-empt the +// bearer token. +test("OAuth is auto-disabled when an explicit Authorization header is present", async () => { + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + transportCalls.length = 0 + await MCP.add("auto-disable-server", { + type: "remote", + url: "https://example.com/mcp", + headers: { + Authorization: "Bearer static-token", + "X-Custom-Header": "x", + }, + }).catch(() => {}) + + expect(transportCalls.length).toBeGreaterThanOrEqual(1) + for (const call of transportCalls) { + expect(call.options.requestInit?.headers).toMatchObject({ + Authorization: "Bearer static-token", + }) + // No authProvider — OAuth was auto-disabled because user provided bearer. + expect(call.options.authProvider).toBeUndefined() + } + }, + }) +}) + +test("OAuth is auto-disabled when Authorization is supplied via headersCommand", async () => { + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + transportCalls.length = 0 + await MCP.add("auto-disable-cmd-server", { + type: "remote", + url: "https://example.com/mcp", + headersCommand: { + Authorization: ["printf", "Bearer dynamic-token"], + }, + }).catch(() => {}) + + expect(transportCalls.length).toBeGreaterThanOrEqual(1) + for (const call of transportCalls) { + expect(call.options.requestInit?.headers).toMatchObject({ + Authorization: "Bearer dynamic-token", }) - // OAuth should be enabled by default, so authProvider should exist + expect(call.options.authProvider).toBeUndefined() + } + }, + }) +}) + +test("OAuth still attaches when Authorization header is present but oauth is explicitly configured", async () => { + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + transportCalls.length = 0 + await MCP.add("explicit-oauth-server", { + type: "remote", + url: "https://example.com/mcp", + headers: { Authorization: "Bearer fallback" }, + oauth: { clientId: "client-xyz" }, + }).catch(() => {}) + + expect(transportCalls.length).toBeGreaterThanOrEqual(1) + for (const call of transportCalls) { + // User explicitly opted in to OAuth, so provider is attached even + // though a static Authorization header is also present. expect(call.options.authProvider).toBeDefined() } }, }) }) +// altimate_change end test("headers are passed to transports when oauth is explicitly disabled", async () => { await using tmp = await tmpdir() diff --git a/packages/opencode/test/mcp/mcp-bearer-auth.test.ts b/packages/opencode/test/mcp/mcp-bearer-auth.test.ts new file mode 100644 index 000000000..0f7e3a4a3 --- /dev/null +++ b/packages/opencode/test/mcp/mcp-bearer-auth.test.ts @@ -0,0 +1,377 @@ +import { describe, test, expect } from "bun:test" +import { Config } from "../../src/config/config" + +// Assert against the *production* lenient schema directly (exported via +// MCP._testing) so this test can never pass against a stale duplicate. The MCP +// module is already imported dynamically by the resolveHeadersCommand tests +// below, so pulling it in here adds no new import surface. +const { MCP } = await import("../../src/mcp") +const { LenientListToolsResultSchema } = MCP._testing + +// --------------------------------------------------------------------------- +// 1. Lenient tools/list schema accepts what real-world servers emit. +// --------------------------------------------------------------------------- +describe("lenient tools/list schema", () => { + test("accepts null annotation hints (Microsoft Fabric Core MCP behavior)", () => { + // Real payload shape we observed from https://api.fabric.microsoft.com/v1/mcp/core + const fabricStyleResponse = { + tools: [ + { + name: "list_workspaces", + description: "Lists all Microsoft fabric workspaces user has access to.", + inputSchema: { type: "object", properties: {} }, + annotations: { + title: "List Workspaces", + readOnlyHint: true, + destructiveHint: null, + idempotentHint: null, + openWorldHint: null, + }, + }, + ], + } + const result = LenientListToolsResultSchema.safeParse(fabricStyleResponse) + expect(result.success).toBe(true) + if (result.success) { + expect(result.data.tools).toHaveLength(1) + expect(result.data.tools[0].name).toBe("list_workspaces") + } + }) + + test("accepts proper boolean annotation hints (compliant servers)", () => { + const compliantResponse = { + tools: [ + { + name: "delete_workspace", + inputSchema: { type: "object" }, + annotations: { + readOnlyHint: false, + destructiveHint: true, + idempotentHint: true, + openWorldHint: false, + }, + }, + ], + } + const result = LenientListToolsResultSchema.safeParse(compliantResponse) + expect(result.success).toBe(true) + }) + + test("accepts tools without annotations at all", () => { + const result = LenientListToolsResultSchema.safeParse({ + tools: [{ name: "minimal", inputSchema: {} }], + }) + expect(result.success).toBe(true) + }) + + test("rejects malformed top-level (missing tools array)", () => { + expect(LenientListToolsResultSchema.safeParse({ tools: "not-an-array" }).success).toBe(false) + expect(LenientListToolsResultSchema.safeParse({}).success).toBe(false) + }) + + test("preserves unknown fields via .loose() (forward compatibility)", () => { + const future = { + tools: [{ name: "x", inputSchema: {}, futureField: { nested: 1 } }], + futureTopLevel: "ok", + } + const result = LenientListToolsResultSchema.safeParse(future) + expect(result.success).toBe(true) + }) +}) + +// --------------------------------------------------------------------------- +// 1b. End-to-end listToolsLenient retry (#792). Locks in the load-bearing +// contract: when the SDK's strict listTools() rejects a Fabric-style payload, +// the rejected error is named `$ZodError` (zod v4-mini), isSchemaError catches +// it, and the lenient client.request() retry returns the tools. A future +// re-narrowing of isSchemaError would break this without tripping the +// schema-only tests above. +// --------------------------------------------------------------------------- +describe("listToolsLenient retry against SDK $ZodError (#792)", () => { + test("retries with lenient schema when strict listTools() rejects Fabric-style nulls", async () => { + const { ListToolsResultSchema } = await import("@modelcontextprotocol/sdk/types.js") + const { safeParse } = await import("@modelcontextprotocol/sdk/server/zod-compat.js") + // Real payload shape from Microsoft Fabric Core MCP: null annotation hints. + const fabricPayload = { + tools: [ + { + name: "list_workspaces", + inputSchema: { type: "object", properties: {} }, + annotations: { readOnlyHint: true, destructiveHint: null, idempotentHint: null, openWorldHint: null }, + }, + ], + } + // Produce the exact error the SDK would reject with (a `$ZodError`). + const strict: any = safeParse(ListToolsResultSchema as any, fabricPayload) + expect(strict.success).toBe(false) + expect(strict.error?.name).toBe("$ZodError") + + let requestCalled = false + const fakeClient = { + listTools: async () => { + throw strict.error + }, + request: async () => { + requestCalled = true + return fabricPayload + }, + } + + const result = await MCP._testing.listToolsLenient(fakeClient) + expect(requestCalled).toBe(true) + expect(result.tools).toHaveLength(1) + expect(result.tools[0].name).toBe("list_workspaces") + }) + + test("does NOT retry (rethrows) when the error is not a schema error", async () => { + const transportError = new Error("ECONNREFUSED") + let requestCalled = false + const fakeClient = { + listTools: async () => { + throw transportError + }, + request: async () => { + requestCalled = true + return { tools: [] } + }, + } + await expect(MCP._testing.listToolsLenient(fakeClient)).rejects.toThrow(/ECONNREFUSED/) + expect(requestCalled).toBe(false) + }) +}) + +// --------------------------------------------------------------------------- +// 1c. listToolsLenient follows `nextCursor` pagination (#986) so tools beyond +// the first page are not silently dropped. Covers both the strict path and the +// case where the schema-error fallback engages mid-pagination. +// --------------------------------------------------------------------------- +describe("listToolsLenient pagination (#986)", () => { + test("follows nextCursor and concatenates tools across strict pages", async () => { + const cursors: (string | undefined)[] = [] + const pages: Record = { + "": { tools: [{ name: "a" }, { name: "b" }], nextCursor: "p2" }, + p2: { tools: [{ name: "c" }], nextCursor: "p3" }, + p3: { tools: [{ name: "d" }] }, + } + const fakeClient = { + listTools: async (params?: { cursor?: string }) => { + cursors.push(params?.cursor) + return pages[params?.cursor ?? ""] + }, + request: async () => { + throw new Error("strict path should not fall back here") + }, + } + const result = await MCP._testing.listToolsLenient(fakeClient) + expect(result.tools.map((t) => t.name)).toEqual(["a", "b", "c", "d"]) + expect(cursors).toEqual([undefined, "p2", "p3"]) + }) + + test("keeps paginating via the lenient path after a schema error, passing the cursor", async () => { + const { ListToolsResultSchema } = await import("@modelcontextprotocol/sdk/types.js") + const { safeParse } = await import("@modelcontextprotocol/sdk/server/zod-compat.js") + // Page 1 is Fabric-style (null hints) and trips strict validation; the retry + // and every later page must go through the lenient request path. + const page1 = { + tools: [ + { + name: "list_workspaces", + inputSchema: { type: "object" }, + annotations: { readOnlyHint: true, destructiveHint: null, idempotentHint: null, openWorldHint: null }, + }, + ], + nextCursor: "p2", + } + const strict: any = safeParse(ListToolsResultSchema as any, page1) + expect(strict.success).toBe(false) + + const requestCursors: (string | undefined)[] = [] + const lenientPages: Record = { + "": page1, + p2: { tools: [{ name: "second_page_tool" }] }, + } + const fakeClient = { + listTools: async (params?: { cursor?: string }) => { + // Strict path only rejects the first (cursorless) page; it is never + // retried on the strict path once we go lenient. + if (!params?.cursor) throw strict.error + throw new Error("strict path should not be reused after fallback") + }, + request: async (req: { params?: { cursor?: string } }) => { + const cursor = req.params?.cursor + requestCursors.push(cursor) + return lenientPages[cursor ?? ""] + }, + } + const result = await MCP._testing.listToolsLenient(fakeClient) + expect(result.tools.map((t) => t.name)).toEqual(["list_workspaces", "second_page_tool"]) + // Lenient path re-fetches page 1 (cursorless) then follows nextCursor to p2. + expect(requestCursors).toEqual([undefined, "p2"]) + }) +}) + +// --------------------------------------------------------------------------- +// 2. McpRemote schema accepts new headersCommand field (issue #791). +// --------------------------------------------------------------------------- +describe("McpRemote.headersCommand schema (#791)", () => { + test("accepts headersCommand as record of header → argv", () => { + const config = { + type: "remote" as const, + url: "https://example.com/mcp", + headersCommand: { + Authorization: ["az", "account", "get-access-token", "--query", "accessToken", "-o", "tsv"], + }, + } + const result = Config.McpRemote.safeParse(config) + expect(result.success).toBe(true) + }) + + test("rejects headersCommand with empty argv (would silently no-op at runtime)", () => { + const result = Config.McpRemote.safeParse({ + type: "remote", + url: "https://example.com/mcp", + headersCommand: { Authorization: [] }, + }) + expect(result.success).toBe(false) + }) + + test("rejects array-shaped headers/headersCommand with an actionable invalid_type error", () => { + // normalizeMcpConfig passes these malformed shapes through unchanged so the + // schema rejects them loudly instead of the normalizer silently dropping + // them (which would connect a header-less server with no feedback). + const headersArr = Config.McpRemote.safeParse({ type: "remote", url: "https://x/mcp", headers: ["a", "b"] }) + expect(headersArr.success).toBe(false) + const cmdArr = Config.McpRemote.safeParse({ type: "remote", url: "https://x/mcp", headersCommand: [["x"]] }) + expect(cmdArr.success).toBe(false) + }) + + test("allows static headers and headersCommand to coexist", () => { + const config = { + type: "remote" as const, + url: "https://example.com/mcp", + headers: { "X-Trace-Id": "abc" }, + headersCommand: { Authorization: ["echo", "Bearer xyz"] }, + } + const result = Config.McpRemote.safeParse(config) + expect(result.success).toBe(true) + }) + + test("headersCommand is optional (existing configs still validate)", () => { + const result = Config.McpRemote.safeParse({ + type: "remote", + url: "https://example.com/mcp", + headers: { Authorization: "Bearer static" }, + }) + expect(result.success).toBe(true) + }) +}) + +// --------------------------------------------------------------------------- +// 3. headersCommand resolution behavior (#791). +// Tests the actual helper from the MCP module. +// --------------------------------------------------------------------------- +describe("resolveHeadersCommand helper", () => { + test("returns empty object when spec is undefined", async () => { + const { MCP } = await import("../../src/mcp") + const result = await MCP._testing.resolveHeadersCommand(undefined) + expect(result).toEqual({}) + }) + + test("runs argv via execFile and uses trimmed stdout as header value", async () => { + const { MCP } = await import("../../src/mcp") + const result = await MCP._testing.resolveHeadersCommand({ + Authorization: ["printf", "Bearer hello-world"], + "X-Trace": ["printf", "trace-123\n"], + }) + expect(result.Authorization).toBe("Bearer hello-world") + expect(result["X-Trace"]).toBe("trace-123") + }) + + test("throws when command emits empty output", async () => { + const { MCP } = await import("../../src/mcp") + await expect(MCP._testing.resolveHeadersCommand({ Authorization: ["true"] })).rejects.toThrow( + /produced empty output/, + ) + }) + + test("throws when command does not exist, naming the failing header", async () => { + const { MCP } = await import("../../src/mcp") + // The error must name the specific header so `mcp list` points to the + // exact failing command rather than a bare ENOENT. + await expect( + MCP._testing.resolveHeadersCommand({ Authorization: ["this-binary-does-not-exist-xyz"] }), + ).rejects.toThrow(/headersCommand\[Authorization\] failed:/) + }) + + test("does not invoke a shell (argv is passed directly to execFile)", async () => { + // If a shell were used, the metacharacters below would be interpreted. + // execFile passes argv directly, so the literal string is echoed back. + const { MCP } = await import("../../src/mcp") + const result = await MCP._testing.resolveHeadersCommand({ + X: ["printf", "%s", "$(whoami); rm -rf /"], + }) + expect(result.X).toBe("$(whoami); rm -rf /") + }) +}) + +// --------------------------------------------------------------------------- +// 4. Authorization-header detection used to auto-disable OAuth (#792). +// --------------------------------------------------------------------------- +describe("hasAuthorizationHeader helper (#792)", () => { + test("matches case-insensitively", async () => { + const { MCP } = await import("../../src/mcp") + expect(MCP._testing.hasAuthorizationHeader({ Authorization: "Bearer x" })).toBe(true) + expect(MCP._testing.hasAuthorizationHeader({ authorization: "Bearer x" })).toBe(true) + expect(MCP._testing.hasAuthorizationHeader({ AUTHORIZATION: "Bearer x" })).toBe(true) + }) + + test("returns false when no auth header is present", async () => { + const { MCP } = await import("../../src/mcp") + expect(MCP._testing.hasAuthorizationHeader({})).toBe(false) + expect(MCP._testing.hasAuthorizationHeader({ "X-Trace": "abc" })).toBe(false) + }) + + test("does not match prefixes that merely contain 'authorization'", async () => { + const { MCP } = await import("../../src/mcp") + expect(MCP._testing.hasAuthorizationHeader({ "X-Authorization-Type": "Bearer" })).toBe(false) + expect(MCP._testing.hasAuthorizationHeader({ "Pre-Authorization": "x" })).toBe(false) + }) +}) + +// --------------------------------------------------------------------------- +// 5. normalizeMcpConfig preserves headersCommand and oauth (round-trip). +// +// Without this, the field-stripping normalizer drops user-supplied values +// silently, leaving the runtime to behave as if the user hadn't configured +// them. See #791 / #792. +// --------------------------------------------------------------------------- +describe("config normalize round-trip", () => { + test("McpRemote with headersCommand survives Mcp parse", () => { + // Simulates the post-normalize entry: with our fix, the load path + // forwards `headersCommand` through into the typed shape. + const entry = { + type: "remote", + url: "https://example.com/mcp", + headersCommand: { Authorization: ["echo", "Bearer x"] }, + } + const result = Config.Mcp.safeParse(entry) + expect(result.success).toBe(true) + if (result.success && result.data.type === "remote") { + expect(result.data.headersCommand).toEqual({ Authorization: ["echo", "Bearer x"] }) + } + }) + + test("McpRemote with oauth=false survives Mcp parse", () => { + const entry = { + type: "remote", + url: "https://example.com/mcp", + headers: { Authorization: "Bearer x" }, + oauth: false, + } + const result = Config.Mcp.safeParse(entry) + expect(result.success).toBe(true) + if (result.success && result.data.type === "remote") { + expect(result.data.oauth).toBe(false) + } + }) +})