Fix npm workspace-member refusal skipped by a stray member lock (#1094)

.github/workflows/ci.yml

@@ -1397,7 +1397,7 @@ jobs:
         # The composer capstones shell out to a real composer; `composer:`
         # pins the release line (1, 2.2 LTS, 2) so the composer.lock grammar
         # the edits assert stays stable across runners.
-        uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # v2
+        uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2
         with:
           php-version: '8.2'
           tools: composer:${{ matrix.composer }}

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -1295,7 +1295,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified
 | `vendor_would_revert_redirect` / `vendor_takeover_reverted_redirect` | `skipped` (advisory event) | vendor / scan / get `--mode vendored` over a hosted pin (every ecosystem, v5.0): dry run — the upstream restore was resolved (registry lookups included) and would succeed (for bun, only after the Bun vendored preflight accepted the lock; a refused lock is previewed as the wet run's `failed <code>` instead) / wet run — the pin's lock entries were restored to their upstream registry entry before vendoring (mode takeover; detail `<purl> was hosted; restored its upstream registry entry (<files>) before vendoring (mode takeover)`), so `vendor --revert` later returns to upstream. Fires on the run that takes over, not on re-runs, and not for a purl whose takeover was rolled back because the backend refused it (see "Takeover reconciliation"). |
 | `redirect_revert_failed` | `failed` | vendor / scan / get `--mode vendored` (dry and wet): the upstream restore of a hosted pin was refused (`--offline`, a registry that does not answer, a lock shape the restore refuses — for `bun.lockb`, a record the codec cannot rebuild) — detail `cannot vendor over the live hosted pin: cannot restore <purl> to its upstream registry entry: <why>; restore it from version control instead (`git checkout -- <files>`)`; nothing vendored for the purl, hosted wiring left in place, exit 1 `partial_failure`. |
 | `patch_fetch_failed` (eject) | `failed` | vendor eject (v5.0): a hosted pin's patch record could not be fetched from `…/patches/view/<uuid>`; the whole eject is refused (`eject_refused`), nothing touched, exit 1. |
-| `redirect_pnpm_lockfile_elsewhere` / `redirect_workspace_lockfile_elsewhere` / `cargo_manifest_not_workspace_root` (hosted) | top-level `errorCode` (`status: "error"`) | scan / get `--mode hosted` (v5.0): the project directory is a workspace member whose lock lives in another directory, so the rewriters, which read only the project directory, would pin nothing (pnpm: no npm-family lock here, and the nearest ancestor `pnpm-workspace.yaml` or the project's `lockfile-dir` (`.npmrc`) / `lockfileDir` (`pnpm-workspace.yaml`) puts `pnpm-lock.yaml` elsewhere; npm / yarn / Bun, `redirect_workspace_lockfile_elsewhere`: no npm-family lock here, and the nearest ancestor `package.json` whose `workspaces` (array, or the object form's `packages`) matches the directory holds `package-lock.json`, `npm-shrinkwrap.json`, `yarn.lock`, `bun.lock` or `bun.lockb`; a matching root with none of them that is itself listed by an outer root's `workspaces` hands the check to that root; vlt, same code: the nearest ancestor `vlt.json` whose `workspaces` (a string, an array, or an object of groups) matches the directory holds `vlt-lock.json`, or, as vlt falls back to it when `vlt.json` has no `workspaces` field, the `package.json` `workspaces` root above holds `vlt-lock.json`, and the nearer of a `vlt.json` and a `package.json` root is named; `workspaces` patterns use the glob grammar the package managers share: `*`, `?`, `**`, brace sets and sequences (`{a,b}`, `{1..3}`) and character classes (`[a-c]`, `[!a]`); when a pnpm workspace also governs the directory, the nearer root is named and a tie goes to `redirect_pnpm_lockfile_elsewhere`) or rewrite the member as a lockless project (cargo: the vendored workspace-root check). Refused before any takeover or write, `--dry-run` included; the message names the directory to run from; exit 1. Disk runs only (an in-memory project has no ancestors). |
+| `redirect_pnpm_lockfile_elsewhere` / `redirect_workspace_lockfile_elsewhere` / `cargo_manifest_not_workspace_root` (hosted) | top-level `errorCode` (`status: "error"`) | scan / get `--mode hosted` (v5.0): the project directory is a workspace member whose lock lives in another directory, so the rewriters, which read only the project directory, would pin nothing (pnpm: no npm-family lock here, and the nearest ancestor `pnpm-workspace.yaml` or the project's `lockfile-dir` (`.npmrc`) / `lockfileDir` (`pnpm-workspace.yaml`) puts `pnpm-lock.yaml` elsewhere; npm / yarn / Bun, `redirect_workspace_lockfile_elsewhere`: no npm-family lock here, and the nearest ancestor `package.json` whose `workspaces` (array, or the object form's `packages`) matches the directory holds `package-lock.json`, `npm-shrinkwrap.json`, `yarn.lock`, `bun.lock` or `bun.lockb`; a matching root with none of them that is itself listed by an outer root's `workspaces` hands the check to that root; vlt, same code: the nearest ancestor `vlt.json` whose `workspaces` (a string, an array, or an object of groups) matches the directory holds `vlt-lock.json`, or, as vlt falls back to it when `vlt.json` has no `workspaces` field, the `package.json` `workspaces` root above holds `vlt-lock.json`, and the nearer of a `vlt.json` and a `package.json` root is named; `workspaces` patterns use the glob grammar the package managers share: `*`, `?`, `**`, brace sets and sequences (`{a,b}`, `{1..3}`) and character classes (`[a-c]`, `[!a]`); when a pnpm workspace also governs the directory, the nearer root is named and a tie goes to `redirect_pnpm_lockfile_elsewhere`; a directory whose only locks are `package-lock.json` / `npm-shrinkwrap.json` is refused the same way when its `package.json` `workspaces` root holds `package-lock.json` or `npm-shrinkwrap.json`, because npm never reads a lock inside a workspace member (#1094; vendored refuses it with `vendor_lockfile_missing`)) or rewrite the member as a lockless project (cargo: the vendored workspace-root check). Refused before any takeover or write, `--dry-run` included; the message names the directory to run from; exit 1. Disk runs only (an in-memory project has no ancestors). |
 | `redirect_pnpm_settings_elsewhere` | top-level `errorCode` (`status: "error"`) | scan / get `--mode hosted`: the project directory is a pnpm workspace member with its own v9 `pnpm-lock.yaml` (`sharedWorkspaceLockfile: false`) and no `pnpm-workspace.yaml` of its own, so its pnpm settings come from the nearest ancestor `pnpm-workspace.yaml`, which pnpm reads alone (a member's own file is ignored). When that file neither carries `trustLockfile: true` nor explicitly sets another value, the trust auto-config has nowhere to go: refused before any takeover or write, `--dry-run` included; the message names the root file to add `trustLockfile: true` to (or `--no-trust-lockfile-config` pins without it); exit 1. Once the root file trusts the lock (or opts out), the member is pinned and no nested `pnpm-workspace.yaml` is created; the `redirect_pnpm_trust_lockfile` warning names the root file. Disk runs only. |
 | `eject_refused` | top-level `errorCode` (`status: "error"`) | vendor eject (v5.0): a record fetch failed or a pin's upstream restore was refused while planning; nothing was changed, exit 1. |
 | `eject_planned` | `applied` (reason) | vendor eject `--dry-run` (v5.0): the pin would be restored upstream and vendored; nothing written. |

crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs

@@ -1781,3 +1781,88 @@ fn assert_refused_workspace_lock_elsewhere(
         "{case}: nothing written in the member"
     );
 }
+
+/// #1094: an npm workspace member holding a stray `package-lock.json` /
+/// `npm-shrinkwrap.json` of its own (one npm never reads; members install
+/// from the root lock) used to skip the #884 refusal. `scan` and `get`
+/// pinned the ignored member lock and exited 0. They now refuse, name the
+/// root lock and the ignored member lock, and leave both untouched.
+#[tokio::test]
+#[serial]
+async fn hosted_scan_from_npm_member_with_stray_lock_refuses() {
+    let server = MockServer::start().await;
+    mock_discovery(&server).await;
+    mock_reference(&server).await;
+    mock_view(&server).await;
+    for (root_lock, member_lock) in [
+        ("package-lock.json", "package-lock.json"),
+        ("package-lock.json", "npm-shrinkwrap.json"),
+        ("npm-shrinkwrap.json", "package-lock.json"),
+    ] {
+        let tmp = tempfile::tempdir().unwrap();
+        let member = write_package_json_workspace(tmp.path(), root_lock, false);
+        let stray = member.join(member_lock);
+        let stray_text = serde_json::json!({
+            "name": "a", "version": "1.0.0", "lockfileVersion": 3,
+            "packages": {
+                "": { "name": "a", "version": "1.0.0", "dependencies": { NAME: VERSION } },
+                format!("node_modules/{NAME}"): {
+                    "version": VERSION,
+                    "resolved": format!("https://registry.npmjs.org/{NAME}/-/{NAME}-{VERSION}.tgz"),
+                    "integrity": "sha512-orig=="
+                }
+            }
+        })
+        .to_string();
+        std::fs::write(&stray, &stray_text).unwrap();
+        let lock = tmp.path().join(root_lock);
+        let before = std::fs::read_to_string(&lock).unwrap();
+        let case = format!("root {root_lock}, member {member_lock}");
+
+        for args in [
+            vec!["scan", "--mode", "hosted"],
+            vec!["get", UUID, "--mode", "hosted"],
+        ] {
+            let out = scrubbed_cli()
+                .args(&args)
+                .args([
+                    "--json",
+                    "--yes",
+                    "--cwd",
+                    member.to_str().unwrap(),
+                    "--api-url",
+                    &server.uri(),
+                    "--org",
+                    ORG,
+                    "--api-token",
+                    "fake",
+                ])
+                .output()
+                .expect("run socket-patch");
+            let doc: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap_or_else(|e| {
+                panic!(
+                    "{case} {args:?}: output is not JSON ({e}):\n{}\n{}",
+                    String::from_utf8_lossy(&out.stdout),
+                    String::from_utf8_lossy(&out.stderr)
+                )
+            });
+            let case = format!("{case} {args:?}");
+            assert_refused_workspace_lock_elsewhere(
+                &case,
+                out.status.code(),
+                &doc,
+                &lock,
+                &before,
+                &member,
+            );
+            let message = doc["error"].as_str().unwrap_or_default();
+            assert!(message.contains(member_lock), "{case}: {message}");
+            assert_eq!(
+                std::fs::read_to_string(&stray).unwrap(),
+                stray_text,
+                "{case}: the stray member lock is untouched"
+            );
+            assert!(!member.join(".npmrc").exists(), "{case}");
+        }
+    }
+}