fix(delivery): reap stale subagent freeze so busy parents keep receiving

src/hooks/claude.rs

@@ -255,15 +255,42 @@ fn route_claude_hook(
 
     // Subagent context check
     let subagent_check_start = Instant::now();
-    let is_in_subagent_ctx = instances::in_subagent_context(db, &session_id);
+    let mut is_in_subagent_ctx = instances::in_subagent_context(db, &session_id);
     timing.subagent_check_ms = Some(subagent_check_start.elapsed().as_secs_f64() * 1000.0);
 
+    // Stale-freeze reconciliation (fork patch).
+    //
+    // A Task subagent that dies without a clean SubagentStop leaves the
+    // parent's `running_tasks.active` stuck true. While it is stuck, every
+    // parent PostToolUse short-circuits in the block below and delivers
+    // nothing, so hcom messages addressed to a busy parent are withheld until
+    // the user happens to type a prompt (UserPromptSubmit runs the same
+    // cleanup). Reap dead subagents on the parent's own PostToolUse and
+    // re-evaluate, so a resumed parent resumes delivery on its very next tool
+    // call instead of waiting for manual input.
+    //
+    // This is scoped to PostToolUse because that is the only suppressed hook
+    // that carries a parent message delivery. It is safe against genuine
+    // freezes because the reap is restricted to deterministic death signals
+    // (`DeathSignals::DefinitiveOnly`): a live subagent keeps its `instances`
+    // row and carries no interrupt marker, so it is never reaped — even if
+    // its transcript mtime crosses the stale threshold during a long tool
+    // call. `active` stays true and parent messages remain deferred to
+    // `end_task` rather than mis-injected into the subagent's own context.
+    if is_in_subagent_ctx && hook_type == HOOK_POST {
+        is_in_subagent_ctx = reconcile_stale_subagent_freeze(
+            db,
+            &session_id,
+            payload.transcript_path.as_deref().unwrap_or(""),
+        );
+    }
+
     if is_in_subagent_ctx {
         timing.context = Some("subagent");
 
         if hook_type == HOOK_USERPROMPTSUBMIT {
             let transcript_path = payload.transcript_path.as_deref().unwrap_or("");
-            cleanup_dead_subagents(db, &session_id, transcript_path);
+            cleanup_dead_subagents(db, &session_id, transcript_path, DeathSignals::All);
             // Fall through to parent handler for PTY message delivery
         }
 
@@ -1431,12 +1458,30 @@ fn find_subagent_transcript_impl(dir: &Path, target: &str, depth: u32) -> Option
     None
 }
 
+/// Which death signals a dead-subagent sweep is allowed to act on.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum DeathSignals {
+    /// All signals: DB row missing, stale transcript-mtime heuristic, and
+    /// interrupt marker. Used at UserPromptSubmit — the user is present, and
+    /// a heuristic false positive is recoverable on the next turn.
+    All,
+    /// Deterministic signals only: DB row missing or interrupt marker.
+    /// Excludes the transcript-mtime heuristic: a live subagent that simply
+    /// has not written its transcript for a while (long tool call, waiting on
+    /// I/O) crosses the stale threshold without being dead. Used by the
+    /// PostToolUse stale-freeze reconciliation, where a false positive would
+    /// unfreeze the parent mid-Task and the very next subagent tool call
+    /// would inject the parent's messages into the subagent's context.
+    DefinitiveOnly,
+}
+
 /// Check for dead subagents by checking multiple death signals.
 fn check_dead_subagents(
     db: &HcomDb,
     transcript_path: &str,
     running_tasks: &instances::RunningTasks,
     subagent_timeout: Option<i64>,
+    signals: DeathSignals,
 ) -> Vec<String> {
     let timeout = subagent_timeout.unwrap_or_else(|| {
         HcomConfig::load(None)
@@ -1506,8 +1551,10 @@ fn check_dead_subagents(
                 continue;
             }
             Ok(meta) => {
-                // Stale check
-                if let Ok(modified) = meta.modified() {
+                // Stale check (heuristic — only when all signals are allowed)
+                if signals == DeathSignals::All
+                    && let Ok(modified) = meta.modified()
+                {
                     let mtime = modified
                         .duration_since(std::time::UNIX_EPOCH)
                         .map(|d| d.as_secs())
@@ -1539,7 +1586,12 @@ fn check_dead_subagents(
 }
 
 /// Clean up dead subagents from parent's running_tasks.
-fn cleanup_dead_subagents(db: &HcomDb, session_id: &str, transcript_path: &str) {
+fn cleanup_dead_subagents(
+    db: &HcomDb,
+    session_id: &str,
+    transcript_path: &str,
+    signals: DeathSignals,
+) {
     let instance_name = match db.get_session_binding(session_id) {
         Ok(Some(name)) => name,
         _ => return,
@@ -1560,6 +1612,7 @@ fn cleanup_dead_subagents(db: &HcomDb, session_id: &str, transcript_path: &str)
         transcript_path,
         &running_tasks,
         instance_data.subagent_timeout,
+        signals,
     );
     if dead_ids.is_empty() {
         return;
@@ -1582,6 +1635,26 @@ fn cleanup_dead_subagents(db: &HcomDb, session_id: &str, transcript_path: &str)
     }
 }
 
+/// Reap dead subagents from a possibly-stale Task freeze, then report the
+/// post-reconciliation subagent-context verdict.
+///
+/// Runs `cleanup_dead_subagents` restricted to deterministic death signals
+/// (`instances` row gone, or interrupt marker in the transcript — NOT the
+/// stale-mtime heuristic, see `DeathSignals::DefinitiveOnly`), resetting
+/// `running_tasks.active` to false once no tracked subagents remain, then
+/// re-reads `in_subagent_context`. Returns true if the parent is still
+/// genuinely inside a subagent freeze, false if the freeze was stale and has
+/// now been cleared. See the PostToolUse call site for the full rationale.
+fn reconcile_stale_subagent_freeze(db: &HcomDb, session_id: &str, transcript_path: &str) -> bool {
+    cleanup_dead_subagents(
+        db,
+        session_id,
+        transcript_path,
+        DeathSignals::DefinitiveOnly,
+    );
+    instances::in_subagent_context(db, session_id)
+}
+
 /// SubagentStart: surface agent_id to subagent.
 fn subagent_start(raw: &Value) -> Option<Value> {
     let agent_id = raw.get("agent_id").and_then(|v| v.as_str())?;
@@ -2733,6 +2806,157 @@ mod tests {
         assert!(subagent_start(&raw).is_none());
     }
 
+    // Stale Task freeze: a subagent that died without a clean SubagentStop
+    // leaves running_tasks.active stuck true, which otherwise withholds every
+    // parent PostToolUse delivery. Reconciliation must reap it and let the
+    // parent's pending message flow again.
+    #[test]
+    fn test_reconcile_clears_stale_freeze_and_delivers_to_parent() {
+        let (_dir, db) = make_delivery_test_db();
+        db.set_session_binding("sess-1", "nova").unwrap();
+        db.conn()
+            .execute(
+                "UPDATE instances SET running_tasks = ? WHERE name = 'nova'",
+                rusqlite::params![
+                    r#"{"active":true,"subagents":[{"agent_id":"dead-agent-1","type":"general"}]}"#
+                ],
+            )
+            .unwrap();
+
+        // Precondition: the parent looks frozen inside a subagent context.
+        assert!(instances::in_subagent_context(&db, "sess-1"));
+
+        // Reconciliation reaps the dead subagent (no instances row) and clears
+        // the freeze.
+        let still_frozen = reconcile_stale_subagent_freeze(&db, "sess-1", "");
+        assert!(
+            !still_frozen,
+            "stale freeze must clear once the dead subagent is reaped"
+        );
+        assert!(!instances::in_subagent_context(&db, "sess-1"));
+
+        // The parent's pending message is now deliverable at PostToolUse.
+        let (output, _ack) = get_posttooluse_messages(&db, "nova").unwrap();
+        let system_message = output["systemMessage"].as_str().unwrap();
+        assert!(system_message.contains("hello"));
+    }
+
+    // A genuine, live subagent must keep the parent frozen. Reconciliation
+    // must NOT reap it, so parent messages stay deferred to end_task rather
+    // than being mis-injected into the subagent's own context.
+    #[test]
+    fn test_reconcile_preserves_live_subagent_freeze() {
+        let (_dir, db) = make_delivery_test_db();
+        db.set_session_binding("sess-1", "nova").unwrap();
+        db.conn()
+            .execute(
+                "INSERT INTO instances (name, tool, status, status_context, status_time, created_at, last_event_id, agent_id)
+                 VALUES ('sub-a', 'claude', 'active', 'subagent', 0, 0, 0, 'live-agent-1')",
+                [],
+            )
+            .unwrap();
+        db.conn()
+            .execute(
+                "UPDATE instances SET running_tasks = ? WHERE name = 'nova'",
+                rusqlite::params![
+                    r#"{"active":true,"subagents":[{"agent_id":"live-agent-1","type":"general"}]}"#
+                ],
+            )
+            .unwrap();
+
+        let still_frozen = reconcile_stale_subagent_freeze(&db, "sess-1", "");
+        assert!(
+            still_frozen,
+            "a live subagent (instances row present) must keep the parent frozen"
+        );
+        assert!(instances::in_subagent_context(&db, "sess-1"));
+    }
+
+    // Mis-kill guard: a live subagent whose transcript mtime has crossed the
+    // stale threshold (long tool call, no transcript writes) trips the mtime
+    // heuristic — which is exactly why reconciliation must not use it. The
+    // stale-mtime signal must reap under DeathSignals::All (UserPromptSubmit
+    // behavior preserved) but must NOT unfreeze the parent via
+    // reconcile_stale_subagent_freeze (DefinitiveOnly).
+    #[test]
+    fn test_reconcile_ignores_stale_mtime_of_live_subagent() {
+        let (dir, db) = make_delivery_test_db();
+        db.set_session_binding("sess-1", "nova").unwrap();
+        db.conn()
+            .execute(
+                "INSERT INTO instances (name, tool, status, status_context, status_time, created_at, last_event_id, agent_id)
+                 VALUES ('sub-b', 'claude', 'active', 'subagent', 0, 0, 0, 'live-agent-2')",
+                [],
+            )
+            .unwrap();
+        let rt_json =
+            r#"{"active":true,"subagents":[{"agent_id":"live-agent-2","type":"general"}]}"#;
+        db.conn()
+            .execute(
+                "UPDATE instances SET running_tasks = ?, subagent_timeout = 1 WHERE name = 'nova'",
+                rusqlite::params![rt_json],
+            )
+            .unwrap();
+
+        // Parent transcript at {dir}/sess.jsonl → subagent transcript at
+        // {dir}/sess/subagents/agent-live-agent-2.jsonl, with an mtime far
+        // beyond the stale threshold (subagent_timeout=1 → threshold 2s) and
+        // no interrupt marker.
+        let parent_transcript = dir.path().join("sess.jsonl");
+        let subagent_dir = dir.path().join("sess").join("subagents");
+        std::fs::create_dir_all(&subagent_dir).unwrap();
+        let sub_transcript = subagent_dir.join("agent-live-agent-2.jsonl");
+        std::fs::write(&sub_transcript, "{\"type\":\"assistant\"}\n").unwrap();
+        let old_mtime = std::time::SystemTime::now() - std::time::Duration::from_secs(3600);
+        let f = std::fs::File::options()
+            .write(true)
+            .open(&sub_transcript)
+            .unwrap();
+        f.set_times(std::fs::FileTimes::new().set_modified(old_mtime))
+            .unwrap();
+        let transcript_path = parent_transcript.to_string_lossy().to_string();
+
+        // Fixture sanity: the full signal set DOES flag it stale-dead, so the
+        // assertions below cannot pass vacuously.
+        let running_tasks = instances::parse_running_tasks(Some(rt_json));
+        let all_dead = check_dead_subagents(
+            &db,
+            &transcript_path,
+            &running_tasks,
+            Some(1),
+            DeathSignals::All,
+        );
+        assert_eq!(
+            all_dead,
+            vec!["live-agent-2".to_string()],
+            "fixture must cross the stale-mtime threshold under DeathSignals::All"
+        );
+
+        // Reconciliation (definitive signals only) must not reap it: the
+        // freeze holds and running_tasks keeps tracking the live subagent.
+        let still_frozen = reconcile_stale_subagent_freeze(&db, "sess-1", &transcript_path);
+        assert!(
+            still_frozen,
+            "stale mtime alone must not unfreeze the parent during reconciliation"
+        );
+        assert!(instances::in_subagent_context(&db, "sess-1"));
+        let rt_after: String = db
+            .conn()
+            .query_row(
+                "SELECT running_tasks FROM instances WHERE name = 'nova'",
+                [],
+                |row| row.get(0),
+            )
+            .unwrap();
+        let parsed = instances::parse_running_tasks(Some(&rt_after));
+        assert!(parsed.active);
+        assert_eq!(parsed.subagents.len(), 1);
+        assert_eq!(
+            parsed.subagents[0].get("agent_id").and_then(|v| v.as_str()),
+            Some("live-agent-2")
+        );
+    }
+
     #[test]
     fn test_posttooluse_delivery_commits_after_output_write() {
         let (_dir, db) = make_delivery_test_db();