huskies: merge 986

server/src/agents/gates.rs

@@ -1,9 +1,109 @@
 //! Acceptance gates — runs test suites and validation scripts in agent worktrees.
+use serde::{Deserialize, Serialize};
 use std::path::Path;
 use std::process::Command;
 use std::time::Duration;
 use wait_timeout::ChildExt;
 
+/// Typed classification of a gate failure, produced at the gate execution boundary.
+///
+/// Downstream decision logic (e.g. `is_self_evident_fix`) matches on the variant
+/// rather than scanning the raw output string for patterns.
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub enum GateFailureKind {
+    /// `cargo fmt --check` or `rustfmt --check` detected formatting drift.
+    Fmt,
+    /// `cargo clippy` produced warnings or errors (promoted via `-D warnings`).
+    Lint,
+    /// Test suite (`script/test`, `cargo nextest`, `cargo test`) failed.
+    Test,
+    /// `source-map-check` gate found missing or incomplete doc comments.
+    SourceMapCheck,
+    /// Git content conflict detected during squash-rebase.
+    ContentConflict,
+    /// Build-level failure (duplicate module files E0761, compile error).
+    Build,
+    /// Unclassified gate failure.
+    Other,
+}
+
+impl GateFailureKind {
+    /// Classify a gate failure from its raw output at the gate execution boundary.
+    ///
+    /// Called once when a gate fails to produce a typed kind.  Downstream code
+    /// matches on the variant and must not call this on subsequent reads.
+    pub fn classify(output: &str) -> Self {
+        if output.contains("CONFLICT (content):") || output.contains("Merge conflict:") {
+            GateFailureKind::ContentConflict
+        } else if output.contains("Diff in ") || output.contains("would reformat") {
+            GateFailureKind::Fmt
+        } else if output.contains("missing-docs direction") {
+            GateFailureKind::SourceMapCheck
+        } else if output.contains("error[clippy::")
+            || output.contains("warning[clippy::")
+            || output.contains("missing_doc_comments")
+        {
+            GateFailureKind::Lint
+        } else {
+            GateFailureKind::Test
+        }
+    }
+
+    /// Whether this failure class is a self-evident fix that a short coder session
+    /// can resolve without human intervention (fmt drift, lint warnings, missing docs).
+    pub fn is_self_evident_fix(&self) -> bool {
+        matches!(
+            self,
+            GateFailureKind::Fmt | GateFailureKind::Lint | GateFailureKind::SourceMapCheck
+        )
+    }
+}
+
+/// Outcome of running quality gates, produced at the gate execution boundary.
+///
+/// `failure_kind` drives routing decisions; `output` is retained for human-readable
+/// display and injection into agent retry prompts only — it must not be used as a
+/// decision source (story 986).
+#[derive(Debug, Clone)]
+pub struct GateOutcome {
+    /// Whether all gates passed.
+    pub passed: bool,
+    /// Typed failure classification; `None` when `passed` is `true`.
+    pub failure_kind: Option<GateFailureKind>,
+    /// Human-readable combined gate output (display/prompt injection only).
+    pub output: String,
+}
+
+impl GateOutcome {
+    /// Passing outcome.
+    pub(crate) fn pass(output: String) -> Self {
+        Self {
+            passed: true,
+            failure_kind: None,
+            output,
+        }
+    }
+
+    /// Failing outcome — classifies `failure_kind` from the output at construction.
+    pub(crate) fn fail(output: String) -> Self {
+        let failure_kind = Some(GateFailureKind::classify(&output));
+        Self {
+            passed: false,
+            failure_kind,
+            output,
+        }
+    }
+
+    /// Failing outcome for a pre-classified build error (e.g. duplicate module files).
+    pub(crate) fn build_error(output: String) -> Self {
+        Self {
+            passed: false,
+            failure_kind: Some(GateFailureKind::Build),
+            output,
+        }
+    }
+}
+
 /// Maximum time any single test command is allowed to run before being killed.
 const TEST_TIMEOUT: Duration = Duration::from_secs(1200); // 20 minutes
 
@@ -214,8 +314,8 @@ fn run_command_with_timeout(
 
 /// Run `cargo clippy` and the project test suite (via `script/test` if present,
 /// otherwise `cargo nextest run` / `cargo test`) in the given directory.
-/// Returns `(gates_passed, combined_output)`.
-pub(crate) fn run_acceptance_gates(path: &Path) -> Result<(bool, String), String> {
+/// Returns a [`GateOutcome`] with a typed failure classification.
+pub(crate) fn run_acceptance_gates(path: &Path) -> Result<GateOutcome, String> {
     // Pre-flight: detect duplicate module files (E0761) before running the
     // full test suite so the failure message is immediately actionable.
     let duplicates = find_duplicate_module_files(path);
@@ -231,17 +331,17 @@ pub(crate) fn run_acceptance_gates(path: &Path) -> Result<(bool, String), String
                 mod_path.display()
             ));
         }
