harness: drop manager special case from plugins notify (#778, mara feedback)
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<String> of failure messages; serve_main<S> iterates them and routes each through S::send_to_parent — the same <parent> 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.
This commit is contained in:
parent
c3b5f970c1
commit
df71d8deac
2 changed files with 43 additions and 63 deletions
|
|
@ -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<S: Surface>(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<String> and we route each through `<parent>`
|
||||
// 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,
|
||||
|
|
|
|||
|
|
@ -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 `<parent>` sentinel
|
||||
/// failure-notify uses everywhere else (#703).
|
||||
pub async fn install_configured(socket: &Path) -> Vec<String> {
|
||||
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<String> = 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
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue