ci: fix three of the checks the Flake job was failing - #1080
Merged
Conversation
The `Flake` job failed 105 of its last 120 runs on main, continuously red
since `e4bbcc2` at 2026-07-28T10:23Z.
The number matters less than the shape. The set of checks it reported
changed run to run on near-identical code -- `smash-hotbar-e2e`,
`smash-selector-e2e`, `minecraft-proto-generated`, `bedwars-bow-e2e`,
`differential-traces` and `minecraft-literals` appearing and disappearing
between adjacent commits. So nobody could use this job to tell a real
regression from noise since the day it was added, and every open pull
request inherited a `Flake: FAILURE` that said nothing about it. That is
the argument for fixing several causes at once rather than any one of
them.
## minecraft-literals -- reuse the const that was already there
`9c57be7` added two raw `minecraft:overworld` literals to
`net/protocol/join.rs`. That file already had `const LEVEL` holding
exactly that string, already recorded in the baseline, and the
`world_clock` lookup is for the level this server serves.
before: 326 raw registry names in Rust string literals, 324 allowed
after: 324 raw registry names in Rust string literals, 324 allowed
Verified green.
## completions-e2e -- the security fix was failing its own test
`48acbc9` (ENG-10871) stopped offering `/perms set` to `Normal` players,
which is the point of it. The gate still asserted the entry was there, so
the fix failed the test written to protect the behaviour it deliberately
changed. The expectation now matches the policy.
Verified green: `RESULT: every completion claim held`.
## differential-traces -- it was reporting a mismatch it never measured
It reported `arrow-crosswind-shot is not reproducible: seeds 4242 and
8675309 disagree` with a physics diff, while the trace had `wrote 0
samples`. The recorder's own `chunks were still not entity ticking after
600 ticks` guard had fired, Minecraft caught it into a crash report and
shut down cleanly, and `onServerExit` wrote the empty trace and exited 0 --
so a comparison of two empty traces produced a confident physics claim.
It now refuses a short trace and exits 1 with what it actually saw, and
`WARMUP_LIMIT` goes 600 -> 2400 so the warmup has room on a loaded runner.
## nix/e2e.nix -- comment only, no behaviour change
The comment claimed the TERM trap enforces the per-gate `timeout`. It does
not: the driver blocks in a foreground pipeline and bash defers the trap
until that returns, so `smash-e2e` declares `timeout = 480` and ran 633s
with the client still logging at 631.23s. The comment now states the
measurement and points at ENG-11370 rather than telling the next reader
the case is handled.
## Not touched
`smash-e2e` is a real product bug -- 23 abilities send no
`level_particles`, and `Cow / Mooshroom Madness` and `Guardian / Target
Laser` declare `buffs_melee` without applying it. Either those abilities
are unfinished or the gate's "every ability draws something" claim is too
strong, and that is a game-design decision.
`smash-hud-e2e` needs the server's own CPU% to cross an integer boundary 3
times in 20s; CI's runner sat at `CPU 4% of 400%` and moved once all run,
so the gate measures the runner's idleness. It needs a reading that does
not depend on host load.
The two dev-boot gates stay red until index's dev-profile fortify fix
lands (indexable-inc/index#4428) and this repo's index input is bumped to
it. No workaround is carried here: `bedwars-dev-boot-e2e` reaches play
state against that branch with no change on this side.
Refs ENG-11364, ENG-11370
Benchmark Results for generalComparing to dd8311f |
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #1080 +/- ##
=======================================
Coverage 54.65% 54.65%
=======================================
Files 361 361
Lines 33257 33257
Branches 1259 1259
=======================================
Hits 18178 18178
Misses 14793 14793
Partials 286 286
🚀 New features to boost your workflow:
|
andrewgazelka
enabled auto-merge
July 30, 2026 01:50
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.
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
1 participant
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.
Three of the
Flakejob's failures, fixed. Plus one comment that was telling readers a broken safeguard worked.Why several at once
The
Flakejob failed 105 of its last 120 runs on main, continuously red sincee4bbcc2at 2026-07-28T10:23Z.The number matters less than the shape. The set of checks it reported changed run to run on near-identical code —
smash-hotbar-e2e,smash-selector-e2e,minecraft-proto-generated,bedwars-bow-e2e,differential-tracesandminecraft-literalsappearing and disappearing between adjacent commits. So nobody could use this job to tell a real regression from noise since the day it was added, and every open pull request inherited aFlake: FAILUREthat said nothing about it. That is the argument for fixing several causes at once rather than any one of them.minecraft-literals— reuse the const that was already there9c57be7added two rawminecraft:overworldliterals tonet/protocol/join.rs. That file already hadconst LEVELholding exactly that string, already in the baseline, and theworld_clocklookup is for the level this server serves.Verified green.
completions-e2e— the security fix was failing its own test48acbc9(ENG-10871) stopped offering/perms settoNormalplayers, which is the point of it. The gate still asserted the entry was there, so the fix failed the test written to protect the behaviour it deliberately changed. The expectation now matches the policy.Verified green:
RESULT: every completion claim held.differential-traces— it was reporting a mismatch it never measuredIt reported
arrow-crosswind-shot is not reproducible: seeds 4242 and 8675309 disagreewith a physics diff, while the trace hadwrote 0 samples. The recorder's ownchunks were still not entity ticking after 600 ticksguard had fired, Minecraft caught it into a crash report and shut down cleanly, andonServerExitwrote the empty trace and exited 0 — so a comparison of two empty traces produced a confident physics claim.It now refuses a short trace and exits 1 with what it actually saw, and
WARMUP_LIMITgoes 600 → 2400 so warmup has room on a loaded runner.nix/e2e.nix— comment only, no behaviour changeThe comment claimed the TERM trap enforces the per-gate
timeout. It does not: the driver blocks in a foreground pipeline and bash defers the trap until that returns.smash-e2edeclarestimeout = 480and ran 633 s, with the client still logging at 631.23 s. The comment now states the measurement and points at ENG-11370 instead of telling the next reader the case is handled.Deliberately not touched
smash-e2eis a real product bug, reproduced locally, not a flake: 23 abilities send nolevel_particles, andCow / Mooshroom MadnessandGuardian / Target Laserdeclarebuffs_meleewithout applying it. Either those abilities are unfinished or the gate's "every ability draws something" claim is too strong. That is a game-design decision.smash-hud-e2eneeds the server's own CPU% to cross an integer boundary 3 times in 20 s; CI's runner sat atCPU 4% of 400%and moved once all run, so the gate is measuring the runner's idleness. It needs a reading that does not depend on host load.The two dev-boot gates (
bedwars-dev-boot-e2e,smash-dev-boot-e2e) stay red until indexable-inc/index#4428 reaches here via an index input bump. One cause: nixpkgs enables_FORTIFY_SOURCEby default, glibc refuses to fortify below-Oand emits a#warning, and jemalloc'sstrerror_rprobes run under-Werror, so configure dies withcannot determine return type of strerror_r. Fixed one layer down so it covers every C dependency rather than that one. No workaround is carried here:bedwars-dev-boot-e2ereaches play state, 4096 chunks,rc=0against that branch with no change on this side.Refs ENG-11364, ENG-11370
(sent by an AI agent via Claude Code, model claude-opus-4-6-20260212)