chore(ci): replace the PR workflows with a single aggregating gate - #2338
Merged
Merged
Conversation
isaque-bock-azion
requested review from
a team,
bruno-andrade-azion,
marcus-souza-azion and
pedro-ribeiro-azion
as code owners
September 10, 2026 19:06
Three defects that had nothing to do with each other, all in the deploy lane: - `prod.yml` nested `workflow_dispatch` inside `push:`, so a manual production deploy was never actually possible. It is now a sibling trigger. - The Algolia credentials were passed as command-line arguments, which puts them in the runner's process list. They go through `env:` instead. - None of the three deploys declared `concurrency`, so two merges landing close together left `gsutil rsync -d` racing over the same bucket. Each environment now serialises, with `cancel-in-progress: false` — a deploy is never cancelled mid-flight. `azion-deploy.yml` is deleted. It was never runnable: the comment lines inside its steps block are tab-indented, which is a YAML parse error, and it ran `npm install` in a repository whose `preinstall` is `only-allow pnpm`.
`lint:linkcheck` still carried `baseUrl: 'https://docs.astro.build'`, inherited from the Astro fork it descends from. The sitemap it reads is generated by Astro from `site`, which is `SITE_URL` — `http://localhost:4321` for a local build, `https://www.azion.com` for production. So the regex that pulls page paths out of `<loc>` matched nothing, every run parsed zero pages, and the checker reported "Found no link issues. Great job!" every time. That is worse than not having the check: the weekly workflow was green for a reason unrelated to the state of the links. With the base URL read from `SITE_URL` the checker parses all 1628 pages and reports real numbers.
The PR gate was one job: a build, plus the frontmatter test that runs inside the build script. No lint, no typecheck, no format check, no link check, no slug check — although every one of those scripts is already written in this repository and simply never invoked. `ci-gate.yml` takes the shape of aziontech/webkit's `governance.yml`: a `changes` job with `dorny/paths-filter`, every job conditioned on one of its outputs, and one aggregator at the end. `CI gate` is the single check to mark required — it passes when every job succeeded or was cleanly skipped, so a job can be added or switched off without touching branch protection. The filter list includes the workflow and the lint configs themselves, so a PR that breaks the gate is not the one PR the gate never sees. What the jobs do: - `security` — `pnpm audit` and TruffleHog over the PR range. This repository is public and had no secret scanning at all. - `content` — the navigation tree blocks; translation slugs ratchet. - `lint` — ESLint, Prettier and Stylelint over the files the PR changed. Over the whole tree Prettier takes three minutes and reports 502 files, and ESLint reports 97 errors across 46. Neither number is any one branch's fault, and reformatting the repository to reach zero would bury every future diff. Scoping to the diff is the ratchet: what you touch, you leave clean. - `types` — `astro check`, ratcheted at the 54 errors already here. This is the class of defect that reaches production silently; DocFrame rendered an empty slot with a build reporting zero errors. - `build` — unchanged, except that it now packs the HTML and the sitemaps into an artifact. 730 MB of HTML compresses to ~94 MB, which is cheaper than a second five-minute build and is what a11y will consume next. - `linkcheck` — runs against that artifact instead of rebuilding, behind the fan-out guard so a red build skips it cleanly. Three checks are too far in debt to block outright, so they ratchet on a count frozen in `ci/baselines.json` (`scripts/ci/ratchet.mjs`): 54 type errors, 745 translations whose slug does not match the English page, and 21664 link issues. The count may fall, never rise. Most of those link issues are footer and header links to www.azion.com pages that live in the site repository rather than here — configuring that boundary is its own piece of work. A ratchet whose command stops producing a number fails rather than passing, so this cannot repeat what `lint:linkcheck` did for years. `pr-checks.yml` and `weekly-linkcheck.yml` are folded into the gate. The weekly one also filed no issues: its guard was `steps.linkcheck.conclusion == 'failure'` on a step with no `continue-on-error`, so the job aborted before reaching it. `pr-title.yml` is deleted. The `type(scope): summary` convention stays in GOVERNANCE.md and the PR template, upheld by the reviewer at squash time rather than by a bot that turns away an external contributor over a capital letter.
`pnpm audit --audit-level high` fails on this tree: 33 high and 1 critical, none of them introduced by any branch — the repository has simply never run an audit. Blocking outright would mean fixing all 34 before any PR could merge, which is the same trap the type errors and the slug mismatches are in. So it joins them: `scripts/ci/audit-count.mjs` reports high + critical from `pnpm audit --json`, and the count ratchets at 34. It reads the JSON metadata rather than the table because the table omits severities that are at zero — a regex for "critical" would stop matching the day the last one is fixed, and a ratchet that stops matching fails. Worth naming, since the audit surfaced it: the critical is astro < 7.2.8, RCE through AVIF image optimization, and this repository is on ^7.2.3. Bumping Astro is its own change with its own build risk, not a side effect of wiring up CI.
…own workflows Both were jobs inside `ci-gate.yml`. They are now `on: workflow_call` workflows of their own, called from the gate with `uses: ./.github/workflows/…`. Being called rather than triggered is what keeps them useful: a called workflow runs inside the caller's run, so Internal links still downloads the artifact the build produced instead of building the site a second time — 8 s of download and unpack against 167 s of rebuild — and `CI gate` is still the single check to mark required, because a call reports its result through `needs` like any job. What the split buys is that each one can move. `internal-links.yml` takes the artifact name as an input, so it is not tied to this graph. And `design-system-adoption.yml` is the seam with the design system: the four-stage gate on chore/webkit-gate replaces its body, and eventually the whole file becomes one `uses:` of @aziontech/webkit's own reusable workflow — churn that now happens outside ci-gate.yml, which stays still. The aggregator reads the two results with bracket notation (`needs['internal-links'].result`); a hyphenated job id after a dot is ambiguous with subtraction in the expression grammar.
…workflows Both were `on: workflow_call`, called from `ci-gate.yml`. They now have their own `on: pull_request` trigger, their own concurrency group and their own status check, and the gate no longer knows about them. Three consequences, none of them free: - Branch protection now needs three names, not one: `CI gate`, `Internal links` and `Design system adoption`. The aggregator still covers everything inside ci-gate.yml, so adding or switching off a job in there is still a one-file change — but these two are outside it now. - Internal links builds the site itself. Artifacts do not cross runs, and a workflow with its own trigger runs in its own run, so the `dist` the gate produces is out of reach. That is ~2 min of runner against the 9 s the download and unpack used to cost. It runs in parallel with the gate, so wall-clock is unchanged. OG image generation is skipped — the checker reads HTML and sitemaps, never the images. - Nothing consumes the artifact any more, so the build stops packing and uploading it. That is 18 s per run of pure waste removed. It comes back the day a job inside the gate needs it. What independence buys: each can be re-run on its own, neither can be held up by an unrelated job, and the design-system one can be replaced wholesale — by the four-stage gate on chore/webkit-gate, and later by a `uses:` of @aziontech/webkit's own reusable workflow — without editing the gate.
… pass Two loose ends from the same source, both belonging to work Marcus did in #2307 and #2313. GOVERNANCE.md still said the CI checks the PR title and that a weekly link check opens an issue. This branch deleted both workflows, so both sentences were false — and by the document's own rule ("if another document in this repo contradicts this one, this one wins and the other gets a PR"), fixing that belongs here rather than in a follow-up. The section now describes the three required checks, what blocks, what ratchets and with which numbers, and what only reports. And `timeout-minutes` reaches the three deploy workflows. It was Marcus's discipline on the PR check from the start; the earlier pass in this branch gave them `concurrency` and `permissions` and forgot the timeout, leaving them on GitHub's six-hour default. Verified: every job in every workflow now declares a timeout, and no sentence in GOVERNANCE.md describes a check that does not exist.
The adoption report ran with `continue-on-error`, so the check was green with 72 violations across 30 files and an adoption score of 65%. A check that is always green while the code is full of violations is worse than no check: it teaches everyone that green means nothing, and it is the same failure mode as the link checker that reported "no link issues" for years because it parsed zero pages. It now ratchets, like the other four. The report still goes to the Summary first, unconditionally — the number reaches the reader whatever the ratchet decides — and then `pnpm ci:ratchet webkit-adoption` fails on a violation the branch ADDS. The 72 already here stay green; fixing one is reported, never punished, and the count can only go down. Baseline read from the report's own Markdown, not from a count on stderr, so the ratchet fails rather than passes if the report stops producing the number.
The ratchet was set at 72, so the check stayed green over the 72 violations already in the tree — which is not what was asked for. The baseline is now zero: no violation is tolerated. This check is red as of this commit and stays red until the 72 violations across 30 files are fixed. That is deliberate. Two rounds ago the step ran with `continue-on-error` and reported green over all of them; a ratchet at 72 kept the merge unblocked, which is the softer version of the same problem. The report still goes to the Summary first, so the number and the by-rule and by-file tables are there for whoever picks the work up: 26 no-style-override, 25 authoring-standards, 16 no-hardcoded-color, 2 no-hardcoded-motion, 2 prefer-tree-shakeable-root, 1 valid-import-path. The four other ratchets keep their inherited baselines — audit 34, astro check 54, slugs 745, links 21664 — and `scripts/ci/ratchet.mjs` now words a zero-baseline failure as "none are tolerated" instead of claiming the branch added all of them.
…fail Review follow-up, plus the class of defect the review names: a check that measures nothing must fail, not pass. Rebased onto f048e07. That rebuild of the template on webkit 5 moved every baseline, so they are re-snapshotted: audit 34 → 32, astro check 54 → 32, slugs 745 → 744, and design-system violations 72 → 0. The design-system check keeps its zero baseline and is now satisfiable — 86 of 86 UI files clean. Three ways a check could have measured nothing and reported success: - The link checker prints "Found no link issues. Great job!" when it parsed zero pages, and the ratchet read that as zero. That is the exact bug this PR was opened to fix, reachable again through a missing build or a base URL that matches no <loc>. It now throws instead, naming both causes. - The adoption report scored 100% when it saw no UI files at all — a stale or empty ESLint report, or a crash swallowed by the `|| true` in `lint:webkit`. It now throws. - `count > undefined` is false, so a missing or mistyped baseline in `ci/baselines.json` silently disabled its check. The ratchet now rejects a baseline that is not a number. The first guard earned its place immediately: the local build cannot resolve the pricing host, so it produced a `dist` with no sitemap, and the link check said so instead of reporting a clean run. Also from the review: `design-system-adoption.yml` loses its `paths:` filter. A required check whose path filter does not match never runs, never reports, and leaves branch protection waiting on it forever — the 36 s it costs on every PR is the cheaper half of that trade. And GOVERNANCE.md said the design-system check "reports; it never blocks" while the workflow blocked; it now describes what runs, and stops copying baseline numbers into prose where they go stale. The `linkcheck` baseline is deliberately left stale in this commit: the build needs network this machine does not have, so the honest number comes from CI.
isaque-bock-azion
force-pushed
the
chore/ci-gate
branch
from
September 11, 2026 18:31
3b28da3 to
2c8faef
Compare
26516, up from 21664. The template rebuild in f048e07 changed the header and the footer, and a template link counts once per page — 1628 of them — so a few edited components move the number by thousands. Almost all of the delta is the same docs↔site boundary as before: links to www.azion.com pages that live in the site repository. Taken from the CI run rather than measured here: the local build cannot resolve the pricing host, so it produces a dist with no sitemap. The guard added in the previous commit is what turned that into a clear failure instead of a green "Found no link issues". Without this, Internal links is red from the first merge — which is the blocking item in the review.
`Lint & format` and `Types` had the same trigger, the same condition and the same setup, and differed only in which command they ran. They are one job now. The split was buying independent failure reporting — a red `Types` next to a green `Lint & format` says what broke without opening anything — and paying a second checkout and install for it, about 24 s of runner. Every check in the merged job carries `if: always()`, which buys the same thing for free: ESLint, Prettier, Stylelint and `astro check` all run and all report, so one failure never hides the other three. The trade that remains is wall-clock. The two jobs ran in parallel, so the job took the slower of them; now it takes their sum. On the last green run that is about +7 s, against -24 s of runner. `steps.diff.outcome == 'success'` guards the three diff-scoped checks: with `always()` they would otherwise run against a file list that was never written.
isaque-bock-azion
added a commit
that referenced
this pull request
Sep 11, 2026
…taged files Two misfires in the lint infrastructure shipped by #2338: - The gate's Prettier step ran without --plugin-search-dir=., and pnpm's nested layout keeps prettier-plugin-astro out of prettier's own node_modules, so any .astro file in the diff died with "Couldn't resolve parser 'astro'" (exit 123). Pass the flag exactly as the format:code script already does. - The pre-commit hook linted the whole tree (npx eslint .), which trips on known debt and on local artifacts, blocking every commit. It now lints only staged files, mirroring the CI gate's changed-files ratchet.
isaque-bock-azion
added a commit
that referenced
this pull request
Sep 11, 2026
Replace the governance checklist template with webkit's three-section shape — Summary, How to test, Notes — adapted to this repo (build commands, permalink/redirect and i18n reminders folded into Notes). The old template still pointed at the PR-title lint that #2338 removed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
O que muda
O portão de PR hoje é um job: um build e o
test:frontmatterque roda dentro do script de build. Não há lint, typecheck, format check, link check nem slugcheck bloqueando um merge — embora todos esses scripts já estejam escritos neste repositório e simplesmente nunca sejam invocados.Este PR troca os workflows de PR por três workflows, três required checks, no formato do
governance.ymldoaziontech/webkit.changes(paths-filter) →security·content·lint·types·build→ agregadorif: always(),success | skipped= okO
CI gateagrega só o que está dentro doci-gate.yml, então ligar ou desligar um job ali continua sendo mudança de um arquivo. Os outros dois são independentes de propósito: gatilho, concurrency e check próprios, e nenhum tempaths:— um required check cujo filtro não casa nunca dispara, nunca reporta, e trava a branch protection esperando por ele.O
internal-linksconstrói o site sozinho (~2 min) porque artifact não atravessa run. Roda em paralelo com o gate, então o custo é de minutos de runner, não de wall-clock.Três coisas encontradas no caminho
1. O link check nunca checou nada.
lint:linkcheckcarregavabaseUrl: 'https://docs.astro.build', herdado do fork do Astro. O sitemap é gerado pelo Astro a partir deSITE_URL, então nenhum<loc>casava: toda execução parseava zero páginas e reportava "Found no link issues. Great job!".2. O cron semanal nunca abriu uma issue. A guarda era
if: steps.linkcheck.conclusion == 'failure'num step semcontinue-on-error, então o job abortava antes de chegar nela. Os dois somados: o repositório passou o tempo todo sem verificação de links, com um workflow verde dizendo o contrário.3. Uma RCE crítica nas dependências. O repositório nunca rodou
pnpm audit. A critical éastro < 7.2.8, execução remota de código via otimização de imagem AVIF. Subir o Astro é mudança própria, com risco de build próprio: entra em outro PR.Catracas, porque a dívida não cabe num PR
Contagens congeladas em
ci/baselines.json(scripts/ci/ratchet.mjs): podem cair, nunca subir.pnpm auditastro checkESLint e Prettier usam a outra forma: só os arquivos que o PR altera. Na árvore inteira são 97 erros de ESLint em 46 arquivos — dívida que não é de branch nenhum, e reformatar o repositório para chegar a zero enterraria todo diff futuro.
A adoção do design system é a exceção deliberada: baseline zero. Ela rodava com
continue-on-errore ficava verde sobre 72 violações; depois do rebuild do template em #2340 são 86 de 86 arquivos de UI limpos, então tolerância zero é satisfazível hoje e é o que impede a primeira regressão.Um check que não mede nada tem que falhar, não passar
É o princípio que este PR existe para instalar, e ele tinha três furos — todos fechados:
|| truedolint:webkit). Agora lança erro.count > undefinedé falso, então baseline ausente ou com typo desligava o check em silêncio. A catraca agora rejeita baseline que não é número.Herança do #2307 — o que preservamos, e o que estamos removendo
O CI deste repositório é do @marcus-souza-azion: o #2307 montou a governança e junto o CI mínimo que a sustentava; o #2313 tirou o pin de pnpm; o #2333 somou o
lint:navcheck. Este PR é a continuação, não a substituição — verificado arquivo por arquivo:concurrencypor PR +cancel-in-progresspermissions: contents: readno topotimeout-minutesversion:(#2313)setupcache: pnpm· heap de 8 GB ·build:locallint:navcheckbloqueante (#2333)contentpathsincluindo o próprio workflowgateDuas coisas dele estão sendo removidas — @marcus-souza-azion, é por isso que peço sua revisão:
pr-title.yml, por decisão de escopo. A convençãotype(scope): resumocontinua noGOVERNANCE.mde no template de PR, sustentada pelo revisor no squash.weekly-linkcheck.yml— a intenção vira gate de PR, mas a implementação sai, pelos dois defeitos acima. O padrão da issue com dedup do seu weekly não se perde: está reservado para onightly.yml, reusando oactions/github-scriptcomo você escreveu.O
GOVERNANCE.mdfoi atualizado aqui, porque ele afirmava que o CI valida o título do PR e que o cron abre issue — as duas frases ficariam falsas — e descrevia a adoção do design system como "never blocks" enquanto o workflow bloqueia.Também consertado
azion-deploy.ymldeletado: nunca foi executável (comentários indentados com TAB, erro de parse de YAML, enpm installnum repo cujopreinstalléonly-allow pnpm). E nos três deploys, oworkflow_dispatchdoprod.ymlestava aninhado dentro dopush:— deploy manual de produção não existia; os segredos do Algolia iam porargv; nenhum declaravaconcurrencynemtimeout-minutes.Como testar
O gate roda neste próprio PR. Vale conferir que o
SummarydoCI gatetraz a tabela das catracas comagora,baselineedelta, e que um PR que só mexe em.mdxpulalintetypes.Antes de tornar os três required: rodar em paralelo com os checks atuais por alguns PRs.
Fora de escopo
Cada uma no seu PR:
redirect-gate(permalink removido exige redirect no mesmo PR); a11y compa11y-ci;nightly.ymlpara links externos etranslation-status; SHA pinning no resto edependabot.yml. Preview por PR,zizmor.ymle o jobOSSF Scorecardficaram fora por decisão — vale registrar que a ruleset da organização exige esse último nome em repo público.