Fix vendored member refusal not naming root (#1904)

crates/socket-patch-cli/tests/vendor/vendor_failure_paths.rs

@@ -176,6 +176,52 @@ fn missing_package_with_no_lock_and_no_ledger_fails_before_any_fetch() {
     );
 }
 
+/// #1904: run from an npm workspace member with no lock of its own, the
+/// vendored refusal keeps `vendor_lockfile_missing` but names the
+/// workspace root to vendor from, instead of the "run your package
+/// manager's install first" remedy (an install in the member only
+/// updates the root lock, so that advice loops). Nothing is written.
+#[test]
+fn lockless_workspace_member_refusal_names_the_workspace_root() {
+    let fx = npm_fixture();
+    let root = fx.root();
+    let member = root.join("packages/a");
+    std::fs::create_dir_all(&member).unwrap();
+    for entry in ["node_modules", ".socket", "package.json"] {
+        std::fs::rename(root.join(entry), member.join(entry)).unwrap();
+    }
+    std::fs::write(
+        root.join("package.json"),
+        br#"{"name":"app","version":"1.0.0","private":true,"workspaces":["packages/*"]}"#,
+    )
+    .unwrap();
+
+    let (code, env) = vendor_cli(&member, &["--offline"]);
+    assert_eq!(code, 1, "the member run must fail closed: {env:#}");
+    let failed = find_event(&env, "failed", Some("vendor_lockfile_missing"));
+    assert_eq!(failed["purl"], PURL);
+    let detail = failed["error"].as_str().unwrap_or_default();
+    let canonical = std::fs::canonicalize(root).unwrap();
+    assert!(
+        detail.contains(&format!("vendor from {}", canonical.display()))
+            && detail.contains("workspace member"),
+        "the refusal names the workspace root: {env:#}"
+    );
+    assert!(
+        !detail.contains("install first"),
+        "no looping install remedy: {env:#}"
+    );
+    assert_eq!(
+        fx.lock_bytes(),
+        fx.original_lock,
+        "the root lock is untouched"
+    );
+    assert!(
+        !member.join(".socket/vendor").exists(),
+        "nothing is vendored into the member"
+    );
+}
+
 // ─────────────────────────────────────────────────────────────────────
 // 3. hosted-pin takeover guard (v5: lockfile pins, no redirect ledger)
 // ─────────────────────────────────────────────────────────────────────

crates/socket-patch-core/src/hosted/governing_root.rs

