From efbfec6d013cca84d1debed990e8b4a76bec6957 Mon Sep 17 00:00:00 2001 From: atlas Date: Fri, 2 Oct 2026 22:32:17 +0200 Subject: [PATCH] config PRs: remove the hive's config-PR webhook, poll and core merge An operator's merge on the forge deploys a config PR through swarm-controller's DeployRequest{rev}. The hive-side path that queued a MergeConfigPr approval and merged the PR as `core` goes: - the `/webhook/config-pr` receiver, its HMAC secret, the WebhookRegister boot node and the org-hook registration; the hive vhost's `/webhook/` location - the 5-minute config-PR poll - ApprovalKind::MergeConfigPr, its dashboard card, and the deploy DAG it drove (DeployWindow, MergeVerify, DeployApply, FinalizeDeploy, DeployTail), with verify_commit, the two-phase meta deploy, rollback refs, the PR-failure comment and forge/pr_merge.rs - `fetched_sha`, `sha_short`/`pr_number` on approval events, and `sha`/`tag` on HelperEvent::ApprovalResolved: only the merge path set them `config_repo`, `merged_pr_for_commit` and `post_pr_comment` move to forge/pr_comment.rs for the merged-rev deploy's refusal comment. Approvals v5 drops stored `merge_config_pr` rows; a test reopens a v4 database holding them. Closes #4850 --- frontend/packages/dashboard/src/call.js | 52 +- hive-agent/prompts/system.md | 4 +- hive-c0re/src/actions.rs | 566 +----------------- hive-c0re/src/coordinator.rs | 37 +- hive-c0re/src/dashboard/approvals.rs | 5 - hive-c0re/src/dashboard/mod.rs | 20 +- hive-c0re/src/dashboard/state_snapshot.rs | 79 +-- hive-c0re/src/dashboard/webhook.rs | 362 ----------- hive-c0re/src/dashboard_events.rs | 18 +- hive-c0re/src/forge/ci_runner.rs | 3 +- hive-c0re/src/forge/config_pr_poll.rs | 189 ------ hive-c0re/src/forge/mod.rs | 227 ++----- hive-c0re/src/forge/pr_comment.rs | 56 ++ hive-c0re/src/forge/pr_merge.rs | 332 ---------- hive-c0re/src/job_queue/exec.rs | 106 +--- hive-c0re/src/job_queue/model.rs | 36 +- hive-c0re/src/job_queue/templates.rs | 103 +--- hive-c0re/src/job_queue/tests.rs | 180 +----- hive-c0re/src/lifecycle/git.rs | 59 -- hive-c0re/src/lifecycle/mod.rs | 5 +- hive-c0re/src/lifecycle/setup.rs | 4 +- hive-c0re/src/main.rs | 52 +- hive-c0re/src/meta.rs | 232 +------ hive-c0re/src/paths.rs | 19 - hive-c0re/src/server.rs | 2 +- .../src/socket_server/config_approvals.rs | 103 ---- hive-c0re/src/socket_server/mod.rs | 9 - hive-c0re/src/socket_server/schedules.rs | 3 - hive-c0re/src/stores/approvals.rs | 166 ++--- hive-c0re/src/swarm_status.rs | 6 +- hive-c0re/src/webhook_secret.rs | 295 --------- hive-c0re/src/workers/auto_update.rs | 8 +- hive-forge/src/client.rs | 2 +- hive-sh4re/src/approvals.rs | 20 +- hive-sh4re/src/manager.rs | 4 - nix/host-modules/hive-gateway/vhosts.nix | 20 +- swarm-controller/src/config_pr.rs | 18 +- swarm-controller/src/forge.rs | 33 +- swarm-controller/src/webhook.rs | 50 +- 39 files changed, 286 insertions(+), 3199 deletions(-) delete mode 100644 hive-c0re/src/dashboard/webhook.rs delete mode 100644 hive-c0re/src/forge/config_pr_poll.rs create mode 100644 hive-c0re/src/forge/pr_comment.rs delete mode 100644 hive-c0re/src/forge/pr_merge.rs delete mode 100644 hive-c0re/src/socket_server/config_approvals.rs delete mode 100644 hive-c0re/src/webhook_secret.rs diff --git a/frontend/packages/dashboard/src/call.js b/frontend/packages/dashboard/src/call.js index f1c2c417..977bdfab 100644 --- a/frontend/packages/dashboard/src/call.js +++ b/frontend/packages/dashboard/src/call.js @@ -137,8 +137,6 @@ export function applyApprovalAdded(ev) { id: ev.id, agent: ev.agent, kind: ev.approval_kind, - sha_short: ev.sha_short || null, - pr_number: ev.pr_number ?? null, description: ev.description || null, // The ApprovalAdded event carries no requested_at; a live-added // approval was queued just now, so client-now is accurate — and @@ -163,7 +161,6 @@ export function applyApprovalResolved(ev) { id: ev.id, agent: ev.agent, kind: ev.approval_kind, - sha_short: ev.sha_short || null, status: ev.status, resolved_at: ev.resolved_at, note: ev.note || null, @@ -223,16 +220,8 @@ export function renderApprovals() { root.append(el("p", { class: "empty" }, "queue empty")); return; } - // forge link base — only when the hive-forge container is up. - const fs = window.__hyperhive_state; - // state.forge_public_url (set from services.hyperhive.forge.publicUrl) - // or null — never guessed from ":3000". The PR-link builder - // below already gates on forgeBase being truthy. - const forgeBase = (fs && fs.forge_present && fs.forge_public_url) || null; - const ul = el("ul", { class: "approvals" }); for (const a of pending) { - const isMergePr = a.kind === "merge_config_pr"; const isUpdateMeta = a.kind === "update_meta_inputs"; const isSchedule = a.kind === "schedule_prompt"; const li = el("li", { class: "approval-card" }); @@ -241,20 +230,11 @@ export function renderApprovals() { const head = el( "div", { class: "approval-head" }, - el( - "span", - { class: "glyph" }, - isUpdateMeta ? "↻" : isSchedule ? "⏱" : "⇒", - ), + el("span", { class: "glyph" }, isUpdateMeta ? "↻" : "⏱"), el("span", { class: "id" }, "#" + a.id), el("span", { class: "agent" }, a.agent), - el( - "span", - { class: "kind" }, - isUpdateMeta ? "meta-update" : isSchedule ? "schedule" : "merge-pr", - ), + el("span", { class: "kind" }, isUpdateMeta ? "meta-update" : "schedule"), ); - if (isMergePr && a.sha_short) head.append(el("code", {}, a.sha_short)); // When the approval was requested — relative time, right-aligned. // Goes amber once it's been pending an hour so a stale request is // obvious at a glance (see docs/web-ui/dashboard.md::Approval card). @@ -280,27 +260,7 @@ export function renderApprovals() { if (a.description) { body.append(el("div", { class: "approval-description" }, a.description)); } - if (isMergePr) { - // PR-based config deploy: link to the reviewed PR on the forge. - // The config diff lives on the forge PR itself. - const drill = el("div", { class: "drill-ins" }); - if (forgeBase && a.pr_number != null) { - drill.append( - el( - "a", - { - class: "panel-trigger", - target: "_blank", - rel: "noopener", - href: `${forgeBase}/agent-configs/${a.agent}/pulls/${a.pr_number}`, - title: "review this config PR on the hive forge", - }, - "↳ review PR on forge ↗", - ), - ); - } - body.append(drill); - } else if (isUpdateMeta) { + if (isUpdateMeta) { let inputs; try { inputs = JSON.parse(a.commit_ref || "[]"); @@ -421,11 +381,7 @@ function renderApprovalHistory(root, history) { el( "span", { class: "kind" }, - a.kind === "update_meta_inputs" - ? "meta-update" - : a.kind === "schedule_prompt" - ? "schedule" - : "merge-pr", + a.kind === "update_meta_inputs" ? "meta-update" : "schedule", ), " ", ); diff --git a/hive-agent/prompts/system.md b/hive-agent/prompts/system.md index f73a1949..973e671f 100644 --- a/hive-agent/prompts/system.md +++ b/hive-agent/prompts/system.md @@ -10,13 +10,13 @@ Need new packages, env vars, or other NixOS config for yourself? You can't edit Your config repo is mounted **read-only** at `/agents/{label}/config/` — `agent.nix` plus whatever extra files define you (declared packages, env vars, MCP servers). Read it to see exactly what defines you before asking for a change, so you can point at the precise file and line. -Approval boundary: starting, stopping and rebuilding containers is the operator's — ask them when any agent needs one. _Creating_ a new agent is not something you can do from here — ask the operator, who scaffolds the new agent's config repo and spawns it from the dashboard. _Changing_ any agent's config is not a tool call at all — it's a forge PR on the agent's `agent-configs/` repo, which queues a `MergeConfigPr` approval on open/update. The operator only signs off on changes; you run the day-to-day. +Approval boundary: starting, stopping and rebuilding containers is the operator's — ask them when any agent needs one. _Creating_ a new agent is not something you can do from here — ask the operator, who scaffolds the new agent's config repo and spawns it from the dashboard. _Changing_ any agent's config is not a tool call at all — it's a forge PR on the agent's `agent-configs/` repo, which an operator reviews and merges on the forge; the merge deploys it. The operator only signs off on changes; you run the day-to-day. Messages from sender `system` are hyperhive helper events (JSON body, `event` field discriminates): `approval_resolved`, `container_crash`, `needs_update`. Use these to react to lifecycle changes: - `needs_update` — agent's flake rev is stale. Ask the operator to rebuild it. - `container_crash` — ask the operator to start it again; if it keeps crashing, say so with what you saw. -- `approval_resolved` — one of your own submitted approvals (a scheduled prompt, a config PR, …) was approved, denied, or failed; the body carries the resolution. +- `approval_resolved` — one of your own submitted approvals (a scheduled prompt, …) was approved, denied, or failed; the body carries the resolution. Lifecycle notices that don't need an immediate turn — a new agent spawned, a container rebuilt/killed/destroyed, or its login state changing — surface as todos instead of messages now. Call `get_loose_ends` to see them. diff --git a/hive-c0re/src/actions.rs b/hive-c0re/src/actions.rs index ae214a94..57a09c66 100644 --- a/hive-c0re/src/actions.rs +++ b/hive-c0re/src/actions.rs @@ -14,16 +14,12 @@ use crate::lifecycle; /// Approve a pending request. Marks the approval row durably, then submits /// the work to the job queue so the dashboard POST returns immediately while -/// the long-running pipeline runs off-thread (operator no longer blocks on a -/// 30-90s spinner for `MergeConfigPr`). +/// the long-running pipeline runs off-thread. /// /// Dispatch: -/// - `MergeConfigPr` → a `DeployWindow` DAG (`MergeVerify → DeployApply → -/// FinalizeDeploy`, plus an `AfterAny` `DeployTail`, under a -/// resource-holding root; ~30-90s) /// - `UpdateMetaInputs` → a `MetaUpdate` DAG (fan-out on completion) /// -/// Every queued kind — deploys included — resolves its approval row via +/// Every queued kind resolves its approval row via /// [`resolve_approval_dag`] when the DAG settles terminal. pub async fn approve(coord: Arc, id: i64) -> Result<()> { let approval = coord.approvals.mark_approved(id)?; @@ -67,155 +63,9 @@ pub async fn approve(coord: Arc, id: i64) -> Result<()> { } result } - ApprovalKind::MergeConfigPr => { - // The work ends in a container rebuild, so route it through the - // rebuild queue. The queue worker dispatches the deploy DAG's - // nodes to `run_deploy_merge_verify` (drift gate + eval), - // `run_deploy_apply` (ff-merge, then grows the rebuild subgraph), - // `run_finalize_deploy` (deploy tag + lock commit) and - // `run_deploy_tail` (compensation + forge mirror). - enqueue_approval_rebuild(&coord, approval.agent.as_str(), id); - Ok(()) - } } } -/// Submit the deploy DAG tied to an approval id. Used by the `MergeConfigPr` -/// dispatch arm — the work ends in a container rebuild routed through the -/// queue. See [`crate::job_queue::templates::approval_deploy`] for the node -/// shape; the executor dispatches each node to the `run_deploy_*` bodies below. -fn enqueue_approval_rebuild(coord: &Arc, agent: &str, approval_id: i64) { - if let Err(e) = coord.job_queue.insert_job(|b| { - crate::job_queue::templates::approval_deploy(b, agent, approval_id); - Vec::new() - }) { - tracing::error!(%agent, approval_id, error = ?e, "insert approval deploy dag failed"); - } - coord.emit_rebuild_queue_snapshot(); -} - -/// Ref under which [`run_deploy_apply`] parks the pre-merge `applied/main` -/// sha, for [`run_deploy_tail`] to compensate with. -/// -/// Deliberately a *git ref in the applied repo* rather than an in-memory value -/// handed between nodes: hive-c0re can restart between the apply and the tail, -/// and the whole point of splitting the deploy is that the tail still knows -/// what to undo when it does. The ref's existence IS the "a merge landed but -/// hasn't been confirmed good yet" flag — [`run_finalize_deploy`] drops it the -/// moment the rebuild has come up clean. -fn rollback_ref(approval_id: i64) -> String { - format!("refs/hyperhive/rollback/{approval_id}") -} - -/// Everything a deploy node needs, re-derived from sqlite on each node rather -/// than cached across the DAG. Nothing here is *computed* by an earlier node — -/// `pr` and `reviewed` are fields of the approval row the operator signed off -/// on — so re-reading is both cheap and the authoritative source of truth. -struct DeployCtx { - approval: hive_sh4re::approvals::Approval, - /// PR number, parsed from `approval.commit_ref`. - pr: u64, - /// The PR head sha the operator reviewed (`approval.fetched_sha`). - reviewed: String, - applied_dir: std::path::PathBuf, - /// The agent's forge config repo (`/`). - repo: String, -} - -fn deploy_ctx(coord: &Coordinator, approval_id: i64) -> Result { - let approval = fetch_approval_for_worker(coord, approval_id, ApprovalKind::MergeConfigPr)?; - let pr: u64 = approval.commit_ref.parse().map_err(|e| { - anyhow::anyhow!( - "parse PR number from commit_ref {:?}: {e}", - approval.commit_ref - ) - })?; - let reviewed = approval.fetched_sha.clone().ok_or_else(|| { - anyhow::anyhow!("merge config pr approval {approval_id} has no reviewed head sha") - })?; - Ok(DeployCtx { - pr, - reviewed, - applied_dir: crate::paths::applied_dir(approval.agent.as_str()), - repo: crate::forge::config_repo(approval.agent.as_str()), - approval, - }) -} - -/// `MergeVerify` node body — everything that can say "no" before anything is -/// mutated. `approval.commit_ref` is the PR number; `approval.fetched_sha` is -/// the PR head the operator reviewed. Steps: -/// 1. drift gate — re-read the live PR head; if it moved since review, abort -/// (the operator must re-review the new head); -/// 2. fetch the reviewed head into the applied repo so later git ops resolve -/// it locally; -/// 3. ancestry gate — the reviewed head must descend from `applied/main`; -/// 4. eval-verify the reviewed commit against the meta flake. -/// -/// Steps 1 and 3 ask different questions and both are load-bearing. The drift -/// gate asks whether the *head* is still what was reviewed; the ancestry gate -/// asks whether the *base* is still underneath it. A PR opened from a stale base -/// passes the drift gate untouched and then rewinds `main` when it lands, -/// silently dropping every commit made in between. -/// -/// Nothing here needs undoing on failure: the fetch only adds objects, and -/// `main` doesn't move. That's the whole reason this is its own node — a -/// failure at this stage leaves [`run_deploy_tail`] with no ref to compensate. -/// -/// # Errors -/// -/// Returns an error if the approval can't be loaded, if the live PR head has -/// drifted from the reviewed sha, if fetching that head into the applied repo -/// fails, if the reviewed head does not descend from `applied/main`, or if the -/// eval-verify of the reviewed commit fails. Every one of these leaves the forge -/// and `main` untouched, so the node is safely retryable. -pub async fn run_deploy_merge_verify( - coord: &Arc, - approval_id: i64, - node_id: Option, -) -> Result<()> { - let ctx = deploy_ctx(coord, approval_id)?; - let pr = ctx.pr; - let reviewed = ctx.reviewed.as_str(); - - // 1. Drift gate: the live PR head must still equal what was reviewed. - let head = crate::forge::pr_head_sha(&ctx.repo, pr) - .await - .map_err(|e| anyhow::anyhow!("read PR #{pr} head: {e}"))?; - if head != reviewed { - bail!( - "PR #{pr} head drifted since review (reviewed {reviewed}, now {head}); re-review before merging" - ); - } - - // 2. Fetch the reviewed head into applied so ff/verify/deploy resolve it. - crate::forge::fetch_pr_head_into_applied(&ctx.repo, pr) - .await - .map_err(|e| anyhow::anyhow!("fetch PR #{pr} head into applied: {e}"))?; - - // 3. Ancestry gate: `main` must be reachable from the reviewed head, or the - // "fast-forward" in prepare_applied_target is really a rewind that drops - // every commit between the PR's base and where `main` actually is now. - let (current_main, descends) = applied_main_under(&ctx.applied_dir, reviewed).await?; - if !descends { - bail!( - "PR #{pr} does not descend from applied/main (main {current_main}, reviewed {reviewed}); \ - merging it would discard commits — rebase the PR onto main and re-review" - ); - } - - // 4. Eval-verify BEFORE the irreversible merge (bad nix fails fast here). - crate::meta::verify_commit( - ctx.approval.agent.as_str(), - &ctx.applied_dir, - reviewed, - node_id, - ) - .await - .map_err(|e| anyhow::anyhow!("verify merge head {reviewed}: {e:#}"))?; - Ok(()) -} - /// `applied/main`, and whether `target` descends from it. Fast-forwarding /// `main` to a `target` that does not is a rewind: it drops every commit on /// `main` that `target` lacks. @@ -262,8 +112,8 @@ pub(crate) async fn advance_applied_to_rev(agent: &str, rev: &str) -> Result( applied_dir: &std::path::Path, rev: &str, @@ -290,210 +140,9 @@ async fn advance_applied_main( Ok(RevAdvance::Advanced) } -/// `DeployApply` node body — the irreversible half. Parks the rollback ref, -/// fast-forward-merges the PR (THE merge), then opens the deploy. -/// -/// The ref is parked *before* the merge, so a hive-c0re crash anywhere from -/// here on still leaves [`run_deploy_tail`] enough to undo. If the merge itself -/// fails, `main` never moved and the tail's compensation is a no-op against the -/// same sha — harmless, and cheaper than trying to be clever about it. -/// -/// Returning `Ok` is the signal for the caller to grow the rebuild subgraph into -/// this DAG under this node; the build itself does not happen here. -/// -/// # Errors -/// -/// Returns an error if the approval can't be loaded, if reading or parking the -/// pre-merge `main` sha fails, if the forge refuses the fast-forward merge -/// (including a head that drifted between verify and merge), or if -/// fast-forwarding `applied/main` / `meta::prepare_deploy` fails. From the merge -/// onward a failure is *not* retryable on its own — [`run_deploy_tail`] runs -/// `AfterAny` to compensate. -pub async fn run_deploy_apply( - coord: &Arc, - approval_id: i64, - node_id: Option, -) -> Result<()> { - let ctx = deploy_ctx(coord, approval_id)?; - let agent = ctx.approval.agent.as_str(); - let pr = ctx.pr; - - let prev_main = lifecycle::git_rev_parse(&ctx.applied_dir, "refs/heads/main") - .await - .map_err(|e| anyhow::anyhow!("read applied/main: {e:#}"))?; - lifecycle::git_update_ref(&ctx.applied_dir, &rollback_ref(approval_id), &prev_main) - .await - .map_err(|e| anyhow::anyhow!("park rollback ref for approval {approval_id}: {e:#}"))?; - - // THE merge: fast-forward-only merge the reviewed head to `main` via the - // forge API, pinned to the reviewed sha (`head_commit_id`). This one call - // both advances `main` to the reviewed head and marks the PR merged — no - // direct push to the protected branch. A failure here means `main` was NOT - // advanced, so it's fatal: we must not deploy a head the forge didn't merge. - match crate::forge::merge_config_pr_ff(&ctx.repo, pr, &ctx.reviewed).await { - Ok(()) => {} - Err(crate::forge::ForgeMergeError::HeadDrift { expected, actual }) => bail!( - "PR #{pr} head drifted before merge (reviewed {expected}, now {actual}); re-review before merging" - ), - Err(e) => bail!("ff-merge PR #{pr}: {e}"), - } - - prepare_applied_target(agent, &ctx.applied_dir, &ctx.reviewed, &prev_main, node_id).await -} - -/// `DeployTail` node body — compensation + bookkeeping, `AfterAny` the apply -/// node so it runs on every outcome including a cancel-cascade. Infallible by -/// construction: it is the recovery step, so it has nothing to hand a failure -/// to. Every fallible call inside warns and continues. -/// -/// 1. If the rollback ref survived, the deploy did not confirm good: tag the -/// merged commit `failed/` (annotated with the DAG's first error, while -/// that sha is still reachable), then roll `applied/main` back to the parked -/// sha, resync the working tree, and drop the staged meta lock so the deploy -/// log only ever shows successes. -/// 2. Mirror the agent's config repo to the forge. `main` is already ff'd by -/// the merge, so only the `deployed/` / `failed/` tag refspec -/// actually lands — that's what gives the merged commit a forge-visible -/// deploy marker. -/// -/// Takes `agent` from the node payload rather than the approval row so it still -/// works if the row vanished underneath the DAG (deny race, purge). -pub async fn run_deploy_tail( - coord: &Arc, - queue_entry_id: Option, - agent: &str, - approval_id: i64, -) { - let applied_dir = crate::paths::applied_dir(agent); - let rollback = rollback_ref(approval_id); - if let Ok(prev_main) = lifecycle::git_rev_parse(&applied_dir, &rollback).await { - // Belt and braces: `run_deploy_apply` drops the ref before it plants - // `deployed/`, so seeing both means the *delete* failed on an - // otherwise-successful deploy. Rolling back there would be the worst - // outcome this node can produce, so the tag wins. - if lifecycle::git_rev_parse(&applied_dir, &format!("deployed/{approval_id}")) - .await - .is_ok() - { - tracing::warn!( - %agent, approval_id, - "deploy tail: rollback ref outlived a successful deploy; dropping it without compensating" - ); - } else { - // Mark the commit that failed to deploy, before undoing the merge - // that put it on `main`. After the rollback below, that sha is only - // reachable through this tag. - // - // Gated on `main` having actually moved: the rollback ref is parked - // *before* the merge, so its existence alone doesn't mean a merge - // happened. A pre-merge rejection (drift gate, eval failure, or the - // ff-merge itself failing) has no deployed commit to blame, and - // tagging the previous — innocent — head would point the operator at - // a commit that never got near a container. - // - // The annotation is read off the DAG rather than passed down from - // the node that failed: this node runs `AfterAny` its subject, so by - // now that node has settled `Failed` with its error recorded. - if let Ok(merged) = lifecycle::git_rev_parse(&applied_dir, "refs/heads/main").await - && merged != prev_main - { - let tag = format!("failed/{approval_id}"); - let body = queue_entry_id - .and_then(|dag_id| coord.job_queue.first_error(dag_id)) - .unwrap_or_else(|| "deploy failed".to_owned()); - if let Err(e) = - lifecycle::git_tag_annotated(&applied_dir, &tag, &merged, &body).await - { - tracing::warn!(%agent, approval_id, error = ?e, "deploy tail: annotate failed tag failed"); - } - } - - if let Err(e) = - lifecycle::git_update_ref(&applied_dir, "refs/heads/main", &prev_main).await - { - tracing::warn!(%agent, approval_id, error = ?e, "deploy tail: main rollback failed"); - } - if let Err(e) = lifecycle::git_read_tree_reset(&applied_dir, "refs/heads/main").await { - tracing::warn!(%agent, approval_id, error = ?e, "deploy tail: rollback read-tree failed"); - } - if let Err(e) = crate::meta::abort_deploy().await { - tracing::warn!(%agent, approval_id, error = ?e, "deploy tail: meta abort_deploy failed"); - } - } - if let Err(e) = lifecycle::git_delete_ref(&applied_dir, &rollback).await { - tracing::warn!(%agent, approval_id, error = ?e, "deploy tail: drop rollback ref failed"); - } - } - - if let Err(e) = crate::forge::push_config(agent).await { - tracing::warn!(%agent, error = ?e, "forge: push_config after merge failed"); - } -} - -/// Max stderr bytes to inline in a PR failure comment. Keeps the comment -/// readable and under forge's size limits while still carrying the tail -/// where the nix/build error actually surfaces. -const PR_FAIL_LOG_TAIL_BYTES: usize = 4000; - -/// On a failed `MergeConfigPr` deploy, post the failing build log back to the -/// config PR as a comment so the manager sees the rejection reason on the PR -/// itself. Best-effort: any error here is logged, never allowed to disturb the -/// approval-resolution path. -/// -/// The failing `build_log` row is located heuristically: the most recent `fail` -/// row for this agent that started at/after the approval was decided (i.e. when -/// its deploy DAG was submitted). Because deploys are serialised per agent -/// through the queue, that is the step which just failed — `verify`, -/// `prepare-deploy`, `prebuild`, or the container rebuild. Pre-build failures -/// (drift gate, fetch) create no `build_log` row, so the comment then carries -/// only the error text. -async fn post_merge_failure_to_pr( - coord: &Arc, - approval: &hive_sh4re::approvals::Approval, - err: &anyhow::Error, -) { - let Ok(pr) = approval.commit_ref.parse::() else { - return; - }; - let repo = crate::forge::config_repo(approval.agent.as_str()); - let since_ts = approval - .resolved_at - .unwrap_or(approval.requested_at) - .timestamp(); - - let log_section = coord - .build_logs - .list_recent_for_agent(approval.agent.as_str(), 10) - .ok() - .and_then(|rows| { - rows.into_iter() - .find(|r| r.status.as_deref() == Some("fail") && r.started_at >= since_ts) - }) - .and_then(|row| coord.build_logs.get_full(row.id).ok().flatten()) - .map(|full| { - let tail = tail_bytes(full.stderr.trim_end(), PR_FAIL_LOG_TAIL_BYTES); - format!( - "\n\n**Failing step:** `{}` (build log #{})\n\n```\n{tail}\n```", - full.header.kind, full.header.id - ) - }) - .unwrap_or_default(); - - let body = format!( - "## ⚠️ config deploy failed\n\n\ - Approval #{} to merge this PR could not be deployed:\n\n\ - ```\n{err:#}\n```{log_section}", - approval.id - ); - - if let Err(e) = crate::forge::post_pr_comment(&repo, pr, &body).await { - tracing::warn!(agent = %approval.agent, %pr, error = ?e, "post merge-failure comment to PR failed"); - } -} - /// Post why the merged commit `rev` of `agent`'s config repo was not deployed -/// onto the PR whose merge produced it. Best-effort, like -/// [`post_merge_failure_to_pr`]: a failure here is logged only. +/// onto the PR whose merge produced it. Best-effort: a failure here is logged +/// only. pub(crate) async fn post_rev_deploy_failure_to_pr(agent: &str, rev: &str, err: &anyhow::Error) { let repo = crate::forge::config_repo(agent); let pr = match crate::forge::merged_pr_for_commit(&repo, rev).await { @@ -513,19 +162,6 @@ pub(crate) async fn post_rev_deploy_failure_to_pr(agent: &str, rev: &str, err: & } } -/// Return the last `max_bytes` of `s`, snapped to a char boundary, prefixed -/// with an elision marker when truncated. -fn tail_bytes(s: &str, max_bytes: usize) -> String { - if s.len() <= max_bytes { - return s.to_owned(); - } - let mut start = s.len() - max_bytes; - while start < s.len() && !s.is_char_boundary(start) { - start += 1; - } - format!("[… truncated …]\n{}", &s[start..]) -} - /// Inline (non-queued) handler for `ApprovalKind::SchedulePrompt`. /// On approve, decode the `SchedulePromptPayload` JSON from the /// approval's `commit_ref`, insert a row into `scheduled_prompts` @@ -554,12 +190,12 @@ async fn run_approval_schedule_prompt( .context("insert scheduled prompt") } .await; - finish_approval(coord, &approval, result, None).await + finish_approval(coord, &approval, result) } /// Resolve an approval row from how its DAG's work ended — the body of the /// [`NodeKind::ResolveApproval`] tail node. Every approval-carrying template -/// resolves here, deploys included: the deploy pipeline is ordinary queue nodes, +/// resolves here: the pipeline is ordinary queue nodes, /// so the work's terminal state is the authoritative outcome and there's no /// in-node resolution to skip around. /// @@ -568,7 +204,7 @@ async fn run_approval_schedule_prompt( /// their own node rather than from one node branching. /// /// [`NodeKind::ResolveApproval`]: crate::job_queue::NodeKind::ResolveApproval -pub(crate) async fn resolve_approval_dag( +pub(crate) fn resolve_approval_dag( coord: &Arc, approval_id: i64, outcome: crate::job_queue::TerminalState, @@ -595,77 +231,15 @@ pub(crate) async fn resolve_approval_dag( } TerminalState::Failed => Err(anyhow::anyhow!("{}", error.unwrap_or("job dag failed"))), }; - let mut terminal_tag = None; - if approval.kind == ApprovalKind::MergeConfigPr { - terminal_tag = deploy_terminal_tag(approval.agent.as_str(), approval_id, outcome).await; - // On a failed deploy, surface the failing build log back onto the - // PR so the manager sees why it was rejected without leaving the - // forge. Posted here rather than inside a node because this is the - // one place that holds the DAG's definitive error — a `MergeVerify` - // rejection and a `DeployApply` build failure both land here. - if let Err(e) = &result { - post_merge_failure_to_pr(coord, &approval, e).await; - } - } - if let Err(e) = finish_approval(coord, &approval, result, terminal_tag).await { + if let Err(e) = finish_approval(coord, &approval, result) { tracing::warn!(approval_id, error = ?e, "approval dag resolved with failure"); } } -/// Which bookkeeping tag a settled deploy DAG actually planted, for the -/// `Rebuilt` event's `tag` field. The state picks the candidate name, but the -/// applied repo has the final say: a pre-merge rejection (`MergeVerify` drift -/// gate, eval failure) fails the DAG without ever planting `failed/`, and -/// tag plants are best-effort. Reporting a tag that isn't there would send the -/// manager looking for a ref that doesn't exist. -async fn deploy_terminal_tag( - agent: &str, - approval_id: i64, - outcome: crate::job_queue::TerminalState, -) -> Option { - use crate::job_queue::TerminalState; - let candidate = match outcome { - TerminalState::Done => format!("deployed/{approval_id}"), - // Nothing ran, so nothing was planted. - TerminalState::Cancelled | TerminalState::Skipped => return None, - TerminalState::Failed => format!("failed/{approval_id}"), - }; - lifecycle::git_rev_parse(&crate::paths::applied_dir(agent), &candidate) - .await - .ok() - .map(|_| candidate) -} - -/// Re-fetch an approval row from sqlite for a queue-worker dispatch. -/// Bails if the row is gone (deny race), if its kind doesn't match, -/// or if the lookup itself fails. The kind check is defensive — the -/// queue's `dispatch` already routes by `QueueKind`, but the approval -/// kind is the authoritative source of truth and a mismatch points -/// at a deeper bug we'd want to surface. -fn fetch_approval_for_worker( - coord: &Coordinator, - approval_id: i64, - expected_kind: ApprovalKind, -) -> Result { - let approval = coord - .approvals - .get(approval_id) - .map_err(|e| anyhow::anyhow!("read approval {approval_id}: {e:#}"))? - .ok_or_else(|| anyhow::anyhow!("approval {approval_id} no longer exists"))?; - if approval.kind != expected_kind { - bail!( - "approval {approval_id} kind mismatch: queue expected {expected_kind:?}, row is {actual:?}", - actual = approval.kind - ); - } - Ok(approval) -} - -async fn finish_approval( +fn finish_approval( coord: &Coordinator, approval: &hive_sh4re::approvals::Approval, result: Result<()>, - terminal_tag: Option, ) -> Result<()> { let (status, note, ok) = match &result { Ok(()) => (ApprovalStatus::Approved, None, true), @@ -683,8 +257,6 @@ async fn finish_approval( commit_ref: approval.commit_ref.clone(), status, note: note.clone(), - sha: approval.fetched_sha.clone(), - tag: terminal_tag.clone(), }, ); // Phase 5b: also fire on the dashboard event channel so the @@ -693,88 +265,18 @@ async fn finish_approval( // approval's logged resolved_at indirectly via `Utc::now()`; // failures already wrote it via mark_failed above. let approval_kind = <&str>::from(approval.kind); - let sha_short = approval - .fetched_sha - .as_deref() - .map(|s| s[..s.len().min(12)].to_owned()); let status_str = if ok { "approved" } else { "failed" }; coord.emit_approval_resolved(crate::coordinator::ApprovalResolved { id: approval.id, agent: approval.agent.as_str(), approval_kind, - sha_short, status: status_str, note: note.clone(), description: approval.description.clone(), }); - // For rebuild approvals, also surface the underlying action so the - // manager knows whether the lifecycle step succeeded. The - // ApprovalResolved event already carries the same `ok` signal but - // separating it lets the manager react to the lifecycle change - // without having to special-case approvals. - match approval.kind { - // MergeConfigPr ends in a container rebuild — surface a Rebuilt - // lifecycle event. - ApprovalKind::MergeConfigPr => { - let summary = crate::coordinator::rebuilt_todo_summary( - approval.agent.as_str(), - ok, - note.as_deref(), - approval.fetched_sha.as_deref(), - terminal_tag.as_deref(), - ); - let _ = coord - .push_todo_submitter( - approval.id, - "core", - Some(format!("rebuilt:{}", approval.agent)), - summary, - None, - ) - .await; - } - // UpdateMetaInputs / SchedulePrompt: ApprovalResolved already - // carries the result. No separate lifecycle event needed. - ApprovalKind::UpdateMetaInputs | ApprovalKind::SchedulePrompt => {} - } result } -/// Open the deploy for the config-PR flow: fast-forward `applied/main` to -/// `target`, sync the working tree, and run phase 1 of the meta two-phase -/// deploy. The container rebuild that used to run inline here is now the -/// subgraph [`run_deploy_apply`]'s node grows into the DAG, and the closing half -/// is [`run_finalize_deploy`]. -/// -/// **Undo is not this function's job.** Every early return here leaves the -/// applied repo dirty on purpose — [`run_deploy_tail`] owns compensation, and -/// it runs whether this returns `Err`, panics, or never returns at all because -/// hive-c0re was restarted underneath it. That's the whole point of parking the -/// pre-merge sha in a git ref instead of a local variable. -/// -/// Caller-specific bits stay OUT of here: fetching the PR head, the -/// `verify_commit` gate, and the ff-merge. The agent always already exists here -/// (a merge is never a first deploy), so there's no `sync_agents` step — the -/// first-deploy DAG's `Provision` node owns first-time meta registration. -async fn prepare_applied_target( - agent: &str, - applied_dir: &std::path::Path, - target: &str, - expected_main: &str, - node_id: Option, -) -> Result<()> { - // Meta input pins `?ref=main`, so this is what makes nix re-lock to the - // target commit on the prepare_deploy step below. - ff_applied_main(applied_dir, target, expected_main).await?; - - // Phase 1 of the meta two-phase deploy: relock without committing. The - // staged lock then stays uncommitted across the whole appended rebuild — - // which is why the `MetaWindow` is held by the deploy root, not by a node. - crate::meta::prepare_deploy(agent, node_id) - .await - .map_err(|e| anyhow::anyhow!("meta prepare_deploy: {e:#}")) -} - /// Fast-forward `applied/main` to `target` and sync the working tree. /// /// Compare-and-swap, not a bare set: `main` must still be `expected_main`, the @@ -798,46 +300,6 @@ async fn ff_applied_main( .map_err(|e| anyhow::anyhow!("read-tree to main: {e:#}")) } -/// `FinalizeDeploy` node body — phase 2 of the meta two-phase deploy, run once -/// the appended rebuild subgraph has built, swapped, and brought the container -/// back up. -/// -/// Drops the rollback ref *first*: from here the deploy is good and -/// [`run_deploy_tail`] must not roll `main` back. Ordering that ahead of the tag -/// plant is what makes the tail's `deployed/` cross-check a second line of -/// defence rather than the only one. No agent kick — the rebuild's own -/// `RebuildBookkeeping` already did it. -/// -/// # Errors -/// -/// Returns an error if the approval can't be loaded, if dropping the rollback -/// ref fails, or if planting the `deployed/` tag fails. Those two git writes -/// *are* the deploy's "confirmed good" signal, so warning past them would let -/// this node report success while leaving the tail looking at the git state of a -/// failure — and the tail would then compensate a good deploy. A failing -/// `meta::finalize_deploy` is deliberately *not* an error: the container is -/// already running the new config by then, and the staged `flake.lock` it -/// couldn't commit is something the operator can land by hand. -pub async fn run_finalize_deploy(coord: &Arc, approval_id: i64) -> Result<()> { - let ctx = deploy_ctx(coord, approval_id)?; - let agent = ctx.approval.agent.as_str(); - let target = ctx.reviewed.as_str(); - - lifecycle::git_delete_ref(&ctx.applied_dir, &rollback_ref(approval_id)) - .await - .map_err(|e| anyhow::anyhow!("drop rollback ref for approval {approval_id}: {e:#}"))?; - - let tag = format!("deployed/{approval_id}"); - lifecycle::git_tag(&ctx.applied_dir, &tag, target) - .await - .map_err(|e| anyhow::anyhow!("plant {tag}: {e:#}"))?; - - if let Err(e) = crate::meta::finalize_deploy(agent, target, &tag).await { - tracing::warn!(%agent, approval_id, error = ?e, "meta finalize_deploy failed"); - } - Ok(()) -} - /// Tear down a sub-agent container. By default this is non-destructive to /// persistent state: the proposed/applied config repos and the Claude /// credentials dir under `/var/lib/hyperhive/{agents,applied}//` are @@ -871,9 +333,7 @@ pub fn deny(coord: &Coordinator, id: i64, note: Option<&str>) -> Result<()> { coord.approvals.mark_denied(id, note)?; tracing::info!(%id, note, "approval denied"); if let Some(a) = approval { - let sha = a.fetched_sha.clone(); let approval_kind = <&str>::from(a.kind); - let sha_short = sha.as_deref().map(|s| s[..s.len().min(12)].to_owned()); let description = a.description.clone(); let agent_owned = a.agent.clone(); coord.notify_submitter( @@ -884,16 +344,12 @@ pub fn deny(coord: &Coordinator, id: i64, note: Option<&str>) -> Result<()> { commit_ref: a.commit_ref, status: ApprovalStatus::Denied, note: note.map(String::from), - sha, - // A denied config PR carries no git tag — it stays open on the forge. - tag: None, }, ); coord.emit_approval_resolved(crate::coordinator::ApprovalResolved { id, agent: agent_owned.as_str(), approval_kind, - sha_short, status: "denied", note: note.map(String::from), description, diff --git a/hive-c0re/src/coordinator.rs b/hive-c0re/src/coordinator.rs index e1417de3..5f03fa59 100644 --- a/hive-c0re/src/coordinator.rs +++ b/hive-c0re/src/coordinator.rs @@ -421,7 +421,6 @@ pub struct ApprovalResolved<'a> { pub id: i64, pub agent: &'a str, pub approval_kind: &'static str, - pub sha_short: Option, pub status: &'static str, pub note: Option, pub description: Option, @@ -430,14 +429,12 @@ pub struct ApprovalResolved<'a> { /// Field-named payload for [`Coordinator::emit_approval_added`]. /// Mirrors the `ApprovalAdded` dashboard-event fields. `agent` /// borrows from the caller; `approval_kind` is a compile-time -/// constant. `pr_number` is set for `merge_config_pr` only. +/// constant. pub struct ApprovalAdded<'a> { pub id: i64, pub agent: &'a str, pub approval_kind: &'static str, - pub sha_short: Option, pub description: Option, - pub pr_number: Option, } /// The two ways [`Coordinator::set_paused_by_name`] can fail. See its own @@ -782,18 +779,14 @@ impl Coordinator { id, agent, approval_kind, - sha_short, description, - pr_number, } = ev; self.emit_dashboard_event(DashboardEvent::ApprovalAdded { seq: self.next_seq(), id, agent: agent.to_owned(), approval_kind, - sha_short, description, - pr_number, }); } @@ -811,7 +804,6 @@ impl Coordinator { id, agent, approval_kind, - sha_short, status, note, description, @@ -826,7 +818,6 @@ impl Coordinator { id, agent: agent.to_owned(), approval_kind, - sha_short, status, resolved_at: hive_sh4re::wire_time::from_secs(resolved_at), note, @@ -1256,24 +1247,6 @@ impl Coordinator { } } - /// `push_todo` to whichever agent submitted approval `approval_id` — - /// same resolution `notify_submitter` uses. Every caller here is a - /// one-shot approval-resolution notice, so `reopen_if_acked` is - /// unconditionally `false` — there's no reconciler/event distinction - /// to make for an event that only ever fires once. - pub async fn push_todo_submitter( - &self, - approval_id: i64, - subsystem: &str, - key: Option, - summary: String, - source: Option, - ) -> Result<(), String> { - let target = self.submitter_or_manager(approval_id); - self.push_todo(&target, subsystem, key, summary, source, false) - .await - } - /// Push a `HelperEvent` into an arbitrary agent's inbox. Encoded /// the same way as `notify_manager` (sender = `SYSTEM_SENDER`, /// body = JSON-encoded event) — e.g. `ContainerCrash`, `NeedsUpdate`. @@ -1476,11 +1449,9 @@ impl Coordinator { } } -/// `push_todo`/`push_todo_submitter` summary text for a rebuild outcome — -/// shared by the two `Rebuilt` call sites (`actions::finish_approval`'s -/// `MergeConfigPr` arm, `job_queue::exec::run_emit_rebuilt`) so the wording -/// stays identical regardless of which path fired. Pure + independently -/// testable, unlike the old `HelperEvent::Rebuilt`'s separate `sha`/`tag` +/// `push_todo` summary text for a rebuild outcome, for +/// `job_queue::exec::run_emit_rebuilt`. Pure + independently testable, unlike +/// the old `HelperEvent::Rebuilt`'s separate `sha`/`tag` /// fields — those become part of the summary text itself now, since a todo /// carries one string, not a structured payload. #[must_use] diff --git a/hive-c0re/src/dashboard/approvals.rs b/hive-c0re/src/dashboard/approvals.rs index 2d6f5d85..6e0a8865 100644 --- a/hive-c0re/src/dashboard/approvals.rs +++ b/hive-c0re/src/dashboard/approvals.rs @@ -89,15 +89,10 @@ pub(super) fn gc_orphans(coord: &Coordinator, approvals: Vec) -> Vec::from(a.kind), - sha_short, status: "failed", note: Some(note.to_owned()), description: a.description.clone(), diff --git a/hive-c0re/src/dashboard/mod.rs b/hive-c0re/src/dashboard/mod.rs index 8cf05905..1b146b17 100644 --- a/hive-c0re/src/dashboard/mod.rs +++ b/hive-c0re/src/dashboard/mod.rs @@ -45,7 +45,6 @@ use crate::lifecycle; (name = "state_files", description = "proxied reads of allow-listed per-agent state files"), (name = "state_snapshot", description = "cold-load dashboard snapshot"), (name = "tombstones", description = "purge of retained state for destroyed agents"), - (name = "webhook", description = "forgejo webhook receivers"), ) )] struct ApiDoc; @@ -67,7 +66,6 @@ mod schedules; mod state_files; mod state_snapshot; mod tombstones; -mod webhook; // Run after lock bumps by the job queue (`job_queue/exec.rs`); the view // type feeds `DashboardEvent::MetaInputsChanged` (`dashboard_events.rs`). @@ -87,11 +85,6 @@ pub(crate) use tombstones::emit_tombstones_snapshot; #[derive(Clone)] struct AppState { coord: Arc, - /// HMAC-SHA256 secret shared with Forgejo webhook registrations. - /// Verified on every incoming `/webhook/*` POST. - /// `None` when the secret could not be loaded at startup — all - /// `/webhook/*` requests are rejected with 503 in that case. - webhook_secret: Option, } #[allow( @@ -101,11 +94,7 @@ struct AppState { handler; splitting that exhaustive list across helpers would \ obscure the route map for no readability gain" )] -pub async fn serve( - port: u16, - coord: Arc, - webhook_secret: Option, -) -> Result<()> { +pub async fn serve(port: u16, coord: Arc) -> Result<()> { // API-only: the gateway static-serves the dashboard dist and proxies // non-static requests here (see `nix/host-modules/hive-gateway/vhosts.nix`). // Unmatched paths 404. @@ -162,7 +151,6 @@ pub async fn serve( .routes(routes!(schedules::post_schedule_resume)) .routes(routes!(schedules::post_schedule_fire_now)) .routes(routes!(schedules::post_rebuild_queue_cancel)) - .routes(routes!(webhook::post_webhook_config_pr)) .routes(routes!(approvals::post_approve)) .routes(routes!(approvals::post_deny)) .routes(routes!(lifecycle_ops::post_destroy)) @@ -191,10 +179,7 @@ pub async fn serve( "/api/openapi.json", get(move || async move { Json(api.clone()) }), ) - .with_state(AppState { - coord, - webhook_secret, - }); + .with_state(AppState { coord }); // Binds loopback-only; external access via gateway. // Rationale: docs/networking/gateway.md::Firewall posture. let addr = SocketAddr::from(([127, 0, 0, 1], port)); @@ -392,7 +377,6 @@ mod router_build_probe { .routes(routes!(schedules::post_schedule_resume)) .routes(routes!(schedules::post_schedule_fire_now)) .routes(routes!(schedules::post_rebuild_queue_cancel)) - .routes(routes!(webhook::post_webhook_config_pr)) .routes(routes!(approvals::post_approve)) .routes(routes!(approvals::post_deny)) .routes(routes!(lifecycle_ops::post_destroy)) diff --git a/hive-c0re/src/dashboard/state_snapshot.rs b/hive-c0re/src/dashboard/state_snapshot.rs index 8f4db3ca..2b46bad0 100644 --- a/hive-c0re/src/dashboard/state_snapshot.rs +++ b/hive-c0re/src/dashboard/state_snapshot.rs @@ -131,8 +131,7 @@ struct ApprovalHistoryView { id: i64, agent: String, kind: &'static str, - /// First 12 chars of the canonical sha (preferred) or - /// manager-supplied ref. None when the approval carries neither. + /// First 12 chars of the manager-supplied ref. None when it is empty. sha_short: Option, /// `approved` / `denied` / `failed`. status: &'static str, @@ -149,25 +148,14 @@ struct ApprovalView { id: i64, agent: String, kind: &'static str, - /// First 12 chars of the reviewed PR head sha, for `MergeConfigPr` - /// only. Display-only (the short chip on the card). - sha_short: Option, /// Manager-supplied description shown on the approval card. #[serde(skip_serializing_if = "Option::is_none")] description: Option, - /// Forge PR number, for `MergeConfigPr` only. Lets the frontend - /// build a "review PR on forge" link - /// (`{forgeBase}/agent-configs/{agent}/pulls/{pr_number}`). `None` - /// for every other kind. - #[serde(skip_serializing_if = "Option::is_none")] - pr_number: Option, /// Raw `commit_ref` payload for `UpdateMetaInputs` (JSON-encoded /// `Vec` of input names; `"[]"` = all inputs) and /// `SchedulePrompt` (JSON-encoded `SchedulePromptPayload`). The /// frontend parses this to render a human-readable card body. - /// `None` for every other kind. - #[serde(skip_serializing_if = "Option::is_none")] - commit_ref: Option, + commit_ref: String, /// RFC 3339 UTC time the approval was queued. Rendered as a /// relative time on the card so the operator can spot a stale /// request. @@ -388,13 +376,11 @@ fn build_transient_views( out } -/// Render each pending approval into its dashboard view (short sha for -/// `MergeConfigPr`). /// Project a resolved sqlite row into the lean shape the dashboard /// history tab consumes — no `diff_html` (rendering 30 of them /// per /api/state poll would mean 30 git diffs per refresh). fn history_view(a: Approval) -> ApprovalHistoryView { - let displayed = a.fetched_sha.as_deref().unwrap_or(&a.commit_ref); + let displayed = a.commit_ref.as_str(); let sha_short = if displayed.is_empty() { None } else { @@ -421,54 +407,17 @@ fn history_view(a: Approval) -> ApprovalHistoryView { } fn build_approval_views(approvals: Vec) -> Vec { - let mut out = Vec::with_capacity(approvals.len()); - for a in approvals { - out.push(match a.kind { - hive_sh4re::approvals::ApprovalKind::UpdateMetaInputs => ApprovalView { - id: a.id, - agent: a.agent.to_string(), - kind: "update_meta_inputs", - sha_short: None, - description: a.description, - pr_number: None, - commit_ref: Some(a.commit_ref), - requested_at: a.requested_at, - }, - hive_sh4re::approvals::ApprovalKind::SchedulePrompt => ApprovalView { - id: a.id, - agent: a.agent.to_string(), - kind: "schedule_prompt", - sha_short: None, - description: a.description, - pr_number: None, - commit_ref: Some(a.commit_ref), - requested_at: a.requested_at, - }, - hive_sh4re::approvals::ApprovalKind::MergeConfigPr => { - // commit_ref = PR number; fetched_sha = the reviewed PR - // head. Show the head sha; the config diff surface lives - // on the forge PR itself. - let sha = a - .fetched_sha - .as_deref() - .map(|s| s[..s.len().min(12)].to_owned()); - // Surface the PR number so the frontend can link to the - // PR on the forge. commit_ref holds the number as text. - let pr_number = a.commit_ref.parse::().ok(); - ApprovalView { - id: a.id, - agent: a.agent.to_string(), - kind: "merge_config_pr", - sha_short: sha, - description: a.description, - pr_number, - commit_ref: None, - requested_at: a.requested_at, - } - } - }); - } - out + approvals + .into_iter() + .map(|a| ApprovalView { + id: a.id, + agent: a.agent.to_string(), + kind: <&str>::from(a.kind), + description: a.description, + commit_ref: a.commit_ref, + requested_at: a.requested_at, + }) + .collect() } /// `/api/jobq/graph` query string — the generic jobq/wire query shape (any diff --git a/hive-c0re/src/dashboard/webhook.rs b/hive-c0re/src/dashboard/webhook.rs deleted file mode 100644 index 44842651..00000000 --- a/hive-c0re/src/dashboard/webhook.rs +++ /dev/null @@ -1,362 +0,0 @@ -//! Forgejo webhook endpoints. -//! -//! - **`/webhook/config-pr`** — `pull_request` events on any `agent-configs/*` -//! repo queue a [`hive_sh4re::approvals::ApprovalKind::MergeConfigPr`] approval row -//! so the operator can review + approve the merge from the dashboard. -//! -//! There was a second endpoint here, `/webhook/knowledge`, which pulled the -//! local `internal/knowledge` clone on push. It is gone along with the -//! per-hive registration that fed it: a webhook has exactly one target URL, -//! so every hive registering one against the shared repository was -//! last-writer-wins. The swarm controller now holds the single registration -//! and addresses an event to each hive over the queue, which -//! [`crate::workers::knowledge`] documents. -//! -//! The endpoint is reached via the gateway (HTTPS, public domain URL) so -//! Forgejo's SSRF guard does not block delivery. Each delivery is verified -//! against the `X-Hub-Signature-256` HMAC header Forgejo attaches; the -//! shared secret is auto-generated at startup and persisted to -//! [`crate::paths::webhook_secret_file()`]. - -use axum::{ - body::Bytes, - extract::State, - http::{HeaderMap, StatusCode}, - response::{IntoResponse, Response}, -}; -use serde::Deserialize; - -use super::AppState; - -// ── HMAC helper ─────────────────────────────────────────────────────────────── - -/// Verify the `X-Hub-Signature-256` header on an incoming Forgejo webhook. -/// Returns `Err` (with a safe-to-log message) on mismatch, missing header, -/// or when the HMAC secret is unavailable (load failure at startup). -fn verify_hmac(state: &AppState, headers: &HeaderMap, body: &Bytes) -> Result<(), String> { - let secret = state - .webhook_secret - .as_deref() - .ok_or_else(|| "webhook HMAC secret unavailable; endpoint disabled".to_owned())?; - let sig = headers - .get("x-hub-signature-256") - .and_then(|v| v.to_str().ok()) - .unwrap_or(""); - if sig.is_empty() { - return Err("missing X-Hub-Signature-256 header".to_owned()); - } - crate::webhook_secret::verify_signature(secret, body, sig).map_err(|e| e.to_string()) -} - -// ── config-PR webhook ────────────────────────────────────────────────────────── - -/// Minimal Forgejo `pull_request`-webhook payload. -/// -/// Forgejo fires this for actions: `opened`, `closed`, `reopened`, -/// `synchronized`, `assigned`, `unassigned`, `label_updated`, -/// `label_cleared`, `milestoned`, `demilestoned`, `review_requested`, -/// `review_request_removed`, `auto_merge_enabled`, `auto_merge_disabled`. -/// We only act on `opened` and `synchronized` (Forgejo spells the -/// push-update action past-tense, unlike GitHub's `synchronize`). -#[derive(Deserialize)] -pub(super) struct PrWebhookPayload { - /// What triggered this event (`opened`, `closed`, `synchronized`, …). - action: Option, - /// PR index on the repo. - number: Option, - pull_request: Option, - repository: Option, -} - -#[derive(Deserialize)] -struct PrWebhookPr { - head: Option, -} - -#[derive(Deserialize)] -struct PrWebhookHead { - sha: Option, -} - -#[derive(Deserialize)] -struct PrWebhookRepo { - full_name: Option, -} - -/// POST `/webhook/config-pr` — Forgejo `pull_request` webhook for -/// `agent-configs/*` repos. -/// -/// On `opened` or `synchronized` for an `agent-configs/` PR: -/// fetches the current PR head sha, queues a `MergeConfigPr` approval row, -/// and emits the `ApprovalAdded` event so the dashboard card appears -/// immediately. -/// -/// All other actions (closed, label changes, etc.) are silently ignored — -/// the operator can deny a pending approval if the PR is later closed. -/// -/// Always returns HTTP 200 (even on queue failure) so Forgejo does not -/// retry the delivery. Failures are logged at `warn` level. -/// -/// Expected Forgejo webhook configuration: -/// - URL: `https:///webhook/config-pr` -/// - Content type: `application/json` -/// - Events: "Pull Request" only -/// - Secret: auto-generated HMAC key (see [`crate::webhook_secret`]) -/// - Organisation: `agent-configs` (org-level hook covers all config repos) -/// -/// hive-c0re registers this hook automatically at startup via -/// [`crate::forge::ensure_config_pr_webhook`]. -#[utoipa::path( - post, - path = "/webhook/config-pr", - request_body( - content = String, - content_type = "application/json", - description = "Forgejo pull_request-webhook payload, taken as raw \ - bytes (not a typed extractor) so HMAC verification \ - runs over the exact wire bytes before any JSON \ - parsing" - ), - responses( - (status = 200, description = "processed (approval queued or ignored)", body = String), - (status = 400, description = "invalid JSON payload"), - (status = 401, description = "bad or missing HMAC signature"), - (status = 503, description = "HMAC secret unavailable at startup"), - ), - tag = "webhook" -)] -pub(super) async fn post_webhook_config_pr( - State(state): State, - headers: HeaderMap, - body: Bytes, -) -> Response { - if let Err(e) = verify_hmac(&state, &headers, &body) { - tracing::warn!("webhook/config-pr: HMAC verification failed: {e}"); - let status = if e.contains("unavailable") { - StatusCode::SERVICE_UNAVAILABLE - } else { - StatusCode::UNAUTHORIZED - }; - return (status, e).into_response(); - } - - let payload = match serde_json::from_slice::(&body) { - Ok(p) => p, - Err(e) => { - tracing::warn!("webhook/config-pr: JSON parse error: {e}"); - return (StatusCode::BAD_REQUEST, "invalid JSON").into_response(); - } - }; - - let action = payload.action.as_deref().unwrap_or(""); - // Only act on a newly-opened PR or a new push to its branch. Forgejo's - // webhook payload spells the push-update action `synchronized` (past - // tense), NOT GitHub's `synchronize` — matching only the GitHub spelling - // silently dropped every PR-update delivery (the whole reason updates were - // "only found by poll"). Accept both so the handler is correct against - // Forgejo and stays GitHub-compatible. - if action != "opened" && action != "synchronize" && action != "synchronized" { - tracing::debug!(action, "webhook/config-pr: ignoring action"); - return (StatusCode::OK, "ignored").into_response(); - } - - let full_name = payload - .repository - .as_ref() - .and_then(|r| r.full_name.as_deref()) - .unwrap_or(""); - - // Expect `agent-configs/`. - let agent = match full_name.strip_prefix(&format!("{}/", crate::forge::CONFIG_ORG)) { - Some(name) if !name.is_empty() && !name.contains('/') => name, - _ => { - tracing::debug!( - full_name, - "webhook/config-pr: ignoring non-config-repo event" - ); - return (StatusCode::OK, "ignored").into_response(); - } - }; - - let pr_number = match payload.number { - Some(n) if n > 0 => n, - _ => { - tracing::warn!(full_name, "webhook/config-pr: missing or zero PR number"); - return (StatusCode::OK, "ignored").into_response(); - } - }; - - // The payload already carries the head sha — use it as an early hint for - // logging, but the canonical sha comes from `submit_merge_config_pr`'s - // fresh forge API call so we don't trust a potentially-stale payload sha. - let payload_sha = payload - .pull_request - .as_ref() - .and_then(|pr| pr.head.as_ref()) - .and_then(|h| h.sha.as_deref()) - .unwrap_or(""); - - tracing::info!( - %full_name, %agent, %pr_number, %payload_sha, %action, - "webhook/config-pr: queuing MergeConfigPr approval" - ); - - // Queue the approval. The description surfaces the action and PR number - // on the dashboard card so the operator has context without opening the - // forge PR. - let description = format!("PR #{pr_number} on {full_name} ({action})"); - if let Err(e) = crate::socket_server::submit_merge_config_pr( - &state.coord, - agent, - pr_number, - Some(&description), - "forge", // submitter — identifies the webhook path in the audit trail - ) - .await - { - tracing::warn!( - %agent, %pr_number, error = ?e, - "webhook/config-pr: failed to queue MergeConfigPr approval" - ); - } - - (StatusCode::OK, "ok").into_response() -} - -#[cfg(test)] -mod tests { - use axum::{ - body::Bytes, - extract::State, - http::{HeaderMap, HeaderValue, StatusCode}, - }; - - use super::{AppState, post_webhook_config_pr, verify_hmac}; - - const SECRET: &str = "s3cr3t"; - - /// The `TempDir` holds the coordinator's sqlite files; keep it alive as - /// long as the state. - fn state(webhook_secret: Option<&str>) -> (tempfile::TempDir, AppState) { - let (dir, coord) = crate::socket_server::coordinator(); - let state = AppState { - coord, - webhook_secret: webhook_secret.map(str::to_owned), - }; - (dir, state) - } - - /// The `X-Hub-Signature-256` header Forgejo would send for `secret` + `body`. - fn signed(secret: &str, body: &[u8]) -> HeaderMap { - use std::fmt::Write as _; - - use hmac::{Hmac, KeyInit, Mac}; - use sha2::Sha256; - let mut mac = Hmac::::new_from_slice(secret.as_bytes()).unwrap(); - mac.update(body); - let mut hex = String::new(); - for b in mac.finalize().into_bytes() { - write!(hex, "{b:02x}").unwrap(); - } - let mut headers = HeaderMap::new(); - headers.insert( - "x-hub-signature-256", - HeaderValue::from_str(&format!("sha256={hex}")).unwrap(), - ); - headers - } - - /// With no secret loaded, nothing verifies — including a delivery signed - /// with the empty key. The message must say "unavailable": the handler - /// maps on that word to 503. - #[test] - fn verify_hmac_rejects_every_delivery_when_no_secret_is_loaded() { - let (_dir, state) = state(None); - let body = Bytes::from_static(b"{}"); - for (label, headers) in [ - ("signed with the empty key", signed("", &body)), - ("signed with some key", signed(SECRET, &body)), - ("unsigned", HeaderMap::new()), - ] { - let err = verify_hmac(&state, &headers, &body).expect_err(label); - assert!(err.contains("unavailable"), "{label}: {err}"); - } - } - - /// An absent header, an empty one and one that is not valid UTF-8 all - /// read as "no signature", and are refused before any HMAC is computed. - #[test] - fn verify_hmac_rejects_a_delivery_with_no_usable_signature_header() { - let (_dir, state) = state(Some(SECRET)); - let body = Bytes::from_static(b"{}"); - let mut empty = HeaderMap::new(); - empty.insert("x-hub-signature-256", HeaderValue::from_static("")); - let mut not_utf8 = HeaderMap::new(); - not_utf8.insert( - "x-hub-signature-256", - HeaderValue::from_bytes(b"sha256=\xff").unwrap(), - ); - for (label, headers) in [ - ("absent", HeaderMap::new()), - ("empty", empty), - ("not UTF-8", not_utf8), - ] { - assert_eq!( - verify_hmac(&state, &headers, &body), - Err("missing X-Hub-Signature-256 header".to_owned()), - "{label}" - ); - } - } - - /// The other side of both guards: a loaded secret and a matching - /// signature pass. - #[test] - fn verify_hmac_accepts_a_correctly_signed_delivery() { - let (_dir, state) = state(Some(SECRET)); - let body = Bytes::from_static(b"{}"); - assert_eq!(verify_hmac(&state, &signed(SECRET, &body), &body), Ok(())); - } - - /// 503 means "this hive cannot verify any delivery", 401 means "this - /// delivery is not authentic". - #[tokio::test] - async fn config_pr_answers_503_when_no_secret_is_loaded() { - let (_dir, state) = state(None); - let body = Bytes::from_static(b"{}"); - let headers = signed(SECRET, &body); - let resp = post_webhook_config_pr(State(state), headers, body).await; - assert_eq!(resp.status(), StatusCode::SERVICE_UNAVAILABLE); - } - - #[tokio::test] - async fn config_pr_answers_401_for_a_missing_or_wrong_signature() { - let body = Bytes::from_static(b"{}"); - for (label, headers) in [ - ("missing", HeaderMap::new()), - ("wrong secret", signed("different-secret", &body)), - ("other body", signed(SECRET, b"tampered")), - ] { - let (_dir, state) = state(Some(SECRET)); - let resp = post_webhook_config_pr(State(state), headers, body.clone()).await; - assert_eq!(resp.status(), StatusCode::UNAUTHORIZED, "{label}"); - } - } - - /// A verified delivery reaches payload handling: an action the handler - /// ignores comes back 200, and an unparseable body 400, neither of which - /// is an auth status. - #[tokio::test] - async fn config_pr_lets_a_correctly_signed_delivery_through() { - for (body, expected) in [ - (&br#"{"action":"closed"}"#[..], StatusCode::OK), - (&b"not json"[..], StatusCode::BAD_REQUEST), - ] { - let (_dir, state) = state(Some(SECRET)); - let body = Bytes::from_static(body); - let headers = signed(SECRET, &body); - let resp = post_webhook_config_pr(State(state), headers, body).await; - assert_eq!(resp.status(), expected); - } - } -} diff --git a/hive-c0re/src/dashboard_events.rs b/hive-c0re/src/dashboard_events.rs index e856e890..7af7d384 100644 --- a/hive-c0re/src/dashboard_events.rs +++ b/hive-c0re/src/dashboard_events.rs @@ -52,7 +52,7 @@ pub enum DashboardEvent { /// enough to render the dashboard row without a `/api/state` /// refetch. /// - /// The approval's own kind (`"merge_config_pr"` / `"schedule_prompt"`) lives + /// The approval's own kind (`"schedule_prompt"` / `"update_meta_inputs"`) lives /// on `approval_kind` rather than `kind` because the latter is taken /// by the serde tag identifying which `DashboardEvent` variant /// this is. @@ -61,15 +61,7 @@ pub enum DashboardEvent { id: i64, agent: String, approval_kind: &'static str, - sha_short: Option, description: Option, - /// Forge PR number, for `merge_config_pr` approvals only — lets - /// the live `applyApprovalAdded` path build the "review PR on - /// forge" link without waiting for a cold `/api/state` refresh - /// (mirrors `ApprovalView::pr_number`). `None` for every other - /// kind. - #[serde(skip_serializing_if = "Option::is_none")] - pr_number: Option, }, /// A pending approval transitioned to a terminal state /// (approved / denied / failed). Clients move the row out of the @@ -79,7 +71,6 @@ pub enum DashboardEvent { id: i64, agent: String, approval_kind: &'static str, - sha_short: Option, /// `"approved"` / `"denied"` / `"failed"`. status: &'static str, resolved_at: DateTime, @@ -307,17 +298,14 @@ mod tests { seq: 1, id: 1, agent: "x".into(), - approval_kind: "merge_config_pr", - sha_short: None, + approval_kind: "schedule_prompt", description: None, - pr_number: None, }, DashboardEvent::ApprovalResolved { seq: 1, id: 1, agent: "x".into(), - approval_kind: "merge_config_pr", - sha_short: None, + approval_kind: "schedule_prompt", status: "approved", resolved_at: hive_sh4re::wire_time::from_secs(0), note: None, diff --git a/hive-c0re/src/forge/ci_runner.rs b/hive-c0re/src/forge/ci_runner.rs index f7eef7fb..a16a82fb 100644 --- a/hive-c0re/src/forge/ci_runner.rs +++ b/hive-c0re/src/forge/ci_runner.rs @@ -128,8 +128,7 @@ fn runner_address_matches(raw: &str, configured: &str) -> bool { /// Bound on reaching the forge. const HTTP_CONNECT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5); -/// Bound on one whole forge admin call, body included; the same budget -/// `config_pr_poll` gives this forge. +/// Bound on one whole forge admin call, body included. const HTTP_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(15); /// A client with both forge bounds applied. diff --git a/hive-c0re/src/forge/config_pr_poll.rs b/hive-c0re/src/forge/config_pr_poll.rs deleted file mode 100644 index 5cb76fce..00000000 --- a/hive-c0re/src/forge/config_pr_poll.rs +++ /dev/null @@ -1,189 +0,0 @@ -//! Polling fallback for the config-PR webhook. -//! -//! The webhook (`/webhook/config-pr`) is the primary path for detecting open -//! PRs on `agent-configs/*` repos and queuing `MergeConfigPr` approvals. -//! But webhooks can be missed — hive-c0re might be down when a PR is opened, -//! or Forgejo might fail a delivery. -//! -//! This module provides [`poll_open_config_prs`], called periodically from -//! `main.rs`, which scans all `agent-configs/*` repos for open PRs that have -//! no pending `MergeConfigPr` approval yet, and queues one. Idempotent: PRs -//! that already have a pending approval are skipped. - -use std::sync::Arc; - -use anyhow::Result; -use forgejo_api::structs::{RepoListPullRequestsQuery, RepoListPullRequestsQueryState}; - -use crate::coordinator::Coordinator; -use crate::forge::CONFIG_ORG; - -const HTTP_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(15); - -/// Scan every repo in `agent-configs` for open PRs that have no pending -/// `MergeConfigPr` approval yet, and queue one for each gap found. -/// -/// Designed to be called on a periodic timer (e.g. every 5 minutes) as a -/// fault-tolerance backstop for the Forgejo webhook. The webhook fires -/// immediately; this catches anything the webhook missed. -pub async fn poll_open_config_prs(core_token: &str, coord: &Arc) -> Result<()> { - let client = crate::forge::api(core_token)?; - - // List all repos in agent-configs org. - let repos = tokio::time::timeout(HTTP_TIMEOUT, client.org_list_repos(CONFIG_ORG).all()) - .await - .map_err(anyhow::Error::from) - .and_then(|r| r.map_err(anyhow::Error::from))?; - - // (agent, pr_number) of every OPEN config PR seen this sweep, plus the set - // of agents whose PR list was fetched successfully. The reconcile pass - // below uses these to cancel pending approvals whose PR is no longer open, - // without wrongly cancelling one when a repo's list failed transiently. - let mut open_prs: std::collections::HashSet<(String, u64)> = std::collections::HashSet::new(); - let mut scanned_agents: std::collections::HashSet = std::collections::HashSet::new(); - - for repo in repos { - let Some(repo_name) = repo.name.as_deref() else { - continue; - }; - // The repo name is the agent name (agent-configs/). - let agent = repo_name; - - let query = RepoListPullRequestsQuery { - state: Some(RepoListPullRequestsQueryState::Open), - sort: None, - milestone: None, - labels: None, - poster: None, - base: None, - head: None, - }; - - let prs = match tokio::time::timeout( - HTTP_TIMEOUT, - client - .repo_list_pull_requests(CONFIG_ORG, repo_name, query) - .all(), - ) - .await - { - Ok(Ok(prs)) => { - scanned_agents.insert(agent.to_owned()); - prs - } - Ok(Err(e)) => { - tracing::debug!( - %agent, error = %e, - "config-pr poll: listing PRs failed, skipping repo" - ); - continue; - } - Err(_) => { - tracing::debug!( - %agent, - "config-pr poll: timeout listing PRs, skipping repo" - ); - continue; - } - }; - - for pr in prs { - let Some(pr_number) = pr.number.and_then(|n| u64::try_from(n).ok()) else { - continue; - }; - open_prs.insert((agent.to_owned(), pr_number)); - - // `submit_merge_config_pr` is idempotent + PR-drift aware: it - // no-ops when an approval pinned to this PR's *current* head is - // already pending, and cancels+re-queues when the head has drifted. - // So the poll can call it unconditionally — it backstops both a - // missed `opened` webhook (no approval yet) and a missed - // `synchronize` (stale approval whose head moved). - let description = format!("PR #{pr_number} on {CONFIG_ORG}/{agent} (poll fallback)"); - if let Err(e) = crate::socket_server::submit_merge_config_pr( - coord, - agent, - pr_number, - Some(&description), - "poll", // submitter — identifies the polling path in the audit trail - ) - .await - { - tracing::warn!( - %agent, %pr_number, error = ?e, - "config-pr poll: failed to reconcile MergeConfigPr approval" - ); - } - } - } - - // Reconcile: cancel pending approvals whose PR is no longer open. - reconcile_stale_config_pr_approvals(coord, &open_prs, &scanned_agents); - - Ok(()) -} - -/// Cancel pending `MergeConfigPr` approvals whose PR is no longer open — merged -/// (incl. outside the approval flow) or closed. Without this the card lingers on -/// the dashboard forever, since the webhook only signals opened PRs and the -/// add-loop only ever queues. -/// -/// `open_prs` is the set of `(agent, pr_number)` seen open this sweep and -/// `scanned_agents` the agents whose PR list was fetched successfully — the -/// reconcile is guarded to those so a transient list failure can't cancel a -/// still-valid approval. -fn reconcile_stale_config_pr_approvals( - coord: &Arc, - open_prs: &std::collections::HashSet<(String, u64)>, - scanned_agents: &std::collections::HashSet, -) { - let pending = match coord.approvals.pending() { - Ok(p) => p, - Err(e) => { - tracing::warn!(error = ?e, "config-pr poll: pending() failed, skipping reconcile"); - return; - } - }; - for a in pending { - if a.kind != hive_sh4re::approvals::ApprovalKind::MergeConfigPr - || !scanned_agents.contains(a.agent.as_str()) - { - continue; - } - let Ok(pr_number) = a.commit_ref.parse::() else { - continue; - }; - if open_prs.contains(&(a.agent.to_string(), pr_number)) { - continue; - } - match coord - .approvals - .mark_cancelled(a.id, "config-pr poll (PR no longer open)") - { - Ok(_) => { - tracing::info!( - agent = %a.agent, pr_number, id = a.id, - "config-pr poll: cancelled stale MergeConfigPr approval (PR merged/closed)" - ); - coord.emit_approval_resolved(crate::coordinator::ApprovalResolved { - id: a.id, - agent: a.agent.as_str(), - approval_kind: "merge_config_pr", - sha_short: a - .fetched_sha - .as_deref() - .map(|s| s[..s.len().min(12)].to_owned()), - status: "cancelled", - note: Some("PR merged/closed outside the approval".to_owned()), - description: a.description.clone(), - }); - } - Err(e) => { - tracing::warn!( - agent = %a.agent, id = a.id, error = ?e, - "config-pr poll: failed to cancel stale MergeConfigPr approval" - ); - } - } - } -} diff --git a/hive-c0re/src/forge/mod.rs b/hive-c0re/src/forge/mod.rs index dd56c5ce..99dd19eb 100644 --- a/hive-c0re/src/forge/mod.rs +++ b/hive-c0re/src/forge/mod.rs @@ -5,16 +5,12 @@ //! No-op when `hive-forge` isn't running. Full design: `docs/integrations/forge.md`. mod ci_runner; -pub mod config_pr_poll; -mod pr_merge; +mod pr_comment; mod reconcile; mod repos; mod users; -pub use pr_merge::{ - ForgeMergeError, config_repo, fetch_pr_head_into_applied, merge_config_pr_ff, - merged_pr_for_commit, post_pr_comment, pr_head_sha, pr_is_open, -}; +pub use pr_comment::{config_repo, merged_pr_for_commit, post_pr_comment}; pub use reconcile::{fetch_forge_main, reconcile_config_apply, reconcile_config_status}; pub use repos::{ clone_config_into_proposed, ensure_config_repo, ensure_meta_remote, ensure_repo, @@ -92,15 +88,14 @@ pub(crate) fn core_auth_header(token: &str) -> String { } /// Forgejo org grouping every agent's config repo. Core is a site admin -/// and reads + writes every repo here. As of the agent-config-PR flow each -/// agent is a **write collaborator on its own** `agent-configs/` repo — -/// the editable PR surface it pushes config-change branches to — but `main` is -/// branch-protected core-only, so only hive-c0re's verify-and-ff-push merge -/// handler lands on it (operator approval required; the agent can't push -/// `main` or self-merge). The repos remain private, so an agent still can't -/// reach *another* agent's config. `main` is fast-forward-only — hive-c0re -/// never force-pushes; the `push_config` mirror runs best-effort until the -/// PR-merge flow retires it. +/// and reads + writes every repo here. Each agent is a **write collaborator +/// on its own** `agent-configs/` repo — the editable PR surface it +/// pushes config-change branches to. `main`'s branch protection is +/// swarm-controller's: merge and approval allowlisted to the `operators` team, +/// so the agent can't push `main` or self-merge, and an operator's merge +/// deploys through swarm-controller. The repos remain private, so an agent +/// still can't reach *another* agent's config. hive-c0re never force-pushes; +/// the `push_config` mirror runs best-effort. pub(crate) const CONFIG_ORG: &str = "agent-configs"; /// Forgejo org hosting the operator-curated shared docs/skills repo /// that every agent gets read-only access to. Agents use it as a @@ -403,183 +398,49 @@ pub async fn ensure_all() { } } -/// Ensure a Forgejo `pull_request` org-webhook for `agent-configs` targets -/// hive-c0re's `/webhook/config-pr` and signs with `webhook_secret`. A hook -/// already at that URL is kept if [`crate::webhook_secret::is_registered`], -/// else deleted and recreated (Forgejo's edit-hook API ignores `secret`). -/// -/// `hive_domain` is the public domain name of the hive; the webhook URL is -/// `https:///webhook/config-pr` (routed through the gateway, -/// avoiding the Forgejo SSRF guard that blocks loopback delivery). -/// -/// `webhook_secret` is the HMAC secret Forgejo will attach as -/// `X-Hub-Signature-256` on each delivery; hive-c0re verifies this header -/// in the dashboard webhook handler (`post_webhook_config_pr`). -/// -/// An org-level hook covers every repo in `agent-configs` automatically, -/// so no per-repo setup is needed as new agents are provisioned. -/// -/// Called at startup. The knowledge repo's one hook is the controller's -/// now; this one has not moved yet. No-op when the core token is absent -/// (forge not yet provisioned). -/// -/// # Errors -/// -/// Returns an error if: -/// - `hive_domain` produces a URL that `url::Url::parse` rejects. -/// - Deleting the hook being replaced, or `org_create_hook`, fails (transport -/// error, auth failure, or the `agent-configs` org does not exist). -/// - The HTTP call times out (10 s limit). -/// -/// Listing failures are treated as best-effort: they fall through to the -/// create attempt rather than surfacing an error. -pub async fn ensure_config_pr_webhook( - core_token: &str, - hive_domain: &str, - webhook_secret: &str, -) -> Result<()> { - use forgejo_api::structs::{CreateHookOption, CreateHookOptionConfig, CreateHookOptionType}; - use std::collections::BTreeMap; - - const HTTP_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); - - let target_url = format!("https://{hive_domain}/webhook/config-pr"); - let client = api(core_token)?; - - // List existing org hooks — skip creation if ours is already there. - // Best-effort: a listing failure falls through to the create attempt. - let listed = tokio::time::timeout(HTTP_TIMEOUT, client.org_list_hooks(CONFIG_ORG).send()) - .await - .map_err(anyhow::Error::from) - .and_then(|r| r.map_err(anyhow::Error::from)); - let listed_ok = listed.is_ok(); - match listed { - Ok(hooks) => { - let secret_registered = crate::webhook_secret::is_registered(webhook_secret); - if config_pr_hook_is_current(&hooks, &target_url, secret_registered) { - tracing::debug!(%target_url, "forge: config-pr webhook already configured"); - return Ok(()); - } - for h in &hooks { - let hook_url = hook_url(h); - let Some(id) = h.id else { continue }; - if hook_url == target_url { - // A failed delete must not fall through to create: the - // old hook would stay beside the new one, and a later - // pass sees the URL present and never removes it. - tracing::info!( - hook_url, - org = CONFIG_ORG, - "forge: replacing config-pr webhook (secret not the registered one)" - ); - tokio::time::timeout( - HTTP_TIMEOUT, - client.org_delete_hook(CONFIG_ORG, id).send(), - ) - .await - .map_err(anyhow::Error::from) - .and_then(|r| r.map_err(anyhow::Error::from)) - .with_context(|| { - format!("delete config-pr webhook {id} to replace its secret") - })?; - } else if hook_url.ends_with("/webhook/config-pr") { - // Our path on a different base (e.g. old loopback hooks - // from before the SSRF-bypass migration). - tracing::info!( - hook_url, - org = CONFIG_ORG, - "forge: deleting stale config-pr webhook (wrong base)" - ); - let _ = tokio::time::timeout( - HTTP_TIMEOUT, - client.org_delete_hook(CONFIG_ORG, id).send(), - ) - .await; - } - } - } - Err(e) => { - tracing::debug!(error = %e, "forge: listing config-pr hooks failed; attempting create"); - } - } - - let mut additional = BTreeMap::new(); - additional.insert("secret".to_owned(), webhook_secret.to_owned()); - - let hook = CreateHookOption { - active: Some(true), - authorization_header: None, - branch_filter: None, - config: CreateHookOptionConfig { - content_type: "json".to_owned(), - url: Url::parse(&target_url).context("parse config-pr webhook target url")?, - additional, - }, - events: Some(vec!["pull_request".to_owned()]), - r#type: CreateHookOptionType::Forgejo, - }; - tokio::time::timeout(HTTP_TIMEOUT, client.org_create_hook(CONFIG_ORG, hook)) - .await - .map_err(anyhow::Error::from) - .and_then(|r| r.map_err(anyhow::Error::from)) - .with_context(|| format!("create config-pr webhook on org {CONFIG_ORG}"))?; - tracing::info!(%target_url, "forge: config-pr webhook created on org {CONFIG_ORG}"); - // Only a listed pass knows no hook with an older secret survived beside - // this one; an unlisted pass leaves the record stale so the next - // registration lists and replaces again. - if listed_ok && let Err(e) = crate::webhook_secret::record_registered(webhook_secret) { - tracing::warn!(error = ?e, "forge: recording the config-pr webhook secret failed"); - } - Ok(()) -} - -/// The `url` a Forgejo hook delivers to, or `""` when it carries none. -fn hook_url(hook: &forgejo_api::structs::Hook) -> &str { - hook.config - .as_ref() - .and_then(|c| c.get("url")) - .map_or("", String::as_str) -} - -/// Whether the config-PR hook needs no work: one targets `target_url`, and -/// the current secret is the one it was registered with. -fn config_pr_hook_is_current( - hooks: &[forgejo_api::structs::Hook], - target_url: &str, - secret_registered: bool, -) -> bool { - secret_registered && hooks.iter().any(|h| hook_url(h) == target_url) -} - #[cfg(test)] mod tests { - use super::{config_pr_hook_is_current, describe_forge_admin}; + use super::{describe_forge_admin, git_url_with_base}; - const TARGET: &str = "https://hive.example/webhook/config-pr"; - - fn hook(url: &str) -> forgejo_api::structs::Hook { - serde_json::from_value(serde_json::json!({ "id": 1, "config": { "url": url } })) - .expect("hook json") + /// The remote carries **no credential**. `argv` is world-readable through + /// `/proc//cmdline`, so a token spliced in here would be published to + /// every local user for the life of the git child. + /// + /// Deliberately not via `forge_git_url`, which reads `HIVE_FORGE_URL` — + /// setting that here would race every other test in this binary. + #[test] + fn forge_git_url_carries_no_credential() { + let url = git_url_with_base("http://forge.example.test", "a/iris"); + assert_eq!(url, "http://forge.example.test/a/iris.git"); + assert!(!url.contains('@'), "no userinfo: {url}"); } #[test] - fn a_hook_at_the_target_with_the_registered_secret_is_kept() { - assert!(config_pr_hook_is_current(&[hook(TARGET)], TARGET, true)); + fn forge_git_url_preserves_https() { + // The scheme is carried through rather than assumed: a swarm + // whose forge is behind TLS must not be downgraded to http. + let url = git_url_with_base("https://forge.example.test", "a/iris"); + assert!(url.starts_with("https://"), "https must survive: {url}"); } - /// Forgejo can't be asked which secret a hook signs with, so a hook at - /// the right URL is not enough: without the matching record it is - /// replaced, or every delivery fails HMAC. + /// The credential goes in an `Authorization` header instead — decodable + /// back to `core:`, so the swap is genuinely equivalent auth and + /// not a silent downgrade to anonymous. #[test] - fn a_hook_at_the_target_with_a_changed_secret_is_replaced() { - assert!(!config_pr_hook_is_current(&[hook(TARGET)], TARGET, false)); - } - - #[test] - fn a_hook_on_another_base_is_not_ours() { - let stale = hook("http://127.0.0.1:7000/webhook/config-pr"); - assert!(!config_pr_hook_is_current(&[stale], TARGET, true)); - assert!(!config_pr_hook_is_current(&[], TARGET, true)); + fn core_auth_header_is_basic_core_token() { + use base64::Engine as _; + let header = super::core_auth_header("s3cret"); + let b64 = header + .strip_prefix("Authorization: Basic ") + .expect("basic auth header"); + let decoded = base64::engine::general_purpose::STANDARD + .decode(b64) + .expect("valid base64"); + assert_eq!(String::from_utf8_lossy(&decoded), "core:s3cret"); + assert!( + !header.contains("s3cret"), + "token not in the clear: {header}" + ); } #[test] diff --git a/hive-c0re/src/forge/pr_comment.rs b/hive-c0re/src/forge/pr_comment.rs new file mode 100644 index 00000000..ec36d3b6 --- /dev/null +++ b/hive-c0re/src/forge/pr_comment.rs @@ -0,0 +1,56 @@ +//! Commenting on an agent's config PRs as the core forge user — how a hive +//! reports a merged config commit it refused to deploy back onto the PR that +//! merged it. + +use anyhow::{Context, Result}; + +use super::{CONFIG_ORG, api, core_token}; + +/// Full `owner/name` path of an agent's config repo on the forge. +pub fn config_repo(agent: &str) -> String { + format!("{CONFIG_ORG}/{agent}") +} + +/// The number of the merged PR that put commit `sha` on `repo`'s base branch. +/// +/// # Errors +/// Absent core token, malformed repo, no such PR, or transport/API failure. +pub async fn merged_pr_for_commit(repo: &str, sha: &str) -> Result { + let token = core_token().context("forge core token absent")?; + let (owner, name) = repo + .split_once('/') + .with_context(|| format!("forge repo `{repo}` is not owner/name"))?; + let pull = api(&token)? + .repo_get_commit_pull_request(owner, name, sha) + .await + .context("GET pull request of commit")?; + pull.number + .and_then(|n| u64::try_from(n).ok()) + .with_context(|| format!("pull request of {sha} in {repo} carries no number")) +} + +/// Post a comment to PR (= issue) `pr` on `repo` as the core forge user. +/// PRs are issues in Forgejo, so the PR number is the issue index. +/// +/// # Errors +/// Absent core token, malformed repo, or transport/API failure. +pub async fn post_pr_comment(repo: &str, pr: u64, body: &str) -> Result<()> { + let token = core_token().context("forge core token absent")?; + let (owner, name) = repo + .split_once('/') + .with_context(|| format!("forge repo `{repo}` is not owner/name"))?; + let index = i64::try_from(pr).with_context(|| format!("PR index {pr} overflows i64"))?; + api(&token)? + .issue_create_comment( + owner, + name, + index, + forgejo_api::structs::CreateIssueCommentOption { + body: body.to_owned(), + updated_at: None, + }, + ) + .await + .context("post PR comment")?; + Ok(()) +} diff --git a/hive-c0re/src/forge/pr_merge.rs b/hive-c0re/src/forge/pr_merge.rs deleted file mode 100644 index b6eab317..00000000 --- a/hive-c0re/src/forge/pr_merge.rs +++ /dev/null @@ -1,332 +0,0 @@ -//! PR-based config-flow merge primitives — the forge-side mechanics -//! hive-c0re's deploy apply node (`actions::run_deploy_apply`) orchestrates to -//! land an operator-approved config PR. Part of the operator trust -//! boundary; moved verbatim from the `forge` module root. - -use anyhow::Context; -use forgejo_api::structs::{MergePullRequestOption, MergePullRequestOptionDo, StateType}; - -use super::{CONFIG_ORG, api, core_auth_header, core_token, forge_git_url}; - -// --------------------------------------------------------------------------- -// PR-based config-flow merge primitives (part of the -// dashboard-approve-driven config-change flow). -// -// The dashboard-approve-driven flow has hive-c0re verify an operator-approved -// config PR, then land it: fast-forward-merge the verified sha into the -// protected default branch via the forge merge API (= the merge). These fns are -// the forge-side mechanics the c0re deploy apply node (`run_deploy_apply`) -// orchestrates; the orchestration fetches the verified sha into the agent's -// applied repo (for the eval-verify) before calling `merge_config_pr_ff`. The -// core token is sourced internally (`core_token`), never passed in. `repo` is -// the agent's editable forge config repo in `owner/name` form (e.g. -// `agent-configs/`). -// --------------------------------------------------------------------------- - -/// Typed failure for the merge primitives so the c0re approve-handler can -/// `match` recoverable drift (surface a re-review message) against a hard -/// failure (fail the approval). `HeadDrift` carries the observed sha; `Other` -/// is everything else (a raced non-ff `main`, transport, API, unexpected) and -/// is not auto-retried. -#[derive(Debug)] -pub enum ForgeMergeError { - /// The PR head moved off `expected` (now at `actual`) since the handler's - /// pre-merge `pr_head_sha` re-read — caught by the `head_commit_id` pin on - /// the merge call, which refuses to merge a head that isn't the reviewed sha. - HeadDrift { expected: String, actual: String }, - /// Transport / API / unexpected failure — hard-fail, no auto-retry. Covers - /// a raced non-fast-forwardable `main` (the `fast-forward-only` merge is - /// refused by the forge) as well. - Other(anyhow::Error), -} - -impl std::fmt::Display for ForgeMergeError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::HeadDrift { expected, actual } => { - write!(f, "PR head drifted: expected {expected}, found {actual}") - } - Self::Other(e) => write!(f, "{e}"), - } - } -} - -impl std::error::Error for ForgeMergeError {} - -impl From for ForgeMergeError { - fn from(e: anyhow::Error) -> Self { - Self::Other(e) - } -} - -/// Agent name from an `owner/name` forge repo string (the trailing segment). -fn repo_agent_name(repo: &str) -> &str { - repo.rsplit('/').next().unwrap_or(repo) -} - -/// Resolve a PR's head sha via `git ls-remote refs/pull//head` -/// (Forgejo exposes PR heads there). Pure read, no mutation — the handler's -/// primary drift gate (compare against the approved sha), and -/// `merge_config_pr_ff` uses it to classify a merge-API rejection as head-drift. -/// -/// # Errors -/// `Other` on transport failure or an empty/missing ref. -pub async fn pr_head_sha(repo: &str, pr: u64) -> Result { - let token = core_token() - .ok_or_else(|| ForgeMergeError::Other(anyhow::anyhow!("forge core token absent")))?; - let url = forge_git_url(repo); - let refspec = format!("refs/pull/{pr}/head"); - let out = crate::lifecycle::git_command_authed(&core_auth_header(&token)) - .args(["ls-remote", &url, &refspec]) - .output() - .await - .context("git ls-remote PR head")?; - if !out.status.success() { - return Err(ForgeMergeError::Other(anyhow::anyhow!( - "git ls-remote {repo} {refspec} failed ({}): {}", - out.status, - String::from_utf8_lossy(&out.stderr).trim() - ))); - } - let stdout = String::from_utf8_lossy(&out.stdout); - let sha = stdout - .split_whitespace() - .next() - .filter(|s| !s.is_empty()) - .ok_or_else(|| { - ForgeMergeError::Other(anyhow::anyhow!( - "no head ref for PR #{pr} in {repo} (ls-remote empty)" - )) - })?; - Ok(sha.to_string()) -} - -/// Check whether PR `pr` on `repo` is still open. Returns `Ok(true)` if -/// open, `Ok(false)` if closed or merged, or an error on transport failure. -/// -/// Called at submission time to give an early, actionable error rather than -/// queuing an approval card that will fail later in the approve handler. -/// -/// # Errors -/// `Other` on transport failure or a missing/malformed PR response. -pub async fn pr_is_open(repo: &str, pr: u64) -> Result { - let token = core_token() - .ok_or_else(|| ForgeMergeError::Other(anyhow::anyhow!("forge core token absent")))?; - let (owner, name) = repo.split_once('/').ok_or_else(|| { - ForgeMergeError::Other(anyhow::anyhow!("forge repo `{repo}` is not owner/name")) - })?; - let index = i64::try_from(pr) - .map_err(|_| ForgeMergeError::Other(anyhow::anyhow!("PR index {pr} overflows i64")))?; - let client = api(&token).map_err(ForgeMergeError::Other)?; - let pull = client - .repo_get_pull_request(owner, name, index) - .await - .map_err(|e| ForgeMergeError::Other(anyhow::Error::from(e).context("GET pull request")))?; - Ok(pull.state == Some(StateType::Open)) -} - -/// Full `owner/name` path of an agent's config repo on the forge — the -/// `agent-configs` org mirror that the PR-merge flow reads + fast-forwards. -pub fn config_repo(agent: &str) -> String { - format!("{CONFIG_ORG}/{agent}") -} - -/// Fetch PR #`pr`'s head into the agent's applied repo via -/// `refs/pull//head` (which Forgejo always serves — a bare-sha fetch can -/// be refused by uploadpack policy). This makes the reviewed head an object -/// in the applied repo so the `git_update_ref(main, …)` in the deploy tail and -/// the eval-verify both resolve it locally before the irreversible merge. -/// -/// # Errors -/// `Other` on transport failure or a non-zero git exit. -pub async fn fetch_pr_head_into_applied(repo: &str, pr: u64) -> Result<(), ForgeMergeError> { - let token = core_token() - .ok_or_else(|| ForgeMergeError::Other(anyhow::anyhow!("forge core token absent")))?; - let url = forge_git_url(repo); - let applied = crate::paths::applied_dir(repo_agent_name(repo)); - let refspec = format!("refs/pull/{pr}/head"); - let out = crate::lifecycle::git_command_authed(&core_auth_header(&token)) - .current_dir(&applied) - .args(["fetch", "--no-tags", &url, &refspec]) - .output() - .await - .context("git fetch PR head into applied")?; - if !out.status.success() { - return Err(ForgeMergeError::Other(anyhow::anyhow!( - "git fetch {repo} {refspec} into applied failed ({}): {}", - out.status, - String::from_utf8_lossy(&out.stderr).trim() - ))); - } - Ok(()) -} - -/// Fast-forward-merge PR `pr` on `repo` to `sha` via the Forgejo merge API — -/// THE merge in the config-PR flow. Uses `Do=fast-forward-only` so `main` only -/// ever advances by fast-forward (never a merge commit; a raced non-ff `main` -/// is refused by the forge → `Other`), and pins `head_commit_id = sha` so the -/// forge atomically refuses to merge a head that isn't the reviewed sha — -/// closing the check-then-merge race without ever pushing the protected branch. -/// `core` needs only the repo's merge whitelist, never push access to `main`. -/// The single call fast-forwards `main` to exactly `sha` and marks the PR -/// merged. -/// -/// # Errors -/// `HeadDrift` if the live PR head no longer matches `sha` (the pin rejected -/// it); `Other` for a raced non-ff `main`, transport, or API failure. -pub async fn merge_config_pr_ff(repo: &str, pr: u64, sha: &str) -> Result<(), ForgeMergeError> { - let token = core_token() - .ok_or_else(|| ForgeMergeError::Other(anyhow::anyhow!("forge core token absent")))?; - let (owner, name) = repo.split_once('/').ok_or_else(|| { - ForgeMergeError::Other(anyhow::anyhow!("forge repo `{repo}` is not owner/name")) - })?; - let index = i64::try_from(pr) - .map_err(|_| ForgeMergeError::Other(anyhow::anyhow!("PR index {pr} overflows i64")))?; - let body = MergePullRequestOption { - r#do: MergePullRequestOptionDo::FastForwardOnly, - merge_commit_id: None, - merge_message_field: None, - merge_title_field: None, - delete_branch_after_merge: None, - force_merge: None, - head_commit_id: Some(sha.to_owned()), - merge_when_checks_succeed: None, - }; - let client = api(&token).map_err(ForgeMergeError::Other)?; - match client - .repo_merge_pull_request(owner, name, index, body) - .await - { - Ok(()) => Ok(()), - Err(e) => { - // Classify: a live head that no longer matches `sha` is drift (the - // `head_commit_id` pin rejected it); everything else — a raced - // non-ff `main`, transport, an API error — is a hard failure. - // Best-effort, since the handler's pre-merge re-read is the primary - // gate; a transport failure that also breaks this read falls through - // to `Other`. - match pr_head_sha(repo, pr).await { - Ok(actual) if actual != sha => Err(ForgeMergeError::HeadDrift { - expected: sha.to_string(), - actual, - }), - _ => Err(ForgeMergeError::Other( - anyhow::Error::from(e).context(format!("ff-merge PR #{pr} in {repo} to {sha}")), - )), - } - } - } -} - -/// The number of the merged PR that put commit `sha` on `repo`'s base branch. -/// -/// # Errors -/// `Other` on absent core token, malformed repo, no such PR, or -/// transport/API failure. -pub async fn merged_pr_for_commit(repo: &str, sha: &str) -> Result { - let token = core_token() - .ok_or_else(|| ForgeMergeError::Other(anyhow::anyhow!("forge core token absent")))?; - let (owner, name) = repo.split_once('/').ok_or_else(|| { - ForgeMergeError::Other(anyhow::anyhow!("forge repo `{repo}` is not owner/name")) - })?; - let client = api(&token).map_err(ForgeMergeError::Other)?; - let pull = client - .repo_get_commit_pull_request(owner, name, sha) - .await - .map_err(|e| { - ForgeMergeError::Other(anyhow::Error::from(e).context("GET pull request of commit")) - })?; - pull.number - .and_then(|n| u64::try_from(n).ok()) - .ok_or_else(|| { - ForgeMergeError::Other(anyhow::anyhow!( - "pull request of {sha} in {repo} carries no number" - )) - }) -} - -/// Post a comment to PR (= issue) `pr` on `repo` as the core forge user. -/// PRs are issues in Forgejo, so the PR number is the issue index. Used to -/// surface a failed config-approval deploy's build log back onto the PR so -/// the manager sees why it was rejected without leaving the forge. `repo` -/// is `owner/name`. -/// -/// # Errors -/// `Other` on absent core token, malformed repo, or transport/API failure. -pub async fn post_pr_comment(repo: &str, pr: u64, body: &str) -> Result<(), ForgeMergeError> { - let token = core_token() - .ok_or_else(|| ForgeMergeError::Other(anyhow::anyhow!("forge core token absent")))?; - let (owner, name) = repo.split_once('/').ok_or_else(|| { - ForgeMergeError::Other(anyhow::anyhow!("forge repo `{repo}` is not owner/name")) - })?; - let index = i64::try_from(pr) - .map_err(|_| ForgeMergeError::Other(anyhow::anyhow!("PR index {pr} overflows i64")))?; - let client = api(&token).map_err(ForgeMergeError::Other)?; - client - .issue_create_comment( - owner, - name, - index, - forgejo_api::structs::CreateIssueCommentOption { - body: body.to_owned(), - updated_at: None, - }, - ) - .await - .map_err(|e| ForgeMergeError::Other(anyhow::Error::from(e).context("post PR comment")))?; - Ok(()) -} - -#[cfg(test)] -mod tests { - use super::repo_agent_name; - use crate::forge::git_url_with_base; - - #[test] - fn repo_agent_name_takes_trailing_segment() { - assert_eq!(repo_agent_name("agent-configs/atlas"), "atlas"); - assert_eq!(repo_agent_name("atlas"), "atlas"); - assert_eq!(repo_agent_name("a/b/c"), "c"); - } - - /// The remote carries **no credential**. `argv` is world-readable through - /// `/proc//cmdline`, so a token spliced in here would be published to - /// every local user for the life of the git child. - /// - /// Deliberately not via `forge_git_url`, which reads `HIVE_FORGE_URL` — - /// setting that here would race every other test in this binary. - #[test] - fn forge_git_url_carries_no_credential() { - let url = git_url_with_base("http://forge.example.test", "a/iris"); - assert_eq!(url, "http://forge.example.test/a/iris.git"); - assert!(!url.contains('@'), "no userinfo: {url}"); - } - - #[test] - fn forge_git_url_preserves_https() { - // The scheme is carried through rather than assumed: a swarm - // whose forge is behind TLS must not be downgraded to http. - let url = git_url_with_base("https://forge.example.test", "a/iris"); - assert!(url.starts_with("https://"), "https must survive: {url}"); - } - - /// The credential goes in an `Authorization` header instead — decodable - /// back to `core:`, so the swap is genuinely equivalent auth and - /// not a silent downgrade to anonymous. - #[test] - fn core_auth_header_is_basic_core_token() { - use base64::Engine as _; - let header = crate::forge::core_auth_header("s3cret"); - let b64 = header - .strip_prefix("Authorization: Basic ") - .expect("basic auth header"); - let decoded = base64::engine::general_purpose::STANDARD - .decode(b64) - .expect("valid base64"); - assert_eq!(String::from_utf8_lossy(&decoded), "core:s3cret"); - assert!( - !header.contains("s3cret"), - "token not in the clear: {header}" - ); - } -} diff --git a/hive-c0re/src/job_queue/exec.rs b/hive-c0re/src/job_queue/exec.rs index f034cc22..c00a4016 100644 --- a/hive-c0re/src/job_queue/exec.rs +++ b/hive-c0re/src/job_queue/exec.rs @@ -121,24 +121,13 @@ pub(super) async fn run_node( // takes it directly instead of re-matching the kind behind a `bail!` // that could never fire. NodeKind::WritePermFile { payload, .. } => run_write_perm_file(coord, agent, payload).await, - NodeKind::MergeVerify { approval_id, .. } => { - run_merge_verify(coord, *approval_id, id).await - } - NodeKind::DeployApply { approval_id, .. } => { - run_deploy_apply(coord, *approval_id, id).await.map(|()| { - super::templates::deploy_rebuild_nodes(&builder, agent, *approval_id); - }) - } - NodeKind::FinalizeDeploy { approval_id, .. } => { - run_finalize_deploy(coord, *approval_id).await - } - NodeKind::DeployTail { approval_id, .. } => { - run_deploy_tail(coord, coord.job_queue.root_of(id), agent, *approval_id).await - } NodeKind::ResolveApproval { approval_id, outcome, - } => run_resolve_approval(coord, coord.job_queue.root_of(id), *approval_id, *outcome).await, + } => { + run_resolve_approval(coord, coord.job_queue.root_of(id), *approval_id, *outcome); + Ok(()) + } NodeKind::EmitRebuilt { ok, .. } => { run_emit_rebuilt(coord, agent, coord.job_queue.root_of(id), *ok).await; Ok(()) @@ -147,10 +136,9 @@ pub(super) async fn run_node( // Braces carry no work of their own; completing one lets it reach // `Finishing` so the nodes under it start. What they declare stays held // until their whole subtree settles. - NodeKind::DeployWindow { .. } | NodeKind::AgentWindow { .. } => Ok(()), + NodeKind::AgentWindow { .. } => Ok(()), NodeKind::ForgeSweep => run_forge_sweep().await, NodeKind::MatrixSweep => run_matrix_sweep().await, - NodeKind::WebhookRegister => run_webhook_register().await, NodeKind::KnowledgePull => run_knowledge_pull(coord).await, NodeKind::WantedPull => run_wanted_pull(coord).await, }; @@ -218,34 +206,6 @@ fn matrix_sweep_banner(ctx: sweep_health::SweepFailure) -> String { ) } -/// Boot-time Forgejo webhook management as a DAG node — see -/// [`NodeKind::WebhookRegister`]. Mirrors the guard chain the -/// `tokio::spawn` block it replaced used: no-op (not an error) when the -/// HMAC secret, core token, or hive domain aren't available yet. -/// -/// Registers the config-PR hook only. The knowledge hook is the swarm -/// controller's, which addresses an event to each hive over the queue. -async fn run_webhook_register() -> Result<()> { - let Ok(webhook_secret) = crate::webhook_secret::load_or_generate() else { - tracing::debug!("webhook secret unavailable; skipping hook registration"); - return Ok(()); - }; - let Some(token) = crate::forge::core_token() else { - return Ok(()); - }; - let domain = std::env::var("HYPERHIVE_HIVE_DOMAIN") - .ok() - .filter(|v| !v.is_empty()); - let Some(domain) = domain else { - tracing::debug!("HYPERHIVE_HIVE_DOMAIN unset; skipping webhook registration"); - return Ok(()); - }; - if let Err(e) = crate::forge::ensure_config_pr_webhook(&token, &domain, &webhook_secret).await { - tracing::warn!(error = ?e, "forge: ensure_config_pr_webhook failed"); - } - Ok(()) -} - /// `/knowledge` pull as a DAG node — see [`NodeKind::KnowledgePull`]. Every /// pass reports here, whoever submitted it: as the node's own error on the /// dashboard, and into the debounced banner. @@ -313,17 +273,16 @@ async fn run_wanted_pull(coord: &Arc) -> Result<()> { /// answer. Best-effort — a resolution failure is logged inside /// [`crate::actions::resolve_approval_dag`], never surfaced as a node failure, /// since the work already happened and failing the tail would only misreport it. -async fn run_resolve_approval( +fn run_resolve_approval( coord: &Arc, dag_id: Option, approval_id: i64, outcome: TerminalState, -) -> Result<()> { +) { let reason = (outcome == TerminalState::Failed) .then(|| dag_id.and_then(|dag| coord.job_queue.first_error(dag))) .flatten(); - crate::actions::resolve_approval_dag(coord, approval_id, outcome, reason.as_deref()).await; - Ok(()) + crate::actions::resolve_approval_dag(coord, approval_id, outcome, reason.as_deref()); } /// Rerender the meta flake from whatever containers still exist on disk. @@ -629,9 +588,8 @@ async fn run_meta_lock( // against whatever `applied/` already happened to be locked to — // normally current, but silently stale forever if a past deploy failed // and nothing since retried it. Relocks against each agent's *declared* - // input (the forge URL, not the local `applied/` mirror - // `prepare_deploy` uses for a reviewed deploy) — this path has no PR to - // review, so there's nothing to gate. One combined call, not per-agent: + // input (the forge URL, not the local `applied/` mirror a rebuild + // relocks against) — this path has nothing to gate. One combined call, not per-agent: // simpler, at the cost of one unreachable/broken agent repo failing the // whole cascade relock rather than just that agent. if !cascade.is_empty() { @@ -890,50 +848,6 @@ async fn run_write_perm_file( Ok(()) } -/// Deploy phase 1 — drift gate, fetch, eval-verify. Mutates nothing, so a -/// failure here cancel-cascades the rest of the subtree with the forge and the -/// applied repo exactly as they were. -async fn run_merge_verify(coord: &Arc, approval_id: i64, id: NodeId) -> Result<()> { - crate::actions::run_deploy_merge_verify(coord, approval_id, Some(id.get())).await -} - -/// Deploy phase 2 — the irreversible half: ff-merge, then phase 1 of the -/// two-phase meta deploy. -/// -/// On success it grows the ordinary rebuild subgraph (plus its closing -/// `FinalizeDeploy`) into this DAG rooted on *this* node — which is what puts -/// the appended nodes inside the `DeployWindow`'s subtree, so the `MetaWindow` -/// their `MetaSync` declares is re-entered rather than deadlocked against the -/// ancestor already holding it. On failure nothing is appended and the tail -/// compensates, exactly as before. -async fn run_deploy_apply(coord: &Arc, approval_id: i64, id: NodeId) -> Result<()> { - crate::actions::run_deploy_apply(coord, approval_id, Some(id.get())).await -} - -/// Deploy phase 3 — close the staged-lock window once the appended rebuild has -/// come up clean: drop the rollback ref, plant the `deployed/` tag, commit -/// the staged lock. -async fn run_finalize_deploy(coord: &Arc, approval_id: i64) -> Result<()> { - crate::actions::run_finalize_deploy(coord, approval_id).await -} - -/// Deploy compensation + bookkeeping tail. `AfterAny` the apply node, so it -/// runs on every outcome; it is deliberately infallible (see -/// [`crate::actions::run_deploy_tail`]) — a failing tail must not flip an -/// otherwise-successful deploy's DAG state. -/// -/// Takes the agent from the node payload so the tail can still compensate when -/// the approval row is gone (deny race, purge). -async fn run_deploy_tail( - coord: &Arc, - dag_id: Option, - agent: &str, - approval_id: i64, -) -> Result<()> { - crate::actions::run_deploy_tail(coord, dag_id, agent, approval_id).await; - Ok(()) -} - /// Compute which agents a `nix flake update ` on the meta /// flake affects — the fan-out set for `MetaUpdate` DAGs. Empty /// `inputs` or any input under `hyperhive` → every container; diff --git a/hive-c0re/src/job_queue/model.rs b/hive-c0re/src/job_queue/model.rs index 3056bb4d..e22badbb 100644 --- a/hive-c0re/src/job_queue/model.rs +++ b/hive-c0re/src/job_queue/model.rs @@ -102,28 +102,11 @@ pub enum NodeKind { WriteDropin { agent: String }, /// Commit `tool-groups.json` / `capabilities.json` per its `payload`. WritePermFile { agent: String, payload: PermPayload }, - /// Group root of the approval-deploy (`MergeConfigPr`) subtree — the - /// **brace** that owns the deploy window. See _Braces_ and _Approvals_. - DeployWindow { agent: String, approval_id: i64 }, /// Group root of a rebuild subtree — the **brace**, scoped to one /// agent. Why braces exist and what they cost: _Braces_. AgentWindow { agent: String }, - /// Deploy phase 1 — **verify only, mutates nothing.** - MergeVerify { agent: String, approval_id: i64 }, - /// Deploy phase 2 — the irreversible fast-forward plus the *opening* - /// half of the two-phase meta deploy. Does **not** build the container - /// itself; grows the ordinary rebuild subgraph in as its own children - /// (see the rebuild-path section's approval-deploy paragraph). - DeployApply { agent: String, approval_id: i64 }, - /// Deploy phase 3 — closes the two-phase meta deploy once - /// [`NodeKind::DeployApply`]'s rebuild subgraph comes up clean. - FinalizeDeploy { agent: String, approval_id: i64 }, - /// Deploy compensation **and bookkeeping** tail. See the `DeployTail` - /// node-inventory row for its three responsibilities and why it isn't - /// named `AbortDeploy`. - DeployTail { agent: String, approval_id: i64 }, - /// Tail node of an approval-carrying DAG (opaque deploy / - /// config-PR merge): resolve the approval row from how the work ended. + /// Tail node of an approval-carrying DAG (a meta-inputs update): resolve + /// the approval row from how the work ended. ResolveApproval { approval_id: i64, /// Which outcome this node reports. A template emits **one per @@ -147,8 +130,6 @@ pub enum NodeKind { /// Matrix user/space sweep (`matrix::ensure_all`): at boot and every 30 /// minutes. Agentless. MatrixSweep, - /// One-shot boot-time Forgejo webhook registration. Agentless. - WebhookRegister, /// `/knowledge` pull (`knowledge::pull`): at boot, on the swarm /// knowledge-changed event, and hourly. Agentless. KnowledgePull, @@ -174,9 +155,6 @@ impl hive_jobq_wire::WireNode for NodeKind { if !agent.is_empty() { data.insert("agent".to_owned(), agent.into()); } - if let NodeKind::DeployWindow { approval_id, .. } = self { - data.insert("approval_id".to_owned(), (*approval_id).into()); - } if let NodeKind::MetaLock { inputs, .. } = self && !inputs.is_empty() { @@ -221,19 +199,13 @@ impl NodeKind { | NodeKind::PauseDrain { agent } | NodeKind::WriteDropin { agent } | NodeKind::WritePermFile { agent, .. } - | NodeKind::DeployWindow { agent, .. } | NodeKind::AgentWindow { agent } - | NodeKind::MergeVerify { agent, .. } - | NodeKind::DeployApply { agent, .. } - | NodeKind::FinalizeDeploy { agent, .. } - | NodeKind::DeployTail { agent, .. } | NodeKind::EmitRebuilt { agent, .. } | NodeKind::SetWanted { agent, .. } => agent, NodeKind::MetaLock { .. } | NodeKind::ResolveApproval { .. } | NodeKind::ForgeSweep | NodeKind::MatrixSweep - | NodeKind::WebhookRegister | NodeKind::KnowledgePull | NodeKind::WantedPull => "", } @@ -280,8 +252,8 @@ impl NodeKind { // bug, and a `true` here would suppress the alert that says so. // - `Reconcile` is a planner; it fans out `Start` / `Stop`, which carry // their own answer. - // - `DeployWindow` brackets a deploy without itself stopping anything. - // - `AgentWindow` likewise, though it *parents* nodes that answer + // - `AgentWindow` brackets a rebuild without itself stopping anything, + // though it *parents* nodes that answer // `true`. This is per-node, not per-subtree, and each of those // children reports for itself — so `true` here would only widen // suppression over the build and tail, where a vanished container is diff --git a/hive-c0re/src/job_queue/templates.rs b/hive-c0re/src/job_queue/templates.rs index 1c3104e3..4ec7bdd0 100644 --- a/hive-c0re/src/job_queue/templates.rs +++ b/hive-c0re/src/job_queue/templates.rs @@ -6,7 +6,7 @@ //! that already held the resource could get away with declaring nothing. //! //! **The one sanctioned exception is a brace** — a pure-resource-holder root -//! ([`NodeKind::AgentWindow`], [`NodeKind::DeployWindow`]) declaring for a +//! ([`NodeKind::AgentWindow`]) declaring for a //! coordinated subtree whose members then declare nothing. See //! `docs/scheduler/coordinator.md`, _Braces_ — which also carries the per-operation DAG //! shapes, so they are not restated here. @@ -314,42 +314,6 @@ pub(crate) fn graceful_rebuild_nodes<'a>( rebuild_subtree(builder, agent, relock, true, after) } -/// The rebuild subgraph a [`NodeKind::DeployApply`] grows into its own DAG once -/// the merge has landed and `prepare_deploy` has staged the lock, plus the -/// [`NodeKind::FinalizeDeploy`] that closes the window behind it. -/// -/// `relock = false` is the whole reason this composes: `prepare_deploy` already -/// relocked and staged `flake.lock`, so the appended `MetaSync` must do the dir -/// prep + `sync_agents` *without* re-locking over it. -/// -/// `FinalizeDeploy` waits on **two** roots, which together reproduce the gate -/// the old fused node had around its inline `rebuild_no_meta` call: -/// - `AfterOk` `Prebuild` — a parent's state is its roll-up, so this is `Done` -/// only once `StopForUpdate` → `Swap` → `RebuildBookkeeping` all are (a failed *or* -/// cancelled child rolls the parent up `Failed`). That's the old -/// `build_result`. -/// - `AfterOk` `Reconcile` — the old call passed `deferred_start = false` on -/// purpose: the container had to come back up *before* the deploy was -/// finalized. `Reconcile` alone would not do, being `AfterAny` — it reaches -/// `Done` even after a failed `Swap`. -/// -/// Declared into a **running** `DeployApply`'s own builder, not submitted: the -/// roots below become children of that node, which puts them inside the -/// `DeployWindow`'s subtree — so the `MetaWindow` this subgraph's `MetaSync` -/// and `FinalizeDeploy` declare is re-entered from the ancestor already holding -/// it rather than deadlocking against it. -pub(crate) fn deploy_rebuild_nodes(builder: &JobBuilder, agent: &str, approval_id: i64) { - let roots = rebuild_nodes(builder, agent, false, None); - let _finalize = builder - .node(NodeKind::FinalizeDeploy { - agent: agent.to_owned(), - approval_id, - }) - .needs(Resource::MetaWindow) - .after_ok(roots.agent_window) - .after_ok(roots.reconcile); -} - /// One uniform rebuild shape — no `was_running` branch. `StopForUpdate` /// noops when already down; the tail `Reconcile` auto-noops the start /// when `wanted = Offline` (a rebuild of a deliberately-stopped agent @@ -375,71 +339,6 @@ pub fn rebuild(builder: &JobBuilder, agent: &str, relock: bool) -> Vec` tag planted) -// while the tail still runs to compensate, and `Reconcile` is deliberately -// still reached — it boots the container back up. -// -// Every declared half of that is asserted by -// `deploy_apply_grows_rebuild_subgraph_and_finalizes_after_it`, which reads -// `deploy_rebuild_nodes`' shape directly: -// -// - "a failed swap still reaches Reconcile" is the `reconcile` row's -// `AfterAny` edge on `prebuild` (`done|failed|skipped`); -// - "finalize is cancel-cascaded" is `finalize_deploy`'s `AfterOk` pair — -// either root failing skips it; -// - "the tail still runs" is `deploy_tail`'s own `done|failed|skipped` edge -// on apply, asserted in the `approval_deploy` table above. -// -// The runtime halves are hive_jobq's: cascade on failure, roll-up, and -// `first_error` digging past a group root that rolled up `Failed` while -// carrying no error of its own (`first_error_skips_a_rolled_up_failure_ -// carrying_no_error`). That last one is why the DAG reports "profile swap -// failed" rather than nothing — the mechanism, not this shape. - -// `deploy_dag_skips_apply_but_still_runs_tail_when_verify_fails` lived here. -// -// A pre-merge rejection (drift gate, eval failure) cancel-cascades the -// irreversible half via its `AfterOk` edge, while the tail is still reached — -// it owns the forge mirror, not just compensation. That test and -// `deploy_dag_runs_phases_in_order_and_tails_a_failed_apply` differed only in -// *where* they injected the failure, and each drove the whole DAG to watch the -// tail run anyway. -// -// Both outcomes follow from one declared edge, which the surviving test now -// asserts directly: the tail accepts `done|failed|skipped` on apply, and -// `skipped` is exactly the state apply lands in when verify failed and it never -// ran. The runtime halves are hive_jobq's and tested there — -// `failed_after_ok_dep_cancels_dependents_but_after_any_still_runs` and -// `failed_child_rolls_parent_up_to_failed` (an Ok tail cannot launder a failed -// deploy into a success). - // ---- history ---- // // The node → build-log link is no longer queue state: the log row carries diff --git a/hive-c0re/src/lifecycle/git.rs b/hive-c0re/src/lifecycle/git.rs index cc5712e0..e4648ec6 100644 --- a/hive-c0re/src/lifecycle/git.rs +++ b/hive-c0re/src/lifecycle/git.rs @@ -135,55 +135,6 @@ pub async fn git_tag(dir: &Path, name: &str, target: &str) -> Result<()> { git(dir, &["tag", name, target]).await } -/// Plant an annotated tag with `body` as the message. Used for -/// `failed/` (body = build error) and `denied/` (body = -/// operator note). Multi-line bodies handled via stdin so we don't -/// have to escape anything. -pub async fn git_tag_annotated(dir: &Path, name: &str, target: &str, body: &str) -> Result<()> { - use tokio::io::AsyncWriteExt; - // Annotated tags are git objects, so they need a tagger identity - // (same constraint as a commit). Pass the hive-c0re identity - // inline rather than relying on a global git config — applied - // repos are hive-c0re-owned and the host's user might not have - // user.email set. - let mut child = git_command() - .current_dir(dir) - .args([ - "-c", - &format!("user.name={GIT_NAME}"), - "-c", - &format!("user.email={GIT_EMAIL}"), - "tag", - "-a", - name, - target, - "-F", - "-", - ]) - .stdin(std::process::Stdio::piped()) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .spawn() - .with_context(|| format!("spawn git tag -a {name} in {}", dir.display()))?; - if let Some(mut stdin) = child.stdin.take() { - stdin - .write_all(body.as_bytes()) - .await - .context("write tag body to git stdin")?; - // Drop closes stdin so git can finish reading. - drop(stdin); - } - let out = child.wait_with_output().await.context("wait git tag -a")?; - if !out.status.success() { - bail!( - "git tag -a {name} failed ({}): {}", - out.status, - String::from_utf8_lossy(&out.stderr).trim() - ); - } - Ok(()) -} - /// Replace working tree + index with the tree at `target` without /// moving HEAD. `applied/main` stays pointing at the last known-good /// `deployed/*` while we let `nixos-container update` evaluate the @@ -252,13 +203,3 @@ pub async fn git_is_ancestor(dir: &Path, ancestor: &str, descendant: &str) -> Re ), } } - -/// Delete a ref. The counterpart to [`git_update_ref`] for the bookkeeping -/// refs a deploy parks in the applied repo (`refs/hyperhive/rollback/`, -/// which records the pre-merge `main` so the deploy tail can compensate a -/// merge that landed but never finalized). `update-ref -d` is a no-op-free -/// delete: it errors if the ref does not exist, so callers that treat absence -/// as "nothing to undo" should check with [`git_rev_parse`] first. -pub async fn git_delete_ref(dir: &Path, refname: &str) -> Result<()> { - git(dir, &["update-ref", "-d", refname]).await -} diff --git a/hive-c0re/src/lifecycle/mod.rs b/hive-c0re/src/lifecycle/mod.rs index 6d3e449c..a5ae2fac 100644 --- a/hive-c0re/src/lifecycle/mod.rs +++ b/hive-c0re/src/lifecycle/mod.rs @@ -8,9 +8,8 @@ mod setup; mod tests; pub use git::{ - git, git_authed, git_command, git_command_authed, git_delete_ref, git_is_ancestor, - git_read_tree_reset, git_rev_parse, git_tag, git_tag_annotated, git_update_ref, - git_update_ref_cas, + git, git_authed, git_command, git_command_authed, git_is_ancestor, git_read_tree_reset, + git_rev_parse, git_tag, git_update_ref, git_update_ref_cas, }; pub use host_config::write_dropins; pub use setup::{ diff --git a/hive-c0re/src/lifecycle/setup.rs b/hive-c0re/src/lifecycle/setup.rs index d97c1562..992a5eba 100644 --- a/hive-c0re/src/lifecycle/setup.rs +++ b/hive-c0re/src/lifecycle/setup.rs @@ -98,8 +98,8 @@ async fn ensure_applied_remote(proposed_dir: &Path, name: &str) -> Result<()> { /// Set up the applied repo. First-spawn only: init the repo, pull /// proposed's initial commit in via `git fetch`, tag it `deployed/0`. /// This is the *only* time hive-c0re reads from `proposed` for an -/// agent — subsequent config changes are fetched from the reviewed -/// forge PR head at merge time (see `actions::run_deploy_apply`). +/// agent — subsequent config changes are fetched from the forge's `main` +/// (see `actions::advance_applied_to_rev`). /// /// `proposed_dir` is `None` on rebuild paths where the repo already /// exists — we just verify it's the right shape and bail otherwise. diff --git a/hive-c0re/src/main.rs b/hive-c0re/src/main.rs index c9ecaa1b..3911e305 100644 --- a/hive-c0re/src/main.rs +++ b/hive-c0re/src/main.rs @@ -40,7 +40,6 @@ mod swarm_queue; mod swarm_status; #[cfg(test)] mod test_env; -mod webhook_secret; mod workers; pub(crate) use agent_config::{capabilities, limits, resource_limits, tool_groups, topology}; @@ -296,54 +295,6 @@ async fn cmd_serve( // `workers::auto_update::submit_startup_sweep_nodes`), submitted // unconditionally on every boot — moved off a bare `tokio::spawn` so it // shows as real work on the dashboard. - // Webhook HMAC secret: load from state dir or generate on first run. - // Used by both the webhook handlers (verification) and the Forgejo - // hook registrations (so Forgejo signs deliveries with the same key). - let webhook_secret: Option = match crate::webhook_secret::load_or_generate() { - Ok(s) => Some(s), - Err(e) => { - tracing::error!( - error = ?e, - "webhook secret load/generate failed; /webhook/* endpoints disabled and hooks not registered" - ); - None - } - }; - // Webhook setup: now a `NodeKind::WebhookRegister` DAG node (see - // `workers::auto_update::submit_startup_sweep_nodes`), registering both - // `internal/knowledge` (push → git pull) and the `agent-configs` org - // (pull_request → queue MergeConfigPr approval) hooks. Its executor - // re-derives the core token / hive domain / HMAC secret itself, mirroring - // the guard chain that used to live here — see `job_queue::exec:: - // run_webhook_register`. - // Config-PR polling fallback: scan agent-configs org every 5 minutes - // for open PRs that have no pending MergeConfigPr approval. Catches - // anything the webhook missed (c0re was down when PR opened, delivery - // failed, etc.). First sweep fires immediately on startup. - let poll_coord = coord.clone(); - let mut poll_shutdown = coord.shutdown_rx(); - tokio::spawn(async move { - let interval = std::time::Duration::from_mins(5); - loop { - if let Some(token) = forge::core_token() { - let result = Box::pin(forge::config_pr_poll::poll_open_config_prs( - &token, - &poll_coord, - )) - .await; - if let Err(e) = result { - tracing::debug!(error = ?e, "config-pr poll: sweep failed (forge may be absent)"); - } - } - tokio::select! { - () = tokio::time::sleep(interval) => {} - _ = poll_shutdown.changed() => { - tracing::info!("config-pr poll: shutdown signal received"); - break; - } - } - } - }); // Knowledge periodic pull: hourly fallback in case a knowledge event from // the swarm is missed (e.g. hive-c0re was down when it was sent). Sleeps // before its first pull: the boot `NodeKind::KnowledgePull` DAG node @@ -506,9 +457,8 @@ async fn cmd_serve( // channel (used by `recv_blocking_batch`) stays untouched. spawn_broker_to_dashboard_forwarder(coord.clone()); let dash_coord = coord.clone(); - let dash_secret = webhook_secret.clone(); tokio::spawn(async move { - if let Err(e) = dashboard::serve(dashboard_port, dash_coord, dash_secret).await { + if let Err(e) = dashboard::serve(dashboard_port, dash_coord).await { tracing::error!(error = ?e, "dashboard failed"); } }); diff --git a/hive-c0re/src/meta.rs b/hive-c0re/src/meta.rs index a10422f0..45aca967 100644 --- a/hive-c0re/src/meta.rs +++ b/hive-c0re/src/meta.rs @@ -1,7 +1,7 @@ //! Single hive-c0re-owned flake at `/var/lib/hyperhive/meta/` that //! exports one `nixosConfiguration` per agent and drives the system-wide -//! deploy audit trail. Flow (`sync_agents`, two-phase `prepare_deploy` / -//! `finalize_deploy` / `abort_deploy`, `lock_update_hyperhive`): +//! deploy audit trail. Flow (`sync_agents`, `lock_update_for_rebuild`, +//! `lock_update_hyperhive`): //! `docs/agent-lifecycle/approvals.md::Meta flake`. use std::path::Path; @@ -28,16 +28,12 @@ const GIT_EMAIL: &str = "c0re@hyperhive.local"; static META_LOCK: Mutex<()> = Mutex::const_new(()); // Exclusivity for meta-repo *windows* that span multiple `META_LOCK` -// acquisitions — above all the two-phase deploy (`prepare_deploy` stages -// `flake.lock` uncommitted for the whole container build; -// `finalize_deploy` / `abort_deploy` resolve it) — is **not** a mutex in -// this module. `META_LOCK` above serializes individual git ops but cannot -// keep another op out of that staged window; that window is owned by the -// job queue instead, as `Resource::MetaWindow`, declared by every -// meta-mutating node kind (it declares `Resource::MetaWindow`). A resource can -// be held by a subtree root across its children, which a `MutexGuard` -// (bounded by one executor fn) cannot — that's what lets the deploy be -// modelled as sub-nodes rather than one opaque node. +// acquisitions is **not** a mutex in this module. `META_LOCK` above +// serializes individual git ops but cannot keep another op out of a window +// spanning several of them; that window is owned by the job queue instead, +// as `Resource::MetaWindow`, declared by every meta-mutating node kind. A +// resource can be held by a subtree root across its children, which a `MutexGuard` +// (bounded by one executor fn) cannot. /// Where the manager sees this directory inside its container (RO bind). pub const CONTAINER_MANAGER_META_MOUNT: &str = "/meta"; @@ -231,75 +227,6 @@ pub async fn sync_agents(hive: &HiveEnv, agents: &[AgentSpec]) -> Result<()> { Ok(()) } -/// Phase 1 of an apply-commit deploy. Updates the locked rev of -/// `agent-` to whatever `applied//main` currently points -/// at and **stages** the lock so `nixos-container update --flake -/// meta#` (which reads via `git+file://`) sees the new rev via -/// the index. Doesn't commit — `finalize_deploy` commits on build -/// success, `abort_deploy` drops the staged change on failure so -/// meta history only carries successful deploys. -pub async fn prepare_deploy(name: &str, node_id: Option) -> Result<()> { - let _guard = META_LOCK.lock().await; - let dir = crate::paths::meta_root(); - let input = format!("agent-{name}"); - // Re-lock the agent input against the LOCAL applied mirror, not the - // persistent forge URL declared in the meta flake (see the `## Meta flake` - // note in docs/agent-lifecycle/approvals.md): the deploy must build the exact reviewed - // config that `verify_commit` gated and `applied//main` was - // fast-forwarded to, and it must keep working when the forge is - // unreachable (rebuilds fire on crash-restart / meta bumps too, not just - // config PRs). `--override-input` writes the applied rev into the lock; - // the forge URL stays the declared, reviewable source of truth. - let applied = applied_override_url(&crate::paths::applied_dir(name)); - nix_logged( - &dir, - &[ - "flake", - "update", - &input, - "--override-input", - &input, - &applied, - ], - name, - "prepare-deploy", - node_id, - ) - .await?; - // Stage the new lock — git+file://'s dirty-tree fetcher reads - // index entries, so the upcoming nixos-container update sees the - // bumped rev without a commit yet. - git(&dir, &["add", "flake.lock"]).await -} - -/// Phase 2-success. Commit the staged lock with the deployed tag + -/// sha as the message. No-op when the rev was already at the right -/// place (nothing staged → nothing to commit). -pub async fn finalize_deploy(name: &str, sha: &str, tag: &str) -> Result<()> { - let _guard = META_LOCK.lock().await; - let dir = crate::paths::meta_root(); - if !paths_dirty(&dir, &["flake.lock"]).await? { - return Ok(()); - } - let short = &sha[..sha.len().min(12)]; - git_commit_paths( - &dir, - &format!("deploy {name} {tag} {short}"), - &["flake.lock"], - ) - .await -} - -/// Phase 2-failure. Unstage + restore the lock so meta returns to -/// the previously-committed shas. The failed proposal is still -/// captured in `applied/`'s annotated `failed/` tag. -pub async fn abort_deploy() -> Result<()> { - let _guard = META_LOCK.lock().await; - let dir = crate::paths::meta_root(); - git(&dir, &["restore", "--staged", "flake.lock"]).await?; - git(&dir, &["restore", "flake.lock"]).await -} - /// One-shot used by the manual-rebuild path: relock just one /// agent's input and commit the lock change if any. Single-phase /// (no separate finalize) because rebuild has no failure-revert @@ -308,9 +235,9 @@ pub async fn lock_update_for_rebuild(name: &str) -> Result<()> { let _guard = META_LOCK.lock().await; let dir = crate::paths::meta_root(); let input = format!("agent-{name}"); - // Re-lock from the local applied mirror, not the persistent forge URL — - // same rationale as `prepare_deploy`: build exactly `applied//main` and - // stay reproducible when the forge is unreachable. + // Re-lock from the local applied mirror, not the persistent forge URL: + // build exactly `applied//main` and stay reproducible when the forge + // is unreachable. let applied = applied_override_url(&crate::paths::applied_dir(name)); nix( &dir, @@ -336,13 +263,6 @@ pub async fn lock_update_for_rebuild(name: &str) -> Result<()> { .await } -/// Build the `--override-input` value pinning an agent's config repo to -/// an exact revision: `git+file://?rev=`. Pure so the -/// URL shape is unit-testable without a nix shellout. -fn agent_input_override(applied_dir: &Path, sha: &str) -> String { - format!("git+file://{}?rev={sha}", applied_dir.display()) -} - /// `--override-input` URL re-locking an agent's config input against its /// LOCAL applied mirror (`git+file://`, current `main` head) /// instead of the persistent forge URL declared in the meta flake. The @@ -354,51 +274,6 @@ fn applied_override_url(applied_dir: &Path) -> String { format!("git+file://{}", applied_dir.display()) } -/// Non-mutating "would this commit apply?" verify for the PR-based config -/// flow. Evaluates the agent's nixos configuration with its meta -/// input overridden to the exact `sha`, WITHOUT moving `applied/main` or -/// writing the real meta `flake.lock`: `--override-input` pins the input -/// at eval time only and `--no-write-lock-file` guarantees the on-disk -/// lock is never touched (a genuinely-needed lock change surfaces as an -/// error rather than a silent mutation). Confirms the flake at `sha` -/// evaluates and the meta lock resolves around it. `Ok(())` = would -/// apply; `Err` carries the eval/lock failure so the operator's approve -/// is rejected before any live-state change. -/// -/// Precondition: `sha` must be reachable in `applied_dir`'s object store -/// — the caller fetches the PR head into `applied_dir` first. Eval-only -/// (no build): catches nix / eval / module-option errors + lock -/// resolution at the same cost profile as the legacy pre-merge check; -/// the real container build still runs (and can still roll back) on the -/// actual apply, so this is the right "would this apply" gate. -pub async fn verify_commit( - name: &str, - applied_dir: &Path, - sha: &str, - node_id: Option, -) -> Result<()> { - let _guard = META_LOCK.lock().await; - let dir = crate::paths::meta_root(); - let input = format!("agent-{name}"); - let over = agent_input_override(applied_dir, sha); - let attr = format!(".#nixosConfigurations.{name}.config.system.build.toplevel.drvPath"); - nix_logged( - &dir, - &[ - "eval", - &attr, - "--override-input", - &input, - &over, - "--no-write-lock-file", - ], - name, - "verify", - node_id, - ) - .await -} - /// Update one or more named inputs in the meta flake and commit /// the resulting lock change with a single combined message. /// Used by the dashboard's "update meta inputs" form so the @@ -447,7 +322,7 @@ pub async fn lock_update_hyperhive() -> Result<()> { /// Write the tool-groups file for `agent` and commit it atomically /// under `META_LOCK`. Ensures the JSON change is staged + committed -/// before the next `prepare_deploy` or `sync_agents` runs, so the +/// before the next `sync_agents` runs, so the /// working tree is never left dirty by an untimely `PermChange` write. pub async fn commit_tool_groups(agent: &str, groups: &[String]) -> Result<()> { let _guard = META_LOCK.lock().await; @@ -490,8 +365,8 @@ pub async fn commit_capabilities(agent: &str, caps: &[String]) -> Result<()> { /// Write the resource-limits file for `agent` and commit it atomically /// under `META_LOCK`. Same rationale as `commit_tool_groups`: the -/// working tree must never be left dirty for the next `prepare_deploy` -/// or `sync_agents` to trip over. +/// working tree must never be left dirty for the next `sync_agents` to trip +/// over. /// /// Unlike the perm files this one is never injected into the container — /// it's a host-side cap *on* the agent — but it lives in the same repo @@ -1048,9 +923,7 @@ where // on-disk state and boot never depends on the forge being reachable. // The forge `agent-configs/` repos stay the review/audit surface // (config PRs land there) but are not the flake's build input. deploy + - // rebuild re-lock this input to `applied/`'s current `main` head; - // `verify_commit` overrides it to a proposed `?rev=` for eval - // before that head moves. + // rebuild re-lock this input to `applied/`'s current `main` head. for spec in agents { let _ = writeln!( out, @@ -1459,8 +1332,7 @@ async fn git_commit(dir: &Path, message: &str) -> Result<()> { } /// Path-limited commit: commits ONLY the given paths, so unrelated -/// staged content — above all a `prepare_deploy`-staged `flake.lock` -/// — can never be swept into someone else's commit. Every targeted +/// staged content can never be swept into someone else's commit. Every targeted /// meta commit (perm files, topology, lock bumps) goes through this; /// only `sync_agents` uses the bare [`git_commit`], because its /// staged set *is* its intentional commit set. @@ -1507,7 +1379,7 @@ fn nix_argv<'a>(args: &[&'a str]) -> Vec<&'a str> { } /// Run `nix ` in `dir` (flakes enabled), capturing combined output. -/// Shared core of [`nix`] and [`nix_logged`]. +/// The core of [`nix`]. async fn nix_output(dir: &Path, args: &[&str]) -> Result { Command::new("nix") .current_dir(dir) @@ -1540,66 +1412,11 @@ async fn nix(dir: &Path, args: &[&str]) -> Result<()> { nix_check(args, &out) } -/// Like [`nix`] but records the invocation (cmdline + stdout + stderr + -/// terminal status) into a `build_logs.sqlite` row so a config-approval -/// eval/deploy step shows up on the dashboard. This is what makes a -/// failing eval-verify visible: it runs before any container build, so -/// without a row a rejected config approval leaves the operator with -/// zero build logs to look at. Best-effort logging: a missing global -/// handle or a failed `start()` just skips the row — the command still -/// runs and its exit status is still enforced. -async fn nix_logged( - dir: &Path, - args: &[&str], - agent: &str, - kind: &str, - node_id: Option, -) -> Result<()> { - let cmdline = format!("nix {}", nix_argv(args).join(" ")); - let logs = crate::build_logs::global(); - let log_id = logs.as_ref().and_then(|h| { - // `node_id` links the row to the queue node that ran it, so the - // dashboard can reach this log from the node instead of only from the - // agent+kind+time listing. Both call paths are queue-only today and - // pass `Some`; it stays an `Option` because these are `meta`'s public - // API and a future non-queue caller has no node to name. - h.start(agent, kind, &cmdline, node_id) - .map_err(|e| { - tracing::warn!(error = ?e, %kind, "build_logs: start failed (meta log dropped)"); - }) - .ok() - }); - - let out = nix_output(dir, args).await?; - - if let (Some(h), Some(id)) = (&logs, log_id) { - for line in String::from_utf8_lossy(&out.stdout).lines() { - h.append_stdout(id, line); - } - for line in String::from_utf8_lossy(&out.stderr).lines() { - h.append_stderr(id, line); - } - h.finish( - id, - if out.status.success() { - crate::build_logs::BuildStatus::Ok - } else { - crate::build_logs::BuildStatus::Fail - }, - ); - } - - nix_check(args, &out) -} - #[cfg(test)] mod tests { use super::*; - /// The regression the deploy-window bug review surfaced: a - /// path-limited commit must leave an unrelated staged file (the - /// prepare_deploy-staged `flake.lock`) untouched, so a later - /// `abort_deploy` still has something to restore. + /// A path-limited commit must leave an unrelated staged file untouched. #[tokio::test] async fn path_limited_commit_leaves_unrelated_staged_file_alone() { let tmp = tempfile::tempdir().expect("tempdir"); @@ -1611,7 +1428,7 @@ mod tests { std::fs::write(dir.join("flake.lock"), "v1").expect("write"); git(dir, &["add", "-A"]).await.expect("add"); git_commit(dir, "seed").await.expect("seed commit"); - // A deploy stages a new lock (uncommitted)… + // Something stages a new lock (uncommitted)… std::fs::write(dir.join("flake.lock"), "v2-staged-by-deploy").expect("write"); git(dir, &["add", "flake.lock"]).await.expect("stage lock"); // …and a perm change commits, path-limited. @@ -1620,7 +1437,7 @@ mod tests { git_commit_paths(dir, "set tool-groups for alice", &["tool-groups.json"]) .await .expect("path-limited commit"); - // The perm file is committed; the deploy's staged lock is not. + // The perm file is committed; the staged lock is not. assert!( !paths_dirty(dir, &["tool-groups.json"]) .await @@ -1641,15 +1458,6 @@ mod tests { } } - #[test] - fn agent_input_override_pins_exact_rev() { - let p = Path::new("/var/lib/hyperhive/agents/iris/applied"); - assert_eq!( - agent_input_override(p, "abc123def"), - "git+file:///var/lib/hyperhive/agents/iris/applied?rev=abc123def" - ); - } - #[test] fn applied_override_url_targets_local_mirror_main_head() { // Deploy + rebuild re-lock against the local applied mirror's `main` diff --git a/hive-c0re/src/paths.rs b/hive-c0re/src/paths.rs index eb3baa3f..b6fa8a2e 100644 --- a/hive-c0re/src/paths.rs +++ b/hive-c0re/src/paths.rs @@ -59,31 +59,12 @@ pub fn db_dir() -> PathBuf { state_root().join("db") } -/// `webhook-secret` — hex-encoded 32-byte HMAC secret shared between -/// hive-c0re's webhook handlers and the Forgejo webhook registrations. -/// Generated on first startup and persisted. The config-PR hook is replaced -/// at the next boot-time registration when this no longer matches -/// [`forge_config_pr_webhook_fingerprint`]. -#[must_use] -pub fn webhook_secret_file() -> PathBuf { - state_root().join("webhook-secret") -} - /// `forge/` — hive-c0re's own forge provisioning markers. #[must_use] pub fn forge_dir() -> PathBuf { state_root().join("forge") } -/// `forge/config-pr-webhook-secret-sha256` — SHA-256 of the webhook secret -/// the config-PR hook was last registered with. Forgejo never returns a -/// hook's secret, so this is the only record of which one it signs with; -/// delete to force the hook to be replaced. -#[must_use] -pub fn forge_config_pr_webhook_fingerprint() -> PathBuf { - forge_dir().join("config-pr-webhook-secret-sha256") -} - /// `forge/core-avatar-set` — marker: core account avatar uploaded. #[must_use] pub fn forge_core_avatar_marker() -> PathBuf { diff --git a/hive-c0re/src/server.rs b/hive-c0re/src/server.rs index 3899c76a..5436c96f 100644 --- a/hive-c0re/src/server.rs +++ b/hive-c0re/src/server.rs @@ -424,7 +424,7 @@ async fn handle_set_resource_limits( // Goes through `meta::commit_resource_limits`, not the bare // `resource_limits::set_limits`: the write has to be staged + // committed under `META_LOCK` or it leaves the meta working tree - // dirty for the next `prepare_deploy` / `sync_agents` to trip over. + // dirty for the next `sync_agents` to trip over. crate::meta::commit_resource_limits(name.as_str(), &limits).await?; // Re-apply the drop-in straight away — same three lines as the job diff --git a/hive-c0re/src/socket_server/config_approvals.rs b/hive-c0re/src/socket_server/config_approvals.rs deleted file mode 100644 index 1c8a9a0c..00000000 --- a/hive-c0re/src/socket_server/config_approvals.rs +++ /dev/null @@ -1,103 +0,0 @@ -//! Config-approval submit helper `submit_merge_config_pr`. -//! -//! `submit_merge_config_pr` is called from the dashboard webhook handler -//! (`dashboard::webhook`) — agents no longer need an MCP tool for config -//! changes; opening a config PR on `agent-configs/` is enough to -//! trigger hive-c0re's webhook-driven queue path. - -use std::sync::Arc; - -use crate::coordinator::Coordinator; - -/// Submit-time half of the PR-merge flow: fetch the PR head sha from the -/// forge, queue the approval row, and emit the `approval_added` event so the -/// dashboard shows the pending card immediately. -/// -/// The PR head sha is stored as `fetched_sha` on the approval row — the -/// "reviewed sha" the deploy's `MergeVerify` node drift-gates -/// against before doing anything irreversible. This does NOT fetch the commit -/// into the applied repo at submission time (that happens inside the approve -/// handler, step 2, after the drift check). No flake pre-flight either — -/// eval-verify happens at approval time too. -pub(crate) async fn submit_merge_config_pr( - coord: &Arc, - agent: &str, - pr_number: u64, - description: Option<&str>, - submitter: &str, -) -> anyhow::Result { - let applied_dir = crate::paths::applied_dir(agent); - if !applied_dir.join(".git").exists() { - anyhow::bail!( - "applied repo missing for agent '{agent}' (expected at {}) — \ - merge_config_pr requires the agent to be fully provisioned; \ - create it first (`swarmctl agent create`) before opening config PRs", - applied_dir.display() - ); - } - let repo = crate::forge::config_repo(agent); - // Verify the PR is still open before queueing an approval that would - // fail at approve time anyway (a closed/merged PR has no live head ref - // for the drift gate to compare against). - if !crate::forge::pr_is_open(&repo, pr_number) - .await - .map_err(|e| anyhow::anyhow!("check PR state for {agent} PR #{pr_number}: {e}"))? - { - anyhow::bail!( - "PR #{pr_number} on {repo} is closed or already merged — \ - merge_config_pr requires an open PR" - ); - } - // Fetch the current PR head sha — becomes the "reviewed" sha. - // Submitted together with the approval row (atomic single INSERT) so a - // crash mid-submit cannot leave a stranded sha-less row. - let sha = crate::forge::pr_head_sha(&repo, pr_number) - .await - .map_err(|e| anyhow::anyhow!("fetch PR head sha for {agent} PR #{pr_number}: {e}"))?; - // Both the webhook (`synchronize`) and the poll fallback call this on - // every PR update. If an approval for this PR is already pending, reconcile - // it against the live head sha rather than blindly queuing another: - // - same sha → the PR hasn't moved, so this is a duplicate signal — no-op. - // - drifted sha → the reviewed head is stale. Don't mutate the - // pending row in place (that races a concurrent approve); cancel it and - // fall through to queue a FRESH approval pinned to the new head. - if let Some((old_id, old_sha)) = coord.approvals.pending_merge_config_pr(agent, pr_number)? { - if old_sha.as_deref() == Some(sha.as_str()) { - return Ok(old_id); - } - let cancelled = coord - .approvals - .mark_cancelled(old_id, "config PR updated — superseded by a fresh approval") - .map_err(|e| anyhow::anyhow!("cancel superseded merge_config_pr approval: {e:#}"))?; - coord.emit_approval_resolved(crate::coordinator::ApprovalResolved { - id: old_id, - agent, - approval_kind: "merge_config_pr", - sha_short: old_sha.map(|s| s[..s.len().min(12)].to_owned()), - status: "cancelled", - note: Some("PR head moved; superseded by a fresh approval".to_owned()), - description: cancelled.description, - }); - } - let id = coord - .approvals - .submit_kind( - agent, - hive_sh4re::approvals::ApprovalKind::MergeConfigPr, - &pr_number.to_string(), - description, - submitter, - Some(&sha), // atomic: sha inserted with the row, not in a separate UPDATE - ) - .map_err(|e| anyhow::anyhow!("queue merge_config_pr approval row: {e:#}"))?; - let sha_short = sha[..sha.len().min(12)].to_owned(); - coord.emit_approval_added(crate::coordinator::ApprovalAdded { - id, - agent, - approval_kind: "merge_config_pr", - sha_short: Some(sha_short), - description: description.map(str::to_owned), - pr_number: Some(pr_number), - }); - Ok(id) -} diff --git a/hive-c0re/src/socket_server/mod.rs b/hive-c0re/src/socket_server/mod.rs index 06a39c2e..572a2403 100644 --- a/hive-c0re/src/socket_server/mod.rs +++ b/hive-c0re/src/socket_server/mod.rs @@ -21,14 +21,10 @@ use tokio::task::JoinHandle; use crate::coordinator::Coordinator; -mod config_approvals; mod schedules; -pub(crate) use config_approvals::submit_merge_config_pr; pub(crate) use schedules::filter_ghost_schedule_targets; pub use schedules::schedule_to_wire_public; -#[cfg(test)] -pub(crate) use schedules::tests::coordinator; use schedules::{ EditSchedulePatch, handle_cancel_schedule, handle_edit_schedule, handle_fire_schedule_now, @@ -735,15 +731,10 @@ fn handle_cancel_loose_end( .mark_cancelled(id, canceller) .map_err(|e| format!("{e:#}"))?; tracing::info!(%id, %canceller, agent = %approval.agent, "approval cancelled"); - let sha_short = approval - .fetched_sha - .as_deref() - .map(|s| s[..s.len().min(12)].to_owned()); coord.emit_approval_resolved(crate::coordinator::ApprovalResolved { id: approval.id, agent: approval.agent.as_str(), approval_kind: <&str>::from(approval.kind), - sha_short, status: "cancelled", note: approval.note, description: approval.description, diff --git a/hive-c0re/src/socket_server/schedules.rs b/hive-c0re/src/socket_server/schedules.rs index cce2829c..564e89c3 100644 --- a/hive-c0re/src/socket_server/schedules.rs +++ b/hive-c0re/src/socket_server/schedules.rs @@ -75,7 +75,6 @@ pub(super) fn handle_request_schedule_prompt( &commit_ref, payload.description.as_deref(), requester, - None, ) { Ok(id) => id, Err(e) => { @@ -96,9 +95,7 @@ pub(super) fn handle_request_schedule_prompt( id, agent: requester, approval_kind: "schedule_prompt", - sha_short: None, description: payload.description.clone(), - pr_number: None, }); Response::Ok } diff --git a/hive-c0re/src/stores/approvals.rs b/hive-c0re/src/stores/approvals.rs index 1e7be46f..1d7dd361 100644 --- a/hive-c0re/src/stores/approvals.rs +++ b/hive-c0re/src/stores/approvals.rs @@ -1,7 +1,6 @@ //! Approval queue. Requests are submitted by an agent -//! (`RequestSchedulePrompt`) or the config-PR webhook (`MergeConfigPr`); the -//! user approves/denies via the host admin CLI; on approval the host runs the -//! corresponding action. +//! (`RequestSchedulePrompt`); the user approves/denies via the host admin +//! CLI; on approval the host runs the corresponding action. //! //! `UpdateMetaInputs` rows are legacy: the MCP tool that queued them was //! removed and nothing produces the kind any more. The variant and @@ -58,6 +57,13 @@ const MIGRATIONS: &[Migration] = &[ sql: "ALTER TABLE approvals ADD COLUMN submitter TEXT", adds_column: Some(("approvals", "submitter")), }, + // v5: drop `merge_config_pr` rows. Config PRs merge on the forge, so no + // approval of that kind can be acted on, and `row_to_approval` rejects + // the kind. + Migration { + sql: "DELETE FROM approvals WHERE kind = 'merge_config_pr'", + adds_column: None, + }, ]; pub struct Approvals { @@ -75,10 +81,7 @@ impl Approvals { }) } - /// Insert a new pending approval row. `fetched_sha` may be supplied - /// when the sha is already known at submission time (e.g. `MergeConfigPr` - /// fetches the PR head before inserting), making the insert + sha-set - /// atomic. Pass `None` when the kind carries no sha (e.g. `SchedulePrompt`). + /// Insert a new pending approval row. pub fn submit_kind( &self, agent: &str, @@ -86,14 +89,12 @@ impl Approvals { commit_ref: &str, description: Option<&str>, submitter: &str, - fetched_sha: Option<&str>, ) -> Result { let conn = self.conn.lock().unwrap(); conn.execute( "INSERT INTO approvals - (agent, kind, commit_ref, requested_at, status, description, submitter, - fetched_sha) - VALUES (?1, ?2, ?3, ?4, 'pending', ?5, ?6, ?7)", + (agent, kind, commit_ref, requested_at, status, description, submitter) + VALUES (?1, ?2, ?3, ?4, 'pending', ?5, ?6)", params![ agent, <&str>::from(kind), @@ -101,7 +102,6 @@ impl Approvals { Utc::now().timestamp(), description, submitter, - fetched_sha, ], )?; Ok(conn.last_insert_rowid()) @@ -128,37 +128,12 @@ impl Approvals { Ok(submitter) } - /// Return the `(id, fetched_sha)` of the pending `merge_config_pr` - /// approval for `(agent, pr_number)`, if one exists. Drives - /// `submit_merge_config_pr`'s idempotency + PR-drift handling: same - /// `fetched_sha` → no new request (the webhook + poll both call submit, - /// so re-submits of an unchanged PR must be no-ops); a drifted head → - /// cancel this stale row and queue a fresh approval. - pub fn pending_merge_config_pr( - &self, - agent: &str, - pr_number: u64, - ) -> Result)>> { - let conn = self.conn.lock().unwrap(); - let row = conn - .query_row( - "SELECT id, fetched_sha FROM approvals \ - WHERE agent = ?1 AND kind = 'merge_config_pr' \ - AND commit_ref = ?2 AND status = 'pending' \ - ORDER BY id DESC LIMIT 1", - params![agent, pr_number.to_string()], - |row| Ok((row.get(0)?, row.get(1)?)), - ) - .optional()?; - Ok(row) - } - /// Last `limit` resolved approvals (approved / denied / failed), /// newest-first. Drives the history tab on the dashboard. pub fn recent_resolved(&self, limit: u64) -> Result> { let conn = self.conn.lock().unwrap(); let mut stmt = conn.prepare( - "SELECT id, agent, kind, commit_ref, requested_at, status, resolved_at, note, fetched_sha, description + "SELECT id, agent, kind, commit_ref, requested_at, status, resolved_at, note, description FROM approvals WHERE status IN ('approved', 'denied', 'failed', 'cancelled') ORDER BY resolved_at DESC, id DESC @@ -171,7 +146,7 @@ impl Approvals { pub fn pending(&self) -> Result> { let conn = self.conn.lock().unwrap(); let mut stmt = conn.prepare( - "SELECT id, agent, kind, commit_ref, requested_at, status, resolved_at, note, fetched_sha, description + "SELECT id, agent, kind, commit_ref, requested_at, status, resolved_at, note, description FROM approvals WHERE status = 'pending' ORDER BY id ASC", @@ -183,7 +158,7 @@ impl Approvals { pub fn get(&self, id: i64) -> Result> { let conn = self.conn.lock().unwrap(); conn.query_row( - "SELECT id, agent, kind, commit_ref, requested_at, status, resolved_at, note, fetched_sha, description + "SELECT id, agent, kind, commit_ref, requested_at, status, resolved_at, note, description FROM approvals WHERE id = ?1", params![id], row_to_approval, @@ -223,7 +198,6 @@ impl Approvals { status: ApprovalStatus::Approved, resolved_at: Some(hive_sh4re::wire_time::from_secs(resolved_at)), note: None, - fetched_sha: row.fetched_sha, description: row.description, }) } @@ -252,7 +226,7 @@ impl Approvals { /// Withdraw a pending approval. Returns the now-updated /// row so the caller can emit `ApprovalResolved` with the right - /// kind / agent / sha. Errors if the approval isn't pending — once + /// kind / agent. Errors if the approval isn't pending — once /// it's approved/denied/failed/cancelled, the resolution is final. pub fn mark_cancelled(&self, id: i64, canceller: &str) -> Result { let mut conn = self.conn.lock().unwrap(); @@ -286,7 +260,6 @@ impl Approvals { status: ApprovalStatus::Cancelled, resolved_at: Some(hive_sh4re::wire_time::from_secs(resolved_at)), note: Some(note), - fetched_sha: row.fetched_sha, description: row.description, }) } @@ -315,15 +288,14 @@ struct ApprovalLookup { commit_ref: String, requested_at: i64, status: String, - fetched_sha: Option, description: Option, } impl ApprovalLookup { /// The single-row lookup by id (`?1`). Column order matches /// [`ApprovalLookup::from_row`]. - const SELECT: &str = "SELECT agent, kind, commit_ref, requested_at, status, fetched_sha, \ - description FROM approvals WHERE id = ?1"; + const SELECT: &str = "SELECT agent, kind, commit_ref, requested_at, status, description \ + FROM approvals WHERE id = ?1"; fn from_row(row: &rusqlite::Row<'_>) -> rusqlite::Result { let agent: String = row.get(0)?; @@ -340,8 +312,7 @@ impl ApprovalLookup { commit_ref: row.get(2)?, requested_at: row.get(3)?, status: row.get(4)?, - fetched_sha: row.get(5)?, - description: row.get(6)?, + description: row.get(5)?, }) } } @@ -379,12 +350,11 @@ fn collect_lenient(rows: impl Iterator>) -> Ve } fn row_to_approval(row: &rusqlite::Row<'_>) -> rusqlite::Result { - // Column order: id, agent, kind, commit_ref, requested_at, status, resolved_at, note, fetched_sha, description. + // Column order: id, agent, kind, commit_ref, requested_at, status, resolved_at, note, description. let kind: String = row.get(2)?; let kind = match kind.as_str() { "update_meta_inputs" => ApprovalKind::UpdateMetaInputs, "schedule_prompt" => ApprovalKind::SchedulePrompt, - "merge_config_pr" => ApprovalKind::MergeConfigPr, other => { return Err(rusqlite::Error::FromSqlConversionFailure( 2, @@ -427,8 +397,7 @@ fn row_to_approval(row: &rusqlite::Row<'_>) -> rusqlite::Result { .get::<_, Option>(6)? .map(hive_sh4re::wire_time::from_secs), note: row.get(7)?, - fetched_sha: row.get(8)?, - description: row.get(9)?, + description: row.get(8)?, }) } @@ -436,7 +405,6 @@ fn kind_from_str(s: &str) -> Result { Ok(match s { "update_meta_inputs" => ApprovalKind::UpdateMetaInputs, "schedule_prompt" => ApprovalKind::SchedulePrompt, - "merge_config_pr" => ApprovalKind::MergeConfigPr, other => bail!("unknown approval kind '{other}'"), }) } @@ -456,21 +424,12 @@ mod tests { #[test] fn mixed_kinds_all_listed() { let (_dir, _path, db) = open_temp(); - db.submit_kind( - "a", - ApprovalKind::MergeConfigPr, - "deadbeef", - None, - "a", - None, - ) - .unwrap(); - db.submit_kind("b", ApprovalKind::SchedulePrompt, "", None, "b", None) + db.submit_kind("b", ApprovalKind::SchedulePrompt, "", None, "b") .unwrap(); - db.submit_kind("c", ApprovalKind::UpdateMetaInputs, "[]", None, "c", None) + db.submit_kind("c", ApprovalKind::UpdateMetaInputs, "[]", None, "c") .unwrap(); let pending = db.pending().expect("pending"); - assert_eq!(pending.len(), 3, "all three kinds must be visible"); + assert_eq!(pending.len(), 2, "both kinds must be visible"); } #[test] @@ -482,11 +441,10 @@ mod tests { let id = db .submit_kind( "bitburner", - ApprovalKind::MergeConfigPr, - "cafef00d", + ApprovalKind::SchedulePrompt, + "{}", Some("test"), "bitburner", - None, ) .unwrap(); let row = db.mark_cancelled(id, "manager").expect("cancel"); @@ -506,14 +464,7 @@ mod tests { // final — re-cancelling errors instead of silently overwriting. let (_dir, _path, db) = open_temp(); let id = db - .submit_kind( - "a", - ApprovalKind::MergeConfigPr, - "deadbeef", - None, - "a", - None, - ) + .submit_kind("a", ApprovalKind::SchedulePrompt, "{}", None, "a") .unwrap(); db.mark_cancelled(id, "manager").expect("first cancel"); let err = db @@ -528,14 +479,7 @@ mod tests { // whole list — collect_lenient skips it instead of failing. let (_dir, path, db) = open_temp(); let good = db - .submit_kind( - "good", - ApprovalKind::MergeConfigPr, - "cafe", - None, - "good", - None, - ) + .submit_kind("good", ApprovalKind::SchedulePrompt, "{}", None, "good") .unwrap(); let raw = Connection::open(&path).unwrap(); raw.execute( @@ -558,21 +502,14 @@ mod tests { // fall back to the root agent. let (_dir, path, db) = open_temp(); let id = db - .submit_kind( - "child", - ApprovalKind::MergeConfigPr, - "cafe", - None, - "parent", - None, - ) + .submit_kind("child", ApprovalKind::SchedulePrompt, "{}", None, "parent") .unwrap(); assert_eq!(db.submitter_of(id).unwrap().as_deref(), Some("parent")); let raw = Connection::open(&path).unwrap(); raw.execute( "INSERT INTO approvals (agent, kind, commit_ref, requested_at, status) - VALUES ('old', 'merge_config_pr', '', 0, 'pending')", + VALUES ('old', 'schedule_prompt', '', 0, 'pending')", [], ) .unwrap(); @@ -580,24 +517,33 @@ mod tests { assert_eq!(db.submitter_of(legacy_id).unwrap(), None); } + /// A database written before v5 still holds `merge_config_pr` rows, + /// pending and resolved. Opening it drops them and keeps every other row, + /// so the lists and `get` read cleanly. #[test] - fn fetched_sha_in_insert_is_readable_via_get() { - // `submit_kind` with `Some(sha)` must store it atomically in the - // INSERT — the `get()` row must reflect it without a separate - // sha-set step. This is the MergeConfigPr path. - let (_dir, _path, db) = open_temp(); - let sha = "abc1234567890abc1234567890abc1234567890ab"; - let id = db - .submit_kind( - "janet", - ApprovalKind::MergeConfigPr, - "42", - None, - "ruth", - Some(sha), - ) + fn a_pre_v5_merge_config_pr_row_is_dropped_on_open() { + let (_dir, path, db) = open_temp(); + let kept = db + .submit_kind("iris", ApprovalKind::SchedulePrompt, "{}", None, "iris") .unwrap(); - let row = db.get(id).unwrap().expect("row must exist"); - assert_eq!(row.fetched_sha.as_deref(), Some(sha)); + drop(db); + let raw = Connection::open(&path).unwrap(); + raw.execute_batch( + "INSERT INTO approvals (agent, kind, commit_ref, requested_at, status, fetched_sha) + VALUES ('iris', 'merge_config_pr', '42', 0, 'pending', 'abc123'); + INSERT INTO approvals (agent, kind, commit_ref, requested_at, status, resolved_at) + VALUES ('iris', 'merge_config_pr', '41', 0, 'approved', 1); + UPDATE schema_versions SET version = 4 WHERE store = 'approvals';", + ) + .unwrap(); + let old: i64 = raw.last_insert_rowid(); + drop(raw); + + let db = Approvals::open(&path).expect("a pre-v5 db opens"); + let pending = db.pending().expect("pending"); + assert_eq!(pending.len(), 1); + assert_eq!(pending[0].id, kept); + assert!(db.recent_resolved(10).unwrap().is_empty()); + assert!(db.get(old).unwrap().is_none()); } } diff --git a/hive-c0re/src/swarm_status.rs b/hive-c0re/src/swarm_status.rs index 0073207a..06b244c7 100644 --- a/hive-c0re/src/swarm_status.rs +++ b/hive-c0re/src/swarm_status.rs @@ -314,9 +314,9 @@ async fn handle_deploy_request( /// Move `agent`'s `applied/main` to `rev` ahead of its rebuild, and say /// whether that rebuild should be queued. A `rev` already applied is taken as -/// deployed or deploying — a dashboard merge deploys the PR it merges — so -/// no second rebuild is queued. That also skips a rev put in place by -/// `hivectl forge reconcile-config`, which does not deploy. +/// deployed or deploying, since every path that moves `applied/main` queues a +/// rebuild with it. The exception is `hivectl forge reconcile-config`, which +/// does not deploy, so a rev it put in place is skipped too. /// /// A refusal is commented on the PR that merged `rev`. A failed rebuild is /// not: it surfaces where every rebuild failure does, on this hive. diff --git a/hive-c0re/src/webhook_secret.rs b/hive-c0re/src/webhook_secret.rs deleted file mode 100644 index e4f44fca..00000000 --- a/hive-c0re/src/webhook_secret.rs +++ /dev/null @@ -1,295 +0,0 @@ -//! Webhook HMAC secret — load-or-generate, persist, verify. -//! -//! A 32-byte secret is generated on first startup, hex-encoded, and stored at -//! [`crate::paths::webhook_secret_file()`]. On subsequent starts the same file -//! is read back so Forgejo and hive-c0re always share the same key without any -//! operator configuration. -//! -//! The secret is used in two places: -//! - **Registration**: passed as the `secret` config key when hive-c0re -//! creates (or re-creates) the Forgejo org/repo webhook. -//! - **Verification**: each incoming webhook POST is verified against the -//! `X-Hub-Signature-256` header Forgejo attaches (`sha256=`). - -use anyhow::{Context as _, Result}; - -/// Load the webhook HMAC secret from disk; generate and persist it if absent. -/// -/// Returns a hex-encoded 32-byte secret string (64 hex chars). -pub fn load_or_generate() -> Result { - load_or_generate_at(&crate::paths::webhook_secret_file()) -} - -/// The path-taking half of [`load_or_generate`], split out so a test can -/// point it at a scratch file — same seam, for the same reason, as -/// `swarm-controller`'s `webhook::load_or_generate_at`. -fn load_or_generate_at(path: &std::path::Path) -> Result { - if let Ok(raw) = std::fs::read_to_string(path) { - let trimmed = raw.trim().to_owned(); - if trimmed.len() == 64 && trimmed.chars().all(|c| c.is_ascii_hexdigit()) { - return Ok(trimmed); - } - // File exists but is malformed — regenerate. - tracing::warn!( - path = %path.display(), - "webhook-secret file malformed (wrong length/chars); regenerating" - ); - } - let secret = generate_hex_secret()?; - std::fs::create_dir_all(path.parent().unwrap_or(path)) - .with_context(|| format!("create dir for {}", path.display()))?; - std::fs::write(path, format!("{secret}\n")) - .with_context(|| format!("write webhook secret to {}", path.display()))?; - tracing::info!(path = %path.display(), "webhook secret generated and persisted"); - Ok(secret) -} - -/// Whether `secret` is the one the config-PR hook was last registered with, -/// per [`crate::paths::forge_config_pr_webhook_fingerprint()`]. A missing or -/// unreadable record counts as "not registered". -pub fn is_registered(secret: &str) -> bool { - is_registered_at(&crate::paths::forge_config_pr_webhook_fingerprint(), secret) -} - -/// Record `secret` as the one the config-PR hook is now registered with. -pub fn record_registered(secret: &str) -> Result<()> { - record_registered_at(&crate::paths::forge_config_pr_webhook_fingerprint(), secret) -} - -fn is_registered_at(path: &std::path::Path, secret: &str) -> bool { - std::fs::read_to_string(path).is_ok_and(|raw| raw.trim() == fingerprint(secret)) -} - -fn record_registered_at(path: &std::path::Path, secret: &str) -> Result<()> { - std::fs::create_dir_all(path.parent().unwrap_or(path)) - .with_context(|| format!("create dir for {}", path.display()))?; - std::fs::write(path, format!("{}\n", fingerprint(secret))) - .with_context(|| format!("write webhook secret fingerprint to {}", path.display())) -} - -/// Hex SHA-256 of `secret` — stored in its place so the record on disk is -/// not a second copy of the key. -fn fingerprint(secret: &str) -> String { - use sha2::{Digest as _, Sha256}; - hex_encode(&Sha256::digest(secret.as_bytes())) -} - -/// Read 32 random bytes from `/dev/urandom` and hex-encode them. -fn generate_hex_secret() -> Result { - use std::io::Read as _; - - let mut buf = [0u8; 32]; - let mut f = - std::fs::File::open("/dev/urandom").context("open /dev/urandom for secret generation")?; - f.read_exact(&mut buf) - .context("read 32 bytes from /dev/urandom")?; - Ok(hex_encode(&buf)) -} - -/// Hex-encode `bytes` as a lowercase string. -fn hex_encode(bytes: &[u8]) -> String { - let mut out = String::with_capacity(bytes.len() * 2); - for b in bytes { - out.push(char::from_digit(u32::from(b >> 4), 16).unwrap_or('0')); - out.push(char::from_digit(u32::from(b & 0xf), 16).unwrap_or('0')); - } - out -} - -/// Verify a Forgejo `X-Hub-Signature-256` header against `body` using -/// `secret`. Returns `Ok(())` when the signature matches, or an error -/// describing the mismatch (safe to log; does not expose the secret). -/// -/// Forgejo sends: `sha256=`. -pub fn verify_signature(secret: &str, body: &[u8], header: &str) -> Result<()> { - use hmac::{Hmac, KeyInit, Mac}; - use sha2::Sha256; - - let sig_hex = header - .strip_prefix("sha256=") - .ok_or_else(|| anyhow::anyhow!("X-Hub-Signature-256 missing 'sha256=' prefix"))?; - - let expected = hex_decode(sig_hex) - .ok_or_else(|| anyhow::anyhow!("X-Hub-Signature-256 contains non-hex chars"))?; - - let mut mac = Hmac::::new_from_slice(secret.as_bytes()) - .map_err(|e| anyhow::anyhow!("HMAC key error: {e}"))?; - mac.update(body); - mac.verify_slice(&expected) - .map_err(|_| anyhow::anyhow!("X-Hub-Signature-256 mismatch")) -} - -/// Decode a lowercase hex string into bytes; returns `None` on invalid input. -fn hex_decode(s: &str) -> Option> { - if !s.len().is_multiple_of(2) { - return None; - } - let mut out = Vec::with_capacity(s.len() / 2); - let mut chars = s.chars(); - while let (Some(hi), Some(lo)) = (chars.next(), chars.next()) { - let hi = u8::try_from(hi.to_digit(16)?).ok()?; - let lo = u8::try_from(lo.to_digit(16)?).ok()?; - out.push((hi << 4) | lo); - } - Some(out) -} - -#[cfg(test)] -mod tests { - use super::{ - hex_encode, is_registered_at, load_or_generate_at, record_registered_at, verify_signature, - }; - - /// No record is the state of every hive before its first replacement, - /// and must read as "not registered" so that hook gets replaced once. - #[test] - fn a_secret_with_no_fingerprint_on_record_is_not_registered() { - let dir = tempfile::tempdir().expect("tempdir"); - let path = dir.path().join("forge").join("fingerprint"); - assert!(!is_registered_at(&path, &"a".repeat(64))); - } - - #[test] - fn a_recorded_secret_is_registered_and_a_changed_one_is_not() { - let dir = tempfile::tempdir().expect("tempdir"); - let path = dir.path().join("forge").join("fingerprint"); - let old = "a".repeat(64); - let new = "b".repeat(64); - - record_registered_at(&path, &old).expect("record"); - assert!( - is_registered_at(&path, &old), - "unchanged secret: keep the hook" - ); - assert!( - !is_registered_at(&path, &new), - "changed secret: replace the hook" - ); - assert!( - !std::fs::read_to_string(&path) - .expect("read back") - .contains(&old), - "the record must not be a copy of the secret" - ); - - record_registered_at(&path, &new).expect("re-record"); - assert!(is_registered_at(&path, &new)); - assert!(!is_registered_at(&path, &old)); - } - - /// Compute the `sha256=` header Forgejo would send for `secret` + - /// `body`, so the "matches" test below isn't asserting against a - /// hand-picked string that happens to look like a signature. - fn sign(secret: &str, body: &[u8]) -> String { - use hmac::{Hmac, KeyInit, Mac}; - use sha2::Sha256; - let mut mac = Hmac::::new_from_slice(secret.as_bytes()).unwrap(); - mac.update(body); - format!("sha256={}", hex_encode(&mac.finalize().into_bytes())) - } - - /// `verify_hmac` (`dashboard/webhook.rs`) delegates the actual HMAC - /// comparison here — this is the only gate on an inbound Forgejo - /// webhook reachable via the public gateway. A signature computed - /// with the right secret over the right body must verify. - #[test] - fn verify_signature_accepts_the_correct_hmac() { - let header = sign("s3cr3t", b"payload"); - assert!(verify_signature("s3cr3t", b"payload", &header).is_ok()); - } - - /// If `mac.verify_slice`'s result were ever inverted (accept on - /// mismatch), an unauthenticated actor could queue an operator - /// approval through the public webhook endpoint. Cover both ways a - /// signature can stop matching: wrong secret, and a body that no - /// longer matches the one that was signed. - #[test] - fn verify_signature_rejects_a_mismatched_hmac() { - let header = sign("s3cr3t", b"payload"); - assert!(verify_signature("different-secret", b"payload", &header).is_err()); - assert!(verify_signature("s3cr3t", b"tampered-payload", &header).is_err()); - } - - /// A stored secret that already parses must be handed back exactly as - /// written, on every start. This is the property Forgejo's copy depends - /// on: the secret is registered *with Forgejo* once, so a load that - /// rotates a perfectly good value silently invalidates every webhook - /// delivery afterwards — and it surfaces as an outage, not as anything - /// security-shaped. The trailing newline is the one this module writes - /// itself (`format!("{secret}\n")`), so the trim is load-bearing rather - /// than defensive: without it the file this code just wrote reads back - /// as 65 chars and fails its own validity check on the next boot. - #[test] - fn a_valid_stored_secret_is_returned_verbatim_and_never_rotated() { - let dir = tempfile::tempdir().expect("tempdir"); - let path = dir.path().join("webhook-secret"); - let seeded = "a".repeat(64); - std::fs::write(&path, format!("{seeded}\n")).expect("seed"); - - assert_eq!( - load_or_generate_at(&path).expect("load"), - seeded, - "a valid secret must be read back, not regenerated" - ); - assert_eq!( - load_or_generate_at(&path).expect("second load"), - seeded, - "and still on the next start" - ); - } - - /// The regeneration branch has two halves and only one of them is in the - /// return value: the replacement must also be *persisted*, or every - /// restart mints a fresh secret and the registration in Forgejo is never - /// the one being verified against. - #[test] - fn a_malformed_secret_file_is_replaced_by_a_valid_persisted_one() { - let dir = tempfile::tempdir().expect("tempdir"); - let path = dir.path().join("webhook-secret"); - std::fs::write(&path, "not-a-hex-secret\n").expect("seed"); - - let secret = load_or_generate_at(&path).expect("regenerates"); - assert_eq!(secret.len(), 64, "hex-encoded 32 bytes"); - assert!(secret.chars().all(|c| c.is_ascii_hexdigit())); - assert_eq!( - std::fs::read_to_string(&path).expect("read back").trim(), - secret, - "the regenerated secret must reach disk, not just the caller" - ); - assert_eq!( - load_or_generate_at(&path).expect("second load"), - secret, - "and must then be stable across a restart" - ); - } - - /// The length + charset check is what stops a truncated or half-written - /// file from being adopted as the HMAC key. `Hmac::new_from_slice` - /// accepts a key of *any* length, empty included — so relaxing this - /// check does not fail anywhere, it just silently keys every signature - /// off a value an attacker can guess. Each near miss is listed - /// separately so a check that stops distinguishing one of them shows up - /// as that case rather than as a single opaque failure. - #[test] - fn a_near_miss_secret_file_is_not_adopted_as_the_key() { - for (label, seeded) in [ - ("empty file", String::new()), - ("whitespace only", " \n".to_owned()), - ("one hex digit short", "b".repeat(63)), - ("one hex digit long", "b".repeat(65)), - ("right length, non-hex char", format!("z{}", "b".repeat(63))), - ] { - let dir = tempfile::tempdir().expect("tempdir"); - let path = dir.path().join("webhook-secret"); - std::fs::write(&path, &seeded).expect("seed"); - - let secret = load_or_generate_at(&path).expect("regenerates"); - assert_ne!(secret, seeded.trim(), "{label} must not become the key"); - assert_eq!(secret.len(), 64, "{label}: replacement is 64 hex chars"); - assert!( - secret.chars().all(|c| c.is_ascii_hexdigit()), - "{label}: replacement is hex" - ); - } - } -} diff --git a/hive-c0re/src/workers/auto_update.rs b/hive-c0re/src/workers/auto_update.rs index ffad6354..ed46f59a 100644 --- a/hive-c0re/src/workers/auto_update.rs +++ b/hive-c0re/src/workers/auto_update.rs @@ -453,14 +453,13 @@ fn boot_action(wanted: Option, fresh: bool, running: bool) } } -/// Submit the boot-time forge/matrix/webhook/knowledge/wanted-state sweeps as -/// DAG nodes — `ForgeSweep`, `MatrixSweep`, `WebhookRegister`, -/// `KnowledgePull`, `WantedPull`. Unlike +/// Submit the boot-time forge/matrix/knowledge/wanted-state sweeps as DAG +/// nodes — `ForgeSweep`, `MatrixSweep`, `KnowledgePull`, `WantedPull`. Unlike /// [`submit_boot_tree`] this runs on **every** boot, quiet or not: these /// aren't config-drift work, they're startup housekeeping that always needs /// to happen, and the point of moving them here is exactly so they show up /// as real work on the dashboard instead of an invisible `tokio::spawn` that -/// only surfaces on failure. Five independent, build-slot- and lease-exempt +/// only surfaces on failure. Four independent, build-slot- and lease-exempt /// roots — no dependency edges between them, matching the existing /// `Reconcile`-root pattern in [`boot_nodes`]. `MatrixSweep` and /// `KnowledgePull` hold their sweep's own resource, so each queues behind any @@ -471,7 +470,6 @@ fn submit_startup_sweep_nodes(coord: &Arc) { if let Err(e) = coord.job_queue.insert_job(|b| { let _ = b.node(NodeKind::ForgeSweep); let _ = crate::job_queue::templates::matrix_sweep(b); - let _ = b.node(NodeKind::WebhookRegister); let _ = crate::job_queue::templates::knowledge_pull(b); let _ = b.node(NodeKind::WantedPull); Vec::new() diff --git a/hive-forge/src/client.rs b/hive-forge/src/client.rs index d7a8ea32..b694fade 100644 --- a/hive-forge/src/client.rs +++ b/hive-forge/src/client.rs @@ -23,7 +23,7 @@ const DEFAULT_URL: &str = "http://localhost:3000"; /// Bound on reaching the forge. const HTTP_CONNECT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5); /// Whole-request budget for a small JSON call (an `/api/v1` GET, or the -/// run-view log streamer's snapshot POST) — `config_pr_poll`'s forge budget. +/// run-view log streamer's snapshot POST). const JSON_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(15); /// Whole-request budget for a GET that downloads a file body — an /// attachment, an Actions artifact zip, or a persisted job log. Sized diff --git a/hive-sh4re/src/approvals.rs b/hive-sh4re/src/approvals.rs index 70398cc7..14f75475 100644 --- a/hive-sh4re/src/approvals.rs +++ b/hive-sh4re/src/approvals.rs @@ -14,17 +14,10 @@ use serde::{Deserialize, Serialize}; pub struct Approval { pub id: i64, pub agent: Ident, - #[serde(default)] pub kind: ApprovalKind, /// Kind-specific payload (git sha / inputs array / schedule /// payload / empty). See the Approval struct doc. pub commit_ref: String, - /// The canonical hive-c0re-vouched sha. For `MergeConfigPr`: the - /// reviewed PR head pinned at submit; if the PR head drifts off it - /// before merge, hive-c0re cancels the stale approval and re-queues a - /// fresh one for re-review. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub fetched_sha: Option, pub requested_at: DateTime, pub status: ApprovalStatus, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -40,9 +33,7 @@ pub struct Approval { /// What action the approval, when granted, will trigger. /// Variant-specific payload encoding + flow lives in /// `docs/agent-lifecycle/approvals.md::Approval kinds (wire shapes)`. -#[derive( - Debug, Clone, Copy, Default, Serialize, Deserialize, PartialEq, Eq, strum::IntoStaticStr, -)] +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, strum::IntoStaticStr)] #[serde(rename_all = "snake_case")] #[strum(serialize_all = "snake_case")] pub enum ApprovalKind { @@ -51,15 +42,6 @@ pub enum ApprovalKind { UpdateMetaInputs, /// Add a scheduled prompt to the broker queue. SchedulePrompt, - /// Merge an operator-reviewed config PR: hive-c0re verifies the - /// reviewed PR head, fast-forwards the forge config repo's `main` - /// to it, marks the PR merged, then runs the deploy tail. This is the - /// sole config-change flow — a manager opens a PR on its - /// `agent-configs/` repo and the operator reviews + approves it. - /// `commit_ref` = PR number; `fetched_sha` = the reviewed PR head - /// pinned at submit. See `docs/agent-lifecycle/approvals.md`. - #[default] - MergeConfigPr, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] diff --git a/hive-sh4re/src/manager.rs b/hive-sh4re/src/manager.rs index c21ed357..df82c18b 100644 --- a/hive-sh4re/src/manager.rs +++ b/hive-sh4re/src/manager.rs @@ -55,10 +55,6 @@ pub enum HelperEvent { status: ApprovalStatus, #[serde(default, skip_serializing_if = "Option::is_none")] note: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - sha: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - tag: Option, }, /// A sub-agent's recorded flake rev is stale relative to hyperhive. NeedsUpdate { agent: String }, diff --git a/nix/host-modules/hive-gateway/vhosts.nix b/nix/host-modules/hive-gateway/vhosts.nix index ed915dd7..fa46a723 100644 --- a/nix/host-modules/hive-gateway/vhosts.nix +++ b/nix/host-modules/hive-gateway/vhosts.nix @@ -135,9 +135,7 @@ let # Shared auth block — separate locations don't inherit auth_basic, so # each dashboard location (`/`, `/api/`) needs it or that surface is - # unauthed. `/webhook/` is intentionally excluded: Forgejo cannot - # send HTTP Basic credentials with webhook deliveries, and the HMAC - # secret (`X-Hub-Signature-256`) protects those endpoints instead. + # unauthed. dashboardAuth = lib.optionalString cfg.auth.enable '' auth_basic "${cfg.auth.realm}"; auth_basic_user_file /var/lib/hive-gateway/conf/gateway.htpasswd; @@ -147,15 +145,14 @@ let ''; # Dashboard: nginx static-serves the dist, c0re is API-only. Routing - # is by PATH, never content-type. c0re serves exactly three prefixes — - # `/api/` (all dashboard data + actions + the SSE streams), `/webhook/` - # (knowledge push + config-PR approval triggers, HMAC-guarded), and + # is by PATH, never content-type. c0re serves exactly two prefixes — + # `/api/` (all dashboard data + actions + the SSE streams) and # `/health/` (liveness + readiness) — so those proxy to c0re and # everything else serves the dist with an SPA fallback to index.html. # Path routing is deterministic where an Accept-header split would make # the SAME url behave differently by content-type (e.g. `/api/state` # fetched with `Accept: text/html` wrongly getting index.html). A new - # top-level c0re route prefix (beyond /api + /webhook + /health) needs + # top-level c0re route prefix (beyond /api + /health) needs # a matching location added here. dashboardProxyLocation = { "/" = { @@ -176,15 +173,8 @@ let ${dashboardAuth} ''; }; - "/webhook/" = { - # No dashboardAuth here: Forgejo cannot send HTTP Basic credentials - # with webhook deliveries. HMAC (X-Hub-Signature-256) is the auth - # for these endpoints; hive-c0re verifies it in the handler. - proxyPass = "http://${cfg.upstreamHost}:${toString cfg.upstreamPort}"; - }; "/health/" = { - # No dashboardAuth here either, for a different reason than - # /webhook/: an external uptime monitor generally can't do + # No dashboardAuth here: an external uptime monitor generally can't do # interactive HTTP Basic. The endpoints themselves are scoped to # status + warning kind/message (see hive-c0re/src/dashboard/ # health.rs) — no tokens, no agent detail — so exposing them diff --git a/swarm-controller/src/config_pr.rs b/swarm-controller/src/config_pr.rs index 2f466e6d..61ef56c3 100644 --- a/swarm-controller/src/config_pr.rs +++ b/swarm-controller/src/config_pr.rs @@ -1,18 +1,15 @@ //! Swarm-level config-PR status, kept current by both a webhook nudge and a //! periodic poll. //! -//! `hive-c0re::forge::config_pr_poll` already scans `agent-configs/*` for -//! open PRs — but it does that once per hive, to queue that hive's own -//! `MergeConfigPr` approval, and nothing at swarm level reads the result. //! swarm-ui's config-PR panel needs a *swarm*-level answer to "does agent X //! have an open config PR" that does not depend on which hive currently //! hosts X being reachable. //! //! Both paths write the same [`ConfigPrCache`]: //! -//! - [`spawn`] — a periodic full rescan, mirroring `hive-c0re`'s own poll -//! shape. The backstop: catches anything a missed delivery loses, and is -//! what populates the cache before the first delivery ever arrives. +//! - [`spawn`] — a periodic full rescan. The backstop: catches anything a +//! missed delivery loses, and is what populates the cache before the first +//! delivery ever arrives. //! - [`ConfigPrCache::apply_webhook_delivery`] — called from //! `crate::webhook::post_webhook_forge` on a verified `ConfigPr` delivery. //! The low-latency path: a PR opening or closing shows up immediately @@ -21,11 +18,6 @@ //! [`merged`] reads the same delivery for a merge, which the webhook handler //! turns into a deploy. The poll has no counterpart: it lists open PRs only, //! so a merge whose delivery is lost deploys nothing. -//! -//! Per mara's review call: ship both from the start rather than the poll -//! alone — the eventual swarm-level replacement for `hive-c0re`'s own -//! poll+webhook pair needs both anyway, so building only half here would be -//! work redone rather than work reused. use std::collections::HashMap; use std::sync::{Arc, Mutex}; @@ -119,9 +111,7 @@ struct WebhookRepository { name: String, } -/// How often to rescan `agent-configs/*`. Matches the interval named in -/// `hive-c0re::forge::config_pr_poll`'s own doc comment — same org, same -/// staleness tolerance, no reason for the two to disagree. +/// How often to rescan `agent-configs/*`. const POLL_INTERVAL: std::time::Duration = std::time::Duration::from_mins(5); /// The latest full scan, replaced atomically each cycle. diff --git a/swarm-controller/src/forge.rs b/swarm-controller/src/forge.rs index e0a1722b..3f1aec1a 100644 --- a/swarm-controller/src/forge.rs +++ b/swarm-controller/src/forge.rs @@ -603,10 +603,7 @@ impl Client { } /// Every agent in [`CONFIG_ORG`] with an open config PR, keyed by agent - /// name. Mirrors `hive-c0re::forge::config_pr_poll::poll_open_config_prs`'s - /// scan shape (list repos in the org, list open PRs per repo) but returns - /// data instead of side-effecting an approval queue — this daemon has no - /// approval system of its own; it exists so `GET + /// name: list repos in the org, then open PRs per repo. It exists so `GET /// /api/agents/{name}/config-pr` has something to answer from, /// independent of any one hive being up. /// @@ -649,9 +646,7 @@ impl Client { } }; // Only the first open PR matters for the panel — a config repo - // is meant to carry at most one live proposal at a time (the - // same assumption `hive-c0re`'s poller and the `MergeConfigPr` - // approval flow both make). + // is meant to carry at most one live proposal at a time. if let Some(pr) = prs.into_iter().next() { let Some(pr_number) = pr.number.and_then(|n| u64::try_from(n).ok()) else { continue; @@ -900,19 +895,9 @@ impl Client { /// forge event reaches [`crate::webhook`] instead of the endpoint only /// being reachable by hand. /// - /// **These are registered ALONGSIDE the per-hive hooks, not instead of - /// them.** Every hive keeps receiving and acting on its own deliveries - /// exactly as today; the controller receives a copy and (for now) logs - /// it. Taking the hive-side registration away is a later step, and it - /// has to be later: fan-out swarm→hive does not exist yet, so a hook - /// moved now would point at a receiver that forwards nowhere — silent on - /// both sides, indistinguishable from no activity. - /// - /// ⛔ **No stale-hook deletion arm, unlike the two per-hive registrars - /// this otherwise mirrors.** Their arm deletes hooks matching their own - /// path with a foreign base; copying it here would delete the hives' - /// live hooks, which are not stale — they are the path still in - /// production. The controller only ever adds its own. + /// Adds its own hooks only. A hive's `/webhook/config-pr` hook left on + /// the `agent-configs` org by an older release stays registered and + /// delivers to a route no hive serves. /// /// Idempotent: an existing hook with the same `target_url` is left /// alone, so this is safe on every boot. @@ -921,8 +906,7 @@ impl Client { /// /// Returns the first failure. A listing failure is not fatal — it falls /// through to the create attempt, which is idempotent server-side by way - /// of the already-exists fold, the same best-effort shape `hive-c0re` - /// uses. + /// of the already-exists fold. pub async fn ensure_swarm_webhooks(&self, public_base: &str, secret: &str) -> Result<()> { for kind in DeliveryKind::ALL { let target_url = kind.target_url(public_base); @@ -989,7 +973,7 @@ impl Client { ); } Err(e) => { - // Best-effort, same as the per-hive registrars: a forge that + // Best-effort: a forge that // cannot be listed may still accept a create, and a // duplicate create is folded into success below. `warn!` // because that fold is an assumption about the forge, not a @@ -1054,8 +1038,7 @@ fn transitive_reach(start: i64, adj: &HashMap>) -> usize { } /// Where a hook lives. The knowledge hook is repo-scoped and the config-PR -/// hook is org-scoped, mirroring exactly where the per-hive registrars put -/// theirs — a hook on the wrong scope would never fire, and forgejo would +/// hook is org-scoped — a hook on the wrong scope would never fire, and forgejo would /// report that as a perfectly healthy hook with no deliveries. enum HookScope<'a> { Repo { diff --git a/swarm-controller/src/webhook.rs b/swarm-controller/src/webhook.rs index e9e9e9b6..353b9c25 100644 --- a/swarm-controller/src/webhook.rs +++ b/swarm-controller/src/webhook.rs @@ -10,16 +10,6 @@ //! the payload and emits a semantic message — *the knowledge repo changed*, //! *deploy agent X at rev Y* — addressed to the hives that need it. One place //! decides what a forge payload means, so no hive re-derives it. -//! -//! Receipt is all that is wired today: the payload is opaque bytes keyed by a -//! [`DeliveryKind`] from the URL path, and nothing consumes it until the -//! swarm→hive channel exists. -//! -//! The HMAC code is deliberately **not** shared with `hive-c0re`: that copy is -//! leaving, and a shared crate is right only when a second consumer arrives. -//! -//! **These hooks are registered ALONGSIDE the per-hive ones** — see -//! [`crate::forge::Client::ensure_swarm_webhooks`]. use anyhow::{Context as _, Result}; use axum::{ @@ -199,15 +189,11 @@ pub(super) enum DeliveryKind { /// The route prefix a registered `target_url` must point at. /// -/// ⚠️ Deliberately **not** `/webhook/knowledge` or `/webhook/config-pr`, the -/// paths the per-hive receivers use. A hive-side registrar deletes any hook -/// whose URL ends with *its* path but has a different base — see -/// `hive-c0re`'s `forge::ensure_config_pr_webhook`. A swarm-level hook under -/// such a path would therefore be deleted by every hive on every boot, and -/// the symptom is a hook that silently stops existing. -/// `webhook_urls_survive_the_hive_side_reapers` pins that, and its own doc -/// records why `/webhook/knowledge` stays in the check even though the -/// knowledge registrar no longer reaps. +/// ⚠️ Deliberately **not** `/webhook/knowledge` or `/webhook/config-pr`. +/// Older `hive-c0re` releases delete, on every boot, any hook whose URL ends +/// with one of those paths on a base other than their own; a swarm-level +/// hook there would silently stop existing while such a hive runs. +/// `webhook_urls_survive_the_hive_side_reapers` pins that. const ROUTE_PREFIX: &str = "/webhook/forge/"; impl DeliveryKind { @@ -661,22 +647,18 @@ mod tests { } } - /// A cross-daemon invariant with nothing else to enforce it: a per-hive - /// registrar in `hive-c0re` **deletes** hooks whose URL ends with its - /// own path but carries a different base. A controller URL matching such - /// a suffix would be deleted by every hive on every boot — the swarm hook - /// would simply cease to exist, with the cause in a different daemon's - /// startup sweep. Serving these under `/webhook/forge/` is what avoids - /// it, and this is the only place that says so in a form that fails. + /// A cross-daemon invariant with nothing else to enforce it: older + /// `hive-c0re` releases **delete** hooks whose URL ends with + /// `/webhook/knowledge` or `/webhook/config-pr` but carries a base other + /// than their own. A controller URL matching such a suffix would be + /// deleted by every such hive on every boot, with the cause in a + /// different daemon's startup sweep. Serving these under + /// `/webhook/forge/` is what avoids it, and this is the only place that + /// says so in a form that fails. /// - /// `forge::ensure_config_pr_webhook` still reaps that way. The knowledge - /// registrar no longer does — it was replaced by a removal that matches - /// the full URL, so it cannot touch another hive's hook. **The - /// `/webhook/knowledge` arm is kept anyway**, because the hazard is not - /// this repository's current code: it is whatever is *deployed*, and a - /// hive still running the previous version reaps by suffix until it is - /// upgraded. Drop that arm once no such hive can exist, not when the - /// source stops mentioning it. + /// Current `hive-c0re` removes only hooks matching its own full URL. The + /// hazard is what is *deployed*, not this tree: drop this test once no + /// hive running an older release can exist. #[test] fn webhook_urls_survive_the_hive_side_reapers() { for suffix in ["/webhook/knowledge", "/webhook/config-pr"] {