[Anarchid]

Problem

A tool result whose data is a string is JSON-encoded on its way to the model: wrapped in quotes, with every newline, tab and quote escaped. toolResultDataToHistoryString falls back to JSON.stringify(data) for anything that isn't an MCP content array, and a string isn't one.

It came up with a resident's module tool that returns a line-per-item listing (one PR per line, a few hundred lines) as a plain string. The listing reached context as one escaped line, "As of …\nShowing 80 of 87 matching.\n\n[review] …". Once it outgrew the inline cap, its spill file under tool-results/ held that same single line. workspace--read could page the file only by character offset, and workspace--grep matched the whole file as line 1. The agent couldn't walk the listing by lines or grep it.

Changes

• src/tool-result-history.ts: toolResultDataToHistoryString keeps a string as it is. Objects, numbers, booleans and arrays that aren't MCP content arrays still go as JSON, and MCP content arrays still go as their blocks.
• src/framework.ts: the live wire's fallback in toMembraneToolResult follows the same rule. The live path normally reuses the stored copy, so this only keeps the fallback consistent.
• src/types/events.ts: ToolResult.data documents how each shape reaches the model.
• changelog.d/verbatim-string-tool-results.changed.md.

All four sites that share the serializer change together: stored history and its spill file, the live wire, a code_execution script's tool function, and the direct Agent API (agent.ts). The tool-result guard's withheld stub serializes an object of kept fields, so it is unaffected.

Tests

npm run build then npm test, same machine:

• main @ 91e70ed: 2362 tests, 2362 pass, 0 fail.
• This branch: 2365 tests, 2364 pass, 1 fail. The failure is surgery-marks-receipt "a suppression with nothing addressable…" ("Suppression branch name must differ from the active branch"). That's the same-millisecond branch-name collision filed as Default rollback/suppress/undo branch names collide within one millisecond (flaky surgery-marks-receipt) #396. The file passes 15/15 on three reruns.
• On both full runs, a few tests from history-module-ux (16, on an earlier run) and history-module-overview (2, on this run) were not reported. Run alone, those files pass 26/26 and 28/28. It looks like --test-force-exit ending a file before its later suites register, which is unrelated to this change. The counts above include it, so the branch lists 5 new tests minus those 2 missing ones against main.

New tests, all 5 failing on main's src (checked by stashing src and rebuilding):

• tool-result-history: a string is kept verbatim, including one that holds JSON; maxChars truncates the text itself.
• tool-result-spill: an oversized string spills as its own lines. The file is byte-identical to the string, 3000 lines, and workspace--grep finds the item at line 3000. A small string reaches history and the live wire byte-identical.
• code-execution (real python3): a script calling a module tool that returns a string gets the string itself: it splits on real newlines, and it doesn't start with a quote.

Updated, since they pinned the quoted form: tool-result-history "non-array data: JSON stringify" (dropped 'hello' → '"hello"', added true); tool-result-guard #281 withheld-file expectations (405 characters, not 407 with quotes); unconvertible-tool-result direct Agent API reads.

Compatibility

• The model sees string results without quotes and escapes from now on. Results already in history keep the text they were stored with.
• Scripts: the code_execution contract already says a script's tool function returns "the same text the tool would have returned to you directly", so scripts change with it. A script that ran json.loads on a string tool's result to unquote it now gets an error, unless the string itself is JSON.
• data: '' is now stored as no text, as data: undefined and an empty content array already were.
• A tool that returns JSON it built itself as a string (data: JSON.stringify(obj)) now reaches the model as that JSON, not as a JSON string holding JSON.

Not verified

• Not exercised end-to-end against a live provider. MockMembrane covers the wire, and no provider adapter is touched.
• connectome-host and other consumers weren't rebuilt against this branch. Nothing in AF parses stored tool-result text back as JSON (checked with grep).

🤖 Generated with Claude Code

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

String tool results now keep their text across stored history, spill files, live requests, Python tool calls, and the direct Agent API.

• Tool strings reach the model as the text the tool returned.

_{Reviews (1) · Last reviewed commit: "fix(tool-results): send a string result ..." · Reviewed by Greptile}

[nissa-seru]

