feat(pnpmfile): emit ctx.log records as pnpm:hook ndjson on stdout#440
feat(pnpmfile): emit ctx.log records as pnpm:hook ndjson on stdout#440
Conversation
Greptile SummaryRoutes pnpmfile Confidence Score: 5/5Safe to merge; no P0/P1 issues found and the design correctly handles forwarder lifetime and ordering. All P0/P1 concerns raised in prior review threads have been addressed: the forwarder is no longer stored in or aborted by ReadPackageHost on drop, drain_forwarder surfaces panics via tracing::warn, and the bats grep check is properly wrapped with run + assert_success. The one remaining new observation (silently ignored I/O errors on the stderr read loop) is P2-only. No files require special attention beyond the minor P2 in spawn_stderr_forwarder. Important Files Changed
Reviews (4): Last reviewed commit: "fix(test): clear leftover _uplinks metad..." | Re-trigger Greptile |
Benchmark changesVersions:
Public ratios: warm installs vs Bun 4x -> 8x; warm installs vs pnpm 5x -> 12x.
86563b8 vs a077bcb | aube/bun/pnpm | 3 scenarios | 3 runs | 500mbit/50ms | generated by Codex. |
1adaf5b to
884b919
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 884b919. Configure here.
Routes `ctx.log(...)` calls from `readPackage` / `afterAllResolved` /
`preResolution` hooks through a stderr sentinel-tagged channel, which a
new forwarder task re-emits as `{"name":"pnpm:hook",...}` on stdout under
`--reporter=ndjson` (matching pnpm's surface) or `[pnpmfile] message` on
stderr in default mode. Both shim paths switch from `Stdio::inherit` to
`Stdio::piped` for stderr; require()-time errors and user `console.error`
output pass through unchanged. Unblocks the ndjson `pnpm:hook` ports
tracked in PNPM_TEST_IMPORT.md (hooks.ts:366, :468).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pnpm tests reference is-positive and is-negative pervasively (notably hooks.ts:217, hooks.ts:404, plus 5 misc.ts ports that previously had to substitute is-odd). Mirrors all published versions of both packages under test/registry/storage/ so future ports can use the real fixtures verbatim. Tightens the afterAllResolved ndjson port to use @pnpm.e2e/pkg-with-1-dep exactly like hooks.ts:468, removing one substitution from the new port. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
884b919 to
4828654
Compare
The committed packument carried `_uplinks.npmjs` etag + fetched
timestamp, divergent from every other fixture (which all have
`_uplinks: {}`). The PR description claimed the metadata was cleared,
but a smoke test that queried the packument through Verdaccio after the
jq cleanup re-populated `_uplinks` — Verdaccio writes that metadata back
to storage on every packument fetch, regardless of whether uplinks are
configured.
Re-clears the metadata and expands the mirror procedure docstring in
test/registry/config.yaml so the next person doesn't fall into the same
trap (tarball fetches through Verdaccio are safe, packument fetches are
not).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Summary
Routes pnpmfile
ctx.log(...)calls through a structured channel, so under--reporter=ndjsonthey appear as{\"name\":\"pnpm:hook\",\"prefix\":<root>,\"from\":<pnpmfile>,\"hook\":...,\"message\":...}records on stdout — matching pnpm's surface — while default mode still gets the friendly[pnpmfile] messageline on stderr. Unblocks the ndjsonpnpm:hookports tracked in test/PNPM_TEST_IMPORT.md:hooks.ts:366(readPackage) andhooks.ts:468(afterAllResolved). hooks.ts:404 stays parked because it additionally requires--global-pnpmfile, which is a separate feature.Also mirrors
is-positiveandis-negativefixtures (referenced pervasively across pnpm's test suite — e.g. hooks.ts:217, :404, plus 5 substitution sites inpnpm_install_misc.bats) so future ports can use the real fixtures verbatim instead of substitutingis-odd/is-even.Implementation
Both pnpmfile shims (the one-shot for
afterAllResolved/preResolutionand the long-lived host forreadPackage) now emitctx.log(...)calls as sentinel-tagged JSON lines on stderr (__AUBE_HOOK_LOG__ {\"hook\":...,\"message\":...}). A newspawn_stderr_forwardertask reads child stderr line-by-line and:name=pnpm:hook+ project root (prefix) + pnpmfile path (from), then either written to stdout as ndjson (when--reporter=ndjsonis active) or to stderr as[pnpmfile] message(default mode).console.erroroutput keep working unchanged.Both spawn paths switch from
Stdio::inherit()toStdio::piped()for stderr. The one-shot path awaits the forwarder before returning so records can't be reordered after the next install step. TheReadPackageHostaborts its forwarder on Drop.A process-wide
pnpmfile::set_ndjson_reporter(bool)is flipped bymainwhen--reporter=ndjsonis in effect; all hook entry points (run_after_all_resolved,run_pre_resolution,ReadPackageHost::spawn) gain aprefix: &Pathparameter that callers in install/mod.rs, update.rs, and commands/mod.rs wire tocwd.Tests
Two new ports in test/pnpm_install_hooks.bats, bumping hooks.ts coverage 8/22 → 10/22:
pnpmfile: readPackage ctx.log surfaces as pnpm:hook ndjson on stdout— portshooks.ts:366. Uses@pnpm.e2e/pkg-with-1-dep+@pnpm.e2e/dep-of-pkg-with-1-depdirectly (the^100.0.0constraint already serves both 100.0.0 and 100.1.0, so the hook's pin to 100.0.0 is a real downgrade without needingaddDistTag).pnpmfile: afterAllResolved ctx.log surfaces as pnpm:hook ndjson on stdout— portshooks.ts:468. Uses pnpm's@pnpm.e2e/pkg-with-1-depfixture verbatim.Both assert
hook,message, non-emptyprefix, non-emptyfrom. The readPackage variant additionally asserts the lockfile reflects the hook's actual mutation (regression guard if the hook silently no-ops).Fixtures
is-positiveversions 1.0.0, 2.0.0, 3.0.0, 3.1.0 (28 KB total)is-negativeversions 1.0.0, 1.0.1, 2.0.0, 2.0.1, 2.0.2, 2.1.0 (40 KB total)http://localhost:4873/...and uplink metadata (_distfiles,_uplinks) cleared to match the existing offline-only fixture format.The doc convention in
PNPM_TEST_IMPORT.mdis updated to call these out alongsideis-odd/is-even/is-number/semver.Test plan
cargo fmt --checkcargo clippy --all-targets -- -D warningscargo test(1221 tests across the workspace, 0 failures)mise run test:bats test/pnpm_install_hooks.bats— 12/12 (10 active + 2 documented divergence skips)mise run test:bats test/install.bats test/pnpmfile.bats— 70/70 unchangedverdaccio --config config.yaml(no uplinks) serves both packuments and tarballs foris-positiveandis-negativecorrectly.aube install --reporter=ndjsonagainst a fixture pnpmfile withctx.logcalls in both readPackage and afterAllResolved emits well-formedpnpm:hookrecords on stdout; default mode emits[pnpmfile] messageon stderr.🤖 Generated with Claude Code
Note
Medium Risk
Touches install/update dependency-resolution hook execution and rewires Node child stderr handling; mistakes could reorder/lose hook logs or affect hook process lifecycle across commands.
Overview
Adds structured forwarding of pnpmfile
ctx.logoutput: Node hook shims now write sentinel-tagged JSON to stderr, and a Rust stderr forwarder re-emits these aspnpm:hookndjson records on stdout when--reporter=ndjsonis enabled (otherwise keeps a[pnpmfile] …stderr fallback).Updates hook spawning APIs (
preResolution,afterAllResolved,readPackage) to take aprefix/cwd, pipes hook stderr instead of inheriting it, and drains per-hook forwarder tasks to keep resolve-time logs ordered ahead ofafterAllResolvedoutput. Extends tests to assert the newpnpm:hooksurface and mirrorsis-positive/is-negativefixtures for future ports.Reviewed by Cursor Bugbot for commit 86563b8. Bugbot is set up for automated code reviews on this repo. Configure here.