From df71d8deac5ed4ef9c621dbef9ef70be6a244dcb Mon Sep 17 00:00:00 2001 From: damocles Date: Sun, 31 May 2026 14:49:17 +0200 Subject: [PATCH] harness: drop manager special case from plugins notify (#778, mara feedback) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mara on #778: 'remove the manager special case argus nitted about'. `plugins::install_configured` no longer takes a `notify_recipient` hardcoding "manager". Now returns a Vec of failure messages; serve_main iterates them and routes each through S::send_to_parent — the same sentinel failure-notify uses everywhere else (#703). Manager plugin failures now reach operator via root → operator fallback (improvement on the pre-PR silent-drop). Also rename FORGE_MENTIONS_ONLY → FORGE_IS_MANAGER to fix the misnomer: the boolean picks which wire enum (AgentRequest::Wake vs ManagerRequest::Wake) the forge_notify poller uses, not anything about mentions-only filtering (that's a separate nix-side option). Real fix is to lift Surface into the lib crate and make forge_notify::run generic; deferred to its own issue. Net: -20 LOC. --- hive-ag3nt/src/bin/hive.rs | 38 +++++++++++---------- hive-ag3nt/src/plugins.rs | 68 +++++++++++++------------------------- 2 files changed, 43 insertions(+), 63 deletions(-) diff --git a/hive-ag3nt/src/bin/hive.rs b/hive-ag3nt/src/bin/hive.rs index 873672ad..53a5f9cb 100644 --- a/hive-ag3nt/src/bin/hive.rs +++ b/hive-ag3nt/src/bin/hive.rs @@ -209,14 +209,15 @@ trait Surface { /// Real deploys always set the env var; the fallback covers /// standalone `nix run .#hive` invocations. const DEFAULT_LABEL: &'static str; - /// First-arg to `plugins::install_configured`. `Some("manager")` - /// on sub-agents pulls the manager's plugin allowlist; `None` on - /// the manager loads its own set. - const PLUGINS_PARENT: Option<&'static str>; - /// First-arg to `forge_notify::run`. `true` switches the notifier - /// to mentions-only mode (manager); `false` keeps the full subscribe - /// firehose (sub-agents). - const FORGE_MENTIONS_ONLY: bool; + /// `is_manager` flag passed to `forge_notify::run`. Picks which + /// wire enum (`AgentRequest::Wake` vs `ManagerRequest::Wake`) the + /// poller uses to push notifications into the harness inbox — the + /// per-role broker socket rejects the wrong type. Not actually + /// about "mentions only" (the skip-reasons drop-list is a separate + /// nix-side option). Real fix is to lift `Surface` into the lib + /// crate and make `forge_notify::run` generic; deferred to its + /// own issue. + const FORGE_IS_MANAGER: bool; /// Ack the in-flight turn. Logs warnings on transport/broker /// errors but never propagates — turn loop continues either way. @@ -271,8 +272,7 @@ struct AgentSurface; impl Surface for AgentSurface { const FLAVOR: mcp::Flavor = mcp::Flavor::Agent; const DEFAULT_LABEL: &'static str = "hive-ag3nt"; - const PLUGINS_PARENT: Option<&'static str> = Some("manager"); - const FORGE_MENTIONS_ONLY: bool = false; + const FORGE_IS_MANAGER: bool = false; async fn ack_turn(socket: &Path) { match client::request::<_, AgentResponse>(socket, &AgentRequest::AckTurn).await { @@ -411,12 +411,7 @@ struct ManagerSurface; impl Surface for ManagerSurface { const FLAVOR: mcp::Flavor = mcp::Flavor::Manager; const DEFAULT_LABEL: &'static str = "hm1nd"; - /// Manager loads its own plugin allowlist — no parent to inherit from. - const PLUGINS_PARENT: Option<&'static str> = None; - /// Mentions-only forge notifications: manager doesn't want the - /// subscription/participation firehose (see #671's role-driven - /// `forge.skipNotifyReasons` default). - const FORGE_MENTIONS_ONLY: bool = true; + const FORGE_IS_MANAGER: bool = true; async fn ack_turn(socket: &Path) { match client::request::<_, ManagerResponse>(socket, &ManagerRequest::AckTurn).await { @@ -576,10 +571,17 @@ async fn serve_main(socket: &Path, poll_ms: u64) -> Result<()> { } let files = turn::TurnFiles::prepare(socket, &label, S::FLAVOR).await?; let turn_lock: TurnLock = Arc::new(tokio::sync::Mutex::new(())); - plugins::install_configured(socket, S::PLUGINS_PARENT).await; + // Plugin install runs role-agnostic now (#778 v3, mara): failures + // come back as a Vec and we route each through `` + // via the same `send_to_parent` failure-notify path the turn loop + // uses. Manager failures now reach operator via root → operator + // fallback (improvement on the pre-#778 silent-drop on manager). + for failure in plugins::install_configured(socket).await { + S::send_to_parent(socket, failure).await; + } tokio::spawn(hive_ag3nt::forge_notify::run( socket.to_path_buf(), - S::FORGE_MENTIONS_ONLY, + S::FORGE_IS_MANAGER, )); tokio::spawn(web_ui::serve( label, diff --git a/hive-ag3nt/src/plugins.rs b/hive-ag3nt/src/plugins.rs index a75246f2..54d505d9 100644 --- a/hive-ag3nt/src/plugins.rs +++ b/hive-ag3nt/src/plugins.rs @@ -15,8 +15,6 @@ use std::path::Path; use tokio::process::Command; -use crate::client; - const PLUGINS_PATH: &str = "/etc/hyperhive/claude-plugins.json"; const MARKETPLACES_PATH: &str = "/etc/hyperhive/claude-marketplaces.json"; const AUTO_UPDATE_PATH: &str = "/etc/hyperhive/claude-plugins-auto-update.json"; @@ -99,25 +97,29 @@ async fn update_marketplaces() { } } -/// Install every plugin in `/etc/hyperhive/claude-plugins.json`. When -/// `notify_recipient` is `Some(name)`, install failures also get sent -/// as a hyperhive message to that recipient (typically `"manager"` for -/// sub-agents) so it surfaces in the inbox rather than being buried in -/// journald. The manager itself passes `None` — there's nobody above -/// it to notify. -pub async fn install_configured(socket: &Path, notify_recipient: Option<&str>) { +/// Install every plugin in `/etc/hyperhive/claude-plugins.json`. +/// Returns a list of human-readable failure messages so the caller can +/// route them through their own per-role surface (turn-failure-style +/// notification, see `Surface::send_to_parent`). Pre-#692 this function +/// hardcoded `"manager"` as the failure-notification recipient via a +/// `notify_recipient: Option<&str>` arg; mara on #778 wanted the +/// manager-name special case gone. Now plugins.rs is wire-agnostic and +/// the caller picks the recipient via the same `` sentinel +/// failure-notify uses everywhere else (#703). +pub async fn install_configured(socket: &Path) -> Vec { + let _ = socket; // Reserved for future telemetry; currently unused. let Ok(raw) = tokio::fs::read_to_string(PLUGINS_PATH).await else { - return; + return Vec::new(); }; let specs: Vec = match serde_json::from_str(&raw) { Ok(v) => v, Err(e) => { tracing::warn!(path = PLUGINS_PATH, error = ?e, "claude-plugins spec parse failed; skipping"); - return; + return Vec::new(); } }; if specs.is_empty() { - return; + return Vec::new(); } add_marketplaces().await; if auto_update_enabled().await { @@ -125,6 +127,7 @@ pub async fn install_configured(socket: &Path, notify_recipient: Option<&str>) { } else { tracing::debug!("claudePluginsAutoUpdate=false, skipping marketplace update"); } + let mut failures = Vec::new(); for spec in specs { match Command::new("claude") .args(["plugin", "install", &spec]) @@ -142,43 +145,18 @@ pub async fn install_configured(socket: &Path, notify_recipient: Option<&str>) { stderr = %stderr, "claude plugin install failed", ); - if let Some(to) = notify_recipient { - notify( - socket, - to, - format!( - "claude plugin install failed for `{spec}`:\n{}", - stderr.trim() - ), - ) - .await; - } + failures.push(format!( + "claude plugin install failed for `{spec}`:\n{}", + stderr.trim() + )); } Err(e) => { tracing::warn!(spec = %spec, error = ?e, "claude plugin install spawn failed"); - if let Some(to) = notify_recipient { - notify( - socket, - to, - format!("claude plugin install spawn failed for `{spec}`: {e}"), - ) - .await; - } + failures.push(format!( + "claude plugin install spawn failed for `{spec}`: {e}" + )); } } } -} - -/// Best-effort hyperhive send. Swallows transport errors — the warn log -/// is already in journald and the harness boot must not stall waiting -/// for the broker to be reachable. -async fn notify(socket: &Path, to: &str, body: String) { - let req = hive_sh4re::AgentRequest::Send { - to: to.to_owned(), - body, - in_reply_to: None, - }; - if let Err(e) = client::request::<_, hive_sh4re::AgentResponse>(socket, &req).await { - tracing::warn!(error = ?e, "failed to notify {to} of plugin install failure"); - } + failures }