Skip to content

feat(ci): flake gate writes one json line per check - #1087

Merged
andrewgazelka merged 1 commit into
mainfrom
perf/flake-gate-runs-checks-concurrently
Jul 30, 2026
Merged

andrewgazelka merged 1 commit into
mainfrom
perf/flake-gate-runs-checks-concurrently

Conversation

@andrewgazelka

@andrewgazelka andrewgazelka commented Jul 30, 2026

Copy link
Copy Markdown
Member

Follow-up to #1084, which merged before its own Flake job finished (this
repository has no required status checks, ENG-10827, so a cancelled gate does
not block auto-merge).

What this adds

flake-gate-results.jsonl in the workspace, uploaded as a CI artifact, one
line per check:

{"attr":"smash-e2e","outcome":"fail","drvPath":"/nix/store/...-hyperion-smash-e2e.drv","seconds":null}

Written from the same arms of the reporting loops that print ok / FAILED /
STALE / excluded, so the machine-readable verdict and the human one cannot
disagree. attr is the flake attribute, not the derivation name, so nothing
downstream has to map hyperion-smash-e2e back to smash-e2e.

An excluded check is recorded by what it did, not by how the gate treats
it: a still-failing exclusion records fail even though it does not fail the
job. Whether this file forgives a check is this file's business and changes
under a consumer.

drvPath, and the gate's only flake load

drvPath comes from one nix eval --raw --apply over the whole checks set.
That is now the gate's only flake evaluation: the per-check realisation probe
takes the derivation path (nix build --max-jobs 0 --builders "" "$drv^*")
instead of the attribute, so 67 probes load no flake at all.

A derivation path is defined at evaluation time even for the 14 checks whose
output path is deferred behind a content-addressed input, which is why it is
the identity the gate can state for every check. Verified that
--max-jobs 0 on a deferred-output derivation answers 1 before it is built and
0 after, without building it.

The realisation itself still goes through the flake attribute rather than the
derivation path, deliberately: 811 paths in run 30500640846 came from
cache.ix.dev, which the flake's nixConfig supplies and a store-path build
would not see.

seconds is null, on purpose

One concurrent nix build means no check has a wall clock to itself.
smash-e2e and bedwars-bow-e2e overlap each other and both overlap the
compile they depend on, so any per-check number would be a share of a clock the
check did not have. Emitting a plausible-looking one would be worse than
emitting none.

Recovering real per-check durations is possible but is a different change:
split the realisation into a dependency phase (one nix build over the union
of the checks' inputDrvs) and a leaf phase (each check's own derivation,
bounded concurrency, individually timed). That gets exact durations and exact
exit codes, at the cost of a bash job pool and a new failure mode where a
broken shared dependency is retried by each dependent leaf. Say the word and I
will do it.

The gate now prints its own total instead:
gate: 67 enforced and 0 excluded checks in <n>s.

Guard

Names are checked at evaluation time to need no JSON escaping, because the
file is assembled with printf rather than jq. Watched it fail:

error: nix/ci/flake-gate.nix writes each check's verdict into a json results file
       with printf, which holds only for names that need no json escaping. These
       do: he said "hi"
       Rename them, or teach the gate to escape a name before it writes one.

Verified

Ran the gate over six real checks on aarch64-darwin (names trimmed to keep it
cheap), green and broken:

  • Six passing: six pass lines, schema and values validated by a JSON parser,
    exit 0.
  • One deliberately broken (e2e-ports-distinct, via two gates sharing an
    e2eOffsets value), first in the list: it records
    {"attr":"e2e-ports-distinct","outcome":"fail",...} and the five after it
    still record pass. Note the failing run's drvPath differs from the passing
    run's, which is exactly the discriminator the consumer wants: same derivation
    and different outcome means flake, different derivation means the change.

Neither this nor #1084 was checked by CI before landing

Recorded rather than apologised for, so that main's history is readable later.

