fix(cli,core,web): stop ANSI color leakage into tool output (#187)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Yaowei Zheng
+103
-50
@@ -38,7 +38,9 @@
|
||||
* not rendered — the child Agent's final text is already streamed through the parent
|
||||
* tool's output gutter.
|
||||
*
|
||||
* No third-party color library is used; only minimal ANSI escapes.
|
||||
* No third-party color library is used; only minimal ANSI escapes — and they are emitted at
|
||||
* all only when the output stream supports color (see `supportsColor`): piped output, e.g. a
|
||||
* nested `penguin run` driven through `exec_command`, must stay plain (#102).
|
||||
*/
|
||||
import { isEventMessage, isModelMessage, parseUserSteeringText } from "@prismshadow/penguin-core";
|
||||
import type {
|
||||
@@ -64,31 +66,71 @@ import { renderFileToolApprovalPayload, renderPartialToolCall } from "./tool-ren
|
||||
import { defaultMessages } from "./i18n.js";
|
||||
import type { Messages } from "./i18n.js";
|
||||
|
||||
const DIM = "\x1b[2m";
|
||||
const GREEN = "\x1b[32m";
|
||||
const RED = "\x1b[31m";
|
||||
const CYAN = "\x1b[36m";
|
||||
const MAGENTA = "\x1b[35m";
|
||||
const RESET = "\x1b[0m";
|
||||
/** The ANSI codes the renderer uses; the plain palette maps every code to "" so all writes degrade to uncolored text with no further branching. */
|
||||
export interface Palette {
|
||||
readonly dim: string;
|
||||
readonly green: string;
|
||||
readonly red: string;
|
||||
readonly cyan: string;
|
||||
readonly magenta: string;
|
||||
readonly reset: string;
|
||||
}
|
||||
|
||||
export function dim(text: string): string {
|
||||
return `${DIM}${text}${RESET}`;
|
||||
const ANSI_PALETTE: Palette = {
|
||||
dim: "\x1b[2m",
|
||||
green: "\x1b[32m",
|
||||
red: "\x1b[31m",
|
||||
cyan: "\x1b[36m",
|
||||
magenta: "\x1b[35m",
|
||||
reset: "\x1b[0m",
|
||||
};
|
||||
|
||||
const PLAIN_PALETTE: Palette = { dim: "", green: "", red: "", cyan: "", magenta: "", reset: "" };
|
||||
|
||||
/**
|
||||
* Whether ANSI color should be emitted on `stream`: requires a TTY with `NO_COLOR`
|
||||
* unset/empty and `TERM` other than `dumb`; a non-empty `FORCE_COLOR` overrides all of that —
|
||||
* `"0"` forces plain, anything else forces color (Node's own semantics, where `FORCE_COLOR`
|
||||
* defeats `NO_COLOR`). Keeps a piped nested CLI — `penguin run` driven through
|
||||
* `exec_command` — from leaking escapes into tool output (#102).
|
||||
*/
|
||||
export function supportsColor(
|
||||
stream: { isTTY?: boolean | undefined },
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
): boolean {
|
||||
const force = env.FORCE_COLOR;
|
||||
if (force !== undefined && force !== "") return force !== "0";
|
||||
if (env.NO_COLOR !== undefined && env.NO_COLOR !== "") return false;
|
||||
if (env.TERM === "dumb") return false;
|
||||
return stream.isTTY === true;
|
||||
}
|
||||
|
||||
/** The palette for one output stream, decided once per renderer rather than per write. */
|
||||
function paletteFor(stream: NodeJS.WritableStream): Palette {
|
||||
return supportsColor(stream as { isTTY?: boolean }) ? ANSI_PALETTE : PLAIN_PALETTE;
|
||||
}
|
||||
|
||||
/** stdout's palette, decided at startup: the default for the standalone helpers below (hosts write their own notices to stdout). */
|
||||
const STDOUT_PALETTE = paletteFor(process.stdout);
|
||||
|
||||
export function dim(text: string, c: Palette = STDOUT_PALETTE): string {
|
||||
return `${c.dim}${text}${c.reset}`;
|
||||
}
|
||||
|
||||
/** The two file tools whose outputs carry git-style diffs; their `+`/`-`/`@@` lines get colored. */
|
||||
const DIFF_OUTPUT_TOOLS = new Set(["edit_file", "write_file"]);
|
||||
|
||||
/** Color for one diff-output line, picked from its first character (null = plain). */
|
||||
function diffLineColor(firstChar: string | undefined): string | null {
|
||||
if (firstChar === "+") return GREEN;
|
||||
if (firstChar === "-") return RED;
|
||||
if (firstChar === "@") return DIM;
|
||||
/** Color for one diff-output line, picked from its first character (null = plain; a colorless palette yields "", equally falsy for callers). */
|
||||
function diffLineColor(firstChar: string | undefined, c: Palette): string | null {
|
||||
if (firstChar === "+") return c.green;
|
||||
if (firstChar === "-") return c.red;
|
||||
if (firstChar === "@") return c.dim;
|
||||
return null;
|
||||
}
|
||||
|
||||
/** Colors a tool call line cyan, distinguishing it from body text/thinking (review comment #5). */
|
||||
function cyan(text: string): string {
|
||||
return `${CYAN}${text}${RESET}`;
|
||||
function cyan(text: string, c: Palette): string {
|
||||
return `${c.cyan}${text}${c.reset}`;
|
||||
}
|
||||
|
||||
/** Takes the last 3 characters of an id as the on-screen pairing number. */
|
||||
@@ -140,8 +182,8 @@ function humanizeDuration(ms: number): string {
|
||||
return `${Math.floor(whole / 60)}m${whole % 60}s`;
|
||||
}
|
||||
|
||||
export function formatAbort(p: AbortPayload, t: Messages): string {
|
||||
return dim(t.abortLabel(p.reason ?? undefined));
|
||||
export function formatAbort(p: AbortPayload, t: Messages, c: Palette = STDOUT_PALETTE): string {
|
||||
return dim(t.abortLabel(p.reason ?? undefined), c);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -165,6 +207,7 @@ export function renderHistory(
|
||||
out: NodeJS.WritableStream,
|
||||
t: Messages = defaultMessages(),
|
||||
): void {
|
||||
const c = paletteFor(out);
|
||||
// tool_call_id -> tool name (keyed with the origin chain: parent/child ids may collide),
|
||||
// so output lines can be labeled with the tool name of their preceding call.
|
||||
const toolNames = new Map<string, string>();
|
||||
@@ -172,7 +215,7 @@ export function renderHistory(
|
||||
for (const msg of messages) {
|
||||
if (isEventMessage(msg)) {
|
||||
const p = msg.payload as { type?: string } & AbortPayload;
|
||||
if (p.type === "abort") out.write(`${formatAbort(p, t)}\n`);
|
||||
if (p.type === "abort") out.write(`${formatAbort(p, t, c)}\n`);
|
||||
continue;
|
||||
}
|
||||
if (!isModelMessage(msg)) continue;
|
||||
@@ -188,7 +231,8 @@ export function renderHistory(
|
||||
tool_call_id?: string;
|
||||
stop_reason?: string;
|
||||
};
|
||||
const marker = p.stop_reason && p.stop_reason !== "completed" ? dim(` [${p.stop_reason}]`) : "";
|
||||
const marker =
|
||||
p.stop_reason && p.stop_reason !== "completed" ? dim(` [${p.stop_reason}]`, c) : "";
|
||||
switch (p.type) {
|
||||
case "text":
|
||||
if (p.role === "user") {
|
||||
@@ -196,7 +240,7 @@ export function renderHistory(
|
||||
// rendered as distinct user-speech lines instead of a raw marker block or a prompt.
|
||||
const steering = parseUserSteeringText(p.text ?? "");
|
||||
if (steering !== null) {
|
||||
writeSteeringLines(out, steering, t);
|
||||
writeSteeringLines(out, steering, t, c);
|
||||
} else {
|
||||
out.write(`\n> ${p.text ?? ""}\n`);
|
||||
}
|
||||
@@ -205,17 +249,17 @@ export function renderHistory(
|
||||
}
|
||||
break;
|
||||
case "image_url":
|
||||
out.write(`\n> ${dim("[image]")}\n`);
|
||||
out.write(`\n> ${dim("[image]", c)}\n`);
|
||||
break;
|
||||
case "thinking":
|
||||
out.write(`${dim(p.thinking ?? "")}${marker}\n`);
|
||||
out.write(`${dim(p.thinking ?? "", c)}${marker}\n`);
|
||||
break;
|
||||
case "tool_call": {
|
||||
if (p.name) toolNames.set(nameKey(msg, p.tool_call_id ?? ""), p.name);
|
||||
const preview =
|
||||
renderPartialToolCall(p.name ?? "", p.arguments ?? "", { final: true }) ??
|
||||
`${p.name} ${p.arguments}`;
|
||||
out.write(`${cyan(`[${callTag(p.tool_call_id ?? "")}] ${preview}`)}${marker}\n`);
|
||||
out.write(`${cyan(`[${callTag(p.tool_call_id ?? "")}] ${preview}`, c)}${marker}\n`);
|
||||
break;
|
||||
}
|
||||
case "tool_call_output": {
|
||||
@@ -226,16 +270,16 @@ export function renderHistory(
|
||||
const label = name ? `${tag} ${name}` : tag;
|
||||
const colorDiff = name !== undefined && DIFF_OUTPUT_TOOLS.has(name);
|
||||
for (const line of (p.output ?? "").split("\n")) {
|
||||
const color = colorDiff ? diffLineColor(line[0]) : null;
|
||||
const color = colorDiff ? diffLineColor(line[0], c) : null;
|
||||
out.write(
|
||||
color
|
||||
? `${DIM}${label} -> ${RESET}${color}${line}${RESET}\n`
|
||||
: `${DIM}${label} -> ${RESET}${line}\n`,
|
||||
? `${c.dim}${label} -> ${c.reset}${color}${line}${c.reset}\n`
|
||||
: `${c.dim}${label} -> ${c.reset}${line}\n`,
|
||||
);
|
||||
}
|
||||
// Attached images aren't rendered by the terminal; print one placeholder line per image.
|
||||
for (const _ of p.images ?? []) {
|
||||
out.write(`${DIM}${label} -> [image]${RESET}\n`);
|
||||
out.write(`${c.dim}${label} -> [image]${c.reset}\n`);
|
||||
}
|
||||
break;
|
||||
}
|
||||
@@ -246,9 +290,14 @@ export function renderHistory(
|
||||
}
|
||||
|
||||
/** Writes a steering message's lines with the colored user prefix (shared by history rendering and the streaming renderer). */
|
||||
function writeSteeringLines(out: NodeJS.WritableStream, text: string, t: Messages): void {
|
||||
function writeSteeringLines(
|
||||
out: NodeJS.WritableStream,
|
||||
text: string,
|
||||
t: Messages,
|
||||
c: Palette,
|
||||
): void {
|
||||
for (const line of text.split("\n")) {
|
||||
out.write(`${MAGENTA}${t.steerLinePrefix()}${line}${RESET}\n`);
|
||||
out.write(`${c.magenta}${t.steerLinePrefix()}${line}${c.reset}\n`);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -260,6 +309,8 @@ function writeSteeringLines(out: NodeJS.WritableStream, text: string, t: Message
|
||||
export class StreamRenderer {
|
||||
private readonly out: NodeJS.WritableStream;
|
||||
private readonly t: Messages;
|
||||
/** This renderer's palette, decided once at construction from the output stream and NO_COLOR/FORCE_COLOR/TERM (see supportsColor). */
|
||||
private readonly c: Palette;
|
||||
|
||||
/** Pending render queue: while the screen is held (a streaming segment is in progress / awaiting user input), messages queue up here. */
|
||||
private pending: OmniMessage[] = [];
|
||||
@@ -386,6 +437,7 @@ export class StreamRenderer {
|
||||
constructor(out: NodeJS.WritableStream = process.stdout, t: Messages = defaultMessages()) {
|
||||
this.out = out;
|
||||
this.t = t;
|
||||
this.c = paletteFor(out);
|
||||
}
|
||||
|
||||
handle(msg: OmniMessage): void {
|
||||
@@ -414,7 +466,7 @@ export class StreamRenderer {
|
||||
toolCall.payload.arguments,
|
||||
);
|
||||
if (payload !== null) {
|
||||
for (const line of payload.split("\n")) this.out.write(`${dim(line)}\n`);
|
||||
for (const line of payload.split("\n")) this.out.write(`${dim(line, this.c)}\n`);
|
||||
// lastLineKey stays on the call's key: the payload lines belong to this call, so
|
||||
// the later noteApprovalDecision must not re-render the call line as "not adjacent".
|
||||
}
|
||||
@@ -449,7 +501,7 @@ export class StreamRenderer {
|
||||
this.renderedDecisions.add(key);
|
||||
this.ensureAdjacentCallLine(toolCall);
|
||||
this.finishLine();
|
||||
this.out.write(`${dim(this.t.approvalDecision(decision))}\n`);
|
||||
this.out.write(`${dim(this.t.approvalDecision(decision), this.c)}\n`);
|
||||
this.lastLineKey = null;
|
||||
}
|
||||
|
||||
@@ -491,7 +543,7 @@ export class StreamRenderer {
|
||||
const preview =
|
||||
renderPartialToolCall(p.name, p.arguments, { final: true }) ?? `${p.name} ${p.arguments}`;
|
||||
this.finishLine();
|
||||
this.out.write(`${cyan(`[${callTag(p.tool_call_id, origin)}] ${preview}`)}\n`);
|
||||
this.out.write(`${cyan(`[${callTag(p.tool_call_id, origin)}] ${preview}`, this.c)}\n`);
|
||||
this.lastLineKey = key;
|
||||
}
|
||||
|
||||
@@ -645,7 +697,7 @@ export class StreamRenderer {
|
||||
const steering = parseUserSteeringText(p.text);
|
||||
if (steering !== null) {
|
||||
this.finishLine();
|
||||
writeSteeringLines(this.out, steering, this.t);
|
||||
writeSteeringLines(this.out, steering, this.t, this.c);
|
||||
this.lastLineKey = null;
|
||||
}
|
||||
}
|
||||
@@ -683,14 +735,14 @@ export class StreamRenderer {
|
||||
const p = payload as ApprovalDecisionPayload;
|
||||
if (this.renderedDecisions.delete(this.callLineKey(p.tool_call_id))) return;
|
||||
this.finishLine();
|
||||
this.out.write(`${dim(this.t.approvalDecision(p.decision))}\n`);
|
||||
this.out.write(`${dim(this.t.approvalDecision(p.decision), this.c)}\n`);
|
||||
this.lastLineKey = null;
|
||||
} else if (payload.type === "abort") {
|
||||
// Run ended (user interrupt / retries exhausted): clear any pending retry state so the next run doesn't mistakenly print a retry line.
|
||||
this.pendingRetry = null;
|
||||
this.pendingRetryAttempt = undefined;
|
||||
this.finishLine();
|
||||
this.out.write(`${formatAbort(payload as AbortPayload, this.t)}\n`);
|
||||
this.out.write(`${formatAbort(payload as AbortPayload, this.t, this.c)}\n`);
|
||||
this.lastLineKey = null;
|
||||
} else if (payload.type === "request_begin") {
|
||||
// The previous request ended in a retryable status -> this request is a retry
|
||||
@@ -703,7 +755,7 @@ export class StreamRenderer {
|
||||
const attempt = this.pendingRetryAttempt ?? 1;
|
||||
this.pendingRetryAttempt = undefined;
|
||||
this.finishLine();
|
||||
this.out.write(`${dim(this.t.reconnectLabel(this.pendingRetry, attempt))}\n`);
|
||||
this.out.write(`${dim(this.t.reconnectLabel(this.pendingRetry, attempt), this.c)}\n`);
|
||||
this.lastLineKey = null;
|
||||
this.pendingRetry = null;
|
||||
}
|
||||
@@ -739,7 +791,7 @@ export class StreamRenderer {
|
||||
this.finishLine();
|
||||
this.compactionActive = true;
|
||||
this.compactionTokens = 0;
|
||||
this.out.write(`${dim(this.t.compactionStart(p.mode, p.reason))}\n`);
|
||||
this.out.write(`${dim(this.t.compactionStart(p.mode, p.reason), this.c)}\n`);
|
||||
this.lastLineKey = null;
|
||||
} else if (payload.type === "compaction_end") {
|
||||
// end signals the result and shows the tokens consumed by the compaction request (if any).
|
||||
@@ -756,7 +808,7 @@ export class StreamRenderer {
|
||||
: undefined;
|
||||
this.compactionTokens = 0;
|
||||
this.out.write(
|
||||
`${dim(this.t.compactionStop(p.mode, p.status, tokens, p.error_message))}\n`,
|
||||
`${dim(this.t.compactionStop(p.mode, p.status, tokens, p.error_message), this.c)}\n`,
|
||||
);
|
||||
this.lastLineKey = null;
|
||||
}
|
||||
@@ -813,7 +865,7 @@ export class StreamRenderer {
|
||||
return;
|
||||
}
|
||||
this.finishLine();
|
||||
this.out.write(`${dim(this.t.approvalDecision(p.decision))}\n`);
|
||||
this.out.write(`${dim(this.t.approvalDecision(p.decision), this.c)}\n`);
|
||||
this.lastLineKey = null;
|
||||
} else if (msg.payload.type === "token_usage") {
|
||||
// Child-session usage counts toward this task's Token delta and the Session total (parent and child use the same accounting); context still follows parent-session accounting.
|
||||
@@ -845,7 +897,7 @@ export class StreamRenderer {
|
||||
return;
|
||||
}
|
||||
if (!this.inDim) {
|
||||
this.out.write(DIM);
|
||||
this.out.write(this.c.dim);
|
||||
this.inDim = true;
|
||||
}
|
||||
if (p.thinking) {
|
||||
@@ -870,7 +922,7 @@ export class StreamRenderer {
|
||||
const preview = renderPartialToolCall(partial.name, partial.arguments, { final: true });
|
||||
if (preview === null) return;
|
||||
this.finishLine();
|
||||
this.out.write(`${cyan(`[${callTag(toolCallId)}] ${preview}`)}\n`);
|
||||
this.out.write(`${cyan(`[${callTag(toolCallId)}] ${preview}`, this.c)}\n`);
|
||||
this.lastLineKey = key;
|
||||
}
|
||||
|
||||
@@ -915,14 +967,14 @@ export class StreamRenderer {
|
||||
if (this.partialToolCallLineId !== p.tool_call_id) {
|
||||
this.finishLine();
|
||||
this.partialToolCallLineId = p.tool_call_id;
|
||||
this.out.write(cyan(`[${callTag(p.tool_call_id)}] ${preview}`));
|
||||
this.out.write(cyan(`[${callTag(p.tool_call_id)}] ${preview}`, this.c));
|
||||
} else if (preview.startsWith(partial.lastPreview)) {
|
||||
this.out.write(cyan(preview.slice(partial.lastPreview.length)));
|
||||
this.out.write(cyan(preview.slice(partial.lastPreview.length), this.c));
|
||||
} else {
|
||||
// The preview usually grows monotonically with the arguments; if escaping/folding makes it non-appendable, start a new line with the current readable state.
|
||||
this.finishLine();
|
||||
this.partialToolCallLineId = p.tool_call_id;
|
||||
this.out.write(cyan(`[${callTag(p.tool_call_id)}] ${preview}`));
|
||||
this.out.write(cyan(`[${callTag(p.tool_call_id)}] ${preview}`, this.c));
|
||||
}
|
||||
partial.lastPreview = preview;
|
||||
this.inLine = true;
|
||||
@@ -945,7 +997,7 @@ export class StreamRenderer {
|
||||
if (p.images && p.images.length > 0) {
|
||||
this.finishLine();
|
||||
for (const _ of p.images) {
|
||||
this.out.write(`${DIM}${label} -> [image]${RESET}\n`);
|
||||
this.out.write(`${this.c.dim}${label} -> [image]${this.c.reset}\n`);
|
||||
}
|
||||
this.lastLineKey = null;
|
||||
}
|
||||
@@ -970,19 +1022,19 @@ export class StreamRenderer {
|
||||
while (i < chunk.length) {
|
||||
let lineColor: string | null = null;
|
||||
if (this.toolOutLineStart) {
|
||||
this.out.write(`${DIM}${label} -> ${RESET}`);
|
||||
this.out.write(`${this.c.dim}${label} -> ${this.c.reset}`);
|
||||
this.toolOutLineStart = false;
|
||||
this.inLine = true;
|
||||
// Diff coloring keys off the line's first character. File-tool outputs arrive as
|
||||
// one delta of whole lines, so the first character is always in this chunk; a
|
||||
// line continued from a previous chunk stays plain.
|
||||
if (colorDiff) lineColor = diffLineColor(chunk[i]);
|
||||
if (colorDiff) lineColor = diffLineColor(chunk[i], this.c);
|
||||
}
|
||||
const nl = chunk.indexOf("\n", i);
|
||||
const end = nl === -1 ? chunk.length : nl;
|
||||
const segment = chunk.slice(i, end);
|
||||
if (segment) {
|
||||
this.out.write(lineColor ? `${lineColor}${segment}${RESET}` : segment);
|
||||
this.out.write(lineColor ? `${lineColor}${segment}${this.c.reset}` : segment);
|
||||
}
|
||||
if (nl === -1) {
|
||||
i = chunk.length;
|
||||
@@ -1038,6 +1090,7 @@ export class StreamRenderer {
|
||||
elapsed: humanizeDuration(this.sessionElapsedMs),
|
||||
elapsedDelta: signedDelta(humanizeDuration(elapsed)),
|
||||
}),
|
||||
this.c,
|
||||
)}\n`,
|
||||
);
|
||||
this.contextAtTaskStart = this.contextNow;
|
||||
@@ -1081,7 +1134,7 @@ export class StreamRenderer {
|
||||
|
||||
private closeDim(): void {
|
||||
if (this.inDim) {
|
||||
this.out.write(RESET);
|
||||
this.out.write(this.c.reset);
|
||||
this.inDim = false;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { Writable } from "node:stream";
|
||||
import {
|
||||
approvalDecision,
|
||||
@@ -21,11 +21,23 @@ import {
|
||||
withOrigin,
|
||||
} from "@prismshadow/penguin-core";
|
||||
import type { MessageOrigin } from "@prismshadow/penguin-core";
|
||||
import { StreamRenderer, formatAbort, humanizeTokens, renderHistory } from "../src/render.js";
|
||||
import {
|
||||
StreamRenderer,
|
||||
formatAbort,
|
||||
humanizeTokens,
|
||||
renderHistory,
|
||||
supportsColor,
|
||||
} from "../src/render.js";
|
||||
import { getMessages } from "../src/i18n.js";
|
||||
|
||||
const t = getMessages("en");
|
||||
|
||||
// Several tests stub FORCE_COLOR/NO_COLOR to pin the palette decision (the collector streams
|
||||
// below are not TTYs); make sure no stub leaks into the next test.
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
function collector(): { stream: Writable; text: () => string } {
|
||||
let buf = "";
|
||||
const stream = new Writable({
|
||||
@@ -226,6 +238,9 @@ describe("StreamRenderer", () => {
|
||||
});
|
||||
|
||||
it("colors edit_file diff output lines green/red and dims hunk headers", () => {
|
||||
// The collector stream is not a TTY, so color must be forced on for this test (and the
|
||||
// renderer captures the decision at construction, so the stub must precede it).
|
||||
vi.stubEnv("FORCE_COLOR", "1");
|
||||
const { stream, text } = collector();
|
||||
const r = new StreamRenderer(stream, t);
|
||||
r.handle(partialToolCall({ eventType: "start", name: "edit_file", toolCallId: "d1" }));
|
||||
@@ -259,6 +274,8 @@ describe("StreamRenderer", () => {
|
||||
});
|
||||
|
||||
it("does not diff-color non-file-tool output", () => {
|
||||
// Forced color, as above: with a colorless palette this assertion would pass vacuously.
|
||||
vi.stubEnv("FORCE_COLOR", "1");
|
||||
const { stream, text } = collector();
|
||||
const r = new StreamRenderer(stream, t);
|
||||
r.handle(partialToolCall({ eventType: "start", name: "exec_command", toolCallId: "d2" }));
|
||||
@@ -985,3 +1002,95 @@ describe("mid-run steering rendering ([user_steering] user messages)", () => {
|
||||
expect(stripAnsi(text())).toContain("tail output");
|
||||
});
|
||||
});
|
||||
|
||||
describe("supportsColor (color gating, issue #102)", () => {
|
||||
it("requires a TTY when no color variables are set", () => {
|
||||
expect(supportsColor({ isTTY: true }, {})).toBe(true);
|
||||
expect(supportsColor({ isTTY: false }, {})).toBe(false);
|
||||
expect(supportsColor({}, {})).toBe(false);
|
||||
});
|
||||
|
||||
it("NO_COLOR (non-empty) and TERM=dumb turn color off even on a TTY", () => {
|
||||
expect(supportsColor({ isTTY: true }, { NO_COLOR: "1" })).toBe(false);
|
||||
expect(supportsColor({ isTTY: true }, { TERM: "dumb" })).toBe(false);
|
||||
// An empty NO_COLOR counts as unset (no-color.org).
|
||||
expect(supportsColor({ isTTY: true }, { NO_COLOR: "" })).toBe(true);
|
||||
});
|
||||
|
||||
it("FORCE_COLOR overrides everything, NO_COLOR and pipes included (Node semantics)", () => {
|
||||
expect(supportsColor({ isTTY: false }, { FORCE_COLOR: "1" })).toBe(true);
|
||||
// The exact environment #102 observed in the nested CLI: FORCE_COLOR=3 + NO_COLOR=1 + TERM=dumb.
|
||||
expect(supportsColor({ isTTY: false }, { FORCE_COLOR: "3", NO_COLOR: "1", TERM: "dumb" })).toBe(
|
||||
true,
|
||||
);
|
||||
expect(supportsColor({ isTTY: true }, { FORCE_COLOR: "0" })).toBe(false);
|
||||
// An empty FORCE_COLOR is not an override; the normal gate applies.
|
||||
expect(supportsColor({ isTTY: false }, { FORCE_COLOR: "" })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("StreamRenderer color wiring (issue #102)", () => {
|
||||
it("a piped (non-TTY) stream gets no ANSI escapes at all", () => {
|
||||
// Neutralize any ambient override; the collector stream is not a TTY, so the palette is plain.
|
||||
vi.stubEnv("FORCE_COLOR", "");
|
||||
const { stream, text } = collector();
|
||||
const r = new StreamRenderer(stream, t);
|
||||
r.handle(partialToolCall({ eventType: "start", name: "exec_command", toolCallId: "p1" }));
|
||||
r.handle(
|
||||
partialToolCall({
|
||||
eventType: "delta",
|
||||
name: "",
|
||||
arguments: '{"cmd":"cat todo.md"}',
|
||||
toolCallId: "p1",
|
||||
}),
|
||||
);
|
||||
r.handle(partialToolCall({ eventType: "stop", name: "", toolCallId: "p1" }));
|
||||
r.handle(partialToolCallOutput({ eventType: "start", toolCallId: "p1" }));
|
||||
r.handle(partialToolCallOutput({ eventType: "delta", output: "hello\n", toolCallId: "p1" }));
|
||||
r.handle(partialToolCallOutput({ eventType: "stop", toolCallId: "p1" }));
|
||||
r.handle(thinkingMessage("hmm"));
|
||||
r.handle(partialThinking("start", ""));
|
||||
r.handle(partialThinking("delta", "pondering"));
|
||||
r.handle(partialThinking("stop"));
|
||||
const raw = text();
|
||||
expect(raw).not.toContain("\x1b");
|
||||
expect(raw).toContain("[tool-p1] exec_command <- $ cat todo.md");
|
||||
expect(raw).toContain("[tool-p1] exec_command -> hello");
|
||||
});
|
||||
|
||||
it("renderHistory on a piped stream is escape-free too", () => {
|
||||
vi.stubEnv("FORCE_COLOR", "");
|
||||
const { stream, text } = collector();
|
||||
renderHistory(
|
||||
[
|
||||
userText("hi"),
|
||||
thinkingMessage("t"),
|
||||
toolCall({ name: "exec_command", arguments: '{"cmd":"ls"}', toolCallId: "h1" }),
|
||||
toolCallOutput({ output: "a.txt\n", toolCallId: "h1" }),
|
||||
],
|
||||
stream,
|
||||
t,
|
||||
);
|
||||
const raw = text();
|
||||
expect(raw).not.toContain("\x1b");
|
||||
expect(raw).toContain("[tool-h1] exec_command <- $ ls");
|
||||
});
|
||||
|
||||
it("FORCE_COLOR=1 re-enables color on a pipe and defeats NO_COLOR", () => {
|
||||
vi.stubEnv("FORCE_COLOR", "1");
|
||||
vi.stubEnv("NO_COLOR", "1");
|
||||
const { stream, text } = collector();
|
||||
const r = new StreamRenderer(stream, t);
|
||||
r.handle(partialToolCall({ eventType: "start", name: "exec_command", toolCallId: "f1" }));
|
||||
r.handle(
|
||||
partialToolCall({
|
||||
eventType: "delta",
|
||||
name: "",
|
||||
arguments: '{"cmd":"ls"}',
|
||||
toolCallId: "f1",
|
||||
}),
|
||||
);
|
||||
r.handle(partialToolCall({ eventType: "stop", name: "", toolCallId: "f1" }));
|
||||
expect(text()).toContain("\x1b[36m");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -33,8 +33,8 @@ const HARDENED_ENV: NodeJS.ProcessEnv = {
|
||||
};
|
||||
|
||||
/**
|
||||
* Harness-owned variables **removed** from the child environment (removed, not blanked: a
|
||||
* program that checks `PORT` for presence rather than value must see nothing at all).
|
||||
* Variables **removed** from the child environment (removed, not blanked: a program that
|
||||
* checks `PORT` for presence rather than value must see nothing at all).
|
||||
*
|
||||
* `PORT` / `HOST` are stripped because they are never about the command being run. On the
|
||||
* serving paths they are the harness's own listener: `penguin web` / `penguin server` write both
|
||||
@@ -56,6 +56,13 @@ const HARDENED_ENV: NodeJS.ProcessEnv = {
|
||||
* PenguinHarness server would otherwise serve the deployment's assets instead of the ones it just
|
||||
* built in the workspace, silently and with no error to read.
|
||||
*
|
||||
* `FORCE_COLOR` / `CLICOLOR_FORCE` are color-forcing overrides that Node (and the chalk-family
|
||||
* libraries) deliberately let defeat `NO_COLOR`, so an inherited value would cancel the
|
||||
* `NO_COLOR=1` + `TERM=dumb` hardening above and leak ANSI escapes into tool output (#102).
|
||||
* Removal, not blanking, matters here too: Node reads an *empty* `FORCE_COLOR` as "force 16
|
||||
* colors on". The vault still wins, so a user who genuinely wants forced color in commands can
|
||||
* set it there.
|
||||
*
|
||||
* Deliberately **not** stripped: `PENGUIN_HOME`, `PENGUIN_WEB_DB` and the rest of the user-facing
|
||||
* `PENGUIN_*` settings. Those select the *data* an Agent-started harness works against, and the
|
||||
* self-development case may legitimately want the same data root — sharing state is a config
|
||||
@@ -66,6 +73,8 @@ const STRIPPED_ENV_KEYS = new Set([
|
||||
"HOST",
|
||||
"PENGUIN_CLI_ENTRY",
|
||||
"PENGUIN_WEB_DIST",
|
||||
"FORCE_COLOR",
|
||||
"CLICOLOR_FORCE",
|
||||
// Desktop-mode process credentials and wiring: the shell's token authorizes the
|
||||
// server shutdown endpoint (and desktop-login until redeemed), and the port file is
|
||||
// the shell's private channel — neither is a user-facing setting, and leaking them
|
||||
|
||||
@@ -355,6 +355,29 @@ describe("harness environment variables never reach a spawned command", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("inherited FORCE_COLOR is removed, so the NO_COLOR=1 hardening actually wins", async () => {
|
||||
// Node deliberately lets FORCE_COLOR defeat NO_COLOR, so a nested `penguin run` under a
|
||||
// color-forcing parent (issue #102 observed FORCE_COLOR=3, NO_COLOR=1, TERM=dumb at once)
|
||||
// would keep emitting ANSI escapes unless the inherited override is removed outright.
|
||||
const saved = {
|
||||
FORCE_COLOR: process.env.FORCE_COLOR,
|
||||
CLICOLOR_FORCE: process.env.CLICOLOR_FORCE,
|
||||
};
|
||||
process.env.FORCE_COLOR = "3";
|
||||
process.env.CLICOLOR_FORCE = "1";
|
||||
try {
|
||||
const res = await runTool(env, "exec_command", {
|
||||
cmd: `node -e "console.log('F=[' + (process.env.FORCE_COLOR ?? '') + '] C=[' + (process.env.CLICOLOR_FORCE ?? '') + '] N=[' + (process.env.NO_COLOR ?? '') + ']')"`,
|
||||
});
|
||||
expect(res.output).toContain("F=[] C=[] N=[1]");
|
||||
} finally {
|
||||
for (const [k, v] of Object.entries(saved)) {
|
||||
if (v === undefined) delete process.env[k];
|
||||
else process.env[k] = v;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("the rest of the host environment still passes through", async () => {
|
||||
process.env.PENGUIN_TEST_PASSTHROUGH = "kept";
|
||||
try {
|
||||
|
||||
@@ -15,6 +15,7 @@ import type { UpdateRunResponse } from "@prismshadow/penguin-server/api";
|
||||
import * as api from "../../api/endpoints";
|
||||
import { S } from "../../lib/strings";
|
||||
import { apiErrorText } from "../../lib/api-error";
|
||||
import { stripAnsi } from "../../lib/strip-ansi";
|
||||
import { Button } from "../ui/button";
|
||||
import { Modal } from "../ui/modal";
|
||||
|
||||
@@ -117,7 +118,8 @@ export function UpdateDialog({
|
||||
)}
|
||||
{result.output !== "" && (
|
||||
<pre className="max-h-56 overflow-auto rounded-md bg-gray-100 p-3 text-xs leading-relaxed whitespace-pre-wrap text-gray-700 dark:bg-gray-800 dark:text-gray-300">
|
||||
{result.output}
|
||||
{/* The update runs a package manager whose output may carry ANSI color when the server env forces it. */}
|
||||
{stripAnsi(result.output)}
|
||||
</pre>
|
||||
)}
|
||||
</div>
|
||||
|
||||
@@ -14,9 +14,10 @@
|
||||
* segment is shown; the wait itself is marked by the amber hourglass icon alone, since the
|
||||
* approval block below the row is always on screen and names the tool and its arguments.
|
||||
*/
|
||||
import { useRef, useState } from "react";
|
||||
import { useMemo, useRef, useState } from "react";
|
||||
import { S } from "../../lib/strings";
|
||||
import { humanizeDuration } from "../../lib/format";
|
||||
import { stripAnsi } from "../../lib/strip-ansi";
|
||||
import type { StopReason } from "@prismshadow/penguin-core/omnimessage";
|
||||
import { approvalKey } from "../../lib/omni/stream-model";
|
||||
import type { ToolCallItem } from "../../lib/omni/stream-model";
|
||||
@@ -208,6 +209,11 @@ export function ToolCallCard({ item, ctx }: { item: ToolCallItem; ctx: StreamRen
|
||||
const pending = ctx.pendingApprovals.get(approvalKey(ctx.origin, item.toolCallId));
|
||||
|
||||
const preview = previewArguments(item.name, item.argumentsText);
|
||||
// Escape sequences are stripped at render time only (the stored stream/trace data keeps its
|
||||
// raw bytes): hardened child envs should no longer produce any, but historical traces and
|
||||
// force-color programs still can (#102). Memoized — the aggregated output can be large and
|
||||
// grows on every streamed delta.
|
||||
const output = useMemo(() => stripAnsi(item.output), [item.output]);
|
||||
// Settled once argument streaming stopped (or the complete call arrived): the subtitle's
|
||||
// completeness gate is lifted — whatever is there is final.
|
||||
const subtitle = headerSubtitle(item.name, item.argumentsText, !item.callStreaming);
|
||||
@@ -390,7 +396,7 @@ export function ToolCallCard({ item, ctx }: { item: ToolCallItem; ctx: StreamRen
|
||||
)}
|
||||
{(item.output || item.outputStreaming) && (
|
||||
<pre className="max-h-72 overflow-auto whitespace-pre-wrap border-t border-gray-100 px-3 py-2 text-xs leading-5 text-gray-600 dark:border-gray-800 dark:text-gray-300">
|
||||
{item.output}
|
||||
{output}
|
||||
{item.outputStreaming && <span className="animate-pulse">▌</span>}
|
||||
</pre>
|
||||
)}
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
/**
|
||||
* ANSI escape stripping for raw command/tool output display.
|
||||
*
|
||||
* The command tools harden the child environment against color (`NO_COLOR=1`, `TERM=dumb`,
|
||||
* inherited `FORCE_COLOR` removed — see core's command session-manager), but traces recorded
|
||||
* before that fix and programs that force color unconditionally can still carry escape
|
||||
* sequences (#102). The chat surface therefore strips them defensively at RENDER time only —
|
||||
* stored trace/stream data is never mutated.
|
||||
*/
|
||||
|
||||
/**
|
||||
* One complete ANSI escape sequence:
|
||||
* - CSI: `ESC [` + parameter bytes (0x30–0x3F) + intermediate bytes (0x20–0x2F) + one final
|
||||
* byte (0x40–0x7E) — covers SGR color codes such as `\x1b[36m` and `\x1b[1;31m`.
|
||||
* - OSC: `ESC ]` + payload, terminated by BEL or ST (`ESC \`) — window titles, hyperlinks.
|
||||
* - Other Fe escapes: `ESC` + one byte in 0x40–0x5F (minus `[` / `]`, handled above).
|
||||
*/
|
||||
const ANSI_SEQUENCE = /\x1b(?:\[[0-9:;<=>?]*[ -/]*[@-~]|\][^\x07\x1b]*(?:\x07|\x1b\\)|[@-Z\\^_])/g;
|
||||
|
||||
/**
|
||||
* An incomplete escape sequence at end-of-string: chunks reassemble into one aggregated
|
||||
* string before rendering, but live output can still pause mid-sequence, so a trailing bare
|
||||
* `ESC`, unfinished CSI (`ESC [ 3`) or unterminated OSC is dropped rather than shown as
|
||||
* garbage; once the tail arrives the whole sequence is removed by the pattern above.
|
||||
*/
|
||||
const ANSI_TRAILER = /\x1b(?:\[[0-9:;<=>?]*[ -/]*|\][^\x07]*)?$/;
|
||||
|
||||
/** Strips ANSI escape sequences for display. Fast path: input without an ESC byte is returned as-is (same reference). */
|
||||
export function stripAnsi(text: string): string {
|
||||
if (!text.includes("\x1b")) return text;
|
||||
return text.replace(ANSI_SEQUENCE, "").replace(ANSI_TRAILER, "");
|
||||
}
|
||||
@@ -0,0 +1,54 @@
|
||||
/**
|
||||
* stripAnsi unit tests: render-time removal of ANSI escape sequences from raw command/tool
|
||||
* output (#102), including sequences split across stream chunks and cut off at end-of-string.
|
||||
*/
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { stripAnsi } from "../src/lib/strip-ansi";
|
||||
|
||||
describe("stripAnsi", () => {
|
||||
it("returns text without escapes unchanged (fast path, same reference)", () => {
|
||||
const s = "plain text, no escapes [36m literal brackets stay";
|
||||
expect(stripAnsi(s)).toBe(s);
|
||||
expect(stripAnsi("")).toBe("");
|
||||
});
|
||||
|
||||
it("strips plain SGR color codes", () => {
|
||||
expect(stripAnsi("\x1b[36mcyan\x1b[0m")).toBe("cyan");
|
||||
expect(stripAnsi("\x1b[2mdim\x1b[0m and \x1b[32mgreen\x1b[0m")).toBe("dim and green");
|
||||
});
|
||||
|
||||
it("strips multi-parameter SGR codes", () => {
|
||||
expect(stripAnsi("\x1b[1;31mbold red\x1b[0m")).toBe("bold red");
|
||||
expect(stripAnsi("\x1b[38;5;208morange\x1b[0m")).toBe("orange");
|
||||
});
|
||||
|
||||
it("cleans the issue's nested-CLI gutter shape", () => {
|
||||
// What #102 showed in the tool card: a nested `penguin run`'s colored call line.
|
||||
const raw = "\x1b[36m[tool-752] $ \x1b[0m\x1b[36mcat todo.md\x1b[0m\n";
|
||||
expect(stripAnsi(raw)).toBe("[tool-752] $ cat todo.md\n");
|
||||
});
|
||||
|
||||
it("strips OSC sequences (BEL- and ST-terminated) and two-byte escapes", () => {
|
||||
expect(stripAnsi("\x1b]0;window title\x07rest")).toBe("rest");
|
||||
expect(stripAnsi("\x1b]8;;https://example.com\x1b\\link text\x1b]8;;\x1b\\")).toBe("link text");
|
||||
expect(stripAnsi("a\x1bMb")).toBe("ab");
|
||||
});
|
||||
|
||||
it("handles a sequence split across a chunk boundary once chunks are concatenated", () => {
|
||||
const chunk1 = "before \x1b[3";
|
||||
const chunk2 = "6mblue\x1b[0m after";
|
||||
expect(stripAnsi(chunk1 + chunk2)).toBe("before blue after");
|
||||
});
|
||||
|
||||
it("drops an incomplete trailing escape sequence (stream cut mid-sequence)", () => {
|
||||
expect(stripAnsi("text\x1b")).toBe("text");
|
||||
expect(stripAnsi("text\x1b[")).toBe("text");
|
||||
expect(stripAnsi("text\x1b[36")).toBe("text");
|
||||
expect(stripAnsi("text\x1b[1;3")).toBe("text");
|
||||
expect(stripAnsi("text\x1b]0;half a title")).toBe("text");
|
||||
});
|
||||
|
||||
it("keeps surrounding text intact when stripping mid-string sequences", () => {
|
||||
expect(stripAnsi("ok\x1b[31mfail\x1b[0mok")).toBe("okfailok");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user