mirror of
https://github.com/ever-co/ever-gauzy.git
synced 2026-10-20 06:09:29 +08:00
Ruslan KonviserandClaude Opus 5
06569082de
Adversarial review of d7c56524d2 found one blocker I had walked straight into,
plus several things that would have made the gate quietly worse.
BLOCKER — `needs:` without `if:` erases four verdicts. A job whose dependency
fails is recorded `skipped`, not `failure`, and a skipped required check does
NOT block a merge. So any producer hiccup would have silently removed the
build-libs / build-api / build-web / build-desktop verdicts from the gate — the
exact "no failure just means never ran" hazard this file's own header warns
about, and the shape of the 2026-08-13 template-error incident it cites. All
four now carry `if: !cancelled()`: the ordering still holds, but a dead producer
yields a real red instead of four silent skips. build-desktop folds it into its
existing branch filter, because two `if:` keys is a duplicate mapping key and
the file will not parse.
Moving the archive shell into a script silently dropped errexit — an inline
`run:` gets `bash -eo pipefail` from GitHub, a script with its own shebang does
not. `tar ... || [ $? -eq 1 ]` therefore stopped aborting on a real tar failure,
so a truncated archive could be published under a good key. Cache entries are
immutable per key, so that poison would have persisted until the lockfile
changed, with every consumer falling back to a multi-hour install. tar's status
is now checked explicitly, and the size floor moves from 100 MB (~3% of reality)
to 1 GB.
The fallback called `yarn bootstrap`, which drops `--frozen-lockfile` and
`--network-timeout` — so the degraded path could build a tree that does not match
the committed lockfile and report green, while the Docker builds that DO install
frozen fail later on develop. Added `bootstrap:ci` carrying the CI flags, used by
both workflows' fallbacks so they cannot drift apart again.
Also from the review:
- `package.json` joins the cache key. Without it a PR that edits package.json
without regenerating yarn.lock hashed identically to develop, hit the cache,
and skipped the only `--frozen-lockfile` execution left in the workflow.
- `lookup-only: true` on the producer: on a hit it did nothing with the archive
(every step is gated on the miss), so it was downloading ~2.9 GB purely to
compute a boolean, in front of four jobs waiting on it.
- `fail-on-cache-miss: true` on the consumers: the producer's success means the
entry exists, so a miss is an eviction, not a normal path. Fail in seconds
rather than letting four jobs re-create the contention this change removes.
- the producer moves to the 8-core pool and 360 minutes. It is now the serial
critical path, its work is CPU-bound, and a cold install here has no yarn
tarball cache behind it — the equivalent deps job measured 88 min, 3h20m and
3h46m. At 180 a cold miss would time out and take the whole gate with it.
- the header comment said the `needs:` edges were gone; it now describes why
they are back and what they cost.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
40 lines
2.2 KiB
Bash
Executable File
40 lines
2.2 KiB
Bash
Executable File
#!/usr/bin/env bash
|
|
#
|
|
# Restores the dependency tree published by the `deps` job, or bootstraps from scratch.
|
|
#
|
|
# Both consumers (build, and each e2e shard) ran identical copies of this inline, and they had
|
|
# already drifted once — a fix applied to one block missed the other. It is also deliberately ONE
|
|
# step rather than an unpack step plus a conditional fallback: a step `if:` carrying no status-check
|
|
# function is implicitly ANDed with `success()`, so a fallback gated on a previous step's outcome
|
|
# never runs after that step fails hard. Doing both here removes that trap by construction.
|
|
set -o pipefail
|
|
|
|
ARCHIVE="${NODE_MODULES_ARCHIVE:?NODE_MODULES_ARCHIVE must be set}"
|
|
|
|
# Explicit decompression rather than tar's magic-byte auto-detect: GNU tar shells out to an external
|
|
# `zstd`, so auto-detect needs tar >= 1.31 AND zstd on THIS host, while the archiving host only
|
|
# needed `--use-compress-program`. Try each in turn so a mismatch degrades to the bootstrap below.
|
|
extract() {
|
|
[ -f "$ARCHIVE" ] || return 1
|
|
if zstd -dc "$ARCHIVE" 2>/dev/null | tar -xf -; then echo "extracted (zstd)"; return 0; fi
|
|
if gzip -dc "$ARCHIVE" 2>/dev/null | tar -xf -; then echo "extracted (gzip)"; return 0; fi
|
|
if tar -xf "$ARCHIVE"; then echo "extracted (tar auto-detect)"; return 0; fi
|
|
return 1
|
|
}
|
|
|
|
if extract && [ -d node_modules ]; then
|
|
rm -f "$ARCHIVE"
|
|
exit 0
|
|
fi
|
|
|
|
echo "::warning::node_modules archive unavailable — falling back to a full bootstrap (1-3 h on these runners)."
|
|
# Clear any partial extraction first: a half-written tree can still carry a
|
|
# `node_modules/.yarn-integrity` that convinces yarn everything is already installed.
|
|
rm -rf "$ARCHIVE" node_modules apps/*/node_modules packages/*/node_modules \
|
|
packages/plugins/*/node_modules tools/node_modules
|
|
# `bootstrap:ci`, not `bootstrap`: the developer-facing script drops --frozen-lockfile (so a
|
|
# package.json/lockfile disagreement would be silently rewritten and reported green, while the
|
|
# Docker image builds that DO install frozen fail later) and --network-timeout (yarn 1 defaults to
|
|
# 30s per request, and this is the degraded path on a pool already documented as slow).
|
|
yarn bootstrap:ci || { echo "::warning::bootstrap failed — retry 1"; yarn bootstrap:ci; }
|