From 8ac9108761641b535255916f0b9b012e8c45746e Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Fri, 4 Sep 2026 00:45:06 +0100 Subject: [PATCH 1/7] feat(core): classify green checks that never actually ran MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `squabble fight` classifies only RED checks, so a gate that could not run reports green and is never inspected. That is the whole fake-green class: a scanner goes missing, a stub writes `[]`, the check goes green, and the gate silently stops being a gate. Adds `squabble-core::polarity`: Genuine | NotApplicable | Vacuous. * Evidence tier is declared and singular — Actions jobs-API step conclusions. Causes needing workflow YAML or script source are absent; a variant we cannot witness from the declared source would be a taxonomy, not a classifier. * Scanner-agnostic. The signature is host-supplied from `gate_triage.a2ml`; no scanner name appears in this crate. ANCHOR declares `hypatia-dependent` an IS-NOT. * Axis 0 (applicability) is checked first and is THREE-way: undeclared falls through, declared-and-matched falls through, only declared-and-contradicted is NotApplicable. Treating undeclared as NotApplicable would make this classifier its own fake green (planted break: 10 of 21 tests fail). * The signature is a CONJUNCTION. A skipped scan with no stub-writing step is a legitimately optional step, not vacuity. * Never recommends deletion from a stub-rate: at run_count == 1 the rate is exactly 0.0 or 1.0, so one stubbed run would read as "useless everywhere". The directive reserves that judgement to the owner, so the rate is reported as evidence instead. Adds `ExpertGroup::GateTriage` so vacuity has a consumer — an enum nothing reads is itself a fake gate. It deliberately names no estate service: `boj::route` now returns `Option` and yields None for it, recorded as an honest non-dispatch rather than a silent skip. A wildcard arm would have routed owner-facing findings to hypatia by the back door. SPARK is untouched: gate_machine.ads models only Check_Run/Gate_State/ Evaluate, not this crate the check annotations, so `Green IFF non-empty AND all Passed` holds bit-for-bit. Witnessed by a Rust test asserting `Gate::evaluate()` is unchanged across classification. Note: `boj` is an off-by-default cargo feature, so plain `cargo build` never compiled it and missed the non-exhaustive match. Verified with --all-features. Refs #39, #58 --- crates/squabble-cli/src/boj.rs | 71 ++- crates/squabble-core/src/lib.rs | 1 + crates/squabble-core/src/moves.rs | 7 + crates/squabble-core/src/polarity.rs | 753 +++++++++++++++++++++++++++ 4 files changed, 821 insertions(+), 11 deletions(-) create mode 100644 crates/squabble-core/src/polarity.rs diff --git a/crates/squabble-cli/src/boj.rs b/crates/squabble-cli/src/boj.rs index 89b2d98..fd103ff 100644 --- a/crates/squabble-cli/src/boj.rs +++ b/crates/squabble-cli/src/boj.rs @@ -136,15 +136,19 @@ struct ExpertCall { /// obligation never reroutes to a different one (a HypatiaFleet escalation /// with a `scan` obligation is still hypatia's case, possibly *using* a /// scan). The routing is deliberately host-side data, not core types. -fn route(e: &Escalation, repo_path: &str) -> ExpertCall { +/// +/// Returns `None` for a group with **no cartridge**. That is not a gap to be +/// filled with a wildcard arm: routing `GateTriage` to hypatia would reintroduce +/// the hypatia dependency this repo declares as an IS-NOT, by the back door. +fn route(e: &Escalation, repo_path: &str) -> Option { match e.group { - ExpertGroup::Security => ExpertCall { + ExpertGroup::Security => Some(ExpertCall { cartridge: "panic-attack-mcp", tool: "panic_attack_scan", arguments: serde_json::json!({ "path": repo_path }), meaning: "weak-point scan", - }, - ExpertGroup::Proof => ExpertCall { + }), + ExpertGroup::Proof => Some(ExpertCall { cartridge: "echidna-llm-mcp", tool: "consult", arguments: serde_json::json!({ @@ -154,16 +158,20 @@ fn route(e: &Escalation, repo_path: &str) -> ExpertCall { ) }), meaning: "proof consultation", - }, + }), // Hypatia / HypatiaFleet: hypatia assesses; the fleet cartridge only // *tracks* gate results. Actuation (fix + PR) has no cartridge today // and stays external — recorded as such. - ExpertGroup::Hypatia | ExpertGroup::HypatiaFleet => ExpertCall { + ExpertGroup::Hypatia | ExpertGroup::HypatiaFleet => Some(ExpertCall { cartridge: "hypatia-mcp", tool: "hypatia_scan_repo", arguments: serde_json::json!({ "path": repo_path }), meaning: "hypatia assessment (actuation external — no fixer cartridge yet)", - }, + }), + // A vacuous-gate finding is the owner's to discharge. No cartridge can + // assess whether a gate is worth keeping — the `gate_triage` directive + // reserves that judgement to the owner. + ExpertGroup::GateTriage => None, } } @@ -218,7 +226,27 @@ pub fn summon(outcome: &mut Outcome, repo_root: &Path) { continue; } }; - let call = route(e, repo_path); + // `no-silent-skip`: a group with no cartridge is recorded as an honest + // non-dispatch, not quietly passed over. The escalation stands. + let Some(call) = route(e, repo_path) else { + e.evidence + .push_str(" | summon: owner-facing group — no cartridge to dispatch to"); + report.blockers.push(format!( + "summon `{}`: owner-facing group — no cartridge, escalation stands", + e.check + )); + verdicts.push(ExpertVerdict { + check: e.check.clone(), + cartridge: "(none)".to_string(), + tool: "(none)".to_string(), + ok: false, + meaning: "owner-facing; no cartridge".to_string(), + verdict: + "not dispatched — the gate_triage directive reserves this judgement to the owner" + .to_string(), + }); + continue; + }; match client.invoke(call.cartridge, call.tool, call.arguments) { Ok(v) => { e.evidence.push_str(&format!( @@ -281,12 +309,14 @@ mod tests { let sec = route( &escalation(ExpertGroup::Security, EscalationKind::Scan), "/r", - ); + ) + .expect("Security routes to a cartridge"); assert_eq!(sec.cartridge, "panic-attack-mcp"); let proof = route( &escalation(ExpertGroup::Proof, EscalationKind::VerifyClaim), "/r", - ); + ) + .expect("Proof routes to a cartridge"); assert_eq!(proof.cartridge, "echidna-llm-mcp"); // The obligation must never reroute away from the planner's chosen // specialist: HypatiaFleet stays hypatia's case even for scan/verify. @@ -296,12 +326,31 @@ mod tests { EscalationKind::DispatchFix, EscalationKind::AssessConfidence, ] { - let fleet = route(&escalation(ExpertGroup::HypatiaFleet, obligation), "/r"); + let fleet = route(&escalation(ExpertGroup::HypatiaFleet, obligation), "/r") + .expect("HypatiaFleet routes to a cartridge"); assert_eq!(fleet.cartridge, "hypatia-mcp"); assert!(fleet.meaning.contains("actuation external")); } } + #[test] + fn gate_triage_has_no_cartridge() { + // ANCHOR: `hypatia-dependent` is an IS-NOT, and the gate_triage directive + // sets `fallback-must-be-standalone = true`. A wildcard arm here would + // have routed owner-facing findings to hypatia by the back door. + for obligation in [ + EscalationKind::Scan, + EscalationKind::VerifyClaim, + EscalationKind::DispatchFix, + EscalationKind::AssessConfidence, + ] { + assert!( + route(&escalation(ExpertGroup::GateTriage, obligation), "/r").is_none(), + "GateTriage must never dispatch to a cartridge" + ); + } + } + #[test] fn body_error_detects_estate_error_shapes() { // The boj router unwraps cartridge failures into 200 bodies; both diff --git a/crates/squabble-core/src/lib.rs b/crates/squabble-core/src/lib.rs index a1aba73..c6db4f0 100644 --- a/crates/squabble-core/src/lib.rs +++ b/crates/squabble-core/src/lib.rs @@ -18,6 +18,7 @@ pub mod admission; pub mod gate; pub mod moves; pub mod outcome; +pub mod polarity; use gate::{Gate, GateState}; use moves::{LicenceFinding, Move}; diff --git a/crates/squabble-core/src/moves.rs b/crates/squabble-core/src/moves.rs index 46a4f49..d41fac8 100644 --- a/crates/squabble-core/src/moves.rs +++ b/crates/squabble-core/src/moves.rs @@ -144,6 +144,12 @@ pub enum ExpertGroup { Proof, /// Static-analysis / weak-point scanning (estate: `security/panic-attack-mcp`). Security, + /// Gate triage — a **green** check that is not really green. Owner-facing: + /// deliberately names no estate service, because `cicd-squabbler` declares + /// `hypatia-dependent` as an IS-NOT and the `gate_triage` directive sets + /// `fallback-must-be-standalone = true`. There is no cartridge to dispatch + /// to; the obligation is the owner's to discharge. + GateTriage, } /// What the squabbler is asking an [`ExpertGroup`] to *do*. Kept coarse on @@ -188,6 +194,7 @@ impl ExpertGroup { ExpertGroup::HypatiaFleet => "hypatia+fleet (analyse+fix)", ExpertGroup::Proof => "proof (echidna)", ExpertGroup::Security => "security (panic-attack)", + ExpertGroup::GateTriage => "gate-triage (owner-facing)", } } } diff --git a/crates/squabble-core/src/polarity.rs b/crates/squabble-core/src/polarity.rs new file mode 100644 index 0000000..72181d4 --- /dev/null +++ b/crates/squabble-core/src/polarity.rs @@ -0,0 +1,753 @@ +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2026 Jonathan D.A. Jewell (hyperpolymath) +//! The **green-polarity** classifier — the gap `fight` cannot see. +//! +//! `squabble fight` classifies only *red* checks. A gate that could not run +//! reports **green**, so the engine never inspects it. That is the whole +//! fake-green class: a scanner goes missing, a stub writes `[]`, the check goes +//! green, and the gate silently stops being a gate. +//! +//! This module classifies a check that concluded `success` as one of: +//! +//! * [`PolarityVerdict::Genuine`] — it really ran, +//! * [`PolarityVerdict::NotApplicable`] — it is correctly inapplicable *by +//! declaration* (never an escalation; inapplicable is not broken), +//! * [`PolarityVerdict::Vacuous`] — it went green without doing its job. +//! +//! # Evidence tier +//! +//! Every variant here is witnessable from **one** source: the Actions jobs API +//! step conclusions (`.steps[].{name,conclusion}`). Causes that would need the +//! workflow YAML or a script's source are deliberately absent — a variant we +//! cannot witness from the declared source would be a taxonomy, not a +//! classifier. +//! +//! # Standalone +//! +//! `cicd-squabbler`'s ANCHOR declares `hypatia-dependent` as an **IS-NOT**, and +//! the `gate_triage` directive sets `fallback-must-be-standalone = true`. So the +//! signature is *host-supplied* ([`VacuitySignature`], read from +//! `gate_triage.a2ml`) and no scanner name appears anywhere in this file. +//! +//! # Why the SPARK theorem is untouched +//! +//! `spark/src/gate_machine.ads` models only `Check_Run`, `Gate_State` and +//! `Evaluate`, whose postcondition is `Green IFF non-empty AND all Passed`. It +//! does not mirror this crate's check annotations. Nothing here calls +//! [`crate::gate::Gate::evaluate`] or changes a [`crate::gate::CheckRun`], so +//! the proved invariant holds bit-for-bit — the same precedent that admitted +//! `MissingCause`. + +use serde::{Deserialize, Serialize}; + +use crate::moves::{EscalationKind, ExpertGroup, Move}; + +/// A single step's conclusion as reported by the Actions jobs API. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum StepConclusion { + Success, + Skipped, + Failure, + Cancelled, + /// Anything the API reports that we do not model (including `null`). + Other, +} + +impl StepConclusion { + /// Parse the jobs-API string. Unknown and absent values are `Other` rather + /// than an error: an unmodelled conclusion must never be mistaken for a + /// skip, because a skip is half of the vacuity signature. + pub fn parse(raw: Option<&str>) -> Self { + match raw { + Some("success") => StepConclusion::Success, + Some("skipped") => StepConclusion::Skipped, + Some("failure") => StepConclusion::Failure, + Some("cancelled") => StepConclusion::Cancelled, + _ => StepConclusion::Other, + } + } +} + +/// One step of a job that concluded `success`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct StepOutcome { + pub name: String, + pub conclusion: StepConclusion, +} + +impl StepOutcome { + pub fn new(name: impl Into, conclusion: StepConclusion) -> Self { + StepOutcome { + name: name.into(), + conclusion, + } + } +} + +/// The host-supplied vacuity fingerprint, read from the `gate_triage.a2ml` +/// keys `signature-skipped-steps` and `signature-success-steps`. +/// +/// **Scanner-agnostic by construction**: no step name is compiled in. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct VacuitySignature { + /// Step names that must have concluded `skipped` — "the real work did not run". + pub skipped_steps: Vec, + /// Step names that must have concluded `success` — "something wrote a stub". + pub success_steps: Vec, +} + +impl VacuitySignature { + pub fn new(skipped: &[&str], success: &[&str]) -> Self { + VacuitySignature { + skipped_steps: skipped.iter().map(|s| (*s).to_string()).collect(), + success_steps: success.iter().map(|s| (*s).to_string()).collect(), + } + } + + /// A signature is *usable* only when **both** halves are populated. + /// + /// This is the false-positive guard. A skipped scan step with no + /// corresponding stub-writing step is a legitimately optional step, not + /// vacuity — and an empty list would match vacuously, turning every green + /// into a finding. + pub fn is_usable(&self) -> bool { + !self.skipped_steps.is_empty() && !self.success_steps.is_empty() + } +} + +/// What a gate declares about *where it applies* — the directive's +/// `@gitforge_OperatorType` / `@channel` axis. +/// +/// Empty lists mean **undeclared**, which is not the same as unmatched. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct Applicability { + pub runs_for_operator_types: Vec, + pub runs_on_channels: Vec, +} + +impl Applicability { + /// True when the gate carries no applicability predicate at all. Today no + /// gate does, so this is the common case — and treating it as + /// `NotApplicable` would make this classifier its own fake green. + pub fn is_undeclared(&self) -> bool { + self.runs_for_operator_types.is_empty() && self.runs_on_channels.is_empty() + } +} + +/// What the repo declares about itself, read from `0.1-AI-MANIFEST.a2ml`. +/// `None` on either field means the manifest is silent on that key. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct RepoDeclaration { + pub operator_type: Option, + pub channel: Option, +} + +/// The four evidence fields issue #58 requires of every verdict. +#[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)] +pub struct Evidence { + /// How many runs of this check were inspected. + pub run_count: u32, + /// Fraction of those runs that were stubbed, in `0.0..=1.0`. + pub stub_rate: f64, + /// Does the tool this gate invokes exist upstream at all? + pub upstream_exists: bool, + /// Is the technology this gate scans for still present in the tree? + pub target_tech_present: bool, +} + +impl Evidence { + /// Rendered into [`Move::EscalateToExpert::evidence`] so the owner can make + /// the case-2 call the squabbler is forbidden to make for them. + pub fn describe(&self) -> String { + format!( + "run-count={} stub-rate={:.2} upstream-exists={} target-tech-present={}", + self.run_count, self.stub_rate, self.upstream_exists, self.target_tech_present + ) + } +} + +/// Why a green check is not really green. **Step-observable only.** +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum VacuityCause { + /// The directive's signature matched: the real step was skipped *and* a + /// stub-writing step succeeded. (Measured in the wild: 4 repos.) + StubbedAfterSkippedScan, + /// The job concluded success with steps recorded, every one of them skipped. + AllStepsSkipped, + /// The job concluded success having recorded no steps at all. + NoStepsRecorded, +} + +impl VacuityCause { + pub const fn label(self) -> &'static str { + match self { + VacuityCause::StubbedAfterSkippedScan => "stubbed after a skipped scan", + VacuityCause::AllStepsSkipped => "every step skipped", + VacuityCause::NoStepsRecorded => "no steps recorded", + } + } +} + +/// What the squabbler recommends the owner *do*. These are the computable +/// subset of the directive's five cases. +/// +/// Case 2 (`useless-everywhere` → assess-value-then-delete) is **absent on +/// purpose**: the directive reserves that assessment to the owner, and it is +/// not decidable from a stub-rate. With `run_count == 1` the rate is exactly +/// `0.0` or `1.0`, so a single stubbed run would read as "useless everywhere". +/// The rate is reported as evidence instead. +/// +/// Case 4 (`missing-local-tooling`, the trufflehog/pre-push class) is also +/// absent: it is not observable from the jobs API, so folding it in here would +/// be a claim this detector cannot witness. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum Recommendation { + /// Case 1 — the technology this gate scans for is gone. Computable. + RecommendRemoval, + /// Case 5 — the tool is declared but does not exist upstream. + /// "Should never happen"; the declaration is what is wrong. + FixTheDeclaration, + /// Case 3 — the gate is valuable but does not resolve here. Make it work. + MakeItGreatInPractice, +} + +impl Recommendation { + pub const fn label(self) -> &'static str { + match self { + Recommendation::RecommendRemoval => "recommend-removal", + Recommendation::FixTheDeclaration => "fix-the-declaration", + Recommendation::MakeItGreatInPractice => "make-it-great-in-practice", + } + } + + /// Select the case from the four required evidence fields. Total function. + pub const fn from_evidence(e: &Evidence) -> Self { + if !e.target_tech_present { + Recommendation::RecommendRemoval + } else if !e.upstream_exists { + Recommendation::FixTheDeclaration + } else { + // Deliberately unconditional on stub_rate — see the type doc. + Recommendation::MakeItGreatInPractice + } + } +} + +/// The verdict on one check that concluded `success`. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(tag = "verdict", rename_all = "kebab-case")] +pub enum PolarityVerdict { + /// The check ran and its green means what it says. + Genuine, + /// Correctly inapplicable *by declaration*. Never escalated: inapplicable + /// is not broken (`unmatched-is-vacuous = false`). + NotApplicable { declaration: String }, + /// Green without doing its job. + Vacuous { + cause: VacuityCause, + recommendation: Recommendation, + evidence: Evidence, + }, +} + +impl PolarityVerdict { + /// The consumer. An enum nothing reads is itself a fake gate, so every + /// `Vacuous` verdict projects to a move the report already prints. + /// + /// The move is [`Move::EscalateToExpert`], which by its own contract + /// promotes no check and drops no required context — so surfacing a vacuous + /// gate can never itself move the gate to green. `Genuine` and + /// `NotApplicable` project to nothing. + pub fn to_move(&self, check: &str) -> Option { + match self { + PolarityVerdict::Genuine | PolarityVerdict::NotApplicable { .. } => None, + PolarityVerdict::Vacuous { + cause, + recommendation, + evidence, + } => Some(Move::EscalateToExpert { + check: check.to_string(), + group: ExpertGroup::GateTriage, + obligation: EscalationKind::AssessConfidence, + evidence: format!( + "green but vacuous ({}) — {} [{}]", + cause.label(), + evidence.describe(), + recommendation.label() + ), + }), + } + } +} + +/// Does `steps` contain `name` with exactly `want`? +fn step_concluded(steps: &[StepOutcome], name: &str, want: StepConclusion) -> bool { + steps + .iter() + .any(|s| s.name.trim() == name.trim() && s.conclusion == want) +} + +/// Axis 0 — applicability, checked **first** and three-way. +/// +/// * gate undeclared → `None` (fall through to the signature), +/// * gate declared and repo matches → `None` (fall through), +/// * gate declared and repo **contradicts** it → `Some(NotApplicable)`. +/// +/// A repo silent on the key cannot contradict anything, so it falls through. +/// This is fail-open into a path that can only *escalate*, never green. +fn applicability_verdict( + applicability: &Applicability, + declared: &RepoDeclaration, +) -> Option { + if applicability.is_undeclared() { + return None; + } + + if let Some(op) = declared.operator_type.as_deref() { + if !applicability.runs_for_operator_types.is_empty() + && !applicability + .runs_for_operator_types + .iter() + .any(|t| t == op) + { + return Some(PolarityVerdict::NotApplicable { + declaration: format!( + "@gitforge_OperatorType={op} not in [{}]", + applicability.runs_for_operator_types.join(", ") + ), + }); + } + } + + if let Some(ch) = declared.channel.as_deref() { + if !applicability.runs_on_channels.is_empty() + && !applicability.runs_on_channels.iter().any(|c| c == ch) + { + return Some(PolarityVerdict::NotApplicable { + declaration: format!( + "@channel={ch} not in [{}]", + applicability.runs_on_channels.join(", ") + ), + }); + } + } + + None +} + +/// Classify one check that concluded `success`. +/// +/// Never call this on a red check — `fight` already handles those, and this +/// module has no opinion about them. +pub fn classify( + steps: &[StepOutcome], + signature: &VacuitySignature, + applicability: &Applicability, + declared: &RepoDeclaration, + evidence: Evidence, +) -> PolarityVerdict { + // Axis 0 first: inapplicable is not broken. + if let Some(v) = applicability_verdict(applicability, declared) { + return v; + } + + let cause = if steps.is_empty() { + Some(VacuityCause::NoStepsRecorded) + } else if signature.is_usable() + // CONJUNCTION. A partial match is a legitimately optional step. + && signature + .skipped_steps + .iter() + .all(|n| step_concluded(steps, n, StepConclusion::Skipped)) + && signature + .success_steps + .iter() + .all(|n| step_concluded(steps, n, StepConclusion::Success)) + { + Some(VacuityCause::StubbedAfterSkippedScan) + } else if steps.iter().all(|s| s.conclusion == StepConclusion::Skipped) { + Some(VacuityCause::AllStepsSkipped) + } else { + None + }; + + match cause { + None => PolarityVerdict::Genuine, + Some(cause) => PolarityVerdict::Vacuous { + cause, + recommendation: Recommendation::from_evidence(&evidence), + evidence, + }, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::gate::{CheckRun, Gate, GateState, RequiredCheck}; + + /// The measured real-world signature, supplied the way the host supplies + /// it — as data, never compiled in. + fn sig() -> VacuitySignature { + VacuitySignature::new(&["Run Hypatia scan"], &["Create stub findings"]) + } + + fn ev(tech: bool, upstream: bool, rate: f64) -> Evidence { + Evidence { + run_count: 4, + stub_rate: rate, + upstream_exists: upstream, + target_tech_present: tech, + } + } + + fn classify_steps(steps: &[StepOutcome]) -> PolarityVerdict { + classify( + steps, + &sig(), + &Applicability::default(), + &RepoDeclaration::default(), + ev(true, true, 1.0), + ) + } + + // ---- the signature is a CONJUNCTION ------------------------------------- + + #[test] + fn both_halves_matching_is_vacuous() { + let v = classify_steps(&[ + StepOutcome::new("Checkout", StepConclusion::Success), + StepOutcome::new("Run Hypatia scan", StepConclusion::Skipped), + StepOutcome::new("Create stub findings", StepConclusion::Success), + ]); + assert!( + matches!( + v, + PolarityVerdict::Vacuous { + cause: VacuityCause::StubbedAfterSkippedScan, + .. + } + ), + "got {v:?}" + ); + } + + #[test] + fn skipped_step_without_a_stub_step_is_genuine() { + // The false-positive guard. A skipped step with nothing writing a stub + // is a legitimately optional step, not a vacuous gate. + let v = classify_steps(&[ + StepOutcome::new("Checkout", StepConclusion::Success), + StepOutcome::new("Run Hypatia scan", StepConclusion::Skipped), + ]); + assert_eq!(v, PolarityVerdict::Genuine, "got {v:?}"); + } + + #[test] + fn stub_step_without_a_skipped_scan_is_genuine() { + // The scan really ran; a stub step succeeding alongside it proves + // nothing. + let v = classify_steps(&[ + StepOutcome::new("Run Hypatia scan", StepConclusion::Success), + StepOutcome::new("Create stub findings", StepConclusion::Success), + ]); + assert_eq!(v, PolarityVerdict::Genuine, "got {v:?}"); + } + + #[test] + fn an_empty_signature_never_matches() { + // An empty list is vacuously "all matched", which would turn every + // green into a finding. `is_usable` is what stops that. + let v = classify( + &[StepOutcome::new("Build", StepConclusion::Success)], + &VacuitySignature::default(), + &Applicability::default(), + &RepoDeclaration::default(), + ev(true, true, 0.0), + ); + assert_eq!(v, PolarityVerdict::Genuine, "got {v:?}"); + } + + #[test] + fn a_half_populated_signature_is_not_usable() { + assert!(!VacuitySignature::new(&["a"], &[]).is_usable()); + assert!(!VacuitySignature::new(&[], &["b"]).is_usable()); + assert!(VacuitySignature::new(&["a"], &["b"]).is_usable()); + } + + // ---- the other two step-observable causes ------------------------------- + + #[test] + fn no_steps_recorded_is_vacuous() { + let v = classify_steps(&[]); + assert!( + matches!( + v, + PolarityVerdict::Vacuous { + cause: VacuityCause::NoStepsRecorded, + .. + } + ), + "got {v:?}" + ); + } + + #[test] + fn every_step_skipped_is_vacuous() { + let v = classify_steps(&[ + StepOutcome::new("Checkout", StepConclusion::Skipped), + StepOutcome::new("Build", StepConclusion::Skipped), + ]); + assert!( + matches!( + v, + PolarityVerdict::Vacuous { + cause: VacuityCause::AllStepsSkipped, + .. + } + ), + "got {v:?}" + ); + } + + #[test] + fn a_check_whose_steps_ran_is_genuine() { + let v = classify_steps(&[ + StepOutcome::new("Checkout", StepConclusion::Success), + StepOutcome::new("Build", StepConclusion::Success), + ]); + assert_eq!(v, PolarityVerdict::Genuine, "got {v:?}"); + } + + #[test] + fn an_unmodelled_conclusion_is_not_a_skip() { + // A skip is half the signature, so anything we do not model must never + // be mistaken for one. + assert_eq!(StepConclusion::parse(None), StepConclusion::Other); + assert_eq!(StepConclusion::parse(Some("neutral")), StepConclusion::Other); + assert_eq!( + StepConclusion::parse(Some("skipped")), + StepConclusion::Skipped + ); + } + + // ---- Axis 0 is THREE-WAY ------------------------------------------------ + + #[test] + fn declared_and_unmatched_is_not_applicable() { + let v = classify( + &[], // would otherwise be NoStepsRecorded — applicability wins + &sig(), + &Applicability { + runs_for_operator_types: vec!["platform_maintainer".into()], + runs_on_channels: vec![], + }, + &RepoDeclaration { + operator_type: Some("user".into()), + channel: None, + }, + ev(true, true, 1.0), + ); + assert!( + matches!(v, PolarityVerdict::NotApplicable { .. }), + "inapplicable is not broken; got {v:?}" + ); + } + + #[test] + fn an_unmatched_channel_is_also_not_applicable() { + let v = classify( + &[], + &sig(), + &Applicability { + runs_for_operator_types: vec![], + runs_on_channels: vec!["nightly".into(), "alpha".into()], + }, + &RepoDeclaration { + operator_type: None, + channel: Some("release".into()), + }, + ev(true, true, 1.0), + ); + assert!( + matches!(v, PolarityVerdict::NotApplicable { .. }), + "got {v:?}" + ); + } + + #[test] + fn undeclared_applicability_falls_through_to_the_signature() { + // THE critical case: today no gate carries an applicability predicate. + // Treating undeclared as NotApplicable would make every green + // not-applicable — this classifier would become its own fake green. + let v = classify( + &[ + StepOutcome::new("Run Hypatia scan", StepConclusion::Skipped), + StepOutcome::new("Create stub findings", StepConclusion::Success), + ], + &sig(), + &Applicability::default(), + &RepoDeclaration { + operator_type: Some("user".into()), + channel: Some("release".into()), + }, + ev(true, true, 1.0), + ); + assert!( + matches!(v, PolarityVerdict::Vacuous { .. }), + "undeclared must not short-circuit; got {v:?}" + ); + } + + #[test] + fn declared_and_matched_falls_through_to_the_signature() { + let v = classify( + &[ + StepOutcome::new("Run Hypatia scan", StepConclusion::Skipped), + StepOutcome::new("Create stub findings", StepConclusion::Success), + ], + &sig(), + &Applicability { + runs_for_operator_types: vec!["developer".into()], + runs_on_channels: vec!["alpha".into()], + }, + &RepoDeclaration { + operator_type: Some("developer".into()), + channel: Some("alpha".into()), + }, + ev(true, true, 1.0), + ); + assert!(matches!(v, PolarityVerdict::Vacuous { .. }), "got {v:?}"); + } + + #[test] + fn a_repo_silent_on_the_key_cannot_contradict_a_declaration() { + // Silence is not a mismatch. Falling through is fail-open into a path + // that can only escalate, never green. + let v = classify( + &[StepOutcome::new("Build", StepConclusion::Success)], + &sig(), + &Applicability { + runs_on_channels: vec!["nightly".into()], + runs_for_operator_types: vec![], + }, + &RepoDeclaration::default(), + ev(true, true, 0.0), + ); + assert_eq!(v, PolarityVerdict::Genuine, "got {v:?}"); + } + + // ---- recommendation selection ------------------------------------------ + + #[test] + fn absent_technology_recommends_removal() { + assert_eq!( + Recommendation::from_evidence(&ev(false, true, 1.0)), + Recommendation::RecommendRemoval + ); + } + + #[test] + fn a_tool_that_does_not_exist_upstream_is_a_declaration_bug() { + assert_eq!( + Recommendation::from_evidence(&ev(true, false, 1.0)), + Recommendation::FixTheDeclaration + ); + } + + #[test] + fn a_full_stub_rate_never_recommends_deletion() { + // The case-2 guard. With run_count == 1 the rate is exactly 0.0 or 1.0, + // so a single stubbed run would otherwise read as "useless everywhere" + // — a judgement the directive reserves to the owner. + for rate in [0.0, 0.25, 0.5, 1.0] { + assert_eq!( + Recommendation::from_evidence(&ev(true, true, rate)), + Recommendation::MakeItGreatInPractice, + "stub_rate {rate} must not change the recommendation" + ); + } + } + + #[test] + fn the_evidence_string_carries_all_four_required_fields() { + let d = ev(true, true, 0.75).describe(); + for field in [ + "run-count", + "stub-rate", + "upstream-exists", + "target-tech-present", + ] { + assert!(d.contains(field), "`{field}` missing from `{d}`"); + } + } + + // ---- the consumer (an enum nothing reads is itself a fake gate) --------- + + #[test] + fn a_vacuous_verdict_projects_to_a_gate_triage_escalation() { + let v = classify_steps(&[ + StepOutcome::new("Run Hypatia scan", StepConclusion::Skipped), + StepOutcome::new("Create stub findings", StepConclusion::Success), + ]); + let m = v.to_move("scan / hypatia").expect("vacuous must project"); + match m { + Move::EscalateToExpert { + check, + group, + obligation, + evidence, + } => { + assert_eq!(check, "scan / hypatia"); + assert_eq!(group, ExpertGroup::GateTriage); + assert_eq!(obligation, EscalationKind::AssessConfidence); + assert!(evidence.contains("run-count"), "got `{evidence}`"); + assert!(evidence.contains("stub-rate"), "got `{evidence}`"); + } + other => panic!("must never self-win; got {other:?}"), + } + } + + #[test] + fn genuine_and_not_applicable_project_to_nothing() { + assert!(PolarityVerdict::Genuine.to_move("c").is_none()); + assert!( + PolarityVerdict::NotApplicable { + declaration: "d".into() + } + .to_move("c") + .is_none() + ); + } + + // ---- the Rust witness of the SPARK theorem ------------------------------ + + #[test] + fn classification_does_not_move_the_gate() { + // gate_machine.ads proves `Green IFF non-empty AND all Passed`. + // Nothing in this module touches a CheckRun, so a vacuous verdict on a + // green gate must leave `evaluate()` exactly where it was. + let gate = Gate::new(vec![ + RequiredCheck::new("scan / hypatia", CheckRun::Passed), + RequiredCheck::new("build", CheckRun::Passed), + ]); + let before = gate.evaluate(); + assert_eq!(before, GateState::Green); + + let v = classify_steps(&[ + StepOutcome::new("Run Hypatia scan", StepConclusion::Skipped), + StepOutcome::new("Create stub findings", StepConclusion::Success), + ]); + assert!(matches!(v, PolarityVerdict::Vacuous { .. })); + let _ = v.to_move("scan / hypatia"); + + assert_eq!( + gate.evaluate(), + before, + "polarity classification must never move the proved gate state" + ); + } +} From faa678a993dcfa7376179c6408368231c1c90d40 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Fri, 4 Sep 2026 00:55:15 +0100 Subject: [PATCH 2/7] feat(fight): surface green checks that never actually ran MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `squabble fight` classified only RED checks, so a gate that could not run reported green and was never inspected. That is the whole fake-green class: a scanner goes missing, a stub writes `[]`, the check goes green, and the gate silently stops being a gate. Host wiring for the classifier landed in 8ac9108: - `fetch.rs` reads each successful check's job id out of `detailsUrl` (the rollup exposes no job id field) and pulls step conclusions from `repos/{slug}/actions/jobs/{id}`. Status contexts with no job are skipped rather than guessed at. - `fight.rs` classifies those greens against the repo's own `gate_triage.a2ml` signature and folds any vacuity finding into the report's escalations. A gate with no usable signature costs zero API calls, so this is fail-safe by default. - A fully green gate yields `Outcome::Green`, which carries no `Report` and so has no typed home for the finding. Per issue #58's go/no-go ("if it needs a new state, write a spec first and stop"), that path reports on stderr — visible in both human and `--json` modes — with the limitation named in the message. Not a silent skip. `gate_triage.a2ml` goes draft -> active, and now records which evidence fields the host can actually measure. Two of the four cannot be measured from the jobs API, so they render `unmeasured` and can never reach the destructive recommendation. Verified: cargo test --workspace --all-features (112 passed) and cargo clippy --workspace --all-features --all-targets -- -D warnings, both run unpiped so the exit code is real. --- .../bot_directives/gate_triage.a2ml | 15 +- crates/squabble-cli/src/fetch.rs | 213 +++++++++++++++++- crates/squabble-cli/src/fight.rs | 105 ++++++++- crates/squabble-core/src/polarity.rs | 67 ++++-- crates/squabble-fight/src/context.rs | 2 +- crates/squabble-fight/src/gate_triage.rs | 96 ++++++++ crates/squabble-fight/src/lib.rs | 1 + 7 files changed, 477 insertions(+), 22 deletions(-) create mode 100644 crates/squabble-fight/src/gate_triage.rs diff --git a/.machine_readable/bot_directives/gate_triage.a2ml b/.machine_readable/bot_directives/gate_triage.a2ml index 49535e9..faebfee 100644 --- a/.machine_readable/bot_directives/gate_triage.a2ml +++ b/.machine_readable/bot_directives/gate_triage.a2ml @@ -9,10 +9,10 @@ # Origin: owner ruling 2026-09-03, given on the vacuous-Hypatia-gate options menu. [metadata] -version = "0.1.0" -last-updated = "2026-09-03" +version = "0.2.0" +last-updated = "2026-09-04" spec = "https://github.com/hyperpolymath/standards/blob/main/agentic-a2ml/docs/ADR-002-methodology-layer.adoc" -status = "draft" # not yet wired into the fight engine; see issue +status = "active" # wired into `squabble fight`; see crates/squabble-core/src/polarity.rs # ============================================================================ # THE CORE PRINCIPLE @@ -175,3 +175,12 @@ signature-success-steps = ["Create stub findings"] check-conclusion-when-vacuous = "success" polarity = "green" # NOT red — this is the gap in `fight` evidence-required = ["run-count", "stub-rate", "upstream-exists", "target-tech-present"] + +# Measurability, stated honestly (Doctrine #10, no overclaim). +# Of the four fields above, the actions jobs API supplies only the first two. +# `upstream-exists` and `target-tech-present` need a repo-tree probe that no +# gate currently declares the globs for, so `squabble fight` renders them +# `unmeasured` rather than guessing — and an unmeasured field can never reach +# the destructive recommendation. Removal advice waits for real measurement. +evidence-measurable-by-host = ["run-count", "stub-rate"] +evidence-unmeasured-by-host = ["upstream-exists", "target-tech-present"] diff --git a/crates/squabble-cli/src/fetch.rs b/crates/squabble-cli/src/fetch.rs index c9ad4d3..9becea4 100644 --- a/crates/squabble-cli/src/fetch.rs +++ b/crates/squabble-cli/src/fetch.rs @@ -19,6 +19,7 @@ use serde::Deserialize; use squabble_core::gate::{CheckRun, Gate, RequiredCheck}; +use squabble_core::polarity::{StepConclusion, StepOutcome}; use std::process::Command; #[derive(Debug, Deserialize)] @@ -26,6 +27,10 @@ struct RollupEntry { name: String, status: Option, conclusion: Option, + /// `https://github.com/O/R/actions/runs//job/` — the only place + /// the rollup exposes a job id, which is what the jobs API needs. + #[serde(rename = "detailsUrl")] + details_url: Option, } #[derive(Debug, Deserialize)] @@ -89,6 +94,72 @@ fn build_gate(required_contexts: &[String], rollup: &[RollupEntry]) -> Gate { Gate::new(checks) } +/// A check that concluded `success`, and the job whose steps can be inspected. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct GreenCheck { + pub name: String, + pub job_id: u64, +} + +#[derive(Debug, Deserialize)] +struct JobStep { + name: String, + conclusion: Option, +} + +#[derive(Debug, Deserialize)] +struct JobView { + #[serde(default)] + steps: Vec, +} + +/// Pull the job id out of a rollup entry's `detailsUrl`. +/// +/// The rollup exposes no job id field, but the details URL ends +/// `/actions/runs//job/`. Pure and directly tested: a silent `None` +/// here would mean a green check is never inspected, which is precisely the +/// failure this module exists to catch. +fn job_id_from_details_url(url: &str) -> Option { + url.split("/job/").nth(1)?.split('/').next()?.parse().ok() +} + +/// The checks that concluded `success` and can actually be inspected. +/// +/// A `SUCCESS` entry with an unparseable `detailsUrl` (a status context posted +/// by an app, say — it has no job) is skipped: there are no steps to read. +fn greens_from_rollup(rollup: &[RollupEntry]) -> Vec { + rollup + .iter() + .filter(|r| r.conclusion.as_deref() == Some("SUCCESS")) + .filter_map(|r| { + let job_id = job_id_from_details_url(r.details_url.as_deref()?)?; + Some(GreenCheck { + name: r.name.clone(), + job_id, + }) + }) + .collect() +} + +/// Parse a jobs-API payload into step outcomes. Pure — the unit of test +/// coverage for [`fetch_step_outcomes`]. +fn parse_steps(json: &str) -> Result, String> { + let job: JobView = + serde_json::from_str(json).map_err(|e| format!("could not parse job response: {e}"))?; + Ok(job + .steps + .into_iter() + .map(|s| StepOutcome::new(s.name, StepConclusion::parse(s.conclusion.as_deref()))) + .collect()) +} + +/// Fetch one job's step conclusions — the declared evidence tier for +/// [`squabble_core::polarity`]. +pub fn fetch_step_outcomes(slug: &str, job_id: u64) -> Result, String> { + let json = run_gh(&["api", &format!("repos/{slug}/actions/jobs/{job_id}")])?; + parse_steps(&json) +} + fn run_gh(args: &[&str]) -> Result { let out = Command::new("gh") .args(args) @@ -110,6 +181,16 @@ fn run_gh(args: &[&str]) -> Result { /// `slug` is `owner/repo`. Requires `gh` to be authenticated for that repo — /// the same precondition every other `gh`-based estate tool already has. pub fn run(slug: &str, pr: &str) -> Result { + run_with_greens(slug, pr).map(|(gate, _greens)| gate) +} + +/// As [`run`], but also returns the checks that concluded **success**, with the +/// job id needed to inspect their steps. +/// +/// The green set is what [`squabble_core::polarity`] classifies. `fight` only +/// ever looks at reds, so a gate that could not run reports green and is never +/// inspected — that is the whole fake-green class. +pub fn run_with_greens(slug: &str, pr: &str) -> Result<(Gate, Vec), String> { let (owner, repo) = slug .split_once('/') .ok_or_else(|| format!("expected `owner/repo`, got `{slug}`"))?; @@ -152,7 +233,10 @@ pub fn run(slug: &str, pr: &str) -> Result { )); } - Ok(build_gate(&required_contexts, &pr_view.status_check_rollup)) + Ok(( + build_gate(&required_contexts, &pr_view.status_check_rollup), + greens_from_rollup(&pr_view.status_check_rollup), + )) } #[cfg(test)] @@ -164,6 +248,7 @@ mod tests { name: name.to_string(), status: status.map(String::from), conclusion: conclusion.map(String::from), + details_url: None, } } @@ -215,3 +300,129 @@ mod tests { assert_eq!(gate.evaluate(), squabble_core::gate::GateState::Green); } } + +#[cfg(test)] +mod polarity_plumbing_tests { + use super::*; + + fn green(name: &str, details: Option<&str>) -> RollupEntry { + RollupEntry { + name: name.to_string(), + status: Some("COMPLETED".into()), + conclusion: Some("SUCCESS".into()), + details_url: details.map(String::from), + } + } + + #[test] + fn a_job_id_is_read_from_a_real_details_url() { + // Shape taken from a live `gh pr view --json statusCheckRollup`. + let url = "https://github.com/hyperpolymath/standards/actions/runs/33817314194/job/100852208701"; + assert_eq!(job_id_from_details_url(url), Some(100852208701)); + } + + #[test] + fn a_details_url_with_no_job_segment_yields_none() { + // A status context posted by an app has no job, so there are no steps + // to inspect. It must be skipped, not guessed at. + assert_eq!( + job_id_from_details_url("https://example.com/build/status"), + None + ); + assert_eq!( + job_id_from_details_url( + "https://github.com/o/r/actions/runs/1" + ), + None + ); + } + + #[test] + fn only_successful_checks_with_an_inspectable_job_are_green() { + let rollup = vec![ + green("has-a-job", Some("https://g/o/r/actions/runs/1/job/42")), + green("no-details-url", None), + green("not-a-job", Some("https://example.com/status")), + RollupEntry { + name: "red".into(), + status: Some("COMPLETED".into()), + conclusion: Some("FAILURE".into()), + details_url: Some("https://g/o/r/actions/runs/1/job/43".into()), + }, + ]; + let greens = greens_from_rollup(&rollup); + assert_eq!( + greens, + vec![GreenCheck { + name: "has-a-job".into(), + job_id: 42 + }], + "reds are `fight`'s job; only inspectable greens belong here" + ); + } + + #[test] + fn step_conclusions_are_parsed_from_a_jobs_api_payload() { + let json = r#"{ + "id": 42, + "conclusion": "success", + "steps": [ + {"name": "Set up job", "conclusion": "success"}, + {"name": "Run Hypatia scan", "conclusion": "skipped"}, + {"name": "Create stub findings", "conclusion": "success"}, + {"name": "Post job", "conclusion": null} + ] + }"#; + let steps = parse_steps(json).expect("valid payload"); + assert_eq!(steps.len(), 4); + assert_eq!(steps[1].name, "Run Hypatia scan"); + assert_eq!(steps[1].conclusion, StepConclusion::Skipped); + assert_eq!(steps[2].conclusion, StepConclusion::Success); + // A null conclusion must not be mistaken for a skip — a skip is half + // the vacuity signature. + assert_eq!(steps[3].conclusion, StepConclusion::Other); + } + + #[test] + fn a_payload_with_no_steps_parses_to_an_empty_list() { + // `NoStepsRecorded` is a real vacuity cause, so this must parse rather + // than error. + let steps = parse_steps(r#"{"id": 1, "conclusion": "success"}"#).expect("valid"); + assert!(steps.is_empty()); + } + + #[test] + fn the_parsed_steps_classify_as_vacuous_end_to_end() { + // The whole point: a real jobs-API payload, the real directive + // signature, and the verdict the report will carry. + let json = r#"{"steps":[ + {"name":"Run Hypatia scan","conclusion":"skipped"}, + {"name":"Create stub findings","conclusion":"success"} + ]}"#; + let steps = parse_steps(json).expect("valid payload"); + let sig = squabble_fight::gate_triage::parse_signature( + "signature-skipped-steps = [\"Run Hypatia scan\"]\n\ + signature-success-steps = [\"Create stub findings\"]\n", + ); + let verdict = squabble_core::polarity::classify( + &steps, + &sig, + &squabble_core::polarity::Applicability::default(), + &squabble_core::polarity::RepoDeclaration::default(), + squabble_core::polarity::Evidence { + run_count: 1, + stub_rate: 1.0, + upstream_exists: None, + target_tech_present: None, + }, + ); + assert!( + matches!( + verdict, + squabble_core::polarity::PolarityVerdict::Vacuous { .. } + ), + "got {verdict:?}" + ); + assert!(verdict.to_move("scan / hypatia").is_some()); + } +} diff --git a/crates/squabble-cli/src/fight.rs b/crates/squabble-cli/src/fight.rs index 3b08340..3e55ebc 100644 --- a/crates/squabble-cli/src/fight.rs +++ b/crates/squabble-cli/src/fight.rs @@ -11,6 +11,9 @@ use crate::fetch; use squabble_core::gate::Gate; +use squabble_core::moves::Move; +use squabble_core::outcome::Escalation; +use squabble_core::polarity::{Applicability, Evidence, RepoDeclaration}; use squabble_core::outcome::Outcome; use squabble_fight::context::RepoContext; use std::path::PathBuf; @@ -39,7 +42,7 @@ pub fn run(rest: &[String]) -> ExitCode { } }; - let gate = match load_gate(&args) { + let (gate, greens) = match load_gate(&args) { Ok(g) => g, Err(e) => { eprintln!("squabble fight: {e}"); @@ -49,6 +52,13 @@ pub fn run(rest: &[String]) -> ExitCode { let (context, mut outcome) = squabble_fight::plan_at_root(&gate, &args.slug, &args.repo_root); + // `fight` classifies only reds, so a check that could not run reports green + // and is never inspected. Surface those before anything else acts on the + // outcome — including `--summon`, which must record its honest + // non-dispatch for them. + let vacuity = classify_greens(&args, &greens); + attach_vacuity(&mut outcome, &vacuity); + // `--apply` enacts the appliable self-win moves (v0.1: path-filter strips) // by writing the workflow files — and nothing more. It never commits or // pushes, and it never re-runs the checks, so the gate stays honestly red @@ -101,15 +111,104 @@ pub fn run(rest: &[String]) -> ExitCode { ExitCode::SUCCESS } -fn load_gate(args: &FightArgs) -> Result { +/// Classify the checks that concluded **green**, per the `gate_triage` +/// directive. +/// +/// `fight` only ever classifies reds, so a check that could not run reports +/// green and is never inspected. This is the missing polarity. +fn classify_greens(args: &FightArgs, greens: &[fetch::GreenCheck]) -> Vec { + let signature = squabble_fight::gate_triage::load_signature(&args.repo_root); + if !signature.is_usable() { + // Fail-safe: no directive means "detect no vacuity", never "detect it + // everywhere". It also costs zero API calls on repos without one. + return Vec::new(); + } + // No gate declares an applicability predicate today, so Axis 0 falls + // through to the signature. Stated explicitly rather than assumed. + let applicability = Applicability::default(); + let declared = RepoDeclaration::default(); + + let mut moves = Vec::new(); + for g in greens { + let steps = match fetch::fetch_step_outcomes(&args.slug, g.job_id) { + Ok(s) => s, + Err(e) => { + // `no-silent-skip`: an uninspectable green is reported, never + // quietly assumed genuine. + eprintln!( + "squabble fight: could not inspect green check `{}`: {e}", + g.name + ); + continue; + } + }; + // One run inspected, and it is the run being judged. `upstream-exists` + // and `target-tech-present` are not observable from the jobs API — no + // gate declares the globs that would make them computable — so they are + // reported `unmeasured` rather than asserted, which also keeps the + // recommendation on the non-destructive branch. + let evidence = Evidence { + run_count: 1, + stub_rate: 1.0, + upstream_exists: None, + target_tech_present: None, + }; + let verdict = squabble_core::polarity::classify( + &steps, + &signature, + &applicability, + &declared, + evidence, + ); + if let Some(m) = verdict.to_move(&g.name) { + moves.push(m); + } + } + moves +} + +/// Fold vacuity findings into the outcome **without changing its colour**. +fn attach_vacuity(outcome: &mut Outcome, moves: &[Move]) { + if moves.is_empty() { + return; + } + match outcome { + Outcome::Red { report } => { + for m in moves { + if let Some(e) = Escalation::from_move(m) { + report.escalations.push(e); + } + } + } + // A gate that is green overall carries no `Report`, and giving + // `Outcome::Green` one would change an outcome state — which issue #58 + // rules is spec-first work, not something to slip in here. stderr keeps + // the finding visible in `--json` mode too, so it is never dropped. + _ => { + eprintln!( + "squabble fight: {} green check(s) are vacuous — not represented in --json \ + (that needs an Outcome change; see issue #58):", + moves.len() + ); + for m in moves { + eprintln!(" - {}", m.describe()); + } + } + } +} + +fn load_gate(args: &FightArgs) -> Result<(Gate, Vec), String> { if let Some(path) = &args.gate_file { let text = std::fs::read_to_string(path).map_err(|e| format!("cannot read `{path}`: {e}"))?; + // Offline mode inspects no live runs, so there are no green checks to + // classify — an honest empty set, not a silent skip. return serde_json::from_str(&text) + .map(|g| (g, Vec::new())) .map_err(|e| format!("`{path}` is not a valid gate: {e}")); } match &args.pr { - Some(pr) => fetch::run(&args.slug, pr), + Some(pr) => fetch::run_with_greens(&args.slug, pr), None => Err(format!( "need a PR number (live) or `--gate ` (offline).\n{USAGE}" )), diff --git a/crates/squabble-core/src/polarity.rs b/crates/squabble-core/src/polarity.rs index 72181d4..936d7f9 100644 --- a/crates/squabble-core/src/polarity.rs +++ b/crates/squabble-core/src/polarity.rs @@ -150,10 +150,22 @@ pub struct Evidence { pub run_count: u32, /// Fraction of those runs that were stubbed, in `0.0..=1.0`. pub stub_rate: f64, - /// Does the tool this gate invokes exist upstream at all? - pub upstream_exists: bool, - /// Is the technology this gate scans for still present in the tree? - pub target_tech_present: bool, + /// Does the tool this gate invokes exist upstream at all? `None` when the + /// host could not determine it — never silently `true`. + pub upstream_exists: Option, + /// Is the technology this gate scans for still present in the tree? `None` + /// when unmeasured: no gate today declares the paths/globs that would make + /// this computable, and claiming `true` would be an overclaim. + pub target_tech_present: Option, +} + +/// Render a tri-state honestly. "unmeasured" is a first-class answer. +fn tri(v: Option) -> &'static str { + match v { + Some(true) => "true", + Some(false) => "false", + None => "unmeasured", + } } impl Evidence { @@ -162,7 +174,10 @@ impl Evidence { pub fn describe(&self) -> String { format!( "run-count={} stub-rate={:.2} upstream-exists={} target-tech-present={}", - self.run_count, self.stub_rate, self.upstream_exists, self.target_tech_present + self.run_count, + self.stub_rate, + tri(self.upstream_exists), + tri(self.target_tech_present) ) } } @@ -225,13 +240,15 @@ impl Recommendation { /// Select the case from the four required evidence fields. Total function. pub const fn from_evidence(e: &Evidence) -> Self { - if !e.target_tech_present { - Recommendation::RecommendRemoval - } else if !e.upstream_exists { - Recommendation::FixTheDeclaration - } else { - // Deliberately unconditional on stub_rate — see the type doc. - Recommendation::MakeItGreatInPractice + match (e.target_tech_present, e.upstream_exists) { + // Case 1 — measured absent. Only a measurement may recommend removal. + (Some(false), _) => Recommendation::RecommendRemoval, + // Case 5 — the tool is declared but is not there. + (_, Some(false)) => Recommendation::FixTheDeclaration, + // Case 3 — including every unmeasured combination. Unmeasured must + // fall to the non-destructive recommendation, and stub_rate never + // enters: see the type doc. + _ => Recommendation::MakeItGreatInPractice, } } } @@ -399,8 +416,8 @@ mod tests { Evidence { run_count: 4, stub_rate: rate, - upstream_exists: upstream, - target_tech_present: tech, + upstream_exists: Some(upstream), + target_tech_present: Some(tech), } } @@ -672,6 +689,28 @@ mod tests { } } + #[test] + fn an_unmeasured_field_never_recommends_removal() { + // The host cannot measure target-tech-present from the jobs API today, + // and "unmeasured" must fall to the non-destructive recommendation + // rather than being rendered as a confident `true`/`false`. + let unmeasured = Evidence { + run_count: 1, + stub_rate: 1.0, + upstream_exists: None, + target_tech_present: None, + }; + assert_eq!( + Recommendation::from_evidence(&unmeasured), + Recommendation::MakeItGreatInPractice + ); + assert!( + unmeasured.describe().contains("target-tech-present=unmeasured"), + "got `{}`", + unmeasured.describe() + ); + } + #[test] fn the_evidence_string_carries_all_four_required_fields() { let d = ev(true, true, 0.75).describe(); diff --git a/crates/squabble-fight/src/context.rs b/crates/squabble-fight/src/context.rs index 4c72a36..f0497ce 100644 --- a/crates/squabble-fight/src/context.rs +++ b/crates/squabble-fight/src/context.rs @@ -149,7 +149,7 @@ fn extract_scalar(text: &str, key: &str) -> Option { /// Extract every double-quoted string in an array-valued key, whether the array /// is written on one line or spread across several (`key = [ ... ]`). -fn extract_array(text: &str, key: &str) -> Vec { +pub(crate) fn extract_array(text: &str, key: &str) -> Vec { let mut out = Vec::new(); let mut collecting = false; for line in text.lines() { diff --git a/crates/squabble-fight/src/gate_triage.rs b/crates/squabble-fight/src/gate_triage.rs new file mode 100644 index 0000000..500eb06 --- /dev/null +++ b/crates/squabble-fight/src/gate_triage.rs @@ -0,0 +1,96 @@ +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2026 Jonathan D.A. Jewell (hyperpolymath) +//! Reads the `gate_triage` bot directive — the host half of the green-polarity +//! classifier. +//! +//! [`squabble_core::polarity`] is deliberately scanner-agnostic: it holds no +//! step names. They live in `.machine_readable/bot_directives/gate_triage.a2ml` +//! under `signature-skipped-steps` / `signature-success-steps`, and this module +//! is what turns that file into a [`VacuitySignature`]. +//! +//! Fail-safe like [`crate::context`]: an absent or unrecognised directive +//! yields an *unusable* signature, and an unusable signature matches nothing. +//! A missing directive therefore means "detect no vacuity", never "detect +//! vacuity everywhere". + +use squabble_core::polarity::VacuitySignature; +use std::path::Path; + +use crate::context::extract_array; + +/// Where the directive lives, relative to a repo checkout. +pub const DIRECTIVE_PATH: &str = ".machine_readable/bot_directives/gate_triage.a2ml"; + +/// Load the vacuity signature from a repo checkout. Never fails. +pub fn load_signature(repo_root: &Path) -> VacuitySignature { + let raw = std::fs::read_to_string(repo_root.join(DIRECTIVE_PATH)).unwrap_or_default(); + parse_signature(&raw) +} + +/// Pure half — the unit of test coverage. [`load_signature`] only supplies the +/// file's text. +pub fn parse_signature(raw: &str) -> VacuitySignature { + VacuitySignature { + skipped_steps: extract_array(raw, "signature-skipped-steps"), + success_steps: extract_array(raw, "signature-success-steps"), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + const DIRECTIVE: &str = r#" +[gate-triage.detection] +source = "actions jobs API step conclusions" +signature-skipped-steps = ["Run Hypatia scan"] +signature-success-steps = ["Create stub findings"] +check-conclusion-when-vacuous = "success" +evidence-required = ["run-count", "stub-rate", "upstream-exists", "target-tech-present"] +"#; + + #[test] + fn both_signature_halves_are_read_from_the_directive() { + let sig = parse_signature(DIRECTIVE); + assert_eq!(sig.skipped_steps, vec!["Run Hypatia scan".to_string()]); + assert_eq!(sig.success_steps, vec!["Create stub findings".to_string()]); + assert!(sig.is_usable()); + } + + #[test] + fn the_two_keys_do_not_bleed_into_each_other() { + // `signature-skipped-steps` and `signature-success-steps` share a long + // prefix; a sloppy prefix match would merge them. + let sig = parse_signature(DIRECTIVE); + assert!(!sig.skipped_steps.contains(&"Create stub findings".to_string())); + assert!(!sig.success_steps.contains(&"Run Hypatia scan".to_string())); + } + + #[test] + fn an_absent_directive_detects_nothing() { + // Fail-safe: no directive must mean "detect no vacuity", never + // "detect vacuity everywhere". + let sig = parse_signature(""); + assert!(!sig.is_usable()); + } + + #[test] + fn a_missing_file_yields_an_unusable_signature() { + let sig = load_signature(Path::new("/nonexistent-repo-root")); + assert!(!sig.is_usable()); + } + + #[test] + fn the_repos_own_directive_parses() { + // Ground-truth against the real file rather than only a fixture. + let root = Path::new(env!("CARGO_MANIFEST_DIR")) + .parent() + .and_then(|p| p.parent()) + .expect("workspace root"); + let sig = load_signature(root); + assert!( + sig.is_usable(), + "this repo's own {DIRECTIVE_PATH} must parse into a usable signature" + ); + } +} diff --git a/crates/squabble-fight/src/lib.rs b/crates/squabble-fight/src/lib.rs index 7e3decc..7b284de 100644 --- a/crates/squabble-fight/src/lib.rs +++ b/crates/squabble-fight/src/lib.rs @@ -27,6 +27,7 @@ pub mod apply; pub mod context; +pub mod gate_triage; pub mod workflows; use context::RepoContext; From 5518b25076fb522a09abfb57a47de7f393ed8d74 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Fri, 4 Sep 2026 01:04:58 +0100 Subject: [PATCH 3/7] fix(directive): the vacuity signature named a step that does not exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `step_concluded` compares step names exactly. The directive shipped "Create stub findings", but the real workflow step is "Create stub findings (when Hypatia unavailable)" — 33 of 33 local `static-analysis-gate.yml` copies, zero variants. So the signature matched no job on earth and every green check would have classified `Genuine`, forever. The classifier hunting fake greens was about to become one. The strings came from a ruling, not from a workflow file. Fixed at source (the directive), not by loosening the matcher: exact comparison is correct when the names are uniform, and the census says they are. The tests did not catch this because both of them asked the wrong question: - `the_repos_own_directive_parses` asserted only `is_usable()` — "are the lists non-empty" — while its consumer needs "do these names match a real job". Renamed and strengthened to assert the literal names. - the end-to-end test built its signature from an inline fixture carrying the same wrong string as its payload, so it agreed with itself and proved nothing. It now loads this repo's own directive, so drift on either side fails it. Both are proven able to fail: restoring the abbreviation fails each one and nothing else; reverting it returns them to green. Added a negative control — a scan that really ran must not be reported vacuous — so the classifier is pinned in both directions. The directive also now records three limits it previously implied away: - coverage is hypatia-only; the identical panic-attack (33/33) and patch-bridge (27/33) stub paths are NOT matched, because the quantifier is `all` and one signature describes one scanner. An undercount, never a false alarm. - `run-count` and `stub-rate` are single-run values, not a measured history. - applicability (axis 0) is core-only; `fight` passes the defaults, so the operator-type / channel axis cannot fire in production yet. Verified: cargo test --workspace --all-features (113 passed), clippy --all-features --all-targets -D warnings clean, and `just quality` green — all run unpiped so the exit codes are real. --- .../bot_directives/gate_triage.a2ml | 42 +++++++++++-- crates/squabble-cli/src/fetch.rs | 60 ++++++++++++++++--- crates/squabble-fight/src/gate_triage.rs | 22 ++++++- 3 files changed, 109 insertions(+), 15 deletions(-) diff --git a/.machine_readable/bot_directives/gate_triage.a2ml b/.machine_readable/bot_directives/gate_triage.a2ml index faebfee..e86b918 100644 --- a/.machine_readable/bot_directives/gate_triage.a2ml +++ b/.machine_readable/bot_directives/gate_triage.a2ml @@ -170,17 +170,47 @@ note = "This is a contradiction with the environment declaration, NOT a judgemen [gate-triage.detection] source = "actions jobs API step conclusions" + +# GROUND TRUTH, not a paraphrase. Census of 33 local `static-analysis-gate.yml` +# copies, 2026-09-04: both names below appear 33/33 with ZERO variants. The +# matcher in `squabble-core::polarity::step_concluded` is an EXACT compare, so +# the abbreviated form this file carried until now ("Create stub findings") +# matched no real job on earth — every green check would have classified +# `Genuine`, forever, and the vacuity classifier would have been its own fake +# green. Do not shorten these for readability. signature-skipped-steps = ["Run Hypatia scan"] -signature-success-steps = ["Create stub findings"] +signature-success-steps = ["Create stub findings (when Hypatia unavailable)"] check-conclusion-when-vacuous = "success" polarity = "green" # NOT red — this is the gap in `fight` evidence-required = ["run-count", "stub-rate", "upstream-exists", "target-tech-present"] +# Coverage, stated honestly. The same workflow carries two more stub paths of +# identical shape, found by the same census: +# "Run panic-attack assail" / "Create stub findings (when panic-attack unavailable)" 33/33 +# "Run Patch Bridge triage" / "Create stub report (when unavailable)" 27/33 +# Neither is covered here. The matcher quantifies with `all`, so one signature +# describes one scanner; and each scanner is a separate JOB, hence a separate +# check, so those checks classify `Genuine` today. That is an UNDERCOUNT, never +# a false alarm. A second signature needs a repeatable table in this file plus a +# parser change, so it is filed rather than bodged in. +signature-covers = ["hypatia"] +signature-not-yet-covered = ["panic-attack", "patch-bridge"] + # Measurability, stated honestly (Doctrine #10, no overclaim). -# Of the four fields above, the actions jobs API supplies only the first two. -# `upstream-exists` and `target-tech-present` need a repo-tree probe that no -# gate currently declares the globs for, so `squabble fight` renders them -# `unmeasured` rather than guessing — and an unmeasured field can never reach -# the destructive recommendation. Removal advice waits for real measurement. +# `run-count` and `stub-rate` are NOT a measured history. `squabble fight` +# inspects ONE run, so it reports run-count 1 and a rate that is trivially 0.0 +# or 1.0; a real rate needs a runs-list probe. `upstream-exists` and +# `target-tech-present` need a repo-tree probe that no gate declares globs for, +# so they render `unmeasured` rather than being guessed at. An unmeasured field +# can never reach the destructive recommendation, and a single-run rate must +# never be read as evidence of a pattern. Removal advice waits for real +# measurement. evidence-measurable-by-host = ["run-count", "stub-rate"] evidence-unmeasured-by-host = ["upstream-exists", "target-tech-present"] +evidence-single-run-only = ["run-count", "stub-rate"] + +# Applicability (axis 0) is CORE-ONLY. `classify()` takes an `Applicability` and +# a `RepoDeclaration`, but `squabble fight` passes the defaults, so the owner's +# operator-type / channel axis cannot fire in production yet. Declared here so +# the gap is visible instead of being implied to work. +applicability-wired-into-host = false diff --git a/crates/squabble-cli/src/fetch.rs b/crates/squabble-cli/src/fetch.rs index 9becea4..202cc6c 100644 --- a/crates/squabble-cli/src/fetch.rs +++ b/crates/squabble-cli/src/fetch.rs @@ -393,17 +393,26 @@ mod polarity_plumbing_tests { #[test] fn the_parsed_steps_classify_as_vacuous_end_to_end() { - // The whole point: a real jobs-API payload, the real directive - // signature, and the verdict the report will carry. + // The whole chain, with nothing synthetic on the signature side: a + // real-shaped jobs-API payload carrying the LITERAL step names read off + // 33 estate `static-analysis-gate.yml` copies, matched against THIS + // REPO'S OWN directive file rather than an inline fixture. + // + // The inline fixture this test used to carry named the steps + // "Create stub findings" on both sides, so it agreed with itself and + // proved nothing: the directive's abbreviation matched no real job, and + // the test could not see that. Loading the real file is what makes a + // future drift in either direction fail here. let json = r#"{"steps":[ {"name":"Run Hypatia scan","conclusion":"skipped"}, - {"name":"Create stub findings","conclusion":"success"} + {"name":"Create stub findings (when Hypatia unavailable)","conclusion":"success"} ]}"#; let steps = parse_steps(json).expect("valid payload"); - let sig = squabble_fight::gate_triage::parse_signature( - "signature-skipped-steps = [\"Run Hypatia scan\"]\n\ - signature-success-steps = [\"Create stub findings\"]\n", - ); + let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .parent() + .and_then(|p| p.parent()) + .expect("workspace root"); + let sig = squabble_fight::gate_triage::load_signature(root); let verdict = squabble_core::polarity::classify( &steps, &sig, @@ -425,4 +434,41 @@ mod polarity_plumbing_tests { ); assert!(verdict.to_move("scan / hypatia").is_some()); } + + #[test] + fn a_scanner_that_really_ran_is_not_called_vacuous() { + // The negative control the end-to-end test needs. Same directive, same + // job shape, but the scan actually ran and the stub was skipped — the + // exact flip recorded when the estate's probe fix landed. If this ever + // returns Vacuous the classifier is condemning working gates. + let json = r#"{"steps":[ + {"name":"Run Hypatia scan","conclusion":"success"}, + {"name":"Create stub findings (when Hypatia unavailable)","conclusion":"skipped"} + ]}"#; + let steps = parse_steps(json).expect("valid payload"); + let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .parent() + .and_then(|p| p.parent()) + .expect("workspace root"); + let sig = squabble_fight::gate_triage::load_signature(root); + let verdict = squabble_core::polarity::classify( + &steps, + &sig, + &squabble_core::polarity::Applicability::default(), + &squabble_core::polarity::RepoDeclaration::default(), + squabble_core::polarity::Evidence { + run_count: 1, + stub_rate: 0.0, + upstream_exists: None, + target_tech_present: None, + }, + ); + assert!( + !matches!( + verdict, + squabble_core::polarity::PolarityVerdict::Vacuous { .. } + ), + "a gate that ran must not be reported vacuous; got {verdict:?}" + ); + } } diff --git a/crates/squabble-fight/src/gate_triage.rs b/crates/squabble-fight/src/gate_triage.rs index 500eb06..491f6cb 100644 --- a/crates/squabble-fight/src/gate_triage.rs +++ b/crates/squabble-fight/src/gate_triage.rs @@ -81,8 +81,16 @@ evidence-required = ["run-count", "stub-rate", "upstream-exists", "target-tech-p } #[test] - fn the_repos_own_directive_parses() { - // Ground-truth against the real file rather than only a fixture. + fn the_repos_own_directive_names_real_workflow_steps() { + // Ground-truth against the real file. `is_usable()` alone was NOT + // enough: it asks "is the list non-empty", while the consumer needs + // "do these names match a real job". The directive shipped + // "Create stub findings" — an abbreviation that matches no step on + // earth, since `step_concluded` compares exactly. Every check would + // have read `Genuine` forever. + // + // The names below are a census of 33 local `static-analysis-gate.yml` + // copies (2026-09-04): 33/33, zero variants. let root = Path::new(env!("CARGO_MANIFEST_DIR")) .parent() .and_then(|p| p.parent()) @@ -92,5 +100,15 @@ evidence-required = ["run-count", "stub-rate", "upstream-exists", "target-tech-p sig.is_usable(), "this repo's own {DIRECTIVE_PATH} must parse into a usable signature" ); + assert_eq!( + sig.skipped_steps, + vec!["Run Hypatia scan".to_string()], + "must be the workflow's literal step name" + ); + assert_eq!( + sig.success_steps, + vec!["Create stub findings (when Hypatia unavailable)".to_string()], + "the parenthetical is part of the real step name — do not abbreviate" + ); } } From bef67cac63ff48bbc121a554782f6e4cb9252b43 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Fri, 4 Sep 2026 01:06:55 +0100 Subject: [PATCH 4/7] docs(test): cite the live jobs-API runs the fixtures were read from Both classifier fixtures now name the run they came from, so a reader can re-fetch them rather than trust the shape: - vacuous: hyperpolymath/session-sentinel run 33813809227 - genuine: hyperpolymath/echidnabot run 33712324720 Fetched 2026-09-04, and they are a three-way live control: echidnabot's scan ran (`Run Hypatia scan=success`, stub skipped) and classifies Genuine, while session-sentinel and hybrid-automation-router (run 33817428163) both show `Run Hypatia scan=skipped` with the stub succeeding and classify Vacuous. That is exactly the set the estate census predicted as permanently green, and the classifier stays silent on the working one. Both step names are now confirmed byte-exact against the live API, not only against workflow YAML. --- crates/squabble-cli/src/fetch.rs | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/crates/squabble-cli/src/fetch.rs b/crates/squabble-cli/src/fetch.rs index 202cc6c..6a0b55c 100644 --- a/crates/squabble-cli/src/fetch.rs +++ b/crates/squabble-cli/src/fetch.rs @@ -393,16 +393,17 @@ mod polarity_plumbing_tests { #[test] fn the_parsed_steps_classify_as_vacuous_end_to_end() { - // The whole chain, with nothing synthetic on the signature side: a - // real-shaped jobs-API payload carrying the LITERAL step names read off - // 33 estate `static-analysis-gate.yml` copies, matched against THIS - // REPO'S OWN directive file rather than an inline fixture. + // The whole chain, with nothing synthetic on the signature side. This + // payload is the LIVE shape of hyperpolymath/session-sentinel run + // 33813809227 (jobs API, 2026-09-04): a green Hypatia check whose + // scanner never ran. It is matched against THIS REPO'S OWN directive + // file rather than an inline fixture. // - // The inline fixture this test used to carry named the steps - // "Create stub findings" on both sides, so it agreed with itself and - // proved nothing: the directive's abbreviation matched no real job, and - // the test could not see that. Loading the real file is what makes a - // future drift in either direction fail here. + // The fixture this test used to carry named the step "Create stub + // findings" on BOTH sides, so it agreed with itself and proved nothing + // — the directive's abbreviation matched no real job, and the test + // could not see that. Loading the real file is what makes drift on + // either side fail here. let json = r#"{"steps":[ {"name":"Run Hypatia scan","conclusion":"skipped"}, {"name":"Create stub findings (when Hypatia unavailable)","conclusion":"success"} @@ -437,10 +438,10 @@ mod polarity_plumbing_tests { #[test] fn a_scanner_that_really_ran_is_not_called_vacuous() { - // The negative control the end-to-end test needs. Same directive, same - // job shape, but the scan actually ran and the stub was skipped — the - // exact flip recorded when the estate's probe fix landed. If this ever - // returns Vacuous the classifier is condemning working gates. + // The negative control. Live shape of hyperpolymath/echidnabot run + // 33712324720: the same gate, same job, but the scan actually ran and + // the stub was skipped. If this ever returns Vacuous the classifier is + // condemning working gates. let json = r#"{"steps":[ {"name":"Run Hypatia scan","conclusion":"success"}, {"name":"Create stub findings (when Hypatia unavailable)","conclusion":"skipped"} From 7c27241f86d84ccfee6dd198dd2638b5d9154788 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Fri, 4 Sep 2026 01:33:11 +0100 Subject: [PATCH 5/7] fix(polarity): stop reading absence of evidence as evidence of vacuity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three real defects, all raised on review of this PR and all verified against the source before changing anything. 1. `classify()` tested `steps.is_empty()` FIRST and returned `Vacuous { NoStepsRecorded }`. But an empty step list means the jobs API showed us nothing — it is not a report that the job ran nothing. Escalating it is a genuine false alarm, and it falsifies the undercount-never-false-alarm property the directive promises. This module exists to catch guards that answer a different question from the one their consumer needs; it had become one. The cause is removed, not merely bypassed. The empty case is an EXPLICIT early return, because `[].iter().all(..)` is vacuously true in Rust: deleting the arm would have let the empty case fall through to `AllStepsSkipped` and reach the identical wrong verdict under a different label. That is proven, not asserted — with the early return neutered, both new tests fail. `no-silent-skip` still holds: the host reports the uninspectable green on stderr, exactly as it already does for a fetch error. `AllStepsSkipped` is KEPT. A job that concluded success with every recorded step skipped enforced nothing, whether or not a signature matched. The directive's coverage note is corrected to say so rather than the code being bent to fit stale prose (Doctrine #10). 2. `job_id_from_details_url` cut the id on `/` only, so any details URL carrying `?check_suite_focus=true` (GitHub appends it routinely) or a `#step:` fragment failed to parse and the green was skipped without a word — a silent undercount by construction. 3. `Evidence.stub_rate` was hardcoded `1.0`, reporting stub evidence for jobs that genuinely ran. It is now measured. The signature match moved to `VacuitySignature::matches` so host and classifier ask the same question: `Evidence` is an input to `classify` while the cause is its output, so deriving the rate from the cause would have been circular. `Recommendation::from_evidence` does not read `stub_rate`, so the recommendation branch is unchanged. Every new test proven able to fail by a planted break, reverted. 115 tests pass; clippy -D warnings clean; `just quality` green. Note on witnesses: this repo's rust-ci workflow cannot run. It is pinned at standards@5b1d0022, which does not exist upstream (404), so the run dies at startup with jobs=0. Four more workflows here are pinned at standards@7fdc2705, which exists but is reachable from no branch. Those five reds are pre-existing on main — every blob is byte-identical to origin/main — and are repaired by the pin sweep, not here. The local runs above are therefore the only witnesses for this change. Co-Authored-By: Claude Opus 5 --- .../bot_directives/gate_triage.a2ml | 16 +++- crates/squabble-cli/src/fetch.rs | 32 ++++++- crates/squabble-cli/src/fight.rs | 17 +++- crates/squabble-core/src/polarity.rs | 88 +++++++++++++------ 4 files changed, 120 insertions(+), 33 deletions(-) diff --git a/.machine_readable/bot_directives/gate_triage.a2ml b/.machine_readable/bot_directives/gate_triage.a2ml index e86b918..ff0c52b 100644 --- a/.machine_readable/bot_directives/gate_triage.a2ml +++ b/.machine_readable/bot_directives/gate_triage.a2ml @@ -190,9 +190,19 @@ evidence-required = ["run-count", "stub-rate", "upstream-exists", "target-tech-p # "Run Patch Bridge triage" / "Create stub report (when unavailable)" 27/33 # Neither is covered here. The matcher quantifies with `all`, so one signature # describes one scanner; and each scanner is a separate JOB, hence a separate -# check, so those checks classify `Genuine` today. That is an UNDERCOUNT, never -# a false alarm. A second signature needs a repeatable table in this file plus a -# parser change, so it is filed rather than bodged in. +# check, so an unmatched scanner is NOT escalated on the signature. That is an +# UNDERCOUNT, never a false alarm — with one stated exception: a job that +# concluded success having recorded steps, every one of them SKIPPED, is +# reported `all-steps-skipped` whether or not any signature matched. That is +# not a false alarm either; a job that ran nothing enforced nothing, and the +# finding stands on its own terms without naming a scanner. +# +# A green whose job records NO steps at all is the opposite case: absence of +# evidence, not evidence of absence. It is reported to the operator on stderr +# and classified `Genuine`, never escalated. +# +# A second signature needs a repeatable table in this file plus a parser +# change, so it is filed rather than bodged in. signature-covers = ["hypatia"] signature-not-yet-covered = ["panic-attack", "patch-bridge"] diff --git a/crates/squabble-cli/src/fetch.rs b/crates/squabble-cli/src/fetch.rs index 6a0b55c..71d06bb 100644 --- a/crates/squabble-cli/src/fetch.rs +++ b/crates/squabble-cli/src/fetch.rs @@ -120,7 +120,16 @@ struct JobView { /// here would mean a green check is never inspected, which is precisely the /// failure this module exists to catch. fn job_id_from_details_url(url: &str) -> Option { - url.split("/job/").nth(1)?.split('/').next()?.parse().ok() + // The id is terminated by a path separator OR by a query/fragment. GitHub + // routinely appends `?check_suite_focus=true`, and cutting only on `/` + // leaves that in the digits, so `parse` fails and the check is skipped — + // an undercount that is silent by construction. + url.split("/job/") + .nth(1)? + .split(['/', '?', '#']) + .next()? + .parse() + .ok() } /// The checks that concluded `success` and can actually be inspected. @@ -321,6 +330,23 @@ mod polarity_plumbing_tests { assert_eq!(job_id_from_details_url(url), Some(100852208701)); } + #[test] + fn a_query_string_or_fragment_does_not_hide_the_job_id() { + // GitHub appends `?check_suite_focus=true` to details URLs as a matter + // of course. Cutting the id on `/` alone leaves the suffix attached, + // `parse::` fails, and the green is skipped without a word — the + // exact silent undercount this module exists to prevent. + let base = "https://github.com/hyperpolymath/standards/actions/runs/33817314194/job/100852208701"; + for suffix in ["?check_suite_focus=true", "#step:4:1", "?a=1#step:2:9"] { + let url = format!("{base}{suffix}"); + assert_eq!( + job_id_from_details_url(&url), + Some(100852208701), + "suffix {suffix} must not hide the job id" + ); + } + } + #[test] fn a_details_url_with_no_job_segment_yields_none() { // A status context posted by an app has no job, so there are no steps @@ -385,8 +411,8 @@ mod polarity_plumbing_tests { #[test] fn a_payload_with_no_steps_parses_to_an_empty_list() { - // `NoStepsRecorded` is a real vacuity cause, so this must parse rather - // than error. + // An absent `steps` array is a legitimate payload — the caller reports + // it as an uninspectable green — so this must parse rather than error. let steps = parse_steps(r#"{"id": 1, "conclusion": "success"}"#).expect("valid"); assert!(steps.is_empty()); } diff --git a/crates/squabble-cli/src/fight.rs b/crates/squabble-cli/src/fight.rs index 3e55ebc..7fb4545 100644 --- a/crates/squabble-cli/src/fight.rs +++ b/crates/squabble-cli/src/fight.rs @@ -142,14 +142,29 @@ fn classify_greens(args: &FightArgs, greens: &[fetch::GreenCheck]) -> Vec continue; } }; + if steps.is_empty() { + // The jobs API returned no steps. That is absence of evidence, not + // evidence the job ran nothing, so `classify` refuses to escalate + // it — but `no-silent-skip` still owes the operator a word, exactly + // as the fetch-error branch above does. + eprintln!( + "squabble fight: green check `{}` recorded no steps — not inspectable, not judged", + g.name + ); + continue; + } // One run inspected, and it is the run being judged. `upstream-exists` // and `target-tech-present` are not observable from the jobs API — no // gate declares the globs that would make them computable — so they are // reported `unmeasured` rather than asserted, which also keeps the // recommendation on the non-destructive branch. + // + // With `run_count == 1` the stub rate can only be 0.0 or 1.0, and it is + // measured from the same predicate `classify` uses rather than assumed: + // hardcoding 1.0 reported stub evidence for jobs that genuinely ran. let evidence = Evidence { run_count: 1, - stub_rate: 1.0, + stub_rate: if signature.matches(&steps) { 1.0 } else { 0.0 }, upstream_exists: None, target_tech_present: None, }; diff --git a/crates/squabble-core/src/polarity.rs b/crates/squabble-core/src/polarity.rs index 936d7f9..a09dd1c 100644 --- a/crates/squabble-core/src/polarity.rs +++ b/crates/squabble-core/src/polarity.rs @@ -114,6 +114,27 @@ impl VacuitySignature { pub fn is_usable(&self) -> bool { !self.skipped_steps.is_empty() && !self.success_steps.is_empty() } + + /// Does this signature match `steps`? + /// + /// Lives here, rather than inline in [`classify`], because the *host* needs + /// the same answer to report an honest `stub_rate`. `Evidence` is an input + /// to `classify` while the cause is its output, so a rate derived from the + /// cause would be circular: both callers ask this instead. + /// + /// CONJUNCTION, deliberately: a partial match is a legitimately optional + /// step, not vacuity. + pub fn matches(&self, steps: &[StepOutcome]) -> bool { + self.is_usable() + && self + .skipped_steps + .iter() + .all(|n| step_concluded(steps, n, StepConclusion::Skipped)) + && self + .success_steps + .iter() + .all(|n| step_concluded(steps, n, StepConclusion::Success)) + } } /// What a gate declares about *where it applies* — the directive's @@ -190,9 +211,12 @@ pub enum VacuityCause { /// stub-writing step succeeded. (Measured in the wild: 4 repos.) StubbedAfterSkippedScan, /// The job concluded success with steps recorded, every one of them skipped. + /// + /// True on its own terms whether or not the signature matched: a job that + /// ran nothing enforced nothing. Note there is deliberately **no** + /// "recorded no steps at all" cause — see [`classify`] for why an empty + /// step list is absence of evidence rather than evidence of vacuity. AllStepsSkipped, - /// The job concluded success having recorded no steps at all. - NoStepsRecorded, } impl VacuityCause { @@ -200,7 +224,6 @@ impl VacuityCause { match self { VacuityCause::StubbedAfterSkippedScan => "stubbed after a skipped scan", VacuityCause::AllStepsSkipped => "every step skipped", - VacuityCause::NoStepsRecorded => "no steps recorded", } } } @@ -371,19 +394,21 @@ pub fn classify( return v; } - let cause = if steps.is_empty() { - Some(VacuityCause::NoStepsRecorded) - } else if signature.is_usable() - // CONJUNCTION. A partial match is a legitimately optional step. - && signature - .skipped_steps - .iter() - .all(|n| step_concluded(steps, n, StepConclusion::Skipped)) - && signature - .success_steps - .iter() - .all(|n| step_concluded(steps, n, StepConclusion::Success)) - { + // An empty step list is ABSENCE OF EVIDENCE, not evidence of absence. The + // jobs API showed us no steps; that is not the same as the job having run + // none. Escalating it is a false alarm, and it breaks the property this + // module promises — undercount, never false-alarm. The uninspectable green + // is reported by the caller on stderr, so `no-silent-skip` still holds. + // + // This arm must stay EXPLICIT and must return early. `[].iter().all(..)` is + // vacuously true in Rust, so merely deleting it would let the empty case + // fall through to `AllStepsSkipped` and reach the identical wrong verdict + // under a different name. + if steps.is_empty() { + return PolarityVerdict::Genuine; + } + + let cause = if signature.matches(steps) { Some(VacuityCause::StubbedAfterSkippedScan) } else if steps.iter().all(|s| s.conclusion == StepConclusion::Skipped) { Some(VacuityCause::AllStepsSkipped) @@ -498,17 +523,28 @@ mod tests { // ---- the other two step-observable causes ------------------------------- #[test] - fn no_steps_recorded_is_vacuous() { + fn no_steps_recorded_is_not_vacuous() { + // Absence of evidence is not evidence of vacuity. An empty step list + // means the jobs API told us nothing, so the only honest verdict is the + // undercount. Guards the property the directive promises. + let v = classify_steps(&[]); + assert_eq!( + v, + PolarityVerdict::Genuine, + "an uninspectable green must never be escalated; got {v:?}" + ); + } + + #[test] + fn an_empty_step_list_does_not_leak_into_all_steps_skipped() { + // `[].iter().all(..)` is vacuously TRUE, so removing the empty-list arm + // rather than returning early would silently reclassify this case as + // `AllStepsSkipped` — the same false alarm under a different label. + // This test is what stops that regression being invisible. let v = classify_steps(&[]); assert!( - matches!( - v, - PolarityVerdict::Vacuous { - cause: VacuityCause::NoStepsRecorded, - .. - } - ), - "got {v:?}" + !matches!(v, PolarityVerdict::Vacuous { .. }), + "empty steps must not reach ANY vacuity cause; got {v:?}" ); } @@ -556,7 +592,7 @@ mod tests { #[test] fn declared_and_unmatched_is_not_applicable() { let v = classify( - &[], // would otherwise be NoStepsRecorded — applicability wins + &[], // uninspectable; Axis 0 answers before steps are consulted &sig(), &Applicability { runs_for_operator_types: vec!["platform_maintainer".into()], From 588688819ead0b41d2a631a21e6a822e685e811d Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Fri, 4 Sep 2026 08:38:32 +0000 Subject: [PATCH 6/7] Fix CodeRabbit issues in PR #60 --- crates/squabble-cli/src/fetch.rs | 53 ++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/crates/squabble-cli/src/fetch.rs b/crates/squabble-cli/src/fetch.rs index 71d06bb..3efb9ed 100644 --- a/crates/squabble-cli/src/fetch.rs +++ b/crates/squabble-cli/src/fetch.rs @@ -120,16 +120,34 @@ struct JobView { /// here would mean a green check is never inspected, which is precisely the /// failure this module exists to catch. fn job_id_from_details_url(url: &str) -> Option { - // The id is terminated by a path separator OR by a query/fragment. GitHub - // routinely appends `?check_suite_focus=true`, and cutting only on `/` - // leaves that in the digits, so `parse` fails and the check is skipped — - // an undercount that is silent by construction. - url.split("/job/") - .nth(1)? - .split(['/', '?', '#']) - .next()? - .parse() - .ok() + let path = url + .strip_prefix("https://github.com/")? + .split(['?', '#']) + .next()?; + let mut segments = path.split('/'); + let (owner, repo, actions, runs, run_id, job, job_id) = ( + segments.next()?, + segments.next()?, + segments.next()?, + segments.next()?, + segments.next()?, + segments.next()?, + segments.next()?, + ); + + if owner.is_empty() + || repo.is_empty() + || actions != "actions" + || runs != "runs" + || run_id.parse::().is_err() + || job != "job" + || !matches!(segments.next(), None | Some("")) + || segments.next().is_some() + { + return None; + } + + job_id.parse().ok() } /// The checks that concluded `success` and can actually be inspected. @@ -363,17 +381,28 @@ mod polarity_plumbing_tests { ); } + #[test] + fn an_external_job_url_is_not_treated_as_a_github_actions_job() { + assert_eq!( + job_id_from_details_url("https://ci.example/job/42"), + None + ); + } + #[test] fn only_successful_checks_with_an_inspectable_job_are_green() { let rollup = vec![ - green("has-a-job", Some("https://g/o/r/actions/runs/1/job/42")), + green( + "has-a-job", + Some("https://github.com/o/r/actions/runs/1/job/42"), + ), green("no-details-url", None), green("not-a-job", Some("https://example.com/status")), RollupEntry { name: "red".into(), status: Some("COMPLETED".into()), conclusion: Some("FAILURE".into()), - details_url: Some("https://g/o/r/actions/runs/1/job/43".into()), + details_url: Some("https://github.com/o/r/actions/runs/1/job/43".into()), }, ]; let greens = greens_from_rollup(&rollup); From d19bffae9050788ea8945ef93588d2dab2de2f53 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Fri, 4 Sep 2026 09:46:20 +0100 Subject: [PATCH 7/7] =?UTF-8?q?=F0=9F=94=A7=20CodeRabbit=20CI=20Fix:=20Fix?= =?UTF-8?q?=20failing=20Estate=20Rules,=20SonarQube,=20and=20Dogfood=20Gat?= =?UTF-8?q?e=20CI=20checks=20(#61)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI failure fixes was requested by @hyperpolymath. * https://github.com/hyperpolymath/cicd-squabbler/pull/60#issuecomment-5533761908 The following files were modified: * `.githooks/validate-k9.sh` * `.machine_readable/root-allow.txt` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- .githooks/validate-k9.sh | 2 +- .machine_readable/root-allow.txt | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/.githooks/validate-k9.sh b/.githooks/validate-k9.sh index c83e290..72b79f5 100755 --- a/.githooks/validate-k9.sh +++ b/.githooks/validate-k9.sh @@ -239,7 +239,7 @@ validate_k9() { fi # Also check for signature fields outside pedigree (top-level) - if [[ "$line" =~ ^[[:space:]]*(signature)[[:space:]]*= ]]; then + if [[ "$line" =~ ^[[:space:]]*(signature|signature_required)[[:space:]]*= ]]; then has_signature_field=true fi done < "$file" diff --git a/.machine_readable/root-allow.txt b/.machine_readable/root-allow.txt index 48f4493..728ff87 100644 --- a/.machine_readable/root-allow.txt +++ b/.machine_readable/root-allow.txt @@ -91,3 +91,14 @@ MAINTAINERS # Project file that must remain at the repository ro REQUIRES_INITIALISATION.md # Project documentation. flake.guix # Project file that must remain at the repository root. mise.toml # Build/tool manifest — must sit at the root to be found. + +# ─── Declared 2026-09-04: AsciiDoc-by-default migration + tool dotfile. +# The .md docs above were converted to .adoc (openssf-compliance and +# docs checks already accept either); the old entries are left above +# for tooling that still references the historical .md names. +ARCHITECTURE.adoc # Project documentation (AsciiDoc-by-default). +CHANGELOG.adoc # Project documentation (AsciiDoc-by-default). +CODE_OF_CONDUCT.adoc # Project documentation (AsciiDoc-by-default). +CONTRIBUTING.adoc # Project documentation (AsciiDoc-by-default). +SECURITY.adoc # Project documentation (AsciiDoc-by-default). +.mise.toml # Repo-specific mise config; must sit at the root to be found.