fix(verify): report unverified commits under required_signatures

CHANGELOG.adoc

@@ -46,6 +46,12 @@ https://semver.org/spec/v2.0.0.html[Semantic Versioning].
 
 ==== Fixed
 
+* verify-satisfied: a PR with an unsigned or unverified commit no longer
+  reads as done when the base requires signed commits. GitHub blocks that
+  merge, squash included; it is now the agent item `unverified_commits`,
+  naming each commit with its signature state and author. Every PR commit's
+  signature is read, paged. A classic-protection `required_signatures`
+  counts as well as a ruleset one.
 * workflows: step-level `- uses:` refs were never read; only job-level
   `uses:` keys were. The issue #15 Actions-policy probes therefore missed every
   step action. Two existing tests witnessed it and were failing on `main`.

crates/squabble-cli/src/fetch.rs

@@ -197,6 +197,14 @@ enum ProtectionProbe {
 #[derive(Debug, Deserialize)]
 struct ClassicProtection {
     required_status_checks: Option<ClassicRequiredChecks>,
+    /// `{"enabled": bool}`; read as not required when absent.
+    required_signatures: Option<ClassicEnabled>,
+}
+
+/// The `{url, enabled}` shape classic protection uses for a toggle setting.
+#[derive(Debug, Deserialize)]
+struct ClassicEnabled {
+    enabled: bool,
 }
 
 #[derive(Debug, Deserialize)]
@@ -273,6 +281,15 @@ fn parse_classic_contexts(json: &str) -> Result<Vec<String>, String> {
     Ok(out)
 }
 
+/// Whether a classic-protection payload requires signed commits. The rules
+/// API reports ruleset rules only, so this is the one place a classic
+/// `required_signatures` becomes visible.
+fn classic_requires_signatures(json: &str) -> Result<bool, String> {
+    let p: ClassicProtection = serde_json::from_str(json)
+        .map_err(|e| format!("could not parse branch-protection response: {e}"))?;
+    Ok(p.required_signatures.is_some_and(|s| s.enabled))
+}
+
 /// The union of required contexts from the two protection APIs, in
 /// enforcement order: ruleset contexts first, then classic-only ones.
 /// `Hidden` is a hard error even when the ruleset yielded contexts — a gate
@@ -656,7 +673,8 @@ fn rule_types_from_json(rules_json: &str) -> Result<Vec<String>, String> {
 }
 
 /// The base branch's gate as `verify-satisfied` needs it: the required-context
-/// union (rulesets ∪ classic protection) and every ruleset rule type.
+/// union (rulesets ∪ classic protection) and every ruleset rule type, plus
+/// `required_signatures` when classic protection requires signed commits.
 ///
 /// 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
@@ -665,7 +683,15 @@ pub fn base_gate(slug: &str, branch: &str) -> Result<(Vec<String>, Vec<String>),
     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)?))
+    let mut rule_types = rule_types_from_json(&rules_json)?;
+    if let ProtectionProbe::Protected(json) = &protection {
+        if classic_requires_signatures(json)?
+            && !rule_types.iter().any(|t| t == "required_signatures")
+        {
+            rule_types.push("required_signatures".into());
+        }
+    }
+    Ok((contexts, rule_types))
 }
 
 #[cfg(test)]
@@ -899,6 +925,20 @@ mod tests {
         assert_eq!(probe_from_stderr("gh: validation failed"), None);
     }
 
