[Mikola Lysenko (mikolalysenko)]

Problem

VEX discovery (vex/discover/) attested two kinds of npm-family pins that hosted and vendored mode both refuse. In both cases the install may not use the attested bytes.

• N1: pnpm gitBranchLockfile (Hosted scan with pnpm gitBranchLockfile pins the stale pnpm-lock.yaml and reports success, while pnpm installs unpatched bytes from pnpm-lock.<branch>.yaml #556). When the setting is on and a pnpm-lock.<branch>.yaml exists, pnpm installs the branch from that lock. Hosted leaves out the root locks and member locks and refuses every pnpm pin (hosted/engine.rs, refuse_git_branch_locks). Vendored refuses the project (vendor/pnpm_lock.rs, git_branch_lock_refusal). Discovery's extract_pnpm never checked the setting.
• N2: user-patched npm and Bun packages (npm patchedDependencies / lock patched record 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, bun patch Hosted and vendored Bun rewiring silently discards the project's own bun patch (patchedDependencies): fresh frozen installs drop the user's patch with exit 0 #367). Hosted skips these packages and records them as refused, and never confirms them. Vendored refuses them (npm v4 lock; Bun refuses the one dependency). Discovery's npm and Bun extractors never read these fields, so a pin written before 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/Hosted and vendored Bun rewiring silently discards the project's own bun patch (patchedDependencies): fresh frozen installs drop the user's patch with exit 0 #367 was attested.

Fix

• N1 (revised per owner decision, 5df67ed): discovery withholds whenever any pnpm-lock.<branch>.yaml exists, at the root or in a workspace member that installs from its own lock. That holds whatever gitBranchLockfile or pnpm's lockfile setting says, so the result is deterministic and fails closed. The check reads the project's files only: no settings, no environment, nothing above the project. Rush is exempt, and the check runs only once a root or member pnpm lock has wired a patch. A new utils::pnpm_workspace::git_branch_lock_files lists the branch locks. When one exists, every ref of the root pnpm lock (or shrinkwrap.yaml) and of each member's own lock is marked UnattestedKind::PnpmGitBranchLock. This goes through UnwiredCopy { scope: <lock>, target: Any }, so only refs that survive the contests get marked. Hosted and vendored refs are both marked, because vendored mode also refuses this state.
• N2: hosted's npm 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 is moved out of rewrite_npm_lock into patch::redirect::NpmUserPatches. It holds the package.json patchedDependencies keys plus the non-null patched records from any present npm lock. The hosted rewriter and npm discovery both use it now (no behaviour change for hosted). Bun discovery uses the rewriters' per-dependency rule, bun_lock_text::patched_dependency_key: for bun.lock it reads the manifest keys and the keys mirrored in the lock, the same as rewrite_bun_lock; for bun.lockb it reads the manifest keys only, the same as the hosted engine and vendor::bun_binary. Each ref is checked against its hosted URL or vendored tarball, so a key naming the pinned tarball itself is still not a conflict. Matching refs are marked UnattestedKind::UserPatchedDependency.
• CLI: vex_sources.rs gets note arms vex_pnpm_git_branch_lockfile / vex_npm_patched_dependency. CLI_CONTRACT.md documents both: the Unattested references bullet, the code table, and a cross-link from the Hosted scan with pnpm gitBranchLockfile pins the stale pnpm-lock.yaml and reports success, while pnpm installs unpatched bytes from pnpm-lock.<branch>.yaml #556 paragraph.

Why keep the ref and mark it unattested, instead of a diagnostic and no ref

In both cases the writers treat the wiring as present but unchangeable, not as dead:

• Hosted, user patch: the hosted warning tells the user to run socket-patch rollback <uuid> for a pin written before 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. That rollback needs the ref.
• Hosted, branch lock: hosted refuses the pin and keeps the vendored wiring on takeover instead of reverting it.
• Re-running doesn't help: a scan / vendor re-run can't clear the evidence in either case.

If the ref were dropped, the ledger liveness checks (hosted_claim / vendored_claim) would report the wiring as reverted. That's the same reasoning as #2005 / #2007, BundledCopy and NpmShrinkwrapOnly.

Choices worth a look

• Discovery ignores environment variables for pnpm settings (sharedWorkspaceLockfile; since 5df67ed it does not read gitBranchLockfile at all). git_branch_locks_in(view, false), the files-only reading of the setting, stays available and reads a governing ancestor's files only from inside the project's git repository. Hosted and vendored also read npm_config_git_branch_lockfile / pnpm_config_git_branch_lockfile from the environment. I followed discovery's existing precedent for the lockfile setting (lockfile_disabled(view, false): "the hosted engine runs in the install's environment, vex need not"), so vex and the ledger checks don't depend on the scanner's shell.
• Bun pins whose user patch Bun would drop are not attested either. For a registry-keyed bun patch over an earlier hosted pin, Bun silently drops the user's patch, so the installed bytes may in fact be the pure Socket bytes. I still don't attest them: hosted refuses and never confirms, and the owner rule makes the modes the ground truth.

Tests

Each new test fails on the code before this change (checked by switching the three new marking paths off: all three panic at their unattested assertions) and passes after:

• vex::discover::npm::tests::pnpm_git_branch_lockfile_marks_the_lock_pins_unattested: root lock via pnpm-workspace.yaml and via .npmrc, plus a sharedWorkspaceLockfile: false member with its own branch lock. Controls: no branch lock, and the setting off.
• vex::discover::npm::tests::npm_user_patched_dependency_pin_is_not_attested: a manifest key, a v4 lock-entry patched record, a record in the sibling shrinkwrap only, and a key for another package or version as the control.
• vex::discover::bun::tests::bun_user_patched_dependency_pin_is_not_attested: hosted bun.lock with versioned and bare keys; a key naming the pinned tarball (no conflict); a vendored ref keyed through bun.lock's mirrored keys; hosted bun.lockb.
• commands::vex_sources::tests::npm_family_unattested_kinds_have_their_own_tags.

pnpm_git_branch_lockfile_marks_the_lock_pins_unattested now covers the owner decision. A branch lock still withholds when the setting is true, false, missing, or turned off in .npmrc, and when pnpm's lockfile setting is off. The same holds for a member's branch lock with no setting. With no branch lock (before it is written, and after it is deleted), the pin attests.

Runs (after 5df67ed):

• cargo test -j4 -p socket-patch-core --lib -- vex::discover:: patch::redirect:: vendor::npm vendor::bun vendor::pnpm hosted:: utils::pnpm_workspace: 2179 passed.
• vex::discover::testing::golden::tests::committed_fixture_corpus_matches_golden passes; no golden changes.
• cargo test -j4 -p socket-patch-cli --lib vex_sources: 15 passed.
• cargo test -p socket-patch-cli --test in_process_redirect_pnpm: 41 passed.
• cargo clippy --locked --workspace --all-features -- -D warnings and cargo fmt --all -- --check are clean.

Not done

• pnpm's own patchedDependencies (pnpm.patchedDependencies / pnpm-workspace.yaml), yarn berry patch: and vlt are not covered. They're out of scope for N2.

Review findings

/code-review high produced 10 findings. I confirmed all 10 and fixed them in 76bb103, a new commit on top; nothing is refuted. Where a finding is a behaviour bug, a new test failed before the fix (each panicked at its target assertion) and passes after.

1. Confirmed, fixed. git_branch_locks_in(view, false) still read npm_config_ / pnpm_config_shared_workspace_lockfile through workspace_members → view_setting. Discovery's existing member_locks call did too. view_setting and workspace_members now take env, and discovery calls a new member_locks_in(view, false). Discovery reads no environment variables anywhere. Test: pnpm_git_branch_lockfile_discovery_reads_no_environment (serial, EnvGuard).
2. Confirmed, fixed. UserPatchedDependency marks were pushed before contest_across_locks, so a dropped ref kept a stale reason. Marks are now UnwiredCopys for exactly one ref (new CopyTarget::Ref { purl, uuid }), and they land only if that ref survives the contests. Tests: a_contested_user_patched_ref_leaves_no_mark (npm) and a_contested_user_patched_bun_ref_leaves_no_mark.
3. Confirmed, fixed. NpmUserPatches::add_mirrored_keys records keys that only bun.lock mirrors, and the detail now says "bun.lock patchedDependencies has …". Test: an added assertion in bun_user_patched_dependency_pin_is_not_attested.
4. Confirmed, fixed. The vex_npm_patched_dependency remedy now names socket-patch rollback <uuid> for a pre-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 pin that fails npm ci with EPATCHFAILED. Test: an assertion in npm_family_unattested_kinds_have_their_own_tags.
5. Confirmed, fixed (perf). An npm lock is walked for patched records only when its bytes contain "patched", as rewrite_npm_lock does.
6. Confirmed, fixed (perf). git_branch_locks_in runs only after a root lock, shrinkwrap.yaml or member pnpm lock has produced a ref.
7. Confirmed, fixed. npm and Bun now share one npm::mark_user_patched (with user_patch_source). Bun's copy of the rule is gone.
8. Confirmed, fixed. A ref with neither url nor artifact_rel is matched with an empty target, so its name@version / bare-name key still applies and every tarball key counts. Test: a_ref_without_a_target_is_still_matched_by_name_and_version.
9. Confirmed, fixed (docs). CLI_CONTRACT.md now says the "another tarball" key rule applies to npm and Bun alike.
10. Confirmed, fixed. The mark names the branch lock in the marked lock's own directory and falls back to the first one. Test: pnpm_git_branch_lock_mark_names_the_locks_own_branch_lock.

Owner rule: no reads outside the git repository root. Discovery's files-only mode now reads a governing ancestor pnpm-workspace.yaml / .npmrc only when that ancestor is at or below the nearest directory with a .git entry. The check only stats paths. Outside a repository, discovery reads no ancestor at all. Hosted and vendored (env mode) are unchanged. Test: utils::pnpm_workspace::tests::the_files_only_reading_stays_inside_the_git_repository.

Note for the stacked #2018: it changes the same code (the Bun tarball-key rule, a shared project walk). Its rebase will need to pick up CopyTarget::Ref / mark_user_patched and NpmUserPatches::add_mirrored_keys.

---

Note

Medium Risk
Touches core VEX discovery and pnpm workspace parsing; incorrect branch-lock or user-patch detection could suppress legitimate attestations, so thorough regression testing is important.

Overview
Aligns vex discovery with hosted-mode rules for pnpm git-branch locks and npm/Bun user patches so those pins are never attested while legitimate contests still remove stale reasons.

pnpm git-branch locks: files-only discovery now enumerates pnpm-lock.<branch>.yaml files per root/member lock without consulting env vars, tags surviving refs as PnpmGitBranchLock, and falls back to fail-closed diagnostics when member membership cannot be resolved. The CLI contract and VEX notes reference the new vex_pnpm_git_branch_lockfile case.

User patches: introduces shared NpmUserPatches tracking patchedDependencies, mirrored Bun keys, and lock patched records; npm/Bun extractors mark conflicts via targeted UnwiredCopys plus new tests/docs covering remediation.

Also updates pnpm_workspace helpers (member_locks_in, git_branch_lock_files, repo-scoped ancestor reads) and expands regression suites to pin the new behavior.

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

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

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 ac92b08. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review at ac92b08.

• CI: 30/32 non-skipped checks green. The only failures are the main-wide e2e … pdm:: --ignored leg (module 'hishel' has no attribute 'SyncSqliteStorage', also red on main; Fix pdm e2e red on every PR: pin hishel<2 #2028 fixes it) and the ci-ok aggregate that follows from it.
• Bugbot: reviewed ac92b08; no unresolved findings.
• Promoted from draft by the burn-down agent this run. No Slack announcement was sent this run (the Slack send action wasn't available), so there's no slack-announced marker yet.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief (for the human approver)

What it does: VEX discovery stops attesting npm-family pins that hosted and vendored mode refuse.

• pnpm branch locks: when any pnpm-lock.<branch>.yaml exists at the root or in a member with its own lock, every root and member pnpm ref is marked PnpmGitBranchLock. The lock's presence alone decides it, as the owner chose.
• User patches: npm and Bun refs of packages the project patches itself are marked UserPatchedDependency. This goes through a new shared NpmUserPatches, extracted from the hosted npm rewriter.

Risk: low. Every change either withholds an attestation or is a refactor that keeps hosted behaviour the same. I compared the hosted rewrite_npm_lock gate before and after: same keys, same record filter, same precedence. Nothing new is written.

Look here

• npm.rs:830-851: the wired gate and branch-lock flow
• npm.rs:318 / :333: user_patch_source and mark_user_patched
• redirect/mod.rs:1423-1496 NpmUserPatches: the hosted refactor
• pnpm_workspace.rs:803 git_branch_lock_files
• bun.rs:199 unattest_user_patched

Verified

• I read the full diff against the description, and they match.
• The marks are applied after contest_across_locks, so they land only on surviving refs.
• patched_dependency_key targets line up between discovery and the rewriters.
• The new tests assert new marks and include controls. CRLF and Windows separators are handled. CHANGELOG.md is untouched.
• CI: 30/32 non-skipped checks green. The only failures are the main-wide pdm/hishel leg and the ci-ok aggregate; Fix pdm e2e red on every PR: pin hishel<2 #2028 fixes both.
• Bugbot: clean on this head.

Changes I made: none.

Open questions (non-blocking)

• Ledger effect: discovery now lists member locks with member_locks_in(view, false) instead of reading sharedWorkspaceLockfile from the environment. If that setting comes only from the environment, hosted member pins become non-refs, not unattested refs, so the ledger may read them as reverted. Is that intended?
• Stale doc comments: they still say the pnpm setting must be on: mod.rs:609-612 and npm.rs:731-733.
• Dead code: git_branch_locks_in(view, false) and governing_file_in_git_repository are now called only from tests.
• Fail-open path: branch_lock_names (pnpm_workspace.rs:990) treats a list_dir error as "no branch locks". That is unlikely in practice.

Auto-merge is not armed yet, because CI is red on the main-wide pdm failure. Once #2028 lands and this goes green, the next final-reviewer run arms it, and approving then sends it straight to the merge queue.

---

Generated by Claude Code