fix(tool-results): a lone surrogate in a tool result's text reaches membrane as U+FFFD, as it's stored

changelog.d/tool-result-lone-surrogate.fixed.md

@@ -0,0 +1 @@
+- A lone UTF-16 surrogate in a tool result's text now reaches membrane as U+FFFD, as the context stores it (#695). Before, a text block's text, a reference's stub (from its `name`), an error result's text and the error text of a failure the tool doesn't flag went to membrane with the lone half raw, while the stored copy held U+FFFD. With the tool-result guard on (`toolResultGuard`), such a result left a stored message the context couldn't read back; with a text block or an error result, the agent's next turn sent no request and `stop()` threw. This change doesn't repair a store that already holds such a message.

src/agent.ts

@@ -1921,14 +1921,21 @@ export class Agent {
     // via buildStoredToolResultContent, with workspace spill). No workspace
     // is reachable from here, so the house default cap applies as plain
     // truncation — bounded beats unbounded, for errors too.
-    const content = results.map((r) => ({
-      type: 'tool_result' as const,
-      toolUseId: r.id,
-      content: r.result.isError
-        ? truncateForHistory(r.result.error ?? 'Unknown error', DEFAULT_TOOL_RESULT_INLINE_MAX_CHARS)
-        : nonErrorResultText(r.result, DEFAULT_TOOL_RESULT_INLINE_MAX_CHARS),
-      isError: r.result.isError,
-    }));
+    const content = results.map((r) => {
+      // This API's results are unchecked (nonErrorResultText), so each field
+      // is read once, and only an error that is a string is made well formed;
+      // any other error value is passed on as it was before.
+      const isError = r.result.isError;
+      const error: unknown = isError ? r.result.error ?? 'Unknown error' : undefined;
+      return {
+        type: 'tool_result' as const,
+        toolUseId: r.id,
+        content: isError
+          ? truncateForHistory(typeof error === 'string' ? error.toWellFormed() : error as string, DEFAULT_TOOL_RESULT_INLINE_MAX_CHARS)
+          : nonErrorResultText(r.result, DEFAULT_TOOL_RESULT_INLINE_MAX_CHARS),
+        isError,
+      };
+    });
 
     return [{
       participant: 'user',

src/framework.ts

@@ -20631,7 +20631,7 @@ export class AgentFramework {
     tc: CompletedToolCall,
     maxChars: number | undefined,
   ): { text: string; shown: ShownRanges | null } {
-    if (tc.result.isError) return { text: tc.result.error ?? 'Unknown error', shown: null };
+    if (tc.result.isError) return { text: (tc.result.error ?? 'Unknown error').toWellFormed(), shown: null };
     const ledger = this.toolImageLedgerFor(agentName);
     let blocks = null as HistoryBlockSpan[] | null;
     const text = nonErrorResultText(tc.result, undefined, {
@@ -21185,7 +21185,8 @@ export class AgentFramework {
       if (b.type === 'text') {
         // A text that isn't a string shows as the converters' placeholder,
         // rather than sending the whole array to the JSON fallback.
-        let text = typeof b.text === 'string' ? b.text : NON_STRING_TEXT_PLACEHOLDER;
+        // Well formed, as the stored copy's text is (nonErrorResultText).
+        let text = typeof b.text === 'string' ? b.text.toWellFormed() : NON_STRING_TEXT_PLACEHOLDER;
         if (maxChars && text.length > maxChars) {
           text = text.slice(0, toolResultCutAt(text, maxChars))
             + '\n\n[truncated — original was ' + text.length + ' chars'

src/mcpl/references.ts

@@ -434,15 +434,18 @@ export function unfetchableReason(record: ReferenceRecord, nowMs = Date.now()):
 /**
  * RFC-005 treatment for one MCPL wire block, for the model-visible text
  * lanes. Returns a replacement string, or null when the block should keep
- * its existing (inline/text) handling.
+ * its existing (inline/text) handling. A reference's stub carries text taken
+ * from the block's own fields, so it's made well formed (a lone UTF-16
+ * surrogate becomes U+FFFD), as the context store would make it: a tool
+ * result's copies then carry the same text (#695).
  */
 export function referenceStubOrNull(block: unknown, provenance?: string, serverId?: string): string | null {
   const c = classifyBlock(block);
   switch (c.kind) {
     case 'text': return null;
     case 'inline':
       return c.contradiction ? INLINE_WITHHELD_TEXT : null;
-    case 'reference': return buildReferenceStub(c.testimony, provenance, serverId);
+    case 'reference': return buildReferenceStub(c.testimony, provenance, serverId).toWellFormed();
     case 'invalid': return c.shown;
   }
 }

src/tool-result-history.ts

@@ -9,7 +9,8 @@
  * `maxChars` slice and re-introduces, one turn deferred, exactly the silent
  * hallucination this feature exists to kill.
  *
- * For MCP-shaped content arrays, this helper keeps text blocks verbatim and
+ * For MCP-shaped content arrays, this helper keeps text blocks verbatim,
+ * apart from a lone UTF-16 surrogate, which becomes U+FFFD (#695), and
  * replaces image blocks with a short `[image: mimeType, ~NKB]` placeholder.
  * 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
@@ -97,6 +98,14 @@ export function toolResultDataToHistoryString(
  *
  * Each field is read once: the direct Agent API passes its caller's own
  * result here, unchecked, and a getter needn't answer the same way twice.
+ *
+ * Each text a tool result's copies carry is made well formed where it's
+ * taken from the result: a lone UTF-16 surrogate becomes U+FFFD, as the
+ * context store makes it when it writes, so the copy sent to membrane and the
+ * copy stored carry the same text (#695). Other data goes through JSON,
+ * which escapes a lone half. toWellFormed() keeps the length,
+ * so the cut, the spans and the slots are computed from the well-formed text
+ * as before; a cut at a lone high half no longer steps back over it.
  */
 export function nonErrorResultText(
   result: { data?: unknown; error?: string },
@@ -106,7 +115,10 @@ export function nonErrorResultText(
   const data = result.data;
   if (data === undefined || data === null) {
     const error = result.error;
-    if (typeof error === 'string' && error) return maxChars ? truncateForHistory(error, maxChars) : error;
+    if (typeof error === 'string' && error) {
+      const text = error.toWellFormed();
+      return maxChars ? truncateForHistory(text, maxChars) : text;
+    }
   }
   return toolResultDataToHistoryString(data, maxChars, options);
 }
@@ -139,7 +151,9 @@ function tryHistoryStringFromContentArray(data: unknown, options?: HistorySerial
     if (b.type === 'text') {
       // A text that isn't a string shows as the converters' placeholder,
       // rather than sending the whole array to the JSON fallback.
-      parts.push({ kind: 'text', text: typeof b.text === 'string' ? b.text : NON_STRING_TEXT_PLACEHOLDER });
+      // Read once: a direct Agent API result is unchecked.
+      const text = b.text;
+      parts.push({ kind: 'text', text: typeof text === 'string' ? text.toWellFormed() : NON_STRING_TEXT_PLACEHOLDER });
     } else if (isInlineContradiction(b)) {
       // RFC-005 vector 2: checked before the data branch so inline data
       // claiming bulk disposition is withheld, not rendered.

test/tool-result-well-formed.test.ts

@@ -0,0 +1,160 @@
+/**
+ * A lone UTF-16 surrogate in a tool result's text reaches membrane as U+FFFD,
+ * as the context store writes it, so the copy sent and the copy stored match
+ * (#695). Each text a result's copies carry is made well formed where it's
+ * taken from the result: a text block's text and a reference's stub, in the
+ * history converter and the native one, and an error result's text, on the
+ * framework's path and the direct Agent API's.
+ */
+import { afterEach, test } from 'node:test';
+import assert from 'node:assert/strict';
+import { mkdtempSync, rmSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import type { NormalizedRequest, NormalizedResponse } from '@animalabs/membrane';
+import { AgentFramework, type AgentConfig, type Module, type ProcessEvent, type ToolResult } from '../src/index.js';
+import { MockMembrane, createMockResponse } from './helpers/mock-membrane.js';
+
+const LONE = 'cut \ud83d here';
+const MENDED = 'cut \ufffd here';
+// A 1×1 transparent PNG.
+const PNG = 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg==';
+const IMAGE = { type: 'image', data: PNG, mimeType: 'image/png' };
+
+const dirs: string[] = [];
+afterEach(() => { while (dirs.length) rmSync(dirs.pop()!, { recursive: true, force: true }); });
+
+/** Each text a tool result's content holds: a string, or its text blocks'. */
+function texts(content: unknown): string[] {
+  if (typeof content === 'string') return [content];
+  if (!Array.isArray(content)) return [];
+  return content.flatMap((b) => (b && typeof b === 'object' && typeof (b as { text?: unknown }).text === 'string'
+    ? [(b as { text: string }).text] : []));
+}
+
+class Canned implements Module {
+  readonly name = 'canned';
+  constructor(private readonly result: ToolResult) {}
+  async start() {}
+  async stop() {}
+  getTools() {
+    return [{ name: 'fetch', description: 'Returns the canned result.', inputSchema: { type: 'object' as const, properties: {} } }];
+  }
+  async handleToolCall() { return this.result; }
+  async onProcess(event: ProcessEvent) {
+    return event.type === 'external-message'
+      ? { addMessages: [{ participant: 'user', content: [{ type: 'text' as const, text: 'fetch it' }] }], requestInference: true }
+      : {};
+  }
+}
+
+async function framework(result: ToolResult, membrane: MockMembrane, agent: Partial<AgentConfig> = {}) {
+  const dir = mkdtempSync(join(tmpdir(), 'af-well-formed-')); dirs.push(dir);
+  return AgentFramework.create({
+    storePath: join(dir, 'store'),
+    membrane: membrane.asMembrane(),
+    agents: [{ name: 'prime', model: 'test-model', systemPrompt: 'You are prime.', allowedTools: 'all', ...agent }],
+    modules: [new Canned(result)],
+    syncIntervalMs: 0,
+  });
+}
+
+function storedToolResult(f: AgentFramework): unknown {
+  const blocks = f.getAgent('prime')!.getContextManager().getAllMessages()
+    .flatMap((m) => m.content.filter((b) => b.type === 'tool_result'));
+  assert.equal(blocks.length, 1, 'one tool result is stored');
+  return (blocks[0] as { content: unknown }).content;
+}
+
+const SHAPES: Array<[string, ToolResult]> = [
+  ['a text block beside an image', { success: true, data: [{ type: 'text', text: LONE }, IMAGE] }],
+  ['a text block alone', { success: true, data: [{ type: 'text', text: LONE }] }],
+  ['a reference named with it, beside an image', { success: true, data: [{ type: 'resource', uri: 'memo://f417-beside', name: LONE }, IMAGE] }],
+  ['a reference named with it, alone', { success: true, data: [{ type: 'resource', uri: 'memo://f417-alone', name: LONE }] }],
+  ['a reference named with it as the whole data', { success: true, data: { type: 'resource', uri: 'memo://f417-bare', name: LONE } }],
+  ['an error result', { success: false, isError: true, error: LONE }],
+  ["a failure the tool doesn't flag as an error", { success: false, error: LONE }],
+];
+
+// With the guard on, the guard stores the result, and on main the stored
+// message couldn't be read back: the next turn sent nothing and stop() threw.
+for (const guard of [false, true]) for (const [shape, result] of SHAPES) {
+  test(`a lone surrogate in ${shape} reaches membrane as the stored copy holds it, U+FFFD${guard ? ', with the guard on' : ''}`, async () => {
+    const membrane = new MockMembrane();
+    membrane.pushResponse(createMockResponse([{ type: 'tool_use', id: 'call_fetch', name: 'canned--fetch', input: {} }], 'tool_use'));
+    membrane.pushResponse(createMockResponse([{ type: 'text', text: 'Handled.' }]));
+    const f = await framework(result, membrane, { toolResultGuard: guard });
+    try {
+      f.pushEvent({ type: 'external-message', source: 'test', content: 'fetch it', metadata: {} });
+      await f.runUntilIdle();
+      const sent = (membrane.lastStream?.receivedToolResults[0]?.[0] as { content: unknown } | undefined)?.content;
+      assert.ok(sent !== undefined, 'a tool result reached membrane');
+      const stored = storedToolResult(f);
+      const sentTexts = texts(sent);
+      assert.ok(sentTexts.length > 0, 'the result sent carries text');
+      for (const t of sentTexts) assert.ok(t.isWellFormed(), `the text sent is well formed: ${JSON.stringify(t)}`);
+      assert.ok(sentTexts.some((t) => t.includes(MENDED)), 'the lone half is sent as U+FFFD');
+      if (typeof sent === 'string') {
+        assert.equal(sent, stored, 'the copy sent is the copy stored');
+      } else {
+        // Sent as native blocks beside the image: the stored copy is their
+        // text, with the image as its placeholder.
+        const storedText = texts(stored).join('\n');
+        for (const t of sentTexts) assert.ok(storedText.includes(t), `the stored copy holds the text sent: ${JSON.stringify(t)}`);
+      }
+    } finally {
+      await f.stop();
+    }
+  });
+}
+
+/**
+ * One round through the direct Agent API: a call answered with `result`,
+ * then the next request, whose tool results' content goes to `check`.
+ */
+async function directRound(result: ToolResult, agentConfig: Partial<AgentConfig>, check: (sent: unknown[], f: AgentFramework) => void) {
+  const membrane = new MockMembrane();
+  const requests: NormalizedRequest[] = [];
+  const responses: NormalizedResponse[] = [
+    createMockResponse([{ type: 'tool_use', id: 'one', name: 'canned--fetch', input: {} }], 'tool_use'),
+    createMockResponse([{ type: 'text', text: 'Handled.' }]),
+  ];
+  (membrane as unknown as { stream: (request: NormalizedRequest) => Promise<NormalizedResponse> }).stream = async (request) => {
+    requests.push(structuredClone(request));
+    const response = responses.shift();
+    assert.ok(response, 'no more inference than scripted');
+    return response;
+  };
+  const f = await framework({ success: true, data: 'unused' }, membrane, agentConfig);
+  try {
+    const agent = f.getAgent('prime')!;
+    agent.getContextManager().addMessage('user', [{ type: 'text', text: 'fetch it' }]);
+    assert.equal((await agent.runInference(f.getAllTools())).toolCalls.length, 1);
+    agent.provideToolResult('one', result);
+    await agent.runInference(f.getAllTools());
+    assert.equal(requests.length, 2);
+    const sent = requests[1].messages.flatMap((m) => m.content).filter((b) => b.type === 'tool_result')
+      .map((b) => (b as { content: unknown }).content);
+    assert.equal(sent.length, 1, 'the second request carries the tool result');
+    check(sent, f);
+  } finally {
+    await f.stop();
+  }
+}
+
+test('a lone surrogate in an error result given through the direct Agent API reaches membrane as U+FFFD, as it is stored', async () => {
+  // The guard stores the direct API's results; without it, the caller keeps
+  // the context, and nothing is stored to compare with.
+  await directRound({ success: false, isError: true, error: LONE }, { toolResultGuard: true }, (sent, f) => {
+    assert.equal(sent[0], MENDED, 'the lone half is sent as U+FFFD');
+    assert.equal(sent[0], storedToolResult(f), 'the copy sent is the copy stored');
+  });
+});
+
+test("an error that isn't a string, given through the direct Agent API, is passed on as it was", async () => {
+  // That API's results are unchecked: only a string error is made well
+  // formed, and any other value goes on as before.
+  await directRound({ success: false, isError: true, error: ['not a string'] as unknown as string }, {}, (sent) => {
+    assert.deepEqual(sent[0], ['not a string']);
+  });
+});