huskies: merge 816

server/src/http/mcp/story_tools/criteria.rs

@@ -18,10 +18,14 @@ use crate::io::story_metadata::{
 use crate::service::story::parse_test_cases;
 use crate::slog_warn;
 #[allow(unused_imports)]
-use crate::workflow::{TestCaseResult, TestStatus, evaluate_acceptance_with_coverage};
+use crate::workflow::{
+    TestCaseResult, TestStatus, WorkflowState, evaluate_acceptance_with_coverage,
+};
 use serde_json::{Value, json};
 use std::collections::HashMap;
 use std::fs;
+use std::path::Path;
+use std::process::Command;
 
 pub(crate) fn tool_get_story_todos(args: &Value, ctx: &AppContext) -> Result<String, String> {
     let story_id = args
@@ -134,6 +138,16 @@ pub(crate) fn tool_check_criterion(args: &Value, ctx: &AppContext) -> Result<Str
         .ok_or("Missing required argument: criterion_index")? as usize;
 
     let root = ctx.state.get_project_root()?;
+
+    // Best-effort validation: log a warning if no corroborating evidence is found.
+    // Always proceeds regardless of validation outcome (operator override).
+    if let Ok(contents) = crate::http::workflow::read_story_content(&root, story_id) {
+        let ac_text = find_unchecked_criterion_text(&contents, criterion_index).unwrap_or_default();
+        if let Ok(workflow) = ctx.workflow.lock() {
+            validate_criterion_check(&root, story_id, &ac_text, &workflow);
+        }
+    }
+
     check_criterion_in_file(&root, story_id, criterion_index)?;
 
     Ok(format!(
@@ -141,6 +155,112 @@ pub(crate) fn tool_check_criterion(args: &Value, ctx: &AppContext) -> Result<Str
     ))
 }
 
+/// Extract the text of the Nth unchecked criterion (`- [ ]`) from story content.
+fn find_unchecked_criterion_text(contents: &str, criterion_index: usize) -> Option<String> {
+    let mut count = 0usize;
+    for line in contents.lines() {
+        let trimmed = line.trim();
+        if let Some(rest) = trimmed.strip_prefix("- [ ] ") {
+            if count == criterion_index {
+                return Some(rest.to_string());
+            }
+            count += 1;
+        }
+    }
+    None
+}
+
+/// Run best-effort validation before marking a criterion as checked.
+///
+/// Checks three signals (OR-joined):
+/// - A: feature branch has at least one commit since master
+/// - B: AC text mentions a file path that the branch's commits touched
+/// - C: a recorded test name fuzzy-matches the AC text
+///
+/// Logs a WARN if none of the signals are satisfied. Always returns `()` so
+/// the caller can proceed regardless (operator override).
+fn validate_criterion_check(
+    project_root: &Path,
+    story_id: &str,
+    ac_text: &str,
+    workflow: &WorkflowState,
+) {
+    let branch = format!("feature/story-{story_id}");
+
+    // ── A: branch has commits vs master ──────────────────────────────────────
+    let commits = Command::new("git")
+        .args(["log", &format!("master..{branch}"), "--oneline"])
+        .current_dir(project_root)
+        .output()
+        .ok()
+        .filter(|o| o.status.success())
+        .map(|o| String::from_utf8_lossy(&o.stdout).trim().to_string())
+        .unwrap_or_default();
+
+    if !commits.is_empty() {
+        // A passes — branch has real work. Validation satisfied.
+        return;
+    }
+
+    // A fails. Check B and C as fallback evidence.
+
+    // ── B: AC text mentions a file touched by the branch ─────────────────────
+    let changed_files: Vec<String> = Command::new("git")
+        .args(["diff", &format!("master...{branch}"), "--name-only"])
+        .current_dir(project_root)
+        .output()
+        .ok()
+        .filter(|o| o.status.success())
+        .map(|o| {
+            String::from_utf8_lossy(&o.stdout)
+                .lines()
+                .map(|l| l.to_string())
+                .collect()
+        })
+        .unwrap_or_default();
+
+    let ac_lower = ac_text.to_lowercase();
+    let file_mentioned = changed_files.iter().any(|f| {
+        let fname = Path::new(f)
+            .file_name()
+            .map(|n| n.to_string_lossy().to_lowercase())
+            .unwrap_or_default();
+        ac_lower.contains(f) || (!fname.is_empty() && ac_lower.contains(fname.as_str()))
+    });
+
+    if file_mentioned {
+        return;
+    }
+
+    // ── C: a recorded test name fuzzy-matches the AC text ────────────────────
+    let ac_words: Vec<&str> = ac_lower
+        .split(|c: char| !c.is_alphanumeric() && c != '_')
+        .filter(|w| w.len() >= 3)
+        .collect();
+
+    let test_match = workflow.results.get(story_id).is_some_and(|results| {
+        let names: Vec<String> = results
+            .unit
+            .iter()
+            .chain(results.integration.iter())
+            .map(|t| t.name.to_lowercase())
+            .collect();
+        ac_words
+            .iter()
+            .any(|word| names.iter().any(|n| n.contains(word)))
+    });
+
+    if !test_match {
+        slog_warn!(
+            "[check_criterion] story '{}': no corroborating evidence for criterion '{}' \
+             — branch '{}' has no commits vs master and no matching files or tests",
+            story_id,
+            ac_text,
+            branch
+        );
+    }
+}
+
 pub(crate) fn tool_edit_criterion(args: &Value, ctx: &AppContext) -> Result<String, String> {
     let story_id = args
         .get("story_id")
@@ -456,6 +576,74 @@ mod tests {
         assert!(result.unwrap_err().contains("blocked"));
     }
 
+    #[test]
+    fn tool_check_criterion_empty_branch_logs_warning() {
+        let tmp = tempfile::tempdir().unwrap();
+
+        // Init a git repo with an initial commit on master.
+        std::process::Command::new("git")
+            .args(["init"])
+            .current_dir(tmp.path())
+            .output()
+            .unwrap();
+        std::process::Command::new("git")
+            .args(["config", "user.email", "test@test.com"])
+            .current_dir(tmp.path())
+            .output()
+            .unwrap();
+        std::process::Command::new("git")
+            .args(["config", "user.name", "Test"])
+            .current_dir(tmp.path())
+            .output()
+            .unwrap();
+        std::process::Command::new("git")
+            .args(["commit", "--allow-empty", "-m", "init"])
+            .current_dir(tmp.path())
+            .output()
+            .unwrap();
+        // Create an empty feature branch (no commits vs master).
+        std::process::Command::new("git")
+            .args(["checkout", "-b", "feature/story-9997_empty_branch"])
+            .current_dir(tmp.path())
+            .output()
+            .unwrap();
+        std::process::Command::new("git")
+            .args(["checkout", "master"])
+            .current_dir(tmp.path())
+            .output()
+            .unwrap();
+
+        crate::db::ensure_content_store();
+        crate::db::write_item_with_content(
+            "9997_empty_branch",
+            "2_current",
+            "---\nname: Empty Branch Test\n---\n## AC\n- [ ] Implement the feature\n",
+        );
+
+        let ctx = test_ctx(tmp.path());
+        let result = tool_check_criterion(
+            &json!({"story_id": "9997_empty_branch", "criterion_index": 0}),
+            &ctx,
+        );
+
+        // Validation is best-effort — check still proceeds.
+        assert!(
+            result.is_ok(),
+            "Expected ok despite empty branch: {result:?}"
+        );
+
+        // A warning should be in the global log buffer.
+        let warnings = crate::log_buffer::global().get_recent(
+            200,
+            Some("9997_empty_branch"),
+            Some(&crate::log_buffer::LogLevel::Warn),
+        );
+        assert!(
+            !warnings.is_empty(),
+            "Expected a validation warning for empty branch check_criterion"
+        );
+    }
+
     #[test]
     fn tool_check_criterion_missing_story_id() {
         let tmp = tempfile::tempdir().unwrap();