From f36aa8d85864cec1a1bc9c51dc7bc10d46b98501 Mon Sep 17 00:00:00 2001 From: Erick Kinnee Date: Sun, 26 Jul 2026 20:11:59 -0500 Subject: [PATCH] chore(ci): add release PR build gate --- .github/workflows/quality.yml | 25 +++++++ .../maintenance/7307-release-pr-build-gate.md | 2 + docs/architecture/QUALITY_GATES.md | 74 +++++++++++-------- docs/ops/QUALITY_GATE_PLAYBOOK.md | 13 ++-- docs/ops/RELEASE_GREEN.md | 16 +++- tests/unit/build/check-workflows.test.ts | 39 ++++++++++ 6 files changed, 131 insertions(+), 38 deletions(-) create mode 100644 changelog.d/maintenance/7307-release-pr-build-gate.md diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 858618e420c0..9be555f0d7e3 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -57,6 +57,31 @@ jobs: git diff --name-only "$BASE_SHA" "$HEAD_SHA" > changed-files.txt node scripts/quality/classify-pr-changes.mjs changed-files.txt >> "$GITHUB_OUTPUT" + build: + name: Build (advisory) + 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 — same fork-safe rule as ci.yml / fast-gates. + 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' }} + # #7307: advisory for the first week of release-PR runs; remove + # continue-on-error after the production-build signal is stable. + continue-on-error: true + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + with: + node-version: ${{ env.CI_NODE_VERSION }} + cache: npm + - uses: ./.github/actions/npm-ci-retry + - run: npm run check:node-runtime + - run: npm run build + env: + OMNIROUTE_USE_TURBOPACK: "1" + # No artifact upload here: the PR-to-release quality workflow has no + # downstream package/e2e jobs that consume the Next.js build output. + # Docs/OpenAPI contract gates only — existence reason is doc accuracy + route refs. # Split out of fast-gates so pure-docs PRs skip typecheck/unit while still validating docs. docs-gates: diff --git a/changelog.d/maintenance/7307-release-pr-build-gate.md b/changelog.d/maintenance/7307-release-pr-build-gate.md new file mode 100644 index 000000000000..943b08b59078 --- /dev/null +++ b/changelog.d/maintenance/7307-release-pr-build-gate.md @@ -0,0 +1,2 @@ +- Add an advisory production build to the PR-to-release quality workflow for non-draft code + changes and Mergify queue branches, mirroring the Node 24 Turbopack build path from full CI. diff --git a/docs/architecture/QUALITY_GATES.md b/docs/architecture/QUALITY_GATES.md index 5fa94abd278d..c7ba27add0f1 100644 --- a/docs/architecture/QUALITY_GATES.md +++ b/docs/architecture/QUALITY_GATES.md @@ -18,34 +18,50 @@ in `CLAUDE.md`. Scripts live under `scripts/check/` (policy gates) and `scripts/quality/` (ratchet engine). The CI source of truth is `.github/workflows/ci.yml`. +### Release PR fast-path (`quality.yml`) + +`.github/workflows/quality.yml` runs on PRs targeting `release/**`. It keeps contributor +branches moving with path-filtered fast gates, plus one advisory production-build signal for code +changes: + +| Job | Scope | Blocking | +| ------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------- | +| `Build (advisory)` | Non-draft code PRs and Mergify queue branches; Node 24, `npm-ci-retry`, `check:node-runtime`, `npm run build` with `OMNIROUTE_USE_TURBOPACK=1`; no artifact upload because no downstream quality job consumes it | **Advisory** (`continue-on-error: true`; remove after one week of stable release-PR runs) | +| `Docs Gates (fast-path)` | Docs/code PRs; API docs refs and docs-all | Yes | +| `Fast Quality Gates` | Code PRs; static checks, typecheck, dashboard typecheck, impacted unit tests | Yes | +| `Vitest (fast-path)` | Code PRs; fast vitest suite | Yes | +| `Unit Tests fast-path` | Code PRs; 4-shard unit suite | Yes | +| `No new ESLint warnings` | Code PRs; suppressions-aware lint guard | Yes for own-origin, advisory for forks | +| `Merge integrity (changelog + generated skills)` | Non-draft PRs; changelog and generated skill sync | Yes for own-origin, advisory for forks | + ### Job: `lint` Runs on every PR to `main`. Blocks merge on failure. -| Script (`npm run ...`) | Validates | Blocking | -| ------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------------------------------------- | -| `check:node-runtime` | Node.js version is within the supported range | Yes | -| `check:cycles` | Circular imports — all `src/` + `open-sse/` modules | Yes | -| `check:route-validation:t06` | Zod schemas present on all routes (Tier 6 policy) | Yes | -| `check:any-budget:t11` | `@ts-expect-error // any` count does not exceed budget (Tier 11 catraca) | Yes | -| `check:provider-consistency` | Every provider in `providers.ts` has a matching entry in `providerRegistry.ts` (and vice-versa, within the allowlist) | Yes | -| `check:fetch-targets` | Every `fetch("/api/...")` in client-side `src/` resolves to a real `route.ts` | Yes | -| `check:deps` | All `npm install`-able deps across every `package.json` in the repo are in `dependency-allowlist.json`; new unpinned or slopsquatted packages flagged | Yes | -| `audit:deps` | `npm audit` (root + electron) — no high/critical advisories (overlaps osv `check:vuln-ratchet`; see Rationalization Backlog) | Yes | -| `check:lockfile` | `package-lock.json` integrity — https registry, integrity hashes, no host overrides | Yes | -| `check:licenses` | SPDX license allowlist for production dependencies | Yes | -| `check:tracked-artifacts` | No build artifacts / committed `node_modules` symlinks (also runs in husky pre-commit; pre-push is intentionally light — #6716) | Yes | -| `check:file-size` | No source file exceeds the per-extension cap (ratchet: frozen large files in `frozen` list) | Yes | -| `check:error-helper` | Error responses in executors/handlers use `buildErrorBody()` / `sanitizeErrorMessage()` (Hard Rule #12) | Yes | -| `check:migration-numbering` | Migration SQL files are sequentially numbered, no gaps or duplicates | Yes | -| `check:public-creds` | No literal OAuth `client_id`/`client_secret` or Firebase Web keys outside `publicCreds.ts` (Hard Rule #11) | Yes | -| `check:db-rules` | No raw SQL outside `src/lib/db/` modules; no barrel-imports from `localDb.ts` (Hard Rules #2/#5) | Yes | -| `check:known-symbols` | Provider executors, routing strategies, and translators registered in their dispatch tables match the files on disk — no orphaned or undeclared symbols | Yes | -| `check:route-guard-membership` | Every route that spawns a child process is classified by `isLocalOnlyPath()` (Hard Rules #15/#17) | Yes | -| `check:test-discovery` | Every `*.test.ts` / `*.spec.ts` file in the repo is collected by at least one test runner (ratchet: orphan list in `test-discovery-baseline.json` can only shrink) | Yes | -| `check:docs-sync` | CHANGELOG version, OpenAPI version, and `llm.txt` are in sync | Yes | -| `typecheck:core` | TypeScript compilation without errors (advisory warnings only) | Yes | -| `typecheck:noimplicit:core` | Strict `noImplicitAny` — forward-looking; many pre-existing call sites still need annotations | **Advisory** (`continue-on-error: true`) | +| Script (`npm run ...`) | Validates | Blocking | +| ------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------- | +| `check:node-runtime` | Node.js version is within the supported range | Yes | +| `check:cycles` | Circular imports — all `src/` + `open-sse/` modules | Yes | +| `check:route-validation:t06` | Zod schemas present on all routes (Tier 6 policy) | Yes | +| `check:any-budget:t11` | `@ts-expect-error // any` count does not exceed budget (Tier 11 catraca) | Yes | +| `check:provider-consistency` | Every provider in `providers.ts` has a matching entry in `providerRegistry.ts` (and vice-versa, within the allowlist) | Yes | +| `check:fetch-targets` | Every `fetch("/api/...")` in client-side `src/` resolves to a real `route.ts` | Yes | +| `check:deps` | All `npm install`-able deps across every `package.json` in the repo are in `dependency-allowlist.json`; new unpinned or slopsquatted packages flagged | Yes | +| `audit:deps` | `npm audit` (root + electron) — no high/critical advisories (overlaps osv `check:vuln-ratchet`; see Rationalization Backlog) | Yes | +| `check:lockfile` | `package-lock.json` integrity — https registry, integrity hashes, no host overrides | Yes | +| `check:licenses` | SPDX license allowlist for production dependencies | Yes | +| `check:tracked-artifacts` | No build artifacts / committed `node_modules` symlinks (also runs in husky pre-commit; pre-push is intentionally light — #6716) | Yes | +| `check:file-size` | No source file exceeds the per-extension cap (ratchet: frozen large files in `frozen` list) | Yes | +| `check:error-helper` | Error responses in executors/handlers use `buildErrorBody()` / `sanitizeErrorMessage()` (Hard Rule #12) | Yes | +| `check:migration-numbering` | Migration SQL files are sequentially numbered, no gaps or duplicates | Yes | +| `check:public-creds` | No literal OAuth `client_id`/`client_secret` or Firebase Web keys outside `publicCreds.ts` (Hard Rule #11) | Yes | +| `check:db-rules` | No raw SQL outside `src/lib/db/` modules; no barrel-imports from `localDb.ts` (Hard Rules #2/#5) | Yes | +| `check:known-symbols` | Provider executors, routing strategies, and translators registered in their dispatch tables match the files on disk — no orphaned or undeclared symbols | Yes | +| `check:route-guard-membership` | Every route that spawns a child process is classified by `isLocalOnlyPath()` (Hard Rules #15/#17) | Yes | +| `check:test-discovery` | Every `*.test.ts` / `*.spec.ts` file in the repo is collected by at least one test runner (ratchet: orphan list in `test-discovery-baseline.json` can only shrink) | Yes | +| `check:docs-sync` | CHANGELOG version, OpenAPI version, and `llm.txt` are in sync | Yes | +| `typecheck:core` | TypeScript compilation without errors (advisory warnings only) | Yes | +| `typecheck:noimplicit:core` | Strict `noImplicitAny` — forward-looking; many pre-existing call sites still need annotations | **Advisory** (`continue-on-error: true`) | | `check:dashboard-typecheck` | `tsc` scoped to `src/app/(dashboard)/**` (#7033) — `typecheck:core`'s curated 27-file allowlist does not include any dashboard TSX, and `next build` never type-checks it either (`next.config.mjs` sets `ignoreBuildErrors: true`), so orphaned-identifier regressions there (#6625/#6909) were invisible to CI. Diffs against a frozen per-file/per-TS-code count baseline (`config/quality/dashboard-typecheck-baseline.json`, same stale-enforcement pattern as `check:known-symbols`) — only NEW errors beyond the baselined count fail the gate; ratchet down with `--update` when a pre-existing error is fixed. | Yes | ### Job: `quality-gate` @@ -179,11 +195,11 @@ pending implementation). Retry is per-runner, never a global blanket — a blanket retry converts real regressions into invisible flakes: -| Runner | Policy | Why | -| --- | --- | --- | -| Playwright (e2e) | `retries: 1` in CI only, with `trace: on-first-retry` | Browser/network timing is genuinely nondeterministic; one retry with a trace turns a flake into a diagnosable artifact | -| Vitest | NO global retry. A proven-flaky test gets an explicit per-test retry (visible in the diff, reviewed in PR) | Keeps the quarantine list in the repo, never opaque | -| node:test (unit) | NO retry, ever | A flaky unit test is a bug in the test — fix it, don't re-roll it | +| Runner | Policy | Why | +| ---------------- | ---------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------- | +| Playwright (e2e) | `retries: 1` in CI only, with `trace: on-first-retry` | Browser/network timing is genuinely nondeterministic; one retry with a trace turns a flake into a diagnosable artifact | +| Vitest | NO global retry. A proven-flaky test gets an explicit per-test retry (visible in the diff, reviewed in PR) | Keeps the quarantine list in the repo, never opaque | +| node:test (unit) | NO retry, ever | A flaky unit test is a bug in the test — fix it, don't re-roll it | Target SLOs once flake telemetry lands (WS5.2/5.3): <1% flake rate per test ("fix now" threshold), ≥95% pass rate per pipeline. Industry reference values — diff --git a/docs/ops/QUALITY_GATE_PLAYBOOK.md b/docs/ops/QUALITY_GATE_PLAYBOOK.md index add5d775b42e..040b6f3a8e0d 100644 --- a/docs/ops/QUALITY_GATE_PLAYBOOK.md +++ b/docs/ops/QUALITY_GATE_PLAYBOOK.md @@ -62,11 +62,11 @@ alignment signal (we didn't copy a checklist; we converged on the right practice ### Honest weaknesses (real gaps) -1. **🔴 The fast-gates split is a structural hole.** `quality.yml` (PR→`release/**`) runs **only - filesystem gates** — no typecheck, no tests, no build, no coverage. A typecheck/test regression - passes in a release PR and only blows up on the forward-merge to `main`. The motivation - (speed) is valid, but the gate should be where the merge happens (shift-left). **Largest - pending structural fix.** +1. **🔴 The fast-gates split still leaves a structural hole.** `quality.yml` (PR→`release/**`) + now runs typecheck, fast deterministic tests, and an advisory production build for code PRs, + but it still does not run the full release-PR surface from `ci.yml` (coverage ratchets, + package artifact, integration, E2E, SonarQube). The motivation (speed) is valid, but the gate + should be where the merge happens (shift-left). **Largest pending structural fix.** 2. **🟠 Gate sprawl/fatigue risk.** ~46 gates + 25 jobs is A LOT. Sonar itself warns: too many conditions cause "gate fatigue" and priority debates, with risk of a gate being ignored. DORA warns that heavy gates cost lead-time. We mitigate with advisory tiers and @@ -236,7 +236,8 @@ gates (for AI: injection red-team). _Output: structural rot and domain failures **P0 — highest ROI, almost ready** 1. **Mutation score ratchet** (after the 1st nightly Stryker produces values). Key antidote against coverage-Goodhart; ~90% done. -2. **Close the fast-gates hole** — add typecheck + impacted tests to `quality.yml` (PR→release). +2. **Close the remaining fast-gates hole** — promote the `quality.yml` production build after its + advisory week and keep moving deterministic release-PR-only checks into the PR→release path. 3. **Branch-protection on `main`** (owner setting) — boosts Scorecard, closes the DSOMM gap. **P1 — valuable** 4. **osv/oasdiff → blocking with the right scope** — osv only CRITICAL+fixable (two-step like Trivy); oasdiff blocks breaking-changes. 5. **`require-tighten` → blocking** (end of cycle) — locks in metric gains. 6. **ROI/timing review per-gate** in `ci-summary` — find and prune slow/low-value gates. diff --git a/docs/ops/RELEASE_GREEN.md b/docs/ops/RELEASE_GREEN.md index 3a9a3cd84d4a..e1c0a9fcc0c1 100644 --- a/docs/ops/RELEASE_GREEN.md +++ b/docs/ops/RELEASE_GREEN.md @@ -8,9 +8,10 @@ title: "Release-Green — keeping the queue and release branch green" The **full gate** (`.github/workflows/ci.yml` — unit shards, vitest, ratchets, `package-artifact`, SonarQube, E2E) runs **only on the release PR** (PR → `main`). PRs targeting -`release/**` receive only the **fast-gates** (`quality.yml`: TIA-impacted tests + typecheck + -lint). Consequence: reds accumulate silently on the release branch and **explode in layers -of ~40 min** at release time, one at a time. +`release/**` receive the **fast-gates** (`quality.yml`: TIA-impacted tests + typecheck + lint) +and, for code changes, an **advisory** production build. Consequence: release-only reds can still +accumulate silently on the release branch and **explode in layers of ~40 min** at release time, +one at a time. The "release-green family" exists to **anticipate** those reds — validate the equivalent of the full gate **locally / outside of release**, at any time, so the release PR is already @@ -34,6 +35,15 @@ green on its first CI run. run **periodically, between releases**. Running independently is the normal use — release is just the moment when running it yields the most value. +## PR-to-release advisory build + +`quality.yml` now includes `Build (advisory)` for non-draft code PRs and Mergify queue branches. +It mirrors the production build recipe from `ci.yml`: Node 24, `npm-ci-retry`, +`check:node-runtime`, and `npm run build` with `OMNIROUTE_USE_TURBOPACK=1`. It intentionally +does not upload a build artifact because no downstream quality job consumes one in this workflow. +Remove `continue-on-error` after one week of stable release-PR runs so the signal becomes a +blocking PR-to-release gate. + ## Solution C — `npm run check:release-green` (the engine) Reproduces release-equivalent validation against the current working tree and classifies each red: diff --git a/tests/unit/build/check-workflows.test.ts b/tests/unit/build/check-workflows.test.ts index 19b27ae798aa..40eb377fd399 100644 --- a/tests/unit/build/check-workflows.test.ts +++ b/tests/unit/build/check-workflows.test.ts @@ -33,6 +33,11 @@ const evaluateZizmor = evaluateZizmorRatchet as ( baseline: number ) => RatchetVerdict; const readZizmorBaseline = readBaselineZizmorValue as (p?: string) => number | null; +const qualityWorkflowPath = new URL("../../../.github/workflows/quality.yml", import.meta.url); + +function readQualityWorkflow(): string { + return fs.readFileSync(qualityWorkflowPath, "utf8"); +} // ───────────────────────────────────────────────────────────────────────────── // parseActionlintOutput @@ -308,3 +313,37 @@ test("readBaselineZizmorValue: invalid JSON returns null (does not throw)", () = assert.equal(readZizmorBaseline(p), null); }); }); + +// ───────────────────────────────────────────────────────────────────────────── +// quality.yml — release PR build gate regression coverage (#7307) +// ───────────────────────────────────────────────────────────────────────────── + +test("#7307 quality.yml adds an advisory production build for release PR code changes", () => { + const source = readQualityWorkflow(); + const buildJob = source.match(/\n build:\n[\s\S]*?\n # Docs\/OpenAPI contract gates only/); + + assert.match(source, /pull_request:\n\s+branches: \["release\/\*\*"\]/); + assert.ok(buildJob, "quality.yml must define the build job before docs-gates"); + assert.match(buildJob[0], /name: Build \(advisory\)/); + assert.match(buildJob[0], /needs: changes/); + assert.match(buildJob[0], /needs\.changes\.outputs\.code == 'true'/); + assert.match(buildJob[0], /github\.event\.pull_request\.draft == false/); + assert.match(buildJob[0], /startsWith\(github\.head_ref, 'mergify\/merge-queue\/'\)/); + assert.match( + buildJob[0], + /github\.event\.pull_request\.head\.repo\.full_name == github\.repository/ + ); + assert.match(buildJob[0], /fromJSON\('\["self-hosted","omni-release"\]'\) \|\| 'ubuntu-latest'/); + assert.match(buildJob[0], /continue-on-error: true/); + assert.match(buildJob[0], /uses: actions\/checkout@[0-9a-f]{40} # v7/); + assert.match(buildJob[0], /uses: actions\/setup-node@[0-9a-f]{40} # v7/); + assert.match(buildJob[0], /uses: \.\/\.github\/actions\/npm-ci-retry/); + assert.match(buildJob[0], /npm run check:node-runtime/); + assert.match(buildJob[0], /npm run build/); + assert.match(buildJob[0], /OMNIROUTE_USE_TURBOPACK: "1"/); + assert.doesNotMatch(buildJob[0], /actions\/upload-artifact/); + assert.match( + buildJob[0], + /remove\s+# continue-on-error after the production-build signal is stable/ + ); +});