Skip to content

Fix hosted runs from npm/yarn/bun workspace members pinning nothing (#884) - #901

Open
Mikola Lysenko (mikolalysenko) wants to merge 9 commits into
mainfrom
agent/fix-npm-family-member-governing-root
Open

Mikola Lysenko (mikolalysenko) wants to merge 9 commits into
mainfrom
agent/fix-npm-family-member-governing-root

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

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

Fixes #884

Root cause

hosted::governing_root::refusal (#598) refuses a workspace member whose lock lives in an ancestor only for pnpm (pnpm-workspace.yaml / lockfile-dir) and cargo. npm, yarn classic, yarn berry and Bun workspaces declare members through the root package.json workspaces field and keep one lock (package-lock.json, npm-shrinkwrap.json, yarn.lock, bun.lock / bun.lockb) at that root. A hosted scan or get <uuid> --mode hosted from a member reads only the member directory, finds no lock and exits 0 with success and redirected: 0. Its only signal is redirect_npm_no_lockfile, which names the wrong package manager. A fresh install then puts the unpatched copy in the member.

Fix

  • governing_root gets an npm-family workspaces case. When the project dir has no npm-family lock of its own (and isn't a Rush repo), it walks the ancestors for the nearest package.json whose workspaces (array, or the object form's packages, which covers yarn classic nohoist) matches the member path. Matching supports *, ? and **, and a later ! negation excludes a path. If that root holds an npm/yarn/Bun lock, the run is refused before any takeover or write with the new code redirect_workspace_lockfile_elsewhere, and the message names the lock and the root to run from. A lockless root, an unlisted directory or a member with its own lock is left alone. vlt is out of scope: it declares workspaces in vlt.json.
  • The "own lock" probe (has_own_npm_family_lock) is now shared by the pnpm and package.json checks.
  • A lockless matching root hands the walk to an outer root that lists it (yarn berry nested worktrees; Bugbot finding, nested_lockless_workspace_defers_to_the_outer_root). When both a pnpm workspace and a package.json workspace govern the member, the nearer root wins and a tie goes to pnpm's redirect_pnpm_lockfile_elsewhere (nearer_package_json_root_beats_an_outer_pnpm_workspace). Only npm/yarn/Bun locks count at a workspaces root. pnpm reads only pnpm-workspace.yaml and vlt only vlt.json, so a stray inner pnpm-lock.yaml is walked past rather than named (nested_pnpm_root_stops_the_package_json_walk, stray_inner_pnpm_lock_does_not_beat_the_outer_pnpm_workspace).
  • CLI_CONTRACT.md documents the code.
  • Vendored mode already fails closed here (vendor_lockfile_missing), so the two modes now agree.
  • Ported Route Gradle digests through utils::digest #878 (Route Gradle digests through utils::digest, cherry-picked as d4462e9). main (9c43dfc) is red on test / coverage / test-release in utils::digest::tests::production_digests_go_through_the_helpers. That failure isn't this PR's. Route Gradle digests through utils::digest #878 is its fix, and the cherry-pick becomes a no-op once Route Gradle digests through utils::digest #878 lands.

Follow-up (not in this PR)

A hosted scan from a hoisted / PnP member with no local copy finds 0 packages and exits 0, as pnpm members do. Whether a member scan should inventory the root's hoisted node_modules, or fail closed on an empty crawl, is a separate discovery-scope decision (Bugbot thread on d4462e9). get <uuid> --mode hosted in that layout is refused.

Per-issue checklist

  • Hosted scan/get run from a yarn classic workspace member still reports success while pinning nothing: the #598 governing-root refusal covers pnpm and cargo only #884: yarn classic member (array and {packages, nohoist} forms), yarn berry (yarn.lock), Bun (bun.lock, bun.lockb), npm (package-lock.json, npm-shrinkwrap.json). Covered by the CLI test in_process_redirect_pnpm::hosted_scan_from_package_json_workspace_member_refuses, which runs scan --mode hosted and get <uuid> --mode hosted for each shape and checks exit 1, redirect_workspace_lockfile_elsewhere and an untouched root lock. Core unit tests in hosted::governing_root::tests: package_json_workspace_member_is_refused_for_every_root_lock, package_json_object_workspaces_member_is_refused, package_json_workspace_non_members_are_left_alone, package_json_workspace_root_is_the_nearest_listing_ancestor, workspaces_patterns_match_like_npm_and_yarn and workspace_patterns_reads_both_field_shapes.

Test evidence

  • Red (new CLI test against the unfixed governing_root.rs from main): cargo test -p socket-patch-cli --all-features --test in_process_redirect_pnpm package_json_workspace failed with package-lock.json (object form: false): a found-but-unpinnable patch is not success … left: Some(0) right: Some(1) and "status":"success".
  • Green: in_process_redirect_pnpm 17/17 passed, governing_root unit tests 14/14 passed.
  • cargo clippy --workspace --all-features -- -D warnings passes. The workspace-wide cargo fmt churn from f92cb6a (118 untouched files) was reverted in 0be357d; each restored file is rustfmt-equivalent to main, so the diff is now just the 6 files this fix touches.
  • cargo test --workspace --all-features --no-fail-fast gave 226 test binaries OK. The only failures were tests that depend on file permissions (chmod 0o555 / unremovable files), which can't fail as root in this sandbox: covgap_commands_vendor (3), in_process_redirect (3 write-failure cases), repair (2), and in core lib copy_tree symlink relax, vlt_heal unremovable lock, pypi_poetry / pypi_requirements write failures. None of them touch this change. The core digest guard failure is fixed by the Route Gradle digests through utils::digest #878 port.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LYxnnBSxyMahcBX9SZag9L


Note

Medium Risk
Changes hosted scan/get exit behavior for monorepo members (fail-closed instead of false success); workspace-root detection is non-trivial but heavily tested.

Overview
Hosted mode no longer silently “succeeds” when run from an npm, Yarn, or Bun workspace member whose lockfile lives at the repo root. hosted::governing_root now walks ancestors for a root package.json whose workspaces patterns match the member (array or { packages } form, with *, ?, **, and ! negation). If that root holds package-lock.json, shrinkwrap, yarn.lock, or bun.lock / bun.lockb, scan / get --mode hosted refuse before any write with redirect_workspace_lockfile_elsewhere, exit 1, and tell the user to run from the workspace root. Members with their own lock, unlisted paths, and Rush roots are unchanged. When both pnpm and package.json workspaces apply, the nearer governing root wins (pnpm message on a tie).

CLI_CONTRACT.md documents the new code alongside the existing pnpm/cargo member refusals. Integration tests cover each lock type for scan and get; core unit tests cover nested workspaces and pnpm/yarn interaction.

Also routes Gradle/JVM SHA-1 and SHA-256 through utils::digest helpers in gradle_cache, jvm_jar, and Maven sidecars (cherry-pick of #878), replacing duplicated inline digest calls.

Reviewed by Cursor Bugbot for commit 0be357d. Configure here.


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
A hosted scan or get run from a member of an npm, yarn (classic or
berry) or Bun workspace found the member's copy of a patched package,
saw no lockfile in the member directory, pinned nothing and exited 0
with "success". The package manager then installed the unpatched copy
from the workspace root's lockfile, and the only hint was a warning
about a missing package-lock.json.

The workspace-member pre-check only knew about pnpm and cargo. It now
also finds the nearest ancestor package.json whose "workspaces" list
matches the member directory. If that root holds a package-lock.json,
npm-shrinkwrap.json, yarn.lock, bun.lock or bun.lockb, the run is
refused before anything is written with
redirect_workspace_lockfile_elsewhere, and the message names the
workspace root to run from. Vendored mode already refused this layout.

Fixes #884

Assisted-by: Claude Code:claude-opus-5-5
main has failed socket-patch-core's lib tests since Gradle support
(#646) and the digest helpers (#865) both landed. The guard test
production_digests_go_through_the_helpers flags three files #646 added
that still hash inline: crawlers/gradle_cache.rs, patch/jvm_jar.rs and
patch/sidecars/maven.rs. That breaks test, test-release and coverage on
every open PR.

Each inline sha1/sha256 call now goes through sha1_hex_of or
sha256_hex_of, which compute the same lowercase hex. Behaviour is
unchanged.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 6, 2026 01:08
@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.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Autofix Details

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

  • ✅ Fixed: Empty crawl skips workspace refusal
    • Modified governing_root::refusal to check npm workspace layouts when candidates is empty and package.json exists, ensuring hoisted/PnP members are refused before returning success.

Create PR

Or push these changes by commenting:

@cursor push 59281f5e08
Preview (59281f5e08)
diff --git a/crates/socket-patch-core/src/hosted/governing_root.rs b/crates/socket-patch-core/src/hosted/governing_root.rs
--- a/crates/socket-patch-core/src/hosted/governing_root.rs
+++ b/crates/socket-patch-core/src/hosted/governing_root.rs
@@ -68,7 +68,12 @@
             return Some(refusal);
         }
     }
-    if candidates.iter().any(|c| c.dep.ecosystem == "npm") {
+    // Check npm-family workspace refusals when there are npm candidates OR
+    // when candidates is empty and a package.json exists (hoisted/PnP
+    // members may have zero discovered packages).
+    let has_npm_candidates = candidates.iter().any(|c| c.dep.ecosystem == "npm");
+    let has_package_json = root.join("package.json").exists();
+    if has_npm_candidates || (candidates.is_empty() && has_package_json) {
         if !has_own_npm_family_lock(root) {
             if let Some(refusal) = package_json_workspace_refusal(root).await {
                 return Some(refusal);

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

Comment thread crates/socket-patch-core/src/hosted/governing_root.rs Outdated
Comment thread crates/socket-patch-core/src/hosted/governing_root.rs Outdated
A workspace root with no lockfile of its own can itself be a member of
an outer workspace (yarn berry's nested worktrees), where the outer
root holds the lockfile both use. The member check stopped at the inner
root and let the hosted run report success with nothing pinned. It now
keeps walking with the inner root as the member and refuses at the
outer root.

Refs #884

Assisted-by: Claude Code:claude-opus-5-5
@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.

Stale Bugbot comment from a previous run.

Comment thread crates/socket-patch-core/src/hosted/governing_root.rs
A pnpm workspace nested inside an outer yarn or npm workspace owns its
members: pnpm installs them from the nested pnpm-lock.yaml. The member
walk treated that nested root as lockless and went on to the outer
root, so the refusal named the wrong directory to run from. The walk
now stops at a root holding any npm-family lock (pnpm, vlt, Rush) and
leaves it to the pnpm check, which names the right root.

Refs #884

Assisted-by: Claude Code:claude-opus-5-5
@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.

Stale Bugbot comment from a previous run.

Comment thread crates/socket-patch-core/src/hosted/governing_root.rs
The previous change stopped the member walk at a nested root holding a
pnpm, vlt or shrinkwrap.yaml lock and left it to the pnpm check. That
check only knows pnpm workspaces with pnpm-workspace.yaml or
lockfile-dir, so a stray pnpm-lock.yaml there could still let a hosted
run from the member report success with nothing pinned.

The pnpm check now runs first, so a pnpm workspace still gets its own
precise message. The package.json walk then refuses at the first
matching root that holds any npm-family lock, naming that root. Only a
Rush root, whose locks live under common/config, ends the walk without
a refusal.

Refs #884

Assisted-by: Claude Code:claude-opus-5-5
@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.

Stale Bugbot comment from a previous run.

Comment thread crates/socket-patch-core/src/hosted/governing_root.rs
When a yarn or npm workspace sits inside a pnpm workspace, the member's
lockfile is the nearer one. The pnpm check ran first and named the outer
pnpm root, so a follow-up run from there would rewrite pnpm-lock.yaml
and leave the member's real lockfile unpatched.

The member check now weighs both roots and names the nearer one. When
they are the same directory, pnpm's own message wins.

Refs #884

Assisted-by: Claude Code:claude-opus-5-5
@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.

Stale Bugbot comment from a previous run.

Comment thread crates/socket-patch-core/src/hosted/governing_root.rs
pnpm reads only pnpm-workspace.yaml and vlt only vlt.json, so a pnpm or
vlt lock sitting at a package.json "workspaces" root does not govern
that root's members. Counting such a stray lock as ownership let it
beat the outer pnpm workspace that really installs the member, and the
refusal pointed at a directory whose run would rewrite the wrong
lockfile.

The workspaces walk now counts only npm, yarn and Bun locks. Roots that
pnpm governs are left to the pnpm check, and when both kinds govern a
member the nearer root still wins.

Refs #884

Assisted-by: Claude Code:claude-opus-5-5
@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.

Stale Bugbot comment from a previous run.

f92cb6a ran a workspace-wide cargo fmt, reformatting 118 files that the
fix does not otherwise touch. That buried the real change in ~2,300
lines of formatting diff and invites merge conflicts with every other
open PR. Restore those files to their merge-base versions; each was
checked to be rustfmt-equivalent to its main version, so behavior is
unchanged.

Co-Authored-By: Claude <noreply@anthropic.com>
@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 0be357d. Configure here.

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

Copy link
Copy Markdown
Collaborator Author

[burn-down agent] Ready for review at head 0be357d (0 behind main).

  • CI: 412/412 check runs completed, 0 failed/pending (406 success, 6 skipped).
  • Bugbot: reviewed 0be357d, no new issues; no unresolved review threads.
  • Change this run: f92cb6a had run a workspace-wide cargo fmt, which reformatted 118 files the fix doesn't otherwise touch. 0be357d restores them to main, and each one was checked to be rustfmt-equivalent, so the diff is now the 6 files this fix touches (governing_root.rs, the CLI test, CLI_CONTRACT.md and the Route Gradle digests through utils::digest #878 Gradle port).
  • Reviewer focus: the workspaces glob matching and nearest-root precedence in hosted::governing_root.

Generated by Claude Code

This branch has not been deployed

No deployments
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