diff --git a/hive-c0re/src/actions.rs b/hive-c0re/src/actions.rs index 041f9d9e..b61b5f8b 100644 --- a/hive-c0re/src/actions.rs +++ b/hive-c0re/src/actions.rs @@ -546,18 +546,15 @@ fn finish_approval( (ApprovalStatus::Failed, Some(note), false) } }; - coord.notify_submitter( - approval.id, - &HelperEvent::ApprovalResolved { - id: approval.id, - agent: approval.agent.clone(), - commit_ref: approval.commit_ref.clone(), - status, - note: note.clone(), - sha: approval.fetched_sha.clone(), - tag: terminal_tag.clone(), - }, - ); + coord.notify_manager(&HelperEvent::ApprovalResolved { + id: approval.id, + agent: approval.agent.clone(), + 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 // browser moves the row out of pending into history without a // snapshot refetch. `approved` rows that succeed get the @@ -593,49 +590,37 @@ fn finish_approval( match approval.kind { ApprovalKind::InitConfig => { if ok { - coord.notify_submitter( - approval.id, - &HelperEvent::ConfigReady { - agent: approval.agent.clone(), - }, - ); + coord.notify_manager(&HelperEvent::ConfigReady { + agent: approval.agent.clone(), + }); } } - ApprovalKind::Spawn => coord.notify_submitter( - approval.id, - &HelperEvent::Spawned { + ApprovalKind::Spawn => coord.notify_manager(&HelperEvent::Spawned { + agent: approval.agent.clone(), + ok, + note, + sha: approval.fetched_sha.clone(), + }), + ApprovalKind::ApplyCommit if is_first_spawn => { + coord.notify_manager(&HelperEvent::Spawned { agent: approval.agent.clone(), ok, note, sha: approval.fetched_sha.clone(), - }, - ), - ApprovalKind::ApplyCommit if is_first_spawn => { - coord.notify_submitter( - approval.id, - &HelperEvent::Spawned { - agent: approval.agent.clone(), - ok, - note, - sha: approval.fetched_sha.clone(), - }, - ); + }); } // MergeConfigPr ends in a container rebuild just like a // non-first-spawn ApplyCommit, so both surface the same Rebuilt // lifecycle event. (MergeConfigPr is never a first spawn — the // agent already exists — so it never hits the Spawned arm above.) ApprovalKind::ApplyCommit | ApprovalKind::MergeConfigPr => { - coord.notify_submitter( - approval.id, - &HelperEvent::Rebuilt { - agent: approval.agent.clone(), - ok, - note, - sha: approval.fetched_sha.clone(), - tag: terminal_tag, - }, - ); + coord.notify_manager(&HelperEvent::Rebuilt { + agent: approval.agent.clone(), + ok, + note, + sha: approval.fetched_sha.clone(), + tag: terminal_tag, + }); } // UpdateMetaInputs / SchedulePrompt: ApprovalResolved already // carries the result. No separate lifecycle event needed. @@ -1052,18 +1037,15 @@ pub async fn deny(coord: &Coordinator, id: i64, note: Option<&str>) -> Result<() 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( - a.id, - &HelperEvent::ApprovalResolved { - id: a.id, - agent: a.agent, - commit_ref: a.commit_ref, - status: ApprovalStatus::Denied, - note: note.map(String::from), - sha, - tag, - }, - ); + coord.notify_manager(&HelperEvent::ApprovalResolved { + id: a.id, + agent: a.agent, + commit_ref: a.commit_ref, + status: ApprovalStatus::Denied, + note: note.map(String::from), + sha, + tag, + }); coord.emit_approval_resolved(crate::coordinator::ApprovalResolved { id, agent: &agent_owned, diff --git a/hive-c0re/src/approvals.rs b/hive-c0re/src/approvals.rs index 652df40e..2abf82bc 100644 --- a/hive-c0re/src/approvals.rs +++ b/hive-c0re/src/approvals.rs @@ -70,21 +70,6 @@ fn ensure_fetched_sha_column(conn: &Connection) -> Result<()> { Ok(()) } -/// Same shape as `ensure_fetched_sha_column` but for `submitter` — the -/// agent that submitted the approval (the authenticated socket caller). -/// Approval-scoped helper events route to this agent. Legacy rows have -/// NULL; callers fall back to the root agent for those. -fn ensure_submitter_column(conn: &Connection) -> Result<()> { - let has: bool = conn - .prepare("SELECT 1 FROM pragma_table_info('approvals') WHERE name = 'submitter'")? - .exists([])?; - if !has { - conn.execute_batch("ALTER TABLE approvals ADD COLUMN submitter TEXT;") - .context("add approvals.submitter column")?; - } - Ok(()) -} - pub struct Approvals { conn: Mutex, } @@ -102,7 +87,6 @@ impl Approvals { ensure_kind_column(&conn).context("migrate approvals.kind")?; ensure_fetched_sha_column(&conn).context("migrate approvals.fetched_sha")?; ensure_description_column(&conn).context("migrate approvals.description")?; - ensure_submitter_column(&conn).context("migrate approvals.submitter")?; Ok(Self { conn: Mutex::new(conn), }) @@ -114,46 +98,22 @@ impl Approvals { kind: ApprovalKind, commit_ref: &str, description: Option<&str>, - submitter: &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)", + "INSERT INTO approvals (agent, kind, commit_ref, requested_at, status, description) + VALUES (?1, ?2, ?3, ?4, 'pending', ?5)", params![ agent, kind_to_str(kind), commit_ref, now_unix(), - description, - submitter + description ], )?; Ok(conn.last_insert_rowid()) } - /// The agent that submitted approval `id` (the authenticated socket - /// caller at submit time). `None` for legacy rows predating the - /// `submitter` column — callers route those to the root agent. - /// - /// # Errors - /// - /// Returns an error if the sqlite prepare/query fails. A missing row - /// or a `NULL` submitter is not an error — both yield `Ok(None)`. - pub fn submitter_of(&self, id: i64) -> Result> { - let conn = self.conn.lock().unwrap(); - let submitter: Option = conn - .query_row( - "SELECT submitter FROM approvals WHERE id = ?1", - params![id], - |row| row.get(0), - ) - .optional()? - .flatten(); - Ok(submitter) - } - /// Record the canonical sha hive-c0re fetched from the proposed repo /// into applied at submission time. Idempotent on identical values. pub fn set_fetched_sha(&self, id: i64, sha: &str) -> Result<()> { @@ -468,13 +428,7 @@ mod tests { // approval then vanished from the dashboard. let (_dir, _path, db) = open_temp(); let id = db - .submit_kind( - "bitburner", - ApprovalKind::InitConfig, - "", - Some("scaffold"), - "bitburner", - ) + .submit_kind("bitburner", ApprovalKind::InitConfig, "", Some("scaffold")) .expect("submit init_config"); let pending = db .pending() @@ -487,11 +441,10 @@ 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) .unwrap(); - db.submit_kind("b", ApprovalKind::Spawn, "", None, "b") - .unwrap(); - db.submit_kind("c", ApprovalKind::InitConfig, "", None, "c") + db.submit_kind("b", ApprovalKind::Spawn, "", None).unwrap(); + db.submit_kind("c", ApprovalKind::InitConfig, "", None) .unwrap(); let pending = db.pending().expect("pending"); assert_eq!(pending.len(), 3, "all three kinds must be visible"); @@ -509,7 +462,6 @@ mod tests { ApprovalKind::ApplyCommit, "cafef00d", Some("test"), - "bitburner", ) .unwrap(); let row = db.mark_cancelled(id, "manager").expect("cancel"); @@ -529,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) .unwrap(); db.mark_cancelled(id, "manager").expect("first cancel"); let err = db @@ -544,7 +496,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") + .submit_kind("good", ApprovalKind::ApplyCommit, "cafe", None) .unwrap(); let raw = Connection::open(&path).unwrap(); raw.execute( @@ -559,26 +511,4 @@ mod tests { assert_eq!(pending.len(), 1); assert_eq!(pending[0].id, good); } - - #[test] - fn submitter_round_trips_and_legacy_reads_none() { - // A submitted approval records its submitter; a legacy row - // (inserted without the column) reads back as None so callers - // fall back to the root agent. - let (_dir, path, db) = open_temp(); - let id = db - .submit_kind("child", ApprovalKind::ApplyCommit, "cafe", 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', 'apply_commit', '', 0, 'pending')", - [], - ) - .unwrap(); - let legacy_id = raw.last_insert_rowid(); - assert_eq!(db.submitter_of(legacy_id).unwrap(), None); - } } diff --git a/hive-c0re/src/coordinator.rs b/hive-c0re/src/coordinator.rs index f8670bb8..a4591049 100644 --- a/hive-c0re/src/coordinator.rs +++ b/hive-c0re/src/coordinator.rs @@ -1295,20 +1295,6 @@ impl Coordinator { self.notify_agent(hive_sh4re::MANAGER_AGENT, event); } - /// Route an approval-scoped helper event to the agent that submitted - /// approval `approval_id` (the authenticated socket caller at submit - /// time). Legacy rows with no recorded submitter — and any lookup - /// failure — fall back to the root agent, preserving the prior - /// always-root behaviour. - pub fn notify_submitter(&self, approval_id: i64, event: &hive_sh4re::HelperEvent) { - let target = self - .approvals - .submitter_of(approval_id) - .unwrap_or_default() - .unwrap_or_else(|| hive_sh4re::MANAGER_AGENT.to_owned()); - self.notify_agent(&target, event); - } - /// Push a `HelperEvent` into an arbitrary agent's inbox. Encoded /// the same way as `notify_manager` (sender = `SYSTEM_SENDER`, /// body = JSON-encoded event). Used to route `QuestionAnswered` diff --git a/hive-c0re/src/dashboard.rs b/hive-c0re/src/dashboard.rs index 28d39e5a..5b4c6e90 100644 --- a/hive-c0re/src/dashboard.rs +++ b/hive-c0re/src/dashboard.rs @@ -1650,13 +1650,11 @@ async fn post_request_spawn( if name.is_empty() { return error_response("spawn: `name` required"); } - match state.coord.approvals.submit_kind( - &name, - hive_sh4re::ApprovalKind::Spawn, - "", - None, - hive_sh4re::MANAGER_AGENT, - ) { + match state + .coord + .approvals + .submit_kind(&name, hive_sh4re::ApprovalKind::Spawn, "", None) + { Ok(id) => { tracing::info!(%id, %name, "operator: spawn approval queued via dashboard"); // Phase 5b: notify the dashboard event channel so live diff --git a/hive-c0re/src/loose_ends.rs b/hive-c0re/src/loose_ends.rs index df70f3bf..6def7c87 100644 --- a/hive-c0re/src/loose_ends.rs +++ b/hive-c0re/src/loose_ends.rs @@ -21,10 +21,10 @@ use hive_sh4re::{LooseEnd, MANAGER_AGENT}; use crate::coordinator::Coordinator; /// Open threads pending against `agent`: -/// - pending approvals where this agent is the submitter (a parent -/// agent with the `approvals` group submits for its children; the -/// root submits for top-level agents). Legacy rows with no recorded -/// submitter count as the root's; +/// - pending approvals where this agent is the submitter (only ever +/// true for the manager — sub-agents don't submit approvals — but +/// we keep the rule per-agent so the manager's MCP surface gets +/// the same shape via a different code path); /// - unanswered questions where `agent` is the asker (waiting on /// someone) OR the target (owes a reply); /// - pending reminders this agent scheduled (`owner == self`). @@ -35,24 +35,20 @@ use crate::coordinator::Coordinator; pub fn for_agent(coord: &Coordinator, agent: &str) -> Result> { let now = now_unix(); let mut out = Vec::new(); - // Show each pending approval to the agent that submitted it. The - // submitter column is NULL for rows predating it; those count as - // the root agent's. - for a in coord.approvals.pending()? { - let submitter = coord - .approvals - .submitter_of(a.id)? - .unwrap_or_else(|| MANAGER_AGENT.to_owned()); - if submitter != agent { - continue; + // Approvals are only submitted by the manager today. When that + // expands (e.g. sub-agents propose changes to their own configs), + // teach the approvals table to track the submitter and filter + // here on that column — for now MANAGER_AGENT == sole submitter. + if agent == MANAGER_AGENT { + for a in coord.approvals.pending()? { + out.push(LooseEnd::Approval { + id: a.id, + agent: a.agent, + commit_ref: a.commit_ref, + description: a.description, + age_seconds: saturating_age(now, a.requested_at), + }); } - out.push(LooseEnd::Approval { - id: a.id, - agent: a.agent, - commit_ref: a.commit_ref, - description: a.description, - age_seconds: saturating_age(now, a.requested_at), - }); } for q in coord.questions.pending_all()? { let role_match = q.asker == agent || q.target.as_deref() == Some(agent); diff --git a/hive-c0re/src/questions.rs b/hive-c0re/src/questions.rs index 2d99dfd8..fb287613 100644 --- a/hive-c0re/src/questions.rs +++ b/hive-c0re/src/questions.rs @@ -144,10 +144,8 @@ pub fn handle_answer( /// Handle `CancelLooseEnd` from a per-agent socket. Dispatches by kind, each /// with its own auth check: question / reminder cancels are ownership-only /// (an agent cancels its own), and approval cancels require the `approvals` -/// tool-group (the grantable capability) AND ownership — the canceller must -/// be the approval's submitter — so no positional / hardcoded privilege and -/// no cross-agent cancellation. (The operator's cancel-anything path is a -/// separate handler.) +/// tool-group (the grantable capability) — no positional / hardcoded +/// privilege. (The operator's cancel-anything path is a separate handler.) /// On question cancel, fires the `QuestionAnswered` event back to the asker /// so the harness loop can react (mirrors the operator-cancel dashboard path). pub fn handle_cancel_loose_end( @@ -197,25 +195,11 @@ pub fn handle_cancel_loose_end( Ok(()) } hive_sh4re::CancelLooseEndKind::Approval => { - // Withdrawing an approval needs the grantable `approvals` - // tool-group (held by any approval-submitting orchestrator) - // AND ownership: only the agent that submitted the approval - // may withdraw it. Without the ownership check, any - // approvals-group agent could cancel any other's approval by - // id. A NULL submitter (legacy row predating the column) is - // owned by the root agent. + // Withdrawing an approval is a hive-wide orchestration action, + // gated on the grantable `approvals` tool-group (held by the + // orchestrator that submits approvals) — not on a positional / + // hardcoded privilege. check_can_cancel_approval(canceller)?; - let submitter = coord - .approvals - .submitter_of(id) - .map_err(|e| format!("{e:#}"))? - .unwrap_or_else(|| hive_sh4re::MANAGER_AGENT.to_owned()); - if submitter != canceller { - return Err(format!( - "cancel_loose_end: approval {id} was submitted by {submitter}, \ - not {canceller}; only the submitting agent can withdraw it" - )); - } let approval = coord .approvals .mark_cancelled(id, canceller) diff --git a/hive-c0re/src/server.rs b/hive-c0re/src/server.rs index 0f69098e..b774253e 100644 --- a/hive-c0re/src/server.rs +++ b/hive-c0re/src/server.rs @@ -80,13 +80,10 @@ async fn dispatch(req: &HostRequest, coord: Arc) -> HostResponse { HostRequest::Spawn { name } => handle_spawn(&coord, name).await?, HostRequest::RequestSpawn { name } => { tracing::info!(%name, "request_spawn"); - let id = coord.approvals.submit_kind( - name, - hive_sh4re::ApprovalKind::Spawn, - "", - None, - hive_sh4re::MANAGER_AGENT, - )?; + let id = + coord + .approvals + .submit_kind(name, hive_sh4re::ApprovalKind::Spawn, "", None)?; tracing::info!(%id, %name, "spawn approval queued"); HostResponse::success() } diff --git a/hive-c0re/src/socket_server.rs b/hive-c0re/src/socket_server.rs index 4fcde8c1..f26abe62 100644 --- a/hive-c0re/src/socket_server.rs +++ b/hive-c0re/src/socket_server.rs @@ -989,7 +989,7 @@ async fn handle_request_apply_commit( return err; } tracing::info!(%agent, %target_agent, %commit_ref, "request_apply_commit"); - match submit_apply_commit(coord, target_agent, commit_ref, description, agent).await { + match submit_apply_commit(coord, target_agent, commit_ref, description).await { Ok((id, sha)) => { tracing::info!(%id, %target_agent, %sha, "apply_commit approval queued"); AgentResponse::Ok @@ -1427,7 +1427,6 @@ fn handle_request_update_meta_inputs( hive_sh4re::ApprovalKind::UpdateMetaInputs, &commit_ref, description, - requester, ) .map_err(|e| anyhow::anyhow!("{e:#}")) { @@ -1528,7 +1527,6 @@ fn handle_request_schedule_prompt( hive_sh4re::ApprovalKind::SchedulePrompt, &commit_ref, payload.description.as_deref(), - requester, ) { Ok(id) => id, Err(e) => { @@ -1803,10 +1801,6 @@ pub(crate) fn submit_init_config( hive_sh4re::ApprovalKind::InitConfig, parent.unwrap_or(""), description.as_deref(), - // `parent` is the requesting agent (becomes the new child's - // parent); it's also the submitter the approval events route - // back to. No declared parent = operator/root path. - parent.unwrap_or(hive_sh4re::MANAGER_AGENT), ) .map_err(|e| anyhow::anyhow!("queue approval row: {e:#}"))?; tracing::info!(%id, %name, "init_config approval queued"); @@ -1838,7 +1832,6 @@ pub(crate) async fn submit_apply_commit( agent: &str, commit_ref: &str, description: Option<&str>, - submitter: &str, ) -> anyhow::Result<(i64, String)> { validate_commit_ref(commit_ref)?; let proposed_dir = crate::coordinator::Coordinator::agent_proposed_dir(agent); @@ -1866,7 +1859,6 @@ pub(crate) async fn submit_apply_commit( hive_sh4re::ApprovalKind::ApplyCommit, commit_ref, description, - submitter, ) .map_err(|e| anyhow::anyhow!("queue approval row: {e:#}"))?; let tag = format!("proposal/{id}");