diff --git a/.claude/rules/policy-modules.md b/.claude/rules/policy-modules.md index fd5f298d1..5af7872b7 100644 --- a/.claude/rules/policy-modules.md +++ b/.claude/rules/policy-modules.md @@ -1,3 +1,9 @@ +--- +paths: + - "policy/**/*.rego" + - "batten.toml" +--- + # Moved to `rules/policy-modules.md` **The authority is [`rules/policy-modules.md`](../../rules/policy-modules.md). This file is a diff --git a/batten.toml b/batten.toml index 9fcb78d16..a844d20d5 100644 --- a/batten.toml +++ b/batten.toml @@ -6846,6 +6846,38 @@ severity = "deny" id = "release-tags" tags = "v*" +# CLOUD-1787's demonstration, and the row that found the defect it demonstrates. +# +# Both gates below read frontmatter, which nothing could do before +# `Format::Markdown`. They ship WITH it rather than after it: non-negotiable +# rule 2 — a rule without a runnable gate is half a change — and a parser with +# no consumer is a mechanism nobody has shown decides anything. +# +# `sources` RATHER THAN `documents`, because the set is a glob and not a list. +# A stub added tomorrow is judged without anybody remembering to name it here, +# which is the whole difference between a gate over a directory and a gate over +# the five files somebody typed. +[[rule]] +id = "prose carry missing" +kind = "policy" +scope = "tree" +module = "policy/rules-paths-trigger.rego" +severity = "deny" +sources = [".claude/rules/*.md"] + +# The second consumer, and the one that is GREEN on the tree it ships against — +# which is the half that makes the pair honest. A gate landed only where it +# already fires proves it can deny and never that it can pass; this one has four +# conforming files under it and a fixture proving it red, so both directions are +# demonstrated rather than one asserted. +[[rule]] +id = "prompt declare partial" +kind = "policy" +scope = "tree" +module = "policy/skill-frontmatter-complete.rego" +severity = "deny" +sources = ["skills/*/SKILL.md", ".claude/skills/*/SKILL.md"] + # The lockfile as it was STAGED (CLOUD-1203 unit A, the successor shape for # `lock-complete`). # @@ -8395,14 +8427,31 @@ measured = "2026-09-07" # COLD'S BASIS MOVES WITH WARM'S, for the 2026-09-06 entry's measured reason: # refreshing one and not the other made the very next lap refuse on the other arm # with warm never breached. +# THE 2026-09-11 REBASE MOVE, 244 -> 255, BOTH BASES, AND NEITHER FLOOR MOVES. +# +# WHAT MOVED THE COUNT is again the rebase rather than the new work alone. +# Replaying CLOUD-1787 onto the current trunk brought main's own additions — +# `release_token_precedence.rs` among them, which took the tree to 254 by itself — +# together with this branch's `frontmatter_gates.rs`. `target-prune` refused at +# `declared 244, live 255, tolerance 10`: the staleness arm, one file past the +# band, and not the disk. +# +# THE FLOORS DO NOT MOVE, for the 2026-09-08 entry's reason and not a new one. A +# basis refresh is a trend counter rather than a byte budget, so the count follows +# the tree and the floors stay where main re-measured them. Scaling them by +# 255/244 would be that entry's "same staleness wearing a newer number", and +# moving a floor still needs the independent measurement CLOUD-1158 owns. +# +# COLD'S BASIS MOVES WITH WARM'S, again because refreshing one and not the other +# makes the very next lap refuse on the other arm with warm never breached. [prune.warm.basis] glob = "crates/batten/tests/**/*.rs" -count = 244 +count = 255 tolerance = 10 [prune.cold.basis] glob = "crates/batten/tests/**/*.rs" -count = 244 +count = 255 tolerance = 10 # THE REGROWABLE ROOTS THE ESCALATION MAY DROP (CLOUD-1157), in the order it drops @@ -10292,6 +10341,80 @@ gloss = "declared and spent nowhere" word = "wrong" gloss = "does not match the declared shape" +[[verdict]] +id = "prose declare missing" +gloss = "a rule stub carries no `paths:` trigger, so the one mechanism it exists for does not fire" +class = """ +The stub is kept for its loading trigger and nothing else — its own body says +deleting it would cost that trigger. Without one it is a file that loads for +nobody, asserting in prose that it does. Measured: one stub had never carried +the trigger, from the commit that wrote the claim onward, because no gate could +read frontmatter to notice (CLOUD-1787, CLOUD-1152). +""" + +[[verdict.route]] +id = "prose write now" +kind = "document" +target = ".claude/rules" + +[[verdict]] +id = "prose read unread" +gloss = "a rule stub's frontmatter would not parse, so its trigger could not be read" +class = """ +Could-not-look, refused rather than reported clean: a module that iterates only +the stubs that parsed reports green over the one it never saw, and a dead gate +and a clean tree are byte-identical on the decision surface. +""" + +[[verdict.route]] +id = "prose write now" +kind = "document" +target = ".claude/rules" + +[[verdict]] +id = "prompt declare missing" +gloss = "a SKILL.md declares no usable `name` or `description`, so a harness has no basis to load it" +class = """ +The failure is silent by construction: a skill a harness never selects looks +exactly like one it selected and that had nothing to say. Blank counts as absent +because `name:` with nothing after it and `name: \"\"` both parse, and a +predicate testing only for the key passes both. +""" + +[[verdict.route]] +id = "prompt declare now" +kind = "document" +target = "skills" + +[[verdict]] +id = "prompt name wrong" +gloss = "a SKILL.md's declared `name` is not the directory it ships in" +class = """ +Two authorities for one identity. A harness routes by directory and displays by +`name`; while they disagree, neither is wrong on its own terms and the +disagreement is invisible to both. +""" + +[[verdict.route]] +id = "prompt declare now" +kind = "document" +target = "skills" + +[[verdict]] +id = "prompt read unread" +gloss = "a SKILL.md carries no frontmatter or would not parse, so its declared fields could not be read" +class = """ +Could-not-look, refused rather than reported clean, for `rules trigger unread`'s +reason. A `SKILL.md` with no fence declares no fields at all, which is the +violation above reaching this module through the acquisition layer instead of +through the document. +""" + +[[verdict.route]] +id = "prompt declare now" +kind = "document" +target = "skills" + [[verdict]] id = "provision declare dropped" gloss = "no [[provision.env]] row carries a no-proxy key, so the provisioned mise wrapper fences nothing" diff --git a/crates/batten/src/baseline.rs b/crates/batten/src/baseline.rs index 6d5fcaa34..c4810df69 100644 --- a/crates/batten/src/baseline.rs +++ b/crates/batten/src/baseline.rs @@ -542,6 +542,7 @@ impl Drifted { "baseline".to_owned(), "--prune".to_owned(), ])), + reason: None, }) } } @@ -709,6 +710,7 @@ mod tests { identity: StoredIdentity::new(FindingKind::Code, fingerprint), check: Check::Reevaluate, remediation: Some(Remediation::NoFix("fix it".to_owned())), + reason: None, } } diff --git a/crates/batten/src/budget.rs b/crates/batten/src/budget.rs index fcb21965d..a86402c46 100644 --- a/crates/batten/src/budget.rs +++ b/crates/batten/src/budget.rs @@ -258,6 +258,7 @@ impl Report { remediation: Some(crate::findings::Remediation::NoFix( "cut instruction text until the set is under its budget".to_owned(), )), + reason: None, }) } } @@ -311,16 +312,22 @@ pub fn loaded(text: &str) -> String { strip_comments(strip_frontmatter(text)) } -/// Drop a leading `---\n … \n---\n` frontmatter fence, if there is one. +/// Drop a leading frontmatter fence, if there is one. +/// +/// **The fence rule is `facts::split_frontmatter`'s and not a second one here** +/// (CLOUD-1787). Both sides answer the same question — where does this file's +/// frontmatter end — and they answered it in two hand-rolled scans until +/// `Format::Markdown` needed the other half of the same split. Two spellings of +/// one rule is how a file gets taxed for bytes the loader drops, or read as a +/// document the loader counts. +/// +/// Sharing it widened this side: a BOM, `\r\n` and a `...` terminator are now +/// recognised where the scan here saw only `---\n` … `\n---\n`. That is a +/// correction rather than a drift — the loader drops those fences too, so +/// counting them was always over-taxing. fn strip_frontmatter(text: &str) -> &str { - let Some(rest) = text.strip_prefix("---\n") else { - return text; - }; - // The closing fence is a `---` on its own line. Without one the document has - // no frontmatter, only a horizontal rule — leave it alone rather than - // guessing where it ends. - match rest.find("\n---\n") { - Some(end) => &rest[end + "\n---\n".len()..], + match crate::facts::split_frontmatter(text) { + Some(found) => found.body, None => text, } } @@ -634,6 +641,16 @@ mod tests { ); // An unterminated fence is not frontmatter. assert_eq!(loaded("---\nname: x\n"), "---\nname: x\n"); + // The three spellings the shared rule recognises that the scan here did + // not (CLOUD-1787). Each was over-taxed before: the loader drops these + // fences too, so counting them charged for bytes that never reach a + // context window. + // + // Fails by: re-hand-rolling the fence scan in this module, which is how + // the two readers disagreed about the same bytes in the first place. + assert_eq!(loaded("\u{feff}---\nname: x\n---\nbody\n"), "body\n"); + assert_eq!(loaded("---\r\nname: x\r\n---\r\nbody\n"), "body\n"); + assert_eq!(loaded("---\nname: x\n...\nbody\n"), "body\n"); } #[test] diff --git a/crates/batten/src/defects.rs b/crates/batten/src/defects.rs index a5808a735..d688688f5 100644 --- a/crates/batten/src/defects.rs +++ b/crates/batten/src/defects.rs @@ -233,6 +233,7 @@ impl Problem { remediation: Some(crate::findings::Remediation::NoFix( "resolve or rewrite the ledger row this points at".to_owned(), )), + reason: None, } } } diff --git a/crates/batten/src/design.rs b/crates/batten/src/design.rs index 6ffde20d0..b0192dc0e 100644 --- a/crates/batten/src/design.rs +++ b/crates/batten/src/design.rs @@ -376,6 +376,7 @@ impl Problem { remediation: Some(crate::findings::Remediation::NoFix( "rewrite or withdraw the design claim this points at".to_owned(), )), + reason: None, } } } diff --git a/crates/batten/src/facts.rs b/crates/batten/src/facts.rs index 2c80157ff..92ba8b0bd 100644 --- a/crates/batten/src/facts.rs +++ b/crates/batten/src/facts.rs @@ -2644,6 +2644,19 @@ pub enum Format { Json5, /// PKL. Declarable, never parsed — see the type's own note. Pkl, + /// Markdown, read as its YAML **frontmatter** and never as its prose + /// (CLOUD-1787). + /// + /// Still a format and not an artifact: markdown is a markup language, the + /// same kind of name as TOML, and which paths carry it stays the consumer's + /// `batten.toml`. + /// + /// **Not a fifth parser**, which CLOUD-846 refused and this keeps refused. + /// The body is never handed to anything — headings and tables stay line + /// predicates. The fence's contents are YAML, parsed by the one YAML + /// authority this type already carries, so nothing here knows a second + /// grammar. + Markdown, } impl Format { @@ -2654,6 +2667,7 @@ impl Format { Format::Json, Format::Json5, Format::Pkl, + Format::Markdown, ]; /// The stable lowercase token used in config and machine output (§6). @@ -2665,6 +2679,7 @@ impl Format { Format::Json => "json", Format::Json5 => "json5", Format::Pkl => "pkl", + Format::Markdown => "markdown", } } @@ -2676,7 +2691,7 @@ impl Format { #[must_use] pub const fn parseable(self) -> bool { match self { - Format::Toml | Format::Yaml | Format::Json | Format::Json5 => true, + Format::Toml | Format::Yaml | Format::Json | Format::Json5 | Format::Markdown => true, Format::Pkl => false, } } @@ -2731,18 +2746,37 @@ impl Format { Format::Json => &["json"], Format::Json5 => &["json5"], Format::Pkl => &[], + Format::Markdown => &["md"], } } /// Parse `text` as this format. /// /// Three-valued by construction (CLOUD-757): a document that parses is - /// [`Look::Is`], and **anything else is [`Look::CouldNotLook`]** — a syntax - /// error, an empty YAML stream, a format this crate cannot parse. It is - /// never [`Look::IsNot`], because a file failing to parse says nothing at - /// all about what it contains. That distinction is the whole point: the - /// hand-rolled readers this replaces default an empty extraction to - /// agreement, so a file they cannot read passes every gate over it. + /// [`Look::Is`], and a syntax error, an empty YAML stream or a format this + /// crate cannot parse is [`Look::CouldNotLook`] — because a file failing to + /// parse says nothing at all about what it contains. That distinction is the + /// whole point: the hand-rolled readers this replaces default an empty + /// extraction to agreement, so a file they cannot read passes every gate + /// over it. + /// + /// # `IsNot` has exactly one meaning here, and only [`Format::Markdown`] has it + /// + /// This arm said *never* [`Look::IsNot`] until CLOUD-1787, and the reason it + /// gave still holds for every case it was written about: a **parse failure** + /// is not evidence of absence. Markdown is not that case. A well-formed + /// markdown file with no frontmatter fence is not one this reader failed on + /// — it is one it read completely, and there is no document in it. That is + /// *looked, and it is not there*, which is what [`Look::IsNot`] means. + /// + /// The vacuous pass the old wording guarded against is closed one layer up + /// rather than here: [`crate::rules::parse_node`] maps this `IsNot` to a + /// **`NotAcquired`** cause of its own, so a file with no frontmatter is a + /// non-answer a module must handle, never an empty document that satisfies + /// every predicate over it. Collapsing it into [`Look::CouldNotLook`] + /// instead would say the reader broke, and send an author looking for a + /// malformed fence that is simply absent — CLOUD-1787's own harm, one + /// argument over. #[must_use] pub fn read(self, text: &str) -> Look { match self { @@ -2750,17 +2784,7 @@ impl Format { Ok(value) => Look::Is(Node::from_toml(&value)), Err(_) => Look::CouldNotLook, }, - Format::Yaml => match yaml_rust2::YamlLoader::load_from_str(text) { - // The first document of the stream, and only it. A multi-document - // stream addressed as one would need a document index in every - // node path, which no consumer here has; taking the first is the - // narrow answer rather than a silent merge of several. - Ok(documents) => match documents.first() { - Some(document) => Look::Is(Node::from_yaml(document)), - None => Look::CouldNotLook, - }, - Err(_) => Look::CouldNotLook, - }, + Format::Yaml => read_yaml(text), Format::Json => match serde_json::from_str::(text) { Ok(value) => Look::Is(Node::from_json(&value)), Err(_) => Look::CouldNotLook, @@ -2771,10 +2795,116 @@ impl Format { }, // Declared, and honestly unanswerable. See the type's own note. Format::Pkl => Look::CouldNotLook, + // The fence, and only the fence. The body is never handed to a + // parser, which is the whole defect CLOUD-1787 reports: a markdown + // body is only accidentally valid YAML, so reading the file as one + // YAML stream makes every verdict depend on whether the prose + // happens to lex. + Format::Markdown => match split_frontmatter(text).map(|found| found.block) { + // Present and empty is an ANSWER, not a non-answer: a file whose + // fence is `---\n---` has frontmatter and it has no keys, which + // is a different authoring fault from having no fence at all and + // must be a different verdict. Routing it through `read_yaml` + // would make it an empty stream and so `CouldNotLook`, which is + // the one reading that is false. + Some(block) if block.trim().is_empty() => { + Look::Is(Node::Map(std::collections::BTreeMap::new())) + } + Some(block) => read_yaml(block), + None => Look::IsNot, + }, } } } +/// The parseable extensions this build owns, as a prose list for a refusal. +/// +/// Derived from [`Format::ALL`] rather than written out, so a format added +/// later cannot leave a refusal naming a stale set — the class of defect the +/// hardcoded "TOML, YAML, JSON and JSON5" was one instance of. +#[must_use] +pub(crate) fn parseable_extensions() -> String { + let mut names: Vec<&str> = Format::ALL + .iter() + .filter(|format| !format.extensions().is_empty()) + .map(|format| format.as_str()) + .collect(); + names.sort_unstable(); + names.join(", ") +} + +/// Parse `text` as a YAML stream and answer with its first document. +/// +/// Shared by [`Format::Yaml`] and [`Format::Markdown`] so there is exactly one +/// YAML reader in this crate — the frontmatter format adds a *container*, never +/// a grammar, and two spellings of one parse is how the two would drift into +/// disagreeing about the same bytes. +/// +/// The first document of the stream, and only it. A multi-document stream +/// addressed as one would need a document index in every node path, which no +/// consumer here has; taking the first is the narrow answer rather than a silent +/// merge of several. +fn read_yaml(text: &str) -> Look { + match yaml_rust2::YamlLoader::load_from_str(text) { + Ok(documents) => match documents.first() { + Some(document) => Look::Is(Node::from_yaml(document)), + None => Look::CouldNotLook, + }, + Err(_) => Look::CouldNotLook, + } +} + +/// The contents of a leading frontmatter fence, if `text` opens with one. +/// +/// **Strict, and the strictness is the point.** The fence opens on the FIRST +/// line or there is none: a `---` further down is a horizontal rule, and a +/// reader that went looking for one would turn ordinary prose into a document +/// and decide over it. A UTF-8 byte-order mark may precede it and nothing else, +/// because a BOM is invisible to the author and refusing it would blame a file +/// for its encoding. +/// +/// The closing delimiter is `---` or `...` alone on a line — YAML's own two +/// document terminators, so a consumer who writes the one this crate did not +/// think of still gets read. Without a terminator there is no frontmatter: +/// guessing where an unterminated fence ends is the same error as finding one +/// mid-file. +/// +/// `\r\n` is accepted throughout. +/// +/// Returns both halves as NAMED fields rather than a tuple, because the two +/// callers want opposite ones — [`Format::Markdown`] reads the block and +/// [`crate::budget`] drops it and counts the body — and a tuple would have each +/// of them spell `, _)`, which `no_axis_match_carries_a_wildcard_arm` refuses +/// in this file for an unrelated and better reason. +pub(crate) fn split_frontmatter(text: &str) -> Option> { + let text = text.strip_prefix('\u{feff}').unwrap_or(text); + let rest = match text.strip_prefix("---\n") { + Some(rest) => rest, + None => text.strip_prefix("---\r\n")?, + }; + let mut offset = 0usize; + for line in rest.split_inclusive('\n') { + let bare = line.strip_suffix('\n').unwrap_or(line); + let bare = bare.strip_suffix('\r').unwrap_or(bare); + if bare == "---" || bare == "..." { + return Some(Frontmatter { + block: &rest[..offset], + body: &rest[offset + line.len()..], + }); + } + offset += line.len(); + } + None +} + +/// A markdown file split at its frontmatter fence, by [`split_frontmatter`]. +pub(crate) struct Frontmatter<'a> { + /// What the fence encloses, without either delimiter line. + pub(crate) block: &'a str, + /// Everything after the closing delimiter. + pub(crate) body: &'a str, +} + /// A parsed document, canonicalised into one shape whatever it was written in. /// /// # One tree, four syntaxes diff --git a/crates/batten/src/findings.rs b/crates/batten/src/findings.rs index 2d8955425..6a1cc6d74 100644 --- a/crates/batten/src/findings.rs +++ b/crates/batten/src/findings.rs @@ -1187,6 +1187,7 @@ mod tests { identity: identity_for("r", "src/a.rs", "TODO"), check: Check::Reevaluate, remediation: Some(Remediation::NoFix("fixture".to_owned())), + reason: None, } } diff --git a/crates/batten/src/hookcost.rs b/crates/batten/src/hookcost.rs index b95eb8988..9b13dace3 100644 --- a/crates/batten/src/hookcost.rs +++ b/crates/batten/src/hookcost.rs @@ -317,6 +317,7 @@ fn finding(rule: &str, subject: String, line: Option, remedy: &str) -> Fi line, check: crate::findings::Check::Reevaluate, remediation: Some(crate::findings::Remediation::NoFix(remedy.to_owned())), + reason: None, } } diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 2e8035b63..25e57129c 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -18160,14 +18160,23 @@ fn report_rule_costs(mode: Mode, err: &mut dyn Write) -> Result<()> { let elapsed: std::time::Duration = costs.iter().map(|cost| cost.elapsed).sum(); let files: usize = costs.iter().map(|cost| cost.files_read).sum(); let bytes: usize = costs.iter().map(|cost| cost.bytes_read).sum(); + // COUNTS RULES, AND THE CENSUS NOW CARRIES A ROW THAT IS NOT ONE + // (CLOUD-1790). The shared acquisition earns its own row so its reads have + // somewhere to land; counting it here would report one more rule than the + // config declares, which is a second small lie in a line that exists to stop + // one. Its files and bytes still ride the totals above, because the run did + // read them. + let rules_counted = costs + .iter() + .filter(|cost| cost.rule != rules::DECLARED_ACQUISITION) + .count(); output::message( mode, Verbosity::Debug, err, &format!( - "rule cost: {} rule(s) {}ms {files} file(s) {bytes} byte(s)", - costs.len(), - elapsed.as_millis(), + "rule cost: {rules_counted} rule(s) {}ms {files} file(s) {bytes} byte(s)", + elapsed.as_millis() ), )?; Ok(()) diff --git a/crates/batten/src/policy.rs b/crates/batten/src/policy.rs index e90f76bab..a2a5fdb1d 100644 --- a/crates/batten/src/policy.rs +++ b/crates/batten/src/policy.rs @@ -2583,7 +2583,7 @@ pub fn tree_input_schema() -> Result { "missing".to_owned(), serde_json::json!({ "type": "object", - "description": "Could-not-look, and NOT a Fact: a declared name the engine could not acquire, mapped to WHY (CLOUD-1309). Distinct from an empty result, which is the distinction that keeps a vacuous pass out (CLOUD-251, CLOUD-845). The cause is one of `absent`, `unparsed`, `unreadable`, `unknown-format` or `root-unset` -- `NotAcquired::as_str`'s own tokens -- and carrying it is what makes `rules/policy-modules.md`'s \"the two causes stay distinct and a module may rely on that\" true of this surface rather than only of the Rust enum. A module keys by name: `input.tree.missing[path] == \"unparsed\"`. It was an array of bare names until CLOUD-1309, so a predicate could see THAT a source failed and never WHY, and firing on membership alone reddened every consumer that declares an optional source.", + "description": "Could-not-look, and NOT a Fact: a declared name the engine could not acquire, mapped to WHY (CLOUD-1309). Distinct from an empty result, which is the distinction that keeps a vacuous pass out (CLOUD-251, CLOUD-845). The cause is one of `absent`, `unparsed`, `unreadable`, `unknown-format`, `root-unset` or `no-document` -- `NotAcquired::as_str`'s own tokens -- and carrying it is what makes `rules/policy-modules.md`'s \"the two causes stay distinct and a module may rely on that\" true of this surface rather than only of the Rust enum. A module keys by name: `input.tree.missing[path] == \"unparsed\"`. It was an array of bare names until CLOUD-1309, so a predicate could see THAT a source failed and never WHY, and firing on membership alone reddened every consumer that declares an optional source.", "additionalProperties": {"type": "string"}, }), ); diff --git a/crates/batten/src/rules.rs b/crates/batten/src/rules.rs index ae1c0aa02..eced5557b 100644 --- a/crates/batten/src/rules.rs +++ b/crates/batten/src/rules.rs @@ -4461,11 +4461,13 @@ impl Rule { return Err(UsageError::raise(format!( "rule {}: `documents` names `{path}`, whose extension this build has no \ parser for — the row would skip silently rather than decide. Parseable \ - extensions are TOML, YAML, JSON and JSON5", - self.id + extensions are {}", + self.id, + crate::facts::parseable_extensions() ))); } } + self.validate_declared_format_matches_glob()?; // THE CONVERSE WAS A REFUSAL AND IS NOT ONE ANY MORE (CLOUD-845). It // read: a tree row with no declared documents "is handed an empty tree // and decides nothing about the repository", so refusing it was better @@ -4488,6 +4490,56 @@ impl Rule { Ok(()) } + /// Refuse a `document` row whose declared `format` contradicts the extension + /// its glob names (CLOUD-1787). + /// + /// # The trap this closes + /// + /// `format` is declared and never inferred, which is right — a `.json` file + /// that is really JSON5 must be declarable as JSON5. But nothing checked the + /// declaration against the paths it would meet, and one pairing is actively + /// dangerous rather than merely wrong: `format = "yaml"` over `*.md` + /// **appears to work**, because `---` is YAML's document-start marker, so a + /// markdown file's frontmatter is document 1 of the stream and gets read. + /// Document 2 is the body, which is only accidentally valid YAML. Measured + /// in CLOUD-1787: with identical correct frontmatter, prose and `**bold**` + /// passed while a table, a task list and a block quote failed — the verdict + /// decided by the prose, not the data. + /// + /// A row whose correctness depends on whether prose happens to lex is not a + /// gate, and it fails silently in both directions, so it is refused at load + /// where a config error costs a config error. + /// + /// # Why the literal suffix and not the glob + /// + /// Deciding what an arbitrary glob *can* match needs a glob engine and + /// answers "possibly" for most inputs. The extension a glob ENDS in is a + /// string, it is what every real row writes (`*.md`, `**/*.md`, + /// `notes/*.md`), and it is exactly the case above. A computed or + /// extensionless glob is not caught here and the message says so rather than + /// implying a totality this does not have. + fn validate_declared_format_matches_glob(&self) -> anyhow::Result<()> { + let (Some(declared), Some(glob)) = (self.format, self.glob.as_deref()) else { + return Ok(()); + }; + let Some(named) = crate::facts::Format::for_path(glob) else { + return Ok(()); + }; + if named == declared { + return Ok(()); + } + Err(UsageError::raise(format!( + "rule {}: `glob` is `{glob}`, whose extension names {}, but `format` says {} — one \ + of the two is wrong, and the pairing is admitted by neither. Declare `format = \ + \"{}\"` to read the file as its extension says. (Checked on the literal extension \ + a glob ends in; a computed glob is not checked.)", + self.id, + named.as_str(), + declared.as_str(), + named.as_str() + ))) + } + /// Validate that the per-kind fields present match the declared `kind`. /// /// The struct is flat (a `#[serde(flatten)]` enum would silently defeat @@ -5879,6 +5931,30 @@ pub struct Finding { /// [`Rule::validate`] would have refused; [`crate::findings::record`] /// refuses to store one. pub remediation: Option, + /// Which of the producing kind's answers this finding is, where the kind + /// has more than one (CLOUD-1787). + /// + /// `None` for every kind whose findings mean one thing. A `document` row + /// has three — the node differs, the node is absent, the document could not + /// be read, and now a fourth: there is no document here — and they were + /// **computed and then dropped**, reaching only + /// [`crate::identity::code_fingerprint`]. So two findings with opposite + /// remedies printed the same `path:1 rule-id`, and an author told their + /// `tier` was wrong went looking at a `tier` that was correct. That is the + /// harm CLOUD-1787 reports first, and it is an OUTPUT defect: the decision + /// was always right. + /// + /// # Still a pointer (rule 4) + /// + /// A closed vocabulary of `&'static str` tokens the engine owns, never a + /// byte of the file. `no-document` says what the reader found, not what the + /// file says. + /// + /// Skipped when absent, so every finding that does not set one serialises + /// exactly as it did before — §6 movement is confined to the kind that + /// gained the field. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub reason: Option<&'static str>, } /// The renderer this type went without (CLOUD-371). @@ -5897,9 +5973,15 @@ pub struct Finding { /// degraded one. impl crate::output::Line for Finding { fn line(&self) -> String { - match self.line { + let at = match self.line { Some(line) => format!("{}:{} {}", self.path, line, self.rule), None => format!("{} {}", self.path, self.rule), + }; + // Appended rather than interpolated into the pointer, so a consumer + // parsing `path:line rule` off the front still parses it. + match self.reason { + Some(reason) => format!("{at} {reason}"), + None => at, } } } @@ -6658,9 +6740,10 @@ fn run( // per consumer. Here the producer pays once and every reader reads. let derived = resolve_derived(rules, root, &files); // Resolved ONCE for the whole run, beside the two above and for the same - // reason (CLOUD-850): every document the rule set declares, read and parsed - // once rather than once per rule. `documents_acquired` is what asserts it. - let documents = acquire_declared(rules, root, &files)?; + // reason (CLOUD-850), and measured onto its own census row. Extracted rather + // than inline only because it put `run` over `too_many_lines`; the reasoning + // it carries is the callee's own. + let documents = acquire_declared_measured(rules, root, &files)?; // WHAT EARLIER RUNS PRODUCED, acquired once for the whole run beside the // three above and for the same reason (CLOUD-851). Bounded by DECLARATION — @@ -6835,9 +6918,6 @@ fn run( }; let mut scan = Scan::default(); - // CLEARED, not appended to: the census describes THIS run, and a caller - // reading rows from the previous one would be reading a different tree. - costs_lock().clear(); evaluate_rules(rules, root, &inputs, &mut scan)?; // BEFORE the sort, deliberately (CLOUD-396): the sort is what makes the // output byte-stable, so a dedup running after it would be reading an order @@ -7490,9 +7570,12 @@ pub(crate) enum NotAcquired { /// /// Decided **before any I/O**, and it is the one arm that is a config fault /// rather than a verdict: no state of the filesystem makes a declared - /// `.md`/`.bats`/`.pkl` path parseable, so reporting it as could-not-look - /// would be a gate reporting a permanent authoring error as a transient - /// one. + /// `.bats`/`.pkl` path parseable, so reporting it as could-not-look would be + /// a gate reporting a permanent authoring error as a transient one. + /// + /// `.md` was in that list until CLOUD-1787 and no longer is — it names + /// [`crate::facts::Format::Markdown`], whose frontmatter is a document this + /// build can read. UnknownFormat, /// `ENOENT` — the tree does not carry the declared path. Absent, @@ -7504,6 +7587,21 @@ pub(crate) enum NotAcquired { Unreadable, /// Read as text, and the parser refused it. Unparsed, + /// Read as text in full, and the format found no document in it + /// (CLOUD-1787). + /// + /// Its own arm rather than [`Unparsed`](Self::Unparsed), for the reason + /// [`RootUnset`](Self::RootUnset) has one below: a different claim and a + /// different remedy. `Unparsed` says *this document is malformed, fix it*; + /// this says *there is no document here to fix*. A markdown file with no + /// frontmatter fence is the only source of it today, and it is precisely the + /// case CLOUD-1787 reports being reported as the other one. + /// + /// **Not silence, and not an empty document.** It reaches `missing` like + /// every other cause, so a module that iterates only what it could read + /// still cannot report green over it — which is the vacuous pass the whole + /// three-valued contract exists to refuse. + NoDocument, /// An `[[rule.external]]` row named a root environment variable this machine /// does not set, or sets to nothing (CLOUD-1167). /// @@ -7526,6 +7624,7 @@ impl NotAcquired { NotAcquired::Absent => "absent", NotAcquired::Unreadable => "unreadable", NotAcquired::Unparsed => "unparsed", + NotAcquired::NoDocument => "no-document", NotAcquired::RootUnset => "root-unset", } } @@ -7735,6 +7834,31 @@ pub(crate) fn acquire(root: &Path, rel_path: &str, want: Option) -> Acquir // share with its siblings before this collapse. Err(_) => return Acquired::No(NotAcquired::Unreadable), }; + // COUNTED HERE, AND IT WAS COUNTED NOWHERE BEFORE (CLOUD-1790). This is the + // one function that acquires a document, so every `policy` and `document` + // rule's reads ran past `count_read` entirely and each reported + // `0 file(s) 0 byte(s)` however much it had read. The `forbid`/`ratchet` + // family counts at its own read sites, so the census was not empty — it was + // SELECTIVELY empty, which is worse: the kinds reporting nothing are exactly + // the kinds whose silence is hardest to tell from a gate that decided + // nothing. + // + // Measured: that false zero is what CLOUD-1790 was filed on, as "tree-scoped + // policy rules receive no input at all". They receive it — a row in the + // reporter's own shape reads its source, decides, and emits a finding while + // reporting zero. A consumer compared that zero against a `forbid` row in the + // same config reporting real numbers and rewrote three modules and about + // twenty predicates around a defect that was in the odometer. + // + // AFTER `fs::read` AND BEFORE THE UTF-8 DECODE, which is `count_read`'s own + // contract read literally: it counts a read that SUCCEEDED, and decoding is + // parsing rather than reading. A file of non-UTF-8 bytes cost exactly the + // open and the bytes this counter exists to account for, and skipping it + // would under-report the case most expensive to discover. The + // `UnknownFormat` arm above stays uncounted for the reason stated there — it + // is decided before any I/O — which is the same asymmetry `DOCUMENTS_ACQUIRED` + // is placed for. + count_read(bytes.len()); let Ok(text) = String::from_utf8(bytes) else { return Acquired::No(NotAcquired::Unreadable); }; @@ -7794,13 +7918,24 @@ pub(crate) fn acquire(root: &Path, rel_path: &str, want: Option) -> Acquir /// [`NotAcquired::Unparsed`] when the grammar refused. A file that will not parse /// says nothing about what it contains, which is `Format::read`'s own /// three-valued contract — and the one thing it must never resolve to is silence. +/// +/// [`NotAcquired::NoDocument`] when the format read the file completely and +/// found no document in it — today only [`crate::facts::Format::Markdown`] over +/// a file with no frontmatter fence. Its own cause rather than `Unparsed`, +/// because the remedy is the opposite one: `Unparsed` says *fix this fence*, +/// this says *there is no fence*. pub(crate) fn parse_node( format: crate::facts::Format, text: &str, ) -> Result { match format.read(text) { crate::facts::Look::Is(node) => Ok(node), - crate::facts::Look::IsNot | crate::facts::Look::CouldNotLook => Err(NotAcquired::Unparsed), + // Both are non-answers, and they are DIFFERENT non-answers. Collapsing + // them — which this did until CLOUD-1787 — reports "no frontmatter" as + // a parse failure and sends an author looking for a malformed fence + // that is simply absent. + crate::facts::Look::IsNot => Err(NotAcquired::NoDocument), + crate::facts::Look::CouldNotLook => Err(NotAcquired::Unparsed), } } @@ -7975,6 +8110,53 @@ fn select_declared( /// Keyed by repo-relative path in a [`BTreeMap`], so iteration order is the /// paths' and never the rules' — the ordering property that has to hold before /// the batch could ever be filled concurrently. +/// [`acquire_declared`] for one whole run, clearing the census first and charging +/// what it read to a row of its own. +/// +/// MEASURED, AND ON ITS OWN CENSUS ROW (CLOUD-1790). The per-rule deltas wrap +/// `run_rule`, and this runs once before that loop — so a `policy` row's declared +/// reads happen outside every rule's measurement window and each such row reports +/// `0 file(s) 0 byte(s)` however much the run read on its behalf. The run total is +/// a SUM of those rows, so the reads were invisible at both levels, and a reader +/// comparing a policy row's zero against a `forbid` row's real numbers concludes +/// the policy row received nothing. That is what CLOUD-1790 was filed on. +/// +/// ATTRIBUTED TO THE STEP RATHER THAN TO A RULE, which is the only honest place +/// for it. The cache is shared deliberately — N rows over one path in one form is +/// one read — so charging it to whichever row the loop reached first would starve +/// the others of an answer they equally caused, the same error the `(path, form)` +/// key exists to prevent one level down. +/// +/// ONLY WHEN THERE IS A STEP TO REPORT: a rule set declaring no documents acquires +/// nothing and gets no row, so a run of `forbid` rows reads exactly as it did +/// before this landed. +/// +/// CLEARED HERE, not appended to, and at the TOP of the run rather than the top of +/// the rule loop — which is where the clear sat and what this needs (CLOUD-1790). +/// The census describes THIS run, and the acquisition is part of it and measured +/// before any rule evaluates, so a clear placed after it wipes the row that +/// measured it — silently, since a missing census row and a step that cost nothing +/// read identically. +fn acquire_declared_measured( + rules: &[Rule], + root: &Path, + files: &[String], +) -> anyhow::Result> { + costs_lock().clear(); + let started = std::time::Instant::now(); + let (files_before, bytes_before) = (files_read(), bytes_read()); + let documents = acquire_declared(rules, root, files)?; + if !documents.is_empty() { + costs_lock().push(RuleCost { + rule: DECLARED_ACQUISITION.to_owned(), + elapsed: started.elapsed(), + files_read: files_read().saturating_sub(files_before), + bytes_read: bytes_read().saturating_sub(bytes_before), + }); + } + Ok(documents) +} + pub(crate) fn acquire_declared( rules: &[Rule], root: &Path, @@ -9580,6 +9762,7 @@ fn policy_rule( // duplication that registry exists to remove. remediation: policy_remediation(registry, &violation.verdict), identity, + reason: None, }); } None @@ -10152,6 +10335,7 @@ fn ratchet_finding( // integer pair. identity::scope_fingerprint(&rule.id, glob), ), + reason: None, }); } @@ -10885,6 +11069,7 @@ fn push_case_finding( identity: identity_of(rule, identity::FindingKind::Code, default), check: rule.settling_check().unwrap_or(Check::Reevaluate), remediation: rule.remediation(), + reason: None, }); } @@ -10992,6 +11177,7 @@ fn unresolved_subject( identity: identity_of(rule, identity::FindingKind::Code, default), check: rule.settling_check().unwrap_or(Check::Reevaluate), remediation: rule.remediation(), + reason: None, }); } @@ -11640,6 +11826,7 @@ fn run_once( line: None, check: rule.settling_check().unwrap_or(Check::Reevaluate), remediation: rule.remediation(), + reason: None, }); } Ok(()) @@ -11829,6 +12016,7 @@ fn forbid_in_files( identity: identity_of(rule, identity::FindingKind::Code, default), check: rule.settling_check().unwrap_or(Check::Reevaluate), remediation: rule.remediation(), + reason: None, }); } } @@ -11929,6 +12117,12 @@ fn document_in_file( | NotAcquired::UnknownFormat | NotAcquired::RootUnset, ) => Some(DOCUMENT_UNREADABLE), + // Read in full, and there is no document in it — its own reason rather + // than folded into the arm above (CLOUD-1787). Still a FINDING and + // never silence, which is CLOUD-772's decision and unchanged: a row + // pointed at a file carrying no document has not been satisfied, it has + // been unable to look. What changes is that it now says which. + Acquired::No(NotAcquired::NoDocument) => Some(DOCUMENT_NO_DOCUMENT), // Unreachable: this site always asks for `Want::Parsed`. Arms rather // than a wildcard so a caller that ever asks for lines or call sites // here has to decide what a node path means over them. @@ -11972,6 +12166,10 @@ fn document_in_file( identity: identity_of(rule, identity::FindingKind::Code, default), check: rule.settling_check().unwrap_or(Check::Reevaluate), remediation: rule.remediation(), + // The reason was already computed and already in the fingerprint above; + // until CLOUD-1787 it stopped there, so `node-differs` and + // `could-not-look` printed the same bytes. + reason: Some(reason), }); Ok(()) } @@ -11995,6 +12193,8 @@ fn unreadable_document(rule: &Rule, rel_path: &str, node_path: &str) -> anyhow:: identity: identity_of(rule, identity::FindingKind::Code, default), check: rule.settling_check().unwrap_or(Check::Reevaluate), remediation: rule.remediation(), + // The same token the fingerprint above already carries. + reason: Some(DOCUMENT_UNREADABLE), }) } @@ -12109,6 +12309,25 @@ const DOCUMENT_NODE_ABSENT: &str = "node-absent"; /// The node is there and holds something other than the declared literal. const DOCUMENT_NODE_DIFFERS: &str = "node-differs"; +/// The census row naming the one shared acquisition rather than a rule +/// (CLOUD-1790). +/// +/// Parenthesised so it cannot collide with a [`Rule::id`]: a row id is a config +/// key and this is a step, and a reader sorting the census by cost must be able +/// to tell "this gate was expensive" from "the run read this much before any gate +/// ran". +pub const DECLARED_ACQUISITION: &str = "(declared documents)"; + +/// The file was read in full and holds no document at all — today a markdown +/// file with no frontmatter fence (CLOUD-1787). +/// +/// Distinct from [`DOCUMENT_UNREADABLE`] because the remedy is the opposite one. +/// `could-not-look` says *this document is malformed*; this says *there is no +/// document here*, and an author told the first about the second goes looking +/// for a fence that is simply absent — which is the report CLOUD-1787 was filed +/// on. +const DOCUMENT_NO_DOCUMENT: &str = "no-document"; + /// The entry whose presence makes a directory a repository of its own — git's /// own boundary marker, and therefore the one this crate reads. /// @@ -12937,6 +13156,16 @@ mod tests { std::fs::write(dir.join("broken.toml"), "key = = =\n").unwrap(); // An extension this build has no parser for. Note it EXISTS: the point // is that the cause is the declaration, not the filesystem. + // + // `.bats` RATHER THAN `.md`, WHICH IS WHAT THIS FIXTURE USED TO BE + // (CLOUD-1787). `.md` names `Format::Markdown` now, so it is parseable + // and lands two lines down under its own cause. Swapping the extension + // keeps the case this assertion is about — a declaration the build + // cannot honour, decided before any I/O — rather than deleting it. + std::fs::write(dir.join("script.bats"), "@test \"x\" { true; }\n").unwrap(); + // Read in full, and there is no document in it. A markdown file with no + // frontmatter fence: not a config fault, not a syntax error, and not an + // empty document either. std::fs::write(dir.join("prose.md"), "# heading\n").unwrap(); let acquire = @@ -12969,20 +13198,30 @@ mod tests { ); assert!( matches!( - acquire("prose.md"), + acquire("script.bats"), super::Acquired::No(super::NotAcquired::UnknownFormat) ), "an extension with no parser is a CONFIG fault, and is reached \ without opening the file" ); + assert!( + matches!( + acquire("prose.md"), + super::Acquired::No(super::NotAcquired::NoDocument) + ), + "A MARKDOWN FILE WITH NO FENCE IS `NoDocument`, NOT `Unparsed` — the \ + pair CLOUD-1787 was filed on. `Unparsed` sends an author looking \ + for a malformed fence that is simply absent" + ); - // And the four tokens are distinct, or naming the cause would not + // And the tokens are distinct, or naming the cause would not // discriminate — the failure this whole split exists to prevent. let tokens = [ super::NotAcquired::UnknownFormat.as_str(), super::NotAcquired::Absent.as_str(), super::NotAcquired::Unreadable.as_str(), super::NotAcquired::Unparsed.as_str(), + super::NotAcquired::NoDocument.as_str(), ]; let unique: std::collections::BTreeSet<&str> = tokens.iter().copied().collect(); assert_eq!( diff --git a/crates/batten/src/secrets.rs b/crates/batten/src/secrets.rs index 8ecda4347..f77cab6f1 100644 --- a/crates/batten/src/secrets.rs +++ b/crates/batten/src/secrets.rs @@ -1098,6 +1098,7 @@ pub fn scan( // would have silently dropped every secret finding at the store // boundary. remediation: rule.remediation(), + reason: None, }); } Ok(()) diff --git a/crates/batten/src/waiver.rs b/crates/batten/src/waiver.rs index d63a46aba..b245be489 100644 --- a/crates/batten/src/waiver.rs +++ b/crates/batten/src/waiver.rs @@ -639,6 +639,7 @@ mod tests { ), check: crate::findings::Check::Reevaluate, remediation: Some(crate::findings::Remediation::NoFix("fixture".to_owned())), + reason: None, } } diff --git a/crates/batten/tests/fixtures/repos/document-no-frontmatter/batten.toml.in b/crates/batten/tests/fixtures/repos/document-no-frontmatter/batten.toml.in new file mode 100644 index 000000000..4a8c9f8e6 --- /dev/null +++ b/crates/batten/tests/fixtures/repos/document-no-frontmatter/batten.toml.in @@ -0,0 +1,14 @@ +version = 1 + +# CLOUD-1787's new cause, end to end over the compiled binary. The glob names a +# markdown file and the format says so, which is the declaration that did not +# exist before: `.md` named no parser at all, so this row was refused at load. +[[rule]] +id = "skill-tier" +kind = "document" +glob = "notes.md" +format = "markdown" +node = "tier" +pattern = "2" +severity = "deny" +scope = "tree" diff --git a/crates/batten/tests/fixtures/repos/document-no-frontmatter/expected.in b/crates/batten/tests/fixtures/repos/document-no-frontmatter/expected.in new file mode 100644 index 000000000..307304813 --- /dev/null +++ b/crates/batten/tests/fixtures/repos/document-no-frontmatter/expected.in @@ -0,0 +1,4 @@ +argv: check +exit: 2 +stdout: +notes.md:1 skill-tier no-document diff --git a/crates/batten/tests/fixtures/repos/document-no-frontmatter/notes.md.in b/crates/batten/tests/fixtures/repos/document-no-frontmatter/notes.md.in new file mode 100644 index 000000000..b4331cbdf --- /dev/null +++ b/crates/batten/tests/fixtures/repos/document-no-frontmatter/notes.md.in @@ -0,0 +1,10 @@ +# notes + +No fence here, only prose — and a body that a whole-stream YAML read cannot +survive, which is the point: the verdict must not depend on it. + +| a | b | +| - | - | +| 1 | 2 | + +- [ ] a task diff --git a/crates/batten/tests/fixtures/repos/document-node-differs/expected.in b/crates/batten/tests/fixtures/repos/document-node-differs/expected.in index 8bc6bbe96..bf127357d 100644 --- a/crates/batten/tests/fixtures/repos/document-node-differs/expected.in +++ b/crates/batten/tests/fixtures/repos/document-node-differs/expected.in @@ -1,4 +1,4 @@ argv: check exit: 2 stdout: -pins.toml:1 pin-agreement +pins.toml:1 pin-agreement node-differs diff --git a/crates/batten/tests/it/document_facts.rs b/crates/batten/tests/it/document_facts.rs index d9f8266bd..c68868ea4 100644 --- a/crates/batten/tests/it/document_facts.rs +++ b/crates/batten/tests/it/document_facts.rs @@ -33,6 +33,13 @@ const WELL_FORMED: &[(Format, &str)] = &[ Format::Json5, "{\n // the pin\n pin: { rust: \"1.97.1\", },\n}", ), + // The body is deliberately the thing that breaks a whole-stream YAML read: + // a table, a task list and a block quote are the three shapes CLOUD-1787 + // measured failing. If this fixture parses, the body is not being read. + ( + Format::Markdown, + "---\npin:\n rust: \"1.97.1\"\n---\n# heading\n\n| a | b |\n| - | - |\n| 1 | 2 |\n\n- [ ] a task\n\n> **NOTE** text\n", + ), ]; /// Text that is not a document in the paired format. Each is malformed in that @@ -43,6 +50,14 @@ const MALFORMED: &[(Format, &str)] = &[ (Format::Yaml, "pin:\n\t- bad tab indent\n - and: [unclosed"), (Format::Json, "{\"pin\": {\"rust\": }"), (Format::Json5, "{ pin: { rust: \"1.97.1\" "), + // A fence that IS there and whose contents are not YAML. The other markdown + // non-answer — no fence at all — is `Look::IsNot` and belongs to + // `a_markdown_file_without_a_fence_is_is_not_and_never_could_not_look`, + // which is the distinction this format exists to make. + ( + Format::Markdown, + "---\npin:\n\t- bad tab indent\n - and: [unclosed\n---\n# heading\n", + ), ]; #[test] @@ -113,6 +128,135 @@ fn a_declared_pkl_path_answers_could_not_look_rather_than_nothing() { assert_eq!(Format::Pkl.as_str(), "pkl"); } +#[test] +fn a_markdown_file_without_a_fence_is_is_not_and_never_could_not_look() { + // CLOUD-1787's distinction, and the reason `Format::Markdown` is the one + // format whose `read` may answer `IsNot`. A well-formed markdown file with + // no frontmatter is not one the reader failed on — it read the whole thing + // and there is no document in it. Reporting that as `CouldNotLook` says the + // reader broke, and sends an author looking for a malformed fence that is + // simply absent. + // + // Fails by: collapsing the no-fence case into `CouldNotLook` (or into + // `Look::Is` of an empty map, which would be the vacuous pass). + for text in [ + "# heading\n\nprose only.\n", + // A `---` that is not on the first line is a horizontal rule. A reader + // that went looking for one anywhere would turn prose into a document. + "# heading\n---\npin:\n rust: \"1.97.1\"\n---\n", + // A leading blank line is not a fence either. + "\n---\npin:\n rust: \"1.97.1\"\n---\n", + // Opened and never closed: guessing where it ends is the same error as + // finding one mid-file. + "---\npin:\n rust: \"1.97.1\"\n\n# heading\n", + ] { + assert_eq!( + Format::Markdown.read(text), + Look::IsNot, + "markdown read a file with no frontmatter as something other than IsNot" + ); + } + assert_ne!( + Look::::IsNot, + Look::::CouldNotLook, + "the two markdown non-answers must not be the same value" + ); +} + +#[test] +fn a_markdown_body_never_reaches_a_parser() { + // THE defect (CLOUD-1787). Identical correct frontmatter; only the body + // varies, over the exact shapes measured failing when the file was read as + // one YAML stream. Every one of them must read `pin.rust` identically, + // because the body is not read at all. + // + // Fails by: handing the whole file to the YAML reader, which is what + // `format = "yaml"` over a `*.md` glob does — four of these six then answer + // `CouldNotLook` and report as a node mismatch. + const FRONTMATTER: &str = "---\npin:\n rust: \"1.97.1\"\n---\n"; + for body in [ + "prose only.\n", + "*emphasis*, **bold**, Rate: 3:1\n", + "a [[wikilink]] mid-sentence.\n", + "[[wikilink]] opening the body.\n", + "- [ ] a task\n", + "| a | b |\n| - | - |\n| 1 | 2 |\n", + "> **HEADING** text\n", + ] { + let text = format!("{FRONTMATTER}{body}"); + let Look::Is(document) = Format::Markdown.read(&text) else { + panic!("a markdown body changed whether the frontmatter parsed"); + }; + assert_eq!( + document.at("pin.rust"), + Look::Is(&Node::Text("1.97.1".to_owned())), + "a markdown body changed what the frontmatter said" + ); + } +} + +#[test] +fn an_empty_fence_is_an_empty_document_and_not_an_absent_one() { + // Present-and-empty is an ANSWER. A file whose fence is `---\n---` has + // frontmatter and it has no keys, which is a different authoring fault from + // having no fence at all — and a gate over `name:` must be able to deny the + // first rather than skip it. + // + // Fails by: routing the empty block through the YAML reader, where an empty + // stream is `CouldNotLook`, or by collapsing it into the no-fence `IsNot`. + let Look::Is(document) = Format::Markdown.read("---\n---\n# heading\n") else { + panic!("an empty fence did not read as a document"); + }; + assert_eq!(document, Node::Map(std::collections::BTreeMap::new())); + // Looked, and the key is not there — which is exactly what it should say, + // and is NOT the same answer as the file having no fence at all. + assert_eq!(document.at("name"), Look::IsNot); + assert_eq!(Format::Markdown.read("# heading\n"), Look::IsNot); +} + +#[test] +fn the_fence_is_recognised_across_the_spellings_a_consumer_actually_writes() { + // A BOM is invisible to the author, `\r\n` is what a Windows editor writes, + // and `...` is YAML's other document terminator. Refusing any of them would + // blame a file for its encoding or for reading the YAML spec. + // + // Fails by: anchoring the fence rule to `---\n` and a `\n---\n` terminator + // alone, which is what the byte scan it shares with `budget` used to do. + for text in [ + "\u{feff}---\npin:\n rust: \"1.97.1\"\n---\n# heading\n", + "---\r\npin:\r\n rust: \"1.97.1\"\r\n---\r\n# heading\r\n", + "---\npin:\n rust: \"1.97.1\"\n...\n# heading\n", + ] { + let Look::Is(document) = Format::Markdown.read(text) else { + panic!("a legitimate fence spelling did not read as a document"); + }; + assert_eq!( + document.at("pin.rust"), + Look::Is(&Node::Text("1.97.1".to_owned())) + ); + } +} + +#[test] +fn markdown_is_declarable_by_extension_and_names_no_second_parser() { + // `.md` was a load-time refusal before CLOUD-1787 ("this build has no parser + // for that extension"), so widening it breaks no config that could exist. + assert_eq!(Format::for_path("notes/entity.md"), Some(Format::Markdown)); + assert!(Format::Markdown.parseable()); + assert_eq!(Format::Markdown.as_str(), "markdown"); + // CLOUD-846's refusal stands: the frontmatter is YAML read by the one YAML + // reader, and no markdown grammar was added. A file whose fence holds YAML + // and a YAML file holding the same bytes are the same document. + let Look::Is(fenced) = Format::Markdown.read("---\npin:\n rust: \"1.97.1\"\n---\nbody\n") + else { + panic!("the fence did not parse"); + }; + let Look::Is(plain) = Format::Yaml.read("pin:\n rust: \"1.97.1\"\n") else { + panic!("the YAML did not parse"); + }; + assert_eq!(fenced, plain); +} + #[test] fn all_covers_every_format() { // The totality anchor, in `RuleKind::ALL`'s shape: the match below is @@ -120,12 +264,17 @@ fn all_covers_every_format() { let mut seen = Vec::new(); for format in Format::ALL { match format { - Format::Toml | Format::Yaml | Format::Json | Format::Json5 | Format::Pkl => { + Format::Toml + | Format::Yaml + | Format::Json + | Format::Json5 + | Format::Pkl + | Format::Markdown => { seen.push(format.as_str()); } } } - assert_eq!(seen, ["toml", "yaml", "json", "json5", "pkl"]); + assert_eq!(seen, ["toml", "yaml", "json", "json5", "pkl", "markdown"]); } #[test] diff --git a/crates/batten/tests/it/frontmatter_gates.rs b/crates/batten/tests/it/frontmatter_gates.rs new file mode 100644 index 000000000..9951f12b2 --- /dev/null +++ b/crates/batten/tests/it/frontmatter_gates.rs @@ -0,0 +1,248 @@ +//! `policy/rules-paths-trigger.rego` and `policy/skill-frontmatter-complete.rego` +//! decide over the compiled engine (CLOUD-1787). +//! +//! # Why this tier and not the modules' own rules +//! +//! Both modules' `test_` cases hand themselves a `documents` object, so they are +//! green over a shape the engine may never build — the hazard +//! `rules/policy-modules.md` names, and the reason both of its measured +//! instances were found by adding this tier rather than by reading. Here that +//! hazard is not hypothetical: until this change `Format::for_path` answered +//! `None` for `.md`, a declared markdown source reached no module at all, and +//! `with input as` would have fabricated every byte of the surface these +//! predicates read. +//! +//! Three things can only be proved against the real boundary: +//! +//! * a `.md` source reaches a module as a PARSED frontmatter node, keyed by +//! path under `input.tree.documents`; +//! * the body is not in it — a file whose prose would sink a whole-stream YAML +//! read still resolves its frontmatter, which is the defect CLOUD-1787 was +//! filed on; +//! * a file with no fence reaches `input.tree.missing` under `no-document` and +//! not under `unparsed`, so the arm that denies on it is reachable. +//! +//! # The case that carries the most +//! +//! `this_repository_is_clean_today` runs both rows over this checkout. Every +//! other fixture is a shape somebody wrote to fail; that one is the shape that +//! has to keep passing — and it is the one that was RED when this change began, +//! on `.claude/rules/policy-modules.md`, whose body asserted a `paths:` trigger +//! it did not carry. + +// Panicking on setup failure is the idiomatic way for a test to fail loudly. +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use crate::common; + +use std::fs; +use std::path::{Path, PathBuf}; + +use batten::rules::{self, Rule}; + +/// The rows as `batten.toml` declares them, deserialized rather than +/// struct-literalled: `Rule` carries `deny_unknown_fields`, so these go through +/// the same column census a consumer's config does. +fn stub_row() -> Rule { + serde_json::from_value(serde_json::json!({ + "id": "prose carry missing", + "kind": "policy", + "scope": "tree", + "sources": [".claude/rules/*.md"], + "module": "policy/rules-paths-trigger.rego", + "severity": "deny", + })) + .expect("the row batten.toml declares") +} + +fn skill_row() -> Rule { + serde_json::from_value(serde_json::json!({ + "id": "prompt declare partial", + "kind": "policy", + "scope": "tree", + "sources": ["skills/*/SKILL.md", ".claude/skills/*/SKILL.md"], + "module": "policy/skill-frontmatter-complete.rego", + "severity": "deny", + })) + .expect("the row batten.toml declares") +} + +/// The COMMITTED module, copied in rather than restated: an inline copy would +/// drift from the shipped one and pass while the real gate was broken. +fn install_module(root: &Path, module: &str) { + let source = common::at_root(module) + .canonicalize() + .expect("the committed module is where the row says it is"); + fs::create_dir_all(root.join("policy")).expect("scratch policy dir"); + fs::copy(source, root.join(module)).expect("install committed module"); +} + +fn scratch_for(name: &str, module: &str) -> PathBuf { + let root = common::scratch(&format!("frontmatter-gates-{name}")); + install_module(&root, module); + root +} + +fn findings_for(root: &Path, row: Rule, vocabulary_root: &Path) -> Vec { + let verdicts = common::verdicts_in(vocabulary_root); + rules::run_static( + &[row], + &[], + batten::policy::Vocabulary { + patterns: &[], + verdicts: &verdicts, + // The scratch tree declares no `[vocabulary]` word lists, which is + // the exemption the field documents rather than a gap in the setup. + words: None, + recorders: &[], + }, + root, + ) + .expect("the read surface runs a policy row") + .findings + .into_iter() + .map(|finding| finding.path) + .collect() +} + +/// The scratch tree's own vocabulary: registry equality runs in BOTH directions, +/// so collecting the real checkout's verdicts would refuse the load for every +/// token these two rows never emit. +fn findings(root: &Path, row: Rule) -> Vec { + findings_for(root, row, root) +} + +/// A body that a whole-stream YAML read cannot survive — the four shapes +/// CLOUD-1787 measured failing, in one file. +const HOSTILE_BODY: &str = "# heading\n\n| a | b |\n| - | - |\n| 1 | 2 |\n\n- [ ] a task\n\n> **NOTE** text\n\n[[wikilink]] opening a line.\n"; + +#[test] +fn a_rules_stub_with_a_paths_trigger_is_clean() { + // The engine half: a `.md` under a declared glob is PARSED, its frontmatter + // becomes the node, and the body — which would sink a YAML read of the whole + // file — does not reach the parser at all. + // + // Fails by: `Format::for_path` answering `None` for `.md` again, which puts + // the path in `missing` under `unknown-format` and reddens this case. + let root = scratch_for("stub-clean", "policy/rules-paths-trigger.rego"); + common::write( + &root, + ".claude/rules/rust.md", + &format!("---\npaths:\n - \"crates/**/*.rs\"\n---\n\n{HOSTILE_BODY}"), + ); + assert_eq!( + findings(&root, stub_row()), + Vec::::new(), + "a stub carrying a trigger is clean, whatever its prose says" + ); +} + +#[test] +fn a_rules_stub_without_a_paths_trigger_is_refused() { + // THE MEASURED CASE, reproduced. A stub with no fence at all: the file is + // read in full, there is no document in it, and the module denies on + // `no-document` rather than abstaining. + // + // Fails by: routing the no-fence case to `unparsed` (which this module reads + // as could-not-look and reports under the other verdict), or to an empty + // document (which would satisfy nothing and deny nothing). + let root = scratch_for("stub-no-fence", "policy/rules-paths-trigger.rego"); + common::write( + &root, + ".claude/rules/policy-modules.md", + "# Moved to `rules/policy-modules.md`\n\nthe frontmatter above is the trigger.\n", + ); + assert_eq!( + findings(&root, stub_row()), + vec![".claude/rules/policy-modules.md".to_owned()], + "a stub asserting a trigger it does not carry is refused" + ); +} + +#[test] +fn a_rules_stub_with_an_empty_trigger_is_refused() { + // Present-and-empty reaches the module as a DOCUMENT rather than through + // `missing`, which is the distinction the empty-fence arm exists for. + let root = scratch_for("stub-empty", "policy/rules-paths-trigger.rego"); + common::write(&root, ".claude/rules/x.md", "---\npaths: []\n---\n\nbody\n"); + assert_eq!( + findings(&root, stub_row()), + vec![".claude/rules/x.md".to_owned()], + "a trigger that fires on nothing is refused" + ); +} + +#[test] +fn a_skill_missing_a_declared_field_is_refused() { + let root = scratch_for("skill-missing", "policy/skill-frontmatter-complete.rego"); + common::write( + &root, + "skills/batten/SKILL.md", + &format!("---\nname: batten\n---\n\n{HOSTILE_BODY}"), + ); + assert_eq!( + findings(&root, skill_row()), + vec!["skills/batten/SKILL.md".to_owned()], + "a skill with no description is refused" + ); +} + +#[test] +fn a_complete_skill_is_clean_and_its_body_is_not_read() { + let root = scratch_for("skill-clean", "policy/skill-frontmatter-complete.rego"); + common::write( + &root, + "skills/batten/SKILL.md", + &format!("---\nname: batten\ndescription: what it does\n---\n\n{HOSTILE_BODY}"), + ); + assert_eq!( + findings(&root, skill_row()), + Vec::::new(), + "a complete skill is clean, and its table-bearing body changed nothing" + ); +} + +#[test] +fn a_skill_whose_name_disagrees_with_its_directory_is_refused() { + let root = scratch_for("skill-misplaced", "policy/skill-frontmatter-complete.rego"); + common::write( + &root, + "skills/batten/SKILL.md", + "---\nname: serena\ndescription: d\n---\n\nbody\n", + ); + assert_eq!( + findings(&root, skill_row()), + vec!["skills/batten/SKILL.md".to_owned()], + "a skill routed by one identity and displayed under another is refused" + ); +} + +#[test] +fn this_repository_is_clean_today() { + // The case that carries the most, and the one the change had to MAKE pass: + // `.claude/rules/policy-modules.md` was the single stub without a trigger, + // and nothing in this tree could see that until frontmatter was parseable. + // + // Fails by: any stub losing its `paths:` block, or any `SKILL.md` losing a + // field or being renamed out of agreement with its directory. + let root = common::at_root(".") + .canonicalize() + .expect("this checkout is where the manifest says it is"); + // The vocabulary comes from a directory holding only the module under test, + // for the reason `findings` states: registry equality runs in BOTH + // directions, so the real checkout's verdicts would refuse the load for + // every token these two rows never raise. + for (row, module) in [ + (stub_row(), "policy/rules-paths-trigger.rego"), + (skill_row(), "policy/skill-frontmatter-complete.rego"), + ] { + let id = row.id.clone(); + let only = common::scratch(&format!("frontmatter-gates-vocabulary-{id}")); + install_module(&only, module); + assert_eq!( + findings_for(&root, row, &only), + Vec::::new(), + "{id} is clean over this checkout" + ); + } +} diff --git a/crates/batten/tests/it/main.rs b/crates/batten/tests/it/main.rs index 1c438fb33..38e36e183 100644 --- a/crates/batten/tests/it/main.rs +++ b/crates/batten/tests/it/main.rs @@ -128,6 +128,7 @@ mod fixture_forks; mod fixture_repos; mod forced_push; mod forge_facts; +mod frontmatter_gates; mod fuzz_corpus; mod gh_guard; mod git_facts; diff --git a/crates/batten/tests/it/policy_tree.rs b/crates/batten/tests/it/policy_tree.rs index b4284f361..887aa9808 100644 --- a/crates/batten/tests/it/policy_tree.rs +++ b/crates/batten/tests/it/policy_tree.rs @@ -543,9 +543,15 @@ violation contains {"rule": "reads-real-keys", "verdict": "fixture z probe"} if /// /// An extension this build has no parser for was checked before any I/O and /// dropped into `missing`, so the rule skipped silently — a migrated gate could -/// go dead by declaring `CLAUDE.md` or a `.bats` suite, with the file never -/// opened. No state of the filesystem fixes that, which is what makes it a -/// config error rather than a could-not-look. +/// go dead by declaring a `.bats` suite, with the file never opened. No state of +/// the filesystem fixes that, which is what makes it a config error rather than a +/// could-not-look. +/// +/// THE FIXTURE WAS `CLAUDE.md` AND IS A `.bats` SUITE (CLOUD-1787). `.md` names +/// `Format::Markdown` now, so it is parseable and this case's own subject moved +/// out from under it. The claim is unchanged — an extension with no parser is a +/// config fault, decided before any I/O — so the extension is swapped rather than +/// the case retired. Its doc named `.bats` as the other example all along. /// /// Asserted through `run_static`, the surface a consumer actually reaches, so /// the case covers the refusal REACHING them rather than a private helper @@ -558,10 +564,10 @@ fn a_document_with_no_parser_is_refused_rather_than_skipped() { // not the tree, so this cannot pass by accident as an absent-file report. // The absent half is asserted by its own case below, because the two are // different claims and only together do they pin the precedence. - fs::write(root.join("CLAUDE.md"), "# prose\n").expect("fixture"); + fs::write(root.join("suite.bats"), "@test \"x\" { true; }\n").expect("fixture"); let err = rules::run_static( - &[tree_row("repo-policy", "policy/", &["CLAUDE.md"])], + &[tree_row("repo-policy", "policy/", &["suite.bats"])], &[], fixtures(&root), &root, @@ -569,7 +575,7 @@ fn a_document_with_no_parser_is_refused_rather_than_skipped() { .expect_err("a declared document this build cannot parse is a config fault"); let message = format!("{err}"); assert!( - message.contains("CLAUDE.md"), + message.contains("suite.bats"), "the refusal names the path: {message}" ); assert!( @@ -578,7 +584,7 @@ fn a_document_with_no_parser_is_refused_rather_than_skipped() { be the silent skip this splits apart: {message}" ); assert!( - !message.contains("# prose"), + !message.contains("@test"), "pointer-only: the file is not even opened, let alone quoted: {message}" ); } @@ -608,17 +614,17 @@ fn a_document_with_a_parser_still_evaluates() { /// `missing`. /// /// Without this, a regression that reordered the checks — testing the tree -/// before the extension — would classify a declared `.md` the tree lacks as a -/// could-not-look, which is the silent skip the split exists to remove, wearing -/// the other cause's name. +/// before the extension — would classify a declared `.bats` path the tree lacks +/// as a could-not-look, which is the silent skip the split exists to remove, +/// wearing the other cause's name. #[test] fn an_absent_unsupported_document_is_still_a_parser_fault() { let root = scratch("no-parser-absent"); write_bundle(&root, NO_STRAY); - // `CLAUDE.md` is deliberately NOT created. + // `suite.bats` is deliberately NOT created. let err = rules::run_static( - &[tree_row("repo-policy", "policy/", &["CLAUDE.md"])], + &[tree_row("repo-policy", "policy/", &["suite.bats"])], &[], fixtures(&root), &root, @@ -626,7 +632,7 @@ fn an_absent_unsupported_document_is_still_a_parser_fault() { .expect_err("the extension is decided before the tree is consulted"); let message = format!("{err}"); assert!( - message.contains("CLAUDE.md"), + message.contains("suite.bats"), "the refusal names the path: {message}" ); assert!( diff --git a/crates/batten/tests/it/rule_cost_census.rs b/crates/batten/tests/it/rule_cost_census.rs index ade915e78..b399b209a 100644 --- a/crates/batten/tests/it/rule_cost_census.rs +++ b/crates/batten/tests/it/rule_cost_census.rs @@ -208,3 +208,155 @@ fn the_census_describes_the_last_run_rather_than_accumulating() { let _ = fs::remove_dir_all(&root); } + +/// A `policy` row over `sources`, with a module that declares a predicate and +/// never violates. +/// +/// Never violating is the point: this case is about what the rule READ, and a +/// finding would drag the verdict registry in for no gain — `vocabulary()` above +/// is empty by design, and a module raising a token nothing declares fails the +/// load. +fn policy_row(id: &str, sources: &[&str]) -> Rule { + serde_json::from_value(serde_json::json!({ + "id": id, + "kind": "policy", + "scope": "tree", + "module": "policy/census-probe.rego", + "sources": sources, + "severity": "deny", + })) + .expect("a tree-scoped policy row the loader accepts") +} + +fn install_probe(root: &Path) { + fs::write( + root.join("policy/census-probe.rego"), + "package batten.census_probe\n\nimport rego.v1\n\nrules contains \"census-probe\"\n", + ) + .expect("probe module"); +} + +/// Write `count` parseable documents of known, distinct sizes; return the total. +/// +/// TOML rather than the `.txt` [`seed`] writes, and the difference is load-bearing +/// here: `.txt` names no `Format`, so a `sources` glob over it resolves to +/// `UnknownFormat` before any I/O and would measure the case below instead of +/// this one. +fn seed_documents(root: &Path, count: usize) -> usize { + (0..count) + .map(|i| { + let body = format!("key = {}\n", "9".repeat(i + 1)); + fs::write(root.join(format!("d{i}.toml")), &body).expect("fixture"); + body.len() + }) + .sum() +} + +#[test] +fn the_shared_acquisition_reports_the_documents_it_read() { + // CLOUD-1790. `acquire` is the one function that acquires a document and it + // counted nothing, so these reads were counted NOWHERE — not in the global + // counters, and therefore not in any census row. + // + // THE ROW IS THE STEP, NOT A RULE, and that is the correction this case + // encodes. Declared documents are acquired ONCE for the whole run, before + // the per-rule loop, so a `policy` row's reads fall outside its own + // measurement window by construction: every such row reported + // `0 file(s) 0 byte(s)`, the run total summed those zeros, and a reader + // comparing that against a `forbid` row's real numbers concluded the policy + // row had received no input. Charging the shared read to whichever row the + // loop reached first would be the other error — the cache is shared on + // purpose, so the cost is the step's. + // + // Fails by: dropping the `count_read` call in `acquire` (both counts go to + // zero while the documents are still read and parsed), or by dropping the + // census row (the step disappears and the reads are unattributed again). + let root = scratch("declared-acquisition"); + fs::create_dir_all(root.join("policy")).expect("scratch policy dir"); + install_probe(&root); + let bytes = seed_documents(&root, 3); + + rules::run_static( + &[policy_row("reads-three-documents", &["**/*.toml"])], + &[], + vocabulary(), + &root, + ) + .expect("the read surface runs the row"); + + let costs = rules::rule_costs(); + let shared = costs + .iter() + .find(|cost| cost.rule == "(declared documents)") + .expect("the shared acquisition earns its own census row"); + assert_eq!( + shared.files_read, 3, + "the shared step reports one read per document the rule set declared" + ); + assert_eq!( + shared.bytes_read, bytes, + "and the bytes those reads returned, not a placeholder" + ); + + let _ = fs::remove_dir_all(&root); +} + +#[test] +fn a_rule_set_declaring_no_documents_earns_no_acquisition_row() { + // The row appears only when there is a step to report. Without this, every + // run of `forbid` rows would grow a zero row for work it never did — which is + // the same "evaluated, and free" misreading the per-rule census already + // refuses one level up. + // + // Fails by: pushing the acquisition row unconditionally. + let root = scratch("no-acquisition"); + seed(&root, 2); + + rules::run_static(&[row("reads-the-txt", "*.txt")], &[], vocabulary(), &root) + .expect("the read surface runs the row"); + + let costs = rules::rule_costs(); + assert!( + !costs.iter().any(|cost| cost.rule == "(declared documents)"), + "a rule set declaring no documents acquires nothing and reports no step" + ); + + let _ = fs::remove_dir_all(&root); +} + +#[test] +fn a_row_refused_before_any_io_reports_nothing_read() { + // The other side of the fix, and the one that keeps it honest. An extension + // this build has no parser for is `UnknownFormat`, decided BEFORE the file is + // opened — so it must still report zero. Counting it would turn a config + // fault that costs nothing into a charged read, which is the same class of + // lie in the opposite direction. + // + // The fixture files EXIST, so a passing assertion here is about the + // declaration rather than about an absent path. + // + // Fails by: moving the `count_read` call above the `UnknownFormat` early + // return in `acquire`. + let root = scratch("policy-unknown-format"); + fs::create_dir_all(root.join("policy")).expect("scratch policy dir"); + install_probe(&root); + seed(&root, 3); + + rules::run_static( + &[policy_row("reads-nothing-parseable", &["**/*.txt"])], + &[], + vocabulary(), + &root, + ) + .expect("the read surface runs the row"); + + let costs = rules::rule_costs(); + let cost = costs.first().expect("the row has a census entry"); + assert_eq!( + (cost.files_read, cost.bytes_read), + (0, 0), + "a declaration decided before any I/O is not charged for a read it never made" + ); + + let _ = fs::remove_dir_all(&root); +} diff --git a/mise.toml b/mise.toml index 0cc7a1044..048ebf874 100644 --- a/mise.toml +++ b/mise.toml @@ -617,7 +617,7 @@ CI_FANIN_WORKFLOW = ".github/workflows/ci.yml" BATS_TEST_TIMEOUT = "300" REGORUS_OPA_COMPLIANCE = "1.2.0" REGORUS_OPA_COMPLIANCE_FOR = "0.11" -MUTANT_GATES = "mise,attestation-check,engine-checks-green,engine-config,engine-doctor,engine-landed,engine-perf,engine-mcp,engine-pinned,engine-ready,engine-verdict,engine-wiring,engine-surface,agentic-experiment-record,awk-regex-check,bats-invocation,batten-glob-check,board-diff-overlap,board-payloads,board-sweep,branch-age-check,cap-drift,cfg-gated-test,ci-cache-declared,ci-hygiene,ci-parity,ci-slow-needed,ci-suite-lane,ci-tools-check,claim-before-code,claim-order-is-stated,claimed-keys,closing-key-check,coderabbit-config-check,commit-hygiene,connector-allow-guard,connector-allow-resolve,container-preflight,darwin-link,deferral-check,denials-outlive-the-turn,digest-major-agreement,doctor,done-check,done-pr-check,duplicate-close-check,egress-fencing,evaluator-closure-check,evaluator-io-check,filed-here,finding-sink-check,fixture-forks,forge-verdict-required,graph-check,harness-grant,harness-wiring,hk-fix-selection,hk-plan-required,hook-pin-check,hook-skip-local,in-progress-drain,install-check,land-divergence-assert,landed-check,landing-loop,landing-roster-guarded,leased-push,license-table-check,linear-check,lock-complete,macos-link-check,mcp-allow-check,mcp-attach-check,mcp-timeout-budget,merged-pr-keys,mise-action-floor,mise-pin-agreement,module-map-check,msrv-pin-agreement,mutation-declared-case,no-doctests,nonverdict-assert,ntia-check,obligations-bound,perf-assert,pinned-toolchain,pipefail-grep-check,plan-complete,pr-partition-restated,pr-unsubscribed,privileged-lane,prose-only,publish-credential-check,ready-cites-check,ready-lint,reclaim-census,release-assets-check,release-due,release-provision-parity,release-tag-shape,release-tracking-check,released,remedy-authorship,repetition-without-progress,report-only-check,review-answered,review-dispatched,run-shape,rust-paths-check,sbom,sbom-inventory,serena-mcp,shell-hygiene,shell-retirement,shell-write-advisory,signing-posture,sonar-gate,spec-ref-check,stop-posture,stop-posture-check,suite-bench-check,suite-subject-retirable,task-substitution,test-targets,timeout-check,token-bench-check,transcript-corpus-check,tree-clean,trunk-based,validator-verdict-clean,verdict-routes-resolve,weakens-declared,worktree-registration,spawn-widening,nextest-slow,engine-lease,engine-handler,engine-speculation,engine-pipeline,engine-policy" +MUTANT_GATES = "mise,attestation-check,engine-checks-green,engine-config,engine-doctor,engine-landed,engine-perf,engine-mcp,engine-pinned,engine-ready,engine-verdict,engine-wiring,engine-surface,agentic-experiment-record,awk-regex-check,bats-invocation,batten-glob-check,board-diff-overlap,board-payloads,board-sweep,branch-age-check,cap-drift,cfg-gated-test,ci-cache-declared,ci-hygiene,ci-parity,ci-slow-needed,ci-suite-lane,ci-tools-check,claim-before-code,claim-order-is-stated,claimed-keys,closing-key-check,coderabbit-config-check,commit-hygiene,connector-allow-guard,connector-allow-resolve,container-preflight,darwin-link,deferral-check,denials-outlive-the-turn,digest-major-agreement,doctor,done-check,done-pr-check,duplicate-close-check,egress-fencing,evaluator-closure-check,evaluator-io-check,filed-here,finding-sink-check,fixture-forks,forge-verdict-required,graph-check,harness-grant,harness-wiring,hk-fix-selection,hk-plan-required,hook-pin-check,hook-skip-local,in-progress-drain,install-check,land-divergence-assert,landed-check,landing-loop,landing-roster-guarded,leased-push,license-table-check,linear-check,lock-complete,macos-link-check,mcp-allow-check,mcp-attach-check,mcp-timeout-budget,merged-pr-keys,mise-action-floor,mise-pin-agreement,module-map-check,msrv-pin-agreement,mutation-declared-case,no-doctests,nonverdict-assert,ntia-check,obligations-bound,perf-assert,pinned-toolchain,pipefail-grep-check,plan-complete,pr-partition-restated,pr-unsubscribed,privileged-lane,prose-only,publish-credential-check,ready-cites-check,ready-lint,reclaim-census,release-assets-check,release-due,release-provision-parity,release-tag-shape,release-tracking-check,released,remedy-authorship,repetition-without-progress,report-only-check,review-answered,review-dispatched,run-shape,rust-paths-check,sbom,sbom-inventory,serena-mcp,shell-hygiene,shell-retirement,shell-write-advisory,signing-posture,sonar-gate,spec-ref-check,stop-posture,stop-posture-check,suite-bench-check,suite-subject-retirable,task-substitution,test-targets,timeout-check,token-bench-check,transcript-corpus-check,tree-clean,trunk-based,validator-verdict-clean,verdict-routes-resolve,weakens-declared,worktree-registration,spawn-widening,nextest-slow,engine-lease,engine-handler,engine-speculation,engine-pipeline,engine-policy,rules-paths-trigger,skill-frontmatter-complete" # --- GitHub reachability behind an egress proxy (Claude Code web sandbox etc.) --- # mise resolves every tool's release through GitHub's *API* host, api.github.com. diff --git a/policy/rules-paths-trigger.rego b/policy/rules-paths-trigger.rego new file mode 100644 index 000000000..457b24f9f --- /dev/null +++ b/policy/rules-paths-trigger.rego @@ -0,0 +1,143 @@ +# METADATA +# description: | +# Every rule stub in the vendor-specific directory carries the loading trigger +# that is its only reason to exist. +# +# THE FINDING THAT MOTIVATED IT (CLOUD-1787, CLOUD-1152). This repository's +# doctrine lives at the root, neutral, because five of the six harnesses +# Batten adjudicates for cannot read a vendor directory. What stays in the +# vendor directory is a stub, and the stub is kept for ONE thing: the +# frontmatter trigger, which is a loading mechanism no neutral location has. +# Its own body says so — "deleting this file would cost that trigger." +# +# One of those stubs never had the trigger. Not lost in an edit: absent from +# the commit that wrote the claim, and absent every day since, while the file +# asserted otherwise in prose and four siblings carried it correctly. Nothing +# could see that, because nothing could read frontmatter — a stub with no +# trigger and a stub with one are byte-identical to every gate this repository +# had. That is the whole class: a claim about the tree that the tree refutes, +# surviving because the mechanism to check it did not exist. +# +# THE COULD-NOT-LOOK ARM DENIES HERE RATHER THAN ABSTAINING, and that is the +# arm the live finding actually exercises. `no-document` means the file was +# read in full and has no frontmatter at all, which for this rule is not an +# inability to judge — it IS the violation, stated by the acquisition layer +# instead of by this module. Reporting it as could-not-look would be the +# vacuous pass arriving through the honest channel. +# +# THE PREDICATE IS THIS CONSUMER'S. That a rule stub declares `paths:` is +# Claude Code's convention and this repository's use of it; the engine supplies +# a parsed frontmatter node and knows nothing about either. +# +# THIS BLOCK IS YAML AND MUST STAY THE LAST COMMENT BLOCK BEFORE `package`. +# schemas: +# - input: schema["policy-input.schema"] +package batten.rules_paths_trigger + +import rego.v1 + +rules contains "prose carry missing" + +# The fence is there and carries no `paths:`, or carries an empty one. +# +# Empty is a violation and not a skip: a stub declaring `paths: []` fires on +# nothing, which is the same outcome as having no trigger and reached by a +# different authoring mistake. The acquisition layer keeps the two apart — +# an empty FENCE is a document with no keys and lands here, a missing fence +# lands in `missing` below — so both are decided rather than one silently +# standing in for the other. +violation contains { + "rule": "prose carry missing", + "verdict": "prose declare missing", + "subjects": [{"path": path}], +} if { + some path, document in input.tree.documents + not triggers_on_something(document) +} + +triggers_on_something(document) if { + count(document.paths) > 0 +} + +# No frontmatter at all. Its own verdict rather than folded into the arm above, +# because the remedy differs: one says fix the trigger you wrote, this says you +# wrote none. +violation contains { + "rule": "prose carry missing", + "verdict": "prose declare missing", + "subjects": [{"path": path}], +} if { + some path, cause in input.tree.missing + cause == "no-document" +} + +# Read and refused. A stub whose fence will not parse is one this module could +# not judge, and a refusal is the only honest answer: iterating only the files +# that parsed reports green over the one it never saw. +violation contains { + "rule": "prose carry missing", + "verdict": "prose read unread", + "subjects": [{"path": path}], +} if { + some path, cause in input.tree.missing + cause in {"unparsed", "unreadable", "unknown-format"} +} + +# --- the load-time tier ------------------------------------------------------ +# +# These pin the PREDICATE. They cannot pin that the ENGINE parses a markdown +# file's frontmatter at all — a `with input as` case fabricates the very shape +# the engine may be unable to produce (CLOUD-845, CLOUD-857), and here that +# shape is the whole new thing: `input.tree.documents` holding a node read out +# of a `.md`, and `missing` carrying `no-document` rather than `unparsed`. +# `crates/batten/tests/it/frontmatter_gates.rs` is that tier, over the compiled +# binary and a real tree. + +stub(paths) := {"tree": { + "documents": {".claude/rules/x.md": {"paths": paths}}, + "missing": {}, +}} + +unread(cause) := {"tree": { + "documents": {}, + "missing": {".claude/rules/x.md": cause}, +}} + +test_a_stub_with_a_trigger_is_clean if { + count(violation) == 0 with input as stub(["crates/**/*.rs"]) +} + +test_a_stub_with_an_empty_trigger_is_refused if { + some v in violation with input as stub([]) + v.verdict == "prose declare missing" +} + +test_a_stub_with_no_trigger_key_is_refused if { + some v in violation with input as {"tree": { + "documents": {".claude/rules/x.md": {"title": "x"}}, + "missing": {}, + }} + v.verdict == "prose declare missing" +} + +# THE MEASURED CASE. A stub with no fence at all reaches the module through +# `missing`, under its own cause, and must deny rather than abstain — the file +# exists, it was read in full, and the trigger it claims to carry is not there. +test_a_stub_with_no_frontmatter_is_refused_and_not_merely_unread if { + some v in violation with input as unread("no-document") + v.verdict == "prose declare missing" +} + +# And the two causes do not collapse: a fence that will not parse is a different +# verdict, because the author's next move is different. +test_a_stub_whose_fence_will_not_parse_is_unread if { + some v in violation with input as unread("unparsed") + v.verdict == "prose read unread" +} + +test_no_stubs_at_all_is_clean if { + count(violation) == 0 with input as {"tree": {"documents": {}, "missing": {}}} +} + +#MUTANT-SUITE crates/batten/tests/it/frontmatter_gates.rs +#MUTANT trigger-absent-unread|s@\tnot triggers_on_something(document)$@\tfalse@|a_rules_stub_without_a_paths_trigger_is_refused diff --git a/policy/skill-frontmatter-complete.rego b/policy/skill-frontmatter-complete.rego new file mode 100644 index 000000000..9e6799595 --- /dev/null +++ b/policy/skill-frontmatter-complete.rego @@ -0,0 +1,140 @@ +# METADATA +# description: | +# Every shipped skill declares the two fields a harness needs to load it, and +# declares a name that matches where it lives. +# +# WHY A GATE AND NOT A CONVENTION (CLOUD-1787). A skill's `name` and +# `description` are the entire basis on which a harness decides whether to +# load it. A skill missing either does not fail loudly — it is simply never +# selected, which is indistinguishable from a skill that was selected and had +# nothing to say. That is the same silence class this repository's whole fact +# model is built against, one layer out, and until frontmatter was parseable +# nothing here could read the fields to check them. +# +# `name` MUST EQUAL THE DIRECTORY, which is the clause worth stating. The two +# are separate authorities today: a harness routes by directory and displays +# by `name`, so a skill whose fields disagree with its path is invoked under +# one identity and reads under another. Asserting agreement collapses them to +# one without either side having to stop existing. +# +# THE PREDICATE IS THIS CONSUMER'S. `SKILL.md`, `name` and `description` are +# an agent harness's vocabulary and this repository's layout; the engine +# supplies a parsed frontmatter node keyed by path and knows neither. +# +# THIS BLOCK IS YAML AND MUST STAY THE LAST COMMENT BLOCK BEFORE `package`. +# schemas: +# - input: schema["policy-input.schema"] +package batten.skill_frontmatter_complete + +import rego.v1 + +rules contains "prompt declare partial" + +# A field is absent, or present and blank. +# +# Blank counts, and the distinction is load-bearing: `name:` with nothing after +# it parses to a null and `name: ""` parses to an empty string, so a predicate +# testing only for the KEY passes both — a skill that declares its fields and +# fills in neither. +violation contains { + "rule": "prompt declare partial", + "verdict": "prompt declare missing", + "subjects": [{"path": path}], +} if { + some path, document in input.tree.documents + some field in ["name", "description"] + not stated(document[field]) +} + +stated(value) if { + is_string(value) + trim_space(value) != "" +} + +# The declared name and the directory it ships in disagree. +violation contains { + "rule": "prompt declare partial", + "verdict": "prompt name wrong", + "subjects": [{"path": path}], +} if { + some path, document in input.tree.documents + stated(document.name) + document.name != directory_of(path) +} + +# `skills/batten/SKILL.md` -> `batten`. The second-to-last segment, which is the +# skill's own directory whatever it is nested under. +directory_of(path) := segment if { + segments := split(path, "/") + count(segments) > 1 + segment := segments[count(segments) - 2] +} + +# Read and refused, or carrying no frontmatter at all. Both are a skill this +# module could not judge, and both refuse rather than report clean: a `SKILL.md` +# with no fence declares no fields, which is the violation above arriving +# through the acquisition layer instead of through the document. +violation contains { + "rule": "prompt declare partial", + "verdict": "prompt read unread", + "subjects": [{"path": path}], +} if { + some path, _ in input.tree.missing +} + +# --- the load-time tier ------------------------------------------------------ +# +# These pin the PREDICATE. The ENGINE half — that a `SKILL.md` reaches +# `input.tree.documents` as a parsed frontmatter node at all — is +# `crates/batten/tests/it/frontmatter_gates.rs`, over the compiled binary. + +skill(fields) := {"tree": { + "documents": {"skills/batten/SKILL.md": fields}, + "missing": {}, +}} + +test_a_complete_skill_is_clean if { + count(violation) == 0 with input as skill({"name": "batten", "description": "what it does"}) +} + +test_a_skill_missing_its_description_is_refused if { + some v in violation with input as skill({"name": "batten"}) + v.verdict == "prompt declare missing" +} + +# Blank is absent. `name:` with nothing after it parses to a null and `name: ""` +# to an empty string, and a predicate testing only for the KEY passes both. +test_a_blank_field_is_refused_like_an_absent_one if { + some v in violation with input as skill({"name": " ", "description": "d"}) + v.verdict == "prompt declare missing" +} + +test_a_null_field_is_refused_like_an_absent_one if { + some v in violation with input as skill({"name": null, "description": "d"}) + v.verdict == "prompt declare missing" +} + +test_a_name_disagreeing_with_its_directory_is_refused if { + some v in violation with input as skill({"name": "serena", "description": "d"}) + v.verdict == "prompt name wrong" +} + +# A skill nested deeper still resolves to its OWN directory rather than the tree +# it hangs under, which is what makes the same rule cover both roots. +test_a_nested_skill_resolves_its_own_directory if { + count(violation) == 0 with input as {"tree": { + "documents": {".claude/skills/batten/SKILL.md": {"name": "batten", "description": "d"}}, + "missing": {}, + }} +} + +test_a_skill_that_could_not_be_read_is_refused if { + some v in violation with input as {"tree": { + "documents": {}, + "missing": {"skills/batten/SKILL.md": "no-document"}, + }} + v.verdict == "prompt read unread" +} + +#MUTANT-SUITE crates/batten/tests/it/frontmatter_gates.rs +#MUTANT blank-field-admitted|s@\tnot stated(document\[field\])$@\tfalse@|a_skill_missing_a_declared_field_is_refused diff --git a/schema/batten.local.schema.json b/schema/batten.local.schema.json index eceaafe20..a07c9bbc1 100644 --- a/schema/batten.local.schema.json +++ b/schema/batten.local.schema.json @@ -389,6 +389,11 @@ "description": "PKL. Declarable, never parsed — see the type's own note.", "type": "string", "const": "pkl" + }, + { + "description": "Markdown, read as its YAML **frontmatter** and never as its prose\n(CLOUD-1787).\n\nStill a format and not an artifact: markdown is a markup language, the\nsame kind of name as TOML, and which paths carry it stays the consumer's\n`batten.toml`.\n\n**Not a fifth parser**, which CLOUD-846 refused and this keeps refused.\nThe body is never handed to anything — headings and tables stay line\npredicates. The fence's contents are YAML, parsed by the one YAML\nauthority this type already carries, so nothing here knows a second\ngrammar.", + "type": "string", + "const": "markdown" } ] }, diff --git a/schema/batten.schema.json b/schema/batten.schema.json index 6aadd4b81..90e873bd7 100644 --- a/schema/batten.schema.json +++ b/schema/batten.schema.json @@ -1858,6 +1858,11 @@ "description": "PKL. Declarable, never parsed — see the type's own note.", "type": "string", "const": "pkl" + }, + { + "description": "Markdown, read as its YAML **frontmatter** and never as its prose\n(CLOUD-1787).\n\nStill a format and not an artifact: markdown is a markup language, the\nsame kind of name as TOML, and which paths carry it stays the consumer's\n`batten.toml`.\n\n**Not a fifth parser**, which CLOUD-846 refused and this keeps refused.\nThe body is never handed to anything — headings and tables stay line\npredicates. The fence's contents are YAML, parsed by the one YAML\nauthority this type already carries, so nothing here knows a second\ngrammar.", + "type": "string", + "const": "markdown" } ] }, diff --git a/schema/policy-input.schema.json b/schema/policy-input.schema.json index c8d8ff8ac..97a767f93 100644 --- a/schema/policy-input.schema.json +++ b/schema/policy-input.schema.json @@ -358,7 +358,7 @@ "additionalProperties": { "type": "string" }, - "description": "Could-not-look, and NOT a Fact: a declared name the engine could not acquire, mapped to WHY (CLOUD-1309). Distinct from an empty result, which is the distinction that keeps a vacuous pass out (CLOUD-251, CLOUD-845). The cause is one of `absent`, `unparsed`, `unreadable`, `unknown-format` or `root-unset` -- `NotAcquired::as_str`'s own tokens -- and carrying it is what makes `rules/policy-modules.md`'s \"the two causes stay distinct and a module may rely on that\" true of this surface rather than only of the Rust enum. A module keys by name: `input.tree.missing[path] == \"unparsed\"`. It was an array of bare names until CLOUD-1309, so a predicate could see THAT a source failed and never WHY, and firing on membership alone reddened every consumer that declares an optional source.", + "description": "Could-not-look, and NOT a Fact: a declared name the engine could not acquire, mapped to WHY (CLOUD-1309). Distinct from an empty result, which is the distinction that keeps a vacuous pass out (CLOUD-251, CLOUD-845). The cause is one of `absent`, `unparsed`, `unreadable`, `unknown-format`, `root-unset` or `no-document` -- `NotAcquired::as_str`'s own tokens -- and carrying it is what makes `rules/policy-modules.md`'s \"the two causes stay distinct and a module may rely on that\" true of this surface rather than only of the Rust enum. A module keys by name: `input.tree.missing[path] == \"unparsed\"`. It was an array of bare names until CLOUD-1309, so a predicate could see THAT a source failed and never WHY, and firing on membership alone reddened every consumer that declares an optional source.", "type": "object" }, "plan": {