bc10ddf (#1084) merged at 03:11 on 2026-07-30 with its own Flake job
cancelled at 377 s. This repository has no required status contexts
(ENG-10827), so nothing stopped it. 256a7b1 (#1088) then turned ci, bench
and conventional into workflow_dispatch only, so nothing will trigger on
main again either, and this PR will land the same way.

Both changes are verified, but locally and by hand rather than by a pipeline:
the gate run over six real checks on aarch64-darwin, green and deliberately
broken, plus the drv-path oracle checked against a synthetic deferred-output
derivation, plus the name guard watched failing. What has not happened is
either change running against all 67 checks on a hosted runner in the shape CI
would run it. A manual gh workflow run ci.yml --repo hyperion-mc/hyperion --ref main is how that gets done from here.

This hour is the boundary. Commits before bc10ddf went in under a running
pipeline whatever its state; commits from bc10ddf on did not.

@andrewgazelka
andrewgazelka enabled auto-merge July 30, 2026 03:13
@github-actions github-actions Bot added the feat label Jul 30, 2026
A consumer differencing this run's failing checks against another run's
had only the job log to read them out of. The gate knows each attribute
name directly, so it says so: `flake-gate-results.jsonl`, one line of
`{"attr","outcome","drvPath","seconds"}` per check, written from the same
arms that print the human report so the two cannot disagree.

`drvPath` comes from one `nix eval --apply` over the whole `checks` set,
which is also now the gate's only flake load: the per-check realisation
probe takes the derivation path instead of the attribute, so it evaluates
nothing. A derivation path is defined at eval time even for the 14 checks
whose output path is deferred behind a content-addressed input, and it is
what lets a consumer tell a flaky check from a broken one. The same
derivation with two outcomes did not change; a different derivation did.

`seconds` is null, and that is the change above rather than an oversight.
One concurrent `nix build` means no check has a wall clock to itself, so
any per-check number would be a share of one. Named in the file, with
what it would take to recover a real one.

Names are checked at evaluation time to need no json escaping, since the
file is assembled with printf. Watched that guard fail on `he said "hi"`.
@andrewgazelka
andrewgazelka force-pushed the perf/flake-gate-runs-checks-concurrently branch from f12b8df to d8cf9a6 Compare July 30, 2026 03:18
@andrewgazelka
andrewgazelka merged commit 4945434 into main Jul 30, 2026
@andrewgazelka
andrewgazelka deleted the perf/flake-gate-runs-checks-concurrently branch July 30, 2026 03:19
andrewgazelka added a commit that referenced this pull request Jul 30, 2026
…#1090)

## The problem, with the evidence

On 2026-07-29 two correct pull requests each displayed exactly the
failures the other one fixes.

- **#1080** repairs `completions-e2e` and `minecraft-literals`. Its CI
reported `bedwars-dev-boot-e2e`, `smash-dev-boot-e2e`, `smash-e2e`.
- **#1081** repairs the two dev-boot gates. Its CI reported
`completions-e2e`, `minecraft-literals`, `smash-e2e`.

Neither could ever be green on its own, so neither could land, so the
failures accumulated. Both were correct. Both looked broken. A gate that
can only say "green" cannot say "better than before", and when main is
already red that is the only question a reviewer has.

## What this does

The gate compares this run's failing set against a baseline that main
publishes on every push. A pull request that leaves the set no larger
passes. One that adds a name blocks, and the summary names it.

Replayed against the real 1080/1081 sets, using this implementation:

| scenario | verdict | detail |
| --- | --- | --- |
| PR 1080 | **pass** | `fixed = [completions-e2e, minecraft-literals]`,
three excused |
| PR 1081 | **pass** | `fixed = [bedwars-dev-boot-e2e,
smash-dev-boot-e2e]`, three excused |
| breaks a check (derivation moved) | **block** | `blocked = [proxy]` |
| same derivation, unchanged check set | pass | reported
nondeterministic, not blamed |
| same derivation, but the check set changed | **block** | `blocked =
[proxy]` |

## Flakiness is answered with a proof, not a statistic

A retry cannot do this job. For a check failing 30% of the time, two
failures in a row happen 9% of the time, so a retry can acquit and never
convict. Any gate whose flake answer is a retry blocks honest work 9% of
the time.

A derivation path is a Merkle root over every input derivation, so an
unchanged path means an unchanged build closure. Therefore:

> **Same derivation, different outcome, is a proof of nondeterminism.**

Not evidence, not a heuristic. If a check passed on the base and fails
here on the byte-identical derivation, the diff did not cause it.
Measured here:

```
run 30428441399 (34eb46c, 06:31Z)  differential-traces  ok
run 30429463435 (dd8311f, 06:49Z)  differential-traces  FAILED
both on /nix/store/fczpzka3v7c9npf4a3i730ahkncla3f5-check-hyperion-differential-traces.drv

java.lang.IllegalStateException: chunks were still not entity ticking after 600 ticks
        at VanillaTrace.tickServer(VanillaTrace.java:514)
```

A wall-clock timeout inside the sandbox. Nothing in the derivation
moved; the runner was slower.

Applied at two scopes: within a verdict when the derivation is
unchanged, and across a rolling record for the common case where a pull
request moves the hash. The record keys on the derivation for the
**proof** and promotes onto the attribute for the **conclusion**,
because nondeterminism is a property of how a check is written and
survives a change to its inputs.

## What the 18 main runs of 2026-07-28/29 actually say

Every one of the 18 was red. Extracted from the `Flake` job logs; `.` =
ok, `X` = FAILED, `-` = check did not exist yet, oldest leftmost.

```
smash-e2e                XXXXXXXXXXXXXXXXXX   18/18   deterministic
completions-e2e          XXXXXXXXXXXXXXXXXX   18/18   deterministic
smash-dev-boot-e2e       -----------XXXXXXX     7/7   deterministic
bedwars-dev-boot-e2e     -----------XXXXXXX     7/7   deterministic
minecraft-literals       ......XXXXXXXXXXXX   12/18   REGRESSION, see below
smash-hud-e2e            X....X.XX.X..X..XX    8/18   44% nondeterministic
differential-traces      .....XXXXX..X....X    7/18   39% nondeterministic
bedwars-bow-e2e          ........XX........    2/18   11% nondeterministic
smash-selector-e2e       ..................    0/18   clean
```

**61% of runs hit at least one flake**, 2.57 gate runs expected per
clean pass. That is measured, not assumed, and the implementation
reproduces it: `dg flake-rate` on the folded real history returns
`0.611` over the same 18 runs.

Three things here were not previously known. `differential-traces` and
`bedwars-bow-e2e` are flaky and nobody had noticed. `smash-selector-e2e`
is no longer flaky. And **`minecraft-literals` is not one of the
standing five, it is a regression that landed mid-window and stuck**
while the gate was already red, which is consequence 2 of a permanently
red gate caught in the act.

### Why the identity has to be the derivation, demonstrated on that data

Folding the real 18-run history two ways:

```
A. every check given a constant derivation
   proven nondeterministic: bedwars-bow-e2e, differential-traces, minecraft-literals, smash-hud-e2e

B. modelling the one real derivation change verified from the logs
   (minecraft-literals 9md01i6l -> 3rwh12gk at run 30418671535)
   proven nondeterministic: bedwars-bow-e2e, differential-traces, smash-hud-e2e
```

A excuses a genuine regression as a coin flip. B does not. The
derivation keying is the whole difference, on real data.

## Guards, and each was watched failing

Immunity is withdrawn when the derivation does not describe the run. The
general form, because the list will grow: **a pull request forfeits
immunity when it changes anything the derivation does not capture but
the run depends on.** Three known members: environment
(`.github/workflows/**`, `nixConfig`), scheduling and concurrency
(`nix/ci/flake-gate.nix`, e2e port offsets), and a **changed check set**
(detected from the two documents rather than a path list, because adding
a check adds contention under a concurrent gate and no path list would
catch every way of doing that).

`nix/ci/delta-gate-tests.sh` is 65 cases with no network, no nix and no
clock, wired as `checks.<system>.delta-gate`. It contains the inverse of
every guard, because the failure mode of a suite like this is passing
for the wrong reason. Six deliberate mutations of the verdict logic were
each caught by a named case:

```
immunity ignores forfeiture          CAUGHT  forfeit: same-drv flip now BLOCKS
every drv counts as identical        CAUGHT  row2 new-failure: gate BLOCKS
cap ignores the shrink exemption     CAUGHT  cap: a PR that shrinks the set still lands
deleted check counts as repaired     CAUGHT  removed: not credited as a repair
eval failure no longer blocks        CAUGHT  eval failure: BLOCKS even though the base fails too
instability keyed on attr not drv    CAUGHT  fold: the check is proven unstable
```

## What keeps the failing set from becoming permanent

- **The ratchet**, automatic and needing nobody: a pull request takes
the newest baseline, so the instant a check passes on main it can never
be excused again. Publishing on green is the revocation mechanism, not
an optimisation.
- **A cap of 6** in `nix/ci/delta-gate.sh`. Past it nothing lands except
pull requests that shrink the set. It is a committed constant, editable
downward only and deliberately never derived from an observed count: a
suite with proven coin flips will eventually have a lucky night, and a
self-lowering cap would latch onto it and freeze the repository the next
ordinary day.
- **A standing issue**, one rewritten in place and closed automatically
on green. In this repository rather than Linear, because the repository
is public and an outside contributor meeting a red gate needs to read
why without an internal account. **Dry-run by default**;
`GATE_ISSUE_APPLY: "1"` is one line in the workflow.

## The exclusion list is deleted

Same idea at lower fidelity, stored where only human attention could
refresh it, and every fix was a merge conflict against every other fix
in flight. The header of `nix/ci/flake-gate.nix` now says where "known
broken" lives so nobody reinvents it. A check that genuinely cannot run
in CI belongs out of `checks` in `flake.nix`, which is a truer home for
"this cannot run here" than a CI-side exception.

## This is NOT a required status context, and this PR does not make it
one

`gh api repos/hyperion-mc/hyperion/rulesets/566717 --jq '.rules[].type'`
returns seven rules and no `required_status_checks`; it is the only
ruleset and `branches/main/protection` returns 404. So one approving
review is the only thing between any pull request and main today, red
pipeline or no pipeline. Making the verdict binding is one
`required_status_checks` entry naming this job. **Four criteria should
hold first, none of which does:**

1. **Flake rate at or under 5%** over 20 consecutive runs. Measured 61%.
2. **At least 30 runs in the instability record.** At n=18 an 11% flake
like `bedwars-bow-e2e` is still 12% likely to be unproven; n=30 puts
anything at or above 10% over 97% likely to be proven.
3. **At most 1 in 20 pull request runs** blocked by a verdict a re-run
then clears, over 20 runs. Only measurable in this report-only phase,
which is why it is its own step.
4. **p95 wall time under 40 minutes.** Measured 31 to 41 over the 18
runs, all red; no green run of this gate has ever been observed, so the
green cost is unknown. The merge queue's
`check_response_timeout_minutes` is 60 while this job is allowed 90, so
those two must stop disagreeing before anything is required.

Criterion 4 makes #1084 (concurrent gate) a prerequisite. The gate is
two long checks and a tail with a 4-second median, so the concurrency
floor is about 11 minutes.

Also worth naming: the queue's `grouping_strategy` is `ALLGREEN` with
`max_entries_to_build: 5`, so once checks are required one flake ejects
a batch of five rather than costing one author a re-run.

## Composing with #1087, which landed while this was being written

#1087 gave the gate a `results.jsonl` of `{attr, outcome, drvPath}` per
check, and wrote the same rationale I had arrived at separately: "the
same derivation with two different outcomes is proof the change did not
cause the failure." So this rebases onto their file rather than
replacing it. Their concurrent build, their `drv_map`, their
`realised()` store oracle and their `unquotable` guard are all
untouched. What this adds is `excluded` deleted, and a verdict stage at
the end that reads `results.jsonl` and nothing else.

That seam is load-bearing and now demonstrated: the build loop changed
from one `nix build` per name to one concurrent `--keep-going` build for
the whole set, and no part of the verdict noticed.

I adopted their vocabulary (`pass`/`fail`, not `ok`/`fail`), and the
library tests for `fail` and treats anything else as passing, so a third
spelling arriving later reads as a pass rather than silently turning
every check red.

## This does NOT restore the CI triggers

#1088 and #1089 removed every automatic trigger while this was in
flight. Three of the reasons given are measurements from this same
investigation: 18 consecutive red runs, no required status context, and
three checks poisoning 61% of runs so a correct change needed 2.6
attempts.

**This change answers two of those three and restores none of the
triggers.** A differential verdict makes a red base survivable and stops
the coin flips being blamed on anybody, but it does not make the checks
green, and green-and-stays-green is the bar #1089 set. Restoring
triggers is a decision for whoever set that bar, not a side effect of
landing machinery that changes what the bar is worth.

The consequence, stated plainly: **on `workflow_dispatch` alone this
machinery is mostly inert.** No push means no published baseline, so the
first pull request to run under it will find none and be judged exactly
as it is today. A manual dispatch on `main` does produce a baseline and
does fold the first instability samples, and the publish conditions are
written as `github.ref == 'refs/heads/main' && github.event_name !=
'pull_request'` so they keep working unchanged whenever the triggers
come back.

## Verification

- `nix build .#checks.aarch64-darwin.delta-gate` passes in the sandbox:
65 of 65.
- `nix build .#checks.aarch64-darwin.flake-gate` builds, so shellcheck
passes on the gate.
- `nix run .#fmt` makes no changes.
- Workflow YAML parses; job and permission structure checked with `yq`.
- Replayed against the real 1080/1081 sets and against the real 18-run
history; `dg flake-rate` on the folded history returns `0.611`, matching
the hand measurement.

**Not verified, and worth knowing before approving:**

- No full `nix run .#flake-gate` on x86_64-linux. The verdict stage is
exercised only through fixtures and replays.
- **No live CI run of any of this.** With triggers manual, this PR does
not run its own CI, so the baseline fetch, the artifact publishing, the
standing issue job and the permission set have never executed. The first
`workflow_dispatch` on `main` after this merges is where they are first
exercised. That is the largest untested surface here and I am not going
to describe it as working.
- The standing issue writes nothing until `GATE_ISSUE_APPLY` is `"1"`.
Its body has been rendered locally against the real folded record; it
has never posted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant