-
-
Notifications
You must be signed in to change notification settings - Fork 7.3k
fix(build): ship & harden plugin/node_modules so hooks resolve zod/v3 (#2379) #2710
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import { existsSync, readFileSync, mkdirSync, appendFileSync, writeFileSync } fr | |
| import { join, dirname, resolve } from 'path'; | ||
| import { homedir } from 'os'; | ||
| import { fileURLToPath } from 'url'; | ||
| import { createRequire } from 'module'; | ||
|
|
||
| const IS_WINDOWS = process.platform === 'win32'; | ||
|
|
||
|
|
@@ -101,6 +102,53 @@ if (!bunPath) { | |
| process.exit(1); | ||
| } | ||
|
|
||
| // Runtime self-heal: ensure the worker's externalized deps are present in | ||
| // plugin/node_modules before we spawn it. The build-time install + tarball | ||
| // bundling (build-hooks.js + package.json `files`) covers the npm channel, | ||
| // but the MARKETPLACE channel is a `git clone` of this repo where | ||
| // `plugin/node_modules` is gitignored and never committed — so a freshly | ||
| // installed marketplace plugin has no node_modules and every hook crashes | ||
| // with `Cannot find module 'zod/v3'` (issues #2407 / #2453 / #2640 / #2379). | ||
| // We can't fix that at build time (the install output is gitignored), so we | ||
| // heal once here, on first run, before the worker is invoked. | ||
| function ensureRuntimeDeps() { | ||
| let pkgJsonPath; | ||
| try { | ||
| pkgJsonPath = join(RESOLVED_PLUGIN_ROOT, 'package.json'); | ||
| if (!existsSync(pkgJsonPath)) return; // not a plugin root with deps | ||
| const pluginRequire = createRequire(pkgJsonPath); | ||
| pluginRequire.resolve('zod/v3'); // resolves → deps present, nothing to do | ||
| return; | ||
| } catch { | ||
| // zod/v3 unresolvable → install the hook-critical deps once. | ||
| // We install ONLY zod + shell-quote (the pure-JS externals the worker | ||
| // needs to boot), with --ignore-scripts. Rationale: npm resolves the full | ||
| // dep tree from the existing package.json, and the tree-sitter grammars | ||
| // are native node-gyp builds — on a Node version without a prebuilt | ||
| // binding (e.g. Node 26) a grammar build fails and aborts the whole | ||
| // install, leaving zod uninstalled. --ignore-scripts skips those native | ||
| // postinstalls (zod/shell-quote are pure JS and need none), so the hook | ||
| // always recovers. Grammar/code-graph deps heal separately via the full | ||
| // `npx claude-mem install` and are not required for hooks to run. | ||
| console.error('[bun-runner] plugin/node_modules missing zod — installing hook-critical deps (first run on this install)...'); | ||
| 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, | ||
| }); | ||
|
Comment on lines
+134
to
+139
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Redirect index 1 of stdio to Prompt To Fix With AIThis 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. |
||
| if (install.error) { | ||
| console.error(`[bun-runner] could not run npm install in ${RESOLVED_PLUGIN_ROOT}: ${install.error.message}`); | ||
| } else if (install.status === 0) { | ||
| console.error('[bun-runner] runtime deps installed.'); | ||
| } else { | ||
| console.error(`[bun-runner] npm install exited with code ${install.status}. Run \`cd ${RESOLVED_PLUGIN_ROOT} && npm install\` manually.`); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| ensureRuntimeDeps(); | ||
|
|
||
| function collectStdin() { | ||
| return new Promise((resolve) => { | ||
| if (process.stdin.isTTY) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
plugin/node_modulesincludes platform-specific native binariesIncluding
plugin/node_modulesinfilesmeans the published tarball will contain the tree-sitter grammar.nodeprebuilt binaries compiled for the platform on whichnpm run buildwas 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 areoptionalDependencies, 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 runningnpm pack --dry-runto verify the delta before releasing.Prompt To Fix With AI