Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/lazy-pagers-relax.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"hunkdiff": patch
---

Fix `hunk pager` pegging a CPU core and growing to gigabytes of memory on large color-heavy
input. Restoring preserved ANSI styling rescanned and reallocated the whole document once per
sequence, so a `git log --graph --color=always` stream from a host like LazyGit took minutes of
solid CPU per process and never produced output. Styling is now restored in a single pass: a 3 MB
branch log pages through in well under a second.
7 changes: 7 additions & 0 deletions .changeset/loud-pipes-deliver.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"hunkdiff": patch
---

Fix `hunk pager` truncating its output at 64 KB when a host reads it through a pipe, which cut off
large documents for Git's pager contract, LazyGit, and `| less`. Headless commands now hand the
whole document to the stdout descriptor before exiting, so a piped consumer receives every byte.
37 changes: 37 additions & 0 deletions src/core/process/pager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,43 @@ describe("plain text pager fallback", () => {
expectNoUnsafeTerminalControls(written);
});

test("pages ANSI-dense git log output promptly and with its color intact", async () => {
// LazyGit and similar hosts point Git's pager at `hunk pager` for whole branch logs, so a
// non-patch `git log --graph --color=always` stream arrives here carrying one SGR sequence
// every ~18 bytes. Sanitizing that used to be quadratic in the sequence count, which pegged a
// core for minutes per concurrent LazyGit job instead of paging the text straight through.
const gitLog = Array.from(
{ length: 5_000 },
(_, index) =>
`\x1b[33m* commit ${index}\x1b[m \x1b[1;36m(\x1b[1;32mHEAD\x1b[1;36m)\x1b[m\n` +
`\x1b[32m| Author: someone\x1b[m\n`,
).join("");
let written = "";

const startedAt = performance.now();
await pagePlainText(
gitLog,
{ PAGER: "less -R" },
createPagerDeps({
spawnImpl() {
const pager = new EventEmitter() as EventEmitter & { stdin: PassThrough };
pager.stdin = new PassThrough();
pager.stdin.on("data", (chunk) => {
written += String(chunk);
});
pager.stdin.on("finish", () => {
queueMicrotask(() => pager.emit("close", 0));
});
return pager as never;
},
}),
);
const elapsedMs = performance.now() - startedAt;

expect(written).toBe(gitLog);
expect(elapsedMs).toBeLessThan(2_000);
});

