fix(gate): SKIPPED/NEUTRAL required checks satisfy, as Skipped

crates/squabble-cli/src/fetch.rs

@@ -15,7 +15,9 @@
 //!
 //! A required context with no matching rollup entry is [`CheckRun::Missing`];
 //! matching-but-incomplete is [`CheckRun::Pending`]; a `SUCCESS` conclusion is
-//! [`CheckRun::Passed`]; anything else that completed is [`CheckRun::Failed`].
+//! [`CheckRun::Passed`]; `SKIPPED`/`NEUTRAL` is [`CheckRun::Skipped`] (satisfies
+//! the ruleset, carries no evidence); anything else that completed is
+//! [`CheckRun::Failed`].
 
 use serde::Deserialize;
 use squabble_core::gate::{CheckRun, Gate, RequiredCheck};
@@ -63,21 +65,15 @@ struct RulesetContext {
     context: String,
 }
 
-/// Parse a `gh pr view --json baseRefName,statusCheckRollup` payload into the
-/// realised-run half of a [`Gate`]. Pure — no IO, fully testable on fixtures.
+/// Classify one entry from `gh pr view`'s `statusCheckRollup`.
+///
+/// Recognised conclusions take precedence over status: `SUCCESS` yields
+/// [`CheckRun::Passed`], `SKIPPED` or `NEUTRAL` yields [`CheckRun::Skipped`],
+/// and `FAILURE`, `ERROR`, `TIMED_OUT`, `CANCELLED` or `STARTUP_FAILURE` yields
+/// [`CheckRun::Failed`]. With an absent or unrecognised conclusion, `COMPLETED`
+/// status yields [`CheckRun::Failed`]; any other status yields [`CheckRun::Pending`].
 fn parse_rollup(entry: &RollupEntry) -> CheckRun {
-    match entry.conclusion.as_deref() {
-        Some("SUCCESS") => CheckRun::Passed,
-        Some("FAILURE")
-        | Some("ERROR")
-        | Some("TIMED_OUT")
-        | Some("CANCELLED")
-        | Some("STARTUP_FAILURE") => CheckRun::Failed,
-        _ => match entry.status.as_deref() {
-            Some("COMPLETED") => CheckRun::Failed, // completed with no recognised conclusion
-            _ => CheckRun::Pending,
-        },
-    }
+    CheckRun::from_github(entry.status.as_deref(), entry.conclusion.as_deref())
 }
 
 /// Build a [`Gate`] from the required-context set and the realised rollup.
@@ -87,11 +83,12 @@ fn build_gate(required_contexts: &[String], rollup: &[RollupEntry]) -> Gate {
     let checks = required_contexts
         .iter()
         .map(|required| {
-            let run = rollup
-                .iter()
-                .find(|r| &r.name == required)
-                .map(parse_rollup)
-                .unwrap_or(CheckRun::Missing);
+            let run = CheckRun::for_context(
+                rollup
+                    .iter()
+                    .filter(|r| &r.name == required)
+                    .map(parse_rollup),
+            );
             RequiredCheck::new(required.clone(), run)
         })
         .collect();
@@ -645,6 +642,32 @@ pub fn run_bundle(slug: &str, pr: &str) -> Result<FetchBundle, FetchError> {
     })
 }
 
+/// Every rule type the base branch's rulesets carry, deduplicated, in order.
+fn rule_types_from_json(rules_json: &str) -> Result<Vec<String>, String> {
+    let rules: Vec<RulesetRule> = serde_json::from_str(rules_json)
+        .map_err(|e| format!("could not parse ruleset response: {e}"))?;
+    let mut out: Vec<String> = Vec::new();
+    for t in rules.into_iter().map(|r| r.rule_type) {
+        if !out.contains(&t) {
+            out.push(t);
+        }
+    }
+    Ok(out)
+}
+
+/// The base branch's gate as `verify-satisfied` needs it: the required-context
+/// union (rulesets ∪ classic protection) and every ruleset rule type.
+///
+/// Unlike [`run_bundle`] an empty context set is *not* `NoGate` here — "done"
+/// is still a meaningful question on an ungated branch. A 403 on classic
+/// protection is still a hard error, for the same vacuous-green reason.
+pub fn base_gate(slug: &str, branch: &str) -> Result<(Vec<String>, Vec<String>), FetchError> {
+    let rules_json = run_gh(&["api", &format!("repos/{slug}/rules/branches/{branch}")])?;
+    let protection = probe_classic_protection(slug, branch)?;
+    let contexts = required_contexts_from_apis(&rules_json, &protection)?;
+    Ok((contexts, rule_types_from_json(&rules_json)?))
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -947,6 +970,20 @@ mod tests {
         );
     }
 
