Skip to content

Fix open npm issues - #1008

Merged
Mikola Lysenko (mikolalysenko) merged 46 commits into
mainfrom
agent/fix-npm-open-issues
Oct 9, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 46 commits into
mainfrom
agent/fix-npm-open-issues

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes every open pm:npm issue except #933, which #934 already fixed on main. Each fix was written in its own worktree with a regression test, reviewed by a second agent, then cherry-picked here. The three refactors were done after the bug fixes, and the branch is merged with current main.

Bug fixes

Issue Fix
#898 When a vendored commit is refused (for example, a symlinked lock), the run deletes the artifact dirs it just added and marks the package failed. It no longer prints "Vendored N packages" or "Next steps". A non-pending vendor_commit_failed gets the same treatment.
#1005 When an eject is rolled back, the summary and next steps are not printed. eject_rolled_back goes to stderr, and the rolled-back packages are re-tagged skipped, with JSON summary counts kept in step.
#1004 Agent-mode scan --json and get --json now include content_mismatch_overwritten in warnings[]. The nested apply passes it up through a new ApplyRunReport.warnings.
#969 Reads npm's effective allow-file setting from env, project, user, global and builtin config, and applies npm's rule for which packages count as root. When npm would refuse the vendored file: wiring, scan warns vendor_npm_allow_file and vendor --check fails.
#954 A vendored re-scan keeps the older vendored patch, with a vendor_prebuilt_pending or vendor_prebuilt_unavailable warning, while the superseding patch is pending_build, build_failed or not_found. It no longer exits 1.
#899 On a project with only npm-shrinkwrap.json, scans warn redirect_npm_shrinkwrap_only (hosted) or vendor_npm_shrinkwrap_only (vendored). VEX does not attest from that file alone (vex_npm_shrinkwrap_only).
#879 A legacy dependencies mirror entry with no resolved but the patched integrity no longer counts against the wired packages entry. vex and vendor --check pass again after npm 7–10 re-saves the lock.
#828 A hosted pin next to a bundled copy, or next to an unpatched copy of the same package in the same lock, is kept in a new Discovery::shadowed list. VEX still doesn't attest it, but rollback, remove, list and the vendored takeover can now find and undo it.
#812 Reads replace-registry-host. Hosted scan and get warn when npm would rewrite the hosted pin to the registry and fail with E404.
#753 Lock entries under a hasShrinkwrap dependency are not rewritten. Hosted warns redirect_npm_shrinkwrapped_instance_skipped, vendor --check fails on such a copy, and VEX does not attest it.
#711 Packages that npm 12's own patchedDependencies (npm patch) already patches stay on the registry, with a warning. When such a package is already pinned, the message names the rollback.
#688 A local file: dir or workspace member with the same name@version no longer makes vendored refuse the registry copies. It is skipped with a vendor_workspace_member_skipped warning.
#554 Agent-mode and report-only scans judge each nested npm project against socket.yml include/ignore paths and the built-in test defaults.
#433 Both remove --preserve-state paths now print the hosted_state_not_preservable note and add the code to JSON warnings[].

