From 8f18682880320089dd95c5db23a9c7d6dbf3a4ad Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 26 Aug 2026 17:56:43 +0000 Subject: [PATCH] fix(ci): run the ship-docsync gate E2E in the evals matrix + silent-skip tripwire MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The evals.yml matrix is hand-enumerated and the Run step never exported EVALS_TIER, so the new whole-file-gated ship-docsync E2E would have self-skipped even with a row — a hollow green one layer deeper than the documented rehomed-monolith incident. Add the e2e-ship-docsync row with a row-level `tier: gate` property, exported as EVALS_TIER by the Run step (empty = unset for every existing row: all readers are `=== ''` or truthiness). New free tripwire test/evals-workflow-matrix.test.ts ratchets the class: matrix files must exist; gate-hosting files must have a row; whole-file-gated matrix files must carry a matching row tier; and the burn-down lists enforce their own cleanup. It enumerates the PRE-EXISTING holes found while wiring this (8 gate-hosting files with no row; codex/gemini rows running zero tests; the pty-plan-smoke row hollow since its files adopted describeE2ETier) — tracked in TODOS as the CI gate-lane hollow-coverage burn-down. Co-Authored-By: Claude Fable 5 --- .github/workflows/evals.yml | 17 +++ TODOS.md | 28 +++++ test/evals-workflow-matrix.test.ts | 196 +++++++++++++++++++++++++++++ 3 files changed, 241 insertions(+) create mode 100644 test/evals-workflow-matrix.test.ts diff --git a/.github/workflows/evals.yml b/.github/workflows/evals.yml index 2a5916dbe..cfe7cbc0b 100644 --- a/.github/workflows/evals.yml +++ b/.github/workflows/evals.yml @@ -135,6 +135,17 @@ jobs: file: test/skill-e2e-coverage-audit.test.ts - name: e2e-triage file: test/skill-e2e-triage.test.ts + # ship-docsync is whole-file tier-gated (describeE2ETier('gate') keeps + # it out of the periodic shard census), so its row MUST set tier: gate + # — without it the self-gate skips every test and the job reports a + # hollow green (the same silent-skip class as the rehomed monolith + # above, one layer deeper). The Run step exports EVALS_TIER from this + # property; rows without it keep EVALS_TIER empty (= unset: every + # reader is `=== ''` or truthiness). Enforced by + # test/evals-workflow-matrix.test.ts. + - name: e2e-ship-docsync + file: test/skill-e2e-ship-docsync.test.ts + tier: gate - name: e2e-routing file: test/skill-routing-e2e.test.ts - name: e2e-codex @@ -337,6 +348,12 @@ jobs: GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }} EVALS_CONCURRENCY: "40" PLAYWRIGHT_BROWSERS_PATH: /opt/playwright-browsers + # Per-row tier activation for whole-file-gated suites. Empty when the + # row declares no tier — every EVALS_TIER reader treats empty as unset + # (`=== ''` comparisons and the truthiness check in + # test/helpers/e2e-helpers.ts:70), so untiered rows are byte-for-byte + # unaffected. + EVALS_TIER: ${{ matrix.suite.tier || '' }} run: EVALS=1 bun test --retry ${{ matrix.suite.retries || 1 }} --concurrent --max-concurrency 40 ${{ matrix.suite.file }} - name: Upload eval results diff --git a/TODOS.md b/TODOS.md index 535ccacf5..d8f0d0936 100644 --- a/TODOS.md +++ b/TODOS.md @@ -2378,6 +2378,34 @@ pin) and `test/skill-e2e-ship-docsync.test.ts` (dispatch E2E, gate tier). **Priority:** P3 **Depends on:** None +### CI gate-lane hollow-coverage burn-down (evals.yml matrix) + +**What:** `test/evals-workflow-matrix.test.ts` (added v1.70.1.0) ratchets two +pre-existing CI coverage holes; burn them down. (1) Eight gate-hosting test +files have no `evals.yml` matrix row, so CI never runs them +(`KNOWN_MATRIX_GAPS` in the test enumerates them — notably the plan-mode and +finding-floor smokes and the AUQ format-compliance gate). (2) Four matrix rows +point at whole-file tier-gated files but set no row `tier:` property, so with +`EVALS_TIER` unexported those suites self-skip: `codex-e2e`/`gemini-e2e` run +ZERO tests and report green on every PR (vestigial rows; the periodic cron +lane owns them — consider deleting the rows), and `e2e-pty-plan-smoke` spends +~7 min on setup then skips every describe (hollow-green since the files +adopted `describeE2ETier('gate')` — set `tier: gate` on the row to reactivate, +after confirming the smokes still pass). + +**Why:** "Gate tier blocks merge" is silently false for these files. Each fix +is a deliberate cost/flake decision (activating paid suites on every PR), so +they're enumerated instead of drive-by-fixed. The mechanism already exists: +per-row `tier:` property, exported as `EVALS_TIER` by the Run step. + +**Context:** Found 2026-08-26 on PR #2700 while adding the `ship-docsync` row. +Fix = add/adjust the matrix row, then DELETE the corresponding burn-down entry +(the tripwire fails on stale entries, so cleanup is enforced). + +**Effort:** S per file (mechanical) + one burn-in run each to confirm green +**Priority:** P2 +**Depends on:** None + ### Periodic paid-test shard census is one ungated file from the detach-timeout floor **What:** The periodic tier's shard census is 67 files — one ungated slot below diff --git a/test/evals-workflow-matrix.test.ts b/test/evals-workflow-matrix.test.ts new file mode 100644 index 000000000..5ae3f8958 --- /dev/null +++ b/test/evals-workflow-matrix.test.ts @@ -0,0 +1,196 @@ +/** + * CI eval-matrix completeness tripwire — kills the silent-skip class where a + * gate-tier test exists in the repo but the hand-enumerated matrix in + * .github/workflows/evals.yml never runs it, so "gate tier blocks merge" is + * quietly false in CI. This has happened before (see the "rehomed from the + * deleted pre-split monolith" comment in evals.yml) and was found again on + * PR #2700: nine gate-hosting files absent from the matrix, plus matrix rows + * whose whole-file tier guards can never fire because the Run step exported + * no EVALS_TIER. + * + * Ratchet, not amnesty: the KNOWN_* lists below enumerate the PRE-EXISTING + * gaps with reasons, so no NEW gap can land while the backlog burns down + * (same pattern as SCANNER_EXEMPT in egress-receipt-wiring). If you fix a + * listed gap (add its matrix row / tier property), this test FAILS until you + * remove the entry — stale exemptions are enforced, not decorative. + * + * Wiring pinned: + * - every matrix `file:` path exists on disk (no stale rows), + * - every gate-hosting paid file (whole-file gate self-gate, or named in the + * dep list of a gate-tier E2E_TOUCHFILES key) appears in the matrix or in + * KNOWN_MATRIX_GAPS, + * - every matrix file with a whole-file tier guard has a matching row-level + * `tier:` property (else the suite self-skips and the job is hollow-green) + * or sits in KNOWN_TIER_UNSET. + */ +import { describe, test, expect } from 'bun:test'; +import * as fs from 'fs'; +import * as path from 'path'; +import { E2E_TOUCHFILES, E2E_TIERS } from './helpers/touchfiles-data'; +import { isPaidTestFile } from './helpers/paid-test-set'; + +const ROOT = path.join(import.meta.dir, '..'); +const WORKFLOW = path.join(ROOT, '.github', 'workflows', 'evals.yml'); + +/** + * Pre-existing gate-hosting files with no matrix row (found 2026-08-26, + * PR #2700). Adding a row activates real paid runs on every PR — a cost and + * flake-surface decision per file, tracked in TODOS.md ("CI gate-lane + * hollow-coverage burn-down"). Fix = add a matrix row (plus `tier: gate` when + * the file is whole-file gated), then DELETE the entry here. + */ +const KNOWN_MATRIX_GAPS = new Set([ + 'test/skill-e2e-ask-user-question-format-compliance.test.ts', + 'test/skill-e2e-hermetic-canary.test.ts', + 'test/skill-e2e-ios.test.ts', + 'test/skill-e2e-plan-ceo-finding-floor.test.ts', + 'test/skill-e2e-plan-ceo-plan-mode.test.ts', + 'test/skill-e2e-plan-design-with-ui.test.ts', + 'test/skill-e2e-plan-devex-finding-floor.test.ts', + 'test/skill-e2e-plan-devex-plan-mode.test.ts', +]); + +/** + * Matrix files whose whole-file tier guard has no matching row `tier:` + * property (pre-existing, found 2026-08-26). Consequences today: + * - codex-e2e / gemini-e2e declare 'periodic' → both jobs run ZERO tests and + * report green on every PR (vestigial rows; the periodic cron lane owns + * these suites). + * - the two PTY plan-mode smokes declare 'gate' → the e2e-pty-plan-smoke job + * spends ~7 min on container setup and skill registration, then bun test + * skips every describe — hollow-green since the files adopted + * describeE2ETier. + * Fixing either means deliberately (re)activating paid suites on every PR — + * tracked in the same TODOS burn-down. Fix = add `tier:` to the row (or + * delete the vestigial row), then DELETE the entry here. + */ +const KNOWN_TIER_UNSET = new Map([ + ['test/codex-e2e.test.ts', 'periodic'], + ['test/gemini-e2e.test.ts', 'periodic'], + ['test/skill-e2e-office-hours-auto-mode.test.ts', 'gate'], + ['test/skill-e2e-plan-mode-no-op.test.ts', 'gate'], +]); + +interface MatrixRow { + name: string; + files: string[]; + tier?: string; +} + +/** Parse the `matrix: suite:` rows (name / file / optional tier) from evals.yml. */ +function parseMatrixRows(source: string): MatrixRow[] { + const rows: MatrixRow[] = []; + let current: MatrixRow | null = null; + for (const line of source.split('\n')) { + const name = line.match(/^\s+- name: (\S+)\s*$/); + if (name) { + if (current) rows.push(current); + current = { name: name[1], files: [] }; + continue; + } + if (!current) continue; + const file = line.match(/^\s+file: (.+?)\s*$/); + if (file) current.files.push(...file[1].trim().split(/\s+/)); + const tier = line.match(/^\s+tier: (\S+)\s*$/); + if (tier) current.tier = tier[1]; + // `steps:` ends the strategy block — stop before step-level keys leak in. + if (/^\s{4}steps:\s*$/.test(line)) break; + } + if (current) rows.push(current); + return rows.filter((r) => r.files.length > 0); +} + +const wholeFileTier = (source: string): string | null => { + const m = + /\b(?:describeE2ETier|e2eTierEnabled)\(\s*['"`](gate|periodic)['"`]/.exec(source) || + /EVALS_TIER\s*===\s*['"`](gate|periodic)['"`]/.exec(source); + return m ? m[1] : null; +}; + +const workflowSource = fs.readFileSync(WORKFLOW, 'utf-8'); +const rows = parseMatrixRows(workflowSource); +const matrixFiles = new Map(); +for (const row of rows) for (const f of row.files) matrixFiles.set(f, row); + +const paidFiles = fs + .readdirSync(path.join(ROOT, 'test')) + .filter((f) => f.endsWith('.test.ts')) + .map((f) => `test/${f}`) + .filter(isPaidTestFile); + +describe('evals.yml matrix completeness (gate-lane silent-skip tripwire)', () => { + test('matrix parse sanity: rows and known suites present', () => { + expect(rows.length).toBeGreaterThanOrEqual(15); + expect(matrixFiles.has('test/skill-e2e-workflow.test.ts')).toBe(true); + expect(matrixFiles.has('test/skill-e2e-ship-docsync.test.ts')).toBe(true); + }); + + test('every matrix file exists on disk', () => { + const missing = [...matrixFiles.keys()].filter( + (f) => !fs.existsSync(path.join(ROOT, f)) + ); + expect(missing).toEqual([]); + }); + + test('every gate-hosting paid file is in the matrix (or the documented backlog)', () => { + const gaps: string[] = []; + for (const file of paidFiles) { + const source = fs.readFileSync(path.join(ROOT, file), 'utf-8'); + const declaresGate = wholeFileTier(source) === 'gate'; + const inGateDeps = Object.entries(E2E_TOUCHFILES).some( + ([key, deps]) => + (E2E_TIERS as Record)[key] === 'gate' && + (deps as string[]).includes(file) + ); + if (!declaresGate && !inGateDeps) continue; + if (matrixFiles.has(file) || KNOWN_MATRIX_GAPS.has(file)) continue; + gaps.push(file); + } + expect( + gaps, + `Gate-hosting test file(s) missing from the evals.yml matrix — CI will ` + + `never run them and "gate tier blocks merge" becomes silently false. ` + + `Add a matrix row (with tier: gate when the file is whole-file gated). ` + + `Do NOT extend KNOWN_MATRIX_GAPS for new files.` + ).toEqual([]); + }); + + test('matrix rows for whole-file-gated files carry a matching tier property', () => { + const mismatches: string[] = []; + for (const [file, row] of matrixFiles) { + if (!fs.existsSync(path.join(ROOT, file))) continue; + const declared = wholeFileTier(fs.readFileSync(path.join(ROOT, file), 'utf-8')); + if (!declared) continue; + if (row.tier === declared) continue; + if (KNOWN_TIER_UNSET.get(file) === declared && row.tier === undefined) continue; + mismatches.push(`${file} declares '${declared}' but row '${row.name}' has tier: ${row.tier ?? 'unset'}`); + } + expect( + mismatches, + `A whole-file tier guard with no matching row tier means the suite ` + + `self-skips and the CI job reports a hollow green. Set tier: ` + + `on the row (the Run step exports it as EVALS_TIER).` + ).toEqual([]); + }); + + test('burn-down lists hold only live gaps (ratchet cleanup enforcement)', () => { + const staleGaps = [...KNOWN_MATRIX_GAPS].filter( + (f) => matrixFiles.has(f) || !fs.existsSync(path.join(ROOT, f)) + ); + expect( + staleGaps, + 'Entry fixed or file removed — delete it from KNOWN_MATRIX_GAPS.' + ).toEqual([]); + const staleTiers = [...KNOWN_TIER_UNSET.entries()].filter(([f, declared]) => { + const row = matrixFiles.get(f); + if (!row) return true; // row deleted — entry no longer applies + if (row.tier === declared) return true; // fixed — entry must go + if (!fs.existsSync(path.join(ROOT, f))) return true; + return wholeFileTier(fs.readFileSync(path.join(ROOT, f), 'utf-8')) !== declared; + }); + expect( + staleTiers.map(([f]) => f), + 'Entry fixed, row removed, or guard changed — delete it from KNOWN_TIER_UNSET.' + ).toEqual([]); + }); +});