+    #[test]
+    fn skipped_and_neutral_map_to_skipped_not_failed() {
+        // Both COMPLETED with a conclusion GitHub accepts for a required check;
+        // before the Skipped variant they fell through to Failed and the gate
+        // read Red on PRs GitHub would merge.
+        for c in ["SKIPPED", "NEUTRAL"] {
+            assert_eq!(
+                parse_rollup(&entry("x", Some("COMPLETED"), Some(c))),
+                CheckRun::Skipped,
+                "{c}"
+            );
+        }
+    }
+
     #[test]
     fn in_progress_maps_to_pending() {
         assert_eq!(

crates/squabble-cli/src/main.rs

@@ -18,6 +18,8 @@
 //! - `2` — a genuine failure: bad usage, `gh` failed, unparseable input.
 //! - `4` — **blocking chain finding** (`squabble chains`): a dependency cycle
 //!   or a dead upstream. Like `3`, a reportable finding, not a tool failure.
+//! - `5` — **not done** (`squabble verify-satisfied`): agent items remain on
+//!   the PR (unresolved threads, a failing required check, automerge unarmed…).
 //! - `6` — **incomplete board** (`squabble board`): rendered and published, but
 //!   a repo was unreadable or had more open PRs than one page; the gap is
 //!   stated at the top of the board.
@@ -39,6 +41,7 @@ mod chains;
 mod fetch;
 mod fight;
 mod inbox;
+mod verify;
 
 use squabble_core::{diagnose, gate::Gate};
 use std::process::ExitCode;
@@ -62,6 +65,7 @@ fn main() -> ExitCode {
         },
         Some("fight") => fight::run(&args[2..]),
         Some("chains") => chains::run(&args[2..]),
+        Some("verify-satisfied") => verify::run(&args[2..]),
         Some("board") => board::run(&args[2..]),
         Some("inbox-sweep") => inbox::run(&args[2..]),
         Some("--version") | Some("-V") => {
@@ -80,14 +84,16 @@ fn main() -> ExitCode {
                  squabble {}\n  \
                  squabble {}\n  \
                  squabble {}\n  \
+                 squabble {}\n  \
                  squabble --version\n\n\
                  EXIT CODES:\n  \
-                 0 ok · 2 failure · 3 no `required_status_checks` rule on the base branch · 4 chains: blocking finding · 6 board or inbox-sweep: incomplete\n",
+                 0 ok · 2 failure · 3 no `required_status_checks` rule on the base branch · 4 chains: blocking finding · 5 verify-satisfied: agent items remain · 6 board or inbox-sweep: incomplete\n",
                 env!("CARGO_PKG_VERSION"),
                 fight::USAGE.trim_start_matches("usage: squabble "),
                 chains::USAGE.trim_start_matches("usage: squabble "),
                 board::USAGE.trim_start_matches("usage: squabble "),
-                inbox::USAGE.trim_start_matches("usage: squabble ")
+                inbox::USAGE.trim_start_matches("usage: squabble "),
+                verify::USAGE.trim_start_matches("usage: squabble ")
             );
             ExitCode::from(2)
         }

crates/squabble-cli/src/verify.rs

@@ -0,0 +1,157 @@
+// SPDX-License-Identifier: MPL-2.0
+// Copyright (c) 2026 Jonathan D.A. Jewell (hyperpolymath) <j.d.a.jewell@open.ac.uk>
+//! `squabble verify-satisfied <owner/repo> <pr>` — is this PR actually done?
+//!
+//! Reads the PR (GraphQL, `squabble-forge::pr_done`) and its base branch's
+//! gate (REST, `fetch::base_gate`), then asks the pure evaluator in
+//! `squabble-core::done`. The verdict goes to stdout as JSON; a human-readable
+//! list goes to stderr. Exit `5` means agent work remains — the one answer a
+//! hook needs, so hook and human read the same implementation.
+
+use crate::fetch;
+use serde::Serialize;
+use squabble_core::done::{evaluate, PrState, Verdict, DEFAULT_REVIEW_APPS};
+use squabble_forge::pr_done::fetch_pr_done;
+use squabble_forge::GhTransport;
+use std::process::ExitCode;
+
+pub const USAGE: &str = "usage: squabble verify-satisfied <owner>/<repo> <pr-number>";
+
+/// Exit code for "the PR is not done: agent items remain".
+pub const NOT_DONE_EXIT: u8 = 5;
+
+#[derive(Serialize)]
+struct Report<'a> {
+    repo: &'a str,
+    pr: u64,
+    head_oid: String,
+    base_ref: String,
+    done: bool,
+    #[serde(flatten)]
+    verdict: Verdict,
+}
+
+pub fn run(args: &[String]) -> ExitCode {
+    let (slug, pr) = match args {
+        [slug, pr] => match (slug.split_once('/'), pr.parse::<u64>()) {
+            (Some(_), Ok(n)) => (slug.as_str(), n),
+            _ => {
+                eprintln!("squabble verify-satisfied: {USAGE}");
+                return ExitCode::from(2);
+            }
+        },
+        _ => {
+            eprintln!("squabble verify-satisfied: {USAGE}");
+            return ExitCode::from(2);
+        }
+    };
+    let (owner, name) = slug.split_once('/').expect("checked above");
+
+    let read = match fetch_pr_done(&GhTransport, owner, name, pr) {
+        Ok(r) => r,
+        Err(e) => {
+            eprintln!("squabble verify-satisfied: {e}");
+            return ExitCode::from(2);
+        }
+    };
+    let mut facts = read.facts;
+    // The base gate only bears on a PR that can still merge: for a merged or
+    // closed PR it feeds nothing but the evidence-free listing, never an agent
+    // item (pinned by `the_base_gate_never_decides_a_closed_or_merged_verdict`).
+    // Skipping the REST call there keeps the commonest done-claim ("landed X")
+    // off the rate limit this PAT shares with every other session.
+    if facts.state == PrState::Open {
+        match fetch::base_gate(slug, &read.base_ref) {
+            Ok((contexts, rule_types)) => {
+                facts.required_contexts = contexts;
+                facts.rule_types = rule_types;
+            }
+            Err(e) => {
+                eprintln!("squabble verify-satisfied: {e}");
+                return ExitCode::from(fetch::FetchError::FAILED_EXIT);
+            }
+        }
+    }
+
+    let verdict = evaluate(&facts, DEFAULT_REVIEW_APPS);
+    let done = verdict.is_done();
+    eprint!("{}", render(slug, pr, &verdict));
+    let report = Report {
+        repo: slug,
+        pr,
+        head_oid: read.head_oid,
+        base_ref: read.base_ref,
+        done,
+        verdict,
+    };
+    match serde_json::to_string_pretty(&report) {
+        Ok(json) => println!("{json}"),
+        Err(e) => {
+            eprintln!("squabble verify-satisfied: could not serialise verdict: {e}");
+            return ExitCode::from(2);
+        }
+    }
+    if done {
+        ExitCode::SUCCESS
+    } else {
+        ExitCode::from(NOT_DONE_EXIT)
+    }
+}
+
+fn render(slug: &str, pr: u64, v: &Verdict) -> String {
+    let mut s = format!(
+        "{slug}#{pr}: {}\n",
+        if v.is_done() {
+            "DONE (nothing left for the agent)"
+        } else {
+            "NOT DONE"
+        }
+    );
+    let mut section = |title: &str, lines: Vec<String>| {
+        if !lines.is_empty() {
+            s.push_str(&format!("  {title}:\n"));
+            for l in lines {
+                s.push_str(&format!("    - {l}\n"));
+            }
+        }
+    };
+    section(
+        "agent must act",
+        v.agent_items.iter().map(|i| i.describe()).collect(),
+    );
+    section(
+        "held by a human",
+        v.held_by_human.iter().map(|i| i.describe()).collect(),
+    );
+    section(
+        "satisfied without evidence (skipped/neutral)",
+        v.evidence_free.clone(),
+    );
+    section("notes", v.notes.clone());
+    s
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// A hook keys on this number; it must not collide with any other code the
+    /// binary already uses (2 failure, 3 no gate, 4 chains blocking).
+    #[test]
+    fn not_done_has_its_own_exit_code() {
+        for other in [0u8, 2, 3, 4] {
+            assert_ne!(NOT_DONE_EXIT, other);
+        }
+    }
+
+    #[test]
+    fn malformed_arguments_are_a_usage_failure_not_a_verdict() {
+        for bad in [
+            vec![],
+            vec!["norepo".to_string(), "1".to_string()],
+            vec!["o/r".to_string(), "x".to_string()],
+        ] {
+            assert_eq!(run(&bad), ExitCode::from(2));
+        }
+    }
+}