Skip to content

Fix npm workspace and alias identities in lockfile updates - #16490

Open
myevolve wants to merge 4 commits into
dependabot:mainfrom
myevolve:fix/npm-workspace-alias-identities
Open

myevolve wants to merge 4 commits into
dependabot:mainfrom
myevolve:fix/npm-workspace-alias-identities

Conversation

@myevolve

@myevolve myevolve commented Oct 7, 2026 •

Copy link
Copy Markdown

What are you trying to accomplish?

Addresses #16488. Keep the tracking issue open until the documented integration and hosted-rollout gates are satisfied.

Prevent npm v3 version-update jobs from treating local workspace paths as registry packages or requesting alias installation names from the registry, without ignoring dependencies or changing native npm update arguments.

  • Skip root/workspace descriptors in modern package maps; retain installed packages at root, nested, scoped and workspace-nested node_modules paths. Legacy dependency traversal remains unchanged.
  • Preserve the installation key in Dependency.name and store the real registry package in typed npm_package_name metadata. Use that identity consistently for registry, tarball, metadata, private-package and scoped credential lookups.
  • Keep combined versions paired with their registry target. Preserve distinct target/version records when installation names collide, including equal versions, and keep resolver/audit candidates within the original target.
  • Select lockfiles by installation name and canonical registry target, comparing only that target’s version. Preserve identity in the resolver’s synthetic update candidate.
  • Scope all_versions to the selected canonical package; retain mixed-target slot records in npm_package_versions. Target-specific resolution/audit fallback and installation-name blocked-version policies retain access to the full slot.
  • Copy singleton exports before metadata annotation so raw records are not mutated into self-referential metadata.
  • Add regressions and document the existing alias limitations in npm_and_yarn/README.md.

Anything you want to highlight for special attention from reviewers?

Canonicalizing Dependency.name is not sufficient: it collapses independently constrained aliases with ordinary installations, and native npm must receive the alias installation key. This change keeps the two identities separate. The common DependencySet adjustment is gated to npm identity metadata; existing version/direct-dependency precedence, unrelated metadata and ordinary case-folding behavior are retained.

This does not enable alias requirement rewriting in package.json or matching canonical security advisories to alias installation names. Dealiased dependency graph identities and native npm command arguments are unchanged. Scheduling and outbound job/PR identities remain installation-name based; this does not independently schedule every canonical target sharing a name. Native npm workspace-update limitations are also unchanged: in a workspace-only alias collision control, both plain npm and the patched checker left the alias at its installed version rather than reporting the ordinary package's version as an alias update.

The deeper native npm identity defect has a separate minimal fix in npm/cli#10090. That PR is limited to validating explicit alias targets; it is not required for the parser/registry fixes here and does not promise workspace-target isolation. The separate named-workspace-update isolation fix is npm/cli#10091. Both native branches are now based on current npm latest and passed independent and composed real-CLI checks, plus complete Arborist suites with 100% coverage. Neither native change is included in this Core PR; maintainer workflow approval, review, release, and updater adoption remain necessary.

Implementation and static reviews were AI-assisted. The runtime results below were executed locally; no managed-updater deployment is implied.

How will you know you've accomplished your goal?

The npm/common suites and native updater probes ran in ghcr.io/dependabot/dependabot-updater-npm:3507ad510e0351d2bcd8e3975e9c323aa6d00e02 with this branch's npm libraries, the changed common DependencySet file and specs mounted. The updater policy suite also passed in ghcr.io/dependabot/dependabot-updater-bundler:3507ad510e0351d2bcd8e3975e9c323aa6d00e02 with only the changed detector and updater specs mounted, without npm parser mounts:

  • 941 npm/common examples + 14 updater policy examples, 0 failures across 14 spec files, including the complete UpdateChecker suite, SubDependencyFilesFilterer and BlockedVersionDetector in addition to the original affected parser/registry/resolver/updater/graph suites.
  • 24 changed Ruby files inspected, no RuboCop offenses using the repository configuration.
  • Real FileParser → registry HTTP → UpdateChecker → FileUpdater → reparse probes for both package-lock.json and npm-shrinkwrap.json: a floating is-number alias advanced 6.0.0 → 7.0.0, while ordinary 5.0.0, pinned alias 4.0.0 and ^6 alias 6.0.0 remained unchanged. Manifest constraints were unchanged; recorded registry GETs used /is-number, not the alias keys.
  • Real mixed-target probe: root ms → npm:is-number advanced 1.0.0 → 7.0.0 while debug's ordinary nested ms stayed 2.0.0. The combined record then represented ordinary ms@2.0.0, but the resolver correctly returned is-number@7.0.0. The pre-fix common aggregation reproduced a mismatched version/registry identity.
  • Ran npm ci on all three generated outputs and loaded the installed packages in Node; asserted their exact versions and runtime behavior.
  • Offline workspace parsing excludes local workspace candidates while retaining installed aliases and their correct registry URLs.
  • A generated 972-case canonical-target/version/insertion-order matrix went from 384 lockfile-selection mismatches to 0. Another 16-case metadata probe retained all sibling records, isolated a synthetic registry advisory from unrelated versions, preserved installation-name blocking, found no metadata cycles, and preserved dependency-graph output.

