fix(coding-agent): strip ANSI sequences split across user bash output chunks

closes #10504
This commit is contained in:
David Brailovsky 2026-10-07 15:02:59 +02:00
parent 592fb57b70
commit 27c7b6ff48
4 changed files with 138 additions and 62 deletions

View file

@ -25,6 +25,7 @@
- Fixed `/mcp` waiting for all servers to connect before opening; the manager now updates live and remains usable while enabling, reconnecting, or disabling servers ([#10562](https://github.com/earendil-works/pi/issues/10562))
- Fixed images being dropped as "could not be resized" when running under `node --watch` on Node 24.19+ and 26.x, where Node posts its own messages on the image resize worker channel ([#10527](https://github.com/earendil-works/pi/issues/10527))
- Fixed clipboard paste doing nothing in Termux, and failed copies there omitting the Termux:API install hint ([#10391](https://github.com/earendil-works/pi/issues/10391))
- Fixed `!` and RPC `bash` output keeping fragments of color codes, such as a stray `m`, when a code was split across output chunks ([#10504](https://github.com/earendil-works/pi/issues/10504))
## [1.0.4] - 2026-10-05

View file

@ -7,7 +7,7 @@
*/
import type { WriteStream } from "node:fs";
import { stripAnsi } from "../utils/ansi.ts";
import { splitIncompleteAnsiSuffix, stripAnsi } from "../utils/ansi.ts";
import { createOutputFileStream } from "../utils/output-files.ts";
import { sanitizeBinaryOutput } from "../utils/shell.ts";
import type { BashOperations } from "./tools/bash.ts";
@ -70,12 +70,15 @@ export async function executeBashWithOperations(
};
const decoder = new TextDecoder();
// Unfinished escape sequence at the end of the previous chunk, completed by the next chunk.
let pendingAnsi = "";
const onData = (data: Buffer) => {
totalBytes += data.length;
const appendText = (rawText: string) => {
// Sanitize: strip ANSI, replace binary garbage, normalize newlines
const text = sanitizeBinaryOutput(stripAnsi(decoder.decode(data, { stream: true }))).replace(/\r/g, "");
const text = sanitizeBinaryOutput(stripAnsi(rawText)).replace(/\r/g, "");
if (!text) {
return;
}
// Start writing to temp file if exceeds threshold
if (totalBytes > DEFAULT_MAX_BYTES) {
@ -100,53 +103,44 @@ export async function executeBashWithOperations(
}
};
const onData = (data: Buffer) => {
totalBytes += data.length;
const { complete, pending } = splitIncompleteAnsiSuffix(pendingAnsi + decoder.decode(data, { stream: true }));
pendingAnsi = pending;
appendText(complete);
};
const flushOutput = () => {
const rest = pendingAnsi + decoder.decode();
pendingAnsi = "";
appendText(rest);
};
let exitCode: number | null = null;
try {
const result = await operations.exec(command, cwd, {
onData,
signal: options?.signal,
});
const fullOutput = outputChunks.join("");
const truncationResult = truncateTail(fullOutput);
if (truncationResult.truncated) {
ensureTempFile();
}
if (tempFileStream) {
tempFileStream.end();
}
const cancelled = options?.signal?.aborted ?? false;
return {
output: truncationResult.truncated ? truncationResult.content : fullOutput,
exitCode: cancelled ? undefined : (result.exitCode ?? undefined),
cancelled,
truncated: truncationResult.truncated,
fullOutputPath: tempFilePath,
};
({ exitCode } = await operations.exec(command, cwd, { onData, signal: options?.signal }));
} catch (err) {
// Check if it was an abort
if (options?.signal?.aborted) {
const fullOutput = outputChunks.join("");
const truncationResult = truncateTail(fullOutput);
if (truncationResult.truncated) {
ensureTempFile();
}
if (tempFileStream) {
tempFileStream.end();
}
return {
output: truncationResult.truncated ? truncationResult.content : fullOutput,
exitCode: undefined,
cancelled: true,
truncated: truncationResult.truncated,
fullOutputPath: tempFilePath,
};
// An aborted command still returns the output it produced so far
if (!options?.signal?.aborted) {
tempFileStream?.end();
throw err;
}
if (tempFileStream) {
tempFileStream.end();
}
throw err;
}
flushOutput();
const fullOutput = outputChunks.join("");
const truncationResult = truncateTail(fullOutput);
if (truncationResult.truncated) {
ensureTempFile();
}
tempFileStream?.end();
const cancelled = options?.signal?.aborted ?? false;
return {
output: truncationResult.truncated ? truncationResult.content : fullOutput,
exitCode: cancelled ? undefined : (exitCode ?? undefined),
cancelled,
truncated: truncationResult.truncated,
fullOutputPath: tempFilePath,
};
}

View file

@ -26,23 +26,45 @@
* SOFTWARE.
*/
function ansiRegex({ onlyFirst = false }: { onlyFirst?: boolean } = {}): RegExp {
// Valid string terminator sequences are BEL, ESC\, and 0x9c
const ST = "(?:\\u0007|\\u001B\\u005C|\\u009C)";
// Valid string terminator sequences are BEL, ESC\, and 0x9c
const ST = "(?:\\u0007|\\u001B\\u005C|\\u009C)";
// OSC sequences only: ESC ] ... ST (non-greedy until the first ST)
const osc = `(?:\\u001B\\][\\s\\S]*?${ST})`;
// OSC sequences: ESC ] ... ST
const OSC_START = "\\u001B\\]";
// CSI and related: ESC/C1, optional intermediates, optional params (supports ; and :) then final byte
const csi = "[\\u001B\\u009B][[\\]()#;?]*(?:\\d{1,4}(?:[;:]\\d{0,4})*)?[\\dA-PR-TZcf-nq-uy=><~]";
// CSI and related: ESC/C1, optional intermediates, optional params (supports ; and :), then final byte
const CSI_START = "[\\u001B\\u009B][[\\]()#;?]*(?:\\d{1,4}(?:[;:]\\d{0,4})*)?";
const CSI_FINAL = "[\\dA-PR-TZcf-nq-uy=><~]";
const pattern = `${osc}|${csi}`;
// Complete sequences. OSC is non-greedy until the first ST.
const ansiRegex = new RegExp(`(?:${OSC_START}[\\s\\S]*?${ST})|${CSI_START}${CSI_FINAL}`, "g");
return new RegExp(pattern, onlyFirst ? undefined : "g");
// Unfinished sequence at the end of the text: OSC without its ST (a trailing ESC may start ESC\),
// or CSI without its final byte.
const unfinishedAnsiAtEndRegex = new RegExp(
`(?:${OSC_START}(?:[^\\u0007\\u009C\\u001B]|\\u001B(?!\\\\))*|${CSI_START})$`,
);
// Longest unfinished sequence held back while streaming. Longer ones are processed as-is.
const MAX_PENDING_ANSI_LENGTH = 256;
/**
* Split streamed text into a part that is safe to pass to stripAnsi now and a trailing
* unfinished escape sequence that should be prepended to the next chunk.
*/
export function splitIncompleteAnsiSuffix(value: string): { complete: string; pending: string } {
if (!value.includes("\u001B") && !value.includes("\u009B")) {
return { complete: value, pending: "" };
}
const windowStart = Math.max(0, value.length - MAX_PENDING_ANSI_LENGTH);
const match = unfinishedAnsiAtEndRegex.exec(value.slice(windowStart));
if (!match) {
return { complete: value, pending: "" };
}
const splitAt = windowStart + match.index;
return { complete: value.slice(0, splitAt), pending: value.slice(splitAt) };
}
const regex = ansiRegex();
export function stripAnsi(value: string): string {
if (typeof value !== "string") {
throw new TypeError(`Expected a \`string\`, got \`${typeof value}\``);
@ -56,5 +78,5 @@ export function stripAnsi(value: string): string {
// Even though the regex is global, we don't need to reset the `.lastIndex`
// because unlike `.exec()` and `.test()`, `.replace()` does it automatically
// and doing it manually has a performance penalty.
return value.replace(regex, "");
return value.replace(ansiRegex, "");
}

View file

@ -337,4 +337,63 @@ describe("AgentSession bash and persistence characterization", () => {
{ id: "bash-1", delta: "world" },
]);
});
// Regression tests for https://github.com/earendil-works/pi/issues/10504
describe("escape sequences split across output chunks", () => {
/** Runs a user bash command whose fake shell emits the given chunks; `beforeExit` sees what was streamed by then. */
async function runChunks(chunks: Array<string | Buffer>, beforeExit?: (streamed: string) => void) {
const harness = await createHarness();
harnesses.push(harness);
const deltas: string[] = [];
const operations: BashOperations = {
exec: async (_command, _cwd, options) => {
for (const chunk of chunks) {
options.onData(typeof chunk === "string" ? Buffer.from(chunk) : chunk);
}
beforeExit?.(deltas.join(""));
return { exitCode: 0 };
},
};
const result = await harness.session.executeBash("custom", (delta) => deltas.push(delta), { operations });
const recorded = harness.session.messages[harness.session.messages.length - 1];
return {
output: result.output,
streamed: deltas.join(""),
recorded: recorded?.role === "bashExecution" ? recorded.output : undefined,
};
}
it("strips a color reset split inside its parameters", async () => {
const { output, streamed, recorded } = await runChunks(["\x1b[31mERROR: file.py:1\x1b[0", "m\n"]);
expect(output).toBe("ERROR: file.py:1\n");
expect(streamed).toBe("ERROR: file.py:1\n");
expect(recorded).toBe("ERROR: file.py:1\n");
});
it("strips a color code split right after ESC", async () => {
const { output, streamed } = await runChunks(["before\x1b", "[32mafter\n"]);
expect(output).toBe("beforeafter\n");
expect(streamed).toBe("beforeafter\n");
});
it("strips an OSC sequence split before its terminator", async () => {
const { output } = await runChunks(["a\x1b]0;window ", "title\x1b", "\\b\n"]);
expect(output).toBe("ab\n");
});
it("flushes an incomplete multi-byte character at the end of output", async () => {
const { output, streamed } = await runChunks(["ok", Buffer.from("\u00e9").subarray(0, 1)]);
expect(output).toBe("ok\uFFFD");
expect(streamed).toBe("ok\uFFFD");
});
it("does not hold back output behind a long unterminated sequence", async () => {
const long = "x".repeat(300);
let streamedBeforeExit = "";
await runChunks([`\x1b]${long}`], (streamed) => {
streamedBeforeExit = streamed;
});
expect(streamedBeforeExit).toBe(`]${long}`);
});
});
});