From d0bc239f7dfde0c3bd923e998515205b030e00c1 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Mon, 31 Aug 2026 05:07:52 +0000 Subject: [PATCH 1/6] fix(prune): reclaim a nested cargo build tree, which no consumer can name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The escalation knew names and not shapes, so every cross-compilation target tree was outside it entirely. Measured on this container while lapping #775, from a completely cleared `target/`: debug 6429 MB partly reachable (`incremental`, `build`) semver-checks 1548 MB declared by name aarch64-apple-darwin 1378 MB UNREACHABLE release 872 MB UNREACHABLE x86_64-pc-windows-gnu 721 MB UNREACHABLE perf 562 MB declared by name Nothing supersedes those trees, so the retention rule cannot reach them either. They are caches of `incremental`'s exact kind: regrowable, unbounded, and superseded by nothing. WHY THIS IS THE ENGINE'S AND NOT TWO MORE `[[prune.regrowable]]` ROWS. The module header argued that which directories a build tree grows is a fact about the consumer's project, so the list belongs in config. That is true of a NAME and not of a SHAPE, and the distinction is the whole of this change: `semver-checks`, `perf` and `flycheck*` are names somebody chose, so they stay declared. `target//` is not — it is what cargo lays down for every `--target`, in every project, and a nested `CARGO_TARGET_DIR` has the identical shape. Recognising that shape compiles in no consumer identifier and matches nothing at all in a project that never cross-compiles, which is what non-negotiable rule 1 is actually about. The header now says so rather than arguing the opposite. The derived roots join the WARM tier and never move the basis, on the file's own existing test: dropping a nested tree makes only *that* build full and leaves the host cargo build warm — the identical reasoning already recorded for `semver-checks` and `perf`. They are taken after the declared pass, and a directory a declared row already removed is skipped, because `remove_dir_all` on an absent path would count a reclaim that freed nothing. SHOWN ABLE TO FAIL (CLOUD-418), because this function ends in `remove_dir_all` and the one case that matters is `target/debug`. A predicate matching it would delete the host build every time the floor was breached and report it as a reclaim. It is excluded twice — structurally (a real `target/debug` holds `deps/`, `build/`, `incremental/`, never a nested profile directory) and by name — and the redundancy is deliberate. Proved rather than asserted: removing the name guard reds `the_host_profile_directories_are_never_taken_as_nested_trees` on the second assertion, and the guard was restored from a backup taken before the mutation. Seven cases: the cross-target recognition, the host exclusion above, a directory with no profile directory inside it, one-level-only so a tree's own `deps/` is not itself a tree, the warm-tier reclaim leaving the basis warm, the cold tier never taking a derived root, and a tree a declared row already took not being counted twice. WHAT THIS DOES NOT FIX, stated rather than implied. The lap still costs 11684 MB and the floor still ratchets up off the worst lap on record — measured this session, a cold lap raised the observed warm floor from 9970 MB to 10997 MB, so the next lap needs ~22.7 GB free at open. This closes 2.1 GB of that gap. `target/release` is deliberately left alone: `perf-gate` rebuilds it partway through the same lap, so reclaiming it would cause a rebuild inside the lap rather than a reclaim from it. Refs: CLOUD-1240, CLOUD-1157, CLOUD-1155, CLOUD-766, CLOUD-418 --- crates/batten/src/prune.rs | 270 +++++++++++++++++++++++++++++++++++++ 1 file changed, 270 insertions(+) diff --git a/crates/batten/src/prune.rs b/crates/batten/src/prune.rs index 941d092d6..529ce828b 100644 --- a/crates/batten/src/prune.rs +++ b/crates/batten/src/prune.rs @@ -88,6 +88,18 @@ //! Which directories a build tree grows is a fact about THIS project, so the list //! is `[prune.regrowable]` and not a constant here (non-negotiable rule 1). //! +//! **That is true of a NAME and not of a SHAPE, and the distinction is CLOUD-1240's** +//! — the paragraph above used to be the whole answer, and it left 2.1 GB +//! unreclaimable on a lap that then had to be rescued by hand. `semver-checks`, +//! `perf` and `flycheck*` are names somebody chose, so they are the consumer's and +//! they stay in the config. `target//` is not: it is what cargo lays down +//! for every `--target`, in every project, and a nested `CARGO_TARGET_DIR` has the +//! identical shape. [`nested_build_trees`] recognises that shape — a directory one +//! level under the root that holds a profile directory — which compiles in no +//! consumer identifier and matches nothing at all in a project that never +//! cross-compiles. So the rule is: **a name is declared, a cargo layout is +//! derived.** +//! //! **Each row declares whether dropping it moves the basis, because they do not //! all cost the same thing.** [`Basis::Cold`] means the next **cargo** build is //! full, and the cold floor is budgeted for precisely that. Dropping `incremental` @@ -1866,9 +1878,114 @@ fn drop_regrowable(root: &Path, declared: &[Regrowable], basis_moving: bool) -> } } } + + // THE DERIVED ROOTS, AFTER THE DECLARED ONES AND ONLY IN THE WARM TIER + // (CLOUD-1240). A nested build tree costs only its own next build, exactly as + // `semver-checks` and `perf` do, so it belongs to the cheap pass and never + // moves the basis — the call site takes this tier first and re-reads free + // space before it considers anything that would. + // + // Last, because the declared list is the consumer's and its ORDER is a + // statement they made; a derived root has no such claim on going first. + if !basis_moving { + for tree in nested_build_trees(root) { + // A declared row may name the same directory — `semver-checks` and + // `perf` are themselves nested build trees, so today they are matched + // twice. Reaching a path the pass above already removed would count a + // reclaim that did not happen and add zero bytes, so the existence + // check is the accounting rather than a guard. + // + // `symlink_metadata` for `directories_named`'s reason: `is_dir` + // follows, and a link left where a tree was is not a tree. + if !std::fs::symlink_metadata(&tree).is_ok_and(|meta| meta.is_dir()) { + continue; + } + let size = directory_bytes(&tree); + removed += 1; + if std::fs::remove_dir_all(&tree).is_ok() { + freed += size; + } + } + } + (removed, freed, basis_moved) } +/// The profile directories cargo lays down inside any build tree. +/// +/// Their presence one level in is what makes a directory a build tree rather +/// than something a consumer happened to put under `target/`, and their names at +/// the top level are what makes `target/debug` the HOST's rather than a nested +/// one. +const PROFILE_DIRS: [&str; 2] = ["debug", "release"]; + +/// Every nested cargo build tree directly under `root` (CLOUD-1240). +/// +/// # Why this is the engine's and not a `[[prune.regrowable]]` row +/// +/// The module header argues that which directories a build tree grows is a fact +/// about the consumer's project, and for `semver-checks`, `perf` and `flycheck*` +/// that is exactly right — those are task names somebody chose. **`target//` +/// is not.** It is what cargo does for every `--target`, in every project, and +/// the identical layout appears under a nested `CARGO_TARGET_DIR`. So recognising +/// it here is repo-agnostic in the sense non-negotiable rule 1 means: no consumer +/// identifier is compiled in, and a consumer that never cross-compiles has no +/// such directory to match. +/// +/// Measured on this repository (CLOUD-1240): `aarch64-apple-darwin` at 1378 MB +/// and `x86_64-pc-windows-gnu` at 721 MB were outside the escalation entirely, +/// on a lap that consumed 11684 MB against a 9970 MB floor — so the only thing +/// that cleared a lap was a human deleting them by hand. +/// +/// # The predicate, and the one case it must not match +/// +/// A directory DIRECTLY under `root` that itself holds a [`PROFILE_DIRS`] entry. +/// One level only: a build tree's own `deps/` and `.fingerprint/` are not build +/// trees, and descending would let a fixture nested three deep be handed to +/// `remove_dir_all`. +/// +/// **`target/debug` is the case that decides this function is safe**, and it is +/// excluded twice over. Structurally it does not match — it holds `deps/`, +/// `build/`, `incremental/` and `.fingerprint/`, never a nested `debug/` or +/// `release/` — and the name check below refuses it regardless. The redundancy is +/// deliberate: matching it would `remove_dir_all` the host build every time the +/// floor was breached and report it as a reclaim, so a structural argument alone +/// is a thinner thing than this call deserves. `prune.rs`'s tests assert both the +/// structural miss and the named refusal, because a case that passed only because +/// of the name would say nothing about the predicate. +fn nested_build_trees(root: &Path) -> Vec { + let mut found = Vec::new(); + let Ok(entries) = std::fs::read_dir(root) else { + return found; + }; + for entry in entries.flatten() { + // `file_type` and not `Path::is_dir`, which follows: a symlink under the + // build tree pointing at somebody's home directory must never reach + // `remove_dir_all`. This is `directories_named`'s own safety argument, + // and it applies here for the same reason (#734). + if !entry.file_type().is_ok_and(|kind| kind.is_dir()) { + continue; + } + let path = entry.path(); + let Some(name) = path.file_name() else { + continue; + }; + if PROFILE_DIRS.iter().any(|profile| name == *profile) { + continue; + } + if PROFILE_DIRS.iter().any(|profile| { + std::fs::symlink_metadata(path.join(profile)).is_ok_and(|meta| meta.is_dir()) + }) { + found.push(path); + } + } + // Sorted so the reclaim order is stable across runs: the count and the bytes + // are reported, and a reader diffing two runs of a partly-failing reclaim + // should not be reading directory-iteration order. + found.sort(); + found +} + /// Whether a directory entry's own name satisfies a declared one. /// /// A SINGLE TRAILING `*` IS THE WHOLE WILDCARD LANGUAGE, and `Prune::validate` @@ -1997,6 +2114,159 @@ fn available_megabytes(path: &Path) -> Result { mod tests { use super::*; + // --- the derived nested build trees (CLOUD-1240) ------------------------- + // + // `unwrap` and `expect` are denied under `src/`, so the fixtures panic + // explicitly. A setup failure is still a loud failure; what it is not is a + // lint waiver this module does not otherwise need. + + fn mkdir(path: &Path) { + if let Err(why) = std::fs::create_dir_all(path) { + panic!("fixture: could not create {}: {why}", path.display()); + } + } + + /// A build root of this test's own, emptied first so a previous run cannot + /// decide this one. Named per case because the suite runs concurrently. + fn build_root(name: &str) -> PathBuf { + let root = std::env::temp_dir().join(format!("batten-prune-{name}")); + let _ = std::fs::remove_dir_all(&root); + mkdir(&root); + root + } + + #[test] + fn a_cross_target_tree_is_recognised_with_no_declared_row_naming_it() { + // The whole point: `aarch64-apple-darwin` is nobody's chosen name, so the + // consumer never declares it, and before this the escalation could not see + // it at all — 1378 MB of it, measured. + let root = build_root("derived-cross"); + mkdir(&root.join("aarch64-apple-darwin/debug/deps")); + mkdir(&root.join("x86_64-pc-windows-gnu/release")); + + let found = nested_build_trees(&root); + assert_eq!( + found, + vec![ + root.join("aarch64-apple-darwin"), + root.join("x86_64-pc-windows-gnu"), + ], + "a directory holding a profile directory is a nested build tree" + ); + } + + /// SHOWN ABLE TO FAIL, and this is the case the function's safety rests on. + /// + /// A predicate that matched `target/debug` would hand the HOST build to + /// `remove_dir_all` every time the floor was breached, and report it as a + /// reclaim. Both exclusions are asserted, because the redundancy is the point: + /// the structural miss is the real argument, and the named refusal is what + /// holds if a tree ever grows `target/debug/release/`. + #[test] + fn the_host_profile_directories_are_never_taken_as_nested_trees() { + let root = build_root("derived-host"); + // The structural arm: a real `target/debug` holds these and no nested + // profile directory, so it does not match on shape. + mkdir(&root.join("debug/deps")); + mkdir(&root.join("debug/build")); + mkdir(&root.join("debug/incremental")); + mkdir(&root.join("debug/.fingerprint")); + assert!( + nested_build_trees(&root).is_empty(), + "target/debug holds no nested profile directory, so it must not match" + ); + + // The named arm: even given the shape, the host roots are refused. + mkdir(&root.join("debug/release")); + mkdir(&root.join("release/debug")); + assert!( + nested_build_trees(&root).is_empty(), + "the host profile directories are refused by name as well as by shape" + ); + } + + #[test] + fn a_directory_with_no_profile_directory_inside_it_is_left_alone() { + // The anti-vacuity twin: the predicate is not "any directory under the + // root", which would be `cargo clean` spelled as a reclaim. + let root = build_root("derived-unrelated"); + mkdir(&root.join("tmp/some-fixture")); + mkdir(&root.join("bats-report")); + assert!( + nested_build_trees(&root).is_empty(), + "nothing here holds a profile directory" + ); + } + + #[test] + fn one_level_only_so_a_trees_own_deps_is_not_itself_a_tree() { + // Descending would let `deps/` — or a fixture nested three deep — reach + // `remove_dir_all`. The walk is deliberately not recursive. + let root = build_root("derived-depth"); + mkdir(&root.join("nested/debug")); + mkdir(&root.join("nested/debug/deps/inner/release")); + assert_eq!( + nested_build_trees(&root), + vec![root.join("nested")], + "only the directory one level under the root is a candidate" + ); + } + + #[test] + fn the_warm_tier_reclaims_a_derived_tree_and_leaves_the_basis_warm() { + // The acceptance clause, over the escalation rather than the predicate: + // no declared row at all, and the tree still goes — with the basis warm, + // because dropping it makes only its OWN next build full. + let root = build_root("derived-warm"); + let tree = root.join("aarch64-apple-darwin"); + mkdir(&tree.join("debug/deps")); + + let (removed, _, basis_moved) = drop_regrowable(&root, &[], false); + assert_eq!(removed, 1, "the derived tree is reclaimed"); + assert!( + !basis_moved, + "a nested tree costs only its own next build, so the cargo basis stays warm" + ); + assert!(!tree.exists(), "and it is actually gone"); + } + + #[test] + fn the_cold_tier_never_takes_a_derived_tree() { + // The tier split, asserted rather than assumed. The call site takes the + // cheap tier, re-reads free space, and only then considers the costly one + // — a derived root appearing in the second pass would be reclaimed after + // the run had already decided it needed a basis-moving drop. + let root = build_root("derived-cold"); + let tree = root.join("x86_64-pc-windows-gnu"); + mkdir(&tree.join("release")); + + let (removed, freed, basis_moved) = drop_regrowable(&root, &[], true); + assert_eq!(removed, 0, "the cold pass takes no derived root"); + assert_eq!(freed, 0); + assert!(!basis_moved); + assert!(tree.exists(), "and leaves it on disk for the warm pass"); + } + + #[test] + fn a_tree_a_declared_row_already_took_is_not_counted_twice() { + // `semver-checks` and `perf` are themselves nested build trees, so they + // match both passes. Counting the second attempt would report a reclaim + // that freed nothing, which is the accounting error the existence check + // exists to prevent. + let root = build_root("derived-twice"); + mkdir(&root.join("semver-checks/debug/deps")); + + let declared = vec![Regrowable { + name: String::from("semver-checks"), + cold: false, + }]; + let (removed, _, _) = drop_regrowable(&root, &declared, false); + assert_eq!( + removed, 1, + "one directory, one reclaim — not one per pass that matched it" + ); + } + #[test] fn a_cargo_hash_suffix_groups_the_copies_of_one_binary() { // The grouping the whole retention rests on: without it every copy is its From b9c03da1a7567cd0d62cc3eb0b7c9c2dc23784a2 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Mon, 31 Aug 2026 06:17:55 +0000 Subject: [PATCH 2/6] fix(prune): the escalation opens on the floor in force, not the declaration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `prune` gated its reclaim on `config.warm.mb` while the refusal that follows is judged against `declared.max(observed)`. Those are one number only until a ratchet observation stands above the declaration. From then on there is a BAND between the two, and inside it a run refuses without ever attempting the reclaim that would have cleared it. MEASURED, ACROSS ONE SESSION, AND EVERY REFUSAL IN IT FELL IN THE BAND. Declared 7264MB; in force 9970MB and then 10997MB. Four refusals at 7896 / 8777 / 9064 / 10931MB free — all four above the declaration, all four below the floor they were judged by — and zero escalations. Each one ended with a human deleting directories by hand. The reclaim was never unable, it was never asked: ballasting the same tree to 6600MB, below the DECLARATION, escalated immediately and reclaimed 6061MB without moving the basis. Those gigabytes were reachable at every one of the four refusals. Both tiers now open on `warm_floor_in_force`. The costly tier matters as much as the cheap one: it re-reads free space to decide whether the cheap pass was enough, so comparing that reading against the declaration inherits the identical gap one step later. The WARM standing is what opens both, and never the cold one, even where the cold floor is what will judge the lap. This gate admits a pass whose rows cost only their own next run; the costly tier sits behind its own re-read. Opening on the cold standing would make a basis-moving drop the entry condition for a pass whose whole contract is that it does not move the basis. SHOWN ABLE TO FAIL (CLOUD-418): `the_declaration_alone_would_have_refused_every_one_of_them_without_trying` asserts the predecessor's reading over the same four numbers — against `config.warm.mb` alone every one of them is ABOVE the gate and none escalates, which is the observed behaviour, kept as an assertion so the two numbers cannot quietly become one again. Two cases hold the direction: an absent observation must not invent a floor (the fresh clone, where this is inert), and an observation BELOW the declaration must not lower it — the declaration is a lower bound, which the reporting half already states and the gate now agrees with. Refs: CLOUD-1244, CLOUD-1241, CLOUD-1240, CLOUD-766, CLOUD-418 --- crates/batten/src/prune.rs | 141 ++++++++++++++++++++++++++++++++++++- 1 file changed, 139 insertions(+), 2 deletions(-) diff --git a/crates/batten/src/prune.rs b/crates/batten/src/prune.rs index 529ce828b..fa58bcd8f 100644 --- a/crates/batten/src/prune.rs +++ b/crates/batten/src/prune.rs @@ -1220,9 +1220,14 @@ pub fn prune( // it; this is what the tree says, and the two answer different questions. // Which question each one belongs to is argued at `lap`'s `floor_basis`. let tree_basis = basis; + // AGAINST THE FLOOR IN FORCE, NOT THE DECLARATION (CLOUD-1244). Resolved here + // rather than inside `escalate`, because the journal is the caller's — see + // [`warm_floor_in_force`] for what the two disagreeing cost. + let warm_in_force = warm_floor_in_force(config, &journal); let escalated_mb = escalate( root, config, + warm_in_force, &mut free_mb, &mut basis, &mut readings, @@ -1301,12 +1306,13 @@ pub fn prune( fn escalate( root: &Path, config: &Prune, + warm_in_force: u64, free_mb: &mut u64, basis: &mut Basis, readings: &mut Readings, measured_at: &Path, ) -> Result> { - if *free_mb >= config.warm.mb { + if *free_mb >= warm_in_force { return Ok(None); } // TWO TIERS, CHEAP FIRST, AND THE EXPENSIVE ONE ONLY IF IT IS STILL SHORT @@ -1333,7 +1339,11 @@ fn escalate( if cheap > 0 { *free_mb = readings.take(measured_at)?; } - if *free_mb < config.warm.mb { + // THE SAME FLOOR THE CHEAP TIER OPENED ON (CLOUD-1244). The re-read decides + // whether the cheap pass was enough; the number it is compared against has to + // be the one that will judge the lap, or this tier inherits the identical gap + // one step later. + if *free_mb < warm_in_force { let (costly, costly_bytes, basis_moved) = drop_regrowable(root, &config.regrowable, true); dropped += costly; bytes += costly_bytes; @@ -1724,6 +1734,37 @@ fn is_executable(_meta: &std::fs::Metadata) -> bool { true } +/// The warm floor as it actually stands: the declaration, raised by any +/// observation the ratchet holds above it (CLOUD-1244). +/// +/// # Why this exists rather than reading `config.warm.mb` at the gate +/// +/// The escalation used to open on the DECLARATION while the refusal is judged +/// against `declared.max(observed)`. Those are the same number only until a +/// ratchet observation stands above the declaration — and from then on there is +/// a band, between the two, where a run **refuses without ever attempting the +/// reclaim that would have cleared it**. +/// +/// Measured across one session, and every refusal in it fell in that band: free +/// 7896, 8777, 9064 and 10931 MB, against a declared 7264 and an in-force floor +/// of 9970 and then 10997. Four refusals, zero escalations, and a human deleting +/// directories by hand each time. Ballasting the same tree to 6600 MB — below the +/// DECLARATION — escalated immediately and reclaimed 6061 MB without moving the +/// basis. The reclaim was never unable; it was never asked. +/// +/// # Why the WARM standing, even where the cold floor is what will apply +/// +/// This opens the cheap tier, whose rows cost only their own next run, and the +/// costly tier re-reads free space behind its own guard. Reading the cold +/// standing here would make a basis-moving drop the entry condition for a pass +/// whose whole contract is that it does not move the basis. +fn warm_floor_in_force(config: &Prune, journal: &LapJournal) -> u64 { + journal + .ratchet + .of(Basis::Warm) + .map_or(config.warm.mb, |observed| config.warm.mb.max(observed.mb)) +} + /// Whether the next cargo build has anything to build ON. /// /// EMPTINESS IS THE SIGNAL, and it is deliberately the artifacts rather than a @@ -2267,6 +2308,102 @@ mod tests { ); } + // --- the floor the escalation opens against (CLOUD-1244) ----------------- + + fn floor(mb: u64) -> Floor { + Floor { + mb, + worst_mb: mb, + multiplier: default_multiplier(), + measured: String::from("2026-08-31"), + basis: None, + } + } + + // Spelled out rather than derived from `Default`: neither type has one, and + // adding one would put a floor of 0 within reach of a config that forgot the + // key — the opposite of what `deny_unknown_fields` buys at load. + fn floors(warm_mb: u64) -> Prune { + Prune { + root: default_root(), + keep: default_keep(), + warm: floor(warm_mb), + cold: floor(warm_mb * 2), + regrowable: Vec::new(), + } + } + + fn journal_standing(warm_mb: u64) -> LapJournal { + let mut journal = LapJournal::default(); + journal.ratchet.raise( + Basis::Warm, + Observed { + mb: warm_mb, + head: String::from("abcdef12"), + measured: String::from("2026-08-31"), + }, + ); + journal + } + + /// THE BAND THAT COST A WHOLE SESSION, as an assertion about the number. + /// + /// Declared 7264, observed 10997: every refusal measured that day sat between + /// them — 7896, 8777, 9064, 10931 MB free — and the escalation, gated on the + /// DECLARATION, never ran once. The reclaim had gigabytes available and was + /// never asked for them. + #[test] + fn the_escalation_opens_against_the_standing_observation_not_the_declaration() { + let config = floors(7264); + let journal = journal_standing(10997); + assert_eq!(warm_floor_in_force(&config, &journal), 10997); + + for free_mb in [7896_u64, 8777, 9064, 10931] { + assert!( + free_mb < warm_floor_in_force(&config, &journal), + "{free_mb}MB is under the floor in force, so the reclaim must be attempted" + ); + } + } + + /// SHOWN ABLE TO FAIL: the predecessor's reading, spelled out. + /// + /// Against `config.warm.mb` alone, all four of those readings are ABOVE the + /// gate and none of them escalates — which is precisely the observed + /// behaviour this replaces. Keeping it as an assertion means the two numbers + /// can never quietly become one again. + #[test] + fn the_declaration_alone_would_have_refused_every_one_of_them_without_trying() { + let config = floors(7264); + for free_mb in [7896_u64, 8777, 9064, 10931] { + assert!( + free_mb >= config.warm.mb, + "the declaration is what let these through ungated" + ); + } + } + + #[test] + fn with_no_observation_standing_the_declaration_is_the_floor_in_force() { + // The unratcheted case, which is every fresh clone: nothing observed, so + // the gate is exactly what it always was and this change is inert. + let config = floors(7264); + assert_eq!( + warm_floor_in_force(&config, &LapJournal::default()), + 7264, + "an absent observation must not invent a floor" + ); + } + + #[test] + fn an_observation_below_the_declaration_does_not_lower_the_floor() { + // The declaration is a lower bound, which the reporting half already + // states. The gate has to agree with it, or a cheap lap would quietly + // relax the number a later lap is judged against. + let config = floors(7264); + assert_eq!(warm_floor_in_force(&config, &journal_standing(4000)), 7264); + } + #[test] fn a_cargo_hash_suffix_groups_the_copies_of_one_binary() { // The grouping the whole retention rests on: without it every copy is its From b3953034f3b597b652dbdf1cf134a63ea9125509 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Mon, 31 Aug 2026 06:28:38 +0000 Subject: [PATCH 3/6] fix(prune): retire the observations a superseded reading took MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reading that recorded a cold lap against the warm basis was corrected. The number it wrote was not, and could not be: `Ratchet::raise` only ever climbs, so a corrected engine reads a figure the corrected reading would never have produced and treats it as measured fact. MEASURED ON THIS CONTAINER, AFTER THE FIX HAD LANDED. `[prune.warm]` declares 7264MB. The journal held a warm observation of 10997MB, written by a lap that was cold. `land` was refused at `verify`'s precondition with 8356MB free — 1092MB ABOVE the declaration — by the artifact of the bug it had just fixed. The only remedy was deleting a file under `$GIT_DIR` by hand. No gate named it, no output mentioned it, and the refusal's own remedy line said "free space outside ./target, or start a fresh session" — advice to work around a number that should have been discarded rather than worked around. A ratchet is a memory of a MEASUREMENT, and a measurement is only as good as the instrument that took it. The journal recorded no instrument, so nothing could tell an observation that is still true from one whose reading has been retired, and every clone that ever ran a pre-fix binary carries the poisoned floor for life. The journal now carries `taken_by`, and a journal some other reading took is discarded at read — the ratchet AND the open lap, because a lap opened by the old reading closes into the new one and its `spent` arithmetic would land in whichever basis the two engines disagree about, which is the misfiling the discard exists to undo. COMPARED FOR EQUALITY, never equal-or-absent. Every journal written before this key existed carries no stamp, and those are exactly the ones the corrected reading did not take. The stamp is bumped BY HAND, only when a fix changes what a reading MEANS — which is why it is not the crate version: a patch release that changes nothing about how a lap is measured must not throw away a history that is still true. Its doc comment is the log of which fix each generation is for, so bumping it costs writing down why. NOT DECAY, which stays rejected. Decay is a forgetting policy over time; it weakens correct observations too and needs a half-life nobody has measured. This is a statement about the engine: an observation a retired reading took is not a weaker fact, it is not a fact, and discarding it is exactly as principled as the existing discard of bytes that will not parse. SHOWN ABLE TO FAIL (CLOUD-418), over the real file rather than a hand-written equivalent: `the_journal_that_refused_this_container_does_not_survive_the_read` carries this container's exact journal bytes, and relaxing the comparison to equal-or-absent — the one plausible weakening — reds it on precisely the 10997MB observation that produced the row. Five cases: those bytes discarded, a journal this reading took read unchanged (a clone whose history is still its own must be untouched, or the ratchet resets every run and no floor is ever observed), a stamp from some other reading also discarded so the mechanism works forwards and the next bump is not inert, the writer re-stamping so a discard happens once rather than every run, and the report saying so while carrying no number out of the record it threw away. Refs: CLOUD-1246, CLOUD-1241, CLOUD-1244, CLOUD-1240, CLOUD-418 --- crates/batten/src/prune.rs | 299 +++++++++++++++++++++++++++++++++++-- 1 file changed, 287 insertions(+), 12 deletions(-) diff --git a/crates/batten/src/prune.rs b/crates/batten/src/prune.rs index fa58bcd8f..e16882d37 100644 --- a/crates/batten/src/prune.rs +++ b/crates/batten/src/prune.rs @@ -710,12 +710,68 @@ pub struct LapStore { #[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize, Serialize)] #[serde(default, deny_unknown_fields)] struct LapJournal { + /// Which READING took the observations below (CLOUD-1246). + /// + /// A ratchet is a memory of a measurement, and a measurement is only as good + /// as the instrument that took it. Without this key nothing distinguishes an + /// observation that is still true from one whose reading has since been + /// corrected — and because [`Ratchet::raise`] only ever climbs, the second + /// kind binds for the life of the clone. + /// + /// See [`JOURNAL_GENERATION`] for what a stamp means and when it moves. + #[serde(default)] + taken_by: String, /// The lap awaiting its closing reading, if a run opened one. open: Option, /// The worst consumption observed, per basis. ratchet: Ratchet, } +/// The reading this engine takes lap observations with. +/// +/// # It moves when a fix changes what a reading MEANS, and never otherwise +/// +/// Bumped BY HAND, which is the whole point of not keying it on the crate +/// version: a patch release that changes nothing about how a lap is measured +/// must not throw away a history that is still true. The log below is the record +/// of which fix each generation is for, so bumping it costs writing down why. +/// +/// * `2026-08-31.basis-every-deps` — CLOUD-1241. Before it, [`basis_of`] asked +/// whether ANY `deps` under the root held anything, so a cold lap beside a +/// surviving `target/release/deps` was recorded against the WARM basis. The +/// observations that reading took are not weaker facts, they are not facts: +/// measured here, a declared warm floor of 7264MB stood at 10997MB from one +/// such lap, and `land` was refused at 8356MB free by the artifact of the bug +/// it had just fixed. +const JOURNAL_GENERATION: &str = "2026-08-31.basis-every-deps"; + +/// What a journal read produced, and what it had to throw away to produce it. +/// +/// Two discards, reported separately because they are different claims about the +/// world: bytes that would not parse say the store is damaged, and a superseded +/// stamp says the store is intact and its contents are no longer meaningful. +#[derive(Default)] +struct JournalRead { + /// What the run should use — the file's contents, or a fresh history. + journal: LapJournal, + /// A journal was there and would not parse. + unreadable: bool, + /// A journal was there, parsed, and a superseded reading had taken it. + superseded: bool, +} + +/// Read the journal where there is somewhere to keep one. +/// +/// A checkout with no `$GIT_DIR` decides on the declared floor alone — which is +/// what every run did before the ratchet existed, and is why an absent journal is +/// a state rather than a failure. Neither discard can have happened there, so the +/// default is the honest answer rather than a placeholder. +fn read_journal(store: Option<&LapStore>) -> JournalRead { + store.map_or_else(JournalRead::default, |store| { + LapJournal::read(&store.git_dir) + }) +} + /// A lap that has been admitted and not yet closed. #[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] #[serde(deny_unknown_fields)] @@ -828,12 +884,40 @@ impl LapJournal { /// error, and the report says so. The alternative directions are both worse: /// failing stops `verify` over a scratch file no commit depends on, and /// staying silent would let a lap history vanish with nothing said. - fn read(git_dir: &Path) -> (Self, bool) { + /// + /// A journal a SUPERSEDED reading took is discarded on exactly that ground + /// and on exactly those terms (CLOUD-1246) — same empty result, same said-out- + /// loud report, a different precondition. + /// + /// THE OPEN LAP GOES WITH THE RATCHET, never one without the other. A lap + /// opened by the old reading would close into the new one, and its `spent` + /// arithmetic would land in whichever basis the two engines disagree about — + /// which is the very misfiling the discard exists to undo, performed once more + /// on the way out. + fn read(git_dir: &Path) -> JournalRead { + let fresh = |unreadable, superseded| JournalRead { + journal: Self::default(), + unreadable, + superseded, + }; let path = Self::path(git_dir); let Ok(raw) = std::fs::read_to_string(&path) else { - return (Self::default(), false); + return fresh(false, false); + }; + let Ok(read): std::result::Result = serde_json::from_str(&raw) else { + return fresh(true, false); }; - serde_json::from_str(&raw).map_or_else(|_| (Self::default(), true), |read| (read, false)) + // COMPARED FOR EQUALITY, never "equal or absent". Every journal written + // before this key existed carries no stamp, and those are exactly the ones + // the corrected reading did not take — so an absent stamp must not pass. + if read.taken_by != JOURNAL_GENERATION { + return fresh(false, true); + } + JournalRead { + journal: read, + unreadable: false, + superseded: false, + } } /// Write it by rename, so a reader sees whole bytes or none. @@ -848,8 +932,14 @@ impl LapJournal { let store = path.parent().unwrap_or(git_dir); std::fs::create_dir_all(store) .with_context(|| format!("target-prune: create the lap journal {}", store.display()))?; - let rendered = - serde_json::to_string(self).context("target-prune: render the lap journal")?; + // STAMPED BY THE WRITER, never carried through from the read. A run that + // discarded a superseded history and then wrote its successor back under + // the old stamp would discard it again on the next read, forever. + let rendered = serde_json::to_string(&Self { + taken_by: String::from(JOURNAL_GENERATION), + ..self.clone() + }) + .context("target-prune: render the lap journal")?; let staged = path.with_extension("json.writing"); std::fs::write(&staged, rendered) .with_context(|| format!("target-prune: write the lap journal {}", staged.display()))?; @@ -988,6 +1078,15 @@ pub struct Outcome { /// history looks exactly like the first run on a new checkout, and the whole /// point of the ratchet is that its basis is auditable. pub journal_unreadable: bool, + /// Whether a lap journal was there, readable, and taken by a superseded + /// reading (CLOUD-1246). + /// + /// SEPARATE FROM [`Self::journal_unreadable`] rather than folded into it, + /// because the two are different claims: unreadable says the store is + /// damaged, and this says the store is intact and its numbers stopped being + /// facts when the reading that took them was corrected. A reader who cannot + /// tell those apart cannot tell a disk problem from an upgrade. + pub journal_superseded: bool, } impl Outcome { @@ -1006,6 +1105,15 @@ impl Outcome { "target-prune: the lap journal could not be read, so this run starts a fresh lap history and the declared floors are the only basis\n", ); } + if self.journal_superseded { + // POINTER-ONLY, AND DELIBERATELY WITHOUT THE DISCARDED NUMBER. The + // whole claim is that those megabytes were never a measurement of + // anything this engine reads; printing them invites a reader to act + // on the one figure the line exists to retire. + line.push_str( + "target-prune: the lap journal's observations were taken by a superseded reading, so they were discarded and the declared floors are the only basis\n", + ); + } if self.unbuilt { line.push_str( "target-prune: nothing built at the configured root yet, so nothing to prune — the floor below is still judged\n", @@ -1189,10 +1297,7 @@ pub fn prune( // THE JOURNAL IS READ BEFORE THE ESCALATION, because the phase is what // decides whether the escalation may run at all — see the guard below. - let (journal, journal_unreadable) = store.map_or_else( - || (LapJournal::default(), false), - |store| LapJournal::read(&store.git_dir), - ); + let read = read_journal(store); let mut readings = Readings::declare()?; let mut free_mb = readings.take(&measured_at)?; // THE BASIS IS ALSO A PROPERTY OF THE TREE, NOT ONLY OF THIS INVOCATION. It @@ -1223,7 +1328,7 @@ pub fn prune( // AGAINST THE FLOOR IN FORCE, NOT THE DECLARATION (CLOUD-1244). Resolved here // rather than inside `escalate`, because the journal is the caller's — see // [`warm_floor_in_force`] for what the two disagreeing cost. - let warm_in_force = warm_floor_in_force(config, &journal); + let warm_in_force = warm_floor_in_force(config, &read.journal); let escalated_mb = escalate( root, config, @@ -1258,14 +1363,17 @@ pub fn prune( consumed: None, floor_source: FloorSource::Declared, journal_unreadable: false, + // Both false and not merely defaulted: with no `$GIT_DIR` there was + // no journal to read, so neither discard can have happened. + journal_superseded: false, }); }; lap( store, config, - journal, - journal_unreadable, + read.journal, + read.unreadable, &Tally { free_mb, reclaimed_mb, @@ -1287,6 +1395,11 @@ pub fn prune( consumed: lap.consumed, floor_source: lap.floor_source, journal_unreadable: lap.journal_unreadable, + // Taken from the read rather than routed through `lap`, which is where + // the answer is: `lap` decides the floor and the ratchet and has nothing + // to say about either discard. Its `journal_unreadable` parameter is a + // pass-through, and a second one would only double that. + journal_superseded: read.superseded, }) } @@ -2308,6 +2421,165 @@ mod tests { ); } + // --- observations a superseded reading took (CLOUD-1246) ----------------- + + /// The bytes this container was actually carrying, kept verbatim. + /// + /// A hand-written equivalent would drift; these are the exact contents of + /// `$GIT_DIR/batten-prune/laps.json` at the moment `land` was refused at + /// 8356MB free against a floor of 10997MB, with `[prune.warm]` declaring 7264. + const POISONED_JOURNAL: &str = r#"{"open":{"free_mb":12192,"basis":"warm","head":"2b7f57b2","measured":"2026-08-31"},"ratchet":{"warm":{"mb":10997,"head":"45601adc","measured":"2026-08-31"},"cold":{"mb":98,"head":"2b7f57b2","measured":"2026-08-31"}}}"#; + + /// Write `raw` where [`LapJournal::read`] will look, and hand back the dir. + fn journal_dir(name: &str, raw: &str) -> PathBuf { + let git_dir = build_root(name); + let path = LapJournal::path(&git_dir); + if let Some(store) = path.parent() { + mkdir(store); + } + if let Err(why) = std::fs::write(&path, raw) { + panic!("fixture: could not write {}: {why}", path.display()); + } + git_dir + } + + /// SHOWN ABLE TO FAIL (CLOUD-418), and on the number that produced the row. + /// + /// The stamp is compared for EQUALITY, so a journal written before the key + /// existed carries none and cannot match. Relaxing that to "equal or absent" + /// — the one plausible weakening — lets exactly these bytes through, and the + /// 10997MB warm observation the corrected reading would never have taken is + /// back in force. That is the assertion, over the real file. + #[test] + fn the_journal_that_refused_this_container_does_not_survive_the_read() { + let git_dir = journal_dir("journal-poisoned", POISONED_JOURNAL); + let read = LapJournal::read(&git_dir); + + assert!(read.superseded, "an unstamped journal is a superseded one"); + assert!(!read.unreadable, "it parses perfectly — that is the point"); + assert_eq!( + read.journal.ratchet.of(Basis::Warm), + None, + "the 10997MB observation must not reach the floor calculation" + ); + assert_eq!( + read.journal.ratchet.of(Basis::Cold), + None, + "and neither basis is kept — the reading was wrong about which is which" + ); + assert!( + read.journal.open.is_none(), + "the open lap goes with it: it would close into a reading that disagrees about its basis" + ); + } + + #[test] + fn a_journal_this_reading_took_is_read_unchanged() { + // The inertness clause. Every clone whose history the CURRENT reading did + // produce must be untouched, or the ratchet would reset on every run and + // the floor would never be observed at all. + let stamped = format!( + r#"{{"taken_by":"{JOURNAL_GENERATION}","open":null,"ratchet":{{"warm":{{"mb":8000,"head":"abcdef12","measured":"2026-08-31"}},"cold":null}}}}"# + ); + let git_dir = journal_dir("journal-current", &stamped); + let read = LapJournal::read(&git_dir); + + assert!(!read.superseded); + assert!(!read.unreadable); + assert_eq!( + read.journal + .ratchet + .of(Basis::Warm) + .map(|observed| observed.mb), + Some(8000), + "a standing observation this reading took still stands" + ); + } + + #[test] + fn a_stamp_from_some_other_reading_is_discarded_too() { + // Not only the ABSENT stamp: the mechanism has to work forwards, or the + // next bump would be inert and the next author would find that out the way + // this one did. + let git_dir = journal_dir( + "journal-foreign", + r#"{"taken_by":"1999-01-01.some-other-reading","open":null,"ratchet":{"warm":{"mb":9999,"head":"abcdef12","measured":"2026-08-31"},"cold":null}}"#, + ); + let read = LapJournal::read(&git_dir); + + assert!(read.superseded); + assert_eq!(read.journal.ratchet.of(Basis::Warm), None); + } + + #[test] + fn the_writer_stamps_it_so_a_discard_happens_once_and_not_every_run() { + // The self-healing half. A run that discarded a superseded history writes + // its successor back under the CURRENT stamp; without that the same + // discard would repeat forever and no observation could ever stand. + let git_dir = journal_dir("journal-restamp", POISONED_JOURNAL); + let discarded = LapJournal::read(&git_dir); + assert!(discarded.superseded); + + let mut next = discarded.journal; + next.ratchet.raise( + Basis::Warm, + Observed { + mb: 7500, + head: String::from("abcdef12"), + measured: String::from("2026-08-31"), + }, + ); + if let Err(why) = next.write(&git_dir) { + panic!("fixture: could not write the journal back: {why}"); + } + + let again = LapJournal::read(&git_dir); + assert!(!again.superseded, "the second read finds its own reading"); + assert_eq!( + again + .journal + .ratchet + .of(Basis::Warm) + .map(|observed| observed.mb), + Some(7500) + ); + } + + #[test] + fn the_discard_is_reported_and_names_no_number_from_what_it_discarded() { + // Non-negotiable rule 4 on this line specifically: the whole claim is that + // the discarded megabytes were never a measurement, so printing them would + // hand a reader the one figure the line exists to retire. + let outcome = Outcome { + pruned: 0, + reclaimed_mb: 0, + escalated_mb: None, + free_mb: 8356, + floor_mb: 7264, + basis: Basis::Warm, + next_basis: Basis::Warm, + unbuilt: false, + phase: Phase::LapOpen, + consumed: None, + floor_source: FloorSource::Declared, + journal_unreadable: false, + journal_superseded: true, + }; + let said = outcome.report(); + assert!( + said.contains("superseded reading"), + "the discard is said out loud: {said}" + ); + assert!( + !said.contains("10997"), + "and it carries no number out of the record it threw away: {said}" + ); + assert!( + outcome.clears_the_floor(), + "8356MB clears the DECLARATION, which is what binds after a discard" + ); + } + // --- the floor the escalation opens against (CLOUD-1244) ----------------- fn floor(mb: u64) -> Floor { @@ -2514,6 +2786,7 @@ mod tests { consumed: None, floor_source: FloorSource::Declared, journal_unreadable: false, + journal_superseded: false, }; let said = warm.report(); assert!( @@ -2564,6 +2837,7 @@ mod tests { consumed: None, floor_source: FloorSource::Declared, journal_unreadable: false, + journal_superseded: false, }; assert!( warm.report().contains("warm floor 6242MB"), @@ -2591,6 +2865,7 @@ mod tests { consumed: None, floor_source: FloorSource::Declared, journal_unreadable: false, + journal_superseded: false, }; assert!(!cold.clears_the_floor(), "9000MB does not fit a cold build"); assert!(cold.report().contains("COLD"), "{}", cold.report()); From 086cc40b9fdaa547b4806afed7ed2386f5c297dc Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Mon, 31 Aug 2026 06:51:05 +0000 Subject: [PATCH 4/6] fix(prune)!: Outcome grows report flags, so declare it non-exhaustive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `semver` refused the branch: `constructible_struct_adds_field`. Adding `journal_superseded` to `prune::Outcome` is a break, because `pub mod prune` exports a struct anyone could build with a literal. THE ROW SAID OTHERWISE AND THE ROW WAS WRONG. CLOUD-1246 §6 read "no config key moves and no public item changes shape; `LapJournal` is private" — the second half true, the first false, and the true half is what made the false one look right. Corrected on the row rather than only here. The break is declared rather than dodged. Nothing outside this crate builds an `Outcome`: `prune` builds it and callers read it, so a struct literal was never the contract, only an accident of it being available. Declaring it makes this the LAST break of its kind rather than the second. `journal_superseded` is the second discard flag the struct has grown, and it arrived because fixing a reading turned out not to retire what that reading had written — the next class of observation will do the same. Without `#[non_exhaustive]` each one is priced at a version bump, which teaches the next author to fold two claims into one boolean. That is exactly what this struct must not do: unreadable says the store is damaged, superseded says the store is intact and its numbers stopped being facts, and a reader who cannot tell those apart cannot tell a disk problem from an upgrade. BREAKING CHANGE: `prune::Outcome` is `#[non_exhaustive]` and carries a new `journal_superseded` field. A struct literal outside this crate no longer compiles; reading the fields is unchanged. The struct is built by `prune` and read by its callers, so no supported use is affected. Refs: CLOUD-1246, CLOUD-1241, CLOUD-1244, CLOUD-1240 --- crates/batten/src/prune.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/crates/batten/src/prune.rs b/crates/batten/src/prune.rs index e16882d37..eb5701b44 100644 --- a/crates/batten/src/prune.rs +++ b/crates/batten/src/prune.rs @@ -1029,7 +1029,22 @@ impl Basis { } /// What the prune did, and what it decided. +/// +/// # `#[non_exhaustive]` because this struct's job is to GROW +/// +/// Every field here is something a run observed and a reader may need, and each +/// new class of observation adds one: `journal_superseded` is the second discard +/// flag, and it arrived because fixing a reading turned out not to retire what +/// the reading had written. Nothing outside this crate builds an `Outcome` — +/// [`prune`] does, and callers read it — so a struct literal was never the +/// contract, only an accident of it being available. +/// +/// Declaring that makes this the LAST break of its kind rather than the second: +/// without it every future report flag is a `constructible_struct_adds_field` +/// break, which prices an honest observation at a version bump and teaches the +/// next author to fold two claims into one boolean instead. #[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] pub struct Outcome { /// Superseded artifacts removed. pub pruned: usize, From 8149058578f32c54329b984903c94b744d210102 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Mon, 31 Aug 2026 14:33:50 +0000 Subject: [PATCH 5/6] test(prune): the superseded-reading discard, over the compiled binary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-1246's five cases all called `LapJournal::read` directly. That tier pins the predicate and proves nothing about whether the ENGINE routes its answer anywhere — `.claude/rules/policy-modules.md`'s argument, arriving on a lap journal instead of a policy module. The new case runs `batten target prune` over the verbatim bytes that refused this container: the journal as it stood when CLOUD-1241's fix had landed and `land` was then refused at 8356MB free against a floor of 10997MB, 1092MB ABOVE the declaration, by the artifact of the bug it had just fixed. No `taken_by`, because the key did not exist when those bytes were written. TWO ASSERTIONS, AND NEITHER ALONE DISCRIMINATES. Without "the run says so", the case passes over an engine that silently starts a fresh history — the failure `journal_unreadable` already exists to prevent. Without "10997 binds nothing", it passes over one that prints the line and obeys the observation anyway. SHOWN ABLE TO FAIL (CLOUD-418): relaxing the stamp compare to equal-or-absent — the one plausible weakening, and the reading every pre-key journal would slip through — reds it on the 10997 assertion. AND IT REPAIRS A CASE OF TRUNK'S THAT THIS CHANGE BROKE. `an_observed_floor_names_the_file_that_holds_it` seeds a journal by hand to prove CLOUD-1218's last acceptance bullet, that a learned floor names the file holding it. Unstamped, that fixture is now discarded before the assertion can see it, and the case went red for the right reason: its own comment says the case "would prove nothing over a number the repair discards", written before a second repair existed. Stamping the fixture restores what it was asserting. The literal is duplicated from `prune::JOURNAL_GENERATION` because an integration test cannot see a private constant. That is a drift hazard with a loud failure rather than a silent one: the next bump reds both cases with the reason in the message. Refs: CLOUD-1246, CLOUD-1218, CLOUD-1244, CLOUD-418 --- crates/batten/tests/target_prune.rs | 50 ++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/crates/batten/tests/target_prune.rs b/crates/batten/tests/target_prune.rs index 7fa0e971e..9a821a0bd 100644 --- a/crates/batten/tests/target_prune.rs +++ b/crates/batten/tests/target_prune.rs @@ -471,9 +471,17 @@ fn an_observed_floor_names_the_file_that_holds_it() { // Above the declared warm floor and BELOW the declared cold one, so it is a // plausible observation that legitimately binds — the case would prove nothing // over a number the repair discards. + // + // STAMPED, and that is CLOUD-1246 arriving in this case's own terms. An + // observation carries the reading that took it, and one whose stamp is not the + // running engine's is discarded rather than obeyed — so an unstamped fixture + // IS "a number the repair discards", exactly as the sentence above warns, and + // this case would then assert nothing. The literal is duplicated from + // `prune::JOURNAL_GENERATION` because an integration test cannot see it; when + // that constant next moves this case reds, loudly, which is the right failure. std::fs::write( journal.join("laps.json"), - r#"{"open":null,"ratchet":{"warm":{"mb":9000,"head":"abcd1234","measured":"2026-08-30"},"cold":null}}"#, + r#"{"taken_by":"2026-08-31.basis-every-deps","open":null,"ratchet":{"warm":{"mb":9000,"head":"abcd1234","measured":"2026-08-30"},"cold":null}}"#, ) .unwrap(); @@ -485,6 +493,46 @@ fn an_observed_floor_names_the_file_that_holds_it() { ); } +/// CLOUD-1246 over the COMPILED BINARY, which the unit tier cannot reach. +/// +/// `prune.rs`'s own cases call `LapJournal::read` directly, so they prove the +/// predicate and not that the engine routes its answer anywhere. This is the tier +/// `.claude/rules/policy-modules.md` argues for: the same bytes that refused this +/// container, through the real verb, asserting that the run says so AND that the +/// number it discarded stopped binding. +/// +/// The pair is what makes it discriminate. Without the second assertion the case +/// passes over an engine that prints the line and obeys the observation anyway; +/// without the first it passes over one that silently starts a fresh history, +/// which is the failure `journal_unreadable` already exists to prevent. +#[test] +fn a_journal_an_older_reading_took_is_discarded_and_stops_binding() { + let repo = lapped("target-prune-superseded-reading"); + built(&repo); + let journal = repo.join(".git/batten-prune"); + std::fs::create_dir_all(&journal).unwrap(); + // Verbatim from `$GIT_DIR/batten-prune/laps.json` on the container where + // CLOUD-1241's fix landed and `land` was then refused at 8356MB free against a + // floor of 10997MB — 1092MB ABOVE the declaration, by the artifact of the bug + // it had just fixed. No `taken_by`, because the key did not exist yet. + std::fs::write( + journal.join("laps.json"), + r#"{"open":{"free_mb":12192,"basis":"warm","head":"2b7f57b2","measured":"2026-08-31"},"ratchet":{"warm":{"mb":10997,"head":"45601adc","measured":"2026-08-31"},"cold":{"mb":98,"head":"2b7f57b2","measured":"2026-08-31"}}}"#, + ) + .unwrap(); + + let said = said(&prune(&repo, "8000", &["-y"])); + assert!( + said.contains("superseded reading"), + "the discard is said out loud rather than starting a fresh history in silence: {said}" + ); + assert!( + !said.contains("10997"), + "and the discarded observation binds nothing — it was never a measurement \ + this reading took: {said}" + ); +} + #[test] fn the_escalation_says_that_the_basis_moved_and_not_only_that_it_ran() { // CLOUD-1030 §5. The predecessor's escalation line reported megabytes From 53e9673fac1e5b8ffe066e2d350385388b09c65d Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Mon, 31 Aug 2026 15:34:29 +0000 Subject: [PATCH 6/6] fix(prune): the escalation spends the undo hedge before it takes the basis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `prune.rs`'s header states the formula and the risk in the same breath: `keep` is 2 rather than 1, and it is a hedge with a stated cost. The newest artifact per stem is what the current build reads; the one before it is what a rebase that reverts would otherwise rebuild from scratch. Retained bytes after a perfectly successful prune are `keep x stems x size`. `keep` is 2 and `size` is stable; `stems` is not. BOTH ARE CORRECT AND TOGETHER THEY ARE THE DEFECT. `stems` was ~41 when CLOUD-766 censused it on 2026-08-20. It is ~130 now — one cargo test target per `crates/batten/tests/*.rs`, with CLOUD-843's campaign adding one per retired gate — at 85-106MB each. So the hedge is ~11GB on a container with ~26GB for `target` plus a lap: it costs more than the lap it is meant to survive. MEASURED, and neither pass could reach it while both reported success: target/debug/deps 18326MB over 3177 files everything else 234MB The retention leaves two copies because that is what `keep = 2` asks. The escalation skips them because they are not a `[[prune.regrowable]]` root. So a run short of disk took `incremental` — a full cold rebuild, the most expensive thing this module can do — with gigabytes nothing will ever read beside it. The cheap lever was not wired up, so it reached for the expensive one. THE FIX IS A THIRD TIER AND ITS PLACEMENT IS THE ARGUMENT. Cost order: 1. a regrowable root that is not the basis -> that tool's next run 2. the undo hedge -> a rebuild only on a revert 3. a basis-moving root -> a full cold rebuild Tier 2 goes BEFORE tier 3, never after. After is too late: tier 3 has already moved the basis, the lap is already judged against the cold floor, and the space tier 2 would have found arrives after the decision it would have changed. `1` is the SCHEMA'S OWN MINIMUM for `keep`, so this tightens to the strictest bound a config could legally have declared rather than inventing a number. No new key, no consumer name, nothing in `batten.toml` moves — the standing policy is untouched and this is only what an escalation may do under pressure. A consumer already at `keep = 1` has no second generation and the tier is a no-op. NOT BASIS-MOVING, which is why it belongs on this side of tier 3: the copy taken is by definition not the one the next build reads, so `deps` stays populated and the lap is still judged warm. The tightened artifacts are counted as SUPERSEDED rather than as cache, because that is what they are — the same pass re-run at a stricter bound. A reader seeing them under "regrowable cache dropped" would think a cache had gone that had not, and the two lines are exactly the distinction a reader deciding whether to worry is deciding between. `Outcome::tightened` is a count; the bytes are already in `reclaimed_mb`. SHOWN ABLE TO FAIL (CLOUD-418): disabling the tier reds `a_breached_floor_spends_the_undo_hedge_before_it_takes_the_basis` on the surviving count AND on `incremental` still existing, and `spending_the_hedge_leaves_the_basis_warm` on the basis. The inertness case `a_tree_above_the_floor_keeps_its_undo_hedge` stays GREEN under the same mutation, which is what makes the pair discriminate rather than merely fire. Four cases: the tier fires and stops before the basis; the basis stays warm; a tree with headroom keeps its hedge; and the count lands in the superseded column rather than the cache one. Refs: CLOUD-1249, CLOUD-1155, CLOUD-766, CLOUD-1240, CLOUD-1244, CLOUD-418 --- crates/batten/src/prune.rs | 129 +++++++++++++++++++++++++--- crates/batten/tests/target_prune.rs | 105 ++++++++++++++++++++++ 2 files changed, 222 insertions(+), 12 deletions(-) diff --git a/crates/batten/src/prune.rs b/crates/batten/src/prune.rs index eb5701b44..bf66c7596 100644 --- a/crates/batten/src/prune.rs +++ b/crates/batten/src/prune.rs @@ -1102,6 +1102,14 @@ pub struct Outcome { /// facts when the reading that took them was corrected. A reader who cannot /// tell those apart cannot tell a disk problem from an upgrade. pub journal_superseded: bool, + /// Artifacts the escalation took by tightening retention (CLOUD-1249). + /// + /// A COUNT AND NOT THE BYTES, and separate from `escalated_mb` for the reason + /// [`Escalated`] carries: the two reclaims cost different things, and a reader + /// deciding whether to worry needs to know the undo hedge went rather than a + /// cache. The bytes are already inside `reclaimed_mb`, where the superseded + /// pass they belong to reports. + pub tightened: Option, } impl Outcome { @@ -1190,6 +1198,18 @@ impl Outcome { line.push('\n'); line.push_str(&escalated); } + if let Some(count) = self.tightened { + // ITS OWN LINE, never folded into the one above (CLOUD-1249). The two + // reclaims cost different things — a cache costs the work that wrote + // it, this costs a rebuild only on an undo that may never come — and a + // reader deciding whether to worry is deciding between exactly those. + // The count is the pointer; the bytes are in `reclaimed_mb` already. + let tightened = format!( + "target-prune: retention tightened to the last generation — {count} superseded artifact(s) taken beyond `keep`. That spends the undo hedge, so a rebase that reverts rebuilds rather than relinks; nothing the next build reads was touched, and the basis is unchanged" + ); + line.push('\n'); + line.push_str(&tightened); + } line } @@ -1308,7 +1328,7 @@ pub fn prune( .to_path_buf() }; - let (pruned, reclaimed) = reclaim_superseded(root, config.keep); + let (mut pruned, mut reclaimed) = reclaim_superseded(root, config.keep); // THE JOURNAL IS READ BEFORE THE ESCALATION, because the phase is what // decides whether the escalation may run at all — see the guard below. @@ -1344,7 +1364,7 @@ pub fn prune( // rather than inside `escalate`, because the journal is the caller's — see // [`warm_floor_in_force`] for what the two disagreeing cost. let warm_in_force = warm_floor_in_force(config, &read.journal); - let escalated_mb = escalate( + let escalated = escalate( root, config, warm_in_force, @@ -1353,6 +1373,16 @@ pub fn prune( &mut readings, &measured_at, )?; + let escalated_mb = escalated.cache_mb; + // THE TIGHTENED PASS IS THE SUPERSEDED PASS, so its artifacts join that count + // rather than the cache one — it is the same reclaim re-run at a stricter + // bound, and a reader who saw them under "regrowable cache dropped" would + // think a cache had gone that had not. + let tightened = escalated.tightened.map(|(count, bytes)| { + pruned += count; + reclaimed += bytes; + count + }); let declared_mb = match basis { Basis::Warm => config.warm.mb, @@ -1381,6 +1411,7 @@ pub fn prune( // Both false and not merely defaulted: with no `$GIT_DIR` there was // no journal to read, so neither discard can have happened. journal_superseded: false, + tightened, }); }; @@ -1415,13 +1446,33 @@ pub fn prune( // to say about either discard. Its `journal_unreadable` parameter is a // pass-through, and a second one would only double that. journal_superseded: read.superseded, + tightened, }) } -/// Drop regrowable caches, but only where the warm floor is already breached. +/// What an escalation gave back, split by what it COST rather than by bytes. /// -/// Returns the megabytes dropped, where anything was, and advances `free_mb` and -/// `basis` in place — the two readings the caller's own accounting is built on. +/// Two kinds, reported separately because a reader deciding whether to worry +/// needs to know which happened: a dropped cache costs the work that wrote it, +/// and a tightened retention costs only a rebuild on an undo that may never come. +/// Summing them would hand that reader one number and no way back to the +/// question. +#[derive(Default)] +struct Escalated { + /// Megabytes of regrowable cache dropped, where any was. + cache_mb: Option, + /// Artifacts the tightened retention took, and the bytes they held. + /// + /// These belong to the caller's SUPERSEDED accounting rather than here — the + /// pass is the same one that runs at the top of [`prune`], re-run at a + /// stricter bound — so they are handed back for it to fold in. + tightened: Option<(usize, u64)>, +} + +/// Reclaim under pressure, in tiers, cheapest cost first. +/// +/// Advances `free_mb` and `basis` in place — the two readings the caller's own +/// accounting is built on. /// /// ESCALATION, AND ONLY WHEN THE WARM FLOOR IS ALREADY BREACHED. The superseded /// pass reclaims artifacts one build made obsolete, and a cache is not superseded @@ -1431,6 +1482,20 @@ pub fn prune( /// /// CONDITIONAL, never unconditional. Dropping a cache costs the work that wrote /// it, so paying that every lap would trade a rare stall for a permanent tax. +/// +/// # Three tiers, and the ORDER is the design (CLOUD-1249) +/// +/// Each tier costs strictly more than the one before it, so the run stops at the +/// cheapest that clears the floor: +/// +/// 1. a regrowable root that is not the cargo basis — costs that tool's next run; +/// 2. the retention's undo hedge — costs a rebuild only on a rebase that reverts; +/// 3. a basis-moving root — costs a full cold rebuild of everything. +/// +/// **Tier 2 must come before tier 3 rather than after, and that is the whole of +/// CLOUD-1249.** After is too late: tier 3 has already moved the basis, so the lap +/// is already judged against the cold floor, and the space tier 2 would have found +/// arrives too late to stop it. fn escalate( root: &Path, config: &Prune, @@ -1439,9 +1504,9 @@ fn escalate( basis: &mut Basis, readings: &mut Readings, measured_at: &Path, -) -> Result> { +) -> Result { if *free_mb >= warm_in_force { - return Ok(None); + return Ok(Escalated::default()); } // TWO TIERS, CHEAP FIRST, AND THE EXPENSIVE ONE ONLY IF IT IS STILL SHORT // (CLOUD-861, measured twice on that row's own landing lap). @@ -1467,10 +1532,43 @@ fn escalate( if cheap > 0 { *free_mb = readings.take(measured_at)?; } - // THE SAME FLOOR THE CHEAP TIER OPENED ON (CLOUD-1244). The re-read decides - // whether the cheap pass was enough; the number it is compared against has to - // be the one that will judge the lap, or this tier inherits the identical gap - // one step later. + // TIER 2: SPEND THE UNDO HEDGE (CLOUD-1249), and only now. + // + // `keep` is 2 because the copy behind the live one is what a rebase that + // reverts would otherwise rebuild — a hedge the header prices at "one copy per + // stem". That price is `keep x stems x size`, and the header says in the same + // breath that `stems` is not stable. It was ~41 in 2026-08-20's census and is + // ~130 now, one cargo test target per `crates/batten/tests/*.rs`, at 85-106MB + // each. Measured on the container this row was written on: 18326MB in `deps` + // over 3177 files, of which the hedge is roughly half — larger than a whole + // lap, and larger than every regrowable root put together. + // + // Neither pass could reach it, and both were succeeding. The retention leaves + // two copies because that is what it was told; the escalation skips them + // because they are not a declared root. So a run that was short of disk took + // the basis-moving lever while gigabytes nothing will ever read sat beside it. + // + // `1` IS THE SCHEMA'S OWN MINIMUM for `keep`, so this tightens to the strictest + // bound the config could legally have declared rather than inventing one. A + // consumer already at `keep = 1` has no second generation and this does + // nothing. The standing policy is untouched: this is what an escalation may do + // under pressure, never what the tree retains at rest. + // + // NOT BASIS-MOVING, and that is why it belongs on this side of tier 3. The + // copy taken is by definition not the one the next build reads, so `deps` + // stays populated and the lap is still judged warm. + let mut tightened = None; + if *free_mb < warm_in_force && config.keep > 1 { + let (count, bytes) = reclaim_superseded(root, 1); + if count > 0 { + tightened = Some((count, bytes)); + *free_mb = readings.take(measured_at)?; + } + } + // THE SAME FLOOR EVERY TIER OPENED ON (CLOUD-1244). The re-read decides + // whether the cheaper passes were enough; the number it is compared against has + // to be the one that will judge the lap, or this tier inherits the identical + // gap one step later. if *free_mb < warm_in_force { let (costly, costly_bytes, basis_moved) = drop_regrowable(root, &config.regrowable, true); dropped += costly; @@ -1491,7 +1589,10 @@ fn escalate( *free_mb = readings.take(measured_at)?; } } - Ok((dropped > 0).then_some(bytes / 1024 / 1024)) + Ok(Escalated { + cache_mb: (dropped > 0).then_some(bytes / 1024 / 1024), + tightened, + }) } /// What this run's reclaim came to, as the lap accounting needs it. @@ -2579,6 +2680,7 @@ mod tests { floor_source: FloorSource::Declared, journal_unreadable: false, journal_superseded: true, + tightened: None, }; let said = outcome.report(); assert!( @@ -2802,6 +2904,7 @@ mod tests { floor_source: FloorSource::Declared, journal_unreadable: false, journal_superseded: false, + tightened: None, }; let said = warm.report(); assert!( @@ -2853,6 +2956,7 @@ mod tests { floor_source: FloorSource::Declared, journal_unreadable: false, journal_superseded: false, + tightened: None, }; assert!( warm.report().contains("warm floor 6242MB"), @@ -2881,6 +2985,7 @@ mod tests { floor_source: FloorSource::Declared, journal_unreadable: false, journal_superseded: false, + tightened: None, }; assert!(!cold.clears_the_floor(), "9000MB does not fit a cold build"); assert!(cold.report().contains("COLD"), "{}", cold.report()); diff --git a/crates/batten/tests/target_prune.rs b/crates/batten/tests/target_prune.rs index 9a821a0bd..ca3345529 100644 --- a/crates/batten/tests/target_prune.rs +++ b/crates/batten/tests/target_prune.rs @@ -551,6 +551,111 @@ fn the_escalation_says_that_the_basis_moved_and_not_only_that_it_ran() { ); } +// --- CLOUD-1249: the undo hedge is the third tier ---------------------------- + +/// A `deps` holding two generations of every stem — what `keep = 2` retains after +/// a perfectly successful prune, and what the header prices as the undo hedge. +fn two_generations(repo: &Path) -> PathBuf { + let deps = repo.join("target/debug/deps"); + for stem in ["cli", "walker", "waivers"] { + artifact(&deps, stem, "1111111111111111", 10); + artifact(&deps, stem, "2222222222222222", 600); + } + deps +} + +#[test] +fn a_breached_floor_spends_the_undo_hedge_before_it_takes_the_basis() { + // THE WHOLE ROW. `keep = 2` is a hedge against a rebase that reverts, priced + // at one copy per stem — and `keep x stems x size` grew past a lap while both + // passes reported success. The retention could not take it (two copies is what + // it was told to leave) and the escalation could not see it (not a declared + // root), so a short run reached for `incremental` instead: a full cold rebuild, + // with gigabytes nothing will read sitting beside it. + let repo = repo("target-prune-hedge-spent"); + let deps = two_generations(&repo); + let incremental = repo.join("target/debug/incremental/batten-1a2b3c"); + std::fs::create_dir_all(&incremental).unwrap(); + std::fs::write(incremental.join("dep-graph.bin"), vec![0_u8; 200_000]).unwrap(); + + // Below the floor at open and after the cheap tier; above it once the hedge is + // spent, so the run stops there and tier 3 is never reached. + let said = said(&prune(&repo, "1,99999", &["-y"])); + + assert_eq!( + survivors(&deps), + 3, + "one generation per stem survives, not two: {said}" + ); + assert!( + said.contains("retention tightened"), + "the run says it spent the hedge: {said}" + ); + assert!( + incremental.exists(), + "and it stopped there — the basis-moving root is untouched, which is the \ + whole point of the ordering: {said}" + ); +} + +#[test] +fn spending_the_hedge_leaves_the_basis_warm() { + // The copy taken is by definition not the one the next build reads, so `deps` + // stays populated and nothing about the next build changed. A tier that moved + // the basis here would raise the floor the lap is judged against from the warm + // number to the cold one — CLOUD-1030's defect, arriving through a new door. + let repo = repo("target-prune-hedge-warm"); + two_generations(&repo); + + let said = said(&prune(&repo, "1,99999", &["-y"])); + assert!(said.contains("retention tightened"), "{said}"); + assert!( + !said.contains("COLD") && !said.contains("cold floor"), + "tightening retention is not a basis-moving reclaim: {said}" + ); +} + +#[test] +fn a_tree_above_the_floor_keeps_its_undo_hedge() { + // INERT ABOVE THE FLOOR, which is what keeps this a reclaim under pressure + // rather than a policy change. The header bought the hedge for a reason and it + // is only spent when the alternative is worse. + let repo = repo("target-prune-hedge-kept"); + let deps = two_generations(&repo); + + let said = said(&prune(&repo, "99999", &["-y"])); + assert_eq!( + survivors(&deps), + 6, + "both generations stand while there is room: {said}" + ); + assert!( + !said.contains("retention tightened"), + "and the run does not claim to have tightened anything: {said}" + ); +} + +#[test] +fn the_tightened_artifacts_are_counted_as_superseded_rather_than_as_cache() { + // POINTER-ONLY AND IN THE RIGHT COLUMN. The tightened pass IS the superseded + // pass re-run at a stricter bound, so its artifacts join that count; a reader + // who saw them under "regrowable cache dropped" would think a cache had gone + // that had not. The two lines cost different things and the report has to keep + // them apart. + let repo = repo("target-prune-hedge-counted"); + two_generations(&repo); + + let said = said(&prune(&repo, "1,99999", &["-y"])); + assert!( + said.contains("3 superseded artifact(s) taken beyond `keep`"), + "the tightened count is its own pointer: {said}" + ); + assert!( + !said.contains("regrowable cache dropped"), + "no cache was dropped here, and the report must not say one was: {said}" + ); +} + // --- CLOUD-861: the escalation is conditional -------------------------------- #[test]