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

[Mikola Lysenko (mikolalysenko)]

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

Refs #1054 (the Windows/macOS slice; #1400 gated the Linux rows). Once this lands, every row of the three pnpm-family suites is gated, so the issue can close.

Root cause

On windows-latest the three pnpm-family e2e suites start pnpm and corepack with a bare Command::new("pnpm") / Command::new("corepack"). Rust's Command only looks for <name>.exe on PATH, while npm and Node install these tools as .cmd shims. Every availability probe therefore says the tool is missing and every test skips, and libtest still counts a skip as passed. From nightly run 38115955529:

suite	Windows	macOS
e2e_safety_pnpm	4 passed in 0.02 s	4 passed in 2.90 s
e2e_redirect_pnpm_build	8 passed in 0.02 s	8 passed in 14.42 s
e2e_redirect_rush_sim	3 passed in 1.01 s	3 passed in 73.59 s

So the Windows pnpm legs have tested nothing, even though the workflow installs pnpm on them. The macOS legs do run the tests.

What it does

• hermetic::tool_command(name) (tests only) resolves a tool through socket_patch_core::utils::process::resolve_tool, the same PATHEXT-aware lookup the CLI uses and that npm_e2e_common already relies on. If the tool isn't on PATH it keeps the bare name, so spawning still fails the way the probes expect.
• e2e_safety_pnpm, e2e_redirect_pnpm_build and e2e_redirect_rush_sim spawn every pnpm / corepack / npm through it, as does common::run_toolchain. pnpm_run uses run_toolchain, and so do npm_run and the other *_run helpers, so suites like e2e_npm / e2e_scan now resolve npm the same way. Nothing changes on Linux or macOS. On Windows a tool that used to fail to spawn now starts, and a probe that skips the test still runs before it, as before. e2e_safety_pnpm gets its own resolved has_pnpm() probe instead of common::has_command.
• ci.yml: pnpm_required: '1' on the 3 Windows and 3 macOS rows of these suites, so a missing toolchain fails the leg instead of passing it.
• scripts/tests/test_ci_e2e_tiers.py: the gate test now requires the gate on every OS (ubuntu, windows, macos) for all three suites. A new check fails if any of the three suites spawns a bare pnpm, corepack or npm again.

Left out on purpose: common::has_command keeps its bare lookup. It's used by about 12 other suites (npm, yarn, bun, bundle, ...), and switching them all would un-skip Windows tests nobody has checked yet. That needs its own follow-up and its own full-scope run.

Evidence

• Python contract tests: python3 -m pytest scripts/tests -q gives 335 passed, 6 skipped. Run against origin/main's ci.yml and suites, the new PnpmRequiredGate tests fail (15 subtests: the 6 Windows/macOS rows and the bare spawns). On this branch they pass.

• Linux, with SOCKET_PATCH_PNPM_E2E_REQUIRED=1: e2e_safety_pnpm 4 passed (2.86 s), e2e_redirect_pnpm_build --ignored --skip pnpm_pinned_matrix 8 passed (25.4 s), e2e_redirect_rush_sim --ignored 3 passed (72.0 s). These are real runs, not skips.

• spawn_env_hygiene 13 passed. cargo fmt --all -- --check and cargo clippy --workspace --all-features -- -D warnings are clean.

• actionlint and zizmor --offline report the same findings on ci.yml as on origin/main: none new.

• Windows/macOS proof: one full-scope run on c8f514e, 38191238691 (dispatched with hosted_e2e=skip; c28558a only tightens the probe and the Python guard). All 6 gated Windows/macOS legs passed, and on Windows the tests now really run:

  Windows leg (gated)	before (nightly 38115955529)	after (38191238691)
  e2e_safety_pnpm	4 passed in 0.02 s (skipped)	4 passed in 5.73 s
  e2e_redirect_pnpm_build	8 passed in 0.02 s (skipped)	8 passed in 93.25 s
  e2e_redirect_rush_sim	3 passed in 1.01 s (skipped)	3 passed in 80.79 s

  The 3 gated macOS legs passed as well. The run's only red job, test (macos-latest, 1), is a Go unit test that is already failing on main (from Fix Go replaces that go.work ignores (#393, #458, #531) #1393), unrelated to this PR. Filed as Go go.work audit: false NotAMember for a use path through a missing dir on macOS (breaks macOS test job) #1409.

• PR CI on c28558a: ci-ok and every other check green. Bugbot found no issues.

/code-review high

Fixed in c28558a:

• has_pnpm now also requires pnpm --version to exit 0. On Windows a resolved .cmd shim runs through cmd.exe, so a successful spawn alone proved nothing.
• The tool_command doc comment no longer claims full parity with the CLI. The CLI also has an App Execution Alias fallback, which this helper doesn't need.
• The shim guard is wider: it now catches .cmd spellings, checks rush_sim's local has_command, and checks the shared run_toolchain that pnpm_run uses.

