huskies: merge 783

2026-04-28 11:17:40 +00:00
parent d70719e23c
commit 6c2bdde695
8 changed files with 1087 additions and 1069 deletions
@@ -0,0 +1,470 @@
+//! Limit-enforcement, kill-respawn, per-session counting, and retry tests
+//! for the watchdog (bugs 624, 646, 650).
+
+use super::super::super::super::{AgentPool, composite_key};
+use super::{write_fake_budget_session_log, write_fake_session_log, write_project_config};
+use crate::agents::{AgentEvent, AgentStatus, TerminationReason};
+
+// ── Limit enforcement integration tests (bug 624) ────────────────────────
+
+#[test]
+fn watchdog_terminates_agent_exceeding_turn_limit() {
+    let tmp = tempfile::tempdir().unwrap();
+    let root = tmp.path();
+
+    write_project_config(
+        root,
+        r#"
+[[agent]]
+name = "coder-1"
+runtime = "claude-code"
+max_turns = 10
+"#,
+    );
+
+    // Write 12 turns in the current session (exceeds limit of 10).
+    write_fake_session_log(root, "story_a", "coder-1", "sess-current", 12);
+
+    let pool = AgentPool::new_test(3001);
+    let tx = pool.inject_test_agent_with_session(
+        "story_a",
+        "coder-1",
+        AgentStatus::Running,
+        "sess-current",
+    );
+    let mut rx = tx.subscribe();
+
+    let found = pool.run_watchdog_pass(Some(root));
+    assert!(found >= 1, "watchdog should detect the over-limit agent");
+
+    // Agent should now be Failed with TurnLimit reason.
+    {
+        let agents = pool.agents.lock().unwrap();
+        let key = composite_key("story_a", "coder-1");
+        let agent = agents.get(&key).unwrap();
+        assert_eq!(agent.status, AgentStatus::Failed);
+        assert_eq!(
+            agent.termination_reason,
+            Some(TerminationReason::TurnLimit),
+            "termination reason must be TurnLimit"
+        );
+    }
+
+    let event = rx.try_recv().expect("watchdog must emit an Error event");
+    assert!(
+        matches!(event, AgentEvent::Error { .. }),
+        "expected AgentEvent::Error, got: {event:?}"
+    );
+}
+
+#[test]
+fn watchdog_terminates_agent_exceeding_budget_limit() {
+    let tmp = tempfile::tempdir().unwrap();
+    let root = tmp.path();
+
+    write_project_config(
+        root,
+        r#"
+[[agent]]
+name = "coder-1"
+runtime = "claude-code"
+max_budget_usd = 5.00
+"#,
+    );
+
+    // Write $6.00 in the current session's log (exceeds limit of $5.00).
+    write_fake_budget_session_log(root, "story_b", "coder-1", "sess-budget", 6.00);
+
+    let pool = AgentPool::new_test(3001);
+    let tx = pool.inject_test_agent_with_session(
+        "story_b",
+        "coder-1",
+        AgentStatus::Running,
+        "sess-budget",
+    );
+    let mut rx = tx.subscribe();
+
+    let found = pool.run_watchdog_pass(Some(root));
+    assert!(found >= 1, "watchdog should detect the over-budget agent");
+
+    {
+        let agents = pool.agents.lock().unwrap();
+        let key = composite_key("story_b", "coder-1");
+        let agent = agents.get(&key).unwrap();
+        assert_eq!(agent.status, AgentStatus::Failed);
+        assert_eq!(
+            agent.termination_reason,
+            Some(TerminationReason::BudgetLimit),
+            "termination reason must be BudgetLimit"
+        );
+    }
+
+    let event = rx.try_recv().expect("watchdog must emit an Error event");
+    assert!(matches!(event, AgentEvent::Error { .. }));
+}
+
+#[test]
+fn watchdog_does_not_terminate_agent_under_limits() {
+    let tmp = tempfile::tempdir().unwrap();
+    let root = tmp.path();
+
+    write_project_config(
+        root,
+        r#"
+[[agent]]
+name = "coder-1"
+runtime = "claude-code"
+max_turns = 50
+max_budget_usd = 10.00
+"#,
+    );
+
+    // Agent is under both limits in the current session.
+    write_fake_session_log(root, "story_c", "coder-1", "sess-ok", 25);
+    write_fake_budget_session_log(root, "story_c", "coder-1", "sess-ok-budget", 3.00);
+
+    let pool = AgentPool::new_test(3001);
+    // Use the turns session (the budget session is a separate file;
+    // resolve_session_log picks the specified one for turns, and
+    // find_latest_log for budget if needed — but here the turns file
+    // has 25 turns < 50 so no violation).
+    pool.inject_test_agent_with_session("story_c", "coder-1", AgentStatus::Running, "sess-ok");
+
+    let found = pool.run_watchdog_pass(Some(root));
+    assert_eq!(found, 0, "agent under limits should not be terminated");
+
+    {
+        let agents = pool.agents.lock().unwrap();
+        let key = composite_key("story_c", "coder-1");
+        let agent = agents.get(&key).unwrap();
+        assert_eq!(
+            agent.status,
+            AgentStatus::Running,
+            "agent under limits should stay Running"
+        );
+        assert!(agent.termination_reason.is_none());
+    }
+}
+
+/// Regression test for the original bug 624 incident:
+/// coder-1 with max_turns=50, max_budget_usd=5.00 ran 5.6× over the turn
+/// limit (280 turns). The watchdog must terminate at the turn limit (turns
+/// hit first in the observed trace), with reason TurnLimit.
+#[test]
+fn regression_bug624_coder1_story623_trajectory() {
+    let tmp = tempfile::tempdir().unwrap();
+    let root = tmp.path();
+
+    write_project_config(
+        root,
+        r#"
+[[agent]]
+name = "coder-1"
+runtime = "claude-code"
+max_turns = 50
+max_budget_usd = 5.00
+"#,
+    );
+
+    // Simulate the trajectory: 55 turns in the current session (just past
+    // the limit) and $2.50 spent (under budget). Turns hit first, so
+    // reason should be TurnLimit.
+    write_fake_session_log(root, "story_623", "coder-1", "sess-624", 55);
+
+    let pool = AgentPool::new_test(3001);
+    let tx = pool.inject_test_agent_with_session(
+        "story_623",
+        "coder-1",
+        AgentStatus::Running,
+        "sess-624",
+    );
+    let mut rx = tx.subscribe();
+
+    let found = pool.run_watchdog_pass(Some(root));
+    assert!(found >= 1, "watchdog must catch the turn-limit violation");
+
+    {
+        let agents = pool.agents.lock().unwrap();
+        let key = composite_key("story_623", "coder-1");
+        let agent = agents.get(&key).unwrap();
+        assert_eq!(agent.status, AgentStatus::Failed);
+        assert_eq!(
+            agent.termination_reason,
+            Some(TerminationReason::TurnLimit),
+            "turns hit first in the observed trace, so reason must be TurnLimit"
+        );
+    }
+
+    // The error event should have been emitted.
+    let event = rx.try_recv().expect("watchdog must emit an Error event");
+    if let AgentEvent::Error { message, .. } = &event {
+        assert!(
+            message.contains("turn limit"),
+            "error message should mention turn limit, got: {message}"
+        );
+    } else {
+        panic!("expected AgentEvent::Error, got: {event:?}");
+    }
+}
+
+// ── Kill-respawn loop fix (bug 646), updated for per-session + retry ───
+
+/// When the watchdog terminates an agent for limit-exceeded AND the story
+/// has exhausted its retries, it must be marked `blocked: true` in CRDT
+/// state so `auto_assign_available_work` won't re-spawn the agent.
+///
+/// This test seeds a single session that legitimately exceeds the limit
+/// and uses `max_retries = 1` so that the first violation blocks.
+#[test]
+fn watchdog_marks_story_blocked_after_limit_termination() {
+    crate::db::ensure_content_store();
+
+    let tmp = tempfile::tempdir().unwrap();
+    let root = tmp.path();
+
+    write_project_config(
+        root,
+        r#"
+max_retries = 1
+
+[[agent]]
+name = "coder-1"
+runtime = "claude-code"
+max_turns = 10
+"#,
+    );
+
+    // Write story content into the CRDT-backed content store so the
+    // watchdog's retry/block path has something to read+update.
+    let story_id = "42_story_runaway";
+    let initial = "---\nname: Runaway Story\n---\n# Runaway Story\n";
+    crate::db::write_content(story_id, initial);
+
+    // 12 turns in a single session exceeds the configured max of 10.
+    write_fake_session_log(root, story_id, "coder-1", "sess-runaway", 12);
+
+    let pool = AgentPool::new_test(3001);
+    let _tx = pool.inject_test_agent_with_session(
+        story_id,
+        "coder-1",
+        AgentStatus::Running,
+        "sess-runaway",
+    );
+
+    let found = pool.run_watchdog_pass(Some(root));
+    assert!(found >= 1, "watchdog should detect the over-limit agent");
+
+    // With max_retries=1, the first violation blocks immediately.
+    let updated = crate::db::read_content(story_id)
+        .expect("story content must still exist after watchdog termination");
+    assert!(
+        updated.contains("blocked: true"),
+        "story must be marked `blocked: true` after limit termination with max_retries=1 — got:\n{updated}"
+    );
+
+    // Sanity: the agent itself is also Failed with the right reason.
+    {
+        let agents = pool.agents.lock().unwrap();
+        let key = composite_key(story_id, "coder-1");
+        let agent = agents.get(&key).unwrap();
+        assert_eq!(agent.status, AgentStatus::Failed);
+        assert_eq!(
+            agent.termination_reason,
+            Some(TerminationReason::TurnLimit),
+            "termination reason must be TurnLimit"
+        );
+    }
+}
+
+// ── Per-session counting (bug 650) ──────────────────────────────────────
+
+/// Seed multiple prior session log files whose combined assistant-event
+/// count exceeds `max_turns`. Then inject a NEW running agent with a
+/// fresh session_id whose log has fewer events than `max_turns`.
+/// Assert the agent is NOT terminated (per-session count is under the
+/// limit) AND the story is NOT marked blocked.
+#[test]
+fn per_session_counting_does_not_terminate_under_limit() {
+    let tmp = tempfile::tempdir().unwrap();
+    let root = tmp.path();
+
+    write_project_config(
+        root,
+        r#"
+[[agent]]
+name = "coder-1"
+runtime = "claude-code"
+max_turns = 10
+"#,
+    );
+
+    // 3 prior sessions with 5 turns each = 15 total (above max_turns=10).
+    write_fake_session_log(root, "story_d", "coder-1", "old-sess-1", 5);
+    write_fake_session_log(root, "story_d", "coder-1", "old-sess-2", 5);
+    write_fake_session_log(root, "story_d", "coder-1", "old-sess-3", 5);
+
+    // New running session has only 3 turns (under limit of 10).
+    write_fake_session_log(root, "story_d", "coder-1", "new-sess", 3);
+
+    let pool = AgentPool::new_test(3001);
+    pool.inject_test_agent_with_session("story_d", "coder-1", AgentStatus::Running, "new-sess");
+
+    let found = pool.run_watchdog_pass(Some(root));
+    assert_eq!(
+        found, 0,
+        "agent under per-session limit should NOT be terminated"
+    );
+
+    {
+        let agents = pool.agents.lock().unwrap();
+        let key = composite_key("story_d", "coder-1");
+        let agent = agents.get(&key).unwrap();
+        assert_eq!(
+            agent.status,
+            AgentStatus::Running,
+            "agent under per-session limit should stay Running"
+        );
+        assert!(agent.termination_reason.is_none());
+    }
+}
+
+/// Same setup as per_session_counting_does_not_terminate_under_limit, but
+/// the new agent's own session log exceeds `max_turns`. Assert the agent
+/// IS terminated AND (with max_retries=1) the story IS marked blocked.
+#[test]
+fn per_session_counting_terminates_over_limit() {
+    crate::db::ensure_content_store();
+
+    let tmp = tempfile::tempdir().unwrap();
+    let root = tmp.path();
+
+    write_project_config(
+        root,
+        r#"
+max_retries = 1
+
+[[agent]]
+name = "coder-1"
+runtime = "claude-code"
+max_turns = 10
+"#,
+    );
+
+    let story_id = "story_e_per_session";
+    crate::db::write_content(story_id, "---\nname: Per-Session Test\n---\n");
+
+    // Prior session with 5 turns (under limit alone).
+    write_fake_session_log(root, story_id, "coder-1", "old-sess", 5);
+
+    // Current session with 12 turns (over limit).
+    write_fake_session_log(root, story_id, "coder-1", "new-sess", 12);
+
+    let pool = AgentPool::new_test(3001);
+    let tx =
+        pool.inject_test_agent_with_session(story_id, "coder-1", AgentStatus::Running, "new-sess");
+    let mut rx = tx.subscribe();
+
+    let found = pool.run_watchdog_pass(Some(root));
+    assert!(
+        found >= 1,
+        "agent over per-session limit must be terminated"
+    );
+
+    {
+        let agents = pool.agents.lock().unwrap();
+        let key = composite_key(story_id, "coder-1");
+        let agent = agents.get(&key).unwrap();
+        assert_eq!(agent.status, AgentStatus::Failed);
+        assert_eq!(agent.termination_reason, Some(TerminationReason::TurnLimit),);
+    }
+
+    let event = rx.try_recv().expect("watchdog must emit an Error event");
+    assert!(matches!(event, AgentEvent::Error { .. }));
+
+    // With max_retries=1, the story is blocked.
+    let updated = crate::db::read_content(story_id).unwrap();
+    assert!(
+        updated.contains("blocked: true"),
+        "story must be blocked after per-session overrun with max_retries=1"
+    );
+}
+
+// ── Retry semantic integration test (bug 650) ───────────────────────────
+
+/// With `max_retries = 3`, simulate separate sessions each exceeding
+/// `max_turns`. After session 1: retry_count=1, NOT blocked. After
+/// session 2: retry_count=2, NOT blocked. After session 3:
+/// retry_count=3 >= max_retries, story IS blocked.
+#[test]
+fn watchdog_retry_semantic_blocks_after_max_retries() {
+    crate::db::ensure_content_store();
+
+    let tmp = tempfile::tempdir().unwrap();
+    let root = tmp.path();
+
+    write_project_config(
+        root,
+        r#"
+max_retries = 3
+
+[[agent]]
+name = "coder-1"
+runtime = "claude-code"
+max_turns = 10
+"#,
+    );
+
+    let story_id = "88_story_retry_watchdog";
+    let initial = "---\nname: Retry Test\n---\n";
+    crate::db::write_content(story_id, initial);
+
+    // Session 1: exceeds limit → retry_count=1, NOT blocked.
+    {
+        write_fake_session_log(root, story_id, "coder-1", "session-1", 12);
+        let pool = AgentPool::new_test(3001);
+        pool.inject_test_agent_with_session(story_id, "coder-1", AgentStatus::Running, "session-1");
+        pool.run_watchdog_pass(Some(root));
+
+        let content = crate::db::read_content(story_id).unwrap();
+        assert!(
+            content.contains("retry_count: 1"),
+            "after session 1, retry_count should be 1 — got:\n{content}"
+        );
+        assert!(
+            !content.contains("blocked: true"),
+            "story should NOT be blocked after session 1"
+        );
+    }
+
+    // Session 2: exceeds limit → retry_count=2, NOT blocked.
+    {
+        write_fake_session_log(root, story_id, "coder-1", "session-2", 12);
+        let pool = AgentPool::new_test(3001);
+        pool.inject_test_agent_with_session(story_id, "coder-1", AgentStatus::Running, "session-2");
+        pool.run_watchdog_pass(Some(root));
+
+        let content = crate::db::read_content(story_id).unwrap();
+        assert!(
+            content.contains("retry_count: 2"),
+            "after session 2, retry_count should be 2 — got:\n{content}"
+        );
+        assert!(
+            !content.contains("blocked: true"),
+            "story should NOT be blocked after session 2"
+        );
+    }
+
+    // Session 3: exceeds limit → retry_count=3 >= max_retries(3), IS blocked.
+    {
+        write_fake_session_log(root, story_id, "coder-1", "session-3", 12);
+        let pool = AgentPool::new_test(3001);
+        pool.inject_test_agent_with_session(story_id, "coder-1", AgentStatus::Running, "session-3");
+        pool.run_watchdog_pass(Some(root));
+
+        let content = crate::db::read_content(story_id).unwrap();
+        assert!(
+            content.contains("blocked: true"),
+            "story must be blocked after session 3 (retry_count=3 >= max_retries=3) — got:\n{content}"
+        );
+    }
+}
@@ -0,0 +1,70 @@
+//! Shared test helpers for the watchdog module.
+
+use std::path::Path;
+
+mod limits_tests;
+mod orphan_tests;
+
+/// Write a fake session log file with `n` assistant turn entries.
+///
+/// The file is named `{agent_name}-{session_id}.log` to match the
+/// real naming convention used by `AgentLogWriter`.
+pub(super) fn write_fake_session_log(
+    project_root: &Path,
+    story_id: &str,
+    agent_name: &str,
+    session_id: &str,
+    n_turns: u64,
+) {
+    let log_dir = project_root.join(".huskies").join("logs").join(story_id);
+    std::fs::create_dir_all(&log_dir).unwrap();
+    let log_path = log_dir.join(format!("{agent_name}-{session_id}.log"));
+    let mut content = String::new();
+    for _ in 0..n_turns {
+        content.push_str(
+            &serde_json::to_string(&serde_json::json!({
+                "timestamp": "2026-04-25T00:00:00Z",
+                "type": "agent_json",
+                "story_id": story_id,
+                "agent_name": agent_name,
+                "data": { "type": "assistant", "message": {} }
+            }))
+            .unwrap(),
+        );
+        content.push('\n');
+    }
+    std::fs::write(log_path, content).unwrap();
+}
+
+/// Write a fake session log containing a `result` event with the given cost.
+///
+/// Used to test budget enforcement via the watchdog's per-session log
+/// reading (not `token_usage.jsonl`).
+pub(super) fn write_fake_budget_session_log(
+    project_root: &Path,
+    story_id: &str,
+    agent_name: &str,
+    session_id: &str,
+    cost_usd: f64,
+) {
+    let log_dir = project_root.join(".huskies").join("logs").join(story_id);
+    std::fs::create_dir_all(&log_dir).unwrap();
+    let log_path = log_dir.join(format!("{agent_name}-{session_id}.log"));
+    let content = serde_json::to_string(&serde_json::json!({
+        "timestamp": "2026-04-25T00:00:00Z",
+        "type": "agent_json",
+        "story_id": story_id,
+        "agent_name": agent_name,
+        "data": { "type": "result", "total_cost_usd": cost_usd }
+    }))
+    .unwrap()
+        + "\n";
+    std::fs::write(log_path, content).unwrap();
+}
+
+/// Write a minimal project.toml with the given agent config.
+pub(super) fn write_project_config(project_root: &Path, config_toml: &str) {
+    let huskies_dir = project_root.join(".huskies");
+    std::fs::create_dir_all(&huskies_dir).unwrap();
+    std::fs::write(huskies_dir.join("project.toml"), config_toml).unwrap();
+}
@@ -0,0 +1,112 @@
+//! Orphan-detection tests for the watchdog (bug 161).
+
+use super::super::super::super::{AgentPool, composite_key};
+use super::super::orphan::check_orphaned_agents;
+use crate::agents::{AgentEvent, AgentStatus};
+
+// ── check_orphaned_agents return value tests (bug 161) ──────────────────
+
+#[tokio::test]
+async fn check_orphaned_agents_returns_count_of_orphaned_agents() {
+    let pool = AgentPool::new_test(3001);
+
+    // Spawn two tasks that finish immediately.
+    let h1 = tokio::spawn(async {});
+    let h2 = tokio::spawn(async {});
+    tokio::time::sleep(std::time::Duration::from_millis(20)).await;
+    assert!(h1.is_finished());
+    assert!(h2.is_finished());
+
+    pool.inject_test_agent_with_handle("story_a", "coder", AgentStatus::Running, h1);
+    pool.inject_test_agent_with_handle("story_b", "coder", AgentStatus::Running, h2);
+
+    let found = check_orphaned_agents(&pool.agents);
+    assert_eq!(found, 2, "should detect both orphaned agents");
+}
+
+#[test]
+fn check_orphaned_agents_returns_zero_when_no_orphans() {
+    let pool = AgentPool::new_test(3001);
+    // Inject agents in terminal states — not orphaned.
+    pool.inject_test_agent("story_a", "coder", AgentStatus::Completed);
+    pool.inject_test_agent("story_b", "qa", AgentStatus::Failed);
+
+    let found = check_orphaned_agents(&pool.agents);
+    assert_eq!(
+        found, 0,
+        "no orphans should be detected for terminal agents"
+    );
+}
+
+#[tokio::test]
+async fn watchdog_detects_orphaned_running_agent() {
+    let pool = AgentPool::new_test(3001);
+
+    let handle = tokio::spawn(async {});
+    tokio::time::sleep(std::time::Duration::from_millis(20)).await;
+    assert!(
+        handle.is_finished(),
+        "task should be finished before injection"
+    );
+
+    let tx =
+        pool.inject_test_agent_with_handle("orphan_story", "coder", AgentStatus::Running, handle);
+    let mut rx = tx.subscribe();
+
+    pool.run_watchdog_once();
+
+    {
+        let agents = pool.agents.lock().unwrap();
+        let key = composite_key("orphan_story", "coder");
+        let agent = agents.get(&key).unwrap();
+        assert_eq!(
+            agent.status,
+            AgentStatus::Failed,
+            "watchdog must mark an orphaned Running agent as Failed"
+        );
+    }
+
+    let event = rx.try_recv().expect("watchdog must emit an Error event");
+    assert!(
+        matches!(event, AgentEvent::Error { .. }),
+        "expected AgentEvent::Error, got: {event:?}"
+    );
+}
+
+#[tokio::test]
+async fn watchdog_orphan_detection_returns_nonzero_enabling_auto_assign() {
+    // This test verifies the contract that `check_orphaned_agents` returns
+    // a non-zero count when orphans exist, which the watchdog uses to
+    // decide whether to trigger auto-assign (bug 161).
+    let pool = AgentPool::new_test(3001);
+
+    let handle = tokio::spawn(async {});
+    tokio::time::sleep(std::time::Duration::from_millis(20)).await;
+
+    pool.inject_test_agent_with_handle("orphan_story", "coder", AgentStatus::Running, handle);
+
+    // Before watchdog: agent is Running.
+    {
+        let agents = pool.agents.lock().unwrap();
+        let key = composite_key("orphan_story", "coder");
+        assert_eq!(agents.get(&key).unwrap().status, AgentStatus::Running);
+    }
+
+    // Run watchdog pass — should return 1 (orphan found).
+    let found = check_orphaned_agents(&pool.agents);
+    assert_eq!(
+        found, 1,
+        "watchdog must return 1 for a single orphaned agent"
+    );
+
+    // After watchdog: agent is Failed.
+    {
+        let agents = pool.agents.lock().unwrap();
+        let key = composite_key("orphan_story", "coder");
+        assert_eq!(
+            agents.get(&key).unwrap().status,
+            AgentStatus::Failed,
+            "orphaned agent must be marked Failed"
+        );
+    }
+}