fix(build): ship & harden plugin/node_modules so hooks resolve zod/v3 (#2379)#2710
fix(build): ship & harden plugin/node_modules so hooks resolve zod/v3 (#2379)#2710praxstack wants to merge 2 commits into
Conversation
Greptile SummaryThis PR fixes the
Confidence Score: 4/5The npm stdout inheritance in The self-heal logic in
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User installs plugin] --> B{Install channel?}
B -->|npm tarball| C[plugin/node_modules shipped\nbuild-time npm install]
B -->|Marketplace git clone| D[plugin/node_modules gitignored\nnot present]
C --> E[bun-runner.js invoked by hook]
D --> E
E --> F{zod/v3 resolvable?}
F -->|Yes| G[Spawn worker-service.cjs]
F -->|No| H[ensureRuntimeDeps: npm install\n--ignore-scripts zod shell-quote]
H --> I{npm install result}
I -->|Success| G
I -->|Failure| J[Log error, spawn worker\nworker crashes]
G --> K[Hook executes normally]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[User installs plugin] --> B{Install channel?}
B -->|npm tarball| C[plugin/node_modules shipped\nbuild-time npm install]
B -->|Marketplace git clone| D[plugin/node_modules gitignored\nnot present]
C --> E[bun-runner.js invoked by hook]
D --> E
E --> F{zod/v3 resolvable?}
F -->|Yes| G[Spawn worker-service.cjs]
F -->|No| H[ensureRuntimeDeps: npm install\n--ignore-scripts zod shell-quote]
H --> I{npm install result}
I -->|Success| G
I -->|Failure| J[Log error, spawn worker\nworker crashes]
G --> K[Hook executes normally]
Reviews (3): Last reviewed commit: "fix(runtime): self-heal missing zod/v3 i..." | Re-trigger Greptile |
| "plugin/scripts/CLAUDE.md", | ||
| "plugin/skills", | ||
| "plugin/ui", | ||
| "plugin/node_modules", |
There was a problem hiding this comment.
plugin/node_modules includes platform-specific native binaries
Including plugin/node_modules in files means the published tarball will contain the tree-sitter grammar .node prebuilt binaries compiled for the platform on which npm run build was run (macOS arm64, x64, Linux, etc.). Users who install the package on a different platform will silently get non-functional grammars even though they would otherwise succeed with an on-machine install. Since the grammars are optionalDependencies, this degrades code-graph parsing rather than causing a crash, but it is worth being aware that each publish will be inadvertently platform-scoped for the optional features. Depending on the size of the grammar binaries (each can be several MB), the tarball size may also increase substantially — worth running npm pack --dry-run to verify the delta before releasing.
Prompt To Fix With AI
This is a comment left during a code review.
Path: package.json
Line: 59
Comment:
**`plugin/node_modules` includes platform-specific native binaries**
Including `plugin/node_modules` in `files` means the published tarball will contain the tree-sitter grammar `.node` prebuilt binaries compiled for the platform on which `npm run build` was run (macOS arm64, x64, Linux, etc.). Users who install the package on a different platform will silently get non-functional grammars even though they would otherwise succeed with an on-machine install. Since the grammars are `optionalDependencies`, this degrades code-graph parsing rather than causing a crash, but it is worth being aware that each publish will be inadvertently platform-scoped for the optional features. Depending on the size of the grammar binaries (each can be several MB), the tarball size may also increase substantially — worth running `npm pack --dry-run` to verify the delta before releasing.
How can I resolve this? If you propose a fix, please make it concise.|
Update — added runtime self-heal for the marketplace (git-clone) channel. While dogfooding the fix I hit the original crash again on a fresh session, and traced it to a second distribution path the build-time fix can't reach:
The marketplace copy is a git clone (confirmed New commit
Verified on macOS arm64 + Node 26: fresh plugin root with no So the PR now covers both channels: build-time bundling for npm, runtime self-heal for git-clone marketplace. Refs #2379. |
| const install = spawnSync('npm', ['install', '--no-save', '--no-audit', '--no-fund', '--ignore-scripts', 'zod@^4.3.6', 'shell-quote@^1.8.3'], { | ||
| cwd: RESOLVED_PLUGIN_ROOT, | ||
| stdio: ['ignore', 'inherit', 'inherit'], | ||
| // npm on Windows is a .cmd shim — spawn without shell hits ENOENT. | ||
| shell: IS_WINDOWS, | ||
| }); |
There was a problem hiding this comment.
npm install stdout contaminates hook's structured output
stdio: ['ignore', 'inherit', 'inherit'] passes npm's stdout straight to bun-runner.js's own stdout, which Claude Code reads as the hook's response. For UserPromptSubmit (and any hook that must return structured JSON on stdout), npm's "added N packages in Xs" summary line — which npm v9/v10 write to stdout, not stderr, even in a non-TTY pipe — would appear before the hook's JSON and cause Claude Code to fail to parse the response. The failure occurs only on first run on a marketplace install (exactly when the fix is most needed), and silently corrupts the hook decision.
Redirect index 1 of stdio to 'pipe' or 'ignore' so npm output stays off the hook's stdout channel.
Prompt To Fix With AI
This is a comment left during a code review.
Path: plugin/scripts/bun-runner.js
Line: 126-131
Comment:
**npm install stdout contaminates hook's structured output**
`stdio: ['ignore', 'inherit', 'inherit']` passes npm's stdout straight to bun-runner.js's own stdout, which Claude Code reads as the hook's response. For `UserPromptSubmit` (and any hook that must return structured JSON on stdout), npm's "added N packages in Xs" summary line — which npm v9/v10 write to **stdout**, not stderr, even in a non-TTY pipe — would appear before the hook's JSON and cause Claude Code to fail to parse the response. The failure occurs only on first run on a marketplace install (exactly when the fix is most needed), and silently corrupts the hook decision.
Redirect index 1 of stdio to `'pipe'` or `'ignore'` so npm output stays off the hook's stdout channel.
How can I resolve this? If you propose a fix, please make it concise.|
Thanks @praxstack — great catch on the grammar-abort gap. I've folded both of those into #2597 (12a4360):
One deliberate difference: I gated on The one piece I did not pull in is the Either way your writeup made the gap obvious, thanks 🙂 |
…thedotmack#2379) The bundled worker externalizes zod, shell-quote and the tree-sitter grammars, so they must exist in plugin/node_modules at runtime. On a marketplace/npm install they don't, and every hook crashes with `Cannot find module 'zod/v3'` (thedotmack#2407 / thedotmack#2453 / thedotmack#2640 / thedotmack#2637). Three changes, combining and extending the approaches in thedotmack#2531 and thedotmack#2597: 1. Build installs plugin deps (npm install --omit=dev) via spawnSync with win32 shell handling and __dirname-anchored cwd; fail-soft. 2. Root package.json `files` adds "plugin/node_modules" so the npm tarball actually ships it (npm force-strips only a top-level node_modules, not a nested one named explicitly in files — verified via `npm pack`). thedotmack#2531 adds this; thedotmack#2597 does not, so thedotmack#2597 alone leaves the tarball broken. 3. Tree-sitter grammars move to optionalDependencies, and a hard resolve gate (`createRequire(...).resolve('zod/v3')`) fails the build loudly if the install left zod missing. Why (3) matters and both prior PRs miss it: grammars are native node-gyp builds. On a Node version without a prebuilt binding (e.g. Node 26) the grammar build fails, npm exits nonzero, and as hard dependencies that aborts the whole install — zod never lands and node_modules ships empty. As optionalDependencies the failure degrades only code-graph parsing; zod and shell-quote still install and hooks still boot. The resolve gate then guarantees a broken artifact can never ship silently. Verified locally on macOS arm64 + Node 26: with grammars optional, npm install succeeds despite the grammar gyp failure and zod/v3 resolves. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…stalls The build-time install + tarball `files` entry fix the npm channel, but the MARKETPLACE channel is a `git clone` where plugin/node_modules is gitignored and never committed — so a fresh marketplace install has no node_modules and every hook crashes with `Cannot find module 'zod/v3'` (thedotmack#2407/thedotmack#2453/thedotmack#2640/thedotmack#2379). The build output can't fix this (it's gitignored), so heal at runtime. On first run, if zod/v3 doesn't resolve from the plugin root, bun-runner installs zod + shell-quote with --ignore-scripts before spawning the worker. --ignore-scripts is required: npm resolves the full dep tree from the existing package.json and the tree-sitter grammars are native node-gyp builds that abort the whole install on a Node version without a prebuilt binding (e.g. Node 26), which would leave zod uninstalled. zod/shell-quote are pure JS and need no build, so skipping scripts always recovers the hook. Idempotent: once zod/v3 resolves the heal is a no-op. Verified on macOS arm64 + Node 26: fresh root with no node_modules recovers on first run, zod/v3 + shell-quote resolve, second run does zero work. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
df172d9 to
287cdd2
Compare
|
Rebased onto the latest Conflict resolution notes, since both sides touched the same spots:
Net diff is unchanged in intent (+113/-1 across the same 3 files). Happy to adjust if you'd prefer the runtime-resolve gate folded into the Rule-A verifier instead of standing alongside it. |
Problem
The bundled
worker-service.cjsmarkszod,shell-quote, and thetree-sitter-*grammars as esbuild externals, so they must exist inplugin/node_modulesat runtime. On a marketplace / npm install they don't, and every hook crashes withCannot find module 'zod/v3'— surfacing in Claude Code as repeatingSessionStart/UserPromptSubmit hook error … node:internal/modules/cjs/loader:1478.Tracked in #2379; reported across #2407, #2453, #2637, #2640.
Reproduced on a clean v13.2.0 marketplace install (macOS arm64, Node 26). With bun's auto-install on, the missing
zod/v3external degenerates intoGET https://registry.npmjs.org/@%2f-darwin-arm64→ 404; withbun --no-installthe real causeCannot find module 'zod/v3'is unmasked.Fix
Three changes, combining and extending #2531 and #2597:
npm install --omit=dev --no-audit --no-fundviaspawnSync, withshell:trueon win32 (npm is a.cmdshim) and__dirname-anchored cwd. Fail-soft.package.jsonfilesgains"plugin/node_modules". npm force-strips only a top-levelnode_modules, not a nested one named explicitly infiles— verified vianpm pack --dry-run. (fix(packaging): bundle plugin/node_modules so marketplace install ships runtime deps (closes #2407) #2531 adds this; fix(build): populate plugin/node_modules during build (partial #2407) #2597 does not, so fix(build): populate plugin/node_modules during build (partial #2407) #2597 alone still ships an empty tarball.)optionalDependencies, and a hard resolve gate (createRequire(plugin/package.json).resolve('zod/v3')+shell-quote) fails the build loudly if the install left zod missing.Why (3) matters — the gap both prior PRs miss
Grammars are native
node-gyp/prebuild-installbuilds. On a Node version with no prebuilt binding (Node 26 here), a grammar build fails →npm installexits nonzero → as hard dependencies that aborts the entire install, so zod never lands andnode_modulesships empty. Confirmed locally: plainnpm installaborted leavingplugin/node_moduleswith 0 entries.As
optionalDependenciesthe failure degrades only code-graph parsing; zod + shell-quote still install and hooks still boot. The resolve gate then guarantees a broken artifact can never ship silently — which lines up with plan-04's "fail loudly, no false success" goal.Verification
macOS arm64, Node 26:
optionalDependencies,npm install --omit=devsucceeds despite the grammar gyp failure.createRequire(plugin/package.json).resolve('zod/v3')andresolve('shell-quote')both succeed → hooks boot.npm pack --dry-runconfirmsplugin/node_modules/...ships in the tarball.node --check scripts/build-hooks.jspasses.Relationship to existing PRs
Supersedes the partial approaches in #2531 (good
filesentry, crudeexecSync 'npm install --production') and #2597 (robust install, nofilesentry). Happy to instead fold these commits into either PR or the plan-04 branch if the maintainer prefers.Refs #2379, #2531, #2597. Closes #2407.
🤖 Generated with Claude Code
PR comparison verdict
Local workaround (until this merges)
cd <plugin cache dir>/<version> && bun add zod@^4.3.6— populatesplugin/node_modulesso hooks boot. Recurs on next plugin update unless #2710 (or equivalent) lands upstream.