From 432b3407c8650b74ade80a62d0eacb4eac6e8596 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sun, 23 Aug 2026 09:22:18 +0000 Subject: [PATCH 01/12] spec(GATE-ANCHOR-PER-JOB): a diff gate must anchor on its OWN last verdict, not on a run-level success (#1773) `main` cannot return to green by itself. `last-gated-commit` resolves the base of every diff-scoped gate with a RUN-level `status=success` query, and a successful run is what those gates are blocking, so a red `main` widens its own range and keeps re-flagging the commits blocking it. Measured at this base. The query returns `fafa16f0f` from 2026-08-12 -- a 484-commit range -- while `documentation-checkpoint` itself concluded `success` at `1fdd3e26d` eleven days later, inside run `32625264281` whose own conclusion is `cancelled`. That is the per-job/run-level distinction `scripts/main-baseline.py` has documented since #274, in the module the job asking the question never calls. The spec also records a second defect, sharper than the cycle: `last-gated-commit` is itself cancellable and its 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 a single push. `main`'s diff gates are therefore green when the resolver dies and red when it lives, and neither reading is about the tree. It refutes the external-fork hypothesis carried by #1764 and #1773. `check-role-discipline.py` reads no ref: for a single-parent commit `arrives_via_row_pr` is commit-message text only. The one non-fork commit of the five being re-flagged fails identically. The real discriminator is the squash subject, which lacks `(#N)` because the merger supplied an explicit `commit_title`. Spec only. The implementation follows in this branch. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- .agents/issue-index.md | 1 + .agents/specs/gate-anchor-per-job.md | 315 +++++++++++++++++++++++++++ 2 files changed, 316 insertions(+) create mode 100644 .agents/specs/gate-anchor-per-job.md diff --git a/.agents/issue-index.md b/.agents/issue-index.md index 53594f411..80d6bcb58 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -652,3 +652,4 @@ 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)` because the merger supplied an explicit `commit_title`, which suppresses the append GitHub otherwise makes even under `squash_merge_commit_title = PR_TITLE`. FIXED IN FLOW by the anchor, not by the rule: each diff-scoped job resolves its own anchor from the last push run in which THAT job concluded, and the checker is untouched. Spec [gate-anchor-per-job.md](specs/gate-anchor-per-job.md) | bug | diff --git a/.agents/specs/gate-anchor-per-job.md b/.agents/specs/gate-anchor-per-job.md new file mode 100644 index 000000000..c8012d57b --- /dev/null +++ b/.agents/specs/gate-anchor-per-job.md @@ -0,0 +1,315 @@ +# 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) +**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.** + +## 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 +append is suppressed only when the merger supplies an explicit `commit_title`, +which is what happened to this batch of five on 2026-08-18 — three of them say +so in their own bodies, under "Maintainer changes on top". + +**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 ` walks the newest `--limit` +(default 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}`. + +**The window is the floor.** When no run in the window qualifies, the anchor is +the `head_sha` of the **oldest** run in the window. A range can therefore never +widen past 20 pushes however long `main` has been red, and the degradation is +toward *more* coverage rather than less — the failure this floor must not have +is silently skipping commits. + +When the query degrades or the branch has no push run at all, the anchor is +empty and the consumer keeps today's `PUSH_BASE` fallback. `REMOTE_UNVERIFIED` +stays what it is: not a pass, and not a claim of absence. + +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 + +`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 per-job +anchor closes exactly that hole and nothing else: from the last run in which the +job concluded, the chain is unbroken by construction, and the span since then is +walked whole. §6's `test_no_commit_is_ever_skipped` asserts the union property +over a synthetic push sequence rather than arguing it. + +## 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. +4. **`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 + +Recorded in `## Outcome` on completion: the red output of each new case before +the change, green after, each mutation's `git diff --stat` and interpreter +output, and the byte-for-byte restore proof. + +## 9. 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. From 26def4c8f59927139eb593bb3c8f02e220a4ac45 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sun, 23 Aug 2026 09:31:10 +0000 Subject: [PATCH 02/12] fix(GATE-ANCHOR-PER-JOB): each diff gate anchors on its own last verdict, so a red main can return to green (#1773) `last-gated-commit` resolved one shared base for every diff-scoped gate from `repos/.../runs?branch=main&event=push&status=success&per_page=1`. `status=` filters on the RUN's conclusion, which `scripts/main-baseline.py` has said since #274 must never be read: `sanitize-cpu` is `continue-on-error`, so that field reads success over a red job, and a cancelled run reads `cancelled` over a job that ran to completion. And because a successful run was exactly what a red gate blocked, the anchor froze while every later push widened `LAST_GREEN..HEAD`, re-including the commits being flagged. The exit condition required the thing it blocked. Measured at `21abaf169`. The query returns `fafa16f0f` from 2026-08-12, a 484-commit range, and `check-role-discipline.py` over it reports five commits from 2026-08-18. Meanwhile `documentation-checkpoint` itself concluded `success` at `1fdd3e26d` on 2026-08-23, inside run `32625264281` whose own conclusion is `cancelled`. That run also shows the second defect, which is a race rather than a cycle. The gates carry `if: always()`, so they outlive their own resolver: in `32625264281` `last-gated-commit` was cancelled, the shared string rendered empty, the step fell back to `PUSH_BASE` and PASSED over a single push. `main`'s diff gates were green when the resolver died and red when it lived, and neither reading was about the tree. `scripts/main-baseline.py` gains `--gate-anchor JOB`, in the module that already owns the per-job doctrine. It walks the newest 20 push runs and returns the head of the first one in which every payload entry for that job carries a conclusion. `failure` counts: the range decides which commits are inspected, never what is demanded of them, and a commit on `main` is immutable, so anchoring on `success` turns one violation into a permanent block while the gate loses the ability to report on new commits. One alarm per violation is a complete alarm. The pull request lane still blocks, over `base..head`, unchanged. Each of `agent-record`, `documentation-checkpoint` and `commit-protocol-tag` now resolves an anchor naming ITSELF, in its own first step. One anchor could not have been right for three jobs anyway: measured live, `documentation-checkpoint` resolves `ff8f72807` (2 commits) while `agent-record` resolves `66d1b0a90` (13), because they have different cancellation profiles. `last-gated-commit` keeps only the closed-pull-request guard `agent-record` inherits through `needs:` (#873). When no run in the window qualifies the anchor is the window's OLDEST head, so a range can never widen past 20 pushes and degrades toward more coverage rather than less; a failed query keeps the `PUSH_BASE` fallback. `check-role-discipline.py` is NOT touched. The external-fork hypothesis carried by #1764 and #1773 is refuted: the checker resolves no ref, and the one non-fork commit of the five fails identically. What the five share is a squash subject with no `(#N)`, because the merger supplied an explicit `commit_title` and suppressed the append GitHub makes even under `squash_merge_commit_title = PR_TITLE`. AGENTS.md gains that landing rule, which is where the defect was introduced. Widening the match to the whole commit body was rejected and is now held shut by a test: AGENTS.md requires every change to name an issue, so a body-wide match would pass every direct-to-main push ever made. Two existing assertions are re-pinned rather than relaxed. `test_every_diff_scoped_step_bases_on_the_last_gated_commit` and `test_the_diff_scoped_base_is_the_last_gated_commit` named the shared string; they now demand that each step consume an anchor its OWN job resolved, which is strictly stricter, and a new test refuses a shared value in the guard job. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- .github/workflows/ci.yml | 246 ++++++++--- AGENTS.md | 10 + scripts/main-baseline.py | 182 ++++++++ tests/scripts/test_check_role_discipline.py | 69 ++++ tests/scripts/test_main_baseline.py | 435 +++++++++++++++++++- 5 files changed, 862 insertions(+), 80 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 51e6f7e18..7cf537263 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -148,15 +148,55 @@ 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 with: fetch-depth: 0 + - name: Resolve this job's own diff anchor (#1773) + # The base this job walks is "the commit after which THIS job has not + # yet returned a verdict", and it is resolved HERE, by the job that + # walks it. + # + # 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. + # + # A failed or rate-limited query prints nothing, and the gate step keeps + # its `PUSH_BASE` fallback. REMOTE_UNVERIFIED is not a pass. + env: + GH_TOKEN: ${{ github.token }} + GITHUB_REPOSITORY: ${{ github.repository }} + EVENT_NAME: ${{ github.event_name }} + BRANCH: ${{ github.ref_name }} + run: | + set -eu + anchor="" + if [ "$EVENT_NAME" = "push" ]; then + anchor="$(python3 scripts/main-baseline.py \ + --gate-anchor agent-record --branch "$BRANCH" || true)" + fi + echo "GATE_ANCHOR=$anchor" >> "$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, @@ -493,7 +533,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 +543,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 +627,54 @@ 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 with: fetch-depth: 0 + - name: Resolve this job's own diff anchor (#1773) + # The base this job walks is "the commit after which THIS job has not + # yet returned a verdict", and it is resolved HERE, by the job that + # walks it. + # + # 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. + # + # A failed or rate-limited query prints nothing, and the gate step keeps + # its `PUSH_BASE` fallback. REMOTE_UNVERIFIED is not a pass. + env: + GH_TOKEN: ${{ github.token }} + GITHUB_REPOSITORY: ${{ github.repository }} + EVENT_NAME: ${{ github.event_name }} + BRANCH: ${{ github.ref_name }} + run: | + set -eu + anchor="" + if [ "$EVENT_NAME" = "push" ]; then + anchor="$(python3 scripts/main-baseline.py \ + --gate-anchor documentation-checkpoint --branch "$BRANCH" || true)" + fi + echo "GATE_ANCHOR=$anchor" >> "$GITHUB_ENV" - name: Every feature checkpoint updates STATUS, BENCHMARKS and NOW 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,11 +682,14 @@ 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 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 @@ -618,60 +701,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 +750,64 @@ 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 with: fetch-depth: 0 + - name: Resolve this job's own diff anchor (#1773) + # The base this job walks is "the commit after which THIS job has not + # yet returned a verdict", and it is resolved HERE, by the job that + # walks it. + # + # 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. + # + # A failed or rate-limited query prints nothing, and the gate step keeps + # its `PUSH_BASE` fallback. REMOTE_UNVERIFIED is not a pass. + env: + GH_TOKEN: ${{ github.token }} + GITHUB_REPOSITORY: ${{ github.repository }} + EVENT_NAME: ${{ github.event_name }} + BRANCH: ${{ github.ref_name }} + run: | + set -eu + anchor="" + if [ "$EVENT_NAME" = "push" ]; then + anchor="$(python3 scripts/main-baseline.py \ + --gate-anchor commit-protocol-tag --branch "$BRANCH" || true)" + fi + echo "GATE_ANCHOR=$anchor" >> "$GITHUB_ENV" - name: Every new commit carries FOLLOWING_AGENTS_PROTOCOL 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 @@ -742,7 +850,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 +861,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..e526e7f23 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -546,6 +546,16 @@ 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: it resolves no ref and reads nothing but the +commit message. Supplying an explicit `commit_title` when merging suppresses the +append, and the change lands looking exactly like a direct push to `main` — on a +commit nobody can retroactively repair. Five commits from 2026-08-18 are that +mistake, 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 title is. + Every commit contains a bare `FOLLOWING_AGENTS_PROTOCOL` paragraph and these trailers: diff --git a/scripts/main-baseline.py b/scripts/main-baseline.py index f81e593e7..a896f1056 100755 --- a/scripts/main-baseline.py +++ b/scripts/main-baseline.py @@ -33,6 +33,18 @@ 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 # used by CI + +THE ANCHOR (#1773). The last mode answers a second question with the same +doctrine: from which commit onward has one diff-scoped JOB 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. """ from __future__ import annotations @@ -313,6 +325,129 @@ 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 job_concluded(jobs: list[dict], job_name: str) -> bool: + """Did `job_name` return a verdict in this run? + + EVERY matching entry must have concluded. A matrix reports one entry per + lane, and half a matrix is half a verdict. + """ + + entries = [ + entry for entry in jobs if job_matches(job_name, entry.get("name", "")) + ] + if not entries: + return False + return all(entry.get("conclusion") in CONCLUDED for entry in entries) + + +def resolve_gate_anchor(runs: list[dict], job_name: str, fetch_jobs) -> Anchor: + """The anchor for `job_name` over `runs`, newest first. + + `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 job did not conclude". Absence of information is not information. + """ + + if not runs: + return Anchor(sha="", source="none") + for run in runs: + jobs = fetch_jobs(run.get("id", 0)) + if jobs is None: + return Anchor( + sha="", + source="none", + degraded="REMOTE_UNVERIFIED: jobs unreadable", + ) + if job_concluded(jobs, job_name): + return Anchor( + sha=run.get("head_sha", ""), + run_id=run.get("id", 0), + source="verdict", + ) + oldest = runs[-1] + # THE FLOOR, and the direction it fails in. The oldest head in the window is + # the WIDEST honest base; `github.event.before` would be the narrowest and + # would skip every commit in between, which is #863. Degrading toward more + # coverage is the only degradation a coverage gate may have. + return Anchor(sha=oldest.get("head_sha", ""), run_id=oldest.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. + """ + + payload, degraded = gh_api( + f"repos/{repo}/actions/workflows/{WORKFLOW_FILE}/runs" + f"?branch={branch}&event=push&per_page={limit}" + ) + if degraded: + return [], degraded + runs = payload.get("workflow_runs", []) if isinstance(payload, dict) else [] + runs.sort(key=lambda run: run.get("created_at", ""), reverse=True) + return runs[:limit], None + + +def gate_anchor(job_name: str, branch: str, window: int) -> Anchor: + """`resolve_gate_anchor` against the live forge.""" + + repo = repository() + runs, degraded = push_runs(repo, window, 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, fetch) + + # -------------------------------------------------------------------------- # Rendering # -------------------------------------------------------------------------- @@ -452,8 +587,55 @@ 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("--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 args.offline: + print("REMOTE_UNVERIFIED: --offline", file=sys.stderr) + return 1 + anchor = gate_anchor(args.gate_anchor, 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: + return 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..b1f2b8748 100644 --- a/tests/scripts/test_check_role_discipline.py +++ b/tests/scripts/test_check_role_discipline.py @@ -183,6 +183,75 @@ def test_synthetic_merge_of_an_unnamed_branch_does_not_arrive(self) -> None: ) +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 = (ROOT / "scripts/check-role-discipline.py").read_text( + encoding="utf-8" + ) + for forbidden in ("ls-remote", "for-each-ref", "origin/"): + self.assertNotIn( + forbidden, + source, + "arrival is decided from the commit message; a ref lookup here " + "would make the fork hypothesis testable, and it 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..e8edcfb14 100644 --- a/tests/scripts/test_main_baseline.py +++ b/tests/scripts/test_main_baseline.py @@ -1493,16 +1493,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 +1521,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 +1544,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 +1758,379 @@ 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", 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", 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", 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.""" + 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", self.fetcher(jobs) + ) + self.assertNotEqual(anchor.source, "verdict") + self.assertEqual(anchor.sha, "a" * 40) + self.assertEqual(anchor.source, "floor") + + 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", self.fetcher(jobs)) + self.assertEqual(anchor.sha, "a" * 40) + + def test_no_qualifying_run_returns_the_WINDOW_FLOOR(self) -> None: + """The floor, and the direction it fails in. + + A range must never widen past the window however long `main` has been + red, and the degradation must be toward MORE coverage. The oldest run in + the window is the widest honest base; `github.event.before` would be the + narrowest and would skip every commit in between (#863). + """ + 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", self.fetcher(jobs) + ) + self.assertEqual(anchor.sha, "a" * 40) + self.assertEqual(anchor.source, "floor") + + def test_an_empty_window_resolves_to_nothing(self) -> None: + anchor = baseline.resolve_gate_anchor([], "documentation-checkpoint", 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 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", 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"]) + + def test_no_commit_is_ever_skipped(self) -> None: + """The invariant the exit from the cycle must not be bought with. + + #863 is a hole in the coverage chain, and escaping a widening range by + skipping commits would be that defect again wearing a fix's face. Run + with P3's job CANCELLED, which is exactly the hole's shape. + """ + jobs = { + 1: [job("documentation-checkpoint", "success")], + 2: [job("documentation-checkpoint", "failure")], + 3: [job("documentation-checkpoint", "cancelled")], + 4: [job("documentation-checkpoint", "failure")], + 5: [job("documentation-checkpoint", "failure")], + } + covered = set() + for index in range(1, len(self.PUSHES)): + head = self.PUSHES[index] + anchor = baseline.resolve_gate_anchor( + self.runs_after(index), + "documentation-checkpoint", + lambda run_id: jobs[run_id], + ) + if anchor.source == "none": + continue + covered.update(self.commits_after(anchor.sha, head)) + self.assertEqual(covered, set(self.PUSHES[1:])) + + +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", + ]], + 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) + + 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) From 140745e6470cec8e9fa6bdc221a021faa4c0f21b Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sun, 23 Aug 2026 09:34:52 +0000 Subject: [PATCH 03/12] record(GATE-ANCHOR-PER-JOB): the measured evidence, including the mutation that turns the 484-commit red GREEN (#1773) Fills the spec's evidence section with what was measured rather than what was planned. The load-bearing rows: the anchor moves from `fafa16f0f` (2026-08-12, 484 commits, five ERRORs) to `ff8f72807` (2 commits, OK); `agent-record` resolves `66d1b0a90` and `documentation-checkpoint` `ff8f72807` on the same tree, eleven commits apart, which is the per-job argument stated as a number. Five mutations, each hashed before, printed with `git diff --stat`, parsed, and restored against the hash. M2 makes the cycle test print the widening range it constructs. M4 is the rejected widening: matching `#N` anywhere in the message turns the whole 484-commit red into `OK`, which is what deleting the obligation looks like from the inside. M5 is the control -- a real direct push inside the NARROWED range is still reported by SHA. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- .agents/specs/gate-anchor-per-job.md | 56 ++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 3 deletions(-) diff --git a/.agents/specs/gate-anchor-per-job.md b/.agents/specs/gate-anchor-per-job.md index c8012d57b..c1b252d3c 100644 --- a/.agents/specs/gate-anchor-per-job.md +++ b/.agents/specs/gate-anchor-per-job.md @@ -299,9 +299,59 @@ existing is relaxed. ## 8. Evidence -Recorded in `## Outcome` on completion: the red output of each new case before -the change, green after, each mutation's `git diff --stat` and interpreter -output, and the byte-for-byte restore proof. +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 + +`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. + +### 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` is `1 0`, and the row +count is 636 on `origin/main` and 637 here, counted again after the merge. ## 9. Stop conditions From 34af7b2213f75ec10e7f3ab17dc5bdf078b9d82c Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sun, 23 Aug 2026 10:39:56 +0000 Subject: [PATCH 04/12] fix(GATE-ANCHOR-PER-JOB): anchor on the STEP's verdict, because a job concludes over a gate that never ran (#1773) The fresh review on #1776 failed the first round on a critical finding, and it was right: anchoring on the JOB re-created #863's hole 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 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. 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. `agent-record` is twelve commits, rescued only by `documentation-checkpoint` running the same checker, which is luck. The unit is now the step. `--gate-step` is required and the flag-less form exits 2, `steps_concluded` reads `steps[].conclusion` and treats an absent step as no verdict, every gate step carries `if: !cancelled() && ...` so an earlier failure cannot silence it, and `documentation-checkpoint`'s two-checker step is split so one gate's abort cannot conclude for the other. Three smaller findings go with it. The floor read one run short, so the oldest candidate's own head fell outside the range it was meant to guarantee; it now reads `window + 1`, and the residual loss past the window is stated with its bound instead of being denied. A degraded query was swallowed by `|| true` and silently narrowed the gate to `PUSH_BASE`, which under a step anchor is not just dishonest but a hole, because the narrowed pass advances the anchor; rc 3 now means REMOTE_UNVERIFIED and the gate skips. The AGENTS.md landing rule loses the file's only em dash, names the flag and the endpoint field, binds the merging account rather than the author, and says plainly that no gate can catch it. `test_no_commit_is_ever_skipped` is re-expressed at step granularity, where it can actually fail, and its red against the unmodified tree is recorded in the spec. The union of the ranges was never the property: a job-level anchor keeps that union whole while the gate is skipped on every push. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- .agents/specs/gate-anchor-per-job.md | 307 +++++++++++++-- .github/workflows/ci.yml | 272 ++++++++++++-- AGENTS.md | 31 +- scripts/main-baseline.py | 151 ++++++-- tests/scripts/test_main_baseline.py | 541 +++++++++++++++++++++++++-- 5 files changed, 1180 insertions(+), 122 deletions(-) diff --git a/.agents/specs/gate-anchor-per-job.md b/.agents/specs/gate-anchor-per-job.md index c1b252d3c..4da60e7b3 100644 --- a/.agents/specs/gate-anchor-per-job.md +++ b/.agents/specs/gate-anchor-per-job.md @@ -145,10 +145,31 @@ 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 -append is suppressed only when the merger supplies an explicit `commit_title`, -which is what happened to this batch of five on 2026-08-18 — three of them say -so in their own bodies, under "Maintainer changes on top". +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. + +| 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 @@ -196,33 +217,125 @@ inherits through `needs:` (#873). ### 4.2 The floor -`scripts/main-baseline.py --gate-anchor ` walks the newest `--limit` -(default 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}`. - -**The window is the floor.** When no run in the window qualifies, the anchor is -the `head_sha` of the **oldest** run in the window. A range can therefore never -widen past 20 pushes however long `main` has been red, and the degradation is -toward *more* coverage rather than less — the failure this floor must not have -is silently skipping commits. - -When the query degrades or the branch has no push run at all, the anchor is -empty and the consumer keeps today's `PUSH_BASE` fallback. `REMOTE_UNVERIFIED` -stays what it is: not a pass, and not a claim of absence. +`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: 21 non-qualifying pushes put the anchor at run +2 and left run 1 outside every future range, and 25 left runs 1 to 6 outside. +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. + +**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 +### 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 per-job -anchor closes exactly that hole and nothing else: from the last run in which the -job concluded, the chain is unbroken by construction, and the span since then is -walked whole. §6's `test_no_commit_is_ever_skipped` asserts the union property -over a synthetic push sequence rather than arguing it. +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 + the shape, including that nothing fallible precedes the checker in its body. + +§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 @@ -279,7 +392,30 @@ existing is relaxed. - 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. -4. **`ArrivalDiscriminatorTests`** — pins §5's decision in +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. +6. **`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. @@ -306,7 +442,7 @@ 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 +### 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`. @@ -344,6 +480,93 @@ both. 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. + +### 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 @@ -353,7 +576,37 @@ rejects. `scripts/agent-preflight.sh` rc 0. `check-commit-style.py` and `git diff --numstat origin/main -- .agents/issue-index.md` is `1 0`, and the row count is 636 on `origin/main` and 637 here, counted again after the merge. -## 9. Stop conditions +## 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. 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. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7cf537263..387528306 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -161,12 +161,23 @@ jobs: contents: read steps: - uses: actions/checkout@v4 + id: checkout with: fetch-depth: 0 - name: Resolve this job's own diff anchor (#1773) - # The base this job walks is "the commit after which THIS job has not - # yet returned a verdict", and it is resolved HERE, by the job that - # walks it. + 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 @@ -182,8 +193,14 @@ jobs: # over ONE push, while the same gate walked 484 commits and failed # whenever the resolver survived. One job, one anchor, no race. # - # A failed or rate-limited query prints nothing, and the gate step keeps - # its `PUSH_BASE` fallback. REMOTE_UNVERIFIED is not a pass. + # 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 }} @@ -192,11 +209,35 @@ jobs: 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" || true)" + --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, @@ -518,6 +559,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. @@ -632,12 +685,23 @@ jobs: contents: read steps: - uses: actions/checkout@v4 + id: checkout with: fetch-depth: 0 - name: Resolve this job's own diff anchor (#1773) - # The base this job walks is "the commit after which THIS job has not - # yet returned a verdict", and it is resolved HERE, by the job that - # walks it. + 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 @@ -653,8 +717,14 @@ jobs: # over ONE push, while the same gate walked 484 commits and failed # whenever the resolver survived. One job, one anchor, no race. # - # A failed or rate-limited query prints nothing, and the gate step keeps - # its `PUSH_BASE` fallback. REMOTE_UNVERIFIED is not a pass. + # 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 }} @@ -663,12 +733,49 @@ jobs: 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" || true)" + --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 }} @@ -682,18 +789,69 @@ jobs: base="$PR_BASE" head="$PR_HEAD" else - # 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 + # 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`, today's behaviour, when the query finds nothing. + # `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") @@ -755,12 +913,23 @@ jobs: contents: read steps: - uses: actions/checkout@v4 + id: checkout with: fetch-depth: 0 - name: Resolve this job's own diff anchor (#1773) - # The base this job walks is "the commit after which THIS job has not - # yet returned a verdict", and it is resolved HERE, by the job that - # walks it. + 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 @@ -776,8 +945,14 @@ jobs: # over ONE push, while the same gate walked 484 commits and failed # whenever the resolver survived. One job, one anchor, no race. # - # A failed or rate-limited query prints nothing, and the gate step keeps - # its `PUSH_BASE` fallback. REMOTE_UNVERIFIED is not a pass. + # 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 }} @@ -786,12 +961,49 @@ jobs: 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" || true)" + --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: GATE_ANCHOR: ${{ env.GATE_ANCHOR }} run: | @@ -837,6 +1049,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 diff --git a/AGENTS.md b/AGENTS.md index e526e7f23..74fc08be2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -546,15 +546,30 @@ 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: it resolves no ref and reads nothing but the -commit message. Supplying an explicit `commit_title` when merging suppresses the -append, and the change lands looking exactly like a direct push to `main` — on a -commit nobody can retroactively repair. Five commits from 2026-08-18 are that -mistake, four of them from external contributors +**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` suppresses the append, and +the change lands looking exactly like a direct push to `main`, on a commit +nobody can retroactively repair. Five commits from 2026-08-18 are that mistake, +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 title is. +problem and never was. The title is. + +In practice this is one flag. 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 the MERGING account, not +the author.** All five of those merges were performed by the automation +`localai-bot`, so the place to comply is the script or workflow that calls the +merge, and a human who edits the title box in the merge dialog is making the +same change by hand. + +**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/main-baseline.py b/scripts/main-baseline.py index a896f1056..af0bc557f 100755 --- a/scripts/main-baseline.py +++ b/scripts/main-baseline.py @@ -33,10 +33,11 @@ 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 # 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 JOB not yet returned a +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 @@ -45,6 +46,15 @@ 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 @@ -55,6 +65,7 @@ import re import subprocess import sys +from collections.abc import Sequence from dataclasses import dataclass, field from pathlib import Path @@ -367,32 +378,70 @@ class Anchor: degraded: str | None = None -def job_concluded(jobs: list[dict], job_name: str) -> bool: - """Did `job_name` return a verdict in this run? +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 have concluded. A matrix reports one entry per - lane, and half a matrix is half a verdict. + 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 - return all(entry.get("conclusion") in CONCLUDED for entry in entries) - - -def resolve_gate_anchor(runs: list[dict], job_name: str, fetch_jobs) -> Anchor: - """The anchor for `job_name` over `runs`, newest first. + 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 job did not conclude". Absence of information is not information. + "this step did not conclude". Absence of information is not information. """ if not runs: return Anchor(sha="", source="none") - for run in runs: + for run in runs[:window]: jobs = fetch_jobs(run.get("id", 0)) if jobs is None: return Anchor( @@ -400,18 +449,28 @@ def resolve_gate_anchor(runs: list[dict], job_name: str, fetch_jobs) -> Anchor: source="none", degraded="REMOTE_UNVERIFIED: jobs unreadable", ) - if job_concluded(jobs, job_name): + if steps_concluded(jobs, job_name, step_names): return Anchor( sha=run.get("head_sha", ""), run_id=run.get("id", 0), source="verdict", ) - oldest = runs[-1] - # THE FLOOR, and the direction it fails in. The oldest head in the window is - # the WIDEST honest base; `github.event.before` would be the narrowest and - # would skip every commit in between, which is #863. Degrading toward more - # coverage is the only degradation a coverage gate may have. - return Anchor(sha=oldest.get("head_sha", ""), run_id=oldest.get("id", 0), source="floor") + # 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. + floor = runs[window] if len(runs) > window else runs[-1] + 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]: @@ -433,11 +492,20 @@ def push_runs(repo: str, limit: int, branch: str) -> tuple[list[dict], str | Non return runs[:limit], None -def gate_anchor(job_name: str, branch: str, window: int) -> Anchor: - """`resolve_gate_anchor` against the live forge.""" +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, branch) + runs, degraded = push_runs(repo, window + 1, branch) if degraded: return Anchor(sha="", source="none", degraded=degraded) @@ -445,7 +513,7 @@ 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, fetch) + return resolve_gate_anchor(runs, job_name, step_names, fetch, window) # -------------------------------------------------------------------------- @@ -592,6 +660,16 @@ def main(argv: list[str] | None = None) -> int: 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", @@ -602,10 +680,23 @@ def main(argv: list[str] | None = None) -> int: 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 1 - anchor = gate_anchor(args.gate_anchor, args.branch, args.window) + 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 @@ -632,7 +723,13 @@ def main(argv: list[str] | None = None) -> int: file=sys.stderr, ) if not anchor.sha: - return 1 + # 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 diff --git a/tests/scripts/test_main_baseline.py b/tests/scripts/test_main_baseline.py index e8edcfb14..ccf440cd4 100644 --- a/tests/scripts/test_main_baseline.py +++ b/tests/scripts/test_main_baseline.py @@ -74,8 +74,47 @@ 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)) @@ -771,11 +860,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 +883,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) @@ -1817,7 +1911,7 @@ def test_a_cancelled_run_whose_job_concluded_is_the_anchor(self) -> None: 1: [job("documentation-checkpoint", "success")], } anchor = baseline.resolve_gate_anchor( - runs, "documentation-checkpoint", self.fetcher(jobs) + runs, "documentation-checkpoint", (GATE_STEP,), self.fetcher(jobs) ) self.assertEqual(anchor.sha, "b" * 40) self.assertEqual(anchor.source, "verdict") @@ -1839,7 +1933,7 @@ def test_a_job_that_concluded_FAILURE_is_still_the_anchor(self) -> None: 1: [job("documentation-checkpoint", "success")], } anchor = baseline.resolve_gate_anchor( - runs, "documentation-checkpoint", self.fetcher(jobs) + runs, "documentation-checkpoint", (GATE_STEP,), self.fetcher(jobs) ) self.assertEqual(anchor.sha, "b" * 40) @@ -1859,7 +1953,7 @@ def test_cancelled_skipped_absent_and_pending_jobs_do_not_anchor(self) -> None: 1: [job("documentation-checkpoint", "failure")], } anchor = baseline.resolve_gate_anchor( - runs, "documentation-checkpoint", self.fetcher(jobs) + runs, "documentation-checkpoint", (GATE_STEP,), self.fetcher(jobs) ) self.assertEqual(anchor.sha, "a" * 40) @@ -1870,7 +1964,7 @@ def test_a_run_never_anchors_itself(self) -> None: 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", self.fetcher(jobs) + runs, "documentation-checkpoint", (GATE_STEP,), self.fetcher(jobs) ) self.assertNotEqual(anchor.source, "verdict") self.assertEqual(anchor.sha, "a" * 40) @@ -1891,16 +1985,20 @@ def test_a_matrix_job_anchors_only_when_every_lane_concluded(self) -> None: job("sanitize-cpu (thread)", "success"), ], } - anchor = baseline.resolve_gate_anchor(runs, "sanitize-cpu", self.fetcher(jobs)) + 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 the direction it fails in. - - A range must never widen past the window however long `main` has been - red, and the degradation must be toward MORE coverage. The oldest run in - the window is the widest honest base; `github.event.before` would be the - narrowest and would skip every commit in between (#863). + """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"), @@ -1909,13 +2007,165 @@ def test_no_qualifying_run_returns_the_WINDOW_FLOOR(self) -> None: ] jobs = {n: [job("documentation-checkpoint", "cancelled")] for n in (1, 2, 3)} anchor = baseline.resolve_gate_anchor( - runs, "documentation-checkpoint", self.fetcher(jobs) + 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", None) + anchor = baseline.resolve_gate_anchor( + [], "documentation-checkpoint", (GATE_STEP,), None + ) self.assertEqual(anchor.sha, "") self.assertEqual(anchor.source, "none") @@ -2005,7 +2255,8 @@ def test_per_job_anchor_reports_the_violation_once(self) -> None: runs = self.runs_after(index) jobs = self.jobs_after(index) anchor = baseline.resolve_gate_anchor( - runs, "documentation-checkpoint", lambda run_id: jobs[run_id] + 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}") @@ -2013,32 +2264,95 @@ def test_per_job_anchor_reports_the_violation_once(self) -> None: reports.append(head) self.assertEqual(reports, ["p2"]) - def test_no_commit_is_ever_skipped(self) -> None: - """The invariant the exit from the cycle must not be bought with. + # (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"), + ] - #863 is a hole in the coverage chain, and escaping a widening range by - skipping commits would be that defect again wearing a fix's face. Run - with P3's job CANCELLED, which is exactly the hole's shape. - """ - jobs = { - 1: [job("documentation-checkpoint", "success")], - 2: [job("documentation-checkpoint", "failure")], - 3: [job("documentation-checkpoint", "cancelled")], - 4: [job("documentation-checkpoint", "failure")], - 5: [job("documentation-checkpoint", "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) } - covered = set() + + @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] - anchor = baseline.resolve_gate_anchor( - self.runs_after(index), - "documentation-checkpoint", + 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:])) + 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): @@ -2093,6 +2407,10 @@ def test_the_anchor_step_runs_the_query_and_exports_it(self) -> None: "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, ) @@ -2110,6 +2428,157 @@ def test_the_anchor_step_does_not_query_on_the_pull_request_lane(self) -> None: 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 test_every_gate_step_SKIPS_rather_than_narrows(self) -> None: + """The three conditions under which a conclusion would be a lie. + + `!cancelled()` is the one that closes F1: without it an earlier step's + failure marks this one `skipped`, and a `skipped` step never returns the + verdict the anchor is waiting for -- while the JOB concludes `failure` + and a job-level rule walks straight past. The other two are the cases + where the step would run without a usable base. + """ + 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): + condition = " ".join( + line.strip() for line in names[step_name] + if line.strip().startswith("if:") + or (line.strip().startswith("&&")) + ) + self.assertIn("!cancelled()", condition) + self.assertIn("steps.checkout.outcome == 'success'", condition) + self.assertIn("steps.anchor.outcome == 'success'", condition) + self.assertIn("env.GATE_ANCHOR_DEGRADED != 'true'", condition) + + 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}: something fallible runs BEFORE the gate, so " + "the step can conclude without the gate ever executing", + ) + + 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()) From 693fa095e8b24046ead8cc110a8bacdf3d96f27d Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sun, 23 Aug 2026 10:44:52 +0000 Subject: [PATCH 05/12] record(GATE-ANCHOR-PER-JOB): the index row keeps the claim the spec just softened, and says so MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `.agents/issue-index.md` is append-only, so the row filed for #1773 cannot be edited to match §3.5's softened wording about `commit_title`. Naming that in the spec keeps the two readable together instead of looking like a contradiction nobody noticed. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- .agents/specs/gate-anchor-per-job.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.agents/specs/gate-anchor-per-job.md b/.agents/specs/gate-anchor-per-job.md index 4da60e7b3..41d35c485 100644 --- a/.agents/specs/gate-anchor-per-job.md +++ b/.agents/specs/gate-anchor-per-job.md @@ -163,6 +163,11 @@ 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 still carries the unsoftened wording**, because +that file is append-only and AGENTS.md forbids editing a row that has landed. +This section is the authority on the claim, and the index row is the record of +what was believed when it was filed. + | pull request | merged by | "Maintainer change on top" | |---|---|---| | [#640](https://github.com/mudler/vllm.cpp/pull/640) | `localai-bot` | yes | From f671ca92d4c6fcaefccec8185c6c52f35004a4c2 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sun, 23 Aug 2026 10:48:36 +0000 Subject: [PATCH 06/12] record(GATE-ANCHOR-PER-JOB): a restored tree read as the mutant, because the bytecode cache survived a same-length mutation M4 swaps `subject` for `message`. The two words are the same length, so the mutant and the original have identical file sizes, and the restore landed in the same second as the mutation. Python validates a cached `.pyc` on source mtime and size alone, so it served the mutant's bytecode from a tree whose sha256 and `git status` both said clean. Hash the source and discard the cache; neither check catches this on its own. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- .agents/specs/gate-anchor-per-job.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/.agents/specs/gate-anchor-per-job.md b/.agents/specs/gate-anchor-per-job.md index 41d35c485..e4306e29c 100644 --- a/.agents/specs/gate-anchor-per-job.md +++ b/.agents/specs/gate-anchor-per-job.md @@ -552,6 +552,28 @@ 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 | From 0095e42728a0c85dcb02534832b32e48fc9735bc Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sun, 23 Aug 2026 11:52:29 +0000 Subject: [PATCH 07/12] fix(GATE-ANCHOR-PER-JOB): resolve the gate guards, and refuse an unreadable runs payload (#1773) The second fresh review on #1776 confirmed the step-granularity design by mutation and failed the pull request on the guards around it. This commit carries the three executable repairs. `test_every_gate_step_SKIPS_rather_than_narrows` asserted four SUBSTRINGS and never resolved the expression. Appending `|| true` to all five gate-step guards leaves every asserted substring byte-for-byte intact and turns the whole conjunction into the constant `true`: the gate then runs on a degraded read, `GATE_ANCHOR` is empty, `base` falls back to `$PUSH_BASE`, a one-push range passes, the step CONCLUDES, and the anchor advances past the entire span the narrowing dropped. That mutation reported `Ran 99 / OK`. The test now RESOLVES the guard over all sixteen states it has to decide, using the evaluator this file already carried for concurrency keys; `_Expression` gained unary `!` and the status-function call form so `!cancelled()` evaluates rather than being read. The same mutation now reports 75 failures across all five steps. `push_runs` reported an unreadable forge as a clean absence. It returned `[]` for anything that was not a dict carrying `workflow_runs`, where `jobs_for` has always answered `REMOTE_UNVERIFIED` for the same case, and the difference is not cosmetic: rc 1 makes `ci.yml` fall back to `$PUSH_BASE` and narrow, where rc 3 makes it SKIP. Driving the real `main` through a fake `gh_api` measured a list payload, a null payload and `{"message": "Not Found"}` all exiting 1. And nothing executed `push_runs` at all -- every test replaced it with a stand-in and `gh_api` appeared in no test, which is the stand-in shape one layer down from the shell shim. `PushRunsPayloadTests` drives the whole chain over the four shapes, plus a genuinely empty window that must stay rc 1 and a readable window that must stay rc 0. `test_one_diff_scoped_checker_per_gate_step` did not hold the property the spec claimed for it. It read the first `python3 `, so a fallible non-`python3` command inserted before the gate was invisible, and `if not checkers: continue` exempted the one gate step whose whole body is an inline shell gate. A `git fetch` planted in either place reported `Ran 99 / OK`. Under `set -eu` that command aborts the step, GitHub concludes it `failure`, and `steps_concluded` reads the conclusion as a verdict about a gate that never ran. `test_nothing_fallible_PRECEDES_the_gate_in_its_own_body` resolves the body instead: the range prelude may use `set`, `[`, `echo` and nothing else, the first command that is not one of those is the GATE, and it must consume the range. Both mutations now red, and the inline-shell step is in the population. `resolve_gate_anchor` could also floor onto the run being pushed. With one run in the window `runs[-1]` is `runs[0]`, so the anchor was the head being pushed and `base..head` was EMPTY -- a gate reporting success over no commits and concluding on it, which advanced its own anchor. Latent here and reachable on a fork's first push. There is no floor to name with a single run, so the answer is the clean absence. The residual short-history bound is asserted rather than denied: below `window + 1` runs the oldest available run is itself a candidate and its own commit falls outside the range it bases, and no payload field can repair that. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- scripts/main-baseline.py | 46 ++- tests/scripts/test_main_baseline.py | 499 ++++++++++++++++++++++++++-- 2 files changed, 520 insertions(+), 25 deletions(-) diff --git a/scripts/main-baseline.py b/scripts/main-baseline.py index af0bc557f..e0f813bd0 100755 --- a/scripts/main-baseline.py +++ b/scripts/main-baseline.py @@ -469,7 +469,33 @@ def resolve_gate_anchor( # 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. - floor = runs[window] if len(runs) > window else runs[-1] + # + # 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") @@ -479,6 +505,17 @@ def push_runs(repo: str, limit: int, branch: str) -> tuple[list[dict], str | Non 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( @@ -487,7 +524,12 @@ def push_runs(repo: str, limit: int, branch: str) -> tuple[list[dict], str | Non ) if degraded: return [], degraded - runs = payload.get("workflow_runs", []) if isinstance(payload, dict) else [] + 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 diff --git a/tests/scripts/test_main_baseline.py b/tests/scripts/test_main_baseline.py index ccf440cd4..76c88f4d2 100644 --- a/tests/scripts/test_main_baseline.py +++ b/tests/scripts/test_main_baseline.py @@ -51,7 +51,7 @@ import sys import tempfile import unittest -from contextlib import redirect_stdout +from contextlib import redirect_stderr, redirect_stdout from pathlib import Path ROOT = Path(__file__).resolve().parents[2] @@ -627,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]+) @@ -723,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": @@ -730,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 " @@ -773,6 +789,86 @@ 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", "do", "done", + "case", "esac", "in", "{", "}", "!"} +) + +# What a gate step's body may run BEFORE its gate. Every entry computes the +# range or narrates it, and none of them can abort the step in a way that would +# make its conclusion a statement about a gate that never executed: +# `set` configures the shell, `[` is a conditional whose failure the surrounding +# `if`/`||` consumes, and `echo` writes to the step log. +RANGE_PRELUDE_COMMANDS = frozenset({"set", "[", "echo", ":"}) + + +def commands_in(body: str) -> list[tuple[str, str]]: + """`(command word, its statement)` for every command a step body runs. + + Whole-line comments are dropped, backslash continuations are joined, and + each logical line is split on the operators that separate statements. Shell + keywords and assignments are not commands. `for` and `case` headers are + skipped whole, because their word list is data rather than a call. + """ + + 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[tuple[str, str]] = [] + for line in logical: + for piece in re.split(r"(?:&&|\|\||;|\|)", line): + words = piece.split() + if words and words[0] in ("for", "case"): + continue + while words and words[0] in SHELL_KEYWORDS: + words = words[1:] + if not words: + continue + head = words[0] + if re.fullmatch(r"[A-Za-z_][A-Za-z0-9_]*=.*", head): + continue + found.append((head, piece.strip())) + return found + + def concurrency_blocks(ci: dict) -> list[tuple[str, dict]]: """Every concurrency block in the workflow, workflow level first. @@ -1960,15 +2056,86 @@ def test_cancelled_skipped_absent_and_pending_jobs_do_not_anchor(self) -> None: 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.""" + 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.sha, "a" * 40) + 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 = [ @@ -2186,6 +2353,144 @@ def test_the_workflow_no_longer_queries_a_RUN_level_success(self) -> None: ) +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 AnchorCycleConstructionTests(unittest.TestCase): """The feedback loop of #1773, CONSTRUCTED rather than read. @@ -2481,28 +2786,97 @@ def test_every_step_that_READS_the_anchor_is_NAMED_by_it(self) -> None: "no --gate-step names it, so nothing waits for its verdict", ) - def test_every_gate_step_SKIPS_rather_than_narrows(self) -> None: - """The three conditions under which a conclusion would be a lie. + 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 - `!cancelled()` is the one that closes F1: without it an earlier step's - failure marks this one `skipped`, and a `skipped` step never returns the - verdict the anchor is waiting for -- while the JOB concludes `failure` - and a job-level rule walks straight past. The other two are the cases - where the step would run without a usable base. + 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): - with self.subTest(job=job_name, step=step_name): - condition = " ".join( - line.strip() for line in names[step_name] - if line.strip().startswith("if:") - or (line.strip().startswith("&&")) - ) - self.assertIn("!cancelled()", condition) - self.assertIn("steps.checkout.outcome == 'success'", condition) - self.assertIn("steps.anchor.outcome == 'success'", condition) - self.assertIn("env.GATE_ANCHOR_DEGRADED != 'true'", condition) + 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. @@ -2526,8 +2900,87 @@ def test_one_diff_scoped_checker_per_gate_step(self) -> None: self.assertIsNotNone(first) self.assertEqual( f"scripts/{checkers.pop()}", first.group(1), - f"{step_name}: something fallible runs BEFORE the gate, so " - "the step can conclude without the gate ever executing", + 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 only use `set`, `[`, `echo` and `:` -- a shell option, a + conditional whose exit status its `if` or `||` consumes, and a message. + None can abort the step. The first command that is NOT one of those 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. + + RESIDUAL, stated rather than hidden: a command inserted before the gate + that itself references `$base` is not distinguishable from the gate by + this test. Command substitution is refused in the prelude so a fallible + call cannot hide inside an allowed one, and one gate per step is + asserted above; a range-consuming impostor is what remains. + """ + 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 head, statement in commands_in(body): + if head not in RANGE_PRELUDE_COMMANDS: + gate = (head, statement) + break + self.assertNotIn( + "$(", statement, + f"{job_name}/{step_name}: command substitution in the " + "range prelude hides a fallible call inside an " + "allowed one", + ) + self.assertNotIn( + "`", statement, + f"{job_name}/{step_name}: backtick substitution in the " + "range prelude hides a fallible call inside an " + "allowed one", + ) + 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: From 6fe471efee095e98e80d7f029aa8ef8095b2695f Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sun, 23 Aug 2026 11:52:49 +0000 Subject: [PATCH 08/12] record(GATE-ANCHOR-PER-JOB): four records the new anchor made false, corrected before they land (#1773) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The second review found four records stating things this row retracted or invalidated. Three are on `main` already and one would have landed with this pull request; that last one is why the correction cannot wait. `.agents/issue-index.md`'s row is ADDED by this pull request and stated as fact both claims the pull request had softened: that all five squashes lack `(#N)` BECAUSE the merger supplied an explicit `commit_title`, and that each job anchors on the last push run in which THAT JOB concluded. The first is inferred and not measured -- the pulls endpoint keeps no merge-input payload, and all five were merged by the same account that appends `(#N)` correctly everywhere else. The second is the job-level rule the review refuted and this row replaced with step granularity. The spec's reason for leaving the row alone was wrong: the row has not landed, and `scripts/check-issue-index-append-only.py` diffs the merge base against the head, so amending a row this branch also added stays a pure addition -- `git diff --numstat` reads `2 0` and the checker returns `OK`. `squash_merge_commit_message = PR_BODY` means exactly one version of that row will ever exist on `main`, so this is the last moment it can be right. `scripts/check-commit-trailers.py` argued its landed exceptions from a property this row removes: "`LAST_GREEN` advances only on a GREEN run, so a range containing an unrepairable red is re-walked ... forever." The anchor now advances on a CONCLUDED run, failure included. The exceptions are still necessary and the old argument is quoted in place so the next reader does not re-derive a property the tree no longer has. #1262 is the live instance. `.github/workflows/ci.yml`'s concurrency prose still said the gates walk from the last SUCCESSFULLY gated commit via `last-gated-commit`. Both halves are now false: the base advances past a concluded step, and `last-gated-commit` resolves nothing and keeps only the closed-pull-request guard. `.agents/specs/ci-concurrency.md` carried the superseded rule as live design and cited `test_every_diff_scoped_step_bases_on_the_last_gated_commit`, a symbol `26def4c8f` renamed away inside this same pull request. That last one is a gap in the tooling, filed as #1787 rather than fixed here. `scripts/check-symbol-anchors.py` resolves a `path::Symbol` citation; a bare backticked symbol name has no path and is outside its population, so the rename left a document naming a symbol that does not exist and every gate stayed green. Teaching it to resolve a bare identifier changes checker semantics and needs its own row, spec and red-before evidence. The spec also gains the two numbers the review corrected: §4.2's pre-fix narration was off by one, because an exclusive base puts runs 1 and 2 outside the range at 21 pushes rather than run 1 alone, and §4.3 item 3 claimed a test held a property it did not. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- .agents/issue-index.md | 3 +- .agents/specs/ci-concurrency.md | 20 ++++- .agents/specs/gate-anchor-per-job.md | 129 ++++++++++++++++++++++++--- .github/workflows/ci.yml | 17 +++- scripts/check-commit-trailers.py | 26 ++++-- 5 files changed, 173 insertions(+), 22 deletions(-) diff --git a/.agents/issue-index.md b/.agents/issue-index.md index 80d6bcb58..49b56f2ba 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -652,4 +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)` because the merger supplied an explicit `commit_title`, which suppresses the append GitHub otherwise makes even under `squash_merge_commit_title = PR_TITLE`. FIXED IN FLOW by the anchor, not by the rule: each diff-scoped job resolves its own anchor from the last push run in which THAT job concluded, and the checker is untouched. Spec [gate-anchor-per-job.md](specs/gate-anchor-per-job.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 index e4306e29c..7c2ee0427 100644 --- a/.agents/specs/gate-anchor-per-job.md +++ b/.agents/specs/gate-anchor-per-job.md @@ -2,7 +2,7 @@ **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) +**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 @@ -29,6 +29,18 @@ 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 @@ -163,10 +175,18 @@ 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 still carries the unsoftened wording**, because -that file is append-only and AGENTS.md forbids editing a row that has landed. -This section is the authority on the claim, and the index row is the record of -what was believed when it was filed. +**`.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" | |---|---|---| @@ -232,8 +252,13 @@ 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: 21 non-qualifying pushes put the anchor at run -2 and left run 1 outside every future range, and 25 left runs 1 to 6 outside. +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. @@ -254,6 +279,26 @@ 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. @@ -324,7 +369,19 @@ anchored at all. `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 - the shape, including that nothing fallible precedes the checker in its body. + 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 nothing else -- and then its GATE, identified as the first + command that is not one of those, which must CONSUME the range. Command + substitution is refused in the prelude so a fallible call cannot hide inside + an allowed one. The population includes the one step whose gate is inline + shell rather than a checker. §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 @@ -420,7 +477,24 @@ existing is relaxed. - `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. -6. **`ArrivalDiscriminatorTests`** — pins §5's decision in +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. + - **`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. @@ -633,7 +707,42 @@ 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. Stop conditions +## 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. 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. + +## 12. 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. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 387528306..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 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 From 2b34a2dd8f5067e0b052c69ced2a2a217191f1ef Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sun, 23 Aug 2026 11:54:21 +0000 Subject: [PATCH 09/12] record(GATE-ANCHOR-PER-JOB): the second round's evidence, each finding reproduced before it was repaired (#1773) The measurements the two commits before this one rest on, written down where a reviewer can re-run them. Each of the five reproductions is the mutation or construction that made the finding real on the tree as the review left it: `|| true` on all five guards reporting `Ran 99 / OK`, three unreadable payloads exiting 1 through the real `main`, a `git fetch` planted before the gate in each of the two step shapes, and a test asserting the empty range its own docstring warned about. Beside each is what the same input does now. The earlier round's mutations are re-run on the repaired tree rather than assumed to still hold: M7 gives 5 failures, M9 gives 5, M10 gives 1. Every mutation was hashed first, printed with `git diff --stat`, parsed with PyYAML or `ast.parse` so a syntax error could not read as a passing test, and restored against the hash with `sha256sum -c`, an empty `git status --porcelain`, an empty `git diff` and a cleared `__pycache__` under `PYTHONDONTWRITEBYTECODE=1`. The cache step is here because the round before this one measured a restored tree still running the mutant's bytecode. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- .agents/specs/gate-anchor-per-job.md | 47 ++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/.agents/specs/gate-anchor-per-job.md b/.agents/specs/gate-anchor-per-job.md index 7c2ee0427..a0fd1afce 100644 --- a/.agents/specs/gate-anchor-per-job.md +++ b/.agents/specs/gate-anchor-per-job.md @@ -677,6 +677,53 @@ rejects. `scripts/agent-preflight.sh` rc 0. `check-commit-style.py` and `git diff --numstat origin/main -- .agents/issue-index.md` is `1 0`, and the row count is 636 on `origin/main` and 637 here, counted again after the merge. +### 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 four 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. + ## 9. The fresh review on PR #1776, and what it changed The review confirmed the fork refutation, the `if: always()` race, the live From b7557fe9de223ef2daf8249527d20b3ef7b8556a Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sun, 23 Aug 2026 11:57:30 +0000 Subject: [PATCH 10/12] style(GATE-ANCHOR-PER-JOB): the em dashes this round added, and one stray blank line `.agents/style/prose.md:136` refuses em dashes and binds new prose. This round's additions to the spec used ten, where the rest of that file uses `--`. Only the lines this round added are rewritten; the file's existing prose is left alone, because rewriting a file to satisfy a style rule is out of scope unless a row asks for it. `34af7b221` also left three consecutive blank lines in the test file's `job` payload builder where PEP 8 wants two. No behaviour and no assertion changes. `tests/scripts/test_main_baseline.py` reports `Ran 109 tests`, `OK`. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- .agents/specs/gate-anchor-per-job.md | 20 ++++++++++---------- tests/scripts/test_main_baseline.py | 1 - 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/.agents/specs/gate-anchor-per-job.md b/.agents/specs/gate-anchor-per-job.md index a0fd1afce..95bb545eb 100644 --- a/.agents/specs/gate-anchor-per-job.md +++ b/.agents/specs/gate-anchor-per-job.md @@ -483,18 +483,18 @@ existing is relaxed. 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 + - `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. - - **`PushRunsPayloadTests`** — the first tests to EXECUTE `push_runs`, driving + - **`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 + `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 +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. @@ -704,7 +704,7 @@ The earlier round's mutations still bite on the repaired tree: M7, dropping 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 +`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 @@ -762,12 +762,12 @@ 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. | +| 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. | +| 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. | +| 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. | @@ -782,7 +782,7 @@ names. Filed rather than fixed. ## 11. Owed -- [#1787](https://github.com/mudler/vllm.cpp/issues/1787) — `check-symbol-anchors.py` +- [#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 diff --git a/tests/scripts/test_main_baseline.py b/tests/scripts/test_main_baseline.py index 76c88f4d2..1e0927c9d 100644 --- a/tests/scripts/test_main_baseline.py +++ b/tests/scripts/test_main_baseline.py @@ -116,7 +116,6 @@ def job(name, conclusion, steps=None): ) - # The real 31448896841 shape: run says success, both sanitizer lanes are red. RUN_31448896841 = { "id": 31448896841, From f26d974884bdc6e6c70891f7c10d0914c1c92121 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sun, 23 Aug 2026 12:58:03 +0000 Subject: [PATCH 11/12] fix(GATE-ANCHOR-PER-JOB): refuse a fallible statement before a gate, whatever binds it (#1773) The third fresh review on #1776 confirmed the TREE as correct: none of the five gate bodies carries a hazardous shape. It failed the pull request because a test and a record assert a completeness they do not hold. This commit carries the executable half. `test_nothing_fallible_PRECEDES_the_gate_in_its_own_body` stated one residual and had three escapes past it. `commands_in` returned on any `name=...` head BEFORE the `$(`/backtick refusal ran, so a binding that carried a substitution was never examined; and `[` was allowlisted on the reasoning that "its failure the surrounding `if`/`||` consumes", while the parser splits on `||` and discards it, so nothing checked that a given `[` has one. Four injections immediately before `check-now-current.py` each reported `Ran 109 / OK`: `base="$(git rev-parse --verify "${base}^{commit}")"`, `_p="$(git fetch -q origin nope)"`, its backtick spelling, and `[ -f .git/NO_SUCH ]`. Executed under `bash` with `python3` shimmed, the first three abort the step at rc 128 and the last at rc 1, with the gate invoked zero times. GitHub then concludes the step `failure`, `steps_concluded` reads that as a verdict, and the anchor advances over a range the gate never walked. The first of those is the one to worry about. Normalising the anchor to a full commit id right before the gate is the natural next edit in that step, and 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. `commands_in` becomes `statements_in`. It returns bindings as statements instead of dropping them, and it keeps the operator that follows each piece, because whether a failure aborts the step depends on what comes after it. The substitution refusal now covers every statement before the gate, and a `[` is admitted only where an `if`/`while` header or an `||`/`&&` reads its status. The repair discriminates rather than banning: `[ -f .git/NO_SUCH ] || echo missing`, an `if` header form, and a plain binding all stay green, and the shell agrees with the test on each. The nine shapes the previous round caught still red. Two residuals are now stated on the test instead of overstated: a statement that itself references `$base` is not distinguishable from the gate, and the allowlist remains a judgement about `set`, `echo` and `:` rather than a proof. `baseline_runs` reported an unreadable forge to a human as a clean absence. Nothing had ever executed it, because the two tests that name it replace `collect` one layer above. It skipped any payload that was not a dict and took `workflow_runs` from any dict that was one, so a list, a null and GitHub's `{"message": "Not Found"}` each returned `([], None)`, which `render` prints as "No completed baseline run found on main." with an instruction to trigger the first run. The rc was never wrong and this is the advisory lane rather than a gate, so no consumer read it as a pass; what was wrong is that unknown was reported as absence. `jobs_for`, one function above, already refuses exactly these shapes. `BaselineRunsPayloadTests` executes the reader for the first time, through real `main` and real `render`, and holds the other half too: a genuinely empty window still reads as an absence. `test_the_checker_resolves_no_ref_to_decide_arrival` forbade `origin/`, `ls-remote` and `for-each-ref` in the raw source, which forbids them in English as well. A comment explaining that this checker does not resolve `origin/main` reddened it with a message about a ref lookup that was not there, and the message was the entire file. `executable_source` blanks the two prose surfaces, comments and docstrings, and deliberately keeps string literals, because a real lookup is spelled as one. Verified both ways at module level: the two comment shapes pass, and three planted lookups still fail. The failure now names the line. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- scripts/main-baseline.py | 16 +- tests/scripts/test_check_role_discipline.py | 67 +++++- tests/scripts/test_main_baseline.py | 228 ++++++++++++++++---- 3 files changed, 256 insertions(+), 55 deletions(-) diff --git a/scripts/main-baseline.py b/scripts/main-baseline.py index e0f813bd0..f1043f2b3 100755 --- a/scripts/main-baseline.py +++ b/scripts/main-baseline.py @@ -316,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 diff --git a/tests/scripts/test_check_role_discipline.py b/tests/scripts/test_check_role_discipline.py index b1f2b8748..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,50 @@ 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. @@ -210,16 +257,18 @@ class ArrivalDiscriminatorTests(unittest.TestCase): FORK_BODY = "Removes POSIX-only constructs ...\n\nIssue: #503\n" def test_the_checker_resolves_no_ref_to_decide_arrival(self) -> None: - source = (ROOT / "scripts/check-role-discipline.py").read_text( - encoding="utf-8" - ) + source = executable_source(ROOT / "scripts/check-role-discipline.py") for forbidden in ("ls-remote", "for-each-ref", "origin/"): - self.assertNotIn( - forbidden, - source, - "arrival is decided from the commit message; a ref lookup here " - "would make the fork hypothesis testable, and it is not what " - "this checker does", + 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: diff --git a/tests/scripts/test_main_baseline.py b/tests/scripts/test_main_baseline.py index 1e0927c9d..fdd963e15 100644 --- a/tests/scripts/test_main_baseline.py +++ b/tests/scripts/test_main_baseline.py @@ -53,6 +53,7 @@ import unittest 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" @@ -814,25 +815,53 @@ def resolve_condition(expression: str, state: dict) -> bool: SHELL_KEYWORDS = frozenset( - {"if", "then", "else", "elif", "fi", "while", "do", "done", + {"if", "then", "else", "elif", "fi", "while", "until", "do", "done", "case", "esac", "in", "{", "}", "!"} ) -# What a gate step's body may run BEFORE its gate. Every entry computes the -# range or narrates it, and none of them can abort the step in a way that would -# make its conclusion a statement about a gate that never executed: -# `set` configures the shell, `[` is a conditional whose failure the surrounding -# `if`/`||` consumes, and `echo` writes to the step log. +# 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 commands_in(body: str) -> list[tuple[str, str]]: - """`(command word, its statement)` for every command a step body runs. +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. Shell - keywords and assignments are not commands. `for` and `case` headers are - skipped whole, because their word list is data rather than a call. + 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] = [] @@ -851,20 +880,29 @@ def commands_in(body: str) -> list[tuple[str, str]]: if pending: logical.append(pending) - found: list[tuple[str, str]] = [] + found: list[Statement] = [] for line in logical: - for piece in re.split(r"(?:&&|\|\||;|\|)", line): + parts = re.split(r"(&&|\|\||;|\|)", line) + pieces, separators = parts[0::2], parts[1::2] + for index, piece in enumerate(pieces): words = piece.split() - if words and words[0] in ("for", "case"): + 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] - if re.fullmatch(r"[A-Za-z_][A-Za-z0-9_]*=.*", head): - continue - found.append((head, piece.strip())) + 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 @@ -2490,6 +2528,77 @@ def test_the_anchor_query_goes_through_gh_api_and_nowhere_else(self) -> None: 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. @@ -2926,13 +3035,13 @@ def test_nothing_fallible_PRECEDES_the_gate_in_its_own_body(self) -> None: 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 only use `set`, `[`, `echo` and `:` -- a shell option, a - conditional whose exit status its `if` or `||` consumes, and a message. - None can abort the step. The first command that is NOT one of those 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. + 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 @@ -2941,11 +3050,36 @@ def test_nothing_fallible_PRECEDES_the_gate_in_its_own_body(self) -> None: trailer step to the same probe -- both consume the range, both pass, and neither is exempt any more. - RESIDUAL, stated rather than hidden: a command inserted before the gate - that itself references `$base` is not distinguishable from the gate by - this test. Command substitution is refused in the prelude so a fallible - call cannot hide inside an allowed one, and one gate per step is - asserted above; a range-consuming impostor is what remains. + 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) @@ -2954,22 +3088,28 @@ def test_nothing_fallible_PRECEDES_the_gate_in_its_own_body(self) -> None: body = step_run_body(names[step_name]) or "" self.assertTrue(body, f"{step_name}: gate step has no run body") gate = None - for head, statement in commands_in(body): - if head not in RANGE_PRELUDE_COMMANDS: - gate = (head, statement) + for entry in statements_in(body): + if (entry.kind == "command" + and entry.head not in RANGE_PRELUDE_COMMANDS): + gate = (entry.head, entry.text) break - self.assertNotIn( - "$(", statement, - f"{job_name}/{step_name}: command substitution in the " - "range prelude hides a fallible call inside an " - "allowed one", - ) - self.assertNotIn( - "`", statement, - f"{job_name}/{step_name}: backtick substitution in the " - "range prelude hides a fallible call inside an " - "allowed one", - ) + 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" ) From 41840d4b5160b6640704b0877c6c786b50bfe1e4 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sun, 23 Aug 2026 12:58:21 +0000 Subject: [PATCH 12/12] record(GATE-ANCHOR-PER-JOB): AGENTS.md stated as measured the mechanism this spec retracts (#1773) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 2 softened the `.agents/issue-index.md` row and this spec to say that the `commit_title` mechanism behind the five unnumbered squashes is INFERRED and cannot be measured, because the pulls endpoint keeps no merge-input payload. `AGENTS.md` kept the unhedged version, and it is the one file every agent loads automatically. It asserted that an explicit `commit_title` suppressed the append, that "Five commits from 2026-08-18 are that mistake", and that because "All five of those merges were performed by the automation `localai-bot`" the place to comply is that automation. A reader takes the strongest of two disagreeing records, so this would be quoted forward as measured, with a named account attached to an unproven cause. The RULE is untouched and still one flag: do not pass `--subject` to `gh pr merge --squash`, do not send `commit_title` to the merge endpoint, and the default title wins. It is worth writing whatever suppressed the append, which is now said in place of the causal claim. What is hedged is the mechanism and the account: `localai-bot` appends the number correctly on every other squash it lands, so that account is not an explanation on its own, and its automation is one place to check rather than a known cause. The rule still binds whoever performs the merge, because the merging side chooses the title. Four record corrections ride with it. §8 said "four gate-step guards" where the same section says five; measured from `ci.yml` it is five, one in `agent-record` and two each in `documentation-checkpoint` and `commit-protocol-tag`. The records paragraph still carried the first round's `1 0` and 637 index rows, which round 2's #1787 row made stale; the branch stands at `2 0` and 638, both rows additions and both last in the file. §4.3 item 3 restated the completeness that the third review then broke, and now describes what is actually held. And 24 added lines in this file carried em dashes, which `.agents/style/prose.md` forbids and which bind new prose; the file is now at 0, as `AGENTS.md` already was. §8 gains the third round's evidence and §11 the review itself, including one judgement recorded rather than left silent: F-D was fixed in flow instead of being owed, because it is the same class as F2, in the same file, and it changes no checker semantics. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- .agents/specs/gate-anchor-per-job.md | 207 ++++++++++++++++++++++----- AGENTS.md | 37 +++-- 2 files changed, 197 insertions(+), 47 deletions(-) diff --git a/.agents/specs/gate-anchor-per-job.md b/.agents/specs/gate-anchor-per-job.md index 95bb545eb..aae8df9bf 100644 --- a/.agents/specs/gate-anchor-per-job.md +++ b/.agents/specs/gate-anchor-per-job.md @@ -1,4 +1,4 @@ -# GATE-ANCHOR-PER-JOB — a diff gate anchors on its OWN last verdict, not on a run-level `success` +# 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) @@ -11,12 +11,12 @@ **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 +- `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 +- `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) @@ -24,7 +24,7 @@ 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 +`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.** @@ -137,7 +137,7 @@ 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 +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 @@ -224,7 +224,7 @@ the change that breaks the cycle, and it is the one to argue for: 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 + 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 @@ -377,12 +377,33 @@ anchored at all. 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 nothing else -- and then its GATE, identified as the first - command that is not one of those, which must CONSUME the range. Command - substitution is refused in the prelude so a fallible call cannot hide inside - an allowed one. The population includes the one step whose gate is inline + `[`, `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, @@ -404,7 +425,7 @@ defect being fixed reporting itself. 23 is the floor working. 484 was the bug. **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` +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. @@ -428,27 +449,27 @@ That is where the defect was introduced and where it can be prevented. All offline. `tests/scripts/test_main_baseline.py` gains four classes; nothing existing is relaxed. -1. **`GateAnchorTests`** — `gate_anchor()` against synthetic payloads. +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 + - 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 + - `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 + - `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 + - `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` +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`; @@ -485,7 +506,16 @@ existing is relaxed. 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. + 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 @@ -517,7 +547,7 @@ existing is relaxed. 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 +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. @@ -526,7 +556,7 @@ reports `OK` on all five afterwards. `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. +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 @@ -551,13 +581,13 @@ both. | # | 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** | +| 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 | +| 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. +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 @@ -674,8 +704,11 @@ change. The probe commit was built with `git commit-tree` against a temporary 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` is `1 0`, and the row -count is 636 on `origin/main` and 637 here, counted again after the merge. +`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`) @@ -684,7 +717,7 @@ 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 four gate-step guards this row adds. +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. @@ -724,6 +757,87 @@ 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 @@ -780,7 +894,32 @@ 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. Owed +## 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 @@ -789,7 +928,7 @@ names. Filed rather than fixed. 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. -## 12. Stop conditions +## 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. diff --git a/AGENTS.md b/AGENTS.md index 74fc08be2..fa4614d48 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -549,20 +549,31 @@ 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` suppresses the append, and -the change lands looking exactly like a direct push to `main`, on a commit -nobody can retroactively repair. Five commits from 2026-08-18 are that mistake, -four of them from external contributors +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 title is. - -In practice this is one flag. 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 the MERGING account, not -the author.** All five of those merges were performed by the automation -`localai-bot`, so the place to comply is the script or workflow that calls the -merge, and a human who edits the title box in the merge dialog is making the -same change by hand. +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`