CI: fail pnpm e2e legs that skip every test (#1054) (#1400)

* CI: fail pnpm e2e legs that skip every test

The pnpm-family e2e suites return early with a SKIP line when pnpm
or corepack is missing, and libtest reports those tests as passed.
Their CI rows set no gate, so a runner without pnpm would turn the
leg green without running anything; with corepack hidden, the PR
row's 8 tests "passed" in 0.02s.

The Linux rows of e2e_redirect_pnpm_build, e2e_safety_pnpm and
e2e_redirect_rush_sim now set SOCKET_PATCH_PNPM_E2E_REQUIRED, and
the three suites fail instead of skipping under it. A bare major
pin (pnpm@10) now passes the version check when the installed
release is inside that major. Empty values count as unset, since
the shared step passes '' on every other leg. A script test pins
the gate to those rows and suites.

Refs #1054

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

* CI: gate pnpm rows with a matrix key

Review follow-up. The pnpm gate is now a per-row pnpm_required key,
like npm_required, instead of an exact-suite-name match in the
shared step, so a multi-suite row can't silently lose it.

Also: e2e_safety_pnpm uses one skip helper, rush_sim's corepack
probe doc is back on its function, the pnpm_build docs no longer
call the non-@10 legs opportunistic, and e2e_vendor_pnpm_build
treats an empty gate as unset like its siblings. The script test
now checks the row key and that each suite reads the gate.

Refs #1054

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

---------

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

Author: mikolalysenko

.github/workflows/ci.yml

@@ -1259,7 +1259,7 @@ jobs:
           # inside the same suite runs in pnpm-compatibility.yml, hence the
           # skip. Node 24 (step below): the
           # corepack pnpm@10/11 legs require it.
-          - {os: ubuntu-latest, suite: e2e_redirect_pnpm_build, test_filter: '--ignored --skip pnpm_pinned_matrix'}
+          - {os: ubuntu-latest, suite: e2e_redirect_pnpm_build, test_filter: '--ignored --skip pnpm_pinned_matrix', pnpm_required: '1'}
           # Real-uv hosted/vendored capstones ending in manifest-less VEX:
           # the oldest and newest line + the 0.5.x boundary (0.5.4 still
           # re-resolves a transitive override / rejects a repointed
@@ -1699,6 +1699,10 @@ jobs:
           # other real-toolchain capstone. All EMPTY on legs that do not set
           # 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.
+          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' || '' }}
           SOCKET_PATCH_UV_E2E_VERSION: ${{ matrix.uv }}
@@ -1829,8 +1833,10 @@ jobs:
           - {os: ubuntu-latest, suite: e2e_redirect_gem_build e2e_vendor_gem_build, ruby: '3.4', bundler: '4.0.15'}
           - os: ubuntu-latest
             suite: e2e_safety_pnpm
+            pnpm_required: '1'
           - os: ubuntu-latest
             suite: e2e_redirect_rush_sim
+            pnpm_required: '1'
           - os: ubuntu-latest
             suite: e2e_redirect_bun_build e2e_vendor_bun_build mode_migration_bun
             bun: '1.1.45'

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

@@ -93,10 +93,10 @@ const TOKEN: &str = "22222222-2222-4222-8222-222222222222";
 /// Marker prepended to the dep's entry point by the synthetic patch.
 const MARKER: &str = "/* SOCKET-PATCHED */\n";
 const GHSA: &str = "GHSA-redirect-pnpm";
-/// Pinned pnpm majors via corepack — @10 is the required leg, the others are
-/// opportunistic (the vendor capstone's ladder convention). @7 and @8 are the
-/// legacy-lock legs: they emit lockfileVersion 5.4 / 6.0, proving the v5/v6
-/// rewrite installs for real.
+/// Pinned pnpm majors via corepack. Locally a leg whose major corepack can't
+/// provide skips; under `SOCKET_PATCH_PNPM_E2E_REQUIRED` (the Linux CI row)
+/// every leg is required. @7 and @8 are the legacy-lock legs: they emit
+/// lockfileVersion 5.4 / 6.0, proving the v5/v6 rewrite installs for real.
 const PNPM_PRIMARY: &str = "pnpm@10";
 const PNPM_SECONDARY: &str = "pnpm@9";
 const PNPM_TERTIARY: &str = "pnpm@11";
@@ -130,6 +130,13 @@ fn pnpm_command(pm: &str) -> Command {
     }
 }
 
