Watch
0
0
Fork
You've already forked hyperhive
0

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: ...".
This commit is contained in:
atlas 2026-10-10 20:40:16 +02:00
commit 9a702d954e
2 changed files with 77 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 the checked ids to `POST /api/todos/mark-done`, which dismisses them
from the harness-local store (same effect as `cancel_loose_end(kind: 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). "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 ## Live view

View file

@ -240,7 +240,9 @@ pub(super) struct MarkTodosDoneForm {
/// MCP tool — unchanged. Unknown/already-acked ids just don't add to the /// 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 /// `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 /// 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 { pub(super) async fn post_mark_todos_done(Form(form): Form<MarkTodosDoneForm>) -> Response {
let ids: Vec<i64> = form let ids: Vec<i64> = form
.ids .ids
@ -250,15 +252,48 @@ pub(super) async fn post_mark_todos_done(Form(form): Form<MarkTodosDoneForm>) ->
if ids.is_empty() { if ids.is_empty() {
return error_response(StatusCode::BAD_REQUEST, "mark-done: no todo ids selected"); 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 { for id in ids {
if let Some(hive_agent_sock::Response::Acked { count }) = answers
crate::todo_server::dial(&hive_agent_sock::Request::MarkTodoDone { id }).await .push(crate::todo_server::dial(&hive_agent_sock::Request::MarkTodoDone { id }).await);
{ }
acked += count; 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)] #[derive(Serialize)]
@ -268,7 +303,38 @@ struct MarkTodosDoneBody {
#[cfg(test)] #[cfg(test)]
mod tests { 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] #[test]
fn an_offered_value_is_accepted() { fn an_offered_value_is_accepted() {