GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Fix npm overrides of git deps being refused (#490) by mikolalysenko · Pull Request #491 · SocketDev/socket-patch

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Fixes #490

Summary

A dependency can declare a package from git, a URL or file:. When the project's overrides pin that package back to a registry version, npm installs the registry release. Since #345, socket-patch still treated that copy as coming from git:

hosted scan skipped it with redirect_npm_non_registry_entry_skipped, exited 0 and left the package unpatched;
vendored scan failed with vendor_lock_entry_not_rewritable;
vex refused to attest an install that was correctly patched.

All three now treat the overridden copy as the registry install it is. They patch it and attest it.

Root cause

vendor::npm_origin::npm_non_registry_entries decides which lock entries come from the registry. The hosted rewriter, the vendored backend and VEX discovery all share it. It flagged an entry whenever some dependent's raw spec for that name wasn't a registry spec, and it never looked at overrides. npm doesn't record overrides in the lock (checked with npm 10: the root "" entry has no overrides, while resolved is the registry tarball and pkga's entry keeps github:…), so the lock alone can't tell this case apart.

Changes

core vendor/npm_origin.rs: new NpmOverrides, parsed from the root package.json (overrides plus the root dependency specs, so $name references resolve). npm_non_registry_entries(lock, &overrides) drops a non-registry edge only when an override that clearly applies to it gives a registry spec. Applicable rules are:
- top-level rules;
- rules nested under the dependent or one of its physical ancestors, outermost first, with no selector or an exact-version parent selector;
- a target selector equal to the edge's own spec.
Exact parent selectors are compared under semver equality, so build metadata is ignored and [email protected]+build.1 matches an installed 1.0.0. Values may be strings, "." values or $name references. As in npm, the rule scoped to the closest ancestor wins, then the most deeply nested one. A winning *, empty or nested-only rule is a no-op that leaves the raw spec in effect (npm's edge.js). The answer is unclear, and no override clears the source check, when:
- two equally close rules disagree;
- a target selector differs from the edge's own spec;
- an enclosing selector that isn't a plain x.y.z[-pre][+build] (pkga@^1, 1.x, =1.0.0, …) may apply and its subtree has a rule for the dependency;
- a $name reference can't be resolved.
The npm hosted and vendored modes rewire git-sourced lock entries, so npm ci silently installs the unpatched git bytes while scan and VEX report success #326 behavior covers everything unclear, so a doubtful copy stays reported as UNPATCHED and is never attested.
Hosted: the engine reads package.json as advisory input when an npm candidate meets an npm lock. A symlinked or unreadable in-memory entry is left out, never refused. The in-memory selector fetches it (EXTRA_TEXT_FILES). The rewriter never writes it.
Vendored (vendor/npm_lock.rs): vendor_npm and the download-plan preflight read it once and pass it to the scan, the sibling-lock scan and the rewire.
VEX (vex/discover/npm.rs): lockfile discovery reads it quietly (no diagnostic, no recognition) next to the lock.
CHANGELOG.md: added to the existing npm hosted and vendored modes rewire git-sourced lock entries, so npm ci silently installs the unpatched git bytes while scan and VEX report success #326 entry.

The npm/pypi/gem wrappers only dispatch to the binary, so they need no change.

Tests

Issue path	Regression test	Without fix	With fix
classification	`vendor::npm_origin::tests::issue_490_*` (registry overrides of every supported shape, fresh and already-redirected locks; innermost wins; nested/scoped ancestors)	4 FAILED	ok
classification (control)	`issue_490_overrides_that_do_not_clearly_apply_keep_the_edge`	ok	ok
closest ancestor wins (Bugbot finding)	`issue_490_the_closest_ancestor_rule_wins_whatever_the_key_order`	FAILED (old depth-only tie-break)	ok
equally close rules disagree (guard)	`issue_490_equally_close_rules_that_disagree_keep_the_edge`	ok	ok
`*` / empty overrides are no-ops (review P1)	`issue_490_wildcard_and_empty_overrides_leave_the_raw_spec`	FAILED on `c657128`	ok
range selectors that may apply (review P1)	`issue_490_range_selectors_that_may_apply_keep_the_edge`	FAILED on `c657128`	ok
build-metadata selectors (follow-up review P1)	`issue_490_exact_selectors_compare_without_build_metadata`	FAILED on `fa440ae`	ok
review probes through the hosted rewriter (`*`, `pkga@^1`, `[email protected]+build.1`)	`patch::redirect::tests::issue_490_unclear_overrides_leave_a_url_dependency_unredirected`	—	ok
hosted rewriter	`patch::redirect::tests::issue_490_a_git_edge_overridden_to_the_registry_is_redirected`	FAILED	ok
hosted engine read (memory + disk; symlinked/unreadable manifest not refused)	`hosted::engine::tests::issue_490_the_root_manifest_overrides_reach_the_npm_rewriter`	n/a (new read)	ok
vendored	`vendor::npm_lock::tests::issue_490_a_git_edge_overridden_to_the_registry_is_vendored`	FAILED (`vendor_lock_entry_not_rewritable`)	ok
vex	`vex::discover::npm::tests::issue_490_a_git_edge_overridden_to_the_registry_is_attested` (hosted + vendored wiring)	FAILED (0 refs)	ok
real npm e2e	`e2e_redirect_npm_build::npm_redirect_overridden_git_dependency_installs_patched_bytes`	—	see below

The existing #326 tests (npm_non_registry_entries_are_skipped_with_loud_warning, non_registry_only_instances_refuse_and_write_nothing, entries_npm_installs_from_a_non_registry_spec_are_not_attested, the golden indexed_npm_lock_rewrite_matches_golden) still pass unchanged.

Real-npm e2e. The new test reuses the hosted capstone fixture with a real npm install of the #490 project shape. Each run checks four things:

scan --mode hosted --vex reports redirected: 1, writes the lock pin and emits one in-run VEX statement;
a fresh npm ci installs the patched bytes byte for byte;
the hash-verified vex attests them;
rollback restores the lock.

CI's npm compatibility matrix runs it end to end on npm 9.9.4, 10.9.9, 11.20.0, 12.0.0 and 12.1.0. It is N/A, not a skip (skip() fails the pinned REQUIRED legs), in two cases:

npm < 8.3, which has no overrides;
npm 9.0.0, where npm itself can't install the project and dies with Invalid comparator: github:… when an override covers a git spec.

CI on 3ef00ef. All 11 workflows pass: CI, npm, pnpm, Bun, vlt, Poetry, PDM, Pipenv, Go, Composer and the GHA audit. Bugbot found no issues on this commit. The first attempt of Poetry 1.1.15 failed one case (direct vendored, rescanIdempotent). This PR doesn't touch Poetry code, the case passed on fa440ae and on main, and the one re-run passed.

Local runs (on 3ef00ef):

cargo clippy --workspace --all-features -- -D warnings: exit 0.
370 related core tests pass (npm_origin, npm_lock, VEX npm discovery, redirect).
An earlier full cargo test --workspace --all-features --no-fail-fast passed except 12 chmod/read-only "write failure" tests, which can't fail under uid 0 in this sandbox (the same set as other branches; see Fix berry mode takeover reverting before gates (#468, #369) #470).

🤖 Generated with Claude Code

https://claude.ai/code/session_01NLJYcEVmFmVgtyWFgtLjez

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Start fix for #490

e8d8b71

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) mentioned this pull request

