CM-73389: Stop generating a lockfile for an npm workspace member - #552
Open
amitturgemancycode wants to merge 2 commits into
Open
amitturgemancycode wants to merge 2 commits into
amitturgemancycode wants to merge 2 commits into
Conversation
npm installs a workspace from the root lockfile and ignores any lockfile inside a member, so the one we generate here is uploaded and never used. Worse, generating it re-resolves the member's transitive ranges against the registry, so the scan sees versions the repository does not install. A directory only counts as the workspace root when its package.json declares a "workspaces" pattern matching the member and its lockfile lists that member. A lockfile entry on its own is not enough: npm records a file: dependency exactly like a workspace member, and a file: target is not resolved through the root lockfile, so it still needs a lockfile of its own. Covers both lockfile shapes that can describe a workspace, the array and object forms of "workspaces", and npm-shrinkwrap.json. A lockfileVersion 1 root has no "packages" map and predates workspaces, so it keeps generating as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
amitturgemancycode
requested review from
avishaiamiel and
omer-roth
as code owners
September 28, 2026 16:35
Collaborator
|
The npm fallback generated a package-lock.json for a member of a yarn, pnpm or bun workspace. The dedicated handlers only look for their lockfile in the member's own folder, and a workspace keeps it solely at the root, so nobody claimed the member and npm took it — wrong package manager, and versions re-resolved against the registry rather than the ones installed. Coverage detection moves into a shared module that all four handlers consult. npm still requires the member to appear in the root lockfile, so a stale root cannot hide it; yarn, pnpm and bun cannot be enumerated cheaply or uniformly, so for those the root lockfile's presence is the signal. pnpm declares its members in pnpm-workspace.yaml, and each package manager resolves membership only against the declaration file it actually reads — a stale workspaces array left behind by a migration does not make a pnpm member. Negated globs are honoured, so an excluded directory still gets its own lockfile instead of being silently dropped. The root lockfile is parsed once per scan rather than once per member. Only the derived member-name set is retained, not the parsed document: for a 24 MB lockfile that is 8 KB instead of 98 MB. Deciding what to skip across 200 members drops from 19.4s to 0.1s. A committed npm-shrinkwrap.json is now used instead of being regenerated, and the collected document keeps that name rather than being reported as a package-lock.json. The ancestor walk stops at the git repository root, falling back to the scanned path when there is none, so a manifest outside the scanned tree cannot suppress a project inside it. Scanning only a member folder still declines, and says so once with the root it found. bun.lockb counts as workspace coverage but is deliberately not treated as an alternative lockfile in the member's own folder: Bun restores only from a text bun.lock, so excluding it there would leave a Bun <1.2 project with no handler at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
omer-roth
reviewed
Sep 30, 2026
Comment on lines
+18
to
+20
| # These lockfiles indicate another package manager owns the project — NPM should not run. | ||
| # bun.lockb is deliberately absent: Bun only restores from a text bun.lock, so excluding it | ||
| # here would leave a Bun <1.2 project with no handler at all. |
Collaborator
There was a problem hiding this comment.
shorten comment and remove Bun reference
omer-roth
reviewed
Sep 30, 2026
Comment on lines
+17
to
+30
| MANIFEST_FILE_NAME = 'package.json' | ||
| PNPM_WORKSPACE_FILE_NAME = 'pnpm-workspace.yaml' | ||
|
|
||
| NPM_PACKAGE_MANAGER = 'npm' | ||
| YARN_PACKAGE_MANAGER = 'yarn' | ||
| PNPM_PACKAGE_MANAGER = 'pnpm' | ||
| BUN_PACKAGE_MANAGER = 'bun' | ||
| DENO_PACKAGE_MANAGER = 'deno' | ||
|
|
||
| NPM_LOCK_FILE_NAME = 'package-lock.json' | ||
| NPM_SHRINKWRAP_FILE_NAME = 'npm-shrinkwrap.json' | ||
|
|
||
| MANIFEST_DECLARED = 'manifest' | ||
| PNPM_WORKSPACE_DECLARED = 'pnpm-workspace' |
Collaborator
There was a problem hiding this comment.
check if you have these values in other files of the same module to avoid stale variables over time
omer-roth
requested changes
Sep 30, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
The CLI no longer generates a
package-lock.jsonfor an npm workspace member. npm installs a workspace from the root lockfile and ignores any lockfile inside a member, so the file we were generating was uploaded and never used.Worse than wasted work: generating it re-resolves the member's transitive ranges against the registry, so the scan sees versions the repository does not install. A committed root lockfile pinning
y18n 5.0.0/tough-cookie 2.3.0became5.0.8/2.3.4in the generated member lockfile — patched versions, hence no findings.When a member is skipped
Both conditions must hold:
package.jsondeclares aworkspacespattern matching the member, andpackage-lock.jsonornpm-shrinkwrap.json) listing the memberCondition 1 is not optional, and a lockfile entry alone is not a substitute. npm records a
file:dependency exactly like a workspace member:A
file:target is not resolved through the root lockfile, so it still needs its own. The only signal that separates the two is the root manifest'sworkspacesfield — which is also what the backend keys on, so both sides now agree by construction.Coverage
lockfileVersion2 and 3 — the only versions that can describe a workspacelockfileVersion1 has nopackagesmap and predates workspaces (npm 7), so it keeps generating as beforeworkspacesarray form and object form ({"packages": [...]})npm-shrinkwrap.json, which the backend already treats as equivalent*stops at a path separator,**does not, sopackages/*does not claimpackages/a/bTesting
30 unit tests,
ruff checkandruff format --checkclean on the pinned 0.15.20.Run against sca-benchmark, all 33 npm scenarios, comparing collected documents before and after:
npm/v3/18-combo-workspace-optionalnpm/v3/20-workspaces-object-formnpm/v3/09-peer-bundle-link-flagsandnpm/v3/10-file-directoryalso have nested manifests with no lockfile and are unchanged — they are the control, and10-file-directoryis the scenario that caught the original predicate being wrong.End to end: the resulting 3-document upload scanned against
dependency-collectoratmainyields 2 detections at the versions the committed lockfile pins.Jira
CM-73389
🤖 Generated with Claude Code