mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-05 14:52:09 +03:00
fix(ci): five workflow defects, one of them shipping the wrong dmg to Intel Macs (#8988)
* fix(ci): five workflow defects, one of them shipping the wrong dmg to Intel Macs
Gaps 31, 19, 16, 30 and 12 of the v3.8.49 process dossier.
## 31 — LIVE BUG: an Intel Mac downloads the ARM dmg
electron-builder runs once per macOS job and each run emits its own
`latest-mac.yml` listing only its own dmg — measured at 338 and 350 bytes,
different content, identical filename. `download-artifact` with
`merge-multiple: true` resolves that collision by ARRIVAL ORDER, so one silently
overwrites the other. arm64 won in the published v3.8.48.
Why that breaks Intel, from electron-updater's own selection code
(out/providers/Provider.js):
files.find(it => [...].some(n => n.includes(process.arch))) ?? files.shift()
The Intel dmg is `OmniRoute-X.Y.Z.dmg` — no arch suffix. On Intel `process.arch`
is "x64", nothing matches, and the fallback takes the FIRST entry. With an
arm64-only manifest that is the ARM build.
So ORDER is the fix, not tidiness: the un-suffixed entry must be first, because
it is the only one reachable through that fallback. `merge-multiple` is now off
(per-artifact subdirectories) and a new
`scripts/release/merge-mac-update-manifest.mjs` merges them deliberately. It
refuses to write when the inputs disagree on version — a manifest stitched from
two builds points at files that were never published together, which is worse
than no manifest.
Validated against the REAL v3.8.49 manifests, not just fixtures: the script
reproduces byte-for-byte the manifest I hand-merged and published, including
both sha512 values and the newer releaseDate.
## 19 — one variable, two opposite machines
`USE_VPS_RUNNER` governed the build and the test jobs together. The build needs
the .113's RAM; the tests need the hosted runner's link. Measured 2026-07-29:
`actions/setup-node` took 20m06s on .113 with 4 concurrent runners versus 16s
hosted (npm cache restore saturating the link), while the tests themselves tied
— 2m54 vs 2m31.
Self-hosted is therefore strictly worse for tests, so rather than add a second
variable to configure, `test-unit`, `test-vitest`, `fast-unit` and `fast-vitest`
are pinned to `ubuntu-latest`. `quality.yml`'s `fast-gates` deliberately keeps
the variable — I have no measurement for it, and guessing is what produced this
gap.
## 16 — a flaky shard sent the publish into the 40-minute build
The artifact reuse filter required `conclusion == "success"` on the whole run, so
any unrelated red shard discarded a perfectly good tree. The artifact is only
uploaded if the Build job succeeded, so its PRESENCE is the accurate signal. Now
it takes the 5 most recent candidate runs and tries each download until one
works. `head_repository.full_name == env.REPO` stays — that clause is the
artifact-poisoning guard, not a filter refinement.
## 30 — the gate that could be bypassed at merge
`check:agent-skills-sync` lived only in quality.yml's PR-only Merge-integrity
job, because the CHANGELOG half of that job needs a base to diff against. This
half does not. Keeping it PR-only left a real hole: this cycle's merge trains
landed with `--admin`, which bypasses required checks, so three SKILL.md files
drifted, rode the release squash into `main`, and the sync-back turned them into
a base-red blocking EVERY PR into release/v3.8.50 until #8954. It now also runs
in ci.yml's lint job, which runs on push to `main`.
## 12 — a cancelled gate reads like a passing one
The dashboard already renders `⚫ CANCELLED` per job, so my dossier entry was
imprecise: they do not vanish, they sit buried mid-table. A cancelled job
reported no verdict at all, and this cycle the Vitest job was cancelled in rounds
1, 2 and 3 — it finished only in round 4, revealing a suite broken the whole
cycle plus two production bugs. The summary now opens with a banner naming every
cancelled job and saying plainly that nothing was checked.
node --import tsx/esm --test tests/unit/mac-update-manifest-merge.test.ts # 11 pass
merge against the real v3.8.49 manifests → both dmgs, Intel first
all four workflows parse; check:workflows --ratchet → 178, baseline 190
* docs(changelog): fragment for #8988
* test(ci): align the artifact-provenance guard with the gap-16 criterion
My own assertion from #8953 encoded the criterion this PR deliberately removes:
it required `.conclusion == "success"` on the whole CI run, which discarded a
perfectly good build tree whenever any unrelated shard went red — pushing the
publish into the 40-minute build the fast path exists to avoid.
Inverted rather than deleted, and the replacement is strictly stronger. It now
pins three things where the old one pinned one: that the loose criterion is gone,
that the step actually probes for the artifact (the accurate signal, since it is
only uploaded when the Build job succeeded), and that it probes MORE THAN ONE
candidate run — without which a single miss still falls back to a full build.
The provenance clause it was originally written to protect
(head_repository.full_name == env.REPO) is untouched and still asserted above.
* fix(ci): finish gap 19 — pin fast-gates and give USE_VPS_RUNNER one meaning
This was left deliberately partial because `fast-gates` had never been measured,
and guessing is what produced gap 19 in the first place. Measured now, and the
evidence is cleaner than expected:
fast-gates, 160 quality.yml runs .... ZERO self-hosted samples
every non-skipped one is "GitHub Actions NNNN"
median duration, 72 successful runs .. 5.6 min hosted
The classifier is not at fault — in the same window ci.yml's Build demonstrably
ran on omniroute-113-7 and omniroute-113-6, so self-hosted runs are visible when
they happen. The USE_VPS_RUNNER expression on this job was dead configuration.
And had it ever fired it would have inherited the measured penalty, because this
job's first two steps are exactly the bottleneck:
actions/setup-node on .113 with 4 concurrent runners .... 20m06s
actions/setup-node hosted .............................. 16s
So it is pinned rather than switched, and the second variable the gap proposed
(USE_VPS_RUNNER_BUILD / _TESTS) turns out to be unnecessary. After this the
variable governs exactly five jobs, all of them build-like:
ci.yml:build · quality.yml:build · npm-publish:publish
nightly-release-green: release-green, main-green
One variable, one meaning: "this job needs the .113's memory". A guard test pins
that — it fails if the variable is ever attached to a test-like job again, and it
also asserts the build KEEPS it, so nobody closes this gap by removing the
variable outright.
node --import tsx/esm --test tests/unit/vps-runner-variable-scope.test.ts # 3 pass
check:workflows --ratchet → 178, baseline 190
---------
Co-authored-by: diegosouzapw <diegosouzapw@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
60cea3cab0
commit
9b3efef806
52
.github/workflows/ci.yml
vendored
52
.github/workflows/ci.yml
vendored
@@ -137,6 +137,17 @@ jobs:
|
||||
- run: npm run check:route-guard-membership
|
||||
- run: npm run check:test-discovery
|
||||
- run: npm run check:tracked-artifacts
|
||||
# (gap 30) Also lives in quality.yml's PR-only "Merge integrity" job — because the
|
||||
# CHANGELOG half of that job needs a base to diff against. This half does NOT: the
|
||||
# generator either reproduces the committed SKILL.md files or it does not.
|
||||
#
|
||||
# Keeping it PR-only left a real hole. This cycle's merge trains landed in batches with
|
||||
# `--admin`, which bypasses required checks, so three SKILL.md files drifted from the route
|
||||
# catalog, rode the release squash into `main`, and the next cycle's sync-back turned them
|
||||
# into a base-red that blocked EVERY PR into release/v3.8.50 until #8954. Running it here
|
||||
# means a push to `main` catches the drift at the source instead of the next cycle
|
||||
# inheriting it.
|
||||
- run: npm run check:agent-skills-sync
|
||||
# WS1.7 (v3.8.49 plan): Dockerfile lint (hadolint, pinned by digest).
|
||||
# failure-threshold=error keeps the 5 pre-existing warnings (DL3008/DL3003/
|
||||
# DL3016 version pinning / WORKDIR) visible without blocking; any ERROR fails.
|
||||
@@ -740,7 +751,13 @@ jobs:
|
||||
test-unit:
|
||||
name: Unit Tests (${{ matrix.shard }}/8)
|
||||
# Same dynamic-runner rule as Build (own-origin only; fallback ubuntu-latest).
|
||||
runs-on: ${{ (vars.USE_VPS_RUNNER == 'true' && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository)) && fromJSON('["self-hosted","omni-release"]') || 'ubuntu-latest' }}
|
||||
# PINNED to hosted, deliberately not on the USE_VPS_RUNNER switch (gap 19). One variable
|
||||
# governed the build and the test jobs, which want OPPOSITE machines: the build needs the
|
||||
# .113's RAM, the tests need the hosted runner's link. Measured on 2026-07-29 —
|
||||
# actions/setup-node took 20m06s on .113 with 4 concurrent runners versus 16s hosted (npm
|
||||
# cache restore saturating the link), while the tests themselves tied, 2m54 vs 2m31. So
|
||||
# self-hosted is strictly worse here and there is nothing to configure.
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 25
|
||||
# needs: changes (not build) — this job never downloads the next-build artifact;
|
||||
# gating it on Build only serialized ~20min of wall-clock for nothing. Jobs that
|
||||
@@ -812,7 +829,13 @@ jobs:
|
||||
test-vitest:
|
||||
name: Vitest (MCP / autoCombo / UI components)
|
||||
# Same dynamic-runner rule as Build (own-origin only; fallback ubuntu-latest).
|
||||
runs-on: ${{ (vars.USE_VPS_RUNNER == 'true' && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository)) && fromJSON('["self-hosted","omni-release"]') || 'ubuntu-latest' }}
|
||||
# PINNED to hosted, deliberately not on the USE_VPS_RUNNER switch (gap 19). One variable
|
||||
# governed the build and the test jobs, which want OPPOSITE machines: the build needs the
|
||||
# .113's RAM, the tests need the hosted runner's link. Measured on 2026-07-29 —
|
||||
# actions/setup-node took 20m06s on .113 with 4 concurrent runners versus 16s hosted (npm
|
||||
# cache restore saturating the link), while the tests themselves tied, 2m54 vs 2m31. So
|
||||
# self-hosted is strictly worse here and there is nothing to configure.
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 15
|
||||
# needs: changes (not build) — no artifact consumed; see test-unit note.
|
||||
needs: changes
|
||||
@@ -1249,6 +1272,8 @@ jobs:
|
||||
- name: Generate dashboard
|
||||
env:
|
||||
EVENT_NAME: ${{ github.event_name }}
|
||||
# Workflow-controlled data (job results), not user input — safe to read here.
|
||||
NEEDS_JSON: ${{ toJSON(needs) }}
|
||||
run: |
|
||||
status() {
|
||||
case "$1" in
|
||||
@@ -1263,6 +1288,29 @@ jobs:
|
||||
echo "# 🚀 CI Dashboard" >> "$GITHUB_STEP_SUMMARY"
|
||||
echo "" >> "$GITHUB_STEP_SUMMARY"
|
||||
|
||||
# (gap 12) A cancelled job never reported a verdict, and in a long table that reads the
|
||||
# same as a green one. `cancel-in-progress` plus incremental fixing cancels jobs on every
|
||||
# push, and this cycle the Vitest job was cancelled in rounds 1, 2 and 3 — it only ran to
|
||||
# completion in round 4, where it revealed a suite that had been broken the whole cycle
|
||||
# plus two production bugs. A gate that never finishes is indistinguishable from one that
|
||||
# passes, so name them at the TOP instead of leaving them to be spotted mid-table.
|
||||
CANCELLED_JOBS=$(printf '%s' "$NEEDS_JSON" \
|
||||
| jq -r 'to_entries | map(select(.value.result == "cancelled")) | .[].key' 2>/dev/null \
|
||||
| sort | paste -sd", " -) || CANCELLED_JOBS=""
|
||||
if [ -n "$CANCELLED_JOBS" ]; then
|
||||
{
|
||||
echo "> ### ⚫ Cancelled — no verdict was reported"
|
||||
echo ">"
|
||||
echo "> \`$CANCELLED_JOBS\`"
|
||||
echo ">"
|
||||
echo "> These did not fail; they never finished, so nothing was checked. Treat this"
|
||||
echo "> run as INCOMPLETE for those gates. If the cancellation came from"
|
||||
echo "> \`cancel-in-progress\` on a newer push, the newer run covers it — otherwise"
|
||||
echo "> re-run them before reading this dashboard as green."
|
||||
echo ""
|
||||
} >> "$GITHUB_STEP_SUMMARY"
|
||||
fi
|
||||
|
||||
echo "## 🧱 Core Checks" >> "$GITHUB_STEP_SUMMARY"
|
||||
echo "| Job | Status |" >> "$GITHUB_STEP_SUMMARY"
|
||||
echo "|-----|--------|" >> "$GITHUB_STEP_SUMMARY"
|
||||
|
||||
26
.github/workflows/electron-release.yml
vendored
26
.github/workflows/electron-release.yml
vendored
@@ -249,11 +249,33 @@ jobs:
|
||||
persist-credentials: false
|
||||
fetch-depth: 0
|
||||
|
||||
# `merge-multiple` is deliberately OFF. It resolves same-name collisions by ARRIVAL
|
||||
# ORDER, and the two macOS jobs each emit their own `latest-mac.yml` listing only their
|
||||
# own dmg (measured: 338 and 350 bytes, different content, identical name). One silently
|
||||
# overwrote the other — arm64 won in the published v3.8.48, and since the Intel dmg
|
||||
# carries no arch suffix in its name, electron-updater's
|
||||
# `files.find(url includes process.arch) ?? files.shift()` sends every Intel Mac to the
|
||||
# ARM dmg. Downloading into per-artifact subdirectories keeps both, so they can be
|
||||
# merged on purpose instead of by luck.
|
||||
- name: Download all artifacts
|
||||
uses: actions/download-artifact@v8
|
||||
with:
|
||||
path: release-assets
|
||||
merge-multiple: true
|
||||
path: artifacts
|
||||
|
||||
# Writes release-assets/latest-mac.yml with BOTH dmgs, un-suffixed entry first (that is
|
||||
# the one electron-updater can only reach through its fallback). Refuses to write when the
|
||||
# inputs disagree on version — a manifest stitched from two builds is worse than none.
|
||||
- name: Merge the per-arch macOS updater manifests
|
||||
run: node scripts/release/merge-mac-update-manifest.mjs artifacts release-assets
|
||||
|
||||
# Everything else moves across as-is. The partial latest-mac.yml files are excluded so
|
||||
# they cannot clobber the merged one; -n is a second belt on the same braces.
|
||||
- name: Collect the remaining artifacts
|
||||
run: |
|
||||
mkdir -p release-assets
|
||||
find artifacts -type f ! -name latest-mac.yml -exec cp -n {} release-assets/ \;
|
||||
echo "release-assets:"
|
||||
ls -la release-assets/
|
||||
|
||||
- name: Create source archives
|
||||
env:
|
||||
|
||||
32
.github/workflows/npm-publish.yml
vendored
32
.github/workflows/npm-publish.yml
vendored
@@ -179,18 +179,36 @@ jobs:
|
||||
REPO: ${{ github.repository }}
|
||||
run: |
|
||||
set -uo pipefail
|
||||
RUN=$(gh api "repos/$REPO/actions/runs?head_sha=$HEAD_SHA&per_page=100" \
|
||||
# The question is "which run HAS the artifact", not "which run passed" (gap 16).
|
||||
# Requiring `conclusion == "success"` on the whole run discarded a perfectly good tree
|
||||
# whenever any unrelated shard went red — one flaky test then pushed the publish into
|
||||
# the 40-minute build this step exists to avoid. The artifact is only uploaded if the
|
||||
# Build job itself succeeded, so its PRESENCE is the accurate signal; the run's overall
|
||||
# conclusion is noise from jobs that have nothing to do with the tree.
|
||||
#
|
||||
# `head_repository.full_name == env.REPO` stays, and it is not a filter refinement:
|
||||
# this tree becomes the published npm tarball, and fork `pull_request` runs execute in
|
||||
# THIS repository's context uploading their own next-build. That clause is the
|
||||
# supply-chain guard (CodeQL actions/artifact-poisoning).
|
||||
CANDIDATES=$(gh api "repos/$REPO/actions/runs?head_sha=$HEAD_SHA&per_page=100" \
|
||||
--jq '[.workflow_runs[]
|
||||
| select(.name == "CI"
|
||||
and .conclusion == "success"
|
||||
and .head_repository.full_name == env.REPO)]
|
||||
| .[0].id // empty') || RUN=""
|
||||
if [ -z "$RUN" ]; then
|
||||
echo "::notice::no successful CI run for $HEAD_SHA — falling back to a full build"
|
||||
| sort_by(.run_started_at) | reverse | .[0:5] | .[].id') || CANDIDATES=""
|
||||
if [ -z "$CANDIDATES" ]; then
|
||||
echo "::notice::no CI run from this repository for $HEAD_SHA — falling back to a full build"
|
||||
exit 0
|
||||
fi
|
||||
if ! gh run download "$RUN" --repo "$REPO" --name next-build --dir /tmp/next-build; then
|
||||
echo "::notice::next-build artifact unavailable for run $RUN (expired?) — falling back to a full build"
|
||||
RUN=""
|
||||
for candidate in $CANDIDATES; do
|
||||
if gh run download "$candidate" --repo "$REPO" --name next-build --dir /tmp/next-build 2>/dev/null; then
|
||||
RUN="$candidate"
|
||||
break
|
||||
fi
|
||||
echo " run $candidate carries no usable next-build — trying the next"
|
||||
done
|
||||
if [ -z "$RUN" ]; then
|
||||
echo "::notice::none of the candidate runs still carries next-build (1-day retention) — falling back to a full build"
|
||||
exit 0
|
||||
fi
|
||||
tar -xzf /tmp/next-build/e2e-build.tar.gz -C .
|
||||
|
||||
31
.github/workflows/quality.yml
vendored
31
.github/workflows/quality.yml
vendored
@@ -112,7 +112,20 @@ jobs:
|
||||
# release captain has USE_VPS_RUNNER=true AND this is not a fork PR (own-origin
|
||||
# branches only — a fork PR must never execute on the LAN runner). Var unset/false
|
||||
# or a fork PR falls back to ubuntu-latest, so this is inert until the flag flips.
|
||||
runs-on: ${{ (vars.USE_VPS_RUNNER == 'true' && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository)) && fromJSON('["self-hosted","omni-release"]') || 'ubuntu-latest' }}
|
||||
# PINNED to hosted (gap 19). This job carried the USE_VPS_RUNNER expression, and that
|
||||
# expression was DEAD CONFIGURATION: across 160 quality.yml runs the job never once landed on
|
||||
# a self-hosted runner — every non-skipped sample is `GitHub Actions NNNN`. The classifier is
|
||||
# not at fault: in the same window ci.yml's Build demonstrably ran on omniroute-113-7 and
|
||||
# omniroute-113-6, so self-hosted runs are visible when they happen.
|
||||
#
|
||||
# And if it ever HAD fired it would have inherited the measured penalty, because this job's
|
||||
# first two steps are exactly the bottleneck: actions/setup-node + npm ci took 20m06s on .113
|
||||
# with 4 concurrent runners versus 16s hosted (npm cache restore saturating the link). Median
|
||||
# here is 5.6 min hosted across 72 successful runs.
|
||||
#
|
||||
# With this pinned, USE_VPS_RUNNER governs ONLY build-like jobs — one variable, one coherent
|
||||
# purpose. That is what gap 19 asked for; a second variable turned out to be unnecessary.
|
||||
runs-on: ubuntu-latest
|
||||
# tsx gates (known-symbols, route-guard-membership) import modules that open
|
||||
# SQLite on load; provide DB env so a fresh CI DB initializes cleanly.
|
||||
env:
|
||||
@@ -260,7 +273,13 @@ jobs:
|
||||
needs: changes
|
||||
if: ${{ github.event_name != 'pull_request' || ((github.event.pull_request.draft == false || startsWith(github.head_ref, 'mergify/merge-queue/')) && needs.changes.outputs.code == 'true') }}
|
||||
# Dynamic runner — see fast-gates (own-origin + flag; fork/unset → ubuntu-latest).
|
||||
runs-on: ${{ (vars.USE_VPS_RUNNER == 'true' && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository)) && fromJSON('["self-hosted","omni-release"]') || 'ubuntu-latest' }}
|
||||
# PINNED to hosted, deliberately not on the USE_VPS_RUNNER switch (gap 19). One variable
|
||||
# governed the build and the test jobs, which want OPPOSITE machines: the build needs the
|
||||
# .113's RAM, the tests need the hosted runner's link. Measured on 2026-07-29 —
|
||||
# actions/setup-node took 20m06s on .113 with 4 concurrent runners versus 16s hosted (npm
|
||||
# cache restore saturating the link), while the tests themselves tied, 2m54 vs 2m31. So
|
||||
# self-hosted is strictly worse here and there is nothing to configure.
|
||||
runs-on: ubuntu-latest
|
||||
env:
|
||||
JWT_SECRET: ci-lint-secret-with-sufficient-length-for-validation
|
||||
API_KEY_SECRET: ci-lint-api-key-secret-long
|
||||
@@ -296,7 +315,13 @@ jobs:
|
||||
# critical path again (~8.5min → ~4.5min on ubuntu-latest; ~2min on the 8-slot
|
||||
# runner box). Node's native --test-shard=N/total takes any denominator — only
|
||||
# this matrix and the TEST_SHARD env below encode the shard count.
|
||||
runs-on: ${{ (vars.USE_VPS_RUNNER == 'true' && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository)) && fromJSON('["self-hosted","omni-release"]') || 'ubuntu-latest' }}
|
||||
# PINNED to hosted, deliberately not on the USE_VPS_RUNNER switch (gap 19). One variable
|
||||
# governed the build and the test jobs, which want OPPOSITE machines: the build needs the
|
||||
# .113's RAM, the tests need the hosted runner's link. Measured on 2026-07-29 —
|
||||
# actions/setup-node took 20m06s on .113 with 4 concurrent runners versus 16s hosted (npm
|
||||
# cache restore saturating the link), while the tests themselves tied, 2m54 vs 2m31. So
|
||||
# self-hosted is strictly worse here and there is nothing to configure.
|
||||
runs-on: ubuntu-latest
|
||||
strategy:
|
||||
fail-fast: false
|
||||
matrix:
|
||||
|
||||
Reference in New Issue
Block a user