Let a text-capable guard stand in front of a command

docs/REFERENCE.md

@@ -55,7 +55,17 @@ does not run.**
 Both halves are checked at load. A rule naming two checks is refused, because
 one of them would be read by nothing while looking enforced. A rule naming no
 place is refused, because it runs nowhere and that reads exactly like a rule
-that passes.
+that passes. `command.before` is refused on a check no shim can consult — the
+shim consults `exec` checkers and the built-ins that can judge arbitrary text
+(`prevent-ai-author`, `prevent-unusual-unicode`, `no-private-repo-names`), and
+anything else reads an index, an identity or a push range and has nothing to say
+about a pull-request body.
+
+A text-capable built-in with `command.before` and no `git.hooks` is a deliberate
+shape, not an omission. `no-private-repo-names` reads the commit message at
+every git hook, and a repository whose own prose cites its issues would have
+every one of those citations refused — so the seam it belongs at is the command
+that publishes text to a forge, and only that one.
 
 Exit codes: `0` clean, `1` violations, `2` the check could not be made.
 

src/config.rs

@@ -903,13 +903,33 @@ impl Rule {
         // it: "nothing says where it runs" is SATISFIED by the very field that
         // cannot be used, so the one check that exists to find a rule with no
         // place is the check this rule slips past.
-        if check != Check::Exec && self.command.is_some() {
+        // A text-capable built-in stands in front of a command too, and it is
+        // the ONLY seam some of them belong at: `no-private-repo-names` reads a
+        // commit message at every git hook, which refuses the issue citations a
+        // repository's own prose is full of, so a repository that wants it over
+        // a pull-request body and nowhere else has no other field to say it in.
+        // Three wrote `command.before` on the built-in independently while this
+        // refused all three.
+        let text_capable = self
+            .builtin()
+            .is_some_and(|builtin| crate::guard::TEXT_GUARDS.contains(&builtin));
+        if check != Check::Exec && !text_capable && self.command.is_some() {
+            let built_in_note = if check == Check::Builtin {
+                format!(
+                    "\nThe built-ins that can judge the text a command publishes are {}; \
+                     any other reads an index, an identity or a push range, and has nothing \
+                     to say about a pull-request body.",
+                    crate::guard::TEXT_GUARDS.join(", ")
+                )
+            } else {
+                String::new()
+            };
             return Err(Fatal::new(format!(
-                "rule {:?}: only an `exec` checker stands in front of a command, so \
-                 `command.before` on a `{check}` rule would be read by nothing and would \
-                 look like configuration that works.\n\
+                "rule {:?}: a `{check}` rule cannot stand in front of a command, so \
+                 `command.before` here would be read by nothing and would look like \
+                 configuration that works.\n\
                  A rule that searches the tree says so with `files.*`, and a built-in \
-                 that fires at a git hook says so with `git.hooks`.",
+                 that fires at a git hook says so with `git.hooks`.{built_in_note}",
                 self.id
             )));
         }
@@ -1299,9 +1319,18 @@ fn validate_shims(policy_path: &Path, rules: &[Rule], shims: &[crate::shim::Shim
         }
     }
 
+    // Both kinds count. An `exec` checker is a program this repository names; a
+    // text-capable built-in is one the binary carries, and `shim::run` consults
+    // both. Counting only the first refused a policy whose shim was checked --
+    // by a guard rather than by a script -- as a shim checked by nothing.
     let checked: BTreeSet<&str> = rules
         .iter()
-        .filter(|rule| rule.is(Check::Exec))
+        .filter(|rule| {
+            rule.is(Check::Exec)
+                || rule
+                    .builtin()
+                    .is_some_and(|builtin| crate::guard::TEXT_GUARDS.contains(&builtin))
+        })
         .filter_map(|rule| rule.command.as_ref())
         .flat_map(|where_| where_.before.iter())
         .filter_map(|line| line.split_whitespace().next())
@@ -1607,18 +1636,20 @@ mod tests {
     #[test]
     /// `command.before` on a check no shim can consult.
     ///
-    /// The third member of the same family, and the one that was missing.
-    /// `shim::run` filters its checkers to `exec` rules, so a built-in whose
+    /// The third member of the same family. `shim::run` consults `exec`
+    /// checkers and text-capable BUILT-INS, so a rule that is neither and whose
     /// only declared place is `command.before` is consulted by nothing and runs
     /// nowhere -- and the "nothing says where it runs" refusal is satisfied by
     /// the very field that cannot be used, so the check meant to catch a rule
     /// with no place is the one this rule walked past.
     fn a_command_place_the_check_cannot_use_is_refused() {
-        // The regexp case carries `files.*` too: it is a rule that really does
-        // run, by the scan, and the `command.before` beside it is the part that
-        // reaches nothing.
+        // A built-in that reads the index, an identity or a push range has
+        // nothing to say about the text a command publishes. The regexp case
+        // carries `files.*` too: it is a rule that really does run, by the scan,
+        // and the `command.before` beside it is the part that reaches nothing.
         for check in [
-            "builtin = \"prevent-ai-author\"",
+            "builtin = \"prevent-public-push\"",
+            "builtin = \"prevent-unusual-unicode-in-files\"",
             "message = \"no\"\nregexp = \"TODO\"\nfiles.include = [\".\"]",
         ] {
             let error = policy_from(&format!(
@@ -1632,6 +1663,38 @@ mod tests {
         }
     }
 
+    #[test]
+    /// A text-capable built-in may stand in front of a command.
+    ///
+    /// It is the only seam some of them belong at. `no-private-repo-names`
+    /// reads a commit message at every git hook, which refuses the issue
+    /// citations a repository's own prose is full of -- so a repository that
+    /// wants it over a pull-request body and nowhere else has no other field to
+    /// say it in. Three wrote `command.before` on the built-in independently
+    /// while the loader refused all three, on the true-but-unhelpful grounds
+    /// that a built-in is not an `exec`.
+    fn a_text_capable_builtin_may_stand_in_front_of_a_command() {
+        for builtin in crate::guard::TEXT_GUARDS {
+            let loaded = policy_from(&format!(
+                "[[shim]]\ncommand = \"gh\"\nmatch = [\"pr:create\"]\n\
+                 text_flags = [\"-b\"]\n\n\
+                 [rule.stands-in-front]\nbuiltin = \"{builtin}\"\n\n\
+                 [rule.stands-in-front.command]\nbefore = [\"gh\"]\n"
+            ));
+            assert!(loaded.is_ok(), "{builtin}: {:?}", loaded.err());
+            let policy = loaded.unwrap();
+            let rule = policy
+                .rules
+                .iter()
+                .find(|rule| rule.id == "stands-in-front");
+            assert!(
+                rule.is_some(),
+                "{builtin}: the rule did not survive the load"
+            );
+            assert_eq!(rule.unwrap().seams(), vec!["shim"], "{builtin}");
+        }
+    }
+
     /// A `command` table that names no command line is a place that selects
     /// nothing, and the "where does it run" check reads it as a place.
     #[test]

src/guard/mod.rs

@@ -169,25 +169,41 @@ pub(crate) fn over_text(
 ) -> Result<Vec<Refusal>> {
     let mut refusals = Vec::new();
     for rule in policy.of_check(Check::Builtin) {
-        let Some(builtin) = rule.builtin() else {
-            continue;
-        };
-        if !TEXT_GUARDS.contains(&builtin) || bypassed(&rule.id) {
-            continue;
-        }
-        let found = match builtin {
-            "prevent-ai-author" => message::ai_author_in(rule, label, text),
-            "prevent-unusual-unicode" => message::unusual_unicode_in(rule, label, text),
-            "no-private-repo-names" => names::in_text(root, rule, label, text)?,
-            _ => None,
-        };
-        if let Some(refusal) = found {
+        if let Some(refusal) = text_refusal(root, rule, label, text)? {
             refusals.push(refusal);
         }
     }
     Ok(refusals)
 }
 
+/// One text-capable guard's verdict over one piece of text.
+///
+/// `None` where the rule is not a text guard, or is bypassed, or found nothing
+/// -- three different reasons a caller does not have to tell apart, because a
+/// guard with nothing to say about a pull-request body is not a guard that
+/// passed it. Extracted so the shim seam consults exactly the same dispatch
+/// `uphold guard --text` does: a text guard that judged a commit message one
+/// way and a PR body another would be two rules under one id.
+pub(crate) fn text_refusal(
+    root: &Path,
+    rule: &Rule,
+    label: &str,
+    text: &str,
+) -> Result<Option<Refusal>> {
+    let Some(builtin) = rule.builtin() else {
+        return Ok(None);
+    };
+    if !TEXT_GUARDS.contains(&builtin) || bypassed(&rule.id) {
+        return Ok(None);
+    }
+    Ok(match builtin {
+        "prevent-ai-author" => message::ai_author_in(rule, label, text),
+        "prevent-unusual-unicode" => message::unusual_unicode_in(rule, label, text),
+        "no-private-repo-names" => names::in_text(root, rule, label, text)?,
+        _ => None,
+    })
+}
+
 /// Run one guard.
 pub(crate) fn evaluate(request: &Request<'_>) -> Result<Option<Refusal>> {
     let id = request.rule.id.as_str();

src/shim.rs

@@ -1117,9 +1117,19 @@ pub(crate) fn run(root: &Path, policy: &Policy, name: &str, argv: &[OsString]) -
     // also asked about a branch name on `git push` and a tarball on `npm
     // publish` -- because the only thing selecting it was `kind = "command"`,
     // which says nothing about which command.
+    // Two kinds stand in front of a command, and both are scoped by the same
+    // `command.before`. An `exec` checker is a program this repository names.
+    // A text-capable BUILT-IN is one the binary already carries -- and a
+    // repository that wanted `no-private-repo-names` over a pull-request body
+    // had no way to say so: the guard reads a commit message at every git hook,
+    // which refuses the issue citations a repository's own prose is full of, so
+    // the seam it belongs at is this one and only this one. Three repositories
+    // wrote `command.before` on the built-in independently and the loader
+    // refused all three, on the true-but-unhelpful grounds that a built-in is
+    // not an `exec`. The field means what they meant now.
     let checkers: Vec<&Rule> = policy
         .before_command(name, &words)
-        .filter(|rule| rule.is(Check::Exec))
+        .filter(|rule| rule.is(Check::Exec) || rule.is(Check::Builtin))
         .collect();
     let mut refusals: Vec<String> = Vec::new();
 
@@ -1152,6 +1162,17 @@ pub(crate) fn run(root: &Path, policy: &Policy, name: &str, argv: &[OsString]) -
                     if crate::guard::bypassed(&rule.id) {
                         continue;
                     }
+                    if rule.is(Check::Builtin) {
+                        // The same dispatch `uphold guard --text` runs, so a
+                        // guard cannot judge a commit message one way and a
+                        // pull-request body another under one id.
+                        if let Some(refusal) =
+                            crate::guard::text_refusal(root, rule, subject.kind, &subject.value)?
+                        {
+                            refusals.push(refusal.report);
+                        }
+                        continue;
+                    }
                     if let Some(refusal) = consult(root, rule, subject)? {
                         refusals.push(format!("{refusal}\n{}", rule.message()));
                     }

tests/shim_cli.rs

@@ -385,3 +385,71 @@ fn an_argument_that_is_not_text_reaches_the_command_it_was_typed_for() {
         stdout(&output)
     );
 }
+
+/// A text-capable built-in standing in front of a command, with no `exec`
+/// checker anywhere and no git hook.
+///
+/// The seam some guards belong at and could not name. `no-private-repo-names`
+/// reads a commit message at every git hook, which refuses the issue citations
+/// a repository's own prose is full of -- so a repository that wants it over a
+/// pull-request body and NOWHERE else had no field to say it in, and three
+/// wrote `command.before` on the built-in independently while the loader
+/// refused all three.
+const BUILTIN_CHECKER: &str = r#"
+[[shim]]
+command = "faux"
+match = ["pr:create"]
+text_flags = ["-t", "--title", "-b", "--body"]
+scope = "always"
+
+[rule.no-private-repo-names]
+builtin = "no-private-repo-names"
+visibility = "public"
+private_owners = ["acme-private"]
+
+[rule.no-private-repo-names.command]
+before = ["faux"]
+"#;
+
+#[test]
+fn a_text_capable_builtin_refuses_the_body_it_stands_in_front_of() {
+    let root = workspace(BUILTIN_CHECKER);
+    let output = shim(
+        &root,
+        &[
+            "faux",
+            "pr",
+            "create",
+            "-b",
+            "this fixes acme-private/thing",
+        ],
+    );
+    assert_eq!(code(&output), 1, "{}", stdout(&output));
+    assert!(
+        stderr(&output).contains("acme-private"),
+        "{}",
+        stderr(&output)
+    );
+    // Refused means not published: the real command must not have run.
+    assert!(!stdout(&output).contains("faux ran"), "{}", stdout(&output));
+}
+
+#[test]
+fn a_clean_body_reaches_the_real_command_through_a_builtin_checker() {
+    let root = workspace(BUILTIN_CHECKER);
+    let output = shim(&root, &["faux", "pr", "create", "-b", "an ordinary change"]);
+    assert_eq!(code(&output), 0, "{}", stderr(&output));
+    assert!(stdout(&output).contains("faux ran"), "{}", stdout(&output));
+}
+
+#[test]
+fn a_builtin_checker_satisfies_the_shim_that_would_otherwise_check_nothing() {
+    // The load refuses a shim no checker names, because a command collected and
+    // consulted by nothing runs anyway -- an invocation that passed because
+    // nothing looked at it. A built-in is a checker, and counting only `exec`
+    // rules refused a policy whose shim WAS checked, by a guard rather than a
+    // script.
+    let root = workspace(BUILTIN_CHECKER);
+    let output = shim(&root, &["faux", "--version"]);
+    assert_eq!(code(&output), 0, "{}{}", stdout(&output), stderr(&output));
+}