From 3f253e3e835ce50c0ceb1e28117ea91ce21f0362 Mon Sep 17 00:00:00 2001 From: Crispin-2713 Date: Thu, 22 Oct 2026 17:40:05 -0400 Subject: [PATCH 1/2] fix: what reaches a request without the context store is made well formed where it leaves for it A request carries only text the context store would keep. What it compiles comes from the store, which writes a lone UTF-16 surrogate as U+FFFD, and a provider refuses a request carrying one: by a live call, 400 "The request body is not valid JSON: unexpected end of hex escape" (#695). Two things reach a request without the store, and a lone half in either went out raw: - Context injections, a module's or a hook's, in every request while they're injected, text and namespace alike (context-manager names a beforeUser or afterUser injection's message system_context:). They're now made well formed where an Agent compiles them into a request: assembleActivationRequest, for a stream and the preview, ahead of its layout hash, and runInferenceWithInjections, for the direct API (injectionsAsStored). A namespace that isn't a string, which the hook orchestrator keeps when it can be read as text, is taken by its text form. Scoping to a conversation still runs first, by the namespace as given, since channel ids are compared as given. - The copies a tool boundary injects into the live round: each heard message's sender's name, its text where the strategy can't render it, and the boundary's own left-out and re-pin notices. The store mends each when it stores it, but the live copies went as built. They now take their name and text as the store keeps them as they go to provideToolResults (nameAsStored, textAsStored). The boundary's gate, which keeps a message named as the agent out of the live round, compares the name as sent. A name that isn't a string passes as it did, since thrown at the boundary it would keep the round's results from the stream. The helpers and the rule are in src/text-as-stored.ts. Tool results are #699's, a server's tool definitions are #700's, and inference a server starts itself is left as it is. Tests, each red on main at its exit: injection-well-formed (hook injections of each shape, namespaces as a string and as an array, a module's beforeUser and system injection, and injections given straight to an Agent), mid-turn-injection-as-compiled (a sender's name, unrendered text, the left-out notice, the gate, and a name that isn't a string), present-while-acting (the re-pin notice), and conversation-fork-injection-scoping (scoping by the namespace as given). Co-Authored-By: Claude Opus 5.5 --- changelog.d/request-text-as-stored.fixed.md | 1 + src/agent.ts | 6 + src/framework.ts | 11 +- src/text-as-stored.ts | 49 +++++ ...onversation-fork-injection-scoping.test.ts | 24 +++ test/fixtures/hook-injection-mcpl-server.mjs | 54 +++++ test/injection-well-formed.test.ts | 191 ++++++++++++++++++ test/mid-turn-injection-as-compiled.test.ts | 56 ++++- test/present-while-acting.test.ts | 28 +++ 9 files changed, 414 insertions(+), 6 deletions(-) create mode 100644 changelog.d/request-text-as-stored.fixed.md create mode 100644 src/text-as-stored.ts create mode 100644 test/fixtures/hook-injection-mcpl-server.mjs create mode 100644 test/injection-well-formed.test.ts diff --git a/changelog.d/request-text-as-stored.fixed.md b/changelog.d/request-text-as-stored.fixed.md new file mode 100644 index 000000000..500fd6d26 --- /dev/null +++ b/changelog.d/request-text-as-stored.fixed.md @@ -0,0 +1 @@ +- What reaches a request without passing through the context store now carries a lone UTF-16 surrogate as U+FFFD, as the store keeps it (#695). That's a context injection's text and namespace, a hook's or a module's, and what a tool boundary injects into the live round: the messages it heard, with their senders' names, and its own notices. Before, every request an injection was in carried the lone half raw, and the API refuses such a request with a 400 ("The request body is not valid JSON: unexpected end of hex escape"). A namespace is still compared with channel ids as given when an injection is kept to its channel's conversation. diff --git a/src/agent.ts b/src/agent.ts index 8cc4fd4ac..744802b77 100644 --- a/src/agent.ts +++ b/src/agent.ts @@ -4,6 +4,7 @@ import { createHash } from 'node:crypto'; import type { CacheWireReceipt, KvUnifiedRequestHooks } from './kv-unified-wire.js'; import { ToolResultGuard } from './tool-result-guard.js'; import { withoutTokenEstimates } from './thinking-token-stamp.js'; +import { injectionsAsStored } from './text-as-stored.js'; import { assertBooleanSetting, holdsSettings } from './config-checks.js'; import { CONTINUE_ROW, @@ -1157,6 +1158,8 @@ export class Agent { if (this._state.status === 'inferring') { throw new Error(`Agent ${this.name} is already inferring`); } + // Unstored, so they leave for the request here (text-as-stored.ts). + injections = injectionsAsStored(injections); if (this._state.status === 'waiting_for_tools') { throw new Error(`Agent ${this.name} is waiting for tool results`); @@ -1332,6 +1335,9 @@ export class Agent { compressionTools: ToolDefinition[] = availableTools, options: ActivationRequestOptions = {}, ): Promise<{ request: NormalizedRequest; continuePrompt?: RequestOnlyPrompt }> { + // Unstored, so they leave for the request here (text-as-stored.ts), the + // layout hash below included. + injections = injectionsAsStored(injections); // Compression may revisit hidden tools in recorded history. Keep its full // definition set separate from the resident's advertised tools: the // autobiographical strategy must declare the same tools on its diff --git a/src/framework.ts b/src/framework.ts index 2673bc646..d0f41c6d3 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -157,6 +157,7 @@ import { bounded, renderList, renderReturn, VALUE_CHARS } from './code-execution import { splitProseRuns, withoutRoundsProse } from './prose-segments.js'; import { cumulativeDelta } from './usage-accounting.js'; import { stampThinkingTokenEstimates } from './thinking-token-stamp.js'; +import { nameAsStored, textAsStored } from './text-as-stored.js'; /** An MCPL tool call refused because its server isn't in the registry: one * closed for good, one removed, or one whose connect failed without @@ -929,7 +930,8 @@ function describeDeferredWrite(msg: { id: string; participant: string; content: function isInjectableMidTurn(msg: { participant: string; content: ContentBlock[] }, agentName: string): boolean { // Exact, as membrane's role test is: only a participant equal to the // request's assistantParticipant (Agent's name) renders as the assistant. - return msg.participant !== agentName + // It's the name as sent, which the boundary takes as the store keeps it. + return nameAsStored(msg.participant) !== agentName && !msg.content.some((b) => b.type === 'tool_use' || b.type === 'tool_result'); } @@ -12145,11 +12147,14 @@ export class AgentFramework { // 0.5.x, and from 0.6.0 declares it can't carry them, so none are // collected for it. Each goes as its participant and content // alone: a stored row's metadata is the host's record, not input - // for membrane. + // for membrane. Each goes with its name and text as the store + // keeps them, whether it's a heard message, rendered or not, or a + // notice the boundary made: this is where they leave for the + // request. currentState.stream.provideToolResults( agent.toolResultGuard.submissionResults(membraneResults), midTurnInjections.length > 0 - ? { injectedMessages: midTurnInjections.map(({ participant, content }) => ({ participant, content })) } + ? { injectedMessages: midTurnInjections.map(({ participant, content }) => ({ participant: nameAsStored(participant), content: textAsStored(content) })) } : undefined, ); // RFC-006 §3.3: consumption is determined at assembly, and this diff --git a/src/text-as-stored.ts b/src/text-as-stored.ts new file mode 100644 index 000000000..4758c21aa --- /dev/null +++ b/src/text-as-stored.ts @@ -0,0 +1,49 @@ +/** + * A request carries only text the context store would keep. What a request + * compiles comes from the store, which writes a lone UTF-16 surrogate as + * U+FFFD, and a provider refuses a request carrying one (#695). Two things + * reach a request without the store, and each is made well formed where it + * leaves for it: context injections, as an Agent compiles them into a + * request, and the copies a tool boundary injects into the live round, as + * they go to the stream. + */ + +import type { ContentBlock } from '@animalabs/membrane'; +import type { ContextInjection } from '@animalabs/context-manager'; + +/** Content with its text as the context store keeps it. A block that's + * already well formed keeps its identity. */ +export function textAsStored(content: ContentBlock[]): ContentBlock[] { + return content.map((b) => b.type === 'text' && typeof b.text === 'string' && !b.text.isWellFormed() + ? { ...b, text: b.text.toWellFormed() } + : b); +} + +/** A sender's name as the context store keeps it, and as a tool boundary + * sends it: a lone UTF-16 surrogate becomes U+FFFD. A name that isn't a + * string, from an untyped module, passes as it did: thrown here, inside the + * boundary, it would keep the round's results from the stream, where it + * would otherwise fail at the next compile. */ +export function nameAsStored(participant: string): string { + return typeof participant === 'string' ? participant.toWellFormed() : participant; +} + +/** Injections with their text as the context store would keep it: each one's + * content, and its namespace, which context-manager writes into a beforeUser + * or afterUser injection's participant name by its text form. A namespace + * that isn't a string, which MCPL §10.4 doesn't allow but the hook + * orchestrator keeps when it can be read as text, is taken by that form. + * They're scoped to a conversation before this, by the namespace as given, + * since channel ids are compared as given. One that's already well formed + * keeps its identity. */ +export function injectionsAsStored(injections: ContextInjection[] | undefined): ContextInjection[] | undefined { + return injections?.map((inj) => { + const ns: unknown = inj.namespace; + const named = typeof ns === 'string' || ns === undefined || ns === null ? ns : String(ns); + const namespace = typeof named === 'string' && !named.isWellFormed() ? named.toWellFormed() : ns; + const content = textAsStored(inj.content); + return namespace === ns && content.every((b, i) => b === inj.content[i]) + ? inj + : { ...inj, namespace: namespace as string, content }; + }); +} diff --git a/test/conversation-fork-injection-scoping.test.ts b/test/conversation-fork-injection-scoping.test.ts index a20790a2b..7d1225a1a 100644 --- a/test/conversation-fork-injection-scoping.test.ts +++ b/test/conversation-fork-injection-scoping.test.ts @@ -199,6 +199,30 @@ describe('conversation fork injection scoping', () => { assertContains(system, 'hook retrieval context'); }); + it('scopes by a namespace as given, so one holding a lone surrogate that names another channel stays out of the fork', async () => { + // The namespace is made well formed for the request only after scoping: + // channel ids are compared as given, and well formed first it would name + // no channel and reach every agent. + membrane.pushResponse(createMockResponse([])); + const other = 'chanB \ud83d'; + const framework = await makeFramework([ + systemInjection('module other context', other), + systemInjection('module home context', 'chanA'), + ], [FORK_AGENT]); + installChannelRegistry(framework, ['chanA', other]); + bindForkHome(framework); + installHookInjections(framework, [systemInjection('hook other context', other)]); + + framework.pushEvent(triggerEvent()); + await framework.runUntilIdle(); + + assert.equal(membrane.calls.length, 1); + const system = membrane.calls[0]!.system; + assertOmits(system, 'module other context'); + assertOmits(system, 'hook other context'); + assertContains(system, 'module home context'); + }); + it('passes module injections through for non-conversation agents', async () => { membrane.pushResponse(createMockResponse([])); const framework = await makeFramework([ diff --git a/test/fixtures/hook-injection-mcpl-server.mjs b/test/fixtures/hook-injection-mcpl-server.mjs new file mode 100644 index 000000000..d4a5d104c --- /dev/null +++ b/test/fixtures/hook-injection-mcpl-server.mjs @@ -0,0 +1,54 @@ +// MCPL fixture for hook-injection tests: answers context/beforeInference with +// one injection whose text is TEXT_JSON's, at POSITION ('beforeUser', 'afterUser' or +// 'system'), as a string when SHAPE is 'string', as a block whose type is the +// text when SHAPE is 'unknown-type', and as a text block otherwise. +// It lists one tool, `ready`, so a test can tell it's connected. +import { createInterface } from 'node:readline'; + +const position = process.env.POSITION || 'beforeUser'; +// TEXT_JSON is the text as JSON: an environment variable can't carry a lone +// UTF-16 surrogate, which the JSON escape can. +const text = process.env.TEXT_JSON ? JSON.parse(process.env.TEXT_JSON) : ''; +// NAMESPACE_JSON, the same way, is the injection's namespace; 'notes' without it. +const namespace = process.env.NAMESPACE_JSON ? JSON.parse(process.env.NAMESPACE_JSON) : 'notes'; +const send = (message) => process.stdout.write(JSON.stringify(message) + '\n'); +const reply = (id, result) => send({ jsonrpc: '2.0', id, result }); + +const rl = createInterface({ input: process.stdin }); +rl.on('close', () => process.exit(0)); +rl.on('line', (line) => { + let msg; + try { msg = JSON.parse(line); } catch { return; } + if (msg.method === 'initialize') { + reply(msg.id, { + protocolVersion: '2024-11-05', + capabilities: { + tools: {}, + experimental: { + mcpl: { + version: '0.5', + contextHooks: { beforeInference: { inject: { [position]: true } } }, + featureSets: { + notes: { description: 'injects a note', uses: [`contextHooks.beforeInference.inject.${position}`] }, + }, + }, + }, + }, + serverInfo: { name: 'hook-injection-fixture', version: '0.0.0' }, + }); + return; + } + if (msg.method === 'context/beforeInference') { + const content = process.env.SHAPE === 'string' ? text + : process.env.SHAPE === 'unknown-type' ? [{ type: text }] + : [{ type: 'text', text }]; + reply(msg.id, { featureSet: 'notes', contextInjections: [{ namespace, position, content }] }); + return; + } + if (msg.method === 'tools/list') { + // A tool the test waits for, which says the server is connected. + reply(msg.id, { tools: [{ name: 'ready', description: 'Says the fixture is connected.', inputSchema: { type: 'object', properties: {} } }] }); + return; + } + if (msg.id !== undefined && msg.id !== null && msg.method) reply(msg.id, {}); +}); diff --git a/test/injection-well-formed.test.ts b/test/injection-well-formed.test.ts new file mode 100644 index 000000000..9d9476a1f --- /dev/null +++ b/test/injection-well-formed.test.ts @@ -0,0 +1,191 @@ +/** + * A context injection, a hook's or a module's, isn't stored, so it goes out + * as given in every request while it's injected. A lone UTF-16 surrogate in + * its text or its namespace reaches the request as U+FFFD, as the context + * store would keep it: a provider refuses a request that carries a lone half + * (#695). + */ +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 { AgentFramework, type Module, type ProcessEvent } from '../src/index.js'; +import { MockMembrane, createMockResponse } from './helpers/mock-membrane.js'; + +const FIXTURE = join(import.meta.dirname, 'fixtures/hook-injection-mcpl-server.mjs'); +const LONE = 'HOOK cut \ud83d here'; +const MENDED = 'HOOK cut \ufffd here'; + +const dirs: string[] = []; +const frameworks: AgentFramework[] = []; +afterEach(async () => { + while (frameworks.length) await frameworks.pop()!.stop(); + while (dirs.length) rmSync(dirs.pop()!, { recursive: true, force: true }); +}); + +/** Turns an external message into a user message and asks for inference. */ +class Trigger implements Module { + readonly name = 'trigger'; + async start() {} + async stop() {} + getTools() { return []; } + async handleToolCall() { return { success: false, isError: true, error: 'no tools' }; } + async onProcess(event: ProcessEvent) { + return event.type === 'external-message' + ? { addMessages: [{ participant: 'user', content: [{ type: 'text' as const, text: 'hello' }] }], requestInference: true } + : {}; + } +} + +async function until(cond: () => boolean, what: string, ms = 15_000): Promise { + const deadline = Date.now() + ms; + while (!cond()) { + if (Date.now() > deadline) throw new Error(`timed out waiting for ${what}`); + await new Promise((r) => setTimeout(r, 25)); + } +} + +/** Every string under `value` that holds the hook's marker. */ +function hookTexts(value: unknown): string[] { + const found: string[] = []; + const walk = (v: unknown): void => { + if (typeof v === 'string') { if (v.includes('HOOK')) found.push(v); return; } + if (Array.isArray(v)) { v.forEach(walk); return; } + if (v && typeof v === 'object') Object.values(v).forEach(walk); + }; + walk(value); + return found; +} + +const SHAPES = { + string: 'a string', + blocks: 'a text block', + 'unknown-type': 'a block of a type this host can\'t show, named by it', +} as const; + +const CASES: Array<{ position: string; shape: keyof typeof SHAPES; grant?: string[]; inNamespace?: 'string' | 'array' }> = [ + { position: 'beforeUser', shape: 'blocks' }, + { position: 'beforeUser', shape: 'unknown-type' }, + { position: 'beforeUser', shape: 'string' }, + { position: 'system', shape: 'blocks', grant: ['tools', 'contextHooks.beforeInference.inject.system'] }, + { position: 'system', shape: 'string', grant: ['tools', 'contextHooks.beforeInference.inject.system'] }, + // context-manager names a beforeUser injection's message by its namespace. + { position: 'beforeUser', shape: 'blocks', inNamespace: 'string' }, + // One that isn't a string, which §10.4 doesn't allow, is read by its text form. + { position: 'beforeUser', shape: 'blocks', inNamespace: 'array' }, +]; + +for (const { position, shape, grant, inNamespace } of CASES) { + const where = inNamespace === 'array' ? "'s namespace given as an array" : inNamespace ? "'s namespace" : ` given as ${SHAPES[shape]}`; + test(`a lone surrogate in a ${position} hook injection${where} reaches the request as U+FFFD`, async () => { + const dir = mkdtempSync(join(tmpdir(), 'af-hook-well-formed-')); dirs.push(dir); + const membrane = new MockMembrane(); + membrane.pushResponse(createMockResponse([{ type: 'text', text: 'Hello.' }])); + const framework = await AgentFramework.create({ + storePath: join(dir, 'store'), + membrane: membrane.asMembrane(), + agents: [{ name: 'prime', model: 'test-model', systemPrompt: 'You are prime.' }], + modules: [new Trigger()], + mcplServers: [{ + id: 'notes', + command: process.execPath, + args: [FIXTURE], + env: { + POSITION: position, + SHAPE: shape, + ...(inNamespace + ? { TEXT_JSON: JSON.stringify('a note'), NAMESPACE_JSON: JSON.stringify(inNamespace === 'array' ? [LONE] : LONE) } + : { TEXT_JSON: JSON.stringify(LONE) }), + }, + ...(grant ? { enabledCapabilities: grant } : {}), + }], + syncIntervalMs: 0, + }); + frameworks.push(framework); + await framework.start(); + await until(() => framework.getAllTools().some((t) => t.name.endsWith('ready')), 'the fixture to connect'); + framework.pushEvent({ type: 'external-message', source: 'test', content: 'hello', metadata: {} }); + await until(() => membrane.calls.length >= 1, 'the request'); + const sent = hookTexts(membrane.calls[0]); + assert.ok(sent.length > 0, "the request carries the hook's injection"); + for (const t of sent) assert.ok(t.isWellFormed(), `the injection's text is well formed: ${JSON.stringify(t)}`); + assert.ok(sent.some((t) => t.includes(MENDED)), 'the lone half goes out as U+FFFD'); + if (inNamespace) { + assert.ok( + membrane.calls[0].messages.some((m) => m.participant === `system_context:${MENDED}`), + "the injection's message is named by its namespace, well formed", + ); + } + }); +} + +/** A Trigger that also gives one context injection, as a module's gatherContext does. */ +class Injecting implements Module { + readonly name = 'injecting'; + private readonly trigger = new Trigger(); + constructor(private readonly position: 'beforeUser' | 'system') {} + async start() {} + async stop() {} + getTools() { return []; } + async handleToolCall() { return { success: false, isError: true, error: 'no tools' }; } + onProcess(event: ProcessEvent) { return this.trigger.onProcess(event); } + async gatherContext() { + return [{ namespace: 'mod', position: this.position, content: [{ type: 'text' as const, text: LONE }] }]; + } +} + +for (const position of ['beforeUser', 'system'] as const) { + test(`a lone surrogate in a module's ${position} injection reaches the request as U+FFFD`, async () => { + const dir = mkdtempSync(join(tmpdir(), 'af-module-well-formed-')); dirs.push(dir); + const membrane = new MockMembrane(); + membrane.pushResponse(createMockResponse([{ type: 'text', text: 'Hello.' }])); + const framework = await AgentFramework.create({ + storePath: join(dir, 'store'), + membrane: membrane.asMembrane(), + agents: [{ name: 'prime', model: 'test-model', systemPrompt: 'You are prime.' }], + modules: [new Injecting(position)], + syncIntervalMs: 0, + }); + frameworks.push(framework); + await framework.start(); + framework.pushEvent({ type: 'external-message', source: 'test', content: 'hello', metadata: {} }); + await until(() => membrane.calls.length >= 1, 'the request'); + const sent = hookTexts(membrane.calls[0]); + assert.ok(sent.length > 0, "the request carries the module's injection"); + for (const t of sent) assert.ok(t.isWellFormed(), `the injection's text is well formed: ${JSON.stringify(t)}`); + assert.ok(sent.some((t) => t.includes(MENDED)), 'the lone half goes out as U+FFFD'); + }); +} + +test("a lone surrogate in an injection given straight to an Agent's inference reaches the request as U+FFFD", async () => { + const dir = mkdtempSync(join(tmpdir(), 'af-agent-well-formed-')); dirs.push(dir); + const membrane = new MockMembrane(); + const framework = await AgentFramework.create({ + storePath: join(dir, 'store'), + membrane: membrane.asMembrane(), + agents: [{ name: 'prime', model: 'test-model', systemPrompt: 'You are prime.' }], + modules: [], + syncIntervalMs: 0, + }); + frameworks.push(framework); + await framework.start(); + const agent = framework.getAgent('prime')!; + // The direct API sends through membrane.stream, which the mock's membrane doesn't have. + const requests: Array<{ messages: Array<{ participant: string }> }> = []; + (agent as unknown as { membrane: unknown }).membrane = { + stream: async (request: { messages: Array<{ participant: string }> }) => { + requests.push(request); + return createMockResponse([{ type: 'text', text: 'Hello.' }]); + }, + }; + await agent.runInferenceWithInjections([], [ + { namespace: LONE, position: 'beforeUser', content: [{ type: 'text', text: LONE }] }, + { namespace: 'sys', position: 'system', content: [{ type: 'text', text: LONE }] }, + ]); + assert.equal(requests.length, 1, 'the premise: one request went out'); + const sent = hookTexts(requests[0]); + assert.ok(sent.length > 0, 'the request carries the injections'); + for (const t of sent) assert.ok(t.isWellFormed(), `the injection's text is well formed: ${JSON.stringify(t)}`); + assert.ok(requests[0]!.messages.some((m) => m.participant === `system_context:${MENDED}`), 'its namespace too'); +}); diff --git a/test/mid-turn-injection-as-compiled.test.ts b/test/mid-turn-injection-as-compiled.test.ts index f09505a82..74f459195 100644 --- a/test/mid-turn-injection-as-compiled.test.ts +++ b/test/mid-turn-injection-as-compiled.test.ts @@ -40,6 +40,8 @@ class InterjectingModule implements Module { interjections: ContentBlock[][] = []; /** Metadata for the interjection at the same index, if any. */ interjectionMetadata: Array | undefined> = []; + /** Whom each interjection is from; the message that starts a turn is Antra's. */ + participant = 'Antra'; async start(_ctx: ModuleContext): Promise {} async stop(): Promise {} @@ -69,7 +71,7 @@ class InterjectingModule implements Module { const go = (event as { content?: unknown }).content === 'go'; return { addMessages: [{ - participant: 'Antra', + participant: blocks ? this.participant : 'Antra', content: blocks ?? [text(String((event as { content?: unknown }).content))], ...(rowMetadata ? { metadata: rowMetadata } : {}), }], @@ -95,11 +97,11 @@ describe('a message injected at a tool boundary', () => { rmSync(tempDir, { recursive: true, force: true }); }); - async function create(strategy: unknown, modules: Module[] = []): Promise { + async function create(strategy: unknown, modules: Module[] = [], agentName = 'assistant'): Promise { framework = await AgentFramework.create({ storePath: join(tempDir, 'test.chronicle'), membrane: membrane.asMembrane(), - agents: [{ name: 'assistant', model: 'test-model', systemPrompt: 'You are a robot pilot.', strategy: strategy as never }], + agents: [{ name: agentName, model: 'test-model', systemPrompt: 'You are a robot pilot.', strategy: strategy as never }], modules: [module, ...modules], }); module.framework = framework; @@ -196,6 +198,18 @@ describe('a message injected at a tool boundary', () => { assert.deepEqual(JSON.parse((await nextRequest())('[not shown]')), [notice]); }); + it("names a left-out message's sender in its notice as the store keeps the name, a lone surrogate as U+FFFD", async () => { + await create(new WindowedPassthroughStrategy({})); + membrane.supportsInjectedMessages = true; + renderWith((ids, real) => real(ids)!.map((c, i) => (i === 0 ? null : c))); + module.participant = 'An \ud83d tra'; + const injected = await turnWith([[text('LEFTOUT')], [text('HEARD')]]); + const notice = JSON.parse(holding(injected, '[not shown]') || '[]') as Array<{ text?: string }>; + assert.ok(notice[0]?.text, 'the premise: the boundary injected its notice'); + assert.ok(notice[0]!.text!.isWellFormed() && notice[0]!.text!.includes('From An \ufffd tra'), `the notice names the sender well formed: ${JSON.stringify(notice[0]!.text)}`); + assert.deepEqual(JSON.parse((await nextRequest())('[not shown]')), notice, 'as the next request sends it'); + }); + it('tells the agent of a boundary whose whole batch is left out, with each one\'s images, and names no message a tune-out holds', async () => { await create(new WindowedPassthroughStrategy({})); membrane.supportsInjectedMessages = true; @@ -277,6 +291,42 @@ describe('a message injected at a tool boundary', () => { assert.deepEqual(JSON.parse(holding(injected, 'LONGMSG')), [text(long)]); }); + it("goes in with its sender's name as the store keeps it, a lone surrogate as U+FFFD", async () => { + await create(new WindowedPassthroughStrategy({ maxMessageTokens: 50 })); + module.participant = 'An \ud83d tra'; + const injected = await turnWith([[text('NAMEDMSG')]]); + const live = injected.find((m) => JSON.stringify(m.content).includes('NAMEDMSG')); + assert.equal(live?.participant, 'An \ufffd tra', 'the live round names its sender well formed'); + membrane.pushResponse(createMockResponse([text('Again.')])); + go(); + await framework.runUntilIdle(); + const stored = membrane.calls[membrane.calls.length - 1]!.messages.find((m) => JSON.stringify(m.content).includes('NAMEDMSG')); + assert.equal(live?.participant, stored?.participant, 'as the next request names it'); + }); + + it("isn't injected where its sender's name, as the store keeps it, is the agent's own", async () => { + // It would render as the agent's own turn mid-round, a prefill. + await create(new WindowedPassthroughStrategy({ maxMessageTokens: 50 }), [], 'Robo \ufffd'); + module.participant = 'Robo \ud83d'; + const injected = await turnWith([[text('SELFMSG')]]); + assert.equal(holding(injected, 'SELFMSG'), '', 'the live round leaves it out'); + }); + + it("gives the round's results to the stream when a queued sender's name isn't a string", { timeout: 20_000 }, async () => { + await create(new WindowedPassthroughStrategy({ maxMessageTokens: 50 })); + module.participant = 42 as unknown as string; + await Promise.race([turnWith([[text('NUMMSG')]]), new Promise((r) => setTimeout(r, 10_000))]); + assert.ok((membrane.lastStream?.receivedToolResults ?? []).length > 0, "the round's results reach the stream"); + }); + + it('goes in with its text as the store keeps it where the strategy has no render, a lone surrogate as U+FFFD', async () => { + await create(new WindowedPassthroughStrategy({ maxMessageTokens: 50 })); + renderWith(() => undefined); + const injected = await turnWith([[text('LONEMSG cut \ud83d here')]]); + assert.deepEqual(JSON.parse(holding(injected, 'LONEMSG')), [text('LONEMSG cut \ufffd here')]); + assert.equal(JSON.parse((await nextRequest())('LONEMSG'))[0].text, 'LONEMSG cut \ufffd here', 'as the next request sends it'); + }); + it('goes in as stored where the render throws, which is logged', async () => { await create(new WindowedPassthroughStrategy({ maxMessageTokens: 50 })); renderWith(() => { throw new Error('render broke'); }); diff --git a/test/present-while-acting.test.ts b/test/present-while-acting.test.ts index 87530eb63..ac170d7d8 100644 --- a/test/present-while-acting.test.ts +++ b/test/present-while-acting.test.ts @@ -353,6 +353,34 @@ describe('present while acting', () => { await framework.stop(); }); + it("injects a re-pin notice with its channel's id as the store keeps it, a lone surrogate as U+FFFD", async () => { + // The notice is built from the channel's id as the server gave it, and + // stored, which mends it; the live round sends it as the store keeps it. + membrane.pushResponse(createMockResponse([ + { type: 'tool_use', id: 'c1', name: 'robot--move', input: { dir: 'north' } }, + ] as ContentBlock[], 'tool_use')); + membrane.pushResponse(createMockResponse([{ type: 'text', text: 'Answering.' }] as ContentBlock[])); + + const framework = await createFramework(); + stubChannelRegistry(framework); + module.toolDelayMs = 25; + module.interjection = 'hey, over here?'; + module.interjectionMetadata = { channelId: 'discord:dm:an\ud83d', tags: ['chat:addressed'] }; + + trigger(framework); + await framework.runUntilIdle(); + + const wired = membrane.lastStream!.receivedToolResultOptions + .flatMap((o) => o?.injectedMessages ?? []) + .flatMap((m) => (m.content as Array<{ text?: string }>).map((b) => b.text ?? '')) + .filter((t) => t.includes('[routing]')); + assert.equal(wired.length, 1, 'the premise: the re-pin notice was injected'); + assert.ok(wired[0]!.isWellFormed(), `the notice is well formed: ${JSON.stringify(wired[0])}`); + assert.ok(wired[0]!.includes('discord:dm:an\ufffd'), 'its channel id holds U+FFFD'); + + await framework.stop(); + }); + it('an ADDRESSED mid-turn injection re-pins the locus for subsequent prose', async () => { // 2026-07-31 Mythos misroutes (n=6): someone explicitly addresses the // agent from another channel mid-turn (mention / reply / DM); the From 89deb471c580ea5cd2adac2f4e37a432f9995e33 Mon Sep 17 00:00:00 2001 From: Crispin-2713 Date: Thu, 22 Oct 2026 18:04:20 -0400 Subject: [PATCH 2/2] fix: every string in content that reaches a request without the store is made well formed RevEngine's review of #705 (medium): contentAsStored, which was textAsStored, mended only a text block's text, so a lone surrogate in another string of injected content, such as an image's link, still went out raw, where the store keeps it as U+FFFD (by run on main: an image link "https://example.com/a\ud83d.png" is stored and compiled as "a\ufffd.png"). It now mends every string, through arrays and plain objects, keeping the identity of what's already well formed. Test: a module's injected image whose link holds the lone half. Also from that review: the rule's header names the agent's own configuration (name, system prompt) as the one other source of a request's text; an afterUser injection's namespace has a case; MockMembrane gains stream, the direct API's call, so the direct-API test no longer replaces the Agent's private membrane; and the non-string sender test says why its wait is bounded and why it passes on main. Co-Authored-By: Claude Opus 5.5 --- src/framework.ts | 4 +- src/text-as-stored.ts | 50 +++++++++++++++------ test/helpers/mock-membrane.ts | 5 +++ test/injection-well-formed.test.ts | 47 ++++++++++++------- test/mid-turn-injection-as-compiled.test.ts | 3 ++ 5 files changed, 79 insertions(+), 30 deletions(-) diff --git a/src/framework.ts b/src/framework.ts index d0f41c6d3..53f989377 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -157,7 +157,7 @@ import { bounded, renderList, renderReturn, VALUE_CHARS } from './code-execution import { splitProseRuns, withoutRoundsProse } from './prose-segments.js'; import { cumulativeDelta } from './usage-accounting.js'; import { stampThinkingTokenEstimates } from './thinking-token-stamp.js'; -import { nameAsStored, textAsStored } from './text-as-stored.js'; +import { contentAsStored, nameAsStored } from './text-as-stored.js'; /** An MCPL tool call refused because its server isn't in the registry: one * closed for good, one removed, or one whose connect failed without @@ -12154,7 +12154,7 @@ export class AgentFramework { currentState.stream.provideToolResults( agent.toolResultGuard.submissionResults(membraneResults), midTurnInjections.length > 0 - ? { injectedMessages: midTurnInjections.map(({ participant, content }) => ({ participant: nameAsStored(participant), content: textAsStored(content) })) } + ? { injectedMessages: midTurnInjections.map(({ participant, content }) => ({ participant: nameAsStored(participant), content: contentAsStored(content) })) } : undefined, ); // RFC-006 §3.3: consumption is determined at assembly, and this diff --git a/src/text-as-stored.ts b/src/text-as-stored.ts index 4758c21aa..444c72aaa 100644 --- a/src/text-as-stored.ts +++ b/src/text-as-stored.ts @@ -1,22 +1,46 @@ /** - * A request carries only text the context store would keep. What a request - * compiles comes from the store, which writes a lone UTF-16 surrogate as - * U+FFFD, and a provider refuses a request carrying one (#695). Two things - * reach a request without the store, and each is made well formed where it - * leaves for it: context injections, as an Agent compiles them into a - * request, and the copies a tool boundary injects into the live round, as + * A request carries only text the context store would keep. Apart from the + * agent's own configuration, such as its name and system prompt, what a + * request compiles comes from the store, which writes a lone UTF-16 + * surrogate as U+FFFD, and a provider refuses a request carrying one (#695). + * Two things reach a request without the store, and each is made well formed + * where it leaves for it: context injections, as an Agent compiles them into + * a request, and the copies a tool boundary injects into the live round, as * they go to the stream. */ import type { ContentBlock } from '@animalabs/membrane'; import type { ContextInjection } from '@animalabs/context-manager'; -/** Content with its text as the context store keeps it. A block that's - * already well formed keeps its identity. */ -export function textAsStored(content: ContentBlock[]): ContentBlock[] { - return content.map((b) => b.type === 'text' && typeof b.text === 'string' && !b.text.isWellFormed() - ? { ...b, text: b.text.toWellFormed() } - : b); +/** A value with every string in it as the context store keeps it, through + * arrays and plain objects. Keys, and objects that aren't plain, pass as + * they are: content blocks are plain JSON data. Whatever is already well + * formed keeps its identity. */ +function stringsAsStored(value: T): T { + if (typeof value === 'string') return (value.isWellFormed() ? value : value.toWellFormed()) as T; + if (Array.isArray(value)) { + const out = value.map(stringsAsStored); + return (out.every((v, i) => v === value[i]) ? value : out) as T; + } + if (value !== null && typeof value === 'object') { + const proto = Object.getPrototypeOf(value); + if (proto !== Object.prototype && proto !== null) return value; + let changed = false; + const out: Record = {}; + for (const [k, v] of Object.entries(value)) { + const s = stringsAsStored(v); + if (s !== v) changed = true; + out[k] = s; + } + return (changed ? out : value) as T; + } + return value; +} + +/** Content with every string in it well formed, as the context store keeps + * a text block's text or an image's link. */ +export function contentAsStored(content: ContentBlock[]): ContentBlock[] { + return stringsAsStored(content); } /** A sender's name as the context store keeps it, and as a tool boundary @@ -41,7 +65,7 @@ export function injectionsAsStored(injections: ContextInjection[] | undefined): const ns: unknown = inj.namespace; const named = typeof ns === 'string' || ns === undefined || ns === null ? ns : String(ns); const namespace = typeof named === 'string' && !named.isWellFormed() ? named.toWellFormed() : ns; - const content = textAsStored(inj.content); + const content = contentAsStored(inj.content); return namespace === ns && content.every((b, i) => b === inj.content[i]) ? inj : { ...inj, namespace: namespace as string, content }; diff --git a/test/helpers/mock-membrane.ts b/test/helpers/mock-membrane.ts index d660cd139..e3d282b3e 100644 --- a/test/helpers/mock-membrane.ts +++ b/test/helpers/mock-membrane.ts @@ -211,6 +211,11 @@ export class MockMembrane { return this.responses[this.responseIndex++]; } + /** The direct Agent API's call, recorded and answered as complete is. */ + async stream(request: NormalizedRequest, _options?: unknown): Promise { + return this.complete(request); + } + streamYielding(request: NormalizedRequest, _options?: unknown): YieldingStream { this.calls.push(request); const remaining = this.responses.slice(this.responseIndex); diff --git a/test/injection-well-formed.test.ts b/test/injection-well-formed.test.ts index 9d9476a1f..e16a200b3 100644 --- a/test/injection-well-formed.test.ts +++ b/test/injection-well-formed.test.ts @@ -72,6 +72,7 @@ const CASES: Array<{ position: string; shape: keyof typeof SHAPES; grant?: strin { position: 'system', shape: 'string', grant: ['tools', 'contextHooks.beforeInference.inject.system'] }, // context-manager names a beforeUser injection's message by its namespace. { position: 'beforeUser', shape: 'blocks', inNamespace: 'string' }, + { position: 'afterUser', shape: 'blocks', inNamespace: 'string' }, // One that isn't a string, which §10.4 doesn't allow, is read by its text form. { position: 'beforeUser', shape: 'blocks', inNamespace: 'array' }, ]; @@ -124,14 +125,17 @@ for (const { position, shape, grant, inNamespace } of CASES) { class Injecting implements Module { readonly name = 'injecting'; private readonly trigger = new Trigger(); - constructor(private readonly position: 'beforeUser' | 'system') {} + constructor( + private readonly position: 'beforeUser' | 'system', + private readonly content: Array> = [{ type: 'text', text: LONE }], + ) {} async start() {} async stop() {} getTools() { return []; } async handleToolCall() { return { success: false, isError: true, error: 'no tools' }; } onProcess(event: ProcessEvent) { return this.trigger.onProcess(event); } async gatherContext() { - return [{ namespace: 'mod', position: this.position, content: [{ type: 'text' as const, text: LONE }] }]; + return [{ namespace: 'mod', position: this.position, content: this.content as never }]; } } @@ -170,22 +174,35 @@ test("a lone surrogate in an injection given straight to an Agent's inference re }); frameworks.push(framework); await framework.start(); - const agent = framework.getAgent('prime')!; - // The direct API sends through membrane.stream, which the mock's membrane doesn't have. - const requests: Array<{ messages: Array<{ participant: string }> }> = []; - (agent as unknown as { membrane: unknown }).membrane = { - stream: async (request: { messages: Array<{ participant: string }> }) => { - requests.push(request); - return createMockResponse([{ type: 'text', text: 'Hello.' }]); - }, - }; - await agent.runInferenceWithInjections([], [ + membrane.pushResponse(createMockResponse([{ type: 'text', text: 'Hello.' }])); + await framework.getAgent('prime')!.runInferenceWithInjections([], [ { namespace: LONE, position: 'beforeUser', content: [{ type: 'text', text: LONE }] }, { namespace: 'sys', position: 'system', content: [{ type: 'text', text: LONE }] }, ]); - assert.equal(requests.length, 1, 'the premise: one request went out'); - const sent = hookTexts(requests[0]); + assert.equal(membrane.calls.length, 1, 'the premise: one request went out'); + const sent = hookTexts(membrane.calls[0]); assert.ok(sent.length > 0, 'the request carries the injections'); for (const t of sent) assert.ok(t.isWellFormed(), `the injection's text is well formed: ${JSON.stringify(t)}`); - assert.ok(requests[0]!.messages.some((m) => m.participant === `system_context:${MENDED}`), 'its namespace too'); + assert.ok(membrane.calls[0]!.messages.some((m) => m.participant === `system_context:${MENDED}`), 'its namespace too'); +}); + +test("a lone surrogate in a module's injected image link reaches the request as U+FFFD", async () => { + const dir = mkdtempSync(join(tmpdir(), 'af-module-image-well-formed-')); dirs.push(dir); + const membrane = new MockMembrane(); + membrane.pushResponse(createMockResponse([{ type: 'text', text: 'Hello.' }])); + const framework = await AgentFramework.create({ + storePath: join(dir, 'store'), + membrane: membrane.asMembrane(), + agents: [{ name: 'prime', model: 'test-model', systemPrompt: 'You are prime.' }], + modules: [new Injecting('beforeUser', [{ type: 'image', source: { type: 'url', url: 'https://example.com/HOOK\ud83d.png' } }])], + syncIntervalMs: 0, + }); + frameworks.push(framework); + await framework.start(); + framework.pushEvent({ type: 'external-message', source: 'test', content: 'hello', metadata: {} }); + await until(() => membrane.calls.length >= 1, 'the request'); + const sent = hookTexts(membrane.calls[0]); + assert.ok(sent.length > 0, "the request carries the module's image"); + for (const t of sent) assert.ok(t.isWellFormed(), `the image's link is well formed: ${JSON.stringify(t)}`); + assert.ok(sent.includes('https://example.com/HOOK\ufffd.png'), 'the lone half goes out as U+FFFD'); }); diff --git a/test/mid-turn-injection-as-compiled.test.ts b/test/mid-turn-injection-as-compiled.test.ts index 74f459195..81f757809 100644 --- a/test/mid-turn-injection-as-compiled.test.ts +++ b/test/mid-turn-injection-as-compiled.test.ts @@ -315,6 +315,9 @@ describe('a message injected at a tool boundary', () => { it("gives the round's results to the stream when a queued sender's name isn't a string", { timeout: 20_000 }, async () => { await create(new WindowedPassthroughStrategy({ maxMessageTokens: 50 })); module.participant = 42 as unknown as string; + // A turn whose results never reach the stream never ends, so the wait is + // bounded and the assertion says what happened. On main it passes: this + // pins that the boundary's name step keeps main's handling. await Promise.race([turnWith([[text('NUMMSG')]]), new Promise((r) => setTimeout(r, 10_000))]); assert.ok((membrane.lastStream?.receivedToolResults ?? []).length > 0, "the round's results reach the stream"); });