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..1e55acab7 --- /dev/null +++ b/changelog.d/verbatim-string-tool-results-scripts.breaking.md @@ -0,0 +1,12 @@ +- **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, without structured + 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 new file mode 100644 index 000000000..65354337f --- /dev/null +++ b/changelog.d/verbatim-string-tool-results.changed.md @@ -0,0 +1,17 @@ +- 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. Data that isn't a + string reaches the model as before. + + 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/src/tool-result-history.ts b/src/tool-result-history.ts index 7406c66e2..36e6aa4e4 100644 --- a/src/tool-result-history.ts +++ b/src/tool-result-history.ts @@ -11,10 +11,11 @@ * * 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, written by RFC-005's rules at - * every depth (historyJson): a reference is shown as its stub wherever it - * sits in the data, so its uri never reaches history, and inline data that - * claims a uri or a disposition is withheld. + * A string is already text and is kept as it is. For anything else it falls + * back to JSON, written by RFC-005's rules at every depth (historyJson): a + * reference is shown as its stub wherever it sits in the data, so its uri + * never reaches history, and inline data that claims a uri or a disposition + * is withheld. */ import { INLINE_WITHHELD_TEXT, isInlineContradiction, referenceStubOrNull } from './mcpl/references.js'; @@ -72,9 +73,19 @@ 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. 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. - const str = fromArray ?? historyJson(data) ?? ''; + // 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.toWellFormed() : historyJson(data)) + ?? ''; return maxChars ? truncateForHistory(str, maxChars) : str; } diff --git a/src/types/events.ts b/src/types/events.ts index e3a14afbc..b1b9f4421 100644 --- a/src/types/events.ts +++ b/src/types/events.ts @@ -186,7 +186,13 @@ 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 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) */ error?: string; @@ -270,7 +276,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/code-execution-structured-results.test.ts b/test/code-execution-structured-results.test.ts index 2e9292eeb..9762baff8 100644 --- a/test/code-execution-structured-results.test.ts +++ b/test/code-execution-structured-results.test.ts @@ -116,9 +116,9 @@ test("a script reads a structured result whole, the model's calls and history ke 'const only = await tools["fixture--inspect"]({ variant: "only" });', 'check(same(only, { content: [], structuredContent: ENVELOPE, isError: false }), "only", only);', // Without structured content, a result is one text block of the - // model's text, which JSON-encodes a text result, and no structured view. + // model's text, which for a text result is that text, and no structured view. 'const plain = await tools["fixture--inspect"]({ variant: "plain" });', - 'check(same(plain, { content: [{ type: "text", text: JSON.stringify("plain text") }], structuredContent: null, isError: false }), "plain", plain);', + 'check(same(plain, { content: [{ type: "text", text: "plain text" }], structuredContent: null, isError: false }), "plain", plain);', 'const absent = await tools["fixture--inspect"]({ variant: "absent" });', 'check(same(absent, { content: [{ type: "text", text: "" }], structuredContent: null, isError: false }), "absent", absent);', 'const error = await tools["fixture--inspect"]({ variant: "error" });', @@ -160,8 +160,8 @@ test("a script reads a structured result whole, the model's calls and history ke const resultOf = (id: string) => results.find((r) => r.toolUseId === id)!.content; // The model reads the content: the text when there is some, and the // structured JSON only when the content has none. - assert.equal(resultOf('model-direct'), JSON.stringify(transcript)); - assert.match(resultOf('model-only'), /\\"runId\\":\\"r41\\"/); + assert.equal(resultOf('model-direct'), transcript); + assert.match(resultOf('model-only'), /"runId":"r41"/); const script = resultOf('model-script'); assert.match(script, /^Script s\d+-[0-9a-z]{4} finished\./, script); assert.ok(script.endsWith('Its output:\nr41 running café 🌱\n'), script); diff --git a/test/code-execution.test.ts b/test/code-execution.test.ts index 56c611f1d..c4bcc75b7 100644 --- a/test/code-execution.test.ts +++ b/test/code-execution.test.ts @@ -290,6 +290,119 @@ describe('code_execution through the framework', () => { }); }); + 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 membrane = new MockMembrane(); + const code = [ + 'const r = await tools["test--listing"]({});', + 'const t = r.content[0].text;', + 'const lines = t.split("\\n");', + 'text("count: " + lines.length);', + 'text("second: " + lines[1]);', + 'text("quoted: " + t.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.' }])); + await withFramework('ce-string-', { membrane, module: new ListingModule() }, async (framework) => { + await runEphemeral(framework, 'worker'); + const [results] = membrane.lastStream!.receivedToolResults as WireResult[][]; + const wire = results![0]!; + assert.strictEqual(wire.toolUseId, 'call_ce_str'); + assert.match(wire.content, /count: 2/); + assert.match(wire.content, /second: \[review\] org\/repo#1 "Fix it"/); + assert.match(wire.content, /quoted: false/); + }); + }); + + 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({ protocolVersion: '2024-11-05', serverInfo: { name: 'echo', version: '1.0.0' }, capabilities: { tools: {}, 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('ce-mcp-string-'); + const membrane = new MockMembrane(); + const code = [ + `const r = await tools["mcpl--echo--echo"]({ text: ${JSON.stringify(listing)} });`, + 'const t = r.content[0].text;', + 'const lines = t.split("\\n");', + 'text("count: " + lines.length);', + 'text("second: " + lines[1]);', + 'text("quoted: " + t.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 { + await framework.start(); + await until(() => framework.getAllTools().some((t) => t.name === 'mcpl--echo--echo'), 'the echo server\'s tool'); + const created = await framework.createEphemeralAgent({ name: 'worker', model: 'test-model', systemPrompt: 'Do the task.', allowedTools: 'all' }); + created.contextManager.addMessage('user', [{ type: 'text', text: 'Go.' }]); + await framework.runEphemeralToCompletion(created.agent, created.contextManager); + const wire = membrane.lastStream!.receivedToolResults[0] as WireResult[]; + 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'); + assert.ok(script, 'the script call has a result'); + assert.match(script.content, /count: 2/); + assert.match(script.content, /second: \[review\] org\/repo#1 "Fix it"/); + assert.match(script.content, /quoted: false/); + } finally { + await framework.stop(); + rmSync(tempDir, { recursive: true, force: true }); + } + }); + it('two code_execution calls in one response both run, and the turn completes', async () => { const membrane = new MockMembrane(); const module = new ScriptToolModule(); diff --git a/test/mcp-modern-framework.test.ts b/test/mcp-modern-framework.test.ts index 3a6ea6cd5..c42997d8e 100644 --- a/test/mcp-modern-framework.test.ts +++ b/test/mcp-modern-framework.test.ts @@ -156,10 +156,10 @@ test('model path: the result reaches the model, structured-only as JSON', async membrane.pushResponse(createMockResponse([{ type: 'text', text: 'Done.' }])); framework.pushEvent({ type: 'external-message', source: 'test', content: 'go', metadata: {} } as never); await waitFor('the tool round', () => (membrane.lastStream?.receivedToolResults.length ?? 0) >= 1); - // The framework renders string data JSON-encoded, as it does for every - // string tool result; decoded, it is the structured value's JSON. + // The result's data is the structured value's JSON, a string, which + // reaches the model as that text. const [delivered] = membrane.lastStream!.receivedToolResults[0] as Array<{ content: string }>; - assert.deepEqual(JSON.parse(JSON.parse(delivered!.content)), { answer: 42, ok: false, nothing: null }); + assert.deepEqual(JSON.parse(delivered!.content), { answer: 42, ok: false, nothing: null }); }); }); @@ -224,11 +224,11 @@ test('scripts get every result in one shape, structured results whole', async () isError: false, }); // Without structured content, content is the model's text: the history - // rendering, which JSON-encodes a text result. structuredContent is null. + // rendering, which for a text result is that text. structuredContent is null. assert.deepEqual(await handle('agent', 'mcpl--modern--echo', { text: 'x' }), { ok: true, isError: false, - value: { content: [{ type: 'text', text: JSON.stringify('echo:x') }], structuredContent: null, isError: false }, + value: { content: [{ type: 'text', text: 'echo:x' }], structuredContent: null, isError: false }, }); }); }); @@ -404,7 +404,7 @@ process.stdin.on('data', (c) => { const [delivered] = membrane.lastStream!.receivedToolResults[0] as Array<{ content: string; isError: boolean }>; assert.equal(delivered!.isError, false); // No server text block: the structured value is shown beside the stubs. - assert.equal(JSON.parse(delivered!.content), + assert.equal(delivered!.content, '[resource link "a.md": https://example.invalid/a.md; not fetched]\n[resource memo://n, text/plain]\nnote body\n{"count":2}'); const status = framework.listMcplServers().find((s) => s.id === 'plain')!; assert.equal(status.family, 'legacy'); diff --git a/test/tool-result-cap-fit.test.ts b/test/tool-result-cap-fit.test.ts index fc859a918..2cf8ecdcd 100644 --- a/test/tool-result-cap-fit.test.ts +++ b/test/tool-result-cap-fit.test.ts @@ -187,7 +187,7 @@ test("a turn's results that fit one message's bound together are each stored as // first would spill, but together they fit, so neither is cut. const payloads = ['y'.repeat(3000), 'z'.repeat(500)]; await withResidents({ agents: [{ name: 'solo', maxMessageTokens: 1000 }], mount: 'files', payloads }, async ({ storedAll, shownAllTo }) => { - assert.deepEqual(storedAll, payloads.map((payload) => JSON.stringify(payload))); + assert.deepEqual(storedAll, payloads); assert.deepEqual(await shownAllTo('solo'), storedAll); }); }); @@ -291,11 +291,21 @@ 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 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. + 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 a JSON string, 3,902 with its quotes: - // past the cap of 3,800, and within the bound of 4,000. + // The 3,900 characters are stored as they are: past the cap of 3,800, and + // within the bound of 4,000. await withResidents({ agents: [{ name: 'solo', maxMessageTokens: 1000 }], mount: 'files', inlineMaxChars: 3800, payload: 'y'.repeat(3900) }, async ({ stored }) => { - assert.match(stored, /\[truncated — showing \d+ of 3902 chars; full content: workspace file files\/tool-results\/\S+\. /); + assert.match(stored, /\[truncated — showing \d+ of 3900 chars; full content: workspace file files\/tool-results\/\S+\. /); assert.match(stored, /raise the inline cap/); }); }); diff --git a/test/tool-result-guard.test.ts b/test/tool-result-guard.test.ts index 151439ef8..c9201aadf 100644 --- a/test/tool-result-guard.test.ts +++ b/test/tool-result-guard.test.ts @@ -1924,14 +1924,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 7117b820c..149bae2c5 100644 --- a/test/tool-result-history.test.ts +++ b/test/tool-result-history.test.ts @@ -67,12 +67,39 @@ test('a text block whose text is not a string shows the placeholder, so a refere 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: 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'); + 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-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'; diff --git a/test/tool-result-spill.test.ts b/test/tool-result-spill.test.ts index e935e4ba5..d6de1459f 100644 --- a/test/tool-result-spill.test.ts +++ b/test/tool-result-spill.test.ts @@ -427,6 +427,132 @@ 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('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. + 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-', diff --git a/test/unconvertible-tool-result.test.ts b/test/unconvertible-tool-result.test.ts index a3e785da2..965cf8a1e 100644 --- a/test/unconvertible-tool-result.test.ts +++ b/test/unconvertible-tool-result.test.ts @@ -424,6 +424,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']); });