diff --git a/.gitignore b/.gitignore index b7ce58d6..a4afea36 100644 --- a/.gitignore +++ b/.gitignore @@ -3,4 +3,3 @@ /result-* /.tmp /.claude/settings.local.json -*.rlib diff --git a/docs/approvals.md b/docs/approvals.md index dfb6c486..b730db34 100644 --- a/docs/approvals.md +++ b/docs/approvals.md @@ -111,20 +111,15 @@ 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`. 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. + `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. - `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 16fe257a..5ad65e21 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 eb8fcadf..d4c8d175 100644 --- a/hive-c0re/src/dashboard/misc_api.rs +++ b/hive-c0re/src/dashboard/misc_api.rs @@ -196,7 +196,6 @@ 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 dd2ab2ca..60facd1f 100644 --- a/hive-c0re/src/dashboard/mod.rs +++ b/hive-c0re/src/dashboard/mod.rs @@ -160,10 +160,9 @@ 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/*` keeps its own prefix + // frontend migrated. `/webhook/knowledge` 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 c5024d5c..a10a4328 100644 --- a/hive-c0re/src/dashboard/webhook.rs +++ b/hive-c0re/src/dashboard/webhook.rs @@ -1,27 +1,15 @@ -//! Forgejo webhook endpoints. +//! Forgejo push-webhook endpoint for the `internal/knowledge` repo. //! -//! - **`/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). +//! 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. 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 { @@ -74,133 +62,3 @@ 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 112dc5d1..167e631f 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_is_open, + pr_head_sha, }; 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. -pub(crate) const CONFIG_ORG: &str = "agent-configs"; +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,82 +294,3 @@ 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 865cce5c..ca268140 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, StateType}; +use forgejo_api::structs::{MergePullRequestOption, MergePullRequestOptionDo}; use super::{CONFIG_ORG, api, core_token, forge_git_url}; @@ -112,30 +112,6 @@ 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 07421b5b..8efa4fcc 100644 --- a/hive-c0re/src/main.rs +++ b/hive-c0re/src/main.rs @@ -303,22 +303,17 @@ async fn cmd_serve( tokio::spawn(async move { forge::ensure_all().await; }); - // 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. + // 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. // No-op when the core token or forge are absent. let webhook_port = dashboard_port; tokio::spawn(async move { - let Some(token) = forge::core_token() else { - return; - }; - if let Err(e) = knowledge::ensure_webhook(&token, webhook_port).await { + if let Some(token) = forge::core_token() + && 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 0320467d..f43da1db 100644 --- a/hive-c0re/src/server.rs +++ b/hive-c0re/src/server.rs @@ -86,7 +86,6 @@ async fn dispatch(req: &HostRequest, coord: Arc) -> HostResponse { "", None, "operator", - None, )?; tracing::info!(%id, %name, "spawn approval queued"); HostResponse::success() @@ -146,7 +145,22 @@ async fn dispatch(req: &HostRequest, coord: Arc) -> HostResponse { HostResponse::dags(dags) } HostRequest::List => HostResponse::list(lifecycle::list().await?), - HostRequest::AgentStatus => handle_agent_status(&coord).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) + } // 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 @@ -227,24 +241,6 @@ 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 37bdc2e7..492322b9 100644 --- a/hive-c0re/src/socket_server/config_approvals.rs +++ b/hive-c0re/src/socket_server/config_approvals.rs @@ -1,13 +1,7 @@ //! Config-approval request handlers: `RequestInitConfig` / -//! `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. +//! `RequestApplyCommit` / `RequestUpdateMetaInputs`, plus the shared +//! submit helpers (`submit_init_config` / `submit_apply_commit`) and the +//! commit-sha shape check (`validate_commit_ref`). use std::sync::Arc; @@ -88,7 +82,6 @@ pub(super) fn handle_request_update_meta_inputs( &commit_ref, description, requester, - None, ) .map_err(|e| anyhow::anyhow!("{e:#}")) { @@ -112,75 +105,6 @@ 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 @@ -237,7 +161,6 @@ 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"); @@ -298,7 +221,6 @@ 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 c71ae5e0..e88e6de8 100644 --- a/hive-c0re/src/socket_server/mod.rs +++ b/hive-c0re/src/socket_server/mod.rs @@ -25,7 +25,6 @@ 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 92f9416a..a8aa0783 100644 --- a/hive-c0re/src/socket_server/schedules.rs +++ b/hive-c0re/src/socket_server/schedules.rs @@ -62,7 +62,6 @@ 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 cb7e8ce3..338c8f13 100644 --- a/hive-c0re/src/stores/approvals.rs +++ b/hive-c0re/src/stores/approvals.rs @@ -70,12 +70,6 @@ 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, @@ -83,22 +77,19 @@ 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, kind.as_str(), commit_ref, now_unix(), description, - submitter, - fetched_sha, + submitter ], )?; Ok(conn.last_insert_rowid()) @@ -424,7 +415,6 @@ mod tests { "", Some("scaffold"), "bitburner", - None, ) .expect("submit init_config"); let pending = db @@ -438,11 +428,11 @@ mod tests { #[test] fn mixed_kinds_all_listed() { let (_dir, _path, db) = open_temp(); - db.submit_kind("a", ApprovalKind::ApplyCommit, "deadbeef", None, "a", None) + db.submit_kind("a", ApprovalKind::ApplyCommit, "deadbeef", None, "a") .unwrap(); - db.submit_kind("b", ApprovalKind::Spawn, "", None, "b", None) + db.submit_kind("b", ApprovalKind::Spawn, "", None, "b") .unwrap(); - db.submit_kind("c", ApprovalKind::InitConfig, "", None, "c", None) + db.submit_kind("c", ApprovalKind::InitConfig, "", None, "c") .unwrap(); let pending = db.pending().expect("pending"); assert_eq!(pending.len(), 3, "all three kinds must be visible"); @@ -461,7 +451,6 @@ mod tests { "cafef00d", Some("test"), "bitburner", - None, ) .unwrap(); let row = db.mark_cancelled(id, "manager").expect("cancel"); @@ -481,7 +470,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", None) + .submit_kind("a", ApprovalKind::Spawn, "deadbeef", None, "a") .unwrap(); db.mark_cancelled(id, "manager").expect("first cancel"); let err = db @@ -496,14 +485,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::ApplyCommit, - "cafe", - None, - "good", - None, - ) + .submit_kind("good", ApprovalKind::ApplyCommit, "cafe", None, "good") .unwrap(); let raw = Connection::open(&path).unwrap(); raw.execute( @@ -526,14 +508,7 @@ 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", - None, - ) + .submit_kind("child", ApprovalKind::ApplyCommit, "cafe", None, "parent") .unwrap(); assert_eq!(db.submitter_of(id).unwrap().as_deref(), Some("parent")); @@ -547,25 +522,4 @@ 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)); - } }