From 33a7a43910ca8c3f71a553dce319571abd945c51 Mon Sep 17 00:00:00 2001 From: Ada-1017 Date: Tue, 13 Oct 2026 00:32:09 -0400 Subject: [PATCH 1/2] fix(tools): a module's tool result that can't be stored becomes an error naming the tool (#309) A module runs in the host's process, so its handleToolCall can return any value: a cyclic object, a BigInt, a toJSON that throws, no result object at all, or an object whose getters throw or answer differently each time. The first code to serialize it threw: buildStoredToolResultContent for a model's call, puppetToolCall's storing, the tool-result guard's audit record, a trace's broadcast. In the event loop that error is only logged, so the agent's turn never settled and stop() didn't return (#309). A result with no data threw the same way: JSON.stringify gives undefined, and the inline cap's length check threw on it, so { success: true } alone left the turn unsettled on main 8e288001 (probed). Every release since b6d96a63's default inline cap (v0.9.0 through v0.21.0) has that check. - One check where the framework stores or sends a module's result: ModuleRegistry.storableToolCall, new, for a model's call, a script's inner call and the utils meta-tool for a model; the read_image tool, which calls the workspace module's handler directly; and the pair puppetToolCall stores (checkedToolResult). The check reads the result once, inside one try, and never throws: each field consumers read (success, isError, endTurn, error, data) by name, so a result class's getters count, and any other own field except a toJSON, since consumers never call a toJSON of the result. The flags become booleans, read for their truth, and error its text form, before anything is serialized. What's returned is the JSON form of those fields, parsed, so every consumer holds plain data and no module code runs again. - What can't be used becomes "Tool 'NAME' returned a result that can't be converted to JSON, so it can't be stored or sent (REASON). The call itself completed.", or "can't be read" when reading it threw. It keeps the module's own error text (", with this error: ...") and endTurn, both read before the data, so a data getter that throws doesn't cost them. A value that isn't a result object (undefined, null, an array, a string, an Error returned rather than thrown), and a non-error result's data that is a function or a symbol, are answered the same way. The host's log gets the same line, prefixed "[tools] agent A:" when an agent made the call. - In-process callers keep main's contract. ModuleRegistry.handleToolCall (exported), ModuleContext.callTool and executeToolCall, for a module's tool or a utility through the utils meta-tool, return the module's own result, unchecked. puppetToolCall returns the tool's own result too, or the error result it stored when nothing usable could be stored, where it used to reject. A module's throw is answered as an error result, as before. - A module's error as text, always a string or none: a string as it is, an Error's message read the same way (a module's message isn't always a string), an object's own message, any other value's JSON, a BigInt's or a symbol's String(), or "an error value with no text form". One that says nothing (null, false, empty) is left out, so consumers say "Unknown error", and a throw with no text has no error text. A thrown value is described as a string inside a try; one String() can't describe (an object with no prototype) is named "a value with no text form". - No data: toolResultDataToHistoryString returns '' for data JSON leaves out, against its declared string return where it gave undefined. nonErrorResultText gives a non-error result's text: its data, or, with no data (undefined or null), its error text, so a failure a tool doesn't flag as an error for the model ({ success: false, error, isError: false }) still says what failed. The stored history and its wire copy, the agent's own path and a script's result use it; the image inventory keeps the data-only reading, since it only counts image placeholders. The script path caps that text as it caps data. Cost: one serialization and one parse of each module result on a path that stores or sends it. Tests (test/unconvertible-tool-result.test.ts, nine): one round of thirty-nine tools covering each kind of result above, each read as expected and each unusable one logged; two results that end the turn (BigInt data, and a data getter that throws after endTurn is read); read_image's handler returning a cyclic result and throwing a value with no prototype; a script reading quiet failures, an unusable result and a throwing getter; the model's utils call on a cyclic utility; puppetToolCall storing and returning the error for an unusable result, and returning a Date as a Date while storing its string; in-process callers (executeToolCall, ModuleContext.callTool, a utility through either) getting a Date as a Date, flags as returned, a getter unread and a cyclic result as itself, and a throw as an error result with string text or none. test/tool-result-history.test.ts now pins '' for no data. All the new tests fail on main's sources, and each piece removed on its own fails one. Changelog: changelog.d/309-unconvertible-tool-result.fixed.md. --- .../309-unconvertible-tool-result.fixed.md | 27 ++ src/agent.ts | 4 +- src/framework.ts | 57 +-- src/module-registry.ts | 167 ++++++++- src/tool-result-history.ts | 21 +- test/tool-result-history.test.ts | 3 +- test/unconvertible-tool-result.test.ts | 353 ++++++++++++++++++ 7 files changed, 595 insertions(+), 37 deletions(-) create mode 100644 changelog.d/309-unconvertible-tool-result.fixed.md create mode 100644 test/unconvertible-tool-result.test.ts diff --git a/changelog.d/309-unconvertible-tool-result.fixed.md b/changelog.d/309-unconvertible-tool-result.fixed.md new file mode 100644 index 000000000..7fe4ec9ac --- /dev/null +++ b/changelog.d/309-unconvertible-tool-result.fixed.md @@ -0,0 +1,27 @@ +- A module's tool result that JSON can't hold, such as a cyclic object, a + BigInt or a `toJSON` that throws, is now answered as an error result + naming the tool wherever the framework stores or sends it: a model's + call, a script's inner call, the `utils` meta-tool for a model, + `read_image`, and the pair `puppetToolCall` stores. So is a result that + isn't a result object (`undefined`, `null`, an array, a string, an `Error` + returned rather than thrown), a result whose getters or Proxy traps throw + while it's read, and a non-error result whose data is a function or a + symbol, which JSON leaves out whole. The error keeps the module's own + error text and its `endTurn` when they can be read. Before, the first + code to store, send or trace such a result threw, and the agent's turn + never settled (#309). Those paths now read a module's result once and use + its JSON form, so its getters and `toJSON` never run a second time. A + caller that uses the result in process (`ModuleContext.callTool`, + `executeToolCall`) still gets the module's own result, and + `puppetToolCall` still returns it, or the error result where it used to + reject. +- A module's `error` is sent as text: an `Error`'s message, an object's own + `message`, any other value's JSON, or a BigInt's or symbol's `String()`. + One that says nothing (`null`, `false` or empty), and a throw with no + text, read as "Unknown error", as a missing one does. A result's + `success`, `isError` and `endTurn` are sent as booleans. +- A tool result with no data is stored as empty text, as an empty content + array is. Before, storing it threw, and the turn never settled. A + failure the tool doesn't flag as an error for the model + (`{ success: false, error, isError: false }`) with no data reads as its + error text, for the model and for a script. diff --git a/src/agent.ts b/src/agent.ts index 412707a33..b4298b380 100644 --- a/src/agent.ts +++ b/src/agent.ts @@ -12,7 +12,7 @@ import { type StoredRowLike, } from './silent-heartbeat.js'; import { - toolResultDataToHistoryString, + nonErrorResultText, truncateForHistory, DEFAULT_TOOL_RESULT_INLINE_MAX_CHARS, } from './tool-result-history.js'; @@ -1277,7 +1277,7 @@ export class Agent { toolUseId: r.id, content: r.result.isError ? truncateForHistory(r.result.error ?? 'Unknown error', DEFAULT_TOOL_RESULT_INLINE_MAX_CHARS) - : toolResultDataToHistoryString(r.result.data, DEFAULT_TOOL_RESULT_INLINE_MAX_CHARS), + : nonErrorResultText(r.result, DEFAULT_TOOL_RESULT_INLINE_MAX_CHARS), isError: r.result.isError, })); diff --git a/src/framework.ts b/src/framework.ts index 174b9eb80..4b556000a 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -64,7 +64,7 @@ import { ProcessQueueImpl } from './queue.js'; import { DEFAULT_SHUTDOWN_TIMEOUT_MS, validateShutdownTimeout, shutdownErrorMessage } from './shutdown.js'; import { REFUSAL_REACTIONS, REFUSAL_REACTION_FALLBACK } from './refusal-reactions.js'; import { Agent } from './agent.js'; -import { ModuleRegistry, isStateExistsError } from './module-registry.js'; +import { ModuleRegistry, checkedToolResult, isStateExistsError, storableToolResult, thrownResult } from './module-registry.js'; import { McplServerRegistry } from './mcpl/server-registry.js'; import { FeatureSetManager } from './mcpl/feature-set-manager.js'; import { computeGrant, CapabilityGrant, expandAdvertisementShorthand } from './mcpl/capability-grant.js'; @@ -90,6 +90,7 @@ import { ConversationRouter, CONVERSATION_ROUTING_DEPRECATION_NOTICE } from './m import { safeSlice } from './safe-slice.js'; import type { WorkspaceModule } from './modules/workspace/index.js'; import { + nonErrorResultText, toolResultDataToHistoryString, truncateForHistory, DEFAULT_TOOL_RESULT_INLINE_MAX_CHARS, @@ -11151,13 +11152,15 @@ export class AgentFramework { let ownedToEnd = false; try { const started = Date.now(); - const result = await this.executeToolCall({ - id: toolUseId, - name: toolName, - input, - callerAgentName: agentName, - }); + const call = { id: toolUseId, name: toolName, input, callerAgentName: agentName }; + const returned = await this.executeToolCall(call); const durationMs = Date.now() - started; + // The pair is stored as a model's call would be, so the result stored, + // traced and logged is its checked form. The caller gets the tool's + // own result back, as an in-process caller does, unless nothing + // usable could be stored: then it gets the error result stored. + const checked = checkedToolResult(call, returned); + const result = checked.result; // Store the pair through the same shapes the ordinary path uses. Build // the result blocks BEFORE storing the tool_use: the spill path awaits. @@ -11205,7 +11208,7 @@ export class AgentFramework { `[puppet] ${agentName}: ${toolName} → ${result.isError ? 'ERROR' : 'ok'} ` + `in ${durationMs}ms (${toolUseId})${ownedToEnd ? '' : ' — deferred behind a live turn'}`, ); - return { toolUseId, result }; + return { toolUseId, result: checked.usable ? returned : result }; } finally { if (this.activeTurnTokens.get(agentName) === turnToken) this.activeTurnTokens.delete(agentName); // Messages deferred while we held the token land now, after the pair @@ -11268,10 +11271,11 @@ export class AgentFramework { // ephemerals, ModuleContext.callTool) reach utilities the same way the // model does. if (call.name === 'utils') { - return this.handleUtilsToolCall(call.callerAgentName ?? '__ephemeral__', call); + return this.handleUtilsToolCall(call.callerAgentName ?? '__ephemeral__', call, { inProcess: true }); } - // Module tools + // Module tools. The caller uses the result in process, so it gets the + // module's own result, as the module returned it. return this.moduleRegistry.handleToolCall(call); } @@ -11773,7 +11777,7 @@ export class AgentFramework { // stays reachable by provenance after the bytes leave the wire. const raw = tc.result.isError ? tc.result.error ?? 'Unknown error' - : toolResultDataToHistoryString(tc.result.data, undefined, { + : nonErrorResultText(tc.result, undefined, { imageRef: (blockIndex, image) => ledger.retain({ toolCallId: tc.id, toolName: tc.name, @@ -11921,7 +11925,7 @@ export class AgentFramework { if (result.isError) { return `Error: ${result.error ?? 'tool call failed'}`; } - if (result.data === undefined) return ''; + if (result.data === undefined || result.data === null) return nonErrorResultText(result, 5_000_000); // RFC-005 "eagerly lazy": a script receiving a result is the strongest // available signal that the bytes are wanted (scripts process, they do // not browse) — materialize every referenced payload now, so the stub @@ -12527,7 +12531,7 @@ export class AgentFramework { const startTime = Date.now(); this.moduleRegistry - .handleToolCall({ ...call, callerAgentName: agentName }) + .storableToolCall({ ...call, callerAgentName: agentName }) .then((result) => { const durationMs = Date.now() - startTime; this.emitTrace({ @@ -15526,7 +15530,11 @@ export class AgentFramework { typeof workspace.getTools === 'function' && workspace.getTools().some((tool) => tool.name === 'read_image') ) { - const result = await workspace.handleToolCall({ ...call, name: 'read_image' }); + // The workspace module's own handler, called directly: its result + // gets the same check a module's result gets in the registry. The + // agent's name is for the check's log line, not the handler. + const moduleCall = { ...call, name: 'read_image' }; + const result = storableToolResult({ ...moduleCall, callerAgentName: agentName }, await workspace.handleToolCall(moduleCall)); if (result.success && Array.isArray(result.data)) { const image = result.data.find( (block): block is { type: 'image'; data: string; mimeType: string } => @@ -15571,11 +15579,7 @@ export class AgentFramework { ], }); } catch (error) { - finish({ - success: false, - error: error instanceof Error ? error.message : String(error), - isError: true, - }); + finish(thrownResult(error)); } })(); } @@ -15978,7 +15982,7 @@ export class AgentFramework { * (Module.getUtilities). Shared by the model-facing dispatcher and the * programmatic path (executeToolCall), so code_execution scripts and * ephemerals reach utilities too. */ - private async handleUtilsToolCall(agentName: string, call: ToolCall): Promise { + private async handleUtilsToolCall(agentName: string, call: ToolCall, options: { inProcess?: boolean } = {}): Promise { const input = (call.input ?? {}) as { action?: unknown; name?: unknown; args?: unknown }; const utilities = this.moduleRegistry.getAllUtilities(); const available = (): string => @@ -16013,13 +16017,12 @@ export class AgentFramework { }; } // Same dispatch as a first-class tool call — the module cannot tell - // which surface the call came through. - return this.moduleRegistry.handleToolCall({ - id: call.id, - name: def.name, - input: args, - callerAgentName: agentName, - }); + // which surface the call came through. An in-process caller gets the + // module's own result; the model's path takes its checked form. + const utilityCall = { id: call.id, name: def.name, input: args, callerAgentName: agentName }; + return options.inProcess + ? this.moduleRegistry.handleToolCall(utilityCall) + : this.moduleRegistry.storableToolCall(utilityCall); } default: return { success: false, isError: true, error: 'action must be "list", "describe", or "run"' }; diff --git a/src/module-registry.ts b/src/module-registry.ts index 061107ed0..623b564e3 100644 --- a/src/module-registry.ts +++ b/src/module-registry.ts @@ -273,7 +273,20 @@ export class ModuleRegistry { } /** - * Handle a tool call by routing to the appropriate module. + * A module tool's call for a path that stores or sends its result: the + * model's dispatch, a script's inner call, the `utils` meta-tool for a + * model. The result is checked and read once, in its JSON form + * ({@link storableToolResult}). + */ + async storableToolCall(call: ToolCall): Promise { + return storableToolResult(call, await this.handleToolCall(call)); + } + + /** + * Handle a tool call by routing to the appropriate module: the module's own + * result, unchecked, as it returned it, for a caller that uses the result + * in process (`ModuleContext.callTool`, `executeToolCall`, or a host + * calling the registry). A module's throw is answered as an error result. */ async handleToolCall(call: ToolCall): Promise { // Parse module--tool name @@ -309,11 +322,7 @@ export class ModuleRegistry { try { return await module.handleToolCall(moduleCall); } catch (error) { - return { - success: false, - error: error instanceof Error ? error.message : String(error), - isError: true, - }; + return thrownResult(error); } } @@ -792,3 +801,149 @@ class ProcessStateImpl implements ProcessState { this.ctx.pushEvent(event); } } + +/** A result's flags, which consumers read for their truth: booleans here. */ +const RESULT_FLAGS = ['success', 'isError', 'endTurn'] as const; + +/** + * A module's tool result as the framework can store, send and trace it. A + * module runs in the host's process, so it can return what JSON can't hold, + * such as a cyclic object, a BigInt or a `toJSON` that throws, no result + * object at all, or one whose getters or Proxy traps throw, or answer + * differently each time they're read. Everything after this (the stored + * history, the request sent to the model, traces, the tool-result guard's + * audit record, a script's result) would throw on it, and the agent's turn + * would never settle. + * + * So the result is read once, here, and this never throws. Each field a + * consumer reads is read by name, so a result class's getters count, and any + * other own field is kept, except a `toJSON`: consumers read a result's + * fields, never a `toJSON` of the result. What's returned is the JSON form of + * those fields, parsed, so every consumer holds plain data and no module code + * runs again. It's taken where the framework stores or sends a result + * ({@link ModuleRegistry.storableToolCall}). A caller that uses the result + * in process (`ModuleContext.callTool`, `executeToolCall`) gets the module's + * own result, as the module returned it ({@link ModuleRegistry.handleToolCall}). + * The flags are booleans, read for their truth as consumers read them. + * `error` is its text form, which is all a consumer reads of it, so an error + * value JSON can't hold costs nothing. An error that says nothing (null, + * false or empty) is left out, so the result reads as having no error text. + * + * A result that can't be read or converted becomes an error result naming the + * tool, keeping the module's own error text and its `endTurn` when they could + * be read. So does data that is a function or a symbol, which JSON leaves out + * whole. + */ +export function storableToolResult(call: ToolCall, result: unknown): ToolResult { + return checkedToolResult(call, result).result; +} + +/** + * {@link storableToolResult}, saying whether the module's result was usable: + * read and converted (`usable`), or replaced by the error result naming the + * tool. A caller that stores the result and hands the module's own result + * back, as `puppetToolCall` does, hands back the error result instead when + * there was nothing usable to store. + */ +export function checkedToolResult(call: ToolCall, result: unknown): { result: ToolResult; usable: boolean } { + // What's been read so far, kept by an error result if a later read throws. + let ownText: string | undefined; + let endTurn = false; + const unusable = (problem: string): { result: ToolResult; usable: boolean } => { + const own = ownText ? `, with this error: ${ownText}` : '.'; + const error = `Tool '${call.name}' ${problem}. The call itself completed${own}`; + console.error(`[tools] ${call.callerAgentName ? `agent ${call.callerAgentName}: ` : ''}${error}`); + return { result: { success: false, isError: true, error, ...(endTurn ? { endTurn: true } : {}) }, usable: false }; + }; + const reasonOf = (error: unknown) => thrownText(error).split('\n')[0] || 'no reason given'; + try { + if (!result || typeof result !== 'object' || Array.isArray(result) || result instanceof Error) { + const what = result === undefined || result === null ? String(result) + : Array.isArray(result) ? 'an array' : result instanceof Error ? 'an Error' : `a ${typeof result}`; + if (result instanceof Error) ownText = errorText(result); + return unusable(`returned ${what} instead of a result object, so there's nothing to store or send`); + } + const candidate = result as Record; + // No prototype, so an own field named like an inherited one is a field. + const fields: Record = Object.create(null); + const read = (key: string) => { if (key in candidate) fields[key] = candidate[key]; }; + // endTurn first and the error's text next, so an error result keeps + // whichever of them could be read when a later read throws. + read('endTurn'); + endTurn = Boolean(fields.endTurn); + read('error'); + ownText = errorText(fields.error); + for (const key of ['success', 'isError', 'data']) read(key); + for (const key of Object.keys(candidate)) { + if (key !== 'toJSON' && !(key in fields)) fields[key] = candidate[key]; + } + // The error is kept as its text, which consumers read; its value isn't. + delete fields.error; + // The flags are kept as their truth, which is all consumers read of them, + // before anything is serialized, so a flag's own value (a BigInt, a + // function, an object with a toJSON) neither breaks nor runs anything. + for (const flag of RESULT_FLAGS) { + if (fields[flag] !== undefined) fields[flag] = Boolean(fields[flag]); + } + // An error result reads as its error, so only a non-error result's data matters. + if (!fields.isError && (typeof fields.data === 'function' || typeof fields.data === 'symbol')) { + return unusable(`returned a ${typeof fields.data} as its data, which JSON leaves out, so there's nothing to store or send`); + } + let json: string; + try { + json = JSON.stringify(fields); + } catch (error) { + return unusable(`returned a result that can't be converted to JSON, so it can't be stored or sent (${reasonOf(error)})`); + } + const storable = JSON.parse(json) as Record; + if (ownText !== undefined) storable.error = ownText; + return { result: storable as unknown as ToolResult, usable: true }; + } catch (error) { + return unusable(`returned a result that can't be read, so it can't be stored or sent (${reasonOf(error)})`); + } +} + +/** A result's `error` as text, always a string or none: a string as it is, + * an `Error`'s message read the same way (a module's value, so not always a + * string), an object's own `message`, any other value's JSON, and a + * primitive JSON can't hold (a BigInt, a symbol) as its String(). None for a + * value that says nothing: absent, null, false or empty, which consumers + * then report as "Unknown error". */ +function errorText(error: unknown): string | undefined { + if (error === undefined || error === null || error === false || error === '') return undefined; + if (typeof error === 'string') return error; + if (error instanceof Error) return errorText(error.message); + const message = (error as { message?: unknown }).message; + if (typeof message === 'string' && message) return message; + try { + const json = JSON.stringify(error) as string | undefined; + if (json !== undefined) return json; + } catch { + // A cyclic or BigInt-holding value: nothing faithful to show as JSON. + } + if (typeof error !== 'object' && typeof error !== 'function') return thrownText(error); + return 'an error value with no text form'; +} + +/** A module's throw, answered as an error result: its text, or none when it + * has none, so consumers say "Unknown error". */ +export function thrownResult(error: unknown): ToolResult { + const text = thrownText(error); + return { success: false, isError: true, ...(text ? { error: text } : {}) }; +} + +/** A thrown value's whole text, always a string, for a result's error, and + * for a value that has none too. An `Error`'s message is a module's value, + * so it may not be a string: one that isn't is given its String(), and an + * absent one is empty. String() throws on an object without a prototype, + * and such a value is named as having no text form. */ +function thrownText(value: unknown): string { + try { + if (!(value instanceof Error)) return String(value); + const message: unknown = value.message; + if (typeof message === 'string') return message; + return message === undefined || message === null ? '' : String(message); + } catch { + return 'a value with no text form'; + } +} diff --git a/src/tool-result-history.ts b/src/tool-result-history.ts index e349cd6af..ca2824f7f 100644 --- a/src/tool-result-history.ts +++ b/src/tool-result-history.ts @@ -52,10 +52,29 @@ export function toolResultDataToHistoryString( options?: HistorySerializeOptions, ): string { const fromArray = tryHistoryStringFromContentArray(data, options); - const str = fromArray ?? JSON.stringify(data); + // JSON leaves out undefined, a function and a symbol: a result with no data + // is stored as no text, as an empty content array is. + const str = fromArray ?? (JSON.stringify(data) as string | undefined) ?? ''; return maxChars ? truncateForHistory(str, maxChars) : str; } +/** + * The text of a tool result that isn't an error result: its data, as history + * stores it, or, when it has no data, its error text. A tool can answer + * `{ success: false, error, isError: false }`, a failure it doesn't flag as + * an error for the model, and the model still reads what failed. + */ +export function nonErrorResultText( + result: { data?: unknown; error?: string }, + maxChars?: number, + options?: HistorySerializeOptions, +): string { + if ((result.data === undefined || result.data === null) && typeof result.error === 'string' && result.error) { + return maxChars ? truncateForHistory(result.error, maxChars) : result.error; + } + return toolResultDataToHistoryString(result.data, maxChars, options); +} + /** Bounded copy of an arbitrary string with the standard truncation notice. * Image slots past the cut are re-appended, never dropped (issue #104). */ export function truncateForHistory(str: string, maxChars: number): string { diff --git a/test/tool-result-history.test.ts b/test/tool-result-history.test.ts index 77e8fbb40..9fe34494a 100644 --- a/test/tool-result-history.test.ts +++ b/test/tool-result-history.test.ts @@ -56,7 +56,8 @@ test('non-array data: JSON stringify', () => { assert.equal(toolResultDataToHistoryString({ ok: true }), '{"ok":true}'); assert.equal(toolResultDataToHistoryString('hello'), '"hello"'); assert.equal(toolResultDataToHistoryString(42), '42'); - assert.equal(toolResultDataToHistoryString(undefined), undefined as any); // JSON.stringify(undefined) === undefined + // JSON leaves undefined out (JSON.stringify gives undefined): no data is no text. + assert.equal(toolResultDataToHistoryString(undefined), ''); }); test('maxChars truncates the resulting string with a marker', () => { diff --git a/test/unconvertible-tool-result.test.ts b/test/unconvertible-tool-result.test.ts new file mode 100644 index 000000000..731fcd014 --- /dev/null +++ b/test/unconvertible-tool-result.test.ts @@ -0,0 +1,353 @@ +/** + * A module runs in the host's process, so its tool can return what JSON can't + * hold: a cyclic object, a BigInt, a toJSON that throws, no result object at + * all, or one whose getters throw. Storing, sending or tracing such a result + * threw, so the agent's turn never settled and stop() didn't return (#309). It + * becomes an error result naming the tool. Every consumer gets a result's JSON + * form, read once. A result with no data is a success with no output. + */ +import { test, type TestContext } 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 } from '../src/index.js'; +import type { EventResponse, Module, ModuleContext, ProcessEvent, ProcessState, ToolCall, ToolDefinition, ToolResult } from '../src/index.js'; +import { MockMembrane, createMockResponse } from './helpers/mock-membrane.js'; + +function cyclic(): Record { + const loop: Record = { label: 'loop' }; + loop.self = loop; + return loop; +} + +/** A result class whose data is a prototype getter, which JSON skips. */ +class GetterResult { + readonly success = true; + get data(): unknown { return { from: 'getter' }; } +} + +/** What each tool of the module returns. */ +const RESULTS: Record unknown> = { + cyclic: () => ({ success: true, data: cyclic() }), + bigint: () => ({ success: true, data: { count: 10n } }), + // toJSON throws a value with no text form at all. + tojson: () => ({ success: true, data: { toJSON() { throw Object.create(null); } } }), + nodata: () => ({ success: true }), + fn: () => ({ success: true, data: () => 1 }), + errcyclic: () => ({ success: false, isError: true, error: 'Not found', data: cyclic() }), + errobjcyclic: () => ({ success: false, isError: true, error: new Error('db timeout'), data: cyclic() }), + objerror: () => ({ success: false, isError: true, error: { code: 'E42' } }), + errerror: () => ({ success: false, isError: true, error: new Error('db timeout') }), + nullerror: () => ({ success: false, isError: true, error: null }), + quietfail: () => ({ success: false, error: 'unknown channel: c1', isError: false }), + noresult: () => undefined, + nullresult: () => null, + arrayresult: () => [], + stringresult: () => 'done', + throwsbare: () => { throw Object.create(null); }, + throwsempty: () => { throw new Error(); }, + falseerror: () => ({ success: false, isError: true, error: false }), + emptyerror: () => ({ success: false, isError: true, error: '' }), + objmsgerror: () => ({ success: false, isError: true, error: { message: 'socket closed', socket: cyclic() } }), + fnerror: () => ({ success: false, isError: true, error: 'Not found', data: () => 1 }), + returnserror: () => new Error('db timeout'), + sym: () => ({ success: true, data: Symbol('s') }), + emptyreason: () => ({ success: true, data: { toJSON() { throw new Error(''); } } }), + bothtext: () => ({ success: true, data: { ok: 1 }, error: 'ignored' }), + nulldataquiet: () => ({ success: false, error: 'gone', isError: false, data: null }), + // endTurn as consumers read it: truthy, not only exactly true. + endturn: () => ({ success: true, endTurn: 1, data: { count: 1n } }), + plain: () => ({ success: true, data: { ok: true } }), + // Consumers read a result's fields, never a toJSON of the result, so the + // check doesn't either: not over a BigInt, and not for a form that isn't a + // result object. + resulttojson: () => ({ success: true, data: 1n, toJSON() { return { success: true, data: '1' }; } }), + tojsonignored: () => ({ success: true, data: { kept: true }, toJSON() { return 'not a result'; } }), + // A toJSON that answers once: consumers get what the check read. + oncetojson: () => ({ success: true, data: { calls: 0, toJSON() { if (this.calls++ > 0) throw new Error('read twice'); return { once: true }; } } }), + getterthrows: () => ({ success: true, get data() { throw Object.create(null); } }), + messagethrows: () => ({ + success: false, + isError: true, + error: Object.defineProperty(new Error(), 'message', { get() { throw new Error('message unreadable'); } }), + }), + protogetter: () => new GetterResult(), + datedata: () => ({ success: true, data: { at: new Date(0) } }), + // Flags reach consumers as booleans, read for their truth, even one JSON + // would leave out. + flagsread: () => ({ success: 1, isError: 0, endTurn: 0, data: { ok: 1 } }), + fnflag: () => ({ success: true, isError: () => true, error: 'flagged by a function', data: { ok: 1 } }), + bigflag: () => ({ success: false, isError: 1n, error: 'flagged by a BigInt' }), + // endTurn and the error's text are read before the data, so they're kept + // when the data can't be read. + endturngetter: () => ({ success: true, endTurn: true, get data() { throw new Error('gone'); } }), + errgetter: () => ({ success: false, isError: true, error: 'kept', get data() { throw new Error('data unreadable'); } }), + // An Error's message is the module's value, not always a string. + bigmessage: () => ({ success: false, isError: true, error: Object.assign(new Error('x'), { message: 1n }) }), + throwsbigmessage: () => { throw Object.assign(new Error('x'), { message: 2n }); }, +}; + +class OddResults implements Module { + readonly name = 'odd'; + /** Kept so a test can call a tool in process, as a module does. */ + ctx?: ModuleContext; + async start(ctx: ModuleContext): Promise { this.ctx = ctx; } + async stop(): Promise {} + getTools(): ToolDefinition[] { + return Object.keys(RESULTS).map((name) => ({ name, description: name, inputSchema: { type: 'object', properties: {} } })); + } + /** Two tools also reached through the `utils` meta-tool. */ + getUtilities(): ToolDefinition[] { + return ['cyclic', 'datedata'].map((name) => ({ name, description: name, inputSchema: { type: 'object', properties: {} } })); + } + async handleToolCall(call: ToolCall): Promise { + return RESULTS[call.name]!() as ToolResult; + } + async onProcess(event: ProcessEvent, _state: ProcessState): Promise { + if (event.type !== 'external-message') return {}; + const text = String((event as { content?: unknown }).content); + return { addMessages: [{ participant: 'User', content: [{ type: 'text', text }] }], requestInference: true }; + } +} + +/** A workspace stand-in whose read_image handler, which the framework's + * read_image tool calls directly, returns what JSON can't hold. */ +class OddWorkspace implements Module { + readonly name = 'workspace'; + constructor(private readonly behaviour: 'cyclic' | 'throws' = 'cyclic') {} + async start(_ctx: ModuleContext): Promise {} + async stop(): Promise {} + async onProcess(): Promise { return {}; } + getTools(): ToolDefinition[] { + return [{ name: 'read_image', description: 'read_image', inputSchema: { type: 'object', properties: {} } }]; + } + async handleToolCall(): Promise { + if (this.behaviour === 'throws') throw Object.create(null); + return { success: true, data: cyclic() }; + } + async writeBinary(): Promise<{ success: boolean }> { return { success: false }; } + async readBinary(): Promise<{ error: string }> { return { error: 'not used' }; } +} + +/** Fails the test instead of hanging when a turn never settles. */ +function within(promise: Promise, ms: number, what: string): Promise { + let timer: ReturnType | undefined; + return Promise.race([ + promise, + new Promise((_, reject) => { timer = setTimeout(() => reject(new Error(`${what} didn't finish within ${ms} ms`)), ms); }), + ]).finally(() => clearTimeout(timer)); +} + +/** A framework whose model calls `tools` in one round, then says "Noted." */ +async function setUp(t: TestContext, tools: string[], extraModules: Module[] = []) { + const dir = mkdtempSync(join(tmpdir(), 'unconvertible-result-')); + t.after(() => rmSync(dir, { recursive: true, force: true })); + const logged: string[] = []; + t.mock.method(console, 'error', (...args: unknown[]) => { logged.push(args.map(String).join(' ')); }); + const membrane = new MockMembrane(); + // `utils:NAME` runs the module's utility NAME through the utils meta-tool. + const toolUse = (tool: string) => tool.startsWith('utils:') + ? { type: 'tool_use' as const, id: `call-${tool}`, name: 'utils', input: { action: 'run', name: `odd--${tool.slice('utils:'.length)}`, args: {} } } + : { type: 'tool_use' as const, id: `call-${tool}`, name: tool.includes('--') || tool === 'read_image' ? tool : `odd--${tool}`, input: tool === 'read_image' ? { path: 'board/x.png' } : {} }; + membrane.pushResponse(createMockResponse(tools.map(toolUse), 'tool_use')); + membrane.pushResponse(createMockResponse([{ type: 'text', text: 'Noted.' }])); + const odd = new OddResults(); + const framework = await AgentFramework.create({ + storePath: join(dir, 'store'), + membrane: membrane.asMembrane(), + agents: [{ name: 'assistant', model: 'test-model', systemPrompt: 'test' }], + modules: [odd, ...extraModules], + syncIntervalMs: 0, + maintenanceIntervalMs: 0, + }); + // Bounded, so a turn that never settled can't keep the file from ending. + t.after(() => within(framework.stop(), 5_000, 'stop() in cleanup').catch(() => {})); + return { framework, membrane, logged, odd }; +} + +const unusable = (tool: string, problem: string, own = '.') => + `Tool 'odd--${tool}' ${problem}. The call itself completed${own}`; +const unconvertible = (tool: string, reason: string, own?: string) => + unusable(tool, `returned a result that can't be converted to JSON, so it can't be stored or sent (${reason})`, own); +const unreadable = (tool: string, reason: string, own?: string) => + unusable(tool, `returned a result that can't be read, so it can't be stored or sent (${reason})`, own); + +test('a round of results JSON cannot hold, or that hold no usable result, settles, and each reads as it should', async (t) => { + const tools = ['cyclic', 'bigint', 'tojson', 'nodata', 'fn', 'errcyclic', 'errobjcyclic', 'objerror', 'errerror', 'nullerror', + 'quietfail', 'noresult', 'nullresult', 'arrayresult', 'stringresult', 'throwsbare', 'throwsempty', 'falseerror', 'emptyerror', + 'objmsgerror', 'fnerror', 'returnserror', 'sym', 'emptyreason', 'bothtext', 'nulldataquiet', 'plain', + 'resulttojson', 'tojsonignored', 'oncetojson', 'getterthrows', 'messagethrows', 'protogetter', 'errgetter', + 'bigmessage', 'throwsbigmessage', 'flagsread', 'fnflag', 'bigflag']; + const { framework, membrane, logged } = await setUp(t, tools); + framework.pushEvent({ type: 'external-message', source: 'test', content: 'Call them all.', metadata: {} }); + await within(framework.runUntilIdle(), 10_000, 'the turn'); + assert.equal(framework.getAgent('assistant')!.state.status, 'idle', 'the turn settled'); + + const sent = membrane.lastStream!.receivedToolResults[0] as Array<{ toolUseId: string; content: unknown; isError: boolean }>; + const byTool = new Map(sent.map((r) => [r.toolUseId.slice('call-'.length), { content: r.content, isError: r.isError }])); + const expected: Record = { + cyclic: { content: unconvertible('cyclic', 'Converting circular structure to JSON'), isError: true }, + bigint: { content: unconvertible('bigint', 'Do not know how to serialize a BigInt'), isError: true }, + tojson: { content: unconvertible('tojson', 'a value with no text form'), isError: true }, + nodata: { content: '', isError: false }, + fn: { content: unusable('fn', "returned a function as its data, which JSON leaves out, so there's nothing to store or send"), isError: true }, + errcyclic: { content: unconvertible('errcyclic', 'Converting circular structure to JSON', ', with this error: Not found'), isError: true }, + errobjcyclic: { content: unconvertible('errobjcyclic', 'Converting circular structure to JSON', ', with this error: db timeout'), isError: true }, + objerror: { content: '{"code":"E42"}', isError: true }, + errerror: { content: 'db timeout', isError: true }, + // An error that says nothing reads as having no error text. + nullerror: { content: 'Unknown error', isError: true }, + // A failure the tool doesn't flag as an error still says what failed. + quietfail: { content: 'unknown channel: c1', isError: false }, + noresult: { content: unusable('noresult', "returned undefined instead of a result object, so there's nothing to store or send"), isError: true }, + nullresult: { content: unusable('nullresult', "returned null instead of a result object, so there's nothing to store or send"), isError: true }, + arrayresult: { content: unusable('arrayresult', "returned an array instead of a result object, so there's nothing to store or send"), isError: true }, + stringresult: { content: unusable('stringresult', "returned a string instead of a result object, so there's nothing to store or send"), isError: true }, + throwsbare: { content: 'a value with no text form', isError: true }, + // A throw or an error that says nothing reads as having no error text. + throwsempty: { content: 'Unknown error', isError: true }, + falseerror: { content: 'Unknown error', isError: true }, + emptyerror: { content: 'Unknown error', isError: true }, + // An error is kept as its text, so a cyclic value inside it costs nothing. + objmsgerror: { content: 'socket closed', isError: true }, + // An error result reads as its error, whatever its data. + fnerror: { content: 'Not found', isError: true }, + returnserror: { content: unusable('returnserror', "returned an Error instead of a result object, so there's nothing to store or send", ', with this error: db timeout'), isError: true }, + sym: { content: unusable('sym', "returned a symbol as its data, which JSON leaves out, so there's nothing to store or send"), isError: true }, + emptyreason: { content: unconvertible('emptyreason', 'no reason given'), isError: true }, + bothtext: { content: '{"ok":1}', isError: false }, + nulldataquiet: { content: 'gone', isError: false }, + plain: { content: '{"ok":true}', isError: false }, + resulttojson: { content: unconvertible('resulttojson', 'Do not know how to serialize a BigInt'), isError: true }, + tojsonignored: { content: '{"kept":true}', isError: false }, + oncetojson: { content: '{"once":true}', isError: false }, + getterthrows: { content: unreadable('getterthrows', 'a value with no text form'), isError: true }, + messagethrows: { content: unreadable('messagethrows', 'message unreadable'), isError: true }, + protogetter: { content: '{"from":"getter"}', isError: false }, + errgetter: { content: unreadable('errgetter', 'data unreadable', ', with this error: kept'), isError: true }, + bigmessage: { content: '1', isError: true }, + throwsbigmessage: { content: '2', isError: true }, + // Flags reach the model as booleans, read for their truth. + flagsread: { content: '{"ok":1}', isError: false }, + fnflag: { content: 'flagged by a function', isError: true }, + bigflag: { content: 'flagged by a BigInt', isError: true }, + }; + for (const tool of tools) assert.deepEqual(byTool.get(tool), expected[tool], tool); + for (const tool of ['cyclic', 'bigint', 'tojson', 'fn', 'errcyclic', 'errobjcyclic', 'noresult', 'nullresult', 'arrayresult', 'stringresult', + 'returnserror', 'sym', 'emptyreason', 'resulttojson', 'getterthrows', 'messagethrows', 'errgetter']) { + assert.ok(logged.includes(`[tools] agent assistant: ${expected[tool]!.content}`), `${tool} is logged:\n${logged.join('\n')}`); + } + await within(framework.stop(), 10_000, 'stop()'); +}); + +for (const [tool, error] of [ + ['endturn', unconvertible('endturn', 'Do not know how to serialize a BigInt')], + ['endturngetter', unreadable('endturngetter', 'gone')], +] as const) { + test(`an unusable result that ends the turn still ends it (${tool})`, async (t) => { + const { framework, membrane } = await setUp(t, [tool]); + framework.pushEvent({ type: 'external-message', source: 'test', content: 'Sleep.', metadata: {} }); + await within(framework.runUntilIdle(), 10_000, 'the turn'); + assert.equal(framework.getAgent('assistant')!.state.status, 'idle'); + // An ended turn stores the result instead of sending it, so the model + // never answers it. + assert.deepEqual(membrane.lastStream!.receivedToolResults, [], 'the result was not sent back to the model'); + const stored = JSON.stringify(framework.getAgent('assistant')!.getContextManager().getAllMessages()); + assert.ok(stored.includes(JSON.stringify(error).slice(1, -1)), stored); + assert.equal(stored.includes('Noted.'), false, 'no reply after the call that ended the turn'); + await within(framework.stop(), 10_000, 'stop()'); + }); +} + +test("the read_image tool's handler, throwing a value with no text form, is answered with an error", async (t) => { + const { framework, membrane } = await setUp(t, ['read_image'], [new OddWorkspace('throws')]); + framework.pushEvent({ type: 'external-message', source: 'test', content: 'Look.', metadata: {} }); + await within(framework.runUntilIdle(), 10_000, 'the turn'); + const sent = membrane.lastStream!.receivedToolResults[0] as Array<{ content: unknown; isError: boolean }>; + assert.deepEqual(sent.map((r) => ({ content: r.content, isError: r.isError })), [{ content: 'a value with no text form', isError: true }]); + await within(framework.stop(), 10_000, 'stop()'); +}); + +test('a script reads a quiet failure as its error text, and an unusable result as the same error', async (t) => { + const { framework } = await setUp(t, []); + // A script's call is answered through the event loop, so it has to run. + framework.start(); + const script = (tool: string) => (framework as unknown as { + handleScriptToolCall(agent: string, tool: string, args: Record): Promise; + }).handleScriptToolCall('assistant', `odd--${tool}`, {}); + assert.equal(await within(script('quietfail'), 10_000, 'the call'), 'unknown channel: c1'); + assert.equal(await within(script('nulldataquiet'), 10_000, 'the call'), 'gone'); + assert.equal(await within(script('cyclic'), 10_000, 'the call'), `Error: ${unconvertible('cyclic', 'Converting circular structure to JSON')}`); + assert.equal(await within(script('getterthrows'), 10_000, 'the call'), `Error: ${unreadable('getterthrows', 'a value with no text form')}`); + await within(framework.stop(), 10_000, 'stop()'); +}); + +test("the read_image tool's own handler gets the same check", async (t) => { + const { framework, membrane, logged } = await setUp(t, ['read_image'], [new OddWorkspace()]); + framework.pushEvent({ type: 'external-message', source: 'test', content: 'Look.', metadata: {} }); + await within(framework.runUntilIdle(), 10_000, 'the turn'); + assert.equal(framework.getAgent('assistant')!.state.status, 'idle', 'the turn settled'); + const sent = membrane.lastStream!.receivedToolResults[0] as Array<{ toolUseId: string; content: unknown; isError: boolean }>; + const error = "Tool 'read_image' returned a result that can't be converted to JSON, so it can't be stored or sent (Converting circular structure to JSON). The call itself completed."; + assert.deepEqual(sent.map((r) => ({ id: r.toolUseId, content: r.content, isError: r.isError })), [{ id: 'call-read_image', content: error, isError: true }]); + assert.ok(logged.includes(`[tools] agent assistant: ${error}`), logged.join('\n')); + await within(framework.stop(), 10_000, 'stop()'); +}); + +test("a puppeted call stores the checked result, returning the error for an unusable one and the tool's own for a usable one, and a direct call gets the module's own result", async (t) => { + const { framework, logged } = await setUp(t, []); + const { result } = await within(framework.puppetToolCall('assistant', 'odd--cyclic', {}), 10_000, 'the puppeted call'); + const error = unconvertible('cyclic', 'Converting circular structure to JSON'); + assert.deepEqual(result, { success: false, isError: true, error }); + const stored = JSON.stringify(framework.getAgent('assistant')!.getContextManager().getAllMessages()); + assert.ok(stored.includes(JSON.stringify(error).slice(1, -1)), stored); + // A usable result is stored in its JSON form, and the caller gets the + // tool's own result back, as on main. + const { result: dated } = await within(framework.puppetToolCall('assistant', 'odd--datedata', {}), 10_000, 'the puppeted call'); + assert.ok((dated.data as { at: unknown }).at instanceof Date, 'the caller gets the Date itself'); + const storedNow = JSON.stringify(framework.getAgent('assistant')!.getContextManager().getAllMessages()); + assert.ok(storedNow.includes('{\\"at\\":\\"1970-01-01T00:00:00.000Z\\"}'), storedNow); + // A direct call uses the result in process, so it gets the module's own + // result, unchecked, and nothing is logged for it. + const direct = await framework.executeToolCall({ id: 'direct-1', name: 'odd--cyclic', input: {} }); + assert.equal(direct.success, true); + const loop = direct.data as Record; + assert.equal(loop.self, loop, 'the cyclic object itself'); + assert.deepEqual(logged.filter((line) => line.startsWith('[tools]')), [`[tools] agent assistant: ${error}`]); + await within(framework.stop(), 10_000, 'stop()'); +}); + +test("a caller that uses the result in process gets the module's own result, as on main", async (t) => { + const { framework, odd } = await setUp(t, []); + const at = (result: ToolResult) => (result.data as { at: unknown }).at; + // executeToolCall, ModuleContext.callTool, and a utility through either. + assert.ok(at(await framework.executeToolCall({ id: 'direct-date', name: 'odd--datedata', input: {} })) instanceof Date); + assert.ok(at(await odd.ctx!.callTool({ id: 'ctx-date', name: 'odd--datedata', input: {} })) instanceof Date); + const viaUtils = { name: 'utils', input: { action: 'run', name: 'odd--datedata', args: {} } }; + assert.ok(at(await framework.executeToolCall({ id: 'utils-date', ...viaUtils })) instanceof Date); + assert.ok(at(await odd.ctx!.callTool({ id: 'ctx-utils-date', ...viaUtils })) instanceof Date); + // Its flags as the module returned them, and its getters unread. + assert.deepEqual(await framework.executeToolCall({ id: 'direct-flags', name: 'odd--flagsread', input: {} }), + { success: 1, isError: 0, endTurn: 0, data: { ok: 1 } }); + const getter = await framework.executeToolCall({ id: 'direct-getter', name: 'odd--getterthrows', input: {} }); + assert.throws(() => getter.data, 'the getter is the module\'s own'); + // A throw is still answered as an error result, its text a string. + assert.deepEqual(await framework.executeToolCall({ id: 'direct-throw', name: 'odd--throwsbigmessage', input: {} }), + { success: false, isError: true, error: '2' }); + assert.deepEqual(await framework.executeToolCall({ id: 'direct-empty', name: 'odd--throwsempty', input: {} }), + { success: false, isError: true }, 'a throw with no text has no error text'); + await within(framework.stop(), 10_000, 'stop()'); +}); + +test("the model's call through the utils meta-tool takes the checked form", async (t) => { + const { framework, membrane } = await setUp(t, ['utils:cyclic']); + framework.pushEvent({ type: 'external-message', source: 'test', content: 'Run it.', metadata: {} }); + await within(framework.runUntilIdle(), 10_000, 'the turn'); + assert.equal(framework.getAgent('assistant')!.state.status, 'idle', 'the turn settled'); + const sent = membrane.lastStream!.receivedToolResults[0] as Array<{ content: unknown; isError: boolean }>; + assert.deepEqual(sent.map((r) => ({ content: r.content, isError: r.isError })), + [{ content: unconvertible('cyclic', 'Converting circular structure to JSON'), isError: true }]); + await within(framework.stop(), 10_000, 'stop()'); +}); From cc875cc2f9e72a4f14e62099a0496eaf3dac4f8a Mon Sep 17 00:00:00 2001 From: Ada-1017 Date: Tue, 13 Oct 2026 00:54:02 -0400 Subject: [PATCH 2/2] fix(tools): a non-error result's text reads each of its fields once nonErrorResultText read a result's data up to three times and its error up to three times. The direct Agent API passes its caller's own result here, unchecked, as main did (main read data once), so a getter that answers once, or differently the second time, broke a call that worked on main. It now reads data once, and error once when there's no data. Test: the Agent's tool-result messages, built from a result whose data getter throws on a second read, and from a quiet failure whose data and error getters do the same, give the data's text and the error text. It fails on the previous helper. --- src/tool-result-history.ts | 11 +++++++--- test/unconvertible-tool-result.test.ts | 30 ++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/tool-result-history.ts b/src/tool-result-history.ts index ca2824f7f..73ddf431e 100644 --- a/src/tool-result-history.ts +++ b/src/tool-result-history.ts @@ -63,16 +63,21 @@ export function toolResultDataToHistoryString( * stores it, or, when it has no data, its error text. A tool can answer * `{ success: false, error, isError: false }`, a failure it doesn't flag as * an error for the model, and the model still reads what failed. + * + * 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. */ export function nonErrorResultText( result: { data?: unknown; error?: string }, maxChars?: number, options?: HistorySerializeOptions, ): string { - if ((result.data === undefined || result.data === null) && typeof result.error === 'string' && result.error) { - return maxChars ? truncateForHistory(result.error, maxChars) : result.error; + const data = result.data; + if (data === undefined || data === null) { + const error = result.error; + if (typeof error === 'string' && error) return maxChars ? truncateForHistory(error, maxChars) : error; } - return toolResultDataToHistoryString(result.data, maxChars, options); + return toolResultDataToHistoryString(data, maxChars, options); } /** Bounded copy of an arbitrary string with the standard truncation notice. diff --git a/test/unconvertible-tool-result.test.ts b/test/unconvertible-tool-result.test.ts index 731fcd014..5b0497fa5 100644 --- a/test/unconvertible-tool-result.test.ts +++ b/test/unconvertible-tool-result.test.ts @@ -351,3 +351,33 @@ test("the model's call through the utils meta-tool takes the checked form", asyn [{ content: unconvertible('cyclic', 'Converting circular structure to JSON'), isError: true }]); await within(framework.stop(), 10_000, 'stop()'); }); + +test("the direct Agent API reads each field of its caller's result once", async () => { + // The Agent class turns a result its caller hands it into history text + // without the framework's check, so a getter is read as it was written. + const { Agent } = await import('../src/agent.js'); + const agent = new Agent( + { name: 'direct', model: 'test-model', systemPrompt: 'sys' }, + {} as never, + {} as never, + ); + const oneRead = (value: unknown) => { + let reads = 0; + return () => { + reads += 1; + if (reads > 1) throw new Error('read a second time'); + return value; + }; + }; + const data = oneRead('the payload'); + const quietError = oneRead('what failed'); + const quietData = oneRead(undefined); + const results = [ + { id: 'a', name: 'withData', result: { success: true, get data() { return data(); } } }, + { id: 'b', name: 'quiet', result: { success: false, isError: false, get data() { return quietData(); }, get error() { return quietError(); } } }, + ]; + const built = (agent as unknown as { buildToolResultMessages(r: unknown[]): Array<{ content: Array<{ content: string }> }> }) + .buildToolResultMessages(results); + assert.deepEqual(built[0].content.map((block) => block.content), [JSON.stringify('the payload'), 'what failed']); +}); +