Skip to content

Fix vendored member refusal not naming root (#1904) - #1913

Merged
Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-vendored-member-lockfile-refusal
Nov 2, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-vendored-member-lockfile-refusal

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

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

Fixes #1904

Summary

When a vendored run (scan --mode vendored, get --mode vendored or vendor) starts in a workspace member that has no lockfile of its own, it refused with "no package-lock.json … run your package manager's install first". Following that advice only updates the workspace root's lock, so the next run refused the same way. The refusal now keeps the vendor_lockfile_missing code (so the JSON contract doesn't change), but it says which lock installs the member and which directory to vendor from:

/repo/packages/a is a workspace member with no lockfile of its own: the workspace root /repo lists it under "workspaces" and installs it from /repo/package-lock.json, so there is no lockfile here for vendoring to rewire; vendor from /repo (the workspace root)

This covers npm, yarn (classic and berry) and Bun package.json workspaces, vlt workspaces, and pnpm workspaces. For a pnpm lockfile-dir project outside a pnpm workspace, the lock's directory holds no project, so the refusal names the setting and points at hosted mode rather than sending the user somewhere else that also doesn't work.

Root cause

Step 4 of the lock-flavor probe (vendor/npm_flavor.rs) always returned the generic detail. It never ran the "the lock lives in another directory" lookup that hosted's redirect_*_lockfile_elsewhere refusal uses in hosted/governing_root.rs.

Change

  • governing_root: the npm part of hosted's refusal() (package.json workspaces, vlt workspaces, the pnpm workspace or lockfile-dir lock, with the nearer root winning) moves into one npm_lock_elsewhere() helper. It returns a LockElsewhere { dir, dir_kind, vendorable_dir, detail }. Hosted builds its refusal from that, and its messages are byte-for-byte unchanged (the existing governing_root tests pass untouched).
  • npm_flavor: detect_vendorable_npm_flavor (forward vendoring; vendor_npm_any and the yarn berry takeover preflight) swaps the generic detail for the governing-root one. preflight_packages and lock_text_refusals keep only the code, so they use a code-only probe and skip the lookup.

Test evidence

Issue Test Without fix With fix
#1904 (npm/yarn/Bun members) vendor::npm_flavor::tests::lockless_workspace_member_refusal_names_the_workspace_root FAILED ok
#1904 (pnpm member) vendor::npm_flavor::tests::lockless_pnpm_member_refusal_names_the_lock_directory FAILED ok
#1904 (CLI, npm member, vendor --json) vendor_failure_paths::lockless_workspace_member_refusal_names_the_workspace_root (--test vendor) FAILED ok
lockfile-dir variant lockfile_dir_project_refusal_names_the_setting_not_a_dead_end (new behavior) ok
a standalone lockless project keeps the install remedy lockless_standalone_project_keeps_the_install_remedy ok ok

To get the red runs I disabled the detail swap locally; with it disabled, both core tests and the CLI test failed.

Commands run locally:

  • cargo fmt --all -- --check: ok
  • cargo clippy --workspace --all-features -- -D warnings: ok
  • cargo test -p socket-patch-core --all-features: 6998 passed, 4 failed. The 4 failures are chmod-based fault-injection tests (copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_…, pypi_requirements::wire_failure_rolls_back_…). They can't fail a write when run as root, and this container runs as uid 0. None of them touch this change. CI runs as non-root.
  • cargo test -p socket-patch-cli --all-features --test vendor -- vendor_failure_paths: 28 passed
  • I didn't run the full cargo test --workspace locally: the container ran out of disk building every test binary. CI covers it.

No wrapper changes: the npm/PyPI/gem wrappers only pass the CLI's output through.

Review findings not fixed (from /code-review high)

  • The read-only probe (detect_npm_lock_flavor, used by vendor --check/--revert, the lock inventory, VEX, crawlers) still has the generic text. Left as is on purpose. Those consumers either keep only the code or don't show this refusal to the user as a remedy. Adding the ancestor walk there would put filesystem reads on every lockless crawl and VEX snapshot, and the VEX snapshot reads are recorded.
  • has_own_npm_family_lock is checked again inside npm_lock_elsewhere and pnpm_lock_elsewhere. Each check is a handful of stats on a refusal path only, and the guard keeps the pub(crate) helper correct for any caller.
  • The standalone-project test assumes no ancestor of TMPDIR is a JS workspace root with a lock. The existing governing_root tests already rely on the same thing.

https://claude.ai/code/session_011f9aeR3rq51BGZkfNJkypH


Note

Medium Risk
Shared workspace-root resolution now drives vendored error text; mistakes could misdirect monorepo users, though hosted behavior is refactored-not-rewritten and covered by existing tests.

Overview
Fixes #1904: vendoring from an npm-family workspace member with no local lock no longer tells users to “run install first” (which only updates the root lock and loops). The vendor_lockfile_missing code is unchanged, but the error text now points at where the lock actually lives and says to vendor from that directory—matching what hosted mode already inferred.

hosted/governing_root: npm “lock elsewhere” detection is consolidated into LockElsewhere and npm_lock_elsewhere() (package.json / vlt / pnpm workspaces and lockfile-dir, with the same nearer-root rules). Hosted refusals are built from that struct so existing hosted messages stay the same; vendored mode reuses the same lookup.

vendor/npm_flavor: detect_vendorable_npm_flavor swaps in lock_elsewhere_detail() on the missing-lock path; read-only callers that only need the code use detect_vendorable_npm_flavor_code to skip the ancestor walk. pnpm lockfile-dir projects outside a real workspace get a different message (hosted scan / fix layout) instead of a bogus “vendor from” hint.

