diff --git a/hive-c0re/src/dashboard.rs b/hive-c0re/src/dashboard.rs index b570254a..21e3f7ce 100644 --- a/hive-c0re/src/dashboard.rs +++ b/hive-c0re/src/dashboard.rs @@ -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() } diff --git a/hive-c0re/src/dashboard/approvals.rs b/hive-c0re/src/dashboard/approvals.rs index b85bfb40..60f7be0f 100644 --- a/hive-c0re/src/dashboard/approvals.rs +++ b/hive-c0re/src/dashboard/approvals.rs @@ -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, AxumPath(id): AxumPath, axum::extract::Query(q): axum::extract::Query, -) -> Response { +) -> Result { 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:#}"))), } } diff --git a/hive-c0re/src/dashboard/journal.rs b/hive-c0re/src/dashboard/journal.rs index a0debbc3..708ddbbd 100644 --- a/hive-c0re/src/dashboard/journal.rs +++ b/hive-c0re/src/dashboard/journal.rs @@ -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, axum::extract::Query(q): axum::extract::Query, -) -> Response { +) -> Result { // 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, -) -> Response { +) -> Result { 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}"))), } } diff --git a/hive-c0re/src/dashboard/permissions.rs b/hive-c0re/src/dashboard/permissions.rs index bb32f6f8..3829eb32 100644 --- a/hive-c0re/src/dashboard/permissions.rs +++ b/hive-c0re/src/dashboard/permissions.rs @@ -114,17 +114,19 @@ pub(super) async fn post_tool_groups( State(state): State, AxumPath(name): AxumPath, axum::Json(body): axum::Json, -) -> Response { +) -> Result { 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, AxumPath(name): AxumPath, axum::Json(body): axum::Json, -) -> Response { +) -> Result { 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>, Option>); pub(super) async fn post_permissions( State(state): State, axum::Json(body): axum::Json, -) -> Response { +) -> Result { 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)] diff --git a/hive-c0re/src/dashboard/questions.rs b/hive-c0re/src/dashboard/questions.rs index b94cdd7e..9517c48f 100644 --- a/hive-c0re/src/dashboard/questions.rs +++ b/hive-c0re/src/dashboard/questions.rs @@ -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 diff --git a/hive-c0re/src/dashboard/reminders.rs b/hive-c0re/src/dashboard/reminders.rs index 9f2abb45..d61550b8 100644 --- a/hive-c0re/src/dashboard/reminders.rs +++ b/hive-c0re/src/dashboard/reminders.rs @@ -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) -> Response { match state.coord.broker.list_pending_reminders() { @@ -24,17 +24,18 @@ pub(super) async fn api_reminders(State(state): State) -> Response { pub(super) async fn post_cancel_reminder( State(state): State, AxumPath(id): AxumPath, -) -> Response { +) -> Result { 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, AxumPath(id): AxumPath, -) -> Response { +) -> Result { 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:#}"))), } } diff --git a/hive-c0re/src/dashboard/schedules.rs b/hive-c0re/src/dashboard/schedules.rs index e5bf1ece..ce0525d1 100644 --- a/hive-c0re/src/dashboard/schedules.rs +++ b/hive-c0re/src/dashboard/schedules.rs @@ -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) -> Response { pub(super) async fn post_schedule_new( State(state): State, axum::Json(payload): axum::Json, -) -> Response { +) -> Result { 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:#}"))), } } diff --git a/hive-c0re/src/dashboard/topology.rs b/hive-c0re/src/dashboard/topology.rs index c768830a..2519b6af 100644 --- a/hive-c0re/src/dashboard/topology.rs +++ b/hive-c0re/src/dashboard/topology.rs @@ -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, Form(form): Form, -) -> Response { +) -> Result { 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}"))), } }