CI: run Windows pnpm e2e tests instead of skipping (#1054) (#1407)

* Start fix for #1054

Assisted-by: Claude Code:claude-opus-5-5

* Run Windows pnpm e2e tests instead of skipping

On windows-latest the pnpm-family e2e suites started pnpm and
corepack with a bare Command::new, which only finds .exe files.
npm and Node install both tools as .cmd shims there, so every
availability probe said the tool was missing and every test
skipped. libtest counts a skip as a pass, so the nightly Windows
legs went green in 0.02 s while testing nothing.

Add hermetic::tool_command, which resolves a tool through PATHEXT
the same way the CLI does, and use it for every pnpm, corepack and
npm spawn in e2e_safety_pnpm, e2e_redirect_pnpm_build and
e2e_redirect_rush_sim, and in common::run_toolchain.

Refs #1054

Assisted-by: Claude Code:claude-opus-5-5

* Fail Windows and macOS pnpm legs on a skip

Set pnpm_required on the Windows and macOS rows of
e2e_safety_pnpm, e2e_redirect_pnpm_build and e2e_redirect_rush_sim.
A missing pnpm or corepack now fails the leg instead of letting it
pass with every test skipped. These rows only run in the full tier
(nightly and dispatch), so PR and merge-queue runs are unchanged.

The workflow contract test now checks the gate on every OS, and a
new check stops these suites from spawning bare pnpm, corepack or
npm again.

Refs #1054

Assisted-by: Claude Code:claude-opus-5-5

* Tighten pnpm probe and shim regression guard

has_pnpm now needs `pnpm --version` to succeed, not just to spawn:
a resolved .cmd shim starts through cmd.exe even when the shim is
broken. The shim guard now also covers `.cmd` spellings, a
suite-local has_command, and the shared run_toolchain that pnpm_run
uses.

Refs #1054

Assisted-by: Claude Code:claude-opus-5-5

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: mikolalysenko

.github/workflows/ci.yml

@@ -1704,8 +1704,7 @@ jobs:
           # the matching matrix key, which the suites treat as unset.
           SOCKET_PATCH_NPM_E2E_REQUIRED: ${{ matrix.npm_required || '' }}
           # `pnpm_required` does the same for the pnpm-family suites
-          # (corepack pnpm / pnpm on PATH). Set on the Linux rows only; the
-          # Windows and macOS rows have not been checked against it yet.
+          # (corepack pnpm / pnpm on PATH), on every OS that runs them.
           SOCKET_PATCH_PNPM_E2E_REQUIRED: ${{ matrix.pnpm_required || '' }}
           SOCKET_PATCH_CARGO_E2E_REQUIRED: ${{ matrix.suite == 'e2e_safety_cargo_build' && '1' || '' }}
           SOCKET_PATCH_UV_E2E_REQUIRED: ${{ matrix.uv != '' && '1' || '' }}
@@ -1946,11 +1945,13 @@ jobs:
         include:
           - os: windows-latest
             suite: e2e_safety_pnpm
+            pnpm_required: '1'
           - os: windows-latest
             suite: e2e_redirect_npm_build
             npm_required: '1'
           - os: windows-latest
             suite: e2e_redirect_rush_sim
+            pnpm_required: '1'
           - os: windows-latest
             suite: e2e_redirect_bun_build e2e_vendor_bun_build mode_migration_bun
             bun: '1.4.2'
@@ -1963,7 +1964,7 @@ jobs:
           - {os: windows-latest, suite: e2e_safety_vlt, vlt: '1.2.0', test_filter: --include-ignored vlt_pinned_matrix, vlt_store_linker: 'hardlink'}
           - {os: windows-latest, suite: e2e_vlt, vlt: '1.2.0', test_filter: --include-ignored vlt_pinned_matrix}
           - {os: windows-latest, suite: e2e_vlt, vlt: '1.0.0-rc.14', test_filter: --include-ignored vlt_pinned_matrix}
-          - {os: windows-latest, suite: e2e_redirect_pnpm_build, test_filter: '--ignored --skip pnpm_pinned_matrix'}
+          - {os: windows-latest, suite: e2e_redirect_pnpm_build, test_filter: '--ignored --skip pnpm_pinned_matrix', pnpm_required: '1'}
           - {os: windows-latest, suite: e2e_vendor_jvm_build, jvm_tool: maven, maven: '3.9.16', test_filter: '--ignored maven_reactor'}
           - {os: windows-latest, suite: e2e_vendor_jvm_build, jvm_tool: gradle, gradle: '8.14.3', java: '17', test_filter: '--ignored gradle_multi_project'}
     runs-on: ${{ vars.DISABLE_DEPOT_RUNNERS != 'true' && (matrix.os == 'ubuntu-latest' && 'depot-ubuntu-24.04-4' || matrix.os == 'ubuntu-22.04' && 'depot-ubuntu-22.04-4') || matrix.os }}
@@ -1983,11 +1984,13 @@ jobs:
         include:
           - os: macos-latest
             suite: e2e_safety_pnpm
+            pnpm_required: '1'
           - os: macos-latest
             suite: e2e_redirect_npm_build
             npm_required: '1'
           - os: macos-latest
             suite: e2e_redirect_rush_sim
+            pnpm_required: '1'
           - os: macos-latest
             suite: e2e_redirect_bun_build
             bun: '1.4.2'
@@ -2005,7 +2008,7 @@ jobs:
           - {os: macos-latest, suite: mode_migration_vlt, vlt: '1.2.0', test_filter: --include-ignored vlt_pinned_matrix}
           - {os: macos-latest, suite: e2e_safety_vlt, vlt: '1.2.0', test_filter: --include-ignored vlt_pinned_matrix, vlt_store_linker: 'hardlink'}
           - {os: macos-latest, suite: e2e_vlt, vlt: '1.2.0', test_filter: --include-ignored vlt_pinned_matrix}
-          - {os: macos-latest, suite: e2e_redirect_pnpm_build, test_filter: '--ignored --skip pnpm_pinned_matrix'}
+          - {os: macos-latest, suite: e2e_redirect_pnpm_build, test_filter: '--ignored --skip pnpm_pinned_matrix', pnpm_required: '1'}
           - {os: macos-latest, suite: e2e_redirect_uv_build, uv: '0.12.17'}
           - {os: macos-latest, suite: e2e_vendor_pypi_build, uv: '0.12.17', test_filter: --include-ignored}
           - {os: macos-latest, suite: e2e_vex_build, test_filter: 'poetry:: --ignored', poetry: '2.4.3'}

crates/socket-patch-cli/tests/common/hermetic.rs

@@ -88,6 +88,19 @@ pub fn binary_command() -> Command {
     command(Path::new(env!("CARGO_BIN_EXE_socket-patch")))
 }
 
+/// A `Command` for the package-manager tool `name` on `PATH`, resolved with
+/// the CLI's `resolve_tool` (every `PATHEXT` extension on Windows). npm and Node install `pnpm`, `corepack` and `npm` as `.cmd`
+/// shims there, and a bare `Command::new("corepack")` only finds
+/// `corepack.exe`, so the pnpm suites' probes reported the tool missing and
+/// every Windows test skipped. A tool that isn't on `PATH` keeps the bare
+/// name, so spawning it still fails the way the probes expect.
+pub fn tool_command(name: &str) -> Command {
+    Command::new(
+        socket_patch_core::utils::process::resolve_tool(name)
+            .map_or_else(|| name.into(), std::path::PathBuf::into_os_string),
+    )
+}
+
 /// Remove every ambient `SOCKET_*` var from `cmd`, except the telemetry
 /// opt-outs (an opted-out developer stays opted out), `SOCKET_NO_CONFIG`
 /// and `SOCKET_NO_UPDATE_CHECK`. [`command`] runs this; package-manager

crates/socket-patch-cli/tests/common/mod.rs

@@ -279,7 +279,7 @@ pub fn vlt_run(cwd: &Path, vlt_js: &Path, args: &[&str], extra_env: &[(&str, &st
 }
 
 fn run_toolchain(cwd: &Path, exe: &str, args: &[&str], extra_env: &[(&str, &str)]) {
-    let mut cmd = Command::new(exe);
+    let mut cmd = hermetic::tool_command(exe);
     cmd.args(args).current_dir(cwd);
     // Sandbox the caches first so `extra_env` can still override any
     // individual one (`Command` env ops are last-write-wins per name).

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

@@ -124,7 +124,7 @@ fn pnpm_command(pm: &str) -> Command {
     if let Some(bin) = std::env::var_os("SOCKET_PATCH_PNPM_E2E_BIN") {
         Command::new(bin)
     } else {
-        let mut cmd = Command::new("corepack");
+        let mut cmd = hermetic::tool_command("corepack");
         cmd.arg(pm);
         cmd
     }

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

@@ -29,7 +29,7 @@ mod common;
 use common::binary;
 
 use std::path::Path;
-use std::process::{Command, Output, Stdio};
+use std::process::{Output, Stdio};
 
 use sha2::{Digest, Sha512};
 use wiremock::matchers::{method, path, path_regex};
@@ -82,7 +82,7 @@ fn has_corepack_pm(pm: &str) -> bool {
     };
     // Isolated too: this probe is what actually downloads the package manager
     // the first time, and corepack stores it under `COREPACK_HOME`.
-    let mut cmd = Command::new("corepack");
+    let mut cmd = hermetic::tool_command("corepack");
     cmd.args([pm, "--version"])
         .current_dir(probe.path())
         .env("COREPACK_ENABLE_DOWNLOAD_PROMPT", "0");
@@ -95,7 +95,7 @@ fn has_corepack_pm(pm: &str) -> bool {
 }
 
 fn has_command(cmd: &str) -> bool {
-    let mut probe = Command::new(cmd);
+    let mut probe = hermetic::tool_command(cmd);
     probe.arg("--version");
     cache_env::isolate(&mut probe);
     probe
@@ -107,7 +107,7 @@ fn has_command(cmd: &str) -> bool {
 }
 
 fn corepack(cwd: &Path, pm: &str, args: &[&str], extra_env: &[(&str, &str)]) -> Output {
-    let mut cmd = Command::new("corepack");
+    let mut cmd = hermetic::tool_command("corepack");
     cmd.arg(pm).args(args).current_dir(cwd);
     hermetic::scrub_socket_vars(&mut cmd);
     hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv]);
@@ -652,7 +652,7 @@ async fn rush_hosted_real_rush_update_install() {
     let rush = |args: &[&str]| -> Output {
         let mut full = vec!["x", "-y", rush_pkg.as_str()];
         full.extend_from_slice(args);
-        let mut cmd = Command::new("npm");
+        let mut cmd = hermetic::tool_command("npm");
         cmd.args(&full).current_dir(root);
         hermetic::scrub_socket_vars(&mut cmd);
         hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv]);

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

@@ -28,7 +28,7 @@ use std::path::{Path, PathBuf};
 mod common;
 
 use common::{
-    assert_run_ok, git_sha256, git_sha256_file, has_command, pnpm_run, write_blob,
+    assert_run_ok, git_sha256, git_sha256_file, hermetic, pnpm_run, write_blob,
     write_minimal_manifest, write_package_json, PatchEntry,
 };
 
@@ -45,6 +45,22 @@ fn pnpm_required() -> bool {
     std::env::var_os("SOCKET_PATCH_PNPM_E2E_REQUIRED").is_some_and(|v| !v.is_empty())
 }
 
+/// `pnpm --version` succeeds. Probed through [`hermetic::tool_command`]: on
+/// Windows pnpm is the `pnpm.cmd` shim `npm install -g` writes, which
+/// `common::has_command`'s bare `Command::new` never finds. The exit status
+/// counts too, since a resolved `.cmd` spawns through `cmd.exe` even when
+/// the shim itself is broken.
+fn has_pnpm() -> bool {
+    let mut probe = hermetic::tool_command("pnpm");
+    probe.arg("--version");
+    common::cache_env::isolate(&mut probe);
+    probe
+        .stdout(std::process::Stdio::null())
+        .stderr(std::process::Stdio::null())
+        .status()
+        .is_ok_and(|s| s.success())
+}
+
 /// Print the SKIP line for a missing pnpm, or panic under [`pnpm_required`].
 fn skip_missing_pnpm() {
     assert!(
@@ -249,7 +265,7 @@ fn file_identity(path: &Path) -> (u64, u64) {
 #[test]
 #[ignore]
 fn pnpm_install_produces_symlinked_layout() {
-    if !has_command("pnpm") {
+    if !has_pnpm() {
         skip_missing_pnpm();
         return;
     }
@@ -313,7 +329,7 @@ fn pnpm_install_produces_symlinked_layout() {
 #[test]
 #[ignore]
 fn apply_in_a_does_not_mutate_b_or_store() {
-    if !has_command("pnpm") {
+    if !has_pnpm() {
         skip_missing_pnpm();
         return;
     }
@@ -417,7 +433,7 @@ fn apply_in_a_does_not_mutate_b_or_store() {
 #[test]
 #[ignore]
 fn pnpm_install_in_b_does_not_revert_a() {
-    if !has_command("pnpm") {
+    if !has_pnpm() {
         skip_missing_pnpm();
         return;
     }
@@ -532,7 +548,7 @@ fn pnpm_install_in_b_does_not_revert_a() {
 #[test]
 #[ignore]
 fn apply_in_pnpm_project_emits_layout_note() {
-    if !has_command("pnpm") {
+    if !has_pnpm() {
         skip_missing_pnpm();
         return;
     }

scripts/tests/test_ci_e2e_tiers.py

@@ -275,8 +275,8 @@ def test_compat_workflow_builds_its_own_binaries(self):
 
 class PnpmRequiredGate(unittest.TestCase):
     """The pnpm-family suites soft-skip when pnpm is missing, and libtest
-    counts a skip as `ok`, so their Linux rows must set the fail-instead-of-
-    skip gate (#1054)."""
+    counts a skip as `ok`, so every row that runs them, on every OS, must set
+    the fail-instead-of-skip gate (#1054)."""
 
     suites = {"e2e_redirect_pnpm_build", "e2e_safety_pnpm", "e2e_redirect_rush_sim"}
     jobs = ("e2e", "e2e-full", "e2e-extended", "e2e-windows", "e2e-macos")
@@ -285,20 +285,42 @@ def test_run_step_reads_the_row_key(self):
         run = dict(rows_mod.steps(JOBS["e2e"]))["Run e2e tests"]
         self.assertIn("SOCKET_PATCH_PNPM_E2E_REQUIRED: ${{ matrix.pnpm_required || '' }}", run)
 
-    def test_every_linux_pnpm_row_sets_the_gate(self):
-        gated = set()
+    def test_every_pnpm_row_sets_the_gate(self):
+        gated = {}
         for job in self.jobs:
             for row in rows(job):
-                if self.suites & set(row["suite"].split()) and row["os"].startswith("ubuntu"):
+                if self.suites & set(row["suite"].split()):
                     with self.subTest(job=job, row=row):
                         self.assertEqual(row.get("pnpm_required"), "1")
-                    gated |= self.suites & set(row["suite"].split())
+                    for suite in self.suites & set(row["suite"].split()):
+                        gated.setdefault(suite, set()).add(row["os"].split("-")[0])
                 elif "pnpm_required" in row:
                     self.fail(f"{job} row outside the checked set sets pnpm_required: {row}")
-        self.assertEqual(gated, self.suites)
+        self.assertEqual(gated, {s: {"ubuntu", "windows", "macos"} for s in self.suites})
         self.assertIn("e2e_redirect_pnpm_build", {r["suite"] for r in rows("e2e") if r.get("pnpm_required")},
                       "the PR tier must keep its gated pnpm row")
 
+    def test_suites_resolve_windows_shims(self):
+        # A bare `Command::new("pnpm")` / `Command::new("corepack")` can't
+        # spawn the `.cmd` shims npm and Node install on Windows, so every
+        # Windows test skipped (passed in 0.02 s) until the gate made that a
+        # failure. `hermetic::tool_command` resolves them through PATHEXT.
+        for suite in sorted(self.suites):
+            with self.subTest(suite=suite):
+                src = (TESTS / f"{suite}.rs").read_text(encoding="utf-8")
+                self.assertNotRegex(src, r'Command::new\("(pnpm|corepack|npm)(\.cmd)?"\)')
+                if "fn has_command(" in src:
+                    # A suite-local probe must resolve shims itself...
+                    local = src[src.index("fn has_command("):]
+                    self.assertIn("hermetic::tool_command(cmd)", local[:local.index("\n}\n")])
+                else:
+                    # ...and `common::has_command` does not.
+                    self.assertNotRegex(src, r'has_command\("(pnpm|corepack|npm)"\)')
+        # `pnpm_run` goes through the shared runner.
+        common = (TESTS / "common" / "mod.rs").read_text(encoding="utf-8")
+        runner = common[common.index("fn run_toolchain("):]
+        self.assertIn("hermetic::tool_command(exe)", runner[:runner.index("\n}\n")])
+
     def test_every_gated_suite_honours_the_gate(self):
         for suite in sorted(self.suites):
             with self.subTest(suite=suite):