diff --git a/.agents/prompts/implementer.md b/.agents/prompts/implementer.md new file mode 100644 index 00000000..c254419e --- /dev/null +++ b/.agents/prompts/implementer.md @@ -0,0 +1,40 @@ +# Implementer prompt + +You implement one task. A different agent will review it by mutating your code. + +## Method + +1. Write the failing test first. Run it. Confirm it fails **for the stated + reason**: a test that fails for the wrong reason pins nothing. +2. Implement the minimum that makes it pass. +3. **Mutate every test you wrote**: delete the line it names, confirm red, + restore. Report the results. If a briefed test does not pin what it claims, + fix it and say so; four implementers before you did exactly that and were + right every time. Read that four as a **dated floor** (2026-08-06), not a + running total: it can only grow, and growing never weakens the rule. +4. Run the project gate (`scripts/agent-preflight.sh`, redirected to a file, + never piped) and confirm `EXIT=0`. When a gate is ALREADY red before you + touch anything, capture that failing set as a baseline FIRST: you are green + when the failing set after your change is identical to it. Name the carried + reds in your report. A gate you did not break is not yours to allowlist, and + reaching a green banner is never a reason to weaken one. +5. Commit in your worktree with the required trailers, and return the SHA. + +## Honesty rules + +- **Never let a failure and an absence look the same.** Every recorded defect + class in this repo is that bug: a substring `--grep` crediting a row with + another row's commits, `.get()` on a missing key reporting a live claim as + finished, a git failure mapped to `""` and read as "no evidence". +- **Report what you did not do.** An empty concerns section is itself a claim. +- **Escalate rather than guess.** Report `BLOCKED` or `NEEDS_CONTEXT` with + specifics. Bad work is worse than no work, and you will not be penalised for + stopping. +- **Never weaken a checker, a budget or a test to make something pass.** If the + gate is red, repair the record. + +## Deviating from the brief + +You may deviate when the brief is wrong, and it sometimes is. State the +deviation explicitly in your report with the evidence that justifies it. Silent +scope expansion is a defect; a disclosed, argued correction is not. diff --git a/.agents/prompts/reviewer.md b/.agents/prompts/reviewer.md new file mode 100644 index 00000000..92818db7 --- /dev/null +++ b/.agents/prompts/reviewer.md @@ -0,0 +1,54 @@ +# Reviewer prompt + +You review one change. You did not write it and you will not fix it. + +## The binding instruction: mutate, don't read + +For each test in the change, **delete or invert the line it names and re-run +the suite. A test that stays green is a finding**, regardless of how it reads. + +This is not a style preference. In the two branches audited to 2026-08, eleven +tests passed with the thing they named deleted, including a gate's own +default (an unrelated line satisfied the assertion), a probe with five +hardcoded fields, and `assertIn("merged", reason)` where the string was +`"unmerged"`. **None was visible by reading the diff.** A reviewer who reads +and comments on style adds nothing this project has not already paid for. + +## Do not trust the report + +Treat the implementer's report as unverified claims about the code. Verify each +against the change. A stated rationale ("kept it simple deliberately", "left +it per YAGNI") is the implementer grading its own work and **never** downgrades +a finding's severity. In that same audit, three implementer reports asserted +something false in good faith; each was caught by reproducing the claim rather +than accepting it. Read every count on this page as a dated floor, not a +running total: it can only grow, and growing never weakens the rule. + +## A plan-mandated finding is still a finding + +Roughly half of all Important findings on the preceding branches were defects in +the **plan text**, not the implementation. A reviewer that treats the plan as +authority cannot find them. Report them, labelled `plan-mandated`, and let the +human decide which governs. + +## Severity + +- **Critical**: corrupts the record, weakens a gate, or leaves a false claim in + a document agents read. +- **Important**: the change cannot be trusted until fixed: wrong or fragile + behavior, a missed requirement, a test that asserts nothing. +- **Minor**: polish. + +Cite `file:line` for every finding and for any check you would otherwise answer +with a bare "yes". Acknowledge what was done well before listing issues. + +## What you may not do + +- Never fix what you found. Findings go back to a fresh implementer. +- Never mutate the reviewed worktree, its index, HEAD or branch state. Work in a + scratch copy. +- Never re-run the full suite merely to reproduce the report's green result; + that confirms nothing the report already claims. This is NOT a budget on + mutation: every mutation you make re-runs the suite, and a review that made + none has not started. Reading may prompt an extra focused check, but reading + is never what decides whether to check. diff --git a/.agents/specs/gate-command-audit-2026-08-06.md b/.agents/specs/gate-command-audit-2026-08-06.md new file mode 100644 index 00000000..cdfa14b2 --- /dev/null +++ b/.agents/specs/gate-command-audit-2026-08-06.md @@ -0,0 +1,646 @@ +# Gate-command audit — 2026-08-06 + +**72 of 97 gated rows cannot name a command a machine can run and see fail.** + +**That is a statement about mechanical checkability, and nothing else.** It is +not a claim that those rows are unverified, unmeasured or ungated in the sense +this project uses the word. Many of them carry more evidence than the rows that +score `runnable` — assertion counts, exit statuses, mutation proofs, oracle +captures, ledger anchors. What they lack is a *command*: a literal string an +operator or a CI job could execute and read `$?` from. Read the finding as +"unverified work" and you would be slandering a large amount of landed, +carefully-gated engineering. See § What this does NOT mean, which is not a +footnote — it is the point. + +**"Many", not "most" — the debt is not homogeneous, and it was counted.** The +72 rows split by state: **18 `DONE`, 37 `ACTIVE`, 11 `READY`, 5 `BLOCKED`, +1 `GATING`**. So the bucket holds at least three different things, and only the +first is landed work: + +- **18 `DONE`** — finished work whose evidence is written as results rather than + as commands. This is where the "better evidenced than `runnable`" claim is + demonstrated (`SPEC-DFLASH-GGUF`, `KERNEL-EW-NORM-ACT`; § Method sample). +- **37 `ACTIVE`** — in flight, evidence partial by definition. +- **16 `READY` + `BLOCKED`** — **work not yet done, with no evidence to be more + or less than anything.** Their `Gates` sections are *prospective*: they state + the bar a future implementation must clear. `BACKEND-DISTRIBUTED-TP` ("a 2-GPU + box **is required** to gate TP") and `KERNEL-GDN-AOT-BF16` ("ported upstream + tests **pass** across boundary shapes") carry zero results, correctly, because + nothing has run. Of three such rows opened by hand, only `SPEC-EAGLE3` carried + MET evidence. + +A prospective gate with no command is a different debt from a landed gate whose +evidence is prose, and step 4 should not treat them alike: the first needs the +work done, the second needs a transcription. + +Recorded **before** anything enforces it, so the ratchet's baseline (step 4) is +a decision with reasoning attached rather than a number someone pasted. + +--- + +## Scope + +- **Subject:** every row at `READY`, `ACTIVE`, `GATING`, `DONE` or `BLOCKED` in + the **six lifecycle matrices**, as of `0a23f966` on `spec/orchestration-harness` + (`.agents/{engine,model,kernel,backend,quantization,feature}-matrix.md`). + 97 rows. `sglang-matrix.md` was listed as a seventh when this audit ran and + contributed **0 rows**; step 5 dropped it from `AUDITED_MATRIX_PATHS` with the + reasoning recorded — see risk 6, which is now **resolved**, not deferred. + Nothing in the counts below changes: 0 rows in, 0 rows out. +- **Instrument:** `scripts/check-gate-commands.py` (landed step 2, `0a23f966`). +- **What the classifier decides:** whether the row's spec has a `Gates` heading, + and whether the body under it contains at least one backticked or fenced span + that names an executable and is not `true` / `:` / `echo …` / piped. +- **What the classifier cannot decide:** whether that command is *the row's + gate*, whether it would pass, whether it is the right gate, or whether the row + is verified by some other means. It reads shape, not meaning. Every one of + those limits shows up concretely in § Risks and decisions. + +`DONE` rows are in scope deliberately. A row that quietly lost its gate command +is exactly the regression worth catching, and `DONE` rows are the ones nobody +looks at again. + +--- + +## Method + +`python3 scripts/check-gate-commands.py --json`. The classification rules, as +the script implements them: + +1. **Locate the section.** The body under the first `^#{1,6}\s*Gates\b` heading, + up to the next heading of any level. Prose elsewhere in the spec does not + count, however gate-like it reads. +2. **Extract candidates.** Every inline backticked span, plus every non-blank + line of every fenced block, inside that section. +3. **A candidate names an executable** if it matches one of: + - a known tool as a **whole word** — + `ctest|pytest|python3?|cmake|bash|sh|make|nsys|ncu|git|gh`. Both word + boundaries are load-bearing on the shipped record: without the trailing + one, `sha256_cbor` matches `sh` and `python@3.14` matches `python`. + - `flock ` — this repo's mandated shape for any + GPU-touching gate. It quotes the real command, putting it out of reach of + every other rule. It requires **both** a lockfile and something to run. + - an **invoked** path — `./anything`, or a `scripts/`/`tests/` path carrying + an executable suffix or arguments. A bare backticked filename is not a + command: `` `docs/BENCHMARKS.md` `` is a thing the gate talks about, not a + thing it runs. +4. **Reject what cannot fail.** `true`, `:`, `echo …` are recognised as + commands *deliberately* and then rejected for the reason that matters — they + cannot fail — rather than merely going unnoticed. Anything containing `|` is + rejected too: `cmd | tail` reports `tail`'s exit status. +5. **Verdict:** `no-spec` (no resolving `.agents/specs/` link) → `no-gates-section` + (no `Gates` heading) → `gates-no-command` (heading, no surviving candidate) → + `runnable`. + +### The hand-verified sample, and its outcome + +A classifier wrong on a sample is wrong on all 97, so six rows were opened and +judged by hand before any number here was trusted — three it called `runnable`, +three it called `gates-no-command`. + +| Row | Verdict | What the `Gates` section actually says | Holds? | +|---|---|---|---| +| `ENG-EXPERT-STREAM` | `runnable` | G1 names `` `flock /tmp/gpu -c './tests/parity/test_qwen36_expert_stream --resident-frac 0.5'` ``; G6 names `` `python3 scripts/check-agent-record.py` ``. Both real, both exit-status-bearing, both genuinely this row's gates. | **yes** | +| `QUANT-GGUF-COMPUTE` | `runnable` | `` `VLLM_CPP_CPU_THREADS=N ctest --test-dir build -L cpu` `` and `` `ctest -R gguf` ``. Real gates. (`N` is a metavariable needing substitution — a nit, not a misclassification.) | **yes** | +| `BACKEND-VULKAN` | `runnable` | The **only** credited command is `` `python3 -m venv ~/mlx-venv && ~/mlx-venv/bin/pip install -U pip mlx-lm` `` — an MLX install recipe, in a *Metal* subsection, for a *Vulkan* row. The section's real gating substance (token-exact vs our own CUDA backend on the same box; NMSE ≤ 5e-4, not `memcmp`) names **no** command. | **rule: yes. intent: no.** | +| `SPEC-DFLASH-GGUF` | `gates-no-command` | Seven gates, exhaustively evidenced: 302/302 assertions exit 0, 58/58 tensors byte-identical, a **mutation proof** (`kCrossQuantAcceptBand = 0` → 15/17, exit 1), a voided earlier reading honestly retracted. Commands: none. `tests/…/test_qwen3_dflash_gguf.cpp` is named as a *file*; `flock` appears bare. | **yes** | +| `MODEL-TEXT-laguna-…` | `gates-no-command` | Build flags (`-DVLLM_CPP_CUDA=OFF`, `-Werror`) and binary names with results (`test_laguna_scaffold` 8/8 · 166 assertions). No invocation. | **yes** | +| `KERNEL-EW-NORM-ACT` | `gates-no-command` | Four gates with 0-ulp bit-exactness, 140/140, 235/235 + 315/315, rollback arms, nsys per-shape timings. Names `test_ops_gdn`, `VT_RMSNORM_GATED_FAST=0`. No invocation. | **yes** | + +**Outcome: 6/6 verdicts hold as the stated rule defines them.** No mismatch, so +the audit proceeds. `BACKEND-VULKAN` is right-for-the-wrong-reason — the rule is +satisfied by a line that is not a gate — which is a known limitation of the rule, +recorded in § Risks and decisions, not a defect in its implementation. + +Note what rows 4–6 demonstrate, because it is the whole argument of this +document: the three `gates-no-command` rows sampled are **better evidenced than +the `runnable` one**. `SPEC-DFLASH-GGUF` carries a mutation proof. `BACKEND-VULKAN` +carries a `pip install`. + +--- + +## Findings + +``` + 25 runnable + 51 gates-no-command + 20 no-gates-section + 1 no-spec + +97 gated rows; 25 carry a command that can fail. +``` + +### Per matrix + +| Matrix | Gated | `runnable` | `gates-no-command` | `no-gates-section` | `no-spec` | +|---|---:|---:|---:|---:|---:| +| `engine-matrix.md` | 43 | 14 | 24 | 5 | 0 | +| `model-matrix.md` | 19 | 5 | 11 | 3 | 0 | +| `backend-matrix.md` | 13 | 3 | 9 | 1 | 0 | +| `kernel-matrix.md` | 10 | 1 | 5 | 4 | 0 | +| `quantization-matrix.md` | 8 | 2 | 0 | 6 | 0 | +| `feature-matrix.md` | 4 | 0 | 2 | 1 | 1 | +| `sglang-matrix.md` — now **excluded**, risk 6 | 0 | 0 | 0 | 0 | 0 | +| **total** | **97** | **25** | **51** | **20** | **1** | + +`quantization-matrix.md` is the outlier: 6 of its 8 gated rows have **no `Gates` +heading at all** and none are `gates-no-command`. Its specs record results in +prose sections under other names. + +`sglang-matrix.md` contributed **zero rows, and not because its rows are all +below `READY`** — see risk 6. It was audited in name only, and is no longer +listed as audited at all. + +### `gates-no-command` — 51 rows + +The spec named is `specs[0]`, the only one the classifier reads (see risk 2). + +| Row | State | Matrix:line | Spec | +|---|---|---|---| +| `KV-PREFIX-CACHE` | DONE | engine:57 | `prefix-prompt-caching-parity.md` | +| `ENG-RUNNER-MODELSHAPE` | ACTIVE | engine:71 | `first-additive-model-qwen3-dense.md` | +| `ENG-MM-INPUT-PIPELINE` | READY | engine:72 | `multimodal-track.md` | +| `ENG-MM-VISION-TOWER` | ACTIVE | engine:73 | `multimodal-track.md` | +| `ENG-MM-TEXT-BACKBONE` | ACTIVE | engine:74 | `multimodal-track.md` | +| `ENG-MM-QWEN36-VL-FORWARD` | ACTIVE | engine:75 | `multimodal-track.md` | +| `ENG-MM-VIDEO-FORWARD` | READY | engine:76 | `multimodal-track.md` | +| `ENG-MM-AUDIO-PIPELINE` | ACTIVE | engine:77 | `audio-track.md` | +| `ENG-MM-AUDIO-ENCODER` | READY | engine:78 | `audio-track.md` | +| `ENG-MM-AUDIO-E2E` | ACTIVE | engine:79 | `audio-track.md` | +| `KV-EVENTS` | ACTIVE | engine:105 | `kv-events.md` | +| `SAMPLE-LOGPROBS` | DONE | engine:131 | `sampling-controls-c7.md` | +| `SAMPLE-BEAM` | ACTIVE | engine:136 | `sampling-controls-c7.md` | +| `SAMPLE-N` | ACTIVE | engine:142 | `sampling-controls-c7.md` | +| `SAMPLE-BEST-OF` | ACTIVE | engine:143 | `sampling-controls-c7.md` | +| `TOOLS-XGRAMMAR` | ACTIVE | engine:150 | `xgrammar-backend.md` | +| `SPEC-MTP-GGUF` | DONE | engine:162 | `gguf-mtp-spec-decode.md` | +| `SPEC-DFLASH-GGUF` | DONE | engine:163 | `gguf-dflash-draft.md` | +| `SPEC-NGRAM` | ACTIVE | engine:169 | `spec-decode-breadth-d3.md` | +| `SPEC-EAGLE3` | BLOCKED | engine:170 | `spec-decode-breadth-d3.md` | +| `SPEC-DRAFT-MODEL` | ACTIVE | engine:180 | `draft-model-medusa-spec.md` | +| `ENG-POOLER-SEQ` | ACTIVE | engine:212 | `pooling-task-class.md` | +| `ENG-POOLING-RUNNER` | ACTIVE | engine:213 | `pooling-task-class.md` | +| `ENG-MOE-HOSTFREE` | DONE | engine:247 | `moe-marlin-host-free.md` | +| `MODEL-TEXT-commandr-cohere-for-causal-lm` | BLOCKED | model:172 | `sweep-recent-dense-batch.md` | +| `MODEL-TEXT-deepseek-v2-deepseek-v2-for-causal-lm` | ACTIVE | model:178 | `mla-deepseek-campaign.md` | +| `MODEL-TEXT-deepseek-v2-deepseek-v3-for-causal-lm` | BLOCKED | model:179 | `mla-deepseek-campaign.md` | +| `MODEL-TEXT-deepseek-v4-deepseek-v4-for-causal-lm` | ACTIVE | model:180 | `deepseek-v4-flash.md` | +| `MODEL-TEXT-kimi-linear-kimi-linear-for-causal-lm` | ACTIVE | model:223 | `kimi-linear.md` | +| `MODEL-TEXT-laguna-laguna-for-causal-lm` | ACTIVE | model:226 | `laguna-s21-w3-2026-07-31.md` | +| `MODEL-TEXT-minimax-m2-mini-max-m2-for-causal-lm` | BLOCKED | model:233 | `mla-deepseek-campaign.md` | +| `MODEL-TEXT-qwen3-qwen3-for-causal-lm` | ACTIVE | model:263 | `first-additive-model-qwen3-dense.md` | +| `MODEL-MM-gemma4-mm-gemma4-for-conditional-generation` | READY | model:384 | `gemma4-multimodal.md` | +| `MODEL-MM-voxtral-voxtral-for-conditional-generation` | READY | model:463 | `audio-track.md` | +| `MODEL-SPEC-deepseek-v4-deep-seek-v4-mtp` | ACTIVE | model:492 | `deepseek-v4-mtp.md` | +| `KERNEL-ACCEL-PROVIDER-SELECT` | ACTIVE | kernel:115 | `metal-mlx-reuse-study.md` | +| `KERNEL-EW-NORM-ACT` | DONE | kernel:129 | `rmsnorm-gated-fast-2026-07-17.md` | +| `KERNEL-GDN-PACKED-DECODE` | DONE | kernel:153 | `gdn-packed-decode.md` | +| `KERNEL-GDN-AOT-BF16` | READY | kernel:154 | `kernel-family-inventory.md` | +| `KERNEL-GDN-SCRATCH` | READY | kernel:155 | `kernel-family-inventory.md` | +| `BACKEND-CUDA-SM087` | ACTIVE | backend:170 | `cuda-architecture-inventory.md` | +| `BACKEND-CUDA-SM110` | ACTIVE | backend:176 | `cuda-architecture-inventory.md` | +| `BACKEND-CUDA-SM120` | ACTIVE | backend:177 | `cuda-architecture-inventory.md` | +| `BACKEND-ACCEL-PROVIDER` | ACTIVE | backend:231 | `metal-mlx-reuse-study.md` | +| `BACKEND-BENCH-CUDA-SGLANG-PREFLIGHT` | GATING | backend:245 | `cuda-sglang-low-concurrency.md` | +| `BACKEND-GATE-CUDA-SGLANG` | BLOCKED | backend:246 | `cuda-sglang-low-concurrency.md` | +| `BACKEND-GATE-CUDA-SGLANG-PREFIX` | READY | backend:247 | `cuda-sglang-low-concurrency.md` | +| `BACKEND-DISTRIBUTED-COMM` | ACTIVE | backend:272 | `scale-out-distributed.md` | +| `BACKEND-DISTRIBUTED-TP` | READY | backend:273 | `scale-out-distributed.md` | +| `QUANT-CUDA-GATES` | DONE | feature:166 | `quantization-coverage.md` | +| `BACKEND-CUDA-OTHER` | ACTIVE | feature:276 | `cuda-architecture-inventory.md` | + +### `no-gates-section` — 20 rows + +These specs have no `Gates` heading. Several record their gating under other +headings; the classifier does not look, by design (rule 1 — prose does not +count, or the section boundary means nothing). + +| Row | State | Matrix:line | Spec | +|---|---|---|---| +| `PAR-TP` | READY | engine:118 | `tensor-parallelism.md` | +| `SPEC-MTP` | DONE | engine:161 | `mtp-spec-decode.md` | +| `SPEC-REJECTION` | ACTIVE | engine:164 | `mtp-spec-decode.md` | +| `SPEC-GDN-SEGMENTS` | ACTIVE | engine:165 | `mtp-spec-decode.md` | +| `SPEC-DFLASH` | DONE | engine:166 | `dflash-spec-decode.md` | +| `MODEL-SPEC-qwen3-dflash-dflash-qwen3-for-causal-lm` | DONE | model:481 | `dflash-spec-decode.md` | +| `MODEL-SPEC-qwen3-5-mtp-qwen3-5-mtp` | DONE | model:508 | `mtp-spec-decode.md` | +| `MODEL-SPEC-qwen3-5-mtp-qwen3-5-moe-mtp` | DONE | model:509 | `mtp-spec-decode.md` | +| `QUANT-GGUF-Q2_K` | ACTIVE | quant:64 | `cuda-keepquant-gemm.md` | +| `QUANT-GGUF-IQ2_XXS` | ACTIVE | quant:69 | `cuda-keepquant-gemm.md` | +| `QUANT-GGUF-IQ3_XXS` | READY | quant:71 | `cuda-keepquant-gemm.md` | +| `QUANT-NVFP4-MO-W4A16` | DONE | quant:121 | `marlin-dropin-feasibility.md` | +| `QUANT-NVFP4-CT-W4A4` | DONE | quant:123 | `qwen27b-w4a4-notes.md` | +| `QUANT-FP8-MO-STATIC` | DONE | quant:124 | `qwen36-forward-notes.md` | +| `KERNEL-QUANT-CIQ-GEMM-CUDA` | ACTIVE | kernel:128 | `cuda-keepquant-gemm.md` | +| `KERNEL-ATTN-DFLASH-BLOCK` | DONE | kernel:140 | `dflash-spec-decode.md` | +| `KERNEL-ATTN-DFLASH-PAGED-BLOCK` | DONE | kernel:141 | `dflash-spec-decode.md` | +| `KERNEL-ATTN-DENSE-FLASH` | ACTIVE | kernel:148 | `multimodal-speed.md` | +| `BACKEND-GATE-METAL-MLXLM` | ACTIVE | backend:254 | `competitive-benchmarks.md` | +| `MODEL-SPEC` | ACTIVE | feature:154 | `mtp-spec-decode.md` | + +### `no-spec` — 1 row + +| Row | State | Matrix:line | Why | +|---|---|---|---| +| `BACKEND-MLX` | ACTIVE | feature:279 | Its only link is to `backend-matrix.md`, an area matrix, not a `.agents/specs/` file. | + +### `runnable` — 25 rows + +| Row | State | Matrix:line | First credited command | +|---|---|---|---| +| `ENG-ASYNC-SCHED` | DONE | engine:62 | `flock /tmp/gpu -c 'ctest -R qwen36_paged_engine'` | +| `ENG-PRIORITY-SCHED` | GATING | engine:63 | `flock /tmp/gpu -c 'ctest -R qwen36_paged_engine'` | +| `ENG-CORE-BUSY-LOOP` | GATING | engine:66 | `flock /tmp/gpu -c 'ctest -R qwen36_paged_engine'` | +| `KV-SLIDING-LOCAL-SPECS` | READY | engine:97 | `cmake -S . -B build-c5-cpu … && cmake --build …` | +| `KV-SLIDING-WINDOW-SPEC` | GATING | engine:98 | `cmake -S . -B build-c5-cpu … && cmake --build …` | +| `KV-CHUNKED-LOCAL-SPEC` | GATING | engine:99 | `cmake -S . -B build-c5-cpu … && cmake --build …` | +| `ENG-EXPERT-STREAM` | READY | engine:110 | `flock /tmp/gpu -c './tests/parity/test_qwen36_expert_stream --resident-frac 0.5'` | +| `TOOLS-STREAMING-PARSER` | ACTIVE | engine:154 | `git diff --stat` **(weak — see risk 3)** | +| `SERVE-STREAM-USAGE` | GATING | engine:200 | `git diff --check` **(weak — see risk 3)** | +| `SERVE-ASYNC-LLM` | GATING | engine:203 | `flock /tmp/gpu -c 'ctest -R qwen36_paged_engine'` | +| `SERVE-HTTP-TRANSPORT` | DONE | engine:204 | `scripts/check-agent-record.py` | +| `ATTN-ROPE-FAMILY` | READY | engine:231 | `cmake -S . -B build-c5-cpu … && cmake --build …` | +| `ATTN-CHUNKED-LOCAL` | GATING | engine:236 | `cmake -S . -B build-c5-cpu … && cmake --build …` | +| `LOAD-SAFETENSORS-DIRECT-DENSE` | GATING | engine:246 | `scripts/check-agent-record.py` (9 commands total) | +| `MODEL-FACTORY-registry` | GATING | model:156 | `python3 scripts/check-agent-record.py` | +| `MODEL-TEXT-gemma4-gemma4-for-causal-lm` | BLOCKED | model:196 | `scripts/check-agent-record.py` (5 checkers) | +| `MODEL-TEXT-glm4-glm4-for-causal-lm` | READY | model:199 | `scripts/check-agent-record.py` | +| `MODEL-TEXT-glm4-moe-lite-…` | ACTIVE | model:201 | `scripts/check-agent-record.py` | +| `MODEL-TEXT-deepseek-v2-glm-moe-dsa-…` | BLOCKED | model:202 | `scripts/check-agent-record.py` | +| `QUANT-GGUF-COMPUTE` | READY | quant:33 | `VLLM_CPP_CPU_THREADS=N ctest --test-dir build -L cpu` | +| `QUANT-NVFP4-CT-W4A16` | ACTIVE | quant:122 | `scripts/qwen3-32b-nvfp4a16-oracle-capture.py --runs 5` | +| `KERNEL-GEMM-CPU-ELEM` | ACTIVE | kernel:126 | `ctest -j2` **(weak — see risk 3; lifted from prose about a FLAKE)** | +| `BACKEND-CUDA-ARCH-ADDITIVITY` | ACTIVE | backend:187 | `scripts/check-agent-record.py` (incl. `cuobjdump -lelf libvllm.a`) | +| `BACKEND-METAL-MLX` | ACTIVE | backend:232 | `python3 -m venv ~/mlx-venv && … pip install … mlx-lm` **(env setup — risk 3)** | +| `BACKEND-VULKAN` | ACTIVE | backend:233 | `python3 -m venv ~/mlx-venv && … pip install … mlx-lm` **(env setup — risk 3)** | + +--- + +## What this does NOT mean + +**A row without a runnable gate command is not ungated work.** The classifier +answers one narrow question — *can a machine re-check this without a human +reading prose?* — and the answer being "no" says nothing about whether the work +was done, measured, or verified. + +The sample proves it directly. `SPEC-DFLASH-GGUF` is `gates-no-command` and its +`Gates` section contains: 302/302 assertions at exit 0; 58 of 58 tensors proven +byte-identical across two loaders; a demonstration that the pass is **not +vacuous** (pointed at `Q4_K_M`, the same case goes 21/302 red, exit 1); a +mutation proof that a tolerance band is load-bearing (`kCrossQuantAcceptBand = 0` +→ 15/17, exit 1, on exactly the two banded assertions); and an earlier "MET" +reading explicitly **voided** because the build lacked `-DVLLM_CPP_CUTLASS_DIR`. +That is a higher standard of evidence than most `runnable` rows meet. +`KERNEL-EW-NORM-ACT` is likewise `gates-no-command` and carries 0-ulp +bit-exactness over 140 assertions plus 235/235 and 315/315 engine token gates +with rollback arms. + +Meanwhile `BACKEND-VULKAN` scores `runnable` on a `pip install`. + +So the ordering the counts imply is not a quality ordering. What separates the +buckets is **notation**: whether the evidence was written as an invocation or as +a result. Much of this repo's record is written as results — "235/235, +exit 0" — which is *more* informative to a human reader and *useless* to a +machine that wants to re-run it. + +Three further reasons a well-gated row lands outside `runnable`: + +- **The evidence lives elsewhere.** `.agents/parity-ledger.md` holds the binding + benchmark rows with their boxes, recipes and reps. The classifier never opens + it. +- **The gate is a test anchor, not a command.** A spec naming + `test_qwen36_paged_engine` and its assertion count is pointing at a real, + runnable, CI-covered binary. It just isn't spelled as a command line. +- **The gate genuinely cannot be run yet** — HW-blocked (`BACKEND-*-XPU`), + oracle-blocked, or needing two Sparks. Recording that plainly is the design's + stated intent (§ Work breakdown item 5: "record honestly that they cannot be + gated yet"). + +The actionable reading is therefore: **72 rows cannot be driven through the +operator loop as written.** Not: 72 rows are unverified. + +What that costs differs by state, per the split in the lede. For the 18 `DONE` +and much of the 37 `ACTIVE`, it is a **transcription** — the evidence exists and +needs writing as an invocation. For the 16 `READY`/`BLOCKED` rows there is no +evidence to transcribe and none should be expected; their gates are prospective, +and they become runnable when the work is done, not before. Step 4 should not +report those 16 as a debt someone forgot to pay. + +--- + +## The ratchet baseline + +**The 25 named rows below — as a SET, pinned exactly.** + +> **Superseding note (step 5).** An earlier draft of this section said the +> baseline was "**25**", "a **floor on a count**", and that the rule was +> **shrink-only**. Both descriptions are wrong about what shipped, and the next +> subsection (§ The baseline's principal false-red mode) already argued why a +> count cannot work. What `check-gate-commands.py` pins is the SET of row IDs, +> and it is pinned by **exact equality**, not as a floor. This section is the +> corrected text; the count-and-floor wording survives nowhere. + +Twenty-five rows scored `runnable` at `0a23f966`. Step 4 pins **their IDs**, in +`RUNNABLE_BASELINE`, and the contract is an **exact pin**: + +- `--check` refuses a row that **lost** its command, and distinguishes that from + a row that legitimately **left** the gated population (next subsection); +- `tests/scripts/test_check_gate_commands.py` additionally asserts the shipped + `runnable` set **equals** `RUNNABLE_BASELINE`, which is what makes "just lower + the number" impossible to do quietly — and which means **growth is red too**. + +So adding a real gate command to a row's spec — textbook improvement — leaves +`--check` at 0 while the suite, preflight and CI go red until the set is +re-pinned. **That is the intended cost, and it is stronger than shrink-only.** +Any movement, up or down, re-pins `RUNNABLE_BASELINE` in the SAME change, naming +the rows that moved and why. Growth is welcome, ordinary work; **silent** growth +is what the pin forbids. + +The pin is not an assertion that these 25 rows are correctly gated. **Five of +them are not** (risk 3). They are pinned anyway, because a ratchet that waits for +a clean baseline never starts, and because the alternative — relaxing the rule +until everything passes — is the failure mode this whole subsystem exists to +prevent. A relaxed gate is worse than no gate. + +Raising the set is ordinary work: transcribe a row's existing evidence into an +invocation, add the row ID here and to `RUNNABLE_BASELINE` in the same change. +That is § Work breakdown item 5, and it is now measured rather than estimated. + +### The baseline's principal false-red mode: the population moves + +**Twenty-five is a headcount over a population that is not fixed, which is the +first reason step 4 pins IDs rather than a number.** Step 2 already observed +the record move — three rows shifted during its own work — which is exactly why +the *total* (97) was deliberately not pinned. The `runnable` population inherits +the same exposure, and a rule written over a bare number reads a legitimate +record edit as a regression. + +A `runnable` row can leave the population without anything being broken: + +- it is **deleted** (a row retired, or folded into another); +- it is **merged** with another row; +- it **transitions out of `GATED_STATES`** — e.g. `READY` → `INVENTORIED` on a + descope, or any move to a state below `READY`; +- its **matrix** leaves `AUDITED_MATRIX_PATHS` (risk 6, done in step 5 for + `sglang-matrix.md` — which cost nothing here only because that matrix + contributed no `runnable` rows to begin with). + +Each drops the headcount below 25 and would turn a number-based gate red on a +correct edit. Risk 4 covers gaming the count *upward*; this is the opposite +failure and is the more likely one, because record edits are routine and gaming +is not. + +**The distinguishing rule step 4 should implement:** + +> A drop below the baseline is a **regression** only if the row that lost its +> `runnable` verdict **still exists and is still gated**. If the population +> itself shrank — the row was deleted, merged, or moved out of `GATED_STATES`, +> or its matrix left the audited set — the baseline is **re-pinned in the same +> change**, with the row ID and the reason recorded in the commit body. + +Re-pinning is a disclosed decision, not an escape hatch: it names which row left +and why, so a reviewer can check the claim. The rule that must never be applied +is lowering the baseline because the number "went down" without saying which row +moved — that is the relaxation this subsystem exists to prevent. + +**Consequence for the checker's output:** step 4 cannot enforce this on a bare +count — this is what supersedes the count-and-floor wording flagged above. It +needs the **set** of `runnable` row IDs, not just `len()`, so a drop +can be attributed to a named row and classified as "lost its command" versus +"left the population". Pinning an integer alone makes the two indistinguishable +— the repo's recorded defect class, one more time. + +--- + +## Risks and decisions + +### 1. The vocabulary misses real gate shapes — but the measured exposure inverts the expectation + +Step 2's review flagged seven shapes the tool vocabulary does not know: +`compute-sanitizer`, `cuobjdump`, `/usr/bin/time -v`, `curl`, `brew install`, +`nvidia-smi`, and a built binary invoked without `./`. The concern was **false +reds** once ratcheting. + +**Measured, this exposure is currently zero.** Every argument-bearing occurrence +of those seven shapes inside a gated row's `Gates` section lives in a row that +is *already* `runnable` via some other command: + +All seven shapes, each measured against the rows currently NOT `runnable`: + +| # | Shape | Argument-bearing occurrence | Rows it would flip | +|---|---|---|---| +| 1 | `compute-sanitizer` | `compute-sanitizer memcheck` — `sweep-qwen3-32b-nvfp4a16.md`, already `runnable` | **0** | +| 2 | `cuobjdump` | `cuobjdump -lelf libvllm.a` — `cuda-arch-additivity.md`, already `runnable` | **0** | +| 3 | `/usr/bin/time -v` | none anywhere in a gated row's `Gates` section | **0** | +| 4 | `curl` | `curl -N` — `async-serving.md` (4 rows), all already `runnable` | **0** | +| 5 | `brew install` | `brew install mlx` / `brew info mlx` — `backend-fanout-metal-vulkan-xpu.md`, already `runnable` | **0** | +| 6 | `nvidia-smi` | bare only, in `expert-streaming.md`, already `runnable` | **0** | +| 7 | built binary without `./` | no `test_*` name carries arguments anywhere | **0** | + +(`vllm-bench --num-prompts 1 …` in `gguf-cpu-threadpool.md` is not among the +seven but was measured alongside them; that row is already `runnable` too.) + +So widening the vocabulary to these seven would change **no row's verdict +today**. The risk is prospective — a *future* gate written with one of these +tools as its only command would be a false red — not a present miscount. + +**The larger risk points the other way, and it is measured too.** Adding a bare +tool name credits *prose that merely mentions the tool*, and the exposure is +several times the false-red one: + +| Naive entry | Falsely credits | +|---|---| +| bare **binary name** (shape 7, e.g. `test_*`) | **15 non-`runnable` rows** | +| bare `compute-sanitizer` (shape 1) | **6 non-`runnable` rows** | + +Shape 7 is the worst of the seven and the easiest to get wrong, because "a built +binary invoked without `./`" reads like an instruction to credit the bare name. +Fifteen rows name a `test_*` binary with its assertion counts and no invocation +— `KERNEL-EW-NORM-ACT`, `MODEL-TEXT-laguna-…`, `ENG-MOE-HOSTFREE`, +`ENG-POOLER-SEQ`, `ENG-POOLING-RUNNER`, `ENG-RUNNER-MODELSHAPE`, `KV-EVENTS`, +`SPEC-NGRAM`, `SPEC-EAGLE3`, `SPEC-DRAFT-MODEL`, `TOOLS-XGRAMMAR`, +`MODEL-TEXT-qwen3-…`, `MODEL-TEXT-kimi-linear-…`, `MODEL-TEXT-deepseek-v4-…`, +`MODEL-SPEC-deepseek-v4-deep-seek-v4-mtp`. Requiring an argument +(`test_\w+\s+\S`) credits **0** of them. + +`compute-sanitizer` appears as a bare backticked word in the `Gates` sections of +**six** rows currently `gates-no-command` (`ENG-MM-INPUT-PIPELINE`, +`ENG-MM-VISION-TOWER`, `ENG-MM-TEXT-BACKBONE`, `ENG-MM-QWEN36-VL-FORWARD`, +`ENG-MM-VIDEO-FORWARD`, `MODEL-TEXT-commandr-cohere-for-causal-lm`). + +The two sets are **disjoint** (verified), so naively adding both would credit +**21 distinct rows on nothing**, taking the ratchet baseline from **25 to 46** — +while every one of those rows stayed exactly as ungatable as it is today. + +**This is the `flock` bug, exactly.** Step 2 added `flock` as a bare vocabulary +entry, credited five rows whose specs name the *lock idiom* rather than a gate, +and produced a count that matched an earlier prediction — which felt like +corroboration and was a bug. The shipped `_WRAPPER` rule requires +`flock ` for that reason. + +**Decision for step 4:** widen only with argument-requiring patterns +(`compute-sanitizer\s+\S`, not `compute-sanitizer`), and re-run this audit +before and after so any baseline movement is attributed to a named row rather +than absorbed into a total. Do not widen as part of landing the ratchet; widen +in its own change, where the count delta is legible. + +### 2. `classify_row` reads only `specs[0]` — this changes 12 verdicts, measured + +`classify_row` takes the **first** resolving `.agents/specs/` link and ignores +the rest. 25 of the 97 gated rows link two or more existing specs. + +Scanning **all** linked specs changes **12 verdicts**, in two groups. Six move +`gates-no-command` → `runnable`, which is the group that moves the baseline: + +| Row | `specs[0]` verdict | Later spec | Command there | +|---|---|---|---| +| `MODEL-MM-gemma4-mm-gemma4-for-conditional-generation` | `gates-no-command` | `sweep-gemma.md` | `scripts/check-agent-record.py` | +| `KERNEL-ACCEL-PROVIDER-SELECT` | `gates-no-command` | `accelerator-seam-audit.md` | `scripts/check-agent-record.py` | +| `BACKEND-CUDA-SM087` | `gates-no-command` | `cuda-arch-additivity.md` | `scripts/check-agent-record.py` | +| `BACKEND-CUDA-SM110` | `gates-no-command` | `cuda-arch-additivity.md` | `scripts/check-agent-record.py` | +| `BACKEND-CUDA-SM120` | `gates-no-command` | `cuda-arch-additivity.md` | `scripts/check-agent-record.py` | +| `BACKEND-ACCEL-PROVIDER` | `gates-no-command` | `dropin-kernel-abi.md` | `python3 scripts/check-agent-record.py` | + +The other six move `no-gates-section` → `gates-no-command` — a later spec has a +`Gates` heading where `specs[0]` has none. These do **not** touch the baseline, +but they do move the published bucket totals: `QUANT-GGUF-Q2_K`, +`QUANT-GGUF-IQ2_XXS`, `QUANT-GGUF-IQ3_XXS`, `QUANT-NVFP4-CT-W4A4`, +`KERNEL-QUANT-CIQ-GEMM-CUDA`, `BACKEND-GATE-METAL-MLXLM`. + +**So the honest all-specs classification is:** + +| | shipped (`specs[0]`) | all-specs | +|---|---:|---:| +| `runnable` | 25 | **31** | +| `gates-no-command` | 51 | 51 | +| `no-gates-section` | 20 | **14** | +| `no-spec` | 1 | 1 | + +(`gates-no-command` holds at 51 because it loses six upward and gains six from +`no-gates-section` — a coincidence of equal counts, not a fixed point.) + +The baseline pinned above is 25 because that is what the shipped classifier +computes, and a baseline must be reproducible by running the tool. Recorded here +so that when `classify_row` is fixed to scan all specs, the jump from 25 to 31 — +and the § Findings `no-gates-section` total dropping 20 → 14 — is **understood as +the fix landing** and not mistaken for rows being gated or degated. Note this +fix is **not** free under the shipped pin, and an earlier draft here said it was: +the baseline is an EXACT pin, so taking `runnable` from 25 to 31 turns the suite +red until the six new IDs are added to `RUNNABLE_BASELINE` in the same change. +That is the pin working — the six rows are named right here, so the re-pin is a +transcription with its reasoning already written. + +### 3. Five of the 25 `runnable` credits are not gates + +They satisfy the stated rule. The ratchet should not lock them in silently. + +- **`BACKEND-VULKAN`** and **`BACKEND-METAL-MLX`** — both credited to the same + MLX `pip install` line, which is environment setup, in a *Metal* subsection. + For `BACKEND-VULKAN` it is not even the right backend. Each row's single + credited command is this line; remove it and both become `gates-no-command`. +- **`TOOLS-STREAMING-PARSER`** — credited solely to `git diff --stat`, which + **exits 0 in every state this gate could encounter**: measured 0 on a clean + tree, 0 on a dirty tree, and 0 on a bogus pathspec. (It returns 129 outside a + git repo — immaterial, since a gate runs in the checkout.) This is a + `_CANNOT_FAIL` shape the rule does not recognise: `true`, `:` and `echo` are + blocked by name, but a real command with no reachable failure mode passes. + Arguably the worst credit in the set, and it was not among the two the step-2 + review named. +- **`SERVE-STREAM-USAGE`** — credited solely to `git diff --check`. This *can* + fail (whitespace errors), so it is not vacuous, but it is not this row's gate. +- **`KERNEL-GEMM-CPU-ELEM`** — credited solely to a bare `` `ctest -j2` `` + extracted from prose reporting a **flake**: the spec says a test "flaked once + under `ctest -j2` on the co-tenanted dev box". That is an incident report, not + an invocation this row is gated by — no `-R`, no test-dir, no assertion of a + result. The classifier reads shape, and the shape of a flake note is the shape + of a command. Found in step 5's whole-branch review; it is the fifth weak + credit and was missed by the count of four above. + +A related weakness, not counted above: three GLM rows +(`MODEL-TEXT-glm4-*`, `MODEL-TEXT-deepseek-v2-glm-moe-dsa-*`) are credited +`scripts/check-doc-checkpoint.py --staged`. The harness spec +(`orchestration-harness.md` § Gate-command discipline, rule 3) names `--staged` +as the exact anti-pattern that let **eleven** commits on the P0 branch be red +while every preflight was green. Those rows also carry +`scripts/check-agent-record.py`, so they are not credited on the weak command +alone. + +**Decision:** record, do not fix. Fixing means editing matrices and specs, which +is out of scope for this task by construction, and each is a judgement about +what a row's real gate should be — an operator call, not a classifier change. + +### 4. What this gate stops being able to see once it has run + +The harness spec asks this of any gate it introduces. For +`check-gate-commands.py` the answer is: **it cannot distinguish a row that +acquired a real gate from a row that acquired a plausible-looking string.** +Once ratcheting, the cheapest way to raise the count is to paste +`scripts/check-agent-record.py` — already among the credited commands of 9 of +the 25 — into a `Gates` section. That would pass, and it would gate nothing about the row. +The classifier reads shape, so shape is what it can be satisfied with. +No mitigation is proposed here beyond naming it; it is a review responsibility, +not a checkable one. + +### 5. Rows the classifier could not decide, and the human call + +- **`BACKEND-MLX` (`no-spec`)** — links only `backend-matrix.md`. **Call: not a + defect.** It is a `feature-matrix.md` index row summarising a backend whose + real gating lives in `backend-matrix.md`'s `BACKEND-METAL-*` rows, which are + audited here in their own right. Counted as `no-spec`, excluded from the + actionable debt. +- **`quantization-matrix.md`'s 6 `no-gates-section` rows** — these specs record + results under other headings. **Call: real debt, low priority.** Adding a + `Gates` heading to a spec that already states its results is a transcription + job, and it is the same job the other 51 need. + +### 6. `sglang-matrix.md` is audited in name only — an absence that looks like a pass + +Step 2 widened `AUDITED_MATRIX_PATHS` beyond `check-agent-record.py`'s +`MATRIX_PATHS` to cover all seven matrices, `sglang-matrix.md` among them. It +contributes **0 rows** to this audit. + +The first reading — "all its rows are below `READY`" — is **wrong**, and was +checked rather than assumed. `record.parse_claim_rows()` returns **zero rows and +zero parse errors** for that file, out of 87 table rows. The cause is in the +matrix's own header: it carries "a **classification** in place of a lifecycle +state" — `FUSED` / `ACTIVE` / `INVENTORIED` / `NOT-APPLICABLE` — so it has no +state column of the shape the row parser recognises, and the parser reports +nothing rather than failing. + +**This is the repo's recorded defect class**: a failure and an absence looking +identical. A file listed as audited, returning no errors, contributing nothing. +Anyone reading `AUDITED_MATRIX_PATHS` would reasonably conclude SGLang rows were +examined and found clean. + +**Resolved in step 5: dropped, with the reasoning recorded.** `sglang-matrix.md` +is no longer in `AUDITED_MATRIX_PATHS`; `scripts/check-gate-commands.py` carries +the reason at the constant, and `test_sglang_is_excluded_and_the_exclusion_is_justified` +pins the justification rather than the bare absence — it asserts that +`parse_claim_rows` really does find zero rows and zero errors there, so the +matrix goes back into the audited set the moment it gains lifecycle rows. The +`runnable` set is unchanged at 25, because 0 rows left. The original call +follows, unchanged. + +**Original call (step 3): record, do not fix here.** Whether `SGLANG-*` rows *should* carry gate +commands is a real question — they are classification rows about a competitor's +surface, and most are `FUSED`, i.e. claims about our existing implementation +whose gates live on the rows they map to. But that argument must be made +explicitly, not arrived at by a silent zero. Step 4 should either drop +`sglang-matrix.md` from the audited set with that reasoning recorded, or teach +the parser its schema. **It must not leave it listed and empty.** — Step 4 did +neither and step 5 took the first branch, above: dropping is the cheaper option +and is defensible, because a classification column (`FUSED` / `INVENTORIED` / +`NOT-APPLICABLE`) is not a lifecycle state and `FUSED` rows' gates live on the +rows they map to, which are audited in their own right. + +--- + +## Provenance + +- Classifier: `scripts/check-gate-commands.py` @ `0a23f966`. +- Rows: the six lifecycle matrices @ `0a23f966` (`sglang-matrix.md` was listed + as a seventh and contributed 0 rows; dropped in step 5, risk 6). +- Reproduce: `python3 scripts/check-gate-commands.py` (summary) or + `--json` (per-row). +- Related: [orchestration-harness.md](orchestration-harness.md) § Gate-command + discipline and § Work breakdown item 5. diff --git a/.agents/specs/operator-helper-protocol.md b/.agents/specs/operator-helper-protocol.md index 28939527..7f179a85 100644 --- a/.agents/specs/operator-helper-protocol.md +++ b/.agents/specs/operator-helper-protocol.md @@ -120,6 +120,18 @@ PR is the claim. 2. **Features only via sub-agents.** The operator may drive feature work, but only by dispatching sub-agents or a dynamic workflow — never by writing the feature itself. Enforced mechanically: see § Enforcement. + The loop itself lives in the manual agents actually read — + [workflow.md § Running a row through sub-agents](../workflow.md#running-a-row-through-sub-agents), + gated by `scripts/check-protocol-consistency.py` — and the two sub-agent + contracts are tracked artifacts: [implementer](../prompts/implementer.md) + and [reviewer](../prompts/reviewer.md). Serially per task: dispatch a fresh + implementer, **run the row's gate yourself** (a "done" that is only the + author's opinion of its own work gives the loop no floor), then dispatch an + INDEPENDENT reviewer whose binding instruction is to **mutate, not read** — + delete the line each test names and re-run, because a test that stays green + is a finding and none of the seventeen found so far was visible in a diff. + **Never fix findings yourself**: a controller fix pollutes the context that + exists to coordinate, and skips review entirely. 3. **Directly permitted, because review needs it:** reading any diff, running any gate, resolving merge conflicts, fixing doc obligations, retuning a ratchet, and running benchmarks. An operator that cannot touch anything diff --git a/.agents/specs/orchestration-harness.md b/.agents/specs/orchestration-harness.md index 89d07641..effb025a 100644 --- a/.agents/specs/orchestration-harness.md +++ b/.agents/specs/orchestration-harness.md @@ -147,6 +147,20 @@ Three rules, each of which this project has already been bitten by: runs `--staged` in preflight, which passes vacuously once work is committed. Eleven commits on the P0 branch were red while every preflight was green. +`scripts/check-gate-commands.py` enforces rules 1 and 2 over the rows that +already carry a gate command, and +[gate-command-audit-2026-08-06.md](gate-command-audit-2026-08-06.md) is the +artifact behind it — **read it before touching that gate.** It records what the +25 pinned rows are, that the pin is EXACT (growth reds the suite too, so any +movement re-pins `RUNNABLE_BASELINE` in the same change), that **72 of 97 gated +rows** cannot name a runnable command today and why that is *not* a claim they +are unverified, and six named risks: the vocabulary's measured false-credit +exposure (a naive widening would credit 21 rows on nothing), `classify_row` +reading only `specs[0]` (12 verdicts, `runnable` 25 → 31), the **five weak +credits** pinned deliberately, what the gate stops being able to see once it +runs, the rows only a human could judge, and `sglang-matrix.md`'s +listed-but-empty audit, now resolved by dropping it. + ### Headless mode Subsystem A made mode a declaration: interactive by default, headless only when diff --git a/.agents/workflow.md b/.agents/workflow.md index 871f2c9a..01819784 100644 --- a/.agents/workflow.md +++ b/.agents/workflow.md @@ -163,6 +163,50 @@ Run `scripts/agent-onboard.py --probe` to see what is still unresolved. by `developer-preferences.md`. The project protocol itself grants no push, merge, force-update, or remote-host authority. + +### Running a row through sub-agents + +If you claimed `operator` in the role interview above, this is the loop. It sits +BELOW the numbered protocol on purpose: steps 0–6 apply to every role, this +section only to an operator, and a helper skipping an operator-only heading must +never skip "Declare your role". You decompose, dispatch, verify and integrate; +you do not write the feature. Tasks run **serially** — never two implementers in +one worktree. + +1. Dispatch a **fresh** implementer ([prompt](prompts/implementer.md)). It works + TDD, commits in its own worktree, and returns the SHA. +2. **Run the row's gate yourself.** Never take the implementer's word for + "done": if done is the author's own opinion of its work, the loop has no + floor at all. +3. Dispatch a **fresh** reviewer ([prompt](prompts/reviewer.md)) — + never the agent that wrote the code. Its binding instruction is to + **mutate, not read**: delete the line each test names, re-run, and a test + that stays green is a finding. Seventeen such tests are known as of + 2026-08-06 — eleven across the two branches that built this protocol, six + more on this one — a gate's own default satisfied by an unrelated line, a + probe with five hardcoded fields, a cannot-fail rule that rejected nothing, + a wiring test green even with its gate in report mode — and **not one was + visible by reading a diff**. Read that as a dated floor, not a running + total: the reviewer prompt still carries the earlier floor of eleven, and + these counts only grow. A reviewer who reads is worth very little. The whole + return is in the mutation step. +4. Findings go back to a fresh implementer, then a **scoped re-review** of the + fix diff only. **Never fix findings yourself** — a controller fix pollutes + the context that exists to coordinate, and skips review entirely. + +A gate command must exit nonzero on failure. Never `true`, never `echo ok`, +never piped — `cmd | tail` reports `tail`'s status, not the command's. +`scripts/check-gate-commands.py` pins the SET of rows that already carry a gate +command, and it is an **exact pin**, not a shrink-only floor: a row may never +lose one, and a row that gains one turns the suite red just the same. Any +movement, up or down, re-pins `RUNNABLE_BASELINE` in the same change, naming the +rows and the reason. Growth is welcome; silent growth is not. + +Interactive is the default. In a **declared** headless run, decide rather than +ask, record every decision in [state.md](state.md), park what will not go +green, and never merge. + + ## Obligated public surfaces These are the surfaces `scripts/check-doc-checkpoint.py` enforces, declared here diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 734089d2..2aba6d5f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -88,6 +88,18 @@ jobs: run: | python3 scripts/check-now-current.py python3 tests/scripts/test_check_now_current.py + - name: A gated row may never lose a gate command that can FAIL + # An EXACT PIN over the SET of rows whose spec names a runnable command + # -- not a count, and not shrink-only. Not a count, because a count + # cannot tell a row that LOST its command from one that legitimately + # left the gated population, and the fix for the second reads as + # lowering the number for the first. Not shrink-only, because the unit + # test asserts the set EQUALS RUNNABLE_BASELINE, so GROWTH reds this job + # too: any movement, up or down, re-pins the baseline in the same + # change, naming the rows and the reason. + run: | + python3 scripts/check-gate-commands.py --check + python3 tests/scripts/test_check_gate_commands.py - name: Agent role machinery and role discipline run: | python3 scripts/check-role-discipline.py diff --git a/AGENTS.md b/AGENTS.md index 45ae1df0..70ca929a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -124,7 +124,18 @@ version, this list is the reminder. helper works in an isolated worktree on `row/` and opens a DRAFT PR at the START: that PR **is** the claim. The operator merges PRs first thing, owns `main` and the GPU, and drives feature work through sub-agents rather than - writing it. Add `--headless` only when the developer has SAID the run is + writing it — the loop is written down in + [workflow.md § Running a row through sub-agents](.agents/workflow.md#running-a-row-through-sub-agents) + and its two sub-agent contracts are tracked artifacts + ([implementer](.agents/prompts/implementer.md), + [reviewer](.agents/prompts/reviewer.md), gated by + `scripts/check-protocol-consistency.py`). Three rules carry it: the operator + RUNS the row's gate itself rather than believing the implementer's report; the + reviewer is a FRESH agent whose binding instruction is to MUTATE, not read, + because every finding that mattered here came from deleting a line and + re-running, never from reading a diff; and findings are NEVER fixed in the + operator's own session — they go back to a fresh implementer, then a scoped + re-review. Add `--headless` only when the developer has SAID the run is unattended; it is declared, never inferred. - **Never three-way merge a keyed record.** `docs/STATUS.md`, `docs/BENCHMARKS.md`, `docs/FEATURES.md`, `.agents/NOW.md`, the matrices and diff --git a/docs/BENCHMARKS.md b/docs/BENCHMARKS.md index 9e4a3aec..7e34bb61 100644 --- a/docs/BENCHMARKS.md +++ b/docs/BENCHMARKS.md @@ -249,7 +249,13 @@ in the tree, default-OFF, for reproducibility; detail in the benchmark record. ## How we measure Record dates are CI-guarded: state anchors dated in the future are rejected -(`check-state-order`), so scoreboard stamps trace to real landing dates. +(`check-state-order`), so scoreboard stamps trace to real landing dates. The +review protocol behind these numbers is guarded the same way: the reviewer and +implementer sub-agent prompts are tracked artifacts checked by +`check-protocol-consistency` (orchestration harness step 5/5), and +`check-gate-commands` pins the 25 record rows that name a gate command able to +FAIL. That pin is exact, not shrink-only: gaining a gate command reddens it too, +so the set is never re-pinned silently in either direction. **Hardware.** NVIDIA GB10 / DGX Spark (sm_121a) for CUDA, `dgx.casa` aarch64 for CPU, Apple M4 for Metal. GB10's 119 GiB pool is unified, so host and device diff --git a/docs/STATUS.md b/docs/STATUS.md index c548d2b2..ed6daf11 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -31,6 +31,8 @@ citing "vLLM 0.25.0" are the last binding measurement against the prior oracle ## Capability status +Orchestration prompts (2026-08-06): tracked pair; 25 gate rows exact-pinned, step 5/5. + Supported-model registry guard (2026-08-06): the public per-architecture list in [FEATURES](FEATURES.md) is CI-bound to the C++ registry by `scripts/check-supported-models.py` (+ mutation test), so the 30 diff --git a/docs/superpowers/plans/2026-08-06-orchestration-harness.md b/docs/superpowers/plans/2026-08-06-orchestration-harness.md new file mode 100644 index 00000000..227a2396 --- /dev/null +++ b/docs/superpowers/plans/2026-08-06-orchestration-harness.md @@ -0,0 +1,1001 @@ +# Orchestration Harness Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make the operator's loop a written, gated protocol — an independent reviewer that mutates rather than reads, and a gate command that can actually fail. + +**Architecture:** Three deliverables, each independently useful. The **reviewer and implementer prompts** become tracked artifacts under `.agents/prompts/`, so the highest-value piece stops being folklore. `scripts/check-gate-commands.py` classifies every `READY`-or-later row's gate command and ships as a **ratchet**, not a demand — 67 of 97 rows cannot state one today, so a gate requiring them would be red on arrival. The **loop itself** lands in `.agents/workflow.md` with `check-protocol-consistency.py` asserting it, exactly as subsystem A landed the role interview. + +**Tech Stack:** Python 3 standard library only, `argparse`, `importlib.util` for hyphenated module loading, `unittest`. Markdown for the tracked prompts. Matches the house style of `scripts/check-agent-record.py`. + +## Global Constraints + +Copied from `AGENTS.md` and `.agents/specs/orchestration-harness.md`. Every task's requirements implicitly include this section. + +- **Every commit carries `FOLLOWING_AGENTS_PROTOCOL`** plus `Assisted-by: Claude Code:claude-opus-5 [ClaudeCode]`. **Never** `Signed-off-by` or `Co-Authored-By` from an AI. +- **A role must be declared before committing** — subsystem A shipped, so `scripts/agent-preflight.sh` fails without one. Run `python3 scripts/agent-role.py show`; this worktree should already hold `helper row=HARNESS-B`. Claim before you commit, never soften the gate. +- **Run `bash scripts/agent-preflight.sh` before committing; it must exit 0.** Never pipe it — redirect to a file and check `$?`. +- **Every commit touching `scripts/`, `tests/` or `.agents/specs/` also updates `docs/STATUS.md` and `docs/BENCHMARKS.md` in the SAME commit.** Verify the **committed** form with `python3 scripts/check-doc-checkpoint.py --commit ` — preflight runs that checker `--staged` only, which passes vacuously after committing, and `--staged` can also fail spuriously once the docs are already committed. `--commit` on the final SHA is the authoritative check. +- **Doc budgets are tight and must be measured, never assumed.** At the merge base: `docs/STATUS.md` 283,992 of a 284,081 cap (**89 chars**), `.agents/NOW.md` 5,973 of 6,000 (**27 chars**), and every `docs/BENCHMARKS.md` prose paragraph is near its 700-char limit. Measure with: + ```bash + python3 -c "import importlib.util,sys; s=importlib.util.spec_from_file_location('c','scripts/check-public-doc-tables.py'); m=importlib.util.module_from_spec(s); sys.modules['c']=m; s.loader.exec_module(m); t=open('docs/STATUS.md').read(); print(len(t), m.STATUS_RATCHET['chars'])" + ``` +- **Use a ROLLING doc surface.** Task 1 adds ONE short line to each page; every later task **edits that line digit-only** (`step 1/5` → `2/5` → … → `step 5/5`) so the pages do not grow. **Never rewrite an unrelated paragraph to buy room** — that was done twice on earlier branches and introduced a factual error both times. If your entry does not fit, shorten **your own** sentence. +- **Python standard library only.** `from __future__ import annotations`, type hints, house style. +- **Never weaken a checker, raise a cap, or relax a budget to make something pass. Repair the record.** +- Stage explicit paths. Never `git add -A`. +- **For every test you write, mutate the line it names and confirm it goes red, and report the result.** Eleven times across the two preceding branches a test passed with its subject deleted — including a gate's own default and a probe's five fields. This is the single most reliable defect class in this repo. + +**Existing interfaces you build on** (read, do not reimplement): + +- `scripts/check-agent-record.py`: `ClaimRow` (fields `path`, `line_no`, `item_id`, `state`, `header`, `cells`, `raw`; method `field(name)`), `parse_claim_rows(path, errors) -> list[ClaimRow]`, `MATRIX_PATHS`, `AGENTS`, `ROOT`, `local_spec_paths(row) -> list[Path]` (resolves a row's `Spike/spec` links to real files under `.agents/specs/`). +- `scripts/check-protocol-consistency.py`: `INTERVIEW_MARKER` (line 56), `INTERVIEW_REQUIRED` (57), `interview_errors(text) -> list[str]` (133), `main()` (144). +- `scripts/agent-preflight.sh`: `CHECKERS=(` at line 57, `SUITES=(` at line 73. +- `.github/workflows/ci.yml`: lines 94–95 run `test_agent_role.py` and `test_agent_onboard.py`. + +**Measured baseline (merge base `35f7cb94`), the number that shapes this plan:** of **97** `READY`-or-later rows — 30 have a `Gates` section containing a runnable command, **46 have a `Gates` section with no command**, **20 have a spec with no `Gates` section**, 1 has no resolving spec. **67 of 97 (69%) cannot state a runnable gate command today.** + +--- + +## File Structure + +| File | Responsibility | +|---|---| +| `.agents/prompts/reviewer.md` (create) | The reviewer contract: mutate, don't read; don't trust the report; a plan-mandated finding is still a finding | +| `.agents/prompts/implementer.md` (create) | The implementer contract: TDD, commit in the worktree, report honestly, escalate rather than guess | +| `scripts/check-gate-commands.py` (create) | Classify each `READY`+ row's gate command; report mode, then a ratchet | +| `tests/scripts/test_check_gate_commands.py` (create) | Unit + mutation suite | +| `.agents/specs/gate-command-audit-2026-08-06.md` (create) | The 67-row debt, recorded honestly | +| `scripts/check-protocol-consistency.py` (modify) | Assert the prompts exist and carry their binding instruction; assert the loop is in `workflow.md` | +| `scripts/agent-preflight.sh`, `.github/workflows/ci.yml` (modify) | Wire the checker and its suite | +| `AGENTS.md`, `.agents/workflow.md`, `.agents/specs/operator-helper-protocol.md` (modify) | The loop, moved with its gate | + +--- + +### Task 1: The tracked prompts + +**Files:** +- Create: `.agents/prompts/reviewer.md`, `.agents/prompts/implementer.md` +- Modify: `scripts/check-protocol-consistency.py` +- Test: `tests/scripts/test_check_protocol_consistency.py` + +**Interfaces:** +- Consumes: `interview_errors(text) -> list[str]` and `main()` from `check-protocol-consistency.py`. +- Produces: `PROMPT_REQUIRED: dict[str, tuple[str, ...]]` mapping each prompt path to phrases it must contain; `prompt_errors() -> list[str]`. + +**Why this is Task 1:** it is the highest-value deliverable and depends on nothing. Across two branches, **every Important finding came from an independent reviewer and none from an implementer's self-review**, and the reviewers found them by mutating code, not by reading diffs. A prompt that lives only in an operator's head is not a protocol. + +- [ ] **Step 1: Write the failing test** + +Append to `tests/scripts/test_check_protocol_consistency.py`, above its `if __name__` block: + +```python +class PromptArtifactTests(unittest.TestCase): + def test_both_prompts_exist_and_are_tracked(self): + for name in ("reviewer.md", "implementer.md"): + path = ROOT / ".agents/prompts" / name + self.assertTrue(path.is_file(), f"{name} must exist") + + def test_the_reviewer_prompt_carries_the_mutation_instruction(self): + # The instruction IS the deliverable. A reviewer told only to "review" + # reads the diff, and reading found none of the eleven tests that + # passed with their subject deleted. + text = (ROOT / ".agents/prompts/reviewer.md").read_text(encoding="utf-8") + for needle in ("mutate", "delete or invert", "stays green"): + self.assertIn(needle, text.lower(), needle) + + def test_the_reviewer_prompt_refuses_to_defer_to_the_plan(self): + text = (ROOT / ".agents/prompts/reviewer.md").read_text(encoding="utf-8") + self.assertIn("plan-mandated", text.lower()) + + def test_checker_rejects_a_prompt_missing_its_instruction(self): + errors = consistency.prompt_errors({"nonexistent-prompt.md": ("mutate",)}) + self.assertTrue(errors) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `python3 tests/scripts/test_check_protocol_consistency.py -v` +Expected: FAIL — the prompt files do not exist, and `AttributeError: module … has no attribute 'prompt_errors'`. + +- [ ] **Step 3: Write the prompts** + +Create `.agents/prompts/reviewer.md`: + +```markdown +# Reviewer prompt + +You review one change. You did not write it and you will not fix it. + +## The binding instruction: mutate, don't read + +For each test in the change, **delete or invert the line it names and re-run +the suite. A test that stays green is a finding**, regardless of how it reads. + +This is not a style preference. Across two branches of this project, eleven +tests passed with the thing they named deleted — including a gate's own +default (an unrelated line satisfied the assertion), a probe with five +hardcoded fields, and `assertIn("merged", reason)` where the string was +`"unmerged"`. **None was visible by reading the diff.** A reviewer who reads +and comments on style adds nothing this project has not already paid for. + +## Do not trust the report + +Treat the implementer's report as unverified claims about the code. Verify each +against the change. A stated rationale — "kept it simple deliberately", "left +it per YAGNI" — is the implementer grading its own work and **never** downgrades +a finding's severity. On two branches, three implementer reports asserted +something false in good faith; each was caught by reproducing the claim rather +than accepting it. + +## A plan-mandated finding is still a finding + +Roughly half of all Important findings on the preceding branches were defects in +the **plan text**, not the implementation. A reviewer that treats the plan as +authority cannot find them. Report them, labelled `plan-mandated`, and let the +human decide which governs. + +## Severity + +- **Critical** — corrupts the record, weakens a gate, or leaves a false claim in + a document agents read. +- **Important** — the change cannot be trusted until fixed: wrong or fragile + behavior, a missed requirement, a test that asserts nothing. +- **Minor** — polish. + +Cite `file:line` for every finding and for any check you would otherwise answer +with a bare "yes". Acknowledge what was done well before listing issues. + +## What you may not do + +- Never fix what you found. Findings go back to a fresh implementer. +- Never mutate the reviewed worktree, its index, HEAD or branch state. Work in a + scratch copy. +- Never re-run the full suite just to confirm the report; run a focused check + when reading raises a specific doubt. +``` + +Create `.agents/prompts/implementer.md`: + +```markdown +# Implementer prompt + +You implement one task. A different agent will review it by mutating your code. + +## Method + +1. Write the failing test first. Run it. Confirm it fails **for the stated + reason** — a test that fails for the wrong reason pins nothing. +2. Implement the minimum that makes it pass. +3. **Mutate every test you wrote**: delete the line it names, confirm red, + restore. Report the results. If a briefed test does not pin what it claims, + fix it and say so — four implementers before you did exactly that and were + right every time. +4. Run the project gate (`scripts/agent-preflight.sh`, redirected to a file, + never piped) and confirm `EXIT=0`. +5. Commit in your worktree with the required trailers, and return the SHA. + +## Honesty rules + +- **Never let a failure and an absence look the same.** Every recorded defect + class in this repo is that bug: a substring `--grep` crediting a row with + another row's commits, `.get()` on a missing key reporting a live claim as + finished, a git failure mapped to `""` and read as "no evidence". +- **Report what you did not do.** An empty concerns section is itself a claim. +- **Escalate rather than guess.** Report `BLOCKED` or `NEEDS_CONTEXT` with + specifics. Bad work is worse than no work, and you will not be penalised for + stopping. +- **Never weaken a checker, a budget or a test to make something pass.** If the + gate is red, repair the record. + +## Deviating from the brief + +You may deviate when the brief is wrong — and it sometimes is. State the +deviation explicitly in your report with the evidence that justifies it. Silent +scope expansion is a defect; a disclosed, argued correction is not. +``` + +- [ ] **Step 4: Write the checker** + +In `scripts/check-protocol-consistency.py`, after `INTERVIEW_REQUIRED` (line 57): + +```python +# The reviewer prompt's value is the MUTATION instruction; a reviewer told only +# to "review" reads the diff, and reading found none of the eleven tests that +# passed with their subject deleted. Pin the instruction, not the file. +PROMPT_REQUIRED = { + ".agents/prompts/reviewer.md": ( + "mutate", + "delete or invert", + "stays green", + "plan-mandated", + ), + ".agents/prompts/implementer.md": ( + "failing test first", + "mutate every test", + "escalate rather than guess", + ), +} + + +def prompt_errors(required: dict[str, tuple[str, ...]] | None = None) -> list[str]: + """Each tracked prompt exists and carries its binding instruction.""" + errors: list[str] = [] + for relative, needles in (required or PROMPT_REQUIRED).items(): + path = ROOT / relative + if not path.is_file(): + errors.append(f"{relative} is missing; the prompt is the protocol") + continue + text = path.read_text(encoding="utf-8").lower() + errors.extend( + f"{relative} omits {needle!r}" for needle in needles if needle not in text + ) + return errors +``` + +Call `prompt_errors()` from `main()` and add its output to the error list, exactly as `interview_errors` is called. + +- [ ] **Step 5: Run test to verify it passes** + +Run: `python3 tests/scripts/test_check_protocol_consistency.py -v` → PASS. +Run: `python3 scripts/check-protocol-consistency.py; echo "EXIT=$?"` → `EXIT=0`. + +- [ ] **Step 6: Mutate** + +Delete the `"delete or invert"` line from `reviewer.md` → the suite must go red. Delete the `prompt_errors()` call from `main()` → `test_checker_rejects_a_prompt_missing_its_instruction` must go red. Restore both and confirm green. Report both results. + +- [ ] **Step 7: Doc surfaces, preflight, commit** + +Add ONE short rolling line to `docs/STATUS.md` (89 chars of headroom — keep it under 80) and one short clause to a `docs/BENCHMARKS.md` prose paragraph that has room; measure first. Both must say `step 1/5`. + +```bash +bash scripts/agent-preflight.sh > /tmp/pf.log 2>&1; echo "EXIT=$?" +git add .agents/prompts/reviewer.md .agents/prompts/implementer.md \ + scripts/check-protocol-consistency.py tests/scripts/test_check_protocol_consistency.py \ + docs/STATUS.md docs/BENCHMARKS.md +git commit -F - <<'EOF' +protocol(prompts): the reviewer contract becomes a tracked artifact (B step 1) + +Across two branches every Important finding came from an independent reviewer +and none from an implementer's self-review, and the reviewers found them by +MUTATING code rather than reading diffs. Eleven tests passed with the thing they +named deleted; none was visible by reading. A prompt that lives only in an +operator's head is not a protocol, so the instruction is tracked and gated. + +FOLLOWING_AGENTS_PROTOCOL +Assisted-by: Claude Code:claude-opus-5 [ClaudeCode] +EOF +python3 scripts/check-doc-checkpoint.py --commit "$(git rev-parse HEAD)"; echo "doc-checkpoint EXIT=$?" +``` + +--- + +### Task 2: Classify gate commands (report only) + +**Files:** +- Create: `scripts/check-gate-commands.py` +- Test: `tests/scripts/test_check_gate_commands.py` + +**Interfaces:** +- Consumes: `check-agent-record.py` — `ClaimRow`, `parse_claim_rows`, `MATRIX_PATHS`, `AGENTS`, `ROOT`, `local_spec_paths`. +- Produces: `GATED_STATES: frozenset[str]`; `AUDITED_MATRIX_PATHS: list[Path]`; `gates_section(text: str) -> str | None`; `runnable_commands(section: str) -> list[str]`; `classify_row(row) -> tuple[str, str]` returning one of `"runnable"`, `"gates-no-command"`, `"no-gates-section"`, `"no-spec"` plus a detail string; `audit() -> list[dict]`; `main(argv=None) -> int`. + +**This task ships NO gate.** It classifies and reports. The ratchet is Task 4, after the debt is recorded — the same ordering the live-state audit used, and for the same reason: a gate wired before the record is repaired has to be relaxed to pass, and a relaxed gate is worse than none. + +- [ ] **Step 1: Write the failing test** + +Create `tests/scripts/test_check_gate_commands.py`: + +```python +#!/usr/bin/env python3 +"""Unit and mutation checks for scripts/check-gate-commands.py. + +A gate command that cannot fail collapses "done" into the implementer's opinion +of its own work. This classifier's only job is to tell a runnable command from +prose that looks like one. +""" + +from __future__ import annotations + +import importlib.util +import sys +import unittest +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[2] + + +def _load(name: str, relative: str): + path = ROOT / relative + spec = importlib.util.spec_from_file_location(name, path) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + sys.modules[name] = module + spec.loader.exec_module(module) + return module + + +gates = _load("check_gate_commands", "scripts/check-gate-commands.py") + + +class GatesSectionTests(unittest.TestCase): + def test_finds_the_gates_heading_at_any_level(self): + for heading in ("## Gates", "### Gates", "#### Gates and evidence"): + text = f"# Spec\n\nintro\n\n{heading}\n\nrun `ctest -R foo`\n\n## Next\n\ntail\n" + section = gates.gates_section(text) + self.assertIsNotNone(section, heading) + self.assertIn("ctest", section) + self.assertNotIn("tail", section, "must stop at the next heading") + + def test_returns_none_when_there_is_no_gates_section(self): + self.assertIsNone(gates.gates_section("# Spec\n\n## Scope\n\nnothing here\n")) + + def test_is_not_fooled_by_the_word_gates_in_prose(self): + # "the gates are green" is not a section heading. + self.assertIsNone(gates.gates_section("# Spec\n\nAll the gates are green.\n")) + + +class RunnableCommandTests(unittest.TestCase): + def test_recognises_a_real_command(self): + for body in [ + "run `ctest -R test_foo`", + "`python3 scripts/check-agent-record.py`", + "```\nbash scripts/agent-preflight.sh\n```", + "`cmake --build build -j`", + ]: + self.assertTrue(gates.runnable_commands(body), body) + + def test_rejects_prose_that_merely_mentions_gating(self): + for body in [ + "Correctness, e2e and performance gates apply.", + "The SACRED gate must pass on GB10.", + "`docs/BENCHMARKS.md`", + ]: + self.assertEqual(gates.runnable_commands(body), [], body) + + def test_rejects_a_command_that_cannot_fail(self): + # These are the exact shapes the spec forbids: a Verify that always + # succeeds turns "done" into an opinion. + for body in ["`true`", "`echo ok`", "`:`", "`echo done && true`"]: + self.assertEqual(gates.runnable_commands(body), [], body) + + def test_rejects_a_piped_command(self): + # `cmd | tail` reports tail's exit status, so the gate cannot fail. + self.assertEqual(gates.runnable_commands("`ctest -R foo | tail -5`"), []) + + +class ShippedRecordTests(unittest.TestCase): + def test_the_audit_covers_every_gated_state(self): + self.assertEqual( + gates.GATED_STATES, + frozenset({"READY", "ACTIVE", "GATING", "DONE", "BLOCKED"}), + ) + + def test_all_seven_matrices_are_audited(self): + names = {p.name for p in gates.AUDITED_MATRIX_PATHS} + self.assertIn("feature-matrix.md", names) + self.assertIn("sglang-matrix.md", names) + self.assertEqual(len(names), 7) + + def test_every_record_carries_a_known_verdict(self): + known = {"runnable", "gates-no-command", "no-gates-section", "no-spec"} + records = gates.audit() + self.assertTrue(records) + for item in records: + self.assertIn(item["verdict"], known) + + def test_report_mode_exits_zero_even_with_debt(self): + # 67 of 97 rows cannot state a command today. Report mode must still + # exit 0 -- the ratchet is step 4, after the debt is recorded. + self.assertEqual(gates.main([]), 0) + + +if __name__ == "__main__": + unittest.main() +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `python3 tests/scripts/test_check_gate_commands.py -v` +Expected: FAIL — `FileNotFoundError`/`AssertionError` from `_load`; the script does not exist. + +- [ ] **Step 3: Write minimal implementation** + +Create `scripts/check-gate-commands.py`: + +```python +#!/usr/bin/env python3 +"""Classify each gated row's Gates section: does it name a command that can FAIL? + +A row's `Gates` field promises "exact commands", and nothing has ever checked +that one exists or that it can fail. A gate that is `true`, `echo ok`, or piped +into another command collapses "done" into the implementer's opinion of its own +work. + +This ships as a CLASSIFIER first and a ratchet second, deliberately: 67 of 97 +gated rows cannot state a runnable command today, so a gate demanding one would +be red on arrival and would have to be relaxed to pass. A relaxed gate is worse +than no gate. + + scripts/check-gate-commands.py # report + scripts/check-gate-commands.py --json # machine-readable +""" + +from __future__ import annotations + +import argparse +import importlib.util +import json +import re +import sys +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] + + +def _load(name: str, relative: str): + path = ROOT / relative + spec = importlib.util.spec_from_file_location(name, path) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + sys.modules[name] = module + spec.loader.exec_module(module) + return module + + +record = _load("agent_record", "scripts/check-agent-record.py") + +# DONE is included: a row that lost its gate command is exactly the regression +# this exists to catch, and DONE rows are the ones people stop looking at. +GATED_STATES = frozenset({"READY", "ACTIVE", "GATING", "DONE", "BLOCKED"}) + +# check-agent-record.py's MATRIX_PATHS covers 5 of the 7 matrices. Audit all +# seven, without widening that constant -- it governs a repo-wide CI gate whose +# row contract these two files have never been held to. +AUDITED_MATRIX_PATHS = [ + *record.MATRIX_PATHS, + record.AGENTS / "feature-matrix.md", + record.AGENTS / "sglang-matrix.md", +] + +_GATES_HEADING = re.compile(r"(?im)^#{1,6}\s*gates\b.*$") +_HEADING = re.compile(r"(?m)^#{1,6}\s") + +# A command is recognised by an executable-looking leading token. +_COMMAND = re.compile( + r"(?:^|\s)(ctest|pytest|python3?|cmake|bash|sh|make|nsys|ncu|git|gh|scripts/|\./|tests/)" +) +# Shapes that cannot fail, so they are not gates at all. +_CANNOT_FAIL = re.compile(r"^\s*(true|:|echo\b)") + + +def gates_section(text: str) -> str | None: + """The body under the first `Gates` HEADING, or None. Prose does not count.""" + match = _GATES_HEADING.search(text) + if not match: + return None + rest = text[match.end() :] + nxt = _HEADING.search(rest) + return rest[: nxt.start()] if nxt else rest + + +def _candidates(section: str) -> list[str]: + inline = re.findall(r"`([^`\n]+)`", section) + fenced = re.findall(r"```[a-z]*\n(.*?)```", section, re.S) + for block in fenced: + inline.extend(line for line in block.splitlines() if line.strip()) + return [c.strip() for c in inline if c.strip()] + + +def runnable_commands(section: str) -> list[str]: + """Commands in this section that could actually fail.""" + good = [] + for candidate in _candidates(section): + if not _COMMAND.search(" " + candidate): + continue + if _CANNOT_FAIL.match(candidate): + continue + if "|" in candidate: # `cmd | tail` reports tail's status + continue + good.append(candidate) + return good + + +def classify_row(row) -> tuple[str, str]: + specs = [p for p in record.local_spec_paths(row) if p.is_file()] + if not specs: + return "no-spec", "no resolving .agents/specs/ link" + text = specs[0].read_text(encoding="utf-8", errors="replace") + section = gates_section(text) + if section is None: + return "no-gates-section", specs[0].name + commands = runnable_commands(section) + if not commands: + return "gates-no-command", specs[0].name + return "runnable", commands[0] + + +def audit() -> list[dict]: + records = [] + for path in AUDITED_MATRIX_PATHS: + errors: list[str] = [] + for row in record.parse_claim_rows(path, errors): + if row.state not in GATED_STATES: + continue + verdict, detail = classify_row(row) + records.append( + { + "id": row.item_id, + "state": row.state, + "path": str(row.path.relative_to(ROOT)), + "line": row.line_no, + "verdict": verdict, + "detail": detail, + } + ) + return records + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description="Classify gated rows' gate commands.") + parser.add_argument("--json", action="store_true", help="machine-readable") + args = parser.parse_args(argv) + + records = audit() + if args.json: + print(json.dumps(records, indent=2, sort_keys=True)) + return 0 + counts: dict[str, int] = {} + for item in records: + counts[item["verdict"]] = counts.get(item["verdict"], 0) + 1 + for verdict in ("runnable", "gates-no-command", "no-gates-section", "no-spec"): + print(f" {counts.get(verdict, 0):4d} {verdict}") + print(f"\n{len(records)} gated rows; {counts.get('runnable', 0)} carry a command that can fail.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) +``` + +Make it executable: `chmod +x scripts/check-gate-commands.py` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `python3 tests/scripts/test_check_gate_commands.py -v` → PASS, 11 tests. + +- [ ] **Step 5: Smoke-test against the real record** + +Run: `python3 scripts/check-gate-commands.py` +Expected: roughly `30 runnable`, `46 gates-no-command`, `20 no-gates-section`, `1 no-spec` over 97 rows. Report the actual numbers — the record moves, and Task 3 consumes them. + +- [ ] **Step 6: Mutate** + +Confirm each goes red, then restore: drop the `"|"` check in `runnable_commands`; drop the `_CANNOT_FAIL` check; make `gates_section` match the word `gates` anywhere rather than at a heading; remove `feature-matrix.md` from `AUDITED_MATRIX_PATHS`. Report all four. + +- [ ] **Step 7: Roll the docs to `step 2/5`, preflight, commit** + +```bash +bash scripts/agent-preflight.sh > /tmp/pf.log 2>&1; echo "EXIT=$?" +git add scripts/check-gate-commands.py tests/scripts/test_check_gate_commands.py \ + docs/STATUS.md docs/BENCHMARKS.md +git commit -F - <<'EOF' +tools(gates): classify whether a gated row names a command that can FAIL (B step 2) + +Report only. 67 of 97 gated rows cannot state a runnable command today, so a +gate demanding one would be red on arrival and would have to be relaxed to +pass -- and a relaxed gate is worse than none. The ratchet is step 4, after +step 3 records the debt. + +FOLLOWING_AGENTS_PROTOCOL +Assisted-by: Claude Code:claude-opus-5 [ClaudeCode] +EOF +python3 scripts/check-doc-checkpoint.py --commit "$(git rev-parse HEAD)"; echo "doc-checkpoint EXIT=$?" +``` + +--- + +### Task 3: Record the debt + +**Files:** +- Create: `.agents/specs/gate-command-audit-2026-08-06.md` + +**Interfaces:** +- Consumes: `scripts/check-gate-commands.py --json`. +- Produces: the artifact Task 4's ratchet baseline cites. + +**No code and no matrix is edited in this task.** The debt lands before the gate, so the reasoning is reviewable separately from the enforcement — and so the ratchet's baseline is a recorded decision rather than a number someone pasted. + +- [ ] **Step 1: Capture the audit** + +```bash +python3 scripts/check-gate-commands.py --json > /tmp/gates.json +python3 scripts/check-gate-commands.py +python3 -c " +import json, collections +r = json.load(open('/tmp/gates.json')) +print(collections.Counter(x['verdict'] for x in r)) +print('by matrix:', collections.Counter(x['path'] for x in r if x['verdict'] != 'runnable')) +" +``` + +- [ ] **Step 2: Hand-verify a sample before trusting the classifier** + +Pick **three** rows it called `runnable` and **three** it called `gates-no-command`. Open each row's spec, read the `Gates` section, and confirm the verdict matches what a human would say. A classifier wrong on a sample is wrong on all 97 — if you find a mismatch, stop and report it rather than writing the artifact. Record the sample and its outcome. + +- [ ] **Step 3: Write the artifact** + +Create `.agents/specs/gate-command-audit-2026-08-06.md` with these sections: + +- **Scope** — the gated rows at `origin/main` @ ``; what the classifier decides and what it cannot. +- **Method** — `scripts/check-gate-commands.py`, the classification rules verbatim, and the Step 2 sample with its outcome. +- **Findings** — the four counts, the per-matrix breakdown, and the full list of rows in each non-`runnable` bucket. +- **What this does NOT mean** — a row without a runnable gate command is not ungated work; many carry real evidence in prose or in `.agents/parity-ledger.md`. The finding is that **the gate cannot be checked mechanically**, not that the work is unverified. Say this plainly; the opposite reading would slander a lot of landed work. +- **The ratchet baseline** — the exact count of `runnable` rows, which Task 4 pins. State that the baseline may only rise. +- **Risks/decisions** — every row the classifier could not decide, and the human call made. + +- [ ] **Step 4: Roll the docs to `step 3/5`, preflight, commit** + +```bash +bash scripts/agent-preflight.sh > /tmp/pf.log 2>&1; echo "EXIT=$?" +git add .agents/specs/gate-command-audit-2026-08-06.md docs/STATUS.md docs/BENCHMARKS.md +git commit -F - <<'EOF' +record(gates): the gate-command debt, measured before anything enforces it (B step 3) + +67 of 97 gated rows cannot state a command that can fail. That is a finding +about MECHANICAL checkability, not about whether the work was verified -- many +of those rows carry real evidence in prose and in the parity ledger, and the +artifact says so plainly. + +Recorded before the ratchet so the baseline is a decision, not a pasted number. + +FOLLOWING_AGENTS_PROTOCOL +Assisted-by: Claude Code:claude-opus-5 [ClaudeCode] +EOF +python3 scripts/check-doc-checkpoint.py --commit "$(git rev-parse HEAD)"; echo "doc-checkpoint EXIT=$?" +``` + +--- + +### Task 4: The ratchet + +> **Superseded on 2026-08-06 by the whole-branch review, in two places. This +> plan text is left as written; where it disagrees with the list below, the list +> governs.** +> +> 1. **"Shrink-only" is wrong** — here, in Task 2's `AUDITED_MATRIX_PATHS` +> comment, in "Done when", and wherever else this plan says it. What shipped +> is an **exact pin**: `RUNNABLE_BASELINE` must EQUAL the audited `runnable` +> set, so growth is red too and any movement re-pins the set in the same +> change. See `.agents/specs/gate-command-audit-2026-08-06.md` § The ratchet +> baseline. +> 2. **`test_all_seven_matrices_are_audited` is wrong**, and its +> `assertIn("sglang-matrix.md", names)` was plan-mandated. `sglang-matrix.md` +> carries a classification column, not a lifecycle state, and contributed 0 +> rows of 87 — listed and empty, which is this repo's recorded defect class. +> It is **dropped** from the audited set with the reasoning recorded; six +> matrices are audited, and the test now pins the justification (zero rows AND +> zero parse errors) instead of the membership. See that artifact's risk 6. + +**Files:** +- Modify: `scripts/check-gate-commands.py` +- Modify: `scripts/agent-preflight.sh` (`CHECKERS=(` line 57, `SUITES=(` line 73) +- Modify: `.github/workflows/ci.yml` (near lines 94–95) +- Test: `tests/scripts/test_check_gate_commands.py` + +**Interfaces:** +- Consumes: `audit()` from Task 2. +- Produces: `RUNNABLE_BASELINE: frozenset[str]` — the SET of row IDs carrying a runnable gate command, **not a count**; `ratchet_errors(records: list[dict]) -> list[str]`; `--check` on `main()`. + +**Why a ratchet and not a demand:** 72 of 97 rows cannot satisfy a demand today, and this repo already uses shrink-only ratchets for exactly this shape (`STATUS_RATCHET`, the device-leakage ratchet). It ships green today and gets stricter every time someone fixes a row. + +**Pin the SET of row IDs, never a count** (specified by step 3's artifact, § the ratchet baseline). A bare integer cannot distinguish *a row lost its gate command* from *a row legitimately left the population* — and the population already moved 3 rows mid-branch, which is why 97 was deliberately never pinned. A count would go red on a legitimate record edit, and the author would "fix" it by lowering the number, which is the gate erasing its own finding. + +**The rule, verbatim from the artifact:** a drop is a regression **only if** the row still exists and is still in `GATED_STATES`. If the row was deleted, merged, or transitioned out, the baseline is re-pinned **in the same change**, naming the row and the reason. `ratchet_errors` must therefore report *which* IDs left and why it could or could not tell. + +- [ ] **Step 1: Write the failing test** + +Append to `tests/scripts/test_check_gate_commands.py`, above `if __name__`: + +```python +class RatchetTests(unittest.TestCase): + def test_the_baseline_matches_the_shipped_record(self): + runnable = {r["id"] for r in gates.audit() if r["verdict"] == "runnable"} + self.assertEqual(runnable, set(gates.RUNNABLE_BASELINE)) + + def test_a_row_that_loses_its_command_is_refused(self): + # Still present, still gated, no longer runnable -- a real regression. + victim = sorted(gates.RUNNABLE_BASELINE)[0] + records = [{"verdict": "gates-no-command", "id": victim, "state": "READY", + "path": "p", "line": 1, "detail": "d"}] + errors = gates.ratchet_errors(records) + self.assertTrue(errors) + self.assertIn(victim, errors[0]) + self.assertIn("Repair the row", errors[0]) + + def test_a_row_that_left_the_population_reports_differently(self): + # Deleted or transitioned out: legitimate, but must re-pin. The two + # cases MUST be distinguishable -- that is why the baseline is a set. + errors = gates.ratchet_errors([]) + self.assertTrue(errors) + self.assertTrue(any("left the gated population" in e for e in errors)) + self.assertFalse(any("Repair the row" in e for e in errors)) + + def test_an_improvement_is_allowed(self): + records = [ + {"verdict": "runnable", "id": rid, "state": "READY", + "path": "p", "line": 1, "detail": "d"} + for rid in sorted(gates.RUNNABLE_BASELINE) + ] + [{"verdict": "runnable", "id": "NEW-ROW", "state": "READY", + "path": "p", "line": 2, "detail": "d"}] + self.assertEqual(gates.ratchet_errors(records), []) + + def test_check_mode_passes_on_the_shipped_record(self): + # The gate ships GREEN. It was wired after the debt was recorded, so it + # never had to be relaxed to pass. + self.assertEqual(gates.main(["--check"]), 0) + + def test_the_checker_is_wired_into_preflight_and_ci(self): + preflight = (ROOT / "scripts/agent-preflight.sh").read_text(encoding="utf-8") + self.assertIn("check-gate-commands", preflight) + self.assertIn("test_check_gate_commands", preflight) + ci = (ROOT / ".github/workflows/ci.yml").read_text(encoding="utf-8") + self.assertIn("scripts/check-gate-commands.py --check", ci) + self.assertIn("tests/scripts/test_check_gate_commands.py", ci) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `python3 tests/scripts/test_check_gate_commands.py -v` +Expected: FAIL with `AttributeError: … has no attribute 'RUNNABLE_RATCHET'`. + +- [ ] **Step 3: Write minimal implementation** + +Append to `scripts/check-gate-commands.py`, above `main()`: + +```python +# Shrink-only, like STATUS_RATCHET in check-public-doc-tables.py -- but a SET of +# row IDs, not a count. A count cannot tell "this row lost its gate command" +# from "this row left the population", and the population moves: 3 rows moved +# mid-branch while step 2 was being written. Pinning a count would go red on a +# legitimate record edit, and the natural "fix" is to lower the number, which is +# the gate erasing its own finding. +RUNNABLE_BASELINE = frozenset({ + # the exact row IDs step 3 recorded as `runnable` +}) + + +def ratchet_errors(records: list[dict]) -> list[str]: + """A row may not silently lose its gate command. + + Leaving the gated population is legitimate; losing the command is not. So + only IDs still PRESENT and still gated can be a regression -- anything else + is a record edit that must re-pin the baseline in the same change. + """ + runnable = {item["id"] for item in records if item["verdict"] == "runnable"} + present = {item["id"] for item in records} + lost = sorted((RUNNABLE_BASELINE - runnable) & present) + departed = sorted(RUNNABLE_BASELINE - runnable - present) + errors = [] + if lost: + errors.append( + "these rows still exist and are still gated but no longer name a " + f"command that can fail: {', '.join(lost)}. Repair the row, never " + "the baseline." + ) + if departed: + errors.append( + f"these baseline rows left the gated population: {', '.join(departed)}. " + "If that is a legitimate record edit, re-pin RUNNABLE_BASELINE in the " + "SAME change, naming each row and the reason." + ) + return errors +``` + +In `main()`, add the flag and return its errors: + +```python + parser.add_argument("--check", action="store_true", help="fail on a ratchet regression") +``` + +```python + if args.check: + errors = ratchet_errors(records) + for line in errors: + print(f"ERROR: {line}", file=sys.stderr) + return 1 if errors else 0 +``` + +- [ ] **Step 4: Wire preflight and CI** + +Add `check-gate-commands` to `CHECKERS=(` (line 57) — note `claim-view` shows the pattern for a checker needing an argument; this one needs `--check`, so follow the `claim-view` branch shape. Add `test_check_gate_commands` to `SUITES=(` (line 73). + +In `.github/workflows/ci.yml`, beside the existing script-suite lines (94–95): + +```yaml + python3 scripts/check-gate-commands.py --check + python3 tests/scripts/test_check_gate_commands.py +``` + +- [ ] **Step 5: Run tests and both gates** + +```bash +python3 tests/scripts/test_check_gate_commands.py -v # PASS, 16 tests +python3 scripts/check-gate-commands.py --check; echo "EXIT=$?" # 0 +bash scripts/agent-preflight.sh > /tmp/pf.log 2>&1; echo "EXIT=$?" # 0 +``` + +- [ ] **Step 6: Mutate** + +Confirm each goes red, then restore: delete a real gate command from a spec whose row is in the baseline (the regression the gate exists to catch); replace the `& present` intersection with nothing, so a departed row is misreported as a loss; delete the `check-gate-commands` line from `CHECKERS`; delete the CI line. Report all four. **If `--check` is red for any reason other than your own mutation, the record regressed — repair the row, never the baseline.** + +- [ ] **Step 7: Roll the docs to `step 4/5`, preflight, commit** + +```bash +bash scripts/agent-preflight.sh > /tmp/pf.log 2>&1; echo "EXIT=$?" +git add scripts/check-gate-commands.py tests/scripts/test_check_gate_commands.py \ + scripts/agent-preflight.sh .github/workflows/ci.yml docs/STATUS.md docs/BENCHMARKS.md +git commit -F - <<'EOF' +gate(gates): a row may never lose its runnable gate command (B step 4) + +Shrink-only ratchet, the same shape as STATUS_RATCHET: the count of gated rows +carrying a command that can FAIL may never fall. It ships green because it was +wired AFTER step 3 recorded the debt, so it never had to be relaxed to pass, +and it gets stricter every time someone fixes a row. + +FOLLOWING_AGENTS_PROTOCOL +Assisted-by: Claude Code:claude-opus-5 [ClaudeCode] +EOF +python3 scripts/check-doc-checkpoint.py --commit "$(git rev-parse HEAD)"; echo "doc-checkpoint EXIT=$?" +``` + +--- + +### Task 5: The loop, moved with its gate + +**Files:** +- Modify: `.agents/workflow.md`, `AGENTS.md`, `.agents/specs/operator-helper-protocol.md` +- Modify: `scripts/check-protocol-consistency.py` +- Test: `tests/scripts/test_check_protocol_consistency.py` + +**Interfaces:** +- Consumes: `prompt_errors()` from Task 1. +- Produces: `LOOP_MARKER = ""`; `loop_errors(text: str) -> list[str]`. + +**Why the checker moves in the same commit:** `check-protocol-consistency.py` exists because an obligation was once migrated in `AGENTS.md` and the checker but not in the manual, which went on instructing agents to do the thing the migration removed. Subsystem A landed the role interview this way; the loop lands the same way. + +- [ ] **Step 1: Write the failing test** + +Append to `tests/scripts/test_check_protocol_consistency.py`, above `if __name__`: + +```python +class OrchestrationLoopTests(unittest.TestCase): + def test_workflow_carries_the_loop(self): + text = (ROOT / ".agents/workflow.md").read_text(encoding="utf-8") + self.assertIn(consistency.LOOP_MARKER, text) + + def test_the_loop_names_the_reviewer_and_the_gate(self): + text = (ROOT / ".agents/workflow.md").read_text(encoding="utf-8").lower() + for needle in ("reviewer", "mutate", "run the gate yourself", "never fix"): + self.assertIn(needle, text, needle) + + def test_the_loop_points_at_the_tracked_prompts(self): + text = (ROOT / ".agents/workflow.md").read_text(encoding="utf-8") + self.assertIn(".agents/prompts/reviewer.md", text) + + def test_checker_rejects_a_workflow_without_the_loop(self): + self.assertTrue(consistency.loop_errors("# workflow\n\nno loop here\n")) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `python3 tests/scripts/test_check_protocol_consistency.py -v` +Expected: FAIL with `AttributeError: … has no attribute 'LOOP_MARKER'`. + +- [ ] **Step 3: Write the prose** + +In `.agents/workflow.md`, after the role-interview block, insert: + +```markdown + +### Running a row through sub-agents + +Decompose, dispatch, verify, integrate. You do not write the feature. + +For each task, serially — never two implementers in one worktree: + +1. Dispatch a **fresh** implementer ([prompt](prompts/implementer.md)). It works + TDD and commits in the worktree, and returns the SHA. +2. **Run the row's gate yourself.** This is the one failure mode nothing else + catches: if "done" is the implementer's opinion of its own work, the loop has + no floor. +3. Dispatch a **fresh reviewer** ([prompt](prompts/reviewer.md)) — never the + agent that wrote the code. Its binding instruction is to **mutate, not read**: + delete the line each test names and re-run. A test that stays green is a + finding. Eleven such tests shipped on the two branches that built this + protocol, and none was visible by reading a diff. +4. Findings go back to the implementer, then a **scoped re-review** of the fix + diff only. **Never fix findings yourself** — a controller fix pollutes the + context that exists to coordinate, and skips review entirely. + +A gate command must exit nonzero on failure. Never `true`, never `echo ok`, +never piped — `cmd | tail` reports `tail`'s status. +`scripts/check-gate-commands.py` ratchets this. + +Interactive is the default. In a **declared** headless run, decide rather than +ask, record every decision in `.agents/state.md`, park what will not go green, +and never merge. + +``` + +In `scripts/check-protocol-consistency.py`, add: + +```python +LOOP_MARKER = "" +LOOP_REQUIRED = ("prompts/reviewer.md", "mutate", "run the row's gate yourself") + + +def loop_errors(text: str) -> list[str]: + """The operator's loop must live where agents read it, not in a prompt.""" + if LOOP_MARKER not in text: + return [".agents/workflow.md is missing the orchestration-loop block"] + lowered = text.lower() + return [ + f".agents/workflow.md loop omits {needle!r}" + for needle in LOOP_REQUIRED + if needle.lower() not in lowered + ] +``` + +Call `loop_errors` from `main()` against `.agents/workflow.md`. + +Update `AGENTS.md`'s operator bullet to point at the loop and the tracked prompts. Update `.agents/specs/operator-helper-protocol.md` to record that the operator drives feature work through sub-agents with an independent reviewer, and link both prompts. + +- [ ] **Step 4: Run tests and the checker** + +```bash +python3 tests/scripts/test_check_protocol_consistency.py -v # PASS +python3 scripts/check-protocol-consistency.py; echo "EXIT=$?" # 0 +``` + +- [ ] **Step 5: Mutate** + +Confirm each goes red, then restore: delete the loop block from `workflow.md`; delete the `loop_errors()` call from `main()`; empty `LOOP_REQUIRED`. Report all three. + +- [ ] **Step 6: Roll the docs to `step 5/5`, preflight, commit** + +```bash +bash scripts/agent-preflight.sh > /tmp/pf.log 2>&1; echo "EXIT=$?" +git add AGENTS.md .agents/workflow.md .agents/specs/operator-helper-protocol.md \ + scripts/check-protocol-consistency.py tests/scripts/test_check_protocol_consistency.py \ + docs/STATUS.md docs/BENCHMARKS.md +git commit -F - <<'EOF' +docs(protocol): the operator's loop ships with the gate that asserts it (B step 5) + +check-protocol-consistency.py exists because an obligation was once migrated in +AGENTS.md and the checker but not in the manual, which went on instructing +agents to do the thing the migration removed. Prose is what agents read, so the +loop lands in workflow.md and the checker asserts it is there -- the same way +the role interview landed. + +FOLLOWING_AGENTS_PROTOCOL +Assisted-by: Claude Code:claude-opus-5 [ClaudeCode] +EOF +python3 scripts/check-doc-checkpoint.py --commit "$(git rev-parse HEAD)"; echo "doc-checkpoint EXIT=$?" +``` + +--- + +## Done when + +- `.agents/prompts/reviewer.md` and `implementer.md` are tracked, and `check-protocol-consistency.py` fails without their binding instructions. +- `scripts/check-gate-commands.py --check` exits 0 and is wired into preflight and CI as a shrink-only ratchet. +- `.agents/specs/gate-command-audit-2026-08-06.md` records the debt honestly, including what it does *not* mean. +- The loop is in `.agents/workflow.md` and the checker fails without it. +- Every commit passes `python3 scripts/check-doc-checkpoint.py --commit `. + +## Out of scope + +Backfilling gate commands for the 67 rows that lack one — that is per-row work needing per-row knowledge, and the ratchet makes it incremental rather than a flag day. Automating the dispatch loop itself: this protocol is executed by an operator reading `workflow.md`, not by a program. Any change to the roles, the interview, or `.env` handling — that was subsystem A and it has shipped. diff --git a/scripts/agent-preflight.sh b/scripts/agent-preflight.sh index d94b9f02..030054a2 100755 --- a/scripts/agent-preflight.sh +++ b/scripts/agent-preflight.sh @@ -68,6 +68,7 @@ CHECKERS=( check-protocol-consistency check-state-order check-now-current + check-gate-commands ) SUITES=( @@ -88,6 +89,7 @@ SUITES=( test_check_state_order test_check_now_current test_audit_live_rows + test_check_gate_commands ) failed=() @@ -138,7 +140,10 @@ fi echo "Record gates:" for checker in "${CHECKERS[@]}"; do case "$checker" in - claim-view) run "$checker" python3 "scripts/$checker.py" --check ;; + # Both default to a REPORT that exits 0 whatever the record says; the gate + # is the flag. Wiring either without --check installs a gate that cannot + # fail, which for check-gate-commands is the very defect it classifies. + claim-view|check-gate-commands) run "$checker" python3 "scripts/$checker.py" --check ;; *) run "$checker" python3 "scripts/$checker.py" ;; esac done diff --git a/scripts/check-gate-commands.py b/scripts/check-gate-commands.py new file mode 100755 index 00000000..d6c5ac5d --- /dev/null +++ b/scripts/check-gate-commands.py @@ -0,0 +1,341 @@ +#!/usr/bin/env python3 +"""Classify each gated row's Gates section: does it name a command that can FAIL? + +A row's `Gates` field promises "exact commands", and nothing has ever checked +that one exists or that it can fail. A gate that is `true`, `echo ok`, or piped +into another command collapses "done" into the implementer's opinion of its own +work. + +This ships as a CLASSIFIER first and a ratchet second, deliberately: most gated +rows cannot state a runnable command today, so a gate demanding one would be red +on arrival and would have to be relaxed to pass. A relaxed gate is worse than no +gate. + +The ratchet is the second half, wired only AFTER the debt was recorded +(.agents/specs/gate-command-audit-2026-08-06.md), so it ships green and never had +to be relaxed to pass. It pins the SET of rows carrying a runnable command, and +it is an EXACT PIN, not a shrink-only floor: `--check` below refuses a row that +LOST its command, and tests/scripts/test_check_gate_commands.py additionally +asserts RUNNABLE_BASELINE equals the shipped set, so growth is red too. That is +deliberate -- an exact pin is what makes "just lower the number" impossible -- +and it means ANY movement, up or down, re-pins RUNNABLE_BASELINE in the SAME +change, naming the rows and the reason. Growth is welcome; silent growth is not. + + scripts/check-gate-commands.py # report + scripts/check-gate-commands.py --json # machine-readable + scripts/check-gate-commands.py --check # gate: no row may lose its command +""" + +from __future__ import annotations + +import argparse +import importlib.util +import json +import re +import sys +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] + + +def _load(name: str, relative: str): + path = ROOT / relative + spec = importlib.util.spec_from_file_location(name, path) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + sys.modules[name] = module + spec.loader.exec_module(module) + return module + + +record = _load("agent_record", "scripts/check-agent-record.py") + +# DONE is included: a row that lost its gate command is exactly the regression +# this exists to catch, and DONE rows are the ones people stop looking at. +GATED_STATES = frozenset({"READY", "ACTIVE", "GATING", "DONE", "BLOCKED"}) + +# check-agent-record.py's MATRIX_PATHS covers 5 of the 7 matrices. feature-matrix +# is added here without widening that constant -- it governs a repo-wide CI gate +# whose row contract these two files have never been held to. +# +# sglang-matrix.md is DELIBERATELY ABSENT, and the reason is recorded rather than +# implied (.agents/specs/gate-command-audit-2026-08-06.md risk 6). Step 2 listed +# it; it contributed 0 rows of its 87 table rows, with 0 parse errors, because it +# carries a CLASSIFICATION column (FUSED / INVENTORIED / NOT-APPLICABLE) in place +# of a lifecycle state, so parse_claim_rows recognises nothing in it. Listed and +# empty is this repo's recorded defect class -- a reader of this constant would +# conclude SGLang rows were examined and found clean. It has no gated rows to +# audit, so it is not audited; the test pins that justification, and goes red if +# the matrix ever gains lifecycle rows. +AUDITED_MATRIX_PATHS = [ + *record.MATRIX_PATHS, + record.AGENTS / "feature-matrix.md", +] + +_GATES_HEADING = re.compile(r"(?im)^#{1,6}\s*gates\b.*$") +_HEADING = re.compile(r"(?m)^#{1,6}\s") + +# A command names an executable: a known tool as a WHOLE WORD, or a path that is +# actually INVOKED. A backticked filename is not a command -- `docs/BENCHMARKS.md` +# and `tests/vllm/models/test_model_registry.cpp` are things a gate talks about, +# not things it runs. Both boundaries matter on the shipped record: without the +# trailing one `sha256_cbor` matches `sh` and `python@3.14` matches `python`. +_TOOL = re.compile( + r"(?:^|\s)(?:ctest|pytest|python3?|cmake|bash|sh|make|nsys|ncu|git|gh)(?:\s|$)" +) +# `flock -c ''` is this repo's MANDATED shape for any gate touching +# the GPU, and the wrapper QUOTES the real command, putting it out of reach of +# every other rule here. It needs a lockfile AND something to run: the bare +# `flock` and `flock /tmp/gpu` that appear in three specs name the idiom, not a +# gate, and a plain vocabulary entry credits all three with a command. +_WRAPPER = re.compile(r"(?:^|\s)flock\s+\S+\s+\S") +# `./anything` is an explicit invocation, arguments or not -- a built test binary +# (`./build-cuda-121a/tests/test_dropin_abi`) is run, not referred to. A bare +# `scripts/`/`tests/` path is only a command when it carries an executable suffix +# or arguments; otherwise it is a filename. +_INVOKED_PATH = re.compile( + r"(?:^|\s)(?:\./\S+|(?:scripts|tests)/\S*(?:\.(?:py|sh)(?:\s|$)|\s+\S))" +) +# Shapes that cannot fail, so they are not gates at all. +_CANNOT_FAIL = re.compile(r"^\s*(true|:|echo\b)") + + +def gates_section(text: str) -> str | None: + """The body under the first `Gates` HEADING, or None. Prose does not count.""" + match = _GATES_HEADING.search(text) + if not match: + return None + rest = text[match.end() :] + nxt = _HEADING.search(rest) + return rest[: nxt.start()] if nxt else rest + + +def _candidates(section: str) -> list[str]: + inline = re.findall(r"`([^`\n]+)`", section) + fenced = re.findall(r"```[a-z]*\n(.*?)```", section, re.S) + for block in fenced: + inline.extend(line for line in block.splitlines() if line.strip()) + return [c.strip() for c in inline if c.strip()] + + +def is_command(candidate: str) -> bool: + """Does this backticked span name something you could RUN at all? + + The no-op shells (`true`, `:`, `echo ...`) are commands, and are recognised + here DELIBERATELY: `runnable_commands` must reject them for the reason that + matters -- they cannot fail -- and not merely fail to notice them. A + classifier that never sees `true` pins nothing about the rule it exists for. + """ + padded = " " + candidate + return bool( + _TOOL.search(padded) + or _WRAPPER.search(padded) + or _INVOKED_PATH.search(padded) + or _CANNOT_FAIL.match(candidate) + ) + + +def runnable_commands(section: str) -> list[str]: + """Commands in this section that could actually fail.""" + good = [] + for candidate in _candidates(section): + if not is_command(candidate): + continue + if _CANNOT_FAIL.match(candidate): + continue + if "|" in candidate: # `cmd | tail` reports tail's status + continue + good.append(candidate) + return good + + +def classify_row(row) -> tuple[str, str]: + specs = [p for p in record.local_spec_paths(row) if p.is_file()] + if not specs: + return "no-spec", "no resolving .agents/specs/ link" + text = specs[0].read_text(encoding="utf-8", errors="replace") + section = gates_section(text) + if section is None: + return "no-gates-section", specs[0].name + commands = runnable_commands(section) + if not commands: + return "gates-no-command", specs[0].name + return "runnable", commands[0] + + +class RecordParseError(RuntimeError): + """A matrix did not parse, so the audit below it is INCOMPLETE. + + This existed as a silent `errors` list nobody read, and the consequence was + exactly this repo's recorded defect class: strip rows from a matrix and the + ratchet reported `these baseline rows left the gated population ... re-pin + RUNNABLE_BASELINE` -- a parse FAILURE wearing the face of a legitimate + record edit, recommending the one action the audit says must never be taken + blindly. A parse failure and a record edit must never look the same. + """ + + def __init__(self, errors: list[str]) -> None: + super().__init__("; ".join(errors)) + self.errors = list(errors) + + +def audit() -> list[dict]: + """Classify every gated row. Raises RecordParseError if a matrix is broken.""" + records = [] + errors: list[str] = [] + for path in AUDITED_MATRIX_PATHS: + for row in record.parse_claim_rows(path, errors): + if row.state not in GATED_STATES: + continue + verdict, detail = classify_row(row) + records.append( + { + "id": row.item_id, + "state": row.state, + "path": str(row.path.relative_to(ROOT)), + "line": row.line_no, + "verdict": verdict, + "detail": detail, + } + ) + if errors: + raise RecordParseError(errors) + return records + + +# An EXACT PIN over a SET of row IDs -- not a count, and NOT shrink-only. +# +# Not a count, because a count cannot tell "this row lost its gate command" from +# "this row left the population", and the population moves: 3 rows moved +# mid-branch while the classifier above was being written, which is why the total +# (97) was deliberately never pinned. Pinning a count would go red on a legitimate +# record edit, and the natural "fix" is to lower the number, which is the gate +# erasing its own finding. +# +# Not shrink-only, because the pin is enforced from BOTH sides: `ratchet_errors` +# below catches a row that lost its command, and +# tests/scripts/test_check_gate_commands.py asserts this frozenset EQUALS the +# shipped runnable set, which is what makes lowering the baseline impossible to +# do quietly. The same equality means GROWTH is red too: add a real gate command +# to a row's spec and `--check` stays 0 while the suite, preflight and CI go red +# until this set is re-pinned. That is the intended cost. Growth is ordinary, +# welcome work -- transcribe a row's existing evidence into an invocation -- but +# ANY movement, up or down, re-pins RUNNABLE_BASELINE in the SAME change, naming +# the rows that moved and why. +# +# This is a pin, not a certificate: five of these credits are weak (two MLX +# `pip install` lines, `git diff --check`, TOOLS-STREAMING-PARSER resting solely +# on `git diff --stat`, which exits 0 unconditionally in a repo, and +# KERNEL-GEMM-CPU-ELEM credited a bare `ctest -j2` lifted from prose describing a +# FLAKE). They are pinned anyway -- see +# .agents/specs/gate-command-audit-2026-08-06.md risk 3. A ratchet that waits for +# a clean baseline never starts. +RUNNABLE_BASELINE = frozenset({ + "ATTN-CHUNKED-LOCAL", + "ATTN-ROPE-FAMILY", + "BACKEND-CUDA-ARCH-ADDITIVITY", + "BACKEND-METAL-MLX", + "BACKEND-VULKAN", + "ENG-ASYNC-SCHED", + "ENG-CORE-BUSY-LOOP", + "ENG-EXPERT-STREAM", + "ENG-PRIORITY-SCHED", + "KERNEL-GEMM-CPU-ELEM", + "KV-CHUNKED-LOCAL-SPEC", + "KV-SLIDING-LOCAL-SPECS", + "KV-SLIDING-WINDOW-SPEC", + "LOAD-SAFETENSORS-DIRECT-DENSE", + "MODEL-FACTORY-registry", + "MODEL-TEXT-deepseek-v2-glm-moe-dsa-for-causal-lm", + "MODEL-TEXT-gemma4-gemma4-for-causal-lm", + "MODEL-TEXT-glm4-glm4-for-causal-lm", + "MODEL-TEXT-glm4-moe-lite-glm4-moe-lite-for-causal-lm", + "QUANT-GGUF-COMPUTE", + "QUANT-NVFP4-CT-W4A16", + "SERVE-ASYNC-LLM", + "SERVE-HTTP-TRANSPORT", + "SERVE-STREAM-USAGE", + "TOOLS-STREAMING-PARSER", +}) + + +def ratchet_errors(records: list[dict]) -> list[str]: + """A row may not silently lose its gate command. + + Leaving the gated population is legitimate; losing the command is not. So + only IDs still PRESENT and still gated can be a regression -- anything else + is a record edit that must re-pin the baseline in the same change. + + The two are reported as SEPARATE, differently worded errors on purpose: a + single "the count fell" message would make a broken row and a retired row + look the same, which is this repo's recorded defect class and the reason + the baseline is a set. + + This half of the pin sees only DROPS. Growth is caught by the equality + assertion in tests/scripts/test_check_gate_commands.py, so a row that gains + a command still re-pins RUNNABLE_BASELINE -- see the note above the set. + """ + runnable = {item["id"] for item in records if item["verdict"] == "runnable"} + present = {item["id"] for item in records} + lost = sorted((RUNNABLE_BASELINE - runnable) & present) + departed = sorted(RUNNABLE_BASELINE - runnable - present) + errors = [] + if lost: + errors.append( + "these rows still exist and are still gated but no longer name a " + f"command that can fail: {', '.join(lost)}. Repair the row, never " + "the baseline." + ) + if departed: + errors.append( + f"these baseline rows left the gated population: {', '.join(departed)}. " + "If that is a legitimate record edit, re-pin RUNNABLE_BASELINE in the " + "SAME change, naming each row and the reason." + ) + return errors + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description="Classify gated rows' gate commands.") + parser.add_argument("--json", action="store_true", help="machine-readable") + parser.add_argument("--check", action="store_true", help="fail on a ratchet regression") + args = parser.parse_args(argv) + + # A matrix that did not parse fails EVERY mode, including --json. Reporting + # a partial audit as if it were the record is how a parse failure ends up + # wearing the face of a legitimate record edit. + try: + records = audit() + except RecordParseError as exc: + for line in exc.errors: + print(f"ERROR: {line}", file=sys.stderr) + print( + "ERROR: a matrix did not PARSE, so this audit is incomplete and its " + "row set means nothing. Repair the matrix. Do NOT re-pin " + "RUNNABLE_BASELINE off a failed parse.", + file=sys.stderr, + ) + return 1 + # BEFORE --json, which returns 0 whatever the record says. If --json won, + # `--check --json` would be a gate that cannot fail -- the shape this whole + # file exists to detect, wearing this file's own face. + if args.check: + errors = ratchet_errors(records) + for line in errors: + print(f"ERROR: {line}", file=sys.stderr) + return 1 if errors else 0 + if args.json: + print(json.dumps(records, indent=2, sort_keys=True)) + return 0 + counts: dict[str, int] = {} + for item in records: + counts[item["verdict"]] = counts.get(item["verdict"], 0) + 1 + for verdict in ("runnable", "gates-no-command", "no-gates-section", "no-spec"): + print(f" {counts.get(verdict, 0):4d} {verdict}") + print(f"\n{len(records)} gated rows; {counts.get('runnable', 0)} carry a command that can fail.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/check-protocol-consistency.py b/scripts/check-protocol-consistency.py index a36f74a5..49572754 100644 --- a/scripts/check-protocol-consistency.py +++ b/scripts/check-protocol-consistency.py @@ -28,6 +28,23 @@ that has not declared a role, so an agent who is never told the question, or never told that `read-only` is one of the answers, meets a red gate with no instructions -- and a gate people cannot satisfy is a gate people route around. + +The same gate also asserts that `.agents/workflow.md` carries the ORCHESTRATION +LOOP, between `` and its `:end`. The prompts +below are what a sub-agent is handed; the loop is what the OPERATOR does with +them, and until it was written here it lived nowhere an agent reads -- neither +`AGENTS.md` nor this manual said that a reviewer must mutate rather than read, +that the controller runs the row's gate itself instead of taking the +implementer's word, or that findings are never fixed in the controller's own +session. + +The same gate finally asserts that the sub-agent prompts under `.agents/prompts` +exist and still carry their binding instructions. Every Important finding across +two branches of this project came from an INDEPENDENT reviewer sub-agent, none +from an implementer's self-review, and the reviewers found them by MUTATING code +rather than reading diffs: eleven tests passed with the thing they named +deleted, and not one was visible by reading. That instruction is the deliverable, +so it is tracked and pinned phrase by phrase, not merely present as a file. """ from __future__ import annotations @@ -56,6 +73,61 @@ INTERVIEW_MARKER = "" INTERVIEW_REQUIRED = ("claim operator", "claim helper --row", "claim read-only", "--headless") +# The same manual must carry the operator's LOOP. The prompts in PROMPT_REQUIRED +# below are handed to sub-agents; nothing told the operator how to run one, and +# the three rules that carry the whole return are exactly the ones an operator +# improvises away: dispatch a FRESH reviewer whose instruction is to MUTATE, +# run the row's gate YOURSELF rather than believing the author's report, and +# never repair a finding in the coordinating session. +# +# The needles are matched INSIDE the block, not across the whole document. +# workflow.md is a long manual that already discusses gates, prompts and +# "closing the loop", so a whole-file search would keep a loop gutted down to +# its two markers green on unrelated prose -- the same "an unrelated line +# satisfied the assertion" failure the reviewer prompt is written to catch. +LOOP_DOCUMENT = ".agents/workflow.md" +LOOP_MARKER = "" +LOOP_END = "" +LOOP_REQUIRED = ( + "prompts/implementer.md", + "prompts/reviewer.md", + "mutate, not read", + "run the row's gate yourself", + "never fix findings yourself", +) + +# The reviewer prompt's value is the MUTATION instruction; a reviewer told only +# to "review" reads the diff, and reading found none of the eleven tests that +# passed with their subject deleted. Pin the instruction, not the file. +# +# The reviewer needle is the full "mutate, don't read", not a bare "mutate": +# the prompt also says "never mutate the reviewed worktree" further down, so the +# short form would stay satisfied by an unrelated sentence after the binding +# instruction was deleted. That is the same "an unrelated line satisfied the +# assertion" failure the prompt itself is written to catch. +# +# Two needles pin REPAIRS to earlier drafts of these prompts, because a prompt +# that once contradicted itself can drift back: the reviewer prompt used to +# forbid re-running the full suite (which reads as a budget on the mutations it +# demands two sections earlier), and the implementer prompt used to demand a +# green gate with no answer for reds that were already there before the work +# started, whose only exits were stalling or an allowlist. +PROMPT_REQUIRED = { + ".agents/prompts/reviewer.md": ( + "mutate, don't read", + "delete or invert", + "stays green", + "every mutation you make re-runs the suite", + "plan-mandated", + ), + ".agents/prompts/implementer.md": ( + "failing test first", + "mutate every test", + "capture that failing set as a baseline", + "escalate rather than guess", + ), +} + # A path in a table cell, e.g. `docs/STATUS.md`. CELL_PATH = re.compile(r"`([^`]+\.md)`") @@ -141,6 +213,62 @@ def interview_errors(text: str) -> list[str]: ] +def loop_block(text: str) -> str | None: + """Return the orchestration-loop block's body, or None if there isn't one. + + An opening marker with no `:end` is NOT a block. Treating it as "everything + after the marker" would silently widen the scope back to the whole + document, which is the incidental-match hole this scoping exists to close. + """ + start = text.find(LOOP_MARKER) + if start == -1: + return None + end = text.find(LOOP_END, start) + if end == -1: + return None + return text[start + len(LOOP_MARKER) : end] + + +def loop_errors(text: str) -> list[str]: + """The operator's loop must live where agents read it, not in a prompt.""" + block = loop_block(text) + if block is None: + return [ + f"{LOOP_DOCUMENT} is missing the orchestration-loop block " + f"({LOOP_MARKER} ... {LOOP_END}); the sub-agent prompts say what a " + "reviewer or implementer does, and nothing else says what the " + "OPERATOR does with them" + ] + lowered = block.lower() + return [ + f"{LOOP_DOCUMENT} loop omits {needle!r}" + for needle in LOOP_REQUIRED + if needle.lower() not in lowered + ] + + +def prompt_errors(required: dict[str, tuple[str, ...]] | None = None) -> list[str]: + """Each tracked prompt exists and carries its binding instruction.""" + # `required or PROMPT_REQUIRED` would silently promote an explicitly EMPTY + # spec into the full live check, which is this repo's recurring defect + # class: an absence and a value that look the same. Only a missing argument + # means "use the default". + errors: list[str] = [] + spec = PROMPT_REQUIRED if required is None else required + for relative, needles in spec.items(): + path = ROOT / relative + if not path.is_file(): + errors.append(f"{relative} is missing; the prompt is the protocol") + continue + text = path.read_text(encoding="utf-8").lower() + errors.extend( + f"{relative} omits {needle!r}" + for needle in needles + if needle.lower() not in text + ) + return errors + + def main() -> int: expected = obligated_surfaces() failures: list[str] = [] @@ -154,6 +282,17 @@ def main() -> int: interview_errors(interview.read_text(encoding="utf-8")) ) + # INTERVIEW_DOCUMENT and LOOP_DOCUMENT are the same manual today, but the + # two obligations are independent and either may move, so each resolves its + # own path rather than sharing one read. + loop_doc = ROOT / LOOP_DOCUMENT + if not loop_doc.exists(): + failures.append(f"{LOOP_DOCUMENT} does not exist") + else: + failures.extend(loop_errors(loop_doc.read_text(encoding="utf-8"))) + + failures.extend(prompt_errors()) + for name in CONTRACT_DOCUMENTS: path = ROOT / name if not path.exists(): @@ -182,7 +321,13 @@ def main() -> int: "in the contract block of every document listed in " "CONTRACT_DOCUMENTS. The role interview is the block between " f"{INTERVIEW_MARKER} and its :end in {INTERVIEW_DOCUMENT}; it must " - "name every answer agent-role.py accepts.", + "name every answer agent-role.py accepts. The operator's loop is " + f"the block between {LOOP_MARKER} and its :end in {LOOP_DOCUMENT}; " + f"it must carry {', '.join(repr(n) for n in LOOP_REQUIRED)} inside " + "the block. The sub-agent prompts in " + f"{', '.join(PROMPT_REQUIRED)} must carry their binding " + "instructions verbatim; a prompt that lives only in an operator's " + "head is not a protocol.", file=sys.stderr, ) return 1 @@ -190,8 +335,10 @@ def main() -> int: print( "OK: the doc-obligation contract in " f"{' and '.join(CONTRACT_DOCUMENTS)} matches " - f"scripts/check-doc-checkpoint.py, and {INTERVIEW_DOCUMENT} carries the " - "role interview." + f"scripts/check-doc-checkpoint.py, {INTERVIEW_DOCUMENT} carries the " + f"role interview and the orchestration loop, and " + f"{len(PROMPT_REQUIRED)} sub-agent prompts carry their binding " + "instructions." ) return 0 diff --git a/tests/scripts/test_check_gate_commands.py b/tests/scripts/test_check_gate_commands.py new file mode 100644 index 00000000..cd19d269 --- /dev/null +++ b/tests/scripts/test_check_gate_commands.py @@ -0,0 +1,338 @@ +#!/usr/bin/env python3 +"""Unit and mutation checks for scripts/check-gate-commands.py. + +A gate command that cannot fail collapses "done" into the implementer's opinion +of its own work. This classifier's only job is to tell a runnable command from +prose that looks like one. +""" + +from __future__ import annotations + +import contextlib +import importlib.util +import io +import re +import sys +import unittest +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[2] + + +def _load(name: str, relative: str): + path = ROOT / relative + spec = importlib.util.spec_from_file_location(name, path) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + sys.modules[name] = module + spec.loader.exec_module(module) + return module + + +gates = _load("check_gate_commands", "scripts/check-gate-commands.py") + + +class GatesSectionTests(unittest.TestCase): + def test_finds_the_gates_heading_at_any_level(self): + for heading in ("## Gates", "### Gates", "#### Gates and evidence"): + text = f"# Spec\n\nintro\n\n{heading}\n\nrun `ctest -R foo`\n\n## Next\n\ntail\n" + section = gates.gates_section(text) + self.assertIsNotNone(section, heading) + self.assertIn("ctest", section) + self.assertNotIn("tail", section, "must stop at the next heading") + + def test_returns_none_when_there_is_no_gates_section(self): + self.assertIsNone(gates.gates_section("# Spec\n\n## Scope\n\nnothing here\n")) + + def test_is_not_fooled_by_the_word_gates_in_prose(self): + # "the gates are green" is not a section heading. + self.assertIsNone(gates.gates_section("# Spec\n\nAll the gates are green.\n")) + + +class RunnableCommandTests(unittest.TestCase): + def test_recognises_a_real_command(self): + for body in [ + "run `ctest -R test_foo`", + "`python3 scripts/check-agent-record.py`", + "```\nbash scripts/agent-preflight.sh\n```", + "`cmake --build build -j`", + # The repo's MANDATED shape for a gate that touches the GPU. The + # wrapper quotes the real command, so nothing else can reach it. + "`flock /tmp/gpu -c 'ctest -R qwen36_paged_engine'`", + # A built test binary, invoked with no arguments at all. + "`./build-cuda-121a/tests/test_dropin_abi`", + ]: + self.assertTrue(gates.runnable_commands(body), body) + + def test_rejects_prose_that_merely_mentions_gating(self): + for body in [ + "Correctness, e2e and performance gates apply.", + "The SACRED gate must pass on GB10.", + "`docs/BENCHMARKS.md`", + # Every one of these is on the shipped record and was credited as a + # runnable command. A backticked FILENAME is not a command, and a + # tool name must be a whole word: `sha256_cbor` is not `sh`, and + # `python@3.14` is not `python`. + "`tests/vllm/models/test_model_registry.cpp`", + "`tests/`", + "`sha256_cbor`", + "`python@3.14`", + # The GPU lock IDIOM, named in three specs. A wrapper with nothing + # to run is not a gate; a plain `flock` vocabulary entry credits + # all three of those rows with a command they do not have. + "`flock`", + "`flock /tmp/gpu`", + ]: + self.assertEqual(gates.runnable_commands(body), [], body) + # ...without rejecting a path that really is invoked. + self.assertTrue(gates.runnable_commands("`scripts/check-agent-record.py`")) + self.assertTrue(gates.runnable_commands("`./build/vllm-cli --model x`")) + + def test_rejects_a_command_that_cannot_fail(self): + # These are the exact shapes the spec forbids: a Verify that always + # succeeds turns "done" into an opinion. + for body in ["`true`", "`echo ok`", "`:`", "`echo done && true`"]: + self.assertEqual(gates.runnable_commands(body), [], body) + # And they are rejected for the RIGHT reason. Without this, the loop + # above passes vacuously on any implementation that simply fails to + # recognise `true` as a command at all, and the cannot-fail rule -- + # the point of this classifier -- is pinned by nothing. + for candidate in ("true", "echo ok", ":", "echo done && true"): + self.assertTrue(gates.is_command(candidate), candidate) + + def test_rejects_a_piped_command(self): + # `cmd | tail` reports tail's exit status, so the gate cannot fail. + self.assertEqual(gates.runnable_commands("`ctest -R foo | tail -5`"), []) + + +class ShippedRecordTests(unittest.TestCase): + def test_the_audit_covers_every_gated_state(self): + self.assertEqual( + gates.GATED_STATES, + frozenset({"READY", "ACTIVE", "GATING", "DONE", "BLOCKED"}), + ) + # ...and audit() must actually FILTER on it. Asserting the constant's + # literal value pins nothing about the denominator: deleting the state + # filter in audit() leaves every other assertion in this file green + # while the report goes from 97 rows to 726. Task 4 ratchets on that + # number, so it is pinned here. + audited = gates.audit() + self.assertTrue(audited) + self.assertLessEqual({item["state"] for item in audited}, gates.GATED_STATES) + # And the filter is only load-bearing if the matrices really do carry + # rows it excludes -- otherwise the assertion above is vacuous. + on_record = set() + for path in gates.AUDITED_MATRIX_PATHS: + for row in gates.record.parse_claim_rows(path, []): + on_record.add(row.state) + self.assertTrue(on_record - gates.GATED_STATES, "filter excludes nothing") + + def test_the_six_lifecycle_matrices_are_audited(self): + names = {p.name for p in gates.AUDITED_MATRIX_PATHS} + self.assertIn("feature-matrix.md", names) + self.assertEqual(len(names), 6) + # The LIST length too, not just the set of names. If check-agent-record's + # MATRIX_PATHS ever gains the one appended here, audit() parses that file + # twice and double-counts every row in it -- the denominator moving + # silently again, which a set comparison reads as still 6. + self.assertEqual(len(gates.AUDITED_MATRIX_PATHS), 6) + + def test_sglang_is_excluded_and_the_exclusion_is_justified(self): + # It was listed and contributed 0 rows: "audited in name only", an + # absence that reads as a pass (audit artifact risk 6). Dropping it is + # only honest if it genuinely has no gated rows, so pin THAT rather than + # the bare absence. sglang-matrix.md carries a CLASSIFICATION column + # (FUSED / INVENTORIED / NOT-APPLICABLE), not a lifecycle state, so the + # row parser recognises nothing in it -- and reports no error either. + # If it ever gains real lifecycle rows this goes red, and the matrix must + # come back into the audited set rather than stay silently skipped. + names = {p.name for p in gates.AUDITED_MATRIX_PATHS} + self.assertNotIn("sglang-matrix.md", names) + sglang = gates.record.AGENTS / "sglang-matrix.md" + self.assertTrue(sglang.is_file()) + errors: list[str] = [] + self.assertEqual(gates.record.parse_claim_rows(sglang, errors), []) + self.assertEqual(errors, []) + + def test_every_record_carries_a_known_verdict(self): + known = {"runnable", "gates-no-command", "no-gates-section", "no-spec"} + records = gates.audit() + self.assertTrue(records) + for item in records: + self.assertIn(item["verdict"], known) + + def test_a_matrix_that_does_not_parse_is_a_hard_failure(self): + # audit() used to build a fresh `errors` list per matrix, hand it to + # parse_claim_rows and never read it. Stripping rows from a matrix then + # surfaced as "these baseline rows left the gated population ... re-pin + # RUNNABLE_BASELINE" -- a parse FAILURE wearing the face of a legitimate + # record edit, recommending the one action the audit forbids doing + # blindly. Every mode must go red, --json included. + original = gates.record.parse_claim_rows + + def broken(path, errors): + rows = original(path, errors) + errors.append(f"{path.name}:1: SOME-ROW has 4 cells; header has 6") + return rows + + gates.record.parse_claim_rows = broken + try: + with self.assertRaises(gates.RecordParseError): + gates.audit() + noise = io.StringIO() + with contextlib.redirect_stderr(noise), contextlib.redirect_stdout(noise): + statuses = [ + gates.main(argv) + for argv in ([], ["--json"], ["--check"], ["--json", "--check"]) + ] + self.assertEqual(statuses, [1, 1, 1, 1]) + finally: + gates.record.parse_claim_rows = original + + def test_a_parse_failure_never_reads_as_a_legitimate_record_edit(self): + # The message is the finding: a broken matrix must not be reported in + # the words that describe a row correctly leaving the population, and + # must not steer the reader toward re-pinning the baseline. + original = gates.record.parse_claim_rows + + def broken(path, errors): + errors.append(f"{path.name}:1: SOME-ROW must have exactly one canonical state") + return [] + + gates.record.parse_claim_rows = broken + buffer = io.StringIO() + try: + with contextlib.redirect_stderr(buffer): + self.assertEqual(gates.main(["--check"]), 1) + finally: + gates.record.parse_claim_rows = original + message = buffer.getvalue() + self.assertIn("did not PARSE", message) + self.assertIn("must have exactly one canonical state", message) + self.assertNotIn("left the gated population", message) + self.assertNotIn("re-pin RUNNABLE_BASELINE in the", message) + + def test_report_mode_exits_zero_even_with_debt(self): + # 67 of 97 rows cannot state a command today. Report mode must still + # exit 0 -- the ratchet is step 4, after the debt is recorded. + self.assertEqual(gates.main([]), 0) + + +def _bash_array(text: str, name: str) -> list[str]: + """The entries of a `NAME=(\n ... \n)` array in a bash script. + + Membership in the ARRAY, never a substring of the whole file: preflight + mentions `check-gate-commands` twice -- once in `CHECKERS`, once in the + `case` branch that adds `--check` -- so a substring test stays green with + the CHECKERS entry deleted. That is this repo's recorded defect class (a + substring `--grep` crediting a row with another row's commits), and it + would hide the exact mutation step 6 requires to go red. + """ + match = re.search(rf"(?m)^{re.escape(name)}=\($(.*?)^\)$", text, re.S) + assert match is not None, f"{name}=( ... ) not found" + return [line.strip() for line in match.group(1).splitlines() if line.strip()] + + +class RatchetTests(unittest.TestCase): + def test_the_baseline_matches_the_shipped_record(self): + # EXACT equality, in both directions, and that is the whole contract: + # this is an exact pin, not a shrink-only floor. Lowering the baseline + # to make a red gate green goes red here, which is the point -- and so + # does GROWTH. Add a real gate command to a row's spec and `--check` + # stays 0 while this assertion, preflight and CI go red until the set + # below is re-pinned. Growth is welcome; silent growth is not. + runnable = {r["id"] for r in gates.audit() if r["verdict"] == "runnable"} + self.assertEqual(runnable, set(gates.RUNNABLE_BASELINE)) + + def test_a_row_that_loses_its_command_is_refused(self): + # Still present, still gated, no longer runnable -- a real regression. + victim = sorted(gates.RUNNABLE_BASELINE)[0] + records = [{"verdict": "gates-no-command", "id": victim, "state": "READY", + "path": "p", "line": 1, "detail": "d"}] + errors = gates.ratchet_errors(records) + self.assertTrue(errors) + self.assertIn(victim, errors[0]) + self.assertIn("Repair the row", errors[0]) + + def test_a_row_that_left_the_population_reports_differently(self): + # Deleted or transitioned out: legitimate, but must re-pin. The two + # cases MUST be distinguishable -- that is why the baseline is a set. + errors = gates.ratchet_errors([]) + self.assertTrue(errors) + self.assertTrue(any("left the gated population" in e for e in errors)) + self.assertFalse(any("Repair the row" in e for e in errors)) + + def test_a_lost_row_and_a_departed_row_are_reported_separately(self): + # The two cases in ONE run, which is the only arrangement that proves + # they are distinguishable rather than merely differently worded. The + # two tests above each see a single case, so both stay green if the + # `& present` split is deleted and every drop is called a loss; here + # the departed row would then be named in the "Repair the row" message + # and the assertion below goes red. + ordered = sorted(gates.RUNNABLE_BASELINE) + self.assertGreaterEqual(len(ordered), 2) + lost, departed = ordered[0], ordered[1] + records = [{"verdict": "gates-no-command", "id": lost, "state": "READY", + "path": "p", "line": 1, "detail": "d"}] + records += [ + {"verdict": "runnable", "id": rid, "state": "READY", + "path": "p", "line": 2, "detail": "d"} + for rid in ordered[2:] + ] + errors = gates.ratchet_errors(records) + self.assertEqual(len(errors), 2, errors) + loss_msg = [e for e in errors if "Repair the row" in e] + gone_msg = [e for e in errors if "left the gated population" in e] + self.assertEqual(len(loss_msg), 1, errors) + self.assertEqual(len(gone_msg), 1, errors) + self.assertIn(lost, loss_msg[0]) + self.assertNotIn(departed, loss_msg[0]) + self.assertIn(departed, gone_msg[0]) + self.assertNotIn(lost, gone_msg[0]) + + def test_an_improvement_is_allowed(self): + records = [ + {"verdict": "runnable", "id": rid, "state": "READY", + "path": "p", "line": 1, "detail": "d"} + for rid in sorted(gates.RUNNABLE_BASELINE) + ] + [{"verdict": "runnable", "id": "NEW-ROW", "state": "READY", + "path": "p", "line": 2, "detail": "d"}] + self.assertEqual(gates.ratchet_errors(records), []) + + def test_check_mode_passes_on_the_shipped_record(self): + # The gate ships GREEN. It was wired after the debt was recorded, so it + # never had to be relaxed to pass. + self.assertEqual(gates.main(["--check"]), 0) + + def test_check_mode_is_never_silently_swallowed_by_json(self): + # --json used to be the only mode, and it returns 0 unconditionally. If + # it short-circuits ahead of --check, `--json --check` is a gate that + # CANNOT FAIL -- the precise shape this classifier exists to detect, + # wearing this classifier's own face. + self.assertEqual(gates.main(["--json", "--check"]), 0) + original = gates.RUNNABLE_BASELINE + try: + gates.RUNNABLE_BASELINE = frozenset(original | {"ROW-THAT-IS-NOT-THERE"}) + self.assertEqual(gates.main(["--check"]), 1) + self.assertEqual(gates.main(["--json", "--check"]), 1) + finally: + gates.RUNNABLE_BASELINE = original + + def test_the_checker_is_wired_into_preflight_and_ci(self): + preflight = (ROOT / "scripts/agent-preflight.sh").read_text(encoding="utf-8") + self.assertIn("check-gate-commands", _bash_array(preflight, "CHECKERS")) + self.assertIn("test_check_gate_commands", _bash_array(preflight, "SUITES")) + # ...and dispatched WITH --check. Report mode exits 0 whatever the + # record says, so a CHECKERS entry without the flag installs a gate + # that cannot fail. + self.assertRegex( + preflight, r"(?m)^\s*[\w|-]*check-gate-commands\)\s+run\b.*--check" + ) + ci = (ROOT / ".github/workflows/ci.yml").read_text(encoding="utf-8") + self.assertIn("scripts/check-gate-commands.py --check", ci) + self.assertIn("tests/scripts/test_check_gate_commands.py", ci) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/scripts/test_check_protocol_consistency.py b/tests/scripts/test_check_protocol_consistency.py index 3ed5f361..6e75821f 100644 --- a/tests/scripts/test_check_protocol_consistency.py +++ b/tests/scripts/test_check_protocol_consistency.py @@ -14,6 +14,7 @@ import io import re import shutil +import subprocess import sys import tempfile import unittest @@ -38,6 +39,73 @@ def _load(name: str, relative: str): EXPECTED = ("docs/STATUS.md", "docs/BENCHMARKS.md", "docs/FEATURES.md") +def _tracked_paths(prefix: str) -> set[str] | None: + """Paths git knows under `prefix`, or None when this is not a checkout. + + A prompt that exists only in a working tree is precisely the thing this + task exists to stop: an instruction nobody else can read. `git ls-files` + sees staged files too, so it is honest before the commit as well as after. + Exported trees (git archive) have no `.git`, so absence of git is a skip + rather than a failure. + """ + try: + completed = subprocess.run( + ["git", "ls-files", "--", prefix], + cwd=ROOT, + capture_output=True, + text=True, + check=True, + ) + except (OSError, subprocess.CalledProcessError): + return None + return {line for line in completed.stdout.splitlines() if line} + + +@contextlib.contextmanager +def _prompt_tree(files: dict[str, str]): + """Point consistency.ROOT at a temp tree holding exactly `files`.""" + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + for relative, text in files.items(): + path = root / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + saved, consistency.ROOT = consistency.ROOT, root + try: + yield root + finally: + consistency.ROOT = saved + + +@contextlib.contextmanager +def _repo_copy(workflow_text: str, *, prompts: bool = True): + """Run consistency.main() against a copy of the repo's own documents. + + Only `.agents/workflow.md` is substituted, so a red from this helper is + attributable to the manual under test rather than to a hand-built fixture + that never resembled the repository. + """ + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + (root / "scripts").mkdir() + (root / ".agents").mkdir() + shutil.copy( + ROOT / "scripts/check-doc-checkpoint.py", + root / "scripts/check-doc-checkpoint.py", + ) + shutil.copy(ROOT / "AGENTS.md", root / "AGENTS.md") + if prompts: + shutil.copytree(ROOT / ".agents/prompts", root / ".agents/prompts") + (root / ".agents/workflow.md").write_text(workflow_text, encoding="utf-8") + saved, consistency.ROOT = consistency.ROOT, root + out, err = io.StringIO(), io.StringIO() + try: + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + yield lambda: (consistency.main(), out.getvalue(), err.getvalue()) + finally: + consistency.ROOT = saved + + def document(*paths: str) -> str: rows = "\n".join(f"| `{path}` | every checkpoint |" for path in paths) return "\n".join( @@ -164,25 +232,9 @@ class InterviewWiring(unittest.TestCase): ) @contextlib.contextmanager - def _tree(self, workflow_text: str): - """Run consistency.main() against a copy of the repo's own documents.""" - with tempfile.TemporaryDirectory() as tmp: - root = Path(tmp) - (root / "scripts").mkdir() - (root / ".agents").mkdir() - shutil.copy( - ROOT / "scripts/check-doc-checkpoint.py", - root / "scripts/check-doc-checkpoint.py", - ) - shutil.copy(ROOT / "AGENTS.md", root / "AGENTS.md") - (root / ".agents/workflow.md").write_text(workflow_text, encoding="utf-8") - saved, consistency.ROOT = consistency.ROOT, root - out, err = io.StringIO(), io.StringIO() - try: - with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): - yield lambda: (consistency.main(), out.getvalue(), err.getvalue()) - finally: - consistency.ROOT = saved + def _tree(self, workflow_text: str, *, prompts: bool = True): + with _repo_copy(workflow_text, prompts=prompts) as run: + yield run def test_faithful_copy_passes(self): """Positive control: the temp tree itself is not what fails below.""" @@ -202,6 +254,321 @@ def test_main_fails_when_the_interview_is_deleted(self): self.assertEqual(code, 1) self.assertIn("role-interview", err) + def test_main_fails_when_the_prompts_are_missing(self): + """main() must CALL prompt_errors, not merely define it. + + Every prompt assertion above calls the function directly, so a main() + that never wires it in leaves them all green while the gate enforces + nothing -- the same drift, one function later. + """ + text = (ROOT / ".agents/workflow.md").read_text(encoding="utf-8") + with self._tree(text, prompts=False) as run: + code, _, err = run() + self.assertEqual(code, 1, err) + self.assertIn(".agents/prompts/reviewer.md", err) + + +class PromptArtifactTests(unittest.TestCase): + def test_both_prompts_exist_and_are_tracked(self): + tracked = _tracked_paths(".agents/prompts") + for name in ("reviewer.md", "implementer.md"): + path = ROOT / ".agents/prompts" / name + self.assertTrue(path.is_file(), f"{name} must exist") + # A silent downgrade to existence-only is the failure/absence + # confusion again, so say so in the run output rather than passing + # quietly with half the assertion skipped. + with self.subTest(tracked=name): + if tracked is None: + self.skipTest("git unavailable: tracking not verifiable here") + self.assertIn( + f".agents/prompts/{name}", + tracked, + f"{name} exists but is untracked; an operator-local prompt " + "is not a protocol", + ) + + def test_the_reviewer_prompt_carries_the_mutation_instruction(self): + # The instruction IS the deliverable. A reviewer told only to "review" + # reads the diff, and reading found none of the eleven tests that + # passed with their subject deleted. + text = (ROOT / ".agents/prompts/reviewer.md").read_text(encoding="utf-8") + for needle in ("mutate", "delete or invert", "stays green"): + self.assertIn(needle, text.lower(), needle) + + def test_the_reviewer_prompt_refuses_to_defer_to_the_plan(self): + text = (ROOT / ".agents/prompts/reviewer.md").read_text(encoding="utf-8") + self.assertIn("plan-mandated", text.lower()) + + def test_checker_rejects_a_prompt_missing_its_instruction(self): + # A missing FILE and a present file missing its INSTRUCTION are two + # different failures. Asserting only the first would leave the needle + # loop -- the part that carries the value -- entirely unpinned. + errors = consistency.prompt_errors({"nonexistent-prompt.md": ("mutate",)}) + self.assertTrue(errors) + self.assertTrue(any("missing" in e for e in errors), errors) + + present = ".agents/prompts/reviewer.md" + self.assertEqual(consistency.prompt_errors({present: ("mutate",)}), []) + omitted = consistency.prompt_errors( + {present: ("no reviewer prompt would ever contain this phrase",)} + ) + self.assertTrue(any("omits" in e for e in omitted), omitted) + + def test_an_explicitly_empty_spec_checks_nothing(self): + # An empty spec must mean "nothing required", not silently fall back to + # the live PROMPT_REQUIRED: an absence and a value that look the same is + # the defect class the implementer prompt names. + with _prompt_tree({}): + self.assertEqual(consistency.prompt_errors({}), []) + self.assertTrue(consistency.prompt_errors()) + + def test_the_checker_enforces_the_phrases_these_tests_demand(self): + # Every assertion above reads the prompt FILES, so emptying, narrowing + # or widening a PROMPT_REQUIRED tuple would leave them all green while + # the gate quietly stopped enforcing what this suite believes it does. + # + # The comparison is EQUALITY, deliberately, not "demanded is a substring + # of enforced". That substring idiom is borrowed from + # test_every_declarable_role_is_named_in_the_interview, where it is safe + # because the demanded side is DERIVED from role.DECLARABLE. Here both + # sides are hand-written literals, and a substring test cannot see the + # one narrowing that matters: reverting the reviewer needle from + # "mutate, don't read" to a bare "mutate" satisfies it while re-opening + # the incidental-match hole check-protocol-consistency.py spends five + # lines arguing is dangerous. Equality means changing what the gate + # enforces is a deliberate two-file act. + demanded = { + ".agents/prompts/reviewer.md": ( + "mutate, don't read", + "delete or invert", + "stays green", + "every mutation you make re-runs the suite", + "plan-mandated", + ), + ".agents/prompts/implementer.md": ( + "failing test first", + "mutate every test", + "capture that failing set as a baseline", + "escalate rather than guess", + ), + } + self.assertEqual( + set(demanded), + set(consistency.PROMPT_REQUIRED), + "PROMPT_REQUIRED covers a different set of prompts than this suite", + ) + for relative, needles in demanded.items(): + with self.subTest(prompt=relative): + self.assertEqual( + set(consistency.PROMPT_REQUIRED[relative]), + set(needles), + f"PROMPT_REQUIRED[{relative!r}] no longer enforces exactly " + "the phrases this suite demands; narrowing one is how the " + "gate stops catching what it was built for", + ) + + def test_a_bare_mutate_needle_would_not_pin_the_binding_instruction(self): + # The executable justification for the full "mutate, don't read" needle. + # Deleting the ENTIRE binding-instruction section still leaves the word + # "mutate" in the file ("Never mutate the reviewed worktree" under What + # you may not do), so a bare needle stays green through the exact + # deletion it exists to catch. If this test ever goes red because the + # incidental match is gone, the needle may safely be simplified. + relative = ".agents/prompts/reviewer.md" + text = (ROOT / relative).read_text(encoding="utf-8") + without_section = re.sub( + r"## The binding instruction.*?(?=\n## )", "", text, flags=re.S + ) + self.assertNotEqual(without_section, text, "the strip pattern matched nothing") + with _prompt_tree({relative: without_section}): + self.assertEqual( + consistency.prompt_errors({relative: ("mutate",)}), + [], + "a bare 'mutate' no longer matches incidentally", + ) + self.assertTrue( + consistency.prompt_errors({relative: ("mutate, don't read",)}), + "the shipped needle failed to catch the section deletion", + ) + + def test_each_required_phrase_is_pinned_individually(self): + # PROMPT_REQUIRED is a hand-written tuple, so a prompt that survives + # losing one of its phrases means that phrase was never enforced. Strip + # each one in turn from a copy of the real file and demand a red. + for relative, needles in consistency.PROMPT_REQUIRED.items(): + text = (ROOT / relative).read_text(encoding="utf-8") + for needle in needles: + with self.subTest(prompt=relative, needle=needle): + damaged = re.sub(re.escape(needle), "", text, flags=re.I) + self.assertNotEqual( + damaged, text, f"{needle!r} does not appear in {relative}" + ) + with _prompt_tree({relative: damaged}): + errors = consistency.prompt_errors({relative: needles}) + self.assertTrue(any("omits" in e for e in errors), errors) + + +class OrchestrationLoopTests(unittest.TestCase): + """The operator's loop must live in the manual agents actually read. + + A loop that exists only in an operator's head, or only in a sub-agent + prompt the operator never opens, is not a protocol: nothing tells the next + session that a reviewer must MUTATE, that the gate is run by the controller + rather than reported by the author, or that findings are never fixed in the + controller's own context. + """ + + def _manual(self) -> str: + return (ROOT / consistency.LOOP_DOCUMENT).read_text(encoding="utf-8") + + def test_workflow_carries_the_loop_exactly_once(self): + # Uniqueness, not mere presence. A duplicated block would make every + # deletion mutation below silently invalid, because removing one copy + # leaves the other behind and the gate stays green for the wrong reason. + text = self._manual() + self.assertEqual(text.count(consistency.LOOP_MARKER), 1) + self.assertEqual(text.count(consistency.LOOP_END), 1) + self.assertLess( + text.index(consistency.LOOP_MARKER), text.index(consistency.LOOP_END) + ) + + def test_the_loop_states_the_rules_that_carry_it(self): + block = consistency.loop_block(self._manual()) + self.assertIsNotNone(block, "the manual has no orchestration-loop block") + lowered = block.lower() + # A bare "reviewer" needle would be UNFALSIFIABLE here: the block links + # `prompts/reviewer.md`, so the word is present no matter what the loop + # says. The reviewer's INDEPENDENCE is the assertion worth making, and + # it is the one rule below that LOOP_REQUIRED does not also pin. + for needle in ( + "never the agent that wrote the code", + "mutate, not read", + "run the row's gate yourself", + "never fix findings yourself", + ): + with self.subTest(needle=needle): + self.assertIn(needle, lowered) + + def test_the_loop_links_both_tracked_prompts(self): + block = consistency.loop_block(self._manual()) + for name in ("implementer.md", "reviewer.md"): + with self.subTest(prompt=name): + self.assertIn(f"prompts/{name}", block) + # The link is relative to `.agents/`, so a link that reads + # perfectly can still resolve to nothing. + self.assertTrue( + (ROOT / ".agents/prompts" / name).is_file(), + f"the loop links prompts/{name}, which does not exist", + ) + + def test_the_real_manual_satisfies_the_gate(self): + # Positive control: every red below is the mutation, not the baseline. + self.assertEqual(consistency.loop_errors(self._manual()), []) + + def test_checker_rejects_a_workflow_without_the_loop(self): + errors = consistency.loop_errors("# workflow\n\nno loop here\n") + self.assertTrue(errors) + self.assertTrue(any("orchestration-loop" in e for e in errors), errors) + + def test_an_unterminated_block_is_rejected(self): + # An opening marker with no `:end` is not a block. Without this the + # scoping below could be satisfied by "everything after the marker". + errors = consistency.loop_errors( + f"# workflow\n{consistency.LOOP_MARKER}\n" + + "\n".join(consistency.LOOP_REQUIRED) + + "\n" + ) + self.assertTrue(errors) + + def test_each_required_phrase_is_pinned_individually(self): + # LOOP_REQUIRED is a hand-written tuple, so a manual that survives + # losing one of its phrases means that phrase was never enforced. + text = self._manual() + for needle in consistency.LOOP_REQUIRED: + with self.subTest(needle=needle): + damaged = re.sub(re.escape(needle), "", text, flags=re.I) + self.assertNotEqual( + damaged, text, f"{needle!r} does not appear in the manual" + ) + self.assertTrue(consistency.loop_errors(damaged), needle) + + def test_the_needles_must_be_INSIDE_the_block(self): + # Executable justification for scoping loop_errors to the block rather + # than searching the whole file. `.agents/workflow.md` is a long manual + # that already talks about gates, prompts and closing loops; a + # whole-file search would keep a loop gutted down to its two markers + # green on unrelated prose that happens to carry the phrases. That is + # the "an unrelated line satisfied the assertion" failure this project + # has now paid for repeatedly. + gutted = "\n".join( + [consistency.LOOP_MARKER, consistency.LOOP_END, *consistency.LOOP_REQUIRED] + ) + self.assertTrue(consistency.loop_errors(gutted)) + + def test_the_checker_enforces_the_phrases_this_suite_demands(self): + # Every assertion above reads the MANUAL, so emptying or narrowing + # LOOP_REQUIRED would leave them all green while the gate quietly + # stopped looking. Equality, deliberately, not containment: a narrowing + # (say, back to a bare "mutate") is exactly the failure to catch, and + # containment cannot see it. + self.assertEqual( + set(consistency.LOOP_REQUIRED), + { + "prompts/implementer.md", + "prompts/reviewer.md", + "mutate, not read", + "run the row's gate yourself", + "never fix findings yourself", + }, + "LOOP_REQUIRED no longer enforces exactly the phrases this suite " + "demands; narrowing one is how the gate stops catching what it was " + "built for", + ) + + +class OrchestrationLoopWiring(unittest.TestCase): + """main() must CALL loop_errors, not merely define it. + + Every assertion in OrchestrationLoopTests exercises the function directly, + so a main() that never wires it in leaves them all green while the gate + enforces nothing -- the same drift this file exists to catch, one function + later. InterviewWiring.test_faithful_copy_passes is the positive control + for the temp tree these two tests run in. + """ + + STRIP = re.compile( + r".*?\n?", re.S + ) + + def test_main_fails_when_the_loop_is_deleted(self): + text = (ROOT / consistency.LOOP_DOCUMENT).read_text(encoding="utf-8") + stripped = self.STRIP.sub("", text) + self.assertNotEqual(stripped, text, "the strip pattern matched nothing") + self.assertNotIn(consistency.LOOP_MARKER, stripped) + # The interview must SURVIVE the strip: otherwise a red here would be + # interview_errors firing and would prove nothing about the loop. + self.assertEqual(consistency.interview_errors(stripped), []) + with _repo_copy(stripped) as run: + code, _, err = run() + self.assertEqual(code, 1) + self.assertIn("missing the orchestration-loop block", err) + + def test_main_fails_when_the_loop_loses_one_phrase(self): + # The marker check and the needle loop are two different wirings. A + # main() that only saw the marker would pass the test above and let a + # block drift into saying nothing binding. + text = (ROOT / consistency.LOOP_DOCUMENT).read_text(encoding="utf-8") + needle = "run the row's gate yourself" + self.assertEqual( + text.lower().count(needle), 1, f"{needle!r} is not a unique anchor" + ) + damaged = re.sub(re.escape(needle), "", text, flags=re.I) + self.assertIn(consistency.LOOP_MARKER, damaged) + with _repo_copy(damaged) as run: + code, _, err = run() + self.assertEqual(code, 1) + self.assertIn("loop omits", err) + if __name__ == "__main__": unittest.main()