diff --git a/AGENTS.md b/AGENTS.md index e17fe588f9..8a9f22f012 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -335,6 +335,76 @@ When the user says `open PR`, `create PR`, or `publish PR` without also requesti - For pull requests that touch ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output, complete the clinical governance preflight in `.github/pull_request_template.md`. - Track known verification debts and staged process improvements in `docs/process-hardening.md` instead of relying on chat-only memory. +## Do not pay twice for the verdict GitHub is about to reach + +`check:gate-manifest` enforces a one-way invariant: CI never runs LESS of the local +`verify:cheap` static set than the local chain does. Read that the other way and it says +something uncomfortable — **every local run of a gate in that chain is work GitHub is +about to repeat.** `gate-receipts.mjs` removed the local-versus-local duplication (the +same gate twice on unchanged content); it explicitly cannot touch this one, because CI +must never reuse a receipt. + +That does not make the local run waste. It is a **bet**: a local run that fails saves a CI +round trip, and a red or superseded push is expensive here (~40% of PR CI runs measured +2026-07-30 were cancellations). A local run that passes bought nothing the CI run would not +have established. So the question is never "local or CI" in the abstract, it is: + +> **Is this gate, on this kind of change, still catching anything?** + +**The rule: run an expensive local gate only while it is still earning its runtime, and +never re-derive a verdict that already exists.** Before running `lint`, `typecheck`, +`test`, `verify:cheap`, or `verify:pr-local`, consult the arbiter and quote its verdict: + +```bash +npm run arbiter -- # RUN / DEFER / PROVEN, with its evidence +npm run arbiter:status # the yield ledger and the duplication bill so far +``` + +It weighs three inputs, none of them hard-coded, so the answer moves as the repo moves: + +1. **CI coverage**, derived live from `package.json` + `.github/workflows/ci.yml`, and + evaluated **for this change** — the step's own `if:` and its job's `if:` are checked + against the current change scope, because a step's presence in the YAML is not + coverage. `lint` and `typecheck` are step-conditional on `static_heavy_changed` and + `test:coverage` is job-conditional on `coverage_changed`, so a docs-only change is + covered by none of them. A gate CI does not re-run is never deferrable — local is the + only gate there is. Delete the CI job and the arbiter stops deferring to it the same day. +2. **Observed yield**, a rolling per-gate, per-change-class window of local outcomes that + the gate wrappers record automatically. A gate that has caught nothing across a full + clean window on this class of change has stopped earning its runtime. The **first catch + resets the window** and the gate runs locally again, so the loop re-arms itself instead + of decaying toward "never check anything". +3. **Content identity** — a verdict GitHub already reached on exactly this content + (recorded with `npm run arbiter -- record-ci ` when a session observes CI + go green) is not re-derived locally. This is the common repetition: CI goes green on a + branch head, and a later session runs the whole suite again on that same head. Name the + gates CI actually ran — the command refuses a bare invocation rather than turning one + observed job into proof for every gate. + +The window is per change class because the classes are not the same bet: docs-only clears +in 3 clean runs, source in 12, and **db, RAG, dependency, container, workflow, UI and +unrecognised scope never defer at all**, however clean the history — the same fail-closed +routing CI itself uses, not a second risk model. + +Non-negotiable boundaries, all of them the conservative direction: + +- **Fail open.** Missing data, unreadable CI, an unknown change class, a git failure — every + one of them runs the gate. A bug in the arbiter costs a redundant run, never a skipped one. +- **CI is never advised by it.** `CI` being set disables the arbiter outright. GitHub stays + the authoritative merge gate and nothing computed locally may influence what it runs. +- **Advisory by default.** A `DEFER` or `PROVEN` verdict is a recommendation printed with + its evidence; the wrappers act on it only under `GATE_ARBITER=enforce`. Silently skipping + a gate a human typed is exactly the failure the evidence rules exist to prevent. +- **A focused run is not full-suite evidence.** A narrowed Vitest invocation records under + its own identity, so a clean run of single-file tests can never let the whole suite defer. +- **A deferred gate is not a passed gate.** Report it as "deferred to CI — has caught + nothing in N consecutive runs", never as green, and never alongside a claim that + the gate ran. The same applies to `PROVEN`: say "reused receipt" or "CI-proven at ``". + +This does not license skipping verification. It licenses not buying the _same_ verdict +twice. The smallest-correct-gate rule above still decides which gate is right; the arbiter +only decides whether that gate has anything left to tell you before you push. + diff --git a/docs/process-hardening.md b/docs/process-hardening.md index c65a9f356a..1124dc6269 100644 --- a/docs/process-hardening.md +++ b/docs/process-hardening.md @@ -81,6 +81,78 @@ what it is scoped to verify, and `ci-change-scope.mjs` already keeps that scoped not waste either — it is what stops a red push costing a full CI cycle plus a fix round. The waste was only ever the _repeat_, and that is what is now gone. +## Gate arbitration: stop paying for a verdict GitHub is about to reach (2026-08-21) + +The receipts section above closes by saying the local run "is not waste either — it is what stops a +red push costing a full CI cycle". True, and incomplete. `check:gate-manifest` enforces that CI never +runs less of the local `verify:cheap` static set than the local chain does, which read the other way +says **every local run of a gate in that chain is work GitHub is about to repeat**. Receipts cannot +touch that duplication by design: `receiptsEnabled()` is false whenever `CI` is set. + +So the local run is a bet, not a certainty. It pays when it fails (a red push costs a CI cycle plus a +fix round, and ~40% of PR CI runs measured 2026-07-30 were cancellations); it pays nothing when it +passes. The bet's value is therefore not fixed — it decays as a gate stops catching things on a given +class of change, and it recovers the moment the gate catches something again. Nothing measured that, +so the decision was made from habit in both directions: running the full suite on a docs typo, and +skipping it on a change that deserved it. + +`scripts/gate-arbiter.mjs` measures it. Three inputs, none hard-coded: + +- **CI coverage**, parsed live from `package.json` + `.github/workflows/ci.yml` using the same + field-anchored `run:` regex as `check-gate-manifest.mjs` (the two must agree — a looser parse here + would defer to a job the manifest check knows does not exist). Resolved by the gate's own name, its + declared CI equivalent (`test` → `test:coverage`), or a CI-invoked aggregate whose package.json body + contains it — and then **evaluated against the current change scope**. A step's presence in the YAML + is not coverage: `lint` and `typecheck` carry a step-level `if: needs.changes.outputs.static_heavy_changed`, + and the `coverage` job is gated on `coverage_changed`, so a docs-only change is covered by none of + the three. A name-only scan reported all three covered, which under `GATE_ARBITER=enforce` produced + the one outcome the module exists to prevent — local gate deferred, CI gate skipped, no verdict + anywhere. Raised as P1 by Codex review on PR #2245 and pinned by `tests/gate-arbiter.test.ts`. + Conditions that are not change-scope flags (draft state, event name) cannot be evaluated from a + worktree; they are reported as assumed preconditions with the decision rather than silently taken as + true. **A gate CI does not re-run for this change is never deferrable.** +- **Observed yield**, a rolling window (40 observations) keyed by `(gate, change class)`, recorded by + `run-heavy.mjs` and `run-vitest.mjs` after every arbitrated run. Recording is pure observation and + never alters the run. An admission-busy exit (75) is not a verdict and is not recorded, so lock + contention can neither manufacture a clean window nor keep a healthy gate running forever. +- **Content identity**, via `record-ci `: a clean worktree plus an empty `git diff HEAD` + proves the content GitHub judged is the content in front of us. Both halves are required — a clean + tree alone does not prove HEAD has not moved, and a matching diff alone cannot see an uncommitted + edit. Reading GitHub is provider-backed, so the arbiter never reaches for it; the session that + already looked at CI passes what it saw. + +Change class comes from `scripts/ci-change-scope.mjs` — the classifier CI itself uses to route jobs — +rather than a second risk model, so the arbiter and CI cannot drift into two opinions about what a +path means. Clean-window sizes: `docs` 3, `source` 12. Every other class (`db`, `rag`, `deps`, +`container`, `workflow`, `ui`, `unknown`) is absent from the window map and never defers at any length. +`tests/gate-arbiter.test.ts` pins that absence, so adding a risky class to the deferrable set fails. + +Boundaries, each of them a test in `tests/gate-arbiter.test.ts`: + +- **Fail open.** Unreadable CI, unknown class, missing observations, git failure — all run the gate. +- **CI never consults it.** `arbiterMode()` returns disabled whenever `CI` is set. +- **Advisory by default.** The wrappers act on a deferral only under `GATE_ARBITER=enforce`; a gate a + human typed still runs. `GATE_ARBITER=off` disables it entirely. +- **The first catch re-arms the window**, so a gate that starts failing again is never left deferred + because it had a long clean run beforehand. +- **A narrowed Vitest run records under its own identity** (`vitest(selected)`), so a clean history of + focused runs can never satisfy the full suite's window. +- **`record-ci` requires an explicit gate list**, and rejects a SHA that does not resolve here, so one + observed green job cannot become stored proof for every arbitrated gate. +- **Observations are re-read immediately before the write**, so two gates finishing together cannot + drop a catch — the unsafe direction, since a lost catch leaves a failing gate deferred. +- **A deferred gate is not a passed gate.** The verdict prints that sentence; report it as "deferred + to CI", never as green. + +`npm run arbiter -- ` gives the verdict and its evidence; `npm run arbiter:status` shows the +yield ledger and the accumulated duplication bill; `npm run arbiter:clear` empties it. The ledger sits +beside the receipt store under `node_modules/.cache/`, so it is per-worktree, never committed, and +destroyed by `npm ci`. + +**What this does not do.** It does not reduce GitHub's work, weaken any required check, or change +which gate is the smallest correct one for a diff. It decides only whether that gate still has +anything left to tell you before you push. + ## Multi-worktree reconciliation hardening (2026-07-23) The cloud-chat reconciliation postmortem and complete issue/fix matrix are in diff --git a/docs/scripts-index.md b/docs/scripts-index.md index a10ad784b1..2a826f89ef 100644 --- a/docs/scripts-index.md +++ b/docs/scripts-index.md @@ -1,6 +1,6 @@ # Scripts index -Curated map of `scripts/` (254 files) and the `package.json` script surface (255 entries), +Curated map of `scripts/` (255 files) and the `package.json` script surface (258 entries), grouped by purpose. This is orientation, not an exhaustive per-file listing — the authoritative command list is `package.json`, and `npm run docs:check-scripts` verifies every `npm run ` referenced in docs resolves to a real script. `npm run docs:update` refreshes the exact counts above. @@ -19,6 +19,7 @@ migration has shipped (see `docs/maturity-backlog-workorders.md` L1). | Script | Role | | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `run-heavy.mjs` | Acquires shared/exclusive cross-worktree leases (`test-run-lock.mjs`) so focused checks can overlap safely | +| `gate-arbiter.mjs` | Decides whether an expensive local gate still earns its runtime, from live CI coverage plus a rolling per-gate/per-change-class yield window (advisory; never in CI) | | `gate-receipts.mjs` | Content-addressed memoisation of `lint`/`typecheck`/Vitest so an unchanged gate is never rerun locally (never in CI) | | `run-tsx.mjs`, `run-vitest.mjs`, `run-playwright.mjs`, `run-eval-safe.mjs` | Typed/test/e2e/eval entrypoint wrappers | | `dev-free-port.mjs`, `ensure-local-server.mjs` | Project-stable localhost port selection + background server ensure | diff --git a/package.json b/package.json index d87c2d65c8..8f69f7f271 100644 --- a/package.json +++ b/package.json @@ -255,6 +255,9 @@ "check:local-presence": "node scripts/check-local-presence.mjs", "receipts": "node scripts/gate-receipts.mjs status", "receipts:clear": "node scripts/gate-receipts.mjs clear", + "arbiter": "node scripts/gate-arbiter.mjs", + "arbiter:status": "node scripts/gate-arbiter.mjs status", + "arbiter:clear": "node scripts/gate-arbiter.mjs clear", "check:drift": "node scripts/run-tsx.mjs scripts/check-drift.ts", "check:migration-history": "node scripts/run-tsx.mjs scripts/check-migration-history-alignment.ts", "drift:manifest": "node scripts/run-tsx.mjs scripts/generate-drift-manifest.ts", diff --git a/scripts/gate-arbiter.mjs b/scripts/gate-arbiter.mjs new file mode 100644 index 0000000000..7d02c6efa4 --- /dev/null +++ b/scripts/gate-arbiter.mjs @@ -0,0 +1,827 @@ +#!/usr/bin/env node +/** + * gate-arbiter.mjs — decide whether an expensive local gate is still worth running. + * + * `gate-receipts.mjs` removed the local-versus-local duplication: the same gate + * twice on content that did not change between the two runs. It deliberately + * cannot touch the *other* duplication, the one that costs the most and that + * nothing measured until now: + * + * run the full suite locally -> push -> GitHub runs the same suite again + * + * `check:gate-manifest` proves that second run always happens. Its invariant is + * one-way — CI must never run LESS of the local static set than `verify:cheap` + * does — so for every gate in that chain, a local run before a push is by + * construction work GitHub is about to repeat. + * + * That does not make the local run waste. It is a bet: a local run that FAILS + * saves a CI round trip, and in this repository a red or superseded push is + * expensive (~40% of PR CI runs measured 2026-07-30 were cancellations). A local + * run that PASSES bought nothing that the CI run would not have established. + * + * So the question is never "local or CI" in the abstract. It is: + * + * is this gate, on this kind of change, still catching anything? + * + * The arbiter answers that from three inputs, none of them hard-coded, so the + * answer moves as the repository moves: + * + * 1. CI coverage, derived live from `package.json` + `.github/workflows/ci.yml`. + * Delete the CI job and the arbiter stops deferring to it the same day. A + * gate CI does not run is never deferrable: local is the only gate there is. + * 2. Observed yield, a rolling per-gate, per-change-class window of local run + * outcomes, recorded automatically by the gate wrappers. A gate that has + * caught nothing across a full clean window on this class of change has + * stopped earning its runtime. The FIRST catch resets the window and the + * gate runs locally again — the loop re-arms itself rather than decaying. + * 3. Content identity, so a verdict GitHub already reached on exactly this + * content is not re-derived locally at all. + * + * Deliberate boundaries, all of them the conservative direction: + * + * - **Fail open, always.** Missing data, unparseable CI, an unknown change class, + * a git failure — every one of them runs the gate. A bug here costs a redundant + * run, never a skipped one. That is the same contract `gate-receipts.mjs` holds. + * - **CI is never advised by this file.** `CI` being set disables the arbiter + * outright. GitHub remains the authoritative merge gate; nothing computed here + * may influence what it decides to run. + * - **High-risk change classes never defer**, however clean the window. Database, + * RAG, dependency, container, workflow and unrecognised scopes run locally. + * This mirrors CI's own fail-closed routing rather than inventing a second + * risk model. + * - **Advisory by default.** A deferral is a recommendation printed with its + * evidence. The wrappers honour it only under `GATE_ARBITER=enforce`, because + * silently skipping a gate a human typed is precisely the failure this + * repository's evidence rules exist to prevent. + * - **A deferred gate is not a passed gate.** The verdict says so in as many + * words, and must be reported as "deferred to CI", never as green. + * + * CLI: + * node scripts/gate-arbiter.mjs decision + evidence for one gate + * node scripts/gate-arbiter.mjs status the yield ledger, per gate/class + * node scripts/gate-arbiter.mjs record-ci [jobs...] + * node scripts/gate-arbiter.mjs clear + */ +import { execFileSync } from "node:child_process"; +import { mkdirSync, readFileSync, renameSync, writeFileSync, rmSync, existsSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +import { consultGateReceipt, describeAge } from "./gate-receipts.mjs"; + +export const LEDGER_FORMAT_VERSION = 1; + +export const LEDGER_RELATIVE_PATH = path.join("node_modules", ".cache", "database-gate-yield.json"); + +/** Observations retained per (gate, class). Long enough to see a trend, short enough to forget stale ones. */ +export const MAX_OBSERVATIONS = 40; + +/** + * The gates worth arbitrating: the ones whose runtime is measured in minutes and + * which CI repeats wholesale. Anything absent is simply run — the arbiter holds no + * opinion about a check that costs a second. + */ +export const ARBITRATED_GATES = new Set([ + "lint", + "lint:internal", + "typecheck", + "typecheck:internal", + "typecheck:source:internal", + "test", + "vitest", + "vitest(selected)", + "verify:cheap", + "verify:pr-local", +]); + +/** + * The yield identity for a Vitest invocation. + * + * A focused run and the full suite are not the same evidence. Recording both as + * plain `vitest` let twelve passing single-file runs build a clean window that would + * then permit `npm test` to skip the entire suite — a history produced entirely by a + * selection that never executed most of it. Reported as P2 by Codex on PR #2245. + * + * Only the canonical whole-suite invocation keeps the `vitest` identity; anything + * that narrows what executes gets `vitest(selected)`, so the two histories never mix. + * Output-only flags (reporters) do not narrow execution and keep the plain identity. + * + * @param {string[]} argumentList + * @returns {"vitest" | "vitest(selected)"} + */ +export function vitestGateIdentity(argumentList) { + const narrowing = (argumentList ?? []).some((argument, index) => { + if (argument === "run") return false; + if (argument.startsWith("-")) { + return /^--?(project|t|testNamePattern|grep|shard|dir|related)(?:[.=]|$)/.test(argument); + } + // A positional that is not the value of a preceding valueless long option is a + // file or directory filter. + const previous = index > 0 ? argumentList[index - 1] : ""; + return !(previous.startsWith("--") && !previous.includes("=")); + }); + return narrowing ? "vitest(selected)" : "vitest"; +} + +/** + * Clean observations required before a gate may be deferred, per change class. + * + * These are not one number because the classes are not one bet. A docs-only change + * that has not broken the Vitest suite three times running is very unlikely to break + * it the fourth; a source change that has not broken it three times running is a + * small sample of a genuinely risky population, so it must clear a much longer run + * before the arbiter will spend a CI round trip on it. + * + * A class absent from this map is not deferrable at any window length. That is the + * fail-closed half of the policy and is why `unknown` is not listed. + */ +export const CLEAN_WINDOW_BY_CLASS = new Map([ + ["docs", 3], + ["source", 12], +]); + +/** + * Change classes that never defer regardless of observed yield. + * + * Deliberately the same shape as CI's own fail-closed routing in + * `scripts/ci-change-scope.mjs`: unknown scope is heavy scope. Adding a class here + * is safe; removing one is a policy change that needs its own review. + */ +export const NEVER_DEFER_CLASSES = new Set(["db", "rag", "deps", "container", "workflow", "ui", "unknown"]); + +function projectRootFromHere() { + return path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +} + +/** + * @param {Record} [env] + * @returns {{ enabled: boolean, enforce: boolean, reason: string }} + */ +export function arbiterMode(env = process.env) { + // CI is the authoritative merge gate; it must never consult a local yield ledger. + if (env.CI) return { enabled: false, enforce: false, reason: "CI is set — the arbiter never advises CI" }; + const setting = String(env.GATE_ARBITER ?? "").toLowerCase(); + if (setting === "off" || setting === "0" || setting === "false") { + return { enabled: false, enforce: false, reason: "GATE_ARBITER is off" }; + } + if (setting === "enforce") return { enabled: true, enforce: true, reason: "GATE_ARBITER=enforce" }; + return { enabled: true, enforce: false, reason: "advisory (set GATE_ARBITER=enforce to act on deferrals)" }; +} + +function runGit(projectRoot, args) { + return execFileSync("git", args, { + cwd: projectRoot, + encoding: "utf8", + maxBuffer: 64 * 1024 * 1024, + stdio: ["ignore", "pipe", "pipe"], + }); +} + +/* ------------------------------------------------------------------ * + * 1. CI coverage — derived, never assumed * + * ------------------------------------------------------------------ */ + +/** + * The `npm run