Two correctness bugs from the AI review of #10010, both confirmed against the
merged code on develop.
1. build.yml - every warm run would fail.
`Restore node_modules archive` used `lookup-only: true`, so on a cache HIT the
archive is never written to disk. `Install`, `Run postinstall`, `Archive` and
`Save` are all gated on `cache-hit != 'true'` and correctly skip - but
`Upload node_modules archive` carries NO `if:` guard and sets
`if-no-files-found: error`. On a hit it therefore uploads nothing and fails
the job, and because the other four jobs `needs:` this one, it takes the whole
gate with it.
The cold run that first writes the key passes, so this is invisible until the
SECOND run - the one the cache exists for. Dropping `lookup-only` makes the
producer always materialise the archive: a ~2.9 GB download costs a few
minutes against the ~90-minute install it replaces, and it is what guarantees
the run-scoped artifact handoff actually has a file to hand off.
2. archive-node-modules.sh - a failed tar could publish a corrupt archive.
The script set only `-o pipefail`, never `-e`, and tolerated tar exit 1 with
`|| [ $? -eq 1 ]`. A tar exit status of 2 - a real error - left the `||`
compound false and execution simply continued to the size check. A ~2.9 GB
tree truncated early still clears the 100 MB floor, so the partial archive
would be published and unpacked by four consumer jobs.
Now `set -e`, with the tar status captured explicitly so exit 1 stays
tolerated and anything else aborts before the archive can be published.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Five jobs each ran their own full `yarn install` of the same tree, concurrently,
on the same self-hosted pool — so they competed with each other for I/O. Same
commit, same run, measured on PR #10010 (run 32358690732):
build-api install 1h49m -> passed
build-monorepo-root install 1h51m -> passed
build-libs install 2h40m -> KILLED at the 180-minute ceiling
build-web install 2h40m -> KILLED at the 180-minute ceiling
The two that died never compiled a line: `Build packages` and `Build web` are
recorded as SKIPPED. Which two survive is a coin flip, and this gate has failed
that way four times during this work on code that builds fine.
`build-monorepo-root` already did install + postinstall and nothing else, so it
becomes the producer: it publishes the tree as one compressed archive, and the
other four `needs:` it and unpack in minutes instead of installing.
A CACHE, not an artifact — the opposite choice to test_playwright.yml, for two
reasons that only apply here. build.yml runs on every pull_request, so a ~2.9 GB
artifact per run would meter against the org's Actions storage (the quota that
just stopped Playwright reports uploading); caches are unbilled and live in a
separate 10 GB per-repo budget. And a cache persists ACROSS runs keyed on the
lockfile, so once develop populates it every PR restores rather than installing
— which an artifact, scoped to its own run, cannot do.
Net cache usage goes DOWN: `cache: yarn` is removed from all five jobs, which
frees the 4.66 GB setup-node yarn cache it kept per ref, and the ~2.9 GB tree
archive replaces it. The archive is only re-saved when yarn.lock, patches/ or
.scripts/postinstall.js change, so most PRs restore and never write.
The archive/restore shell is now shared with test_playwright.yml as
.github/scripts/archive-node-modules.sh and restore-node-modules.sh — one copy
behind eight call sites, rather than the drift that already bit the e2e workflow
once.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