No new runtime dependencies, ignore rules, direct-only filters, manifest rewrites or lockfile-format downgrades were introduced. The complete multi-ecosystem monorepo suite was not run; validation was targeted to the affected surfaces plus the native end-to-end probes above.

CI status

Hosted Updater CI at 037bc26 exposed a test-isolation mistake: the new regression imported an npm parser unavailable in the bundler-only updater image. Fixed in 5c071c0 by constructing dependency records using the suite’s existing dummy-package-manager pattern; all 14 policy examples then passed in the isolated bundler image. The real-parser integration smoke remains separately verified. Current head a923659cfbb0d0fbd9790eafe82bf1905b64f56e includes current main b4b31b9 through a history-preserving merge. All 21 required checks on this head pass, including npm_and_yarn Specs, Updater, integration, and Sorbet. The complete check rollup has finished: 149 successful, 4 skipped, 3 failed optional Smoke checks. All 21 required checks pass. Each of the three failures (npm, group-rules, group-semver) has a normalized unified-diff body exactly equal to its independent unmodified-base control: two hunks and 12 changed lines, confined to follow-redirects lockfile version/tarball/integrity. The separate mismatch is tracked in dependabot/smoke-tests#592; no expectations or ignore rules were changed. Final CI evidence.

At the initial head (8c005c7), two upstream Smoke comparisons differed only in follow-redirects lock entries: expected 1.16.0 versus actual 1.16.1 (group-rules and group-semver). Their snapshots/ignore rules were not modified to hide these differences; local results above are not a claim of an all-green remote suite.

Checklist

  • I have run the complete test suite to ensure all tests and linters pass. (Affected suites and changed-file lint passed; full monorepo suite not run.)
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

Skip local package-map records while retaining nested installations.
Keep alias installation names for native npm, and use canonical package
names at registry and metadata boundaries.

Preserve target/version pairs during dependency aggregation and avoid
updating unrelated namesakes when an alias is absent. Add regressions
and document the existing alias update limitations.

Fixes dependabot#16488
@myevolve

myevolve commented Oct 7, 2026 •

Copy link
Copy Markdown
Author

Completed CI for a923659cfbb0d0fbd9790eafe82bf1905b64f56e

All 156 checks have finished: 149 successful, 4 skipped, 3 failed optional Smoke checks. All 21 required checks pass. No queued or running checks remain. The branch includes current main b4b31b9e409bbf7665e2fb9756bee5092cefc723 without rewritten history.

Required successes include npm_and_yarn Specs, Updater, integration, and Sorbet. This is not an all-green rollup claim.

All three failures also occur without this patch

Fixture Fresh Core run Unmodified-base run Verified delta
group-semver 112654519698 112629421951 Identical diff: two hunks, 12 changed lines
group-rules 112654519603 112629422010 Identical diff: two hunks, 12 changed lines
npm 112654520801 112629420946 Identical diff: two hunks, 12 changed lines

After stripping log timestamps, each current unified-diff body exactly matches its respective independent baseline (including hunk ranges and context). The only differing expected/actual fields are embedded follow-redirects lockfile version, tarball URL, and integrity: expected1.16.0, actual1.16.1. Tracked separately in dependabot/smoke-tests#592.

The independent image build checked out the unmodified base b4b31b9 and published manifest sha256:db1779e4bb0bf43f9cd2bbbc982ca92e37a8386b81c87e62d495aaa03beb09d3, config sha256:e837ff607b45db5b3559f71b7b0d05621e2901868d51789dc4c178723f7db37c. All three independent jobs explicitly used that config digest. This isolates the observed mismatches from this patch but is not a hermetic clock-frozen replay and does not establish the underlying recording/resolver cause. No expectations, ignore rules, or coverage thresholds were changed.

Historical NuGet failure

On earlier head 5c071c0, job112633251844 failed in the targeting-pack image build with HttpIOException: ResponseEnded before the updater test ran. That was a prior-run infrastructure failure; it is not one of the three failures in the completed current rollup.

Native fixes and remaining gates

  • npm/cli#10090: three intended files, 5,225 passing assertions, three platform skips, 100% coverage.
  • npm/cli#10091: four intended files, 5,147 passing assertions, three platform skips, 100% coverage.
  • Independent and composed npm12.2.0 source-CLI install/update/ci/module checks passed. In composition, ordinary root ms stayed2.0.0, selected ms→is-number advanced1.0.0→7.0.0, and unselected ms→is-odd stayed2.0.0; manifest and ci lock bytes were unchanged.

Existing review requests already target the appropriate maintainer teams. Both native Arborist workflows remain action_required and need maintainer approval. All three PRs still require review; current credentials cannot approve upstream workflows or merge. Release/adoption, hosted rollout, and canonical-alias scheduling/advisory gaps are not claimed resolved. The tracking issue remains open.

@myevolve

myevolve commented Oct 7, 2026

Copy link
Copy Markdown
Author

Follow-up to the matching base-image smoke failures: the clock experiment proved both native cooldown replay sensitivity and a separate resolver/writer policy mismatch. The independent fix is #16492 (234 examples passed; four actual resolver/writer scenarios agreed). Full evidence and remaining replay-clock limitation: dependabot/smoke-tests#592 (comment)

This PR/branch is unchanged. No smoke expectations were relaxed or repinned; #16492 does not claim to eliminate time sensitivity in the existing recordings.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant