From a2bebc5f8226965ca1726ebf61fb1723f3063953 Mon Sep 17 00:00:00 2001 From: youwang <61931019+MelodyVAR@users.noreply.github.com> Date: Sun, 27 Sep 2026 17:53:10 +0000 Subject: [PATCH 1/3] fix(coding-agent): bound and retain final tool result text --- packages/agent-core/README.md | 9 + packages/agent-core/src/agent-loop.ts | 40 ++- packages/agent-core/src/agent.ts | 4 + packages/agent-core/src/types.ts | 11 + .../test/tool-result-transform.test.ts | 196 +++++++++++++++ .../docs/tool-output-retention.md | 32 +++ .../coding-agent/src/core/agent-session.ts | 45 +++- .../src/core/tools/tool-output.ts | 91 +++++++ .../suite/agent-session-tool-output.test.ts | 234 ++++++++++++++++++ .../coding-agent/test/tool-output.test.ts | 147 +++++++++++ 10 files changed, 798 insertions(+), 11 deletions(-) create mode 100644 packages/agent-core/test/tool-result-transform.test.ts create mode 100644 packages/coding-agent/docs/tool-output-retention.md create mode 100644 packages/coding-agent/src/core/tools/tool-output.ts create mode 100644 packages/coding-agent/test/suite/agent-session-tool-output.test.ts create mode 100644 packages/coding-agent/test/tool-output.test.ts diff --git a/packages/agent-core/README.md b/packages/agent-core/README.md index 3a978ea7..a9bb81b1 100644 --- a/packages/agent-core/README.md +++ b/packages/agent-core/README.md @@ -130,6 +130,15 @@ The `beforeToolCall` hook runs after `tool_execution_start` and validated argume Tools, blocked `beforeToolCall` results, and `afterToolCall` overrides can return `terminate: true` to hint that the automatic follow-up LLM call should be skipped. The loop only stops early when every finalized tool result in that batch sets `terminate: true`. Mixed batches continue normally. +`transformToolResult` is an optional final content-only transform on `AgentOptions` +and `AgentLoopConfig`. It runs before terminal tool events and result messages for +all outcomes, including validation failures, denied calls and rejected truncated +calls. It runs after `afterToolCall` for executed tools; existing execution-hook +semantics are unchanged. Its returned content cannot override usage, metadata or +termination hints. A thrown error becomes a bounded error result while preserving +those fields. Applications can use this boundary to retain and limit output; +agent-core itself does not own storage or output-limit policy. + The `Agent` class accepts `shouldStopAfterTurn` in `AgentOptions`. Low-level loop callers can set the same hook in `AgentLoopConfig`: ```typescript diff --git a/packages/agent-core/src/agent-loop.ts b/packages/agent-core/src/agent-loop.ts index 0de9ec85..c25600d7 100644 --- a/packages/agent-core/src/agent-loop.ts +++ b/packages/agent-core/src/agent-loop.ts @@ -242,7 +242,7 @@ async function runLoop( // them all instead of executing potentially borked calls. const executedToolBatch = message.stopReason === "length" - ? await failToolCallsFromTruncatedMessage(toolCalls, emit) + ? await failToolCallsFromTruncatedMessage(toolCalls, config, signal, emit) : await executeToolCalls(currentContext, message, config, signal, emit); toolResults.push(...executedToolBatch.messages); hasMoreToolCalls = !executedToolBatch.terminate; @@ -417,6 +417,8 @@ async function streamAssistantResponse( */ async function failToolCallsFromTruncatedMessage( toolCalls: AgentToolCall[], + config: AgentLoopConfig, + signal: AbortSignal | undefined, emit: AgentEventSink, ): Promise { const messages: ToolResultMessage[] = []; @@ -434,7 +436,7 @@ async function failToolCallsFromTruncatedMessage( ), isError: true, }; - await emitToolExecutionEnd(finalized, emit); + await emitToolExecutionEnd(finalized, config, signal, emit); const toolResultMessage = createToolResultMessage(finalized); await emitToolResultMessage(toolResultMessage, emit); messages.push(toolResultMessage); @@ -506,7 +508,7 @@ async function executeToolCallsSequential( ); } - await emitToolExecutionEnd(finalized, emit); + await emitToolExecutionEnd(finalized, config, signal, emit); const toolResultMessage = createToolResultMessage(finalized); await emitToolResultMessage(toolResultMessage, emit); finalizedCalls.push(finalized); @@ -548,7 +550,7 @@ async function executeToolCallsParallel( result: preparation.result, isError: preparation.isError, } satisfies FinalizedToolCallOutcome; - await emitToolExecutionEnd(finalized, emit); + await emitToolExecutionEnd(finalized, config, signal, emit); finalizedCalls.push(finalized); if (signal?.aborted) { break; @@ -566,7 +568,7 @@ async function executeToolCallsParallel( config, signal, ); - await emitToolExecutionEnd(finalized, emit); + await emitToolExecutionEnd(finalized, config, signal, emit); return finalized; }); if (signal?.aborted) { @@ -801,7 +803,33 @@ function createErrorToolResult(message: string): AgentToolResult { }; } -async function emitToolExecutionEnd(finalized: FinalizedToolCallOutcome, emit: AgentEventSink): Promise { +async function emitToolExecutionEnd( + finalized: FinalizedToolCallOutcome, + config: AgentLoopConfig, + signal: AbortSignal | undefined, + emit: AgentEventSink, +): Promise { + if (config.transformToolResult) { + try { + const content = await config.transformToolResult(finalized.result.content ?? [], signal); + finalized.result = { ...finalized.result, content }; + } catch (error) { + const reason = error instanceof Error ? error.message.slice(0, 512) : "Unknown output processing error"; + finalized.result = { + ...finalized.result, + content: [ + { + type: "text", + text: `Tool output could not be prepared: ${reason}. The tool may already have run; check its effects before repeating a state-changing call.`, + }, + ...(Array.isArray(finalized.result.content) + ? finalized.result.content.filter((part) => part?.type === "image") + : []), + ], + }; + finalized.isError = true; + } + } await emit({ type: "tool_execution_end", toolCallId: finalized.toolCall.id, diff --git a/packages/agent-core/src/agent.ts b/packages/agent-core/src/agent.ts index 68365590..4d1b0bab 100644 --- a/packages/agent-core/src/agent.ts +++ b/packages/agent-core/src/agent.ts @@ -105,6 +105,7 @@ export interface AgentOptions { onResponse?: SimpleStreamOptions["onResponse"]; beforeToolCall?: (context: BeforeToolCallContext, signal?: AbortSignal) => Promise; afterToolCall?: (context: AfterToolCallContext, signal?: AbortSignal) => Promise; + transformToolResult?: AgentLoopConfig["transformToolResult"]; shouldStopAfterTurn?: (context: ShouldStopAfterTurnContext, signal?: AbortSignal) => boolean | Promise; prepareNextTurn?: ( signal?: AbortSignal, @@ -190,6 +191,7 @@ export class Agent { context: AfterToolCallContext, signal?: AbortSignal, ) => Promise; + public transformToolResult?: AgentLoopConfig["transformToolResult"]; public shouldStopAfterTurn?: ( context: ShouldStopAfterTurnContext, signal?: AbortSignal, @@ -225,6 +227,7 @@ export class Agent { this.onResponse = runtimeOptions.onResponse; this.beforeToolCall = runtimeOptions.beforeToolCall; this.afterToolCall = runtimeOptions.afterToolCall; + this.transformToolResult = runtimeOptions.transformToolResult; this.shouldStopAfterTurn = runtimeOptions.shouldStopAfterTurn; this.prepareNextTurn = runtimeOptions.prepareNextTurn; this.prepareNextTurnWithContext = runtimeOptions.prepareNextTurnWithContext; @@ -457,6 +460,7 @@ export class Agent { toolExecution: this.toolExecution, beforeToolCall: this.beforeToolCall, afterToolCall: this.afterToolCall, + transformToolResult: this.transformToolResult, shouldStopAfterTurn: shouldStopAfterTurn ? async (context) => await shouldStopAfterTurn(context, this.signal) : undefined, diff --git a/packages/agent-core/src/types.ts b/packages/agent-core/src/types.ts index 5db470b4..85947b11 100644 --- a/packages/agent-core/src/types.ts +++ b/packages/agent-core/src/types.ts @@ -303,6 +303,17 @@ export interface AgentLoopConfig extends SimpleStreamOptions { * The hook receives the agent abort signal and is responsible for honoring it. */ afterToolCall?: (context: AfterToolCallContext, signal?: AbortSignal) => Promise; + + /** + * Final content-only transform for every tool outcome, including validation failures, + * denied calls and truncated-call rejections. Runs after execution hooks and before + * terminal events or result messages; it cannot override usage or termination policy. + * A transform failure becomes an error result while preserving the original metadata. + */ + transformToolResult?: ( + content: AgentToolResult["content"], + signal?: AbortSignal, + ) => Promise["content"]>; } /** diff --git a/packages/agent-core/test/tool-result-transform.test.ts b/packages/agent-core/test/tool-result-transform.test.ts new file mode 100644 index 00000000..be2dd004 --- /dev/null +++ b/packages/agent-core/test/tool-result-transform.test.ts @@ -0,0 +1,196 @@ +import { fauxAssistantMessage, fauxToolCall } from "@step-harness/providers"; +import { registerFauxProvider, streamSimple } from "@step-harness/providers/compat"; +import { Type } from "typebox"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { Agent } from "../src/agent.ts"; +import type { AgentEvent, AgentTool, AgentToolResult, ToolExecutionMode } from "../src/types.ts"; + +const providers: ReturnType[] = []; +afterEach(() => { + while (providers.length) providers.pop()?.unregister(); +}); + +function setup(toolExecution: ToolExecutionMode, tools: AgentTool[]) { + const provider = registerFauxProvider(); + providers.push(provider); + const agent = new Agent({ + streamFn: streamSimple, + getApiKey: () => "test-key", + initialState: { model: provider.getModel(), tools }, + toolExecution, + }); + const events: AgentEvent[] = []; + agent.subscribe((event) => { + events.push(event); + }); + return { provider, agent, events }; +} + +function tool(name: string): AgentTool { + return { + name, + label: name, + description: name, + parameters: Type.Object({ count: Type.Number() }), + execute: async () => { + if (name === "broken") throw new Error("execution failed"); + return { content: [{ type: "text", text: "original" }], details: { kept: true } }; + }, + }; +} + +describe.each(["sequential", "parallel"])("final tool result transform (%s)", (mode) => { + it("transforms executed, invalid, missing and denied results before publication", async () => { + const { provider, agent, events } = setup(mode, [tool("ok"), tool("invalid"), tool("denied"), tool("broken")]); + const after = vi.fn(async () => ({ content: [{ type: "text" as const, text: "after hook" }] })); + agent.beforeToolCall = async ({ toolCall }) => + toolCall.name === "denied" ? { block: true, reason: "denied", terminate: true } : undefined; + agent.afterToolCall = after; + const transform = vi.fn(async (content: AgentToolResult["content"]) => [ + { type: "text" as const, text: `final:${JSON.stringify(content)}` }, + ]); + agent.transformToolResult = transform; + provider.setResponses([ + fauxAssistantMessage( + [ + fauxToolCall("ok", { count: 1 }), + fauxToolCall("invalid", { count: "bad" }), + fauxToolCall("missing", {}), + fauxToolCall("denied", { count: 1 }), + fauxToolCall("broken", { count: 1 }), + ], + { stopReason: "toolUse" }, + ), + fauxAssistantMessage("done"), + ]); + await agent.prompt("run tools"); + expect(transform).toHaveBeenCalledTimes(5); + expect(after).toHaveBeenCalledTimes(2); + const ended = events.filter((event) => event.type === "tool_execution_end"); + expect(ended).toHaveLength(5); + for (const event of ended) + expect(event.result.content[0]).toMatchObject({ text: expect.stringMatching(/^final:/) }); + const results = agent.state.messages.filter((message) => message.role === "toolResult"); + expect(results.map((message) => message.isError)).toEqual([false, true, true, true, true]); + for (const message of results) + expect(message.content[0]).toMatchObject({ text: expect.stringMatching(/^final:/) }); + }); + + it("keeps a denied batch terminating and leaves the execution hook untouched", async () => { + const blocked = tool("denied"); + const execute = vi.spyOn(blocked, "execute"); + const { provider, agent } = setup(mode, [blocked]); + agent.beforeToolCall = async () => ({ block: true, reason: "policy says no", terminate: true }); + const after = vi.fn(); + agent.afterToolCall = after; + agent.transformToolResult = async () => [{ type: "text", text: "bounded denial" }]; + provider.setResponses([fauxAssistantMessage(fauxToolCall("denied", { count: 1 }), { stopReason: "toolUse" })]); + await agent.prompt("denied"); + expect(execute).not.toHaveBeenCalled(); + expect(after).not.toHaveBeenCalled(); + expect(agent.state.messages.at(-1)).toMatchObject({ + role: "toolResult", + isError: true, + content: [{ type: "text", text: "bounded denial" }], + }); + }); + + it("reports transform failure without dropping the termination hint", async () => { + const done: AgentTool = { + ...tool("done"), + execute: async () => ({ + content: [{ type: "text", text: "completed" }], + details: { kept: true }, + terminate: true, + }), + }; + const { provider, agent } = setup(mode, [done]); + agent.transformToolResult = async () => { + throw new Error("cannot retain output"); + }; + provider.setResponses([fauxAssistantMessage(fauxToolCall("done", { count: 1 }), { stopReason: "toolUse" })]); + await agent.prompt("run"); + expect(agent.state.messages.at(-1)).toMatchObject({ + role: "toolResult", + isError: true, + details: { kept: true }, + content: [{ type: "text", text: expect.stringContaining("cannot retain output") }], + }); + }); +}); + +it("transforms tools rejected for a truncated assistant message", async () => { + const { provider, agent } = setup("parallel", [tool("ok")]); + agent.transformToolResult = async () => [{ type: "text", text: "bounded length rejection" }]; + provider.setResponses([ + fauxAssistantMessage(fauxToolCall("ok", { count: 1 }), { stopReason: "length" }), + fauxAssistantMessage("done"), + ]); + await agent.prompt("run"); + expect(agent.state.messages.find((message) => message.role === "toolResult")).toMatchObject({ + content: [{ type: "text", text: "bounded length rejection" }], + isError: true, + }); +}); + +it("honors the constructor transform while preserving usage and newly discovered tool names", async () => { + const provider = registerFauxProvider(); + providers.push(provider); + const usage = { + input: 1, + output: 2, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 3, + cost: { input: 0.01, output: 0.02, cacheRead: 0, cacheWrite: 0, total: 0.03 }, + }; + const discover: AgentTool = { + ...tool("discover"), + execute: async () => ({ + content: [{ type: "text", text: "large source" }], + details: { kept: true }, + usage, + addedToolNames: ["new_tool"], + terminate: true, + }), + }; + const agent = new Agent({ + streamFn: streamSimple, + getApiKey: () => "test-key", + initialState: { model: provider.getModel(), tools: [discover] }, + transformToolResult: async () => [{ type: "text", text: "retained preview" }], + }); + provider.setResponses([fauxAssistantMessage(fauxToolCall("discover", { count: 1 }), { stopReason: "toolUse" })]); + await agent.prompt("discover"); + expect(agent.state.messages.at(-1)).toMatchObject({ + role: "toolResult", + content: [{ type: "text", text: "retained preview" }], + details: { kept: true }, + usage, + addedToolNames: ["new_tool"], + isError: false, + }); +}); + +it("keeps image blocks when final text processing fails", async () => { + const image = { type: "image" as const, data: "aGVsbG8=", mimeType: "image/png" }; + const capture: AgentTool = { + ...tool("capture"), + execute: async () => ({ + content: [image, { type: "text", text: "x".repeat(60000) }], + details: {}, + terminate: true, + }), + }; + const { agent, provider } = setup("parallel", [capture]); + agent.transformToolResult = async () => { + throw new Error("storage unavailable"); + }; + provider.setResponses([fauxAssistantMessage(fauxToolCall("capture", { count: 1 }), { stopReason: "toolUse" })]); + await agent.prompt("capture"); + const result = agent.state.messages.at(-1); + expect(result?.role).toBe("toolResult"); + if (result?.role !== "toolResult") throw new Error("Expected tool result"); + expect(result.isError).toBe(true); + expect(result.content.filter((part) => part.type === "image")).toEqual([image]); +}); diff --git a/packages/coding-agent/docs/tool-output-retention.md b/packages/coding-agent/docs/tool-output-retention.md new file mode 100644 index 00000000..8e75e30f --- /dev/null +++ b/packages/coding-agent/docs/tool-output-retention.md @@ -0,0 +1,32 @@ +# Tool output retention + +Coding-agent bounds the combined text in each final tool result to 2,000 lines +and 50 KiB, including its truncation notice. This applies to built-in, MCP and +extension tools, text replaced by `tool_result` hooks, validation errors and +blocked calls. Accepted `message_end` tool-result replacements are checked again +before session persistence and model replay. Images are preserved separately. Tool details, usage, error +status and termination hints retain their existing contracts. + +An oversized result keeps a prefix and a `Full output:` path. The complete text +from all text blocks, joined by newlines, is saved before the preview is +published. Individual oversized lines may leave no complete line in the +preview; the full file remains readable with the normal file tools. A producer's +`truncated` metadata does not disable this final bound. If a producer already +lost content before returning, this file contains only what it returned. + +Artifacts live in `tool-output` under the session directory. If the session has +no storage directory, the configured agent directory (or its default) is used. +Paths in tool results are absolute JSON-quoted strings, so spaces and line breaks +in directory names do not change the notice's structure. New artifact directories are private and +files are created with exclusive creation and owner-only permissions. Each +retention operation makes a bounded best-effort pass over expired owned files; +files older than seven days may be removed. Cleanup skips unrelated names, +directories and symlinks, and a cleanup failure does not fail the tool. + +If complete output cannot be saved, the result reports an output-processing +error. It states that the tool may already have run and that effects should be +checked before repeating a state-changing call. Image blocks remain available even if text retention fails. Existing denial and +termination policy remains in force. Small results create no artifact and are unchanged. + +This bounds final model-facing text, not streaming progress updates or arbitrary +typed `details` payloads. No new permission bypass or external upload is added. diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 945f3200..fd45a7f4 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -14,7 +14,7 @@ */ import { readFileSync } from "node:fs"; -import { basename, dirname } from "node:path"; +import { basename, dirname, join } from "node:path"; import type { Agent, AgentContext, @@ -34,6 +34,7 @@ import type { Model, ProviderHeaders, TextContent, + ToolResultMessage, Usage, } from "@step-harness/providers/compat"; import { @@ -48,6 +49,7 @@ import { resetApiProviders, streamSimple, } from "@step-harness/providers/compat"; +import { getAgentDir } from "../config.ts"; import { getThemeByName, theme } from "../theme/theme.ts"; import { stripFrontmatter } from "../utils/frontmatter.ts"; import { sleep } from "../utils/sleep.ts"; @@ -114,6 +116,7 @@ import { type BuildSystemPromptOptions, buildSystemPrompt, type SystemPromptProd import { type BashOperations, createLocalBashOperations } from "./tools/bash.ts"; import { createAllToolDefinitions } from "./tools/index.ts"; import { createToolDefinitionFromAgentTool } from "./tools/tool-definition-wrapper.ts"; +import { boundToolResultContent } from "./tools/tool-output.ts"; import { addUsageToTotals, createUsageTotals } from "./usage-totals.ts"; // ============================================================================ @@ -500,6 +503,14 @@ export class AgentSession { } } + private _boundToolContent(content: ToolResultMessage["content"], signal?: AbortSignal) { + return boundToolResultContent( + content, + join(this.sessionManager.getSessionDir() || this._agentDir || getAgentDir(), "tool-output"), + signal, + ); + } + /** * Install tool hooks once on the Agent instance. * @@ -509,6 +520,8 @@ export class AgentSession { * happens here instead of in wrappers. */ private _installAgentToolHooks(): void { + this.agent.transformToolResult = (content, signal) => this._boundToolContent(content, signal); + this.agent.beforeToolCall = async ({ toolCall, args }) => { const runner = this._extensionRunner; if (!runner.hasHandlers("tool_call")) { @@ -712,7 +725,7 @@ export class AgentSession { private _lastAssistantMessage: AssistantMessage | undefined = undefined; /** Internal handler for agent events - shared by subscribe and reconnect */ - private _handleAgentEvent = async (event: AgentEvent): Promise => { + private _handleAgentEvent = async (event: AgentEvent, signal?: AbortSignal): Promise => { // When a user message starts, check if it's from either queue and remove it BEFORE emitting // This ensures the UI sees the updated queue state if (event.type === "message_start" && event.message.role === "user") { @@ -736,7 +749,7 @@ export class AgentSession { } // Emit to extensions first - await this._emitExtensionEvent(event); + await this._emitExtensionEvent(event, signal); // Notify all listeners this._emit(event.type === "agent_end" ? { ...event, willRetry: this._willRetryAfterAgentEnd(event) } : event); @@ -838,7 +851,7 @@ export class AgentSession { } /** Emit extension events based on agent events */ - private async _emitExtensionEvent(event: AgentEvent): Promise { + private async _emitExtensionEvent(event: AgentEvent, signal?: AbortSignal): Promise { if (event.type === "agent_start") { this._turnIndex = 0; await this._extensionRunner.emit({ type: "agent_start" }); @@ -882,7 +895,7 @@ export class AgentSession { if (replacement) { // Untyped extension handlers can return messages with null/missing content; // normalize so it never enters agent state or session history. - const normalized = + let normalized = (replacement.role === "user" || replacement.role === "assistant" || replacement.role === "toolResult" || @@ -890,6 +903,28 @@ export class AgentSession { replacement.content == null ? ({ ...replacement, content: [] } as AgentMessage) : replacement; + // message_end replacements happen after terminal tool events. Bound an + // accepted replacement before the shared message is persisted or replayed. + if (normalized.role === "toolResult") { + try { + normalized = { ...normalized, content: await this._boundToolContent(normalized.content, signal) }; + } catch (error) { + const reason = error instanceof Error ? error.message.slice(0, 512) : "Unknown retention error"; + normalized = { + ...normalized, + isError: true, + content: [ + { + type: "text", + text: `Tool result replacement could not be retained: ${reason}. The tool may already have run; check its effects before repeating a state-changing call.`, + }, + ...(Array.isArray(normalized.content) + ? normalized.content.filter((part) => part?.type === "image") + : []), + ], + }; + } + } this._replaceMessageInPlace(event.message, normalized); } } else if (event.type === "tool_execution_start") { diff --git a/packages/coding-agent/src/core/tools/tool-output.ts b/packages/coding-agent/src/core/tools/tool-output.ts new file mode 100644 index 00000000..de8e0ca5 --- /dev/null +++ b/packages/coding-agent/src/core/tools/tool-output.ts @@ -0,0 +1,91 @@ +import { randomBytes } from "node:crypto"; +import { lstat, mkdir, open, opendir, unlink } from "node:fs/promises"; +import { join, resolve } from "node:path"; +import type { ImageContent, TextContent } from "@step-harness/providers"; +import { DEFAULT_MAX_BYTES, DEFAULT_MAX_LINES, truncateHead } from "./truncate.ts"; + +type Content = (TextContent | ImageContent)[]; +const RETENTION_MS = 7 * 24 * 60 * 60 * 1000; +const CLEANUP_SCAN_LIMIT = 100; +const OWNED_FILE = /^tool-[0-9a-f]{32}\.txt$/u; + +/** Bound final model-visible text while keeping the full textual view available on disk. */ +export async function boundToolResultContent( + content: Content, + directory: string, + signal?: AbortSignal, +): Promise { + const text = content + .filter((part) => part.type === "text") + .map((part) => part.text) + .join("\n"); + const original = truncateHead(text); + if (!original.truncated) return content; + + signal?.throwIfAborted(); + const outputDirectory = resolve(directory); + await mkdir(outputDirectory, { recursive: true, mode: 0o700 }); + const path = join(outputDirectory, `tool-${randomBytes(16).toString("hex")}.txt`); + const file = await open(path, "wx", 0o600); + let written = false; + try { + await file.writeFile(text, { encoding: "utf8", signal }); + written = true; + } finally { + await file.close(); + if (!written) await unlink(path).catch(() => undefined); + } + + const marker = (lines: number) => + `[Showing first ${lines} of ${original.totalLines} lines (${original.totalBytes} bytes total). Full output: ${JSON.stringify(path)}]`; + const preview = truncateHead(text, { + maxLines: DEFAULT_MAX_LINES - 2, + maxBytes: DEFAULT_MAX_BYTES - Buffer.byteLength(marker(DEFAULT_MAX_LINES), "utf8") - 2, + }); + const bounded: Content = []; + let offset = 0; + let seenText = false; + let marked = false; + for (const part of content) { + if (part.type !== "text") { + bounded.push(part); + continue; + } + const start = offset + (seenText ? 1 : 0); + offset = start + part.text.length; + seenText = true; + if (marked) continue; + // Include empty blocks at an exact boundary: they represent the final + // newline in a retained prefix, and dropping one misreports shown lines. + if (preview.outputLines > 0 && start <= preview.content.length) { + const kept = part.text.slice(0, preview.content.length - start); + bounded.push(kept === part.text ? part : { ...part, text: kept }); + if (offset <= preview.content.length) continue; + } + bounded.push({ type: "text", text: marker(preview.outputLines) }); + marked = true; + } + if (!marked) bounded.push({ type: "text", text: marker(preview.outputLines) }); + + await cleanupExpiredOutput(outputDirectory, signal); + return bounded; +} + +/** Inspect a bounded part of this module's own namespace; cleanup never fails the tool. */ +async function cleanupExpiredOutput(directory: string, signal?: AbortSignal): Promise { + try { + const entries = await opendir(directory); + let inspected = 0; + for await (const entry of entries) { + if (signal?.aborted || inspected++ >= CLEANUP_SCAN_LIMIT) break; + if (!entry.isFile() || !OWNED_FILE.test(entry.name)) continue; + const path = join(directory, entry.name); + const stats = await lstat(path).catch(() => undefined); + if (stats?.isFile() && stats.mtimeMs < Date.now() - RETENTION_MS) { + await unlink(path).catch(() => undefined); + } + } + } catch { + // Retention remains useful if a concurrent cleanup or filesystem error prevents a sweep. + } +} diff --git a/packages/coding-agent/test/suite/agent-session-tool-output.test.ts b/packages/coding-agent/test/suite/agent-session-tool-output.test.ts new file mode 100644 index 00000000..06607da6 --- /dev/null +++ b/packages/coding-agent/test/suite/agent-session-tool-output.test.ts @@ -0,0 +1,234 @@ +import { readFile, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import type { AgentTool, ToolExecutionMode } from "@step-harness/agent-core"; +import { fauxAssistantMessage, fauxToolCall } from "@step-harness/providers"; +import { Type } from "typebox"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { DEFAULT_MAX_BYTES, DEFAULT_MAX_LINES } from "../../src/core/tools/truncate.ts"; +import { createHarness, type Harness } from "./harness.ts"; + +const harnesses: Harness[] = []; +afterEach(() => { + while (harnesses.length) harnesses.pop()?.cleanup(); + vi.restoreAllMocks(); +}); + +function text(content: readonly { type: string; text?: string }[]) { + return content + .filter((part) => part.type === "text") + .map((part) => part.text) + .join("\n"); +} + +async function runOutput(output: string, hookOutput?: string) { + const remote: AgentTool = { + name: "remote_output", + label: "Remote output", + description: "Return remote text", + parameters: Type.Object({}), + execute: async () => ({ + content: [{ type: "text", text: output }], + details: { truncated: true, preserved: "metadata" }, + }), + }; + const harness = await createHarness({ + tools: [remote], + settings: { compaction: { enabled: false } }, + extensionFactories: + hookOutput === undefined + ? undefined + : [ + (pi) => { + pi.on("tool_result", () => ({ content: [{ type: "text", text: hookOutput }] })); + }, + ], + }); + harnesses.push(harness); + vi.spyOn(harness.sessionManager, "getSessionDir").mockReturnValue(join(harness.tempDir, "sessions")); + let requestText = ""; + harness.setResponses([ + fauxAssistantMessage(fauxToolCall("remote_output", {}), { stopReason: "toolUse" }), + (context) => { + const result = context.messages.find((message) => message.role === "toolResult"); + if (result?.role === "toolResult") requestText = text(result.content); + return fauxAssistantMessage("done"); + }, + ]); + await harness.session.prompt("get remote output"); + return { harness, requestText }; +} + +describe("AgentSession final tool output", () => { + it.each([ + ["many lines", Array.from({ length: 2100 }, (_, n) => `line ${n}`).join("\n")], + ["one long line", "x".repeat(DEFAULT_MAX_BYTES + 1000)], + ["multibyte output", "δΈ­ζ–‡πŸ™‚\n".repeat(9000)], + ])("bounds %s and retains the exact complete text", async (_name, original) => { + const { harness, requestText } = await runOutput(original); + expect(Buffer.byteLength(requestText)).toBeLessThanOrEqual(DEFAULT_MAX_BYTES); + expect(requestText.split("\n").length).toBeLessThanOrEqual(DEFAULT_MAX_LINES); + const match = requestText.match(/Full output: (.+)\]/); + expect(match).not.toBeNull(); + const path = JSON.parse(match![1]) as string; + expect(path.startsWith(harness.tempDir)).toBe(true); + expect(await readFile(path, "utf8")).toBe(original); + const result = harness.session.messages.find((message) => message.role === "toolResult"); + expect(result).toMatchObject({ details: { truncated: true, preserved: "metadata" }, isError: false }); + expect(harness.eventsOfType("tool_execution_end")[0].result.content).toEqual(result?.content); + }); + + it("bounds text supplied by a tool_result extension", async () => { + const original = "hooked\n".repeat(9000); + const { requestText } = await runOutput("small", original); + expect(Buffer.byteLength(requestText)).toBeLessThanOrEqual(DEFAULT_MAX_BYTES); + const match = requestText.match(/Full output: (.+)\]/); + expect(match).not.toBeNull(); + expect(await readFile(JSON.parse(match![1]), "utf8")).toBe(original); + }); + + it.each(["sequential", "parallel"])( + "bounds validation and blocked results in %s mode", + async (mode) => { + const remote: AgentTool = { + name: "validate", + label: "Validate", + description: "Validate", + parameters: Type.Object({ count: Type.Number() }), + execute: vi.fn(async () => ({ content: [{ type: "text" as const, text: "unreachable" }], details: {} })), + }; + const harness = await createHarness({ + tools: [remote], + settings: { compaction: { enabled: false } }, + extensionFactories: [ + (pi) => { + pi.on("tool_call", () => ({ block: true, reason: "denied ".repeat(9000), terminate: true })); + }, + ], + }); + harnesses.push(harness); + vi.spyOn(harness.sessionManager, "getSessionDir").mockReturnValue(join(harness.tempDir, "sessions")); + harness.session.agent.toolExecution = mode; + harness.setResponses([ + fauxAssistantMessage( + [fauxToolCall("validate", { count: "bad ".repeat(20000) }), fauxToolCall("validate", { count: 1 })], + { stopReason: "toolUse" }, + ), + fauxAssistantMessage("done"), + ]); + await harness.session.prompt("validate"); + expect(remote.execute).not.toHaveBeenCalled(); + const results = harness.session.messages.filter((message) => message.role === "toolResult"); + expect(results).toHaveLength(2); + for (const result of results) { + expect(result.isError).toBe(true); + expect(Buffer.byteLength(text(result.content))).toBeLessThanOrEqual(DEFAULT_MAX_BYTES); + expect(text(result.content)).toContain("Full output:"); + } + }, + ); + + it("reports retention failure without losing a terminating tool's metadata", async () => { + const execute = vi.fn(async () => ({ + content: [{ type: "text" as const, text: "x".repeat(60000) }], + details: { effect: "already completed" }, + terminate: true, + })); + const harness = await createHarness({ + tools: [{ name: "finish", label: "Finish", description: "Finish", parameters: Type.Object({}), execute }], + settings: { compaction: { enabled: false } }, + }); + harnesses.push(harness); + const unavailable = join(harness.tempDir, "not-a-directory"); + await writeFile(unavailable, "keep"); + vi.spyOn(harness.sessionManager, "getSessionDir").mockReturnValue(unavailable); + harness.setResponses([fauxAssistantMessage(fauxToolCall("finish", {}), { stopReason: "toolUse" })]); + await harness.session.prompt("finish"); + expect(execute).toHaveBeenCalledTimes(1); + const result = harness.session.messages.at(-1); + expect(result).toMatchObject({ role: "toolResult", isError: true, details: { effect: "already completed" } }); + if (result?.role !== "toolResult") throw new Error("Expected tool result"); + expect(text(result.content)).toContain("may already have run"); + expect(Buffer.byteLength(text(result.content))).toBeLessThanOrEqual(DEFAULT_MAX_BYTES); + expect(await readFile(unavailable, "utf8")).toBe("keep"); + }); + + it.each([false, true])( + "bounds message_end replacements before persistence (storage failure: %s)", + async (storageFailure) => { + const original = "replacement".repeat(6000); + const image = { type: "image" as const, data: "aGVsbG8=", mimeType: "image/png" }; + const execute = vi.fn(async () => ({ + content: [{ type: "text" as const, text: "small" }], + details: {}, + terminate: storageFailure, + })); + const harness = await createHarness({ + tools: [ + { + name: "replace_late", + label: "Replace late", + description: "Replace late", + parameters: Type.Object({}), + execute, + }, + ], + settings: { compaction: { enabled: false } }, + extensionFactories: [ + (pi) => { + pi.on("message_end", (event) => { + if (event.message.role !== "toolResult") return; + return { + message: { + ...event.message, + content: [{ type: "text", text: original }, image], + details: { replacement: "kept" }, + }, + }; + }); + }, + ], + }); + harnesses.push(harness); + const directory = join(harness.tempDir, "sessions"); + if (storageFailure) await writeFile(directory, "unavailable"); + vi.spyOn(harness.sessionManager, "getSessionDir").mockReturnValue(directory); + harness.setResponses([fauxAssistantMessage(fauxToolCall("replace_late", {}), { stopReason: "toolUse" })]); + if (!storageFailure) { + harness.appendResponses([ + (context) => { + const result = context.messages.find((message) => message.role === "toolResult"); + if (result?.role !== "toolResult") throw new Error("Expected model tool result"); + expect(Buffer.byteLength(text(result.content))).toBeLessThanOrEqual(DEFAULT_MAX_BYTES); + return fauxAssistantMessage("done"); + }, + ]); + } + await harness.session.prompt("replace late"); + expect(execute).toHaveBeenCalledTimes(1); + const result = harness.session.messages.find((message) => message.role === "toolResult"); + if (result?.role !== "toolResult") throw new Error("Expected tool result"); + expect(Buffer.byteLength(text(result.content))).toBeLessThanOrEqual(DEFAULT_MAX_BYTES); + expect(result.content.filter((part) => part.type === "image")).toEqual([image]); + expect(result.details).toEqual({ replacement: "kept" }); + expect(result.isError).toBe(storageFailure); + const stored = harness.sessionManager + .getBranch() + .filter((entry) => entry.type === "message" && entry.message.role === "toolResult") + .at(-1); + if (stored?.type !== "message" || stored.message.role !== "toolResult") + throw new Error("Expected persisted tool result"); + expect(stored.message.content).toEqual(result.content); + if (storageFailure) { + expect(text(result.content)).toContain("may already have run"); + } else { + const path = JSON.parse(text(result.content).match(/Full output: (.+)\]/)![1]); + expect(await readFile(path, "utf8")).toBe(original); + } + }, + ); + + it("keeps small output unchanged", async () => { + const { requestText } = await runOutput("unchanged"); + expect(requestText).toBe("unchanged"); + }); +}); diff --git a/packages/coding-agent/test/tool-output.test.ts b/packages/coding-agent/test/tool-output.test.ts new file mode 100644 index 00000000..e864a38f --- /dev/null +++ b/packages/coding-agent/test/tool-output.test.ts @@ -0,0 +1,147 @@ +import { mkdir, mkdtemp, readdir, readFile, rm, stat, symlink, utimes, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { isAbsolute, join, relative } from "node:path"; +import type { ImageContent, TextContent } from "@step-harness/providers"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { boundToolResultContent } from "../src/core/tools/tool-output.ts"; +import { DEFAULT_MAX_BYTES, DEFAULT_MAX_LINES } from "../src/core/tools/truncate.ts"; + +type Content = (TextContent | ImageContent)[]; +let root: string; +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), "step-tool-output-test-")); +}); +afterEach(async () => { + await rm(root, { recursive: true, force: true }); +}); + +function text(content: Content) { + return content + .filter((part) => part.type === "text") + .map((part) => part.text) + .join("\n"); +} +function artifact(content: Content) { + const match = text(content).match(/Full output: (.+)\]/); + expect(match).not.toBeNull(); + return JSON.parse(match![1]) as string; +} + +it("does not create storage for an unchanged small result", async () => { + const content: Content = [{ type: "text", text: "small" }]; + expect(await boundToolResultContent(content, join(root, "not-created"))).toBe(content); + expect(await readdir(root)).toEqual([]); +}); + +it("returns an absolute readable artifact path for a relative storage directory", async () => { + const content = await boundToolResultContent( + [{ type: "text", text: "x".repeat(60000) }], + relative(process.cwd(), join(root, "relative")), + ); + expect(isAbsolute(artifact(content))).toBe(true); + expect(await readFile(artifact(content), "utf8")).toBe("x".repeat(60000)); +}); + +it("bounds combined text while preserving image identity and ordering", async () => { + const first: ImageContent = { type: "image", data: "first", mimeType: "image/png" }; + const second: ImageContent = { type: "image", data: "second", mimeType: "image/png" }; + const a = "Ξ±\n".repeat(1500); + const b = "δΈ­ζ–‡\n".repeat(1500); + const result = await boundToolResultContent( + [{ type: "text", text: a }, first, { type: "text", text: b }, second], + root, + ); + const images = result.filter((part) => part.type === "image"); + expect(images).toEqual([first, second]); + expect(images[0]).toBe(first); + expect(images[1]).toBe(second); + expect(Buffer.byteLength(text(result))).toBeLessThanOrEqual(DEFAULT_MAX_BYTES); + expect(text(result).split("\n").length).toBeLessThanOrEqual(DEFAULT_MAX_LINES); + expect(await readFile(artifact(result), "utf8")).toBe(`${a}\n${b}`); +}); + +it("accounts for empty text blocks in the preview line count", async () => { + const result = await boundToolResultContent( + Array.from({ length: 3001 }, () => ({ type: "text", text: "" })), + root, + ); + const texts = result.filter((part) => part.type === "text"); + const notice = texts.at(-1)!.text; + const shown = Number(notice.match(/Showing first (\d+)/)![1]); + expect(text(texts.slice(0, -1))).toBe("\n".repeat(shown - 1)); + expect(text(result).split("\n").length).toBeLessThanOrEqual(DEFAULT_MAX_LINES); +}); + +it("retains a UTF-8-safe prefix and the entire multibyte source", async () => { + const original = "πŸ™‚δΈ­ζ–‡\n".repeat(10000); + const result = await boundToolResultContent([{ type: "text", text: original }], root); + expect(Buffer.byteLength(text(result))).toBeLessThanOrEqual(DEFAULT_MAX_BYTES); + expect(text(result)).not.toContain("\uFFFD"); + expect(await readFile(artifact(result), "utf8")).toBe(original); +}); + +it("honors cancellation before creating an artifact", async () => { + const controller = new AbortController(); + controller.abort(new Error("user cancelled")); + await expect( + boundToolResultContent([{ type: "text", text: "x".repeat(60000) }], join(root, "cancelled"), controller.signal), + ).rejects.toThrow("user cancelled"); + expect(await readdir(root)).toEqual([]); +}); + +it("does not claim retention succeeded when storage is unavailable", async () => { + const directory = join(root, "not-a-directory"); + await writeFile(directory, "keep"); + await expect(boundToolResultContent([{ type: "text", text: "x".repeat(60000) }], directory)).rejects.toThrow(); + expect(await readFile(directory, "utf8")).toBe("keep"); +}); + +it("creates distinct private artifacts for repeated calls", async () => { + const directory = join(root, "private"); + const content: Content = [{ type: "text", text: "x".repeat(60000) }]; + const results = await Promise.all([ + boundToolResultContent(content, directory), + boundToolResultContent(content, directory), + ]); + const files = results.map(artifact); + expect(files[0]).not.toBe(files[1]); + if (process.platform !== "win32") { + for (const file of files) expect((await stat(file)).mode & 0o077).toBe(0); + expect((await stat(directory)).mode & 0o077).toBe(0); + } +}); + +describe("owned artifact retention", () => { + it("cleans expired owned files without following symlinks or removing unrelated files", async () => { + const directory = join(root, "output"); + await mkdir(directory); + const expired = join(directory, `tool-${"a".repeat(32)}.txt`); + const unrelated = join(directory, "notes.txt"); + const outside = join(root, "outside.txt"); + await Promise.all([writeFile(expired, "old"), writeFile(unrelated, "keep"), writeFile(outside, "outside")]); + const old = new Date(Date.now() - 8 * 24 * 60 * 60 * 1000); + await Promise.all([utimes(expired, old, old), utimes(unrelated, old, old)]); + if (process.platform !== "win32") await symlink(outside, join(directory, `tool-${"b".repeat(32)}.txt`)); + const result = await boundToolResultContent([{ type: "text", text: "x".repeat(60000) }], directory); + await expect(stat(expired)).rejects.toMatchObject({ code: "ENOENT" }); + expect(await readFile(unrelated, "utf8")).toBe("keep"); + expect(await readFile(outside, "utf8")).toBe("outside"); + expect(await readFile(artifact(result), "utf8")).toBe("x".repeat(60000)); + }); +}); + +it("keeps the notice within the line budget when the artifact path contains line breaks", async () => { + if (process.platform === "win32") return; + const directory = join(root, "part\none\ntwo\nthree"); + const original = "line\n".repeat(2500); + const result = await boundToolResultContent([{ type: "text", text: original }], directory); + expect(text(result).split("\n").length).toBeLessThanOrEqual(DEFAULT_MAX_LINES); + const encodedPath = text(result).match(/Full output: (.+)\]/)![1]; + expect(await readFile(JSON.parse(encodedPath), "utf8")).toBe(original); +}); + +it("keeps a retained empty first line consistent with the notice", async () => { + const original = `\n${"x".repeat(60000)}`; + const result = await boundToolResultContent([{ type: "text", text: original }], root); + expect(text(result)).toMatch(/^\n\[Showing first 1 of 2 lines/); +}); From 138de500fc3e5aa760c239a289ec2f2a13494c5e Mon Sep 17 00:00:00 2001 From: longyongshen Date: Mon, 28 Sep 2026 19:34:09 +0800 Subject: [PATCH 2/3] fix(providers): drop empty text blocks from Anthropic tool results A tool result with images is sent in block form, and whitespace-only text blocks were forwarded unchanged. The API rejects them, and the result stays in history, so every later request in the session fails. Drop them as the user and assistant paths already do; the image placeholder still applies when no text remains. --- .../providers/src/api/anthropic-messages.ts | 6 +- .../anthropic-tool-result-empty-text.test.ts | 83 +++++++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 packages/providers/test/anthropic-tool-result-empty-text.test.ts diff --git a/packages/providers/src/api/anthropic-messages.ts b/packages/providers/src/api/anthropic-messages.ts index a5742e8b..46643e9b 100644 --- a/packages/providers/src/api/anthropic-messages.ts +++ b/packages/providers/src/api/anthropic-messages.ts @@ -78,8 +78,10 @@ function convertContentBlocks(content: (TextContent | ImageContent)[]): return sanitizeSurrogates(content.map((c) => (c as TextContent).text).join("\n")); } - // If we have images, convert to content block array - const blocks = content.map((block) => { + // If we have images, convert to content block array. Anthropic rejects + // whitespace-only text blocks, so drop them as user and assistant content does. + const kept = content.filter((block) => block.type !== "text" || block.text.trim().length > 0); + const blocks = kept.map((block) => { if (block.type === "text") { return { type: "text" as const, diff --git a/packages/providers/test/anthropic-tool-result-empty-text.test.ts b/packages/providers/test/anthropic-tool-result-empty-text.test.ts new file mode 100644 index 00000000..aea00eb5 --- /dev/null +++ b/packages/providers/test/anthropic-tool-result-empty-text.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it } from "vitest"; +import { streamSimple } from "../src/compat.ts"; +import type { AssistantMessage, Context, ImageContent, TextContent } from "../src/types.ts"; +import { anthropicModel } from "./helpers/step-fixtures.ts"; + +interface AnthropicBlock { + type: string; + text?: string; + content?: string | AnthropicBlock[]; +} + +interface AnthropicPayload { + messages: Array<{ content: string | AnthropicBlock[] }>; +} + +class PayloadCaptured extends Error {} + +const toolCall: AssistantMessage = { + role: "assistant", + content: [{ type: "toolCall", id: "call_1", name: "screenshot", arguments: {} }], + api: "anthropic-messages", + provider: "anthropic", + model: "claude-opus-4-6", + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "toolUse", + timestamp: 2, +}; + +async function toolResultContent(content: (TextContent | ImageContent)[]): Promise { + const context: Context = { + messages: [ + { role: "user", content: "Look", timestamp: 1 }, + toolCall, + { role: "toolResult", toolCallId: "call_1", toolName: "screenshot", content, isError: false, timestamp: 3 }, + ], + }; + let captured: AnthropicPayload | undefined; + const stream = streamSimple(anthropicModel({ input: ["text", "image"], baseUrl: "http://127.0.0.1:9" }), context, { + apiKey: "fake-key", + onPayload: (payload) => { + captured = payload as AnthropicPayload; + throw new PayloadCaptured(); + }, + }); + await stream.result(); + for (const message of captured?.messages ?? []) { + if (typeof message.content === "string") continue; + const result = message.content.find((block) => block.type === "tool_result"); + if (result && Array.isArray(result.content)) return result.content; + } + throw new Error("No block-form tool result in payload"); +} + +describe("Anthropic tool results with images", () => { + const image: ImageContent = { type: "image", data: "AAAA", mimeType: "image/png" }; + + it("drops whitespace-only text blocks the API would reject", async () => { + const content = await toolResultContent([ + { type: "text", text: "head" }, + { type: "text", text: "" }, + { type: "text", text: " \n" }, + { type: "text", text: "[notice]" }, + image, + ]); + expect(content.map((block) => block.type)).toEqual(["text", "text", "image"]); + expect(content.filter((block) => block.type === "text").map((block) => block.text)).toEqual(["head", "[notice]"]); + }); + + it("keeps the image placeholder when every text block is empty", async () => { + const content = await toolResultContent([{ type: "text", text: "" }, image]); + expect(content).toEqual([ + { type: "text", text: "(see attached image)" }, + expect.objectContaining({ type: "image" }), + ]); + }); +}); From 547c0cdfb1c3f31ba345c6a57a0f46e5b2e712cc Mon Sep 17 00:00:00 2001 From: longyongshen Date: Mon, 28 Sep 2026 19:34:21 +0800 Subject: [PATCH 3/3] fix(coding-agent): leave room for producer truncation trailers Native bash keeps the last 2,000 lines / 50 KiB, then appends its notice and exit status, which puts the result just over the final bound. The head-only pass then cut exactly that tail: the last lines, the full-log path and "Command exited with code N" of a failed build. Allow 8 lines and 1 KiB of trailer before bounding; larger results are still bounded as before. --- .../docs/tool-output-retention.md | 7 ++- .../src/core/tools/tool-output.ts | 10 +++- .../suite/agent-session-tool-output.test.ts | 53 ++++++++++++++++++- .../coding-agent/test/tool-output.test.ts | 16 ++++++ 4 files changed, 82 insertions(+), 4 deletions(-) diff --git a/packages/coding-agent/docs/tool-output-retention.md b/packages/coding-agent/docs/tool-output-retention.md index 8e75e30f..d253a8b7 100644 --- a/packages/coding-agent/docs/tool-output-retention.md +++ b/packages/coding-agent/docs/tool-output-retention.md @@ -11,8 +11,11 @@ An oversized result keeps a prefix and a `Full output:` path. The complete text from all text blocks, joined by newlines, is saved before the preview is published. Individual oversized lines may leave no complete line in the preview; the full file remains readable with the normal file tools. A producer's -`truncated` metadata does not disable this final bound. If a producer already -lost content before returning, this file contains only what it returned. +`truncated` metadata does not disable this final bound. Results that exceed the +limits by at most 8 lines and 1 KiB are left unchanged: built-in tools truncate +to the same limits and then append a notice and, for bash, an exit status, and +that tail must not be cut again. If a producer already lost content before +returning, this file contains only what it returned. Artifacts live in `tool-output` under the session directory. If the session has no storage directory, the configured agent directory (or its default) is used. diff --git a/packages/coding-agent/src/core/tools/tool-output.ts b/packages/coding-agent/src/core/tools/tool-output.ts index de8e0ca5..186a1c05 100644 --- a/packages/coding-agent/src/core/tools/tool-output.ts +++ b/packages/coding-agent/src/core/tools/tool-output.ts @@ -8,6 +8,11 @@ type Content = (TextContent | ImageContent)[]; const RETENTION_MS = 7 * 24 * 60 * 60 * 1000; const CLEANUP_SCAN_LIMIT = 100; const OWNED_FILE = /^tool-[0-9a-f]{32}\.txt$/u; +// Built-in tools truncate to the same limits, then append a notice and, for +// bash, an exit status. Leave room for that trailer so a tail they kept on +// purpose is not cut by a second, head-only pass. +const PRODUCER_TRAILER_LINES = 8; +const PRODUCER_TRAILER_BYTES = 1024; /** Bound final model-visible text while keeping the full textual view available on disk. */ export async function boundToolResultContent( @@ -19,7 +24,10 @@ export async function boundToolResultContent( .filter((part) => part.type === "text") .map((part) => part.text) .join("\n"); - const original = truncateHead(text); + const original = truncateHead(text, { + maxLines: DEFAULT_MAX_LINES + PRODUCER_TRAILER_LINES, + maxBytes: DEFAULT_MAX_BYTES + PRODUCER_TRAILER_BYTES, + }); if (!original.truncated) return content; signal?.throwIfAborted(); diff --git a/packages/coding-agent/test/suite/agent-session-tool-output.test.ts b/packages/coding-agent/test/suite/agent-session-tool-output.test.ts index 06607da6..c8568a67 100644 --- a/packages/coding-agent/test/suite/agent-session-tool-output.test.ts +++ b/packages/coding-agent/test/suite/agent-session-tool-output.test.ts @@ -1,10 +1,14 @@ +import { mkdtempSync, rmSync } from "node:fs"; import { readFile, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; import { join } from "node:path"; import type { AgentTool, ToolExecutionMode } from "@step-harness/agent-core"; import { fauxAssistantMessage, fauxToolCall } from "@step-harness/providers"; import { Type } from "typebox"; import { afterEach, describe, expect, it, vi } from "vitest"; import { DEFAULT_MAX_BYTES, DEFAULT_MAX_LINES } from "../../src/core/tools/truncate.ts"; +import { createStepExtension } from "../../src/features/step.ts"; +import { createStepToolProfile } from "../../src/step/tool-profile.ts"; import { createHarness, type Harness } from "./harness.ts"; const harnesses: Harness[] = []; @@ -61,7 +65,7 @@ async function runOutput(output: string, hookOutput?: string) { describe("AgentSession final tool output", () => { it.each([ ["many lines", Array.from({ length: 2100 }, (_, n) => `line ${n}`).join("\n")], - ["one long line", "x".repeat(DEFAULT_MAX_BYTES + 1000)], + ["one long line", "x".repeat(DEFAULT_MAX_BYTES + 2000)], ["multibyte output", "δΈ­ζ–‡πŸ™‚\n".repeat(9000)], ])("bounds %s and retains the exact complete text", async (_name, original) => { const { harness, requestText } = await runOutput(original); @@ -231,4 +235,51 @@ describe("AgentSession final tool output", () => { const { requestText } = await runOutput("unchanged"); expect(requestText).toBe("unchanged"); }); + + it("keeps the tail and exit status of a failed command that bash already truncated", async () => { + if (process.platform === "win32") return; + const sandbox = mkdtempSync(join(tmpdir(), "step-tool-output-bash-")); + vi.stubEnv("STEP_CODING_AGENT_DIR", join(sandbox, "agent")); + try { + const harness = await createHarness({ + tools: [], + initialActiveToolNames: ["run_command"], + settings: { compaction: { enabled: false } }, + extensionFactories: [ + createStepExtension({ + permission: { env: {}, initialPreset: "bypass", toolOverrides: { run_command: "allow" } }, + }), + (pi) => { + const tool = createStepToolProfile(sandbox, { agentDir: join(sandbox, "agent") }).find( + (candidate) => candidate.name === "run_command", + ); + if (!tool) throw new Error("Step run_command tool is missing"); + pi.registerTool(tool); + }, + ], + }); + harnesses.push(harness); + await harness.session.bindExtensions({ mode: "print" }); + let requestText = ""; + harness.setResponses([ + fauxAssistantMessage(fauxToolCall("run_command", { command: "seq 1 3000; exit 1", cwd: sandbox }), { + stopReason: "toolUse", + }), + (context) => { + const result = context.messages.find((message) => message.role === "toolResult"); + if (result?.role === "toolResult") requestText = text(result.content); + return fauxAssistantMessage("done"); + }, + ]); + await harness.session.prompt("run the build"); + await harness.session.extensionRunner.emit({ type: "session_shutdown", reason: "quit" }); + expect(requestText).toContain("\n3000\n"); + expect(requestText).toMatch(/Full output: \S*step-bash-\S+\.log\]/); + expect(requestText.endsWith("Command exited with code 1")).toBe(true); + expect(requestText).not.toContain("Showing first"); + } finally { + vi.unstubAllEnvs(); + rmSync(sandbox, { recursive: true, force: true }); + } + }); }); diff --git a/packages/coding-agent/test/tool-output.test.ts b/packages/coding-agent/test/tool-output.test.ts index e864a38f..b0504ef2 100644 --- a/packages/coding-agent/test/tool-output.test.ts +++ b/packages/coding-agent/test/tool-output.test.ts @@ -145,3 +145,19 @@ it("keeps a retained empty first line consistent with the notice", async () => { const result = await boundToolResultContent([{ type: "text", text: original }], root); expect(text(result)).toMatch(/^\n\[Showing first 1 of 2 lines/); }); + +it("leaves a producer's own truncation notice and exit status in place", async () => { + // Native bash keeps the last 2,000 lines, then appends its notice and status. + const kept = Array.from({ length: DEFAULT_MAX_LINES }, (_, n) => `line ${n + 1001}`).join("\n"); + const original = `${kept}\n\n[Showing lines 1001-3000 of 3000. Full output: /tmp/step-bash-0123456789abcdef.log]\n\nCommand exited with code 1`; + const content: Content = [{ type: "text", text: original }]; + expect(await boundToolResultContent(content, join(root, "not-created"))).toBe(content); + expect(await readdir(root)).toEqual([]); +}); + +it("still bounds output that exceeds the room left for a producer trailer", async () => { + const original = "line\n".repeat(DEFAULT_MAX_LINES + 20); + const result = await boundToolResultContent([{ type: "text", text: original }], root); + expect(text(result).split("\n").length).toBeLessThanOrEqual(DEFAULT_MAX_LINES); + expect(await readFile(artifact(result), "utf8")).toBe(original); +});