Description

The default reporter pushes every lifecycle stdio line into an in-memory Vec until the script exits,
so a dependency that prints large output can cause unbounded memory growth and potentially crash
pacquet (OOM). This is reachable during normal installs because lifecycle events are emitted per
output line and the DefaultReporter is now the default.

Code

pacquet/crates/default-reporter/src/state.rs[R736-739]

+            LifecycleMessage::Stdio { line, stdio, .. } => {
+                let formatted = self.format_indented_output(line, *stdio);
+                self.lifecycle.get_mut(key).unwrap().output.push(formatted);
+            }

Evidence

update_lifecycle_cache appends every stdio line into LifecycleEntry.output with no cap, while
lifecycle output is emitted one event per line by the executor’s spawn_line_pump, so a
large-output script drives unbounded allocations until Exit.

pacquet/crates/default-reporter/src/state.rs[709-757]
pacquet/crates/executor/src/lifecycle.rs[446-482]

Agent prompt

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

## Issue description
`ReporterState` buffers lifecycle stdio output lines unboundedly in `LifecycleEntry.output` and only collapses at render time. A chatty/malicious lifecycle script can print arbitrarily many lines, causing the reporter to allocate unbounded memory and potentially OOM/crash.
### Issue Context
Lifecycle output is produced line-by-line by `spawn_line_pump`, which emits one `LifecycleMessage::Stdio` per line. The default reporter stores every formatted line until `Exit`.
### Fix Focus Areas
- pacquet/crates/default-reporter/src/state.rs[709-757]
- pacquet/crates/executor/src/lifecycle.rs[446-482]
### Suggested fix approach
- Replace `LifecycleEntry.output: Vec<String>` with a bounded buffer (e.g., `VecDeque<String>` + `total_lines: usize`).
- Keep only the last N lines (matching the UI’s “show last 10 lines” behavior) while incrementing `total_lines` for collapsed count.
- Decide behavior for non-zero exits:
- Option A (safe): still keep last N and show “(output truncated)” rather than attempting to print everything.
- Option B (full fidelity): stream full output to a temp file while keeping last N in memory; on failure, include a pointer/path to the file (or optionally print the file content up to a limit).
- Ensure the rendering logic uses `total_lines` (not buffer length) for the collapsed line count.

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

Description

CliArgs::run uses matches!(command, ...) to compute is_install_family, which moves command
and makes the later match command { ... } a use-after-move (compile error). This blocks building
the CLI.

Code

pacquet/crates/cli/src/cli_args.rs[R207-215]

+        let started_at = now_millis();
+        let is_install_family = matches!(
+            command,
+            CliCommand::Add(_)
+                | CliCommand::Update(_)
+                | CliCommand::Remove(_)
+                | CliCommand::Install(_)
+                | CliCommand::Dlx(_),
+        );

Evidence

matches!(command, ...) is computed before match command { ... }, so command must remain
available. The current code matches by value and then later dispatches on command again.

pacquet/crates/cli/src/cli_args.rs[199-215]
pacquet/crates/cli/src/cli_args.rs[262-299]

Agent prompt

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

## Issue description
`matches!(command, ...)` consumes `command` (non-`Copy`), but `command` is used later in `match command { ... }`, causing a Rust ownership error.
### Issue Context
`CliArgs::run(self, ...)` owns `command` and later dispatches on it. `matches!` expands to a `match` over the expression, so it will move unless you match on a reference.
### Fix
Change to `matches!(&command, ...)` (or equivalent) so `command` is only borrowed.
### Fix Focus Areas
- pacquet/crates/cli/src/cli_args.rs[206-216]

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

Description

symlink_package now returns ForceSymlinkOutcome (including a warning when an existing file/dir
had to be moved aside), but the callers ignore outcome.warning, so pacquet can rename/move
occupants silently. This hides important diagnostics about surprising filesystem mutations (and
intended pnpm parity behavior).

Code

pacquet/crates/package-manager/src/symlink_direct_dependencies.rs[R439-455]

+        let outcome = symlink_package(target, &modules_dir.join(name_str)).map_err(|source| {
           SymlinkDirectDependenciesError::SymlinkPackage {
               importer_id: importer_id.to_string(),
               name: name_str.clone(),
               source,
           }
       })?;

+        // Only a freshly-created symlink is a `pnpm:root added`. A symlink
+        // already pointing at the target is "reused", and pnpm skips the
+        // event for it (`if ((await symlinkDependency(...)).reused) return`
+        // at linkDirectDeps.ts:127-129), so a `pacquet add <new>` summary
+        // lists only the new dependency, not every already-linked one.
+        if outcome.reused {
+            return Ok(());
+        }
+

Evidence

force_symlink_dir is designed to return a user-facing warning when it had to move aside a
non-symlink occupant, but the updated call sites only inspect reused (or drop the outcome
entirely), so that warning can never reach any reporter output.

pacquet/crates/fs/src/symlink_dir.rs[142-154]
pacquet/crates/fs/src/symlink_dir.rs[230-262]
pacquet/crates/package-manager/src/symlink_direct_dependencies.rs[436-455]
pacquet/crates/package-manager/src/create_symlink_layout.rs[57-79]

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_fs::force_symlink_dir` returns `ForceSymlinkOutcome { reused, warning }`, where `warning` is set when a non-symlink occupant had to be moved/removed to create the symlink. The updated `symlink_package` API now returns this outcome, but higher-level linkers drop `warning`, so users never learn that pacquet renamed something to `.ignored_*`.
### Issue Context
- `ForceSymlinkOutcome.warning` is explicitly documented as user-facing and is populated when an occupant is moved aside.
- `symlink_direct_dependencies` already has a `Reporter` generic and emits other user-facing events, so it’s the natural place to surface this.
### Fix Focus Areas
- pacquet/crates/package-manager/src/symlink_direct_dependencies.rs[439-455]
### Suggested fix
1. After `symlink_package(...)` returns `outcome`, if `outcome.warning.is_some()`, emit a warning-level reporter event (e.g. `LogEvent::Pnpm(PnpmLog { level: LogLevel::Warn, message: warning, prefix: prefix.clone() })`) so it appears in both the default reporter and `--reporter=ndjson`.
2. Keep the existing `outcome.reused` logic as-is (still skip `pnpm:root added` when reused).

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

Description

The default reporter’s ignored-scripts message instructs users to run pnpm approve-builds, but
pacquet has no such CLI subcommand/interactive flow. This produces incorrect remediation guidance in
a user-facing warning path.

Code

pacquet/crates/default-reporter/src/state.rs[R846-853]

+    fn on_ignored_scripts(&mut self, log: &IgnoredScriptsLog) {
+        if log.package_names.is_empty() {
+            return;
+        }
+        let list = log.package_names.join(", ");
+        self.push_block(format!(
+            "Ignored build scripts: {list}.\nRun \"pnpm approve-builds\" to pick which dependencies should be allowed to run scripts.",
+        ));

Evidence

The reporter hard-codes a pnpm approve-builds instruction, but the pacquet CLI does not expose an
approve-builds command and explicitly notes that the interactive approve-builds flow is not
ported.

pacquet/crates/default-reporter/src/state.rs[846-853]
pacquet/crates/cli/src/cli_args.rs[117-148]
pacquet/crates/cli/src/cli_args/dlx.rs[33-40]
pacquet/crates/config/src/lib.rs[1001-1014]

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 default reporter currently prints: `Run "pnpm approve-builds" ...` when build scripts are ignored. Pacquet does not implement `approve-builds`, so the message is not actionable.
### Issue Context
- Pacquet already models script allow/deny via config (`allow_builds` / `dangerously_allow_all_builds`).
- Pacquet CLI subcommands do not include `approve-builds`, and docs indicate the interactive prompt was not ported.
### Fix Focus Areas
- pacquet/crates/default-reporter/src/state.rs[846-854]
### Suggested fix
Update the message to a pacquet-actionable remediation, e.g.:
- Point users to configuring `pnpm-workspace.yaml` (`pnpm.allowBuilds` / `dangerouslyAllowAllBuilds`) or the relevant pacquet config mechanism.
- Avoid mentioning `pnpm approve-builds` unless pacquet actually supports it.

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

Description

The in-place TTY renderer captures terminal column width once in Sink::new and then uses that
cached value for soft-wrap row counting/cursor-up behavior, so resizing the terminal mid-command can
corrupt redraw and leave stray lines. Refresh columns/width periodically (e.g., before each frame
write) and adjust prev_rows when the width changes.

Code

pacquet/crates/default-reporter/src/lib.rs[R104-116]

+    fn new() -> Self {
+        let is_tty = std::io::stdout().is_terminal();
+        let append_only = !is_tty || FORCE_APPEND_ONLY.get().copied().unwrap_or(false);
+        let columns = if is_tty { terminal_columns().unwrap_or(80) } else { 80 };
+        // pnpm's `outputMaxWidth`: `columns - 2` on a TTY, else 80.
+        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 throttle =
+            if append_only { Duration::from_secs(1) } else { Duration::from_millis(200) };
+        Sink { state, columns, prev_rows: 0, throttle, last_write: None }
+    }

Evidence

Sink::new computes and stores a single columns value, and write_output derives prev_rows
from that cached width for cursor movement; there is no later refresh path, so terminal resizes can
desynchronize wrapping vs cursor-up math.

pacquet/crates/default-reporter/src/lib.rs[103-172]

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` caches `columns`/`width` once at startup. If the user resizes the terminal while an install is running, `count_rows` will compute cursor-up moves using stale width and the reporter will redraw incorrectly.
### Issue Context
`Sink::new` sets `columns` and `width` once; later `write_output` uses `self.columns` for `count_rows` and the `ReporterState` continues to use the original `width` for truncation/layout.
### Fix Focus Areas
- pacquet/crates/default-reporter/src/lib.rs[103-172]
### Suggested fix approach
- On each `Output::Frame` write (TTY only), re-query `terminal_columns()`.
- If the value changed:
- update `self.columns`
- update the `ReporterState` width (add a method like `state.set_width(new_width)`), and/or reconstruct `ReporterState`’s width-dependent rendering parameters
- consider resetting `prev_rows` to 0 (or force a full clear) to avoid cursor-up based on old wrap rules.
- Keep the non-TTY/append-only path unchanged.

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

