From a02a81a0ae39adc1edd8158eaff16fad73018fcd Mon Sep 17 00:00:00 2001 From: CLAUDE <355856132+HoTT-stuff@users.noreply.github.com> Date: Fri, 13 Nov 2026 12:02:20 +0000 Subject: [PATCH 1/2] feat(panicbot): directives from the repo deed, deny-only means full suite minus deny, deed-declared dynamic target Owner rulings 2026-11-13. - Directives are read from the (bot panicbot ...) clause of _chora.deed first, then the legacy panicbot.a2ml (read only), else the full suite. A new strict S-expression reader (deed.rs) fails loudly on malformed input. - A directive with only deny is the full suite minus deny; explicit allow still narrows; deny wins. The old static-trio default is removed. - (target :build :program) declares the dynamic-mode target. :build runs via sh -c in the scanned repo under the suite budget; a failed build is an error part; pull_request_target / workflow_run refuse the build too. No target: dynamic parts are not_applicable with the deed-terms reason. --target overrides. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- bots/panicbot/src/build.rs | 145 +++++ bots/panicbot/src/config.rs | 40 +- bots/panicbot/src/deed.rs | 463 ++++++++++++++ bots/panicbot/src/directives.rs | 640 ++++++++++++++++++-- bots/panicbot/src/fleet.rs | 2 +- bots/panicbot/src/lib.rs | 2 + bots/panicbot/src/suite.rs | 69 ++- bots/panicbot/tests/envelope_parity_test.rs | 63 ++ docs/REPORT-BACK.adoc | 22 +- 9 files changed, 1344 insertions(+), 102 deletions(-) create mode 100644 bots/panicbot/src/build.rs create mode 100644 bots/panicbot/src/deed.rs diff --git a/bots/panicbot/src/build.rs b/bots/panicbot/src/build.rs new file mode 100644 index 00000000..af55e67b --- /dev/null +++ b/bots/panicbot/src/build.rs @@ -0,0 +1,145 @@ +// SPDX-License-Identifier: MPL-2.0 +//! Runs the deed-declared `:build` command for the dynamic modes. +//! +//! `sh -c COMMAND` in the scanned directory, in its own process group, with a +//! timeout. The group is killed on timeout so a build that forked helpers does +//! not outlive the lane. Output is captured (never written to panicbot's +//! stdout, which carries the JSON) and the tail of it is quoted in the error. +//! This EXECUTES repository code: callers gate it exactly like the dynamic +//! modes (see [`crate::suite::dynamic_refusal`]). + +use anyhow::{anyhow, bail, Context, Result}; +use std::io::Read; +use std::path::Path; +use std::process::{Command, Stdio}; +use std::sync::mpsc; +use std::time::{Duration, Instant}; + +/// How much of the build output an error quotes. +const TAIL_BYTES: usize = 1500; + +/// Run `command` with `sh -c` in `dir`. `Ok` only on exit status 0. +pub fn run_build(command: &str, dir: &Path, timeout: Duration) -> Result<()> { + use std::os::unix::process::CommandExt; + let mut child = Command::new("sh") + .arg("-c") + .arg(command) + .current_dir(dir) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .process_group(0) + .spawn() + .with_context(|| format!("could not start the build `{command}`"))?; + + let (tx, rx) = mpsc::channel::>(); + for stream in [ + child + .stdout + .take() + .map(|s| Box::new(s) as Box), + child + .stderr + .take() + .map(|s| Box::new(s) as Box), + ] + .into_iter() + .flatten() + { + let tx = tx.clone(); + std::thread::spawn(move || { + let mut stream = stream; + let mut tail: Vec = Vec::new(); + let mut buf = [0u8; 4096]; + while let Ok(n) = stream.read(&mut buf) { + if n == 0 { + break; + } + tail.extend_from_slice(&buf[..n]); + if tail.len() > TAIL_BYTES * 2 { + tail.drain(..tail.len() - TAIL_BYTES); + } + } + let _ = tx.send(tail); + }); + } + drop(tx); + + let start = Instant::now(); + let status = loop { + if let Some(st) = child.try_wait().context("waiting for the build failed")? { + break st; + } + if start.elapsed() >= timeout { + let _ = Command::new("kill") + .args(["-KILL", "--", &format!("-{}", child.id())]) + .status(); + let _ = child.kill(); + let _ = child.wait(); + bail!( + "the build `{command}` timed out after {}s and was killed", + timeout.as_secs() + ); + } + std::thread::sleep(Duration::from_millis(20)); + }; + + if status.success() { + return Ok(()); + } + let mut out: Vec = Vec::new(); + while let Ok(part) = rx.recv_timeout(Duration::from_millis(500)) { + out.extend(part); + } + let text = String::from_utf8_lossy(&out); + let text = text.trim(); + let text = if text.len() > TAIL_BYTES { + let mut from = text.len() - TAIL_BYTES; + while !text.is_char_boundary(from) { + from += 1; + } + &text[from..] + } else { + text + }; + Err(anyhow!( + "the build `{command}` failed ({status}){}", + if text.is_empty() { + String::new() + } else { + format!(": {}", text.replace('\n', " | ")) + } + )) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_successful_build_runs_in_the_directory() { + let d = tempfile::tempdir().unwrap(); + run_build("touch built", d.path(), Duration::from_secs(10)).unwrap(); + assert!(d.path().join("built").exists()); + } + + #[test] + fn a_failing_build_quotes_its_output() { + let d = tempfile::tempdir().unwrap(); + let e = run_build("echo boom >&2; exit 3", d.path(), Duration::from_secs(10)) + .unwrap_err() + .to_string(); + assert!(e.contains("failed") && e.contains("boom"), "{e}"); + } + + #[test] + fn a_hung_build_is_killed_at_the_timeout() { + let d = tempfile::tempdir().unwrap(); + let t = Instant::now(); + let e = run_build("sleep 30 & sleep 30", d.path(), Duration::from_millis(300)) + .unwrap_err() + .to_string(); + assert!(e.contains("timed out"), "{e}"); + assert!(t.elapsed() < Duration::from_secs(10)); + } +} diff --git a/bots/panicbot/src/config.rs b/bots/panicbot/src/config.rs index 4c10dc7f..a8719aa5 100644 --- a/bots/panicbot/src/config.rs +++ b/bots/panicbot/src/config.rs @@ -36,11 +36,28 @@ pub struct PanicbotConfig { #[serde(default)] pub notes: Vec, + /// The dynamic-mode target the repository declares in its deed + /// (`(target :build ... :program ...)`). None: the dynamic parts are + /// `not_applicable` unless the CLI gives `--target`. + #[serde(default)] + pub target: Option, + /// Per-category confidence overrides. /// If absent, `translator::default_confidence()` is used. pub confidence_overrides: Vec, } +/// A dynamic-mode target declared by the repository: an optional build command +/// and the program it produces. Both come from the deed and both EXECUTE +/// repository code; see [`crate::suite`] for the checks around them. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct TargetDecl { + /// Shell command (`sh -c`, run in the scanned directory) that builds the program. + pub build: Option, + /// The built executable, relative to the scanned directory. + pub program: String, +} + /// A panic-attack subcommand panicbot can run (class (a) of the inventory in /// [`crate::suite`]). #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] @@ -184,17 +201,6 @@ impl AllowedCommands { } } - /// What a directive file granted before the full-suite ruling: the static - /// trio. A directive file that says nothing about `allow` keeps exactly - /// this, so a repository's scope is never widened behind its back. - pub fn legacy_static() -> Self { - let mut a = Self::none(); - a.assail = true; - a.adjudicate = true; - a.diagnostics = true; - a - } - /// Whether `cmd` is allowed. pub fn allows(&self, cmd: Subcommand) -> bool { match cmd { @@ -272,6 +278,7 @@ impl Default for PanicbotConfig { timeout: Duration::from_secs(300), budget: DEFAULT_BUDGET, notes: Vec::new(), + target: None, confidence_overrides: Vec::new(), } } @@ -410,15 +417,4 @@ mod suite_tests { } } } - - #[test] - fn legacy_static_is_exactly_the_old_default() { - let a = AllowedCommands::legacy_static(); - let on: Vec<&str> = Subcommand::ALL - .into_iter() - .filter(|c| a.allows(*c)) - .map(|c| c.name()) - .collect(); - assert_eq!(on, ["assail", "adjudicate", "diagnostics"]); - } } diff --git a/bots/panicbot/src/deed.rs b/bots/panicbot/src/deed.rs new file mode 100644 index 00000000..d1a07ef9 --- /dev/null +++ b/bots/panicbot/src/deed.rs @@ -0,0 +1,463 @@ +// SPDX-License-Identifier: MPL-2.0 +//! A small, strict reader for the repo deed (`_chora.deed`). +//! +//! No other crate in gitbot-fleet reads deeds (checked 2026-11-13: no +//! `chora`/`deed` reader in shared-context or any bot), so this module reads +//! just enough of the grammar (`standards/1-formats/deed/spec/abnf/deed.abnf`) +//! to find one clause and its fields: +//! +//! - `(` is the only bracket; `;` starts a comment that runs to the line end; +//! - a clause is `(head item...)`, an item is a `:keyword value` field or a +//! nested clause, separated by spaces or line ends (a tab is an error); +//! - values: `"strings"` (escapes `\"` `\\` `\n` `\t` only), bare symbols, +//! integers, `#t` / `#f`, the `#u5"..."` / `#u8"..."` / `#uuid"..."` +//! literals, `'quoted` symbols and lists, and `( lists )`; +//! - the whole input must be consumed. +//! +//! One deliberate leniency: inside a `bot` clause a bare symbol is accepted as +//! an item, so both `(bot panicbot ...)` and `(bot :name panicbot ...)` read +//! (the standards fixture spells it the second way). Anywhere else a bare +//! symbol is an error. +//! +//! This is a reader, not the validator: `deed_lint` in standards stays the +//! conformance gate (header, `:schema-version`, line length, UUID shape). The +//! reader's job is to never guess: every malformed input is an `Err` with a +//! line number, never a silent default. + +use std::fmt; + +/// One parenthesised clause. +#[derive(Debug, Clone, PartialEq)] +pub struct Clause { + /// The symbol after `(`. + pub head: String, + /// Fields, nested clauses and (in `bot` only) bare symbols, in order. + pub items: Vec, + /// 1-based line of the `(`. + pub line: usize, +} + +/// An item inside a clause. +#[derive(Debug, Clone, PartialEq)] +pub enum Item { + /// `:key value`. + Field { + key: String, + value: Value, + line: usize, + }, + /// A nested clause. + Clause(Clause), + /// A bare symbol (only inside a `bot` clause). + Atom { name: String, line: usize }, +} + +/// A field value. +#[derive(Debug, Clone, PartialEq)] +pub enum Value { + Str(String), + Sym(String), + Int(i64), + Bool(bool), + /// `#u5"..."`, `#u8"..."` or `#uuid"..."`: carried as its text, unused here. + Literal(String), + Quoted(Box), + List(Vec), +} + +impl Value { + fn describe(&self) -> &'static str { + match self { + Value::Str(_) => "a string", + Value::Sym(_) => "a symbol", + Value::Int(_) => "an integer", + Value::Bool(_) => "a boolean", + Value::Literal(_) => "a literal", + Value::Quoted(_) => "a quoted value", + Value::List(_) => "a list", + } + } +} + +/// A deed parse or shape error, with the 1-based line it was found on. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DeedError { + pub line: usize, + pub message: String, +} + +impl fmt::Display for DeedError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "line {}: {}", self.line, self.message) + } +} + +impl std::error::Error for DeedError {} + +/// Shorthand for results in this module. +pub type Result = std::result::Result; + +impl Clause { + /// The nested clauses with this head. + pub fn clauses<'a>(&'a self, head: &'a str) -> impl Iterator + 'a { + self.items.iter().filter_map(move |i| match i { + Item::Clause(c) if c.head == head => Some(c), + _ => None, + }) + } +} + +impl Value { + /// The value as a bare symbol, or an error naming `what`. + pub fn as_symbol(&self, what: &str, line: usize) -> Result<&str> { + match self { + Value::Sym(s) => Ok(s), + other => Err(DeedError { + line, + message: format!("{what} must be a symbol, found {}", other.describe()), + }), + } + } + + /// The value as a string, or an error naming `what`. + pub fn as_string(&self, what: &str, line: usize) -> Result<&str> { + match self { + Value::Str(s) => Ok(s), + other => Err(DeedError { + line, + message: format!("{what} must be a string, found {}", other.describe()), + }), + } + } + + /// The value as a list of bare symbols, or an error naming `what`. + pub fn as_symbol_list(&self, what: &str, line: usize) -> Result> { + match self { + Value::List(vs) => vs.iter().map(|v| v.as_symbol(what, line)).collect(), + other => Err(DeedError { + line, + message: format!("{what} must be a list, found {}", other.describe()), + }), + } + } +} + +/// Parse a whole deed: one top-level clause, nothing after it. +pub fn parse(text: &str) -> Result { + let mut p = Parser { + s: text.chars().collect(), + i: 0, + line: 1, + }; + p.skip_sep()?; + if p.peek() != Some('(') { + return Err(p.err("a deed starts with '('")); + } + let root = p.clause()?; + p.skip_sep()?; + if p.peek().is_some() { + return Err(p.err("trailing content after the closing ')'")); + } + Ok(root) +} + +struct Parser { + s: Vec, + i: usize, + line: usize, +} + +fn is_sym_start(c: char) -> bool { + c.is_ascii_alphabetic() +} + +fn is_sym_char(c: char) -> bool { + c.is_ascii_alphanumeric() || ".*/<>=!?+-_".contains(c) +} + +impl Parser { + fn err(&self, message: &str) -> DeedError { + DeedError { + line: self.line, + message: message.to_string(), + } + } + + fn peek(&self) -> Option { + self.s.get(self.i).copied() + } + + fn bump(&mut self) -> Option { + let c = self.peek()?; + self.i += 1; + if c == '\n' { + self.line += 1; + } + Some(c) + } + + /// Skip spaces, line ends and comments. Returns whether anything was + /// consumed. A tab is an error. + fn skip_sep(&mut self) -> Result { + let start = self.i; + loop { + match self.peek() { + Some(' ' | '\n') => { + self.bump(); + } + Some('\r') => { + self.bump(); + } + Some('\t') => return Err(self.err("a tab is not a valid separator (use spaces)")), + Some(';') => { + while let Some(c) = self.peek() { + if c == '\n' { + break; + } + self.bump(); + } + } + _ => break, + } + } + Ok(self.i > start) + } + + fn symbol(&mut self) -> Result { + let mut out = String::new(); + match self.peek() { + Some(c) if is_sym_start(c) => { + out.push(c); + self.bump(); + } + _ => return Err(self.err("expected a symbol")), + } + while let Some(c) = self.peek() { + if is_sym_char(c) { + out.push(c); + self.bump(); + } else { + break; + } + } + Ok(out) + } + + fn clause(&mut self) -> Result { + let line = self.line; + self.bump(); // '(' + let head = self.symbol()?; + let mut items = Vec::new(); + loop { + let sep = self.skip_sep()?; + match self.peek() { + None => { + return Err(DeedError { + line, + message: format!("clause '{head}' is never closed"), + }) + } + Some(')') => { + self.bump(); + return Ok(Clause { head, items, line }); + } + Some(_) if !sep => return Err(self.err("expected a space between items")), + Some('(') => items.push(Item::Clause(self.clause()?)), + Some(':') => { + let line = self.line; + self.bump(); + let key = self.symbol()?; + if !self.skip_sep()? { + return Err(self.err("expected a space between a keyword and its value")); + } + let value = self.value()?; + items.push(Item::Field { key, value, line }); + } + Some(c) if head == "bot" && is_sym_start(c) => { + let line = self.line; + let name = self.symbol()?; + items.push(Item::Atom { name, line }); + } + Some(c) => { + return Err(self.err(&format!( + "unexpected '{c}' in clause '{head}' (expected a :field or a clause)" + ))) + } + } + } + } + + fn value(&mut self) -> Result { + match self.peek() { + None => Err(self.err("expected a value, found the end of the file")), + Some('"') => Ok(Value::Str(self.string()?)), + Some('(') => self.list(), + Some('\'') => { + self.bump(); + match self.peek() { + Some('(') => Ok(Value::Quoted(Box::new(self.list()?))), + Some(c) if is_sym_start(c) => { + Ok(Value::Quoted(Box::new(Value::Sym(self.symbol()?)))) + } + _ => Err(self.err("only a symbol or a list can be quoted")), + } + } + Some('#') => self.hash_literal(), + Some(c) if c.is_ascii_digit() || c == '-' => self.integer(), + Some(c) if is_sym_start(c) => Ok(Value::Sym(self.symbol()?)), + Some(c) => Err(self.err(&format!("unexpected '{c}' where a value was expected"))), + } + } + + fn list(&mut self) -> Result { + let line = self.line; + self.bump(); // '(' + let mut out = Vec::new(); + loop { + let sep = self.skip_sep()?; + match self.peek() { + None => { + return Err(DeedError { + line, + message: "list is never closed".into(), + }) + } + Some(')') => { + self.bump(); + return Ok(Value::List(out)); + } + Some(_) if !sep && !out.is_empty() => { + return Err(self.err("expected a space between list values")) + } + Some(_) => out.push(self.value()?), + } + } + } + + fn string(&mut self) -> Result { + self.bump(); // opening quote + let mut out = String::new(); + loop { + match self.bump() { + None => return Err(self.err("string is never closed")), + Some('"') => return Ok(out), + Some('\\') => match self.bump() { + Some('"') => out.push('"'), + Some('\\') => out.push('\\'), + Some('n') => out.push('\n'), + Some('t') => out.push('\t'), + _ => { + return Err(self.err("invalid escape (only \\\" \\\\ \\n \\t are allowed)")) + } + }, + Some(c) if (c as u32) < 0x20 || c as u32 == 0x7f => { + return Err(self.err("a control character in a string")) + } + Some(c) => out.push(c), + } + } + } + + fn integer(&mut self) -> Result { + let mut text = String::new(); + if self.peek() == Some('-') { + text.push('-'); + self.bump(); + } + let digits_from = text.len(); + while let Some(c) = self.peek() { + if c.is_ascii_digit() { + text.push(c); + self.bump(); + } else { + break; + } + } + if text.len() == digits_from { + return Err(self.err("expected digits after '-'")); + } + if self.peek().is_some_and(is_sym_char) { + return Err(self.err("an integer must end at a separator")); + } + text.parse::() + .map(Value::Int) + .map_err(|_| self.err("integer out of range")) + } + + fn hash_literal(&mut self) -> Result { + self.bump(); // '#' + let mut tag = String::new(); + while let Some(c) = self.peek() { + if c.is_ascii_alphanumeric() { + tag.push(c); + self.bump(); + } else { + break; + } + } + match tag.as_str() { + "t" | "f" => { + if self.peek().is_some_and(is_sym_char) { + return Err(self.err("a boolean must end at a separator")); + } + Ok(Value::Bool(tag == "t")) + } + "u5" | "u8" | "uuid" => { + if self.peek() != Some('"') { + return Err(self.err(&format!("#{tag} must be followed by a string"))); + } + Ok(Value::Literal(format!("#{tag}\"{}\"", self.string()?))) + } + _ => Err(self.err(&format!("unknown literal '#{tag}'"))), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn reads_fields_clauses_comments_and_every_value_kind() { + let c = parse( + ";; SPDX-License-Identifier: MPL-2.0\n; note\n(repo-deed\n :schema-version \"1.0.0\" ; tail\n :id #u5\"a/b\"\n :n -12\n :on #t\n :off #f\n :sym thing.v1\n :q 'sym\n :ql '(a b)\n :l (\n a \"b\" 3)\n (inner :k \"a\\\"b\\\\c\\nd\\te\"))\n", + ) + .unwrap(); + assert_eq!(c.head, "repo-deed"); + let inner = c.clauses("inner").next().unwrap(); + match &inner.items[0] { + Item::Field { value, .. } => assert_eq!(value, &Value::Str("a\"b\\c\nd\te".into())), + other => panic!("{other:?}"), + } + assert_eq!(c.items.len(), 10); + } + + #[test] + fn bare_symbols_are_only_allowed_in_a_bot_clause() { + assert!(parse("(x (bot panicbot :a b))").is_ok()); + assert!(parse("(x (other panicbot))").is_err()); + } + + #[test] + fn malformed_input_is_an_error_with_a_line() { + for (label, text) in [ + ("unclosed", "(a\n :b c"), + ("trailing", "(a) (b)"), + ("tab", "(a\n\t:b c)"), + ("bad escape", "(a :b \"\\u0041\")"), + ("unclosed string", "(a :b \"x)"), + ("no value", "(a :b)"), + ("no space", "(a :b c(d))"), + ("toml", "key = \"v\""), + ("bare in clause", "(a b)"), + ("bad literal", "(a :b #x)"), + ("bad bool", "(a :b #true)"), + ("quoted string", "(a :b '\"x\")"), + ("empty", ""), + ] { + assert!(parse(text).is_err(), "{label} should not parse"); + } + let e = parse("(a\n\n :b c\n :d").unwrap_err(); + assert!(e.line >= 3, "{e}"); + } +} diff --git a/bots/panicbot/src/directives.rs b/bots/panicbot/src/directives.rs index 2bb4c703..99b9e0c9 100644 --- a/bots/panicbot/src/directives.rs +++ b/bots/panicbot/src/directives.rs @@ -1,32 +1,52 @@ // SPDX-License-Identifier: MPL-2.0 -//! Directives — per-repo configuration parser for `.machine_readable/bot_directives/panicbot.a2ml`. +//! Directives — per-repo configuration for panicbot. //! -//! Each repository can customise panicbot's behaviour via a TOML-shaped file. -//! A2ML is retired and the deed has no per-bot directive clause yet (the -//! standards mapping only covers the `instructions-dir` pointer), so this -//! module only READS the legacy file; it never writes or extends one. +//! ## Where directives come from (owner rulings 2026-11-13) //! -//! ## What a repository gets +//! 1. **The repo deed**, `_chora.deed` at the scanned directory's root: +//! the `(bot panicbot ...)` entry inside `(bot-directives ...)`. Canonical. +//! 2. **The legacy file** `.machine_readable/bot_directives/panicbot.a2ml` +//! (TOML-shaped), READ ONLY, used only when the deed has no panicbot entry. +//! A2ML is retired: this module never writes or extends one. +//! 3. **Neither: the full suite**, every class (a) subcommand (see +//! [`crate::suite`]), all severities, 300 s per command, no target. //! -//! - **No directive file: the full suite** — every class (a) panic-attack -//! subcommand (see [`crate::suite`]), all severities, 300 s per command -//! inside the suite budget (owner ruling 2026-11-13). -//! - **A directive file keeps exactly what it allows and denies.** Before the -//! full-suite ruling a file with no `allow` key granted the static trio -//! (`assail`, `adjudicate`, `diagnostics`); it still grants exactly that, so -//! no repository is widened behind its back. `allow = [...]` replaces that -//! set; `deny = [...]` removes from it, and a deny wins over an allow of the -//! same name. Widening needs an explicit `allow`. -//! - **A file that exists but cannot be parsed is an error**, not a fallback to -//! the (now full) default: failing open would widen a repository whose -//! directive is broken. Callers must surface the error. +//! If the deed has a panicbot entry it wins and the legacy file is ignored +//! (with a note). A deed or legacy file that exists but cannot be read or +//! parsed is an **error**, never a fallback to the (full) default: failing open +//! would widen a repository whose directive is broken. Callers must surface it. //! -//! ## Directive Format (A2ML / TOML-shaped) +//! ## Deed clause +//! +//! ```text +//! (bot-directives +//! (bot panicbot +//! :allow (assail assay diagnostics) ; optional; absent = full suite +//! :deny (assault ambush) ; optional; deny wins over allow +//! (target :build "cargo build --release" :program "target/release/foo") +//! (config :min-severity low :timeout-seconds 300))) +//! ``` +//! +//! `(bot :name panicbot ...)` reads the same (the standards fixture spells it +//! so). Every part is optional except that a `target` needs `:program`. +//! Unknown fields, unknown subcommand names, a wrong value type, a duplicate +//! entry and an ambiguous deed (more than one `*_chora.deed` at the root) are +//! errors: the deed reader is strict where the legacy file only notes. +//! +//! - **Deny-only means the full suite minus the deny.** An explicit `allow` +//! narrows to exactly what it names; a deny always wins. +//! - **`target`** declares the program for the dynamic modes (assault, ambush, +//! then adjudicate). `:build`, if present, is run with `sh -c` in the scanned +//! directory before them; `:program` is what is passed as the target. The +//! CLI `--target` replaces the declared target, build included. See +//! [`crate::suite`] for the checks and the privileged-trigger refusal. +//! - **`config`**: `:min-severity` (`low`, `medium`, `high`, `critical`) and +//! `:timeout-seconds` (a positive integer). +//! +//! ## Legacy file format (read-only) //! //! ```toml -//! schema_version = "1.0" //! bot = "panicbot" -//! scope = "static-analysis" //! allow = ["assail", "adjudicate", "diagnostics"] //! deny = ["attack", "assault", "ambush", "amuck", "abduct", "axial"] //! @@ -35,46 +55,318 @@ //! timeout-seconds = 300 //! ``` //! +//! The same allow/deny rules apply (a file with no `allow` is the full suite +//! minus its `deny`). Unknown names are ignored with a note, which can only +//! narrow. The legacy file has no `target`: only the deed declares one. +//! //! Names are panic-attack subcommands: `assail`, `assay`, `adjudicate`, //! `diagnostics`, `attack`, `assault`, `ambush`, `amuck`, `abduct`, `axial`. -//! An unknown name is ignored with a note (it can only ever narrow, never -//! widen), and the note is printed. -//! -//! The SCM form was retired 2026-04-17; no fallback is supported. -use crate::config::{AllowedCommands, MinSeverity, PanicbotConfig, Subcommand}; -use anyhow::{Context, Result}; +use crate::config::{AllowedCommands, MinSeverity, PanicbotConfig, Subcommand, TargetDecl}; +use crate::deed::{self, Clause, Item}; +use anyhow::{bail, Context, Result}; use serde::Deserialize; use std::path::Path; use std::time::Duration; -/// Path to the bot directive file within a repository. +/// Path to the legacy bot directive file within a repository. const DIRECTIVE_PATH: &str = ".machine_readable/bot_directives/panicbot.a2ml"; -/// Load panicbot configuration from a repository's directive file. -/// -/// If the file doesn't exist, returns the default config: the full suite, all -/// severities, 300 s per command. If the file exists but can't be read or -/// parsed, returns an error (never the default: see the module docs). +/// Load panicbot configuration for a repository: the deed clause, else the +/// legacy file, else the full suite (see the module docs). pub fn load_directives(repo_path: &Path) -> Result { - let directive_file = repo_path.join(DIRECTIVE_PATH); + let legacy_file = repo_path.join(DIRECTIVE_PATH); + let legacy_present = legacy_file.exists(); + + if let Some(deed_path) = find_deed(repo_path)? { + let text = std::fs::read_to_string(&deed_path) + .with_context(|| format!("Failed to read {}", deed_path.display()))?; + let root = deed::parse(&text) + .with_context(|| format!("Failed to parse {}", deed_path.display()))?; + if let Some(mut config) = from_deed(&root) + .with_context(|| format!("Invalid (bot panicbot) clause in {}", deed_path.display()))? + { + if legacy_present { + config.notes.push(format!( + "{DIRECTIVE_PATH} exists but is ignored: the deed's (bot panicbot) clause wins" + )); + } + return Ok(config); + } + tracing::debug!("{} has no (bot panicbot) entry", deed_path.display()); + } - if !directive_file.exists() { + if !legacy_present { tracing::debug!( - "No directive file at {}, using the full suite", - directive_file.display() + "No directives for {}, using the full suite", + repo_path.display() ); return Ok(PanicbotConfig::default()); } - let content = std::fs::read_to_string(&directive_file) - .with_context(|| format!("Failed to read {}", directive_file.display()))?; + let content = std::fs::read_to_string(&legacy_file) + .with_context(|| format!("Failed to read {}", legacy_file.display()))?; + parse_directive(&content).with_context(|| format!("Failed to parse {}", legacy_file.display())) +} - parse_directive(&content) - .with_context(|| format!("Failed to parse {}", directive_file.display())) +/// The one `*_chora.deed` at the repo root, if any. More than one is an error. +fn find_deed(repo_path: &Path) -> Result> { + let entries = match std::fs::read_dir(repo_path) { + Ok(e) => e, + // A scan target that is not a directory has no deed. + Err(_) => return Ok(None), + }; + let mut found: Vec = Vec::new(); + for entry in entries { + let entry = entry.with_context(|| format!("Failed to list {}", repo_path.display()))?; + let name = entry.file_name(); + let name = name.to_string_lossy(); + if name.ends_with("_chora.deed") && entry.path().is_file() { + found.push(entry.path()); + } + } + found.sort(); + match found.len() { + 0 => Ok(None), + 1 => Ok(found.pop()), + _ => bail!( + "more than one *_chora.deed at the repository root ({}); cannot tell which is the repo deed", + found + .iter() + .filter_map(|p| p.file_name()) + .map(|n| n.to_string_lossy().into_owned()) + .collect::>() + .join(", ") + ), + } +} + +fn shape(line: usize, message: impl Into) -> anyhow::Error { + anyhow::Error::new(deed::DeedError { + line, + message: message.into(), + }) } -/// Raw A2ML shape. +/// Read the `(bot panicbot ...)` entry of a parsed deed. `None`: the deed has +/// no such entry (not an error). Anything wrong inside an entry is an error. +fn from_deed(root: &Clause) -> Result> { + let mut sections = root.clauses("bot-directives"); + let Some(section) = sections.next() else { + return Ok(None); + }; + if let Some(second) = sections.next() { + return Err(shape(second.line, "more than one (bot-directives) clause")); + } + + let mut mine: Option<&Clause> = None; + for bot in section.clauses("bot") { + if bot_name(bot)? == "panicbot" { + if let Some(first) = mine { + return Err(shape( + bot.line, + format!( + "(bot panicbot) appears twice (first at line {})", + first.line + ), + )); + } + mine = Some(bot); + } + } + let Some(bot) = mine else { + return Ok(None); + }; + + let mut config = PanicbotConfig::default(); + let mut allow: Option> = None; + let mut deny: Vec = Vec::new(); + let mut seen: Vec<&str> = Vec::new(); + let mut target_seen = false; + let mut config_seen = false; + + for item in &bot.items { + match item { + Item::Atom { .. } => {} // the name, checked by bot_name + Item::Field { key, value, line } => { + if seen.contains(&key.as_str()) { + return Err(shape(*line, format!("duplicate :{key}"))); + } + match key.as_str() { + "name" => { + value.as_symbol(":name", *line)?; + } + "allow" => allow = Some(subcommands(":allow", value, *line)?), + "deny" => deny = subcommands(":deny", value, *line)?, + other => { + return Err(shape( + *line, + format!("unknown field :{other} in (bot panicbot)"), + )) + } + } + seen.push(key); + } + Item::Clause(c) => match c.head.as_str() { + "target" => { + if std::mem::replace(&mut target_seen, true) { + return Err(shape(c.line, "more than one (target) clause")); + } + config.target = Some(read_target(c)?); + } + "config" => { + if std::mem::replace(&mut config_seen, true) { + return Err(shape(c.line, "more than one (config) clause")); + } + read_config(c, &mut config)?; + } + other => { + return Err(shape( + c.line, + format!("unknown clause ({other} ...) in (bot panicbot)"), + )) + } + }, + } + } + + // Absent allow = the full suite; an allow narrows to what it names. + config.allowed_commands = match &allow { + None => AllowedCommands::all(), + Some(names) => { + let mut a = AllowedCommands::none(); + for c in names { + a.set(*c, true); + } + a + } + }; + for c in &deny { + if allow.as_ref().is_some_and(|a| a.contains(c)) { + config.notes.push(format!( + "directive lists '{}' in both allow and deny: deny wins", + c.name() + )); + } + config.allowed_commands.set(*c, false); + } + Ok(Some(config)) +} + +/// The bot's name: `:name SYM` or a single bare symbol. +fn bot_name(bot: &Clause) -> Result<&str> { + let mut name: Option<&str> = None; + for item in &bot.items { + let (n, line) = match item { + Item::Atom { name, line } => (name.as_str(), *line), + Item::Field { + key, + value: deed::Value::Sym(s), + line, + } if key == "name" => (s.as_str(), *line), + _ => continue, + }; + if name.is_some() { + return Err(shape(line, "a (bot) clause names one bot")); + } + name = Some(n); + } + name.ok_or_else(|| shape(bot.line, "a (bot) clause needs a name: (bot NAME ...)")) +} + +fn subcommands(what: &str, value: &deed::Value, line: usize) -> Result> { + value + .as_symbol_list(what, line)? + .into_iter() + .map(|n| { + Subcommand::parse(n).ok_or_else(|| { + shape( + line, + format!("{what}: unknown subcommand '{n}' (expected one of {})", { + Subcommand::ALL + .iter() + .map(|c| c.name()) + .collect::>() + .join(", ") + }), + ) + }) + }) + .collect() +} + +fn read_target(c: &Clause) -> Result { + let (mut build, mut program) = (None, None); + for item in &c.items { + match item { + Item::Field { key, value, line } => match key.as_str() { + "build" => { + if build.is_some() { + return Err(shape(*line, "duplicate :build")); + } + let s = value.as_string(":build", *line)?; + if s.trim().is_empty() { + return Err(shape(*line, ":build is empty")); + } + build = Some(s.to_string()); + } + "program" => { + if program.is_some() { + return Err(shape(*line, "duplicate :program")); + } + let s = value.as_string(":program", *line)?; + if s.trim().is_empty() { + return Err(shape(*line, ":program is empty")); + } + program = Some(s.to_string()); + } + other => return Err(shape(*line, format!("unknown field :{other} in (target)"))), + }, + Item::Clause(n) => { + return Err(shape( + n.line, + format!("unexpected clause ({}) in (target)", n.head), + )) + } + Item::Atom { line, .. } => return Err(shape(*line, "unexpected symbol in (target)")), + } + } + let program = + program.ok_or_else(|| shape(c.line, "(target) needs :program, the built executable"))?; + Ok(TargetDecl { build, program }) +} + +fn read_config(c: &Clause, config: &mut PanicbotConfig) -> Result<()> { + let mut seen: Vec<&str> = Vec::new(); + for item in &c.items { + let Item::Field { key, value, line } = item else { + return Err(shape(c.line, "(config) takes only :fields")); + }; + if seen.contains(&key.as_str()) { + return Err(shape(*line, format!("duplicate :{key}"))); + } + seen.push(key); + match key.as_str() { + "min-severity" => { + let s = value.as_symbol(":min-severity", *line)?; + config.min_severity = s.parse::().map_err(|_| { + shape( + *line, + format!("unknown :min-severity '{s}' (low, medium, high, critical)"), + ) + })?; + } + "timeout-seconds" => match value { + deed::Value::Int(n) if *n > 0 => config.timeout = Duration::from_secs(*n as u64), + _ => return Err(shape(*line, ":timeout-seconds must be a positive integer")), + }, + other => return Err(shape(*line, format!("unknown field :{other} in (config)"))), + } + } + Ok(()) +} + +/// Raw legacy A2ML (TOML) shape. #[derive(Debug, Default, Deserialize)] struct DirectiveFile { #[serde(default)] @@ -97,18 +389,15 @@ struct ConfigBlock { timeout_seconds: Option, } -/// Parse a panicbot directive from its A2ML/TOML content. +/// Parse a legacy panicbot directive from its A2ML/TOML content. /// -/// The content is a directive FILE, so the baseline is what a file granted -/// before the full-suite ruling (the static trio), not the no-file default. +/// With no `allow` the baseline is the full suite (owner ruling 2026-11-13: +/// a deny-only directive means everything minus the deny). fn parse_directive(content: &str) -> Result { - let mut config = PanicbotConfig { - allowed_commands: AllowedCommands::legacy_static(), - ..PanicbotConfig::default() - }; + let mut config = PanicbotConfig::default(); let file: DirectiveFile = toml::from_str(content).context("Failed to parse TOML")?; - // Allow list: an explicit allow REPLACES the baseline. + // Allow list: an explicit allow REPLACES the full-suite baseline. if let Some(allowed) = &file.allow { config.allowed_commands = AllowedCommands::none(); for name in allowed { @@ -219,10 +508,8 @@ timeout-seconds = 60 #[test] fn test_parse_empty_content() { let config = parse_directive("").unwrap(); - // Should return defaults (TOML parses empty as empty struct) - assert!(config.allowed_commands.assail); - assert!(config.allowed_commands.adjudicate); - assert!(config.allowed_commands.diagnostics); + // Empty is the full suite (TOML parses empty as an empty struct). + assert_eq!(config.allowed_commands, AllowedCommands::all()); assert_eq!(config.min_severity, MinSeverity::Low); } @@ -306,20 +593,22 @@ timeout-seconds = 300 } #[test] - fn a_file_with_no_allow_key_keeps_the_static_trio_and_is_not_widened() { + fn a_file_with_no_allow_key_is_the_full_suite_per_the_2026_11_13_ruling() { let d = repo_with_directive("[config]\nmin-severity = \"high\"\n"); let config = load_directives(d.path()).unwrap(); - assert_eq!( - allowed_names(&config), - ["assail", "adjudicate", "diagnostics"] - ); + assert_eq!(config.allowed_commands, AllowedCommands::all()); assert_eq!(config.min_severity, MinSeverity::High); } #[test] - fn deny_alone_narrows_the_baseline() { + fn deny_alone_is_the_full_suite_minus_the_deny() { let config = parse_directive("deny = [\"adjudicate\"]\n").unwrap(); - assert_eq!(allowed_names(&config), ["assail", "diagnostics"]); + let expect: Vec<&str> = Subcommand::ALL + .into_iter() + .map(|c| c.name()) + .filter(|n| *n != "adjudicate") + .collect(); + assert_eq!(allowed_names(&config), expect); } #[test] @@ -385,3 +674,232 @@ timeout-seconds = 300 assert!(load_directives(d.path()).is_err()); } } + +#[cfg(test)] +mod deed_tests { + use super::*; + + fn names(config: &PanicbotConfig) -> Vec<&'static str> { + Subcommand::ALL + .into_iter() + .filter(|c| config.allowed_commands.allows(*c)) + .map(|c| c.name()) + .collect() + } + + fn deed_for(clause: &str) -> String { + format!( + ";; SPDX-License-Identifier: MPL-2.0\n(repo-deed\n :schema-version \"1.0.0\"\n :canonical-name \"x\"\n (bot-directives\n (methodology :mode hybrid)\n {clause}))\n" + ) + } + + fn repo(deed: Option<&str>, legacy: Option<&str>) -> tempfile::TempDir { + let d = tempfile::tempdir().unwrap(); + if let Some(t) = deed { + std::fs::write(d.path().join("x_chora.deed"), t).unwrap(); + } + if let Some(t) = legacy { + let dir = d.path().join(".machine_readable/bot_directives"); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("panicbot.a2ml"), t).unwrap(); + } + d + } + + #[test] + fn deny_only_in_the_deed_is_the_full_suite_minus_the_deny() { + let d = repo( + Some(&deed_for("(bot panicbot :deny (assault ambush))")), + None, + ); + let c = load_directives(d.path()).unwrap(); + let expect: Vec<&str> = Subcommand::ALL + .into_iter() + .map(|c| c.name()) + .filter(|n| *n != "assault" && *n != "ambush") + .collect(); + assert_eq!(names(&c), expect); + assert!(c.target.is_none()); + } + + #[test] + fn an_explicit_allow_narrows_and_deny_wins_over_it() { + let d = repo( + Some(&deed_for( + "(bot panicbot :allow (assail assay diagnostics) :deny (assay))", + )), + None, + ); + let c = load_directives(d.path()).unwrap(); + assert_eq!(names(&c), ["assail", "diagnostics"]); + assert!(c.notes.iter().any(|n| n.contains("deny wins"))); + } + + #[test] + fn the_full_clause_reads_target_and_config() { + let d = repo( + Some(&deed_for( + "(bot :name panicbot\n :allow (assail assault)\n (target :build \"cargo build --release\" :program \"target/release/foo\")\n (config :min-severity high :timeout-seconds 60))", + )), + None, + ); + let c = load_directives(d.path()).unwrap(); + assert_eq!(names(&c), ["assail", "assault"]); + assert_eq!( + c.target, + Some(TargetDecl { + build: Some("cargo build --release".into()), + program: "target/release/foo".into() + }) + ); + assert_eq!(c.min_severity, MinSeverity::High); + assert_eq!(c.timeout, Duration::from_secs(60)); + } + + #[test] + fn a_target_without_a_build_is_allowed() { + let d = repo( + Some(&deed_for("(bot panicbot (target :program \"bin/x\"))")), + None, + ); + let c = load_directives(d.path()).unwrap(); + assert_eq!(c.target.unwrap().build, None); + } + + #[test] + fn the_deed_clause_wins_over_the_legacy_file() { + let d = repo( + Some(&deed_for("(bot panicbot :deny (assay))")), + Some("allow = [\"assail\"]\n"), + ); + let c = load_directives(d.path()).unwrap(); + assert!(c.allowed_commands.assail && c.allowed_commands.assault); + assert!(!c.allowed_commands.assay); + assert!( + c.notes.iter().any(|n| n.contains("ignored")), + "{:?}", + c.notes + ); + } + + #[test] + fn the_legacy_file_is_the_fallback_when_the_deed_has_no_panicbot_entry() { + for deed in [ + "(repo-deed :schema-version \"1.0.0\")\n".to_string(), + deed_for("(bot hypatia :deny (assay))"), + deed_for("(bot rhodibot)"), + ] { + let d = repo(Some(&deed), Some("allow = [\"assail\"]\n")); + let c = load_directives(d.path()).unwrap(); + assert_eq!(names(&c), ["assail"], "{deed}"); + } + } + + #[test] + fn neither_deed_nor_legacy_is_the_full_suite_with_no_target() { + let d = repo(None, None); + let c = load_directives(d.path()).unwrap(); + assert_eq!(c.allowed_commands, AllowedCommands::all()); + assert!(c.target.is_none()); + // A deed without the clause behaves the same. + let d = repo(Some("(repo-deed :schema-version \"1.0.0\")\n"), None); + assert_eq!( + load_directives(d.path()).unwrap().allowed_commands, + AllowedCommands::all() + ); + } + + #[test] + fn a_malformed_deed_or_clause_is_an_error_never_a_default() { + let legacy = Some("allow = [\"assail\"]\n"); + let cases: Vec<(&str, String)> = vec![ + ( + "unbalanced deed", + "(repo-deed :schema-version \"1.0.0\"\n".into(), + ), + ( + "unknown subcommand", + deed_for("(bot panicbot :deny (asault))"), + ), + ("allow not a list", deed_for("(bot panicbot :allow assail)")), + ( + "allow of strings", + deed_for("(bot panicbot :allow (\"assail\"))"), + ), + ("unknown field", deed_for("(bot panicbot :dney (assault))")), + ( + "unknown clause", + deed_for("(bot panicbot (targt :program \"x\"))"), + ), + ( + "target without program", + deed_for("(bot panicbot (target :build \"make\"))"), + ), + ( + "build not a string", + deed_for("(bot panicbot (target :build make :program \"x\"))"), + ), + ( + "empty build", + deed_for("(bot panicbot (target :build \"\" :program \"x\"))"), + ), + ( + "two targets", + deed_for("(bot panicbot (target :program \"a\") (target :program \"b\"))"), + ), + ( + "bad severity", + deed_for("(bot panicbot (config :min-severity severe))"), + ), + ( + "bad timeout", + deed_for("(bot panicbot (config :timeout-seconds 0))"), + ), + ( + "string timeout", + deed_for("(bot panicbot (config :timeout-seconds \"5\"))"), + ), + ( + "duplicate deny", + deed_for("(bot panicbot :deny (assay) :deny (assault))"), + ), + ( + "two panicbot entries", + deed_for("(bot panicbot) (bot panicbot)"), + ), + ("nameless bot", deed_for("(bot :deny (assay))")), + ( + "two bot-directives", + format!( + "{}{}", + "(repo-deed (bot-directives (bot panicbot)) (bot-directives (bot panicbot)))", + "\n" + ), + ), + ]; + for (label, text) in cases { + let d = repo(Some(&text), legacy); + assert!( + load_directives(d.path()).is_err(), + "{label} must be an error" + ); + } + } + + #[test] + fn two_deeds_at_the_root_are_an_error() { + let d = repo(Some(&deed_for("(bot panicbot)")), None); + std::fs::write(d.path().join("y_chora.deed"), deed_for("(bot panicbot)")).unwrap(); + let e = load_directives(d.path()).unwrap_err(); + assert!(format!("{e:#}").contains("more than one"), "{e:#}"); + } + + #[test] + fn the_estate_template_deed_parses_with_this_reader() { + // Guards the reader against rejecting a real, valid deed. + let p = std::path::Path::new("/home/user/rsr-template-repo/rsr-template-repo_chora.deed"); + if let Ok(text) = std::fs::read_to_string(p) { + deed::parse(&text).expect("the template deed must read"); + } + } +} diff --git a/bots/panicbot/src/fleet.rs b/bots/panicbot/src/fleet.rs index f17c4e33..cb3023dd 100644 --- a/bots/panicbot/src/fleet.rs +++ b/bots/panicbot/src/fleet.rs @@ -3,7 +3,7 @@ //! //! Registers as `BotId::Panicbot` in gitbot-shared-context. Provides the //! standard fleet entry point (`run_fleet_scan`) that: -//! 1. Reads per-repo directives (`.machine_readable/bot_directives/panicbot.scm`) +//! 1. Reads per-repo directives (the repo deed's `(bot panicbot ...)` clause, else the legacy `.machine_readable/bot_directives/panicbot.a2ml`) //! 2. Runs the allowed panic-attack suite (full suite by default, see `suite`) //! 3. Translates each part's results into Finding structs (assail WeakPoints, //! assay candidates, dynamic-mode results when a target is given) diff --git a/bots/panicbot/src/lib.rs b/bots/panicbot/src/lib.rs index d0f8522b..11560df4 100644 --- a/bots/panicbot/src/lib.rs +++ b/bots/panicbot/src/lib.rs @@ -36,8 +36,10 @@ //! - **Guardian**: Block PRs on critical findings. pub mod a2ml_writer; +pub mod build; pub mod cli; pub mod config; +pub mod deed; pub mod directives; pub mod fleet; pub mod scanner; diff --git a/bots/panicbot/src/suite.rs b/bots/panicbot/src/suite.rs index f5cb113d..71de56d7 100644 --- a/bots/panicbot/src/suite.rs +++ b/bots/panicbot/src/suite.rs @@ -49,18 +49,26 @@ //! //! ## Dynamic modes: where the target comes from //! -//! `assault` and `ambush` execute a program. Panicbot does not build the -//! scanned repository, so there is no program unless the operator gives one: -//! `--target PATH` (a built executable inside the scanned directory). Without -//! it the dynamic parts report `not_applicable` with that reason. None of the -//! CI callers passes a target, so in CI the dynamic parts are `not_applicable`. +//! `assault` and `ambush` execute a program. The repository declares it in its +//! deed: `(bot-directives (bot panicbot (target :build "..." :program "...")))` +//! (see [`crate::directives`]). If a `:build` is declared it is run first, once, +//! with `sh -c` in the scanned directory, under a timeout drawn from the suite +//! budget; a failed or timed-out build is an `error` on each dynamic part and +//! nothing is attacked. `:program` is then the target, subject to the checks +//! below: canonicalised inside the scanned directory, an executable file. +//! `--target PATH` on the command line replaces the declared target (the build +//! included: the operator is then responsible for the program existing). +//! With no target from either, the dynamic parts report `not_applicable` with +//! "no target declared in the deed (bot-directives (bot panicbot (target …)))". //! //! The security gate (checked 2026-11-13 over gitbot-fleet //! `fleet-report-back-reusable.yml` and standards `fork-governance.yml`): both //! run on plain `pull_request` with `contents: read` and no secrets, which is //! unprivileged. A reusable workflow cannot know its caller's trigger, though, //! so [`dynamic_refusal`] makes panicbot refuse to execute repository code when -//! `GITHUB_EVENT_NAME` is `pull_request_target` or `workflow_run`. +//! `GITHUB_EVENT_NAME` is `pull_request_target` or `workflow_run`. That covers +//! the `:build` too: a refused run never builds, so no repository code runs at +//! all (the parts report the refusal as an `error`). //! //! ## Rule ids //! @@ -237,6 +245,8 @@ pub trait Engine { fn adjudicate(&self, reports: &[PathBuf], timeout: Duration) -> Result; /// The tool's health. fn diagnostics(&self, timeout: Duration) -> Result; + /// The deed-declared `:build` command, run with `sh -c` in `dir`. + fn build(&self, command: &str, dir: &Path, timeout: Duration) -> Result<()>; } /// The real panic-attack binary. @@ -265,6 +275,9 @@ impl Engine for PanicAttack { fn diagnostics(&self, timeout: Duration) -> Result { scanner::run_diagnostics(timeout) } + fn build(&self, command: &str, dir: &Path, timeout: Duration) -> Result<()> { + crate::build::run_build(command, dir, timeout) + } } /// Why panicbot must not execute repository code right now, if it must not. @@ -490,6 +503,15 @@ pub fn run_suite_with( let mut dynamic_reports: Vec = Vec::new(); let mut target_rel: Option = None; let mut scratch: Option = None; + // The CLI --target replaces the deed target, build included; otherwise the + // deed's declaration supplies both. The build runs at most once. + let (program_spec, build_cmd): (Option, Option<&str>) = + match (&opts.target, &config.target) { + (Some(t), _) => (Some(t.clone()), None), + (None, Some(d)) => (Some(PathBuf::from(&d.program)), d.build.as_deref()), + (None, None) => (None, None), + }; + let mut build_state: Option> = None; let min = config.min_severity.threshold(); // The part's timeout: the per-command timeout, capped by what is left. @@ -574,7 +596,7 @@ pub fn run_suite_with( } } Subcommand::Assault | Subcommand::Ambush => { - let Some(target) = &opts.target else { + let Some(target) = &program_spec else { push(PartStatus::NotApplicable, no_target_msg(), 0); continue; }; @@ -582,6 +604,23 @@ pub fn run_suite_with( push(PartStatus::Error, why, 0); continue; } + if let Some(cmdline) = build_cmd { + if build_state.is_none() { + let Some(timeout) = slot(&start) else { + push(PartStatus::Error, budget_msg(config), 0); + continue; + }; + build_state = Some( + engine + .build(cmdline, dir, timeout) + .map_err(|e| format!("{e:#}")), + ); + } + if let Some(Err(why)) = &build_state { + push(PartStatus::Error, why.clone(), 0); + continue; + } + } let program = match resolve_target(dir, target) { Ok(p) => p, Err(why) => { @@ -714,9 +753,7 @@ fn budget_msg(config: &PanicbotConfig) -> String { } fn no_target_msg() -> String { - "no target program: panicbot does not build the scanned repository and none was \ - given (--target PATH)" - .into() + "no target declared in the deed (bot-directives (bot panicbot (target …)))".into() } #[cfg(test)] @@ -732,6 +769,7 @@ mod tests { timeouts: RefCell>, keeps: RefCell>, adjudicated: RefCell>, + built: RefCell>, fail: Vec<&'static str>, healthy: bool, crash: bool, @@ -746,6 +784,7 @@ mod tests { timeouts: RefCell::new(Vec::new()), keeps: RefCell::new(Vec::new()), adjudicated: RefCell::new(Vec::new()), + built: RefCell::new(Vec::new()), fail: Vec::new(), healthy: true, crash: true, @@ -889,6 +928,12 @@ mod tests { }], }) } + fn build(&self, command: &str, dir: &Path, timeout: Duration) -> Result<()> { + self.built + .borrow_mut() + .push((command.to_string(), dir.to_path_buf())); + self.note("build", timeout) + } fn diagnostics(&self, timeout: Duration) -> Result { self.note("diagnostics", timeout)?; Ok(DiagnosticsReport { @@ -954,7 +999,9 @@ mod tests { // With no target the dynamic parts say so: never a silent skip, never a pass. for cmd in [Subcommand::Assault, Subcommand::Ambush] { assert_eq!(status_of(&run, cmd), PartStatus::NotApplicable); - assert!(detail_of(&run, cmd).contains("no target program")); + assert!(detail_of(&run, cmd).contains( + "no target declared in the deed (bot-directives (bot panicbot (target …)))" + )); } for cmd in [ Subcommand::Attack, diff --git a/bots/panicbot/tests/envelope_parity_test.rs b/bots/panicbot/tests/envelope_parity_test.rs index 73e93934..1e43903c 100644 --- a/bots/panicbot/tests/envelope_parity_test.rs +++ b/bots/panicbot/tests/envelope_parity_test.rs @@ -429,3 +429,66 @@ fn a_crashed_diagnostics_is_an_error_part() { && p["status"] == "error" && p["detail"].as_str().unwrap().contains("signal"))); } + +fn tree_with_deed(clause: &str) -> TempDir { + let t = TempDir::new().unwrap(); + fs::write(t.path().join("a.rs"), "fn main() {}\n").unwrap(); + fs::write( + t.path().join("x_chora.deed"), + format!( + ";; SPDX-License-Identifier: MPL-2.0\n(repo-deed\n :schema-version \"1.0.0\"\n (bot-directives\n {clause}))\n" + ), + ) + .unwrap(); + t +} + +#[test] +fn a_deed_deny_only_runs_the_rest_and_no_target_says_so_in_the_deed_terms() { + let bin = TempDir::new().unwrap(); + stub(bin.path(), &full_stub_body(ASSAY_NONE)); + let tree = tree_with_deed("(bot panicbot :deny (assay))"); + let o = scan(bin.path(), tree.path()); + assert!( + o.status.success(), + "stderr: {}", + String::from_utf8_lossy(&o.stderr) + ); + let v: serde_json::Value = serde_json::from_slice(&o.stdout).unwrap(); + assert!(v.is_array(), "{v}"); + let err = String::from_utf8_lossy(&o.stderr); + assert!( + err.contains("no target declared in the deed (bot-directives (bot panicbot (target"), + "{err}" + ); +} + +#[test] +fn a_deed_target_whose_build_fails_is_an_error_not_a_pass() { + let bin = TempDir::new().unwrap(); + stub(bin.path(), &full_stub_body(ASSAY_NONE)); + let tree = tree_with_deed( + "(bot panicbot (target :build \"echo nope >&2; exit 1\" :program \"bin/x\"))", + ); + let o = scan(bin.path(), tree.path()); + assert_eq!( + o.status.code(), + Some(3), + "stderr: {}", + String::from_utf8_lossy(&o.stderr) + ); + let v: serde_json::Value = serde_json::from_slice(&o.stdout).expect("an object"); + assert_eq!(v["status"], "error"); + assert!(v["reason"].as_str().unwrap().contains("assault"), "{v}"); +} + +#[test] +fn a_malformed_deed_clause_stops_the_run() { + let bin = TempDir::new().unwrap(); + stub(bin.path(), &full_stub_body(ASSAY_NONE)); + let tree = tree_with_deed("(bot panicbot :deny (assalt))"); + let o = scan(bin.path(), tree.path()); + assert!(!o.status.success()); + assert!(o.stdout.is_empty()); + assert!(String::from_utf8_lossy(&o.stderr).contains("bot directives unreadable")); +} diff --git a/docs/REPORT-BACK.adoc b/docs/REPORT-BACK.adoc index 9eae5daf..be29bbf2 100644 --- a/docs/REPORT-BACK.adoc +++ b/docs/REPORT-BACK.adoc @@ -118,9 +118,14 @@ that emits a path-less finding; rhodibot and seambot already comply. | panicbot | `panicbot --format json scan ` | Runs the **full panic-attack suite** unless the repository's bot directives - say otherwise (owner ruling 2026-11-13): `assail`, `assay`, and the dynamic - modes when a built `--target` is given (none of the CI callers gives one, so - there they report `not_applicable` with the reason). The inventory of every + say otherwise (owner rulings 2026-11-13): `assail`, `assay`, and the dynamic + modes (`assault`, `ambush`, then `adjudicate`) when the repo deed declares a + target, `(bot-directives (bot panicbot (target :build "..." :program "...")))`, + or a built `--target` is given. A declared `:build` is run first with `sh -c` + in the scanned repo under the suite budget; a failed build is an `error` part. + Under `pull_request_target` / `workflow_run` nothing is built or run. With no + target the dynamic parts report `not_applicable` with "no target declared in + the deed (bot-directives (bot panicbot (target …)))". The inventory of every panic-attack subcommand and why each is in or out is in `bots/panicbot/src/suite.rs`. panicbot's own translator maps critical → error, high → warning. The grid already carries a direct panic-attack row from the Hypatia lane, so a repository that enrols panicbot sees the same tool twice, @@ -135,10 +140,13 @@ that emits a path-less finding; rhodibot and seambot already comply. [...]}` and exits 3 (the failed part is named in the reason; the findings of the parts that ran are carried in the envelope file, but the grid reads only the status), and a run where no part analysed anything (everything denied or not - applicable) prints `status: not_run` and exits 4. A directive file that exists - but cannot be parsed stops the run (error), never a fallback to the full suite. - A repository with a directive file keeps exactly what it allows and denies; one - without gets the full suite. + applicable) prints `status: not_run` and exits 4. Directives are read from the repo deed + `_chora.deed` first (`(bot panicbot :allow ... :deny ... (target ...) + (config ...))`), then from the legacy `.machine_readable/bot_directives/panicbot.a2ml` + (read only, no target), else the full suite. A `deny` with no `allow` means the + full suite minus the deny; an explicit `allow` narrows; deny wins. A deed or + legacy file that exists but cannot be parsed (or a malformed clause) stops the + run (error), never a fallback to the full suite. | ponsbot | `ponsbot --format json scan ` From 86a33422d698f6199a6556698cb51cfb6e5a41ee Mon Sep 17 00:00:00 2001 From: CLAUDE <355856132+HoTT-stuff@users.noreply.github.com> Date: Fri, 13 Nov 2026 12:21:47 +0000 Subject: [PATCH 2/2] fix(panicbot): one spelling for the deed bot entry, (bot :name panicbot ...) The deed grammar rejects a bare symbol after bot (standards #1502), so the reader accepts only the :name form and the bare form is an error naming the canonical spelling. Tests, docs and the no-target reason use the canonical form. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- bots/panicbot/src/deed.rs | 22 ++-- bots/panicbot/src/directives.rs | 105 ++++++++++++-------- bots/panicbot/src/fleet.rs | 2 +- bots/panicbot/src/suite.rs | 8 +- bots/panicbot/tests/envelope_parity_test.rs | 20 +++- docs/REPORT-BACK.adoc | 6 +- 6 files changed, 99 insertions(+), 64 deletions(-) diff --git a/bots/panicbot/src/deed.rs b/bots/panicbot/src/deed.rs index d1a07ef9..dd0bbe14 100644 --- a/bots/panicbot/src/deed.rs +++ b/bots/panicbot/src/deed.rs @@ -14,10 +14,9 @@ //! literals, `'quoted` symbols and lists, and `( lists )`; //! - the whole input must be consumed. //! -//! One deliberate leniency: inside a `bot` clause a bare symbol is accepted as -//! an item, so both `(bot panicbot ...)` and `(bot :name panicbot ...)` read -//! (the standards fixture spells it the second way). Anywhere else a bare -//! symbol is an error. +//! There is one spelling of a bot entry: `(bot :name panicbot ...)`. A bare +//! symbol is not a field or a clause, so the grammar rejects `(bot panicbot ...)` +//! and so does this reader, with an error naming the canonical spelling. //! //! This is a reader, not the validator: `deed_lint` in standards stays the //! conformance gate (header, `:schema-version`, line length, UUID shape). The @@ -31,7 +30,7 @@ use std::fmt; pub struct Clause { /// The symbol after `(`. pub head: String, - /// Fields, nested clauses and (in `bot` only) bare symbols, in order. + /// Fields and nested clauses, in order. pub items: Vec, /// 1-based line of the `(`. pub line: usize, @@ -48,8 +47,6 @@ pub enum Item { }, /// A nested clause. Clause(Clause), - /// A bare symbol (only inside a `bot` clause). - Atom { name: String, line: usize }, } /// A field value. @@ -274,9 +271,10 @@ impl Parser { items.push(Item::Field { key, value, line }); } Some(c) if head == "bot" && is_sym_start(c) => { - let line = self.line; let name = self.symbol()?; - items.push(Item::Atom { name, line }); + return Err(self.err(&format!( + "bare symbol '{name}' in a (bot) clause: the canonical spelling is (bot :name {name} ...)" + ))); } Some(c) => { return Err(self.err(&format!( @@ -433,8 +431,10 @@ mod tests { } #[test] - fn bare_symbols_are_only_allowed_in_a_bot_clause() { - assert!(parse("(x (bot panicbot :a b))").is_ok()); + fn a_bare_symbol_is_an_error_everywhere_and_a_bot_clause_names_the_canonical_form() { + assert!(parse("(x (bot :name panicbot :a b))").is_ok()); + let e = parse("(x (bot panicbot :a b))").unwrap_err(); + assert!(e.message.contains("(bot :name panicbot ...)"), "{e}"); assert!(parse("(x (other panicbot))").is_err()); } diff --git a/bots/panicbot/src/directives.rs b/bots/panicbot/src/directives.rs index 99b9e0c9..f20e3f91 100644 --- a/bots/panicbot/src/directives.rs +++ b/bots/panicbot/src/directives.rs @@ -4,7 +4,7 @@ //! ## Where directives come from (owner rulings 2026-11-13) //! //! 1. **The repo deed**, `_chora.deed` at the scanned directory's root: -//! the `(bot panicbot ...)` entry inside `(bot-directives ...)`. Canonical. +//! the `(bot :name panicbot ...)` entry inside `(bot-directives ...)`. Canonical. //! 2. **The legacy file** `.machine_readable/bot_directives/panicbot.a2ml` //! (TOML-shaped), READ ONLY, used only when the deed has no panicbot entry. //! A2ML is retired: this module never writes or extends one. @@ -20,15 +20,15 @@ //! //! ```text //! (bot-directives -//! (bot panicbot +//! (bot :name panicbot //! :allow (assail assay diagnostics) ; optional; absent = full suite //! :deny (assault ambush) ; optional; deny wins over allow //! (target :build "cargo build --release" :program "target/release/foo") //! (config :min-severity low :timeout-seconds 300))) //! ``` //! -//! `(bot :name panicbot ...)` reads the same (the standards fixture spells it -//! so). Every part is optional except that a `target` needs `:program`. +//! The bot is named with `:name`, the one spelling (standards #1502); the bare +//! form `(bot panicbot ...)` is an error that says so. Every part is optional except that a `target` needs `:program`. //! Unknown fields, unknown subcommand names, a wrong value type, a duplicate //! entry and an ambiguous deed (more than one `*_chora.deed` at the root) are //! errors: the deed reader is strict where the legacy file only notes. @@ -83,17 +83,20 @@ pub fn load_directives(repo_path: &Path) -> Result { .with_context(|| format!("Failed to read {}", deed_path.display()))?; let root = deed::parse(&text) .with_context(|| format!("Failed to parse {}", deed_path.display()))?; - if let Some(mut config) = from_deed(&root) - .with_context(|| format!("Invalid (bot panicbot) clause in {}", deed_path.display()))? - { + if let Some(mut config) = from_deed(&root).with_context(|| { + format!( + "Invalid (bot :name panicbot) clause in {}", + deed_path.display() + ) + })? { if legacy_present { config.notes.push(format!( - "{DIRECTIVE_PATH} exists but is ignored: the deed's (bot panicbot) clause wins" + "{DIRECTIVE_PATH} exists but is ignored: the deed's (bot :name panicbot) clause wins" )); } return Ok(config); } - tracing::debug!("{} has no (bot panicbot) entry", deed_path.display()); + tracing::debug!("{} has no (bot :name panicbot) entry", deed_path.display()); } if !legacy_present { @@ -148,7 +151,7 @@ fn shape(line: usize, message: impl Into) -> anyhow::Error { }) } -/// Read the `(bot panicbot ...)` entry of a parsed deed. `None`: the deed has +/// Read the `(bot :name panicbot ...)` entry of a parsed deed. `None`: the deed has /// no such entry (not an error). Anything wrong inside an entry is an error. fn from_deed(root: &Clause) -> Result> { let mut sections = root.clauses("bot-directives"); @@ -166,7 +169,7 @@ fn from_deed(root: &Clause) -> Result> { return Err(shape( bot.line, format!( - "(bot panicbot) appears twice (first at line {})", + "(bot :name panicbot) appears twice (first at line {})", first.line ), )); @@ -187,7 +190,6 @@ fn from_deed(root: &Clause) -> Result> { for item in &bot.items { match item { - Item::Atom { .. } => {} // the name, checked by bot_name Item::Field { key, value, line } => { if seen.contains(&key.as_str()) { return Err(shape(*line, format!("duplicate :{key}"))); @@ -201,7 +203,7 @@ fn from_deed(root: &Clause) -> Result> { other => { return Err(shape( *line, - format!("unknown field :{other} in (bot panicbot)"), + format!("unknown field :{other} in (bot :name panicbot)"), )) } } @@ -223,7 +225,7 @@ fn from_deed(root: &Clause) -> Result> { other => { return Err(shape( c.line, - format!("unknown clause ({other} ...) in (bot panicbot)"), + format!("unknown clause ({other} ...) in (bot :name panicbot)"), )) } }, @@ -253,12 +255,11 @@ fn from_deed(root: &Clause) -> Result> { Ok(Some(config)) } -/// The bot's name: `:name SYM` or a single bare symbol. +/// The bot's name: its `:name SYM` field. fn bot_name(bot: &Clause) -> Result<&str> { let mut name: Option<&str> = None; for item in &bot.items { let (n, line) = match item { - Item::Atom { name, line } => (name.as_str(), *line), Item::Field { key, value: deed::Value::Sym(s), @@ -271,7 +272,12 @@ fn bot_name(bot: &Clause) -> Result<&str> { } name = Some(n); } - name.ok_or_else(|| shape(bot.line, "a (bot) clause needs a name: (bot NAME ...)")) + name.ok_or_else(|| { + shape( + bot.line, + "a (bot) clause needs a name: (bot :name NAME ...)", + ) + }) } fn subcommands(what: &str, value: &deed::Value, line: usize) -> Result> { @@ -328,7 +334,6 @@ fn read_target(c: &Clause) -> Result { format!("unexpected clause ({}) in (target)", n.head), )) } - Item::Atom { line, .. } => return Err(shape(*line, "unexpected symbol in (target)")), } } let program = @@ -709,7 +714,7 @@ mod deed_tests { #[test] fn deny_only_in_the_deed_is_the_full_suite_minus_the_deny() { let d = repo( - Some(&deed_for("(bot panicbot :deny (assault ambush))")), + Some(&deed_for("(bot :name panicbot :deny (assault ambush))")), None, ); let c = load_directives(d.path()).unwrap(); @@ -726,7 +731,7 @@ mod deed_tests { fn an_explicit_allow_narrows_and_deny_wins_over_it() { let d = repo( Some(&deed_for( - "(bot panicbot :allow (assail assay diagnostics) :deny (assay))", + "(bot :name panicbot :allow (assail assay diagnostics) :deny (assay))", )), None, ); @@ -759,7 +764,9 @@ mod deed_tests { #[test] fn a_target_without_a_build_is_allowed() { let d = repo( - Some(&deed_for("(bot panicbot (target :program \"bin/x\"))")), + Some(&deed_for( + "(bot :name panicbot (target :program \"bin/x\"))", + )), None, ); let c = load_directives(d.path()).unwrap(); @@ -769,7 +776,7 @@ mod deed_tests { #[test] fn the_deed_clause_wins_over_the_legacy_file() { let d = repo( - Some(&deed_for("(bot panicbot :deny (assay))")), + Some(&deed_for("(bot :name panicbot :deny (assay))")), Some("allow = [\"assail\"]\n"), ); let c = load_directives(d.path()).unwrap(); @@ -786,8 +793,8 @@ mod deed_tests { fn the_legacy_file_is_the_fallback_when_the_deed_has_no_panicbot_entry() { for deed in [ "(repo-deed :schema-version \"1.0.0\")\n".to_string(), - deed_for("(bot hypatia :deny (assay))"), - deed_for("(bot rhodibot)"), + deed_for("(bot :name hypatia :deny (assay))"), + deed_for("(bot :name rhodibot)"), ] { let d = repo(Some(&deed), Some("allow = [\"assail\"]\n")); let c = load_directives(d.path()).unwrap(); @@ -819,60 +826,60 @@ mod deed_tests { ), ( "unknown subcommand", - deed_for("(bot panicbot :deny (asault))"), + deed_for("(bot :name panicbot :deny (asault))"), ), - ("allow not a list", deed_for("(bot panicbot :allow assail)")), + ("allow not a list", deed_for("(bot :name panicbot :allow assail)")), ( "allow of strings", - deed_for("(bot panicbot :allow (\"assail\"))"), + deed_for("(bot :name panicbot :allow (\"assail\"))"), ), - ("unknown field", deed_for("(bot panicbot :dney (assault))")), + ("unknown field", deed_for("(bot :name panicbot :dney (assault))")), ( "unknown clause", - deed_for("(bot panicbot (targt :program \"x\"))"), + deed_for("(bot :name panicbot (targt :program \"x\"))"), ), ( "target without program", - deed_for("(bot panicbot (target :build \"make\"))"), + deed_for("(bot :name panicbot (target :build \"make\"))"), ), ( "build not a string", - deed_for("(bot panicbot (target :build make :program \"x\"))"), + deed_for("(bot :name panicbot (target :build make :program \"x\"))"), ), ( "empty build", - deed_for("(bot panicbot (target :build \"\" :program \"x\"))"), + deed_for("(bot :name panicbot (target :build \"\" :program \"x\"))"), ), ( "two targets", - deed_for("(bot panicbot (target :program \"a\") (target :program \"b\"))"), + deed_for("(bot :name panicbot (target :program \"a\") (target :program \"b\"))"), ), ( "bad severity", - deed_for("(bot panicbot (config :min-severity severe))"), + deed_for("(bot :name panicbot (config :min-severity severe))"), ), ( "bad timeout", - deed_for("(bot panicbot (config :timeout-seconds 0))"), + deed_for("(bot :name panicbot (config :timeout-seconds 0))"), ), ( "string timeout", - deed_for("(bot panicbot (config :timeout-seconds \"5\"))"), + deed_for("(bot :name panicbot (config :timeout-seconds \"5\"))"), ), ( "duplicate deny", - deed_for("(bot panicbot :deny (assay) :deny (assault))"), + deed_for("(bot :name panicbot :deny (assay) :deny (assault))"), ), ( "two panicbot entries", - deed_for("(bot panicbot) (bot panicbot)"), + deed_for("(bot :name panicbot) (bot :name panicbot)"), ), ("nameless bot", deed_for("(bot :deny (assay))")), ( "two bot-directives", format!( "{}{}", - "(repo-deed (bot-directives (bot panicbot)) (bot-directives (bot panicbot)))", + "(repo-deed (bot-directives (bot :name panicbot)) (bot-directives (bot :name panicbot)))", "\n" ), ), @@ -886,10 +893,26 @@ mod deed_tests { } } + #[test] + fn the_bare_bot_form_is_an_error_naming_the_canonical_spelling() { + for clause in ["(bot panicbot)", "(bot panicbot :deny (assay))"] { + let d = repo(Some(&deed_for(clause)), Some("allow = [\"assail\"]\n")); + let e = format!("{:#}", load_directives(d.path()).unwrap_err()); + assert!(e.contains("(bot :name panicbot ...)"), "{clause}: {e}"); + } + // Another bot written the bare way breaks the deed too: one spelling. + let d = repo(Some(&deed_for("(bot hypatia)")), None); + assert!(load_directives(d.path()).is_err()); + } + #[test] fn two_deeds_at_the_root_are_an_error() { - let d = repo(Some(&deed_for("(bot panicbot)")), None); - std::fs::write(d.path().join("y_chora.deed"), deed_for("(bot panicbot)")).unwrap(); + let d = repo(Some(&deed_for("(bot :name panicbot)")), None); + std::fs::write( + d.path().join("y_chora.deed"), + deed_for("(bot :name panicbot)"), + ) + .unwrap(); let e = load_directives(d.path()).unwrap_err(); assert!(format!("{e:#}").contains("more than one"), "{e:#}"); } diff --git a/bots/panicbot/src/fleet.rs b/bots/panicbot/src/fleet.rs index cb3023dd..5f4d16b7 100644 --- a/bots/panicbot/src/fleet.rs +++ b/bots/panicbot/src/fleet.rs @@ -3,7 +3,7 @@ //! //! Registers as `BotId::Panicbot` in gitbot-shared-context. Provides the //! standard fleet entry point (`run_fleet_scan`) that: -//! 1. Reads per-repo directives (the repo deed's `(bot panicbot ...)` clause, else the legacy `.machine_readable/bot_directives/panicbot.a2ml`) +//! 1. Reads per-repo directives (the repo deed's `(bot :name panicbot ...)` clause, else the legacy `.machine_readable/bot_directives/panicbot.a2ml`) //! 2. Runs the allowed panic-attack suite (full suite by default, see `suite`) //! 3. Translates each part's results into Finding structs (assail WeakPoints, //! assay candidates, dynamic-mode results when a target is given) diff --git a/bots/panicbot/src/suite.rs b/bots/panicbot/src/suite.rs index 71de56d7..23b31c0e 100644 --- a/bots/panicbot/src/suite.rs +++ b/bots/panicbot/src/suite.rs @@ -50,7 +50,7 @@ //! ## Dynamic modes: where the target comes from //! //! `assault` and `ambush` execute a program. The repository declares it in its -//! deed: `(bot-directives (bot panicbot (target :build "..." :program "...")))` +//! deed: `(bot-directives (bot :name panicbot (target :build "..." :program "...")))` //! (see [`crate::directives`]). If a `:build` is declared it is run first, once, //! with `sh -c` in the scanned directory, under a timeout drawn from the suite //! budget; a failed or timed-out build is an `error` on each dynamic part and @@ -59,7 +59,7 @@ //! `--target PATH` on the command line replaces the declared target (the build //! included: the operator is then responsible for the program existing). //! With no target from either, the dynamic parts report `not_applicable` with -//! "no target declared in the deed (bot-directives (bot panicbot (target …)))". +//! "no target declared in the deed (bot-directives (bot :name panicbot (target …)))". //! //! The security gate (checked 2026-11-13 over gitbot-fleet //! `fleet-report-back-reusable.yml` and standards `fork-governance.yml`): both @@ -753,7 +753,7 @@ fn budget_msg(config: &PanicbotConfig) -> String { } fn no_target_msg() -> String { - "no target declared in the deed (bot-directives (bot panicbot (target …)))".into() + "no target declared in the deed (bot-directives (bot :name panicbot (target …)))".into() } #[cfg(test)] @@ -1000,7 +1000,7 @@ mod tests { for cmd in [Subcommand::Assault, Subcommand::Ambush] { assert_eq!(status_of(&run, cmd), PartStatus::NotApplicable); assert!(detail_of(&run, cmd).contains( - "no target declared in the deed (bot-directives (bot panicbot (target …)))" + "no target declared in the deed (bot-directives (bot :name panicbot (target …)))" )); } for cmd in [ diff --git a/bots/panicbot/tests/envelope_parity_test.rs b/bots/panicbot/tests/envelope_parity_test.rs index 1e43903c..680a9b58 100644 --- a/bots/panicbot/tests/envelope_parity_test.rs +++ b/bots/panicbot/tests/envelope_parity_test.rs @@ -447,7 +447,7 @@ fn tree_with_deed(clause: &str) -> TempDir { fn a_deed_deny_only_runs_the_rest_and_no_target_says_so_in_the_deed_terms() { let bin = TempDir::new().unwrap(); stub(bin.path(), &full_stub_body(ASSAY_NONE)); - let tree = tree_with_deed("(bot panicbot :deny (assay))"); + let tree = tree_with_deed("(bot :name panicbot :deny (assay))"); let o = scan(bin.path(), tree.path()); assert!( o.status.success(), @@ -458,7 +458,7 @@ fn a_deed_deny_only_runs_the_rest_and_no_target_says_so_in_the_deed_terms() { assert!(v.is_array(), "{v}"); let err = String::from_utf8_lossy(&o.stderr); assert!( - err.contains("no target declared in the deed (bot-directives (bot panicbot (target"), + err.contains("no target declared in the deed (bot-directives (bot :name panicbot (target"), "{err}" ); } @@ -468,7 +468,7 @@ fn a_deed_target_whose_build_fails_is_an_error_not_a_pass() { let bin = TempDir::new().unwrap(); stub(bin.path(), &full_stub_body(ASSAY_NONE)); let tree = tree_with_deed( - "(bot panicbot (target :build \"echo nope >&2; exit 1\" :program \"bin/x\"))", + "(bot :name panicbot (target :build \"echo nope >&2; exit 1\" :program \"bin/x\"))", ); let o = scan(bin.path(), tree.path()); assert_eq!( @@ -486,9 +486,21 @@ fn a_deed_target_whose_build_fails_is_an_error_not_a_pass() { fn a_malformed_deed_clause_stops_the_run() { let bin = TempDir::new().unwrap(); stub(bin.path(), &full_stub_body(ASSAY_NONE)); - let tree = tree_with_deed("(bot panicbot :deny (assalt))"); + let tree = tree_with_deed("(bot :name panicbot :deny (assalt))"); let o = scan(bin.path(), tree.path()); assert!(!o.status.success()); assert!(o.stdout.is_empty()); assert!(String::from_utf8_lossy(&o.stderr).contains("bot directives unreadable")); } + +#[test] +fn the_bare_bot_spelling_stops_the_run_and_names_the_canonical_one() { + let bin = TempDir::new().unwrap(); + stub(bin.path(), &full_stub_body(ASSAY_NONE)); + let tree = tree_with_deed("(bot panicbot :deny (assay))"); + let o = scan(bin.path(), tree.path()); + assert!(!o.status.success()); + assert!(o.stdout.is_empty()); + let err = String::from_utf8_lossy(&o.stderr); + assert!(err.contains("(bot :name panicbot ...)"), "{err}"); +} diff --git a/docs/REPORT-BACK.adoc b/docs/REPORT-BACK.adoc index be29bbf2..4a8f8746 100644 --- a/docs/REPORT-BACK.adoc +++ b/docs/REPORT-BACK.adoc @@ -120,12 +120,12 @@ that emits a path-less finding; rhodibot and seambot already comply. | Runs the **full panic-attack suite** unless the repository's bot directives say otherwise (owner rulings 2026-11-13): `assail`, `assay`, and the dynamic modes (`assault`, `ambush`, then `adjudicate`) when the repo deed declares a - target, `(bot-directives (bot panicbot (target :build "..." :program "...")))`, + target, `(bot-directives (bot :name panicbot (target :build "..." :program "...")))`, or a built `--target` is given. A declared `:build` is run first with `sh -c` in the scanned repo under the suite budget; a failed build is an `error` part. Under `pull_request_target` / `workflow_run` nothing is built or run. With no target the dynamic parts report `not_applicable` with "no target declared in - the deed (bot-directives (bot panicbot (target …)))". The inventory of every + the deed (bot-directives (bot :name panicbot (target …)))". The inventory of every panic-attack subcommand and why each is in or out is in `bots/panicbot/src/suite.rs`. panicbot's own translator maps critical → error, high → warning. The grid already carries a direct panic-attack row from the Hypatia lane, so a repository that enrols panicbot sees the same tool twice, @@ -141,7 +141,7 @@ that emits a path-less finding; rhodibot and seambot already comply. the parts that ran are carried in the envelope file, but the grid reads only the status), and a run where no part analysed anything (everything denied or not applicable) prints `status: not_run` and exits 4. Directives are read from the repo deed - `_chora.deed` first (`(bot panicbot :allow ... :deny ... (target ...) + `_chora.deed` first (`(bot :name panicbot :allow ... :deny ... (target ...) (config ...))`), then from the legacy `.machine_readable/bot_directives/panicbot.a2ml` (read only, no target), else the full suite. A `deny` with no `allow` means the full suite minus the deny; an explicit `allow` narrows; deny wins. A deed or