[nissa-seru]

A tool result whose text holds a lone UTF-16 surrogate went to membrane with the lone half raw, while the context stored U+FFFD in its place. With the tool-result guard on, the message the context stored couldn't be read back at all. This change makes each text a tool result's copies carry well formed where it's taken from the result, so the copy sent and the copy stored carry the same text.

Problem

String.prototype.toWellFormed() turns a lone surrogate into U+FFFD. The context store does the same when it writes a message. On main at 8e0bb04e, the framework took a tool result's text as it was in these places:

• a text block's text, in the history converter (tryHistoryStringFromContentArray) and in the native one (tryNativeToolResultContent), which sends the blocks when the result carries an image;
• a reference's stub, in both converters and where a bare reference is the result's whole data, since its name passes sanitizeLabel, which keeps a lone half;
• an error result's text (storedToolResultText), and the error text of a failure the tool doesn't flag as an error (nonErrorResultText);
• an error result given through the direct Agent API (buildToolResultMessages).

So for that round, the request sent to membrane carried the lone half, and the stored copy held U+FFFD. framework.ts keeps the two byte-matched on purpose, because the window stores what membrane sends. By the notes at src/modules/workspace/text.ts:17 and in membrane's src/utils/image-media.ts, a model provider can refuse a request that carries a lone surrogate. No such request went to a provider here.

With the tool-result guard on (toolResultGuard, off by default, and per agent through agent_settings tool_result_guard), the harm is worse. Run on main with a text block or an error result holding a lone half:

• reading the agent's stored messages throws Deserialization error: unexpected end of hex escape, from context-manager's MessageStore.getInternal; it throws first in the guard's settlement, whose message edit can't read back what it stored;
• the agent's next turn sends no request;
• stop() throws the same error.

This change's new tests fail on main at 8e0bb04e (see Checks).

Changes

• Each text a tool result's copies carry is made well formed where it's taken from the result, with toWellFormed():
  • a text block's text, in both converters;
  • a reference's stub, where referenceStubOrNull builds it, which every stub of a tool result passes through;
  • an error result's text, in storedToolResultText;
  • the error text of a failure the tool doesn't flag, in nonErrorResultText;
  • an error result given through the direct Agent API, in buildToolResultMessages.

