[Mikola Lysenko (mikolalysenko)]

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

Fixes #1576

Summary

Vendoring an npm project whose package-lock.json is lockfileVersion: 4 (written by npm >= 12.1 for its native npm patch) is refused with vendor_lockfile_version_unsupported. The refusal told users to remove patchedDependencies and "regenerate the lock with npm install". npm keeps an existing v4 lock at v4 on a plain install, so following the advice refused again with the same text, forever.

Now:

• The remedy names npm install --lockfile-version 3, the step that rewrites the lock as v3 and clears the refusal (as the issue's matrix shows for npm 12.1 and 12.2).
• A v4 lock with no native patch left (no patchedDependencies key in package.json, no non-null patched record in the lock) gets a detail that names only that downgrade, instead of claiming vendoring "would drop or break that patch".
• "Still has a native patch" is read the way hosted mode's 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) #711 gate reads it (bun_lock_text::patched_dependency_keys on package.json, plus non-null patched records), so the two modes agree.
• docs/testing/npm-compatibility.md and CLI_CONTRACT.md describe the new remedy.

The refusal code is unchanged; only the detail text differs.

Root cause

lock_version_gate (crates/socket-patch-core/src/vendor/npm_lock.rs) gave every v4 lock the same remedy, and that remedy (npm install) cannot change the lock version. The comment right above the text already said so.

Tests

Issue	Test	Without fix	With fix
#1576 (remedy loops, native patch in package.json only)	vendor::npm_lock::tests::lockfile_v4_manifest_patch_remedy_downgrades_the_lock	FAILED	ok
#1576 (v4 lock with no native patch left)	vendor::npm_lock::tests::lockfile_v4_without_native_patch_names_only_the_downgrade	FAILED	ok
Bugbot: FIFO at package.json must not hang the probe	vendor::npm_lock::tests::native_patch_probe_does_not_block_on_a_fifo_manifest	FAILED (5 s wedge)	ok
#711 regression, now also checks the remedy	vendor::npm_lock::tests::lockfile_v4_refusal_names_npm_patch		ok

Commands run locally:

• cargo fmt --all -- --check: clean
• cargo clippy --workspace --all-features -- -D warnings: clean
• cargo test -p socket-patch-core --all-features --lib lockfile_v4: 3 passed. With only the tests applied on main's gate, 2 failed.
• cargo test --workspace --all-features --no-fail-fast: everything that ran passed except 8 tests that fail only because this sandbox runs as root (permission-denial fixtures such as copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry) or can't reach patches-api.socket.dev (e2e_bun_lockb). None of them touch npm_lock.rs. The run hit its 10-minute cap before the last CLI suites; CI covers them.
• No ecosystem e2e asserts this detail text, and the npm/pypi/gem wrappers don't touch it.

Code review notes

Bugbot (on b3eaff4) flagged the plain std::fs::read_to_string of package.json as able to block on a FIFO; fixed in 2b1c49e by reading through utils::fs::read_regular_to_string_sync.

/code-review high ran on the diff. Fixed: package.json patchedDependencies now counts, "patched": null doesn't, CLI_CONTRACT updated, duplicate test folded into the #711 one. Not changed:

• A v4 npm-shrinkwrap.json: npm >= 12 never reads or writes the shrinkwrap, so npm 12 can't produce one in v4. It would take a hand-renamed file, which is too unlikely to be worth its own branch.
• The empty "Start fix" commit is the routine's claim marker; it goes away in the squash merge.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Unguarded package.json read can hang
  • Replaced bare std::fs::read_to_string with read_regular_to_string_sync to prevent FIFO/device blocking in has_native_patch.

Or push these changes by commenting:

@cursor push 73613062ab

Preview (73613062ab)

diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs
--- a/crates/socket-patch-core/src/vendor/npm_lock.rs
+++ b/crates/socket-patch-core/src/vendor/npm_lock.rs
@@ -22,7 +22,7 @@
 use crate::constants::SOCKET_DIR;
 use crate::manifest::schema::PatchRecord;
 use crate::patch::apply::PatchSources;
-use crate::utils::fs::{atomic_write_bytes_preserving_mode, read_regular_to_bytes};
+use crate::utils::fs::{atomic_write_bytes_preserving_mode, read_regular_to_bytes, read_regular_to_string_sync};
 use crate::utils::socket_dir::remove_tree_and_prune;
 
 use super::common::{parse_json_manifest, refused, JsonLayout};
@@ -487,7 +487,7 @@
 /// `patchedDependencies` key, or a non-null `patched` record on a lock
 /// entry. Only the refusal path calls it, so the blocking read is rare.
 fn has_native_patch(lock: &Value, project_root: &Path) -> bool {
-    let manifest = std::fs::read_to_string(project_root.join("package.json")).ok();
+    let manifest = read_regular_to_string_sync(&project_root.join("package.json")).ok();
     if !super::bun_lock_text::patched_dependency_keys(manifest.as_deref(), None).is_empty() {
         return true;
     }

_{You can send follow-ups to the cloud agent here.}

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

✅ 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 2b1c49e. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Labeled Ready for review by the burn-down agent.

• Head: 2b1c49e92
• CI: 54/54 checks passed or skipped (35 success, 19 skipped); mergeable, no conflicts
• Bugbot: reviewed 2b1c49e and found no new issues; the earlier thread on b3eaff4 is resolved and outdated
• Reviewers should look at the new lockfileVersion 4 refusal advice in crates/socket-patch-core/src/vendor/npm_lock.rs and its wording in CLI_CONTRACT.md / docs/testing/npm-compatibility.md.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief (for the human approver)

What it does
Vendoring an npm project with a lockfileVersion: 4 lock is refused, and the refusal told users to "regenerate the lock with npm install", which keeps the lock at v4, so the advice looped (#1576). The refusal now names npm install --lockfile-version 3, and a v4 lock with no native npm patch left gets a detail that names only that downgrade. The refusal code is unchanged; only the detail text differs.

Risk: low. It only changes the detail text on a path that already refuses. The one new I/O is a read of the root package.json on that refusal path, done through the regular-file reader so a FIFO can't block it (covered by a test).

Look here

• npm_lock.rs:454-470: the two v4 details
• npm_lock.rs:490-503: has_native_patch, which reads the native patch the same way hosted mode's 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) #711 gate does
• npm_lock.rs:2963, :3000, :3031: the new tests
• CLI_CONTRACT.md:1559: the updated contract row

Verified

• I read the whole diff (3 files, +142/-11). The scope matches the description, there's no debug code, CHANGELOG.md is untouched, and there are no path or Windows concerns (the FIFO test is cfg(unix)).
• I ran cargo test -p socket-patch-core --all-features --lib -- lockfile_v4 native_patch_probe on 2b1c49e: 4 passed.
• CI: 54/54 on the head (35 success, 19 skipped), with ci-ok and clippy green. Mergeable, with no unresolved threads.
• Bugbot: clean on 2b1c49e. Its earlier FIFO finding on b3eaff4 is fixed and the thread is resolved.

Open questions (non-blocking)

• Here a lock that still has a patched record but no patchedDependencies key in package.json still gets the "remove the patchedDependencies entries" wording. The downgrade step it also names still clears the refusal, so this is harmless.

Auto-merge is armed, so approving sends this straight to the merge queue.

---

Generated by Claude Code