Oct 1, 2026

npm hosted and vendored modes refuse a registry-installed package when its dependent's git spec is replaced by an overrides entry (regression from #345) #490

Closed


          Patch npm deps overrides send to the registry

fbaea5b

When a dependency declares another package from git, a URL or file:,
and the project's package.json "overrides" pins it back to a registry
version, npm installs the registry release. socket-patch still treated
that copy as installed from git: hosted scan skipped it and exited 0
with the package unpatched, vendored scan failed, and vex refused to
attest a correctly patched install.

The check that decides which lock entries come from the registry now
reads the root package.json overrides (top-level rules, rules nested
under the dependent or its ancestors, "." values and $name refs). An
override that clearly sends the edge to a registry spec makes the copy
patchable again in hosted mode, vendored mode and vex. Anything less
clear keeps the old, cautious behavior.

Fixes #490

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) force-pushed the agent/fix-npm-origin-overrides branch from 871c293 to fbaea5b Compare

October 1, 2026 18:38

Claude (claude) added 2 commits

October 1, 2026 18:58


          Add real-npm e2e for overridden git deps

9b885bd

The hosted capstone suite gains the #490 project shape: a local
package depends on left-pad from git and the project's overrides pin
it to the registry release. The test checks that scan redirects it,
that a fresh npm ci installs the patched bytes, that vex verifies
them, and that rollback restores the lock. It skips on npm older
than 8.3, which has no overrides.

