From 2b9b98109cb2845d292dda2fc93bdcd0c1ee343a Mon Sep 17 00:00:00 2001 From: Rupert-2634 Date: Fri, 16 Oct 2026 16:04:59 -0400 Subject: [PATCH] fix: text cut for an agent to a fixed length keeps surrogate pairs whole Six places cut text an agent reads to a fixed length with a plain slice: the event gate's quote of a debounced event (200 units), utils list's one-liner (160), a failed spill's reason in a truncated tool result (200), health's snapshot of recent errors (400), the last 2000 units of a dead background script's output, in the wake that tells the agent it died, and thrownText, as a subconscious tool's failed result reports what it threw (500). A cut that fell inside a surrogate pair, half of an emoji for instance, kept one half. Text stored as text reads back with U+FFFD where the half was, and a result's data, stored as its JSON, holds an escape such as \ud83d. Five now cut with safeSlice, which leaves out a pair the cut would halve: the four that keep the text's start end before a high surrogate at their end, and the script's tail starts after a low surrogate at its start. thrownText has a twin in boundedErrorMessage (error-message.ts): the same 500-character bound, fallback and note, with characters counted as code points, so its cut never halves a pair. framework.ts's thrownText, and the copy of it in mcpl/checkpoint-manager.ts, are now that helper, imported under their own name, so none of their call sites change. The "(N characters)" they give now counts code points, in every quote of a thrown value they make, most of them refusals when stored state can't be read. Tests, each failing without this change: a pair placed across the cut is left out whole, in the event gate's debounce, utils list, the spill's failure note, health's snapshot, and a dead script's wake, where two runs give the cut both parities; a thrown value's text keeps the pair whole and counts code points, in a subconscious tool's failed result and in the checkpoint manager's refusal; and a thrown value of 300 emoji, 600 UTF-16 units, comes back whole. Co-Authored-By: Claude Opus 5.5 --- changelog.d/cuts-keep-pairs.fixed.md | 19 +++++++++++ src/framework.ts | 30 +++--------------- src/gate/event-gate.ts | 3 +- src/mcpl/checkpoint-manager.ts | 12 +------ src/modules/health/index.ts | 3 +- test/boot-state-read-errors.test.ts | 25 +++++++++++++++ test/code-execution.test.ts | 45 +++++++++++++++++++++++++++ test/event-gate.test.ts | 20 ++++++++++++ test/health-snapshot.test.ts | 18 +++++++++++ test/stop-with-work-in-flight.test.ts | 29 +++++++++++++++++ test/tool-result-spill.test.ts | 25 +++++++++++++++ test/utils-surface.test.ts | 21 +++++++++++++ 12 files changed, 212 insertions(+), 38 deletions(-) create mode 100644 changelog.d/cuts-keep-pairs.fixed.md diff --git a/changelog.d/cuts-keep-pairs.fixed.md b/changelog.d/cuts-keep-pairs.fixed.md new file mode 100644 index 000000000..bd4699bcb --- /dev/null +++ b/changelog.d/cuts-keep-pairs.fixed.md @@ -0,0 +1,19 @@ +- **Text cut to a fixed length for an agent to read no longer ends or + starts on half a surrogate pair.** An emoji or another character outside + the Basic Multilingual Plane that fell across such a cut left half of its + pair, which the agent read as a replacement character (U+FFFD) or, in a + result's data, as an escape such as `\ud83d`. These cuts now leave out a + pair they would halve: + - the event gate's quote of a debounced event's content (200 units); + - a utility's one-liner in `utils list` (160 units); + - a failed spill's reason in a truncated tool result (200 units); + - each recent error in health's `snapshot` (400 units); + - the last 2000 units of a dead background script's output, in the wake + that tells the agent it died; + - a thrown error's text, as a subconscious tool's failed result reports + it. It's now cut at 500 characters counted as code points, as + `boundedErrorMessage` cuts, and its "(N characters)" counts code points. + A text within 500 characters now comes back whole, even past 500 UTF-16 + units. So do the framework's and the MCPL checkpoint manager's other + quotes made through `thrownText`, such as a refusal when stored state + can't be read. diff --git a/src/framework.ts b/src/framework.ts index 4c12b2002..7981de693 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -102,6 +102,7 @@ import { ChannelRegistry, type ChannelToolOrigin } from './mcpl/channel-registry import { ConversationRouter, CONVERSATION_ROUTING_DEPRECATION_NOTICE } from './mcpl/conversation-router.js'; import type { IncomingMessageFacts, RoutingDecision } from './mcpl/conversation-router.js'; import { safeSlice } from './safe-slice.js'; +import { boundedErrorMessage as thrownText } from './error-message.js'; import { ProviderWaits, EVERY_MODEL, isReleaseModel, withoutControls, type ProviderWait } from './provider-waits.js'; import type { WorkspaceModule } from './modules/workspace/index.js'; import type { ToolResultGuard, WithheldSpill } from './tool-result-guard.js'; @@ -388,7 +389,7 @@ const UTILS_TOOL: import('./types/index.js').ToolDefinition = { const utilityOneLiner = (description: string): string => { const line = description.split('\n')[0]!; const dot = line.indexOf('. '); - return (dot >= 0 ? line.slice(0, dot + 1) : line).slice(0, 160); + return safeSlice(dot >= 0 ? line.slice(0, dot + 1) : line, 0, 160); }; /** Shallow validation of run args against a utility's declared inputSchema: @@ -1672,28 +1673,6 @@ export function shedMayHelp(errorType: string | undefined, httpStatus: number | return errorType === 'invalid_request' && (httpStatus === undefined || httpStatus === 400 || httpStatus === 422); } -/** The most of a thrown value's or a tool error's text that reaches a log line or a server's error answer. */ -const THROWN_TEXT_LIMIT = 500; - -/** - * What a thrown value says, safely and briefly: an Error's message, or any - * other value, as a string. Every inspection and coercion of the value is - * inside the try, so reading it can't throw: a revoked Proxy (whose - * instanceof throws), a hostile getter, an Error whose message isn't a - * string, or a value with no string form reads as text or as unreadable. - * The text is bounded, since a provider or handler error can carry a whole - * request body. - */ -function thrownText(error: unknown): string { - let text: string; - try { - text = String(error instanceof Error ? error.message : error); - } catch { - text = '(a value that could not be read)'; - } - return text.length <= THROWN_TEXT_LIMIT ? text : `${text.slice(0, THROWN_TEXT_LIMIT)}… (${text.length} characters)`; -} - /** A stored value that read but isn't in a form this code takes, as against * a read that failed: starting again changes nothing until it's repaired, * so the refusal that quotes it says so instead of "once it can be read". */ @@ -15890,7 +15869,8 @@ export class AgentFramework { // promise die silently. Deliberate cancels/disposes stay silent (the // agent or host chose them); clean exits stay silent (the null path). if (record.status === 'died' && !record.cancelled) { - const tail = (exec.tail ?? exec.stderr ?? '').slice(-2000); + const output = exec.tail ?? exec.stderr ?? ''; + const tail = safeSlice(output, Math.max(0, output.length - 2000), output.length); const elapsedMin = Math.round((Date.now() - record.startedAt) / 60_000); const envelope = `[background script ${record.id}] Your background script DIED ${elapsedMin}m after start ` + @@ -16137,7 +16117,7 @@ export class AgentFramework { } catch (err) { failure = err instanceof Error ? err.message : String(err); } - failure = failure.slice(0, 200); + failure = safeSlice(failure, 0, 200); this.emitTrace({ type: 'tool:spill_failed', label: safeLabel, diff --git a/src/gate/event-gate.ts b/src/gate/event-gate.ts index 5ec0e082c..d64b43328 100644 --- a/src/gate/event-gate.ts +++ b/src/gate/event-gate.ts @@ -14,6 +14,7 @@ import { existsSync, readFileSync, statSync, writeFileSync, mkdirSync } from 'node:fs'; import { dirname, join } from 'node:path'; import { GateScript } from './gate-script.js'; +import { safeSlice } from '../safe-slice.js'; import type { ToolDefinition } from '../types/events.js'; import type { @@ -1484,7 +1485,7 @@ export class EventGate { const event: PendingEvent = { policyName: policy.name, content: info.content.length > MAX_CONTENT_SNIPPET - ? info.content.slice(0, MAX_CONTENT_SNIPPET) + '...' + ? safeSlice(info.content, 0, MAX_CONTENT_SNIPPET) + '...' : info.content, eventType: info.eventType, timestamp: Date.now(), diff --git a/src/mcpl/checkpoint-manager.ts b/src/mcpl/checkpoint-manager.ts index 63974f92c..17171e1a4 100644 --- a/src/mcpl/checkpoint-manager.ts +++ b/src/mcpl/checkpoint-manager.ts @@ -14,6 +14,7 @@ import type { JsStore } from '@animalabs/chronicle'; import { ensureStateRegistered } from '../module-registry.js'; +import { boundedErrorMessage as thrownText } from '../error-message.js'; import type { StateCheckpoint, JsonPatchOperation } from './types.js'; @@ -31,17 +32,6 @@ const STATE_PREFIX = 'mcpl/state'; const MCPL_STOPS = `MCPL doesn't start without them, and neither does create() with MCPL servers configured, ` + `nor a first connectMcplServer()`; -/** A thrown value's text, bounded and never throwing, as framework.ts's thrownText. */ -function thrownText(error: unknown): string { - let text: string; - try { - text = String(error instanceof Error ? error.message : error); - } catch { - text = '(a value that could not be read)'; - } - return text.length <= 500 ? text : `${text.slice(0, 500)}… (${text.length} characters)`; -} - /** A stored JSON value's kind, for a refusal that names what it found without its content. */ function storedKind(value: unknown): string { return value === undefined ? 'missing' diff --git a/src/modules/health/index.ts b/src/modules/health/index.ts index efde3db55..f40e9a7ee 100644 --- a/src/modules/health/index.ts +++ b/src/modules/health/index.ts @@ -25,6 +25,7 @@ import type { ToolDefinition, ToolCall, ToolResult, ProcessEvent } from '../../t import type { EventResponse, ProcessState } from '../../types/module.js'; import type { AgentFramework } from '../../framework.js'; import { formatZonedDateTime, resolveTimeZone } from '../../timezone.js'; +import { safeSlice } from '../../safe-slice.js'; export interface HealthModuleConfig { /** IANA zone for timestamps returned by the snapshot tool. */ @@ -196,7 +197,7 @@ export class HealthModule implements Module { .map((s) => ({ timestamp: s.timestamp, agentName: s.agentName, - error: typeof s.error === 'string' ? s.error.slice(0, 400) : s.error, + error: typeof s.error === 'string' ? safeSlice(s.error, 0, 400) : s.error, requestId: s.requestId, })); diff --git a/test/boot-state-read-errors.test.ts b/test/boot-state-read-errors.test.ts index deff1b50d..99334fa2c 100644 --- a/test/boot-state-read-errors.test.ts +++ b/test/boot-state-read-errors.test.ts @@ -594,6 +594,31 @@ test("MCPL checkpoint trees that can't be read, or that aren't checkpoint trees, await f.create(); }); +test("an unreadable checkpoint store's error is quoted to 500 characters, counted in code points, without halving a surrogate pair", inStoreDir('boot-read-long-error-', async (storePath) => { + const store = JsStore.openOrCreate({ path: storePath }); + // 499 characters, then an emoji whose two UTF-16 units straddle unit 500. + // Counted in code points, the emoji is the 500th character. + const message = `${'x'.repeat(499)}😀 and more`; + const proto = JsStore.prototype as unknown as { getStateJson(this: JsStore, id: string): unknown }; + const original = proto.getStateJson; + proto.getStateJson = function (this: JsStore, id: string) { + if (id === 'mcpl/checkpoints') throw new Error(message); + return original.call(this, id); + }; + try { + assert.throws(() => new CheckpointManager(store, () => {}), (error: Error) => { + assert.ok( + error.message.startsWith(`the MCPL checkpoint trees (mcpl/checkpoints) could not be read: ${'x'.repeat(499)}😀… (509 characters). MCPL doesn't start`), + `the cut keeps the emoji whole, and the count is of code points: …${JSON.stringify(error.message.slice(555, 600))}`, + ); + return true; + }); + } finally { + proto.getStateJson = original; + store.close(); + } +})); + /** A stored host-managed checkpoint tree for `server`'s `featureSet`, holding `state`, as the checkpoint manager stores them. */ function seedHostManaged(storePath: string, server: string, featureSet: string, state: unknown): void { const store = JsStore.openOrCreate({ path: storePath }); diff --git a/test/code-execution.test.ts b/test/code-execution.test.ts index db3ae45a5..1a5b844f7 100644 --- a/test/code-execution.test.ts +++ b/test/code-execution.test.ts @@ -1339,6 +1339,51 @@ describe('framework background scripts + spill (real python3)', () => { } }); + it('a dead script\'s last output is cut at 2000 units without halving a surrogate pair', async () => { + // 1500 emoji, then the traceback. Whichever parity the cut 2000 units + // from the end has, one of the two runs (with or without one more unit + // after the emoji) puts it on a pair's second half. + for (const after of ['', 'x']) { + const { tempDir, storePath } = tempStorePath('pytc-bgpair-'); + const membrane = new MockMembrane(); + membrane.pushResponse(createMockResponse([{ type: 'text', text: 'noted' }])); + const framework = await createBgFramework(storePath, membrane, {}); + try { + framework.start(); + const started = await framework.executeToolCall({ + id: 'c1', + name: 'code_execution', + input: { + code: `import asyncio\nprint("\\U0001F600" * 1500 + "${after}", flush=True)\nawait asyncio.sleep(0.1)\nraise ValueError("done")`, + background: true, + }, + callerAgentName: 'prime', + }); + assert.strictEqual(started.success, true, JSON.stringify(started)); + + const deadline = Date.now() + 20_000; + let crashMessage: { content: ContentBlockLike[] } | null = null; + while (Date.now() < deadline && !crashMessage) { + await new Promise((r) => setTimeout(r, 100)); + const cm = framework.getAgent('prime')?.getContextManager(); + const msgs = (cm?.queryMessages({}).messages ?? []) as unknown as Array<{ content: ContentBlockLike[]; metadata?: { source?: string } }>; + crashMessage = msgs.find((m) => m.metadata?.source === 'background-script') ?? null; + } + assert.ok(crashMessage, 'crash wake should be injected'); + const text = (crashMessage.content[0] as { text?: string }).text ?? ''; + // A halved pair would be stored as U+FFFD, or kept as a lone surrogate. + const last = text.split('Last output:\n')[1] ?? ''; + assert.ok(last.startsWith('\u{1F600}'), + `the last output starts on a whole emoji (after ${JSON.stringify(after)}): ${JSON.stringify(last.slice(0, 4))}`); + assert.ok(!text.includes('\ufffd') && !/[\ud800-\udbff](?![\udc00-\udfff])|(? { const { tempDir, storePath } = tempStorePath('pytc-spill-'); const mountDir = join(tempDir, 'mount'); diff --git a/test/event-gate.test.ts b/test/event-gate.test.ts index 70d6eb817..5c88a6a04 100644 --- a/test/event-gate.test.ts +++ b/test/event-gate.test.ts @@ -409,6 +409,26 @@ describe('debounce', () => { assert.doesNotMatch(text, /##/); }); + it('a quoted event\'s content is cut where no surrogate pair is halved', (t) => { + t.mock.timers.enable({ apis: ['setTimeout'] }); + const path = writeConfig('debounce-pair.json', { + policies: [ + { name: 'files', match: { scope: ['workspace:file-changed'] }, behavior: { debounce: 100 } }, + ], + default: 'skip', + }); + const { gate, messages } = makeGate(path); + + // 199 units, then an emoji whose two units straddle the 200-unit cut. + gate.evaluate(event({ eventType: 'workspace:file-changed', content: `${'a'.repeat(199)}😀 and on` })); + t.mock.timers.tick(100); + + assert.strictEqual(messages.length, 1); + const text = (messages[0].content as Array<{ text: string }>)[0].text; + assert.ok(text.includes(`: ${'a'.repeat(199)}...`), `the cut leaves the emoji out whole: ${JSON.stringify(text)}`); + assert.ok(!/[\ud800-\udbff](?![\udc00-\udfff])|(? { const path = writeConfig('debounce-provenance.json', { policies: [ diff --git a/test/health-snapshot.test.ts b/test/health-snapshot.test.ts index 619f0f8c7..82a9314af 100644 --- a/test/health-snapshot.test.ts +++ b/test/health-snapshot.test.ts @@ -96,3 +96,21 @@ describe('HealthModule snapshot timestamp resilience', () => { ); }); }); + +describe('HealthModule snapshot error text', () => { + it('cuts a recent error at 400 units without halving a surrogate pair', async () => { + // 399 units, then an emoji whose two units straddle the 400-unit cut. + const error = `${'e'.repeat(399)}😀 and on`; + const entries = [ + { sequence: 0, entry: { timestamp: 1751900000000, agentName: 'a', requestId: 'r0', success: false, error, request: {}, durationMs: 1 } }, + ]; + const health = new HealthModule({ timeZone: 'UTC' }); + health.bind(frameworkStub(entries), {} as JsStore); + + const result = await health.handleToolCall(snapshotCall()); + + assert.equal(result.success, true, `snapshot failed: ${result.error}`); + const shown = (result.data as { inferences: { recentErrors: Array<{ error: string }> } }).inferences.recentErrors[0]!.error; + assert.equal(shown, 'e'.repeat(399), 'the cut leaves the emoji out whole'); + }); +}); diff --git a/test/stop-with-work-in-flight.test.ts b/test/stop-with-work-in-flight.test.ts index ac513baf2..3de37e67c 100644 --- a/test/stop-with-work-in-flight.test.ts +++ b/test/stop-with-work-in-flight.test.ts @@ -871,6 +871,35 @@ describe('stop() with work still in flight', () => { await framework.stop(); } }); + + it('a rejection over 500 characters is cut there, counting code points, without halving a surrogate pair', async () => { + const { framework, settle, pushed } = await withTuneOut(); + try { + // 499 characters, then an emoji whose two UTF-16 units straddle unit + // 500. Counted in code points, the emoji is the 500th character. + const message = `${'x'.repeat(499)}😀 and more`; + settle().reject(new Error(message)); + await until(() => pushed.length > 0, 'the failed result'); + const result = (pushed[0] as unknown as { result: ToolResult }).result; + assert.equal(result.error, `${'x'.repeat(499)}😀… (509 characters)`, 'the cut keeps the emoji whole, and the count is of code points'); + } finally { + await framework.stop(); + } + }); + + it('a rejection of 500 characters or fewer comes back whole, though it is longer than 500 UTF-16 units', async () => { + const { framework, settle, pushed } = await withTuneOut(); + try { + // 300 emoji: 600 UTF-16 units, 300 characters. + const message = '😀'.repeat(300); + settle().reject(new Error(message)); + await until(() => pushed.length > 0, 'the failed result'); + const result = (pushed[0] as unknown as { result: ToolResult }).result; + assert.equal(result.error, message, 'within 500 characters, nothing is cut and no note is added'); + } finally { + await framework.stop(); + } + }); }); it('every tool result the framework produces names its tool, and the call it answers', () => { diff --git a/test/tool-result-spill.test.ts b/test/tool-result-spill.test.ts index 87444c5cd..8c8336079 100644 --- a/test/tool-result-spill.test.ts +++ b/test/tool-result-spill.test.ts @@ -526,6 +526,31 @@ describe('tool-result spill completion (issue #89)', () => { } }); + it('cuts a failed spill\'s reason at 200 units without halving a surrogate pair', async () => { + // 199 units, then an emoji whose two units straddle the 200-unit cut. + const reason = `${'r'.repeat(199)}😀 and the rest of the reason`; + const proto = WorkspaceModule.prototype as unknown as { writeBinary: (...args: unknown[]) => Promise }; + const writeBinary = proto.writeBinary; + proto.writeBinary = async () => ({ success: false, error: reason }); + let h: SpillHarness | undefined; + try { + h = await startSpillTurn({ + prefix: 'spill-pair-', + result: { success: true, data: { blob: 'w'.repeat(60_000) } }, + withWorkspace: true, + }); + const stored = await waitForStoredToolResult(h.framework); + assert.ok(stored, 'tool result should be stored'); + assert.ok(stored.content.includes(`FAILED (${'r'.repeat(199)})`), `the cut leaves the emoji out whole: ${stored.content.slice(-400)}`); + } finally { + proto.writeBinary = writeBinary; + if (h) { + await h.framework.stop(); + rmSync(h.tempDir, { recursive: true, force: true }); + } + } + }); + it('clamps the default down to the strategy bound and reports it', async () => { // maxMessageTokens=1000 → strategy bound 4000 < default 24000. This branch // decides the cap for every resident on a bounded strategy — pin it. diff --git a/test/utils-surface.test.ts b/test/utils-surface.test.ts index e44a46e12..4ae15890d 100644 --- a/test/utils-surface.test.ts +++ b/test/utils-surface.test.ts @@ -121,6 +121,27 @@ describe('utils surface', () => { } finally { await framework.stop(); rmSync(dir, { recursive: true, force: true }); } }); + it('lists a first line over 160 units cut there, without halving a surrogate pair', async () => { + const long = { + name: 'long', + async start(): Promise {}, + async stop(): Promise {}, + getTools: (): ToolDefinition[] => [], + // 159 units, then an emoji whose two units straddle the 160-unit cut. + getUtilities: (): ToolDefinition[] => [ + { name: 'op', description: `${'d'.repeat(159)}😀 and the rest of a long first line`, inputSchema: { type: 'object' } }, + ], + handleToolCall: async (): Promise => ({ success: true }), + onProcess: async (): Promise => ({}), + } as unknown as Module; + const { framework, dir } = await makeFramework([long]); + try { + const list = await framework.executeToolCall({ id: 'c1', name: 'utils', input: { action: 'list' }, callerAgentName: 'agent' }); + assert.ok(list.success, list.error); + assert.deepEqual(list.data, [{ name: 'long--op', description: 'd'.repeat(159) }], 'the cut leaves the emoji out whole'); + } finally { await framework.stop(); rmSync(dir, { recursive: true, force: true }); } + }); + it('arg misses teach by bounce: schema rides the error', async () => { const stub = new StubModule(); const { framework, dir } = await makeFramework([stub]);