fix(tool-results): send a string result as its text, not JSON-quoted

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.

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.

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;
 }
 

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,

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);

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<ToolResult> {
+        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();

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');

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/);
   });
 });

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(); }
 });