Refs #490

Assisted-by: Claude Code:claude-opus-5-5


          Don't fail old-npm legs on the overrides e2e

f170c96

npm before 8.3 has no overrides, so the #490 case doesn't exist there.
Return without calling skip(), which panics on the pinned (REQUIRED)
CI legs for npm 6 and 7.

Refs #490

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) marked this pull request as ready for review

October 1, 2026 19:03

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Treat npm 9.0's override crash as N/A in e2e

9356ac9

npm 9.0.0 can't install the #490 project at all: it dies with
"Invalid comparator: github:..." when an override covers a git spec.
The case doesn't exist on that npm, so the e2e returns instead of
failing CI's pinned 9.0.0 leg. npm 9.9 and later run it fully.

Refs #490

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 1, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

crates/socket-patch-core/src/vendor/npm_origin.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Let the closest npm override rule win

c657128

Two override rules scoped to different ancestors at the same nesting
depth tied, and the one later in key order won. npm uses the rule of
the closest ancestor, so a farther registry rule could clear an edge
that a closer git or file: rule keeps on that source. socket-patch
could then patch or attest a copy npm still installs from the spec.

Rules are now ranked by how close their ancestor is to the
dependent, then by depth. Equally ranked rules that disagree count as
unclear, so the copy stays reported as unpatched.

Refs #490

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 1, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label

Oct 1, 2026

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Copy link

Copy Markdown

Collaborator Author

Burn-down agent: ready for review at c657128 (c657128f2872ee00702ab5472c983dc28b75b8fc).

CI: all 490 check runs success or skipped on the head SHA, no failures or pending runs
Bugbot: reviewed c657128, no findings; 0 unresolved review threads
Mergeable, no conflicts
Reviewer note: Check the closest-override-wins rule in npm override classification.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

Reviewed c657128f2872ee00702ab5472c983dc28b75b8fc. Recommendation: changes needed.

P1 — Wildcard overrides can falsely report a URL dependency as patched (npm_origin.rs:169). For a dependency on the left-pad tarball URL, "overrides": {"left-pad":"*"} leaves the raw URL in effect in npm 10.9.4, but rule_value + npm_spec_is_registry clear its non-registry classification. The rewriter changes the lock with no warning, although npm ci still fetches the original URL. Preserve no-op wildcard/empty overrides as non-registry; this classification also feeds vendoring and VEX. npm's source explicitly excludes * as a replacement.
P1 — Unmodeled nested selectors can incorrectly fall back to a broader registry override (npm_origin.rs:147). A top-level "left-pad":"1.3.0" plus "pkga@^1": {"left-pad":"https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz"} is resolved by npm using the narrower URL rule for [email protected]. This code ignores ^1 and accepts the broader registry rule. Unknown potentially applicable selectors must prevent clearing the source check, or be matched using npm semantics.

Validation: 17 npm_origin tests and 10 issue_490 tests passed. Two additional review-only public-rewriter probes failed, one per finding. In real npm 10.9.4 / Node 24.21.0 fixtures, changing the lock's resolved URL to an unreachable localhost endpoint still let fresh-cache npm ci succeed by fetching the original upstream URL in both cases. The full workspace suite was not rerun.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Claude (claude) added 2 commits

October 2, 2026 14:28


          Keep npm overrides that may not apply cautious

5dff221

Two override shapes still let socket-patch treat a git or URL
dependency as a registry install while npm ci kept fetching the
original spec:

- A "*" or empty override is a no-op in npm, which keeps the
  dependent's raw spec, but socket-patch read "*" as a registry
  range and patched the entry.
- A rule nested under a range selector such as "pkga@^1" was
  ignored, so a broader top-level registry rule cleared the entry
  even though npm uses the narrower rule.

A winning "*", empty or nested-only rule now leaves the raw spec in
effect. A range selector that may apply, or a target selector other
than the edge's own spec, makes the result unclear, so the copy stays
reported as unpatched.

