dashboard: return problem details directly from client-error handlers

This commit is contained in:
damocles 2026-06-22 15:07:57 +02:00 committed by mara
commit 65ad994c85
8 changed files with 102 additions and 100 deletions

View file

@ -1657,18 +1657,23 @@ fn strip_container_prefix(name: &str) -> String {
.to_owned()
}
/// Convenience wrapper for the common internal-error case: a 500
/// RFC 9457 (`application/problem+json`) response via the
/// `problem_details` crate. `from_status_code` sets `status` + `title`
/// (the canonical reason phrase) and leaves `type` as the default
/// `about:blank`; `with_detail` carries the caller message; the crate's
/// axum `IntoResponse` emits the `application/problem+json` body the
/// frontend parses (it reads `detail`). Most dashboard handlers funnel
/// their errors through here; handlers with a more specific client
/// failure (bad input, not found) build the same `ProblemDetails`
/// inline with the right status.
fn error_response(message: &str) -> Response {
/// The common internal-error case as a `ProblemDetails`: a 500 RFC 9457
/// (`application/problem+json`) value via the `problem_details` crate.
/// `from_status_code` sets `status` + `title` (the canonical reason phrase)
/// and leaves `type` as the default `about:blank`; `with_detail` carries the
/// caller message; the crate's axum `IntoResponse` emits the
/// `application/problem+json` body the frontend parses (it reads `detail`).
/// Handlers that surface client failures return `Result<_, ProblemDetails>`
/// and hand this (or an inline `from_status_code(4xx)`) straight to `Err` —
/// no manual `.into_response()`.
fn error_problem(message: &str) -> problem_details::ProblemDetails {
problem_details::ProblemDetails::from_status_code(StatusCode::INTERNAL_SERVER_ERROR)
.with_detail(message)
.into_response()
}
/// `Response` wrapper around [`error_problem`] for the many handlers typed
/// `-> Response` whose only failure mode is a 500 — they funnel errors
/// through here rather than threading a `Result` return type.
fn error_response(message: &str) -> Response {
error_problem(message).into_response()
}

View file

@ -18,7 +18,7 @@ use serde::Deserialize;
use problem_details::ProblemDetails;
use super::{AppState, error_response};
use super::{AppState, error_problem, error_response};
use crate::actions;
use crate::coordinator::Coordinator;
use crate::lifecycle;
@ -180,19 +180,22 @@ pub(super) async fn get_approval_diff(
State(state): State<AppState>,
AxumPath(id): AxumPath<i64>,
axum::extract::Query(q): axum::extract::Query<DiffBaseQuery>,
) -> Response {
) -> Result<Response, ProblemDetails> {
let base = q.base.as_deref().unwrap_or("applied");
let approval = match state.coord.approvals.get(id) {
Ok(Some(a)) => a,
Ok(None) => return error_response(&format!("approval {id} not found")),
Err(e) => return error_response(&format!("approval {id}: {e:#}")),
Ok(None) => return Err(error_problem(&format!("approval {id} not found"))),
Err(e) => return Err(error_problem(&format!("approval {id}: {e:#}"))),
};
if !matches!(approval.kind, hive_sh4re::ApprovalKind::ApplyCommit) {
return error_response("spawn approvals carry no commit to diff");
return Err(error_problem("spawn approvals carry no commit to diff"));
}
let applied = Coordinator::agent_applied_dir(&approval.agent);
if !applied.join(".git").exists() {
return plain_text(format!("(no applied git repo at {})", applied.display()));
return Ok(plain_text(format!(
"(no applied git repo at {})",
applied.display()
)));
}
let target = format!("refs/tags/proposal/{id}");
let base_ref = match base {
@ -212,21 +215,22 @@ pub(super) async fn get_approval_diff(
.map(|n| format!("refs/tags/proposal/{n}"))
}
other => {
return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail(format!("unknown diff base {other:?}"))
.into_response();
return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail(format!("unknown diff base {other:?}")));
}
};
let Some(base_ref) = base_ref else {
return plain_text(match base {
return Ok(plain_text(match base {
"approved" => "(no earlier approved proposal to diff against)".to_owned(),
_ => "(no previous proposal to diff against)".to_owned(),
});
}));
};
match git_diff_refs(&applied, &base_ref, &target).await {
Ok(s) if s.is_empty() => plain_text("(identical — no changes vs this base)".to_owned()),
Ok(s) => plain_text(s),
Err(e) => error_response(&format!("git diff: {e:#}")),
Ok(s) if s.is_empty() => Ok(plain_text(
"(identical — no changes vs this base)".to_owned(),
)),
Ok(s) => Ok(plain_text(s)),
Err(e) => Err(error_problem(&format!("git diff: {e:#}"))),
}
}

