From 5c7f515710b44610be775bb1c5ad0668e7d5b11b Mon Sep 17 00:00:00 2001 From: Anarchid Date: Fri, 16 Oct 2026 14:22:27 +0300 Subject: [PATCH 1/7] fix(tool-results): send a string result as its text, not JSON-quoted A tool result whose data is a string was JSON-encoded on its way to the model: quoted, with every newline, tab and quote escaped. A module tool that answers with a line-per-item listing reached context, the live wire and its spill file as one escaped line, which workspace--read could then page only by character and workspace--grep matched as a single line. toolResultDataToHistoryString now keeps a string as it is; objects, other scalars and non-MCP arrays still go as JSON, MCP content arrays as their blocks. The live wire's fallback in toMembraneToolResult follows the same rule, so the four sites that share the serializer (stored history, the live wire, scripts, the direct Agent API) stay converged. Tests: the serializer keeps a string verbatim and truncates its text; an oversized string spills as its own lines and grep finds one by line number; a small one reaches history and the wire byte-identical; a code_execution script gets the string itself. Two tests that pinned the quoted form are updated. Co-Authored-By: Claude Opus 5.5 --- .../verbatim-string-tool-results.changed.md | 14 +++++ src/framework.ts | 9 +-- src/tool-result-history.ts | 10 +++- src/types/events.ts | 5 +- test/code-execution.test.ts | 54 +++++++++++++++++ test/tool-result-guard.test.ts | 6 +- test/tool-result-history.test.ts | 20 ++++++- test/tool-result-spill.test.ts | 58 +++++++++++++++++++ test/unconvertible-tool-result.test.ts | 2 +- 9 files changed, 166 insertions(+), 12 deletions(-) create mode 100644 changelog.d/verbatim-string-tool-results.changed.md diff --git a/changelog.d/verbatim-string-tool-results.changed.md b/changelog.d/verbatim-string-tool-results.changed.md new file mode 100644 index 000000000..fc9964b99 --- /dev/null +++ b/changelog.d/verbatim-string-tool-results.changed.md @@ -0,0 +1,14 @@ +- A tool result whose `data` is a string now reaches the model as that text. + It was JSON-encoded on the way, quoted with every newline, tab and quote + escaped, so a multi-line answer, such as a module tool's line-per-item + listing, arrived as one escaped line: in context, on the live wire, and in + its spill file under `tool-results/`, which `workspace--read` could then + page only by character and `workspace--grep` matched as a single line. A + spill file now holds the string as the tool returned it. Objects, numbers, + booleans and arrays other than MCP content arrays are still sent as JSON, + and MCP content arrays as their blocks. + + A `code_execution` script's tool function returns the same text, so it now + gets a string result as it is. A script that ran `json.loads` on such a + result to unquote it now gets an error unless the string itself is JSON. + `data: ''` is stored as no text, as `data: undefined` already was. diff --git a/src/framework.ts b/src/framework.ts index 94f1035ff..b0060306f 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -14851,9 +14851,10 @@ export class AgentFramework { }; } // MCPL tool results arrive as `data: McpToolResultContent[]` — preserve image - // blocks natively rather than JSON-stringifying them away. Anything else - // (objects, scalars) falls through to JSON. The error path was handled - // above, so isError is always false on these return paths. + // blocks natively rather than JSON-stringifying them away. A string goes + // as it is, as history stores it; anything else (objects, other scalars) + // falls through to JSON. The error path was handled above, so isError is + // always false on these return paths. const blocks = this.tryNativeToolResultContent(afResult.data, maxChars, precomputed?.filePath ?? null); if (blocks) { return { toolUseId: callId, content: blocks, isError: false }; @@ -14864,7 +14865,7 @@ export class AgentFramework { // JSON.stringify returns the VALUE undefined (not a string) for undefined // input — a module tool returning `{ success: true }` with no data would // otherwise make `content.length` below throw a TypeError mid-turn. - let content = JSON.stringify(afResult.data) ?? ''; + let content = typeof afResult.data === 'string' ? afResult.data : JSON.stringify(afResult.data) ?? ''; if (maxChars && content.length > maxChars) { content = safeSlice(content, 0, maxChars) + '\n\n[truncated — original was ' + content.length + ' chars]'; diff --git a/src/tool-result-history.ts b/src/tool-result-history.ts index 73ddf431e..3e99ecfb1 100644 --- a/src/tool-result-history.ts +++ b/src/tool-result-history.ts @@ -11,7 +11,8 @@ * * For MCP-shaped content arrays, this helper keeps text blocks verbatim and * replaces image blocks with a short `[image: mimeType, ~NKB]` placeholder. - * For anything else it falls back to JSON. + * A string is already text and is kept as it is. For anything else it falls + * back to JSON. */ import { INLINE_WITHHELD_TEXT, isInlineContradiction, referenceStubOrNull } from './mcpl/references.js'; @@ -52,9 +53,14 @@ export function toolResultDataToHistoryString( options?: HistorySerializeOptions, ): string { const fromArray = tryHistoryStringFromContentArray(data, options); + // A string is the text the tool means to say, kept as it is: JSON would + // quote it and escape every newline, so a multi-line result would reach + // the model, a script and its spill file as one escaped line. // JSON leaves out undefined, a function and a symbol: a result with no data // is stored as no text, as an empty content array is. - const str = fromArray ?? (JSON.stringify(data) as string | undefined) ?? ''; + const str = fromArray + ?? (typeof data === 'string' ? data : JSON.stringify(data) as string | undefined) + ?? ''; return maxChars ? truncateForHistory(str, maxChars) : str; } diff --git a/src/types/events.ts b/src/types/events.ts index 0ef8de110..f3e5e12a6 100644 --- a/src/types/events.ts +++ b/src/types/events.ts @@ -186,7 +186,10 @@ export interface ModuleEvent { export interface ToolResult { /** Whether the tool succeeded */ success: boolean; - /** Result data (if success) */ + /** + * Result data (if success). The model reads a string as that text, an MCP + * content array as its blocks, and anything else as JSON. + */ data?: unknown; /** Error message (if failure) */ error?: string; diff --git a/test/code-execution.test.ts b/test/code-execution.test.ts index 5deec59aa..075433758 100644 --- a/test/code-execution.test.ts +++ b/test/code-execution.test.ts @@ -521,6 +521,60 @@ describe('framework code_execution integration (real python3)', () => { } }); + it('a module tool that returns a string hands the script that string, not its JSON encoding', async () => { + // The tool contract: a script's tool function returns "the same text the + // tool would have returned to you directly". For a string result that + // is the string itself, newlines and quotes intact. + const listing = 'As of 10:00.\n[review] org/repo#1 "Fix it"'; + class ListingModule extends ScriptToolModule { + override getTools(): ToolDefinition[] { + return [...super.getTools(), { + name: 'listing', + description: 'A line-per-item listing.', + inputSchema: { type: 'object', properties: {} }, + }]; + } + override async handleToolCall(call: ToolCall): Promise { + if (call.name.endsWith('listing')) return { success: true, data: listing }; + return super.handleToolCall(call); + } + } + const { tempDir, storePath } = tempStorePath('pytc-string-'); + const membrane = new MockMembrane(); + const code = [ + 'r = await test__listing({})', + 'lines = r.split("\\n")', + 'print("count:", len(lines))', + 'print("second:", lines[1])', + 'print("quoted:", r.startswith(\'"\'))', + ].join('\n'); + membrane.pushResponse(createMockResponse([ + { type: 'tool_use', id: 'call_ce_str', name: 'code_execution', input: { code } }, + ], 'tool_use')); + membrane.pushResponse(createMockResponse([{ type: 'text', text: 'Done.' }])); + + const framework = await createFrameworkWithCodeExecution(storePath, membrane, new ListingModule()); + try { + const created = await framework.createEphemeralAgent({ + name: 'worker', model: 'test-model', systemPrompt: 'Do the task.', allowedTools: 'all', + }); + created.contextManager.addMessage('user', [{ type: 'text', text: 'Go.' }]); + const promise = framework.runEphemeralToCompletion(created.agent, created.contextManager); + framework.start(); + await promise; + + const wireResult = membrane.lastStream!.receivedToolResults[0][0] as { content: string }; + const out = JSON.parse(wireResult.content) as { stdout: string; return_code: number }; + assert.strictEqual(out.return_code, 0, wireResult.content); + assert.match(out.stdout, /count: 2\n/); + assert.match(out.stdout, /second: \[review\] org\/repo#1 "Fix it"\n/); + assert.match(out.stdout, /quoted: False/); + } finally { + await framework.stop(); + rmSync(tempDir, { recursive: true, force: true }); + } + }); + it('two code_execution calls in one response: one runs, the other is refused, the turn completes', async () => { const { tempDir, storePath } = tempStorePath('pytc-two-'); const membrane = new MockMembrane(); diff --git a/test/tool-result-guard.test.ts b/test/tool-result-guard.test.ts index 1f999935a..f97ece6fd 100644 --- a/test/tool-result-guard.test.ts +++ b/test/tool-result-guard.test.ts @@ -1814,14 +1814,14 @@ test('#281: a withheld result over the inline cap is written as stored, and its const withheld = await h.workspace.readBinary(path); assert.ok('data' in withheld); const text = (withheld as { data: Buffer }).data.toString('utf8'); - // The data is a string, stored as its JSON: 407 characters with the quotes. - assert.match(text, /^"long-x{94}\n\n\[truncated — showing 100 of 407 chars; full content: workspace file /, + // The data is a string, stored as it is: 405 characters. + assert.match(text, /^long-x{95}\n\n\[truncated — showing 100 of 405 chars; full content: workspace file /, 'the withheld file holds the capped text, as stored'); const whole = /full content: workspace file (work\/tool-results\/[^\s\]"]+\.txt)/.exec(text)?.[1]; assert.ok(whole && whole !== path, `its note names the file with the whole output: ${text}`); const full = await h.workspace.readBinary(whole); assert.ok('data' in full); - assert.match((full as { data: Buffer }).data.toString('utf8'), /^"long-x{400}"$/); + assert.match((full as { data: Buffer }).data.toString('utf8'), /^long-x{400}$/); assert.doesNotMatch(stub, /full result/, 'the stub says "its result", which the file holds'); } finally { await h.framework.stop(); } }); diff --git a/test/tool-result-history.test.ts b/test/tool-result-history.test.ts index 9fe34494a..8787af7f7 100644 --- a/test/tool-result-history.test.ts +++ b/test/tool-result-history.test.ts @@ -54,12 +54,30 @@ test('mixed known + unknown block shape falls back to JSON (all-or-nothing)', () test('non-array data: JSON stringify', () => { assert.equal(toolResultDataToHistoryString({ ok: true }), '{"ok":true}'); - assert.equal(toolResultDataToHistoryString('hello'), '"hello"'); assert.equal(toolResultDataToHistoryString(42), '42'); + assert.equal(toolResultDataToHistoryString(true), 'true'); // JSON leaves undefined out (JSON.stringify gives undefined): no data is no text. assert.equal(toolResultDataToHistoryString(undefined), ''); }); +test('string data: kept as it is, not JSON-quoted', () => { + // A module tool that answers in prose or a line-per-item listing returns + // a string. JSON would quote it and turn every newline into `\n`, so the + // model (and the spill file, and a script) got one escaped line. + const listing = 'As of 10:00. 3 open.\n[review] a/b#1 "Fix it"\n[merge] a/b#2 tab\there'; + assert.equal(toolResultDataToHistoryString(listing), listing); + assert.equal(toolResultDataToHistoryString('hello'), 'hello'); + assert.equal(toolResultDataToHistoryString(''), ''); + // A string that happens to hold JSON is the tool's text too: not re-encoded. + assert.equal(toolResultDataToHistoryString('{"ok":true}'), '{"ok":true}'); +}); + +test('string data: maxChars truncates the text itself with a marker', () => { + const out = toolResultDataToHistoryString('line\n'.repeat(100), 50); + assert.ok(out.startsWith('line\nline\n'), 'truncation keeps real newlines'); + assert.match(out, /\[truncated — original was 500 chars\]/); +}); + test('maxChars truncates the resulting string with a marker', () => { const data = [{ type: 'text', text: 'a'.repeat(1000) }]; const out = toolResultDataToHistoryString(data, 100); diff --git a/test/tool-result-spill.test.ts b/test/tool-result-spill.test.ts index 87444c5cd..79f2f9536 100644 --- a/test/tool-result-spill.test.ts +++ b/test/tool-result-spill.test.ts @@ -426,6 +426,64 @@ describe('tool-result spill completion (issue #89)', () => { } }); + it('spills a plain string result as its text: lines intact, greppable, not one JSON-quoted line', async () => { + // A module tool that answers with a line-per-item listing returns a + // string. It used to be JSON-quoted on the way to history, so its spill + // file held one escaped line that could only be paged by character. + const body = Array.from({ length: 3000 }, (_, i) => `[review] org/repo#${i} "title ${i}"\tsize ${'t'.repeat(10)}`).join('\n'); + const h = await startSpillTurn({ + prefix: 'spill-string-', + result: { success: true, data: body }, + withWorkspace: true, + }); + try { + const stored = await waitForStoredToolResult(h.framework); + assert.ok(stored, 'tool result should be stored'); + assert.ok(stored.content.startsWith('[review] org/repo#0 "title 0"\t'), 'inline head is the text itself, unquoted'); + + const refMatch = stored.content.match(/workspace file (files\/tool-results\/\S+\.txt)/); + assert.ok(refMatch, 'reference should name the spill file'); + const file = await h.workspace!.readBinary(refMatch[1]); + assert.ok('data' in file, 'spill file should be readable'); + const saved = (file as { data: Buffer }).data.toString('utf8'); + assert.strictEqual(saved, body, 'the spill file holds the string verbatim'); + assert.strictEqual(saved.split('\n').length, 3000, 'one line per item'); + + // grep finds one item, on its own line. + const grep = await h.workspace!.handleToolCall({ + id: 'grep', name: 'grep', input: { pattern: 'repo#2999 ', path: refMatch[1] }, + }); + assert.strictEqual(grep.success, true, grep.error); + const matches = (grep.data as { results: Array<{ matches: Array<{ line: number; text: string }> }> }) + .results.flatMap((r) => r.matches); + assert.deepStrictEqual(matches.map((m) => [m.line, m.text]), [[3000, body.split('\n')[2999]]]); + } finally { + await h.framework.stop(); + rmSync(h.tempDir, { recursive: true, force: true }); + } + }); + + it('a plain string result reaches history and the live wire as its text, byte-identical', async () => { + const body = 'As of 10:00. 2 open.\n[review] org/repo#1 "Fix it"\n[merge] org/repo#2 done'; + const h = await startSpillTurn({ + prefix: 'string-wire-', + result: { success: true, data: body }, + withWorkspace: false, + }); + try { + const stored = await waitForStoredToolResult(h.framework); + assert.ok(stored, 'tool result should be stored'); + assert.strictEqual(stored.content, body, 'history holds the string, not its JSON encoding'); + const wire = h.membrane.lastStream?.receivedToolResults[0] as + Array<{ content: unknown }> | undefined; + assert.ok(wire && wire.length === 1, 'one wire tool result expected'); + assert.strictEqual(wire[0].content, body, 'the wire carries the same text'); + } finally { + await h.framework.stop(); + rmSync(h.tempDir, { recursive: true, force: true }); + } + }); + it('falls back to EXPLICIT plain truncation with no writable workspace', async () => { const h = await startSpillTurn({ prefix: 'spill-nows-', diff --git a/test/unconvertible-tool-result.test.ts b/test/unconvertible-tool-result.test.ts index 5b0497fa5..2d056db74 100644 --- a/test/unconvertible-tool-result.test.ts +++ b/test/unconvertible-tool-result.test.ts @@ -378,6 +378,6 @@ test("the direct Agent API reads each field of its caller's result once", async ]; const built = (agent as unknown as { buildToolResultMessages(r: unknown[]): Array<{ content: Array<{ content: string }> }> }) .buildToolResultMessages(results); - assert.deepEqual(built[0].content.map((block) => block.content), [JSON.stringify('the payload'), 'what failed']); + assert.deepEqual(built[0].content.map((block) => block.content), ['the payload', 'what failed']); }); From 56783fb9022909f7b852ede213a718e7472847c5 Mon Sep 17 00:00:00 2001 From: Hilda-2681 Date: Fri, 16 Oct 2026 14:24:59 -0400 Subject: [PATCH 2/7] test(code-execution): an MCP tool's text-only result reaches the model and a script as that text The MCPL tool-result mapping (src/framework.ts, the "Convert MCP tool result to framework ToolResult" block) hands a result whose blocks are all text on as their joined text, so an MCP tool's text answer is string data too, and the serializer's string rule reaches it as it reaches a module tool's string. This pins that case end to end: a real stdio MCPL server whose tool answers with one text block, called once by the model and once from a code_execution script (real python3). The model's tool_result holds the text as the server sent it, and the script gets the same text: it splits into its two lines, and it doesn't start with a quote. The server accepts the framework's initial featureSets/update, as the fixtures' servers do, so the test doesn't wait out the policy request's 15 s timeout. On main's src it fails at the model's assertion: the model gets the text as a JSON string literal. With that assertion left out, it fails at the script's, since the script gets the same literal, one line, and has no second line to print. It passes with this branch's string rule. Co-Authored-By: Claude Opus 5.5 --- test/code-execution.test.ts | 75 +++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/test/code-execution.test.ts b/test/code-execution.test.ts index 075433758..8a9488386 100644 --- a/test/code-execution.test.ts +++ b/test/code-execution.test.ts @@ -575,6 +575,81 @@ describe('framework code_execution integration (real python3)', () => { } }); + it("an MCP tool's text-only result reaches the model and a script as that text, not its JSON encoding", async () => { + // The MCPL tool-result mapping hands a result whose blocks are all text + // on as their joined text, so an MCP tool's text answer is string data + // too: the model and a script get the text as the server sent it. + const listing = 'As of 10:00.\n[review] org/repo#1 "Fix it"'; + // A stdio MCPL server whose one tool answers with a single text block + // holding the `text` it was given. It accepts the framework's initial + // featureSets/update, as the fixtures' servers do, so the connection + // doesn't wait out the policy request's timeout. + const server = ` + let buf = ''; + process.stdin.on('data', (c) => { + buf += c.toString('utf8'); + let i; + while ((i = buf.indexOf('\\n')) >= 0) { + const line = buf.slice(0, i); buf = buf.slice(i + 1); + if (!line.trim()) continue; + let m; try { m = JSON.parse(line); } catch { continue; } + const reply = (result) => process.stdout.write(JSON.stringify({ jsonrpc: '2.0', id: m.id, result }) + '\\n'); + if (m.method === 'initialize') reply({ capabilities: { experimental: { mcpl: { version: '0.4' } } } }); + else if (m.method === 'tools/list') reply({ tools: [{ name: 'echo', description: 'e', inputSchema: { type: 'object' } }] }); + else if (m.method === 'tools/call') reply({ content: [{ type: 'text', text: String(m.params.arguments.text) }] }); + else if (m.method === 'featureSets/update') { if (m.id !== undefined) reply({ accepted: true }); } + } + }); + `; + const { tempDir, storePath } = tempStorePath('pytc-mcp-string-'); + const membrane = new MockMembrane(); + const code = [ + `r = await mcpl__echo__echo({"text": ${JSON.stringify(listing)}})`, + 'lines = r.split("\\n")', + 'print("count:", len(lines))', + 'print("second:", lines[1])', + 'print("quoted:", r.startswith(\'"\'))', + ].join('\n'); + membrane.pushResponse(createMockResponse([ + { type: 'tool_use', id: 'call_mcp_direct', name: 'mcpl--echo--echo', input: { text: listing } }, + { type: 'tool_use', id: 'call_mcp_script', name: 'code_execution', input: { code } }, + ], 'tool_use')); + membrane.pushResponse(createMockResponse([{ type: 'text', text: 'Done.' }])); + + const framework = await AgentFramework.create({ + storePath, + membrane: membrane.asMembrane(), + agents: [], + modules: [], + mcplServers: [{ id: 'echo', command: process.execPath, args: ['-e', server] }], + syncIntervalMs: 0, + codeExecution: { enabled: true }, + }); + try { + const created = await framework.createEphemeralAgent({ + name: 'worker', model: 'test-model', systemPrompt: 'Do the task.', allowedTools: 'all', + }); + created.contextManager.addMessage('user', [{ type: 'text', text: 'Go.' }]); + const promise = framework.runEphemeralToCompletion(created.agent, created.contextManager); + framework.start(); + await promise; + + const wire = membrane.lastStream!.receivedToolResults[0] as Array<{ toolUseId: string; content: unknown }>; + const direct = wire.find((r) => r.toolUseId === 'call_mcp_direct'); + assert.strictEqual(direct?.content, listing, 'the model gets the text, not its JSON encoding'); + const script = wire.find((r) => r.toolUseId === 'call_mcp_script') as { content: string } | undefined; + assert.ok(script, 'the script call has a result'); + const out = JSON.parse(script.content) as { stdout: string; return_code: number }; + assert.strictEqual(out.return_code, 0, script.content); + assert.match(out.stdout, /count: 2\n/); + assert.match(out.stdout, /second: \[review\] org\/repo#1 "Fix it"\n/); + assert.match(out.stdout, /quoted: False/); + } finally { + await framework.stop(); + rmSync(tempDir, { recursive: true, force: true }); + } + }); + it('two code_execution calls in one response: one runs, the other is refused, the turn completes', async () => { const { tempDir, storePath } = tempStorePath('pytc-two-'); const membrane = new MockMembrane(); From af11da09a107ab92c15c9bd4c3205435cfce49bf Mon Sep 17 00:00:00 2001 From: Hilda-2681 Date: Sat, 17 Oct 2026 02:27:45 -0400 Subject: [PATCH 3/7] changelog: file the change to scripts' tool results as breaking A code_execution script's tool function returns the same text the model reads. With this change, a script that ran json.loads on a string result to unquote it now raises, and one that parsed a tool's JSON text with two json.loads calls now raises at the second, since the first already returns the data. CONTRIBUTING's "Breaking changes" asks that such a change be filed as breaking, so that npm version opens a new minor for it and a dependent's caret range doesn't take it at its next fresh install. - verbatim-string-tool-results-scripts.breaking.md: an audience-scoped entry for script authors, with who acts, the migration and what's unchanged. - verbatim-string-tool-results.changed.md: its two sentences about scripts move to that entry. The model's and the spill files' change stays a change. Its `data: ''` sentence now says it holds for a result without an error, since nonErrorResultText falls back to a result's error only when `data` is undefined or null. Co-Authored-By: Claude Opus 5.5 --- .../verbatim-string-tool-results-scripts.breaking.md | 8 ++++++++ changelog.d/verbatim-string-tool-results.changed.md | 6 ++---- 2 files changed, 10 insertions(+), 4 deletions(-) create mode 100644 changelog.d/verbatim-string-tool-results-scripts.breaking.md diff --git a/changelog.d/verbatim-string-tool-results-scripts.breaking.md b/changelog.d/verbatim-string-tool-results-scripts.breaking.md new file mode 100644 index 000000000..006ef8e12 --- /dev/null +++ b/changelog.d/verbatim-string-tool-results-scripts.breaking.md @@ -0,0 +1,8 @@ +- **Script authors (`code_execution`):** a script's tool function now + returns a tool's text result as that text, where it returned the text + JSON-encoded. That covers every MCP tool whose result is text only, and + every module tool whose `data` is a string. Drop a `json.loads` that only + unquoted such a result. Where the text is itself JSON, one `json.loads` now + returns the data, so drop the second. Unchanged: an object, a number, a + boolean, or an array other than an MCP content array still arrives as its + JSON. diff --git a/changelog.d/verbatim-string-tool-results.changed.md b/changelog.d/verbatim-string-tool-results.changed.md index fc9964b99..1a416b4b3 100644 --- a/changelog.d/verbatim-string-tool-results.changed.md +++ b/changelog.d/verbatim-string-tool-results.changed.md @@ -8,7 +8,5 @@ booleans and arrays other than MCP content arrays are still sent as JSON, and MCP content arrays as their blocks. - A `code_execution` script's tool function returns the same text, so it now - gets a string result as it is. A script that ran `json.loads` on such a - result to unquote it now gets an error unless the string itself is JSON. - `data: ''` is stored as no text, as `data: undefined` already was. + For a result without an error, `data: ''` is stored as no text, as + `data: undefined` already was. From a191e7cfeacc518d259a4d89c4a90d5ad222cc7a Mon Sep 17 00:00:00 2001 From: Cuthbert-2453 Date: Thu, 22 Oct 2026 13:36:27 -0400 Subject: [PATCH 4/7] test: an empty string result is stored and sent as a result with no data is JSON quoted an empty string as '""'. Kept as it is, it gives the text a result with no data already gave, and the test pins that the stored copy and the live wire are the same for both. ToolCall.resultMaxChars's doc measures a success's data as its stored text, which for a string is now the string itself. Co-Authored-By: Claude Opus 5.5 --- src/types/events.ts | 2 +- test/tool-result-spill.test.ts | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/types/events.ts b/src/types/events.ts index 1a513f853..4b93ffd14 100644 --- a/src/types/events.ts +++ b/src/types/events.ts @@ -273,7 +273,7 @@ export interface ToolCall { /** * How long this call's result may be and still reach its caller whole, * measured as the framework stores it: an error's text, or a success's data - * as JSON (a content array as its text). For a model's call it is the + * as JSON (a string or a content array as its text). For a model's call it is the * agent's inline cap, past which the framework spills the result to a * workspace file and sends a preview. A result that long is stored whole * unless its round's results together pass the readers' per-message bound, diff --git a/test/tool-result-spill.test.ts b/test/tool-result-spill.test.ts index 90f24bc1c..d9122cedc 100644 --- a/test/tool-result-spill.test.ts +++ b/test/tool-result-spill.test.ts @@ -485,6 +485,27 @@ describe('tool-result spill completion (issue #89)', () => { } }); + it('an empty string result is stored and sent as a result with no data is: as no text', async () => { + // JSON quoted an empty string as '""'. Kept as it is, it is the text a + // result with no data already gave, so nothing downstream meets a new shape. + const seen: Array<{ stored: string; wire: unknown }> = []; + for (const result of [{ success: true, data: '' }, { success: true }] as ToolResult[]) { + const h = await startSpillTurn({ prefix: 'empty-string-', result, withWorkspace: false }); + try { + const stored = await waitForStoredToolResult(h.framework); + assert.ok(stored, 'tool result should be stored'); + const wire = h.membrane.lastStream?.receivedToolResults[0] as Array<{ content: unknown }> | undefined; + assert.ok(wire && wire.length === 1, 'one wire tool result expected'); + seen.push({ stored: stored.content, wire: wire[0].content }); + } finally { + await h.framework.stop(); + rmSync(h.tempDir, { recursive: true, force: true }); + } + } + assert.deepStrictEqual(seen[0], seen[1], 'an empty string goes as no data does'); + assert.deepStrictEqual(seen[0], { stored: '', wire: '' }); + }); + it('falls back to EXPLICIT plain truncation with no writable workspace', async () => { const h = await startSpillTurn({ prefix: 'spill-nows-', From dcecd76be4cd3dfc27b04ab29eb9fbb948788d16 Mon Sep 17 00:00:00 2001 From: Cuthbert-2453 Date: Thu, 22 Oct 2026 13:36:27 -0400 Subject: [PATCH 5/7] fix(tool-results): keep a string result well formed; say what reaches the model A string result now goes to the model as it is, where JSON used to escape a lone UTF-16 surrogate as six characters of text. Kept raw, one would be handed to membrane, and by our reading of membrane reach the provider's request, which can be refused for it, and it would reach the spill file, which UTF-8 can't hold it in. So the string is made well formed first: a lone surrogate reads as U+FFFD, and a pair is kept. A new test pins both. ToolResult.data's doc said the model reads anything other than a string or an MCP content array as JSON. It now says what toolResultDataToHistoryString does: a content array only when it knows every block, RFC-005's stub for a reference and its notice for inline data claiming a uri or a disposition, and the error text when there's no data. The changelog fragments promised more than the code does. The changed fragment now says data that isn't a string reaches the model as before, that a string is measured as its own text, so one whose JSON form passed the inline cap and whose text doesn't now stays inline, and that a lone surrogate reads as U+FFFD. The scripts fragment scopes the text to results without structured content, which reach a script as before: a result with structured content gets no text block when its data is a string. The reference-uri tests' header names the string case. Co-Authored-By: Claude Opus 5.5 --- ...verbatim-string-tool-results-scripts.breaking.md | 11 +++++------ changelog.d/verbatim-string-tool-results.changed.md | 13 ++++++++----- src/tool-result-history.ts | 7 +++++-- src/types/events.ts | 8 ++++++-- test/tool-result-history.test.ts | 9 +++++++++ test/tool-result-reference-uri.test.ts | 7 ++++--- 6 files changed, 37 insertions(+), 18 deletions(-) diff --git a/changelog.d/verbatim-string-tool-results-scripts.breaking.md b/changelog.d/verbatim-string-tool-results-scripts.breaking.md index 831716990..9f00afaaf 100644 --- a/changelog.d/verbatim-string-tool-results-scripts.breaking.md +++ b/changelog.d/verbatim-string-tool-results-scripts.breaking.md @@ -1,9 +1,8 @@ - **Script authors (`code_execution`):** a tool's text result now reaches a script as that text, in its result's `content[0].text`, where it was the text JSON-encoded. That covers every MCP tool whose result is text only, - and every module tool whose `data` is a string. Drop a `JSON.parse` that - only unquoted such a result. Where the text is itself JSON, one - `JSON.parse` now returns the data, so drop the second. Unchanged: an - object, a number, a boolean, or an array other than an MCP content array - still arrives as its JSON, and a result with structured content keeps its - blocks as the tool sent them. + and every module tool whose `data` is a string, without structured + content. Drop a `JSON.parse` that only unquoted such a result. Where the + text is itself JSON, one `JSON.parse` now returns the data, so drop the + second. A result whose data isn't a string, and a result with structured + content, reach a script as before. diff --git a/changelog.d/verbatim-string-tool-results.changed.md b/changelog.d/verbatim-string-tool-results.changed.md index 1a416b4b3..4c642d53c 100644 --- a/changelog.d/verbatim-string-tool-results.changed.md +++ b/changelog.d/verbatim-string-tool-results.changed.md @@ -4,9 +4,12 @@ listing, arrived as one escaped line: in context, on the live wire, and in its spill file under `tool-results/`, which `workspace--read` could then page only by character and `workspace--grep` matched as a single line. A - spill file now holds the string as the tool returned it. Objects, numbers, - booleans and arrays other than MCP content arrays are still sent as JSON, - and MCP content arrays as their blocks. + spill file now holds the string as the tool returned it. Data that isn't a + string reaches the model as before. - For a result without an error, `data: ''` is stored as no text, as - `data: undefined` already was. + A string is measured as its own text, so one whose JSON form passed the + inline cap but whose text doesn't, such as a long listing of short lines, + now stays inline where it used to spill to a file. A lone UTF-16 surrogate + in the string, which JSON escaped, reads as U+FFFD. For a result without + an error, `data: ''` is stored as no text, as `data: undefined` already + was. diff --git a/src/tool-result-history.ts b/src/tool-result-history.ts index b314a8ec8..36e6aa4e4 100644 --- a/src/tool-result-history.ts +++ b/src/tool-result-history.ts @@ -75,13 +75,16 @@ export function toolResultDataToHistoryString( const fromArray = tryHistoryStringFromContentArray(data, options); // A string is the text the tool means to say, kept as it is: JSON would // quote it and escape every newline, so a multi-line result would reach - // the model, a script and its spill file as one escaped line. + // the model, a script and its spill file as one escaped line. Kept well + // formed: a lone UTF-16 surrogate, which JSON escaped, isn't text, a + // provider can refuse a request that carries one, and UTF-8 can't store + // it in a spill file, so it reads as U+FFFD. // JSON leaves out undefined, a function and a symbol: a result with no data // is stored as no text, as an empty content array is. // RFC-005 shows only objects otherwise (shownInHistory), so a string has // nothing for historyJson to stub or withhold. const str = fromArray - ?? (typeof data === 'string' ? data : historyJson(data)) + ?? (typeof data === 'string' ? data.toWellFormed() : historyJson(data)) ?? ''; return maxChars ? truncateForHistory(str, maxChars) : str; } diff --git a/src/types/events.ts b/src/types/events.ts index 4b93ffd14..3224601de 100644 --- a/src/types/events.ts +++ b/src/types/events.ts @@ -187,8 +187,12 @@ export interface ToolResult { /** Whether the tool succeeded */ success: boolean; /** - * Result data (if success). The model reads a string as that text, an MCP - * content array as its blocks, and anything else as JSON. + * Result data (if success). The model reads a string as that text, and + * other data as toolResultDataToHistoryString renders it: an MCP content + * array whose every block it knows as those blocks, and anything else as + * JSON, where RFC-005 shows a reference as its stub and withholds inline + * data that claims a uri or a disposition. With no data, it reads the + * result's error text, if any. */ data?: unknown; /** Error message (if failure) */ diff --git a/test/tool-result-history.test.ts b/test/tool-result-history.test.ts index ed0ef00d8..149bae2c5 100644 --- a/test/tool-result-history.test.ts +++ b/test/tool-result-history.test.ts @@ -85,6 +85,15 @@ test('string data: kept as it is, not JSON-quoted', () => { assert.equal(toolResultDataToHistoryString('{"ok":true}'), '{"ok":true}'); }); +test('string data: a lone surrogate reads as U+FFFD, and a pair is kept', () => { + // JSON escaped a lone surrogate as six characters of text. Kept as it is, + // it would reach the provider's request, which can be refused for it, and + // a spill file, which UTF-8 can't hold it in. + assert.equal(toolResultDataToHistoryString('cut \ud83d here'), 'cut \ufffd here'); + assert.equal(toolResultDataToHistoryString('end \udc00'), 'end \ufffd'); + assert.equal(toolResultDataToHistoryString('a pair \ud83d\ude00 stays'), 'a pair \ud83d\ude00 stays'); +}); + test('string data: maxChars truncates the text itself with a marker', () => { const out = toolResultDataToHistoryString('line\n'.repeat(100), 50); assert.ok(out.startsWith('line\nline\n'), 'truncation keeps real newlines'); diff --git a/test/tool-result-reference-uri.test.ts b/test/tool-result-reference-uri.test.ts index 6865ed422..66b1ff182 100644 --- a/test/tool-result-reference-uri.test.ts +++ b/test/tool-result-reference-uri.test.ts @@ -1,9 +1,10 @@ /** * A tool result's data never reaches the agent, or its history, with an * RFC-005 reference's uri, wherever in the data it sits: data that isn't a - * content array of blocks the serializer recognizes is written as JSON with - * each reference as its stub and inline data claiming a uri or a disposition - * withheld. Everything else in it is written as JSON writes it, as before. + * string or a content array of blocks the serializer recognizes is written + * as JSON with each reference as its stub and inline data claiming a uri or + * a disposition withheld. Everything else in it is written as JSON writes + * it, as before. */ import { describe, it } from 'node:test'; import assert from 'node:assert/strict'; From b3c0ab73b8e24716ca92f57f2d30bbfec400454a Mon Sep 17 00:00:00 2001 From: Cuthbert-2453 Date: Thu, 22 Oct 2026 13:36:28 -0400 Subject: [PATCH 6/7] test: a stored quoted result goes as stored; a string spills by its own length Two claims the change makes now have tests. A string result stored in its old JSON-quoted form, seeded into the store directly, is sent byte for byte after a restart, so upgrading doesn't move a cached prompt's prefix. That is history's replay, which this change must keep, so the test passes on the base too. And a string is measured as its own text, so 1,000 short lines, 4,002 characters as JSON and past a cap of 3,800, are 3,000 as text and stored whole. ToolResult.data's doc no longer names toolResultDataToHistoryString, which the package doesn't export. Co-Authored-By: Claude Opus 5.5 --- src/types/events.ts | 11 ++++---- test/tool-result-cap-fit.test.ts | 10 +++++++ test/tool-result-spill.test.ts | 47 ++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 6 deletions(-) diff --git a/src/types/events.ts b/src/types/events.ts index 3224601de..b1b9f4421 100644 --- a/src/types/events.ts +++ b/src/types/events.ts @@ -187,12 +187,11 @@ export interface ToolResult { /** Whether the tool succeeded */ success: boolean; /** - * Result data (if success). The model reads a string as that text, and - * other data as toolResultDataToHistoryString renders it: an MCP content - * array whose every block it knows as those blocks, and anything else as - * JSON, where RFC-005 shows a reference as its stub and withholds inline - * data that claims a uri or a disposition. With no data, it reads the - * result's error text, if any. + * Result data (if success). The model reads a string as that text, an MCP + * content array whose every block the framework knows as those blocks, and + * anything else as JSON, where RFC-005 shows a reference as its stub and + * withholds inline data that claims a uri or a disposition. With no data, + * it reads the result's error text, if any. */ data?: unknown; /** Error message (if failure) */ diff --git a/test/tool-result-cap-fit.test.ts b/test/tool-result-cap-fit.test.ts index d764b2a98..081edf6f4 100644 --- a/test/tool-result-cap-fit.test.ts +++ b/test/tool-result-cap-fit.test.ts @@ -291,6 +291,16 @@ test("a cap the bound's fit already holds the head under gets no offer to raise }); }); +test('a string is measured as its own text, so one whose JSON passes the cap and whose text fits is stored whole', async () => { + // As JSON, 1,000 lines of "ab" are 4,002 characters, past the cap of 3,800: + // each newline escapes to two. As text they are 3,000, so the result stays + // inline where its JSON form spilled. + const payload = 'ab\n'.repeat(1000); + await withResidents({ agents: [{ name: 'solo', maxMessageTokens: 1000 }], mount: 'files', inlineMaxChars: 3800, payload }, async ({ stored }) => { + assert.equal(stored, payload); + }); +}); + test("a result between the cap and the bound is offered a higher cap, which would store it whole", async () => { // The 3,900 characters are stored as they are: past the cap of 3,800, and // within the bound of 4,000. diff --git a/test/tool-result-spill.test.ts b/test/tool-result-spill.test.ts index d9122cedc..d6de1459f 100644 --- a/test/tool-result-spill.test.ts +++ b/test/tool-result-spill.test.ts @@ -485,6 +485,53 @@ describe('tool-result spill completion (issue #89)', () => { } }); + it('a result stored JSON-quoted, as strings were before they went as their text, is sent as stored after a restart', async () => { + // History keeps each result's text as it was stored, so a string result + // stored in its old quoted form reaches the next request byte for byte, + // and a cached prompt's prefix doesn't move. This doesn't depend on how a + // string is rendered now: the store is seeded with the old form directly. + const quoted = JSON.stringify('As of 10:00. 2 open.\n[review] org/repo#1 "Fix it"'); + const { tempDir, storePath } = tempStorePath('string-stored-'); + try { + const seed = await AgentFramework.create({ + storePath, + membrane: new MockMembrane().asMembrane(), + agents: [{ name: 'prime', model: 'test-model', systemPrompt: 'You are prime.', allowedTools: 'all' }], + modules: [], + syncIntervalMs: 0, + }); + try { + const cm = seed.getAgent('prime')!.getContextManager(); + cm.addMessage('prime', [{ type: 'tool_use', id: 'call_old', name: 'canned--fetch', input: {} }]); + cm.addMessage('user', [{ type: 'tool_result', toolUseId: 'call_old', content: quoted }]); + } finally { + await seed.stop(); + } + const h = await startSpillTurn({ + prefix: 'unused-', + result: { success: true, data: 'new' }, + withWorkspace: false, + tempDir, + storePath, + }); + try { + const deadline = Date.now() + 20_000; + while (h.membrane.calls.length === 0 && Date.now() < deadline) await new Promise((r) => setTimeout(r, 50)); + const request = h.membrane.calls[0]; + assert.ok(request, 'the boot makes a request'); + const sent = request.messages + .flatMap((m) => (Array.isArray(m.content) ? m.content : []) as Array<{ type: string; toolUseId?: string; content?: unknown }>) + .filter((b) => b.type === 'tool_result' && b.toolUseId === 'call_old') + .map((b) => b.content); + assert.deepStrictEqual(sent, [quoted], 'the stored result goes as it was stored'); + } finally { + await h.framework.stop(); + } + } finally { + rmSync(tempDir, { recursive: true, force: true }); + } + }); + it('an empty string result is stored and sent as a result with no data is: as no text', async () => { // JSON quoted an empty string as '""'. Kept as it is, it is the text a // result with no data already gave, so nothing downstream meets a new shape. From 783a93255aa47edc597ec949456d438c53f6ab4b Mon Sep 17 00:00:00 2001 From: Cuthbert-2453 Date: Thu, 22 Oct 2026 13:38:44 -0400 Subject: [PATCH 7/7] docs: say what a resident and a script author meet The changed fragment says results stored before the upgrade keep their quoted text, so a conversation shows both forms until its older results leave its context, and that data '' reads as an empty result. Its sentence on the inline cap says "went over", which "passed" could be read against. The scripts fragment says a script saved before the upgrade meets the change too, and how a script that parsed every result can read any result now, including what a string whose text is JSON reads as. The cap-fit test's name says "goes over" too. Co-Authored-By: Claude Opus 5.5 --- ...erbatim-string-tool-results-scripts.breaking.md | 12 ++++++++---- .../verbatim-string-tool-results.changed.md | 14 ++++++++------ test/tool-result-cap-fit.test.ts | 2 +- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/changelog.d/verbatim-string-tool-results-scripts.breaking.md b/changelog.d/verbatim-string-tool-results-scripts.breaking.md index 9f00afaaf..1e55acab7 100644 --- a/changelog.d/verbatim-string-tool-results-scripts.breaking.md +++ b/changelog.d/verbatim-string-tool-results-scripts.breaking.md @@ -2,7 +2,11 @@ script as that text, in its result's `content[0].text`, where it was the text JSON-encoded. That covers every MCP tool whose result is text only, and every module tool whose `data` is a string, without structured - content. Drop a `JSON.parse` that only unquoted such a result. Where the - text is itself JSON, one `JSON.parse` now returns the data, so drop the - second. A result whose data isn't a string, and a result with structured - content, reach a script as before. + content. A script saved before the upgrade meets the change too. Drop a + `JSON.parse` that only unquoted such a result. Where the text is itself + JSON, one `JSON.parse` now returns the data, so drop the second. A script + that read every result with `JSON.parse` can parse inside a `try` and + keep the text when it throws; a string whose text is itself JSON, such as + the text `123` or `true`, then reads as the value it spells. A result + whose data isn't a string, and a result with structured content, reach a + script as before. diff --git a/changelog.d/verbatim-string-tool-results.changed.md b/changelog.d/verbatim-string-tool-results.changed.md index 4c642d53c..65354337f 100644 --- a/changelog.d/verbatim-string-tool-results.changed.md +++ b/changelog.d/verbatim-string-tool-results.changed.md @@ -7,9 +7,11 @@ spill file now holds the string as the tool returned it. Data that isn't a string reaches the model as before. - A string is measured as its own text, so one whose JSON form passed the - inline cap but whose text doesn't, such as a long listing of short lines, - now stays inline where it used to spill to a file. A lone UTF-16 surrogate - in the string, which JSON escaped, reads as U+FFFD. For a result without - an error, `data: ''` is stored as no text, as `data: undefined` already - was. + Results stored before the upgrade keep the quoted text they were stored + with, so a conversation shows both forms until its older results leave + its context. A string is measured as its own text, so one whose JSON form + went over the inline cap while its text doesn't, such as a long listing + of short lines, now stays inline where it used to spill to a file. A lone + UTF-16 surrogate in the string, which JSON escaped, reads as U+FFFD. For + a result without an error, `data: ''` reads as an empty result, as + `data: undefined` already did. diff --git a/test/tool-result-cap-fit.test.ts b/test/tool-result-cap-fit.test.ts index 081edf6f4..2cf8ecdcd 100644 --- a/test/tool-result-cap-fit.test.ts +++ b/test/tool-result-cap-fit.test.ts @@ -291,7 +291,7 @@ test("a cap the bound's fit already holds the head under gets no offer to raise }); }); -test('a string is measured as its own text, so one whose JSON passes the cap and whose text fits is stored whole', async () => { +test('a string is measured as its own text, so one whose JSON goes over the cap while its text fits is stored whole', async () => { // As JSON, 1,000 lines of "ab" are 4,002 characters, past the cap of 3,800: // each newline escapes to two. As text they are 3,000, so the result stays // inline where its JSON form spilled.