From dc4c5460d5676919959e2a703b040689e67a04a8 Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 22 Jun 2026 14:15:38 +0200 Subject: [PATCH 1/9] feat(#1877): restructure hive-forge into pr/issue sub-verbs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Group issue/PR operations under `pr` and `issue` parent commands (`hive-forge pr close 42`, `issue create …`, `pr status --pr 42`) per the operator decision — kind-namespaced verbs replace the flat surface. - new `verbs::pr_cmd` / `verbs::issue_cmd` parent commands wrap the existing per-verb modules (reuse their Args + run fns) under `#[command(subcommand)]`. - kind-validation (the win over the old generic verbs): the generics that work on both (view/comment/comments/close/labels/assign/timeline) call `assert_kind` first, so `pr close ` / `issue close ` are rejected with a 'use the other command' message. PR-only / issue-only verbs are kind-correct by construction. `number` exposed `pub(crate)` on the shared verbs so the wrappers can probe it. - every flat kind verb (`close`, `pr-create`, `pr-status`, `issue-edit`, …) kept as a `#[command(hide = true)]` back-compat alias — still parses, dropped from --help; removed in a later sweep once usage migrates. (`pr`/`issue` bare-show become `pr show` / `issue show` — the names are now parents.) - docs/tools/forge.md documents the new surface + the deprecated aliases. cargo build/clippy/fmt clean, 54 tests pass; --help surface + alias parsing smoke-tested. --- docs/tools/forge.md | 18 ++++++ hive-forge/src/main.rs | 31 +++++++++-- hive-forge/src/verbs/assign.rs | 2 +- hive-forge/src/verbs/close.rs | 2 +- hive-forge/src/verbs/comment.rs | 2 +- hive-forge/src/verbs/comments.rs | 2 +- hive-forge/src/verbs/issue_cmd.rs | 81 +++++++++++++++++++++++++++ hive-forge/src/verbs/labels.rs | 2 +- hive-forge/src/verbs/mod.rs | 33 +++++++++++ hive-forge/src/verbs/pr_cmd.rs | 93 +++++++++++++++++++++++++++++++ hive-forge/src/verbs/timeline.rs | 2 +- hive-forge/src/verbs/view.rs | 2 +- 12 files changed, 257 insertions(+), 13 deletions(-) create mode 100644 hive-forge/src/verbs/issue_cmd.rs create mode 100644 hive-forge/src/verbs/pr_cmd.rs diff --git a/docs/tools/forge.md b/docs/tools/forge.md index 53b5cb51..42e50906 100644 --- a/docs/tools/forge.md +++ b/docs/tools/forge.md @@ -12,7 +12,25 @@ as a proper Rust binary). Use it instead of ad-hoc curl pipelines. ## Verbs +**Kind-namespaced commands (preferred):** issue/PR operations are grouped +under `issue` and `pr` parent commands — `hive-forge pr close 42`, +`hive-forge issue create --title …`, `hive-forge pr status --pr 42`. The +`pr ` / `issue ` forms validate the number's kind (e.g. `pr close` +refuses an issue number, which the old generic `close` couldn't). Run +`hive-forge pr --help` / `hive-forge issue --help` for the full subcommand +list (show/create/edit/status/merge/reviews/commits/diff/view/comment/ +comments/close/labels/assign/timeline as applicable). + +The flat forms below (`close 42`, `pr-create …`, `pr-status …`, …) still work +as **hidden back-compat aliases** during the transition and are dropped from +`--help`; prefer the namespaced form. They'll be removed in a later sweep. + ```bash +hive-forge pr close 42 # close a PR (kind-validated) +hive-forge issue close 42 # close an issue (kind-validated) +hive-forge pr status --pr 42 # PR health (mergeable / CI / reviews) +hive-forge issue create --title "..." --body "..." +# --- flat aliases below remain valid (hidden) --- hive-forge view 42 # title + body + comments hive-forge comments 42 # list all comments (human-readable) hive-forge comments 42 --tail 10 # last 10 comments (count-then-page; efficient on long threads) diff --git a/hive-forge/src/main.rs b/hive-forge/src/main.rs index 9aab6341..bc0f26a9 100644 --- a/hive-forge/src/main.rs +++ b/hive-forge/src/main.rs @@ -47,39 +47,54 @@ struct Cli { #[derive(Subcommand)] enum Verb { + // The kind verbs below are hidden back-compat aliases of the new + // `pr ` / `issue ` forms (`pr-close` -> `pr close`, bare + // `close` -> `pr close` / `issue close`, etc.). They still parse but are + // dropped from `--help`; a later change removes them once usage migrates. /// Dump title + body + all comments for an issue or PR. + #[command(hide = true)] View(verbs::view::Args), - /// Print key fields of an issue as JSON. - Issue(verbs::issue::Args), + /// Issue-scoped commands: `issue …`. + Issue(verbs::issue_cmd::Args), /// Create an issue. Prints the issue URL on success. + #[command(hide = true)] IssueCreate(verbs::issue_create::Args), /// Edit an issue's title, body, state, or milestone. + #[command(hide = true)] IssueEdit(verbs::issue_edit::Args), - /// Print key fields of a PR as JSON. - Pr(verbs::pr::Args), + /// PR-scoped commands: `pr …`. + Pr(verbs::pr_cmd::Args), /// List a PR's commits as JSON (sha, message, author date, author). /// Survives rebase-rewritten shas — message + author date let a /// caller match the rows against linear `main` history. + #[command(hide = true)] PrCommits(verbs::pr_commits::Args), /// Create a pull request. Prints the PR URL on success. + #[command(hide = true)] PrCreate(verbs::pr_create::Args), /// Post a comment on an issue or PR. + #[command(hide = true)] Comment(verbs::comment::Args), /// List all comments on an issue or PR. + #[command(hide = true)] Comments(verbs::comments::Args), /// Print the body (or full JSON) of a single comment by id. CommentShow(verbs::comment_show::Args), /// Edit an existing comment by id. CommentEdit(verbs::comment_edit::Args), /// Assign or unassign a user on an issue or PR. + #[command(hide = true)] Assign(verbs::assign::Args), /// Close an issue or PR. + #[command(hide = true)] Close(verbs::close::Args), /// List, add, or remove labels on an issue or PR. + #[command(hide = true)] Labels(verbs::labels::Args), /// PR health view: mergeable state, CI checks, requested reviewers + /// review verdicts, last-comment time (`--pr `). `--sha` is a /// CI-only fast path. Exit code is a merge-readiness verdict. + #[command(hide = true)] PrStatus(verbs::pr_status::Args), /// Clone a forge repo (default `-r`/`HIVE_FORGE_REPO`) with /// credentials auto-injected. Pairs with `pr-create --agit`. @@ -113,21 +128,25 @@ enum Verb { /// Merge a PR (`--method merge|rebase`, default merge). Refuses unless /// mergeable + CI not red + no changes requested (`--force` overrides). /// Deletes the head branch unless `--keep-branch`. No squash option. + #[command(hide = true)] PrMerge(verbs::pr_merge::Args), /// List a PR's reviews, or submit one: `--approve` / /// `--request-changes` / `--comment` (with `-m` for the body). + #[command(hide = true)] PrReviews(verbs::pr_reviews::Args), /// List branches, optionally filtered. Branches(verbs::branches::Args), /// Print the tree SHA at a branch or commit. TreeSha(verbs::tree_sha::Args), /// Print the unified diff for a PR. + #[command(hide = true)] Diff(verbs::diff::Args), /// Get or set this user's watch subscription on a repo. Subscription(verbs::subscription::Args), /// List timeline events on an issue or PR (closes, label adds, /// assignments, commit refs, pushes, etc.) — the audit trail /// `view` + `comments` don't surface. + #[command(hide = true)] Timeline(verbs::timeline::Args), /// Upload a file as an attachment to an issue. AttachIssue(verbs::attach::IssueArgs), @@ -152,10 +171,10 @@ fn main() -> Result<()> { let client = client::Client::from_env(cli.repo, cli.json).context("initialize forge client")?; match cli.verb { Verb::View(a) => verbs::view::run(&client, a), - Verb::Issue(a) => verbs::issue::run(&client, a), + Verb::Issue(a) => verbs::issue_cmd::run(&client, a), Verb::IssueCreate(a) => verbs::issue_create::run(&client, a), Verb::IssueEdit(a) => verbs::issue_edit::run(&client, a), - Verb::Pr(a) => verbs::pr::run(&client, a), + Verb::Pr(a) => verbs::pr_cmd::run(&client, a), Verb::PrCommits(a) => verbs::pr_commits::run(&client, a), Verb::PrCreate(a) => verbs::pr_create::run(&client, a), Verb::Comment(a) => verbs::comment::run(&client, a), diff --git a/hive-forge/src/verbs/assign.rs b/hive-forge/src/verbs/assign.rs index d257e239..c4a7ff8f 100644 --- a/hive-forge/src/verbs/assign.rs +++ b/hive-forge/src/verbs/assign.rs @@ -13,7 +13,7 @@ use crate::verbs::print_json; #[derive(ClapArgs)] pub struct Args { /// Issue or PR number. - number: u64, + pub(crate) number: u64, /// User login to assign (or unassign with `--remove`). user: String, /// Remove the user instead of adding. diff --git a/hive-forge/src/verbs/close.rs b/hive-forge/src/verbs/close.rs index 0d01ce27..d67c5aea 100644 --- a/hive-forge/src/verbs/close.rs +++ b/hive-forge/src/verbs/close.rs @@ -10,7 +10,7 @@ use crate::verbs::print_json; #[derive(ClapArgs)] pub struct Args { /// Issue or PR number. - number: u64, + pub(crate) number: u64, } pub fn run(client: &Client, args: Args) -> Result<()> { diff --git a/hive-forge/src/verbs/comment.rs b/hive-forge/src/verbs/comment.rs index 2a99a380..f5bd774c 100644 --- a/hive-forge/src/verbs/comment.rs +++ b/hive-forge/src/verbs/comment.rs @@ -12,7 +12,7 @@ use crate::verbs::print_json; #[derive(ClapArgs)] pub struct Args { /// Issue or PR number. - number: u64, + pub(crate) number: u64, /// Inline body text. #[arg(long, conflicts_with = "body_file")] body: Option, diff --git a/hive-forge/src/verbs/comments.rs b/hive-forge/src/verbs/comments.rs index 6c5bcb6a..36783665 100644 --- a/hive-forge/src/verbs/comments.rs +++ b/hive-forge/src/verbs/comments.rs @@ -31,7 +31,7 @@ const PAGE_SIZE: usize = 50; #[derive(ClapArgs)] pub struct Args { /// Issue or PR number. - number: u64, + pub(crate) number: u64, /// Page size for the head-of-thread shape (Forgejo caps at 50). /// Mutually exclusive with `--tail`. #[arg(long, default_value_t = 50, conflicts_with = "tail")] diff --git a/hive-forge/src/verbs/issue_cmd.rs b/hive-forge/src/verbs/issue_cmd.rs new file mode 100644 index 00000000..993cbaeb --- /dev/null +++ b/hive-forge/src/verbs/issue_cmd.rs @@ -0,0 +1,81 @@ +//! `issue ` — issue-scoped sub-commands. Wraps the per-verb modules +//! under an `issue` parent so `hive-forge issue close 42`, `issue create …`, +//! etc. read as kind-namespaced commands. The generic verbs that also work on +//! PRs (view/comment/comments/close/labels/assign/timeline) kind-check the +//! number is an issue first (`assert_kind`); the issue-only verbs are +//! kind-correct by construction. The flat `issue-*` + bare generic verbs stay +//! as hidden back-compat aliases (see `main.rs`). + +use anyhow::Result; +use clap::{Args as ClapArgs, Subcommand}; + +use crate::client::Client; +use crate::verbs::{self, Kind, assert_kind}; + +#[derive(ClapArgs)] +pub struct Args { + #[command(subcommand)] + cmd: Cmd, +} + +#[derive(Subcommand)] +enum Cmd { + /// Show issue metadata as JSON. + Show(verbs::issue::Args), + /// Create an issue. + Create(verbs::issue_create::Args), + /// Edit an issue's title / body / state / milestone. + Edit(verbs::issue_edit::Args), + /// Show title + body + comments. + View(verbs::view::Args), + /// Post a comment on the issue. + Comment(verbs::comment::Args), + /// List comments on the issue. + Comments(verbs::comments::Args), + /// Close the issue. + Close(verbs::close::Args), + /// List / add / remove labels. + Labels(verbs::labels::Args), + /// Assign or unassign a user. + Assign(verbs::assign::Args), + /// List timeline events. + Timeline(verbs::timeline::Args), +} + +pub fn run(client: &Client, args: Args) -> Result<()> { + match args.cmd { + // Issue-only verbs. + Cmd::Show(a) => verbs::issue::run(client, a), + Cmd::Create(a) => verbs::issue_create::run(client, a), + Cmd::Edit(a) => verbs::issue_edit::run(client, a), + // Generics shared with `pr` — verify the number is an issue first. + Cmd::View(a) => { + assert_kind(client, a.number, Kind::Issue)?; + verbs::view::run(client, a) + } + Cmd::Comment(a) => { + assert_kind(client, a.number, Kind::Issue)?; + verbs::comment::run(client, a) + } + Cmd::Comments(a) => { + assert_kind(client, a.number, Kind::Issue)?; + verbs::comments::run(client, a) + } + Cmd::Close(a) => { + assert_kind(client, a.number, Kind::Issue)?; + verbs::close::run(client, a) + } + Cmd::Labels(a) => { + assert_kind(client, a.number, Kind::Issue)?; + verbs::labels::run(client, a) + } + Cmd::Assign(a) => { + assert_kind(client, a.number, Kind::Issue)?; + verbs::assign::run(client, a) + } + Cmd::Timeline(a) => { + assert_kind(client, a.number, Kind::Issue)?; + verbs::timeline::run(client, a) + } + } +} diff --git a/hive-forge/src/verbs/labels.rs b/hive-forge/src/verbs/labels.rs index 20db93b1..a3118303 100644 --- a/hive-forge/src/verbs/labels.rs +++ b/hive-forge/src/verbs/labels.rs @@ -11,7 +11,7 @@ use crate::verbs::print_json; #[derive(ClapArgs)] pub struct Args { /// Issue or PR number. - number: u64, + pub(crate) number: u64, #[command(subcommand)] action: Option, } diff --git a/hive-forge/src/verbs/mod.rs b/hive-forge/src/verbs/mod.rs index e56f99ff..e1b28c33 100644 --- a/hive-forge/src/verbs/mod.rs +++ b/hive-forge/src/verbs/mod.rs @@ -17,6 +17,7 @@ pub mod comment_show; pub mod comments; pub mod diff; pub mod issue; +pub mod issue_cmd; pub mod issue_create; pub mod issue_edit; pub mod labels; @@ -24,6 +25,7 @@ pub mod lint; pub mod list; pub mod milestone; pub mod pr; +pub mod pr_cmd; pub mod pr_commits; pub mod pr_create; pub mod pr_merge; @@ -52,6 +54,37 @@ pub(crate) fn print_json(v: &Value) -> Result<()> { Ok(()) } +/// Issue-vs-PR kind, for the `pr ` / `issue ` sub-command +/// validation. +#[derive(Clone, Copy)] +pub(crate) enum Kind { + Pr, + Issue, +} + +/// Verify `number` is the expected kind before a kind-namespaced verb (one +/// of the generics that work on both — close/comment/labels/…) acts on it — +/// the validation win the `pr ` / `issue ` split buys over the +/// old generic verbs. Forgejo's `/issues/{n}` endpoint serves both issues and +/// PRs and marks PRs with a non-null `pull_request` field, so one GET +/// classifies it. Errors with a "use the other command" message on mismatch. +pub(crate) fn assert_kind(client: &Client, number: u64, expected: Kind) -> Result<()> { + let repo = client.repo(); + let v = client.get_json(&format!("/repos/{repo}/issues/{number}"))?; + let is_pr = v.get("pull_request").is_some_and(|p| !p.is_null()); + match (expected, is_pr) { + (Kind::Pr, false) => { + anyhow::bail!( + "#{number} is an issue, not a PR — use `hive-forge issue {number}`" + ) + } + (Kind::Issue, true) => { + anyhow::bail!("#{number} is a PR, not an issue — use `hive-forge pr {number}`") + } + _ => Ok(()), + } +} + /// Minimal RFC 3986 unreserved-set percent encoder. Covers the subset of /// characters that show up in the values we splice into request paths — /// usernames, label names, artifact names — without pulling in a fresh diff --git a/hive-forge/src/verbs/pr_cmd.rs b/hive-forge/src/verbs/pr_cmd.rs new file mode 100644 index 00000000..ea380166 --- /dev/null +++ b/hive-forge/src/verbs/pr_cmd.rs @@ -0,0 +1,93 @@ +//! `pr ` — PR-scoped sub-commands. Wraps the per-verb modules under a +//! `pr` parent so `hive-forge pr close 42`, `pr status --pr 42`, etc. read as +//! kind-namespaced commands. The generic verbs that also work on issues +//! (view/comment/comments/close/labels/assign/timeline) kind-check the number +//! is a PR first (`assert_kind`); the PR-only verbs hit `/pulls/…` and are +//! kind-correct by construction. The flat `pr-*` + bare generic verbs stay as +//! hidden back-compat aliases (see `main.rs`). + +use anyhow::Result; +use clap::{Args as ClapArgs, Subcommand}; + +use crate::client::Client; +use crate::verbs::{self, Kind, assert_kind}; + +#[derive(ClapArgs)] +pub struct Args { + #[command(subcommand)] + cmd: Cmd, +} + +#[derive(Subcommand)] +enum Cmd { + /// Show PR metadata as JSON. + Show(verbs::pr::Args), + /// List the PR's commits as JSON. + Commits(verbs::pr_commits::Args), + /// Create a pull request. + Create(verbs::pr_create::Args), + /// PR health view: mergeable / CI / reviews. + Status(verbs::pr_status::Args), + /// Merge the PR. + Merge(verbs::pr_merge::Args), + /// List a PR's reviews, or submit one. + Reviews(verbs::pr_reviews::Args), + /// Print the PR's unified diff. + Diff(verbs::diff::Args), + /// Show title + body + comments. + View(verbs::view::Args), + /// Post a comment on the PR. + Comment(verbs::comment::Args), + /// List comments on the PR. + Comments(verbs::comments::Args), + /// Close the PR. + Close(verbs::close::Args), + /// List / add / remove labels. + Labels(verbs::labels::Args), + /// Assign or unassign a user. + Assign(verbs::assign::Args), + /// List timeline events. + Timeline(verbs::timeline::Args), +} + +pub fn run(client: &Client, args: Args) -> Result<()> { + match args.cmd { + // PR-only verbs — kind-correct by construction (hit `/pulls/…`). + Cmd::Show(a) => verbs::pr::run(client, a), + Cmd::Commits(a) => verbs::pr_commits::run(client, a), + Cmd::Create(a) => verbs::pr_create::run(client, a), + Cmd::Status(a) => verbs::pr_status::run(client, a), + Cmd::Merge(a) => verbs::pr_merge::run(client, a), + Cmd::Reviews(a) => verbs::pr_reviews::run(client, a), + Cmd::Diff(a) => verbs::diff::run(client, a), + // Generics shared with `issue` — verify the number is a PR first. + Cmd::View(a) => { + assert_kind(client, a.number, Kind::Pr)?; + verbs::view::run(client, a) + } + Cmd::Comment(a) => { + assert_kind(client, a.number, Kind::Pr)?; + verbs::comment::run(client, a) + } + Cmd::Comments(a) => { + assert_kind(client, a.number, Kind::Pr)?; + verbs::comments::run(client, a) + } + Cmd::Close(a) => { + assert_kind(client, a.number, Kind::Pr)?; + verbs::close::run(client, a) + } + Cmd::Labels(a) => { + assert_kind(client, a.number, Kind::Pr)?; + verbs::labels::run(client, a) + } + Cmd::Assign(a) => { + assert_kind(client, a.number, Kind::Pr)?; + verbs::assign::run(client, a) + } + Cmd::Timeline(a) => { + assert_kind(client, a.number, Kind::Pr)?; + verbs::timeline::run(client, a) + } + } +} diff --git a/hive-forge/src/verbs/timeline.rs b/hive-forge/src/verbs/timeline.rs index bd662e72..4b99a159 100644 --- a/hive-forge/src/verbs/timeline.rs +++ b/hive-forge/src/verbs/timeline.rs @@ -26,7 +26,7 @@ use crate::verbs::print_json; #[derive(ClapArgs)] pub struct Args { /// Issue or PR number. - number: u64, + pub(crate) number: u64, /// Page size (Forgejo caps at 50). Returns the first `N` events. #[arg(long, default_value_t = 50)] limit: u64, diff --git a/hive-forge/src/verbs/view.rs b/hive-forge/src/verbs/view.rs index 01494cdc..b0ab2474 100644 --- a/hive-forge/src/verbs/view.rs +++ b/hive-forge/src/verbs/view.rs @@ -10,7 +10,7 @@ use crate::client::Client; #[derive(ClapArgs)] pub struct Args { /// Issue or PR number. - number: u64, + pub(crate) number: u64, } pub fn run(client: &Client, args: Args) -> Result<()> { From 5dc1b3933aa5bb5545bc1762443ba07ecc338fd9 Mon Sep 17 00:00:00 2001 From: damocles Date: Mon, 22 Jun 2026 13:57:27 +0200 Subject: [PATCH 2/9] return accurate http status codes for dashboard client errors --- hive-c0re/src/dashboard/approvals.rs | 9 +++++++-- hive-c0re/src/dashboard/journal.rs | 17 +++++++++++++---- hive-c0re/src/dashboard/permissions.rs | 22 +++++++++++++++++----- hive-c0re/src/dashboard/questions.rs | 7 +++++-- hive-c0re/src/dashboard/reminders.rs | 12 +++++++++--- hive-c0re/src/dashboard/schedules.rs | 14 ++++++++++---- hive-c0re/src/dashboard/topology.rs | 4 ++-- 7 files changed, 63 insertions(+), 22 deletions(-) diff --git a/hive-c0re/src/dashboard/approvals.rs b/hive-c0re/src/dashboard/approvals.rs index c523f728..8604483c 100644 --- a/hive-c0re/src/dashboard/approvals.rs +++ b/hive-c0re/src/dashboard/approvals.rs @@ -16,7 +16,7 @@ use axum::{ use hive_sh4re::Approval; use serde::Deserialize; -use super::{AppState, error_response}; +use super::{AppState, error_response, problem_response}; use crate::actions; use crate::coordinator::Coordinator; use crate::lifecycle; @@ -209,7 +209,12 @@ pub(super) async fn get_approval_diff( .max() .map(|n| format!("refs/tags/proposal/{n}")) } - other => return error_response(&format!("unknown diff base {other:?}")), + other => { + return problem_response( + StatusCode::BAD_REQUEST, + &format!("unknown diff base {other:?}"), + ); + } }; let Some(base_ref) = base_ref else { return plain_text(match base { diff --git a/hive-c0re/src/dashboard/journal.rs b/hive-c0re/src/dashboard/journal.rs index cdcba9fb..5a661d51 100644 --- a/hive-c0re/src/dashboard/journal.rs +++ b/hive-c0re/src/dashboard/journal.rs @@ -13,7 +13,7 @@ use axum::{ }; use serde::Deserialize; -use super::{error_response, strip_container_prefix, validate_agent_name}; +use super::{error_response, problem_response, strip_container_prefix, validate_agent_name}; use crate::lifecycle; #[derive(Deserialize)] @@ -48,7 +48,10 @@ 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 error_response(&format!("journal: no managed container {prefixed:?}")); + return problem_response( + StatusCode::NOT_FOUND, + &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()) { @@ -61,7 +64,10 @@ pub(super) async fn get_journal( format!("{u}.service") }; if !allowed.contains(&unit.as_str()) { - return error_response(&format!("journal: unknown unit {unit:?}")); + return problem_response( + StatusCode::BAD_REQUEST, + &format!("journal: unknown unit {unit:?}"), + ); } Some(unit) } @@ -121,7 +127,10 @@ pub(super) async fn get_journal_host( format!("{u}.service") }; if !allowed.contains(&unit.as_str()) { - return error_response(&format!("journal-host: unknown unit {unit:?}")); + return problem_response( + StatusCode::BAD_REQUEST, + &format!("journal-host: unknown unit {unit:?}"), + ); } cmd.args(["-u", &unit]); } diff --git a/hive-c0re/src/dashboard/permissions.rs b/hive-c0re/src/dashboard/permissions.rs index 60044b4b..ff379188 100644 --- a/hive-c0re/src/dashboard/permissions.rs +++ b/hive-c0re/src/dashboard/permissions.rs @@ -12,7 +12,7 @@ use axum::{ }; use serde::{Deserialize, Serialize}; -use super::{AppState, error_response, guard_agent_name, strip_container_prefix}; +use super::{AppState, guard_agent_name, problem_response, strip_container_prefix}; #[derive(Serialize)] pub(super) struct ToolGroupsSnapshot { @@ -120,7 +120,10 @@ pub(super) async fn post_tool_groups( // 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 error_response(&format!("invalid tool-groups for {logical}: {e}")); + return problem_response( + StatusCode::BAD_REQUEST, + &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 @@ -204,7 +207,10 @@ pub(super) async fn post_capabilities( .collect(); for cap in &body.caps { if !known.contains(&cap.as_str()) { - return error_response(&format!("unknown capability: {cap}")); + return problem_response( + StatusCode::BAD_REQUEST, + &format!("unknown capability: {cap}"), + ); } } // Enqueue a PermChange so the JSON file write is serialised through @@ -272,12 +278,18 @@ pub(super) async fn post_permissions( if let Some(groups) = &change.tool_groups && let Err(e) = crate::tool_groups::validate_groups(groups) { - return error_response(&format!("invalid tool-groups for {logical}: {e}")); + return problem_response( + StatusCode::BAD_REQUEST, + &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 error_response(&format!("unknown capability for {logical}: {cap}")); + return problem_response( + StatusCode::BAD_REQUEST, + &format!("unknown capability for {logical}: {cap}"), + ); } } } diff --git a/hive-c0re/src/dashboard/questions.rs b/hive-c0re/src/dashboard/questions.rs index 5d6530e0..ef50aff8 100644 --- a/hive-c0re/src/dashboard/questions.rs +++ b/hive-c0re/src/dashboard/questions.rs @@ -13,7 +13,7 @@ use axum::{ }; use serde::Deserialize; -use super::{AppState, error_response}; +use super::{AppState, error_response, problem_response}; #[derive(Deserialize)] pub(super) struct AnswerForm { @@ -41,7 +41,10 @@ pub(super) async fn post_answer_question( ) -> Response { let answer = form.answer.trim(); if answer.is_empty() { - return with_cors(error_response("answer: required")); + return with_cors(problem_response( + StatusCode::BAD_REQUEST, + "answer: required", + )); } let resp = match state .coord diff --git a/hive-c0re/src/dashboard/reminders.rs b/hive-c0re/src/dashboard/reminders.rs index e29e885c..75789b0f 100644 --- a/hive-c0re/src/dashboard/reminders.rs +++ b/hive-c0re/src/dashboard/reminders.rs @@ -10,7 +10,7 @@ use axum::{ response::{IntoResponse, Response}, }; -use super::{AppState, error_response}; +use super::{AppState, error_response, problem_response}; pub(super) async fn api_reminders(State(state): State) -> Response { match state.coord.broker.list_pending_reminders() { @@ -24,7 +24,10 @@ pub(super) async fn post_cancel_reminder( AxumPath(id): AxumPath, ) -> Response { match state.coord.broker.cancel_reminder(id) { - Ok(0) => error_response(&format!("reminder {id} not pending (already delivered?)")), + Ok(0) => problem_response( + StatusCode::NOT_FOUND, + &format!("reminder {id} not pending (already delivered?)"), + ), Ok(_) => { tracing::info!(%id, "operator cancelled reminder"); state.coord.emit_reminders_snapshot(); @@ -44,7 +47,10 @@ pub(super) async fn post_retry_reminder( AxumPath(id): AxumPath, ) -> Response { match state.coord.broker.reset_reminder_failure(id) { - Ok(0) => error_response(&format!("reminder {id} not pending (already delivered?)")), + Ok(0) => problem_response( + StatusCode::NOT_FOUND, + &format!("reminder {id} not pending (already delivered?)"), + ), Ok(_) => { tracing::info!(%id, "operator reset reminder failure for retry"); state.coord.emit_reminders_snapshot(); diff --git a/hive-c0re/src/dashboard/schedules.rs b/hive-c0re/src/dashboard/schedules.rs index 535ae9c0..ccabe878 100644 --- a/hive-c0re/src/dashboard/schedules.rs +++ b/hive-c0re/src/dashboard/schedules.rs @@ -11,7 +11,7 @@ use axum::{ response::{IntoResponse, Response}, }; -use super::{AppState, error_response}; +use super::{AppState, error_response, problem_response}; /// `GET /api/schedules` — snapshot of every schedule for the /// scheduled-prompts tab. Returns the wire shape directly @@ -51,13 +51,19 @@ pub(super) async fn post_schedule_new( axum::Json(payload): axum::Json, ) -> Response { if payload.targets.is_empty() { - return error_response("schedule must have at least one target"); + return problem_response( + StatusCode::BAD_REQUEST, + "schedule must have at least one target", + ); } if payload.body.trim().is_empty() { - return error_response("schedule body must be non-empty"); + return problem_response(StatusCode::BAD_REQUEST, "schedule body must be non-empty"); } if let Some(0) = payload.interval_seconds { - return error_response("interval_seconds must be > 0 (use None for one-shot)"); + return problem_response( + StatusCode::BAD_REQUEST, + "interval_seconds must be > 0 (use None for one-shot)", + ); } let new = crate::scheduled_prompts::NewSchedule { owner: hive_sh4re::OPERATOR_RECIPIENT.to_owned(), diff --git a/hive-c0re/src/dashboard/topology.rs b/hive-c0re/src/dashboard/topology.rs index 7482a846..5751d9a0 100644 --- a/hive-c0re/src/dashboard/topology.rs +++ b/hive-c0re/src/dashboard/topology.rs @@ -13,7 +13,7 @@ use axum::{ }; use serde::Deserialize; -use super::{AppState, error_response}; +use super::{AppState, error_response, problem_response}; /// `POST /api/topology/set-parent` body. `child` is required. /// `new_parent` may be: @@ -53,7 +53,7 @@ pub(super) async fn post_set_parent( ) -> Response { let child = form.child.trim().to_owned(); if child.is_empty() { - return error_response("set-parent: `child` required"); + return problem_response(StatusCode::BAD_REQUEST, "set-parent: `child` required"); } // Empty / whitespace-only `new_parent` ⇒ promote to root. Web // forms submit the empty string for a "no value" radio button, From cdf1bfe7db6e5f2b86be591f4e4a75ecd46217d9 Mon Sep 17 00:00:00 2001 From: damocles Date: Mon, 22 Jun 2026 14:11:05 +0200 Subject: [PATCH 3/9] use the problem_details crate for problem+json responses --- Cargo.lock | 24 ++++++++++++++ hive-c0re/Cargo.toml | 1 + hive-c0re/src/dashboard.rs | 66 ++++++++++++++------------------------ 3 files changed, 49 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d483ac59..228e132f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1367,6 +1367,7 @@ dependencies = [ "hive-sh4re", "libc", "listenfd", + "problem_details", "reqwest", "rusqlite", "serde", @@ -1508,6 +1509,16 @@ version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9171a2ea8a68358193d15dd5d70c1c10a2afc3e7e4c5bc92bc9f025cebd7359c" +[[package]] +name = "http-serde" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0f056c8559e3757392c8d091e796416e4649d8e49e88b8d76df6c002f05027fd" +dependencies = [ + "http", + "serde", +] + [[package]] name = "httparse" version = "1.10.1" @@ -2627,6 +2638,19 @@ dependencies = [ "syn", ] +[[package]] +name = "problem_details" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d50e8b46a2f32e61ae82888734e24627ea0f8c9bc7c5fc8d0c3e0eb7ed0ff5ab" +dependencies = [ + "axum", + "http", + "http-serde", + "serde", + "serde_json", +] + [[package]] name = "proc-macro-crate" version = "3.5.0" diff --git a/hive-c0re/Cargo.toml b/hive-c0re/Cargo.toml index f57c3a4d..f97ef7b0 100644 --- a/hive-c0re/Cargo.toml +++ b/hive-c0re/Cargo.toml @@ -24,6 +24,7 @@ tokio.workspace = true tokio-stream.workspace = true tracing.workspace = true tracing-subscriber.workspace = true +problem_details = { version = "0.9.0", features = ["axum"] } [dev-dependencies] tempfile = "3" diff --git a/hive-c0re/src/dashboard.rs b/hive-c0re/src/dashboard.rs index 4b4d7507..944ab5de 100644 --- a/hive-c0re/src/dashboard.rs +++ b/hive-c0re/src/dashboard.rs @@ -1133,19 +1133,21 @@ mod tests { use super::*; #[test] - fn problem_body_has_rfc9457_members() { - // about:blank type → title is the canonical status reason phrase, - // status is the numeric code, detail is the caller message. - let body = problem_body(StatusCode::BAD_REQUEST, "bad input"); - assert_eq!(body["type"], "about:blank"); - assert_eq!(body["title"], "Bad Request"); - assert_eq!(body["status"], 400); - assert_eq!(body["detail"], "bad input"); - // error_response (the 500 wrapper) carries the same shape with the - // internal-error status. - let five = problem_body(StatusCode::INTERNAL_SERVER_ERROR, "boom"); - assert_eq!(five["status"], 500); - assert_eq!(five["title"], "Internal Server Error"); + fn problem_details_carry_rfc9457_status_and_detail() { + // Contract the frontend depends on: the problem_details crate + // serialises the RFC 9457 members we rely on — `status` (numeric) + // and `detail` (the caller message; the FE reads `.detail`). + let pd = problem_details::ProblemDetails::from_status_code(StatusCode::BAD_REQUEST) + .with_detail("bad input"); + let v = serde_json::to_value(&pd).expect("problem details serialise"); + assert_eq!(v["status"], 400); + assert_eq!(v["detail"], "bad input"); + // The 500 wrapper path carries the internal-error status. + let five = + problem_details::ProblemDetails::from_status_code(StatusCode::INTERNAL_SERVER_ERROR) + .with_detail("boom"); + let fv = serde_json::to_value(&five).expect("problem details serialise"); + assert_eq!(fv["status"], 500); } #[test] @@ -1655,36 +1657,16 @@ fn strip_container_prefix(name: &str) -> String { .to_owned() } -/// The RFC 9457 problem-details media type. -const PROBLEM_JSON_CONTENT_TYPE: &str = "application/problem+json"; - -/// Build the RFC 9457 problem-details body for `status` + `detail`. The -/// object carries the standard members: `type` ("about:blank", i.e. no -/// problem-specific type), `title` (the HTTP status reason phrase), -/// `status` (numeric code) and `detail` (the caller-supplied message). -/// Split from [`problem_response`] so the member shape is unit-testable -/// without axum response plumbing. -fn problem_body(status: StatusCode, detail: &str) -> serde_json::Value { - serde_json::json!({ - "type": "about:blank", - "title": status.canonical_reason().unwrap_or("Error"), - "status": status.as_u16(), - "detail": detail, - }) -} - -/// Build an RFC 9457 (`application/problem+json`) error response. -/// Centralising this keeps every dashboard error on one machine-readable -/// shape the frontend can parse (read `detail` for display) instead of -/// guessing between plain text and JSON. +/// Build an RFC 9457 (`application/problem+json`) error 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 body with the `application/problem+json` +/// content type. Centralising this keeps every dashboard error on one +/// machine-readable shape the frontend parses (it reads `detail`). fn problem_response(status: StatusCode, detail: &str) -> Response { - let body = serde_json::to_string(&problem_body(status, detail)) - .expect("problem+json body is always serialisable"); - ( - status, - [(axum::http::header::CONTENT_TYPE, PROBLEM_JSON_CONTENT_TYPE)], - body, - ) + problem_details::ProblemDetails::from_status_code(status) + .with_detail(detail) .into_response() } From 2b0c51badf92c72cccbb2ffa5954bd5c1a4c8cc5 Mon Sep 17 00:00:00 2001 From: damocles Date: Mon, 22 Jun 2026 14:31:30 +0200 Subject: [PATCH 4/9] inline problem_details builder at call sites; drop the one-caller wrapper --- hive-c0re/src/dashboard.rs | 28 +++++++++------------- hive-c0re/src/dashboard/approvals.rs | 11 +++++---- hive-c0re/src/dashboard/journal.rs | 25 ++++++++++---------- hive-c0re/src/dashboard/permissions.rs | 32 ++++++++++++-------------- hive-c0re/src/dashboard/questions.rs | 13 +++++++---- hive-c0re/src/dashboard/reminders.rs | 18 +++++++-------- hive-c0re/src/dashboard/schedules.rs | 22 ++++++++++-------- hive-c0re/src/dashboard/topology.rs | 8 +++++-- 8 files changed, 79 insertions(+), 78 deletions(-) diff --git a/hive-c0re/src/dashboard.rs b/hive-c0re/src/dashboard.rs index 944ab5de..b570254a 100644 --- a/hive-c0re/src/dashboard.rs +++ b/hive-c0re/src/dashboard.rs @@ -1657,24 +1657,18 @@ fn strip_container_prefix(name: &str) -> String { .to_owned() } -/// Build an RFC 9457 (`application/problem+json`) error response via the +/// 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 body with the `application/problem+json` -/// content type. Centralising this keeps every dashboard error on one -/// machine-readable shape the frontend parses (it reads `detail`). -fn problem_response(status: StatusCode, detail: &str) -> Response { - problem_details::ProblemDetails::from_status_code(status) - .with_detail(detail) +/// `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 { + problem_details::ProblemDetails::from_status_code(StatusCode::INTERNAL_SERVER_ERROR) + .with_detail(message) .into_response() } - -/// Convenience wrapper for the common internal-error case: a 500 -/// problem-details response (see [`problem_response`]). Most dashboard -/// handlers funnel their errors through here; handlers with a more -/// specific failure (bad input, not found) call [`problem_response`] -/// directly with the right status. -fn error_response(message: &str) -> Response { - problem_response(StatusCode::INTERNAL_SERVER_ERROR, message) -} diff --git a/hive-c0re/src/dashboard/approvals.rs b/hive-c0re/src/dashboard/approvals.rs index 8604483c..b85bfb40 100644 --- a/hive-c0re/src/dashboard/approvals.rs +++ b/hive-c0re/src/dashboard/approvals.rs @@ -16,7 +16,9 @@ use axum::{ use hive_sh4re::Approval; use serde::Deserialize; -use super::{AppState, error_response, problem_response}; +use problem_details::ProblemDetails; + +use super::{AppState, error_response}; use crate::actions; use crate::coordinator::Coordinator; use crate::lifecycle; @@ -210,10 +212,9 @@ pub(super) async fn get_approval_diff( .map(|n| format!("refs/tags/proposal/{n}")) } other => { - return problem_response( - StatusCode::BAD_REQUEST, - &format!("unknown diff base {other:?}"), - ); + return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST) + .with_detail(format!("unknown diff base {other:?}")) + .into_response(); } }; let Some(base_ref) = base_ref else { diff --git a/hive-c0re/src/dashboard/journal.rs b/hive-c0re/src/dashboard/journal.rs index 5a661d51..a0debbc3 100644 --- a/hive-c0re/src/dashboard/journal.rs +++ b/hive-c0re/src/dashboard/journal.rs @@ -13,7 +13,9 @@ use axum::{ }; use serde::Deserialize; -use super::{error_response, problem_response, strip_container_prefix, validate_agent_name}; +use problem_details::ProblemDetails; + +use super::{error_response, strip_container_prefix, validate_agent_name}; use crate::lifecycle; #[derive(Deserialize)] @@ -48,10 +50,9 @@ 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 problem_response( - StatusCode::NOT_FOUND, - &format!("journal: no managed container {prefixed:?}"), - ); + return ProblemDetails::from_status_code(StatusCode::NOT_FOUND) + .with_detail(format!("journal: no managed container {prefixed:?}")) + .into_response(); } let lines = q.lines.unwrap_or(500).min(5000); let unit = match q.unit.as_deref().filter(|s| !s.is_empty()) { @@ -64,10 +65,9 @@ pub(super) async fn get_journal( format!("{u}.service") }; if !allowed.contains(&unit.as_str()) { - return problem_response( - StatusCode::BAD_REQUEST, - &format!("journal: unknown unit {unit:?}"), - ); + return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST) + .with_detail(format!("journal: unknown unit {unit:?}")) + .into_response(); } Some(unit) } @@ -127,10 +127,9 @@ pub(super) async fn get_journal_host( format!("{u}.service") }; if !allowed.contains(&unit.as_str()) { - return problem_response( - StatusCode::BAD_REQUEST, - &format!("journal-host: unknown unit {unit:?}"), - ); + return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST) + .with_detail(format!("journal-host: unknown unit {unit:?}")) + .into_response(); } cmd.args(["-u", &unit]); } diff --git a/hive-c0re/src/dashboard/permissions.rs b/hive-c0re/src/dashboard/permissions.rs index ff379188..bb32f6f8 100644 --- a/hive-c0re/src/dashboard/permissions.rs +++ b/hive-c0re/src/dashboard/permissions.rs @@ -12,7 +12,9 @@ use axum::{ }; use serde::{Deserialize, Serialize}; -use super::{AppState, guard_agent_name, problem_response, strip_container_prefix}; +use problem_details::ProblemDetails; + +use super::{AppState, guard_agent_name, strip_container_prefix}; #[derive(Serialize)] pub(super) struct ToolGroupsSnapshot { @@ -120,10 +122,9 @@ pub(super) async fn post_tool_groups( // 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 problem_response( - StatusCode::BAD_REQUEST, - &format!("invalid tool-groups for {logical}: {e}"), - ); + return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST) + .with_detail(format!("invalid tool-groups for {logical}: {e}")) + .into_response(); } // Enqueue a PermChange so the JSON file write is serialised through // the FIFO worker. Prevents concurrent batch-apply actions for @@ -207,10 +208,9 @@ pub(super) async fn post_capabilities( .collect(); for cap in &body.caps { if !known.contains(&cap.as_str()) { - return problem_response( - StatusCode::BAD_REQUEST, - &format!("unknown capability: {cap}"), - ); + return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST) + .with_detail(format!("unknown capability: {cap}")) + .into_response(); } } // Enqueue a PermChange so the JSON file write is serialised through @@ -278,18 +278,16 @@ pub(super) async fn post_permissions( if let Some(groups) = &change.tool_groups && let Err(e) = crate::tool_groups::validate_groups(groups) { - return problem_response( - StatusCode::BAD_REQUEST, - &format!("invalid tool-groups for {logical}: {e}"), - ); + return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST) + .with_detail(format!("invalid tool-groups for {logical}: {e}")) + .into_response(); } if let Some(caps) = &change.capabilities { for cap in caps { if !known_caps.contains(&cap.as_str()) { - return problem_response( - StatusCode::BAD_REQUEST, - &format!("unknown capability for {logical}: {cap}"), - ); + return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST) + .with_detail(format!("unknown capability for {logical}: {cap}")) + .into_response(); } } } diff --git a/hive-c0re/src/dashboard/questions.rs b/hive-c0re/src/dashboard/questions.rs index ef50aff8..b94cdd7e 100644 --- a/hive-c0re/src/dashboard/questions.rs +++ b/hive-c0re/src/dashboard/questions.rs @@ -13,7 +13,9 @@ use axum::{ }; use serde::Deserialize; -use super::{AppState, error_response, problem_response}; +use problem_details::ProblemDetails; + +use super::{AppState, error_response}; #[derive(Deserialize)] pub(super) struct AnswerForm { @@ -41,10 +43,11 @@ pub(super) async fn post_answer_question( ) -> Response { let answer = form.answer.trim(); if answer.is_empty() { - return with_cors(problem_response( - StatusCode::BAD_REQUEST, - "answer: required", - )); + return with_cors( + ProblemDetails::from_status_code(StatusCode::BAD_REQUEST) + .with_detail("answer: required") + .into_response(), + ); } let resp = match state .coord diff --git a/hive-c0re/src/dashboard/reminders.rs b/hive-c0re/src/dashboard/reminders.rs index 75789b0f..9f2abb45 100644 --- a/hive-c0re/src/dashboard/reminders.rs +++ b/hive-c0re/src/dashboard/reminders.rs @@ -10,7 +10,9 @@ use axum::{ response::{IntoResponse, Response}, }; -use super::{AppState, error_response, problem_response}; +use problem_details::ProblemDetails; + +use super::{AppState, error_response}; pub(super) async fn api_reminders(State(state): State) -> Response { match state.coord.broker.list_pending_reminders() { @@ -24,10 +26,9 @@ pub(super) async fn post_cancel_reminder( AxumPath(id): AxumPath, ) -> Response { match state.coord.broker.cancel_reminder(id) { - Ok(0) => problem_response( - StatusCode::NOT_FOUND, - &format!("reminder {id} not pending (already delivered?)"), - ), + Ok(0) => ProblemDetails::from_status_code(StatusCode::NOT_FOUND) + .with_detail(format!("reminder {id} not pending (already delivered?)")) + .into_response(), Ok(_) => { tracing::info!(%id, "operator cancelled reminder"); state.coord.emit_reminders_snapshot(); @@ -47,10 +48,9 @@ pub(super) async fn post_retry_reminder( AxumPath(id): AxumPath, ) -> Response { match state.coord.broker.reset_reminder_failure(id) { - Ok(0) => problem_response( - StatusCode::NOT_FOUND, - &format!("reminder {id} not pending (already delivered?)"), - ), + Ok(0) => ProblemDetails::from_status_code(StatusCode::NOT_FOUND) + .with_detail(format!("reminder {id} not pending (already delivered?)")) + .into_response(), Ok(_) => { tracing::info!(%id, "operator reset reminder failure for retry"); state.coord.emit_reminders_snapshot(); diff --git a/hive-c0re/src/dashboard/schedules.rs b/hive-c0re/src/dashboard/schedules.rs index ccabe878..e5bf1ece 100644 --- a/hive-c0re/src/dashboard/schedules.rs +++ b/hive-c0re/src/dashboard/schedules.rs @@ -11,7 +11,9 @@ use axum::{ response::{IntoResponse, Response}, }; -use super::{AppState, error_response, problem_response}; +use problem_details::ProblemDetails; + +use super::{AppState, error_response}; /// `GET /api/schedules` — snapshot of every schedule for the /// scheduled-prompts tab. Returns the wire shape directly @@ -51,19 +53,19 @@ pub(super) async fn post_schedule_new( axum::Json(payload): axum::Json, ) -> Response { if payload.targets.is_empty() { - return problem_response( - StatusCode::BAD_REQUEST, - "schedule must have at least one target", - ); + return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST) + .with_detail("schedule must have at least one target") + .into_response(); } if payload.body.trim().is_empty() { - return problem_response(StatusCode::BAD_REQUEST, "schedule body must be non-empty"); + return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST) + .with_detail("schedule body must be non-empty") + .into_response(); } if let Some(0) = payload.interval_seconds { - return problem_response( - StatusCode::BAD_REQUEST, - "interval_seconds must be > 0 (use None for one-shot)", - ); + return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST) + .with_detail("interval_seconds must be > 0 (use None for one-shot)") + .into_response(); } let new = crate::scheduled_prompts::NewSchedule { owner: hive_sh4re::OPERATOR_RECIPIENT.to_owned(), diff --git a/hive-c0re/src/dashboard/topology.rs b/hive-c0re/src/dashboard/topology.rs index 5751d9a0..c768830a 100644 --- a/hive-c0re/src/dashboard/topology.rs +++ b/hive-c0re/src/dashboard/topology.rs @@ -13,7 +13,9 @@ use axum::{ }; use serde::Deserialize; -use super::{AppState, error_response, problem_response}; +use problem_details::ProblemDetails; + +use super::{AppState, error_response}; /// `POST /api/topology/set-parent` body. `child` is required. /// `new_parent` may be: @@ -53,7 +55,9 @@ pub(super) async fn post_set_parent( ) -> Response { let child = form.child.trim().to_owned(); if child.is_empty() { - return problem_response(StatusCode::BAD_REQUEST, "set-parent: `child` required"); + return ProblemDetails::from_status_code(StatusCode::BAD_REQUEST) + .with_detail("set-parent: `child` required") + .into_response(); } // Empty / whitespace-only `new_parent` ⇒ promote to root. Web // forms submit the empty string for a "no value" radio button, From 65ad994c8510bacab9bc586ddc34274b5fa777c7 Mon Sep 17 00:00:00 2001 From: damocles Date: Mon, 22 Jun 2026 15:07:57 +0200 Subject: [PATCH 5/9] dashboard: return problem details directly from client-error handlers --- hive-c0re/src/dashboard.rs | 29 ++++++++++-------- hive-c0re/src/dashboard/approvals.rs | 32 +++++++++++--------- hive-c0re/src/dashboard/journal.rs | 32 ++++++++++---------- hive-c0re/src/dashboard/permissions.rs | 41 +++++++++++++------------- hive-c0re/src/dashboard/questions.rs | 6 ++-- hive-c0re/src/dashboard/reminders.rs | 26 ++++++++-------- hive-c0re/src/dashboard/schedules.rs | 23 +++++++-------- hive-c0re/src/dashboard/topology.rs | 13 ++++---- 8 files changed, 102 insertions(+), 100 deletions(-) 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}"))), } } From 4342a508954b927015cdff0223c236d8826335ab Mon Sep 17 00:00:00 2001 From: iris Date: Mon, 22 Jun 2026 15:56:50 +0200 Subject: [PATCH 6/9] docs(web-ui): refresh matrix-accounts page for heartbeat + age-dimming + problem+json MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The M4TR1X ACC0UNTS section described the pre-heartbeat state: a snapshot 'written at startup / rewritten each daemon (re)start' with an 'ambiguous' as_of, and the live-status dot as a 'dashboard-side follow-up'. All three shipped since: - the daemon now force-rewrites the snapshot every ~30s (heartbeat), so as_of advances while alive and a stalled value is an honest dead-daemon signal — documented, with the full dot state table (green / dim-green 'no heartbeat' age case / amber container-down + offline / grey); - the login endpoint moved to /api/matrix-account-login and its failure body is RFC 9457 application/problem+json (message in detail), not the old 4xx { error } — corrected, with the 400/500 status scheme. --- docs/web-ui/dashboard.md | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/docs/web-ui/dashboard.md b/docs/web-ui/dashboard.md index 0918dab6..639bbdec 100644 --- a/docs/web-ui/dashboard.md +++ b/docs/web-ui/dashboard.md @@ -231,20 +231,37 @@ read from `GET /api/matrix-accounts?agent=` → `{ accounts: [ { name, homeserver, token_present, live, user_id } ], as_of_unix }`. `token_present` is whether a token is **stored**; `live`, `homeserver`, and `user_id` are backfilled from the matrix daemon's -`matrix-accounts.json` snapshot — a host-visible file the daemon writes at -startup after its sessions restore (an account with a token but absent -from the snapshot reports `live: false`). `as_of_unix` is the snapshot's -mtime (null when absent), so the dot can show "live as of N ago". The -snapshot is rewritten each daemon (re)start, so an old `as_of_unix` is -ambiguous (stable uptime vs dead daemon) — the live-status dot rendering -(3-state + snapshot-age tooltip, cross-referencing container-running -state) is the dashboard-side follow-up. +`matrix-accounts.json` snapshot — a host-visible file the daemon +**force-rewrites every ~30s** (a heartbeat), so `as_of_unix` (the +snapshot mtime) advances while the daemon is alive and a *stalled* value +genuinely means "stopped publishing", not just "old snapshot". An account +with a token but absent from the snapshot reports `live: false`. + +The status dot renders these states: + +- **green** — `live` and the container is running: online. +- **dim green** — `live` but `as_of_unix` hasn't advanced in > ~90s (3 + missed heartbeats) while the container is *not* down: the daemon stopped + publishing, so the snapshot's `live` is no longer trustworthy (likely + dead/wedged). Labelled "online · no heartbeat". +- **amber** — `live` but the container is **down** (a stopped container + ⟹ a dead daemon, so the snapshot is stale); also the `token_present && + !live` "provisioned but offline" case. +- **grey** — no token (not provisioned). + +The container-down cross-reference (`/api/state`) takes precedence over +the age check. `as_of_unix` is tooltipped ("live as of N ago") throughout +so freshness is always legible. When `live` is absent (an older backend +without the snapshot) the dot falls back to a token-present rendering. The provision form (account name, homeserver, login method) posts -`POST /matrix-account-login` (`x-www-form-urlencoded`, operator-auth): +`POST /api/matrix-account-login` (`x-www-form-urlencoded`, operator-auth): fields `agent, account, homeserver, mode=password|token, user_id?, -password?, token?` → `2xx { ok, user_id }` on success or -`4xx { error }` on failure. The host coordinator performs the login +password?, token?` → `200 { ok, user_id }` on success. Failures come back +as RFC 9457 `application/problem+json` (`{ type, title, status, detail }`) +with the human-readable message in `detail` and the status code reflecting +the cause (400 for a validation error, 500 for a login / `whoami` / +internal failure); the page reads `detail` for display. The host coordinator performs the login (password) or validates the token (`whoami`) and writes the bearer to the agent's `matrixAccounts..tokenFile` via the same privileged write path as the hive-internal `matrix-token`; the token is From f5ac6d79e3b2618580f41fe0ba333199eecc2d4c Mon Sep 17 00:00:00 2001 From: damocles Date: Mon, 22 Jun 2026 16:23:04 +0200 Subject: [PATCH 7/9] forge_notify: leave delivered threads unread, dedupe wakes in-memory --- docs/forge.md | 43 +++++++++++- hive-ag3nt/src/forge_notify.rs | 116 +++++++++++++++++++++++++++++---- 2 files changed, 143 insertions(+), 16 deletions(-) diff --git a/docs/forge.md b/docs/forge.md index f3bf9d00..a2dfd793 100644 --- a/docs/forge.md +++ b/docs/forge.md @@ -70,9 +70,46 @@ Background task spawned once per harness boot. Polls `GET /api/v1/notifications?all=false` every 30 seconds (Forgejo's unread-only filter), formats each notification as a broker `Wake { from: "forge" }` message, and delivers it to the agent's own -inbox so claude's normal turn loop picks it up. Mark-read happens -after successful delivery so a failed-delivery notification -resurfaces on the next tick. +inbox so claude's normal turn loop picks it up. + +### Mark-read on read, not on delivery + +Delivered conversation threads are deliberately left **unread** in +forge. The hive-forge read-before-comment guard keys off forge's own +notification read-state (`GET /notifications?all=false`) to refuse a +comment when a thread has unread activity by others — so the agent +reading the thread via the CLI (`hive-forge comments` / `view`, +which `PATCH`es `/notifications/threads/{id}`) is the single +mark-read point. If `forge_notify` marked threads read on delivery, +that unread signal would be consumed before the agent acts and the +guard could never fire. + +Because a delivered thread stays unread, it reappears in every +`?all=false` poll. An in-memory **delivery-dedupe cursor** (thread +id → last-delivered `updated_at`, held in the poll loop) stops the +same version from re-firing a wake; a new comment bumps `updated_at` +so genuinely new activity re-delivers. The cursor is pure anti-spam, +not a correctness oracle: lost on harness restart it just +re-delivers currently-unread threads once (harmless — `recv` +tolerates redelivery), so it carries none of the persisted-mirror +fragility that ruled out an on-disk seen-cursor. Each poll prunes +the cursor to the threads still in the unread set. A failed wake +delivery is left unread **and** out of the cursor, so it resurfaces +next tick. + +Two paths still mark-read directly (no read-before-comment value): +self-echo notifications (the agent's own writes, see below) and +`HIVE_FORGE_NOTIFY_SKIP_REASONS` drop-listed reasons. + +> Note: the unread list grows for threads the agent never reads via +> the CLI, since nothing else trims it. This does not affect guard +> correctness (the guard does a per-thread, repo-scoped query) nor +> wake delivery (Forgejo orders unread newest-first, so new activity +> always lands in the polled window). Bounding the unread list via a +> reason-independent firehose-reduction is a separate follow-up — the +> existing auto-unsubscribe below is gated on a `reason` field that +> this Forgejo's notification API does not actually emit, so it never +> fires today. ### Activation gates (graceful no-ops) diff --git a/hive-ag3nt/src/forge_notify.rs b/hive-ag3nt/src/forge_notify.rs index 86e0753b..b99026a1 100644 --- a/hive-ag3nt/src/forge_notify.rs +++ b/hive-ag3nt/src/forge_notify.rs @@ -1,7 +1,13 @@ //! Background Forgejo notification poller. Polls //! `GET /notifications?all=false` every 30s, formats each unread //! notification as a broker `Wake { from: "forge" }` message, and -//! marks it read after delivery so failures resurface next tick. +//! delivers it to the agent's inbox. Delivered threads are deliberately +//! left UNREAD in forge — the hive-forge read-before-comment guard keys +//! off forge's own unread-state, and the agent reading the thread via the +//! CLI is what marks it read. An in-memory delivery-dedupe cursor +//! (thread id → last-delivered `updated_at`) stops the still-unread +//! notification from re-firing a wake every poll; self-echo and +//! drop-listed notifications are still marked read directly. //! //! Activation gates, self-notification filtering, body excerpt + //! truncation + heading escape, wrapper formats (comment / review / @@ -9,7 +15,7 @@ //! reason drop-list, and auto-unsubscribe on broad watches all live //! in [`docs/forge.md::Notification poller`](../../../docs/forge.md). -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::fmt::Write as _; use std::path::{Path, PathBuf}; use std::time::Duration; @@ -139,6 +145,19 @@ pub async fn run(socket: PathBuf) { // across polls so we don't hammer DELETE on every cycle. let mut unsubbed_repos: HashSet = HashSet::new(); + // Delivery-dedupe cursor: notification thread id -> the `updated_at` + // of the version we last woke the agent for. We no longer mark a + // thread read on delivery (that would consume the unread signal the + // hive-forge read-before-comment guard relies on), so this in-memory + // map is what stops the same unread notification from re-firing a + // wake every poll. A new comment bumps `updated_at`, so the thread + // re-delivers. This is purely anti-spam, NOT a correctness oracle: + // lost on harness restart it just re-delivers currently-unread + // threads once (harmless — recv tolerates redelivery), so it carries + // none of the persisted-mirror fragility that sank the on-disk + // cursor approach. + let mut delivered: HashMap = HashMap::new(); + loop { interval.tick().await; poll_once( @@ -148,6 +167,7 @@ pub async fn run(socket: PathBuf) { &socket, keep_subscriptions, &mut unsubbed_repos, + &mut delivered, &own_login, &skip_reasons, ) @@ -723,9 +743,9 @@ fn days_from_civil(y: i64, m: i64, d: i64) -> i64 { #[allow( clippy::too_many_arguments, - reason = "the notification poll's config + mutable subscription state, \ - wired once from the poll loop; a struct would just move the \ - same fields one level out" + reason = "the notification poll's config + mutable subscription / \ + delivery-dedupe state, wired once from the poll loop; a struct \ + would just move the same fields one level out" )] #[allow( clippy::too_many_lines, @@ -740,6 +760,7 @@ async fn poll_once( socket: &Path, keep_subscriptions: bool, unsubbed_repos: &mut HashSet, + delivered: &mut HashMap, own_login: &str, skip_reasons: &[String], ) { @@ -784,6 +805,17 @@ async fn poll_once( continue; }; + // Delivery-dedupe: we no longer mark threads read on delivery, so + // an unread notification reappears in every `?all=false` poll. + // Skip it silently unless its `updated_at` advanced since the + // version we last delivered a wake for (i.e. genuinely new + // activity). See the `delivered` cursor note in `run`. + let updated_at = notif["updated_at"].as_str().unwrap_or("").to_owned(); + if !should_deliver(delivered, id, &updated_at) { + debug!(%id, "forge_notify: skipping (already delivered this version)"); + continue; + } + // Reason drop-list: suppress noisy reasons; null/unknown pass // through so directed signals stay deliverable (see // `docs/forge.md::Reason drop-list`). @@ -809,10 +841,10 @@ async fn poll_once( body, transient: false, }; - let delivered = crate::client::request::<_, hive_sh4re::Response>(socket, &req) + let deliver_result = crate::client::request::<_, hive_sh4re::Response>(socket, &req) .await .map(|_| ()); - match delivered { + match deliver_result { Ok(()) => { debug!(%id, "forge_notify: delivered"); } @@ -822,9 +854,13 @@ async fn poll_once( } } - // Mark as read only after successful delivery so a failed-delivery - // notification resurfaces on the next poll tick. - mark_read(client, forge_url, token, id).await; + // Record the delivered version in the dedupe cursor INSTEAD of + // marking the thread read. Leaving it unread is deliberate: the + // hive-forge read-before-comment guard keys off forge's own + // unread-state, and the agent reading the thread via the CLI is + // what marks it read. A failed delivery (above) is left + // unrecorded so it re-delivers next tick. + delivered.insert(id, updated_at); // Auto-unsubscribe from broad repo watches after delivering a // `subscribed` notification. Gated by HIVE_FORGE_KEEP_SUBSCRIPTIONS @@ -856,12 +892,34 @@ async fn poll_once( } } } + + // Prune the dedupe cursor down to the threads still present in this + // poll's unread set. Once the agent reads a thread (marking it read + // via the CLI) it drops out of `?all=false`, so its cursor entry is + // dead weight; dropping it bounds the map to the current unread size. + // If such a thread later goes unread again it carries a fresh + // `updated_at` and re-delivers correctly. + let current_ids: HashSet = notifications + .iter() + .filter_map(|n| n["id"].as_u64()) + .collect(); + delivered.retain(|id, _| current_ids.contains(id)); +} + +/// Whether a notification should be delivered as a wake given the +/// delivery-dedupe cursor. Delivers when the thread has never been +/// delivered, or when its `updated_at` advanced since the last delivered +/// version (genuinely new activity). Pure for unit testing. +fn should_deliver(delivered: &HashMap, id: u64, updated_at: &str) -> bool { + delivered.get(&id).is_none_or(|seen| seen != updated_at) } /// Mark a notification thread as read. Best-effort — logs on failure but -/// does not abort the poll loop. A notification left unread will resurface -/// on the next poll tick (desirable for delivery failures; for self-echo -/// silencing we call this without prior delivery). +/// does not abort the poll loop. Called only on the self-echo and +/// drop-listed paths (the agent's own writes / explicitly-suppressed +/// reasons) — delivered threads are deliberately left unread for the +/// read-before-comment guard, and a failed delivery is left unread + out +/// of the dedupe cursor so it resurfaces on the next poll tick. async fn mark_read(client: &reqwest::Client, forge_url: &str, token: &str, id: u64) { let mark_url = format!("{forge_url}/api/v1/notifications/threads/{id}"); match client @@ -886,6 +944,38 @@ async fn mark_read(client: &reqwest::Client, forge_url: &str, token: &str, id: u mod tests { use super::*; + #[test] + fn should_deliver_when_thread_never_seen() { + let delivered = HashMap::new(); + assert!(should_deliver(&delivered, 42, "2026-06-22T16:00:00Z")); + } + + #[test] + fn should_not_deliver_same_version_again() { + // The dedupe case: an unread thread reappears every poll with the + // same `updated_at` — must not re-fire a wake. + let mut delivered = HashMap::new(); + delivered.insert(42, "2026-06-22T16:00:00Z".to_owned()); + assert!(!should_deliver(&delivered, 42, "2026-06-22T16:00:00Z")); + } + + #[test] + fn should_deliver_when_updated_at_advanced() { + // A new comment bumps `updated_at` → genuinely new activity → + // deliver again. + let mut delivered = HashMap::new(); + delivered.insert(42, "2026-06-22T16:00:00Z".to_owned()); + assert!(should_deliver(&delivered, 42, "2026-06-22T16:05:00Z")); + } + + #[test] + fn should_deliver_tracks_per_thread() { + // A cursor for one thread says nothing about another. + let mut delivered = HashMap::new(); + delivered.insert(42, "2026-06-22T16:00:00Z".to_owned()); + assert!(should_deliver(&delivered, 99, "2026-06-22T16:00:00Z")); + } + #[test] fn escape_md_headings_escapes_top_level_atx() { // Argus reviews start with `## argus review`, which would From eb09ec4e287d969729ddbfb3090bff18923fbce3 Mon Sep 17 00:00:00 2001 From: damocles Date: Mon, 22 Jun 2026 16:45:24 +0200 Subject: [PATCH 8/9] forge_notify: record dedupe cursor inside the ok arm for clarity --- hive-ag3nt/src/forge_notify.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/hive-ag3nt/src/forge_notify.rs b/hive-ag3nt/src/forge_notify.rs index b99026a1..b56b4c7d 100644 --- a/hive-ag3nt/src/forge_notify.rs +++ b/hive-ag3nt/src/forge_notify.rs @@ -847,6 +847,15 @@ async fn poll_once( match deliver_result { Ok(()) => { debug!(%id, "forge_notify: delivered"); + // Record the delivered version in the dedupe cursor INSTEAD + // of marking the thread read. Leaving it unread is + // deliberate: the hive-forge read-before-comment guard keys + // off forge's own unread-state, and the agent reading the + // thread via the CLI is what marks it read. Recorded only + // here in the Ok arm — a failed delivery hits the Err arm + // and `continue`s without recording, so it re-delivers next + // tick. + delivered.insert(id, updated_at); } Err(e) => { warn!(%id, error = ?e, "forge_notify: deliver failed — leaving unread"); @@ -854,14 +863,6 @@ async fn poll_once( } } - // Record the delivered version in the dedupe cursor INSTEAD of - // marking the thread read. Leaving it unread is deliberate: the - // hive-forge read-before-comment guard keys off forge's own - // unread-state, and the agent reading the thread via the CLI is - // what marks it read. A failed delivery (above) is left - // unrecorded so it re-delivers next tick. - delivered.insert(id, updated_at); - // Auto-unsubscribe from broad repo watches after delivering a // `subscribed` notification. Gated by HIVE_FORGE_KEEP_SUBSCRIPTIONS // for triage / firehose agents (see From edad6f863cc8c667e672eefd402c809b2742f99e Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 22 Jun 2026 14:29:24 +0200 Subject: [PATCH 9/9] feat(#1886): trust a peer hive's root CA hive-wide for self-signed federation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add swarm.peers..caCert (path to a peer hive's root CA PEM), trusted everywhere the hive's own internal CA is — so a self-signed peer hive can federate (matrix) and any in-hive consumer validates its certs. Mechanism (reuses the existing hive-CA embedding): the meta-flake renderer embeds a LIST of CA files next to each agent's flake — hive-ca.pem (the hive's own self-signed CA, when active) plus each peer caCert as peer-ca-.pem — and emits them all in security.pki.certificateFiles, so every agent trusts them at build time. The matrix container trusts the same peer CAs for federation TLS. Nothing is installed in the host trust store; the certs live in the nix store (no mutable host file). - meta.rs: embedded_ca_files() = hive CA + peer CAs (from new HIVE_PEER_CA_PATHS env); ca_embed_state() tracks the list (content + add/remove); sync_agents materialises + stages the list; render emits the multi-entry certificateFiles. Tests cover hive-only / hive+peers / peers-only / none. - hive-c0re.nix: HIVE_PEER_CA_PATHS service env (colon-joined caCerts); caCert / certFingerprint option docs updated to the hive-wide scope. - hive-matrix.nix + docs/swarm.md: scope + comment updates. certFingerprint stays the c0re-only leaf-pin path. --- docs/swarm.md | 54 ++++---- hive-c0re/src/meta.rs | 237 +++++++++++++++++++++++++++++------- nix/modules/hive-c0re.nix | 50 +++++++- nix/modules/hive-matrix.nix | 15 +++ 4 files changed, 287 insertions(+), 69 deletions(-) diff --git a/docs/swarm.md b/docs/swarm.md index 4e0d6fb8..eb0d6b8d 100644 --- a/docs/swarm.md +++ b/docs/swarm.md @@ -40,25 +40,35 @@ and `qualify()` / `qualified_label()` semantics. ```nix services.hyperhive.swarm.peers = { "lab.example.com" = { }; # CA-trusted (Let's Encrypt etc.) - "edge.corp" = { certFingerprint = "sha256:…"; }; # self-signed TLS + "edge.corp" = { certFingerprint = "sha256:…"; }; # self-signed TLS, c0re peer checks only + "mesh.internal" = { caCert = ./mesh-ca.pem; }; # self-signed, trusted for matrix federation }; ``` -The attrset key is the peer's DNS domain. `certFingerprint` is -optional: +The attrset key is the peer's DNS domain. Two independent, optional +trust knobs — pick by what you need to trust: -- **Omitted / null** — the system CA bundle validates the peer's TLS - cert. Correct for peers with Let's Encrypt or any standard CA cert. -- **Set** (`"sha256:…"`) — pin a specific cert fingerprint. Use this - for peers whose self-signed TLS cert doesn't chain to a CA your - host trusts. - -`certFingerprint` scopes **only** to hive-c0re's own peer HTTPS checks -(the P33RS dashboard links and agent peer discovery below). It is -**not** consulted by matrix federation — tuwunel validates a peer's -federation certificate against the system CA bundle independently (see -*Matrix federation* below), so pinning a fingerprint here does nothing -for a self-signed matrix gateway cert. +- **`certFingerprint`** (`"sha256:…"`) — pin the peer's TLS *leaf* + fingerprint. Scopes **only** to hive-c0re's own peer HTTPS checks + (the P33RS dashboard links + agent peer discovery below). It is + **not** consulted by matrix federation — tuwunel validates a peer's + federation certificate against the system CA bundle independently + (see *Matrix federation* below), so a fingerprint pin does nothing + for a self-signed matrix cert. +- **`caCert`** (path to the peer's root CA PEM) — embeds that CA (at + build time, into the nix store — no runtime file on the host) and + trusts it **everywhere the hive's own internal CA is**: it rides + alongside `hive-ca.pem` in every agent's + `security.pki.certificateFiles` (via the meta-flake renderer) **and** + in the matrix container's trust bundle, so tuwunel validates the + peer's *federation* TLS when it chains to that CA. Trust stays + **inside the hive** (agents + the matrix container), never the host + system trust store. **This is the knob that unblocks federation with + a self-signed peer hive** — use it instead of `certFingerprint` when + you control the peer's CA. (It does not affect hive-c0re's own peer + HTTPS checks — those stay on `certFingerprint` / the system bundle.) +- **Both omitted** — the stock system CA bundle validates the peer + (correct for Let's Encrypt / any publicly-trusted peer). ### Fingerprint format @@ -114,13 +124,13 @@ environment and forwarded to agent containers. 3. **Matrix federation** — when `matrix.enable` is on, tuwunel federates with the peer's matrix server (discovered via the peer's `.well-known/matrix/server` delegation, which the gateway serves). - Federation validates the peer's TLS certificate against the - **system CA bundle** — independently of `certFingerprint`, which it - never consults. A self-signed gateway certificate therefore won't - federate even with a fingerprint pinned above: the peers need - CA-issued certs (ACME) or a shared private CA trusted on both - gateway hosts. See `docs/matrix.md` for federation firewall + TLS - requirements. + Federation validates the peer's TLS certificate against the matrix + **container's** trust bundle — independently of `certFingerprint`, + which it never consults. A self-signed gateway certificate therefore + won't federate unless the peer's root CA is trusted: set `caCert` + above (embeds the peer CA into the matrix container's trust bundle), + or give the peers CA-issued certs (ACME). See `docs/matrix.md` for + federation firewall + TLS requirements. ## Bilateral setup diff --git a/hive-c0re/src/meta.rs b/hive-c0re/src/meta.rs index 1479b842..87ef7b5c 100644 --- a/hive-c0re/src/meta.rs +++ b/hive-c0re/src/meta.rs @@ -67,12 +67,12 @@ pub async fn sync_agents(hive: &HiveEnv, agents: &[AgentSpec]) -> Result<()> { let on_disk = std::fs::read_to_string(&flake_path).unwrap_or_default(); let initial = !dir.join(".git").exists(); - // Hive CA embedding (self-signed TLS): keep `./hive-ca.pem` at the meta - // root in lockstep with the host CA so the build-time `certificateFiles` - // reference render_flake emits always resolves. `ca_desired` is empty - // when self-signed TLS isn't active (cert / ACME mode). - let ca_path = dir.join(HIVE_CA_FILE); - let (ca_desired, ca_changed) = hive_ca_state(&dir); + // Embedded-CA list (self-signed hive CA + peer CAs): keep the + // `./hive-ca.pem` / `./peer-ca-.pem` files at the meta root in + // lockstep with their host sources so the build-time `certificateFiles` + // list render_flake emits always resolves. Empty when neither a + // self-signed hive CA nor any peer CA is configured. + let (ca_files, ca_changed) = ca_embed_state(&dir); // Skip only when both the flake AND the embedded CA are unchanged — a // CA rotation with an otherwise-identical flake must still re-commit. @@ -98,15 +98,10 @@ pub async fn sync_agents(hive: &HiveEnv, agents: &[AgentSpec]) -> Result<()> { std::fs::write(&flake_path, &new_flake) .with_context(|| format!("write {}", flake_path.display()))?; - // Materialise (or drop) the embedded hive CA next to flake.nix. When - // self-signed TLS is off, `ca_desired` is empty and we remove any stale - // cert so the flake (which no longer references it) stays buildable. - if ca_desired.is_empty() { - let _ = std::fs::remove_file(&ca_path); - } else if ca_changed { - std::fs::write(&ca_path, &ca_desired) - .with_context(|| format!("write {}", ca_path.display()))?; - } + // Materialise the embedded CA list next to flake.nix + drop any stale + // CA file; `ca_touched` is every filename written or removed, staged + // for commit below. Public CA certs only; no private key is embedded. + let ca_touched = materialise_ca_files(&dir, &ca_files)?; // Reconcile topology.json against the live agent set — adds // entries for newly-spawned agents (default: manager as parent, @@ -148,11 +143,13 @@ pub async fn sync_agents(hive: &HiveEnv, agents: &[AgentSpec]) -> Result<()> { // contain '/flake.nix'". Lock then commit once with both // flake.nix and flake.lock — single commit per change. git(&dir, &["add", "flake.nix"]).await?; - // Stage the embedded hive CA — added/updated when self-signed TLS is on, - // or its deletion when it was just removed. `git add ` stages a - // deletion when the path is tracked and now gone; best-effort so the - // never-tracked-and-absent case (pathspec mismatch) is a harmless no-op. - let _ = git(&dir, &["add", "--", HIVE_CA_FILE]).await; + // Stage every embedded CA file we wrote or removed (hive CA + peer + // CAs). `git add ` stages a deletion when the path is tracked + // and now gone; best-effort so the never-tracked-and-absent case + // (pathspec mismatch) is a harmless no-op. + for name in &ca_touched { + let _ = git(&dir, &["add", "--", name]).await; + } // Stage topology.json on every sync (regenerated by reconcile // above when the agent set changed). git add is a no-op when the // file content is unchanged. @@ -200,6 +197,7 @@ pub async fn sync_agents(hive: &HiveEnv, agents: &[AgentSpec]) -> Result<()> { "flake.nix" => Some("flake"), "flake.lock" => Some("lock"), "hive-ca.pem" => Some("hive-ca"), + f if f.starts_with("peer-ca-") && has_pem_ext(f) => Some("peer-ca"), "topology.json" => Some("topology"), "capabilities.json" => Some("capabilities"), "tool-groups.json" => Some("tool-groups"), @@ -576,10 +574,10 @@ fn forwarded_env_vars() -> Vec<(&'static str, String)> { .collect() } -/// Filename the hive CA cert is embedded under at the meta-flake root. -/// `sync_agents` writes it and `render_flake` references `./hive-ca.pem` -/// in `security.pki.certificateFiles` so every agent trusts it at build -/// time. +/// Filename the hive's own self-signed CA cert is embedded under at the +/// meta-flake root. One entry of the embedded-CA list `render_flake` +/// references in `security.pki.certificateFiles` (see `embedded_ca_files`); +/// peer CAs sit alongside it as `peer-ca-.pem`. const HIVE_CA_FILE: &str = "hive-ca.pem"; /// Host path of the hive CA *certificate*, when self-signed TLS is active. @@ -597,17 +595,113 @@ fn hive_ca_source() -> Option { Some(path) } -/// Embedded-CA state for the meta repo: `(desired_contents, changed)`. -/// `desired_contents` is the host hive CA cert (empty when self-signed TLS -/// is inactive); `changed` is true when it differs from what's already -/// embedded at `/hive-ca.pem`, so a CA rotation re-commits even when -/// the flake itself is byte-identical. -fn hive_ca_state(dir: &std::path::Path) -> (String, bool) { - let on_disk = std::fs::read_to_string(dir.join(HIVE_CA_FILE)).unwrap_or_default(); - let desired = hive_ca_source() - .and_then(|p| std::fs::read_to_string(p).ok()) - .unwrap_or_default(); - let changed = desired != on_disk; +/// Host paths of peer-hive root CA certificates, from `HIVE_PEER_CA_PATHS` +/// (colon-separated; set by hive-c0re.nix from `swarm.peers..caCert`). +/// Each is embedded alongside the hive CA so a peer's CA is trusted +/// everywhere the hive's own internal CA is — i.e. by every agent. Empty +/// segments and paths that don't resolve to a file are dropped, so we +/// never reference a `certificateFiles` entry we couldn't embed. +fn peer_ca_sources() -> Vec { + let Ok(raw) = std::env::var("HIVE_PEER_CA_PATHS") else { + return Vec::new(); + }; + raw.split(':') + .map(str::trim) + .filter(|p| !p.is_empty() && std::path::Path::new(p).is_file()) + .map(ToOwned::to_owned) + .collect() +} + +/// The ordered set of CA certs embedded next to the meta flake, as +/// `(filename, host_source_path)`. The self-signed hive CA (when active) +/// is `hive-ca.pem`; each peer CA is `peer-ca-.pem` in declaration +/// order. `render_flake` emits exactly these filenames into +/// `security.pki.certificateFiles` and `sync_agents` materialises them, +/// so the rendered reference and the embedded files always agree. +fn embedded_ca_files() -> Vec<(String, String)> { + let mut out = Vec::new(); + if let Some(p) = hive_ca_source() { + out.push((HIVE_CA_FILE.to_owned(), p)); + } + for (i, p) in peer_ca_sources().into_iter().enumerate() { + out.push((format!("peer-ca-{i}.pem"), p)); + } + out +} + +/// Write each desired embedded CA file next to `flake.nix` and remove +/// any stale one (a hive CA turned off, or a peer dropped from config), +/// so the flake never references a file we didn't write. Returns every +/// filename written or removed, for the caller to stage. The public CA +/// certs only; no private key is ever embedded. +fn materialise_ca_files(dir: &Path, ca_files: &[(String, String)]) -> Result> { + let desired: std::collections::HashSet<&str> = + ca_files.iter().map(|(n, _)| n.as_str()).collect(); + let mut touched: Vec = Vec::new(); + if let Ok(entries) = std::fs::read_dir(dir) { + for e in entries.flatten() { + let fname = e.file_name(); + let Some(name) = fname.to_str() else { continue }; + if is_embedded_ca_name(name) && !desired.contains(name) { + let _ = std::fs::remove_file(dir.join(name)); + touched.push(name.to_owned()); + } + } + } + for (name, content) in ca_files { + let path = dir.join(name); + std::fs::write(&path, content).with_context(|| format!("write {}", path.display()))?; + touched.push(name.clone()); + } + Ok(touched) +} + +/// True for a filename `embedded_ca_files` can produce — the hive CA or +/// a `peer-ca-.pem`. Lets `sync_agents` find stale CA files to clean +/// up (a CA dropped from config) without touching unrelated meta files. +fn is_embedded_ca_name(name: &str) -> bool { + name == HIVE_CA_FILE || (name.starts_with("peer-ca-") && has_pem_ext(name)) +} + +/// True when `name` ends in a `.pem` extension (case-insensitive). Split +/// out so the embedded-CA filename checks share one spelling and dodge +/// clippy's case-sensitive-extension lint. +fn has_pem_ext(name: &str) -> bool { + Path::new(name) + .extension() + .is_some_and(|ext| ext.eq_ignore_ascii_case("pem")) +} + +/// Embedded-CA state for the meta repo: `(desired_files, changed)`. +/// `desired_files` is `(filename, contents)` for every CA that should sit +/// next to flake.nix (the hive CA + each peer CA). `changed` is true when +/// the on-disk set differs in any way — a file's contents changed, a new +/// CA appeared, or a previously-embedded CA (`hive-ca.pem` / +/// `peer-ca-*.pem`) is no longer wanted (stale, to be removed). Drives +/// both the re-commit decision and the materialise/cleanup in +/// `sync_agents`, so a CA rotation or a peer-set change re-commits even +/// when the flake itself is byte-identical. +fn ca_embed_state(dir: &std::path::Path) -> (Vec<(String, String)>, bool) { + let desired: Vec<(String, String)> = embedded_ca_files() + .into_iter() + .filter_map(|(name, path)| std::fs::read_to_string(&path).ok().map(|c| (name, c))) + .collect(); + let desired_names: std::collections::HashSet<&str> = + desired.iter().map(|(n, _)| n.as_str()).collect(); + + let mut changed = desired.iter().any(|(name, content)| { + std::fs::read_to_string(dir.join(name)).unwrap_or_default() != *content + }); + + // A previously-embedded CA file no longer wanted → stale (removal is + // a change even when every desired file already matches on disk). + if !changed && let Ok(entries) = std::fs::read_dir(dir) { + changed = entries.flatten().any(|e| { + e.file_name() + .to_str() + .is_some_and(|name| is_embedded_ca_name(name) && !desired_names.contains(name)) + }); + } (desired, changed) } @@ -748,16 +842,26 @@ where { "#, ); - // Self-signed TLS trust: embed the hive CA so every agent validates the - // gateway's self-signed leaf at build time. `security.pki.certificateFiles` - // is build-time, so the CA travels with the flake source — `sync_agents` - // writes `./hive-ca.pem` next to flake.nix and stages it. Only the public - // CA cert is embedded; the private key never leaves the host. Emitted only - // when hive-tls.nix signalled a CA (HIVE_TLS_CA_PATH) and the cert exists, - // matching the write condition in `sync_agents` so we never reference a - // file we didn't embed. - if hive_ca_source().is_some() { - out.push_str(" security.pki.certificateFiles = [ ./hive-ca.pem ];\n"); + // CA trust: embed every hive-trusted CA so each agent validates them at + // build time. The list is the hive's own self-signed CA (when active) + // plus every peer-hive root CA (`swarm.peers..caCert`) — a peer CA is + // trusted everywhere the hive's own internal CA is. `certificateFiles` is + // build-time, so the certs travel with the flake source: `sync_agents` + // writes `./hive-ca.pem` + `./peer-ca-.pem` next to flake.nix and + // stages them. Only public CA certs are embedded; no private key ever + // leaves the host. The filename list matches `sync_agents` exactly (both + // derive it from `embedded_ca_files`), so we never reference a file we + // didn't embed; emitted only when the list is non-empty. + let ca_refs: Vec = embedded_ca_files() + .into_iter() + .map(|(name, _)| format!("./{name}")) + .collect(); + if !ca_refs.is_empty() { + let _ = writeln!( + out, + " security.pki.certificateFiles = [ {} ];", + ca_refs.join(" ") + ); } out.push_str( r#" # The harness service inside the container runs as a @@ -1228,23 +1332,64 @@ mod tests { ) }; + // Two peer-hive CA temp files for the list cases. + let peer0 = std::env::temp_dir().join(format!("peer-ca0-test-{}.pem", std::process::id())); + let peer1 = std::env::temp_dir().join(format!("peer-ca1-test-{}.pem", std::process::id())); + std::fs::write( + &peer0, + "-----BEGIN CERTIFICATE-----\np0\n-----END CERTIFICATE-----\n", + ) + .expect("write peer CA 0"); + std::fs::write( + &peer1, + "-----BEGIN CERTIFICATE-----\np1\n-----END CERTIFICATE-----\n", + ) + .expect("write peer CA 1"); + let peer_paths = format!("{}:{}", peer0.display(), peer1.display()); + + // All env mutations are serialised within this one test (no other + // test asserts on these vars), restored before returning. unsafe { + std::env::remove_var("HIVE_PEER_CA_PATHS"); std::env::set_var("HIVE_TLS_CA_PATH", &ca_file); } let with_ca = render(); + // Hive CA + peer CAs: the list carries all three, hive CA first. + unsafe { + std::env::set_var("HIVE_PEER_CA_PATHS", &peer_paths); + } + let with_peers = render(); + // Peers only (this hive on ACME, federating with self-signed peers). unsafe { std::env::remove_var("HIVE_TLS_CA_PATH"); } + let peers_only = render(); + unsafe { + std::env::remove_var("HIVE_PEER_CA_PATHS"); + } let without_ca = render(); let _ = std::fs::remove_file(&ca_file); + let _ = std::fs::remove_file(&peer0); + let _ = std::fs::remove_file(&peer1); assert!( with_ca.contains("security.pki.certificateFiles = [ ./hive-ca.pem ]"), "CA cert must be wired into certificateFiles when signalled:\n{with_ca}" ); + assert!( + with_peers.contains( + "security.pki.certificateFiles = [ ./hive-ca.pem ./peer-ca-0.pem ./peer-ca-1.pem ]" + ), + "hive CA + peer CAs must all appear in the certificateFiles list:\n{with_peers}" + ); + assert!( + peers_only + .contains("security.pki.certificateFiles = [ ./peer-ca-0.pem ./peer-ca-1.pem ]"), + "peer CAs must be trusted even when this hive has no self-signed CA:\n{peers_only}" + ); assert!( !without_ca.contains("security.pki.certificateFiles"), - "no certificateFiles reference without the HIVE_TLS_CA_PATH signal:\n{without_ca}" + "no certificateFiles reference without any CA signal:\n{without_ca}" ); } } diff --git a/nix/modules/hive-c0re.nix b/nix/modules/hive-c0re.nix index eeaf1c4b..4d440a35 100644 --- a/nix/modules/hive-c0re.nix +++ b/nix/modules/hive-c0re.nix @@ -213,6 +213,35 @@ in then strip the colons and prepend `sha256:`. A malformed value is ignored with a warning rather than weakening trust. See docs/swarm.md for the full recipe. + + Scopes only to hive-c0re's own peer HTTPS checks — it does + NOT help Matrix federation (tuwunel validates against its + container trust bundle). For a self-signed peer whose root + CA you want trusted hive-wide (every agent + Matrix + federation), set `caCert` below. + ''; + }; + + caCert = lib.mkOption { + type = lib.types.nullOr lib.types.path; + default = null; + example = "./peers/edge-ca.pem"; + description = '' + Path to this peer hive's root CA certificate (PEM). When + set, the CA is embedded (at build time, into the nix store + — no runtime file on the host) and trusted **everywhere the + hive's own internal CA is**: it rides alongside `hive-ca.pem` + in each agent's `security.pki.certificateFiles` (via the + meta-flake renderer), and is added to the Matrix homeserver + container's trust bundle so tuwunel validates *federation* + TLS from a self-signed peer hive whose cert chains to it. + This is the CA-trust path that `certFingerprint` + (leaf-pinning, c0re-only) can't cover, and is what unblocks + Matrix federation with a self-signed peer hive. Trust stays + inside the hive (agents + the Matrix container), never the + host system trust store. Mutually complementary with + `certFingerprint`; set `caCert` for the federation case. See + docs/swarm.md. ''; }; @@ -825,7 +854,26 @@ in } ) config.services.hyperhive.swarm.peers ); - }; + } + // + lib.optionalAttrs + (lib.any (p: p.caCert != null) (lib.attrValues config.services.hyperhive.swarm.peers)) + { + # Peer-hive root CA file paths (colon-joined), one per peer that + # declares `swarm.peers..caCert`. hive-c0re's meta-flake + # renderer (meta.rs) embeds each next to every agent's flake and + # adds it to `security.pki.certificateFiles`, so a peer CA is + # trusted everywhere the hive's own internal CA (`hive-ca.pem`) + # is — i.e. by every agent. The matrix container trusts the same + # CAs separately for federation TLS. The `caCert` files are + # copied into the nix store at build, so these are store paths — + # nothing mutable lives on the host. + HIVE_PEER_CA_PATHS = lib.concatStringsSep ":" ( + lib.filter (c: c != null) ( + lib.mapAttrsToList (_domain: p: p.caCert) config.services.hyperhive.swarm.peers + ) + ); + }; serviceConfig = { ExecStart = "${cfg.package}/bin/hive-c0re --socket /run/hyperhive/host.sock serve --config ${serveConfig}"; # Migrate hive-c0re's *own* state to the service user after an diff --git a/nix/modules/hive-matrix.nix b/nix/modules/hive-matrix.nix index 555b8d4f..838bd08e 100644 --- a/nix/modules/hive-matrix.nix +++ b/nix/modules/hive-matrix.nix @@ -358,6 +358,21 @@ in { system.stateVersion = "26.05"; + # Peer-hive root CAs (`swarm.peers..caCert`) added to THIS + # container's trust bundle so tuwunel validates *federation* TLS + # from a self-signed peer hive (it checks the peer's federation + # cert against its trust bundle). Peer CAs are trusted everywhere + # the hive's own internal CA is — agents get them via the + # meta-flake renderer (`HIVE_PEER_CA_PATHS` → each agent's + # `security.pki.certificateFiles`); this block is the matrix + # container's copy, since the host `security.pki` store doesn't + # cross the container boundary. They are never installed in the + # HOST trust store. Null entries (CA-bundle / fingerprint-pinned + # peers) drop out. + security.pki.certificateFiles = lib.filter (c: c != null) ( + lib.mapAttrsToList (_domain: p: p.caCert) config.services.hyperhive.swarm.peers + ); + # tuwunel hard-fails to boot if `/etc/resolv.conf` has no # `nameserver` line (`Failed to configure DNS resolver ... no # nameservers found in config` → exit 1). This declarative