Not fixed here, on purpose (follow-up on #1054 / #824):

• Make common::has_command resolve shims for every caller, and drop the local has_pnpm and rush_sim has_command copies. About 12 suites probe npm / yarn / bun / bundle through it. On Windows that would un-skip tests (e2e_npm, e2e_scan, e2e_hosted_production, e2e_vendored_production, ...) that have never run there. They need their own full-scope run and probably their own fixes.
• The same bare corepack / npm spawns in other suites (e2e_vendor_pnpm_build, mode_migration_npm, e2e_vendored_production, yarn_classic_vex, e2e_vendor_yarn_classic_dev_flow, in_process_alternate_installers). None of them runs in a Windows CI row today, so they skip only on a developer's Windows machine. Same follow-up.
• Fold npm_e2e_common::path_npm and yarn_berry_common::corepack_command into hermetic::tool_command. A helper cleanup, out of scope here.

CI cost

The changed rows run only in the full tier (schedule / dispatch / CI_SCOPE=full). A lean PR or merge-queue run is unchanged: same ~28 Linux jobs, about 30 job-minutes, about 8 min wall time. In the nightly, the 3 Windows legs will now run their tests: about +0.5 min (safety), +0.3 min (pnpm build) and +1.2 min (rush sim), based on the macOS timings. Measured: about +3 Windows job-minutes a night. No job is added.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GMkBLzuHJrRmvDmWU3neVb

---

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 c28558a. Configure here.}

[Mikola Lysenko (mikolalysenko)]

[agent] The full-scope run on c8f514e (38191238691) has one red job: test (macos-latest, 1). It is not caused by this PR. The failing test is vendor::go_consumer_sync::tests::go_work_that_does_not_use_the_project_makes_the_replace_inert, which #1393 added to socket-patch-core after the last nightly. It fails on main on macOS because the /var symlink breaks path matching for a use path that goes through a missing directory. This PR changes no socket-patch-core code. Every other job in the run passed, including all 6 newly gated Windows/macOS pnpm legs. I filed #1409 with a Linux repro and a verified one-line patch. It's a Go product fix, so it stays out of this PR. I didn't re-run the job because the failure is deterministic. The PR's required checks on c28558a are green, and Bugbot found no issues.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Ready for review (burn-down agent)

• Head: c28558a
• CI: all non-skipped checks on this head green (path-filtered jobs skipped); mergeable, no conflicts.
• Bugbot: reviewed c28558a, no findings; no open review threads.
• Main-wide failure (not this PR's): test (macos-latest, 1) in the full-scope run fails on go_work_that_does_not_use_the_project_makes_the_replace_inert on main too; tracked in Go go.work audit: false NotAMember for a use path through a missing dir on macOS (breaks macOS test job) #1409, fix in Fix macOS go.work member match through ./x/../app #1411.
• Reviewer focus: the Windows pnpm/corepack e2e legs now resolve .cmd shims through tests/common/hermetic.rs and actually run instead of silently skipping (Make non-vlt e2e legs fail when every test soft-skips #1054); ci.yml gates the new Windows/macOS legs.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief (final reviewer agent)

What it does. On Windows, the pnpm-family e2e suites spawned pnpm/corepack/npm by bare name. Rust's Command only finds .exe that way, and these tools are .cmd shims, so the tests silently skipped. The PR adds a test-only hermetic::tool_command that resolves the tool through the core crate's PATHEXT-aware resolve_tool, and sets pnpm_required: '1' on the 6 Windows/macOS rows, so a missing toolchain now fails the leg instead of passing.

Risk: low. The changes are test-only plus matrix keys in ci.yml. No jobs are added, path filters and ci-ok aggregation are unchanged, and the gated rows run only in the full tier.

Look here

• tests/common/hermetic.rs:91-102: tool_command. It falls back to the bare name, so the existing probe/skip logic still applies.
• e2e_safety_pnpm.rs:48-62: the probe now checks the exit status as well as the spawn. This is needed because a .cmd always spawns via cmd.exe.
• tests/common/mod.rs:282: run_toolchain now resolves as well. This also affects npm_run, so e2e_npm/e2e_scan resolve npm too. That's harmless on Linux/macOS and can only help on Windows, but the description doesn't mention it.
• ci.yml:1944-2007: the 6 new gates. test_ci_e2e_tiers.py:286-320 pins them and forbids bare spawns again.

Verified

• Read the full diff and resolve_tool (PATHEXT order, so .exe/.cmd win over .ps1 and npm's extensionless sh shim is never picked).
• scripts/tests/test_ci_e2e_tiers.py: 23/23 pass.
• cargo clippy -D warnings is clean on the changed suites e2e_safety_pnpm and e2e_redirect_pnpm_build. On e2e_redirect_rush_sim it reports 2 duplicate_mod errors, on mod lines this PR doesn't touch. CI's clippy job is green.
• cargo fmt --check is clean, and CHANGELOG.md is untouched.
• CI: 37/37 non-skipped checks green (50 path-skipped), including ci-ok and clippy. Bugbot is green with no findings, and there are no open threads.
• I did not run the e2e suites locally. The description cites full-scope run 38191238691, where all 6 newly gated legs passed. Its one red macOS job is the main-wide Go failure tracked in Go go.work audit: false NotAMember for a use path through a missing dir on macOS (breaks macOS test job) #1409 / Fix macOS go.work member match through ./x/../app #1411.

Changes I made: none.

Open questions (non-blocking)

• rush_hosted_real_rush_update_install still skips on every OS, because CI never sets RUSH_E2E. This predates the PR.
• The Windows/macOS rush rows rely on the runner image's Node bundling corepack. If the image moves to Node 25+, these legs would fail loudly.

Auto-merge is armed, so approving sends this straight to the merge queue.

---

Generated by Claude Code