From 743f66e547279916d5e37454e78b11eb4b54dca3 Mon Sep 17 00:00:00 2001 From: Kai Date: Tue, 7 Jul 2026 11:40:27 +0800 Subject: [PATCH] refactor: move tool-result metadata into a structured note side channel (#1437) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: stop rendering notes from tool results in the terminal and web UIs Tool results carry blocks as side-channel notes for the model (ReadMediaFile summaries, Read status, MCP image captions, error/empty sentinels). Keep them in history for the model, but strip them at every core-to-UI boundary so they no longer render as plain text. vis is intentionally left untouched to preserve the model's-eye view for debugging. * fix: keep error/empty status text visible when stripping tool-result tags Unwrap the tool error/empty sentinels (ERROR: ..., Tool output is empty.) instead of deleting them: keep the human-readable text and drop only the tags. Otherwise a failed or empty tool result rendered as a blank output, indistinguishable from a rendering bug. The model still reads the wrapped form in history. * refactor: move tool-result metadata into a structured note side channel Tool-produced model-facing metadata (ReadMediaFile summaries, Read status lines, MCP image-compression captions) was baked into tool output as text, so every UI had to strip it back out and three copies of the model-view normalization had silently drifted apart. - ExecutableToolResult gains `note`: content rendered to the model but never to UIs; records and history now store the raw output plus the structured isError/note fields - the model view is rendered exactly once at the LLM projection boundary by renderToolResultForModel; the transcript and vis hand-copies are deleted (vis now calls the same function for its model view, fixing their drifted empty-output checks) - ReadMediaFile / Read / MCP captions write `note`; tool outputs stay pure data, and text-only results keep a single text part (note joined with a newline) so provider tool content stays a plain string - all UI-side stripping is removed; failed tools show their own error text with the structured isError flag - wire protocol 1.4 -> 1.5 migrates existing records' tool-produced blocks into `note` on resume * fix: provider-neutral wording, no wire migration, direct optional fields - "The attached image was downsampled" replaces directional wording that depended on provider serialization order (inline media vs flatten-and- re-attach) - drop the 1.4 -> 1.5 wire migration: legacy records replay verbatim, so the model view of old sessions stays byte-identical to what the model originally saw and UIs show the legacy text as-is; this also removes the risk of the migration misclassifying user data that quotes tool metadata, and the additive note field needs no version bump - pass optional result fields as undefined instead of conditional spreads (repo convention) * fix: enforce the note contract at the trust boundary; narrow the TUI system-tag guard - normalizeToolResult now keeps a note only when it is a non-empty string: tools and finalize hooks are arbitrary JS, and a malformed note (null, number, object) would previously persist into the record and crash every subsequent LLM projection of the session. Everything downstream now trusts note to be string | undefined. - the TUI tool body suppression matches the full tag instead of any tag (file contents, MCP text) stays visible, covered through the real ToolCallComponent path. * fix: return MCP compression captions as data instead of extracting them from text compressImageContentParts now returns { parts, captions } — captions come back from the compressor as structured data and are never inserted into the parts, so the MCP pipeline no longer pattern-matches text to move them into the note side channel. Tool output that merely quotes a caption (a doc, a log, a test fixture) stays verbatim in the output. Also corrects the stale claim that prompt ingestion uses this helper (it compresses per image while constructing the part). * docs: correct the image-compression re-export comment; export CompressedContentParts The package-root comment still described compressImageContentParts as the input-stage helper every ingestion site calls; prompt ingestion compresses per image with compressBase64ForModel / compressImageForModel, and the MCP pipeline is the walker's only caller. Also export the walker's CompressedContentParts return type so public-API consumers can name it. * feat: wrap tool status sentinels in so the model can tell harness verdicts from tool output The error/empty status text is model-only after the note refactor (UIs render the raw output and style failures via the structured isError flag), so the earlier plain-text wording served no remaining audience. Wrapping the statuses in gives every piece of system-generated text inside a tool result the same marker: - failed calls get 'ERROR: Tool execution failed.' unconditionally — the ERROR:-prefix guard is removed, so the harness verdict can no longer be confused with tool output that happens to start with error-like text - empty outputs render as 'Tool output is empty.'; the plain placeholder the loop layer bakes into records is still recognized and upgraded at projection time * style: collapse an internal helper docstring per the services subtree convention --- .changeset/hide-tool-result-system-notes.md | 5 + .../src/tui/components/messages/tool-call.ts | 12 +- .../messages/tool-renderers/media.ts | 11 +- .../tui/components/messages/tool-call.test.ts | 27 +++- .../messages/tool-renderers/media.test.ts | 2 - .../messages/tool-renderers/truncated.test.ts | 13 ++ apps/vis/server/src/lib/context-projector.ts | 77 +---------- .../server/test/lib/context-projector.test.ts | 8 +- .../agent-core/src/agent/context/index.ts | 52 +------- .../agent-core/src/agent/context/projector.ts | 17 ++- .../src/agent/context/tool-result-render.ts | 108 ++++++++++++++++ .../agent-core/src/agent/context/types.ts | 6 + packages/agent-core/src/index.ts | 11 +- packages/agent-core/src/loop/tool-call.ts | 15 ++- packages/agent-core/src/loop/types.ts | 10 ++ packages/agent-core/src/mcp/output.ts | 37 ++++-- .../src/services/message/transcript.ts | 53 ++------ .../src/tools/builtin/file/read-media.md | 2 +- .../src/tools/builtin/file/read-media.ts | 34 ++--- .../agent-core/src/tools/builtin/file/read.ts | 12 +- .../src/tools/support/image-compress.ts | 54 +++++--- .../agent-core/test/agent/context.test.ts | 115 ++++++++++++++++- .../test/agent/records/index.test.ts | 61 +++++++++ packages/agent-core/test/agent/resume.test.ts | 2 +- .../test/agent/tool-result-render.test.ts | 121 ++++++++++++++++++ .../test/loop/tool-call.e2e.test.ts | 52 ++++++++ packages/agent-core/test/mcp/output.test.ts | 96 ++++++++++---- .../test/services/message-service.test.ts | 25 ++++ .../test/services/message-transcript.test.ts | 16 +-- .../test/tools/builtin-current.test.ts | 9 +- .../test/tools/image-compress.test.ts | 38 +++--- .../agent-core/test/tools/read-file.test.ts | 2 +- .../agent-core/test/tools/read-media.test.ts | 95 +++++++------- packages/agent-core/test/tools/read.test.ts | 100 +++++++-------- 34 files changed, 889 insertions(+), 409 deletions(-) create mode 100644 .changeset/hide-tool-result-system-notes.md create mode 100644 packages/agent-core/src/agent/context/tool-result-render.ts create mode 100644 packages/agent-core/test/agent/tool-result-render.test.ts diff --git a/.changeset/hide-tool-result-system-notes.md b/.changeset/hide-tool-result-system-notes.md new file mode 100644 index 000000000..e4a3936aa --- /dev/null +++ b/.changeset/hide-tool-result-system-notes.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Stop showing tool-produced `` metadata in tool outputs; failed tools now show their own error text. diff --git a/apps/kimi-code/src/tui/components/messages/tool-call.ts b/apps/kimi-code/src/tui/components/messages/tool-call.ts index 53fc30dfa..a25c88e64 100644 --- a/apps/kimi-code/src/tui/components/messages/tool-call.ts +++ b/apps/kimi-code/src/tui/components/messages/tool-call.ts @@ -2130,10 +2130,14 @@ export class ToolCallComponent extends Container { return; } - // Outputs that start with a `` tag are harness-injected - // reminders piggy-backing on a tool result. They are noise for the - // user, so suppress the body while keeping the header chip intact. - if (result.output.trimStart().startsWith('` tag are harness-injected + // reminders piggy-backing on a tool result (e.g. a finalize hook rewrote + // the output). They are noise for the user, so suppress the body while + // keeping the header chip intact. Match the full reminder tag only: tool + // metadata no longer travels inside `output` (it rides the result's + // `note` side channel), so real output starting with a literal `` + // is user data and must stay visible. + if (result.output.trimStart().startsWith('')) { return; } diff --git a/apps/kimi-code/src/tui/components/messages/tool-renderers/media.ts b/apps/kimi-code/src/tui/components/messages/tool-renderers/media.ts index 100968daf..b798cc8e5 100644 --- a/apps/kimi-code/src/tui/components/messages/tool-renderers/media.ts +++ b/apps/kimi-code/src/tui/components/messages/tool-renderers/media.ts @@ -27,11 +27,9 @@ export interface ReadMediaSummary { mimeType?: string; bytes?: number; url?: string; - originalSize?: string; } const PATH_TAG_RE = /^<(image|video)\s+path="([^"]+)">$/; -const ORIGINAL_SIZE_RE = /original size\s+(\d+x\d+px)/; const DATA_URL_RE = /^data:([^;]+);base64,(.*)$/s; function bytesFromBase64(b64: string): number { @@ -55,7 +53,6 @@ export function parseReadMediaOutput(output: string): ReadMediaSummary | null { let mimeType: string | undefined; let bytes: number | undefined; let url: string | undefined; - let originalSize: string | undefined; let foundMedia = false; for (const raw of parsed) { @@ -64,15 +61,11 @@ export function parseReadMediaOutput(output: string): ReadMediaSummary | null { const type = part['type']; if (type === 'text' && typeof part['text'] === 'string') { - const text = part['text']; - const tag = PATH_TAG_RE.exec(text); + const tag = PATH_TAG_RE.exec(part['text']); if (tag) { kind = tag[1] as 'image' | 'video'; path = tag[2]; - continue; } - const size = ORIGINAL_SIZE_RE.exec(text); - if (size) originalSize = size[1]; continue; } @@ -103,7 +96,6 @@ export function parseReadMediaOutput(output: string): ReadMediaSummary | null { if (mimeType !== undefined) summary.mimeType = mimeType; if (bytes !== undefined) summary.bytes = bytes; if (url !== undefined) summary.url = url; - if (originalSize !== undefined) summary.originalSize = originalSize; return summary; } @@ -117,7 +109,6 @@ function metaSegments(summary: ReadMediaSummary): string[] { const segs: string[] = []; if (summary.mimeType !== undefined) segs.push(summary.mimeType); if (summary.bytes !== undefined) segs.push(formatBytes(summary.bytes)); - if (summary.originalSize !== undefined) segs.push(summary.originalSize); return segs; } diff --git a/apps/kimi-code/test/tui/components/messages/tool-call.test.ts b/apps/kimi-code/test/tui/components/messages/tool-call.test.ts index 06d7ec419..84680e3bb 100644 --- a/apps/kimi-code/test/tui/components/messages/tool-call.test.ts +++ b/apps/kimi-code/test/tui/components/messages/tool-call.test.ts @@ -283,7 +283,7 @@ describe('ToolCallComponent', () => { }); }); - it('hides tool output bodies that start with a { + it('hides tool output bodies that start with a { const reminderOutput = '\nThe task tools have not been used recently.\n'; const component = new ToolCallComponent( @@ -310,7 +310,7 @@ describe('ToolCallComponent', () => { expect(expanded).not.toContain('task tools'); }); - it('hides { + it('hides { const component = new ToolCallComponent( { id: 'call_hidden_err', @@ -329,6 +329,29 @@ describe('ToolCallComponent', () => { expect(out).not.toContain('do not show'); }); + it('renders output that merely starts with a literal tag', () => { + // Tool metadata no longer travels inside `output` (it rides the result's + // `note` side channel), so real output starting with the literal tag — + // a file that contains it, an MCP tool's text — must stay visible. + const component = new ToolCallComponent( + { + id: 'call_literal', + name: 'Bash', + args: { command: 'cat notes.txt' }, + }, + { + tool_call_id: 'call_literal', + output: 'literal text from a user file\nsecond line', + is_error: false, + }, + ); + + component.setExpanded(true); + const out = strip(component.render(100).join('\n')); + expect(out).toContain('literal text from a user file'); + expect(out).toContain('second line'); + }); + it('renders AgentSwarm results as a one-line summary without raw XML', () => { const output = [ '', diff --git a/apps/kimi-code/test/tui/components/messages/tool-renderers/media.test.ts b/apps/kimi-code/test/tui/components/messages/tool-renderers/media.test.ts index e56cb8e0e..691f1d88a 100644 --- a/apps/kimi-code/test/tui/components/messages/tool-renderers/media.test.ts +++ b/apps/kimi-code/test/tui/components/messages/tool-renderers/media.test.ts @@ -38,7 +38,6 @@ function imageOutput(path: string, b64 = PNG_B64, mime = 'image/png'): string { { type: 'text', text: `` }, { type: 'image_url', imageUrl: { url: `data:${mime};base64,${b64}` } }, { type: 'text', text: '' }, - { type: 'text', text: `Loaded image file "${path}" (${mime}, 70 bytes, original size 1x1px).` }, ]); } @@ -58,7 +57,6 @@ describe('parseReadMediaOutput', () => { expect(m?.path).toBe('/tmp/a.png'); expect(m?.mimeType).toBe('image/png'); expect(m?.bytes).toBeGreaterThan(0); - expect(m?.originalSize).toBe('1x1px'); }); it('extracts video kind and mime', () => { diff --git a/apps/kimi-code/test/tui/components/messages/tool-renderers/truncated.test.ts b/apps/kimi-code/test/tui/components/messages/tool-renderers/truncated.test.ts index ecdf2a1f5..4c90c0392 100644 --- a/apps/kimi-code/test/tui/components/messages/tool-renderers/truncated.test.ts +++ b/apps/kimi-code/test/tui/components/messages/tool-renderers/truncated.test.ts @@ -74,4 +74,17 @@ describe('TruncatedOutputComponent', () => { expect(visibleWidth(line)).toBeLessThanOrEqual(37); } }); + + it('renders output verbatim, including literal text in file content', () => { + // Tool metadata no longer travels inside `output` (it rides the result's + // `note` side channel), so the renderer must not eat user data that + // merely contains the literal tag. + const component = new TruncatedOutputComponent( + 'literal text from a user file\n', + { expanded: true, isError: false }, + ); + const out = strip(component.render(80).join('\n')); + expect(out).toContain('literal text from a user file'); + expect(out).toContain(''); + }); }); diff --git a/apps/vis/server/src/lib/context-projector.ts b/apps/vis/server/src/lib/context-projector.ts index 4def1c631..4cb2468ab 100644 --- a/apps/vis/server/src/lib/context-projector.ts +++ b/apps/vis/server/src/lib/context-projector.ts @@ -4,6 +4,7 @@ import { buildCompactionElisionText, collectCompactableUserMessages, isRealUserInput, + renderToolResultForModel, selectCompactionUserMessages, selectRecentUserMessages, } from '@moonshot-ai/agent-core'; @@ -193,14 +194,12 @@ export function projectContext( } openSteps.delete(ev.uuid); } else if (ev.type === 'tool.result') { - // Mirror what the MODEL saw, not the raw output. agent-core's - // ContextMemory.appendLoopEvent (`tool.result` case) stores - // `createToolMessage(toolCallId, toolResultOutputForModel(result))`, - // which normalizes error / empty outputs with sentinel strings. Using - // `ev.result.output` directly would surface content the model never - // received for failed / empty tool calls. See - // `toolResultContentForModel` below. - const content = toolResultContentForModel(ev.result); + // Mirror what the MODEL saw, not the raw output. This calls the + // SAME `renderToolResultForModel` agent-core applies at its LLM + // projection boundary (error status prefix, empty-output + // placeholder, trailing note), so vis's model view is the real + // projection rather than a hand-kept copy. + const content = renderToolResultForModel(ev.result); const toolMsg: ContextMessage = { role: 'tool', content, @@ -564,68 +563,6 @@ function addUsage(into: TokenUsage, src: TokenUsage): void { (into as any).inputCacheCreation += src.inputCacheCreation; } -// ── Tool-result normalization (mirror of agent-core) ───────────────────────── -// These replicate agent-core's `toolResultOutputForModel` so vis's model-view -// shows the EXACT content the model received for a tool result. The constants -// and branch conditions are copied verbatim from -// `packages/agent-core/src/agent/context/index.ts` (lines 18-22, 350-377). Keep -// them byte-identical with that source — if agent-core changes the sentinels or -// branch logic, update here too. -const TOOL_ERROR_STATUS = 'ERROR: Tool execution failed.'; -const TOOL_EMPTY_STATUS = 'Tool output is empty.'; -const TOOL_EMPTY_ERROR_STATUS = - 'ERROR: Tool execution failed. Tool output is empty.'; -const TOOL_OUTPUT_EMPTY_TEXT = 'Tool output is empty.'; - -/** Mirrors agent-core `isEmptyOutputText` - * (`packages/agent-core/src/agent/context/index.ts` ~line 375). */ -function isEmptyOutputText(output: string): boolean { - return output.length === 0 || output.trim() === TOOL_OUTPUT_EMPTY_TEXT; -} - -/** Mirrors agent-core `toolResultOutputForModel` - * (`packages/agent-core/src/agent/context/index.ts` ~line 350), then wraps the - * result into `ContentPart[]` exactly as `createToolMessage` does (a string - * output → a single `{ type: 'text', text }` part). The model saw this - * normalized content in BOTH model and full views (agent-core normalizes at - * append time, before any of the destructive lifecycle events), so the - * tool.result branch uses this output mode-independently. */ -function toolResultContentForModel(result: { - output: string | ContentPart[]; - isError?: boolean; -}): ContentPart[] { - const output = result.output; - if (typeof output === 'string') { - let normalized: string; - if (result.isError === true) { - if (output.length === 0) { - normalized = TOOL_EMPTY_ERROR_STATUS; - } else if (output.trimStart().startsWith('ERROR:')) { - normalized = output; - } else { - normalized = `${TOOL_ERROR_STATUS}\n${output}`; - } - } else { - normalized = isEmptyOutputText(output) ? TOOL_EMPTY_STATUS : output; - } - // Match createToolMessage: a string output becomes a single text part. - return [{ type: 'text', text: normalized }]; - } - - if (output.length === 0) { - return [ - { - type: 'text', - text: result.isError === true ? TOOL_EMPTY_ERROR_STATUS : TOOL_EMPTY_STATUS, - }, - ]; - } - if (result.isError === true) { - return [{ type: 'text', text: TOOL_ERROR_STATUS }, ...output]; - } - return output; -} - const MICRO_TRUNCATED_MARKER = '[Old tool result content cleared]'; const MICRO_MIN_CONTENT_TOKENS = 100; diff --git a/apps/vis/server/test/lib/context-projector.test.ts b/apps/vis/server/test/lib/context-projector.test.ts index e6b164a5b..0a5ded18f 100644 --- a/apps/vis/server/test/lib/context-projector.test.ts +++ b/apps/vis/server/test/lib/context-projector.test.ts @@ -215,10 +215,12 @@ describe('context-projector', () => { ]); }); - it('tool.result: error string already starting with ERROR: is passed through (no double prefix)', () => { - const text = 'ERROR: already wrapped\ndetails here'; + it('tool.result: error string starting with ERROR: still gets the wrapped status', () => { + // The -wrapped status is the harness verdict; the tool's own + // "ERROR:" text is data, so the status is added unconditionally. + const text = 'ERROR: already wrapped\ndetails here'; const msg = projectToolResult({ output: text, isError: true }); - expect(msg.content).toEqual([{ type: 'text', text }]); + expect(msg.content).toEqual([{ type: 'text', text: `${TOOL_ERROR_STATUS}\n${text}` }]); }); it('tool.result: empty string output (non-error) becomes the empty sentinel', () => { diff --git a/packages/agent-core/src/agent/context/index.ts b/packages/agent-core/src/agent/context/index.ts index 66608c12a..b8951ba93 100644 --- a/packages/agent-core/src/agent/context/index.ts +++ b/packages/agent-core/src/agent/context/index.ts @@ -2,7 +2,7 @@ import { createToolMessage, type ContentPart, type Message } from '@moonshot-ai/ import type { Agent } from '..'; import { ErrorCodes, KimiError } from '../../errors'; -import type { ExecutableToolResult, LoopRecordedEvent } from '../../loop'; +import type { LoopRecordedEvent } from '../../loop'; import { extractImageCompressionCaptions } from '../../tools/support/image-compress'; import { estimateTokens, estimateTokensForMessages } from '../../utils/tokens'; import { escapeXml } from '../../utils/xml-escape'; @@ -34,11 +34,6 @@ import { export * from './types'; export * from './dynamic-tools'; -const TOOL_ERROR_STATUS = 'ERROR: Tool execution failed.'; -const TOOL_EMPTY_STATUS = 'Tool output is empty.'; -const TOOL_EMPTY_ERROR_STATUS = - 'ERROR: Tool execution failed. Tool output is empty.'; -const TOOL_OUTPUT_EMPTY_TEXT = 'Tool output is empty.'; const TOOL_INTERRUPTED_ON_RESUME_OUTPUT = 'Tool execution was interrupted before its result was recorded. Do not assume the tool completed successfully.'; @@ -641,11 +636,16 @@ export class ContextMemory { // closed in place at a step boundary (a stale duplicate from an older // tail-only finishResume), or its call is gone. if (!this.pendingToolResultIds.has(event.toolCallId)) return; - const message = createToolMessage(event.toolCallId, toolResultOutputForModel(event.result)); + // History stores the fact verbatim: the tool's own output plus the + // structured isError/note fields. Model-facing status text (error + // prefix, empty placeholder) and the note are rendered only at LLM + // projection time (see tool-result-render.ts). + const message = createToolMessage(event.toolCallId, event.result.output); this.pushHistory({ ...message, role: 'tool', isError: event.result.isError, + note: event.result.note, }); this.pendingToolResultIds.delete(event.toolCallId); this.flushDeferredMessagesIfToolExchangeClosed(); @@ -695,40 +695,6 @@ export class ContextMemory { } } -function toolResultOutputForModel(result: ExecutableToolResult): string | ContentPart[] { - const output = result.output; - if (typeof output === 'string') { - if (result.isError === true) { - if (output.length === 0) return TOOL_EMPTY_ERROR_STATUS; - if (output.trimStart().startsWith('ERROR:')) return output; - return `${TOOL_ERROR_STATUS}\n${output}`; - } - return isEmptyOutputText(output) ? TOOL_EMPTY_STATUS : output; - } - - // Treat an array output with no sendable content (empty, or only empty/ - // whitespace-only text blocks) the same as an empty string output: emit the - // placeholder. Otherwise projection would strip the blank text blocks, leave - // the tool message empty, and throw on every send — bricking the session. A - // non-text part (image/etc.) or any non-whitespace text keeps the real output. - if (isEmptyEquivalentContentArray(output)) { - return [ - { - type: 'text', - text: result.isError === true ? TOOL_EMPTY_ERROR_STATUS : TOOL_EMPTY_STATUS, - }, - ]; - } - if (result.isError === true) { - return [{ type: 'text', text: TOOL_ERROR_STATUS }, ...output]; - } - return output; -} - -function isEmptyEquivalentContentArray(output: readonly ContentPart[]): boolean { - return output.every((part) => part.type === 'text' && part.text.trim().length === 0); -} - // Split inline image-compression captions (see buildImageCompressionCaption) // out of user prompt content. A caption may be a standalone text part (server // route, ACP) or merged into an adjacent text segment (TUI paste), so each @@ -758,10 +724,6 @@ function splitImageCompressionCaptions(content: readonly ContentPart[]): { return { captions, parts }; } -function isEmptyOutputText(output: string): boolean { - return output.trim().length === 0 || output.trim() === TOOL_OUTPUT_EMPTY_TEXT; -} - function formatUndoUnavailableMessage( requestedCount: number, undoableCount: number, diff --git a/packages/agent-core/src/agent/context/projector.ts b/packages/agent-core/src/agent/context/projector.ts index cf77fd6a1..b4083f603 100644 --- a/packages/agent-core/src/agent/context/projector.ts +++ b/packages/agent-core/src/agent/context/projector.ts @@ -1,6 +1,7 @@ import type { ContentPart, Message, TextPart } from '@moonshot-ai/kosong'; import { ErrorCodes, KimiError } from '../../errors'; +import { renderToolResultForModel } from './tool-result-render'; import type { ContextMessage } from './types'; export interface ProjectOptions { @@ -359,26 +360,34 @@ function prepareMessageForProjection( ): ContextMessage | null { if (message.partial === true) return null; + // Tool results are stored as facts (raw output + structured isError/note). + // Render the model-visible form — error status prefix, empty-output + // placeholder, trailing note — exactly here, at the projection boundary. + const source = + message.role === 'tool' + ? { ...message, content: renderToolResultForModel({ output: message.content, note: message.note, isError: message.isError }) } + : message; + let content: ContentPart[] | undefined; - for (const [index, part] of message.content.entries()) { + for (const [index, part] of source.content.entries()) { // Strict providers reject a text block that is empty OR whitespace-only // ("text content blocks must contain non-whitespace text"). Drop both; a // block with surrounding whitespace but real content is kept verbatim. if (part.type === 'text' && part.text.trim().length === 0) { - content ??= message.content.slice(0, index); + content ??= source.content.slice(0, index); // Report only whitespace-only (non-empty) blocks: a truly empty `''` block // is routine cleanup (e.g. a trailing empty text part after a tool call), // whereas a block that is non-empty yet all-whitespace signals something // upstream fed blank content and is worth surfacing for debugging. if (part.text.length > 0) { - onAnomaly?.({ kind: 'whitespace_text_dropped', role: message.role }); + onAnomaly?.({ kind: 'whitespace_text_dropped', role: source.role }); } continue; } content?.push(part); } - const next = content === undefined ? message : { ...message, content }; + const next = content === undefined ? source : { ...source, content }; if (next.role === 'tool' && next.content.length === 0) { throw new KimiError( ErrorCodes.REQUEST_INVALID, diff --git a/packages/agent-core/src/agent/context/tool-result-render.ts b/packages/agent-core/src/agent/context/tool-result-render.ts new file mode 100644 index 000000000..9e1b7646b --- /dev/null +++ b/packages/agent-core/src/agent/context/tool-result-render.ts @@ -0,0 +1,108 @@ +/** + * The single place where a stored tool result (pure data + structured + * status/note) is rendered into the content the model actually receives. + * + * History and wire records store facts: the tool's own `output`, the + * structured `isError` flag, and an optional `note` (content routed to the + * model but never to user-facing UIs — see `ExecutableToolResult.note`). + * Rendering those facts into model-visible text is a provider-boundary + * concern and happens exactly once, here: + * + * - a failed call gets a ``-wrapped `ERROR:` status line, added + * unconditionally so the harness verdict is never confused with tool + * output that merely contains error-like text; + * - an empty output is replaced with a ``-wrapped placeholder so + * strict providers do not reject an empty tool message; + * - the note, when present, is appended verbatim. No wrapping is added: any + * formatting is the producing tool's choice. A text-only result keeps a + * SINGLE text part (note joined with a newline): providers serialize that + * as plain string tool content — some OpenAI-compatible backends reject + * content-part arrays on tool messages, and joining providers (Google + * GenAI, `extract_text`) concatenate parts without a separator. Media- + * bearing results get the note as their own trailing text part. + * + * Together with the producers' own ``-wrapped notes, every piece of + * system-generated text inside a tool result carries the same marker, so + * the model can always tell harness information from tool data. UIs never + * see any of it — they render the raw output and style failures via the + * structured `isError` flag. + * + * Callers: the live LLM projection (`agent/context/projector.ts`) and the + * vis debugger's model view, which must mirror the live projection exactly. + */ +import type { ContentPart } from '@moonshot-ai/kosong'; + +export const TOOL_ERROR_STATUS = 'ERROR: Tool execution failed.'; +export const TOOL_EMPTY_STATUS = 'Tool output is empty.'; +export const TOOL_EMPTY_ERROR_STATUS = + 'ERROR: Tool execution failed. Tool output is empty.'; + +/** + * The plain placeholder the loop layer bakes into records for empty outputs + * (`normalizeToolResult`'s `TOOL_OUTPUT_EMPTY`). The projection recognizes + * it as empty-equivalent and emits the wrapped {@link TOOL_EMPTY_STATUS}. + */ +const TOOL_OUTPUT_EMPTY_TEXT = 'Tool output is empty.'; + +export interface RenderableToolResult { + readonly output: string | readonly ContentPart[]; + readonly note?: string | undefined; + readonly isError?: boolean | undefined; +} + +export function renderToolResultForModel(result: RenderableToolResult): ContentPart[] { + const rendered = renderStatus(result); + if (result.note === undefined || result.note.length === 0) { + return rendered; + } + const only = rendered[0]; + if (rendered.length === 1 && only?.type === 'text') { + return [textPart(`${only.text}\n${result.note}`)]; + } + return [...rendered, textPart(result.note)]; +} + +function renderStatus(result: RenderableToolResult): ContentPart[] { + const output = result.output; + + // String outputs — and their history form, a single text part — keep the + // legacy joined shape: the status prefix shares one text part with the + // output so provider serialization is unchanged. + const single = typeof output === 'string' ? output : singleTextPart(output); + if (single !== undefined) { + if (result.isError === true) { + if (single.length === 0) return [textPart(TOOL_EMPTY_ERROR_STATUS)]; + return [textPart(`${TOOL_ERROR_STATUS}\n${single}`)]; + } + return isEmptyOutputText(single) ? [textPart(TOOL_EMPTY_STATUS)] : [textPart(single)]; + } + + const parts = output as readonly ContentPart[]; + // An array with no sendable content (empty, or only empty/whitespace-only + // text blocks) gets the placeholder. Otherwise projection would drop the + // blank text blocks, leave the tool message empty, and throw on every send. + if (isEmptyEquivalentContentArray(parts)) { + return [textPart(result.isError === true ? TOOL_EMPTY_ERROR_STATUS : TOOL_EMPTY_STATUS)]; + } + if (result.isError === true) { + return [textPart(TOOL_ERROR_STATUS), ...parts]; + } + return [...parts]; +} + +function singleTextPart(output: readonly ContentPart[]): string | undefined { + const first = output[0]; + return output.length === 1 && first?.type === 'text' ? first.text : undefined; +} + +function textPart(text: string): ContentPart { + return { type: 'text', text }; +} + +function isEmptyOutputText(output: string): boolean { + return output.trim().length === 0 || output.trim() === TOOL_OUTPUT_EMPTY_TEXT; +} + +function isEmptyEquivalentContentArray(output: readonly ContentPart[]): boolean { + return output.every((part) => part.type === 'text' && part.text.trim().length === 0); +} diff --git a/packages/agent-core/src/agent/context/types.ts b/packages/agent-core/src/agent/context/types.ts index 9286a8dbf..d16b0c520 100644 --- a/packages/agent-core/src/agent/context/types.ts +++ b/packages/agent-core/src/agent/context/types.ts @@ -103,6 +103,12 @@ export type PromptOrigin = export type ContextMessage = Message & { readonly origin?: PromptOrigin | undefined; readonly isError?: boolean; + /** + * Tool-result side channel rendered to the model but never to UIs; see + * `ExecutableToolResult.note`. Appended to the projected tool message at + * the provider boundary and stripped from the wire message itself. + */ + readonly note?: string; }; export interface UserMessageRecord { diff --git a/packages/agent-core/src/index.ts b/packages/agent-core/src/index.ts index f5b84f9d9..01e159c0e 100644 --- a/packages/agent-core/src/index.ts +++ b/packages/agent-core/src/index.ts @@ -30,6 +30,8 @@ export type { SessionLogHandle, } from './logging/types'; export { USER_PROMPT_ORIGIN } from './agent/context'; +export { renderToolResultForModel } from './agent/context/tool-result-render'; +export type { RenderableToolResult } from './agent/context/tool-result-render'; export type { AgentContextData, ContextMessage, @@ -45,9 +47,11 @@ export type { } from './agent/background'; export type { ToolServices } from './tools/support/services'; -// Image compression — the input-stage helper each ingestion site (CLI paste, -// server upload resolution, ACP, ReadMediaFile, MCP) calls to shrink oversized -// images while constructing the content part. Compression is never silent: +// Image compression — prompt-ingestion sites (CLI paste, server upload +// resolution, ACP) call compressBase64ForModel / compressImageForModel per +// image while constructing the content part; the MCP tool-result pipeline +// walks whole part lists with compressImageContentParts, which returns the +// generated captions as data. Compression is never silent: // buildImageCompressionCaption renders the shared "what was compressed" note, // persistOriginalImage keeps the pre-compression bytes readable, and // cropImageForModel reads a region of an original back at full fidelity. @@ -65,6 +69,7 @@ export { } from './tools/support/image-compress'; export type { CompressAnnotateOptions, + CompressedContentParts, CompressImageOptions, CompressImageResult, CompressBase64Result, diff --git a/packages/agent-core/src/loop/tool-call.ts b/packages/agent-core/src/loop/tool-call.ts index 0f0b2d7b9..264b4ede5 100644 --- a/packages/agent-core/src/loop/tool-call.ts +++ b/packages/agent-core/src/loop/tool-call.ts @@ -659,12 +659,19 @@ function normalizeToolResult(r: ExecutableToolResult): ExecutableToolResult { output = textJoined.length > 0 ? textJoined : TOOL_OUTPUT_EMPTY; } } + // Rebuild keeps the persisted contract only: `note` rides into the record + // (the model reads it at projection), while `stopTurn`/`message` are + // loop/UI-local and are dropped here. Tools are arbitrary JS, so this is + // also where the note contract (string | undefined) is enforced: a + // malformed or empty note is discarded — the tool's actual output is + // still valid, and everything downstream trusts the contract. + const base: { output: typeof output; note?: string; truncated?: true } = { output }; + if (typeof r.note === 'string' && r.note.length > 0) base.note = r.note; + if (r.truncated === true) base.truncated = true; if (r.isError === true) { - return r.truncated === true - ? { output, isError: true, truncated: true } - : { output, isError: true }; + return { ...base, isError: true }; } - return r.truncated === true ? { output, truncated: true } : { output }; + return base; } function makeToolResult( diff --git a/packages/agent-core/src/loop/types.ts b/packages/agent-core/src/loop/types.ts index 9d290f235..63810fbb0 100644 --- a/packages/agent-core/src/loop/types.ts +++ b/packages/agent-core/src/loop/types.ts @@ -78,6 +78,14 @@ export interface ExecutableToolSuccessResult { * this to the user. */ readonly message?: string | undefined; + /** + * Optional side channel in the opposite direction of `message`: content + * that is rendered to the model but never to user-facing UIs. Routed + * verbatim — any formatting (tags, wording) is the producing tool's + * choice. Appended to the tool result as a trailing text part when the + * history is projected for the provider. + */ + readonly note?: string | undefined; /** * True when the tool has already returned a partial result because it * truncated, paged, or otherwise dropped original output. Later generic @@ -91,6 +99,8 @@ export interface ExecutableToolErrorResult { readonly isError: true; /** See {@link ExecutableToolSuccessResult.message}. */ readonly message?: string | undefined; + /** See {@link ExecutableToolSuccessResult.note}. */ + readonly note?: string | undefined; /** See {@link ExecutableToolSuccessResult.stopTurn}. */ readonly stopTurn?: boolean | undefined; /** See {@link ExecutableToolSuccessResult.truncated}. */ diff --git a/packages/agent-core/src/mcp/output.ts b/packages/agent-core/src/mcp/output.ts index 06b217936..6c5b5a777 100644 --- a/packages/agent-core/src/mcp/output.ts +++ b/packages/agent-core/src/mcp/output.ts @@ -14,7 +14,9 @@ * would silently reintroduce the very degradation the caption reports. * 4. Compress oversized inline images, announcing each compression with a * caption (original vs. sent size, readback path to the persisted - * original) so downsampling is never silent. + * original) so downsampling is never silent. The captions come back + * from the compressor as data and ride the result's `note` side + * channel — rendered to the model at projection time, never to UIs. * 5. Apply the per-part 10 MB binary cap: oversized binary parts * (image/audio/video URLs) collapse to a notice, so a single * screenshot cannot evict every text part. @@ -151,7 +153,12 @@ export async function mcpResultToExecutableOutput( result: MCPToolResult, qualifiedToolName: string, options: McpOutputOptions = {}, -): Promise<{ output: string | ContentPart[]; isError: boolean; truncated?: true }> { +): Promise<{ + output: string | ContentPart[]; + isError: boolean; + note?: string; + truncated?: true; +}> { const converted: ContentPart[] = []; for (const block of result.content) { const part = convertMCPContentBlock(block); @@ -161,18 +168,21 @@ export async function mcpResultToExecutableOutput( } const wrapped = wrapMediaOnly(converted, qualifiedToolName); - // Text budget FIRST, on the tool's own text only: captions inserted by the - // compression step below must never compete with a chatty tool's text for - // the budget — an evicted or mid-string-sliced caption silently - // reintroduces the downsampling this pipeline promises to announce. + // Text budget FIRST, on the tool's own text only: captions produced by the + // compression step below ride the `note` side channel and never compete + // with a chatty tool's text for the budget — an evicted or mid-string- + // sliced caption would silently reintroduce the downsampling this pipeline + // promises to announce. const budgeted = applyTextBudget(wrapped); // Shrink oversized images BEFORE the per-part byte cap, so a large but // compressible screenshot is downsampled and kept rather than dropped to a - // text notice. Compression is never silent: each re-encoded image gains a + // text notice. Compression is never silent: each re-encoded image yields a // caption stating what the original was, and the original bytes are // persisted (best effort, into the session's media-originals dir when // known) so the model can read detail back via ReadMediaFile + region. - // Parts that cannot be compressed pass through. + // Parts that cannot be compressed pass through. The captions come back as + // DATA (never inserted into the parts), so tool output that merely quotes + // a caption can never be mistaken for a generated one. const compressed = await compressImageContentParts(budgeted.parts, { annotate: { persistOriginal: (bytes, mimeType) => @@ -183,12 +193,15 @@ export async function mcpResultToExecutableOutput( ), }, }); - const capped = applyBinaryPartCap(compressed); + const capped = applyBinaryPartCap(compressed.parts); const truncated = budgeted.truncated || capped.truncated; const output = collapseSingleText(capped.parts); - return truncated - ? { output, isError: result.isError, truncated: true } - : { output, isError: result.isError }; + return { + output, + isError: result.isError, + note: compressed.captions.length > 0 ? compressed.captions.join('\n') : undefined, + truncated: truncated ? true : undefined, + }; } /** diff --git a/packages/agent-core/src/services/message/transcript.ts b/packages/agent-core/src/services/message/transcript.ts index 2acfa0f4c..c3d2e25ab 100644 --- a/packages/agent-core/src/services/message/transcript.ts +++ b/packages/agent-core/src/services/message/transcript.ts @@ -20,8 +20,9 @@ * - `context.append_message` → append (deferred while a tool exchange is open) * - `context.append_loop_event` → step.begin/content.part/tool.call mutate the * open assistant message; tool.result appends a - * tool message with the same `` status - * wrapping as `toolResultOutputForModel` + * tool message with the raw output plus the + * structured isError/note fields, exactly like + * `ContextMemory` history * - `context.apply_compaction` → keep the full history, append the * user-role summary marker (origin * `compaction_summary`), and recover @@ -63,13 +64,6 @@ type ContentPart = ContextMessage['content'][number]; const BLOBREF_PROTOCOL = 'blobref:'; const MISSING_MEDIA_PLACEHOLDER = '[media missing]'; -// Status strings must match agent-core's toolResultOutputForModel so the -// transcript renders tool results byte-identically to getContext().history. -const TOOL_ERROR_STATUS = 'ERROR: Tool execution failed.'; -const TOOL_EMPTY_STATUS = 'Tool output is empty.'; -const TOOL_EMPTY_ERROR_STATUS = - 'ERROR: Tool execution failed. Tool output is empty.'; -const TOOL_OUTPUT_EMPTY_TEXT = 'Tool output is empty.'; const TOOL_INTERRUPTED_ON_RESUME_OUTPUT = 'Tool execution was interrupted before its result was recorded. Do not assume the tool completed successfully.'; @@ -95,6 +89,7 @@ interface MutableMessage { toolCalls: { type: 'function'; id: string; name: string; arguments: string | null }[]; toolCallId?: string; isError?: boolean | undefined; + note?: string | undefined; origin?: ContextMessage['origin']; } @@ -139,10 +134,7 @@ export function reduceWireRecords(records: Iterable): { push({ message: { role: 'tool', - content: toolResultContent({ - output: TOOL_INTERRUPTED_ON_RESUME_OUTPUT, - isError: true, - }), + content: [{ type: 'text', text: TOOL_INTERRUPTED_ON_RESUME_OUTPUT }], toolCalls: [], toolCallId, isError: true, @@ -201,10 +193,11 @@ export function reduceWireRecords(records: Iterable): { push({ message: { role: 'tool', - content: toolResultContent(event.result), + content: rawToolResultContent(event.result.output), toolCalls: [], toolCallId: event.toolCallId, isError: event.result.isError, + note: event.result.note, }, time, }); @@ -325,35 +318,9 @@ export function reduceWireRecords(records: Iterable): { return { entries: transcript as TranscriptEntry[], foldedLength }; } -/** Mirrors agent-core's `toolResultOutputForModel` + `createToolMessage`. */ -function toolResultContent(result: ExecutableToolResult): ContentPart[] { - const output = result.output; - if (typeof output === 'string') { - let text: string; - if (result.isError === true) { - if (output.length === 0) text = TOOL_EMPTY_ERROR_STATUS; - else if (output.trimStart().startsWith('ERROR:')) text = output; - else text = `${TOOL_ERROR_STATUS}\n${output}`; - } else { - text = - output.length === 0 || output.trim() === TOOL_OUTPUT_EMPTY_TEXT - ? TOOL_EMPTY_STATUS - : output; - } - return [{ type: 'text', text }]; - } - if (output.length === 0) { - return [ - { - type: 'text', - text: result.isError === true ? TOOL_EMPTY_ERROR_STATUS : TOOL_EMPTY_STATUS, - }, - ]; - } - if (result.isError === true) { - return [{ type: 'text', text: TOOL_ERROR_STATUS }, ...output]; - } - return [...output]; +/** Mirrors `createToolMessage`: raw output verbatim — status text is added only at LLM projection. */ +function rawToolResultContent(output: ExecutableToolResult['output']): ContentPart[] { + return typeof output === 'string' ? [{ type: 'text', text: output }] : [...output]; } /** diff --git a/packages/agent-core/src/tools/builtin/file/read-media.md b/packages/agent-core/src/tools/builtin/file/read-media.md index 31b2b4c92..a46828e5d 100644 --- a/packages/agent-core/src/tools/builtin/file/read-media.md +++ b/packages/agent-core/src/tools/builtin/file/read-media.md @@ -2,7 +2,7 @@ Read media content from a file. **Tips:** - Make sure you follow the description of each tool parameter. -- A `` tag is given before the file content; it summarizes the mime type, byte size and, for images, the original pixel dimensions, and states how the image was delivered (untouched, downsampled, cropped, or native resolution). When outputting coordinates, give relative coordinates first and compute absolute coordinates from the original image size. After generating or editing media via commands or scripts, read the result back before continuing. +- A `` tag accompanies the media content; it summarizes the mime type, byte size and, for images, the original pixel dimensions, and states how the image was delivered (untouched, downsampled, cropped, or native resolution). When outputting coordinates, give relative coordinates first and compute absolute coordinates from the original image size. After generating or editing media via commands or scripts, read the result back before continuing. - Large images are downsampled by default to fit model limits, which can blur fine detail (small text, dense UI). Compute absolute coordinates from the original dimensions reported in the `` block, never by measuring the displayed copy. When the `` tag reports downsampling and you need that detail, call this tool again with the `region` parameter (original-image pixel coordinates) to view a crop at full fidelity, or set `full_resolution` to true when the whole file fits the per-image byte limit. Re-reading the same file without these parameters just reproduces the same downsampled image. - The system will notify you when there is anything wrong when reading the file. - This tool is a tool that you typically want to use in parallel. Always read multiple files in one response when possible. diff --git a/packages/agent-core/src/tools/builtin/file/read-media.ts b/packages/agent-core/src/tools/builtin/file/read-media.ts index 977467191..f654829a8 100644 --- a/packages/agent-core/src/tools/builtin/file/read-media.ts +++ b/packages/agent-core/src/tools/builtin/file/read-media.ts @@ -1,17 +1,18 @@ /** * ReadMediaFileTool — read image/video files as multi-modal content. * - * Returns a 4-part wrap: - * `[TextPart('…'), TextPart(''), - * ImageContent|VideoContent, TextPart('')]` - * and gates on the model's `image_in` / `video_in` capability. + * Returns a 3-part wrap as `output`: + * `[TextPart(''), ImageContent|VideoContent, + * TextPart('')]` + * plus a `note` side channel (rendered to the model, never to UIs), and + * gates on the model's `image_in` / `video_in` capability. * - * The leading `` block summarizes mime type, byte size and (for - * images) original pixel dimensions, states exactly how the image was - * delivered (untouched, downsampled, cropped, or native resolution) so - * compression is never silent, guides the model to derive absolute - * coordinates from the original size, and reminds it to re-read any media - * it generates or edits. + * The note — this tool wraps it in a `` block as its own wording + * choice — summarizes mime type, byte size and (for images) original pixel + * dimensions, states exactly how the image was delivered (untouched, + * downsampled, cropped, or native resolution) so compression is never + * silent, guides the model to derive absolute coordinates from the original + * size, and reminds it to re-read any media it generates or edits. * * Images support two opt-in delivery controls: `region` cuts a rectangle * (original-image pixel coordinates) out of the file so fine detail survives @@ -141,7 +142,9 @@ interface ImageDelivery { } /** - * Build the `` summary that precedes the media content. + * Build the media summary returned as the tool result's `note` (model-only + * side channel). The `` wrapping is this tool's wording choice; the + * note channel itself adds nothing. * * Carries mime type, byte size and (for images) the original pixel * dimensions, plus the delivery note above. When the dimensions are known it @@ -149,7 +152,7 @@ interface ImageDelivery { * size (crops get offset-mapping guidance instead); it always reminds the * model to re-read any media it generates or edits. */ -function buildSystemSummary(input: { +function buildMediaNote(input: { readonly kind: 'image' | 'video'; readonly mimeType: string; readonly byteSize: number; @@ -172,7 +175,7 @@ function buildSystemSummary(input: { const delivery = input.delivery; if (delivery?.kind === 'downsampled') { parts.push( - `The image below was downsampled to ${String(delivery.width)}x${String(delivery.height)} pixels ` + + `The attached image was downsampled to ${String(delivery.width)}x${String(delivery.height)} pixels ` + `(${delivery.mimeType}, ${formatByteSize(delivery.byteLength)}) to fit model limits; ` + 'fine detail may be lost.', 'To inspect fine detail, call ReadMediaFile again with the region parameter ' + @@ -411,7 +414,7 @@ export class ReadMediaFileTool implements BuiltinTool { const openText = `<${tag} path="${safePath}">`; const closeText = ``; - const systemText = buildSystemSummary({ + const note = buildMediaNote({ kind: fileType.kind, mimeType: fileType.mimeType, byteSize: stat.stSize, @@ -420,13 +423,12 @@ export class ReadMediaFileTool implements BuiltinTool { }); const output: ContentPart[] = [ - { type: 'text', text: systemText }, { type: 'text', text: openText }, mediaPart, { type: 'text', text: closeText }, ]; - return { output, isError: false }; + return { output, note, isError: false }; } catch (error) { return { isError: true, diff --git a/packages/agent-core/src/tools/builtin/file/read.ts b/packages/agent-core/src/tools/builtin/file/read.ts index d6abd5764..0c2f76be1 100644 --- a/packages/agent-core/src/tools/builtin/file/read.ts +++ b/packages/agent-core/src/tools/builtin/file/read.ts @@ -527,17 +527,15 @@ export class ReadTool implements BuiltinTool { } private finishReadResult(input: FinishReadResultInput): ExecutableToolResult { + // The status line rides the `note` side channel (model-only); `output` is + // the rendered file content and nothing else. The `` wrapping is + // this tool's wording choice. return { - output: this.finishOutput(input.renderedLines, this.finishMessage(input)), + output: input.renderedLines.join('\n'), + note: `${this.finishMessage(input)}`, }; } - private finishOutput(renderedLines: readonly string[], message: string): string { - const rendered = renderedLines.join('\n'); - const status = `${message}`; - return rendered.length > 0 ? `${rendered}\n${status}` : status; - } - private finishMessage(input: FinishReadResultInput): string { const lineCount = input.renderedLines.length; const lineWord = lineCount === 1 ? 'line' : 'lines'; diff --git a/packages/agent-core/src/tools/support/image-compress.ts b/packages/agent-core/src/tools/support/image-compress.ts index 0c640b7be..34f62ecb5 100644 --- a/packages/agent-core/src/tools/support/image-compress.ts +++ b/packages/agent-core/src/tools/support/image-compress.ts @@ -275,26 +275,41 @@ export async function compressBase64ForModel( }; } +export interface CompressedContentParts { + /** The input parts with oversized inline images re-encoded in place. */ + readonly parts: ContentPart[]; + /** + * One {@link buildImageCompressionCaption} note per re-encoded image, in + * encounter order, when `annotate` is set. Returned as data — never + * inserted into `parts` — so the caller picks the channel (the MCP path + * joins them into the tool result's `note`) and quoted caption text in + * the tool's own output can never be mistaken for a generated one. + */ + readonly captions: readonly string[]; +} + /** - * Compress any inline base64 image parts in a content-part list — the single - * helper used by the prompt-ingestion chokepoint (every client's images) and - * the MCP tool-result path. Image parts whose URL is not a `data:` URL (e.g. a - * remote http(s) image) are passed through, as are non-image parts. Best - * effort: a part that fails to compress is left unchanged. + * Compress any inline base64 image parts in a content-part list — used by + * the MCP tool-result path (prompt ingestion compresses per image with + * {@link compressBase64ForModel} while constructing the part). Image parts + * whose URL is not a `data:` URL (e.g. a remote http(s) image) are passed + * through, as are non-image parts. Best effort: a part that fails to + * compress is left unchanged. * - * With `annotate` set, every image that was actually re-encoded gains a - * {@link buildImageCompressionCaption} text part immediately before it, so the - * model knows it is looking at a downsampled copy. `annotate.persistOriginal` - * additionally saves the pre-compression bytes and puts the returned path in - * the caption so the model can read the original back; persistence failures - * degrade to a caption without a path. + * With `annotate` set, every image that was actually re-encoded gets a + * caption in {@link CompressedContentParts.captions} so the model knows it + * is looking at a downsampled copy. `annotate.persistOriginal` additionally + * saves the pre-compression bytes and puts the returned path in the caption + * so the model can read the original back; persistence failures degrade to + * a caption without a path. */ export async function compressImageContentParts( parts: readonly ContentPart[], options: CompressImageOptions & { readonly annotate?: CompressAnnotateOptions } = {}, -): Promise { +): Promise { const { annotate, ...compressOptions } = options; const out: ContentPart[] = []; + const captions: string[] = []; for (const part of parts) { if (part.type === 'image_url') { const parsed = parseImageDataUrl(part.imageUrl.url); @@ -313,9 +328,8 @@ export async function compressImageContentParts( originalPath = null; } } - out.push({ - type: 'text', - text: buildImageCompressionCaption({ + captions.push( + buildImageCompressionCaption({ original: { width: result.originalWidth, height: result.originalHeight, @@ -330,7 +344,7 @@ export async function compressImageContentParts( }, originalPath, }), - }); + ); } out.push({ type: 'image_url', @@ -342,7 +356,7 @@ export async function compressImageContentParts( } out.push(part); } - return out; + return { parts: out, captions }; } export interface CompressAnnotateOptions { @@ -558,8 +572,10 @@ export interface ImageCompressionCaptionInput { * back (via ReadMediaFile `region`) for full-fidelity detail. * * Two channels consume this note differently: - * - Tool results (MCP images) keep it inline — `` status text inside - * tool output is the established convention there. + * - Tool results (MCP images): {@link compressImageContentParts} returns + * the captions as data and the MCP output pipeline joins them into the + * result's `note` side channel (rendered to the model at projection + * time, never to UIs). * - User prompts must not render raw `` markup in the UI, so the * context layer detects the caption via * {@link extractImageCompressionCaptions} and reroutes it through the diff --git a/packages/agent-core/test/agent/context.test.ts b/packages/agent-core/test/agent/context.test.ts index 9d65556f3..432187410 100644 --- a/packages/agent-core/test/agent/context.test.ts +++ b/packages/agent-core/test/agent/context.test.ts @@ -400,7 +400,10 @@ describe('Agent context', () => { { role: 'tool', content: [ - { type: 'text', text: 'ERROR: Tool execution failed.\npermission denied' }, + { + type: 'text', + text: 'ERROR: Tool execution failed.\npermission denied', + }, ], toolCallId: 'call_error', }, @@ -412,6 +415,100 @@ describe('Agent context', () => { ]); }); + it('keeps raw tool result output in history; error status is added only in projection', () => { + const ctx = testAgent(); + ctx.configure(); + + ctx.dispatch({ + type: 'context.append_loop_event', + event: { type: 'step.begin', uuid: 's1', turnId: 't', step: 1 }, + }); + ctx.dispatch({ + type: 'context.append_loop_event', + event: { + type: 'tool.call', + uuid: 'call_raw', + turnId: 't', + step: 1, + stepUuid: 's1', + toolCallId: 'call_raw', + name: 'Run', + args: {}, + }, + }); + ctx.dispatch({ + type: 'context.append_loop_event', + event: { + type: 'tool.result', + parentUuid: 'call_raw', + toolCallId: 'call_raw', + result: { output: 'permission denied', isError: true }, + }, + }); + + // History stores the fact: the tool's own output plus the structured + // isError flag. The ERROR status line is a model-projection concern and + // must not be materialized here (UIs read history/replay verbatim). + const stored = ctx.agent.context.history.find((m) => m.toolCallId === 'call_raw')!; + expect(stored.content).toEqual([{ type: 'text', text: 'permission denied' }]); + expect(stored.isError).toBe(true); + + const projected = ctx.agent.context.messages.find((m) => m.toolCallId === 'call_raw')!; + expect(projected.content).toEqual([ + { + type: 'text', + text: 'ERROR: Tool execution failed.\npermission denied', + }, + ]); + }); + + it('carries a tool result note into history and appends it in the LLM projection', () => { + const ctx = testAgent(); + ctx.configure(); + + ctx.dispatch({ + type: 'context.append_loop_event', + event: { type: 'step.begin', uuid: 's1', turnId: 't', step: 1 }, + }); + ctx.dispatch({ + type: 'context.append_loop_event', + event: { + type: 'tool.call', + uuid: 'call_note', + turnId: 't', + step: 1, + stepUuid: 's1', + toolCallId: 'call_note', + name: 'Run', + args: {}, + }, + }); + ctx.dispatch({ + type: 'context.append_loop_event', + event: { + type: 'tool.result', + parentUuid: 'call_note', + toolCallId: 'call_note', + result: { output: 'file body', note: '10 lines read.' }, + }, + }); + + // The note rides the message as a structured field (like origin/isError): + // output stays pure data, so UIs can render it without any stripping. + const stored = ctx.agent.context.history.find((m) => m.toolCallId === 'call_note')!; + expect(stored.content).toEqual([{ type: 'text', text: 'file body' }]); + expect(stored.note).toBe('10 lines read.'); + + // The model projection joins the note into the text-only output (single + // part keeps providers on the plain-string tool content path) and drops + // the structured field from the wire message. + const projected = ctx.agent.context.messages.find((m) => m.toolCallId === 'call_note')!; + expect(projected.content).toEqual([ + { type: 'text', text: 'file body\n10 lines read.' }, + ]); + expect('note' in projected).toBe(false); + }); + it('drops empty and whitespace-only text parts in LLM projection', () => { const history: ContextMessage[] = [ { @@ -482,7 +579,7 @@ describe('Agent context', () => { expect(history[1]?.content).toEqual([{ type: 'text', text: '' }]); }); - it('rejects tool result messages left empty by LLM projection cleanup', () => { + it('heals an empty tool result into the placeholder during LLM projection', () => { const history: ContextMessage[] = [ { role: 'assistant', @@ -497,9 +594,17 @@ describe('Agent context', () => { }, ]; - expect(() => project(history)).toThrow( - 'Tool result message content cannot be empty after removing empty text blocks.', - ); + // History stores the empty output as a fact; the projection renders it + // into the model-visible placeholder so strict providers never see an + // empty tool message. + expect(project(history)).toMatchObject([ + { role: 'assistant' }, + { + role: 'tool', + content: [{ type: 'text', text: 'Tool output is empty.' }], + toolCallId: 'call_empty', + }, + ]); }); it('projects hook result messages into LLM projection', async () => { diff --git a/packages/agent-core/test/agent/records/index.test.ts b/packages/agent-core/test/agent/records/index.test.ts index 47571446b..0f43deeb2 100644 --- a/packages/agent-core/test/agent/records/index.test.ts +++ b/packages/agent-core/test/agent/records/index.test.ts @@ -161,6 +161,67 @@ describe('AgentRecords persistence metadata', () => { expect(migrated.message.toolCalls[0]?.['function']).toBeUndefined(); }); + it('replays legacy tool-baked metadata verbatim without migration', async () => { + // Pre-note records carry tool metadata inline in the output. They are + // intentionally NOT migrated: the model view stays byte-identical to + // what the model originally saw, and UIs show the legacy text as-is. + const summary = + 'Read image file. Mime type: image/png. Size: 70 bytes. ' + + 'Original dimensions: 4x2 pixels.'; + const legacyOutput = [ + { type: 'text', text: summary }, + { type: 'text', text: '' }, + { type: 'image_url', imageUrl: { url: 'data:image/png;base64,A' } }, + { type: 'text', text: '' }, + ]; + const persistence = new RecordingInMemoryAgentRecordPersistence([ + { + type: 'metadata', + protocol_version: AGENT_WIRE_PROTOCOL_VERSION, + created_at: 1, + }, + { + type: 'context.append_loop_event', + event: { type: 'step.begin', uuid: 's1', turnId: 't', step: 1 }, + } as unknown as AgentRecord, + { + type: 'context.append_loop_event', + event: { + type: 'tool.call', + uuid: 'call_media', + turnId: 't', + step: 1, + stepUuid: 's1', + toolCallId: 'call_media', + name: 'ReadMediaFile', + args: {}, + }, + } as unknown as AgentRecord, + { + type: 'context.append_loop_event', + event: { + type: 'tool.result', + parentUuid: 'call_media', + toolCallId: 'call_media', + result: { output: legacyOutput }, + }, + } as unknown as AgentRecord, + ]); + const agent = testAgent({ persistence }).agent; + + await agent.records.replay(); + + expect(persistence.rewrites).toEqual([]); + const stored = agent.context.history.find((m) => m.toolCallId === 'call_media')!; + expect(stored.note).toBeUndefined(); + expect(stored.content).toEqual(legacyOutput); + + // Projection passes the legacy content through untouched — the model + // sees exactly the bytes it saw before the note side channel existed. + const projected = agent.context.messages.find((m) => m.toolCallId === 'call_media')!; + expect(projected.content).toEqual(legacyOutput); + }); + it('warns but continues when replaying records from a newer wire version', async () => { const persistence = new InMemoryAgentRecordPersistence([ { diff --git a/packages/agent-core/test/agent/resume.test.ts b/packages/agent-core/test/agent/resume.test.ts index 2540d7340..24b14a8ae 100644 --- a/packages/agent-core/test/agent/resume.test.ts +++ b/packages/agent-core/test/agent/resume.test.ts @@ -753,7 +753,7 @@ describe('Agent resume', () => { 'user', ]); expect(textContent(llmHistory[3])).toContain( - 'ERROR: Tool execution failed.', + 'ERROR: Tool execution failed.', ); expect(textContent(llmHistory[3])).toContain( 'Tool execution was interrupted before its result was recorded', diff --git a/packages/agent-core/test/agent/tool-result-render.test.ts b/packages/agent-core/test/agent/tool-result-render.test.ts new file mode 100644 index 000000000..17afd4892 --- /dev/null +++ b/packages/agent-core/test/agent/tool-result-render.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, it } from 'vitest'; + +import { renderToolResultForModel } from '../../src/agent/context/tool-result-render'; + +const text = (t: string) => ({ type: 'text', text: t }) as const; + +const ERROR_STATUS = 'ERROR: Tool execution failed.'; +const EMPTY_STATUS = 'Tool output is empty.'; +const EMPTY_ERROR_STATUS = 'ERROR: Tool execution failed. Tool output is empty.'; + +describe('renderToolResultForModel', () => { + describe('string output (and its single-text-part history form)', () => { + it('passes successful output through unchanged', () => { + expect(renderToolResultForModel({ output: 'hello' })).toEqual([text('hello')]); + expect(renderToolResultForModel({ output: [text('hello')] })).toEqual([text('hello')]); + }); + + it('prefixes the wrapped error status on a newline', () => { + expect(renderToolResultForModel({ output: 'permission denied', isError: true })).toEqual([ + text(`${ERROR_STATUS}\npermission denied`), + ]); + }); + + it('adds the status uniformly, even when tool output already starts with ERROR:', () => { + // The wrapper is the harness verdict; the tool's own "ERROR:" + // text is data. Every failed call gets exactly one wrapped status, so + // the model never has to guess whether a failure was flagged. + expect(renderToolResultForModel({ output: 'ERROR: no such file', isError: true })).toEqual([ + text(`${ERROR_STATUS}\nERROR: no such file`), + ]); + }); + + it('replaces an empty error output with the combined status', () => { + expect(renderToolResultForModel({ output: '', isError: true })).toEqual([ + text(EMPTY_ERROR_STATUS), + ]); + }); + + it('replaces empty or whitespace-only success output with the placeholder', () => { + expect(renderToolResultForModel({ output: '' })).toEqual([text(EMPTY_STATUS)]); + expect(renderToolResultForModel({ output: ' \n ' })).toEqual([text(EMPTY_STATUS)]); + }); + + it('recognizes the plain record placeholder and emits the wrapped form', () => { + // The loop layer writes the plain placeholder into records + // (normalizeToolResult); the projection upgrades it to the wrapped + // system status rather than double-wrapping or passing it as data. + expect(renderToolResultForModel({ output: 'Tool output is empty.' })).toEqual([ + text(EMPTY_STATUS), + ]); + }); + }); + + describe('content-part array output', () => { + it('passes a media-bearing array through unchanged on success', () => { + const parts = [ + text(''), + { type: 'image_url', imageUrl: { url: 'data:image/png;base64,x' } } as const, + ]; + expect(renderToolResultForModel({ output: parts })).toEqual(parts); + }); + + it('prepends the wrapped error status as its own part on a multi-part error', () => { + const parts = [text('a'), text('b')]; + expect(renderToolResultForModel({ output: parts, isError: true })).toEqual([ + text(ERROR_STATUS), + ...parts, + ]); + }); + + it('collapses an empty-equivalent array to the placeholder', () => { + expect(renderToolResultForModel({ output: [] })).toEqual([text(EMPTY_STATUS)]); + expect(renderToolResultForModel({ output: [text(' \n')] })).toEqual([ + text(EMPTY_STATUS), + ]); + expect(renderToolResultForModel({ output: [text('')], isError: true })).toEqual([ + text(EMPTY_ERROR_STATUS), + ]); + }); + }); + + describe('note', () => { + it('joins the note into a text-only result with a newline, keeping one part', () => { + // Text-only results must stay a single text part: providers serialize + // that as plain string content (some OpenAI-compatible backends reject + // arrays on tool messages), and joining providers keep the separator. + expect( + renderToolResultForModel({ output: 'body', note: 'meta' }), + ).toEqual([text('body\nmeta')]); + }); + + it('does not wrap or alter the note text', () => { + expect(renderToolResultForModel({ output: 'body', note: 'plain words' })).toEqual([ + text('body\nplain words'), + ]); + }); + + it('appends the note as its own part after media-bearing output', () => { + const parts = [ + text(''), + { type: 'image_url', imageUrl: { url: 'data:image/png;base64,x' } } as const, + ]; + expect( + renderToolResultForModel({ output: parts, note: 'meta' }), + ).toEqual([...parts, text('meta')]); + }); + + it('joins the note after the error status and after the empty placeholder', () => { + expect( + renderToolResultForModel({ output: 'oops', isError: true, note: 'n' }), + ).toEqual([text(`${ERROR_STATUS}\noops\nn`)]); + expect(renderToolResultForModel({ output: '', note: 'n' })).toEqual([ + text(`${EMPTY_STATUS}\nn`), + ]); + }); + + it('ignores an empty note', () => { + expect(renderToolResultForModel({ output: 'body', note: '' })).toEqual([text('body')]); + }); + }); +}); diff --git a/packages/agent-core/test/loop/tool-call.e2e.test.ts b/packages/agent-core/test/loop/tool-call.e2e.test.ts index 654fabe26..368d5b08d 100644 --- a/packages/agent-core/test/loop/tool-call.e2e.test.ts +++ b/packages/agent-core/test/loop/tool-call.e2e.test.ts @@ -97,6 +97,58 @@ describe('runTurn — tool-call behaviour', () => { expect(trs[0]?.result.isError).toBeUndefined(); }); + it('preserves a tool result note through normalization into the recorded event', async () => { + const blocks = new ContentBlocksTool({ + output: 'payload', + note: 'meta for the model', + }); + const { context } = await runTurn({ + tools: [blocks], + responses: [ + makeToolUseResponse([makeToolCall('blocks', {}, 'tc-note')]), + makeEndTurnResponse('done'), + ], + }); + + const result = context.toolResults()[0]?.result; + expect(result?.output).toBe('payload'); + // note is part of the persisted result contract (unlike stopTurn/message, + // which normalization drops before the record is written). + expect(result?.note).toBe('meta for the model'); + }); + + it('enforces the note contract (string | undefined) at the normalization boundary', async () => { + // Tools are arbitrary JS: a malformed note (null, number, object, empty + // string) must never reach the record — everything downstream (history, + // projection, vis) trusts the contract instead of re-validating. + const malformed = [null, 42, { text: 'x' }, '']; + const tools = malformed.map( + (note, i) => + new ContentBlocksTool({ output: `payload-${String(i)}`, note } as never), + ); + for (const [i, tool] of tools.entries()) { + Object.defineProperty(tool, 'name', { value: `blocks${String(i)}` }); + } + + const { context } = await runTurn({ + tools, + responses: [ + makeToolUseResponse( + tools.map((tool, i) => makeToolCall(tool.name, {}, `tc-bad-${String(i)}`)), + ), + makeEndTurnResponse('done'), + ], + }); + + const results = context.toolResults(); + expect(results).toHaveLength(malformed.length); + for (const [i, entry] of results.entries()) { + expect(entry.result.output).toBe(`payload-${String(i)}`); + expect(entry.result.isError).toBeUndefined(); + expect('note' in entry.result).toBe(false); + } + }); + it('skips side-effecting tools when usage recording stops the turn', async () => { const echo = new EchoTool(); const { result, sink, llm } = await runTurn({ diff --git a/packages/agent-core/test/mcp/output.test.ts b/packages/agent-core/test/mcp/output.test.ts index 68894ef90..11edc738c 100644 --- a/packages/agent-core/test/mcp/output.test.ts +++ b/packages/agent-core/test/mcp/output.test.ts @@ -332,7 +332,7 @@ describe('mcpResultToExecutableOutput', () => { { type: 'text', text: 'A'.repeat(100_000) }, { type: 'image_url', imageUrl: { url: 'data:image/png;base64,' + 'B'.repeat(500_000) } }, ]); - expect(out).not.toHaveProperty('truncated'); + expect(out.truncated).toBeUndefined(); }); test('downsamples an oversized real image instead of leaving it full-size', async () => { @@ -359,7 +359,7 @@ describe('mcpResultToExecutableOutput', () => { expect(joined).not.toContain('image_url dropped'); }); - test('annotates a downsampled image with a caption and a readable original', async () => { + test('annotates a downsampled image with a caption note and a readable original', async () => { const bigBytes = Buffer.from( await new Jimp({ width: 2600, height: 2600, color: 0x3366ccff }).getBuffer('image/png'), ); @@ -369,19 +369,19 @@ describe('mcpResultToExecutableOutput', () => { 'mcp__s__shot', ); + // The caption rides the `note` side channel (model-only), keeping its + // `` wrapping; the output itself carries only the media. + expect(out.note).toContain('Image compressed'); + expect(out.note).toContain('2600x2600'); const parts = out.output as ContentPart[]; - const captionIndex = parts.findIndex( - (p) => p.type === 'text' && p.text.includes('Image compressed'), + expect(parts.some((p) => p.type === 'text' && p.text.includes('Image compressed'))).toBe( + false, ); - expect(captionIndex).toBeGreaterThanOrEqual(0); - const caption = (parts[captionIndex] as { text: string }).text; - expect(caption).toContain('2600x2600'); - // The caption sits immediately before the image it describes. - expect(parts[captionIndex + 1]?.type).toBe('image_url'); + expect(parts.some((p) => p.type === 'image_url')).toBe(true); // The caption points at a persisted copy of the ORIGINAL bytes, so the // model can read fine detail back with ReadMediaFile + region. - const pathMatch = /saved at "([^"]+)"/.exec(caption); + const pathMatch = /saved at "([^"]+)"/.exec(out.note ?? ''); expect(pathMatch).not.toBeNull(); const persisted = await readFile(pathMatch![1]!); expect(persisted.equals(bigBytes)).toBe(true); @@ -398,6 +398,7 @@ describe('mcpResultToExecutableOutput', () => { 'mcp__s__shot', ); + expect(out.note).toBeUndefined(); const parts = out.output as ContentPart[]; const joined = parts.map((p) => (p.type === 'text' ? p.text : '')).join(''); expect(joined).not.toContain('Image compressed'); @@ -415,10 +416,7 @@ describe('mcpResultToExecutableOutput', () => { { originalsDir: dir }, ); - const parts = out.output as ContentPart[]; - const caption = parts.find((p) => p.type === 'text' && p.text.includes('Image compressed')); - if (caption?.type !== 'text') throw new Error('expected a compression caption'); - const pathMatch = /saved at "([^"]+)"/.exec(caption.text); + const pathMatch = /saved at "([^"]+)"/.exec(out.note ?? ''); expect(pathMatch).not.toBeNull(); // The original lands inside the session-scoped dir, not the tmp cache. expect(pathMatch![1]!.startsWith(dir)).toBe(true); @@ -453,14 +451,66 @@ describe('mcpResultToExecutableOutput', () => { const toolText = parts[0]; if (toolText?.type !== 'text') throw new Error('expected the tool text part first'); expect(toolText.text).toContain('Output truncated'); - // …and the caption survives INTACT, still pointing at the original. - const caption = parts.find((p) => p.type === 'text' && p.text.includes('Image compressed')); - if (caption?.type !== 'text') throw new Error('expected a compression caption'); - expect(caption.text).toMatch(/<\/system>$/); - expect(caption.text).toContain('saved at'); + // …and the caption survives INTACT in the note, still pointing at the + // original — the note channel is exempt from the text budget by + // construction. + expect(out.note).toMatch(/<\/system>$/); + expect(out.note).toContain('saved at'); await rm(dir, { recursive: true, force: true }); }); + test('keeps MCP text that quotes a compression caption in the output', async () => { + // Captions reach the note as structured data straight from the + // compressor — never by pattern-matching text — so tool output that + // merely QUOTES a caption (a doc, a log, a test fixture) stays verbatim. + const quoted = + 'Image compressed to fit model limits: original 100x100 image/png (1.0 MB) -> ' + + 'sent 50x50 image/jpeg (100 KB). Fine detail may be lost. ' + + 'The uncompressed original was not preserved.'; + const small = Buffer.from( + await new Jimp({ width: 32, height: 32, color: 0x3366ccff }).getBuffer('image/png'), + ).toString('base64'); + + const out = await mcpResultToExecutableOutput( + result([ + { type: 'text', text: `doc quoting a caption: ${quoted}` }, + { type: 'image', data: small, mimeType: 'image/png' }, + ]), + 'mcp__s__t', + ); + + expect(out.note).toBeUndefined(); + const parts = out.output as ContentPart[]; + const joined = parts.map((p) => (p.type === 'text' ? p.text : '')).join(''); + expect(joined).toContain(quoted); + }); + + test('separates a real compression caption from quoted caption text', async () => { + const quoted = + 'Image compressed to fit model limits: original 100x100 image/png (1.0 MB) -> ' + + 'sent 50x50 image/jpeg (100 KB). Fine detail may be lost. ' + + 'The uncompressed original was not preserved.'; + const big = Buffer.from( + await new Jimp({ width: 2600, height: 2600, color: 0x3366ccff }).getBuffer('image/png'), + ).toString('base64'); + + const out = await mcpResultToExecutableOutput( + result([ + { type: 'text', text: quoted }, + { type: 'image', data: big, mimeType: 'image/png' }, + ]), + 'mcp__s__t', + ); + + // The real caption (2600x2600) rides the note; the quoted one (100x100) + // is tool output and stays where the tool put it. + expect(out.note).toContain('2600x2600'); + expect(out.note).not.toContain('100x100'); + const parts = out.output as ContentPart[]; + const joined = parts.map((p) => (p.type === 'text' ? p.text : '')).join(''); + expect(joined).toContain('100x100'); + }); + test('does not slice the caption when the budget is nearly exhausted', async () => { // 99,900 chars of tool text fit the budget on their own; the caption // must not be charged the remaining 100 chars and sliced mid-string @@ -482,11 +532,9 @@ describe('mcpResultToExecutableOutput', () => { const parts = out.output as ContentPart[]; // The tool text fits — nothing is truncated at all. expect(out.truncated).toBeUndefined(); - const caption = parts.find((p) => p.type === 'text' && p.text.includes('Image compressed')); - if (caption?.type !== 'text') throw new Error('expected a compression caption'); - expect(caption.text).toMatch(/^Image compressed/); - expect(caption.text).toMatch(/<\/system>$/); - expect(caption.text).toContain('saved at'); + expect(out.note).toMatch(/^Image compressed/); + expect(out.note).toMatch(/<\/system>$/); + expect(out.note).toContain('saved at'); const joined = parts.map((p) => (p.type === 'text' ? p.text : '')).join(''); expect(joined).not.toContain('Output truncated'); await rm(dir, { recursive: true, force: true }); diff --git a/packages/agent-core/test/services/message-service.test.ts b/packages/agent-core/test/services/message-service.test.ts index 1e34acb15..3a1c46077 100644 --- a/packages/agent-core/test/services/message-service.test.ts +++ b/packages/agent-core/test/services/message-service.test.ts @@ -359,3 +359,28 @@ describe('MessageService', () => { failingImpl.dispose(); }); }); + + +describe('toProtocolMessage tool-result output passthrough', () => { + it('flattens tool result text verbatim — no stripping, tool metadata rides `note`', () => { + // Tool metadata no longer travels inside `output` (producers put it on + // the result's `note` side channel), so the protocol mapper must not eat + // content that merely contains a literal tag. + const toolMessage: ContextMessage = { + role: 'tool', + toolCallId: 'call_1', + content: [ + { type: 'text', text: 'literal text from a user file' }, + { type: 'text', text: '' }, + { type: 'image_url', imageUrl: { url: 'data:image/png;base64,A' } }, + { type: 'text', text: '' }, + ], + toolCalls: [], + }; + const [part] = toProtocolMessage(SESSION_ID, 0, toolMessage, SESSION_CREATED_AT).content; + expect(part?.type).toBe('tool_result'); + const output = (part as { output: string }).output; + expect(output).toContain('literal text from a user file'); + expect(output).toContain(''); + }); +}); diff --git a/packages/agent-core/test/services/message-transcript.test.ts b/packages/agent-core/test/services/message-transcript.test.ts index 5e3a3155c..c2655bf9d 100644 --- a/packages/agent-core/test/services/message-transcript.test.ts +++ b/packages/agent-core/test/services/message-transcript.test.ts @@ -375,9 +375,10 @@ describe('reduceWireRecords', () => { // Synthetic result spliced in place (index 2), before the deferred prompt. expect(entries[2]!.message.toolCallId).toBe('call_interrupted'); expect(entries[2]!.message.isError).toBe(true); + // Raw fact, like ContextMemory history: the ERROR status prefix is a + // model-projection concern, not part of the stored message. expect(textOf(entries[2]!.message)).toBe( - 'ERROR: Tool execution failed.\n' + - 'Tool execution was interrupted before its result was recorded. ' + + 'Tool execution was interrupted before its result was recorded. ' + 'Do not assume the tool completed successfully.', ); expect(textOf(entries[3]!.message)).toBe('keep going'); @@ -467,7 +468,7 @@ describe('reduceWireRecords', () => { expect(foldedLength).toBe(2); }); - it('wraps tool errors and empty outputs with statuses like agent-core', () => { + it('keeps raw tool output with the structured isError/note fields like agent-core history', () => { const { entries } = reduceWireRecords([ loopEvent({ type: 'step.begin', uuid: 's1', turnId: 't', step: 0 }), ...['call_err', 'call_empty'].map((toolCallId) => @@ -492,14 +493,13 @@ describe('reduceWireRecords', () => { type: 'tool.result', parentUuid: 's1', toolCallId: 'call_empty', - result: { output: '' }, + result: { output: '', note: 'meta' }, }), ]); - expect(textOf(entries[1]!.message)).toBe( - 'ERROR: Tool execution failed.\nboom', - ); + expect(textOf(entries[1]!.message)).toBe('boom'); expect(entries[1]!.message.isError).toBe(true); - expect(textOf(entries[2]!.message)).toBe('Tool output is empty.'); + expect(textOf(entries[2]!.message)).toBe(''); + expect(entries[2]!.message.note).toBe('meta'); }); }); diff --git a/packages/agent-core/test/tools/builtin-current.test.ts b/packages/agent-core/test/tools/builtin-current.test.ts index 3e6aecb65..977d32b83 100644 --- a/packages/agent-core/test/tools/builtin-current.test.ts +++ b/packages/agent-core/test/tools/builtin-current.test.ts @@ -136,12 +136,9 @@ describe('current builtin file and shell tools', () => { }); const result = await executeTool(tool, context({ path: '/workspace/a.txt' })); - expect(result.output).toBe( - [ - '1\talpha', - '2\tbeta', - '2 lines read from file starting from line 1. Total lines in file: 2. End of file reached.', - ].join('\n'), + expect(result.output).toBe(['1\talpha', '2\tbeta'].join('\n')); + expect(result.note).toBe( + '2 lines read from file starting from line 1. Total lines in file: 2. End of file reached.', ); }); diff --git a/packages/agent-core/test/tools/image-compress.test.ts b/packages/agent-core/test/tools/image-compress.test.ts index cd26adedb..3eb3f32f7 100644 --- a/packages/agent-core/test/tools/image-compress.test.ts +++ b/packages/agent-core/test/tools/image-compress.test.ts @@ -22,7 +22,7 @@ * honors skipResize with a hard byte-budget failure * - caption: buildImageCompressionCaption renders a consistent * `` note (dims, sizes, readback path) - * - annotate: compressImageContentParts can insert that caption next to + * - annotate: compressImageContentParts can collect that caption for * each compressed image and persist the original via a callback */ @@ -327,7 +327,7 @@ describe('compressImageContentParts', () => { { type: 'text' as const, text: 'look at this' }, { type: 'image_url' as const, imageUrl: { url: dataUrl('image/png', big) } }, ]; - const out = await compressImageContentParts(parts); + const { parts: out } = await compressImageContentParts(parts); expect(out[0]).toEqual({ type: 'text', text: 'look at this' }); const imagePart = out[1]; @@ -342,7 +342,7 @@ describe('compressImageContentParts', () => { const small = await solidPng(48, 48); const url = dataUrl('image/png', small); const parts = [{ type: 'image_url' as const, imageUrl: { url } }]; - const out = await compressImageContentParts(parts); + const { parts: out } = await compressImageContentParts(parts); expect(out[0]).toEqual({ type: 'image_url', imageUrl: { url } }); }); @@ -350,7 +350,7 @@ describe('compressImageContentParts', () => { const parts = [ { type: 'image_url' as const, imageUrl: { url: 'https://example.com/pic.png' } }, ]; - const out = await compressImageContentParts(parts); + const { parts: out } = await compressImageContentParts(parts); expect(out[0]).toEqual({ type: 'image_url', imageUrl: { url: 'https://example.com/pic.png' } }); }); @@ -359,7 +359,7 @@ describe('compressImageContentParts', () => { const parts = [ { type: 'image_url' as const, imageUrl: { url: dataUrl('image/png', big), id: 'att-1' } }, ]; - const out = await compressImageContentParts(parts); + const { parts: out } = await compressImageContentParts(parts); const imagePart = out[0]; if (imagePart?.type !== 'image_url') throw new Error('expected image_url'); expect(imagePart.imageUrl.id).toBe('att-1'); @@ -648,7 +648,7 @@ describe('compressImageContentParts — annotate', () => { return `data:${mime};base64,${Buffer.from(bytes).toString('base64')}`; } - it('inserts a caption before a compressed image and persists the original', async () => { + it('collects a caption for a compressed image and persists the original', async () => { const big = await solidPng(2600, 2600); const persisted: { bytes: Uint8Array; mimeType: string }[] = []; const parts = [{ type: 'image_url' as const, imageUrl: { url: dataUrl('image/png', big) } }]; @@ -661,25 +661,26 @@ describe('compressImageContentParts — annotate', () => { }, }); - expect(out).toHaveLength(2); - const caption = out[0]; - if (caption?.type !== 'text') throw new Error('expected caption text part'); - expect(caption.text).toContain('2600x2600'); - expect(caption.text).toContain('/tmp/originals/big.png'); - expect(out[1]?.type).toBe('image_url'); + // The caption comes back as data, never inserted into the parts. + expect(out.parts).toHaveLength(1); + expect(out.parts[0]?.type).toBe('image_url'); + expect(out.captions).toHaveLength(1); + expect(out.captions[0]).toContain('2600x2600'); + expect(out.captions[0]).toContain('/tmp/originals/big.png'); expect(persisted).toHaveLength(1); expect(persisted[0]?.mimeType).toBe('image/png'); expect(persisted[0]?.bytes.length).toBe(big.length); }); - it('adds no caption when the image passes through unchanged', async () => { + it('collects no caption when the image passes through unchanged', async () => { const small = await solidPng(48, 48); const url = dataUrl('image/png', small); const out = await compressImageContentParts([{ type: 'image_url' as const, imageUrl: { url } }], { annotate: {}, }); - expect(out).toHaveLength(1); - expect(out[0]).toEqual({ type: 'image_url', imageUrl: { url } }); + expect(out.parts).toHaveLength(1); + expect(out.parts[0]).toEqual({ type: 'image_url', imageUrl: { url } }); + expect(out.captions).toEqual([]); }); it('captions without a path when persistence fails', async () => { @@ -688,9 +689,8 @@ describe('compressImageContentParts — annotate', () => { const out = await compressImageContentParts(parts, { annotate: { persistOriginal: () => Promise.resolve(null) }, }); - expect(out).toHaveLength(2); - const caption = out[0]; - if (caption?.type !== 'text') throw new Error('expected caption text part'); - expect(caption.text).toMatch(/not preserved/i); + expect(out.parts).toHaveLength(1); + expect(out.captions).toHaveLength(1); + expect(out.captions[0]).toMatch(/not preserved/i); }); }); diff --git a/packages/agent-core/test/tools/read-file.test.ts b/packages/agent-core/test/tools/read-file.test.ts index 35337bb70..abe4340ea 100644 --- a/packages/agent-core/test/tools/read-file.test.ts +++ b/packages/agent-core/test/tools/read-file.test.ts @@ -66,6 +66,6 @@ describe('ReadTool — total-lines message channel', () => { expect(result.isError).toBeFalsy(); expect(result.output).toContain('3\tc'); - expect(result.output).toContain('Total lines in file: 5.'); + expect(result.note).toContain('Total lines in file: 5.'); }); }); diff --git a/packages/agent-core/test/tools/read-media.test.ts b/packages/agent-core/test/tools/read-media.test.ts index 9baf4c2fb..b04adde77 100644 --- a/packages/agent-core/test/tools/read-media.test.ts +++ b/packages/agent-core/test/tools/read-media.test.ts @@ -77,6 +77,14 @@ function outputParts(result: ExecutableToolResult): ContentPart[] { return result.output as ContentPart[]; } +// The media summary rides the result's `note` side channel (rendered to the +// model at projection time, never to UIs); the tool keeps its own `` +// wrapping as a wording choice. +function noteText(result: ExecutableToolResult): string { + expect(typeof result.note).toBe('string'); + return result.note as string; +} + describe('ReadMediaFileTool', () => { it('has name, parameters, and path-scoped resource accesses', () => { const tool = makeReadMediaTool(); @@ -123,7 +131,7 @@ describe('ReadMediaFileTool', () => { ).toThrow(/image_in or video_in/); }); - it('returns a system/text/image/text wrap for PNG files', async () => { + it('returns a text/image/text wrap plus a note for PNG files', async () => { const data = Buffer.concat([PNG_HEADER, Buffer.from('pngdata')]); const tool = makeReadMediaTool({ stat: vi.fn().mockResolvedValue({ ...DEFAULT_STAT, stSize: data.length }), @@ -137,16 +145,15 @@ describe('ReadMediaFileTool', () => { signal, }); + expect(noteText(result)).toMatch(/^.*<\/system>$/s); const parts = outputParts(result); - expect(parts).toHaveLength(4); - expect(parts[0]).toMatchObject({ type: 'text' }); - expect((parts[0] as { text: string }).text).toMatch(/^.*<\/system>$/s); - expect(parts[1]).toEqual({ type: 'text', text: '' }); - expect(parts[2]).toMatchObject({ type: 'image_url' }); - expect((parts[2] as { imageUrl: { url: string } }).imageUrl.url).toBe( + expect(parts).toHaveLength(3); + expect(parts[0]).toEqual({ type: 'text', text: '' }); + expect(parts[1]).toMatchObject({ type: 'image_url' }); + expect((parts[1] as { imageUrl: { url: string } }).imageUrl.url).toBe( `data:image/png;base64,${data.toString('base64')}`, ); - expect(parts[3]).toEqual({ type: 'text', text: '' }); + expect(parts[2]).toEqual({ type: 'text', text: '' }); }); it('emits a summary with mime type and byte size for images', async () => { @@ -163,8 +170,7 @@ describe('ReadMediaFileTool', () => { signal, }); - const parts = outputParts(result); - const systemText = (parts[0] as { text: string }).text; + const systemText = noteText(result); expect(systemText).toContain('image/png'); expect(systemText).toContain(`${String(data.length)} bytes`); // The re-read reminder is included regardless of dimensions. @@ -190,8 +196,7 @@ describe('ReadMediaFileTool', () => { signal, }); - const parts = outputParts(result); - const systemText = (parts[0] as { text: string }).text; + const systemText = noteText(result); expect(systemText).toContain('4x2'); // With the original size known, the coordinate guidance is included. expect(systemText).toMatch(/relative coordinates first/i); @@ -215,8 +220,7 @@ describe('ReadMediaFileTool', () => { signal, }); - const parts = outputParts(result); - const systemText = (parts[0] as { text: string }).text; + const systemText = noteText(result); // mime type and byte size are still reported … expect(systemText).toContain('image/png'); expect(systemText).toContain(`${String(data.length)} bytes`); @@ -243,8 +247,7 @@ describe('ReadMediaFileTool', () => { signal, }); - const parts = outputParts(result); - const systemText = (parts[0] as { text: string }).text; + const systemText = noteText(result); expect(systemText).toContain('video/mp4'); expect(systemText).toContain(`${String(MP4_HEADER.length)} bytes`); // The re-read reminder is included for videos too. @@ -266,8 +269,8 @@ describe('ReadMediaFileTool', () => { }); const parts = outputParts(result); - expect(parts[1]).toEqual({ type: 'text', text: '' }); - expect((parts[2] as { imageUrl: { url: string } }).imageUrl.url).toContain('image/png'); + expect(parts[0]).toEqual({ type: 'text', text: '' }); + expect((parts[1] as { imageUrl: { url: string } }).imageUrl.url).toContain('image/png'); }); it('expands leading tilde paths using the kaos home directory', async () => { @@ -288,7 +291,7 @@ describe('ReadMediaFileTool', () => { const parts = outputParts(result); expect(readBytes).toHaveBeenCalledWith('/home/test/images/sample.png', MEDIA_SNIFF_BYTES); expect(readBytes).toHaveBeenCalledWith('/home/test/images/sample.png'); - expect(parts[1]).toEqual({ type: 'text', text: '' }); + expect(parts[0]).toEqual({ type: 'text', text: '' }); }); it('returns a text/video/text wrap for MP4 files', async () => { @@ -307,16 +310,15 @@ describe('ReadMediaFileTool', () => { signal, }); + expect(noteText(result)).toMatch(/^.*<\/system>$/s); const parts = outputParts(result); - expect(parts).toHaveLength(4); - expect(parts[0]).toMatchObject({ type: 'text' }); - expect((parts[0] as { text: string }).text).toMatch(/^.*<\/system>$/s); - expect(parts[1]).toEqual({ type: 'text', text: '' }); }); it('falls back to a media extension when the header cannot be sniffed', async () => { @@ -334,8 +336,8 @@ describe('ReadMediaFileTool', () => { }); const parts = outputParts(result); - expect(parts[1]).toEqual({ type: 'text', text: '