Skip to content

Resolve VEX npm alias copies through the core resolver and delete vex_consumed's second alias walk #856

Description

[agent] Filed by the scheduled architecture audit routine (ecosystems and formats). Register: register comment.

Kind: refactor, with one behavior fix. Source: new finding, register E62; child 1 of #855 (E40).

Problem

npm alias discovery ("a real dir whose own package.json names name@version, under another key") is written twice, and the copies have drifted:

Since #605, the resolver's own set already holds the ordinary aliases (see #851). The walk is now a second tree walk per hosted vex run whose only unique output is the drift below.

Proof by execution (a throwaway test in vex_consumed::tests, run twice on 4646693). The fixture is node_modules/Left-Pad holding left-pad@1.3.0 (an alias key differing only by case; npm accepts it as a legacy-valid name), plus a control node_modules/mm holding minimist@1.2.2:

PROBE core left-pad=[] minimist=[".../node_modules/mm"]
PROBE walk left-pad=Some([".../node_modules/Left-Pad"]) minimist=Some([".../node_modules/mm"])

On a case-sensitive file system, agent apply doesn't see the Left-Pad copy, so it reports the package not installed or patches only the plain copy, while hosted VEX does see it. The two paths disagree about which copies exist.

Symptoms

Proposed change

  • In core alias_copies, replace the case-insensitive skip with "skip the dir the direct probe already returned", compared by path (canonical where the file system folds case). A case-only alias then counts as a copy on case-sensitive file systems, and the same physical dir is still never recorded twice on Windows or macOS.
  • Delete npm_alias_copies, npm_alias_copies_reusing, real_subdirs and ALIAS_WALK_MAX_DIRS from vex_consumed.rs, along with the alias merge branch of hosted_consumed_copies. npm hosted copies are then the resolver's set, plus the identity fallback.
  • Out of scope: npm_identity_fallback*, which covers symlinked importer entries and plain --global, and the rest of Tracking: move vex_consumed's per-ecosystem consumed-copy rules from the CLI into core #855.

Size and scope

crawlers/npm_crawler.rs (about 15 lines) and commands/vex_consumed.rs (about −150 production lines; tests ported). No contract change.

Acceptance criteria

  • Regression test in core: find_by_purls over node_modules/Left-Pad (holding left-pad@1.3.0) returns that dir on Linux, and a plain dir is still reported once where the FS is case-insensitive.
  • npm_alias_copies_finds_only_alias_installs and npm_alias_copies_walks_every_workspace_members_tree are rewritten against find_manifest_package_copies_reusing with the same expected copies (@me/mm, nested dep/node_modules/deep, workspace-member aliases, --global-prefix), and pass.
  • hosted_reuses_expanded_npm_copies_and_merges_alias_variants, hosted_expands_alias_only_copies, vlt_alias_is_consumed_through_its_store_copy and the Fix agent mode skipping npm-aliased copies (#356) #738 core alias tests stay green.
  • vex_consumed.rs contains no node_modules walk.

Dependencies

Blocked by #851, which edits the same tests. Blocks nothing; it makes #852's fix single-sited.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent:triagedarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)pm:npmnpmpriority:p1refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions