From 85b4c7a25ebb62cfa1b4de3c31fb1382e26ec739 Mon Sep 17 00:00:00 2001 From: Kaiyi Date: Mon, 6 Jul 2026 15:49:58 +0800 Subject: [PATCH 1/9] 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. --- .changeset/hide-tool-result-system-notes.md | 5 ++ .changeset/media-chip-original-dimensions.md | 5 ++ .../messages/tool-renderers/media.ts | 4 +- .../messages/tool-renderers/truncated.ts | 3 +- .../messages/tool-renderers/media.test.ts | 5 +- .../messages/tool-renderers/truncated.test.ts | 11 ++++ packages/agent-core/src/agent/turn/index.ts | 3 +- packages/agent-core/src/index.ts | 1 + .../src/services/message/message.ts | 7 +-- .../agent-core/src/utils/strip-system-tags.ts | 36 +++++++++++++ .../test/services/message-service.test.ts | 23 +++++++++ .../test/utils/strip-system-tags.test.ts | 51 +++++++++++++++++++ packages/node-sdk/src/index.ts | 4 ++ 13 files changed, 150 insertions(+), 8 deletions(-) create mode 100644 .changeset/hide-tool-result-system-notes.md create mode 100644 .changeset/media-chip-original-dimensions.md create mode 100644 packages/agent-core/src/utils/strip-system-tags.ts create mode 100644 packages/agent-core/test/utils/strip-system-tags.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 0000000000..78ea804410 --- /dev/null +++ b/.changeset/hide-tool-result-system-notes.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix tool-result `` notes being rendered as plain text in the terminal and web UIs. diff --git a/.changeset/media-chip-original-dimensions.md b/.changeset/media-chip-original-dimensions.md new file mode 100644 index 0000000000..1ac5b47bc4 --- /dev/null +++ b/.changeset/media-chip-original-dimensions.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Show original image dimensions in the ReadMediaFile tool chip. 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 100968daf8..b754c9be9b 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 @@ -31,7 +31,7 @@ export interface ReadMediaSummary { } const PATH_TAG_RE = /^<(image|video)\s+path="([^"]+)">$/; -const ORIGINAL_SIZE_RE = /original size\s+(\d+x\d+px)/; +const ORIGINAL_SIZE_RE = /Original dimensions:\s*(\d+x\d+)\s*pixels/; const DATA_URL_RE = /^data:([^;]+);base64,(.*)$/s; function bytesFromBase64(b64: string): number { @@ -72,7 +72,7 @@ export function parseReadMediaOutput(output: string): ReadMediaSummary | null { continue; } const size = ORIGINAL_SIZE_RE.exec(text); - if (size) originalSize = size[1]; + if (size) originalSize = `${size[1]}px`; continue; } diff --git a/apps/kimi-code/src/tui/components/messages/tool-renderers/truncated.ts b/apps/kimi-code/src/tui/components/messages/tool-renderers/truncated.ts index 036ae0a204..a464395353 100644 --- a/apps/kimi-code/src/tui/components/messages/tool-renderers/truncated.ts +++ b/apps/kimi-code/src/tui/components/messages/tool-renderers/truncated.ts @@ -1,3 +1,4 @@ +import { stripSystemTags } from '@moonshot-ai/kimi-code-sdk'; import { Text, truncateToWidth, type Component } from '@moonshot-ai/pi-tui'; import { currentTheme } from '#/tui/theme'; @@ -51,7 +52,7 @@ export class TruncatedOutputComponent implements Component { this.indent = options.indent ?? DEFAULT_INDENT; this.expandHint = options.expandHint ?? true; this.tail = options.tail ?? false; - const cleaned = trimTrailingEmptyLines(output.split('\n')).join('\n'); + const cleaned = stripSystemTags(trimTrailingEmptyLines(output.split('\n')).join('\n')); this.textComponent = new Text( options.isError ? currentTheme.fg('error', cleaned) : currentTheme.dim(cleaned), this.indent, 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 e56cb8e0ee..d8abcd1fa7 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,10 @@ 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).` }, + { + type: 'text', + text: `ReadMediaFile summary: Read image file. Mime type: ${mime}. Size: 70 bytes. Original dimensions: 1x1 pixels.`, + }, ]); } 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 ecdf2a1f58..770a79a02b 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,15 @@ describe('TruncatedOutputComponent', () => { expect(visibleWidth(line)).toBeLessThanOrEqual(37); } }); + + it('strips blocks from output before rendering', () => { + const component = new TruncatedOutputComponent( + 'Read image file. Mime type: image/png.\n', + { expanded: true, isError: false }, + ); + const out = strip(component.render(80).join('\n')); + expect(out).not.toContain(''); + expect(out).not.toContain('Read image file'); + expect(out).toContain(''); + }); }); diff --git a/packages/agent-core/src/agent/turn/index.ts b/packages/agent-core/src/agent/turn/index.ts index db4904a347..f5d77123f6 100644 --- a/packages/agent-core/src/agent/turn/index.ts +++ b/packages/agent-core/src/agent/turn/index.ts @@ -23,6 +23,7 @@ import { makeErrorPayload, toKimiErrorPayload, } from '#/errors'; +import { stripSystemFromOutput } from '#/utils/strip-system-tags'; import { isAbortError, isMaxStepsExceededError } from '../../loop/errors'; import { createLoopEventDispatcher, @@ -1127,7 +1128,7 @@ function mapLoopEvent(event: LoopEvent, turnId: number): AgentEvent | undefined type: 'tool.result', turnId, toolCallId: event.toolCallId, - output: event.result.output, + output: stripSystemFromOutput(event.result.output), isError: event.result.isError, }; case 'turn.interrupted': diff --git a/packages/agent-core/src/index.ts b/packages/agent-core/src/index.ts index f5b84f9d93..6158c7d253 100644 --- a/packages/agent-core/src/index.ts +++ b/packages/agent-core/src/index.ts @@ -18,6 +18,7 @@ export { export { resolveLoggingConfig } from './logging/resolve-config'; export type { ResolveLoggingInput } from './logging/resolve-config'; export { installGlobalProxyDispatcher } from './utils/proxy'; +export { stripSystemTags, stripSystemFromOutput } from './utils/strip-system-tags'; export type { LogContext, LogEntry, diff --git a/packages/agent-core/src/services/message/message.ts b/packages/agent-core/src/services/message/message.ts index 07bfe98779..080ee900b4 100644 --- a/packages/agent-core/src/services/message/message.ts +++ b/packages/agent-core/src/services/message/message.ts @@ -45,6 +45,7 @@ import { createDecorator } from '../../di'; import type { ContextMessage } from '../../agent/context'; +import { stripSystemTags } from '../../utils/strip-system-tags'; import type { CursorQuery, Message, @@ -197,9 +198,9 @@ function buildProtocolContent(msg: ContextMessage): MessageContent[] { // fall back to text passthrough so we don't lose user-visible content. return msg.content.map((p) => mapContentPart(p)); } - const flattenedOutput = msg.content - .map((p) => (p.type === 'text' ? p.text : '')) - .join(''); + const flattenedOutput = stripSystemTags( + msg.content.map((p) => (p.type === 'text' ? p.text : '')).join(''), + ); const part: MessageContent = msg.isError === true ? { type: 'tool_result', diff --git a/packages/agent-core/src/utils/strip-system-tags.ts b/packages/agent-core/src/utils/strip-system-tags.ts new file mode 100644 index 0000000000..4f34708327 --- /dev/null +++ b/packages/agent-core/src/utils/strip-system-tags.ts @@ -0,0 +1,36 @@ +/** + * Strip `...` blocks from text for UI display. + * + * Tool results and user messages may carry `` blocks as side-channel + * notes meant for the model — ReadMediaFile's mime/dimension summary, or the + * tool error/empty status sentinels (`ERROR: …`, + * `Tool output is empty.`). They must stay in history (the + * model reads them) but should never render in user-facing UIs. Call this at + * every core→UI output boundary — the server protocol mapper, the live event + * mapper, and the TUI output component — so the stripping rule lives in exactly + * one place instead of being reimplemented per UI. + * + * Only well-formed, paired tags are removed. A lone `` without its + * closing `` is left untouched, so user data that merely contains the + * literal substring is not eaten. + */ +import type { ContentPart } from '@moonshot-ai/kosong'; + +const SYSTEM_TAG_RE = /[\s\S]*?<\/system>/g; + +export function stripSystemTags(text: string): string { + if (!text.includes('')) return text; + return text.replace(SYSTEM_TAG_RE, ''); +} + +/** + * Strip `` blocks from a tool result's `output`, which may be a plain + * string or a list of content parts (only `text` parts carry tags). Returns a + * value of the same shape; non-text parts pass through unchanged. + */ +export function stripSystemFromOutput(output: string | ContentPart[]): string | ContentPart[] { + if (typeof output === 'string') return stripSystemTags(output); + return output.map((part) => + part.type === 'text' ? { ...part, text: stripSystemTags(part.text) } : part, + ); +} diff --git a/packages/agent-core/test/services/message-service.test.ts b/packages/agent-core/test/services/message-service.test.ts index 1e34acb154..257ea49a21 100644 --- a/packages/agent-core/test/services/message-service.test.ts +++ b/packages/agent-core/test/services/message-service.test.ts @@ -359,3 +359,26 @@ describe('MessageService', () => { failingImpl.dispose(); }); }); + + +describe('toProtocolMessage tool-result stripping', () => { + it('strips blocks from a tool result output before it reaches the wire shape', () => { + const toolMessage: ContextMessage = { + role: 'tool', + toolCallId: 'call_1', + content: [ + { type: 'text', text: 'Read image file. Mime type: image/png.' }, + { 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).not.toContain(''); + expect(output).not.toContain('Read image file'); + expect(output).toContain(''); + }); +}); diff --git a/packages/agent-core/test/utils/strip-system-tags.test.ts b/packages/agent-core/test/utils/strip-system-tags.test.ts new file mode 100644 index 0000000000..a9dd254df7 --- /dev/null +++ b/packages/agent-core/test/utils/strip-system-tags.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from 'vitest'; + +import { stripSystemFromOutput, stripSystemTags } from '../../src/utils/strip-system-tags'; + +describe('stripSystemTags', () => { + it('removes a paired system block', () => { + expect(stripSystemTags('before secret note after')).toBe('before after'); + }); + + it('removes multiple blocks', () => { + expect(stripSystemTags('axb')).toBe('x'); + }); + + it('removes multiline blocks', () => { + expect(stripSystemTags('line1\nline2')).toBe(''); + }); + + it('leaves a lone opening tag untouched so user data is not eaten', () => { + expect(stripSystemTags('value not closed')).toBe('value not closed'); + }); + + it('leaves text without system tags unchanged', () => { + expect(stripSystemTags('plain text')).toBe('plain text'); + }); + + it('strips tool error/empty sentinels but keeps surrounding text', () => { + expect(stripSystemTags('ERROR: Tool execution failed.\nreal stderr')).toBe( + '\nreal stderr', + ); + expect(stripSystemTags('Tool output is empty.')).toBe(''); + }); +}); + +describe('stripSystemFromOutput', () => { + it('strips string output', () => { + expect(stripSystemFromOutput('xhi')).toBe('hi'); + }); + + it('strips only text parts in a content-part array and keeps media parts', () => { + const out = stripSystemFromOutput([ + { type: 'text', text: 'note' }, + { type: 'text', text: '' }, + { type: 'image_url', imageUrl: { url: 'data:image/png;base64,A' } }, + ]); + expect(out).toEqual([ + { type: 'text', text: '' }, + { type: 'text', text: '' }, + { type: 'image_url', imageUrl: { url: 'data:image/png;base64,A' } }, + ]); + }); +}); diff --git a/packages/node-sdk/src/index.ts b/packages/node-sdk/src/index.ts index 0fb4cc928e..381ed7e719 100644 --- a/packages/node-sdk/src/index.ts +++ b/packages/node-sdk/src/index.ts @@ -67,6 +67,10 @@ export { effectiveModelAlias, loadRuntimeConfigSafe, resolveConfigPath } from '@ // outbound fetch honors HTTP_PROXY / HTTPS_PROXY / NO_PROXY. export { installGlobalProxyDispatcher } from '@moonshot-ai/agent-core'; +// UI display helper — strips `...` side-channel notes from tool +// output before rendering, so hosts (CLI/TUI) don't surface model-bound markup. +export { stripSystemTags } from '@moonshot-ai/agent-core'; + // Image compression — ingestion sites (e.g. the CLI's clipboard paste, the ACP // adapter) shrink oversized images while constructing the content part, before // it enters a prompt. Best effort: returns the original on any failure. From 56fa94f7d581580f87ddce60be456152e7bc568c Mon Sep 17 00:00:00 2001 From: Kaiyi Date: Mon, 6 Jul 2026 16:14:24 +0800 Subject: [PATCH 2/9] 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. --- .../agent-core/src/utils/strip-system-tags.ts | 30 ++++++++++++++++--- .../test/utils/strip-system-tags.test.ts | 9 ++++-- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/packages/agent-core/src/utils/strip-system-tags.ts b/packages/agent-core/src/utils/strip-system-tags.ts index 4f34708327..6fb3646719 100644 --- a/packages/agent-core/src/utils/strip-system-tags.ts +++ b/packages/agent-core/src/utils/strip-system-tags.ts @@ -10,17 +10,39 @@ * mapper, and the TUI output component — so the stripping rule lives in exactly * one place instead of being reimplemented per UI. * - * Only well-formed, paired tags are removed. A lone `` without its - * closing `` is left untouched, so user data that merely contains the - * literal substring is not eaten. + * Most `` blocks are removed entirely. The exception is the tool + * error/empty status sentinels, which carry user-meaningful state and are + * UNWRAPPED — the text is kept, only the tags are dropped — so a failed or + * empty tool result does not render as a blank output. A lone `` + * without its closing `` is left untouched, so user data that merely + * contains the literal substring is not eaten. */ import type { ContentPart } from '@moonshot-ai/kosong'; const SYSTEM_TAG_RE = /[\s\S]*?<\/system>/g; +// Status sentinels carrying user-meaningful state ("tool failed" / "tool +// produced no output"). The model reads them with the `` wrapper in +// history, but for UI we UNWRAP them — keep the human-readable text, drop only +// the tags — instead of deleting them. Otherwise a failed or empty tool result +// renders as a blank output, indistinguishable from a rendering bug. Match the +// longest first so the combined sentinel is unwrapped cleanly. +const SENTINELS: readonly [string, string][] = [ + [ + 'ERROR: Tool execution failed. Tool output is empty.', + 'ERROR: Tool execution failed. Tool output is empty.', + ], + ['ERROR: Tool execution failed.', 'ERROR: Tool execution failed.'], + ['Tool output is empty.', 'Tool output is empty.'], +]; + export function stripSystemTags(text: string): string { if (!text.includes('')) return text; - return text.replace(SYSTEM_TAG_RE, ''); + let out = text; + for (const [tag, plain] of SENTINELS) { + out = out.replaceAll(tag, plain); + } + return out.replace(SYSTEM_TAG_RE, ''); } /** diff --git a/packages/agent-core/test/utils/strip-system-tags.test.ts b/packages/agent-core/test/utils/strip-system-tags.test.ts index a9dd254df7..99483f6d77 100644 --- a/packages/agent-core/test/utils/strip-system-tags.test.ts +++ b/packages/agent-core/test/utils/strip-system-tags.test.ts @@ -23,11 +23,14 @@ describe('stripSystemTags', () => { expect(stripSystemTags('plain text')).toBe('plain text'); }); - it('strips tool error/empty sentinels but keeps surrounding text', () => { + it('unwraps error/empty sentinels, keeping the status text without the tags', () => { expect(stripSystemTags('ERROR: Tool execution failed.\nreal stderr')).toBe( - '\nreal stderr', + 'ERROR: Tool execution failed.\nreal stderr', ); - expect(stripSystemTags('Tool output is empty.')).toBe(''); + expect(stripSystemTags('Tool output is empty.')).toBe('Tool output is empty.'); + expect( + stripSystemTags('ERROR: Tool execution failed. Tool output is empty.'), + ).toBe('ERROR: Tool execution failed. Tool output is empty.'); }); }); From 5cb44dba41e6bf66513868686709409b2fde14cf Mon Sep 17 00:00:00 2001 From: Kaiyi Date: Mon, 6 Jul 2026 21:59:45 +0800 Subject: [PATCH 3/9] 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 --- .changeset/hide-tool-result-system-notes.md | 2 +- .changeset/media-chip-original-dimensions.md | 5 - .../messages/tool-renderers/media.ts | 11 +- .../messages/tool-renderers/truncated.ts | 3 +- .../messages/tool-renderers/media.test.ts | 5 - .../messages/tool-renderers/truncated.test.ts | 10 +- apps/vis/server/src/lib/context-projector.ts | 77 +--------- .../server/test/lib/context-projector.test.ts | 11 +- .../agent-core/src/agent/context/index.ts | 52 +------ .../agent-core/src/agent/context/projector.ts | 17 ++- .../src/agent/context/tool-result-render.ts | 94 +++++++++++++ .../agent-core/src/agent/context/types.ts | 6 + .../src/agent/records/migration/index.ts | 4 +- .../src/agent/records/migration/v1.5.ts | 113 +++++++++++++++ packages/agent-core/src/agent/turn/index.ts | 3 +- packages/agent-core/src/index.ts | 3 +- packages/agent-core/src/loop/tool-call.ts | 12 +- packages/agent-core/src/loop/types.ts | 10 ++ packages/agent-core/src/mcp/output.ts | 67 +++++++-- .../src/services/message/message.ts | 7 +- .../src/services/message/transcript.ts | 58 ++------ .../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 | 5 +- .../agent-core/src/utils/strip-system-tags.ts | 58 -------- .../agent-core/test/agent/context.test.ts | 113 ++++++++++++++- .../agent-core/test/agent/permission.test.ts | 10 +- .../test/agent/records/index.test.ts | 69 +++++++++ .../test/agent/records/migration/v1.4.test.ts | 2 +- .../test/agent/records/migration/v1.5.test.ts | 132 ++++++++++++++++++ packages/agent-core/test/agent/resume.test.ts | 2 +- .../test/agent/tool-result-render.test.ts | 113 +++++++++++++++ .../test/loop/tool-call.e2e.test.ts | 20 +++ packages/agent-core/test/mcp/output.test.ts | 42 +++--- .../test/services/message-service.test.ts | 12 +- .../test/services/message-transcript.test.ts | 16 +-- .../test/tools/builtin-current.test.ts | 9 +- .../agent-core/test/tools/read-file.test.ts | 2 +- .../agent-core/test/tools/read-media.test.ts | 94 ++++++------- packages/agent-core/test/tools/read.test.ts | 100 +++++++------ .../test/utils/strip-system-tags.test.ts | 54 ------- packages/node-sdk/src/index.ts | 4 - 43 files changed, 958 insertions(+), 517 deletions(-) delete mode 100644 .changeset/media-chip-original-dimensions.md create mode 100644 packages/agent-core/src/agent/context/tool-result-render.ts create mode 100644 packages/agent-core/src/agent/records/migration/v1.5.ts delete mode 100644 packages/agent-core/src/utils/strip-system-tags.ts create mode 100644 packages/agent-core/test/agent/records/migration/v1.5.test.ts create mode 100644 packages/agent-core/test/agent/tool-result-render.test.ts delete mode 100644 packages/agent-core/test/utils/strip-system-tags.test.ts diff --git a/.changeset/hide-tool-result-system-notes.md b/.changeset/hide-tool-result-system-notes.md index 78ea804410..ab61d63296 100644 --- a/.changeset/hide-tool-result-system-notes.md +++ b/.changeset/hide-tool-result-system-notes.md @@ -2,4 +2,4 @@ "@moonshot-ai/kimi-code": patch --- -Fix tool-result `` notes being rendered as plain text in the terminal and web UIs. +Stop showing tool-produced `` metadata in tool outputs; failed tools now show their own error text. Existing sessions are migrated automatically. diff --git a/.changeset/media-chip-original-dimensions.md b/.changeset/media-chip-original-dimensions.md deleted file mode 100644 index 1ac5b47bc4..0000000000 --- a/.changeset/media-chip-original-dimensions.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@moonshot-ai/kimi-code": patch ---- - -Show original image dimensions in the ReadMediaFile tool chip. 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 b754c9be9b..b798cc8e5b 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 dimensions:\s*(\d+x\d+)\s*pixels/; 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]}px`; 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/src/tui/components/messages/tool-renderers/truncated.ts b/apps/kimi-code/src/tui/components/messages/tool-renderers/truncated.ts index a464395353..036ae0a204 100644 --- a/apps/kimi-code/src/tui/components/messages/tool-renderers/truncated.ts +++ b/apps/kimi-code/src/tui/components/messages/tool-renderers/truncated.ts @@ -1,4 +1,3 @@ -import { stripSystemTags } from '@moonshot-ai/kimi-code-sdk'; import { Text, truncateToWidth, type Component } from '@moonshot-ai/pi-tui'; import { currentTheme } from '#/tui/theme'; @@ -52,7 +51,7 @@ export class TruncatedOutputComponent implements Component { this.indent = options.indent ?? DEFAULT_INDENT; this.expandHint = options.expandHint ?? true; this.tail = options.tail ?? false; - const cleaned = stripSystemTags(trimTrailingEmptyLines(output.split('\n')).join('\n')); + const cleaned = trimTrailingEmptyLines(output.split('\n')).join('\n'); this.textComponent = new Text( options.isError ? currentTheme.fg('error', cleaned) : currentTheme.dim(cleaned), this.indent, 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 d8abcd1fa7..691f1d88a1 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,10 +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: `ReadMediaFile summary: Read image file. Mime type: ${mime}. Size: 70 bytes. Original dimensions: 1x1 pixels.`, - }, ]); } @@ -61,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 770a79a02b..4c90c03925 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 @@ -75,14 +75,16 @@ describe('TruncatedOutputComponent', () => { } }); - it('strips blocks from output before rendering', () => { + 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( - 'Read image file. Mime type: image/png.\n', + 'literal text from a user file\n', { expanded: true, isError: false }, ); const out = strip(component.render(80).join('\n')); - expect(out).not.toContain(''); - expect(out).not.toContain('Read image file'); + 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 4def1c6314..4cb2468abb 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 e6b164a5ba..325c380a05 100644 --- a/apps/vis/server/test/lib/context-projector.test.ts +++ b/apps/vis/server/test/lib/context-projector.test.ts @@ -149,10 +149,9 @@ describe('context-projector', () => { // that normalization so the model-view shows the content the model actually // received for failed / empty tool calls. - 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_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.'; /** Build a minimal wire fixture: one assistant step with a tool call, then a * `tool.result` loop event carrying `result`. Returns the projected tool @@ -215,8 +214,8 @@ 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 already starting with ERROR: is passed through (no double prefix)', () => { + const text = 'ERROR: already wrapped\ndetails here'; const msg = projectToolResult({ output: text, isError: true }); expect(msg.content).toEqual([{ type: 'text', text }]); }); diff --git a/packages/agent-core/src/agent/context/index.ts b/packages/agent-core/src/agent/context/index.ts index 22beabebdc..6b519ad8e6 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'; @@ -32,11 +32,6 @@ import { export * from './types'; -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.'; @@ -630,11 +625,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(); @@ -684,40 +684,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 @@ -747,10 +713,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 75ee0fbcb2..921c8cb059 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 0000000000..58c352563c --- /dev/null +++ b/packages/agent-core/src/agent/context/tool-result-render.ts @@ -0,0 +1,94 @@ +/** + * 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 plain-text `ERROR:` status prefix (meant for both + * the model and humans — UIs that show projected text may show it too); + * - an empty output is replaced with a 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. + * + * 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.'; + +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)]; + if (single.trimStart().startsWith('ERROR:')) return [textPart(single)]; + 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_EMPTY_STATUS; +} + +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 9286a8dbf0..d16b0c5204 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/agent/records/migration/index.ts b/packages/agent-core/src/agent/records/migration/index.ts index 8431c16329..2d12e951b8 100644 --- a/packages/agent-core/src/agent/records/migration/index.ts +++ b/packages/agent-core/src/agent/records/migration/index.ts @@ -2,13 +2,14 @@ import { migrateV1_0ToV1_1 } from './v1.1'; import { migrateV1_1ToV1_2 } from './v1.2'; import { migrateV1_2ToV1_3 } from './v1.3'; import { migrateV1_3ToV1_4 } from './v1.4'; +import { migrateV1_4ToV1_5 } from './v1.5'; // Wire protocol versions currently support only the `number.number` format. // Bump this only for changes that require migration of existing records or // change how existing records must be interpreted. Do not bump it only because // a new feature adds a new wire record type: older versions do not implement // that feature and do not need to understand the new record type. -export const AGENT_WIRE_PROTOCOL_VERSION = '1.4'; +export const AGENT_WIRE_PROTOCOL_VERSION = '1.5'; export interface WireMigrationRecord { readonly type: string; @@ -26,6 +27,7 @@ const MIGRATIONS: readonly WireMigration[] = [ migrateV1_1ToV1_2, migrateV1_2ToV1_3, migrateV1_3ToV1_4, + migrateV1_4ToV1_5, ]; export function isNewerWireVersion(readVersion: string): boolean { diff --git a/packages/agent-core/src/agent/records/migration/v1.5.ts b/packages/agent-core/src/agent/records/migration/v1.5.ts new file mode 100644 index 0000000000..3700f9cfc3 --- /dev/null +++ b/packages/agent-core/src/agent/records/migration/v1.5.ts @@ -0,0 +1,113 @@ +/** + * 1.4 → 1.5: tool-produced `` metadata moves out of tool-result + * `output` into the structured `note` side channel. + * + * Through 1.4, ReadMediaFile / Read / the MCP image-compression pipeline + * baked their model-facing metadata into the output text as `…` + * blocks, which every UI then had to strip back out. 1.5 stores that + * metadata as `result.note` (rendered to the model at projection time, + * never to UIs), so old records are rewritten to the same shape. + * + * Only blocks that anchor on the exact openings those three producers used + * are moved; any other `` text is user data and stays untouched. + */ +import { extractImageCompressionCaptions } from '../../../tools/support/image-compress'; +import type { WireMigration, WireMigrationRecord } from './index'; + +/** Entire-part match for the ReadMediaFile summary (was the first part). */ +const READ_MEDIA_NOTE_RE = /^Read (?:image|video) file\. Mime type: [\s\S]*<\/system>$/; + +/** Trailing Read status block, matching `finishMessage`'s fixed openings. */ +const READ_STATUS_TAIL_RE = + /\n?(?:\d+ lines? read from file starting from line \d+\.|No lines read from file\.) Total lines in file: \d+\.[\s\S]*?<\/system>$/; + +interface LoosePart { + readonly type?: unknown; + readonly text?: unknown; + readonly [key: string]: unknown; +} + +interface LooseToolResult { + readonly output?: unknown; + readonly note?: unknown; + readonly [key: string]: unknown; +} + +interface LooseToolResultEvent { + readonly type?: unknown; + readonly result?: unknown; + readonly [key: string]: unknown; +} + +export const migrateV1_4ToV1_5: WireMigration = { + sourceVersion: '1.4', + targetVersion: '1.5', + migrateRecord(record: WireMigrationRecord): WireMigrationRecord { + if (record.type !== 'context.append_loop_event') return record; + const event = record['event'] as LooseToolResultEvent | undefined; + if (event === undefined || event.type !== 'tool.result') return record; + const result = event.result as LooseToolResult | undefined; + if (result === undefined || result === null || typeof result !== 'object') return record; + if (typeof result.note === 'string') return record; + + const extracted = extractToolNote(result.output); + if (extracted === null) return record; + + return { + ...record, + event: { + ...event, + result: { ...result, output: extracted.output, note: extracted.note }, + }, + }; + }, +}; + +function extractToolNote( + output: unknown, +): { output: string | LoosePart[]; note: string } | null { + if (typeof output === 'string') return extractFromString(output); + if (Array.isArray(output)) return extractFromParts(output as LoosePart[]); + return null; +} + +function extractFromString(output: string): { output: string; note: string } | null { + const notes: string[] = []; + + const captioned = extractImageCompressionCaptions(output); + let remainder = captioned.text; + notes.push(...captioned.captions.map((body) => `${body}`)); + + const statusMatch = READ_STATUS_TAIL_RE.exec(remainder); + if (statusMatch !== null) { + remainder = remainder.slice(0, statusMatch.index); + notes.push(statusMatch[0].startsWith('\n') ? statusMatch[0].slice(1) : statusMatch[0]); + } + + if (notes.length === 0) return null; + return { output: remainder, note: notes.join('\n') }; +} + +function extractFromParts(parts: LoosePart[]): { output: LoosePart[]; note: string } | null { + const kept: LoosePart[] = []; + const notes: string[] = []; + for (const part of parts) { + if (part !== null && typeof part === 'object' && part.type === 'text' && typeof part.text === 'string') { + if (READ_MEDIA_NOTE_RE.test(part.text)) { + notes.push(part.text); + continue; + } + const captioned = extractImageCompressionCaptions(part.text); + if (captioned.captions.length > 0) { + notes.push(...captioned.captions.map((body) => `${body}`)); + if (captioned.text.trim().length > 0) { + kept.push({ ...part, text: captioned.text }); + } + continue; + } + } + kept.push(part); + } + if (notes.length === 0) return null; + return { output: kept, note: notes.join('\n') }; +} diff --git a/packages/agent-core/src/agent/turn/index.ts b/packages/agent-core/src/agent/turn/index.ts index f5d77123f6..db4904a347 100644 --- a/packages/agent-core/src/agent/turn/index.ts +++ b/packages/agent-core/src/agent/turn/index.ts @@ -23,7 +23,6 @@ import { makeErrorPayload, toKimiErrorPayload, } from '#/errors'; -import { stripSystemFromOutput } from '#/utils/strip-system-tags'; import { isAbortError, isMaxStepsExceededError } from '../../loop/errors'; import { createLoopEventDispatcher, @@ -1128,7 +1127,7 @@ function mapLoopEvent(event: LoopEvent, turnId: number): AgentEvent | undefined type: 'tool.result', turnId, toolCallId: event.toolCallId, - output: stripSystemFromOutput(event.result.output), + output: event.result.output, isError: event.result.isError, }; case 'turn.interrupted': diff --git a/packages/agent-core/src/index.ts b/packages/agent-core/src/index.ts index 6158c7d253..eee11da2e5 100644 --- a/packages/agent-core/src/index.ts +++ b/packages/agent-core/src/index.ts @@ -18,7 +18,6 @@ export { export { resolveLoggingConfig } from './logging/resolve-config'; export type { ResolveLoggingInput } from './logging/resolve-config'; export { installGlobalProxyDispatcher } from './utils/proxy'; -export { stripSystemTags, stripSystemFromOutput } from './utils/strip-system-tags'; export type { LogContext, LogEntry, @@ -31,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, diff --git a/packages/agent-core/src/loop/tool-call.ts b/packages/agent-core/src/loop/tool-call.ts index e4f0337805..7fde09a8d5 100644 --- a/packages/agent-core/src/loop/tool-call.ts +++ b/packages/agent-core/src/loop/tool-call.ts @@ -657,12 +657,16 @@ 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. + const base: { output: typeof output; note?: string; truncated?: true } = { output }; + if (r.note !== undefined) 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 9d290f2354..63810fbb0c 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 06b217936f..d5c0923fe8 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 are collected + * into 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. @@ -27,7 +29,10 @@ import type { ContentPart } from '@moonshot-ai/kosong'; -import { compressImageContentParts } from '../tools/support/image-compress'; +import { + compressImageContentParts, + extractImageCompressionCaptions, +} from '../tools/support/image-compress'; import { persistOriginalImage } from '../tools/support/image-originals'; import type { MCPContentBlock, MCPToolResult } from './types'; @@ -151,7 +156,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,10 +171,11 @@ 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 @@ -183,12 +194,46 @@ export async function mcpResultToExecutableOutput( ), }, }); - const capped = applyBinaryPartCap(compressed); + // The compression helper inserts each caption inline (the prompt-ingestion + // caller depends on that); here the captions move to the `note` side + // channel so the tool output stays pure data. + const split = splitCompressionCaptions(compressed); + const capped = applyBinaryPartCap(split.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, + ...(split.note === undefined ? {} : { note: split.note }), + ...(truncated ? { truncated: true as const } : {}), + }; +} + +/** + * Pull the inline image-compression captions out of `parts` and join them + * into a single `note` string (re-wrapped in ``, one caption per + * line). Returns `note: undefined` when nothing was compressed. + */ +function splitCompressionCaptions(parts: readonly ContentPart[]): { + parts: ContentPart[]; + note?: string | undefined; +} { + const kept: ContentPart[] = []; + const captions: string[] = []; + for (const part of parts) { + if (part.type === 'text') { + const extracted = extractImageCompressionCaptions(part.text); + if (extracted.captions.length > 0) { + captions.push(...extracted.captions.map((body) => `${body}`)); + if (extracted.text.trim().length > 0) { + kept.push({ type: 'text', text: extracted.text }); + } + continue; + } + } + kept.push(part); + } + return captions.length > 0 ? { parts: kept, note: captions.join('\n') } : { parts: kept }; } /** diff --git a/packages/agent-core/src/services/message/message.ts b/packages/agent-core/src/services/message/message.ts index 080ee900b4..07bfe98779 100644 --- a/packages/agent-core/src/services/message/message.ts +++ b/packages/agent-core/src/services/message/message.ts @@ -45,7 +45,6 @@ import { createDecorator } from '../../di'; import type { ContextMessage } from '../../agent/context'; -import { stripSystemTags } from '../../utils/strip-system-tags'; import type { CursorQuery, Message, @@ -198,9 +197,9 @@ function buildProtocolContent(msg: ContextMessage): MessageContent[] { // fall back to text passthrough so we don't lose user-visible content. return msg.content.map((p) => mapContentPart(p)); } - const flattenedOutput = stripSystemTags( - msg.content.map((p) => (p.type === 'text' ? p.text : '')).join(''), - ); + const flattenedOutput = msg.content + .map((p) => (p.type === 'text' ? p.text : '')) + .join(''); const part: MessageContent = msg.isError === true ? { type: 'tool_result', diff --git a/packages/agent-core/src/services/message/transcript.ts b/packages/agent-core/src/services/message/transcript.ts index 2acfa0f4c8..1f296592aa 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,14 @@ 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 agent-core's `createToolMessage`: the stored message carries the + * tool's raw output verbatim (plus the structured isError/note fields). + * Model-facing status text is a projection concern + * (`renderToolResultForModel`), not part of the transcript. + */ +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 31b2b4c924..942b1f3dda 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 is appended after 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 9774671911..027f1e70a6 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 image above 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 d6abd57647..0c2f76be1b 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 0c640b7be3..5b1e2e7bc7 100644 --- a/packages/agent-core/src/tools/support/image-compress.ts +++ b/packages/agent-core/src/tools/support/image-compress.ts @@ -558,8 +558,9 @@ 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): the MCP output pipeline splits the inline + * captions out 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/src/utils/strip-system-tags.ts b/packages/agent-core/src/utils/strip-system-tags.ts deleted file mode 100644 index 6fb3646719..0000000000 --- a/packages/agent-core/src/utils/strip-system-tags.ts +++ /dev/null @@ -1,58 +0,0 @@ -/** - * Strip `...` blocks from text for UI display. - * - * Tool results and user messages may carry `` blocks as side-channel - * notes meant for the model — ReadMediaFile's mime/dimension summary, or the - * tool error/empty status sentinels (`ERROR: …`, - * `Tool output is empty.`). They must stay in history (the - * model reads them) but should never render in user-facing UIs. Call this at - * every core→UI output boundary — the server protocol mapper, the live event - * mapper, and the TUI output component — so the stripping rule lives in exactly - * one place instead of being reimplemented per UI. - * - * Most `` blocks are removed entirely. The exception is the tool - * error/empty status sentinels, which carry user-meaningful state and are - * UNWRAPPED — the text is kept, only the tags are dropped — so a failed or - * empty tool result does not render as a blank output. A lone `` - * without its closing `` is left untouched, so user data that merely - * contains the literal substring is not eaten. - */ -import type { ContentPart } from '@moonshot-ai/kosong'; - -const SYSTEM_TAG_RE = /[\s\S]*?<\/system>/g; - -// Status sentinels carrying user-meaningful state ("tool failed" / "tool -// produced no output"). The model reads them with the `` wrapper in -// history, but for UI we UNWRAP them — keep the human-readable text, drop only -// the tags — instead of deleting them. Otherwise a failed or empty tool result -// renders as a blank output, indistinguishable from a rendering bug. Match the -// longest first so the combined sentinel is unwrapped cleanly. -const SENTINELS: readonly [string, string][] = [ - [ - 'ERROR: Tool execution failed. Tool output is empty.', - 'ERROR: Tool execution failed. Tool output is empty.', - ], - ['ERROR: Tool execution failed.', 'ERROR: Tool execution failed.'], - ['Tool output is empty.', 'Tool output is empty.'], -]; - -export function stripSystemTags(text: string): string { - if (!text.includes('')) return text; - let out = text; - for (const [tag, plain] of SENTINELS) { - out = out.replaceAll(tag, plain); - } - return out.replace(SYSTEM_TAG_RE, ''); -} - -/** - * Strip `` blocks from a tool result's `output`, which may be a plain - * string or a list of content parts (only `text` parts carry tags). Returns a - * value of the same shape; non-text parts pass through unchanged. - */ -export function stripSystemFromOutput(output: string | ContentPart[]): string | ContentPart[] { - if (typeof output === 'string') return stripSystemTags(output); - return output.map((part) => - part.type === 'text' ? { ...part, text: stripSystemTags(part.text) } : part, - ); -} diff --git a/packages/agent-core/test/agent/context.test.ts b/packages/agent-core/test/agent/context.test.ts index 9d65556f3f..fc2733f89e 100644 --- a/packages/agent-core/test/agent/context.test.ts +++ b/packages/agent-core/test/agent/context.test.ts @@ -347,7 +347,7 @@ describe('Agent context', () => { { role: 'assistant', toolCalls: [{ id: 'call_ws' }] }, { role: 'tool', - content: [{ type: 'text', text: 'Tool output is empty.' }], + content: [{ type: 'text', text: 'Tool output is empty.' }], toolCallId: 'call_ws', }, ]); @@ -400,18 +400,109 @@ 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', }, { role: 'tool', - content: [{ type: 'text', text: 'Tool output is empty.' }], + content: [{ type: 'text', text: 'Tool output is empty.' }], toolCallId: 'call_empty', }, ]); }); + 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 +573,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 +588,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/permission.test.ts b/packages/agent-core/test/agent/permission.test.ts index 094bd8d8c5..3b31a8dd32 100644 --- a/packages/agent-core/test/agent/permission.test.ts +++ b/packages/agent-core/test/agent/permission.test.ts @@ -242,10 +242,10 @@ describe('Agent permission', () => { [emit] turn.step.started { "turnId": 0, "step": 2, "stepId": "" } [emit] assistant.delta { "turnId": 0, "delta": "I will not run the command." } [wire] context.append_loop_event { "event": { "type": "content.part", "uuid": "", "turnId": "0", "step": 2, "stepUuid": "", "part": { "type": "text", "text": "I will not run the command." } }, "time": "