From caebfe1f4795dc9ef935b3287e7d13e16cf9cf04 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 19 Aug 2026 23:15:26 +0000 Subject: [PATCH 1/4] test(trust): fail on any config field with no weakening verdict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-721. `trust::weakenings` compares six things against a `Config` that declares twenty-eight fields, and nothing kept the two in step: `hk.pkl` runs `config-lint` on a diff touching `trust.rs`, never on one that grows `config.rs` a key. So every key landed since CLOUD-31 arrived with no prompt to ask whether it has a weakening direction — and for `config lint --config-from` the weakening list IS the verdict, so an uncovered key is a weakening that gate cannot see. The mechanism is a census, not a longer list (non-negotiable rule 2). The field list is read off `Config`'s own source — the struct-source scan `config.rs` already performs for its validation census — and `CENSUS` only records what happens to each field. A field added to the struct fails this test until somebody records one of three answers: compared (with its kind), no monotone reading (with the reason), or not policy-bearing (with the reason). Silence is not one of the three. This commit is the gate alone, and it goes RED naming twenty-two fields. That list is the work; the coverage follows it. `WeakeningKind::ALL` arrives with it, in the shape `Effect::ALL` and `Watched::ALL` already use, so the census can range over the vocabulary: a kind no field claims is a comparison whose key nobody can name, and one two fields claim leaves the verdict ambiguous about which key moved. Refs: CLOUD-721 --- crates/batten/src/trust.rs | 211 +++++++++++++++++++++++++++++++++++++ 1 file changed, 211 insertions(+) diff --git a/crates/batten/src/trust.rs b/crates/batten/src/trust.rs index 14414f449..7e3b01911 100644 --- a/crates/batten/src/trust.rs +++ b/crates/batten/src/trust.rs @@ -98,6 +98,22 @@ pub enum WeakeningKind { } impl WeakeningKind { + /// Every kind, so anything ranging over the vocabulary is derived rather + /// than re-typed — the idiom [`crate::effect::Effect::ALL`] and + /// [`crate::outputs::Watched::ALL`] already use. + /// + /// [`CENSUS`] reads this: a kind added here without a field claiming it, or + /// claimed by two fields, fails the census rather than sitting unattributed. + pub const ALL: &'static [WeakeningKind] = &[ + WeakeningKind::StrictnessLowered, + WeakeningKind::PromotionDisabled, + WeakeningKind::ProtectedRemoved, + WeakeningKind::UnlandedRemoved, + WeakeningKind::RuleRemoved, + WeakeningKind::SeverityLowered, + WeakeningKind::WaiverAdded, + ]; + /// The stable, lowercase identifier used in machine output (§6). #[must_use] pub const fn as_str(self) -> &'static str { @@ -113,6 +129,67 @@ impl WeakeningKind { } } +/// What [`weakenings`] does about one [`Config`] field. +/// +/// Three answers and no fourth, because the fourth is silence — and silence is +/// what let this comparison fall to six keys of a twenty-eight-key struct +/// without anything noticing (CLOUD-721). A field is either compared, or it has +/// no monotone reading, or it is not policy-bearing; the last two carry their +/// reason here so the next person reads it instead of re-deriving it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] +pub enum Coverage { + /// Compared, by exactly these kinds. Each kind belongs to one field. + Compared(&'static [WeakeningKind]), + /// Policy-bearing, but neither direction lowers a bar — with the reason. + NoMonotoneReading(&'static str), + /// Not policy-bearing at all: no reading of the key sets a bar to lower. + NotPolicyBearing(&'static str), +} + +/// One [`Config`] field and what this module does about it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct FieldCoverage { + /// The field's name in [`Config`], exactly as the struct spells it. + pub field: &'static str, + /// The verdict. + pub coverage: Coverage, +} + +/// The verdict for every [`Config`] field, as data. +/// +/// **Not the source of the field list** — that is [`Config`] itself, read from +/// its own source by [`tests::every_config_field_carries_a_verdict`]. This table +/// only says what happens to each one, so a field added to the struct fails the +/// census until somebody decides. A hand-kept list of *fields* would drift on the +/// next key added, which is the defect rather than a second copy of it. +pub const CENSUS: &[FieldCoverage] = &[ + FieldCoverage { + field: "strictness", + coverage: Coverage::Compared(&[WeakeningKind::StrictnessLowered]), + }, + FieldCoverage { + field: "fail_on_warning", + coverage: Coverage::Compared(&[WeakeningKind::PromotionDisabled]), + }, + FieldCoverage { + field: "protected", + coverage: Coverage::Compared(&[WeakeningKind::ProtectedRemoved]), + }, + FieldCoverage { + field: "unlanded", + coverage: Coverage::Compared(&[WeakeningKind::UnlandedRemoved]), + }, + FieldCoverage { + field: "rules", + coverage: Coverage::Compared(&[WeakeningKind::RuleRemoved, WeakeningKind::SeverityLowered]), + }, + FieldCoverage { + field: "waivers", + coverage: Coverage::Compared(&[WeakeningKind::WaiverAdded]), + }, +]; + /// One key the working tree weakened relative to the base ref. /// /// Pointer-only by construction (non-negotiable rule 4): a key path and two @@ -603,4 +680,138 @@ mod tests { "batten.toml:rule[no-todo].severity deny→warn" ); } + + /// Every field [`Config`] declares, read off its own source. + /// + /// The struct-source scan `config.rs`'s own + /// `every_typed_config_table_has_a_validation_call_site` already performs, + /// for the same reason: the authority on which fields exist is the struct, + /// and any second list of them is the drift being fixed. + fn config_fields() -> Vec<&'static str> { + let source = include_str!("config.rs"); + let start = source + .find("pub struct Config {") + .expect("Config is declared here"); + let rest = &source[start..]; + let body = &rest[..rest.find("\n}").expect("the struct closes")]; + body.lines() + .filter_map(|line| line.trim().strip_prefix("pub ")) + .filter_map(|rest| rest.split_once(':')) + .map(|(field, _)| field) + .collect() + } + + #[test] + fn every_config_field_carries_a_verdict() { + // CLOUD-721's gate, and the reason it is written before the coverage it + // demands: on arrival this failed with twenty-two field names, and that + // list WAS the work. A key added to `Config` with a weakening direction + // nobody considered is how the comparison fell to six of twenty-eight, + // and prose asking the next author to consider it is feedforward only + // (non-negotiable rule 2). + let fields = config_fields(); + assert!( + fields.len() > 10, + "the struct scan must actually find fields: {fields:?}" + ); + + let missing: Vec<&str> = fields + .iter() + .copied() + .filter(|field| !CENSUS.iter().any(|row| row.field == *field)) + .collect(); + assert!( + missing.is_empty(), + "these `Config` fields carry no weakening verdict: {missing:?}. Say what \ + `trust` does about each one — compared (with its kind), no monotone \ + reading (with the reason), or not policy-bearing (with the reason). \ + Silence is not one of the three." + ); + + for row in CENSUS { + assert!( + fields.contains(&row.field), + "the census names `{}`, which `Config` no longer declares", + row.field + ); + assert_eq!( + CENSUS + .iter() + .filter(|other| other.field == row.field) + .count(), + 1, + "`{}` carries two verdicts; one field, one answer", + row.field + ); + match row.coverage { + Coverage::Compared(kinds) => assert!( + !kinds.is_empty(), + "`{}` is recorded compared by no kind at all", + row.field + ), + Coverage::NoMonotoneReading(reason) | Coverage::NotPolicyBearing(reason) => { + assert!( + !reason.trim().is_empty(), + "`{}` declines to compare without saying why", + row.field + ); + } + } + } + } + + #[test] + fn every_weakening_kind_is_claimed_by_exactly_one_field() { + // The other half of the census: a kind nothing claims is a comparison + // whose key nobody can name, and a kind two fields claim makes the + // verdict ambiguous about which key moved. + for kind in WeakeningKind::ALL { + let claimants: Vec<&str> = CENSUS + .iter() + .filter(|row| match row.coverage { + Coverage::Compared(kinds) => kinds.contains(kind), + _ => false, + }) + .map(|row| row.field) + .collect(); + assert_eq!( + claimants.len(), + 1, + "{} is claimed by {claimants:?}; exactly one field owns a kind", + kind.as_str() + ); + } + } + + #[test] + fn the_kind_vocabulary_is_derived_rather_than_re_typed() { + // `ALL` is what the census ranges over, so a variant missing from it + // would be a kind the census cannot see — the same hole one level down. + let source = include_str!("trust.rs"); + let start = source + .find("pub enum WeakeningKind {") + .expect("the kind enum is declared here"); + let rest = &source[start..]; + let body = &rest[..rest.find("\n}").expect("the enum closes")]; + let declared = body + .lines() + .filter(|line| { + let line = line.trim(); + line.ends_with(',') + && !line.starts_with("///") + && line.starts_with(|c: char| c.is_ascii_uppercase()) + }) + .count(); + assert_eq!( + declared, + WeakeningKind::ALL.len(), + "every variant must be in `ALL`" + ); + + let mut tokens: Vec<&str> = WeakeningKind::ALL.iter().map(|k| k.as_str()).collect(); + tokens.sort_unstable(); + let count = tokens.len(); + tokens.dedup(); + assert_eq!(tokens.len(), count, "two kinds share one token"); + } } From d697061289ba143eac0bef93a82d4bf009cf505c Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 19 Aug 2026 23:22:39 +0000 Subject: [PATCH 2/4] feat(trust): compare every config key the census demanded a verdict for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-721, following the failure list the census printed. Twenty-two fields had no recorded answer; each now has one. Compared, each with its own kind and its own key path so two weakenings of one kind cannot collapse (CLOUD-233): `min_batten_version` (a floor lowered or deleted admits a binary that does not understand the rules), `epoch.tracked`, `verb` (a removed row un-gates a mutating call at the PreToolUse boundary — the most consequential of them), `marker`, `exec_pattern`, `provision`, `budget` (set, counted file, embedded declaration, and both ceilings), `must_land_on`, `worktree`, `judge` (raw classes and the payload ceiling), `design`, `ci`, `defects`, `transcript`, and `attribution`'s deny lists and carve-out. Recorded as no monotone reading, with the reason where the verdict is: `scope` (§8 counts narrowing as tightening and widening polices more), `exec`, `hook`, `drain` and `commit`. Not policy-bearing: `version` and `redirect`. Two keys the comparison already claimed are deepened, which is what keying the census on weakening-direction rather than on field name is for. A waiver's identity omits `expires`, so a base lapsed in 2020 and a working one live until 2099 were the same key and nothing was reported — while the second suppresses every finding of its rule; the pairing is by that same key and the comparison is one file's date against the other's, never against today, so §6 holds. And a rule whose glob narrowed to match nothing kept its id and severity, so the comparison called it unchanged while it gated nothing. The rule half is read off the rule's own serialization rather than a hand-kept column list: a column added to `Rule` is compared until somebody exempts it with a reason, which is the fail-safe direction and the same defect one level down from the one the census closes. Its tokens are digests — a pointer names which column moved without carrying what the pattern now says. Ceilings invert, and `ceiling_raised` states it once: for a threshold a larger number forgives more, and an absent one is widest of all because the predicate stops participating. The defaulted ceilings compare effective values, so deleting a key and writing the default cannot read differently. Refs: CLOUD-721 --- crates/batten/src/trust.rs | 838 ++++++++++++++++++++++++++++++++++++- 1 file changed, 835 insertions(+), 3 deletions(-) diff --git a/crates/batten/src/trust.rs b/crates/batten/src/trust.rs index 7e3b01911..089b90f77 100644 --- a/crates/batten/src/trust.rs +++ b/crates/batten/src/trust.rs @@ -95,6 +95,92 @@ pub enum WeakeningKind { /// which §6 forbids. The expiry is what the *run* evaluates; whether the diff /// added a suppression is a property of the two files alone. WaiverAdded, + /// A waiver's expiry moved later, extending a suppression the base ref had + /// bounded (CLOUD-721). + /// + /// The identity in [`crate::waiver::Waiver::key`] deliberately omits + /// `expires`, so a base waiver lapsed in 2020 and a working one live until + /// 2099 are the *same* key and [`added_entries`] sees nothing — while the + /// second suppresses every finding of its rule and the first suppresses + /// none. Extending a dead waiver is the canonical way to weaken a + /// suppression, and this is the key that exists to catch it. + /// + /// `WaiverAdded`'s note about dates does not reach this: that one is about + /// judging one waiver against *today*, where a clock would make the pointer + /// depend on the run's date. This compares one file's `expires` against the + /// other's, which is date-independent and byte-stable. + WaiverExpiryExtended, + /// A rule's predicate changed while its id and severity stayed put — a glob + /// narrowed, a pattern rewritten, an exclusion added. + /// + /// Reported as a *change*, never as a ranking: whether one glob is narrower + /// than another is a judgement this module refuses to make, but that the + /// predicate moved at all is a byte comparison. A glob narrowed to match + /// nothing is the case that used to survive silently. + RulePredicateChanged, + /// The `min_batten_version` floor dropped, or stopped being declared — + /// admitting a binary that does not understand the rules (CLOUD-33). + MinVersionLowered, + /// A path is gone from `epoch.tracked`, so the `config_epoch` attributes + /// less than it did (CLOUD-32). + EpochPathRemoved, + /// A `[[verb]]` row is gone, so a mutating tool call is no longer mediated + /// at the `PreToolUse` boundary (CLOUD-36). + VerbRemoved, + /// A `[[marker]]` row is gone, so its suppressions stop being counted. + MarkerRemoved, + /// An `[[exec_pattern]]` row is gone, so a lying exit `0` carrying it stops + /// being promoted (CLOUD-117). + ExecPatternRemoved, + /// A `[[provision]]` row is gone, so a pinned tool stops being verified. + ProvisionRemoved, + /// A required check is gone from the `[ci]` projection (CLOUD-54). + CiMergeCheckRemoved, + /// The merge-method constraint admits a method it did not, or stopped + /// constraining methods at all. + CiMergeMethodAdded, + /// A pattern is gone from one of `[attribution]`'s deny lists, so a + /// spelling it refused is admitted again (CLOUD-274). + AttributionDenyRemoved, + /// A pattern arrived in `attribution.trailer_allow`, widening the carve-out + /// from the trailer deny list. + AttributionAllowAdded, + /// The `[defects]` table is gone, so the append-only ledger gate is no + /// longer active (CLOUD-52). + DefectsLedgerRemoved, + /// A class arrived in `defects.classes`, so a record the ledger refused is + /// admitted. + DefectsClassAdded, + /// The transcript path is gone, so `check` stops reading the completed + /// session it judged against (CLOUD-95). + TranscriptPathRemoved, + /// A `[budget.]` table is gone, so nothing is counted for it + /// (CLOUD-50). + BudgetSetRemoved, + /// A counted file glob is gone from a budget set. + BudgetFileRemoved, + /// An `[[budget..embedded]]` declaration is gone, so a string a host + /// always loads stops being counted (CLOUD-298). + BudgetEmbeddedRemoved, + /// A ceiling rose, or stopped being declared. For a budget smaller is + /// stricter, so §8's "may not weaken" reads as "may not raise" — the + /// direction inverts, exactly as `design::effective_cap` says. + BudgetLimitRaised, + /// `must_land_on` is gone, so `worktree status` has no target to judge work + /// against (CLOUD-51). + MustLandOnRemoved, + /// The worktree pileup threshold rose, or stopped being declared. The + /// verdict is `count >= threshold`, so a higher number tolerates more + /// (CLOUD-46). + PileupThresholdRaised, + /// A content class arrived in `judge.raw`, so bytes that could not cross + /// into a model call now can (CLOUD-135). + JudgeRawClassAdded, + /// The assembled-payload ceiling rose, so more bytes may cross. + JudgePayloadLimitRaised, + /// The per-capture byte ceiling rose, so a larger capture stops being worth + /// a second look (CLOUD-53). + DesignCaptureLimitRaised, } impl WeakeningKind { @@ -112,6 +198,30 @@ impl WeakeningKind { WeakeningKind::RuleRemoved, WeakeningKind::SeverityLowered, WeakeningKind::WaiverAdded, + WeakeningKind::WaiverExpiryExtended, + WeakeningKind::RulePredicateChanged, + WeakeningKind::MinVersionLowered, + WeakeningKind::EpochPathRemoved, + WeakeningKind::VerbRemoved, + WeakeningKind::MarkerRemoved, + WeakeningKind::ExecPatternRemoved, + WeakeningKind::ProvisionRemoved, + WeakeningKind::CiMergeCheckRemoved, + WeakeningKind::CiMergeMethodAdded, + WeakeningKind::AttributionDenyRemoved, + WeakeningKind::AttributionAllowAdded, + WeakeningKind::DefectsLedgerRemoved, + WeakeningKind::DefectsClassAdded, + WeakeningKind::TranscriptPathRemoved, + WeakeningKind::BudgetSetRemoved, + WeakeningKind::BudgetFileRemoved, + WeakeningKind::BudgetEmbeddedRemoved, + WeakeningKind::BudgetLimitRaised, + WeakeningKind::MustLandOnRemoved, + WeakeningKind::PileupThresholdRaised, + WeakeningKind::JudgeRawClassAdded, + WeakeningKind::JudgePayloadLimitRaised, + WeakeningKind::DesignCaptureLimitRaised, ]; /// The stable, lowercase identifier used in machine output (§6). @@ -125,6 +235,30 @@ impl WeakeningKind { WeakeningKind::RuleRemoved => "rule-removed", WeakeningKind::SeverityLowered => "severity-lowered", WeakeningKind::WaiverAdded => "waiver-added", + WeakeningKind::WaiverExpiryExtended => "waiver-expiry-extended", + WeakeningKind::RulePredicateChanged => "rule-predicate-changed", + WeakeningKind::MinVersionLowered => "min-version-lowered", + WeakeningKind::EpochPathRemoved => "epoch-path-removed", + WeakeningKind::VerbRemoved => "verb-removed", + WeakeningKind::MarkerRemoved => "marker-removed", + WeakeningKind::ExecPatternRemoved => "exec-pattern-removed", + WeakeningKind::ProvisionRemoved => "provision-removed", + WeakeningKind::CiMergeCheckRemoved => "ci-merge-check-removed", + WeakeningKind::CiMergeMethodAdded => "ci-merge-method-added", + WeakeningKind::AttributionDenyRemoved => "attribution-deny-removed", + WeakeningKind::AttributionAllowAdded => "attribution-allow-added", + WeakeningKind::DefectsLedgerRemoved => "defects-ledger-removed", + WeakeningKind::DefectsClassAdded => "defects-class-added", + WeakeningKind::TranscriptPathRemoved => "transcript-path-removed", + WeakeningKind::BudgetSetRemoved => "budget-set-removed", + WeakeningKind::BudgetFileRemoved => "budget-file-removed", + WeakeningKind::BudgetEmbeddedRemoved => "budget-embedded-removed", + WeakeningKind::BudgetLimitRaised => "budget-limit-raised", + WeakeningKind::MustLandOnRemoved => "must-land-on-removed", + WeakeningKind::PileupThresholdRaised => "pileup-threshold-raised", + WeakeningKind::JudgeRawClassAdded => "judge-raw-class-added", + WeakeningKind::JudgePayloadLimitRaised => "judge-payload-limit-raised", + WeakeningKind::DesignCaptureLimitRaised => "design-capture-limit-raised", } } } @@ -164,6 +298,18 @@ pub struct FieldCoverage { /// census until somebody decides. A hand-kept list of *fields* would drift on the /// next key added, which is the defect rather than a second copy of it. pub const CENSUS: &[FieldCoverage] = &[ + FieldCoverage { + field: "version", + coverage: Coverage::NotPolicyBearing( + "the schema version this build understands. A file declaring another one is \ + refused at parse rather than partially interpreted, so no value of it lowers \ + a bar", + ), + }, + FieldCoverage { + field: "min_batten_version", + coverage: Coverage::Compared(&[WeakeningKind::MinVersionLowered]), + }, FieldCoverage { field: "strictness", coverage: Coverage::Compared(&[WeakeningKind::StrictnessLowered]), @@ -172,6 +318,22 @@ pub const CENSUS: &[FieldCoverage] = &[ field: "fail_on_warning", coverage: Coverage::Compared(&[WeakeningKind::PromotionDisabled]), }, + FieldCoverage { + field: "rules", + coverage: Coverage::Compared(&[ + WeakeningKind::RuleRemoved, + WeakeningKind::SeverityLowered, + WeakeningKind::RulePredicateChanged, + ]), + }, + FieldCoverage { + field: "scope", + coverage: Coverage::NoMonotoneReading( + "§8 lists narrowing scope among the TIGHTENING moves — a smaller scope polices \ + less but forgives nothing inside what remains — and widening it polices more. \ + Neither direction lowers a bar", + ), + }, FieldCoverage { field: "protected", coverage: Coverage::Compared(&[WeakeningKind::ProtectedRemoved]), @@ -181,12 +343,127 @@ pub const CENSUS: &[FieldCoverage] = &[ coverage: Coverage::Compared(&[WeakeningKind::UnlandedRemoved]), }, FieldCoverage { - field: "rules", - coverage: Coverage::Compared(&[WeakeningKind::RuleRemoved, WeakeningKind::SeverityLowered]), + field: "epoch", + coverage: Coverage::Compared(&[WeakeningKind::EpochPathRemoved]), + }, + FieldCoverage { + field: "verbs", + coverage: Coverage::Compared(&[WeakeningKind::VerbRemoved]), + }, + FieldCoverage { + field: "redirects", + coverage: Coverage::NotPolicyBearing( + "a redirect changes what a refusal SAYS, never whether it fires (CLOUD-280); \ + `the_protected_weakening_key_survives_the_redirect_table` asserts both \ + directions are clean", + ), + }, + FieldCoverage { + field: "exec_patterns", + coverage: Coverage::Compared(&[WeakeningKind::ExecPatternRemoved]), + }, + FieldCoverage { + field: "exec", + coverage: Coverage::NoMonotoneReading( + "dispatch shape and presentation: process-group ownership, tee, format, style, \ + jobs, continue-on-error. None of them decides whether a finding is produced or \ + what it is judged against — `exec`'s verdict is the child's own exit code plus \ + `exec_pattern`, which is compared on its own row", + ), + }, + FieldCoverage { + field: "markers", + coverage: Coverage::Compared(&[WeakeningKind::MarkerRemoved]), }, FieldCoverage { field: "waivers", - coverage: Coverage::Compared(&[WeakeningKind::WaiverAdded]), + coverage: Coverage::Compared(&[ + WeakeningKind::WaiverAdded, + WeakeningKind::WaiverExpiryExtended, + ]), + }, + FieldCoverage { + field: "budget", + coverage: Coverage::Compared(&[ + WeakeningKind::BudgetSetRemoved, + WeakeningKind::BudgetFileRemoved, + WeakeningKind::BudgetEmbeddedRemoved, + WeakeningKind::BudgetLimitRaised, + ]), + }, + FieldCoverage { + field: "must_land_on", + coverage: Coverage::Compared(&[WeakeningKind::MustLandOnRemoved]), + }, + FieldCoverage { + field: "worktree", + coverage: Coverage::Compared(&[WeakeningKind::PileupThresholdRaised]), + }, + FieldCoverage { + field: "hook", + coverage: Coverage::NoMonotoneReading( + "an action is a side effect attached to a hook event (CLOUD-91), not a bar: \ + removing one stops something running and adding one runs more, and neither \ + forgives a finding. What an action may BE is refused at load, which is where \ + that risk is decided", + ), + }, + FieldCoverage { + field: "judge", + coverage: Coverage::Compared(&[ + WeakeningKind::JudgeRawClassAdded, + WeakeningKind::JudgePayloadLimitRaised, + ]), + }, + FieldCoverage { + field: "design", + coverage: Coverage::Compared(&[WeakeningKind::DesignCaptureLimitRaised]), + }, + FieldCoverage { + field: "ci", + coverage: Coverage::Compared(&[ + WeakeningKind::CiMergeCheckRemoved, + WeakeningKind::CiMergeMethodAdded, + ]), + }, + FieldCoverage { + field: "defects", + coverage: Coverage::Compared(&[ + WeakeningKind::DefectsLedgerRemoved, + WeakeningKind::DefectsClassAdded, + ]), + }, + FieldCoverage { + field: "provisions", + coverage: Coverage::Compared(&[WeakeningKind::ProvisionRemoved]), + }, + FieldCoverage { + field: "transcript", + coverage: Coverage::Compared(&[WeakeningKind::TranscriptPathRemoved]), + }, + FieldCoverage { + field: "drain", + coverage: Coverage::NoMonotoneReading( + "`resolve` already makes this argument for the same table: an interval has no \ + direction at all — a longer window is quieter and a shorter one is louder, and \ + neither is a weakening the raise-only clamp could measure. The caps beside it \ + pace the same emitter", + ), + }, + FieldCoverage { + field: "attribution", + coverage: Coverage::Compared(&[ + WeakeningKind::AttributionDenyRemoved, + WeakeningKind::AttributionAllowAdded, + ]), + }, + FieldCoverage { + field: "commit", + coverage: Coverage::NoMonotoneReading( + "one key, a subject pattern, and two regexes cannot be ranked without a \ + judgement. Its absence forgives nothing either: the gate reports an absent \ + table as exit 1, never as a clean pass over commits it had no rule to judge", + ), }, ]; @@ -322,10 +599,467 @@ pub fn weakenings(base: &Config, working: &Config) -> Vec { .collect::>(), )); + // Everything below arrived with CLOUD-721, in `Config`'s own declaration + // order. Which keys are compared at all is no longer a matter of what + // occurred to an author: `CENSUS` records a verdict for every field and its + // test fails on any field carrying none. + + found.extend(min_version_weakening(base, working)); + + // The epoch's tracked set: a path removed is a file the `config_epoch` stops + // attributing, so a change to it stamps nothing (CLOUD-32). + found.extend(removed_entries( + WeakeningKind::EpochPathRemoved, + &tracked_paths(base), + &tracked_paths(working), + "epoch.tracked", + )); + + // The mutating-verb table: a removed row un-gates a tool call at the + // `PreToolUse` boundary, which is the most consequential of these. + found.extend(removed_entries( + WeakeningKind::VerbRemoved, + &verb_entries(base), + &verb_entries(working), + "verb", + )); + + found.extend(removed_entries( + WeakeningKind::MarkerRemoved, + &ids(base.markers.iter().map(|marker| marker.id.clone())), + &ids(working.markers.iter().map(|marker| marker.id.clone())), + "marker", + )); + found.extend(removed_entries( + WeakeningKind::ExecPatternRemoved, + &ids(base.exec_patterns.iter().map(|row| row.id.clone())), + &ids(working.exec_patterns.iter().map(|row| row.id.clone())), + "exec_pattern", + )); + found.extend(removed_entries( + WeakeningKind::ProvisionRemoved, + &ids(base.provisions.iter().map(|row| row.name.clone())), + &ids(working.provisions.iter().map(|row| row.name.clone())), + "provision", + )); + + // The escape hatch's second direction (CLOUD-721): the key pairs the two + // files, and the expiry inside it is what the pairing was blind to. + found.extend(waiver_expiry_weakenings(&base.waivers, &working.waivers)); + + found.extend(budget_weakenings( + base.budget.as_ref(), + working.budget.as_ref(), + )); + + // `must_land_on` gone leaves `worktree status` with no target — exit 1, and + // a gate that cannot judge. A *changed* ref is not compared: two trunk names + // cannot be ranked without knowing which repository they belong to. + if base.must_land_on.is_some() && working.must_land_on.is_none() { + found.push(Weakening::new( + WeakeningKind::MustLandOnRemoved, + "must_land_on", + "present", + "absent", + )); + } + + // `count >= pileup_threshold`, so a higher number tolerates more and an + // absent one takes the predicate out of the verdict entirely (CLOUD-46). + found.extend(ceiling_raised( + WeakeningKind::PileupThresholdRaised, + "worktree.pileup_threshold", + base.worktree + .as_ref() + .and_then(|table| table.pileup_threshold), + working + .worktree + .as_ref() + .and_then(|table| table.pileup_threshold), + )); + + // The judge's privacy boundary (CLOUD-135). Compared from both sides + // regardless of whether either declares the table: an absent `[judge]` is + // the *tightest* setting — pointer-only, at the engine's ceiling — so a + // working tree that adds one widens the boundary and must be reported. + found.extend(added_entries( + WeakeningKind::JudgeRawClassAdded, + &raw_classes(base), + &raw_classes(working), + )); + found.extend(ceiling_raised( + WeakeningKind::JudgePayloadLimitRaised, + "judge.max_payload_bytes", + Some(payload_ceiling(base)), + Some(payload_ceiling(working)), + )); + + // `design::effective_cap` states the direction: for a budget smaller is + // stricter, so §8's "may not weaken" reads as "may not raise" here. + found.extend(ceiling_raised( + WeakeningKind::DesignCaptureLimitRaised, + "design.max_capture_bytes", + Some(capture_ceiling(base)), + Some(capture_ceiling(working)), + )); + + found.extend(ci_weakenings(base.ci.as_ref(), working.ci.as_ref())); + found.extend(attribution_weakenings( + base.attribution.as_ref(), + working.attribution.as_ref(), + )); + found.extend(defects_weakenings( + base.defects.as_ref(), + working.defects.as_ref(), + )); + + // A transcript path gone stops `check` reading the session it judged + // against (CLOUD-95). Keyed on the effective path rather than the table, so + // deleting `[transcript]` and blanking its `path` report the same key. + if transcript_path(base).is_some() && transcript_path(working).is_none() { + found.push(Weakening::new( + WeakeningKind::TranscriptPathRemoved, + "transcript.path", + "present", + "absent", + )); + } + found.sort(); found } +/// The `epoch.tracked` set, or an empty one when the table is absent. +fn tracked_paths(config: &Config) -> Vec { + config + .epoch + .as_ref() + .map_or_else(Vec::new, |epoch| epoch.tracked.clone()) +} + +/// Each `[[verb]]` row as the entry a weakening keys on. +/// +/// The rendering is the report's, not a second definition of a verb's identity: +/// it is the same pair `crate::verbs` matches on — the program and the +/// subcommand that qualifies it — written the way an author typed the row, so +/// `verb[git push]` reads as the call it un-gates. +fn verb_entries(config: &Config) -> Vec { + config + .verbs + .iter() + .map(|row| match row.subcommand.as_deref() { + None => row.verb.clone(), + Some(subcommand) => format!("{} {subcommand}", row.verb), + }) + .collect() +} + +/// The ids of a table, collected so [`removed_entries`] can compare them. +fn ids(entries: impl Iterator) -> Vec { + entries.collect() +} + +/// The content classes admitted raw into a model call, as rendered key paths. +/// +/// Empty — including for a config with no `[judge]` at all — is the pointer-only +/// default, which is why an absent table compares as the tightest setting. +fn raw_classes(config: &Config) -> Vec { + config.judge.as_ref().map_or_else(Vec::new, |judge| { + judge + .raw + .iter() + .map(|class| format!("judge.raw[{}]", class.as_str())) + .collect() + }) +} + +/// The effective assembled-payload ceiling: the declared one, or the engine's. +fn payload_ceiling(config: &Config) -> usize { + config + .judge + .as_ref() + .and_then(|judge| judge.max_payload_bytes) + .unwrap_or(crate::judge::DEFAULT_MAX_PAYLOAD_BYTES) +} + +/// The effective per-capture ceiling: the declared one, or the engine's. +fn capture_ceiling(config: &Config) -> usize { + config + .design + .as_ref() + .and_then(|design| design.max_capture_bytes) + .unwrap_or(crate::design::DEFAULT_MAX_CAPTURE_BYTES) +} + +/// The effective transcript path, or `None` when the table or the key is absent. +fn transcript_path(config: &Config) -> Option<&str> { + config + .transcript + .as_ref() + .and_then(|table| table.path.as_deref()) +} + +/// A ceiling that rose, or stopped being declared at all. +/// +/// The inverted direction, stated once: for a threshold a *larger* number +/// forgives more, so `working > base` is the weakening — and `None` where the +/// base had a value is the widest of all, because the predicate stops +/// participating. Callers that have a compiled-in default pass the effective +/// value on both sides instead, so "the key was deleted" and "the key was set to +/// the default" cannot read differently. +fn ceiling_raised( + kind: WeakeningKind, + key: &str, + base: Option, + working: Option, +) -> Option { + match (base, working) { + (Some(base), None) => Some(Weakening::new(kind, key, base.to_string(), "absent")), + (Some(base), Some(working)) if working > base => Some(Weakening::new( + kind, + key, + base.to_string(), + working.to_string(), + )), + _ => None, + } +} + +/// Entries rendered as their own key paths, for [`added_entries`]. +fn keyed(key: &str, entries: &[String]) -> Vec { + entries + .iter() + .map(|entry| format!("{key}[{entry}]")) + .collect() +} + +/// The `min_batten_version` floor, lowered or dropped. +/// +/// A binary below the floor is refused at parse (CLOUD-33), so lowering it +/// admits one that does not understand the rules — and deleting the key admits +/// every build there has ever been, which is why absence is the weakest value +/// rather than "no opinion". An unparseable version cannot reach here: the same +/// parse that produced these configs refuses it. +fn min_version_weakening(base: &Config, working: &Config) -> Option { + let declared = base.min_batten_version.as_deref()?; + let floor = semver::Version::parse(declared).ok()?; + match working.min_batten_version.as_deref() { + None => Some(Weakening::new( + WeakeningKind::MinVersionLowered, + "min_batten_version", + declared, + "absent", + )), + Some(candidate) => match semver::Version::parse(candidate) { + Ok(parsed) if parsed < floor => Some(Weakening::new( + WeakeningKind::MinVersionLowered, + "min_batten_version", + declared, + candidate, + )), + _ => None, + }, + } +} + +/// Waivers whose expiry the working tree pushed further out. +/// +/// Paired by [`crate::waiver::Waiver::key`], which is the identity two waivers +/// may not share — and which deliberately omits `expires`, so this comparison is +/// exactly the blind spot that identity leaves: same rule, same path, a date +/// moved from lapsed to live suppresses everything the base ref reported and +/// [`added_entries`] sees no new key at all. +/// +/// Date-independent, so §6 holds: one file's `expires` against the other's, +/// never against today. A malformed expiry is refused at load, so a pair that +/// cannot both be parsed is unreachable through the loader and is skipped rather +/// than guessed at. +fn waiver_expiry_weakenings(base: &[waiver::Waiver], working: &[waiver::Waiver]) -> Vec { + base.iter() + .filter_map(|row| { + let other = working.iter().find(|other| other.key() == row.key())?; + let (Ok(from), Ok(to)) = (row.expiry(), other.expiry()) else { + return None; + }; + (to > from).then(|| { + Weakening::new( + WeakeningKind::WaiverExpiryExtended, + format!("{}.expires", row.key()), + row.expires.clone(), + other.expires.clone(), + ) + }) + }) + .collect() +} + +/// Budget sets removed, files or embedded declarations dropped, ceilings raised. +fn budget_weakenings( + base: Option<&crate::budget::Budget>, + working: Option<&crate::budget::Budget>, +) -> Vec { + let mut found = Vec::new(); + let Some(base) = base else { + return found; + }; + for (name, set) in base.sets() { + let Some(other) = working + .and_then(|table| table.sets().find(|(other, _)| *other == name)) + .map(|(_, set)| set) + else { + found.push(Weakening::new( + WeakeningKind::BudgetSetRemoved, + format!("budget[{name}]"), + "present", + "absent", + )); + continue; + }; + found.extend(removed_entries( + WeakeningKind::BudgetFileRemoved, + &set.files, + &other.files, + &format!("budget[{name}].files"), + )); + found.extend(removed_entries( + WeakeningKind::BudgetEmbeddedRemoved, + &embedded_entries(set), + &embedded_entries(other), + &format!("budget[{name}].embedded"), + )); + found.extend(ceiling_raised( + WeakeningKind::BudgetLimitRaised, + &format!("budget[{name}].max_tokens"), + Some(set.max_tokens), + Some(other.max_tokens), + )); + found.extend(ceiling_raised( + WeakeningKind::BudgetLimitRaised, + &format!("budget[{name}].max_lines"), + set.max_lines, + other.max_lines, + )); + } + found +} + +/// Each embedded declaration as one entry: the document, then the key in it. +fn embedded_entries(set: &crate::budget::BudgetSet) -> Vec { + set.embedded + .iter() + .map(|decl| format!("{}#{}", decl.path, decl.key)) + .collect() +} + +/// Required checks dropped, and merge methods admitted. +/// +/// The projection is a copy of the host ruleset a gate polices (CLOUD-54), so +/// dropping a check from it is how a branch would stop `config lint --host-rules` +/// asking about that check at all. An absent `allowed_merge_methods` is +/// *unconstrained*, which is why losing the key is a weakening on its own and +/// carries a key of its own rather than one entry per method nobody listed. +fn ci_weakenings(base: Option<&crate::ci::Ci>, working: Option<&crate::ci::Ci>) -> Vec { + let mut found = Vec::new(); + let Some(base) = base else { + return found; + }; + let working_checks = working.map_or_else(Vec::new, |ci| ci.required_checks.clone()); + found.extend(removed_entries( + WeakeningKind::CiMergeCheckRemoved, + &base.required_checks, + &working_checks, + "ci.required_checks", + )); + + if let Some(declared) = base.allowed_merge_methods.as_ref() { + match working.and_then(|ci| ci.allowed_merge_methods.as_ref()) { + None => found.push(Weakening::new( + WeakeningKind::CiMergeMethodAdded, + "ci.allowed_merge_methods", + "constrained", + "unconstrained", + )), + Some(candidate) => found.extend(added_entries( + WeakeningKind::CiMergeMethodAdded, + &keyed("ci.allowed_merge_methods", declared), + &keyed("ci.allowed_merge_methods", candidate), + )), + } + } + found +} + +/// Deny patterns dropped, and carve-outs added. +/// +/// The `identity` beneath them is not compared: which name and email a +/// repository holds itself accountable to is that repository's own decision, and +/// two identities cannot be ranked. Nor is the table's absence — the gate reports +/// an absent `[attribution]` as exit 1, never as a clean pass, so removing it +/// forgives nothing. +fn attribution_weakenings( + base: Option<&crate::attribution::Attribution>, + working: Option<&crate::attribution::Attribution>, +) -> Vec { + let mut found = Vec::new(); + let Some(base) = base else { + return found; + }; + let empty: Vec = Vec::new(); + for (index, (key, declared)) in deny_lists(base).into_iter().enumerate() { + let candidate = working.map_or(&empty, |other| deny_lists(other)[index].1); + found.extend(removed_entries( + WeakeningKind::AttributionDenyRemoved, + declared, + candidate, + key, + )); + } + let candidate = working.map_or(&empty, |other| &other.trailer_allow); + found.extend(added_entries( + WeakeningKind::AttributionAllowAdded, + &keyed("attribution.trailer_allow", &base.trailer_allow), + &keyed("attribution.trailer_allow", candidate), + )); + found +} + +/// The three deny lists, keyed as `batten.toml` spells them. +fn deny_lists(attribution: &crate::attribution::Attribution) -> [(&'static str, &Vec); 3] { + [ + ("attribution.identity_deny", &attribution.identity_deny), + ("attribution.trailer_deny", &attribution.trailer_deny), + ("attribution.body_deny", &attribution.body_deny), + ] +} + +/// The ledger gone, or its class set widened. +/// +/// Absence here reads differently from `[attribution]`'s: a repository with no +/// `[defects]` keeps no in-tree ledger and the gate is simply not active, so +/// deleting the table is a silent deactivation rather than a loud refusal. A +/// class *added* admits a record the ledger used to refuse. +fn defects_weakenings( + base: Option<&crate::defects::Defects>, + working: Option<&crate::defects::Defects>, +) -> Vec { + let Some(base) = base else { + return Vec::new(); + }; + let Some(working) = working else { + return vec![Weakening::new( + WeakeningKind::DefectsLedgerRemoved, + "defects", + "present", + "absent", + )]; + }; + added_entries( + WeakeningKind::DefectsClassAdded, + &keyed("defects.classes", &base.classes), + &keyed("defects.classes", &working.classes), + ) +} + /// Entries present in `base` and absent from `working`, as weakenings of `key`. fn removed_entries( kind: WeakeningKind, @@ -384,9 +1118,107 @@ fn rule_weakenings(base: &[Rule], working: &[Rule]) -> Vec { Some(_) => None, } }) + .chain(base.iter().flat_map(|rule| { + working + .iter() + .find(|other| other.id == rule.id) + .map_or_else(Vec::new, |other| rule_predicate_weakenings(rule, other)) + })) + .collect() +} + +/// Rule columns that are not the rule's predicate, each with why not. +/// +/// Every *other* column is compared, which is the fail-safe direction: a column +/// added to [`Rule`] is compared until somebody exempts it here with a reason, +/// rather than silently joining the set nothing looks at — the same defect one +/// level down from the one [`CENSUS`] closes. +const RULE_NON_PREDICATE: &[(&str, &str)] = &[ + ( + "id", + "the rule's identity: it is what this comparison is keyed BY, so a changed id \ + is a removed rule and an added one", + ), + ( + "severity", + "compared as a rank of its own by `severity-lowered`, because severity has an \ + ordering where a predicate does not", + ), + ( + "reason", + "prose a refusal prints. It changes what a reader is told, never whether the \ + rule fires", + ), + ( + "policy_url", + "a pointer at the documentation behind `reason`, for `reason`'s reason", + ), + ( + "fix", + "the remediation offered after a finding, never whether one is produced", + ), +]; + +/// Predicate columns of one rule that the working tree changed. +/// +/// **A change, never a ranking.** Whether a narrowed glob is weaker than the one +/// it replaced is a judgement about two patterns, and this module does not make +/// judgements. That the predicate moved at all is a byte comparison, and it is +/// the fact that used to be reported nowhere: a rule whose glob matches nothing +/// keeps its id and its severity, so the comparison called it unchanged while it +/// gated nothing. +/// +/// Read off the rule's own serialization rather than a hand-kept column list, +/// which would drift the next time [`Rule`] grows one. The tokens are digests: +/// a config's patterns are the consumer's, and a pointer names *which* column +/// moved without carrying what it now says (non-negotiable rule 4). +fn rule_predicate_weakenings(base: &Rule, working: &Rule) -> Vec { + let (Ok(serde_json::Value::Object(from)), Ok(serde_json::Value::Object(to))) = + (serde_json::to_value(base), serde_json::to_value(working)) + else { + return Vec::new(); + }; + let mut columns: Vec<&String> = from.keys().chain(to.keys()).collect(); + columns.sort_unstable(); + columns.dedup(); + columns + .into_iter() + .filter(|column| { + !RULE_NON_PREDICATE + .iter() + .any(|(exempt, _)| exempt == column) + }) + .filter_map(|column| { + let (was, now) = ( + from.get(column).unwrap_or(&serde_json::Value::Null), + to.get(column).unwrap_or(&serde_json::Value::Null), + ); + (was != now).then(|| { + Weakening::new( + WeakeningKind::RulePredicateChanged, + format!("rule[{}].{column}", base.id), + column_token(was), + column_token(now), + ) + }) + }) .collect() } +/// One column's value as a stable token: a digest, or `absent`. +/// +/// Never the value itself. A rule's patterns are the consumer's config, and §6's +/// byte-stability is satisfied by a hash of them exactly as it is by a name — +/// two runs over the same pair of files produce the same token, and neither run +/// prints what the pattern says. +fn column_token(value: &serde_json::Value) -> String { + if value.is_null() { + return "absent".to_owned(); + } + let digest = crate::receipt::hex_sha256(value.to_string().as_bytes()); + format!("sha256:{}", &digest[..12]) +} + /// The lowercase token a [`Strictness`] is written as, read off its `ValueEnum` /// derive rather than re-tabulated here. fn strictness_token(strictness: Strictness) -> String { From c7a33926e959bbf8184d1c47b55c85da839fbf84 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 19 Aug 2026 23:27:48 +0000 Subject: [PATCH 3/4] test(trust): a both-directions case per compared key, and one that reaches the verb MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-721's test obligation. Every kind gets a case that fails if the comparison is wired backwards — asserting the tightening direction is clean is what a "something was reported" assertion cannot do. Two of them are the cases the issue named, and both were reported CLEAN on arrival: a waiver whose expiry moves from 2020 to 2099 under an unchanged rule and path, and a rule whose glob narrows to match nothing under an unchanged id and severity. A kind census sits beside the field census: every `WeakeningKind::ALL` variant must be named by a case in the module. It found `unlanded` had never had one of its own — the set is evaluated independently of `protected` (CLOUD-37), so a case covering only the sibling would not notice if the two were collapsed — and that case is here now. The E2E pair takes three of the keys through the compiled binary, where two things can only be checked at the boundary: each key carries its own pointer so none collapses into another (CLOUD-233), and the digest token in the rule pointer is byte-stable across two runs while never carrying the glob it stands for (non-negotiable rule 4). Dropping the whole `[attribution]` table reports one pointer per dropped deny pattern rather than one saying the table is gone, which is the same per-key rule: what distinguishes two weakenings belongs in the key, and which refusals a branch dropped is what a reviewer needs to see. The module map records the census, the rule-column exemption list and the expiry pairing, so the next reader meets them where every other module's shape is recorded. Refs: CLOUD-721 --- .serena/memories/core.md | 11 +- crates/batten/src/trust.rs | 608 ++++++++++++++++++++++++++++- crates/batten/tests/config_lint.rs | 99 +++++ 3 files changed, 713 insertions(+), 5 deletions(-) diff --git a/.serena/memories/core.md b/.serena/memories/core.md index 6e0089ab2..bd50d35d1 100644 --- a/.serena/memories/core.md +++ b/.serena/memories/core.md @@ -388,7 +388,16 @@ period (2)` in SCHEMA order — so the report never depends on the order the aut expired, since the diff is a fact about two files and the lapse is the run's. Pointer-only `Weakening`s (key path + two verdict tokens), sorted so the report is byte-stable. `config lint` (CLOUD-87) reuses both - rather than growing a second trusted-load path. + rather than growing a second trusted-load path. **What is compared is a census, + not a habit (CLOUD-721)**: `CENSUS` carries a verdict per `Config` field — + compared (with its kinds), no monotone reading, or not policy-bearing, the last + two with the reason — and its test reads the field list off `config.rs`'s own + source, so a key added to the struct fails until somebody decides. `Rule`'s + predicate columns work the same way one level down: every column is compared as + a byte change (`RulePredicateChanged`, digest tokens, never a ranking of two + globs) unless `RULE_NON_PREDICATE` exempts it with a reason. `WaiverExpiryExtended` + is the pairing `Waiver::key` cannot see — same rule and path, expiry pushed out — + and stays date-independent by comparing one file's `expires` against the other's. - `resolve.rs` — house-style §8 precedence resolver: `flag > env > local file > repo config > default`, declared as data in `SETTINGS` (per-key env var/flag), not hard-coded per field. diff --git a/crates/batten/src/trust.rs b/crates/batten/src/trust.rs index 089b90f77..ccc9dfe21 100644 --- a/crates/batten/src/trust.rs +++ b/crates/batten/src/trust.rs @@ -40,6 +40,22 @@ //! key rather than of this module. Adding a `protected` path is still clean; //! adding a waiver is not. //! +//! # Coverage is a census, not a habit (CLOUD-721) +//! +//! This comparison once covered six keys of a twenty-eight-key struct, and +//! nothing noticed: `hk` re-runs `config-lint` on a diff touching *this* file +//! and never on one that grows `config.rs` a key, so every key landed since +//! CLOUD-31 arrived with no prompt to ask whether it has a weakening direction. +//! For `check` that only under-reports; for `config lint` the weakening list +//! **is** the verdict, so an uncovered key is a weakening that gate cannot see. +//! +//! [`CENSUS`] closes it as a mechanism rather than a longer list. Its field list +//! is read off [`Config`]'s own source, so a field added to the struct fails the +//! census until somebody records one of three answers — compared, no monotone +//! reading, or not policy-bearing — with the reason beside the last two. Silence +//! is not one of the three, which is what the first version of this module let +//! it be. +//! //! Every comparison is key-local and order-independent, so the report is a //! deterministic function of the two configs and nothing else. @@ -991,11 +1007,16 @@ fn ci_weakenings(base: Option<&crate::ci::Ci>, working: Option<&crate::ci::Ci>) /// Deny patterns dropped, and carve-outs added. /// -/// The `identity` beneath them is not compared: which name and email a +/// Dropping the whole table drops every pattern with it, and it is reported that +/// way — one pointer per pattern rather than one saying `[attribution]` is gone. +/// The per-key precision is CLOUD-233's rule: what distinguishes two weakenings +/// belongs in the key, and "which refusals this branch dropped" is exactly what a +/// reviewer needs. The gate refuses an absent table loudly on its own (exit 1), +/// which is a second signal rather than a reason for this one to stay quiet. +/// +/// The `identity` beneath the lists is not compared: which name and email a /// repository holds itself accountable to is that repository's own decision, and -/// two identities cannot be ranked. Nor is the table's absence — the gate reports -/// an absent `[attribution]` as exit 1, never as a clean pass, so removing it -/// forgives nothing. +/// two identities cannot be ranked. fn attribution_weakenings( base: Option<&crate::attribution::Attribution>, working: Option<&crate::attribution::Attribution>, @@ -1346,6 +1367,27 @@ mod tests { assert!(weakenings(&base, &working).is_empty()); } + #[test] + fn removing_an_unlanded_path_is_a_weakening() { + // `unlanded` is evaluated independently of `protected` (CLOUD-37), so it + // needs its own case: the two sets must never be collapsed, and a test + // that only covered one would not notice if they were. That this case was + // missing is what `every_kind_is_exercised_by_a_case_in_this_module` + // found on the tree CLOUD-721 arrived at. + let base = parse("version = 1\nunlanded = [\"a\", \"b\"]\n"); + let working = parse("version = 1\nunlanded = [\"a\"]\n"); + assert_eq!( + weakenings(&base, &working), + vec![Weakening::new( + WeakeningKind::UnlandedRemoved, + "unlanded[b]", + "present", + "absent" + )] + ); + assert!(weakenings(&working, &base).is_empty()); + } + #[test] fn narrowing_scope_is_not_a_weakening() { // §8 lists "narrow scope" among the *tightening* moves: a smaller scope @@ -1646,4 +1688,562 @@ mod tests { tokens.dedup(); assert_eq!(tokens.len(), count, "two kinds share one token"); } + + // --- CLOUD-721: one both-directions case per newly compared key ---------- + // + // The shape the six original cases use, and the reason it is repeated per + // key rather than folded into a table: the *direction* is a property of the + // key, so a case that only proved "something is reported" would pass on a + // comparison wired backwards. + + fn config(extra: &str) -> Config { + parse(&format!("version = 1\n{extra}")) + } + + /// The single weakening a pair must produce, or a panic naming what it did. + fn only(base: &Config, working: &Config) -> Weakening { + let found = weakenings(base, working); + assert_eq!(found.len(), 1, "expected exactly one weakening: {found:?}"); + found[0].clone() + } + + #[test] + fn lowering_or_deleting_the_version_floor_is_a_weakening() { + // Below the running build in both directions, or `parse` would refuse + // the fixture before the comparison could see it (CLOUD-33). + let base = config("min_batten_version = \"0.0.10\"\n"); + let lower = config("min_batten_version = \"0.0.5\"\n"); + assert_eq!( + only(&base, &lower), + Weakening::new( + WeakeningKind::MinVersionLowered, + "min_batten_version", + "0.0.10", + "0.0.5", + ) + ); + // Deleting it admits every build there has ever been, so absence is the + // weakest value rather than "no opinion". + assert_eq!(only(&base, &config("")).working, "absent"); + // The other direction, and the unchanged one. + assert!(weakenings(&lower, &base).is_empty()); + assert!(weakenings(&base, &base).is_empty()); + } + + #[test] + fn dropping_a_tracked_epoch_path_is_a_weakening() { + let base = config("[epoch]\ntracked = [\"a\", \"b\"]\n"); + let working = config("[epoch]\ntracked = [\"a\"]\n"); + assert_eq!( + only(&base, &working), + Weakening::new( + WeakeningKind::EpochPathRemoved, + "epoch.tracked[b]", + "present", + "absent", + ) + ); + assert!(weakenings(&working, &base).is_empty()); + } + + fn verb_row(verb: &str, subcommand: Option<&str>) -> String { + let qualifier = + subcommand.map_or_else(String::new, |sub| format!("subcommand = \"{sub}\"\n")); + format!("\n[[verb]]\nverb = \"{verb}\"\neffect = \"write\"\n{qualifier}") + } + + #[test] + fn removing_a_mutating_verb_row_is_a_weakening() { + // The most consequential of the keys CLOUD-721 added: a row that is gone + // is a tool call nothing mediates at the `PreToolUse` boundary. + let base = config(&format!( + "{}{}", + verb_row("rm", None), + verb_row("git", Some("push")) + )); + let working = config(&verb_row("rm", None)); + assert_eq!( + only(&base, &working), + Weakening::new( + WeakeningKind::VerbRemoved, + "verb[git push]", + "present", + "absent", + ), + "the subcommand is part of the row's identity, so it is part of the key" + ); + assert!(weakenings(&working, &base).is_empty()); + } + + #[test] + fn removing_a_marker_or_an_exec_pattern_or_a_provision_is_a_weakening() { + let marker = "\n[[marker]]\nid = \"m\"\ntoken = \"SUPPRESSED-HERE\"\n"; + assert_eq!( + only(&config(marker), &config("")), + Weakening::new( + WeakeningKind::MarkerRemoved, + "marker[m]", + "present", + "absent", + ) + ); + assert!(weakenings(&config(""), &config(marker)).is_empty()); + + let pattern = + "\n[[exec_pattern]]\nid = \"p\"\npattern = \"warning\"\nreason = \"fix it\"\n"; + assert_eq!( + only(&config(pattern), &config("")), + Weakening::new( + WeakeningKind::ExecPatternRemoved, + "exec_pattern[p]", + "present", + "absent", + ) + ); + assert!(weakenings(&config(""), &config(pattern)).is_empty()); + + let provision = concat!( + "\n[[provision]]\nname = \"tool\"\nversion = \"1.0.0\"\n", + "unpack = \"tar_gz\"\nbinary = \"tool\"\n", + "url = \"https://example.invalid/tool.tar.gz\"\n", + "sha256 = \"", + "0000000000000000000000000000000000000000000000000000000000000000", + "\"\n" + ); + assert_eq!( + only(&config(provision), &config("")), + Weakening::new( + WeakeningKind::ProvisionRemoved, + "provision[tool]", + "present", + "absent", + ) + ); + assert!(weakenings(&config(""), &config(provision)).is_empty()); + } + + fn dated_waiver(rule: &str, expires: &str) -> String { + format!("\n[[waiver]]\nrule = \"{rule}\"\nreason = \"tracked\"\nexpires = \"{expires}\"\n") + } + + #[test] + fn extending_a_lapsed_waiver_is_a_weakening() { + // CLOUD-721's named case, and it was reported CLEAN before this landed: + // `Waiver::key` omits `expires`, so a base waiver lapsed in 2020 and a + // working one live until 2099 are the same key and `added_entries` sees + // nothing — while the second suppresses every finding of its rule and the + // first suppresses none. + let base = config(&dated_waiver("r", "2020-01-01")); + let working = config(&dated_waiver("r", "2099-01-01")); + assert_eq!( + only(&base, &working), + Weakening::new( + WeakeningKind::WaiverExpiryExtended, + "waiver[r].expires", + "2020-01-01", + "2099-01-01", + ) + ); + // Pulling an expiry IN raises the bar, and the comparison is against the + // other file rather than against today, so neither direction depends on + // the date the comparison ran (§6). + assert!(weakenings(&working, &base).is_empty()); + assert!(weakenings(&working, &working).is_empty()); + } + + #[test] + fn a_narrowed_waiver_path_keeps_its_own_expiry_key() { + // Two waivers of one rule differ by their path, so the expiry pointers do + // too — neither may swallow the other (CLOUD-233). + let row = |path: &str, expires: &str| { + format!( + "\n[[waiver]]\nrule = \"r\"\nreason = \"tracked\"\nexpires = \"{expires}\"\npath = \"{path}\"\n" + ) + }; + let base = config(&format!( + "{}{}", + row("src/**", "2020-01-01"), + row("vendor/**", "2020-01-01") + )); + let working = config(&format!( + "{}{}", + row("src/**", "2099-01-01"), + row("vendor/**", "2099-01-01") + )); + let found = weakenings(&base, &working); + assert_eq!(found.len(), 2, "one pointer per waiver, not one per rule"); + assert_eq!(found[0].key, "waiver[r][src/**].expires"); + assert_eq!(found[1].key, "waiver[r][vendor/**].expires"); + } + + #[test] + fn narrowing_a_rules_glob_to_match_nothing_is_reported() { + // The other case CLOUD-721 named, also clean before this landed: the id + // and the severity are untouched, so the comparison called the rule + // unchanged while it gated nothing. + let base = config(&rule("r", "deny")); + let working = config(&rule("r", "deny").replace("**/*.rs", "nothing/here/**")); + let found = only(&base, &working); + assert_eq!(found.kind, WeakeningKind::RulePredicateChanged); + assert_eq!(found.key, "rule[r].glob"); + // A pointer, never the pattern: the glob is the consumer's config, and + // naming which column moved does not require printing what it now says + // (non-negotiable rule 4). + assert!( + !found.working.contains("nothing"), + "the token must be a digest: {found:?}" + ); + assert!(found.working.starts_with("sha256:")); + // Byte-stable: the same pair produces the same tokens on a second run. + assert_eq!(weakenings(&base, &working), weakenings(&base, &working)); + // And a rule nobody touched reports nothing at all. + assert!(weakenings(&base, &base).is_empty()); + } + + #[test] + fn a_lowered_severity_is_not_also_reported_as_a_predicate_change() { + // `severity` is exempt from the predicate columns because it has a rank + // of its own — reporting both would be one edit with two pointers. + let base = config(&rule("r", "deny")); + let working = config(&rule("r", "warn")); + assert_eq!(only(&base, &working).kind, WeakeningKind::SeverityLowered); + } + + #[test] + fn every_rule_column_exemption_names_a_real_column_and_a_reason() { + // The fail-safe direction one level down from `CENSUS`: a column added to + // `Rule` is compared until somebody exempts it here, so this only has to + // keep the exemptions honest. + let source = include_str!("rules.rs"); + let start = source + .find("pub struct Rule {") + .expect("Rule is declared here"); + let rest = &source[start..]; + let body = &rest[..rest.find("\n}").expect("the struct closes")]; + for (column, reason) in RULE_NON_PREDICATE { + assert!( + body.contains(&format!("pub {column}:")), + "`{column}` is exempted from the predicate comparison but `Rule` has no such column" + ); + assert!( + !reason.trim().is_empty(), + "`{column}` is exempted without saying why" + ); + } + } + + const BUDGET: &str = "\n[budget.set]\nfiles = [\"a.md\", \"b.md\"]\nmax_tokens = 100\nmax_lines = 10\n\n[[budget.set.embedded]]\npath = \"x.toml\"\nkey = \"a.b\"\n"; + + #[test] + fn every_way_of_relaxing_a_budget_is_a_weakening() { + let base = config(BUDGET); + // The whole set gone: nothing is counted for it at all. + assert_eq!( + only(&base, &config("")), + Weakening::new( + WeakeningKind::BudgetSetRemoved, + "budget[set]", + "present", + "absent", + ) + ); + // A counted file gone. + assert_eq!( + only( + &base, + &config(&BUDGET.replace("\"a.md\", \"b.md\"", "\"a.md\"")) + ), + Weakening::new( + WeakeningKind::BudgetFileRemoved, + "budget[set].files[b.md]", + "present", + "absent", + ) + ); + // An embedded declaration gone: a string the host always loads stops + // being counted (CLOUD-298). + let without_embedded = &BUDGET[..BUDGET.find("\n\n[[budget.set.embedded]]").unwrap()]; + assert_eq!( + only(&base, &config(without_embedded)), + Weakening::new( + WeakeningKind::BudgetEmbeddedRemoved, + "budget[set].embedded[x.toml#a.b]", + "present", + "absent", + ) + ); + // A ceiling raised. The direction inverts for a budget: bigger forgives + // more, which `design::effective_cap` states in the same words. + assert_eq!( + only( + &base, + &config(&BUDGET.replace("max_tokens = 100", "max_tokens = 200")) + ), + Weakening::new( + WeakeningKind::BudgetLimitRaised, + "budget[set].max_tokens", + "100", + "200", + ) + ); + // A ceiling deleted, which is wider still: the predicate stops + // participating. + assert_eq!( + only(&base, &config(&BUDGET.replace("max_lines = 10\n", ""))), + Weakening::new( + WeakeningKind::BudgetLimitRaised, + "budget[set].max_lines", + "10", + "absent", + ) + ); + // Tightening in each direction is clean. + assert!( + weakenings( + &base, + &config(&BUDGET.replace("max_tokens = 100", "max_tokens = 50")) + ) + .is_empty() + ); + assert!(weakenings(&config(""), &base).is_empty()); + } + + #[test] + fn losing_the_landing_target_is_a_weakening() { + let base = config("must_land_on = \"origin/main\"\n"); + assert_eq!( + only(&base, &config("")), + Weakening::new( + WeakeningKind::MustLandOnRemoved, + "must_land_on", + "present", + "absent", + ) + ); + assert!(weakenings(&config(""), &base).is_empty()); + } + + #[test] + fn raising_the_pileup_threshold_is_a_weakening() { + // `count >= threshold`, so a higher number tolerates more (CLOUD-46). + let base = config("[worktree]\npileup_threshold = 3\n"); + let raised = config("[worktree]\npileup_threshold = 5\n"); + assert_eq!( + only(&base, &raised), + Weakening::new( + WeakeningKind::PileupThresholdRaised, + "worktree.pileup_threshold", + "3", + "5", + ) + ); + assert_eq!(only(&base, &config("")).working, "absent"); + assert!(weakenings(&raised, &base).is_empty()); + } + + #[test] + fn widening_the_judges_privacy_boundary_is_a_weakening() { + // An absent `[judge]` is the TIGHTEST setting — pointer-only, at the + // engine's ceiling — so a working tree that adds one widens the boundary + // and both halves compare from an absent base. + let none = config(""); + let raw = config("[judge]\nraw = [\"span_text\"]\n"); + assert_eq!( + only(&none, &raw), + Weakening::new( + WeakeningKind::JudgeRawClassAdded, + "judge.raw[span_text]", + "absent", + "present", + ) + ); + assert!(weakenings(&raw, &none).is_empty()); + + let base = config("[judge]\nmax_payload_bytes = 100\n"); + assert_eq!( + only(&base, &config("[judge]\nmax_payload_bytes = 200\n")), + Weakening::new( + WeakeningKind::JudgePayloadLimitRaised, + "judge.max_payload_bytes", + "100", + "200", + ) + ); + // Deleting the key is compared against the engine's default rather than + // read as "no change", the trap `strictness` already documents. + assert_eq!( + only(&base, &none).working, + crate::judge::DEFAULT_MAX_PAYLOAD_BYTES.to_string() + ); + } + + #[test] + fn raising_the_capture_ceiling_is_a_weakening() { + let base = config("[design]\nmax_capture_bytes = 1024\n"); + assert_eq!( + only(&base, &config("[design]\nmax_capture_bytes = 2048\n")), + Weakening::new( + WeakeningKind::DesignCaptureLimitRaised, + "design.max_capture_bytes", + "1024", + "2048", + ) + ); + assert!(weakenings(&base, &config("[design]\nmax_capture_bytes = 512\n")).is_empty()); + } + + #[test] + fn relaxing_the_merge_contract_projection_is_a_weakening() { + let base = config( + "[ci]\nrequired_checks = [\"a\", \"b\"]\nallowed_merge_methods = [\"squash\"]\n", + ); + let fewer = + config("[ci]\nrequired_checks = [\"a\"]\nallowed_merge_methods = [\"squash\"]\n"); + assert_eq!( + only(&base, &fewer), + Weakening::new( + WeakeningKind::CiMergeCheckRemoved, + "ci.required_checks[b]", + "present", + "absent", + ) + ); + assert!(weakenings(&fewer, &base).is_empty()); + + let more = config( + "[ci]\nrequired_checks = [\"a\", \"b\"]\nallowed_merge_methods = [\"squash\", \"merge\"]\n", + ); + assert_eq!( + only(&base, &more), + Weakening::new( + WeakeningKind::CiMergeMethodAdded, + "ci.allowed_merge_methods[merge]", + "absent", + "present", + ) + ); + // Losing the key entirely is unconstrained, which is wider than any list + // — and it carries its own pointer rather than one per method nobody + // listed. + assert_eq!( + only(&base, &config("[ci]\nrequired_checks = [\"a\", \"b\"]\n")), + Weakening::new( + WeakeningKind::CiMergeMethodAdded, + "ci.allowed_merge_methods", + "constrained", + "unconstrained", + ) + ); + } + + const ATTRIBUTION: &str = "\n[attribution]\nidentity_deny = [\"vendor\", \"other\"]\ntrailer_deny = [\"Co-Made-By\"]\nbody_deny = [\"advert\"]\ntrailer_allow = [\"Signed-off-by\"]\n\n[attribution.identity]\nname = \"A Person\"\nemail = \"person@example.invalid\"\n"; + + #[test] + fn shrinking_a_deny_list_or_widening_the_carve_out_is_a_weakening() { + let base = config(ATTRIBUTION); + assert_eq!( + only(&base, &config(&ATTRIBUTION.replace("\"vendor\", ", ""))).key, + "attribution.identity_deny[vendor]" + ); + assert_eq!( + only( + &base, + &config(&ATTRIBUTION.replace( + "trailer_allow = [\"Signed-off-by\"]", + "trailer_allow = [\"Signed-off-by\", \"Co-Made-By\"]" + )) + ), + Weakening::new( + WeakeningKind::AttributionAllowAdded, + "attribution.trailer_allow[Co-Made-By]", + "absent", + "present", + ) + ); + // Dropping the whole table drops every pattern with it, and each one + // keeps its own pointer rather than collapsing into "the table is gone" + // (CLOUD-233). Four patterns, four keys. + let dropped = weakenings(&base, &config("")); + assert_eq!( + dropped.len(), + 4, + "one pointer per dropped pattern: {dropped:?}" + ); + assert!( + dropped + .iter() + .all(|found| found.kind == WeakeningKind::AttributionDenyRemoved), + "the carve-out shrank too, and a shrinking carve-out is tightening: {dropped:?}" + ); + // The other direction: declaring the table where there was none refuses + // more than before. + assert!(weakenings(&config(""), &base).is_empty()); + } + + #[test] + fn deactivating_the_defect_ledger_or_widening_its_classes_is_a_weakening() { + let base = config("[defects]\npath = \"defects.jsonl\"\nclasses = [\"a\"]\n"); + assert_eq!( + only(&base, &config("")), + Weakening::new( + WeakeningKind::DefectsLedgerRemoved, + "defects", + "present", + "absent", + ), + "an absent [defects] is a silent deactivation, unlike [attribution]'s loud one" + ); + assert_eq!( + only( + &base, + &config("[defects]\npath = \"defects.jsonl\"\nclasses = [\"a\", \"b\"]\n") + ), + Weakening::new( + WeakeningKind::DefectsClassAdded, + "defects.classes[b]", + "absent", + "present", + ) + ); + assert!(weakenings(&config(""), &base).is_empty()); + } + + #[test] + fn losing_the_transcript_path_is_a_weakening() { + let base = config("[transcript]\npath = \"session.jsonl\"\n"); + assert_eq!( + only(&base, &config("")), + Weakening::new( + WeakeningKind::TranscriptPathRemoved, + "transcript.path", + "present", + "absent", + ) + ); + // Keyed on the effective path rather than the table, so blanking the key + // and deleting the table report the same pointer. + assert_eq!( + only(&base, &config("[transcript]\n")).key, + "transcript.path" + ); + assert!(weakenings(&config(""), &base).is_empty()); + } + + #[test] + fn every_kind_is_exercised_by_a_case_in_this_module() { + // Rules ship with their mechanism (non-negotiable rule 2): a kind with + // no case is a comparison nothing shows can fire, which is how a + // comparison comes to report the wrong direction and pass. + let source = include_str!("trust.rs"); + let tests = &source[source + .find("mod tests {") + .expect("the test module is declared here")..]; + for kind in WeakeningKind::ALL { + assert!( + tests.contains(&format!("WeakeningKind::{kind:?}")), + "{} has no case in this module", + kind.as_str() + ); + } + } } diff --git a/crates/batten/tests/config_lint.rs b/crates/batten/tests/config_lint.rs index 4d20693bd..6e53d5472 100644 --- a/crates/batten/tests/config_lint.rs +++ b/crates/batten/tests/config_lint.rs @@ -344,6 +344,105 @@ fn the_smell_ids_are_the_same_names_the_check_delta_uses() { assert!(checked.contains("batten.toml:strictness"), "got: {checked}"); } +#[test] +fn the_keys_cloud_721_added_reach_the_lint_with_their_own_pointers() { + // The two cases CLOUD-721 named were reported CLEAN by a comparison that + // claimed to cover their key, and a third — the verb table — was not + // compared at all. All three arrive through the one definition of + // "weakened", so this asserts the whole path rather than the module's own + // view of it. + let base = concat!( + "version = 1\n", + "\n[[verb]]\nverb = \"rm\"\neffect = \"destructive\"\n", + "\n[[rule]]\nid = \"no-todo\"\nkind = \"forbid\"\nglob = \"**/*.rs\"\n", + "pattern = \"TODO\"\nseverity = \"deny\"\n", + "\n[[waiver]]\nrule = \"no-todo\"\nreason = \"tracked\"\nexpires = \"2020-01-01\"\n", + ); + // Same rule, same severity, same waiver key: the glob is narrowed to match + // nothing, the lapsed waiver is extended past the decade, and the mutating + // verb row is deleted. + let working = concat!( + "version = 1\n", + "\n[[rule]]\nid = \"no-todo\"\nkind = \"forbid\"\nglob = \"nothing/here/**\"\n", + "pattern = \"TODO\"\nseverity = \"deny\"\n", + "\n[[waiver]]\nrule = \"no-todo\"\nreason = \"tracked\"\nexpires = \"2099-01-01\"\n", + ); + let repo = pr_fixture("lint-cloud-721-keys", base, working); + + // Single-tree, all three are invisible: every one of them is a statement + // about the pair of files. + assert_eq!(lint(&repo, &[]).status.code(), Some(0)); + + let output = lint(&repo, &["--config-from", "origin/main"]); + assert_eq!(output.status.code(), Some(2)); + assert_eq!( + stdout(&output), + "batten.toml:rule[no-todo].glob rule-predicate-changed\n\ + batten.toml:verb[rm] verb-removed\n\ + batten.toml:waiver[no-todo].expires waiver-expiry-extended\n\ + config-lint: 3 smell(s)\n", + "each key carries its own pointer, so none of the three can collapse \ + into another (CLOUD-233)" + ); + + // Byte-stable across two runs over the same tree (§6), which the digest + // token in the rule pointer is the newest way to get wrong. + assert_eq!( + stdout(&lint(&repo, &["--config-from", "origin/main"])), + stdout(&output) + ); + + // Pointer-only: the narrowed glob is config content and never reaches the + // output (non-negotiable rule 4). + assert!( + !stdout(&output).contains("nothing/here"), + "got: {}", + stdout(&output) + ); +} + +#[test] +fn the_reverse_edit_of_those_keys_is_clean() { + // The direction half, at the same boundary the verdict is taken: a widened + // glob, an expiry pulled in, and a verb row ADDED lower no bar. + let tight = concat!( + "version = 1\n", + "\n[[verb]]\nverb = \"rm\"\neffect = \"destructive\"\n", + "\n[[rule]]\nid = \"no-todo\"\nkind = \"forbid\"\nglob = \"**/*.rs\"\n", + "pattern = \"TODO\"\nseverity = \"deny\"\n", + "\n[[waiver]]\nrule = \"no-todo\"\nreason = \"tracked\"\nexpires = \"2020-01-01\"\n", + ); + let loose = concat!( + "version = 1\n", + "\n[[rule]]\nid = \"no-todo\"\nkind = \"forbid\"\nglob = \"nothing/here/**\"\n", + "pattern = \"TODO\"\nseverity = \"deny\"\n", + "\n[[waiver]]\nrule = \"no-todo\"\nreason = \"tracked\"\nexpires = \"2099-01-01\"\n", + ); + let repo = pr_fixture("lint-cloud-721-reverse", loose, tight); + let output = lint(&repo, &["--config-from", "origin/main"]); + assert_eq!( + stdout(&output), + // The lapsed waiver in the WORKING tree is a single-tree smell of its + // own, and it is there either way — it is what makes the pair honest: + // the base-ref class contributes exactly one line, the predicate change, + // reported as a CHANGE in both directions because ranking two globs + // would be a judgement. + "batten.toml:15 waiver-expired\n\ + batten.toml:rule[no-todo].glob rule-predicate-changed\n\ + config-lint: 2 smell(s)\n" + ); + assert!( + !stdout(&output).contains("verb-removed"), + "adding a mediated verb row raises the bar: {}", + stdout(&output) + ); + assert!( + !stdout(&output).contains("waiver-expiry-extended"), + "pulling an expiry IN raises the bar: {}", + stdout(&output) + ); +} + // --- errors are usage errors, never verdicts --------------------------------- #[test] From cde9916c3dca739aa40e90f87934177c92bddaa2 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 19 Aug 2026 23:32:57 +0000 Subject: [PATCH 4/4] refactor(trust): split the comparison in two so each half stays readable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-721 took `weakenings` from six keys to the whole `Config` surface, and past a hundred lines a function is one a reader checks by sampling — which for the definition of "weakened" is the wrong reading habit to encourage. `entry_weakenings` holds the keys whose entries are a set, and `scalar_weakenings` the ones keyed on a threshold, a presence, or a table's contents — the half where direction is the subtle part. No behaviour change: the same comparisons in the same order, and the sort in `weakenings` still decides the output. Refs: CLOUD-721 --- crates/batten/src/trust.rs | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/crates/batten/src/trust.rs b/crates/batten/src/trust.rs index ccc9dfe21..f34782035 100644 --- a/crates/batten/src/trust.rs +++ b/crates/batten/src/trust.rs @@ -619,6 +619,20 @@ pub fn weakenings(base: &Config, working: &Config) -> Vec { // order. Which keys are compared at all is no longer a matter of what // occurred to an author: `CENSUS` records a verdict for every field and its // test fails on any field carrying none. + found.extend(entry_weakenings(base, working)); + found.extend(scalar_weakenings(base, working)); + + found.sort(); + found +} + +/// Keys whose entries are a set: an entry gone is a gate that stops firing. +/// +/// Split out of [`weakenings`] because the comparison is now over a +/// twenty-eight-key struct rather than six keys, and a function long enough to +/// scroll is one a reader checks by sampling. +fn entry_weakenings(base: &Config, working: &Config) -> Vec { + let mut found = Vec::new(); found.extend(min_version_weakening(base, working)); @@ -659,6 +673,16 @@ pub fn weakenings(base: &Config, working: &Config) -> Vec { "provision", )); + found +} + +/// Keys whose weakening is a threshold, a presence, or a table's own contents. +/// +/// The sibling of [`entry_weakenings`], and the half where direction is the +/// subtle part: a ceiling inverts, an absent `[judge]` is the tightest setting +/// there is, and a dropped table means different things to different gates. +fn scalar_weakenings(base: &Config, working: &Config) -> Vec { + let mut found = Vec::new(); // The escape hatch's second direction (CLOUD-721): the key pairs the two // files, and the expiry inside it is what the pairing was blind to. found.extend(waiver_expiry_weakenings(&base.waivers, &working.waivers)); @@ -741,7 +765,6 @@ pub fn weakenings(base: &Config, working: &Config) -> Vec { )); } - found.sort(); found }