From 0ae87c8b676ab638001e02fbbd35fd3872a7c8a4 Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Thu, 30 Jul 2026 12:35:36 -0300 Subject: [PATCH] =?UTF-8?q?fix(ci):=20finish=20gap=2019=20=E2=80=94=20pin?= =?UTF-8?q?=20fast-gates=20and=20give=20USE=5FVPS=5FRUNNER=20one=20meaning?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .github/workflows/quality.yml | 15 +++- tests/unit/vps-runner-variable-scope.test.ts | 82 ++++++++++++++++++++ 2 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 tests/unit/vps-runner-variable-scope.test.ts diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index bf207a7300..7a9a1d2f87 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -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: diff --git a/tests/unit/vps-runner-variable-scope.test.ts b/tests/unit/vps-runner-variable-scope.test.ts new file mode 100644 index 0000000000..9d5fd60c89 --- /dev/null +++ b/tests/unit/vps-runner-variable-scope.test.ts @@ -0,0 +1,82 @@ +/** + * Guard for what `USE_VPS_RUNNER` is allowed to govern (gap 19). + * + * One variable used to switch the build AND the test jobs, which want OPPOSITE machines. The + * build needs the .113's RAM — the production `next build` wants 18–20 GB and a hosted runner + * has 16, which killed the same step nine times in the v3.8.49 cycle. The test jobs need the + * hosted runner's link: + * + * actions/setup-node on .113, 4 concurrent runners ..... 20m06s + * actions/setup-node hosted ............................ 16s + * the tests themselves .................. 2m54 vs 2m31 — a tie + * + * So there was no setting that served both, and flipping the variable for a release traded a + * working build for twenty-minute test jobs. + * + * The fix is not a second variable. Self-hosted is strictly worse for anything whose cost is + * dominated by `setup-node` + `npm ci`, so those jobs are pinned to hosted and the variable now + * means exactly one thing: "this job needs the .113's memory". + * + * `fast-gates` was the last holdout and the evidence for pinning it is unusually clean: across + * 160 quality.yml runs it NEVER landed on a self-hosted runner — every non-skipped sample is + * `GitHub Actions NNNN`. That was dead configuration, not a working escape hatch, and the + * classifier is not at fault: ci.yml's Build ran on omniroute-113-7 and -6 in the same window. + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../.."); +const workflowDir = path.join(repoRoot, ".github/workflows"); + +/** Map every `runs-on:` line mentioning USE_VPS_RUNNER back to the job that owns it. */ +function jobsSwitchedByVpsVar(): Array<{ file: string; job: string }> { + const out: Array<{ file: string; job: string }> = []; + for (const name of fs.readdirSync(workflowDir)) { + if (!name.endsWith(".yml") && !name.endsWith(".yaml")) continue; + const lines = fs.readFileSync(path.join(workflowDir, name), "utf-8").split("\n"); + let job = ""; + for (const line of lines) { + const m = /^ {2}([A-Za-z0-9_-]+):\s*$/.exec(line); + if (m) job = m[1]; + if (line.includes("USE_VPS_RUNNER") && line.includes("runs-on")) { + out.push({ file: name, job }); + } + } + } + return out; +} + +/** Jobs whose cost is dominated by setup-node + npm ci, never by memory. */ +const TEST_LIKE = /(^|-)(test|tests|vitest|unit|gates)($|-)/i; + +test("USE_VPS_RUNNER never governs a test-like job", () => { + const offenders = jobsSwitchedByVpsVar().filter((j) => TEST_LIKE.test(j.job)); + assert.deepEqual( + offenders.map((j) => `${j.file}:${j.job}`), + [], + "self-hosted is strictly worse for these — setup-node measured 20m06s there vs 16s hosted, " + + "while the tests themselves tie. Pin the job to ubuntu-latest instead of switching it." + ); +}); + +test("it still governs the build-like jobs — the ones that actually need the RAM", () => { + const jobs = jobsSwitchedByVpsVar().map((j) => j.job); + assert.ok( + jobs.includes("build"), + "the build is the whole reason the variable exists: 18-20 GB working set vs a 16 GB hosted runner" + ); + assert.ok(jobs.includes("publish"), "npm-publish falls back to a full build when the artifact is missing"); +}); + +test("fast-gates specifically stays hosted", () => { + // The holdout. 160 runs, zero self-hosted samples — dead configuration, and it would have + // inherited the setup-node penalty the day it fired. + const wf = fs.readFileSync(path.join(workflowDir, "quality.yml"), "utf-8"); + const block = wf.split(/^ {2}fast-gates:\s*$/m)[1] ?? ""; + const runsOn = /^ {4}runs-on:\s*(.+)$/m.exec(block); + assert.ok(runsOn, "fast-gates must declare runs-on"); + assert.match(runsOn[1].trim(), /^ubuntu-latest$/, "fast-gates must be pinned, not switched"); +});