Performance (#993)

The regression is real. Measured as instructions retired on the bench's 3000-package npm fixture:

Instructions retired
Base (2463257a) 2.647G
Regressed head 2.755G (+4.1%)
This branch 2.353G (−11% vs base)

JSON output is byte-identical across all three. The hosted path no longer runs extra lockfile discoveries when the vendor ledger has no entries, and it parses each npm lock once.

Refactors

Known residuals

Testing

Ran locally:

  • cargo test --locked --workspace: 11451 passed. 3 failed, none caused by this branch:
    • the berry test above;
    • two x86-only cargo old-toolchain tests that can't run on the arm Mac.
  • cargo clippy --workspace --all-features -D warnings: clean.

Fixes #1005
Fixes #1004
Fixes #993
Fixes #969
Fixes #954
Fixes #922
Fixes #899
Fixes #898
Fixes #879
Fixes #856
Fixes #828
Fixes #812
Fixes #753
Fixes #711
Fixes #688
Fixes #663
Fixes #554
Fixes #433
Part of #920

🤖 Generated with Claude Code


Note

Medium Risk
Changes lockfile discovery, vendoring commits, and attestation rules across npm installs; behavior is mostly fail-closed with new warnings, but mistakes could affect rollback/VEX correctness on contested or shrinkwrap-only projects.

Overview
This PR tightens npm-family behavior across hosted redirect, vendored wiring, VEX, rollback/remove, and agent scans, and documents the contracts in CLI_CONTRACT.md.

Hosted / lockfile edge cases: Skips or warns when npm's own patchedDependencies (#711), nested hasShrinkwrap installs (#753), shrinkwrap-only locks (#899), or replace-registry-host would break hosted pins (#812). Contested pins beside bundled/unreachable copies are tracked so lifecycle commands can still unwind them even when VEX won't attest (#828).

Vendored mode: Honors npm allow-file with vendor_npm_allow_file / --check failures (#969); keeps the old vendored patch when a superseding prebuild isn't ready (#954); skips workspace file: members with a warning (#688); on failed group commits, tears down artifact dirs and reports packages as failed instead of success text (#898).

Agent / policy: Nested lockfile roots respect socket.yml path policy (#554) with policy_shared_copy; agent get/scan --json now surfaces apply content_mismatch_overwritten in warnings[] (#1004). Eject rollback messaging and per-package skipped tagging are aligned (#1005, #433).

Internals: Shared npm_lock_entries parsing and npm-family vendor refactors (#663, #920, #922); hosted scan perf work (#993); VEX npm alias resolution consolidated (#856).

Reviewed by Cursor Bugbot for commit 800fa07. Configure here.


Generated by Claude Code

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This was referenced Oct 7, 2026
On a manifest-less hosted project (the default v5 shape after a bare
`scan`), `remove <purl> --preserve-state` routes through
`remove_hosted_only`, which restored the pin to upstream but never said
so: the "no preservable local state" note existed only on the
manifest-backed hosted leg, and neither path put the
`hosted_state_not_preservable` code in the JSON envelope's `warnings[]`
(only `rollback --preserve-state` did).

Share the warning between rollback and remove
(`rollback::hosted_state_not_preservable_warning`), and have both remove
paths print the `Note:` line in human mode and carry the warning in
`--json`. CLI_CONTRACT.md now lists remove as a reporter of the code.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
pdm_dir_candidates only reads unix_default on non-macOS Unix, so a
macOS build warned about an unused variable, and clippy -D warnings
failed on macOS hosts. Widen the existing Windows-only
allow(unused_variables) to macOS as well.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
npm >= 8 rewrites the origin of a lock's `resolved` URL to the
configured registry when `replace-registry-host` is `always` or equals
the URL's hostname. Hosted pins rewritten that way are fetched from
`<registry>/patch/npm/...` and every `npm ci` / `npm install` fails
E404, yet the hosted scan / get exited 0 with a success summary and
no warning (and the "already on hosted patches" re-run stayed silent).
socket-patch never read the setting.

The npm config layer walk (`resolve_outer_allow_remote`) now also
resolves `replace-registry-host` from the env var and the user /
global / builtin config files; `effective_replace_registry_host` adds
the project `.npmrc` in npm's precedence order and
`replace_registry_host_rewrites` matches a pinned host the way
@npmcli/arborist does (`always`, or the exact hostname; `npmjs` =
registry.npmjs.org). Whenever a root npm lock carries a hosted pin, the
engine emits a new `redirect_npm_replace_registry_host` warning naming
the layer that sets it and the remedies (`replace-registry-host=npmjs`
in the project .npmrc, or vendored mode). The setting is never
rewritten and the exit status is unchanged.

CLI_CONTRACT.md, docs/ecosystems.md and the npm compatibility suite
table describe the new warning.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Closing (burn-down agent): this draft has no changes. Its only commit is an empty placeholder ("Start npm open-issue sweep"), the diff against main is 0 files, and nothing has been pushed in about 2.5 hours with no agent heartbeat. The linked issues stay open and untouched. The branch is kept: reopen this PR, or open a new one, once fixes are pushed.


Generated by Claude Code

#688)

scan_lock_matches returned LockScan::WorkspaceMember as soon as it met any
`packages` key outside node_modules/ whose name@version matched the patch,
before looking at the other entries. A project with a normal registry
install of left-pad@1.3.0 plus an unrelated `file:` directory dependency
(or workspace member) whose package.json says left-pad@1.3.0 therefore had
the whole package refused with vendor_workspace_member, although the
registry copies are fully rewritable (hosted mode already pins them).

The namesake local source is now skipped like link / inBundle /
non-registry entries, with a vendor_workspace_member_skipped warning naming
it, and the refusal fires only when no rewritable instance remains. The
same scan feeds sibling-lock wiring and vendor --check's wiring audit, so
that audit now also covers the registry copies of such a project instead
of skipping the lock entirely.

The takeover half of the issue (hosted pin restored before the refusal)
was already fixed by #963.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
npm >= 12.1's native `npm patch` records the project's own diff in the
root package.json `patchedDependencies`, adds a `patched: {integrity,
path}` record to the lock entry and writes lockfileVersion 4. Every
install extracts the locked tarball and then applies that diff, failing
EPATCHFAILED when it no longer applies. The hosted npm lock rewriter knew
none of this: it pinned the entry to the hosted tarball, kept the
`patched` record and reported a clean switch, so every later `npm ci` /
`npm install` failed (or, for a non-overlapping diff, installed bytes VEX
can never attest). The vendored backend refused the v4 lock but told the
user to upgrade with npm >= 7, which cannot help.

Hosted: `rewrite_npm_lock` now leaves a dep on its registry entries in
every present npm lock when the root manifest has a `patchedDependencies`
key for `name@version` (or the bare name), or any present lock's matching
`packages` entry carries a non-null `patched` record. It warns
`redirect_npm_patched_dependency_skipped` naming the key or entry and the
remedy, marks the uuid bundled-skipped so the in-run VEX never assumes it,
and records it in a new `refused_npm_uuids` set so the hosted engine never
confirms it from a sibling lock. Other packages in the lock are still
pinned. The entry-identity derivation is factored into
`npm_lock_entry_identity` and shared.

Vendored: the lockfileVersion 4 refusal (same code,
`vendor_lockfile_version_unsupported`) now names `npm patch` /
`patchedDependencies` and the real remedies instead of the npm >= 7
upgrade advice.

CLI_CONTRACT.md and docs/testing/npm-compatibility.md document the new
warning and the v4 refusal.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

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.

…sues

Co-Authored-By: Claude <noreply@anthropic.com>
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Merged main (03b9418) to clear conflicts; head is now 800fa07. Main's #1058 moved takeover classification onto a caller-passed discovery; this PR's #993 short-circuit (no lockfile discovery when the vendored ledger is empty) is re-applied on top in both classify_overlap_takeover and scan/hosted.rs, and takeover_classifier_discovers_at_most_once was adapted to the new signature. Discovery keeps both this PR's shadowed and main's vlt_bundled_copies; hosted_inventory keeps both sides' new tests. Local: workspace check clean; core lib 5865 pass, cli 6153 pass, hosted_inventory 12/12; remaining failures are sandbox-only (root ignores read-only perms; one pypi.org network test).


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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 800fa07. Configure here.

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

Copy link
Copy Markdown
Collaborator Author

[burn-down agent] Labeled Ready for review at 800fa07b4e.


Generated by Claude Code

@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Oct 9, 2026
Merged via the queue into main with commit f3c6313 Oct 9, 2026
467 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-npm-open-issues branch October 9, 2026 03:14
Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 9, 2026
#1008 moved the pnpm vendor wiring into an NpmLockBackend impl and the
vendor run's closing lines into print_vendor_closing. Re-apply this
branch's changes on that structure instead of reverting it:

- PnpmBackend::preflight warns vendor_config_dependency_unpatched when the
  lock's env document also lists the package (#466);
- PnpmBackend::wire skips the pnpm-workspace.yaml mirror for a project
  pinned to pnpm 9.0-10.4 with no workspace file (#734), and writes the
  env-document prefix back ahead of the project lock (#466);
- print_vendor_closing passes whether pnpm-workspace.yaml exists to
  commit_hint (#734);
- engine.rs imports and CLI_CONTRACT.md keep both sides' additions.

Co-Authored-By: Claude <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 9, 2026
…-mode

apply.rs: #1008 added mismatch_overwrite_warnings (#1004) inside the block
this branch deletes with the diff download path; keep the new function
and the deletion. CLI_CONTRACT.md: keep both the "(blob" wording and
#1008's agent-mode warnings[] pointer.

Co-Authored-By: Claude <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 9, 2026
engine.rs imports: keep this branch's removal of url_host from the
guidance import list and add #1008's NPM_REPLACE_REGISTRY_HOST_CODE.

Co-Authored-By: Claude <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 9, 2026
main (#1008) added Discovery::shadowed for pins withheld over a copy
no rewire can reach, and HostedPin::all now includes them so the
management commands unwind them. Keep that as is and record the pins
withheld over a copy a re-run CAN rewire (an npm: alias, the npm twin
lock, another manager's lock, npm 6's legacy mirror, a Bun registry
entry) in a separate Discovery::rewirable, which only the rollout's
recorded view (HostedPin::recorded) reads.

Assisted-by: Claude Code:claude-opus-5-5
Mikola Lysenko (mikolalysenko) added a commit that referenced this pull request Oct 9, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 9, 2026
Resolve conflicts with #1008, which moved npm-family vendoring into a shared flow: re-apply this PR's bun additions (default-trust warning #371, non-registry tarball warnings and not-rewritable refusal #497, and bun.lockb duplicate-record folding #861) on top of the new backend structure.

Co-Authored-By: Claude <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 9, 2026
Assisted-by: Claude Code:claude-opus-5-5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hosted npm scan pins a package that npm 12's native patchedDependencies also patches, so every later npm ci / npm install fails EPATCHFAILED (and vendored refuses the lockfileVersion 4 lock with wrong advice) npm vendored refuses a registry package with vendor_workspace_member whenever a local file: directory (or workspace member) has the same name@version, and the hosted→vendored takeover then un-hosts it, leaving it unpatched Walk package-lock entries once for inventory, vendored, hosted and restore Agent-mode scan ignores socket.yml includePaths / ignorePaths (and the built-in tests/ default) for nested npm projects, patching every nested project's node_modules remove --preserve-state on a manifest-less hosted npm project silently restores the pin without the documented hosted_state_not_preservable note

3 participants