+    /// Only `enabled: true` turns the rule on; an absent key or `false` leaves it off.
+    #[test]
+    fn classic_required_signatures_is_read_only_when_enabled() {
+        let on = r#"{"required_signatures": {"url": "u", "enabled": true}}"#;
+        let off = r#"{"required_signatures": {"url": "u", "enabled": false}}"#;
+        assert!(classic_requires_signatures(on).expect("parse"));
+        assert!(!classic_requires_signatures(off).expect("parse"));
+        assert!(
+            !classic_requires_signatures(r#"{"enforce_admins": {"enabled": true}}"#)
+                .expect("parse")
+        );
+        assert!(classic_requires_signatures("not json").is_err());
+    }
+
     #[test]
     fn classic_checks_shape_is_read_without_contexts() {
         // Some payloads carry only the newer `checks` objects.

crates/squabble-core/src/done.rs

@@ -42,8 +42,12 @@ pub const DEFAULT_REVIEW_APPS: &[&str] = &[
 ];
 
 /// Ruleset rule types whose effect this gate evaluates, or which cannot hold a
-/// squash merge (a squash lands one GitHub-signed commit, so `required_signatures`
-/// and `required_linear_history` are met by construction).
+/// squash merge. `required_linear_history` is met by construction: a squash
+/// lands one commit. `required_signatures` is **not**: GitHub holds the PR
+/// `BLOCKED` while any commit on it lacks a verified signature, squash armed or
+/// not (measured 2026-10-02 on boj-server-cartridges#155: automerge armed,
+/// blocked by one unsigned `coderabbitai[bot]` autofix commit). It is evaluated
+/// over [`PrFacts::unverified_commits`].
 const ACCOUNTED_RULE_TYPES: &[&str] = &[
     "required_status_checks",
     "pull_request",
@@ -95,6 +99,17 @@ pub struct Thread {
     pub url: String,
 }
 
+/// A commit on the PR whose signature GitHub does not verify.
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub struct UnverifiedCommit {
+    pub oid: String,
+    /// GitHub's `GitSignatureState` (`INVALID`, `UNKNOWN_KEY`, …), or
+    /// `UNSIGNED` when the commit carries no signature at all.
+    pub state: String,
+    /// The author's login (`[bot]` removed), else their git name.
+    pub author: String,
+}
+
 /// Everything the verdict needs, fetched once.
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 pub struct PrFacts {
@@ -115,8 +130,13 @@ pub struct PrFacts {
     pub unresolved_threads: Vec<Thread>,
     /// Reviewers whose latest opinionated review is `CHANGES_REQUESTED`.
     pub changes_requested_by: Vec<String>,
-    /// `[.[].type]` of the effective rules on the base branch.
+    /// `[.[].type]` of the effective rules on the base branch, plus
+    /// `required_signatures` when classic branch protection requires it.
     pub rule_types: Vec<String>,
+    /// Commits on the PR that GitHub does not mark verified, from a complete
+    /// read of the PR's commits. Agent work only under `required_signatures`.
+    #[serde(default)]
+    pub unverified_commits: Vec<UnverifiedCommit>,
     /// The PR description, verbatim. Where a red non-required check is
     /// acknowledged (see [`acknowledged_in`]). Empty when there is none.
     pub body: String,
@@ -138,6 +158,7 @@ pub enum Item {
     WrongMergeMethod { method: String },
     BranchBehind,
     MergeQueue,
+    UnverifiedCommits { commits: Vec<UnverifiedCommit> },
     AwaitingApproval { review_decision: String },
     AwaitingDeployment,
     AwaitingHumanMerge { merge_state: String },
@@ -189,6 +210,23 @@ impl Item {
             Self::MergeQueue => "base branch uses a merge queue — verify-satisfied cannot \
                  evaluate queue entry, so it refuses rather than pass vacuously"
                 .into(),
+            Self::UnverifiedCommits { commits } => format!(
+                "the base requires signed commits and {} commit(s) here are not verified \
+                 ({}) — GitHub blocks the merge, squash included; re-sign them on the \
+                 branch (`git rebase --exec 'git commit --amend --no-edit -S' <merge-base>`) \
+                 and push with --force-with-lease",
+                commits.len(),
+                commits
+                    .iter()
+                    .map(|c| format!(
+                        "{} {} by {}",
+                        c.oid.get(..10).unwrap_or(&c.oid),
+                        c.state,
+                        c.author
+                    ))
+                    .collect::<Vec<_>>()
+                    .join(", ")
+            ),
             Self::AwaitingApproval { review_decision } => {
                 format!("awaiting a human review (reviewDecision={review_decision})")
             }
@@ -408,6 +446,15 @@ pub fn evaluate(facts: &PrFacts, review_apps: &[&str]) -> Verdict {
     if facts.rule_types.iter().any(|t| t == "required_deployments") {
         human.push(Item::AwaitingDeployment);
     }
+    // A squash does not satisfy `required_signatures` (see ACCOUNTED_RULE_TYPES):
+    // every commit on the PR must verify, and re-signing them is branch work.
+    if facts.rule_types.iter().any(|t| t == "required_signatures")
+        && !facts.unverified_commits.is_empty()
+    {
+        agent.push(Item::UnverifiedCommits {
+            commits: facts.unverified_commits.clone(),
+        });
+    }
     let unaccounted: Vec<&str> = facts
         .rule_types
         .iter()
@@ -511,10 +558,20 @@ mod tests {
             unresolved_threads: vec![],
             changes_requested_by: vec![],
             rule_types: vec!["required_status_checks".into(), "deletion".into()],
+            unverified_commits: vec![],
             body: String::new(),
         }
     }
 
+    /// A commit with no signature at all, authored by the CodeRabbit bot.
+    fn unsigned(oid: &str) -> UnverifiedCommit {
+        UnverifiedCommit {
+            oid: oid.into(),
+            state: "UNSIGNED".into(),
+            author: "coderabbitai".into(),
+        }
+    }
+
     fn kinds(items: &[Item]) -> Vec<String> {
         items
             .iter()
@@ -779,6 +836,53 @@ mod tests {
         );
     }
 
+    /// The planted positive: an armed squash PR that GitHub holds BLOCKED on one
+    /// unsigned bot commit (boj-server-cartridges#155, 2026-10-02) must not read
+    /// as done.
+    #[test]
+    fn an_unverified_commit_under_required_signatures_is_agent_work() {
+        let mut f = done_pr();
+        f.rule_types.push("required_signatures".into());
+        f.unverified_commits.push(unsigned("8546edbde6aa"));
+        let v = evaluate(&f, DEFAULT_REVIEW_APPS);
+        assert_eq!(kinds(&v.agent_items), ["unverified_commits"]);
+        let line = v.agent_items[0].describe();
+        assert!(line.contains("8546edbde6"), "{line}");
+        assert!(line.contains("--force-with-lease"), "{line}");
+        assert!(
+            !v.notes.iter().any(|n| n.contains("required_signatures")),
+            "an accounted rule type must not also be reported as unevaluated: {:?}",
+            v.notes
+        );
+    }
+
+    /// The rule is on, but every commit verifies: nothing is owed.
+    #[test]
+    fn verified_commits_under_required_signatures_are_done() {
+        let mut f = done_pr();
+        f.rule_types.push("required_signatures".into());
+        assert!(evaluate(&f, DEFAULT_REVIEW_APPS).is_done());
+    }
+
+    /// Without the rule, GitHub merges unsigned commits, so they are no work.
+    #[test]
+    fn an_unverified_commit_without_required_signatures_is_not_agent_work() {
+        let mut f = done_pr();
+        f.unverified_commits.push(unsigned("8546edbde6aa"));
+        assert!(evaluate(&f, DEFAULT_REVIEW_APPS).is_done());
+    }
+
+    /// After the merge the signatures can no longer be changed, so they are no work.
+    #[test]
+    fn a_merged_pr_owes_nothing_for_its_unverified_commits() {
+        let mut f = done_pr();
+        f.state = PrState::Merged;
+        f.auto_merge = None;
+        f.rule_types.push("required_signatures".into());
+        f.unverified_commits.push(unsigned("8546edbde6aa"));
+        assert!(evaluate(&f, DEFAULT_REVIEW_APPS).is_done());
+    }
+
     /// `verify-satisfied` skips the base-gate REST read for a PR that is no
     /// longer open. That is sound only while the base gate cannot move a closed
     /// or merged verdict's agent items — pin it, with a gate that would fail an

crates/squabble-forge/graphql/pr_done.graphql

@@ -2,12 +2,15 @@
 # Copyright (c) 2026 Jonathan D.A. Jewell (hyperpolymath) <j.d.a.jewell@open.ac.uk>
 #
 # Everything `squabble verify-satisfied` reads about one pull request, in one
-# query. Two connections page independently (head contexts, review threads):
-# `fetch_pr_done` in src/pr_done.rs re-issues this document with each cursor until
-# both are exhausted and ignores a finished connection's repeat nodes. Heads in
-# this estate carry up to ~95 contexts; an unpaginated read reports present
-# checks as absent, which would pass a pending review bot vacuously.
-query PrDone($owner: String!, $name: String!, $number: Int!, $ctxAfter: String, $thrAfter: String) {
+# query. Three connections page independently (head contexts, review threads,
+# the PR's commits): `fetch_pr_done` in src/pr_done.rs re-issues this document
+# with each cursor until all are exhausted and ignores a finished connection's
+# repeat nodes. Heads in this estate carry up to ~95 contexts; an unpaginated
+# read reports present checks as absent, which would pass a pending review bot
+# vacuously. `prCommits` is aliased because `commits(last: 1)` already reads the
+# head; every commit on the PR is read for `required_signatures`, which GitHub
+# applies to all of them, not to the squash it lands.
+query PrDone($owner: String!, $name: String!, $number: Int!, $ctxAfter: String, $thrAfter: String, $cmtAfter: String) {
   rateLimit { cost remaining resetAt }
   repository(owner: $owner, name: $name) {
     pullRequest(number: $number) {
@@ -33,6 +36,16 @@ query PrDone($owner: String!, $name: String!, $number: Int!, $ctxAfter: String,
           comments(first: 1) { nodes { author { login } url } }
         }
       }
+      prCommits: commits(first: 100, after: $cmtAfter) {
+        pageInfo { hasNextPage endCursor }
+        nodes {
+          commit {
+            oid
+            author { name user { login } }
+            signature { isValid state }
+          }
+        }
+      }
       commits(last: 1) {
         nodes {
           commit {