From ac0e84955880034d20209f74914bf7252988e120 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 19 Aug 2026 14:45:50 +0000 Subject: [PATCH 1/3] feat(redirect)!: key the sanctioned mutation to the protected path, not the verb MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-96 put the redirect on the `[[verb]]` row because a per-class one was not expressible then, and recorded the gap as an amendment. CLOUD-122 made every deny carry a `Fix`, which turned that gap from a rough edge into the contract's weakest point: the useful remedy is a property of what is protected, not of the program reaching for it. `rm` against agent memory, against a committed workflow, and against generated output want three different answers, and one string on `rm` has to be vague enough for all three. The duplication is measured, not predicted. Once CLOUD-312 made the write tools verbs and CLOUD-442 added the qualifier columns, this repository's own table reached seventeen `redirect` strings of which TEN repeat the same per-path clause verbatim — one fact about `.serena/memories/**`, copy-pasted across every program that can reach it, because there was nowhere else to put it. `[[redirect]]` is a glob->mutation table consulted before the verb's own redirect, so the tiers are: the path class the consumer declared, then the verb's general remedy, then `Fix::None`. The last two are CLOUD-96's behaviour untouched, which makes the floor structural rather than careful — `Fix::declared(Option<&str>)` was built for exactly this seam. A sibling table rather than a wider `protected`: widening the element type would break `trust::removed_entries`, whose `protected[]` keys are how the raise-only comparison names a removed guard. `protected` keeps `Vec`. Declaration order decides, first match wins — the tie-break `shape_rules` already uses, for the reason stated there: a reviewer reads a table top to bottom, and any cleverer precedence is a rule about rules the config does not state. The lookup takes the normalised path the protected check was asked about, so the two tables cannot disagree about which path is under discussion. A redirect is not policy-bearing — it changes what a refusal says, never whether it fires — so no raise-only clamp applies, and a test asserts exactly that rather than leaving it to inference. BREAKING CHANGE: `Config` and `OverrideConfig` gain a `redirects` field. Both are public structs with public fields, so a downstream struct literal naming every field no longer compiles — `cargo-semver-checks` reports it as `constructible_struct_adds_field`. Declared rather than worked around: the alternative is `#[non_exhaustive]`, a larger and separate API decision than this table needs. Below 0.1.0 release-plz bumps the patch whatever the type says. Refs: CLOUD-280 --- crates/batten/src/config.rs | 23 ++++ crates/batten/src/hook.rs | 201 +++++++++++++++++++++++++++++-- crates/batten/src/lib.rs | 1 + crates/batten/src/redirect.rs | 214 ++++++++++++++++++++++++++++++++++ crates/batten/src/resolve.rs | 41 +++++++ 5 files changed, 470 insertions(+), 10 deletions(-) create mode 100644 crates/batten/src/redirect.rs diff --git a/crates/batten/src/config.rs b/crates/batten/src/config.rs index 5f2b7c7a4..8dde23974 100644 --- a/crates/batten/src/config.rs +++ b/crates/batten/src/config.rs @@ -162,6 +162,18 @@ pub struct Config { /// lookup are [`crate::verbs`]. #[serde(default, rename = "verb", skip_serializing_if = "Vec::is_empty")] pub verbs: Vec, + /// The per-path-class redirect table (CLOUD-280): what to run instead, + /// keyed by what is protected rather than by the verb reaching for it. + /// + /// Consulted before [`MutatingVerb::redirect`], which stays the fallback, so + /// the behaviour CLOUD-96 shipped is the floor rather than a regression. + /// Deliberately a sibling of [`Config::protected`] rather than a widening of + /// it: that set keeps its element type, so [`crate::trust`]'s + /// `protected[]` weakening keys are untouched. + /// + /// [`MutatingVerb::redirect`]: crate::verbs::MutatingVerb::redirect + #[serde(default, rename = "redirect", skip_serializing_if = "Vec::is_empty")] + pub redirects: Vec, /// Output predicates over a wrapped command's captured streams (CLOUD-117): /// literals that, found in `batten exec`'s output, promote a lying exit `0` /// to a violation. Consumer-specific by nature — which warning means @@ -420,6 +432,14 @@ pub struct OverrideConfig { skip_serializing_if = "Vec::is_empty" )] pub exec_patterns: Vec, + /// Redirects this file **adds**. A duplicate glob is refused. + /// + /// Needs no raise-only clamp, and that is a decision rather than an + /// oversight: a redirect changes what a refusal *says*, never whether it + /// fires, so there is no bar here to lower. Refusing a redefinition is + /// coherence with the other append-only tables. + #[serde(default, rename = "redirect", skip_serializing_if = "Vec::is_empty")] + pub redirects: Vec, /// Waivers this file adds, for rules the authority does not declare. A /// waiver over a committed rule lowers that bar and is refused. #[serde(default, rename = "waiver", skip_serializing_if = "Vec::is_empty")] @@ -497,6 +517,7 @@ fn parse_ungated(text: &str, source: &str) -> Result { // too: `batten.local.toml` may add verb rows, and a raise-only override that // adds an inert one has still written something that cannot mean anything. crate::verbs::validate(&config.verbs)?; + crate::redirect::validate(&config.redirects)?; // And the marker table, for the identical reason in the identical shape // (CLOUD-253). Both tables arrived in one commit; CLOUD-242 wired one of // them up and nobody checked the sibling, so an empty `token` — which @@ -650,6 +671,7 @@ impl Config { unlanded: Vec::new(), epoch: None, verbs: Vec::new(), + redirects: Vec::new(), markers: Vec::new(), exec: None, exec_patterns: Vec::new(), @@ -891,6 +913,7 @@ mod tests { /// [`parse_ungated`] that does it. Deleting a call fails the test below. const VALIDATED_AT_LOAD: &[(&str, &str)] = &[ ("verbs", "crate::verbs::validate("), + ("redirects", "crate::redirect::validate("), ("markers", "crate::markers::validate("), ("rules", "crate::rules::validate("), ("exec_patterns", "crate::outputs::validate("), diff --git a/crates/batten/src/hook.rs b/crates/batten/src/hook.rs index b9ed11bb9..09a37541b 100644 --- a/crates/batten/src/hook.rs +++ b/crates/batten/src/hook.rs @@ -45,6 +45,7 @@ use serde::Serialize; use serde_json::Value; use crate::receipt::Validity; +use crate::redirect::{self, Redirect}; use crate::refusal::{Fix, Refusal}; use crate::resolve::Resolved; use crate::rules::{PathSet, ReceiptKey, ReceiptTrigger, Rule, RuleKind, RuleScope}; @@ -1492,6 +1493,12 @@ pub struct Policy { /// the cross product as rules would need one row per verb × path pair, and /// the config would restate what an intersection already says. protected: PathSet, + /// What to run instead, per protected path class (CLOUD-280). + /// + /// Message composition only — it never decides whether the gate fires, which + /// is why it sits beside `protected` rather than inside it and why no + /// raise-only clamp applies to it. + redirects: Vec, } impl Policy { @@ -1507,6 +1514,7 @@ impl Policy { fail_on_warning: false, verbs: Vec::new(), protected: PathSet::empty(), + redirects: Vec::new(), } } @@ -1534,6 +1542,7 @@ impl Policy { fail_on_warning: resolved.fail_on_warning, verbs: resolved.verbs.clone(), protected: PathSet::includes("protected", &resolved.protected)?, + redirects: resolved.redirects.clone(), }) } @@ -1741,7 +1750,7 @@ fn adjudicated( && let Some(verb) = crate::verbs::classify(&policy.verbs, &envelope.tool) && policy.protected.contains(normalise(path)) { - return Decision::Deny(protected_refusal(&Target { + return Decision::Deny(protected_refusal(&policy.redirects, &Target { program: &envelope.tool, subcommand: None, path, @@ -2309,7 +2318,7 @@ fn protected_mutation(policy: &Policy, command: &str) -> Decision { if !policy.protected.contains(normalise(target.path)) { continue; } - return Decision::Deny(protected_refusal(&target)); + return Decision::Deny(protected_refusal(&policy.redirects, &target)); } } Decision::Allow @@ -2388,18 +2397,22 @@ fn normalise(path: &str) -> &str { /// typed, and naming it is the difference between an actionable refusal and a /// riddle. The file's contents never appear. /// -/// The fix is the verb's own declared `redirect`, and [`Fix::None`] where the -/// consumer declared none — stated rather than papered over with a catch-all -/// that pretends to be specific. That absence is the seam CLOUD-280 fills: the -/// useful redirect is a property of what is being protected, not of the verb -/// reaching for it, so a per-path-class table lands *here* and this fallback -/// becomes the third tier rather than the second. +/// The fix is three-tiered (CLOUD-280): the `[[redirect]]` row for this path +/// class, else the verb's own declared `redirect`, else [`Fix::None`] — stated +/// rather than papered over with a catch-all that pretends to be specific. The +/// useful remedy is a property of what is being protected, not of the verb +/// reaching for it, which is why the table is consulted first; the two fallbacks +/// are CLOUD-96's behaviour unchanged, so the floor cannot regress. +/// +/// It makes a refusal SPECIFIC; it does not make the named surface reachable. +/// CLOUD-663 was canceled on exactly that distinction — a redirect pointing at a +/// surface that is down is a defect in the surface. /// /// A subcommand-qualified row names the whole action, not just the front-end: /// ` ` is what the caller typed and what a reader has to /// recognise, and a refusal naming only the front-end would read as a ban on /// every use of it (CLOUD-442). -fn protected_refusal(target: &Target<'_>) -> Refusal { +fn protected_refusal(redirects: &[Redirect], target: &Target<'_>) -> Refusal { let action = match target.subcommand { Some(subcommand) => format!("{} {subcommand}", target.program), None => target.program.to_owned(), @@ -2407,7 +2420,21 @@ fn protected_refusal(target: &Target<'_>) -> Refusal { Refusal::new( PROTECTED_MUTATION, format!("`{action}` targets the protected path {}", target.path), - Fix::declared(target.verb.redirect.as_deref()), + // Three tiers, narrowest first (CLOUD-280): the path class the consumer + // declared, then the verb's own general remedy, then `Fix::None` — which + // renders an explicit "none declared" and names the gate. The two + // fallbacks are exactly CLOUD-96's behaviour, so this can only ever make + // a refusal more specific, never less. + // + // The lookup takes `normalise`d path, the same value + // `policy.protected.contains` was asked about, so the two tables cannot + // disagree about WHICH path is under discussion. The message keeps the + // path as the caller typed it, because that is the pointer they can act + // on. + Fix::declared( + redirect::resolve(redirects, normalise(target.path)) + .or(target.verb.redirect.as_deref()), + ), ) } @@ -2991,6 +3018,11 @@ mod tests { /// one protected glob. Both tables are the consumer's, so a test supplies /// them exactly as a `batten.toml` would. fn protected_policy(verbs: Vec) -> Policy { + protected_policy_with(verbs, Vec::new()) + } + + /// The same fixture with a declared `[[redirect]]` table (CLOUD-280). + fn protected_policy_with(verbs: Vec, redirects: Vec) -> Policy { Policy { shapes: Vec::new(), fail_on_warning: false, @@ -3000,6 +3032,7 @@ mod tests { &[".serena/memories/**".to_owned(), "batten.toml".to_owned()], ) .expect("the fixture protected set is well formed"), + redirects, } } @@ -3022,6 +3055,7 @@ mod tests { Policy { verbs: Vec::new(), protected: PathSet::empty(), + redirects: Vec::new(), shapes: vec![ shape("gh-pr-merge", "gh pr merge", None), shape( @@ -3543,6 +3577,7 @@ mod tests { fail_on_warning: false, verbs: Vec::new(), protected: PathSet::empty(), + redirects: Vec::new(), }; assert_eq!( adjudicate( @@ -3568,6 +3603,7 @@ mod tests { fail_on_warning: false, verbs: Vec::new(), protected: PathSet::empty(), + redirects: Vec::new(), }; assert_eq!( adjudicate( @@ -3587,6 +3623,7 @@ mod tests { fail_on_warning: true, verbs: Vec::new(), protected: PathSet::empty(), + redirects: Vec::new(), }; assert!( matches!( @@ -3617,6 +3654,7 @@ mod tests { fail_on_warning: false, verbs: Vec::new(), protected: PathSet::empty(), + redirects: Vec::new(), }; let reason = denial_text(adjudicate( &policy, @@ -3653,6 +3691,7 @@ mod tests { fail_on_warning: false, verbs: Vec::new(), protected: PathSet::empty(), + redirects: Vec::new(), }; let reason = denial_text(adjudicate( &policy, @@ -3707,6 +3746,7 @@ mod tests { fail_on_warning: false, verbs: Vec::new(), protected: PathSet::empty(), + redirects: Vec::new(), } } @@ -3746,6 +3786,7 @@ mod tests { fail_on_warning: false, verbs: Vec::new(), protected: PathSet::empty(), + redirects: Vec::new(), } } @@ -4373,6 +4414,143 @@ mod tests { assert!(reason.contains("surface that owns it"), "got: {reason}"); } + /// Adjudicate against the protected fixture with a declared redirect table. + fn guarded_with(redirects: Vec, command: &str) -> Decision { + adjudicate( + &protected_policy_with( + vec![ + verb("rm", Some("restore it with git")), + verb("mv", None), + ], + redirects, + ), + &envelope(command), + false, + &None, + &None, + &crate::stop::StopFacts::default(), + ) + } + + fn redirect_row(glob: &str, mutation: &str) -> Redirect { + Redirect { + glob: glob.to_owned(), + mutation: mutation.to_owned(), + } + } + + #[test] + fn the_path_class_redirect_outranks_the_verbs_own() { + // Tier one (CLOUD-280). `rm` declares "restore it with git", which is + // true of most paths and useless for this one: agent memory has a write + // surface, and that is the fact the PATH knows and the verb cannot. + let refusal = denial(guarded_with( + vec![redirect_row( + ".serena/memories/**", + "write it through the memory surface that owns it", + )], + "rm .serena/memories/core.md", + )); + assert_eq!( + refusal.fix().declared_alternative(), + Some("write it through the memory surface that owns it"), + "the class the config declared beats the verb's general remedy" + ); + } + + #[test] + fn a_class_no_row_claims_falls_back_to_the_verbs_redirect() { + // Tier two, and the floor this issue promised not to regress: with a + // table declared but silent about this path, the answer is exactly what + // CLOUD-96 shipped. + let refusal = denial(guarded_with( + vec![redirect_row(".serena/memories/**", "use the memory surface")], + "rm batten.toml", + )); + assert_eq!( + refusal.fix().declared_alternative(), + Some("restore it with git"), + "an unclaimed class leaves the verb's own redirect standing" + ); + } + + #[test] + fn neither_tier_declaring_anything_still_names_the_gate() { + // Tier three, also unchanged: `mv` declares no redirect and no row + // claims the path, so the absence is stated as a value. + let decision = guarded_with( + vec![redirect_row("somewhere/else/**", "irrelevant")], + "mv batten.toml elsewhere", + ); + let refusal = denial(decision.clone()); + assert_eq!(refusal.fix(), &Fix::None); + assert!(denial_text(decision).contains(PROTECTED_MUTATION)); + } + + #[test] + fn the_redirect_lookup_sees_the_same_path_the_protected_check_did() { + // Both tables must discuss ONE path. `./x` and `x` are the same file, and + // `protected.contains` is asked about the normalised form — so a lookup + // on the raw operand would guard the path and then fail to find the class + // that speaks for it, producing a deny whose fix silently fell back a + // tier for a spelling. + let refusal = denial(guarded_with( + vec![redirect_row("batten.toml", "change it in a reviewed PR")], + "rm ./batten.toml", + )); + assert_eq!( + refusal.fix().declared_alternative(), + Some("change it in a reviewed PR") + ); + // And the message still points at what the caller actually typed. + assert!( + refusal.reason().contains("./batten.toml"), + "got: {}", + refusal.reason() + ); + } + + #[test] + fn the_declared_order_decides_which_class_answers() { + // The tie-break, at the surface that consumes it rather than only in the + // table's own unit tests: two rows match, and the one declared first + // wins, because the config author orders the table. + let refusal = denial(guarded_with( + vec![ + redirect_row(".serena/memories/**", "the narrow answer"), + redirect_row("**", "the catch-all"), + ], + "rm .serena/memories/core.md", + )); + assert_eq!( + refusal.fix().declared_alternative(), + Some("the narrow answer") + ); + } + + #[test] + fn a_redirect_changes_the_message_and_never_the_verdict() { + // The claim that exempts this table from the raise-only clamp, asserted + // rather than assumed: the same command against the same policy is a + // deny with the table and a deny without it, and an ALLOW stays an allow. + // If a redirect could ever flip a verdict it would be policy-bearing and + // would need a clamp (the issue's stated assumption 1). + let rows = vec![redirect_row("**", "some remedy")]; + assert!(matches!( + guarded_with(rows.clone(), "rm batten.toml"), + Decision::Deny(_) + )); + assert!(matches!( + guarded_with(Vec::new(), "rm batten.toml"), + Decision::Deny(_) + )); + assert_eq!( + guarded_with(rows, "rm target/debug/scratch"), + Decision::Allow, + "a redirect matching every path cannot make an unprotected one deny" + ); + } + #[test] fn flags_are_never_treated_as_paths() { // And `--` ends option parsing, so a dash-leading operand after it is @@ -4425,6 +4603,7 @@ mod tests { fail_on_warning: false, verbs: vec![verb("rm", None)], protected: PathSet::empty(), + redirects: Vec::new(), }; assert_eq!( adjudicate( @@ -4503,6 +4682,7 @@ mod tests { verbs: verbs.clone(), protected: PathSet::includes("protected", &["guarded/**".to_owned()]) .expect("well formed"), + redirects: Vec::new(), }; let elsewhere = Policy { shapes: Vec::new(), @@ -4510,6 +4690,7 @@ mod tests { verbs, protected: PathSet::includes("protected", &["other/**".to_owned()]) .expect("well formed"), + redirects: Vec::new(), }; let call = envelope("rm guarded/thing"); assert!( diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index dfcd17dcc..9f79ab87d 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -44,6 +44,7 @@ pub mod output; pub mod outputs; pub mod provision; pub mod receipt; +pub mod redirect; pub mod refusal; pub mod render; pub mod resolve; diff --git a/crates/batten/src/redirect.rs b/crates/batten/src/redirect.rs new file mode 100644 index 000000000..41dbcabed --- /dev/null +++ b/crates/batten/src/redirect.rs @@ -0,0 +1,214 @@ +//! The per-path-class redirect table (CLOUD-280): what to run instead, keyed by +//! **what is protected** rather than by the verb reaching for it. +//! +//! CLOUD-96 shipped the sanctioned mutation on the `[[verb]]` row, because a +//! per-class one was not expressible then. That gets the refusal contract +//! (CLOUD-122) mostly right — `rm` has one obvious alternative most of the time +//! — but the *useful* remedy is a property of the target: `rm` against agent +//! memory should name the memory-write surface, `rm` against a committed +//! workflow should say to delete it in a PR, `rm` against generated output +//! should say to re-run the generator. One string on the verb has to be vague +//! enough to cover all three, and a vague fix is the un-actionable refusal the +//! contract exists to prevent. +//! +//! The duplication that forced this was measured rather than predicted. Once +//! CLOUD-312 made the write *tools* verbs and CLOUD-442 added the qualifier +//! columns, this repository's own table reached seventeen `redirect` strings of +//! which **ten** carried the same per-path clause verbatim — one fact about +//! `.serena/memories/**`, copy-pasted across every program that can reach it, +//! because there was nowhere else to put it. +//! +//! Load-bearing choices: +//! +//! * **A sibling table, not a wider `protected`.** The direct approach — +//! widening `protected` to `{glob, mutation}` — breaks +//! [`crate::trust::weakenings`], whose `protected[]` keys are how the +//! raise-only comparison names a removed guard. Those keys stay byte-identical +//! because `protected` keeps its element type; this table only says what to +//! *suggest*, so the two can never disagree about coverage. +//! * **Declaration order decides, first match wins.** The same tie-break +//! [`crate::hook`]'s shape rows use, and for the reason stated there: a +//! reviewer reads a table top to bottom, and any cleverer precedence is a rule +//! about rules that the config does not state. "Most specific glob" has no +//! cheap definition and would be one. +//! * **A redirect is not policy-bearing.** It changes what a refusal *says*, +//! never whether the refusal fires, so §8's raise-only clamp does not apply to +//! it — stated here rather than left to inference. What the local layer still +//! may not do is *redefine* a committed row, which is coherence with every +//! other table rather than a weakening this one could express. +//! * **It makes a message specific; it does not make a surface reachable.** +//! CLOUD-663 was canceled on exactly this distinction: a redirect naming a +//! surface that is down is a defect in the surface, not in the redirect, and +//! a second sanctioned writer would have hidden it permanently. +//! +//! The glob matcher is [`crate::rules::glob_match`] — one glob semantics for the +//! whole engine, never a second implementation. + +use anyhow::Result; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use crate::error::UsageError; +use crate::rules::glob_match; + +/// One declared path class and the mutation sanctioned for it. +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct Redirect { + /// The path class this redirect speaks for, in the engine's one glob + /// dialect. + /// + /// Deliberately independent of `protected`: this table answers "what should + /// they run instead", never "is this guarded". A glob here matching a path + /// no `protected` entry covers simply never comes up, which is the harmless + /// direction — the alternative, deriving protection from a redirect, is the + /// set-collapsing CLOUD-37 exists to prevent. + pub glob: String, + /// The sanctioned mutation for that class — the "run this instead" a deny + /// carries. + pub mutation: String, +} + +/// Reject a table that would make a refusal dishonest or silent. +/// +/// # Errors +/// +/// Returns a [`UsageError`] (→ exit `1`) for an empty `glob`, an empty +/// `mutation`, or a duplicated `glob`. The last is the load-bearing one: two +/// rows for one path class are two answers to one question, and silently taking +/// the first is how a corrected remedy gets lost behind a stale row — the same +/// refusal [`crate::verbs::validate`] gives a verb declared twice. +/// +/// An empty `mutation` is refused rather than treated as absent, because it +/// would render a fix clause that is present and says nothing, which reads worse +/// than the explicit "none declared" a row's absence already produces. +pub fn validate(table: &[Redirect]) -> Result<()> { + for (index, entry) in table.iter().enumerate() { + if entry.glob.trim().is_empty() { + return Err(UsageError::raise( + "redirect: `glob` must not be empty".to_owned(), + )); + } + if entry.mutation.trim().is_empty() { + return Err(UsageError::raise(format!( + "redirect {}: `mutation` must not be empty — a redirect that names nothing is a \ + fix clause that says nothing", + entry.glob + ))); + } + if table[..index].iter().any(|prior| prior.glob == entry.glob) { + return Err(UsageError::raise(format!( + "redirect {}: declared twice; a path class has one sanctioned mutation", + entry.glob + ))); + } + } + Ok(()) +} + +/// The mutation declared for `path`, if any row claims it. +/// +/// First match in declaration order — see the module doc for why that is the +/// tie-break and not "most specific". `None` means no row speaks for this path, +/// which leaves the caller to fall back to the verb's own redirect. +#[must_use] +pub fn resolve<'table>(table: &'table [Redirect], path: &str) -> Option<&'table str> { + table + .iter() + .find(|entry| glob_match(&entry.glob, path)) + .map(|entry| entry.mutation.as_str()) +} + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used)] +mod tests { + use super::*; + + fn row(glob: &str, mutation: &str) -> Redirect { + Redirect { + glob: glob.to_owned(), + mutation: mutation.to_owned(), + } + } + + #[test] + fn a_declared_class_resolves_to_its_mutation() { + let table = [row("guarded/**", "use the surface that owns it")]; + assert_eq!( + resolve(&table, "guarded/thing.md"), + Some("use the surface that owns it") + ); + } + + #[test] + fn a_path_no_row_claims_resolves_to_nothing() { + // Not an error and not a default: "no row speaks for this path" is what + // lets the caller fall back to the verb's own redirect, which is the + // tier CLOUD-96 shipped and this table sits in front of. + let table = [row("guarded/**", "use the surface that owns it")]; + assert_eq!(resolve(&table, "elsewhere/thing.md"), None); + assert_eq!(resolve(&[], "guarded/thing.md"), None); + } + + #[test] + fn the_first_matching_row_wins_in_declaration_order() { + // The tie-break, as a test rather than a comment. Both globs match, and + // the narrower one is declared FIRST — which is the whole point: the + // config author orders the table, the engine does not rank it. + let table = [ + row("guarded/secrets/**", "rotate it, do not edit it"), + row("guarded/**", "use the surface that owns it"), + ]; + assert_eq!( + resolve(&table, "guarded/secrets/key.md"), + Some("rotate it, do not edit it") + ); + // And the later row still answers for everything the earlier one misses, + // so ordering narrow-first is a usable discipline rather than a trap. + assert_eq!( + resolve(&table, "guarded/notes.md"), + Some("use the surface that owns it") + ); + } + + #[test] + fn a_class_declared_twice_is_a_usage_error() { + // Two answers to one question. Taking the first silently is how a + // corrected remedy gets lost behind a stale row. + let table = [row("guarded/**", "first"), row("guarded/**", "second")]; + let err = validate(&table).unwrap_err(); + assert!(err.downcast_ref::().is_some()); + } + + #[test] + fn an_empty_glob_or_mutation_is_a_usage_error() { + assert!(validate(&[row("", "something")]).is_err()); + assert!(validate(&[row("guarded/**", "")]).is_err()); + // Whitespace is not a declaration either — it would render `Fix: .` + assert!(validate(&[row("guarded/**", " ")]).is_err()); + assert!(validate(&[row("guarded/**", "something")]).is_ok()); + } + + #[test] + fn two_distinct_classes_coexist() { + let table = [row("a/**", "first"), row("b/**", "second")]; + assert!(validate(&table).is_ok()); + assert_eq!(resolve(&table, "b/x"), Some("second")); + } + + #[test] + fn the_source_bakes_in_no_path_class() { + // Non-negotiable rule 1, in the `verbs::the_source_bakes_in_no_verb` + // idiom: which paths a repository protects, and what it wants run + // instead, are the consumer's policy. Asserted behaviourally — the same + // path must get opposite answers from two tables differing only in their + // rows, which a hardcoded class could not produce. + let declaring = [row("guarded/**", "the declared remedy")]; + let elsewhere = [row("other/**", "the declared remedy")]; + assert_eq!( + resolve(&declaring, "guarded/thing"), + Some("the declared remedy") + ); + assert_eq!(resolve(&elsewhere, "guarded/thing"), None); + } +} diff --git a/crates/batten/src/resolve.rs b/crates/batten/src/resolve.rs index 3bae8585b..88737683c 100644 --- a/crates/batten/src/resolve.rs +++ b/crates/batten/src/resolve.rs @@ -220,6 +220,12 @@ pub struct Resolved { /// The mutating-verb table, consumer data the authority supplies. #[serde(rename = "verb")] pub verbs: Vec, + /// The per-path-class redirect table (CLOUD-280), authority rows plus any a + /// local file **added**. Local rows append after committed ones, and the + /// lookup takes the first match, so an uncommitted file can add a class the + /// authority never named and can never change what a committed row says. + #[serde(rename = "redirect")] + pub redirects: Vec, /// The suppression-marker table, consumer data the authority supplies. #[serde(rename = "marker")] pub markers: Vec, @@ -579,6 +585,7 @@ pub fn resolve_with_env( rules_source: declared_by(present, !repo.rules.is_empty()), rules: repo.rules.clone(), exec_patterns: repo.exec_patterns.clone(), + redirects: repo.redirects.clone(), waivers: repo.waivers.clone(), }; @@ -675,6 +682,7 @@ struct Tables { rules: Vec, rules_source: Source, exec_patterns: Vec, + redirects: Vec, waivers: Vec, } @@ -742,6 +750,7 @@ fn apply_local( tables.rules_source = Source::LocalFile; } merge_local_patterns(&mut tables.exec_patterns, local.exec_patterns)?; + merge_local_redirects(&mut tables.redirects, local.redirects)?; merge_local_waivers(&mut tables.waivers, local.waivers, &repo.rules)?; // §8's three policy-bearing path sets, raise-only. Before CLOUD-239 these // were parsed and discarded: an author who wrote `protected` here got no @@ -865,6 +874,36 @@ fn merge_local_patterns( Ok(()) } +/// Add a local file's redirects to the committed ones, refusing a redefinition. +/// +/// The same append-only shape [`merge_local_patterns`] uses, keyed on `glob`, +/// and for coherence rather than for safety: a redirect is **not +/// policy-bearing** — it changes what a refusal says, never whether it fires — +/// so §8's raise-only clamp has no bar here to protect. What refusing a +/// redefinition buys is that a committed remedy cannot be quietly reworded by an +/// uncommitted file, which is a claim about provenance, not about strictness. +/// +/// Appending is what makes that hold: [`crate::redirect::resolve`] takes the +/// **first** matching row, so a local row can only ever answer for a class the +/// authority left unclaimed. +fn merge_local_redirects( + committed: &mut Vec, + local: Vec, +) -> Result<()> { + for entry in local { + if committed.iter().any(|row| row.glob == entry.glob) { + return Err(UsageError::raise(format!( + "redirect {}: {LOCAL_CONFIG_FILE} may not redefine a redirect from {}; an \ + override may only add path classes the authority does not claim (§8)", + entry.glob, + config::CONFIG_FILE, + ))); + } + committed.push(entry); + } + Ok(()) +} + /// Add a local file's waivers to the committed ones, refusing any that touch a /// committed rule. /// @@ -940,6 +979,7 @@ fn assemble( unlanded: paths.unlanded, epoch: repo.epoch.clone(), verbs: repo.verbs.clone(), + redirects: tables.redirects, markers: repo.markers.clone(), exec: repo.exec, exec_patterns: tables.exec_patterns, @@ -1001,6 +1041,7 @@ fn attribution( authority_set(!repo.exec_patterns.is_empty()), ), ("exec", authority_set(repo.exec.is_some())), + ("redirect", authority_set(!repo.redirects.is_empty())), ("waiver", authority_set(!repo.waivers.is_empty())), ("budget", authority_set(repo.budget.is_some())), ("must_land_on", authority_set(repo.must_land_on.is_some())), From aa9a1861870f500016baaf7c9a5f81578d3b1c30 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 19 Aug 2026 14:45:54 +0000 Subject: [PATCH 2/3] feat(redirect): declare the two path classes whose remedy the verb cannot know MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consumer #1 adopts the table, and the adoption is narrower than the issue assumed — which is the interesting part. `.github/workflows/**` and `batten.toml` get rows: for both, the useful remedy is a property of the path and the verb's is actively misleading. `rm` says "restore it with `git checkout --`", but a workflow file is CI's definition of green, so the answer is that the change goes through review, not that the bytes come back. Measured before and after on the live binary: before Fix: restore it with `git checkout --`, or change it through the surface that owns it after Fix: change it in a pull request — these files are CI's definition of green, so the change has to be reviewed rather than restored `.serena/memories/**` is DELIBERATELY not declared, and finding out why changed the shape of this commit. The ten duplicated redirect tails that motivated the issue are not in fact identical: four name different Serena tools, because the right one depends on the action — `write_memory` for a write, `edit_memory` for an in-place edit, `rename_memory` for a move (the only route that rewrites `mem:` referrers), `delete_memory` for a delete. A path-class row would override all four with one weaker sentence, which is the opposite of what this table is for. So the tails stay, and the fallback tier is the correct answer there rather than a leftover. That is the boundary this feature has, stated in the config where the next author meets it: per-path beats per-verb only where the path fact dominates. Two censuses caught the new table and both are now satisfied — the override-key list in `config_schema.rs` and `hk.pkl`'s `schema-check` glob, without which a commit touching only this module would move a published schema without firing the drift gate. Schemas regenerated. Refs: CLOUD-280 --- batten.toml | 30 ++++++++++++++ crates/batten/src/hook.rs | 25 +++++++----- crates/batten/src/resolve.rs | 60 ++++++++++++++++++++++++++++ crates/batten/src/trust.rs | 36 +++++++++++++++++ crates/batten/tests/cli.rs | 55 +++++++++++++++++++++++++ crates/batten/tests/config_schema.rs | 1 + hk.pkl | 1 + schema/batten.local.schema.json | 26 ++++++++++++ schema/batten.schema.json | 26 ++++++++++++ 9 files changed, 249 insertions(+), 11 deletions(-) diff --git a/batten.toml b/batten.toml index c3892f156..f39384130 100644 --- a/batten.toml +++ b/batten.toml @@ -94,6 +94,36 @@ protected = [ ".github/workflows/**", ] +# --------------------------------------------------------------------------- +# What to run instead, per protected path class (CLOUD-280). +# +# The refusal contract (CLOUD-122) is that every deny names the fix. CLOUD-96 +# put that text on the `[[verb]]` row, which is right when the VERB knows the +# remedy and wrong when the PATH does: `rm` says "restore it with `git checkout +# --`", which is true of most files and actively misleading for a workflow, +# where the point is not recovering the bytes but that CI's definition of green +# changes under review. +# +# Consulted BEFORE the verb's own `redirect`; a class named here answers for +# every verb that reaches it. Declaration order decides, first match wins. +# +# WHY `.serena/memories/**` IS DELIBERATELY ABSENT, which is the interesting +# boundary of this table rather than an omission. Its remedy is not a property +# of the path alone: a write wants `write_memory`, an in-place edit wants +# `edit_memory`, a move wants `rename_memory` (the only route that rewrites +# `mem:` referrers), a delete wants `delete_memory`. The verb rows below already +# name the right tool per action, and a row here would OVERRIDE all four with +# one weaker sentence — the opposite of what this table is for. Per-path beats +# per-verb only where the path fact dominates; where the verb fact does, the +# fallback tier is the correct answer and not a leftover. +[[redirect]] +glob = ".github/workflows/**" +mutation = "change it in a pull request — these files are CI's definition of green, so the change has to be reviewed rather than restored" + +[[redirect]] +glob = "batten.toml" +mutation = "change it in a pull request — this file is the policy authority every gate reads, and `mise run config-lint` is what checks the edit before it lands" + # --------------------------------------------------------------------------- # The worktree pileup threshold (CLOUD-46). # diff --git a/crates/batten/src/hook.rs b/crates/batten/src/hook.rs index 09a37541b..0bf036277 100644 --- a/crates/batten/src/hook.rs +++ b/crates/batten/src/hook.rs @@ -1750,12 +1750,15 @@ fn adjudicated( && let Some(verb) = crate::verbs::classify(&policy.verbs, &envelope.tool) && policy.protected.contains(normalise(path)) { - return Decision::Deny(protected_refusal(&policy.redirects, &Target { - program: &envelope.tool, - subcommand: None, - path, - verb, - })); + return Decision::Deny(protected_refusal( + &policy.redirects, + &Target { + program: &envelope.tool, + subcommand: None, + path, + verb, + }, + )); } // The write-triggered receipt gate (CLOUD-444), reached whether or not this // call also carries a command — a write tool carries none, and the early @@ -4418,10 +4421,7 @@ mod tests { fn guarded_with(redirects: Vec, command: &str) -> Decision { adjudicate( &protected_policy_with( - vec![ - verb("rm", Some("restore it with git")), - verb("mv", None), - ], + vec![verb("rm", Some("restore it with git")), verb("mv", None)], redirects, ), &envelope(command), @@ -4464,7 +4464,10 @@ mod tests { // table declared but silent about this path, the answer is exactly what // CLOUD-96 shipped. let refusal = denial(guarded_with( - vec![redirect_row(".serena/memories/**", "use the memory surface")], + vec![redirect_row( + ".serena/memories/**", + "use the memory surface", + )], "rm batten.toml", )); assert_eq!( diff --git a/crates/batten/src/resolve.rs b/crates/batten/src/resolve.rs index 88737683c..eb9b65916 100644 --- a/crates/batten/src/resolve.rs +++ b/crates/batten/src/resolve.rs @@ -1088,6 +1088,66 @@ mod tests { None } + /// A committed authority declaring one redirect class. + const REDIRECT_AUTHORITY: &str = "version = 1\nprotected = [\"guarded/**\"]\n\n[[redirect]]\nglob = \"guarded/**\"\nmutation = \"use the surface that owns it\"\n"; + + #[test] + fn a_local_file_may_add_a_redirect_class_the_authority_does_not_claim() { + // The permitted direction, and the reason it needs no clamp: a redirect + // changes what a refusal SAYS, never whether it fires, so an added class + // lowers no bar. A session gating a scratch tree can name its own remedy + // without touching committed policy. + let dir = repo( + "redirect-local-add", + REDIRECT_AUTHORITY, + Some( + "version = 1\n\n[[redirect]]\nglob = \"vendor/**\"\nmutation = \"re-run the generator\"\n", + ), + ); + let resolved = resolve_with_env(&dir, &Overrides::default(), &no_env).unwrap(); + assert_eq!(resolved.redirects.len(), 2); + // Appended AFTER the committed rows, which is what makes first-match-wins + // safe: a local row can only ever answer for a class the authority left + // unclaimed. + assert_eq!(resolved.redirects[0].glob, "guarded/**"); + assert_eq!(resolved.redirects[1].glob, "vendor/**"); + assert_eq!( + crate::redirect::resolve(&resolved.redirects, "guarded/thing"), + Some("use the surface that owns it"), + "the committed remedy still answers for its own class" + ); + } + + #[test] + fn a_local_file_may_not_redefine_a_committed_redirect() { + // Not a strictness clamp — there is no bar here to lower — but a + // provenance one: a committed remedy must not be quietly reworded by an + // uncommitted file, the same refusal every other append-only table gives. + let dir = repo( + "redirect-local-redefine", + REDIRECT_AUTHORITY, + Some( + "version = 1\n\n[[redirect]]\nglob = \"guarded/**\"\nmutation = \"do whatever\"\n", + ), + ); + let err = resolve_with_env(&dir, &Overrides::default(), &no_env).unwrap_err(); + assert!(is_usage_error(&err), "got: {err}"); + assert!( + err.to_string().contains("guarded/**"), + "the refusal names the class: {err}" + ); + } + + #[test] + fn an_authority_declaring_no_redirect_resolves_an_empty_table() { + // Absent is not empty-and-wrong: a repository that names no path class + // simply falls through to the verb's own redirect, which is CLOUD-96's + // behaviour and the floor this table sits on top of. + let dir = repo("redirect-absent", "version = 1\n", None); + let resolved = resolve_with_env(&dir, &Overrides::default(), &no_env).unwrap(); + assert!(resolved.redirects.is_empty()); + } + fn is_usage_error(err: &anyhow::Error) -> bool { err.downcast_ref::().is_some() } diff --git a/crates/batten/src/trust.rs b/crates/batten/src/trust.rs index 3b4a35e82..14414f449 100644 --- a/crates/batten/src/trust.rs +++ b/crates/batten/src/trust.rs @@ -394,6 +394,42 @@ mod tests { ); } + #[test] + fn the_protected_weakening_key_survives_the_redirect_table() { + // CLOUD-280's load-bearing non-change. The obvious way to give a + // protected path its own redirect is to widen `protected` to a table of + // `{glob, mutation}` — and that breaks THIS key, because `removed_entries` + // renders `format!("{key}[{entry}]")` over a list of strings. A consumer + // reading `protected[b]` out of a trust report, and every gate keyed on + // that spelling, would start seeing something else. + // + // So the redirect landed as a sibling table and `protected` kept its + // element type. Asserted byte-for-byte, and with a redirect declared, so + // the assertion is about the shape that shipped rather than about a + // config the feature does not exist in. + let base = parse( + "version = 1\nprotected = [\"a\", \"b\"]\n\n[[redirect]]\nglob = \"b\"\nmutation = \"use the surface that owns it\"\n", + ); + let working = parse( + "version = 1\nprotected = [\"a\"]\n\n[[redirect]]\nglob = \"b\"\nmutation = \"use the surface that owns it\"\n", + ); + let found = weakenings(&base, &working); + assert_eq!(found.len(), 1, "one removed path, one weakening: {found:?}"); + assert_eq!( + found[0].key, "protected[b]", + "the key format is the signature this design preserves" + ); + // And the redirect table itself contributes no weakening in either + // direction: it is not policy-bearing, so adding or removing a row + // cannot lower a bar. + let with_row = parse( + "version = 1\nprotected = [\"a\"]\n\n[[redirect]]\nglob = \"a\"\nmutation = \"x\"\n", + ); + let without = parse("version = 1\nprotected = [\"a\"]\n"); + assert!(weakenings(&with_row, &without).is_empty()); + assert!(weakenings(&without, &with_row).is_empty()); + } + #[test] fn adding_a_protected_path_is_not_a_weakening() { let base = parse("version = 1\nprotected = [\"a\"]\n"); diff --git a/crates/batten/tests/cli.rs b/crates/batten/tests/cli.rs index 3b2f931c6..0c1a28a70 100644 --- a/crates/batten/tests/cli.rs +++ b/crates/batten/tests/cli.rs @@ -1349,6 +1349,61 @@ fn a_deny_with_no_safe_remedy_declares_it_rather_than_omitting_the_clause() { ); } +/// A gate whose protected class carries its own redirect, and a verb whose +/// general remedy is the wrong answer for it (CLOUD-280). +const PER_CLASS_REDIRECT_CONFIG: &str = r#"version = 1 +protected = ["guarded/**", "vendor/**"] + +[[redirect]] +glob = "guarded/**" +mutation = "change it in a pull request" + +[[verb]] +verb = "rm" +effect = "destructive" +redirect = "restore it with git" +"#; + +#[test] +fn a_deny_names_the_path_classs_own_mutation_over_the_verbs() { + // The three tiers over the compiled binary, because a refusal is a contract + // only where a host reads it. Same command, same verb, two paths: the class + // that declares a remedy gets it, and the class that does not falls back to + // the verb's — which is CLOUD-96's behaviour, asserted here so the floor is + // proven rather than assumed. + let dir = repo_with_config("refusal-per-class", PER_CLASS_REDIRECT_CONFIG); + + let claimed = run_hook_in( + &dir, + "exit-code", + &claude_payload("rm guarded/thing.md"), + false, + ); + assert_eq!(claimed.status.code(), Some(2), "the protected gate denies"); + let stderr = String::from_utf8_lossy(&claimed.stderr); + assert!( + stderr.contains("Fix: change it in a pull request"), + "the declared class answers, got: {stderr}" + ); + assert!( + !stderr.contains("restore it with git"), + "the verb's general remedy must not also appear, got: {stderr}" + ); + + let unclaimed = run_hook_in( + &dir, + "exit-code", + &claude_payload("rm vendor/thing.md"), + false, + ); + assert_eq!(unclaimed.status.code(), Some(2), "still denied"); + let stderr = String::from_utf8_lossy(&unclaimed.stderr); + assert!( + stderr.contains("Fix: restore it with git"), + "an unclaimed class leaves the verb's redirect standing, got: {stderr}" + ); +} + // --- the normalized event census (CLOUD-43) --------------------------------- // // The envelope carried an `event` field from CLOUD-202 and never dispatched on diff --git a/crates/batten/tests/config_schema.rs b/crates/batten/tests/config_schema.rs index a3649938a..36e828d70 100644 --- a/crates/batten/tests/config_schema.rs +++ b/crates/batten/tests/config_schema.rs @@ -427,6 +427,7 @@ fn the_override_schema_describes_only_keys_the_loader_honours() { "fail_on_warning", "min_batten_version", "protected", + "redirect", "rule", "scope", "strictness", diff --git a/hk.pkl b/hk.pkl index ebd620237..a50eb76fe 100644 --- a/hk.pkl +++ b/hk.pkl @@ -336,6 +336,7 @@ local gate = new Mapping { "crates/batten/src/markers.rs", "crates/batten/src/outputs.rs", "crates/batten/src/provision.rs", + "crates/batten/src/redirect.rs", "crates/batten/src/rules.rs", "crates/batten/src/severity.rs", "crates/batten/src/transcript.rs", diff --git a/schema/batten.local.schema.json b/schema/batten.local.schema.json index 41232e0cf..826a1f40a 100644 --- a/schema/batten.local.schema.json +++ b/schema/batten.local.schema.json @@ -32,6 +32,13 @@ "type": "string" } }, + "redirect": { + "description": "Redirects this file **adds**. A duplicate glob is refused.\n\nNeeds no raise-only clamp, and that is a decision rather than an\noversight: a redirect changes what a refusal *says*, never whether it\nfires, so there is no bar here to lower. Refusing a redefinition is\ncoherence with the other append-only tables.", + "type": "array", + "items": { + "$ref": "#/$defs/Redirect" + } + }, "rule": { "description": "Rules this file **adds**. Redefining a committed id is refused.", "type": "array", @@ -177,6 +184,25 @@ } ] }, + "Redirect": { + "description": "One declared path class and the mutation sanctioned for it.", + "type": "object", + "properties": { + "glob": { + "description": "The path class this redirect speaks for, in the engine's one glob\ndialect.\n\nDeliberately independent of `protected`: this table answers \"what should\nthey run instead\", never \"is this guarded\". A glob here matching a path\nno `protected` entry covers simply never comes up, which is the harmless\ndirection — the alternative, deriving protection from a redirect, is the\nset-collapsing CLOUD-37 exists to prevent.", + "type": "string" + }, + "mutation": { + "description": "The sanctioned mutation for that class — the \"run this instead\" a deny\ncarries.", + "type": "string" + } + }, + "additionalProperties": false, + "required": [ + "glob", + "mutation" + ] + }, "Rule": { "description": "One declarative rule from `batten.toml`'s `[[rule]]` array.\n\n`deny_unknown_fields` keeps the surface narrow (§8): a mistyped key is a hard\nerror, never a silently ignored setting that disables a gate. The struct is\nflat rather than an enum with `#[serde(flatten)]` precisely so this guarantee\nholds — `flatten` silently defeats `deny_unknown_fields`.\n`severity` is required by every kind but `judge`, which is refused it\n(CLOUD-445). The field is an `Option` so a judge row can omit it, so the\nderived `required` list cannot carry it — and a schema that merely dropped it\nwould stop flagging the missing key on the four kinds that must have one,\nmoving a check out of the editor with nothing taking its place.\n\nThis conditional puts it back, stated once here and derived into both\npublished schemas. It mirrors [`RuleKind::requires`] and\n[`RuleKind::permits`]; `tests::the_schema_conditional_matches_the_column_census`\nis what keeps the two from drifting.", "type": "object", diff --git a/schema/batten.schema.json b/schema/batten.schema.json index 692123868..ca0004678 100644 --- a/schema/batten.schema.json +++ b/schema/batten.schema.json @@ -174,6 +174,13 @@ "$ref": "#/$defs/Provision" } }, + "redirect": { + "description": "The per-path-class redirect table (CLOUD-280): what to run instead,\nkeyed by what is protected rather than by the verb reaching for it.\n\nConsulted before [`MutatingVerb::redirect`], which stays the fallback, so\nthe behaviour CLOUD-96 shipped is the floor rather than a regression.\nDeliberately a sibling of [`Config::protected`] rather than a widening of\nit: that set keeps its element type, so [`crate::trust`]'s\n`protected[]` weakening keys are untouched.\n\n[`MutatingVerb::redirect`]: crate::verbs::MutatingVerb::redirect", + "type": "array", + "items": { + "$ref": "#/$defs/Redirect" + } + }, "rule": { "description": "The declarative rules run against the repository. Absent or empty means\n\"no rules configured\" and nothing is reported. Which of these a given\nverb admits is the §5 effect split: `check` runs only non-spawning kinds\nand refuses the rest, `enforce` runs all of them (CLOUD-170).\n\nEvery rule pins its `severity` explicitly — the key is required, with no\nimplicit fallback — and carries a separate `scope` key whose vocabulary\nnever conflates with severity's (CLOUD-61). Both disciplines are\nenforced at parse time: omission or conflation is a usage error here,\nnever a value quietly assumed.", "type": "array", @@ -1024,6 +1031,25 @@ } ] }, + "Redirect": { + "description": "One declared path class and the mutation sanctioned for it.", + "type": "object", + "properties": { + "glob": { + "description": "The path class this redirect speaks for, in the engine's one glob\ndialect.\n\nDeliberately independent of `protected`: this table answers \"what should\nthey run instead\", never \"is this guarded\". A glob here matching a path\nno `protected` entry covers simply never comes up, which is the harmless\ndirection — the alternative, deriving protection from a redirect, is the\nset-collapsing CLOUD-37 exists to prevent.", + "type": "string" + }, + "mutation": { + "description": "The sanctioned mutation for that class — the \"run this instead\" a deny\ncarries.", + "type": "string" + } + }, + "additionalProperties": false, + "required": [ + "glob", + "mutation" + ] + }, "Rule": { "description": "One declarative rule from `batten.toml`'s `[[rule]]` array.\n\n`deny_unknown_fields` keeps the surface narrow (§8): a mistyped key is a hard\nerror, never a silently ignored setting that disables a gate. The struct is\nflat rather than an enum with `#[serde(flatten)]` precisely so this guarantee\nholds — `flatten` silently defeats `deny_unknown_fields`.\n`severity` is required by every kind but `judge`, which is refused it\n(CLOUD-445). The field is an `Option` so a judge row can omit it, so the\nderived `required` list cannot carry it — and a schema that merely dropped it\nwould stop flagging the missing key on the four kinds that must have one,\nmoving a check out of the editor with nothing taking its place.\n\nThis conditional puts it back, stated once here and derived into both\npublished schemas. It mirrors [`RuleKind::requires`] and\n[`RuleKind::permits`]; `tests::the_schema_conditional_matches_the_column_census`\nis what keeps the two from drifting.", "type": "object", From 1cb3c5ce7ec06e0dbc387be11d70f99c612f85e9 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Wed, 19 Aug 2026 14:45:58 +0000 Subject: [PATCH 3/3] docs(core): map redirect.rs, and record the boundary its adoption found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `module-map-check` requires a row per crate module, and the row is worth more than the gate: the interesting fact about this table is not what it does but where it stops. Consumer #1 declares `.github/workflows/**` and `batten.toml` and deliberately not `.serena/memories/**`, because that class's remedy depends on the ACTION — `write_memory`, `edit_memory`, `rename_memory`, `delete_memory` — so a path row would override four correct per-verb answers with one weaker sentence. Per-path beats per-verb only where the path fact dominates, and the next author meets that boundary here rather than rediscovering it. Written through the Serena tool, which is the surface the protected-path gate names. The session that wrote the code could not add this row: Serena lost its handshake by ~80ms against a budget the host ignores, measured and recorded on CLOUD-668 with CLOUD-700 filed for the gate that would catch it. Refs: CLOUD-280 --- .serena/memories/core.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/.serena/memories/core.md b/.serena/memories/core.md index 22f655b54..6e0089ab2 100644 --- a/.serena/memories/core.md +++ b/.serena/memories/core.md @@ -623,6 +623,31 @@ repo config > default`, declared as data in `SETTINGS` (per-key env var/flag), crate↔config contract, hence the constant. Stated limits: no `cwd`, so an absolute or `..` path is compared as written, and expansion/substitution hide operands. Both under-deny, the sanctioned direction. +- `redirect.rs` — the per-path-class redirect table (CLOUD-280): what to run + instead, keyed by **what is protected** rather than by the verb reaching for + it. `[[redirect]]` is `{glob, mutation}`, and `hook::protected_refusal` + consults it BEFORE the verb's own `redirect` — three tiers, table then verb + then `Fix::None`, where the last two are CLOUD-96's behaviour untouched, so the + floor is structural rather than careful (`Fix::declared(Option<&str>)` was + built for this seam). Matching is `rules::glob_match` — one glob semantics for + the engine — over the SAME normalised path `protected.contains` was asked + about, or the two tables would disagree about which path is under discussion. + **Declaration order decides, first match wins**, the tie-break `shape_rules` + already uses and for its stated reason: a reviewer reads a table top to bottom, + and any cleverer precedence is a rule about rules the config does not state. + A **sibling** table rather than a wider `protected`, which keeps `Vec` + so `trust::removed_entries`'s `protected[]` weakening keys are + byte-identical (asserted). Not policy-bearing — it changes what a refusal says, + never whether it fires — so no raise-only clamp applies; the local layer may + add a class and may not redefine a committed one, and since local rows append + after committed ones, first-match-wins means an uncommitted file can never + change what a committed row says. **The boundary worth knowing**: consumer #1 + declares `.github/workflows/**` and `batten.toml`, and deliberately NOT + `.serena/memories/**` — that class's remedy depends on the ACTION + (`write_memory` / `edit_memory` / `rename_memory` / `delete_memory`), so a path + row would override four correct per-verb answers with one weaker sentence. + Per-path beats per-verb only where the path fact dominates. It makes a message + specific; it does not make the named surface reachable (CLOUD-663). - `refusal.rs` — the refusal contract (CLOUD-122): ONE `Refusal` value — `{rule, reason, fix}` — constructed at every deny site and projected onto whatever channel a host reads, so the shape is never re-typed per harness.