diff --git a/hive-subagent-mcp/src/session.rs b/hive-subagent-mcp/src/session.rs index 071a3064..37d48b90 100644 --- a/hive-subagent-mcp/src/session.rs +++ b/hive-subagent-mcp/src/session.rs @@ -814,52 +814,6 @@ fn validate_name(name: &str) -> anyhow::Result<()> { .map_err(|e| anyhow::anyhow!("invalid subagent name {name:?}: {e}")) } -/// Env var carrying the nix-configured `hyperhive.availableModels` as a -/// comma-separated list, forwarded onto this daemon's unit by -/// `nix/agent-modules/mcp.nix` the same way `HIVE_TOOL_GROUPS` is. -const AVAILABLE_MODELS_ENV: &str = "HIVE_AVAILABLE_MODELS"; - -/// The models this daemon may spawn a subagent on, from -/// [`AVAILABLE_MODELS_ENV`]. `None` when the var is absent. -fn available_models() -> Option { - std::env::var(AVAILABLE_MODELS_ENV).ok() -} - -/// Refuse a `model` the operator hasn't made available to this agent. -/// -/// Default-open on purpose: this is a safety rail, not a security boundary, -/// so an absent `allowed` (or one naming nothing) restricts nothing and an -/// agent deployed before this gate existed keeps working unchanged. A `None` -/// model isn't a choice of model either — it lets claude fall back to its own -/// default — so there is nothing to check. -/// -/// # Errors -/// -/// A named model absent from `allowed`; the message names it and every -/// permitted one, since it is all the caller sees. -fn check_model(model: Option<&str>, allowed: Option<&str>) -> anyhow::Result<()> { - let (Some(model), Some(allowed)) = (model, allowed) else { - return Ok(()); - }; - // Same split/trim/skip-empty rules as `ToolGroup::parse_list`. - let allowed: Vec<&str> = allowed - .split(',') - .map(str::trim) - .filter(|token| !token.is_empty()) - .collect(); - if allowed.is_empty() - || allowed - .iter() - .any(|entry| entry.eq_ignore_ascii_case(model.trim())) - { - return Ok(()); - } - anyhow::bail!( - "model `{model}` is not available to this agent — {AVAILABLE_MODELS_ENV} permits: {}", - allowed.join(", ") - ); -} - /// Extend the ambient `OTEL_RESOURCE_ATTRIBUTES` with a `subagent=` /// attribute, so every token/cost/tool-call data point this subagent's own /// claude process emits carries it alongside the parent's `agent=` @@ -995,8 +949,7 @@ pub struct StartRequest { /// /// # Errors /// -/// A name already running, an invalid name, a `model` the operator hasn't -/// made available (see `check_model`), an archive failure, or the +/// A name already running, an invalid name, an archive failure, or the /// underlying `Claude::spawn` failing (binary missing, etc.) — the last /// case is the only one that can happen *after* commit-to-run, and it's /// exactly why nothing is registered in `running` until spawn actually @@ -1004,7 +957,6 @@ pub struct StartRequest { pub fn start(state: &Arc, req: StartRequest) -> anyhow::Result { let name = req.name.as_str(); validate_name(name)?; - check_model(req.model.as_deref(), available_models().as_deref())?; if !state.reserve(name) { anyhow::bail!("subagent `{name}` is already running — use `continue` or `interrupt`"); } @@ -1086,7 +1038,8 @@ fn start_reserved( /// Give an existing named session a new turn — resuming it whether that /// means "the previous turn finished, here's the next instruction" or "the -/// daemon restarted, reattaching." Refuses a name already running. +/// daemon restarted, reattaching." Refuses a name already running (same +/// concurrent-run hazard as `start`). /// /// A name with **no** session to resume is still not *pre-checked* here: /// claude's own `--resume` answers that, and the pre-check that used to live @@ -1111,9 +1064,8 @@ fn start_reserved( /// /// # Errors /// -/// An invalid name, one already running, a `model` the operator hasn't made -/// available (see `check_model`), `Claude::spawn` failing, or the resumed -/// turn failing within `RESUME_GRACE` — in practice a missed resume. +/// An invalid name, one already running, `Claude::spawn` failing, or the +/// resumed turn failing within `RESUME_GRACE` — in practice a missed resume. pub async fn continue_( state: &Arc, name: &str, @@ -1123,7 +1075,6 @@ pub async fn continue_( dir: Option<&str>, ) -> anyhow::Result { validate_name(name)?; - check_model(model.as_deref(), available_models().as_deref())?; if !state.reserve(name) { anyhow::bail!( "subagent `{name}` is already running — use `interrupt` first if you meant to redirect it" @@ -3374,50 +3325,4 @@ mod tests { "the safety property is unchanged: no ambient MCP discovery either way" ); } - - #[test] - fn an_unset_model_whitelist_allows_any_model() { - assert!(check_model(Some("opus"), None).is_ok()); - } - - #[test] - fn an_unset_model_whitelist_allows_an_omitted_model() { - assert!(check_model(None, None).is_ok()); - } - - #[test] - fn a_listed_model_is_allowed() { - assert!(check_model(Some("sonnet"), Some("haiku,sonnet,opus")).is_ok()); - } - - #[test] - fn an_unlisted_model_is_rejected_naming_what_is_permitted() { - let err = check_model(Some("opus"), Some("haiku,sonnet")) - .expect_err("a model outside the list must be refused") - .to_string(); - assert!( - err.contains("opus") && err.contains("haiku, sonnet"), - "the rejection is all the caller sees, so it must name the model \ - and the permitted ones: {err}" - ); - } - - #[test] - fn a_set_whitelist_still_allows_an_omitted_model() { - assert!( - check_model(None, Some("haiku")).is_ok(), - "omitting `model` lets claude pick its own default — it is not a \ - choice of model, so there is nothing to gate" - ); - } - - #[test] - fn whitespace_and_empty_entries_are_ignored() { - assert!(check_model(Some("sonnet"), Some(" haiku , sonnet ,, ")).is_ok()); - assert!(check_model(Some("opus"), Some(" haiku , sonnet ,, ")).is_err()); - assert!( - check_model(Some("opus"), Some(" ,, ")).is_ok(), - "a var naming nothing restricts nothing — default open" - ); - } } diff --git a/nix/agent-modules/mcp.nix b/nix/agent-modules/mcp.nix index bbda68b2..3c96ab36 100644 --- a/nix/agent-modules/mcp.nix +++ b/nix/agent-modules/mcp.nix @@ -388,20 +388,6 @@ in # `null` when the agent has no groups declared, which systemd drops # — the same "absent" the harness itself would see. HIVE_TOOL_GROUPS = config.systemd.services.hive-agent.environment.HIVE_TOOL_GROUPS or null; - # Same `hyperhive.availableModels` the harness's own assertions gate - # the primary session's model against, so a subagent can't be spawned - # on a model the operator didn't make available to this agent. The - # option renders into the *global* environment for the web UI's - # quick-picker, which a systemd unit doesn't inherit, so set it here - # from the option directly. `null` for an empty list, which systemd - # drops: the daemon reads an absent var as "no restriction" (this is - # a safety rail, not a security boundary), matching the harness - # assertion that an empty list waives too. - HIVE_AVAILABLE_MODELS = - if config.hyperhive.availableModels == [ ] then - null - else - lib.concatStringsSep "," config.hyperhive.availableModels; # HYPERHIVE_HARNESS_DIR / HYPERHIVE_STATE_DIR: see # `hive-bash-daemon`'s own comment above — same global injection, # same reasoning.