• Create PR with unit tests

Description

The new pacquet differential renderer is not terminal-height-aware: it does not track rows and
emits potentially unbounded vertical cursor moves (ESC[A/ESC[B) based solely on frame
coordinates. When the frame is taller than the terminal viewport, terminals may not reliably apply
these moves (e.g., can’t move into scrollback), causing mispositioned updates, unstable redraw, and
flicker/non-converging repaint in small terminals.

Code

pacquet/crates/default-reporter/src/diff.rs[R105-118]

+    fn move_to(&mut self, out: &mut String, x: usize, y: usize) {
+        if x > self.x {
+            let _ = write!(out, "\x1b[{}C", x - self.x);
+        } else if x < self.x {
+            let _ = write!(out, "\x1b[{}D", self.x - x);
+        }
+        if y > self.y {
+            let _ = write!(out, "\x1b[{}B", y - self.y);
+        } else if y < self.y {
+            let _ = write!(out, "\x1b[{}A", self.y - y);
+        }
+        self.x = x;
+        self.y = y;
+    }

Evidence

PR Compliance ID 2 requires redraw/cursor movement to be bounded relative to terminal height, but
the current implementation only queries terminal columns and constructs Diff with
Diff::new(columns). Diff tracks only width, and its move_to emits raw ESC[A/ESC[B
vertical cursor movements without any clamp or reference to terminal row height, so it cannot
satisfy height-aware behavior when rendered frames exceed the viewport.

Interactive reporter redraw must be height-aware and must not flicker when frame exceeds terminal height
pacquet/crates/default-reporter/src/lib.rs[104-112]
pacquet/crates/default-reporter/src/diff.rs[22-30]
pacquet/crates/default-reporter/src/diff.rs[105-118]
pacquet/crates/default-reporter/src/lib.rs[100-113]
pacquet/crates/default-reporter/src/diff.rs[14-20]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The pacquet default reporter’s differential renderer (`diff::Diff`) is not terminal-height-aware: it is constructed using only terminal columns/`width`, never queries/passes terminal `rows`, and can emit large `ESC[<N>A`/`ESC[<N>B` cursor-up/down sequences without clamping to the visible viewport. This breaks the requirement that interactive redraw/cursor movement be bounded relative to terminal height and can lead to mispositioned updates and flicker/non-converging repaint when frames exceed the terminal viewport.
## Issue Context
Compliance requires interactive redraw logic to be height-aware and to prevent unstable redraw behavior when the rendered frame is taller than the terminal. The current Rust implementation computes cursor `y` from the full frame and moves the cursor with raw ANSI vertical movements without any reference to terminal height; in contrast, the TypeScript version restores `ansi-diff` with `height: stream.rows` to avoid unbounded cursor-up behavior. To align behavior, the Rust port needs an equivalent terminal-rows query and a height parameter in `Diff`, plus bounded movement and/or a safe fallback mode when the frame’s effective height exceeds the viewport.
## Fix Focus Areas
- pacquet/crates/default-reporter/src/lib.rs[100-157]
- pacquet/crates/default-reporter/src/diff.rs[14-20]
- pacquet/crates/default-reporter/src/diff.rs[22-30]
- pacquet/crates/default-reporter/src/diff.rs[105-118]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

pacquet’s new Diff computes cursor movement from visible_width() and only treats \n as a
cursor-moving control, but lifecycle output lines are ingested as raw text and can contain \r
(carriage return) which resets the terminal column. When such a line enters a rendered frame,
Diff’s internal (row,col) model can diverge from the terminal’s real cursor position and later
updates may emit incorrect cursor moves, corrupting interactive output.

Code

pacquet/crates/default-reporter/src/diff.rs[R147-199]

+    fn new(text: &str, row: usize, newline: bool, width: usize) -> Self {
+        let length = visible_width(text);
+        let (height, remainder) = match width {
+            0 => (0, length),
+            term_width => {
+                let line_height = length / term_width;
+                let line_remainder = length % term_width;
+                if line_height > 0 && line_remainder == 0 {
+                    (line_height - 1, term_width)
+                } else {
+                    (line_height, line_remainder)
+                }
+            }
+        };
+        Line { raw: text.to_string(), row, length, height, remainder, newline }
+    }
+
+    fn split(input: &str, width: usize) -> Vec<Self> {
+        let parts: Vec<&str> = input.split('\n').collect();
+        let count = parts.len();
+        let mut row_offset = 0;
+        parts
+            .into_iter()
+            .enumerate()
+            .map(|(idx, text)| {
+                let newline = idx < count - 1;
+                let line = Line::new(text, row_offset, newline, width);
+                row_offset += line.height + u8::from(newline) as usize;
+                line
+            })
+            .collect()
+    }
+
+    /// Inline diff: if only a few characters changed, write just those
+    /// instead of the whole line. Only attempted on lines without ANSI
+    /// escape codes (plain text like progress lines).
+    fn try_inline_diff(
+        &self,
+        other: &Self,
+        out: &mut String,
+        col: &mut usize,
+        row: &mut usize,
+        width: usize,
+    ) -> bool {
+        if self.length != other.length
+            || self.row != other.row
+            || !self.newline
+            || !other.newline
+            || self.raw.contains('\u{1b}')
+            || other.raw.contains('\u{1b}')
+        {
+            return false;
+        }

Evidence

Lifecycle script output is emitted as raw per-line text and passed through formatting without
stripping control chars, while Diff uses visible_width() and only gates inline diff on presence
of ESC. This makes \r (and similar controls) capable of altering the real cursor without being
reflected in Diff’s internal cursor math.

pacquet/crates/executor/src/lifecycle.rs[412-444]
pacquet/crates/reporter/src/lib.rs[497-506]
pacquet/crates/default-reporter/src/state.rs[835-842]
pacquet/crates/default-reporter/src/format.rs[67-87]
pacquet/crates/default-reporter/src/diff.rs[147-199]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`pacquet/crates/default-reporter`’s new `Diff` assumes frame content advances the cursor only via printable characters (counted by `visible_width`) and `\n`. Raw lifecycle output lines can contain control characters like `\r` that change the terminal cursor (column reset) without contributing to visible width, which can desync `Diff`’s internal cursor tracking and cause incorrect relative cursor moves on later updates.
### Issue Context
Lifecycle script stdout/stderr is emitted as raw text and then formatted into the interactive frame without stripping control characters. `visible_width()` only skips ANSI CSI sequences starting with ESC, so `\r` is treated as a visible character (width +1) even though terminals interpret it as a cursor movement.
### Fix Focus Areas
- pacquet/crates/default-reporter/src/state.rs[835-842]
- pacquet/crates/default-reporter/src/diff.rs[147-217]
- pacquet/crates/default-reporter/src/format.rs[67-87]
- pacquet/crates/executor/src/lifecycle.rs[412-444]
- pacquet/crates/reporter/src/lib.rs[497-506]
### Suggested fix approach
- Prefer sanitizing at ingestion/formatting time (so both rendering and width math agree): in `format_indented_output` (or immediately when receiving `LifecycleMessage::Stdio`), strip or replace ASCII control characters other than `\n` (e.g. remove `\r`, `\x08`, etc., or render them as printable placeholders like `^M`).
- Additionally (defense-in-depth), consider making `Diff::try_inline_diff()` bail out when either line contains non-printable control characters (not just `ESC`).
- Add a targeted unit test to demonstrate the problem and the fix, e.g. a lifecycle line containing `"foo\rbar"` should not desync subsequent updates (or should be sanitized to a stable printable form).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

pacquet’s TTY frame update is emitted via three separate stdout writes ("\r", then the diff, then
"\x1b[K\x1b[0J"), allowing external-process output on the same TTY to interleave between them and
corrupt cursor/erase control sequences. This can partially reintroduce the “external output
remnants”/screen corruption class of issues this PR is explicitly trying to harden against.

Code

pacquet/crates/default-reporter/src/lib.rs[R139-152]

+            Output::Frame(mut frame) => {
+                // A trailing newline keeps an interactive prompt on a fresh line
+                // below the frame rather than joined onto its last line, and it
+                // leaves the differ's tracked cursor at column 0 so it stays in
+                // sync with the `\r` prepended on the next update (otherwise the
+                // inline diff computes relative moves from a stale column).
+                if !frame.ends_with('\n') {
+                    frame.push('\n');
           }
-                // Reset the column to 0 (cursor-up alone keeps the column) so the
-                // redraw starts cleanly even when an external process left the
-                // cursor mid-line.
-                buf.push('\r');
-                buf.push_str("\x1b[0J");
-                buf.push_str(&frame);
-                buf.push('\n');
-                let _ = out.write_all(buf.as_bytes());
-                self.prev_rows = count_rows(&frame, self.columns);
+                let diff_output = self.diff.update(&frame);
+                let _ = out.write_all(b"\r");
+                let _ = out.write_all(diff_output.as_bytes());
+                let _ = out.write_all(b"\x1b[K\x1b[0J");
       }

Evidence

The pacquet sink currently performs three separate writes for a single frame update, which increases
the chance that unrelated output (from other processes sharing the TTY) lands between cursor reset,
diff output, and erase sequences. The TypeScript reporter demonstrates the intended single-write
pattern for the same update semantics.

pacquet/crates/default-reporter/src/lib.rs[139-152]
pnpm11/cli/default-reporter/src/index.ts[73-101]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Sink::write_output()` emits the interactive frame update using three `write_all()` calls. Because external processes can write to the same TTY concurrently, output can interleave between these calls, producing torn ANSI sequences and leaving artifacts.
### Issue Context
The TS implementation writes the entire update as a single string, reducing the interleaving window. pacquet should match that robustness when trying to clear external output below/after the frame.
### Fix Focus Areas
- pacquet/crates/default-reporter/src/lib.rs[139-152]
### Suggested fix
- Build a single buffer containing: `"\r" + diff_output + "\x1b[K\x1b[0J"` and `write_all()` once.
- Prefer reusing an allocation (e.g., store a `String`/`Vec<u8>` scratch buffer in `Sink`) if you want to avoid per-frame heap churn, but keep the terminal write atomic.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

initDefaultReporter() now always constructs an ansi-diff differ in the non-append-only path
using stream.rows/stream.columns, but the package’s runCli() wrapper calls it without
switching to append-only when stdout is not a TTY (a common case when reading NDJSON from stdin).
This can lead to interactive ANSI control output being written into redirected output and to the
differ being configured with missing terminal dimensions.

Code

pnpm11/cli/default-reporter/src/index.ts[R73-78]

+  const stream = opts.useStderr ? proc.stderr : proc.stdout
+  const write = stream.write.bind(stream)
+  const diff = createDiffer({
+    height: stream.rows,
+    width: stream.columns ?? outputMaxWidth,
+  })

Evidence

The default-reporter CLI wrapper does not follow pnpm core’s non-TTY append-only selection, yet
initDefaultReporter()’s interactive path now depends on terminal geometry (rows/columns) for
ansi-diff initialization.

pnpm11/cli/default-reporter/src/index.ts[48-78]
pnpm11/cli/default-reporter/src/cli.ts[22-28]
pnpm11/pnpm/src/main.ts[193-198]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`initDefaultReporter()` initializes `ansi-diff` with `height: stream.rows` and `width: stream.columns ?? outputMaxWidth` whenever `appendOnly` is not explicitly enabled. The `@pnpm/cli.default-reporter` package also ships a CLI (`src/cli.ts`) that feeds it log lines from stdin, but it does not force append-only when stdout is redirected/non-TTY, unlike pnpm’s main CLI.
### Issue Context
- pnpm core explicitly selects `append-only` when `!process.stdout.isTTY`, so the default reporter’s interactive mode is normally TTY-only.
- The default-reporter package’s own `runCli()` helper is a realistic path where stdin is piped, and stdout may also be piped/redirected.
### Fix Focus Areas
- pnpm11/cli/default-reporter/src/index.ts[73-78]
- pnpm11/cli/default-reporter/src/cli.ts[22-28]
### Suggested fix
Implement a non-TTY safety fallback:
1) In `src/cli.ts`, set `reportingOptions.appendOnly` based on `process.stdout.isTTY` (and/or `process.stderr.isTTY` if using stderr).
2) Additionally (defensive), in `initDefaultReporter()` treat `stream.rows`/`stream.columns` being null/undefined (or `stream.isTTY !== true`) as a signal to use append-only mode (or to supply safe defaults) so the differ is not initialized with missing terminal geometry.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The new pacquet frame differ computes wrapped-row height using visible_width, which counts 1
column per Unicode scalar instead of terminal display width (wcwidth). This can undercount
soft-wrapped physical rows for CJK/emoji text and misposition cursor moves during redraws.

Code

pacquet/crates/default-reporter/src/diff.rs[R148-160]

+        let length = visible_width(text);
+        let (height, remainder) = match width {
+            0 => (0, length),
+            term_width => {
+                let line_height = length / term_width;
+                let line_remainder = length % term_width;
+                if line_height > 0 && line_remainder == 0 {
+                    (line_height - 1, term_width)
+                } else {
+                    (line_height, line_remainder)
+                }
+            }
+        };

Evidence

Rule 4 requires redraw positioning to account for soft-wrapped physical rows. The new Line::new()
uses visible_width(text) to compute height and remainder, and the file comment states
visible_width counts 1 column per char (not wcwidth), which makes row calculations incorrect for
wide glyphs that wrap.

Row counting for redraw positioning must account for soft-wrapped lines
pacquet/crates/default-reporter/src/diff.rs[14-18]
pacquet/crates/default-reporter/src/diff.rs[148-160]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Diff` computes line `height`/`remainder` using `visible_width`, which counts 1 column per char. This miscounts soft-wrapped physical rows for wide glyphs (CJK/emoji), risking incorrect cursor positioning during interactive redraw.
## Issue Context
Compliance requires row counting to account for soft-wrapped lines accurately. The current implementation explicitly documents that wide characters are undercounted.
## Fix Focus Areas
- pacquet/crates/default-reporter/src/diff.rs[148-160]
- pacquet/crates/default-reporter/src/diff.rs[14-18]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

In pacquet's Sink::new(), frame content is formatted at width = columns.saturating_sub(2) but
Diff::new() receives the full columns value. The differ's soft-wrap height/remainder
calculations therefore use a 2-column-wider value than the actual frame content, causing incorrect
cursor positioning for lines near the terminal width.

Code

pacquet/crates/default-reporter/src/lib.rs[R106-109]

let width = if is_tty { columns.saturating_sub(2) } else { 80 };
let colors = Colors { enabled: is_tty && std::env::var_os("NO_COLOR").is_none() };
let state = ReporterState::new(cwd(), width, colors, append_only);
-        // pnpm's `throttleProgress`: 200ms in place, 1000ms append-only.
+        let diff = diff::Diff::new(columns);

Evidence

lib.rs line 106 sets width = columns.saturating_sub(2) and passes it to ReporterState::new() for
frame formatting. Line 109 passes the full columns to Diff::new(). In diff.rs, Line::new()
uses this width for height = length / term_width and remainder = length % term_width. A frame
line of exactly columns-1 visible chars has height=0 in the frame (fits within columns-2 is false,
but fits within columns-1 is true), but the differ computes height=0 with the full columns value
too—the mismatch becomes observable when a line is exactly columns-1 or columns chars wide,
causing the differ to emit wrong cursor-up/down moves.

pacquet/crates/default-reporter/src/lib.rs[106-109]
pacquet/crates/default-reporter/src/diff.rs[147-162]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Diff::new()` is initialized with `columns` but frame content is formatted at `columns - 2`. The differ's wrap math uses a wider value than the actual content, causing incorrect height/remainder for lines near the terminal edge.
## Issue Context
`Line::new()` in diff.rs computes `height = length / term_width` and `remainder = length % term_width`. These must match the width used by `ReporterState` to format the frame.
## Fix Focus Areas
- pacquet/crates/default-reporter/src/lib.rs[106-109]: Change `diff::Diff::new(columns)` to `diff::Diff::new(width)` so the differ uses the same `columns - 2` value as the frame formatter.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

In pacquet, Diff::update() clears removed trailing rows by emitting per-row "\x1b[0K\n", but the
sink already appends "\x1b[K\x1b[0J" after every update, which already clears the rest of the frame
area. This duplicates terminal output and work proportional to the number of removed rows when
frames shrink (worst-case large in tall-frame scenarios).

Code

pacquet/crates/default-reporter/src/diff.rs[R96-105]

+        if let Some(old_last) = self.lines.last() {
+            let new_last_row = next.last().map_or(0, |line| line.row + line.height);
+            let old_last_row = old_last.row + old_last.height;
+            if new_last_row < old_last_row {
+                self.move_to(&mut out, 0, new_last_row);
+                for _ in new_last_row..old_last_row {
+                    out.push_str("\x1b[0K\n");
+                    self.row += 1;
+                }
+            }

Evidence

Diff::update() emits per-row clearing when shrinking (\x1b[0K\n loop). The sink always appends
\x1b[K\x1b[0J after applying the diff, and \x1b[0J already clears everything below the cursor
(i.e., the removed trailing rows), making the loop redundant overhead.

pacquet/crates/default-reporter/src/diff.rs[96-105]
pacquet/crates/default-reporter/src/lib.rs[139-150]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Diff::update()` emits a loop of `\x1b[0K\n` sequences to clear rows when the new frame is shorter than the old frame. In this PR, the sink already appends `\x1b[K\x1b[0J` after every update (`\x1b[0J` erases from cursor to end of display), so the per-row clearing is largely redundant and increases output size and CPU/terminal work when frames shrink.
### Issue Context
The module docstring explicitly states the caller wraps output with `\r` + `\x1b[0J`, and the sink does that (and also adds `\x1b[K`). With the cursor positioned at the end of the rendered frame, `\x1b[0J` clears all removed lines below the frame.
### Fix Focus Areas
- pacquet/crates/default-reporter/src/diff.rs[96-105]
- pacquet/crates/default-reporter/src/lib.rs[139-150]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

pacquet’s try_inline_diff() computes the shared-prefix offset as a char count and uses it as a
terminal column in ESC[C/ESC[D moves, which is wrong for double-width glyphs (CJK/emoji) and can
corrupt in-place rendering. This can be triggered by non-ANSI frames (e.g. NO_COLOR) that include
user filesystem paths or other Unicode text, making the differential update place and overwrite text
at the wrong screen column.

Code

pacquet/crates/default-reporter/src/diff.rs[R181-193]

+        let a_chars: Vec<char> = self.raw.chars().collect();
+        let b_chars: Vec<char> = other.raw.chars().collect();
+        let left = a_chars.iter().zip(&b_chars).take_while(|(a, b)| a == b).count();
+        let right =
+            a_chars.iter().rev().zip(b_chars.iter().rev()).take_while(|(a, b)| a == b).count();
+        let changed_len = a_chars.len().saturating_sub(left + right);
+        if left + right <= 4 || left + changed_len >= width.saturating_sub(1) {
+            return false;
+        }
+        move_to(out, x, y, left, self.y);
+        let changed: String = a_chars[left..left + changed_len].iter().collect();
+        out.push_str(&changed);
+        *x = left + changed_len;

Evidence

try_inline_diff() derives left via .count() over zipped char iterators and passes that
directly to move_to() as the target x-coordinate (terminal column). The renderer can include
user-controlled filesystem paths (which may contain arbitrary Unicode) in frame text, and when
NO_COLOR disables ANSI, the inline-diff path is eligible and can be taken.

pacquet/crates/default-reporter/src/diff.rs[161-194]
pacquet/crates/default-reporter/src/lib.rs[61-65]
pacquet/crates/default-reporter/src/state.rs[923-931]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`try_inline_diff()` uses `left` (a character-count prefix length) as the terminal column when emitting cursor movement. Terminals move by *cells*, and for wide glyphs (e.g. CJK/emoji) character count != cell width, so the inline diff can write at the wrong column and corrupt the frame.
### Issue Context
This is new behavior introduced by the pacquet port’s inline-diff optimization. It’s most likely to show up when frames are plain-text (no ANSI), e.g. when `NO_COLOR` disables coloring, and the output contains Unicode from paths.
### Fix Focus Areas
- pacquet/crates/default-reporter/src/diff.rs[161-195]
Suggested implementation direction (pick one):
1) **Safe & simple**: only attempt inline diffs for ASCII-only lines:
- `if !self.raw.is_ascii() || !other.raw.is_ascii() { return false; }`
2) **Correct & complete**: make inline-diff cursor positioning display-width-aware:
- compute prefix/suffix boundaries in *cell widths* and slice accordingly (may require a width function that matches terminal cell behavior).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

pacquet’s TTY frame rendering now allocates an additional string each refresh by calling
format!("\r{diff_output}\x1b[0J") before writing. This adds avoidable allocation/copy overhead to
the high-frequency redraw path and can increase CPU/memory churn during large installs or frequent
progress ticks.

Code

pacquet/crates/default-reporter/src/lib.rs[R139-142]

Output::Frame(frame) => {
-                let mut buf = String::new();
-                if self.prev_rows > 0 {
-                    let _ = write!(buf, "\x1b[{}A", self.prev_rows);
-                }
-                // Reset the column to 0 (cursor-up alone keeps the column) so the
-                // redraw starts cleanly even when an external process left the
-                // cursor mid-line.
-                buf.push('\r');
-                buf.push_str("\x1b[0J");
-                buf.push_str(&frame);
-                buf.push('\n');
-                let _ = out.write_all(buf.as_bytes());
-                self.prev_rows = count_rows(&frame, self.columns);
+                let diff_output = self.diff.update(&frame);
+                let _ = out.write_all(format!("\r{diff_output}\x1b[0J").as_bytes());
}

Evidence

The Output::Frame arm constructs diff_output (already a String) and then immediately allocates
another String via format! to prepend \r and append \x1b[0J. This code is executed on every
frame refresh, which is the hot-path for interactive progress rendering.

pacquet/crates/default-reporter/src/lib.rs[115-126]
pacquet/crates/default-reporter/src/lib.rs[139-142]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Output::Frame` currently does `format!("\r{diff_output}\x1b[0J")`, creating a new owned string per refresh in addition to the already-allocated `diff_output`. This adds extra allocations and copies on the frequent redraw path.
### Issue Context
This sits directly in the stdout redraw loop for interactive TTY mode.
### Fix Focus Areas
- pacquet/crates/default-reporter/src/lib.rs[129-145]
Suggested implementation direction:
- Keep the write atomic (single `write_all`) but avoid `format!` by building into a pre-sized buffer:
- `let mut buf = String::with_capacity(1 + diff_output.len() + 4);`
- `buf.push('\r'); buf.push_str(&diff_output); buf.push_str("\x1b[0J");`
- `out.write_all(buf.as_bytes())`

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools