huskies: merge 915

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

@@ -140,13 +140,14 @@ pub(crate) fn tool_check_criterion(args: &Value, ctx: &AppContext) -> Result<Str
 
     let root = ctx.state.get_project_root()?;
 
-    // Best-effort validation: log a warning if no corroborating evidence is found.
+    // Hard gate: reject if no corroborating evidence exists for this criterion.
-    // 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() {
+        let workflow = ctx
-            validate_criterion_check(&root, story_id, &ac_text, &workflow);
+            .workflow
-        }
+            .lock()
+            .map_err(|e| format!("Lock error: {e}"))?;
+        validate_criterion_check(&root, story_id, &ac_text, &workflow)?;
     }
 
     check_criterion_in_file(&root, story_id, criterion_index)?;
@@ -171,21 +172,21 @@ fn find_unchecked_criterion_text(contents: &str, criterion_index: usize) -> Opti
     None
 }
 
-/// Run best-effort validation before marking a criterion as checked.
+/// Validate that there is corroborating evidence before marking a criterion done.
 ///
 /// 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
+/// Returns `Ok(())` when at least one signal passes, or `Err(message)` describing
-/// the caller can proceed regardless (operator override).
+/// what evidence is missing so the agent knows what to do next.
 fn validate_criterion_check(
     project_root: &Path,
     story_id: &str,
     ac_text: &str,
     workflow: &WorkflowState,
-) {
+) -> Result<(), String> {
     let branch = format!("feature/story-{story_id}");
 
     // ── A: branch has commits vs master ──────────────────────────────────────
@@ -199,8 +200,7 @@ fn validate_criterion_check(
         .unwrap_or_default();
 
     if !commits.is_empty() {
-        // A passes — branch has real work. Validation satisfied.
+        return Ok(());
-        return;
     }
 
     // A fails. Check B and C as fallback evidence.
@@ -230,7 +230,7 @@ fn validate_criterion_check(
     });
 
     if file_mentioned {
-        return;
+        return Ok(());
     }
 
     // ── C: a recorded test name fuzzy-matches the AC text ────────────────────
@@ -251,15 +251,16 @@ fn validate_criterion_check(
             .any(|word| names.iter().any(|n| n.contains(word)))
     });
 
-    if !test_match {
+    if test_match {
-        slog_warn!(
+        return Ok(());
-            "[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
-        );
     }
+
+    Err(format!(
+        "No corroborating evidence for criterion '{ac_text}'. \
+         To proceed: commit your work to '{branch}' (currently has no commits vs master), \
+         add a passing test whose name matches the criterion, \
+         or change a file mentioned in the criterion text."
+    ))
 }
 
 pub(crate) fn tool_edit_criterion(args: &Value, ctx: &AppContext) -> Result<String, String> {
@@ -587,7 +588,7 @@ mod tests {
     }
 
     #[test]
-    fn tool_check_criterion_empty_branch_logs_warning() {
+    fn tool_check_criterion_empty_branch_returns_error() {
         let tmp = tempfile::tempdir().unwrap();
 
         // Init a git repo with an initial commit on master.
@@ -639,21 +640,27 @@ mod tests {
             &ctx,
         );
 
-        // Validation is best-effort — check still proceeds.
+        // No evidence — must error and NOT mark the criterion.
         assert!(
-            result.is_ok(),
+            result.is_err(),
-            "Expected ok despite empty branch: {result:?}"
+            "Expected error when branch has no commits: {result:?}"
+        );
+        let err = result.unwrap_err();
+        assert!(
+            err.contains("No corroborating evidence"),
+            "Error should describe missing evidence, got: {err}"
+        );
+        assert!(
+            err.contains("feature/story-9997_empty_branch"),
+            "Error should name the branch, got: {err}"
         );
 
-        // A warning should be in the global log buffer.
+        // Criterion must still be unchecked in the CRDT.
-        let warnings = crate::log_buffer::global().get_recent(
+        let contents = crate::db::read_content("9997_empty_branch")
-            200,
+            .expect("story content should still be in CRDT");
-            Some("9997_empty_branch"),
-            Some(&crate::log_buffer::LogLevel::Warn),
-        );
         assert!(
-            !warnings.is_empty(),
+            contents.contains("- [ ] Implement the feature"),
-            "Expected a validation warning for empty branch check_criterion"
+            "Criterion should remain unchecked after rejected check_criterion"
         );
     }
 
@@ -691,6 +698,18 @@ mod tests {
         );
 
         let ctx = test_ctx(tmp.path());
+
+        // Provide signal-C evidence: a test whose name matches "first" from the criterion text.
+        tool_record_tests(
+            &json!({
+                "story_id": "9904_test",
+                "unit": [{"name": "first_criterion_check", "status": "pass"}],
+                "integration": []
+            }),
+            &ctx,
+        )
+        .unwrap();
+
         let result = tool_check_criterion(
             &json!({"story_id": "9904_test", "criterion_index": 0}),
             &ctx,