fix(pager): stop CPU pegging and truncation on large piped documents - #978
Merged
Conversation
`hunk pager` pegged a CPU core and grew to gigabytes of resident memory on large color-heavy input, such as the `git log --graph --color=always` streams LazyGit routes through Git's pager. Three concurrent LazyGit branch-log jobs each ran at ~100% CPU indefinitely and never emitted output. `sanitizeTerminalText` swapped every preserved SGR sequence for a placeholder token, then restored them with one `replaceAll` per sequence. Each of those rescanned and reallocated the whole document, making the work quadratic in the sequence count. Real `git log --graph` output carries a sequence roughly every 18 bytes, so a 3 MB branch log holds ~170k of them: minutes of solid CPU and hundreds of megabytes of churn per process, none of it interruptible, since the loop is synchronous and cannot react to the host closing the pipe. Restore every placeholder in a single pass instead. Output is unchanged, and the spoofing guard still holds because input delimiters are stripped before tokenization, so each surviving token indexes a captured style. Measured on the same 3 MB fixture and environment: 240s+ still running with no output, before; 0.4s and 163 MB peak resident, after. A differential fuzz over 20k generated and real cases confirms byte-identical output. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011eVgGiCsPXU4N8TgsHLy4w
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
Greptile SummaryReplaces quadratic ANSI placeholder restoration with a single-pass replacement while preserving the existing anti-spoofing behavior.
Confidence Score: 5/5The PR appears safe to merge with no actionable correctness, security, or repository-rule violations identified. The new restoration pass is behaviorally equivalent to the prior per-token loop for generated placeholders while removing its quadratic rescanning cost, and the affected sanitizer and pager paths have focused regression coverage. Important Files Changed
Reviews (1): Last reviewed commit: "fix(pager): restore ANSI styling in one ..." | Re-trigger Greptile |
`hunk pager` truncated its output at 65536 bytes, one Linux pipe buffer, whenever a host read it through a pipe. That is exactly how LazyGit and Git's pager contract consume it, so branch logs arrived silently cut off. Writing to a file or a terminal absorbed the whole document, which is why the loss only appeared under a pipe. Headless commands write their document in one call and then `process.exit`, which drops whatever stdout has not handed over yet. Bun offers nothing to wait on here: for a pipe, `process.stdout.write` returns true and `writableLength` stays 0 while only one buffer has actually been delivered, and the write callback fires just as early, so draining before exit is not available. Write straight to the stdout descriptor instead, resuming partial writes until the consumer has taken every byte. A consumer that stops reading early (`| head`) ends the run quietly rather than failing, which `Bun.write` does not. Verified on a 513 KB document: 65536 bytes delivered through a pipe before, the full 525601 after, matching what the same run writes to a file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011eVgGiCsPXU4N8TgsHLy4w
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes two independent bugs on the
hunk pagerpath. The first explains the reported CPU pegging; the second was found while verifying the first, and without it the reported scenario still fails, just quickly instead of slowly.1. Quadratic ANSI restoration pegged a core
hunk pagerpegs a CPU core and grows to gigabytes of resident memory on large color-heavy input, such as thegit log --graph --color=alwaysstreams LazyGit routes through Git's pager. Reporters saw three concurrent LazyGit branch-log jobs each running at ~100% CPU indefinitely, never emitting output.sanitizeTerminalTextpreserved ANSI color by swapping each SGR sequence for a placeholder token, then restoring them one at a time:Each call rescans and reallocates the whole document, making the work quadratic in the sequence count. Real
git log --graphoutput carries a sequence roughly every 18 bytes, so a 3 MB branch log holds ~170k of them.That also explains why the processes outlived LazyGit's jobs: the loop is synchronous, so the process cannot react to its pipe closing until the loop finishes. The pager already exits promptly on pipe close, SIGHUP, and parent death once it is not stuck.
Both LazyGit-facing paths hit this, since both request
preserveAnsiStyle: the captured-pager passthrough insrc/main.tsxandpagePlainTextinsrc/core/process/pager.ts.Fix: restore every placeholder in a single pass. The anti-spoofing guard is unchanged, since input delimiters are still stripped before tokenization, so every surviving token indexes a captured style.
Same 3 MB fixture, same environment, real CLI:
Isolated sanitizer timings showed clean quadratic growth before the fix, from 0.55 s at 64 KB to 17.9 s at 512 KB, extrapolating to roughly ten minutes of CPU at 3 MB.
A differential fuzz over 20,002 generated and real cases confirms byte-identical output against the old implementation, across every option combination and adversarial delimiter input.
2. Piped output was silently truncated at 64 KB
Found while verifying the above, and reproduced on unmodified
main: any document read through a pipe stopped at exactly 65536 bytes, one Linux pipe buffer, with exit code 0 and no error. Writing to a file or a terminal delivered everything, which is why the loss stayed hidden. A pipe is exactly how LazyGit and Git's pager contract consume Hunk, so the reported scenario would have produced fast but truncated logs after fix 1 alone.Headless commands write their document in one call and then
process.exit, which drops whatever stdout has not handed over yet. Bun offers nothing to wait on: for a pipe,process.stdout.writereturns true andwritableLengthstays 0 while only one buffer has actually been delivered, and the write callback fires just as early. Draining before exit is not available.Fix: write straight to the stdout descriptor, resuming partial writes until the consumer has taken every byte. A consumer that stops reading early (
| head) ends the run quietly rather than failing, whichBun.writedoes not.Verified on a 513 KB document, captured-pager host:
Tests
src/lib/terminalText.test.ts: correct styled output on 30k sequences within a 2 s budget. Verified to fail at 18.8 s against the pre-fix implementation.src/core/process/pager.test.ts: the same input throughpagePlainText.src/core/process/stdout.test.ts: partial-write resumption, multi-byte characters split across writes, non-blocking retry, early close, unexpected errno.test/cli/pager-pipe-output.test.ts: black-box CLI check that a piped consumer receives the whole document. Verified to fail against the pre-fix entrypoint.Typecheck, lint, format, dependency rules, the unit suite, the CLI contract suite, and the PTY suites pass. Remaining failures in this container are pre-existing and environmental: one Jujutsu test needs the
jjbinary, and two watch-mode tests require Bun 1.3.14 (container has 1.3.11).Note on
--fast--fastdoes not help this scenario and never did. It only setsoffloadLargeDiffonDiffPaneandlaunchFastinAppHost, both inside the mounted TUI, which non-patch pager input never reaches. Measured on unmodified main with a 208 KB colored log: 3.3 s without it, 3.0 s with it, both truncated to 65536 bytes.Still buffered
The pager continues to read stdin fully and write once, so peak memory scales with input (about 163 MB for 3 MB). That is bounded and linear now rather than unbounded. Streaming the passthrough path would cut it further and is a reasonable follow-up, but it is a larger change than these two fixes.
🤖 Generated with Claude Code
https://claude.ai/code/session_011eVgGiCsPXU4N8TgsHLy4w