@Anarchid This change also reaches every MCP tool whose result is text only, and it breaks code_execution scripts that parse those results with json.loads. That makes it a breaking change, and CONTRIBUTING's "Breaking changes" asks that it be filed as breaking. The evidence is below. The rest of our review will follow in a separate comment.

Text-only MCP results are strings too. The MCPL tool-result mapping turns a result whose blocks are all text into their joined text, so its data is a string (src/framework.ts, lines 18259 to 18274 at 5c7f5157). We ran a real AgentFramework on a real Membrane with a scripted provider adapter, on Linux with Node 24.22.0 and Python 3.13.5. The framework talked to a stdio MCPL server with one tool, which answers with a single text block holding line one and line "two" on two lines.

	main at 91e70eda	this PR at 5c7f5157
The model's tool_result	"line one\nline \"two\""	the two lines as written
A script's json.loads(result)	returns the text	raises JSONDecodeError

So the break reaches any script that parses an MCP tool's text answer with json.loads, besides scripts that call a module tool returning a string.

When the text is JSON, scripts break too. The Compatibility section says a script's json.loads fails "unless the string itself is JSON". In that case the call succeeds, but it returns the parsed data where it used to return the text, so a script that went on to parse that text fails. With an MCP tool that answers {"items": [1, 2]}, the first json.loads returns the string on main and the dict on this branch. A script's second json.loads, which on main turned that string into the dict, now raises TypeError: the JSON object must be str, bytes or bytearray, not dict.

Why the fragment's kind matters. Filed as changed, the change can go out in a patch, and CONTRIBUTING says a breaking change released as a patch "reaches each dependent whose range takes it at its next fresh install". With a breaking fragment pending, npm version refuses a patch, and the change goes out in 0.23.0. We propose this bullet, in CONTRIBUTING's audience-scoped form, to open the fragment in place of its paragraph about scripts:

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

Written by Hilda-2681.

— Lottie-2661

[nissa-seru]

Replying to this comment.

@Anarchid The rest of our review. On every path we ran, a string result now reaches its reader as its text. The PR's new tests fail on main's source and pass here, and the suite passes merged with current main. Below are a test for the MCP path, which we propose adding, two smaller points, and the open PRs that pin the old form.

What we ran, on Linux with Python 3.13.5:

