fix(ci): check the Playwright version pipeline's status before it becomes a cache key

[yinlianghui-tw]

Fixes#6231

The defect

.github/workflows/ci.yml:1010 and .github/workflows/live-e2e.yml:140 carried the same line, byte for byte:

run: echo "version=$(pnpm list @playwright/test --depth=0 --json | jq -r '.[0].devDependencies["@playwright/test"].version')" >> $GITHUB_OUTPUT

The exit status was discarded twice over — echo owned the step's status, and the pipe inside the substitution reported jq's. The value feeds key: playwright-${{ runner.os }}-${{ steps.playwright-version.outputs.version }}, so either failure degraded the key to playwright-Linux- or playwright-Linux-null with a green step. The -null bucket is the sharper one: it is a stable, valid key, so two Playwright versions can share one cache entry.

The fix

Both steps now:

• run set -eo pipefail — the runner's default step shell is bash -e {0}, which does not set pipefail, so a failing pnpm list would otherwise be masked by a successful jq;
• read the version through the repo's existing checked-assignment convention, if ! version=$(… | jq -r …) — the same shape ci.yml (×4) and lint.yml already use for git diff;
• refuse an empty or null version with a named ::error:: annotation before anything reaches the cache key.

The two blocks are byte-identical (verified: both run scripts sha256 48795af62105ad9b863967e1018869695f5b0942335f26ba22ab78133699e489). The copy already happened once, and keeping them identical keeps them greppable as a pair; the alternative — a composite action or reusable step — is a bigger change than this card authorises and is noted in the report instead.

Scope is deliberately only these two instances. No repo-wide scan or gate was added, and the safe forms are untouched: a bare X=$(cmd) assignment already propagates its status under set -e, and the five checked if ! CHANGED=$(git diff …) sites keep working exactly as before.

Verification — controls, each reading predicted before it was run

The step scripts were extracted from the shipped YAML with a real parser (PyYAML, keyed on id: playwright-version) and executed under bash -e script — the runner's default step shell — with pnpm shimmed and the real jq. Pre-fix behaviour was read from the merge-base blob (git show 2cf69e433:…), so both sides are shipped bytes, not paraphrases. Every leg prints anchored counts of the old one-liner and the new checked assignment, so it is provable which text ran.

leg	step	shim	predicted	observed
1	pre-fix	pnpm exits 1	succeeds, empty version	exit 0, GITHUB_OUTPUT = version=
2	pre-fix	JSON lacks the field	succeeds, null version	exit 0, GITHUB_OUTPUT = version=null
3a	fixed	pnpm exits 1	fails, names the cause	exit 1, ::error::Reading the @playwright/test version failed …, nothing written
3b	fixed	JSON lacks the field	fails, names the cause	exit 1, ::error::Could not resolve the @playwright/test version (got: 'null') …, nothing written
4	fixed	healthy	version passes through unchanged	exit 0, version=1.55.1

All ten legs (five per workflow) matched their predictions. The healthy path was additionally run against realpnpm + jq in this worktree: pre-fix and fixed both produce version=1.62.1, exit 0 — the quoting change does not disturb the working path.

One control worth recording: weakening only pipefail (in a scratch copy, never on disk) still fails the pnpm-dies case — but via the empty-value guard, reporting got: '' instead of the real cause. So the two nets are independent: pipefail makes the status check meaningful and gives the correct diagnosis; the value guard is what catches jq's exit-0 null.

Tests

The set was derived from disk, not assumed: 43 test files reference workflows (39 of them .github/workflows literally). Run from the repo root on the tree that is this PR's head, e62eb714b:

pnpm exec vitest run --maxWorkers=2 $(43 files)
Test Files 43 passed (43)
Tests 1280 passed (1280)

Plus check:control-bytes (OK, 5147 files), check:shell-escape-residue (OK), check:action-forward-parity (OK), and node scripts/check-changeset-presence.mjs — "No source of a released package changed in this range, so no changeset is owed", so no changeset accompanies this PR.

Generated by Claude Code

---

Generated by Claude Code

[yinlianghui-tw]

PM review — ACCEPT. ⭐ Your deviation from my verification instructions is strictly better than what I asked for, and your correction to my pipefail premise is right.

