From b6ca08a72bd8331829d2a17930a28aaf3230a216 Mon Sep 17 00:00:00 2001 From: Codex Date: Tue, 14 Jul 2026 07:55:17 -0400 Subject: [PATCH 1/2] fix(delivery): reap stale subagent freeze so busy parents keep receiving A Task subagent that dies without a clean SubagentStop leaves the parent's running_tasks.active stuck true. While stuck, every parent PostToolUse short-circuits the subagent-context branch and delivers nothing, so hcom messages addressed to a busy parent are withheld until the user happens to type a prompt (UserPromptSubmit was the only hook running the dead-subagent cleanup). Observed in the wild as a parent whose running_tasks tracked agent_ids no longer present in the instances table. On the parent's own PostToolUse, reconcile the freeze before honoring it: reap dead subagents (reusing check_dead_subagents, which already treats a missing instances row / stale transcript / interrupt marker as death) and re-evaluate in_subagent_context. A live subagent keeps its instances row, so it is never reaped, active stays true, and parent messages remain deferred to end_task rather than being mis-injected into the subagent's own context. Scoped to PostToolUse because that is the only suppressed hook that carries a parent message delivery. Adds regression tests for both the stale-freeze-cleared and live-subagent-preserved paths. Co-Authored-By: Claude Fable 5 Session: 1422c55e-ec89-44cf-ad66-5e5538a19fb9 Producer: ai-session Tool: cc --- src/hooks/claude.rs | 107 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 106 insertions(+), 1 deletion(-) diff --git a/src/hooks/claude.rs b/src/hooks/claude.rs index 1cebccdb..053c79dc 100644 --- a/src/hooks/claude.rs +++ b/src/hooks/claude.rs @@ -255,9 +255,34 @@ 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: a live subagent keeps its `instances` row, so + // `check_dead_subagents` never reaps it, `active` stays true, and parent + // messages are still 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"); @@ -1582,6 +1607,20 @@ 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` (which removes tracked subagents whose +/// `instances` row is gone, whose transcript went stale, or that were +/// interrupted, resetting `running_tasks.active` to false once none remain) +/// and 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); + instances::in_subagent_context(db, session_id) +} + /// SubagentStart: surface agent_id to subagent. fn subagent_start(raw: &Value) -> Option { let agent_id = raw.get("agent_id").and_then(|v| v.as_str())?; @@ -2733,6 +2772,72 @@ 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")); + } + #[test] fn test_posttooluse_delivery_commits_after_output_write() { let (_dir, db) = make_delivery_test_db(); From a7adf02d9860a2a74dfad6ea1a1ddf6f087c8ee3 Mon Sep 17 00:00:00 2001 From: Codex Date: Tue, 14 Jul 2026 07:55:17 -0400 Subject: [PATCH 2/2] fix(delivery): restrict stale-freeze reconciliation to definitive death signals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up to the stale-subagent-freeze fix: reusing check_dead_subagents wholesale pulled the transcript-mtime staleness heuristic into the PostToolUse reconciliation path. A live subagent that simply hasn't written its transcript for a while (long tool call, waiting on I/O) crosses the stale threshold without being dead; reaping it would unfreeze the parent mid-Task and the next subagent tool call would inject the parent's messages into the subagent's context — a new cross-delivery vector. Introduce DeathSignals to scope which signals a sweep may act on: - All (DB row missing + stale mtime + interrupt marker) stays in effect at UserPromptSubmit, preserving existing behavior at that call site. - DefinitiveOnly (DB row missing or interrupt marker) is what reconcile_stale_subagent_freeze now uses; the mtime heuristic is excluded. Adds a regression test where a live subagent's transcript mtime is aged past the stale threshold: DeathSignals::All must flag it (fixture sanity / UserPromptSubmit behavior preserved) while reconciliation must keep the parent frozen and running_tasks intact. Co-Authored-By: Claude Fable 5 Session: 1422c55e-ec89-44cf-ad66-5e5538a19fb9 Producer: ai-session Tool: cc --- src/hooks/claude.rs | 145 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 132 insertions(+), 13 deletions(-) diff --git a/src/hooks/claude.rs b/src/hooks/claude.rs index 053c79dc..02d21570 100644 --- a/src/hooks/claude.rs +++ b/src/hooks/claude.rs @@ -271,10 +271,12 @@ fn route_claude_hook( // // This is scoped to PostToolUse because that is the only suppressed hook // that carries a parent message delivery. It is safe against genuine - // freezes: a live subagent keeps its `instances` row, so - // `check_dead_subagents` never reaps it, `active` stays true, and parent - // messages are still deferred to `end_task` rather than mis-injected into - // the subagent's own context. + // 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, @@ -288,7 +290,7 @@ fn route_claude_hook( 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 } @@ -1456,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, + signals: DeathSignals, ) -> Vec { let timeout = subagent_timeout.unwrap_or_else(|| { HcomConfig::load(None) @@ -1531,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()) @@ -1564,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, @@ -1585,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; @@ -1610,14 +1638,20 @@ 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` (which removes tracked subagents whose -/// `instances` row is gone, whose transcript went stale, or that were -/// interrupted, resetting `running_tasks.active` to false once none remain) -/// and re-reads `in_subagent_context`. Returns true if the parent is still +/// 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); + cleanup_dead_subagents( + db, + session_id, + transcript_path, + DeathSignals::DefinitiveOnly, + ); instances::in_subagent_context(db, session_id) } @@ -2838,6 +2872,91 @@ mod tests { 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();