diff --git a/docs/MCP.md b/docs/MCP.md index 52850f56d..ff64d2bd6 100644 --- a/docs/MCP.md +++ b/docs/MCP.md @@ -226,6 +226,12 @@ command, and no credentials in the config (Linear authorizes over OAuth): Because only `url` is set, `type` defaults to `http`. +Write tools (`save_issue`, `save_comment`, `save_issue_label`) return `id`, +`url`, `status`, and `title` (plus `identifier` when Linear sends it) — not the +body just written. List tools (`list_issues`, `list_comments`, …) use the same +short field set per row unless the call passes `fields`, which expands each row +to those keys, or `fields: "all"` / `["*"]` for the full payload. + ### Authorization on first run The first time Corbits Code connects to an http server that requires auth, the TUI diff --git a/src/auth/store.test.ts b/src/auth/store.test.ts index a29f23cf6..ae0d7a9ed 100644 --- a/src/auth/store.test.ts +++ b/src/auth/store.test.ts @@ -11,7 +11,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { type } from "arktype"; -import { withMockedModule } from "../../tests/helpers/mock-module.js"; +import { withMockedModule } from "../../testkit/mock-module.js"; import type { BaseTokens } from "./store.js"; let failingUnlinkPath: string | undefined; diff --git a/src/mcp/plugin.test.ts b/src/mcp/plugin.test.ts index 28bcc2bcf..9296a9bd2 100644 --- a/src/mcp/plugin.test.ts +++ b/src/mcp/plugin.test.ts @@ -669,6 +669,134 @@ 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, diff --git a/src/mcp/plugin.ts b/src/mcp/plugin.ts index 9cb205fae..897a8d9ca 100644 --- a/src/mcp/plugin.ts +++ b/src/mcp/plugin.ts @@ -12,13 +12,14 @@ import { type SpillBlobWriter, } from "../plugins/result-truncation-plugin.js"; import type { CompactionArchive } from "../session/compaction-archive.js"; -import type { - MCPClient, - MCPContentBlock, - MCPToolResultEnvelope, +import { + unwrapToolContent, + type MCPClient, + type MCPContentBlock, + type MCPToolResultEnvelope, } from "./client.js"; +import { applyMcpResultProjection } from "./result-projection.js"; import { mcpToolName } from "./tool-name.js"; -import { unwrapToolContent } from "./client.js"; export const MCP_RECONNECTING_TOOL_ERROR = "MCP server is reconnecting; retry the call once it reports connected."; @@ -138,10 +139,6 @@ export function mcpClientTools( unknown >); const isError = envelope.isError === true; - const serializedStructured = - scrubbedStructured === undefined - ? undefined - : serializeStructuredContent(scrubbedStructured); const archive = getEvidenceArchive?.(); let archivedFullEnvelope = false; if (archive !== undefined) { @@ -163,13 +160,33 @@ export function mcpClientTools( // Archive write must not fail a successful tool result. } } - const flattened = unwrapToolContent(authorizedBlocks); + 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 + ? undefined + : serializeStructuredContent(projected.structuredContent); + const flattened = unwrapToolContent(projected.blocks); const baseContent = flattened !== "" ? flattened : serializedStructured !== undefined ? `${MCP_STRUCTURED_CONTENT_MARKER}\n${serializedStructured.serialized}` - : scrubbedStructured !== undefined + : projected.structuredContent !== 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/result-projection.test.ts b/src/mcp/result-projection.test.ts new file mode 100644 index 000000000..0fb4fcda6 --- /dev/null +++ b/src/mcp/result-projection.test.ts @@ -0,0 +1,255 @@ +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("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("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 new file mode 100644 index 000000000..f8fe7ddd1 --- /dev/null +++ b/src/mcp/result-projection.ts @@ -0,0 +1,245 @@ +import type { MCPContentBlock } from "./client.js"; + +/** + * Default model-facing keys for Linear write/list results. Identifier stays so + * callers that only need the issue id still 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; +} + +export interface McpProjectionOutput { + blocks: MCPContentBlock[]; + structuredContent?: Record; +} + +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 + : asRecord( + 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; + + const recordArrays = Object.entries(value).filter( + ([, item]) => Array.isArray(item) && item.some((entry) => isRecord(entry)), + ); + if (recordArrays.length > 0) { + const out = Object.create(null) as Record; + for (const [key, item] of Object.entries(value)) { + if (Array.isArray(item) && item.some((entry) => isRecord(entry))) { + 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 asRecord(value: unknown): Record | undefined { + return isRecord(value) ? value : undefined; +} + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +}