From 174967142adc2a6219d9f5edc13baea25095fa18 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Fri, 11 Sep 2026 07:37:19 +0000 Subject: [PATCH 1/4] feat(facts): read a markdown file's frontmatter, and say which non-answer it is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Format` had no markdown member, so the only route to a markdown file's YAML frontmatter was `format = "yaml"` over a `*.md` glob. That appears to work: `---` is YAML's document-start marker and the reader takes the first document of the stream, so the frontmatter parses. 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, a block quote and a leading `[[wikilink]]` failed. The verdict was decided by the prose. `Format::Markdown` reads the fence and never hands the body to a parser. It is not a fifth parser and CLOUD-846's refusal stands: the fence holds YAML, read by `read_yaml`, which `Format::Yaml` now also calls so there is exactly one YAML reader. The fence rule itself is shared with `budget::strip_frontmatter`, which had hand-rolled the same scan — sharing it widens that side to a BOM, CRLF and a `...` terminator, all of which the loader drops and so were over-taxed. Markdown is the first format for which `Look::IsNot` is meaningful, and the contract saying `read` never returns it is amended rather than worked around. A well-formed markdown file with no fence is not one the reader failed on: it read the file completely and there is no document in it. `parse_node` stops collapsing `IsNot` into `Unparsed` and maps it to `NotAcquired::NoDocument`, so "there is no fence" and "this fence is malformed" are different causes with opposite remedies. An empty fence is a third answer again — present, and empty — which is what lets a gate over `name:` deny a blank skill rather than skip it. Findings now carry the reason their kind computed. A `document` row had four answers reaching only the fingerprint, so `node-differs` and `could-not-look` printed the same `path:1 rule-id` and sent an author looking at a value that was correct. `reason` is a closed vocabulary of engine-owned tokens, skipped when absent, so every other kind serialises unchanged. A `document` row whose declared `format` contradicts the extension its glob ends in is refused at load, naming the remedy: the trap above stayed admissible otherwise, since `format` is declared and nothing compared it to the paths it would meet. Two gates ship with the parser, because a mechanism with no consumer has not been shown to decide anything. `rules-paths-trigger` asserts every rule stub carries the `paths:` frontmatter that is its only reason to exist; `skill-frontmatter-complete` asserts every `SKILL.md` declares a usable `name` and `description` and lives in the directory it names. The first one found a defect on its first run. `.claude/rules/policy-modules.md` had never carried the trigger — absent from the commit that wrote the claim, and absent every day since, while the file asserted otherwise in prose and its four siblings carried it correctly. CLOUD-1152 gives that trigger as the file's entire reason to exist. Nothing could see it, because a stub with a trigger and a stub without one were byte-identical to every gate this repository had. Refs: CLOUD-1787, CLOUD-846, CLOUD-772, CLOUD-757, CLOUD-1152 --- .claude/rules/policy-modules.md | 6 + batten.toml | 106 +++++++++ crates/batten/src/baseline.rs | 2 + crates/batten/src/budget.rs | 35 ++- crates/batten/src/defects.rs | 1 + crates/batten/src/design.rs | 1 + crates/batten/src/facts.rs | 166 +++++++++++-- crates/batten/src/findings.rs | 1 + crates/batten/src/hookcost.rs | 1 + crates/batten/src/policy.rs | 2 +- crates/batten/src/rules.rs | 178 +++++++++++++- crates/batten/src/secrets.rs | 1 + crates/batten/src/waiver.rs | 1 + crates/batten/tests/it/document_facts.rs | 153 +++++++++++- crates/batten/tests/it/frontmatter_gates.rs | 245 ++++++++++++++++++++ crates/batten/tests/it/main.rs | 1 + mise.toml | 2 +- policy/rules-paths-trigger.rego | 143 ++++++++++++ policy/skill-frontmatter-complete.rego | 140 +++++++++++ schema/batten.local.schema.json | 5 + schema/batten.schema.json | 5 + schema/policy-input.schema.json | 2 +- 22 files changed, 1156 insertions(+), 41 deletions(-) create mode 100644 crates/batten/tests/it/frontmatter_gates.rs create mode 100644 policy/rules-paths-trigger.rego create mode 100644 policy/skill-frontmatter-complete.rego 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..934576a16 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 = "rules-paths-trigger" +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 = "skill-frontmatter-complete" +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`). # @@ -10292,6 +10324,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/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..a20301c5d 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, } } } @@ -7490,9 +7572,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 +7589,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 +7626,7 @@ impl NotAcquired { NotAcquired::Absent => "absent", NotAcquired::Unreadable => "unreadable", NotAcquired::Unparsed => "unparsed", + NotAcquired::NoDocument => "no-document", NotAcquired::RootUnset => "root-unset", } } @@ -7794,13 +7895,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), } } @@ -9580,6 +9692,7 @@ fn policy_rule( // duplication that registry exists to remove. remediation: policy_remediation(registry, &violation.verdict), identity, + reason: None, }); } None @@ -10152,6 +10265,7 @@ fn ratchet_finding( // integer pair. identity::scope_fingerprint(&rule.id, glob), ), + reason: None, }); } @@ -10885,6 +10999,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 +11107,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 +11756,7 @@ fn run_once( line: None, check: rule.settling_check().unwrap_or(Check::Reevaluate), remediation: rule.remediation(), + reason: None, }); } Ok(()) @@ -11829,6 +11946,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 +12047,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 +12096,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 +12123,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 +12239,16 @@ 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 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 +13077,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 +13119,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/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..b9e0180ab --- /dev/null +++ b/crates/batten/tests/it/frontmatter_gates.rs @@ -0,0 +1,245 @@ +//! `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": "rules-paths-trigger", + "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": "skill-frontmatter-complete", + "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, + 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/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..2bf63eb98 --- /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 "rules-paths-trigger" + +# 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": "rules-paths-trigger", + "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": "rules-paths-trigger", + "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": "rules-paths-trigger", + "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..570315328 --- /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 "skill-frontmatter-complete" + +# 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": "skill-frontmatter-complete", + "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": "skill-frontmatter-complete", + "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": "skill-frontmatter-complete", + "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": { From b0e3776ff9f2a84afbb7dfada4a96bd784a66b5b Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Fri, 11 Sep 2026 17:56:54 +0000 Subject: [PATCH 2/4] fix(rules): count the reads `acquire` makes, so the rule-cost census stops lying MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-1790 reports, at Urgent, that tree-scoped policy rules receive no input at all and therefore every policy rule and preset passes silently. Reproduced in a minimal committed tree: the rule read its declared source, decided over it, and emitted a finding — while reporting `0 file(s) 0 byte(s)`. Repairing the fixture made it go clean, so both directions hold. The input is built, the module reads it, the verdict lands. What was broken is the odometer. Two defects, and the second only surfaced because the first was fixed. `acquire` is the one function that acquires a document and it never called `count_read`; all five call sites belong to the `forbid`/`ratchet` family. So every `policy` and `document` row reported zero however much it read, while a `forbid` row in the same config reported real numbers. The census was not empty, it was SELECTIVELY empty — and the kinds it zeroed are the ones whose silence is hardest to tell from a gate that decided nothing. That comparison is what the report reasoned from before rewriting three modules and about twenty predicates. Counting alone does not surface it. Declared documents are acquired ONCE for the whole run, before the per-rule loop, so those reads fall outside every rule's measurement window by construction — and the run total is a sum of the per-rule rows, so they were invisible at both levels. The shared cache is deliberate (N rows over one path in one form is one read), so charging the cost to whichever row the loop reached first would starve the others of an answer they equally caused. It is the step's cost, and it now gets the step's own census row. `costs_lock().clear()` sat after that acquisition and wiped the row measuring it. Hoisted to the actual top of the run, which is what its own comment already claimed. The row is pushed only when documents were acquired, so a run of `forbid` rows reads exactly as it did before. The summary line counts rules and excludes the step, since reporting one more rule than the config declares would be a second small lie in the line that exists to stop one. before: rule cost: rules-paths-trigger 1ms 0 file(s) 0 byte(s) rule cost: 1 rule(s) 1ms 0 file(s) 0 byte(s) after: rule cost: rules-paths-trigger 12ms 0 file(s) 0 byte(s) rule cost: (declared documents) 0ms 5 file(s) 4568 byte(s) rule cost: 1 rule(s) 13ms 5 file(s) 4568 byte(s) The census suite did not catch this because its `row()` helper builds a `forbid`: every case exercised the one kind that already counted. The new cases are a policy row over declared sources, and a rule set declaring no documents earning no row. Two fixtures moved out from under CLOUD-1787 rather than being weakened. `policy_tree`'s pair used `CLAUDE.md` as their example of an extension this build has no parser for, and `.md` now names one; both swapped to the `.bats` suite their own docs already named, with the claim unchanged. `document-node-differs` gains the reason token the finding now carries, and a new `document-no-frontmatter` fixture drives the `no-document` cause end to end over the compiled binary — the route CLOUD-1787 had covered only at unit level. Two of the issue's own recommendations are deliberately not taken here. Surfacing `0 file(s)` at default verbosity is unsafe until this lands, since it would broadcast a false zero on every policy and document rule in every repo; it is correct only after, and is an output-contract change of its own. Refusing a policy rule whose declared inputs resolve to zero files is a new refusal class rather than a metering fix, and is filed separately. Why the reporter's own modules emitted nothing remains open, and this does not answer it: the reasoning that reached "no input at all" rested on the broken counter, so the observation needs re-taking against a build that counts. Refs: CLOUD-1790, CLOUD-1217, CLOUD-1787, CLOUD-418 --- crates/batten/src/lib.rs | 15 +- crates/batten/src/rules.rs | 75 ++++++++- .../document-no-frontmatter/batten.toml.in | 14 ++ .../repos/document-no-frontmatter/expected.in | 4 + .../repos/document-no-frontmatter/notes.md.in | 10 ++ .../repos/document-node-differs/expected.in | 2 +- crates/batten/tests/it/policy_tree.rs | 32 ++-- crates/batten/tests/it/rule_cost_census.rs | 152 ++++++++++++++++++ 8 files changed, 284 insertions(+), 20 deletions(-) create mode 100644 crates/batten/tests/fixtures/repos/document-no-frontmatter/batten.toml.in create mode 100644 crates/batten/tests/fixtures/repos/document-no-frontmatter/expected.in create mode 100644 crates/batten/tests/fixtures/repos/document-no-frontmatter/notes.md.in 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/rules.rs b/crates/batten/src/rules.rs index a20301c5d..9d0cf5e23 100644 --- a/crates/batten/src/rules.rs +++ b/crates/batten/src/rules.rs @@ -6742,7 +6742,45 @@ fn run( // 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. + // + // MEASURED, AND ON ITS OWN CENSUS ROW (CLOUD-1790). The per-rule deltas below + // 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, not appended to: the census describes THIS run, and a caller + // reading rows from the previous one would be reading a different tree. + // + // AT THE TOP OF THE RUN RATHER THAN AT THE TOP OF THE RULE LOOP, which is + // where it sat and what the line below needs (CLOUD-1790). The acquisition + // is part of this run and is 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. + costs_lock().clear(); + let acquisition_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: acquisition_started.elapsed(), + files_read: files_read().saturating_sub(files_before), + bytes_read: bytes_read().saturating_sub(bytes_before), + }); + } // WHAT EARLIER RUNS PRODUCED, acquired once for the whole run beside the // three above and for the same reason (CLOUD-851). Bounded by DECLARATION — @@ -6917,9 +6955,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 @@ -7836,6 +7871,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); }; @@ -12239,6 +12299,15 @@ 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). /// 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/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); +} From 9e444d4da7c9ba96cea0f1db6b705334afe3118c Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Fri, 11 Sep 2026 23:43:50 +0000 Subject: [PATCH 3/4] refactor(config)!: carry this branch's two gates onto the new trunk The replay onto current `main` crosses CLOUD-1638, which made the rule-id grammar enforceable rather than conventional. Three repairs, all of them the rebase's rather than the work's, and none separable from it. THE RULE IDS ARE NOW REFUSED AT LOAD. `config::validate_tables` passes every `[[rule]]` id to `verdict::check_rule_id` once the consumer declares a vocabulary, and this repository declares 153 words. `rules-paths-trigger` and `skill-frontmatter-complete` normalise to three words, but none of those words is declared in the slot its position names, so both rows would be refused before either gate ran. Renamed to `prose carry missing` and `prompt declare partial`, every word drawn from the declared lists. THE MODULE NAMES DO NOT MOVE WITH THEM, and that is the point rather than an oversight. `$MUTANT_GATES` entries are resolved by `mutate::sources_for` against `policy/{name}.rego` on disk, so they are filenames and stay kebab -- main keeps `leased-push` while that rule's id is `branch write unsafe`. Renaming them would stand the census's subjects up against names no file carries, which is the reverse direction it reports as `names-no-subject`. THE PRUNE BASIS FOLLOWS THE TREE. `target-prune` refused at `declared 244, live 255, tolerance 10`: main's own replay took the tree to 254 by itself and `frontmatter_gates.rs` is the file that tips it one past the band. Both bases move to 255 and neither floor does, for the 2026-09-08 entry's reason, recorded above the rows. `policy::Vocabulary` gained a `words` field in that same change. The frontmatter tier's literal is the one the replay could not carry it into, this side's file being new. Refs: CLOUD-1787, CLOUD-1790, CLOUD-1638 --- batten.toml | 25 +++++++++++++++++---- crates/batten/tests/it/frontmatter_gates.rs | 7 ++++-- policy/rules-paths-trigger.rego | 8 +++---- policy/skill-frontmatter-complete.rego | 8 +++---- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/batten.toml b/batten.toml index 934576a16..a844d20d5 100644 --- a/batten.toml +++ b/batten.toml @@ -6858,7 +6858,7 @@ tags = "v*" # which is the whole difference between a gate over a directory and a gate over # the five files somebody typed. [[rule]] -id = "rules-paths-trigger" +id = "prose carry missing" kind = "policy" scope = "tree" module = "policy/rules-paths-trigger.rego" @@ -6871,7 +6871,7 @@ sources = [".claude/rules/*.md"] # conforming files under it and a fixture proving it red, so both directions are # demonstrated rather than one asserted. [[rule]] -id = "skill-frontmatter-complete" +id = "prompt declare partial" kind = "policy" scope = "tree" module = "policy/skill-frontmatter-complete.rego" @@ -8427,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 diff --git a/crates/batten/tests/it/frontmatter_gates.rs b/crates/batten/tests/it/frontmatter_gates.rs index b9e0180ab..9951f12b2 100644 --- a/crates/batten/tests/it/frontmatter_gates.rs +++ b/crates/batten/tests/it/frontmatter_gates.rs @@ -45,7 +45,7 @@ use batten::rules::{self, Rule}; /// the same column census a consumer's config does. fn stub_row() -> Rule { serde_json::from_value(serde_json::json!({ - "id": "rules-paths-trigger", + "id": "prose carry missing", "kind": "policy", "scope": "tree", "sources": [".claude/rules/*.md"], @@ -57,7 +57,7 @@ fn stub_row() -> Rule { fn skill_row() -> Rule { serde_json::from_value(serde_json::json!({ - "id": "skill-frontmatter-complete", + "id": "prompt declare partial", "kind": "policy", "scope": "tree", "sources": ["skills/*/SKILL.md", ".claude/skills/*/SKILL.md"], @@ -91,6 +91,9 @@ fn findings_for(root: &Path, row: Rule, vocabulary_root: &Path) -> Vec { 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, diff --git a/policy/rules-paths-trigger.rego b/policy/rules-paths-trigger.rego index 2bf63eb98..457b24f9f 100644 --- a/policy/rules-paths-trigger.rego +++ b/policy/rules-paths-trigger.rego @@ -36,7 +36,7 @@ package batten.rules_paths_trigger import rego.v1 -rules contains "rules-paths-trigger" +rules contains "prose carry missing" # The fence is there and carries no `paths:`, or carries an empty one. # @@ -47,7 +47,7 @@ rules contains "rules-paths-trigger" # lands in `missing` below — so both are decided rather than one silently # standing in for the other. violation contains { - "rule": "rules-paths-trigger", + "rule": "prose carry missing", "verdict": "prose declare missing", "subjects": [{"path": path}], } if { @@ -63,7 +63,7 @@ triggers_on_something(document) if { # because the remedy differs: one says fix the trigger you wrote, this says you # wrote none. violation contains { - "rule": "rules-paths-trigger", + "rule": "prose carry missing", "verdict": "prose declare missing", "subjects": [{"path": path}], } if { @@ -75,7 +75,7 @@ violation contains { # 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": "rules-paths-trigger", + "rule": "prose carry missing", "verdict": "prose read unread", "subjects": [{"path": path}], } if { diff --git a/policy/skill-frontmatter-complete.rego b/policy/skill-frontmatter-complete.rego index 570315328..9e6799595 100644 --- a/policy/skill-frontmatter-complete.rego +++ b/policy/skill-frontmatter-complete.rego @@ -28,7 +28,7 @@ package batten.skill_frontmatter_complete import rego.v1 -rules contains "skill-frontmatter-complete" +rules contains "prompt declare partial" # A field is absent, or present and blank. # @@ -37,7 +37,7 @@ rules contains "skill-frontmatter-complete" # testing only for the KEY passes both — a skill that declares its fields and # fills in neither. violation contains { - "rule": "skill-frontmatter-complete", + "rule": "prompt declare partial", "verdict": "prompt declare missing", "subjects": [{"path": path}], } if { @@ -53,7 +53,7 @@ stated(value) if { # The declared name and the directory it ships in disagree. violation contains { - "rule": "skill-frontmatter-complete", + "rule": "prompt declare partial", "verdict": "prompt name wrong", "subjects": [{"path": path}], } if { @@ -75,7 +75,7 @@ directory_of(path) := segment if { # with no fence declares no fields, which is the violation above arriving # through the acquisition layer instead of through the document. violation contains { - "rule": "skill-frontmatter-complete", + "rule": "prompt declare partial", "verdict": "prompt read unread", "subjects": [{"path": path}], } if { From f7ceb110ac223142e4c611966ed514ef224f1830 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Sat, 12 Sep 2026 00:31:47 +0000 Subject: [PATCH 4/4] refactor(rules): extract the measured acquisition so `run` stays under its cap Counting the declared acquisition and giving it its own census row took `run` to 105 lines against clippy's cap of 100, so the step moves out whole -- `costs_lock().clear()`, the timing, `acquire_declared` and the conditional census push -- into `acquire_declared_measured`, beside the function it wraps. NOT A SEPARATE CONCERN, and the extraction carries CLOUD-1790's reasoning down with it rather than leaving the comments behind at the call site. The clear belongs at the top of the RUN, and after this move the run's top is the callee's first line, which is the same instant: the acquisition is measured before any rule evaluates, so a clear placed after it wipes the row that measured it. Refs: CLOUD-1790, CLOUD-1787 --- crates/batten/src/rules.rs | 92 +++++++++++++++++++++----------------- 1 file changed, 51 insertions(+), 41 deletions(-) diff --git a/crates/batten/src/rules.rs b/crates/batten/src/rules.rs index 9d0cf5e23..eced5557b 100644 --- a/crates/batten/src/rules.rs +++ b/crates/batten/src/rules.rs @@ -6740,47 +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. - // - // MEASURED, AND ON ITS OWN CENSUS ROW (CLOUD-1790). The per-rule deltas below - // 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, not appended to: the census describes THIS run, and a caller - // reading rows from the previous one would be reading a different tree. - // - // AT THE TOP OF THE RUN RATHER THAN AT THE TOP OF THE RULE LOOP, which is - // where it sat and what the line below needs (CLOUD-1790). The acquisition - // is part of this run and is 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. - costs_lock().clear(); - let acquisition_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: acquisition_started.elapsed(), - files_read: files_read().saturating_sub(files_before), - bytes_read: bytes_read().saturating_sub(bytes_before), - }); - } + // 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 — @@ -8147,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,