diff --git a/.agents/issue-index.md b/.agents/issue-index.md index 53594f411..49b56f2ba 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -652,3 +652,5 @@ rather than merged. `scripts/check-agent-record.py` gates both. | [#1770](https://github.com/mudler/vllm.cpp/issues/1770) | `VT-CONV1D-TIME-BLOCK` | **The two b0 op-level losses the `out_channels * kernel <= in_len` condition was derived from do not reproduce on a second boot, and the condition measures NEUTRAL on the window.** Found while closing [#1683](https://github.com/mudler/vllm.cpp/issues/1683) in `rc` job `16b594ec-7987-4cae-b377-414adbe0f944` on `thor:gpu0`, worker `rc-worker-kk96r`, boot id `e2112cac-660b-434e-911d-33cbd29b9176`, with arms A, C and D built from `origin/main` at `8eecc05a9` inside one lease and differing only in the row's own files. `.agents/specs/vt-conv1d-time-block.md` §2b prices the UNCONDITIONAL arm C against the baseline at 86 latents and reads `b0_res_conv1` 0.82x and `b0_res_conv2` 0.89x; §3b's condition exists to decline exactly those two shapes and §9 records unconditional blocking as REJECTED on that evidence. The second job, same instrument, same length, three alternated rounds, medians: `b0_res_conv1` C 0.04073 s against D 0.03824 s = **1.065x**, keeping the direction at a quarter of the size, and `b0_res_conv2` C 0.00897 s against D 0.01134 s = **0.791x**, REVERSED -- blocking that shape is 21 % faster and the rule declines it. Over the two shapes together C reads 0.04970 s against D's 0.04958 s, a tie. The window agrees the condition is neutral: at 86 latents the rule decides differently on exactly four shapes, `vocoder.conv1d` makes 54 calls per window so they run 2, 2, 6 and 6 times, and the per-call deltas bound the condition's whole window effect at **0.36 ms, 0.01 % of a 3.5 s window**; the paired 7-round pairing reads C 3.3899 s against D 3.5072 s (0.97x), which at 3 % therefore CANNOT be the condition, arm D's legs being bimodal at 3.394-3.429 and 3.507-3.530 s against arm C's 3.368-3.468 s; at 20 latents the arms are within 1.2 % at every thread count from 1 to 14. Nothing is broken: the shipped arm is correct, bit-identical at two fingerprints across three arms and five thread counts, and scales 11.54x of 14. What is unsupported is that the condition BUYS anything on this box, so either the b0 readings are boot-sensitive at a magnitude the row did not budget for, or one of the two jobs measured something other than the geometry it named. NOT FIXED IN FLOW: settling it needs a fresh lease and a per-geometry spread rather than a median of three rounds, at both 20 and 86 latents, plus the probe's own `--control` residency sweep at the b0 footprint. Owned by [`vt-conv1d-time-block.md`](specs/vt-conv1d-time-block.md) `## Owed` | verification | | [#1715](https://github.com/mudler/vllm.cpp/issues/1715) | `BACKEND-TENSTORRENT-GDN` | **The Qwen3.5/3.8 GDN-hybrid family cannot run on Tenstorrent: the GDN linear-attention op chain (`kGdnPrefill`, `kGdnDecode`, `kL2Norm`, `kRmsNormGated`, `kCausalConv1dFwd`/`kCausalConv1dUpdate`, `kGdnStateGather`/`kGdnStateScatter`) has no TT kernel, and the P150 is discrete so an op miss refuses by name rather than falling back** (`src/vt/op_provider.cpp` `Resolve`; `src/vllm/platforms/tenstorrent.cpp:55` allow-lists only OPT/Qwen3-dense/Mistral). Planned at the 2026-08-22 Qwen3.8 gap pass as the family's hard prerequisite on TT. Substrate anchor: the pinned tt-metal ships `ttnn::transformer::chunk_gated_delta_rule` (FLA chunked GDN forward, on-core state), mapping onto `kGdnPrefill` behind a varlen+state-permute adapter; decode composes the rank-1 update from matmul+eltwise with a device shadow keyed by host pointer (the `PagedKvShadow` pattern). Owned by this issue; spec [tenstorrent-gdn.md](specs/tenstorrent-gdn.md) | feature | | [#1765](https://github.com/mudler/vllm.cpp/issues/1765) | `GATE-METAL-MLX-COMPILE` | **[#1692](https://github.com/mudler/vllm.cpp/issues/1692) says `src/vt/metal/metal_mlx_provider.mm` is "compiled by NO job in this repository". MEASURED, and that is wrong in a way that changes what is owed:** `.github/workflows/release.yml:347` `mlx_arm64` builds it on `macos-15` against the real `mlx==0.32.0` wheel. The defect is the TRIGGER -- `release.yml` fires on a `v*` tag or a manual dispatch, never on a pull request and never on a push to `main`, and `ci.yml` has no Apple runner (all 17 `runs-on` lines are ubuntu-latest x14, ubuntu-24.04-arm x1, windows-2022 x2; its single `macos` string is `test_release_macos_metadata.py` at `:199`, which compiles nothing). So the only build of the file happens AFTER a change lands, and a break presents as a BLOCKED RELEASE rather than a red check; #1584's edit to `MlxFallback` is the case that proved it. On a configured CPU tree the file has **0** occurrences in `build.ninja` and **0** in `compile_commands.json` -- as does every other `.mm` -- against 3 for `src/vt/op_provider.cpp` as a control. **Closed by a never-linked OBJECT library** (`vllm_metal_mlx_provider_syntax_check`, the `vllm_rocm_platform_syntax_check` idiom) that compiles the file on Linux: it is the ONLY Metal TU that can be, carrying 0 Objective-C constructs against 10/5/19 for `metal_ops.mm`/`metal_backend.mm`/`metal_context.mm`, because `metal_context.h:22` hands it every Metal handle as a `void*` on purpose. `LANGUAGE CXX` for `-x c++`; `-Wno-deprecated` for `#import`, MEASURED narrow (a `[[deprecated]]` call still fails as `-Werror=deprecated-declarations`); guarded `NOT VLLM_CPP_MLX` (the LANGUAGE property is directory-scoped and must not reach the shipping build) and `NOT MSVC` (`#import` there means a type library). **The limit is stated rather than glossed:** the stubs under `src/vt/metal/stubs` are written from this file's call sites, so the gate proves it compiles against the REAL `vt::` seam -- the #1584 defect class exactly -- and is blind to every MLX API change, which `mlx_arm64` alone can see. Red-before/green-after and the `## Owed` residue in [gate-metal-mlx-compile.md](specs/gate-metal-mlx-compile.md) | bug | +| [#1773](https://github.com/mudler/vllm.cpp/issues/1773) | `GATE-ANCHOR-PER-JOB` | **`main` cannot return to green by itself: the diff gates anchor on a RUN-level `status=success`, so a red or cancelled `main` widens its own range and keeps re-flagging the commits blocking it.** Measured at `21abaf169`: the query returns `fafa16f0f` from 2026-08-12, a **484-commit** range, while `documentation-checkpoint` itself concluded `success` at `1fdd3e26d` eleven days later inside a run whose own conclusion is `cancelled` (run `32625264281`) -- exactly the per-job/run-level distinction `scripts/main-baseline.py` has documented since [#274](https://github.com/mudler/vllm.cpp/issues/274) and which the job asking the question never called. A SECOND defect, sharper than the cycle: `last-gated-commit` is itself cancellable and the consumers carry `if: always()`, so when the resolver loses the race `LAST_GREEN` renders empty, the step falls back to `PUSH_BASE`, and the gate passes over one push -- **`main`'s diff gates are green when the resolver dies and red when it lives**, neither reading being about the tree. The re-flagged population is FIVE commits, not the three [#1764](https://github.com/mudler/vllm.cpp/issues/1764) saw (`dd8a3b0e1`, `8daf58e77`, `38ec0da4a`, `5073df622`, `65d6cdaed`); a widening range accretes offenders. **The external-fork hypothesis in #1764 and #1773 is REFUTED:** `check-role-discipline.py` reads no ref at all -- for a single-parent commit `arrives_via_row_pr` is commit-message text only -- and the one NON-fork commit of the five (`8daf58e77`, from `mudler/vllm.cpp:row/ENG-RELEASE-WINDOWS-test-thread-raii`) fails identically. The real discriminator is the squash SUBJECT: all five lack `(#N)`, where every other squash on `main` carries one even under `squash_merge_commit_title = PR_TITLE`. **What suppressed it is INFERRED, NOT MEASURED.** An explicit `commit_title` at merge is the documented way to suppress the append and the only candidate this row found, but the pulls endpoint keeps no merge-input payload, so the hypothesis is consistent with the evidence rather than demonstrated by it -- and all five were merged by the same `localai-bot` account that appends `(#N)` correctly on every other squash it lands. The design does not rest on it: the anchor fix is correct whatever suppressed the append. FIXED IN FLOW by the anchor, not by the rule: each diff-scoped job resolves its own anchor in its own first step, and **the unit of the verdict is the STEP, not the job** -- GitHub concludes a job `failure` the moment any step fails and marks every remaining step `skipped`, so a job-level question reads `failure` over a gate that refused the range and over a gate that never executed and cannot tell them apart (measured on `commit-protocol-tag`, runs `32601353990` through `32623377380`: job `failure` five times running with the strict-trailer step `skipped` every time, and `038ff61e5..a4f2a9585`, six commits, never gated). A gate step also SKIPS rather than narrows on a degraded anchor read, because a narrowed pass would conclude and advance the anchor past what the narrowing dropped. The checker is untouched. Spec [gate-anchor-per-job.md](specs/gate-anchor-per-job.md) | bug | +| [#1787](https://github.com/mudler/vllm.cpp/issues/1787) | `GATE-ANCHOR-PER-JOB` | **`scripts/check-symbol-anchors.py` resolves a citation written as `path::Symbol`, so a document that cites a symbol as a BARE backticked name with no path is outside its population entirely and the citation can name nothing at all.** Measured on `row/GATE-ANCHOR-PER-JOB` at `f671ca92d`: `.agents/specs/ci-concurrency.md:151` named `test_every_diff_scoped_step_bases_on_the_last_gated_commit`, which `26def4c8f` renamed to `test_every_diff_scoped_step_bases_on_its_own_jobs_anchor` in the same pull request ([#1776](https://github.com/mudler/vllm.cpp/pull/1776)); a `grep -rn` returns one hit, in the spec, and none in `tests/`, while the suite is green and `scripts/agent-preflight.sh` is rc 0. DISTINCT from [#1199](https://github.com/mudler/vllm.cpp/issues/1199), which measured the `path::Symbol` form WORKING, and from [#1601](https://github.com/mudler/vllm.cpp/issues/1601), which is a bare LINE citation into an upstream: this is the bare SYMBOL form, the one this repository's own style prefers and the one the checker never sees. **NOT FIXED IN FLOW:** resolving a bare backticked identifier is a change to checker semantics needing its own row, spec and red-before evidence, and it carries a real design question -- which backticked tokens are citations and which are prose -- so a naive rule would red on every backticked word in every document. The stale citation itself IS repaired in #1776. Owned by `GATE-ANCHOR-PER-JOB` and listed under `## Owed` in [`gate-anchor-per-job.md`](specs/gate-anchor-per-job.md) | gap | diff --git a/.agents/specs/ci-concurrency.md b/.agents/specs/ci-concurrency.md index cde0ecd01..4e5b9569e 100644 --- a/.agents/specs/ci-concurrency.md +++ b/.agents/specs/ci-concurrency.md @@ -57,6 +57,18 @@ commits the cancelled one skipped. `github.event.before` remains the fallback when no successful run is found, which preserves today's behaviour on a fresh branch. +> **SUPERSEDED, 2026-08-23, by `GATE-ANCHOR-PER-JOB` +> ([#1773](https://github.com/mudler/vllm.cpp/issues/1773)).** The paragraph +> above describes the shape this row shipped and it is no longer what the tree +> does. A RUN-level `status=success` froze the base for eleven days and 484 +> commits: `main` could not return to green by itself, because a red run never +> advanced the anchor and each push therefore re-flagged the same landed +> commits over a wider range. The base is now resolved PER GATE STEP, by the +> job that consumes it, from the newest push run in which that STEP returned a +> verdict -- `success` or `failure`, because a commit on `main` is immutable +> and one alarm per violation is a complete alarm. Read +> [`gate-anchor-per-job.md`](gate-anchor-per-job.md) for the current rule. + This is what makes cancellation lossless. It is the enabling change, not a side change. @@ -148,8 +160,12 @@ a group-free job, on the theory that group-freeness was the invariant. It is not. A workflow-level `cancel-in-progress` cancels every job in the run, including jobs carrying no group, so the move protects nothing once the push lane is latest-only. The self-healing base is the whole repair; the move is -tidiness. `test_every_diff_scoped_step_bases_on_the_last_gated_commit` asserts -the real invariant and says so in its docstring. +tidiness. `test_every_diff_scoped_step_bases_on_its_own_jobs_anchor` asserts the +real invariant and says so in its docstring. (It was +`test_every_diff_scoped_step_bases_on_the_last_gated_commit` until #1773 +renamed it with the rule; the citation is a bare backticked name with no +`path::Symbol` form, so `scripts/check-symbol-anchors.py` did not catch the +staleness -- see `gate-anchor-per-job.md` §10.) **What the tests found.** Writing the group-free assertion surfaced a second diff-scoped gate nobody had noticed: `check-role-discipline.py --base/--head` diff --git a/.agents/specs/gate-anchor-per-job.md b/.agents/specs/gate-anchor-per-job.md new file mode 100644 index 000000000..aae8df9bf --- /dev/null +++ b/.agents/specs/gate-anchor-per-job.md @@ -0,0 +1,940 @@ +# GATE-ANCHOR-PER-JOB: a diff gate anchors on its OWN last verdict, not on a run-level `success` + +**Row:** `GATE-ANCHOR-PER-JOB` +**Issue:** [#1773](https://github.com/mudler/vllm.cpp/issues/1773) +**Refs:** [#1764](https://github.com/mudler/vllm.cpp/issues/1764) (the live reds), [#274](https://github.com/mudler/vllm.cpp/issues/274) / [`main-verifiability.md`](main-verifiability.md) (the tool that already knew), [#822](https://github.com/mudler/vllm.cpp/issues/822) and [#863](https://github.com/mudler/vllm.cpp/issues/863) (why the anchor exists at all), [#1262](https://github.com/mudler/vllm.cpp/issues/1262) (a landed-exception whose argument this row invalidates) +**Base:** `origin/main` `21abaf169f1ce0bcaf2598056c6a0278e8bf0241` +**Status:** ACTIVE, 2026-08-23 + +## 1. Scope + +**In.** How the three diff-scoped jobs in `.github/workflows/ci.yml` resolve the +`base` of the range they walk on the push lane, and the reader that resolves it: + +- `documentation-checkpoint` -- `check-now-current.py`, `check-role-discipline.py` +- `commit-protocol-tag` -- the `FOLLOWING_AGENTS_PROTOCOL` walk, `check-commit-trailers.py` +- `agent-record`'s role-discipline step -- `check-role-discipline.py` +- `scripts/main-baseline.py` -- gains the anchor query, in the module that + already owns the per-job doctrine +- `last-gated-commit` -- stops resolving a shared string; keeps its closed-PR + guard role, which is the only thing `agent-record` may inherit an `if:` from + (#873) + +**Out of scope, deliberately.** Every rule the three jobs enforce once the range +exists. `check-role-discipline.py` is **not** modified: not its path +classification, not its cutovers, not `arrives_via_row_pr`. §5 records why, and +that decision is the one a reviewer should attack first. Also out: the +`SiteGuard` error and the `commit-protocol-tag` cause in #1764 §1 and §3 -- this +row owns #1764 §2 only. + +**This alters which commits a gate examines, never what is demanded of them.** + +**One record outside `ci.yml` is invalidated by the change and is reconciled in +it.** `scripts/check-commit-trailers.py`'s landed-exception block argued its own +necessity from the old rule: "`LAST_GREEN` advances only on a GREEN run, so a +range containing an unrepairable red is re-walked ... forever. `ci.yml:74` relies +on exactly that property." The anchor now advances on a CONCLUDED run, failure +included, so that sentence is false here. The exceptions are still necessary -- +an unrepairable message on `main` still reds the push that lands it, still reds +every run whose floor reaches back over it, and still reds every pull request +whose merge base predates it -- but the argument had to be rewritten rather than +left to be re-derived from a property the tree no longer has. +[#1262](https://github.com/mudler/vllm.cpp/issues/1262) is the live instance. + +## 2. Anchors + +Local CI plumbing. No vLLM counterpart: vLLM's CI gates a different repository +layout and has no record protocol to diff-scope. Nothing is ported. + +| What | Where | +|---|---| +| The defective query | `.github/workflows/ci.yml`, job `last-gated-commit`, step `resolve` | +| Its three consumers | `ci.yml` jobs `agent-record`, `documentation-checkpoint`, `commit-protocol-tag` | +| The doctrine it contradicts | `scripts/main-baseline.py` module docstring, "this is why nothing here reads `run.conclusion`" | +| The per-job verdict machinery being reused | `scripts/main-baseline.py` `job_matches`, `jobs_for`, `gh_api` | +| What actually reddens today | `scripts/check-role-discipline.py` `arrives_via_row_pr` | + +## 3. What is measured, at the base revision + +Five facts. Each was read from the forge or from this tree on 2026-08-23, and +each is reproducible with the command beside it. + +### 3.1 The anchor is eleven days stale, and the range is 484 commits + +``` +gh api "repos/mudler/vllm.cpp/actions/workflows/ci.yml/runs?branch=main&event=push&status=success&per_page=1" \ + --jq '.workflow_runs[0].head_sha' + -> fafa16f0f32acc8255e113a2cbc35f8b99cf2072 (2026-08-12T23:53:24Z) + +git rev-list --count fafa16f0f..origin/main -> 484 +``` + +That is what `documentation-checkpoint` walks on the push lane today. + +### 3.2 The gate's verdict on `main` is decided by a race, not by the tree + +Run `32625264281`, push of `1fdd3e26d` (2026-08-23T07:19:38Z): + +``` +last-gated-commit: completed/cancelled +documentation-checkpoint: completed/success +commit-protocol-tag: completed/success +run conclusion: cancelled +``` + +`documentation-checkpoint` carries `if: always()`, so it runs even when the job +that resolves its base was cancelled. `LAST_GREEN` then renders empty, the step +falls back to `PUSH_BASE`, and the gate passes over one push. When +`last-gated-commit` survives instead, the same job walks 484 commits and fails. + +**So `main`'s diff gates are green when the resolver loses the cancellation race +and red when it wins.** Neither reading is about the tree. This is sharper than +the cycle #1773 describes and it is the same root: a shared string, resolved in +a separate cancellable job, at the run level. + +### 3.3 A run-level query cannot see a job that concluded + +The six newest completed push runs on `main`, run conclusion against +`documentation-checkpoint`'s own conclusion: + +| run | head | run conclusion | `documentation-checkpoint` | +|---|---|---|---| +| 32627787237 | `15298f033` | cancelled | absent (never started) | +| 32626139993 | `aa67130cc` | cancelled | absent | +| 32626052337 | `6991b78d2` | cancelled | absent | +| 32625688839 | `38e6ac0a3` | cancelled | absent | +| 32625264281 | `1fdd3e26d` | cancelled | **success** | +| 32625120219 | `175733000` | cancelled | absent | + +A per-job reader anchors on `1fdd3e26d`, eleven days newer than what the +run-level query returns. `main-baseline.py` has said "the verdict is computed +from PER-JOB conclusions, always" since #274; the job that asks the question +does not call it. + +### 3.4 The commits being re-flagged are five, not three, and every one arrived through a merged pull request + +`python3 scripts/check-role-discipline.py --base fafa16f0f --head HEAD` reports +`dd8a3b0e1`, `8daf58e77`, `38ec0da4a`, `5073df622`, `65d6cdaed`. #1764 saw three +because it measured at an older head: the widening range **accretes** offenders, +which is the cycle's signature. + +| commit | pull request | head branch | fork? | +|---|---|---|---| +| `dd8a3b0e1` | [#640](https://github.com/mudler/vllm.cpp/pull/640) | `ElderOrb/vllm.cpp:fix/windows-msvc-vulkan-build` | yes | +| `8daf58e77` | [#1159](https://github.com/mudler/vllm.cpp/pull/1159) | `mudler/vllm.cpp:row/ENG-RELEASE-WINDOWS-test-thread-raii` | **no** | +| `38ec0da4a` | [#1056](https://github.com/mudler/vllm.cpp/pull/1056) | `tbrasser/vllm.cpp:row/BACKEND-ROCM-ATTN-REGISTER` | yes | +| `5073df622` | [#1065](https://github.com/mudler/vllm.cpp/pull/1065) | `tbrasser/vllm.cpp:row/BACKEND-ROCM-ATTN-RUNNER` | yes | +| `65d6cdaed` | [#945](https://github.com/mudler/vllm.cpp/pull/945) | `jimmykarily/vllm.cpp:row/BUILD-GCC16` | yes | + +All five are `merged`, all five have `merge_commit_sha` equal to the flagged +commit, and all five landed on 2026-08-18. + +### 3.5 The fork hypothesis is REFUTED, and the real discriminator is the squash subject + +#1764 and #1773 both carry the hypothesis that an external contributor's branch +lives on a fork, "so a checker that verifies the change arrived on a task branch +by looking at `origin`'s refs cannot find it and never will." + +**`check-role-discipline.py` reads no ref.** For a single-parent commit +`arrives_via_row_pr` is `ROW_BRANCH.search(subject + body) or +PR_REFERENCE.search(subject)` -- commit message text and nothing else. `git +ls-remote` is never called, `origin/*` is never resolved. The hypothesis +predicts that the one non-fork commit passes; `8daf58e77` came from +`mudler/vllm.cpp:row/…` and fails identically. The hypothesis is refuted by its +own table. + +What the five share is their **subject**: + +``` +windows: fix native MSVC/Vulkan build portability +fix(ENG-RELEASE-WINDOWS): the api-server gate can report its own failure again +feat(BACKEND-ROCM): register a ROCm attention backend for kROCM +feat(BACKEND-ROCM): select the attention backend in the runner +build: make the tree compile on gcc 16, and add a CI lane so it stays that way +``` + +No `(#N)`. Every other squash on `main` has one, because GitHub appends the +number even under this repository's `squash_merge_commit_title = PR_TITLE`: +[PR #1752](https://github.com/mudler/vllm.cpp/pull/1752)'s title contains no `#` +at all and it landed as `fix(V1-LOGITSPROC-HOST-ADDRESSABLE): gate the +logits-processor bounce on host addressability, not unified memory (#1752)`. + +**The mechanism is INFERRED, not measured, and this row cannot measure it.** An +explicit `commit_title` is the documented way to suppress the append, and it is +the only one this row could find. What the forge does not record is whether the +merge call actually carried that field: the pulls endpoint keeps no merge-input +payload, so the hypothesis is consistent with the evidence rather than +demonstrated by it. Two facts weaken it further, and both are stated here rather +than left out. Four of the five pull requests carry a "Maintainer change on top" +section in their bodies, not three as this spec first said, and #1159 carries +none. And all five were merged by the SAME account, `localai-bot`, that appends +`(#N)` correctly on every other squash it lands, so whatever differed on +2026-08-18 differed within one actor's own behaviour. + +The design does not rest on this. §4 is a coverage fix and is correct whatever +suppressed the append; the landing rule in AGENTS.md is worth writing because +the default title is right regardless of which path produced the exception. + +**`.agents/issue-index.md`'s row was corrected to match**, in this pull request, +and the reason first given for leaving it was wrong. It said the file is +append-only and AGENTS.md forbids editing a row that has landed. This row has +not landed: this pull request ADDS it, and +`scripts/check-issue-index-append-only.py` diffs two points, the merge base and +the head, so a further commit on this branch that amends a row this branch also +added still reads as a pure addition -- `git diff --numstat` stays `1 0` and the +checker returns `OK`. `squash_merge_commit_message = PR_BODY` means exactly one +version of the row will ever exist on `main` and it cannot be corrected +afterwards, so the last moment to fix it is before the merge. The row now labels +the mechanism inferred and states the step granularity this section and §4.3 +settled on. + +| pull request | merged by | "Maintainer change on top" | +|---|---|---| +| [#640](https://github.com/mudler/vllm.cpp/pull/640) | `localai-bot` | yes | +| [#1159](https://github.com/mudler/vllm.cpp/pull/1159) | `localai-bot` | no | +| [#1056](https://github.com/mudler/vllm.cpp/pull/1056) | `localai-bot` | yes | +| [#1065](https://github.com/mudler/vllm.cpp/pull/1065) | `localai-bot` | yes | +| [#945](https://github.com/mudler/vllm.cpp/pull/945) | `localai-bot` | yes, singular | + +**So the rule is not unsatisfiable by an external contributor.** A fork pull +request merged with the default squash title passes today. There is nothing to +teach the checker and nothing to waive. §5 is that decision, argued. + +## 4. Design + +### 4.1 The anchor is per JOB, resolved BY that job + +The base a diff-scoped gate walks from is defined by one question: + +> From which commit onward has **this job** not yet returned a verdict? + +Three properties follow, and each fixes one measured defect. + +**PER JOB, not per run.** `sanitize-cpu` is `continue-on-error`, so a run's +conclusion can read `success` over a red job (#274); and a run's conclusion reads +`cancelled` over a job that ran to completion (§3.3). The run conclusion answers +a different question and must never be read. This is `main-baseline.py`'s +doctrine, applied where the question is asked. + +**CONCLUDED, not GREEN.** The anchor advances past a commit at which the job +concluded `failure` as readily as past one where it concluded `success`. This is +the change that breaks the cycle, and it is the one to argue for: + +- The gate is **per commit**. Every commit in range is inspected on its own; the + range decides only which commits are inspected. +- A commit on `main` is immutable. Once a violation lands, no later push can + repair it. Anchoring on `success` therefore converts one violation into a + permanent block on every future push -- and the gate stops being able to say + anything about new commits, because it is drowned by an old one it cannot fix. +- Anchoring on "concluded" gives every commit **exactly one** verdict, from the + first run of that job whose range contains it. One alarm per violation is a + complete alarm. A blocked branch is not enforcement; it is a broken alarm. +- The rule is still **blocking** where blocking works: on the pull request lane + the same checkers run over `base..head` and refuse the merge. The push lane is + the detector for whatever bypassed that, and its verdict is a report. + +**RESOLVED BY THE CONSUMING JOB.** §3.2 is not a variant of the cycle, it is a +second defect: a shared string resolved in a separate job that `if: always()` +consumers outlive. Each job resolves its own anchor in its own first step, so +the anchor exists exactly when the gate does. `last-gated-commit` stops +resolving anything and keeps only the closed-PR guard that `agent-record` +inherits through `needs:` (#873). + +### 4.2 The floor + +`scripts/main-baseline.py --gate-anchor --gate-step ...` walks the +newest 20 push runs on the branch, newest first, and returns the `head_sha` of +the first run in which every payload entry matching that job id carries a +conclusion in `{success, failure}` **and every named step inside it does too**. + +**The window bounds the RANGE. It does not bound the loss, and the first draft +of this section claimed otherwise.** When no run in the window qualifies the +anchor falls back to a floor, so a range never widens past the window however +long `main` has been red. The sentence "the degradation is toward more coverage +rather than less" was false of the code it described. Measured on the +implementation as first written, which read `window` runs and floored on +`runs[window - 1]`: 21 non-qualifying pushes put the anchor at run 2, and +because a base is EXCLUSIVE the range is then `3..21` -- runs 1 **and** 2 both +fall outside every future range. 25 pushes leave runs 1 to 6 outside. The first +draft of this paragraph said "left run 1 outside" and was itself off by one; the +comment in `resolve_gate_anchor` describes the FIXED code and is correct as +written. +Commits roll off the back permanently, because a range that starts inside the +window can never reach behind it again. + +Two things are done about that, and neither is a denial. + +**The off-by-one is fixed.** The query now reads `window + 1` runs. The first +`window` are anchor CANDIDATES and the extra one is the floor, so the oldest +candidate's own head sits INSIDE the floor range instead of on its base. It used +to sit on the base and its commit was excluded from the very range the floor +exists to guarantee. This costs nothing: it is the same single runs call with +`per_page` one higher. + +**The residual loss is stated with its bound.** A commit is lost only when its +own gate step has failed to return a verdict on `window` consecutive pushes, and +only after the pull request lane already gated it at merge. Walking further is +the alternative, and it costs an unbounded number of API calls per push across +three jobs. The trade is deliberate. +`test_past_the_window_commits_roll_off_PERMANENTLY` asserts it as a property of +the code, so it cannot quietly stop being true. + +**And the off-by-one is fixed only where the branch is LONGER than the window.** +`window + 1` runs put a run past the candidates in hand. Below that threshold no +such run exists, the oldest available run is itself a candidate, its head becomes +the base, and its own commit falls outside the range it bases. It cannot be +repaired from this payload: naming the parent is what would be needed, and a +`workflow_run` object carries `head_sha` and no parent. +`test_a_SHORT_history_floors_on_the_oldest_run_and_EXCLUDES_it` states the +residual rather than leaving the code to imply it is not there. + +**The degenerate case of that bound was a live defect and is fixed.** With ONE +run in the window `runs[-1]` is `runs[0]`, so the anchor resolved to the head +being pushed and `base..head` was EMPTY: a gate reporting success over no commits +at all, and CONCLUDING, which advanced its own anchor. On `mudler/vllm.cpp`, +whose window is always full, it is latent; on a fork's first push it is +reachable. The floor is now never the run being pushed -- with a single run there +is nothing to floor on, so the answer is the clean absence and `ci.yml` uses +`$PUSH_BASE`. `test_the_floor_is_NEVER_the_head_being_pushed` holds it over every +window size, and `test_a_run_never_anchors_itself`, which used to assert the harm +its own docstring named, now asserts the absence. + +**A degraded query SKIPS the gate. It does not narrow it.** `--gate-anchor` +exits 3 on `REMOTE_UNVERIFIED` and 1 on a clean absence, the split AGENTS.md +already documents for `scripts/agent-pr-body.py`. The two may not be collapsed. +On a clean absence the branch has no gated history and `PUSH_BASE` is the honest +base. On a degraded read the base is UNKNOWN, and falling back to `PUSH_BASE` +would run a narrowed pass whose success then advances the step's own anchor past +everything the narrowing dropped. The anchor step exports +`GATE_ANCHOR_DEGRADED=true` and the gate steps' `if:` turns that into a skip. A +skipped step is not a verdict, so the anchor cannot advance and the next +readable run walks the span whole. + +The current run never anchors itself: its own job is `in_progress`, so its +conclusion is `null` and it does not qualify. + +### 4.3 What "no commit is skipped" means, exactly, and why the JOB is the wrong unit + +`github.event.before` chains: each push's `before` is the previous push's `sha`, +so the union of the naive ranges covers every commit **provided every push's job +runs**. #863 was the hole a cancelled job leaves in that chain. + +**The first draft of this row re-created that hole one level down, and the claim +"unbroken by construction" was false.** GitHub concludes a job `failure` the +moment any step fails, and marks every REMAINING step `skipped`. A job-level +question therefore reads `failure` over a gate that refused the range and over a +gate that never executed, and cannot tell them apart. Both `agent-record` and +`commit-protocol-tag` place their diff-scoped gate AFTER other steps, so under a +job-level anchor the base advances past commits the gate never looked at. + +Measured live on `commit-protocol-tag`, where the strict trailer walk runs +nowhere else on the push lane: + +| run | head | job | strict-trailer step | +|---|---|---|---| +| 32599040638 | `038ff61e5` | success | success | +| 32601353990 | `1a1d17e53` | failure | **skipped** | +| 32608320394 | `6354755ba` | failure | **skipped** | +| 32613454280 | `b508cbce6` | failure | **skipped** | +| 32616777372 | `66d1b0a90` | failure | **skipped** | +| 32623377380 | `a4f2a9585` | failure | **skipped** | +| 32625264281 | `1fdd3e26d` | success | success | + +A job-level anchor walks `038ff61e5` to `1a1d17e53` to `1fdd3e26d`, and +`038ff61e5..a4f2a9585`, six commits, never receives a trailer verdict from any +run. `agent-record` has the same shape: the last push run in which its +role-discipline step actually executed is 32580850008 at `8540a2755`, and +`8540a2755..66d1b0a90` is twelve commits. Its coverage survived only because +`documentation-checkpoint` ran the same checker, which is luck rather than +construction. The old run-level rule could not do this, because a failed +`agent-record` forces the RUN conclusion to `failure` and that run never +anchored at all. + +**The unit is therefore the STEP, and three things enforce it.** + +1. `--gate-step` is REQUIRED. `steps_concluded` reads `steps[].conclusion` from + the jobs payload and qualifies a run only when every named step of every + matching entry concluded. A named step that is absent does not qualify: it is + missing because it was never reached, because it was renamed, or because the + payload is truncated, and none of the three is evidence that the gate ran. + The flag-less form exits 2 rather than defaulting to the job. +2. **Every diff-scoped step runs on every push.** Each gate step carries + `if: !cancelled() && steps.checkout.outcome == 'success' && steps.anchor.outcome == 'success' && env.GATE_ANCHOR_DEGRADED != 'true'`. + The first condition is the one that closes the hole: without it an earlier + step's failure marks the gate `skipped`, and the anchor waits for a verdict + that never comes. The other three are exactly the cases where a conclusion + would be a lie, and in each the step skips rather than reporting one. +3. **One gate per step.** `documentation-checkpoint` ran `check-now-current.py` + and `check-role-discipline.py` in ONE step under `set -eu`, so a + `check-now-current` failure aborted before the arrival gate ran while the + step still concluded. That is the same hazard inside a single step, and the + step is now split in two. `test_one_diff_scoped_checker_per_gate_step` holds + one gate per step, and + `test_nothing_fallible_PRECEDES_the_gate_in_its_own_body` holds the other + half. The first draft claimed the single test held both, and it did not: it + read the first `python3 ` and compared it with the step's one checker, so + a fallible NON-`python3` command inserted before the gate was invisible, and a + step running no `scripts/check-*.py` left the population entirely through + `if not checkers: continue`. Both were confirmed by mutation. The property is + now held by resolving the body: a gate step runs the range prelude -- `set`, + `[`, `echo`, `:` and variable bindings, and nothing else -- and then its + GATE, identified as the first command that is not one of those, which must + CONSUME the range. The population includes the one step whose gate is inline + shell rather than a checker. + + **The first version of that repair over-claimed in turn, and round 3 found + three more escapes past it.** It said command substitution was refused in the + prelude so a fallible call could not hide inside an allowed one. It was not: + the parser `continue`d on ANY `name=...` statement before the refusal ran, so + `base="$(git rev-parse --verify "${base}^{commit}")"`, `_p="$(git fetch -q + origin nope)"` and the backtick spelling of it were all invisible while each + aborted the step under `set -eu`. The first is the realistic one: normalising + the anchor to a full commit id immediately before the gate is the natural + next edit in that step, and it exits 128 whenever the anchor is not in the + checkout. Separately, `[` was allowlisted on the reasoning that "its failure + the surrounding `if`/`||` consumes", but the parser splits on `||` and + discards it, so nothing checked that a given `[` HAS one and a bare + `[ -f .git/NO_SUCH ]` aborted at rc 1. All four gave `Ran 109 / OK`. + + The refusal now runs over EVERY statement before the gate, bindings included, + and a `[` is admitted only where an `if`/`while` header or an `||`/`&&` reads + its status. Two residuals are stated on the test rather than hidden: a + statement that itself references `$base` is still not distinguishable from + the gate, and the allowlist remains a judgement about `set`, `echo` and `:` + rather than a proof. `[` was the member with a routine failing form and it is + the one now consumption-checked. + +§6's `test_no_commit_is_ever_skipped` asserts the property at the granularity +that can actually fail. The union of the ranges is NOT that property: a +job-level anchor keeps the union whole while the gate is skipped on every push, +because `failure` advances the base whether or not the gate executed. The +property is that for every commit there is at least one push whose range +contains it AND whose gate step returned a verdict. + +**The transition costs one wide range, in the safe direction.** Measured on +`origin/main` at `11ccdcf76`: `commit-protocol-tag` resolves a verdict anchor at +`ff8f728071` because both its step names already exist, while `agent-record` and +`documentation-checkpoint` fall back to the window floor `08c81a892`, a bounded +23 commits. `documentation-checkpoint` floors because its second step is new in +this change and has no history, and `agent-record` floors because its +role-discipline step was skipped on all 20 pushes in the window, which is the +defect being fixed reporting itself. 23 is the floor working. 484 was the bug. + +## 5. The decision on the five commits, and what was rejected + +**Decision: `check-role-discipline.py` is not touched, and no exception is +recorded.** With §4 in place the five commits are older than the newest run in +which `documentation-checkpoint` concluded, so they leave the range on the first +push after this lands. They were already reported -- `documentation-checkpoint` +concluded `failure` at `dd8a3b0e1`'s own push (run `32080067480`) and at +`8daf58e77`'s (run `32108685135`), and every red run since has named all five. +The alarm rang. It cannot ring them into a state they can no longer reach. + +Three alternatives were evaluated and rejected. + +| Option | Rejected because | +|---|---| +| Teach the checker about fork-origin branches | It would fix nothing: §3.5 shows the checker reads no ref, and the one **non**-fork commit of the five fails identically. It repairs a hypothesis, not a defect. | +| Widen `PR_REFERENCE` from the subject to the whole message | It deletes the obligation. `dd8a3b0e1`'s body says `Issue: #503`, and *every* commit in this repository names an issue in its body, because AGENTS.md requires one. The gate would pass every direct-to-main push ever made. §6's `test_a_body_only_issue_reference_does_not_satisfy_arrival` pins this shut. | +| A bounded exception list of the five SHAs | AGENTS.md: "The project has no waiver registry. An exception registry is a state log, and this protocol has no state log." A five-line allowlist in a checker is that registry, and it would outlive its reason. | + +**What the recurrence needs instead is a landing rule, not a checker change.** +AGENTS.md gains one sentence under `## Landing work`: land a squash with the +default title so GitHub appends `(#N)`, because the arrival gate reads the +commit message and a custom `commit_title` suppresses the only evidence it has. +That is where the defect was introduced and where it can be prevented. + +## 6. Tests + +All offline. `tests/scripts/test_main_baseline.py` gains four classes; nothing +existing is relaxed. + +1. **`GateAnchorTests`** -- `gate_anchor()` against synthetic payloads. + - a run whose *conclusion* is `cancelled` but whose named job concluded + `success` **is** the anchor. RED before: no such function. + - a run whose named job concluded `failure` **is** the anchor. + - a run whose named job is `cancelled`, `skipped`, absent, or still `null` is + **not** the anchor and the walk continues. + - a matrix job anchors only when **every** lane concluded. + - no qualifying run in the window returns the window's oldest head -- the + floor -- and says so. +2. **`AnchorCycleConstructionTests`** -- the feedback loop, built rather than + read. A synthetic sequence of pushes P1..P6 in which P2 is a violating commit + and P3..P6 are ordinary, run through **both** anchor rules: + - `test_run_level_anchor_widens_across_pushes` -- the run-level rule holds the + anchor at P1 and the range grows 1, 2, 3, 4, 5, re-including P2 every time. + This is the cycle, asserted as a sequence of range sizes. + - `test_per_job_anchor_reports_the_violation_once` -- the per-job rule reports + P2 exactly once and the range never exceeds the gap since the last verdict. + - `test_no_commit_is_ever_skipped` -- the union of every range equals every + commit, on both rules, including across a cancelled run. The per-job rule + may not buy its exit from the cycle with a hole. +3. **`AnchorStepTests`** -- executes the three real step bodies out of `ci.yml` + under the existing `run_shimmed` argv recorder, with `gh`/`python3` shimmed: + - each of the three jobs resolves an anchor naming **its own** job id; + - a push whose resolver returns nothing falls back to `PUSH_BASE`; + - the `pull_request` lane still uses `PR_BASE`/`PR_HEAD` and never queries + the forge. +5. **The step-granularity cases added by the review repair** (#1776). All in + `tests/scripts/test_main_baseline.py`: + - `test_no_commit_is_ever_skipped`, RE-EXPRESSED at step granularity. The + job-level version modelled a job as one atomic verdict and could not fail + for this defect, which made it a correct test of the wrong thing. + - `test_a_SKIPPED_gate_step_in_a_CONCLUDED_job_does_not_anchor` states F1 as + one assertion, on the live shape of runs 32601353990 to 32623377380. + - `test_a_gate_step_ABSENT_from_the_payload_does_not_anchor` and + `test_every_named_step_must_conclude_not_just_one`. + - `test_an_anchor_with_no_named_step_is_REFUSED`, where the flag-less form raises + rather than silently meaning the job. + - `test_the_floor_covers_the_oldest_CANDIDATES_own_head` and + `test_past_the_window_commits_roll_off_PERMANENTLY` cover F2, fixed where it + could be and asserted where it could not. + - `test_every_named_gate_step_exists_in_its_own_job`, + `test_every_step_that_READS_the_anchor_is_NAMED_by_it`, + `test_every_gate_step_SKIPS_rather_than_narrows` and + `test_one_diff_scoped_checker_per_gate_step` hold the workflow shape the fix + depends on, selected as a POPULATION by "reads `GATE_ANCHOR`" so a newly + added diff-scoped gate fails here rather than landing uncovered. + - `test_a_DEGRADED_query_skips_the_gate_instead_of_narrowing_it` and + `test_a_CLEAN_absence_still_falls_back_to_push_base` cover F3, executed + through the real step body with the `python3` shim exiting 3 and then 1. +7. **The cases added by the SECOND review repair** (#1776), all in + `tests/scripts/test_main_baseline.py`: + - `test_every_gate_step_SKIPS_rather_than_narrows`, RE-EXPRESSED. It asserted + four substrings and is now a RESOLVED boolean over all sixteen states a + guard has to decide. `_Expression` gained unary `!` and the status-function + form so `!cancelled()` evaluates rather than being read. + - `test_nothing_fallible_PRECEDES_the_gate_in_its_own_body` -- the half of the + one-gate-per-step shape the old test did not hold, over a population that + now includes the inline-shell gate. `commands_in` became `statements_in` + in round 3: it returns bindings as statements instead of dropping them, and + keeps the `||`/`&&` that follows each piece, because whether a failure + aborts the step depends on what comes after it. + - **`BaselineRunsPayloadTests`** -- the first tests to EXECUTE `baseline_runs`, + the advisory lane's reader, which the two tests naming it had replaced at + the `collect` layer. A list, a null and `{"message": "Not Found"}` used to + render as "No completed baseline run found on main."; they are now + `REMOTE_UNVERIFIED`, and a genuinely empty window still reads as an + absence. + - **`PushRunsPayloadTests`** -- the first tests to EXECUTE `push_runs`, driving + the real `main` through a faked `gh_api`: a degraded call, a list payload, a + null payload and `{"message": "Not Found"}` all exit 3, a genuinely empty + window still exits 1, a readable window exits 0 and prints the SHA, and + `push_runs` is proven to have no transport of its own. + - `test_the_floor_is_NEVER_the_head_being_pushed` and + `test_a_SHORT_history_floors_on_the_oldest_run_and_EXCLUDES_it` -- the fix + and the stated bound from §4.2. +8. **`ArrivalDiscriminatorTests`** -- pins §5's decision in + `tests/scripts/test_check_role_discipline.py`: a subject carrying `(#N)` + satisfies arrival, and a body-only `#N` with a bare subject does **not**. + This is the test that must red if anyone widens the match later. + +## 7. Gates + +- `python3 tests/scripts/test_main_baseline.py` and + `python3 tests/scripts/test_check_role_discipline.py`, both with the new cases + shown RED against the unmodified tree. +- `python3 -c "import yaml"` round-trip of `ci.yml` **plus an explicit + duplicate-key scan**: PyYAML accepts duplicate mapping keys that GitHub + rejects, so a parse is not a validation. +- `scripts/agent-preflight.sh` and `python3 scripts/agent-ready.py`. +- `python3 scripts/check-role-discipline.py` over this branch's own range. +- `python3 scripts/main-baseline.py --gate-anchor documentation-checkpoint` + against the live forge, reported with the SHA it returns. + +## 8. Evidence + +Measured on `row/GATE-ANCHOR-PER-JOB`, base `21abaf169`, merged onto +`11ccdcf76`. Every mutation below was applied to a tree whose five files were +hashed first, printed with `git diff --stat`, parsed to prove it was not a +build failure wearing a pass, and restored against the hash -- never against a +harness's own cleanup. `git status --porcelain` is empty and `sha256sum -c` +reports `OK` on all five afterwards. + +### RED before, GREEN after, first round (head `140745e64`) + +`python3 tests/scripts/test_main_baseline.py` on the unmodified tree: +`Ran 81 tests`, `FAILED (failures=7, errors=11)`. After: `Ran 82 tests`, `OK`. +`test_run_level_anchor_widens_across_pushes` and the `PUSH_BASE` fallback case +pass on both sides by design -- they characterise the defect and the degrade. + +`python3 tests/scripts/test_check_role_discipline.py`: `Ran 22 tests`, `OK`. +Its four new cases are characterisation pins, so mutation is the only thing that +shows they bite. M4 below is that. + +### The live gate, before and after + +| anchor | source | SHA | `fafa16f0f..` or `SHA..origin/main` | `check-role-discipline.py` | +|---|---|---|---|---| +| run-level `status=success` (removed) | run conclusion | `fafa16f0f` (2026-08-12) | **484** commits | `rc=1`, five ERRORs | +| `--gate-anchor documentation-checkpoint` | verdict, run `32626481436` | `ff8f72807` | **2** commits | `rc=0`, `OK` | +| `--gate-anchor commit-protocol-tag` | verdict, run `32626481436` | `ff8f72807` | 2 commits | `check-commit-trailers.py` `OK` | +| `--gate-anchor agent-record` | verdict, run `32616777372` | `66d1b0a90` | **13** commits | `rc=0`, `OK` | + +The last row is the argument for a per-job anchor stated as a number: two jobs +in the same workflow have anchors eleven commits apart, because they have +different cancellation profiles. One shared string could not have been right for +both. + +### Mutations + +| # | Mutation | `git diff --stat` | Result | +|---|---|---|---| +| M1 | `CONCLUDED` admits `cancelled` | `1 insertion(+), 1 deletion(-)` | 3 FAIL: the cancelled/skipped/absent case, the matrix case, the floor case | +| M2 | `resolve_gate_anchor` reads `run["conclusion"] == "success"` again | `1 +, 1 -` | 3 FAIL, including `test_per_job_anchor_reports_the_violation_once` with `AssertionError: 2 != 1 : range at p3: ['p2', 'p3']` -- **the cycle, printed by the test that constructs it** | +| M3 | delete `documentation-checkpoint`'s anchor step from `ci.yml` (the production call site) | `34 deletions(-)` | 2 FAIL + 2 ERROR, including the re-pinned `ConcurrencySemanticsTests` case | +| M4 | `PR_REFERENCE.search(subject)` becomes `…search(message)` -- §5's rejected option | `1 +, 1 -` | 2 FAIL, and `check-role-discipline.py` over the 484-commit range turns **`OK`**. The widening "fixes" #1764 §2 by deleting the obligation, and the two new cases are what stop it | +| M5 | a genuine direct push inside the NARROWED range: one commit on `probe/direct-push` at `ff8f72807` touching `src/vllm/version.cpp`, subject with no `(#N)`, issue in the body only | `1 file changed, 1 insertion(+)` | `ERROR: 03fd91554: repository change (src/vllm/version.cpp) reached main without arriving on a task branch`, `rc=1`. **The narrowed range still catches what the gate exists for.** Branch deleted, tree restored | + +M3 and M5 each parse and apply -- M3's YAML loads, M5's commit exists and is +reported by SHA -- so neither reading is an unapplied edit wearing a pass. + +### The review repair (#1776), RED before and GREEN after + +`test_no_commit_is_ever_skipped`, re-expressed at step granularity and run +against the UNMODIFIED implementation. The adapter it calls through forwards +everything except the step names, so the red and the green assert byte-identical +properties and the only difference is whether the anchor may see a step: + +``` +FAIL: test_no_commit_is_ever_skipped +AssertionError: Items in the second set but not the first: +'p2' +'p3' : commits with no verdict from any run that ran the gate: ['p2', 'p3'] +``` + +After: `python3 tests/scripts/test_main_baseline.py` reports `Ran 99 tests`, +`OK`, up from 82. `python3 tests/scripts/test_check_role_discipline.py` reports +`Ran 22 tests`, `OK`, unchanged. + +The F1 defect is also confirmed against the live forge rather than only in the +model. The table in §4.3 is `gh api .../jobs?filter=latest` on the seven runs, +and `git rev-list --count 038ff61e5..a4f2a9585` is `6` while +`8540a2755..66d1b0a90` is `12`. + +### The live gate after the repair + +Measured against `origin/main` at `11ccdcf76`: + +| anchor | source | SHA | range | +|---|---|---|---| +| `commit-protocol-tag`, both steps named | verdict, run `32626481436` | `ff8f728071` | 2 commits | +| `agent-record` | **floor**, run `32594040335` | `08c81a892` | 23 commits | +| `documentation-checkpoint` | **floor**, run `32594040335` | `08c81a892` | 23 commits | + +The two floors are the transition, and they are the design working rather than +failing. `documentation-checkpoint` floors because its second gate step is new +in this change and has no history on `main` yet. `agent-record` floors because +its role-discipline step was `skipped` on all 20 pushes in the window, which is +the defect this row fixes reporting its own extent. 23 commits is bounded by the +window. 484 was the bug. + +### Mutations, repair round + +Every mutation below was applied to a tree hashed first, printed with +`git diff --stat`, parsed to prove it was not a syntax error wearing a pass, and +restored against the hash rather than against the harness's cleanup. The harness +asserts its anchor matches before writing, which caught one mis-written mutation +that would otherwise have read as a passing test. + +| # | Mutation | `git diff --stat` | Result | +|---|---|---|---| +| M6 | `steps_concluded` ignores `steps[]`, restoring job granularity: **the finding itself** | `121 +, 27 -` | 4 FAIL, led by `test_no_commit_is_ever_skipped` | +| M7 | drop `!cancelled()` from all five gate-step guards | `248 +, 24 -` | 5 FAIL, one per gate step | +| M8 | drop `env.GATE_ANCHOR_DEGRADED != 'true'` from all five guards | `243 +, 24 -` | 5 FAIL | +| M9 | the anchor CLI exits 1 on a degraded read instead of 3 | `123 +, 26 -` | 1 FAIL, `test_the_anchor_CLI_exits_3_on_a_degraded_read` | +| M10 | `gate_anchor` fetches `window` runs, not `window + 1`: F2's off-by-one restored | `123 +, 26 -` | 1 FAIL, `test_gate_anchor_reads_one_run_PAST_the_window` | +| M11 | a `--gate-step` flag is dropped from `ci.yml`, leaving a gate nothing waits for | `247 +, 24 -` | 2 FAIL, including the population test | +| M12 | the arrival gate goes back to sharing a step with `check-now-current.py` | `249 +, 24 -` | 1 FAIL, `test_one_diff_scoped_checker_per_gate_step` | +| M13 | a gate step is renamed in `ci.yml` and the `--gate-step` value is left behind | `249 +, 25 -` | 2 FAIL + 2 ERROR | + +M9 and M10 initially came back GREEN against a first draft of these tests. Both +were real gaps rather than mutation errors: the F3 test drove the shim's exit +code and never the script's own, and the floor test used exactly `window + 1` +runs, where `runs[window]` and `runs[-1]` are the same entry and the assertion +could not discriminate. `test_the_anchor_CLI_exits_3_on_a_degraded_read` and +`test_gate_anchor_reads_one_run_PAST_the_window` were added for that reason, and +the table above is the re-run. + +### A restored tree that still behaved like the mutant + +Worth recording, because it nearly became a false finding. After M4 was restored +and its sha256 verified, `tests/scripts/test_check_role_discipline.py` still +reported the mutant's two failures. The source was correct: the hash matched, +`git status --porcelain` was empty, and line 168 read `PR_REFERENCE.search( +subject)`. The stale artefact was `scripts/__pycache__/`. + +M4 replaces `subject` with `message`, and the two words are the SAME LENGTH, so +the mutant and the original are byte-for-byte the same SIZE. Python validates a +cached `.pyc` on source mtime and size only, and the mutation and its restore +both landed inside one second. The `.pyc` therefore recorded +`mtime=1787481140 size=17091`, matched the restored file exactly, and the +interpreter served the MUTANT's bytecode from a tree that was provably clean. + +`find . -name __pycache__ -type d -exec rm -rf {} +` clears it, after which both +suites are `OK`. The general form is the one this repository already knows in +its other direction: an artefact that was not rebuilt reads as a verdict about +source that was. A same-length mutation defeats a size check, and a fast restore +defeats an mtime check, so hash the SOURCE and discard the CACHE rather than +trusting either. + +### The five original mutations, re-run + +| # | Result now | +|---|---| +| M1 | 5 FAIL, up from 3: the two new floor tests also detect it | +| M2 | 3 FAIL, including `test_per_job_anchor_reports_the_violation_once` | +| M3 | 3 FAIL + 8 ERROR, up from 2 + 2: deleting the production anchor step now also breaks the step-shape population | +| M4 | 2 FAIL, unchanged, on a checker this change does not touch | +| M5 | reproduces: `ERROR: 7c16436d8: repository change (src/vllm/version.cpp) reached main without arriving on a task branch`, `rc=1` | + +**M5 needs one note.** Run from THIS worktree it reports `REPORT` and `rc=0`, not +`ERROR` and `rc=1`. That is `has_reached_main` +(`scripts/check-role-discipline.py:317-325`) reading the checkout's own branch +name: on a `row/*` branch an unmerged commit is pending disposition rather than +landed history. Re-run from a detached checkout, which is CI's shape, it gives +`ERROR` and `rc=1` exactly as recorded. `scripts/check-role-discipline.py` is +byte-identical to `origin/main`, so the difference is the checkout and not this +change. The probe commit was built with `git commit-tree` against a temporary +`GIT_INDEX_FILE`, so no ref, index or working tree was touched. + +### Records and shape + +`.github/workflows/ci.yml` parses under PyYAML **and** an explicit duplicate-key +scan reports `duplicate keys: 0`, because PyYAML accepts duplicates GitHub +rejects. `scripts/agent-preflight.sh` rc 0. `check-commit-style.py` and +`check-commit-trailers.py` `OK` over the branch's own range. +`git diff --numstat origin/main -- .agents/issue-index.md` was `1 0` at this point, and the +row count was 636 on `origin/main` and 637 here, counted again after the merge. +Round 2 appended the [#1787](https://github.com/mudler/vllm.cpp/issues/1787) +row, so the branch now stands at `2 0` and 638. Both rows are additions, both +are last in the file, and the diff removes nothing. + +### The SECOND repair round, RED before and GREEN after (head `f671ca92d`) + +`python3 tests/scripts/test_main_baseline.py` reports `Ran 109 tests`, `OK`, up +from 99. `scripts/agent-preflight.sh` reports `All gates green.`, rc 0. +`.github/workflows/ci.yml` parses under PyYAML with `duplicate keys: 0`, and a +job-by-job comparison against `origin/main` `c98ffd4d0` reports 17 jobs on both +sides, none added, none dropped, no job-level `if:` and no `needs:` changed. The +only `if:` differences are the five gate-step guards this row adds. + +Each finding was reproduced BEFORE its repair, on the tree as the review left +it, and the reproduction is what the repair had to invalidate. + +| # | Reproduction, before | After | +|---|---|---| +| F1 | MZ: `\|\| true` on all five guards. `Ran 99`, **`OK`** | `Ran 109`, `FAILED (failures=75)`, all five steps named | +| F2 | four payload shapes through the real `main`: `A degraded -> 3`, `B list -> 1`, `C null -> 1`, `D {"message":"Not Found"} -> 1` | `A 3`, `B 3`, `C 3`, `D 3`; empty window still 1; readable window still 0 | +| F3 | MW: `git fetch` before `check-now-current.py`. `Ran 99`, **`OK`** | 1 FAIL, `documentation-checkpoint` / `Every feature checkpoint …` | +| F3 | MV: `git fetch` in the inline-shell gate step. `Ran 99`, **`OK`** | 1 FAIL, `commit-protocol-tag` / `Every new commit carries FOLLOWING_AGENTS_PROTOCOL` | +| F7 | `test_a_run_never_anchors_itself` ASSERTED the empty range its own docstring warned about | 5 FAIL when the fix is reverted, over every window size | + +The earlier round's mutations still bite on the repaired tree: M7, dropping +`!cancelled()` from all five guards, gives 5 FAIL, one per gate step; M9, exiting +1 on a degraded read, gives 5 FAIL, four of them from the new +`PushRunsPayloadTests`; M10, reading `window` runs instead of `window + 1`, gives +1 FAIL. + +Every mutation above was applied to a tree hashed first, printed with +`git diff --stat`, parsed -- PyYAML for `ci.yml`, `ast.parse` for the scripts -- to +prove it was not a syntax error wearing a pass, then restored and re-verified +with `sha256sum -c` reporting `OK` on all four files, an empty +`git status --porcelain`, an empty `git diff`, and `__pycache__` cleared under +`PYTHONDONTWRITEBYTECODE=1` on both sides of every run. That last step is not +ceremony here: it is the defect the round before this one recorded. + +The live anchors on `origin/main` at `c98ffd4d0`: + +| anchor | source | SHA | +|---|---|---| +| `commit-protocol-tag`, both steps named | verdict, run `32629309570` | `21abaf169` | +| `documentation-checkpoint`, both steps named | **floor**, run `32594040335` | `08c81a892` | +| `agent-record` | **floor**, run `32594040335` | `08c81a892` | + +`git diff --numstat origin/main -- .agents/issue-index.md` is `2 0`: the row this +row adds, amended in place before it lands, and the row for +[#1787](https://github.com/mudler/vllm.cpp/issues/1787). +`scripts/check-issue-index-append-only.py` returns `OK`, rc 0. + +### The THIRD repair round, RED before and GREEN after (head `b7557fe9d`) + +`python3 tests/scripts/test_main_baseline.py` reports `Ran 111 tests`, `OK`, up +from 109. `scripts/agent-preflight.sh` reports `All gates green.`, rc 0, against +`origin/main` `c98ffd4d0` over a 12-commit range, so no trailer or style gate is +vacuous. `ci.yml` is UNTOUCHED this round: it parses under PyYAML with +`duplicate keys: 0` on both sides, both report 17 jobs, the job sets are equal, +and no job-level `if:` and no `needs:` differs from `origin/main`. + +**F-A. The body test had THREE escapes, not the one residual it claimed.** The +parser `continue`d on any `name=...` head before the substitution refusal ran, +and `[` was allowlisted without checking that anything consumed it. Each +injection below went in immediately before `check-now-current.py` +(`ci.yml:813`). The step body was executed under `bash` with `python3` shimmed, +so the abort is measured rather than argued. + +| injected before the gate | step body, `set -eu` | gate ran | suite BEFORE | suite AFTER | +|---|---|---|---|---| +| `base="$(git rev-parse --verify "${base}^{commit}")"` | rc 128 when the anchor is absent, rc 0 when it resolves | 0x | `Ran 109`, **`OK`** | 1 FAIL | +| `_p="$(git fetch -q origin nope)"` | rc 128 | 0x | `Ran 109`, **`OK`** | 1 FAIL | +| `` _p=`git fetch -q origin nope` `` | rc 128 | 0x | `Ran 109`, **`OK`** | 1 FAIL | +| `[ -f .git/NO_SUCH ]` | rc 1 | 0x | `Ran 109`, **`OK`** | 1 FAIL | + +Row 1 is the realistic one, and its two rcs are the point: it is harmless while +the anchor resolves and aborts at 128 once it does not, which is the shallow +fetch, the rewritten branch, and the floor SHA aged out of the window. Every +failure names `documentation-checkpoint` and the step. + +**The repair discriminates rather than banning.** Three controls stay GREEN, and +in each the shell agrees with the test: + +| injected | step body | suite | +|---|---|---| +| `[ -f .git/NO_SUCH ] \|\| echo missing` | rc 0 | `Ran 109`, `OK` | +| `if [ -f .git/NO_SUCH ]; then echo x; fi` | rc 0 | `Ran 109`, `OK` | +| `_x=1` | rc 0 | `Ran 109`, `OK` | + +So `[` is consumption-checked, not forbidden, and a binding is still allowed. +The nine shapes the previous round already caught were re-run and all still +red: a heredoc, an `&&` chain, a background `&`, a function definition, a +`.` source, a `$VAR` used as a command, a `while` header, `echo "$( )"` and +`: $( )`. + +**F-D. `baseline_runs` reported an unreadable forge as a clean absence.** It had +never been executed: the two tests naming it replace `collect` one layer above. +Driven through real `main` and real `render`, a list, a null and +`{"message": "Not Found"}` each printed `No completed baseline run found on +main.` with an instruction to trigger the first run. The rc was never wrong, so +this is the advisory lane rather than a gate, but it told a human a green field +where there was no answer. `jobs_for` already refused these shapes; the two now +agree. Reverting the guard reds the three shapes and nothing else. + +**F-E. The ref-lookup assertion forbade three substrings in ENGLISH.** A comment +saying the checker does not resolve `origin/main` reddened +`test_the_checker_resolves_no_ref_to_decide_arrival` with a message about a +lookup that was not there, and the message was the whole 20 KB source. Probes, +each inserted at module level past the docstring and each verified to have +applied: + +| planted at `check-role-discipline.py` line 54 | survives prose-stripping | suite | +|---|---|---| +| a comment naming `origin/main` | no | `OK` | +| a comment naming `ls-remote` and `for-each-ref` | no | `OK` | +| `_x = git("rev-parse", "origin/main")` | yes | `FAILED` | +| `_x = git("ls-remote", "origin")` | yes | `FAILED` | +| `_x = git("for-each-ref", "refs/heads")` | yes | `FAILED` | + +String literals are kept deliberately, because a real lookup is spelled as one. +The first pass of this probe inserted at line 20, which is INSIDE the module +docstring, so the two code shapes came back `OK` and read as a passing test. +That is the unapplied-mutation trap, and it is why the table records where each +probe landed. + +**The earlier mutations still bite.** MZ, appending `|| true` to all five gate +guards, gives 75 failures naming all five steps. MW gives 1, naming +`documentation-checkpoint`. MV gives 1, naming the inline-shell gate step. +Reverting `push_runs` to its pre-F2 shape reds its three payload tests. Every +mutation was applied under a hash check, parsed, restored from a pristine copy, +and verified with `sha256sum -c`, with `__pycache__` cleared and +`PYTHONDONTWRITEBYTECODE=1` set on every run, for the reason recorded above. + +## 9. The fresh review on PR #1776, and what it changed + +The review confirmed the fork refutation, the `if: always()` race, the live +anchors, the YAML cleanliness and all five mutations, and FAILED the pull +request on four findings. All four are repaired here. + +| # | Finding | Repair | +|---|---|---| +| F1 | **Critical.** The fix re-created #863's hole at STEP granularity. `job_concluded` asked only whether the JOB concluded, and GitHub marks every remaining step `skipped` when an earlier one fails, so the anchor advanced past commits the gate never ran on. Six commits on `commit-protocol-tag`, twelve on `agent-record`. | The unit is the step: `--gate-step` is required, `steps_concluded` reads `steps[].conclusion`, each gate step carries `if: !cancelled() && …` so it runs on every push, and `documentation-checkpoint`'s two-checker step is split in two. §4.3. | +| F2 | **Medium.** Past the 20-push window the floor degraded toward LESS coverage, and §4.2 claimed the opposite. | The off-by-one is fixed by reading `window + 1` runs. The residual loss is stated with its bound and asserted by a test instead of denied. §4.2. | +| F3 | **Low.** `\|\| true` swallowed `REMOTE_UNVERIFIED`, so a degraded query silently narrowed the gate to `PUSH_BASE`. With a step-level anchor that is not only dishonest, it is a coverage hole: the narrowed pass advances the anchor past what the narrowing dropped. | rc 3 means degraded and rc 1 means clean absence, the split AGENTS.md already documents for `agent-pr-body.py`. A degraded query SKIPS the gate. §4.2. | +| F4 | **Low.** The new AGENTS.md rule added the file's only em dash, named no command, and addressed the contributor rather than the automation that performed all five merges. | Rewritten with no em dash, naming `gh pr merge --squash --subject` and the merge endpoint's `commit_title`, binding the MERGING account, and saying plainly that no gate can catch it. | + +**F5 is noted and NOT repaired here.** It is informational and pre-existing: +`test_every_diff_scoped_step_bases_on_its_own_jobs_anchor` selects its steps by +`github.event.before` appearing in a step `env:` block, so +`commit-protocol-tag`'s first gate step, which interpolates +`${{ github.event.before }}` inline instead, is outside that test's population. +The step is nonetheless covered by the population test added here, which selects +on `GATE_ANCHOR` and does include it. Normalising the two steps onto one +env-based form is a tidy-up that belongs to whichever row touches that job next, +not to a review repair. + +**One claim was softened rather than defended.** §3.5 said the offending squashes +happened because "the merger supplied an explicit `commit_title`". The review +found that plausible but not measured, and it is right: the forge records no +merge-input payload. §3.5 now labels the mechanism inferred, corrects the +"Maintainer change on top" count from three to four, and records that all five +merges were performed by the same account that appends `(#N)` correctly today. + +## 10. The SECOND fresh review on PR #1776, and what it changed + +The second review confirmed the step-granularity design by mutation and told the +implementer not to redesign it. It failed the pull request on the guards around +it. Eight findings, all repaired here. + +| # | Finding | Repair | +|---|---|---| +| F1 | **Critical.** `test_every_gate_step_SKIPS_rather_than_narrows` asserted four SUBSTRINGS and never resolved the expression. Mutation MZ -- append `\|\| true` to all five guards -- keeps every asserted substring byte-for-byte and turns the conjunction into the constant `true`, restoring the degraded-read narrowing in full. `Ran 99 / OK`. | The test RESOLVES the guard now, over all sixteen states, using the evaluator the file already had for concurrency keys. `_Expression` gained unary `!` and the status-function call form. MZ produces 75 failures across all five steps. | +| F2 | **High.** `push_runs` returned `[]` for any payload that was not a dict carrying `workflow_runs`, so an unreadable forge read as a CLEAN ABSENCE: rc 1, `$PUSH_BASE`, a narrowed pass that concludes and advances the anchor. `jobs_for` had always refused the same case. And nothing executed `push_runs` at all -- every test replaced it with a stand-in and `gh_api` appeared in no test. | `push_runs` mirrors `jobs_for`: a non-dict payload and a missing or non-list `workflow_runs` are both `REMOTE_UNVERIFIED`. `PushRunsPayloadTests` drives the real `main` through a faked `gh_api` over all four shapes plus the empty-window control and a readable window. | +| F3 | **Medium.** §4.3 item 3 claimed `test_one_diff_scoped_checker_per_gate_step` held "nothing fallible precedes the checker in its body". Both halves were false: MW put a `git fetch` before `check-now-current.py` and MV put one in the inline-shell gate step, and both gave `Ran 99 / OK`. | The property is held rather than claimed. §4.3 item 3. | +| F4 | **Medium.** `.agents/issue-index.md`'s row -- added by this pull request -- stated as fact the two claims the pull request had retracted, and §3.5's reason for leaving it was wrong. | The row is corrected and §3.5 records why editing it is legitimate and why this was the last moment. §3.5. | +| F5 | `check-commit-trailers.py`'s landed-exception rationale argued from `LAST_GREEN` advancing only on GREEN, which this row makes false. | Rewritten in place, with the old argument quoted so it is not re-derived, and #1262 named. §1. | +| F6 | `ci.yml:73-76` still described the base as the last SUCCESSFULLY gated commit via `last-gated-commit`; `ci-concurrency.md` §Design described the superseded rule and cited a symbol `26def4c8f` renamed away. | Both corrected. The stale citation is a bare backticked name with no `path::Symbol` form, which is why `scripts/check-symbol-anchors.py` cannot see it -- noted below rather than fixed here. | +| F7 | **Latent.** With one push run in the window `runs[-1]` is `runs[0]`, so the anchor was the head being pushed and the range was empty: a vacuous pass that CONCLUDES. Unreachable here, reachable on a fork's first push. | Fixed, and the residual short-history bound is stated with a test. §4.2. | +| F8 | §4.2's pre-fix narration was off by one. | Corrected. §4.2. | + +**A gap in `check-symbol-anchors.py`, noted and not repaired here.** It resolves +citations written as `path::Symbol`. `ci-concurrency.md` cited +`test_every_diff_scoped_step_bases_on_the_last_gated_commit` as a bare backticked +name with no path, so the rename in `26def4c8f` left a citation that named +nothing and no gate could see it. Teaching the checker to resolve bare symbol +names is a change to checker semantics and needs its own row, spec and +red-before evidence; doing it inside a review repair is the bypass AGENTS.md +names. Filed rather than fixed. + +## 11. The THIRD fresh review on PR #1776, and what it changed + +The third review confirmed the tree at `b7557fe9d` as CORRECT: none of the five +gate bodies contains a hazardous shape, and it verified the guards, the payload +shapes, the floor, reachability, the records and the preflight without finding a +defect in them. Both of its findings are that a TEST and a RECORD assert a +completeness they do not hold. That is the same class as F1 and F3 before them, +one layer further in, and it is worth naming: three rounds running, the defect +has been the description of the guarantee rather than the guarantee. + +| # | Finding | Repair | +|---|---|---| +| F-A | **Medium-high.** `test_nothing_fallible_PRECEDES_the_gate_in_its_own_body` had THREE escapes past the one residual its docstring stated. `commands_in` dropped every `name=...` statement before the `$(`/backtick refusal ran, so an assignment carrying a substitution was invisible; and `[` was allowlisted on a justification the parser destroyed, since it splits on `||` and discards it. Four injections, `Ran 109 / OK` on each. | `statements_in` returns bindings and keeps the following operator. The refusal covers every statement before the gate; `[` must be consumption-checked. Both residuals are now stated on the test. §4.3 item 3, §8. | +| F-B | **Medium.** `AGENTS.md` asserted as fact the `commit_title` mechanism that this spec and the index row retract, and named `localai-bot` as the cause. It is the one file every agent loads, so it would be quoted forward as measured. | The RULE is unchanged and still actionable. The causal and the account attribution are hedged, and the reader is pointed here for the evidence and the two facts that weaken it. | +| F-C | `§8` said "four gate-step guards" where the same section says five. Measured from `ci.yml`: `agent-record` 1, `documentation-checkpoint` 2, `commit-protocol-tag` 2. | Corrected to five. | +| F-D | `baseline_runs` was entered by none of the tests and reported an unreadable forge to a human as a clean absence, which `.agents/verification.md` forbids. Advisory lane, so the rc was never wrong. | Fixed rather than owed, mirroring `jobs_for`. `BaselineRunsPayloadTests` executes it for the first time. §8. | +| F-E | `test_the_checker_resolves_no_ref_to_decide_arrival` forbade `origin/`, `ls-remote` and `for-each-ref` in the RAW source, so a future comment mentioning any of them would red it with a message about a ref lookup. | `executable_source` blanks comments and docstrings and keeps string literals, which is where a real lookup lives. The message now names the line instead of printing the file. | +| F-F | 24 added lines in this spec carried em dashes, which `.agents/style/prose.md` forbids and which bind new prose. | Converted. The file is at 0, as `AGENTS.md` already was. | + +**One judgement recorded rather than left silent.** F-D was fixed in flow instead +of being listed under `## Owed`. It is the same class as F2, in the same file, +found by the same question, and AGENTS.md's in-flow rule covers a small and clear +fix. It changes no checker semantics: it makes one reader refuse the shapes the +reader beside it already refuses. + +## 12. Owed + +- [#1787](https://github.com/mudler/vllm.cpp/issues/1787) -- `check-symbol-anchors.py` + cannot see a bare backticked symbol citation, which is why `26def4c8f`'s rename + left `.agents/specs/ci-concurrency.md` naming a symbol that does not exist and + no gate reported it. The stale citation is repaired in this pull request; the + checker gap is not, because resolving a bare identifier changes checker + semantics and needs its own row, spec and red-before evidence. §10 argues it. + +## 13. Stop conditions + +- Stop if `test_no_commit_is_ever_skipped` cannot be made to hold. Escaping the + cycle by skipping commits is #863 again and is worse than the cycle. +- Stop if any rule in `check-role-discipline.py` has to move to make the range + work. §1 says the range changes and the demands do not. +- Stop if the anchor query costs more than a bounded number of API calls per + job; an unbounded walk is a new failure mode, not a fix. +- Stop and return `NEEDS_DECISION` if the "concluded, not green" argument in + §4.1 is rejected, because every other part of the fix depends on it. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 51e6f7e18..352bff2ae 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -70,10 +70,19 @@ on: # # This was previously unsafe and is now safe. It was unsafe because a cancelled # run's diff-scoped range was never re-covered: the next run's `before` is this -# run's `sha`. The gates now walk from the last SUCCESSFULLY gated commit -# (`last-gated-commit`), so a cancelled run is lossless -- the next run walks a -# wider range and reports the same red. Reverting that base to `before` without -# reverting this reintroduces #863. +# run's `sha`. Each diff-scoped gate STEP now walks from the newest push run in +# which that STEP returned a verdict, resolved by its own job (#1773), so a +# cancelled run is lossless -- the next run walks a wider range and reports the +# same red. Reverting that base to `before` without reverting this reintroduces +# #863. +# +# 2026-08-23 (#1773): BOTH HALVES OF THE SENTENCE ABOVE USED TO BE FALSE. It +# said "the last SUCCESSFULLY gated commit (`last-gated-commit`)". The base is +# not the last SUCCESSFUL one -- it advances past a CONCLUDED step, failure +# included, because `main` is immutable and anchoring on success converts one +# landed violation into a permanent block on every later push. And it no longer +# comes from `last-gated-commit`, which resolves nothing now and keeps only the +# closed-pull-request guard `agent-record` inherits through `needs:` (#873). # # `schedule` and `workflow_dispatch` keep their own partition through the # event_name token and stay non-cancellable, so the baseline lane (#274) is @@ -148,15 +157,96 @@ jobs: # implicit `success()` couples this job to `last-gated-commit` SUCCEEDING, # not merely running, while `documentation-checkpoint` and # `commit-protocol-tag` on the same `needs:` keep `always()` and run - # regardless. A failure in the sha resolver therefore silences this job's - # checkers rather than reding them -- which is why that job does nothing but - # resolve a string and falls back instead of failing. + # regardless. A failure in that job therefore silences this job's checkers + # rather than reding them -- which is why it now does nothing at all. Since + # #1773 it resolves no value and makes no network call, so the only thing + # left that can fail there is the runner itself; each gate resolves its own + # anchor in its own first step, where a failed query degrades the range + # rather than skipping the gate. needs: [last-gated-commit] runs-on: ubuntu-latest + permissions: + actions: read + contents: read steps: - uses: actions/checkout@v4 + id: checkout with: fetch-depth: 0 + - name: Resolve this job's own diff anchor (#1773) + id: anchor + # The base each gate below walks is "the commit after which THAT STEP + # has not yet returned a verdict", and it is resolved HERE, by the job + # that walks it. + # + # The unit is the STEP. A job-level question re-opens #863 one level + # down: GitHub concludes a job `failure` the moment any step fails and + # marks every REMAINING step `skipped`, so "the job concluded" reads + # `failure` over a gate that never executed. Measured on + # `commit-protocol-tag`, runs 32601353990 through 32623377380: job + # `failure` five pushes running, strict-trailer step `skipped` every + # time, and a job-level anchor walks `038ff61e5..a4f2a9585` -- six + # commits -- straight past. `--gate-step` is required for that reason. + # + # It used to be one shared string from `...&status=success`, which was + # wrong twice: `status=` filters on the RUN's conclusion, which reads + # success over a red `continue-on-error` job (#274) and `cancelled` + # over a job that ran to completion -- and because a successful run was + # exactly what a red gate blocked, the anchor froze while the range + # widened on every push, re-flagging the commits blocking it (#1773). + # + # Resolving it in a SEPARATE job was the second defect. A gate carrying + # `if: always()` outlives its own resolver: on run 32625264281 + # `last-gated-commit` was cancelled, the shared string rendered empty, + # and `documentation-checkpoint` fell back to `PUSH_BASE` and passed + # over ONE push, while the same gate walked 484 commits and failed + # whenever the resolver survived. One job, one anchor, no race. + # + # REMOTE_UNVERIFIED is not a pass, and since #1776 it is not one + # operationally either. A degraded query used to be swallowed by + # `|| true`, leaving an empty anchor that the gate silently replaced + # with `PUSH_BASE` -- a narrower range, a green step, and an anchor + # advanced past everything the narrowing dropped. The gate steps now + # carry `env.GATE_ANCHOR_DEGRADED != 'true'`, so an unreadable forge + # SKIPS them. A skipped step is not a verdict, the anchor cannot + # advance, and the next readable run walks the whole span. + env: + GH_TOKEN: ${{ github.token }} + GITHUB_REPOSITORY: ${{ github.repository }} + EVENT_NAME: ${{ github.event_name }} + BRANCH: ${{ github.ref_name }} + run: | + set -eu + anchor="" + degraded="" + if [ "$EVENT_NAME" = "push" ]; then + # rc 3 is REMOTE_UNVERIFIED and rc 1 is a clean absence, the split + # AGENTS.md documents for `scripts/agent-pr-body.py`. They may NOT + # be collapsed with `|| true`: on rc 1 the branch genuinely has no + # gated history and `PUSH_BASE` is the honest base, while on rc 3 + # the base is UNKNOWN and falling back to `PUSH_BASE` would run a + # narrowed pass whose success then advances this step's own anchor + # past every commit the narrowing dropped. + set +e + anchor="$(python3 scripts/main-baseline.py \ + --gate-anchor agent-record --branch "$BRANCH" \ + --gate-step "Agent role machinery and role discipline" \ + )" + rc=$? + set -e + if [ "$rc" -eq 3 ]; then + anchor="" + degraded=true + echo "::warning::REMOTE_UNVERIFIED resolving agent-record's diff anchor." + echo "::warning::Its diff-scoped steps SKIP rather than pass over a" + echo "::warning::narrower range. The anchor does not advance, so the" + echo "::warning::next readable run walks this span whole." + elif [ "$rc" -ne 0 ]; then + anchor="" + fi + fi + echo "GATE_ANCHOR=$anchor" >> "$GITHUB_ENV" + echo "GATE_ANCHOR_DEGRADED=$degraded" >> "$GITHUB_ENV" - name: Canonical roadmap tables and links are consistent # --report, wired the way check-device-leakage's is below: the record # anchor ratchet (ENG-RECORD-ANCHOR-RATCHET, #632) gates on a baseline, @@ -478,6 +568,18 @@ jobs: # time can catch it; this is the only place it can be caught. run: python3 tests/scripts/test_gpu_lock_one_truth.py - name: Agent role machinery and role discipline + # SKIPS, never narrows. `!cancelled()` because an earlier step's + # failure must not turn this gate into a `skipped` that the anchor + # then walks past -- that is exactly the hole this row closes, and + # this step's own conclusion IS the anchor's verdict. The three + # preconditions are the cases where a conclusion would be a lie: + # no checkout means no history to walk, a failed resolver means no + # base, and a degraded query means an UNKNOWN base. In each, this + # step is skipped, the anchor does not advance, and the span is + # walked whole by the next run that can answer. + if: ${{ !cancelled() && steps.checkout.outcome == 'success' + && steps.anchor.outcome == 'success' + && env.GATE_ANCHOR_DEGRADED != 'true' }} # Same split as the step above: the two suites are tree-scoped and run # on every lane; `check-role-discipline.py --base/--head` is DIFF-scoped # and is skipped where there is no range. @@ -493,7 +595,7 @@ jobs: PR_BASE: ${{ github.event.pull_request.base.sha }} PR_HEAD: ${{ github.event.pull_request.head.sha }} PUSH_BASE: ${{ github.event.before }} - LAST_GREEN: ${{ needs.last-gated-commit.outputs.sha }} + GATE_ANCHOR: ${{ env.GATE_ANCHOR }} PUSH_HEAD: ${{ github.sha }} run: | set -eu @@ -503,11 +605,14 @@ jobs: head="$PR_HEAD" pending_args=(--pending-pr-head "$PR_HEAD") else - # The last SUCCESSFULLY gated commit, not the previous push: a - # cancelled run must not advance the base, or its commits are - # skipped forever. That is what lets the push lane be latest-only - # (#822, #863). Falls back to `before`, today's behaviour. - base="${LAST_GREEN:-}" + # The commit after which THIS job has not yet returned a + # verdict, resolved by this job's own anchor step above. Not the + # previous push: a cancelled run must not advance the base, or its + # commits are skipped forever (#822, #863). Not the last SUCCESSFUL + # RUN either: that froze the anchor for eleven days and 484 commits + # while the range widened on every push (#1773). Falls back to + # `before`, today's behaviour, when the query finds nothing. + base="${GATE_ANCHOR:-}" [ -n "$base" ] || base="$PUSH_BASE" head="$PUSH_HEAD" fi @@ -584,17 +689,108 @@ jobs: if: always() && github.event.action != 'closed' && (github.event_name != 'schedule' && github.event_name != 'workflow_dispatch') needs: [last-gated-commit] runs-on: ubuntu-latest + permissions: + actions: read + contents: read steps: - uses: actions/checkout@v4 + id: checkout with: fetch-depth: 0 + - name: Resolve this job's own diff anchor (#1773) + id: anchor + # The base each gate below walks is "the commit after which THAT STEP + # has not yet returned a verdict", and it is resolved HERE, by the job + # that walks it. + # + # The unit is the STEP. A job-level question re-opens #863 one level + # down: GitHub concludes a job `failure` the moment any step fails and + # marks every REMAINING step `skipped`, so "the job concluded" reads + # `failure` over a gate that never executed. Measured on + # `commit-protocol-tag`, runs 32601353990 through 32623377380: job + # `failure` five pushes running, strict-trailer step `skipped` every + # time, and a job-level anchor walks `038ff61e5..a4f2a9585` -- six + # commits -- straight past. `--gate-step` is required for that reason. + # + # It used to be one shared string from `...&status=success`, which was + # wrong twice: `status=` filters on the RUN's conclusion, which reads + # success over a red `continue-on-error` job (#274) and `cancelled` + # over a job that ran to completion -- and because a successful run was + # exactly what a red gate blocked, the anchor froze while the range + # widened on every push, re-flagging the commits blocking it (#1773). + # + # Resolving it in a SEPARATE job was the second defect. A gate carrying + # `if: always()` outlives its own resolver: on run 32625264281 + # `last-gated-commit` was cancelled, the shared string rendered empty, + # and `documentation-checkpoint` fell back to `PUSH_BASE` and passed + # over ONE push, while the same gate walked 484 commits and failed + # whenever the resolver survived. One job, one anchor, no race. + # + # REMOTE_UNVERIFIED is not a pass, and since #1776 it is not one + # operationally either. A degraded query used to be swallowed by + # `|| true`, leaving an empty anchor that the gate silently replaced + # with `PUSH_BASE` -- a narrower range, a green step, and an anchor + # advanced past everything the narrowing dropped. The gate steps now + # carry `env.GATE_ANCHOR_DEGRADED != 'true'`, so an unreadable forge + # SKIPS them. A skipped step is not a verdict, the anchor cannot + # advance, and the next readable run walks the whole span. + env: + GH_TOKEN: ${{ github.token }} + GITHUB_REPOSITORY: ${{ github.repository }} + EVENT_NAME: ${{ github.event_name }} + BRANCH: ${{ github.ref_name }} + run: | + set -eu + anchor="" + degraded="" + if [ "$EVENT_NAME" = "push" ]; then + # rc 3 is REMOTE_UNVERIFIED and rc 1 is a clean absence, the split + # AGENTS.md documents for `scripts/agent-pr-body.py`. They may NOT + # be collapsed with `|| true`: on rc 1 the branch genuinely has no + # gated history and `PUSH_BASE` is the honest base, while on rc 3 + # the base is UNKNOWN and falling back to `PUSH_BASE` would run a + # narrowed pass whose success then advances this step's own anchor + # past every commit the narrowing dropped. + set +e + anchor="$(python3 scripts/main-baseline.py \ + --gate-anchor documentation-checkpoint --branch "$BRANCH" \ + --gate-step "Every feature checkpoint updates STATUS, BENCHMARKS and NOW" \ + --gate-step "Every commit in the range arrived on a task branch" \ + )" + rc=$? + set -e + if [ "$rc" -eq 3 ]; then + anchor="" + degraded=true + echo "::warning::REMOTE_UNVERIFIED resolving documentation-checkpoint's diff anchor." + echo "::warning::Its diff-scoped steps SKIP rather than pass over a" + echo "::warning::narrower range. The anchor does not advance, so the" + echo "::warning::next readable run walks this span whole." + elif [ "$rc" -ne 0 ]; then + anchor="" + fi + fi + echo "GATE_ANCHOR=$anchor" >> "$GITHUB_ENV" + echo "GATE_ANCHOR_DEGRADED=$degraded" >> "$GITHUB_ENV" - name: Every feature checkpoint updates STATUS, BENCHMARKS and NOW + # SKIPS, never narrows. `!cancelled()` because an earlier step's + # failure must not turn this gate into a `skipped` that the anchor + # then walks past -- that is exactly the hole this row closes, and + # this step's own conclusion IS the anchor's verdict. The three + # preconditions are the cases where a conclusion would be a lie: + # no checkout means no history to walk, a failed resolver means no + # base, and a degraded query means an UNKNOWN base. In each, this + # step is skipped, the anchor does not advance, and the span is + # walked whole by the next run that can answer. + if: ${{ !cancelled() && steps.checkout.outcome == 'success' + && steps.anchor.outcome == 'success' + && env.GATE_ANCHOR_DEGRADED != 'true' }} env: EVENT_NAME: ${{ github.event_name }} PR_BASE: ${{ github.event.pull_request.base.sha }} PR_HEAD: ${{ github.event.pull_request.head.sha }} PUSH_BASE: ${{ github.event.before }} - LAST_GREEN: ${{ needs.last-gated-commit.outputs.sha }} + GATE_ANCHOR: ${{ env.GATE_ANCHOR }} PUSH_HEAD: ${{ github.sha }} run: | set -eu @@ -602,15 +798,69 @@ jobs: base="$PR_BASE" head="$PR_HEAD" else - # The last SUCCESSFULLY gated commit, not the previous push: a - # cancelled run must not advance the base, or its commits are - # skipped forever. That is what lets the push lane be latest-only - # (#822, #863). Falls back to `before`, today's behaviour. - base="${LAST_GREEN:-}" + # The commit after which THIS STEP has not yet returned a verdict, + # resolved by this job's own anchor step above. Not the previous + # push: a cancelled run must not advance the base, or its commits + # are skipped forever (#822, #863). Not the last SUCCESSFUL RUN + # either: that froze the anchor for eleven days and 484 commits + # while the range widened on every push (#1773). Falls back to + # `before` when the branch has no gated history at all; a DEGRADED + # query does not reach here, because this step is skipped instead. + base="${GATE_ANCHOR:-}" [ -n "$base" ] || base="$PUSH_BASE" head="$PUSH_HEAD" fi python3 scripts/check-now-current.py --base "$base" --head "$head" + - name: Every commit in the range arrived on a task branch + # SPLIT OUT of the step above (#1776). It used to be its second command + # under `set -eu`, which made the step's conclusion a lie about it: a + # `check-now-current` failure aborted the step BEFORE this checker ran, + # while the step still concluded, so a step-level anchor advanced past + # commits whose arrival nothing had checked. `documentation-checkpoint` + # is the job where that hazard lived INSIDE one step rather than + # between two, and one gate per step is the shape that removes it. + # + # Redundant with `agent-record`'s copy of this checker, and DELIBERATELY + # so: the two jobs have different cancellation profiles, and until this + # row that redundancy was the only thing covering agent-record's own + # skipped-step hole. Luck is not construction, so both are now anchored. + # SKIPS, never narrows. `!cancelled()` because an earlier step's + # failure must not turn this gate into a `skipped` that the anchor + # then walks past -- that is exactly the hole this row closes, and + # this step's own conclusion IS the anchor's verdict. The three + # preconditions are the cases where a conclusion would be a lie: + # no checkout means no history to walk, a failed resolver means no + # base, and a degraded query means an UNKNOWN base. In each, this + # step is skipped, the anchor does not advance, and the span is + # walked whole by the next run that can answer. + if: ${{ !cancelled() && steps.checkout.outcome == 'success' + && steps.anchor.outcome == 'success' + && env.GATE_ANCHOR_DEGRADED != 'true' }} + env: + EVENT_NAME: ${{ github.event_name }} + PR_BASE: ${{ github.event.pull_request.base.sha }} + PR_HEAD: ${{ github.event.pull_request.head.sha }} + PUSH_BASE: ${{ github.event.before }} + GATE_ANCHOR: ${{ env.GATE_ANCHOR }} + PUSH_HEAD: ${{ github.sha }} + run: | + set -eu + if [ "$EVENT_NAME" = "pull_request" ]; then + base="$PR_BASE" + head="$PR_HEAD" + else + # The commit after which THIS STEP has not yet returned a verdict, + # resolved by this job's own anchor step above. Not the previous + # push: a cancelled run must not advance the base, or its commits + # are skipped forever (#822, #863). Not the last SUCCESSFUL RUN + # either: that froze the anchor for eleven days and 484 commits + # while the range widened on every push (#1773). Falls back to + # `before` when the branch has no gated history at all; a DEGRADED + # query does not reach here, because this step is skipped instead. + base="${GATE_ANCHOR:-}" + [ -n "$base" ] || base="$PUSH_BASE" + head="$PUSH_HEAD" + fi pending_args=() if [ "$EVENT_NAME" = "pull_request" ]; then pending_args=(--pending-pr-head "$PR_HEAD") @@ -618,60 +868,42 @@ jobs: python3 scripts/check-role-discipline.py \ --base "$base" --head "$head" "${pending_args[@]}" last-gated-commit: - # The head of the most recent SUCCESSFUL push run of this workflow on this - # branch. It is the base the diff-scoped gates walk from, instead of - # `github.event.before`. - # - # `before` is the previous push's sha whether or not that push was gated, so - # a cancelled run's commits are skipped and NOTHING re-covers them -- the - # reason a superseded main run could not be cancelled at all (#822, #863). - # Basing on the last GREEN commit makes a cancelled run lossless: the next - # run simply walks a wider range. + # A GUARD, and since #1773 nothing else. The name is kept because + # `agent-record` inherits its closed-pull-request skip through `needs:` on + # this job, and `agent-record` may not carry an `if:` of its own (#873). # - # Carries NO concurrency group, like the gates that consume it. + # It USED to resolve one shared base for every diff-scoped gate, from + # `...&event=push&status=success&per_page=1`. That query was wrong twice + # over, and #1773 records both: `status=` filters on the RUN's conclusion, + # which reads success over a red `continue-on-error` job (#274) and + # `cancelled` over a job that ran to completion; and because a successful + # run was exactly what a red gate blocked, the anchor froze while the range + # widened on every push. Measured at `21abaf169`: 484 commits, anchored + # eleven days back, re-flagging five commits from 2026-08-18 on every push. # - # This is workflow plumbing resolving an input, not a checker. The - # protocol's ban on network calls is about a CHECKER failing on - # connectivity; this falls back to `before` when the query returns nothing, - # so a failed query is the status quo rather than a skipped gate. + # Worse, a SHARED base resolved in a SEPARATE job is a race. The consumers + # carry `if: always()`, so they outlive this job's cancellation: on run + # 32625264281 this job was cancelled, the shared string rendered empty, and + # `documentation-checkpoint` fell back to `PUSH_BASE` and PASSED over one + # push -- the same gate that fails over 484 commits whenever this job + # survives. Each gate now resolves its OWN anchor, in its own first step, + # from the last push run in which THAT job returned a verdict. # - # It runs on EVERY lane except a closed pull request, and resolves a sha - # only on the push lane -- there is no gated `main` run to find for a pull - # request or a schedule. The wider condition is deliberate: `agent-record` + # It runs on EVERY lane except a closed pull request. The wider condition is + # deliberate: `agent-record` # may not carry an `if:` of its own without un-registering three checkers # (#873), so this job's condition is the closed-PR guard it inherits through # `needs:`. A skipped dependency skips its dependents, which is the same # skip #822 asked for, expressed where a checker can still see the gate. if: github.event.action != 'closed' runs-on: ubuntu-latest - permissions: - actions: read - contents: read - outputs: - sha: ${{ steps.resolve.outputs.sha }} steps: - - id: resolve - env: - GH_TOKEN: ${{ github.token }} - REPO: ${{ github.repository }} - BRANCH: ${{ github.ref_name }} - EVENT_NAME: ${{ github.event_name }} + - name: The closed-pull-request guard, and nothing else run: | - set -eu - # Only the push lane has a gated `main` history to resolve against. - # Every other lane leaves this EMPTY and its consumers fall back, - # which is what they already do when the query finds nothing. - sha="" - if [ "$EVENT_NAME" = "push" ]; then - sha="$(gh api \ - "repos/$REPO/actions/workflows/ci.yml/runs?branch=$BRANCH&event=push&status=success&per_page=1" \ - --jq '.workflow_runs[0].head_sha' 2>/dev/null || true)" - fi - # NOT `[ ... ] && sha=""`: that returns non-zero when the test is - # false, which aborts the step under `set -e`. - if [ "$sha" = "null" ]; then sha=""; fi - echo "last successfully gated commit: ${sha:-}" - echo "sha=$sha" >> "$GITHUB_OUTPUT" + echo "This job resolves no value. It exists so that agent-record can" + echo "carry the closed-pull-request skip through \`needs:\` without an" + echo "\`if:\` of its own, which would un-register three checkers (#873)." + echo "Each diff-scoped gate now resolves its OWN anchor (#1773)." commit-protocol-tag: # Gate: every NEW commit must carry the FOLLOWING_AGENTS_PROTOCOL trailer, # asserting the contributor read AGENTS.md. See .agents/ai-coding-assistants.md. @@ -685,21 +917,118 @@ jobs: needs: [last-gated-commit] if: always() && github.event.action != 'closed' && github.event_name != 'schedule' && github.event_name != 'workflow_dispatch' runs-on: ubuntu-latest + permissions: + actions: read + contents: read steps: - uses: actions/checkout@v4 + id: checkout with: fetch-depth: 0 + - name: Resolve this job's own diff anchor (#1773) + id: anchor + # The base each gate below walks is "the commit after which THAT STEP + # has not yet returned a verdict", and it is resolved HERE, by the job + # that walks it. + # + # The unit is the STEP. A job-level question re-opens #863 one level + # down: GitHub concludes a job `failure` the moment any step fails and + # marks every REMAINING step `skipped`, so "the job concluded" reads + # `failure` over a gate that never executed. Measured on + # `commit-protocol-tag`, runs 32601353990 through 32623377380: job + # `failure` five pushes running, strict-trailer step `skipped` every + # time, and a job-level anchor walks `038ff61e5..a4f2a9585` -- six + # commits -- straight past. `--gate-step` is required for that reason. + # + # It used to be one shared string from `...&status=success`, which was + # wrong twice: `status=` filters on the RUN's conclusion, which reads + # success over a red `continue-on-error` job (#274) and `cancelled` + # over a job that ran to completion -- and because a successful run was + # exactly what a red gate blocked, the anchor froze while the range + # widened on every push, re-flagging the commits blocking it (#1773). + # + # Resolving it in a SEPARATE job was the second defect. A gate carrying + # `if: always()` outlives its own resolver: on run 32625264281 + # `last-gated-commit` was cancelled, the shared string rendered empty, + # and `documentation-checkpoint` fell back to `PUSH_BASE` and passed + # over ONE push, while the same gate walked 484 commits and failed + # whenever the resolver survived. One job, one anchor, no race. + # + # REMOTE_UNVERIFIED is not a pass, and since #1776 it is not one + # operationally either. A degraded query used to be swallowed by + # `|| true`, leaving an empty anchor that the gate silently replaced + # with `PUSH_BASE` -- a narrower range, a green step, and an anchor + # advanced past everything the narrowing dropped. The gate steps now + # carry `env.GATE_ANCHOR_DEGRADED != 'true'`, so an unreadable forge + # SKIPS them. A skipped step is not a verdict, the anchor cannot + # advance, and the next readable run walks the whole span. + env: + GH_TOKEN: ${{ github.token }} + GITHUB_REPOSITORY: ${{ github.repository }} + EVENT_NAME: ${{ github.event_name }} + BRANCH: ${{ github.ref_name }} + run: | + set -eu + anchor="" + degraded="" + if [ "$EVENT_NAME" = "push" ]; then + # rc 3 is REMOTE_UNVERIFIED and rc 1 is a clean absence, the split + # AGENTS.md documents for `scripts/agent-pr-body.py`. They may NOT + # be collapsed with `|| true`: on rc 1 the branch genuinely has no + # gated history and `PUSH_BASE` is the honest base, while on rc 3 + # the base is UNKNOWN and falling back to `PUSH_BASE` would run a + # narrowed pass whose success then advances this step's own anchor + # past every commit the narrowing dropped. + set +e + anchor="$(python3 scripts/main-baseline.py \ + --gate-anchor commit-protocol-tag --branch "$BRANCH" \ + --gate-step "Every new commit carries FOLLOWING_AGENTS_PROTOCOL" \ + --gate-step "Every new commit satisfies the strict trailer contract" \ + )" + rc=$? + set -e + if [ "$rc" -eq 3 ]; then + anchor="" + degraded=true + echo "::warning::REMOTE_UNVERIFIED resolving commit-protocol-tag's diff anchor." + echo "::warning::Its diff-scoped steps SKIP rather than pass over a" + echo "::warning::narrower range. The anchor does not advance, so the" + echo "::warning::next readable run walks this span whole." + elif [ "$rc" -ne 0 ]; then + anchor="" + fi + fi + echo "GATE_ANCHOR=$anchor" >> "$GITHUB_ENV" + echo "GATE_ANCHOR_DEGRADED=$degraded" >> "$GITHUB_ENV" - name: Every new commit carries FOLLOWING_AGENTS_PROTOCOL + # SKIPS, never narrows. `!cancelled()` because an earlier step's + # failure must not turn this gate into a `skipped` that the anchor + # then walks past -- that is exactly the hole this row closes, and + # this step's own conclusion IS the anchor's verdict. The three + # preconditions are the cases where a conclusion would be a lie: + # no checkout means no history to walk, a failed resolver means no + # base, and a degraded query means an UNKNOWN base. In each, this + # step is skipped, the anchor does not advance, and the span is + # walked whole by the next run that can answer. + if: ${{ !cancelled() && steps.checkout.outcome == 'success' + && steps.anchor.outcome == 'success' + && env.GATE_ANCHOR_DEGRADED != 'true' }} env: - LAST_GREEN: ${{ needs.last-gated-commit.outputs.sha }} + GATE_ANCHOR: ${{ env.GATE_ANCHOR }} run: | set -eu if [ "${{ github.event_name }}" = "pull_request" ]; then base="${{ github.event.pull_request.base.sha }}" head="${{ github.event.pull_request.head.sha }}" else - # Same self-healing base as the strict walk below (#822, #863). - base="${LAST_GREEN:-}" + # The commit after which THIS job has not yet returned a + # verdict, resolved by this job's own anchor step above. Not the + # previous push: a cancelled run must not advance the base, or its + # commits are skipped forever (#822, #863). Not the last SUCCESSFUL + # RUN either: that froze the anchor for eleven days and 484 commits + # while the range widened on every push (#1773). Falls back to + # `before`, today's behaviour, when the query finds nothing. + base="${GATE_ANCHOR:-}" [ -n "$base" ] || base="${{ github.event.before }}" head="${{ github.sha }}" fi @@ -729,6 +1058,18 @@ jobs: fi echo "OK: all new commits carry FOLLOWING_AGENTS_PROTOCOL." - name: Every new commit satisfies the strict trailer contract + # SKIPS, never narrows. `!cancelled()` because an earlier step's + # failure must not turn this gate into a `skipped` that the anchor + # then walks past -- that is exactly the hole this row closes, and + # this step's own conclusion IS the anchor's verdict. The three + # preconditions are the cases where a conclusion would be a lie: + # no checkout means no history to walk, a failed resolver means no + # base, and a degraded query means an UNKNOWN base. In each, this + # step is skipped, the anchor does not advance, and the span is + # walked whole by the next run that can answer. + if: ${{ !cancelled() && steps.checkout.outcome == 'success' + && steps.anchor.outcome == 'success' + && env.GATE_ANCHOR_DEGRADED != 'true' }} # DIFF-scoped, and therefore HERE rather than in agent-record. It lived # in that job until #863: agent-record carries a cancellable group keyed # on `github.ref`, which for a push is the constant `refs/heads/main`, so @@ -742,7 +1083,7 @@ jobs: PR_BASE: ${{ github.event.pull_request.base.sha }} PR_HEAD: ${{ github.event.pull_request.head.sha }} PUSH_BASE: ${{ github.event.before }} - LAST_GREEN: ${{ needs.last-gated-commit.outputs.sha }} + GATE_ANCHOR: ${{ env.GATE_ANCHOR }} PUSH_HEAD: ${{ github.sha }} # Passed as an ENV VAR, never interpolated into the script: a body is # attacker-controlled text on a fork pull request. @@ -753,12 +1094,14 @@ jobs: base="$PR_BASE" head="$PR_HEAD" else - # The last SUCCESSFULLY gated commit, not the previous push. A - # cancelled or failed run must not advance the base, or its commits - # are skipped forever -- that is the whole reason a superseded main - # run could not be cancelled before (#822, #863). Falls back to - # `before`, which is the previous behaviour, when nothing is found. - base="${LAST_GREEN:-}" + # The commit after which THIS job has not yet returned a + # verdict, resolved by this job's own anchor step above. Not the + # previous push: a cancelled run must not advance the base, or its + # commits are skipped forever (#822, #863). Not the last SUCCESSFUL + # RUN either: that froze the anchor for eleven days and 484 commits + # while the range widened on every push (#1773). Falls back to + # `before`, today's behaviour, when the query finds nothing. + base="${GATE_ANCHOR:-}" [ -n "$base" ] || base="$PUSH_BASE" head="$PUSH_HEAD" fi diff --git a/AGENTS.md b/AGENTS.md index 8597f3437..fa4614d48 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -546,6 +546,42 @@ is never a pass. The command is a belt to the CI guard's braces and not a replacement for it: the forge reads the body again from its own event payload, which is what catches an edit made after you looked. +**Land a squash under its DEFAULT title.** GitHub appends `(#N)` to it, and that +number is the only evidence `scripts/check-role-discipline.py` has that the +change arrived on a task branch. The checker resolves no ref and reads nothing +but the commit message. An explicit `commit_title` is the documented way to +suppress that append, and a squash that lands without the number looks exactly +like a direct push to `main`, on a commit nobody can retroactively repair. + +Five commits from 2026-08-18 landed without the number, four of them from +external contributors +([#1773](https://github.com/mudler/vllm.cpp/issues/1773)). The fork is not the +problem and never was. The missing number is what the checker sees. +**Why it was missing on those five is INFERRED, and this repository cannot +measure it.** The pulls endpoint keeps no merge-input payload, so nothing +records whether those merge calls carried a `commit_title` at all. +[`.agents/specs/gate-anchor-per-job.md`](.agents/specs/gate-anchor-per-job.md) +holds the evidence and the two facts that weaken the hypothesis. Do not quote +the mechanism as measured. + +In practice this is one flag, and the rule stands whatever suppressed the +append. Do not pass `--subject` to `gh pr merge --squash`. Do not send a +`commit_title` field to `PUT /repos/OWNER/REPO/pulls/NUMBER/merge`. Omit both +and the default title wins. **The rule binds whoever performs the MERGE, not +the author**, because the merging side chooses the title, whether that is a +script calling the API or a human editing the title box in the merge dialog. +All five of those merges were performed by `localai-bot`, which appends the +number correctly on every other squash it lands. That account is therefore not +an explanation on its own. Its automation is one place to check, not a known +cause. + +**No gate can catch this one.** The evidence is destroyed at the moment of the +merge, the commit is immutable once written, and a checker reading `main` +afterwards sees a subject with no number and cannot tell it from a direct push. +That is why the rule is written here rather than added to a checker, and +[`.agents/specs/gate-anchor-per-job.md`](.agents/specs/gate-anchor-per-job.md) +records the decision not to weaken `check-role-discipline.py` to compensate. + Every commit contains a bare `FOLLOWING_AGENTS_PROTOCOL` paragraph and these trailers: diff --git a/scripts/check-commit-trailers.py b/scripts/check-commit-trailers.py index b785276f5..b8dae0f46 100755 --- a/scripts/check-commit-trailers.py +++ b/scripts/check-commit-trailers.py @@ -386,11 +386,27 @@ class LandedException(NamedTuple): # EXCEPTIONS FOR MESSAGES THAT HAVE ALREADY LANDED. Visible debt, not success. # # A malformed message on `main` cannot be repaired: correcting it rewrites -# `main`, which nobody may do. It also does not clear itself. The main lane -# walks `LAST_GREEN..head` and `LAST_GREEN` advances only on a GREEN run, so a -# range containing an unrepairable red is re-walked by every later push, by -# every session, forever. `ci.yml:74` relies on exactly that property to make a -# cancelled run lossless -- the same property makes this red permanent. +# `main`, which nobody may do. That is the whole reason these entries exist, and +# it is independent of how the push lane picks its range. +# +# 2026-08-23 (#1773): THE SECOND HALF OF THIS RATIONALE IS NO LONGER TRUE, and +# it is corrected rather than deleted because the difference is what makes the +# entries below still necessary. It used to read: "The main lane walks +# `LAST_GREEN..head` and `LAST_GREEN` advances only on a GREEN run, so a range +# containing an unrepairable red is re-walked by every later push, by every +# session, forever. `ci.yml:74` relies on exactly that property." It did, and +# that property was the defect: a red that could never be repaired blocked every +# later push and drowned the gate's report on new commits. The anchor now +# advances past a CONCLUDED step, failure included, so an unrepairable red is +# reported ONCE and then leaves the range. +# +# WITHOUT THESE ENTRIES THAT WOULD STILL BE A RED ONCE PER MESSAGE, on the push +# that lands it and on every run whose floor reaches back over it, and on the +# PULL REQUEST lane every branch whose merge base predates the commit walks it +# again. `#1262` is exactly that: a body malformed by a squash, on `main`, +# uncorrectable. So the exception stays. What changed is its argument, not its +# necessity, and the old argument is written out here so the next reader does +# not re-derive a property the tree no longer has. # # Keyed on the FULL commit oid AND the EXACT rendered error string. A commit oid # covers its message, so an entry names one immutable byte string and cannot diff --git a/scripts/main-baseline.py b/scripts/main-baseline.py index f81e593e7..f1043f2b3 100755 --- a/scripts/main-baseline.py +++ b/scripts/main-baseline.py @@ -33,6 +33,28 @@ scripts/main-baseline.py --limit 20 scripts/main-baseline.py --json scripts/main-baseline.py --run-id N --emit-summary # used by CI + scripts/main-baseline.py --gate-anchor documentation-checkpoint \ + --gate-step "Every feature checkpoint updates STATUS, BENCHMARKS and NOW" + +THE ANCHOR (#1773). The last mode answers a second question with the same +doctrine: from which commit onward has one diff-scoped STEP not yet returned a +verdict? `ci.yml` used to ask the runs endpoint for `status=success`, which is +the run's OWN conclusion and therefore wrong in both directions -- it reads +success over a red `continue-on-error` job, and `cancelled` over a job that ran +to completion. Run 32625264281 is the second shape: conclusion `cancelled`, +`documentation-checkpoint` inside it `success`. That query pinned the anchor +eleven days and 484 commits behind the newest commit the gate had cleared, and +because a successful run was what the red gate was blocking, the range widened +on every push and kept re-flagging the commits blocking it. + +THE UNIT IS THE STEP, and `--gate-step` is therefore REQUIRED. Asking the same +question one level up re-opens #863: GitHub concludes a job `failure` the +moment any step fails and marks the remaining steps `skipped`, so a job-level +"did it conclude" reads `failure` over a gate that never executed and walks the +anchor past commits nothing inspected. Measured on `commit-protocol-tag`: five +consecutive runs concluded `failure` with the strict-trailer step `skipped`, +and a job-level anchor skips `038ff61e5..a4f2a9585`, six commits, forever. +Refusing the flag-less form is what keeps a future caller from re-creating it. """ from __future__ import annotations @@ -43,6 +65,7 @@ import re import subprocess import sys +from collections.abc import Sequence from dataclasses import dataclass, field from pathlib import Path @@ -293,8 +316,20 @@ def baseline_runs(repo: str, limit: int) -> tuple[list[dict], str | None]: ) if degraded: return [], degraded - if isinstance(payload, dict): - collected.extend(payload.get("workflow_runs", [])) + # An unreadable window is REMOTE_UNVERIFIED, not an empty one. Skipping + # a payload that is not a dict returned `([], None)`, which `render` + # prints as "No completed baseline run found on main." plus an + # instruction to trigger the first run -- an unreadable forge described + # to a human as a green field. `gh api` exits 0 and prints a bare list + # or `{"message": ...}` on some paths, so rc 0 is not on its own + # evidence that the question was answered. `jobs_for` above already + # refuses these shapes; this makes the two agree (#1776). + if not isinstance(payload, dict) or "workflow_runs" not in payload: + return [], ( + "REMOTE_UNVERIFIED: unexpected runs payload for " + f"event={event}" + ) + collected.extend(payload["workflow_runs"]) collected.sort(key=lambda run: run.get("created_at", ""), reverse=True) return collected[:limit], None @@ -313,6 +348,228 @@ def collect(limit: int) -> tuple[list[Verdict], str | None]: return verdicts, None +# -------------------------------------------------------------------------- +# THE DIFF-GATE ANCHOR (#1773) +# -------------------------------------------------------------------------- + +# A verdict. `cancelled` and `skipped` mean the job returned nothing, and `None` +# means it has not finished -- which is what the CURRENT run's own job looks +# like while it asks this question, so a run can never anchor itself. +CONCLUDED = frozenset({"success", "failure"}) + +# `failure` is IN that set, and it is the whole exit from #1773's cycle. +# +# The gate is per COMMIT: the range decides which commits are inspected, never +# what is demanded of them. A commit on `main` is immutable, so a violation can +# never be repaired by a later push -- anchoring on `success` therefore converts +# one violation into a permanent block on every future push, and the gate stops +# being able to say anything about NEW commits because it is drowned by an old +# one it cannot fix. Anchoring on "returned a verdict" gives every commit +# exactly one verdict, from the first run of that job whose range contains it. +# One alarm per violation is a complete alarm; a blocked branch is a broken one. +# +# The rule still BLOCKS where blocking works. On the pull request lane the same +# checkers run over `base..head` and refuse the merge. The push lane is the +# detector for whatever bypassed that, and its verdict is a report. + +# How far back the window reaches, and therefore the floor: a range can never +# widen past this many pushes however long `main` has been red. +GATE_ANCHOR_WINDOW = 20 + + +@dataclass +class Anchor: + """Where one diff-scoped job should start walking, and why.""" + + sha: str + run_id: int = 0 + # verdict -- the newest run in which this job concluded + # floor -- no run in the window did; the window's oldest head + # none -- no run in the window at all, or the query degraded + source: str = "none" + degraded: str | None = None + + +def steps_concluded(jobs: list[dict], job_name: str, step_names: Sequence[str]) -> bool: + """Did every named STEP of `job_name` return a verdict in this run? + + The unit is the step, not the job, and the difference is a coverage hole + the job-level question cannot see. GitHub concludes a job `failure` as soon + as any step fails and marks every REMAINING step `skipped`; the job's own + conclusion is then `failure` whether the diff-scoped gate refused the range + or never executed at all. Measured on `commit-protocol-tag`, runs + 32601353990 through 32623377380: job `failure` five times running, the + strict-trailer step `skipped` every time, and `038ff61e5..a4f2a9585` is six + commits that a job-level anchor walks straight past. That is #863 again at + step granularity. + + EVERY matching entry must qualify, and within it every named step. A matrix + reports one entry per lane, and half a matrix is half a verdict. + + A named step that is ABSENT from the payload does not qualify. A step is + missing because it was never reached, because it was renamed, or because + the payload is truncated, and none of those three is evidence that the gate + ran. Absence of information is not information. + """ + + if not step_names: + raise ValueError("an anchor needs the step names whose verdict it means") + entries = [ + entry for entry in jobs if job_matches(job_name, entry.get("name", "")) + ] + if not entries: + return False + for entry in entries: + if entry.get("conclusion") not in CONCLUDED: + return False + steps = entry.get("steps") or [] + for wanted in step_names: + matched = [step for step in steps if step.get("name") == wanted] + if not matched: + return False + if not all(step.get("conclusion") in CONCLUDED for step in matched): + return False + return True + + +def resolve_gate_anchor( + runs: list[dict], + job_name: str, + step_names: Sequence[str], + fetch_jobs, + window: int = GATE_ANCHOR_WINDOW, +) -> Anchor: + """The anchor for `job_name`'s `step_names` over `runs`, newest first. + + `runs` carries `window + 1` entries where the branch is long enough. The + first `window` are anchor CANDIDATES and the extra one is the floor, so the + oldest candidate's own head is inside the floor range instead of falling + off the back of it. + + `fetch_jobs(run_id) -> list[dict] | None` supplies one run's job payload; + `None` is a degraded read and stops the walk rather than being counted as + "this step did not conclude". Absence of information is not information. + """ + + if not runs: + return Anchor(sha="", source="none") + for run in runs[:window]: + jobs = fetch_jobs(run.get("id", 0)) + if jobs is None: + return Anchor( + sha="", + source="none", + degraded="REMOTE_UNVERIFIED: jobs unreadable", + ) + if steps_concluded(jobs, job_name, step_names): + return Anchor( + sha=run.get("head_sha", ""), + run_id=run.get("id", 0), + source="verdict", + ) + # THE FLOOR, and it is a BOUND ON THE RANGE, not a guarantee of coverage. + # The run just past the candidate window is the widest base this query is + # willing to pay for, and it covers every candidate in the window. + # + # What it does NOT do is preserve coverage indefinitely, and the earlier + # claim that "degrading toward more coverage is the only degradation a + # coverage gate may have" was false of this code. Measured: at `window + 1` + # non-qualifying pushes the oldest push's head leaves every future range + # permanently; at `window + 5`, five do. The trade is deliberate -- walking + # further costs an unbounded number of API calls per push, on three jobs -- + # and it is bounded: a commit is lost only after its own job has failed to + # return a verdict on `window` consecutive pushes, and only after the pull + # request lane already gated it at merge. It is a real loss, and it is + # stated here rather than denied. + # + # THE FLOOR IS NEVER THE RUN BEING PUSHED. With a single run in the window + # -- a fork's first push, and unreachable on `mudler/vllm.cpp`, which has + # thousands -- `runs[-1]` IS `runs[0]`, the anchor resolves to the head + # being pushed, and `base..head` is EMPTY. That range passes vacuously, the + # step concludes on it, and the conclusion advances this step's own anchor: + # a gate reporting success over nothing at all. There is no floor to name in + # that case, so the answer is the clean absence, and `ci.yml` uses + # `$PUSH_BASE` -- the honest base for a branch with no gated history. + # + # AND THE OFF-BY-ONE IS ONLY FIXED WHEN THE BRANCH IS LONGER THAN THE + # WINDOW. `window + 1` runs put a run PAST the candidates in hand, and its + # head is a base that covers every candidate. Below that threshold no such + # run exists: the oldest available run is itself a candidate, its head + # becomes the base, and its own commit falls outside the range. It cannot be + # repaired from this payload -- naming its PARENT is what would be needed + # and a `workflow_run` object carries no parent, only `head_sha`. So the + # bound is stated instead of denied: on a branch with `1 < n <= window` push + # runs and no verdict among them, the oldest run's own head commit is not in + # the floor range. `test_a_SHORT_history_floors_on_the_oldest_run_and_EXCLUDES_it` + # holds that as a property rather than leaving the code to imply otherwise. + if len(runs) > window: + floor = runs[window] + elif len(runs) > 1: + floor = runs[-1] + else: + return Anchor(sha="", source="none") + return Anchor(sha=floor.get("head_sha", ""), run_id=floor.get("id", 0), source="floor") + + +def push_runs(repo: str, limit: int, branch: str) -> tuple[list[dict], str | None]: + """The newest `limit` push runs of this workflow on `branch`, newest first. + + NO `status=` filter. That parameter selects on the RUN's conclusion, which + is the defect #1773 is about; every run in the window is a candidate and the + per-job payload decides. + + AN UNREADABLE PAYLOAD IS `REMOTE_UNVERIFIED`, NEVER AN EMPTY WINDOW. This + function returned `[]` for anything that was not a dict carrying + `workflow_runs`, which is what `jobs_for` has always refused to do. The + difference is not cosmetic: an empty window is a CLEAN ABSENCE, `main` exits + 1, and `ci.yml` falls back to `$PUSH_BASE` -- a one-push range that passes, + concludes, and advances this step's own anchor past every commit the + narrowing dropped. Measured on #1776 by driving `main` through a fake + `gh_api`: a list payload, a null payload and `{"message": "Not Found"}` all + exited 1 where they had to exit 3. Absence of information is not + information. + """ + + payload, degraded = gh_api( + f"repos/{repo}/actions/workflows/{WORKFLOW_FILE}/runs" + f"?branch={branch}&event=push&per_page={limit}" + ) + if degraded: + return [], degraded + if not isinstance(payload, dict): + return [], "REMOTE_UNVERIFIED: unexpected runs payload" + runs = payload.get("workflow_runs") + if not isinstance(runs, list): + return [], "REMOTE_UNVERIFIED: runs payload carries no workflow_runs list" + runs = list(runs) + runs.sort(key=lambda run: run.get("created_at", ""), reverse=True) + return runs[:limit], None + + +def gate_anchor( + job_name: str, step_names: Sequence[str], branch: str, window: int +) -> Anchor: + """`resolve_gate_anchor` against the live forge. + + Reads `window + 1` runs: `window` candidates plus the floor beyond them. + The COST is one runs call plus one jobs call per candidate INSPECTED, and + the walk stops at the first verdict -- so the ordinary push spends two + calls per job, and only a branch whose gate has not concluded in a long + time approaches `window + 1`. + """ + + repo = repository() + runs, degraded = push_runs(repo, window + 1, branch) + if degraded: + return Anchor(sha="", source="none", degraded=degraded) + + def fetch(run_id: int) -> list[dict] | None: + jobs, failed = jobs_for(repo, run_id) + return None if failed else (jobs or []) + + return resolve_gate_anchor(runs, job_name, step_names, fetch, window) + + # -------------------------------------------------------------------------- # Rendering # -------------------------------------------------------------------------- @@ -452,8 +709,84 @@ def main(argv: list[str] | None = None) -> int: help="write GITHUB_STEP_SUMMARY and exit non-zero when not green", ) parser.add_argument("--offline", action="store_true", help="skip every network call") + parser.add_argument( + "--gate-anchor", + metavar="JOB", + help="print the commit from which JOB has not yet returned a verdict", + ) + parser.add_argument( + "--gate-step", + metavar="NAME", + action="append", + default=[], + help=( + "a diff-scoped step of JOB whose conclusion IS the verdict; " + "repeatable, and required with --gate-anchor" + ), + ) + parser.add_argument("--branch", default=BRANCH, help="branch for --gate-anchor") + parser.add_argument( + "--window", + type=int, + default=GATE_ANCHOR_WINDOW, + help="push runs --gate-anchor may look back over; also its floor", + ) args = parser.parse_args(argv) + if args.gate_anchor: + if not args.gate_step: + # NOT a default. A job-level anchor is #863 at step granularity + # (see the module docstring), and a caller who forgot the flag must + # find out here rather than in six months of skipped commits. + print( + "--gate-anchor needs at least one --gate-step: the verdict " + "belongs to a STEP, and a job's conclusion cannot tell a gate " + "that refused from a gate that never ran.", + file=sys.stderr, + ) + return 2 + if args.offline: + print("REMOTE_UNVERIFIED: --offline", file=sys.stderr) + return 3 + anchor = gate_anchor( + args.gate_anchor, tuple(args.gate_step), args.branch, args.window + ) + if anchor.degraded: + print(anchor.degraded, file=sys.stderr) + # The SHA alone on stdout, so a shell can take it with `$(...)`; every + # word of explanation goes to stderr, where it reaches the job log + # without reaching the variable. + if anchor.source == "verdict": + print( + f"{args.gate_anchor} last returned a verdict at {anchor.sha} " + f"(run {anchor.run_id})", + file=sys.stderr, + ) + elif anchor.source == "floor": + print( + f"no run in the last {args.window} pushes on {args.branch} has a " + f"verdict from {args.gate_anchor}; falling back to the WINDOW " + f"FLOOR {anchor.sha} (run {anchor.run_id}). The range is bounded " + "by the window and covers every commit inside it.", + file=sys.stderr, + ) + else: + print( + f"no anchor for {args.gate_anchor} on {args.branch}. This is not " + "a statement that nothing needs gating.", + file=sys.stderr, + ) + if not anchor.sha: + # 3 is REMOTE_UNVERIFIED and 1 is a clean absence, the same split + # `scripts/agent-pr-body.py` uses and AGENTS.md documents. The + # caller MUST be able to tell them apart: a clean absence may fall + # back to a narrower base, and an unreadable forge may not, because + # a narrowed pass would advance this job's own anchor past the + # commits the narrowing dropped. + return 3 if anchor.degraded else 1 + print(anchor.sha) + return 0 + if args.run_id: if args.offline: print("REMOTE_UNVERIFIED: --offline", file=sys.stderr) diff --git a/tests/scripts/test_check_role_discipline.py b/tests/scripts/test_check_role_discipline.py index c941026bb..64c24d280 100644 --- a/tests/scripts/test_check_role_discipline.py +++ b/tests/scripts/test_check_role_discipline.py @@ -17,9 +17,12 @@ from __future__ import annotations +import ast import importlib.util +import io import subprocess import sys +import tokenize import unittest from pathlib import Path @@ -183,6 +186,121 @@ def test_synthetic_merge_of_an_unnamed_branch_does_not_arrive(self) -> None: ) +def executable_source(path: Path) -> str: + """`path`'s source with its PROSE blanked: comments and docstrings. + + The assertion below forbids three ref-resolving substrings anywhere in the + checker. Applied to the RAW text it forbade them in English too, so a + comment explaining why this checker does not consult `origin/main` reddened + the suite with a message about a lookup that was not there (#1776). This is + the `code_lines` idiom from `test_main_baseline.py`, widened from whole-line + YAML comments to Python's two prose surfaces. + + String LITERALS are deliberately KEPT. A real ref lookup is spelled as one + -- `git("rev-parse", "origin/main")` -- so dropping strings would delete the + obligation instead of narrowing it. Line numbering is preserved, so a hit + reports where it is. + """ + + text = path.read_text(encoding="utf-8") + lines = text.splitlines() + prose: set[int] = set() + for node in ast.walk(ast.parse(text)): + if not isinstance( + node, (ast.Module, ast.ClassDef, ast.FunctionDef, ast.AsyncFunctionDef) + ): + continue + body = getattr(node, "body", None) + if not body: + continue + first = body[0] + if ( + isinstance(first, ast.Expr) + and isinstance(first.value, ast.Constant) + and isinstance(first.value.value, str) + ): + prose.update(range(first.lineno - 1, first.end_lineno)) + for token in tokenize.generate_tokens(io.StringIO(text).readline): + if token.type != tokenize.COMMENT: + continue + row = token.start[0] - 1 + if row not in prose: + lines[row] = lines[row][: token.start[1]] + return "\n".join("" if index in prose else line + for index, line in enumerate(lines)) + + +class ArrivalDiscriminatorTests(unittest.TestCase): + """WHERE the evidence of arrival must live, pinned so it cannot be widened. + + #1764 and #1773 both carry the hypothesis that these commits fail because an + external contributor's branch lives on a fork and cannot be found on + `origin`. The hypothesis is refuted by this class's first test: the checker + resolves no ref at all. For a single-parent commit the whole decision is + `ROW_BRANCH.search(subject + body) or PR_REFERENCE.search(SUBJECT)`, and the + one NON-fork commit of the five being re-flagged fails identically to the + four fork ones. + + What the five actually share is a subject with no `(#N)`, because the merger + supplied an explicit `commit_title` and suppressed the append GitHub + otherwise makes. The tempting repair is to look for `#N` anywhere in the + message instead of in the subject. That deletes the obligation: AGENTS.md + requires every change to start from an issue, so EVERY commit body in this + repository names one, and a body-wide match passes every direct-to-main push + ever made. `test_a_body_only_issue_reference_does_not_satisfy_arrival` is + that mutation, held shut. + """ + + # `dd8a3b0e1`, verbatim: a real fork squash that the gate flags, with the + # body reference that must NOT rescue it. + FORK_SUBJECT = "windows: fix native MSVC/Vulkan build portability" + FORK_BODY = "Removes POSIX-only constructs ...\n\nIssue: #503\n" + + def test_the_checker_resolves_no_ref_to_decide_arrival(self) -> None: + source = executable_source(ROOT / "scripts/check-role-discipline.py") + for forbidden in ("ls-remote", "for-each-ref", "origin/"): + hits = [ + f"{number}: {line.strip()}" + for number, line in enumerate(source.splitlines(), start=1) + if forbidden in line + ] + self.assertEqual( + hits, [], + f"arrival is decided from the commit message, and {forbidden!r} " + "resolves a ref. A lookup here would make the fork hypothesis " + "testable, and that is not what this checker does", + ) + + def test_a_fork_squash_carrying_the_pr_number_arrives(self) -> None: + """The rule is satisfiable by an external contributor. A fork pull + request merged with the DEFAULT squash title passes, because GitHub + appends the number to it.""" + self.assertTrue( + checker.arrives_via_row_pr( + ["a" * 40], f"{self.FORK_SUBJECT} (#640)", self.FORK_BODY + ) + ) + + def test_a_body_only_issue_reference_does_not_satisfy_arrival(self) -> None: + """The widening that must never land.""" + self.assertFalse( + checker.arrives_via_row_pr( + ["a" * 40], self.FORK_SUBJECT, self.FORK_BODY + ) + ) + + def test_a_direct_push_naming_its_issue_in_the_body_still_fails(self) -> None: + """The same widening, seen from the case the gate exists for.""" + self.assertTrue( + violations( + ["src/vllm/engine.cpp"], + govern_integration=True, + subject="fix: quick repair on the shared checkout", + body="Issue: #1773\n\nFOLLOWING_AGENTS_PROTOCOL\n", + ) + ) + + class CutoverWiring(unittest.TestCase): def test_role_cutover_is_a_full_sha(self) -> None: self.assertRegex(checker.ROLE_DISCIPLINE_SINCE or "", r"\A[0-9a-f]{40}\Z") diff --git a/tests/scripts/test_main_baseline.py b/tests/scripts/test_main_baseline.py index 584b0faa9..fdd963e15 100644 --- a/tests/scripts/test_main_baseline.py +++ b/tests/scripts/test_main_baseline.py @@ -51,8 +51,9 @@ import sys import tempfile import unittest -from contextlib import redirect_stdout +from contextlib import redirect_stderr, redirect_stdout from pathlib import Path +from typing import NamedTuple ROOT = Path(__file__).resolve().parents[2] WORKFLOW = ROOT / ".github/workflows/ci.yml" @@ -74,8 +75,46 @@ def load_module(): baseline = load_module() -def job(name, conclusion): - return {"name": name, "conclusion": conclusion, "status": "completed"} +# The step whose conclusion IS `commit-protocol-tag`'s verdict. Named once so +# the payload builders and the assertions cannot drift apart. +GATE_STEP = "Every new commit satisfies the strict trailer contract" + + +def job_with_steps(name, conclusion, steps): + """A jobs-API entry carrying its `steps` array, as the real payload does. + + `steps[].conclusion` is the only field that says whether a given step RAN. + A job that concludes `failure` because step 3 failed reports steps 4..N as + `skipped`, and the job's own conclusion cannot tell the two apart. + """ + + return { + "name": name, + "conclusion": conclusion, + "status": "completed", + "steps": [ + { + "name": step_name, + "conclusion": step_conclusion, + "status": "completed", + "number": position + 1, + } + for position, (step_name, step_conclusion) in enumerate(steps) + ], + } + + +def job(name, conclusion, steps=None): + """A jobs-API entry. + + `steps` defaults to the ORDINARY case: one step, named `GATE_STEP`, that + reached the same conclusion the job did. The interesting case is the one + where they DIFFER, and `job_with_steps` states that one out loud. + """ + + return job_with_steps( + name, conclusion, [(GATE_STEP, conclusion)] if steps is None else steps + ) # The real 31448896841 shape: run says success, both sanitizer lanes are red. @@ -331,6 +370,56 @@ def run_main(self, argv, collected): baseline.collect = original return code, buffer.getvalue() + def run_anchor(self, argv, anchor): + original = baseline.gate_anchor + baseline.gate_anchor = lambda *args, **kwargs: anchor + buffer = io.StringIO() + try: + with redirect_stdout(buffer): + code = baseline.main(argv) + finally: + baseline.gate_anchor = original + return code, buffer.getvalue() + + def test_the_anchor_CLI_exits_3_on_a_degraded_read(self) -> None: + """3 is REMOTE_UNVERIFIED and 1 is a clean absence. + + Collapsing them is F3: the caller cannot tell "this branch has no gated + history, so `PUSH_BASE` is honest" from "the forge did not answer, so + the base is unknown". The second must SKIP the gate, because a narrowed + pass would advance the step's own anchor past what the narrowing + dropped. `ci.yml` branches on exactly this number. + """ + code, _ = self.run_anchor( + ["--gate-anchor", "documentation-checkpoint", "--gate-step", "s"], + baseline.Anchor(sha="", source="none", degraded="REMOTE_UNVERIFIED: x"), + ) + self.assertEqual(code, 3) + + def test_the_anchor_CLI_exits_1_on_a_CLEAN_absence(self) -> None: + code, _ = self.run_anchor( + ["--gate-anchor", "documentation-checkpoint", "--gate-step", "s"], + baseline.Anchor(sha="", source="none", degraded=None), + ) + self.assertEqual(code, 1) + + def test_the_anchor_CLI_exits_3_when_told_to_stay_OFFLINE(self) -> None: + """`--offline` is the same class of answer: the forge was not asked.""" + buffer = io.StringIO() + with redirect_stdout(buffer): + code = baseline.main( + ["--gate-anchor", "x", "--gate-step", "s", "--offline"] + ) + self.assertEqual(code, 3) + + def test_the_anchor_CLI_REFUSES_without_a_gate_step(self) -> None: + """Not a default. Exiting 2 is what stops a future caller re-creating + the job-granularity hole by simply omitting the flag.""" + buffer = io.StringIO() + with redirect_stdout(buffer): + code = baseline.main(["--gate-anchor", "documentation-checkpoint"]) + self.assertEqual(code, 2) + def test_no_completed_baseline_run_exits_nonzero(self) -> None: """`main-baseline.py && echo ok` must not print ok when nothing ran.""" code, text = self.run_main([], ([], None)) @@ -538,7 +627,7 @@ def resolve_boolean(expression: str, event: str) -> bool: VALUE_TOKEN = re.compile( r"""\s*(?: - (?P&&|\|\||==|!=|\(|\)) + (?P&&|\|\||==|!=|!|\(|\)) | (?P'[^']*') | (?Ptrue|false|null) | (?P[0-9]+) @@ -634,6 +723,11 @@ def parse_primary(self): if self.take() != ("op", ")"): raise AssertionError(f"unbalanced parenthesis in {self.source!r}") return value + if (kind, text) == ("op", "!"): + # Unary negation binds tighter than `==`, as it does on the forge. + # It is here for `!cancelled()`, the condition that keeps a gate + # step running after an earlier step failed (#1776). + return not _truthy(self.parse_primary()) if kind == "text": return text[1:-1] if kind == "number": @@ -641,6 +735,17 @@ def parse_primary(self): if kind == "literal": return {"true": True, "false": False, "null": None}[text] if kind == "ctx": + if self.peek() == ("op", "("): + # A STATUS FUNCTION -- `cancelled()`, `success()`, `failure()`, + # `always()`. It is resolved from `contexts` under its called + # name, so an unmodelled one raises exactly as an unmodelled + # context does. None of them takes an argument in this subset. + self.take() + if self.take() != ("op", ")"): + raise AssertionError( + f"{text}(...) takes no argument in {self.source!r}" + ) + text = f"{text}()" if text not in self.contexts: raise AssertionError( f"unknown context {text!r} in {self.source!r}; add it to " @@ -684,6 +789,123 @@ def varies_per_run(expression: str, event: str) -> bool: ) +def resolve_condition(expression: str, state: dict) -> bool: + """Resolve a step-level `if:` to the boolean GitHub decides to run on. + + A step guard is not a concurrency key, so it gets its own entry point, but + it uses the same evaluator for the same reason: the mutation that matters + keeps every substring. Appending `|| true` to the guards of all five gate + steps leaves `!cancelled()`, both `steps.*.outcome == 'success'` clauses and + `env.GATE_ANCHOR_DEGRADED != 'true'` byte-for-byte intact, and turns the + whole conjunction into the constant `true` -- which restores exactly the + degraded-read narrowing this row exists to remove. A substring assertion + cannot see that. A resolved boolean cannot miss it. + + `state` supplies every context AND every status function the guard names. + Anything it does not model raises, so a guard that grows a new term reds + this suite instead of resolving to something plausible. + """ + + inner = expression.strip() + if inner.startswith("${{") and inner.endswith("}}"): + inner = inner[3:-2] + elif "${{" in inner: + raise AssertionError(f"partial interpolation in a step guard: {expression!r}") + return _truthy(_Expression(inner, state).parse()) + + +SHELL_KEYWORDS = frozenset( + {"if", "then", "else", "elif", "fi", "while", "until", "do", "done", + "case", "esac", "in", "{", "}", "!"} +) + +# The words that open a compound condition. A statement under one of these has +# its exit status READ by the shell rather than propagated, so `set -e` does not +# abort the script when it is non-zero. +CONDITION_HEADERS = frozenset({"if", "elif", "while", "until"}) + +# What a gate step's body may run BEFORE its gate. Each computes the range or +# narrates it: `set` configures the shell, `[` tests it, `echo` writes to the +# step log, and `:` expands its arguments and succeeds. +RANGE_PRELUDE_COMMANDS = frozenset({"set", "[", "echo", ":"}) + +# The prelude commands with a ROUTINE failing form, admitted only where the +# shell consumes their exit status. `[` returns 1 on a false test, which under +# `set -e` aborts the step unless an `if`/`while` header or an `||`/`&&` reads +# it. Membership in the allowlist above is therefore necessary and not +# sufficient. +MUST_BE_CONSUMED = frozenset({"["}) + + +class Statement(NamedTuple): + """One statement of a step body, with the context `set -e` decides on.""" + + kind: str #: ``"command"`` or ``"assignment"`` + head: str #: the command word, or the name an assignment binds + text: str #: the statement itself, separators stripped + consumed: bool #: its status is read by a condition header or an ``||``/``&&`` + + +def statements_in(body: str) -> list[Statement]: + """Every statement a step body runs, ASSIGNMENTS INCLUDED. + + Whole-line comments are dropped, backslash continuations are joined, and + each logical line is split on the operators that separate statements -- + keeping those operators, because whether a statement's failure aborts the + step depends on what follows it. Shell keywords are not commands. `for` and + `case` headers are skipped whole, because their word list is data rather + than a call. + + An assignment is a statement here rather than a skip, and that is the fix + this function exists for. `commands_in`, which it replaces, `continue`d on + any `name=...` head BEFORE any other check ran, so + `base="$(git rev-parse --verify "${base}^{commit}")"` inserted ahead of a + gate was invisible to the caller below while still aborting the step under + `set -eu` whenever the anchor was not in the checkout (#1776, round 3). + """ + + logical: list[str] = [] + pending = "" + for raw in body.splitlines(): + text = raw.strip() + if not text or text.startswith("#"): + continue + if pending: + text = f"{pending} {text}" + pending = "" + if text.endswith("\\"): + pending = text[:-1].strip() + continue + logical.append(text) + if pending: + logical.append(pending) + + found: list[Statement] = [] + for line in logical: + parts = re.split(r"(&&|\|\||;|\|)", line) + pieces, separators = parts[0::2], parts[1::2] + for index, piece in enumerate(pieces): + words = piece.split() + if not words or words[0] in ("for", "case"): + continue + header = words[0] in CONDITION_HEADERS + while words and words[0] in SHELL_KEYWORDS: + words = words[1:] + if not words: + continue + head = words[0] + following = separators[index] if index < len(separators) else "" + consumed = header or following in ("&&", "||") + binding = re.fullmatch(r"([A-Za-z_][A-Za-z0-9_]*)=.*", head) + found.append(Statement( + "assignment" if binding else "command", + binding.group(1) if binding else head, + piece.strip(), + consumed, + )) + return found + + def concurrency_blocks(ci: dict) -> list[tuple[str, dict]]: """Every concurrency block in the workflow, workflow level first. @@ -771,11 +993,15 @@ def step_run_body(step: list[str]) -> str | None: return "\n".join(body) + "\n" -def run_shimmed(body: str, environment: dict[str, str]) -> tuple[int, list[list[str]], str]: +def run_shimmed( + body: str, environment: dict[str, str], python_exit: int = 0 +) -> tuple[int, list[list[str]], str]: """Execute a step body with `python3` replaced by an argv recorder. No checker actually runs; what is under test is the SHELL logic that decides - which checkers get invoked, and with which range. + which checkers get invoked, and with which range. `python_exit` makes the + recorder fail on demand, which is how the REMOTE_UNVERIFIED path -- rc 3 -- + gets executed rather than described. """ with tempfile.TemporaryDirectory(prefix="vllm-baseline-step-") as temporary: @@ -790,7 +1016,8 @@ def run_shimmed(body: str, environment: dict[str, str]) -> tuple[int, list[list[ " printf 'ARGV'\n" ' for a in "$@"; do printf \'\\t%s\' "$a"; done\n' " printf '\\n'\n" - '} >> "$VLLM_BASELINE_ARGV"\n', + '} >> "$VLLM_BASELINE_ARGV"\n' + f"exit {python_exit}\n", encoding="utf-8", ) recorder.chmod(0o700) @@ -1493,16 +1720,26 @@ def test_the_strict_trailer_walk_lives_in_a_group_free_job(self) -> None: "cancelled run would skip that range forever (#863)", ) - def test_every_diff_scoped_step_bases_on_the_last_gated_commit(self) -> None: + def test_every_diff_scoped_step_bases_on_its_own_jobs_anchor(self) -> None: """THE invariant, and it is not group-freeness. A workflow-level `cancel-in-progress` cancels every job in the run, including jobs that carry no group of their own. So moving a gate to a group-free job protects nothing once the push lane is latest-only. What protects it is the base: `github.event.before` is the previous push - whether or not it was gated, and the last GREEN commit is not. Any step - that consumes `before` without that fallback loses its range the first - time a push supersedes it. + whether or not it was gated, and the anchor is not. + + RE-PINNED, not relaxed (#1773). The assertion used to name `LAST_GREEN`, + the one shared string `last-gated-commit` resolved from a RUN-level + `status=success`. That value was wrong in both directions (#274) and it + froze whenever a gate was red, so the range widened on every push. It + also could not be resolved for three jobs at once: they have three + different cancellation profiles, and an `if: always()` consumer outlives + its own resolver, which is how run 32625264281 passed this gate over a + single push. The demand here is now STRICTER -- each such step consumes + an anchor its OWN job resolved -- and + `AnchorStepTests.test_each_diff_scoped_job_resolves_its_own_anchor` + asserts the resolution names that job. """ for name, job in self.ci["jobs"].items(): for step in job.get("steps") or []: @@ -1511,11 +1748,20 @@ def test_every_diff_scoped_step_bases_on_the_last_gated_commit(self) -> None: continue with self.subTest(job=name, step=step.get("name")): self.assertIn( - "LAST_GREEN", " ".join(env), - "a diff-scoped step that does not consume the last " - "gated commit; a cancelled push skips its range forever", + "GATE_ANCHOR", " ".join(env), + "a diff-scoped step that does not consume an anchor; a " + "cancelled push skips its range forever", + ) + self.assertIn('GATE_ANCHOR:-', str(step.get("run", ""))) + resolvers = [ + s for s in job.get("steps") or [] + if f"--gate-anchor {name}" in str(s.get("run", "")) + ] + self.assertEqual( + len(resolvers), 1, + f"{name} consumes an anchor it does not resolve for " + "itself; a shared anchor is the race #1773 measured", ) - self.assertIn('LAST_GREEN:-', str(step.get("run", ""))) def test_agent_record_no_longer_walks_a_diff_range(self) -> None: """It keeps its cancellable group, so it must hold nothing diff-scoped.""" @@ -1525,16 +1771,34 @@ def test_agent_record_no_longer_walks_a_diff_range(self) -> None: self.assertNotIn("--range", body) self.assertIsNotNone(self.ci["jobs"]["agent-record"].get("concurrency")) - def test_the_diff_scoped_base_is_the_last_gated_commit(self) -> None: - """The enabling half. Without it, cancelling a push loses the range.""" + def test_the_diff_scoped_base_is_this_jobs_own_anchor(self) -> None: + """The enabling half. Without it, cancelling a push loses the range. + + `last-gated-commit` is still asserted to exist, because `agent-record` + inherits its closed-pull-request skip through `needs:` on it (#873). It + no longer resolves the base: see the re-pinning note above. + """ self.assertIn("last-gated-commit", self.ci["jobs"]) owner = self.owning_job("check-commit-trailers.py --range") - self.assertIn("last-gated-commit", str(self.ci["jobs"][owner].get("needs"))) walk = next( s for s in self.ci["jobs"][owner]["steps"] if "check-commit-trailers.py --range" in str(s.get("run", "")) ) - self.assertIn("LAST_GREEN", str(walk.get("env"))) + self.assertIn("GATE_ANCHOR", str(walk.get("env"))) + self.assertTrue( + any( + f"--gate-anchor {owner}" in str(s.get("run", "")) + for s in self.ci["jobs"][owner]["steps"] + ), + f"{owner} does not resolve its own anchor", + ) + + def test_the_guard_job_resolves_no_shared_value(self) -> None: + """A shared base resolved elsewhere is the race, not the fix.""" + guard = self.ci["jobs"]["last-gated-commit"] + self.assertIsNone(guard.get("outputs")) + body = " ".join(str(s.get("run", "")) for s in guard["steps"]) + self.assertNotIn("gh api", body) def test_the_base_falls_back_when_no_successful_run_is_found(self) -> None: """A failed or rate-limited query must degrade to today's behaviour, @@ -1721,5 +1985,1213 @@ def test_a_container_tag_run_is_never_cancelled(self) -> None: self.assertIn("refs/tags/", cancel) +# -------------------------------------------------------------------------- +# GATE-ANCHOR-PER-JOB (#1773). The base a diff-scoped gate walks from. +# -------------------------------------------------------------------------- + + +def push_run(run_id: int, sha: str, conclusion: str | None, created_at: str) -> dict: + """One entry of the workflow-runs payload for the push lane.""" + + return { + "id": run_id, + "head_sha": sha, + "conclusion": conclusion, + "status": "completed" if conclusion else "in_progress", + "created_at": created_at, + } + + +def run_level_anchor(runs: list[dict]) -> str: + """The rule this row replaces, modelled exactly. + + `last-gated-commit` asked the API for + `...?branch=main&event=push&status=success&per_page=1` and took + `.workflow_runs[0].head_sha`. `status=success` filters on the RUN's own + conclusion, so this is "the newest push run whose conclusion is success", + and nothing else. Modelled here rather than queried so the cycle can be + constructed offline. + """ + + for run in runs: + if run.get("conclusion") == "success": + return run.get("head_sha", "") + return "" + + +class GateAnchorTests(unittest.TestCase): + """`gate_anchor` reads PER-JOB conclusions, like everything else here. + + Run 32625264281 is the shape that makes this necessary and it is not + hypothetical: its own conclusion is `cancelled` while + `documentation-checkpoint` inside it concluded `success`. A run-level query + cannot see that job, so the anchor it returns was eleven days and 484 + commits older than the newest commit the gate had actually cleared. + """ + + def fetcher(self, mapping): + return lambda run_id: mapping[run_id] + + def test_a_cancelled_run_whose_job_concluded_is_the_anchor(self) -> None: + runs = [ + push_run(3, "c" * 40, "cancelled", "2026-08-23T07:39:25Z"), + push_run(2, "b" * 40, "cancelled", "2026-08-23T07:19:38Z"), + push_run(1, "a" * 40, "success", "2026-08-12T23:53:24Z"), + ] + jobs = { + 3: [job("build-test-cpu", "cancelled")], + 2: [job("documentation-checkpoint", "success")], + 1: [job("documentation-checkpoint", "success")], + } + anchor = baseline.resolve_gate_anchor( + runs, "documentation-checkpoint", (GATE_STEP,), self.fetcher(jobs) + ) + self.assertEqual(anchor.sha, "b" * 40) + self.assertEqual(anchor.source, "verdict") + + def test_a_job_that_concluded_FAILURE_is_still_the_anchor(self) -> None: + """The change that breaks the cycle, asserted on its own. + + A commit on `main` is immutable, so a violation can never be repaired by + a later push. Anchoring on `success` therefore turns one violation into + a permanent block, and the gate stops being able to report anything + about new commits. A verdict is a verdict. + """ + runs = [ + push_run(2, "b" * 40, "failure", "2026-08-23T07:19:38Z"), + push_run(1, "a" * 40, "success", "2026-08-12T23:53:24Z"), + ] + jobs = { + 2: [job("documentation-checkpoint", "failure")], + 1: [job("documentation-checkpoint", "success")], + } + anchor = baseline.resolve_gate_anchor( + runs, "documentation-checkpoint", (GATE_STEP,), self.fetcher(jobs) + ) + self.assertEqual(anchor.sha, "b" * 40) + + def test_cancelled_skipped_absent_and_pending_jobs_do_not_anchor(self) -> None: + runs = [ + push_run(5, "e" * 40, None, "2026-08-23T08:49:48Z"), + push_run(4, "d" * 40, "cancelled", "2026-08-23T08:15:39Z"), + push_run(3, "c" * 40, "cancelled", "2026-08-23T07:46:52Z"), + push_run(2, "b" * 40, "cancelled", "2026-08-23T07:39:25Z"), + push_run(1, "a" * 40, "failure", "2026-08-23T07:19:38Z"), + ] + jobs = { + 5: [job("documentation-checkpoint", None)], # this run, in flight + 4: [job("documentation-checkpoint", "cancelled")], + 3: [job("documentation-checkpoint", "skipped")], + 2: [job("build-test-cpu", "cancelled")], # absent entirely + 1: [job("documentation-checkpoint", "failure")], + } + anchor = baseline.resolve_gate_anchor( + runs, "documentation-checkpoint", (GATE_STEP,), self.fetcher(jobs) + ) + self.assertEqual(anchor.sha, "a" * 40) + + def test_a_run_never_anchors_itself(self) -> None: + """The current run's own job is `in_progress`, so its conclusion is + `null`. If that qualified, every gate would walk an empty range and pass + vacuously forever -- the loudest possible version of this defect. + + THIS TEST USED TO ASSERT THE HARM ITS OWN DOCSTRING NAMES. It checked + that the VERDICT path refused the in-flight run and then asserted the + floor returned `a * 40` -- the head being pushed. `base..head` with both + ends at that commit is the empty range, so the gate passed over nothing, + the step concluded on it, and the conclusion advanced the anchor. + Refusing to anchor on the run and then flooring onto it is the same + answer by a different route. Found by the second review of #1776. + + With one run there is no floor to name, so the honest answer is the + clean absence: `main` exits 1 and `ci.yml` uses `$PUSH_BASE`. Reachable + on a fork's first push; on `mudler/vllm.cpp`, whose window is always + full, it is latent. + """ + runs = [push_run(1, "a" * 40, None, "2026-08-23T08:49:48Z")] + jobs = {1: [job("documentation-checkpoint", None)]} + anchor = baseline.resolve_gate_anchor( + runs, "documentation-checkpoint", (GATE_STEP,), self.fetcher(jobs) + ) + self.assertNotEqual(anchor.source, "verdict") + self.assertEqual(anchor.source, "none") + self.assertEqual(anchor.sha, "") + self.assertIsNone(anchor.degraded) + + def test_the_floor_is_NEVER_the_head_being_pushed(self) -> None: + """The property behind the case above, over every window size. + + An anchor equal to the head makes `base..head` empty. Whatever else a + degraded window does, it may not produce a gate that reports success + over no commits AND concludes, because the conclusion is what advances + the anchor. + """ + for count in range(1, 6): + for window in (1, 2, 3, 20): + with self.subTest(runs=count, window=window): + runs = [ + push_run(i, chr(ord("a") + i) * 40, None, + f"2026-08-23T0{9 - i}:00:00Z") + for i in range(count) + ] + anchor = baseline.resolve_gate_anchor( + runs, "documentation-checkpoint", (GATE_STEP,), + lambda run_id: [job("documentation-checkpoint", None)], + window=window, + ) + self.assertNotEqual( + anchor.sha, runs[0]["head_sha"], + "the anchor is the head being pushed, so the range is empty", + ) + + def test_a_SHORT_history_floors_on_the_oldest_run_and_EXCLUDES_it(self) -> None: + """The bound, stated as a property rather than as a claim. + + `window + 1` runs put a run PAST the candidates in hand, and its head is + a base that covers every candidate. A branch with fewer push runs than + the window has no such run: the oldest available run is itself a + candidate, its head becomes the base, and `base..head` therefore does + NOT contain that commit. It cannot be repaired from this payload -- the + parent is what would be needed and a `workflow_run` object carries only + `head_sha`. + + Asserted here so the residual cannot quietly grow or quietly be denied. + """ + runs = [ + push_run(3, "c" * 40, None, "2026-08-23T09:00:00Z"), + push_run(2, "b" * 40, "failure", "2026-08-23T08:00:00Z"), + push_run(1, "a" * 40, "failure", "2026-08-23T07:00:00Z"), + ] + anchor = baseline.resolve_gate_anchor( + runs, "documentation-checkpoint", (GATE_STEP,), + lambda run_id: [job("documentation-checkpoint", None)], + window=20, + ) + self.assertEqual(anchor.source, "floor") + # The OLDEST run's own head, so run 1's commit is outside the range it + # bases. That is the residual, and it is what a longer branch does not + # have: with more than `window` runs the floor comes from `runs[window]`, + # a run past every candidate. + self.assertEqual(anchor.sha, "a" * 40) + + def test_a_matrix_job_anchors_only_when_every_lane_concluded(self) -> None: + runs = [ + push_run(2, "b" * 40, "cancelled", "2026-08-23T07:39:25Z"), + push_run(1, "a" * 40, "cancelled", "2026-08-23T07:19:38Z"), + ] + jobs = { + 2: [ + job("sanitize-cpu (address,undefined)", "failure"), + job("sanitize-cpu (thread)", "cancelled"), + ], + 1: [ + job("sanitize-cpu (address,undefined)", "failure"), + job("sanitize-cpu (thread)", "success"), + ], + } + anchor = baseline.resolve_gate_anchor( + runs, "sanitize-cpu", (GATE_STEP,), self.fetcher(jobs) + ) + self.assertEqual(anchor.sha, "a" * 40) + + def test_no_qualifying_run_returns_the_WINDOW_FLOOR(self) -> None: + """The floor, and what it actually bounds. + + It bounds the RANGE: a range never widens past the window however long + `main` has been red. It does NOT bound the loss, and the earlier claim + that "degrading toward more coverage is the only degradation a coverage + gate may have" was false of this code -- past the window, commits roll + off the back permanently. `test_past_the_window_commits_roll_off` states + that trade as an executable fact rather than denying it. + """ + runs = [ + push_run(3, "c" * 40, "cancelled", "2026-08-23T07:46:52Z"), + push_run(2, "b" * 40, "cancelled", "2026-08-23T07:39:25Z"), + push_run(1, "a" * 40, "cancelled", "2026-08-23T07:19:38Z"), + ] + jobs = {n: [job("documentation-checkpoint", "cancelled")] for n in (1, 2, 3)} + anchor = baseline.resolve_gate_anchor( + runs, "documentation-checkpoint", (GATE_STEP,), self.fetcher(jobs) + ) + self.assertEqual(anchor.sha, "a" * 40) + self.assertEqual(anchor.source, "floor") + + def test_a_SKIPPED_gate_step_in_a_CONCLUDED_job_does_not_anchor(self) -> None: + """F1, as one assertion. The live shape of runs 32601353990..32623377380. + + GitHub concludes a job `failure` as soon as any step fails and marks + every remaining step `skipped`. The job says `failure` either way, so + the job-level question cannot tell a gate that REFUSED the range from a + gate that never ran. Run 2 here is the second, and anchoring on it + would walk past every commit run 2 introduced. + """ + runs = [ + push_run(2, "b" * 40, "failure", "2026-08-23T07:39:25Z"), + push_run(1, "a" * 40, "success", "2026-08-23T07:19:38Z"), + ] + jobs = { + 2: [job("commit-protocol-tag", "failure", + [("Some earlier step", "failure"), (GATE_STEP, "skipped")])], + 1: [job("commit-protocol-tag", "success", + [("Some earlier step", "success"), (GATE_STEP, "success")])], + } + anchor = baseline.resolve_gate_anchor( + runs, "commit-protocol-tag", (GATE_STEP,), self.fetcher(jobs) + ) + self.assertEqual(anchor.sha, "a" * 40) + self.assertEqual(anchor.source, "verdict") + + def test_a_gate_step_ABSENT_from_the_payload_does_not_anchor(self) -> None: + """A renamed, unreached or truncated step is not a verdict. + + This is the mutation route a future rename would take: change the step + name in `ci.yml` and leave the `--gate-step` value behind, and a rule + that treated "not found" as "fine" would anchor on every run forever. + """ + runs = [push_run(1, "a" * 40, "failure", "2026-08-23T07:19:38Z")] + jobs = {1: [job("commit-protocol-tag", "failure", [("Other", "failure")])]} + anchor = baseline.resolve_gate_anchor( + runs, "commit-protocol-tag", (GATE_STEP,), self.fetcher(jobs) + ) + self.assertNotEqual(anchor.source, "verdict") + + def test_every_named_step_must_conclude_not_just_one(self) -> None: + """`documentation-checkpoint` runs TWO diff-scoped gates, and half a + job's gates concluding is half a verdict.""" + runs = [ + push_run(2, "b" * 40, "failure", "2026-08-23T07:39:25Z"), + push_run(1, "a" * 40, "failure", "2026-08-23T07:19:38Z"), + ] + jobs = { + 2: [job("documentation-checkpoint", "failure", + [("first", "failure"), ("second", "skipped")])], + 1: [job("documentation-checkpoint", "failure", + [("first", "failure"), ("second", "failure")])], + } + anchor = baseline.resolve_gate_anchor( + runs, "documentation-checkpoint", ("first", "second"), self.fetcher(jobs) + ) + self.assertEqual(anchor.sha, "a" * 40) + + def test_an_anchor_with_no_named_step_is_REFUSED(self) -> None: + """Not defaulted to the job. A job-level anchor is #863 one level down, + so the flag-less form has to be unavailable rather than lenient.""" + with self.assertRaises(ValueError): + baseline.steps_concluded([job("x", "success")], "x", ()) + + def test_the_floor_covers_the_oldest_CANDIDATES_own_head(self) -> None: + """F2's off-by-one. The floor is the run PAST the candidate window. + + With the floor set to the oldest CANDIDATE's own head, that head sat on + the wrong side of `base..head` and its commit was excluded from the very + range the floor exists to guarantee. Reading one extra run costs nothing + -- it is the same single runs call -- and puts every candidate inside. + """ + runs = [ + push_run(5, "e" * 40, "cancelled", "2026-08-23T07:52:11Z"), + push_run(4, "d" * 40, "cancelled", "2026-08-23T07:49:52Z"), + push_run(3, "c" * 40, "cancelled", "2026-08-23T07:46:52Z"), + push_run(2, "b" * 40, "cancelled", "2026-08-23T07:39:25Z"), + push_run(1, "a" * 40, "cancelled", "2026-08-23T07:19:38Z"), + ] + jobs = {n: [job("documentation-checkpoint", "cancelled")] + for n in (1, 2, 3, 4, 5)} + anchor = baseline.resolve_gate_anchor( + runs, "documentation-checkpoint", (GATE_STEP,), self.fetcher(jobs), + window=3, + ) + self.assertEqual(anchor.source, "floor") + # The candidates are e, d, c. The floor is runs[window], `b`, so `c`'s + # own head is INSIDE `b..HEAD`. There are deliberately MORE runs than + # `window + 1` here: with exactly `window + 1`, `runs[window]` and + # `runs[-1]` are the same entry and the assertion cannot tell the fixed + # code from the off-by-one. + self.assertEqual(anchor.sha, "b" * 40) + + def test_past_the_window_commits_roll_off_PERMANENTLY(self) -> None: + """F2, stated rather than denied. + + The window is a cost bound: walking further is an unbounded number of + API calls per push, on three jobs. The price is that a job which + returns no verdict for more than `window` consecutive pushes loses the + commits older than the window, and no later push ever sees them again. + Asserted here so the trade is a measured property of the code and not a + sentence in a spec that could quietly stop being true. + """ + window = 3 + heads = ["a", "b", "c", "d", "e", "f"] + runs = [ + push_run(n + 1, heads[n] * 40, "cancelled", f"2026-08-23T0{n}:00:00Z") + for n in range(len(heads)) + ] + runs.reverse() + jobs = {n: [job("documentation-checkpoint", "cancelled")] + for n in range(1, len(heads) + 1)} + anchor = baseline.resolve_gate_anchor( + runs, "documentation-checkpoint", (GATE_STEP,), self.fetcher(jobs), + window=window, + ) + # Six non-qualifying pushes, a window of three: the floor is runs[3], + # which is `c`. `a` and `b` are older than the base of every range this + # job will ever walk again. + self.assertEqual(anchor.sha, "c" * 40) + self.assertEqual(anchor.source, "floor") + + def test_gate_anchor_reads_one_run_PAST_the_window(self) -> None: + """Where the off-by-one actually lives. + + `resolve_gate_anchor` can only put the oldest candidate inside the floor + range if it was GIVEN a run older than the window. Fetching exactly + `window` runs makes `runs[window]` and `runs[-1]` the same entry, and + the floor silently becomes the oldest candidate again -- the defect, + restored, with the slicing still looking correct. + """ + seen = {} + original_push_runs = baseline.push_runs + original_jobs_for = baseline.jobs_for + original_repository = baseline.repository + baseline.repository = lambda: "mudler/vllm.cpp" + baseline.jobs_for = lambda repo, run_id: ([], None) + + def recording(repo, limit, branch): + seen["limit"] = limit + return [], None + + baseline.push_runs = recording + try: + baseline.gate_anchor("documentation-checkpoint", (GATE_STEP,), "main", 20) + finally: + baseline.push_runs = original_push_runs + baseline.jobs_for = original_jobs_for + baseline.repository = original_repository + self.assertEqual(seen["limit"], 21) + + def test_an_empty_window_resolves_to_nothing(self) -> None: + anchor = baseline.resolve_gate_anchor( + [], "documentation-checkpoint", (GATE_STEP,), None + ) + self.assertEqual(anchor.sha, "") + self.assertEqual(anchor.source, "none") + + def test_the_workflow_no_longer_queries_a_RUN_level_success(self) -> None: + """The defect itself, pinned out of the file. + + `status=success` on the runs endpoint filters on the run's conclusion. + `sanitize-cpu` is `continue-on-error`, so that field can read success + over a red job; and a run that was cancelled reads `cancelled` over a + job that ran to completion. It answers a different question in both + directions. + """ + # `code_lines` drops whole-line YAML comments: the job comments RECORD + # the retired query, and a substring test that could not tell a comment + # from a call would forbid documenting what was fixed. + self.assertNotIn( + "status=success", "\n".join(code_lines(workflow_text())) + ) + + +class PushRunsPayloadTests(unittest.TestCase): + """`push_runs` against every shape the forge hands back, through `main`. + + NOTHING EXECUTED `push_runs` BEFORE #1776's second review. Every anchor test + replaced it with a stand-in, and `gh_api` appeared in no test at all -- the + M9/M10 shape one layer down, with the stand-in moved from the shell shim to + the module function. So the chain is driven here end to end: real `main`, + real `gate_anchor`, real `push_runs`, real `jobs_for`, and only `gh_api` + faked, because it is the one function that opens a socket. + + What it holds is the rc SPLIT `ci.yml` acts on, and the split is the whole + point. rc 3 is REMOTE_UNVERIFIED and makes the gate step SKIP. rc 1 is a + clean absence and makes it fall back to `$PUSH_BASE`. Three unreadable + payloads used to exit 1: a list, a null, and GitHub's `{"message": "Not + Found"}` error object. Each of them made a diff gate walk one push, pass, + conclude, and advance its own anchor past the whole span the narrowing + dropped. + """ + + JOB = "documentation-checkpoint" + STEP = "Every feature checkpoint updates STATUS, BENCHMARKS and NOW" + + def anchor_exit(self, reply) -> tuple[int, str, str]: + """Run the real `--gate-anchor` CLI with `gh_api` faked. + + `reply` is either a fixed `(payload, degraded)` pair or a callable of + the request path, which is how the readable case answers the runs + endpoint and the jobs endpoint differently. + """ + + original_gh_api = baseline.gh_api + original_repository = baseline.repository + seen: list[str] = [] + + def fake(path: str): + seen.append(path) + return reply(path) if callable(reply) else reply + + baseline.gh_api = fake + baseline.repository = lambda: "mudler/vllm.cpp" + out, err = io.StringIO(), io.StringIO() + try: + with redirect_stdout(out), redirect_stderr(err): + code = baseline.main( + ["--gate-anchor", self.JOB, "--gate-step", self.STEP] + ) + finally: + baseline.gh_api = original_gh_api + baseline.repository = original_repository + self.assertTrue(seen, "the anchor never asked the forge anything") + self.assertIn("event=push", seen[0]) + self.assertNotIn("status=", seen[0]) + return code, out.getvalue(), err.getvalue() + + def test_a_degraded_gh_call_exits_3(self) -> None: + """The case that was already right, pinned so the fix cannot regress it.""" + code, stdout, stderr = self.anchor_exit( + (None, "REMOTE_UNVERIFIED: gh api failed") + ) + self.assertEqual(code, 3, stderr) + self.assertEqual(stdout.strip(), "") + self.assertIn("REMOTE_UNVERIFIED", stderr) + + def test_a_LIST_runs_payload_is_REMOTE_UNVERIFIED_not_an_empty_window(self) -> None: + code, stdout, stderr = self.anchor_exit(([], None)) + self.assertEqual(code, 3, stderr) + self.assertEqual(stdout.strip(), "") + self.assertIn("REMOTE_UNVERIFIED", stderr) + + def test_a_NULL_runs_payload_is_REMOTE_UNVERIFIED_not_an_empty_window(self) -> None: + code, stdout, stderr = self.anchor_exit((None, None)) + self.assertEqual(code, 3, stderr) + self.assertEqual(stdout.strip(), "") + self.assertIn("REMOTE_UNVERIFIED", stderr) + + def test_an_ERROR_OBJECT_is_REMOTE_UNVERIFIED_not_an_empty_window(self) -> None: + """`gh api` exits 0 and prints this body for a 404 on some paths, so a + dict is not on its own evidence that the question was answered.""" + code, stdout, stderr = self.anchor_exit(({"message": "Not Found"}, None)) + self.assertEqual(code, 3, stderr) + self.assertEqual(stdout.strip(), "") + self.assertIn("REMOTE_UNVERIFIED", stderr) + + def test_a_GENUINELY_empty_window_stays_a_CLEAN_absence(self) -> None: + """rc 1 and rc 3 may not be collapsed in either direction. + + A branch whose workflow has never run on a push has no anchor and never + will; skipping there would gate nothing forever. This is the case the + fix must NOT convert into REMOTE_UNVERIFIED. + """ + code, stdout, stderr = self.anchor_exit( + ({"total_count": 0, "workflow_runs": []}, None) + ) + self.assertEqual(code, 1, stderr) + self.assertEqual(stdout.strip(), "") + self.assertNotIn("REMOTE_UNVERIFIED", stderr) + + def test_a_READABLE_window_with_a_verdict_exits_0_and_prints_the_sha(self) -> None: + """The positive control. Without it the four refusals above are also + satisfied by a `push_runs` that refuses everything.""" + runs = { + "total_count": 1, + "workflow_runs": [ + {"id": 11, "head_sha": "a" * 40, "created_at": "2026-08-23T09:00:00Z"} + ], + } + jobs = { + "jobs": [ + job_with_steps( + self.JOB, "success", [(self.STEP, "success")] + ) + ] + } + + def reply(path: str): + return (jobs, None) if "/jobs" in path else (runs, None) + + code, stdout, stderr = self.anchor_exit(reply) + self.assertEqual(code, 0, stderr) + self.assertEqual(stdout.strip(), "a" * 40) + + def test_the_anchor_query_goes_through_gh_api_and_nowhere_else(self) -> None: + """The stand-in hazard, closed at its source. + + If `push_runs` ever grew its own transport, every test above would keep + passing while measuring a function the workflow no longer calls. + """ + original_gh_api = baseline.gh_api + baseline.gh_api = lambda path: (_ for _ in ()).throw( + AssertionError(f"unexpected second transport for {path!r}") + ) + try: + with self.assertRaises(AssertionError): + baseline.push_runs("mudler/vllm.cpp", 21, "main") + finally: + baseline.gh_api = original_gh_api + + +class BaselineRunsPayloadTests(unittest.TestCase): + """`baseline_runs` on the ADVISORY lane, against the same forge shapes. + + NOTHING EXECUTED `baseline_runs` BEFORE #1776's third review either. The two + tests that name it replace `collect` wholesale, one layer above it, so the + reader itself had never run. Driven here for the first time, through real + `main` and real `render`, with only `gh_api` and `repository` faked. + + What it did: it skipped any payload that was not a dict, took + `payload.get("workflow_runs", [])` from any dict that was one, contributed + nothing either way, and returned `([], None)`. `render` reads that pair as a + green field and prints "No completed baseline run found on main." followed + by an instruction to trigger the first run. + + THE RC WAS NEVER WRONG. An empty window exits non-zero on every one of these + shapes, so no consumer ever read an unreadable forge as a pass, and this is + the advisory lane rather than a gate. What was wrong is what a HUMAN was + told. `.agents/verification.md` puts it as unknown being neither absence nor + success, and `jobs_for`, one function above, already refuses exactly these + shapes with `REMOTE_UNVERIFIED`. This makes the two agree. + """ + + UNREADABLE = { + "a LIST where an object was expected": [], + "a NULL body": None, + "GitHub's error object, returned under rc 0": {"message": "Not Found"}, + } + + def run_main(self, payload) -> tuple[int, str]: + original_gh_api, original_repository = baseline.gh_api, baseline.repository + seen: list[str] = [] + + def fake(path: str): + seen.append(path) + return payload, None + + baseline.gh_api = fake + baseline.repository = lambda: "mudler/vllm.cpp" + out = io.StringIO() + try: + with redirect_stdout(out), redirect_stderr(io.StringIO()): + code = baseline.main([]) + finally: + baseline.gh_api = original_gh_api + baseline.repository = original_repository + self.assertTrue(seen, "the advisory lane never asked the forge anything") + return code, out.getvalue() + + def test_an_unreadable_window_is_NOT_reported_as_an_absence(self) -> None: + for label, payload in self.UNREADABLE.items(): + with self.subTest(shape=label): + code, text = self.run_main(payload) + self.assertNotEqual(code, 0, text) + self.assertNotIn( + "No completed baseline run found on main.", text, + f"{label}: the forge could not be read, and this reports it " + "to a human as a clean absence with an instruction to " + "trigger a run", + ) + self.assertIn("REMOTE_UNVERIFIED", text) + + def test_a_GENUINE_absence_is_still_reported_as_an_absence(self) -> None: + """The other half. Refusing every empty window would delete the message + rather than narrow it, and a branch whose first scheduled run has not + happened is a real and common state.""" + code, text = self.run_main({"workflow_runs": []}) + self.assertNotEqual(code, 0, text) + self.assertIn("No completed baseline run found on main.", text) + self.assertNotIn("REMOTE_UNVERIFIED", text) + + +class AnchorCycleConstructionTests(unittest.TestCase): + """The feedback loop of #1773, CONSTRUCTED rather than read. + + Six pushes P1..P6. P1's run is fully green. P2 lands a commit that a + diff-scoped gate flags, so from P2 onward no run is ever `success` again -- + which is the premise, not an assumption: the gate is what would have made it + success. Under the run-level rule the anchor is pinned at P1 forever and + each push walks a wider range that re-includes P2. Under the per-job rule + the gate returns a verdict on every push, so the anchor advances and P2 is + reported once. + """ + + PUSHES = ["p1", "p2", "p3", "p4", "p5", "p6"] + + def runs_after(self, index: int) -> list[dict]: + """The payload as it stands when push number `index` starts, newest first. + + Every run from P2 onward concludes `failure`, because the diff gate in + it is red. P1 is the last `success`. + """ + entries = [] + for position in range(index): + conclusion = "success" if position == 0 else "failure" + entries.append( + push_run(position + 1, self.PUSHES[position], conclusion, + f"2026-08-23T0{position}:00:00Z") + ) + return list(reversed(entries)) + + def jobs_after(self, index: int) -> dict: + return { + position + 1: [ + job("documentation-checkpoint", + "success" if position == 0 else "failure") + ] + for position in range(index) + } + + def commits_after(self, base: str, head: str) -> list[str]: + """`base..head` over the linear push sequence.""" + order = self.PUSHES + start = order.index(base) + 1 if base in order else 0 + return order[start:order.index(head) + 1] + + def test_run_level_anchor_widens_across_pushes(self) -> None: + """THE CYCLE. The anchor never moves and the range grows without bound. + + This is the rule being removed, so it passes before and after; what it + pins is the SHAPE of the defect the replacement has to break. + """ + widths, always_reincluded = [], [] + for index in range(1, len(self.PUSHES)): + head = self.PUSHES[index] + base = run_level_anchor(self.runs_after(index)) + self.assertEqual(base, "p1") + commits = self.commits_after(base, head) + widths.append(len(commits)) + always_reincluded.append("p2" in commits) + self.assertEqual(widths, [1, 2, 3, 4, 5]) + self.assertTrue(all(always_reincluded)) + + def test_per_job_anchor_reports_the_violation_once(self) -> None: + """The exit condition no longer requires the thing it blocks.""" + reports = [] + for index in range(1, len(self.PUSHES)): + head = self.PUSHES[index] + runs = self.runs_after(index) + jobs = self.jobs_after(index) + anchor = baseline.resolve_gate_anchor( + runs, "documentation-checkpoint", (GATE_STEP,), + lambda run_id: jobs[run_id] + ) + commits = self.commits_after(anchor.sha, head) + self.assertEqual(len(commits), 1, f"range at {head}: {commits}") + if "p2" in commits: + reports.append(head) + self.assertEqual(reports, ["p2"]) + + # (job conclusion, gate-step conclusion) for P1..P6. + # + # P2 onward is the LIVE shape of `commit-protocol-tag` over runs + # 32601353990..32623377380: an earlier step of the job failed, GitHub + # concluded the JOB `failure` and marked every remaining step `skipped`, + # and the diff-scoped gate never executed. P4 is #863's original hole, a + # cancelled job, so both shapes are modelled in one sequence. + STEP_PROFILE = [ + ("success", "success"), + ("failure", "skipped"), + ("failure", "skipped"), + ("cancelled", "skipped"), + ("failure", "failure"), + ("failure", "failure"), + ] + + def profile_runs(self, index: int) -> list[dict]: + entries = [ + push_run( + position + 1, + self.PUSHES[position], + self.STEP_PROFILE[position][0], + f"2026-08-23T0{position}:00:00Z", + ) + for position in range(index) + ] + return list(reversed(entries)) + + def profile_jobs(self, index: int) -> dict: + return { + position + 1: [ + job_with_steps( + "commit-protocol-tag", + self.STEP_PROFILE[position][0], + [ + ("Resolve this job's own diff anchor (#1773)", "success"), + (GATE_STEP, self.STEP_PROFILE[position][1]), + ], + ) + ] + for position in range(index) + } + + @staticmethod + def anchor(runs, job_name, step_names, fetch): + """The rule under test, called through one adapter. + + The adapter exists so the red-before and the green-after assert BYTE + IDENTICAL properties: what changes between them is whether the anchor + is allowed to see `step_names`, which is the whole fix. + """ + + return baseline.resolve_gate_anchor(runs, job_name, step_names, fetch) + + def test_no_commit_is_ever_skipped(self) -> None: + """Every commit gets a verdict from a run that actually RAN the gate. + + The union of the ranges is NOT the property. A job-level anchor keeps + that union whole while the gate step is skipped on every push, because + `failure` advances the anchor whether or not the gate executed. The + property is narrower and is the one #863 is about: for every commit + there is at least one push whose range contains it AND whose gate step + returned a verdict. A range walked by a step that never ran measures + nothing. + """ + + covered: set[str] = set() + for index in range(1, len(self.PUSHES)): + head = self.PUSHES[index] + jobs = self.profile_jobs(index) + anchor = self.anchor( + self.profile_runs(index), + "commit-protocol-tag", + (GATE_STEP,), + lambda run_id: jobs[run_id], + ) + if anchor.source == "none": + continue + if self.STEP_PROFILE[index][1] not in {"success", "failure"}: + # The gate did not EXECUTE on this push. Its range is not a + # verdict about anything, so it may not count as coverage. + continue + covered.update(self.commits_after(anchor.sha, head)) + self.assertEqual( + covered, + set(self.PUSHES[1:]), + "commits with no verdict from any run that ran the gate: " + f"{sorted(set(self.PUSHES[1:]) - covered)}", + ) + + +class AnchorStepTests(unittest.TestCase): + """The three diff-scoped jobs resolve an anchor NAMED AFTER THEMSELVES. + + One shared anchor cannot be right for three jobs with three different + cancellation profiles: `agent-record` carries a cancellable group of its + own, and the other two do not. It is also what made run 32625264281 + pathological -- `last-gated-commit` was cancelled while its `if: always()` + consumers ran, so the anchor rendered EMPTY, the step fell back to + `PUSH_BASE`, and the gate passed over a single push. The verdict on `main` + was decided by which job won a cancellation race. Resolving inside the + consuming job removes the race by removing the second job. + """ + + JOBS = ("agent-record", "documentation-checkpoint", "commit-protocol-tag") + + def anchor_steps(self, job_name: str) -> list[list[str]]: + block = job_block(workflow_text(), job_name) + return [ + step for step in steps_of(block) + if "--gate-anchor" in "\n".join(step) + ] + + def test_each_diff_scoped_job_resolves_its_own_anchor(self) -> None: + for job_name in self.JOBS: + with self.subTest(job=job_name): + steps = self.anchor_steps(job_name) + self.assertTrue(steps, f"{job_name} resolves no anchor of its own") + body = "\n".join(steps[0]) + self.assertIn(f"--gate-anchor {job_name}", body) + + def test_the_anchor_step_runs_the_query_and_exports_it(self) -> None: + block = job_block(workflow_text(), "documentation-checkpoint") + step = next(s for s in steps_of(block) if "--gate-anchor" in "\n".join(s)) + body = step_run_body(step) + self.assertIsNotNone(body) + code, argv, output = run_shimmed( + body, + { + "EVENT_NAME": "push", + "BRANCH": "main", + "GITHUB_ENV": "/dev/null", + }, + ) + self.assertEqual(code, 0, output) + self.assertEqual( + argv, + [[ + "scripts/main-baseline.py", + "--gate-anchor", + "documentation-checkpoint", + "--branch", + "main", + "--gate-step", + "Every feature checkpoint updates STATUS, BENCHMARKS and NOW", + "--gate-step", + "Every commit in the range arrived on a task branch", + ]], + output, + ) + + def test_the_anchor_step_does_not_query_on_the_pull_request_lane(self) -> None: + """A pull request has no gated `main` history to anchor on, and its own + base is authoritative. Querying there would spend an API call to + resolve a value nothing reads.""" + block = job_block(workflow_text(), "documentation-checkpoint") + step = next(s for s in steps_of(block) if "--gate-anchor" in "\n".join(s)) + code, argv, output = run_shimmed( + step_run_body(step), + {"EVENT_NAME": "pull_request", "BRANCH": "main", "GITHUB_ENV": "/dev/null"}, + ) + self.assertEqual(code, 0, output) + self.assertEqual(argv, [], output) + + # ------------------------------------------------------------------ + # The STEP is the unit (#1776) + # ------------------------------------------------------------------ + + def gate_step_names(self, job_name: str) -> list[str]: + """The step names this job's anchor query claims a verdict from.""" + + body = "\n".join(self.anchor_steps(job_name)[0]) + return re.findall(r'--gate-step "([^"]+)"', body) + + def named_steps(self, job_name: str) -> dict[str, list[str]]: + block = job_block(workflow_text(), job_name) + found = {} + for step in steps_of(block): + match = re.match(r" - name: (.*)", step[0]) + if match: + found[match.group(1)] = step + return found + + def test_every_named_gate_step_exists_in_its_own_job(self) -> None: + """The rename hazard, closed. A `--gate-step` value that names nothing + would make `steps_concluded` refuse every run and pin the anchor at the + floor forever -- silent, and only visible as a slowly widening range.""" + for job_name in self.JOBS: + with self.subTest(job=job_name): + names = self.named_steps(job_name) + wanted = self.gate_step_names(job_name) + self.assertTrue(wanted, f"{job_name} names no gate step") + for step_name in wanted: + self.assertIn(step_name, names) + + def test_every_step_that_READS_the_anchor_is_NAMED_by_it(self) -> None: + """The population, not a sample. + + A diff-scoped step the anchor does not name is a gate whose verdict + nothing waits for: the anchor advances on the steps it does name, and + this one's range moves out from under it. Selecting the population by + "reads GATE_ANCHOR" is what makes a newly added diff-scoped gate fail + here instead of landing uncovered. + """ + for job_name in self.JOBS: + with self.subTest(job=job_name): + wanted = set(self.gate_step_names(job_name)) + for name, step in self.named_steps(job_name).items(): + body = step_run_body(step) or "" + if "GATE_ANCHOR" not in body or "--gate-anchor" in body: + continue + self.assertIn( + name, wanted, + f"{job_name}: step {name!r} walks the anchor's range but " + "no --gate-step names it, so nothing waits for its verdict", + ) + + def guard_of(self, job_name: str, step_name: str) -> str: + """One gate step's `if:`, from the PARSED workflow. + + Parsed rather than grepped out of the raw lines, because a guard is + written over three physical lines and YAML folds it; reassembling it by + hand is how a test ends up holding a shape rather than a value. + """ + + import yaml + + ci = yaml.safe_load(workflow_text()) + for step in ci["jobs"][job_name]["steps"]: + if step.get("name") == step_name: + condition = step.get("if") + self.assertIsNotNone( + condition, + f"{job_name}: gate step {step_name!r} carries no `if:` at all, " + "so it runs on a degraded read and narrows the range", + ) + return condition + raise AssertionError(f"{job_name}: no step named {step_name!r}") + + # Every state a gate step's guard has to decide, and what it must decide. + # `cancelled()` is the run's cancellation, the two outcomes are the steps + # this one depends on, and the environment value is what the anchor step + # exports. A gate step may run in exactly ONE of these sixteen states. + GUARD_STATES = tuple( + ( + { + "cancelled()": cancelled, + "steps.checkout.outcome": checkout, + "steps.anchor.outcome": anchor, + "env.GATE_ANCHOR_DEGRADED": degraded, + }, + not cancelled + and checkout == "success" + and anchor == "success" + and degraded != "true", + ) + for cancelled in (False, True) + for checkout in ("success", "failure") + for anchor in ("success", "failure") + for degraded in ("", "true") + ) + + def test_every_gate_step_SKIPS_rather_than_narrows(self) -> None: + """The guard RESOLVED, over every state it has to decide. + + This test checked four substrings until #1776's second review, and the + mutation that walked through it is the one the module docstring already + names one level up: append `|| true` to all five guards. Every substring + survives byte-for-byte, the whole conjunction becomes the constant + `true`, and the gate step then runs on a degraded read -- `GATE_ANCHOR` + empty, so `base` falls back to `$PUSH_BASE`, a one-push range passes, + the step concludes `success`, and the anchor advances past the entire + span the narrowing dropped. That is the defect this row exists to + remove, restored in full and invisible. + + So resolve it. Sixteen states, one boolean each: + + - `cancelled()` false, checkout `success`, anchor `success`, degraded + not `'true'` -- and ONLY then -- the gate runs. + - anything else -- the run is being cancelled, there is no tree to walk, + the resolver failed, or the base is UNKNOWN -- the gate SKIPS. A skip + is not a verdict, so the anchor does not advance and the next readable + run walks the span whole. + + `!cancelled()` is the term that closes the original F1: without it an + earlier step's failure marks this one `skipped`, and a `skipped` step + never returns the verdict the anchor waits for while the JOB concludes + `failure` and a job-level rule walks straight past. It is now asserted + by its EFFECT -- drop it and the eight `cancelled()`-true states resolve + the wrong way. + + The evaluator raises on any context or status function `GUARD_STATES` + does not model, so a guard that grows a new term reds here rather than + resolving to something plausible. + """ + for job_name in self.JOBS: + names = self.named_steps(job_name) + for step_name in self.gate_step_names(job_name): + condition = self.guard_of(job_name, step_name) + self.assertIn(step_name, names) + for state, expected in self.GUARD_STATES: + with self.subTest(job=job_name, step=step_name, state=state): + self.assertEqual( + resolve_condition(condition, state), + expected, + f"{job_name}/{step_name}: guard {condition!r} resolves " + f"{not expected} in state {state}", + ) + + def test_one_diff_scoped_checker_per_gate_step(self) -> None: + """`documentation-checkpoint`'s regression, pinned shut. + + Its gate ran `check-now-current.py` and `check-role-discipline.py` in + ONE step under `set -eu`. A `check-now-current` failure aborted before + the arrival gate ran, and the step still concluded -- so the step's + conclusion was not a verdict about the checker the anchor was waiting + for. Two gates in one step is that hazard, whatever the two are. + """ + for job_name in self.JOBS: + names = self.named_steps(job_name) + for step_name in self.gate_step_names(job_name): + with self.subTest(job=job_name, step=step_name): + body = step_run_body(names[step_name]) or "" + checkers = set(re.findall(r"scripts/(check-[a-z0-9-]+\.py)", body)) + self.assertLessEqual(len(checkers), 1, sorted(checkers)) + if not checkers: + continue + first = re.search(r"python3 (\S+)", body) + self.assertIsNotNone(first) + self.assertEqual( + f"scripts/{checkers.pop()}", first.group(1), + f"{step_name}: a DIFFERENT python3 gate runs before this " + "step's own, so the step can conclude without it", + ) + + def test_nothing_fallible_PRECEDES_the_gate_in_its_own_body(self) -> None: + """The half of the shape above that it did not hold, and two mutations. + + Spec §4.3 item 3 claimed this test held "that nothing fallible precedes + the checker in its body". It did not. It read the first `python3 ` + and compared it with the step's one checker, so anything that was not + `python3` was invisible, and a step that runs no `scripts/check-*.py` at + all left the population through `if not checkers: continue`. Both halves + were confirmed by mutation on #1776: + + - insert `git fetch -q origin +refs/heads/main:refs/remotes/origin/main` + before `python3 scripts/check-now-current.py` -- 99 tests, OK; + - insert the same line into `Every new commit carries + FOLLOWING_AGENTS_PROTOCOL`, whose whole body IS an inline shell gate + and which named no checker -- 99 tests, OK. + + Under `set -eu` the inserted command fails, the step aborts, GitHub + concludes it `failure`, and `steps_concluded` reads that as a verdict -- + the anchor then advances over a range the gate never looked at. That is + the `documentation-checkpoint` defect again, inside a single step. + + The property, held rather than described: a gate step's body runs the + RANGE PRELUDE and then its GATE, and nothing else in between. The + prelude may bind variables, and may call `set`, `[`, `echo` and `:` -- + a shell option, a test, a message, and a no-op. Nothing in it may run a + command substitution, and a `[` must have its status consumed. The + first COMMAND that is not one of those four is the gate, and it must + CONSUME the range: reference `$base`. A command that runs before the + gate and does not touch the range is by construction not part of + computing it, and both mutations are exactly that shape. + + The population is every gate step, INCLUDING the one whose gate is + inline shell rather than a checker. `commit-protocol-tag`'s + `FOLLOWING_AGENTS_PROTOCOL` walk resolves its gate to + `git cat-file -e "${base}^{commit}"`, and `commit-protocol-tag`'s strict + trailer step to the same probe -- both consume the range, both pass, and + neither is exempt any more. + + WHAT THIS HOLDS, and it is narrower than the first version SAID. Round 3 + found three escapes past that version, each aborting the step under + `set -eu` while the suite reported `Ran 109`, `OK`: + + - `base="$(git rev-parse --verify "${base}^{commit}")"`, rc 128 once the + anchor is not in the checkout -- and normalising the anchor right + before the gate is the natural next edit here, so this is the + realistic one; + - `_p="$(git fetch -q origin nope)"` and its backtick spelling, rc 128. + The substitution refusal below could not see any of the three, because + the parser dropped every `name=...` statement BEFORE that refusal ran; + - `[ -f .git/NO_SUCH ]`, rc 1. `[` was allowlisted on the reasoning that + "its failure the surrounding `if`/`||` consumes", but nothing checked + that a given `[` HAS one, and a bare failing test aborts. + + So the refusal now runs over EVERY statement before the gate, + assignments included, and a `[` is admitted only where an `if`/`while` + header or an `||`/`&&` reads its status -- `MUST_BE_CONSUMED`. + + RESIDUAL, stated rather than hidden, and it is two things now. + + 1. A statement inserted before the gate that itself references `$base` + is not distinguishable from the gate. One gate per step is asserted + above, which bounds but does not close this. + 2. The allowlist is a judgement about `set`, `echo` and `:`, not a + proof. `[` was the member with a routine failing form and it is now + consumption-checked; the other three are admitted because their + failing forms here -- a bad `set -o`, a write error on the step log + -- are not shapes an editor of this file produces by accident. A + fourth member added later needs the same judgement made again. + """ + for job_name in self.JOBS: + names = self.named_steps(job_name) + for step_name in self.gate_step_names(job_name): + with self.subTest(job=job_name, step=step_name): + body = step_run_body(names[step_name]) or "" + self.assertTrue(body, f"{step_name}: gate step has no run body") + gate = None + for entry in statements_in(body): + if (entry.kind == "command" + and entry.head not in RANGE_PRELUDE_COMMANDS): + gate = (entry.head, entry.text) + break + for token, name in (("$(", "command"), ("`", "backtick")): + self.assertNotIn( + token, entry.text, + f"{job_name}/{step_name}: {name} substitution in " + f"{entry.text!r} runs a fallible call BEFORE the " + "gate -- it aborts the step under `set -eu` and " + "the step concludes `failure` over a gate that " + "never ran", + ) + if entry.kind == "command" and entry.head in MUST_BE_CONSUMED: + self.assertTrue( + entry.consumed, + f"{job_name}/{step_name}: {entry.text!r} runs " + "before the gate and NOTHING consumes its exit " + "status, so a false test aborts the step under " + "`set -eu` before its gate executes", + ) + self.assertIsNotNone( + gate, f"{job_name}/{step_name}: no gate runs in this step" + ) + head, statement = gate + self.assertRegex( + statement, r"\$\{?base\b", + f"{job_name}/{step_name}: {head!r} runs before the gate and " + "does not consume the range, so this step can abort under " + "`set -eu` and CONCLUDE without its gate ever executing -- " + "and the anchor then advances over commits nothing checked", + ) + + def test_a_DEGRADED_query_skips_the_gate_instead_of_narrowing_it(self) -> None: + """F3, executed. rc 3 is REMOTE_UNVERIFIED and it is not a pass. + + `|| true` used to swallow it: the anchor rendered empty, the gate fell + back to `PUSH_BASE`, passed over one push, and its success advanced this + step's own anchor past every commit the narrowing dropped. The anchor + step now exports `GATE_ANCHOR_DEGRADED=true`, which the gate steps' + `if:` turns into a SKIP. + """ + block = job_block(workflow_text(), "documentation-checkpoint") + step = next(s for s in steps_of(block) if "--gate-anchor" in "\n".join(s)) + with tempfile.TemporaryDirectory(prefix="vllm-gate-env-") as temporary: + github_env = Path(temporary) / "env" + github_env.write_text("", encoding="utf-8") + code, _, output = run_shimmed( + step_run_body(step), + {"EVENT_NAME": "push", "BRANCH": "main", + "GITHUB_ENV": str(github_env)}, + python_exit=3, + ) + exported = github_env.read_text(encoding="utf-8") + self.assertEqual(code, 0, output) + self.assertIn("GATE_ANCHOR=\n", exported) + self.assertIn("GATE_ANCHOR_DEGRADED=true", exported) + self.assertIn("REMOTE_UNVERIFIED", output) + + def test_a_CLEAN_absence_still_falls_back_to_push_base(self) -> None: + """rc 1 is not rc 3, and collapsing them would be its own defect. + + A branch with no gated history has no anchor and never will; skipping + there would gate nothing forever. `PUSH_BASE` is the honest base in that + case, and the run is not degraded. + """ + block = job_block(workflow_text(), "documentation-checkpoint") + step = next(s for s in steps_of(block) if "--gate-anchor" in "\n".join(s)) + with tempfile.TemporaryDirectory(prefix="vllm-gate-env-") as temporary: + github_env = Path(temporary) / "env" + github_env.write_text("", encoding="utf-8") + code, _, output = run_shimmed( + step_run_body(step), + {"EVENT_NAME": "push", "BRANCH": "main", + "GITHUB_ENV": str(github_env)}, + python_exit=1, + ) + exported = github_env.read_text(encoding="utf-8") + self.assertEqual(code, 0, output) + self.assertIn("GATE_ANCHOR=\n", exported) + self.assertIn("GATE_ANCHOR_DEGRADED=\n", exported) + + def test_every_anchor_job_may_read_the_actions_api(self) -> None: + import yaml + ci = yaml.safe_load(workflow_text()) + for job_name in self.JOBS: + with self.subTest(job=job_name): + permissions = ci["jobs"][job_name].get("permissions") + self.assertIsNotNone( + permissions, + f"{job_name} queries the Actions API and declares no scope", + ) + self.assertEqual(permissions.get("actions"), "read") + + def test_the_gate_still_falls_back_when_the_query_returns_nothing(self) -> None: + """A rate-limited or failed query degrades to today's behaviour, never + to an empty range that passes vacuously.""" + block = job_block(workflow_text(), "documentation-checkpoint") + step = next( + s for s in steps_of(block) if "check-now-current.py" in "\n".join(s) + ) + self.assertIn('base="$PUSH_BASE"', step_run_body(step)) + + if __name__ == "__main__": unittest.main(verbosity=2)