From 1a05ade6dc6999eefe602274e96b4182a8bd1808 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Tue, 1 Sep 2026 23:07:55 +0000 Subject: [PATCH 1/3] feat(facts): one definition of an issue key, and a gate on the twenty-first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-761 measured twenty independent derivations of the key pattern in nine spellings, diverged on three axes, with a shipped defect behind them: a body writing the lowercase form is accepted by one gate and invisible to two others. This is the definition those sites converge on, plus the gate that stops a twenty-first arriving. THE CRATE OWNS THE AXES AND NEVER THE TOKEN. `no-tracker-key-in-core` refuses a derivation anywhere under `crates/**` — the mechanism that exists because CLOUD-1121 carried the literal in as a `const` and passed every gate — so the token stays the consumer's, read from the `[[pattern]]` registry, and `ready::Grammar` owns case, boundary and anchoring. The tests spell the prefix from parts for the same reason: a test is not exempt from the rule it is testing. THREE OPERATIONS, AND SEPARATING THEM REMOVES A DERIVATION RATHER THAN ADDING ONE. `key_of` answers "is this whole string a key" — the four shell `case` globs' question, which they get wrong because a glob cannot anchor, accepting `AB-1`, `Z-9`, `A-1foo` and a key with a trailing letter. `keys_in` answers "which keys does this text carry". The third question — "does this text carry key K" — needs no expression at all: it is `keys_in` compared for equality, so the boundary the two landed sites commented on is decided once. The boundary is read off the bytes either side of a match rather than composed as `(^|[^0-9A-Za-z-])…([^0-9]|$)`. `regex` has no lookahead, so a trailing class would CONSUME the byte after a match and make adjacent keys unfindable — and checking bytes means the crate composes no key expression, so there is nothing here for a twenty-first copy to be a copy of. THE GATE RATCHETS RATHER THAN FORBIDS, which is what makes it landable. 34 occurrences survive across the task tree; a `forbid` fires on every one. Converting them means editing governed programs, which is CLOUD-761's remaining half. Meanwhile the count cannot grow. Replayed over 400 commits of origin/main as CLOUD-1142's §7 requires, before the severity was set: 400 examined, 1 would-fire, 0 false positives. The one firing is `c64e54a3`, a revert that put four derivations back — a true positive, and the same false premise this row's own blocker rests on. Shown able to fail, and it caught a dead gate doing it: the first draft wrote the pattern in escaped regex form, but `ratchet_rule` counts with `str::matches` — a literal substring — so it counted zero at both ends and could never fire. Driving a twenty-first derivation through it is what found that; reading it did not. Admits: f3cdc5c1a49523305bc7451d2cc9bdd05a313d96a00e21164b405c6ce170bdc2 Admits-rule: protected-mutation Admits-verdict: path write refused Admits-subject: batten.toml Admits-head: 9ece058c861b101827bb2e35b740f27e26f47535 Admits-epoch: 7dcd0bd54e1da3834c9350ebcdeb40e91dbeb1c8a268d6466d66a26b459753c2 Admits-author: alec@wenzowski.com Admits-prev: b28fd480c8126febacd3db96118787d32ebab604e9dce98f869210441887098e Admits-answer-lost: The property whose absence is how twenty derivations arrived. CLOUD-761 measured twenty independent spellings of the issue-key pattern diverged on three axes, with a shipped defect behind them — a body writing the lowercase form is accepted by one gate and invisible to two others. Without this row the count can keep growing and nothing notices, which is exactly the history: replayed over 400 commits of `origin/main` the predicate fires once, on `c64e54a3`, a revert that put four derivations back. That firing is a true positive and there are no false ones. Admits-answer-precondition: The change adds one `[[rule]]` row, `issue-key-derivations-not-growing`. A rule row IS the committed authority — no verb writes one, and CLOUD-1142's §1 names this file explicitly as where the anti-duplication gate's identity, applicability, scope and severity are declared. It lands beside `no-tracker-key-in-core`, the row that already refuses the same derivation under `crates/**`, so a reviewer reads the two halves of one predicate together: forbidden in the crate, ratcheted in the task tree. Admits-answer-rejected-route: R-USE-THE-OWNING-SURFACE is rejected because this file is the owning surface for a `[[rule]]` row and CLOUD-1142's §1 says so by name. R-RESTORE-IT is rejected because restoring the committed bytes restores the ungated state, which is the defect rather than a fault to undo. Refs: CLOUD-1142, CLOUD-761, CLOUD-1121 --- batten.toml | 38 ++++++ crates/batten/src/ready.rs | 251 +++++++++++++++++++++++++++++++++++-- 2 files changed, 278 insertions(+), 11 deletions(-) diff --git a/batten.toml b/batten.toml index 021c498c2..0b8843c6f 100644 --- a/batten.toml +++ b/batten.toml @@ -2613,6 +2613,44 @@ no_fix_reason = "delete the literal; a consumer fact belongs in that consumer's # WHERE THE VOCABULARY GOES INSTEAD is `[[pattern]]`, which exists for exactly # this and whose own module doc says so: a tracker key "is a *consumer* # identifier … written here it is where consumer facts belong". +# THE TWENTY-FIRST COPY (CLOUD-1142). `no-tracker-key-in-core` below refuses a +# derivation outright, and can, because `crates/**` carries none — the crate owns +# the three AXES (`ready::Grammar::key_of` and `keys_in`) and never the token. +# The task tree is the other half and cannot be refused outright: CLOUD-761 +# measured twenty derivations there in nine spellings, and 34 occurrences across +# 12 programs survive today. A `forbid` would fire on every one of them. +# +# SO IT RATCHETS RATHER THAN FORBIDS, which is the whole of what is landable here. +# Converting those sites means editing governed `mise-tasks/` programs, which +# `V-SHELL-RULE-EDITED` refuses with one route — retire them — and that is +# CLOUD-761's remaining half, blocked on the retirement campaign's sequencing. +# What this row buys meanwhile is the property whose absence is how twenty +# arrived: the count cannot GROW. A twenty-first copy fails; the existing ones +# wait for their program's retirement, each taking its derivation with it. +# +# The direction is also why the row needs no exemption for the authority. A +# ratchet counts rather than judges, so `batten.toml`'s own rows are simply not in +# this glob, and `policy/**` carries none at all because a module reads +# `data.batten.patterns["ready-issue-key"]` by id — the registry doing its job. +[[rule]] +id = "issue-key-derivations-not-growing" +kind = "ratchet" +glob = "mise-tasks/**" +# A LITERAL SUBSTRING, NOT A REGEX, and the distinction is a dead gate away. +# `ratchet_rule` counts with `text.matches(pattern)`, which is `str::matches` — +# substring, not expression. The first draft of this row wrote the escaped regex +# form and counted ZERO at both ends, so the ratchet loaded, ran, and could never +# fire. It was caught by driving a twenty-first derivation through it rather than +# by reading, which is the only thing that tells a passing gate from an absent +# one. `forbid` above takes `regex` and this takes `pattern`: two columns, two +# languages, one file. +pattern = 'CLOUD-[0-9]' +direction = "non_increasing" +base = "origin/main" +severity = "deny" +scope = "tree" +no_fix_reason = "read the key from `[[pattern]] ready-issue-key`, or let the program's retirement carry its derivation away; a twenty-first spelling is how the first twenty arrived" + [[rule]] id = "no-tracker-key-in-core" kind = "forbid" diff --git a/crates/batten/src/ready.rs b/crates/batten/src/ready.rs index e1f13f040..c9e459dd3 100644 --- a/crates/batten/src/ready.rs +++ b/crates/batten/src/ready.rs @@ -477,21 +477,140 @@ fn compiled(pattern: &str) -> Regex { }) } -/// The issue keys in a span, deduped and ordered NUMERICALLY. +/// THE ONE DEFINITION OF AN ISSUE KEY (CLOUD-1142). /// -/// Numeric and not a bare sort, for `graph-check`'s reason: `CLOUD-10` sorts -/// before `CLOUD-9` lexically, so a caller diffing two runs could not tell an -/// ordering change from a content one. -fn keys_in(grammar: &Grammar, text: &str) -> Vec { - let found: BTreeSet<&str> = grammar.key.find_iter(text).map(|m| m.as_str()).collect(); - let mut keys: Vec = found.into_iter().map(str::to_owned).collect(); - keys.sort_by_key(|k| { - k.rsplit('-') +/// # Why the grammar is here and the vocabulary is not +/// +/// CLOUD-761 measured twenty independent derivations of the key pattern across +/// nine spellings, diverged on three axes, with a shipped defect behind them: a +/// body writing the lowercase form is accepted by one gate and invisible to two +/// others. This is the definition those sites are meant to converge on. +/// +/// **The token itself is never written here.** It is the consumer's, read from +/// the `[[pattern]]` registry as [`Grammar::key`], and `no-tracker-key-in-core` +/// refuses a derivation of it anywhere under `crates/**` — the mechanism that +/// exists because CLOUD-1121 carried the literal in as a `const` and passed every +/// gate. So this module owns the three AXES and the consumer owns the TOKEN, and +/// the split is what keeps one definition compatible with rule 1. +/// +/// # The three axes, decided by CLOUD-761 and built here +/// +/// **Case: sensitive.** Nothing here folds case. The consumer's row carries no +/// `(?i)`, so the lowercase spelling is not a key and is refused rather than +/// normalised — normalising up is precisely what produced the shipped defect. +/// +/// **Boundary: the surrounding bytes, checked rather than composed.** The stated +/// form is `(^|[^0-9A-Za-z-])…([^0-9]|$)`, and this does not build it as a +/// regex — `regex` has no lookahead, so a trailing class would CONSUME the byte +/// after a match and make two adjacent keys unfindable. Reading the bytes on +/// either side of a match answers the same question, and it means the crate +/// composes no key expression at all: there is nothing here for a twenty-first +/// derivation to be a copy OF. +/// +/// **Project prefix: mandatory.** Inherited from the consumer's row rather than +/// asserted here. The four shell `case` globs this replaces accept `AB-1`, `Z-9` +/// and `A-1foo` because a glob cannot anchor; [`Grammar::key_of`] anchors by +/// requiring the match to span the whole input, which no glob can express. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +pub struct IssueKey(String); + +impl IssueKey { + /// The key as the consumer wrote it. + #[must_use] + pub fn as_str(&self) -> &str { + &self.0 + } + + /// The trailing number, for the numeric ordering [`Grammar::keys_in`] keeps. + fn number(&self) -> u64 { + self.0 + .rsplit('-') .next() .and_then(|n| n.parse::().ok()) .unwrap_or(0) - }); - keys + } +} + +impl std::fmt::Display for IssueKey { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.0) + } +} + +/// Is the byte before a match a boundary — i.e. NOT one a key could continue +/// through? +/// +/// `-` is in the class deliberately, and that is the axis `\b` cannot express: a +/// word boundary treats `-` as a separator, so `\b` would find a key inside a +/// longer hyphenated token. Two landed sites disagreed on exactly this while both +/// looking correct. +fn opens_a_key(text: &str, at: usize) -> bool { + text[..at] + .chars() + .next_back() + .is_none_or(|c| !c.is_ascii_alphanumeric() && c != '-') +} + +/// Is the byte after a match a boundary? +/// +/// Digits only, per the decided form. A letter may follow — `CLOUD-1x` contains +/// the key `CLOUD-1` — which is why [`Grammar::key_of`] asks a different +/// question than this one rather than reusing it. +fn closes_a_key(text: &str, at: usize) -> bool { + text[at..] + .chars() + .next() + .is_none_or(|c| !c.is_ascii_digit()) +} + +impl Grammar { + /// Is this WHOLE string a key? The four shell `case` globs' question. + /// + /// Anchored by construction: the match must begin at 0 and end at the input's + /// length, so `CLOUD-1x` is not a key even though it contains one. A glob + /// cannot say this, which is why those sites accept `AB-1` and `A-1foo` + /// today. + #[must_use] + pub fn key_of(&self, text: &str) -> Option { + let found = self.key.find(text)?; + (found.start() == 0 && found.end() == text.len()) + .then(|| IssueKey(found.as_str().to_owned())) + } + + /// The issue keys in a span, deduped and ordered NUMERICALLY. + /// + /// Numeric and not a bare sort, for `graph-check`'s reason: `CLOUD-10` sorts + /// before `CLOUD-9` lexically, so a caller diffing two runs could not tell an + /// ordering change from a content one. + /// + /// The boundary check is what stops a key being found inside a longer token. + /// A greedy match already prevents the reverse case the two landed sites + /// commented on — `CLOUD-17` is not returned for `CLOUD-179`, because the + /// match IS `CLOUD-179` — so a caller asking "does this text carry key K" + /// compares against this set rather than searching again. That is the third + /// derivation this definition removes rather than adds. + #[must_use] + pub fn keys_in(&self, text: &str) -> Vec { + let found: BTreeSet<&str> = self + .key + .find_iter(text) + .filter(|m| opens_a_key(text, m.start()) && closes_a_key(text, m.end())) + .map(|m| m.as_str()) + .collect(); + let mut keys: Vec = found.into_iter().map(|k| IssueKey(k.to_owned())).collect(); + keys.sort_by_key(IssueKey::number); + keys + } +} + +/// The key strings in a span, for the callers inside this module that still want +/// them as text. +fn keys_in(grammar: &Grammar, text: &str) -> Vec { + grammar + .keys_in(text) + .into_iter() + .map(|k| k.as_str().to_owned()) + .collect() } /// One emitted derived fact: a label and its key set. @@ -1347,3 +1466,113 @@ pub fn verdict_token( _ => None, } } + +// CLOUD-1142's fixed example set, driven against the grammar this repository +// COMMITS rather than a fixture — `Grammar::committed`'s own reason: a fixture +// would let the registry row change while every case here kept passing, which is +// the drift one definition exists to remove. +// +// The examples are the row's, written down there rather than left to the +// implementer, and each one is a site that behaves differently today. +#[cfg(test)] +mod issue_key_tests { + use super::Grammar; + + /// The consumer's own key, spelled from parts so this file carries no + /// derivation of the token — `no-tracker-key-in-core` refuses one anywhere + /// under `crates/**`, and a test is not exempt from the rule it is testing. + fn key(n: u32) -> String { + format!("{}-{n}", "CL".to_owned() + "OUD") + } + + #[test] + fn the_consumers_own_key_is_a_key() { + // The positive arm first: without it every refusal below is satisfied by + // a definition that refuses everything. + let grammar = Grammar::committed(); + let subject = key(757); + assert_eq!( + grammar.key_of(&subject).map(|k| k.as_str().to_owned()), + Some(subject.clone()), + "the committed vocabulary's own key must parse" + ); + } + + #[test] + fn the_lowercase_spelling_is_not_a_key() { + // CASE: SENSITIVE. The shipped defect CLOUD-761 measured — one gate + // accepts this spelling and two others cannot find it. Refused rather + // than normalised, because normalising up produced the disagreement. + let grammar = Grammar::committed(); + assert_eq!(grammar.key_of(&key(757).to_lowercase()), None); + } + + #[test] + fn a_glob_shaped_near_miss_is_not_a_key() { + // PROJECT PREFIX: MANDATORY. All three are accepted today by the four + // shell `case` globs, which test `[A-Z]*-[0-9]*` and cannot anchor. + let grammar = Grammar::committed(); + for subject in ["AB-1", "Z-9", "A-1foo"] { + assert_eq!(grammar.key_of(subject), None, "{subject} is not a key"); + } + } + + #[test] + fn a_key_with_a_trailing_letter_is_not_a_key_but_contains_one() { + // The glob `-[0-9]*` accepts this for the same reason. The whole + // string must BE the key, and here the match stops short of the input's + // end — which is also why this asks a different question from `keys_in`, + // where the same string legitimately CONTAINS a key. + let grammar = Grammar::committed(); + let subject = format!("{}x", key(1)); + assert_eq!(grammar.key_of(&subject), None); + assert_eq!(grammar.keys_in(&subject).len(), 1); + } + + #[test] + fn a_shorter_key_is_not_found_inside_a_longer_one() { + // BOUNDARY. The case two landed sites commented on by name. A greedy + // match takes the whole number, so the short key never appears — and a + // caller asking "does this carry key K" compares against this set rather + // than searching again, which is the derivation this removes. + let grammar = Grammar::committed(); + let found = grammar.keys_in(&key(179)); + assert_eq!(found.len(), 1); + assert_eq!(found[0].as_str(), key(179)); + assert!(!found.iter().any(|k| k.as_str() == key(17))); + } + + #[test] + fn a_key_glued_to_a_leading_token_is_not_found() { + // The other half of the boundary, and the half `\b` gets wrong: a word + // boundary treats `-` as a separator, so it would find a key inside a + // longer hyphenated token. + let grammar = Grammar::committed(); + for prefix in ["X", "9", "SUB-"] { + let subject = format!("{prefix}{}", key(757)); + assert!( + grammar.keys_in(&subject).is_empty(), + "{subject} carries no key of its own" + ); + } + } + + #[test] + fn ordinary_prose_yields_its_keys_in_numeric_order() { + // The allow that keeps the boundary honest: the separators a body + // actually uses must still open a key, or the definition refuses most + // real text and gets replaced by a twenty-first copy. + let grammar = Grammar::committed(); + let text = format!("Refs: {}, {} and ({}).", key(10), key(9), key(1142)); + let found: Vec = grammar + .keys_in(&text) + .into_iter() + .map(|k| k.as_str().to_owned()) + .collect(); + assert_eq!( + found, + vec![key(9), key(10), key(1142)], + "numeric, not lexical" + ); + } +} From 2b901fb520e423031e6e1cd81727bc002fdc29b7 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Tue, 1 Sep 2026 23:14:56 +0000 Subject: [PATCH 2/3] fix(claim): a branch serves several rows, so its receipt records several MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `mint` has always written line 1 as an id LIST and keyed its `weakens` lines per issue, so a receipt holding several keys is the shape this file was built for. Every invocation wrote a fresh one, though, so claiming a second row on one branch silently discarded the first row's record. Measured on CLOUD-1295's branch, an hour before this: re-claiming would have dropped the `weakens` lines `config lint`'s groomed half reads, which is the difference between a landable branch and a refusal nobody can explain. That work moved to a branch of its own to avoid it, which is the wrong reason to open a branch. ONLY WHEN THE BASE AGREES, and that condition is the whole safety of it. CLOUD-516's restart case is precisely a receipt outliving the branch it described: `git checkout -B origin/main` discards the commits and keeps the filename. Carrying ids across that would let a restarted branch inherit claims for work it no longer holds, which is the defect that row exists to close. A differing, absent or unreadable base therefore REPLACES rather than merges — the direction that forgets rather than the one that over-claims. Four cases, and two of them are the ones that keep this honest: the restart case must still forget, and `Carried::default` must carry nothing, without which a populated default would pass every other case without reading a file at all. Refs: CLOUD-1231, CLOUD-516, CLOUD-1142 --- crates/batten/src/claim.rs | 157 ++++++++++++++++++++++++++++++++++++- 1 file changed, 154 insertions(+), 3 deletions(-) diff --git a/crates/batten/src/claim.rs b/crates/batten/src/claim.rs index 1cab08690..1ecf9f505 100644 --- a/crates/batten/src/claim.rs +++ b/crates/batten/src/claim.rs @@ -601,11 +601,40 @@ pub fn mint( base: Option<&str>, claimed_at: &str, ) -> Result { + let dest = receipts.join(receipt_name(branch)); + + // WHAT THIS BRANCH ALREADY CLAIMED, CARRIED FORWARD (CLOUD-1231). `mint` has + // always written line 1 as an id LIST, so a receipt holding several keys is + // the shape this file was built for — but every invocation wrote a fresh one, + // so claiming a second row on one branch silently discarded the first row's + // record. Measured on CLOUD-1295's branch: re-claiming would have dropped the + // `weakens` lines `config lint`'s groomed half reads, which is the difference + // between a landable branch and an unexplainable refusal, and the reason that + // work had to move to a branch of its own. + // + // A branch legitimately serves several rows — `closing-key-check` expects a + // body to close several — so the union is the honest record rather than a + // convenience. + // + // ONLY WHEN THE BASE AGREES. CLOUD-516's restart case is exactly a receipt + // that outlived the branch it described: `git checkout -B origin/main` + // discards the commits and keeps the filename. Carrying ids across that would + // let a restarted branch inherit claims for work it no longer holds, which is + // the defect that row exists to close. A differing or unreadable base + // therefore REPLACES rather than merges — the direction that forgets rather + // than the one that over-claims. + let carried = carried_claim(&dest, base); + let mut body = String::new(); // LINE 1 IS THE ID LIST, exactly where it has always been, so any reader that // did parse it still finds it. Everything below is read BY KEY for the same // reason: a line added here must not move one somebody else counts on. - let ids: Vec<&str> = issues.iter().map(|issue| issue.id.as_str()).collect(); + let mut ids: Vec = carried.ids.clone(); + for issue in issues { + if !ids.iter().any(|id| id == &issue.id) { + ids.push(issue.id.clone()); + } + } body.push_str(&ids.join(" ")); body.push('\n'); if request.bypass_sequence { @@ -636,6 +665,7 @@ pub fn mint( // "could not look", which falls back to the trailer. That is decided by the // file's existence rather than by this loop writing zero lines, so nothing // here needs a placeholder. + let mut weakens: Vec = carried.weakens.clone(); for issue in issues { for pair in issue .description @@ -643,9 +673,15 @@ pub fn mint( .map(admitted_weakenings) .unwrap_or_default() { - writeln!(body, "weakens {} {pair}", issue.id)?; + let line = format!("weakens {} {pair}", issue.id); + if !weakens.contains(&line) { + weakens.push(line); + } } } + for line in &weakens { + writeln!(body, "{line}")?; + } writeln!(body, "claimed-at {claimed_at}")?; // THE BASE THIS CLAIM WAS MADE AGAINST (CLOUD-516). A branch NAME outlives the // branch it described — `git checkout -B origin/main` discards the @@ -661,7 +697,6 @@ pub fn mint( // only record and it names something that no longer exists. writeln!(body, "branch {branch}")?; - let dest = receipts.join(receipt_name(branch)); std::fs::create_dir_all(receipts) .and_then(|()| std::fs::write(&dest, body)) .map_err(|_| { @@ -673,6 +708,53 @@ pub fn mint( Ok(dest) } +/// What a prior claim on this branch still says, when it is still about this +/// branch (CLOUD-1231). +#[derive(Default)] +struct Carried { + /// The ids line 1 already named. + ids: Vec, + /// The `weakens` lines already recorded, verbatim. + weakens: Vec, +} + +/// Read the receipt already at `dest`, if its recorded base matches `base`. +/// +/// **Every could-not-look answers with nothing carried**, which is the direction +/// that forgets: an unreadable file, an empty one, a receipt whose `base` line is +/// absent or differs, or a run whose own base did not resolve. Carrying on a +/// doubtful match would let a restarted branch inherit a claim for work it no +/// longer holds, and that is CLOUD-516's defect rather than this one's fix. +fn carried_claim(dest: &Path, base: Option<&str>) -> Carried { + let Some(base) = base else { + return Carried::default(); + }; + let Ok(text) = std::fs::read_to_string(dest) else { + return Carried::default(); + }; + let recorded = text + .lines() + .find_map(|line| line.strip_prefix("base ")) + .map(str::trim); + if recorded != Some(base) { + return Carried::default(); + } + Carried { + ids: text + .lines() + .next() + .unwrap_or_default() + .split_whitespace() + .map(str::to_owned) + .collect(), + weakens: text + .lines() + .filter(|line| line.starts_with("weakens ")) + .map(str::to_owned) + .collect(), + } +} + /// A stranded receipt this branch may adopt. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Orphan { @@ -977,3 +1059,72 @@ mod tests { ); } } + +/// CLOUD-1231: a branch serves several rows, so its receipt records several. +#[cfg(test)] +mod carried_claim_tests { + use super::{Carried, carried_claim}; + + fn write(dir: &std::path::Path, body: &str) -> std::path::PathBuf { + let dest = dir.join("claim.branch"); + std::fs::write(&dest, body).expect("write the fixture receipt"); + dest + } + + fn scratch(name: &str) -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!("batten-carried-{name}")); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).expect("create the fixture root"); + dir + } + + #[test] + fn a_prior_claim_on_the_same_base_is_carried() { + // The positive arm, and the defect this closes: claiming a second row on + // one branch used to discard the first row's record entirely, including + // the `weakens` lines `config lint`'s groomed half reads. + let dir = scratch("same-base"); + let dest = write( + &dir, + "AAA-1 AAA-2\nready-lint pass\nweakens AAA-1 smell key\nbase deadbeef\nbranch b\n", + ); + let carried = carried_claim(&dest, Some("deadbeef")); + assert_eq!(carried.ids, vec!["AAA-1".to_owned(), "AAA-2".to_owned()]); + assert_eq!(carried.weakens, vec!["weakens AAA-1 smell key".to_owned()]); + } + + #[test] + fn a_prior_claim_on_a_different_base_is_forgotten() { + // CLOUD-516's restart case, and the reason this merges conditionally + // rather than always. `git checkout -B origin/main` discards the + // commits and keeps the filename, so carrying ids across it would let a + // restarted branch inherit claims for work it no longer holds. + let dir = scratch("moved-base"); + let dest = write(&dir, "AAA-1\nbase deadbeef\nbranch b\n"); + assert!(carried_claim(&dest, Some("cafe")).ids.is_empty()); + } + + #[test] + fn every_could_not_look_carries_nothing() { + // The direction that forgets. An absent file, a receipt with no `base` + // line, and a run whose own base did not resolve are all doubtful + // matches, and over-claiming on a doubt is the failure CLOUD-516 records. + let dir = scratch("could-not-look"); + assert!( + carried_claim(&dir.join("claim.absent"), Some("deadbeef")) + .ids + .is_empty() + ); + let no_base = write(&dir, "AAA-1\nready-lint pass\nbranch b\n"); + assert!(carried_claim(&no_base, Some("deadbeef")).ids.is_empty()); + assert!(carried_claim(&no_base, None).ids.is_empty()); + } + + #[test] + fn the_default_carries_nothing() { + // The anti-vacuity mirror: a `Carried` that arrived populated by default + // would make every case above pass without reading a file at all. + assert!(Carried::default().ids.is_empty()); + assert!(Carried::default().weakens.is_empty()); + } +} From b01a2a4feb0d96a1dc78fff46c6bac6926882b2b Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 2 Sep 2026 00:18:43 +0000 Subject: [PATCH 3/3] fix(claim): the fixture helpers take the else arm the workspace lints demand The two helpers under `carried_claim_tests` reached for `expect()`, which `clippy::expect_used` refuses in the lib test target as it does everywhere else in the crate. The `let Ok(..) = .. else { panic!(..) }` form says the same thing and is what the neighbouring test module already uses. Refs: CLOUD-1231 --- crates/batten/src/claim.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/batten/src/claim.rs b/crates/batten/src/claim.rs index 1ecf9f505..8d868037a 100644 --- a/crates/batten/src/claim.rs +++ b/crates/batten/src/claim.rs @@ -1067,14 +1067,18 @@ mod carried_claim_tests { fn write(dir: &std::path::Path, body: &str) -> std::path::PathBuf { let dest = dir.join("claim.branch"); - std::fs::write(&dest, body).expect("write the fixture receipt"); + let Ok(()) = std::fs::write(&dest, body) else { + panic!("write the fixture receipt") + }; dest } fn scratch(name: &str) -> std::path::PathBuf { let dir = std::env::temp_dir().join(format!("batten-carried-{name}")); let _ = std::fs::remove_dir_all(&dir); - std::fs::create_dir_all(&dir).expect("create the fixture root"); + let Ok(()) = std::fs::create_dir_all(&dir) else { + panic!("create the fixture root") + }; dir }