diff --git a/Cargo.lock b/Cargo.lock index c298878..5b526f7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -82,19 +82,6 @@ dependencies = [ "generic-array", ] -[[package]] -name = "buzz-backend-hive" -version = "0.1.0" -dependencies = [ - "anyhow", - "hex", - "hive-core", - "hive-spec", - "serde", - "serde_json", - "sha2", -] - [[package]] name = "cfg-if" version = "1.0.4" @@ -237,6 +224,7 @@ dependencies = [ "anyhow", "hive-core", "hive-spec", + "serde_json", "toml", ] diff --git a/Cargo.toml b/Cargo.toml index 38bc089..7d51c93 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,6 @@ members = [ "crates/hived", "crates/hive-cli", "crates/hive-acp", - "crates/buzz-backend-hive", ] [workspace.package] diff --git a/crates/buzz-backend-hive/Cargo.toml b/crates/buzz-backend-hive/Cargo.toml deleted file mode 100644 index e806fcf..0000000 --- a/crates/buzz-backend-hive/Cargo.toml +++ /dev/null @@ -1,24 +0,0 @@ -[package] -name = "buzz-backend-hive" -description = "Buzz desktop provider shim: deploys agents to a hive host over SSH." -version.workspace = true -edition.workspace = true -rust-version.workspace = true -license.workspace = true -repository.workspace = true -authors.workspace = true - -# The FILENAME is what Buzz shows in its picker: the desktop discovers -# `buzz-backend-*` executables and derives the displayed name from the suffix. -[[bin]] -name = "buzz-backend-hive" -path = "src/main.rs" - -[dependencies] -hive-spec.workspace = true -hive-core.workspace = true -serde.workspace = true -serde_json.workspace = true -anyhow.workspace = true -sha2.workspace = true -hex.workspace = true diff --git a/crates/buzz-backend-hive/src/main.rs b/crates/buzz-backend-hive/src/main.rs deleted file mode 100644 index e60f681..0000000 --- a/crates/buzz-backend-hive/src/main.rs +++ /dev/null @@ -1,649 +0,0 @@ -//! `buzz-backend-hive` — the Buzz desktop provider shim. -//! -//! Buzz's desktop discovers `buzz-backend-*` executables and speaks a tiny -//! JSON-on-stdin protocol to them: `{"op":"info"}` and `{"op":"deploy"}`. This -//! shim runs on the DESKTOP, translates a deploy request into a hive spec, and -//! ships it to the hive host over SSH. `hived` on the far side notices the new -//! spec and reconciles it. -//! -//! It writes a file and stores a credential; it does not create containers. That -//! keeps the desktop's role declarative and means an agent deployed this way is -//! identical to one deployed by editing a spec by hand. -//! -//! # Two constraints inherited from the desktop -//! -//! **Config keys are filtered.** `validate_provider_config` rejects any key -//! containing `secret`, `password`, `token`, `key` or `credential`. A field named -//! `claude_token_path` is silently dropped; `claude_auth_file` survives. This is -//! why the schema below reads slightly awkwardly. -//! -//! **One agent, one relay, one container.** `buzz-acp` takes a scalar -//! `BUZZ_RELAY_URL`, so the same agent identity on two relays is two processes. -//! The desktop models this as `{pubkey, relay_url}`. Without a relay suffix in -//! the name, deploying an agent to a second relay REPLACES its first container -//! instead of running alongside it, because the redeploy path reads them as the -//! same agent. The primary relay keeps the bare name so the common single-relay -//! case stays readable. - -use std::io::{BufRead, Write}; -use std::process::{Command, Stdio}; - -use anyhow::{bail, Context, Result}; -use serde_json::{json, Value}; - -const VERSION: &str = env!("CARGO_PKG_VERSION"); - -fn main() { - // Every failure path must still emit valid JSON on stdout: the desktop - // parses whatever it gets, and a bare panic message surfaces as an - // unexplained provider error. - let response = match run() { - Ok(v) => v, - Err(e) => json!({ "error": e.to_string() }), - }; - println!("{response}"); -} - -fn run() -> Result { - let mut line = String::new(); - std::io::stdin().lock().read_line(&mut line)?; - let req: Value = if line.trim().is_empty() { - json!({}) - } else { - serde_json::from_str(&line).context("parsing request")? - }; - - match req.get("op").and_then(Value::as_str) { - Some("info") => Ok(info()), - Some("deploy") => deploy(&req), - other => bail!("unknown op {:?}", other.unwrap_or("")), - } -} - -fn info() -> Value { - json!({ - "id": "hive", - "name": "hive (isolated container per agent)", - "version": VERSION, - // REQUIRED. The desktop's WhereToRunSection probes this provider, reads - // config_schema, and renders one form field per property, pre-filled - // from `default`. Without it the UI shows no fields at all and every - // value silently falls back to this program's defaults. - // - // Types matter: coerceConfigValues() converts "integer"/"number" with - // Number() and "boolean" with value === "true" before sending. Declaring - // the wrong type delivers a string where a bool or int is expected. - // Neither transport field is REQUIRED: exactly one of them is, and the - // desktop's schema cannot express that. Requiring ssh_host would make - // the local case impossible to fill in; requiring neither means deploy() - // has to explain the choice, which it does. - "config_schema": { - "type": "object", - "properties": { - "ssh_host": { - "type": "string", - "title": "hive host (remote)", - "description": "user@host running hived. Uses your existing SSH key; no daemon is exposed to the network. Leave blank if hived runs on THIS machine.", - "default": "" - }, - "hived_container": { - "type": "string", - "title": "hived container (local)", - "description": "Name of a local hived container to deploy into, instead of connecting over SSH. Required on macOS and Windows, where hived must run inside the Docker VM. Ignored when 'hive host' is set.", - "default": "" - }, - "spec_dir": { - "type": "string", - "title": "Spec directory", - "description": "Where hived watches for agent specs on that host.", - "default": "/etc/hive/agents" - }, - "harness": { - "type": "string", - "title": "Harness", - "description": "claude, codex, goose, grok, opencode, kimi, amp, omp or cursor. Run `hive harnesses` on the host for the current list.", - "default": "claude" - }, - "memory": { - "type": "string", - "title": "Memory limit", - "description": "A CEILING, not a reservation — an idle agent uses ~60MB. Roomier than one harness needs, because an agent may shell out to a second one.", - "default": "3g" - }, - "cpus": { - "type": "number", - "title": "CPU limit", - "default": 2.0 - }, - "observer": { - "type": "boolean", - "title": "Publish observer frames", - "description": "ON by default. The harness defaults it off, which makes a remote agent work perfectly while appearing to do nothing — a local agent is observed over stdio, and a container has no stdio to observe.", - "default": true - }, - "mcp_url": { - "type": "string", - "title": "MCP server URL", - "description": "Optional HTTP MCP server. Its credential is served per-connection from the broker and never enters the container. Leave empty to skip.", - "default": "" - }, - // NOT `mcp_token_key` or similar: any config key containing - // secret/password/token/key/credential is dropped by the - // desktop before it reaches this program. - "mcp_auth_ref": { - "type": "string", - "title": "MCP credential name", - "description": "Name of the stored credential in the hive broker, e.g. mcp/parachute. Store it with: hive secret put mcp/parachute", - "default": "" - } - } - } - }) -} - -fn deploy(req: &Value) -> Result { - let agent = req.get("agent").cloned().unwrap_or_else(|| json!({})); - let cfg = req.get("provider_config").cloned().unwrap_or_else(|| json!({})); - - let get = |k: &str| cfg.get(k).and_then(Value::as_str).unwrap_or("").to_string(); - // ssh_host wins when both are set, because a filled-in remote host is an - // explicit statement about WHERE the agent should run, while - // hived_container may be left over from a local experiment. Silently - // deploying to the wrong machine is the expensive mistake here. - let target = Target::choose(&get("ssh_host"), &get("hived_container"))?; - let spec_dir = { - let d = get("spec_dir"); - if d.is_empty() { "/etc/hive/agents".to_string() } else { d } - }; - - let display_name = agent.get("name").and_then(Value::as_str).unwrap_or("agent"); - let nsec = agent - .get("private_key_nsec") - .and_then(Value::as_str) - .context("agent.private_key_nsec is required")?; - let relay_url = agent - .get("relay_url") - .and_then(Value::as_str) - .context("agent.relay_url is required")?; - let pubkey = agent.get("pubkey").and_then(Value::as_str).unwrap_or(""); - let owner = agent - .get("owner_pubkey") - .and_then(Value::as_str) - .unwrap_or(""); - - // Passed in rather than read inside agent_name: a function whose result - // depends on ambient environment is one you cannot test without mutating - // the process, and in Rust 2024 that is an unsafe operation. - let primary = std::env::var("HIVE_PRIMARY_RELAY").ok(); - let name = agent_name(display_name, relay_url, primary.as_deref()); - // The IDENTITY key is keyed on the base slug, without the relay suffix, so - // deploying the same agent to a second relay reuses one stored private key - // instead of writing a second copy that goes stale on rotation. - let identity_key = format!("nsec/{}", slugify(display_name)); - - let mut warnings: Vec = Vec::new(); - if owner.is_empty() && agent.get("auth_tag").is_none() { - // Not fatal here — hived's validation will refuse it — but saying so at - // deploy time is far more useful than a container that starts and - // ignores everyone. - warnings.push( - "no owner_pubkey or auth_tag: the agent would start and respond to nobody".into(), - ); - } - - // BYOH: the desktop resolves the harness — builtin, preset or user-defined - // JSON — down to a concrete command and sends it as agent_command/agent_args. - // Honour that rather than the provider_config field, or a harness picked in - // the UI is silently overridden by a setting the user last touched elsewhere. - let cmd = agent.get("agent_command").and_then(Value::as_str).unwrap_or(""); - let cmd_args: Vec = agent - .get("agent_args") - .and_then(Value::as_array) - .map(|a| a.iter().filter_map(|v| v.as_str().map(String::from)).collect()) - .unwrap_or_default(); - - let harness = resolve_harness(cmd, &cmd_args, &get("harness"), &mut warnings); - let spec = build_spec(&agent, &cfg, pubkey, relay_url, owner, &identity_key, &harness); - - // The nsec goes over stdin, never as an argument: arguments land in shell - // history and in `ps` output for every user on the box — and for the local - // transport, in `docker inspect` too. It is not written into the spec, - // which is meant to be committable. - target - .stdin_to(&["hive", "secret", "put", &identity_key], nsec) - .context("storing the agent key in the hive broker")?; - - // `cat > file` rather than scp: one connection, no temp file on either side, - // and it works when the remote has no scp. `docker exec -i` accepts the - // same shell, so one code path serves both transports. - let spec_path = format!("{spec_dir}/{name}.toml"); - target - .stdin_to(&["sh", "-c", &format!("mkdir -p {spec_dir} && cat > {spec_path}")], &spec) - .context("writing the agent spec")?; - - warnings.push(format!( - "spec written to {}:{spec_path}. hived will reconcile it on its next pass; \ - run `hive status` there to watch.", - target.describe() - )); - - Ok(json!({ "agent_id": name, "warnings": warnings })) -} - -/// Agent name, including a relay suffix for non-primary relays. -/// -/// See the module docs: without this, an agent deployed to a second relay -/// replaces its first container rather than running alongside it. -fn agent_name(display_name: &str, relay_url: &str, primary_relay: Option<&str>) -> String { - use sha2::{Digest, Sha256}; - let slug = slugify(display_name); - - match primary_relay { - Some(primary) if primary != relay_url => { - let tag = hex::encode(&Sha256::digest(relay_url.as_bytes())[..4]); - format!("{slug}-{tag}") - } - _ => slug, - } -} - -/// How the spec should name the harness. -enum HarnessChoice { - /// Resolved to a catalog entry, which knows its model syntax and credentials. - Catalog(&'static str), - /// A BYOH custom harness the catalog does not know. Expressed as an explicit - /// command, which requires an explicit image containing it. - Custom { command: String, args: Vec }, -} - -/// Map the desktop's resolved invocation onto the catalog. -fn resolve_harness( - command: &str, - args: &[String], - config_fallback: &str, - warnings: &mut Vec, -) -> HarnessChoice { - if command.is_empty() { - // Older records, or a create path that never pinned a command. - let id = if config_fallback.is_empty() { "claude" } else { config_fallback }; - return HarnessChoice::Catalog( - hive_core::harness::lookup(id).map(|h| h.id).unwrap_or("claude"), - ); - } - if let Some(h) = hive_core::harness::lookup_by_command(command, args) { - return HarnessChoice::Catalog(h.id); - } - // A custom harness from the desktop's custom_harnesses/. hive can express it, - // but only the image can say whether the binary is actually there — so warn - // rather than fail here, and let the container entrypoint report it clearly. - warnings.push(format!( - "harness '{command}' is not in hive's catalog. The spec will name it explicitly, \ - but the agent image must contain that binary — check `hive harnesses` on the host." - )); - HarnessChoice::Custom { command: command.to_string(), args: args.to_vec() } -} - -/// Agent name without the relay suffix. Also the identity key's basis, so the -/// two cannot drift apart. -fn slugify(display_name: &str) -> String { - let slug: String = display_name - .to_lowercase() - .chars() - .map(|c| if c.is_ascii_alphanumeric() { c } else { '-' }) - .collect::() - .trim_matches('-') - .replace("--", "-"); - if slug.is_empty() { "agent".to_string() } else { slug } -} - -fn build_spec( - agent: &Value, - cfg: &Value, - pubkey: &str, - relay_url: &str, - owner: &str, - identity_key: &str, - harness: &HarnessChoice, -) -> String { - let s = |k: &str| cfg.get(k).and_then(Value::as_str).unwrap_or(""); - let memory = if s("memory").is_empty() { "3g" } else { s("memory") }; - let cpus = cfg.get("cpus").and_then(Value::as_f64).unwrap_or(2.0); - let observer = cfg.get("observer").and_then(Value::as_bool).unwrap_or(true); - - let mut out = String::new(); - out.push_str("# Written by buzz-backend-hive. Safe to edit and to commit:\n"); - out.push_str("# it contains no secrets, only names of credentials the broker holds.\n\n"); - out.push_str("[identity]\n"); - out.push_str(&format!("pubkey = {}\n", toml_str(pubkey))); - out.push_str(&format!("relay_url = {}\n", toml_str(relay_url))); - // Named explicitly rather than left to default to nsec/: the file - // name carries a relay suffix for non-primary relays, and the identity does - // not vary by relay. - out.push_str(&format!("credential = {}\n", toml_str(identity_key))); - if let Some(tag) = agent.get("auth_tag").and_then(Value::as_str) { - out.push_str(&format!("auth_tag = {}\n", toml_str(tag))); - } else if !owner.is_empty() { - out.push_str(&format!("owner_pubkey = {}\n", toml_str(owner))); - } - - out.push_str("\n[harness]\n"); - match harness { - HarnessChoice::Catalog(id) => out.push_str(&format!("id = {}\n", toml_str(id))), - HarnessChoice::Custom { command, args } => { - out.push_str(&format!("command = {}\n", toml_str(command))); - if !args.is_empty() { - let rendered: Vec = args.iter().map(|a| toml_str(a)).collect(); - out.push_str(&format!("args = [{}]\n", rendered.join(", "))); - } - // An explicit command requires an explicit image; validation enforces it. - out.push_str("image = \"hive-agent:latest\"\n"); - } - } - - out.push_str("\n[agent]\n"); - out.push_str(&format!("observer = {observer}\n")); - if let Some(m) = agent.get("model").and_then(Value::as_str) { - out.push_str(&format!("model = {}\n", toml_str(m))); - } - if let Some(p) = agent.get("system_prompt").and_then(Value::as_str) { - out.push_str(&format!("system_prompt = {}\n", toml_str(p))); - } - // The desktop already sends these; dropping them silently reverts the agent - // to harness defaults that do not match what the UI shows. - if let Some(r) = agent.get("respond_to").and_then(Value::as_str) { - out.push_str(&format!("respond_to = {}\n", toml_str(r))); - } - if let Some(n) = agent.get("parallelism").and_then(Value::as_u64) { - out.push_str(&format!("parallelism = {n}\n")); - } - if let Some(t) = agent.get("idle_timeout_seconds").and_then(Value::as_u64).filter(|t| *t > 0) { - out.push_str(&format!("idle_timeout = {t}\n")); - } - if let Some(t) = agent.get("max_turn_duration_seconds").and_then(Value::as_u64).filter(|t| *t > 0) { - out.push_str(&format!("max_turn_duration = {t}\n")); - } - - out.push_str(&format!("\n[resources]\nmemory = {}\ncpus = {cpus}\npids = 512\n", toml_str(memory))); - - let url = s("mcp_url"); - if !url.is_empty() { - out.push_str("\n[[mcp]]\nname = \"mcp\"\ntransport = \"http\"\n"); - out.push_str(&format!("url = {}\n", toml_str(url))); - let auth = s("mcp_auth_ref"); - if !auth.is_empty() { - out.push_str(&format!("credential = {}\n", toml_str(auth))); - } - } - out -} - -/// TOML-quote a string. Basic strings escape backslash and quote; a spec is -/// generated from user-supplied names, so this cannot be a bare format!(). -fn toml_str(s: &str) -> String { - let mut out = String::with_capacity(s.len() + 2); - out.push('"'); - for c in s.chars() { - match c { - '"' => out.push_str("\\\""), - '\\' => out.push_str("\\\\"), - '\n' => out.push_str("\\n"), - '\r' => out.push_str("\\r"), - '\t' => out.push_str("\\t"), - c if (c as u32) < 0x20 => out.push_str(&format!("\\u{:04X}", c as u32)), - c => out.push(c), - } - } - out.push('"'); - out -} - -/// Where `hived` is, and therefore how to hand it a spec and a secret. -/// -/// The shim only ever runs two commands — store a credential, write a file — -/// so the transport is the entire difference between a remote and a local hive. -// Debug so tests can use `unwrap_err()`. Both variants hold a destination, not -// a credential, so there is nothing here that must not be printed. -#[derive(Debug)] -enum Target { - /// `hived` on another machine, reached with the user's existing SSH key. - Ssh(String), - /// `hived` in a container on THIS machine. - /// - /// This is not merely a convenience for single-box setups: on macOS and - /// Windows it is the only arrangement that works. `hived` bind-mounts a - /// per-agent unix socket into each agent container, and a socket created - /// on the host side of a Docker VM cannot be connected to from inside it - /// (`connect()` returns ENOTSUP). So `hived` must live in the VM, and the - /// shim reaches it through `docker exec` rather than over a network. - Container(String), -} - -impl Target { - fn choose(ssh_host: &str, container: &str) -> Result { - match (ssh_host.trim(), container.trim()) { - ("", "") => bail!( - "set either 'hive host' (for a remote hived over SSH) or \ - 'hived container' (for one running locally in Docker)" - ), - (h, _) if !h.is_empty() => Ok(Target::Ssh(h.to_string())), - (_, c) => Ok(Target::Container(c.to_string())), - } - } - - /// For messages shown to the user. Not a shell-safe value. - fn describe(&self) -> String { - match self { - Target::Ssh(h) => h.clone(), - Target::Container(c) => format!("container {c}"), - } - } - - /// Run a command where `hived` lives, feeding `input` to its stdin. - fn stdin_to(&self, argv: &[&str], input: &str) -> Result<()> { - let (bin, lead): (String, Vec) = match self { - Target::Ssh(host) => ( - find_ssh()?, - // Fail rather than hang on an unknown host: this runs under a - // GUI with no terminal to answer a prompt on, so an - // interactive question is an indefinite hang with no visible - // cause. - vec!["-o".into(), "BatchMode=yes".into(), host.clone()], - ), - Target::Container(name) => ( - find_docker()?, - // -i, not -it: there is no tty here, and `docker exec -t` - // without one fails outright. - vec!["exec".into(), "-i".into(), name.clone()], - ), - }; - - let mut child = Command::new(&bin) - .args(&lead) - .args(argv) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - .with_context(|| format!("spawning {bin}"))?; - child - .stdin - .take() - .context("child stdin")? - .write_all(input.as_bytes())?; - let out = child.wait_with_output()?; - if !out.status.success() { - bail!("{}: {}", self.describe(), String::from_utf8_lossy(&out.stderr).trim()); - } - Ok(()) - } -} - -/// Locate ssh without trusting PATH. -/// -/// A GUI-launched process on macOS inherits a minimal launchd environment whose -/// PATH often lacks the directories a developer's shell has. The same command -/// then works perfectly in a terminal and not at all from the app. -fn find_ssh() -> Result { - for p in ["/usr/bin/ssh", "/usr/local/bin/ssh", "/opt/homebrew/bin/ssh"] { - if std::path::Path::new(p).is_file() { - return Ok(p.to_string()); - } - } - Ok("ssh".to_string()) -} - -/// Locate the Docker CLI without trusting PATH, for the same reason as -/// [`find_ssh`] — and more acutely, because Docker is never in the default -/// launchd PATH on macOS. The candidate list mirrors -/// `hive_core::docker::DockerBackend::discover`. -fn find_docker() -> Result { - for p in [ - "/usr/local/bin/docker", - "/opt/homebrew/bin/docker", - "/usr/bin/docker", - "/Applications/Docker.app/Contents/Resources/bin/docker", - ] { - if std::path::Path::new(p).is_file() { - return Ok(p.to_string()); - } - } - Ok("docker".to_string()) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn info_declares_a_config_schema_or_the_desktop_renders_nothing() { - // Without config_schema the settings UI shows no fields at all and every - // value silently falls back to this program's defaults. - let i = info(); - assert!(i["config_schema"]["properties"].is_object()); - assert_eq!(i["id"], "hive"); - } - - #[test] - fn an_unconfigured_transport_is_an_error_not_a_guessed_host() { - // This previously defaulted to `root@hive-host`, so a deploy with no - // configuration attempted SSH to a host that does not exist and failed - // with a name-resolution error — which reads as a network problem - // rather than as "you have not said where hive is". - let e = Target::choose("", "").unwrap_err().to_string(); - assert!(e.contains("hive host"), "the error must name the fields to set: {e}"); - assert!(e.contains("hived container"), "the error must offer the local option: {e}"); - } - - #[test] - fn a_local_deploy_needs_no_ssh_host() { - // The whole point of local mode: on macOS there is no host to ssh to, - // because hived runs in the Docker VM alongside the agents. - match Target::choose("", "hived").unwrap() { - Target::Container(c) => assert_eq!(c, "hived"), - Target::Ssh(h) => panic!("chose ssh to {h} with no host configured"), - } - } - - #[test] - fn a_configured_ssh_host_is_not_overridden_by_a_leftover_container_name() { - // Both fields are free text in the desktop UI and neither is required, - // so a container name left over from a local experiment can easily sit - // beside a real remote host. Preferring the container would deploy the - // agent to the wrong machine and report success. - match Target::choose("root@box", "hived").unwrap() { - Target::Ssh(h) => assert_eq!(h, "root@box"), - Target::Container(c) => panic!("deployed to local container {c} despite a remote host"), - } - } - - #[test] - fn whitespace_only_config_counts_as_unset() { - // A field the user cleared can come back as " " rather than "", and a - // space-only ssh host would otherwise be spawned as a real destination. - assert!(Target::choose(" ", " ").is_err()); - match Target::choose(" ", "hived").unwrap() { - Target::Container(c) => assert_eq!(c, "hived"), - Target::Ssh(h) => panic!("treated whitespace as a host: '{h}'"), - } - } - - #[test] - fn no_config_key_contains_a_word_the_desktop_strips() { - // validate_provider_config drops any key containing these substrings, so - // a field named `mcp_token` would vanish between the UI and here — and - // the symptom is a setting that will not stick. - let i = info(); - let props = i["config_schema"]["properties"].as_object().unwrap(); - for k in props.keys() { - for banned in ["secret", "password", "token", "key", "credential"] { - assert!( - !k.contains(banned), - "config key '{k}' contains '{banned}' and will be dropped by the desktop" - ); - } - } - } - - #[test] - fn a_second_relay_gets_its_own_agent_name() { - // buzz-acp takes a scalar relay URL, so the same identity on two relays - // is two containers. Without the suffix the second deploy replaces the - // first rather than running beside it. - let primary = Some("wss://primary.example"); - let a = agent_name("Uni", "wss://primary.example", primary); - let b = agent_name("Uni", "wss://other.example", primary); - assert_eq!(a, "uni", "the primary relay keeps the bare name"); - assert_ne!(a, b, "a second relay must not collide with the first"); - assert!(b.starts_with("uni-")); - } - - #[test] - fn the_generated_spec_contains_no_secret() { - // The spec is meant to be committable. The nsec goes to the broker over - // stdin and must never reach the file. - let agent = json!({ - "name": "Uni", - "private_key_nsec": "nsec1verysecret", - "relay_url": "wss://relay.example", - "owner_pubkey": "b".repeat(64), - }); - let spec = build_spec(&agent, &json!({}), &"a".repeat(64), "wss://relay.example", &"b".repeat(64), "nsec/uni", &HarnessChoice::Catalog("claude")); - assert!(!spec.contains("nsec1verysecret"), "the spec leaked the private key"); - assert!(spec.contains("owner_pubkey")); - } - - #[test] - fn the_generated_spec_parses_and_validates() { - // Emitting TOML by hand is a good way to produce something that reads - // fine and does not parse. Round-trip it through the real parser. - let agent = json!({ - "name": "Uni", - "private_key_nsec": "nsec1x", - "relay_url": "wss://relay.example", - "owner_pubkey": "b".repeat(64), - }); - let cfg = json!({ "harness": "claude", "mcp_url": "https://v.example/mcp", "mcp_auth_ref": "mcp/parachute" }); - let spec = build_spec(&agent, &cfg, &"a".repeat(64), "wss://relay.example", &"b".repeat(64), "nsec/uni", &HarnessChoice::Catalog("claude")); - let parsed = hive_spec::AgentSpec::from_toml(&spec) - .unwrap_or_else(|e| panic!("generated spec does not parse: {e}\n---\n{spec}")); - let report = parsed.validate(); - assert!(report.errors.is_empty(), "generated spec is invalid: {:?}\n{spec}", report.errors); - assert_eq!(parsed.mcp.len(), 1); - } - - #[test] - fn names_with_quotes_do_not_break_the_generated_toml() { - // Display names come from the desktop and are arbitrary user text. - let agent = json!({ - "name": "weird", - "private_key_nsec": "x", - "relay_url": "wss://relay.example", - "system_prompt": "say \"hello\"\nand \\ that", - "owner_pubkey": "b".repeat(64), - }); - let spec = build_spec(&agent, &json!({}), &"a".repeat(64), "wss://relay.example", &"b".repeat(64), "nsec/uni", &HarnessChoice::Catalog("claude")); - let parsed = hive_spec::AgentSpec::from_toml(&spec) - .unwrap_or_else(|e| panic!("quoting broke the spec: {e}\n---\n{spec}")); - assert!(parsed.agent.system_prompt.unwrap().contains('"')); - } -} diff --git a/crates/hive-acp/Cargo.toml b/crates/hive-acp/Cargo.toml index 0a3bfec..bfd6acc 100644 --- a/crates/hive-acp/Cargo.toml +++ b/crates/hive-acp/Cargo.toml @@ -12,3 +12,4 @@ hive-spec.workspace = true hive-core.workspace = true anyhow.workspace = true toml.workspace = true +serde_json.workspace = true diff --git a/crates/hive-acp/src/main.rs b/crates/hive-acp/src/main.rs index cd1fcc7..5e9de26 100644 --- a/crates/hive-acp/src/main.rs +++ b/crates/hive-acp/src/main.rs @@ -4,37 +4,61 @@ //! //! Buzz has two extension seams, and which one hive occupies decides a lot. //! -//! As a **backend provider** ("where to run"), hive competes with Buzz's own -//! notion of location — which is why the desktop shim had to reimplement remote -//! deployment over ssh. As a **harness**, `harness=hive` composes with whatever -//! Buzz already does about location: local, a remote provider, or this box -//! acting as a provider for a laptop. Location stays Buzz's axis; the -//! environment becomes hive's. +//! A **backend provider** answers "where does this agent run": it receives the +//! deploy payload — nsec, relay url, `agent_command`, `env_vars` — and starts +//! `buzz-acp` somewhere. A **harness** answers "what does it run in". The old +//! shim took the provider seam and therefore had to reimplement remote +//! deployment over ssh, which is not a container-runtime problem. +//! +//! As a harness, `harness=hive` composes with whatever Buzz already does about +//! location, because the deploy payload carries `agent_command` and `env_vars` +//! verbatim: a provider that puts `buzz-acp` on another host will spawn +//! `hive-acp` there with `HIVE_ENV` intact, and neither end needs to know about +//! the other. Location stays Buzz's axis; the environment becomes hive's. //! //! So this is a Tier-3 custom harness (BYOH, buzz v0.5.0). `buzz-acp` spawns it //! exactly as it would spawn `claude-agent-acp`, and it speaks ACP on stdio. //! Below, it runs the real harness inside that agent's container — with hive's //! isolation, MCP servers and broker-held credentials already in place. //! -//! # Why it does not parse the protocol +//! # Why it intercepts exactly one method +//! +//! ACP's `McpServer` is a union — `McpServerHttp`, `McpServerSse`, +//! `McpServerAcp`, `McpServerStdio` — and the HTTP variant carries a `headers` +//! array. Buzz implements only the stdio one, which is why an HTTP MCP server +//! cannot be configured from the desktop. The protocol was never the limit. +//! +//! So `session/new` is rewritten on its way down: hive appends the agent's own +//! MCP servers, with credentials fetched from the broker, to whatever Buzz +//! sent. Verified against `claude-agent-acp`, which advertises +//! `mcpCapabilities {http, sse}` and duly listed every Parachute tool. //! -//! It could: intercept `initialize`, route by session id, and gain the ability -//! to switch harnesses mid-conversation. That is a real feature and it is not -//! this. +//! That is worth doing here rather than by writing harness config files, +//! because it is protocol rather than per-harness plumbing: it works for any +//! harness advertising `mcpCapabilities.http`, not only the one whose config +//! format hive happens to know. //! -//! Everything that would justify parsing is already decided before the first -//! byte moves. The agent is known from `HIVE_AGENT`, its harness from its spec, -//! its credentials and MCP servers from hived. There is nothing left to choose, -//! so there is nothing to intercept — and a proxy that re-frames JSON-RPC it -//! does not need to read is a proxy that can corrupt a stream it was only -//! supposed to carry. ACP's framing is also not something to assume: a byte -//! pipe is correct whether messages are newline-delimited or length-prefixed, -//! and stays correct when that changes. +//! The same request carries a `cwd`, and that one is a genuine impedance +//! mismatch rather than a missing feature: the client picks a directory on the +//! machine *it* is running on, and the harness is on the other side of a +//! container boundary where that path means nothing. `claude-agent-acp` +//! validates it and rejects the session outright, so a client that never asked +//! for a container gets `cwd does not exist on the machine running the agent` +//! and no session. hive substitutes the agent's workspace — but only when the +//! path is genuinely absent inside the container, so a deliberately +//! bind-mounted host path still resolves to itself. //! -//! The moment routing between several backends is wanted, this becomes a real -//! router. Until then, being a pipe is the feature. +//! **Everything else is copied verbatim, in both directions.** A proxy that +//! re-frames JSON-RPC it has no reason to read can corrupt a stream it was only +//! meant to carry, so anything that is not a `session/new` request — including +//! every response and notification coming back up — is passed through +//! untouched, and a line that does not parse as JSON is forwarded as bytes +//! rather than dropped. +//! +//! Routing between several backends, and switching harness mid-conversation, +//! would need real session bookkeeping. That is a different program. -use std::io::{Read, Write}; +use std::io::{BufRead, BufReader, Read, Write}; use std::process::{Command, Stdio}; use anyhow::{bail, Context, Result}; @@ -66,61 +90,951 @@ struct Config { container: String, /// The harness entrypoint and its arguments, from hive's catalog. argv: Vec, + /// HTTP MCP servers from this agent's spec, injected at `session/new`. + mcp: Vec, + /// Env vars this harness's model-provider credential can arrive in. + credential_env: Vec, + /// Where a file-shaped credential for this harness would be, if it has one. + credential_file: Option, + /// The spec's own harness id, when `HIVE_HARNESS` overrode it. + overridden: Option, + /// The parsed spec, kept so a hold can be explained before waiting on a + /// container that is never going to appear. + spec: AgentSpec, + /// Where hived lives, for the same reason. + daemon: String, +} + +#[derive(Clone)] +struct McpEntry { + name: String, + url: String, + /// Broker key. `None` for a server that needs no credential. + credential: Option, +} + +/// Ask the broker for a server's headers, from inside the container. +/// +/// The broker listens on a unix socket bind-mounted into the agent's container, +/// which this process — running on the host, possibly on the far side of a +/// Docker VM — cannot reach. `hive-headers` already lives in the image for +/// exactly this conversation, so it is reused rather than reimplemented: the +/// fetch stays grant-checked and audited, and there is one code path that talks +/// to the broker instead of two. +/// +/// A failure is logged and the server is attached WITHOUT credentials rather +/// than dropped. An MCP server the agent can see and gets 401 from is a +/// diagnosable problem; a server that silently vanished is not. +fn fetch_headers(container: &str, server: &str) -> Vec<(String, String)> { + let out = Command::new(find_docker()) + .args(["exec", "-e"]) + .arg(format!("CLAUDE_CODE_MCP_SERVER_NAME={server}")) + .args([container, "hive-headers"]) + .output(); + let out = match out { + Ok(o) if o.status.success() => o, + Ok(o) => { + eprintln!( + "hive-acp: no credentials for MCP server {server:?}: {}", + String::from_utf8_lossy(&o.stderr).trim() + ); + return Vec::new(); + } + Err(e) => { + eprintln!("hive-acp: could not reach the broker for {server:?}: {e}"); + return Vec::new(); + } + }; + let body = String::from_utf8_lossy(&out.stdout); + // The helper prints `{"headers": {name: value, …}}` — the map is NESTED, + // not the whole document. Parsing it flat yields zero headers and an MCP + // server that 401s, with nothing in the logs to say why. + let parsed: serde_json::Value = match serde_json::from_str(body.trim()) { + Ok(v) => v, + Err(e) => { + eprintln!("hive-acp: broker returned unparseable headers for {server:?}: {e}"); + return Vec::new(); + } + }; + let Some(map) = parsed.get("headers").and_then(|h| h.as_object()) else { + eprintln!("hive-acp: broker response for {server:?} had no headers object"); + return Vec::new(); + }; + map.iter() + .filter_map(|(k, v)| v.as_str().map(|s| (k.clone(), s.to_string()))) + .collect() +} + +/// Is this an existing directory *inside the container*? +/// +/// Used to tell a path the container genuinely has — a bind-mounted host +/// directory, say — from one that only exists on the client's machine. +fn dir_exists_in(container: &str, path: &str) -> bool { + Command::new(find_docker()) + .args(["exec", container, "test", "-d", path]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .map(|s| s.success()) + .unwrap_or(false) +} + +/// Where the harness should work when the client's `cwd` is a host path. +/// +/// `HIVE_ACP_WORKSPACE` wins, so an operator whose image is laid out +/// differently is not stuck. Otherwise `/home/agent/work`, which hive's agent +/// image creates for exactly this; then the image's own `WORKDIR`. The last +/// resort is `/`, which every container has — a wrong-but-present directory +/// still starts a session the operator can inspect, where a missing one just +/// reproduces the failure this function exists to prevent. +fn resolve_workspace(container: &str) -> String { + if let Some(v) = std::env::var("HIVE_ACP_WORKSPACE").ok().filter(|s| !s.is_empty()) { + return v; + } + if dir_exists_in(container, "/home/agent/work") { + return "/home/agent/work".to_string(); + } + let out = Command::new(find_docker()) + .args(["inspect", "--format", "{{.Config.WorkingDir}}", container]) + .output(); + if let Ok(o) = out { + let dir = String::from_utf8_lossy(&o.stdout).trim().to_string(); + if o.status.success() && !dir.is_empty() { + return dir; + } + } + "/".to_string() +} + +/// Adapt a `session/new` request to the container it is really headed for. +/// +/// Returns `None` for anything that is not a `session/new` request, including +/// lines that are not JSON — the caller then forwards the original bytes. +/// +/// Buzz's own `mcpServers` are preserved and hive's are appended, rather than +/// replaced: a stdio server configured in the desktop and an HTTP server +/// configured in hive are not alternatives, and silently dropping the former +/// would make hive's involvement look like a Buzz bug. +fn rewrite_session_new(line: &str, mcp: &[McpEntry], container: &str) -> Option { + inject( + line, + mcp, + &|e: &McpEntry| match &e.credential { + Some(_) => fetch_headers(container, &e.name), + None => Vec::new(), + }, + // Resolving the workspace costs two `docker` calls, so it is deferred + // until something actually needs it — which is only when the client + // sent a cwd the container does not have. + &|| resolve_workspace(container), + &|path: &str| dir_exists_in(container, path), + ) +} + +/// The pure half: everything except talking to the broker. +/// +/// Split out so the rewrite can be tested without Docker — the parts most +/// likely to be wrong are the passthrough conditions, not the header fetch. +fn inject( + line: &str, + mcp: &[McpEntry], + headers_for: &dyn Fn(&McpEntry) -> Vec<(String, String)>, + workspace: &dyn Fn() -> String, + dir_exists: &dyn Fn(&str) -> bool, +) -> Option { + let mut msg: serde_json::Value = serde_json::from_str(line.trim()).ok()?; + if msg.get("method")?.as_str()? != "session/new" { + return None; + } + let params = msg.get_mut("params")?.as_object_mut()?; + let mut rewrote = false; + + // The client chose this directory on its own machine. Replace it only when + // the container really lacks it — a spec that bind-mounts the very path + // Buzz is pointing at is a working setup, and redirecting that to the + // agent's workspace would quietly ignore what the operator asked for. + let stale_cwd = params + .get("cwd") + .and_then(|c| c.as_str()) + .filter(|cwd| !dir_exists(cwd)) + .map(String::from); + if let Some(cwd) = stale_cwd { + let target = workspace(); + eprintln!("hive-acp: cwd {cwd:?} is not in the container; using {target:?}"); + params.insert("cwd".into(), serde_json::Value::String(target)); + rewrote = true; + } + + if !mcp.is_empty() { + let servers = params + .entry("mcpServers") + .or_insert_with(|| serde_json::Value::Array(Vec::new())) + .as_array_mut()?; + for e in mcp { + let headers: Vec = headers_for(e) + .into_iter() + .map(|(name, value)| serde_json::json!({ "name": name, "value": value })) + .collect(); + // `type: "http"` selects the McpServerHttp variant of ACP's + // McpServer union. `headers` is required by the schema even when + // empty. + servers.push(serde_json::json!({ + "type": "http", + "name": e.name, + "url": e.url, + "headers": headers, + })); + eprintln!("hive-acp: attached MCP server {} -> {}", e.name, e.url); + } + rewrote = true; + } + + // Nothing to change: let the caller forward the original bytes rather than + // a re-serialized copy that differs in key order and spacing. + if !rewrote { + return None; + } + let mut s = msg.to_string(); + s.push('\n'); + Some(s) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn entry(name: &str) -> McpEntry { + McpEntry { + name: name.into(), + url: format!("https://vault.example/{name}/mcp"), + credential: Some(format!("mcp/{name}")), + } + } + fn creds(_: &McpEntry) -> Vec<(String, String)> { + vec![("Authorization".into(), "Bearer TOKEN".into())] + } + fn none(_: &McpEntry) -> Vec<(String, String)> { + Vec::new() + } + + /// A container whose only directory is the agent workspace — so any cwd the + /// client sends is a host path, which is the case that matters. + fn only_workspace(path: &str) -> bool { + path == "/home/agent/work" + } + fn workspace() -> String { + "/home/agent/work".to_string() + } + /// A container that has whatever it is asked for: nothing needs rewriting. + fn everything(_: &str) -> bool { + true + } + + /// The common case in these tests: MCP injection with a cwd that is already + /// valid inside the container, so only `mcpServers` moves. + fn inject_mcp_only( + line: &str, + mcp: &[McpEntry], + headers_for: &dyn Fn(&McpEntry) -> Vec<(String, String)>, + ) -> Option { + inject(line, mcp, headers_for, &workspace, &everything) + } + + #[test] + fn only_session_new_is_touched() { + // Everything else — prompts, cancels, and every response coming back — + // must reach the far side byte-for-byte. Rewriting a message this + // program has no reason to read is how a proxy corrupts a stream it was + // only meant to carry. + let e = [entry("parachute")]; + for line in [ + r#"{"jsonrpc":"2.0","id":3,"method":"session/prompt","params":{"sessionId":"s"}}"#, + r#"{"jsonrpc":"2.0","id":1,"method":"initialize","params":{}}"#, + r#"{"jsonrpc":"2.0","id":2,"result":{"sessionId":"s"}}"#, + ] { + assert!(inject_mcp_only(line, &e, &none).is_none(), "rewrote: {line}"); + } + } + + #[test] + fn a_line_that_is_not_json_is_passed_through_rather_than_dropped() { + // ACP framing is not something to assume. If messages ever stop being + // newline-delimited, this must degrade to a transparent pipe instead of + // silently swallowing traffic. + assert!(inject_mcp_only("not json at all", &[entry("p")], &none).is_none()); + assert!(inject_mcp_only("", &[entry("p")], &none).is_none()); + } + + #[test] + fn an_agent_with_no_mcp_servers_is_never_rewritten() { + let line = r#"{"jsonrpc":"2.0","id":2,"method":"session/new","params":{"mcpServers":[]}}"#; + assert!(inject_mcp_only(line, &[], &creds).is_none()); + } + + #[test] + fn the_clients_own_mcp_servers_are_kept_and_hives_are_appended() { + // A stdio server configured in Buzz and an HTTP server configured in + // hive are not alternatives. Dropping the client's would look like a + // Buzz bug rather than something hive did. + let line = r#"{"jsonrpc":"2.0","id":2,"method":"session/new","params":{"cwd":"/w","mcpServers":[{"name":"buzzy","command":"x","args":[],"env":[]}]}}"#; + let out = inject_mcp_only(line, &[entry("parachute")], &creds).expect("rewritten"); + let v: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); + let servers = v["params"]["mcpServers"].as_array().unwrap(); + assert_eq!(servers.len(), 2, "{out}"); + assert_eq!(servers[0]["name"], "buzzy"); + assert_eq!(servers[1]["name"], "parachute"); + assert_eq!(servers[1]["type"], "http"); + assert_eq!(v["params"]["cwd"], "/w", "unrelated params must survive"); + } + + #[test] + fn the_injected_server_matches_the_mcpserverhttp_variant() { + // ACP selects the variant by `type`, and `headers` is required by the + // schema even when empty. Emitting the stdio shape here means the + // harness never reaches the server at all. + let out = inject_mcp_only( + r#"{"jsonrpc":"2.0","id":2,"method":"session/new","params":{}}"#, + &[entry("parachute")], + &creds, + ) + .expect("rewritten"); + let v: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); + let s = &v["params"]["mcpServers"][0]; + assert_eq!(s["type"], "http"); + assert_eq!(s["url"], "https://vault.example/parachute/mcp"); + assert_eq!(s["headers"][0]["name"], "Authorization"); + assert_eq!(s["headers"][0]["value"], "Bearer TOKEN"); + } + + #[test] + fn a_server_whose_credential_cannot_be_fetched_is_still_attached() { + // Attached-and-401 is diagnosable; silently absent is not. The agent + // reports an authorization failure naming the server, which points at + // the broker rather than at a mystery. + let out = inject_mcp_only( + r#"{"jsonrpc":"2.0","id":2,"method":"session/new","params":{}}"#, + &[entry("parachute")], + &none, + ) + .expect("rewritten"); + let v: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); + assert_eq!(v["params"]["mcpServers"][0]["name"], "parachute"); + assert!(v["params"]["mcpServers"][0]["headers"].as_array().unwrap().is_empty()); + } + + #[test] + fn the_rewritten_line_stays_newline_terminated() { + // The child reads line-delimited JSON. Dropping the terminator makes + // the harness wait for the rest of a message that already arrived. + let out = inject_mcp_only( + r#"{"jsonrpc":"2.0","id":2,"method":"session/new","params":{}}"#, + &[entry("p")], + &none, + ) + .expect("rewritten"); + assert!(out.ends_with('\n'), "{out:?}"); + } + + #[test] + fn a_client_cwd_the_container_lacks_becomes_the_workspace() { + // The regression this exists for: Buzz sends the directory it is + // sitting in on the host, `claude-agent-acp` checks it inside the + // container, and rejects the session with `cwd does not exist on the + // machine running the agent` — which reads as a hive failure with no + // hint that a path was the problem. + let line = r#"{"jsonrpc":"2.0","id":2,"method":"session/new","params":{"cwd":"/private/tmp","mcpServers":[]}}"#; + let out = inject(line, &[], &none, &workspace, &only_workspace).expect("rewritten"); + let v: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); + assert_eq!(v["params"]["cwd"], "/home/agent/work"); + } + + #[test] + fn a_cwd_the_container_really_has_is_left_alone() { + // A spec can bind-mount a host directory into the container at the same + // path. Redirecting that to the workspace would silently ignore the + // mount the operator configured on purpose. + let line = r#"{"jsonrpc":"2.0","id":2,"method":"session/new","params":{"cwd":"/srv/shared"}}"#; + assert!( + inject(line, &[], &none, &workspace, &everything).is_none(), + "a valid cwd and no MCP servers is nothing to rewrite" + ); + } + + #[test] + fn the_cwd_is_fixed_even_for_an_agent_with_no_mcp_servers() { + // These are independent reasons to rewrite. Gating the cwd fix on + // having MCP servers would leave the plainest possible agent — no MCP + // at all — as the one that cannot open a session. + let line = r#"{"jsonrpc":"2.0","id":2,"method":"session/new","params":{"cwd":"/Users/someone/code"}}"#; + let out = inject(line, &[], &creds, &workspace, &only_workspace).expect("rewritten"); + let v: serde_json::Value = serde_json::from_str(out.trim()).unwrap(); + assert_eq!(v["params"]["cwd"], "/home/agent/work"); + assert!(v["params"].get("mcpServers").is_none(), "invented an mcpServers key"); + } + + #[test] + fn a_stale_cwd_on_any_other_method_is_not_touched() { + // Only `session/new` declares a cwd. Rewriting a field that happens to + // share the name on some other method would corrupt it. + let line = r#"{"jsonrpc":"2.0","id":3,"method":"session/prompt","params":{"cwd":"/private/tmp"}}"#; + assert!(inject(line, &[entry("p")], &creds, &workspace, &only_workspace).is_none()); + } + + #[test] + fn the_workspace_is_not_resolved_when_the_cwd_is_already_good() { + // Resolving it costs two `docker` calls per session. This asserts the + // laziness rather than trusting the call order to stay that way. + let line = r#"{"jsonrpc":"2.0","id":2,"method":"session/new","params":{"cwd":"/srv/shared"}}"#; + let calls = std::cell::Cell::new(0); + let counting = || { + calls.set(calls.get() + 1); + "/home/agent/work".to_string() + }; + let _ = inject(line, &[entry("p")], &creds, &counting, &everything); + assert_eq!(calls.get(), 0, "resolved the workspace it did not need"); + } +} + +/// Every credential key the broker currently holds. +fn stored_secrets(daemon: &str) -> Vec { + Command::new(find_docker()) + .args(["exec", daemon, "hive", "secret", "list"]) + .output() + .ok() + .filter(|o| o.status.success()) + .map(|o| { + String::from_utf8_lossy(&o.stdout) + .lines() + .map(|l| l.trim().to_string()) + .filter(|l| !l.is_empty()) + .collect() + }) + .unwrap_or_default() +} + +/// Name the credentials this spec needs that the broker does not hold. +/// +/// hived HOLDS an agent whose credentials are missing rather than starting one +/// that cannot work — correct, and invisible: the reason goes to the daemon's +/// log, while the caller sits waiting for a container that is never coming and +/// eventually reports a timeout. This turns that into a sentence naming the key. +fn missing_credentials(daemon: &str, spec: &AgentSpec, agent: &str) -> Vec { + let Ok(reqs) = hive_core::agent::requirements(spec, agent) else { + return Vec::new(); + }; + let out = Command::new(find_docker()) + .args(["exec", daemon, "hive", "secret", "list"]) + .output(); + let Ok(out) = out else { return Vec::new() }; + if !out.status.success() { + return Vec::new(); + } + let held: Vec = String::from_utf8_lossy(&out.stdout) + .lines() + .map(|l| l.trim().to_string()) + .filter(|l| !l.is_empty()) + .collect(); + reqs.iter() + .map(|r| r.key.as_str().to_string()) + .filter(|k| !held.iter().any(|h| h == k)) + .collect() +} + +/// Block until the container is running, or give up and let the exec fail. +/// +/// Returns rather than erroring on timeout: the `docker exec` that follows +/// produces a better message than anything this function could invent, and a +/// container that is merely slow should not be reported as absent. +fn wait_for_container(container: &str) { + const PATIENCE: std::time::Duration = std::time::Duration::from_secs(90); + let deadline = std::time::Instant::now() + PATIENCE; + let mut announced = false; + loop { + let running = Command::new(find_docker()) + .args(["inspect", "--format", "{{.State.Running}}", container]) + .output() + .map(|o| o.status.success() && String::from_utf8_lossy(&o.stdout).trim() == "true") + .unwrap_or(false); + if running || std::time::Instant::now() >= deadline { + if announced && running { + eprintln!("hive-acp: {container} is up"); + } + return; + } + if !announced { + eprintln!("hive-acp: waiting for {container} — hived reconciles on a timer"); + announced = true; + } + std::thread::sleep(std::time::Duration::from_secs(2)); + } +} + +/// The harness to run: `--harness` first, then `HIVE_HARNESS`. +fn chosen_harness() -> Option { + flag("harness").or_else(|| std::env::var("HIVE_HARNESS").ok().filter(|s| !s.is_empty())) +} + +/// Where operator-wide environment defaults live. +/// +/// On the secrets volume rather than the spec directory, because hived reads +/// every `*.toml` in the latter as an agent and would try to reconcile this +/// one. +const DEFAULTS_PATH: &str = "/var/lib/hive/defaults.toml"; + +/// TOML appended to every environment hive creates. Empty when absent. +fn read_defaults(daemon: &str) -> String { + Command::new(find_docker()) + .args(["exec", daemon, "cat", DEFAULTS_PATH]) + .output() + .ok() + .filter(|o| o.status.success()) + .map(|o| { + let text = String::from_utf8_lossy(&o.stdout).to_string(); + if text.trim().is_empty() { text } else { format!("\n{}", text.trim_end()) } + }) + .unwrap_or_default() } -/// Resolve everything from the agent name plus its spec. +/// Create an environment that does not exist yet, through the daemon. +/// +/// Deliberately minimal. An environment is a container to run a harness in: +/// identity belongs to whatever spawned this process, credentials are named +/// rather than stored in a spec, and MCP servers are added afterwards with +/// `hive mcp add`. So the generated file says only which harness and which +/// mode, and everything else is a considered addition rather than a default +/// somebody has to discover and undo. +/// +/// `HIVE_HARNESS` picks the harness when set, so the Buzz entry that named a +/// harness also gets one configured for it. +fn create_environment(daemon: &str, name: &str) -> Result<()> { + let harness = chosen_harness().unwrap_or_else(|| "claude".to_string()); + + // Reuse a credential this box already holds, in the FORM the harness reads + // it. + // + // Defaulting to broker+env is right for claude, whose credential is a token + // in CLAUDE_CODE_OAUTH_TOKEN — and wrong for codex and grok, whose normal + // auth is a JSON file. A spec that asks for `harness/codex` when what exists + // is `codex/auth` is held forever for a credential nobody is going to + // create, and the new agent simply never starts. + // + // Both key spellings are tried because both are in use: `harness/` is + // what hive asks for by default, `/auth` is what a file credential gets + // called when it is stored by hand. + let def = CATALOG.iter().find(|h| h.id == harness); + let held = stored_secrets(daemon); + + // A harness that owns and rewrites its credential file cannot be handed a + // copy of somebody else's. Declaring `auth = "file"` for one of those is + // worse than declaring nothing: hived injects a snapshot, the harness finds + // it stale, cannot refresh a token whose refresh half has already been + // rotated elsewhere, and deletes it — leaving an unauthenticated container + // whose spec says a credential was delivered. + let auth_block = if def.is_some_and(|d| d.credential_file_rotates) { + eprintln!( + "hive-acp: {harness} manages its own credential and rotates it, so it cannot be \ + injected. Log in once inside this environment:\n\ + \x20 hive shell {name} -- grok login --device-auth" + ); + // Not `broker`: hived would hold the agent forever waiting for a key + // that by design lives in the state volume, and hold it so hard the + // container needed to log in never starts. + "auth = \"interactive\"\n".to_string() + } else { + match def + .and_then(|d| d.credential_file) + .and_then(|path| { + [format!("harness/{harness}"), format!("{harness}/auth")] + .into_iter() + .find(|k| held.iter().any(|h| h == k)) + .map(|key| (key, path)) + }) { + Some((key, path)) => { + eprintln!("hive-acp: {harness} authenticates from a file; using {key}"); + format!( + "auth = \"file\"\n\ + \n\ + [[file]]\n\ + credential = {key:?}\n\ + target = {path:?}\n\ + mode = \"0600\"\n" + ) + } + None => String::new(), + } + }; + + // Anything the operator wants every new environment to have. + // + // Without this a generated environment has no MCP servers at all, so the + // vault every agent is supposed to reach is reachable only from the one + // environment somebody configured by hand. Defaults are appended verbatim + // and validated by `spec-put`, so a broken defaults file fails at creation + // with a reason rather than producing an agent that quietly lacks its + // tools. + // + // NOT in the spec directory: hived treats every *.toml there as an agent, + // so a defaults file next to the specs would be reconciled as one. + let defaults = read_defaults(daemon); + if !defaults.trim().is_empty() { + eprintln!("hive-acp: applying environment defaults from {DEFAULTS_PATH}"); + } + + let spec = format!( + "# Created by hive-acp for Buzz agent {name:?}.\n\ + # An environment: a container to run a harness in. The identity lives\n\ + # with buzz-acp outside it, which is why there is no [identity] block.\n\ + #\n\ + # Add MCP servers with `hive mcp add --url --agent {name}`.\n\ + \n\ + [harness]\n\ + id = {harness:?}\n\ + {auth_block}\ + \n\ + [agent]\n\ + mode = \"environment\"\n\ + {defaults}" + ); + + eprintln!("hive-acp: {name:?} has no environment yet; creating one (harness {harness})"); + let mut child = Command::new(find_docker()) + .args(["exec", "-i", daemon, "hive", "spec-put", name]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .with_context(|| format!("creating an environment for {name} via {daemon}"))?; + child + .stdin + .take() + .context("child stdin")? + .write_all(spec.as_bytes())?; + let out = child.wait_with_output()?; + if !out.status.success() { + bail!( + "could not create an environment for {name}: {}", + String::from_utf8_lossy(&out.stderr).trim() + ); + } + // hived reconciles on a timer, so the container does not exist yet. The + // caller only needs the SPEC to read a harness and MCP list out of; the + // container is waited for where it is actually used. + Ok(()) +} + +/// A `--harness ` / `--env ` flag, if present on the command line. +/// +/// Reading these from ARGV and not only from the environment is load-bearing on +/// the deploy path. Buzz layers a harness definition's `env` into a LOCAL spawn +/// (`readiness.rs`, "definition env"), but `build_deploy_payload` merges only +/// global, persona and per-agent env — the definition's is dropped. So a +/// harness entry that carries `HIVE_HARNESS` in `env` works perfectly on this +/// computer and silently becomes the default harness when deployed to another +/// one: three agents created as claude, codex and grok all came up as claude, +/// with nothing in any log to say why. +/// +/// `agent_args` IS in the deploy payload, so it survives the trip. +fn flag(name: &str) -> Option { + let args: Vec = std::env::args().skip(1).collect(); + let mut i = 0; + while i < args.len() { + let a = &args[i]; + if let Some(v) = a.strip_prefix(&format!("--{name}=")) + && !v.is_empty() + { + return Some(v.to_string()); + } + if a == &format!("--{name}") + && let Some(v) = args.get(i + 1) + && !v.is_empty() + { + return Some(v.clone()); + } + i += 1; + } + None +} + +/// Which hive environment to run in — `HIVE_ENV`, or the older `HIVE_AGENT`. +/// +/// It selects a **container**: image, state volume, network, credentials, MCP +/// servers. It says nothing about who the agent is. In `mode = "environment"` +/// the container has no Nostr key at all — `buzz-acp` holds it on the host and +/// Buzz strips it from the harness environment before spawning. +/// +/// So `HIVE_AGENT` named the wrong thing. "Agent" is what Buzz calls the entity +/// with the identity, and several Buzz agents pointing at one `HIVE_AGENT` were +/// silently one container sharing skills, sessions and credentials — a bug the +/// name actively hid. `HIVE_ENV` says what it selects. +/// +/// The old name still works, with a warning rather than a break: it is written +/// into harness definitions that already exist, and a rename that takes an +/// agent offline mid-conversation to make a point is not an improvement. +fn env_name() -> String { + let read = |k: &str| std::env::var(k).ok().filter(|s| !s.is_empty()); + if let Some(v) = flag("env").or_else(|| read("HIVE_ENV")) { + return v; + } + if let Some(v) = read("HIVE_AGENT") { + eprintln!( + "hive-acp: HIVE_AGENT is deprecated, use HIVE_ENV. It selects a container, not an \ + identity — and two Buzz agents sharing one value share one container." + ); + return v; + } + // The name the supervisor gave this agent. + // + // A deployed agent should not need an environment variable set by hand + // before it can run, and asking for one invites the mistake it was meant to + // prevent: reusing a neighbour's value, and silently sharing that + // container's sessions, skills and credentials. buzz-host publishes its + // unit name, which is already unique per agent on that machine. + if let Some(v) = read("BUZZ_HOST_UNIT") { + eprintln!("hive-acp: HIVE_ENV unset; using this agent's unit name {v:?}"); + return v; + } + + // Nothing has named this agent, which means it does not exist yet: the + // desktop is asking a harness what models it supports while the user is + // still filling in the dialog. Failing here leaves the model list empty + // with no way to fill it — there is nothing to set HIVE_ENV *to* before the + // agent exists, and pinning one in the harness definition is exactly what + // put every agent in a single shared container in the first place. + // + // So discovery gets a scratch environment. One PER HARNESS, because + // `session/new` needs that harness's own credentials before it will answer + // — a single shared probe container would report claude's models for every + // entry, or fail outright for the ones it cannot authenticate. + let scratch = format!("probe-{}", chosen_harness().unwrap_or_else(|| "claude".into())); + eprintln!( + "hive-acp: no HIVE_ENV and no supervisor — using the shared discovery environment \ + {scratch:?}. A real agent sets HIVE_ENV, or is deployed by something that names it." + ); + scratch +} + +/// Resolve everything from the environment name plus its spec. /// /// The harness is read from the spec rather than from an environment variable /// because the spec is what hived reconciled the container from. Taking it from /// the environment would let the two disagree — and the symptom would be a /// harness that starts, answers `initialize`, and has none of the credentials /// the container was built for. +/// Read an environment's spec through the daemon, or locally when visible. +/// +/// Where hived runs in a container — which on macOS and Windows it must, +/// because the broker's unix sockets cannot cross the Docker VM boundary — the +/// spec directory is a volume this process cannot see. Requiring the operator +/// to bind-mount it onto the host as well would mean two sources of truth for +/// the same file, and the one this program read would be the one nobody edited. +fn read_spec(spec_dir: &str, daemon: &str, name: &str) -> Option { + let path = std::path::Path::new(spec_dir).join(format!("{name}.toml")); + if let Ok(t) = std::fs::read_to_string(&path) { + return Some(t); + } + let out = Command::new(find_docker()) + .args(["exec", daemon, "cat"]) + .arg(&path) + .output() + .ok()?; + out.status.success().then(|| String::from_utf8_lossy(&out.stdout).to_string()) +} + fn resolve() -> Result { - let agent = std::env::var("HIVE_AGENT").ok().filter(|s| !s.is_empty()).context( - "HIVE_AGENT is not set. hive-acp runs one specific agent's harness; set it in the \ - harness definition's env, or per-agent in Buzz's agent environment variables.", - )?; + let mut agent = env_name(); let spec_dir = std::env::var("HIVE_SPEC_DIR").unwrap_or_else(|_| "/etc/hive/agents".to_string()); - let path = std::path::Path::new(&spec_dir).join(format!("{agent}.toml")); - let text = std::fs::read_to_string(&path) - .with_context(|| format!("reading {} — has this agent been deployed?", path.display()))?; - let spec: AgentSpec = - toml::from_str(&text).with_context(|| format!("{} is not a valid agent spec", path.display()))?; + let daemon = std::env::var("HIVE_DAEMON_CONTAINER").unwrap_or_else(|_| "hived".to_string()); + + // Creating an environment is for a REAL agent, never for a model-discovery + // probe. + // + // The desktop re-probes the harness on every keystroke while someone types + // into the HIVE_ENV field, so provisioning unconditionally created one + // environment per keystroke: typing "uni" left behind `u` and `un`, each + // with a spec, a container and a volume, all within the same second. + // + // A deployed agent carries BUZZ_HOST_UNIT — its supervisor named it. A + // probe does not. So a probe naming an environment that does not exist + // falls back to the shared discovery one rather than conjuring it. + let text = match read_spec(&spec_dir, &daemon, &agent) { + Some(t) => t, + None => { + let deployed = + std::env::var("BUZZ_HOST_UNIT").ok().filter(|s| !s.is_empty()).is_some(); + if !deployed { + let scratch = + format!("probe-{}", chosen_harness().unwrap_or_else(|| "claude".into())); + eprintln!( + "hive-acp: {agent:?} does not exist and nothing has deployed it, so this is \ + a discovery probe — using {scratch:?} instead of creating it. Deploy the \ + agent, or create the environment with `hive spec-put`." + ); + agent = scratch; + } + match read_spec(&spec_dir, &daemon, &agent) { + Some(t) => t, + None => { + // Either a real deployment whose environment does not exist + // yet, or the discovery environment on its first ever use. + create_environment(&daemon, &agent)?; + read_spec(&spec_dir, &daemon, &agent).with_context(|| { + format!( + "created {agent} but still cannot read its spec via {daemon}. \ + Is hived running? `hive status`" + ) + })? + } + } + } + }; + let spec: AgentSpec = toml::from_str(&text) + .with_context(|| format!("{agent} does not have a valid agent spec"))?; // A spec names either a catalog id or an explicit command. The escape hatch // is honoured here too: a harness the catalog does not know still runs, it // just brings its own image, and refusing it would make hive-acp stricter // than hived about the very same spec. - let argv: Vec = match (&spec.harness.id, &spec.harness.command) { + // `HIVE_HARNESS` overrides the spec's harness id. + // + // The image carries every harness in the catalog, so one container can run + // any of them — which is what lets a single `hive-acp` binary back several + // Buzz harness entries (`hive (claude)`, `hive (codex)`, …) instead of one + // per agent. Buzz's harness picker then stays the harness picker, and each + // entry reports the models its own harness actually supports rather than a + // merged list hive would have to maintain. + // + // What it does NOT move is credentials. hived provisions the container from + // the spec, so the only model-provider credential present is the spec + // harness's. Overriding to a harness the container cannot authenticate is + // allowed — refusing would make it impossible to start a container in order + // to log in — but it is called out below rather than left to surface as an + // unexplained "authentication required" three steps later. + let override_id = chosen_harness(); + let effective_id = override_id.as_deref().or(spec.harness.id.as_deref()); + + let mut credential_env: Vec = Vec::new(); + let mut credential_file: Option = None; + let argv: Vec = match (effective_id, &spec.harness.command) { (Some(id), _) => { - let def = CATALOG.iter().find(|h| h.id == id.as_str()).with_context(|| { + let def = CATALOG.iter().find(|h| h.id == id).with_context(|| { format!( - "{agent} names harness {id:?}, which is not in hive's catalog. \ + "harness {id:?} is not in hive's catalog. \ `hive harnesses` lists the ids this build knows." ) })?; if let Some(reason) = &def.unsupported { bail!("harness {id:?} is deliberately absent from the image: {reason:?}"); } + credential_env = def.credential_env.iter().map(|s| s.to_string()).collect(); + credential_file = def.credential_file.map(String::from); let mut v = vec![def.command.to_string()]; v.extend(def.args.iter().map(|s| s.to_string())); v } + // An explicit command wins only when no id is in play at all. A spec + // that names both, plus an override, would otherwise silently run the + // command and ignore the harness that was just picked. (None, Some(cmd)) => cmd.split_whitespace().map(String::from).collect(), (None, None) => bail!( "{agent} names neither [harness].id nor [harness].command, so there is nothing to run" ), }; + let overridden = match (&override_id, &spec.harness.id) { + (Some(o), Some(s)) if o != s => Some(s.clone()), + _ => None, + }; + + // Only HTTP servers are attached over ACP. A stdio server in a spec is + // hived's business — it is spawned inside the container — and duplicating + // it here would start a second copy. + let mcp = spec + .mcp + .iter() + .filter(|m| m.url.is_some()) + .map(|m| McpEntry { + name: m.name.clone(), + url: m.url.clone().unwrap_or_default(), + credential: m.credential.clone(), + }) + .collect(); + Ok(Config { container: std::env::var("HIVE_CONTAINER").unwrap_or_else(|_| format!("hive-{agent}")), + daemon: std::env::var("HIVE_DAEMON_CONTAINER").unwrap_or_else(|_| "hived".to_string()), + spec, agent, argv, + mcp, + credential_env, + credential_file, + overridden, }) } +/// Warn when the chosen harness has no credential in the container. +/// +/// Only worth saying when the harness was overridden: for the spec's own +/// harness, hived already refuses to start the agent without its credential, so +/// a missing one there is not this program's news to break. +fn warn_if_unauthenticated(cfg: &Config) { + let Some(spec_harness) = &cfg.overridden else { + return; + }; + if cfg.credential_env.is_empty() && cfg.credential_file.is_none() { + return; + } + + // Subscription auth is the normal way these tools are used, and for codex + // it is a file rather than an env var. Checking only the environment + // reported a working container as unauthenticated — the warning fired, and + // then codex authenticated from auth.json and listed its models. A false + // alarm here is worse than none: it teaches the reader to ignore the line + // that will one day be true. + let env_present = cfg.credential_env.iter().any(|var| { + Command::new(find_docker()) + .args(["exec", &cfg.container, "printenv", var]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .map(|s| s.success()) + .unwrap_or(false) + }); + let file_present = cfg + .credential_file + .as_deref() + .is_some_and(|path| file_exists_in(&cfg.container, path)); + if env_present || file_present { + return; + } + + let harness = cfg.argv.first().map(String::as_str).unwrap_or("the harness"); + let mut wanted = cfg.credential_env.clone(); + if let Some(path) = &cfg.credential_file { + wanted.push(path.clone()); + } + eprintln!( + "hive-acp: harness overridden to {harness} but this container was provisioned for \ + {spec_harness}, and none of {} is present. The harness will report that authentication \ + is required. Give {harness} its own agent, or add its credential to {}'s spec.", + wanted.join(" / "), + cfg.agent, + ); +} + +/// Is this an existing regular file inside the container? +fn file_exists_in(container: &str, path: &str) -> bool { + Command::new(find_docker()) + .args(["exec", container, "test", "-f", path]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .map(|s| s.success()) + .unwrap_or(false) +} + fn main() -> Result<()> { // stderr, never stdout: stdout is the ACP channel and one stray line of // logging corrupts the stream. The failure looks like a harness that @@ -134,11 +1048,12 @@ fn main() -> Result<()> { }; eprintln!( - "hive-acp: agent={} container={} harness={}", + "hive-acp: env={} container={} harness={}", cfg.agent, cfg.container, cfg.argv.join(" ") ); + warn_if_unauthenticated(&cfg); // -i, no -t. There is no terminal here, and `docker exec -t` fails outright // when stdin is a pipe — which it always is, because buzz-acp owns it. @@ -155,6 +1070,25 @@ fn main() -> Result<()> { cmd.arg(&cfg.container); cmd.args(&cfg.argv); + // A freshly created environment has a spec but not yet a container: hived + // reconciles on a timer. Without this the first session after creating an + // agent fails with "no such container" and the second one works, which + // reads as flakiness rather than as a wait. + // Say why before waiting, not after. hived holds an agent whose credentials + // are missing, so the container never appears and the wait always burns its + // full patience before failing with a timeout that names nothing. + let missing = missing_credentials(&cfg.daemon, &cfg.spec, &cfg.agent); + if !missing.is_empty() { + eprintln!( + "hive-acp: {} is held — the broker has no {}. Store it with `hive secret put {}`, \ + or give this environment a file credential if that is how the harness authenticates.", + cfg.agent, + missing.join(" or "), + missing.first().map(String::as_str).unwrap_or(""), + ); + } + wait_for_container(&cfg.container); + let mut child = cmd .stdin(Stdio::piped()) .stdout(Stdio::piped()) @@ -189,18 +1123,28 @@ fn main() -> Result<()> { } }); + let mcp = cfg.mcp.clone(); + let container = cfg.container.clone(); let down = std::thread::spawn(move || { - let mut buf = [0u8; 16 * 1024]; - let mut stdin = std::io::stdin().lock(); + // Line-oriented only in this direction, and only to find `session/new`. + // A line that is not JSON, or is any other method, is written through + // byte-for-byte — so a framing change or an unknown method degrades to + // the transparent behaviour rather than to a corrupted stream. + let stdin = std::io::stdin(); + let mut reader = BufReader::new(stdin.lock()); + let mut line = String::new(); loop { - match stdin.read(&mut buf) { - Ok(0) => break, - Ok(n) => { - if to_child.write_all(&buf[..n]).is_err() || to_child.flush().is_err() { - break; - } - } - Err(_) => break, + line.clear(); + match reader.read_line(&mut line) { + Ok(0) | Err(_) => break, + Ok(_) => {} + } + let out = match rewrite_session_new(&line, &mcp, &container) { + Some(modified) => modified, + None => line.clone(), + }; + if to_child.write_all(out.as_bytes()).is_err() || to_child.flush().is_err() { + break; } } // Closing the child's stdin is what tells the harness the client is diff --git a/crates/hive-cli/src/main.rs b/crates/hive-cli/src/main.rs index 2069b16..3ae173f 100644 --- a/crates/hive-cli/src/main.rs +++ b/crates/hive-cli/src/main.rs @@ -45,6 +45,23 @@ struct Cli { enum Command { /// Check a spec file without deploying it. Works offline. Validate { file: PathBuf }, + /// Install an agent spec, read from stdin. + /// + /// The only way to get a spec to the daemon that does not assume the spec + /// directory is a path on the caller's filesystem. Where hived runs in a + /// container — which on macOS and Windows it must — `/etc/hive/agents` is a + /// volume, so `ssh host 'cat > /etc/hive/agents/x.toml'` fails with + /// "Permission denied" on a directory the host does not have. Routing + /// through the CLI means one code path serves a native daemon, a + /// containerized one, and a remote one over ssh. + /// + /// Validated before it lands: a spec that would be rejected never reaches + /// the directory, so hived is not asked to reconcile something it will only + /// hold and log about. + SpecPut { + /// Agent name. The spec is written as `.toml`. + name: String, + }, /// Which harnesses this build can run, and which it refuses. Harnesses, /// What the daemon currently believes. @@ -184,6 +201,7 @@ fn main() -> Result<()> { let cli = Cli::parse(); match &cli.command { Command::Validate { file } => validate(file), + Command::SpecPut { name } => spec_put(&cli.spec_dir, name), Command::Harnesses => harnesses(), Command::Status => status(&cli.control_socket), Command::Ps => ps(), @@ -216,6 +234,62 @@ fn main() -> Result<()> { } } +/// Install a spec read from stdin, after validating it. +/// +/// Rejects a bad spec BEFORE it lands. hived holds an invalid spec and logs +/// about it, which is correct but happens on the far side of an ssh call whose +/// output nobody is reading — the deploy reports success and the agent never +/// appears. Failing here puts the reason on the caller's stderr. +fn spec_put(spec_dir: &std::path::Path, name: &str) -> Result<()> { + // The name becomes a filename in a directory hive owns. A traversal here + // would let a deploy write anywhere the daemon can reach. + if name.is_empty() + || !name + .chars() + .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '-' || c == '_') + { + bail!( + "invalid agent name {name:?}: use lowercase letters, digits, '-' and '_'. \ + It becomes a filename and a container name." + ); + } + + let mut text = String::new(); + std::io::Read::read_to_string(&mut std::io::stdin(), &mut text) + .context("reading the spec from stdin")?; + if text.trim().is_empty() { + bail!("no spec on stdin"); + } + + let spec = AgentSpec::from_toml(&text).context("parsing spec")?; + let report = spec.validate(); + for w in &report.warnings { + eprintln!("warning: {w}"); + } + if !report.errors.is_empty() { + for e in &report.errors { + eprintln!("error: {e}"); + } + bail!("spec is not valid; nothing written"); + } + agent::resolve_harness(&spec)?; + + std::fs::create_dir_all(spec_dir) + .with_context(|| format!("creating {}", spec_dir.display()))?; + let path = spec_dir.join(format!("{name}.toml")); + // Write-then-rename: hived watches this directory, and a partially written + // file is a spec it will try to parse and reject. + let tmp = spec_dir.join(format!(".{name}.toml.tmp")); + std::fs::write(&tmp, text.as_bytes()) + .with_context(|| format!("writing {}", tmp.display()))?; + std::fs::rename(&tmp, &path) + .with_context(|| format!("installing {}", path.display()))?; + + println!("wrote {} ({} bytes)", path.display(), text.len()); + println!("hived will reconcile it on its next pass; `hive status` to watch."); + Ok(()) +} + fn validate(file: &PathBuf) -> Result<()> { let text = std::fs::read_to_string(file) .with_context(|| format!("reading {}", file.display()))?; diff --git a/crates/hive-cli/src/mcp.rs b/crates/hive-cli/src/mcp.rs index 961d002..0193ec6 100644 --- a/crates/hive-cli/src/mcp.rs +++ b/crates/hive-cli/src/mcp.rs @@ -181,14 +181,29 @@ pub fn login( println!(" authorization server: {}", auth.issuer); println!(" resource: {}", auth.resource); - // Requested scopes default to everything the resource advertises. Narrowing - // is a deliberate act (--scope), because a token that silently lacks write - // fails at the first create-note rather than at login. + // Default to everything the resource advertises; `--scope ""` sends none. + // + // Letting the consent screen decide is the better model in principle — it + // knows which scopes exist, which this user may hold, and how to ask, while + // `scopes_supported` describes what the RESOURCE understands rather than + // what the server will grant. Naming scopes from it caps the token at the + // published set and cannot reach anything outside it. + // + // It is not the better DEFAULT, because a server that receives no scope is + // free to grant nothing, and Parachute does exactly that: the consent + // screen offers no choices and the resulting token is useless. A default + // that depends on the server having an opinion fails silently on the ones + // that do not. + // + // So: advertised scopes by default, and `--scope ""` for a server whose + // consent screen should own the decision. let requested: Vec = match scopes { Some(s) => s.split(&[',', ' '][..]).filter(|p| !p.is_empty()).map(String::from).collect(), None => auth.scopes_supported.clone(), }; - if !requested.is_empty() { + if requested.is_empty() { + println!(" scopes: (none requested — the consent screen decides)"); + } else { println!(" scopes: {}", requested.join(" ")); } diff --git a/crates/hive-core/src/agent.rs b/crates/hive-core/src/agent.rs index e193ad2..b5d80ab 100644 --- a/crates/hive-core/src/agent.rs +++ b/crates/hive-core/src/agent.rs @@ -59,15 +59,28 @@ pub fn resolve_harness(spec: &AgentSpec) -> Result<&'static HarnessDef, PlanErro /// cannot work. pub fn requirements(spec: &AgentSpec, agent: &str) -> Result, PlanError> { let h = resolve_harness(spec)?; - let mut reqs = vec![Requirement { - // Not derived from the agent name: several specs may share one identity - // across relays, and the secret key must live in exactly one place. - key: CredentialKey::new(spec.identity.credential_key(agent)), - // The agent's Nostr identity. Necessarily an env var: buzz-acp reads - // BUZZ_PRIVATE_KEY at startup and there is no file or helper form. - delivery: Delivery::Env { var: "BUZZ_PRIVATE_KEY".into() }, - purpose: "the agent's Nostr identity; without it it cannot join the relay at all", - }]; + let mut reqs = Vec::new(); + + // The Nostr identity, but ONLY when this container connects to a relay. + // + // An environment container never starts buzz-acp — the desktop's does, and + // it holds the identity. Demanding an nsec here would hold the agent + // forever waiting for a key that by design lives somewhere else, and the + // hold reads as a missing credential rather than as a mode mismatch. + if spec.agent.mode.unwrap_or_default() == hive_spec::AgentMode::Relay + && let Some(identity) = &spec.identity + { + reqs.push(Requirement { + // Not derived from the agent name: several specs may share one + // identity across relays, and the secret key must live in exactly + // one place. + key: CredentialKey::new(identity.credential_key(agent)), + // Necessarily an env var: buzz-acp reads BUZZ_PRIVATE_KEY at + // startup and there is no file or helper form. + delivery: Delivery::Env { var: "BUZZ_PRIVATE_KEY".into() }, + purpose: "the agent's Nostr identity; without it it cannot join the relay at all", + }); + } // The model-provider credential, but only when it is actually an env var. // Codex subscription auth is a file with no env form, and an interactively @@ -78,8 +91,16 @@ pub fn requirements(spec: &AgentSpec, agent: &str) -> Result, P if spec.harness.auth == hive_spec::HarnessAuth::Broker && let Some(var) = h.credential_env.first() { + // The spec may point at a different key: a second subscription, or + // someone else's tokens. Defaults to `harness/` so the common case + // stays untyped. + let key = spec + .harness + .credential + .clone() + .unwrap_or_else(|| format!("harness/{}", h.id)); reqs.push(Requirement { - key: CredentialKey::new(format!("harness/{}", h.id)), + key: CredentialKey::new(key), delivery: Delivery::Env { var: (*var).to_string() }, purpose: "the model provider credential; without it the agent joins and cannot think", }); @@ -131,27 +152,37 @@ pub fn mcp_token_env(server: &str) -> String { pub fn environment(spec: &AgentSpec, h: &HarnessDef, agent: &str) -> Result, PlanError> { let mut env = BTreeMap::new(); - env.insert("BUZZ_RELAY_URL".into(), spec.identity.relay_url.clone()); - - // Owner attestation. NIP-OA is preferred: the agent then derives relay - // access from its owner's membership (NIP-AA virtual membership) instead of - // needing its own enrollment. - match (&spec.identity.auth_tag, &spec.identity.owner_pubkey) { - (Some(tag), _) => { - env.insert("BUZZ_AUTH_TAG".into(), tag.clone()); - } - (None, Some(owner)) => { - env.insert("BUZZ_ACP_AGENT_OWNER".into(), owner.clone()); + // An environment has no identity: buzz-acp runs outside the container and + // holds it. Setting a relay url or an owner here would be describing a + // connection this container never makes. + if let Some(identity) = &spec.identity { + env.insert("BUZZ_RELAY_URL".into(), identity.relay_url.clone()); + + // Owner attestation. NIP-OA is preferred: the agent then derives relay + // access from its owner's membership (NIP-AA virtual membership) instead + // of needing its own enrollment. + match (&identity.auth_tag, &identity.owner_pubkey) { + (Some(tag), _) => { + env.insert("BUZZ_AUTH_TAG".into(), tag.clone()); + } + (None, Some(owner)) => { + env.insert("BUZZ_ACP_AGENT_OWNER".into(), owner.clone()); + } + // Without either, the harness starts, connects, and responds to + // nobody. It looks completely healthy. Validation rejects this too; + // this is the second gate, because the cost of missing it is hours. + (None, None) => return Err(PlanError::NoOwner(agent.to_string())), } - // Without either, the harness starts, connects, and responds to nobody. - // It looks completely healthy. Validation rejects this too; this is the - // second gate, because the cost of missing it is hours. - (None, None) => return Err(PlanError::NoOwner(agent.to_string())), } env.insert("BUZZ_ACP_AGENT_COMMAND".into(), h.command.to_string()); if !h.args.is_empty() { - env.insert("BUZZ_ACP_AGENT_ARGS".into(), h.args.join(" ")); + // COMMA, not space: buzz-acp parses this field with + // `value_delimiter = ','`. Joined with spaces, grok's + // `agent --always-approve stdio` arrives as a single argument and the + // harness refuses to start — latent until a multi-arg harness runs in + // relay mode. + env.insert("BUZZ_ACP_AGENT_ARGS".into(), h.args.join(",")); } let cfg = &spec.agent; @@ -287,10 +318,21 @@ pub fn container_plan( Ok(ContainerPlan { name: Names::container(agent), image: image.to_string(), - // The container runs buzz-acp, which spawns the harness named by - // BUZZ_ACP_AGENT_COMMAND. The image's ENTRYPOINT wraps this to create - // state directories first. - command: vec!["buzz-acp".into()], + // In relay mode the container runs buzz-acp, which spawns the harness + // named by BUZZ_ACP_AGENT_COMMAND. In environment mode buzz-acp lives + // outside — wherever the desktop is — so the container must not start + // one: it has no nsec and would crash-loop, and `docker exec` into a + // crash-looping container fails intermittently rather than cleanly. It + // idles instead, and hive-acp execs the harness in. + // + // Either way the image's ENTRYPOINT runs first and creates the state + // directories, which an environment container needs just as much. + command: match spec.agent.mode.unwrap_or_default() { + hive_spec::AgentMode::Relay => vec!["buzz-acp".into()], + hive_spec::AgentMode::Environment => { + vec!["sh".into(), "-c".into(), "exec sleep infinity".into()] + } + }, env, labels: labels_for(agent, &spec.hash(), h.id), network: Names::network(agent), @@ -341,6 +383,46 @@ id = "claude" AgentSpec::from_toml(&base).expect("valid spec") } + #[test] + fn a_relay_agent_runs_buzz_acp_and_an_environment_agent_does_not() { + // The two topologies need different containers. In relay mode buzz-acp + // lives inside and holds the identity. In environment mode it lives + // wherever the desktop is, and hive-acp execs the harness in from + // outside — so a container that started its own buzz-acp would have no + // nsec, crash-loop, and make `docker exec` fail intermittently rather + // than cleanly. + let relay = container_plan( + &spec_toml(""), + "alice", + "hive-agent:latest", + Default::default(), + &Default::default(), + None, + ) + .expect("relay plan"); + assert_eq!(relay.command, vec!["buzz-acp".to_string()]); + + let env_mode = container_plan( + &spec_toml("\n[agent]\nmode = \"environment\"\n"), + "alice", + "hive-agent:latest", + Default::default(), + &Default::default(), + None, + ) + .expect("environment plan"); + assert!( + !env_mode.command.iter().any(|c| c.contains("buzz-acp")), + "environment containers must not start buzz-acp: {:?}", + env_mode.command + ); + assert!( + env_mode.command.iter().any(|c| c.contains("sleep")), + "environment containers must stay up so hive-acp can exec in: {:?}", + env_mode.command + ); + } + #[test] fn observer_is_turned_on_explicitly_because_the_harness_defaults_it_off() { // BUZZ_ACP_RELAY_OBSERVER has default_value_t = false upstream. A @@ -381,7 +463,7 @@ id = "claude" // NIP-OA lets the agent derive relay access from its owner's membership // rather than needing its own enrollment. let mut s = spec_toml(""); - s.identity.auth_tag = Some("[\"auth\",\"owner\",\"cond\",\"sig\"]".into()); + s.identity.as_mut().unwrap().auth_tag = Some("[\"auth\",\"owner\",\"cond\",\"sig\"]".into()); let h = resolve_harness(&s).unwrap(); let env = environment(&s, h, "alice").unwrap(); assert!(env.contains_key("BUZZ_AUTH_TAG")); @@ -391,8 +473,8 @@ id = "claude" #[test] fn an_agent_with_no_owner_is_refused_rather_than_silently_idle() { let mut s = spec_toml(""); - s.identity.owner_pubkey = None; - s.identity.auth_tag = None; + s.identity.as_mut().unwrap().owner_pubkey = None; + s.identity.as_mut().unwrap().auth_tag = None; let h = harness::lookup("claude").unwrap(); assert!(matches!(environment(&s, h, "alice"), Err(PlanError::NoOwner(_)))); } @@ -422,8 +504,8 @@ id = "claude" // up stored twice, where one copy goes stale on rotation. let mut home = spec_toml(""); let mut other = spec_toml(""); - other.identity.relay_url = "wss://other.example".into(); - other.identity.credential = Some("nsec/uni".into()); + other.identity.as_mut().unwrap().relay_url = "wss://other.example".into(); + other.identity.as_mut().unwrap().credential = Some("nsec/uni".into()); // Different agent names, because they are different containers. let a = requirements(&home, "uni").unwrap(); @@ -436,13 +518,51 @@ id = "claude" assert_eq!(key_of(&b), "nsec/uni", "the second relay must reuse the same key"); // ...and they are still separate containers with separate state. - home.identity.relay_url = "wss://home.example".into(); + home.identity.as_mut().unwrap().relay_url = "wss://home.example".into(); assert_ne!( crate::backend::Names::volume("uni"), crate::backend::Names::volume("uni-other") ); } + #[test] + fn an_agent_can_name_its_own_harness_credential() { + // Two agents, two subscriptions. Without this every agent on the box + // shares one key, so a second subscription cannot be expressed and an + // agent running on someone else's tokens cannot say so — and swapping + // one means overwriting the credential every other agent is using. + let mut s = spec_toml(""); + s.harness.id = Some("claude".into()); + + let default = requirements(&s, "a").unwrap(); + assert!(default.iter().any(|r| r.key.as_str() == "harness/claude")); + + s.harness.credential = Some("harness/claude-second".into()); + let named = requirements(&s, "a").unwrap(); + assert!( + named.iter().any(|r| r.key.as_str() == "harness/claude-second"), + "the spec's key was ignored" + ); + assert!( + !named.iter().any(|r| r.key.as_str() == "harness/claude"), + "still demands the default key, so the agent needs both" + ); + } + + #[test] + fn a_named_credential_still_respects_non_broker_auth() { + // Naming a key must not resurrect the demand that `auth` just removed: + // an interactively logged-in harness holds its credential in the state + // volume, and hive never sees it whatever the key is called. + let mut s = spec_toml(""); + s.harness.id = Some("claude".into()); + s.harness.credential = Some("harness/claude-second".into()); + s.harness.auth = hive_spec::HarnessAuth::Interactive; + + let reqs = requirements(&s, "a").unwrap(); + assert!(!reqs.iter().any(|r| r.key.as_str().starts_with("harness/"))); + } + #[test] fn file_and_interactive_auth_do_not_demand_a_broker_key() { // The trap this closes: codex subscription auth is a JSON file with no diff --git a/crates/hive-core/src/harness.rs b/crates/hive-core/src/harness.rs index 5e36d77..3c62431 100644 --- a/crates/hive-core/src/harness.rs +++ b/crates/hive-core/src/harness.rs @@ -77,6 +77,36 @@ pub struct HarnessDef { /// in precedence order. These are what the broker mints; they are never /// written into a spec. pub credential_env: &'static [&'static str], + /// Where this harness reads a *file*-shaped credential, when it has one. + /// + /// Subscription auth is the default way people use these tools, and for + /// several of them it is a JSON blob on disk rather than a key in the + /// environment — codex writes `auth.json`, and an API key is the fallback, + /// not the norm. A harness whose credential can only be an env var leaves + /// this `None`. + /// + /// Absolute, and under the state volume: a credential written anywhere else + /// is destroyed on the next container recreate, after which the agent + /// silently reverts to unauthenticated. + pub credential_file: Option<&'static str>, + /// The harness OWNS that file and rewrites it — so a copy taken from one + /// machine cannot be injected into another. + /// + /// grok is the case this exists for. Its access token lasts hours, and its + /// refresh token rotates on use: whichever grok refreshes first invalidates + /// every other copy. Injecting a snapshot therefore fails in the least + /// helpful way available — grok reads the stale file, cannot refresh it, + /// and DELETES it, leaving a container that is unauthenticated while its + /// spec says a credential was delivered. + /// + /// Observed, not assumed: the file was present at 1733 bytes immediately + /// after create and gone once grok started, with the stored token 24 + /// minutes past `expires_at`. + /// + /// Such a harness is logged in INSIDE its own environment, once per + /// environment, and keeps its credential in the state volume where it + /// survives recreates. + pub credential_file_rotates: bool, pub model_syntax: ModelSyntax, /// Set when the harness is deliberately absent from the image. pub unsupported: Option, @@ -115,6 +145,8 @@ pub const CATALOG: &[HarnessDef] = &[ // switches a subscription agent to metered API billing — the image // refuses to set it at all, and spec validation bans it. credential_env: &["CLAUDE_CODE_OAUTH_TOKEN"], + credential_file: None, + credential_file_rotates: false, model_syntax: ModelSyntax::Bare, unsupported: None, note: "Subscription auth via CLAUDE_CODE_OAUTH_TOKEN from `claude setup-token`.", @@ -126,6 +158,8 @@ pub const CATALOG: &[HarnessDef] = &[ args: &[], requires: &["codex-acp", "codex"], credential_env: &["CODEX_API_KEY", "OPENAI_API_KEY"], + credential_file: Some("/home/agent/state/codex/auth.json"), + credential_file_rotates: false, model_syntax: ModelSyntax::Bracketed, unsupported: None, note: "Subscription auth also works by injecting ~/.codex/auth.json into CODEX_HOME \ @@ -139,6 +173,8 @@ pub const CATALOG: &[HarnessDef] = &[ args: &["acp"], requires: &["goose"], credential_env: &["OPENAI_API_KEY", "ANTHROPIC_API_KEY", "GOOSE_PROVIDER"], + credential_file: None, + credential_file_rotates: false, model_syntax: ModelSyntax::Passthrough, unsupported: None, note: "No subscription path. Can target any OpenAI-compatible endpoint, which makes it \ @@ -160,11 +196,15 @@ pub const CATALOG: &[HarnessDef] = &[ args: &["agent", "--always-approve", "stdio"], requires: &["grok"], credential_env: &["XAI_API_KEY"], + credential_file: Some("/home/agent/state/grok/auth.json"), + credential_file_rotates: true, model_syntax: ModelSyntax::Passthrough, unsupported: None, - note: "First-party xAI. Starts and speaks JSON-RPC with no credentials present; auth is \ - only needed for an actual turn. GROK_HOME must stay a shared read-only install \ - path — pointed at per-agent state, every agent re-bootstraps 127 MB on first run.", + note: "First-party xAI. GROK_HOME is both install dir and state dir, and grok WRITES to it \ + — sessions, settings, and auth.json, which it reads from there rather than from \ + ~/.grok. Read-only, session/new fails FS_PERMISSION_DENIED while separately \ + reporting no credentials. The image points it at per-agent state and symlinks the \ + shared 127 MB binary in.", }, HarnessDef { id: "opencode", @@ -173,6 +213,8 @@ pub const CATALOG: &[HarnessDef] = &[ args: &["acp"], requires: &["opencode"], credential_env: &["ANTHROPIC_API_KEY", "OPENAI_API_KEY"], + credential_file: None, + credential_file_rotates: false, model_syntax: ModelSyntax::Passthrough, unsupported: None, note: "Multi-provider; `opencode providers` manages auth interactively.", @@ -184,6 +226,8 @@ pub const CATALOG: &[HarnessDef] = &[ args: &["acp"], requires: &["kimi"], credential_env: &["MOONSHOT_API_KEY", "KIMI_API_KEY", "KIMI_MODEL_API_KEY"], + credential_file: None, + credential_file_rotates: false, model_syntax: ModelSyntax::Passthrough, unsupported: None, note: "First-party Moonshot, MIT. Smallest harness in the image at ~40 MB.", @@ -195,6 +239,8 @@ pub const CATALOG: &[HarnessDef] = &[ args: &[], requires: &["amp-acp", "amp"], credential_env: &["AMP_API_KEY"], + credential_file: None, + credential_file_rotates: false, model_syntax: ModelSyntax::Passthrough, unsupported: None, note: "amp-acp is a third-party adapter over @ampcode/cli (@sourcegraph/amp is \ @@ -209,6 +255,8 @@ pub const CATALOG: &[HarnessDef] = &[ args: &["acp"], requires: &["omp"], credential_env: &["XAI_API_KEY", "ANTHROPIC_API_KEY", "OPENAI_API_KEY"], + credential_file: None, + credential_file_rotates: false, model_syntax: ModelSyntax::Passthrough, unsupported: None, note: "Installed from the release binary, NOT npm: the npm package requires Bun, and \ @@ -221,6 +269,8 @@ pub const CATALOG: &[HarnessDef] = &[ args: &["acp"], requires: &["cursor-agent"], credential_env: &["CURSOR_API_KEY"], + credential_file: None, + credential_file_rotates: false, model_syntax: ModelSyntax::Passthrough, unsupported: None, note: "`acp` is a HIDDEN subcommand — absent from --help, but it resolves and is the \ @@ -237,6 +287,8 @@ pub const CATALOG: &[HarnessDef] = &[ args: &["acp", "--accept-hooks"], requires: &["hermes"], credential_env: &[], + credential_file: None, + credential_file_rotates: false, model_syntax: ModelSyntax::Passthrough, unsupported: Some(Unsupported::NotReproducible), note: "Installs non-interactively and works, but clones the default branch with no \ @@ -250,6 +302,8 @@ pub const CATALOG: &[HarnessDef] = &[ args: &["acp"], requires: &["openclaw"], credential_env: &[], + credential_file: None, + credential_file_rotates: false, model_syntax: ModelSyntax::Passthrough, unsupported: Some(Unsupported::NeedsExternalService), note: "`openclaw acp` is a bridge to a running OpenClaw Gateway, not a self-contained \ diff --git a/crates/hive-spec/src/lib.rs b/crates/hive-spec/src/lib.rs index d2f2a58..fa7bdd0 100644 --- a/crates/hive-spec/src/lib.rs +++ b/crates/hive-spec/src/lib.rs @@ -20,7 +20,18 @@ pub use validate::{ValidationError, ValidationReport}; /// is never written back here. #[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize)] pub struct AgentSpec { - pub identity: Identity, + /// Who the agent is on the relay. Required in `mode = "relay"`, where hive + /// runs buzz-acp itself; absent in `mode = "environment"`, where the + /// container is a place to run a harness and the identity belongs to + /// whatever spawned it. + /// + /// Optional because an environment genuinely has none — buzz-acp holds the + /// key on the host and Buzz strips it from the harness environment before + /// spawning, so a pubkey in the spec would be a value nothing reads. Making + /// it mandatory meant every generated spec carried four lines of fiction + /// that read as configuration. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub identity: Option, pub harness: Harness, #[serde(default)] pub agent: AgentConfig, @@ -165,6 +176,19 @@ pub struct Harness { /// container to log in. #[serde(default)] pub auth: HarnessAuth, + + /// Which broker key holds this harness's credential. Defaults to + /// `harness/`. + /// + /// One key per harness id means every agent on this box shares one + /// subscription, which is wrong in both directions: an agent running on + /// someone else's tokens has no way to say so, and a second subscription — + /// the obvious move when the first one runs out — cannot be expressed at + /// all. Naming the key here makes the credential a per-agent choice, and + /// swapping one is `hive secret put` against a different key rather than + /// overwriting the credential every other agent is using. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub credential: Option, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Default, serde::Serialize, serde::Deserialize)] @@ -183,8 +207,36 @@ pub enum HarnessAuth { Interactive, } +/// Whether an agent container connects to the relay itself, or is only a +/// sandbox something else drives. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum AgentMode { + /// The container runs `buzz-acp` and holds the relay connection. Default, + /// and the only mode that survives the desktop being closed. + #[default] + Relay, + /// The container is a sandbox only; `hive-acp` execs the harness in from + /// outside. Nothing in here talks to a relay. + Environment, +} + #[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize)] pub struct AgentConfig { + /// Whether this container holds its own relay connection. + /// + /// Default `relay`: the container runs `buzz-acp`, connects to the relay + /// itself, and outlives any desktop. That is the standalone agent. + /// + /// `environment` is for the other topology — hive occupying Buzz's + /// *harness* seam rather than its backend seam. There `buzz-acp` runs + /// wherever the desktop runs and holds the identity, and `hive-acp` execs + /// the harness into this container from outside. The container is then only + /// a sandbox and must NOT run `buzz-acp` itself: it has no nsec, so it + /// would crash-loop, and `docker exec` into a crash-looping container + /// fails intermittently rather than cleanly. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub mode: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub system_prompt: Option, /// Harness-specific model id. NOT portable between harnesses: Claude @@ -232,6 +284,10 @@ fn default_true() -> bool { impl Default for AgentConfig { fn default() -> Self { Self { + // None, not Some(Relay): an explicit value here would be written + // back out on every serialize, putting `mode = "relay"` into specs + // that never asked for it. + mode: None, system_prompt: None, model: None, respond_to: None, diff --git a/crates/hive-spec/src/validate.rs b/crates/hive-spec/src/validate.rs index 3e558d1..f343f28 100644 --- a/crates/hive-spec/src/validate.rs +++ b/crates/hive-spec/src/validate.rs @@ -75,56 +75,75 @@ pub fn validate(spec: &AgentSpec) -> ValidationReport { let mut r = ValidationReport::default(); // ---- identity ---- - if !is_hex64(&spec.identity.pubkey) { - r.errors.push(ValidationError::new( - "identity.pubkey", - "must be 64 lowercase hex characters", - )); - } - if !spec.identity.relay_url.starts_with("ws://") - && !spec.identity.relay_url.starts_with("wss://") - { - r.errors.push(ValidationError::new( - "identity.relay_url", - "must be a ws:// or wss:// URL", - )); - } - - // Without an owner the harness drops every event and sits idle — no error, - // no log line, just an agent that never answers. Catch it here rather than - // letting someone debug a silent agent. - if spec.identity.owner_pubkey.is_none() && spec.identity.auth_tag.is_none() { - r.errors.push(ValidationError::new( + // + // Required only where hive runs buzz-acp itself. An environment container + // has no identity by design — the process holding the key runs outside it — + // and demanding one there produced specs carrying a pubkey nothing reads. + let is_relay = spec.agent.mode.unwrap_or_default() == crate::AgentMode::Relay; + match (&spec.identity, is_relay) { + (None, true) => r.errors.push(ValidationError::new( "identity", - "one of owner_pubkey or auth_tag is required — without either, \ - the harness silently drops every event and the agent never responds", - )); - } - if let Some(owner) = &spec.identity.owner_pubkey - && !is_hex64(owner) - { - r.errors.push(ValidationError::new( - "identity.owner_pubkey", - "must be 64 lowercase hex characters", - )); - } - if spec.identity.owner_pubkey.is_some() && spec.identity.auth_tag.is_none() { - r.warnings.push( - "identity: using owner_pubkey without auth_tag — the agent needs its own \ - relay membership. A NIP-OA auth_tag would let it derive access from its \ - owner's membership instead, so revoking the human revokes the agent." + "required for mode = \"relay\": this container runs buzz-acp itself, \ + which cannot join a relay without a key, a url and an owner", + )), + (Some(_), false) => r.warnings.push( + "identity is set but mode = \"environment\": nothing reads it. buzz-acp \ + runs outside this container and holds the identity itself." .into(), - ); + ), + _ => {} } - if let Some(c) = &spec.identity.credential - && looks_like_a_secret(c) - { - r.errors.push(ValidationError::new( - "identity.credential", - "this is a broker KEY, not the private key itself — store the value with \ - `hive secret put` and name it here", - )); + if let Some(identity) = &spec.identity { + if !is_hex64(&identity.pubkey) { + r.errors.push(ValidationError::new( + "identity.pubkey", + "must be 64 lowercase hex characters", + )); + } + if !identity.relay_url.starts_with("ws://") && !identity.relay_url.starts_with("wss://") { + r.errors.push(ValidationError::new( + "identity.relay_url", + "must be a ws:// or wss:// URL", + )); + } + + // Without an owner the harness drops every event and sits idle — no + // error, no log line, just an agent that never answers. Catch it here + // rather than letting someone debug a silent agent. + if identity.owner_pubkey.is_none() && identity.auth_tag.is_none() { + r.errors.push(ValidationError::new( + "identity", + "one of owner_pubkey or auth_tag is required — without either, \ + the harness silently drops every event and the agent never responds", + )); + } + if let Some(owner) = &identity.owner_pubkey + && !is_hex64(owner) + { + r.errors.push(ValidationError::new( + "identity.owner_pubkey", + "must be 64 lowercase hex characters", + )); + } + if identity.owner_pubkey.is_some() && identity.auth_tag.is_none() { + r.warnings.push( + "identity: using owner_pubkey without auth_tag — the agent needs its own \ + relay membership. A NIP-OA auth_tag would let it derive access from its \ + owner's membership instead, so revoking the human revokes the agent." + .into(), + ); + } + + if let Some(c) = &identity.credential + && looks_like_a_secret(c) + { + r.errors.push(ValidationError::new( + "identity.credential", + "this is a broker KEY, not the private key itself — store the value with \ + `hive secret put` and name it here", + )); + } } // ---- harness ---- @@ -167,6 +186,25 @@ pub fn validate(spec: &AgentSpec) -> ValidationReport { ); } } + + // buzz-acp refuses to start unless idle_timeout < max_turn_duration: the + // wall-clock cap would otherwise fire first and make idle_timeout a dead + // letter. Checked here as well because the container is where that refusal + // happens — `hive validate` said the spec was fine, hived reconciled it, and + // the agent crash-looped with the reason visible only in `docker logs`. + // Found by deploying one. + if let (Some(idle), Some(max)) = (spec.agent.idle_timeout, spec.agent.max_turn_duration) + && idle >= max + { + r.errors.push(ValidationError::new( + "agent.idle_timeout", + format!( + "must be less than max_turn_duration ({max}s), got {idle}s — the harness \ + refuses to start, because the wall-clock cap would fire before the idle \ + timeout ever could" + ), + )); + } if let Some(p) = spec.agent.parallelism && p == 0 { @@ -347,23 +385,23 @@ fn parse_memory_gb(s: &str) -> Option { } #[cfg(test)] -mod tests { +pub(crate) mod tests { use super::*; use crate::*; use std::collections::BTreeMap; const PK: &str = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; - fn base() -> AgentSpec { + pub(crate) fn base() -> AgentSpec { AgentSpec { - identity: Identity { + identity: Some(Identity { pubkey: PK.into(), relay_url: "wss://buzz.example.org".into(), owner_pubkey: Some(PK.into()), auth_tag: None, credential: None, - }, - harness: Harness { id: Some("claude".into()), command: None, image: None, auth: HarnessAuth::Broker }, + }), + harness: Harness { id: Some("claude".into()), command: None, image: None, auth: HarnessAuth::Broker, credential: None }, agent: AgentConfig { observer: true, ..Default::default() }, resources: Resources::default(), network: Network::default(), @@ -385,8 +423,8 @@ mod tests { #[test] fn missing_owner_is_an_error_not_a_silent_idle_agent() { let mut s = base(); - s.identity.owner_pubkey = None; - s.identity.auth_tag = None; + s.identity.as_mut().unwrap().owner_pubkey = None; + s.identity.as_mut().unwrap().auth_tag = None; let r = s.validate(); assert!(!r.is_ok()); assert!(r.errors.iter().any(|e| e.to_string().contains("owner_pubkey or auth_tag"))); @@ -539,7 +577,7 @@ mod tests { #[test] fn explicit_command_requires_explicit_image() { let mut s = base(); - s.harness = Harness { id: None, command: Some("opencode acp".into()), image: None, auth: HarnessAuth::Broker }; + s.harness = Harness { id: None, command: Some("opencode acp".into()), image: None, auth: HarnessAuth::Broker, credential: None }; assert!(!s.validate().is_ok()); s.harness.image = Some("hive/harness-opencode:1.4.2".into()); @@ -602,3 +640,43 @@ mod tests { assert_eq!(s, back); } } + +#[cfg(test)] +mod timeout_tests { + use super::*; + + fn has_idle_error(s: &AgentSpec) -> bool { + s.validate().errors.iter().any(|e| { + let ValidationError::Invalid { field, .. } = e; + field == "agent.idle_timeout" + }) + } + + fn spec_with(idle: Option, max: Option) -> AgentSpec { + let mut s = super::tests::base(); + s.agent.idle_timeout = idle; + s.agent.max_turn_duration = max; + s + } + + #[test] + fn an_idle_timeout_at_or_above_the_turn_cap_is_refused() { + // The combination buzz-acp rejects at startup. Without this the spec + // validates, hived reconciles it, and the agent crash-loops with the + // reason only in `docker logs` — which is how it was found. + for (idle, max) in [(900, 600), (600, 600)] { + assert!(has_idle_error(&spec_with(Some(idle), Some(max))), "accepted idle={idle} max={max}"); + } + } + + #[test] + fn a_sane_pair_and_a_partial_one_are_both_fine() { + assert!(!has_idle_error(&spec_with(Some(300), Some(600)))); + // Only one set: the other takes a default this crate does not own, so + // there is nothing to compare against and guessing would reject valid + // specs. + for (idle, max) in [(Some(900), None), (None, Some(600)), (None, None)] { + assert!(!has_idle_error(&spec_with(idle, max)), "rejected {idle:?}/{max:?}"); + } + } +} diff --git a/crates/hived/src/main.rs b/crates/hived/src/main.rs index 8570a86..07da3ae 100644 --- a/crates/hived/src/main.rs +++ b/crates/hived/src/main.rs @@ -381,7 +381,11 @@ fn load_specs(dir: &Path) -> Result> { fn duplicate_identities(specs: &BTreeMap) -> HashMap { let mut seen: HashMap<(&str, &str), Vec<&str>> = HashMap::new(); for (name, spec) in specs { - seen.entry((&spec.identity.pubkey, &spec.identity.relay_url)) + // Environments have no identity and therefore cannot collide. Treating + // a missing one as a shared empty key would make every environment spec + // a duplicate of every other and hold all of them. + let Some(identity) = &spec.identity else { continue }; + seen.entry((identity.pubkey.as_str(), identity.relay_url.as_str())) .or_default() .push(name); } diff --git a/images/agent/Dockerfile b/images/agent/Dockerfile index dffbd29..8266868 100644 --- a/images/agent/Dockerfile +++ b/images/agent/Dockerfile @@ -58,6 +58,50 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ bash git ca-certificates curl bzip2 xz-utils libxcb1 libdbus-1-3 jq \ && rm -rf /var/lib/apt/lists/* +# --- what an agent is expected to already have --------------------------------- +# An agent in a container cannot `apt-get install` its way out of a missing tool: +# it has no root, and anything it did install would be destroyed on the next +# recreate. So the image has to anticipate, and the failure of not doing so is +# quiet — a harness that shells out to `rg` and silently falls back to something +# slower, a build that dies on a native npm module, `git log` with no pager. +# +# The list is "what a competent person would expect on a working machine", not +# "everything imaginable". Language toolchains beyond node and python are +# deliberately absent: they are large, and an agent that needs Go or Rust wants +# a specific version rather than whatever Debian ships. +RUN apt-get update && apt-get install -y --no-install-recommends \ + ripgrep fd-find \ + python3 python3-pip python3-venv \ + build-essential pkg-config \ + openssh-client rsync \ + less vim-tiny nano tree file \ + unzip zip \ + sqlite3 \ + ca-certificates \ + && ln -sf /usr/bin/fdfind /usr/local/bin/fd \ + && ln -sf /usr/bin/vim.tiny /usr/local/bin/vim \ + && rm -rf /var/lib/apt/lists/* \ + # Assert rather than hope: a base image that stops shipping one of these + # should fail the build, not produce agents that quietly cannot search. + && for t in rg fd python3 pip3 make gcc ssh rsync less tree unzip sqlite3; do \ + command -v "$t" >/dev/null || { echo "missing after install: $t" >&2; exit 1; }; \ + done + +# gh, because reading issues and opening pull requests is ordinary agent work +# and the alternative is hand-rolled curl against the REST API. From GitHub's +# own release rather than apt: Debian does not package it, and the tarball is a +# single static binary. +ARG GH_VERSION=2.63.2 +RUN set -eux; \ + arch="$(dpkg --print-architecture)"; \ + case "$arch" in amd64) gharch=amd64 ;; arm64) gharch=arm64 ;; *) echo "unknown arch $arch" >&2; exit 1 ;; esac; \ + curl -fsSL "https://github.com/cli/cli/releases/download/v${GH_VERSION}/gh_${GH_VERSION}_linux_${gharch}.tar.gz" \ + -o /tmp/gh.tgz; \ + tar -xzf /tmp/gh.tgz -C /tmp; \ + mv "/tmp/gh_${GH_VERSION}_linux_${gharch}/bin/gh" /usr/local/bin/gh; \ + rm -rf /tmp/gh.tgz "/tmp/gh_${GH_VERSION}_linux_${gharch}"; \ + gh --version | head -1 + # --- THE npm LIFECYCLE-SCRIPT GATE ------------------------------------------ # node:24-bookworm-slim ships npm 11.16.0, which refuses to run dependency # install scripts by default AND STILL EXITS 0 (`strict-allow-scripts` is @@ -115,16 +159,35 @@ RUN set -eux; \ chmod 0755 /usr/local/bin/goose # --- grok (Grok Build) — first-party xAI ------------------------------------ -# GROK_HOME is the INSTALL location, not per-agent state. It must stay a shared -# read-only path: point it into the per-agent volume and every agent -# re-bootstraps its own 127 MB copy of the binary on first run. +# GROK_HOME is BOTH the install location and the state directory, and grok needs +# to WRITE to it: session storage, auth.json, settings. An earlier version of +# this file pinned it read-only at /opt/grok to stop every agent re-bootstrapping +# its own 127 MB copy of the binary — which worked, and made grok unusable: +# +# session search bootstrap failed: unable to open database file: +# /opt/grok/sessions/session_search.sqlite +# No credentials found: no login token and no model api_key/env_key +# → session/new fails FS_PERMISSION_DENIED (os error 13) +# +# The credential half is the nastier one: with GROK_HOME set, grok reads +# auth.json from there and NOT from ~/.grok, so a perfectly good login is +# invisible and the error names a filesystem problem. +# +# So the install stays shared at /opt/grok and GROK_HOME points into the +# per-agent volume, with the binary symlinked in by the entrypoint. Writable +# state per agent, one 127 MB copy on disk. ARG GROK_VERSION=0.2.112 +# Set for the INSTALL only — the postinstall bootstraps the 127 MB binary into +# whatever GROK_HOME points at, and unset it lands in root's home where the +# agent user cannot reach it. Overridden to the per-agent path further down; +# ENV is sequential, so the last one wins at runtime. ENV GROK_HOME=/opt/grok RUN npm install -g --no-fund --no-audit \ --allow-scripts=@xai-official/grok \ "@xai-official/grok@${GROK_VERSION}" \ && npm cache clean --force \ - && chmod -R a+rX /opt/grok 2>/dev/null || true + && chmod -R a+rX /opt/grok 2>/dev/null || true \ + && test -x /opt/grok/bin/grok # --- opencode --------------------------------------------------------------- ARG OPENCODE_VERSION=1.18.8 @@ -279,7 +342,9 @@ ENV XDG_DATA_HOME=/home/agent/state/data # entrypoint creates the targets, because on a FIRST run the volume is empty and # a dangling symlink makes the harness's own `mkdir -p` fail with EEXIST. ENV AMP_HOME=/home/agent/state/amp -# GROK_HOME is deliberately NOT redirected here — see the install step. +# GROK_HOME is per-agent: grok writes sessions there AND reads auth.json from +# it. The entrypoint symlinks the shared 127 MB binary in. +ENV GROK_HOME=/home/agent/state/grok # Create the state root AS THE AGENT USER, before VOLUME. # @@ -289,7 +354,13 @@ ENV AMP_HOME=/home/agent/state/amp # thing every harness does is fail: goose panics on "Failed to create session # database directory", opencode dies with EACCES on mkdir. Creating the # directory here first means the volume inherits agent ownership instead. -RUN mkdir -p /home/agent/state /home/agent/work \ +# /home/agent/work is deliberately NOT created here. As a real directory in the +# image it lands on the container's ephemeral layer, and since hive-acp points +# the client's cwd at it, every recreate would silently delete the agent's work +# while its sessions and credentials survived on the volume. The entrypoint +# links it into the state volume instead — and `link_state` will not replace a +# real directory, so creating one here would defeat it. +RUN mkdir -p /home/agent/state \ && test -w /home/agent/state VOLUME ["/home/agent/state"] diff --git a/images/agent/entrypoint.sh b/images/agent/entrypoint.sh index 86d1862..d308a45 100755 --- a/images/agent/entrypoint.sh +++ b/images/agent/entrypoint.sh @@ -31,12 +31,41 @@ link_state() { ln -sfn "$target" "$HOME/$name" fi } -link_state .grok grok # grok login -> ~/.grok/auth.json (GROK_HOME is the install dir, not this) +# The agent's working directory. NOT decoration: hive-acp redirects the +# client's cwd here, so this is where an agent actually writes. Left as a plain +# directory in the image it sits on the container's ephemeral layer, and every +# recreate — which a spec edit triggers — silently deletes the agent's work +# while its sessions and credentials survive on the volume. +link_state work work + +link_state .grok grok link_state .kimi kimi link_state .cursor cursor link_state .opencode opencode link_state .omp omp +# grok wants ONE directory for both its install and its state: it writes +# sessions and settings under $GROK_HOME, and reads auth.json from there rather +# than from ~/.grok. Pointing it at the read-only install made session/new fail +# `FS_PERMISSION_DENIED` while reporting no credentials — a filesystem error +# standing in for two separate problems. +# +# So $GROK_HOME is per-agent and writable, with the 127 MB binary symlinked from +# the shared install instead of copied. `grok` on PATH resolves through here, so +# the version stays whatever the image installed. +GROK_INSTALL=/opt/grok +if [ -d "$GROK_INSTALL/bin" ]; then + mkdir -p "$STATE/grok/bin" + for f in "$GROK_INSTALL"/bin/*; do + [ -e "$f" ] || continue + ln -sfn "$f" "$STATE/grok/bin/$(basename "$f")" + done + # config.toml records how grok was installed; copied, not linked, because + # grok rewrites it and the install is read-only. + [ -f "$GROK_INSTALL/config.toml" ] && [ ! -f "$STATE/grok/config.toml" ] \ + && cp "$GROK_INSTALL/config.toml" "$STATE/grok/config.toml" +fi + # A harness that is selected but absent fails inside buzz-acp as "agent failed # to spawn: No such file or directory", which surfaces on the desktop as "all N # agents failed to start" — true, and useless. Say the actual thing instead. diff --git a/images/agent/smoke.sh b/images/agent/smoke.sh index 131a74a..5f8e30d 100644 --- a/images/agent/smoke.sh +++ b/images/agent/smoke.sh @@ -42,6 +42,41 @@ probe() { fi } +# --- persistence ------------------------------------------------------------ +# Every path an agent writes to must resolve ONTO the state volume. A path left +# on the container's ephemeral layer looks perfect until the first recreate — +# which a spec edit triggers — and then the data is gone with no error anywhere. +# +# This exists because two of them were wrong at once: $GROK_HOME pointed at the +# read-only install, and /home/agent/work was a plain directory in the image, so +# an agent's working files were deleted on every recreate while its sessions and +# credentials survived. Both read fine in the Dockerfile. +STATE="${HIVE_STATE_DIR:-/home/agent/state}" + +persists() { + name="$1"; path="$2" + real=$(readlink -f "$path" 2>/dev/null) + case "$real" in + "$STATE"|"$STATE"/*) printf ' %-10s PASS %s\n' "$name" "$real" ;; + "") printf ' %-10s FAIL %s does not exist\n' "$name" "$path"; fail=1 ;; + *) printf ' %-10s FAIL %s -> %s is NOT on the state volume\n' \ + "$name" "$path" "$real"; fail=1 ;; + esac +} + +echo "persistence:" +persists work "$HOME/work" +persists grok "${GROK_HOME:-$HOME/.grok}" +persists claude "${CLAUDE_CONFIG_DIR:-$HOME/.claude}" +persists codex "${CODEX_HOME:-$HOME/.codex}" +persists kimi "$HOME/.kimi" +persists cursor "$HOME/.cursor" +persists opencode "$HOME/.opencode" +persists omp "$HOME/.omp" +persists amp "${AMP_HOME:-$HOME/.amp}" +echo +echo "acp:" + # The probe table. Kept in sync with hive-core's harness catalog by a test in # crates/hive-core/src/lib.rs — edit the catalog, not just this list. # HARNESS_TABLE_BEGIN diff --git a/images/hived/Dockerfile b/images/hived/Dockerfile index 9f64a22..4c0d8d8 100644 --- a/images/hived/Dockerfile +++ b/images/hived/Dockerfile @@ -67,6 +67,19 @@ RUN cargo build --release --locked -p hived --bin hived -p hive-cli --bin hive \ FROM debian:bookworm-slim +# curl, because `hive mcp login` walks an OAuth flow with it, and ca-certificates +# because every step of that flow is https. +# +# This image had neither, so the whole subcommand failed at its first request +# with `No such file or directory (os error 2)` — a message that names a file +# and does not name curl. The feature was written and tested against a host +# build, where curl is simply always there, and had never once run where it +# actually ships. +RUN apt-get update \ + && apt-get install -y --no-install-recommends curl ca-certificates \ + && rm -rf /var/lib/apt/lists/* \ + && curl --version | head -1 + # hive drives Docker through the CLI rather than the API, so the CLI is a hard # runtime dependency. Copied from the official image rather than installed from # apt: it is a mostly-static Go binary, and this pins the version instead of diff --git a/packaging/hive-wrapper.sh b/packaging/hive-wrapper.sh index fca718a..7ab1d5b 100755 --- a/packaging/hive-wrapper.sh +++ b/packaging/hive-wrapper.sh @@ -41,8 +41,37 @@ if [ "${1:-}" = "secret" ] && [ "${2:-}" = "put" ]; then needs_stdin=true fi +# Locate docker without trusting PATH. +# +# A non-interactive `ssh host 'hive ...'` — which is exactly how +# buzz-backend-hive deploys — gets PATH=/usr/bin:/bin:/usr/sbin:/sbin, with +# neither Homebrew nor /usr/local/bin on it. Bare `docker` then fails with +# "exec: docker: not found" from inside this wrapper, which reads as hive being +# broken rather than as a login-shell difference. Same list as hive-acp's +# find_docker and hive_core::docker::DockerBackend::discover. +DOCKER="${HIVE_DOCKER:-}" +if [ -z "$DOCKER" ]; then + for candidate in \ + /usr/local/bin/docker \ + /opt/homebrew/bin/docker \ + /usr/bin/docker \ + /Applications/Docker.app/Contents/Resources/bin/docker + do + if [ -x "$candidate" ]; then DOCKER="$candidate"; break; fi + done +fi +if [ -z "$DOCKER" ]; then + DOCKER=$(command -v docker 2>/dev/null) || true +fi +if [ -z "$DOCKER" ]; then + echo "hive: cannot find the docker CLI." >&2 + echo "hive: looked in /usr/local/bin, /opt/homebrew/bin, /usr/bin and Docker.app." >&2 + echo "hive: set HIVE_DOCKER=/path/to/docker if it lives somewhere else." >&2 + exit 127 +fi + if [ -t 0 ] && [ -t 1 ] && [ "$needs_stdin" = false ]; then - exec docker exec -i -t "$CONTAINER" hive "$@" + exec "$DOCKER" exec -i -t "$CONTAINER" hive "$@" else - exec docker exec -i "$CONTAINER" hive "$@" + exec "$DOCKER" exec -i "$CONTAINER" hive "$@" fi