From e1633fbc3f2d332d59b1377efd395438d93f7104 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Tue, 21 Jul 2026 18:03:01 +0800 Subject: [PATCH] fix(tools): preserve canonical output boundaries --- docs/config-catalog.md | 2 +- docs/cordis-catalog/services.md | 2 +- docs/core-data-structures/session.md | 15 +-- docs/persistence-catalog.md | 25 ++-- .../snapshots/cancel-tool-calls/session.jsonl | 4 +- .../escalation-rejected/session.jsonl | 2 +- .../snapshots/fs-policy-reject/session.jsonl | 2 +- .../hook-cc-posttool-block/session.jsonl | 2 +- .../hook-cc-pretool-ask/session.jsonl | 2 +- .../hook-cc-pretool-deny/session.jsonl | 2 +- .../hook-codex-posttool-block/session.jsonl | 2 +- .../hook-codex-pretool-block/session.jsonl | 2 +- .../session.2.jsonl | 2 +- .../tests/tool-result-prune.spec.ts | 4 +- .../cordis/tool-cordis/src/api-catalog.ts | 2 +- packages/core/agent-loop/src/tool-calls.ts | 2 +- packages/core/agent-loop/tests/cancel.spec.ts | 2 +- .../tests/contract-regressions.spec.ts | 4 +- .../core/agent-loop/tests/tool-calls.spec.ts | 16 +-- packages/core/session/README.md | 2 +- packages/core/session/src/repair.ts | 5 +- packages/core/session/src/types.ts | 15 +-- packages/core/session/tests/repair.spec.ts | 2 +- packages/core/tools/src/index.ts | 40 ++++++- packages/core/tools/tests/tools.spec.ts | 9 +- packages/fs/tool-fs/src/read-render.ts | 1 + packages/mcp/mcp-client/package.json | 6 +- packages/mcp/mcp-client/src/tools.ts | 69 +++++++---- .../mcp/mcp-client/tests/mcp-client.spec.ts | 113 +++++++++++++++++- .../session-persistence/tests/contract.ts | 2 +- packages/support/invariants/src/index.ts | 2 +- .../invariants/tests/invariants.spec.ts | 6 +- pnpm-lock.yaml | 6 +- 33 files changed, 269 insertions(+), 103 deletions(-) diff --git a/docs/config-catalog.md b/docs/config-catalog.md index 47a9926575..6285eac69a 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -1303,7 +1303,7 @@ export interface Config { export type ToolPresentationMode = 'native' | 'code' | 'both' ``` -Source: [`packages/core/tools/src/index.ts:448`](../packages/core/tools/src/index.ts) +Source: [`packages/core/tools/src/index.ts:467`](../packages/core/tools/src/index.ts) ## `@deepseek-ai/dsh-tui` diff --git a/docs/cordis-catalog/services.md b/docs/cordis-catalog/services.md index 53180d3621..e93d734d82 100644 --- a/docs/cordis-catalog/services.md +++ b/docs/cordis-catalog/services.md @@ -1361,7 +1361,7 @@ async execute(exec: ToolExecutionInput): Promise Types: [ScopeKey](../core-data-structures/scope.md) · [ToolDefinition](../core-data-structures/tools.md) · [ToolExecutionInput](../core-data-structures/tools.md) · [ToolExecutionMode](../core-data-structures/tools.md) · [ToolExecutionResult](../core-data-structures/tools.md) · [ToolGuard](../core-data-structures/tools.md) · [ToolRestriction](../core-data-structures/tools.md) · [ToolSchema](../core-data-structures/tools.md) -Source: [`packages/core/tools/src/index.ts:504`](../../packages/core/tools/src/index.ts) +Source: [`packages/core/tools/src/index.ts:523`](../../packages/core/tools/src/index.ts) ## `ctx.userInteraction` — `UserInteractionService` diff --git a/docs/core-data-structures/session.md b/docs/core-data-structures/session.md index 2df677790b..b8f2d64136 100644 --- a/docs/core-data-structures/session.md +++ b/docs/core-data-structures/session.md @@ -73,12 +73,13 @@ interface SessionEventMap { */ 'tool/call': { turn: number; step: number; callId: CallId; name: string; arguments: string } /** - * A completed tool call's model-facing result, canonical failure detail, and - * optional tool-private `meta` presentation payload. `meta` is opaque to the - * core (the producing tool owns its shape and reads it back in `presentResult`) - * but MUST be JSON-serializable: `Session.append` runtime-validates all event - * data with `isJsonValue`, so a non-serializable `meta` is rejected at the - * source, and the durable log reproduces the identical card on replay. Absent + * A completed tool call's model-facing result, optional internal failure + * identity, and optional tool-private `meta` presentation payload. `meta` is + * opaque to the core (the producing tool owns its shape and reads it back in + * `presentResult`) but MUST be JSON-serializable: `Session.append` + * runtime-validates all event data with `isJsonValue`, so a non-serializable + * `meta` is rejected at the source, and the durable log reproduces the + * identical card on replay. Absent * unless the tool attaches one (e.g. `dsh-tool-fs` carries its result-time * contextual diff here). */ @@ -88,7 +89,7 @@ interface SessionEventMap { callId: CallId content: ContentBlock[] isError: boolean - error?: { message: string; info?: { name: string; code: string } } + error?: { name: string; code: string } meta?: JsonValue } /** Steering content injected between steps of a running turn. */ diff --git a/docs/persistence-catalog.md b/docs/persistence-catalog.md index 9ca545854c..f310c22c44 100644 --- a/docs/persistence-catalog.md +++ b/docs/persistence-catalog.md @@ -79,7 +79,7 @@ export type SessionEvent = { }[T] ``` -Sources: [`packages/core/session/src/types.ts:276`](../packages/core/session/src/types.ts) · [`packages/core/session/src/types.ts:283`](../packages/core/session/src/types.ts) · [`packages/core/session/src/types.ts:313`](../packages/core/session/src/types.ts) · [`packages/core/session/src/types.ts:345`](../packages/core/session/src/types.ts) +Sources: [`packages/core/session/src/types.ts:277`](../packages/core/session/src/types.ts) · [`packages/core/session/src/types.ts:284`](../packages/core/session/src/types.ts) · [`packages/core/session/src/types.ts:314`](../packages/core/session/src/types.ts) · [`packages/core/session/src/types.ts:346`](../packages/core/session/src/types.ts) ## Events @@ -356,7 +356,7 @@ Source: [`packages/core/session/src/types.ts:213`](../packages/core/session/src/ 'request/header': { header: EpochHeader; reason: RequestHeaderReason } ``` -Source: [`packages/core/session/src/types.ts:272`](../packages/core/session/src/types.ts) +Source: [`packages/core/session/src/types.ts:273`](../packages/core/session/src/types.ts) ### `sandbox/*` @@ -387,7 +387,7 @@ Source: [`packages/sandbox/sandbox-policy/src/session-mode.ts:34`](../packages/s Types: [ContentBlock](core-data-structures/core.md) · [MessageSource](core-data-structures/core.md) -Source: [`packages/core/session/src/types.ts:265`](../packages/core/session/src/types.ts) +Source: [`packages/core/session/src/types.ts:266`](../packages/core/session/src/types.ts) ### `step/*` @@ -420,7 +420,7 @@ Source: [`packages/core/session/src/types.ts:204`](../packages/core/session/src/ Types: [TodoItem](core-data-structures/session.md) -Source: [`packages/core/session/src/types.ts:267`](../packages/core/session/src/types.ts) +Source: [`packages/core/session/src/types.ts:268`](../packages/core/session/src/types.ts) ### `tool/*` @@ -468,12 +468,13 @@ Source: [`packages/core/tools/src/code-mode.ts:34`](../packages/core/tools/src/c ```ts persistence-catalog /** - * A completed tool call's model-facing result, canonical failure detail, and - * optional tool-private `meta` presentation payload. `meta` is opaque to the - * core (the producing tool owns its shape and reads it back in `presentResult`) - * but MUST be JSON-serializable: `Session.append` runtime-validates all event - * data with `isJsonValue`, so a non-serializable `meta` is rejected at the - * source, and the durable log reproduces the identical card on replay. Absent + * A completed tool call's model-facing result, optional internal failure + * identity, and optional tool-private `meta` presentation payload. `meta` is + * opaque to the core (the producing tool owns its shape and reads it back in + * `presentResult`) but MUST be JSON-serializable: `Session.append` + * runtime-validates all event data with `isJsonValue`, so a non-serializable + * `meta` is rejected at the source, and the durable log reproduces the + * identical card on replay. Absent * unless the tool attaches one (e.g. `dsh-tool-fs` carries its result-time * contextual diff here). */ @@ -483,14 +484,14 @@ Source: [`packages/core/tools/src/code-mode.ts:34`](../packages/core/tools/src/c callId: CallId content: ContentBlock[] isError: boolean - error?: { message: string; info?: { name: string; code: string } } + error?: { name: string; code: string } meta?: JsonValue } ``` Types: [CallId](core-data-structures/core.md) · [ContentBlock](core-data-structures/core.md) -Source: [`packages/core/session/src/types.ts:255`](../packages/core/session/src/types.ts) +Source: [`packages/core/session/src/types.ts:256`](../packages/core/session/src/types.ts) ### `turn/*` diff --git a/examples/acp-agent/tests/snapshots/cancel-tool-calls/session.jsonl b/examples/acp-agent/tests/snapshots/cancel-tool-calls/session.jsonl index db08475efd..cdb76be163 100644 --- a/examples/acp-agent/tests/snapshots/cancel-tool-calls/session.jsonl +++ b/examples/acp-agent/tests/snapshots/cancel-tool-calls/session.jsonl @@ -13,8 +13,8 @@ {"type":"assistant/chunk","seq":11,"time":1784437195077,"data":{"turn":1,"step":1,"chunk":{"type":"finish","reason":{"kind":"tool-calls"}}}} {"type":"assistant/message","seq":12,"time":1784437195078,"data":{"turn":1,"step":1,"content":[{"type":"tool-call","id":"call_wait","name":"bash","arguments":"{\"command\":\"node -e \\\"setInterval(() => {}, 1000)\\\"\",\"description\":\"Wait until cancellation\"}"},{"type":"tool-call","id":"call_skipped","name":"bash","arguments":"{\"command\":\"printf skipped > skipped.txt\",\"description\":\"Write skipped marker\"}"}],"provenance":{"provider":"deepseek","model":"deepseek-v4-flash"},"usage":{"inputTokens":10,"outputTokens":10}},"sourceEventSeqs":[4,5,6,7,8,9,10,11],"surfaceOp":"append"} {"type":"tool/call","seq":13,"time":1784437195078,"data":{"turn":1,"step":1,"callId":"call_wait","name":"bash","arguments":"{\"command\":\"node -e \\\"setInterval(() => {}, 1000)\\\"\",\"description\":\"Wait until cancellation\"}"}} -{"type":"tool/result","seq":14,"time":1784437195089,"data":{"turn":1,"step":1,"callId":"call_wait","content":[{"type":"text","text":"Error: command aborted"}],"isError":true,"error":{"message":"command aborted"}},"sourceEventSeqs":[13],"surfaceOp":"append"} +{"type":"tool/result","seq":14,"time":1784437195089,"data":{"turn":1,"step":1,"callId":"call_wait","content":[{"type":"text","text":"Error: command aborted"}],"isError":true},"sourceEventSeqs":[13],"surfaceOp":"append"} {"type":"tool/call","seq":15,"time":1784437195089,"data":{"turn":1,"step":1,"callId":"call_skipped","name":"bash","arguments":"{\"command\":\"printf skipped > skipped.txt\",\"description\":\"Write skipped marker\"}"}} -{"type":"tool/result","seq":16,"time":1784437195089,"data":{"turn":1,"step":1,"callId":"call_skipped","content":[{"type":"text","text":"Error: tool call skipped because the step was aborted before execution"}],"isError":true,"error":{"message":"tool call skipped because the step was aborted before execution","info":{"name":"AbortError","code":"ABORTED"}}},"sourceEventSeqs":[15],"surfaceOp":"append"} +{"type":"tool/result","seq":16,"time":1784437195089,"data":{"turn":1,"step":1,"callId":"call_skipped","content":[{"type":"text","text":"Error: tool call skipped because the step was aborted before execution"}],"isError":true,"error":{"name":"AbortError","code":"ABORTED"}},"sourceEventSeqs":[15],"surfaceOp":"append"} {"type":"step/end","seq":17,"time":1784437195090,"data":{"turn":1,"step":1}} {"type":"turn/end","seq":18,"time":1784437195090,"data":{"turn":1,"reason":{"kind":"aborted","reason":"session/cancel"}}} diff --git a/examples/acp-agent/tests/snapshots/escalation-rejected/session.jsonl b/examples/acp-agent/tests/snapshots/escalation-rejected/session.jsonl index 4190704a7f..ff6d1187b3 100644 --- a/examples/acp-agent/tests/snapshots/escalation-rejected/session.jsonl +++ b/examples/acp-agent/tests/snapshots/escalation-rejected/session.jsonl @@ -157,7 +157,7 @@ {"type":"tool/call","seq":155,"time":1783962246274,"data":{"turn":1,"step":1,"callId":"call_00_WB1vnPomi8yr6MlcFKTj7912","name":"bash","arguments":"{\"command\": \"printf 'escalated\\\\n' > /tmp/dsh-escalated.txt && cat /tmp/dsh-escalated.txt && rm /tmp/dsh-escalated.txt\", \"description\": \"Write to /tmp and verify, then clean up\", \"sandbox_permissions\": \"danger-full-access\", \"justification\": \"the user asked to write a file outside the workspace\"}"}} {"type":"approval/asked","seq":156,"time":1783962246275,"data":{"id":"46c8dba4-52c9-4a6a-b6a6-5f34c95c28df","toolName":"bash","callId":"call_00_WB1vnPomi8yr6MlcFKTj7912","reason":"escalate sandbox to danger-full-access: the user asked to write a file outside the workspace"}} {"type":"approval/decided","seq":157,"time":1783962246275,"data":{"id":"46c8dba4-52c9-4a6a-b6a6-5f34c95c28df","outcome":"rejected"}} -{"type":"tool/result","seq":158,"time":1783962246275,"data":{"turn":1,"step":1,"callId":"call_00_WB1vnPomi8yr6MlcFKTj7912","content":[{"type":"text","text":"Error: the user rejected escalating this command to \"danger-full-access\""}],"isError":true,"error":{"message":"the user rejected escalating this command to \"danger-full-access\""}},"sourceEventSeqs":[155],"surfaceOp":"append"} +{"type":"tool/result","seq":158,"time":1783962246275,"data":{"turn":1,"step":1,"callId":"call_00_WB1vnPomi8yr6MlcFKTj7912","content":[{"type":"text","text":"Error: the user rejected escalating this command to \"danger-full-access\""}],"isError":true},"sourceEventSeqs":[155],"surfaceOp":"append"} {"type":"step/end","seq":159,"time":1783962246276,"data":{"turn":1,"step":1}} {"type":"step/start","seq":160,"time":1783962246276,"data":{"turn":1,"step":2}} {"type":"assistant/chunk","seq":161,"time":1783860683140,"data":{"turn":1,"step":2,"chunk":{"type":"block-start","index":0,"blockType":"reasoning"}}} diff --git a/examples/acp-agent/tests/snapshots/fs-policy-reject/session.jsonl b/examples/acp-agent/tests/snapshots/fs-policy-reject/session.jsonl index 5b1ccd60fc..66efd5f934 100644 --- a/examples/acp-agent/tests/snapshots/fs-policy-reject/session.jsonl +++ b/examples/acp-agent/tests/snapshots/fs-policy-reject/session.jsonl @@ -77,7 +77,7 @@ {"type":"assistant/chunk","seq":75,"time":1783611703969,"data":{"turn":1,"step":1,"chunk":{"type":"finish","reason":{"kind":"tool-calls"}}}} {"type":"assistant/message","seq":76,"time":1783611703972,"data":{"turn":1,"step":1,"content":[{"type":"reasoning","text":"The user wants me to use the edit tool to replace \"blue\" with \"green\" in settings.txt without reading the file first, and then reply with just \"DONE\"."},{"type":"tool-call","id":"call_00_x0zlnXl5JOxLrAYL9y7P0119","name":"edit","arguments":"{\"file_path\": \"settings.txt\", \"old_string\": \"blue\", \"new_string\": \"green\"}"}],"provenance":{"provider":"deepseek","model":"deepseek-v4-flash"},"usage":{"inputTokens":3132,"outputTokens":115,"cacheReadTokens":0,"reasoningTokens":36}},"sourceEventSeqs":[4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75],"surfaceOp":"append"} {"type":"tool/call","seq":77,"time":1783611703972,"data":{"turn":1,"step":1,"callId":"call_00_x0zlnXl5JOxLrAYL9y7P0119","name":"edit","arguments":"{\"file_path\": \"settings.txt\", \"old_string\": \"blue\", \"new_string\": \"green\"}"}} -{"type":"tool/result","seq":78,"time":1783611703978,"data":{"turn":1,"step":1,"callId":"call_00_x0zlnXl5JOxLrAYL9y7P0119","content":[{"type":"text","text":"Error: edit requires reading \"/var/folders/2c/psb0_fmx7hbgz558xjt_f0l00000gn/T/acp-snap-cwd-QzoqnB/settings.txt\" first"}],"isError":true,"error":{"message":"edit requires reading \"/var/folders/2c/psb0_fmx7hbgz558xjt_f0l00000gn/T/acp-snap-cwd-QzoqnB/settings.txt\" first","info":{"name":"FsError","code":"FS_NOT_OBSERVED"}}},"sourceEventSeqs":[77],"surfaceOp":"append"} +{"type":"tool/result","seq":78,"time":1783611703978,"data":{"turn":1,"step":1,"callId":"call_00_x0zlnXl5JOxLrAYL9y7P0119","content":[{"type":"text","text":"Error: edit requires reading \"/var/folders/2c/psb0_fmx7hbgz558xjt_f0l00000gn/T/acp-snap-cwd-QzoqnB/settings.txt\" first"}],"isError":true,"error":{"name":"FsError","code":"FS_NOT_OBSERVED"}},"sourceEventSeqs":[77],"surfaceOp":"append"} {"type":"step/end","seq":79,"time":1783611703978,"data":{"turn":1,"step":1}} {"type":"step/start","seq":80,"time":1783611703978,"data":{"turn":1,"step":2}} {"type":"assistant/chunk","seq":81,"time":1783611704825,"data":{"turn":1,"step":2,"chunk":{"type":"block-start","index":0,"blockType":"reasoning"}}} diff --git a/examples/acp-agent/tests/snapshots/hook-cc-posttool-block/session.jsonl b/examples/acp-agent/tests/snapshots/hook-cc-posttool-block/session.jsonl index 8cab55c391..c362de7a26 100644 --- a/examples/acp-agent/tests/snapshots/hook-cc-posttool-block/session.jsonl +++ b/examples/acp-agent/tests/snapshots/hook-cc-posttool-block/session.jsonl @@ -75,7 +75,7 @@ {"type":"tool/call","seq":73,"time":1783962505993,"data":{"turn":1,"step":1,"callId":"call_00_VAByyMjsct4c7P6k1ysX9256","name":"bash","arguments":"{\"command\": \"echo HELLO\", \"description\": \"Echo HELLO to stdout\"}"}} {"type":"hook/invoked","seq":74,"time":1783962506001,"data":{"turn":1,"point":"PostToolUse","dialect":"claude","handlerId":"claude:PostToolUse:1","matcher":"bash"}} {"type":"hook/result","seq":75,"time":1783962506011,"data":{"turn":1,"point":"PostToolUse","handlerId":"claude:PostToolUse:1","decision":"block","exitCode":2,"stderrSummary":"tool output rejected by policy: retry once","durationMs":9.922291999999743}} -{"type":"tool/result","seq":76,"time":1783962506011,"data":{"turn":1,"step":1,"callId":"call_00_VAByyMjsct4c7P6k1ysX9256","content":[{"type":"text","text":"tool output rejected by policy: retry once"}],"isError":true,"error":{"message":"tool output rejected by policy: retry once"}},"sourceEventSeqs":[73],"surfaceOp":"append"} +{"type":"tool/result","seq":76,"time":1783962506011,"data":{"turn":1,"step":1,"callId":"call_00_VAByyMjsct4c7P6k1ysX9256","content":[{"type":"text","text":"tool output rejected by policy: retry once"}],"isError":true},"sourceEventSeqs":[73],"surfaceOp":"append"} {"type":"step/end","seq":77,"time":1783962506012,"data":{"turn":1,"step":1}} {"type":"step/start","seq":78,"time":1783962506012,"data":{"turn":1,"step":2}} {"type":"assistant/chunk","seq":79,"time":1783962507038,"data":{"turn":1,"step":2,"chunk":{"type":"block-start","index":0,"blockType":"reasoning"}}} diff --git a/examples/acp-agent/tests/snapshots/hook-cc-pretool-ask/session.jsonl b/examples/acp-agent/tests/snapshots/hook-cc-pretool-ask/session.jsonl index 1309b5c30d..247e13a075 100644 --- a/examples/acp-agent/tests/snapshots/hook-cc-pretool-ask/session.jsonl +++ b/examples/acp-agent/tests/snapshots/hook-cc-pretool-ask/session.jsonl @@ -57,7 +57,7 @@ {"type":"hook/result","seq":55,"time":1783352172573,"data":{"turn":1,"point":"PreToolUse","handlerId":"claude:PreToolUse:1","decision":"ask","exitCode":0,"durationMs":14.113374999999905}} {"type":"approval/asked","seq":56,"time":1783962235813,"data":{"id":"68f1e09d-f3e5-4e39-8a51-da082ba3ba99","toolName":"bash","callId":"call_00_6k0oGSliVHxGSgqBmMEO4311","reason":"bash requires manual approval in this session"}} {"type":"approval/decided","seq":57,"time":1783962235813,"data":{"id":"68f1e09d-f3e5-4e39-8a51-da082ba3ba99","outcome":"rejected"}} -{"type":"tool/result","seq":58,"time":1783962235814,"data":{"turn":1,"step":1,"callId":"call_00_6k0oGSliVHxGSgqBmMEO4311","content":[{"type":"text","text":"Error: the user rejected tool \"bash\""}],"isError":true,"error":{"message":"the user rejected tool \"bash\""}},"sourceEventSeqs":[53],"surfaceOp":"append"} +{"type":"tool/result","seq":58,"time":1783962235814,"data":{"turn":1,"step":1,"callId":"call_00_6k0oGSliVHxGSgqBmMEO4311","content":[{"type":"text","text":"Error: the user rejected tool \"bash\""}],"isError":true},"sourceEventSeqs":[53],"surfaceOp":"append"} {"type":"step/end","seq":59,"time":1783962235814,"data":{"turn":1,"step":1}} {"type":"step/start","seq":60,"time":1783962235814,"data":{"turn":1,"step":2}} {"type":"assistant/chunk","seq":61,"time":1783352173584,"data":{"turn":1,"step":2,"chunk":{"type":"block-start","index":0,"blockType":"reasoning"}}} diff --git a/examples/acp-agent/tests/snapshots/hook-cc-pretool-deny/session.jsonl b/examples/acp-agent/tests/snapshots/hook-cc-pretool-deny/session.jsonl index 869fca1eca..4193c7fe80 100644 --- a/examples/acp-agent/tests/snapshots/hook-cc-pretool-deny/session.jsonl +++ b/examples/acp-agent/tests/snapshots/hook-cc-pretool-deny/session.jsonl @@ -55,7 +55,7 @@ {"type":"tool/call","seq":53,"time":1783352166514,"data":{"turn":1,"step":1,"callId":"call_00_JliP571Bh0QQ8QExbSPk0080","name":"bash","arguments":"{\"command\": \"echo HELLO\", \"description\": \"Run echo HELLO\"}"}} {"type":"hook/invoked","seq":54,"time":1783352166515,"data":{"turn":1,"point":"PreToolUse","dialect":"claude","handlerId":"claude:PreToolUse:1","matcher":"bash"}} {"type":"hook/result","seq":55,"time":1783352166528,"data":{"turn":1,"point":"PreToolUse","handlerId":"claude:PreToolUse:1","decision":"block","exitCode":2,"stderrSummary":"bash is disabled by policy in this session","durationMs":12.367074000000684}} -{"type":"tool/result","seq":56,"time":1783352166528,"data":{"turn":1,"step":1,"callId":"call_00_JliP571Bh0QQ8QExbSPk0080","content":[{"type":"text","text":"Error: bash is disabled by policy in this session"}],"isError":true,"error":{"message":"bash is disabled by policy in this session"}},"sourceEventSeqs":[53],"surfaceOp":"append"} +{"type":"tool/result","seq":56,"time":1783352166528,"data":{"turn":1,"step":1,"callId":"call_00_JliP571Bh0QQ8QExbSPk0080","content":[{"type":"text","text":"Error: bash is disabled by policy in this session"}],"isError":true},"sourceEventSeqs":[53],"surfaceOp":"append"} {"type":"step/end","seq":57,"time":1783352166529,"data":{"turn":1,"step":1}} {"type":"step/start","seq":58,"time":1783352166529,"data":{"turn":1,"step":2}} {"type":"assistant/chunk","seq":59,"time":1783352167307,"data":{"turn":1,"step":2,"chunk":{"type":"block-start","index":0,"blockType":"reasoning"}}} diff --git a/examples/acp-agent/tests/snapshots/hook-codex-posttool-block/session.jsonl b/examples/acp-agent/tests/snapshots/hook-codex-posttool-block/session.jsonl index 4a05a2daa7..dc68891c14 100644 --- a/examples/acp-agent/tests/snapshots/hook-codex-posttool-block/session.jsonl +++ b/examples/acp-agent/tests/snapshots/hook-codex-posttool-block/session.jsonl @@ -66,7 +66,7 @@ {"type":"tool/call","seq":64,"time":1783986963664,"data":{"turn":1,"step":1,"callId":"call_00_1rmSWHhVchVg7PDTmegT0421","name":"bash","arguments":"{\"command\": \"echo HELLO\", \"description\": \"Echo HELLO to stdout\"}"}} {"type":"hook/invoked","seq":65,"time":1783986963673,"data":{"turn":1,"point":"PostToolUse","dialect":"codex","handlerId":"codex:PostToolUse:1","matcher":"bash"}} {"type":"hook/result","seq":66,"time":1783986963677,"data":{"turn":1,"point":"PostToolUse","handlerId":"codex:PostToolUse:1","decision":"block","exitCode":2,"stderrSummary":"tool output rejected by codex policy: summarize instead","durationMs":4.42941699999983}} -{"type":"tool/result","seq":67,"time":1783986963678,"data":{"turn":1,"step":1,"callId":"call_00_1rmSWHhVchVg7PDTmegT0421","content":[{"type":"text","text":"tool output rejected by codex policy: summarize instead"}],"isError":true,"error":{"message":"tool output rejected by codex policy: summarize instead"}},"sourceEventSeqs":[64],"surfaceOp":"append"} +{"type":"tool/result","seq":67,"time":1783986963678,"data":{"turn":1,"step":1,"callId":"call_00_1rmSWHhVchVg7PDTmegT0421","content":[{"type":"text","text":"tool output rejected by codex policy: summarize instead"}],"isError":true},"sourceEventSeqs":[64],"surfaceOp":"append"} {"type":"step/end","seq":68,"time":1783986963678,"data":{"turn":1,"step":1}} {"type":"step/start","seq":69,"time":1783986963679,"data":{"turn":1,"step":2}} {"type":"assistant/chunk","seq":70,"time":1783986964555,"data":{"turn":1,"step":2,"chunk":{"type":"block-start","index":0,"blockType":"reasoning"}}} diff --git a/examples/acp-agent/tests/snapshots/hook-codex-pretool-block/session.jsonl b/examples/acp-agent/tests/snapshots/hook-codex-pretool-block/session.jsonl index eb928d7dff..c2675ae3af 100644 --- a/examples/acp-agent/tests/snapshots/hook-codex-pretool-block/session.jsonl +++ b/examples/acp-agent/tests/snapshots/hook-codex-pretool-block/session.jsonl @@ -55,7 +55,7 @@ {"type":"tool/call","seq":53,"time":1783352215804,"data":{"turn":1,"step":1,"callId":"call_00_tv0SMeLXaTuyuVrOxnV97085","name":"bash","arguments":"{\"command\": \"echo HELLO\", \"description\": \"Run echo HELLO\"}"}} {"type":"hook/invoked","seq":54,"time":1783352215805,"data":{"turn":1,"point":"PreToolUse","dialect":"codex","handlerId":"codex:PreToolUse:1","matcher":"bash"}} {"type":"hook/result","seq":55,"time":1783352215832,"data":{"turn":1,"point":"PreToolUse","handlerId":"codex:PreToolUse:1","decision":"block","exitCode":2,"stderrSummary":"bash is disabled by codex policy in this session","durationMs":26.08518500000082}} -{"type":"tool/result","seq":56,"time":1783352215832,"data":{"turn":1,"step":1,"callId":"call_00_tv0SMeLXaTuyuVrOxnV97085","content":[{"type":"text","text":"Error: bash is disabled by codex policy in this session"}],"isError":true,"error":{"message":"bash is disabled by codex policy in this session"}},"sourceEventSeqs":[53],"surfaceOp":"append"} +{"type":"tool/result","seq":56,"time":1783352215832,"data":{"turn":1,"step":1,"callId":"call_00_tv0SMeLXaTuyuVrOxnV97085","content":[{"type":"text","text":"Error: bash is disabled by codex policy in this session"}],"isError":true},"sourceEventSeqs":[53],"surfaceOp":"append"} {"type":"step/end","seq":57,"time":1783352215833,"data":{"turn":1,"step":1}} {"type":"step/start","seq":58,"time":1783352215834,"data":{"turn":1,"step":2}} {"type":"assistant/chunk","seq":59,"time":1783352216779,"data":{"turn":1,"step":2,"chunk":{"type":"block-start","index":0,"blockType":"reasoning"}}} diff --git a/examples/acp-agent/tests/snapshots/subagent-depth-two-rejection/session.2.jsonl b/examples/acp-agent/tests/snapshots/subagent-depth-two-rejection/session.2.jsonl index ed8f566219..7b36136970 100644 --- a/examples/acp-agent/tests/snapshots/subagent-depth-two-rejection/session.2.jsonl +++ b/examples/acp-agent/tests/snapshots/subagent-depth-two-rejection/session.2.jsonl @@ -10,7 +10,7 @@ {"type":"assistant/chunk","seq":8,"time":0,"data":{"turn":1,"step":1,"chunk":{"type":"finish","reason":{"kind":"tool-calls"}}}} {"type":"assistant/message","seq":9,"time":1784540790335,"data":{"turn":1,"step":1,"content":[{"type":"tool-call","id":"call_depth_three_rejected","name":"subagent","arguments":"{\"description\":\"Exceed depth cap\",\"prompt\":\"This child must never start.\"}"}],"provenance":{"provider":"deepseek","model":"deepseek-v4-flash"},"usage":{"inputTokens":10,"outputTokens":5}},"sourceEventSeqs":[4,5,6,7,8],"surfaceOp":"append"} {"type":"tool/call","seq":10,"time":1784540790335,"data":{"turn":1,"step":1,"callId":"call_depth_three_rejected","name":"subagent","arguments":"{\"description\":\"Exceed depth cap\",\"prompt\":\"This child must never start.\"}"}} -{"type":"tool/result","seq":11,"time":1784540790337,"data":{"turn":1,"step":1,"callId":"call_depth_three_rejected","content":[{"type":"text","text":"Error: subagent depth 3 exceeds maxDepth 2"}],"isError":true,"error":{"message":"subagent depth 3 exceeds maxDepth 2"}},"sourceEventSeqs":[10],"surfaceOp":"append"} +{"type":"tool/result","seq":11,"time":1784540790337,"data":{"turn":1,"step":1,"callId":"call_depth_three_rejected","content":[{"type":"text","text":"Error: subagent depth 3 exceeds maxDepth 2"}],"isError":true},"sourceEventSeqs":[10],"surfaceOp":"append"} {"type":"step/end","seq":12,"time":1784540790338,"data":{"turn":1,"step":1}} {"type":"step/start","seq":13,"time":1784540790338,"data":{"turn":1,"step":2}} {"type":"assistant/chunk","seq":14,"time":1784540790339,"data":{"turn":1,"step":2,"chunk":{"type":"block-start","index":0,"blockType":"text"}}} diff --git a/packages/compact/compact-tool-result-prune/tests/tool-result-prune.spec.ts b/packages/compact/compact-tool-result-prune/tests/tool-result-prune.spec.ts index 0d3478b1c0..bc382c8e4e 100644 --- a/packages/compact/compact-tool-result-prune/tests/tool-result-prune.spec.ts +++ b/packages/compact/compact-tool-result-prune/tests/tool-result-prune.spec.ts @@ -151,7 +151,7 @@ describe('ToolResultPruneService session transaction', () => { text: 'x'.repeat(100), }], { isError: true, - error: { message: 'exit 1', info: { name: 'ExitError', code: 'EXIT_1' } }, + error: { name: 'ExitError', code: 'EXIT_1' }, meta: { diff: ['a', 'b'] }, futureField: { nested: true }, }) @@ -180,7 +180,7 @@ describe('ToolResultPruneService session transaction', () => { step: 1, callId: CallId('one'), isError: true, - error: { message: 'exit 1', info: { name: 'ExitError', code: 'EXIT_1' } }, + error: { name: 'ExitError', code: 'EXIT_1' }, meta: { diff: ['a', 'b'] }, futureField: { nested: true }, }, diff --git a/packages/cordis/tool-cordis/src/api-catalog.ts b/packages/cordis/tool-cordis/src/api-catalog.ts index e8ddc86f83..1537aa5960 100644 --- a/packages/cordis/tool-cordis/src/api-catalog.ts +++ b/packages/cordis/tool-cordis/src/api-catalog.ts @@ -1450,7 +1450,7 @@ export const TYPE_API: readonly TypeApiEntry[] = [ }, { name: 'SessionEventMap', - declaration: 'export interface SessionEventMap {\n \'turn/start\': {\n turn: number;\n trigger: TurnTrigger;\n };\n \'turn/end\': {\n turn: number;\n reason: TurnEndReason;\n };\n \'step/start\': {\n turn: number;\n step: number;\n };\n \'step/end\': {\n turn: number;\n step: number;\n };\n \'user/message\': {\n content: ContentBlock[];\n source: MessageSource;\n };\n \'prompt/blocked\': {\n content: ContentBlock[];\n source: MessageSource;\n reason: string;\n };\n \'context/message\': {\n content: ContentBlock[];\n source: MessageSource;\n meta?: JsonValue;\n };\n \'assistant/chunk\': {\n turn: number;\n step: number;\n chunk: StreamChunk;\n };\n \'assistant/message\': {\n turn: number;\n step: number;\n content: ContentBlock[];\n provenance: AssistantProvenance;\n usage?: TokenUsage;\n };\n \'tool/call\': {\n turn: number;\n step: number;\n callId: CallId;\n name: string;\n arguments: string;\n };\n \'tool/result\': {\n turn: number;\n step: number;\n callId: CallId;\n content: ContentBlock[];\n isError: boolean;\n error?: {\n message: string;\n info?: {\n name: string;\n code: string;\n };\n };\n meta?: JsonValue;\n };\n \'steering/message\': {\n turn: number;\n content: Content /* …truncated — full shape in source */', + declaration: 'export interface SessionEventMap {\n \'turn/start\': {\n turn: number;\n trigger: TurnTrigger;\n };\n \'turn/end\': {\n turn: number;\n reason: TurnEndReason;\n };\n \'step/start\': {\n turn: number;\n step: number;\n };\n \'step/end\': {\n turn: number;\n step: number;\n };\n \'user/message\': {\n content: ContentBlock[];\n source: MessageSource;\n };\n \'prompt/blocked\': {\n content: ContentBlock[];\n source: MessageSource;\n reason: string;\n };\n \'context/message\': {\n content: ContentBlock[];\n source: MessageSource;\n meta?: JsonValue;\n };\n \'assistant/chunk\': {\n turn: number;\n step: number;\n chunk: StreamChunk;\n };\n \'assistant/message\': {\n turn: number;\n step: number;\n content: ContentBlock[];\n provenance: AssistantProvenance;\n usage?: TokenUsage;\n };\n \'tool/call\': {\n turn: number;\n step: number;\n callId: CallId;\n name: string;\n arguments: string;\n };\n \'tool/result\': {\n turn: number;\n step: number;\n callId: CallId;\n content: ContentBlock[];\n isError: boolean;\n error?: {\n name: string;\n code: string;\n };\n meta?: JsonValue;\n };\n \'steering/message\': {\n turn: number;\n content: ContentBlock[];\n source: MessageSource;\n };\n \'todo/write\': {\n /* …truncated — full shape in source */', }, { name: 'SessionEventReadRequest', diff --git a/packages/core/agent-loop/src/tool-calls.ts b/packages/core/agent-loop/src/tool-calls.ts index 61a6d37947..e9a255dbce 100644 --- a/packages/core/agent-loop/src/tool-calls.ts +++ b/packages/core/agent-loop/src/tool-calls.ts @@ -248,7 +248,7 @@ function appendToolResult( callId: block.id, content: result.content, isError: result.isError, - ...result.error ? { error: result.error } : {}, + ...result.error?.info ? { error: result.error.info } : {}, // The tool's private presentation payload (e.g. a result-time diff), // persisted so a UI bridge reproduces the card on replay. ...result.meta !== undefined ? { meta: result.meta } : {}, diff --git a/packages/core/agent-loop/tests/cancel.spec.ts b/packages/core/agent-loop/tests/cancel.spec.ts index 301ebbf992..784855262b 100644 --- a/packages/core/agent-loop/tests/cancel.spec.ts +++ b/packages/core/agent-loop/tests/cancel.spec.ts @@ -397,7 +397,7 @@ describe('Agent.cancel()', () => { expect(result?.type === 'tool/result' ? result.data : undefined).toMatchObject({ callId: 'c1', isError: true, - error: { info: { name: 'AbortError', code: 'ABORTED' } }, + error: { name: 'AbortError', code: 'ABORTED' }, }) send(agent, 'continue safely') diff --git a/packages/core/agent-loop/tests/contract-regressions.spec.ts b/packages/core/agent-loop/tests/contract-regressions.spec.ts index 17f12de400..97bf57ad72 100644 --- a/packages/core/agent-loop/tests/contract-regressions.spec.ts +++ b/packages/core/agent-loop/tests/contract-regressions.spec.ts @@ -259,7 +259,7 @@ describe('abort during tool execution ends the turn', () => { case 'assistant/message': order.push('assistant/message'); break case 'tool/call': order.push(`tool/call:${event.data.callId}`); break case 'tool/result': { - const outcome = event.data.error?.info?.code === 'ABORTED' ? 'synthetic-aborted' : 'real' + const outcome = event.data.error?.code === 'ABORTED' ? 'synthetic-aborted' : 'real' order.push(`tool/result:${event.data.callId}:${outcome}`) break } @@ -308,7 +308,7 @@ describe('abort during tool execution ends the turn', () => { expect(results[1]!.data).toMatchObject({ callId: CallId('c2'), isError: true, - error: { info: { name: 'AbortError', code: 'ABORTED' } }, + error: { name: 'AbortError', code: 'ABORTED' }, }) }) diff --git a/packages/core/agent-loop/tests/tool-calls.spec.ts b/packages/core/agent-loop/tests/tool-calls.spec.ts index e13caa13c5..0d6d20b287 100644 --- a/packages/core/agent-loop/tests/tool-calls.spec.ts +++ b/packages/core/agent-loop/tests/tool-calls.spec.ts @@ -479,18 +479,12 @@ describe('tool-call scheduler: abort handling', () => { { callId: CallId('c1'), isError: true, - error: { - message: 'tool call skipped because the step was aborted before execution', - info: { name: 'AbortError', code: 'ABORTED' }, - }, + error: { name: 'AbortError', code: 'ABORTED' }, }, { callId: CallId('c2'), isError: true, - error: { - message: 'tool call skipped because the step was aborted before execution', - info: { name: 'AbortError', code: 'ABORTED' }, - }, + error: { name: 'AbortError', code: 'ABORTED' }, }, ]) }) @@ -523,7 +517,7 @@ describe('tool-call scheduler: abort handling', () => { expect(events(agent).filter(e => e.type === 'tool/result').map(e => e.data.callId)) .toEqual([CallId('c1'), CallId('c2')]) expect(events(agent).filter(e => e.type === 'tool/result').at(-1)?.data) - .toMatchObject({ callId: CallId('c2'), isError: true, error: { info: { name: 'AbortError', code: 'ABORTED' } } }) + .toMatchObject({ callId: CallId('c2'), isError: true, error: { name: 'AbortError', code: 'ABORTED' } }) }) it('stops replenishing after abort, commits started results, and drains accepted additional contexts', async () => { @@ -555,7 +549,7 @@ describe('tool-call scheduler: abort handling', () => { expect(events(agent).filter(e => e.type === 'tool/result').slice(-2).map(e => ({ callId: e.data.callId, isError: e.data.isError, - errorInfo: e.data.error?.info, + errorInfo: e.data.error, }))) .toEqual([ { callId: CallId('c3'), isError: true, errorInfo: { name: 'AbortError', code: 'ABORTED' } }, @@ -601,6 +595,6 @@ describe('tool-call scheduler: abort handling', () => { expect(events(agent).filter(e => e.type === 'tool/call').map(e => e.data.callId)) .toEqual([CallId('c1'), CallId('c2'), CallId('c3')]) expect(events(agent).filter(e => e.type === 'tool/result').at(-1)?.data) - .toMatchObject({ callId: CallId('c3'), isError: true, error: { info: { name: 'AbortError', code: 'ABORTED' } } }) + .toMatchObject({ callId: CallId('c3'), isError: true, error: { name: 'AbortError', code: 'ABORTED' } }) }) }) diff --git a/packages/core/session/README.md b/packages/core/session/README.md index aaf1d429ee..107eaf25bf 100644 --- a/packages/core/session/README.md +++ b/packages/core/session/README.md @@ -58,7 +58,7 @@ Durable values need one accepted representation, not a check followed by a secon `context/message` renders its `content` verbatim as a user-role message, and may attach JSON `meta` for replayable plugin state; metadata remains durable but is excluded from `deriveMessages()`. -`tool/result` persists the model-facing content, canonical failure detail, and optional presentation metadata. A tool's successful canonical `value` is deliberately execution-local and never enters the session event, so replay reconstructs the Native/model presentation but cannot recover intermediate programmatic values. This does not change `SESSION_FORMAT_VERSION`: the persisted projection remains authoritative. +`tool/result` persists the model-facing content, optional internal failure identity, and optional presentation metadata. A tool's successful canonical `value` and human-readable canonical failure message remain execution-local; rendered error content is the replay-authoritative message. This preserves the existing event shape and does not change `SESSION_FORMAT_VERSION`. ### Session event vocabulary (`types.ts`) diff --git a/packages/core/session/src/repair.ts b/packages/core/session/src/repair.ts index fa3b8bbb50..efbb3d2004 100644 --- a/packages/core/session/src/repair.ts +++ b/packages/core/session/src/repair.ts @@ -92,10 +92,7 @@ export function interruptedTurnClosers(events: readonly SessionEvent[]): Session callId, content: [{ type: 'text', text: 'Tool call interrupted by a crash; no result was recorded.' }], isError: true, - error: { - message: 'Tool call interrupted by a crash; no result was recorded.', - info: { name: 'InterruptedError', code: 'interrupted' }, - }, + error: { name: 'InterruptedError', code: 'interrupted' }, }, surfaceOp: 'append', ...callSeq !== undefined ? { sourceEventSeqs: [callSeq] } : {}, diff --git a/packages/core/session/src/types.ts b/packages/core/session/src/types.ts index cc7ea81ba1..0d253e9091 100644 --- a/packages/core/session/src/types.ts +++ b/packages/core/session/src/types.ts @@ -243,12 +243,13 @@ export interface SessionEventMap { */ 'tool/call': { turn: number; step: number; callId: CallId; name: string; arguments: string } /** - * A completed tool call's model-facing result, canonical failure detail, and - * optional tool-private `meta` presentation payload. `meta` is opaque to the - * core (the producing tool owns its shape and reads it back in `presentResult`) - * but MUST be JSON-serializable: `Session.append` runtime-validates all event - * data with `isJsonValue`, so a non-serializable `meta` is rejected at the - * source, and the durable log reproduces the identical card on replay. Absent + * A completed tool call's model-facing result, optional internal failure + * identity, and optional tool-private `meta` presentation payload. `meta` is + * opaque to the core (the producing tool owns its shape and reads it back in + * `presentResult`) but MUST be JSON-serializable: `Session.append` + * runtime-validates all event data with `isJsonValue`, so a non-serializable + * `meta` is rejected at the source, and the durable log reproduces the + * identical card on replay. Absent * unless the tool attaches one (e.g. `dsh-tool-fs` carries its result-time * contextual diff here). */ @@ -258,7 +259,7 @@ export interface SessionEventMap { callId: CallId content: ContentBlock[] isError: boolean - error?: { message: string; info?: { name: string; code: string } } + error?: { name: string; code: string } meta?: JsonValue } /** Steering content injected between steps of a running turn. */ diff --git a/packages/core/session/tests/repair.spec.ts b/packages/core/session/tests/repair.spec.ts index 826d410dc8..765502b8ce 100644 --- a/packages/core/session/tests/repair.spec.ts +++ b/packages/core/session/tests/repair.spec.ts @@ -64,7 +64,7 @@ describe('interruptedTurnClosers', () => { expect(closers.map(e => e.seq)).toEqual([3, 4, 5]) const result = closers[0]! expect(result.type === 'tool/result' && result.data).toMatchObject({ - turn: 2, step: 1, callId: CallId('call-1'), isError: true, error: { info: { code: 'interrupted' } }, + turn: 2, step: 1, callId: CallId('call-1'), isError: true, error: { code: 'interrupted' }, }) }) diff --git a/packages/core/tools/src/index.ts b/packages/core/tools/src/index.ts index f6fe38d4da..58b6824ada 100644 --- a/packages/core/tools/src/index.ts +++ b/packages/core/tools/src/index.ts @@ -349,6 +349,25 @@ export class ToolOutputError extends HarnessError { } } +/** Convert one projector exception into the canonical invalid-output failure. */ +function projectionError(toolName: string, projector: 'render' | 'presentationMeta', error: unknown): ToolOutputError { + return new ToolOutputError(toolName, [`output.${projector} failed: ${errorMessage(error)}`]) +} + +/** Snapshot one projector result before later durable-result materialization. */ +function snapshotProjection(toolName: string, projector: 'render' | 'presentationMeta', candidate: T): T { + try { + const detached = snapshotJsonValue(candidate) + if (detached === undefined) { + throw new ToolOutputError(toolName, [`output.${projector} returned non-lossless JSON`]) + } + return detached + } catch (error: unknown) { + if (error instanceof ToolOutputError) throw error + throw projectionError(toolName, projector, error) + } +} + /** Successful canonical tool execution, including its Native/model projection. */ export interface ToolExecutionSuccess { readonly isError: false @@ -1156,10 +1175,23 @@ export class ToolRegistry extends Service { const violations = validateJsonSchemaValue(tool.output.schema, detached, 'value') if (violations.length > 0) throw new ToolOutputError(tool.name, violations) const value = deepFreeze(detached as JsonValue) - const content = tool.output.render(exec.arguments, value) - const meta = exec.parent === undefined && tool.output.presentationMeta !== undefined - ? tool.output.presentationMeta(exec.arguments, value) - : undefined + let rendered: ContentBlock[] + try { + rendered = tool.output.render(exec.arguments, value) + } catch (error: unknown) { + throw projectionError(tool.name, 'render', error) + } + const content = snapshotProjection(tool.name, 'render', rendered) + let meta: JsonValue | undefined + if (exec.parent === undefined && tool.output.presentationMeta !== undefined) { + let projected: JsonValue + try { + projected = tool.output.presentationMeta(exec.arguments, value) + } catch (error: unknown) { + throw projectionError(tool.name, 'presentationMeta', error) + } + meta = snapshotProjection(tool.name, 'presentationMeta', projected) + } return this.markCanonical(this.materializeFinalResult({ isError: false, value, diff --git a/packages/core/tools/tests/tools.spec.ts b/packages/core/tools/tests/tools.spec.ts index a8ccf384d5..d237430dda 100644 --- a/packages/core/tools/tests/tools.spec.ts +++ b/packages/core/tools/tests/tools.spec.ts @@ -147,6 +147,7 @@ describe('ToolRegistry', () => { }) expect(result.isError).toBe(true) expect(result.content[0]?.type === 'text' && result.content[0].text).toContain('Error:') + expect(result.error).toMatchObject({ info: { name: 'ToolOutputError', code: 'INVALID_TOOL_OUTPUT' } }) expect(observedError).toBe(true) }) @@ -209,10 +210,10 @@ describe('ToolRegistry', () => { })) const result = await ctx.tools.execute({ callId: CallId(projector), name: `throwing-${projector}`, arguments: {} }) - expect(result).toMatchObject({ - isError: true, - error: { message: projector === 'render' ? 'renderer exploded' : 'metadata exploded' }, - }) + expect(result.isError).toBe(true) + expect(result.error?.message) + .toContain(projector === 'render' ? 'renderer exploded' : 'metadata exploded') + expect(result.error?.info).toEqual({ name: 'ToolOutputError', code: 'INVALID_TOOL_OUTPUT' }) expect('value' in result).toBe(false) }) diff --git a/packages/fs/tool-fs/src/read-render.ts b/packages/fs/tool-fs/src/read-render.ts index de9e530a13..e30dad7bcd 100644 --- a/packages/fs/tool-fs/src/read-render.ts +++ b/packages/fs/tool-fs/src/read-render.ts @@ -138,6 +138,7 @@ export async function buildWindow( appendToLineBuffer(chunk.slice(startPos, newlinePos)) flushLine() startPos = newlinePos + 1 + if (acc.done) return finish(acc, request, displayPath) } appendToLineBuffer(chunk.slice(startPos)) } diff --git a/packages/mcp/mcp-client/package.json b/packages/mcp/mcp-client/package.json index 6b8f145108..ff34be015c 100644 --- a/packages/mcp/mcp-client/package.json +++ b/packages/mcp/mcp-client/package.json @@ -28,14 +28,14 @@ }, "dependencies": { "@modelcontextprotocol/sdk": "^1.12.0", - "schemastery": "^3.18.0" + "schemastery": "^3.18.0", + "zod": "^4.4.3" }, "devDependencies": { "@deepseek-ai/dsh-tools": "workspace:^", "@deepseek-ai/dsh-llm": "workspace:^", "@modelcontextprotocol/server-everything": "^2026.7.4", "@modelcontextprotocol/server-filesystem": "^2026.7.4", - "cordis": "^4.0.0-rc.7", - "zod": "^4.4.3" + "cordis": "^4.0.0-rc.7" } } diff --git a/packages/mcp/mcp-client/src/tools.ts b/packages/mcp/mcp-client/src/tools.ts index 1a8eabf416..be0c6ebea7 100644 --- a/packages/mcp/mcp-client/src/tools.ts +++ b/packages/mcp/mcp-client/src/tools.ts @@ -14,6 +14,8 @@ import { createHash } from 'node:crypto' import type { Client } from '@modelcontextprotocol/sdk/client/index.js' +import { ListToolsResultSchema } from '@modelcontextprotocol/sdk/types.js' +import { z } from 'zod' import type { Context } from 'cordis' import type { ToolDefinition, ToolExecution } from '@deepseek-ai/dsh-tools' import { assertSupportedJsonSchema } from '@deepseek-ai/dsh-tools' @@ -46,6 +48,35 @@ const INVALID_NAME_CHARS = /[^A-Za-z0-9_-]/g /** Hex chars of the SHA-256 identity hash appended on lossy normalization. */ const HASH_LENGTH = 12 +/** Raw result record: the bridge owns JSON-value validation after transport. */ +const RawCallToolResultSchema = z.record(z.string(), z.unknown()) + +/** List without mutating the SDK's per-page output-validator cache. */ +function listToolsUncached(client: Client, cursor?: string) { + return client.request( + { method: 'tools/list', ...cursor === undefined ? {} : { params: { cursor } } }, + ListToolsResultSchema, + ) +} + +/** Call without the SDK pre-validating an output schema the bridge may not support. */ +function callToolUncached( + client: Client, + rawName: string, + args: Record, + exec: ToolExecution, + opts: ToolBridgeOptions, +) { + return client.request( + { method: 'tools/call', params: { name: rawName, arguments: args } }, + RawCallToolResultSchema, + { + ...exec.signal ? { signal: exec.signal } : {}, + timeout: opts.toolCallTimeoutMs, + }, + ) +} + /** * Derive the model-facing public name for one MCP tool. * @@ -73,7 +104,7 @@ export function publicToolName(serverName: string, rawName: string): string { * * Two phases keep the swap safe: * - * 1. Fetch: drain `client.listTools()` pagination and build the full next + * 1. Fetch: drain uncached `tools/list` pagination and build the full next * generation of `ToolDefinition`s under public names. Any failure here * (network error, duplicate raw name in the server's list) rejects and * leaves the previous generation registered untouched. @@ -101,7 +132,7 @@ export async function syncTools( const definitions = new Map() let cursor: string | undefined do { - const response = await client.listTools(cursor ? { cursor } : undefined) + const response = await listToolsUncached(client, cursor) for (const tool of response.tools) { const publicName = publicToolName(opts.serverName, tool.name) if (definitions.has(publicName)) { @@ -114,7 +145,7 @@ export async function syncTools( description: tool.description ?? '', parameters: tool.inputSchema, output: createOutput(tool.name, supportedOutputSchema(tool.outputSchema)), - execute: createExecutor(client, tool.name, opts), + execute: createExecutor(client, tool.name, tool.execution?.taskSupport === 'required', opts), }) } cursor = response.nextCursor @@ -170,7 +201,7 @@ function createOutput(rawName: string, structuredSchema: JsonSchemaNode | undefi content: { type: 'array', items: {} }, structuredContent: structuredSchema ?? {}, }, - required: ['content'], + required: structuredSchema === undefined ? ['content'] : ['content', 'structuredContent'], additionalProperties: false, }, render(_args, value) { @@ -182,9 +213,10 @@ function createOutput(rawName: string, structuredSchema: JsonSchemaNode | undefi /** * Create an execute function for one MCP tool. The executor closes over the - * raw MCP tool name and calls `client.callTool` with it (never the public - * name), with abort signal and timeout, then maps the result to harness - * ContentBlocks. + * raw MCP tool name and sends an uncached `tools/call` request with it (never + * the public name), with abort signal and timeout, then maps the result to + * harness ContentBlocks. Owning the raw request prevents the SDK's internal + * per-page schema cache from pre-validating a different contract. * * When the MCP server returns `isError: true`, the executor throws so that * the ToolRegistry's catch path produces an `isError` result for the model. @@ -192,33 +224,30 @@ function createOutput(rawName: string, structuredSchema: JsonSchemaNode | undefi function createExecutor( client: Client, rawName: string, + taskRequired: boolean, opts: ToolBridgeOptions, ): ToolDefinition['execute'] { return async (args: unknown, exec: ToolExecution) => { + if (taskRequired) { + throw new Error(`Tool "${rawName}" requires task-based execution, which this bridge does not support`) + } // The agent loop passes `JSON.parse(model_arguments)` which is usually an // object, but can be any JSON value if the model misbehaves (outputs a bare // string/number/null). Fallback to {} lets the MCP server produce a // specific "missing required param" error the model can learn from. const argsObj = (typeof args === 'object' && args !== null ? args : {}) as Record - const result = await client.callTool( - { name: rawName, arguments: argsObj }, - undefined, - { - ...exec.signal ? { signal: exec.signal } : {}, - timeout: opts.toolCallTimeoutMs, - }, - ) + const result = await callToolUncached(client, rawName, argsObj, exec, opts) // The SDK may return a legacy `toolResult` shape; normalize to content array. - if (!('content' in result) || !Array.isArray(result.content)) { + if (!Array.isArray(result.content)) { const rendered: unknown = 'toolResult' in result ? JSON.stringify(result.toolResult) : '(no output)' const text = typeof rendered === 'string' ? rendered : '(no output)' - if ('isError' in result && result.isError === true) throw new Error(text) + if (result.isError === true) throw new Error(text) return { content: [{ type: 'text', text }], - ...'structuredContent' in result && result.structuredContent !== undefined + ...result.structuredContent !== undefined ? { structuredContent: result.structuredContent as JsonValue } : {}, } @@ -232,13 +261,13 @@ function createExecutor( const text = extractText(content, rawName) // MCP isError → throw so ToolRegistry produces an isError result for the model. - if ('isError' in result && result.isError === true) { + if (result.isError === true) { throw new Error(text) } return { content, - ...'structuredContent' in result && result.structuredContent !== undefined + ...result.structuredContent !== undefined ? { structuredContent: result.structuredContent as JsonValue } : {}, } diff --git a/packages/mcp/mcp-client/tests/mcp-client.spec.ts b/packages/mcp/mcp-client/tests/mcp-client.spec.ts index 30e7a6297c..77c61e53a7 100644 --- a/packages/mcp/mcp-client/tests/mcp-client.spec.ts +++ b/packages/mcp/mcp-client/tests/mcp-client.spec.ts @@ -1,4 +1,6 @@ import { describe, expect, it, vi, beforeEach } from 'vitest' +import { Client } from '@modelcontextprotocol/sdk/client/index.js' +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js' import { Context } from 'cordis' import { CallId } from '@deepseek-ai/dsh-llm' import SystemPrompt from '@deepseek-ai/dsh-system-prompt' @@ -14,6 +16,7 @@ interface MockTool { description?: string inputSchema: Record outputSchema?: Record + execution?: { taskSupport?: 'optional' | 'required' | 'forbidden' } } interface MockCallResult { @@ -23,9 +26,26 @@ interface MockCallResult { } function createMockClient(tools: MockTool[], callResult: MockCallResult = { content: [{ type: 'text', text: 'ok' }] }) { + const listTools = vi.fn(async ( + _params?: Record, + ): Promise<{ tools: MockTool[]; nextCursor: string | undefined }> => ({ tools, nextCursor: undefined })) + const callTool = vi.fn(async ( + _params?: Record, + _compatibilitySchema?: unknown, + _options?: unknown, + ): Promise> => ({ ...callResult })) return { - listTools: vi.fn().mockResolvedValue({ tools, nextCursor: undefined }), - callTool: vi.fn().mockResolvedValue(callResult), + listTools, + callTool, + request: vi.fn(async ( + request: { method: string; params?: Record }, + _schema: unknown, + options?: unknown, + ): Promise => { + if (request.method === 'tools/list') return listTools(request.params) + if (request.method === 'tools/call') return callTool(request.params, undefined, options) + throw new Error(`unexpected MCP request: ${request.method}`) + }), setNotificationHandler: vi.fn(), connect: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined), @@ -205,6 +225,80 @@ describe('syncTools', () => { expect(ctx.tools.get('mcp__srv__page1')).toBeDefined() expect(ctx.tools.get('mcp__srv__page2')).toBeDefined() }) + + it('owns output validation independently of the SDK per-page cache', async () => { + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair() + serverTransport.onmessage = (message) => { + if (!('id' in message) || !('method' in message)) return + const params = 'params' in message ? message.params : undefined + let result: Record + if (message.method === 'initialize') { + const protocolVersion = params && 'protocolVersion' in params + ? params.protocolVersion + : '2025-11-25' + result = { + protocolVersion, + capabilities: { tools: {} }, + serverInfo: { name: 'raw-test', version: '1' }, + } + } else if (message.method === 'tools/list') { + const cursor = params && 'cursor' in params ? params.cursor : undefined + result = cursor === undefined + ? { + tools: [{ + name: 'supported', + inputSchema: { type: 'object' }, + outputSchema: { + type: 'object', + additionalProperties: false, + properties: { answer: { type: 'integer' } }, + required: ['answer'], + }, + }], + nextCursor: 'page-2', + } + : { + tools: [{ + name: 'future-schema', + inputSchema: { type: 'object' }, + outputSchema: { type: 'object', patternProperties: { '^x-': { type: 'string' } } }, + }], + } + } else if (message.method === 'tools/call') { + const name = params && 'name' in params ? params.name : undefined + result = name === 'supported' + ? { content: [{ type: 'text', text: 'missing structured content' }] } + : { content: [42, null], structuredContent: ['kept', { nested: true }] } + } else { + result = {} + } + void serverTransport.send({ jsonrpc: '2.0', id: message.id, result }) + } + await serverTransport.start() + const client = new Client({ name: 'cache-independent-test', version: '1' }) + await client.connect(clientTransport) + + try { + await syncTools(client, ctx, defaultOpts, new Map()) + + const missing = await ctx.tools.execute({ + callId: CallId('missing'), name: 'mcp__srv__supported', arguments: {}, + }) + expect(missing.error).toMatchObject({ info: { code: 'INVALID_TOOL_OUTPUT' } }) + expect(missing.error?.message).toContain('structuredContent') + + const fallback = await ctx.tools.execute({ + callId: CallId('fallback'), name: 'mcp__srv__future-schema', arguments: {}, + }) + if (fallback.isError) throw new Error('unsupported schema must use the bridge fallback') + expect(fallback.value).toEqual({ + content: [42, null], + structuredContent: ['kept', { nested: true }], + }) + } finally { + await client.close() + } + }) }) describe('tool execution', () => { @@ -360,6 +454,21 @@ describe('tool execution', () => { expect('value' in result).toBe(false) }) + it('rejects tools that require task-based execution', async () => { + const client = createMockClient([ + { name: 'task-only', inputSchema: { type: 'object' }, execution: { taskSupport: 'required' } }, + ]) + + await syncTools(client as never, ctx, defaultOpts, new Map()) + const result = await ctx.tools.execute({ + callId: CallId('task-only'), name: 'mcp__srv__task-only', arguments: {}, + }) + + expect(result.isError).toBe(true) + expect(result.error?.message).toContain('requires task-based execution') + expect(client.callTool).not.toHaveBeenCalled() + }) + it('passes abort signal to callTool', async () => { const controller = new AbortController() const client = createMockClient( diff --git a/packages/session-persistence/session-persistence/tests/contract.ts b/packages/session-persistence/session-persistence/tests/contract.ts index d36ed98234..84386c016b 100644 --- a/packages/session-persistence/session-persistence/tests/contract.ts +++ b/packages/session-persistence/session-persistence/tests/contract.ts @@ -149,7 +149,7 @@ export function runPersistenceContract(name: string, make: () => Promise e.type === 'tool/result') expect(synthetic?.type === 'tool/result' && synthetic.data).toMatchObject({ - callId: CallId('call-x'), isError: true, error: { info: { code: 'interrupted' } }, + callId: CallId('call-x'), isError: true, error: { code: 'interrupted' }, }) // The synthetic result carries the SAME callId as the orphaned tool-call, // so deriveMessages() pairs them — no provider-invalid dangling call. diff --git a/packages/support/invariants/src/index.ts b/packages/support/invariants/src/index.ts index 4a6b39be72..91b4da4566 100644 --- a/packages/support/invariants/src/index.ts +++ b/packages/support/invariants/src/index.ts @@ -165,7 +165,7 @@ function validateEvent(trace: SessionTrace, event: SessionEvent): SessionTraceTr // A result needs a prior matching call in the same step. (The converse // does NOT hold: a call may have no result — a throwing tool-execution // pipeline step ends the turn with no tool/result, which is legal.) - const syntheticInterrupted = event.data.isError && event.data.error?.info?.code === 'interrupted' + const syntheticInterrupted = event.data.isError && event.data.error?.code === 'interrupted' if (!trace.pendingCalls.has(event.data.callId) && !syntheticInterrupted) { throw new InvariantError(`tool/result for ${event.data.callId} with no prior tool/call in this step`) } diff --git a/packages/support/invariants/tests/invariants.spec.ts b/packages/support/invariants/tests/invariants.spec.ts index e9a09ea356..4f066eff8c 100644 --- a/packages/support/invariants/tests/invariants.spec.ts +++ b/packages/support/invariants/tests/invariants.spec.ts @@ -217,7 +217,7 @@ describe('session-log invariants', () => { callId: CallId('crashed'), content: [{ type: 'text', text: 'interrupted' }], isError: true, - error: { message: 'interrupted', info: { name: 'InterruptedError', code: 'interrupted' } }, + error: { name: 'InterruptedError', code: 'interrupted' }, }, { surfaceOp: 'append' }) session.append('step/end', { turn: 1, step: 1 }) session.append('turn/end', { turn: 1, reason: { kind: 'interrupted' } }) @@ -500,7 +500,7 @@ describe('surface contract under the invariants composition', () => { callId: CallId('rewrite'), content: [{ type: 'text' as const, text: 'original' }], isError: true, - error: { message: 'exit 1', info: { name: 'ExitError', code: 'EXIT_1' } }, + error: { name: 'ExitError', code: 'EXIT_1' }, meta: { presentation: { kind: 'terminal', output: 'full output' } }, futureField: { nested: ['preserve', 1] }, } @@ -585,7 +585,7 @@ describe('surface contract under the invariants composition', () => { ['callId', { callId: CallId('forged') }], ['turn', { turn: 2 }], ['step', { step: 2 }], - ['error', { error: { message: 'exit 1', info: { name: 'ExitError', code: 'DIFFERENT' } } }], + ['error', { error: { name: 'ExitError', code: 'DIFFERENT' } }], ['meta', { meta: { presentation: { kind: 'generic' } } }], ['future data', { futureField: { nested: ['changed'] } }], ])('rejects a content rewrite with altered %s', async (_label, altered) => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 903d8a7ca2..ee04300c15 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1447,6 +1447,9 @@ importers: schemastery: specifier: ^3.18.0 version: 3.18.0 + zod: + specifier: ^4.4.3 + version: 4.4.3 devDependencies: '@deepseek-ai/dsh-llm': specifier: workspace:^ @@ -1463,9 +1466,6 @@ importers: cordis: specifier: ^4.0.0-rc.7 version: 4.0.0-rc.7(@cordisjs/plugin-include@1.0.4)(@cordisjs/plugin-loader@1.0.0-rc.5) - zod: - specifier: ^4.4.3 - version: 4.4.3 packages/sandbox/sandbox: devDependencies: