feat(pnpmfile): wire hooks into update; add preResolution hook by jdx · Pull Request #423 · endevco/aube

jdx · 2026-04-30T21:32:30Z

Summary

Closes three pnpmfile gaps surfaced while porting more cases from pnpm/test/install/hooks.ts (follow-up to #419):

aube update now runs the full pnpmfile cycle. Previously update built its resolver via commands::build_resolver without attaching the readPackage host, then handed off to install::run in frozen-prefer mode — which consumes the lockfile as-is, so the hook never fired on update. Now update wires preResolution + readPackage + afterAllResolved the same way install does.
aube update --ignore-pnpmfile is now accepted and honored, matching the install-side flag and pnpm.
preResolution hook is now supported. The hook receives a context object with lockfileDir, storeDir, currentLockfile, wantedLockfile, existsCurrentLockfile, existsNonEmptyWantedLockfile, and a registries map (default + scoped), matching pnpm's wire shape (camelCase). It fires before resolve in install (lockfile-only and streaming paths) and update; the spawn is skipped when the pnpmfile doesn't reference preResolution, so hook-less files don't pay the per-install node-startup cost.

Implementation factors out a run_one_shot_hook helper in crates/aube/src/pnpmfile.rs so afterAllResolved and preResolution share the same node-spawn / pipe / wait scaffolding, and a commands::run_pnpmfile_pre_resolution helper in crates/aube/src/commands/mod.rs so install and update build the context from the same sources.

Also adds three more ports from hooks.ts (test/pnpm_install_hooks.bats) — these are the originals that surfaced the divergences during the port and pass once the gaps above are closed:

readPackage during aube update (hooks.ts:263)
--ignore-pnpmfile on aube update (hooks.ts:338)
preResolution hook fires before resolve (hooks.ts:624)

Test plan

mise run test:bats test/pnpm_install_hooks.bats — 8 ok, 2 skip (the two pre-existing divergences from test: port hook tests from pnpm install/hooks.ts #419), 0 fail.
mise run test:bats test/pnpmfile.bats — 9 ok.
mise run test:bats test/update.bats — 10 ok.
mise run test:bats test/install.bats — 61 ok.
cargo test -p aube — 330 unit tests + 4 e2e, 0 failures.
cargo clippy --all-targets -- -D warnings clean.
cargo fmt --check clean.

🤖 Generated with Claude Code

Note

Medium Risk
Changes when and how .pnpmfile.* hooks execute (including spawning node) during update and early install resolution, which can affect dependency graphs and lockfile output.

Overview
aube update now runs the pnpmfile hook lifecycle (runs preResolution, attaches readPackage, then runs afterAllResolved) so hook-driven dependency mutations are reflected in the updated lockfile.

Adds support for the pnpmfile preResolution hook with a pnpm-shaped context snapshot, and factors one-shot hook execution into a shared run_one_shot_hook helper. Introduces update --ignore-pnpmfile and propagates it through the chained frozen-prefer install.

Updates CLI usage/docs and adds new Bats coverage for readPackage during update, --ignore-pnpmfile on update, and preResolution firing before resolve.

^{Reviewed by Cursor Bugbot for commit 1ad36a5. Bugbot is set up for automated code reviews on this repo. Configure here.}

greptile-apps · 2026-04-30T21:35:14Z

Greptile Summary

This PR closes three pnpmfile gaps: aube update now runs the full hook pipeline (preResolution → readPackage → afterAllResolved), --ignore-pnpmfile is accepted by update and propagated to the chained frozen install, and the new preResolution hook fires with a pnpm-compatible context snapshot before resolution. The run_one_shot_hook refactor cleanly shares node-spawn scaffolding between afterAllResolved and preResolution, and the new has_hook fast-path correctly skips spawning node for pnpmfiles that don't declare the hook.

Confidence Score: 5/5

Safe to merge; all findings are P2 quality/efficiency suggestions with no runtime impact.

No P0 or P1 findings. The three comments are P2: redundant load_npm_config call, missing has_hook guard on afterAllResolved, and unwrap_or_default() on workspace config in update. The core hook-wiring logic, chained install propagation, and bats test coverage are all correct.

crates/aube/src/commands/update.rs — workspace config loaded fresh with unwrap_or_default() rather than reusing pre-loaded config

Important Files Changed

Filename	Overview
crates/aube/src/pnpmfile.rs	Refactors one-shot hook scaffolding into `run_one_shot_hook`; adds `PreResolutionContext` struct and `run_pre_resolution`; generalises `has_hook`. New `preResolution` path correctly short-circuits when hook absent; `afterAllResolved` still lacks the same guard.
crates/aube/src/commands/mod.rs	Adds `run_pnpmfile_pre_resolution` shared helper; re-reads `.npmrc` internally even though callers have already loaded the npm config.
crates/aube/src/commands/update.rs	Wires `preResolution` / `readPackage` / `afterAllResolved` into update; adds `--ignore-pnpmfile` propagated to chained install. Workspace config loaded separately for pnpmfile detection via `unwrap_or_default()`, unlike install which reuses pre-loaded config.
crates/aube/src/commands/install/mod.rs	Calls `run_pnpmfile_pre_resolution` in both the lockfile-only and streaming paths before the resolver runs. Insertion points look correct.
test/pnpm_install_hooks.bats	Adds three new bats tests for readPackage-during-update, --ignore-pnpmfile-during-update, and preResolution hook; prior reviewer concerns addressed (lockfile guard, two-step setup).
aube.usage.kdl	Adds `--ignore-pnpmfile` flag declaration to the update command spec.
docs/cli/update.md	Documents the new `--ignore-pnpmfile` flag for `aube update`.
docs/cli/commands.json	Machine-generated CLI spec updated with `ignore-pnpmfile` entry for update command.
test/PNPM_TEST_IMPORT.md	Tracking doc updated: 8/22 hooks.ts cases now ported (was 5/22), Phase 0 marked done, remaining cases reclassified.

_{Reviews (7): Last reviewed commit: "feat(pnpmfile): wire hooks into update; ..." | Re-trigger Greptile}

github-actions · 2026-04-30T21:45:13Z

Benchmark changes

Versions:

aube: 1.5.1 -> 1.5.2
pnpm: 11.0.2 -> 11.0.3

Public ratios: warm installs vs Bun 4x -> 10x; warm installs vs pnpm 5x -> 12x.

Benchmark	aube	bun	pnpm
Fresh install (warm cache)	1021ms -> 213ms (-79%)	4134ms -> 2078ms (-50%)	4717ms -> 2543ms (-46%)
CI install (warm cache, GVS disabled)	2920ms -> 971ms (-67%)	3396ms -> 2146ms (-37%)	4864ms -> 2972ms (-39%)
CI install (cold cache, GVS disabled)	10801ms -> 4100ms (-62%)	10012ms -> 4523ms (-55%)	9722ms -> 4983ms (-49%)

cursor · 2026-04-30T22:00:07Z

+        crate::pnpmfile::run_after_all_resolved(p, &mut graph)
+            .await
+            .wrap_err("pnpmfile afterAllResolved hook failed")?;
+    }


Update's chained install doesn't propagate ignore_pnpmfile flag

Low Severity

When aube update --ignore-pnpmfile is used, the update step correctly skips pnpmfile hooks during resolution. However, the subsequent install::run call uses InstallOptions::with_mode(...), which hardcodes ignore_pnpmfile: false. If the chained install's frozen-prefer mode encounters drift and falls through to re-resolve, pnpmfile hooks would fire despite the user's explicit --ignore-pnpmfile flag, contradicting the intended behavior.

Additional Locations (1)

crates/aube/src/commands/install/mod.rs#L443-L444

^{Reviewed by Cursor Bugbot for commit c0c06ed. Configure here.}

Good catch — propagated args.ignore_pnpmfile through to the chained install::run call in 6eefd32. Frozen-prefer should normally short-circuit so this isn't observable today, but the flag now flows correctly if drift forces a re-resolve.

This comment was generated by Claude.

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 659ac9f. Configure here.}

Closes three pnpmfile gaps surfaced while porting more cases from pnpm/test/install/hooks.ts (follow-up to #419): - `aube update` now runs the full pnpmfile cycle (preResolution + readPackage + afterAllResolved). Previously it built its resolver via `commands::build_resolver` without attaching the readPackage host, then handed off to `install::run` in frozen-prefer mode — which consumes the lockfile as-is, so the hook never fired on update. - `aube update --ignore-pnpmfile` is now accepted and honored, matching the install-side flag (and pnpm). - The `preResolution` hook is now supported. The hook receives a context object with `lockfileDir`, `storeDir`, `currentLockfile`, `wantedLockfile`, `existsCurrentLockfile`, `existsNonEmptyWantedLockfile`, and a `registries` map (default + scoped), matching pnpm's shape on the wire (camelCase). It fires before resolve in install (lockfile-only and streaming paths) and update; the spawn is skipped when the pnpmfile doesn't reference `preResolution`, so hook-less files don't pay the per-install node-startup cost. Implementation factors out a `run_one_shot_hook` helper in pnpmfile.rs so afterAllResolved and preResolution share the same node-spawn / pipe / wait scaffolding, and a `commands::run_pnpmfile_pre_resolution` helper so install and update build the context from the same sources. Also un-skips the three previously-skipped tests in test/pnpm_install_hooks.bats now that the underlying gaps are closed; they were the originals that surfaced these divergences during the port. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

greptile-apps Bot reviewed Apr 30, 2026

View reviewed changes

Comment thread test/pnpm_install_hooks.bats

Comment thread test/pnpm_install_hooks.bats