diff --git a/crates/socket-patch-cli/tests/vendor/vendor_failure_paths.rs b/crates/socket-patch-cli/tests/vendor/vendor_failure_paths.rs index 20f7e068a..7379a963e 100644 --- a/crates/socket-patch-cli/tests/vendor/vendor_failure_paths.rs +++ b/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) // ───────────────────────────────────────────────────────────────────── diff --git a/crates/socket-patch-core/src/hosted/governing_root.rs b/crates/socket-patch-core/src/hosted/governing_root.rs index 8b4a42f1d..3215f2b4b 100644 --- a/crates/socket-patch-core/src/hosted/governing_root.rs +++ b/crates/socket-patch-core/src/hosted/governing_root.rs @@ -119,55 +119,6 @@ pub async fn refusal( None } -/// The directory whose npm-family lock installs `root` when `root` is a -/// workspace member with no lock of its own (or only locks its manager -/// never reads): a pnpm workspace or `lockfile-dir`, an npm, yarn or Bun -/// `package.json` workspace, or a vlt workspace, the nearer root winning. -/// Returns that root with hosted mode's refusal for it. Vendored commands -/// that read only `root` (`vendor`, `vendor --revert`, `vendor --check`) -/// use the root to refuse instead of reporting nothing to do (#1921). -pub async fn npm_family_governing_root(root: &Path) -> Option<(PathBuf, Refusal)> { - 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); - // Canonical, like the other branches' roots: a relative - // `lockfile-dir` (`../..`) would otherwise name a `..` path. - let pnpm_dir = tokio::fs::canonicalize(dir) - .await - .unwrap_or_else(|_| dir.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_root, refusal)) = workspace { - if ws_root != pnpm_dir && ws_root.starts_with(&pnpm_dir) { - return Some((ws_root, refusal)); - } - } - return Some(( - pnpm_dir, - 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() - ), - }, - )); - } - workspace -} - /// A pnpm v9 project lock (the one the trust auto-config serves) in a /// workspace member (listed by the `packages:` globs of the nearest /// ancestor `pnpm-workspace.yaml`, #1006) whose settings come from that @@ -221,6 +172,108 @@ async fn cargo_member_refusal(root: &Path) -> Option { }) } +/// The directory whose npm-family lock installs `root` when `root` is a +/// workspace member with no lock of its own (or only locks its manager +/// never reads): a pnpm workspace or `lockfile-dir`, an npm, yarn or Bun +/// `package.json` workspace, or a vlt workspace, the nearer root winning +/// ([`npm_lock_elsewhere`]). Returns that root with hosted mode's refusal +/// for it. Vendored commands that read only `root` (`vendor`, `vendor +/// --revert`, `vendor --check`) use the root to refuse instead of +/// reporting nothing to do (#1921). +pub async fn npm_family_governing_root(root: &Path) -> Option<(PathBuf, Refusal)> { + if has_own_npm_family_lock(root) { + return member_stray_lock_refusal(root).await; + } + let elsewhere = npm_lock_elsewhere(root).await?; + let refusal = elsewhere.hosted_refusal(); + Some((elsewhere.dir, 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, canonical like the other roots. + pub(crate) dir: PathBuf, + /// [`Self::dir`] as hosted's refusal has always named it. + shown_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.shown_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 { + 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 shown_dir = lock.parent().unwrap_or(&lock).to_path_buf(); + // Canonical, like the other branches' roots: a relative `lockfile-dir` + // (`../..`) would otherwise name a `..` path. + let dir = tokio::fs::canonicalize(&shown_dir) + .await + .unwrap_or_else(|_| shown_dir.clone()); + // 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 { + if ws.dir != dir && ws.dir.starts_with(&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, + shown_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: /// @@ -306,26 +359,25 @@ 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 { 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)) + shown_dir: ancestor.clone(), + dir: ancestor, + dir_kind: "workspace root", + vendorable_dir: true, + }) } /// A workspace member's own locks that its package manager never reads @@ -737,7 +789,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 @@ -950,25 +1002,24 @@ 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 { 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)) + shown_dir: ancestor.clone(), + dir: ancestor, + dir_kind: "workspace root", + vendorable_dir: true, + }) } /// The file a vlt workspace root lists its members in. @@ -999,7 +1050,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 @@ -1177,13 +1228,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, b: Option) -> Option { 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 diff --git a/crates/socket-patch-core/src/vendor/npm_flavor.rs b/crates/socket-patch-core/src/vendor/npm_flavor.rs index 153255f5a..c3393179b 100644 --- a/crates/socket-patch-core/src/vendor/npm_flavor.rs +++ b/crates/socket-patch-core/src/vendor/npm_flavor.rs @@ -355,8 +355,27 @@ async fn sniff_yarn_lock(view: &ProjectView<'_>) -> Result Result<(NpmLockFlavor, Vec), (&'static str, String)> { + match detect_vendorable_npm_flavor_code(project_root).await { + Err(("vendor_lockfile_missing", detail)) => Err(( + "vendor_lockfile_missing", + lock_elsewhere_detail(project_root).await.unwrap_or(detail), + )), + found => found, + } +} + +/// [`detect_vendorable_npm_flavor`] for callers that keep only the +/// refusal code: the detail is the probe's own. +async fn detect_vendorable_npm_flavor_code( + project_root: &Path, ) -> Result<(NpmLockFlavor, Vec), (&'static str, String)> { detect_vendorable_npm_flavor_with(project_root, || { crate::crawlers::pkg_managers::yarn_node_linker(project_root) @@ -400,6 +419,39 @@ async fn detect_vendorable_npm_flavor_with( )) } +/// #1904: the `vendor_lockfile_missing` detail for a project with no lock +/// of its own that its package manager installs from a lock in another +/// directory (a workspace member, or a pnpm `lockfile-dir`). Running +/// install in the project only updates that other lock, so the generic +/// "run install first" remedy would loop: a workspace member is told to +/// vendor from the workspace root, a `lockfile-dir` project that vendor +/// cannot wire it. `None` when no other directory governs the project. +/// Hosted's `redirect_*_lockfile_elsewhere` refusal reads the same lookup. +async fn lock_elsewhere_detail(project_root: &Path) -> Option { + // The member's own path, not the default `--cwd` `.`. + let shown = tokio::fs::canonicalize(project_root) + .await + .unwrap_or_else(|_| project_root.to_path_buf()); + let elsewhere = crate::hosted::governing_root::npm_lock_elsewhere(&shown).await?; + Some(if elsewhere.vendorable_dir { + format!( + "{}, so there is no lockfile here for vendoring to rewire; vendor from {} (the {})", + elsewhere.detail, + elsewhere.dir.display(), + elsewhere.dir_kind + ) + } else { + format!( + "{}; vendor does not rewire a pnpm lock kept in another directory \ + (`lockfile-dir`): use `socket-patch scan --mode hosted` from {} (the {}), or \ + remove `lockfile-dir` and run `pnpm install` to write a lock here", + elsewhere.detail, + elsewhere.dir.display(), + elsewhere.dir_kind + ) + }) +} + /// A workspace member whose own lock its package manager never reads /// (npm #1094, Bun #1101, vlt #1134: members install from the workspace /// root's lock), so vendoring into it would wire nothing. Refused as a @@ -624,7 +676,7 @@ pub async fn preflight_packages( project_root: &Path, packages: &[(&str, &PatchRecord)], ) -> Vec> { - let flavor = match detect_vendorable_npm_flavor(project_root).await { + let flavor = match detect_vendorable_npm_flavor_code(project_root).await { Ok((flavor, _probe_warnings)) => flavor, Err((code, _detail)) => return vec![Err(code); packages.len()], }; @@ -663,7 +715,7 @@ pub async fn lock_text_refusals( project_root: &Path, packages: &[(&str, &PatchRecord)], ) -> Vec> { - let flavor = match detect_vendorable_npm_flavor(project_root).await { + let flavor = match detect_vendorable_npm_flavor_code(project_root).await { Ok((flavor, _)) => flavor, Err(_) => return vec![None; packages.len()], }; @@ -1076,6 +1128,97 @@ mod tests { tokio::fs::write(root.join(name), content).await.unwrap(); } + /// #1904: a workspace member with no lock of its own is installed from + /// the workspace root's lock. The vendored refusal keeps its code but + /// names that root, as hosted's refusal and the stray-lock refusal + /// (#1094) do, instead of a "run install first" remedy that loops. + #[tokio::test] + async fn lockless_workspace_member_refusal_names_the_workspace_root() { + for lock in ["package-lock.json", "yarn.lock", "bun.lock"] { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + touch( + root, + "package.json", + r#"{"name":"app","private":true,"workspaces":["packages/*"]}"#, + ) + .await; + touch(root, lock, "").await; + let member = root.join("packages/a"); + tokio::fs::create_dir_all(&member).await.unwrap(); + touch(&member, "package.json", r#"{"name":"a"}"#).await; + + let (code, detail) = detect_vendorable_npm_flavor(&member).await.unwrap_err(); + assert_eq!(code, "vendor_lockfile_missing", "{lock}"); + let canonical = tokio::fs::canonicalize(root).await.unwrap(); + assert!( + detail.contains(&format!("vendor from {}", canonical.display())) + && detail.contains("(the workspace root)") + && detail.contains(&canonical.join(lock).display().to_string()) + && detail.contains("workspace member"), + "{lock}: {detail}" + ); + assert!(!detail.contains("install first"), "{lock}: {detail}"); + } + } + + /// #1904, pnpm: a pnpm workspace member is installed from the root's + /// pnpm-lock.yaml; the refusal names the directory holding it. + #[tokio::test] + async fn lockless_pnpm_member_refusal_names_the_lock_directory() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + touch(root, "pnpm-workspace.yaml", "packages:\n - packages/*\n").await; + touch(root, "pnpm-lock.yaml", "lockfileVersion: '9.0'\n").await; + let member = root.join("packages/a"); + tokio::fs::create_dir_all(&member).await.unwrap(); + touch(&member, "package.json", r#"{"name":"a"}"#).await; + + let (code, detail) = detect_vendorable_npm_flavor(&member).await.unwrap_err(); + assert_eq!(code, "vendor_lockfile_missing"); + let canonical = tokio::fs::canonicalize(root).await.unwrap(); + assert!( + detail.contains(&format!("vendor from {}", canonical.display())) + && detail.contains("pnpm-lock.yaml"), + "{detail}" + ); + } + + /// #1904, pnpm `lockfile-dir` outside a pnpm workspace: the lock's + /// directory holds no project, so the refusal names the setting and + /// hosted mode instead of telling the user to vendor from there. + #[tokio::test] + async fn lockfile_dir_project_refusal_names_the_setting_not_a_dead_end() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + touch(root, "pnpm-lock.yaml", "lockfileVersion: '9.0'\n").await; + let proj = root.join("proj"); + tokio::fs::create_dir_all(&proj).await.unwrap(); + touch(&proj, "package.json", r#"{"name":"p"}"#).await; + touch(&proj, ".npmrc", "lockfile-dir=..\n").await; + + let (code, detail) = detect_vendorable_npm_flavor(&proj).await.unwrap_err(); + assert_eq!(code, "vendor_lockfile_missing"); + assert!( + detail.contains("lockfile-dir") && detail.contains("scan --mode hosted"), + "{detail}" + ); + assert!(!detail.contains("vendor from"), "{detail}"); + assert!(!detail.contains("install first"), "{detail}"); + } + + /// A lockless project that no workspace governs keeps the generic + /// "run your package manager's install first" refusal. + #[tokio::test] + async fn lockless_standalone_project_keeps_the_install_remedy() { + let tmp = tempfile::tempdir().unwrap(); + touch(tmp.path(), "package.json", r#"{"name":"solo"}"#).await; + let (code, detail) = detect_vendorable_npm_flavor(tmp.path()).await.unwrap_err(); + assert_eq!(code, "vendor_lockfile_missing"); + assert!(detail.contains("install first"), "{detail}"); + assert!(!detail.contains("vendor from"), "{detail}"); + } + const YARN_V1: &str = "# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.\n\ # yarn lockfile v1\n\n\nleft-pad@^1.3.0:\n version \"1.3.0\"\n"; const YARN_BERRY: &str =