Watch
0
0
Fork
You've already forked hyperhive
0

Compare commits

..
Author SHA1 Message Date
atlas
0472ed3ab5 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.
2026-10-10 21:27:23 +02:00
atlas
9a702d954e 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: ...".
2026-10-10 20:43:20 +02:00
2 changed files with 135 additions and 8 deletions

View file

@ -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

View file

@ -8,6 +8,7 @@ use axum::{
response::{IntoResponse, Response},
};
use serde::{Deserialize, Serialize};
use std::future::Future;
use super::{AppState, SigintOutcome, error_response};
@ -240,7 +241,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<MarkTodosDoneForm>) -> Response {
let ids: Vec<i64> = form
.ids
@ -250,15 +253,61 @@ pub(super) async fn post_mark_todos_done(Form(form): Form<MarkTodosDoneForm>) ->
if ids.is_empty() {
return error_response(StatusCode::BAD_REQUEST, "mark-done: no todo ids selected");
}
let mut acked = 0u64;
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<F, Fut>(ids: Vec<i64>, dial: F) -> Response
where
F: Fn(i64) -> Fut,
Fut: Future<Output = Option<hive_agent_sock::Response>>,
{
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(dial(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<Option<hive_agent_sock::Response>>) -> Result<u64, String> {
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 +317,82 @@ struct MarkTodosDoneBody {
#[cfg(test)]
mod tests {
use super::accept_or_refuse;
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() {
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() {