Skip to content

ci: add GitHub Actions workflow across os and node matrix#3

Merged
agalwood merged 6 commits into
mainfrom
ci/github-actions
Jul 17, 2026
Merged

ci: add GitHub Actions workflow across os and node matrix#3
agalwood merged 6 commits into
mainfrom
ci/github-actions

Conversation

@agalwood

@agalwood agalwood commented Jul 17, 2026

Copy link
Copy Markdown
Member

What

Adds .github/workflows/ci.yml — the repo's first CI. On every push to main, every PR targeting main, and manual workflow_dispatch, it runs the four gates in order:

pnpm lintpnpm typecheckpnpm testpnpm build

across a 3×3 matrix: ubuntu-latest / windows-latest / macos-latest × Node 20 / 22 / 24, with fail-fast: false so one failing combo doesn't mask the others. Getting the matrix green also required one real product fix and a set of test-portability fixes (below).

Why windows is in the matrix

The self-update win32 code in src/run-command.ts (cross-spawn-style cmd.exe escaping, absolute-path taskkill, exit-code command-not-found normalization) only executes on win32 — the windows runners are the only place it actually runs. This paid off immediately: the first windows run caught a real bug (see below).

What the first runs caught

  1. node 20 × pnpm 11: pnpm 11.13.0 refuses to run on node < 22.13, and both pnpm/action-setup standalone mode and npm i -g @pnpm/exe are broken for 11.13.0 — the @pnpm/* platform packages on npm were published without their binary (Error after installing in standalone mode when trying to install packages with a CLI globally - This: not found pnpm/action-setup#277). The workflow instead downloads the standalone binary straight from the pnpm GitHub release (a separate, intact pipeline), versioned by the packageManager field. pnpm runs on its own bundled runtime; the gates run on the matrix node.
  2. CRLF: windows runners check out with core.autocrlf=true, failing biome check (repo pins lineEnding: lf). Fixed at the git layer with .gitattributes (* text=auto eol=lf).
  3. Real win32 bug in run-command.ts: looksLikeBatchShim treated every extensionless bare name as a .cmd shim, so node arguments got the double meta-escape. cmd.exe parses an .exe command line only once, so literal carets reached node's argv and node -e <script> broke outright. Now only the JS package managers invoked by bare name (npm/npx/pnpm/pnpx/yarn/corepack — actual .cmd shims) get the double escape; the BatBadBut mitigation for them is unchanged.
  4. POSIX assumptions in tests: hardcoded / in path expectations (now join()-built so the platform-selection logic stays covered on every host), 0o600 mode assertions (windows has no POSIX mode bits — skipped there), and spawn-failure/timeout semantics (win32 goes through cmd.exe: missing command = non-zero exit + "is not recognized" stderr, not spawn ENOENT; taskkill = non-zero code, not signal null).

Known pre-existing limitation, unchanged by this PR: a standalone pnpm.exe invoked as bare pnpm still gets double-escaped (caret-mangled) args on win32 — it fails loudly, and resolving the real extension via PATHEXT (as cross-spawn does) is the complete future fix.

Design notes

  • Top-level permissions: contents: read (least privilege) and persist-credentials: false on checkout — no step needs the token after clone.
  • concurrency keyed by workflow + ref cancels superseded runs.
  • CI only: no publish workflow, no NPM_TOKEN or other secrets. Publishing stays manual pnpm publish + OTP.

Test plan

  • All four gates pass locally on macOS (217/217 tests) with pnpm install --frozen-lockfile.
  • All 9 matrix combos green on this PR's Checks tab, including the three windows jobs which now exercise the win32 self-update path for the first time.

agalwood added 6 commits July 17, 2026 11:56
Run lint, typecheck, test, and build on every push to main and every
PR targeting main, across ubuntu/windows/macos x node 20/22/24 with
fail-fast disabled. The windows runners are the only environment that
exercises the win32 self-update path (cmd.exe escaping, absolute-path
taskkill, exit-code 9009 command-not-found normalization).

pnpm is installed via pnpm/action-setup before setup-node so the
pnpm cache works; the pnpm version comes from the packageManager
field. CI only — publishing stays a manual pnpm publish + OTP.
GitHub windows runners check out with core.autocrlf=true, converting
every file to crlf and failing pnpm lint (biome.json pins
formatter.lineEnding to lf). Forcing eol=lf at the git layer keeps
checkouts biome-clean on all platforms.
pnpm 11.13.0 refuses to start on node < 22.13, which killed every
node 20 combo at pnpm install. The standalone @pnpm/exe bundles its
own runtime, while project scripts (vitest/tsc/tsup) still run on the
matrix node — so the node 20 jobs genuinely test the engines floor.
pnpm/action-setup standalone mode is broken for pnpm 11.13.0: the
@pnpm/* platform packages on npm were published without their binary,
so every installer path leaves @pnpm/exe's placeholder bin in place
(pnpm/action-setup#277). The GitHub release assets are built by a
different pipeline and verified good, so download the standalone
binary directly, versioned by the packageManager field.
looksLikeBatchShim treated every extensionless bare name as a .cmd
shim, so 'node' args got the double meta-escape. cmd.exe parses an
.exe command line only once, leaving literal carets in argv —
node -e <script> broke outright (first observed on windows CI).
Only the JS package managers we invoke by bare name ship as .cmd
shims; classify exactly those, keep explicit .cmd/.bat paths, and
correct the isCommandMissing doc (cmd exits 1, not 9009; the
stderr phrase is the live signal).
Three POSIX assumptions surfaced by the new windows CI jobs:
- path expectations hardcoded '/' while the code joins with the host
  separator — build expectations with join() so the platform-selection
  logic stays covered on every host
- stat.mode & 0o777 === 0o600 — windows has no POSIX mode bits
  (reports 0o666), skip those assertions there
- missing-binary and timeout results — win32 goes through cmd.exe, so
  a missing command is a non-zero exit + 'is not recognized' stderr
  (not a spawn ENOENT), and taskkill yields a non-zero code (not a
  signal null); branch the assertions by platform
@agalwood
agalwood merged commit a0e1540 into main Jul 17, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant