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,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.

changelog.d/verbatim-string-tool-results.changed.md

@@ -0,0 +1,12 @@
+- 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.
+
+  For a result without an error, `data: ''` is stored as no text, as
+  `data: undefined` already was.

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

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

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;

test/code-execution.test.ts

@@ -521,6 +521,135 @@ 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<ToolResult> {
+        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("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();

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

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

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-',

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