From 8e7ae7199c178b2d2f89f7a15f4bc595b10b1699 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 2 Sep 2026 14:17:41 +0000 Subject: [PATCH 01/16] revert(hook): a declared row was deleted against a world that rewrites itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `hooks-wiring-check` was red in every session in this environment, which is the pre-commit gate refusing every commit on a condition no commit in this repository can reach. Two independent causes were found. ONE OF THEM IS NO LONGER THIS COMMIT'S, and saying so is the point of this paragraph rather than a footnote. WHAT THIS COMMIT NOW CARRIES is the second cause only: CLOUD-1314's deletion of both merged `declared` rows, on the premise that "both registrations are gone and both programs are deleted". They are not gone. Measured 2026-09-02: `~/.claude/stop-hook-git-check.sh` and `session-start-git-identity.sh` both present, mtime 04:22 — three hours AFTER the commit that deleted their rows at 01:18 — and `launcher-settings.json` registering both. The rows are back, naming CLOUD-1079, which is the row `batten.toml` already identifies as owning the environment half. WHAT IT NO LONGER CARRIES is the withdrawal of `[hook] exclusive = true`. This commit removed it, on a measurement: the note read "`siblings: 0` and `merged_siblings: 0`", and measured on the declaring commit it was `merged_siblings: 2`. `origin/main` reached the same withdrawal independently, under CLOUD-1324, with a fuller note — so the rebase left this half with nothing to apply and the `batten.toml` hunk is gone. The `Admits:` block that rode with it is gone too, which is the gate working rather than a tidy-up: an admission naming a write this commit does not make is exactly the declared-but-unreachable class the branch spent the session closing. AND MAIN'S NOTE NEEDS THIS COMMIT TO BE TRUE. It says "The two rows are therefore back in `policy/harness-declared.json` naming CLOUD-1314"; on `origin/main` that file is `{}`. The restoration below is what makes the landed sentence describe the landed tree. ITS ACCEPTANCE CASE IS WITHDRAWN RATHER THAN DELETED, because a case removed is indistinguishable from a case that never held. `test_the_retired_launcher_hooks_ are_refused_if_they_return` asserted a property the world does not have; `test_the_launcher_hooks_are_declared_rather_than_refused` asserts the one it does, and `test_an_undeclared_hook_beside_the_declared_pair_is_still_refused` is the direction that stops the withdrawal being satisfied by a module which simply stopped refusing on merged surfaces. `spent` and `stale` still watch both rows, so they leave when the launcher stops writing them and not before. WHAT IS NOT DONE HERE, and why. A per-surface `exclusive` — declarable on the committed side while the `$HOME` pair stays a count — is what `policy/harness-wiring.rego`'s own note recommends and is strictly better than waiting for a global zero. It narrows a landed refusal, so it owes a groomed `Weakens:` clause and a row of its own rather than riding in on a branch about something else. CLOUD-1160's shape; CLOUD-1079's environment half. `policy test`: 45 bundles, 571 passed, 0 failed. `hooks-wiring-check`: 5 harnesses, 0 unwired, 0 findings. Refs: CLOUD-1314, CLOUD-1079 --- policy/harness-declared.json | 5 ++- policy/harness-wiring.rego | 67 +++++++++++++++++++++++++++--------- 2 files changed, 54 insertions(+), 18 deletions(-) diff --git a/policy/harness-declared.json b/policy/harness-declared.json index 0967ef424..322564926 100644 --- a/policy/harness-declared.json +++ b/policy/harness-declared.json @@ -1 +1,4 @@ -{} +{ + "stop-hook-git-check.sh": "CLOUD-1079", + "session-start-git-identity.sh": "CLOUD-1079" +} diff --git a/policy/harness-wiring.rego b/policy/harness-wiring.rego index 84b625dc8..dec4e2b46 100644 --- a/policy/harness-wiring.rego +++ b/policy/harness-wiring.rego @@ -105,13 +105,22 @@ mediator := "batten" # §2 refuses merging two units because that "would manufacture the glue this # partition exists to avoid" -- so this row points at that unit rather than being # discharged here. -# THE TWO MERGED ROWS ARE GONE BECAUSE THEIR SUBJECTS ARE (CLOUD-1314). Both -# launcher hooks were removed from `~/.claude/launcher-settings.json` and both -# programs deleted, so the rows would now match nothing and `stale` would refuse -# them -- which is that direction working, and the reason a retirement deletes its -# own row rather than leaving a licence behind. Nothing on a merged surface is -# declared any more: a command there that is not the mediator is a stray, and -# `hook wire duplicate` refuses their return, which is CLOUD-1314's acceptance. +# THE TWO MERGED ROWS ARE BACK, BECAUSE THEIR SUBJECTS NEVER LEFT (CLOUD-1079). +# CLOUD-1314 removed them on the premise that both launcher hooks were gone from +# `~/.claude/launcher-settings.json` and both programs deleted, so the rows would +# match nothing and `stale` would refuse them. +# +# The premise does not survive a session boundary, and `batten.toml` says so at its +# own site: these two are "launcher-provisioned and rewritten at every session +# start, so no commit here can clear them". Measured 2026-09-02 — both files +# present with mtime 04:22, three hours AFTER the commit that deleted their rows at +# 01:18, and the launcher registering both again. +# +# What the deletion bought was therefore not a closed exemption but `harness-wiring` +# red in every session, on a condition no commit in this repository can reach. The +# rows name CLOUD-1079, which is the row `batten.toml` already identifies as owning +# the environment half; `spent` and `stale` still watch them, so they leave when the +# launcher stops writing them and not before. # THE TABLE IS A DOCUMENT, NOT A CONSTANT, and that is what keeps its three # directions testable once it is EMPTY -- which is the campaign's goal state and is # where this repository now is. @@ -554,10 +563,9 @@ test_a_declaration_matching_something_is_not_stale if { # --- the merged half ----------------------------------------------------------- -# The two commands CLOUD-1314 removed. Kept as a fixture rather than deleted with -# their rows, because "these exact programs are refused if they come back" is the -# acceptance clause that row names, and it needs the names to assert it. -retired_hooks := { +# The two launcher-provisioned commands, and they are a fixture because they are +# STILL HERE rather than as a memorial to their removal. +launcher_hooks := { "~/.claude/stop-hook-git-check.sh", "~/.claude/session-start-git-identity.sh", } @@ -567,12 +575,37 @@ test_a_merged_registration_the_table_does_not_declare_is_refused if { v.verdict == "hook wire duplicate" } -# CLOUD-1314'S ACCEPTANCE. The rows are gone because the subjects are, so these two -# are no longer excused by anything -- re-provisioning either is refused like any -# other second decider. Without this case the deletion would be indistinguishable -# from having quietly stopped watching them. -test_the_retired_launcher_hooks_are_refused_if_they_return if { - vs := verdicts with input as launcher(retired_hooks) +# CLOUD-1314'S ACCEPTANCE IS WITHDRAWN, ON A MEASUREMENT RATHER THAN AN ARGUMENT. +# It read: the rows are gone because the subjects are, so re-provisioning either is +# refused like any other second decider — and its case asserted exactly that. +# +# The subjects are not gone. `~/.claude/launcher-settings.json` registers both and +# the launcher REWRITES them at every session start, which `batten.toml` states at +# its own site: "launcher-provisioned and rewritten at every session start, so no +# commit here can clear them". Measured 2026-09-02: both files present, mtime +# 04:22, three hours AFTER the commit that deleted their rows at 01:18. +# +# So the case was asserting a property of the world that the world does not have, +# and what it bought was `harness-wiring` red in every session — the pre-commit gate +# refusing every commit, on a condition no commit can reach. The rows are back, +# keyed to CLOUD-1079, which is the row `batten.toml` already names as owning the +# environment half. They leave when the launcher stops writing them, not before. +test_the_launcher_hooks_are_declared_rather_than_refused if { + vs := verdicts with input as launcher_with(launcher_hooks, { + "stop-hook-git-check.sh": "CLOUD-1079", + "session-start-git-identity.sh": "CLOUD-1079", + }) + not "hook wire duplicate" in vs +} + +# THE OTHER DIRECTION, and without it the case above is satisfied by a module that +# stopped refusing anything on a merged surface — which is what withdrawing an +# acceptance clause has to be held to. +test_an_undeclared_hook_beside_the_declared_pair_is_still_refused if { + vs := verdicts with input as launcher_with( + {"~/.claude/stop-hook-git-check.sh", "~/.claude/other-hook.sh"}, + {"stop-hook-git-check.sh": "CLOUD-1079"}, + ) "hook wire duplicate" in vs } From ce169d337563aa8da40b1944741c90899b9de871 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 2 Sep 2026 14:18:39 +0000 Subject: [PATCH 02/16] feat(facts)!: a vendored prompt's dispatch becomes a fact the tree can read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-472. `Fact::Review` is the second occupant of `Cost::Effect` and the third adopter of the delegated-analyser shape: `secrets.rs` (ripsecrets), then `symbols.rs` (clippy), now this. What carries across is a declared program, flags pinned beside the parser, provenance recorded because a fact whose meaning depends on an unrecorded tool version is not canonical, and one invariant verbatim — clean is never inferred from a stream that failed to parse. WHAT IT ANSWERS IS DELIBERATELY NARROW: that a particular prompt ran over these exact bytes. Not whether the review was good and not whether its findings are real, because those are judgements and non-negotiable rule 3 forbids a gate deciding one. A gate over this fact refuses ABSENCE, which is a comparison of two digests, and the agent's findings reach a module as pointers that carry no claim to weigh. That bound is what makes an LLM in a resolution path legal, and the schema is shaped so it holds structurally: there is no string field an agent's prose could occupy. WHY THE CHEAPER TIERS DO NOT SUBSTITUTE. `ready-lint` gates the SHAPE of a refinement block, and shape is what an author optimises against once the gate exists — the measured failure that opened this row, where every clause was present and none had been pressure-tested. `obligations-bound` binds a §7 entry to a killer mutation but only at implementation time; at refinement there is no code, no case file and no `#MUTANT` row for it to reach. Confirming a named prompt ran over these bytes is a hash comparison no better-shaped prose can satisfy, because the prose is the input to the hash. SPAWN ON MISS, READ ON HIT — `step-receipt`'s pattern rather than a new one. A review costs minutes and tokens and `check` runs every landing lap, so an unconditional resolve would be unaffordable and the gate would be switched off. The record is keyed by (prompt digest, subject digest), so the agent runs once per unique subject and every later lap is a cache hit. The keying is the anti-staleness property rather than an optimisation: edit the ticket body or push a commit and the record lives under a name nothing looks up. THE DISPATCH IS THE ENGINE'S, and that is the whole difference from a producer-writes-outside store. `tool-verdict` has the identical read shape and is measured dead — `validator-verdict-clean` reads a record nothing ever writes, so the predicate resolves `null` on every real checkout (CLOUD-1265) — because somebody has to remember to run the tool and pipe its output. A fact the engine resolves has no such gap, and needs no spelling per harness. THE PROMPT IS VENDORED, compiled in the way `policy/presets/**` are, so its digest is a constant of the build. That is what makes "a particular prompt" a checkable claim: a consumer cannot satisfy the gate by pointing it at an easier one, because the digest in the key is not theirs to choose. Every failure path leaves NO record — runner missing, non-zero exit, unparseable stream — so a broken agent is indistinguishable from one that never ran and both refuse. `pointers_in` rejects the whole stream rather than filtering it, because a parser that skipped what it did not understand would store whichever lines looked like pointers and call the rest absent. `ReviewQuery` lives on the rule row rather than in a top-level table, for `ToolQuery`'s reason: the declaration and the rule that reads it are one object, so a row cannot name a review nothing enables. Not yet landed, and named here so the gap is visible rather than implied: the consumer-side rego module that refuses absence, the two test tiers, and a `[[rule]]` row that enables any of it. This commit lands the fact and its resolution; nothing reads them yet. Refs: CLOUD-760, CLOUD-1171, CLOUD-1265, CLOUD-418 BREAKING CHANGE: `Fact` gains a `Review` variant, `Resolved` gains a `review` field, and `ReviewQuery` is a new constructible struct. `semver check` names the three lints — `enum_variant_added`, `enum_no_repr_variant_discriminant_changed` and `constructible_struct_adds_field` — and it is right about all three: a downstream exhaustive `match` on `Fact` stops compiling, and so does a struct literal for `Resolved`. Declared here rather than on a later commit because this is where the variant and the field arrive; the `probe` and `prompt_arg` fields a later commit adds to `ReviewQuery` are the same break widening, not a second one. Below 0.1.0 release-plz bumps the patch whatever the type says, so the `!` buys a reader the notice rather than a different version. Admits: e2396335346ddac886cb3b7db31a58250f0592684ec8222f28792c5e9032d4ba Admits-rule: protected-mutation Admits-verdict: path write refused Admits-subject: .serena/memories/core.md Admits-head: 9f35e4166f331a6121a429e4a85c2fac514793b3 Admits-epoch: a1a148a28e969c732dd6bf887133eecb19230d317fefb66fb2b847035606d3b9 Admits-author: alec@wenzowski.com Admits-prev: 4ec572594ee30d8023fbc708c2772377d39a85bea5105083bef41de7b83ec75b Admits-answer-lost: `module-map-check` refuses the commit outright: a new module with no map row is a gate failure, not a style note. The alternative is landing `review.rs` with the map silently stale, which is the drift the map exists to prevent. Admits-answer-precondition: `.serena/memories/core.md` carries the per-module map, and `module-map-check` fails a `crates/batten/src/*.rs` with no row. This commit adds `review.rs`, so the row is obligatory and there is no surface that writes it — the map is prose in a memory, and no verb appends to it. It lands in the diff a reviewer reads, beside the module it describes. Admits-answer-rejected-route: `config read first` does not apply — the map is a memory, not config, and reading it is how the missing row was identified. `patch run first` has nothing to patch: no verb writes a module-map row, which is why `module-map-check` is a gate over prose rather than a generator. --- .claude/rules/policy-modules.md | 15 +- .serena/memories/core.md | 29 ++ crates/batten/src/config.rs | 1 + crates/batten/src/facts.rs | 177 +++++++++ crates/batten/src/hook.rs | 5 + crates/batten/src/lib.rs | 2 + crates/batten/src/review.rs | 354 ++++++++++++++++++ .../batten/src/review/ready-pressure-test.md | 15 + crates/batten/src/rules.rs | 85 +++++ crates/batten/tests/it/facts.rs | 4 +- schema/batten.local.schema.json | 53 +++ schema/batten.schema.json | 53 +++ schema/policy-input.schema.json | 64 ++++ 13 files changed, 855 insertions(+), 2 deletions(-) create mode 100644 crates/batten/src/review.rs create mode 100644 crates/batten/src/review/ready-pressure-test.md diff --git a/.claude/rules/policy-modules.md b/.claude/rules/policy-modules.md index 589f66c47..2fe03d2ad 100644 --- a/.claude/rules/policy-modules.md +++ b/.claude/rules/policy-modules.md @@ -187,7 +187,20 @@ by content, carries no clock, and would answer a question about a mutable field from whichever read sorts first in digest order (CLOUD-1310); and `input.tree.captured` is a declared REDUCTION over the capture store — `present`, `count` or a bounded token, never a payload -(CLOUD-1188). +(CLOUD-1188); and `input.tree.review` is whether a VENDORED agent prompt was +dispatched over a declared subject, keyed by (prompt digest, subject digest) so +editing the subject leaves the record under a name nothing looks up (CLOUD-472). + +**`review` is the one key whose ARM a module must get right rather than merely +its spelling, so it is worth the extra sentence.** A declared id ABSENT from the +map was never dispatched, and that absence is the ONLY thing a predicate over it +may refuse on. Its `findings` are pointers — `{path, line, clause}`, with no +field an agent's prose could occupy — and a module refusing on what the agent +CONCLUDED would be a model verdict wearing an exit code, which non-negotiable +rule 3 forbids. `forge-verdict-required` refuses the opposite arm for a reason +that does not carry: the forge is a third party that may legitimately not have +judged yet, where a review this branch was supposed to dispatch and did not is +the branch's own conduct. A **mediated-call** module (`scope = "mediated_call"`, run by `batten hook`) reads `input.call.command`, `input.call.segments`, `input.call.programs`, diff --git a/.serena/memories/core.md b/.serena/memories/core.md index 86f2e96b2..a11c5a315 100644 --- a/.serena/memories/core.md +++ b/.serena/memories/core.md @@ -1145,6 +1145,35 @@ transcript CONTENT needs 1029 first, and nothing landed authorises one. row would override four correct per-verb answers with one weaker sentence. Per-path beats per-verb only where the path fact dominates. It makes a message specific; it does not make the named surface reachable (CLOUD-663). +- `review.rs` — the vendored-prompt dispatch tier (CLOUD-472): the SECOND + occupant of `Cost::Effect` and the third adopter of `secrets.rs`' + delegated-analyser shape, after `symbols.rs`. It answers one narrow question — + **did a particular vendored prompt run over these exact bytes** — and that + narrowness is the mechanism rather than a limitation: a gate over it refuses + ABSENCE, which is a comparison of two digests, so no model verdict reaches an + exit code (rule 3). The findings reach a module as `{path, line, clause}` + pointers with no field an agent's prose could occupy, so rule 4 holds + structurally rather than by the parser remembering to strip. + **It is what the cheaper tiers cannot be.** `ready-lint` gates the SHAPE of a + refinement block, and shape is what an author optimises against once the gate + exists — the measured failure that opened CLOUD-472, where every clause was + present and none had been pressure-tested. `obligations-bound` binds a §7 entry + to a killer mutation, but only at implementation time; at refinement there is + no code, no case file and no `#MUTANT` row to reach. A hash comparison is what + better-shaped prose cannot satisfy, because the prose is the input to the hash. + **Spawn on miss, read on hit**, keyed by (prompt digest, subject digest) — + `step-receipt`'s pattern, so the agent runs once per unique subject rather than + once per landing lap, and editing the ticket body or pushing a commit leaves + the record under a name nothing looks up. The keying is the anti-staleness + property, not an optimisation. + **The dispatch is the ENGINE's**, which is the whole difference from + `tool-verdict`'s producer-writes-outside store — identical read shape, measured + dead, because somebody has to remember to run the tool and pipe its output + (CLOUD-1265). The prompt is compiled in the way `policy/presets/**` are, so its + digest is a constant of the build and a consumer cannot satisfy the gate by + pointing it at an easier prompt. Every failure path — runner missing, non-zero + exit, unparseable stream — leaves NO record, so a broken agent and one that + never ran are indistinguishable and both refuse. - `refusal.rs` — the refusal contract (CLOUD-122): ONE `Refusal` value — `{rule, reason, fix}` — constructed at every deny site and projected onto whatever channel a host reads, so the shape is never re-typed per harness. diff --git a/crates/batten/src/config.rs b/crates/batten/src/config.rs index eebc5adb7..3c74f4a79 100644 --- a/crates/batten/src/config.rs +++ b/crates/batten/src/config.rs @@ -1718,6 +1718,7 @@ fn default_rules() -> Vec { extract: Vec::new(), landing: Vec::new(), symbols: false, + review: Vec::new(), delta_sources: Vec::new(), run: None, verbatim: None, diff --git a/crates/batten/src/facts.rs b/crates/batten/src/facts.rs index a12be3856..16821f922 100644 --- a/crates/batten/src/facts.rs +++ b/crates/batten/src/facts.rs @@ -669,6 +669,16 @@ pub enum Fact { /// Where the crate uses a type a delegated analyser resolved by NAME, rather /// than by spelling (CLOUD-760). The first `Cost::Effect` fact. Symbols, + /// That a **vendored** agent prompt was dispatched over a **declared** + /// subject, and what it pointed at (CLOUD-472). + /// + /// The second `Cost::Effect` fact, and the third adopter of `secrets.rs`' + /// delegated-analyser shape. What it answers is deliberately narrow: THAT a + /// particular prompt ran over these exact bytes. What the agent concluded + /// reaches the input as pointers and never as prose, and no gate may decide + /// on it — see [`REVIEW`] for why that bound is what makes an LLM in the + /// resolution path compatible with non-negotiable rule 3. + Review, /// How the **declared** globs' paths differ from a **declared** base rev: /// added, edited, deleted (CLOUD-1059). BaseDelta, @@ -1233,6 +1243,66 @@ pub const LANDING: Class = Class::new(Cost::Read, Surface::Check); /// SECOND class of the same fact is buildable later, and would be a different /// `Class` rather than a quiet reinterpretation of this one. pub const SYMBOLS: Class = Class::new(Cost::Effect, Surface::Check); + +/// [`Fact::Review`] — **the second occupant of [`Cost::Effect`]**, and the one +/// that had to state why an LLM in the resolution path is not a model verdict in +/// a gate. +/// +/// `effect` x `check`, both halves for [`SYMBOLS`]' reasons: resolving it RUNS A +/// PROGRAM, and `run_static` already refuses a spawning kind on the mediated +/// path, so a fact resolvable there would weaken a structural guarantee into a +/// convention. +/// +/// # What it answers, and the narrowness IS the mechanism +/// +/// **THAT a vendored prompt ran over these exact bytes.** Not whether the review +/// was good, not whether its findings are real, not whether the subject is ready. +/// Those are judgements and non-negotiable rule 3 forbids a gate deciding any of +/// them. +/// +/// The distinction is what makes this fact legal at all. A gate reading this +/// fact refuses **ABSENCE** — no record for this (prompt, subject) pair — which +/// is a comparison of two digests and is as deterministic as any other fact +/// here. A gate refusing on what the agent CONCLUDED would be a model verdict +/// wearing an exit code, and `review-dispatched.rego` is written so it cannot +/// express one: the findings reach the module as pointers, and a pointer carries +/// no claim to weigh. +/// +/// **This is why the cheaper tiers do not substitute.** `ready-lint` gates the +/// SHAPE of a refinement block, and shape is what an author optimises against +/// once the gate exists — the measured failure that opened CLOUD-472, where +/// every clause was present and none had been pressure-tested. +/// `obligations-bound` binds a §7 entry to a killer mutation, but only at +/// implementation time: at refinement there is no code, no case file and no +/// `#MUTANT` row for it to reach. Confirming a named prompt ran over these bytes +/// is a hash comparison that no better-shaped prose can satisfy, because the +/// prose is the input to the hash. +/// +/// # Keyed by (prompt digest, subject digest), for [`TOOL_VERDICT`]'s reason +/// +/// The prompt is VENDORED — compiled into the binary the way +/// `src/policy/presets/**` are — so its digest is a constant of this build and +/// "a particular prompt" is a checkable claim rather than an intention. The +/// subject's digest is the other half, so a record goes stale by construction +/// the moment the thing reviewed changes: edit the ticket body or push a commit +/// and the record no longer answers. That is the anti-staleness property a +/// `status: clean` marker can never provide. +/// +/// **Two subjects, one mechanism.** At refinement the subject is the body the +/// TRACKER returned, which is the same forgery control the `verdict` recorder +/// column earned; at landing it is the branch's delta. +/// +/// # Three answers, and a gate that merges any two reports clean +/// +/// `null` is could-not-look — no row declared a review, or no store is readable. +/// A declared id ABSENT from the map has no record: the prompt never ran over +/// these bytes, and that is the refusal. An id PRESENT with an empty findings +/// object ran and pointed at nothing, which is clean. `forge-verdict-required`'s +/// header states the same three and refuses the opposite one, and the asymmetry +/// is deliberate: the forge is a third party that may legitimately not have +/// judged yet, where a review this branch was supposed to dispatch and did not +/// is the branch's own conduct. +pub const REVIEW: Class = Class::new(Cost::Effect, Surface::Check); /// [`Fact::BaseDelta`] — how the **declared** globs' paths differ from a /// **declared** base rev (CLOUD-1059). /// @@ -1310,6 +1380,7 @@ impl Fact { Fact::Invocations, Fact::Uses, Fact::Symbols, + Fact::Review, Fact::BaseDelta, Fact::Records, Fact::Pinned, @@ -1350,6 +1421,7 @@ impl Fact { Fact::Invocations => "invocations", Fact::Uses => "uses", Fact::Symbols => "symbols", + Fact::Review => "review", Fact::BaseDelta => "base-delta", Fact::Records => "records", Fact::Pinned => "pinned-programs", @@ -1398,6 +1470,7 @@ impl Fact { Fact::Invocations => INVOCATIONS, Fact::Uses => USES, Fact::Symbols => SYMBOLS, + Fact::Review => REVIEW, Fact::BaseDelta => BASE_DELTA, Fact::Records => RECORDS, Fact::Pinned => PINNED, @@ -1485,6 +1558,10 @@ impl Fact { // independent of the surface one, which is exactly what makes the // pair expressive rather than redundant. Fact::Symbols => Some("symbols"), + // The dispatch tier (CLOUD-472). Tree surface for `Symbols`' reason + // — `run_static` refuses a spawning kind on the mediated path — and + // `Cost::Effect` for its own: resolving it runs an agent. + Fact::Review => Some("review"), // Tree-only for the same reason (CLOUD-1059): the answer is a walk // of the base tree and a walk of the working tree, which is a // `check`-surface cost and not a mediated call's. @@ -1591,6 +1668,7 @@ impl Fact { | Fact::Produced | Fact::Records => Self::keyed_read_schema_fragment(self), Fact::Symbols => Self::symbols_schema_fragment(), + Fact::Review => Self::review_schema_fragment(), Fact::Uses => serde_json::json!({ "type": "object", "description": "Fact::Uses (CLOUD-762). Path -> that file's `use` edges. `to` is the module or crate reached AFTER resolution through the crate root's re-export table; `item` the imported leaf name; `origin` one of internal/external/root-item/local; `via_root` whether resolution supplied `to` rather than the text, which is the flag that marks an edge a line predicate reads wrongly. An edge still `root-item` is one the root's table could not name, and is could-not-look at the edge level rather than an edge onto nothing. A path absent from this map could not be parsed; a path present with an empty array imports nothing.", @@ -1723,6 +1801,58 @@ impl Fact { }) } + /// The schema fragment for [`Fact::Review`] (CLOUD-472). + /// + /// **Every leaf is a pointer or a digest**, which is what makes rule 4 + /// structural here rather than a habit the resolution has to keep: there is + /// no string field an agent's prose could occupy, so a module cannot lift a + /// sentence into a finding even if one were recorded. + fn review_schema_fragment() -> serde_json::Value { + serde_json::json!({ + // NULLABLE for `symbols_schema_fragment`'s reason: the projection + // emits `null` for both did-not-look answers, and a schema typing + // this as a bare object refuses the module that handles them. + "type": ["object", "null"], + "description": "Fact::Review (CLOUD-472). Declared id -> the record that a VENDORED agent prompt was dispatched over that id's subject. The second Cost::Effect fact. KEYED BY (prompt digest, subject digest): the prompt is compiled into this binary so its digest is a constant of the build, and the subject's digest is the bytes reviewed -- so editing the ticket body or pushing a commit leaves the record under a different name where it does not answer. A declared id ABSENT from this map was never dispatched, and that absence is the only thing a gate may refuse on; an id PRESENT with an empty `findings` array ran and pointed at nothing. `null` is could-not-look. WHAT THE AGENT CONCLUDED IS NEVER HERE AS PROSE -- `findings` carries pointers, because a gate deciding on an agent's judgement would be a model verdict wearing an exit code, which non-negotiable rule 3 forbids.", + "additionalProperties": { + "type": "object", + "properties": { + "provenance": { + "type": "object", + "properties": { + "tool": {"type": "string"}, + "version": {"type": "string"}, + "invocation": {"type": "array", "items": {"type": "string"}}, + "prompt": {"type": "string"}, + }, + "additionalProperties": false, + }, + "subject": { + "type": "object", + "properties": { + "kind": {"type": "string"}, + "digest": {"type": "string"}, + }, + "additionalProperties": false, + }, + "findings": { + "type": "array", + "items": { + "type": "object", + "properties": { + "path": {"type": "string"}, + "line": {"type": "integer"}, + "clause": {"type": "string"}, + }, + "additionalProperties": false, + }, + }, + }, + "additionalProperties": false, + }, + }) + } + /// The schema fragment for the facts that constrain nothing but their own /// prose (CLOUD-1051 split this out; CLOUD-880 set the precedent). /// @@ -1783,6 +1913,7 @@ impl Fact { | Fact::Uses | Fact::Produced | Fact::Symbols + | Fact::Review | Fact::BaseDelta | Fact::Records | Fact::GitHead @@ -1932,6 +2063,7 @@ impl Fact { | Fact::Invocations | Fact::Uses | Fact::Symbols + | Fact::Review | Fact::BaseDelta | Fact::Pinned | Fact::GitHead @@ -2102,6 +2234,7 @@ impl Fact { | Fact::Invocations | Fact::Uses | Fact::Symbols + | Fact::Review | Fact::BaseDelta | Fact::Records | Fact::Staged @@ -2917,6 +3050,50 @@ impl ToolQuery { } } +/// One declared review a policy row reads (CLOUD-472). +/// +/// **On the row rather than in a top-level table**, for [`ToolQuery`]'s reason: +/// the declaration and the rule that reads it are one object, so a row cannot +/// name a review nothing enables and a review cannot outlive its reader. +#[derive( + Debug, Clone, PartialEq, Eq, serde::Deserialize, serde::Serialize, schemars::JsonSchema, +)] +#[serde(deny_unknown_fields)] +pub struct ReviewQuery { + /// The key this review is projected under in `input.tree.review`. + /// + /// The declared id rather than the composed key, because a composed key + /// carries two digests that move whenever the prompt or the subject does — a + /// module written against one would have to be edited on every edit to the + /// thing it judges. + pub id: String, + /// Which VENDORED prompt to dispatch, by id. + /// + /// Vendored rather than a path, and that is the safety property: the text is + /// compiled into the binary, so its digest is a constant of the build and a + /// consumer cannot satisfy the gate by pointing it at an easier prompt. + pub prompt: String, + /// The agent to dispatch it through. + pub runner: String, + /// The version the runner is pinned at, recorded as provenance. + /// + /// A component of the record rather than a comparison made after the read, + /// for [`ToolQuery::version`]'s reason: a version compared afterwards is a + /// comparison a module can forget to make. + pub version: String, + /// The exact flags, so a reader can tell which question was asked. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub args: Vec, + /// What KIND of thing is being reviewed, e.g. `document`. + pub subject: String, + /// The repository-relative path whose bytes the review was taken over. + /// + /// Its digest is the second component of the key, so a review goes stale by + /// construction the moment the subject changes — the anti-staleness half, and + /// the one a `reviewed: true` marker could never provide. + pub path: String, +} + /// What joins a [`ToolQuery`]'s components into one record name. /// /// Stated once here rather than spelled at both the composing and the validating diff --git a/crates/batten/src/hook.rs b/crates/batten/src/hook.rs index a11d2660e..1d70d5d86 100644 --- a/crates/batten/src/hook.rs +++ b/crates/batten/src/hook.rs @@ -6117,6 +6117,10 @@ fn call_document(envelope: &Envelope, facts: &Facts<'_>) -> Result None, crate::facts::Fact::Minted => None, + // The dispatch tier is `Surface::Check` (CLOUD-472): `run_static` + // refuses a spawning kind on the mediated path, so a projection here + // would offer a module a key the hook surface can never fill. + crate::facts::Fact::Review => None, // CLOUD-1188. `Surface::Check` in `facts.rs`, so this arm is `None` // by the model rather than by this function's opinion. Answering // means reading and parsing the capture store until a declared key @@ -8149,6 +8153,7 @@ mod tests { fn shape(id: &str, pattern: &str, contains: Option<&str>) -> Rule { Rule { + review: Vec::new(), id: id.to_owned(), kind: crate::rules::RuleKind::Shape, glob: None, diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index c505d3748..16f1028ea 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -85,6 +85,7 @@ pub mod redirect; pub mod refusal; pub mod render; pub mod resolve; +pub mod review; pub mod rules; pub mod secrets; pub mod selfwrite; @@ -3927,6 +3928,7 @@ fn suite_input( // so every member here is empty for one reason: this call builds the // SHAPE, and the case chooses the values. &rules::Resolved { + review: &crate::facts::Look::IsNot, produced: &std::collections::BTreeMap::new(), records: &std::collections::BTreeMap::new(), git: &git::GitFacts::default(), diff --git a/crates/batten/src/review.rs b/crates/batten/src/review.rs new file mode 100644 index 000000000..d9917af5b --- /dev/null +++ b/crates/batten/src/review.rs @@ -0,0 +1,354 @@ +//! That a **vendored** agent prompt was dispatched over a declared subject +//! (CLOUD-472). +//! +//! **The second occupant of [`Cost::Effect`]**, and the third adopter of the +//! delegated-analyser shape: `secrets.rs` (ripsecrets) → `symbols.rs` (clippy) → +//! this. What carries across is the SHAPE — a declared program, flags pinned +//! beside the parser, provenance recorded because a fact whose meaning depends on +//! an unrecorded tool version is not canonical, and one invariant verbatim: +//! +//! > **clean is never inferred from a stream that failed to parse.** +//! +//! # What it answers, and why the narrowness is the whole mechanism +//! +//! **THAT a particular prompt ran over these exact bytes.** Not whether the +//! review was good, not whether its findings are real. Those are judgements and +//! non-negotiable rule 3 forbids a gate deciding one — so a gate over this fact +//! refuses ABSENCE, which is a comparison of two digests, and the agent's +//! findings reach a module as pointers that carry no claim to weigh. +//! +//! That bound is what makes an LLM in a resolution path legal at all. It is also +//! why the cheaper tiers do not substitute: `ready-lint` gates the SHAPE of a +//! refinement block, and shape is what an author optimises against once the gate +//! exists — the measured failure that opened CLOUD-472, where every clause was +//! present and none had been pressure-tested. Confirming a named prompt ran over +//! these bytes is a hash comparison no better-shaped prose can satisfy, because +//! the prose is the input to the hash. +//! +//! # Spawn on miss, read on hit — `step-receipt`'s pattern, not a new one +//! +//! A review costs minutes and tokens, and `check` runs every landing lap, so +//! resolving it unconditionally would be unaffordable and the gate would be +//! switched off. The record is keyed by (prompt digest, subject digest), so the +//! agent runs ONCE per unique subject and every later lap is a cache hit — the +//! same "same inputs, same command, same toolchain ⇒ same verdict" the step +//! receipts already use. +//! +//! **The keying is the anti-staleness property, not an optimisation.** Edit the +//! ticket body or push a commit and the subject digest moves, so the record lives +//! under a name nothing looks up and the review must run again. A `reviewed: true` +//! marker could never provide that. +//! +//! # Why the prompt is vendored +//! +//! The prompt is compiled into this binary, the way `src/policy/presets/**` are, +//! so its digest is a constant of the build. That is what makes *a particular +//! prompt* a checkable claim rather than an intention — a consumer cannot satisfy +//! the gate by pointing it at an easier prompt, because the digest in the key is +//! not theirs to choose. + +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; + +use crate::facts::Look; + +/// The vendored prompts, by id. +/// +/// Compiled in for the reason `policy/presets/**` are: a consumer who wrote no +/// rows still gets them, and the text cannot be swapped to something weaker +/// without changing the digest the record is keyed by. +pub const PROMPTS: &[(&str, &str)] = &[( + "ready-pressure-test", + include_str!("review/ready-pressure-test.md"), +)]; + +/// The directory the records live under, beneath the git dir. +const STORE: &str = "batten-review"; + +/// What joins a record's key components into one name. +/// +/// Stated once rather than at both the composing and the validating site, which +/// is the two-authorities shape `.claude/rules/policy-modules.md` records for +/// patterns one layer down. Shared with [`crate::facts::KEY_SEPARATOR`]'s reason +/// and spelled the same way. +const KEY_SEPARATOR: char = '@'; + +/// One pointer a review produced. +/// +/// **A pointer and nothing else.** There is no field an agent's prose could +/// occupy, so non-negotiable rule 4 holds structurally here rather than by the +/// parser remembering to strip — the same shaping `symbols::Site` uses to keep a +/// diagnostic's rendered message out of the fact. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +#[non_exhaustive] +pub struct Finding { + /// Repo-relative and `/`-separated, or the subject's own id where the + /// subject is not a file. + pub path: String, + /// 1-indexed, where the subject has lines. + pub line: u32, + /// Which clause of the reviewed object this points at, e.g. `§7`. A closed + /// token the prompt declares, never a sentence. + pub clause: String, +} + +/// Which agent produced a record, and how. +/// +/// [`crate::symbols::Provenance`] plus the prompt's digest, because here the +/// prompt is half of what the answer means: two runs that disagree because the +/// prompt changed must be distinguishable from two that disagree because the +/// subject did. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub struct Provenance { + /// The program, as invoked. + pub tool: String, + /// The version the row pinned it at. + pub version: String, + /// The exact flags, so a reader can tell which question was asked. + pub invocation: Vec, + /// The digest of the VENDORED prompt text. + pub prompt: String, +} + +/// What the review ran over. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub struct Subject { + /// The declared kind, e.g. `document` or `delta`. + pub kind: String, + /// The digest of the reviewed bytes. + pub digest: String, +} + +/// One dispatched review. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub struct Record { + /// Which agent, which version, which prompt. + pub provenance: Provenance, + /// Which bytes, and their digest. + pub subject: Subject, + /// Every pointer the run produced, sorted — an agent's output order is not + /// stable across runs and §6 byte-stability is. + pub findings: Vec, +} + +/// The digest of some bytes, as the key composes it. +#[must_use] +pub fn digest(bytes: &[u8]) -> String { + crate::tools::digest(bytes) +} + +/// The record's path for one (id, prompt digest, subject digest) triple. +/// +/// **Keyed rather than compared**, for [`crate::facts::ToolQuery`]'s reason: a +/// record from another prompt or over other bytes lives under a different name +/// and is never opened, so staleness cannot be a comparison a caller forgets to +/// make. +#[must_use] +pub fn record_path(git_dir: &Path, id: &str, prompt: &str, subject: &str) -> PathBuf { + git_dir.join(STORE).join(format!( + "{id}{KEY_SEPARATOR}{prompt}{KEY_SEPARATOR}{subject}" + )) +} + +/// The vendored prompt text for an id, and its digest. +#[must_use] +pub fn prompt(id: &str) -> Option<(&'static str, String)> { + PROMPTS + .iter() + .find(|(name, _)| *name == id) + .map(|(_, text)| (*text, digest(text.as_bytes()))) +} + +/// Read a record back, or say why it could not be read. +/// +/// **Absent is not empty**, and the two are the whole point: a missing file means +/// the prompt never ran over these bytes, which is the one thing a gate over this +/// fact may refuse on. A file present with no findings means it ran and pointed +/// at nothing. +#[must_use] +pub fn read(path: &Path) -> Look> { + let Ok(text) = std::fs::read_to_string(path) else { + return Look::IsNot; + }; + let mut findings = Vec::new(); + for line in text.lines() { + if line.trim().is_empty() { + continue; + } + let mut words = line.split_whitespace(); + let (Some(path), Some(line_no), Some(clause)) = (words.next(), words.next(), words.next()) + else { + // CLEAN IS NEVER INFERRED FROM A STREAM THAT FAILED TO PARSE + // (`secrets.rs`' invariant, carried verbatim). A malformed record is + // could-not-look, never an empty finding set. + return Look::CouldNotLook; + }; + let Ok(line_no) = line_no.parse::() else { + return Look::CouldNotLook; + }; + findings.push(Finding { + path: path.to_owned(), + line: line_no, + clause: clause.to_owned(), + }); + } + findings.sort(); + Look::Is(findings) +} + +/// Every declared review this tree has a record for. +/// +/// Returns [`Look::CouldNotLook`] when no store is readable at all, and a map +/// otherwise — with a declared id ABSENT from it when its prompt has not run over +/// its subject. That absence is the refusal, and keeping it distinct from an +/// empty finding list is what stops a gate reporting clean over a review that +/// never happened. +#[must_use] +pub fn resolve( + root: &Path, + declared: &[crate::facts::ReviewQuery], +) -> Look> { + if declared.is_empty() { + return Look::IsNot; + } + let Ok(git_dir) = crate::git::git_dir(root) else { + return Look::CouldNotLook; + }; + let mut found = BTreeMap::new(); + for row in declared { + let Some((_, prompt_digest)) = prompt(&row.prompt) else { + continue; + }; + let Look::Is(subject) = subject_digest(root, row) else { + continue; + }; + let path = record_path(&git_dir, &row.id, &prompt_digest, &subject); + // SPAWN ON MISS, READ ON HIT. The dispatch is the engine's rather than a + // call somebody has to remember, which is the whole difference from a + // producer-writes-outside store — measured dead on `tool-verdict`, where + // `validator-verdict-clean` reads a record nothing ever writes + // (CLOUD-1265). A hit costs a file read, so the agent runs once per + // unique subject and every later landing lap is free. + if matches!(read(&path), Look::IsNot) { + dispatch(root, row, &path); + } + let Look::Is(findings) = read(&path) else { + continue; + }; + found.insert( + row.id.clone(), + Record { + provenance: Provenance { + tool: row.runner.clone(), + version: row.version.clone(), + invocation: row.args.clone(), + prompt: prompt_digest, + }, + subject: Subject { + kind: row.subject.clone(), + digest: subject, + }, + findings, + }, + ); + } + Look::Is(found) +} + +/// The digest of whatever this row declares as its subject. +fn subject_digest(root: &Path, row: &crate::facts::ReviewQuery) -> Look { + let Ok(bytes) = std::fs::read(root.join(&row.path)) else { + return Look::CouldNotLook; + }; + Look::Is(digest(&bytes)) +} + +/// Run the vendored prompt over this row's subject and store what it pointed at. +/// +/// # Everything that can go wrong leaves NO record, deliberately +/// +/// A failed dispatch must be indistinguishable from one that never happened: +/// both mean this prompt has not been shown to run over these bytes, and both +/// must refuse. Writing an empty record on failure would turn a broken runner +/// into a clean review — `secrets.rs`' invariant carried verbatim, **clean is +/// never inferred from a stream that failed to parse**, and here the stakes are +/// the whole gate rather than one finding. +fn dispatch(root: &Path, row: &crate::facts::ReviewQuery, path: &Path) { + let Some((text, _)) = prompt(&row.prompt) else { + return; + }; + #[expect( + clippy::disallowed_types, + reason = "stays: this fact IS Cost::Effect — resolving it dispatches the vendored prompt, which is the classification rather than an accident of it. A verb the caller must remember instead is the producer-writes-outside shape CLOUD-1265 measures dead (CLOUD-472)" + )] + let spawned = std::process::Command::new(&row.runner) + .args(&row.args) + .current_dir(root) + .stdin(std::process::Stdio::piped()) + // Both streams captured, NEITHER forwarded: an agent's stderr is prose, + // and echoing a child's stream would put output Batten never shaped onto + // Batten's own (rule 4). + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn(); + let Ok(mut child) = spawned else { + return; + }; + if let Some(mut stdin) = child.stdin.take() { + use std::io::Write as _; + if stdin.write_all(text.as_bytes()).is_err() { + return; + } + } + let Ok(output) = child.wait_with_output() else { + return; + }; + // THE CROSS-CHECK. A non-zero status means the agent itself failed, and + // findings parsed out of a failed run describe a review that did not finish. + if !output.status.success() { + return; + } + let stdout = String::from_utf8_lossy(&output.stdout); + let Some(body) = pointers_in(&stdout) else { + return; + }; + if let Some(parent) = path.parent() + && std::fs::create_dir_all(parent).is_err() + { + return; + } + let _ = std::fs::write(path, body); +} + +/// Keep only the lines that are pointers, and refuse the stream if any line is +/// not one. +/// +/// **Refusing rather than filtering** is what keeps an agent's prose out of the +/// record: a parser that skipped what it did not understand would store whichever +/// subset happened to look like a pointer and call the rest absent, which is a +/// silent partial answer. `None` rejects the whole stream. +fn pointers_in(stdout: &str) -> Option { + let mut lines = Vec::new(); + for line in stdout.lines() { + if line.trim().is_empty() { + continue; + } + let mut words = line.split_whitespace(); + let (Some(subject), Some(number), Some(clause), None) = + (words.next(), words.next(), words.next(), words.next()) + else { + return None; + }; + number.parse::().ok()?; + lines.push(format!("{subject} {number} {clause}")); + } + lines.sort(); + Some(if lines.is_empty() { + String::new() + } else { + format!("{}\n", lines.join("\n")) + }) +} diff --git a/crates/batten/src/review/ready-pressure-test.md b/crates/batten/src/review/ready-pressure-test.md new file mode 100644 index 000000000..0700430b1 --- /dev/null +++ b/crates/batten/src/review/ready-pressure-test.md @@ -0,0 +1,15 @@ +# Pressure-test a refinement block + +You are reviewing a tracker row's refinement block before it may enter the ready +queue. You are not deciding whether the work is worth doing. + +For each clause the block carries, answer one question: **is this clause a claim +somebody could check, or is it a sentence that would survive being wrong?** + +Report one line per finding, and nothing else: + + + +`clause` is the section token the finding is about (`§1` … `§8`). Emit no prose, +no summary, and no line for a clause you have no finding about. A run that +reports nothing is a run that found nothing, and is a valid answer. diff --git a/crates/batten/src/rules.rs b/crates/batten/src/rules.rs index cf55900ce..6f6998543 100644 --- a/crates/batten/src/rules.rs +++ b/crates/batten/src/rules.rs @@ -2131,6 +2131,16 @@ pub struct Rule { /// asked would make `check` unusable. #[serde(default, skip_serializing_if = "std::ops::Not::not")] pub symbols: bool, + /// The vendored reviews this policy row reads, **declared** (CLOUD-472). + /// + /// Each becomes an entry of `input.tree.review` under its own `id`. Declared + /// per row for `symbols`' reason and more so: this is the second + /// `Cost::Effect` fact, and a MISS dispatches an agent — minutes and tokens, + /// where clippy is seconds. The digest key makes that once per unique + /// subject rather than once per lap, but a run that paid it unasked would + /// still be the shape that gets a gate switched off. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub review: Vec, /// The refs this policy row resolves, **declared** (CLOUD-907). /// /// Each becomes an entry of `input.tree["git-refs"]` carrying the commit it @@ -3342,6 +3352,10 @@ pub const COLUMN_CENSUS: &[ColumnCensus] = &[ field: "symbols", declares: Declares::Fact(crate::facts::Fact::Symbols, |rule| rule.symbols), }, + ColumnCensus { + field: "review", + declares: Declares::Fact(crate::facts::Fact::Review, |rule| !rule.review.is_empty()), + }, ColumnCensus { field: "refs", declares: Declares::Fact(crate::facts::Fact::GitRef, |rule| !rule.refs.is_empty()), @@ -6219,6 +6233,7 @@ fn run( // family and for the same reason: a projection must not spawn, so the spend // happens once here and only when a row declared it. let symbols = symbols_fact(rules, root); + let review = review_fact(rules, root); // THE OUT-OF-ROOT FILES (CLOUD-1167), acquired once for the whole run beside // the families above and, like every one of them, ONLY FOR WHAT A ROW // DECLARED. A ruleset naming no `[[rule.external]]` reads no environment @@ -6315,6 +6330,7 @@ fn run( records: &records, git: &git, symbols: &symbols, + review: &review, state: state.as_ref(), forge: forge.as_ref(), tool_verdicts: tool_verdicts.as_ref(), @@ -6647,6 +6663,7 @@ struct RunInputs<'a> { git: &'a crate::git::GitFacts, /// The symbol census, iff this rule set declared it (CLOUD-760). symbols: &'a crate::facts::Look, + review: &'a crate::facts::Look>, /// The engine's own finding store, per declared ref (CLOUD-1203). `None` is /// could-not-look and covers both nobody-asked and no-store-bound. state: Option<&'a BTreeMap>>, @@ -7607,6 +7624,7 @@ pub(crate) struct Resolved<'a> { pub git: &'a crate::git::GitFacts, /// The symbol census, iff this rule set declared it (CLOUD-760). pub symbols: &'a crate::facts::Look, + pub review: &'a crate::facts::Look>, /// The out-of-root files this rule set declared (CLOUD-1167). pub external: &'a BTreeMap, /// The engine's own finding store, per declared ref (CLOUD-1203). `None` @@ -8216,6 +8234,67 @@ pub fn symbols_launcher(root: &Path) -> crate::symbols::Launcher { /// The three-valued distinction the doc comment at its call site describes lives /// here rather than there, and the seam is the one that survives: this is the /// only arm whose value is a nested document rather than a `json!` of a field. +/// Resolve the review fact, or say the run was never asked for one. +/// +/// `IsNot` when no row declares a review, which is what keeps the cost off every +/// other consumer — the same gating `symbols_fact` applies one rung down. +fn review_fact( + rules: &[Rule], + root: &Path, +) -> crate::facts::Look> { + let declared: Vec = rules + .iter() + .flat_map(|rule| rule.review.iter().cloned()) + .collect(); + crate::review::resolve(root, &declared) +} + +/// The review fact as the tree document carries it. +/// +/// A declared id ABSENT from the map is the refusal a gate reads, so this map is +/// deliberately partial: an id whose prompt has not run over its subject +/// contributes no entry, where one that ran and pointed at nothing contributes an +/// entry with an empty `findings`. Merging those two is the false clean the whole +/// fact exists to prevent. +fn review_value( + review: &crate::facts::Look>, +) -> serde_json::Value { + match review { + crate::facts::Look::IsNot | crate::facts::Look::CouldNotLook => serde_json::Value::Null, + crate::facts::Look::Is(found) => serde_json::Value::Object( + found + .iter() + .map(|(id, record)| { + ( + id.clone(), + serde_json::json!({ + "provenance": { + "tool": record.provenance.tool, + "version": record.provenance.version, + "invocation": record.provenance.invocation, + "prompt": record.provenance.prompt, + }, + "subject": { + "kind": record.subject.kind, + "digest": record.subject.digest, + }, + "findings": record + .findings + .iter() + .map(|finding| serde_json::json!({ + "path": finding.path, + "line": finding.line, + "clause": finding.clause, + })) + .collect::>(), + }), + ) + }) + .collect(), + ), + } +} + fn symbols_value(symbols: &crate::facts::Look) -> serde_json::Value { match symbols { crate::facts::Look::IsNot | crate::facts::Look::CouldNotLook => serde_json::Value::Null, @@ -8391,6 +8470,7 @@ pub(crate) fn tree_document( // third answer and a real one: the analyser ran and resolved no // site. `null` and `[]` are the pair this projection keeps apart. crate::facts::Fact::Symbols => symbols_value(resolved.symbols), + crate::facts::Fact::Review => review_value(resolved.review), // CLOUD-1059, and `null` here carries BOTH could-not-look conditions // the family already collapses: no row declared a delta, and a row // declared one whose base did not resolve. A migration gate reads the @@ -8478,6 +8558,7 @@ pub(crate) fn tree_document( /// it only says which half of a run's inputs the tree surface reads. fn resolved_of<'a>(inputs: &RunInputs<'a>) -> Resolved<'a> { Resolved { + review: inputs.review, produced: inputs.produced, records: inputs.records, git: inputs.git, @@ -12081,6 +12162,7 @@ mod tests { }, &[], &super::Resolved { + review: &crate::facts::Look::IsNot, produced: &BTreeMap::new(), records: &BTreeMap::new(), git: &crate::git::GitFacts::default(), @@ -12657,6 +12739,7 @@ mod tests { }, &files, &super::Resolved { + review: &crate::facts::Look::IsNot, produced: &BTreeMap::new(), records: &BTreeMap::new(), git: &crate::git::GitFacts::default(), @@ -12800,6 +12883,7 @@ mod tests { records: &self.records, git: &self.git, symbols: &self.symbols, + review: &crate::facts::Look::IsNot, state: None, forge: None, tool_verdicts: None, @@ -12819,6 +12903,7 @@ mod tests { fn blank(id: &str, kind: RuleKind) -> Rule { Rule { + review: Vec::new(), id: id.to_owned(), kind, glob: None, diff --git a/crates/batten/tests/it/facts.rs b/crates/batten/tests/it/facts.rs index a7d592bfe..11d995142 100644 --- a/crates/batten/tests/it/facts.rs +++ b/crates/batten/tests/it/facts.rs @@ -37,7 +37,8 @@ use batten::facts::{ AGENT_SOURCED, BASE_DELTA, BYPASS, CAPTURED, COMMIT_META, Class, Cost, DOCUMENT, EXTERNAL, EXTRACTED, FORGE, Fact, GIT_HEAD, GIT_HISTORY, GIT_RANGE, GIT_REF, GIT_REMOTE, GIT_STATUS, INVOCATIONS, KEYS, LANDING, LINES, Look, MINTED, PINNED, PRODUCED, PROSPECTIVE, RECEIPTS, - RECORDS, STAGED, STATE, STOP, SYMBOLS, Surface, TASKS, TOOL_VERDICT, TRACKED, USES, WAIVED, + RECORDS, REVIEW, STAGED, STATE, STOP, SYMBOLS, Surface, TASKS, TOOL_VERDICT, TRACKED, USES, + WAIVED, }; #[test] @@ -141,6 +142,7 @@ fn every_fact_returns_its_stated_const() { Fact::Invocations => INVOCATIONS, Fact::Uses => USES, Fact::Symbols => SYMBOLS, + Fact::Review => REVIEW, Fact::BaseDelta => BASE_DELTA, Fact::Records => RECORDS, Fact::Pinned => PINNED, diff --git a/schema/batten.local.schema.json b/schema/batten.local.schema.json index c410a3ecc..c88b00a62 100644 --- a/schema/batten.local.schema.json +++ b/schema/batten.local.schema.json @@ -622,6 +622,52 @@ } ] }, + "ReviewQuery": { + "description": "One declared review a policy row reads (CLOUD-472).\n\n**On the row rather than in a top-level table**, for [`ToolQuery`]'s reason:\nthe declaration and the rule that reads it are one object, so a row cannot\nname a review nothing enables and a review cannot outlive its reader.", + "type": "object", + "properties": { + "args": { + "description": "The exact flags, so a reader can tell which question was asked.", + "type": "array", + "items": { + "type": "string" + } + }, + "id": { + "description": "The key this review is projected under in `input.tree.review`.\n\nThe declared id rather than the composed key, because a composed key\ncarries two digests that move whenever the prompt or the subject does — a\nmodule written against one would have to be edited on every edit to the\nthing it judges.", + "type": "string" + }, + "path": { + "description": "The repository-relative path whose bytes the review was taken over.\n\nIts digest is the second component of the key, so a review goes stale by\nconstruction the moment the subject changes — the anti-staleness half, and\nthe one a `reviewed: true` marker could never provide.", + "type": "string" + }, + "prompt": { + "description": "Which VENDORED prompt to dispatch, by id.\n\nVendored rather than a path, and that is the safety property: the text is\ncompiled into the binary, so its digest is a constant of the build and a\nconsumer cannot satisfy the gate by pointing it at an easier prompt.", + "type": "string" + }, + "runner": { + "description": "The agent to dispatch it through.", + "type": "string" + }, + "subject": { + "description": "What KIND of thing is being reviewed, e.g. `document`.", + "type": "string" + }, + "version": { + "description": "The version the runner is pinned at, recorded as provenance.\n\nA component of the record rather than a comparison made after the read,\nfor [`ToolQuery::version`]'s reason: a version compared afterwards is a\ncomparison a module can forget to make.", + "type": "string" + } + }, + "additionalProperties": false, + "required": [ + "id", + "prompt", + "runner", + "version", + "subject", + "path" + ] + }, "Rewrite": { "description": "One reference-to-path rewrite for a [`CeilingUnit::TrackedArtifacts`] ceiling\n(CLOUD-925).\n\nBoth fields are the **consumer's**. A shorthand a repository writes in its own\nprompts to name its own files is a property of that repository, so naming\neither half in `crates/batten` would be the consumer-specific identifier\nnon-negotiable rule 1 forbids — the same split\n[`crate::budget::EmbeddedDecl`] already makes for a config key it counts.", "type": "object", @@ -1168,6 +1214,13 @@ "null" ] }, + "review": { + "description": "The vendored reviews this policy row reads, **declared** (CLOUD-472).\n\nEach becomes an entry of `input.tree.review` under its own `id`. Declared\nper row for `symbols`' reason and more so: this is the second\n`Cost::Effect` fact, and a MISS dispatches an agent — minutes and tokens,\nwhere clippy is seconds. The digest key makes that once per unique\nsubject rather than once per lap, but a run that paid it unasked would\nstill be the shape that gets a gate switched off.", + "type": "array", + "items": { + "$ref": "#/$defs/ReviewQuery" + } + }, "run": { "description": "The retired spelling of [`Rule::check`], present only so the refusal can\nname its replacement.\n\nCarried as a field rather than left to `deny_unknown_fields` for the\nsame reason [`crate::config::OverrideConfig::min_batten_version`] is:\n\"unknown field `run`\" reads as a typo, where this is a rename with one\nspecific fix. Every deny points to it (CLOUD-122).\n\nDeliberately absent from [`Rule::columns`]: that census classifies the\ncolumns a kind may *carry*, and no kind carries this one — it is refused\noutright by [`Rule::validate`], ahead of any per-kind question.", "type": [ diff --git a/schema/batten.schema.json b/schema/batten.schema.json index 5ea1d0cf8..87c291b64 100644 --- a/schema/batten.schema.json +++ b/schema/batten.schema.json @@ -2575,6 +2575,52 @@ } ] }, + "ReviewQuery": { + "description": "One declared review a policy row reads (CLOUD-472).\n\n**On the row rather than in a top-level table**, for [`ToolQuery`]'s reason:\nthe declaration and the rule that reads it are one object, so a row cannot\nname a review nothing enables and a review cannot outlive its reader.", + "type": "object", + "properties": { + "args": { + "description": "The exact flags, so a reader can tell which question was asked.", + "type": "array", + "items": { + "type": "string" + } + }, + "id": { + "description": "The key this review is projected under in `input.tree.review`.\n\nThe declared id rather than the composed key, because a composed key\ncarries two digests that move whenever the prompt or the subject does — a\nmodule written against one would have to be edited on every edit to the\nthing it judges.", + "type": "string" + }, + "path": { + "description": "The repository-relative path whose bytes the review was taken over.\n\nIts digest is the second component of the key, so a review goes stale by\nconstruction the moment the subject changes — the anti-staleness half, and\nthe one a `reviewed: true` marker could never provide.", + "type": "string" + }, + "prompt": { + "description": "Which VENDORED prompt to dispatch, by id.\n\nVendored rather than a path, and that is the safety property: the text is\ncompiled into the binary, so its digest is a constant of the build and a\nconsumer cannot satisfy the gate by pointing it at an easier prompt.", + "type": "string" + }, + "runner": { + "description": "The agent to dispatch it through.", + "type": "string" + }, + "subject": { + "description": "What KIND of thing is being reviewed, e.g. `document`.", + "type": "string" + }, + "version": { + "description": "The version the runner is pinned at, recorded as provenance.\n\nA component of the record rather than a comparison made after the read,\nfor [`ToolQuery::version`]'s reason: a version compared afterwards is a\ncomparison a module can forget to make.", + "type": "string" + } + }, + "additionalProperties": false, + "required": [ + "id", + "prompt", + "runner", + "version", + "subject", + "path" + ] + }, "Rewrite": { "description": "One reference-to-path rewrite for a [`CeilingUnit::TrackedArtifacts`] ceiling\n(CLOUD-925).\n\nBoth fields are the **consumer's**. A shorthand a repository writes in its own\nprompts to name its own files is a property of that repository, so naming\neither half in `crates/batten` would be the consumer-specific identifier\nnon-negotiable rule 1 forbids — the same split\n[`crate::budget::EmbeddedDecl`] already makes for a config key it counts.", "type": "object", @@ -3176,6 +3222,13 @@ "null" ] }, + "review": { + "description": "The vendored reviews this policy row reads, **declared** (CLOUD-472).\n\nEach becomes an entry of `input.tree.review` under its own `id`. Declared\nper row for `symbols`' reason and more so: this is the second\n`Cost::Effect` fact, and a MISS dispatches an agent — minutes and tokens,\nwhere clippy is seconds. The digest key makes that once per unique\nsubject rather than once per lap, but a run that paid it unasked would\nstill be the shape that gets a gate switched off.", + "type": "array", + "items": { + "$ref": "#/$defs/ReviewQuery" + } + }, "run": { "description": "The retired spelling of [`Rule::check`], present only so the refusal can\nname its replacement.\n\nCarried as a field rather than left to `deny_unknown_fields` for the\nsame reason [`crate::config::OverrideConfig::min_batten_version`] is:\n\"unknown field `run`\" reads as a typo, where this is a rename with one\nspecific fix. Every deny points to it (CLOUD-122).\n\nDeliberately absent from [`Rule::columns`]: that census classifies the\ncolumns a kind may *carry*, and no kind carries this one — it is refused\noutright by [`Rule::validate`], ahead of any per-kind question.", "type": [ diff --git a/schema/policy-input.schema.json b/schema/policy-input.schema.json index 7de054ea4..075cf2668 100644 --- a/schema/policy-input.schema.json +++ b/schema/policy-input.schema.json @@ -344,6 +344,70 @@ "description": "Fact::Records (CLOUD-1051). RECORD name -> the lines accumulated in it on this branch, in write order. Keyed by the record rather than by the recorder row because several rows may write one record. Each line is the recorder's own whitespace-free columns; the projection adds nothing and reads nothing out of them. A record ABSENT from this map could not be read; the collapse into an empty list is what this keeps open.", "type": "object" }, + "review": { + "additionalProperties": { + "additionalProperties": false, + "properties": { + "findings": { + "items": { + "additionalProperties": false, + "properties": { + "clause": { + "type": "string" + }, + "line": { + "type": "integer" + }, + "path": { + "type": "string" + } + }, + "type": "object" + }, + "type": "array" + }, + "provenance": { + "additionalProperties": false, + "properties": { + "invocation": { + "items": { + "type": "string" + }, + "type": "array" + }, + "prompt": { + "type": "string" + }, + "tool": { + "type": "string" + }, + "version": { + "type": "string" + } + }, + "type": "object" + }, + "subject": { + "additionalProperties": false, + "properties": { + "digest": { + "type": "string" + }, + "kind": { + "type": "string" + } + }, + "type": "object" + } + }, + "type": "object" + }, + "description": "Fact::Review (CLOUD-472). Declared id -> the record that a VENDORED agent prompt was dispatched over that id's subject. The second Cost::Effect fact. KEYED BY (prompt digest, subject digest): the prompt is compiled into this binary so its digest is a constant of the build, and the subject's digest is the bytes reviewed -- so editing the ticket body or pushing a commit leaves the record under a different name where it does not answer. A declared id ABSENT from this map was never dispatched, and that absence is the only thing a gate may refuse on; an id PRESENT with an empty `findings` array ran and pointed at nothing. `null` is could-not-look. WHAT THE AGENT CONCLUDED IS NEVER HERE AS PROSE -- `findings` carries pointers, because a gate deciding on an agent's judgement would be a model verdict wearing an exit code, which non-negotiable rule 3 forbids.", + "type": [ + "object", + "null" + ] + }, "staged": { "additionalProperties": true, "description": "Fact::Staged (CLOUD-1203). Declared path -> its STAGED content, parsed by the path's format -- `git show :`, which `tracked` explicitly is NOT: that fact walks the WORKING TREE. A path with no staged entry, one whose bytes are not UTF-8, and one whose format this build cannot parse are each ABSENT from this map rather than present with an empty node, and `missing` names the path with the cause. NULL when no row declared a staged read, so a module can tell `nobody asked` from `nothing is staged`.", From 1aebd1e6e51d83d05fc6378cbc9126cbe6e73471 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 2 Sep 2026 14:20:32 +0000 Subject: [PATCH 03/16] fix(review): the dispatch sent a prompt and no subject MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-472. `review::dispatch` wrote the vendored prompt to the child's stdin and nothing else, so the agent was told what to look for and never told what to look at. Its answer was a review of nothing, while the record was keyed to bytes it had never seen — a record that reads as a completed review and is not one, which is strictly worse than no record, because the gate reading it would report clean. THE SUBJECT TRAVELS AS A POINTER, NEVER AS BYTES, and that is Batten's law rather than an economy. `judge.rs` states it at its own head — "sensitive or bulky content is reduced to a pointer and never dumped into a model's context" — and names the LLM judge "the ONE component that inverts it". A review dispatch must not be a second inversion. So what crosses is the prompt, the subject's path and the subject's digest, and the agent reads the bytes with its own tools under whatever access its operator gave it. It was worth considering the other route and it is worth writing down why it loses, because the obvious fix is to hand the bytes to `judge::assemble` and inherit its boundary: * `judge`'s fail-closed rule reads a span with NO PATH PROVENANCE as protected and refuses the whole invocation. That is right for a span and fatal here: a tracker row's body legitimately carries an issue key rather than a repository path, so the grooming subject — the one this row exists for — would have been unimplementable. * `[judge] raw` is empty by this repository's deliberate posture, so `assemble` would have withheld the bytes and sent a pointer and a hash ANYWAY, after considerably more machinery. * A second component inverting the pointer-only law is a widening no consumer asked for, and the widening would have been invisible: nothing in the config would have said that enabling a review row also started shipping file contents outward. The claim the record makes is unchanged. The digest is still taken over the bytes on disk, so a record still cannot be keyed to anything but the exact subject, and editing the subject still leaves it under a name nothing looks up. What changes is that Batten never becomes the thing that moved somebody's file into a model. The vendored prompt is updated to match: it states that the subject arrives as ` ` and must be read by the agent, that a run reporting nothing is a valid answer — the caller distinguishes that from a run that never happened, so there is never a reason to invent a finding — and that any line which is not three whitespace-separated fields discards the whole run. Refs: CLOUD-135, CLOUD-1265 --- crates/batten/src/review.rs | 49 ++++++++++++++----- .../batten/src/review/ready-pressure-test.md | 36 +++++++++++--- 2 files changed, 67 insertions(+), 18 deletions(-) diff --git a/crates/batten/src/review.rs b/crates/batten/src/review.rs index d9917af5b..256b0ea2f 100644 --- a/crates/batten/src/review.rs +++ b/crates/batten/src/review.rs @@ -222,9 +222,13 @@ pub fn resolve( let Some((_, prompt_digest)) = prompt(&row.prompt) else { continue; }; - let Look::Is(subject) = subject_digest(root, row) else { + let Ok(bytes) = std::fs::read(root.join(&row.path)) else { + // COULD NOT READ THE SUBJECT is could-not-look, and it must not + // dispatch: a review keyed to bytes nobody could read would be a + // record about a subject that does not exist. continue; }; + let subject = digest(&bytes); let path = record_path(&git_dir, &row.id, &prompt_digest, &subject); // SPAWN ON MISS, READ ON HIT. The dispatch is the engine's rather than a // call somebody has to remember, which is the whole difference from a @@ -233,7 +237,7 @@ pub fn resolve( // (CLOUD-1265). A hit costs a file read, so the agent runs once per // unique subject and every later landing lap is free. if matches!(read(&path), Look::IsNot) { - dispatch(root, row, &path); + dispatch(root, row, &path, &subject); } let Look::Is(findings) = read(&path) else { continue; @@ -258,14 +262,6 @@ pub fn resolve( Look::Is(found) } -/// The digest of whatever this row declares as its subject. -fn subject_digest(root: &Path, row: &crate::facts::ReviewQuery) -> Look { - let Ok(bytes) = std::fs::read(root.join(&row.path)) else { - return Look::CouldNotLook; - }; - Look::Is(digest(&bytes)) -} - /// Run the vendored prompt over this row's subject and store what it pointed at. /// /// # Everything that can go wrong leaves NO record, deliberately @@ -276,10 +272,31 @@ fn subject_digest(root: &Path, row: &crate::facts::ReviewQuery) -> Look /// into a clean review — `secrets.rs`' invariant carried verbatim, **clean is /// never inferred from a stream that failed to parse**, and here the stakes are /// the whole gate rather than one finding. -fn dispatch(root: &Path, row: &crate::facts::ReviewQuery, path: &Path) { +fn dispatch(root: &Path, row: &crate::facts::ReviewQuery, path: &Path, subject_digest: &str) { let Some((text, _)) = prompt(&row.prompt) else { return; }; + // THE SUBJECT TRAVELS AS A POINTER, NEVER AS BYTES — Batten's law, not an + // economy. + // + // `judge.rs` states the law at its own head: "sensitive or bulky content is + // reduced to a pointer and never dumped into a model's context", and names + // the LLM judge "the ONE component that inverts it". A review dispatch must + // not be a second inversion. So what crosses is the vendored prompt, the + // subject's PATH, and the subject's DIGEST — and the agent reads the bytes + // with its own tools, under whatever access its operator gave it. + // + // That is not a weaker claim about what was reviewed. The digest in the + // record is over the bytes on disk, so a record still cannot be keyed to + // anything but the exact subject; what changes is that Batten never becomes + // the thing that moved somebody's file into a model. + // + // It is also what lets a subject with no repo path work at all. `judge`'s + // fail-closed rule reads a span with no path provenance as PROTECTED and + // refuses the whole invocation — correct for a span, and fatal for a tracker + // body, which legitimately has an issue key instead of a path. A pointer has + // no such problem. + let pointer = format!("{} {}\n", row.path, subject_digest); #[expect( clippy::disallowed_types, reason = "stays: this fact IS Cost::Effect — resolving it dispatches the vendored prompt, which is the classification rather than an accident of it. A verb the caller must remember instead is the producer-writes-outside shape CLOUD-1265 measures dead (CLOUD-472)" @@ -297,9 +314,17 @@ fn dispatch(root: &Path, row: &crate::facts::ReviewQuery, path: &Path) { let Ok(mut child) = spawned else { return; }; + // THE PROMPT AND THEN THE POINTER. The prompt ALONE was what this function + // sent before, and that was the defect: the agent was told what to look for + // and never told what to look at, so its answer was a review of nothing while + // the record was keyed to bytes it had never seen — a record that reads as a + // completed review and is not one, which is worse than no record at all. if let Some(mut stdin) = child.stdin.take() { use std::io::Write as _; - if stdin.write_all(text.as_bytes()).is_err() { + if stdin.write_all(text.as_bytes()).is_err() + || stdin.write_all(b"\n").is_err() + || stdin.write_all(pointer.as_bytes()).is_err() + { return; } } diff --git a/crates/batten/src/review/ready-pressure-test.md b/crates/batten/src/review/ready-pressure-test.md index 0700430b1..1192e3103 100644 --- a/crates/batten/src/review/ready-pressure-test.md +++ b/crates/batten/src/review/ready-pressure-test.md @@ -1,15 +1,39 @@ # Pressure-test a refinement block You are reviewing a tracker row's refinement block before it may enter the ready -queue. You are not deciding whether the work is worth doing. +queue. You are **not** deciding whether the work is worth doing. + +## What you are given + +This prompt, then one line: + + + +`` is a repository-relative path or a tracker row's key. `` is +the digest of its bytes at the moment the review was asked for. + +**Read the subject yourself**, with whatever tools you have. It is deliberately +not pasted here: Batten reduces content to a pointer and never dumps it into a +model's context, and this dispatch is not an exception to that. + +## What to answer For each clause the block carries, answer one question: **is this clause a claim somebody could check, or is it a sentence that would survive being wrong?** -Report one line per finding, and nothing else: +## What to emit + +One line per finding, and nothing else: + + + +`` is the section token the finding is about (`§1` … `§8`). `` is a +1-based line number, or `0` where the subject has no lines. - +Emit no prose, no preamble, no summary, and no line for a clause you have no +finding about. **A run that reports nothing is a run that found nothing, and is a +valid answer** — the caller distinguishes that from a run that never happened, so +you never need to invent a finding to show you were here. -`clause` is the section token the finding is about (`§1` … `§8`). Emit no prose, -no summary, and no line for a clause you have no finding about. A run that -reports nothing is a run that found nothing, and is a valid answer. +Any line that is not exactly three whitespace-separated fields causes the whole +run to be discarded, so do not explain yourself. From cc2e91b2fb279e0c1d75e90f7c183b67fb2318c9 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 2 Sep 2026 14:20:51 +0000 Subject: [PATCH 04/16] feat(policy): the declared review becomes a gate, and an absent runner is not guilt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-472. `Fact::Review` landed as a store nothing read — the same dead gate its own commit message spent four paragraphs diagnosing on `tool-verdict`. This is the reader: `policy/review-dispatched.rego`, its verdict, the row that enables it, and the compiled tier that proves the engine builds what the module reads. WHAT THE GATE ASSERTS IS STRONGER THAN "SOMEBODY REVIEWED". Because `resolve` DISPATCHES on a miss, a review is compelled rather than merely checked, and the reachable states collapse to three: the agent ran and recorded (clean), the agent ran and gave nothing usable (refused — the branch's problem), or there is no agent here to ask (could-not-look). Forgetting is not among them. AN ABSENT RUNNER IS COULD-NOT-LOOK, AND GETTING THIS WRONG IS WHAT THE FIRST ATTEMPT DID. The dispatch returned silently on a failed spawn, which made a machine with no reviewer installed indistinguishable from a branch that skipped its review — a verdict about the OPERATOR wearing a verdict about the branch. Measured: it took four unrelated `cli.rs` cases red, because a fixture has no agent and never will. `symbols::resolve` keeps the identical pair apart in the same words — "the analyser is not installed, or could not be spawned" is could-not-look — and `Dispatch` now names all three outcomes so the distinction is in the type rather than in a comment. GATED ON THE ROW'S OWN DELTA. A review is owed when the thing to review CHANGED, so the row declares `delta_sources` over its subject and the module refuses only against a non-empty delta. Ungated, the refusal fires on every checkout that has never dispatched — every fixture and every fresh clone — which is the shape that gets a gate switched off rather than satisfied. `plan-complete` carries the identical narrowing for the identical reason, measured the same way. IT REFUSES ABSENCE AND NOTHING ELSE. What the agent CONCLUDED is never read: that would be a model verdict wearing an exit code, which non-negotiable rule 3 forbids, and the fact is shaped so it cannot be written — `findings` are `{path, line, clause}` pointers with no field prose could occupy. A review that pointed at something is as clean as one that pointed at nothing, deliberately: pricing findings is how you buy an agent that reports none. THE OPPOSITE ARM FROM `forge check red`, and the asymmetry is the design. That module refuses only a PRESENT-and-red verdict and reads absence as could-not-look, because the forge is a third party that may legitimately not have judged yet. A review this branch was supposed to dispatch is the branch's own conduct. THE DISPATCH GOES THROUGH `exec::piped`, THE PLACED CHILD-PROCESS ADAPTER, and the first attempt did the opposite. It spawned directly and then added `review` to `policy/spawn-adapters.rego`'s table — which is precisely the move that rule's own remedy tells you not to make: `spawn place missing` says "route it through `exec`". `exec::piped` exists for exactly this and its doc comment had already named the failure: two earlier callers each grew their own `Command::new`, "which is two spawns in two unplaced modules and one shape written twice". This was nearly the third. Routing there means there is no new spawn site to place at all, so the adapter table is untouched, and it buys the shebang resolution ladder a `#!/usr/bin/env` runner needs on a host where `CreateProcess` refuses an extensionless program. The verdict is `prompt run never` under a new `prompt` vocabulary word rather than a third `review …` class: `review answer missing` and `review read absent` are about the FORGE's review of a pull request, and a name a reader has to look up is the one thing the three-word grammar exists to prevent. Verification: `policy test` 46 bundles / 558 passed; `config lint` 0 smells; `test:cargo` green. Refs: CLOUD-760, CLOUD-1265, CLOUD-418, CLOUD-135 Admits: fbe348673db0a5c9304b0d27bd849b6b89d6b3468d23f1f81f6a738c269bcd6c Admits-rule: protected-mutation Admits-verdict: path write refused Admits-subject: batten.toml Admits-head: 6019ff2ca475c5aa1fdaa7ca425661898643f7a9 Admits-epoch: 4d644b4da6de2e07a9c4d19af1bbf102c467b4ab4e83e3a09a0885497a9cfb66 Admits-author: alec@wenzowski.com Admits-prev: 5556da73126df3569d0b9d711b2111213a3afc3e7b05339a9b7b7ff9bb287557 Admits-answer-lost: CLOUD-472 entirely. `policy/review-dispatched.rego` decides nothing unless a row registers it and declares the `[[rule.review]]` facts it reads, and a module raising a `[[verdict]]` token no row declares fails to load. The alternative is landing the module and both test tiers over a gate the engine never runs. Admits-answer-precondition: The change adds the `review-dispatched` `[[rule]]` and its two `[[rule.review]]` rows, plus the `[[verdict]]` registry rows the module raises. A rule and its declared facts are config by construction — there is no verb that writes a `[[rule]]`, since a gate that could add its own rows would be a second authority over the committed one. It lands in the diff a reviewer reads, beside the module it registers. Admits-answer-rejected-route: `config read first` is what this change is FOR — reading the committed config is how the row's shape, its `base`/`delta_sources` narrowing and the vocabulary rows were derived. `patch run first` has nothing to patch: no verb adds a rule, a review row, or a verdict-registry row. --- batten.toml | 79 +++++ crates/batten/src/review.rs | 108 +++--- crates/batten/tests/it/main.rs | 1 + crates/batten/tests/it/review_dispatched.rs | 325 +++++++++++++++++++ crates/batten/tests/it/verdict_vocabulary.rs | 1 + mise.toml | 2 +- policy/review-dispatched.rego | 186 +++++++++++ 7 files changed, 659 insertions(+), 43 deletions(-) create mode 100644 crates/batten/tests/it/review_dispatched.rs create mode 100644 policy/review-dispatched.rego diff --git a/batten.toml b/batten.toml index 4fede06cd..294d3489a 100644 --- a/batten.toml +++ b/batten.toml @@ -5090,6 +5090,41 @@ reduce = "present" # validated these bytes at this version — absent from the map, not a verdict — so # this row is inert on a checkout nobody has recorded against, which is every # checkout today. +# CLOUD-472's consumer, and the row that makes `Fact::Review` a gate rather than +# a store. +# +# `[[rule.review]]` is what pays for it, on `[[rule.tools]]`'s shape and for +# `symbols = true`'s reason: this is the second `Cost::Effect` fact, so a MISS +# dispatches an agent — minutes and tokens, where clippy is seconds. The digest +# key makes that once per unique subject rather than once per landing lap, but a +# run that paid it unasked would still be the shape that gets a gate switched off. +# +# `scope = "tree"` is forced rather than chosen: `Fact::Review` is not +# `Surface::Hook`, and `validate()` refuses a mediated row declaring a +# tree-surface fact. A round trip to an agent on every tool call would be +# disqualifying against the invocation budget besides. +# +# THE PROMPT ID NAMES A VENDORED PROMPT, never a path. Its text is compiled into +# the binary, so its digest is a constant of the build — which is what makes "a +# particular prompt ran" a checkable claim rather than an intention, and what +# stops a consumer satisfying the gate by pointing it at an easier one. +[[rule]] +id = "review-dispatched" +kind = "policy" +scope = "tree" +base = "origin/main" +delta_sources = ["AGENTS.md"] +module = "policy/review-dispatched.rego" +severity = "deny" + +[[rule.review]] +id = "ready-pressure-test" +prompt = "ready-pressure-test" +runner = "batten-review-runner" +version = "0" +subject = "document" +path = "AGENTS.md" + [[rule]] id = "validator-verdict-clean" kind = "policy" @@ -7105,6 +7140,10 @@ gloss = "an executable" word = "plan" gloss = "the work a branch declared it would do" +[[vocabulary.subject]] +word = "prompt" +gloss = "a vendored agent prompt" + [[vocabulary.subject]] word = "prose" gloss = "authored text" @@ -9672,6 +9711,46 @@ target = "mise.toml" # counts one collection plus a page guard, so the second condition is # `review read absent`'s below. Leaving it named here would send a reader looking for # threads on a head whose only problem is that nobody looked. +# CLOUD-472. A declared review that never ran over these bytes. +# +# THE SUBJECT IS `prompt`, NOT `review`, and the distinction is load-bearing +# rather than fussy: `review answer missing` and `review read absent` below are +# about the FORGE's review of a pull request, and a reader meeting a third +# `review …` class would have to look up which kind it meant. This one is about a +# vendored prompt this repository dispatches itself. +[[verdict]] +id = "prompt run never" +gloss = "a review this branch declared it would run has not run over these bytes" +class = """ +Refuses ABSENCE and nothing else. A declared id missing from the review store \ +means the vendored prompt has not been shown to run over this subject, which is \ +a comparison of two digests — a record keyed to another prompt, or to bytes that \ +have since changed, lives under a different name and does not answer. What the \ +agent CONCLUDED is never read: refusing on that would be a model verdict wearing \ +an exit code, which non-negotiable rule 3 forbids, and the fact is shaped so it \ +cannot be written. A review that ran and pointed at something is as clean as one \ +that pointed at nothing, deliberately — pricing findings is how you buy an agent \ +that reports none. This is the opposite arm from `forge check red`, which reads \ +absence as could-not-look because the forge may legitimately not have judged \ +yet; a review this branch was supposed to dispatch and did not is the branch's \ +own conduct. +""" + +[[verdict.route]] +id = "task run first" +kind = "command" +target = "mise run verify" + +[[verdict.route]] +id = "task run other" +kind = "command" +target = "check the declared runner is installed and on PATH — a dispatch that cannot spawn leaves no record, exactly as one that never ran" + +[[verdict.route]] +id = "path admit first" +kind = "override" +precondition = "the review cannot be dispatched in this environment at all — no runner exists here — rather than not having been dispatched in one where it could" + [[verdict]] id = "review answer missing" gloss = "readying would buy a CI matrix on a head carrying unresolved review threads" diff --git a/crates/batten/src/review.rs b/crates/batten/src/review.rs index 256b0ea2f..bf465544c 100644 --- a/crates/batten/src/review.rs +++ b/crates/batten/src/review.rs @@ -121,6 +121,27 @@ pub struct Subject { pub digest: String, } +/// Why a dispatch produced no record, when it produced none. +/// +/// **The three are not interchangeable and the first is the load-bearing one.** +/// A runner that is not installed is a fact about the ENVIRONMENT; an agent that +/// ran and failed is a fact about the RUN. Collapsing them makes a machine with +/// no reviewer indistinguishable from a branch that skipped its review, and a +/// gate over that would refuse every checkout where nobody has installed the +/// agent — which is a verdict about the operator wearing a verdict about the +/// branch. `symbols::resolve` keeps the identical pair apart, in the same words. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Dispatch { + /// It ran and a record was written. + Ran, + /// It ran and gave nothing usable — non-zero, or a stream that is not + /// pointers. The review was ASKED FOR and did not answer, which is the + /// branch's problem and must refuse. + Failed, + /// There is no runner here to ask. Could-not-look, never a finding. + NoRunner, +} + /// One dispatched review. #[derive(Debug, Clone, PartialEq, Eq)] #[non_exhaustive] @@ -236,8 +257,16 @@ pub fn resolve( // `validator-verdict-clean` reads a record nothing ever writes // (CLOUD-1265). A hit costs a file read, so the agent runs once per // unique subject and every later landing lap is free. - if matches!(read(&path), Look::IsNot) { - dispatch(root, row, &path, &subject); + if matches!(read(&path), Look::IsNot) + && dispatch(root, row, &path, &subject) == Dispatch::NoRunner + { + // COULD NOT LOOK, FOR THE WHOLE FACT rather than for this row. + // `input.tree.review` is `null` and every gate over it goes quiet, + // which is the honest answer on a machine that has no reviewer: the + // question "was this reviewed" cannot be put where nothing can + // review. A per-row absence here would read as "this one was + // skipped", which is a different and false claim. + return Look::CouldNotLook; } let Look::Is(findings) = read(&path) else { continue; @@ -272,9 +301,14 @@ pub fn resolve( /// into a clean review — `secrets.rs`' invariant carried verbatim, **clean is /// never inferred from a stream that failed to parse**, and here the stakes are /// the whole gate rather than one finding. -fn dispatch(root: &Path, row: &crate::facts::ReviewQuery, path: &Path, subject_digest: &str) { +fn dispatch( + root: &Path, + row: &crate::facts::ReviewQuery, + path: &Path, + subject_digest: &str, +) -> Dispatch { let Some((text, _)) = prompt(&row.prompt) else { - return; + return Dispatch::NoRunner; }; // THE SUBJECT TRAVELS AS A POINTER, NEVER AS BYTES — Batten's law, not an // economy. @@ -297,55 +331,45 @@ fn dispatch(root: &Path, row: &crate::facts::ReviewQuery, path: &Path, subject_d // body, which legitimately has an issue key instead of a path. A pointer has // no such problem. let pointer = format!("{} {}\n", row.path, subject_digest); - #[expect( - clippy::disallowed_types, - reason = "stays: this fact IS Cost::Effect — resolving it dispatches the vendored prompt, which is the classification rather than an accident of it. A verb the caller must remember instead is the producer-writes-outside shape CLOUD-1265 measures dead (CLOUD-472)" - )] - let spawned = std::process::Command::new(&row.runner) - .args(&row.args) - .current_dir(root) - .stdin(std::process::Stdio::piped()) - // Both streams captured, NEITHER forwarded: an agent's stderr is prose, - // and echoing a child's stream would put output Batten never shaped onto - // Batten's own (rule 4). - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .spawn(); - let Ok(mut child) = spawned else { - return; - }; - // THE PROMPT AND THEN THE POINTER. The prompt ALONE was what this function - // sent before, and that was the defect: the agent was told what to look for - // and never told what to look at, so its answer was a review of nothing while - // the record was keyed to bytes it had never seen — a record that reads as a - // completed review and is not one, which is worse than no record at all. - if let Some(mut stdin) = child.stdin.take() { - use std::io::Write as _; - if stdin.write_all(text.as_bytes()).is_err() - || stdin.write_all(b"\n").is_err() - || stdin.write_all(pointer.as_bytes()).is_err() - { - return; - } - } - let Ok(output) = child.wait_with_output() else { - return; + // THROUGH `exec::piped`, WHICH IS THE PLACED CHILD-PROCESS ADAPTER, rather + // than a `Command::new` of this module's own. + // + // That helper exists for exactly this and says so: two callers had each grown + // their own spawn, "which is two spawns in two unplaced modules and one shape + // written twice". This was very nearly the third — the first attempt spawned + // directly and then placed `review` in `policy/spawn-adapters.rego`'s table, + // which is the move that rule's own remedy tells you not to make. Routing + // here instead means there is no new spawn site to place at all. + // + // It also buys the shebang resolution ladder, which a `#!/usr/bin/env` + // runner needs on a host where `CreateProcess` refuses an extensionless + // program — a refusal the caller would otherwise read as could-not-look. + // + // `None` is the runner not being here, which is the distinction the whole + // `Dispatch` enum exists to preserve. + let Some((code, stdout)) = crate::exec::piped( + root, + Path::new(&row.runner), + &row.args, + &format!("{text}\n{pointer}"), + ) else { + return Dispatch::NoRunner; }; // THE CROSS-CHECK. A non-zero status means the agent itself failed, and // findings parsed out of a failed run describe a review that did not finish. - if !output.status.success() { - return; + if code != 0 { + return Dispatch::Failed; } - let stdout = String::from_utf8_lossy(&output.stdout); let Some(body) = pointers_in(&stdout) else { - return; + return Dispatch::Failed; }; if let Some(parent) = path.parent() && std::fs::create_dir_all(parent).is_err() { - return; + return Dispatch::Failed; } let _ = std::fs::write(path, body); + Dispatch::Ran } /// Keep only the lines that are pointers, and refuse the stream if any line is diff --git a/crates/batten/tests/it/main.rs b/crates/batten/tests/it/main.rs index bf07c1e0e..ecf1bb372 100644 --- a/crates/batten/tests/it/main.rs +++ b/crates/batten/tests/it/main.rs @@ -171,6 +171,7 @@ mod refusal_ceiling; mod remedy_authorship; mod retirement_doctrine; mod review_answered; +mod review_dispatched; mod rule_cost_census; mod rules_builtin_claims; mod rules_drift; diff --git a/crates/batten/tests/it/review_dispatched.rs b/crates/batten/tests/it/review_dispatched.rs new file mode 100644 index 000000000..23813034e --- /dev/null +++ b/crates/batten/tests/it/review_dispatched.rs @@ -0,0 +1,325 @@ +//! `review-dispatched`, over the engine that builds its input (CLOUD-472). +//! +//! # The seam, and why the module's own suite cannot reach it +//! +//! `policy/review-dispatched.rego`'s `test_` rules pin the predicate against a +//! fabricated document. The question that decides whether this gate is alive is a +//! different one, and it has two halves the module cannot ask: +//! +//! * does the ENGINE dispatch the vendored prompt on a miss, and put what came +//! back at `input.tree.review` under the declared id? +//! * is the record KEYED so that a review of other bytes does not answer? +//! +//! The second is the one a `with input as` case actively hides: it fabricates the +//! map, so it fabricates the keying the whole fact turns on. A module suite would +//! pass identically over an engine that ignored the subject digest entirely. +//! +//! # The runner is a stub, and that is the whole contract +//! +//! `judge_kind.rs` states the doctrine for this repository and it carries here +//! unchanged: the engine's contract with a dispatched program is *what it writes +//! and what it exits*, so a stub that exits on demand exercises the whole of it. +//! Driving a real agent would make these cases a test of somebody's model rather +//! than of this wiring, and would make the failure arms — a non-zero exit, a +//! stream that is not pointers — unreachable, since you cannot ask a real agent to +//! misbehave on demand. + +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use crate::common; + +use std::fs; +use std::path::{Path, PathBuf}; + +use batten::rules::{self, Rule, RuleKind, RuleScope}; + +const RULE: &str = "review-dispatched"; +const REVIEW: &str = "ready-pressure-test"; +const SUBJECT: &str = "subject.md"; + +/// A fixture repository carrying a subject and a stub runner. +/// +/// `emits` is what the stub prints and `code` what it exits, so every arm of the +/// dispatch contract is reachable from one helper. +fn repo(name: &str, subject: &str, emits: &str, code: i32) -> PathBuf { + let root = common::scratch(name); + common::git_in(&root, &["init", "--quiet", "--initial-branch", "work"]); + common::git_in(&root, &["config", "user.email", "t@example.com"]); + common::git_in(&root, &["config", "user.name", "t"]); + fs::write(root.join(SUBJECT), subject).expect("the subject"); + common::git_in(&root, &["add", "-A"]); + common::git_in(&root, &["commit", "--quiet", "-m", "base"]); + // `origin/main` at the EMPTY tree, so the subject reads as added — the delta + // the row's narrowing asks about. A local ref rather than a fetch: the + // question is entirely local and a network round trip would make every case + // below depend on it. + let empty = common::git_in(&root, &["hash-object", "-t", "tree", "/dev/null"]); + let base = common::git_in(&root, &["commit-tree", empty.trim(), "-m", "empty"]); + common::git_in( + &root, + &["update-ref", "refs/remotes/origin/main", base.trim()], + ); + + install_module(&root); + stub(&root, emits, code); + root +} + +/// The stub runner, written where the row points and made executable. +/// +/// It records every invocation by appending to `calls`, which is what the +/// cache-hit case reads: "did not spawn again" is only checkable by counting +/// spawns, never by looking at the record, since a hit and a second identical +/// dispatch leave byte-identical records. +fn stub(root: &Path, emits: &str, code: i32) { + let path = root.join("runner.sh"); + fs::write( + &path, + format!( + "#!/bin/sh\ncat >/dev/null\necho x >> \"$(dirname \"$0\")/calls\"\nprintf '%s' {emits:?}\nexit {code}\n" + ), + ) + .expect("write the stub"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + fs::set_permissions(&path, fs::Permissions::from_mode(0o755)).expect("make it executable"); + } +} + +fn calls(root: &Path) -> usize { + fs::read_to_string(root.join("calls")).map_or(0, |text| text.lines().count()) +} + +fn install_module(root: &Path) { + let source = common::at_root("policy/review-dispatched.rego") + .canonicalize() + .expect("the committed module is where the row says it is"); + fs::create_dir_all(root.join("policy")).expect("scratch policy dir"); + fs::copy(source, root.join("policy/review-dispatched.rego")).expect("install committed module"); +} + +/// The committed row's shape, with the runner repointed at the stub. +/// +/// `declared` false builds the row with an EMPTY review list, which is how the +/// could-not-look arm is reached over the engine: no row declares the fact, so +/// the projection writes `null` rather than a map. +fn row(root: &Path, declared: bool) -> Rule { + let reviews = if declared { + serde_json::json!([{ + "id": REVIEW, + "prompt": REVIEW, + "runner": root.join("runner.sh").display().to_string(), + "version": "0", + "subject": "document", + "path": SUBJECT, + }]) + } else { + serde_json::json!([]) + }; + serde_json::from_value(serde_json::json!({ + "id": RULE, + "kind": "policy", + "scope": "tree", + "base": "origin/main", + "delta_sources": [SUBJECT], + "module": "policy/review-dispatched.rego", + "severity": "deny", + "review": reviews, + })) + .expect("the loader accepts the committed row's shape") +} + +fn verdicts_for(root: &Path, declared: bool) -> Vec { + let verdicts = common::verdicts_in(root); + rules::run_static( + &[row(root, declared)], + &[], + batten::policy::Vocabulary { + patterns: &[], + verdicts: &verdicts, + recorders: &[], + }, + root, + ) + .expect("the read surface runs a policy row") + .findings + .into_iter() + .map(|finding| finding.rule) + .collect() +} + +fn verdicts(root: &Path) -> Vec { + verdicts_for(root, true) +} + +// --------------------------------------------------------------------------- +// THE DISPATCH SEAM. +// --------------------------------------------------------------------------- + +/// The engine dispatches on a miss and the record reaches the predicate. Without +/// this the whole module is a `with input as` suite over a key nothing fills. +#[test] +fn a_dispatched_review_reaches_the_predicate_and_is_clean() { + let root = repo("review-dispatched-clean", "body\n", "", 0); + assert!( + verdicts(&root).is_empty(), + "the stub ran and wrote a record, so nothing is undispatched: {:?}", + verdicts(&root) + ); + assert_eq!(calls(&root), 1, "the miss dispatched exactly once"); +} + +/// A REVIEW THAT POINTED AT SOMETHING STILL RAN. Refusing here would price +/// finding something, and the cheapest way past such a gate is an agent that +/// reports nothing. +#[test] +fn a_review_that_reported_findings_is_still_clean() { + let root = repo("review-dispatched-findings", "body\n", "a.md 3 §7\n", 0); + assert!(verdicts(&root).is_empty(), "{:?}", verdicts(&root)); +} + +/// NO RUNNER HERE IS COULD-NOT-LOOK, NEVER A REFUSAL, and this is the case that +/// keeps the gate from being a verdict about the operator. +/// +/// A machine with no reviewer installed cannot be asked whether it reviewed. +/// Refusing here would fail every fixture, every fresh clone and every CI runner +/// that has not installed the agent — measured on this suite before the arm +/// existed, where it took four unrelated `cli.rs` cases red with it. +#[test] +fn a_missing_runner_is_could_not_look_and_never_a_refusal() { + let root = repo("review-dispatched-no-runner", "body\n", "", 0); + fs::remove_file(root.join("runner.sh")).expect("remove the runner"); + assert!( + verdicts(&root).is_empty(), + "an environment with no reviewer is unjudgeable, not guilty: {:?}", + verdicts(&root) + ); +} + +/// AND THE DELTA IS WHAT DECIDES WHETHER THE QUESTION IS PUT AT ALL. A branch +/// that did not touch the subject owes no review of it, so nothing is dispatched +/// and nothing is refused. +#[test] +fn a_branch_that_did_not_touch_the_subject_owes_no_review() { + let root = repo("review-dispatched-untouched", "body\n", "", 0); + // Move `origin/main` up to HEAD, so the subject is no longer in the delta. + let head = common::git_in(&root, &["rev-parse", "HEAD"]); + common::git_in( + &root, + &["update-ref", "refs/remotes/origin/main", head.trim()], + ); + assert!( + verdicts(&root).is_empty(), + "an untouched subject asks no question: {:?}", + verdicts(&root) + ); +} + +/// COULD NOT LOOK IS NOT A REFUSAL, and this arm is only reachable over the +/// engine: the fact is `null` when no row declared a review, which a fabricated +/// input cannot distinguish from a map that happens to be empty. +#[test] +fn a_review_the_engine_could_not_look_at_is_not_refused() { + let root = repo("review-dispatched-undeclared", "body\n", "", 0); + assert!( + verdicts_for(&root, false).is_empty(), + "a row declaring no review asked no question: {:?}", + verdicts_for(&root, false) + ); + assert_eq!(calls(&root), 0, "and nothing was dispatched"); +} + +// --------------------------------------------------------------------------- +// EVERY FAILURE LEAVES NO RECORD, so a broken agent and one that never ran are +// indistinguishable and both refuse. Writing an empty record on failure would +// turn a broken runner into a clean review. +// --------------------------------------------------------------------------- + +/// THE REFUSAL ARM. The runner is here and was asked, and it gave nothing usable +/// — which is the branch's problem rather than the environment's, and the one +/// state this gate exists to refuse. +#[test] +fn an_absent_record_is_refused_over_the_engines_own_projection() { + let root = repo("review-dispatched-red", "body\n", "", 1); + assert_eq!(verdicts(&root), vec![RULE.to_owned()]); +} + +/// CLEAN IS NEVER INFERRED FROM A STREAM THAT FAILED TO PARSE. An agent that +/// answered in prose has not produced pointers, and storing the subset that +/// happened to parse would be a silent partial answer. +#[test] +fn a_runner_that_answers_in_prose_leaves_no_record() { + let root = repo( + "review-dispatched-prose", + "body\n", + "I reviewed it and it looks fine.\n", + 0, + ); + assert_eq!(verdicts(&root), vec![RULE.to_owned()]); +} + +// --------------------------------------------------------------------------- +// THE KEYING, which is the half a `with input as` case actively hides. +// --------------------------------------------------------------------------- + +/// A record taken over other bytes does not answer. This is the anti-staleness +/// property the whole fact turns on, and the only tier that can see it. +#[test] +fn a_record_over_other_bytes_does_not_answer() { + let root = repo("review-dispatched-stale", "body\n", "", 0); + assert!(verdicts(&root).is_empty(), "the first run records"); + assert_eq!(calls(&root), 1); + + // The subject moves, so the composed key moves with it. + fs::write(root.join(SUBJECT), "edited\n").expect("edit the subject"); + assert!( + verdicts(&root).is_empty(), + "the edit re-dispatches rather than refusing" + ); + assert_eq!( + calls(&root), + 2, + "the record under the old digest did not answer for the new bytes" + ); +} + +/// AND A HIT DOES NOT RE-SPAWN, which is what makes an agent affordable inside a +/// gate that runs every landing lap. Only a spawn count can show it: a hit and a +/// second identical dispatch leave byte-identical records. +#[test] +fn an_unchanged_subject_is_a_cache_hit() { + let root = repo("review-dispatched-hit", "body\n", "", 0); + assert!(verdicts(&root).is_empty()); + assert!(verdicts(&root).is_empty()); + assert_eq!(calls(&root), 1, "the second run read the record"); +} + +/// ANTI-VACUITY over the whole file: the row this suite exercises is the one the +/// committed config declares, so a rename or a scope change reddens here rather +/// than leaving every case above passing over a module nothing runs. +#[test] +fn the_committed_row_is_the_one_these_cases_exercise() { + let committed = batten::config::load(&common::at_root("batten.toml")) + .expect("the committed config loads") + .rules; + let declared = committed + .iter() + .find(|rule| rule.id == RULE) + .expect("the committed config declares the row this suite exercises"); + assert_eq!(declared.kind, RuleKind::Policy); + assert_eq!(declared.scope, RuleScope::Tree); + let review = declared + .review + .first() + .expect("the committed row declares a review, or the gate reads a null fact"); + assert_eq!( + review.prompt, REVIEW, + "the prompt id must name a VENDORED prompt: an id nothing vendors resolves \ + to no text, so no key is composable and the row goes silently inert" + ); + assert!( + batten::review::prompt(&review.prompt).is_some(), + "the committed row's prompt id is one this binary vendors" + ); +} diff --git a/crates/batten/tests/it/verdict_vocabulary.rs b/crates/batten/tests/it/verdict_vocabulary.rs index e5d96d2ab..1a549ed0b 100644 --- a/crates/batten/tests/it/verdict_vocabulary.rs +++ b/crates/batten/tests/it/verdict_vocabulary.rs @@ -109,6 +109,7 @@ const CANDIDATES: &[&str] = &[ "point", "port", "program", + "prompt", "prose", "provision", "reach", diff --git a/mise.toml b/mise.toml index 1ba6d1b84..091936328 100644 --- a/mise.toml +++ b/mise.toml @@ -475,7 +475,7 @@ CI_FANIN_WORKFLOW = ".github/workflows/ci.yml" # which is a property of the world and belongs on a clock (`lock-complete`). REGORUS_OPA_COMPLIANCE = "1.2.0" REGORUS_OPA_COMPLIANCE_FOR = "0.11" -MUTANT_GATES = "alive,attestation-check,awk-regex-check,bats-invocation,batten-glob-check,board-diff-overlap,board-payloads,board-sweep,branch-age-check,cap-drift,ci-hygiene,ci-lease-precondition,ci-parity,ci-slow-needed,ci-suite-lane,ci-tools-check,claim-before-code,claim-race-check,claimed-keys,closing-key-check,coderabbit-config-check,commit-hygiene,connector-allow-guard,connector-allow-resolve,container-preflight,darwin-link,deferral-check,denials-outlive-the-turn,digest-major-agreement,doctor,done-check,done-pr-check,duplicate-close-check,evaluator-closure-check,evaluator-io-check,filed-here,finding-sink-check,forge-verdict-required,graph-check,harness-grant,harness-wiring,hk-fix-selection,hook-matcher-check,hook-pin-check,in-progress-drain,install-check,land,land-divergence-assert,land-lock,land-lock-check,landed-check,landing-loop,leased-push,license-table-check,linear-check,lock-complete,macos-link-check,mcp-allow-check,mcp-attach-check,mcp-timeout-budget,merged-pr-keys,mise-action-floor,mise-pin-agreement,module-map-check,msrv-pin-agreement,no-doctests,nonverdict-assert,ntia-check,obligations-bound,perf-assert,perf-compare,perf-gate,pinned-toolchain,pipefail-grep-check,plan-complete,pr-unsubscribed,privileged-lane,prose-only,publish-credential-check,ready-cites-check,ready-guard,ready-lint,reclaim-census,release-assets-check,release-due,release-tag-shape,release-tracking-check,released,remedy-authorship,report-only-check,review-answered,run-shape,rust-paths-check,sbom,sbom-inventory,serena-mcp,shell-hygiene,shell-retirement,shell-write-advisory,signing-posture,sonar-gate,spec-ref-check,stop-posture,stop-posture-check,suite-bench-check,suite-subject-retirable,task-substitution,timeout-check,token-bench-check,transcript-corpus-check,tree-clean,trunk-based,validator-verdict-clean,verdict-routes-resolve,verified,weakens-declared" +MUTANT_GATES = "alive,attestation-check,awk-regex-check,bats-invocation,batten-glob-check,board-diff-overlap,board-payloads,board-sweep,branch-age-check,cap-drift,ci-hygiene,ci-lease-precondition,ci-parity,ci-slow-needed,ci-suite-lane,ci-tools-check,claim-before-code,claim-race-check,claimed-keys,closing-key-check,coderabbit-config-check,commit-hygiene,connector-allow-guard,connector-allow-resolve,container-preflight,darwin-link,deferral-check,denials-outlive-the-turn,digest-major-agreement,doctor,done-check,done-pr-check,duplicate-close-check,evaluator-closure-check,evaluator-io-check,filed-here,finding-sink-check,forge-verdict-required,graph-check,harness-grant,harness-wiring,hk-fix-selection,hook-matcher-check,hook-pin-check,in-progress-drain,install-check,land,land-divergence-assert,land-lock,land-lock-check,landed-check,landing-loop,leased-push,license-table-check,linear-check,lock-complete,macos-link-check,mcp-allow-check,mcp-attach-check,mcp-timeout-budget,merged-pr-keys,mise-action-floor,mise-pin-agreement,module-map-check,msrv-pin-agreement,no-doctests,nonverdict-assert,ntia-check,obligations-bound,perf-assert,perf-compare,perf-gate,pinned-toolchain,pipefail-grep-check,plan-complete,pr-unsubscribed,privileged-lane,prose-only,publish-credential-check,ready-cites-check,ready-guard,ready-lint,reclaim-census,release-assets-check,release-due,release-tag-shape,release-tracking-check,released,remedy-authorship,report-only-check,review-answered,review-dispatched,run-shape,rust-paths-check,sbom,sbom-inventory,serena-mcp,shell-hygiene,shell-retirement,shell-write-advisory,signing-posture,sonar-gate,spec-ref-check,stop-posture,stop-posture-check,suite-bench-check,suite-subject-retirable,task-substitution,timeout-check,token-bench-check,transcript-corpus-check,tree-clean,trunk-based,validator-verdict-clean,verdict-routes-resolve,verified,weakens-declared" # --- GitHub reachability behind an egress proxy (Claude Code web sandbox etc.) --- # mise resolves every tool's release through GitHub's *API* host, api.github.com. diff --git a/policy/review-dispatched.rego b/policy/review-dispatched.rego new file mode 100644 index 000000000..5c80b7cc9 --- /dev/null +++ b/policy/review-dispatched.rego @@ -0,0 +1,186 @@ +# METADATA +# description: | +# A declared review ran over these bytes, or the branch does not land +# (CLOUD-472). +# +# THIS REFUSES ABSENCE, AND ABSENCE IS ALL IT MAY REFUSE. A declared id missing +# from `input.tree.review` was never dispatched — the prompt has not been shown +# to run over this subject — and that is a comparison of two digests, which is +# a thing a gate may decide. What the agent CONCLUDED is not, and refusing on +# it would be a model verdict wearing an exit code (non-negotiable rule 3). The +# fact is shaped so that cannot be written: `findings` are `{path, line, +# clause}` pointers with no field prose could occupy. +# +# THE OPPOSITE ARM FROM `forge-verdict-required`, DELIBERATELY. That module +# refuses only a PRESENT-and-red verdict and reads absence as could-not-look, +# because the forge is a third party that may legitimately not have judged yet. +# A review this branch was supposed to dispatch and did not is the branch's own +# conduct, so here absence is the finding and a present record is clean whatever +# it points at. Reading one module's posture as the family's is how a gate ends +# up refusing the wrong arm. +# +# THREE ANSWERS AND THE MODULE READS ALL THREE. `null` is could-not-look — +# no row declared a review, or no store is readable. A declared id ABSENT from +# the map has no record. An id PRESENT with an empty `findings` array ran and +# pointed at nothing, which is clean and must stay clean: a gate that refused it +# would price finding nothing, and the cheapest way past such a gate is an agent +# that invents a finding. +# +# WHY THE CHEAPER TIERS DO NOT SUBSTITUTE. `ready-lint` gates the SHAPE of a +# refinement block, and shape is what an author optimises against once the gate +# exists — the measured failure that opened CLOUD-472, where every clause was +# present and none had been pressure-tested. `obligations-bound` binds a §7 +# entry to a killer mutation, but only at implementation time; at refinement +# there is no code and no `#MUTANT` row to reach. A hash comparison is what +# better-shaped prose cannot satisfy, because the prose is the input to the +# hash. +# +# THE BRACKETS ARE NOT STYLE: the schema file carries a hyphen, so the dotted +# form is a parse error reported as `invalid schema reference`. +# THIS BLOCK IS YAML AND MUST STAY THE LAST COMMENT BLOCK BEFORE `package`. +# schemas: +# - input: schema["policy-input.schema"] +package batten.review_dispatched + +import rego.v1 + +rules contains "review-dispatched" + +# The reviews this repository declares it will not land without. +# +# The ids are the CONSUMER's, named here rather than derived from the fact: the +# fact carries what was dispatched, and a module that refused only over what it +# found could never refuse an absence — the one thing it exists to refuse. This +# is the same shape as `forge-verdict-required`'s `required := "final"`, and the +# same reason: the engine supplies what happened, the module decides what must. +required contains "ready-pressure-test" + +# Whether the engine could look at the review store at all. +# +# GUARDED on `is_object`: the key is `null` when nobody declared a review or no +# store is readable, and reaching into `null` is a hard evaluation FAULT in Rego +# rather than a silent miss — the failure `forge-verdict-required`'s own comment +# records. +looked if { + is_object(input.tree.review) +} + +# The branch's own diff, as the engine resolved it — the same reading +# `plan-complete` and `filed-here` take, and `null` when the base does not +# resolve, so `changed` stays empty and every arm goes quiet rather than +# fabricating a verdict. +delta := input.tree["base-delta"] + +changed contains path if { + some path in delta.added +} + +changed contains path if { + some path in delta.edited +} + +changed contains path if { + some path in delta.deleted +} + +# Every declared review with no record for the current subject. +# +# GATED ON THE ROW'S OWN DELTA, and without it this gate is unusable rather than +# merely noisy. The row declares `delta_sources` over the subject it reviews, so a +# non-empty delta means the thing to be reviewed CHANGED on this branch — which is +# when a review is owed. Ungated, the refusal fires on every checkout that has +# never dispatched, which is every fixture and every fresh clone: measured here, +# four `cli.rs` cases that only wanted to exercise other rules went red at once. +# That is the shape that gets a gate switched off, and `plan-complete` carries the +# identical narrowing for the identical reason. +# +# The digests stay the ENGINE's business: a record keyed to another prompt, or to +# bytes that have since changed, lives under a different name and never reaches +# the map. So "absent from the map" already means "not dispatched over what is +# here now", and a module re-deriving that would be a second authority over a key +# the engine composes. +undispatched contains id if { + looked + count(changed) > 0 + some id in required + not input.tree.review[id] +} + +violation contains { + "rule": "review-dispatched", + "verdict": "prompt run never", + "subjects": [{"artifact": id}], +} if { + some id in undispatched +} + +# --- the load-time tier ------------------------------------------------------ +# +# These pin the PREDICATE. They cannot pin that the ENGINE builds +# `input.tree.review` from a record the dispatch wrote — a `with input as` case +# fabricates the very shape the engine may be unable to produce, and here it +# would fabricate the KEYING the whole fact turns on. +# `crates/batten/tests/it/review_dispatched.rs` is that tier. + +touched := {"added": ["AGENTS.md"], "edited": [], "deleted": [], "code-changed": []} + +dispatched(findings) := {"tree": { + "base-delta": touched, + "review": {"ready-pressure-test": { + "provenance": {"tool": "stub", "version": "0", "invocation": [], "prompt": "p"}, + "subject": {"kind": "document", "digest": "d"}, + "findings": findings, + }}, +}} + +test_a_dispatched_review_is_clean if { + count(violation) == 0 with input as dispatched([]) +} + +# A REVIEW THAT POINTED AT SOMETHING IS STILL A REVIEW THAT RAN. Refusing here +# would price finding something, and the cheapest way past that gate is an agent +# that reports nothing — which is the incentive this module must not create. +test_a_review_with_findings_is_still_clean if { + count(violation) == 0 with input as dispatched([{"path": "a.md", "line": 1, "clause": "§7"}]) +} + +test_an_undispatched_review_is_refused if { + some v in violation with input as {"tree": {"base-delta": touched, "review": {}}} + v.verdict == "prompt run never" +} + +# THE REFUSAL NAMES WHICH REVIEW, so a reader is not left to work out which of +# several declared ids is missing. +test_the_refusal_names_the_review if { + ids := {v.subjects[0].artifact | some v in violation} with input as {"tree": {"base-delta": touched, "review": {}}} + ids == {"ready-pressure-test"} +} + +# A RECORD UNDER ANOTHER ID DOES NOT ANSWER. The map is keyed by the declared id, +# so a review of something else is not this one having run. +test_another_reviews_record_does_not_answer if { + some v in violation with input as {"tree": {"base-delta": touched, "review": {"other": {"findings": []}}}} + v.verdict == "prompt run never" +} + +# COULD-NOT-LOOK, and without the `is_object` guard this case does not merely +# fail — it faults, taking the whole bundle with it. +test_could_not_look_does_not_fault if { + count(violation) == 0 with input as {"tree": {"base-delta": touched, "review": null}} +} + +# A BRANCH THAT DID NOT TOUCH THE SUBJECT OWES NO REVIEW. Without this the gate +# refuses every checkout that has never dispatched — every fixture and every +# fresh clone — which is the shape that gets a gate switched off rather than +# satisfied. +test_an_untouched_subject_is_not_refused if { + count(violation) == 0 with input as {"tree": { + "base-delta": {"added": [], "edited": [], "deleted": [], "code-changed": []}, + "review": {}, + }} +} + +#MUTANT-SUITE crates/batten/tests/it/review_dispatched.rs +#MUTANT absence-unread|s@^\tnot input.tree.review\[id\]$@\tfalse@|an_absent_record_is_refused_over_the_engines_own_projection +#MUTANT could-not-look-refused|s@^\tlooked$@\ttrue@|a_missing_runner_is_could_not_look_and_never_a_refusal +#MUTANT untouched-subject-priced|s@^\tcount(changed) > 0$@\ttrue@|a_branch_that_did_not_touch_the_subject_owes_no_review From 08fd1b3c51ae3c65539132c66ca4284bb877dfb7 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 2 Sep 2026 14:21:31 +0000 Subject: [PATCH 05/16] feat(ready): a refinement block owes a dispatched pressure test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-472's originating ask, and the half the landing gate could not reach. A subagent verifies and pressure-tests a row BEFORE it enters the ready queue, because a deterministic confirmation that a particular prompt has run beats any amount of improved tooling — and none of the tooling could serve it. `obligations-bound` binds a §7 entry to a killer mutation but only at implementation time; at refinement there is no code, no case file and no `#MUTANT` row to bind to. Nothing gated this. WHAT IT ASKS THAT NO OTHER CLAUSE CAN. Every sibling in `ready::lint` reads the block and infers quality from its SHAPE, which is precisely what an author optimises against once the gate exists — the measured failure that opened this row, where every clause was present and none had been pressure-tested. This asks whether a PARTICULAR vendored prompt ran over these exact bytes. It is a comparison of two digests, so no model verdict reaches the exit code (rule 3), and better prose cannot satisfy it because the prose is the input to the hash. THE BODY IS THE TRACKER'S, which is the whole of what makes it worth anything. `payload.description` is what the tracker RETURNED — the same source the recorder's `verdict` authority column reads. A digest over a caller-assembled payload would certify whatever the caller typed, measured green three times against text in a local file, once under an id no row carried. COULD-NOT-LOOK PASSES THREE WAYS, and they are three different absences rather than one: no declared cutover (the consumer has not asked), a payload with no creation instant (unplaceable against one), and no runner installed (an environment that cannot be asked). Only a review that was ASKED FOR and did not answer refuses. `graph-check.sh` IS NOT EDITED, and could not be: it is governed by `shell-retirement`, whose only landable shapes are retire-whole or leave-alone. It already enforces `Todo => ready-lint exits 0`, so a finding here gates the ready queue for free. A RATCHET RATHER THAN A FLIP. `pressure_test_required_from` is a sibling of `prose_dialect_required_from` rather than a reuse of it, because the two convert different things and a consumer must be able to move one without the other — folding them would mean the day you demand a structured claims object is also the day you demand an agent dispatch. Without a cutover the flip refuses the standing Todo queue at once and takes the ready frontier dark, which is CLOUD-858's measured shape. `trust.rs` now compares BOTH cutovers from one table rather than two hand-written blocks: they are the same class of weakening — an instant moved later, or dropped — and the second block would be the first copied, which is what goes stale when a third lands. Extracting it was not optional either; the second cutover took `entry_weakenings` past its declared line budget, which is that budget working. `ReviewQuery.path` becomes OPTIONAL, because a refinement block has an issue key rather than a repository path. Demanding one would force a row to name a file it never reads. The tree surface skips a `document` review with no path rather than guessing, and `lib.rs` filters the body kind out of the grammar — so the two gates share one declaration and neither silently answers for the other's subject. THE POSITIVE CASE IS WHAT GIVES THE OTHERS MEANING. Four of the six assert an ABSENCE, and a suite of those passes trivially if the finding can never fire — the shape a dead gate and a clean tree share. `a_runner_that_answers_nothing_ usable_is_refused` drives a stub that exits non-zero and asserts the class appears at exit 2. It was nearly missed: the filter used to run these matched only four of the six by name, and 6/6 green was about to be read as coverage. Verification: `test:cargo` 3986/3986; `config lint` 0 smells; `mutate census` 115 gates, every one enforced or exempt. Refs: CLOUD-858, CLOUD-418, CLOUD-1265 Admits: 842945f1bb989e350d9e822beca3824c822285e88582b6601acfaee08d943334 Admits-rule: protected-mutation Admits-verdict: path write refused Admits-subject: batten.toml Admits-head: 83354819d887dd2284a0deb57b904b0fc6e2c502 Admits-epoch: 200899838d8b40ad40d533695eb90d00972cc6b93e62b7fbfaa498a73f4094ce Admits-author: alec@wenzowski.com Admits-prev: e03ad425042415b5ee8a05a3859b9fa0f13de28eae0df7c82bbf0620653b8ecf Admits-answer-lost: The grooming gate becomes unreachable. `ready::lint` filters the declared rows for `subject = "tracker-body"`, so with no such row the loop is empty and the arm decides nothing on every payload — a gate that compiles, tests green in its own fixtures, and refuses nothing in the real repository. That is the exact dead-gate shape this branch exists to remove, and CLOUD-1265 is it already happening on `tool-verdict`. Admits-answer-precondition: A `[[rule.review]]` sub-table is the one surface that can declare which review a repository owes, and `batten.toml` is the committed authority that holds it — there is no other place to say it, so writing the protected path directly is the only route. The write is additive, one row, and lands in this diff beside the `ready lint` arm that reads it and the six cases that drive it. Admits-answer-rejected-route: `config read first` names this case and does not apply: reading the config is how the sibling `document` row's shape was taken, and reading it again does not remove the need to WRITE a second row. `patch run first` rejected too — nothing derives or generates a review declaration, so a patch would be the same bytes through another door. --- batten.toml | 21 +++++ crates/batten/src/config.rs | 15 ++++ crates/batten/src/facts.rs | 11 ++- crates/batten/src/lib.rs | 29 ++++-- crates/batten/src/ready.rs | 78 ++++++++++++++++ crates/batten/src/review.rs | 56 +++++++++++- crates/batten/src/trust.rs | 75 +++++++++++----- crates/batten/tests/it/ready.rs | 154 ++++++++++++++++++++++++++++++++ schema/batten.local.schema.json | 10 ++- schema/batten.schema.json | 17 +++- 10 files changed, 427 insertions(+), 39 deletions(-) diff --git a/batten.toml b/batten.toml index 294d3489a..e263648da 100644 --- a/batten.toml +++ b/batten.toml @@ -5125,6 +5125,27 @@ version = "0" subject = "document" path = "AGENTS.md" +# THE GROOMING SUBJECT, and the reason it carries no `path` (CLOUD-472). +# +# A refinement block lives on the TRACKER, so there is nothing in the tree to +# digest: the subject is the description the tracker RETURNED, which reaches the +# engine on `ready lint`'s payload. That is the same forgery control the +# recorder's `verdict` authority column earned — a digest over a payload the +# caller assembled would certify whatever the caller typed, measured green three +# times against text in a local file, once under an id no row carried. +# +# THIS ROW IS READ BY `ready lint`, NOT BY `batten check`. The tree surface skips +# a review with no path rather than guessing one, and `lib.rs` filters this kind +# out for the grammar. Two subjects, one declaration, and neither gate can answer +# for the other: `review-dispatched` refuses at landing, `ready lint` refuses at +# the ready queue, and only the second can see a body that never touches disk. +[[rule.review]] +id = "ready-pressure-test-body" +prompt = "ready-pressure-test" +runner = "batten-review-runner" +version = "0" +subject = "tracker-body" + [[rule]] id = "validator-verdict-clean" kind = "policy" diff --git a/crates/batten/src/config.rs b/crates/batten/src/config.rs index 3c74f4a79..5672bd49e 100644 --- a/crates/batten/src/config.rs +++ b/crates/batten/src/config.rs @@ -590,6 +590,21 @@ pub struct Ready { /// creation time, so this assumes nothing about how a consumer spells a key. #[serde(default, skip_serializing_if = "Option::is_none")] pub prose_dialect_required_from: Option, + /// The instant from which a row owes a dispatched PRESSURE TEST (CLOUD-472). + /// + /// A sibling of the field above rather than a reuse of it, and the + /// separation is the point: these two ratchets convert different things and + /// a consumer must be able to move one without moving the other. Folding + /// them into one instant would mean the day you demand a structured claims + /// object is also the day you demand an agent dispatch, and a repository + /// that wants the first and cannot yet afford the second would have no way + /// to say so. + /// + /// Read exactly as its sibling is — fixed-width ISO-8601 UTC compared + /// lexically, absent is could-not-look, and moving it LATER is the only + /// direction that tightens. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub pressure_test_required_from: Option, } /// The `[trust]` table: what `--config-from` may do when the ref is unreachable. diff --git a/crates/batten/src/facts.rs b/crates/batten/src/facts.rs index 16821f922..c0ec750b3 100644 --- a/crates/batten/src/facts.rs +++ b/crates/batten/src/facts.rs @@ -3091,7 +3091,16 @@ pub struct ReviewQuery { /// Its digest is the second component of the key, so a review goes stale by /// construction the moment the subject changes — the anti-staleness half, and /// the one a `reviewed: true` marker could never provide. - pub path: String, + /// + /// **OPTIONAL, because not every subject is a file.** A `tracker-body` + /// review is taken over the description the TRACKER returned, which reaches + /// the engine through the refinement payload and has an issue key rather + /// than a repository path. Demanding a path there would force a row to name + /// a file it does not read, which is a declaration that means nothing and + /// would be compared to nothing. A `document` review without one is skipped + /// rather than guessed at. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub path: Option, } /// What joins a [`ToolQuery`]'s components into one record name. diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 16f1028ea..79790ad83 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -2798,13 +2798,32 @@ fn render_findings(findings: &[checks_green::Finding]) -> String { /// module exists to avoid. fn board_grammar(overrides: &Overrides) -> Result { let config = resolve::resolve(Path::new("."), overrides)?; - Ok( - ready::Grammar::resolve(&config.patterns)?.with_prose_threshold( + Ok(ready::Grammar::resolve(&config.patterns)? + .with_prose_threshold( config .ready - .and_then(|ready| ready.prose_dialect_required_from), - ), - ) + .as_ref() + .and_then(|ready| ready.prose_dialect_required_from.clone()), + ) + .with_pressure_test_threshold( + config + .ready + .as_ref() + .and_then(|ready| ready.pressure_test_required_from.clone()), + ) + // THE SUBJECT KIND IS FILTERED HERE, where the config is in hand. + // A `document` review is the tree surface's — `batten check` reads + // it off the disk — and a `tracker-body` one has no path to read, so + // only the second kind can be answered from a refinement payload. + .with_pressure_test_reviews( + config + .rules + .iter() + .flat_map(|rule| rule.review.iter()) + .filter(|row| row.subject == "tracker-body") + .cloned() + .collect(), + )) } fn run_claim( diff --git a/crates/batten/src/ready.rs b/crates/batten/src/ready.rs index d3a178427..fdb8f8df2 100644 --- a/crates/batten/src/ready.rs +++ b/crates/batten/src/ready.rs @@ -292,6 +292,16 @@ pub struct Grammar { /// lexical order IS chronological order. Moving it later is the only /// direction that tightens, which makes it a ratchet rather than a switch. prose_dialect_required_from: Option, + /// The instant from which a row owes a dispatched pressure test + /// (CLOUD-472), read exactly as the field above is. + pressure_test_required_from: Option, + /// The reviews a refinement block owes, as the consumer declared them. + /// + /// Carried here rather than re-read from config because `lint` is a pure + /// function of (grammar, payload, root) and must stay one: a second config + /// load inside it would be a second authority over which reviews are owed, + /// and the two could disagree for a caller that resolved with overrides. + pressure_test_reviews: Vec, bump_label: Regex, commit_type: Regex, bump_token: Regex, @@ -402,6 +412,29 @@ impl Grammar { self } + /// The cutover from which a row owes a dispatched pressure test. + /// + /// Absent on [`Self::from_compiled`]'s path for `with_prose_threshold`'s + /// reason: the recorder resolves a grammar to answer an `{authority:…}` + /// column and holds no consumer config, so it gets could-not-look rather + /// than a threshold guessed from somewhere else. + #[must_use] + pub fn with_pressure_test_threshold(mut self, from: Option) -> Self { + self.pressure_test_required_from = from; + self + } + + /// The declared reviews whose subject is a refinement body. + /// + /// Filtered by the CALLER rather than here: which subject kinds exist is + /// `review`'s vocabulary, and a grammar that knew the token would be a + /// second place to change when a kind is added. + #[must_use] + pub fn with_pressure_test_reviews(mut self, rows: Vec) -> Self { + self.pressure_test_reviews = rows; + self + } + /// A row the consumer's table does not declare. /// /// **Could-not-look, and it says so** — a clause whose anchor has no @@ -441,6 +474,8 @@ impl Grammar { relatedto_tail: find("ready-relatedto-tail")?, defer_verb: find("ready-defer-verb")?, prose_dialect_required_from: None, + pressure_test_required_from: None, + pressure_test_reviews: Vec::new(), key: find("ready-issue-key")?, closing_verb: find("ready-closing-verb")?, mention_markup: find("ready-issue-mention-markup")?, @@ -926,6 +961,49 @@ pub fn lint(grammar: &Grammar, payload: &Payload, root: &Path) -> Result }); } + // THE PRESSURE TEST, CONFIRMED RATHER THAN JUDGED (CLOUD-472). + // + // Every other clause here reads the block and infers quality from its SHAPE, + // which is what an author optimises against once the gate exists — the + // measured failure that opened this row, where every clause was present and + // none had been pressure-tested. This asks a different question that shape + // cannot answer: has a PARTICULAR vendored prompt run over these exact bytes? + // It is a comparison of two digests, so no model verdict reaches the exit + // code (rule 3), and it cannot be satisfied by better prose because the prose + // is the input to the hash. + // + // THE BODY IS THE TRACKER'S, which is the forgery control that makes this + // worth anything: `payload.description` is what the tracker RETURNED, the + // same source the recorder's `verdict` authority column reads. A digest over + // a self-assembled payload would certify whatever the caller typed — + // measured green three times against text in a local file, once under an id + // no row carried. + // + // COULD-NOT-LOOK PASSES, THREE WAYS OVER, and each is a different absence: + // no declared cutover (the consumer has not asked for this), a payload with + // no creation instant (unplaceable against one), and no runner installed (an + // environment that cannot be asked). `review::for_body` returns + // `CouldNotLook` for the third, so the only refusal left is a review that + // was asked for and did not answer. + if let Some(from) = grammar.pressure_test_required_from.as_deref() + && payload + .created_at + .as_deref() + .is_some_and(|created| created >= from) + { + for row in &grammar.pressure_test_reviews { + if matches!( + crate::review::for_body(root, row, &payload.description), + crate::facts::Look::IsNot + ) { + report.findings.push(Finding { + line: ready_start, + rule: format!("pressure-test-undispatched ({})", row.id), + }); + } + } + } + if !structured { check_bump(grammar, root, &block_lines, &line_of, &mut report)?; } diff --git a/crates/batten/src/review.rs b/crates/batten/src/review.rs index bf465544c..5fa480ec8 100644 --- a/crates/batten/src/review.rs +++ b/crates/batten/src/review.rs @@ -243,7 +243,14 @@ pub fn resolve( let Some((_, prompt_digest)) = prompt(&row.prompt) else { continue; }; - let Ok(bytes) = std::fs::read(root.join(&row.path)) else { + // A DOCUMENT REVIEW WITHOUT A PATH IS SKIPPED, never guessed at. The + // column is optional because a `tracker-body` review has an issue key + // instead, and `for_body` is that subject's entry point — reaching for a + // default here would take a review over bytes nobody declared. + let Some(subject_path) = row.path.as_deref() else { + continue; + }; + let Ok(bytes) = std::fs::read(root.join(subject_path)) else { // COULD NOT READ THE SUBJECT is could-not-look, and it must not // dispatch: a review keyed to bytes nobody could read would be a // record about a subject that does not exist. @@ -330,7 +337,13 @@ fn dispatch( // refuses the whole invocation — correct for a span, and fatal for a tracker // body, which legitimately has an issue key instead of a path. A pointer has // no such problem. - let pointer = format!("{} {}\n", row.path, subject_digest); + // THE POINTER NAMES THE SUBJECT, whatever kind it is: a repository path for + // a `document` review, and the row's own id for a `tracker-body` one, where + // the id IS the issue key the agent must look up. `row.id` is the fallback + // rather than a placeholder — a review with no path has a name, and a name is + // what a pointer is for. + let subject_name = row.path.as_deref().unwrap_or(&row.id); + let pointer = format!("{subject_name} {subject_digest}\n"); // THROUGH `exec::piped`, WHICH IS THE PLACED CHILD-PROCESS ADAPTER, rather // than a `Command::new` of this module's own. // @@ -401,3 +414,42 @@ fn pointers_in(stdout: &str) -> Option { format!("{}\n", lines.join("\n")) }) } + +/// Whether a vendored prompt has run over a BODY the caller holds, dispatching +/// on a miss. +/// +/// # Why this exists beside [`resolve`] rather than inside it +/// +/// [`resolve`] answers for the whole declared set at `check` time, over subjects +/// it can read off the disk. A refinement body is neither: it arrives on the +/// `ready lint` payload, from the TRACKER, and there is nothing in the tree to +/// read. Folding it into `resolve` would mean handing the tree surface a byte +/// string from a caller — a second way for a subject to enter the engine, and the +/// one a caller could forge. +/// +/// **The forgery control is the caller's, and it is the one already earned.** +/// `ready lint`'s payload is what the recorder's `verdict` authority column +/// reads, so the body here is what the tracker returned rather than a payload +/// somebody assembled — the distinction measured when `ready-lint` over a +/// self-assembled payload came back green three times against text in a local +/// file, once under an id no row carried. +#[must_use] +pub fn for_body(root: &Path, row: &crate::facts::ReviewQuery, body: &str) -> Look> { + let Some((_, prompt_digest)) = prompt(&row.prompt) else { + return Look::CouldNotLook; + }; + let Ok(git_dir) = crate::git::git_dir(root) else { + return Look::CouldNotLook; + }; + let subject = digest(body.as_bytes()); + let path = record_path(&git_dir, &row.id, &prompt_digest, &subject); + // SPAWN ON MISS, READ ON HIT, and NoRunner stays could-not-look — the same + // three answers the tree surface gives, because a machine with no reviewer + // installed cannot be asked whether it reviewed. + if matches!(read(&path), Look::IsNot) + && dispatch(root, row, &path, &subject) == Dispatch::NoRunner + { + return Look::CouldNotLook; + } + read(&path) +} diff --git a/crates/batten/src/trust.rs b/crates/batten/src/trust.rs index 64f277a45..763c1773c 100644 --- a/crates/batten/src/trust.rs +++ b/crates/batten/src/trust.rs @@ -1683,6 +1683,57 @@ fn mint_weakenings(base: &Config, working: &Config) -> Vec { found } +/// The `[ready]` cutovers, compared as one class. +/// +/// **BOTH CUTOVERS, ONE COMPARISON.** `[ready]` carries two independent ratchets +/// (CLOUD-472 added the second), and they are the same class of weakening: an +/// instant moved LATER, or dropped entirely, exempts rows that were owed +/// something. A per-field block would be this one copied, and the copy is what +/// goes stale when a third cutover lands — so the FIELD is data here and the +/// comparison is written once. +/// +/// Split out of [`entry_weakenings`] rather than inlined because the second +/// cutover took that function past its declared line budget, which is the budget +/// working: a function accumulating one self-contained block per config key is +/// exactly what it exists to interrupt. +fn cutover_weakenings(base: &Config, working: &Config) -> Vec { + type Cutover = (&'static str, fn(&Config) -> Option); + const CUTOVERS: &[Cutover] = &[ + ("ready.prose_dialect_required_from", |config| { + config + .ready + .as_ref() + .and_then(|ready| ready.prose_dialect_required_from.clone()) + }), + ("ready.pressure_test_required_from", |config| { + config + .ready + .as_ref() + .and_then(|ready| ready.pressure_test_required_from.clone()) + }), + ]; + + let mut found = Vec::new(); + for (field, cutover) in CUTOVERS { + let Some(was) = cutover(base) else { + continue; + }; + let now = cutover(working); + // Absent renders as the same could-not-look token every other + // three-valued read in this tree uses, so a reader of the finding sees + // WHICH move was made rather than an empty string. + if now.as_ref().is_none_or(|now| now > &was) { + found.push(Weakening::new( + WeakeningKind::ReadyCutoverRelaxed, + *field, + was, + now.unwrap_or_else(|| "-".to_owned()), + )); + } + } + found +} + fn entry_weakenings(base: &Config, working: &Config) -> Vec { let mut found = Vec::new(); @@ -1703,29 +1754,7 @@ fn entry_weakenings(base: &Config, working: &Config) -> Vec { // reads as could-not-look and exempts every row. Compared as strings because // both sides are fixed-width ISO-8601 UTC, which is the same reading // `policy/filed-here.rego` takes of a tracker stamp. - { - let cutover = |config: &Config| { - config - .ready - .as_ref() - .and_then(|ready| ready.prose_dialect_required_from.clone()) - }; - if let Some(was) = cutover(base) { - let now = cutover(working); - // Absent renders as the same could-not-look token every other - // three-valued read in this tree uses, so a reader of the finding - // sees WHICH move was made rather than an empty string. - let relaxed = now.as_ref().is_none_or(|now| now > &was); - if relaxed { - found.push(Weakening::new( - WeakeningKind::ReadyCutoverRelaxed, - "ready.prose_dialect_required_from", - was, - now.unwrap_or_else(|| "-".to_owned()), - )); - } - } - } + found.extend(cutover_weakenings(base, working)); // The mutating-verb table: a removed row un-gates a tool call at the // `PreToolUse` boundary, which is the most consequential of these. diff --git a/crates/batten/tests/it/ready.rs b/crates/batten/tests/it/ready.rs index 0d3a01b9e..dd6087abe 100644 --- a/crates/batten/tests/it/ready.rs +++ b/crates/batten/tests/it/ready.rs @@ -318,6 +318,160 @@ fn dated_payload(created_at: Option<&str>, description: &str) -> String { // 2026-09-01 the object was used by nothing at all. // --------------------------------------------------------------------------- +// --------------------------------------------------------------------------- +// THE PRESSURE-TEST CUTOVER (CLOUD-472), the SECOND ratchet on `[ready]`. +// +// Its own fixture rather than `with_tasks`, because the two cutovers are +// deliberately independent: a case that moved both at once could not tell which +// one refused, which is the whole reason they are separate fields. +// +// The runner is a STUB, on `judge_kind.rs`'s doctrine — the engine's contract +// with a dispatched program is what it writes and what it exits, so a stub +// exercises the whole of it, and it makes the no-runner arm reachable by simply +// not writing one. +// --------------------------------------------------------------------------- + +/// A fixture declaring the pressure-test cutover and one `tracker-body` review. +/// +/// `runner` is absolute so the row does not depend on `PATH`, and `with_runner` +/// false leaves the program absent — which is the could-not-look arm rather than +/// a refusal, and the case that keeps this gate from being a verdict about the +/// operator's machine. +fn with_pressure_test(name: &str, runner_exits: Option) -> PathBuf { + // BUILT FIRST, CONFIGURED SECOND. The row's `runner` must be an absolute + // path — a relative program is resolved against the PARENT's working + // directory rather than the child's, which is a footgun that would make this + // fixture pass or fail depending on where the suite was invoked from — and + // the path is not known until the fixture exists. + // A PLACEHOLDER CONFIG FIRST, because `base_commit` commits the tree and a + // fixture with no files has nothing to commit. The real config is written + // below, once the runner's absolute path exists to put in it. + let dir = Fixture::new(name) + .config("version = 1\n") + .git() + .base_commit() + .build(); + let runner = dir.join("runner.sh"); + common::write( + &dir, + "batten.toml", + &format!( + "version = 1\n\n[ready]\npressure_test_required_from = \"2026-06-01T00:00:00.000Z\"\n\n\ + [[rule]]\nid = \"review-dispatched\"\nkind = \"policy\"\nscope = \"tree\"\n\ + module = \"policy/review-dispatched.rego\"\nseverity = \"deny\"\n\n\ + [[rule.review]]\nid = \"ready-pressure-test-body\"\nprompt = \"ready-pressure-test\"\n\ + runner = \"{}\"\nversion = \"0\"\nsubject = \"tracker-body\"\n\n{}", + runner.display(), + declared_patterns() + ), + ); + if let Some(code) = runner_exits { + std::fs::write(&runner, format!("#!/bin/sh\ncat >/dev/null\nexit {code}\n")) + .expect("write the stub"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + std::fs::set_permissions(&runner, std::fs::Permissions::from_mode(0o755)) + .expect("make it executable"); + } + } + dir +} + +/// PAST THE CUTOVER WITH NO RECORD IS REFUSED — the gate's whole point, and the +/// question no other clause here can ask. Every sibling reads the block and +/// infers quality from its SHAPE; this asks whether a named prompt ran over +/// these exact bytes, which better-shaped prose cannot satisfy because the prose +/// is the input to the hash. +#[test] +fn a_row_past_the_pressure_test_cutover_owes_a_dispatch() { + let dir = with_pressure_test("ready-pressure-past-cutover", Some(0)); + // The stub exits 0 having printed nothing, which is a review that RAN and + // pointed at nothing — so the first lint records and the row is clean. + let output = lint(&dir, &dated_payload(Some(AFTER_CUTOVER), &block(""))); + assert!( + !stderr(&output).contains("pressure-test-undispatched"), + "the stub ran and recorded, so nothing is undispatched: {}", + stderr(&output) + ); +} + +/// THE REFUSAL FIRES, and this case is what gives every other one here meaning. +/// +/// The four cases around it assert an ABSENCE — that the class does not appear — +/// and a suite of those passes trivially if the finding can never fire at all, +/// which is the shape a dead gate and a clean tree share. Here the runner is +/// present and was asked, and it exited non-zero: the review was owed, it was +/// dispatched, and it did not answer. That is the branch's problem rather than +/// the environment's, and the one state this gate exists to refuse. +#[test] +fn a_runner_that_answers_nothing_usable_is_refused() { + let dir = with_pressure_test("ready-pressure-red-runner", Some(1)); + let output = lint(&dir, &dated_payload(Some(AFTER_CUTOVER), &block(""))); + assert!( + stderr(&output).contains("pressure-test-undispatched"), + "a dispatched review that gave nothing usable must refuse: {}", + stderr(&output) + ); + assert_eq!(code(&output), 2, "{}", stderr(&output)); +} + +/// BEFORE THE CUTOVER IS UNJUDGED. Without this the flip refuses the standing +/// Todo queue at once and takes the ready frontier dark — CLOUD-858's measured +/// shape, and the reason this is a ratchet rather than a switch. +#[test] +fn a_row_created_before_the_pressure_test_cutover_is_not_judged() { + let dir = with_pressure_test("ready-pressure-before-cutover", None); + let output = lint(&dir, &dated_payload(Some(BEFORE_CUTOVER), &block(""))); + assert!( + !stderr(&output).contains("pressure-test-undispatched"), + "a row predating the cutover owes nothing: {}", + stderr(&output) + ); +} + +/// A PAYLOAD WITH NO CREATION INSTANT CANNOT BE PLACED against a cutover, so it +/// is could-not-look. Reading absent as "past the cutover" would turn a verdict +/// about the payload into a verdict about the row. +#[test] +fn a_payload_with_no_creation_instant_is_not_judged() { + let dir = with_pressure_test("ready-pressure-undated", None); + let output = lint(&dir, &dated_payload(None, &block(""))); + assert!( + !stderr(&output).contains("pressure-test-undispatched"), + "an unplaceable row owes nothing: {}", + stderr(&output) + ); +} + +/// NO RUNNER IS COULD-NOT-LOOK, NEVER A REFUSAL. A machine with no reviewer +/// installed cannot be asked whether it reviewed, and refusing there would fail +/// every fresh clone and every CI runner that has not installed the agent — a +/// verdict about the operator wearing a verdict about the row. +#[test] +fn a_row_past_the_cutover_with_no_runner_is_not_judged() { + let dir = with_pressure_test("ready-pressure-no-runner", None); + let output = lint(&dir, &dated_payload(Some(AFTER_CUTOVER), &block(""))); + assert!( + !stderr(&output).contains("pressure-test-undispatched"), + "an environment with no reviewer is unjudgeable, not guilty: {}", + stderr(&output) + ); +} + +/// A CONSUMER THAT DECLARED NO CUTOVER HAS NOT ASKED FOR THIS, so the standing +/// fixture — which declares only the prose cutover — must never see the class. +#[test] +fn a_consumer_declaring_no_pressure_test_cutover_is_untouched() { + let dir = with_tasks("ready-pressure-undeclared"); + let output = lint(&dir, &dated_payload(Some(AFTER_CUTOVER), &block(""))); + assert!( + !stderr(&output).contains("pressure-test-undispatched"), + "an undeclared cutover asks nothing: {}", + stderr(&output) + ); +} + #[test] fn a_prose_block_past_the_cutover_is_refused() { let dir = with_tasks("ready-prose-past-cutover"); diff --git a/schema/batten.local.schema.json b/schema/batten.local.schema.json index c88b00a62..e213e2937 100644 --- a/schema/batten.local.schema.json +++ b/schema/batten.local.schema.json @@ -638,8 +638,11 @@ "type": "string" }, "path": { - "description": "The repository-relative path whose bytes the review was taken over.\n\nIts digest is the second component of the key, so a review goes stale by\nconstruction the moment the subject changes — the anti-staleness half, and\nthe one a `reviewed: true` marker could never provide.", - "type": "string" + "description": "The repository-relative path whose bytes the review was taken over.\n\nIts digest is the second component of the key, so a review goes stale by\nconstruction the moment the subject changes — the anti-staleness half, and\nthe one a `reviewed: true` marker could never provide.\n\n**OPTIONAL, because not every subject is a file.** A `tracker-body`\nreview is taken over the description the TRACKER returned, which reaches\nthe engine through the refinement payload and has an issue key rather\nthan a repository path. Demanding a path there would force a row to name\na file it does not read, which is a declaration that means nothing and\nwould be compared to nothing. A `document` review without one is skipped\nrather than guessed at.", + "type": [ + "string", + "null" + ] }, "prompt": { "description": "Which VENDORED prompt to dispatch, by id.\n\nVendored rather than a path, and that is the safety property: the text is\ncompiled into the binary, so its digest is a constant of the build and a\nconsumer cannot satisfy the gate by pointing it at an easier prompt.", @@ -664,8 +667,7 @@ "prompt", "runner", "version", - "subject", - "path" + "subject" ] }, "Rewrite": { diff --git a/schema/batten.schema.json b/schema/batten.schema.json index 87c291b64..1128a4a84 100644 --- a/schema/batten.schema.json +++ b/schema/batten.schema.json @@ -2356,6 +2356,13 @@ "description": "The `[ready]` table: the refinement gate's consumer-set thresholds.\n\n# Why a value and not a `[[pattern]]` row (CLOUD-472)\n\nThe first draft of the prose-dialect ratchet spelled its threshold as a\nregex over the exempt key range. That is wrong twice. The pattern registry\nexists so that one CONCEPT has one spelling — arithmetic is not a concept,\nand a range encoded in alternation is unreadable and unmovable. And it makes\nthe decision turn on key TEXT, which the tracker is known to rewrite: this\nconsumer already declares `ready-issue-mention-markup` because a bare key\ncomes back wrapped in `` markup, so matching key text is matching\nthe one thing the round trip mangles.", "type": "object", "properties": { + "pressure_test_required_from": { + "description": "The instant from which a row owes a dispatched PRESSURE TEST (CLOUD-472).\n\nA sibling of the field above rather than a reuse of it, and the\nseparation is the point: these two ratchets convert different things and\na consumer must be able to move one without moving the other. Folding\nthem into one instant would mean the day you demand a structured claims\nobject is also the day you demand an agent dispatch, and a repository\nthat wants the first and cannot yet afford the second would have no way\nto say so.\n\nRead exactly as its sibling is — fixed-width ISO-8601 UTC compared\nlexically, absent is could-not-look, and moving it LATER is the only\ndirection that tightens.", + "type": [ + "string", + "null" + ] + }, "prose_dialect_required_from": { "description": "From which creation instant a Ready block must carry the fenced claims\nobject rather than prose. An ISO-8601 UTC stamp, compared verbatim\nagainst the tracker's own `createdAt`.\n\n**A RATCHET: moving it later is the only direction that tightens.**\nAbsent is could-not-look and exempts everything, so a consumer that has\nnot opted in is never refused for a question it did not ask.\n\n**An instant rather than a key ordinal, and that is a portability\ndecision.** An ordinal reaches no consumer literal — the trailing digits,\nno separator assumed — so it passes `no-tracker-key-in-core`. It still\nrequires keys that are numeric AND monotonic with creation order, which\nthree popular trackers give and a slug- or UUID-keyed one does not, and it\nwould fail SILENTLY there rather than loudly. Every tracker stamps a\ncreation time, so this assumes nothing about how a consumer spells a key.", "type": [ @@ -2591,8 +2598,11 @@ "type": "string" }, "path": { - "description": "The repository-relative path whose bytes the review was taken over.\n\nIts digest is the second component of the key, so a review goes stale by\nconstruction the moment the subject changes — the anti-staleness half, and\nthe one a `reviewed: true` marker could never provide.", - "type": "string" + "description": "The repository-relative path whose bytes the review was taken over.\n\nIts digest is the second component of the key, so a review goes stale by\nconstruction the moment the subject changes — the anti-staleness half, and\nthe one a `reviewed: true` marker could never provide.\n\n**OPTIONAL, because not every subject is a file.** A `tracker-body`\nreview is taken over the description the TRACKER returned, which reaches\nthe engine through the refinement payload and has an issue key rather\nthan a repository path. Demanding a path there would force a row to name\na file it does not read, which is a declaration that means nothing and\nwould be compared to nothing. A `document` review without one is skipped\nrather than guessed at.", + "type": [ + "string", + "null" + ] }, "prompt": { "description": "Which VENDORED prompt to dispatch, by id.\n\nVendored rather than a path, and that is the safety property: the text is\ncompiled into the binary, so its digest is a constant of the build and a\nconsumer cannot satisfy the gate by pointing it at an easier prompt.", @@ -2617,8 +2627,7 @@ "prompt", "runner", "version", - "subject", - "path" + "subject" ] }, "Rewrite": { From 9195fc2be23d984f61f99109ed513519dbd536c2 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 2 Sep 2026 14:23:08 +0000 Subject: [PATCH 06/16] fix(review): the prompt went down a channel a real runner ignores MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-472. Reading a reviewer customers actually run refuted three things the dispatch assumed. None of them would have shown up in a record: all three produce a file byte-identical to a correct dispatch, which is why the suite was green over them. THE PROMPT WAS DISCARDED. A reviewer in this class takes its focus as a POSITIONAL argument and wires stdin only for a different subcommand, so a dispatch that writes the prompt to stdin runs UNSTEERED and exits zero — and the record then attests to a completed review of whatever the agent chose to look at. That is the same defect as the subject never being sent, one level out: the first told the agent what to look for and not what to look at, this told it neither. `prompt_arg` declares the channel, defaulting to stdin so a row that says nothing keeps the landed contract. THE TEST HAD TO ASSERT ON THE INVOCATION, because nothing in the record can show it. `last_call` reads the stub's argv. The assertion is on the subject POINTER rather than a phrase from the prompt — prose is edited freely, and the first version of this case failed against correct code because the prompt's heading is `# Pressure-test` and the assertion looked for `pressure-test`. READINESS IS THE RUNNER'S OWN ANSWER. Inferring "no runner" from a missing file cannot tell a machine that never installed the reviewer from one where it is installed and unauthenticated, and those are different remedies — the operator gets neither. `probe` declares a subcommand whose `ready` boolean decides could-not-look. Conservative in the direction that costs nothing: anything but an explicit `true` reads as not-ready, so a probe that cannot spawn, exits non-zero or answers in an unrecognised shape leaves the review unjudged rather than refusing a branch over an environment nobody can see. Parsed as JSON rather than scanned, so a `"ready": false` beside a remedy string mentioning the word cannot read as true. An undeclared probe skips the whole path. COMPLETION IS THE CONTRACT, NOT A PARSE. The gate refuses ABSENCE and never reads a finding, so demanding Batten's line format from the agent coupled the record to a schema nothing consumes — and made every reviewer that speaks its own into a failed dispatch, leaving no record and refusing the branch for somebody else's stdout. A run that exits 0 records, findings or not; a runner that does emit pointers still gets them stored. `a_runner_that_answers_in_prose_leaves_no_record` is retired rather than deleted quietly: it asserted the behaviour this reverses. What replaces it is sharper — a PERFECTLY PARSEABLE stream with a non-zero exit, which discriminates the two reasons a record can be absent where prose-plus-failure conflated them and passed under either rule. `secrets.rs`' invariant still holds where it belongs: a run that did not COMPLETE records nothing. NOTHING HERE NAMES THE REVIEWER, and the measurement that produced it is CLOUD-472's rather than this crate's (non-negotiable rule 1). No preset, no `[[provision]]` row and no pin: batten does not use that tool, and vendoring a consumer's runner contract into the core is the violation a well-meaning migration makes. Verification: `test:cargo` 3991/3991; `config lint` 0 smells; each new case run by its own name, since a substring filter silently matched four of six earlier in this session and 6/6 green was nearly read as coverage. Refs: CLOUD-1265 --- crates/batten/src/facts.rs | 67 +++++++ crates/batten/src/review.rs | 88 +++++++-- crates/batten/tests/it/review_dispatched.rs | 193 ++++++++++++++++++-- schema/batten.local.schema.json | 26 +++ schema/batten.schema.json | 26 +++ 5 files changed, 376 insertions(+), 24 deletions(-) diff --git a/crates/batten/src/facts.rs b/crates/batten/src/facts.rs index c0ec750b3..5aae9d969 100644 --- a/crates/batten/src/facts.rs +++ b/crates/batten/src/facts.rs @@ -3084,6 +3084,37 @@ pub struct ReviewQuery { /// The exact flags, so a reader can tell which question was asked. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub args: Vec, + /// How the prompt reaches the runner. + /// + /// **Declared because a runner that ignores the channel discards the prompt + /// SILENTLY**, and the record then reads as a completed review of whatever + /// the agent chose to look at. Measured against a real reviewer whose review + /// subcommand takes its focus as a POSITIONAL and wires stdin only for a + /// different subcommand: the dispatch succeeds, exits zero and steers + /// nothing. Which reviewer, and the reading, are CLOUD-472's — a consumer's + /// tool is not this crate's to name (non-negotiable rule 1). + /// + /// Defaults to stdin, which is what the first landed runner contract + /// assumed, so a row that says nothing keeps its behaviour. + #[serde(default, skip_serializing_if = "PromptArg::is_default")] + pub prompt_arg: PromptArg, + /// A subcommand that answers whether this runner can review AT ALL. + /// + /// **Readiness as a declared reading rather than an inference.** Without it + /// the engine infers "no runner" from a missing file, which cannot tell a + /// machine that never installed the reviewer from one where it is installed + /// and not authenticated — different remedies, and the operator gets neither. + /// + /// The runner is the authority on its own readiness: it knows whether its + /// dependencies resolve and whether a session is authenticated, and the + /// engine can see neither. Reviewers in this class expose exactly such a + /// subcommand, reporting a boolean beside the remedy steps; the row names + /// which one, because whose reviewer it is is the consumer's business. + /// + /// Absent keeps today's behaviour, so a row that declares none is judged + /// exactly as it was before this column existed. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub probe: Vec, /// What KIND of thing is being reviewed, e.g. `document`. pub subject: String, /// The repository-relative path whose bytes the review was taken over. @@ -3103,6 +3134,42 @@ pub struct ReviewQuery { pub path: Option, } +/// How a dispatched prompt reaches its runner (CLOUD-472). +/// +/// Two spellings rather than a bool, because the axis is not "on or off" — it is +/// WHICH channel, and a third (a temporary file, say) is a plausible fourth +/// runner's answer. A bool would have to be renamed to admit one. +#[derive( + Debug, + Clone, + Copy, + PartialEq, + Eq, + Default, + serde::Deserialize, + serde::Serialize, + schemars::JsonSchema, +)] +#[serde(rename_all = "kebab-case")] +pub enum PromptArg { + /// Written to the child's stdin. The default, and what the first landed + /// runner contract assumed. + #[default] + Stdin, + /// Appended to `args` as the last positional. What a runner taking a focus + /// string needs — and what the reviewer batten's customers run requires. + Positional, +} + +impl PromptArg { + /// Whether this is the serde default, so a row that says nothing stays quiet + /// in the emitted config rather than growing a key nobody wrote. + #[must_use] + pub fn is_default(&self) -> bool { + *self == PromptArg::Stdin + } +} + /// What joins a [`ToolQuery`]'s components into one record name. /// /// Stated once here rather than spelled at both the composing and the validating diff --git a/crates/batten/src/review.rs b/crates/batten/src/review.rs index 5fa480ec8..429b5063b 100644 --- a/crates/batten/src/review.rs +++ b/crates/batten/src/review.rs @@ -134,14 +134,47 @@ pub struct Subject { enum Dispatch { /// It ran and a record was written. Ran, - /// It ran and gave nothing usable — non-zero, or a stream that is not - /// pointers. The review was ASKED FOR and did not answer, which is the - /// branch's problem and must refuse. + /// It ran and did not complete — a non-zero exit. The review was ASKED FOR + /// and did not answer, which is the branch's problem and must refuse. + /// + /// **A stream that is not pointers is NOT this**, and used to be: completion + /// is the whole contract the gate reads, so demanding the agent speak + /// Batten's line format made every real reviewer's output a failed dispatch. Failed, - /// There is no runner here to ask. Could-not-look, never a finding. + /// There is no runner here to ask, or its own probe says it cannot review — + /// not installed, or installed and unauthenticated. Could-not-look, never a + /// finding: this is a fact about the machine rather than about the branch. NoRunner, } +/// Whether the runner's own probe says it can review here. +/// +/// **The runner is the authority on its own readiness**, which is why this is a +/// declared subcommand rather than a check Batten invents. A reviewer knows +/// whether its CLI is installed AND whether a session is authenticated; the +/// engine can see neither, and inferring readiness from a file's existence +/// answers a narrower question than the one that matters. +/// +/// Conservative in the direction that costs nothing: anything other than an +/// explicit `"ready": true` reads as NOT ready, so a probe that fails to spawn, +/// exits non-zero, or answers in a shape this does not recognise leaves the +/// review unjudged rather than refusing a branch over an environment nobody can +/// see. `serde_json` rather than a substring scan, so a `"ready": false` beside +/// a `"nextSteps"` mentioning the word cannot read as true. +fn ready(root: &Path, row: &crate::facts::ReviewQuery) -> bool { + let Some((code, stdout)) = crate::exec::piped(root, Path::new(&row.runner), &row.probe, "") + else { + return false; + }; + if code != 0 { + return false; + } + serde_json::from_str::(&stdout) + .ok() + .and_then(|value| value.get("ready").and_then(serde_json::Value::as_bool)) + .unwrap_or(false) +} + /// One dispatched review. #[derive(Debug, Clone, PartialEq, Eq)] #[non_exhaustive] @@ -360,12 +393,31 @@ fn dispatch( // // `None` is the runner not being here, which is the distinction the whole // `Dispatch` enum exists to preserve. - let Some((code, stdout)) = crate::exec::piped( - root, - Path::new(&row.runner), - &row.args, - &format!("{text}\n{pointer}"), - ) else { + // THE PROBE FIRST, so "cannot review here" is a READING rather than an + // inference from a missing file. + // + // `is_file()` alone cannot tell a machine that never installed the reviewer + // from one where it is installed and unauthenticated, and those are different + // remedies for the operator. A declared probe answers it: the reviewer + // batten's customers run reports `ready` alongside `node`, `npm`, `codex` and + // `auth`, with `nextSteps` attached. An undeclared probe skips this entirely, + // so a row that says nothing is judged exactly as before. + if !row.probe.is_empty() && !ready(root, row) { + return Dispatch::NoRunner; + } + // THE PROMPT GOES DOWN THE CHANNEL THE RUNNER ACTUALLY READS. A runner that + // ignores stdin discards it in SILENCE and reviews whatever it chose to look + // at, so the record would attest to a review that never asked the question. + let mut args = row.args.clone(); + let stdin = match row.prompt_arg { + crate::facts::PromptArg::Stdin => format!("{text}\n{pointer}"), + crate::facts::PromptArg::Positional => { + args.push(format!("{text}\n{pointer}")); + String::new() + } + }; + let Some((code, stdout)) = crate::exec::piped(root, Path::new(&row.runner), &args, &stdin) + else { return Dispatch::NoRunner; }; // THE CROSS-CHECK. A non-zero status means the agent itself failed, and @@ -373,9 +425,19 @@ fn dispatch( if code != 0 { return Dispatch::Failed; } - let Some(body) = pointers_in(&stdout) else { - return Dispatch::Failed; - }; + // COMPLETION IS THE CONTRACT, AND THE FINDINGS ARE A BONUS. + // + // The gate over this fact refuses ABSENCE and never reads a finding, so + // demanding the agent speak Batten's line format couples the record to a + // schema nothing consumes — and guarantees that a real reviewer's output is + // rejected as unparseable, leaving no record and a refusal that blames the + // branch for somebody else's stdout. A run that exited 0 reviewed the + // subject; that is what is being attested. + // + // A runner that DOES emit pointers still gets them stored, so a consumer who + // wires one loses nothing. What changed is that failing to is no longer a + // failed dispatch. + let body = pointers_in(&stdout).unwrap_or_default(); if let Some(parent) = path.parent() && std::fs::create_dir_all(parent).is_err() { diff --git a/crates/batten/tests/it/review_dispatched.rs b/crates/batten/tests/it/review_dispatched.rs index 23813034e..91b3c5f76 100644 --- a/crates/batten/tests/it/review_dispatched.rs +++ b/crates/batten/tests/it/review_dispatched.rs @@ -76,10 +76,18 @@ fn stub(root: &Path, emits: &str, code: i32) { fs::write( &path, format!( - "#!/bin/sh\ncat >/dev/null\necho x >> \"$(dirname \"$0\")/calls\"\nprintf '%s' {emits:?}\nexit {code}\n" + "#!/bin/sh\n\ + if [ \"$1\" = setup ]; then printf '%s' \"$(cat \"$(dirname \"$0\")/ready\")\"; exit 0; fi\n\ + cat >/dev/null\n\ + echo \"$@\" >> \"$(dirname \"$0\")/calls\"\n\ + printf '%s' {emits:?}\n\ + exit {code}\n" ), ) .expect("write the stub"); + // The probe's answer lives beside the stub so a case can set it without + // rewriting the program: readiness is what varies, not the runner. + fs::write(root.join("ready"), r#"{"ready": true}"#).expect("write the probe answer"); #[cfg(unix)] { use std::os::unix::fs::PermissionsExt as _; @@ -245,18 +253,25 @@ fn an_absent_record_is_refused_over_the_engines_own_projection() { assert_eq!(verdicts(&root), vec![RULE.to_owned()]); } -/// CLEAN IS NEVER INFERRED FROM A STREAM THAT FAILED TO PARSE. An agent that -/// answered in prose has not produced pointers, and storing the subset that -/// happened to parse would be a silent partial answer. +/// THE EXIT STATUS DECIDES, NOT THE PARSE — and this case is what tells them +/// apart, because it hands the runner a PERFECTLY PARSEABLE stream and a +/// non-zero exit. +/// +/// It replaces a case that asserted the opposite: a prose answer used to be a +/// failed dispatch, on `secrets.rs`' invariant that clean is never inferred from +/// a stream that failed to parse. That invariant still holds where it belongs — +/// a run that did not COMPLETE records nothing — but it was reaching too far. +/// The gate refuses absence and never reads a finding, so demanding Batten's +/// line format from the agent made every reviewer that speaks its own into a +/// failure, and refused the branch for somebody else's stdout. #[test] -fn a_runner_that_answers_in_prose_leaves_no_record() { - let root = repo( - "review-dispatched-prose", - "body\n", - "I reviewed it and it looks fine.\n", - 0, +fn a_runner_that_exits_non_zero_leaves_no_record_even_with_clean_output() { + let root = repo("review-red-but-parseable", "body\n", "a.md 3 §7\n", 1); + assert_eq!( + verdicts(&root), + vec![RULE.to_owned()], + "completion is the contract, and this run did not complete" ); - assert_eq!(verdicts(&root), vec![RULE.to_owned()]); } // --------------------------------------------------------------------------- @@ -323,3 +338,159 @@ fn the_committed_row_is_the_one_these_cases_exercise() { "the committed row's prompt id is one this binary vendors" ); } + +/// The last argument the stub was invoked with, or the empty string. +/// +/// This is what proves the prompt REACHED the runner. Nothing in the record can +/// show it: a review dispatched with the prompt discarded writes a record +/// byte-identical to one dispatched with it delivered, which is exactly why the +/// defect this case exists for survived being tested. +fn last_call(root: &Path) -> String { + fs::read_to_string(root.join("calls")).unwrap_or_default() +} + +fn row_with(root: &Path, extra: &serde_json::Value) -> Rule { + let mut review = serde_json::json!({ + "id": REVIEW, + "prompt": REVIEW, + "runner": root.join("runner.sh").display().to_string(), + "version": "0", + "subject": "document", + "path": SUBJECT, + }); + let (Some(base), Some(more)) = (review.as_object_mut(), extra.as_object()) else { + panic!("both are objects"); + }; + for (key, value) in more { + base.insert(key.clone(), value.clone()); + } + serde_json::from_value(serde_json::json!({ + "id": RULE, + "kind": "policy", + "scope": "tree", + "base": "origin/main", + "delta_sources": [SUBJECT], + "module": "policy/review-dispatched.rego", + "severity": "deny", + "review": [review], + })) + .expect("the loader accepts the row") +} + +fn verdicts_with(root: &Path, extra: &serde_json::Value) -> Vec { + let verdicts = common::verdicts_in(root); + rules::run_static( + &[row_with(root, extra)], + &[], + batten::policy::Vocabulary { + patterns: &[], + verdicts: &verdicts, + recorders: &[], + }, + root, + ) + .expect("the read surface runs a policy row") + .findings + .into_iter() + .map(|finding| finding.rule) + .collect() +} + +// --------------------------------------------------------------------------- +// THE PROMPT REACHES THE RUNNER, which no record can attest to. +// --------------------------------------------------------------------------- + +/// A POSITIONAL runner is handed the prompt as an argument. +/// +/// Measured against a real reviewer whose review subcommand takes focus as a +/// POSITIONAL and wires stdin only for a different subcommand: a prompt sent down +/// stdin is discarded in silence and the review runs unsteered, exiting zero. The +/// record is identical either way — only the invocation shows it. +#[test] +fn a_positional_runner_is_handed_the_prompt_as_an_argument() { + let root = repo("review-prompt-positional", "body\n", "", 0); + assert!( + verdicts_with(&root, &serde_json::json!({"prompt_arg": "positional"})).is_empty(), + "the dispatch completed" + ); + // THE POINTER, not a phrase from the prompt: the prose is edited freely and + // an assertion on it breaks for a reason that has nothing to do with the + // channel. The subject pointer is the last thing written into the payload, so + // finding it in argv proves the WHOLE payload arrived positionally. + assert!( + last_call(&root).contains(SUBJECT), + "the prompt and its pointer must arrive as an argument: {:?}", + last_call(&root) + ); +} + +/// AND THE DEFAULT STILL SENDS IT ON STDIN, so the landed contract is unchanged +/// for a row that declares nothing. +#[test] +fn a_row_declaring_no_channel_still_sends_the_prompt_on_stdin() { + let root = repo("review-prompt-stdin", "body\n", "", 0); + assert!(verdicts(&root).is_empty(), "the dispatch completed"); + assert!( + !last_call(&root).contains(SUBJECT), + "the default channel is stdin, so no argument carries the payload: {:?}", + last_call(&root) + ); +} + +// --------------------------------------------------------------------------- +// READINESS IS THE RUNNER'S OWN ANSWER. +// --------------------------------------------------------------------------- + +/// A runner that is INSTALLED and says it cannot review is could-not-look. +/// +/// This is the arm `is_file()` cannot reach: the program is right there, so +/// every file-existence check says "runner present", and the branch would be +/// refused for an environment that is simply not authenticated. +#[test] +fn a_runner_whose_probe_says_it_is_not_ready_is_could_not_look() { + let root = repo("review-probe-unready", "body\n", "", 0); + fs::write( + root.join("ready"), + r#"{"ready": false, "nextSteps": ["authenticate the reviewer"]}"#, + ) + .expect("the probe answers not-ready"); + assert!( + verdicts_with(&root, &serde_json::json!({"probe": ["setup", "--json"]})).is_empty(), + "an unauthenticated reviewer is unjudgeable, not guilty" + ); + assert_eq!(calls(&root), 0, "and nothing was dispatched"); +} + +/// A ready runner dispatches, so the probe is a gate rather than a wall. +#[test] +fn a_runner_whose_probe_says_it_is_ready_dispatches() { + let root = repo("review-probe-ready", "body\n", "", 0); + assert!( + verdicts_with(&root, &serde_json::json!({"probe": ["setup", "--json"]})).is_empty(), + "the dispatch completed" + ); + assert_eq!(calls(&root), 1, "the probe passed and the review ran"); +} + +// --------------------------------------------------------------------------- +// COMPLETION IS THE CONTRACT. +// --------------------------------------------------------------------------- + +/// A REAL REVIEWER'S OUTPUT IS NOT A FAILED DISPATCH. The gate refuses absence +/// and never reads a finding, so demanding Batten's line format would reject +/// every reviewer that speaks its own — leaving no record, and refusing the +/// branch for somebody else's stdout. +#[test] +fn a_runner_that_answers_in_its_own_format_still_records() { + let root = repo( + "review-foreign-format", + "body\n", + "{\"exitStatus\":0,\"payload\":{\"findings\":[]}}\n", + 0, + ); + assert!( + verdicts(&root).is_empty(), + "a completed review records whatever it said: {:?}", + verdicts(&root) + ); +} diff --git a/schema/batten.local.schema.json b/schema/batten.local.schema.json index e213e2937..4be2f516f 100644 --- a/schema/batten.local.schema.json +++ b/schema/batten.local.schema.json @@ -531,6 +531,21 @@ } ] }, + "PromptArg": { + "description": "How a dispatched prompt reaches its runner (CLOUD-472).\n\nTwo spellings rather than a bool, because the axis is not \"on or off\" — it is\nWHICH channel, and a third (a temporary file, say) is a plausible fourth\nrunner's answer. A bool would have to be renamed to admit one.", + "oneOf": [ + { + "description": "Written to the child's stdin. The default, and what the first landed\nrunner contract assumed.", + "type": "string", + "const": "stdin" + }, + { + "description": "Appended to `args` as the last positional. What a runner taking a focus\nstring needs — and what the reviewer batten's customers run requires.", + "type": "string", + "const": "positional" + } + ] + }, "ReceiptKey": { "description": "Which git fact a receipt is keyed to, and therefore what invalidates it.\n\nThe distinction is not a tuning knob, it is what the receipt *attests*.\nA `head` receipt claims something about those exact bytes, so an amend or a\nrebase must expire it. A `branch` receipt claims a decision about the work,\nwhich every commit on the branch continues to serve, so a SHA-keyed one\nwould demand a re-claim per commit — the false-positive rate that gets a\nguard bypassed. Both spellings are carried from the shell layer that proved\nthem (`ready-guard` keys by SHA, `claim-check` by branch).\n\n**`ValueEnum` because the CLI selects the same keying** (CLOUD-741). A\n`receipt` rule is pinned to [`RuleScope::MediatedCall`], so `batten check`\ncan never evaluate one and `verify` cannot reach this predicate through the\nengine — which left `verify` re-implementing it in shell, weakly enough that\nCLOUD-516's own incident passed. `receipt status --key branch` is how the\ntree surface reaches the one implementation instead, so config and CLI must\nname the keying with the same tokens or the two surfaces disagree about what\nthey asked for. `clap`'s and serde's renames both land on `head`/`branch`;\nthe `clap(rename_all)` is stated rather than inferred so a future variant\ncannot drift them apart.", "oneOf": [ @@ -644,10 +659,21 @@ "null" ] }, + "probe": { + "description": "A subcommand that answers whether this runner can review AT ALL.\n\n**Readiness as a declared reading rather than an inference.** Without it\nthe engine infers \"no runner\" from a missing file, which cannot tell a\nmachine that never installed the reviewer from one where it is installed\nand not authenticated — different remedies, and the operator gets neither.\n\nThe runner is the authority on its own readiness: it knows whether its\ndependencies resolve and whether a session is authenticated, and the\nengine can see neither. Reviewers in this class expose exactly such a\nsubcommand, reporting a boolean beside the remedy steps; the row names\nwhich one, because whose reviewer it is is the consumer's business.\n\nAbsent keeps today's behaviour, so a row that declares none is judged\nexactly as it was before this column existed.", + "type": "array", + "items": { + "type": "string" + } + }, "prompt": { "description": "Which VENDORED prompt to dispatch, by id.\n\nVendored rather than a path, and that is the safety property: the text is\ncompiled into the binary, so its digest is a constant of the build and a\nconsumer cannot satisfy the gate by pointing it at an easier prompt.", "type": "string" }, + "prompt_arg": { + "description": "How the prompt reaches the runner.\n\n**Declared because a runner that ignores the channel discards the prompt\nSILENTLY**, and the record then reads as a completed review of whatever\nthe agent chose to look at. Measured against a real reviewer whose review\nsubcommand takes its focus as a POSITIONAL and wires stdin only for a\ndifferent subcommand: the dispatch succeeds, exits zero and steers\nnothing. Which reviewer, and the reading, are CLOUD-472's — a consumer's\ntool is not this crate's to name (non-negotiable rule 1).\n\nDefaults to stdin, which is what the first landed runner contract\nassumed, so a row that says nothing keeps its behaviour.", + "$ref": "#/$defs/PromptArg" + }, "runner": { "description": "The agent to dispatch it through.", "type": "string" diff --git a/schema/batten.schema.json b/schema/batten.schema.json index 1128a4a84..29b573743 100644 --- a/schema/batten.schema.json +++ b/schema/batten.schema.json @@ -2192,6 +2192,21 @@ "path" ] }, + "PromptArg": { + "description": "How a dispatched prompt reaches its runner (CLOUD-472).\n\nTwo spellings rather than a bool, because the axis is not \"on or off\" — it is\nWHICH channel, and a third (a temporary file, say) is a plausible fourth\nrunner's answer. A bool would have to be renamed to admit one.", + "oneOf": [ + { + "description": "Written to the child's stdin. The default, and what the first landed\nrunner contract assumed.", + "type": "string", + "const": "stdin" + }, + { + "description": "Appended to `args` as the last positional. What a runner taking a focus\nstring needs — and what the reviewer batten's customers run requires.", + "type": "string", + "const": "positional" + } + ] + }, "Provision": { "description": "One provisioned tool.\n\nThe `oneOf` mirrors [`validate_artifact_spelling`]'s xor into the derived\nschema, the same way [`crate::rules::Rule`] mirrors its severity conditional:\nan author editing `batten.toml` against the published schema gets the refusal\nin their editor rather than on the next run. It is a **second expression of\none rule, never a second authority** — the loader's check is what decides, and\nit is what the error message comes from.", "type": "object", @@ -2604,10 +2619,21 @@ "null" ] }, + "probe": { + "description": "A subcommand that answers whether this runner can review AT ALL.\n\n**Readiness as a declared reading rather than an inference.** Without it\nthe engine infers \"no runner\" from a missing file, which cannot tell a\nmachine that never installed the reviewer from one where it is installed\nand not authenticated — different remedies, and the operator gets neither.\n\nThe runner is the authority on its own readiness: it knows whether its\ndependencies resolve and whether a session is authenticated, and the\nengine can see neither. Reviewers in this class expose exactly such a\nsubcommand, reporting a boolean beside the remedy steps; the row names\nwhich one, because whose reviewer it is is the consumer's business.\n\nAbsent keeps today's behaviour, so a row that declares none is judged\nexactly as it was before this column existed.", + "type": "array", + "items": { + "type": "string" + } + }, "prompt": { "description": "Which VENDORED prompt to dispatch, by id.\n\nVendored rather than a path, and that is the safety property: the text is\ncompiled into the binary, so its digest is a constant of the build and a\nconsumer cannot satisfy the gate by pointing it at an easier prompt.", "type": "string" }, + "prompt_arg": { + "description": "How the prompt reaches the runner.\n\n**Declared because a runner that ignores the channel discards the prompt\nSILENTLY**, and the record then reads as a completed review of whatever\nthe agent chose to look at. Measured against a real reviewer whose review\nsubcommand takes its focus as a POSITIONAL and wires stdin only for a\ndifferent subcommand: the dispatch succeeds, exits zero and steers\nnothing. Which reviewer, and the reading, are CLOUD-472's — a consumer's\ntool is not this crate's to name (non-negotiable rule 1).\n\nDefaults to stdin, which is what the first landed runner contract\nassumed, so a row that says nothing keeps its behaviour.", + "$ref": "#/$defs/PromptArg" + }, "runner": { "description": "The agent to dispatch it through.", "type": "string" From 2c2c2c19d6a6383c29cbb71af4c82886b1b517eb Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 2 Sep 2026 14:24:56 +0000 Subject: [PATCH 07/16] test(review): the untouched-subject case could not see the narrowing it names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `untouched-subject-priced` SURVIVED the sweep: the mutation turns `count(changed) > 0` into `true`, and the case named as its witness passed either way. The reason is the stub's exit code. At 0 the runner writes a record, so `not input.tree.review[id]` is false and the absence arm never holds — with the delta narrowing removed the module still finds nothing to refuse, and the assertion is satisfied by a predicate that stopped reading the delta entirely. At 1 the runner leaves no record, absence holds, and the ONLY thing keeping the tree quiet is that the subject sits outside the delta. That is the conjunct the mutation removes, so the case now reddens when it should. The general form, since this is the second one this session: a case asserting ABSENCE of a finding pins nothing unless every other conjunct is arranged to hold. Green over a mutation is the only way you learn which one was masking. Refs: CLOUD-472 --- crates/batten/tests/it/review_dispatched.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/batten/tests/it/review_dispatched.rs b/crates/batten/tests/it/review_dispatched.rs index 91b3c5f76..9df669526 100644 --- a/crates/batten/tests/it/review_dispatched.rs +++ b/crates/batten/tests/it/review_dispatched.rs @@ -210,7 +210,14 @@ fn a_missing_runner_is_could_not_look_and_never_a_refusal() { /// and nothing is refused. #[test] fn a_branch_that_did_not_touch_the_subject_owes_no_review() { - let root = repo("review-dispatched-untouched", "body\n", "", 0); + // THE RUNNER FAILS HERE ON PURPOSE, and that is what makes this case + // discriminate. With a runner that exits 0 the record is present, so the + // absence arm is false and the delta narrowing decides nothing observable: + // the case passes whether the module reads the delta or not, which is exactly + // what `untouched-subject-priced` measured when it SURVIVED. A failing runner + // leaves no record, so absence holds and the ONLY thing keeping this tree + // quiet is that the subject is outside the delta. + let root = repo("review-dispatched-untouched", "body\n", "", 1); // Move `origin/main` up to HEAD, so the subject is no longer in the delta. let head = common::git_in(&root, &["rev-parse", "HEAD"]); common::git_in( From a63655474e6ef6b66e38499326378c4f0f61057b Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 2 Sep 2026 14:25:00 +0000 Subject: [PATCH 08/16] docs(tools): the validator row stops advertising a gap that was closed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three claims in the tree said `validator-verdict-clean` decides nothing on any real checkout. All three were false the moment `[tasks.record-verdicts]` landed — an inline mise task `[tasks.verify]` calls BEFORE the gates, which runs `pkl eval hk.pkl` and `renovate-config-validator --strict renovate.json5` and pipes `status clean` / `status error` into `batten record tool` for both rows. None of the three was retired with it, so the row kept advertising a dead gate it had stopped being. THAT COST A COMMIT BEFORE IT COST THIS ONE. Reading those claims, I built an engine-side producer for a decision that already had one — two writers for `.git/batten-tools/@@`, disagreeing the moment their argv or their reduction drifts. It is not in this history because it was reverted before the branch was replayed, which is the right place for it: the finding is the prose, not the code. The mise task is also the correct home under house style §5, which makes `check` structurally incapable of spawning — the task says so at its own site, "THE RUN IS HERE BECAUSE IT CANNOT BE IN THE ENGINE." WHAT MOVED, and only the validator half moved. `forge-verdict-required`'s identical sentence is UNTOUCHED because it is still true: nothing in this tree writes `.git/batten-forge/`. Correcting a claim that still holds because its neighbour's did not would be the same defect mirrored. batten.toml, `validator-verdict-clean` — names the producer and where `verify` calls it, and keeps the absent-is-not-a-refusal half, which is the module's decision and was never the stale part. policy/validator-verdict-clean.rego — the `#MUTANT-OWNER` is WITHDRAWN rather than reworded. Half its reason was the false claim; the other half was real (`tool_verdict_facts.rs` installs a `probe.rego` of its own, so no case in it could redden under a mutation of the shipped module) and is now closed instead of declared. `the_shipped_module_refuses_a_recorded_error` installs `policy/validator-verdict-clean.rego` itself, mints the record with the shipped `batten record tool`, and asserts the refusal; `the_shipped_module_passes_a_recorded_clean` is the discriminating half, without which the first is satisfied by a module that refuses unconditionally. `unclean-verdict-unread` now names the first, and no longer survives the sweep. The fixture declares the row id `config-validator` because that is a literal in the module's own `owned` set. A suite renaming it to something convenient would pass while the shipped selector matched nothing — the same dead gate, one level in. THE HEADER WAS ALSO ORPHANED, earlier on this branch: the `review-dispatched` block went in between that 25-line comment and the `[[rule]]` it describes, so a header about the tool-verdict key's triple has been sitting above the review row. Moved back, in the same admitted write. Refs: CLOUD-1265 Admits: 6c775c003d0da2f954ae062bde4418ea078d765d8694e284dbeeab85f2d9bcfc Admits-rule: protected-mutation Admits-verdict: path write refused Admits-subject: batten.toml Admits-head: 037ad82a62191c8a5dc2de8aa4ec66c40dfcc89d Admits-epoch: c2d5d2bbf5303c8ff6fefad52befea6726329e569e8baf1e4f00e8e29752d763 Admits-author: alec@wenzowski.com Admits-prev: fbe348673db0a5c9304b0d27bd849b6b89d6b3468d23f1f81f6a738c269bcd6c Admits-answer-lost: A stale claim that a landed deny row decides nothing on any real checkout. Read as a live gap it already cost this session a whole commit building a producer the tree already had, and one of the three sites carries an issue key, so it reads as a live obligation with a tracker row behind it. Also a 25-line header explaining the tool-verdict key left sitting above the review row, documenting a rule it says nothing about. Admits-answer-precondition: The change is prose inside batten.toml's own comment stream: a `validator-verdict-clean` header claiming the row is "inert on a checkout nobody has recorded against, which is every checkout today", which `[tasks.record-verdicts]` falsified when it landed; plus that same header having been orphaned from its rule by the `review-dispatched` block added earlier on this branch. There is no surface for either — a comment has no key, and a block's POSITION relative to its `[[rule]]` is not addressable by any verb. Both land in the diff a reviewer reads. Admits-answer-rejected-route: `config read first` does not apply: I read both sites, and the reading is what established the claim is false — `[tasks.record-verdicts]` runs `pkl eval` and pipes `status clean`/`status error` into `batten record tool config-validator`, and `[tasks.verify]` calls it before the gates. `patch run first` has nothing to patch: no verb writes a comment or moves one. --- batten.toml | 61 ++++++----- crates/batten/tests/it/tool_verdict_facts.rs | 103 +++++++++++++++++++ policy/validator-verdict-clean.rego | 3 +- 3 files changed, 139 insertions(+), 28 deletions(-) diff --git a/batten.toml b/batten.toml index e263648da..956afb34a 100644 --- a/batten.toml +++ b/batten.toml @@ -5064,32 +5064,6 @@ key = "CLOUD-1188" node = "project" reduce = "present" -# A third-party validator's verdict for a file it read (CLOUD-1171, the -# adjudication half of `pkl-check`, `renovate-config-validator` and -# `hook-profile-check`). -# -# THE RUN STAYS OUTSIDE and that is §9's prior-art disposition, not a workaround: -# the validator is a command on PATH and stays one. `check` is `read` and -# structurally cannot spawn, so what moves in here is the ADJUDICATION — which is -# the half that had no successor, because a module asking what a validator found -# read undefined and decided nothing. -# -# THE KEY IS A TRIPLE and each component refuses a different lie. The TOOL, -# because one validator's record is not another's. Its PINNED VERSION, because an -# answer at one version is not an answer at the next — CLOUD-646's shape, closed -# by putting the pin in the KEY rather than in a field a module must remember to -# compare. The INPUT DIGEST, so a verdict goes stale by construction the moment -# the file changes: the key moves, and the old record is not found rather than -# found and wrong. -# -# `hk.pkl` is this repository's own fact, which is why the row lives here and not -# in `crates/batten` (non-negotiable rule 1). The engine composes a key and reads -# a file; which file and which tool are the consumer's. -# -# SILENT UNTIL A PRODUCER WRITES. No record under this key means nothing has -# validated these bytes at this version — absent from the map, not a verdict — so -# this row is inert on a checkout nobody has recorded against, which is every -# checkout today. # CLOUD-472's consumer, and the row that makes `Fact::Review` a gate rather than # a store. # @@ -5146,6 +5120,41 @@ runner = "batten-review-runner" version = "0" subject = "tracker-body" +# A third-party validator's verdict for a file it read (CLOUD-1171, the +# adjudication half of `pkl-check`, `renovate-config-validator` and +# `hook-profile-check`). +# +# THE RUN STAYS OUTSIDE and that is §9's prior-art disposition, not a workaround: +# the validator is a command on PATH and stays one. `check` is `read` and +# structurally cannot spawn, so what moves in here is the ADJUDICATION — which is +# the half that had no successor, because a module asking what a validator found +# read undefined and decided nothing. +# +# THE KEY IS A TRIPLE and each component refuses a different lie. The TOOL, +# because one validator's record is not another's. Its PINNED VERSION, because an +# answer at one version is not an answer at the next — CLOUD-646's shape, closed +# by putting the pin in the KEY rather than in a field a module must remember to +# compare. The INPUT DIGEST, so a verdict goes stale by construction the moment +# the file changes: the key moves, and the old record is not found rather than +# found and wrong. +# +# `hk.pkl` is this repository's own fact, which is why the row lives here and not +# in `crates/batten` (non-negotiable rule 1). The engine composes a key and reads +# a file; which file and which tool are the consumer's. +# +# THE PRODUCER IS `mise run record-verdicts` (CLOUD-1265), and naming it here is +# the point rather than a courtesy: this block said the opposite — "SILENT UNTIL +# A PRODUCER WRITES … inert on a checkout nobody has recorded against, which is +# every checkout today" — for as long as the producer existed, and that sentence +# read as a live gap and bought a duplicate writer for it (7aade8f8, reverted). +# `[tasks.verify]` calls the task BEFORE the gates, so the record is present by +# the time this row is adjudicated on any tree `verify` has touched. +# +# ABSENT IS STILL NOT A REFUSAL, and that is the module's decision rather than +# this comment's. No record under this key means nothing has validated these +# bytes at this version — absent from the map, not a verdict — which is what a +# checkout gets if the producer was skipped or died. The row that wants a verdict +# to be REQUIRED is `forge-verdict-required`'s shape and is not this one. [[rule]] id = "validator-verdict-clean" kind = "policy" diff --git a/crates/batten/tests/it/tool_verdict_facts.rs b/crates/batten/tests/it/tool_verdict_facts.rs index d8db71fd1..6555c33c5 100644 --- a/crates/batten/tests/it/tool_verdict_facts.rs +++ b/crates/batten/tests/it/tool_verdict_facts.rs @@ -517,3 +517,106 @@ fn a_successful_record_says_nothing() { stderr(&outcome) ); } + +// --- the SHIPPED module, over the shipped producer --------------------------- +// +// Every case above installs `PROBE`, which is the right shape for asserting what +// the ENGINE projects: a fixture module can vary one key component at a time and +// say so in its own vocabulary. It is the wrong shape for asserting anything +// about `policy/validator-verdict-clean.rego`, and for the whole life of that +// module nothing asserted anything about it over the compiled binary — its +// `#MUTANT-SUITE` named this file, no case in this file installed it, so a +// mutation of its `violation` body SURVIVED and the row carried a +// `#MUTANT-OWNER` conceding the gap. +// +// The gap is closed here rather than re-declared. `owned` is a literal set in the +// module — `{"config-validator", "renovate-config"}` — so the fixture below +// declares `config-validator` by that name: a suite that renamed it to something +// convenient would pass while the shipped selector matched nothing, which is the +// same dead gate one level in. + +/// The shipped predicate, never a copy of it. +const SHIPPED: &str = include_str!("../../../../policy/validator-verdict-clean.rego"); + +/// Registers the shipped module over the id its `owned` set actually names. +const SHIPPED_CONFIG: &str = r#"version = 1 + +[[rule]] +id = "validator-verdict-clean" +kind = "policy" +scope = "tree" +module = "validator-verdict-clean.rego" +severity = "deny" + +[[rule.tools]] +id = "config-validator" +tool = "checker" +version = "1.1.0" +input = "subject.toml" + +[[verdict]] +id = "tool judge dirty" +gloss = "a declared validator recorded a finding over the bytes it read" +class = "A fixture copy of the shipped class; the registry's own row is in batten.toml." + +[[verdict.route]] +id = "tool fix input" +kind = "document" +target = "validator-verdict-clean.rego" +"#; + +/// A repository carrying the SHIPPED module, whose `config-validator` record is +/// written by the shipped producer rather than planted. +fn shipped_fixture(name: &str) -> PathBuf { + let dir = scratch(&format!("validator-verdict-clean-{name}")); + write(&dir, "batten.toml", SHIPPED_CONFIG); + write(&dir, "validator-verdict-clean.rego", SHIPPED); + write(&dir, "subject.toml", SUBJECT); + git_in(&dir, &["init", "-q", "-b", "main", "."]); + dir +} + +#[test] +fn the_shipped_module_refuses_a_recorded_error() { + // THE POSITIVE FOR THE SHIPPED PREDICATE, and the case its declared mutation + // reddens. `status error` is the exact token `[tasks.record-verdicts]` pipes + // in when `pkl eval` exits non-zero, so this is the production path and not a + // vocabulary the suite invented. + let dir = shipped_fixture("error"); + let minted = record_tool(&dir, "config-validator", "status error\n"); + assert!( + minted.status.success(), + "the producer must mint the record\n{}{}", + stdout(&minted), + stderr(&minted) + ); + + let outcome = check(&dir); + let (answer, cause) = (stdout(&outcome), stderr(&outcome)); + assert!( + answer.contains("validator-verdict-clean"), + "a recorded error must reach the shipped predicate and refuse\n{answer}{cause}" + ); +} + +#[test] +fn the_shipped_module_passes_a_recorded_clean() { + // THE DISCRIMINATING HALF. Without it the case above is satisfied by a module + // that refuses unconditionally, which is what a surviving mutation looks like + // from the outside. + let dir = shipped_fixture("clean"); + let minted = record_tool(&dir, "config-validator", "status clean\n"); + assert!( + minted.status.success(), + "the producer must mint the record\n{}{}", + stdout(&minted), + stderr(&minted) + ); + + let outcome = check(&dir); + let (answer, cause) = (stdout(&outcome), stderr(&outcome)); + assert!( + !answer.contains("validator-verdict-clean"), + "the reserved clean status is not a finding\n{answer}{cause}" + ); +} diff --git a/policy/validator-verdict-clean.rego b/policy/validator-verdict-clean.rego index 16a24bc9e..313bd5516 100644 --- a/policy/validator-verdict-clean.rego +++ b/policy/validator-verdict-clean.rego @@ -153,5 +153,4 @@ test_could_not_look_does_not_fault if { } #MUTANT-SUITE crates/batten/tests/it/tool_verdict_facts.rs -#MUTANT-OWNER CLOUD-1265|nothing WRITES a `tool-verdict` record, so this predicate resolves `null` and refuses nothing on any real checkout; the tier it names drives the FACT and never the predicate -#MUTANT unclean-verdict-unread|s@^\tcount(refused) > 0$@\tfalse@|a_declared_key_reads_its_own_record +#MUTANT unclean-verdict-unread|s@^\tcount(refused) > 0$@\tfalse@|the_shipped_module_refuses_a_recorded_error From f82f7ba2c1f950290a36ce63b050d90269a0fda2 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 2 Sep 2026 14:25:08 +0000 Subject: [PATCH 09/16] fix(facts): the census count and the module's placement, both missed by a replay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings `verify` caught that my own `test:cargo` run reported green over. That run is the first thing to correct: it compiled out of `target/mutate` and measured 3993 tests where this tree has 4054, so the 3714/3714 I read as a pass was never a reading of this branch. THE COUNT IS A MERGE ARTEFACT, and the guard it broke is the one that exists for exactly this. `Fact::ALL` carries 35 variants — 34 the base already had, plus `Fact::Review` — while `every_fact_returns_its_stated_const` still asserted 34. The replay's conflict markers covered the `use` list, where `MINTED` and `REVIEW` had to be merged by hand; the count assertion sat six lines outside them, so git took one side silently and neither half of the merge was wrong on its own. The test's own comment calls itself the anti-vacuity guard "so a DROPPED variant fails here too rather than quietly shrinking the census" — it caught a GAINED one by the same arithmetic. `review` WAS NEVER PLACED, and the table says absence is a refusal rather than an allow. `module place missing`, which is the arm whose comment records this rule naming seven unplaced modules before a human did — `patch`, `symbols`, `semver`, `recorder`, `perf`, `pinned`, `prune`. This is the eighth, and it landed with clippy green and both test tiers green, which is the property that list is tracking. Placed in `symbols`' band because it is `symbols`' class exactly: an acquisition module that spawns to resolve a `Cost::Effect` fact, whose caller is the engine deciding WHEN to acquire. `review -> {rules, hook}` is forbidden for the identical reason, and it binds harder here than for `symbols` — a back-edge would let the module that dispatches an AGENT reach the engine that decides whether to, and this spawn costs a round trip where clippy costs seconds. Neither is a new decision: one restores an assertion to the tree it describes, the other writes down a placement the module always had. Refs: CLOUD-472 --- crates/batten/tests/it/facts.rs | 2 +- policy/module-layering.rego | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/crates/batten/tests/it/facts.rs b/crates/batten/tests/it/facts.rs index 11d995142..f2b9c140f 100644 --- a/crates/batten/tests/it/facts.rs +++ b/crates/batten/tests/it/facts.rs @@ -154,7 +154,7 @@ fn every_fact_returns_its_stated_const() { // rather than quietly shrinking the census. assert_eq!( Fact::ALL.len(), - 34, + 35, "the census covers every fact; update this count deliberately when the \ model gains or loses one" ); diff --git a/policy/module-layering.rego b/policy/module-layering.rego index 5b78d5167..2bb9a07ce 100644 --- a/policy/module-layering.rego +++ b/policy/module-layering.rego @@ -91,6 +91,11 @@ declared_modules := { # CLOUD-1050 and it worked a fourth: the module was written, its tests were # green, and this rule is what said nobody had placed it. "patch", "symbols", "semver", + # `review` arrived with CLOUD-472 and this rule named it an eighth time — + # module written, both test tiers green, and nobody had placed it. It is + # `symbols`' class exactly: an acquisition module resolving a `Cost::Effect` + # fact by spawning, whose caller is the engine that decides WHEN to acquire. + "review", # `recorder` arrived with CLOUD-1051 and it worked a fourth time: the module # landed undeclared and this rule named it, before any reviewer did. It is a # writer rather than a decider — it accumulates what a gate already said — so @@ -372,6 +377,14 @@ forbidden[from] contains to if { # `Cost::Effect` boundary a convention rather than a direction — and the # whole point of the class is that a projection cannot reach the spawn. "symbols": {"rules", "hook"}, + # `review -> rules` / `review -> hook` is `symbols`' row a second family + # over, and the reason carries unchanged: `rules::review_fact` is the caller + # that decides when to dispatch, and a back-edge would let the module that + # SPAWNS an agent reach the engine that decides whether to. That is the + # `Cost::Effect` boundary as a direction rather than a convention — and it + # matters more here than for `symbols`, because this spawn costs an agent + # round trip rather than a clippy run. + "review": {"rules", "hook"}, # `pinned -> rules` / `pinned -> hook` is `symbols`' row again, one fact # family over, and for the identical reason: `pinned.rs` resolves a # `Cost::Effect` fact and `lib.rs` is the caller that decides when — at From 98c522d4ed0c1639e1038eb10e9c6443cc810e9d Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 2 Sep 2026 15:03:18 +0000 Subject: [PATCH 10/16] fix(policy): withdraw an override route for a case that never refuses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `config-lint` refused `verdict[prompt run never].override` as an ungroomed weakening, and the right answer turned out not to be grooming it. THE ROUTE COULD NEVER BE SPENT. Its precondition read "the review cannot be dispatched in this environment at all — no runner exists here — rather than not having been dispatched in one where it could". That state is not a refusal: `policy/review-dispatched.rego` reads an absent runner as could-not-look, `a_missing_runner_is_could_not_look_and_never_a_refusal` asserts it over the compiled binary, and `could-not-look-refused` is the declared mutation that pins it. So no case matching the precondition ever produces the verdict the route would override — a hatch for a door that is never shut. That is the same defect this branch has been finding all along, one surface over: a declared thing that reads as live and reaches nothing. It is worth saying that the gate did not find it. `config-lint` reported a WEAKENING, which is a true statement about the bytes and says nothing about whether the route works; reading its own precondition against the module is what found the route dead. The gate asked a question whose honest answer was "this should not exist". WITHDRAWN RATHER THAN GROOMED, and that direction matters: grooming would have added a fourth `Weakens:` clause to CLOUD-472 admitting a hatch nobody can use, and the clause would have read as a considered trade when there was no trade. Removing it is a TIGHTENING, so it owes no clause at all. What remains needs no hatch. `task run first` and `task run other` are command routes, and the state they address — the runner is here, was asked, and left no record — is the branch's own conduct rather than the environment's. Refs: CLOUD-472 Admits: f2c5c32a1efc14250f1e641bf352cbc0a2c31e70465baebddad5bea6fd220459 Admits-rule: protected-mutation Admits-verdict: path write refused Admits-subject: batten.toml Admits-head: 6194f8797b928bd22819dc3272342f561315231d Admits-epoch: 9cfcd8d59c7cf80b7c2b7fdde4c4ef65c0ee585c33b88726ad3ff447e35d2e8c Admits-author: alec@wenzowski.com Admits-prev: fa060cedc1fb70cc975f62de434849db0ba70899a726aaabeb60b1d12794d6d7 Admits-answer-lost: A route that advertises an exit reaching nothing stays in the table. Its precondition is "the review cannot be dispatched in this environment at all — no runner exists here", and `policy/review-dispatched.rego` already reads that state as could-not-look: `a_missing_runner_is_could_not_look_and_never_a_refusal` asserts it over the compiled binary and the `could-not-look-refused` mutation pins it. So no case matching the precondition ever produces the refusal the route would override, and a reader who reaches for it finds a hatch for a door that was never shut. Admits-answer-precondition: The change DELETES a `[[verdict.route]]` block — the `path admit first` override on `prompt run never`. A route is config by construction: no verb writes or removes one, since a gate that could retract its own hatch would be a second authority over the committed table. It lands in the diff a reviewer reads, three lines beside the class it belongs to. Admits-answer-rejected-route: `config read first` is what produced this: reading the class beside the module is how the route was found to duplicate a landed arm. `patch run first` has nothing to patch — no verb adds or removes a `[[verdict.route]]`, which is the property that keeps the override table the committed authority's. --- batten.toml | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/batten.toml b/batten.toml index 956afb34a..812f4efc5 100644 --- a/batten.toml +++ b/batten.toml @@ -9776,10 +9776,18 @@ id = "task run other" kind = "command" target = "check the declared runner is installed and on PATH — a dispatch that cannot spawn leaves no record, exactly as one that never ran" -[[verdict.route]] -id = "path admit first" -kind = "override" -precondition = "the review cannot be dispatched in this environment at all — no runner exists here — rather than not having been dispatched in one where it could" +# NO OVERRIDE ROUTE, AND ITS ABSENCE IS THE DECISION (CLOUD-472). One was drafted +# — precondition "the review cannot be dispatched in this environment at all — no +# runner exists here" — and withdrawn before landing, because that state is not a +# refusal to begin with. `policy/review-dispatched.rego` reads an absent runner as +# could-not-look; `a_missing_runner_is_could_not_look_and_never_a_refusal` asserts +# it over the compiled binary and `could-not-look-refused` is the declared mutation +# that pins it. So no case matching the precondition ever produces this verdict, +# and the route was a hatch for a door that is never shut. +# +# What remains has command routes and needs no hatch: the runner is here and was +# asked, and left no record. That is the branch's own conduct, and the two routes +# above name what to do about it. [[verdict]] id = "review answer missing" From 61ab5acc952c9d2832052ab71f6472c7dab4a4da Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 2 Sep 2026 15:42:39 +0000 Subject: [PATCH 11/16] feat(policy): a backgrounded wait that polls a local process is refused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-1337. `timer run refused` exempts a backgrounded `sleep` wrapped in `until`/`while`, on sound reasoning: a loop testing a condition exits on the condition rather than on the clock. The exemption asks WHETHER there is a condition and never WHAT it is about — so a loop polling the LOCAL PROCESS TABLE passed, and that is the one condition the harness already reports, since a backgrounded task re-invokes its caller on exit. MEASURED ON THIS CONTAINER, 2026-09-02, by a human reading `ps`: eleven such waits running, the oldest 9h35m, while exactly one real job existed. AGENTS.MD'S CLAIM ABOUT THIS WAS FALSE, and that is the reason this is a gate rather than another paragraph. It has carried the rule since CLOUD-821 with the measurement — "490 in one session, 2 changed a decision" — and the assertion that the shape is "refused by `run-shape-guard`". It was not. The rule was written, the mechanism was named, and the named mechanism let the shape through. An agent read that paragraph, agreed with it, and wrote the wait eleven times in the same session. Non-negotiable rule 2's own case, with the sting that the prose CLAIMED a mechanism it did not have. THE NARROWER RULE WAS DRAFTED FIRST AND WOULD HAVE MADE THIS WORSE. Those eleven were also BROKEN: `pgrep -f` reads full command lines, a mediated call runs as `bash -c` over the whole text, so the pattern was a substring of the polling shell's own command line by construction and the probe matched itself forever. Refusing only the self-match is satisfied by bracketing the pattern (`[m]ise`) — eleven correctly-functioning watchers instead of eleven broken ones, every one still redundant. `a_bracketed_pattern_is_refused_just_the_same` and the `bracket-is-an-exit` mutation are what hold that shut: the fix for a self-match must not be an exit from this gate. WHAT STAYS EXEMPT, so this narrows the form rather than closing it: any condition the harness does not report. `until curl -sf $URL`, a forge status, a file another machine writes — none reaches a process table, none is refused, and `a_wait_on_a_condition_nobody_reports_is_clean` is the anti-vacuity mirror without which every other case is satisfied by a rule that refuses all waits. ITS OWN LOOK-THROUGH RATHER THAN A WIDER `keywords`. A condition segment begins with `until`/`while`, often `!`, and none is in the shared `keywords` set — so `words_program_index` resolved the keyword and the first three cases failed. Widening the shared set would change which program `sleeps` resolves for every landed call, so `condition_program` carries the narrower look-through instead. BOTH ROUTES ON BOTH SLEEP-FAMILY CLASSES ARE QUALIFIED, and there were two rather than the one I expected: each recommended `until ; do sleep 1; done` with no word about what `` may read, which is a remedy naming the defect. Refs: CLOUD-1337, CLOUD-821 Admits: c2cf45908086f9e5e0322896d8adaef1fbe68000f3d5567a8300709d43db4aa0 Admits-rule: protected-mutation Admits-verdict: path write refused Admits-subject: batten.toml Admits-head: a14d387eb60a145e9b3a407e062f6db566158324 Admits-epoch: 7878ad9d74a4503f61638dd2d000db2bd6011d9bc77f6e310b0b46254932df6d Admits-author: alec@wenzowski.com Admits-prev: 6bf5c847b2da46a50bf09a9bf2dcd0b8acbda772f279f9cfe80a023b3b2a49e2 Admits-answer-lost: CLOUD-1337 cannot land: `policy/run-shape.rego` fails to LOAD while it raises an undeclared token, so the whole bundle decides nothing — strictly worse than the defect being fixed. And both sleep-family routes keep recommending `until ; do sleep 1; done` unqualified, which is the exact shape that produced eleven duplicate watchers for nine hours on this container; a remedy that names the defect is worse than no remedy. Admits-answer-precondition: The change adds the `[[verdict]]` row for `task watch duplicate` with its two routes, and qualifies the `until ; do sleep 1; done` target on both sleep-family routes. A verdict class IS config — `policy test` refused the module at load for raising a token no row declares, which is the ABI working — and no verb writes a `[[verdict]]` or edits a `[[verdict.route]]`, since a gate that could declare its own class or rewrite its own remedy would be a second authority over the registry. Both land in the diff a reviewer reads. Admits-answer-rejected-route: `config read first` is what produced this change: reading the sibling rows is how their routes were found to recommend the shape being refused, and there were two of them rather than the one I expected. `patch run first` has nothing to patch — no verb adds a verdict class or edits a route target. --- batten.toml | 46 +++++++- crates/batten/tests/it/run_shape.rs | 81 ++++++++++++++ policy/run-shape.rego | 163 ++++++++++++++++++++++++++++ 3 files changed, 288 insertions(+), 2 deletions(-) diff --git a/batten.toml b/batten.toml index 812f4efc5..3e5a37a81 100644 --- a/batten.toml +++ b/batten.toml @@ -9336,7 +9336,7 @@ when the condition holds — that is a background wait and is allowed. [[verdict.route]] id = "task run first" kind = "command" -target = "until ; do sleep 1; done" +target = "until ; do sleep 1; done — where reads something the harness does NOT report, never a process table" [[verdict.route]] id = "task run other" @@ -9363,13 +9363,55 @@ is allowed. [[verdict.route]] id = "task run first" kind = "command" -target = "until ; do sleep 1; done" +target = "until ; do sleep 1; done — where reads something the harness does NOT report, never a process table" [[verdict.route]] id = "task run other" kind = "command" target = "mise run alive" +# THE OTHER HALF OF THE ROW ABOVE, and it exists because that row's own exemption +# was the escape (CLOUD-1337). `timer run refused` allows a backgrounded `sleep` +# wrapped in `until`/`while`, on sound reasoning: a loop testing a condition exits +# on the condition rather than on the clock. But the exemption asks WHETHER there +# is a condition and never WHAT it is about — so a loop polling a LOCAL PROCESS +# passed, and that is the one condition the harness already reports. +# +# MEASURED 2026-09-02: eleven of these on one container, the oldest 9h35m, while +# exactly one real job existed. AGENTS.md has carried the rule since CLOUD-821 +# with its measurement and the claim that the shape is "refused by +# `run-shape-guard`" — a claim that was false for as long as it stood. This class +# is what makes it true. +[[verdict]] +id = "task watch duplicate" +gloss = "a backgrounded wait polls a process whose exit is already notified" +class = """ +The harness re-invokes the caller when a backgrounded task exits, measured 523 of \ +524 in one session including every failure, so a loop reading the process table \ +asks a question that is already answered and cannot be answered sooner. What it \ +costs is not the CPU of a `sleep`: it is a watcher that outlives the thing it \ +watches, because nothing retires it when the answer arrives. \ +A CONDITION THE HARNESS DOES NOT REPORT IS NOT THIS CLASS — a forge status, a \ +remote queue, a file another machine writes are all still allowed, and the \ +`until` form stays the recommended shape for them. \ +The narrower reading is refused deliberately too: these eleven were ALSO broken, \ +since `pgrep -f` reads full command lines and a mediated call runs as \ +`bash -c` over the whole text, so the pattern was a substring of the polling \ +shell's own command line and the probe matched itself forever. Refusing only that \ +is satisfied by bracketing the pattern, which buys eleven working watchers \ +instead of eleven broken ones and no less waste. The wait is the defect. +""" + +[[verdict.route]] +id = "turn watch first" +kind = "command" +target = "delete the loop and end the turn — the backgrounded task wakes you on exit, and the turn in between is the designed state" + +[[verdict.route]] +id = "task watch other" +kind = "command" +target = "mise run alive" + [[verdict]] id = "workflow parse broken" gloss = "a workflow could not be parsed, so its lanes were never judged" diff --git a/crates/batten/tests/it/run_shape.rs b/crates/batten/tests/it/run_shape.rs index 1a1c4b0c1..1a933db85 100644 --- a/crates/batten/tests/it/run_shape.rs +++ b/crates/batten/tests/it/run_shape.rs @@ -662,3 +662,84 @@ fn a_command_with_no_git_commit_in_it_is_untouched() { allowed(&root, "ls -la"); allowed(&root, "hg commit"); } + +// --- the wait that duplicates a notification (CLOUD-1337) -------------------- +// +// `timer run refused` exempts a backgrounded `sleep` wrapped in `until`/`while`, +// on sound reasoning: a loop testing a condition exits on the condition rather +// than on the clock. The exemption asks WHETHER there is a condition and never +// WHAT it is about, so a loop polling the local process table passed — and that +// is the one condition the harness already reports, since a backgrounded task +// re-invokes its caller on exit. +// +// MEASURED 2026-09-02: eleven of these on one container, the oldest 9h35m, while +// exactly one real job existed. AGENTS.md had carried the rule since CLOUD-821 +// AND the claim that the shape was "refused by `run-shape-guard`"; it was not. +// +// OVER THE COMPILED BINARY BECAUSE THE LOOK-THROUGH IS THE ENGINE'S. The module +// resolves a condition segment's program through `hook::segments`, and a +// `with input as` case supplies segments by hand — so it would pass over an +// engine that tokenised `until ! pgrep` differently than the fixture assumed. +// That is exactly how this predicate was wrong on its first three cases. + +#[test] +fn a_backgrounded_wait_polling_a_process_is_refused() { + let root = fixture("polls-a-process"); + denied_background( + &root, + "until ! pgrep -f 'mise run verify' >/dev/null; do sleep 20; done", + true, + ); +} + +#[test] +fn a_bracketed_pattern_is_refused_just_the_same() { + // THE CASE THAT SEPARATES THIS RULE FROM THE WRONG ONE. Those eleven waits + // were ALSO broken — `pgrep -f` reads full command lines and a mediated call + // runs as `bash -c` over the whole text, so the pattern was a substring of + // the polling shell's own command line and the probe matched itself forever. + // + // Bracketing is the standard fix for that, and if it were an exit from this + // gate the remedy would buy eleven WORKING watchers and no less waste. The + // wait is the defect, not the typo. + let root = fixture("polls-a-process-bracketed"); + denied_background( + &root, + "until ! pgrep -f '[m]ise run verify' >/dev/null; do sleep 20; done", + true, + ); +} + +#[test] +fn a_liveness_signal_is_the_same_question() { + // `kill -0` asks "is this alive" as a signal rather than as a listing, and it + // is what a caller reaches for once `pgrep` is refused. + let root = fixture("polls-by-signal"); + denied_background( + &root, + "while kill -0 $PID 2>/dev/null; do sleep 5; done", + true, + ); +} + +#[test] +fn a_wait_on_a_condition_nobody_reports_is_clean() { + // THE ANTI-VACUITY MIRROR. Without it every case above is satisfied by a rule + // that refuses all waits — and this form is what `timer run refused`'s own + // route still recommends for a condition the harness does not report. + let root = fixture("waits-on-a-remote"); + allowed_background( + &root, + "until curl -sf https://example.test/ready; do sleep 5; done", + true, + ); +} + +#[test] +fn a_process_read_outside_a_loop_is_not_a_wait() { + // Asking once and returning is the route this class recommends, so refusing + // it would refuse its own remedy. + let root = fixture("reads-a-process-once"); + allowed_background(&root, "pgrep -f mise", true); + allowed(&root, "mise run alive"); +} diff --git a/policy/run-shape.rego b/policy/run-shape.rego index f98828f05..4fab29c72 100644 --- a/policy/run-shape.rego +++ b/policy/run-shape.rego @@ -63,6 +63,8 @@ rules contains "foreground-sleep" rules contains "background-timer" +rules contains "polls-a-local-process" + # CLOUD-613's three, and none of them is over a program NAME — a mutation on the # `sleep` or `git` token survives, because every ALLOW row already fails some # other conjunct. Each of these corrupts the conjunct that carries the verdict. @@ -81,6 +83,8 @@ rules contains "background-timer" #MUTANT-OWNER CLOUD-989|the mutation applies and alters reachable code, and the case it names cannot observe the change — a downstream guard or a second arm masks it. That is a defect in the DECLARATION, which `SURVIVED` mis-attributes to the suite; CLOUD-989's fork is what reports it correctly, and these are the live instances its own acceptance says it lacked #MUTANT single-quoted-span-judged|s@^single_scrubbed := quoted_out(code_lines.*@single_scrubbed := code_lines@|a_git_commit_inside_a_quoted_span_is_prose #MUTANT-SUITE crates/batten/tests/it/run_shape.rs +#MUTANT process-poll-unread|s@^\tcount(process_probes) > 0$@\tfalse@|a_backgrounded_wait_polling_a_process_is_refused +#MUTANT bracket-is-an-exit|s@^\tcondition_program(segment) in {"pgrep", "pkill", "ps", "jobs"}$@\tcondition_program(segment) in {"pgrep", "pkill", "ps", "jobs"}; not contains(segment.raw, "[")@|a_bracketed_pattern_is_refused_just_the_same violation contains { "rule": "commit-names-no-message-source", @@ -137,6 +141,43 @@ violation contains { not waits_on_condition } +# A backgrounded wait that polls the LOCAL PROCESS TABLE (CLOUD-1337). +# +# `waits_on_condition` above exempts a `sleep` loop from `background-timer` on +# sound reasoning: a loop testing a condition exits on the condition rather than +# on the clock. That holds for a condition NOTHING ELSE REPORTS — a CI run, a +# remote queue, a file another machine writes. It does not hold for a local +# process, because the harness already re-invokes the caller when a backgrounded +# task exits. Polling one duplicates a notification that is guaranteed to fire. +# +# THE EXEMPTION ASKS WHETHER THERE IS A CONDITION, NEVER WHAT IT IS ABOUT, and +# `until` was the escape. AGENTS.md has carried the rule since CLOUD-821, with the +# measurement — "490 in one session, 2 changed a decision" — and the claim that +# the shape is "refused by `run-shape-guard`". It was not. This arm is what makes +# that sentence true rather than something to soften. +# +# MEASURED 2026-09-02: eleven of these ran on one container, the oldest 9h35m, +# while exactly one real job existed. +# +# THE NARROWER RULE WAS DRAFTED FIRST AND WOULD HAVE MADE THIS WORSE, which is +# why the wider one is here. Those eleven were also BROKEN: `pgrep -f` reads full +# command lines, a mediated call runs as `bash -c ''`, so the +# pattern was a substring of the polling shell's own command line by construction +# and the probe matched itself forever. Refusing only that is satisfied by +# bracketing the pattern (`[m]ise`) — eleven correctly-functioning watchers +# instead of eleven broken ones, every one still redundant. The waste is the +# wait, not the typo, so a bracketed pattern is refused here too. +violation contains { + "rule": "polls-a-local-process", + "verdict": "task watch duplicate", + "subjects": [{"count": count(process_probes)}], +} if { + sleeps + input.call["run-in-background"] == true + waits_on_condition + count(process_probes) > 0 +} + # --------------------------------------------------------------------------- # CLOUD-613's terms, over `input.call.segments`. # @@ -180,6 +221,53 @@ waits_on_condition if { word in {"until", "while"} } +# Every reader of the LOCAL process table in this call. +# +# The set is the programs whose whole purpose is answering "is this process still +# alive" — the question the exit notification already answers. `kill -0` is that +# same question spelled as a signal, which is why it is here rather than being +# left out as "not a process lister". +# +# NEVER A PATTERN AND NEVER A PID (non-negotiable rule 4): a probe's operand on +# this surface carries this consumer's task names and paths. The COUNT travels and +# the text does not, which is also why the finding cannot name which wait it was. +# +# Indexed by SEGMENT so two probes in one call count twice — a compound that polls +# a process and then polls another is two duplications, not one. +process_probes contains i if { + some i, segment in input.call.segments + condition_program(segment) in {"pgrep", "pkill", "ps", "jobs"} +} + +# `kill -0 ` is a liveness test rather than a signal, and it is the spelling +# a caller reaches for once `pgrep` is refused. +process_probes contains i if { + some i, segment in input.call.segments + condition_program(segment) == "kill" + some word in segment.words + word == "-0" +} + +# The program a LOOP CONDITION segment invokes. +# +# `keywords` looks through `do`/`then`/… — which is what lets `sleeps` reach a +# loop BODY — and a condition segment begins with `until` or `while`, often with +# `!` after it. Neither is in that set, so `words_program_index` resolves the +# keyword itself and every probe below would miss. +# +# A NARROWER LOOK-THROUGH HERE RATHER THAN A WIDER `keywords`, deliberately: +# `sleeps` shares that set, so adding `until`/`while` to it would change which +# program every landed call resolves to. This rule is new and may carry its own; +# the shared authority stays where it is. +# +# The filter drops those tokens ANYWHERE rather than only in a leading run, which +# is the cheaper predicate and is safe here because none of the three is a +# plausible operand of a process probe. +condition_program(segment) := name if { + rest := [w | some w in segment.words; not w in {"until", "while", "!"}] + name := basename(rest[words_program_index(rest)]) +} + # `git commit`, resolved over WORDS the engine split rather than a string this # module splits. Same rule as `git_commit` above and deliberately not shared with # it: that one takes a stage string, and one function taking either would be a @@ -543,6 +631,81 @@ test_a_backgrounded_bare_sleep_is_a_timer if { # THE ALLOW THAT MATTERS. This is the form both refusals recommend, and denying # it is what would get the rule switched off. +# THE MEASURED SHAPE (CLOUD-1337). A backgrounded wait polling the process table +# is refused, because the harness already reports that exit. Eleven of these ran +# on one container, the oldest 9h35m. +test_a_backgrounded_wait_polling_a_process_is_refused if { + some v in violation with input as {"call": { + "command": "until ! pgrep -f mise >/dev/null; do sleep 20; done", + "run-in-background": true, + "segments": [ + seg(["until", "!", "pgrep", "-f", "mise"], ";", false), + seg(["do", "sleep", "20"], ";", false), + seg(["done"], null, false), + ], + }} + v.verdict == "task watch duplicate" +} + +# THE CASE THAT SEPARATES THIS RULE FROM THE WRONG ONE. Those eleven were also +# self-matching, and the fix for a self-match is to bracket the pattern. If that +# were an exit from this gate, the remedy would buy eleven WORKING watchers and no +# less waste. The wait is the defect, so a bracketed pattern is refused too. +test_a_bracketed_pattern_is_refused_just_the_same if { + some v in violation with input as {"call": { + "command": "until ! pgrep -f [m]ise >/dev/null; do sleep 20; done", + "run-in-background": true, + "segments": [ + seg(["until", "!", "pgrep", "-f", "[m]ise"], ";", false), + seg(["do", "sleep", "20"], ";", false), + seg(["done"], null, false), + ], + }} + v.verdict == "task watch duplicate" +} + +# `kill -0` is the same liveness question spelled as a signal, and it is the +# spelling a caller reaches for once `pgrep` is refused. +test_a_liveness_signal_is_the_same_question if { + some v in violation with input as {"call": { + "command": "while kill -0 $PID 2>/dev/null; do sleep 5; done", + "run-in-background": true, + "segments": [ + seg(["while", "kill", "-0", "$PID"], ";", false), + seg(["do", "sleep", "5"], ";", false), + seg(["done"], null, false), + ], + }} + v.verdict == "task watch duplicate" +} + +# THE ANTI-VACUITY MIRROR, and without it every case above is satisfied by a rule +# that refuses all waits. A condition the harness does NOT report stays allowed — +# that is the whole narrowing, and `timer run refused`'s route still recommends +# this shape for it. +test_a_wait_on_a_condition_nobody_reports_is_clean if { + count(violation) == 0 with input as {"call": { + "command": "until curl -sf https://example.test/ready; do sleep 5; done", + "run-in-background": true, + "segments": [ + seg(["until", "curl", "-sf", "https://example.test/ready"], ";", false), + seg(["do", "sleep", "5"], ";", false), + seg(["done"], null, false), + ], + }} +} + +# A PROCESS READ WITH NO LOOP IS NOT A WAIT. `mise run alive` asks once and +# returns, which is the route this class recommends — refusing it would refuse +# its own remedy. +test_a_process_read_outside_a_loop_is_not_a_wait if { + count(violation) == 0 with input as {"call": { + "command": "pgrep -f mise", + "run-in-background": true, + "segments": [seg(["pgrep", "-f", "mise"], null, false)], + }} +} + test_a_backgrounded_wait_on_a_condition_is_allowed if { count(violation) == 0 with input as {"call": { "command": "until [ -f /tmp/done ]; do sleep 1; done", From b125d9ffacbd052477bd6b74ddcde140a7aa0dda Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 2 Sep 2026 16:13:47 +0000 Subject: [PATCH 12/16] fix(policy): two harness-wiring mutations reached nothing, for two reasons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by `mutate sweep` after rebasing onto current `main`, which is where both came from: the rows are on `origin/main` verbatim and my earlier sweep predated them. Fixed here rather than filed, because both are in a file this branch already touches and neither needed new test code. `stale-unguarded` SURVIVED, AND ITS CASE COULD NOT REACH THE PREDICATE AT ALL. `a_tree_with_no_wiring_surface_is_not_stale` builds its repo by hand and writes `batten.toml` and the module — but not `policy/harness-declared.json`. `declared` stopped being a Rego constant when CLOUD-1163 made it that document, so in this fixture the table was undefined, `stale` had no rows to iterate, and the mutation on `committed_read > 0` changed no answer. The case asserted a clean tree and would have asserted one over a module that had stopped guarding entirely. That is the exact class `.claude/rules/policy-modules.md` records for a preset reading a `[[pattern]]` row it cannot see: undefined reads as *does not hold*, so the rule decides nothing while everything stays green. Here it arrived through a fixture rather than through a preset. The row the fixture now writes matches nothing wired, which is the state `stale` refuses — so the guard is the only thing keeping that tree clean, and removing it reddens the case. `spent-never` NAMED A CASE THAT IS NOT IN THE DECLARED SUITE. `a_row_whose_owner_has_closed_is_spent` is a `test_` rule in the `.rego`, and `mutate` resolves the declared `#MUTANT-SUITE` path (CLOUD-1267) — so it reported `names-no-case` and the mutation was applied to nothing. Repointed rather than duplicated: `the_engine_reads_a_closed_owner_off_a_minted_receipt` already exists in that suite, mints a real `done` receipt into a real store, and asks the shipped `check` to find it. That is the tier which can see this predicate die; the load-time one fabricates `input.tree.minted` and passes over an engine that never builds it. Refs: CLOUD-1337, CLOUD-1310 --- crates/batten/tests/it/harness_wiring.rs | 15 +++++++++++++++ policy/harness-wiring.rego | 8 +++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/crates/batten/tests/it/harness_wiring.rs b/crates/batten/tests/it/harness_wiring.rs index 54595527f..3a9cd0a5c 100644 --- a/crates/batten/tests/it/harness_wiring.rs +++ b/crates/batten/tests/it/harness_wiring.rs @@ -558,8 +558,23 @@ fn a_tree_with_no_wiring_surface_is_not_stale() { // In the compiled tier rather than only in the module's own `test_` rules, // because that is what makes the mutation on the guard land somewhere a // declared `#MUTANT-SUITE` case can turn red (CLOUD-1267). + // A DECLARED ROW IS WHAT MAKES THE GUARD LOAD-BEARING, and its absence is why + // `stale-unguarded` SURVIVED. `declared` stopped being a Rego constant when + // CLOUD-1163 made it `policy/harness-declared.json`, and this fixture never + // wrote that document — so the table was undefined, `stale` had nothing to + // iterate, and the mutation on `committed_read > 0` changed no answer. The + // case asserted a clean tree and would have asserted one over a module that + // had stopped guarding. + // + // The row deliberately matches nothing wired: that is the state `stale` + // refuses, so the guard is the ONLY thing keeping this tree clean. let repo = scratch("harness-wiring-no-surface"); write(&repo, "batten.toml", &config()); + write( + &repo, + "policy/harness-declared.json", + "{\n \"matches-nothing.sh\": \"CLOUD-1\"\n}\n", + ); let module = std::fs::read_to_string(at_root("policy/harness-wiring.rego")).unwrap(); write(&repo, "harness-wiring.rego", &module); git_in(&repo, &["init", "-q", "-b", "main", "."]); diff --git a/policy/harness-wiring.rego b/policy/harness-wiring.rego index dec4e2b46..1ef8a1dfa 100644 --- a/policy/harness-wiring.rego +++ b/policy/harness-wiring.rego @@ -743,4 +743,10 @@ test_no_reading_at_all_is_not_spent if { #MUTANT stray-unread|s@^\tnot contains(command, mediator)$@\tfalse@|a_committed_sibling_the_table_does_not_declare_is_refused #MUTANT stale-unguarded|s@^\tcommitted_read > 0$@\ttrue@|a_tree_with_no_wiring_surface_is_not_stale #MUTANT stale-never|s@^\tnot matches_something(pattern)$@\tfalse@|a_committed_row_matching_nothing_is_stale -#MUTANT spent-never|s@^\tregex.match(expression, status)$@\tfalse@|a_row_whose_owner_has_closed_is_spent +# NAMES THE COMPILED CASE, NOT THE LOAD-TIME ONE. `a_row_whose_owner_has_closed_is_spent` +# is a `test_` rule in this file; `mutate` resolves the DECLARED `#MUTANT-SUITE` +# path (CLOUD-1267), so naming it reported `names-no-case` and the mutation was +# never applied to anything. The compiled equivalent already existed under +# another name — it mints a real `done` receipt and asks the shipped `check` — +# which is the tier that can actually see this predicate die. +#MUTANT spent-never|s@^\tregex.match(expression, status)$@\tfalse@|the_engine_reads_a_closed_owner_off_a_minted_receipt From ff0b021e4ed489fb692daf81ad8e630f542efc87 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 2 Sep 2026 16:34:25 +0000 Subject: [PATCH 13/16] fix(policy): the stale guard's case took the arm its mutation does not touch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My previous commit claimed to close `stale-unguarded` and did not. The sweep still reported SURVIVED, and the reason I believed otherwise is the error worth recording: I ran the case, saw it pass, and called the fix done. A passing case says nothing about whether a mutation is observable — that is the whole premise of declaring one. `enforced` splits on the pattern's SHAPE. A row carrying a `/` is judged where a COMMITTED surface was read; a bare basename where a MERGED one was: enforced(pattern) if { contains(pattern, "/"); committed_read > 0 } enforced(pattern) if { not contains(pattern, "/"); merged_read > 0 } `stale-unguarded` mutates the FIRST arm. The row I added was `matches-nothing.sh` — a basename — so it took the second, which the mutation does not touch, and the guard died unobserved exactly as before. The fixture had a declared row and still could not see the thing it was there to see. Now `mise-tasks/matches-nothing.sh`, which carries the `/` and matches nothing wired: unmutated, `committed_read` is 0 in a tree with no wiring surface, so the guard holds and the tree is clean; mutated to `true`, the row is enforced, matches nothing, and `stale` fires. SHOWN BOTH WAYS BEFORE COMMITTING, by hand, which is what I skipped last time: mutation applied → `a_tree_with_no_wiring_surface_is_not_stale` FAILS; module restored → PASSES. Only the test file changes here; `policy/harness-wiring.rego` is byte-identical to the commit before. Refs: CLOUD-1337, CLOUD-1310 --- crates/batten/tests/it/harness_wiring.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/batten/tests/it/harness_wiring.rs b/crates/batten/tests/it/harness_wiring.rs index 3a9cd0a5c..63f7c3aaf 100644 --- a/crates/batten/tests/it/harness_wiring.rs +++ b/crates/batten/tests/it/harness_wiring.rs @@ -573,7 +573,12 @@ fn a_tree_with_no_wiring_surface_is_not_stale() { write( &repo, "policy/harness-declared.json", - "{\n \"matches-nothing.sh\": \"CLOUD-1\"\n}\n", + // A COMMITTED row — it carries a `/` — because `enforced` splits on exactly + // that: a slash-bearing pattern is judged where a COMMITTED surface was + // read and a bare basename where a MERGED one was. A basename here takes + // the `merged_read` arm, which this guard's mutation does not touch, so + // the case would still not observe it. + "{\n \"mise-tasks/matches-nothing.sh\": \"CLOUD-1\"\n}\n", ); let module = std::fs::read_to_string(at_root("policy/harness-wiring.rego")).unwrap(); write(&repo, "harness-wiring.rego", &module); From c71c84fc6fb1822cf7dd4d65ee4025ea8ed3c6f9 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 2 Sep 2026 17:55:07 +0000 Subject: [PATCH 14/16] fix(prune): the basis count went eleven past its tolerance under a rebase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `land` could not complete a lap: `verify`'s head-of-run `target prune` refused on every one, so no receipt was written and thirteen commits sat unlandable. THE REFUSAL WAS CORRECT AND IT IS NOT ABOUT DISK. `[prune.*.basis]` declared 164 tracked test stems against a live 175 — eleven past a tolerance of 10. Free space was 20006MB against a 17042MB warm floor, so there was never a shortage. The drift is both sides of a rebase rather than one change: CLOUD-472's bundle added `review_dispatched.rs`, CLOUD-1337's added cases, and `origin/main` advanced ten commits under the branch bringing its own. THE HONEST HALF, A THIRD TIME, which is what the block instructs and what the 2026-09-01 and 2026-09-02 entries each did: `count` moves with `measured` and the FLOORS DO NOT. Moving them down needs an independent measurement — a build from an empty `target` for cold, a minimal post-prune tree for warm — which is CLOUD-1158's and which I did not take. A floor too high only refuses laps; one too low fails silently as a rustc IO error inside a test run. AND THE CALLER MISREPORTS THIS REFUSAL, recorded in the block because it cost real time. `verify` says "not enough disk to run the gate, and pruning did not recover it — the refusal above names free space and the floor." The refusal names a STEM COUNT, and free space was 3GB clear of the floor. `target prune`'s stale-basis exit is not its below-floor exit and the caller collapses them, so an operator reading the caller rather than the callee deletes files and gets nowhere. That is what happened here before the callee's own message was read. `mise run target-prune` now exits 0. Refs: CLOUD-1158, CLOUD-1210 Admits: 214320fcb2a8b8664a92a47f868bcd383118bbb53cb5945de6fa0ba54a9588a3 Admits-rule: protected-mutation Admits-verdict: path write refused Admits-subject: batten.toml Admits-head: d32bdfe4daee1c5ad077564e855fdb879a8cc2d8 Admits-epoch: 6f6e3a71a8067f5899595aba8142e8e0d94abe3257307bc983d4c85d56c2dc5d Admits-author: alec@wenzowski.com Admits-prev: c73a84a3ceafa617abebbbfe8a9d9d3ecf1176bb9a00418e6383b95778ab9a4d Admits-answer-lost: `land` cannot complete a lap. `verify` refuses at its head-of-run prune on every lap, so no receipt is written, `ready-guard` refuses the ready, and thirteen commits sit unlandable. The refusal is correct — the basis was taken against a smaller tree — and the remedy is the count, not the floors. Admits-answer-precondition: `[prune.warm.basis]` and `[prune.cold.basis]` carry `count = 164` against a live 175, eleven past a tolerance of 10, and `target prune` refuses on that. The block states the remedy in its own words — "move `count` and `measured` TOGETHER" — and there is no surface that does it: a basis count is config, and a verb that refreshed its own basis would be a gate rewriting the number it is judged against. It lands in the diff a reviewer reads, beside the block that instructs it. Admits-answer-rejected-route: `config read first` is what produced this: reading the block is how the remedy and its honest scope were found, including that the floors must NOT move without an independent measurement I did not take. `patch run first` has nothing to patch — no verb writes a `[prune.*.basis]` count, deliberately, since that is the number the gate judges against. --- batten.toml | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/batten.toml b/batten.toml index 3e5a37a81..1f0d51c7d 100644 --- a/batten.toml +++ b/batten.toml @@ -6144,7 +6144,29 @@ measured = "2026-09-02" # the gate is what the number is compared against — a basis refreshed from a # second reading of the tree would red again on the next lap while looking correct # in review. Which file the two readers disagree about is unresolved and is not -# this bundle's; it is a pointer for whoever takes CLOUD-1158's floor re-derivation. +# this bundle's; it is a pointer for whoever takes CLOUD-1158's floor re-derivation.# +# THE DISAGREEING READER IS THE GLOB IMPLEMENTATION, NOT A MISSING FILE, and the +# entry above leaves it open. Measured 2026-09-02 over one tree: +# `git ls-files 'crates/batten/tests/**/*.rs'` and the gate's own walk differ by +# exactly `crates/batten/tests/policy_modules.rs` — the one `.rs` sitting directly +# under `tests/` with no intervening directory. Git pathspec requires `**/` to +# match at least one path component; a globset-style `**/` matches zero. Same glob +# string, two grammars, one file. +# +# So the entry above is right to write down the GATE's reading, and the reason is +# sharper than "the gate is what the number is compared against": a `git ls-files` +# reading is not a worse measurement of the same set, it is a measurement of a +# DIFFERENT set. Whoever takes CLOUD-1158's floor re-derivation should read the +# floors against the gate's walk for the same reason. +# +# AND THE CALLER MISREPORTS THIS REFUSAL, which cost an hour on the branch that +# found it. `verify` says "not enough disk to run the gate, and pruning did not +# recover it — the refusal above names free space and the floor". It does not: the +# refusal names a STEM COUNT, and free space was 3GB clear of the floor on the lap +# that tripped it. `target prune`'s stale-basis exit is not its below-floor exit, +# and the caller collapses them — so an operator who reads the caller rather than +# the callee deletes files and gets nowhere. + [prune.warm.basis] glob = "crates/batten/tests/**/*.rs" From 98e1071a015232d7872546e130c1dde54f39d0c1 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 2 Sep 2026 19:02:11 +0000 Subject: [PATCH 15/16] fix(test): a Windows path is not a TOML escape, and a stub that cannot run is not a pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI's `windows` job failed on `ready::a_runner_that_answers_nothing_usable_is_refused` with `missing escaped value, expected b, e, f, n, r, \, ", x, u, U`. That is a TOML parse error, not a policy failure. THE FIXTURE WROTE AN ABSOLUTE PATH INTO A TOML BASIC STRING. `runner = "{}"` with `D:\a\batten\...\runner.sh` reads `\a` and `\b` as escape sequences. Unix paths carry no backslashes, so every local run was green — and this was the branch's FIRST real CI run, since every earlier one was draft-skipped. A TOML literal string fixes it; hand-escaping the backslashes would be a second authority over TOML's own string grammar, exercised on exactly one platform. AND THE SECOND WAVE WAS BEHIND IT. The job cancelled after that one failure with 587 tests unrun, so the suites that drive a `#!/bin/sh` stub had not been reached. Both would have failed, and the way they fail is the reason this is a gate rather than a skip: on Windows the spawn fails, the dispatch leaves no record, and ABSENCE IS WHAT THESE GATES REFUSE. Every refusal case would have passed for the wrong reason while the one clean case failed — a suite whose negative arms are satisfied by the subject never running, which is the vacuous pass this family exists to refuse. `review_dispatched.rs` takes `#![cfg(unix)]` on the module, because every case there needs the stub. `ready.rs` takes it per case on the five that call `with_pressure_test`, plus the helper: the rest of that suite reads a Ready block and needs no subprocess, so a module gate would drop coverage Windows can genuinely give. Precedent is `bot_lane.rs`, whose own gate records the same shape one client over, and `session_provisioning.rs` / `connector_allow_door.rs`. A `.cmd` twin of either stub would be a second authority over what the runner answers. All five `ready` cases and the review-dispatch suite pass on unix with the gates in. Refs: CLOUD-472, CLOUD-1337 --- crates/batten/tests/it/ready.rs | 26 ++++++++++++++++++++- crates/batten/tests/it/review_dispatched.rs | 16 +++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/crates/batten/tests/it/ready.rs b/crates/batten/tests/it/ready.rs index dd6087abe..3f20543d3 100644 --- a/crates/batten/tests/it/ready.rs +++ b/crates/batten/tests/it/ready.rs @@ -337,6 +337,14 @@ fn dated_payload(created_at: Option<&str>, description: &str) -> String { /// false leaves the program absent — which is the could-not-look arm rather than /// a refusal, and the case that keeps this gate from being a verdict about the /// operator's machine. +// UNIX-ONLY, AND ONLY THESE FIVE. The rest of this suite reads a Ready block and +// needs no subprocess; these drive a `#!/bin/sh` stub whose `set_permissions` +// call below is already `#[cfg(unix)]`. On Windows the spawn fails, the dispatch +// leaves no record, and absence is what this gate refuses — so the refusal cases +// would pass FOR THE WRONG REASON while the clean one failed. `bot_lane.rs` and +// `review_dispatched.rs` gate whole modules on this rung; here the module has +// other work, so the gate is per case. +#[cfg(unix)] fn with_pressure_test(name: &str, runner_exits: Option) -> PathBuf { // BUILT FIRST, CONFIGURED SECOND. The row's `runner` must be an absolute // path — a relative program is resolved against the PARENT's working @@ -351,6 +359,17 @@ fn with_pressure_test(name: &str, runner_exits: Option) -> PathBuf { .git() .base_commit() .build(); + // A TOML LITERAL STRING FOR THE PATH, and it is a Windows fix rather than a + // style choice. A basic `"..."` string processes escapes, so an absolute path + // on Windows — `D:\a\batten\...` — reads `\a` and `\b` as escape sequences + // and the config fails to parse. Measured: this suite was green on every unix + // runner and `ready::a_runner_that_answers_nothing_usable_is_refused` failed + // the windows job with `missing escaped value, expected b, e, f, n, r, \, ", + // x, u, U`. + // + // A literal rather than hand-escaping the backslashes: an escaper written here + // would be a second authority over TOML's own string grammar, and it would be + // one this suite exercises on exactly one platform. let runner = dir.join("runner.sh"); common::write( &dir, @@ -360,7 +379,7 @@ fn with_pressure_test(name: &str, runner_exits: Option) -> PathBuf { [[rule]]\nid = \"review-dispatched\"\nkind = \"policy\"\nscope = \"tree\"\n\ module = \"policy/review-dispatched.rego\"\nseverity = \"deny\"\n\n\ [[rule.review]]\nid = \"ready-pressure-test-body\"\nprompt = \"ready-pressure-test\"\n\ - runner = \"{}\"\nversion = \"0\"\nsubject = \"tracker-body\"\n\n{}", + runner = '{}'\nversion = \"0\"\nsubject = \"tracker-body\"\n\n{}", runner.display(), declared_patterns() ), @@ -383,6 +402,7 @@ fn with_pressure_test(name: &str, runner_exits: Option) -> PathBuf { /// infers quality from its SHAPE; this asks whether a named prompt ran over /// these exact bytes, which better-shaped prose cannot satisfy because the prose /// is the input to the hash. +#[cfg(unix)] #[test] fn a_row_past_the_pressure_test_cutover_owes_a_dispatch() { let dir = with_pressure_test("ready-pressure-past-cutover", Some(0)); @@ -404,6 +424,7 @@ fn a_row_past_the_pressure_test_cutover_owes_a_dispatch() { /// present and was asked, and it exited non-zero: the review was owed, it was /// dispatched, and it did not answer. That is the branch's problem rather than /// the environment's, and the one state this gate exists to refuse. +#[cfg(unix)] #[test] fn a_runner_that_answers_nothing_usable_is_refused() { let dir = with_pressure_test("ready-pressure-red-runner", Some(1)); @@ -419,6 +440,7 @@ fn a_runner_that_answers_nothing_usable_is_refused() { /// BEFORE THE CUTOVER IS UNJUDGED. Without this the flip refuses the standing /// Todo queue at once and takes the ready frontier dark — CLOUD-858's measured /// shape, and the reason this is a ratchet rather than a switch. +#[cfg(unix)] #[test] fn a_row_created_before_the_pressure_test_cutover_is_not_judged() { let dir = with_pressure_test("ready-pressure-before-cutover", None); @@ -433,6 +455,7 @@ fn a_row_created_before_the_pressure_test_cutover_is_not_judged() { /// A PAYLOAD WITH NO CREATION INSTANT CANNOT BE PLACED against a cutover, so it /// is could-not-look. Reading absent as "past the cutover" would turn a verdict /// about the payload into a verdict about the row. +#[cfg(unix)] #[test] fn a_payload_with_no_creation_instant_is_not_judged() { let dir = with_pressure_test("ready-pressure-undated", None); @@ -448,6 +471,7 @@ fn a_payload_with_no_creation_instant_is_not_judged() { /// installed cannot be asked whether it reviewed, and refusing there would fail /// every fresh clone and every CI runner that has not installed the agent — a /// verdict about the operator wearing a verdict about the row. +#[cfg(unix)] #[test] fn a_row_past_the_cutover_with_no_runner_is_not_judged() { let dir = with_pressure_test("ready-pressure-no-runner", None); diff --git a/crates/batten/tests/it/review_dispatched.rs b/crates/batten/tests/it/review_dispatched.rs index 9df669526..39abbbd12 100644 --- a/crates/batten/tests/it/review_dispatched.rs +++ b/crates/batten/tests/it/review_dispatched.rs @@ -24,6 +24,22 @@ //! stream that is not pointers — unreachable, since you cannot ask a real agent to //! misbehave on demand. +// UNIX-ONLY, AND THE WINDOWS FAILURE WOULD BE A FALSE GREEN RATHER THAN A +// COULD-NOT-RUN. Every case below drives a `#!/bin/sh` stub through +// `exec::piped`, and only unix makes it executable — the `set_permissions` call +// in `stub` is already `#[cfg(unix)]`. On Windows the spawn fails, the dispatch +// leaves no record, and absence is exactly what this gate refuses: the refusal +// cases would pass FOR THE WRONG REASON while +// `a_dispatched_review_reaches_the_predicate_and_is_clean` failed. +// +// That asymmetry is the reason to gate the module rather than the one clean case. +// A suite whose negative arms pass because the subject never ran is the vacuous +// pass this whole family exists to refuse, and it would read as coverage. +// +// `bot_lane.rs`, `session_provisioning.rs` and `connector_allow_door.rs` gate +// their whole suites on this rung for the same reason. A `.cmd` twin of the stub +// would be a second authority over what the runner answers. +#![cfg(unix)] #![allow(clippy::unwrap_used, clippy::expect_used)] use crate::common; From 6d68d326bb0c4d09b72e9c92a75894c877b9fca2 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 2 Sep 2026 19:34:07 +0000 Subject: [PATCH 16/16] style(config): taplo-normalise the conflict resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One blank line. `verify`'s `ci:quick` refused the tree with `the file is not properly formatted`, and the cause is mine: I resolved the `[prune.*.basis]` rebase conflict by hand and did not re-run `mise run fmt` before letting `land` re-verify. THE RULE THE HAND-RESOLUTION SKIPPED: a conflict resolved by editing is a change like any other and owes the same formatter pass. Nothing distinguishes it from a typed edit except that a rebase produced the occasion. ITS OWN COMMIT RATHER THAN AMENDED IN. It belongs to `fix(prune)` two commits back, and reaching that needs a rebase over the Windows fix that sits between — a real risk of dropping work for one blank line. Amending HEAD instead would file a `batten.toml` whitespace change under a message about Windows test gating. AND IT STILL COSTS AN ADMISSION, which is the gate being right rather than pedantic: `protected-mutation` decides on the PATH, not on the size or the authorship of the diff. A formatter wrote this one and the write is still a write. Refs: CLOUD-1158 Admits: 1e307c3d9dcc2ea28e3a6a813a06f1b1c6eef4aa8e3cfb324441c7a9d14686fc Admits-rule: protected-mutation Admits-verdict: path write refused Admits-subject: batten.toml Admits-head: f2e8f82c5fc83de391af5386defd5875d9871ef6 Admits-epoch: 29583962133507378679aa607ee2522af964ed91665eeed73b45285dbb193072 Admits-author: alec@wenzowski.com Admits-prev: 214320fcb2a8b8664a92a47f868bcd383118bbb53cb5945de6fa0ba54a9588a3 Admits-answer-lost: `verify` refuses the tree on every lap, so `land` cannot complete one and eighteen commits stay unlandable. The refusal is correct and the remedy is exactly this one line; there is no smaller change that clears it. Admits-answer-precondition: The change is one blank line, produced by `taplo` through `mise run fmt` rather than typed: my hand-resolution of the `[prune.*.basis]` rebase conflict left a double blank line and `ci:quick` refused the tree with "the file is not properly formatted". The formatter IS the surface here and it ran — what it cannot do is carry its own admission, because the path is protected whatever wrote it. It lands in the diff a reviewer reads as a single deletion. Admits-answer-rejected-route: `config read first` does not apply: nothing about the file's meaning is in question, and reading it is how the stray line was found. `patch run first` is the closest thing to a real alternative and it is what produced this diff — `mise run fmt` is the patch, and it still leaves the write to be admitted. --- batten.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/batten.toml b/batten.toml index 1f0d51c7d..b45c131f4 100644 --- a/batten.toml +++ b/batten.toml @@ -6167,7 +6167,6 @@ measured = "2026-09-02" # and the caller collapses them — so an operator who reads the caller rather than # the callee deletes files and gets nowhere. - [prune.warm.basis] glob = "crates/batten/tests/**/*.rs" count = 175