fix(daemon): treat any harness output as a heartbeat for the idle watchdog

src/sdk/src/daemon/providers/execute.rs

@@ -4,6 +4,7 @@
 //! opencode SQLite-lock exits with jittered exponential backoff.
 
 use std::process::Stdio;
+use std::sync::atomic::{AtomicU64, Ordering};
 use std::sync::{Arc, Mutex};
 use std::time::Duration;
 
@@ -335,10 +336,22 @@ async fn run_provider_attempt(
     let stdout = child.stdout.take().ok_or("child has no stdout")?;
     let stderr = child.stderr.take().ok_or("child has no stderr")?;
 
-    // stderr tail collector.
+    // stderr tail collector, which doubles as a heartbeat source: a child that
+    // is logging to stderr is demonstrably alive even while it emits no parsed
+    // events, and killing it as "idle" throws away real work.
     let stderr_tail = Arc::new(Mutex::new(String::new()));
+    // Monotonic origin for encoding stderr beats (see `stderr_beat`); sharing
+    // one base with the watchdog below lets it tell how old a beat is.
+    let beat_base = Instant::now();
+    // Holds the timestamp (micros since `beat_base`) of the most recent stderr
+    // line rather than a bare counter, so the idle watchdog can re-arm from the
+    // beat's *own* time instead of from whenever it next happens to check.
+    let stderr_beat = Arc::new(AtomicU64::new(0));
     let stderr_task = {
         let stderr_tail = stderr_tail.clone();
+        let stderr_beat = stderr_beat.clone();
+        // `beat_base` is `Copy`, so the `async move` block captures it by copy;
+        // it remains in scope for the watchdog's stale-beat arithmetic below.
         tokio::spawn(async move {
             let mut reader = BufReader::new(stderr);
             let mut buf = Vec::new();
@@ -348,6 +361,8 @@ async fn run_provider_attempt(
                     Ok(0) => break,
                     Ok(_) => {
                         let chunk = String::from_utf8_lossy(&buf);
+                        stderr_beat
+                            .store(beat_base.elapsed().as_micros() as u64, Ordering::Relaxed);
                         let mut tail = stderr_tail.lock().unwrap();
                         tail.push_str(&chunk);
                         *tail = tail_bytes(&tail);
@@ -380,9 +395,18 @@ async fn run_provider_attempt(
     let mut line_no: i64 = 0;
     let mut stdout_tail = String::new();
 
-    // Idle watchdog: killed only after `timeout_ms` with NO new event; each event
-    // pushes the deadline out. Armed at start to cover a child that emits nothing.
+    // Idle watchdog: killed only after `timeout_ms` with NO sign of life; each
+    // one pushes the deadline out. Armed at start to cover a child that emits
+    // nothing at all.
+    //
+    // "Sign of life" is deliberately wider than "parsed event". A harness that
+    // spends twenty minutes inside one tool call — a cold `cargo test`, a long
+    // lint — emits no semantic events for the whole of it, and treating that as
+    // a hang killed sessions mid-task and discarded everything they had not yet
+    // pushed. Any output on either pipe now counts, so the watchdog still fires
+    // on a genuinely wedged child while a working one is left alone.
     let mut deadline = Instant::now() + Duration::from_millis(spec.timeout_ms);
+    let mut seen_stderr = stderr_beat.load(Ordering::Relaxed);
     let mut buf = Vec::new();
 
     let idle_error = format!(
@@ -401,6 +425,26 @@ async fn run_provider_attempt(
                 return Err(format!("{} task aborted", provider_name(spec.provider)));
             }
             _ = tokio::time::sleep_until(deadline) => {
+                // stderr arrives on its own task, so it cannot push the deadline
+                // out directly; the deadline firing is where it is claimed. A
+                // beat since the deadline was armed means the child spoke during
+                // the window and is not idle.
+                let beat = stderr_beat.load(Ordering::Relaxed);
+                if beat != seen_stderr {
+                    seen_stderr = beat;
+                    // Re-arm from the beat's *own* timestamp, not from now: a
+                    // beat may have gone stale while stdout kept pushing the
+                    // window out, and a stale beat must not grant the child a
+                    // second full timeout once it finally hangs. If even the
+                    // beat's window has lapsed, the child is idle after all.
+                    let beat_deadline = beat_base
+                        + Duration::from_micros(beat)
+                        + Duration::from_millis(spec.timeout_ms);
+                    if beat_deadline > Instant::now() {
+                        deadline = beat_deadline;
+                        continue;
+                    }
+                }
                 let _ = child.start_kill();
                 let _ = child.wait().await;
                 report_workspace_context(&mapper, spec);
@@ -440,9 +484,11 @@ async fn run_provider_attempt(
                             on_event.as_mut(),
                         );
                         line_no += 1;
-                        if produced {
-                            deadline = Instant::now() + Duration::from_millis(spec.timeout_ms);
-                        }
+                        // Any line at all is proof of life, mapped or not: a
+                        // record this build does not understand still came from
+                        // a running child, and only a silent pipe means idle.
+                        let _ = produced;
+                        deadline = Instant::now() + Duration::from_millis(spec.timeout_ms);
                     }
                     Err(_) => break,
                 }

src/sdk/src/daemon/providers/tests.rs

@@ -407,3 +407,103 @@ async fn abort_cancelled_resolves_when_signalled() {
     // Already-aborted: cancelled returns immediately.
     abort.cancelled().await;
 }
+
+/// Build a fake claude CLI at `path` from `body`, and the options that run it
+/// with `timeout_ms` as the idle budget. Shared by the watchdog tests below.
+#[cfg(unix)]
+fn idle_probe_options(
+    dir: &std::path::Path,
+    body: &str,
+    timeout_ms: u64,
+) -> (std::path::PathBuf, RunTaskOptions) {
+    use std::os::unix::fs::PermissionsExt;
+
+    let harness = dir.join("fake-claude");
+    std::fs::write(&harness, body).unwrap();
+    std::fs::set_permissions(&harness, std::fs::Permissions::from_mode(0o755)).unwrap();
+    let options = RunTaskOptions {
+        transport: Default::default(),
+        conversation: "peer".into(),
+        session_class: crate::sessions::SessionClass::Unbound,
+        resume_session_id: None,
+        workspace_context: Default::default(),
+        provider: HarnessProvider::Claude,
+        prompt: "go".into(),
+        cwd: dir.to_string_lossy().into_owned(),
+        env: HashMap::from([(
+            "MEDULLA_CLAUDE_BIN".into(),
+            harness.to_string_lossy().into_owned(),
+        )]),
+        timeout_ms,
+        model: None,
+        agent: None,
+        extra_args: Vec::new(),
+        skip_permissions: false,
+        abort: Abort::new(),
+        router: None,
+        attribution: false,
+        hooks: crate::harness_hooks::HooksConfig::default(),
+        on_event: None,
+        on_stdin: None,
+        on_session: None,
+        on_workspace_context: None,
+    };
+    (harness, options)
+}
+
+/// A harness deep inside one long tool call logs progress to stderr and emits no
+/// parsed event for far longer than the idle budget. That is a working child,
+/// not a hung one, and killing it discards everything it has not yet pushed.
+#[cfg(unix)]
+#[tokio::test]
+async fn stderr_chatter_keeps_a_working_child_alive() {
+    let dir = tempfile::tempdir().unwrap();
+    let (_bin, options) = idle_probe_options(
+        dir.path(),
+        "#!/bin/sh\ni=0\nwhile [ $i -lt 12 ]; do echo \"compiling crate $i\" >&2; sleep 0.1; i=$((i+1)); done\nprintf '%s\\n' '{\"type\":\"result\",\"result\":\"built\"}'\n",
+        300,
+    );
+
+    let result = super::execute::run_provider_task(options).await;
+
+    assert_eq!(result.unwrap().reply, "built");
+}
+
+/// stdout records this build does not map still prove the child is running, so
+/// they must push the deadline out too — only a silent pipe means idle.
+#[cfg(unix)]
+#[tokio::test]
+async fn unmapped_stdout_records_keep_a_working_child_alive() {
+    let dir = tempfile::tempdir().unwrap();
+    let (_bin, options) = idle_probe_options(
+        dir.path(),
+        "#!/bin/sh\ni=0\nwhile [ $i -lt 12 ]; do printf '%s\\n' '{\"type\":\"not_a_kind_we_map\"}'; sleep 0.1; i=$((i+1)); done\nprintf '%s\\n' '{\"type\":\"result\",\"result\":\"done\"}'\n",
+        300,
+    );
+
+    let result = super::execute::run_provider_task(options).await;
+
+    assert_eq!(result.unwrap().reply, "done");
+}
+
+/// The watchdog still exists: a child that says nothing at all on either pipe
+/// is killed on the idle budget rather than hanging the run.
+#[cfg(unix)]
+#[tokio::test]
+async fn a_wholly_silent_child_is_still_killed_as_idle() {
+    let dir = tempfile::tempdir().unwrap();
+    let (_bin, options) = idle_probe_options(
+        dir.path(),
+        "#!/bin/sh\nsleep 5\nprintf '%s\\n' '{\"type\":\"result\",\"result\":\"too late\"}'\n",
+        300,
+    );
+
+    let error = super::execute::run_provider_task(options)
+        .await
+        .expect_err("a silent child must trip the watchdog");
+
+    assert!(
+        error.contains("idle for 300ms"),
+        "unexpected error: {error}"
+    );
+}