From 2dcbb78b40182ed756f90da48c537fd16a34bbd0 Mon Sep 17 00:00:00 2001 From: damocles Date: Sat, 25 Jul 2026 17:40:11 +0200 Subject: [PATCH 1/2] hive-agent: add debug logging around todo upsert/clear/mark-done diagnostic instrumentation for #2678 (phantom 'you have todos' wakes after clearing bash-task todos). logs subsystem/key/id/changed on UpsertTodo, subsystem/key/all/count on ClearTodo, id/count on MarkTodoDone, and a marker when the serve loop actually consumes a todo_wake notification. no behavior change - RUST_LOG=debug only. --- hive-agent/src/main.rs | 5 +- hive-agent/src/todo_server.rs | 105 ++++++++++++++++++++-------------- 2 files changed, 66 insertions(+), 44 deletions(-) diff --git a/hive-agent/src/main.rs b/hive-agent/src/main.rs index 1206a5f0..60be8018 100644 --- a/hive-agent/src/main.rs +++ b/hive-agent/src/main.rs @@ -585,7 +585,10 @@ async fn serve_loop( } } { RecvOutcome::Message(first) => first, - RecvOutcome::LocalTodo => synthetic_todo_message(), + RecvOutcome::LocalTodo => { + tracing::debug!("todo wake consumed, sending synthetic todo message"); + synthetic_todo_message() + } RecvOutcome::Empty => { // Idle: no message this poll. Service a queued operator // `/compact` here so it runs even when no turn is driving diff --git a/hive-agent/src/todo_server.rs b/hive-agent/src/todo_server.rs index 3bc280a1..a45e4784 100644 --- a/hive-agent/src/todo_server.rs +++ b/hive-agent/src/todo_server.rs @@ -165,49 +165,10 @@ fn dispatch( bus: &Bus, ) -> Response { match req { - Request::UpsertTodo { - subsystem, - key, - summary, - source, - } => match store.upsert(&subsystem, key.as_deref(), &summary, source.as_deref()) { - Ok((_, changed)) => { - if changed { - wake.notify_one(); - } - Response::Ok - } - Err(e) => err(&e), - }, - Request::ClearTodo { - subsystem, - key, - all, - } => { - let result = if all { - store.clear_subsystem(&subsystem) - } else { - store.clear(&subsystem, key.as_deref()) - }; - match result { - Ok(count) => Response::Acked { - count: u64::try_from(count).unwrap_or(0), - }, - Err(e) => err(&e), - } - } - Request::ListTodos { subsystem } => match store.list(subsystem.as_deref()) { - Ok(todos) => Response::LooseEnds { - loose_ends: todos.into_iter().map(to_loose_end).collect(), - }, - Err(e) => err(&e), - }, - Request::MarkTodoDone { id } => match store.mark_done(id) { - Ok(count) => Response::Acked { - count: u64::try_from(count).unwrap_or(0), - }, - Err(e) => err(&e), - }, + Request::UpsertTodo { .. } + | Request::ClearTodo { .. } + | Request::ListTodos { .. } + | Request::MarkTodoDone { .. } => dispatch_todo(req, store, wake), Request::StoreReminder { message, timing, @@ -260,6 +221,64 @@ fn dispatch( } } +/// Todo-family requests (loose-ends v2). `req` is guaranteed by [`dispatch`] +/// to be one of the four todo variants; any other variant is a caller bug. +fn dispatch_todo(req: Request, store: &Todos, wake: &Notify) -> Response { + match req { + Request::UpsertTodo { + subsystem, + key, + summary, + source, + } => match store.upsert(&subsystem, key.as_deref(), &summary, source.as_deref()) { + Ok((id, changed)) => { + tracing::debug!(subsystem = %subsystem, key = ?key, id, changed, "todo upsert"); + if changed { + wake.notify_one(); + } + Response::Ok + } + Err(e) => err(&e), + }, + Request::ClearTodo { + subsystem, + key, + all, + } => { + let result = if all { + store.clear_subsystem(&subsystem) + } else { + store.clear(&subsystem, key.as_deref()) + }; + match result { + Ok(count) => { + tracing::debug!(subsystem = %subsystem, key = ?key, all, count, "todo clear"); + Response::Acked { + count: u64::try_from(count).unwrap_or(0), + } + } + Err(e) => err(&e), + } + } + Request::ListTodos { subsystem } => match store.list(subsystem.as_deref()) { + Ok(todos) => Response::LooseEnds { + loose_ends: todos.into_iter().map(to_loose_end).collect(), + }, + Err(e) => err(&e), + }, + Request::MarkTodoDone { id } => match store.mark_done(id) { + Ok(count) => { + tracing::debug!(id, count, "todo mark-done"); + Response::Acked { + count: u64::try_from(count).unwrap_or(0), + } + } + Err(e) => err(&e), + }, + _ => unreachable!("dispatch only routes todo variants here"), + } +} + /// `Request::Compact` handler: gate on context usage, then queue the same /// deferred `compact_pending` flag the operator's `/compact` button sets. /// Mirrors `hive-agent::web_ui::actions::post_compact` but reachable from From 6c886d3fa630a2d9491aa22f7465de12226eae06 Mon Sep 17 00:00:00 2001 From: damocles Date: Sat, 25 Jul 2026 18:42:10 +0200 Subject: [PATCH 2/2] hive-agent: inline todo dispatch arms instead of a sub-match + unreachable! per mara's review on #2679: replace the separate dispatch_todo sub-match (with its trailing unreachable! arm) with four small handler functions called directly from dispatch's existing match. same behavior, no unreachable! left in the todo path. --- hive-agent/src/todo_server.rs | 131 ++++++++++++++++++++-------------- 1 file changed, 77 insertions(+), 54 deletions(-) diff --git a/hive-agent/src/todo_server.rs b/hive-agent/src/todo_server.rs index a45e4784..1336f1fb 100644 --- a/hive-agent/src/todo_server.rs +++ b/hive-agent/src/todo_server.rs @@ -165,10 +165,26 @@ fn dispatch( bus: &Bus, ) -> Response { match req { - Request::UpsertTodo { .. } - | Request::ClearTodo { .. } - | Request::ListTodos { .. } - | Request::MarkTodoDone { .. } => dispatch_todo(req, store, wake), + Request::UpsertTodo { + subsystem, + key, + summary, + source, + } => upsert_todo( + store, + wake, + &subsystem, + key.as_deref(), + &summary, + source.as_deref(), + ), + Request::ClearTodo { + subsystem, + key, + all, + } => clear_todo(store, &subsystem, key.as_deref(), all), + Request::ListTodos { subsystem } => list_todos(store, subsystem.as_deref()), + Request::MarkTodoDone { id } => mark_todo_done(store, id), Request::StoreReminder { message, timing, @@ -221,61 +237,68 @@ fn dispatch( } } -/// Todo-family requests (loose-ends v2). `req` is guaranteed by [`dispatch`] -/// to be one of the four todo variants; any other variant is a caller bug. -fn dispatch_todo(req: Request, store: &Todos, wake: &Notify) -> Response { - match req { - Request::UpsertTodo { - subsystem, - key, - summary, - source, - } => match store.upsert(&subsystem, key.as_deref(), &summary, source.as_deref()) { - Ok((id, changed)) => { - tracing::debug!(subsystem = %subsystem, key = ?key, id, changed, "todo upsert"); - if changed { - wake.notify_one(); - } - Response::Ok +/// `UpsertTodo` handler: writes/refreshes a todo row, logs the outcome, and +/// fires `wake` on a new-or-changed upsert so the serve loop runs a turn. +fn upsert_todo( + store: &Todos, + wake: &Notify, + subsystem: &str, + key: Option<&str>, + summary: &str, + source: Option<&str>, +) -> Response { + match store.upsert(subsystem, key, summary, source) { + Ok((id, changed)) => { + tracing::debug!(%subsystem, ?key, id, changed, "todo upsert"); + if changed { + wake.notify_one(); } - Err(e) => err(&e), - }, - Request::ClearTodo { - subsystem, - key, - all, - } => { - let result = if all { - store.clear_subsystem(&subsystem) - } else { - store.clear(&subsystem, key.as_deref()) - }; - match result { - Ok(count) => { - tracing::debug!(subsystem = %subsystem, key = ?key, all, count, "todo clear"); - Response::Acked { - count: u64::try_from(count).unwrap_or(0), - } - } - Err(e) => err(&e), + Response::Ok + } + Err(e) => err(&e), + } +} + +/// `ClearTodo` handler: drops one keyed todo, or every todo in `subsystem` +/// when `all` is set. +fn clear_todo(store: &Todos, subsystem: &str, key: Option<&str>, all: bool) -> Response { + let result = if all { + store.clear_subsystem(subsystem) + } else { + store.clear(subsystem, key) + }; + match result { + Ok(count) => { + tracing::debug!(%subsystem, ?key, all, count, "todo clear"); + Response::Acked { + count: u64::try_from(count).unwrap_or(0), } } - Request::ListTodos { subsystem } => match store.list(subsystem.as_deref()) { - Ok(todos) => Response::LooseEnds { - loose_ends: todos.into_iter().map(to_loose_end).collect(), - }, - Err(e) => err(&e), + Err(e) => err(&e), + } +} + +/// `ListTodos` handler: read-only, so no debug logging — not relevant to +/// diagnosing wake behaviour. +fn list_todos(store: &Todos, subsystem: Option<&str>) -> Response { + match store.list(subsystem) { + Ok(todos) => Response::LooseEnds { + loose_ends: todos.into_iter().map(to_loose_end).collect(), }, - Request::MarkTodoDone { id } => match store.mark_done(id) { - Ok(count) => { - tracing::debug!(id, count, "todo mark-done"); - Response::Acked { - count: u64::try_from(count).unwrap_or(0), - } + Err(e) => err(&e), + } +} + +/// `MarkTodoDone` handler: marks a single todo done by id. +fn mark_todo_done(store: &Todos, id: i64) -> Response { + match store.mark_done(id) { + Ok(count) => { + tracing::debug!(id, count, "todo mark-done"); + Response::Acked { + count: u64::try_from(count).unwrap_or(0), } - Err(e) => err(&e), - }, - _ => unreachable!("dispatch only routes todo variants here"), + } + Err(e) => err(&e), } }