View file

@ -15,7 +15,7 @@ use serde::Deserialize;
use problem_details::ProblemDetails;
use super::{error_response, strip_container_prefix, validate_agent_name};
use super::{error_problem, strip_container_prefix, validate_agent_name};
use crate::lifecycle;
#[derive(Deserialize)]
@ -36,13 +36,14 @@ pub(super) struct JournalQuery {
pub(super) async fn get_journal(
AxumPath(name): AxumPath<String>,
axum::extract::Query(q): axum::extract::Query<JournalQuery>,
) -> Response {
) -> Result<Response, ProblemDetails> {
// Defense-in-depth format check so weird chars never reach the
// shellout below — the `lifecycle::list()` existence check would
// catch them anyway, but rejecting at the boundary keeps the
// failure mode crisp.
if let Some(reason) = validate_agent_name(&name) {
return (StatusCode::BAD_REQUEST, format!("bad agent name: {reason}")).into_response();
return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail(format!("bad agent name: {reason}")));
}
// Validate the container name against the list of managed
// containers so we don't shell out with arbitrary input.
@ -50,9 +51,8 @@ pub(super) async fn get_journal(
let prefixed = format!("{}{container}", lifecycle::AGENT_PREFIX);
let live = lifecycle::list().await.unwrap_or_default();
if !live.iter().any(|c| c == &prefixed) {
return ProblemDetails::from_status_code(StatusCode::NOT_FOUND)
.with_detail(format!("journal: no managed container {prefixed:?}"))
.into_response();
return Err(ProblemDetails::from_status_code(StatusCode::NOT_FOUND)
.with_detail(format!("journal: no managed container {prefixed:?}")));
}
let lines = q.lines.unwrap_or(500).min(5000);
let unit = match q.unit.as_deref().filter(|s| !s.is_empty()) {
@ -65,9 +65,8 @@ pub(super) async fn get_journal(
format!("{u}.service")
};
if !allowed.contains(&unit.as_str()) {
return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail(format!("journal: unknown unit {unit:?}"))
.into_response();
return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail(format!("journal: unknown unit {unit:?}")));
}
Some(unit)
}
@ -92,9 +91,9 @@ pub(super) async fn get_journal(
body.push_str("\n--- stderr ---\n");
body.push_str(&stderr);
}
([("content-type", "text/plain; charset=utf-8")], body).into_response()
Ok(([("content-type", "text/plain; charset=utf-8")], body).into_response())
}
Err(e) => error_response(&format!("journal read: {e:#}")),
Err(e) => Err(error_problem(&format!("journal read: {e:#}"))),
}
}
@ -114,7 +113,7 @@ pub(super) struct JournalHostQuery {
/// dashboard binding to a host-only port.
pub(super) async fn get_journal_host(
axum::extract::Query(q): axum::extract::Query<JournalHostQuery>,
) -> Response {
) -> Result<Response, ProblemDetails> {
let lines = q.lines.unwrap_or(500).min(5000);
let allowed = ["hive-c0re.service"];
let mut cmd = tokio::process::Command::new("journalctl");
@ -127,9 +126,8 @@ pub(super) async fn get_journal_host(
format!("{u}.service")
};
if !allowed.contains(&unit.as_str()) {
return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail(format!("journal-host: unknown unit {unit:?}"))
.into_response();
return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail(format!("journal-host: unknown unit {unit:?}")));
}
cmd.args(["-u", &unit]);
}
@ -140,8 +138,8 @@ pub(super) async fn get_journal_host(
body.push_str("\n--- stderr ---\n");
body.push_str(&String::from_utf8_lossy(&out.stderr));
}
([("content-type", "text/plain; charset=utf-8")], body).into_response()
Ok(([("content-type", "text/plain; charset=utf-8")], body).into_response())
}
Err(e) => error_response(&format!("journalctl spawn: {e}")),
Err(e) => Err(error_problem(&format!("journalctl spawn: {e}"))),
}
}

View file

@ -114,17 +114,19 @@ pub(super) async fn post_tool_groups(
State(state): State<AppState>,
AxumPath(name): AxumPath<String>,
axum::Json(body): axum::Json<SetToolGroupsBody>,
) -> Response {
) -> Result<Response, ProblemDetails> {
let logical = strip_container_prefix(&name);
// `guard_agent_name` yields a ready-made rejection `Response`; pass it
// through as `Ok` (axum sends it verbatim) rather than re-deriving a
// `ProblemDetails` — the guard is shared with `-> Response` handlers.
if let Some(reject) = guard_agent_name(&state, &logical).await {
return reject;
return Ok(reject);
}
// Validate group names before queuing — fail fast so the operator
// sees the error immediately rather than waiting for the worker.
if let Err(e) = crate::tool_groups::validate_groups(&body.groups) {
return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail(format!("invalid tool-groups for {logical}: {e}"))
.into_response();
return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail(format!("invalid tool-groups for {logical}: {e}")));
}
// Enqueue a PermChange so the JSON file write is serialised through
// the FIFO worker. Prevents concurrent batch-apply actions for
@ -139,7 +141,7 @@ pub(super) async fn post_tool_groups(
);
state.coord.emit_rebuild_queue_snapshot();
tracing::info!(agent = %logical, groups = ?body.groups, "operator: set tool-groups via dashboard");
(StatusCode::OK, "ok").into_response()
Ok((StatusCode::OK, "ok").into_response())
}
#[derive(Serialize)]
@ -197,10 +199,10 @@ pub(super) async fn post_capabilities(
State(state): State<AppState>,
AxumPath(name): AxumPath<String>,
axum::Json(body): axum::Json<SetCapabilitiesBody>,
) -> Response {
) -> Result<Response, ProblemDetails> {
let logical = strip_container_prefix(&name);
if let Some(reject) = guard_agent_name(&state, &logical).await {
return reject;
return Ok(reject);
}
let known: Vec<&str> = hive_sh4re::Capability::ALL
.iter()
@ -208,9 +210,8 @@ pub(super) async fn post_capabilities(
.collect();
for cap in &body.caps {
if !known.contains(&cap.as_str()) {
return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail(format!("unknown capability: {cap}"))
.into_response();
return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail(format!("unknown capability: {cap}")));
}
}
// Enqueue a PermChange so the JSON file write is serialised through
@ -226,7 +227,7 @@ pub(super) async fn post_capabilities(
);
state.coord.emit_rebuild_queue_snapshot();
tracing::info!(agent = %logical, caps = ?body.caps, "operator: set capabilities via dashboard");
(StatusCode::OK, "ok").into_response()
Ok((StatusCode::OK, "ok").into_response())
}
/// One agent's slice of a batch permission change. Sparse: an omitted
@ -261,7 +262,7 @@ type StagedPerm = (String, Option<Vec<String>>, Option<Vec<String>>);
pub(super) async fn post_permissions(
State(state): State<AppState>,
axum::Json(body): axum::Json<BatchPermsBody>,
) -> Response {
) -> Result<Response, ProblemDetails> {
let known_caps: Vec<&str> = hive_sh4re::Capability::ALL
.iter()
.map(|c| c.as_str())
@ -273,21 +274,19 @@ pub(super) async fn post_permissions(
for change in &body.changes {
let logical = strip_container_prefix(&change.agent);
if let Some(reject) = guard_agent_name(&state, &logical).await {
return reject;
return Ok(reject);
}
if let Some(groups) = &change.tool_groups
&& let Err(e) = crate::tool_groups::validate_groups(groups)
{
return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail(format!("invalid tool-groups for {logical}: {e}"))
.into_response();
return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail(format!("invalid tool-groups for {logical}: {e}")));
}
if let Some(caps) = &change.capabilities {
for cap in caps {
if !known_caps.contains(&cap.as_str()) {
return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail(format!("unknown capability for {logical}: {cap}"))
.into_response();
return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail(format!("unknown capability for {logical}: {cap}")));
}
}
}
@ -310,7 +309,7 @@ pub(super) async fn post_permissions(
tracing::info!(agent = %logical, "operator: batch perm change via dashboard");
}
state.coord.emit_rebuild_queue_snapshot();
(StatusCode::OK, "ok").into_response()
Ok((StatusCode::OK, "ok").into_response())
}
#[cfg(test)]