Tests cover npm/yarn/Bun members, pnpm members, lockfile-dir, standalone lockless projects, and a CLI vendor --offline case.

Reviewed by Cursor Bugbot for commit c9d1b5f. Configure here.


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
Running a vendored scan, get or vendor from a workspace member with no
lockfile of its own refused with "no lockfile ... run your package
manager's install first". Following that advice only updated the
workspace root's lock, so the next run refused the same way.

The vendored refusal now keeps its vendor_lockfile_missing code but
says which lock installs the member and which directory to vendor
from, the same lookup hosted mode's refusal already used. The lookup
(package.json and vlt workspaces, pnpm workspace or lockfile-dir) moves
out of the hosted refusal into one shared helper; hosted messages are
unchanged.

Fixes #1904

Assisted-by: Claude Code:claude-opus-5-5
Review follow-ups to the workspace-member refusal:

- A pnpm project whose lock lives elsewhere through lockfile-dir was
  told to vendor from the lock's directory, which holds no project.
  It now names the setting and points at hosted mode instead.
- The message showed "." for the default --cwd; it now shows the
  member's own path.
- Callers that keep only the refusal code skip the workspace lookup.
- The run-from wording follows the stray-lock refusal's convention.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review November 1, 2026 04:53
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

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

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: labeled Ready for review.

  • Head: 8a97c8ddd6715e7d056951ce06bbd1d17d75dd6d, mergeable.
  • CI: 32/32 ran green (50 skipped by path filters).
  • Bugbot: reviewed this head, no findings; no open review threads.
  • Reviewer focus: governing_root.rs refactor now shared between hosted and vendored refusal paths; new npm-flavor test in vendor_failure_paths.rs.
  • Slack announcement could not be sent this run (send tool unavailable); next run retries.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief (8a97c8d)

What it does. Today, a vendored run started in a workspace member that has no lockfile of its own refuses with "run your package manager's install first", and following that advice just loops (#1904). This PR moves hosted's "the lock lives in another directory" lookup into a shared npm_lock_elsewhere() helper. The vendored refusal now uses that helper:

  • For npm, yarn, Bun, vlt and pnpm-workspace members, it names the root to vendor from.
  • For a pnpm lockfile-dir project, it points at hosted mode instead.

The error code stays vendor_lockfile_missing; only the message changes.

Risk: low. The new text only appears on a path that already fails closed (exit 1, nothing written), and the JSON code is unchanged. I traced the hosted refactor by hand:

  • The "stray lock first" order is kept.
  • The pnpm, workspace and vlt messages come out byte-identical.
  • "Nearer root wins, a tie goes to pnpm" is kept.
  • The existing hosted tests pass unchanged.

Look here

Verified

Changes I made: none.

Open questions (not blocking)

  • Should a member or lockfile-dir refusal get its own code (for example vendor_workspace_lockfile_elsewhere), so JSON tooling can tell it apart from a project that has no lock at all? The issue raised this. The PR keeps the existing code for contract stability.
  • For a pnpm member that inherits lockfile-dir from the workspace, the message says "run pnpm install to write a lock here". pnpm would actually write the lock at the workspace root. That is imprecise but not a loop, because the next run gives the correct "vendor from root" refusal. Only the standalone lockfile-dir case is tested.
  • vendorable_dir only checks that pnpm-workspace.yaml exists in the lock's directory. It does not check that the workspace lists this project. That is an edge case.

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


Generated by Claude Code

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[agent] Reviewed 8a97c8ddd6715e7d056951ce06bbd1d17d75dd6d. Checked shared governing-root lookup and vendored refusal guidance for workspace and lockfile-dir layouts. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.

@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Nov 2, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Nov 2, 2026
@mikolalysenko Mikola Lysenko (mikolalysenko) removed the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Nov 2, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] The merge queue dropped this PR with MERGE_CONFLICT. #1928 (for #1921) had moved the same hosted npm block in governing_root.rs into a public npm_family_governing_root, which vendor.rs now calls. I merged main and kept that function's signature and behavior, but it now delegates to this PR's shared npm_lock_elsewhere. It still returns the canonical pnpm root, and the hosted message text is unchanged. I also took #1971's Workspaces::listed_patterns change.

Local checks: cargo fmt --check and cargo clippy --workspace --all-features -D warnings are clean. Core governing_root/npm_flavor: 95 passed. CLI --test vendor: 158 passed, including #1928's member-refusal tests and this PR's regression test.

The push dismissed the approval, so this needs a re-approval and a re-queue.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


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 c9d1b5f. Configure here.

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

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: re-applied Ready for review at c9d1b5f92dd785b59a809f46471955b6026cd5b1.

  • I removed the label briefly while starting a conflict fix, then backed off when I saw another agent had already merged main.
  • Mergeable, CI 50/50 passed or skipped on this head, and Bugbot reviewed this head with no findings.
  • The earlier approval was on the pre-merge head, so it needs re-approval.

Generated by Claude Code

@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Nov 2, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[final reviewer] Re-enqueued at c9d1b5f92: approved (approval stays valid after the post-approval pushes), ci-ok and clippy green, mergeable, no unresolved threads. Armed squash auto-merge, which put it back into the merge queue after the earlier dequeue.


Generated by Claude Code

Merged via the queue into main with commit dfc230a Nov 2, 2026
50 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-vendored-member-lockfile-refusal branch November 2, 2026 18:01
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

3 participants