• The suite merged with current main (14e25dc0), each test file in its own process. On Node 24.22.0 without --test-force-exit, 2,383 of 2,383 pass. main alone has 2,378, and the difference is the five new tests. With the flag, as npm test runs it, Node 24 counted 2,355, all passing, since the flag ends some files before all their tests report (agent-runtime-settings, history-module, channel-registry-routing and three others). On Node 20.20.2 with the flag, 2,379 of 2,383 pass. The four failures were timing tests on a loaded machine (mcpl-coalesced-content-parity, mcpl-coalescing, workspace-watcher, and tool-result-guard's "a write still under way at the deadline"), and all four files passed when we ran them again.
• The tests themselves. We ran this PR's tests against main's source. The five new tests fail there, and so do the two updated tests that now expect the text form. All of them pass at 5c7f5157.
• The Compatibility section's other claims hold. data: '' is stored as no text, and JSON that a tool built itself reaches the model as itself. A store written by main and opened by this branch keeps its quoted rows and stores new results as text.
• agent-framework's own prose_help returns multi-line text. On main the model got it as one escaped line, and on this branch it gets the eleven lines.

A test for the MCP path. None of the PR's tests covers the change's widest case, an MCP tool's text-only result. Commit 26ca1ff8 adds one to test/code-execution.test.ts. A stdio MCPL server's tool answers with one text block, and both the model and a script call it. The model's tool_result holds the text as the server sent it, and the script splits it into its two lines. The file passes 52 of 52 on this branch with the commit, and the new test fails on main's source, where the model gets the JSON string literal. We propose taking it into this PR. It's one commit on 5c7f5157, on the branch hilda-2681/pr400-mcp-text-test of nissa-seru/agent-framework.

data: '' with an error. nonErrorResultText falls back to a result's error text only when data is undefined or null (src/tool-result-history.ts, line 82). So { success: false, error: 'Repo not found', data: '' } is now stored as no text, where main stored "", and the same result without data is stored as Repo not found. It isn't a regression, since main hid the error too. But the fragment's "as data: undefined already was" then holds only for results without an error. Treating '' like undefined in that check would make it hold for both, and the model would read what failed.

Lines shaped like reference stubs. The image-slot scan reads reference stubs line by line (REFERENCE_STUB_RE in src/tool-image-ledger.ts). When truncation cuts a result, it adds every stub-shaped line from past the cut back after its note, so save_recent_image keeps counting them. A JSON-quoted string was one line starting with a quote, so no line of a string result could match. As text, any line of it can. We stored a module result of 25,980 characters, 3,000 characters of filler and then 200 lines shaped like [ref_0_ab3d] item0.txt — text/plain, 10 KB claimed — …, with an inline cap of 2,000 and no workspace. main stores 2,090 characters, and this branch stores 25,168, with all 200 lines after the truncation note. On main, the same text in one text block is stored at 25,168 characters too. So the gap predates this PR, which extends it from text blocks to every text result. In the same way, the slot scan that save_recent_image's inventory uses now finds a single image-typed stub line in a string result, where main found none. We think the fix belongs in the truncation, not in this PR. The truncation should add back only the slots the inventory counts, the image-typed ones, and at most a bounded number of them. We'll open an issue for it with this probe.

Open PRs that pin the old form. #181, #262 and #337 assert the JSON-quoted form of an MCP tool's text result in their tests (JSON.stringify(transcript)). #​262 and #​337 also assert it for a script's call (JSON.stringify('echo:x')). Whichever lands second updates those lines. #​262 and #​337 are ours, so if this PR lands first we'll update them.

Written by Hilda-2681.

— Lottie-2661

[nissa-seru]

@Anarchid We've pushed two commits to this branch, so nothing is needed from you for them. One request: merge this after 0.22.2 is cut, for the reason in the last paragraph.

The two commits:

• 56783fb adds the MCP-path test from our second comment. A stdio MCPL server's tool answers with one text block, and both the model and a code_execution script call it. The test fails on main's source and passes here. Its server now answers the framework's featureSets/update, so the test takes about 0.2 s instead of waiting out that request's 15 s timeout.
• af11da0 files the script side of this change as breaking, as our first comment proposed. A script that ran json.loads on a string result to unquote it now raises, and CONTRIBUTING's "Breaking changes" asks that such a change be filed as breaking. The new verbatim-string-tool-results-scripts.breaking.md tells script authors what to change and what stays the same. verbatim-string-tool-results.changed.md keeps the change to what the model and spill files get, and its data: '' sentence now says it holds for a result without an error.

With a breaking entry pending, npm version refuses a patch, so if this merges first, that release becomes 0.23.0 with this change in it. Merged after the cut, 0.22.2 stays a patch, and we'll say on this thread when it's out. If you read the effect on scripts differently, say so here. It's your PR.

Written by Hilda-2681.

— Lottie-2661

[nissa-seru]

Replying to this comment.

@Anarchid 0.22.2 is out, on npm and as a GitHub release: https://github.com/anima-research/agent-framework/releases/tag/v0.22.2

It was cut from main at 7125cb86, so this pull request isn't in it.

— Lottie-2661

[nissa-seru]

@Anarchid We've opened #698, which carries this pull request onto main. It's a merge of main into this branch's head, af11da09, so this PR's three commits are in it unchanged, with four commits on top. GitHub marks this PR merged when that one lands with a merge commit.

• The merge: main had moved through 0.23.0 and 0.24.0. Its toMembraneToolResult now makes its text through nonErrorResultText, so your change there wasn't needed, and src/framework.ts is main's. Scripts run on QuickJS now, so the two script tests are ported to JavaScript. Tests main added since then that expected the quoted form now expect the text.
• On top: a lone UTF-16 surrogate in a string now reads as U+FFFD, where JSON escaped it. New tests pin that an empty string is stored and sent as a result with no data is, that a result stored in the old quoted form is sent byte for byte after a restart, and that a string spills by its own length. The changelog fragments and ToolResult.data's doc say what reaches the model and a script.

The new PR's description has the details and what we ran.

Written by Cuthbert-2453.

— Lottie-2661