From 91e08b9720fb3c3ef94799d616651b647bfca06d Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 15 Oct 2026 21:46:57 -0700 Subject: [PATCH] refactor(mcp): drop result projection and duplicate tool-call paths --- e2e/mcp-late-dispatch.test.ts | 11 ++ e2e/subagent-permission.test.ts | 32 ++- src/mcp/add-server.ts | 134 ++++--------- src/mcp/client.ts | 63 ++---- src/mcp/plugin.test.ts | 180 ++++------------- src/mcp/plugin.ts | 56 +----- src/mcp/quiet.ts | 14 ++ src/mcp/result-projection.test.ts | 313 ------------------------------ src/mcp/result-projection.ts | 265 ------------------------- src/tools/web-fetch.test.ts | 15 +- src/tools/web-fetch.ts | 5 +- src/tools/web-search.test.ts | 11 +- src/tools/web-search.ts | 13 +- testkit/mcp-connect-mock.ts | 11 ++ 14 files changed, 190 insertions(+), 933 deletions(-) create mode 100644 src/mcp/quiet.ts delete mode 100644 src/mcp/result-projection.test.ts delete mode 100644 src/mcp/result-projection.ts diff --git a/e2e/mcp-late-dispatch.test.ts b/e2e/mcp-late-dispatch.test.ts index b9f291072..ad2a9b8e4 100644 --- a/e2e/mcp-late-dispatch.test.ts +++ b/e2e/mcp-late-dispatch.test.ts @@ -55,6 +55,17 @@ function linearClient(opts: { }, ], call: opts.call ?? (async () => "ISSUE-1"), + async callResult(toolName, args, signal) { + const text = await (opts.call ?? (async () => "ISSUE-1"))( + toolName, + args, + signal, + ); + return { + blocks: [{ type: "text", text }], + isError: false, + }; + }, async close() { return undefined; }, diff --git a/e2e/subagent-permission.test.ts b/e2e/subagent-permission.test.ts index 341b1885f..698b2379e 100644 --- a/e2e/subagent-permission.test.ts +++ b/e2e/subagent-permission.test.ts @@ -296,9 +296,13 @@ test.serial( inputSchema: { type: "object", properties: {} }, }, ], - call: async () => { + call: async () => "changed", + callResult: async () => { calls++; - return "changed"; + return { + blocks: [{ type: "text", text: "changed" }], + isError: false, + }; }, close: async () => undefined, }; @@ -350,9 +354,13 @@ test.serial( inputSchema: { type: "object", properties: {} }, }, ], - call: async () => { + call: async () => "changed", + callResult: async () => { calls++; - return "changed"; + return { + blocks: [{ type: "text", text: "changed" }], + isError: false, + }; }, close: async () => undefined, }; @@ -480,9 +488,13 @@ test.serial( inputSchema: { type: "object", properties: {} }, }, ], - call: async () => { + call: async () => "changed", + callResult: async () => { calls++; - return "changed"; + return { + blocks: [{ type: "text", text: "changed" }], + isError: false, + }; }, close: async () => undefined, }; @@ -561,9 +573,13 @@ test("storing parent-gated MCP tools then wrapping again denies without requestA inputSchema: { type: "object", properties: {} }, }, ], - call: async () => { + call: async () => "changed", + callResult: async () => { calls++; - return "changed"; + return { + blocks: [{ type: "text", text: "changed" }], + isError: false, + }; }, close: async () => undefined, }; diff --git a/src/mcp/add-server.ts b/src/mcp/add-server.ts index a68e09a68..d3a9b96e7 100644 --- a/src/mcp/add-server.ts +++ b/src/mcp/add-server.ts @@ -14,44 +14,34 @@ import { import { deleteAuthState } from "./auth-store.js"; import { EXA_MCP_SERVER_NAME } from "./exa.js"; -export interface GlobalSettingsWriter { - readonly enqueue: (job: () => Promise) => Promise; - readonly update: ( - apply: (base: Settings) => Settings | null, - ) => Promise; +interface SettingsWriter { + readonly enqueue: (job: () => Promise) => Promise; + readonly update: (apply: (base: T) => T | null) => Promise; readonly updateAt: ( path: string, - apply: (base: Settings) => Settings | null, - ) => Promise; - readonly mutate: ( - apply: (base: Settings) => Settings | null, - ) => Promise<"ok" | "skipped">; + apply: (base: T) => T | null, + ) => Promise; + readonly mutate: (apply: (base: T) => T | null) => Promise<"ok" | "skipped">; readonly mutateAt: ( path: string, - apply: (base: Settings) => Settings | null, + apply: (base: T) => T | null, ) => Promise<"ok" | "skipped">; } -interface GlobalSettingsWriterDeps { - readonly load: (path: string) => Promise; - readonly save: (path: string, settings: Settings) => Promise; -} +export type GlobalSettingsWriter = SettingsWriter; +export type LocalSettingsWriter = SettingsWriter; -const defaultWriterDeps: GlobalSettingsWriterDeps = { - load: loadGlobalSettingsWriteBase, - save: saveGlobalSettings, -}; +interface SettingsWriterDeps { + readonly load: (path: string) => Promise; + readonly save: (path: string, settings: T) => Promise; +} -export function createGlobalSettingsWriter( +function createSettingsWriter( path: string, - deps: Partial = {}, -): GlobalSettingsWriter { - const writerDeps: GlobalSettingsWriterDeps = { - ...defaultWriterDeps, - ...deps, - }; + deps: SettingsWriterDeps, +): SettingsWriter { let tail = Promise.resolve(); - const enqueue = (job: () => Promise): Promise => { + const enqueue = (job: () => Promise): Promise => { const run = tail.then(job); tail = run.then( () => undefined, @@ -62,20 +52,20 @@ export function createGlobalSettingsWriter( const updateAt = ( settingsPath: string, - apply: (base: Settings) => Settings | null, - ): Promise => + apply: (base: T) => T | null, + ): Promise => enqueue(async () => { - const base = await writerDeps.load(settingsPath); + const base = await deps.load(settingsPath); if (base === null) return null; const next = apply(base); if (next === null) return base; - await writerDeps.save(settingsPath, next); + await deps.save(settingsPath, next); return next; }); const mutateAt = async ( settingsPath: string, - apply: (base: Settings) => Settings | null, + apply: (base: T) => T | null, ): Promise<"ok" | "skipped"> => (await updateAt(settingsPath, apply)) === null ? "skipped" : "ok"; @@ -88,78 +78,26 @@ export function createGlobalSettingsWriter( }; } -export interface LocalSettingsWriter { - readonly enqueue: (job: () => Promise) => Promise; - readonly update: ( - apply: (base: LocalSettings) => LocalSettings | null, - ) => Promise; - readonly updateAt: ( - path: string, - apply: (base: LocalSettings) => LocalSettings | null, - ) => Promise; - readonly mutate: ( - apply: (base: LocalSettings) => LocalSettings | null, - ) => Promise<"ok" | "skipped">; - readonly mutateAt: ( - path: string, - apply: (base: LocalSettings) => LocalSettings | null, - ) => Promise<"ok" | "skipped">; -} - -interface LocalSettingsWriterDeps { - readonly load: (path: string) => Promise; - readonly save: (path: string, settings: LocalSettings) => Promise; +export function createGlobalSettingsWriter( + path: string, + deps: Partial> = {}, +): GlobalSettingsWriter { + return createSettingsWriter(path, { + load: loadGlobalSettingsWriteBase, + save: saveGlobalSettings, + ...deps, + }); } -const defaultLocalWriterDeps: LocalSettingsWriterDeps = { - load: loadLocalSettingsWriteBase, - save: saveLocalSettings, -}; - export function createLocalSettingsWriter( path: string, - deps: Partial = {}, + deps: Partial> = {}, ): LocalSettingsWriter { - const writerDeps: LocalSettingsWriterDeps = { - ...defaultLocalWriterDeps, + return createSettingsWriter(path, { + load: loadLocalSettingsWriteBase, + save: saveLocalSettings, ...deps, - }; - let tail = Promise.resolve(); - const enqueue = (job: () => Promise): Promise => { - const run = tail.then(job); - tail = run.then( - () => undefined, - () => undefined, - ); - return run; - }; - - const updateAt = ( - settingsPath: string, - apply: (base: LocalSettings) => LocalSettings | null, - ): Promise => - enqueue(async () => { - const base = await writerDeps.load(settingsPath); - if (base === null) return null; - const next = apply(base); - if (next === null) return base; - await writerDeps.save(settingsPath, next); - return next; - }); - - const mutateAt = async ( - settingsPath: string, - apply: (base: LocalSettings) => LocalSettings | null, - ): Promise<"ok" | "skipped"> => - (await updateAt(settingsPath, apply)) === null ? "skipped" : "ok"; - - return { - enqueue, - update: (apply) => updateAt(path, apply), - updateAt, - mutate: (apply) => mutateAt(path, apply), - mutateAt, - }; + }); } export type PersistMCPServerResult = diff --git a/src/mcp/client.ts b/src/mcp/client.ts index 254c49e19..899126cfe 100644 --- a/src/mcp/client.ts +++ b/src/mcp/client.ts @@ -26,6 +26,7 @@ import { } from "../telemetry/product-events.js"; import { liveTelemetry } from "../telemetry/singleton.js"; import { MCP_CLIENT_NAME } from "../branding.js"; +import { closeQuietly, isAbortError } from "./quiet.js"; export interface MCPTool { name: string; @@ -60,16 +61,9 @@ export interface MCPClient { args: Record, signal: AbortSignal, ): Promise; - /** Validated content blocks before flattening — for post-policy - * archive capture. */ - callBlocks?( - toolName: string, - args: Record, - signal: AbortSignal, - ): Promise; /** Full tool-result envelope: blocks plus tool-level * isError/structuredContent. */ - callResult?( + callResult( toolName: string, args: Record, signal: AbortSignal, @@ -176,15 +170,6 @@ function isRecoverableAuthError( return err instanceof UnauthorizedError || err instanceof OAuthError; } -function isAbortError(err: unknown): boolean { - return ( - typeof err === "object" && - err !== null && - "name" in err && - err.name === "AbortError" - ); -} - export const MAX_BROWSER_AUTH_ATTEMPTS = 1; export const BROWSER_AUTH_COOLDOWN_MS = 5 * 60_000; export const BROWSER_AUTH_WAIT_MS = 2 * 60_000; @@ -612,12 +597,13 @@ async function finishClient( serverName, tools, async callResult(toolName, args, signal) { - const context = - authContext === undefined ? undefined : { ...authContext, signal }; - const result = await withHTTPAuthorizationRecovery(context, () => - client.callTool({ name: toolName, arguments: args }, undefined, { - signal, - }), + const result = await withHTTPAuthorizationRecovery( + authContext, + () => + client.callTool({ name: toolName, arguments: args }, undefined, { + signal, + }), + signal, ); const envelope: MCPToolResultEnvelope = { blocks: validateMcpContentBlocks(result.content), @@ -634,34 +620,17 @@ async function finishClient( } return envelope; }, - async callBlocks(toolName, args, signal) { - const result = await withHTTPAuthorizationRecovery( - authContext, - () => - client.callTool({ name: toolName, arguments: args }, undefined, { - signal, - }), - signal, - ); - return validateMcpContentBlocks(result.content); - }, - async call(toolName, args, signal) { - const result = await withHTTPAuthorizationRecovery( - authContext, - () => - client.callTool({ name: toolName, arguments: args }, undefined, { - signal, - }), - signal, + async call(this: MCPClient, toolName, args, signal) { + return unwrapToolContent( + (await this.callResult(toolName, args, signal)).blocks, ); - return unwrapToolContent(validateMcpContentBlocks(result.content)); }, async close() { // Disarm first: the owner's hook reports unintentional death only. if (liveTransport !== undefined) liveTransport.onclose = () => undefined; closeLifecycle?.(); authContext?.callback.close(); - await client.close().catch(() => undefined); + await closeQuietly(() => client.close()); }, }; } @@ -730,7 +699,7 @@ async function connectStdio( ), }; } catch (err) { - await client.close().catch(() => undefined); + await closeQuietly(() => client.close()); return { ok: false, serverName: config.name, @@ -798,7 +767,7 @@ async function connectHttp( signal: lifecycle.signal, }); } finally { - await probeClient.close().catch(() => undefined); + await closeQuietly(() => probeClient.close()); } }, }; @@ -855,7 +824,7 @@ async function connectHttp( }; } catch (err) { closeLifecycle(); - await client?.close().catch(() => undefined); + await closeQuietly(() => client?.close() ?? Promise.resolve()); try { callback?.close(); } catch { diff --git a/src/mcp/plugin.test.ts b/src/mcp/plugin.test.ts index 9296a9bd2..e8e10202f 100644 --- a/src/mcp/plugin.test.ts +++ b/src/mcp/plugin.test.ts @@ -32,13 +32,15 @@ function fakeEnvelopeClient(envelope: ScriptedMcpEnvelope): MCPClient { }, ], call: async () => "", - callBlocks: async () => envelope.blocks, + callResult: async () => ({ + blocks: envelope.blocks, + isError: envelope.isError === true, + ...(envelope.structuredContent !== undefined + ? { structuredContent: envelope.structuredContent } + : {}), + }), close: async () => undefined, }; - // callResult is the new envelope channel (GREEN); absent on RED code. - Object.assign(client, { - callResult: async () => envelope, - }); return client; } @@ -68,7 +70,10 @@ function fakeClient(reply: string): MCPClient { }, ], call: async () => reply, - callBlocks: async () => [{ type: "text", text: reply }], + callResult: async () => ({ + blocks: [{ type: "text", text: reply }], + isError: false, + }), close: async () => undefined, }; } @@ -568,7 +573,7 @@ describe("mcpClientToAgentTools", () => { expect(payloads.map((payload) => payload.isError)).toEqual([false, true]); }); - test("falls back to legacy call when block and envelope methods are absent", async () => { + test("returns callResult text when call is also present", async () => { let calls = 0; const client: MCPClient = { serverName: "legacy", @@ -579,9 +584,13 @@ describe("mcpClientToAgentTools", () => { inputSchema: { type: "object", properties: {} }, }, ], - call: async () => { + call: async () => "unused", + callResult: async () => { calls++; - return "legacy response"; + return { + blocks: [{ type: "text", text: "legacy response" }], + isError: false, + }; }, close: async () => undefined, }; @@ -614,16 +623,11 @@ describe("mcpClientToAgentTools", () => { call: async () => { throw new Error("transport exploded"); }, - callBlocks: async () => { + callResult: async () => { throw new Error("transport exploded"); }, close: async () => undefined, }; - Object.assign(client, { - callResult: async () => { - throw new Error("transport exploded"); - }, - }); const [tool] = mcpClientToAgentTools(client, gate); if (tool?.kind !== "full") throw new Error("expected full tool"); @@ -640,7 +644,7 @@ describe("mcpClientToAgentTools", () => { const gate = skipGate(); const client: MCPClient = { ...fakeClient("unused"), - callBlocks: (_tool, _args, signal) => + callResult: (_tool, _args, signal) => new Promise((_resolve, reject) => { if (signal.aborted) { reject(signal.reason ?? new Error("aborted")); @@ -669,134 +673,6 @@ describe("mcpClientToAgentTools", () => { }); }); -describe("mcpClientToAgentTools Linear result projection", () => { - const fullIssue = { - id: "issue-uuid", - identifier: "CL-9495", - url: "https://linear.app/corbits/issue/CL-9495", - title: "Project MCP results", - status: "In Progress", - description: "# long body that must not echo", - }; - - function linearClient( - toolName: string, - envelope: ScriptedMcpEnvelope, - ): MCPClient { - const client: MCPClient = { - serverName: "linear", - tools: [ - { - name: toolName, - description: toolName, - inputSchema: { type: "object", properties: {} }, - }, - ], - call: async () => "", - callBlocks: async () => envelope.blocks, - close: async () => undefined, - }; - Object.assign(client, { - callResult: async () => envelope, - }); - return client; - } - - async function runLinear( - toolName: string, - envelope: ScriptedMcpEnvelope, - args: Record = {}, - spillOptions?: Parameters[2], - ) { - const [tool] = mcpClientToAgentTools( - linearClient(toolName, envelope), - skipGate(), - spillOptions, - ); - if (tool?.kind !== "full") throw new Error("expected full tool"); - return tool.handler( - { id: "c-linear", name: `mcp__linear__${toolName}`, arguments: args }, - new AbortController().signal, - ); - } - - test("save_issue returns id url status title and not the body just sent", async () => { - const result = await runLinear("save_issue", { - blocks: [{ type: "text", text: JSON.stringify(fullIssue) }], - }); - const parsed = JSON.parse(String(result.content)) as Record< - string, - unknown - >; - expect(parsed).toEqual({ - id: "issue-uuid", - identifier: "CL-9495", - url: "https://linear.app/corbits/issue/CL-9495", - status: "In Progress", - title: "Project MCP results", - }); - expect(String(result.content)).not.toContain("long body"); - }); - - test("list_issues is short by default and expands when fields asks", async () => { - const envelope: ScriptedMcpEnvelope = { - blocks: [ - { - type: "text", - text: JSON.stringify({ issues: [fullIssue], hasNextPage: false }), - }, - ], - }; - const shortResult = await runLinear("list_issues", envelope, {}); - const shortParsed = JSON.parse(String(shortResult.content)) as { - issues: Record[]; - hasNextPage: boolean; - }; - expect(shortParsed.hasNextPage).toBe(false); - expect(shortParsed.issues[0]).toEqual({ - id: "issue-uuid", - identifier: "CL-9495", - url: "https://linear.app/corbits/issue/CL-9495", - status: "In Progress", - title: "Project MCP results", - }); - expect(String(shortResult.content)).not.toContain("long body"); - - const expanded = await runLinear("list_issues", envelope, { - fields: ["id", "description"], - }); - expect(JSON.parse(String(expanded.content))).toEqual({ - issues: [ - { - id: "issue-uuid", - description: "# long body that must not echo", - }, - ], - hasNextPage: false, - }); - }); - - test("archives the full entity and returns the projection to the model", async () => { - const { archive } = memoryEvidenceArchive(); - const result = await runLinear( - "save_issue", - { - blocks: [{ type: "text", text: JSON.stringify(fullIssue) }], - structuredContent: fullIssue, - }, - {}, - { getEvidenceArchive: () => archive }, - ); - expect(String(result.content)).not.toContain("long body"); - const [occurrence] = await archive.listOccurrences(); - if (occurrence === undefined) throw new Error("missing archive occurrence"); - const payload = await archive.readAuthorizedPayload( - occurrence.occurrenceId, - ); - expect(payload).toContain("long body"); - }); -}); - function makeFakeClient(serverName: string, toolNames: string[]): MCPClient { return { serverName, @@ -808,6 +684,12 @@ function makeFakeClient(serverName: string, toolNames: string[]): MCPClient { async call() { return "result"; }, + async callResult() { + return { + blocks: [{ type: "text", text: "result" }], + isError: false, + }; + }, async close() { return undefined; }, @@ -828,10 +710,16 @@ describe("mcpClientToAgentTools (production gated path)", () => { inputSchema: { type: "object" }, }, ], - async call(toolName, args) { + async call() { + return ""; + }, + async callResult(toolName, args) { capturedName = toolName; capturedArgs = args; - return "done"; + return { + blocks: [{ type: "text", text: "done" }], + isError: false, + }; }, async close() { return undefined; diff --git a/src/mcp/plugin.ts b/src/mcp/plugin.ts index 2ecfe54fa..c5a177db3 100644 --- a/src/mcp/plugin.ts +++ b/src/mcp/plugin.ts @@ -17,9 +17,7 @@ import { unwrapToolContent, type MCPClient, type MCPContentBlock, - type MCPToolResultEnvelope, } from "./client.js"; -import { applyMcpResultProjection } from "./result-projection.js"; import { mcpToolName } from "./tool-name.js"; import { LOG_NAMESPACE_ROOT } from "../branding.js"; @@ -109,31 +107,11 @@ export function mcpClientTools( signal: AbortSignal, ): Promise => { try { - const envelope: MCPToolResultEnvelope = - typeof client.callResult === "function" - ? await client.callResult(tool.name, call.arguments, signal) - : typeof client.callBlocks === "function" - ? { - blocks: await client.callBlocks( - tool.name, - call.arguments, - signal, - ), - isError: false, - } - : { - blocks: [ - { - type: "text", - text: await client.call( - tool.name, - call.arguments, - signal, - ), - } satisfies MCPContentBlock, - ], - isError: false, - }; + const envelope = await client.callResult( + tool.name, + call.arguments, + signal, + ); const authorizedBlocks = applyPolicyToBlocks(envelope.blocks); const scrubbedStructured = envelope.structuredContent === undefined @@ -168,33 +146,17 @@ export function mcpClientTools( logger.warn`evidence archive skipped for successful MCP tool result call=${call.id} error=${scrubSecretShapedContent(message)}`; } } - const projected = isError - ? { - blocks: authorizedBlocks, - ...(scrubbedStructured !== undefined - ? { structuredContent: scrubbedStructured } - : {}), - } - : applyMcpResultProjection({ - serverName: client.serverName, - toolName: tool.name, - args: call.arguments, - blocks: authorizedBlocks, - ...(scrubbedStructured !== undefined - ? { structuredContent: scrubbedStructured } - : {}), - }); const serializedStructured = - projected.structuredContent === undefined + scrubbedStructured === undefined ? undefined - : serializeStructuredContent(projected.structuredContent); - const flattened = unwrapToolContent(projected.blocks); + : serializeStructuredContent(scrubbedStructured); + const flattened = unwrapToolContent(authorizedBlocks); const baseContent = flattened !== "" ? flattened : serializedStructured !== undefined ? `${MCP_STRUCTURED_CONTENT_MARKER}\n${serializedStructured.serialized}` - : projected.structuredContent !== undefined + : scrubbedStructured !== undefined ? `${MCP_STRUCTURED_CONTENT_MARKER}\n[structured content unavailable]` : isError ? `MCP tool ${client.serverName}/${tool.name} reported an error with empty content.` diff --git a/src/mcp/quiet.ts b/src/mcp/quiet.ts new file mode 100644 index 000000000..eebfeee0e --- /dev/null +++ b/src/mcp/quiet.ts @@ -0,0 +1,14 @@ +export function isAbortError(err: unknown): boolean { + return ( + typeof err === "object" && + err !== null && + "name" in err && + err.name === "AbortError" + ); +} + +export async function closeQuietly( + close: () => Promise, +): Promise { + await close().catch(() => undefined); +} diff --git a/src/mcp/result-projection.test.ts b/src/mcp/result-projection.test.ts deleted file mode 100644 index 621c835dd..000000000 --- a/src/mcp/result-projection.test.ts +++ /dev/null @@ -1,313 +0,0 @@ -import { describe, expect, test } from "bun:test"; -import { - applyMcpResultProjection, - MCP_DEFAULT_ENTITY_FIELDS, - projectMcpJsonValue, -} from "./result-projection.js"; - -const ISSUE = { - id: "issue-uuid", - identifier: "CL-9495", - url: "https://linear.app/corbits/issue/CL-9495", - title: "Project MCP results", - status: { - id: "state-1", - name: "In Progress", - type: "started", - color: "#f00", - }, - description: "# long body that must not echo", - body: "also not this", - extra: { nested: "drop me" }, -}; - -describe("projectMcpJsonValue", () => { - test("write-shaped record keeps id url status title and drops the body", () => { - const projected = projectMcpJsonValue(ISSUE, MCP_DEFAULT_ENTITY_FIELDS); - expect(projected).toEqual({ - id: "issue-uuid", - identifier: "CL-9495", - url: "https://linear.app/corbits/issue/CL-9495", - status: { id: "state-1", name: "In Progress", type: "started" }, - title: "Project MCP results", - }); - expect(JSON.stringify(projected)).not.toContain("long body"); - expect(JSON.stringify(projected)).not.toContain("drop me"); - }); - - test("aliases Linear state and name onto status and title", () => { - const projected = projectMcpJsonValue( - { - id: "c1", - identifier: "CL-1", - url: "https://linear.app/c1", - state: { name: "In Review" }, - name: "Label", - description: "nope", - }, - MCP_DEFAULT_ENTITY_FIELDS, - ); - expect(projected).toEqual({ - id: "c1", - identifier: "CL-1", - url: "https://linear.app/c1", - status: { name: "In Review" }, - state: { name: "In Review" }, - title: "Label", - name: "Label", - }); - }); - - test("list envelope projects each row and keeps pagination siblings", () => { - const projected = projectMcpJsonValue( - { - issues: [ - ISSUE, - { ...ISSUE, id: "two", identifier: "CL-2", description: "x" }, - ], - hasNextPage: true, - cursor: "next", - }, - MCP_DEFAULT_ENTITY_FIELDS, - ); - expect(projected).toEqual({ - issues: [ - { - id: "issue-uuid", - identifier: "CL-9495", - url: "https://linear.app/corbits/issue/CL-9495", - status: { id: "state-1", name: "In Progress", type: "started" }, - title: "Project MCP results", - }, - { - id: "two", - identifier: "CL-2", - url: "https://linear.app/corbits/issue/CL-9495", - status: { id: "state-1", name: "In Progress", type: "started" }, - title: "Project MCP results", - }, - ], - hasNextPage: true, - cursor: "next", - }); - }); - - test("empty list envelope keeps the collection and pagination siblings", () => { - const projected = projectMcpJsonValue( - { - issues: [], - hasNextPage: false, - cursor: null, - }, - MCP_DEFAULT_ENTITY_FIELDS, - ); - expect(projected).toEqual({ - issues: [], - hasNextPage: false, - cursor: null, - }); - }); - - test("explicit fields expand to those keys including description", () => { - const projected = projectMcpJsonValue( - ISSUE, - ["id", "description"], - false, - false, - ); - expect(projected).toEqual({ - id: "issue-uuid", - description: "# long body that must not echo", - }); - }); -}); - -describe("applyMcpResultProjection", () => { - test("save_issue projects text JSON and structured content", () => { - const result = applyMcpResultProjection({ - serverName: "linear", - toolName: "save_issue", - args: { title: "Project MCP results", description: "# long body" }, - blocks: [{ type: "text", text: JSON.stringify(ISSUE) }], - structuredContent: ISSUE, - }); - const text = result.blocks[0]?.text ?? ""; - expect(JSON.parse(text)).toEqual({ - id: "issue-uuid", - identifier: "CL-9495", - url: "https://linear.app/corbits/issue/CL-9495", - status: { id: "state-1", name: "In Progress", type: "started" }, - title: "Project MCP results", - }); - expect(text).not.toContain("long body"); - expect(result.structuredContent).toEqual({ - id: "issue-uuid", - identifier: "CL-9495", - url: "https://linear.app/corbits/issue/CL-9495", - status: { id: "state-1", name: "In Progress", type: "started" }, - title: "Project MCP results", - }); - }); - - test("save_comment drops the body just sent", () => { - const result = applyMcpResultProjection({ - serverName: "claude_ai_Linear", - toolName: "save_comment", - args: { issueId: "CL-9495", body: "done" }, - blocks: [ - { - type: "text", - text: JSON.stringify({ - id: "comment-1", - url: "https://linear.app/comment-1", - body: "done", - }), - }, - ], - }); - expect(JSON.parse(result.blocks[0]?.text ?? "")).toEqual({ - id: "comment-1", - url: "https://linear.app/comment-1", - }); - }); - - test("save_issue_label keeps identity fields and drops color dumps", () => { - const result = applyMcpResultProjection({ - serverName: "acme", - toolName: "save_issue_label", - args: { name: "bug" }, - blocks: [ - { - type: "text", - text: JSON.stringify({ - id: "label-1", - name: "bug", - color: "#ff0000", - description: "a label description", - }), - }, - ], - }); - expect(JSON.parse(result.blocks[0]?.text ?? "")).toEqual({ - id: "label-1", - title: "bug", - name: "bug", - }); - }); - - test("list_issues is short by default", () => { - const result = applyMcpResultProjection({ - serverName: "linear", - toolName: "list_issues", - args: { team: "COR" }, - blocks: [ - { - type: "text", - text: JSON.stringify({ issues: [ISSUE], hasNextPage: false }), - }, - ], - }); - const parsed = JSON.parse(result.blocks[0]?.text ?? "") as { - issues: Record[]; - }; - expect(parsed.issues[0]).not.toHaveProperty("description"); - expect(parsed.issues[0]?.id).toBe("issue-uuid"); - expect(parsed.issues[0]?.title).toBe("Project MCP results"); - }); - - test("list_issues with fields keeps the requested expansion", () => { - const result = applyMcpResultProjection({ - serverName: "linear", - toolName: "list_issues", - args: { fields: ["id", "description"] }, - blocks: [ - { - type: "text", - text: JSON.stringify({ issues: [ISSUE] }), - }, - ], - }); - expect(JSON.parse(result.blocks[0]?.text ?? "")).toEqual({ - issues: [ - { - id: "issue-uuid", - description: "# long body that must not echo", - }, - ], - }); - }); - - test("list_issues with fields all leaves the full payload", () => { - const payload = { issues: [ISSUE], hasNextPage: false }; - const result = applyMcpResultProjection({ - serverName: "linear", - toolName: "list_issues", - args: { fields: "all" }, - blocks: [{ type: "text", text: JSON.stringify(payload) }], - }); - expect(JSON.parse(result.blocks[0]?.text ?? "")).toEqual(payload); - }); - - test("empty list_issues envelope is not wiped to an empty object", () => { - const payload = { issues: [], hasNextPage: false, cursor: null }; - const result = applyMcpResultProjection({ - serverName: "linear", - toolName: "list_issues", - args: {}, - blocks: [{ type: "text", text: JSON.stringify(payload) }], - structuredContent: payload, - }); - expect(JSON.parse(result.blocks[0]?.text ?? "")).toEqual(payload); - expect(result.structuredContent).toEqual(payload); - }); - - test("list_comments keeps a projected structuredContent array", () => { - const comments = [ - { - id: "comment-1", - url: "https://linear.app/comment-1", - body: "done", - extra: "drop", - }, - { - id: "comment-2", - url: "https://linear.app/comment-2", - body: "also done", - }, - ]; - const result = applyMcpResultProjection({ - serverName: "linear", - toolName: "list_comments", - args: {}, - blocks: [{ type: "text", text: JSON.stringify(comments) }], - structuredContent: comments, - }); - const projected = [ - { id: "comment-1", url: "https://linear.app/comment-1" }, - { id: "comment-2", url: "https://linear.app/comment-2" }, - ]; - expect(JSON.parse(result.blocks[0]?.text ?? "")).toEqual(projected); - expect(result.structuredContent).toEqual(projected); - }); - - test("get_issue is not projected", () => { - const result = applyMcpResultProjection({ - serverName: "linear", - toolName: "get_issue", - args: { id: "CL-9495" }, - blocks: [{ type: "text", text: JSON.stringify(ISSUE) }], - }); - expect(JSON.parse(result.blocks[0]?.text ?? "")).toEqual(ISSUE); - }); - - test("non-Linear list tools are not projected", () => { - const payload = { items: [{ id: "1", description: "keep" }] }; - const result = applyMcpResultProjection({ - serverName: "github", - toolName: "list_issues", - args: {}, - blocks: [{ type: "text", text: JSON.stringify(payload) }], - }); - expect(JSON.parse(result.blocks[0]?.text ?? "")).toEqual(payload); - }); -}); diff --git a/src/mcp/result-projection.ts b/src/mcp/result-projection.ts deleted file mode 100644 index 4bb1c3c07..000000000 --- a/src/mcp/result-projection.ts +++ /dev/null @@ -1,265 +0,0 @@ -import type { MCPContentBlock } from "./client.js"; - -/** - * Default model-facing keys for Linear results. `id` stays so callers that - * only need the issue id work; body/description do not. - */ -export const MCP_DEFAULT_ENTITY_FIELDS = [ - "id", - "identifier", - "url", - "status", - "title", -] as const; - -const LINEAR_WRITE_TOOLS = new Set([ - "save_issue", - "save_comment", - "save_issue_label", -]); - -const DEFAULT_FIELD_ALIASES: Readonly> = { - status: ["state"], - title: ["name"], -}; - -const NESTED_REF_FIELDS = ["id", "identifier", "name", "type"] as const; - -export interface McpProjectionInput { - serverName: string; - toolName: string; - args: Record; - blocks: MCPContentBlock[]; - structuredContent?: Record | unknown[]; -} - -export interface McpProjectionOutput { - blocks: MCPContentBlock[]; - structuredContent?: Record | unknown[]; -} - -export function applyMcpResultProjection( - input: McpProjectionInput, -): McpProjectionOutput { - const fields = resolveProjectionFields( - input.serverName, - input.toolName, - input.args, - ); - if (fields === undefined) { - return { - blocks: input.blocks, - ...(input.structuredContent !== undefined - ? { structuredContent: input.structuredContent } - : {}), - }; - } - const shortenNestedRefs = fields.kind === "default"; - const applyAliases = fields.kind === "default"; - const blocks = input.blocks.map((block) => - projectBlock(block, fields.keys, shortenNestedRefs, applyAliases), - ); - const structuredContent = - input.structuredContent === undefined - ? undefined - : asProjectedStructured( - projectMcpJsonValue( - input.structuredContent, - fields.keys, - shortenNestedRefs, - applyAliases, - ), - ); - return { - blocks, - ...(structuredContent !== undefined ? { structuredContent } : {}), - }; -} - -export function projectMcpJsonValue( - value: unknown, - fields: readonly string[], - shortenNestedRefs = true, - applyAliases = true, -): unknown { - if (Array.isArray(value)) { - return value.map((item) => - isRecord(item) - ? pickEntityFields(item, fields, shortenNestedRefs, applyAliases) - : item, - ); - } - if (!isRecord(value)) return value; - - if (isListEnvelope(value)) { - const out = Object.create(null) as Record; - for (const [key, item] of Object.entries(value)) { - if (isProjectedList(item)) { - out[key] = item.map((entry) => - isRecord(entry) - ? pickEntityFields(entry, fields, shortenNestedRefs, applyAliases) - : entry, - ); - } else { - out[key] = item; - } - } - return out; - } - return pickEntityFields(value, fields, shortenNestedRefs, applyAliases); -} - -function resolveProjectionFields( - serverName: string, - toolName: string, - args: Record, -): { kind: "default" | "fields"; keys: readonly string[] } | undefined { - if (LINEAR_WRITE_TOOLS.has(toolName)) { - return { kind: "default", keys: MCP_DEFAULT_ENTITY_FIELDS }; - } - if (!isLinearServer(serverName) || !toolName.startsWith("list_")) { - return undefined; - } - const requested = requestedFields(args.fields); - if (requested === "passthrough") return undefined; - if (requested === undefined) { - return { kind: "default", keys: MCP_DEFAULT_ENTITY_FIELDS }; - } - return { kind: "fields", keys: requested }; -} - -function isLinearServer(serverName: string): boolean { - return /linear/i.test(serverName); -} - -function requestedFields( - fields: unknown, -): readonly string[] | "passthrough" | undefined { - if (fields === undefined || fields === null) return undefined; - if (fields === true) return "passthrough"; - if (typeof fields === "string") { - const trimmed = fields.trim(); - if (trimmed.length === 0) return undefined; - if (trimmed === "*" || trimmed.toLowerCase() === "all") - return "passthrough"; - const parts = trimmed.split(/[\s,]+/).filter((part) => part.length > 0); - return parts.length === 0 ? undefined : parts; - } - if (Array.isArray(fields)) { - if (fields.length === 0) return undefined; - if (fields.some((item) => item === "*" || item === true)) { - return "passthrough"; - } - const names = fields.filter( - (item): item is string => typeof item === "string" && item.length > 0, - ); - return names.length === 0 ? "passthrough" : names; - } - return "passthrough"; -} - -function projectBlock( - block: MCPContentBlock, - fields: readonly string[], - shortenNestedRefs: boolean, - applyAliases: boolean, -): MCPContentBlock { - if (block.type !== "text" || typeof block.text !== "string") return block; - const projected = projectJsonText( - block.text, - fields, - shortenNestedRefs, - applyAliases, - ); - if (projected === block.text) return block; - return { ...block, text: projected }; -} - -function projectJsonText( - text: string, - fields: readonly string[], - shortenNestedRefs: boolean, - applyAliases: boolean, -): string { - const trimmed = text.trim(); - if (trimmed.length === 0) return text; - try { - const parsed: unknown = JSON.parse(trimmed); - return JSON.stringify( - projectMcpJsonValue(parsed, fields, shortenNestedRefs, applyAliases), - ); - } catch { - return text; - } -} - -function pickEntityFields( - record: Record, - fields: readonly string[], - shortenNestedRefs: boolean, - applyAliases: boolean, -): Record { - const out = Object.create(null) as Record; - for (const key of fields) { - if (Object.hasOwn(record, key)) { - const copied = copyField(record[key], shortenNestedRefs); - if (copied !== undefined) out[key] = copied; - continue; - } - if (!applyAliases) continue; - const aliases = DEFAULT_FIELD_ALIASES[key]; - if (aliases === undefined) continue; - for (const alias of aliases) { - if (!Object.hasOwn(record, alias)) continue; - const copied = copyField(record[alias], shortenNestedRefs); - if (copied === undefined) break; - out[key] = copied; - if (!Object.hasOwn(out, alias)) out[alias] = copied; - break; - } - } - return out; -} - -function copyField(value: unknown, shortenNestedRefs: boolean): unknown { - if (!shortenNestedRefs || !isRecord(value)) return value; - const nested = Object.create(null) as Record; - for (const key of NESTED_REF_FIELDS) { - if (!Object.hasOwn(value, key)) continue; - const inner = value[key]; - if (isRecord(inner) || Array.isArray(inner)) continue; - nested[key] = inner; - } - return Object.keys(nested).length > 0 ? nested : undefined; -} - -function asProjectedStructured( - value: unknown, -): Record | unknown[] | undefined { - if (isRecord(value) || Array.isArray(value)) return value; - return undefined; -} - -function isListEnvelope(value: Record): boolean { - let sawRecordArray = false; - let sawEmptyArray = false; - for (const item of Object.values(value)) { - if (!Array.isArray(item)) continue; - if (item.some((entry) => isRecord(entry))) sawRecordArray = true; - else if (item.length === 0) sawEmptyArray = true; - } - if (sawRecordArray) return true; - if (!sawEmptyArray) return false; - return !Object.hasOwn(value, "id") && !Object.hasOwn(value, "identifier"); -} - -function isProjectedList(value: unknown): value is unknown[] { - return ( - Array.isArray(value) && - (value.length === 0 || value.some((entry) => isRecord(entry))) - ); -} - -function isRecord(value: unknown): value is Record { - return typeof value === "object" && value !== null && !Array.isArray(value); -} diff --git a/src/tools/web-fetch.test.ts b/src/tools/web-fetch.test.ts index 72d19da04..df35d5f23 100644 --- a/src/tools/web-fetch.test.ts +++ b/src/tools/web-fetch.test.ts @@ -1,6 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { createServer, type Server } from "node:http"; -import type { MCPClient } from "../mcp/client.js"; import { createDynamicToolRunner } from "../agent/dynamic-tool-runner.js"; import { createExaMCPWebFetchTool, @@ -134,7 +133,13 @@ describe("runWebFetch", () => { }); describe("createExaMCPWebFetchTool", () => { - function createTool(call: MCPClient["call"]) { + function createTool( + call: ( + name: string, + args: Record, + signal: AbortSignal, + ) => Promise, + ) { return createExaMCPWebFetchTool( { connect: async () => ({ @@ -144,7 +149,11 @@ describe("createExaMCPWebFetchTool", () => { tools: [ { name: "web_fetch_exa", description: "Fetch", inputSchema: {} }, ], - call, + call: async () => "", + callResult: async (name, args, signal) => ({ + blocks: [{ type: "text", text: await call(name, args, signal) }], + isError: false, + }), close: async () => undefined, }, }), diff --git a/src/tools/web-fetch.ts b/src/tools/web-fetch.ts index 790b20242..fd92de19f 100644 --- a/src/tools/web-fetch.ts +++ b/src/tools/web-fetch.ts @@ -7,7 +7,7 @@ import { checkUrlForSsrf } from "./ssrf-guard.js"; import { htmlToMarkdown, htmlToText } from "./html-convert.js"; import { COMMAND_NAME } from "../branding.js"; import { toolInputSchema } from "../util/tool-input-schema.js"; -import type { MCPClient } from "../mcp/client.js"; +import { unwrapToolContent, type MCPClient } from "../mcp/client.js"; import pkg from "../../package.json" with { type: "json" }; export const MAX_FETCH_BYTES = 5 * 1024 * 1024; // 5 MB @@ -360,11 +360,12 @@ export function createExaMCPWebFetchTool( "Error: Exa MCP web_fetch unavailable: connected Exa server did not advertise web_fetch_exa.", }; } - const content = await connection.client.call( + const envelope = await connection.client.callResult( "web_fetch_exa", { urls: [parsed.url] }, callSignal, ); + const content = unwrapToolContent(envelope.blocks); return { callId: call.id, content }; })(), timeoutPromise, diff --git a/src/tools/web-search.test.ts b/src/tools/web-search.test.ts index 2992477e4..3a6d94f7c 100644 --- a/src/tools/web-search.test.ts +++ b/src/tools/web-search.test.ts @@ -19,9 +19,16 @@ await withMockedModule( client: { serverName: config.name, tools: [], - call: async (toolName: string, args: Record) => { + call: async () => "", + callResult: async ( + toolName: string, + args: Record, + ) => { calls.push({ toolName, args }); - return "mock result"; + return { + blocks: [{ type: "text", text: "mock result" }], + isError: false, + }; }, close: async () => undefined, }, diff --git a/src/tools/web-search.ts b/src/tools/web-search.ts index a03dee729..65be27173 100644 --- a/src/tools/web-search.ts +++ b/src/tools/web-search.ts @@ -3,7 +3,11 @@ import { stringTool } from "@intx/agent"; import type { AgentTool } from "@intx/agent"; import type { ToolDefinition } from "@intx/types/runtime"; -import { connectMCPServer, type MCPClient } from "../mcp/client.js"; +import { + connectMCPServer, + unwrapToolContent, + type MCPClient, +} from "../mcp/client.js"; import { toolInputSchema } from "../util/tool-input-schema.js"; import type { MCPServerConfig } from "../config/settings.js"; import { EXA_MCP_URL } from "../mcp/exa.js"; @@ -157,7 +161,12 @@ export function createWebSearchTool(): AgentTool { const client = await getClient(provider); const args = provider === "parallel" ? parallelArgs(parsed) : exaArgs(parsed); - const result = await client.call(toolNameFor(provider), args, signal); + const envelope = await client.callResult( + toolNameFor(provider), + args, + signal, + ); + const result = unwrapToolContent(envelope.blocks); return result.length > 0 ? result : "No results."; } catch (err) { return `Error: web_search (${provider}) failed: ${err instanceof Error ? err.message : String(err)}`; diff --git a/testkit/mcp-connect-mock.ts b/testkit/mcp-connect-mock.ts index a6a957ec3..02cf5b39c 100644 --- a/testkit/mcp-connect-mock.ts +++ b/testkit/mcp-connect-mock.ts @@ -267,6 +267,17 @@ export async function installMcpConnectMock( mock.calls.push({ toolName, args, signal }); return mock.toolCallResult; }, + callResult: async ( + toolName: string, + args: Record, + signal: AbortSignal, + ) => { + mock.calls.push({ toolName, args, signal }); + return { + blocks: [{ type: "text" as const, text: mock.toolCallResult }], + isError: false, + }; + }, close, }, };