+/// Set by every CI leg that runs this suite: a missing toolchain or a
+/// failed fixture install is then a failure, never a skip. The workflow
+/// passes an EMPTY value on legs that run other suites, so empty = unset.
+fn pnpm_required() -> bool {
+    std::env::var_os("SOCKET_PATCH_PNPM_E2E_REQUIRED").is_some_and(|v| !v.is_empty())
+}
+
 fn has_corepack_pm(pm: &str) -> bool {
     let probe = tempfile::tempdir().unwrap();
     let mut cmd = pnpm_command(pm);
@@ -138,13 +145,16 @@ fn has_corepack_pm(pm: &str) -> bool {
     cmd.env("COREPACK_ENABLE_DOWNLOAD_PROMPT", "0");
     let output = cmd.output();
     let ok = output.as_ref().is_ok_and(|o| o.status.success());
-    if std::env::var_os("SOCKET_PATCH_PNPM_E2E_REQUIRED").is_some() {
+    if pnpm_required() {
         assert!(ok, "required pnpm toolchain unavailable: {output:?}");
         if let Ok(output) = output {
-            assert_eq!(
-                String::from_utf8_lossy(&output.stdout).trim(),
-                pm.strip_prefix("pnpm@").unwrap(),
-                "matrix must run the pinned version"
+            // An exact pin (the pinned matrix) must match exactly; a bare
+            // major (`pnpm@10`, the e2e legs) must resolve inside it.
+            let got = String::from_utf8_lossy(&output.stdout).trim().to_string();
+            let want = pm.strip_prefix("pnpm@").unwrap();
+            assert!(
+                got == want || got.starts_with(&format!("{want}.")),
+                "matrix must run the pinned version: want {want}, got {got}"
             );
         }
     }
@@ -523,7 +533,7 @@ async fn redirect_scanned_pnpm_project(
     );
     if !install.status.success() {
         assert!(
-            std::env::var_os("SOCKET_PATCH_PNPM_E2E_REQUIRED").is_none(),
+            !pnpm_required(),
             "required {pm} fixture install failed: {:?}",
             install
         );
@@ -1136,7 +1146,7 @@ async fn pnpm_get_uuid_hosted_fresh_checkout_frozen_install() {
     assert_fixture_manifestless_vex(&fx, &fresh, "pnpm10 get-uuid");
 }
 
-/// Opportunistic pnpm@9 leg (the vendor capstone's secondary convention):
+/// pnpm@9 leg (the vendor capstone's secondary convention):
 /// same positive chain, no tamper twin needed — @10 already carries it.
 #[tokio::test(flavor = "multi_thread")]
 #[serial_test::serial]

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

@@ -59,6 +59,20 @@ const VULNS: &[(&str, &[&str])] = &[(GHSA, &["CVE-2026-5151"])];
 
 // ── self-contained helpers ────────────────────────────────────────────
 
+/// Set by the CI legs that run this suite: a missing corepack pnpm is then
+/// a failure, never a skip. Empty (the value on other legs) = unset. The
+/// tier-2 `RUSH_E2E` opt-in skip is not affected.
+fn pnpm_required() -> bool {
+    std::env::var_os("SOCKET_PATCH_PNPM_E2E_REQUIRED").is_some_and(|v| !v.is_empty())
+}
+
+/// Print the SKIP line for a missing toolchain, or panic under
+/// [`pnpm_required`].
+fn skip_missing_toolchain(msg: &str) {
+    assert!(!pnpm_required(), "required toolchain unavailable: {msg}");
+    println!("SKIP {msg}");
+}
+
 /// Probe corepack from a NEUTRAL temp dir (a `packageManager` field in an
 /// ancestor package.json — e.g. this monorepo root — otherwise makes corepack
 /// refuse a different manager).
@@ -400,7 +414,7 @@ fn assert_rush_manifestless_vex(root: &Path, patch_server: &str, patched: &[u8],
 #[ignore = "wall-bound real-rush/pnpm install (~70s); runs on all 3 OSes as an e2e CI matrix leg"]
 async fn rush_hosted_scan_then_simulated_pnpm_install_lands_patched_bytes() {
     if !has_corepack_pm("pnpm@9") {
-        println!("SKIP e2e_redirect_rush_sim: `corepack pnpm@9` unavailable");
+        skip_missing_toolchain("e2e_redirect_rush_sim: `corepack pnpm@9` unavailable");
         return;
     }
 
@@ -516,7 +530,7 @@ fn assert_rush_stale_store_not_attested(
 #[ignore = "wall-bound real-rush/pnpm install (~70s); runs on all 3 OSes as an e2e CI matrix leg"]
 async fn rush_hosted_tampered_tarball_fails_simulated_install() {
     if !has_corepack_pm("pnpm@9") {
-        println!("SKIP e2e_redirect_rush_sim (tampered): `corepack pnpm@9` unavailable");
+        skip_missing_toolchain("e2e_redirect_rush_sim (tampered): `corepack pnpm@9` unavailable");
         return;
     }
 
@@ -581,7 +595,7 @@ async fn rush_hosted_real_rush_update_install() {
         return;
     }
     if !has_command("npm") || !has_corepack_pm("pnpm@9") {
-        println!("SKIP e2e_redirect_rush_sim (tier2): npm / corepack pnpm@9 unavailable");
+        skip_missing_toolchain("e2e_redirect_rush_sim (tier2): npm / corepack pnpm@9 unavailable");
         return;
     }
 

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

@@ -39,6 +39,21 @@ const PATCH_UUID: &str = "00000000-0000-4000-8000-00000000e2e5";
 /// Pinned: published npm tarballs are immutable.
 const BEFORE_HASH: &str = "311f1e893e6eac502693fad8617dcf5353a043ccc0f7b4ba9fe385e838b67a10";
 
+/// Set by the CI legs that run this suite: pnpm missing from PATH is then a
+/// failure, never a skip. Empty (the value on other legs) = unset.
+fn pnpm_required() -> bool {
+    std::env::var_os("SOCKET_PATCH_PNPM_E2E_REQUIRED").is_some_and(|v| !v.is_empty())
+}
+
+/// Print the SKIP line for a missing pnpm, or panic under [`pnpm_required`].
+fn skip_missing_pnpm() {
+    assert!(
+        !pnpm_required(),
+        "required toolchain unavailable: pnpm not on PATH"
+    );
+    eprintln!("SKIP: pnpm not on PATH");
+}
+
 /// Appended to the pristine `index.js` to form the patched bytes.
 const PATCH_MARKER: &[u8] = b"\n// socket-patch e2e_safety_pnpm synthetic patch\n";
 
@@ -235,7 +250,7 @@ fn file_identity(path: &Path) -> (u64, u64) {
 #[ignore]
 fn pnpm_install_produces_symlinked_layout() {
     if !has_command("pnpm") {
-        eprintln!("SKIP: pnpm not on PATH");
+        skip_missing_pnpm();
         return;
     }
     let root = tempfile::tempdir().unwrap();
@@ -299,7 +314,7 @@ fn pnpm_install_produces_symlinked_layout() {
 #[ignore]
 fn apply_in_a_does_not_mutate_b_or_store() {
     if !has_command("pnpm") {
-        eprintln!("SKIP: pnpm not on PATH");
+        skip_missing_pnpm();
         return;
     }
     let root = tempfile::tempdir().unwrap();
@@ -403,7 +418,7 @@ fn apply_in_a_does_not_mutate_b_or_store() {
 #[ignore]
 fn pnpm_install_in_b_does_not_revert_a() {
     if !has_command("pnpm") {
-        eprintln!("SKIP: pnpm not on PATH");
+        skip_missing_pnpm();
         return;
     }
     let root = tempfile::tempdir().unwrap();
@@ -518,7 +533,7 @@ fn pnpm_install_in_b_does_not_revert_a() {
 #[ignore]
 fn apply_in_pnpm_project_emits_layout_note() {
     if !has_command("pnpm") {
-        eprintln!("SKIP: pnpm not on PATH");
+        skip_missing_pnpm();
         return;
     }
     let root = tempfile::tempdir().unwrap();

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

@@ -108,9 +108,10 @@ fn socket_bin() -> PathBuf {
 }
 
 /// Set by the pinned-matrix workflow: a missing toolchain or a failed
-/// fixture install is then a failure, never a skip.
+/// fixture install is then a failure, never a skip. Empty = unset, as in the
+/// sibling suites (ci.yml's shared e2e step passes '' on other legs).
 fn pnpm_required() -> bool {
-    std::env::var_os("SOCKET_PATCH_PNPM_E2E_REQUIRED").is_some()
+    std::env::var_os("SOCKET_PATCH_PNPM_E2E_REQUIRED").is_some_and(|v| !v.is_empty())
 }
 
 /// `corepack <pm>`, or — in the pinned matrix — the provisioned pnpm

scripts/tests/test_ci_e2e_tiers.py

@@ -271,6 +271,42 @@ def test_compat_workflow_builds_its_own_binaries(self):
         self.assertIn("gradle-probe", text)
 
 
+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)."""
+
+    suites = {"e2e_redirect_pnpm_build", "e2e_safety_pnpm", "e2e_redirect_rush_sim"}
+    jobs = ("e2e", "e2e-full", "e2e-extended", "e2e-windows", "e2e-macos")
+
+    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()
+        for job in self.jobs:
+            for row in rows(job):
+                if self.suites & set(row["suite"].split()) and row["os"].startswith("ubuntu"):
+                    with self.subTest(job=job, row=row):
+                        self.assertEqual(row.get("pnpm_required"), "1")
+                    gated |= self.suites & set(row["suite"].split())
+                elif "pnpm_required" in row:
+                    self.fail(f"{job} row outside the checked set sets pnpm_required: {row}")
+        self.assertEqual(gated, 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_every_gated_suite_honours_the_gate(self):
+        for suite in sorted(self.suites):
+            with self.subTest(suite=suite):
+                src = (TESTS / f"{suite}.rs").read_text(encoding="utf-8")
+                # Empty means unset: the step passes '' on every other leg.
+                self.assertIn('var_os("SOCKET_PATCH_PNPM_E2E_REQUIRED").is_some_and(|v| !v.is_empty())', src)
+                # ...and every skip path goes through it.
+                self.assertGreater(len(re.findall(r"pnpm_required\(\)", src)), 1, "gate defined but never read")
+
+
 class PdmCapstone(unittest.TestCase):
     job = rows_mod.jobs(PDM.read_text(encoding="utf-8"))["capstone"]