build: support npm and pnpm plugin installation - #23
Merged
Conversation
`npm ci` was hardcoded in the manifest's build steps, so `herdr plugin install`
failed outright on a machine with Node but no npm binary — pnpm-managed Node
installs, for instance. Nothing about herdr requires npm; the plugin declares
its own build commands, so this was ours to fix.
Both steps now dispatch through node. npm is preferred when present, because
package-lock.json pins the exact tree CI verifies; pnpm is the fallback and
re-resolves the declared ranges. `hunkdiff` is pinned to an exact version in
package.json rather than only in the lockfile, so the reviewer users actually
run is identical either way. HUNKDIFF_PACKAGE_MANAGER forces one.
Preferred rather than detected: which lockfile is committed is the only honest
signal of the verified tree. Guessing at a preference would hand some users an
untested dependency graph, and adding a `packageManager` field would make
Corepack refuse pnpm outright.
No pnpm lockfile is committed. A second lockfile drifts after every Dependabot
bump unless a workflow regenerates it, and the pin that matters does not live
in the lockfile.
CI gains a pnpm job covering install, build, tests, and one thing worth
guarding explicitly: resolveHunkLauncher hardcodes
`node_modules/hunkdiff/bin/hunk.cjs`, and pnpm's symlinked store is the layout
most likely to break it. Verified by hand that the symlink resolves, the
prebuilt platform binary is found, and hunk reports the pinned version. hunk
tries that prebuilt binary before its bundled bun runtime, so pnpm declining to
run bun's postinstall — it leaves behind a stub that errors by design — never
surfaces on a platform with a prebuilt binary.
eslint's node globals covered `{js,ts}` only, which no .mjs script had needed
until these used `process`.
Fixes #16.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI caught what local testing could not: pnpm 12 turns skipped dependency build scripts into ERR_PNPM_IGNORED_BUILDS and fails the install. pnpm 10, which is what I had locally, only warns. hunkdiff depends on bun, whose postinstall pnpm declines to run by default, so every pnpm install of this plugin on current pnpm would have failed outright. Requesting --ignore-scripts explicitly succeeds, and costs nothing: hunk runs its prebuilt platform binary, which is a plain file from an optional dependency, and reaches for its bundled bun runtime only when no prebuilt binary exists. Neither ignoredBuiltDependencies nor onlyBuiltDependencies silences it — pnpm 12 reads those from pnpm-workspace.yaml, and honours neither in a single-package repository. Verified against pnpm 12.3.4: install, build, launcher and 670 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jhochenbaum
changed the title
jhochenbaum
marked this pull request as ready for review
September 7, 2026 21:29
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
1 participant
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 #16.
Build the plugin with npm when available, falling back to pnpm.
HUNKDIFF_PACKAGE_MANAGER=npm|pnpmselects a manager explicitly.npm uses
package-lock.json; pnpm resolves the declared dependency ranges with dependency scripts disabled. Both use the pinned Hunk version and its prebuilt platform binary.Add process-level tests for manager selection, invalid input, exit codes, and paths containing spaces. CI exercises the manifest build steps with npm and pnpm 10/12 on Linux and Windows, including the bundled Hunk launcher.
Validation: 686 local tests passed, plus a clean install/build/test with npm absent from PATH. Formatting, lint, TypeScript build, workflow validation, and dependency audit passed. The combined tree with #22 merges cleanly and passes 723 tests.