From 9a702d954eb54bd597b8c1b8b56f4014efcd3bf8 Mon Sep 17 00:00:00 2001 From: atlas Date: Sat, 10 Oct 2026 20:40:16 +0200 Subject: [PATCH 1/2] hive-agent: web UI "mark done" reports a failed todo store instead of success post_mark_todos_done dropped every dial failure and Response::Err and answered 200 {"acked":0}, so the todos panel showed "marked done" and cleared its checkboxes while the todos stayed. Any failing id now fails the request with a 500 naming how many ids failed; the panel already shows non-OK bodies as "failed: ...". --- docs/web-ui/agent.md | 3 ++ hive-agent/src/web_ui/actions.rs | 82 ++++++++++++++++++++++++++++---- 2 files changed, 77 insertions(+), 8 deletions(-) diff --git a/docs/web-ui/agent.md b/docs/web-ui/agent.md index eb6bc4c2..40cfcec7 100644 --- a/docs/web-ui/agent.md +++ b/docs/web-ui/agent.md @@ -160,6 +160,9 @@ select-all / select-none / `✓ mark done` bulk row above the list POSTs the checked ids to `POST /api/todos/mark-done`, which dismisses them from the harness-local store (same effect as `cancel_loose_end(kind: "todo")`, just from the web UI instead of the agent's own tool calls). +If the todo store errors or doesn't answer for any id, the endpoint returns +500 naming how many ids failed, and the panel shows `failed: …` instead of +`✓ marked done`. ## Live view diff --git a/hive-agent/src/web_ui/actions.rs b/hive-agent/src/web_ui/actions.rs index a09488fd..f3fc004b 100644 --- a/hive-agent/src/web_ui/actions.rs +++ b/hive-agent/src/web_ui/actions.rs @@ -240,7 +240,9 @@ pub(super) struct MarkTodosDoneForm { /// MCP tool — unchanged. Unknown/already-acked ids just don't add to the /// `acked` count (same "acking twice is not a new action" semantics as the /// single-id path); a request with no ids or where every id fails to parse -/// is rejected as a client error rather than silently acking nothing. +/// is rejected as a client error rather than silently acking nothing. An id +/// whose store call errors or gets no answer fails the request with a 500 +/// naming how many failed. pub(super) async fn post_mark_todos_done(Form(form): Form) -> Response { let ids: Vec = form .ids @@ -250,15 +252,48 @@ pub(super) async fn post_mark_todos_done(Form(form): Form) -> if ids.is_empty() { return error_response(StatusCode::BAD_REQUEST, "mark-done: no todo ids selected"); } - let mut acked = 0u64; + let mut answers = Vec::with_capacity(ids.len()); for id in ids { - if let Some(hive_agent_sock::Response::Acked { count }) = - crate::todo_server::dial(&hive_agent_sock::Request::MarkTodoDone { id }).await - { - acked += count; + answers + .push(crate::todo_server::dial(&hive_agent_sock::Request::MarkTodoDone { id }).await); + } + match settle_mark_done(answers) { + Ok(acked) => axum::Json(MarkTodosDoneBody { acked }).into_response(), + Err(message) => error_response(StatusCode::INTERNAL_SERVER_ERROR, &message), + } +} + +/// Folds the per-id answers into the acked total, or the operator-facing +/// failure. `None` is a dial that got no answer; `Response::Err` and any +/// other variant are failures. `Acked { count: 0 }` is a success. +fn settle_mark_done(answers: Vec>) -> Result { + let total = answers.len(); + let mut acked = 0u64; + let mut failed = 0usize; + let mut first_error = None; + for answer in answers { + match answer { + Some(hive_agent_sock::Response::Acked { count }) => acked += count, + Some(hive_agent_sock::Response::Err { message }) => { + failed += 1; + first_error.get_or_insert(message); + } + Some(other) => { + failed += 1; + first_error.get_or_insert(format!("unexpected response: {other:?}")); + } + None => { + failed += 1; + first_error.get_or_insert_with(|| "the todo store did not answer".to_owned()); + } } } - axum::Json(MarkTodosDoneBody { acked }).into_response() + match first_error { + None => Ok(acked), + Some(error) => Err(format!( + "mark-done: {failed} of {total} failed ({acked} marked done): {error}" + )), + } } #[derive(Serialize)] @@ -268,7 +303,38 @@ struct MarkTodosDoneBody { #[cfg(test)] mod tests { - use super::accept_or_refuse; + use super::{accept_or_refuse, settle_mark_done}; + + #[test] + fn mark_done_with_no_answer_is_an_error() { + let err = settle_mark_done(vec![None]).unwrap_err(); + assert!(err.contains("1 of 1 failed"), "{err}"); + } + + #[test] + fn mark_done_surfaces_the_store_error() { + let answers = vec![Some(hive_agent_sock::Response::Err { + message: "database is locked".to_owned(), + })]; + let err = settle_mark_done(answers).unwrap_err(); + assert!(err.contains("database is locked"), "{err}"); + } + + #[test] + fn mark_done_partial_failure_reports_both_counts() { + let answers = vec![Some(hive_agent_sock::Response::Acked { count: 1 }), None]; + let err = settle_mark_done(answers).unwrap_err(); + assert!(err.contains("1 of 2 failed (1 marked done)"), "{err}"); + } + + #[test] + fn mark_done_acking_nothing_new_is_still_success() { + let answers = vec![ + Some(hive_agent_sock::Response::Acked { count: 0 }), + Some(hive_agent_sock::Response::Acked { count: 2 }), + ]; + assert_eq!(settle_mark_done(answers).unwrap(), 2); + } #[test] fn an_offered_value_is_accepted() { From 0472ed3ab5fea89104700dbdbb542770c5d172dd Mon Sep 17 00:00:00 2001 From: atlas Date: Sat, 10 Oct 2026 21:27:23 +0200 Subject: [PATCH 2/2] hive-agent: test the mark-done handler's 500 through an injected dial post_mark_todos_done's body moves into mark_ids_done, which takes the per-id dial as a closure, so tests cover the 500 and 200 responses without reaching the live in-agent socket. --- hive-agent/src/web_ui/actions.rs | 64 ++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 3 deletions(-) diff --git a/hive-agent/src/web_ui/actions.rs b/hive-agent/src/web_ui/actions.rs index f3fc004b..21d2243f 100644 --- a/hive-agent/src/web_ui/actions.rs +++ b/hive-agent/src/web_ui/actions.rs @@ -8,6 +8,7 @@ use axum::{ response::{IntoResponse, Response}, }; use serde::{Deserialize, Serialize}; +use std::future::Future; use super::{AppState, SigintOutcome, error_response}; @@ -252,10 +253,23 @@ pub(super) async fn post_mark_todos_done(Form(form): Form) -> if ids.is_empty() { return error_response(StatusCode::BAD_REQUEST, "mark-done: no todo ids selected"); } + mark_ids_done(ids, |id| async move { + crate::todo_server::dial(&hive_agent_sock::Request::MarkTodoDone { id }).await + }) + .await +} + +/// The handler body behind `dial`, which answers one id's `MarkTodoDone` +/// (`None` when the store doesn't answer). Tests inject it so they never +/// reach the live in-agent socket. +async fn mark_ids_done(ids: Vec, dial: F) -> Response +where + F: Fn(i64) -> Fut, + Fut: Future>, +{ let mut answers = Vec::with_capacity(ids.len()); for id in ids { - answers - .push(crate::todo_server::dial(&hive_agent_sock::Request::MarkTodoDone { id }).await); + answers.push(dial(id).await); } match settle_mark_done(answers) { Ok(acked) => axum::Json(MarkTodosDoneBody { acked }).into_response(), @@ -303,7 +317,51 @@ struct MarkTodosDoneBody { #[cfg(test)] mod tests { - use super::{accept_or_refuse, settle_mark_done}; + use super::{accept_or_refuse, mark_ids_done, settle_mark_done}; + + async fn body_text(response: axum::response::Response) -> String { + let bytes = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + String::from_utf8(bytes.to_vec()).unwrap() + } + + #[tokio::test] + async fn mark_done_handler_answers_500_with_the_message_when_the_store_does_not_answer() { + let response = mark_ids_done(vec![1, 2], |_| async { None }).await; + assert_eq!( + response.status(), + axum::http::StatusCode::INTERNAL_SERVER_ERROR + ); + let body = body_text(response).await; + assert!(body.contains("2 of 2 failed"), "{body}"); + assert!(body.contains("did not answer"), "{body}"); + } + + #[tokio::test] + async fn mark_done_handler_answers_500_with_the_store_error() { + let response = mark_ids_done(vec![7], |_| async { + Some(hive_agent_sock::Response::Err { + message: "database is locked".to_owned(), + }) + }) + .await; + assert_eq!( + response.status(), + axum::http::StatusCode::INTERNAL_SERVER_ERROR + ); + assert!(body_text(response).await.contains("database is locked")); + } + + #[tokio::test] + async fn mark_done_handler_answers_200_with_the_acked_total() { + let response = mark_ids_done(vec![1, 2], |_| async { + Some(hive_agent_sock::Response::Acked { count: 1 }) + }) + .await; + assert_eq!(response.status(), axum::http::StatusCode::OK); + assert_eq!(body_text(response).await, r#"{"acked":2}"#); + } #[test] fn mark_done_with_no_answer_is_an_error() {