diff --git a/crates/batten/src/land.rs b/crates/batten/src/land.rs index 134472001..62b1661bf 100644 --- a/crates/batten/src/land.rs +++ b/crates/batten/src/land.rs @@ -1459,6 +1459,45 @@ impl Basis { } } +/// Did THIS clone just redden the metered matrix on a base of its own? +/// +/// The predicate behind the poison cooldown (CLOUD-1797). It is the same cell +/// [`progress_of`] answers `Stop` for, narrowed by the basis, and each part of +/// it is load-bearing: +/// +/// - **`Wait` and `Violation`** is the wait's OWN refusal. Keying on a red +/// reading alone would fire on a wait that succeeded while a stale red sat in +/// `seen` — the defect `progress_of`'s own comment records one cell below. +/// - **`Red`, never `Pending` or `None`.** A could-not-look is not a poisoning, +/// and a cooldown placed on a network blip holds a clone back over nothing. +/// - **`Own`, never `Borrowed`.** On a borrowed base the red may be the +/// speculated base's fault, so charging it here would let a neighbour's bad +/// tree evict an innocent agent — the asymmetry `charge_the_lap` already +/// refuses for the same reason. +/// +/// Pure, and free-standing rather than a branch inside the lap, so the cell can +/// be decided in a test instead of only through a git resolution and a +/// filesystem write. +//MUTANT-SUITE crates/batten/tests/it/lease_lifecycle.rs +//MUTANT poison-never-recorded|s@ matches!(@ !matches!(@|a_red_wait_on_this_clones_own_base_is_what_poisons_it +#[must_use] +pub const fn poisons_this_clone( + step: Step, + code: crate::exit::ExitCode, + seen: Option, + basis: Basis, +) -> bool { + matches!( + (step, code, seen, basis), + ( + Step::Wait, + crate::exit::ExitCode::Violation, + Some(TapVerdict::Red), + Basis::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 0df8b2216..a83d7c700 100644 --- a/crates/batten/src/lease.rs +++ b/crates/batten/src/lease.rs @@ -2309,6 +2309,67 @@ pub fn notice(body: &Body, from: &str) -> Body { } } +/// Should `ours` ask a holder writing `theirs` to stand down? +/// +/// The predicate behind the only sender of [`notice`] (CLOUD-1798). Strictly +/// newer, and **every uncertain reading answers `false`**: +/// +/// - **An equal build is not out-ranked.** Two clones on the same version asking +/// each other to stand down is a request loop with no newer party to win it. +/// - **An unreadable version on either side is not evidence of anything.** A +/// body minted by a build whose version this one cannot parse might be newer; +/// reading it as older would let an old clone evict the whole fleet, which is +/// the inversion [`Body::writer`]'s own doc refuses when it says the field +/// decides nothing on its own. +/// +/// A pre-release or build suffix is cut before the comparison rather than +/// ordered, because the question here is "is the holder behind us" and no +/// decision in this repository turns on `-rc.1` ordering. Saying so is cheaper +/// than implementing an ordering nothing asks for. +#[must_use] +pub fn outranks(ours: &str, theirs: &str) -> bool { + fn triple(version: &str) -> Option<(u64, u64, u64)> { + let core = version.trim().split(['-', '+']).next().unwrap_or_default(); + let mut parts = core.split('.'); + let mut next = || parts.next()?.parse::().ok(); + let (major, minor, patch) = (next()?, next()?, next()?); + // A FOURTH SEGMENT IS NOT A VERSION THIS UNDERSTANDS. Reading `1.2.3.4` + // as `1.2.3` would rank two distinct builds equal. + parts.next().is_none().then_some((major, minor, patch)) + } + match (triple(ours), triple(theirs)) { + (Some(ours), Some(theirs)) => ours > theirs, + _ => false, + } +} + +/// Is it worth `asker` leaving a stand-down request on this body? +/// +/// The three guards of the notice sender as one pure predicate, so the decision +/// is testable without a ref, a CAS or a wire (CLOUD-1798). Each clause is a +/// distinct way to get this wrong: +/// +/// 1. **Strictly newer** — [`outranks`], which refuses an equal build and every +/// unreadable one. +/// 2. **Not already asked** — idempotence. Without it every lap of every waiter +/// rewrites the ref and the holder's own heartbeat CAS fails against a stream +/// of cosmetic updates, evicting by contention rather than by asking. +/// 3. **Never self-addressed** — the mirror of +/// [`stand_down_requested`]'s own guard, one step earlier: that one stops a +/// holder honouring a notice it minted onto itself, this one stops it being +/// written. +/// +/// A released or expired body is not excluded here and does not need to be: the +/// caller only reaches this on `Turn::Wait`, which those states never produce. +//MUTANT-SUITE crates/batten/tests/it/lease_lifecycle.rs +//MUTANT notice-ignores-the-writer|s@ outranks(\&writer_version(), \&body.writer)@ true@|a_waiter_asks_only_a_strictly_older_holder_to_stand_down +#[must_use] +pub fn worth_asking(body: &Body, asker: &str) -> bool { + outranks(&writer_version(), &body.writer) + && body.stand_down.trim().is_empty() + && body.holder != asker +} + /// 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 @@ -2398,6 +2459,62 @@ impl Local { Ok(minted) } + /// Record that THIS clone's head reddened the metered matrix on `trunk`. + /// + /// The written value is a trunk POSITION, never an instant, and that is the + /// whole reason [`cooling`] can decide without a clock: the cooldown lapses + /// when the trunk moves off this sha or when the pool goes idle, both of them + /// observable events (CLOUD-1784's decomposition, CLOUD-1797's wiring). + /// + /// Beside `holder` in the same per-clone directory, because it is the same + /// kind of fact — something this clone knows about itself that must outlive + /// the process, since the `land` that reddened CI and the `acquire` that must + /// be held back are different processes. + /// + /// # THE RECORD NAMES ITS OWN REF, and that is not decoration + /// + /// It stores `\t`, so the reader resolves the ref the + /// WRITER used rather than a constant of its own. A reader that assumed + /// `origin/main` would compare this clone's poisoned sha against an unrelated + /// ref on any other base, never match, and answer "not cooling" — a + /// fail-open arm inside a mechanism that exists to fail closed, which is the + /// shape CLOUD-1792 was filed over. Naming the ref costs one field and + /// removes the assumption entirely. + /// + /// # Errors + /// + /// A directory or file this clone cannot write. **Never swallowed**: a poison + /// that failed to record reads as an unpoisoned clone on the next acquire, + /// which is the defect this exists to close. + pub fn poison(&self, tracking: &str, at: &str) -> Result<()> { + std::fs::create_dir_all(&self.dir)?; + std::fs::write(self.dir.join("poisoned"), format!("{tracking}\t{at}\n"))?; + Ok(()) + } + + /// The ref and position this clone last poisoned, if it has poisoned one. + /// + /// `None` for an absent, empty or malformed record, and for one this clone + /// cannot read. **Unreadable reads as unpoisoned on purpose** — the opposite + /// of [`Local::holder`]'s rule, and for the opposite reason: a holder id that + /// defaulted would let two clones claim one lease, while a cooldown that + /// defaulted to ON would hold a clone back over a file it cannot read, which + /// is the single-agent stranding [`cooling`]'s idle-pool clause exists to + /// prevent. + /// + /// A record missing its tab is malformed rather than a bare sha, because a + /// bare sha is what the FIRST spelling of this wrote and reading it loosely + /// would resurrect the assumption the tab exists to remove. + #[must_use] + pub fn poisoned(&self) -> Option<(String, String)> { + let record = std::fs::read_to_string(self.dir.join("poisoned")).ok()?; + let (tracking, at) = record.trim().split_once('\t')?; + if tracking.is_empty() || at.is_empty() { + return None; + } + Some((tracking.to_owned(), at.to_owned())) + } + /// How long `token` has been what this clone sees under `name`, on OUR clock. /// /// **Expiry alone is not safe to steal on**, which is the whole reason this diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 25e57129c..b6f2fab2a 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -7401,6 +7401,9 @@ fn run_land_lap( // different world than the one that answered (CLOUD-1306). let basis = land::Basis::of(bet.live()); note_the_poison(step, code, basis, &mut bet); + if land::poisons_this_clone(step, code, seen, basis) { + note_the_cooldown(root, reference, out, err)?; + } match land::progress_of(step, code, seen, basis) { land::Progress::Proceed => {} // `None` is not merged, or nobody could say. Either way this is @@ -8052,6 +8055,77 @@ fn note_the_poison( } } +/// Record the poison cooldown when THIS branch's own head reddened the matrix. +/// +/// The mirror of [`note_the_poison`], and deliberately not folded into it: that +/// one is about somebody ELSE's base and lives in the `Bet`, which dies with the +/// process. This one is about THIS clone and must outlive the process, because +/// the `land` that reddened CI and the `acquire` that must be held back are +/// different runs (CLOUD-1797). +/// +/// # The cell, and why each part of it is load-bearing +/// +/// `(Wait, Violation, Red, Own)` — the same cell [`land::progress_of`] answers +/// `Stop` for, narrowed by the basis: +/// +/// - **`Wait` and `Violation`** is the wait's own refusal. Keying on a red +/// reading alone would fire on a wait that SUCCEEDED while a stale red sat in +/// `seen`, which is the defect `progress_of`'s own comment records. +/// - **`Red`, never `Pending` or `None`.** Could-not-look is not a poisoning, and +/// a cooldown placed on a network blip would hold this clone back over nothing. +/// - **`Own`, never `Borrowed`.** On a borrowed base the red may be the +/// speculated base's fault, and charging it to this clone would let a +/// neighbour's bad tree evict an innocent agent — the same asymmetry +/// `charge_the_lap` already refuses. +/// +/// # What it records +/// +/// The trunk POSITION, never an instant. [`lease::cooling`] lapses the cooldown +/// when the trunk moves off that sha or when the pool goes idle, so the whole +/// mechanism is two observable events and no clock (CLOUD-1784). +/// +/// # Why a failure here reports rather than returns +/// +/// The lap is already stopping — this cell is `Progress::Stop`. Failing the land +/// over a bookkeeping write would replace a reported stop with an unreported one +/// and tell the operator less. It is loud on `err` because a cooldown that did +/// not record reads as an unpoisoned clone on the next acquire, which is exactly +/// the hole this closes. +/// +/// **The cell is [`land::poisons_this_clone`], asked by the caller**, for the +/// reason [`land::progress_of`] is a free function rather than a branch inside +/// this loop: the decision is the part worth testing, and a decision reachable +/// only through a git resolution and a filesystem write is a decision nothing +/// tests. +fn note_the_cooldown( + root: &Path, + reference: &str, + out: &mut dyn Write, + err: &mut dyn Write, +) -> Result<()> { + let tracking = land::tracking_ref(reference); + let Ok(Some(trunk)) = git::resolve_ref(root, &tracking) else { + writeln!( + err, + "::error:: land: {tracking} would not read, so the poison cooldown has no base to name" + )?; + return Ok(()); + }; + match lease_identity(root).and_then(|(local, _)| { + local + .poison(&tracking, &trunk) + .map_err(|why| format!("cannot record the poison cooldown: {why}")) + }) { + // Pointer-only: the base, never the record. + Ok(()) => writeln!( + out, + "land: cooling on {trunk} — this head reddened the matrix" + )?, + Err(why) => writeln!(err, "::error:: land: {why}")?, + } + Ok(()) +} + /// 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 @@ -10798,6 +10872,19 @@ fn run_lease_acquire( match &observed { lease::Observed::Held { body, .. } => { writeln!(out, "lease: held by {}", body.holder)?; + // THE ONLY SENDER OF A STAND-DOWN NOTICE (CLOUD-1798). The + // receiving half — `stand_down_requested`, `Beat::StandDown`, + // `honour_the_notice` — landed wired and this arm is what was + // missing, so `stand_down` was empty on every lease in the + // fleet. + // + // IT DOES NOT CHANGE THE WAIT. The waiter still answers + // `Violation`, still does not take the lease and still does + // not retry: asking is a REQUEST the holder may decline, + // which is what makes it the eviction path that is safe + // under a false suspicion. `turn`'s steal arms are the + // unsafe ones and they are unchanged. + ask_the_holder_to_stand_down(terms, &observed, body, &holder, now, out, err)?; Ok(ExitCode::Violation) } // **A REF THAT IS NOT A LEASE IS A WAIT, NOT AN INTERNAL ERROR** @@ -10833,6 +10920,16 @@ fn run_lease_acquire( } } lease::Turn::Take(why) => { + // THE POISON COOLDOWN, ASKED AFTER THE TURN AND BEFORE THE CAS + // (CLOUD-1797). `turn` decides whose turn it is; this decides whether + // THIS clone may take a turn it is otherwise owed, and the order is + // what keeps the two separable — folding it into `turn` would put a + // local file inside a decision every other clone must be able to + // reproduce from the ref alone. + if let Some(reason) = still_cooling(root, &local, &observed) { + writeln!(out, "lease: {reason}")?; + return Ok(ExitCode::Violation); + } let body = lease::claim(terms, &holder, branch, &head, now); match lease::cas(terms, &observed, &body, now) { Ok(lease::Outcome::Applied) => { @@ -10857,6 +10954,73 @@ fn run_lease_acquire( } } +/// Leave a stand-down request on a holder this clone out-ranks, if it should. +/// +/// **Three guards, and each is a way to get this wrong:** +/// +/// - **Strictly newer only** ([`lease::outranks`]). A waiter asking an equal or +/// NEWER holder to stand down inverts the design and would let an old clone +/// evict the fleet. +/// - **Idempotent.** A body already carrying a request is left alone. Without +/// this, every lap of every waiter rewrites the ref, and the holder's own +/// heartbeat CAS fails against a stream of cosmetic updates — a request that +/// bullies the holder off the lease by contention rather than by asking. +/// - **Never self-addressed.** Guarded here as well as in +/// [`lease::stand_down_requested`], because the reader's guard protects the +/// holder from a notice it minted onto itself and this one stops it being +/// written at all. +/// +/// # Why a lost CAS is silent +/// +/// Somebody else moved the ref — the holder beat, released, or another waiter +/// asked first. All three make the request moot or already-made, and none is +/// this clone's problem. Retrying would be the tight spin `run_lease_acquire`'s +/// own CAS arm refuses for the same reason. +fn ask_the_holder_to_stand_down( + terms: &lease::Terms, + observed: &lease::Observed, + body: &lease::Body, + holder: &str, + now: i64, + out: &mut dyn Write, + err: &mut dyn Write, +) -> Result<()> { + if !lease::worth_asking(body, holder) { + return Ok(()); + } + match lease::cas(terms, observed, &lease::notice(body, holder), now) { + // Pointer-only: who was asked and which build, never the body. + Ok(lease::Outcome::Applied) => writeln!( + out, + "lease: asked {} to stand down — it writes {} and this clone writes {}", + body.holder, + body.writer, + lease::writer_version() + )?, + Ok(lease::Outcome::Rejected { .. }) => {} + Err(reason) => writeln!(err, "::error:: lease: the stand-down request: {reason}")?, + } + Ok(()) +} + +/// Why this clone may not take a turn it is otherwise owed, or `None`. +/// +/// The reading half of [`note_the_cooldown`], and the whole of the decision is +/// [`lease::cooling`] — this resolves the two inputs and renders the reason. +/// +/// **It resolves the ref the WRITER recorded**, never a constant of its own. A +/// ref that will not read answers `None`: a could-not-look is not evidence this +/// clone is still poisoned, and holding it back on one would strand a clone over +/// a ref that was renamed or pruned. +fn still_cooling(root: &Path, local: &lease::Local, observed: &lease::Observed) -> Option { + let (tracking, at) = local.poisoned()?; + let trunk = git::resolve_ref(root, &tracking).ok().flatten()?; + // Pointer-only: the ref and the position, never the lease body. + lease::cooling(Some(&at), &trunk, observed).then(|| { + format!("cooling — this clone reddened the matrix on {tracking} at {at}, which is still trunk, and the pool is not idle") + }) +} + /// `lease renew`: extend this clone's lease by one term. fn run_lease_renew( root: &Path, diff --git a/crates/batten/src/perf.rs b/crates/batten/src/perf.rs index e9b372cf9..b7fd787b5 100644 --- a/crates/batten/src/perf.rs +++ b/crates/batten/src/perf.rs @@ -87,8 +87,48 @@ const WARMUP_VAR: &str = "BENCH_WARMUP"; const OUT_DIR_VAR: &str = "BENCH_OUT_DIR"; const DEFAULT_BASE_REF: &str = "origin/main"; -const DEFAULT_RUNS: &str = "100"; -const DEFAULT_WARMUP: &str = "10"; + +/// The sample size, **measured against this comparison's own null** (CLOUD-1803). +/// +/// [`REGRESSION_RATIO`] records the 2026-08-11 null — 100 runs after 10 warmups, +/// the identical binary as both arms, spreading 0.966 to 1.102 — and closes with +/// "Re-measure with `perf pair --null`". Re-measured 2026-09-12 on a busier +/// container at those same 100/10 defaults, that spread is **0.45 to 1.88**: +/// +/// | path | ratio, identical binaries | +/// | -- | -- | +/// | `check` | **1.88** | +/// | `noop` | **0.45** | +/// | `posttool` | 1.12 | +/// | `passthrough` | 1.08 | +/// | `wired` | 1.05 | +/// | `hook` | 1.00 | +/// +/// A control at 1.88 is refused by a 1.30 threshold, so the gate was deciding +/// about the machine. Measured again at **300 runs after 30 warmups**, same +/// container, same binary: 0.95 to 1.145 — back inside the band the original +/// experiment produced, and clear of the threshold. +/// +/// **So the constant moved and the threshold did not.** The differencing premise +/// in this module's header — both arms on one machine within seconds, so the +/// noise divides out — holds for a machine under steady load and fails when a +/// preemption burst lands inside one arm's window and not the other's. A larger +/// sample is what makes a burst a smaller fraction of each window; it is the +/// remedy that adds no machinery, and interleaving the arms, re-measuring on a +/// failure, and self-calibrating the threshold per run were each considered and +/// are each a way of working around a sample that simply needs to be bigger. +/// +/// Raising [`REGRESSION_RATIO`] instead was rejected outright: it would have to +/// clear 1.88, which is above the 1.462 real regression CLOUD-875 measured and +/// caught, trading a false positive for a false negative. +/// +/// The cost is three times the runs, paid only by a commit that touches crate +/// source, a manifest or the lockfile — [`Decision`] skips clean otherwise. +//MUTANT-SUITE crates/batten/tests/it/perf_pair.rs +//MUTANT sample-too-small-to-decide|s@const DEFAULT_RUNS: \&str = "300";@const DEFAULT_RUNS: \&str = "100";@|the_default_sample_is_at_least_what_the_null_was_remeasured_at +const DEFAULT_RUNS: &str = "300"; +/// Warmups, moved with [`DEFAULT_RUNS`] and measured in the same pair of nulls. +const DEFAULT_WARMUP: &str = "30"; const DEFAULT_OUT_DIR: &str = "target/perf"; /// What a diff has to touch for the measurement to be worth taking. diff --git a/crates/batten/tests/it/lease_lifecycle.rs b/crates/batten/tests/it/lease_lifecycle.rs index 2920345f5..1eccb7ba0 100644 --- a/crates/batten/tests/it/lease_lifecycle.rs +++ b/crates/batten/tests/it/lease_lifecycle.rs @@ -459,3 +459,255 @@ fn the_rendered_body_opens_with_the_banner_and_ends_with_the_nonce() { "and the head that is about to become main" ); } + +// --- the poison cooldown, end to end through its three pieces (CLOUD-1797) ---- +// +// `lease::cooling` landed pure, documented and unit-tested with NOTHING calling +// it: no code path produced a `Some` for its `poisoned_at`, so every real call +// would have passed `None` and returned `false` on the first line. The cases +// below pin the two halves that were missing — the cell that decides a clone +// poisoned itself, and the record that carries the decision across processes. + +/// **THE CELL.** A red wait, on this clone's OWN base, is what poisons it. +/// +/// Each of the four negatives is a way to hold an innocent clone back, and each +/// was reachable before the qualifier set was complete. The borrowed-base row is +/// the one a simplification deletes first: it looks like a duplicate of the +/// `Own` row above it and is the entire difference between charging a clone for +/// its own bad tree and charging it for a neighbour's. +#[test] +fn a_red_wait_on_this_clones_own_base_is_what_poisons_it() { + use batten::exit::ExitCode; + use batten::land::{Basis, Step, TapVerdict, poisons_this_clone}; + + assert!( + poisons_this_clone( + Step::Wait, + ExitCode::Violation, + Some(TapVerdict::Red), + Basis::Own + ), + "a red wait this clone owns is the whole subject of the cooldown" + ); + + assert!( + !poisons_this_clone( + Step::Wait, + ExitCode::Violation, + Some(TapVerdict::Red), + Basis::Borrowed + ), + "a borrowed base's red may be the base's fault, and charging it here \ + would let a neighbour's bad tree evict an innocent agent" + ); + assert!( + !poisons_this_clone(Step::Wait, ExitCode::Violation, None, Basis::Own), + "could-not-look is not a poisoning" + ); + assert!( + !poisons_this_clone( + Step::Wait, + ExitCode::Violation, + Some(TapVerdict::Pending), + Basis::Own + ), + "pending is not red" + ); + assert!( + !poisons_this_clone( + Step::Wait, + ExitCode::Success, + Some(TapVerdict::Red), + Basis::Own + ), + "a wait that SUCCEEDED with a stale red in hand is not a refusal — this \ + is the cell progress_of's own comment records as the defect" + ); + assert!( + !poisons_this_clone( + Step::Verify, + ExitCode::Violation, + Some(TapVerdict::Red), + Basis::Own + ), + "verify's refusal is a gate's, not the metered matrix going red" + ); +} + +/// **THE RECORD, and the assumption its shape removes.** +/// +/// It round-trips, and it stores the ref BESIDE the sha so the reader resolves +/// what the writer used. A bare sha would have made the reader assume +/// `origin/main`, compare against an unrelated ref on any other base, never +/// match, and answer "not cooling" — a fail-open arm inside a mechanism that +/// exists to fail closed, which is exactly what CLOUD-1792 was filed over one +/// module away. +#[test] +fn the_poison_record_names_the_ref_it_resolved_not_only_the_sha() { + let dir = crate::common::scratch("lease-poison-record"); + let local = batten::lease::Local::under(&dir); + + assert_eq!( + local.poisoned(), + None, + "an unpoisoned clone records nothing" + ); + + local + .poison("origin/release/1.x", "deadbeef") + .expect("the record is writable"); + assert_eq!( + local.poisoned(), + Some((String::from("origin/release/1.x"), String::from("deadbeef"))), + "the reader gets back the ref the writer used, not a constant of its own" + ); + + // A LATER POISONING REPLACES THE EARLIER ONE. Two records would leave the + // reader choosing, and the cooldown is a claim about ONE base. + local + .poison("origin/main", "cafe1234") + .expect("the record is rewritable"); + assert_eq!( + local.poisoned(), + Some((String::from("origin/main"), String::from("cafe1234"))) + ); +} + +/// **A MALFORMED RECORD READS AS UNPOISONED, and that direction is chosen.** +/// +/// The opposite of `Local::holder`'s rule, for the opposite reason: a holder id +/// that defaulted would let two clones claim one lease, while a cooldown that +/// defaulted to ON would hold a clone back over a file it cannot parse — the +/// single-agent stranding `cooling`'s idle-pool clause exists to prevent. +/// +/// The bare-sha row is not hypothetical: it is what the first spelling of this +/// wrote, and reading it loosely would resurrect the very assumption the tab +/// exists to remove. +#[test] +fn an_empty_or_untabbed_poison_record_reads_as_unpoisoned() { + let dir = crate::common::scratch("lease-poison-malformed"); + let local = batten::lease::Local::under(&dir); + std::fs::create_dir_all(&local.dir).expect("the bookkeeping directory"); + let record = local.dir.join("poisoned"); + + for malformed in ["", " \n", "deadbeef\n", "\tdeadbeef\n", "origin/main\t\n"] { + std::fs::write(&record, malformed).expect("the fixture is writable"); + assert_eq!( + local.poisoned(), + None, + "a record this clone cannot parse is not evidence it is still poisoned: {malformed:?}" + ); + } +} + +// --- the stand-down sender, and the three ways it goes wrong (CLOUD-1798) ----- +// +// `lease::notice` landed with the whole receiving half wired — `stand_down_ +// requested`, `Beat::StandDown`, `honour_the_notice` — and not one non-test +// caller, so `stand_down` was empty on every lease in the fleet. `worth_asking` +// is the sender's decision, pure so it can be decided without a ref or a CAS. + +/// **THE CASE.** A waiter asks a STRICTLY older holder, and nobody else. +/// +/// The equal-build row is the one that matters most and looks most like a +/// formality: two clones on the same version asking each other to stand down is +/// a request loop with no newer party to win it. The newer-holder row is the +/// inversion — an old clone evicting the fleet — which is what `Body::writer`'s +/// own doc refuses when it says the field decides nothing by itself. +#[test] +fn a_waiter_asks_only_a_strictly_older_holder_to_stand_down() { + use batten::lease::{worth_asking, writer_version}; + + let older = Body { + writer: String::from("0.0.1"), + ..held("work", NOW + 60) + }; + assert!( + worth_asking(&older, RIVAL), + "a holder behind this build is the whole subject of the request" + ); + + let equal = Body { + writer: writer_version(), + ..held("work", NOW + 60) + }; + assert!( + !worth_asking(&equal, RIVAL), + "an equal build is not out-ranked, and asking it would be a loop with no \ + newer party to win it" + ); + + let newer = Body { + writer: String::from("999.0.0"), + ..held("work", NOW + 60) + }; + assert!( + !worth_asking(&newer, RIVAL), + "asking a NEWER holder to stand down lets an old clone evict the fleet" + ); +} + +/// **AN UNREADABLE VERSION IS NOT AN OLD ONE**, on either side. +/// +/// A body minted by a build whose version this one cannot parse might be newer. +/// Reading it as older is the same inversion as the row above, reached through a +/// parse instead of a comparison — and a body written before `writer` existed +/// carries an EMPTY one, which is the shape that actually occurs on upgrade. +#[test] +fn a_writer_this_build_cannot_parse_is_never_treated_as_older() { + use batten::lease::worth_asking; + + for unreadable in ["", "tomorrow", "1.2", "1.2.3.4", "v1.2.3", "1.2.x"] { + let body = Body { + writer: String::from(unreadable), + ..held("work", NOW + 60) + }; + assert!( + !worth_asking(&body, RIVAL), + "an unparseable writer is not evidence the holder is behind: {unreadable:?}" + ); + } +} + +/// **IDEMPOTENT, AND NEVER SELF-ADDRESSED.** +/// +/// Without the first, every lap of every waiter rewrites the ref and the +/// holder's own heartbeat CAS fails against a stream of cosmetic updates — +/// eviction by contention rather than by asking, which is precisely the steal +/// this path exists to avoid. The second is the mirror of +/// `stand_down_requested`'s guard, one step earlier. +#[test] +fn a_body_already_asked_or_asked_by_its_own_holder_is_left_alone() { + use batten::lease::worth_asking; + + let base = Body { + writer: String::from("0.0.1"), + ..held("work", NOW + 60) + }; + + let asked = Body { + stand_down: String::from("clone-c"), + ..base.clone() + }; + assert!( + !worth_asking(&asked, RIVAL), + "a request already standing is not re-written every lap" + ); + + assert!( + !worth_asking(&base, HOLDER), + "the holder does not ask itself to stand down" + ); + + // AND THE WHITESPACE SPELLING IS THE SAME STATE. A `stand-down: ` line that + // round-tripped through a render carries a blank rather than an absence, and + // reading that as "nobody asked" would re-write on every lap after all. + let blank = Body { + stand_down: String::from(" "), + ..base.clone() + }; + assert!( + worth_asking(&blank, RIVAL), + "a blank request is an absence, so this one IS worth asking" + ); +} diff --git a/crates/batten/tests/it/perf_pair.rs b/crates/batten/tests/it/perf_pair.rs index 9e48cbaee..a1dc51375 100644 --- a/crates/batten/tests/it/perf_pair.rs +++ b/crates/batten/tests/it/perf_pair.rs @@ -348,3 +348,74 @@ fn every_path_perf_assert_budgets_is_paired() { ); } } + +/// **The default sample is at least what the null was re-measured at.** +/// +/// A FLOOR, never an equality, and the difference is the whole case. Asserting +/// `== "300"` would pass any later edit back to a sample this comparison is +/// measured to fail at, so long as whoever made it moved the case too. A floor +/// says what was established rather than what is currently written. +/// +/// # What was established +/// +/// `perf pair --null` measures the identical binary as both arms, so every ratio +/// it produces is 1.0 plus pure noise. `REGRESSION_RATIO` records the original: +/// 2026-08-11, 100 runs after 10 warmups, spread 0.966 to 1.102. Re-measured +/// 2026-09-12 on a busier container at those same defaults the spread was 0.45 +/// to 1.88 — a control at 1.88 against a 1.30 threshold, which is the gate +/// deciding about the machine. At 300 runs after 30 warmups: 0.95 to 1.145. +/// +/// # Why this is a test and not only a comment +/// +/// Because the number it defends is invisible in its effect. A sample too small +/// does not fail loudly — it makes a random budgeted path exceed the threshold on +/// a random run, which reads as a regression in whatever branch happened to be +/// measured. `scanner_taxonomy.rs`'s shape: the prose carries the measurement and +/// the assertion stops it evaporating. It does NOT re-run the null, for +/// `lease_namespace_premise.rs`'s reason — a case that benchmarked a real binary +/// would take minutes and would itself be the noisy measurement it is about. +#[test] +fn the_default_sample_is_at_least_what_the_null_was_remeasured_at() { + /// The re-measured sample, below which the null is known to exceed the + /// threshold on this class of container. + const MEASURED_AT: u32 = 300; + + let module = std::fs::read_to_string(common::at_root("crates/batten/src/perf.rs")) + .expect("the module is where the ledger says it is"); + + let declared = |name: &str| -> u32 { + let needle = format!("const {name}: &str = \""); + let rest = module + .split_once(&needle) + .unwrap_or_else(|| panic!("{name} is declared as a string constant")) + .1; + rest.split_once('"') + .expect("the constant is closed") + .0 + .parse() + .expect("the constant is a count") + }; + + assert!( + declared("DEFAULT_RUNS") >= MEASURED_AT, + "the default run count is below the sample the null was re-measured at \ + ({MEASURED_AT}); at 100 runs the identical binary measured 1.88x against \ + a 1.30x threshold, so the gate decides about the machine — see CLOUD-1803" + ); + assert!( + declared("DEFAULT_WARMUP") >= MEASURED_AT / 10, + "the warmups moved with the runs in the same pair of nulls, and a sample \ + grown without them re-admits the cold-start the warmups exist to drop" + ); + + // THE MEASUREMENT IS NAMED SO IT CAN BE RE-RUN, which is what separates this + // from a bare constant somebody must trust. Both arms of the controlled pair + // have to stay legible or the next reader re-derives instead of re-measuring. + for evidence in ["--null", "1.88", "CLOUD-1803"] { + assert!( + module.contains(evidence), + "crates/batten/src/perf.rs must keep naming {evidence:?}, so the \ + sample size reads as a measurement rather than a preference" + ); + } +} diff --git a/mise.toml b/mise.toml index 048ebf874..2049fc0b0 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,engine-pipeline,engine-policy,rules-paths-trigger,skill-frontmatter-complete" +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,engine-policy,rules-paths-trigger,skill-frontmatter-complete,engine-land" # --- 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.