fix(#2635): close review nits on the questions mirror (inc2 pt2)

hive-agent-mcp/src/mcp/mod.rs

@@ -242,9 +242,22 @@ impl AgentServer {
                     answer: args.answer,
                 })
                 .await;
-            if resp.is_ok() {
-                // Best-effort — this agent is done owing a reply for `id`;
-                // drop its `answering` mirror row.
+            // Clear the local `answering` mirror row whenever this agent is
+            // done owing a reply for `id`: either a genuine success, or
+            // c0re telling us the question already resolved without us
+            // (the asker cancelled/answered it first, surfacing as an
+            // "already answered"/"not found" rejection) — in both cases
+            // nothing is still owed, so the row would otherwise linger
+            // stale. Any other rejection (e.g. wrong answerer) means the
+            // question is still genuinely outstanding, so leave it be.
+            let should_clear = match &resp {
+                Ok(hive_core_agent_sock::Response::Ok) => true,
+                Ok(hive_core_agent_sock::Response::Err { message }) => {
+                    message.contains("not found") || message.contains("already answered")
+                }
+                _ => false,
+            };
+            if should_clear {
                 let _ = dial_agent_socket(&hive_agent_sock::Request::ClearQuestion { id }).await;
             }
             annotate_retries(

hive-agent/src/questions.rs

@@ -19,12 +19,14 @@
 //!   `answer()` for `id` (see `hive-agent-mcp`'s tool impl).
 //!
 //! Known gap: if the asker cancels their own question, the target is not
-//! notified today (`hive-c0re::questions::handle_cancel_loose_end` only
-//! notifies a *different* canceller than the asker, which never happens
-//! via the ownership-gated agent-socket cancel path). A target's
-//! `answering` row then lingers until they call `answer()` (now a "no
-//! such question" error from c0re) or the row goes stale. Flagged on the
-//! tracking issue rather than fixed here.
+//! proactively notified today (`hive-c0re::questions::handle_cancel_loose_end`
+//! only notifies a *different* canceller than the asker, which never
+//! happens via the ownership-gated agent-socket cancel path). A target's
+//! `answering` row is cleared the next time they call `answer()` — c0re's
+//! "already answered"/"not found" rejection is treated as resolved-without-
+//! us and clears the mirror row (see `hive-agent-mcp`'s `answer()` tool) —
+//! but if they never call `answer()` at all, the row lingers with no
+//! proactive nudge. Flagged on the tracking issue rather than fully fixed.
 
 use std::path::Path;
 use std::sync::Mutex;
@@ -141,11 +143,15 @@ impl Questions {
     }
 
     /// List every mirrored row, oldest-asked first — for `get_loose_ends`
-    /// rendering.
+    /// rendering. A row whose `role` column doesn't parse (corruption —
+    /// should never happen via this module's own writes) is logged and
+    /// skipped rather than either propagating a hard error (which would
+    /// hide every other, valid row) or silently misattributing it as
+    /// `Asked` (which would lie about who owes whom a reply).
     ///
     /// # Errors
     ///
-    /// Propagates the sqlite prepare / query failures.
+    /// Propagates the sqlite prepare / query / column-read failures.
     ///
     /// # Panics
     ///
@@ -155,25 +161,27 @@ impl Questions {
         let mut stmt = conn.prepare(
             "SELECT id, role, peer, question, asked_at FROM questions ORDER BY asked_at ASC",
         )?;
-        let rows = stmt
-            .query_map([], row_to_question)?
-            .collect::<rusqlite::Result<Vec<_>>>()?;
-        Ok(rows)
+        let mut out = Vec::new();
+        let mut rows = stmt.query([])?;
+        while let Some(row) = rows.next()? {
+            let id: i64 = row.get(0)?;
+            let role_str: String = row.get(1)?;
+            let Some(role) = Role::parse(&role_str) else {
+                tracing::warn!(%id, %role_str, "questions mirror: unknown role, skipping row");
+                continue;
+            };
+            out.push(QuestionMirror {
+                id,
+                role,
+                peer: row.get(2)?,
+                question: row.get(3)?,
+                asked_at: row.get(4)?,
+            });
+        }
+        Ok(out)
     }
 }
 
-fn row_to_question(row: &rusqlite::Row) -> rusqlite::Result<QuestionMirror> {
-    let role_str: String = row.get(1)?;
-    let role = Role::parse(&role_str).unwrap_or(Role::Asked);
-    Ok(QuestionMirror {
-        id: row.get(0)?,
-        role,
-        peer: row.get(2)?,
-        question: row.get(3)?,
-        asked_at: row.get(4)?,
-    })
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

hive-agent/src/todo_server.rs

@@ -172,9 +172,9 @@ async fn handle_conn(
 /// Apply one request to the store, firing `wake` on a new/changed upsert so
 /// the serve loop runs a turn. Each arm calls a small named handler function
 /// directly — no sub-match/`unreachable!()` indirection per family (that
-/// pattern got reviewed out of the todo family in #2679; kept the reminder
-/// and question families consistent with it here rather than reintroducing
-/// it). `reminders`/`questions` are `None` when that store failed to open at
+/// pattern got reviewed out of the todo family in the diagnostic-logging
+/// follow-up PR; kept the reminder and question families consistent with it
+/// here rather than reintroducing it). `reminders`/`questions` are `None` when that store failed to open at
 /// boot, in which case every op in that family returns an `Err` — each
 /// handler checks for its own `None` case.
 fn dispatch(