From 12d96ef875420f3c04a2843f4df00c8fd088e88a Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 9 Sep 2026 19:30:58 +0000 Subject: [PATCH 01/18] fix(hook): deny a call this build cannot adjudicate, rather than exiting non-zero MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `load_policy` failed with `?`, which raises a `UsageError` — exit `1` — and `exit.rs` makes only `2` a denial precisely so no failure path can block a call. The harness therefore read a config this build could not load as a non-blocking hook error and ran the mediated tool anyway. Measured over one 5-day session: 1,149 calls proceeded unjudged through seven windows of a mid-edit `batten.toml`, and ~456 more through a preset the installed build did not ship — `policy.rs`'s unknown-preset arm raises exactly this error. The discrimination is CLOUD-1572's, one level up. Where the engine is guessing about the call — unreadable stdin, an undecodable payload, an event the host does not declare — allowing is right, because nothing is known. Here the engine has read its own authority and been told it cannot enforce it, so proceeding reports a clean allow over rules that never ran. It renders rather than propagates, because `render` owns the per-harness deny channel: Claude Code answers in its JSON decision object at exit `0`, where the document is the deny, and the neutral adapter answers `Violation`. A `Denial` raised here would send `2` to a host that reads the document instead. The bypass is honoured first, which is what keeps a container recoverable: a stale binary meeting a newer config denies every call until one of them moves. Refs: CLOUD-1688 --- crates/batten/src/lib.rs | 65 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 2232a6c43..7fb220a5d 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -12281,8 +12281,71 @@ fn run_hook( // arms, ~0.7 ms against a 100 ms budget. `!adjudicable` keeps its old // behaviour, because an event with nothing to adjudicate has no protected // gate to run either, and that is the arm the hot path actually rides. + // A CONFIG THIS BUILD CANNOT READ IS CERTAINTY, AND CERTAINTY DENIES + // (CLOUD-1688). `?` here propagated a `UsageError` — exit `1` — and + // `exit.rs` makes only `2` a denial precisely so no FAILURE path can block a + // call. So the harness read this whole class as a non-blocking hook error + // and ran the tool anyway. Measured over one 5-day session: 1,149 calls + // proceeded unjudged through seven windows of a mid-edit `batten.toml`, and + // ~456 more through a preset this build did not ship (`policy.rs`'s + // unknown-preset arm, which raises exactly this error). + // + // THE DISCRIMINATION IS THE SAME ONE `UNREADABLE_STDIN` SITS ON THE OTHER + // SIDE OF, and CLOUD-1572 drew it one level down. Where the engine is + // GUESSING about the call — stdin it could not read, a payload that would + // not decode, an event the host does not declare — allowing is right, + // because nothing is known and refusing would make Batten the reason a + // session cannot proceed. Here the engine has READ its own authority and + // been told it cannot enforce it: the rule set is named, and unavailable. + // Proceeding then is not caution — it is a gate reporting a clean allow over + // rules it never ran, which is the false green this engine exists to catch. + // + // A DECISION, NOT AN ERROR, which is why it RENDERS rather than propagates. + // `render` owns the per-harness deny channel, so Claude Code gets its JSON + // decision object at exit `0` — where the document is the deny — and the + // neutral adapter gets `Violation`. Raising a `Denial` here would send `2` + // to a host that reads the document instead, which is the one number that + // host does not consult. + // + // THE HATCH IS HONOURED FIRST, and that is what keeps a container + // recoverable rather than bricked. A stale binary meeting a newer config + // denies every call until one of them moves, so the operator's declared + // escape has to still work — the bootstrap window CLOUD-1688 flags as + // needing a decision is exactly this state, and this arm is the part of it + // that can be settled without one. let (policy, waivers) = if adjudicable { - load_policy(overrides, harness)? + match load_policy(overrides, harness) { + Ok(loaded) => loaded, + Err(_) if bypass => (hook::Policy::declaring_nothing(harness), Vec::new()), + Err(unreadable) => { + // Pointer-only (non-negotiable rule 4): the loader's own message + // names the key or path that would not load, never its contents. + let refusal = Refusal::new( + "engine-cannot-adjudicate", + format!( + "this build could not load the rules it is registered to enforce, so \ + nothing judged this call: {unreadable}" + ), + // No remedy the ENGINE may declare: the repair is rebuilding + // or reinstalling the binary, or fixing the config, and both + // are the consumer's own commands (non-negotiable rule 1). + Fix::None, + ); + let rendering = Rendering { + hatch: hook::BYPASS_ENV, + ceiling: None, + }; + return render( + harness, + &envelope, + hook::Decision::Deny(refusal), + &rendering, + mode, + out, + err, + ); + } + } } else { (hook::Policy::declaring_nothing(harness), Vec::new()) }; From 8f1850df1d8c2cf6765da64f99036421cc3e5b18 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 9 Sep 2026 19:36:59 +0000 Subject: [PATCH 02/18] test(hook): show the fail-open arm can fail, and that its fix is not an outage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four cases over the compiled binary, because the defect is not in `adjudicate` — which is pure and whose unit cases passed throughout — but in what the boundary does with a load that failed. `mediated_admission.rs` records the same lesson from the other side. The pairing is the point. Under the declared mutation `unloadable-config-allows`, which restores the old fall-through, the two deny cases redden and the two allow cases stay green: FAIL a_config_this_build_cannot_load_denies_rather_than_failing_open FAIL on_claude_code_the_refusal_is_the_document_rather_than_the_number PASS a_loadable_config_still_allows_an_ordinary_call PASS the_declared_hatch_still_reaches_a_clone_whose_config_will_not_load Proved by hand rather than left to the nightly. The mirror is what stops the change being satisfied by an adjudicator that denies every call in the fleet, which is an outage wearing a fix's clothes; the hatch case is what keeps a container recoverable when a stale binary meets a newer config. The fixture is a `batten.toml` mid-edit, which is the largest measured bucket: seven windows across one 5-day session, 1,149 calls, none of them judged. Refs: CLOUD-1688 --- crates/batten/src/lib.rs | 2 + crates/batten/tests/it/adjudicate_absent.rs | 149 ++++++++++++++++++++ crates/batten/tests/it/main.rs | 1 + 3 files changed, 152 insertions(+) create mode 100644 crates/batten/tests/it/adjudicate_absent.rs diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 7fb220a5d..6811c6ab0 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -12313,6 +12313,8 @@ fn run_hook( // escape has to still work — the bootstrap window CLOUD-1688 flags as // needing a decision is exactly this state, and this arm is the part of it // that can be settled without one. + //MUTANT-SUITE crates/batten/tests/it/adjudicate_absent.rs + //MUTANT unloadable-config-allows|s@ Err(_) if bypass => (hook::Policy::declaring_nothing(harness), Vec::new()),@ Err(_) => (hook::Policy::declaring_nothing(harness), Vec::new()),@|a_config_this_build_cannot_load_denies_rather_than_failing_open let (policy, waivers) = if adjudicable { match load_policy(overrides, harness) { Ok(loaded) => loaded, diff --git a/crates/batten/tests/it/adjudicate_absent.rs b/crates/batten/tests/it/adjudicate_absent.rs new file mode 100644 index 000000000..232ebca04 --- /dev/null +++ b/crates/batten/tests/it/adjudicate_absent.rs @@ -0,0 +1,149 @@ +//! A call this build cannot adjudicate is DENIED, never allowed by a failure. +//! +//! The tier that proves the engine does not fail open when it has read its own +//! authority and been told it cannot enforce it. Unit cases over `adjudicate` +//! cannot host this: the defect is not in the decision, it is in what the +//! BOUNDARY does with a load that failed, and only the compiled binary answers +//! that — `mediated_admission.rs`'s header records the same lesson, where unit +//! cases passed while the binary allowed the write. +//! +//! # The mirror is not decoration +//! +//! `a_loadable_config_still_allows_an_ordinary_call` is what stops this being +//! satisfied by an adjudicator that denies everything. A fail-closed hook that +//! refuses each call is not a fix, it is an outage — CLOUD-1688's falsifier says +//! so in as many words, and that is why the pair lands together. +//! +//! # Scope +//! +//! The CONFIG half of CLOUD-1688: a `batten.toml` this build cannot load. The +//! VERB half — a registration spelling a subcommand the installed binary does +//! not have — needs `doctor` to interrogate the installed artifact rather than +//! itself, because a self-check runs in the build that mise resolves and never +//! in the one the hook does. It lands with that part and belongs in this file. + +// Panicking on setup failure is the idiomatic way for a test to fail loudly. +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use crate::common; + +use std::path::{Path, PathBuf}; + +use common::{Fixture, run_with_stdin}; + +/// A `batten.toml` mid-edit, which is the largest measured bucket: seven windows +/// across one 5-day session, 1,149 calls, every one of them unjudged. +const WILL_NOT_PARSE: &str = "version = 1\nthis is not toml\n"; + +/// A config that loads and declares nothing this call matches. +const LOADS: &str = "version = 1\n"; + +/// A fixture carrying `body` as its committed authority. +fn fixture(name: &str, body: &str) -> PathBuf { + Fixture::new(name) + .config(body) + .file("notes.md", "ordinary\n") + .git() + .base_commit() + .build() +} + +/// A Claude Code `PreToolUse` envelope carrying a command, so the boundary +/// treats it as adjudicable and reaches the config load at all. +fn payload() -> String { + "{\"hook_event_name\":\"PreToolUse\",\"tool_name\":\"Bash\",\ + \"tool_input\":{\"command\":\"echo hello\"}}" + .to_owned() +} + +/// Adjudicate on the neutral adapter, where the VERDICT IS THE NUMBER. +/// +/// `exit-code` rather than `claude-code` for the two cases that ask what the +/// code is: on Claude Code the deny is the JSON document at exit `0`, so a case +/// asserting a number there would assert the wrong channel. The document is +/// checked separately below. +fn code(dir: &Path) -> Option { + run_with_stdin(dir, &["adjudicate", "--harness", "exit-code"], &payload()) + .status + .code() +} + +#[test] +fn a_config_this_build_cannot_load_denies_rather_than_failing_open() { + // The defect: this was `2` only if the load succeeded. A `?` on the load + // raised a `UsageError` — exit `1` — and a harness reads `1` as a + // non-blocking hook error, so the mediated tool ran with nothing judging it. + let dir = fixture("adjudicate-unloadable", WILL_NOT_PARSE); + assert_eq!( + code(&dir), + Some(2), + "a call nothing could judge must be refused, not allowed by the failure" + ); +} + +#[test] +fn a_loadable_config_still_allows_an_ordinary_call() { + // THE MIRROR. Without it the case above is satisfied by an adjudicator that + // denies every call in the fleet, which is an outage wearing a fix's clothes. + let dir = fixture("adjudicate-loadable", LOADS); + assert_eq!( + code(&dir), + Some(0), + "a config that loads and refuses nothing must still allow" + ); +} + +#[test] +fn the_declared_hatch_still_reaches_a_clone_whose_config_will_not_load() { + // What keeps a container recoverable rather than bricked. A stale binary + // meeting a newer config refuses every call until one of them moves, so the + // operator's declared escape has to survive exactly the state that needs it. + // + // `common::batten()` scrubs every bypass variable by construction, so setting + // one here is the only way it is present — a case that inherited it from the + // developer's shell would pass without testing anything. + use std::io::Write as _; + use std::process::Stdio; + + let dir = fixture("adjudicate-hatch", WILL_NOT_PARSE); + let mut child = common::batten() + .args(["adjudicate", "--harness", "exit-code"]) + .current_dir(&dir) + .env("BATTEN_HOOK_BYPASS", "1") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("the binary runs"); + child + .stdin + .as_mut() + .expect("stdin is piped") + .write_all(payload().as_bytes()) + .expect("the payload is writable"); + let output = child.wait_with_output().expect("the binary answers"); + assert_eq!( + output.status.code(), + Some(0), + "the declared hatch must still pass a call the engine cannot judge" + ); +} + +#[test] +fn on_claude_code_the_refusal_is_the_document_rather_than_the_number() { + // THE CHANNEL IS PER-HARNESS AND THE NUMBER IS NOT (`run_hook`'s own + // contract). This host reads the JSON decision object and ignores the code, + // so a deny raised as `2` here would be a refusal nobody receives — which is + // the reason this arm renders rather than raising a `Denial`. + let dir = fixture("adjudicate-document", WILL_NOT_PARSE); + let output = run_with_stdin( + dir.as_path(), + &["adjudicate", "--harness", "claude-code"], + &payload(), + ); + let rendered = String::from_utf8_lossy(&output.stdout); + assert!( + rendered.contains("deny"), + "the decision object must carry the deny: {rendered}" + ); +} diff --git a/crates/batten/tests/it/main.rs b/crates/batten/tests/it/main.rs index 70c2072bb..07e8d7501 100644 --- a/crates/batten/tests/it/main.rs +++ b/crates/batten/tests/it/main.rs @@ -42,6 +42,7 @@ mod acquisition_metric; mod acquisition_sweep; mod address_resolve; mod address_transport; +mod adjudicate_absent; mod admission; mod admission_narrowing; mod advisory_drain; From 1da23d400347606673caaa6f8dbb7845abf2070b Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 9 Sep 2026 20:20:57 +0000 Subject: [PATCH 03/18] fix(hook): extract the refusal, and move four asserted codes off the fail-open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `run_hook` went to 126 lines against a 100 budget, so the deny arm becomes `deny_unadjudicable` rather than gaining an `#[allow]` — a boundary this load-bearing reads better on its own than as a match arm nine levels in. The four `call_arguments` cases are the substantive half, and the change is deliberate rather than green-making. Each asserted that a malformed config on the ADJUDICATE path answers `1`: a bound of zero is a usage error, not a very strict policy a named key with no projection is a usage error a row that can never fire is a usage error, not a silently inert gate a projection on a branch-keyed row is a usage error, not an ignored column Every one of those classifications is still true and none is edited. What changed is that `1` is the code a harness reads as a non-blocking hook error, so on the mediated boundary each of these let the call through unjudged — 1,149 calls did exactly that over one measured session. The surfaces stay separate rather than one principle beating the other: `doctor` still never answers `2` (`a_failing_diagnosis_is_never_a_policy_verdict`), and the CLI verbs still raise a usage error over a config they cannot read. `adjudicate` is the one surface where "cannot judge" must not resolve to "proceed", because there the alternative is a tool call nobody looked at. The diagnostics ride through unchanged, which the neighbouring assertion that stderr still names `max_age = 0` is what proves. Refs: CLOUD-1688 --- crates/batten/src/lib.rs | 83 ++++++++++++++++-------- crates/batten/tests/it/call_arguments.rs | 35 ++++++++-- 2 files changed, 86 insertions(+), 32 deletions(-) diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 6811c6ab0..8a33a2ba1 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -12320,32 +12320,7 @@ fn run_hook( Ok(loaded) => loaded, Err(_) if bypass => (hook::Policy::declaring_nothing(harness), Vec::new()), Err(unreadable) => { - // Pointer-only (non-negotiable rule 4): the loader's own message - // names the key or path that would not load, never its contents. - let refusal = Refusal::new( - "engine-cannot-adjudicate", - format!( - "this build could not load the rules it is registered to enforce, so \ - nothing judged this call: {unreadable}" - ), - // No remedy the ENGINE may declare: the repair is rebuilding - // or reinstalling the binary, or fixing the config, and both - // are the consumer's own commands (non-negotiable rule 1). - Fix::None, - ); - let rendering = Rendering { - hatch: hook::BYPASS_ENV, - ceiling: None, - }; - return render( - harness, - &envelope, - hook::Decision::Deny(refusal), - &rendering, - mode, - out, - err, - ); + return deny_unadjudicable(harness, &envelope, &unreadable, mode, out, err); } } } else { @@ -12589,6 +12564,62 @@ fn run_hook( render(harness, &envelope, decision, &rendering, mode, out, err) } +/// Refuse a call whose rules this build could not load (CLOUD-1688). +/// +/// Lifted out of [`run_hook`] rather than left inline because that function is +/// already at its line budget, and a boundary this load-bearing should be +/// readable on its own rather than as a match arm nine levels in. +/// +/// **A DECISION, NOT AN ERROR, WHICH IS WHY IT RENDERS.** [`render`] owns the +/// per-harness deny channel: Claude Code answers in its JSON decision object at +/// exit `0`, where the document *is* the deny, and the neutral adapter answers +/// [`ExitCode::Violation`]. Raising a [`Denial`] here would send `2` to the one +/// host that reads the document instead of the number. +/// +/// **The rendering carries no ceiling and the general hatch**, because both live +/// on the policy that would not load. `None` reads downstream as "no declared +/// bound" rather than as a bound of zero, which is the direction that keeps a +/// refusal about an unreadable config from being truncated by a value nobody +/// could read. +fn deny_unadjudicable( + harness: hook::Harness, + envelope: &hook::Envelope, + unreadable: &dyn std::fmt::Display, + mode: Mode, + out: &mut dyn Write, + err: &mut dyn Write, +) -> Result { + // Pointer-only (non-negotiable rule 4): the loader's own message names the + // key or the path that would not load, never the file's contents. The + // existing `max_age = 0` and unknown-key diagnostics ride through here + // unchanged, which is what keeps the operator's repair as findable as it was + // when this arm exited `1`. + let refusal = Refusal::new( + "engine-cannot-adjudicate", + format!( + "this build could not load the rules it is registered to enforce, so nothing judged \ + this call: {unreadable}" + ), + // No remedy the ENGINE may declare: the repair is rebuilding or + // reinstalling the binary, or fixing the config, and each is the + // consumer's own command (non-negotiable rule 1). + Fix::None, + ); + let rendering = Rendering { + hatch: hook::BYPASS_ENV, + ceiling: None, + }; + render( + harness, + envelope, + hook::Decision::Deny(refusal), + &rendering, + mode, + out, + err, + ) +} + /// Run the refusing row's declared repair, and say what the boundary decides now. /// /// **Every other decision passes straight through**, and most `Deny`s do too: diff --git a/crates/batten/tests/it/call_arguments.rs b/crates/batten/tests/it/call_arguments.rs index 693fbae26..6a1076ed7 100644 --- a/crates/batten/tests/it/call_arguments.rs +++ b/crates/batten/tests/it/call_arguments.rs @@ -257,7 +257,10 @@ reason = "unreachable" .build(); assert_eq!( verdict(&contradictory, "mcp__Linear__save_issue", r"{}"), - Some(1), + // `2` rather than `1` for CLOUD-1688's reason, stated in full at the + // `max_age = 0` case: the classification below is unchanged, but on the + // mediated boundary `1` is non-blocking and let the call through. + Some(2), "a row that can never fire is a usage error, not a silently inert gate" ); @@ -429,8 +432,11 @@ reason = "unreachable" "mcp__Linear__save_issue", r#"{"id":"CLOUD-1"}"# ), - Some(1), - "a named key with no projection is a usage error" + // `2` rather than `1` for CLOUD-1688's reason, stated in full at the + // `max_age = 0` case below: the classification is unchanged, but on the + // mediated boundary `1` is non-blocking and let the call through. + Some(2), + "a call whose rules would not load must be refused, not allowed by the failure" ); let wrong_key = Fixture::new("args-from-wrong-key") @@ -454,7 +460,10 @@ reason = "unreachable" .build(); assert_eq!( verdict(&wrong_key, "mcp__Linear__save_issue", r#"{"id":"CLOUD-1"}"#), - Some(1), + // `2` rather than `1` for CLOUD-1688's reason, stated in full at the + // `max_age = 0` case: the classification below is unchanged, but on the + // mediated boundary `1` is non-blocking and let the call through. + Some(2), "a projection on a branch-keyed row is a usage error, not an ignored column" ); } @@ -628,10 +637,24 @@ reason = "unreachable" .git() .base_commit() .build(); + // WAS `1`, AND THE CHANGE IS THE POINT (CLOUD-1688). The classification this + // asserted is still true — a bound of zero is a misconfiguration, never a + // very strict policy — but `1` is the code the harness reads as a + // NON-BLOCKING hook error, so on the mediated path it let the call through + // unjudged. Measured over one 5-day session, 1,149 calls proceeded exactly + // this way. + // + // The surfaces stay separate rather than one winning: `doctor` still never + // answers `2` (`a_failing_diagnosis_is_never_a_policy_verdict`), and the CLI + // verbs still raise a usage error over a config they cannot read. This is the + // one surface where "cannot judge" must not resolve to "proceed", because + // here the alternative is a tool call nobody looked at. + // + // The diagnostic is unchanged, which the next assertion is what proves. assert_eq!( verdict(&zero, "mcp__Linear__save_issue", r#"{"id":"CLOUD-1"}"#), - Some(1), - "a bound of zero is a usage error, not a very strict policy" + Some(2), + "a call whose rules would not load must be refused, not allowed by the failure" ); let refusal = run_with_stdin( &zero, From 2e6dafefb35b938e598e8d8658eb7799cf200bd7 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 9 Sep 2026 20:37:31 +0000 Subject: [PATCH 04/18] refactor(hook): give the adjudicable predicate a name, and the fifth code its reason MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `run_hook` sat at exactly its 100-line budget, so the deny arm put it over. Extracting `is_adjudicable` buys the room, and the predicate reads better named than as a five-clause disjunction mid-function: every clause was added by a separate measured defect — a dead `Stop` gate whose own suite stayed green (CLOUD-1051), a `SessionStart` mint that could not see its manifests (CLOUD-856) — and the doc keeps that history where the next reader meets it. `call_ceiling`'s partial-ceiling case is the fifth of the same class as the four in `call_arguments`: a config fault on the mediated path asserted as `1`. Its comment cited `rules/rust.md`'s rule that no Batten failure may read as a deny, and that rule still holds where it was written — `doctor` and the CLI verbs. The mediated boundary is the exception, because there `1` is non-blocking and the call it could not judge simply ran. The `measures` diagnostic it pins is unchanged. Refs: CLOUD-1688 --- crates/batten/src/lib.rs | 109 +++++++++++++++---------- crates/batten/tests/it/call_ceiling.rs | 12 ++- 2 files changed, 73 insertions(+), 48 deletions(-) diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 8a33a2ba1..69f73e0b3 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -12251,18 +12251,7 @@ fn run_hook( // end-of-turn surface. The retired shell hook this replaces paid ~330-440ms // at the same boundary; `perf`'s `passthrough` and `noop` arms are pre-tool // shapes and are untouched by this clause. - let adjudicable = !envelope.command.is_empty() - || envelope.writes.is_some() - || envelope.event == hook::Event::Stop - // A FOURTH TIME, and for a mint rather than a verdict (CLOUD-856). Session - // start carries no command, no write and no tool name, so this predicate - // was false there and config was never loaded — which means the receipt - // this event exists to mint could not know which manifests were declared. - // The cost is one config load per SESSION, not per call, which is the - // same trade the `Stop` clause above makes, and it buys the whole reason - // `Fact::Document` can stay `None` on the mediated path. - || envelope.event == hook::Event::SessionStart - || (envelope.event == hook::Event::PreTool && !envelope.raw_tool.is_empty()); + let adjudicable = is_adjudicable(&envelope); // A BYPASSED CALL NOW PAYS THE CONFIG READ, and that invariant is retired // deliberately rather than eroded. // @@ -12281,38 +12270,9 @@ fn run_hook( // arms, ~0.7 ms against a 100 ms budget. `!adjudicable` keeps its old // behaviour, because an event with nothing to adjudicate has no protected // gate to run either, and that is the arm the hot path actually rides. - // A CONFIG THIS BUILD CANNOT READ IS CERTAINTY, AND CERTAINTY DENIES - // (CLOUD-1688). `?` here propagated a `UsageError` — exit `1` — and - // `exit.rs` makes only `2` a denial precisely so no FAILURE path can block a - // call. So the harness read this whole class as a non-blocking hook error - // and ran the tool anyway. Measured over one 5-day session: 1,149 calls - // proceeded unjudged through seven windows of a mid-edit `batten.toml`, and - // ~456 more through a preset this build did not ship (`policy.rs`'s - // unknown-preset arm, which raises exactly this error). - // - // THE DISCRIMINATION IS THE SAME ONE `UNREADABLE_STDIN` SITS ON THE OTHER - // SIDE OF, and CLOUD-1572 drew it one level down. Where the engine is - // GUESSING about the call — stdin it could not read, a payload that would - // not decode, an event the host does not declare — allowing is right, - // because nothing is known and refusing would make Batten the reason a - // session cannot proceed. Here the engine has READ its own authority and - // been told it cannot enforce it: the rule set is named, and unavailable. - // Proceeding then is not caution — it is a gate reporting a clean allow over - // rules it never ran, which is the false green this engine exists to catch. - // - // A DECISION, NOT AN ERROR, which is why it RENDERS rather than propagates. - // `render` owns the per-harness deny channel, so Claude Code gets its JSON - // decision object at exit `0` — where the document is the deny — and the - // neutral adapter gets `Violation`. Raising a `Denial` here would send `2` - // to a host that reads the document instead, which is the one number that - // host does not consult. - // - // THE HATCH IS HONOURED FIRST, and that is what keeps a container - // recoverable rather than bricked. A stale binary meeting a newer config - // denies every call until one of them moves, so the operator's declared - // escape has to still work — the bootstrap window CLOUD-1688 flags as - // needing a decision is exactly this state, and this arm is the part of it - // that can be settled without one. + // A config this build cannot read is CERTAINTY, and certainty denies rather + // than exiting non-zero — `deny_unadjudicable` carries the whole argument, + // including why the hatch is read first. //MUTANT-SUITE crates/batten/tests/it/adjudicate_absent.rs //MUTANT unloadable-config-allows|s@ Err(_) if bypass => (hook::Policy::declaring_nothing(harness), Vec::new()),@ Err(_) => (hook::Policy::declaring_nothing(harness), Vec::new()),@|a_config_this_build_cannot_load_denies_rather_than_failing_open let (policy, waivers) = if adjudicable { @@ -12564,12 +12524,73 @@ fn run_hook( render(harness, &envelope, decision, &rendering, mode, out, err) } +/// Whether this envelope has anything for the config to decide about. +/// +/// **The gate on whether a call pays a config read at all**, which is why the +/// hot path stays cheap: `perf`'s `passthrough` arm — a `Read` with a +/// `file_path`, no command, no write — takes the `false` branch, and its +/// below-`noop` reading comes from doing so. +/// +/// Every clause was added by a measurement rather than by symmetry, and the +/// history is the argument for keeping them enumerated here: +/// +/// * a command or a write is the original shape; +/// * `Stop` carries neither, so a `mediated_call` module registered for the end +/// of turn could not run at all — a dead gate whose own suite stayed green, +/// because a `with input as` case fabricates the shape the boundary never +/// built (CLOUD-1051); +/// * `SessionStart` likewise, and for a MINT rather than a verdict (CLOUD-856): +/// the receipt that event exists to write could not know which manifests were +/// declared. One config load per session, not per call; +/// * a `PreTool` call naming a tool is the shape a tool-keyed row exists to +/// judge, and without it such a row was loaded for no call that could match. +fn is_adjudicable(envelope: &hook::Envelope) -> bool { + !envelope.command.is_empty() + || envelope.writes.is_some() + || envelope.event == hook::Event::Stop + || envelope.event == hook::Event::SessionStart + || (envelope.event == hook::Event::PreTool && !envelope.raw_tool.is_empty()) +} + /// Refuse a call whose rules this build could not load (CLOUD-1688). /// /// Lifted out of [`run_hook`] rather than left inline because that function is /// already at its line budget, and a boundary this load-bearing should be /// readable on its own rather than as a match arm nine levels in. /// +/// # Certainty denies; guessing allows +/// +/// The load used to propagate with `?`, raising a [`UsageError`] — exit `1` — +/// and [`crate::exit`] makes only `2` a denial precisely so no FAILURE path can +/// block a call. So a harness read this whole class as a non-blocking hook error +/// and ran the mediated tool anyway. Measured over one 5-day session: 1,149 +/// calls proceeded unjudged through seven windows of a mid-edit `batten.toml`, +/// and ~456 more through a preset the installed build did not ship — the +/// unknown-preset arm in [`crate::policy`] raises exactly this error. +/// +/// This is the discrimination `UNREADABLE_STDIN` sits on the other side of, and +/// the one CLOUD-1572 drew one level down. Where the engine is GUESSING about +/// the call — stdin it could not read, a payload that would not decode, an event +/// the host does not declare — allowing is right, because nothing is known and +/// refusing would make Batten the reason a session cannot proceed. Here the +/// engine has READ its own authority and been told it cannot enforce it: the +/// rule set is named, and unavailable. Proceeding is not caution then, it is a +/// gate reporting a clean allow over rules it never ran. +/// +/// # The surfaces stay separate +/// +/// `doctor` still never answers `2` — a diagnosis is not a policy verdict — and +/// the CLI verbs still raise a usage error over a config they cannot read. The +/// mediated boundary is the one place where "cannot judge" must not resolve to +/// "proceed", because here the alternative is a tool call nobody looked at. +/// +/// # The hatch is read before this is reached +/// +/// [`run_hook`] takes the bypass arm first, and that is what keeps a container +/// recoverable rather than bricked: a stale binary meeting a newer config +/// refuses every call until one of them moves, so the operator's declared escape +/// has to survive exactly the state that needs it. +/// /// **A DECISION, NOT AN ERROR, WHICH IS WHY IT RENDERS.** [`render`] owns the /// per-harness deny channel: Claude Code answers in its JSON decision object at /// exit `0`, where the document *is* the deny, and the neutral adapter answers diff --git a/crates/batten/tests/it/call_ceiling.rs b/crates/batten/tests/it/call_ceiling.rs index 2e53fb73f..2042b3a5f 100644 --- a/crates/batten/tests/it/call_ceiling.rs +++ b/crates/batten/tests/it/call_ceiling.rs @@ -169,12 +169,16 @@ reason = "..." &["adjudicate", "--harness", "exit-code"], &payload(&prompt_of(10)), ); - // Exit 1 is the usage code: a config fault, never a policy verdict, so no - // Batten failure can read as a deny (`rules/rust.md`). + // A CONFIG FAULT IS STILL NOT A POLICY VERDICT — and on THIS surface it is + // still a refusal (CLOUD-1688). `rules/rust.md`'s rule that no Batten failure + // may read as a deny is what keeps `doctor` and the CLI verbs on `1`; the + // mediated boundary is the exception, because there `1` is non-blocking and + // the call it could not judge simply ran. The classification below is + // unchanged and so is the `measures` diagnostic the next assertion pins. assert_eq!( output.status.code(), - Some(1), - "a partial ceiling is a config fault: {}", + Some(2), + "a partial ceiling is a config fault, and one this call cannot be judged under: {}", stderr(&output) ); assert!( From 75679faee2cdc9ac80d543d5870a062dc30b24b3 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 9 Sep 2026 22:56:15 +0000 Subject: [PATCH 05/18] fix(hook): refuse only a declaration nothing could read, and say why MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first pass refused on every `load_policy` failure. That conflates three faults the tree already separates, and only one of them is a refusal. Gates are registered fail-open, so a gate that fails open is INERT — it neither allows nor denies, it is absent. A config fault is therefore never a choice between refusing and allowing: it is a choice between keeping the enforcement surface we still have and losing it entirely. An unknown key costs its own row, a table whose validator refuses names that table, a version this build is too old for still says so — each leaves every other row readable and enforceable, and leaves an agent that can be told to repair the broken one. Refusing there trades a working partial gate for nothing. A file that is not TOML has no partial function to preserve: zero rows are readable, nothing is enforced, and the refusal is the only signal left. That asymmetry is the whole scope of the change. `Native::ConfigUnreadable` carries it. The class is a DISCRIMINATOR rather than a label, and it is identified positively: keying on "carries no declared class" would also have caught the unsupported-version and `min_batten_version` refusals, which leave the file readable, and would widen what denies with every future unclassed error. The syntax probe runs on the error path only. `toml::de::Error` is one type for two unlike faults and renders both as "TOML parse error at line N" — measured on the `[[fact]]`-with-no-`returns` fixture, a schema fault the message alone classed as unreadable. A `Table` parse answers it exactly, and costs nothing until a parse has already failed, which is the probe `parse_ungated` records as removed for costing one on the hot path. Under `unloadable-config-allows` the two deny cases redden and the mirror, the hatch and the rule-4 case stay green. 798 tests pass across the six suites this touches; nine assertions from the first pass are reverted to their originals. Refs: CLOUD-1677 --- crates/batten/src/config.rs | 38 ++++- crates/batten/src/lib.rs | 131 +++++++++++++++--- crates/batten/src/lint.rs | 2 +- crates/batten/src/verdict.rs | 44 ++++++ crates/batten/tests/it/adjudicate_absent.rs | 44 +++++- crates/batten/tests/it/call_arguments.rs | 35 +---- crates/batten/tests/it/call_ceiling.rs | 12 +- crates/batten/tests/it/cli.rs | 56 ++++++-- .../tests/it/config_forward_compatible.rs | 27 +++- 9 files changed, 311 insertions(+), 78 deletions(-) diff --git a/crates/batten/src/config.rs b/crates/batten/src/config.rs index 73084dd12..16aa5bb8a 100644 --- a/crates/batten/src/config.rs +++ b/crates/batten/src/config.rs @@ -1386,7 +1386,8 @@ pub const RETIRED_KEYS: &[(&str, &str)] = &[( /// /// [`trust::load_base`]: crate::trust::load_base pub fn parse_base(text: &str, source: &str) -> Result { - let mut table: toml::Table = toml::from_str(text).map_err(|err| config_error(source, &err))?; + let mut table: toml::Table = + toml::from_str(text).map_err(|err| config_error(source, text, &err))?; // Nothing is reported when a key is dropped: the report this feeds is a // comparison of two policies, and "the base declared a key this build no // longer has" is a fact about the build rather than about either policy. @@ -1518,7 +1519,8 @@ pub fn parse_override(text: &str, source: &str) -> Result { prune_unresolvable::(text, binary_is_behind_the_config(source, text)); let config = match pruned.config { Some(config) => config, - None => toml::from_str(&pruned.text).map_err(|err| config_error(source, &err))?, + None => toml::from_str(&pruned.text) + .map_err(|err| config_error(source, &pruned.text, &err))?, }; (config, pruned.dropped) }; @@ -1967,8 +1969,35 @@ fn names_an_unknown_key(rendered: &str) -> bool { //MUTANT-SUITE crates/batten/tests/it/config_skew.rs //MUTANT skew-reads-as-malformed|s@ if !names_an_unknown_key(&rendered) {@ if true {@|an_unknown_key_names_the_rebuild //MUTANT every-parse-error-blames-skew|s@ if !names_an_unknown_key(&rendered) {@ if false {@|a_malformed_config_does_not_mention_a_rebuild -pub(crate) fn config_error(source: &str, err: &toml::de::Error) -> anyhow::Error { +pub(crate) fn config_error(source: &str, text: &str, err: &toml::de::Error) -> anyhow::Error { let rendered = err.to_string(); + // THE SYNTAX PROBE, AND IT RUNS ONLY HERE — ON THE ERROR PATH (CLOUD-1677). + // + // `toml::de::Error` is the one type for two very different faults, and the + // rendering hides it: a missing field and an invalid type both arrive as + // "TOML parse error at line N, column C", exactly like a stray brace. Reading + // the message cannot tell them apart, and `names_an_unknown_key` answers a + // third question again — measured on the `[[fact]]`-with-no-`returns` fixture, + // which is a SCHEMA fault that renders as a parse error and was classed as + // unreadable by the message alone. + // + // A `Table` parse answers it exactly: if the bytes are well-formed TOML then + // whatever failed was the SCHEMA over them, and the file still has rows a + // build can read. This is the probe the comment in `parse_ungated` records as + // removed for costing a parse on the hot path — it is free here, because + // nothing reaches this function until a parse has already failed. + if toml::from_str::(text).is_err() { + // CLASSED, AND THE CLASS IS A DISCRIMINATOR RATHER THAN A LABEL. This is + // the one config fault with no partial function left to preserve — the + // file is not TOML, so no row is readable and none can be enforced. Every + // other fault leaves the rest of the file deciding, which is what lets an + // agent be told to repair the broken part instead of losing the gate + // surface entirely. `Native::ConfigUnreadable` carries the full argument. + return UsageError::raise_as( + crate::verdict::Native::ConfigUnreadable, + format!("invalid config {source}: {err}"), + ); + } if !names_an_unknown_key(&rendered) { return UsageError::raise(format!("invalid config {source}: {err}")); } @@ -3042,7 +3071,8 @@ fn parse_ungated(text: &str, source: &str) -> Result { let pruned = prune_unresolvable::(text, binary_is_behind_the_config(source, text)); let config = match pruned.config { Some(config) => config, - None => toml::from_str(&pruned.text).map_err(|err| config_error(source, &err))?, + None => toml::from_str(&pruned.text) + .map_err(|err| config_error(source, &pruned.text, &err))?, }; (config, pruned.dropped) }; diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 69f73e0b3..e392904af 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -12140,8 +12140,7 @@ fn run_hook( // The note rides the ladder above `normal`, because on the hosts where this // is reachable it is the ordinary state rather than news. let capabilities = harness.capabilities(); - if !capabilities.emits(envelope.event) && envelope.event != hook::Event::Unrecognized { - let note = unsupported_event_note(harness, &capabilities, envelope.event); + if let Some(note) = undeclared_event_note(harness, &capabilities, envelope.event) { output::message(mode, Verbosity::Verbose, err, ¬e)?; return Ok(ExitCode::Success); } @@ -12270,18 +12269,38 @@ fn run_hook( // arms, ~0.7 ms against a 100 ms budget. `!adjudicable` keeps its old // behaviour, because an event with nothing to adjudicate has no protected // gate to run either, and that is the arm the hot path actually rides. - // A config this build cannot read is CERTAINTY, and certainty denies rather - // than exiting non-zero — `deny_unadjudicable` carries the whole argument, - // including why the hatch is read first. + // A DECLARATION NOTHING COULD READ IS THE ONE FAULT THAT REFUSES, and the + // narrowness is the decision rather than caution (CLOUD-1677). + // + // Gates are registered fail-open, so a gate that fails open is INERT — it + // neither allows nor denies, it is absent. A config fault is therefore never + // a choice between refusing and allowing: it is a choice between keeping the + // enforcement surface we still have and losing it entirely. An unknown key, a + // version this build is too old for, a table whose validator refused — each + // leaves every other row readable and enforceable, and leaves an agent that + // can still be TOLD to repair the broken one. Refusing there would trade a + // working partial surface for nothing. + // + // `Native::ConfigUnreadable` is the one class with no partial function left: + // the file is not TOML, so no row is readable and none can be enforced. Then + // the refusal is the only signal available, and the hatch below is how the + // container gets back. //MUTANT-SUITE crates/batten/tests/it/adjudicate_absent.rs - //MUTANT unloadable-config-allows|s@ Err(_) if bypass => (hook::Policy::declaring_nothing(harness), Vec::new()),@ Err(_) => (hook::Policy::declaring_nothing(harness), Vec::new()),@|a_config_this_build_cannot_load_denies_rather_than_failing_open + //MUTANT unloadable-config-allows|s@ Err(unreadable) if unreadable_declaration(\&unreadable) => {@ Err(unreadable) if false \&\& unreadable_declaration(\&unreadable) => {@|a_config_this_build_cannot_load_denies_rather_than_failing_open let (policy, waivers) = if adjudicable { match load_policy(overrides, harness) { Ok(loaded) => loaded, + // The declared hatch, read before the refusal so a stale binary + // meeting a newer config leaves a container recoverable, not bricked. Err(_) if bypass => (hook::Policy::declaring_nothing(harness), Vec::new()), - Err(unreadable) => { + Err(unreadable) if unreadable_declaration(&unreadable) => { return deny_unadjudicable(harness, &envelope, &unreadable, mode, out, err); } + // EVERY OTHER FAULT KEEPS ITS OLD BEHAVIOUR, deliberately. Today that + // is still a whole-file refusal at exit `1`, which is the outcome the + // argument above says is wrong — making these actually preserve the + // rows they can read is its own change, over `prune_unresolvable`. + Err(other) => return Err(other), } } else { (hook::Policy::declaring_nothing(harness), Vec::new()) @@ -12524,6 +12543,25 @@ fn run_hook( render(harness, &envelope, decision, &rendering, mode, out, err) } +/// The note for an event this host does not declare, or `None` to carry on. +/// +/// **`Unrecognized` is not undeclared**, and collapsing the two is why this is a +/// named predicate rather than an inline `&&`: an event nobody could parse has +/// no capability row to be absent from, so it falls through to the ordinary +/// path instead of being reported as a host that offers less. +/// +/// Returning the note rather than a `bool` keeps [`unsupported_event_note`]'s +/// call beside the condition that earns it — a caller that tested one and +/// rendered the other could report an event the table actually declares. +fn undeclared_event_note( + harness: hook::Harness, + capabilities: &hook::Capabilities, + event: hook::Event, +) -> Option { + (!capabilities.emits(event) && event != hook::Event::Unrecognized) + .then(|| unsupported_event_note(harness, capabilities, event)) +} + /// Whether this envelope has anything for the config to decide about. /// /// **The gate on whether a call pays a config read at all**, which is why the @@ -12552,7 +12590,25 @@ fn is_adjudicable(envelope: &hook::Envelope) -> bool { || (envelope.event == hook::Event::PreTool && !envelope.raw_tool.is_empty()) } -/// Refuse a call whose rules this build could not load (CLOUD-1688). +/// Whether this load failure is a declaration nothing could read at all. +/// +/// **Positively identified, never inferred from an absence.** The tempting +/// spelling is "carries no declared class", and it is wrong: the +/// unsupported-version and `min_batten_version` refusals carry none either, and +/// both leave every row in the file readable. Keying on absence would refuse +/// those too — and every future unclassed refusal after them, silently widening +/// what denies. +/// +/// So the loader says which one this is. `config_error` already separates a +/// syntax failure from an unknown key, and since CLOUD-1677 its syntax arm raises +/// under [`verdict::Native::ConfigUnreadable`]. +fn unreadable_declaration(err: &anyhow::Error) -> bool { + err.downcast_ref::() + .and_then(|usage| usage.verdict) + .is_some_and(|class| class == verdict::Native::ConfigUnreadable) +} + +/// Refuse a call whose rules this build could not load (CLOUD-1677). /// /// Lifted out of [`run_hook`] rather than left inline because that function is /// already at its line budget, and a boundary this load-bearing should be @@ -12610,16 +12666,32 @@ fn deny_unadjudicable( out: &mut dyn Write, err: &mut dyn Write, ) -> Result { - // Pointer-only (non-negotiable rule 4): the loader's own message names the - // key or the path that would not load, never the file's contents. The - // existing `max_age = 0` and unknown-key diagnostics ride through here - // unchanged, which is what keeps the operator's repair as findable as it was - // when this arm exited `1`. + // THE FIRST LINE ONLY, AND THAT IS NON-NEGOTIABLE RULE 4 RATHER THAN BREVITY. + // + // A `toml` parse error renders as a multi-line span — a header naming the + // position, then the OFFENDING SOURCE LINE with a caret under it. Interpolating + // the whole thing puts a byte of the unreadable config into a refusal that + // reaches the model, the host's log and the transcript, which is exactly the + // payload rule 4 keeps out of every report this engine writes. Measured by + // `the_declaration_that_would_not_parse_is_named_without_quoting_it`, which + // fails on the un-truncated form. + // + // The first line is the POINTER and loses nothing an operator needs: for a + // parse failure it is `TOML parse error at line L, column C`, and for the + // skew and unknown-key arms the whole message is one line already — the + // `max_age = 0` and `command_matcher` diagnostics ride through intact, which + // is what keeps the repair as findable as it was when this arm exited `1`. + let pointer = unreadable + .to_string() + .lines() + .next() + .unwrap_or_default() + .to_owned(); let refusal = Refusal::new( "engine-cannot-adjudicate", format!( "this build could not load the rules it is registered to enforce, so nothing judged \ - this call: {unreadable}" + this call: {pointer}" ), // No remedy the ENGINE may declare: the repair is rebuilding or // reinstalling the binary, or fixing the config, and each is the @@ -12630,7 +12702,16 @@ fn deny_unadjudicable( hatch: hook::BYPASS_ENV, ceiling: None, }; - render( + // WHICH CHANNEL CARRIED THE REFUSAL IS `render`'S OWN ANSWER, and reading it + // here is what lets the number say could-not-look without ever spending the + // refusal to do it (CLOUD-1677's exit-code half). + // + // `render` denies through the host's protocol: a document harness gets its + // decision object and answers `Ok`, while the neutral adapter's ONLY deny + // channel is the number, so it answers `Err(Denial)` — `run_hook`'s own + // contract says as much. The two arms below are therefore not a preference + // between codes, they are the two protocols. + match render( harness, envelope, hook::Decision::Deny(refusal), @@ -12638,7 +12719,25 @@ fn deny_unadjudicable( mode, out, err, - ) + ) { + // The DOCUMENT already refuses, so the number is free to be honest: §6–§7 + // reserve `3` for could-not-look, and nothing was judged about this call. + // `Internal` rather than `Violation` also keeps `exit.rs`'s guarantee + // whole — `Usage` and `Internal` are the only codes a failure of Batten's + // own may produce, *so that fail-open is structural* — and an unreadable + // declaration is such a failure. Answering `2` here would have bought the + // refusal twice and spent that guarantee for the second copy. + Ok(_) => Ok(ExitCode::Internal), + // THE NUMBER IS THIS HARNESS'S ONLY CHANNEL, so it stays the deny. Turning + // it into `3` would be honest about the cause and silent about the verdict, + // which is the fail-open this row exists to close. + // + // **The cost is stated rather than absorbed** (CLOUD-1677's Replay asks for + // the per-harness table): on this adapter the boundary cannot say BOTH + // "refused" and "nothing could be read", so the could-not-look half is + // unavailable there until that protocol grows a way to carry it. + Err(denial) => Err(denial), + } } /// Run the refusing row's declared repair, and say what the boundary decides now. diff --git a/crates/batten/src/lint.rs b/crates/batten/src/lint.rs index 011cf024f..9bddf29d2 100644 --- a/crates/batten/src/lint.rs +++ b/crates/batten/src/lint.rs @@ -401,7 +401,7 @@ pub fn smells( // same message it would anywhere else rather than this module's own. let config = config::parse(text, source)?; let located: Located = - toml::from_str(text).map_err(|err| config::config_error(source, &err))?; + toml::from_str(text).map_err(|err| config::config_error(source, text, &err))?; let mut found = Vec::new(); diff --git a/crates/batten/src/verdict.rs b/crates/batten/src/verdict.rs index ff125021b..4aa095466 100644 --- a/crates/batten/src/verdict.rs +++ b/crates/batten/src/verdict.rs @@ -1213,6 +1213,33 @@ pub enum Native { // moment it fires. That also makes them resolvable from `vendored()` with // no config load, which is what keeps `policy explain` usable over a config // that will not parse. + /// The declaration could not be READ AT ALL — not valid TOML. + /// + /// **Deliberately not in [`Native::CONFIG_FAULTS`]**, which is the per-TABLE + /// set and is censused in both directions against `config.rs`'s own list. This + /// one has no table: it is raised before any table exists, by the parse that + /// every table's validator runs after. + /// + /// # Why it needs a class when the other parse failures do not + /// + /// A class here is not for `explain` — it is the DISCRIMINATOR the mediated + /// boundary acts on (CLOUD-1677). Gates are registered fail-open, and a gate + /// that fails open is inert: it neither allows nor denies, it is absent. So a + /// config fault is never a choice between refusing and allowing, it is a + /// choice between keeping the enforcement surface we still have and losing it + /// entirely. + /// + /// An unknown key, a version this build is too old for, a row whose validator + /// refused — each leaves every OTHER row readable and enforceable, and leaves + /// an agent that can still be told to repair the one that is broken. Failing + /// the whole load there buys nothing and costs the surface that would have + /// carried the repair instruction. + /// + /// A file that is not TOML is the one case with no partial function to + /// preserve: zero rows are readable, so refusing the call is the only signal + /// left, and the declared hatch is the recovery path. That asymmetry is why + /// this class exists and why it is exactly one class wide. + ConfigUnreadable, /// The `[[verb]]` table would not load. VerbTableRefused, /// The `[[pattern]]` table would not load. @@ -1291,6 +1318,7 @@ impl Native { Native::CallFixSilent, Native::ContentRefused, Native::KeyMissing, + Native::ConfigUnreadable, Native::VerbTableRefused, Native::PatternTableRefused, Native::VerdictTableRefused, @@ -1373,6 +1401,7 @@ impl Native { Native::CallFixSilent => "call fix silent", Native::ContentRefused => "input write refused", Native::KeyMissing => "issue name missing", + Native::ConfigUnreadable => "config read refused", Native::VerbTableRefused => "verb declare refused", Native::PatternTableRefused => "pattern declare refused", Native::VerdictTableRefused => "verdict declare refused", @@ -1891,6 +1920,20 @@ it serves.", // Every route is the config itself, which is not a placeholder: a config // fault is edited in exactly one file, and a `command` route would have to // name a task that can run over a config that does not load. + VendoredVerdict { + id: "config read refused", + gloss: "the declaration is not TOML, so no rule in it could be read", + class: "Every other config fault leaves the rest of the file deciding -- an unknown key \ +costs its own row, a table whose validator refuses names that table, and a version this build \ +is too old for still says so. Each of those keeps a working gate surface and an agent that can \ +be told to repair the broken part. This one has no partial function to preserve: the bytes are \ +not TOML, so zero rows are readable and nothing is enforced. That is why it is the one class \ +the mediated boundary refuses a call under, rather than reporting and proceeding -- a gate that \ +fails open is inert, and an inert gate over an unreadable authority is the false green this \ +engine exists to catch.", + routes: &[read("config read first", "batten.toml")], + applicability: Applicability::Advice, + }, VendoredVerdict { id: "verb declare refused", gloss: "the verb table would not load", @@ -2367,6 +2410,7 @@ mod tests { | Native::VerdictTrailing | Native::RunOrphaned | Native::CeilingExceeded + | Native::ConfigUnreadable | Native::ShapeRefused | Native::CallRetryNow | Native::CallFixSilent diff --git a/crates/batten/tests/it/adjudicate_absent.rs b/crates/batten/tests/it/adjudicate_absent.rs index 232ebca04..300938ce4 100644 --- a/crates/batten/tests/it/adjudicate_absent.rs +++ b/crates/batten/tests/it/adjudicate_absent.rs @@ -70,9 +70,14 @@ fn code(dir: &Path) -> Option { #[test] fn a_config_this_build_cannot_load_denies_rather_than_failing_open() { - // The defect: this was `2` only if the load succeeded. A `?` on the load - // raised a `UsageError` — exit `1` — and a harness reads `1` as a - // non-blocking hook error, so the mediated tool ran with nothing judging it. + // WAS `1`, WHICH A HARNESS READS AS A NON-BLOCKING HOOK ERROR, so the + // mediated tool ran with nothing judging it (CLOUD-1677). + // + // `2` HERE AND `3` ON A DOCUMENT HARNESS, and the split is the protocol + // rather than a preference: this adapter's ONLY deny channel is the number, + // so the number has to carry the refusal. Where the decision object carries + // it instead, the number is free to say could-not-look — the case below + // asserts that side. let dir = fixture("adjudicate-unloadable", WILL_NOT_PARSE); assert_eq!( code(&dir), @@ -142,8 +147,39 @@ fn on_claude_code_the_refusal_is_the_document_rather_than_the_number() { &payload(), ); let rendered = String::from_utf8_lossy(&output.stdout); + // The row's falsifier names the field rather than the word: a `contains("deny")` + // would pass on a document that merely mentioned it, including one that said + // the opposite. assert!( - rendered.contains("deny"), + rendered.contains(r#""permissionDecision":"deny""#), "the decision object must carry the deny: {rendered}" ); + // AND THE NUMBER IS FREE TO BE HONEST, which is the half that needs the + // document to exist. §6-§7 reserve `3` for could-not-look, and `exit.rs` + // keeps `Usage` and `Internal` the only codes a failure of Batten's own may + // produce *so that fail-open is structural*. Answering `2` here would buy the + // refusal a second time and spend that guarantee for the copy. + assert_eq!( + output.status.code(), + Some(3), + "where the document refuses, the number says nothing was judged" + ); +} + +#[test] +fn the_declaration_that_would_not_parse_is_named_without_quoting_it() { + // Non-negotiable rule 4, and the row asks for this clause by name: the reason + // carries the parse position, never the config's contents. The fixture's body + // is `this is not toml`, so its presence in the output would be the leak. + let dir = fixture("adjudicate-pointer-only", WILL_NOT_PARSE); + let output = run_with_stdin( + dir.as_path(), + &["adjudicate", "--harness", "claude-code"], + &payload(), + ); + let rendered = String::from_utf8_lossy(&output.stdout); + assert!( + !rendered.contains("this is not toml"), + "a refusal about an unreadable config must not quote it: {rendered}" + ); } diff --git a/crates/batten/tests/it/call_arguments.rs b/crates/batten/tests/it/call_arguments.rs index 6a1076ed7..693fbae26 100644 --- a/crates/batten/tests/it/call_arguments.rs +++ b/crates/batten/tests/it/call_arguments.rs @@ -257,10 +257,7 @@ reason = "unreachable" .build(); assert_eq!( verdict(&contradictory, "mcp__Linear__save_issue", r"{}"), - // `2` rather than `1` for CLOUD-1688's reason, stated in full at the - // `max_age = 0` case: the classification below is unchanged, but on the - // mediated boundary `1` is non-blocking and let the call through. - Some(2), + Some(1), "a row that can never fire is a usage error, not a silently inert gate" ); @@ -432,11 +429,8 @@ reason = "unreachable" "mcp__Linear__save_issue", r#"{"id":"CLOUD-1"}"# ), - // `2` rather than `1` for CLOUD-1688's reason, stated in full at the - // `max_age = 0` case below: the classification is unchanged, but on the - // mediated boundary `1` is non-blocking and let the call through. - Some(2), - "a call whose rules would not load must be refused, not allowed by the failure" + Some(1), + "a named key with no projection is a usage error" ); let wrong_key = Fixture::new("args-from-wrong-key") @@ -460,10 +454,7 @@ reason = "unreachable" .build(); assert_eq!( verdict(&wrong_key, "mcp__Linear__save_issue", r#"{"id":"CLOUD-1"}"#), - // `2` rather than `1` for CLOUD-1688's reason, stated in full at the - // `max_age = 0` case: the classification below is unchanged, but on the - // mediated boundary `1` is non-blocking and let the call through. - Some(2), + Some(1), "a projection on a branch-keyed row is a usage error, not an ignored column" ); } @@ -637,24 +628,10 @@ reason = "unreachable" .git() .base_commit() .build(); - // WAS `1`, AND THE CHANGE IS THE POINT (CLOUD-1688). The classification this - // asserted is still true — a bound of zero is a misconfiguration, never a - // very strict policy — but `1` is the code the harness reads as a - // NON-BLOCKING hook error, so on the mediated path it let the call through - // unjudged. Measured over one 5-day session, 1,149 calls proceeded exactly - // this way. - // - // The surfaces stay separate rather than one winning: `doctor` still never - // answers `2` (`a_failing_diagnosis_is_never_a_policy_verdict`), and the CLI - // verbs still raise a usage error over a config they cannot read. This is the - // one surface where "cannot judge" must not resolve to "proceed", because - // here the alternative is a tool call nobody looked at. - // - // The diagnostic is unchanged, which the next assertion is what proves. assert_eq!( verdict(&zero, "mcp__Linear__save_issue", r#"{"id":"CLOUD-1"}"#), - Some(2), - "a call whose rules would not load must be refused, not allowed by the failure" + Some(1), + "a bound of zero is a usage error, not a very strict policy" ); let refusal = run_with_stdin( &zero, diff --git a/crates/batten/tests/it/call_ceiling.rs b/crates/batten/tests/it/call_ceiling.rs index 2042b3a5f..2e53fb73f 100644 --- a/crates/batten/tests/it/call_ceiling.rs +++ b/crates/batten/tests/it/call_ceiling.rs @@ -169,16 +169,12 @@ reason = "..." &["adjudicate", "--harness", "exit-code"], &payload(&prompt_of(10)), ); - // A CONFIG FAULT IS STILL NOT A POLICY VERDICT — and on THIS surface it is - // still a refusal (CLOUD-1688). `rules/rust.md`'s rule that no Batten failure - // may read as a deny is what keeps `doctor` and the CLI verbs on `1`; the - // mediated boundary is the exception, because there `1` is non-blocking and - // the call it could not judge simply ran. The classification below is - // unchanged and so is the `measures` diagnostic the next assertion pins. + // Exit 1 is the usage code: a config fault, never a policy verdict, so no + // Batten failure can read as a deny (`rules/rust.md`). assert_eq!( output.status.code(), - Some(2), - "a partial ceiling is a config fault, and one this call cannot be judged under: {}", + Some(1), + "a partial ceiling is a config fault: {}", stderr(&output) ); assert!( diff --git a/crates/batten/tests/it/cli.rs b/crates/batten/tests/it/cli.rs index 1a1df93e3..ce96741e7 100644 --- a/crates/batten/tests/it/cli.rs +++ b/crates/batten/tests/it/cli.rs @@ -2680,24 +2680,56 @@ fn hook_allows_when_no_authority_is_configured() { } #[test] -fn hook_fails_open_and_loud_on_an_unloadable_authority() { - // The opposite case, and the one CLOUD-40 could not reach because `hook` - // loaded no config: an authority that EXISTS and cannot be read means the - // rules the operator wrote are not being applied. Allowing silently there - // would be the false green the engine exists to catch, so it is a usage - // error — loud on stderr, exit 1, and structurally not a deny, because §7 - // spends 2 on the verdict alone. +fn hook_refuses_and_is_loud_on_an_unloadable_authority() { + // RENAMED FROM `hook_fails_open_and_loud_…`, because the first half stopped + // being true (CLOUD-1677) — and ONLY for this fixture's fault. + // + // The reasoning it was written on is what the row acted on: an authority that + // EXISTS and cannot be read means the operator's rules are not being applied, + // and allowing silently is the false green the engine exists to catch. What + // was wrong is that "loud" was the whole remedy — exit `1` is a NON-BLOCKING + // hook error, so the tool ran anyway and the loudness reached a log nobody + // gates on. Measured: 1,149 calls proceeded through seven windows of a + // mid-edit `batten.toml`, which is itself a protected path, so the gate + // guarding the config stopped guarding it during the one operation that + // changes it. + // + // **THIS FIXTURE IS `not toml at all`, WHICH IS THE WHOLE SCOPE.** A config + // that parses and merely fails a validator, or names a key this build + // predates, still answers `1` — every other row in it remains readable and + // enforceable, and an agent can still be told to repair the broken one. + // Refusing there would trade a working partial gate surface for nothing. + // Here there is no partial surface: nothing parsed, so nothing is enforced. let dir = repo_with_config("hook-broken-authority", "this is not toml at all\n"); for harness in harnesses() { let output = run_hook_in(&dir, harness, &claude_payload("gh pr view 42"), false); let code = output.status.code(); - assert_eq!(code, Some(1), "{harness}: an unreadable authority is usage"); - assert_ne!(code, Some(2), "{harness}: must never deny"); - assert!(output.stdout.is_empty(), "{harness}: no decision document"); + let stdout = String::from_utf8_lossy(&output.stdout); + // WHAT THIS CASE PROVES IS THAT NO HARNESS FAILS OPEN, and it deliberately + // does not assert the channel. Six protocols write six different decision + // documents, so a `permissionDecision` assertion inside this loop tests + // Claude Code's spelling five times and passes it off as coverage. The + // per-protocol detail — the document on a host that reads one, the `2` on + // the neutral adapter whose only channel is the number — belongs where it + // can be stated exactly, which is `adjudicate_absent.rs`. + // + // `1` was the whole defect: non-blocking, so the call ran. `0` would be a + // clean allow over an authority nothing could read. + assert!( + matches!(code, Some(2 | 3)), + "{harness}: a call under an unreadable authority must not proceed, got {code:?}" + ); + // NEVER SILENT, AND NEVER ANONYMOUS — the half of the original case that + // was always right, asked of whichever channel actually carried the + // refusal. The original asked stderr of every harness, which was true + // while the answer was always an exit-`1` diagnostic; now a document + // harness puts the reason in the document and leaves stderr empty, so + // asserting stderr alone would fail on the hosts that refuse best. let stderr = String::from_utf8_lossy(&output.stderr); + let spoken = format!("{stderr}{stdout}"); assert!( - stderr.contains("batten.toml"), - "{harness}: the failure names the file, got: {stderr}" + spoken.contains("batten.toml"), + "{harness}: the failure names the file, got: {spoken}" ); } } diff --git a/crates/batten/tests/it/config_forward_compatible.rs b/crates/batten/tests/it/config_forward_compatible.rs index 69a9018e3..c1a623148 100644 --- a/crates/batten/tests/it/config_forward_compatible.rs +++ b/crates/batten/tests/it/config_forward_compatible.rs @@ -658,14 +658,33 @@ fn the_report_never_tells_the_reader_to_install_an_older_release() { /// fault from a well-formed row naming a key from a newer schema, and collapsing /// the two is what produced the defect — a prune that swallowed a syntax error /// would load "no rules configured" over a broken file. +/// +/// **AND SINCE CLOUD-1677 THE REFUSAL REACHES THE CALL.** The claim above is +/// unchanged and is exactly why: a newer-schema row leaves every other row +/// readable, so the file still decides and an agent can be told to repair the one +/// key. A file that is not TOML leaves nothing — zero rows readable, nothing +/// enforced — and a gate that fails open there is inert, which is the false green +/// the engine exists to catch. So this is the one config fault that denies. +/// +/// It was `1`: the usage code, non-blocking, and the `rm /` below simply ran. #[test] fn a_file_that_is_not_toml_is_still_refused() { let dir = repo("config-forward-broken", "[[rule\nbroken\n"); - let (code, _, stderr) = adjudicate(&dir); - assert_eq!(code, Some(1), "malformed TOML is a usage error: {stderr}"); + let (code, stdout, stderr) = adjudicate(&dir); + // THE DENY IS THE JSON, as the case above states for this harness. What + // differs here is the number beside it: with the document carrying the + // refusal, the code is free to say could-not-look — §6-§7's `3` — rather than + // claiming a verdict was reached. `exit.rs` keeps `Usage` and `Internal` the + // only codes a Batten failure produces, so refusing costs that guarantee + // nothing. + assert_eq!(code, Some(3), "nothing could be judged: {stdout} {stderr}"); + assert!( + stdout.contains(r#""permissionDecision":"deny""#), + "a call under an unreadable authority is refused: {stdout}" + ); assert!( - stderr.contains("TOML parse error"), - "the refusal says the file is not TOML: {stderr}" + format!("{stdout}{stderr}").contains("TOML parse error"), + "the refusal says the file is not TOML: {stdout} {stderr}" ); } From e54b87fea5626eeee7bc3956711daea5c41da63f Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 9 Sep 2026 23:26:39 +0000 Subject: [PATCH 06/18] refactor(hook): name the two fail-open boundaries, and let the raw payload travel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `run_hook` sits at its 100-line budget and the speculative tree pushed it to 102, so the stdin read and the decode become `read_envelope`. The grouping is the point rather than the line count: unreadable stdin and an undecodable payload are one answer — the engine does not know what this call IS, and a guard must never be the reason a session cannot proceed. That is the opposite side of `unreadable_declaration`, where the engine knows the call perfectly well and has been told it cannot enforce the rules over it. Naming them apart is what stops the next reader collapsing the two. The raw bytes travel with the decoded value because `dispatch_handlers` hands a declared handler the payload as it arrived: stdin is consumed, so re-reading is not available, and re-serializing would hand a handler a document the host never sent. Caught by the compiler on the first extraction, and written down so the tuple is not a mystery. 784 tests pass across the six suites this touches, `handler_dispatch` included. Refs: CLOUD-1677 --- crates/batten/src/lib.rs | 44 ++++++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index e392904af..eb12c5bfb 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -12113,16 +12113,10 @@ fn run_hook( out: &mut dyn Write, err: &mut dyn Write, ) -> Result { - let mut raw = String::new(); - if std::io::stdin().read_to_string(&mut raw).is_err() { - output::message(mode, Verbosity::Normal, err, UNREADABLE_STDIN)?; - return Ok(ExitCode::Success); - } - let bypass = std::env::var_os(hook::BYPASS_ENV).is_some_and(|value| !value.is_empty()); - let Some(mut envelope) = hook::decode(harness, &raw) else { - output::message(mode, Verbosity::Normal, err, UNDECODABLE_PAYLOAD)?; + let Some((raw, mut envelope)) = read_envelope(harness, mode, err)? else { return Ok(ExitCode::Success); }; + let bypass = std::env::var_os(hook::BYPASS_ENV).is_some_and(|value| !value.is_empty()); // THE WRITE TARGET IS READ AS THE REPOSITORY READS IT (CLOUD-1133), and this // is the one place that can do it: `decode` is pure and has no repository, // and the readers below — the protected gate, and any module over @@ -12590,6 +12584,40 @@ fn is_adjudicable(envelope: &hook::Envelope) -> bool { || (envelope.event == hook::Event::PreTool && !envelope.raw_tool.is_empty()) } +/// The mediated call on stdin, or `None` where the call must simply proceed. +/// +/// **THE TWO FAIL-OPEN BOUNDARIES, TOGETHER BECAUSE THEY ARE ONE ANSWER.** Stdin +/// that will not read and a payload that will not decode are both "the engine +/// does not know what this call IS", and neither may block it: a guard must never +/// be the reason a session cannot proceed. That is the opposite side of +/// [`unreadable_declaration`], where the engine knows the call perfectly well and +/// has been told it cannot enforce the rules over it. +/// +/// **Loud, never silent** (CLOUD-43). A guard that cannot read its input is a gate +/// that did not run, and the silent version of that is byte-identical to a clean +/// allow — the false green this engine exists to catch, in the one place nobody +/// would think to look. +/// **The RAW bytes travel with the decoded value**, because `dispatch_handlers` +/// hands a declared handler the payload as it arrived. Re-reading stdin for it is +/// not an option — the stream is consumed — and re-serializing the envelope would +/// hand a handler a document the host never sent. +fn read_envelope( + harness: hook::Harness, + mode: Mode, + err: &mut dyn Write, +) -> Result> { + let mut raw = String::new(); + if std::io::stdin().read_to_string(&mut raw).is_err() { + output::message(mode, Verbosity::Normal, err, UNREADABLE_STDIN)?; + return Ok(None); + } + let Some(envelope) = hook::decode(harness, &raw) else { + output::message(mode, Verbosity::Normal, err, UNDECODABLE_PAYLOAD)?; + return Ok(None); + }; + Ok(Some((raw, envelope))) +} + /// Whether this load failure is a declaration nothing could read at all. /// /// **Positively identified, never inferred from an absence.** The tempting From fffd7234f6dc896faa88e0af998b732d2d95c535 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 9 Sep 2026 23:37:49 +0000 Subject: [PATCH 07/18] fix(verdict): append the new class rather than grouping it, since position is API `Native` carries no `repr`, so a variant added in the middle shifts every later discriminant. Placing `ConfigUnreadable` beside the other config classes for readability moved eighteen of them, and `semver check` read the whole tail as broken under `enum_no_repr_variant_discriminant_changed`. Appended, and the reason is written onto the variant so the next reader who wants to group it tidily meets the cost first. Declaration order is API and is append-only; the reading order in `ALL` and `as_str` is free, and both keep the class beside its siblings where a reader looks for it. `semver check`: the API delta is patch-compatible against origin/main, so no break is declared and none is owed. 459 tests pass across the census and the affected suites. Refs: CLOUD-1677 --- crates/batten/src/verdict.rs | 60 ++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/crates/batten/src/verdict.rs b/crates/batten/src/verdict.rs index 4aa095466..067b79ad8 100644 --- a/crates/batten/src/verdict.rs +++ b/crates/batten/src/verdict.rs @@ -1213,33 +1213,6 @@ pub enum Native { // moment it fires. That also makes them resolvable from `vendored()` with // no config load, which is what keeps `policy explain` usable over a config // that will not parse. - /// The declaration could not be READ AT ALL — not valid TOML. - /// - /// **Deliberately not in [`Native::CONFIG_FAULTS`]**, which is the per-TABLE - /// set and is censused in both directions against `config.rs`'s own list. This - /// one has no table: it is raised before any table exists, by the parse that - /// every table's validator runs after. - /// - /// # Why it needs a class when the other parse failures do not - /// - /// A class here is not for `explain` — it is the DISCRIMINATOR the mediated - /// boundary acts on (CLOUD-1677). Gates are registered fail-open, and a gate - /// that fails open is inert: it neither allows nor denies, it is absent. So a - /// config fault is never a choice between refusing and allowing, it is a - /// choice between keeping the enforcement surface we still have and losing it - /// entirely. - /// - /// An unknown key, a version this build is too old for, a row whose validator - /// refused — each leaves every OTHER row readable and enforceable, and leaves - /// an agent that can still be told to repair the one that is broken. Failing - /// the whole load there buys nothing and costs the surface that would have - /// carried the repair instruction. - /// - /// A file that is not TOML is the one case with no partial function to - /// preserve: zero rows are readable, so refusing the call is the only signal - /// left, and the declared hatch is the recovery path. That asymmetry is why - /// this class exists and why it is exactly one class wide. - ConfigUnreadable, /// The `[[verb]]` table would not load. VerbTableRefused, /// The `[[pattern]]` table would not load. @@ -1285,6 +1258,39 @@ pub enum Native { /// than in a consumer `[[verdict]]` row. No consumer can name the class, /// because the engine is what takes the plan and what compares it. PlanReadStale, + /// The declaration could not be READ AT ALL — not valid TOML. + /// + /// **APPENDED, NEVER INSERTED.** This enum carries no `repr`, so a variant + /// added in the middle shifts every later discriminant and + /// `enum_no_repr_variant_discriminant_changed` reads the whole tail as + /// broken — measured here, where placing it beside the other config classes + /// moved eighteen of them. Position is API; the reading order below is not. + /// + /// **Deliberately not in [`Native::CONFIG_FAULTS`]**, which is the per-TABLE + /// set and is censused in both directions against `config.rs`'s own list. This + /// one has no table: it is raised before any table exists, by the parse that + /// every table's validator runs after. + /// + /// # Why it needs a class when the other parse failures do not + /// + /// A class here is not for `explain` — it is the DISCRIMINATOR the mediated + /// boundary acts on (CLOUD-1677). Gates are registered fail-open, and a gate + /// that fails open is inert: it neither allows nor denies, it is absent. So a + /// config fault is never a choice between refusing and allowing, it is a + /// choice between keeping the enforcement surface we still have and losing it + /// entirely. + /// + /// An unknown key, a version this build is too old for, a row whose validator + /// refused — each leaves every OTHER row readable and enforceable, and leaves + /// an agent that can still be told to repair the one that is broken. Failing + /// the whole load there buys nothing and costs the surface that would have + /// carried the repair instruction. + /// + /// A file that is not TOML is the one case with no partial function to + /// preserve: zero rows are readable, so refusing the call is the only signal + /// left, and the declared hatch is the recovery path. That asymmetry is why + /// this class exists and why it is exactly one class wide. + ConfigUnreadable, } impl Native { From c461c9025e7d549f48b3ed3a21f16a2fa53d534d Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Thu, 10 Sep 2026 06:05:30 +0000 Subject: [PATCH 08/18] fix(land): publish the borrowed base to every body gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-1770: a speculative lap threw away two lease acquisitions on keys it never served. `closing-key-check` reads the PR's commit range through `claimed-keys.sh`, which narrows on `$BATTEN_SPEC_BASE` — CLOUD-748's boundary, and the shared reader that row asks for. The reader was shared; the FACT was not. `speculation::PUBLISHED_AS` was published into the environment of `verify`'s child alone. `closing-key-check` runs at the Ready step, and `land::ready` shelled out with no environment at all — so the narrowing found nothing, fell back to the full range, and counted the lease holder's borrowed commits as this branch's own stranded keys. Measured over one session: two acquisitions taken, each spent on a 21-28 minute gate, each handed straight back. The bet now reaches every body gate through the same `Bet::published()` the verify step uses, so the publication stays a function of the bet rather than a side effect kept in step with it: a lap with no bet outstanding publishes nothing, and the entry is always emitted so `None` REMOVES the variable rather than leaving an inherited one. The body FETCH is deliberately not given it — that call reads the pull request's body, and the bet is a fact about the commit range. The case drives a child process's real environment rather than reading `std::env::var`, which would pass against the defect it exists to catch, and pins both directions: a bet published reaches the gate, and no bet publishes nothing. BREAKING CHANGE: `land::ready` takes the published speculation as a new fourth argument, so a consumer calling it must pass the publication set. An empty slice reproduces the previous behaviour, which published nothing to any gate. `cargo-semver-checks` names this `function_parameter_count_changed`. The crate is unpublished (CLOUD-205) and below `0.1.0` every release is a patch, so the type stays `fix` and the footer is what declares the break — the shape `4bc4f57` set for `handler::dispatch`'s new fourth argument. Refs: CLOUD-1770 --- crates/batten/src/exec.rs | 49 +++++++++++++++++++--- crates/batten/src/land.rs | 81 +++++++++++++++++++++++++++++++++---- crates/batten/src/lib.rs | 27 +++++++++++-- crates/batten/src/repair.rs | 2 +- 4 files changed, 142 insertions(+), 17 deletions(-) diff --git a/crates/batten/src/exec.rs b/crates/batten/src/exec.rs index 07e9d9511..9bf4d15af 100644 --- a/crates/batten/src/exec.rs +++ b/crates/batten/src/exec.rs @@ -1742,7 +1742,15 @@ pub(crate) fn piped( // relative name cannot arise and handing a directory would only be a guess at // one. // `Drop`: both callers of this entry point parse the string it returns. - piped_through(root, None, path.to_str()?, args, stdin, Diagnostics::Drop) + piped_through( + root, + None, + path.to_str()?, + args, + stdin, + Diagnostics::Drop, + &[], + ) } /// The one spawn both piped entry points share. @@ -1777,16 +1785,38 @@ fn piped_through( args: &[String], stdin: &str, diagnostics: Diagnostics, + published: &[(String, Option)], ) -> Option<(i32, String)> { let mut child = crate::rules::spawn_resolving(resolve_root, program, |resolved, extra| { - Command::new(OsString::from(resolved)) + let mut command = Command::new(OsString::from(resolved)); + command .args(extra.iter().map(OsString::from)) .args(args) .current_dir(root) .stdin(Stdio::piped()) .stdout(Stdio::piped()) - .stderr(diagnostics.redirection()) - .spawn() + .stderr(diagnostics.redirection()); + // THE BET TRAVELS TO EVERY GATE THAT READS THE COMMIT RANGE, not only to + // `verify`'s (CLOUD-1770). `BATTEN_SPEC_BASE` is the boundary + // `claimed-keys` narrows on, and it was published into the verify child's + // environment alone — so `closing-key-check`, which runs at a later step + // and delegates to the same reader, saw no bet and counted the holder's + // borrowed keys as this branch's own. Measured: two lease acquisitions + // spent and handed back on keys the speculation adopted. + // A `None` REMOVES rather than skips, and the difference is the whole + // reason this is `Option` (measured: this file's own suite runs INSIDE a + // `land` gate, which exports the variable, so a child that merely + // inherited it read a bet that was not its own). Publishing must be a + // FUNCTION of the bet — a lap with none outstanding has to say so, or a + // stale value from an outer process narrows the inner lap's commit range + // against a base it never borrowed. + for (name, value) in published { + match value { + Some(value) => command.env(name, value), + None => command.env_remove(name), + }; + } + command.spawn() }) .ok()?; // TAKEN AND DROPPED EVEN WHEN EMPTY, because a gate that reads stdin blocks @@ -1975,13 +2005,22 @@ pub(crate) fn piped_argv( argv: &[String], stdin: &str, diagnostics: Diagnostics, + published: &[(String, Option)], ) -> Option<(i32, String)> { let (program, operands) = argv.split_first()?; // `Some(root)`, where [`piped`] passes `None`: the first word here is a NAME // the ladder resolves, so rung 3 needs a directory to read a shebang out of. // That one argument IS the difference between the two entry points, which is // why they share [`piped_through`] and not a signature. - piped_through(root, Some(root), program, operands, stdin, diagnostics) + piped_through( + root, + Some(root), + program, + operands, + stdin, + diagnostics, + published, + ) } /// This process's next dispatch number, for the live-capture key. diff --git a/crates/batten/src/land.rs b/crates/batten/src/land.rs index ee84e9c33..50ec0d5de 100644 --- a/crates/batten/src/land.rs +++ b/crates/batten/src/land.rs @@ -1597,7 +1597,12 @@ pub enum Readied { /// right way round: a gate that cannot run has not passed, and treating it as /// clean is exactly how a retired or renamed gate goes silently dead. #[must_use] -pub fn ready(root: &Path, gates: &[Vec], body: &str) -> Readied { +pub fn ready( + root: &Path, + gates: &[Vec], + body: &str, + published: &[(String, Option)], +) -> Readied { if body.trim().is_empty() { return Readied::Clear; } @@ -1611,8 +1616,15 @@ pub fn ready(root: &Path, gates: &[Vec], body: &str) -> Readied { // is identical across every possible finding is not a pointer (review of // #848). let gate = argv.join(" "); + // `published` CARRIES THE BET (CLOUD-1770). A body gate that reads the + // PR's commit range — `closing-key-check` does, through `claimed-keys` — + // cannot otherwise tell a commit this branch authored from one the + // speculation adopted, and counts the holder's keys as this branch's + // stranded ones. CLOUD-748 fixed that for `claim-race-check` by + // publishing the base into `verify`'s child; this is the same boundary + // reaching the same reader through its other caller. let Some((code, output)) = - crate::exec::piped_argv(root, argv, body, crate::exec::Diagnostics::Keep) + crate::exec::piped_argv(root, argv, body, crate::exec::Diagnostics::Keep, published) else { return Readied::Unrunnable { gate }; }; @@ -1838,7 +1850,7 @@ pub fn admits_the_landing(root: &Path, gates: &[Vec], pr: &str) -> Admit let gate = with_pr.join(" "); with_pr.push(pr.to_owned()); let Some((code, output)) = - crate::exec::piped_argv(root, &with_pr, "", crate::exec::Diagnostics::Keep) + crate::exec::piped_argv(root, &with_pr, "", crate::exec::Diagnostics::Keep, &[]) else { // AN ADVISORY GATE THAT WILL NOT RUN IS NOT A REFUSAL EITHER, which // is the same reading one line down rather than a separate decision: @@ -3041,13 +3053,13 @@ mod tests { )]]; assert_eq!( - super::ready(&root, &gate, " \n "), + super::ready(&root, &gate, " \n ", &[]), super::Readied::Clear, "a body the fetch never produced says nothing, so there is nothing to judge" ); assert_eq!( - super::ready(&root, &gate, "Closes CLOUD-1"), + super::ready(&root, &gate, "Closes CLOUD-1", &[]), super::Readied::Unrunnable { gate: String::from("batten-no-such-program-for-the-ready-phase"), }, @@ -3077,7 +3089,7 @@ mod tests { ]]; assert_eq!( - super::ready(&root, &gate, "Closes CLOUD-1"), + super::ready(&root, &gate, "Closes CLOUD-1", &[]), super::Readied::Unrunnable { gate: String::from( "batten-no-such-runner-for-the-ready-phase run closing-key-check" @@ -3087,12 +3099,67 @@ mod tests { ); } + /// **THE BET REACHES A BODY GATE, AND FOR ITS WHOLE LIFE IT DID NOT** + /// (CLOUD-1770). + /// + /// `BATTEN_SPEC_BASE` is the boundary `claimed-keys` narrows the commit range + /// on. It was published into `verify`'s child environment alone, so + /// `closing-key-check` — a LATER step delegating to that same reader — saw no + /// bet and counted the lease holder's borrowed commits as keys this branch + /// had served and stranded. Measured over one session: two lease acquisitions + /// taken, spent on a 21–28 minute gate, and handed straight back. + /// + /// The gate here is `sh -c` over the variable, so its verdict is a fact about + /// the CHILD's environment rather than about this process's — a case reading + /// `std::env::var` would pass against the defect it exists to catch. + #[test] + fn a_body_gate_is_told_which_base_the_lap_borrowed() { + let root = std::env::temp_dir(); + let gate = vec![vec![ + String::from("sh"), + String::from("-c"), + String::from("test -n \"$BATTEN_SPEC_BASE\""), + ]]; + let published = vec![( + String::from(crate::speculation::PUBLISHED_AS), + Some(String::from("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")), + )]; + // A lap with NO bet, spelled as the removal it has to be. `&[]` would + // leave whatever the parent exported in place — and this suite runs + // inside a `land` gate that exports exactly this variable, which is how + // the first version of this case failed against its own subject. + let unpublished = vec![(String::from(crate::speculation::PUBLISHED_AS), None)]; + + assert_eq!( + super::ready(&root, &gate, "Closes CLOUD-1", &published), + super::Readied::Clear, + "a speculative lap must tell its body gates which base it borrowed" + ); + + // **THE MIRROR, and without it the case above passes on any environment + // that happens to carry the variable** — a developer's shell, an outer + // `land`, or a sibling test leaking one. That is not hypothetical: this + // suite runs as a child of the gate `mise run land` drives, which + // publishes this very variable, and the first version of this case read + // that outer bet and failed. + // + // So "no bet" is an explicit REMOVAL rather than an omission, and the + // mechanism now matches the claim: publication is a function of the bet. + assert!( + matches!( + super::ready(&root, &gate, "Closes CLOUD-1", &unpublished), + super::Readied::Refused { .. } + ), + "a lap carrying no bet must publish no base" + ); + } + /// No declared gates is a clear ready, and the distinction from `Unrunnable` /// is the optional-versus-dead one the driver's own header states. #[test] fn a_consumer_declaring_no_body_gates_is_clear_rather_than_unrunnable() { assert_eq!( - super::ready(&std::env::temp_dir(), &[], "Closes CLOUD-1"), + super::ready(&std::env::temp_dir(), &[], "Closes CLOUD-1", &[]), super::Readied::Clear ); } diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index eb12c5bfb..4db075f49 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -7300,7 +7300,7 @@ fn run_land_lap( run_land_verify(root, &bet, branch, Some(reference), out, err)? } land::Step::Lease => run_land_lease(root, branch, out, err)?, - land::Step::Ready => run_land_ready(root, branch, &mut ledger, out, err)?, + land::Step::Ready => run_land_ready(root, branch, &bet, &mut ledger, out, err)?, land::Step::Push => run_land_push(root, url, branch, out)?, land::Step::Wait => { let (code, verdict) = run_land_wait(root, reference, branch, out, err)?; @@ -9242,6 +9242,7 @@ fn trunk_watch(reference: &str, base: &str, repo: &str, interval: u64) -> main_w fn run_land_ready( root: &Path, branch: &str, + bet: &speculation::Bet, ledger: &mut land::Ledger, out: &mut dyn Write, err: &mut dyn Write, @@ -9265,12 +9266,30 @@ fn run_land_ready( // `Drop`: this string is PARSED as the body, so a client's notice // on stderr would become text the author never wrote. The gates // below take `Keep`, because their stderr IS their reason. - .and_then(|argv| exec::piped_argv(root, argv, "", exec::Diagnostics::Drop)) + // NO BET PUBLISHED TO THE FETCH, deliberately. This call reads the + // pull request's BODY; the bet is a fact about the commit RANGE, and + // handing it to a client that does not read one would be a variable + // in an environment for no reader. + .and_then(|argv| exec::piped_argv(root, argv, "", exec::Diagnostics::Drop, &[])) .filter(|(code, _)| *code == 0) .map(|(_, body)| body) .unwrap_or_default(); - match land::ready(root, &gates, &body) { + // THE SAME PUBLICATION `run_land_verify` MAKES, and for the same reason + // one step later (CLOUD-1770). `Bet::published` is `None` with no bet + // outstanding, so the variable is simply absent from a non-speculative + // lap's gates — the publication stays a function of the bet rather than a + // side effect kept in step with it. + // ONE ENTRY ALWAYS, and its VALUE is the bet. An empty list would leave + // an inherited `BATTEN_SPEC_BASE` in place — and this lap's own gates run + // as children of a `verify` that exports it, so "no bet" has to be said + // out loud rather than left unsaid (measured: the case for this asserted + // the mirror and read an outer process's bet). + let published: Vec<(String, Option)> = vec![( + speculation::PUBLISHED_AS.to_owned(), + bet.published().map(str::to_owned), + )]; + match land::ready(root, &gates, &body, &published) { land::Readied::Clear => { writeln!(out, "land: {} body gate(s) clear", gates.len())?; } @@ -10803,7 +10822,7 @@ fn note_release(root: &Path) { let Some(argv) = land::body_gates(&declared).into_iter().next() else { return; }; - let _ = exec::piped_argv(root, &argv, "", exec::Diagnostics::Keep); + let _ = exec::piped_argv(root, &argv, "", exec::Diagnostics::Keep, &[]); } /// `lease release`: a tombstone, never a delete. diff --git a/crates/batten/src/repair.rs b/crates/batten/src/repair.rs index 69d1b05e6..0126c7365 100644 --- a/crates/batten/src/repair.rs +++ b/crates/batten/src/repair.rs @@ -163,7 +163,7 @@ pub fn run(root: &Path, fix: &str, key: Option<&str>, applicability: Applicabili // `Diagnostics::Drop`: the repair's own chatter is not a finding, and a // consumer's command could print anything at all. let Some((code, _output)) = - crate::exec::piped_argv(root, &words, "", crate::exec::Diagnostics::Drop) + crate::exec::piped_argv(root, &words, "", crate::exec::Diagnostics::Drop, &[]) else { // The program would not resolve. A declared repair naming something this // host does not have is a config defect, and the ordinary refusal is the From 660b98d7296497b3f7c8602616050855cc9c34d5 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Thu, 10 Sep 2026 06:57:06 +0000 Subject: [PATCH 09/18] fix(land): refuse the publish while a bet is outstanding, and lap instead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-1681. `land.sh`'s invariant, quoted in `pipeline.rs`: "there is no path from a losing bet to a push, which is what makes speculating safe rather than merely fast." `Settle::Lost` has none — `Precheck::BetSettled` unwinds it at the top of the lap. `Settle::Pending` had one, because that precheck deliberately KEEPS a pending bet while the `Step::Push` row carried `precheck: None`. Measured twice: four commits of another branch published under this one on 2026-09-08, eight on 2026-09-10, the second costing a full matrix on a head that could not merge when it was graded. WHY A PENDING BET IS UNPUBLISHABLE, corrected. The reason is not that the holder's shas churn: landing is fast-forward and PRESERVES the sha, so a bet that pays off was exactly right and its work carries over intact. The defect is publishing a bet that has NOT paid off — the borrowed range is another branch's unlanded commits, and this pull request's head must carry only its own, because that head is what trunk fast-forwards onto. `Bet::pushed`'s own doc names the outcome: the measured two-PRs-at-one-sha state. (An earlier revision of this change argued from sha churn instead. That premise is false and is corrected here rather than carried forward; the mechanism it justified is unchanged and right. `mem:decision/landing-architecture` is the authority.) `Precheck::BetLive` answers `Lap`, never `Stop`. `Stop` would strand the waiter behind CLOUD-1306's poisoned base indefinitely and empty the pipeline the speculation exists to keep full — trading a wasted matrix for an unbounded stall. The lap drops the borrowed range and re-enters: replay onto real trunk, verify the unspeculated tree, publish this branch's own commits alone. TWO THINGS THE ROW'S §3 DOES NOT STATE, both established from the tree. The unwind is done by the arm rather than by the lap's compensations. No `Compensation` drops a bet — `Nothing`, `Redraft`, `Abandon`, `ReleaseLease` — so answering `Lap` alone would carry the borrowed range into the next lap. It reuses `unwind_the_bet`, the same drop `Settle::Lost` already takes. And `Bet::declined` is what makes the lap terminate. `place_the_bet`'s guards are about the HOLDER, so without a memory of the decision the next lap re-bets the same one, returns to this row and unwinds again until the lap budget is spent. A bool rather than a base, because it records a decision about THIS LANDING and not a judgement about a commit; it survives `forget`, which is what the unwind calls. BREAKING CHANGE: `speculation::Bet` gains a public `declined` field. `Bet` is a constructible struct carrying no `#[non_exhaustive]`, so a downstream struct literal over it stops compiling; `false` reproduces the previous behaviour, which had no memory of a declined landing. `cargo-semver-checks` names this `constructible_struct_adds_field` — the same lint `2ef3df5` declared for `Config` and `resolve::Resolved`. Refs: CLOUD-1681 --- crates/batten/src/lib.rs | 36 ++++++ crates/batten/src/pipeline.rs | 89 ++++++++++++- crates/batten/src/speculation.rs | 18 +++ crates/batten/tests/it/land_speculation.rs | 138 +++++++++++++++++++++ crates/batten/tests/it/main.rs | 1 + mise.toml | 2 +- 6 files changed, 282 insertions(+), 2 deletions(-) create mode 100644 crates/batten/tests/it/land_speculation.rs diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 4db075f49..873d73377 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -8025,6 +8025,14 @@ fn place_the_bet( // second guard's own comment reasons that "`run_land_replay` immediately // fetches a fresh trunk", which it does — one step AFTER this runs. advance_trunk(root, reference); + // THIS LANDING ALREADY DECLINED TO PUBLISH ONE (CLOUD-1681). The `Push` + // precheck unwound a bet that reached the publish, so betting again buys + // nothing before this branch lands and costs another unwind at the same row. + // Checked FIRST, because it is a fact about us rather than about the holder + // and no reading below can change it. + if bet.declined { + return Ok(()); + } let tracking = land::tracking_ref(reference); // THE HOLDER ALREADY LANDED. Their head is on the trunk, so an ordinary replay // reaches it and a bet would borrow a range that is not borrowed. @@ -10029,6 +10037,34 @@ fn asks_before_the_step( )?; Ok(Answered::Stop(exit::ExitCode::Violation)) } + // A LIVE BET NEVER REACHES THE PUBLISH (CLOUD-1681). + // + // The unwind is done HERE rather than left to the lap's compensations, + // and that is the arm's correctness rather than its convenience: no + // `Compensation` drops a bet — `Nothing`, `Redraft`, `Abandon`, + // `ReleaseLease` — so answering `Lap` alone would carry the borrowed + // range straight into the next lap and back to this row. + // + // `unwind_the_bet` is the same drop `Settle::Lost` already takes, so + // this is a second caller for an existing arm rather than new machinery. + Some(pipeline::Precheck::BetLive) if bet.live() => { + writeln!( + out, + "land: a speculation is still outstanding, so this head is not publishable — unwinding and lapping onto real trunk" + )?; + if let Some(code) = unwind_the_bet(root, url, branch, bet, reference, out, err)? { + // The unwind itself refused — a tree it could not rewind is the + // one thing that stops rather than laps, because carrying on + // would push another branch's commits under this one. + return Ok(Answered::Stop(code)); + } + // AND WE DO NOT BET AGAIN THIS LANDING, which is what makes the lap + // terminate. `place_the_bet`'s own guards are about the HOLDER, so + // without this the next lap re-bets the same one, arrives back here, + // and unwinds again until the lap budget is spent. + bet.declined = true; + Ok(Answered::Lap) + } _ => Ok(Answered::Go), } } diff --git a/crates/batten/src/pipeline.rs b/crates/batten/src/pipeline.rs index dfa8b7c9f..4c5e5186a 100644 --- a/crates/batten/src/pipeline.rs +++ b/crates/batten/src/pipeline.rs @@ -51,6 +51,11 @@ //! That is raise-only in the same spirit as the deny-only Rego surface: a //! consumer may compose any pipeline, but not one that leaks spend. +// CLOUD-1681: dropping the `Push` row's precheck restores the path from a +// pending bet to a publish, which is the state measured twice. +//MUTANT-SUITE crates/batten/tests/it/land_speculation.rs +//MUTANT live-bet-reaches-the-push|s@ precheck: Some(Precheck::BetLive),@ precheck: None,@|a_live_bet_is_refused_by_the_row_that_would_publish_it + use crate::land::Step; /// A step's declared undo, run when a lap leaves without landing. @@ -243,6 +248,44 @@ pub enum Precheck { /// that reads as a narrowing: the next reader should find the gap written /// down rather than infer its absence. LeaseHeld, + /// Is a speculation still outstanding on the head we are about to publish? + /// + /// **THE INVARIANT THIS FILE ALREADY QUOTES AND DID NOT HOLD** (CLOUD-1681). + /// `mise-tasks/land.sh`, verbatim above: *"there is no path from a losing bet + /// to a push, which is what makes speculating safe rather than merely fast."* + /// [`Settle::Lost`] has no such path — [`Self::BetSettled`] unwinds it at the + /// top of the lap. [`Settle::Pending`] does, because that precheck + /// deliberately KEEPS a pending bet, and this row carried no precheck at all. + /// + /// # A PENDING BET IS EXACTLY AS UNPUBLISHABLE AS A LOST ONE + /// + /// A bet names the holder's SHAS, and the holder lands by rebase — which + /// mints new ones for the same patches. So a published pending head does not + /// merely risk going stale: the moment the holder lands it is DIVERGENT, and + /// the fast-forward is impossible by construction rather than by race. The + /// bet is unwinnable at the instant it is placed, so publishing it guarantees + /// a wasted matrix rather than risking one. + /// + /// Measured twice — four commits of another branch published under this one + /// on 2026-09-08, eight on 2026-09-10, the second costing a full matrix on a + /// head that could not merge when it was graded. + /// + /// **It answers `Lap`, never `Stop`, and that is the design.** `Stop` strands + /// the waiter behind CLOUD-1306's poisoned base indefinitely, trading a + /// wasted matrix for an unbounded stall. `Lap` drops the borrowed range and + /// re-enters: replay onto real trunk, verify the UNSPECULATED tree, publish + /// this branch's own commits alone — one extra local verify and zero CI. + /// + /// **Speculation keeps its whole value.** A holder that lands during `verify` + /// settles [`Settle::Landed`], the bet is forgotten, nothing unwinds, and the + /// lap publishes the pre-linearized head exactly as before. This fires only + /// where the bet failed to pay off — precisely where carrying the borrowed + /// range is worthless AND harmful. + /// + /// [`Settle::Lost`]: crate::speculation::Settle::Lost + /// [`Settle::Pending`]: crate::speculation::Settle::Pending + /// [`Settle::Landed`]: crate::speculation::Settle::Landed + BetLive, } /// A declared landing pipeline. @@ -450,7 +493,11 @@ impl Default for Pipeline { step: Step::Push, effectful: true, compensate: Compensation::ReleaseLease, - precheck: None, + // THE ROW THAT PUBLISHES IS THE ROW THAT ASKS (CLOUD-1681). + // `Replay` asks whether the bet is worth KEEPING; this asks + // whether it is fit to PUBLISH, and the two answers differ + // for a pending bet — which is the whole gap. + precheck: Some(Precheck::BetLive), }, StepRow { step: Step::Ready, @@ -530,6 +577,46 @@ mod tests { ); } + /// **THE PUBLISHING ROW ASKS BEFORE IT PUBLISHES** (CLOUD-1681). + /// + /// `mise-tasks/land.sh`'s invariant, quoted in this file's own header, is + /// *"there is no path from a losing bet to a push"* — and for the whole of + /// this table's life the `Push` row carried `precheck: None` while `Replay`, + /// `Ready` and the commit point each carried one. A pending bet reached the + /// publish, twice measured. + /// + /// **The mirror is the half that discriminates.** Asserting only that `Push` + /// has a precheck passes over a table where every row has one, which would be + /// its own defect — `Verify` spends the gate and must not be gated on the bet, + /// or a speculating lap could never verify the tree it speculated. + #[test] + fn the_push_row_asks_about_the_bet_and_the_verify_row_does_not() { + let shipped = Pipeline::default(); + let precheck = |step: Step| { + shipped + .steps + .iter() + .find(|row| row.step == step) + .and_then(|row| row.precheck) + }; + + assert_eq!( + precheck(Step::Push), + Some(Precheck::BetLive), + "the row that publishes must ask whether the head is publishable" + ); + assert_eq!( + precheck(Step::Verify), + None, + "gating the gate on the bet would stop a speculating lap verifying at all" + ); + assert_eq!( + precheck(Step::Replay), + Some(Precheck::BetSettled), + "and the settle stays where it was — the two ask different questions" + ); + } + /// **`Abandon` IS THE ONLY ATTEMPT-OWED UNDO, and the mirror is what makes /// this case discriminate.** Without the second half, a predicate returning /// `true` for everything would satisfy the first — and that predicate would diff --git a/crates/batten/src/speculation.rs b/crates/batten/src/speculation.rs index 671695cea..3400636b9 100644 --- a/crates/batten/src/speculation.rs +++ b/crates/batten/src/speculation.rs @@ -168,6 +168,23 @@ pub struct Bet { /// it, and reading it as "no more bets at all" would give up speculating for /// the rest of the landing over one bad candidate. pub conflicts: Option, + /// This landing has already declined to publish a speculation (CLOUD-1681). + /// + /// **THE TERMINATION HALF, and without it the precheck is a spin.** The + /// `Push` row's precheck unwinds a live bet and laps; nothing in the lap + /// otherwise remembers that, so `place_the_bet` would bet on the same holder + /// at the top of the next lap, reach the same precheck, and unwind again — + /// bounded by the lap budget rather than by the one extra local verify the + /// design promises. + /// + /// A `bool` rather than a base, unlike [`Bet::conflicts`]: that + /// records a judgement about a COMMIT and stays true of it afterwards. This records a decision about THIS LANDING — we got as far + /// as the push with a bet outstanding, so speculating again buys nothing + /// before this branch lands. Naming a base would invite re-betting on the + /// next holder and paying the same unwind a second time. + /// + /// Survives [`Bet::forget`], because `forget` is what the unwind calls. + pub declined: bool, } impl Bet { @@ -521,6 +538,7 @@ mod tests { recovered: true, pushed: true, conflicts: Some(String::from("feedface")), + declined: true, }; bet.forget(); assert!(bet.is_forgotten(), "a settled bet carried state forward"); diff --git a/crates/batten/tests/it/land_speculation.rs b/crates/batten/tests/it/land_speculation.rs new file mode 100644 index 000000000..82fa1511b --- /dev/null +++ b/crates/batten/tests/it/land_speculation.rs @@ -0,0 +1,138 @@ +//! A live bet never reaches the publish (CLOUD-1681). +//! +//! # What this tier is for +//! +//! `mise-tasks/land.sh`'s invariant, quoted verbatim in `pipeline.rs`: +//! +//! > there is no path from a losing bet to a push, which is what makes +//! > speculating safe rather than merely fast. +//! +//! `Settle::Lost` has no such path — `Precheck::BetSettled` unwinds it at the top +//! of the lap. `Settle::Pending` had one, because that precheck deliberately +//! KEEPS a pending bet and the `Step::Push` row carried `precheck: None`. +//! Measured twice: four commits of another branch published under this one on +//! 2026-09-08, eight on 2026-09-10 — the second costing a full CI matrix on a +//! head that could not merge when it was graded. +//! +//! # Why a pending bet is exactly as unpublishable as a lost one +//! +//! A bet names the holder's SHAS, and the holder lands by rebase, which mints new +//! ones for the same patches. So a published pending head is not merely at risk +//! of going stale — the moment the holder lands it is DIVERGENT, and the +//! fast-forward is impossible by construction rather than by race. +//! +//! # The composition is the subject, not a lap +//! +//! Every case here decides over `Pipeline::default()` and `speculation::Bet`, +//! both of which are pure. Driving a real lap would need a remote, a lease and a +//! matrix to answer a question the step table already answers — and the defect +//! was never in the lap's execution, it was in a row that declared no question. + +// Panicking on a failed assertion is how a test fails loudly. +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use batten::land::Step; +use batten::pipeline::{Pipeline, Precheck}; +use batten::speculation::Bet; + +/// The row that publishes, and the question it now asks. +fn precheck_of(step: Step) -> Option { + Pipeline::default() + .steps + .iter() + .find(|row| row.step == step) + .and_then(|row| row.precheck) +} + +/// A bet that is outstanding, as `place_the_bet` leaves one. +fn placed() -> Bet { + Bet { + base: Some(String::from("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")), + undo: Some(String::from("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb")), + main_at_bet: Some(String::from("cccccccccccccccccccccccccccccccccccccccc")), + ..Bet::default() + } +} + +/// **A LAP HOLDING A LIVE BET DOES NOT REACH THE PUSH.** +/// +/// The two halves of the reading are asserted together because either alone is +/// satisfiable by the defect: the row could declare the precheck while the bet +/// answers `false`, or the bet could be live over a row that asks nothing. The +/// pair is what the lap actually evaluates. +#[test] +fn a_live_bet_is_refused_by_the_row_that_would_publish_it() { + assert_eq!( + precheck_of(Step::Push), + Some(Precheck::BetLive), + "the row that publishes must ask whether this head is publishable" + ); + assert!( + placed().live(), + "a placed bet is outstanding, which is the state the precheck refuses" + ); +} + +/// **THE MIRROR — A SETTLED BET STILL PUSHES.** +/// +/// Without this the case above is satisfied by a pipeline that never publishes +/// at all, which is an outage rather than a fix. A holder that lands during +/// `verify` settles `Landed`, `drop_the_bet` forgets it, and the lap publishes +/// the pre-linearized head exactly as before — which is the whole value of +/// speculating and the reason this refuses `live()` rather than "a bet was ever +/// placed". +#[test] +fn a_settled_bet_still_reaches_the_push() { + let mut settled = placed(); + settled.forget(); + assert!( + !settled.live(), + "a settled bet is not outstanding, so the precheck admits the push" + ); +} + +/// **AND THE GATE DOES NOT SPREAD TO THE ROW THAT SPENDS THE GATE.** +/// +/// `Verify` must stay unguarded on the bet, or a speculating lap could never +/// verify the tree it speculated — which is the point of speculating. Asserting +/// only that `Push` carries a precheck would pass over a table that gated every +/// row, so this is where the arm's boundary is written down. +#[test] +fn the_verify_row_is_not_gated_on_the_bet() { + assert_eq!( + precheck_of(Step::Verify), + None, + "gating the gate on the bet would stop a speculating lap verifying at all" + ); + assert_eq!( + precheck_of(Step::Replay), + Some(Precheck::BetSettled), + "and the settle stays on Replay — the two rows ask different questions" + ); +} + +/// **THE TERMINATION PROPERTY, which the row's own §3 does not state.** +/// +/// The precheck answers `Lap`, and no `Compensation` drops a bet — `Nothing`, +/// `Redraft`, `Abandon`, `ReleaseLease`. `place_the_bet`'s own guards are about +/// the HOLDER (already landed; trunk has passed it), so without a memory of the +/// decision the next lap re-bets the same holder, arrives back at this row, and +/// unwinds again until the lap budget is spent — rather than the one extra local +/// verify and zero CI the design promises. +/// +/// `declined` is that memory, and it survives `forget` because `forget` is what +/// the unwind calls. +#[test] +fn a_declined_landing_does_not_speculate_again() { + let mut bet = placed(); + bet.declined = true; + bet.forget(); + assert!( + bet.declined, + "the decision must outlive the unwind that made it, or the lap spins" + ); + assert!( + !bet.live(), + "and the borrowed range is gone, which is what the next lap replays without" + ); +} diff --git a/crates/batten/tests/it/main.rs b/crates/batten/tests/it/main.rs index 07e8d7501..a983d312a 100644 --- a/crates/batten/tests/it/main.rs +++ b/crates/batten/tests/it/main.rs @@ -159,6 +159,7 @@ mod land_entry_gates; mod land_forge_reads; mod land_hand_stepping; mod land_lap; +mod land_speculation; mod land_verify_advice; mod landed_check; mod landing_roster; diff --git a/mise.toml b/mise.toml index 2673c3aa2..2e1feade9 100644 --- a/mise.toml +++ b/mise.toml @@ -617,7 +617,7 @@ CI_FANIN_WORKFLOW = ".github/workflows/ci.yml" BATS_TEST_TIMEOUT = "300" REGORUS_OPA_COMPLIANCE = "1.2.0" REGORUS_OPA_COMPLIANCE_FOR = "0.11" -MUTANT_GATES = "mise,attestation-check,engine-checks-green,engine-config,engine-doctor,engine-landed,engine-perf,engine-mcp,engine-pinned,engine-ready,engine-verdict,engine-wiring,engine-surface,agentic-experiment-record,awk-regex-check,bats-invocation,batten-glob-check,board-diff-overlap,board-payloads,board-sweep,branch-age-check,cap-drift,cfg-gated-test,ci-cache-declared,ci-hygiene,ci-parity,ci-slow-needed,ci-suite-lane,ci-tools-check,claim-before-code,claim-order-is-stated,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,egress-fencing,evaluator-closure-check,evaluator-io-check,filed-here,finding-sink-check,fixture-forks,forge-verdict-required,graph-check,harness-grant,harness-wiring,hk-fix-selection,hk-plan-required,hook-pin-check,hook-skip-local,in-progress-drain,install-check,land-divergence-assert,landed-check,landing-loop,landing-roster-guarded,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,mutation-declared-case,no-doctests,nonverdict-assert,ntia-check,obligations-bound,perf-assert,pinned-toolchain,pipefail-grep-check,plan-complete,pr-partition-restated,pr-unsubscribed,privileged-lane,prose-only,publish-credential-check,ready-cites-check,ready-lint,reclaim-census,release-assets-check,release-due,release-provision-parity,release-tag-shape,release-tracking-check,released,remedy-authorship,repetition-without-progress,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,test-targets,timeout-check,token-bench-check,transcript-corpus-check,tree-clean,trunk-based,validator-verdict-clean,verdict-routes-resolve,weakens-declared,worktree-registration,spawn-widening,nextest-slow,engine-lease,engine-handler,engine-speculation" +MUTANT_GATES = "mise,attestation-check,engine-checks-green,engine-config,engine-doctor,engine-landed,engine-perf,engine-mcp,engine-pinned,engine-ready,engine-verdict,engine-wiring,engine-surface,agentic-experiment-record,awk-regex-check,bats-invocation,batten-glob-check,board-diff-overlap,board-payloads,board-sweep,branch-age-check,cap-drift,cfg-gated-test,ci-cache-declared,ci-hygiene,ci-parity,ci-slow-needed,ci-suite-lane,ci-tools-check,claim-before-code,claim-order-is-stated,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,egress-fencing,evaluator-closure-check,evaluator-io-check,filed-here,finding-sink-check,fixture-forks,forge-verdict-required,graph-check,harness-grant,harness-wiring,hk-fix-selection,hk-plan-required,hook-pin-check,hook-skip-local,in-progress-drain,install-check,land-divergence-assert,landed-check,landing-loop,landing-roster-guarded,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,mutation-declared-case,no-doctests,nonverdict-assert,ntia-check,obligations-bound,perf-assert,pinned-toolchain,pipefail-grep-check,plan-complete,pr-partition-restated,pr-unsubscribed,privileged-lane,prose-only,publish-credential-check,ready-cites-check,ready-lint,reclaim-census,release-assets-check,release-due,release-provision-parity,release-tag-shape,release-tracking-check,released,remedy-authorship,repetition-without-progress,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,test-targets,timeout-check,token-bench-check,transcript-corpus-check,tree-clean,trunk-based,validator-verdict-clean,verdict-routes-resolve,weakens-declared,worktree-registration,spawn-widening,nextest-slow,engine-lease,engine-handler,engine-speculation,engine-pipeline" # --- 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. From b17ba038917bb7134400e789d15a471e1b196009 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Fri, 11 Sep 2026 01:27:54 +0000 Subject: [PATCH 10/18] fix(commit): a path with a sanctioned mutation owes no articulation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `judge_admissions` demanded an articulation block for every staged protected path. A path a `[[redirect]]` speaks for is written through a surface that refuses nothing, so no admission is ever issued and there is nothing to articulate — while the override route's own precondition says it exists for when "writing the protected path directly is the only route left". Demanding a block there left an honest author a choice between a false articulation and not committing at all. Measured: `.serena/memories/**` joined `protected` and the `[[redirect]]` table together, and the first commit writing a memory through `write_memory` was refused six times over. `ec32a76` is the root commit and the only commit in this history touching that glob, so the pairing had never been exercised by either half of the clause. ONLY THE DEMAND IS DROPPED, NEVER THE INSPECTION. An earlier shape of this fix exempted the path before looking at it, which silently lost `admits-tampered` for the whole exempted set. Here the redirect answers only the absence case; a block that claims a redirected path is still verified and still reported when it does not recompute, and a declared `//MUTANT` pins that the exemption cannot widen to cover it. Not a receipt, and that is the design. CLOUD-1303's filed remedy was a receipt minted where the write is allowed and read by this clause — but a receipt lives in the container-scoped store, and CI runs the range half over the same protected list on a runner that has never seen it. That is the local-pass, CI-abstain asymmetry this module already forbids in prose and pins with `the_clause_needs_no_store_to_decide`. The redirect table is config, so both halves decide identically anywhere. Closes CLOUD-1303 --- crates/batten/src/commit.rs | 69 ++++++++++++--- crates/batten/src/lib.rs | 18 ++-- crates/batten/tests/it/commit_admission.rs | 99 ++++++++++++++++++++++ 3 files changed, 170 insertions(+), 16 deletions(-) diff --git a/crates/batten/src/commit.rs b/crates/batten/src/commit.rs index eb8238190..e40609ca1 100644 --- a/crates/batten/src/commit.rs +++ b/crates/batten/src/commit.rs @@ -197,8 +197,40 @@ impl Commit { /// which is the whole reason the block spells every binding field out instead of /// carrying a reference. A tier that needed the store would pass locally and /// abstain in CI, which is the shape of a gate that is not there. +/// +/// # A path with a sanctioned mutation owes no block, and that is the override +/// route's own precondition read back (CLOUD-1303) +/// +/// `path write refused` declares its override route for the case where *"the +/// surface this class names cannot express the change, so writing the protected +/// path directly is the only route left"*. A path a `[[redirect]]` speaks for is +/// the negation of that sentence: the surface exists, the write goes through it, +/// nothing is refused, and so **no admission is ever issued and there is nothing +/// to articulate**. Demanding a block there leaves exactly one route — an +/// override whose precondition is false — so the honest author must either write +/// a false articulation or not commit at all. Measured: `.serena/memories/**` +/// joined `protected` and the `[[redirect]]` table together, and the first commit +/// to write a memory through `write_memory` was refused six times over. +/// +/// Articulation is therefore owed by protected paths with **no** sanctioned +/// mutation — which is the set the precondition describes. +/// +/// **ONLY THE DEMAND IS DROPPED, NEVER THE INSPECTION.** The earlier shape of +/// this fix exempted the path before looking at it, which silently lost +/// `admits-tampered` for the whole exempted set — a doctored block on a memory +/// would have gone unexamined. Here the redirect answers only the *absence* case: +/// a block that claims the path is still verified, and still reported when it +/// does not recompute. The graver finding keeps its whole subject set. +/// +/// `redirects` is the config's own table, passed in rather than resolved here for +/// [`ArmSequence`]'s reason: the predicate stays a pure function of its arguments, +/// so it decides identically on a runner that cannot reach the store OR the +/// config — which is what keeps the range half and the pending half from drifting. #[must_use] -pub fn judge_admissions(writes: &[crate::git::CommitWrite]) -> Vec { +pub fn judge_admissions( + writes: &[crate::git::CommitWrite], + redirects: &[crate::redirect::Redirect], +) -> Vec { let mut found = Vec::new(); for write in writes { let blocks = crate::admission::blocks(&write.message); @@ -207,15 +239,23 @@ pub fn judge_admissions(writes: &[crate::git::CommitWrite]) -> Vec { .iter() .filter(|block| block.binding.subject == *path) .collect(); - // A path with no block at all is the missing case. A path with blocks - // of which at least one verifies is clean — several are legitimate, - // since re-articulating the same path on one commit chains rather than - // replaces. + // A path with no block at all is the missing case — unless a + // `[[redirect]]` sanctions a mutation for it, which means the write + // had a route that refuses nothing and issues nothing. A path with + // blocks of which at least one verifies is clean — several are + // legitimate, since re-articulating the same path on one commit chains + // rather than replaces. let field = if claims.is_empty() { + if crate::redirect::resolve(redirects, path).is_some() { + continue; + } "admits" } else if claims.iter().any(|block| block.recomputes()) { continue; } else { + // REACHED FOR A REDIRECTED PATH TOO, and that is the half the + // superseded fix dropped. + //MUTANT admits-tampered-survives-the-redirect|s/^ } else if claims/ } else if crate::redirect::resolve(redirects, path).is_some() || claims/|a tampered block on a redirected path is still refused "admits-tampered" }; found.push(Finding { @@ -239,12 +279,19 @@ pub fn judge_admissions(writes: &[crate::git::CommitWrite]) -> Vec { /// explicitly not what this commit is about, and demanding a block for it would /// refuse a commit that does not touch the path at all. #[must_use] -pub fn judge_pending(message: &str, staged: &std::collections::BTreeSet) -> Vec { - judge_admissions(&[crate::git::CommitWrite { - commit: "pending".to_owned(), - message: message.to_owned(), - paths: staged.clone(), - }]) +pub fn judge_pending( + message: &str, + staged: &std::collections::BTreeSet, + redirects: &[crate::redirect::Redirect], +) -> Vec { + judge_admissions( + &[crate::git::CommitWrite { + commit: "pending".to_owned(), + message: message.to_owned(), + paths: staged.clone(), + }], + redirects, + ) } /// One commit's two conserves-ledger sets, ready to intersect (CLOUD-1402). diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 873d73377..9a890e1e0 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -11375,7 +11375,14 @@ fn commit_admissions( overrides: &Overrides, ) -> Result> { let root = Path::new("."); - let protected = resolve::resolve(root, overrides)?.protected.clone(); + // ONE resolve for both tables. `protected` answers "is this guarded" and + // `redirects` answers "what should they run instead" — deliberately + // independent sets (`redirect.rs`), and the articulation clause is the one + // reader that needs both: it demands a block from the first set minus the + // second (CLOUD-1303). + let resolved = resolve::resolve(root, overrides)?; + let protected = resolved.protected.clone(); + let redirects = resolved.redirects.clone(); match (range, message) { (Some(range), None) => { // Already validated above; re-split rather than threaded, because a @@ -11384,9 +11391,10 @@ fn commit_admissions( let Some((base, head)) = range.split_once("..") else { return Ok(Vec::new()); }; - Ok(commit::judge_admissions(&git::writes_in_range( - root, base, head, &protected, - )?)) + Ok(commit::judge_admissions( + &git::writes_in_range(root, base, head, &protected)?, + &redirects, + )) } (None, Some(message)) => { let body = std::fs::read_to_string(message).map_err(|error| { @@ -11405,7 +11413,7 @@ fn commit_admissions( .into_iter() .filter(|path| selectors.iter().any(|selector| selector.matches(path))) .collect(); - Ok(commit::judge_pending(&body, &staged)) + Ok(commit::judge_pending(&body, &staged, &redirects)) } // Both modes and neither are refused above, before this is reached. _ => Ok(Vec::new()), diff --git a/crates/batten/tests/it/commit_admission.rs b/crates/batten/tests/it/commit_admission.rs index dcdd47842..830729475 100644 --- a/crates/batten/tests/it/commit_admission.rs +++ b/crates/batten/tests/it/commit_admission.rs @@ -41,6 +41,10 @@ use common::{Fixture, git_in, run, stdout, write}; const GUARDED: &str = "guarded.toml"; const ORDINARY: &str = "notes.md"; +/// A protected path that ALSO has a `[[redirect]]` naming its sanctioned +/// mutation — the CLOUD-1303 set. +const SURFACED: &str = "memories/note.md"; + /// A fixture protecting one path, with a base commit that predates every case. /// /// `[commit]` is present because `commit check` refuses a config without one @@ -59,6 +63,29 @@ fn fixture(name: &str) -> PathBuf { .build() } +/// The same fixture, plus a second protected path that a `[[redirect]]` speaks +/// for (CLOUD-1303). +/// +/// **BOTH paths are protected and only one is redirected**, which is what makes +/// the third case below able to fail. A fixture where the redirect covered the +/// whole protected set would pass an implementation that dropped the demand +/// entirely. +fn fixture_with_redirect(name: &str) -> PathBuf { + Fixture::new(name) + .config(&format!( + "version = 1\nprotected = [\"{GUARDED}\", \"memories/**\"]\n\n\ + [[redirect]]\nglob = \"memories/**\"\n\ + mutation = \"use the memory tools\"\n\n\ + [commit]\nsubject_pattern = \"^(feat|fix|chore)(\\\\(.+\\\\))?!?: .+\"\n" + )) + .file(GUARDED, "original = 1\n") + .file(SURFACED, "original\n") + .file(ORDINARY, "just notes\n") + .git() + .base_commit() + .build() +} + /// Commit everything staged in `dir` with `message`, and return the range that /// covers exactly that commit. /// @@ -255,6 +282,78 @@ fn a_deleted_protected_path_owes_an_articulation_too() { ); } +#[test] +fn a_protected_path_with_a_sanctioned_mutation_owes_no_block() { + // CLOUD-1303, and the whole of it. A write through the surface the class + // declares is refused by nothing, so no admission is issued and there is + // nothing to articulate — while the override route's own precondition says it + // exists for when "writing the protected path directly is the only route + // left". Demanding a block here leaves an honest author a choice between a + // false articulation and not committing. + let dir = fixture_with_redirect("commit-admits-surfaced"); + write(&dir, SURFACED, "changed\n"); + let range = commit(&dir, "chore(memory): record something"); + let (code, report) = check(&dir, &range); + assert_eq!( + code, + Some(0), + "a path with a declared mutation surface owes no articulation: {report}" + ); +} + +#[test] +fn a_tampered_block_on_a_redirected_path_is_still_refused() { + // THE HALF THE SUPERSEDED FIX DROPPED, and the reason CLOUD-1303 refuted it. + // Exempting the path before inspecting it loses `admits-tampered` for exactly + // the exempted set: a doctored block on a memory would go unexamined. The + // redirect answers only the ABSENCE case; a block that claims the path is + // still verified. + let dir = fixture_with_redirect("commit-admits-surfaced-tampered"); + write(&dir, SURFACED, "changed\n"); + let block = articulate(&dir, SURFACED); + let doctored = block.replace( + "the owning surface cannot express it", + "a reason nobody articulated", + ); + assert_ne!(doctored, block, "the case must actually change the answer"); + let range = commit( + &dir, + &format!("chore(memory): record something\n\n{doctored}"), + ); + let (code, report) = check(&dir, &range); + assert_eq!( + code, + Some(2), + "a doctored block is refused whether or not the path has a surface: {report}" + ); + assert!( + report.contains(&format!("admits-tampered {SURFACED}")), + "the tampered finding keeps its whole subject set: {report}" + ); +} + +#[test] +fn a_redirect_for_one_path_does_not_excuse_another() { + // Without this, an implementation that dropped the demand for every protected + // path the moment ANY redirect was declared would pass the case above, and the + // clause would be off. The commit writes both; only the redirected one is + // excused. + let dir = fixture_with_redirect("commit-admits-surfaced-narrow"); + write(&dir, SURFACED, "changed\n"); + write(&dir, GUARDED, "original = 2\n"); + let range = commit(&dir, "chore(config): change both"); + let (code, report) = check(&dir, &range); + assert_eq!(code, Some(2), "the unredirected path still owes: {report}"); + assert!( + report.contains(&format!("admits {GUARDED}")), + "the finding names the path with no sanctioned surface: {report}" + ); + assert!( + !report.contains(SURFACED), + "the redirected path is not reported: {report}" + ); +} + #[test] fn the_clause_needs_no_store_to_decide() { // THE PROPERTY THAT MAKES THIS A CI TIER RATHER THAN A LOCAL ONE. The block From 83a9367d1b797c49beb62de106af533a787aaa82 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Fri, 11 Sep 2026 01:32:56 +0000 Subject: [PATCH 11/18] docs(memory): record the landing architecture, the ADR shape, and what counts as evidence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three new memories and three updated ones, none of which existed anywhere but a context window that dies with the container. `evidence-hierarchy` is the rule that would have prevented two errors in one session. Code and tests as they execute now, and the output of a command I ran myself, are evidence. Doc comments, the board, PR bodies and prior handoffs are claims of equal standing: comments are written by the same agents who write the board, reviewed no harder, and go stale the same way. Measured: 5 of 13 audited speculation and lease rows are wrong, and a row or comment citing a retired artefact is wrong 4 times in 5. `decision/landing-architecture` carries the design. The fast-forward sha-preservation guarantee, and that the holder does NOT land by rebase — #934's body is named as the source of that error, because it will be read again. The pipeline model: serial metered trunk, parallel free agents, speculation as pipelining rather than opportunism, and the three metrics nothing measures. Why k=1. Why CAS and not Paxos, with openraft and raft-rs named as considered-and-declined so nobody reopens it cold. Fencing rather than voting, because no failure detector is accurate and the design must be safe under a false suspicion. Log-as-branch, the priority queue and its starvation caveat, the identity tuple, and transcript preservation as parentless orphans. `decision/adr-process` records the shape: one file per decision, rewritten in place, git blame as the amendment history, no supersession markers and no version suffixes — a reader sees only the current correct state of the world. The updates: `github-access` gains the leak form that bit this session, the fact that only mise is wrapped while the engine and git run unfenced, and the measured stale-binary instance. `workflow/landing-loop` demotes the lease's "it is a BRANCH" premise from settled fact to an open question with the experiment named. `core` routes the three new ones. Refs: CLOUD-1778 --- .serena/memories/core.md | 8 + .serena/memories/decision/adr-process.md | 41 +++ .../memories/decision/landing-architecture.md | 249 ++++++++++++++++++ .serena/memories/evidence-hierarchy.md | 68 +++++ .serena/memories/github-access.md | 67 +++++ .serena/memories/workflow/landing-loop.md | 27 +- 6 files changed, 456 insertions(+), 4 deletions(-) create mode 100644 .serena/memories/decision/adr-process.md create mode 100644 .serena/memories/decision/landing-architecture.md create mode 100644 .serena/memories/evidence-hierarchy.md diff --git a/.serena/memories/core.md b/.serena/memories/core.md index 08f98bed6..60c3b7c25 100644 --- a/.serena/memories/core.md +++ b/.serena/memories/core.md @@ -11,6 +11,14 @@ This memory is the graph root: every other memory is reached from here, and the trigger for each is stated here rather than inside it (`mem:memory_maintenance`). Read on demand, never all of them. +- `mem:evidence-hierarchy` — **before acting on anything a doc comment, a + `CLOUD-*` row, a PR body or a handoff asserts**, and before citing one as the + reason for a decision. The board audited at ~38% wrong; comments are no better. +- `mem:decision/adr-process` — recording a decision; about to add a `status:`, + `superseded-by:` or version suffix to any document. +- `mem:decision/landing-architecture` — touching `land.rs`, `lease.rs`, + `speculation.rs`, `pipeline.rs` or the landing workflows; any row about the + lease, the lap, speculation, eviction or landing throughput. - `mem:workflow/board-states` — starting or finishing a `CLOUD-*` issue; reasoning about what is in flight. - `mem:workflow/agent-fanout` — spawning a subagent, or running more than one diff --git a/.serena/memories/decision/adr-process.md b/.serena/memories/decision/adr-process.md new file mode 100644 index 000000000..f718742f5 --- /dev/null +++ b/.serena/memories/decision/adr-process.md @@ -0,0 +1,41 @@ +# Decision records: git is the amendment history + +Read when: recording an architectural decision, or about to add a `status:`, +`superseded-by:`, `-v2`, `-amended` or "see the current version" pointer to any +document in this repository. + +## The rule + +**A reader must see only the current correct state of the world.** + +- **One file per decision, always rewritten in place.** Never a chain. +- **`git log` / `git blame` is the amendment history**, reachable only by + explicitly spelunking. History exists, costs nothing to keep, and is invisible + until asked for. Git already does versioned-document-with-history better than + any convention layered on top. +- **No `status: superseded` field**, because a live file is the only kind there + is. A decision that no longer holds is a file that no longer says it. +- **A gate, or it is not a process**: no decision record may carry a supersession + marker, a version suffix in its filename, or a pointer to another record "for + the current version." + +## Why, measured + +Superseded documents, `-amended` suffixes and a folder of near-duplicates make a +doc tree **actively worse than no doc tree**: a reader cannot tell which file is +live, and an agent will confidently quote the dead one. In one 2026-09-10 session +this shape cost two wrong conclusions before the pattern was named — see +`mem:evidence-hierarchy`. + +## Home + +`.serena/memories/decision/`, beside the operational memories. + +Chosen because it needs **no change to non-negotiable rule 7** (`no-docs-tree` +fails a tracked `docs/` path and sends research to the tracker), and because that +directory is already checked in, already read on demand, and already the surface +a future session loads. Routing is `mem:core`'s existing table — the mechanism in +use, not a second one invented alongside it. + +Writes go through Serena's memory tools; `protected-mutation` denies every other +route, and `rename_memory` is the only one that rewrites `mem:` referrers. diff --git a/.serena/memories/decision/landing-architecture.md b/.serena/memories/decision/landing-architecture.md new file mode 100644 index 000000000..ef7f21f70 --- /dev/null +++ b/.serena/memories/decision/landing-architecture.md @@ -0,0 +1,249 @@ +# The landing system: what it is for, and the decisions that follow + +Read when: touching `land.rs`, `lease.rs`, `speculation.rs`, `pipeline.rs`, the +land/lease region of `lib.rs`, the landing workflows, or any `CLOUD-*` row about +the lease, the lap, speculation, eviction or landing throughput. + +This file is a DECISION RECORD under `mem:decision/adr-process`: it is rewritten +in place to the current truth, carries no supersession markers, and `git blame` +is its amendment history. + +## The guarantee everything hangs off + +**Fast-forward-only linearized trunk. Landing PRESERVES the sha.** + +> CI goes green on head `H`; trunk fast-forwards to `H`; the runs on `H` **are** +> the runs on trunk. **CI never runs twice.** + +**The holder does NOT land by rebase.** PR #934's body and commit `3a18fb5`'s +message both say it does ("minting new ones for the same patches") and both are +wrong; a 2026-09-10 session quoted that approvingly and built a whole fencing +argument on it. A waiter that speculates on `H_A` and sees the holder land `H_A` +unchanged was **exactly right** — speculation converges, it does not churn. + +## What the lock is for + +**The lock is the LINEARIZATION mechanism, not a spend limiter.** Its purpose is +to eliminate contention and keep CI **maximally saturated with only green +matrices on the critical path.** + +The two resources are asymmetric, and this is the whole design pressure: + +- **The trunk is serial and metered.** One branch lands at a time and each + occupies a CI matrix that costs money. (Free public runners today; paid before, + when private; paid again on the bigger runners coming.) +- **Agents are parallel and free.** In-process agent work has no monetary cost, + so **agents are the parallelization lever** precisely because trunk + linearization is the bottleneck. + +So: push the maximum work into the free parallel phase, the minimum into the +metered serial one. An agent must prove — with receipts for everything CI will +run — that its tree is already green **before** admission to a matrix. The matrix +VERIFIES a claim; it does not discover. Agent local prep being ≥ half the landing +wall clock, usually much more, is the design working. + +**Speculation is the pipelining mechanism, not a workaround.** While the holder +occupies the slot with `H_A` in CI, a waiter rebases onto `H_A` — the trunk that +is about to exist — and spends its free local prep there, so the instant the +holder lands, the waiter's matrix starts against an already-proven tree. + +**k = 1, and a semaphore is not the lever.** Widening the slot means k heads +racing for one fast-forward target; every loser's matrix is spent on a head that +can no longer land. Throughput comes from pipelining the slot, not widening it. + +## The metrics that do not exist, and must + +None of the three quantities that define the objective are instrumented: + +1. **Skew** — last green matrix end → next green matrix start. The primary + metric; speculation done right drives it toward zero. +2. **Cost per run in BOTH currencies** — CI minutes (monetary) and wall clock. + They diverge and are optimised differently. +3. **Agent local prep wall clock** — free in money, dominant in time, the phase + the design deliberately loads. Unmeasured, so nobody can tell exhaustive + proving from spinning. + +CLOUD-492 measures divergence-from-linear, which is adjacent and not this. **You +cannot tune a pipeline you cannot see** — every design argument here is a guess +until these exist. + +## The poison cascade, sized correctly + +Because speculation is pipelining, a poisoned holder is not one branch's problem: +**every waiter that prepped against `H_A` spent its slow-but-free phase against a +base that will never exist.** The cost is an emptied pipeline plus N agents' prep +discarded — not a wasted matrix. Hence: detect poison with the fastest oracle, +broadcast it, and make the cooldown proportionate. + +**CI is the oracle for poison, not a waiter's local verify.** A waiter's own red +cannot distinguish "the borrowed base is bad" from "my change is bad" without a +second run. That is why the pre-2026-09-10 design needed a two-lap discrimination +dance, a suspicion surviving a process, and a promotion step — all of it working +around asking the wrong oracle. + +## Consensus: we already have it — do NOT implement Paxos or Raft + +**We are not in the message-passing model.** Each ref is a register, +`--force-with-lease` is an atomic compare-and-swap, fetch is a read. + +- **CAS has consensus number ∞** (Herlihy, _Wait-Free Synchronization_). A single + CAS register solves consensus for any number of processes, wait-free. **The + remote already IS our consensus.** Paxos on top would be a second source of + truth that can disagree with the first. +- **There is no peer transport.** Agents have no inbound addresses; every + "message" is a write to the remote another agent reads. "Gossip" here _is_ the + remote, which is already the serialization point — an eventually-consistent + overlay on a linearizable store trades away the strongest property we have. +- **Quorum members are reclaimable containers**, so quorum loss would be routine + rather than exceptional. Raft over ephemeral agents is a liveness liability. + +This is the Chubby argument (Burrows, OSDI'06): most systems want a +lock/consensus **service** their clients call, not an embedded consensus library. +The forge is our Chubby. `openraft` and TiKV's `raft-rs` were considered and +declined — the reason is the substrate, not their quality. + +## What to build instead: a replicated log on the register + +The missing primitive is not agreement but **total order**. **The log is a branch +whose commits are its entries**; appending is commit-then-CAS-the-tip, retrying on +conflict (lock-free, correct for a fleet this size). + +One construction pays for four things: + +1. **A real monotonic epoch** — the log index. Better than the trunk tip, which + only advances when something lands; the index advances on every fleet event, + so bets, evictions and cooldowns can be fenced against activity too. +2. **The queue, with agreement by construction.** Everyone reads the same + sequence, so everyone computes the same order. No votes, no round trips. +3. **Front-of-queue skips as a declared priority class.** `release` and `human` + (the manual no-agent fast-forward) sort ahead of `agent`; FIFO by log index + within a class. The release path stops being a special case that races the + fleet. **Starvation must be designed against** — strict class priority starves + `agent` under sustained release load, so this needs aging or a reserved share, + declared rather than discovered. +4. **An audit trail** — ordered, attributable entries. + +## Eviction: fencing, not voting + +**No failure detector is accurate in an asynchronous system.** FLP kills +consensus with one crash fault under message passing; the CAS register sidesteps +that for _agreement_, but nothing sidesteps _detection_. Every suspicion of a +dead holder is a heuristic, and a vote does not make a wrong suspicion right — it +makes several agents confidently wrong together. + +So the design must be **safe under a false suspicion**, which fencing gives and +voting does not: eviction is a CAS advancing the lease's fencing token. The +evicted holder is not asked and need not be reachable; its next write presents a +stale token and is refused, so **a wrongly-evicted holder cannot corrupt anything +— it loses and re-laps.** (Kleppmann's fencing argument.) + +Therefore the eviction notice is **advisory** (a peer's request the holder may +honour early and cheaply) while the eviction itself is **unilateral and fenced** +(the referee's CAS). Both halves, each doing the job it is sound for. + +## The referee is a declared ROLE, not a named platform + +Non-negotiable rule 1: batten adjudicates for six harnesses (`Harness`, +`crates/batten/src/hook.rs`) and runs on agent containers, CI runners and +developer machines. So **"a GitHub Action" must not appear in `crates/batten`.** +The core defines _an executor whose binary version is attested_ and _which +decisions require one_; a consumer's `batten.toml` names the instantiation. + +In this repository that instantiation already half-exists: five workflows run +`batten lease guard` from a binary installed **from trunk**, not from the PR. It +reads shared state and cancels unauthorised matrices; it never writes. Extending +it to write is the referee. + +**A consumer with no such executor must degrade honestly** — fall back to +epoch-and-TTL expiry, weaker but not wrong. A mandatory referee in the core is a +consumer fact smuggled into the agnostic half. + +Same discipline: the epoch is the **declared trunk's** tip, not `main`'s; the CAS +address is a ref on the declared remote; and the forge behaviours the lock leans +on (atomic ref update, fast-forward enforcement) are **declared capabilities, not +assumptions compiled in.** + +## Identity: the tuple, because a branch name is not an identity + +`host-pid-nonce` dies with the process, and **branch names get reused**. The +stable identity carried IN each entry — never derived from live state that may +have moved — is: + +``` +{account, session-id, session-name, session-url, pr-number, branch, + batten-version, epoch} +``` + +Why each: **pr-number** because a branch name alone cannot be matched to its pull +request after the fact, and an audit trail is about what was true then; +**session-url** so something clickable leads to the harness session that wrote +the entry; **session-name** so it is findable in the GUI; **account** so you know +**which human to hold responsible for a misbehaving agent cluster** — on a fleet +with a world-writable lock, the operationally important question; +**batten-version** so skew is visible rather than inferred. + +**Unresolved and must be settled before designing this in:** which commit-metadata +surface may carry it. `[attribution] identity_deny` governs committer/author and +non-negotiable rule 8 refuses a harness identity request there; `trailer_deny` +forbids `^Claude-Session:` and `claude.ai/code` outright. Neutrality is about _who +is credited_; blame attribution is a different requirement and may need its own +declared surface. Settle against `rules/commits.md` and +`no-denied-identity-prescribed`, not after a gate refuses it. + +## Transcript preservation: `refs/sessions/` + +A redacted transcript per session, from every harness, so agents can investigate +**clusters of repo misbehaviour** without touching code or the landing refs. + +**Shape: a parentless orphan per transcript, content-addressed.** No parent, no +children, created by a plain push — the same test-and-set the lock uses, except +the address IS the content hash, so there is one writer per address by +construction and no contention at all. + +**Inertness is structural, not conventional.** A parentless commit can never +_become_ an ancestor of trunk: it cannot be fast-forwarded into anything and +nothing can descend from it. Keeping it out of the lap's fetch path is then a +performance concern, not a correctness one. + +**Recanting is cheap**: delete the ref and the object is unreachable and +collectable, with nothing orphaned. So **the risk is what has already been read, +not permanence** — which is why fail-closed redaction is a strong default rather +than an absolute bar. + +**Residual risk is not uniform**, and this is the part to design to: + +- **A GitHub credential leaked through GitHub is the safest case** — GitHub scans + its own token formats and auto-revokes its own disclosed credentials. It is + self-defending. +- **Third-party credentials are the actual residual**, because nothing revokes + them. Near-zero in this environment by construction (MCP is credentialless, no + other system access) — but **batten runs across many environments**, so the + core must not assume one where the only credential self-revokes. The redactor's + patterns are a consumer fact; failing closed is the agnostic rule. + +Already built: CLOUD-651 (Done) is the collector; `ripsecrets` is provisioned; +CLOUD-59 (Done) added the `secrets` rule kind. **Durable publication is the +missing half.** + +## The open questions, stated rather than assumed + +1. Which commit-metadata surface carries the blame identity (above). +2. **Log compaction** — an append-only branch grows without bound and every agent + fetches it. Needs snapshot-and-truncate with a measured ceiling, or it becomes + the bandwidth defect the design already warns about. +3. **Starvation policy** for the priority queue — aging, or a reserved share. +4. **Whether CAS addresses must live under `refs/heads`** — untested, and the + claim traces to a misdiagnosed credential failure. See + `mem:evidence-hierarchy` and CLOUD-416. + +## Deutsch's fallacies, as commitments to check against + +| Fallacy | Commitment | +| ----------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| The network is reliable | Every read fails open; no fleet-binding decision rests on one failed read; every write is CAS-or-retry. | +| Latency is zero | **No wall clocks anywhere** — counts and supplied instants only. The epoch design keeps this by making expiry an EVENT, not a duration. | +| Bandwidth / transport is free | The sharded and log designs make this WORSE and must pay for it: ref advertisement is O(refs), and a 79,973-byte advertisement is already on record. Reaper plus a measured ceiling, or the fix is the next defect. | +| The network is secure | World-writable by construction; the threat is accident, not malice. Trunk stays sacred: the referee re-derives rather than trusting a request. | +| Topology doesn't change | Agents vanish without deregistering. Membership is derived from observation with expiry, never from a registry needing clean exit. | +| There is one administrator | **False by construction** — agents run different binary versions. Every ref body carries a schema major; readers ignore unknown keys, reject an unknown major, and ship a release before writers. | +| The network is homogeneous | Six harnesses, three environment classes. No environment's quirk may reach `crates/batten` (rule 1). | diff --git a/.serena/memories/evidence-hierarchy.md b/.serena/memories/evidence-hierarchy.md new file mode 100644 index 000000000..c6762bbe6 --- /dev/null +++ b/.serena/memories/evidence-hierarchy.md @@ -0,0 +1,68 @@ +# What counts as evidence, and what is only a claim + +Read when: about to act on anything a doc comment, a `CLOUD-*` row, a PR body, a +previous session's handoff, `AGENTS.md` or another memory asserts — and ALWAYS +before citing one as the reason for a design decision. Also read it before +writing a claim into any of those surfaces yourself. + +## The ranking + +1. **Code and tests as they execute right now**, and **the output of a command + you ran and read yourself.** +2. **Everything else is a CLAIM**: doc comments, the board, PR bodies, commit + messages, `AGENTS.md`, these memories. Comments are written by the same agents + who write the board, reviewed no harder, and go stale the same way. +3. **Even (1) goes stale.** A command's output describes the moment it ran, and + the remote is shared and mutable. + +**The operational rule: if a claim is load-bearing, re-derive it from something +executable. If nothing executable exists, THAT ABSENCE IS THE FINDING.** A +constraint worth designing around is worth a test. + +`rules/scanning.md` row five is the same rule one level down: to know what a gate +DECIDES, run it and read the exit code. Reading source to predict a verdict "is +worse than guessing, because it looks like rigour." + +## The board's measured error rate + +**5 of 13** speculation/lease rows audited against the tree on 2026-09-10 were +STALE or REFUTED — ~38%. Rows are written by agents, many running a stale binary +that only ever saw a half-implementation. + +**A cheap discriminator, worth keeping:** a row or comment that cites a **retired +artefact** — a shell variable (`spec_undo`, `spec_base`, `LAND_LOCK_HOLDER_PID`), +a line number in `mise-tasks/land.sh` or `land-lock.sh` — was wrong **4 times in +5**. A row citing live Rust paths was usually at least partly live. It is triage, +not a substitute for reading the tree. + +## Worked examples, so the shape is recognisable + +- **A doc comment that is the foundation of a design and has no test.** + `lease.rs:1310-1315` asserts the agent proxy 403s a push outside `refs/heads`, + and concludes the landing lock must therefore be a branch. No test anywhere + asserts it; `git ls-remote origin` shows `refs/notes` DOES exist on the remote; + and `mem:github-access` measures the real mechanism — proxied, `git` + authenticates with the INJECTED token, which 403s writes it is not scoped for. + Same symptom, different cause. **CLOUD-416 records that this misdiagnosis cost + the lease being implemented four times.** +- **A Done row whose landing claim is false.** CLOUD-1399 (Done) says commit + `46530200` landed the `egress-is-unproxied` `[[startup]]` row. + `git log -S'egress-is-unproxied' --all -- batten.toml` is EMPTY and `46530200` + is not a valid object. The detector (`doctor egress`) landed; the repair never + did. +- **A false premise propagating from a PR body into pushed history and then into + a plan.** PR #934's body and commit `3a18fb5`'s message both state "the holder + lands by rebase, minting new ones for the same patches." **False** — landing is + fast-forward and PRESERVES the sha (`mem:decision/landing-architecture`). A + later session quoted it approvingly and built a fencing argument on it. +- **Two Urgent rows refuted by inspection.** CLOUD-240 reasons over + `mise-tasks/land.sh:162`, a deleted file; CLOUD-1423 says `batten land` has zero + callers, but `land lap` is `mise.toml:3494`. + +## When you find one + +A row the tree refutes goes **back to Backlog with a comment saying what the tree +says instead** — never a note edited into its body, which leaves the false claim +in place above the correction. Then it is not worked until re-filed against +reality. `mem:workflow/board-states`: a state is a claim about the tree, and the +tree wins. diff --git a/.serena/memories/github-access.md b/.serena/memories/github-access.md index 471bf0e64..e8ece4779 100644 --- a/.serena/memories/github-access.md +++ b/.serena/memories/github-access.md @@ -81,10 +81,67 @@ mediated gate permitted for half a session: `no-tool-substitution`, `trailer_deny` trailer reach the remote. **At the hook boundary, refuse and permit are the same byte.** `mise run install:local` is the unblock. +**Measured again 2026-09-10, with the version numbers, because this class recurs.** +Installed binary **0.0.156**, tree at **0.0.159**. `batten.toml` declares +`command_matcher` (commit `4bc4f57`'s key, whose own footer reads +"`handler::Handler` gains a `command_matcher` field") and 0.0.156 cannot parse +it, so `batten startup` refused the **whole config** — every mediated gate +permitting, `egress-is-unproxied` never reachable, `hook-surfaces-are-battens` +never reaped. `mise run install:local` fixed it; the gates went live immediately +and began correctly denying `no-tool-substitution` and `background-redirect` on +calls that had gone unjudged minutes earlier. `contract-drift` then self-reported: +_"this session's SessionStart registration did not run … every mediated call until +it appeared failed open and said nothing."_ + +This is CLOUD-1775's shape and it is **self-referential**: the skew detector is +declared with `command_matcher`, the very key a stale binary cannot parse, so it +is unreachable in exactly the case it exists for. + +**`batten startup --repair` was ALSO needed by hand** in the same session, and +`hook-surfaces-are-battens failed not-provisioned` went to `ok` only after it — +which is what leaves the launcher's own stop hooks firing when batten is supposed +to have zeroed them in place. Needing the repair by hand is the finding, not the +fix. + A repair the host refuses is the ONE ask to put to a human (CLOUD-680's shape). Name the refusal you actually got; never assert which settings key would have granted it unless you measured that key doing something. +## The fence covers `mise` and NOT batten itself — measured 2026-09-10 + +`/root/.local/bin/mise` is a `provision-exec` wrapper whose declared env fences +`github.com` and four sibling hosts out of `NO_PROXY`, sources the token from a +first-set chain including `BATTEN_GITHUB_TOKEN`, carries +`reject_prefix = "proxy-"` to refuse the injected placeholder, and unsets +`HTTPS_PROXY` where the trust store names Anthropic. It is correct and complete. + +**It covers exactly one binary.** `/root/.local/bin/batten` is a bare stripped +ELF, not a wrapper — so the engine, the session shell and `git` all run +un-fenced. Confirmed in-container: `NO_PROXY` carries no `github.com`, while the +PAT reaches GitHub off-proxy (`rate_limit` core **5000**, not 15000). + +`fetch.rs` honouring `*_PROXY` is **deliberate and correct**, not a defect — +CLOUD-1399 argues it explicitly: _"that behaviour is correct and is what lets +batten work on a host where a proxy is mandatory. The defect is a container's +values."_ Only `get_direct` (the credential probe) refuses a proxy, because a +question about a credential has no answer on a route that substitutes its own. + +**But the repair never landed.** CLOUD-1399 is Done and claims commit `46530200` +shipped an `egress-is-unproxied` `[[startup]]` row. +`git log -S'egress-is-unproxied' --all -- batten.toml` is EMPTY and `46530200` is +not a valid object here. `batten doctor egress` correctly answers +`egress failed egress-unfenced` and **nothing repairs it** — non-negotiable rule +2's own words: _a gate that detects what it is wired to fix and waits to be asked +is sensor only._ `BATTEN_ENVIRONMENT=disposable` IS set, so the trigger is not +the problem; the row does not exist. + +CLOUD-1400 is the row for batten owning its own proxy (a decision spike, not +work). CLOUD-1475 replaces the PAT-in-environment entirely with +`repository_dispatch` as an authenticated bus and a webhook listener holding the +secrets — zero bearer in the VM. Until then the standing constraints are: a live +PAT in the environment, rotated frequently; **no access to any other system**; +MCP is credentialless. + ## Why the toolchain runs here (a per-host fence, NOT unsetting the proxy) **`mise` NEEDS BOTH HALVES, AND THE FIRST ONE IS `NO_PROXY`.** Re-measured @@ -302,6 +359,16 @@ dropped), confirm green, and land. just the old SHA dropped when you pushed a new head.) - Never echo a credential. Check presence with `${VAR:+SET}` — never a bare `$VAR` or a `${VAR:-…}` that expands the value into the transcript. +- **`${VAR:+SET}${VAR:-UNSET}` LEAKS THE VALUE, and it reads as safe.** Measured + 2026-09-10: it printed a live `ghp_` PAT into a session transcript. `:-` + substitutes only when the variable is UNSET or empty, so when it is set the + second half expands the credential. The concatenated "report either way" form + is the trap; `${VAR:+SET}` alone is the whole check. +- **Rotate a leaked PAT AFTER the session, never during.** A refreshed PAT can + only reach a session through its context, so rotating mid-session strands the + container with a dead credential and no route to the new one. Note also that a + GitHub credential disclosed _to GitHub_ is the least-bad case: GitHub scans its + own token formats and auto-revokes its own disclosed credentials. ## Transparent TLS interception — tools that carry their own CA roots diff --git a/.serena/memories/workflow/landing-loop.md b/.serena/memories/workflow/landing-loop.md index 48ada8816..7e50c5946 100644 --- a/.serena/memories/workflow/landing-loop.md +++ b/.serena/memories/workflow/landing-loop.md @@ -16,6 +16,15 @@ listed at the bottom with their bypasses so a refusal can be told from a defect. fetch → rebase → `verify` → `verified` → push → `ci-wait` ∥ `main-watch` → `/fast-forward` → read the answer → lap. +**What the lap is FOR, before any of its mechanics: landing is fast-forward and +PRESERVES the sha**, so the runs that went green on the pushed head _are_ the +runs on trunk and CI never runs twice. The lock is therefore the **linearization** +mechanism — it exists to keep CI saturated with only green matrices on the +critical path — and **speculation is pipelining**, not opportunism. The holder +does NOT land by rebase; PR #934's body and commit `3a18fb5`'s message both claim +it does and both are wrong. Full architecture, metrics and the consensus +reasoning: `mem:decision/landing-architecture`. + A refusal is **the design working**, not a failure. Each lap rebases onto a little more landed work, so conflicts arrive one small resolvable increment at a time; batching laps removes no refusal and only makes each one bigger. An agent @@ -52,10 +61,20 @@ with the branch gone (`git ls-remote` confirms). There is nothing to repair here operation, no service, no API. Four things the design was pressure-tested into, each of which cost an incident: -- **It is a BRANCH.** The agent proxy 403s a push outside `refs/heads`, and - GitHub does not enforce the fast-forward rule off `refs/heads` either — a - parentless orphan `PATCH` with `force:false` was _accepted_ on a custom - namespace. The atomicity the design rests on exists only on `refs/heads`. +- **It is a BRANCH — and this premise is UNTESTED and probably a misdiagnosis.** + The claim was: the agent proxy 403s a push outside `refs/heads`, and GitHub does + not enforce the fast-forward rule off `refs/heads` either. **No test anywhere + asserts either half**; the code only _defaults_ unqualified names to + `refs/heads/` (`lease.rs:2845`), which is a naming convenience. `git ls-remote +origin` shows `refs/notes` DOES exist on this remote. And `mem:github-access` + measures the real mechanism: **proxied, `git` authenticates with the INJECTED + token, which 403s any write it is not scoped for** — same symptom, different + cause, and the same root cause as CLOUD-1569. CLOUD-416 (Urgent, never started) + records that this misdiagnosis cost **the lease being implemented four times**, + and CLOUD-416 itself repeats the wrong cause, so its write probe must run + fenced and PAT-authenticated or it will re-measure the credential bug and bake + it in as an environment fact. Treat the namespace question as open; see + `mem:decision/landing-architecture`. - **Renewal is `--force-with-lease=:`**, a true CAS. `PATCH` with `force:false` does not give one. Create stays a plain push, so acquire is an atomic test-and-set. From dc59cbc433a1b14b34bfbfc0f6910b761b7e45d2 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Fri, 11 Sep 2026 01:38:35 +0000 Subject: [PATCH 12/18] fix(lease): the ref-namespace premise was a misdiagnosed credential failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Terms` explained why the lease lives on `refs/heads` with two claims, and only one was true. The false one has shaped the design for four implementations. REFUTED: that this sandbox's git proxy permits writes under `refs/heads` and rejects every other namespace. Measured as a controlled pair — same object, same ref, same push, one variable. With `github.com` fenced out of the proxy and the PAT supplied through a credential helper, a parentless orphan pushed to `refs/sessions/` exits 0. Unfenced, with the harness-injected token, the identical push returns HTTP 403. `refs/batten-probe/` behaved the same way, and both probe refs were deleted fenced. So the 403 is a credential that is unscoped for the write, and the namespace never came into it. CONFIRMED, and it is the reason that survives: the forge does not enforce the fast-forward rule off `refs/heads`, because that same run had its parentless orphan accepted. Stated precisely, because the old text conflated two mechanisms: this is not what makes the CAS work — `--force-with-lease` swaps atomically in any namespace, and a lease renewal is a non-fast-forward update by design — it is what protects everything else that reads this ref as branch-shaped. And off `refs/heads` the same missing rule is a feature rather than a loss. A log branch wants the rule; a content-addressed transcript wants exactly the orphan the rule would refuse, and is inert because it can never become an ancestor of trunk. One property, opposite signs, decided per structure — so the old conclusion was wrong to draw once and reuse everywhere. The refuted sentence is paraphrased rather than quoted, and a case asserts it cannot return in any spelling. A verbatim refuted claim is one a reader lifts out of its correction, which is how this one survived; the first draft of these tests failed on exactly that. CLOUD-416 records the cost: the lease was implemented four times, and two of those passes existed only because the environment lied and nothing said so. Refs: CLOUD-416 --- crates/batten/src/lease.rs | 58 +++++++-- .../tests/it/lease_namespace_premise.rs | 120 ++++++++++++++++++ crates/batten/tests/it/main.rs | 1 + 3 files changed, 171 insertions(+), 8 deletions(-) create mode 100644 crates/batten/tests/it/lease_namespace_premise.rs diff --git a/crates/batten/src/lease.rs b/crates/batten/src/lease.rs index 5f69088ac..895ed5be0 100644 --- a/crates/batten/src/lease.rs +++ b/crates/batten/src/lease.rs @@ -1307,14 +1307,56 @@ pub fn authorises(observed: Option<&Observed>, want: &str, now: i64) -> Authorit /// Where the lease lives, and the bounds the design was pressure-tested into. /// -/// **`refs/heads`, and it is an environment limitation rather than a preference.** -/// A custom namespace is the better home — invisible to a remote branch listing, -/// untouched by a push of every ref, absent from base pickers, and the CAS behaves -/// identically there — but this sandbox proxies git and its write policy refuses -/// any push outside `refs/heads`. GitHub also does not enforce the fast-forward -/// rule off `refs/heads`: a parentless orphan was ACCEPTED on a custom namespace, -/// which is the whole safety property gone. Moving is a one-line change the day -/// the proxy allows it. +/// **`refs/heads`, and the reason is the forge's fast-forward rule — NOT a +/// namespace restriction.** This paragraph used to claim both, and the first half +/// was wrong for two years. +/// +/// # The namespace claim was a misdiagnosed credential failure (CLOUD-416) +/// +/// It asserted that this sandbox's git proxy permits writes under `refs/heads` +/// and rejects every other namespace. **Deliberately paraphrased rather than +/// quoted**: a refuted sentence left sitting here verbatim is one a future reader +/// lifts out of its correction, which is how it survived this long. +/// +/// Measured 2026-09-11 as a controlled pair — same object, same ref, same push, +/// one variable: +/// +/// ```text +/// fenced, PAT via credential helper: push → refs/sessions/ exit 0 +/// unfenced, ambient injected token: push → refs/sessions/ HTTP 403 +/// ``` +/// +/// `refs/batten-probe/` behaved identically. So the 403 is the injected +/// credential being unscoped for the write, and the namespace never came into it +/// — the symptom `mem:github-access` measures and this comment attributed to a +/// proxy write policy. CLOUD-416 records the lease being implemented four times, +/// "two of those passes existed only because the environment lied and nothing said +/// so"; this is the sentence that carried the lie forward. +/// +/// **A custom namespace is therefore available**, which unblocks the per-agent +/// refs and `refs/sessions/` transcripts rather than this one struct. +/// +/// # The fast-forward claim is the one that survived, and it is the real reason +/// +/// The same run confirmed the other half: a **parentless orphan was ACCEPTED** on +/// `refs/sessions/`, so the forge does not enforce the fast-forward rule off +/// `refs/heads`. That is what keeps the lease here. +/// +/// Read precisely, because the old text conflated two mechanisms. It is not what +/// makes the CAS work — `--force-with-lease` sends the expected old value and the +/// forge swaps atomically, in any namespace, and a lease RENEWAL is a non-fast- +/// forward update by design. What the rule protects is everything else that reads +/// this ref as a branch-shaped thing, and the blast radius of a bug that writes a +/// wrong object here. +/// +/// **And off `refs/heads` that same missing rule is a FEATURE, not a loss.** An +/// append-only log branch wants fast-forward enforcement and belongs here too; a +/// content-addressed transcript wants exactly the parentless orphan the rule would +/// refuse, and is structurally inert precisely because it can never become an +/// ancestor of trunk. One property, opposite signs, decided per structure — which +/// is why the old text's sweeping conclusion — that losing the rule forfeits the +/// safety property outright — was wrong to draw once about the lease and then +/// reuse everywhere. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Terms { /// The remote the lease lives on, as a URL the transport can reach. diff --git a/crates/batten/tests/it/lease_namespace_premise.rs b/crates/batten/tests/it/lease_namespace_premise.rs new file mode 100644 index 000000000..ee6837ae7 --- /dev/null +++ b/crates/batten/tests/it/lease_namespace_premise.rs @@ -0,0 +1,120 @@ +//! The lease's ref-namespace premise is measured, not remembered (CLOUD-416). +//! +//! `lease::Terms` explains why the lease lives on `refs/heads`. For two years the +//! explanation carried two claims and only one was true: +//! +//! 1. *"this sandbox proxies git and its write policy refuses any push outside +//! `refs/heads`"* — **false**. Measured 2026-09-11 as a controlled pair, one +//! variable: the same parentless orphan pushed to the same `refs/sessions/` +//! exits `0` with `github.com` fenced and the PAT supplied through a credential +//! helper, and returns `HTTP 403` unfenced with the harness-injected token. +//! `refs/batten-probe/` behaved identically. The 403 is a credential that +//! is unscoped for the write; the namespace never came into it. +//! 2. *"GitHub also does not enforce the fast-forward rule off `refs/heads`"* — +//! **true**, and the same run confirmed it: the parentless orphan was accepted. +//! +//! # Why this is a test and not a comment +//! +//! Because it was a comment, and the comment was wrong, and the tree believed it. +//! CLOUD-416 records the lease being implemented four times with "two of those +//! passes existed only because the environment lied and nothing said so". A +//! premise that expensive earns an assertion; `rules/scanning.md`'s own rule is +//! that a claim worth designing around is worth something executable, and the +//! absence of one is itself the finding. +//! +//! # WHAT THIS ASSERTS, AND WHAT IT DELIBERATELY DOES NOT +//! +//! It asserts **presence and absence in the prose**: the refuted namespace claim +//! is gone, the surviving fast-forward reason is stated, and the measurement is +//! named so the next reader can re-run it rather than re-derive it. That is the +//! same shape as `scanner_taxonomy.rs` and `spawn_census.rs` — the prose carries +//! the position and the test keeps it from evaporating. +//! +//! It does **not** re-run the probe. A test that pushed to a real remote would +//! need a credential, would write to a shared forge from `cargo test`, and would +//! fail on every host that is not this one — which is the homogeneity fallacy +//! compiled into a test target. The probe is an operator procedure; this is the +//! ratchet that stops its answer being forgotten. + +// Panicking on setup failure is the idiomatic way for a test to fail loudly. +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use crate::common; + +use std::fs; + +use common::at_root; + +/// The module whose doc comment carries the premise. +const LEASE: &str = "crates/batten/src/lease.rs"; + +fn lease_source() -> String { + fs::read_to_string(at_root(LEASE)).expect("the lease module is readable") +} + +#[test] +fn the_refuted_namespace_claim_is_not_stated_anywhere_in_the_lease() { + // THE PREMISE CASE. The sentence below is what sent four implementations down + // the wrong path; if it returns in any spelling, this file is the thing that + // says so. + let source = lease_source(); + for refuted in [ + "refuses any push outside", + "write policy refuses", + "the day the proxy allows it", + ] { + assert!( + !source.contains(refuted), + "{LEASE} states the refuted namespace claim again ({refuted:?}); \ + the 403 is a credential, not a namespace — see CLOUD-416" + ); + } +} + +#[test] +fn the_surviving_reason_is_the_fast_forward_rule() { + // Deleting the false half must not take the true half with it. The lease is on + // `refs/heads` because the forge does not enforce fast-forward off it — and a + // reader who finds no reason at all will move the lease and lose that. + let source = lease_source(); + assert!( + source.contains("fast-forward"), + "{LEASE} must still state the fast-forward rule as the reason the lease \ + lives on refs/heads" + ); + assert!( + source.contains("parentless orphan"), + "{LEASE} must still name the orphan acceptance that measures it" + ); +} + +#[test] +fn the_measurement_is_named_so_it_can_be_rerun() { + // A correction that says only "this was wrong" leaves the next reader with a + // bare assertion to trust, which is the failure one layer over. Both arms of + // the controlled pair have to be legible. + let source = lease_source(); + assert!( + source.contains("403"), + "{LEASE} must record the unfenced arm's status" + ); + assert!( + source.contains("CLOUD-416"), + "{LEASE} must point at the row that owns the misdiagnosis" + ); +} + +#[test] +fn the_correction_does_not_generalise_the_fast_forward_loss_into_a_ban() { + // The old text concluded "which is the whole safety property gone", and that + // conclusion — drawn once about the lease — is what would refuse the log branch + // and the transcript refs if it were reused. One property, opposite signs, + // decided per structure. + let source = lease_source(); + assert!( + !source.contains("the whole safety property gone"), + "{LEASE} must not restate the over-general conclusion: a missing \ + fast-forward rule is a loss for a branch-shaped ref and the enabling \ + property for a content-addressed one" + ); +} diff --git a/crates/batten/tests/it/main.rs b/crates/batten/tests/it/main.rs index a983d312a..75eab3d9d 100644 --- a/crates/batten/tests/it/main.rs +++ b/crates/batten/tests/it/main.rs @@ -165,6 +165,7 @@ mod landed_check; mod landing_roster; mod lease_health; mod lease_lifecycle; +mod lease_namespace_premise; mod lease_precondition; mod lease_record; mod locator_index; From 52fadd4534545ba97cc0d40143d53055cd3f2491 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Fri, 11 Sep 2026 02:04:05 +0000 Subject: [PATCH 13/18] fix(land): a gate refusal over a borrowed base laps instead of blaming this branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two halves of one mechanism. Neither is worth anything alone, which is what the first attempt at this got wrong. `progress_of` gains a `Basis` qualifier — `Own` or `Borrowed` — reaching exactly one cell: the gate's own refusal. The base table keeps `Stop`, which is right for a branch on trunk; `progress`'s reasoning that "the replay and the gate both answer about THIS tree" is exactly right there and exactly wrong over a holder's unlanded commits, where the refusal has two candidate causes and the exit code separates neither. Stopping attributed the failure to the author on no evidence, discarded a prepared branch from the pipeline so the slot freed with nothing ready to take it, burned the agent's remaining tokens, and left the poisoned-base path inert — its own comment says "the NEXT lap settles Poisoned" and behind that cell there was no next lap. The second half is what makes the first honest. `unwind_lap` calls `Bet::forget`, which clears `base`, and `would_rebet` compares `base` — so lapping would have re-borrowed the identical commits, refused for the identical reason, and proved nothing. `Bet::poisoned` is `conflicts`'s twin: a judgement about a COMMIT rather than about this landing, surviving `forget` for the same reason and read by the same function. Written before the progress arms run, since the lap settles the bet. The inverted assertion in `the_same_candidate_is_not_bet_on_twice` is the review's cue that behaviour moved, which is what that pin asked for. Bounded by `Bound::SpeculativeRefusals`, default 2, refunding its lap and charging its own bound. Small where the two neighbours are 60: those count events carrying no information, and one lap here IS the experiment — the next replay goes to trunk and answers with `Basis::Own`, which either stops with real evidence or clears it. The refund is what stops a neighbour's base exhausting an innocent branch's backstop and the exhaustion then being reported against that branch, which is this row's own misattribution arriving one bound lower down. Stated rather than overclaimed: a refusal over a borrowed base has two causes this clone cannot separate — the base is broken, or base and branch are each fine and conflict semantically. The behaviour is correct under both. Under the second the error does not survive, because when the holder lands the next replay puts their change on trunk and the same gate refuses with `Basis::Own`, stopping with the conflict correctly attributed. Worst case is one deferred stop, never a wrong one. `settle` is deliberately unchanged. It answers from the holder and the trunk, and from there a poisoned base is byte-identical to a slow holder; no fourth arm is derivable from inputs that do not include the gate's verdict. The case pinning that is renamed to say so and its assertion is untouched. Closes CLOUD-1306 --- crates/batten/src/land.rs | 220 +++++++++++++++++++++++++++-- crates/batten/src/lib.rs | 98 ++++++++++++- crates/batten/src/speculation.rs | 106 ++++++++++++-- crates/batten/tests/it/land_lap.rs | 8 +- 4 files changed, 404 insertions(+), 28 deletions(-) diff --git a/crates/batten/src/land.rs b/crates/batten/src/land.rs index 50ec0d5de..3c0afc746 100644 --- a/crates/batten/src/land.rs +++ b/crates/batten/src/land.rs @@ -1428,11 +1428,27 @@ impl Step { } } +/// Whose base this lap's tree is sitting on when a step answers. +/// +/// The one fact that decides whether a refused gate is a statement about THIS +/// branch (CLOUD-1306). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Basis { + /// The branch is replayed onto trunk itself. A gate's refusal is about this + /// tree and nothing else. + Own, + /// The branch is replayed onto a lease holder's unlanded commits — "the main + /// that is about to exist". A gate's refusal is about this tree **and** that + /// borrowed base, and nothing in the exit code separates them. + Borrowed, +} + #[must_use] pub const fn progress_of( step: Step, code: crate::exit::ExitCode, seen: Option, + basis: Basis, ) -> Progress { // THE REFUSAL, NOT THE STEP. Keying on `(Wait, Red)` alone reads a wait that // SUCCEEDED as a stop whenever a red reading is in hand — which the driver @@ -1443,6 +1459,39 @@ pub const fn progress_of( { return Progress::Stop; } + // A REFUSED GATE OVER A BORROWED BASE IS NOT YET THIS BRANCH'S ANSWER. + // + // `progress`'s stop cell reasons that "the replay and the gate both answer + // about THIS tree, so a refusal from either is a decision no rebase clears". + // That is exactly right when the branch sits on trunk and exactly wrong when + // it does not: a speculative lap's tree is this branch's commits replayed + // onto a holder's unlanded ones, so a red gate has two candidate causes and + // the exit code distinguishes neither. Stopping attributes the failure to the + // author on no evidence. + // + // The cost of getting it wrong is not one branch's. Under fast-forward + // linearization the waiter's local prep is the free half of the pipeline, and + // a stop here discards a prepared branch — so when the slot frees there is + // nothing proven ready to take it, and the skew that speculation exists to + // drive toward zero opens back up. It also burns the agent's remaining + // tokens, which is the opposite of what this loop is for. + // + // **AND IT MADE THE POISONED-BASE MECHANISM INERT.** That path's own comment + // says "the NEXT lap settles `Poisoned`", and behind this cell there was no + // next lap — a mechanism whose every part existed and whose entry cell voided + // it, which is the dead-gate class this module already shipped once at + // `(Ready | Verify, Internal)`. + // + // LAPPING IS THE DISCRIMINATION, NOT AN EVASION. The next replay drops the + // borrowed base, so the same gate runs again over this branch on trunk — and + // a refusal THERE arrives with `Basis::Own` and stops, carrying the evidence + // the first refusal lacked. That is one extra local gate run, which costs no + // CI matrix at all, to avoid blaming an author for a neighbour's tree. The + // driver bounds the repeat under `Bound::SpeculativeRefusals` so a base that + // keeps refusing cannot lap forever. + if let (Step::Verify, crate::exit::ExitCode::Violation, Basis::Borrowed) = (step, code, basis) { + return Progress::Lap; + } progress(step, code) } @@ -1893,6 +1942,22 @@ pub enum Bound { /// as anything wrong with the branch. The remedy is a shorter gate or a /// quieter trunk, never a re-run. GateReclaims, + /// The gate refused over a borrowed base, repeatedly (CLOUD-1306). + /// + /// Like [`Bound::GateReclaims`] this has spent NO CI — `verify` refuses + /// before the matrix — so a caller reports it as a speculation that did not + /// pay off rather than as a verdict on the branch. **And that is the whole + /// reason it is its own bound rather than a lap charge**: the refusals it + /// counts are the ones nothing has yet attributed to this tree, so spending + /// the general lap budget on them would let a neighbour's poisoned base + /// exhaust an innocent branch and then report the exhaustion against it. + /// + /// The bound is SMALL by design. One lap off the borrowed base is the + /// discrimination — the same gate re-runs on trunk and answers with + /// [`Basis::Own`] — so a second and third refusal buy progressively less. The + /// cost of too few is abandoning a branch whose base merely churned; the cost + /// of too many is re-proving somebody else's defect on this agent's clock. + SpeculativeRefusals, } /// What a charge decided. @@ -1944,6 +2009,8 @@ pub struct Ledger { pub transients: u32, /// Passes whose gate was reclaimed because the base moved under it. pub gate_reclaims: u32, + /// Passes whose gate refused over a base borrowed from a lease holder. + pub speculative_refusals: u32, } impl Ledger { @@ -2020,6 +2087,30 @@ impl Ledger { } } + /// A pass whose gate refused over a borrowed base (CLOUD-1306). + /// + /// [`Ledger::reclaimed`]'s shape, and the refund is honest for the identical + /// reason: `verify` refuses before the matrix, so the pass bought no CI. What + /// differs is what the count MEANS. A reclaimed gate is a fact about the + /// trunk's pace; this is a refusal nothing has yet attributed to anybody, and + /// the next lap — replayed onto trunk rather than onto the holder — is the + /// experiment that attributes it. + /// + /// **Refunding the lap is what stops a neighbour's defect exhausting this + /// branch.** Charged to `laps`, a poisoned base would spend an innocent + /// branch's runaway backstop and the exhaustion would then be reported + /// against the branch, which is the misattribution this whole cell exists to + /// end — one bound lower down instead of one cell higher up. + pub const fn speculative_refusal(&mut self, max: u32) -> Charge { + self.laps = self.laps.saturating_sub(1); + self.speculative_refusals = self.speculative_refusals.saturating_add(1); + if self.speculative_refusals > max { + Charge::Stop(Bound::SpeculativeRefusals) + } else { + Charge::Lap + } + } + /// A run that failed before reaching a verdict. pub const fn transient(&mut self, max: u32) -> Charge { self.laps = self.laps.saturating_sub(1); @@ -2577,7 +2668,7 @@ pub fn rerun_failed(repo: &str, run: &str) -> bool { #[cfg(test)] mod lap_tests { - use super::{Progress, Step, progress}; + use super::{Basis, Progress, Step, progress, progress_of}; use crate::exit::ExitCode::{Internal, Success, Usage, Violation}; /// **The discriminating claim: a refusal a rebase would clear laps, and one @@ -2650,6 +2741,59 @@ mod lap_tests { assert_ne!(progress(Step::Verify, Violation), Progress::Lap); } + /// **THE UNQUALIFIED TABLE IS NOT THE WHOLE ANSWER ANY MORE** (CLOUD-1306). + /// + /// [`progress`] above is the base table and still stops, which is correct for + /// a branch on trunk. [`progress_of`] is the qualified reading the driver + /// actually takes, and over a borrowed base it laps — because the tree that + /// refused is this branch's commits replayed onto a LEASE HOLDER's unlanded + /// ones, so the refusal has two candidate causes and the exit code separates + /// neither. + /// + /// This is deliberately the same shape as the `Wait`/`Red` qualifier beside + /// it: the base table carries the general answer and the qualifier reaches + /// exactly one cell. Keeping the general answer as `Stop` is what makes the + /// premise case above still meaningful. + #[test] + fn a_refused_tree_over_a_borrowed_base_laps_instead_of_blaming_this_branch() { + assert_eq!( + progress_of(Step::Verify, Violation, None, Basis::Borrowed), + Progress::Lap, + "a refusal over somebody else's base is not yet this branch's verdict" + ); + } + + /// The premise for the case above: the qualifier must not swallow the cell. + /// + /// Without this, "stop attributing failures to the author" is implementable as + /// "never stop", which is the dead-gate class — a branch with a genuinely + /// broken tree would lap until its budget ran out and then report exhaustion + /// rather than the defect it actually has. + #[test] + fn a_refused_tree_on_its_own_base_still_stops_under_the_qualifier() { + assert_eq!( + progress_of(Step::Verify, Violation, None, Basis::Own), + Progress::Stop, + "on trunk the gate answers about THIS tree, and no rebase clears it" + ); + } + + /// The qualifier reaches ONE cell, and the neighbours prove it. + /// + /// `Basis` is read on every step, so a qualifier written one match arm too + /// wide would turn a refused replay or a refused ready into a lap — and both + /// of those are statements no rebase clears, exactly as before. + #[test] + fn the_borrowed_base_qualifier_does_not_reach_the_other_stops() { + for step in [Step::Replay, Step::Ready] { + assert_eq!( + progress_of(step, Violation, None, Basis::Borrowed), + progress(step, Violation), + "{step:?} must answer the same whoever's base this is" + ); + } + } + /// **The freshness probe fails OPEN, which is the opposite of every gate in /// this module and is the whole of its correctness.** /// @@ -3237,6 +3381,45 @@ mod tests { assert_eq!(ledger.lease_waits, 1, "and charged to its own bound"); } + /// A speculative refusal is refunded and charged to its own bound + /// (CLOUD-1306). + /// + /// Fails by: charging `laps` instead, which is the misattribution this whole + /// cell exists to end, arriving one bound lower down — a neighbour's poisoned + /// base would spend an innocent branch's runaway backstop and the exhaustion + /// would then be reported against that branch. + #[test] + fn a_speculative_refusal_refunds_its_lap_and_charges_its_own_bound() { + let mut ledger = Ledger::default(); + ledger.attempt(); + assert_eq!(ledger.speculative_refusal(2), Charge::Lap); + assert_eq!(ledger.laps, 0, "the attempt was refunded"); + assert_eq!( + ledger.speculative_refusals, 1, + "and charged to its own bound" + ); + assert_eq!( + ledger.paid, 0, + "verify refuses before the matrix, which is what makes the refund honest" + ); + } + + /// The bound is real, so a base that keeps refusing cannot lap forever. + /// + /// The premise for lapping at all: without a bound, "do not blame the author" + /// becomes an unbounded token burn on somebody else's defect — which is the + /// cost CLOUD-1306 is about, reintroduced by the fix for it. + #[test] + fn enough_speculative_refusals_stop_under_their_own_bound() { + let mut ledger = Ledger::default(); + assert_eq!(ledger.speculative_refusal(1), Charge::Lap); + assert_eq!( + ledger.speculative_refusal(1), + Charge::Stop(Bound::SpeculativeRefusals), + "past the bound the laps are re-proving a neighbour's defect" + ); + } + /// **A RECLAIMED GATE IS THE SAME CLASS, AND THE RACE MADE IT COMMON** /// (CLOUD-1586). /// @@ -3576,22 +3759,30 @@ mod tests { use crate::exit::ExitCode::Violation; assert_eq!( - progress_of(Step::Wait, Violation, Some(TapVerdict::Red)), + progress_of(Step::Wait, Violation, Some(TapVerdict::Red), Basis::Own), Progress::Stop, "no rebase clears a failing test" ); assert_eq!( - progress_of(Step::Wait, Violation, None), + progress_of(Step::Wait, Violation, None, Basis::Own), Progress::Lap, "the staleness arm won the race, and the next replay is the remedy" ); } - /// The qualifier reaches ONE cell and leaves the table alone otherwise — - /// without this, a validator that stopped everything would satisfy the case + /// The qualifiers reach TWO cells and leave the table alone otherwise — + /// without this, a validator that stopped everything would satisfy the cases /// above. + /// + /// **`Basis::Own` IS THE WHOLE SWEEP, AND THAT IS THE STRONGEST THING THIS + /// CASE SAYS** (CLOUD-1306). A branch on trunk reads exactly as it did before + /// the borrowed-base qualifier existed, for every step, every code and every + /// reading — so the change cannot have moved a cell for the ordinary, + /// non-speculating landing. The one borrowed-base cell is asserted by its own + /// case rather than carved out of this sweep, which is what keeps the + /// exception legible instead of hidden in a predicate here. #[test] - fn the_reading_qualifies_the_wait_row_and_nothing_else() { + fn the_reading_qualifies_two_cells_and_nothing_else() { use crate::exit::ExitCode::{Internal, Success, Usage, Violation}; for step in [ @@ -3605,19 +3796,30 @@ mod tests { for code in [Success, Usage, Violation, Internal] { for seen in [None, Some(TapVerdict::Green), Some(TapVerdict::Pending)] { assert_eq!( - progress_of(step, code, seen), + progress_of(step, code, seen, Basis::Own), progress(step, code), - "{step:?}/{code:?}/{seen:?} must read as the table does" + "{step:?}/{code:?}/{seen:?} on its own base must read as the table does" ); } // And the red reading moves only the wait's own refusal. if !(step == Step::Wait && code == Violation) { assert_eq!( - progress_of(step, code, Some(TapVerdict::Red)), + progress_of(step, code, Some(TapVerdict::Red), Basis::Own), progress(step, code), "{step:?}/{code:?} is not the wait's refusal" ); } + // The borrowed base moves the gate's refusal and NOTHING else, so + // every other cell answers identically whoever's base it is. + if !(step == Step::Verify && code == Violation) { + for seen in [None, Some(TapVerdict::Green), Some(TapVerdict::Red)] { + assert_eq!( + progress_of(step, code, seen, Basis::Borrowed), + progress_of(step, code, seen, Basis::Own), + "{step:?}/{code:?}/{seen:?} is not the gate's refusal" + ); + } + } } } } diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 9a890e1e0..da3af899a 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -6820,8 +6820,25 @@ fn base_moved( fn charge_the_lap( step: land::Step, code: ExitCode, + basis: land::Basis, ledger: &mut land::Ledger, ) -> Option { + // THE GATE REFUSED OVER SOMEBODY ELSE'S BASE (CLOUD-1306). Charged to its own + // bound for `reclaimed`'s reason and one of its own: the pass bought no matrix + // — `verify` refuses before the ready — and the refusal is not yet attributed + // to anybody, so spending this branch's runaway backstop on it would let a + // neighbour's poisoned base exhaust an innocent branch and report the + // exhaustion against it. + // + // The pair identifies it without a second channel, exactly as the reclaim arm + // below does: `Violation` is the gate's own refusal, and `Basis::Borrowed` is + // the only reason `progress_of` turned that refusal into a lap at all. + if step == land::Step::Verify && code == ExitCode::Violation && basis == land::Basis::Borrowed { + return match ledger.speculative_refusal(speculative_refusal_bound()) { + land::Charge::Lap => None, + land::Charge::Stop(bound) => Some(bound), + }; + } // THE RECLAIMED GATE, and it is charged to its own bound for `waited`'s // reason (CLOUD-1586). `verify_raced` aborts the gate when the base moves, // so this lap bought nothing — no matrix, no completed gate, no push — and @@ -6844,6 +6861,27 @@ fn charge_the_lap( } } +/// How many gate refusals over a borrowed base a landing absorbs before it stops. +/// +/// **DELIBERATELY SMALL, and the two neighbours above are not the precedent.** +/// `lease_wait_bound` and `gate_reclaim_bound` default to 60 because what they +/// wait out is other branches landing, and the thing they are counting carries no +/// information. This one does: the very next lap replays onto trunk instead of +/// onto the holder, so the same gate re-runs with the borrowed base gone and +/// answers with [`land::Basis::Own`] — which either stops the loop with real +/// evidence or clears it outright. +/// +/// One lap is therefore the experiment, and the default buys a second in case the +/// first also drew a holder. Past that the laps are re-proving somebody else's +/// defect on this agent's clock, which is the token burn CLOUD-1306 is about. +fn speculative_refusal_bound() -> u32 { + std::env::var("LAND_MAX_SPECULATIVE_REFUSALS") + .ok() + .and_then(|raw| raw.trim().parse::().ok()) + .filter(|refusals| *refusals > 0) + .unwrap_or(2) +} + /// How many reclaimed gates a landing absorbs before it stops. /// /// **`lease_wait_bound`'s default, on `lease_wait_bound`'s reasoning.** A lap @@ -7323,7 +7361,18 @@ fn run_land_lap( entered.push(step); } note_the_push(step, code, &mut bet); - match land::progress_of(step, code, seen) { + // READ HERE, ONCE, AND BEFORE ANY OF THE ARMS BELOW CAN MOVE IT. The + // bet is what makes this lap's tree speculative, and `unwind_lap` on + // the lap arm can settle or forget it — so a second read inside the + // charge would ask about a different world than the one that answered + // (CLOUD-1306). + let basis = if bet.live() { + land::Basis::Borrowed + } else { + land::Basis::Own + }; + note_the_poison(step, code, basis, &mut bet); + match land::progress_of(step, code, seen, basis) { land::Progress::Proceed => {} // `None` is not merged, or nobody could say. Either way this is // a lap rather than a retirement — see `landed_for_real`. @@ -7355,7 +7404,7 @@ fn run_land_lap( // `refusal` rather than shadowing `code`: the STEP's code is // what tells a reclaimed gate from a lease wait, so the // charge needs it and a shadow would hide it (CLOUD-1586). - if let Some(refusal) = charge_or_refuse(step, code, &mut ledger, err)? { + if let Some(refusal) = charge_or_refuse(step, code, basis, &mut ledger, err)? { unwind_lap(root, branch, &pipeline, &mut entered, seen, out, err)?; return Ok(refusal); } @@ -7804,10 +7853,11 @@ fn bet_liveness(root: &Path, branch: &str, base: &str) -> speculation::Live { fn charge_or_refuse( step: land::Step, code: ExitCode, + basis: land::Basis, ledger: &mut land::Ledger, err: &mut dyn Write, ) -> Result> { - let Some(bound) = charge_the_lap(step, code, ledger) else { + let Some(bound) = charge_the_lap(step, code, basis, ledger) else { return Ok(None); }; // NO CATCH-ALL, and clippy is what insisted: the match is total inside this @@ -7826,6 +7876,13 @@ fn charge_or_refuse( land::Bound::Transients => { "CI kept failing before reaching a verdict, so the provisioning path is broken rather than flaky" } + // NAMES THE BORROWED BASE AS THE SUSPECT, and says what to do about it. + // The whole point of lapping here was to stop attributing a neighbour's + // refusal to this author; a diagnosis that said "the gate refused" would + // give the attribution back in the last sentence. + land::Bound::SpeculativeRefusals => { + "the gate kept refusing over a base borrowed from the branch ahead, and never over this branch on trunk — so the evidence points at that base rather than at this tree; re-run once the holder has landed or been evicted" + } }; writeln!( err, @@ -7851,6 +7908,41 @@ fn note_the_push(step: land::Step, code: ExitCode, bet: &mut speculation::Bet) { } } +/// Record that the gate refused over the base this lap borrowed (CLOUD-1306). +/// +/// **THE WRITER WITHOUT WHICH THE LAP IS A SPIN.** `land::Basis::Borrowed` turns +/// a refused gate into a lap rather than a stop, on the argument that the next +/// lap re-runs the same gate with the borrowed base gone. That argument is only +/// true if the next lap declines to borrow the same base — and `unwind_lap` calls +/// `Bet::forget`, which clears `base`, and `would_rebet` compares `base`. So +/// without this line the next lap bets on the identical holder, the gate refuses +/// for the identical reason, and the loop proves nothing until its bound stops +/// it. `speculation.rs` calls that arm "CLOUD-1306's other half". +/// +/// **Written BEFORE the progress arms run, beside `note_the_push` and for its +/// reason**: the lap arm's `unwind_lap` settles or forgets the bet, so a writer +/// downstream of it would be recording against a bet that no longer names the +/// base that failed. `Bet::forget` deliberately preserves this field, exactly as +/// it preserves `conflicts`. +/// +/// Keyed on the pair the same way the charge is: `Violation` is the gate's own +/// refusal (`Internal` is `Refusal::Moved`, which is the base moving and says +/// nothing about the base's tree), and `Basis::Borrowed` is what makes the +/// refusal somebody else's candidate rather than this branch's verdict. +fn note_the_poison( + step: land::Step, + code: ExitCode, + basis: land::Basis, + bet: &mut speculation::Bet, +) { + if step == land::Step::Verify && code == ExitCode::Violation && basis == land::Basis::Borrowed { + // The BORROWED base, never the head: the head is this branch's replayed + // commits and is minted fresh every lap, so recording it would name + // something no later lap can recognise. + bet.poisoned = bet.published().map(str::to_owned); + } +} + /// Refresh the trunk's remote-tracking ref before a speculation reads it. /// /// **EVERY SPECULATION READ OF THE TRUNK WAS ONE LAP BEHIND** (CLOUD-1620's diff --git a/crates/batten/src/speculation.rs b/crates/batten/src/speculation.rs index 3400636b9..03541580d 100644 --- a/crates/batten/src/speculation.rs +++ b/crates/batten/src/speculation.rs @@ -168,6 +168,51 @@ pub struct Bet { /// it, and reading it as "no more bets at all" would give up speculating for /// the rest of the landing over one bad candidate. pub conflicts: Option, + /// The holder's base whose tree was REFUSED by the gate (CLOUD-1306). + /// + /// **[`Bet::conflicts`]'s twin, and for the identical reason one field up.** + /// That one records a base known to conflict with this branch; this one + /// records a base known to poison it — the replay succeeded, the tree is well + /// formed, and `verify` refused it. Both are judgements about a COMMIT rather + /// than about this landing, so both survive [`Bet::forget`] and both are read + /// by [`Bet::would_rebet`]. + /// + /// **WITHOUT IT, LAPPING ON A POISONED BASE IS A SPIN RATHER THAN A + /// DISCRIMINATION.** `land::Basis::Borrowed` turns a refused gate into a lap + /// so the failure is not blamed on this author, and the whole argument for + /// that is that the NEXT lap re-runs the same gate with the borrowed base + /// gone. The lap unwinds and calls `forget`, which clears `base` — and + /// `would_rebet` compares `base`, so a forgotten bet answered `true` for the + /// same holder and the next lap borrowed the identical poisoned commits. The + /// gate would then refuse again, for the same reason, having proved nothing, + /// until the bound stopped it. `the_same_candidate_is_not_bet_on_twice` + /// records that arm as *"CLOUD-1306's other half"*, and it is the half that + /// makes the first half worth anything. + /// + /// An `Option` rather than a `bool`, for `conflicts`'s stated reason: + /// a flag cannot tell the holder that poisoned this branch from the one that + /// replaced it, so reading it as "no more bets at all" would give up + /// speculating for the rest of the landing over one bad candidate — and + /// speculation is the pipelining, so giving it up is the throughput loss this + /// mechanism exists to avoid. + /// + /// # "Poisoned" names the PAIR, and the field is right for both causes + /// + /// A gate refusing over a borrowed base has two candidate causes and this + /// clone can distinguish neither from here: the base is genuinely broken, or + /// the base and this branch are each fine and conflict semantically. The name + /// leans toward the first; the behaviour is correct under both, which is why + /// the field is a pair judgement — *this base, with these commits* — rather + /// than a verdict on the base alone. + /// + /// Under the second cause, declining to re-borrow is still right, and the + /// error does not survive: when the holder lands, the next replay puts their + /// change on trunk, the same gate refuses with [`crate::land::Basis::Own`], + /// and the loop STOPS with the conflict correctly attributed to a tree this + /// branch is now actually responsible for. So the worst case is one deferred + /// stop rather than a wrong one, and nothing here asserts a verdict about + /// somebody else's commits that this clone has not earned. + pub poisoned: Option, /// This landing has already declined to publish a speculation (CLOUD-1681). /// /// **THE TERMINATION HALF, and without it the precheck is a spin.** The @@ -226,6 +271,15 @@ impl Bet { if self.conflicts.as_deref() == Some(candidate) { return false; } + // AND A BASE THAT POISONED THIS BRANCH IS NOT BORROWED AGAIN + // (CLOUD-1306). Same shape as the conflict arm above and the same + // argument: the judgement is about that COMMIT, it stays true of it after + // the bet is forgotten, and re-betting would spend the next lap proving + // it a second time. Declining sends the lap onto trunk, which is the + // experiment `land::Basis` laps in order to run. + if self.poisoned.as_deref() == Some(candidate) { + return false; + } self.base.as_deref() != Some(candidate) } @@ -538,6 +592,7 @@ mod tests { recovered: true, pushed: true, conflicts: Some(String::from("feedface")), + poisoned: Some(String::from("deadbeef")), declined: true, }; bet.forget(); @@ -735,19 +790,23 @@ mod tests { assert_eq!(settle(&adopted, Some(MOVED), false, Live::No), Settle::Lost); } - /// **CLOUD-1306, PORTED AS-IS AND PINNED SO IT CANNOT BE FIXED BY ACCIDENT.** + /// **`settle` STILL CANNOT SEE A POISONED BASE, AND THAT STAYS TRUE.** /// - /// A poisoned base — one whose tree will never pass `verify` — is - /// byte-identical here to a holder that is simply slow: the lease is held, - /// `main` has not moved, so `settle` says pending and the waiter sits. This - /// case asserts that reading rather than the one a fixed version would give, - /// because a port that quietly improved behaviour could not be shown to - /// conserve it. + /// This case was pinned as CLOUD-1306's defect, with a note that its changing + /// would be the review's cue that behaviour moved. Behaviour has moved and + /// this case has NOT changed, which is the honest outcome rather than a + /// missed update: `settle` answers "is the bet won, lost or outstanding" from + /// the holder and the trunk, and from there a base that will never pass + /// `verify` is byte-identical to a holder that is merely slow. No fourth arm + /// can be derived from these inputs, because the discriminating fact — the + /// gate's verdict on the borrowed tree — is not among them. /// - /// When CLOUD-1306 lands, this case is the one that must change, and its - /// changing is the review's cue that behaviour moved. + /// The fix lives where that fact exists: the lap records [`Bet::poisoned`] + /// when the gate refuses over a borrowed base, and [`Bet::would_rebet`] + /// declines it afterwards. So this reading is conserved and the mechanism + /// sits beside it rather than inside it. #[test] - fn a_poisoned_base_is_conserved_as_pending_because_cloud_1306_owns_the_fix() { + fn settle_cannot_tell_a_poisoned_base_from_a_slow_holder() { assert_eq!( settle(&placed(), Some(MAIN), false, Live::Yes), Settle::Pending, @@ -756,15 +815,36 @@ mod tests { ); } - /// One outstanding bet at a time. + /// One outstanding bet at a time — and never twice on a base that poisoned us. + /// + /// **THE LAST ASSERTION INVERTED, WHICH IS CLOUD-1306's OTHER HALF LANDING.** + /// It used to assert that a forgotten bet re-bets on the same holder, and + /// named that as the defect. It was load-bearing for the defect and is now + /// load-bearing for the fix: `land::Basis::Borrowed` laps a refused gate on + /// the argument that the next lap runs without the borrowed base, and a + /// forgotten bet that re-borrowed the same commits would make that argument + /// false and the lap a spin. #[test] fn the_same_candidate_is_not_bet_on_twice() { let bet = placed(); assert!(!bet.would_rebet(HOLDER), "already the outstanding bet"); assert!(bet.would_rebet(MOVED), "a different candidate is a new bet"); + + // A FORGOTTEN BET STILL DECLINES THE BASE THAT POISONED IT. `forget` + // clears `base`, so this arm is reached only because `poisoned` survives + // it — which is the whole reason that field is a judgement about the + // commit rather than about this landing. + let mut poisoned = placed(); + poisoned.poisoned = Some(String::from(HOLDER)); + poisoned.forget(); assert!( - Bet::default().would_rebet(HOLDER), - "and a forgotten bet re-bets on the same holder — CLOUD-1306's other half" + !poisoned.would_rebet(HOLDER), + "a base whose tree the gate refused is not borrowed a second time" + ); + assert!( + poisoned.would_rebet(MOVED), + "and one bad candidate does not end speculation for the whole landing, \ + because speculation is the pipelining" ); } diff --git a/crates/batten/tests/it/land_lap.rs b/crates/batten/tests/it/land_lap.rs index 173e11ac4..94af42b6e 100644 --- a/crates/batten/tests/it/land_lap.rs +++ b/crates/batten/tests/it/land_lap.rs @@ -248,9 +248,11 @@ fn every_lap_ending_is_reachable_from_some_step_and_code() { Some(TapVerdict::Red), Some(TapVerdict::Pending), ] { - let progress = land::progress_of(step, code, verdict); - if !seen.contains(&progress) { - seen.push(progress); + for basis in [land::Basis::Own, land::Basis::Borrowed] { + let progress = land::progress_of(step, code, verdict, basis); + if !seen.contains(&progress) { + seen.push(progress); + } } } } From fc24de5b0e5a2d3f5cade3ba513db98b4426c996 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Fri, 11 Sep 2026 02:20:19 +0000 Subject: [PATCH 14/18] feat(lease): the body carries a refusable major and advertises its writer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "There is one administrator" is false here by construction: the fleet runs whatever versions its agents happen to carry and nothing coordinates an upgrade. The body had no way to say "you do not speak this" and the parser had no way to hear it — unknown keys were ignored, so a format change would have become silent disagreement across a mixed fleet, with a lock under it. `schema:` is the one field a reader may refuse over. A major above what this build speaks does not parse, which the caller already renders as `Observed::Garbage`: still held, still respected until it ages out, and now diagnosable rather than silently misread. A `schema:` that will not parse as a number is refused on the same ground — a body whose version field is unintelligible was written by something that does not agree with us about what the field is, and reading it as the oldest format is the loose parse the field exists to stop. ABSENT MEANS 1, which is the compatibility hinge. Every lease written before this field existed carries no such line and those bodies ARE major 1, so the default is a reading rather than a guess: a reader that refused them would stop the fleet on the deploy that introduced the field. READERS SHIP BEFORE WRITERS, which is why this emits 1 rather than 2. An old reader ignores an unknown key, so it would parse a `schema: 2` body loosely and act on it — the exact failure being closed here. The major may only be raised once a release that refuses an unknown one is out across the fleet. `writer:` advertises the build that minted the body, so version skew is visible rather than inferred from behaviour. It is advisory and decides nothing: refusing to honour a lease because its writer is old would hand every upgrade a fleet-wide outage, and an identity another clone could derive is one another clone could accidentally claim. It is stamped at every mint rather than carried forward, because `reservation` is minted by a WAITER onto the holder's body — carrying it would attribute that write to the holder and make the one field that exists to expose skew the field that hides it. The holder is still carried, which is what keeps that from being a steal. Refs: CLOUD-1778 --- crates/batten/src/lease.rs | 209 +++++++++++++++++++++- crates/batten/tests/it/lease_health.rs | 1 + crates/batten/tests/it/lease_lifecycle.rs | 1 + 3 files changed, 209 insertions(+), 2 deletions(-) diff --git a/crates/batten/src/lease.rs b/crates/batten/src/lease.rs index 895ed5be0..cb3a6ebfe 100644 --- a/crates/batten/src/lease.rs +++ b/crates/batten/src/lease.rs @@ -1095,10 +1095,62 @@ pub struct Body { /// EQUALITY OVER TIME and never interprets it, so no clock crosses the wire. /// Advisory. pub progress: String, + /// The body format's MAJOR, and the one field a reader may refuse over. + /// + /// **"There is one administrator" is false here by construction**: the fleet + /// runs whatever versions its agents happen to carry, and nothing coordinates + /// an upgrade. So a body needs a way to say "you do not speak this", and a + /// reader needs a way to hear it that is not "parse it loosely and act on the + /// fields I recognised" — which is what the old parser did, and which turns a + /// format change into silent disagreement across a mixed fleet. + /// + /// **ABSENT MEANS 1, AND THAT IS THE COMPATIBILITY HINGE.** Every lease + /// written before this field existed carries no `schema:` line, and those + /// bodies are exactly schema 1 — so the default is a reading rather than a + /// guess. A reader seeing a major it does not know treats the lease as + /// [`Observed::Garbage`]: held, respected until it ages out, and diagnosable. + /// That is the same safe arm an unreadable lease already takes, and the + /// opposite of ignoring the field and racing the writer. + /// + /// **READERS SHIP BEFORE WRITERS**, which is why this lands emitting 1 rather + /// than 2. An old reader ignores an unknown key, so it would parse a `schema: + /// 2` body loosely and act on it — the exact failure this field prevents. The + /// major may only be raised once a release that REFUSES an unknown one is out + /// across the fleet. + pub schema: u32, + /// The batten version that minted this body. Advisory, and never a gate. + /// + /// Advertised so that version skew is VISIBLE rather than inferred from + /// behaviour. A waiter that sees a holder older than itself can say so, and a + /// stale agent can be asked to stand down by a peer — but the asking is + /// advisory and this field decides nothing, for [`Body`]'s stated reason: an + /// identity another clone could derive is one another clone could accidentally + /// claim. Refusing to honour a lease because its writer is old would hand + /// every upgrade a fleet-wide outage. + pub writer: String, /// What makes every mint a distinct object. See [`lease_object`]. pub nonce: String, } +/// The body major this build writes and is the highest it can read. +/// +/// One constant for both directions deliberately: a build that wrote a major it +/// could not read would be announcing a format to a fleet it is not itself part +/// of. +pub const BODY_SCHEMA: u32 = 1; + +/// This build's own version, as [`Body::writer`] advertises it. +/// +/// `CARGO_PKG_VERSION` here, and `spec.rs`'s argument against it does not reach +/// this use: that one refuses it for a SPEC version, where the number is a claim +/// about a document's content and must not move just because the crate shipped. +/// This is the opposite — the claim IS "which build wrote this", so tracking the +/// crate is the whole point. +#[must_use] +pub fn writer_version() -> String { + String::from(env!("CARGO_PKG_VERSION")) +} + /// The banner every lease body opens with, so a commit that is not a lease is not /// read as one. const BANNER: &str = "land-lock"; @@ -1109,9 +1161,23 @@ impl Body { pub fn render(&self) -> String { // `nonce:` STAYS LAST. Its uniqueness argument is what makes every mint a // distinct sha, and the check half treats it as the terminal line. + // + // `schema:` IS FIRST AFTER THE BANNER, so a reader that refuses an unknown + // major has seen it before it has read a single field it might act on. + // Ordering is not what makes the refusal correct — the parser reads the + // whole body before deciding — but a format whose version is buried in the + // middle invites a streaming reader that acts before it checks. format!( - "{BANNER}\nholder: {}\nexpires: {}\nbranch: {}\nhead: {}\nnext: {}\nprogress: {}\nnonce: {}\n", - self.holder, self.expires, self.branch, self.head, self.next, self.progress, self.nonce + "{BANNER}\nschema: {}\nholder: {}\nexpires: {}\nbranch: {}\nhead: {}\nnext: {}\nprogress: {}\nwriter: {}\nnonce: {}\n", + self.schema, + self.holder, + self.expires, + self.branch, + self.head, + self.next, + self.progress, + self.writer, + self.nonce ) } @@ -1149,6 +1215,10 @@ pub fn parse_body(object: &[u8]) -> Option { let mut body = Body::default(); let mut banner = false; let mut expires = None; + // `Option>`: absent, present-and-unreadable, present-and-a-number. + // The three are different answers and collapsing the middle into either + // neighbour is the loose parse this field exists to refuse. + let mut schema: Option> = None; for line in text.lines() { if line == BANNER { banner = true; @@ -1177,13 +1247,30 @@ pub fn parse_body(object: &[u8]) -> Option { "head" => value.clone_into(&mut body.head), "next" => value.clone_into(&mut body.next), "progress" => value.clone_into(&mut body.progress), + "writer" => value.clone_into(&mut body.writer), "nonce" => value.clone_into(&mut body.nonce), + // PARSED PERMISSIVELY, REFUSED BELOW. A `schema:` line this reader + // cannot turn into a number is not schema 1 — it is a body written by + // something that does not agree with us about what the field even is, + // and reading it as the oldest format would be the loose parse this + // field exists to stop. `None` here falls into the unknown-major arm. + "schema" => schema = Some(value.parse::().ok()), _ => {} } } if !banner { return None; } + // ABSENT IS 1, UNREADABLE AND TOO-NEW ARE REFUSALS. Every body written before + // this field existed is schema 1, so the absent case is a reading rather than + // a default — and a major above what this build speaks yields `None`, which + // the caller renders as `Observed::Garbage`: still held, still respected until + // it ages out, and now diagnosable instead of silently misread. + body.schema = match schema { + None => BODY_SCHEMA, + Some(Some(major)) if major <= BODY_SCHEMA => major, + Some(_) => return None, + }; body.expires = expires?; Some(body) } @@ -1925,6 +2012,8 @@ pub fn claim(terms: &Terms, holder: &str, branch: &str, head: &str, now: i64) -> head: head.to_owned(), next: String::new(), progress: String::new(), + schema: BODY_SCHEMA, + writer: writer_version(), nonce: nonce(), } } @@ -1978,6 +2067,14 @@ pub fn lands_by_fast_forward(branch: &str, prefixes: &[String]) -> bool { pub fn tombstone(body: &Body) -> Body { Body { expires: 0, + // STAMPED, NEVER CARRIED. Every mint here is THIS build writing a body + // now, so advertising the version it inherited would name a writer that + // did not write it — and on `reservation`, which a WAITER mints onto the + // holder's body, the inherited value is a different agent's entirely. + // Carrying forward would make the one field that exists to expose skew + // the field that hides it. + schema: BODY_SCHEMA, + writer: writer_version(), nonce: nonce(), ..body.clone() } @@ -1996,6 +2093,8 @@ pub fn renewal(terms: &Terms, body: &Body, progress: Option<&str>, now: i64) -> Body { expires: now + terms.ttl, progress: progress.map_or_else(|| body.progress.clone(), str::to_owned), + schema: BODY_SCHEMA, + writer: writer_version(), nonce: nonce(), ..body.clone() } @@ -2077,6 +2176,12 @@ pub fn beat(root: &Path, terms: &Terms, progress: Option<&str>, now: i64) -> boo pub fn reservation(body: &Body, want: &str) -> Body { Body { next: want.to_owned(), + // THE WAITER'S VERSION, NOT THE HOLDER'S — see `tombstone`. This is the + // one mint where the writer is a DIFFERENT agent from the holder the body + // names, so carrying `writer` forward would attribute this write to the + // holder and make the skew field actively misleading. + schema: BODY_SCHEMA, + writer: writer_version(), nonce: nonce(), ..body.clone() } @@ -4106,6 +4211,8 @@ mod tests { progress: String::from("100.200"), nonce: String::from("aaaaaaaaaaaaaaaa"), next: String::new(), + schema: BODY_SCHEMA, + writer: String::from("0.0.1"), }; let reserved = reservation(&body, "claude/b"); assert_eq!(reserved.next, "claude/b"); @@ -4229,12 +4336,110 @@ mod tests { head: String::from("2222222222222222222222222222222222222222"), next: String::from("claude/y"), progress: String::from("1700000000.1700000030"), + schema: BODY_SCHEMA, + writer: String::from("0.0.1"), nonce: String::from("deadbeefdeadbeef"), }; let object = lease_object(&body.render(), 1_700_000_000).expect("mint"); assert_eq!(parse_body(&object.body), Some(body)); } + /// **THE COMPATIBILITY HINGE, and the case the rollout turns on.** + /// + /// Every lease written before `schema:` existed carries no such line, and + /// those bodies ARE schema 1. A reader that refused them would stop the fleet + /// on the deploy that introduced the field; one that defaulted them to + /// anything else would mislabel every lease in flight. + #[test] + fn a_body_written_before_the_schema_field_reads_as_the_first_major() { + let object = lease_object( + "land-lock\nholder: host-1-aa\nexpires: 1700000060\nnonce: bb\n", + 1_700_000_000, + ) + .expect("mint"); + let body = parse_body(&object.body).expect("a pre-schema body still parses"); + assert_eq!( + body.schema, BODY_SCHEMA, + "an absent major is a reading, not a guess: these bodies are major 1" + ); + assert_eq!(body.holder, "host-1-aa", "and the rest still parses"); + assert!( + body.writer.is_empty(), + "a pre-schema body advertises no writer, and inventing one would \ + report skew that was never measured" + ); + } + + /// A major above what this build speaks does not parse, so the caller renders + /// it as `Observed::Garbage` — held, respected until it ages out, diagnosable. + /// + /// **This is the arm that makes the field worth having.** Without it the + /// parser ignores the unknown key, acts on the fields it recognised, and a + /// format change becomes silent disagreement across a mixed fleet — which is + /// the "one administrator" fallacy with a lock under it. + #[test] + fn a_body_from_a_newer_major_is_refused_rather_than_read_loosely() { + let object = lease_object( + "land-lock\nschema: 99\nholder: host-9-zz\nexpires: 1700000060\nnonce: bb\n", + 1_700_000_000, + ) + .expect("mint"); + assert_eq!( + parse_body(&object.body), + None, + "a body this build does not speak must not be acted on" + ); + } + + /// A `schema:` this reader cannot turn into a number is refused too. + /// + /// Reading it as the oldest format would be exactly the loose parse the field + /// exists to stop: a body whose version field is unintelligible was written by + /// something that does not agree with us about what the field is. + #[test] + fn an_unreadable_major_is_refused_rather_than_treated_as_the_oldest() { + let object = lease_object( + "land-lock\nschema: tomorrow\nholder: a\nexpires: 1700000060\nnonce: bb\n", + 1_700_000_000, + ) + .expect("mint"); + assert_eq!(parse_body(&object.body), None); + } + + /// Every mint stamps THIS build, and never inherits a predecessor's. + /// + /// `reservation` is the discriminating one: a WAITER mints it onto the + /// holder's body, so a carried `writer` would attribute the write to the + /// holder — making the one field that exists to expose skew the field that + /// hides it. The holder itself must still be carried, which is what separates + /// this from a steal. + #[test] + fn every_mint_advertises_the_build_that_wrote_it() { + let held = Body { + holder: String::from("host-1-aa"), + expires: 1_700_000_060, + writer: String::from("0.0.1"), + schema: BODY_SCHEMA, + ..Body::default() + }; + for minted in [ + reservation(&held, "claude/y"), + tombstone(&held), + renewal(&Terms::default(), &held, None, 1_700_000_000), + ] { + assert_eq!( + minted.writer, + writer_version(), + "a mint advertises the build that performed it" + ); + assert_eq!(minted.schema, BODY_SCHEMA); + assert_eq!( + minted.holder, "host-1-aa", + "and stamping the writer must not become a steal of the holder" + ); + } + } + #[test] fn a_commit_that_is_not_a_lease_does_not_parse_as_an_unheld_one() { // The same refusal the advertisement makes: a foreign object parsed diff --git a/crates/batten/tests/it/lease_health.rs b/crates/batten/tests/it/lease_health.rs index 65fb69cb8..a00d2796f 100644 --- a/crates/batten/tests/it/lease_health.rs +++ b/crates/batten/tests/it/lease_health.rs @@ -274,6 +274,7 @@ fn a_stalled_holder_is_stealable_once_its_beat_has_published() { next: String::new(), progress: progress.to_owned(), nonce: String::from("n1"), + ..Body::default() }; let observed = |progress: &str| Observed::Held { sha: "f".repeat(40), diff --git a/crates/batten/tests/it/lease_lifecycle.rs b/crates/batten/tests/it/lease_lifecycle.rs index f5f98f5fe..2920345f5 100644 --- a/crates/batten/tests/it/lease_lifecycle.rs +++ b/crates/batten/tests/it/lease_lifecycle.rs @@ -157,6 +157,7 @@ fn held(branch: &str, expires: i64) -> Body { next: String::new(), progress: String::from("verify"), nonce: String::from("n1"), + ..Body::default() } } From d71f48eb0443d411cf3a7e89857afb364106bc7e Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Fri, 11 Sep 2026 03:03:00 +0000 Subject: [PATCH 15/18] feat(lease): a peer can ask the holder to stand down, and the holder decides MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A waiter behind a wedged holder could do nothing but wait out the TTL. This adds the advisory half of eviction: `stand-down:` on the lease body, written by a peer through the same CAS that admits a successor, read by the holder's beat, and honoured at a lap boundary. IT IS NOT A STEAL WEARING A DIFFERENT NAME, which is the property to protect. `notice` moves one field: the requester does not become the holder and does not become the admitted successor, and the expiry is re-minted as it was. Honouring a notice RELEASES the lease; the asker then races the next acquire like anybody else. A route by which naming yourself gets you the lease is a steal however politely it is spelled, and "I suspect you are stuck" would be the fastest way to the front of the queue. IT IS A REQUEST, NOT AN EVICTION, and that is forced rather than chosen. No failure detector is accurate in an asynchronous system, so every suspicion that a holder is wedged is a heuristic and some are wrong. A peer that could TAKE the lease on suspicion would be wrong in exactly those cases. So the field carries no authority: the holder reads it and the holder decides. The unilateral half — a fenced compare-and-set whose advanced token makes the evicted holder's next write fail — is the referee's and is not built here. Both exist because each is sound for something the other is not. AND IT IS HONOURED AT A LAP BOUNDARY, NEVER MID-MATRIX. The beat keeps renewing while a notice is pending, which looks like ignoring it and is the opposite: a holder that stopped renewing would lose the lease inside its own CI matrix and a second lander would start — precisely the overlap the heartbeat was written to close, reintroduced by the mechanism meant to be polite. Between laps there is no matrix in flight, so releasing costs the fleet nothing and costs this branch one re-run. `beat` answers a `Beat` rather than a `bool` because it is the only thing that reads the lease every few seconds, so it is where a notice is seen and a `bool` had nowhere to put it. Collapsing it into `false` would have been worse than dropping it: `false` means "one beat did not write", which the caller is explicitly told is not news. The request is latched across laps rather than re-read, because the beat re-mints the body and a later writer can overwrite the field — once asked, this holder has been asked. Standing down exits 3, never 2: nothing about this tree is wrong and the head is still landable, so it reads as "the loop stopped asking" and the caller runs it again. A 2 would send an agent to fix a defect that does not exist. A self-named notice is ignored, because the holder re-mints the whole body every beat and such a notice would survive its own release and ask every future holder to stand down forever. A fresh claim clears the field for the same reason. Refs: CLOUD-1778 BREAKING CHANGE: `lease::beat` answers `lease::Beat` rather than `bool`, and `lease::Body` gains `schema`, `writer` and `stand_down`. The body's wire format gains three lines; readers of an older major are unaffected because an absent `schema:` reads as major 1. --- crates/batten/src/land.rs | 16 +++ crates/batten/src/lease.rs | 195 +++++++++++++++++++++++++++++-- crates/batten/src/lib.rs | 228 ++++++++++++++++++++++++++++--------- 3 files changed, 379 insertions(+), 60 deletions(-) diff --git a/crates/batten/src/land.rs b/crates/batten/src/land.rs index 3c0afc746..dbce97b2e 100644 --- a/crates/batten/src/land.rs +++ b/crates/batten/src/land.rs @@ -1443,6 +1443,22 @@ pub enum Basis { Borrowed, } +impl Basis { + /// The basis for a lap whose bet is or is not outstanding. + /// + /// One place that turns "is a bet live" into this reading, so a caller cannot + /// spell the mapping backwards — which on this type would silently invert the + /// whole cell it qualifies. + #[must_use] + pub const fn of(speculating: bool) -> Self { + if speculating { + Self::Borrowed + } else { + Self::Own + } + } +} + #[must_use] pub const fn progress_of( step: Step, diff --git a/crates/batten/src/lease.rs b/crates/batten/src/lease.rs index cb3a6ebfe..f0d3e465b 100644 --- a/crates/batten/src/lease.rs +++ b/crates/batten/src/lease.rs @@ -1091,6 +1091,42 @@ pub struct Body { pub head: String, /// The one admitted successor, or empty. Advisory. pub next: String, + /// A peer's request that the holder release, naming the peer. Advisory, and + /// the most advisory field here (CLOUD-1778). + /// + /// # Why a REQUEST rather than an eviction + /// + /// No failure detector is accurate in an asynchronous system, so every + /// suspicion that a holder is wedged is a heuristic and some of them are + /// wrong. A design that let a peer TAKE the lease on suspicion would be wrong + /// under exactly those cases. So this field carries no authority at all: the + /// holder reads it, and the holder decides. + /// + /// The unilateral half — a fenced compare-and-set that advances a token the + /// evicted holder's next write then fails — is the referee's and is not built + /// here. Both halves exist because they are sound for different things: a + /// request is cheap and correct when the holder is healthy but idle, and + /// fencing is the only thing that is safe when it is not. + /// + /// # IT IS NOT A STEAL WEARING A DIFFERENT NAME + /// + /// Honouring a notice RELEASES the lease; it does not hand it to the + /// requester, who must then win the next acquire like anybody else. That is + /// the property to protect, and it is the same sentence [`reservation`] + /// carries about the holder id: a mechanism by which naming yourself gets you + /// the lease is a steal however politely it is spelled. A waiter that could + /// evict-and-inherit would make "I suspect you are stuck" the fastest route to + /// the front of the queue. + /// + /// # AND IT IS HONOURED AT A LAP BOUNDARY, NEVER MID-MATRIX + /// + /// The beat keeps renewing while a notice is pending, which looks like + /// ignoring it and is the opposite. A holder that stopped renewing here would + /// lose the lease inside its own CI matrix, a second lander would start, and + /// that is precisely the overlap [`beat`] was written to close. The notice is + /// reported to the lap driver, which honours it where releasing costs nothing + /// — between laps, with no matrix in flight. + pub stand_down: String, /// The holder's own progress token. **Opaque by design**: a rival tests it for /// EQUALITY OVER TIME and never interprets it, so no clock crosses the wire. /// Advisory. @@ -1168,13 +1204,14 @@ impl Body { // whole body before deciding — but a format whose version is buried in the // middle invites a streaming reader that acts before it checks. format!( - "{BANNER}\nschema: {}\nholder: {}\nexpires: {}\nbranch: {}\nhead: {}\nnext: {}\nprogress: {}\nwriter: {}\nnonce: {}\n", + "{BANNER}\nschema: {}\nholder: {}\nexpires: {}\nbranch: {}\nhead: {}\nnext: {}\nstand-down: {}\nprogress: {}\nwriter: {}\nnonce: {}\n", self.schema, self.holder, self.expires, self.branch, self.head, self.next, + self.stand_down, self.progress, self.writer, self.nonce @@ -1234,6 +1271,7 @@ pub fn parse_body(object: &[u8]) -> Option { "branch" => body.branch.clear(), "head" => body.head.clear(), "next" => body.next.clear(), + "stand-down" => body.stand_down.clear(), "progress" => body.progress.clear(), _ => {} } @@ -1248,6 +1286,7 @@ pub fn parse_body(object: &[u8]) -> Option { "next" => value.clone_into(&mut body.next), "progress" => value.clone_into(&mut body.progress), "writer" => value.clone_into(&mut body.writer), + "stand-down" => value.clone_into(&mut body.stand_down), "nonce" => value.clone_into(&mut body.nonce), // PARSED PERMISSIVELY, REFUSED BELOW. A `schema:` line this reader // cannot turn into a number is not schema 1 — it is a body written by @@ -2011,6 +2050,11 @@ pub fn claim(terms: &Terms, holder: &str, branch: &str, head: &str, now: i64) -> branch: branch.to_owned(), head: head.to_owned(), next: String::new(), + // EMPTY ON ACQUISITION, which is what keeps a notice from outliving the + // holder it was aimed at. A fresh claim inheriting a stand-down would ask + // every future holder to release on the strength of one request made of + // somebody else. + stand_down: String::new(), progress: String::new(), schema: BODY_SCHEMA, writer: writer_version(), @@ -2141,22 +2185,57 @@ pub fn renewal(terms: &Terms, body: &Body, progress: Option<&str>, now: i64) -> /// [`crate::run_land_singleton`]'s heartbeat reads it from the task registry with /// [`progress_of`] — must pass it, and only a caller with nothing to say passes /// `None`. +/// What one beat found. +/// +/// **A richer answer than the `bool` this replaced, because the beat is the only +/// thing that reads the lease every few seconds** — so it is where a peer's +/// stand-down notice is seen, and a `bool` had nowhere to put it. Collapsing the +/// notice into `false` would have been worse than dropping it: `false` means "one +/// beat did not write", which the caller is explicitly told is not news. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Beat { + /// The lease was renewed. + Renewed, + /// Nothing was written, and it is not news — no identity, no lease, a lease + /// held by somebody else, a rejected CAS, a push that did not land. See + /// [`beat`] for why one of these is survivable by construction. + Quiet, + /// Renewed, AND a peer has asked this holder to release, naming itself. + /// + /// **Renewed as well, and the conjunction is the point.** Not renewing while a + /// notice is pending would drop the lease inside this holder's own CI matrix + /// and let a second lander start — the overlap [`beat`] exists to close, + /// reintroduced by the mechanism meant to be polite. The caller honours this + /// at a lap boundary, where releasing costs nothing. + StandDown(String), +} + #[must_use] -pub fn beat(root: &Path, terms: &Terms, progress: Option<&str>, now: i64) -> bool { +pub fn beat(root: &Path, terms: &Terms, progress: Option<&str>, now: i64) -> Beat { let Ok((_, holder)) = crate::lease_identity(root) else { - return false; + return Beat::Quiet; }; let Ok(observed) = observe(terms) else { - return false; + return Beat::Quiet; }; if !holds_now(&observed, &holder, now) { - return false; + return Beat::Quiet; } let Observed::Held { body, .. } = &observed else { - return false; + return Beat::Quiet; }; + // READ BEFORE THE RE-MINT, because the re-mint carries `stand_down` forward + // like every other advisory field and the reading must be of what the peer + // actually wrote rather than of our own copy of it. + let asked = stand_down_requested(body, &holder).map(str::to_owned); let renewed = renewal(terms, body, progress, now); - matches!(cas(terms, &observed, &renewed, now), Ok(Outcome::Applied)) + if !matches!(cas(terms, &observed, &renewed, now), Ok(Outcome::Applied)) { + return Beat::Quiet; + } + // THE RENEWAL FIRST, THE NOTICE SECOND. A holder that reported the notice + // without renewing would be honouring it immediately, which is exactly the + // mid-matrix release this ordering exists to prevent. + asked.map_or(Beat::Renewed, Beat::StandDown) } /// Fill the one successor slot, re-minting every other field verbatim. @@ -2187,6 +2266,44 @@ pub fn reservation(body: &Body, want: &str) -> Body { } } +/// A peer's request that the holder release, naming the peer (CLOUD-1778). +/// +/// [`reservation`]'s shape exactly — one field moved, everything else re-minted +/// verbatim — and for the same reasons. A notice that recomputed the holder id +/// would be a steal; one that recomputed the expiry would hand the holder a fresh +/// TTL for the trouble of being asked to leave. +/// +/// **`next` IS NOT SET HERE, AND THAT IS THE WHOLE SAFETY PROPERTY.** The +/// requester does not become the successor by asking. It releases the holder and +/// then races the next acquire like anybody else, because a route by which naming +/// yourself gets you the lease is a steal however politely it is spelled. A caller +/// that wants both must take both, separately and visibly. +#[must_use] +pub fn notice(body: &Body, from: &str) -> Body { + Body { + stand_down: from.to_owned(), + schema: BODY_SCHEMA, + writer: writer_version(), + nonce: nonce(), + ..body.clone() + } +} + +/// Has a peer asked this holder to stand down? +/// +/// A pure reading over a body already in hand, so the beat pays no extra fetch +/// for it and a caller can ask the same question of a lease it observed for some +/// other reason. +/// +/// **A notice naming the holder itself is ignored**, which is not a courtesy: the +/// holder re-mints the whole body every beat, so a self-named notice would survive +/// its own release and ask every future holder to stand down forever. +#[must_use] +pub fn stand_down_requested<'body>(body: &'body Body, holder: &str) -> Option<&'body str> { + let from = body.stand_down.trim(); + (!from.is_empty() && from != holder).then_some(from) +} + /// Sixteen hex characters of entropy, which is what keeps every mint distinct. /// /// See [`lease_object`] for why a mint that agreed with another mint on every @@ -4211,6 +4328,7 @@ mod tests { progress: String::from("100.200"), nonce: String::from("aaaaaaaaaaaaaaaa"), next: String::new(), + stand_down: String::new(), schema: BODY_SCHEMA, writer: String::from("0.0.1"), }; @@ -4335,6 +4453,7 @@ mod tests { branch: String::from("claude/x"), head: String::from("2222222222222222222222222222222222222222"), next: String::from("claude/y"), + stand_down: String::from("host-2-bb"), progress: String::from("1700000000.1700000030"), schema: BODY_SCHEMA, writer: String::from("0.0.1"), @@ -4344,6 +4463,68 @@ mod tests { assert_eq!(parse_body(&object.body), Some(body)); } + /// **A NOTICE IS NOT A STEAL, and this is the case that says so.** + /// + /// The requester does not become the holder and does not become the + /// successor: it releases the holder and then races the next acquire like + /// anybody else. A mechanism by which naming yourself gets you the lease is a + /// steal however politely it is spelled, and "I suspect you are stuck" would + /// be the fastest route to the front of the queue. + #[test] + fn a_stand_down_notice_moves_one_field_and_grants_the_asker_nothing() { + let held = Body { + holder: String::from("host-1-aa"), + expires: 1_700_000_060, + branch: String::from("claude/a"), + next: String::new(), + ..Body::default() + }; + let asked = notice(&held, "host-2-bb"); + assert_eq!(asked.stand_down, "host-2-bb", "the peer names itself"); + assert_eq!( + asked.holder, "host-1-aa", + "and does NOT become the holder by asking" + ); + assert!( + asked.next.is_empty(), + "nor the admitted successor — that is a separate, visible act" + ); + assert_eq!( + asked.expires, held.expires, + "asking must not hand the holder a fresh TTL for the trouble" + ); + assert_eq!(asked.branch, held.branch); + } + + /// The holder reads it; a notice naming the holder itself does not. + /// + /// Self-named is ignored because the holder re-mints the whole body every + /// beat, so such a notice would survive its own release and ask every future + /// holder to stand down forever. + #[test] + fn a_notice_is_read_by_the_holder_and_a_self_named_one_is_not() { + let asked = Body { + stand_down: String::from("host-2-bb"), + ..Body::default() + }; + assert_eq!( + stand_down_requested(&asked, "host-1-aa"), + Some("host-2-bb"), + "a peer's request reaches the holder" + ); + assert_eq!( + stand_down_requested(&asked, "host-2-bb"), + None, + "a holder does not ask itself to leave, and a self-named notice would \ + outlive every release" + ); + assert_eq!( + stand_down_requested(&Body::default(), "host-1-aa"), + None, + "and no notice is no request" + ); + } + /// **THE COMPATIBILITY HINGE, and the case the rollout turns on.** /// /// Every lease written before `schema:` existed carries no such line, and diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index da3af899a..5a01abecb 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -6742,7 +6742,7 @@ fn run_land( // The verdict is the LAP's to read; a hand-driven wait reports the code // and nothing else, exactly as it did before the tap needed one. cli::LandCommand::Wait { reference } => { - run_land_wait(root, reference, &branch, out, err).map(|(code, _)| code) + run_land_wait(root, reference, &branch, out, err).map(|(code, _, _)| code) } cli::LandCommand::Push => { let Some(url) = land_remote(root, err)? else { @@ -7286,7 +7286,14 @@ fn run_land_lap( // deferred. `pipeline::unwind` already dedupes, so a step entered twice // across two laps is still compensated once. let mut entered: Vec = Vec::new(); + // Who asked this holder to stand down, latched across laps — see + // `honour_the_notice` for why it is a latch and why it is read here. + let mut stood_down: Option = None; 'laps: for lap in 1..=laps { + // Checked BEFORE `attempt`, so a lap that will not run is not charged. + if let Some(from) = stood_down.take() { + return honour_the_notice(&from, root, branch, &pipeline, &mut entered, out, err); + } ledger.attempt(); writeln!(out, "land: lap {lap} of {laps}")?; // WHAT THE WAIT SAW, or `None` where no lap took a reading. The tap @@ -7308,11 +7315,9 @@ fn run_land_lap( let step = row.step; // THE ROW'S OWN QUESTION, where the driver used to carry a // `step == Verify` exception. A pre-check runs BEFORE the primitive - // and can only lap, never land: it exists to spend nothing. - // THE BET IS SETTLED BEFORE ANYTHING IS SPENT, and before the - // replay that would otherwise build on somebody else's commits. - // THE TWO ROWS THAT CAN END A LAP BEFORE IT SPENDS, asked together - // because they share an unwind and differ only in what follows it. + // and can only lap, never land: it exists to spend nothing, and it + // settles the bet before the replay that would otherwise build on + // somebody else's commits. let asked = asks_before_the_step(*row, this_lap, &mut trunk_poll, &mut bet, out, err)?; if let Answered::Stop(code) = asked { unwind_lap(root, branch, &pipeline, &mut entered, seen, out, err)?; @@ -7323,30 +7328,15 @@ fn run_land_lap( continue 'laps; } // THE PHASE, PUSHED BEFORE THE STEP RUNS RATHER THAN AFTER IT. A step - // is what this lap is doing WHILE it blocks, and the whole reason a - // reader wants it is that a gate can hold for minutes — so announcing - // it on completion would name every phase exactly when it stopped - // being true. `land.sh` pushed at the transition for the same reason. + // is what this lap is doing WHILE it blocks, and a gate can hold for + // minutes — so announcing it on completion would name every phase + // exactly when it stopped being true. guard.phase(step.as_str(), lap); - let code = match step { - // NO RESOLUTIONS ON THE LAP, ever. A lap runs unattended, so a - // resolution it could apply would be one nobody looked at — - // `gitwrite`'s auto-resolution refusal, reached through the - // driver instead of through a flag. - land::Step::Replay => run_land_replay(root, url, reference, branch, &[], out)?, - land::Step::Verify => { - run_land_verify(root, &bet, branch, Some(reference), out, err)? - } - land::Step::Lease => run_land_lease(root, branch, out, err)?, - land::Step::Ready => run_land_ready(root, branch, &bet, &mut ledger, out, err)?, - land::Step::Push => run_land_push(root, url, branch, out)?, - land::Step::Wait => { - let (code, verdict) = run_land_wait(root, reference, branch, out, err)?; - seen = verdict; - code - } - land::Step::FastForward => run_land_fast_forward(root, branch, out, err)?, - }; + let (code, verdict, asked) = run_the_step(step, this_lap, &bet, &mut ledger, out, err)?; + // The wait is the only step with readings to carry, and both are + // latched rather than acted on here — see `honour_the_notice`. + seen = verdict.or(seen); + stood_down = stood_down.or(asked); // ENTERED ON SUCCESS, OR ON THE ATTEMPT WHERE THE UNDO SAYS SO. The // first half is the discrimination the undo rests on: a `Ready` that // REFUSED bought no matrix, so re-drafting over it would draft a pull @@ -7361,16 +7351,10 @@ fn run_land_lap( entered.push(step); } note_the_push(step, code, &mut bet); - // READ HERE, ONCE, AND BEFORE ANY OF THE ARMS BELOW CAN MOVE IT. The - // bet is what makes this lap's tree speculative, and `unwind_lap` on - // the lap arm can settle or forget it — so a second read inside the - // charge would ask about a different world than the one that answered - // (CLOUD-1306). - let basis = if bet.live() { - land::Basis::Borrowed - } else { - land::Basis::Own - }; + // READ ONCE, HERE, BEFORE ANY ARM BELOW CAN MOVE IT: `unwind_lap` can + // settle or forget the bet, so a later read would ask about a + // different world than the one that answered (CLOUD-1306). + let basis = land::Basis::of(bet.live()); note_the_poison(step, code, basis, &mut bet); match land::progress_of(step, code, seen, basis) { land::Progress::Proceed => {} @@ -7891,6 +7875,86 @@ fn charge_or_refuse( Ok(Some(ExitCode::Internal)) } +/// Run one step of the lap, and hand back what only the wait produces. +/// +/// Split out of [`run_land_lap`] on this tree's own precedent — `trust.rs` and +/// `cli.rs` both record splitting rather than suppressing `too_many_lines`, and +/// the driver is the function that grows every time a step learns something new. +/// +/// The two `Option`s are the wait's alone and are `None` for every other step: +/// the tap's verdict, which the exit code cannot carry because a stale base and +/// an unanswered wait are both a lap, and a peer's stand-down notice, which is +/// latched by the caller and honoured at a lap boundary. +fn run_the_step( + step: land::Step, + asked: Asked<'_>, + bet: &speculation::Bet, + ledger: &mut land::Ledger, + out: &mut dyn Write, + err: &mut dyn Write, +) -> Result<(ExitCode, Option, Option)> { + let Asked { + root, + url, + reference, + branch, + .. + } = asked; + let code = match step { + // NO RESOLUTIONS ON THE LAP, ever. A lap runs unattended, so a resolution + // it could apply would be one nobody looked at — `gitwrite`'s + // auto-resolution refusal, reached through the driver rather than a flag. + land::Step::Replay => run_land_replay(root, url, reference, branch, &[], out)?, + land::Step::Verify => run_land_verify(root, bet, branch, Some(reference), out, err)?, + land::Step::Lease => run_land_lease(root, branch, out, err)?, + land::Step::Ready => run_land_ready(root, branch, bet, ledger, out, err)?, + land::Step::Push => run_land_push(root, url, branch, out)?, + land::Step::Wait => return run_land_wait(root, reference, branch, out, err), + land::Step::FastForward => run_land_fast_forward(root, branch, out, err)?, + }; + Ok((code, None, None)) +} + +/// Release the lease because a peer asked, and say who asked (CLOUD-1778). +/// +/// **THE LAP BOUNDARY IS WHERE A NOTICE IS HONOURED, and it is the only place it +/// can be honoured cheaply.** Between laps there is no matrix in flight and no +/// ready pull request this lap created, so releasing costs the fleet nothing and +/// costs this branch one re-run. A holder that stood down mid-wait would abandon a +/// matrix it had already paid for AND let a second lander start — the overlap the +/// heartbeat exists to close, reintroduced by the mechanism meant to be polite. +/// +/// **EXIT 3, NEVER 2.** Standing down is not a verdict about this branch: nothing +/// about the tree is wrong and the head is still landable. `3` is the same reading +/// a spent lap budget gets — the loop stopped asking — and the caller runs it +/// again. A `2` here would tell an agent to go and fix a defect that does not +/// exist. +/// +/// # The caller's latch, and why it is one +/// +/// The request is made inside a wait and honoured at the NEXT lap's boundary, so +/// the driver holds it in a binding declared outside the loop — a per-lap one +/// would drop it in the gap between the two, which is the whole distance it has +/// to travel. And a latch rather than a live re-read, because the beat re-mints +/// the body every few seconds and a later writer can overwrite the field: once +/// asked, this holder has been asked. +fn honour_the_notice( + from: &str, + root: &Path, + branch: &str, + pipeline: &pipeline::Pipeline, + entered: &mut Vec, + out: &mut dyn Write, + err: &mut dyn Write, +) -> Result { + writeln!( + out, + "land: {from} asked this holder to stand down; releasing at the lap boundary rather than mid-matrix" + )?; + unwind_lap(root, branch, pipeline, entered, None, out, err)?; + Ok(ExitCode::Internal) +} + /// Record that the speculative range reached the remote. /// /// **THE ONE WRITER OF [`speculation::Bet::pushed`], which had none** (review of @@ -9776,10 +9840,10 @@ fn run_land_wait( branch: &str, out: &mut dyn Write, err: &mut dyn Write, -) -> Result<(ExitCode, Option)> { +) -> Result<(ExitCode, Option, Option)> { let Ok(sha) = git::head_commit(root) else { writeln!(err, "::error:: land: cannot read this clone's HEAD")?; - return Ok((ExitCode::Internal, None)); + return Ok((ExitCode::Internal, None, None)); }; let required = std::env::var("CI_REQUIRED_CHECKS").unwrap_or_default(); let roster = checks_green::Roster { @@ -9798,7 +9862,7 @@ fn run_land_wait( // would be a hang whose cause is a typo. if let Err(problem) = checks_green::decide(&[], &roster) { writeln!(err, "::error:: land wait: {problem}")?; - return Ok((ExitCode::Usage, None)); + return Ok((ExitCode::Usage, None, None)); } // The base as this clone last saw it. The wait is asking whether the REMOTE @@ -9822,7 +9886,7 @@ fn run_land_wait( err, "::error:: land wait: {tracking} will not resolve, so this wait has no base to judge staleness against and would race with one arm" )?; - return Ok((ExitCode::Internal, None)); + return Ok((ExitCode::Internal, None, None)); }; let config = pr_watch::Config { @@ -9843,7 +9907,7 @@ fn run_land_wait( err, "::error:: land wait: no repository resolved, so every check-run read would 404 — set $GH_REPO, or run this in a clone whose remote names one" )?; - return Ok((ExitCode::Usage, None)); + return Ok((ExitCode::Usage, None, None)); } // A COUNT, never a deadline (CLOUD-1177). The default is generous because // the cost of too many asks is a few conditional requests the forge answers @@ -9883,8 +9947,39 @@ fn run_land_wait( land::Waited::Unanswered => (land::answers(&sha, None, None), ExitCode::Internal), }; land::record_wait(root, branch, &answers)?; + say_what_the_wait_saw(&waited, &sha, reference, asks, out, err)?; + // THE READING TRAVELS WITH THE CODE, because the exit table cannot carry it: + // a stale base and an unanswered wait are both a lap, and only one of them + // took a checks reading at all. Deriving the tap's verdict from the code in + // the driver would be a second authority over an answer this function holds. + // + // AND SO DOES A PEER'S NOTICE, for the same reason and one of its own: the + // heartbeat is scoped to THIS wait, so a request latched inside it dies here + // unless it is handed back. The driver honours it at the lap boundary, which + // is the only place releasing costs nothing. + Ok(( + code, + land::tap_verdict(&waited), + holding.was_asked_to_stand_down(), + )) +} - match &waited { +/// What the wait saw, said once, in the channel each reading belongs in. +/// +/// Split out of [`run_land_wait`] because the two do different jobs: that one +/// decides and records, this one reports. They were one function until the +/// reporting arms grew past the line ceiling, and the ceiling was right — a +/// decision and its narration are separable, and separating them is what keeps a +/// new arm from being added to the wrong one. +fn say_what_the_wait_saw( + waited: &land::Waited, + sha: &str, + reference: &str, + asks: u32, + out: &mut dyn Write, + err: &mut dyn Write, +) -> Result<()> { + match waited { land::Waited::Green { .. } => { writeln!(out, "land: {sha} is green; the loser was voided unread")?; } @@ -9926,11 +10021,7 @@ fn run_land_wait( writeln!(out, "land: no answer yet on {sha} after {asks} ask(s)")?; } } - // THE READING TRAVELS WITH THE CODE, because the exit table cannot carry it: - // a stale base and an unanswered wait are both a lap, and only one of them - // took a checks reading at all. Deriving the tap's verdict from the code in - // the driver would be a second authority over an answer this function holds. - Ok((code, land::tap_verdict(&waited))) + Ok(()) } /// The lap's landing-lease heartbeat, paced by the terms it read once. @@ -9976,6 +10067,18 @@ struct Heartbeat<'clone> { git_dir: Option, pid: u32, last: std::sync::atomic::AtomicI64, + /// Set once a peer has asked this holder to stand down, naming the peer. + /// + /// **A LATCH, not a level.** The beat re-mints the body every few seconds and + /// a peer's notice could be overwritten by a later writer, so a caller that + /// re-read the live value at the lap boundary could miss a request that was + /// made and then lost. Once asked, this holder has been asked. + /// + /// Written from inside the wait's poll and read at the lap boundary, which is + /// why it is a shared cell rather than a field on the lap: the two live in + /// different places and the whole point is that the request crosses between + /// them. + stood_down: std::sync::Mutex>, } impl<'clone> Heartbeat<'clone> { @@ -9986,9 +10089,21 @@ impl<'clone> Heartbeat<'clone> { git_dir: crate::git::git_dir(root).ok(), pid: std::process::id(), last: std::sync::atomic::AtomicI64::new(0), + stood_down: std::sync::Mutex::new(None), } } + /// Who asked this holder to stand down, if anybody has. + /// + /// A poisoned lock reads as "nobody asked", which is the fail-open this whole + /// struct takes: the notice is advisory, so losing one costs a holder that + /// keeps its lease until TTL or completion — the behaviour before the notice + /// existed. Failing closed here would let a panic anywhere in the poll release + /// a healthy lease. + fn was_asked_to_stand_down(&self) -> Option { + self.stood_down.lock().ok()?.clone() + } + /// This lap's own progress token, or `None` when the registry has nothing to /// read — which [`lease::progress_of`] documents as the honest answer for a /// lap whose bookkeeping never registered, not as evidence of a stall. @@ -10035,9 +10150,16 @@ impl<'clone> Heartbeat<'clone> { ); } let progress = self.progress(); - // Bound rather than dropped: `beat` answers a `bool`, and dropping a - // `Copy` is a lint of its own. - let _renewed = lease::beat(self.root, terms, progress.as_deref(), now); + // THE ONE READER OF A PEER'S NOTICE, because the beat is the only thing + // that looks at the lease every few seconds. Latched here and honoured at + // the lap boundary — never here, where a release would drop the lease + // inside this holder's own matrix and let a second lander start. + if let lease::Beat::StandDown(from) = + lease::beat(self.root, terms, progress.as_deref(), now) + && let Ok(mut asked) = self.stood_down.lock() + { + asked.get_or_insert(from); + } } } From c38c7cc62c5aeb78d05ff0df72c65160f7c6ea81 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Fri, 11 Sep 2026 03:04:32 +0000 Subject: [PATCH 16/18] docs(memory): the record layer, the clock decomposition, and why not vector clocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fallacy table was seven rows and one of them was wrong. Bandwidth and transport cost were merged, which deleted the fallacy this project is most directly a response to; the reliability row said "every read fails open", which the tree contradicts exactly where it matters — `Live::decide` fails CLOSED because failing open there makes a network blip the thing that lands somebody else's work, while `authorises` fails open because a lease it cannot read would stop the whole fleet. Same read, opposite directions, decided by the question. The replacement commitment is per (read, decision) pair. The record layer is new. What we rent from the forge — CAS atomicity, rollback protection, admission control — against what we only assumed: any binding between a body and the writer who wrote it. The forge authenticates the push, never the record. Self-certifying addresses cover the immutable half and git gives us that for free; BEP 44 covers the mutable half and we have it nowhere. The Sybil half of the DHT toolkit is not needed here because the writer population is not open. Replay is where the analogy runs out: signatures do not stop a rollback, and the sequence has to anchor in a ref with server-side rollback protection because our readers are containers with no memory. And the clocks, which is what the fallacy row was really about. `now_unix` is CLOCK_REALTIME and answers 0 on failure, which makes `expired` say nothing is ever expired. But no clock choice fixes the real defect: an absolute instant crosses the wire and is compared against another agent's predicate. Expiry must be observer-local, and the progress token beside it already is. Two jobs, one of which needs real time: ordering is the log index, and failure detection irreducibly needs a timeout that can only be confined, never removed. Vector clocks recorded as considered-and-declined, with the one case that would require them — independent logs under a multi-address design. Refs: CLOUD-1778 Refs: CLOUD-1784 --- .../memories/decision/landing-architecture.md | 208 +++++++++++++++++- 1 file changed, 199 insertions(+), 9 deletions(-) diff --git a/.serena/memories/decision/landing-architecture.md b/.serena/memories/decision/landing-architecture.md index ef7f21f70..b22583552 100644 --- a/.serena/memories/decision/landing-architecture.md +++ b/.serena/memories/decision/landing-architecture.md @@ -236,14 +236,204 @@ missing half.** claim traces to a misdiagnosed credential failure. See `mem:evidence-hierarchy` and CLOUD-416. +## The record layer: what we rent from the forge, and what we must provide + +World-writable means **shared address space**, and the honest question is which +security properties we are renting and which we only assumed. + +- **Rented**: CAS atomicity on ref update; rollback protection on protected refs; + admission control (who may write the repo at all). +- **NOT rented, and currently absent**: any binding between a **body** and the + **writer who wrote it**. The forge authenticates the PUSH, never the RECORD. Its + ACL says "can write this repo" and we have been reading it as "can write this + agent's ref." + +**"Accident, not malice" was the wrong frame** and it is worth naming as an error +rather than quietly dropping. At the record layer a stale agent writing a wrong +body and an attacker writing a wrong body are the SAME EVENT: a verifier cannot +tell them apart and does not need to. The split does no work, and it was being +used to justify less mechanism. + +### DHTs solve exactly this, and they split on mutability + +Vanilla Kademlia has no authentication and is open to storage poisoning. What +shipped in real systems is two constructions: + +1. **Immutable → self-certifying address.** `key = H(value)` (BitTorrent + info-hash, IPFS CID). Fetch from `K`, hash what came back, discard on + mismatch. Tamper-evident with **zero key material**. +2. **Mutable → the address is derived from a public key.** BEP 44 mutable items, + IPNS: `addr = H(pubkey [+ salt])`, body signed, plus a monotonic `seq`; + readers reject a bad signature or a non-increasing sequence. **No PKI, no CA, + no key distribution** — the address IS the identity. That is the whole trick. +3. S/Kademlia adds Sybil resistance (crypto puzzles on node ids, disjoint lookup + paths). **We do not need that half**: the writer population is not open, the + forge ACL already bounds it. This is the precise content of the "no Byzantine + fault" advantage — we need DHT **data authentication**, not DHT **admission + control**. + +### Mapping — git gives construction 1 for free and construction 2 nowhere + +| structure | mutability | have | gap | +| --------------------- | ---------------------- | ------------------------------------------ | ------------------------ | +| `refs/sessions/` | immutable | **self-certifying** — ref name IS the hash | none | +| the log branch | append-only | **Merkle chain** — entries name parents | rewind | +| per-agent ref | mutable, single-writer | nothing | **the whole BEP 44 gap** | +| the lease | mutable, multi-writer | CAS | attribution | + +Per-agent refs are BEP 44 verbatim: an ephemeral keypair per session, ref at +`refs/batten/agent/`, body signed over `(seq, payload)`, readers +verify-or-discard — and a discarded record folds into the membership rule already +committed (_derived from observation with expiry_), so an unverifiable agent reads +as absent. **Fail-closed on content, fail-open on availability.** It also kills a +defect class for free: two agents cannot collide on an id and none can squat +another's address. + +For the **lease**, signing changes nothing about eviction — that stays a fenced +CAS, safe under false suspicion. What it buys is that **you cannot write a body +falsely naming another session as holder**, and the fencing token becomes +attributable. + +### Where the analogy runs out: replay + +**Signatures do not stop rollback.** An old signed body is perfectly valid; +force-pushing a ref back to it passes every check. BEP 44 stops this with a `seq` +**readers remember** — DHTs can because many nodes do, which is precisely the +quorum rejected above (no peer transport, ephemeral containers). Our readers are +reclaimable containers with no memory. + +So monotonicity must anchor in **a ref with server-side rollback protection** +(branch protection forbidding force-push), and ultimately trunk. We borrow the +forge's rollback protection for monotonicity the same way we borrow its CAS for +consensus — and that dependency is a **declared forge capability**, because forge +behaviour demonstrably varies by namespace. + +CLOUD-877 already asks for this construction (_"give it a portable signed form"_). +CLOUD-591 decided not to sign COMMITS — a different surface, orthogonal, the way +rule 8 splits trailer from committer. + ## Deutsch's fallacies, as commitments to check against -| Fallacy | Commitment | -| ----------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| The network is reliable | Every read fails open; no fleet-binding decision rests on one failed read; every write is CAS-or-retry. | -| Latency is zero | **No wall clocks anywhere** — counts and supplied instants only. The epoch design keeps this by making expiry an EVENT, not a duration. | -| Bandwidth / transport is free | The sharded and log designs make this WORSE and must pay for it: ref advertisement is O(refs), and a 79,973-byte advertisement is already on record. Reaper plus a measured ceiling, or the fix is the next defect. | -| The network is secure | World-writable by construction; the threat is accident, not malice. Trunk stays sacred: the referee re-derives rather than trusting a request. | -| Topology doesn't change | Agents vanish without deregistering. Membership is derived from observation with expiry, never from a registry needing clean exit. | -| There is one administrator | **False by construction** — agents run different binary versions. Every ref body carries a schema major; readers ignore unknown keys, reject an unknown major, and ship a release before writers. | -| The network is homogeneous | Six harnesses, three environment classes. No environment's quirk may reach `crates/batten` (rule 1). | +**Eight, not seven.** An earlier version merged _bandwidth is infinite_ with +_transport cost is zero_. They are different, and merging them hid the one this +project is most directly a response to: transport cost is the entire economic +premise (serial metered trunk, parallel free agents). + +| # | Fallacy | Commitment | Mechanism | +| --- | -------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------- | +| 1 | The network is reliable | **Name both errors and fail toward the cheaper one, per (read, decision) pair.** Never a blanket direction — see below. | partial, per call site | +| 2 | Latency is zero | No wall clock is READ inside a decision; instants are supplied. **But expiry IS a clock, and cross-agent skew is assumed and unmeasured.** | `clippy.toml` sleep ban, `tests/sleep_ban.rs`; skew: **none** | +| 3 | Bandwidth is infinite | Ref advertisement is O(total refs); 79,973 bytes already on record. Per-agent refs and transcripts must not ride it. | **none** — needs `ls-refs` `ref-prefix` scoping | +| 4 | The network is secure | Every mutable record carries a signature verifiable against its own address, plus a sequence anchored in a rollback-protected ref; a reader discards what it cannot verify. | **none yet** — CLOUD-877 | +| 5 | Topology doesn't change | Membership derived from observation, never a registry needing clean exit — **and a vanished agent's leavings are recoverable.** | `Bet::recovered`; orphan refs **CLOUD-1747, open** | +| 6 | There is one administrator | Every body carries a schema major; readers reject an unknown one and ignore unknown keys. | `Body::schema` **landed**; "readers before writers" **no gate** | +| 7 | Transport cost is zero | **The premise, not a caveat.** Trunk serial and metered, agents parallel and free; speculation is pipelining, the lock is linearization. | the speculation design; metrics **none** | +| 8 | The network is homogeneous | No host's quirk reaches `crates/batten` — **and no forge's either.** | rule 1 **enforced**; forge capabilities **prose only** | + +### 1 was actively wrong, and the wrong version would license a real bug + +It read _"every read fails open."_ The tree contradicts that where it matters. +`Live::decide` fails **closed** — _"failing open there would make a network blip +the thing that lands somebody else's work"_ — while `authorises` fails **open** — +_"a lease it cannot read stops EVERY job in the fleet, where waving one matrix +through costs one matrix."_ + +Same read, opposite directions, because the **question** differs. The direction is +a property of the (read, decision) pair, structurally the same argument +`rules/scanning.md` makes for decider-vs-floor being relational and therefore not +expressible as a column. So: at every could-not-look, state the two errors and +their costs and fail toward the cheaper — and a new call site owes that sentence. + +The old second clause — _"no fleet-binding decision rests on one failed read"_ — +**is not implemented.** `authorises` reads once. A gap, not a property. + +### 2's real gap is clocks, and it is worse than skew + +The sleep ban is real and enforced. The clock underneath is not. + +**`now_unix()` is `SystemTime::now()` — `CLOCK_REALTIME`, not monotonic.** NTP +step, VM resume and container migration all move it backwards, so a holder that +beats across a backward step re-mints an EARLIER `expires` than the one already +published. **And it answers `0` when the clock will not read**, which makes +`expired(now)` (`now >= expires`) say nothing is ever expired — a clock failure +wedges the whole fleet rather than degrading. `expires == 0` is separately the +tombstone sentinel (`released()`), so zero carries two meanings. + +**But no clock choice fixes the real defect.** `expired(now)` compares MY `now` +against YOUR `expires` — an absolute timestamp crossing the wire — and monotonic +clocks are not comparable across machines either. **Expiry must never cross the +wire.** The correct shape is observer-local staleness: B observes the lease at its +own monotonic instant with progress token `P`; if after B's own elapsed ΔT the +token is still `P`, B may judge the holder stalled. B never reads A's clock. + +**That mechanism already exists beside it.** `progress` is opaque and compared +_"for EQUALITY OVER TIME… so no clock crosses the wire"_. So `expires` is largely +redundant with its neighbour and is the half carrying every hazard. The change: +**promote the progress token to the primary liveness signal, demote expiry to a +backstop, and if a timeout is kept make it an `Instant` measured locally** — never +a timestamp shipped in a body. + +### Clocks do two jobs and only one needs real time + +- **Ordering and validity** — "is this record newer", "is my bet against the + current epoch" → **logical clock: the log index.** No real time at all. +- **Failure detection** — "is the holder dead" → **irreducibly needs a timeout.** + FLP: a crash cannot be detected without one, and no logical clock answers + liveness. This part cannot be designed away, only confined. + +### Why NOT vector clocks + +Considered and declined, so nobody reopens it cold — and recorded because the +first version of this file did not consider them at all. + +1. **They reconstruct a partial order where there is no serialization point. We + have one.** Every fleet event goes through a CAS on a single ref. Same shape as + the Paxos argument above: a vector relates events across independent replicas, + and we deliberately have one totally-ordered log. **The log index is already a + Lamport clock and the total order makes a vector strictly redundant.** +2. **O(N) entries with unbounded, churning N.** Agents are reclaimable containers; + vector-clock GC for departed processes is unsolved, so entries for dead agents + accumulate in every body forever — straight into fallacy 3. +3. **They DETECT concurrency; the lease must PREVENT it.** Learning after the fact + that two writes were concurrent gives no mutual exclusion. `--force-with-lease` + refuses the loser; CAS is strictly stronger than detection for this job. +4. **On single-writer sharded refs a vector degenerates to a scalar** — no + concurrency to relate, so a per-writer sequence is exactly sufficient, which is + what BEP 44 uses. + +**The case where a vector IS required, stated as a constraint on the multi-address +design:** N independent CAS addresses means N independent logs and no single total +order, and relating events across them genuinely needs a vector with its GC cost. +So: **one log → a scalar index suffices; multiple independent logs → vector clocks +and their costs.** An argument for one log, or for declaring separate logs +causally independent and never compared. + +The sleep ban is real and enforced. But `Body::expires` is an absolute instant and +`expired(now)` compares against one, so the liveness model **is** a wall clock — +merely supplied across the boundary rather than read inside. It assumes bounded +skew and nothing measures it: a holder with a slow clock holds past TTL while a +rival with a fast clock steals mid-matrix, which is **two landers** — the exact +failure the lease exists to prevent. + +The skew-free primitive already exists: `progress` is opaque and compared for +EQUALITY OVER TIME, so no clock crosses the wire. That is a logical clock. +**Promote it to the primary liveness signal and demote expiry to a backstop**, and +meanwhile advertise each agent's clock beside its version so skew is measured +rather than assumed. + +### 3 — and the transcript design above violates it + +"Keep it out of the lap's fetch path" is not enough: **ref advertisement is not +fetch.** The v0/v1 handshake lists every ref whatever you fetch, so transcripts — +the largest objects in the system — would bloat every lease read. The fix is +protocol v2 `ls-refs` with `ref-prefix`, scoping the advertisement server-side. + +### 5, 6, 8 each have a named unbuilt half + +A reclaimed container leaves a held lease, a readied PR, a burning matrix and +stranded `refs/batten-spec/*` (CLOUD-1747). _Readers ship before writers_ is a +process promise with no gate, which rule 2 calls half a change — `Body::writer` is +the mechanism, refusing to raise the major until the observed fleet minimum is +high enough. And rule 1 governs hosts, not **forges**: fast-forward enforcement was +measured differing by namespace on one forge, so declared forge capabilities need +to be a table. From dceb9f8dd7258b764e1e2a30b1d4a05a7ffe0dec Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Fri, 11 Sep 2026 03:23:17 +0000 Subject: [PATCH 17/18] fix(semver): route every spawn through the resolution ladder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five sites reached `Command::new` with the program named directly, so rung 0 — the pin — never fired. `policy/spawn-adapters.rego` places this module, which sanctions the SPAWN; it says nothing about how the program is RESOLVED, and that gap is what let the bypass sit here. `rules.rs` already measured the cost: "nothing a toolchain manager provides is on bare PATH -- nor should it be". So a `cargo` the project pins was reached around every time and whatever the ambient environment exposed was used instead. `toolchain()` was the worst of the five, and its own doc says why: it calls itself a READ of the pin rather than a fourth copy of the number. It read that pin off a `rustc` resolved on bare PATH — a different compiler from the one the project pins — so the authority was whichever toolchain the environment happened to expose. The `+{toolchain}` ordering survives. That rule is about cargo's own argv, and where rung 3 fires the program becomes an interpreter while `extra` carries the script, so `+{toolchain}` is still the first argument cargo itself sees. The baseline doc build resolves against `root` rather than the materialized worktree, which carries no toolchain configuration of its own and would fall through to bare PATH — the same bypass through a different door. A case asserts no spawn in the module names its program directly. It took three attempts, and the failures are the argument for the idiom rather than noise: the first found the call spelled in its own doc comment, the second found its own search literal in executable code. `git.rs` already assembles its needle from parts for exactly this reason, undocumented, so each new scanner rediscovered the defect instead of the remedy. That is now written down in rules/scanning.md. Refs: CLOUD-1494 --- crates/batten/src/semver.rs | 328 +++++++++++++++++++++++------------- rules/scanning.md | 29 ++++ 2 files changed, 240 insertions(+), 117 deletions(-) diff --git a/crates/batten/src/semver.rs b/crates/batten/src/semver.rs index 7639f0750..9c64bd4d0 100644 --- a/crates/batten/src/semver.rs +++ b/crates/batten/src/semver.rs @@ -218,22 +218,36 @@ pub fn against_rev( baseline: &str, release_type: &str, ) -> Option { - let output = std::process::Command::new(ANALYSER) - .arg(format!("+{toolchain}")) - .args(["semver-checks", "check-release"]) - .args(["--package", package]) - .args(["--baseline-rev", baseline]) - .args(["--release-type", release_type]) - // Overriding whatever the caller's environment set, and load-bearing rather than - // cosmetic: the report below is PARSED, and a gate that parses colour is - // CLOUD-199's defect — an anchored pattern that can never match because - // escape sequences sit between the anchor and the word. - .env("CARGO_TERM_COLOR", "never") - .current_dir(root) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .output() - .ok()?; + // THROUGH THE LADDER, never `Command::new` directly (CLOUD-1494). The adapter + // placement above sanctions the SPAWN; it says nothing about how the program + // is resolved, and resolving it here bypassed rung 0 — the pin. `rules.rs` + // measured that exact shape: *"nothing a toolchain manager provides is on + // bare `PATH` -- nor should it be"*, so a `cargo` the project pins was + // reached around every time and an unpinned one silently used instead. + // + // `extra` goes ahead of `+{toolchain}` and that is still correct. The + // ordering rule below is about cargo's OWN argv; where rung 3 fires the + // program becomes an interpreter and `extra` carries the script, so + // `+{toolchain}` remains the first argument cargo itself sees. + let output = crate::rules::spawn_resolving(Some(root), ANALYSER, |program, extra| { + std::process::Command::new(program) + .args(extra) + .arg(format!("+{toolchain}")) + .args(["semver-checks", "check-release"]) + .args(["--package", package]) + .args(["--baseline-rev", baseline]) + .args(["--release-type", release_type]) + // Overriding whatever the caller's environment set, and load-bearing rather than + // cosmetic: the report below is PARSED, and a gate that parses colour is + // CLOUD-199's defect — an anchored pattern that can never match because + // escape sequences sit between the anchor and the word. + .env("CARGO_TERM_COLOR", "never") + .current_dir(root) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + }) + .ok()?; Some(Compared { code: output.status.code(), report: merged(&output), @@ -255,32 +269,38 @@ pub fn against_rustdoc( current: Option<&Path>, release_type: &str, ) -> Option { - let mut command = std::process::Command::new(ANALYSER); - // `+toolchain` FIRST, always: cargo reads it as argv[1] and nowhere else, so - // an option pushed ahead of it silently runs the default toolchain — the - // failure that has no symptom until a version-dependent build breaks. - command - .arg(format!("+{toolchain}")) - .args(["semver-checks", "check-release"]) - .args(["--package", package]) - .arg("--baseline-rustdoc") - .arg(rustdoc); - // `--current-rustdoc` only when one was built. Absent, the tool generates the - // head side itself through the scratch resolve — which is the path CLOUD-1399 - // measured failing, so this is the arm that matters here; it stays optional - // because a caller that could not build the head side is still better served - // by the tool's own generation than by no comparison at all. - if let Some(current) = current { - command.arg("--current-rustdoc").arg(current); - } - let output = command - .args(["--release-type", release_type]) - .env("CARGO_TERM_COLOR", "never") - .current_dir(root) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .output() - .ok()?; + // Through the ladder, for `against_rev`'s reason (CLOUD-1494). + let output = crate::rules::spawn_resolving(Some(root), ANALYSER, |program, extra| { + let mut command = std::process::Command::new(program); + // `+toolchain` FIRST, always: cargo reads it as argv[1] and nowhere else, so + // an option pushed ahead of it silently runs the default toolchain — the + // failure that has no symptom until a version-dependent build breaks. + // `extra` is the ladder's own prefix and precedes it, which keeps + // `+{toolchain}` first in the argv cargo itself receives. + command + .args(extra) + .arg(format!("+{toolchain}")) + .args(["semver-checks", "check-release"]) + .args(["--package", package]) + .arg("--baseline-rustdoc") + .arg(rustdoc); + // `--current-rustdoc` only when one was built. Absent, the tool generates the + // head side itself through the scratch resolve — which is the path CLOUD-1399 + // measured failing, so this is the arm that matters here; it stays optional + // because a caller that could not build the head side is still better served + // by the tool's own generation than by no comparison at all. + if let Some(current) = current { + command.arg("--current-rustdoc").arg(current); + } + command + .args(["--release-type", release_type]) + .env("CARGO_TERM_COLOR", "never") + .current_dir(root) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + }) + .ok()?; Some(Compared { code: output.status.code(), report: merged(&output), @@ -312,13 +332,21 @@ pub fn toolchain(root: &Path) -> Option { { return Some(named); } - let output = std::process::Command::new("rustc") - .arg("--version") - .current_dir(root) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::null()) - .output() - .ok()?; + // Through the ladder (CLOUD-1494), and this one is the site where bypassing it + // was most clearly wrong: the doc above says the pin is READ rather than + // restated, and reading it off a `rustc` resolved on bare `PATH` reads a + // DIFFERENT compiler from the one the project pins — so the "authority" was + // whichever toolchain the ambient environment happened to expose. + let output = crate::rules::spawn_resolving(Some(root), "rustc", |program, extra| { + std::process::Command::new(program) + .args(extra) + .arg("--version") + .current_dir(root) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()) + .output() + }) + .ok()?; let text = String::from_utf8_lossy(&output.stdout).into_owned(); let version = text.split_whitespace().nth(1)?; (!version.is_empty()).then(|| version.to_owned()) @@ -383,43 +411,51 @@ pub fn baseline_rustdoc( // this whole gate exists against. crate::git::materialize_rev(root, baseline, &worktree) .map_err(|err| format!("the baseline tree could not be materialized: {err}"))?; - let built = std::process::Command::new(ANALYSER) - .arg(format!("+{toolchain}")) - .args([ - "doc", - "--locked", - "--no-deps", - "--lib", - "--package", - package, - ]) - .env("RUSTC_BOOTSTRAP", "1") - .env( - "RUSTDOCFLAGS", - "-Z unstable-options --output-format json --document-private-items", - ) - .env("CARGO_TARGET_DIR", &target) - .env("CARGO_TERM_COLOR", "never") - // THE OUTER CARGO'S ENVIRONMENT IS NOT THIS BUILD'S. When the binary - // itself is launched through `cargo run`, cargo exports its own manifest - // and toolchain into the child, and a nested `cargo doc` reads them as - // instructions about a package that is not the one in front of it. Every - // one is removed rather than overridden, because overriding requires - // knowing the whole set and removal does not. - .env_remove("CARGO") - .env_remove("CARGO_MANIFEST_DIR") - .env_remove("CARGO_MANIFEST_PATH") - .env_remove("CARGO_PKG_NAME") - .env_remove("CARGO_PKG_VERSION") - .env_remove("CARGO_MAKEFLAGS") - .env_remove("RUSTC") - .env_remove("RUSTDOC") - .env_remove("RUSTUP_TOOLCHAIN") - .current_dir(&worktree) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .output() - .map_err(|err| format!("the baseline doc build could not be run: {err}"))?; + // Through the ladder, for `against_rev`'s reason (CLOUD-1494). Resolved + // against `root` rather than the scratch worktree: the pin is the PROJECT's, + // and a materialized baseline tree carries no toolchain configuration of its + // own — asking it would resolve nothing and fall through to bare `PATH`, + // which is the bypass this change closes. + let built = crate::rules::spawn_resolving(Some(root), ANALYSER, |program, extra| { + std::process::Command::new(program) + .args(extra) + .arg(format!("+{toolchain}")) + .args([ + "doc", + "--locked", + "--no-deps", + "--lib", + "--package", + package, + ]) + .env("RUSTC_BOOTSTRAP", "1") + .env( + "RUSTDOCFLAGS", + "-Z unstable-options --output-format json --document-private-items", + ) + .env("CARGO_TARGET_DIR", &target) + .env("CARGO_TERM_COLOR", "never") + // THE OUTER CARGO'S ENVIRONMENT IS NOT THIS BUILD'S. When the binary + // itself is launched through `cargo run`, cargo exports its own manifest + // and toolchain into the child, and a nested `cargo doc` reads them as + // instructions about a package that is not the one in front of it. Every + // one is removed rather than overridden, because overriding requires + // knowing the whole set and removal does not. + .env_remove("CARGO") + .env_remove("CARGO_MANIFEST_DIR") + .env_remove("CARGO_MANIFEST_PATH") + .env_remove("CARGO_PKG_NAME") + .env_remove("CARGO_PKG_VERSION") + .env_remove("CARGO_MAKEFLAGS") + .env_remove("RUSTC") + .env_remove("RUSTDOC") + .env_remove("RUSTUP_TOOLCHAIN") + .current_dir(&worktree) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + }) + .map_err(|err| format!("the baseline doc build could not be run: {err}"))?; if !built.status.success() { return Err(format!( "the baseline doc build failed: {}", @@ -482,37 +518,41 @@ pub fn current_rustdoc( .canonicalize() .map_err(|err| format!("the current scratch directory could not be resolved: {err}"))? .join("target"); - let built = std::process::Command::new(ANALYSER) - .arg(format!("+{toolchain}")) - .args([ - "doc", - "--locked", - "--no-deps", - "--lib", - "--package", - package, - ]) - .env("RUSTC_BOOTSTRAP", "1") - .env( - "RUSTDOCFLAGS", - "-Z unstable-options --output-format json --document-private-items", - ) - .env("CARGO_TARGET_DIR", &target) - .env("CARGO_TERM_COLOR", "never") - .env_remove("CARGO") - .env_remove("CARGO_MANIFEST_DIR") - .env_remove("CARGO_MANIFEST_PATH") - .env_remove("CARGO_PKG_NAME") - .env_remove("CARGO_PKG_VERSION") - .env_remove("CARGO_MAKEFLAGS") - .env_remove("RUSTC") - .env_remove("RUSTDOC") - .env_remove("RUSTUP_TOOLCHAIN") - .current_dir(root) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .output() - .map_err(|err| format!("the current doc build could not be run: {err}"))?; + // Through the ladder, for `against_rev`'s reason (CLOUD-1494). + let built = crate::rules::spawn_resolving(Some(root), ANALYSER, |program, extra| { + std::process::Command::new(program) + .args(extra) + .arg(format!("+{toolchain}")) + .args([ + "doc", + "--locked", + "--no-deps", + "--lib", + "--package", + package, + ]) + .env("RUSTC_BOOTSTRAP", "1") + .env( + "RUSTDOCFLAGS", + "-Z unstable-options --output-format json --document-private-items", + ) + .env("CARGO_TARGET_DIR", &target) + .env("CARGO_TERM_COLOR", "never") + .env_remove("CARGO") + .env_remove("CARGO_MANIFEST_DIR") + .env_remove("CARGO_MANIFEST_PATH") + .env_remove("CARGO_PKG_NAME") + .env_remove("CARGO_PKG_VERSION") + .env_remove("CARGO_MAKEFLAGS") + .env_remove("RUSTC") + .env_remove("RUSTDOC") + .env_remove("RUSTUP_TOOLCHAIN") + .current_dir(root) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .output() + }) + .map_err(|err| format!("the current doc build could not be run: {err}"))?; if !built.status.success() { return Err(format!( "the current doc build failed: {}", @@ -666,6 +706,60 @@ mod tests { } } + /// **EVERY SPAWN HERE GOES THROUGH THE LADDER** (CLOUD-1494). + /// + /// `policy/spawn-adapters.rego` places this module, which sanctions the + /// SPAWN — it says nothing about how the program is RESOLVED, and that gap is + /// what let five sites reach `Command::new(ANALYSER)` directly and skip rung + /// 0. `rules.rs` measured the cost of exactly that: *"nothing a toolchain + /// manager provides is on bare `PATH` -- nor should it be"*, so a pinned + /// `cargo` was reached around every time and whatever the ambient environment + /// exposed was used instead. `toolchain()` was the worst of the five: its own + /// doc calls itself a READ of the pin, and it read a different compiler. + /// + /// Asserted over this file's own text because the property is syntactic — + /// the argument to `Command::new` — and no exit code reaches it. The shape is + /// `git.rs:5127`'s, which greps this crate for `Command::new("git")` and + /// fails on a hit; here the refusal is narrower, since the spawn is legitimate + /// and only the unresolved program is not. + #[test] + fn no_spawn_in_this_module_names_its_program_directly() { + // COMMENT LINES ARE DROPPED FIRST, and this file's own first draft is why. + // The prose above names the defect in its own words, so a scan over raw + // text found the call spelled in a COMMENT and reported it as a live site. + // `rules/scanning.md` row two is exactly this — a syntax question answered + // with a text scanner — and CLOUD-843 measured the same class, where + // `ci-local-parity` landed in the wrong bucket because the string appeared + // in a comment. A line filter is the cheap half of the right instrument; + // it is sound here because every real call sits in command position on a + // line of its own. + let code: String = include_str!("semver.rs") + .lines() + .filter(|line| !line.trim_start().starts_with("//")) + .collect::>() + .join("\n"); + // THE NEEDLE IS ASSEMBLED, never written whole, and `git.rs`'s own + // crate-wide scan does the same for the same reason: a scanner spelled + // literally matches ITSELF, so the first version reported its own search + // string as a defect. Stripping comments was not enough — this one is in + // executable code. + let needle = ["Command", "::new("].concat(); + for site in code.match_indices(needle.as_str()) { + let (_, tail) = code.split_at(site.0 + needle.len()); + // `unwrap_or` rather than `expect`: a call with no closing parenthesis + // is not this test's subject, and an empty argument fails the assertion + // below anyway — so the degenerate parse reports as a finding rather + // than as a panic in the checker. + let argument = tail.split(')').next().unwrap_or_default(); + assert_eq!( + argument, "program", + "a spawn here must take the program the ladder resolved, never a \ + literal or a constant — that skips rung 0 and reaches around a \ + pinned toolchain" + ); + } + } + #[test] fn a_graded_zero_run_is_never_a_pass() { let compared = report(" Checked [ 0.1s] 0 checks: 0 pass, 0 fail\n"); diff --git a/rules/scanning.md b/rules/scanning.md index 5f290bee0..42cd82b93 100644 --- a/rules/scanning.md +++ b/rules/scanning.md @@ -101,6 +101,35 @@ forge, and a command-position pass over the same files, comments stripped, gave forge bucket because the string appeared in a **comment**. Both passes were an hour apart, and the substring one was nearly published as the campaign's scoping. +## A scanner that reads the file it lives in will match itself + +Row one's instrument has one failure mode that belongs to no other row, because +it only arises when the corpus and the scanner are the same file. A presence or +absence test written with `include_str!` over its own module — the shape +`spawn_census.rs` and `scanner_taxonomy.rs` use — is scanning its own source, +and **the needle it searches for is in that source.** + +Measured 2026-09-11, three times in one session, on two different files: + +- A test asserting a refuted claim cannot return **quoted the claim verbatim** in + its own doc, so it found the sentence in its own correction and failed. +- A test asserting no spawn names its program directly **named one in its doc** as + the example of what it forbids, and found that. +- The same test, with comments stripped, then matched **its own search literal** + in executable code. + +Each was the test working exactly as written. `git.rs` already carries the fix +and has for longer than this section: it assembles its needle from parts +(`["Command", "::new(\""].concat()`) so the string never appears whole. The +idiom was undocumented, so three tests rediscovered the defect instead of the +remedy. + +**So, for a scanner over its own file:** assemble the needle rather than spell it, +strip comments when the property is about code, and **paraphrase the thing you +forbid rather than quoting it** — a forbidden or refuted literal sitting in prose +is one a reader lifts out of its context, which is the same reason the lease's +own corrected premise is paraphrased rather than quoted. + ## Row four's subject is not the tree, which is why it kept getting answered from memory The first three questions are about the tree, and the instrument for each is a From 95f80c815354980754dd92ab2511a6f0d7bc45f4 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Fri, 11 Sep 2026 03:23:34 +0000 Subject: [PATCH 18/18] feat(lease): an agent that poisoned CI takes no turn until the trunk moves or the pool idles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing stopped it before. A holder whose matrix goes red releases and may win the very next acquire, replay onto the same trunk and buy another matrix on the same defect — spending the metered resource twice to learn what it already knows, while every waiter that prepped behind it sits through both. THE CONDITION IS TWO EVENTS, NEVER A DURATION. This is the mechanism most likely to have been written as a timer, and both lapse conditions are observable directly, so the no-wall-clock invariant survives it. The trunk moving lapses it because the cooldown is a claim about ONE base — that this branch, on that trunk, is red. Once the trunk advances the claim is about a tree that no longer exists, and holding the agent back further punishes it for a state nobody is in. An idle pool lapses it, and that arm is the anti-outage half rather than a kindness. Waiting exists so somebody else gets the slot; with no lease on the ref there is nobody else, and a cooldown that survived an empty pool would strand a single-agent fleet completely — one red matrix and nothing can ever land again. CLOUD-1043 makes the same argument for its own mechanism and it is preserved here rather than rediscovered. A tombstone counts as idle for the same reason: the holder handed it back, so the ref exists and nobody is holding it. A ref nobody can read counts as NOT idle. Reading a failed parse as an empty pool would let the one agent that just poisoned CI take the lease on the strength of it, and `turn` already answers Wait for that state — the cooldown agrees with its neighbour rather than arguing with it. Refs: CLOUD-1778 --- crates/batten/src/lease.rs | 112 +++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/crates/batten/src/lease.rs b/crates/batten/src/lease.rs index f0d3e465b..1e8daea1d 100644 --- a/crates/batten/src/lease.rs +++ b/crates/batten/src/lease.rs @@ -2502,6 +2502,58 @@ pub fn turn( Turn::Wait } +/// Does a poison cooldown still hold this clone back from taking the lease? +/// +/// **The condition is two EVENTS, never a duration** (CLOUD-1778). An agent whose +/// head turned CI red takes no further turn until either the trunk moves or the +/// pool goes idle — both observable, neither a timer, so the no-wall-clock +/// invariant survives a mechanism that would obviously have been written as one. +/// +/// # Why an agent that poisoned CI must not simply re-take +/// +/// Nothing stops it today. A holder whose matrix goes red releases and may win +/// the very next acquire, replay onto the same trunk and buy another matrix on +/// the same defect — spending the metered resource twice to learn what it already +/// knows, while every waiter that prepped behind it sits through both. +/// +/// # Why the trunk moving lapses it +/// +/// The cooldown is a claim about ONE base: that this branch, on that trunk, is +/// red. When the trunk advances the claim is about a tree that no longer exists, +/// and holding the agent back further would punish it for a state nobody is in. +/// +/// # Why an idle pool lapses it, and this is the anti-outage half +/// +/// Waiting exists so somebody ELSE gets the slot. With no lease on the ref there +/// is nobody else, and a cooldown that survived an empty pool would strand a +/// single-agent fleet completely — one red matrix and nothing can ever land +/// again. `CLOUD-1043`'s releaser clause is the same argument for its own +/// mechanism, and it is preserved here rather than rediscovered. +/// +/// A released lease counts as idle for the same reason: the tombstone says the +/// holder handed it back, so the ref exists but nobody is holding it. +#[must_use] +pub fn cooling(poisoned_at: Option<&str>, trunk_now: &str, observed: &Observed) -> bool { + let Some(at) = poisoned_at else { + return false; + }; + // THE TRUNK MOVED — the claim was about a base that is no longer trunk. + if at != trunk_now { + return false; + } + // THE POOL IS IDLE — nobody is waiting, so waiting buys nothing and would + // strand a single-agent fleet. + match observed { + Observed::Absent => false, + Observed::Held { body, .. } => !body.released(), + // A ref nobody can read is NOT evidence the pool is idle, and reading it + // as idle would let the one agent that just poisoned CI take the lease on + // the strength of a failed parse. `turn` already answers `Wait` for this + // state; the cooldown agrees rather than arguing with it. + Observed::Garbage { .. } => true, + } +} + /// What the holder's own bookkeeping says about whether it is MOVING. /// /// **Liveness answers a different question, and answers it happily for a process @@ -4169,6 +4221,66 @@ mod tests { )); } + /// The poison cooldown lapses on either event and on neither timer + /// (CLOUD-1778). + /// + /// Fails by: making it a duration, which is the obvious implementation and + /// the one the no-wall-clock invariant forbids; or by dropping the idle arm, + /// which strands a single-agent fleet after one red matrix. + #[test] + fn a_poison_cooldown_lapses_when_the_trunk_moves_or_the_pool_idles() { + let held = Observed::Held { + sha: String::from("aaaa"), + body: Body { + holder: String::from("someone-else"), + expires: 1_700_000_060, + ..Body::default() + }, + }; + assert!( + cooling(Some("trunk-1"), "trunk-1", &held), + "same trunk and somebody is holding: the cooldown holds" + ); + assert!( + !cooling(Some("trunk-1"), "trunk-2", &held), + "the trunk moved, so the claim is about a base that is no longer trunk" + ); + assert!( + !cooling(Some("trunk-1"), "trunk-1", &Observed::Absent), + "nobody is waiting, so waiting buys nothing — and a surviving cooldown \ + would strand a single-agent fleet after one red matrix" + ); + assert!( + !cooling( + Some("trunk-1"), + "trunk-1", + &Observed::Held { + sha: String::from("bbbb"), + body: Body { + expires: 0, + ..Body::default() + }, + }, + ), + "a tombstone is the holder handing it back: the ref exists, nobody holds it" + ); + assert!( + !cooling(None, "trunk-1", &held), + "and an agent that poisoned nothing is never held back" + ); + assert!( + cooling( + Some("trunk-1"), + "trunk-1", + &Observed::Garbage { + sha: String::from("cccc"), + why: String::from("not a lease"), + }, + ), + "a ref nobody can read is not evidence the pool is idle" + ); + } + #[test] fn an_expired_lease_is_not_taken_until_its_sha_has_sat_a_beat() { // THE ONE EXTRA BEAT. Expiry is an instant on the HOLDER's clock; the