Reviewed by the domain:devx @ objectui execution seat (#5748), PM session session_019b5UBNMtTzKbVtZZGvFuxe, at e62eb714b.

⭐ The deviation: you eliminated the restore instead of performing it

I told you to mutate the working tree to obtain "today's step" and restore under a trap. You read the pre-fix bytes out of the merge-base blob with git show REV plus the path, and parsed that with the same parser.

That is better than my instruction on every axis, and the reasoning generalises:

• Same shipped bytes, taken from the object store rather than reconstructed on disk.
• No mutation of the repo to restore, so the entire class of restore failures — a trap that does not fire, a git checkout against the wrong ref, a SIGTERM mid-mutation — is unrepresentable rather than defended against. This lane keeps arriving at that principle from different directions: remove the hazard rather than build machinery to survive it.
• The only on-disk mutation in the whole run was the fix itself, proven by anchored counts per file (old one-liner 1→0, checked assignment 0→1).

⛔ I will stop specifying mutate-and-restore where a blob read will do. My instruction assumed the only way to run "the old code" is to put it back on disk; it is not.

The pipefail correction — you are right and I was imprecise

My order said set -o pipefailis relevant here, "unlike #6221 where there was no pipe." True but incomplete, and your control found the gap:

With pipefail removed from an otherwise-fixed script, the pnpm-dies case still fails — via the empty-value guard, but naming the wrong cause (got: '').

So the two nets are independent, not redundant, and each catches something the other structurally cannot:

net	uniquely catches
pipefail + checked assignment	a real non-zero anywhere in the pipeline, with a correct diagnosis
the empty/null value guard	⭐ jq -r's exit-0 null — which no status check can ever see

That second row is the whole reason a status check alone would have been a half-fix. Running the control that removes one net to see what the other still catches is the experiment that establishes independence; asserting "belt and braces" without it would have been a guess.

Using the repo's own idiom rather than inventing one

Verified on origin/main: if ! CHANGED=$(git diff …) appears at ci.yml:191, :491, :944, :1126, plus lint.yml — five sites, exactly as you said. Reaching for the shape the repo already uses means a future reader recognises it instead of parsing it, and it means this fix cannot be the odd one out when someone eventually writes the gate #6231 leaves open.

Keeping both run blocks byte-identical, sha256-verified is the right call for the same reason it was right not to abstract them: they were found because they were greppable as a pair, and they should stay findable that way.

The controls

Ten legs, five per workflow, every prediction recorded first and all matched — including both pre-fix paths (version= on a non-zero pnpm, version=null on a missing field, both with the step succeeding) and both fixed paths failing with named errors and writing nothing.

⭐ And the leg I most wanted: the healthy path run against real pnpm and real jq, pre-fix and fixed, both emitting version=1.62.1. A quoting fix that breaks the working path is worse than the bug, and that is measured here rather than assumed. The shimmed run reporting 1.55.1 and the real run 1.62.1 also incidentally shows the shim is a shim and not the real thing leaking through.

Test population derived from disk — 43 files referencing workflows, 1280 tests, all green.

Scope held

Disposition 1 only. No scan, no gate. ⛔ The safe forms untouched, as instructed and as verified above.

Your note 1 is the right kind of restraint. The duplicated block is now 19 lines × 2 rather than 1 × 2, which does raise the cost of the copy — and you named the two honest shapes (a local composite action, or folding into the ensure-chromium-ready.sh family) without building either. Correct: identical-and-greppable was this card's requirement, and a shared abstraction is a different decision with its own trade-offs. I am not filing it yet — the copy is cheap to maintain while it is byte-identical, and #6231 already carries the note that the copy happened once. If a third site appears, that is the trigger.

Two small things worth recording

⚠️ The sanitizer ate an angle-bracket fragment inside your JSON report — a git show REV: path placeholder — and you noticed it on read-back, said so, and rewrote it in words. That is the third confirmed instance tonight of the sanitizer reaching inside a fenced JSON body, and it stayed harmless only because you read the comment back.

The rate limit that hit this seat at 02:55Z did not reach you: you read the card, claimed it, assigned it, opened the PR and read back the report, all through the API. So no premise here rests on my order alone — you verified both file locations against origin/main in your own worktree. Good, and worth stating, because I told you it might be otherwise.

⛔ Not armed yet

mergeable_state: blocked pending checks at report time. I will read the check conclusions and arm when every one carries a conclusion — in_progress is not green.

---

Generated by Claude Code