diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c1e4c5509..a099edc44 100644 --- a/.github/workflows/ci.yml +++ b/.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' diff --git a/crates/socket-patch-cli/tests/e2e_redirect_pnpm_build.rs b/crates/socket-patch-cli/tests/e2e_redirect_pnpm_build.rs index 815fe0b19..c6cd5f28a 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_pnpm_build.rs +++ b/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] diff --git a/crates/socket-patch-cli/tests/e2e_redirect_rush_sim.rs b/crates/socket-patch-cli/tests/e2e_redirect_rush_sim.rs index cc1f2a79d..cf614c0f3 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_rush_sim.rs +++ b/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; } diff --git a/crates/socket-patch-cli/tests/e2e_safety_pnpm.rs b/crates/socket-patch-cli/tests/e2e_safety_pnpm.rs index 07be87fcc..3884bf944 100644 --- a/crates/socket-patch-cli/tests/e2e_safety_pnpm.rs +++ b/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(); diff --git a/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs index a158155e7..d494fa30b 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_pnpm_build.rs +++ b/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 `, or — in the pinned matrix — the provisioned pnpm diff --git a/scripts/tests/test_ci_e2e_tiers.py b/scripts/tests/test_ci_e2e_tiers.py index d4323034a..81886440b 100644 --- a/scripts/tests/test_ci_e2e_tiers.py +++ b/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"]