Skip to content

fix: skip postinstall/prepare scripts when installed as npm package#3

Open
simongdavies wants to merge 1 commit intohyperlight-dev:mainfrom
simongdavies:fix-npm-postinstall
Open

fix: skip postinstall/prepare scripts when installed as npm package#3
simongdavies wants to merge 1 commit intohyperlight-dev:mainfrom
simongdavies:fix-npm-postinstall

Conversation

@simongdavies
Copy link
Contributor

The postinstall and prepare hooks reference scripts in scripts/ which are not included in the published npm package (files: ['dist/'] only). This caused 'npm install -g @hyperlight-dev/hyperagent' to fail with MODULE_NOT_FOUND.

Use '[ ! -f script ] || command' pattern so hooks skip gracefully when scripts don't exist (published package) but still fail loudly when they do exist and error (dev context).

The postinstall and prepare hooks reference scripts in scripts/ which
are not included in the published npm package (files: ['dist/'] only).
This caused 'npm install -g @hyperlight-dev/hyperagent' to fail with
MODULE_NOT_FOUND.

Use '[ ! -f script ] || command' pattern so hooks skip gracefully when
scripts don't exist (published package) but still fail loudly when they
do exist and error (dev context).

Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 23, 2026 12:05
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjust npm lifecycle hooks so global installs from the published package (which ships only dist/) don’t fail when scripts/* is absent, while preserving the existing behavior in a source/dev checkout.

Changes:

  • Update prepare to no-op when scripts/build-modules.js is not present.
  • Update postinstall to no-op when scripts/patch-vscode-jsonrpc.js is not present, otherwise run the existing patch + runtime check.

Comment on lines +28 to +29
"prepare": "[ ! -f scripts/build-modules.js ] || npm run build:modules",
"postinstall": "[ ! -f scripts/patch-vscode-jsonrpc.js ] || (node scripts/patch-vscode-jsonrpc.js && node scripts/check-native-runtime.js)"
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new prepare/postinstall guards use POSIX [ ... ] tests and subshell syntax, which will fail (or behave incorrectly) when npm runs scripts under cmd.exe on Windows. If Windows installs are expected now or in the future, consider switching to a Node-based existence check (e.g., a tiny JS entrypoint in dist/ that checks fs.existsSync(...) before running the real script) so the hooks are cross-platform while still skipping cleanly in the published tarball.

Suggested change
"prepare": "[ ! -f scripts/build-modules.js ] || npm run build:modules",
"postinstall": "[ ! -f scripts/patch-vscode-jsonrpc.js ] || (node scripts/patch-vscode-jsonrpc.js && node scripts/check-native-runtime.js)"
"prepare": "node -e \"const fs = require('fs'); const { spawnSync } = require('child_process'); if (!fs.existsSync('scripts/build-modules.js')) process.exit(0); const npmCmd = process.platform === 'win32' ? 'npm.cmd' : 'npm'; const result = spawnSync(npmCmd, ['run', 'build:modules'], { stdio: 'inherit' }); process.exit(result.status == null ? 0 : result.status);\"",
"postinstall": "node -e \"const fs = require('fs'); const { spawnSync } = require('child_process'); if (!fs.existsSync('scripts/patch-vscode-jsonrpc.js')) process.exit(0); const nodePath = process.execPath; let r = spawnSync(nodePath, ['scripts/patch-vscode-jsonrpc.js'], { stdio: 'inherit' }); if (r.status) process.exit(r.status); r = spawnSync(nodePath, ['scripts/check-native-runtime.js'], { stdio: 'inherit' }); process.exit(r.status == null ? 0 : r.status);\""

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants