fix: use package-lock/shrinkwrap when calling homebrew/npm MONGOSH-2485#2694
fix: use package-lock/shrinkwrap when calling homebrew/npm MONGOSH-2485#2694tculig wants to merge 14 commits intomongodb-js:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the build/release publishing pipeline to generate and use npm-shrinkwrap.json (lockfile) during npm/Homebrew publishing so downstream consumers install locked dependency versions (per MONGOSH-2485).
Changes:
- Make npm publishing paths properly
awaitthe publish step to avoid racing subsequent steps. - Add shrinkwrap generation for the release packages as part of
PackagePublisher.publishToNpm(). - Ignore
npm-shrinkwrap.jsonin git to keep the repo clean during release automation.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/build/src/release.ts | Await auxiliary publish so the release command doesn’t continue early. |
| packages/build/src/publish-mongosh.ts | Await npm publish before proceeding to Homebrew publish. |
| packages/build/src/publish-auxiliary.ts | Make auxiliary publish async and await npm publish. |
| packages/build/src/npm-packages/publish.ts | Inject shrinkwrap generation into the npm publish flow and make it async. |
| packages/build/src/npm-packages/publish.spec.ts | Update publish test to await async publish; add shrinkwrap stub wiring. |
| packages/build/src/npm-packages/index.ts | Re-export shrinkwrap helpers from the npm-packages module. |
| packages/build/src/npm-packages/generate-shrinkwrap.ts | New helper to generate npm-shrinkwrap.json via temp dir + npm commands. |
| .gitignore | Ignore npm-shrinkwrap.json artifacts created during publish. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
addaleax
left a comment
addaleax
left a comment
There was a problem hiding this comment.
The approach to generating the shrinkwrap file looks good 👍
I do want to mention – it would probably be a bit nicer to have this as part of the "Update automatically generated files" GHA workflow, since that would mean having an up-to-date version of the file checked into the repo.
Doing this during the publishing phase also works, but we try to avoid adding new steps there unless necessary, since that means that the only way through which we can properly test the feature is through actually publishing.
There was a problem hiding this comment.
Pull request overview
Adds automation to generate npm-shrinkwrap.json for the release packages from the monorepo root package-lock.json, and wires it into the scheduled “auto-generated files” workflow so shrinkwraps stay up to date.
Changes:
- Introduces a shrinkwrap generator that prunes the root lockfile down to production deps for
packages/mongoshandpackages/cli-repl. - Adds a runnable script (
npm run update-shrinkwrap) to generate shrinkwraps from the repo root. - Updates the cron workflow to run the generator and commit the resulting shrinkwrap files.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/update-shrinkwrap.ts | New entrypoint script to generate shrinkwraps for release packages. |
| packages/build/src/npm-packages/index.ts | Re-exports shrinkwrap generator APIs from the npm-packages module. |
| packages/build/src/npm-packages/generate-shrinkwrap.ts | Implements shrinkwrap generation by pruning the root lockfile and post-processing workspace entries. |
| package.json | Adds update-shrinkwrap npm script using ts-node. |
| .github/workflows/cron-tasks.yml | Runs update-shrinkwrap during scheduled auto-update workflow and stages generated files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| spawnSyncStub = sinon.stub(spawnSyncModule, 'spawnSync'); | ||
| fsReadFile.onFirstCall().resolves(JSON.stringify(mockRootLockfile)); | ||
| fsReadFile.onSecondCall().resolves(JSON.stringify(mockPackageJson)); | ||
| fsReadFile.onThirdCall().resolves(JSON.stringify(mockPrunedLockfile)); |
There was a problem hiding this comment.
I'm a bit lost here, if reading the file is mocked, what is this checking?
There was a problem hiding this comment.
Claude says: "The tests are verifying the post-processing logic — dev entry removal, workspace link conversion, workspace path filtering — not the file I/O itself. By mocking the inputs we can test that transformation logic in isolation without touching the filesystem or running npm."
But I agree with you that these are not super useful.
I'll attempt to replace this with an integration test that actually calls npm...
There was a problem hiding this comment.
feel free to give it a go but if it ends up complex, it's fine to leave it as is
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Ticket: https://jira.mongodb.org/browse/MONGOSH-2485