[Mikola Lysenko (mikolalysenko)]

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

Fixes #1921

Summary

vendor (eject), vendor --revert and vendor --check run from an npm-family workspace member directory (or with --cwd packages/a) used to look only at that directory. They found nothing and exited 0, so a per-package CI vendor --check passed even when the root's committed artifact was tampered. Now they refuse with vendor_lockfile_missing (exit 1) and name the workspace root to run from. That matches hosted scan and the vendored refusal docs/ecosystems.md already describes.

Root cause

None of the three forms asked whether an ancestor workspace root governs --cwd. Hosted scan already asks, through hosted::governing_root::refusal, which finds npm/yarn/Bun package.json workspaces, pnpm workspaces and lockfile-dir, and vlt workspaces. The vendor command never called it.

Change

• core hosted/governing_root.rs: the npm-family branch of refusal moves into a public npm_family_governing_root(dir) -> Option<(root, Refusal)>. Hosted behavior is unchanged, except that the pnpm root it returns is now canonical (a relative lockfile-dir no longer yields a .. path). The hosted refusal message is unchanged.

• cli commands/vendor.rs: a new member_refusal(dir, form) refuses only when all of these hold:

  • the directory has a package.json;
  • its vendor ledger is empty or absent;
  • the caller found no hosted pins and no manifest;
  • an ancestor root governs it.

  It is wired into the no-manifest eject arm, into --revert (when there is no manifest), and into --check (after read_manifest returns no manifest, so a manifest read error keeps its own code).

• CLI_CONTRACT.md: documents the refusal in the vendor section.

Tests (red → green)

New tests in crates/socket-patch-cli/tests/vendor/vendor_failure_paths.rs:

Test	Without fix	With fix
vendor_check_from_workspace_member_refuses_and_names_root: root vendored, then its tgz tampered; --check from the member (absolute --cwd, relative --cwd packages/a, and human mode)	exit 0 (FAILED)	exit 1, vendor_lockfile_missing, names root
vendor_eject_from_workspace_member_refuses_and_names_root: root lock hosted-pinned; eject from member (json + human)	exit 0, "nothing to vendor" (FAILED)	exit 1; root lock untouched; no .socket/ in member
vendor_revert_from_workspace_member_refuses_and_names_root	exit 0 (FAILED)	exit 1; root stays vendored
vendor_outside_a_workspace_keeps_the_no_manifest_exit	ok	ok (noManifest, exit 0)
vendor_in_a_non_npm_dir_under_a_pnpm_workspace_is_not_refused	ok	ok (exit 0 for eject, check, revert)
vendor_revert_in_a_member_with_its_own_manifest_is_not_refused	ok	ok

Per-issue checklist:

• vendor and vendor --check run from an npm workspace member directory exit 0 without looking at the root lock: the eject says "nothing to vendor" and the check passes over a tampered artifact #1921 vendor from a member: vendor_eject_from_workspace_member_refuses_and_names_root
• vendor and vendor --check run from an npm workspace member directory exit 0 without looking at the root lock: the eject says "nothing to vendor" and the check passes over a tampered artifact #1921 vendor --check from a member, including --cwd packages/a from the root: vendor_check_from_workspace_member_refuses_and_names_root

Local runs:

• cargo fmt --all -- --check: ok.
• cargo clippy --workspace --all-features -- -D warnings: ok.
• cargo test -p socket-patch-cli --all-features --test vendor: 157 passed.
• cargo test -p socket-patch-cli --all-features --lib: 940 passed.
• cargo test -p socket-patch-core --all-features --lib: 6995 passed, 4 failed. All 4 failures are permission fault-injection tests (copy_tree, vlt_heal, pypi_poetry, pypi_requirements) that can't fail as root in this container, and none is in a touched file.
• A full cargo test --workspace didn't fit the container's disk; CI runs it.

Review notes (/code-review high)

Fixed:

• non-npm directories under a pnpm workspace;
• manifest errors masked by the refusal;
• a misplaced doc comment;
• a message that claimed the member had no lock (a member with a stray lock is refused too);
• a non-canonical pnpm root;
• --revert refusing a member that has its own manifest.

Not fixed:

• The no-manifest eject path reads the ledger twice. That's a micro-cost on one path, and a fix would thread state through member_refusal for no behavior change.
• Cargo and uv workspace members have the same false exit 0 on vendor --check / --revert. They're out of scope for this npm issue and a candidate follow-up; governing_root already has cargo_member_refusal and governing_uv_workspace to reuse.

Yarn and Bun members go through the same package.json workspace lookup, and pnpm and vlt through their own branches. Only npm was exercised end to end.

https://claude.ai/code/session_011f9aeRgLZEtRSKeWxfgoZ3

---

Generated by Claude Code

[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 1dff586. Configure here.}

[Mikola Lysenko (mikolalysenko)]

[burn-down agent] Labeled Ready for review at 1dff586.

• CI: 82/82 green on 1dff586 (31 success, 1 neutral, 50 skipped); mergeable, no conflicts.
• Bugbot: reviewed 1dff586, no findings.
• Reviewer note: vendor / vendor --check / vendor --revert run inside a workspace member now check the governing ancestor root instead of exiting 0. See the governing_root.rs change and the CLI_CONTRACT.md addition.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does: Running vendor, vendor --revert or vendor --check from inside an npm-family workspace member used to exit 0 and do nothing. Now it refuses with vendor_lockfile_missing (exit 1) and names the workspace root to run from. The refusal only fires when the directory has a package.json, no manifest and an empty ledger, and (for vendor) no hosted pins in scope. In core, the npm-family root lookup moved out of refusal() into a public npm_family_governing_root().

Risk: low. The guard is narrow and runs before any lock is taken or file is written. Core's hosted refusal codes and messages are unchanged. The only behaviour change in core is that the pnpm directory is now canonicalized before it is returned.

Look here:

• vendor.rs:1513: member_refusal, the gate (package.json + empty ledger + core lookup).
• vendor.rs:1056, :1156, :1301: the three call sites. --check passes project_root(); the other two pass cwd.
• governing_root.rs:128: the extracted npm_family_governing_root.
• CLI_CONTRACT.md:1089: the contract text, which matches the code.

Verified:

• Read the full diff.
• New CLI tests pass 7/7. With vendor.rs reverted to main, 3 of them (eject, check, revert) fail, so they really cover the fix.
• Full vendor test target: 157/157 pass.
• Core governing_root tests: 49/49 pass.
• Clippy reports nothing on the changed files.
• CHANGELOG.md is untouched and there's no debug code left in.
• CI: ci-ok and clippy are green on the head.
• Bugbot's run on this head was cancelled, so its status here is neutral. It had reviewed the earlier head.

Changes I made: none.

Open questions (non-blocking):

• vendor --vex with no manifest inside a member is now refused too. The contract doesn't mention that case. Is that the intent?
• CLI tests cover only an npm workspaces member. pnpm, yarn, bun and vlt rely on core's unit tests.
• On Windows the message prints the member path as given next to a canonical \\?\ root. Other core refusals already do the same.

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

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] Answers to the review brief's open questions, from the diff:

• vendor --vex inside a member is refused too: intended. With no manifest, no ledger entries and no hosted pins in the member, a manifest-less VEX there could only attest nothing (exit 0, "nothing to attest") or attest the member's view of a project the root installs. Both mislead the same way the calm noManifest exit did. The refusal fires before VEX generation and names the root, where vendor --vex attests the real wiring. I can add a sentence to the CLI_CONTRACT paragraph if you want it spelled out.
• Bugbot: the check run on 1dff586 shows neutral, but Bugbot did post a review of exactly 1dff586 (review 5727669756, "found no new issues").
• pnpm/yarn/bun/vlt coverage and the Windows \\\\?\\ root: agreed as described. The lookup is core's shared, unit-tested npm_family_governing_root, and the path format matches the existing hosted refusals.

---

Generated by Claude Code