Refs #490

Assisted-by: Claude Code:claude-opus-5-5


          Fix clippy lint in npm override check

fa440ae

The previous commit used bool::then with a closure where then_some is
enough, which fails the CI clippy gate (-D warnings).

Refs #490

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

[agent] Both findings confirmed and fixed in 5dff221. fa440ae is a follow-up clippy fix.

Wildcard / empty overrides. As in npm's edge.js, a winning *, empty or {".": "*"} rule is now a no-op that leaves the dependent's raw URL or git spec in effect. A target rule that holds only nested rules is a no-op too. Both still shadow a farther registry rule, so the entry stays non-registry. Test: issue_490_wildcard_and_empty_overrides_leave_the_raw_spec.
Unmodelled selectors. An enclosing selector that isn't an exact version (pkga@^1, 1.x, >=1, *, 1) now marks the result unclear when its subtree has a rule for the dependency. So does a target selector other than the edge's own spec. When the result is unclear, no override clears the source check. An exact selector for another version still can't apply. Test: issue_490_range_selectors_that_may_apply_keep_the_edge.
Your rewriter probes, both shapes. Test: patch::redirect::tests::issue_490_unclear_overrides_leave_a_url_dependency_unredirected. The lock is untouched and redirect_npm_non_registry_entry_skipped is emitted.

The two classification tests fail on c657128 and pass now. Locally: 369 related core tests pass (npm_origin, npm_lock, VEX npm discovery, redirect), and cargo clippy --workspace --all-features -D warnings is clean.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 2, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

Follow-up review of fa440ae7a96a8e54d80f777fe53ff34e2d0ef2f0: changes still needed. Both original exact reproductions now pass: wildcard no-op handling and the pkga@^1 shadowing case are fixed.

P1: build-qualified ancestor selectors can still bypass the source guard (npm_origin.rs:170). Raw-string inequality does not prove two npm versions differ: ancestor [email protected] matches selector [email protected]+build.1 because npm ignores build metadata. A nested URL override under that selector can shadow a broader registry override, but this branch skips it and the rewriter reports a registry patch with no warning. Compare using npm semver matching or leave this selector uncertain.

At this exact head, the three independent public-rewriter probes produced 2 passed, 1 failed, confirming that remaining case. The same fixture was independently checked with npm 10.9.4 / Node 24.21.0: fresh-cache npm ci fetched the original URL even after the lock's resolved URL was changed to unreachable localhost. npm's Arborist also matches this selector against registry spec pkga@^1.0.0 with ".": "1.0.0" preserving the parent version.

The 20 npm_origin and 13 issue_490 tests passed on the preceding commit; the final commit only contains the clippy expression cleanup.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Match npm override selectors without build tags

3ef00ef

An override scoped to "[email protected]+build.1" applies to an installed
pkga 1.0.0, because semver ignores build metadata. socket-patch
compared the selector to the version as raw text, skipped the rule,
and let a broader registry override patch a dependency that npm ci
still fetches from its URL.

Exact-version selectors now compare without build metadata: equal
versions apply the nested rules, and different ones (prereleases
included) still can't.

Refs #490

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

[agent] Confirmed and fixed in 3ef00ef. An exact-version enclosing selector is now compared to the ancestor's version under semver equality, with build metadata stripped on both sides. [email protected]+build.1 matches an installed [email protected], so its nested rules apply. In your probe, that means the narrower URL rule wins and the entry stays non-registry. A different version, prereleases included, still can't apply. Selectors that aren't plain x.y.z[-pre][+build], such as =1.0.0 and v1.0.0, still take the cautious "may apply" path.

New test: issue_490_exact_selectors_compare_without_build_metadata. It fails with the old raw-string comparison and passes now.
issue_490_unclear_overrides_leave_a_url_dependency_unredirected now also covers the [email protected]+build.1 rewriter probe.
Locally: 370 related core tests pass, and cargo clippy --workspace --all-features -D warnings exits 0.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 2, 2026

View reviewed changes

cursor Bot left a comment

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 3ef00ef. Configure here.}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

[agent] CI note on 3ef00ef: native (ubuntu-latest, 1.1.15) in Poetry patch compatibility failed one of its 7 cases. That case is 1.1.15 direct vendored, and the failed checks are rescanIdempotent and warmInstallReplacesUpstream. The other six Poetry 1.1.15 cases pass, including populated/crlf vendored.

