From 73d098e2030ab90b2244dd1c4fefd078fa6b0ade Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Thu, 17 Sep 2026 16:02:36 +0000 Subject: [PATCH 1/2] fix(config): grant the verbs this workflow calls, without the reduced connector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two prompts per session for `unsubscribe_pr_activity` is the acute symptom, and it is absurd on its face: `permissions.deny` already blocks SUBSCRIBING, so unsubscribing is the call that restores the declared posture. It asked for approval anyway, twice, because it was in neither list. WHAT IS DELIBERATELY NOT HERE, and it is the half worth reading. Two attempts at this added Linear grants and `connector grant loose` refused the tree for both. The first added the reads (`get_issue`, `list_issues`, `list_comments`); the second kept the WRITES (`save_issue`, `save_comment`) on the theory that a `record = "board-writes"` row is captured rather than reduced. Both were wrong, and the module says why in one line: reduced_connector := "mcp__Linear" The predicate is a PREFIX over the whole server, not a per-tool list — "a grant naming one tool and a grant globbing the whole server are the same finding, because either one puts the unreduced payload back on the model's surface". So no `mcp__Linear*` entry belongs here at all, and the route stays `batten mcp call`. The measurement behind it: the raw tools WERE registered once, so that session called them 973 times for 13.2 MB and 73% of all its tool output, against 1.9 MB for Bash, Grep and Read together. What goes in is what has no reduction to defeat: the two unsubscribe verbs, the forge verbs this loop actually calls, `get_session`, and `./target/debug/batten`, which `Bash(batten:*)` does not match because it is path-qualified. `timeout` is 473 calls and stays out: it wraps an arbitrary command, so allowlisting it is a general execution grant wearing a prefix. The `hooks` block is untouched. Refs: CLOUD-1780 --- .claude/settings.json | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) 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", From 3e8d14b84796d1f5ec868bf2d16df7194fd6a8ee Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Fri, 18 Sep 2026 08:38:21 +0000 Subject: [PATCH 2/2] fix(policy): a skipped fan-in is a non-answer, not a red verdict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `forge check red` wedged its own branch. `land` lap 1 waits for CI, and its undo returns the pull request to draft. A draft buys no runner, so the run concludes with every required check `skipped` — including `final`. `record-verdicts` writes whenever the fan-in has a non-null conclusion, and `skipped` is non-null, so the record lands. This row then read it as red. The only declared remedy for `forge check red` is `mise run land`, and `land` runs `verify` first, which this row stops. `HEAD~1` is live `main`, so the head cannot change on its own. Measured on #973, and again on #974 where it cost four laps. 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 the 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(..)` already does. THE ANTI-VACUITY CASE IS THE LOAD-BEARING ONE. A non-answer is admitted only where the fan-in SAID so: a record omitting `final` is still refused, and a leaf skipping beside a genuinely failed fan-in is still refused. Without that pair, "distinguish a non-answer" collapses into "admit anything that is not success". THE COMPLEMENT LIVES IN `mise.toml` as `CI_ANSWERED_CONCLUSIONS`, whose own comment predicted a third reader of this one word and now names this module. The two are complements rather than copies, for the reason above, so a change to either owes a look at the other. The mutation pair: drop the `declined` conjunct and the wedge returns; widen `non_answers` to every non-success conclusion and a genuine `failure` stops being refused. A fix surviving both would be one that admitted everything. `crates/batten/tests/it/forge_facts.rs` now installs THIS module from the tree rather than `probe.rego`, which is what makes those mutations observable — so the CLOUD-845 owner note on `refusal-unread` is withdrawn: its premise ("the tier never installs the module") stopped being true. Refs: CLOUD-1831 --- crates/batten/tests/it/forge_facts.rs | 112 +++++++++++++++++++++++++- mise.toml | 11 +++ policy/forge-verdict-required.rego | 77 +++++++++++++++++- 3 files changed, 197 insertions(+), 3 deletions(-) diff --git a/crates/batten/tests/it/forge_facts.rs b/crates/batten/tests/it/forge_facts.rs index ef776ab4b..52453f412 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, 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,116 @@ 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 `final` is `conclusion`. +fn real_fixture(name: &str, conclusion: &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); + git_in(&dir, &["init", "-q", "-b", "main", "."]); + let store = dir.join(".git").join("batten-forge"); + std::fs::create_dir_all(&store).expect("record store"); + std::fs::write(store.join(DECLARED_SHA), format!("final {conclusion}\n")) + .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", "skipped"); + 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", "cancelled"); + 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 = scratch("forge-real-failed-with-skip"); + write(&dir, "batten.toml", &real_config(DECLARED_SHA)); + 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); + git_in(&dir, &["init", "-q", "-b", "main", "."]); + let store = dir.join(".git").join("batten-forge"); + std::fs::create_dir_all(&store).expect("record store"); + std::fs::write(store.join(DECLARED_SHA), "final failure\nwindows skipped\n") + .expect("write record"); + 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/mise.toml b/mise.toml index bd09d20ab..2ce5d2692 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