String data is already sent as its text, made well formed (toolResultDataToHistoryString, from #698).

What each part is for

• Where the text is taken, not where it's sent. Each copy is then built from text already well formed, so the copy sent and the copy stored carry the same text whichever path builds them. A result sent as native blocks beside an image was never byte-matched to its stored copy, which shows the image as a placeholder; its text blocks now appear in the stored copy as they're sent.
• toWellFormed() keeps the length. The cut at the cap, a block's span in the stored copy and the slots a stored copy keeps are computed from the well-formed text, the same way as before. By our reading, two outcomes can move: a cut that falls just after a lone high half no longer steps back over it, since it's U+FFFD now (safeSlice); and a reference nested in other data, whose lone half JSON wrote as a six-character escape, now shows U+FFFD in its stub, so that JSON is five characters shorter.
• The stubs, where they're built. A reference's stub is text in both copies, and its name can carry a lone half. Made well formed in referenceStubOrNull, it is so in both converters, as a bare reference's whole data, and in the guard's kept text (rfc005TextOf).
• The error text. It reaches the copies by its own paths, which the converters don't see. The direct Agent API's results are unchecked, so there an error that isn't a string is left as it was, and a text block's text is read once.
• Left as they were: toMembraneToolResult's two fallbacks, for an error result and for any other. Both of its callers pass the copy buildStoredToolResultContent made for every result, so, by our reading, neither fallback is reached from them.

+40 −15 lines in src with comments, +21 −13 without.

Tests

• tool-result-well-formed, new. Each test gives a tool result with a lone high half, cut \ud83d here. Each asserts that every text sent to membrane is well formed and holds U+FFFD, and that the copy sent is the copy stored. Where the result is sent as native blocks beside an image, it asserts that the stored copy holds each text sent. Each of these shapes runs with the guard off and with it on:

  • a text block beside an image, and a text block alone;
  • a reference named with it, beside an image, alone, and as the result's whole data;
  • an error result, and a failure the tool doesn't flag as an error.

  One more test gives an error result through the direct Agent API, with the guard on, since the guard is what stores that API's results. Another gives an error that isn't a string through the direct Agent API, with the guard off, and asserts it's sent as it was, since that API's results are unchecked.

• It protects against a text taken from a result raw, which leaves the copy sent and the copy stored different.

Checks

• Red-first on main at 8e0bb04e, with this change's test file as of 0e62d7a6: all 15 tests fail. With the guard off, each shape fails at "the text sent is well formed". With the guard on, each shape and the direct API's test fail with Deserialization error: unexpected end of hex escape. The later test of an error that isn't a string pins what main already does, by our reading of truncateForHistory, so it isn't a red-first test.
• Removals: each of the 7 sites taken out on its own fails at least one test: the history converter's text 3, the stub 6, an unflagged failure's error 2, a stored error result 2, the native text 2, the direct API's error 1, and the direct API's check that only a string error is made well formed 1. Taking out the stub's call fails the six reference tests.
• Focused runs on Linux with Node 24.22.0. At this head, the typecheck is clean, tool-result-well-formed passes 16/16 and tool-result-guard 150/150. At 0e62d7a6, whose src this head keeps unchanged:
  • the typecheck is clean;
  • tool-result-well-formed 15/15, tool-result-history 22/22, unconvertible-tool-result 11/11, tool-result-guard 150/150, mcp-modern-framework 18/18, tool-result-spill 23/23, tool-result-cap-fit 13/13, tool-result-reference-uri 21/21, mcp-tool-result-normalize 21/21.
• The guard's next turn, by a probe on main at 8e0bb04e and at 0e62d7a6, for a text block, string data and an error result, each with the guard off and on. On main, a text block or an error result with the guard on leaves a stored message that can't be read back, a next turn that sends no request, and a stop() that throws. At 0e62d7a6 every case runs normally.

Not verified

• A provider's refusal. No request with a lone surrogate went to a provider.
• A restart through the agent. We didn't reopen an agent's store after the guard's wedge. Through context-manager and through chronicle alone, reopening leaves the store unreadable (An edit whose JSON escapes a lone surrogate half makes its whole state unreadable, for good chronicle#52).

What this leaves

• Other ways into the store. With the guard on, a lone half that reaches the store by some other route than a tool result's text could leave the same unreadable message. By runs on context-manager's main, a message edit stores a lone half as a JSON escape that chronicle's replay can't parse, and by our reading the guard's settlement edit is that write (An edit whose JSON escapes a lone surrogate half makes its whole state unreadable, for good chronicle#52). This change closes the tool-result route.
• A store that already holds such a message isn't repaired by this change. An edit whose JSON escapes a lone surrogate half makes its whole state unreadable, for good chronicle#52 names a lenient replay that would.

Fixes #695.

Written by Crispin-2713 (Claude Opus 5.5). Effie-2289 (Claude Opus 5.5) read each round and approved it. Our internal review read this change before its last round; this head carries the fixes it led to, and Effie-2289 read that round.

Description checked and posted by Fiona-2757 (Claude Opus 5.5).

🤖 Written using Anthropic's Claude Opus 5.5 through the Rookery's tooling, and posted from the shared nissa-seru account.

[github-actions]

Automated review

Recommendation: merge. The review found nothing of medium or high severity.

Reviewed commit 0e62d7a with claude-opus-5-5, by reading the code. It did not run the code or look at the checks.

Does this change belong in the project?

It fits the project. This bug fix belongs in the project. The framework already makes some text well formed with toWellFormed() before it reaches membrane, and this change applies that to the tool-result text it missed, inside the existing converters and a tests-plus-changelog PR shape.

4 reasons with sources

• It fixes existing behavior: what gets sent to membrane no longer differs from what gets stored, and guard-stored messages can be read back again. No new feature, interface or dependency is added. It extends the sent copy = stored copy rule the framework already keeps to text blocks, reference stubs and error text. (src/framework.ts (tool-result copies are kept byte-matched to what membrane is sent))
• It repeats a pattern the codebase already uses rather than adding a second mechanism. String data is already made well formed with toWellFormed() in toolResultDataToHistoryString (#698), and inbound-source does the same. This PR applies that call at the remaining places where text is taken from a tool result. (src/tool-result-history.ts:88, src/mcpl/inbound-source.ts:190)
• The edits stay in the modules that already own this work. The history converter, the native converter, the reference-stub builder (referenceStubOrNull) and the direct Agent API's buildToolResultMessages each change at the point where they read the text. (src/tool-result-history.ts, src/mcpl/references.ts, src/agent.ts, src/framework.ts)
• It brings a test file that fails on main and a changelog fragment, as CONTRIBUTING asks. It adds test/tool-result-well-formed.test.ts, reported as failing first on main, and changelog.d/tool-result-lone-surrogate.fixed.md. Coverage gaps in those tests are left to the code review. (CONTRIBUTING.md (What a PR should contain; Changelog))

Findings

No findings of medium or high severity.

2 more findings, less severe

• Low severity, test coverage, test/tool-result-well-formed.test.ts:108-142. Direct Agent API test covers only the error-result branch. buildToolResultMessages has two branches that this change affects: the isError branch (the new typeof/toWellFormed code) and nonErrorResultText (an unflagged failure's error text and text blocks). The only direct-API test feeds { isError: true, error: LONE }. Nothing checks the non-error branch through the direct API, or that a non-string error is still passed on unchanged as the new comment promises. Add cases for an unflagged failure and for a non-string error value.
• Low severity, test coverage, src/mcpl/references.ts:448. Change to nested reference stubs inside JSON data is untested. Making the stub well formed in referenceStubOrNull also changes historyJson's output for a reference nested in other data: the lone half used to appear as a six-character JSON escape and now appears as U+FFFD, five characters shorter. The PR description says so. referenceStubOrNull is also used by the push lanes (convert-block.ts), so their stubs change too. None of the new tests covers a nested reference or the push lane, so the change to the stored length and spans for that shape is unchecked.