Walkthrough

A new pacquet-default-reporter crate is introduced implementing a pnpm-style visual terminal reporter with ANSI color utilities, string/path formatting helpers, and a ReporterState event-folding renderer. The LogEvent enum gains an ExecutionTime variant. The CLI's default reporter changes from Silent to Default, and command dispatch is refactored around a new InstallPipeline struct. Symlink outcome tracking is refined to prevent duplicate event emission when symlinks are reused.

Changes

Default Reporter and Event Handling Refinement

Sequence Diagram(s)

sequenceDiagram
  participant main as main()
  participant cli as CliArgs::run
  participant install as InstallPipeline::run~Reporter~
  participant reporter as DefaultReporter::emit
  participant sink as global Mutex~Sink~

  main->>main: set_package_version(PACQUET_VERSION)
  main->>cli: run(ReporterType::Default)
  cli->>reporter: set_cwd(root)
  cli->>cli: started_at = now_millis()
  cli->>cli: is_install_family = (cmd == Add | Remove | Install)
  cli->>install: pipeline.run::<DefaultReporter>()
  install->>reporter: emit(LogEvent::Progress/Stats/...)
  reporter->>sink: serialize and handle
  cli->>reporter: emit(LogEvent::ExecutionTime)
  sink->>sink: render "Done in X ms using pacquet v…"

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related issues

• pnpm/pnpm#11633: Implements the visual default reporter for terminal output specified in the Rust Roadmap's Stage 1 Tier 4 item, introducing the new pacquet-default-reporter crate with state-based rendering, formatting, and color support to achieve visual parity with pnpm's install output.

Possibly related PRs

• pnpm/pnpm#12024: Both PRs modify pacquet/crates/package-manager/src/symlink_direct_dependencies.rs to refine logic around direct-dependency symlink creation and pnpm:root added event emission.

Suggested labels

area: cli/dlx

Suggested reviewers

• KSXGitHub

Poem

🐇 A brand new reporter has arrived,
With colors and frames, our terminal's revived!
Done in X ms — the footer now shows,
Through TTY or pipe, the progress bar glows.
From Silent to Default, the rabbit rejoiced,
With owo_colors bright, our logs have a voice! 🎨

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

• X
• Mastodon
• Reddit
• LinkedIn

• Introduces a new pacquet-default-reporter crate that renders pnpm-identical install frames.
• Switches the CLI default reporter from silent to default, with non-TTY append-only fallback.
• Adds pnpm:execution-time log events to drive a Done in … using pacquet v… footer.

The following are alternative approaches to this PR:

Recommendation: Given the existing Reporter trait shape (static emit) and the explicit goal of pnpm-identical UX without new dependencies, the PR’s approach—folding pnpm:* events into a mutex-guarded in-process state machine and rendering either in-place (TTY) or append-only (non-TTY)—is the most pragmatic path. The main follow-up worth considering is a longer-term refactor toward an instance-based reporter API to eliminate global state and make embedding/testability even cleaner, but that’s appropriately out of scope for landing parity output now.

cli_args.rs Make 'default' reporter the CLI default and emit execution-time footer +135/-67

Make 'default' reporter the CLI default and emit execution-time footer

• Adds a 'Default' reporter type and makes it the '--reporter' default, while keeping 'ndjson' and 'silent'. Introduces a reporter-dispatch helper ('reporter_emit'), seeds the default reporter with cwd and command start time, emits 'pnpm:execution-time' after install-family commands, and refactors install into an 'InstallPipeline' to reduce duplicated generic wiring.

