From 2841bd47e2ac0ef3cc1208f710c44247f15effef Mon Sep 17 00:00:00 2001 From: Aliou Diallo Date: Mon, 27 Jul 2026 18:18:19 +0200 Subject: [PATCH] fix(tool): bound persisted process output Closes #58. `process output` no longer persists raw stdout/stderr arrays in tool-result `details`. Output details now contain only metadata (action, success, message, log file paths, optional truncation info), while the bounded text preview is returned as tool-result `content`. Multi-megabyte single lines, CR-only progress output, and JSON-escaping expansion can no longer produce multi-megabyte session entries. The agent still receives the newest bounded output and complete-log paths. New and legacy results render without errors. No manager or config behavior changes. --- .changeset/output-no-raw-details.md | 5 + src/constants/types.ts | 13 + src/tools/actions/debug.ts | 39 +-- src/tools/actions/output-truncate.ts | 187 +++++++++++++ src/tools/actions/output.test.ts | 392 +++++++++++++++++++++++++++ src/tools/actions/output.ts | 378 +++++++++++++++++++------- 6 files changed, 898 insertions(+), 116 deletions(-) create mode 100644 .changeset/output-no-raw-details.md create mode 100644 src/tools/actions/output-truncate.ts create mode 100644 src/tools/actions/output.test.ts diff --git a/.changeset/output-no-raw-details.md b/.changeset/output-no-raw-details.md new file mode 100644 index 0000000..73284a6 --- /dev/null +++ b/.changeset/output-no-raw-details.md @@ -0,0 +1,5 @@ +--- +"@aliou/pi-processes": patch +--- + +Fix issue #58: `process output` no longer persists raw stdout/stderr arrays in tool-result `details`. Output details now contain only metadata (action, success, message, log file paths, optional truncation info), while the bounded text preview is returned as tool-result `content`. Multi-megabyte single lines, CR-only progress output, and JSON-escaping expansion can no longer produce multi-megabyte session entries. diff --git a/src/constants/types.ts b/src/constants/types.ts index f07da95..0e71503 100644 --- a/src/constants/types.ts +++ b/src/constants/types.ts @@ -89,14 +89,27 @@ export interface StartOptions { logWatches?: LogWatch[]; } +import type { TruncationDetails } from "../tools/actions/output-truncate"; + export interface ProcessesDetails { action: ProcessAction; success: boolean; message: string; process?: ProcessInfo; processes?: ProcessInfo[]; + /** + * Legacy-only raw output arrays retained so historical session results can + * still render. New executions must never populate this field; the bounded + * preview lives in tool-result `content` and a truncation summary lives in + * `truncation`. + */ output?: { stdout: string[]; stderr: string[]; status: string }; logFiles?: { stdoutFile: string; stderrFile: string }; + /** + * Metadata only (no raw output) describing how `content` was bounded. + * Present when the body exceeded the byte or line limits. + */ + truncation?: TruncationDetails; cleared?: number; } diff --git a/src/tools/actions/debug.ts b/src/tools/actions/debug.ts index e844909..fef2ee3 100644 --- a/src/tools/actions/debug.ts +++ b/src/tools/actions/debug.ts @@ -101,26 +101,33 @@ export function executeDebugPreview(params: DebugParams): ExecuteResult { } if (preview === "output") { + const message = + '"demo-server" (proc_42) [running]: 4 stdout lines, 2 stderr lines'; + const content = [ + message, + "stdout:", + "starting...", + "loading config", + "ready on http://localhost:3000", + "watching for changes", + "", + "stderr:", + "warn: deprecated option in config", + "error: simulated stack trace line", + "", + "Process is still running. Use watches instead of polling.", + "", + "[Complete currently-retained logs:", + "stdout=/tmp/pi-processes-demo/proc_42-stdout.log", + "stderr=/tmp/pi-processes-demo/proc_42-stderr.log]", + ].join("\n"); + return { - content: [{ type: "text", text: "Debug preview: output" }], + content: [{ type: "text", text: content }], details: { action: "output", success: true, - message: - '"demo-server" (proc_42) [running]: 4 stdout lines, 2 stderr lines', - output: { - status: "running", - stdout: [ - "starting...", - "loading config", - "ready on http://localhost:3000", - "watching for changes", - ], - stderr: [ - "warn: deprecated option in config", - "error: simulated stack trace line", - ], - }, + message, logFiles: { stdoutFile: "/tmp/pi-processes-demo/proc_42-stdout.log", stderrFile: "/tmp/pi-processes-demo/proc_42-stderr.log", diff --git a/src/tools/actions/output-truncate.ts b/src/tools/actions/output-truncate.ts new file mode 100644 index 0000000..f1dd463 --- /dev/null +++ b/src/tools/actions/output-truncate.ts @@ -0,0 +1,187 @@ +/** + * Local tail-truncation for the `process output` action. + * + * The installed Pi version (0.75.x) does not export its shared truncation + * helper, so this module keeps a local implementation that mirrors the v0.10 + * behavior the agent depends on. + * + * Invariants: + * - Always keeps the newest output within the byte and line budgets. + * - A single oversized line yields a UTF-8-safe suffix instead of being + * dropped entirely. + * - ANSI and terminal control characters must be stripped by callers before + * truncation; this module operates on already-cleaned text. + * + * The `truncation` metadata returned alongside the content matches the shape + * Pi 0.10 exposes through `TruncationResult`, minus the `content` field, so it + * can be persisted in `details` without re-embedding raw output. + */ + +export const MAX_OUTPUT_BYTES = 50 * 1024; + +/** + * Ceiling applied to the JSON-escaped serialized content. Newline-heavy or + * tab-heavy output inflates under `JSON.stringify`, so the composed result is + * measured against this tighter budget in addition to the raw byte limit. + */ +export const MAX_OUTPUT_JSON_BYTES = 96 * 1024; + +export interface TruncationResult { + content: string; + truncated: boolean; + truncatedBy: "bytes" | "lines" | null; + lastLinePartial: boolean; + outputLines: number; + outputBytes: number; + totalLines: number; + totalBytes: number; + maxBytes: number; + maxLines: number; +} + +/** + * Metadata persisted in tool-result `details`. Mirrors `TruncationResult` + * minus the `content` field, so the bounded preview stays in `content` only. + */ +export type TruncationDetails = Omit; + +interface TruncateOptions { + maxBytes: number; + maxLines: number; +} + +/** + * Truncate `text` from the tail, keeping the newest lines within the byte and + * line budgets. A single oversized line is sliced to a UTF-8-safe suffix + * rather than dropped entirely. + */ +export function truncateTail( + text: string, + options: TruncateOptions, +): TruncationResult { + const { maxBytes, maxLines } = options; + const totalBytes = Buffer.byteLength(text, "utf-8"); + const lines = text.split("\n"); + const totalLines = lines.length; + + if (totalLines <= maxLines && totalBytes <= maxBytes) { + return { + content: text, + truncated: false, + truncatedBy: null, + lastLinePartial: false, + outputLines: totalLines, + outputBytes: totalBytes, + totalLines, + totalBytes, + maxBytes, + maxLines, + }; + } + + // Walk backwards, keeping whole lines that fit. Track whether the byte + // budget was the binding constraint so the notice wording is accurate. + const kept: string[] = []; + let keptBytes = 0; + let hitBytes = false; + + for (let i = lines.length - 1; i >= 0 && kept.length < maxLines; i--) { + const line = lines[i] ?? ""; + const lineBytes = + Buffer.byteLength(line, "utf-8") + (kept.length > 0 ? 1 : 0); + + if (keptBytes + lineBytes > maxBytes) { + hitBytes = true; + break; + } + + kept.unshift(line); + keptBytes += lineBytes; + } + + let lastLinePartial = false; + + if (kept.length === 0) { + // The newest line alone exceeds the byte budget. Keep a UTF-8-safe suffix + // instead of returning an empty body, so the agent still sees recent + // output. The single byte budget minus one byte for the skipped joiner. + const lastLine = lines[lines.length - 1] ?? ""; + const suffix = utf8Suffix(lastLine, maxBytes - 1); + kept.push(suffix); + keptBytes = Buffer.byteLength(suffix, "utf-8"); + lastLinePartial = true; + } + + const content = kept.join("\n"); + + return { + content, + truncated: true, + truncatedBy: hitBytes ? "bytes" : "lines", + lastLinePartial, + outputLines: kept.length, + outputBytes: keptBytes, + totalLines, + totalBytes, + maxBytes, + maxLines, + }; +} + +/** + * Slice a UTF-8-safe suffix from `line` at most `maxBytes` long. The suffix + * keeps the newest content (the tail of the line) and is prefixed with an + * ellipsis marker. The cut point walks forward past any leading continuation + * bytes (0b10xxxxxx) so the result never starts mid-code-point. + */ +function utf8Suffix(line: string, maxBytes: number): string { + const buffer = Buffer.from(line, "utf-8"); + if (buffer.length <= maxBytes) return line; + + const marker = "…"; + const markerBytes = Buffer.byteLength(marker, "utf-8"); + const budget = Math.max(0, maxBytes - markerBytes); + if (budget <= 0) return marker; + + // Start the cut so the suffix is the last `budget` bytes of the line. + let start = buffer.length - budget; + // If the cut lands inside a multibyte code point, the first byte is a + // continuation byte (0b10xxxxxx). Walk forward to the next lead byte. + while (start < buffer.length && (buffer[start] ?? 0) >> 6 === 0b10) { + start++; + } + if (start >= buffer.length) return marker; + + return `${marker}${buffer.subarray(start).toString("utf-8")}`; +} + +export function formatSize(bytes: number): string { + if (bytes < 1024) return `${bytes}B`; + if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(1)}KB`; + return `${(bytes / (1024 * 1024)).toFixed(1)}MB`; +} + +/** + * Count logical lines, matching `text.split("\n").length`. Empty text has + * zero lines so the line budget is not consumed by a stray newline. + */ +export function countLines(text: string): number { + if (text.length === 0) return 0; + return text.split("\n").length; +} + +/** + * Compose a truncation notice string matching the agent-facing wording other + * tools use, so renderers can locate it reliably. + */ +export function formatTruncationNotice(truncation: TruncationResult): string { + if (!truncation.truncated) return ""; + const limit = + truncation.truncatedBy === "bytes" + ? `${formatSize(truncation.maxBytes)} byte limit` + : `${truncation.maxLines} line limit`; + const partialNote = truncation.lastLinePartial + ? " (final line is a partial suffix)" + : ""; + return `[Preview truncated by ${limit}${partialNote}; showing ${truncation.outputLines} lines / ${formatSize(truncation.outputBytes)} of ${truncation.totalLines} lines / ${formatSize(truncation.totalBytes)}.]`; +} diff --git a/src/tools/actions/output.test.ts b/src/tools/actions/output.test.ts new file mode 100644 index 0000000..f601550 --- /dev/null +++ b/src/tools/actions/output.test.ts @@ -0,0 +1,392 @@ +import type { Theme } from "@earendil-works/pi-coding-agent"; +import { beforeAll, describe, expect, it, vi } from "vitest"; +import { configLoader } from "../../config"; +import type { ExecuteResult, ProcessInfo } from "../../constants"; +import type { ProcessManager } from "../../manager"; +import { executeOutput, renderOutputResult } from "./output"; +import { MAX_OUTPUT_BYTES, truncateTail } from "./output-truncate"; + +const STDOUT_FILE = "/tmp/proc_1-stdout.log"; +const STDERR_FILE = "/tmp/proc_1-stderr.log"; + +interface OutputSnapshot { + stdout: string[]; + stderr: string[]; + status: string; +} + +function mockProcess(overrides: Partial = {}): ProcessInfo { + return { + id: "proc_1", + name: "server", + pid: 1234, + command: "npm start", + cwd: "/project", + startTime: 1, + endTime: null, + status: "running", + exitCode: null, + success: null, + stdoutFile: STDOUT_FILE, + stderrFile: STDERR_FILE, + alertOnSuccess: false, + alertOnFailure: true, + alertOnKill: false, + ...overrides, + }; +} + +function mockManager( + output: OutputSnapshot | null, + processOverrides: Partial = {}, +): ProcessManager { + const process = mockProcess(processOverrides); + return { + get: vi.fn().mockReturnValue(process), + getOutput: vi.fn().mockReturnValue(output), + getLogFiles: vi.fn().mockReturnValue({ + stdoutFile: STDOUT_FILE, + stderrFile: STDERR_FILE, + combinedFile: "/tmp/proc_1-combined.log", + }), + } as unknown as ProcessManager; +} + +function runOutput(output: OutputSnapshot): ExecuteResult { + return executeOutput({ id: "proc_1" }, mockManager(output)); +} + +beforeAll(async () => { + await configLoader.load(); +}); + +function contentText(result: ExecuteResult): string { + const block = result.content[0]; + return block && block.type === "text" ? block.text : ""; +} + +describe("executeOutput", () => { + it("formats normal stdout and stderr", () => { + const result = runOutput({ + stdout: ["line 1", "line 2"], + stderr: ["err 1"], + status: "running", + }); + + const content = contentText(result); + expect(content).toContain("stdout:"); + expect(content).toContain("line 1"); + expect(content).toContain("line 2"); + expect(content).toContain("stderr:"); + expect(content).toContain("err 1"); + expect(content).toContain("2 stdout lines, 1 stderr lines"); + }); + + it("strips ANSI and terminal control characters before persistence", () => { + const result = runOutput({ + stdout: ["\u001b[31mred\u001b[0m", "step 1\rstep 2\b done"], + stderr: [], + status: "exited", + }); + + const content = contentText(result); + expect(content).not.toContain("\u001b[31m"); + expect(content).not.toContain("\r"); + expect(content).not.toContain("\b"); + expect(content).toContain("red"); + expect(content).toContain("step 1step 2 done"); + }); + + it("never persists stdout, stderr, or output arrays in new result details", () => { + const result = runOutput({ + stdout: ["line 1"], + stderr: ["err 1"], + status: "running", + }); + + const { details } = result; + expect(details.output).toBeUndefined(); + }); + + it("does not embed raw process output in serialized details", () => { + const result = runOutput({ + stdout: ["SECRET-stdout-token"], + stderr: ["SECRET-stderr-token"], + status: "running", + }); + + const serialized = JSON.stringify(result.details); + expect(serialized).not.toContain("SECRET-stdout-token"); + expect(serialized).not.toContain("SECRET-stderr-token"); + // The raw-output property is absent from new result details. + expect(result.details.output).toBeUndefined(); + expect("output" in JSON.parse(serialized)).toBe(false); + }); + + it("bounds a 2 MiB single line without growing the session entry", () => { + const huge = "x".repeat(2 * 1024 * 1024); + const result = runOutput({ stdout: [huge], stderr: [], status: "running" }); + + const serialized = JSON.stringify(result); + // Serialized result (content + details) stays below a fixed safe ceiling. + expect(Buffer.byteLength(serialized, "utf-8")).toBeLessThan(128 * 1024); + // The newest output still surfaces a partial suffix. + expect(contentText(result)).toContain("x"); + expect(result.details.truncation?.truncated).toBe(true); + expect(result.details.truncation?.lastLinePartial).toBe(true); + }); + + it("handles CR-only progress output without session growth", () => { + // CR-only progress collapses into a single line carrying the final state. + const progress = `${Array.from({ length: 1000 }, (_, i) => `phase ${i}\r`).join("")}done`; + const result = runOutput({ + stdout: [progress], + stderr: [], + status: "running", + }); + + const serialized = JSON.stringify(result); + expect(Buffer.byteLength(serialized, "utf-8")).toBeLessThan(128 * 1024); + expect(contentText(result)).not.toContain("\r"); + expect(contentText(result)).toContain("done"); + }); + + it("accounts for JSON-escaping expansion for a tab-heavy line", () => { + // Tabs survive stripAnsi but expand under JSON.stringify. + const tabHeavy = `${"\t".repeat(200_000)}tail-marker`; + const result = runOutput({ + stdout: [tabHeavy], + stderr: [], + status: "running", + }); + + const serialized = JSON.stringify(result); + expect(Buffer.byteLength(serialized, "utf-8")).toBeLessThan(128 * 1024); + expect(contentText(result)).toContain("tail-marker"); + }); + + it("keeps a UTF-8-safe suffix when an oversized multibyte line is truncated", () => { + // Use a 4-byte UTF-8 codepoint (U+1F680) so an arbitrary mid-byte cut + // would produce replacement characters. Keep enough multibyte content so + // the suffix definitely lands inside a code point run. + const emoji = "\u{1F680}".repeat(50_000); // ~200 KiB + const result = runOutput({ + stdout: [emoji], + stderr: [], + status: "running", + }); + + expect(result.details.truncation?.lastLinePartial).toBe(true); + }); + + it("bounds combined large stdout and stderr", () => { + const stdout = Array.from({ length: 5000 }, (_, i) => `out ${i}`); + const stderr = Array.from({ length: 5000 }, (_, i) => `err ${i}`); + const result = runOutput({ stdout, stderr, status: "running" }); + + const serialized = JSON.stringify(result); + expect(Buffer.byteLength(serialized, "utf-8")).toBeLessThan(128 * 1024); + // Newest stderr lines are kept (they form the tail of the combined body). + expect(contentText(result)).toContain("err 4999"); + }); + + it("keeps the final serialized tool result below a fixed safe ceiling", () => { + const stdout = Array.from({ length: 10_000 }, (_, i) => `line ${i}`); + const stderr = Array.from({ length: 10_000 }, (_, i) => `err ${i}`); + const result = runOutput({ stdout, stderr, status: "running" }); + + expect(Buffer.byteLength(JSON.stringify(result), "utf-8")).toBeLessThan( + 128 * 1024, + ); + }); + + it("always retains complete log file paths in the textual result", () => { + const stdout = Array.from({ length: 10_000 }, (_, i) => `line ${i}`); + const result = runOutput({ stdout, stderr: [], status: "running" }); + + const content = contentText(result); + expect(content).toContain(STDOUT_FILE); + expect(content).toContain(STDERR_FILE); + }); + + it("returns failure details without log files when the process is missing", () => { + const manager = { + get: vi.fn().mockReturnValue(null), + getOutput: vi.fn(), + getLogFiles: vi.fn(), + } as unknown as ProcessManager; + + const result = executeOutput({ id: "missing" }, manager); + expect(result.details.success).toBe(false); + expect(result.details.logFiles).toBeUndefined(); + expect(result.details.output).toBeUndefined(); + }); + + it("includes truncation metadata in details when content is truncated", () => { + const result = runOutput({ + stdout: Array.from({ length: 5000 }, (_, i) => `line ${i}`), + stderr: [], + status: "running", + }); + + expect(result.details.truncation).toBeDefined(); + expect(result.details.truncation?.truncated).toBe(true); + expect(result.details.truncation?.totalLines).toBeGreaterThan( + result.details.truncation?.outputLines ?? 0, + ); + // Raw output never leaks through truncation metadata. + expect(JSON.stringify(result.details.truncation)).not.toContain( + "line 4999", + ); + }); +}); + +describe("truncateTail (hardening)", () => { + it("keeps the newest lines within the byte and line budgets", () => { + const text = Array.from({ length: 100 }, (_, i) => `line ${i}`).join("\n"); + const result = truncateTail(text, { + maxBytes: MAX_OUTPUT_BYTES, + maxLines: 10, + }); + + expect(result.truncated).toBe(true); + expect(result.content).toContain("line 99"); + expect(result.content).not.toContain("line 0"); + expect(result.outputLines).toBeLessThanOrEqual(10); + }); + + it("returns a UTF-8-safe suffix from a single oversized line", () => { + const huge = "\u{1F680}".repeat(50_000); + const result = truncateTail(huge, { maxBytes: 1024, maxLines: 10 }); + + expect(result.truncated).toBe(true); + expect(result.lastLinePartial).toBe(true); + expect(result.content.length).toBeGreaterThan(0); + // No partial UTF-8 sequences reach the output. + expect(Buffer.from(result.content, "utf-8").toString("utf-8")).toBe( + result.content, + ); + expect(result.content).not.toContain("\uFFFD"); + }); +}); + +// --- Renderer coverage --- + +function mockTheme(): Theme { + return { + fg: (_color: string, text: string) => text, + bg: (_color: string, text: string) => text, + bold: (text: string) => text, + italic: (text: string) => text, + underline: (text: string) => text, + inverse: (text: string) => text, + strikethrough: (text: string) => text, + getFgAnsi: () => "", + getBgAnsi: () => "", + getColorMode: () => "truecolor", + getThinkingBorderColor: () => (text: string) => text, + getBashModeBorderColor: () => (text: string) => text, + } as unknown as Theme; +} + +function render( + result: ExecuteResult, + options: { expanded?: boolean } = {}, +): string[] { + const body = renderOutputResult( + result as never, + { expanded: options.expanded ?? false } as never, + mockTheme(), + ); + return body.render(120); +} + +describe("renderOutputResult", () => { + it("renders expanded content-based output from tool-result content", () => { + const result = runOutput({ + stdout: ["ready on http://localhost:3000"], + stderr: [], + status: "running", + }); + + const lines = render(result, { expanded: true }); + const joined = lines.join("\n"); + + expect(joined).toContain("ready on http://localhost:3000"); + expect(joined).toContain("Log files:"); + expect(joined).toContain(STDOUT_FILE); + expect(joined).toContain(STDERR_FILE); + }); + + it("renders a collapsed preview from the bounded content", () => { + const result = runOutput({ + stdout: ["first", "second", "third"], + stderr: [], + status: "running", + }); + + const lines = render(result, { expanded: false }); + const joined = lines.join("\n"); + + expect(joined).toContain("third"); + expect(joined).toContain("Output"); + }); + + it("surfaces the truncation notice and log paths in expanded view", () => { + const result = runOutput({ + stdout: Array.from({ length: 5000 }, (_, i) => `line ${i}`), + stderr: [], + status: "running", + }); + + const lines = render(result, { expanded: true }); + const joined = lines.join("\n"); + + expect(result.details.truncation?.truncated).toBe(true); + expect(joined).toContain("Preview truncated"); + expect(joined).toContain(STDOUT_FILE); + expect(joined).toContain(STDERR_FILE); + }); + + it("renders legacy session results that still carry details.output", () => { + const legacy = { + content: [{ type: "text" as const, text: "legacy content" }], + details: { + action: "output" as const, + success: true, + message: '"server" (proc_1) [running]: 1 stdout lines, 0 stderr lines', + output: { + stdout: ["legacy stdout line"], + stderr: [], + status: "running", + }, + logFiles: { + stdoutFile: STDOUT_FILE, + stderrFile: STDERR_FILE, + }, + }, + }; + + const lines = render(legacy, { expanded: true }); + const joined = lines.join("\n"); + + expect(joined).toContain("legacy stdout line"); + expect(joined).toContain("Log files:"); + expect(joined).toContain(STDOUT_FILE); + }); + + it("does not mutate result details during rendering", () => { + const result = runOutput({ + stdout: ["line 1"], + stderr: ["err 1"], + status: "running", + }); + + const before = JSON.stringify(result.details); + render(result, { expanded: true }); + render(result, { expanded: false }); + const after = JSON.stringify(result.details); + + expect(after).toBe(before); + }); +}); diff --git a/src/tools/actions/output.ts b/src/tools/actions/output.ts index 6ac8ae2..98c888d 100644 --- a/src/tools/actions/output.ts +++ b/src/tools/actions/output.ts @@ -9,13 +9,23 @@ import { configLoader } from "../../config"; import type { ExecuteResult, ProcessesDetails } from "../../constants"; import type { ProcessManager } from "../../manager"; import { formatStatus, hasAnsi, stripAnsi } from "../../utils"; - -const MAX_BYTES = 50 * 1024; // 50KB +import { + countLines, + formatTruncationNotice, + MAX_OUTPUT_BYTES, + MAX_OUTPUT_JSON_BYTES, + type TruncationDetails, + type TruncationResult, + truncateTail, +} from "./output-truncate"; interface OutputParams { id?: string; } +/** Marker delimiting the always-present complete-log footer in content. */ +const LOG_FOOTER_MARKER = "[Complete currently-retained logs:"; + export function renderOutputCall( args: OutputParams, theme: Theme, @@ -35,28 +45,83 @@ export function renderOutputResult( options: ToolRenderResultOptions, theme: Theme, ): ToolBody { - const { details } = result; - - if (!details.output) { - return new ToolBody( - { - fields: [ - { - label: "Error", - value: "Missing output details", - showCollapsed: true, - }, - ], - }, - options, - theme, + const { details, content } = result; + + const textBlock = Array.isArray(content) + ? content.find((block) => block.type === "text") + : undefined; + const contentText = + textBlock && textBlock.type === "text" ? textBlock.text : ""; + + // Legacy session results still carry raw stdout/stderr arrays in details. + // Render them so historical entries remain visible without errors. + if (details.output) { + return renderLegacyOutput(details, theme, options); + } + + const bodyLines = extractOutputBody(contentText, details); + let hadAnsi = false; + + const lines: string[] = [theme.fg("muted", details.message)]; + + if (bodyLines.length > 0) { + lines.push(""); + for (const line of bodyLines) { + if (!hadAnsi && hasAnsi(line)) hadAnsi = true; + lines.push(line); + } + } else { + lines.push("", theme.fg("muted", "(no output)")); + } + + if (details.truncation) { + lines.push( + "", + theme.fg("muted", buildTruncationSummary(details.truncation)), + ); + } + + if (details.logFiles) { + lines.push( + "", + theme.fg("success", "Log files:"), + ` stdout: ${theme.fg("accent", details.logFiles.stdoutFile)}`, + ` stderr: ${theme.fg("accent", details.logFiles.stderrFile)}`, + ); + } + + if (hadAnsi) { + lines.push( + "", + theme.fg("muted", "ANSI escape codes were stripped from output"), ); } + const fields: Array< + { label: string; value: string; showCollapsed?: boolean } | Text + > = [new Text(lines.join("\n"), 0, 0)]; + + // Collapsed preview: the last couple of body lines. + const preview = + bodyLines.slice(-2).join("\n") || theme.fg("muted", "(empty)"); + fields.push({ + label: "Output", + value: theme.fg("muted", preview), + showCollapsed: true, + }); + + return new ToolBody({ fields }, options, theme); +} + +function renderLegacyOutput( + details: ProcessesDetails, + theme: Theme, + options: ToolRenderResultOptions, +): ToolBody { const lines: string[] = [theme.fg("muted", details.message)]; let hadAnsi = false; - if (details.output.stdout.length > 0) { + if (details.output?.stdout.length) { lines.push("", theme.fg("accent", "stdout:")); for (const line of details.output.stdout.slice(-20)) { if (!hadAnsi && hasAnsi(line)) hadAnsi = true; @@ -72,7 +137,7 @@ export function renderOutputResult( } } - if (details.output.stderr.length > 0) { + if (details.output?.stderr.length) { lines.push("", theme.fg("warning", "stderr:")); for (const line of details.output.stderr.slice(-10)) { if (!hadAnsi && hasAnsi(line)) hadAnsi = true; @@ -104,30 +169,102 @@ export function renderOutputResult( ); } - const fields: Array< - { label: string; value: string; showCollapsed?: boolean } | Text - > = [new Text(lines.join("\n"), 0, 0)]; - - // Collapsed summary - const previewSource = - details.output.stdout.length > 0 - ? details.output.stdout - : details.output.stderr; + const previewSource = details.output?.stdout.length + ? details.output.stdout + : (details.output?.stderr ?? []); const preview = previewSource .slice(-2) .map((l) => stripAnsi(l)) .join("\n"); - fields.push({ - label: "Output", - value: preview - ? `${theme.fg("muted", preview)}` - : theme.fg("muted", "(empty)"), - showCollapsed: true, - }); + + const fields: Array< + { label: string; value: string; showCollapsed?: boolean } | Text + > = [ + new Text(lines.join("\n"), 0, 0), + { + label: "Output", + value: preview + ? theme.fg("muted", preview) + : theme.fg("muted", "(empty)"), + showCollapsed: true, + }, + ]; return new ToolBody({ fields }, options, theme); } +/** + * Extract the bounded process-output body from tool-result content text. + * + * Content is structured as: + * - a one-line header (`details.message`); + * - the bounded output body (already ANSI-stripped); + * - an optional truncation notice line; + * - an always-present complete-log footer. + * + * Only the body is returned. The renderer uses `details` for metadata, + * truncation state, and log paths; this function never re-parses stream + * labels, so a real log line containing `stderr:` cannot confuse it. + * + * Legacy content that lost its header through tail truncation is accepted: if + * the first line does not match the expected header, the whole content is + * treated as body up to the footer. + */ +function extractOutputBody( + contentText: string, + details: ProcessesDetails, +): string[] { + if (!contentText) return []; + + const lines = contentText.split("\n"); + const header = details.message; + + let bodyStart = lines[0] === header ? 1 : 0; + // Skip the blank separator following the header. + if (bodyStart > 0 && lines[bodyStart] === "") { + bodyStart++; + } + + const footerStart = findLastLineIndex( + lines, + (line) => line === LOG_FOOTER_MARKER, + ); + let bodyEnd = footerStart >= 0 ? footerStart : lines.length; + + // Exclude the running-guidance line, which sits between the body and the + // footer/notice. It is metadata, not process output. + const guidance = "Process is still running. Use watches instead of polling."; + const guidanceIndex = findLastLineIndex( + lines, + (line, index) => index < bodyEnd && line === guidance, + ); + if (guidanceIndex >= bodyStart) { + bodyEnd = guidanceIndex; + } + + // Exclude a preceding blank line that separated body from the footer/notice. + while (bodyEnd > bodyStart && (lines[bodyEnd - 1] ?? "") === "") { + bodyEnd--; + } + + return lines.slice(bodyStart, bodyEnd); +} + +function buildTruncationSummary(truncation: TruncationDetails): string { + const partialNote = truncation.lastLinePartial ? " · partial final line" : ""; + return `Preview truncated · ${truncation.outputLines}/${truncation.totalLines} lines${partialNote}`; +} + +function findLastLineIndex( + lines: string[], + predicate: (line: string, index: number) => boolean, +): number { + for (let i = lines.length - 1; i >= 0; i--) { + if (predicate(lines[i] ?? "", i)) return i; + } + return -1; +} + export function executeOutput( params: OutputParams, manager: ProcessManager, @@ -156,7 +293,7 @@ export function executeOutput( }; } - const { defaultTailLines } = configLoader.getConfig().output; + const { defaultTailLines, maxOutputLines } = configLoader.getConfig().output; const output = manager.getOutput(proc.id, defaultTailLines); if (!output) { const message = `Could not read output for "${proc.name}" (${proc.id})`; @@ -175,95 +312,136 @@ export function executeOutput( const stderrLines = output.stderr.length; const message = `"${proc.name}" (${proc.id}) [${formatStatus(proc)}]: ${stdoutLines} stdout lines, ${stderrLines} stderr lines`; - // Build the full text content (ANSI-stripped), then truncate from the tail - // like bash does, so the agent sees the most recent output. - const outputParts: string[] = [message]; + // Build the stripped body text. stdout/stderr stay local and are never + // persisted in `details`; only the bounded preview survives in `content`. + const bodyLines: string[] = []; if (output.stdout.length > 0) { - outputParts.push("\nstdout:"); - outputParts.push(...output.stdout.map(stripAnsi)); + bodyLines.push("stdout:"); + bodyLines.push(...output.stdout.map(stripAnsi)); } if (output.stderr.length > 0) { - outputParts.push("\nstderr:"); - outputParts.push(...output.stderr.map(stripAnsi)); + if (bodyLines.length > 0) bodyLines.push(""); + bodyLines.push("stderr:"); + bodyLines.push(...output.stderr.map(stripAnsi)); } - const fullText = outputParts.join("\n"); - const { maxOutputLines } = configLoader.getConfig().output; - const contentText = truncateTail(fullText, logFiles, maxOutputLines); + const guidance = + output.status === "running" + ? "Process is still running. Use watches instead of polling." + : null; + + const { contentText, truncation } = buildBoundedOutput( + message, + bodyLines.join("\n"), + guidance, + logFiles, + maxOutputLines, + ); + + const details: ProcessesDetails = { + action: "output", + success: true, + message, + logFiles: logFiles + ? { + stdoutFile: logFiles.stdoutFile, + stderrFile: logFiles.stderrFile, + } + : undefined, + }; + + if (truncation.truncated) { + const { content: _content, ...rest } = truncation; + details.truncation = rest; + } return { content: [{ type: "text", text: contentText }], - details: { - action: "output", - success: true, - message, - output, - logFiles: logFiles - ? { - stdoutFile: logFiles.stdoutFile, - stderrFile: logFiles.stderrFile, - } - : undefined, - }, + details, }; } /** - * Truncate text from the tail (keep last N lines / MAX_BYTES), matching - * the behaviour of pi's built-in bash tool. When truncated, appends a - * notice pointing the agent to the full log files. + * Compose the bounded content text. The header, optional guidance, truncation + * notice, and complete-log footer live outside the truncation window, so the + * agent always receives log paths even when the body is truncated. + * + * The byte and line budgets apply to the composed+JSON-escaped result. If the + * first pass overflows (because JSON escaping expands control characters or + * the fixed metadata consumes the budget), the body budget is shrunk and the + * body re-truncated while keeping the newest output. */ -function truncateTail( - text: string, +function buildBoundedOutput( + header: string, + body: string, + guidance: string | null, logFiles: { stdoutFile: string; stderrFile: string } | null, maxLines: number, -): string { - const totalBytes = Buffer.byteLength(text, "utf-8"); - const lines = text.split("\n"); - const totalLines = lines.length; - - if (totalLines <= maxLines && totalBytes <= MAX_BYTES) { - return text; - } - - // Work backwards, collecting lines that fit - const kept: string[] = []; - let keptBytes = 0; - let hitBytes = false; +): { contentText: string; truncation: TruncationResult } { + let maxBodyBytes = MAX_OUTPUT_BYTES; + let maxBodyLines = maxLines; - for (let i = lines.length - 1; i >= 0 && kept.length < maxLines; i--) { - const line = lines[i] ?? ""; - const lineBytes = - Buffer.byteLength(line, "utf-8") + (kept.length > 0 ? 1 : 0); + let truncation = truncateTail(body, { + maxBytes: maxBodyBytes, + maxLines: maxBodyLines, + }); + let contentText = composeContent(header, truncation, guidance, logFiles); + + for (let attempt = 0; attempt < 10; attempt++) { + const excessBytes = + Buffer.byteLength(contentText, "utf-8") - MAX_OUTPUT_BYTES; + const excessLines = countLines(contentText) - maxLines; + const excessJsonBytes = + Buffer.byteLength(JSON.stringify(contentText), "utf-8") - + MAX_OUTPUT_JSON_BYTES; + + if (excessBytes <= 0 && excessLines <= 0 && excessJsonBytes <= 0) { + return { contentText, truncation }; + } - if (keptBytes + lineBytes > MAX_BYTES) { - hitBytes = true; - break; + maxBodyBytes = Math.max(0, maxBodyBytes - Math.max(0, excessBytes)); + if (excessJsonBytes > 0) { + maxBodyBytes = Math.floor(maxBodyBytes / 2); } + maxBodyLines = Math.max(1, maxBodyLines - Math.max(0, excessLines)); - kept.unshift(line); - keptBytes += lineBytes; + truncation = truncateTail(body, { + maxBytes: maxBodyBytes, + maxLines: maxBodyLines, + }); + contentText = composeContent(header, truncation, guidance, logFiles); } - let result = kept.join("\n"); + // Pathological metadata can still consume the whole budget. Bound the final + // composed string so a session entry cannot grow unbounded. + const finalTruncation = truncateTail(contentText, { + maxBytes: MAX_OUTPUT_BYTES, + maxLines: maxLines, + }); + return { contentText: finalTruncation.content, truncation }; +} - // Append a notice so the agent knows output was truncated - const shownLines = kept.length; - const startLine = totalLines - shownLines + 1; - const sizeNote = hitBytes ? ` (${formatSize(MAX_BYTES)} limit)` : ""; - result += `\n\n[Showing lines ${startLine}-${totalLines} of ${totalLines}${sizeNote}.`; +function composeContent( + header: string, + truncation: TruncationResult, + guidance: string | null, + logFiles: { stdoutFile: string; stderrFile: string } | null, +): string { + const sections: string[] = [header, truncation.content]; - if (logFiles) { - result += ` Full logs: ${logFiles.stdoutFile} , ${logFiles.stderrFile}`; + if (guidance) { + sections.push(guidance); } - result += "]"; + if (truncation.truncated) { + sections.push(formatTruncationNotice(truncation)); + } - return result; -} + if (logFiles) { + sections.push( + `${LOG_FOOTER_MARKER}\nstdout=${logFiles.stdoutFile}\nstderr=${logFiles.stderrFile}]`, + ); + } -function formatSize(bytes: number): string { - if (bytes < 1024) return `${bytes}B`; - if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(1)}KB`; - return `${(bytes / (1024 * 1024)).toFixed(1)}MB`; + return sections.filter((section) => section.length > 0).join("\n\n"); } -- 2.51.2