Skip to content

Walk package-lock entries once for inventory, vendored, hosted and restore #663

Description

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

Kind: refactor. Source: review Part 4.4 / 4.7 E ("one package-lock walk + JsonLayout everywhere"), register E07.

Problem

Verified on 045d7ec. The serializer half of E07 is done: #357 moved all three package-lock writers onto JsonLayout. The entry walk is still written four times, each with its own copy of the identity and skip rules:

Path Walk Entry identity Skips
inventory + VEX lock_inventory/npm.rs#L77-L160 walk_npm_lock name field, else key after last node_modules/ link, inBundle/bundled; legacy depth ≤ 64
vendored npm_lock.rs#L854-L941 scan_lock_matches + entry_name, #L978-L1080 rewrite_legacy_tree same rule, own copy link, inBundle, non-registry; legacy unbounded; warns legacy aliases
hosted redirect/mod.rs#L881-L1060 package_ids (comment: "Mirrors vendor::npm_lock's entry_name"), #L1095-L1160 rewrite_npm_v2_deps same rule, own copy link, inBundle/bundled, non-registry; legacy unbounded; silent on legacy aliases
upstream restore upstream/npm.rs#L62-L138 npm_lock_hits + v2_hits same rule, own copy by hosted resolved; legacy depth ≤ 64

JSON-pointer escaping is byte-identical twice: escape_json_pointer_token (npm_lock.rs#L1077) and json_pointer_escape (upstream/npm.rs#L136). The legacy-key rule `legacy_packages_key` is already shared ([`npm_origin.rs#L341`](https://github.com/SocketDev/socket-patch/blob/045d7ec783d788bf3c5a1310724b51e09fb6505d/crates/socket-patch-core/src/vendor/npm_origin.rs#L341-L347)).``

Drift so far: recursion bounds (64 in two walks, none in two), and the legacy-alias warning exists in vendored mode only. Neither is a reachable bug today (serde_json's 128-level parse limit bounds depth), but every new rule (bundled, aliases, non-registry #326) has had to be added four times.

Symptoms

None open. #324 and #326 were each fixed in several of these walks separately.

Proposed change

  • Add one addressed walk next to walk_npm_lock: npm_lock_entries(doc) -> Vec<NpmLockEntry> with { pointer, packages_key, name, version, resolved, integrity, flags: link/in_bundle/legacy }, one depth bound, one entry_name rule and one pointer escape. Mutation stays with each caller via Value::pointer_mut(pointer).
  • Rebuild npm_lock_nodes / npm_lock_bundled_nodes on it; replace scan_lock_matches' loop, rewrite_legacy_tree's descent, package_ids + rewrite_npm_v2_deps's descent, and npm_lock_hits + v2_hits with a filter over it.
  • Delete: entry_name, escape_json_pointer_token, json_pointer_escape, v2_hits, the recursive halves of rewrite_legacy_tree / rewrite_npm_v2_deps. Per-mode warning codes and skip policies stay where they are.

Size and scope

lock_inventory/npm.rs, npm_lock.rs, redirect/mod.rs (npm section only), upstream/npm.rs. Estimated −250 / +150 production lines. Pure refactor: no warning code, wiring key, edit key or JSON pointer changes. Out of scope: a shared splice/revert engine (E24), moving the codec under formats/ (E20).

Acceptance criteria

  • One entry walk for package-lock / npm-shrinkwrap; the four callers above use it.
  • Ledger and edit keys are unchanged: existing vendored and hosted revert tests, lock_index_equivalence_tests, the redirect goldens and VEX npm discovery tests stay green without fixture edits.
  • New unit test: one lock with an alias, a scoped package, a ~// key, a nested legacy tree, link, inBundle and a git entry yields the same entries for all four callers' filters.

Dependencies

Blocked on nothing, but overlaps open PRs on these files: #660 and #589 (npm_lock.rs, lock_inventory/npm.rs), #657 and #597 (redirect/mod.rs, upstream/npm.rs). Land after those. Prepares E20 (codec into formats/npm_lock).

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