fix: remaining fixes from PR 1025 #1027
Merged
Conversation
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
- Add prebuild hook and generate-packages.mjs script to ensure template packages are generated before CLI build runs - Always copy cli.js to dist directory (required for dist/index.js to work) - Add Node.js CLI flags whitelist to prevent --help, --version, etc from being forwarded to Python CLI - Remove unused --prod flag from build script - Simplify cli.js copy to use copyFileSync directly instead of spawning node
- Add isDebug mock to npm-base.test.mts and install.test.mts to ensure --loglevel args are consistently added (isDebug() was returning true in CI due to environment variables) - Use hoisted mocks for whichRealSync in path-resolve.test.mts to ensure reliable mock behavior in CI (dynamic import pattern was failing)
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
- Handle --flag=value syntax in nodeCliFlags whitelist by extracting base flag name - Use nullish coalescing for process.exit in generate-packages.mjs to handle signal-killed processes (code is null, which coerces to 0)
The test was mocking `resolveBinPathSync` but the actual function is `resolveRealBinSync`. This caused tests to use the real implementation in CI, which resolved to the actual npm path on the runner.
The CI workflow was using `lookup-only: true` for cache restore, which only checks if the cache exists but doesn't actually restore files. This caused test jobs to skip the build step when cache existed, but the build artifacts weren't actually present on disk. Remove `lookup-only` and always run the build step. The build script already has its own caching logic and will skip unnecessary work.
Contributor
Author
|
@cursor review |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Applies the remaining fixes from the closed PR #1025 that were not related to the @socketsecurity/lib update:
prebuildhook andgenerate-packages.mjsscript to ensure template packages are generated before CLI build runscli.jsto dist directory (required fordist/index.jsto work)--help,--version, etc from being forwarded to Python CLI--prodflag from build scriptcopyFileSyncdirectly instead of spawning nodeRelated
Test plan
Note
Focuses on reliable builds and correct flag handling.
prebuildhook runningscripts/generate-packages.mjsto generate template packages beforebuildbuild/cli.jstodist/cli.jsviacopyFileSync; removes unused--prodpath and simplifies build script--help,--version,--config, etc.) and supports--flag=valueparsing when deciding Python forwardingWritten by Cursor Bugbot for commit 9521bcf. Configure here.