Skip to content

Commit 43f0519

Browse files
fix(mcp): surface tool failures and structured content (#1170)
* test(mcp): cover tool-level isError and structuredContent * fix(mcp): surface tool-level isError and structuredContent * test(mcp): use live-shaped token in structured redaction test The criterion-4 test fed the redaction marker itself, asserting both P and not-P. Feed a constructed live-shaped token and assert the marker is present with the raw token absent in content and detail. * fix(session): omit oversized tool result detail from hook payloads Hook payloads capped content but passed detail through verbatim, letting a verbose server blow up payloads. Omit serialized detail over the payload budget while preserving small detail. * fix(mcp): bound and scrub structured result evidence * fix(mcp): harden structured result evidence
1 parent 7d8e29e commit 43f0519

8 files changed

Lines changed: 1006 additions & 42 deletions

File tree

‎src/mcp/client-envelope.test.ts‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import { describe, expect, test } from "bun:test";
2+
import { defined } from "../../tests/helpers/defined.js";
3+
import { withMockedModule } from "../../tests/helpers/mock-module.js";
4+
import { connectMCPServer } from "./client.js";
5+
6+
let scriptedCallToolResult: unknown = { content: [] };
7+
8+
await withMockedModule(
9+
import.meta.resolve("@modelcontextprotocol/sdk/client/index.js"),
10+
(real: typeof import("@modelcontextprotocol/sdk/client/index.js")) => ({
11+
...real,
12+
Client: class {
13+
async connect(): Promise<void> {
14+
return undefined;
15+
}
16+
async listTools(): Promise<{ tools: [] }> {
17+
return { tools: [] };
18+
}
19+
async callTool(): Promise<unknown> {
20+
return scriptedCallToolResult;
21+
}
22+
async close(): Promise<void> {
23+
return undefined;
24+
}
25+
},
26+
}),
27+
);
28+
29+
describe("mcp client tool envelope", () => {
30+
test("callResult preserves isError and structuredContent from the SDK", async () => {
31+
scriptedCallToolResult = {
32+
content: [{ type: "text", text: "tool failed: bad input" }],
33+
isError: true,
34+
structuredContent: { reason: "bad input" },
35+
};
36+
const connected = await connectMCPServer(
37+
{ name: "envelope", command: "true" },
38+
{},
39+
);
40+
if (!connected.ok) throw new Error("expected stdio connect to succeed");
41+
const envelope = await defined(
42+
connected.client.callResult,
43+
"mcp client callResult",
44+
)("do_thing", {}, new AbortController().signal);
45+
46+
expect(envelope.isError).toBe(true);
47+
expect(envelope.blocks).toEqual([
48+
{ type: "text", text: "tool failed: bad input" },
49+
]);
50+
expect(envelope.structuredContent).toEqual({ reason: "bad input" });
51+
await connected.client.close();
52+
});
53+
54+
test("legacy call still flattens text blocks", async () => {
55+
scriptedCallToolResult = {
56+
content: [{ type: "text", text: "hello" }],
57+
};
58+
const connected = await connectMCPServer(
59+
{ name: "envelope", command: "true" },
60+
{},
61+
);
62+
if (!connected.ok) throw new Error("expected stdio connect to succeed");
63+
const text = await connected.client.call(
64+
"do_thing",
65+
{},
66+
new AbortController().signal,
67+
);
68+
69+
expect(text).toBe("hello");
70+
await connected.client.close();
71+
});
72+
});

‎src/mcp/client.ts‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,23 @@ export interface MCPContentBlock {
2727
[key: string]: unknown;
2828
}
2929

30+
/**
31+
* Scope lock (CL-8992): the pinned @modelcontextprotocol/sdk v1 CallToolResult
32+
* is `{ content: blocks[] (default []), structuredContent?: Record<string,
33+
* unknown>, isError?: boolean }` — a tool-level failure still succeeds at the
34+
* protocol layer. The envelope carries all three so the plugin can surface
35+
* failures as errors and structured-only payloads as readable text.
36+
* `structuredContent` reaches the model JSON-serialized into the content
37+
* string under MCP_STRUCTURED_CONTENT_MARKER (see plugin.ts). Small
38+
* policy-scrubbed records are preserved under ToolResult `detail`; the full
39+
* scrubbed record is retained in the evidence archive — never raw.
40+
*/
41+
export interface MCPToolResultEnvelope {
42+
blocks: MCPContentBlock[];
43+
isError: boolean;
44+
structuredContent?: Record<string, unknown>;
45+
}
46+
3047
export interface MCPClient {
3148
serverName: string;
3249
tools: MCPTool[];
@@ -41,6 +58,12 @@ export interface MCPClient {
4158
args: Record<string, unknown>,
4259
signal: AbortSignal,
4360
): Promise<MCPContentBlock[]>;
61+
/** Full tool-result envelope: blocks plus tool-level isError/structuredContent. */
62+
callResult?(
63+
toolName: string,
64+
args: Record<string, unknown>,
65+
signal: AbortSignal,
66+
): Promise<MCPToolResultEnvelope>;
4467
close(): Promise<void>;
4568
}
4669

@@ -566,6 +589,29 @@ async function finishClient(
566589
return {
567590
serverName,
568591
tools,
592+
async callResult(toolName, args, signal) {
593+
const context =
594+
authContext === undefined ? undefined : { ...authContext, signal };
595+
const result = await withHTTPAuthorizationRecovery(context, () =>
596+
client.callTool({ name: toolName, arguments: args }, undefined, {
597+
signal,
598+
}),
599+
);
600+
const envelope: MCPToolResultEnvelope = {
601+
blocks: validateMcpContentBlocks(result.content),
602+
isError: result.isError === true,
603+
};
604+
if (
605+
result.structuredContent !== null &&
606+
typeof result.structuredContent === "object"
607+
) {
608+
envelope.structuredContent = result.structuredContent as Record<
609+
string,
610+
unknown
611+
>;
612+
}
613+
return envelope;
614+
},
569615
async callBlocks(toolName, args, signal) {
570616
const context =
571617
authContext === undefined ? undefined : { ...authContext, signal };

0 commit comments

Comments
 (0)