diff --git a/.claude/settings.json b/.claude/settings.json index 9c8f4ae56..2e8e2e8cd 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -140,7 +140,18 @@ "mcp__serena__replace_symbol_body", "mcp__serena__safe_delete_symbol", "mcp__serena__write_memory", - "mcp__github__pull_request_read" + "mcp__github__pull_request_read", + "Bash(./target/debug/batten:*)", + "mcp__Claude_Code_Remote__unsubscribe_pr_activity", + "mcp__github__unsubscribe_pr_activity", + "mcp__Claude_Code_Remote__get_session", + "mcp__github__create_pull_request", + "mcp__github__update_pull_request", + "mcp__github__add_issue_comment", + "mcp__github__add_reply_to_pull_request_comment", + "mcp__github__list_pull_requests", + "mcp__github__get_check_run", + "mcp__github__get_job_logs" ], "deny": [ "mcp__Claude_Code_Remote__send_later", diff --git a/batten.toml b/batten.toml index e9a914fbd..147798e74 100644 --- a/batten.toml +++ b/batten.toml @@ -1403,11 +1403,25 @@ bytes the tracker returned, never a re-typed copy; do NOT re-type one by hand."" # at load any `while_marker` naming a family that sweep does not carry — so the # deny cannot outlive the work it was about. # -# NO OVERRIDE ROUTE AND NO `bypass_env`, deliberately. CLOUD-1311 measured that a -# prose escape hatch is the thing being fixed rather than a safety valve on it: -# the punt this refuses is itself a well-argued sentence, so a field accepting a -# well-argued sentence gates nothing. The honest escape is the one the remedy -# names. +# NO `bypass_env`, and CLOUD-1311's reason still holds for the SHAPE it ruled on: +# a prose escape hatch on the punt itself is the thing being fixed rather than a +# safety valve on it, because the punt this refuses is itself a well-argued +# sentence, so a field accepting a well-argued sentence gates nothing. +# +# "AND NO OVERRIDE ROUTE" IS WITHDRAWN (CLOUD-1823), because the sentence that +# followed it — "the honest escape is the one the remedy names" — became false in +# the one state that matters. The remedy names `verify`; where `verify` is red for +# reasons only a write can repair, re-running it cannot change its answer, and the +# write that would is this row's own subject. Measured twice, the second time +# costing a whole session, with `BATTEN_HOOK_BYPASS` as the only exit — the +# password shape CLOUD-1051 retired. +# +# THE ROUTE IS DECLARED ON THE CLASS, not here: `receipt read other` is vendored +# (`crates/batten/src/verdict.rs`) and shared with every other receipt row, so the +# repair reaches all of them. It is also a TIGHTENING — `honours_hatch` reads that +# field, so declaring the route is what stops the bare variable working — which is +# the opposite of the valve CLOUD-1311 refused. The difference is checkability: a +# red check on a named head is a fact a reader can verify, not a sentence. [[rule]] id = "turn mint ahead" kind = "receipt" diff --git a/crates/batten/src/verdict.rs b/crates/batten/src/verdict.rs index 64711b51e..43a242b98 100644 --- a/crates/batten/src/verdict.rs +++ b/crates/batten/src/verdict.rs @@ -1856,7 +1856,39 @@ covered the bytes it read and nothing later, so it is not evidence about this he the check against what is here now. Kept apart from the trunk case because the two name \ different things that moved, and a refusal that says the wrong one sends the reader after \ the wrong repair.", - routes: &[read("config read first", "batten.toml")], + routes: &[ + read("config read first", "batten.toml"), + // THE RE-RUN IS THE RIGHT FIRST ANSWER AND SOMETIMES CANNOT BE TAKEN + // (CLOUD-1823), which is the same hole `path write refused` had: + // a class whose only route is the thing the refusal blocks. + // + // Measured twice. `turn mint ahead` is keyed to the head and triggers + // on WRITE, so where the named check is red for reasons only a write + // can repair, "re-run the check" prescribes a run that cannot change + // its answer, and the write that would is the call being refused. The + // branch has no exit: every edit is denied, and the denial is + // discharged only by the edits. With no override route + // `admission::questions_for` returns `None`, so the sole remaining way + // through was `BATTEN_HOOK_BYPASS` — a knowable string that records + // nothing, which this repository already ruled on for `issue file + // same`: *the point of the admission mechanism is that the bare + // variable stops working*. Declaring this route is what MAKES it stop, + // because `hook::Policy::honours_hatch` reads exactly this field. + // + // The precondition is what the asker must be ABLE TO STATE, never a + // judgement the gate makes (non-negotiable rule 3). Both halves are + // checkable by a reader: whether the check is red on this head, and + // whether the work the receipt was about is on a remote. The second + // half is the one that matters — it is the harm the refusing row + // actually guards, and an asker who cannot state it is being stopped + // for the right reason. + admit( + "articulate the stale receipt", + "the check this receipt names is red on this head for a reason only a write can \ +repair, so re-running it cannot change its answer, and the work the receipt was taken about is \ +already pushed", + ), + ], applicability: Applicability::Advice, }, VendoredVerdict { diff --git a/crates/batten/tests/it/forge_facts.rs b/crates/batten/tests/it/forge_facts.rs index ef776ab4b..dd0e7a368 100644 --- a/crates/batten/tests/it/forge_facts.rs +++ b/crates/batten/tests/it/forge_facts.rs @@ -18,7 +18,7 @@ use crate::common; use std::path::{Path, PathBuf}; -use common::{batten, git_in, run_with_stdin, scratch, stderr, stdout, write}; +use common::{at_root, batten, git_in, init_repo, run_with_stdin, scratch, stderr, stdout, write}; /// The sha the row declares, and the one a record must be keyed to. const DECLARED_SHA: &str = "1111111111111111111111111111111111111111"; @@ -134,6 +134,112 @@ fn check(dir: &Path) -> std::process::Output { command.output().expect("run batten check") } +/// The committed config, pointed at the REAL module rather than the probe. +/// +/// The probe above pins the FACT — that the engine builds `input.tree.forge` and +/// keys it to the declared sha. It installs no module of this repository's, which +/// is exactly what `#MUTANT-OWNER CLOUD-845` declares on the suite: no case in it +/// can turn red under a mutation of the shipping predicate. +/// +/// So the cases below install the real thing. They are the second tier +/// `rules/policy-modules.md` demands and the only one a `#MUTANT` row over +/// `forge-verdict-required` can name honestly. +fn real_config(sha: &str) -> String { + format!( + r#"version = 1 + +[[rule]] +id = "forge check red" +kind = "policy" +scope = "tree" +module = "forge-verdict-required.rego" +severity = "deny" +forge = ["{sha}"] + +[[verdict]] +id = "forge check red" +gloss = "the forge judged this commit and its fan-in check did not pass" +class = "A fixture copy of the committed row; the id is what the module raises." + +[[verdict.route]] +id = "module read first" +kind = "document" +target = "forge-verdict-required.rego" +"# + ) +} + +/// A repository carrying the real module and a record whose body is `record`. +/// +/// THE WHOLE RECORD rather than one conclusion, because the anti-vacuity case +/// needs a second line — a leaf skipping beside a fan-in that genuinely failed. +fn real_fixture(name: &str, record: &str) -> PathBuf { + let dir = scratch(&format!("forge-real-{name}")); + write(&dir, "batten.toml", &real_config(DECLARED_SHA)); + // THE REAL MODULE, read off the tree rather than restated. A fixture copy + // would drift from the thing that ships, which is the whole failure this + // tier exists to catch one level up. + let module = std::fs::read_to_string(at_root("policy/forge-verdict-required.rego")) + .expect("read the shipping module"); + write(&dir, "forge-verdict-required.rego", &module); + // `init_repo`, never a `git init` fork: main's fixture-fork ratchet + // (`test add duplicate`) prices every ADDED fork, and under + // `CARGO_TARGET_TMPDIR` this copies the published template at zero forks. + init_repo(&dir); + let store = dir.join(".git").join("batten-forge"); + std::fs::create_dir_all(&store).expect("record store"); + std::fs::write(store.join(DECLARED_SHA), record).expect("write record"); + dir +} + +#[test] +fn a_skipped_fan_in_is_not_refused() { + // CLOUD-1831, over the compiled binary. A draft's checks are stamped + // `skipped`; `record-verdicts` writes that record; this row used to refuse + // it, and readying the pull request — the only thing that makes `final` + // report anything else — is downstream of the `land` the refusal stops. + // + // Measured on #973 and again on #974, where it cost four laps. + let dir = real_fixture("skipped", "final skipped\n"); + let outcome = check(&dir); + assert!( + outcome.status.success(), + "a declined fan-in was read as a refusal\n{}{}", + stdout(&outcome), + stderr(&outcome) + ); +} + +#[test] +fn a_cancelled_fan_in_is_not_refused() { + // The same class one cause over: a run superseded before it could answer. + // CLOUD-363 fixed this same word for `ci-wait` and `checks-green`. + let dir = real_fixture("cancelled", "final cancelled\n"); + let outcome = check(&dir); + assert!( + outcome.status.success(), + "a cancelled fan-in was read as a refusal\n{}{}", + stdout(&outcome), + stderr(&outcome) + ); +} + +#[test] +fn a_failed_fan_in_beside_a_skipped_leaf_is_still_refused() { + // THE HALF THAT KEEPS THE FIX FROM BECOMING CLOUD-900's FALSE PASS, and + // without it the two cases above are satisfied by a module that refuses + // nothing. The fan-in genuinely failed; a leaf skipping alongside it changes + // nothing about that. + let dir = real_fixture("failed-with-skip", "final failure\nwindows skipped\n"); + let outcome = check(&dir); + assert!( + !outcome.status.success(), + "a failed fan-in stopped being refused\n{}{}", + stdout(&outcome), + stderr(&outcome) + ); +} + #[test] fn a_declared_sha_reads_its_own_record() { // THE POSITIVE. Before this family a tree-scoped module asking about a diff --git a/crates/batten/tests/it/punt_receipt.rs b/crates/batten/tests/it/punt_receipt.rs index 8c75abec5..ae17129e3 100644 --- a/crates/batten/tests/it/punt_receipt.rs +++ b/crates/batten/tests/it/punt_receipt.rs @@ -35,7 +35,7 @@ use crate::common; use std::path::{Path, PathBuf}; -use common::{Fixture, git_in, run_with_stdin, stderr}; +use common::{Fixture, batten, git_in, run_with_stdin, stderr}; /// The committed row's shape, with nothing else declared. /// @@ -175,6 +175,123 @@ fn the_refusal_names_the_row_and_its_remedy() { ); } +/// The wedge, built the way it actually happened: a `verify` receipt minted at +/// one head, then a commit that moves the head past it. +/// +/// **Minted by the verb rather than written by hand**, because the receipt is an +/// in-toto statement whose `recorded_git_dir` and config epoch are read from this +/// checkout. A fabricated file would answer [`Validity::Missing`] and the cases +/// below would then be about the wrong class — `receipt read missing`, which has +/// always had a reachable remedy. +fn superseded(name: &str) -> PathBuf { + let dir = repo(name); + let recorded = batten() + .current_dir(&dir) + .args(["receipt", "record", "verify"]) + .output() + .expect("run batten receipt record"); + assert!( + recorded.status.success(), + "the premise is a receipt that WAS valid: {}", + stderr(&recorded) + ); + // The commit is what supersedes it, and it is the ordinary remedial action: + // AGENTS.md mandates committing early and often, and `key = "head"` makes + // exactly that void the evidence. + std::fs::write(dir.join("src/tracked.rs"), "// moved on\n").expect("move the bytes"); + git_in(&dir, &["add", "-A"]); + git_in(&dir, &["commit", "-q", "-m", "chore: move the head"]); + punt(&dir); + dir +} + +#[test] +fn a_superseded_receipt_still_refuses_an_unarticulated_write() { + // ANTI-VACUITY, and it is the half that keeps CLOUD-1823's route from being a + // password. Declaring an `override` route changes what is AVAILABLE, never + // what is decided: a caller who has articulated nothing is refused exactly as + // before, and the class it is refused under is unchanged. + let dir = superseded("punt-superseded-bare"); + let output = run_with_stdin( + &dir, + &["adjudicate", "--harness", "exit-code"], + &write_payload("src/tracked.rs"), + ); + assert_eq!( + output.status.code(), + Some(2), + "an unadmitted write over a stale receipt is still refused" + ); + let said = stderr(&output); + assert!( + said.contains("receipt read other"), + "and under the amend-or-rebase class: {said}" + ); +} + +#[test] +fn the_bare_variable_no_longer_clears_a_superseded_receipt() { + // THE TIGHTENING, and the assertion a reviewer of CLOUD-1823 should look for + // first. `hook::Policy::honours_hatch` stops honouring `BATTEN_HOOK_BYPASS` + // for any class declaring an `override` route with a precondition, so + // DECLARING the route is what takes the password away. Before it, this exact + // call exited 0 — measured on this repository's own wedged branch, where the + // variable was the only exit that existed. + // + // Pinned here rather than left to `hook.rs`'s unit tier because the unit tier + // fabricates a registry, and what this asserts is that the VENDORED registry + // the binary ships carries the route. + let dir = superseded("punt-superseded-hatch"); + let mut command = batten(); + command + .current_dir(&dir) + .args(["adjudicate", "--harness", "exit-code"]) + .env(batten::hook::BYPASS_ENV, "1") + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()); + let mut child = command.spawn().expect("spawn the bypassed adjudication"); + { + use std::io::Write as _; + child + .stdin + .take() + .expect("piped") + .write_all(write_payload("src/tracked.rs").as_bytes()) + .expect("write the payload"); + } + let output = child.wait_with_output().expect("collect the verdict"); + assert_eq!( + output.status.code(), + Some(2), + "the bare variable must not open a class that declares an articulation route: {}", + stderr(&output) + ); +} + +#[test] +fn the_class_declares_the_route_that_makes_the_wedge_escapable() { + // CLOUD-1823's own predicate, asked of the registry rather than of a refusal. + // + // The wedge was not that the refusal was wrong — it was that the ONLY declared + // remedy, re-running the named check, cannot change its answer when that check + // is red for reasons only a write repairs, and the write is what is refused. + // An `override` route carrying a precondition is what `admission::questions_for` + // reads, so its presence IS the exit existing. + let entry = batten::verdict::vendored() + .into_iter() + .find(|entry| entry.id == "receipt read other") + .expect("the class is vendored"); + assert!( + entry + .routes + .iter() + .any(|route| route.kind == batten::verdict::RouteKind::Override + && route.precondition.is_some()), + "the class must declare an articulation route, or the wedge returns" + ); +} + #[test] fn a_marker_no_sweep_clears_is_refused_at_load() { // THE SPEND IS WHAT MAKES THE REFUSAL FINITE. A `while_marker` naming a family diff --git a/mise.toml b/mise.toml index 85f06cdeb..c05199cb3 100644 --- a/mise.toml +++ b/mise.toml @@ -538,6 +538,17 @@ CI_ABSENT_OK_CHECKS = "zizmor,cross,musl,darwin-link (aarch64-apple-darwin),darw # conclusions — they are the absence of an answer, and reading either as red # wedges a branch with no exit. # +# THERE IS A THIRD READER, AND IT RECURRED EXACTLY AS THIS COMMENT PREDICTED +# (CLOUD-1831). `policy/forge-verdict-required.rego` is a `severity = "deny"` row +# that read `final skipped` as red — the same one word, the same wedge, measured +# on #973 and again on #974 where it cost four laps. It now carries the +# complement of this set as `non_answers`, and the two are COMPLEMENTS RATHER +# THAN COPIES for a reason stated there: the module cannot spell this as the +# answered set without reintroducing CLOUD-900's false pass, because a record +# that omits the fan-in entirely fails an is-an-answer test too and would stop +# being refused. So a change to either list owes a look at the other, and the +# module's own comment points back here. +# # THE CATCH-ALL IS GONE, which is the half that makes the next unknown conclusion # FAIL SAFE. `checks-green` used to define red by negation, so a conclusion nobody # had seen would be reported red on a head that had not been judged. Not in this diff --git a/policy/forge-verdict-required.rego b/policy/forge-verdict-required.rego index 2b5cd40f3..a98281e4d 100644 --- a/policy/forge-verdict-required.rego +++ b/policy/forge-verdict-required.rego @@ -61,6 +61,15 @@ refused contains sha if { # every failure becomes manufacturable by omitting a job. Negating a helper # is what makes absent and wrong the same refusal. not passed(checks) + + # AND THE FORGE ACTUALLY ANSWERED (CLOUD-1831). `passed` alone read "the forge + # declined to run" as "the forge said no", which wedges a branch with no exit: + # a draft's checks are stamped `skipped`, `record-verdicts` writes that record, + # this row refuses it, `verify` exits 2 — and readying the pull request, the + # only thing that would make `final` report anything else, is downstream of the + # `land` that `verify` just stopped. Measured 2026-09-17 on #973, and again on + # #974 where it cost four laps. + not declined(checks) } # The fan-in reported, and it passed. Anything else — reported and failed, or @@ -69,6 +78,30 @@ passed(checks) if { checks[required] == "success" } +# The fan-in DECLINED to answer rather than answering no. +# +# STATED AS THE NON-ANSWER SET AND NEGATED, which is forced rather than stylistic. +# The obvious spelling is the complement — require `checks[required]` to be one of +# `CI_ANSWERED_CONCLUSIONS` — and it silently reintroduces CLOUD-900's false pass: +# a record that OMITS `final` leaves `checks[required]` undefined, so an +# is-an-answer test does not hold either, and the omission stops being refused. +# Negating the non-answer keeps absent and wrong the same refusal, exactly as +# `not passed(..)` above does. +# +# THE COMPLEMENT LIVES IN `mise.toml` as `CI_ANSWERED_CONCLUSIONS`, which +# `checks-green` and `ci-wait` read (CLOUD-363 fixed this same word in those two). +# The two spellings are complements rather than copies — this one cannot be +# written as the answered set for the reason above — so `mise.toml`'s own comment +# points here, and a change to either owes a look at the other. Three readers of +# one word is how this recurred. +declined(checks) if { + checks[required] in non_answers +} + +# `skipped` is a draft's stamp and `cancelled` is a superseded run's; neither is +# a judgement about the commit. +non_answers := {"skipped", "cancelled"} + violation contains { "rule": "forge check red", "verdict": "forge check red", @@ -115,6 +148,46 @@ test_could_not_look_does_not_fault if { count(violation) == 0 with input as {"tree": {"forge": null}} } +# THE FORGE DECLINED TO RUN (CLOUD-1831). A draft's checks are stamped `skipped`, +# and a skip is the absence of an answer rather than an answer of no. Refusing it +# wedges the branch: readying the pull request is the only thing that makes +# `final` report anything else, and it is downstream of the `land` this refusal +# stops. +test_a_skipped_fan_in_is_not_refused if { + count(violation) == 0 with input as recorded({"final": "skipped"}) +} + +# `cancelled` is the same class, one cause over: a run superseded before it could +# answer. CLOUD-363 fixed this same word for `ci-wait` and `checks-green`. +test_a_cancelled_fan_in_is_not_refused if { + count(violation) == 0 with input as recorded({"final": "cancelled"}) +} + +# THE PAIR THAT KEEPS THE FIX FROM BECOMING CLOUD-900's FALSE PASS, and it is the +# load-bearing half. A non-answer is admitted only where the fan-in SAID so — a +# record that omits `final` entirely is still refused (the case above), and a +# leaf skipping while the fan-in genuinely failed is still refused here. Without +# this, "distinguish a non-answer" collapses into "admit anything that is not +# success", which is the predicate this module was written to avoid. +test_a_failed_fan_in_beside_a_skipped_leaf_is_still_refused if { + some v in violation with input as recorded({"final": "failure", "windows": "skipped"}) + v.verdict == "forge check red" +} + #MUTANT-SUITE crates/batten/tests/it/forge_facts.rs -#MUTANT-OWNER CLOUD-845|the tier this module names drives `input.tree.forge` and never installs the module, so no case in it can turn red under a mutation of the predicate -#MUTANT refusal-unread|s@^\tcount(refused) > 0$@\tfalse@|a_declared_sha_reads_its_own_record +# THE CLOUD-845 OWNER NOTE IS WITHDRAWN, because its premise stopped being true +# (CLOUD-1831). It read "the tier this module names drives `input.tree.forge` and +# never installs the module, so no case in it can turn red under a mutation of +# the predicate" — correct while every case there drove `probe.rego`. The suite +# now installs THIS module from the tree in `real_fixture`, so a mutation of the +# predicate is observable and the exemption would be a stale claim about the +# world rather than a declared gap. +#MUTANT refusal-unread|s@^\tcount(refused) > 0$@\tfalse@|a_failed_fan_in_beside_a_skipped_leaf_is_still_refused +# CLOUD-1831's two, and they are a PAIR that cannot shadow each other — which is +# the whole reason the row names two. The first reinstates the wedge: drop the +# declined conjunct and a skipped fan-in is refused again. The second reinstates +# CLOUD-900's false pass from the other side: widen `non_answers` to admit every +# non-success conclusion and a genuine `failure` stops being refused. A fix that +# survived both would be one that admitted everything. +#MUTANT skipped-fan-in-refused|s@^\tnot declined(checks)$@\ttrue@|a_skipped_fan_in_is_not_refused +#MUTANT missing-fan-in-admitted|s@^\tchecks\[required\] in non_answers$@\tchecks[required] != "success"@|a_failed_fan_in_beside_a_skipped_leaf_is_still_refused