I don't believe this failure comes from this PR:

The diff touches only npm lock origin classification. Its one shared-engine change, reading package.json, runs only for npm candidates.
The same workflow passed on fa440ae.
The same workflow passed on main at d63ae5f, which is the base CI merges into.

I'll re-run the failed job once when the workflow finishes; GitHub refuses a re-run while it's still running. If it fails again, I'll treat it as real and investigate.

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Tanmay Singla (Tanmay182003) approved these changes

Oct 2, 2026

View reviewed changes

Mikola Lysenko (mikolalysenko) merged commit 73b17db into main

Oct 2, 2026

519 of 520 checks passed

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) deleted the agent/fix-npm-origin-overrides branch

October 2, 2026 16:20

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Copy link

Copy Markdown

Collaborator Author

Second-pass verification of 3ef00ef2f6d738e41b645e2663057fae04e37f3a: the remaining P1 is fixed. The exact-ancestor comparison now strips build metadata from both versions, so [email protected]+build.1 correctly matches [email protected] and its narrower URL override prevents a false registry-patch claim.

I reran all three original independent public-rewriter probes at this exact head: 3 passed (wildcard no-op, uncertain range, and build-qualified ancestor). No further code change was needed. This supersedes my earlier changes-needed recommendation; this PR had already merged before this verification.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request

Oct 2, 2026


          Merge main into agent/fix-yarn-berry-hosted-auth-leak

236d4f3

Main now reads the root package.json for the npm lock rewriter as
advisory input (#490/#491). Beside a berry yarn.lock the manifest is
still read strictly, since the berry pin writes it; otherwise the
advisory read applies. The berry-only memory selection rule is dropped
because main always fetches package.json.

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) mentioned this pull request

Oct 7, 2026

Perf regression: npm/hosted wall +15% (2463257a..9c43dfc9) #993

Closed

Mikola Lysenko (mikolalysenko) added a commit that referenced this pull request

Oct 7, 2026


          Fix npm hosted scan re-walking lockfiles it no longer needs (#993)

3ded39d

The npm/hosted bench drifted +15% wall / +12% CPU over a week of main
(2463257..9c43dfc). The new npm lock passes from #345, #491, #646 and
#799 each added a walk of the 3000-entry package-lock.json, on top of
two costs that scale badly with every extra walk:

- Every hosted scan ran a full second lockfile discovery after the
  rewrite (`hosted_state_from_lockfiles`) only to classify a hosted vs
  vendored takeover, even with no vendored ledger to overlap, and then
  a THIRD one inside `classify_overlap_takeover_with` when there was
  one. The classifier now discovers once, and not at all when the
  vendored ledger has no entries (the hosted common case).
- `Discovery::resolved_elsewhere` deduped with `Vec::contains` on every
  call, so recording a lock's registry entries was quadratic in its
  size. `finalize` already sorts and dedups the list and every earlier
  reader only asks whether some entry matches, so the per-call check
  is gone.

Also trimmed per-entry allocations on the new passes: the npm extractor
builds each entry's purl once (it was built for `mention` and again for
`unwired`), `resolved_is_non_registry` no longer formats a prefix per
entry, and `npm_spec_is_registry` no longer lowercases every edge spec.

Instructions retired for `scan --json --dry-run` on the bench's npm
fixture (median of 9, macOS): base 2463257 2.647G, branch head 2.755G
(+4.1%), this commit 2.353G (-11.1% vs base, -14.6% vs head). Scan
output is byte-identical to before.

Regression test: `takeover_classifier_discovers_at_most_once` counts
discoveries (test-only counter in `discover_wiring`); new unit tests pin
the rewritten registry/tarball predicates.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

Reviewers

cursor[bot]

cursor[bot] left review comments

Tanmay Singla (Tanmay182003)

Tanmay182003 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

Ready for review

Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

npm hosted and vendored modes refuse a registry-installed package when its dependent's git spec is replaced by an overrides entry (regression from #345)

Uh oh!

There was an error while loading. Please reload this page.

3 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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

LLM Description written by Claude Code:claude-opus-5-5

Summary

Root cause

Changes

Tests

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.