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/batten.toml b/batten.toml index 4fede06cd..b45c131f4 100644 --- a/batten.toml +++ b/batten.toml @@ -5064,6 +5064,62 @@ key = "CLOUD-1188" node = "project" reduce = "present" +# 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" + +# 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" + # 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`). @@ -5086,10 +5142,19 @@ reduce = "present" # 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. +# 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" @@ -6079,7 +6144,28 @@ 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" @@ -7105,6 +7191,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" @@ -9267,7 +9357,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" @@ -9294,13 +9384,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" @@ -9672,6 +9804,54 @@ 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" + +# 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" gloss = "readying would buy a CI matrix on a head carrying unresolved review threads" diff --git a/crates/batten/src/config.rs b/crates/batten/src/config.rs index eebc5adb7..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. @@ -1718,6 +1733,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..5aae9d969 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,126 @@ 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, + /// 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. + /// + /// 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. + /// + /// **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, +} + +/// 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/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..79790ad83 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; @@ -2797,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( @@ -3927,6 +3947,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/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 new file mode 100644 index 000000000..429b5063b --- /dev/null +++ b/crates/batten/src/review.rs @@ -0,0 +1,517 @@ +//! 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, +} + +/// 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 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, 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] +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; + }; + // 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. + 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 + // 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, &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; + }; + 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) +} + +/// 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, + subject_digest: &str, +) -> Dispatch { + let Some((text, _)) = prompt(&row.prompt) else { + return Dispatch::NoRunner; + }; + // 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. + // 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. + // + // 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. + // 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 + // findings parsed out of a failed run describe a review that did not finish. + if code != 0 { + 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() + { + 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 +/// 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")) + }) +} + +/// 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/review/ready-pressure-test.md b/crates/batten/src/review/ready-pressure-test.md new file mode 100644 index 000000000..1192e3103 --- /dev/null +++ b/crates/batten/src/review/ready-pressure-test.md @@ -0,0 +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. + +## 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?** + +## 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. + +Any line that is not exactly three whitespace-separated fields causes the whole +run to be discarded, so do not explain yourself. 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/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/facts.rs b/crates/batten/tests/it/facts.rs index a7d592bfe..f2b9c140f 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, @@ -152,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/crates/batten/tests/it/harness_wiring.rs b/crates/batten/tests/it/harness_wiring.rs index 54595527f..63f7c3aaf 100644 --- a/crates/batten/tests/it/harness_wiring.rs +++ b/crates/batten/tests/it/harness_wiring.rs @@ -558,8 +558,28 @@ 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", + // 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); git_in(&repo, &["init", "-q", "-b", "main", "."]); 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/ready.rs b/crates/batten/tests/it/ready.rs index 0d3a01b9e..3f20543d3 100644 --- a/crates/batten/tests/it/ready.rs +++ b/crates/batten/tests/it/ready.rs @@ -318,6 +318,184 @@ 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. +// 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 + // 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(); + // 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, + "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. +#[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)); + // 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. +#[cfg(unix)] +#[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. +#[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); + 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. +#[cfg(unix)] +#[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. +#[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); + 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/crates/batten/tests/it/review_dispatched.rs b/crates/batten/tests/it/review_dispatched.rs new file mode 100644 index 000000000..39abbbd12 --- /dev/null +++ b/crates/batten/tests/it/review_dispatched.rs @@ -0,0 +1,519 @@ +//! `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. + +// 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; + +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\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 _; + 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() { + // 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( + &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()]); +} + +/// 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_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" + ); +} + +// --------------------------------------------------------------------------- +// 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" + ); +} + +/// 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/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/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/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/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..1ef8a1dfa 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 } @@ -710,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 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 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 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", 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 diff --git a/schema/batten.local.schema.json b/schema/batten.local.schema.json index c410a3ecc..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": [ @@ -622,6 +637,65 @@ } ] }, + "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.\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" + ] + }, + "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" + }, + "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" + ] + }, "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 +1242,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..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", @@ -2356,6 +2371,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": [ @@ -2575,6 +2597,65 @@ } ] }, + "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.\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" + ] + }, + "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" + }, + "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" + ] + }, "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 +3257,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`.",