diff --git a/.agents/issue-index.md b/.agents/issue-index.md index fe989ba5c..15d062501 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -670,6 +670,7 @@ rather than merged. `scripts/check-agent-record.py` gates both. | [#1812](https://github.com/mudler/vllm.cpp/issues/1812) | `KERNEL-ACCEL-PROVIDER-DECLINE-EXACT` | **The two exact `declines == 1` routing assertions in `tests/vt/test_ops_attention_cross.cpp` are the only thing in the tree that pins the CUDA half of [#1584](https://github.com/mudler/vllm.cpp/issues/1584)'s repair, and the run CI performs cannot see it move.** Measured on `thor:gpu0` (sm_110) on 2026-08-23 while discharging [#1692](https://github.com/mudler/vllm.cpp/issues/1692), `rc` jobs `d452b91f` and `43a27be9` at `bacb71109`. Two mutations, each rebuilt and rerun on the device: reintroducing the #1584 double count in the seam (`GetOpFallbackUncounted` delegating with `/*count=*/true`), and reverting this row's whole CUDA edit (`GetOpFallbackUncounted` -> `GetOpFallback` in `src/vt/cuda/cuda_attention_cross.cu`). **Both leave the full `test_ops_attention_cross` GREEN at 20 cases / 156 assertions**, and both turn the two cases RED with `CHECK( 2 == 1 )` under a per-case `-tc=` filter. `tests/CMakeLists.txt` registers ONE ctest entry per suite, so the second row of that pair is the consequence stated plainly: the CUDA call site can be reverted to the counting resolver and every gate in this repository stays green. **Why the full run is blind was measured, not reasoned:** the reachability mutation reds `test_ops_attention_cross.cpp:326` and `:334` first -- plain `attention-cross:` cases that predate the blocked provider, assert nothing about `declines`, and run geometries `BlockedShape` rejects, so each of them resolves `BlockedFallback()`'s function-local static OUTSIDE any counted window and a later `ResetOpProviderStats` erases the extra increment. The suite warms itself, exactly as #1555's deleted `WarmDeclineOnce` warmed it by hand. The file's own comment is accurate -- exact "standalone and under `-tc=`" -- but nothing executes them that way, and `.agents/specs/op-provider-decline-exact.md` §9 said the opposite until this measurement corrected it in place. Candidate repairs: per-case ctest entries with a trailing wildcard (the depth-decoder name contains a comma and `-tc=` splits on commas, so a bare filter matches nothing and prints `0 cases ran` + `SUCCESS!`) and an asserted `test cases: 1`; or a way to reset the static between cases, which changes production code to suit a gate. Listed as O3 under `## Owed` in [op-provider-decline-exact.md](specs/op-provider-decline-exact.md) | bug | | [#1804](https://github.com/mudler/vllm.cpp/issues/1804) | `MODEL-MM-dots3-note-dots3-note-for-causal-lm` | **`.agents/specs/dots3-note.md` §4 item 6 read `is_neox_style=False` as belonging to the dots3-note SLIDING rope ONLY, and it belongs to both MLA ropes** — the sentence would have sent a W3 implementer to rotate the 13 full-attention layers split-half NeoX. `Dots3NoteSlidingAttention` does pass `is_neox_style=False` literally (`model.py:408` @ vLLM `origin/main` `c205726108df54bb6fbf15b19e725a4a3add2b18`), which is the half W0 read; `Dots3NoteFullAttention` (`model.py:219`) inherits the SAME hard-coded value from `deepseek_v2.py`::`DeepseekV2MLAAttention.__init__` (`:1093-1098`). So the two geometries do NOT differ on the RoPE layout at all — they differ on the THETA, `swa_rope_theta` 5e4 on 33 layers against `rope_theta` 8e7 on 13. The polarity that DOES flip is the INDEXER's, and that is §4 trap 2's point rather than item 6's: `deepseek_v2.py:1148` sets the indexer rope to `is_neox_style = not indexer_rope_interleave`, so at DeepSeek-V3.2's absent-key default the indexer runs NeoX beside an MLA rope that is GPT-J, and `indexer_rope_interleave = True` (`configs/dots3_note.py:23`) is what makes dots3-note's two agree. Being wrong in this direction is SILENT: the same 64 coordinates are rotated either way, so nothing changes shape and nothing throws — the §4 defect class exactly, on a row that spec §6.4 says has no oracle anywhere to catch it. FIXED IN FLOW on `row/MODEL-MM-dots3-note-W1`: §4 item 6 corrected in place and it says what it used to say (`main` is never rewritten), `ParseDots3NoteParams` resolves `rope_is_neox_style = false` on BOTH geometries with the two citations beside it, and `tests/vllm/models/test_dots3_note_scaffold.cpp` asserts both plus the indexer's agreement with them. The assertion was captured RED against the NeoX reading, on an arm that compiled and ran, before the corrected value existed | bug | | [#1802](https://github.com/mudler/vllm.cpp/issues/1802) | `BACKEND-CUDA-SM110` | **Re-measuring the sm_110 `ctest` baseline at `6756f9131` (2026-08-23, job `8bf39567-9334-4f7e-aa27-43a2aa867bb7`, artifacts `/mnt/nas_share/rc/thor-w05-955/out/`) found six names red that were green at `0764ded2b`, none with an owner**, plus two standing entries whose recorded cause no longer holds. Arrivals, all mode `Failed`: `test_cuda_ops` (`:106` `CHECK(bad == 0)` -> `6 == 0` and `7 == 0`, silu_and_mul vs CPU; also red on GB10 at 439/440 against 438/440 here, [#907](https://github.com/mudler/vllm.cpp/issues/907)), `test_backend_cross_device` (`:2063` MoeSiluMul and `:2601` MoE combine/gate vs the CPU oracle, 80205/80207), `test_llama_embedding_fold` (`:254` engine vs direct at `epsilon(1e-5)`), `test_mtp_depth` (`:738` `CHECK(st.capture_shapes == 0)`), `test_qwen3_dflash2_draft` (`:2574` `CHECK(r.generate_threw.empty())`), and `test_ops_attention_dense_fa2` (`:692` `CHECK(Mismatches(on, ref) > 0)` -> `0 > 0`, which reads as a TEST arch-assumption: the case asserts the knob-ON path DIFFERS from the scalar reference, and with `fa2` DISABLED for `[110]` they are the same kernel). Four of the six are CUDA-vs-CPU numeric comparisons off by about one ulp, so they MAY be one sm_110 difference wearing four names -- a hypothesis from the logs, not a measurement. Also folded in: `test_gguf_device_fit_reach`, red and unattributed since 2026-08-15 (`:463`, moved from `:278`, assertion unchanged); and `test_serve_low_tools`, whose `(name, mode)` pair never moved while its CAUSE changed completely -- `73ada0df8` (#1661/#1662) fixed the [#961](https://github.com/mudler/vllm.cpp/issues/961) guard so it skips, the string `shellcheck` appears nowhere in the run's log, and the failure is now four `tests/tools/test_dflash2_speed_harness.py` `ShellDriverTest` cases of 517. The CONTROL was rerun and now falsifies the old conclusion: the baseline reads `FAILED (failures=3, errors=1, skipped=1)` and the control with `shellcheck` 0.9.0 installed reads `FAILED (failures=3, errors=1)` -- the same four cases, differing only in the vanished skip. **That vanished `skipped=1` is a second proof**: the one test that skipped in the baseline is the `shellcheck` guard itself, so with the binary present it stopped skipping and PASSED, showing the instrument was the only thing the install changed. #961 was CLOSED COMPLETED on 2026-08-23 acting on that prompt -- `73ada0df8` fixed its guard while referencing the sibling filing #1661/#1662, which had left #961 orphaned rather than resolved. Not in scope here, because they have owners and did not worsen: the four FA-2 refusals, `test_platform`/`test_op_parity` hardcoding GB10, `test_linear_method` and the three `qwen3_5_gdn_spec_routing` tests (#907), the FP8 pair ([#1725](https://github.com/mudler/vllm.cpp/issues/1725), which improved `SEGFAULT` -> `Failed`), and [#962](https://github.com/mudler/vllm.cpp/issues/962), which reproduced byte-identically at `bitdiff=15/32768`. Table and diff in [`environment.md`](environment.md); the re-measure debt itself is [#955](https://github.com/mudler/vllm.cpp/issues/955) | bug | +| [#1809](https://github.com/mudler/vllm.cpp/issues/1809) | `GATE-CI-ENFORCEMENT-FLOOR` | **`commit-protocol-tag` and `documentation-checkpoint` have been red on `main` since 2026-08-13 and cannot recover on their own: their walk bases on the head of the last SUCCESSFUL push run, so one unrepairable commit freezes the base and every later push re-walks the same violations over a range one commit wider.** Re-measured 2026-08-23 at `bacb71109`: `LAST_GREEN` is `fafa16f0f` of 2026-08-13T01:53, the range is **499** first-parent commits with **0** merges, and **41 distinct commits** in it can never be repaired because they are on `main` and adding a trailer or a task-branch arrival needs a rewrite `AGENTS.md` forbids — **35** fail `check-commit-trailers.py --range` (of which **20** carry no `FOLLOWING_AGENTS_PROTOCOL` marker at all and the other **15** carry it 1 to 9 times in a form the strict contract rejects, so the two counts describe the same population under different contracts, not different walks) and **6** disjoint ones fail `check-role-discipline.py` for a repository change that reached `main` with no task branch; `check-now-current.py` passes over the whole range and forgives nothing. The `LAST_GREEN` base is RIGHT about what it was built for and is not reverted: `github.event.before` skips a cancelled run's commits and nothing re-covers them, which is what lets the push lane be latest-only ([#822](https://github.com/mudler/vllm.cpp/issues/822), [#863](https://github.com/mudler/vllm.cpp/issues/863)), and cancelled runs are common here ([#1285](https://github.com/mudler/vllm.cpp/issues/1285)). FIXED IN FLOW with a recorded **enforcement floor** in `scripts/ci-enforcement-floor.txt` — one commit the walk never goes behind — taken as the newer of `LAST_GREEN` and the floor by ANCESTRY (`git merge-base --is-ancestor`, never by commit date, which is author-controlled and can go backwards). Losslessness survives because the floor is a lower clamp on an otherwise unchanged base: while the floor sits behind `LAST_GREEN`, which is the steady state, the resolved base is byte-identical to today's, and the only window that skips anything is `LAST_GREEN..floor` right after an advance — bounded, reviewed, and enumerated in the spec. The four byte-similar copies of the selection in `.github/workflows/ci.yml` collapse into `scripts/ci-walk-base.py`, which is the point of the extraction: of those four inline blocks exactly ONE was ever replayed by a test (`test_main_baseline.py::AgentRecordDiffRangeTests`, on `agent-record`'s body, under a shim that stubs every `python3` call and therefore pins which checker ran with which range string rather than the base rule), and the other three were executed by nothing, so the property most likely to break silently now has a test that replays a cancelled-run sequence against a real throwaway repository with the naive `before` base as its positive control. A per-commit exemption list was REJECTED — `AGENTS.md` has no waiver registry, the mechanism would have to be built three more times in checkers that have no exemption concept, and it never shrinks the walk, so the range grows by one per merge forever. This NARROWS enforcement by 41 commits and says so: enforcing on an immutable landed commit is a permanent red rather than a gate, because no contributor action can clear it, and a permanent red is read by nobody ([#1722](https://github.com/mudler/vllm.cpp/issues/1722)'s effect). No assertion is deleted and no checker is modified; only the base of the walk moves, and a new violation after the floor still reds, proved by a scratch-commit mutation rather than by reading the diff. Spec [ci-enforcement-floor.md](specs/ci-enforcement-floor.md) | bug | | [#1823](https://github.com/mudler/vllm.cpp/issues/1823) | `BACKEND-ATTN-REGISTRY` | **`FlashAttentionBackend::supports_compute_capability` is upstream's NVIDIA `capability >= (8,0)` (`include/vllm/v1/attention/backend.h:401-404`, `flash_attn.py:200-202`), and `MetalPlatform::get_device_capability` (`src/vllm/platforms/metal.cpp:35-38`) answers with the MTLGPUFamilyApple GENERATION — deliberately, with a comment saying `has_device_capability(N, 0)` means "Apple family >= N". `validate_configuration` at `src/vllm/v1/attention/backend.cpp:197` compares the two whenever `capability.present()`, so an SM-version bar is applied to an Apple family number.** Apple family 9 on the M4 gate box clears it by coincidence; a GitHub `macos-15` runner reports lower, FLASH_ATTN — the ONLY entry in `MetalPlatform::get_attn_backend_priority()` — is refused, and `SelectAttentionBackendName` throws. MEASURED 2026-08-23 on the first execution of `test_metal_backend` since `7020de936` (v0.0.2, 2026-08-11), run [32668677681](https://github.com/mudler/vllm.cpp/actions/runs/32668677681) at `744abeca3`: `tests/vt/test_metal_backend.cpp:170` ERROR `"No valid attention backend for device type 2 from {FLASH_ATTN: [compute capability not supported]}"`, suite 26 cases / 25 passed / **1 failed** / 3 skipped, 112336 assertions, `Status: FAILURE!`. **Red since `369ea7fd4` (2026-08-19), which is NOT an ancestor of `7020de936`** — four days in the exposure window [#1765](https://github.com/mudler/vllm.cpp/issues/1765) measured, with no lane able to run the case. **The prose above `backend.cpp:197` states the premise that is false**: it argues `DeviceCapability::present()` is already false for every platform that cannot answer, and Metal answers in a different unit; `vulkan.cpp` and `tenstorrent.cpp` need the same check before any repair. **Not fixed in flow**: every candidate repair (drop the predicate for non-SM platforms, stop reporting `present()` on Metal — which `test_metal_backend.cpp:153-154` asserts on purpose — or make the backend device-type aware) changes what selection means for kCPU/kMETAL/kVULKAN/kTENSTORRENT, so it takes the normal row, spec and fresh-review path. Until then `macos-metal-mlx` is RED on `main`, deliberately not hidden behind `continue-on-error` or a skip. Listed under `## Found in flow, filed, not fixed here` in [attn-validate-configuration.md](specs/attn-validate-configuration.md) | bug | | [#1819](https://github.com/mudler/vllm.cpp/issues/1819) | `ENG-MM-INPUT-PIPELINE` | **`scripts/mm/tower_skip_rss.sh` configured both build directories with `-DVLLM_CPP_BUILD_EXAMPLES=OFF` and then ran `ninja -C "$d" -j 4 vllm-server`, and `vllm-server` is an `examples/` target** -- the `OUTPUT_NAME` of `server` (`examples/CMakeLists.txt:91,108`), in a directory the root `CMakeLists.txt:2828` adds only under `if(VLLM_CPP_BUILD_EXAMPLES)`. Reproduced with the harness's own flags: the configure returns 0 and `ninja` answers `unknown target 'vllm-server'`, so the run `exit 4`s at arm A before any RSS exists. The block landed on `main` in `bacb71109` (#1364) and had never been executed. **Nothing could catch it**: `tests/scripts/test_tower_skip_rss_report.py` covers `--report-only`, `--check-source` and `--stage-check` -- every path needing no checkpoint -- while the configure, the build, `run_arm`, the `/health` poll and the kill/wait only ever run on a leased box, so the suite was 41/41 green over a harness that could not build its own binary. FIXED IN FLOW: `-DVLLM_CPP_BUILD_EXAMPLES=ON` (measured: `ninja -j 4 vllm-server` then returns 0 and writes `/examples/vllm-server`, the one file of that name in the tree and the path `docs/USAGE.md:54,95,128,204` names), the binary is NAMED rather than found by a `find` piped into `head -1`, and a new `--dry-run` prints the `cmake`/`ninja`/`run_arm` invocations out of the same variables the run issues them from and asserts that CMake defines the requested target under those flags -- statically, so it builds nothing and runs in CI, plus a live `ninja -t targets` prong on an already-configured tree that skips BY NAME when there is none. `DryRunTests` runs it against the script as committed, which is the case that reds on this defect, and against scratch copies with the flag flipped OFF, the flag dropped, and the binary path pointed away from where CMake writes it | bug | | [#1647](https://github.com/mudler/vllm.cpp/issues/1647) | `LOAD-MODELOPT-NVFP4-BORROW` | **`LoadNvfp4AnyNaming`'s ModelOpt branch never attempted the ENG-LOAD-DIRECT-UPLOAD borrow, so `r0b0tlab/Qwen3.8-27B-NVFP4-MTP-sm121`'s 21 GiB of packed weights landed as ANONYMOUS heap and then got a second, device copy on a box where the two come out of the same 119 GiB.** Measured on the committed tree at `27d8bfa70`, 2026-08-23, on `dgx:gpu0` (GB10, 119 GiB UNIFIED): `VT_LOAD_STATS=1` reads `host_copy=16.394 GiB borrowed=2.373 GiB device_upload=0.000 GiB`, which FALSIFIES the reflex diagnosis — the weights reach the host PACKED, not dequantized to bf16, and nothing here chases a bf16 materialization. What it shows instead is 16.394 GiB of anonymous heap where upstream holds reclaimable PAGE CACHE. A small run (`--kv-cache-memory 512MiB --max-num-seqs 1 --max-model-len 1024`) loads with 85 GiB left; the serving shape (`8GiB` KV, `--max-num-seqs 4`, `--max-model-len 4096`, `fp8` KV) collapses `avail 37 GiB -> 28 GiB in ONE second` and, unguarded, reaches 0 and reboots the host with NVRM `NV_ERR_NO_MEMORY ... _memdescAllocInternal` — five times. vLLM on the SAME checkpoint and the SAME box at `--gpu-memory-utilization 0.70` sits at 71-83 GiB available and serves. MECHANISM, derived from the tree before the fix was written: `LoadCtNvfp4Raw` has carried `BorrowStTensorBytes`-then-`MakeOwned` since ENG-LOAD-DIRECT-UPLOAD, and the ModelOpt sibling goes straight to `MakeOwned` + `std::memcpy` for BOTH `packed` and `scale`; this checkpoint declares `quant_method: "modelopt"`, so all 193 NVFP4 modules take that branch. The expensive consequence is the second one: `AdoptDeviceBytesAsHost` is gated on `w.mmap_src != nullptr && w.bytes.borrowed()`, so `ResidentNvfp4`'s post-upload adoption — which is what releases the source pages and, where device memory is host-addressable, collapses the host and device copies onto one buffer — was a SILENT NO-OP for every ModelOpt weight. `ResidentNvfp4` has no aliasing branch (unlike `ResidentWeight`), so the device copy is a genuine second allocation: ~42 GiB of a 119 GiB pool before one KV byte is reserved. FIXED IN FLOW: the ModelOpt branch gets the identical borrow-then-fallback shape, byte-size `VT_CHECK`s and fallback preserved — a widening of an already-gated path to a second checkpoint SPELLING, not a new policy. UPSTREAM AT THE PIN `5559679229bc961848b121ccdeaa8fa5d79bec98`, read in a local checkout at that exact SHA: vLLM allocates the FINAL PACKED parameter ON THE DEVICE before opening any file (`base_loader.py:52-58` `with target_device:` around `initialize_model`, `modelopt.py:1313-1345` uint8 packed + `float8_e4m3fn` scales with NO `device=`, `:483-491` for the FP8 half) and streams the shard into it one tensor at a time through an mmap generator (`weight_utils.py:969-974`, consumed at `:1247` `param.data.copy_`), so its host-side residency is one loop iteration of page cache. A SECOND EDIT reconciles two predicates that disagree about one device: `DirectDeviceLoadEligible` required `!platform.is_unified_memory()`, which is true on GB10, while `needs_weight_staging()` is unconditionally true on CUDA and `src/vllm/platforms/cuda.cpp:85` says why in its own comment — "regardless of GB10 being physically unified" — and `include/vllm/platforms/interface.h:294-298` records that `is_unified_memory()` "answers the OPPOSITE question". `ResidentNvfp4` proves the staging predicate right by construction. THE SECOND EDIT DOES NOT FIX THE OOM and the spec says so rather than implying otherwise: `StageAndReleaseLoadedDense` is reached only while `IsPlainBf16Qwen3_5Dense` holds, which this checkpoint fails at its first NVFP4 projection, and `ReleaseResidentQwen3_5DenseHostWeights` walks only `OwnedTensor`s. Its measurable effect is a plain bf16 dense checkpoint on unified memory. It is not the use-after-free the release site's comment warns about: that site asks the invariant `HostMirrorIsRedundant` (`d_dev != nullptr`), and the aliasing arm leaves `d_dev` null. NOT CLOSED: #1647's own `## Owed` — the DFlash2 startup instrumentation, the double dequant-B question, a memory bound that refuses before the global OOM killer takes `coredns` with it, and the re-run — is live and listed under `## Owed` in the spec, together with upstream's integrated-GPU memory accounting (`mem_utils.py:148-155`, `:54-83`), which this tree has nowhere, and the allocate-device-first polarity that is the faithful long-term target. Every gate here is CPU-only: this session had no GPU and no fleet access, so the `avail` curve at this head is owed. Spec [`load-modelopt-nvfp4-borrow.md`](specs/load-modelopt-nvfp4-borrow.md) | bug | diff --git a/.agents/specs/ci-enforcement-floor.md b/.agents/specs/ci-enforcement-floor.md new file mode 100644 index 000000000..2c2d64bae --- /dev/null +++ b/.agents/specs/ci-enforcement-floor.md @@ -0,0 +1,752 @@ +# The diff-scoped gates get a floor, so an unrepairable commit cannot freeze them + +Issue: [#1809](https://github.com/mudler/vllm.cpp/issues/1809) +Row: `GATE-CI-ENFORCEMENT-FLOOR` + +The main-branch diff-scoped gates walk from the head of the last SUCCESSFUL push +run. That base is what makes a cancelled run lossless (#822, #863). It is also +what turns one unrepairable commit into a permanent red: no green run means the +base never advances, so every later push re-walks the same violations and adds +its own commit to the range. + +This row keeps the self-healing base and clamps it from below with a recorded +**enforcement floor** — one commit the walk never goes behind. It also moves the +base selection out of four copies of inline workflow shell and into one script +that has a test suite. + +## Why + +### Measured, 2026-08-23, at `bacb71109c8d63b5f862c9b121dd86e04e1a07ee` + +`gh api repos/mudler/vllm.cpp/actions/workflows/ci.yml/runs?branch=main&event=push&status=success&per_page=1` +returns `fafa16f0f32acc8255e113a2cbc35f8b99cf2072`, whose commit date is +2026-08-13T01:53:21+02:00. That is `LAST_GREEN`, and it is ten days stale. + +| Quantity | Value | +|---|---| +| First-parent commits in `fafa16f0f..bacb71109` | 499 | +| Of those, merge commits | 0 | +| `commit-protocol-tag` grep step: commits with no `FOLLOWING_AGENTS_PROTOCOL` | 20 | +| `commit-protocol-tag` strict step: commits failing `check-commit-trailers.py --range` | 35 | +| `documentation-checkpoint`: commits failing `check-role-discipline.py` | 6 | +| `documentation-checkpoint`: `check-now-current.py` | passes | + +The range is linear — `rev-list` and `rev-list --first-parent` both return the +same 499 commits — so the two trailer walks cover the same population and the +grep step's 20 are a strict subset of the strict step's 35. The difference is the +contract, not the walk: the grep asks whether the marker appears at all, and the +other 15 commits carry it (once to nine times, measured) in a form +`check-commit-trailers.py` rejects. Presence is not parseability. The 6 +role-discipline commits are disjoint from the 35. **41 distinct commits.** + +That count is the measurement at `bacb71109` and is left as measured. A 42nd +commit, `c00b99c7c`, landed on 2026-08-24 while this row was in flight and moved +the split to 35 trailers / 7 role discipline. `## The 42 forgiven commits` and +`### The first advance` carry it. + +### Why no remedy exists + +Every one of the 41 is on `main`. A trailer or a task-branch arrival can only be +added to a commit by rewriting it, and `AGENTS.md` forbids rewriting `main` +without exception. So the gate asks for a repair that cannot be performed, and +the loop closes: + +``` +main is red -> no successful push run -> LAST_GREEN frozen + -> next push walks a range one commit wider + -> re-hits the same 41 -> still red +``` + +The practical effect is #1722's effect: a job that is always red trains every +reader to skip it, and a skipped job protects nothing. Violations kept arriving +during the freeze and nobody read the gate that named them — +`1757330006f6` landed without the trailer on 2026-08-23, and `6e73bdee3` landed +without a task branch the same day. + +### The property that must survive + +`ci.yml` records the reason for the `LAST_GREEN` base at its definition: +`github.event.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**. +That is what allows the push lane to be latest-only (#822), and reverting the +base to `before` reintroduces exactly the gap #863 measured. Cancelled runs are +common here (#1285). Losslessness is therefore a requirement on the fix, not a +nice-to-have. + +## Design + +### The floor + +One commit sha, recorded in `scripts/ci-enforcement-floor.txt`. The walk never +starts behind it. + +``` +base = last_green or before # unchanged +if floor is newer than base: # newer == base is a proper ancestor of floor + base = floor +``` + +"Newer" is decided by ancestry, not by date: `git merge-base --is-ancestor`. +A commit date is author-controlled and can go backwards across a rebase, so a +date comparison can choose the wrong commit; ancestry on a linear first-parent +`main` cannot. + +The floor is set to `c00b99c7c8b64f9247230ed6220598cc5c0e347e`, which is the +last of the 42 commits enumerated below. The floor is the forgiven commit +itself rather than its child, because the walk is `FLOOR..HEAD` and excludes +`FLOOR`; every commit after it stays enforced. + +It was first set to `bacb71109c8d63b5f862c9b121dd86e04e1a07ee`, which was past +the 41 commits then known. `c00b99c7c` landed while this row was in flight and +the value was advanced once, before merge, under +`### Advancing the floor`. See `### The first advance` below. + +### The base selection moves into a script + +`scripts/ci-walk-base.py` resolves the base for every diff-scoped gate. +`.github/workflows/ci.yml` had four byte-similar copies of the selection — +`agent-record`'s role-discipline step, `documentation-checkpoint`, +and both steps of `commit-protocol-tag`. Four copies of a rule is four places to +get the floor wrong, and the rule was very nearly untested. Measured rather than +assumed: `tests/scripts/test_main_baseline.py::AgentRecordDiffRangeTests` replays +exactly ONE of the four bodies — `agent-record`'s — and it does so under a shim +that stubs every `python3` call, so what it pins is which checker gets invoked +with which range string. It cannot see the base rule itself, and the other three +bodies were executed by no test at all. Nothing in the tree could have caught the +ratchet. + +The extraction gives the rule a test surface, and that is what makes requirement +2 — a cancelled run stays lossless — an executable assertion instead of a claim. +The `AgentRecordDiffRangeTests` shim now executes the resolver for real rather +than stubbing it, so those cases test the real composition of the resolver and +the step shell. + +Contract: + +``` +scripts/ci-walk-base.py --event --head + [--pr-base ] [--push-base ] [--last-green ] + [--floor-file ] [--floor ] +``` + +It prints the resolved base on stdout, diagnostics on stderr, and exits non-zero +only on an unusable floor record or an invalid argument. + +Resolution order, in one place: + +1. `pull_request` lane: return `pull_request.base.sha` unchanged. The floor does + not apply — see "Why the floor is push-lane only". +2. Otherwise `base = LAST_GREEN`, falling back to `github.event.before`. +3. If `base` is empty or unknown to git — the all-zero sha of a new branch, or a + force-push whose `before` is gone — return it unchanged, so the existing + downstream guard still degrades to the tip commit alone. The floor raises a + USABLE base; it never substitutes for an unusable one. +4. If the floor is unknown to git, or is not an ancestor of `head`, warn on + stderr and return `base` unchanged. A floor that the current history does not + contain cannot bound that history, and `floor..head` for an unrelated floor + is not a range anybody asked for. +5. If the floor is an ancestor of `base`, `base` is already at or past the floor: + return `base`. +6. Otherwise return the floor. + +### Why the floor is push-lane only + +The pull-request lane bases on `pull_request.base.sha` and has been green +throughout the freeze — verified on #1786 on 2026-08-23. Applying the floor +there would only ever raise a base, which is a narrowing of what that lane +enforces, and no defect asks for it. The narrowest change that fixes the bug +leaves the PR lane byte-identical. + +### Cancelled runs stay lossless + +The floor is a lower clamp on a base that is otherwise chosen exactly as it is +today. While the floor is behind `LAST_GREEN`, step 5 returns `LAST_GREEN` and +the resolved base is byte-identical to today's. A cancelled run does not advance +`LAST_GREEN`, the next run walks the wider range, and the cancelled run's +commits are covered. + +**That is not the regime this repository is in, and the spec should not pretend +otherwise.** `LAST_GREEN` advances only on a green push run, and measured on +2026-08-23 it is still `fafa16f0f`, the run of 2026-08-12T23:53:24Z — the same +instant `### Measured` records above as `2026-08-13T01:53:21+02:00`, in the +commit's own zone. Of the 100 most recent `push` runs of `ci.yml` on `main`, +every one of them after that date, 93 are `cancelled`, 6 are `failure` and one +is still running; none is a success. `.agents/verification.md` records the same +shape from the other side: of 40 consecutive runs measured for #274, 26 were +`cancelled` and exactly one completed. So the floor sits **ahead** of +`LAST_GREEN`, not behind it, the resolved base is the floor on every push, and +the walk grows by one commit per merge — the same unbounded growth this file +levels at the rejected exemption list in `### The alternative that was +rejected`. It stays that way until a `push` run on `main` finishes green, which +nothing in this row brings closer. + +Correctness is unaffected: `floor..HEAD` still covers every commit that landed +after the floor, which is every commit any contributor can still do anything +about. What is affected is the claim, so the claim is corrected rather than +repeated. + +The one window where losslessness is suspended is the interval +`LAST_GREEN..floor`, which opens at a floor advance and closes at the next green +push run — and on the evidence above that can be a long time, not a moment. What +the window skips does **not** grow while it is open: it is fixed at the advance +by the two recorded shas, and every commit after the floor is still walked. That +window is exactly the forgiveness being asked for, it is bounded by a recorded +sha, and what it forgives is enumerated below. It is not silent. + +`tests/scripts/test_ci_walk_base.py::CancelledRunLosslessTests` builds a real +throwaway repository and replays the sequence: C1 gated green, C2 pushed and its +run cancelled, C3 pushed. It asserts the resolved base is C1 and that +`rev-list base..C3` CONTAINS C2 — and, as the positive control that proves the +assertion discriminates, that the naive `github.event.before` base for the same +push resolves to C2 and its range does NOT contain C2. + +### What is narrowed, and the argument for it + +This change narrows enforcement: 42 commits that the gate currently reports are +no longer walked. The argument is that enforcing on an immutable already-landed +commit is not enforcement. There is no action any contributor can take that +turns those 42 reds green, because the only action that would is a `main` +rewrite the protocol forbids. A gate with no available remedy is a permanent +red, and a permanent red is read by nobody — which is a strictly worse outcome +than a smaller gate that is read. + +The gate's purpose is to stop a NEW violation, and that is untouched: a commit +landing after the floor with no trailer, or with no task branch, still reds the +job. Proved by mutation, not by reading the diff — see `## Gates`. + +No assertion is deleted. `check-commit-trailers.py`, `check-role-discipline.py` +and `check-now-current.py` are not modified by this row, and the grep step's +condition is unchanged. Only the base of the walk moves. + +**The floor forgives by RANGE, not by violation, and that is a real cost.** The +42 commits are what the three checkers report *today*. A checker written +tomorrow that finds a new class of defect in the pre-floor range will be +forgiven for that range too, silently, without anybody deciding to forgive it +and without a line appearing anywhere. An exemption list would not have that +property: it names shas and one error each, so a new checker's finding on an +old commit would still red. This is the strongest argument against the shape +chosen here, and `### The alternative that was rejected` is not an honest +comparison without it. It is accepted because the alternative's three costs are +judged worse and because the range is bounded by a sha a reviewer can read, +not because this cost is small. + +**The floor also absorbs the one in-checker exception, and nobody chose that +either.** `check-commit-trailers.py` carries a single annotated +`LANDED_MESSAGE_EXCEPTIONS` entry for `281b4bc76c0e` (#1262), and every run that +applies it prints `1 landed-message exception(s) applied. This is DEBT, not +success`. That commit is dated 2026-08-18 and is an ancestor of the floor, so +after this lands the main push lane never walks it. Measured: +`check-commit-trailers.py --range fafa16f0f..origin/main` prints that banner +once, and `--range bacb71109..origin/main` prints it zero times. The commit is +therefore forgiven twice, and its DEBT line — written precisely so a reader of +a green lane can see what the lane is carrying — stops reaching that reader. The +entry is not deleted, duplicated or bypassed, and +`tests/scripts/test_check_commit_trailers.py::LandedMessageExceptions` still +pins its count, key shape and error string, so only its runtime visibility on +the push lane is lost. Recording it here is the replacement, and it is a weaker +one than the banner. + +### Advancing the floor + +Editing one line of `scripts/ci-enforcement-floor.txt`, in a reviewed pull +request whose body says which commits the advance forgives and why each is +unrepairable. Git is the history of the floor: `git log -p` on that file lists +every advance with its reason. There is no registry and no accumulating list. + +This does not make an unrepairable commit free. It makes it cost a reviewed +commit that has to name it, which is what `AGENTS.md` means by visible debt. + +### Where the floor lives, and the record-lock rule + +`AGENTS.md` `## Records` forbids a surface that every pull request must write. +The floor is not one: an ordinary pull request never touches it, and only a +deliberate advance does. A one-value data file beside the script that reads it +matches `scripts/*-allowlist.txt`, which are the tree's existing shape for a +script's data. + +Rejected homes: + +- **A top-level `env:` in `ci.yml`.** No new file, but `ci.yml` has no top-level + `env:` block today and seven checkers and test suites parse that file. A + structural addition risks a red that has nothing to do with this row. +- **A git ref or tag.** Movable without review and invisible in a diff, which + removes the whole reason for choosing a floor over a time window. +- **A new `.agents/` document.** `check-pr-size.py::classify_path` fails closed + on an unclassified path, so the file would require an edit to + `check-pr-size.py`, which is itself a governance-checker change requiring its + own mutation evidence. A cascade in exchange for nothing. + +### The alternative that was rejected + +**A per-commit exemption list**: keep walking `LAST_GREEN..HEAD` forever and +name the 42 shas in a file the checkers consult. It records more precisely than a +floor does, and `check-commit-trailers.py` already carries one landed-message +exception, so the mechanism is not foreign. + +Rejected on three grounds: + +1. `AGENTS.md` `## Changing the rules or a checker` states the project has no + waiver registry, because an exception registry is a state log and this + protocol has no state log. One in-checker exception carrying its reason is + not a registry; a file of 42 growing to N is precisely one. +2. It needs the mechanism built three more times. The grep step and + `check-role-discipline.py` have no exemption concept, so the change would add + an exemption surface to code that currently has none — more new enforcement + machinery than the fix it delivers. +3. It never shrinks the walk. The range stays 499 commits and grows by one per + merge forever, so the cost and the log noise of every run grow without bound, + and the next unrepairable commit appends to the list rather than being + confronted. The floor bounds the walk and makes forgiveness cost a review. + +Also considered and rejected: reverting the base to `github.event.before` +(reintroduces #863 outright); making the two jobs report-only (`AGENTS.md` +`## Gates`: a permanent report-only state is not a result); and a rolling +time-window base such as "the newer of `LAST_GREEN` and 7 days ago", which +forgives continuously and silently and records nothing. + +## Scope + +In scope: + +- `scripts/ci-walk-base.py`, new. +- `scripts/ci-enforcement-floor.txt`, new: the recorded floor. +- `tests/scripts/test_ci_walk_base.py`, new. +- `.github/workflows/ci.yml`: the four base-selection blocks call the script. +- `scripts/agent-preflight.sh`: the new suite joins `SUITES`. + +Out of scope, deliberately: + +- Repairing the 42 commits. It cannot be done without rewriting `main`. +- The three checkers themselves. Not one line changes. +- `agent-record`'s missing-`hugo` red (#1722, fix in flight as #1726) and the two + `windows-msvc` reds (#584). Both are inherited and neither is this row's. +- The PR lane's base selection, which is unchanged. + +## The 42 forgiven commits + +Real protocol violations that landed unread between 2026-08-13 and 2026-08-24, +recorded here because after the floor advances no gate will name them again. + +35 fail the strict trailer contract and 7 fail role discipline. The two sets are +disjoint. 41 of them were enumerated when the floor was first recorded at +`bacb71109`; the 42nd, `c00b99c7c`, arrived afterwards and moved the split from +35/6 to 35/7 — see `### The first advance`. + +### Fail `check-commit-trailers.py --range` (35) + +| Commit | Date | Subject | +|---|---|---| +| `7572b0f4e2fb` | 2026-08-13 | guard the parity pin header against declaration-order breaks (#558) | +| `7ba9a675f491` | 2026-08-13 | feat(rocm): implement the vt::Backend graph-capture seam on hipGraph (W1, #332) (#473) | +| `7965f12bf4bc` | 2026-08-13 | fix(GATE-PR-SIZE-BINARY): retire the fail-closed binary guard (#615) (#619) | +| `a3aa02e197ec` | 2026-08-14 | spec(MODEL-MUSIC-MUSIC3): scope MiniMax-Music3 (#672) (#679) | +| `34dc578760d0` | 2026-08-14 | oracle(MODEL-MUSIC-MUSIC3): the diffusers oracle GENERATES AUDIO (#672) (#708) | +| `8d0c2779b91a` | 2026-08-14 | feat(MODEL-MUSIC-MUSIC3): W1 — the modular checkpoint loader (#672) (#714) | +| `373aa125142a` | 2026-08-14 | spec(BACKEND-ROCM): the ROCm head_dim=128 decode arm (#564) | +| `f8cbc2310bca` | 2026-08-14 | fix(#664): the video registry's existence probes stop reaching Windows with POSIX stat | +| `0011bedf0c75` | 2026-08-14 | fix(#720): M_PI is not defined by MSVC | +| `fc903b8dd73f` | 2026-08-14 | fix(#674): the LTX-2.5 VAE loader read the safetensors mmap through a uint16_t* | +| `9f2b9bb9a30b` | 2026-08-14 | feat(tenstorrent): allowlist MistralForCausalLM + device-aware gate (#431) | +| `d8efb1fa0ccf` | 2026-08-14 | build(nix): add rocwmma to the ROCm dev shell (#444) (#638) | +| `3921160e569d` | 2026-08-14 | fix(#757): six C4456 shadowed locals block the Windows test compile | +| `c629b5d0ff78` | 2026-08-14 | feat(ltx-2.5): image conditioning at crf=0 (#644) | +| `5da1d7f2fa89` | 2026-08-14 | fix(GATE-FORK-ANCESTRY): diff a PR from its merge base (#773) (#782) | +| `ddff090936bb` | 2026-08-15 | policy(POLICY-SINGLE-PR-AND-STYLE): one PR carries the spec and its code (#827) | +| `be4a3edf17b2` | 2026-08-15 | fix(GATE-WINDOWS-WARNING-POLICY): /WX- is not /WX (#774) (#795) | +| `6680aab68912` | 2026-08-15 | fix(GATE-AUDIT-BRANCH-EVIDENCE): reach the IN-FLIGHT verdict in CI (#726) (#802) | +| `ca01719e6b29` | 2026-08-15 | fix(#772): four loaders cast mmap'd safetensors to uint16_t* (#815) | +| `3ce5a1dc1b0f` | 2026-08-15 | feat(MUSIC3-W7): a gated GGUF Q4_K arm (#672) (#832) | +| `51e0cb5b15fe` | 2026-08-15 | policy(POLICY-ISSUE-INTAKE): the issue index moves out of the roadmap (#846) | +| `b5a5f3b182d7` | 2026-08-15 | feat(MODEL-MUSIC-MUSIC3): W2's remainder (#672) (#831) | +| `6e6bba63d7c1` | 2026-08-15 | fix(GATE-OP-PARITY-MANIFEST): refuse a throwing golden by name (#776) (#853) | +| `bc570da0d387` | 2026-08-15 | MODEL-NEMOTRON-H: the WEIGHT LOADER (#752) | +| `34962d96bea0` | 2026-08-15 | fix(#775): the NemotronH forward's downcast was a promise, not a check (#868) | +| `b3d0f3ed5dc8` | 2026-08-15 | fix(capi): hoist SpeechRegistry() out of extern "C" (#805) (#814) | +| `1e2408526419` | 2026-08-15 | docs(dspark): the user-facing docs asserted a ratio measurement has refuted (#442) (#894) | +| `04be1390b227` | 2026-08-15 | fix(FIX-REGISTRY-DOWNCAST-SWEEP): open every registry handle with a check (#901) | +| `b5f27c9a4c7d` | 2026-08-15 | record(intake): place #904, the third sanitize-cpu red (#906) | +| `2688e6586675` | 2026-08-15 | fix(MODEL-MUSIC-MUSIC3): the e2e gate asked for 60 s of music (#852, #925) (#942) | +| `e34d71379e70` | 2026-08-16 | fix(qwen3.5): drop redundant AppleClang capture (#1054) | +| `aba8d5ffb77c` | 2026-08-18 | instrument(MUSIC3): a per-stage split (#672) (#1231) | +| `055ff1143704` | 2026-08-21 | docs: align README with current surfaces (#1302) | +| `2d2a66715ef4` | 2026-08-22 | fix(#817): CAMPPlus trusted a default over the weight (#1739) | +| `1757330006f6` | 2026-08-23 | fix(BACKEND-TENSTORRENT-GDN): W2 review repairs (#1715) | + +The 20 that also fail the grep step are the subset of the above whose message +carries no `FOLLOWING_AGENTS_PROTOCOL` string at all: `7572b0f4e2fb`, +`7ba9a675f491`, `7965f12bf4bc`, `373aa125142a`, `9f2b9bb9a30b`, `be4a3edf17b2`, +`6680aab68912`, `ca01719e6b29`, `b5a5f3b182d7`, `6e6bba63d7c1`, `bc570da0d387`, +`34962d96bea0`, `b3d0f3ed5dc8`, `04be1390b227`, `b5f27c9a4c7d`, `2688e6586675`, +`e34d71379e70`, `aba8d5ffb77c`, `2d2a66715ef4`, `1757330006f6`. The remaining 15 +carry the marker in a form the strict contract rejects. + +### Fail `check-role-discipline.py` (7) + +Repository changes that reached `main` without arriving on a task branch. + +| Commit | Date | Subject | +|---|---|---| +| `dd8a3b0e184c` | 2026-08-17 | windows: fix native MSVC/Vulkan build portability | +| `8daf58e7752f` | 2026-08-18 | fix(ENG-RELEASE-WINDOWS): the api-server gate can report its own failure again | +| `38ec0da4aae8` | 2026-08-18 | feat(BACKEND-ROCM): register a ROCm attention backend for kROCM | +| `5073df62228e` | 2026-08-18 | feat(BACKEND-ROCM): select the attention backend in the runner | +| `65d6cdaed3e2` | 2026-08-18 | build: make the tree compile on gcc 16, and add a CI lane so it stays that way | +| `6e73bdee3ea1` | 2026-08-23 | fix(LTX25-POSITION-CONTRACT): gate the tower positions as integers | +| `c00b99c7c8b6` | 2026-08-24 | fix(LTX25-DIT-ATTN-ARM-PARSE): match every DiT attention arm exactly and refuse a fourth value | + +`c00b99c7c8b6` is the 42nd and the newest. It landed +`src/vllm/model_executor/models/ltx2_device.cpp` and +`tests/vllm/models/test_ltx2_device.cpp` — product code and its test — straight +onto `main` with no `row/` branch in its history, which is what +`check-role-discipline.py` names. The checker writes one unwrapped line, and +this is that line verbatim, copied from +`python3 scripts/check-role-discipline.py --base b207f34d3 --head c00b99c7c`, +which exits 1: + +``` +ERROR: c00b99c7c: repository change (src/vllm/model_executor/models/ltx2_device.cpp, tests/vllm/models/test_ltx2_device.cpp) reached main without arriving on a task branch. Work happens in its own worktree on a `row/` branch and lands through a reviewed PR or an authorized local merge naming that branch; never directly on the shared checkout +``` + +Its message is clean: it carries `FOLLOWING_AGENTS_PROTOCOL` and passes +`check-commit-trailers.py`. This is a **role-discipline** violation and not a +trailer one, so it is the seventh row of this table and not the thirty-sixth of +the one above, and the split across the 42 is 35 trailers / 7 role discipline +rather than 35 / 6. + +`check-now-current.py` passes over the whole range and forgives nothing. + +**Every one of the 42 shas above resolves.** Verified with +`git rev-parse --verify -q '^{commit}'` over all 42, first at `origin/main` +`d60692c89` and again at `3574065e7` after the merge, both times against the +shas parsed back out of this committed table rather than a hand-kept copy: 42 +resolved, 0 missing, and all 42 are ancestors of `origin/main` by +`git merge-base --is-ancestor`. This is a full sweep and not a +spot-check, because after this lands the enumeration is the only witness that +these violations happened, and an earlier round of this list carried two shas +that resolved to nothing. + +### The first advance + +**This is the first exercise of `### Advancing the floor`, and it happened +before the pull request that introduces the mechanism had merged.** The row's +own risk 1 — "a violating commit lands between the recorded floor and the merge +of this row" — arrived on 2026-08-24, one day after the floor was recorded, and +was resolved by the procedure the row defines rather than by an exception to it. + +Two things are worth reading off that, and they point in opposite directions. +The mechanism works: the deadlock the row exists to break re-formed at a scale +of one commit instead of 41, and a one-line reviewed edit cleared it. And the +violations are still arriving: `c00b99c7c` is the second role-discipline +violation in two days, after `6e73bdee3ea1` on 2026-08-23. The floor is a way +to stop an unrepairable commit freezing a gate. It is not a fix for whatever is +putting product code on `main` without a task branch. + +**The advance is minimal by construction.** Measured at `origin/main` +`d60692c89`, ten first-parent commits sat above `bacb71109`, and exactly one of +them violated anything. The floor moved to that commit and no further. Setting +it to `origin/main` instead would have been one character of extra typing and +would have forgiven `e6f4f566f` and `d60692c89` unexamined, which is the abuse +risk 2 says nothing in this mechanism can detect. Both stay enforced. + +## Gates + +| Gate | Command | Result | +|---|---|---| +| G1 base selection | `python3 tests/scripts/test_ci_walk_base.py` | PASS, 31 tests | +| G2 cancelled-run losslessness | `python3 -m unittest tests.scripts.test_ci_walk_base.CancelledRunLosslessTests`, plus the live replay in `## Outcome` | PASS | +| G3 a new violation still reds | a scratch commit replayed through the real `ci.yml` step bodies | PASS, red in both directions | +| G4 the deadlock is broken | the same bodies replayed on unmutated `HEAD` | PASS, three gates green | +| G5 the floor is load-bearing | the floor moved back to the frozen base, and the four call sites deleted | PASS, both mutations red | +| G6 preflight | `scripts/agent-preflight.sh` | PASS | + +## Risks + +1. **A violating commit lands between the recorded floor and the merge of this + row.** The gate reds on that one commit, correctly, and the remedy now exists: + advance the floor in a reviewed commit that names it. This is the designed + behaviour and not a regression, but it means the floor value has to be + re-checked immediately before merge. **This risk fired.** `c00b99c7c` landed + on 2026-08-24 and the floor was advanced to it by exactly that remedy; see + `### The first advance`. The obligation it names does not expire with this + one discharge — `main` moves roughly every twenty minutes, so whoever merges + re-runs the four commands over `..origin/main` again. +2. **The floor is set too far forward by mistake.** It would skip commits nobody + examined, and **nothing in this change detects it.** The two ancestry guards + cover a different mistake: `resolve_base` warns and leaves the base alone + when the floor is not an ancestor of `HEAD`, and `RecordedFloorTests` fails + when the recorded floor is not a real ancestor of `HEAD`. Both are about a + floor that is not on this history — typed ahead of `HEAD`, or from another + branch — and a floor advanced too far to a commit that really is on `main` + is an ancestor of `HEAD`, so both accept it. Measured on this branch at + `f7ef4fe19`, with the floor set to `HEAD` itself, the maximally + over-forgiving value: `read_floor`'s sha pattern accepts it, `known` and + `is_ancestor(floor, HEAD)` are both true — which is every assertion + `RecordedFloorTests` makes — the resolver prints `base fafa16f0f… is behind + the enforcement floor; walking from f7ef4fe19… instead` and returns `HEAD`, + and the walk `HEAD..HEAD` is empty, so the grep step iterates zero commits + and `check-commit-trailers.py`, `check-role-discipline.py` and + `check-now-current.py` each return **rc 0 vacuously**. This is not the + fail-closed case in `## Outcome` G5; an unreadable record is an error, and an + over-forward but readable one is a silent pass. The only mitigation is the + one in `### Advancing the floor`: the value moves only in a reviewed pull + request whose body names every commit the advance forgives. The review is + the control, and there is no second one. +3. **The script fails and takes four gates with it.** It runs under `set -eu` in + a command substitution, so a crash reds the job. That is fail-closed and the + right direction, but it makes the script's own suite load-bearing; it is + registered in `agent-record` and in `scripts/agent-preflight.sh`. +4. **Someone re-inlines the base selection into the YAML.** The suite asserts + that `ci.yml` carries no residual `base="${LAST_GREEN:-}"` fallback and that + the script is invoked once per diff-scoped step, so a re-inlining reds. + +## Owed + +Nothing. The 42 commits are recorded above rather than owed: no future change can +repair them. + + +## Outcome + +Measured on `row/1809` at `6de046d36`, against a clone of that commit with `main` +pointed at it, so a push to `main` could be replayed without touching `main`. + +### The instrument + +`scripts/ci-walk-base.py` is exercised by its own suite. The three gates are +exercised by READING their `run:` bodies out of `.github/workflows/ci.yml` with +a YAML parser and executing those exact bytes, with each step's declared `env:` +resolved from a supplied event payload. Nothing about the gates is transcribed, +so a change to the workflow changes the evidence. + +One trap was hit and is recorded because it invalidates this class of result: the +first no-trailer mutation carried the words "no `FOLLOWING_AGENTS_PROTOCOL` +paragraph" in its own body, which SATISFIED the presence grep and read as a +passing gate. The mutation was re-authored to name no marker at all +(`marker_count=0`, printed before each run) and the gate then went red. + +### G4 — the deadlock is broken + +Replaying a push of `6de046d36` with `LAST_GREEN = fafa16f0f` (the real frozen +value) and `before = bacb71109`: + +| Step | rc | +|---|---| +| `commit-protocol-tag` / presence grep | 0 | +| `commit-protocol-tag` / strict trailer walk | 0 | +| `documentation-checkpoint` | 0 | + +The resolver printed `base fafa16f0f… is behind the enforcement floor; walking +from bacb71109… instead` on each. All three are red on `main` today. + +### G5 — the floor is what makes them green + +Two mutations, each restored and each verified restored by `sha256sum -c` with a +clean `git status`. + +| Mutation | Diff | Result | +|---|---|---| +| the floor moved back to `fafa16f0f`, the frozen base | `scripts/ci-enforcement-floor.txt \| 2 +-` | rc 1, 1, 1 — the same 20 grep violations, the same 35 strict ones, the same 6 role-discipline ones | +| the four resolver call sites replaced by the old inline selection | `.github/workflows/ci.yml \| 40 ++++----` | rc 1, 1, 1, and `test_ci_walk_base.py` red at 2 of 31 (`WorkflowWiringTests`) | +| the floor record emptied to a comment, testing FAIL-CLOSED | `scripts/ci-enforcement-floor.txt \| 26 +-` | rc **2**, 2, 2, each step aborting under `set -eu` with `must hold exactly one commit sha outside its comments, found 0` before any checker ran | + +The third is the one that had been asserted rather than executed. A floor record +that cannot be read is an ERROR and never "no floor": reading a broken record as +absent would restore the ratchet silently, which is the failure this file exists +to end. `set -eu` makes the command substitution's non-zero status abort the +step, so the direction is fail-closed and now measured. + +The second is the reachability mutation: a resolver nothing calls resolves +nothing, and both the gates and the focused suite notice the deletion. + +### G3 — a new violation on a new commit still reds + +Each mutant is a real commit authored on top of `6de046d36`, replayed as a push +whose `before` is `6de046d36`. A positive control shares the mutants' path and +trailers so a red is attributable to the violation and not to the harness. + +| Mutant | grep | strict | doc-checkpoint | +|---|---|---|---| +| `059dfb59d`, no marker and no trailers | **1** | **1** | 0 | +| `d47fd7e8b`, product path, full trailers, no PR reference | 0 | 0 | **1** | +| `003b71af0`, the control: same product path, full trailers, `(#1809)` | 0 | 0 | 0 | + +The strict walk named `059dfb59d` on all three of its contract clauses, and the +role-discipline step named `d47fd7e8b` with the path it touched. + +### G2 — a cancelled run is still lossless + +The unit case is `CancelledRunLosslessTests`, which replays C1-green, +C2-cancelled, C3-pushed against a throwaway repository and carries the naive +`github.event.before` base as its positive control. + +Replayed live through the real step bodies as well, with the floor in place. C2 +is a VIOLATING commit whose run is cancelled, so `LAST_GREEN` stays at +`6de046d36`, and C3 is clean: + +| Run | base | grep | strict | +|---|---|---|---| +| C3 pushed, `before = C2`, `LAST_GREEN = 6de046d36` | `6de046d36` | **1**, naming `7f28451310f5` | **1** | + +The cancelled run's violation is caught by the next run. Under the naive base, +`rev-list C2..C3` returns `['54d9021a…']` alone and `7f28451310f5` is covered by +nothing, which is #863 exactly. The floor did not interfere in this replay +because it was placed behind `LAST_GREEN`. That is the arrangement the +losslessness argument needs, and, as +`### Cancelled runs stay lossless` now records, it is **not** the arrangement +`main` is in: `LAST_GREEN` has been frozen since 2026-08-12 and the floor is +ahead of it, so the resolved base on `main` is the floor. The replay proves the +clamp does not break losslessness when it is behind; it does not claim `main` +is there. + +### What was rejected while implementing + +Three existing assertions matched the old inline shell as a STRING and had to +move rather than be deleted, because the rule they were about now lives in the +resolver: + +- `test_main_baseline.py`'s shim stubbed every `python3` call, which made the + resolver return an empty base and skipped the very checker calls two cases + exist to require. The shim now EXECUTES the resolver — it is not a checker, it + is the thing that decides what the checkers get — so those cases test the real + composition rather than a transcription of the rule. +- `test_the_base_falls_back_when_no_successful_run_is_found` asserted the literal + `base="$PUSH_BASE"`. It now asserts that both event values reach the resolver + AND executes the resolver to prove the degradation, which is stronger than the + literal it replaces. +- `test_agent_gates.py`'s `test_ci_role_suite_uses_exact_event_range_not_detached_head` + asserted `base="$PR_BASE"`. It now asserts the event values reach the resolver, + refuses three checkout-derived base forms, and executes the resolver on both + lanes. + +Widening any of the three to make it pass was available and was not taken. A +string match that no longer sees the rule is not a weaker gate, it is no gate. + +### Residue + +`main` advanced to `849a7dd73` while this row was in flight, and the floor was +re-checked against it rather than assumed. Both new commits, `0a0a53e5a` and +its child `849a7dd73`, are CLEAN on all three gates — +`check-commit-trailers.py --range` returns `OK: commit trailer contract` and +`check-role-discipline.py` returns `OK: every change on main arrived on a task +branch` over `bacb71109..849a7dd73`. That verdict is true of that range and +**superseded** by the re-measurement at `e6f4f566f` further down: `main` moved +again and the floor no longer stands. + +The merge commit on this branch names `0a0a53e5a` as the tip, which is wrong: +`git log --oneline` prints newest first and the pair was read in that order. +`849a7dd73` is the tip and `0a0a53e5a` is its parent. The range measured was +`bacb71109..origin/main`, which covered both either way, so the verdict is +unaffected and only the name was. + +This re-check is not a formality, it is risk 1 arriving. If a violating commit +lands before this merges, the gate reds on that one commit, which is the +designed behaviour, and the remedy is a one-line reviewed floor advance that +names it. Whoever merges this should repeat those commands over +`bacb71109..origin/main`, and one of them now reds — see below. + +`scripts/agent-preflight.sh` does **not** report "All gates green" on this +branch, and the earlier claim that it did at `173b7f32d` was wrong. Rerun at +`f7ef4fe19` against `origin/main` `e6f4f566f`: **rc 0, zero failures, and two +SKIPS** — `commit-trailers` and `commit-style`, both with the reason +`origin/main … is not an ancestor of HEAD, so this branch is behind it and the +trailer gates did NOT run`. The script says so itself in the same breath: +`NOT a green preflight: a skipped gate reported nothing about this tree`. A rc 0 +that carries a SKIP is exactly the third state `scripts/agent-preflight.sh` +documents at its top and the exit status cannot express, so reading the rc alone +is how the wrong claim was made. + +Both skipped gates were therefore run by hand over the branch's own range, +`849a7dd73..HEAD` — the merge base with `origin/main` to the head, which covers +every commit this branch adds. Measured at `f7ef4fe19` and measured again at the +head of this repair, with the same verdict both times: + +| Gate | Command | rc | +|---|---|---| +| `commit-trailers` | `python3 scripts/check-commit-trailers.py --range 849a7dd73..HEAD` | **0**, `OK: commit trailer contract` | +| `commit-style` | `python3 scripts/check-commit-style.py --range 849a7dd73..HEAD` | **0**, `OK: commit writing style` | + +The SKIP was not a defect in these commits: it was the branch being behind +`origin/main`, and a trial merge conflicted in `scripts/agent-preflight.sh`'s +`SUITES` array against `af320abb2`, which also meant GitHub could produce no +merge ref for the pull request lane to check out. + +**Both are resolved.** `origin/main` `3574065e7` is merged into the branch and +the conflict is taken as a union of the two additions: `af320abb2`'s +`test_ltx2_dit_attn_knob_arms`, `test_ltx25_ab_memwatch` and +`test_tower_skip_rss_report`, plus this branch's `test_ci_walk_base`. Rerun +after the merge, `scripts/agent-preflight.sh` reports `ok commit-trailers` and +`ok commit-style` inside the script, against `origin/main` `3574065e7`, with no +SKIP. The merge is also what makes the advanced floor recordable: +`RecordedFloorTests::test_recorded_floor_is_an_ancestor_of_head` asserts the +recorded value is an ancestor of `HEAD`, `c00b99c7c` landed on `main` after this +branch left it, and the suite reds at 1 of 31 without the merge and is 31 of 31 +with it. `.agents/issue-index.md`'s #1809 row appears exactly once afterwards +and is no longer the tail, which is correct for an append-only union file; its +prose still says 41 because an index row is never rewritten. + +One site of the corrected `LAST_GREEN` claim is deliberately left alone: +`scripts/ci-walk-base.py`'s module docstring still calls a floor behind +`LAST_GREEN` "the steady state". The resolver, its data file, `ci.yml`, its +suite and the three checkers were all excluded from this review repair so that +the reviewed mechanism stays byte-identical, and a docstring edit inside that +boundary is not worth reopening it for. It is named here rather than left to be +found: the sentence is wrong for the same reason `### Cancelled runs stay +lossless` was, and it should go in whichever change next touches that file. + +**Risk 1 arrived, and the floor has been advanced once. RESOLVED.** It was +first measured 2026-08-23T22:30Z at `origin/main` `e6f4f566f`, over +`bacb71109..origin/main`, nine first-parent commits: the presence grep rc 0, +`check-commit-trailers.py` rc 0, `check-now-current.py` rc 0, and +**`check-role-discipline.py` rc 1** on `c00b99c7c` +(`fix(LTX25-DIT-ATTN-ARM-PARSE)`), which reached `main` with +`src/vllm/model_executor/models/ltx2_device.cpp` and its test without arriving +on a task branch. That review repair deliberately left the floor alone, because +advancing it silently inside a review repair is what `### Advancing the floor` +forbids. + +The advance is this commit, and it is a separate reviewed act with the argument +in its body. Re-measured independently at `origin/main` `d60692c89`, ten +first-parent commits, so the finding was reproduced rather than inherited: + +| Floor | grep step | `check-commit-trailers.py` | `check-role-discipline.py` | `check-now-current.py` | +|---|---|---|---|---| +| `bacb71109` (old) | rc 0 | rc 0 | **rc 1**, `c00b99c7c` | rc 0 | +| `c00b99c7c` (new) | rc 0 | rc 0 | rc 0 | rc 0 | + +Each rc was captured as `rc=$?` on the command itself and never after a pipe, +which reports the last stage of the pipeline and has misread a red as a green +twice in this row's history. `git merge-base --is-ancestor c00b99c7c +origin/main` is rc 0, so the new floor is on this history and the resolver's +ancestry guard accepts it. `tests/scripts/test_ci_walk_base.py` re-run over the +new value: 31 tests, 0 failures, 0 errors. + +The gate still bites over the new floor. A scratch commit on top of +`origin/main` whose message contains no `FOLLOWING_AGENTS_PROTOCOL` string +anywhere — marker count printed as **0** before the run, because a mutation +whose own message mentions the marker satisfies the presence grep and reads as +a pass — reds both trailer steps by name over `c00b99c7c..`. The tree +was restored and the restore proved by sha256 on both changed files, not by a +`git status` that a mutation can leave clean. + +Nothing about the value is permanent. `main` keeps moving, so whoever merges +repeats the four commands over `c00b99c7c..origin/main` and, if one reds again, +performs another advance the same way. + +`test_cpu_x86_llamacpp_floor` was red in two earlier runs and is #618, not this +row. It was discriminated rather than asserted, twice over. Pristine +`origin/main` at `0a0a53e5a`, run serially in its own clone with no change from +this branch in it, fails +`test_a_contended_leg_is_discarded_and_never_summarised` and +`test_the_published_figures_are_computed_not_transcribed` with +`NO_QUIET_WINDOW` at loadavg 63-68. And the green run above is the same tree as +the red one plus a prose edit, taken after the box quietened, which is what a +load-dependent harness does. Never asserted from the green run alone: a gate +that passes once is not a gate that cannot fail. + +`agent-record`'s missing-`hugo` red (#1722, #1726) and the two `windows-msvc` +reds (#584) are inherited and unaffected by this row. + +## Now + +`DONE` pending review. Spec committed ahead of the implementation on `row/1809`; +the evidence above was measured on the implementation commit. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a9d50e139..8600c0833 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -533,18 +533,21 @@ jobs: set -eu pending_args=() if [ "$EVENT_NAME" = "pull_request" ]; then - base="$PR_BASE" 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:-}" - [ -n "$base" ] || base="$PUSH_BASE" head="$PUSH_HEAD" fi + # ONE resolver for every diff-scoped base in this file (#1809). The base + # is still the last SUCCESSFULLY gated commit, so a cancelled run stays + # lossless (#822, #863). It is now clamped from below by the recorded + # enforcement floor, `scripts/ci-enforcement-floor.txt`, so a commit + # already on `main` that no contributor can repair cannot freeze the + # base and turn the walk into a ratchet. + base="$(python3 scripts/ci-walk-base.py \ + --event "$EVENT_NAME" --head "$head" \ + --pr-base "${PR_BASE:-}" --push-base "${PUSH_BASE:-}" \ + --last-green "${LAST_GREEN:-}")" if [ -z "$base" ]; then echo "no diff range on the $EVENT_NAME lane: role discipline is" echo "diff-scoped and every commit's own push already ran it." @@ -554,6 +557,23 @@ jobs: fi python3 tests/scripts/test_agent_role.py python3 tests/scripts/test_agent_onboard.py + - name: The diff-scoped gates resolve their base through one tested rule (#1809) + # Registered in TWO places, like the baseline and preflight suites above: + # here and in the `SUITES` array of `scripts/agent-preflight.sh`. + # + # The rule it covers WAS four byte-similar copies of inline shell in this + # file. `test_main_baseline.py::AgentRecordDiffRangeTests` replayed ONE of + # them, `agent-record`'s, under a shim that stubs every `python3` call -- + # so it pinned which checker ran with which range string and could not + # see the base rule -- and the other three were executed by nothing. That + # is why the ratchet in #1809, an unrepairable commit freezing the base so + # every later push re-walked the same 41 violations over a range one + # commit wider, could only be found by reading a job log. The suite replays a + # cancelled-run sequence against a real throwaway repository, with the + # naive `github.event.before` base as its positive control, so the + # property #822 and #863 bought cannot be lost silently. + run: | + python3 tests/scripts/test_ci_walk_base.py - name: Claim view, helper queue and PR reviewability run: | python3 scripts/claim-view.py --check @@ -633,17 +653,20 @@ jobs: run: | set -eu if [ "$EVENT_NAME" = "pull_request" ]; then - 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:-}" - [ -n "$base" ] || base="$PUSH_BASE" head="$PUSH_HEAD" fi + # ONE resolver for every diff-scoped base in this file (#1809). The base + # is still the last SUCCESSFULLY gated commit, so a cancelled run stays + # lossless (#822, #863). It is now clamped from below by the recorded + # enforcement floor, `scripts/ci-enforcement-floor.txt`, so a commit + # already on `main` that no contributor can repair cannot freeze the + # base and turn the walk into a ratchet. + base="$(python3 scripts/ci-walk-base.py \ + --event "$EVENT_NAME" --head "$head" \ + --pr-base "${PR_BASE:-}" --push-base "${PUSH_BASE:-}" \ + --last-green "${LAST_GREEN:-}")" python3 scripts/check-now-current.py --base "$base" --head "$head" pending_args=() if [ "$EVENT_NAME" = "pull_request" ]; then @@ -725,18 +748,29 @@ jobs: fetch-depth: 0 - name: Every new commit carries FOLLOWING_AGENTS_PROTOCOL 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 }} + PUSH_HEAD: ${{ github.sha }} run: | set -eu - if [ "${{ github.event_name }}" = "pull_request" ]; then - base="${{ github.event.pull_request.base.sha }}" - head="${{ github.event.pull_request.head.sha }}" + if [ "$EVENT_NAME" = "pull_request" ]; then + head="$PR_HEAD" else - # Same self-healing base as the strict walk below (#822, #863). - base="${LAST_GREEN:-}" - [ -n "$base" ] || base="${{ github.event.before }}" - head="${{ github.sha }}" + head="$PUSH_HEAD" fi + # ONE resolver for every diff-scoped base in this file (#1809). The base + # is still the last SUCCESSFULLY gated commit, so a cancelled run stays + # lossless (#822, #863). It is now clamped from below by the recorded + # enforcement floor, `scripts/ci-enforcement-floor.txt`, so a commit + # already on `main` that no contributor can repair cannot freeze the + # base and turn the walk into a ratchet. + base="$(python3 scripts/ci-walk-base.py \ + --event "$EVENT_NAME" --head "$head" \ + --pr-base "${PR_BASE:-}" --push-base "${PUSH_BASE:-}" \ + --last-green "${LAST_GREEN:-}")" # Enforce on the FIRST-PARENT mainline (the commits the submitter # directly lands on the target branch, incl. merge commits which are # skipped below). Merged-in feature-branch commits are owned by the @@ -784,18 +818,20 @@ jobs: run: | set -eu if [ "$EVENT_NAME" = "pull_request" ]; then - 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:-}" - [ -n "$base" ] || base="$PUSH_BASE" head="$PUSH_HEAD" fi + # ONE resolver for every diff-scoped base in this file (#1809). The base + # is still the last SUCCESSFULLY gated commit, so a cancelled run stays + # lossless (#822, #863). It is now clamped from below by the recorded + # enforcement floor, `scripts/ci-enforcement-floor.txt`, so a commit + # already on `main` that no contributor can repair cannot freeze the + # base and turn the walk into a ratchet. + base="$(python3 scripts/ci-walk-base.py \ + --event "$EVENT_NAME" --head "$head" \ + --pr-base "${PR_BASE:-}" --push-base "${PUSH_BASE:-}" \ + --last-green "${LAST_GREEN:-}")" if [ -z "$base" ] || ! git cat-file -e "${base}^{commit}" 2>/dev/null; then echo "no usable diff range on the $EVENT_NAME lane: base=${base:-}" echo "checking the tip commit alone rather than passing vacuously." diff --git a/scripts/agent-preflight.sh b/scripts/agent-preflight.sh index 4b74b362d..0989f43e3 100755 --- a/scripts/agent-preflight.sh +++ b/scripts/agent-preflight.sh @@ -173,6 +173,7 @@ SUITES=( test_ltx2_dit_attn_knob_arms test_ltx25_ab_memwatch test_tower_skip_rss_report + test_ci_walk_base ) failed=() diff --git a/scripts/ci-enforcement-floor.txt b/scripts/ci-enforcement-floor.txt new file mode 100644 index 000000000..d59c18f47 --- /dev/null +++ b/scripts/ci-enforcement-floor.txt @@ -0,0 +1,32 @@ +# THE ENFORCEMENT FLOOR for the diff-scoped gates on `main`. +# +# One commit sha. The gates that walk `BASE..HEAD` -- the two trailer steps of +# `commit-protocol-tag`, `documentation-checkpoint`, and `agent-record`'s role +# discipline step -- never start behind it. `scripts/ci-walk-base.py` reads this +# file and takes the base as the newer of the last GREEN push run and this +# commit, decided by ancestry. +# +# WHY A FLOOR EXISTS. The base is the head of the last SUCCESSFUL push run, so a +# cancelled run is lossless: the next run simply walks wider (#822, #863). That +# is right, and it is not reverted. What it did not anticipate is a violation +# with NO AVAILABLE REMEDY -- a commit already on `main`, which can only be +# repaired by rewriting `main`. One of those freezes the base forever and the +# walk becomes a ratchet (#1809). +# +# ADVANCING IT IS A DELIBERATE, REVIEWED ACT and never a side effect. Edit this +# one line in a pull request whose body names every commit the advance forgives +# and says why each cannot be repaired. `git log -p` on this file is the +# complete history of the floor; there is no registry and no growing list. +# +# THE CURRENT VALUE forgives 42 commits dated 2026-08-13 to 2026-08-24, each one +# enumerated with its sha, date and subject in +# `.agents/specs/ci-enforcement-floor.md`. That record is the only place that +# now says those violations happened. +# +# THE FLOOR IS THE FORGIVEN COMMIT ITSELF, not the one after it. The walk is +# `FLOOR..HEAD`, which excludes `FLOOR`, so setting it to the violating commit +# forgives that commit and every commit before it, and leaves every commit +# after it enforced. Advance it to the violation, never past it: a floor set +# further forward forgives whatever lands in between, unexamined, and nothing +# in this mechanism can detect that. The review is the only control. +c00b99c7c8b64f9247230ed6220598cc5c0e347e diff --git a/scripts/ci-walk-base.py b/scripts/ci-walk-base.py new file mode 100755 index 000000000..e939c7f9a --- /dev/null +++ b/scripts/ci-walk-base.py @@ -0,0 +1,205 @@ +#!/usr/bin/env python3 +"""Resolve the BASE commit for the diff-scoped gates, clamped by a recorded floor. + +`.github/workflows/ci.yml` runs four diff-scoped steps over `BASE..HEAD`: both +trailer steps of `commit-protocol-tag`, `documentation-checkpoint`, and +`agent-record`'s role-discipline step. Each one used to choose its own base in a +byte-similar copy of the same inline shell, and no test in this tree executed any +of those four blocks. + +WHAT THE BASE IS. On the pull-request lane it is `pull_request.base.sha`. On the +push lane it is the head of the last SUCCESSFUL push run of this workflow, +falling back to `github.event.before`. The successful-run base is deliberate: a +cancelled run must not advance the base, or its commits are skipped and nothing +re-covers them, which is what lets the push lane be latest-only (#822, #863). + +WHY IT NEEDS A FLOOR. That design assumes a green run is eventually reachable. +A commit already on `main` that violates a gate cannot be repaired, because the +only repair is a rewrite `AGENTS.md` forbids, so no green run is reachable, the +base freezes, and every later push re-walks the same violations over a range one +commit wider (#1809). The floor -- `scripts/ci-enforcement-floor.txt` -- is one +commit the walk never goes behind. + +CANCELLED RUNS STAY LOSSLESS. The floor is a LOWER CLAMP on an otherwise +unchanged base. While it sits behind the last green commit, which is the steady +state because that commit advances on every green push and the floor advances +only when a human commits an advance, the resolved base is byte-identical to +what it was before this script existed. The only window that skips anything is +`last_green..floor` immediately after an advance, which is the forgiveness being +asked for and is enumerated in `.agents/specs/ci-enforcement-floor.md`. + +NEWER MEANS ANCESTRY, NOT DATE. A commit date is author-controlled and can move +backwards across a rebase, so comparing dates can pick the wrong commit. +`git merge-base --is-ancestor` is the honest primitive and is what is used here. + +Unit-tested by tests/scripts/test_ci_walk_base.py. +""" + +from __future__ import annotations + +import argparse +import re +import subprocess +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +DEFAULT_FLOOR_FILE = ROOT / "scripts/ci-enforcement-floor.txt" + +SHA = re.compile(r"\A[0-9a-f]{40}\Z") + + +class FloorError(ValueError): + """The recorded floor cannot be read as one commit sha.""" + + +def read_floor(path: Path) -> str: + """Return the single sha recorded in ``path``. + + Fails closed. A floor record that cannot be read as exactly one lowercase + 40-byte sha is an error rather than "no floor": silently reading a broken + record as absent restores the ratchet this file exists to break, and does it + without saying so. + """ + + try: + text = path.read_text(encoding="utf-8") + except OSError as exc: + raise FloorError(f"cannot read the enforcement floor {path}: {exc}") from exc + values = [ + line.strip() + for line in text.splitlines() + if line.strip() and not line.lstrip().startswith("#") + ] + if len(values) != 1: + raise FloorError( + f"{path} must hold exactly one commit sha outside its comments, found {len(values)}" + ) + if SHA.fullmatch(values[0]) is None: + raise FloorError( + f"{path} must hold one lowercase 40-byte commit sha, found {values[0]!r}" + ) + return values[0] + + +def _git(repo: Path, *args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["git", "-C", str(repo), *args], + text=True, + capture_output=True, + check=False, + ) + + +def known(repo: Path, revision: str) -> bool: + """Whether git can resolve ``revision`` to a commit object in ``repo``.""" + + if not revision: + return False + return _git(repo, "cat-file", "-e", f"{revision}^{{commit}}").returncode == 0 + + +def is_ancestor(repo: Path, older: str, newer: str) -> bool: + """Whether ``older`` is an ancestor of ``newer``. Equal commits count.""" + + return _git(repo, "merge-base", "--is-ancestor", older, newer).returncode == 0 + + +def resolve_base( + *, + repo: Path, + event: str, + head: str, + pr_base: str, + push_base: str, + last_green: str, + floor: str, + warn=lambda message: print(message, file=sys.stderr), +) -> str: + """Return the base commit the diff-scoped walk starts from.""" + + if event == "pull_request": + # UNCHANGED, on purpose. This lane bases on the merge base and has been + # green throughout the freeze the floor exists to end. Applying the + # floor here could only ever RAISE a base, which narrows what the lane + # enforces, and no defect asks for that. + return pr_base + + base = last_green or push_base + + if not base or not known(repo, base): + # The all-zero sha of a new branch, or a `before` the history no longer + # contains. The caller's own guard degrades to the tip commit alone, and + # that behaviour is preserved byte-for-byte: the floor RAISES a usable + # base and never substitutes for an unusable one. + return base + + if not floor: + return base + + if not known(repo, floor): + warn(f"enforcement floor {floor} is not in this checkout; base left at {base}") + return base + + if not is_ancestor(repo, floor, head): + # A floor the current history does not contain cannot bound it, and + # `floor..head` across unrelated history is not a range anybody asked + # for. This is also the guard against a floor typed AHEAD of HEAD. + warn(f"enforcement floor {floor} is not an ancestor of {head}; base left at {base}") + return base + + if is_ancestor(repo, floor, base): + return base + + warn(f"base {base} is behind the enforcement floor; walking from {floor} instead") + return floor + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--event", required=True, help="github.event_name") + parser.add_argument("--head", default="", help="the walk's head commit") + parser.add_argument("--pr-base", default="", help="pull_request.base.sha") + parser.add_argument("--push-base", default="", help="github.event.before") + parser.add_argument("--last-green", default="", help="head of the last successful push run") + parser.add_argument( + "--floor", + default=None, + help="override the recorded floor; an empty value means no floor", + ) + parser.add_argument( + "--floor-file", + default=None, + help=f"read the floor from this file instead of {DEFAULT_FLOOR_FILE}", + ) + parser.add_argument("--repo", default=".", help="repository to resolve commits in") + args = parser.parse_args(argv) + + if args.floor is not None: + floor = args.floor.strip() + if floor and SHA.fullmatch(floor) is None: + print(f"--floor must be one lowercase 40-byte commit sha, got {floor!r}", file=sys.stderr) + return 2 + else: + path = Path(args.floor_file) if args.floor_file else DEFAULT_FLOOR_FILE + try: + floor = read_floor(path) + except FloorError as exc: + print(str(exc), file=sys.stderr) + return 2 + + base = resolve_base( + repo=Path(args.repo), + event=args.event, + head=args.head, + pr_base=args.pr_base, + push_base=args.push_base, + last_green=args.last_green, + floor=floor, + ) + print(base) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/scripts/test_agent_gates.py b/tests/scripts/test_agent_gates.py index d4e334015..fb9b54c59 100755 --- a/tests/scripts/test_agent_gates.py +++ b/tests/scripts/test_agent_gates.py @@ -78,12 +78,44 @@ def test_ci_role_suite_uses_exact_event_range_not_detached_head(self) -> None: role_step = workflow.split( "- name: Agent role machinery and role discipline", 1 )[1].split("- name: Claim view, helper queue and PR reviewability", 1)[0] - self.assertIn('base="$PR_BASE"', role_step) self.assertIn('head="$PR_HEAD"', role_step) - self.assertIn('base="$PUSH_BASE"', role_step) self.assertIn('head="$PUSH_HEAD"', role_step) + # The base selection moved out of this step and into + # `scripts/ci-walk-base.py` (#1809). What this case is about does not + # change: the base comes from the EVENT payload and never from the + # runner's checkout. Both event values still reach the resolver, the + # resolver's output is the base, and the lane split it applies is + # asserted by EXECUTING it rather than by matching a string that no + # longer exists. + self.assertIn('base="$(python3 scripts/ci-walk-base.py', role_step) + self.assertIn('--pr-base "${PR_BASE:-}"', role_step) + self.assertIn('--push-base "${PUSH_BASE:-}"', role_step) + for detached in ('base="HEAD', '--base "HEAD', 'base="$(git '): + self.assertNotIn(detached, role_step, "the base must not come from the checkout") self.assertIn('pending_args=(--pending-pr-head "$PR_HEAD")', role_step) self.assertIn('--base "$base" --head "$head"', role_step) + resolver = [ + sys.executable, + str(ROOT / "scripts/ci-walk-base.py"), + "--head", "b" * 40, + "--pr-base", "a" * 40, + "--push-base", "c" * 40, + "--last-green", "d" * 40, + "--floor", "", + "--repo", str(ROOT), + ] + self.assertEqual( + subprocess.check_output( + [*resolver, "--event", "pull_request"], text=True + ).strip(), + "a" * 40, + "the pull request lane must base on the pull request event's base", + ) + self.assertEqual( + subprocess.check_output([*resolver, "--event", "push"], text=True).strip(), + "d" * 40, + "the push lane must base on the last gated commit", + ) class ReadyContractTests(unittest.TestCase): diff --git a/tests/scripts/test_ci_walk_base.py b/tests/scripts/test_ci_walk_base.py new file mode 100644 index 000000000..79a441748 --- /dev/null +++ b/tests/scripts/test_ci_walk_base.py @@ -0,0 +1,351 @@ +#!/usr/bin/env python3 +"""Unit and behaviour checks for scripts/ci-walk-base.py. + +The base of the diff-scoped walk used to be four byte-similar copies of inline +shell inside `.github/workflows/ci.yml`. One of them, `agent-record`'s, was +replayed by `test_main_baseline.py::AgentRecordDiffRangeTests` under a shim that +stubs every `python3` call: that pins WHICH checker is invoked and with WHICH +range string, and it cannot see the base rule. The other three were executed by +nothing. So the ratchet in #1809 could only be found by reading a job log. + +Two properties are load-bearing and are tested against a REAL throwaway +repository rather than a mock, because both are statements about ancestry: + + * a CANCELLED run stays lossless -- its commits are still covered by the next + run. This is the property #822 and #863 bought, and the one a floor is most + likely to break silently. `CancelledRunLosslessTests` replays the sequence + and carries the naive `github.event.before` base as its POSITIVE CONTROL, so + a test that stopped discriminating would fail rather than pass; + * the floor CLAMPS the base from below and never substitutes for an unusable + one, so the new-branch/force-push guard downstream keeps its behaviour. +""" + +from __future__ import annotations + +import importlib.util +import re +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +MODULE = ROOT / "scripts/ci-walk-base.py" +FLOOR_FILE = ROOT / "scripts/ci-enforcement-floor.txt" +WORKFLOW = ROOT / ".github/workflows/ci.yml" + +SPEC = importlib.util.spec_from_file_location("ci_walk_base", MODULE) +assert SPEC is not None and SPEC.loader is not None +mod = importlib.util.module_from_spec(SPEC) +sys.modules[SPEC.name] = mod +SPEC.loader.exec_module(mod) + +ZERO = "0" * 40 + + +def git(repo: Path, *args: str) -> str: + return subprocess.check_output( + ["git", "-C", str(repo), *args], text=True, stderr=subprocess.STDOUT + ).strip() + + +class ScratchRepo: + """A linear throwaway history, so ancestry questions have real answers.""" + + def __init__(self, directory: Path) -> None: + self.path = directory + git(directory, "init", "--quiet", "--initial-branch=main", ".") + git(directory, "config", "user.email", "test@example.invalid") + git(directory, "config", "user.name", "Test") + git(directory, "config", "commit.gpgsign", "false") + + def commit(self, name: str) -> str: + (self.path / name).write_text(name, encoding="utf-8") + git(self.path, "add", name) + git(self.path, "commit", "--quiet", "-m", name) + return git(self.path, "rev-parse", "HEAD") + + +class RepoCase(unittest.TestCase): + def setUp(self) -> None: + self._tmp = tempfile.TemporaryDirectory(prefix="ci-walk-base-") + self.addCleanup(self._tmp.cleanup) + self.repo = ScratchRepo(Path(self._tmp.name)) + + def resolve(self, **kwargs) -> str: + warnings: list[str] = [] + base = mod.resolve_base( + repo=self.repo.path, + event=kwargs.pop("event", "push"), + head=kwargs.pop("head", ""), + pr_base=kwargs.pop("pr_base", ""), + push_base=kwargs.pop("push_base", ""), + last_green=kwargs.pop("last_green", ""), + floor=kwargs.pop("floor", ""), + warn=warnings.append, + ) + self.assertFalse(kwargs, f"unexpected keyword arguments {sorted(kwargs)}") + self.warnings = warnings + return base + + +class FloorRecordTests(unittest.TestCase): + """The floor record fails CLOSED. A broken record is never read as absent.""" + + def write(self, text: str) -> Path: + tmp = tempfile.NamedTemporaryFile( + "w", suffix=".txt", delete=False, encoding="utf-8" + ) + tmp.write(text) + tmp.close() + path = Path(tmp.name) + self.addCleanup(path.unlink) + return path + + def test_comments_and_blank_lines_are_ignored(self) -> None: + path = self.write(f"# a reason\n\n \n{'a' * 40}\n") + self.assertEqual(mod.read_floor(path), "a" * 40) + + def test_two_shas_are_refused(self) -> None: + path = self.write(f"{'a' * 40}\n{'b' * 40}\n") + with self.assertRaises(mod.FloorError): + mod.read_floor(path) + + def test_empty_record_is_refused_rather_than_read_as_no_floor(self) -> None: + path = self.write("# only a comment\n") + with self.assertRaises(mod.FloorError): + mod.read_floor(path) + + def test_short_sha_is_refused(self) -> None: + path = self.write("bacb71109\n") + with self.assertRaises(mod.FloorError): + mod.read_floor(path) + + def test_uppercase_sha_is_refused(self) -> None: + path = self.write("A" * 40 + "\n") + with self.assertRaises(mod.FloorError): + mod.read_floor(path) + + def test_missing_file_is_refused(self) -> None: + with self.assertRaises(mod.FloorError): + mod.read_floor(Path("/nonexistent/ci-enforcement-floor.txt")) + + +class RecordedFloorTests(unittest.TestCase): + """The floor this repository actually carries is a real ancestor of HEAD.""" + + def test_recorded_floor_parses(self) -> None: + self.assertRegex(mod.read_floor(FLOOR_FILE), r"\A[0-9a-f]{40}\Z") + + def test_recorded_floor_is_an_ancestor_of_head(self) -> None: + floor = mod.read_floor(FLOOR_FILE) + self.assertTrue( + mod.known(ROOT, floor), f"the recorded floor {floor} is not in this checkout" + ) + self.assertTrue( + mod.is_ancestor(ROOT, floor, "HEAD"), + f"the recorded floor {floor} is not an ancestor of HEAD", + ) + + +class BaseResolutionTests(RepoCase): + def test_last_green_ahead_of_the_floor_is_kept(self) -> None: + floor = self.repo.commit("c1") + green = self.repo.commit("c2") + head = self.repo.commit("c3") + self.assertEqual(self.resolve(head=head, last_green=green, floor=floor), green) + self.assertEqual(self.warnings, []) + + def test_last_green_behind_the_floor_is_raised(self) -> None: + green = self.repo.commit("c1") + floor = self.repo.commit("c2") + head = self.repo.commit("c3") + self.assertEqual(self.resolve(head=head, last_green=green, floor=floor), floor) + self.assertEqual(len(self.warnings), 1) + + def test_a_base_equal_to_the_floor_is_kept(self) -> None: + floor = self.repo.commit("c1") + head = self.repo.commit("c2") + self.assertEqual(self.resolve(head=head, last_green=floor, floor=floor), floor) + + def test_before_is_used_when_no_run_was_green(self) -> None: + floor = self.repo.commit("c1") + before = self.repo.commit("c2") + head = self.repo.commit("c3") + self.assertEqual(self.resolve(head=head, push_base=before, floor=floor), before) + + def test_before_behind_the_floor_is_raised(self) -> None: + before = self.repo.commit("c1") + floor = self.repo.commit("c2") + head = self.repo.commit("c3") + self.assertEqual(self.resolve(head=head, push_base=before, floor=floor), floor) + + def test_all_zero_before_is_returned_unchanged(self) -> None: + floor = self.repo.commit("c1") + head = self.repo.commit("c2") + # The new-branch guard downstream turns this into a tip-only check. The + # floor must not quietly widen that into a range. + self.assertEqual(self.resolve(head=head, push_base=ZERO, floor=floor), ZERO) + + def test_a_before_the_history_no_longer_contains_is_returned_unchanged(self) -> None: + floor = self.repo.commit("c1") + head = self.repo.commit("c2") + gone = "d" * 40 + self.assertEqual(self.resolve(head=head, push_base=gone, floor=floor), gone) + + def test_an_empty_base_stays_empty(self) -> None: + floor = self.repo.commit("c1") + head = self.repo.commit("c2") + self.assertEqual(self.resolve(head=head, floor=floor), "") + + def test_no_floor_leaves_the_base_alone(self) -> None: + green = self.repo.commit("c1") + head = self.repo.commit("c2") + self.assertEqual(self.resolve(head=head, last_green=green, floor=""), green) + + def test_a_floor_absent_from_the_checkout_warns_and_changes_nothing(self) -> None: + green = self.repo.commit("c1") + head = self.repo.commit("c2") + self.assertEqual( + self.resolve(head=head, last_green=green, floor="e" * 40), green + ) + self.assertEqual(len(self.warnings), 1) + self.assertIn("not in this checkout", self.warnings[0]) + + def test_a_floor_that_is_not_an_ancestor_of_head_changes_nothing(self) -> None: + green = self.repo.commit("c1") + head = self.repo.commit("c2") + git(self.repo.path, "checkout", "--quiet", "-b", "side", green) + sideways = self.repo.commit("s1") + # `sideways` is newer by date and unreachable from `head`, which is the + # case a date comparison gets wrong and ancestry gets right. + self.assertEqual( + self.resolve(head=head, last_green=green, floor=sideways), green + ) + self.assertIn("not an ancestor", self.warnings[0]) + + def test_a_floor_ahead_of_head_changes_nothing(self) -> None: + green = self.repo.commit("c1") + head = self.repo.commit("c2") + ahead = self.repo.commit("c3") + self.assertEqual(self.resolve(head=head, last_green=green, floor=ahead), green) + self.assertIn("not an ancestor", self.warnings[0]) + + def test_the_pull_request_lane_is_untouched_by_the_floor(self) -> None: + pr_base = self.repo.commit("c1") + floor = self.repo.commit("c2") + head = self.repo.commit("c3") + self.assertEqual( + self.resolve(event="pull_request", head=head, pr_base=pr_base, floor=floor), + pr_base, + ) + self.assertEqual(self.warnings, []) + + +class CancelledRunLosslessTests(RepoCase): + """#822/#863: a cancelled run's commits must still be covered by a later run. + + The sequence: C1's run is green, C2 is pushed and its run is CANCELLED, C3 is + pushed. The last successful run is still C1, so the C3 run must walk from C1 + and its range must contain C2. + """ + + def setUp(self) -> None: + super().setUp() + self.c0 = self.repo.commit("c0") + self.c1 = self.repo.commit("c1") + self.c2 = self.repo.commit("c2") + self.c3 = self.repo.commit("c3") + + def walked(self, base: str, head: str) -> list[str]: + return git(self.repo.path, "rev-list", f"{base}..{head}").splitlines() + + def test_the_cancelled_commit_is_covered_with_a_floor_behind_the_base(self) -> None: + base = self.resolve(head=self.c3, last_green=self.c1, push_base=self.c2, floor=self.c0) + self.assertEqual(base, self.c1) + self.assertIn(self.c2, self.walked(base, self.c3)) + + def test_the_cancelled_commit_is_covered_with_the_floor_at_the_base(self) -> None: + base = self.resolve(head=self.c3, last_green=self.c1, push_base=self.c2, floor=self.c1) + self.assertEqual(base, self.c1) + self.assertIn(self.c2, self.walked(base, self.c3)) + + def test_positive_control_the_naive_before_base_loses_the_cancelled_commit(self) -> None: + # Without this case the two above could pass on a resolver that returned + # anything at all: this is the failure they are asserting the ABSENCE of, + # made to happen on purpose. + self.assertNotIn(self.c2, self.walked(self.c2, self.c3)) + + def test_a_floor_advance_is_the_one_window_that_skips(self) -> None: + # Stated rather than hidden. Advancing the floor past a cancelled run's + # commits skips them, which is exactly the forgiveness a floor advance + # asks for, and is why an advance is a reviewed commit that must name + # what it forgives. + base = self.resolve(head=self.c3, last_green=self.c1, push_base=self.c2, floor=self.c2) + self.assertEqual(base, self.c2) + self.assertNotIn(self.c2, self.walked(base, self.c3)) + + +class CommandLineTests(RepoCase): + def run_script(self, *args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [sys.executable, str(MODULE), "--repo", str(self.repo.path), *args], + text=True, + capture_output=True, + check=False, + ) + + def test_it_prints_the_raised_base(self) -> None: + green = self.repo.commit("c1") + floor = self.repo.commit("c2") + head = self.repo.commit("c3") + result = self.run_script( + "--event", "push", "--head", head, "--last-green", green, "--floor", floor + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.strip(), floor) + + def test_an_unreadable_floor_record_exits_nonzero(self) -> None: + head = self.repo.commit("c1") + result = self.run_script( + "--event", "push", "--head", head, + "--floor-file", "/nonexistent/ci-enforcement-floor.txt", + ) + self.assertEqual(result.returncode, 2) + self.assertIn("enforcement floor", result.stderr) + + def test_a_malformed_floor_argument_exits_nonzero(self) -> None: + head = self.repo.commit("c1") + result = self.run_script("--event", "push", "--head", head, "--floor", "nope") + self.assertEqual(result.returncode, 2) + + +class WorkflowWiringTests(unittest.TestCase): + """Every diff-scoped base in `ci.yml` goes through the script, or this reds. + + A resolver nothing calls resolves nothing. This is the reachability half: + re-inlining the rule into the YAML, or dropping one call site, fails here. + """ + + def setUp(self) -> None: + self.text = WORKFLOW.read_text(encoding="utf-8") + + def test_the_four_diff_scoped_steps_call_the_resolver(self) -> None: + calls = re.findall(r"scripts/ci-walk-base\.py", self.text) + self.assertEqual( + len(calls), + 4, + "the four diff-scoped steps each resolve their base through the script", + ) + + def test_no_step_still_chooses_its_own_base(self) -> None: + inlined = re.findall(r'base="\$\{LAST_GREEN:-\}"', self.text) + self.assertEqual(inlined, [], "a step is choosing its base inline again") + + def test_the_resolver_suite_runs_on_a_lane(self) -> None: + self.assertIn("tests/scripts/test_ci_walk_base.py", self.text) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/scripts/test_main_baseline.py b/tests/scripts/test_main_baseline.py index 7f3721b93..e6554dbe2 100644 --- a/tests/scripts/test_main_baseline.py +++ b/tests/scripts/test_main_baseline.py @@ -783,6 +783,13 @@ def run_shimmed(body: str, environment: dict[str, str]) -> tuple[int, list[list[ No checker actually runs; what is under test is the SHELL logic that decides which checkers get invoked, and with which range. + + ONE exception, and it is deliberate: `scripts/ci-walk-base.py` is executed + for real. It is not a checker -- it RESOLVES the base the step then passes to + the checkers -- so stubbing it out would make every case below read an empty + base and skip the very calls they exist to require (#1809). Running it means + these cases test the real composition of the resolver and the step shell + rather than a transcription of the resolver's rule. """ with tempfile.TemporaryDirectory(prefix="vllm-baseline-step-") as temporary: @@ -797,7 +804,10 @@ 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' + "case \"$1\" in\n" + " *ci-walk-base.py) exec \"$VLLM_BASELINE_PYTHON\" \"$@\" ;;\n" + "esac\n", encoding="utf-8", ) recorder.chmod(0o700) @@ -806,6 +816,7 @@ def run_shimmed(body: str, environment: dict[str, str]) -> tuple[int, list[list[ env = dict(os.environ) env["PATH"] = f"{shim}{os.pathsep}{env.get('PATH', '')}" env["VLLM_BASELINE_ARGV"] = str(trace) + env["VLLM_BASELINE_PYTHON"] = sys.executable env.update(environment) result = subprocess.run( ["bash", str(script)], @@ -1362,6 +1373,14 @@ def test_the_baseline_lane_invokes_no_range_scoped_checker_and_exits_zero(self) "a diff-scoped checker ran with no diff range", ) self.assertNotIn(f"..{self.FAKE_HEAD}", argv) + if any("ci-walk-base.py" in item for item in argv): + # The RESOLVER is handed every candidate, and on this + # lane every candidate is legitimately empty -- that + # is the input it exists to decide on, and its own + # suite pins what it returns. The rule below is about + # a CHECKER receiving an empty base, which is what + # would pass vacuously. + continue self.assertNotIn("", argv, "an empty argument means an empty base") def test_push_and_pull_request_still_get_the_full_range_scoped_checks(self) -> None: @@ -1545,13 +1564,47 @@ def test_the_diff_scoped_base_is_the_last_gated_commit(self) -> None: 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, - never to an empty range that passes vacuously.""" + never to an empty range that passes vacuously. + + The fallback moved out of this step's inline shell and into + `scripts/ci-walk-base.py` (#1809), so both halves are asserted: the step + still hands the resolver `github.event.before` alongside the last gated + commit, and the resolver EXECUTES the degradation. A string match on the + step alone would no longer see the rule at all, which is how a moved rule + becomes an unenforced one. + """ owner = self.owning_job("check-commit-trailers.py --range") walk = next( s for s in self.ci["jobs"][owner]["steps"] if "check-commit-trailers.py --range" in str(s.get("run", "")) ) - self.assertIn('base="$PUSH_BASE"', str(walk["run"])) + run = str(walk["run"]) + self.assertIn("scripts/ci-walk-base.py", run) + self.assertIn('--push-base "${PUSH_BASE:-}"', run) + self.assertIn('--last-green "${LAST_GREEN:-}"', run) + head = subprocess.check_output( + ["git", "-C", str(ROOT), "rev-parse", "HEAD"], text=True + ).strip() + before = subprocess.check_output( + ["git", "-C", str(ROOT), "rev-parse", "HEAD~1"], text=True + ).strip() + # `--floor ""` isolates the fallback. The floor's interaction with the + # base is the subject of tests/scripts/test_ci_walk_base.py; here the + # question is only what an empty LAST_GREEN degrades to. + resolved = subprocess.check_output( + [ + sys.executable, + str(ROOT / "scripts/ci-walk-base.py"), + "--event", "push", + "--head", head, + "--push-base", before, + "--last-green", "", + "--floor", "", + "--repo", str(ROOT), + ], + text=True, + ).strip() + self.assertEqual(resolved, before) def test_the_push_lane_is_latest_only(self) -> None: group = self.ci["concurrency"]["group"]