diff --git a/.serena/memories/workflow/landing-loop.md b/.serena/memories/workflow/landing-loop.md index 0343e8a27..af365ae17 100644 --- a/.serena/memories/workflow/landing-loop.md +++ b/.serena/memories/workflow/landing-loop.md @@ -220,6 +220,67 @@ The composition itself is a declared list with a durable compensation per step (CLOUD-1556 carries the Pkl default and its schema; CLOUD-1564 `Push`'s row), and the sleep policy that the raced wait needs is CLOUD-1557. +## The write gate, and the two stores that decide when it opens + +`turn mint ahead` (`batten.toml:1411`) denies every mediated **write** while the +`unlanded-nudged` marker is set, until a GREEN `verify` receipt exists **for the +current HEAD** (`checks = ["verify"]`, `key = "head"`). No override route and no +`bypass_env`, deliberately. Two things follow that cost a session each. + +**Record the branch plan TERMINAL before running `verify`.** `plan cover partial` +/ `plan declare held` (`batten.toml:6218`) is **tree-scoped**: any `pending` or +`in_progress` entry in `.git/batten-receipts/plan.` reds every +`batten-check`, therefore every `verify`, therefore writes no receipt, therefore +shuts the write gate. `deleted` is the withdrawal token the gate itself names +(`policy/plan-complete.rego:76-81` splits terminal from in-flight), and +`batten record plan` stays reachable while the write gate holds — it is a verb, +not a mediated write. `printf 'CLOUD-1234 deleted' | batten record plan` is the +free exit; escalating this to a human as an override request is reading the +refusal instead of the rule. + +**A red `verify` at HEAD with the marker set is a genuine deadlock, and the exit +is to move HEAD, not to route around the gate.** Measured 2026-09-18: a landed +`clippy` denial needed a one-line source edit, the edit was a write, and the write +needed a green receipt the edit was the only way to earn. Running `verify` again +does not open it — a red run writes nothing. What works, and what does not +launder the gate: **push first** (so nothing is lost), then `git reset --soft` to +the last commit that HAS a green `verify.` receipt under +`$GIT_DIR/batten-receipts/`. The tree keeps every change, HEAD is a head the gate +already accepts, and the fix lands in that window. A `--force-with-lease` push +afterwards must name the sha it replaces (`=:`); `branch write unsafe` +refuses the bare form, because a bare lease compares against a tracking ref a +`fetch` just moved. + +Amending or letting `land` rebase invalidates the receipt the same way — so do +the whole edit inside one green window and commit last, rather than committing +into a window you then have to re-earn. + +**`refusal::first_sighting` keys its store under `$GIT_DIR`**, not the state root +(`refusal.rs:367`, `STORE = "batten-sightings"` at `:402`, fail-open → `true`). A +repeat renders SHORT — `hook::deny_text` returns early at `hook.rs:4832` and drops +the `— ` clause with every remedy. For a TEST this is the trap: a fixture +with no `.git` of its own resolves the **enclosing repository's** store, so a +"cold state root" controls nothing and two fixtures share one sighting history. +Give the fixture its own repository. A tree-wide `GIT_CEILING_DIRECTORIES` is the +wrong instrument and reds `acceptance_corpus`, whose fixtures legitimately inherit +the enclosing repo. And a case asserting two refusals render ALIKE passes +vacuously when both are short: assert the first-sighting rendering (the `—`) too, +or the mutant survives. + +## A control must control the thing the code actually reads + +Three wrong diagnoses in one session, each reported before any was reproduced, +one of them filed as an Urgent row that then had to be rewritten with a +correction: concurrency over `/tmp`; a shared state root; a fixture-builder +mismatch. The real cause was the `$GIT_DIR` store above — which none of the three +controls touched. The reproduction that "confirmed" a divergence reused one +fixture repository, so runs 2–4 were repeats **by construction** and the varying +output was the short rendering, not the variable under test. + +Before filing, reproduce with the suspected variable held fixed by a mechanism you +have **read in the source**, not assumed from its name. If the control does not +provably reach the value the code reads, the run measured nothing. + ## Rollout posture Every mechanism here fails open on a clone that predates it, so none of them diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index c867e55f4..21bd406f6 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -18536,6 +18536,22 @@ fn apply_admissions( // and `land` replays exactly that commit. Resolved lazily — one read per // run, and none on a run the store answers or that has nothing to admit. let mut head_message: Option> = None; + // AND ITS SECOND PARENT, WHERE HEAD IS A MERGE THE FORGE MINTED (CLOUD-1674 + // again, on the surface it was written for). A forge that checks a pull + // request out by its MERGE REF hands the runner a commit IT synthesised — + // parents (base, head), message `Merge into ` — rather than the + // branch head. So the block arm above read the one commit that structurally + // cannot carry a block, and CLOUD-1674's whole "exit 0 here, exit 2 in CI" + // asymmetry survived its own fix. Measured: `batten-check` admitted a spend + // locally and refused the identical commit on the runner. + // + // `HEAD^2` AND NOT A RANGE WALK, which this function's own doc rejects for a + // reason that still holds: a walk would let an old block admit a later + // finding sharing its fingerprint. A merge's second parent is one commit, + // and on a forge-minted merge ref it is exactly the branch head `land` + // replays — the same commit the HEAD arm reads everywhere else. On a HEAD + // that is not a merge there is no such parent and nothing changes. + let mut merged_head_message: Option> = None; let mut kept = Vec::with_capacity(findings.len()); for finding in findings { @@ -18566,6 +18582,25 @@ fn apply_admissions( ) }); } + //MUTANT-SUITE crates/batten/tests/it/admission.rs + //MUTANT merge-parent-arm-removed|s@^ if admitted.is_none() {$@ if false {@|a_spent_block_on_a_merge_refs_second_parent_admits + if admitted.is_none() { + let message = merged_head_message.get_or_insert_with(|| { + git::commit_record(root, "HEAD^2") + .ok() + .map(|record| record.body) + }); + admitted = message.as_deref().and_then(|body| { + admission::admitted_by_block( + body, + &finding.rule, + class, + &finding.path, + &anchor, + &epoch, + ) + }); + } match admitted { Some(address) => output::message( mode, diff --git a/crates/batten/tests/it/admission.rs b/crates/batten/tests/it/admission.rs index 9e13125e8..94938ecde 100644 --- a/crates/batten/tests/it/admission.rs +++ b/crates/batten/tests/it/admission.rs @@ -1088,6 +1088,90 @@ fn a_spent_block_in_the_head_commit_admits_with_no_store() { ); } +/// Put `root` on a forge-shaped merge ref: HEAD is a merge commit carrying +/// `message`, and its SECOND parent is a commit whose message is `carried`. +/// +/// What `actions/checkout` hands a runner on a `pull_request` event, reduced to +/// the two properties that decide this: the checked-out commit is synthesised by +/// the forge, so its message is never an author's, and the branch head is its +/// second parent. `ci.yml:252` already records that this is what CI checks out. +fn merge_ref_over(root: &Path, carried: &str, message: &str) { + let base = common::git_in(root, &["rev-parse", "HEAD"]); + common::git_in(root, &["checkout", "-q", "-b", "pr"]); + common::git_in(root, &["commit", "-q", "--allow-empty", "-m", carried]); + common::git_in(root, &["checkout", "-q", &base]); + common::git_in(root, &["merge", "-q", "--no-ff", "-m", message, "pr"]); +} + +#[test] +fn a_spent_block_on_a_merge_refs_second_parent_admits() { + // CLOUD-1674's OWN SURFACE, which its fix did not reach. The block arm above + // reads HEAD, on the premise that HEAD is the commit carrying the admitted + // change. On the runner it is not: `actions/checkout` checks out the pull + // request's MERGE REF, so HEAD is a commit the forge synthesised whose + // message is `Merge into ` and which structurally cannot carry + // a block. Measured on #984 — `batten-check` admitted the spend locally and + // refused the identical commit in CI, which is the exact asymmetry the block + // was introduced to end. + let root = admits_fixture("block-merge-ref"); + let (address, block) = spend_block_for(&root, "a.rs", "the block must survive the merge ref"); + forget_store(&root); + + merge_ref_over( + &root, + &format!("carry\n\n{block}"), + "Merge deadbeef into cafef00d", + ); + + // The merge commit's OWN message carries nothing — if this arm ever passes + // because the block leaked into HEAD's message, the case is testing the arm + // above instead of this one. + let head = common::git_in(&root, &["log", "-1", "--format=%B"]); + assert!( + !head.contains("Admits:"), + "the merge commit must carry no block, or this case proves nothing: {head}" + ); + + let admitted = common::run(&root, &["check"]); + assert_eq!( + admitted.status.code(), + Some(batten::exit::ExitCode::Success.code()), + "the block on the merge ref's second parent admits: {}", + common::stderr(&admitted) + ); + let reported = common::stderr(&admitted); + assert!( + reported.contains("admitted a.rs") && reported.contains(&address), + "the run names what it admitted and which record did it: {reported}" + ); +} + +#[test] +fn a_merge_ref_whose_second_parent_carries_no_block_still_refuses() { + // THE ANTI-VACUITY ARM, and it is the load-bearing one: reading `HEAD^2` at + // all is only sound while it admits what that commit actually carries. A + // second parent with no block must leave the finding exactly where the store + // and the HEAD arm already left it — otherwise "look one commit further" + // has become "stop asking". + let root = admits_fixture("block-merge-ref-empty"); + let (_, _) = spend_block_for(&root, "a.rs", "spent, then left off the commit"); + forget_store(&root); + + merge_ref_over( + &root, + "carry, with no block", + "Merge deadbeef into cafef00d", + ); + + let refused = common::run(&root, &["check"]); + assert_eq!( + refused.status.code(), + Some(batten::exit::ExitCode::Violation.code()), + "neither HEAD nor its second parent carries a block, so the finding stands: {}", + common::stderr(&refused) + ); +} + #[test] fn a_tampered_block_in_the_head_commit_admits_nothing() { // THE TAMPER CHECK the store arm gets for free by owning its records, paid