-        return Ok((false, msg));
+        return Ok(GateOutcome::build_error(msg));
     }
 
     // Run script/test (or fallback to cargo test). Project-specific linting
     // and test commands belong in script/test.
     let (test_success, test_out) = run_project_tests(path)?;
     if !test_success {
-        return Ok((false, test_out));
+        return Ok(GateOutcome::fail(test_out));
     }
 
-    Ok((true, test_out))
+    Ok(GateOutcome::pass(test_out))
 }
 
 /// Scan `root` recursively for Rust source files where both `path/X.rs` and
@@ -717,4 +817,135 @@ mod tests {
             "untracked file should be restored after cargo check"
         );
     }
+
+    // ── GateFailureKind::classify ─────────────────────────────────────────────
+
+    #[test]
+    fn classify_fmt_from_diff_in() {
+        assert_eq!(
+            GateFailureKind::classify("Diff in server/src/lib.rs\n--- original\n+++ reformatted"),
+            GateFailureKind::Fmt
+        );
+    }
+
+    #[test]
+    fn classify_fmt_from_would_reformat() {
+        assert_eq!(
+            GateFailureKind::classify(
+                "Checking server/src/lib.rs\nwould reformat server/src/lib.rs"
+            ),
+            GateFailureKind::Fmt
+        );
+    }
+
+    #[test]
+    fn classify_lint_from_clippy_error() {
+        assert_eq!(
+            GateFailureKind::classify("error[clippy::unused_variable]: unused variable `x`"),
+            GateFailureKind::Lint
+        );
+    }
+
+    #[test]
+    fn classify_lint_from_clippy_warning() {
+        assert_eq!(
+            GateFailureKind::classify("warning[clippy::needless_return]: unneeded return"),
+            GateFailureKind::Lint
+        );
+    }
+
+    #[test]
+    fn classify_lint_from_missing_doc_comments() {
+        assert_eq!(
+            GateFailureKind::classify(
+                "error: missing_doc_comments: public item lacks documentation"
+            ),
+            GateFailureKind::Lint
+        );
+    }
+
+    #[test]
+    fn classify_source_map_check_from_missing_docs_direction() {
+        assert_eq!(
+            GateFailureKind::classify("missing-docs direction: server/src/foo.rs:42 pub fn bar"),
+            GateFailureKind::SourceMapCheck
+        );
+    }
+
+    #[test]
+    fn classify_content_conflict() {
+        assert_eq!(
+            GateFailureKind::classify("CONFLICT (content): Merge conflict in server/src/lib.rs"),
+            GateFailureKind::ContentConflict
+        );
+    }
+
+    #[test]
+    fn classify_test_failure_for_unrecognised_output() {
+        assert_eq!(
+            GateFailureKind::classify("test result: FAILED. 3 passed; 1 failed"),
+            GateFailureKind::Test
+        );
+    }
+
+    // ── GateFailureKind::is_self_evident_fix ─────────────────────────────────
+
+    #[test]
+    fn fmt_is_self_evident_fix() {
+        assert!(GateFailureKind::Fmt.is_self_evident_fix());
+    }
+
+    #[test]
+    fn lint_is_self_evident_fix() {
+        assert!(GateFailureKind::Lint.is_self_evident_fix());
+    }
+
+    #[test]
+    fn source_map_check_is_self_evident_fix() {
+        assert!(GateFailureKind::SourceMapCheck.is_self_evident_fix());
+    }
+
+    #[test]
+    fn test_failure_is_not_self_evident_fix() {
+        assert!(!GateFailureKind::Test.is_self_evident_fix());
+    }
+
+    #[test]
+    fn content_conflict_is_not_self_evident_fix() {
+        assert!(!GateFailureKind::ContentConflict.is_self_evident_fix());
+    }
+
+    #[test]
+    fn build_error_is_not_self_evident_fix() {
+        assert!(!GateFailureKind::Build.is_self_evident_fix());
+    }
+
+    #[test]
+    fn other_is_not_self_evident_fix() {
+        assert!(!GateFailureKind::Other.is_self_evident_fix());
+    }
+
+    // ── GateOutcome constructors ──────────────────────────────────────────────
+
+    #[test]
+    fn gate_outcome_pass_has_no_failure_kind() {
+        let outcome = GateOutcome::pass("all tests passed".to_string());
+        assert!(outcome.passed);
+        assert!(outcome.failure_kind.is_none());
+        assert_eq!(outcome.output, "all tests passed");
+    }
+
+    #[test]
+    fn gate_outcome_fail_classifies_kind() {
+        let outcome = GateOutcome::fail("Diff in server/src/lib.rs".to_string());
+        assert!(!outcome.passed);
+        assert_eq!(outcome.failure_kind, Some(GateFailureKind::Fmt));
+    }
+
+    #[test]
+    fn gate_outcome_build_error_sets_build_kind() {
+        let outcome = GateOutcome::build_error("ERROR [E0761]: duplicate module files".to_string());
+        assert!(!outcome.passed);
+        assert_eq!(outcome.failure_kind, Some(GateFailureKind::Build));
+    }
 }