@@ -106,41 +106,12 @@ pub async fn refusal(
         }
     }
     if candidates.iter().any(|c| c.dep.ecosystem == "npm") {
-        let workspace = if has_own_npm_family_lock(root) {
-            member_stray_lock_refusal(root).await
-        } else {
-            nearer_root(
-                package_json_workspace_refusal(root).await,
-                vlt_workspace_refusal(root).await,
-            )
-        };
-        if let Some(lock) = pnpm_lock_elsewhere(root).await {
-            let dir = lock.parent().unwrap_or(&lock);
-            // A `package.json` workspace root nested inside the pnpm lock's
-            // directory is nearer the member and owns its lock (Bugbot on
-            // #901); otherwise pnpm's workspace or `lockfile-dir` governs.
-            if let Some((ws_root, refusal)) = workspace {
-                let pnpm_dir = tokio::fs::canonicalize(dir)
-                    .await
-                    .unwrap_or_else(|_| dir.to_path_buf());
-                if ws_root != pnpm_dir && ws_root.starts_with(&pnpm_dir) {
-                    return Some(refusal);
-                }
+        if has_own_npm_family_lock(root) {
+            if let Some(refusal) = member_stray_lock_refusal(root).await {
+                return Some(refusal);
             }
-            return Some(Refusal {
-                code: PNPM_LOCKFILE_ELSEWHERE.to_string(),
-                message: format!(
-                    "{} has no lockfile of its own: pnpm installs it from {}, which a \
-                     hosted run here cannot see; run socket-patch from {} (the directory \
-                     holding pnpm-lock.yaml); nothing was written",
-                    root.display(),
-                    lock.display(),
-                    dir.display()
-                ),
-            });
-        }
-        if let Some((_, refusal)) = workspace {
-            return Some(refusal);
+        } else if let Some(elsewhere) = npm_lock_elsewhere(root).await {
+            return Some(elsewhere.hosted_refusal());
         }
         if trust_lockfile_config {
             if let Some(refusal) = pnpm_settings_elsewhere(root) {
@@ -204,6 +175,86 @@ async fn cargo_member_refusal(root: &Path) -> Option<Refusal> {
     })
 }
 
+/// A project directory with no npm-family lock of its own that its
+/// package manager installs from a lock in another directory: a member of
+/// a `package.json` or vlt workspace, or a directory pnpm installs from an
+/// ancestor workspace's (or `lockfile-dir`'s) `pnpm-lock.yaml`.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub(crate) struct LockElsewhere {
+    /// The hosted refusal code.
+    code: &'static str,
+    /// The directory to run from.
+    pub(crate) dir: PathBuf,
+    /// What [`Self::dir`] is, for the "run from" hint (`workspace root`;
+    /// worded like [`MemberStrayLock::root_kind`]).
+    pub(crate) dir_kind: &'static str,
+    /// Whether a vendored run from [`Self::dir`] would wire the project:
+    /// `true` for a workspace root, which installs its members, and
+    /// `false` for a pnpm `lockfile-dir` outside any pnpm workspace, which
+    /// holds only the lock.
+    pub(crate) vendorable_dir: bool,
+    /// Which lock installs the project, without a remedy.
+    pub(crate) detail: String,
+}
+
+impl LockElsewhere {
+    /// The hosted refusal: a hosted run here would pin a lock nothing reads.
+    fn hosted_refusal(&self) -> Refusal {
+        Refusal {
+            code: self.code.to_string(),
+            message: format!(
+                "{}, which a hosted run here cannot see; run socket-patch from {} (the {}); \
+                 nothing was written",
+                self.detail,
+                self.dir.display(),
+                self.dir_kind
+            ),
+        }
+    }
+}
+
+/// `Some` when `root` holds no npm-family lock of its own and its package
+/// manager installs it from a lock in another directory (see
+/// [`LockElsewhere`]). Shared by the hosted refusal and the vendored
+/// `vendor_lockfile_missing` refusal (#1904), so both name the directory
+/// to run from. Of a `package.json` or vlt workspace root and a pnpm lock
+/// directory, the nearer one governs; a tie goes to pnpm.
+pub(crate) async fn npm_lock_elsewhere(root: &Path) -> Option<LockElsewhere> {
+    if has_own_npm_family_lock(root) {
+        return None;
+    }
+    let workspace = nearer_root(
+        package_json_workspace_elsewhere(root).await,
+        vlt_workspace_elsewhere(root).await,
+    );
+    let Some(lock) = pnpm_lock_elsewhere(root).await else {
+        return workspace;
+    };
+    let dir = lock.parent().unwrap_or(&lock).to_path_buf();
+    // A `package.json` workspace root nested inside the pnpm lock's
+    // directory is nearer the member and owns its lock (Bugbot on #901);
+    // otherwise pnpm's workspace or `lockfile-dir` governs.
+    if let Some(ws) = workspace {
+        let pnpm_dir = tokio::fs::canonicalize(&dir)
+            .await
+            .unwrap_or_else(|_| dir.clone());
+        if ws.dir != pnpm_dir && ws.dir.starts_with(&pnpm_dir) {
+            return Some(ws);
+        }
+    }
+    Some(LockElsewhere {
+        code: PNPM_LOCKFILE_ELSEWHERE,
+        detail: format!(
+            "{} has no lockfile of its own: pnpm installs it from {}",
+            root.display(),
+            lock.display()
+        ),
+        vendorable_dir: dir.join(PNPM_WORKSPACE).is_file(),
+        dir,
+        dir_kind: "directory holding pnpm-lock.yaml",
+    })
+}
+
 /// The `pnpm-lock.yaml` pnpm reads for a project directory that holds no
 /// npm-family lock of its own, when it lives elsewhere and exists:
 ///
@@ -289,26 +340,24 @@ const WORKSPACE_ROOT_FAMILIES: [NpmLockFamily; 3] =
 /// lock may itself be a member of an outer workspace (yarn berry's nested
 /// worktrees), so the walk goes on with that root as the member and
 /// refuses at the first root that holds an npm, yarn or Bun lock; a chain
-/// that ends without one (never installed), or at a Rush root, refuses
-/// nothing. Returns the
-/// governing root with the refusal, so [`refusal`] can weigh it against
-/// the pnpm check (the nearer root wins; a tie goes to pnpm's message).
-async fn package_json_workspace_refusal(root: &Path) -> Option<(PathBuf, Refusal)> {
+/// that ends without one (never installed), or at a Rush root, finds
+/// nothing. [`npm_lock_elsewhere`] weighs the root against the pnpm check
+/// (the nearer root wins; a tie goes to pnpm).
+async fn package_json_workspace_elsewhere(root: &Path) -> Option<LockElsewhere> {
     let (ancestor, locks) = package_json_workspace_root(root).await?;
-    let refusal = Refusal {
-        code: WORKSPACE_LOCKFILE_ELSEWHERE.to_string(),
-        message: format!(
+    Some(LockElsewhere {
+        code: WORKSPACE_LOCKFILE_ELSEWHERE,
+        detail: format!(
             "{} is a workspace member with no lockfile of its own: the workspace \
-             root {} lists it under \"workspaces\" and installs it from {}, which a \
-             hosted run here cannot see; run socket-patch from {} (the workspace \
-             root); nothing was written",
+             root {} lists it under \"workspaces\" and installs it from {}",
             root.display(),
             ancestor.display(),
             join_paths(&ancestor, &locks),
-            ancestor.display()
         ),
-    };
-    Some((ancestor, refusal))
+        dir: ancestor,
+        dir_kind: "workspace root",
+        vendorable_dir: true,
+    })
 }
 
 /// A workspace member's own locks that its package manager never reads
@@ -697,18 +746,17 @@ fn pnpm_lists_unmodeled(globs: &[String], rel: &[String]) -> bool {
 }
 
 /// The hosted refusal for [`member_stray_lock`].
-async fn member_stray_lock_refusal(root: &Path) -> Option<(PathBuf, Refusal)> {
+async fn member_stray_lock_refusal(root: &Path) -> Option<Refusal> {
     let stray = member_stray_lock(root).await?;
-    let refusal = Refusal {
+    Some(Refusal {
         code: WORKSPACE_LOCKFILE_ELSEWHERE.to_string(),
         message: format!(
             "{}; run socket-patch from {} (the {}); nothing was written",
             stray.detail,
             stray.root.display(),
             stray.root_kind
         ),
-    };
-    Some((stray.root, refusal))
+    })
 }
 
 fn join_paths(dir: &Path, names: &[&str]) -> String {
@@ -720,7 +768,7 @@ fn join_paths(dir: &Path, names: &[&str]) -> String {
 }
 
 /// The governing `package.json` workspace root of a member and the
-/// workspace locks it holds (see [`package_json_workspace_refusal`]).
+/// workspace locks it holds (see [`package_json_workspace_elsewhere`]).
 async fn package_json_workspace_root(root: &Path) -> Option<(PathBuf, Vec<&'static str>)> {
     let canonical = tokio::fs::canonicalize(root)
         .await
@@ -933,25 +981,23 @@ async fn read_head(path: &Path) -> String {
 /// would find the member's copy, pin nothing and report success. The
 /// nearest ancestor whose patterns match is the root; one without a lock
 /// (never installed) refuses nothing.
-async fn vlt_workspace_refusal(root: &Path) -> Option<(PathBuf, Refusal)> {
+async fn vlt_workspace_elsewhere(root: &Path) -> Option<LockElsewhere> {
     let (ancestor, source) = vlt_workspace_root(root).await?;
-    let lock = ancestor.join(VLT_LOCK);
-    let refusal = Refusal {
-        code: WORKSPACE_LOCKFILE_ELSEWHERE.to_string(),
-        message: format!(
+    Some(LockElsewhere {
+        code: WORKSPACE_LOCKFILE_ELSEWHERE,
+        detail: format!(
             "{} is a workspace member with no lockfile of its own: the workspace \
-             root {} lists it under \"{}\" in {} and installs it from {}, \
-             which a hosted run here cannot see; run socket-patch from {} (the \
-             workspace root); nothing was written",
+             root {} lists it under \"{}\" in {} and installs it from {}",
             root.display(),
             ancestor.display(),
             source.field(),
             ancestor.join(source.file()).display(),
-            lock.display(),
-            ancestor.display()
+            ancestor.join(VLT_LOCK).display(),
         ),
-    };
-    Some((ancestor, refusal))
+        dir: ancestor,
+        dir_kind: "workspace root",
+        vendorable_dir: true,
+    })
 }
 
 /// The file a vlt workspace root lists its members in.
@@ -982,7 +1028,7 @@ impl VltWorkspaceSource {
 
 /// The nearest ancestor whose vlt workspace patterns ([`vlt_workspace_groups`])
 /// match the project directory, when it holds `vlt-lock.json` (see
-/// [`vlt_workspace_refusal`]), with the file that lists the member.
+/// [`vlt_workspace_elsewhere`]), with the file that lists the member.
 async fn vlt_workspace_root(root: &Path) -> Option<(PathBuf, VltWorkspaceSource)> {
     let canonical = tokio::fs::canonicalize(root)
         .await
@@ -1160,13 +1206,10 @@ async fn vlt_json_declares_workspaces(dir: &Path) -> bool {
         .is_some()
 }
 
-/// The refusal of the nearer (deeper) of two governing roots.
-fn nearer_root(
-    a: Option<(PathBuf, Refusal)>,
-    b: Option<(PathBuf, Refusal)>,
-) -> Option<(PathBuf, Refusal)> {
+/// The nearer (deeper) of two governing roots; a tie goes to `a`.
+fn nearer_root(a: Option<LockElsewhere>, b: Option<LockElsewhere>) -> Option<LockElsewhere> {
     match (a, b) {
-        (Some(a), Some(b)) => Some(if b.0.starts_with(&a.0) && b.0 != a.0 {
+        (Some(a), Some(b)) => Some(if b.dir.starts_with(&a.dir) && b.dir != a.dir {
             b
         } else {
             a