test("spawns pager commands without a shell", async () => {
const pager = new EventEmitter() as EventEmitter & { stdin: PassThrough };
pager.stdin = new PassThrough();
Expand Down
11 changes: 10 additions & 1 deletion src/core/process/pager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { spawn, type ChildProcess, type SpawnOptions } from "node:child_process"
import { parse as parseShellCommand, type ParseEntry } from "shell-quote";
import { stripTerminalControl } from "../patch/sanitize";
import { sanitizeTerminalText } from "../../lib/terminalText";
import { writeStdout } from "./stdout";

/** Detect whether generic pager stdin looks like a diff/patch that Hunk should review. */
export function looksLikePatchInput(text: string) {
Expand Down Expand Up @@ -146,7 +147,15 @@ export async function pagePlainText(
text: string,
env: NodeJS.ProcessEnv = process.env,
deps: PlainTextPagerDeps = {
stdout: process.stdout,
// Write through the descriptor rather than `process.stdout`: a piped consumer takes one
// buffer at a time, and the caller exits as soon as this returns.
stdout: {
isTTY: process.stdout.isTTY,
write: (chunk) => {
writeStdout(String(chunk));
return true;
},
},
spawnImpl: spawn,
},
) {
Expand Down
100 changes: 100 additions & 0 deletions src/core/process/stdout.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import { describe, expect, test } from "bun:test";
import { writeStdout } from "./stdout";

/**
* Record descriptor writes, optionally accepting only part of each chunk.
*
* Chunks are kept as bytes and decoded once at the end: a partial write can split a multi-byte
* character, so decoding each chunk on its own would report corruption the descriptor never saw.
*/
function createRecordingWrite(acceptBytes?: number) {
const chunks: Buffer[] = [];
const writeImpl = (_fd: number, buffer: Uint8Array, offset: number, length: number) => {
const written = acceptBytes === undefined ? length : Math.min(acceptBytes, length);
chunks.push(Buffer.from(Buffer.from(buffer).subarray(offset, offset + written)));
return written;
};
return { chunks, writeImpl, text: () => Buffer.concat(chunks).toString("utf8") };
}

/** Build an errno failure the way `writeSync` reports one. */
function errnoError(code: string) {
return Object.assign(new Error(code), { code });
}

describe("writeStdout", () => {
test("hands the whole document to the descriptor", () => {
const recorder = createRecordingWrite();

writeStdout("hello pager", { writeImpl: recorder.writeImpl });

expect(recorder.text()).toBe("hello pager");
});

test("resumes partial writes until the consumer has taken every byte", () => {
// A pipe accepts one buffer at a time, so a large document is always written in pieces.
const document = "x".repeat(200_000);
const recorder = createRecordingWrite(65_536);

writeStdout(document, { writeImpl: recorder.writeImpl });

expect(recorder.text()).toBe(document);
expect(recorder.chunks.length).toBeGreaterThan(1);
});

test("preserves multi-byte characters split across partial writes", () => {
const document = "日本語".repeat(1_000);
const recorder = createRecordingWrite(7);

writeStdout(document, { writeImpl: recorder.writeImpl });

expect(recorder.text()).toBe(document);
});

test("waits for room instead of spinning when the descriptor is non-blocking", () => {
const recorder = createRecordingWrite();
const sleeps: number[] = [];
let refusals = 2;

writeStdout("deferred", {
writeImpl: (fd, buffer, offset, length) => {
if (refusals > 0) {
refusals -= 1;
throw errnoError("EAGAIN");
}
return recorder.writeImpl(fd, buffer, offset, length);
},
sleepImpl: (ms) => sleeps.push(ms),
});

expect(recorder.text()).toBe("deferred");
expect(sleeps).toEqual([1, 1]);
});

test("stops quietly when the consumer closes early", () => {
const recorder = createRecordingWrite(4);

expect(() =>
writeStdout("long document", {
writeImpl: (fd, buffer, offset, length) => {
if (offset > 0) {
throw errnoError("EPIPE");
}
return recorder.writeImpl(fd, buffer, offset, length);
},
}),
).not.toThrow();

expect(recorder.text()).toBe("long");
});

test("surfaces unexpected descriptor failures", () => {
expect(() =>
writeStdout("text", {
writeImpl: () => {
throw errnoError("ENOSPC");
},
}),
).toThrow("ENOSPC");
});
});
48 changes: 48 additions & 0 deletions src/core/process/stdout.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* Hands a finished document to stdout and waits for the consumer to take all of it.
*
* Headless commands write their whole document in one call and then exit. Bun's `process.stdout`
* reports no backpressure for a pipe — `write` returns true and `writableLength` stays 0 while only
* one pipe buffer (64 KB on Linux) has actually been handed over — so the exit discarded the rest
* and silently truncated output for every consumer reading through a pipe: Git's pager contract,
* LazyGit, `| less`. A file or a terminal takes the whole document at once, which is why the loss
* only appeared under a pipe. Writing straight to the descriptor blocks until the consumer has
* taken every byte, so the caller can exit as soon as this returns.
*/
import { writeSync } from "node:fs";

const STDOUT_FD = 1;
/** Pause before retrying a descriptor that is momentarily full, rather than spinning on it. */
const NON_BLOCKING_RETRY_MS = 1;

/** Test seams for descriptor writes; production always targets the real stdout descriptor. */
export interface WriteStdoutDeps {
writeImpl?: (fd: number, buffer: Uint8Array, offset: number, length: number) => number;
sleepImpl?: (ms: number) => void;
}

/** Write text to stdout, resuming partial writes until the consumer has taken the whole document. */
export function writeStdout(text: string, deps: WriteStdoutDeps = {}) {
const write = deps.writeImpl ?? writeSync;
const sleep = deps.sleepImpl ?? Bun.sleepSync;
const buffer = Buffer.from(text, "utf8");
let offset = 0;

while (offset < buffer.length) {
try {
offset += write(STDOUT_FD, buffer, offset, buffer.length - offset);
} catch (error) {
const code = (error as NodeJS.ErrnoException).code;
// A consumer that stops reading early (`| head`) leaves nothing left to deliver.
if (code === "EPIPE") {
return;
}
// Only a non-blocking descriptor reports this, and only until it has room again.
if (code === "EAGAIN") {
sleep(NON_BLOCKING_RETRY_MS);
continue;
}
throw error;
}
}
}
24 changes: 24 additions & 0 deletions src/lib/terminalText.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,30 @@ describe("sanitizeTerminalText", () => {
expect(output).toBe("safe0\x1b[31mred\x1b[m");
});

test("restores dense ANSI styling in linear time", () => {
// `hunk pager` receives whole `git log --graph --color=always` streams from hosts like
// LazyGit: a few megabytes carrying ~170k SGR sequences. Restoring styles one at a time
// rescanned the entire document per sequence, so this input pegged a core for minutes and
// grew to gigabytes. Assert both the styled output and a wall-clock budget that only
// quadratic restoration can exceed.
const commitCount = 5_000;
const input = Array.from(
{ length: commitCount },
(_, index) =>
`\x1b[33m* commit ${index}\x1b[m \x1b[1;36m(\x1b[1;32mHEAD\x1b[1;36m)\x1b[m\n` +
`\x1b[32m| Author: someone\x1b[m\n`,
).join("");
const sequenceCount = input.match(/\x1b\[[0-9;:]*m/g)?.length ?? 0;
expect(sequenceCount).toBeGreaterThan(25_000);

const startedAt = performance.now();
const output = sanitizeTerminalText(input, { preserveAnsiStyle: true });
const elapsedMs = performance.now() - startedAt;

expect(output).toBe(input);
expect(elapsedMs).toBeLessThan(2_000);
});

test("renders path controls as visible escapes without confusing literal backslashes", () => {
const output = formatTerminalPath("dir/literal\\t-tab\tline\nescape\x1b");

Expand Down
17 changes: 13 additions & 4 deletions src/lib/terminalText.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const sevenBitControlStrings =
const c1ControlStrings = /[\x90\x98\x9d\x9e\x9f][\s\S]*?(?:\x07|\x1b\\|\x9c)/g;
const c1Csi = /\x9b[0-?]*[ -/]*[@-~]/g;
const preservedStyleTokenDelimiters = /[\u{f0000}\u{f0001}]/gu;
const preservedStyleTokens = /\u{f0000}(\d+)\u{f0001}/gu;

/** Normalize untrusted terminal-bound text before rendering it in Hunk UI surfaces. */
export function sanitizeTerminalText(
Expand Down Expand Up @@ -51,17 +52,25 @@ export function sanitizeTerminalText(
// an internal token that later restores an ANSI sequence at the wrong location.
const tokenSafeText = preserveAnsiStyle ? text.replace(preservedStyleTokenDelimiters, "") : text;

let sanitized = tokenSafeText
const sanitized = tokenSafeText
.replace(sevenBitControlStrings, preserveStyle)
.replace(c1ControlStrings, "")
.replace(c1Csi, "")
.replace(controlCharacters, "");

for (const [index, sequence] of preservedStyles.entries()) {
sanitized = sanitized.replaceAll(`\u{f0000}${index}\u{f0001}`, sequence);
if (preservedStyles.length === 0) {
return sanitized;
}

return sanitized;
// Restore every placeholder in a single pass. Replacing one style at a time rescans and
// reallocates the whole document per preserved sequence, which is quadratic in ANSI-dense
// input: a few megabytes of `git log --graph --color=always` piped through `hunk pager`
// carries ~170k sequences and would peg a core for minutes while churning gigabytes.
// Input delimiters were stripped above, so every surviving token indexes a captured style.
return sanitized.replace(
preservedStyleTokens,
(_token, index: string) => preservedStyles[Number(index)] ?? "",
);
}

/** Sanitize a single terminal row or cell where newlines must never be preserved. */
Expand Down
17 changes: 9 additions & 8 deletions src/main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import { formatCliError } from "./core/run/errors";
import { pagePlainText } from "./core/process/pager";
import { writeStdout } from "./core/process/stdout";
import { prepareStartupPlan } from "./app/startup";
import { sanitizeTerminalText } from "./lib/terminalText";
import { serveSessionBrokerDaemon } from "./session/broker/brokerServer";
Expand All @@ -11,7 +12,7 @@ async function main() {
const startupPlan = await prepareStartupPlan();

if (startupPlan.kind === "help") {
process.stdout.write(startupPlan.text);
writeStdout(startupPlan.text);
process.exit(0);
}

Expand All @@ -27,7 +28,7 @@ async function main() {
}

if (startupPlan.kind === "session-command") {
process.stdout.write(await runSessionCommand(startupPlan.input));
writeStdout(await runSessionCommand(startupPlan.input));
process.exit(0);
}

Expand All @@ -40,7 +41,7 @@ async function main() {
const canConfirm = Boolean(process.stdin.isTTY) && Boolean(process.stdout.isTTY);
process.exit(
await runExtensionManageCommand(startupPlan.input, {
stdout: (text) => process.stdout.write(text),
stdout: (text) => writeStdout(text),
stderr: (text) => process.stderr.write(text),
confirm: canConfirm
? async (question) => {
Expand All @@ -64,22 +65,22 @@ async function main() {
const { runSelfUpdateCommand } = await import("./core/install/selfUpdate");
process.exit(
await runSelfUpdateCommand(startupPlan.input, {
stdout: (text) => process.stdout.write(text),
stdout: (text) => writeStdout(text),
stderr: (text) => process.stderr.write(text),
}),
);
}

if (startupPlan.kind === "markup-guide") {
const { runMarkupGuideCommand } = await import("./ui/lib/stml/cli");
process.exit(runMarkupGuideCommand({ stdout: (text) => process.stdout.write(text) }));
process.exit(runMarkupGuideCommand({ stdout: (text) => writeStdout(text) }));
}

if (startupPlan.kind === "markup-render") {
const { runMarkupRenderCommand } = await import("./ui/lib/stml/cli");
process.exit(
await runMarkupRenderCommand(startupPlan.input, {
stdout: (text) => process.stdout.write(text),
stdout: (text) => writeStdout(text),
stderr: (text) => process.stderr.write(text),
stdoutIsTTY: Boolean(process.stdout.isTTY),
readStdinText: () => new Response(Bun.stdin.stream()).text(),
Expand All @@ -93,15 +94,15 @@ async function main() {
}

if (startupPlan.kind === "passthrough") {
process.stdout.write(
writeStdout(
sanitizeTerminalText(startupPlan.text, { preserveAnsiStyle: startupPlan.preserveColor }),
);
process.exit(0);
}

if (startupPlan.kind === "static-diff-pager") {
const { renderStaticDiffPager } = await import("./ui/staticDiffPager");
process.stdout.write(
writeStdout(
await renderStaticDiffPager(startupPlan.text, startupPlan.options, {
customThemes: startupPlan.customThemes,
stderr: process.stderr,
Expand Down
Loading
Loading