View file

@ -28,7 +28,8 @@ pub(super) struct AnswerForm {
/// cross-origin form-POST couldn't already reach. This shim disappears
/// once the unifying gateway makes the agent page same-origin; see
/// `docs/boundary.md`.
fn with_cors(mut resp: Response) -> Response {
fn with_cors(resp: impl IntoResponse) -> Response {
let mut resp = resp.into_response();
resp.headers_mut().insert(
axum::http::header::ACCESS_CONTROL_ALLOW_ORIGIN,
axum::http::HeaderValue::from_static("*"),
@ -45,8 +46,7 @@ pub(super) async fn post_answer_question(
if answer.is_empty() {
return with_cors(
ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail("answer: required")
.into_response(),
.with_detail("answer: required"),
);
}
let resp = match state

View file

@ -12,7 +12,7 @@ use axum::{
use problem_details::ProblemDetails;
use super::{AppState, error_response};
use super::{AppState, error_problem, error_response};
pub(super) async fn api_reminders(State(state): State<AppState>) -> Response {
match state.coord.broker.list_pending_reminders() {
@ -24,17 +24,18 @@ pub(super) async fn api_reminders(State(state): State<AppState>) -> Response {
pub(super) async fn post_cancel_reminder(
State(state): State<AppState>,
AxumPath(id): AxumPath<i64>,
) -> Response {
) -> Result<Response, ProblemDetails> {
match state.coord.broker.cancel_reminder(id) {
Ok(0) => ProblemDetails::from_status_code(StatusCode::NOT_FOUND)
.with_detail(format!("reminder {id} not pending (already delivered?)"))
.into_response(),
Ok(0) => Err(ProblemDetails::from_status_code(StatusCode::NOT_FOUND)
.with_detail(format!("reminder {id} not pending (already delivered?)"))),
Ok(_) => {
tracing::info!(%id, "operator cancelled reminder");
state.coord.emit_reminders_snapshot();
(StatusCode::OK, "ok").into_response()
Ok((StatusCode::OK, "ok").into_response())
}
Err(e) => error_response(&format!("cancel reminder {id} failed: {e:#}")),
Err(e) => Err(error_problem(&format!(
"cancel reminder {id} failed: {e:#}"
))),
}
}
@ -46,16 +47,15 @@ pub(super) async fn post_cancel_reminder(
pub(super) async fn post_retry_reminder(
State(state): State<AppState>,
AxumPath(id): AxumPath<i64>,
) -> Response {
) -> Result<Response, ProblemDetails> {
match state.coord.broker.reset_reminder_failure(id) {
Ok(0) => ProblemDetails::from_status_code(StatusCode::NOT_FOUND)
.with_detail(format!("reminder {id} not pending (already delivered?)"))
.into_response(),
Ok(0) => Err(ProblemDetails::from_status_code(StatusCode::NOT_FOUND)
.with_detail(format!("reminder {id} not pending (already delivered?)"))),
Ok(_) => {
tracing::info!(%id, "operator reset reminder failure for retry");
state.coord.emit_reminders_snapshot();
(StatusCode::OK, "ok").into_response()
Ok((StatusCode::OK, "ok").into_response())
}
Err(e) => error_response(&format!("retry reminder {id} failed: {e:#}")),
Err(e) => Err(error_problem(&format!("retry reminder {id} failed: {e:#}"))),
}
}

View file

@ -13,7 +13,7 @@ use axum::{
use problem_details::ProblemDetails;
use super::{AppState, error_response};
use super::{AppState, error_problem, error_response};
/// `GET /api/schedules` — snapshot of every schedule for the
/// scheduled-prompts tab. Returns the wire shape directly
@ -51,21 +51,18 @@ pub(super) async fn api_schedules(State(state): State<AppState>) -> Response {
pub(super) async fn post_schedule_new(
State(state): State<AppState>,
axum::Json(payload): axum::Json<hive_sh4re::SchedulePromptPayload>,
) -> Response {
) -> Result<Response, ProblemDetails> {
if payload.targets.is_empty() {
return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail("schedule must have at least one target")
.into_response();
return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail("schedule must have at least one target"));
}
if payload.body.trim().is_empty() {
return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail("schedule body must be non-empty")
.into_response();
return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail("schedule body must be non-empty"));
}
if let Some(0) = payload.interval_seconds {
return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail("interval_seconds must be > 0 (use None for one-shot)")
.into_response();
return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail("interval_seconds must be > 0 (use None for one-shot)"));
}
let new = crate::scheduled_prompts::NewSchedule {
owner: hive_sh4re::OPERATOR_RECIPIENT.to_owned(),
@ -79,9 +76,9 @@ pub(super) async fn post_schedule_new(
match state.coord.scheduled_prompts.submit(&new) {
Ok(id) => {
state.coord.emit_schedules_snapshot();
axum::Json(serde_json::json!({"id": id})).into_response()
Ok(axum::Json(serde_json::json!({"id": id})).into_response())
}
Err(e) => error_response(&format!("schedule submit: {e:#}")),
Err(e) => Err(error_problem(&format!("schedule submit: {e:#}"))),
}
}

View file

@ -15,7 +15,7 @@ use serde::Deserialize;
use problem_details::ProblemDetails;
use super::{AppState, error_response};
use super::{AppState, error_problem, error_response};
/// `POST /api/topology/set-parent` body. `child` is required.
/// `new_parent` may be:
@ -52,12 +52,11 @@ pub(super) struct SetParentBulkEntry {
pub(super) async fn post_set_parent(
State(state): State<AppState>,
Form(form): Form<SetParentForm>,
) -> Response {
) -> Result<Response, ProblemDetails> {
let child = form.child.trim().to_owned();
if child.is_empty() {
return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail("set-parent: `child` required")
.into_response();
return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
.with_detail("set-parent: `child` required"));
}
// Empty / whitespace-only `new_parent` ⇒ promote to root. Web
// forms submit the empty string for a "no value" radio button,
@ -83,9 +82,9 @@ pub(super) async fn post_set_parent(
new_parent = ?new_parent,
"operator: set-parent via dashboard"
);
(StatusCode::OK, "ok").into_response()
Ok((StatusCode::OK, "ok").into_response())
}
Err(e) => error_response(&format!("set-parent {child} failed: {e}")),
Err(e) => Err(error_problem(&format!("set-parent {child} failed: {e}"))),
}
}