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

[Mikola Lysenko (mikolalysenko)]

LLM Description written by Claude Code:claude-opus-5-5

Refs #1054. The Linux rows are fixed here. The issue stays open for the Windows and macOS pnpm-family rows (full tier only), which I couldn't check against the gate from a Linux sandbox.

Problem

The pnpm-family e2e suites return early with a SKIP line when pnpm or corepack is missing, and libtest reports those tests as passed. The e2e step's passed > 0 guard (ci.yml:1759) can't tell a skip from a real pass. Every other non-vlt e2e row already sets a *_REQUIRED gate, but these three rows set none:

• e2e_redirect_pnpm_build (PR tier, ci.yml:1262)
• e2e_safety_pnpm, e2e_redirect_rush_sim (full tier)

So a runner without pnpm would turn those legs green without testing anything. Today they do run: merge-queue run 38170303367 ran all 8 pnpm tests.

Change

• New per-row matrix key pnpm_required: '1', set on the Linux rows of the three suites. The run step passes it on as SOCKET_PATCH_PNPM_E2E_REQUIRED: ${{ matrix.pnpm_required || '' }}, the same pattern as npm_required.
• The three suites now fail instead of skipping under the gate. e2e_redirect_pnpm_build already read the variable for the (disabled) pinned-matrix workflow, but only accepted an exact version. A bare major pin (pnpm@10) now passes when the installed release is inside that major. Exact pins still have to match exactly.
• An empty value counts as unset, since the shared step passes '' on every other leg. e2e_vendor_pnpm_build (not in ci.yml) gets the same rule so the four pnpm suites agree.
• scripts/tests/test_ci_e2e_tiers.py::PnpmRequiredGate checks the step reads the key, that every Linux row running these suites sets it (and no other row does), and that each suite defines and reads the gate.
• The rush tier-2 RUSH_E2E=1 opt-in skip is unchanged.
• Under the gate, the PR row now requires all five corepack majors (7–11), not only Anson (@10). Merge-queue run 38170303367 shows all five already install there.

Runner usage

No job, step or runner change. A lean PR run keeps the same 28 Linux jobs and about the same job-minutes before and after. The pnpm leg already does the real installs, so the gate adds no time.

Test evidence

Hiding corepack/pnpm (PATH=/usr/bin:/bin:~/.cargo/bin):

suite (--ignored)	gate unset	gate =1
e2e_redirect_pnpm_build (PR filter)	8 passed in 0.02s (all skipped)	8 failed
e2e_redirect_rush_sim	3 passed in 0.01s	2 failed, 1 passed (the tier-2 RUSH_E2E opt-in)
e2e_safety_pnpm	4 passed in 0.00s	4 failed

With the real toolchain (node 22 corepack, pnpm on PATH) and the gate set: pnpm_build 8 passed (26s), rush_sim 3 passed (73s), safety_pnpm 4 passed (9s).

• python3 -m pytest scripts/tests -q: 334 passed, 6 skipped. The new PnpmRequiredGate tests fail on origin/main (4 failed) and pass here.
• actionlint on ci.yml: the new matrix.pnpm_required read adds the same "property is not defined" warning that every per-row key in this anchor-shared step already gets (npm_required, sbt, pip, ...): 3 more of the 165 existing. zizmor --offline: same 59 findings as origin/main.
• cargo fmt --all -- --check: clean. cargo clippy --locked --workspace --all-features -- -D warnings: clean. Clippy on the pnpm_build and safety_pnpm test targets: no new lints. The rush_sim test target has a pre-existing duplicate_mod lint on origin/main; CI doesn't lint test targets.

/code-review high

9 findings, none a correctness bug. Fixed: exact-suite-name gate replaced by the pnpm_required row key, docs that still called the non-Anson (@10) legs opportunistic, rush_sim's displaced doc comment, the four pasted skip blocks in e2e_safety_pnpm (now one helper), e2e_vendor_pnpm_build's is_some() disagreeing on empty values, and the brittle script-test checks.
Not fixed:

• The full-tier rush_sim and safety_pnpm rows aren't exercised by this PR's CI. Last nightly (run 38115955529) ran rush_sim on Linux with real corepack pnpm@9 work (71s), and both suites pass locally under the gate.
• rush_sim's corepack probe discards stderr, so the panic gives no detail. That probe predates this PR.
• Sharing pnpm_required() through tests/common/. That belongs to the Tracking: share CLI test helpers through one test-support module instead of 100+ per-file copies #824 helper-dedup tracker.

Follow-up (stays on #1054)

Gate the Windows and macOS rows of the same suites once a full-scope run confirms corepack/pnpm is present there.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Q7te2UYzmfvjJPqfjfGazq

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 21dcac0. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review at 21dcac0.

• CI: 37/37 non-skipped checks green on the head (50 skipped by path filters); no main-wide failures.
• Bugbot: reviewed 21dcac0, no findings.
• Reviewer focus: the pnpm-family e2e rows now fail rather than skip when pnpm is missing, so a CI leg without pnpm can no longer report green while running nothing.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. Three pnpm-family e2e suites print SKIP and return when pnpm/corepack is missing, and libtest counts that as a pass, so their CI legs could go green without testing anything. This adds a pnpm_required matrix key on their Linux rows, passed through as SOCKET_PATCH_PNPM_E2E_REQUIRED, which turns every such skip into a failure. Test-and-CI-only change; no product code touched.

Risk: low. Only test files, ci.yml and a script test change. The main effect is stricter: under the gate the PR-tier pnpm row now needs all five corepack majors (7–11), so a corepack download failure there fails the leg instead of skipping silently. That's the intent, but it could show up as a new kind of flake.

Look here

• ci.yml:1705: the shared step passes the key, empty on every other leg.
• ci.yml:1262, 1836, 1839: the three Linux rows that set it (Windows/macOS rows deliberately not gated yet; Make non-vlt e2e legs fail when every test soft-skips #1054 stays open for them).
• e2e_redirect_pnpm_build.rs:148-158:`` the version check now accepts a bare major (pnpm@10 → `10.x.y`), and exact pins still have to match exactly. `starts_with("{want}.")` won't let `1` match `10.x`.
• e2e_redirect_rush_sim.rs:62-74:``` the shared skip helper. The RUSH_E2E opt-in skip is unchanged.
• test_ci_e2e_tiers.py:274: guards that only those Linux rows set the key and every suite reads it.

Verified

• Read the full diff (6 files, +103/−21). Empty-means-unset is now the same in all four pnpm suites, every SKIP path in the three gated suites goes through the gate, and nothing outside the gated rows sets the key.
• CI: ci-ok and clippy green on 21dcac0. The PR-tier leg e2e (ubuntu-latest, e2e_redirect_pnpm_build, …, 1) passed with the gate on, so all 8 tests really ran (a skip would have panicked).
• Bugbot: success, no findings.
• Locally: python3 -m unittest scripts.tests.test_ci_e2e_tiers (22 OK), rustfmt --check on the four touched test files is clean, and cargo check -p socket-patch-cli --tests is clean.
• CHANGELOG.md untouched.

Changes I made: none.

Open questions: none blocking. The full-tier rows (e2e_safety_pnpm, e2e_redirect_rush_sim) only run nightly, so the gate's first real test there is the next nightly.

Auto-merge (squash) is armed, so approving sends it straight to the merge queue.

---

Generated by Claude Code