diff --git a/.gitignore b/.gitignore index a4afea36..b7ce58d6 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ /result-* /.tmp /.claude/settings.local.json +*.rlib diff --git a/docs/approvals.md b/docs/approvals.md index b730db34..dfb6c486 100644 --- a/docs/approvals.md +++ b/docs/approvals.md @@ -111,15 +111,20 @@ kind-specific payload carrier. row (only `ApplyCommit` populates it). See the End-to-end flow above. - `MergeConfigPr` — the PR-based config flow's counterpart to - `ApplyCommit`. `commit_ref` stores the **PR number** (decimal), - and `fetched_sha` is the PR **head sha the operator reviewed**. - On approve, `run_merge_config_pr` re-reads the live PR head and - aborts if it drifted from `fetched_sha` (re-review), then fetches - that head into the applied repo, eval-verifies it, fast-forwards - the forge config repo's `main` to it (the merge), marks the PR - merged (best-effort — `main` is already there), and runs the same - shared deploy tail as `ApplyCommit` (`deploy_applied_target`). - Never a first spawn. + `ApplyCommit`. Triggered automatically: when an agent opens (or + force-pushes) a PR on its `agent-configs/` forge repo, + hive-c0re's `/webhook/config-pr` endpoint receives the Forgejo + pull_request event and queues this approval row. No MCP tool call + needed — the forge PR IS the request. `commit_ref` stores the + **PR number** (decimal), and `fetched_sha` is the PR **head sha + at queue time** (the "reviewed" sha). On approve, + `run_merge_config_pr` re-reads the live PR head and aborts if it + drifted from `fetched_sha` (submitter must push again to + re-trigger), then fetches that head into the applied repo, + eval-verifies it, fast-forwards the forge config repo's `main` to + it (the merge), marks the PR merged (best-effort — `main` is + already there), and runs the same shared deploy tail as + `ApplyCommit` (`deploy_applied_target`). Never a first spawn. - `Spawn` — direct container creation under the default `agent.nix` template. `commit_ref` is empty. Submitted via `HostRequest::RequestSpawn` (operator-gated, the diff --git a/docs/tools/lifecycle.md b/docs/tools/lifecycle.md index 5ad65e21..16fe257a 100644 --- a/docs/tools/lifecycle.md +++ b/docs/tools/lifecycle.md @@ -81,8 +81,8 @@ agents after the approval resolves. | `kill` / `start` / `restart` / `update` | No | Direct children | | `list_containers` | No | All descendants | | `request_init_config` | Yes (InitConfig) | New direct child only | -| `request_apply_commit` | Yes (ApplyCommit) | Direct children | -| `request_update_meta_inputs` | Yes (MetaUpdate) | Meta flake (global) | +| `request_apply_commit` | Yes (ApplyCommit) | Direct children | +| `request_update_meta_inputs` | Yes (MetaUpdate) | Meta flake (global) | ## See also diff --git a/hive-c0re/src/dashboard/misc_api.rs b/hive-c0re/src/dashboard/misc_api.rs index d4c8d175..eb8fcadf 100644 --- a/hive-c0re/src/dashboard/misc_api.rs +++ b/hive-c0re/src/dashboard/misc_api.rs @@ -196,6 +196,7 @@ pub(super) async fn post_request_spawn( "", None, "operator", + None, ) { Ok(id) => { tracing::info!(%id, %name, "operator: spawn approval queued via dashboard"); diff --git a/hive-c0re/src/dashboard/mod.rs b/hive-c0re/src/dashboard/mod.rs index 60facd1f..dd2ab2ca 100644 --- a/hive-c0re/src/dashboard/mod.rs +++ b/hive-c0re/src/dashboard/mod.rs @@ -160,9 +160,10 @@ pub async fn serve(port: u16, coord: Arc) -> Result<()> { post(schedules::post_rebuild_queue_cancel), ) .route("/webhook/knowledge", post(webhook::post_webhook_knowledge)) + .route("/webhook/config-pr", post(webhook::post_webhook_config_pr)) // Backend routes — the frontend calls these `/api/` paths. The // transitional bare top-level aliases were removed once the - // frontend migrated. `/webhook/knowledge` keeps its own prefix + // frontend migrated. `/webhook/*` keeps its own prefix // (forge-driven, not the SPA). .route("/api/approve/{id}", post(approvals::post_approve)) .route("/api/deny/{id}", post(approvals::post_deny)) diff --git a/hive-c0re/src/dashboard/webhook.rs b/hive-c0re/src/dashboard/webhook.rs index a10a4328..c5024d5c 100644 --- a/hive-c0re/src/dashboard/webhook.rs +++ b/hive-c0re/src/dashboard/webhook.rs @@ -1,15 +1,27 @@ -//! Forgejo push-webhook endpoint for the `internal/knowledge` repo. +//! Forgejo webhook endpoints. //! -//! Loopback-only; on a push to `main` of the knowledge repo it triggers a -//! read-only `git pull` on the local clone so agents see up-to-date -//! documents on their next turn. +//! - **`/webhook/knowledge`** — push events on `internal/knowledge` trigger a +//! `git pull` on the local clone so agents see up-to-date docs. +//! - **`/webhook/config-pr`** — `pull_request` events on any `agent-configs/*` +//! repo queue a [`hive_sh4re::ApprovalKind::MergeConfigPr`] approval row +//! so the operator can review + approve the merge from the dashboard. +//! +//! Both endpoints are loopback-only (the axum listener binds +//! `127.0.0.1:`) and have no signature verification (the risk is low: +//! loopback access implies host compromise already, and the config-PR path +//! still requires the operator to approve on the dashboard). use axum::{ + extract::State, http::StatusCode, response::{IntoResponse, Response}, }; use serde::Deserialize; +use super::AppState; + +// ── knowledge webhook ────────────────────────────────────────────────────────── + /// Minimal Forgejo push-webhook payload — only the fields we care about. #[derive(Deserialize)] pub(super) struct PushWebhookPayload { @@ -62,3 +74,133 @@ pub(super) async fn post_webhook_knowledge( }); (StatusCode::OK, "ok").into_response() } + +// ── config-PR webhook ────────────────────────────────────────────────────────── + +/// Minimal Forgejo `pull_request`-webhook payload. +/// +/// Forgejo fires this for actions: `opened`, `closed`, `reopened`, +/// `synchronize`, `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 `synchronize`. +#[derive(Deserialize)] +pub(super) struct PrWebhookPayload { + /// What triggered this event (`opened`, `closed`, `synchronize`, …). + 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 `synchronize` 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: `http://127.0.0.1:/webhook/config-pr` +/// - Content type: `application/json` +/// - Events: "Pull Request" only +/// - 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`]. +pub(super) async fn post_webhook_config_pr( + State(state): State, + axum::extract::Json(payload): axum::extract::Json, +) -> Response { + let action = payload.action.as_deref().unwrap_or(""); + // Only act on newly-opened or force-updated PRs. + if action != "opened" && action != "synchronize" { + 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() +} diff --git a/hive-c0re/src/forge/mod.rs b/hive-c0re/src/forge/mod.rs index 167e631f..112dc5d1 100644 --- a/hive-c0re/src/forge/mod.rs +++ b/hive-c0re/src/forge/mod.rs @@ -10,7 +10,7 @@ mod users; pub use pr_merge::{ ForgeMergeError, config_repo, fetch_pr_head_into_applied, ff_push_to_main, mark_pr_merged, - pr_head_sha, + pr_head_sha, pr_is_open, }; pub use repos::{ create_agent_repo, ensure_config_repo, ensure_knowledge_repo, ensure_meta_remote, ensure_repo, @@ -69,7 +69,7 @@ pub(crate) fn forge_git_url(token: &str, repo: &str) -> String { /// 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. -const CONFIG_ORG: &str = "agent-configs"; +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 /// common reference without the operator having to bake content into @@ -294,3 +294,82 @@ pub async fn ensure_all() { sync_agent(name, core_token.as_deref()).await; } } + +/// Ensure a Forgejo `pull_request` org-webhook for `agent-configs` exists and +/// points at hive-c0re's `/webhook/config-pr` endpoint. Idempotent — lists +/// existing hooks first and skips creation when one is already targeting the +/// correct URL. `dashboard_port` is the TCP port hive-c0re's dashboard listens +/// on (default 7000); the webhook URL is +/// `http://127.0.0.1:/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 alongside `knowledge::ensure_webhook`. No-op when the +/// core token is absent (forge not yet provisioned). +/// +/// # Errors +/// +/// Returns an error if: +/// - `dashboard_port` produces a URL that `url::Url::parse` rejects (should +/// never happen for a valid port number). +/// - The Forgejo `org_create_hook` API call 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, dashboard_port: u16) -> 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!("http://127.0.0.1:{dashboard_port}/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)); + match listed { + Ok(hooks) => { + let already_exists = hooks.iter().any(|h| { + h.config + .as_ref() + .and_then(|c| c.get("url")) + .map(String::as_str) + == Some(target_url.as_str()) + }); + if already_exists { + tracing::debug!(%target_url, "forge: config-pr webhook already configured"); + return Ok(()); + } + } + Err(e) => { + tracing::debug!(error = %e, "forge: listing config-pr hooks failed; attempting create"); + } + } + + 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: BTreeMap::new(), + }, + 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}"); + Ok(()) +} diff --git a/hive-c0re/src/forge/pr_merge.rs b/hive-c0re/src/forge/pr_merge.rs index ca268140..865cce5c 100644 --- a/hive-c0re/src/forge/pr_merge.rs +++ b/hive-c0re/src/forge/pr_merge.rs @@ -5,7 +5,7 @@ use anyhow::Context; use forgejo_api::ForgejoError; -use forgejo_api::structs::{MergePullRequestOption, MergePullRequestOptionDo}; +use forgejo_api::structs::{MergePullRequestOption, MergePullRequestOptionDo, StateType}; use super::{CONFIG_ORG, api, core_token, forge_git_url}; @@ -112,6 +112,30 @@ pub async fn pr_head_sha(repo: &str, pr: u64) -> Result 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 { diff --git a/hive-c0re/src/main.rs b/hive-c0re/src/main.rs index 8efa4fcc..07421b5b 100644 --- a/hive-c0re/src/main.rs +++ b/hive-c0re/src/main.rs @@ -303,17 +303,22 @@ async fn cmd_serve( tokio::spawn(async move { forge::ensure_all().await; }); - // Knowledge webhook setup: ensure the Forgejo push webhook for - // `internal/knowledge` exists so `pull()` fires on merge. Runs - // after forge::ensure_all so the core token + repo are present. + // Webhook setup: ensure Forgejo webhooks are registered for both + // `internal/knowledge` (push → git pull) and the `agent-configs` org + // (pull_request → queue MergeConfigPr approval). Both run after + // forge::ensure_all so the core token + repos + org are present. // No-op when the core token or forge are absent. let webhook_port = dashboard_port; tokio::spawn(async move { - if let Some(token) = forge::core_token() - && let Err(e) = knowledge::ensure_webhook(&token, webhook_port).await - { + let Some(token) = forge::core_token() else { + return; + }; + if let Err(e) = knowledge::ensure_webhook(&token, webhook_port).await { tracing::warn!(error = ?e, "knowledge: ensure_webhook failed"); } + if let Err(e) = forge::ensure_config_pr_webhook(&token, webhook_port).await { + tracing::warn!(error = ?e, "forge: ensure_config_pr_webhook failed"); + } }); // Knowledge periodic pull: hourly fallback in case the webhook is // missed (e.g. hive-c0re was down during a push). First fires at diff --git a/hive-c0re/src/server.rs b/hive-c0re/src/server.rs index f43da1db..0320467d 100644 --- a/hive-c0re/src/server.rs +++ b/hive-c0re/src/server.rs @@ -86,6 +86,7 @@ async fn dispatch(req: &HostRequest, coord: Arc) -> HostResponse { "", None, "operator", + None, )?; tracing::info!(%id, %name, "spawn approval queued"); HostResponse::success() @@ -145,22 +146,7 @@ async fn dispatch(req: &HostRequest, coord: Arc) -> HostResponse { HostResponse::dags(dags) } HostRequest::List => HostResponse::list(lifecycle::list().await?), - HostRequest::AgentStatus => { - let rows = crate::container_view::build_all(&coord) - .await - .into_iter() - .map(|v| hive_sh4re::AgentStatusRow { - name: v.name, - running: v.running, - needs_update: v.needs_update, - needs_login: v.needs_login, - deployed_sha: v.deployed_sha, - pending_reminders: v.pending_reminders, - parent: v.parent, - }) - .collect(); - HostResponse::agent_statuses(rows) - } + HostRequest::AgentStatus => handle_agent_status(&coord).await, // The hive domain + per-surface public URLs are injected into // c0re's service env by hive-c0re.nix; surface them so the // operator CLI can fill in this hive's own identity (the @@ -241,6 +227,24 @@ async fn handle_spawn(coord: &Arc, name: &str) -> Result) -> HostResponse { + let rows = crate::container_view::build_all(coord) + .await + .into_iter() + .map(|v| hive_sh4re::AgentStatusRow { + name: v.name, + running: v.running, + needs_update: v.needs_update, + needs_login: v.needs_login, + deployed_sha: v.deployed_sha, + pending_reminders: v.pending_reminders, + parent: v.parent, + }) + .collect(); + HostResponse::agent_statuses(rows) +} + /// Single-agent queue verbs the admin socket exposes. Each submits the /// matching DAG (persisting the `wanted` intent, serializing on the /// agent's lease, with the transient/crash-watch suppression the old diff --git a/hive-c0re/src/socket_server/config_approvals.rs b/hive-c0re/src/socket_server/config_approvals.rs index 492322b9..37bdc2e7 100644 --- a/hive-c0re/src/socket_server/config_approvals.rs +++ b/hive-c0re/src/socket_server/config_approvals.rs @@ -1,7 +1,13 @@ //! Config-approval request handlers: `RequestInitConfig` / -//! `RequestApplyCommit` / `RequestUpdateMetaInputs`, plus the shared -//! submit helpers (`submit_init_config` / `submit_apply_commit`) and the -//! commit-sha shape check (`validate_commit_ref`). +//! `RequestApplyCommit` / `RequestUpdateMetaInputs`, +//! plus the shared submit helpers (`submit_init_config` / `submit_apply_commit` +//! / `submit_merge_config_pr`) and the commit-sha shape check +//! (`validate_commit_ref`). +//! +//! `submit_merge_config_pr` is called from the dashboard webhook handler +//! (`dashboard::webhook`) — agents no longer need an MCP tool for this; +//! opening a config PR on `agent-configs/` is enough to trigger +//! hive-c0re's webhook-driven queue path. use std::sync::Arc; @@ -82,6 +88,7 @@ pub(super) fn handle_request_update_meta_inputs( &commit_ref, description, requester, + None, ) .map_err(|e| anyhow::anyhow!("{e:#}")) { @@ -105,6 +112,75 @@ pub(super) fn handle_request_update_meta_inputs( AgentResponse::Ok } +/// 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 approve handler (`run_merge_config_pr`) drift-gates +/// against before doing anything irreversible. Unlike `submit_apply_commit` +/// 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; \ + use request_apply_commit for the first config deploy", + 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 between submit and set_fetched_sha cannot leave a stranded 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}"))?; + let id = coord + .approvals + .submit_kind( + agent, + hive_sh4re::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), + diff: None, // diff is not pre-computed; the dashboard fetches it on demand + description: description.map(str::to_owned), + pr_number: Some(pr_number), + }); + Ok(id) +} + /// `request_apply_commit` takes a commit SHA only — not a branch or /// tag name. A branch is mutable; pinning the proposal to a concrete /// sha keeps "what the manager asked to deploy" unambiguous and means @@ -161,6 +237,7 @@ pub(crate) fn submit_init_config( // parent); it's also the submitter the approval events route // back to. No declared parent = operator-initiated path. parent.unwrap_or("operator"), + None, // no sha for InitConfig ) .map_err(|e| anyhow::anyhow!("queue approval row: {e:#}"))?; tracing::info!(%id, %name, "init_config approval queued"); @@ -221,6 +298,7 @@ pub(crate) async fn submit_apply_commit( commit_ref, description, submitter, + None, // sha resolved after git_fetch_to_tag below; set via set_fetched_sha ) .map_err(|e| anyhow::anyhow!("queue approval row: {e:#}"))?; let tag = format!("proposal/{id}"); diff --git a/hive-c0re/src/socket_server/mod.rs b/hive-c0re/src/socket_server/mod.rs index e88e6de8..c71ae5e0 100644 --- a/hive-c0re/src/socket_server/mod.rs +++ b/hive-c0re/src/socket_server/mod.rs @@ -25,6 +25,7 @@ mod lifecycle_handlers; mod reminders; 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; diff --git a/hive-c0re/src/socket_server/schedules.rs b/hive-c0re/src/socket_server/schedules.rs index a8aa0783..92f9416a 100644 --- a/hive-c0re/src/socket_server/schedules.rs +++ b/hive-c0re/src/socket_server/schedules.rs @@ -62,6 +62,7 @@ pub(super) fn handle_request_schedule_prompt( &commit_ref, payload.description.as_deref(), requester, + None, ) { Ok(id) => id, Err(e) => { diff --git a/hive-c0re/src/stores/approvals.rs b/hive-c0re/src/stores/approvals.rs index 338c8f13..cb7e8ce3 100644 --- a/hive-c0re/src/stores/approvals.rs +++ b/hive-c0re/src/stores/approvals.rs @@ -70,6 +70,12 @@ 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 sha is resolved after insertion (e.g. + /// `ApplyCommit`'s `git_fetch_to_tag` step) and call [`set_fetched_sha`] + /// separately. pub fn submit_kind( &self, agent: &str, @@ -77,19 +83,22 @@ 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) - VALUES (?1, ?2, ?3, ?4, 'pending', ?5, ?6)", + (agent, kind, commit_ref, requested_at, status, description, submitter, + fetched_sha) + VALUES (?1, ?2, ?3, ?4, 'pending', ?5, ?6, ?7)", params![ agent, kind.as_str(), commit_ref, now_unix(), description, - submitter + submitter, + fetched_sha, ], )?; Ok(conn.last_insert_rowid()) @@ -415,6 +424,7 @@ mod tests { "", Some("scaffold"), "bitburner", + None, ) .expect("submit init_config"); let pending = db @@ -428,11 +438,11 @@ mod tests { #[test] fn mixed_kinds_all_listed() { let (_dir, _path, db) = open_temp(); - db.submit_kind("a", ApprovalKind::ApplyCommit, "deadbeef", None, "a") + db.submit_kind("a", ApprovalKind::ApplyCommit, "deadbeef", None, "a", None) .unwrap(); - db.submit_kind("b", ApprovalKind::Spawn, "", None, "b") + db.submit_kind("b", ApprovalKind::Spawn, "", None, "b", None) .unwrap(); - db.submit_kind("c", ApprovalKind::InitConfig, "", None, "c") + db.submit_kind("c", ApprovalKind::InitConfig, "", None, "c", None) .unwrap(); let pending = db.pending().expect("pending"); assert_eq!(pending.len(), 3, "all three kinds must be visible"); @@ -451,6 +461,7 @@ mod tests { "cafef00d", Some("test"), "bitburner", + None, ) .unwrap(); let row = db.mark_cancelled(id, "manager").expect("cancel"); @@ -470,7 +481,7 @@ mod tests { // final — re-cancelling errors instead of silently overwriting. let (_dir, _path, db) = open_temp(); let id = db - .submit_kind("a", ApprovalKind::Spawn, "deadbeef", None, "a") + .submit_kind("a", ApprovalKind::Spawn, "deadbeef", None, "a", None) .unwrap(); db.mark_cancelled(id, "manager").expect("first cancel"); let err = db @@ -485,7 +496,14 @@ mod tests { // whole list — collect_lenient skips it instead of failing. let (_dir, path, db) = open_temp(); let good = db - .submit_kind("good", ApprovalKind::ApplyCommit, "cafe", None, "good") + .submit_kind( + "good", + ApprovalKind::ApplyCommit, + "cafe", + None, + "good", + None, + ) .unwrap(); let raw = Connection::open(&path).unwrap(); raw.execute( @@ -508,7 +526,14 @@ mod tests { // fall back to the root agent. let (_dir, path, db) = open_temp(); let id = db - .submit_kind("child", ApprovalKind::ApplyCommit, "cafe", None, "parent") + .submit_kind( + "child", + ApprovalKind::ApplyCommit, + "cafe", + None, + "parent", + None, + ) .unwrap(); assert_eq!(db.submitter_of(id).unwrap().as_deref(), Some("parent")); @@ -522,4 +547,25 @@ mod tests { let legacy_id = raw.last_insert_rowid(); assert_eq!(db.submitter_of(legacy_id).unwrap(), None); } + + #[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 + // `set_fetched_sha` call. 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), + ) + .unwrap(); + let row = db.get(id).unwrap().expect("row must exist"); + assert_eq!(row.fetched_sha.as_deref(), Some(sha)); + } }