pacquet/crates/cli/src/cli_args.rs

---

lib.rs Seed default reporter with pacquet version before first log event +3/-0

Seed default reporter with pacquet version before first log event

• Initializes 'pacquet-default-reporter' with the pacquet version early so fast-path installs can still render the correct 'Done in … using pacquet v…' footer.

pacquet/crates/cli/src/lib.rs

---

colors.rs Add chalk-like color palette wrapper over 'owo-colors' +51/-0

Add chalk-like color palette wrapper over 'owo-colors'

• Implements a 'Colors' helper that gates styling behind an 'enabled' flag (TTY + 'NO_COLOR' semantics). Includes a pnpm-matching '[WARN]' label style and named helpers corresponding to pnpm/chalk usage.

pacquet/crates/default-reporter/src/colors.rs

---

format.rs Port pnpm reporter formatting utilities (bytes, ms, truncation, paths) +193/-0

Port pnpm reporter formatting utilities (bytes, ms, truncation, paths)

• Adds helpers equivalent to pnpm’s JS utilities: pretty-bytes/pretty-ms formatting, visible width skipping ANSI escapes, truncation for lifecycle lines, and path normalization/relative/zooming helpers used by frame renderers.

pacquet/crates/default-reporter/src/format.rs

---

lib.rs Implement 'DefaultReporter' sink with TTY redraw + append-only fallback +143/-0

Implement 'DefaultReporter' sink with TTY redraw + append-only fallback

• Introduces the 'DefaultReporter' implementing 'pacquet_reporter::Reporter' using a process-global mutex-protected sink/state. Detects TTY and terminal width, controls ANSI cursor movement to redraw frames in place, and exposes 'set_cwd' / 'set_package_version' one-time initialization hooks.

pacquet/crates/default-reporter/src/lib.rs

---

state.rs Add event-folding state machine that renders pnpm-like frames +1056/-0

Add event-folding state machine that renders pnpm-like frames

• Implements 'ReporterState' that folds 'LogEvent's into a frame model with scrolling blocks and pinned fixed blocks (progress/footer). Renders context, live progress counts, big-tarball downloads, packages +/- bar, dependency diff summary, lifecycle script output (including collapsing rules), warning collapsing after five, and the execution-time footer.

pacquet/crates/default-reporter/src/state.rs

---

lib.rs Add 'pnpm:execution-time' log event and payload type +20/-0

Add 'pnpm:execution-time' log event and payload type

• Extends the 'LogEvent' enum with 'ExecutionTime' and adds the corresponding 'ExecutionTimeLog' struct using pnpm’s camelCase wire shape. Enables reporters to render a command-level 'Done in …' footer.

pacquet/crates/reporter/src/lib.rs

---

run.rs Update run test expectations for non-silent default reporter +4/-5

Update run test expectations for non-silent default reporter

• Adjusts documentation/comments in the test to reflect that the default reporter is now non-silent, and only '--reporter=silent' suppresses the stderr '$ <script>' echo.

pacquet/crates/cli/tests/run.rs

---

render.rs Add frame-level tests asserting pnpm-equivalent output rendering +318/-0

Add frame-level tests asserting pnpm-equivalent output rendering

• Adds targeted tests that drive sequences of 'LogEvent's through 'ReporterState' and assert exact rendered frames/lines across progress, stats bar, summary grouping, context gating, execution-time footer, warning collapse behavior, append-only mode, and lifecycle indentation.

pacquet/crates/default-reporter/tests/render.rs

---

Cargo.toml Register 'pacquet-default-reporter' as a workspace crate +1/-0

Register 'pacquet-default-reporter' as a workspace crate

• Adds the new 'pacquet-default-reporter' crate path to the workspace members/dependencies so it can be used from other crates.

Cargo.toml

---

Cargo.toml Add CLI dependency on 'pacquet-default-reporter' +1/-0

Add CLI dependency on 'pacquet-default-reporter'

• Wires the CLI crate to depend on the new default reporter so it can be selected via '--reporter' and used as the new default.

pacquet/crates/cli/Cargo.toml

---

Cargo.toml Introduce new 'pacquet-default-reporter' crate manifest +25/-0

Introduce new 'pacquet-default-reporter' crate manifest

• Defines the new crate, depends only on existing workspace deps ('pacquet-reporter', 'libc', 'owo-colors', 'serde_json'), and adds snapshot/assertion test dependencies.

pacquet/crates/default-reporter/Cargo.toml

---

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.