diff --git a/CLAUDE.md b/CLAUDE.md index 80e867cf..c900c2df 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -101,12 +101,15 @@ hand-maintained per-file tree drifts out of sync with the code. - **`hive-subagent-mcp/`** — per-agent claude-subagent runner daemon (`hive-subagent-daemon`); spawns nested claude sessions on request and serves `start`/`continue`/`status`/`interrupt` directly over - streamable-http (no stdio bridge). Independent of `hive-bash-mcp` (a - subagent is a much heavier capability than a bash command). No task - files — the daemon's only state is an in-memory map of currently-running - processes, live only as long as the process is; the actual claude - session survives a daemon restart independently (see `session.rs`'s - module doc). + streamable-http (no stdio bridge), plus a second subagent-facing route + carrying `goal_reached`/`need_help` — served per session under a minted + token, since neither tool takes a session name. Independent of `hive-bash-mcp` (a + subagent is a much heavier capability than a bash command). A `start` + with a `goal` is a multi-turn run: the daemon re-prompts the subagent + toward the goal each time a turn ends, up to a per-session turn cap. No + task files — the daemon's state is in-memory only, live only as long as + the process is; the actual claude session survives a daemon restart + independently (see `session.rs`'s module doc). - **`hive-sh4re/`** — shared wire types (Agent / Manager request + response, `Message`, `Approval`, `HelperEvent`) used across the unix sockets. Host-admin-socket and hive-priv-socket wire types have been diff --git a/Cargo.lock b/Cargo.lock index c3342892..29c3f839 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2004,6 +2004,7 @@ dependencies = [ "tokio", "tracing", "tracing-subscriber", + "uuid", ] [[package]] diff --git a/docs/tools/subagent.md b/docs/tools/subagent.md index 1cd7184e..f797904c 100644 --- a/docs/tools/subagent.md +++ b/docs/tools/subagent.md @@ -24,11 +24,42 @@ infrastructure, not the agent-facing API. Served under the `subagent` MCP server (`mcp__subagent__`): `start`, `continue`, `status`, `interrupt`. +A second route on the same port serves the two tools a **subagent** calls +about its own run — `goal_reached` and `need_help`. It isn't part of the +`subagent` server an agent's own config points at; the daemon writes it +into each subagent's `--mcp-config` itself, under `subagent_control`. Two +routes rather than six tools on one, so that being able to report on a run +never carries the ability to start one: there's no route a subagent holds +that `start` is reachable from. + +## A subagent can't name a session, not even its own + +Neither signal tool takes a session name. **The URL is the identity.** At +each spawn the daemon mints that run an unguessable token, serves it at +`/signal/mcp/`, and writes that one URL into that one subagent's own +`--mcp-config` — a file per session, not a shared one. A request is +resolved to a session before it's dispatched, and the tools read the +session off the resolution. + +A subagent therefore has no field in which to name a sibling, and knowing +a sibling's name buys nothing: a name isn't a token. Two subagents running +concurrently can't signal each other — which the earlier shape, a shared +route plus a `name` argument guarded by a liveness check, allowed. + +A token that resolves to nothing — never minted, or revoked when its run +ended — gets a bare **404**, the same answer for every token, so nothing +about the refusal says whether some other session exists. + ## State In-memory only: what's running now, where each name's session lives, how -each name's last turn ended, and when each running turn last produced -output. All of it lives only as long as the daemon process does. A daemon +each name's last turn ended, when each running turn last produced output, +what each session is working toward, how far through its turn budget it +is, why its run stopped, where it writes its report, and which signal +token belongs to it. All of it lives only as long as the daemon process +does — so a daemon restart invalidates every signal URL it had issued, +which is the same thing as it having stopped the runs those URLs belonged +to. A daemon restart stops whatever was running rather than adopting it. The durable record of a subagent's existence is claude's own on-disk session (`hive_claude::SessionStore`), which `continue` reattaches to independent @@ -40,14 +71,68 @@ after a restart has to re-supply `dir` when the session lives anywhere other than the daemon's own working directory — and a restart is the situation you reach for `continue` in most often. +## Goals, and turns toward them + +`start` takes an optional `goal`. Without one a session is a single turn, +exactly as it always was. With one, the daemon keeps the session going: +when a turn ends and nothing has said to stop, it starts another turn +re-prompting the subagent toward that goal, quoting it verbatim and saying +which turn of the budget this is. `max_turns` caps that, defaulting to +**5**. + +`status` reports `Turn N of M` for such a session in every state it +reaches. Read alongside the last-event age below, it's what separates a +subagent that's working from one that's wedged from one that's out of +turns — without `ps` and without opening a file. + +Four things stop a run, and each is recorded distinctly, reported by +`status`, and appended to the one todo the daemon pushes when the run ends: + +- **the turn ended and there was no goal** — the single-turn case; +- **`goal_reached`**, which the subagent calls itself; +- **`need_help`**, likewise; +- **the turn cap**, which says so rather than stopping quietly: the todo + states that the harness limit was reached and the goal was never + reported reached, so the work stopped where it had got to. + +A killed or failed turn ends the run too, and keeps the records it already +had — see [A killed turn](#a-killed-turn). `interrupt` therefore stops a +whole goal run, not just the turn in flight. + +When the session was told where its report goes — `start`'s `report_file`, +or the path the subagent names when it signals — the stop reason is +appended to that file as well, so the artifact you were going to read +anyway also says how the run ended. Nothing is inferred: with no path +given, no file is touched. + +## `goal_reached` is a label, not a gate + +Both signals stop the continuation and **extend** the done message. Extend, +not replace: the turn's observed end and the reason the run stopped are +different facts, and the second never stands in for the first. + +`goal_reached` is **self-reported**, by a subagent that has just been +re-prompted with "you haven't reached the goal" — which is precisely the +incentive to claim it. It's the same failure class as a build report +asserting "done, tests pass": a claim about an artifact, not the artifact. +Nothing in this daemon treats it as verification, and every surface that +renders it says so. Read the diff and the gate output regardless. + +`need_help` is the blocking signal. It stops the run and shows up in +`status` as its own state — blocked, with the subagent's reason — so a +parent that polls `status` sees the block without reading anything else. +`continue` is how you answer it. + ## Is it working, or is it wedged? `status` reporting **running** says a process is tracked, which a wedged subagent satisfies as fully as a busy one. A running answer therefore carries the age of that turn's last event too: seconds means it's working, -an age climbing into the minutes with no end-of-turn todo means it's -stuck. That one number replaces inferring the same thing from `ps` output -and CPU-time deltas. +an age climbing into the minutes means it's stuck. That one number +replaces inferring the same thing from `ps` output and CPU-time deltas. +It resets at each turn's spawn, so on a goal run it describes the turn in +flight rather than the run — which is what you want, since a run that's +making progress spends several perfectly healthy minutes. Every line the subagent's `claude` process writes bumps the timestamp — stream-json events, plain stdout chatter and stderr alike — and what the @@ -135,10 +220,11 @@ Own systemd unit, defined alongside the other per-agent MCP daemons in ## MCP servers available to a subagent -A subagent runs with `--strict-mcp-config` and no `--mcp-config` by -default — zero MCP servers, full stop; it falls back to claude's own -native tools (`Bash`, `WebFetch`, etc.), not the parent's `mcp__bash__*` / -`mcp__hyperhive__*` surface. Nothing implicit reaches it: the built-in +A subagent runs with `--strict-mcp-config` and, by default, exactly one +MCP server: the two-tool `subagent_control` route above. It otherwise +falls back to claude's own native tools (`Bash`, `WebFetch`, etc.), not +the parent's `mcp__bash__*` / `mcp__hyperhive__*` surface. Nothing +implicit reaches it: the built-in hyperhive surface (todos/messaging) isn't an `extraMcpServers` entry at all, and the automatically injected `bash`/`subagent` entries default to excluded too (a subagent can't spawn hive-bash tasks or its own nested subagents @@ -148,5 +234,8 @@ Set `hyperhive.extraMcpServers..availableToSubagents = true` on a specific entry to hand that one server to subagents as well — useful for, say, a read-only lookup or scraper MCP a subagent's bounded, single-batch task might need. `hive-subagent-mcp`'s `mcp_config` module renders the -opted-in subset into its own `--mcp-config` file per turn; an entry left -at the default `false` never appears there. +opted-in subset into a `--mcp-config` file per session, rewritten each +turn; an entry left at the default `false` never appears there. Per +session rather than one shared file, because the `subagent_control` entry +in it carries that session's own signal URL — one file for everyone would +be a race over whose identity each subagent reads at startup. diff --git a/hive-subagent-mcp/Cargo.toml b/hive-subagent-mcp/Cargo.toml index 025735ea..f7223e86 100644 --- a/hive-subagent-mcp/Cargo.toml +++ b/hive-subagent-mcp/Cargo.toml @@ -23,6 +23,10 @@ serde_json.workspace = true tokio.workspace = true tracing.workspace = true tracing-subscriber.workspace = true +# Signal-route tokens. `Uuid::new_v4` draws from the OS CSPRNG (getrandom), +# which is the property the per-session URL rests on — see +# `session::State::mint_signal_url`. +uuid.workspace = true # `test-util` for `#[tokio::test(start_paused = true)]`: the `continue` # resume-grace tests assert what happens when the bound is actually reached, @@ -33,7 +37,9 @@ tokio = { workspace = true, features = ["test-util"] } # `hive-subagent-daemon` — long-running per-agent claude-subagent runner. # Independent of `hive-bash-mcp` (own crate, own binary, own MCP server) — # see lib.rs's module doc for why. Serves its MCP tools (`start`/ -# `continue`/`interrupt`) directly over streamable-http — no stdio bridge. +# `continue`/`status`/`interrupt`, plus the subagent-facing +# `goal_reached`/`need_help` route) directly over streamable-http — no +# stdio bridge. [[bin]] name = "hive-subagent-daemon" path = "src/main.rs" diff --git a/hive-subagent-mcp/README.md b/hive-subagent-mcp/README.md index 27ae605b..5e5d8f51 100644 --- a/hive-subagent-mcp/README.md +++ b/hive-subagent-mcp/README.md @@ -2,9 +2,10 @@ Per-agent daemon (`hive-subagent-daemon`) that spawns nested headless `claude` sessions on request and serves the tool surface -(`start`/`continue`/`interrupt`) directly over streamable-http. No -stdio bridge, no per-turn respawn — claude reconnects to the same -stable URL every turn. +(`start`/`continue`/`status`/`interrupt`, plus a separate +subagent-facing `goal_reached`/`need_help` route, one per-session URL) +directly over streamable-http. No stdio bridge, no per-turn respawn — an +agent's claude reconnects to the same stable URL every turn. Independent of `hive-bash-mcp` — a subagent spawns a full nested `claude` session, a much heavier capability than a bash command, worth @@ -18,10 +19,17 @@ own lib (`src/lib.rs`): - **`session.rs`** — the actual claude-facing logic: `Claude::spawn` + `RunningClaude::wait`/`cancel_handle` (not `InfiniteSession::run`, which has no cancel handle to reach in — see the module doc for the - v1 scope this trades away), the in-memory `running` map that's the - _only_ state this daemon keeps (no task files — a restart stops - whatever's running; the actual claude session is the durable store, - found again by name via `hive_claude::SessionStore`). -- **`mcp.rs`** — the `rmcp` tool router (`start`/`continue`/`interrupt`) - - `serve_http`. + v1 scope this trades away), the turn-continuation loop a `goal` + switches on, and the in-memory maps that are the _only_ state this + daemon keeps (no task files — a restart stops whatever's running; + the actual claude session is the durable store, found again by name + via `hive_claude::SessionStore`). +- **`mcp.rs`** — the `rmcp` tool routers (the parent's + `start`/`continue`/`status`/`interrupt` on `/mcp`, the subagent's + `goal_reached`/`need_help` on `/signal/mcp/`) + `serve_http`. + Neither signal tool takes a session name: the token in the path is + minted per run and resolved to a session before dispatch, so a subagent + has no way to name — and therefore no way to signal — a sibling. One + route with a path parameter, because the `Router` is built once at + startup and sessions come and go for the daemon's whole life. - **`paths.rs`** — the in-agent todo-socket path. diff --git a/hive-subagent-mcp/src/lib.rs b/hive-subagent-mcp/src/lib.rs index 666088be..365eb8cc 100644 --- a/hive-subagent-mcp/src/lib.rs +++ b/hive-subagent-mcp/src/lib.rs @@ -1,13 +1,20 @@ //! Library for `hive-subagent-daemon`: spawns nested claude sessions on -//! request and serves the `start`/`continue`/`interrupt` MCP tool surface -//! directly over streamable-http — no stdio bridge, no round-trip socket. -//! Independent of `hive-bash-mcp` — a subagent is a much heavier capability -//! than a bash command (a full nested `claude` process), worth its own -//! deployable/restartable unit rather than sharing one. +//! request and serves the `start`/`continue`/`status`/`interrupt` MCP tool +//! surface directly over streamable-http — no stdio bridge, no round-trip +//! socket. Independent of `hive-bash-mcp` — a subagent is a much heavier +//! capability than a bash command (a full nested `claude` process), worth +//! its own deployable/restartable unit rather than sharing one. +//! +//! A second route on the same listener serves `goal_reached`/`need_help` to +//! the *subagents*, which is how a run says it's done or stuck; see +//! [`mcp`]'s module doc for why that is a separate surface rather than two +//! more tools on the parent's, and why it is served per session under a +//! minted token rather than at one shared path — neither tool takes a +//! session name, so a subagent has no way to say it is somebody else. //! //! See [`session`]'s module doc for the actual design: no task files, no -//! restart recovery, no mid-turn compaction — the daemon's only state is an -//! in-memory `name -> Cancel` map, live only as long as the process is. +//! restart recovery, no mid-turn compaction — the daemon's only state is a +//! handful of in-memory maps, live only as long as the process is. pub mod mcp; pub mod mcp_config; diff --git a/hive-subagent-mcp/src/main.rs b/hive-subagent-mcp/src/main.rs index 40f5fdc3..147d9554 100644 --- a/hive-subagent-mcp/src/main.rs +++ b/hive-subagent-mcp/src/main.rs @@ -1,7 +1,9 @@ //! `hive-subagent-daemon` binary — spawns nested claude sessions on -//! request and serves the `start`/`continue`/`interrupt` MCP tool surface -//! directly over streamable-http on `--http ` — no stdio bridge, no -//! separate bin claude has to respawn every turn. +//! request and serves the `start`/`continue`/`status`/`interrupt` MCP tool +//! surface directly over streamable-http on `--http ` — no stdio +//! bridge, no separate bin claude has to respawn every turn. The same +//! address also carries the subagent-facing `goal_reached`/`need_help` +//! route, which is the only place the daemon can learn its own URL from. use std::sync::Arc; @@ -42,7 +44,17 @@ async fn main() -> Result<()> { "hive-subagent-daemon starting" ); - let state = Arc::new(hive_subagent_mcp::session::State::new(todo_socket)); + // The one place the subagent-facing signal URLs can come from: the + // address this process was told to listen on. Anything else would be a + // guess at the deployment's own port assignment. This is only their + // common prefix — each session's actual URL is this plus a token minted + // for that session alone, which is what makes a signal's identity a + // property of the endpoint rather than of the payload. + let signal_base = format!("http://{}{}", cli.http, hive_subagent_mcp::mcp::SIGNAL_PATH); + let state = Arc::new(hive_subagent_mcp::session::State::new( + todo_socket, + signal_base, + )); // Serve the MCP tools over streamable-http forever. No background poll // loop to start — unlike the bash daemon's task-file queue, `start`/ diff --git a/hive-subagent-mcp/src/mcp.rs b/hive-subagent-mcp/src/mcp.rs index fd5b7bef..9d1bf134 100644 --- a/hive-subagent-mcp/src/mcp.rs +++ b/hive-subagent-mcp/src/mcp.rs @@ -1,9 +1,30 @@ //! The MCP tool surface: `start` / `continue` / `status` / `interrupt`, //! served directly over streamable-http — no stdio bridge, no round-trip //! socket. +//! +//! Two surfaces, two routes. `/mcp` is the parent's: the four tools above. +//! `/signal/mcp/` is the *subagent's*, and carries `goal_reached` and +//! `need_help` only — it is what a subagent is handed in its own +//! `--mcp-config` (see [`crate::mcp_config`]), so the ability to report on +//! its own run can't be the ability to spawn a nested one. +//! +//! **The token in that path is the subagent's identity, and neither signal +//! tool takes a session name.** One route is registered, once, at startup +//! (sessions come and go far faster than a `Router` can be rebuilt); every +//! request resolves its token to a session and dispatches to a handler bound +//! to *that* session, so the name a signal lands against is never a value the +//! sender chose. A token that resolves to nothing — never minted, or revoked +//! when its run ended — is a bare 404, which is also why the route says +//! nothing that would let a subagent enumerate its siblings. -use std::sync::Arc; +use std::collections::HashMap; +use std::sync::{Arc, Mutex, PoisonError}; +use axum::extract::{Path, Request, State as RouteState}; +use axum::response::{IntoResponse as _, Response}; +use rmcp::transport::streamable_http_server::{ + StreamableHttpServerConfig, StreamableHttpService, session::local::LocalSessionManager, +}; use rmcp::{ ServerHandler, handler::server::wrapper::Parameters, @@ -57,12 +78,62 @@ struct StartArgs { /// this daemon tracks in memory. #[serde(default)] dir: Option, + /// What this subagent is working *toward*, in its own words — set it and + /// the daemon keeps giving it turns until it says it's done, says it's + /// stuck, or runs out. Omit it and the session is a single turn, exactly + /// as before. Written for the subagent to read: it's quoted back at it + /// verbatim at the start of every continued turn, so "get `cargo clippy + /// --workspace` to pass with no warnings" continues far better than "fix + /// the lints". + #[serde(default)] + goal: Option, + /// How many turns the continuation may spend before the harness stops it + /// itself. Default 5. Only meaningful alongside `goal` — without one + /// there's nothing to re-prompt toward, so nothing to cap. The cap + /// bounds *unattended* re-prompting: a `continue` you issue yourself + /// starts the allowance over. + #[serde(default)] + max_turns: Option, + /// Where the instructions in `prompt_file` told this subagent to write + /// its report. The daemon appends the run's stop reason to that file when + /// the run ends, so the artifact you were going to read anyway also says + /// how it stopped. Nothing is inferred: if you don't pass it (and the + /// subagent doesn't name it when it signals), no file is touched. + #[serde(default)] + report_file: Option, } fn default_trigger() -> String { "Carry out the task described in your instructions.".to_owned() } +/// No session name, deliberately: which session this signal is about is +/// decided by the URL it arrives on (see this module's doc), so there is no +/// field here in which one subagent could name another. +#[derive(Debug, Deserialize, JsonSchema)] +struct GoalReachedArgs { + /// Optionally, what you did. It's shown to whoever spawned you. + #[serde(default)] + msg: Option, + /// Optionally, the path you wrote your report to, so the stop reason + /// gets appended to it. + #[serde(default)] + report_file: Option, +} + +/// No session name here either — same reason as [`GoalReachedArgs`]. +#[derive(Debug, Deserialize, JsonSchema)] +struct NeedHelpArgs { + /// What is blocking you, specifically enough for someone else to act on + /// it. This is the whole content of the signal, which is why it's + /// required. + msg: String, + /// Optionally, the path you wrote your report to, so the stop reason + /// gets appended to it. + #[serde(default)] + report_file: Option, +} + #[derive(Debug, Deserialize, JsonSchema)] struct ContinueArgs { /// The existing session's name (from a prior `start`). @@ -127,17 +198,26 @@ impl SubagentMcp { first (real fresh start, not a silent resume); a *currently running* one is \ refused. Runs unattended — every tool-call permission prompt is pre-approved rather \ than interactively confirmed — with its MCP server set fixed to what this daemon \ - configures for it. See the `base:claude-subagents` skill for when to reach for this." + configures for it. Pass `goal` to make this a multi-turn run: the daemon re-prompts \ + the subagent toward that goal each time a turn ends, up to `max_turns` (default 5), \ + stopping early when the subagent reports the goal reached or asks for help. Whichever \ + way it stops, one todo is pushed at the end and `status` says which. See the \ + `base:claude-subagents` skill for when to reach for this." )] fn start(&self, Parameters(args): Parameters) -> String { match session::start( &self.state, - &args.name, - args.model, - args.effort, - &args.prompt_file, - args.trigger, - args.dir.as_deref(), + session::StartRequest { + name: args.name, + model: args.model, + effort: args.effort, + prompt_file: args.prompt_file, + trigger: args.trigger, + dir: args.dir, + goal: args.goal, + max_turns: args.max_turns, + report_file: args.report_file, + }, ) { Ok(msg) => msg, Err(e) => format!("start error: {e:#}"), @@ -193,7 +273,9 @@ impl SubagentMcp { never launches a process, unlike `continue`. The answer says what state it found \ and what to do about it. For a running one it also reports how long since that turn \ last produced any output, which is how you tell a subagent that's working from one \ - that has wedged without resorting to `ps`." + that has wedged without resorting to `ps`. For a session started with a `goal` it \ + reports which turn of the budget it is on, and — once the run has stopped — why it \ + stopped: goal reported reached, blocked and needing help, or out of turns." )] fn status(&self, Parameters(args): Parameters) -> String { match session::status(&self.state, &args.name, args.dir.as_deref()) { @@ -206,35 +288,297 @@ impl SubagentMcp { #[tool_handler] impl ServerHandler for SubagentMcp {} +/// The surface a *subagent* gets, served on its own route: two tools, both +/// of them ways for a running subagent to say how its own run should end. +/// Separate handler rather than two more tools on [`SubagentMcp`] so the +/// split is structural — there is no route a subagent holds that `start` is +/// reachable from. +/// +/// `session` is the whole point of the type: an instance is built only by +/// `signal_route`, from a token that had already resolved, and both tools +/// read the session they act on out of `self`. Nothing in an incoming +/// payload can reach that field. +#[derive(Clone)] +struct SubagentSignalMcp { + state: Arc, + session: String, +} + +#[tool_router] +impl SubagentSignalMcp { + #[tool( + description = "Report that you have reached the goal you were given. Stops the harness \ + from starting another turn to re-prompt you toward it, and extends the \"subagent \ + done\" message your parent gets with what you say here. It always applies to your \ + own session and takes no session name — the endpoint you are calling it on is yours \ + alone, so there is nothing to address it to. This records a claim, not a result: \ + whoever spawned you reads the diff and the gate output regardless, so calling it \ + does not make unfinished work finished. Call it when the goal is actually met — \ + otherwise keep working, or call `need_help` if you can't proceed." + )] + fn goal_reached(&self, Parameters(args): Parameters) -> String { + session::goal_reached( + &self.state, + &self.session, + args.msg, + args.report_file.as_deref(), + ) + } + + #[tool( + description = "Report that you cannot proceed, and why. Stops the harness from starting \ + another turn to re-prompt you toward your goal, marks your session as blocked so \ + whoever spawned you sees it in `status` without reading any file, and extends the \ + \"subagent done\" message with your reason. Like `goal_reached` it takes no session \ + name and always applies to your own session. Use it for a genuine block — a missing \ + credential, a decision that isn't yours, an instruction that contradicts what you \ + found — not for work that is merely hard. Say enough that someone else can act on it." + )] + fn need_help(&self, Parameters(args): Parameters) -> String { + session::need_help( + &self.state, + &self.session, + args.msg, + args.report_file.as_deref(), + ) + } +} + +#[tool_handler] +impl ServerHandler for SubagentSignalMcp {} + +/// Prefix the subagent-facing signal surface is served under, and the part +/// of a subagent's URL that is *not* secret: every real endpoint is this plus +/// that session's own token. Kept here, next to the route that answers it, so +/// the two can't drift. Nothing is served at the bare prefix. +pub const SIGNAL_PATH: &str = "/signal/mcp"; + +/// Everything the one registered signal route needs to answer a request for +/// any session: the daemon state the token resolves against, and a session +/// manager per live token. +/// +/// **Per token, not one shared manager.** rmcp keeps an MCP session (created +/// by `initialize`, named by an `mcp-session-id` header) inside its manager, +/// and a manager shared across tokens would make that header a second way to +/// pick which bound handler serves a request — an id is unguessable, but this +/// is the one property the route exists to make structural rather than +/// probabilistic. With a manager per token, an id minted under one token +/// simply does not exist under another. +#[derive(Clone)] +struct SignalRouting { + state: Arc, + managers: Arc>>>, +} + +impl SignalRouting { + /// The session manager for `token`, created on its first request. + /// + /// Entries for tokens that no longer resolve are dropped on the way + /// past — the map would otherwise grow for the daemon's whole life, and + /// a revoked token's MCP sessions have nothing left to serve. It's a + /// linear scan of a map with one entry per *live* subagent, on a request + /// a subagent makes a handful of times per run. + fn manager_for(&self, token: &str) -> Arc { + let mut managers = self.managers.lock().unwrap_or_else(PoisonError::into_inner); + managers.retain(|t, _| self.state.session_for_signal_token(t).is_some()); + Arc::clone( + managers + .entry(token.to_owned()) + .or_insert_with(session_manager), + ) + } +} + +/// A session manager with this daemon's keep-alive: a subagent turn can run +/// considerably longer than a bash command — same 24h rationale as the bash +/// and matrix daemons. +fn session_manager() -> Arc { + let mut manager = LocalSessionManager::default(); + manager.session_config.keep_alive = Some(std::time::Duration::from_hours(24)); + Arc::new(manager) +} + +/// Resolve the request's token to a session and serve it with a handler bound +/// to that session — or 404, for a token this daemon never minted or has +/// since revoked. +/// +/// The `StreamableHttpService` is built per request, which is cheap (a few +/// `Arc`s) and is what lets the bound session come from the URL: rmcp's +/// service factory takes no request, so the binding has to happen outside it. +/// Only a request that starts an MCP session ever calls the factory; the rest +/// are answered from the token's own manager, against the handler built when +/// that session was initialized. +async fn signal_route( + RouteState(routing): RouteState, + Path(token): Path, + request: Request, +) -> Response { + let Some(session) = routing.state.session_for_signal_token(&token) else { + tracing::debug!("signal route: unknown or expired token, refusing with 404"); + return axum::http::StatusCode::NOT_FOUND.into_response(); + }; + let manager = routing.manager_for(&token); + let state = Arc::clone(&routing.state); + let service = StreamableHttpService::new( + move || { + Ok(SubagentSignalMcp { + state: Arc::clone(&state), + session: session.clone(), + }) + }, + manager, + StreamableHttpServerConfig::default(), + ); + service.handle(request).await.into_response() +} + /// Run the MCP server over HTTP (rmcp streamable-http transport) on `addr`. /// Loopback-only bind, one long-lived session — same shape as the bash and /// matrix daemons' own `serve_http`. /// +/// Two routes off one listener: `/mcp` for the parent's four tools, and +/// `/{token}` for the subagent's two. Separate session managers +/// because they're separate MCP servers to separate clients — the parent's +/// harness on one, each subagent's own claude on its own token's. +/// +/// The signal route is registered **once**, with the token as a path +/// parameter, because this `Router` is built at startup and subagents come +/// and go for the daemon's whole life: there is no mounting a fresh route per +/// session. Resolution therefore happens per request, in `signal_route`. +/// /// # Errors /// /// Returns an error if the listener cannot bind `addr` or the HTTP server /// exits with a fatal error. pub async fn serve_http(addr: std::net::SocketAddr, state: Arc) -> anyhow::Result<()> { - use rmcp::transport::streamable_http_server::{ - StreamableHttpServerConfig, StreamableHttpService, session::local::LocalSessionManager, - }; - let mut session_manager = LocalSessionManager::default(); - // A subagent turn can run considerably longer than a bash command — - // same 24h keep-alive rationale as the bash/matrix daemons. - session_manager.session_config.keep_alive = Some(std::time::Duration::from_hours(24)); - let session_manager = std::sync::Arc::new(session_manager); + let parent_state = Arc::clone(&state); let service = StreamableHttpService::new( move || { Ok(SubagentMcp { - state: Arc::clone(&state), + state: Arc::clone(&parent_state), }) }, - session_manager, + session_manager(), StreamableHttpServerConfig::default(), ); - let app = axum::Router::new().nest_service("/mcp", service); + let routing = SignalRouting { + state, + managers: Arc::new(Mutex::new(HashMap::new())), + }; + let app = axum::Router::new() + .nest_service("/mcp", service) + .route( + &format!("{SIGNAL_PATH}/{{token}}"), + axum::routing::any(signal_route), + ) + .with_state(routing); let listener = tokio::net::TcpListener::bind(addr).await?; - tracing::info!(%addr, "serving hive-subagent MCP over streamable-http at /mcp"); + tracing::info!( + %addr, + signal = SIGNAL_PATH, + "serving hive-subagent MCP over streamable-http at /mcp" + ); axum::serve(listener, app).await?; Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + /// The state a route test resolves against, with `name` already minted a + /// token — the same shape `start` leaves behind. + fn state() -> Arc { + Arc::new(State::new( + std::path::PathBuf::from("/dev/null"), + format!("http://127.0.0.1:1{SIGNAL_PATH}"), + )) + } + + fn routing(state: Arc) -> SignalRouting { + SignalRouting { + state, + managers: Arc::new(Mutex::new(HashMap::new())), + } + } + + /// Mint `name` a signal URL the way a spawning run does, and hand back + /// just the token — what the route would have taken out of the path. + fn minted(state: &State, name: &str) -> String { + let url = state.mint_signal_url(name); + url.rsplit('/') + .next() + .expect("a minted URL always has a last segment") + .to_owned() + } + + fn post(uri: &str) -> Request { + axum::http::Request::builder() + .method("POST") + .uri(uri) + .body(axum::body::Body::empty()) + .expect("a well-formed test request") + } + + #[test] + fn the_signal_tools_take_no_session_name() { + // The fix itself, pinned: a subagent has no field in which to name a + // sibling. Asserted against the generated JSON schema rather than the + // struct, because the schema is what actually reaches the subagent — + // a `name` re-added under any serde rename would show up here. + for (tool, schema) in [ + ("goal_reached", schemars::schema_for!(GoalReachedArgs)), + ("need_help", schemars::schema_for!(NeedHelpArgs)), + ] { + let schema = serde_json::to_value(&schema).expect("a schema serializes"); + let properties = schema + .get("properties") + .and_then(serde_json::Value::as_object) + .cloned() + .unwrap_or_default(); + assert!( + !properties.contains_key("name"), + "`{tool}` must not take a session name — identity comes from the route's token, \ + and an argument for it is an argument one subagent can fill in with another's \ + name. Properties offered: {:?}", + properties.keys().collect::>() + ); + } + } + + #[tokio::test] + async fn an_unminted_signal_token_is_a_bare_404() { + // No enumeration: a token this daemon never issued gets the same + // answer whether or not a session by any name exists. + let state = state(); + minted(&state, "live"); + let response = signal_route( + RouteState(routing(state)), + Path("not-a-token".to_owned()), + post("/signal/mcp/not-a-token"), + ) + .await; + assert_eq!(response.status(), axum::http::StatusCode::NOT_FOUND); + } + + #[tokio::test] + async fn a_minted_signal_token_reaches_the_surface_at_all() { + // The other half of the 404 above: it is the *resolution* refusing, + // not the route being unreachable. The body here is not a valid MCP + // request, so rmcp rejects it — on its own terms, with some answer + // that is not this route's "no such token". + let state = state(); + let token = minted(&state, "live"); + let response = signal_route( + RouteState(routing(state)), + Path(token.clone()), + post(&format!("/signal/mcp/{token}")), + ) + .await; + assert_ne!( + response.status(), + axum::http::StatusCode::NOT_FOUND, + "a live session's own token must resolve" + ); + } +} diff --git a/hive-subagent-mcp/src/mcp_config.rs b/hive-subagent-mcp/src/mcp_config.rs index 74520314..3818cdcd 100644 --- a/hive-subagent-mcp/src/mcp_config.rs +++ b/hive-subagent-mcp/src/mcp_config.rs @@ -1,35 +1,74 @@ //! Builds a subagent's own filtered `--mcp-config`: only //! `hyperhive.extraMcpServers` entries with `availableToSubagents = true` //! (see `hive_agent_sock::extra_mcp::ExtraMcpServer::available_to_subagents`) ever reach -//! a subagent's claude invocation. Everything else — the built-in hyperhive +//! a subagent's claude invocation, plus this daemon's own two-tool signal +//! surface. Everything else — the built-in hyperhive //! surface (todos/messaging), the automatically injected `bash` and `subagent` //! entries — stays unreachable by construction: none of those default to //! opted in, and the built-in surface isn't an `extraMcpServers` entry at //! all, so there's no name for an operator to opt it in under even if they //! wanted to. +//! +//! The signal surface is the one server a subagent always gets. It is a +//! *different route* on this daemon's listener from the one the parent +//! uses, serving `goal_reached` and `need_help` and nothing else — so +//! "a subagent can say it is done or stuck" never widens into "a subagent +//! can spawn subagents", which is what handing it the parent's route would +//! have meant. +//! +//! **One file per session, not one file.** The signal URL carries the +//! session's own token (`State::mint_signal_url`), so the +//! file it is written into has to be the session's own too: a single shared +//! path is a race between two `start`s — whichever wrote last decides which +//! token the other's `claude` reads at startup, which would hand a subagent +//! a sibling's identity by accident. use std::path::PathBuf; -/// Filename the rendered config lives at, under [`crate::paths::harness_dir`]. -const CONFIG_FILE: &str = "subagent-mcp-config.json"; +/// Filename the rendered config lives at, under [`crate::paths::harness_dir`], +/// per session. The name is a validated [`hive_types::Ident`] by the time it +/// gets here (`crate::session::validate_name`), so it is a single safe path +/// segment and never escapes the directory. +fn config_file(name: &str) -> String { + format!("subagent-mcp-config-{name}.json") +} -/// Render the subagent-eligible extra-MCP servers to a `--mcp-config` file, -/// returning its path — or `None` when no entry opts in (or the render/write -/// fails), so [`hive_claude::Config::mcp_config`] stays unset and the -/// subagent gets literally zero MCP servers, the same default as before this -/// toggle existed. Re-rendered on every call (cheap: a filter plus a small +/// Name the signal surface appears under in a subagent's own MCP config, +/// and therefore the prefix its tools are called by +/// (`mcp__subagent_control__goal_reached`). +const SIGNAL_SERVER: &str = "subagent_control"; + +/// Render a subagent's `--mcp-config` file, returning its path — or `None` +/// when there is nothing to put in it (or the render/write fails), so +/// [`hive_claude::Config::mcp_config`] stays unset and the subagent gets +/// literally zero MCP servers. +/// +/// `signal_url` is `name`'s *own* `goal_reached`/`need_help` route — this +/// daemon's signal path plus the token minted for this run, and the only +/// place that token is ever written. It is always included when given, since +/// a subagent that can't say it's done or stuck is exactly the one the turn +/// cap has to stop on its behalf. `None` reproduces the pre-continuation +/// shape — only the opted-in extras, and no file at all when none opt in. +/// +/// Re-rendered on every call (cheap: a filter plus a small /// file write) rather than cached once at daemon startup, so a config change /// takes effect on this subagent's next `start`/`continue` without needing /// the daemon itself restarted. #[must_use] -pub fn build() -> Option { +pub fn build(name: &str, signal_url: Option<&str>) -> Option { let state_dir = crate::paths::state_dir(); - let servers: serde_json::Map = + let mut servers: serde_json::Map = hive_agent_sock::extra_mcp::load_extra_mcp() .into_iter() .filter(|(_, spec)| spec.available_to_subagents()) .map(|(name, spec)| (name, spec.to_json_entry(&state_dir))) .collect(); + if let Some(url) = signal_url { + servers.insert( + SIGNAL_SERVER.to_owned(), + serde_json::json!({ "type": "http", "url": url }), + ); + } if servers.is_empty() { return None; } @@ -44,7 +83,7 @@ pub fn build() -> Option { ); return None; } - let path = dir.join(CONFIG_FILE); + let path = dir.join(config_file(name)); if let Err(e) = std::fs::write(&path, body) { tracing::warn!( error = ?e, diff --git a/hive-subagent-mcp/src/session.rs b/hive-subagent-mcp/src/session.rs index e2747ae7..37d2ff05 100644 --- a/hive-subagent-mcp/src/session.rs +++ b/hive-subagent-mcp/src/session.rs @@ -1,14 +1,14 @@ -//! The claude-facing half of this daemon: spawn a subagent turn, track it -//! only while it's alive, and push exactly one todo when it ends — saying -//! whether it finished or was killed. +//! The claude-facing half of this daemon: spawn a subagent turn, keep giving +//! it turns until it says it's done or runs out of them, track it only while +//! it's alive, and push exactly one todo when the whole run stops — saying +//! whether it finished, was killed, or stopped for one of the four reasons +//! the continuation loop records. //! //! **No task files, no restart recovery.** The daemon's only state is an -//! in-memory `name -> Option` map (see `State`'s own doc for what -//! the `None`/`Some` split is for), a `name -> dir` memory so `start`'s -//! `dir` doesn't have to be repeated on every later `continue`/`status` -//! (`State::resolve_dir`), and a `name -> last_event_at` liveness clock -//! (`State::note_event`) — all of it living for exactly as long as the -//! process is. A daemon restart means whatever was running gets killed +//! in-memory `name -> Option` map (see `State`'s own doc for the +//! rest of the maps and for what the `None`/`Some` split is for) — all of +//! it living for exactly as long as the process is. A daemon restart means +//! whatever was running gets killed //! with it (`tokio`'s own child-process drop semantics), not adopted, and //! the remembered `dir` is gone too. The durable record of a subagent's //! existence is `hive_claude::SessionStore` — claude's own on-disk @@ -28,6 +28,32 @@ //! a missed resume as its own error, naming the directory searched //! (`classify_end`). See `docs/tools/subagent.md`. +//! **One `start` can be more than one turn.** With a `goal` set, a turn that +//! ends without a stop signal is followed by another re-prompting the +//! subagent toward it, up to `max_turns` (default five). The loop lives in +//! `spawn_and_track`'s background task, so both tool-call contracts are +//! unchanged and only one todo is pushed, when the *run* stops. No goal +//! means what it always did: one turn, one todo. +//! +//! **A subagent's own stop signals are labels, not gates.** `goal_reached` +//! and `need_help` stop the loop and extend the done message; neither +//! verifies anything. `goal_reached` is self-reported by a subagent that has +//! just been re-prompted with "you have not reached the goal", which is +//! exactly the incentive to claim it — the same failure class as a build +//! report asserting "done, tests pass". Everything here treats it as a claim +//! about the work, never as the work, and says so. See +//! `docs/tools/subagent.md`. + +//! **A subagent cannot say who it is.** Neither signal tool takes a session +//! name. Each run is minted an unguessable token at spawn +//! (`State::mint_signal_url`), the URL carrying it is written into that one +//! subagent's own `--mcp-config`, and the route resolves it back to a session +//! before dispatching — so the identity of a signal is a property of the +//! endpoint it arrived on, not a field its sender filled in. An unminted or +//! revoked token is a 404. The `occupancy()` liveness check that used to +//! stand here instead was a guard on an assertion: two siblings running +//! concurrently could each satisfy it for the other's name. + //! **A killed turn is not a finished turn.** A child that died on a signal //! arrives as a `hive_claude::Error::Exit` carrying its `ExitStatus`, so the //! "how" is there to be read: `classify_end` takes the signal out of it and @@ -72,6 +98,68 @@ use tokio::sync::oneshot; /// any of those runs, while still bounding the one case that reaches it. const RESUME_GRACE: Duration = Duration::from_secs(5); +/// How many turns a goal-continued session gets before the harness stops it +/// itself. Five is the number the feature was specified with, not one tuned +/// here: enough for a bounded batch to converge, short enough that a +/// subagent which has misunderstood its goal can't re-attempt it forever on +/// someone else's budget. `start`'s `max_turns` overrides it per session, +/// which is where a caller that genuinely needs a longer leash says so. +const DEFAULT_MAX_TURNS: u32 = 5; + +/// Why a session's turn continuation stopped — recorded per name, reported +/// by `status`, appended to the end-of-turn todo, and written into the +/// session's report file when it has one. +/// +/// Only the four ends of the *continuation loop* live here. A turn that was +/// killed or that failed outright never reaches the loop's decision at all: +/// those keep the records they already had (`State::killed`, the todo's own +/// killed wording), and giving them a second home here would have handed +/// `status` two rival answers for one fact. +#[derive(Debug, Clone, PartialEq, Eq)] +enum StopReason { + /// The turn ended and there was no goal to continue toward — the + /// single-turn shape every session had before continuation existed. + Done, + /// The subagent called `goal_reached`. **Self-reported.** It records + /// that the subagent claimed the goal, never that the goal was met. + GoalReached(Option), + /// The subagent called `need_help`: it can't proceed, and says why. + NeedHelp(String), + /// `turns` turns ran and the goal was never reported reached. + TurnCap { turns: u32 }, +} + +impl StopReason { + /// The one sentence that says why, shared by the todo extension and the + /// report-file line so a parent reading either sees the same words. + /// + /// `GoalReached` carries its "self-reported" caveat in the sentence + /// itself rather than leaving it to whichever surface renders it: the + /// caveat is the load-bearing half of that claim, and a surface that + /// forgot to add it would read as verification. + fn sentence(&self) -> String { + match self { + Self::Done => "its turn ended and there was no goal to continue toward".to_owned(), + Self::GoalReached(msg) => { + let said = msg + .as_deref() + .map_or_else(String::new, |m| format!(": {m}")); + format!( + "the subagent reported its goal reached{said} — self-reported, not verified, \ + so read what it actually changed before acting on it" + ) + } + Self::NeedHelp(msg) => { + format!("the subagent called `need_help` and can't proceed: {msg}") + } + Self::TurnCap { turns } => format!( + "the harness turn limit was reached ({turns} turns) without the goal ever being \ + reported reached, so the work stopped where it had got to" + ), + } + } +} + /// How a subagent's turn ended, as far as the daemon can tell from what /// [`hive_claude::RunningClaude::wait`] returned. /// @@ -185,10 +273,10 @@ fn searched_location(config: &Config) -> Option { )) } -/// This daemon's whole state: which names currently have a live process or -/// a reservation in flight, and where to push the completion todo. -/// `Arc`-wrapped so the background task that drives a turn to completion -/// can outlive the tool call that started it. +/// This daemon's whole state: which names have a live process or a +/// reservation in flight, what each is working toward and how it stopped, +/// and where to push the completion todo. `Arc`-wrapped so the background +/// task driving a run outlives the tool call that started it. /// /// The map value is `Option`: `None` means `name` is reserved for /// an in-flight `start`/`continue` that hasn't reached a confirmed @@ -220,24 +308,136 @@ pub struct State { dirs: Mutex>, killed: Mutex>, last_event: Mutex>, + /// Turn continuation's three records. Unlike the four maps above they + /// deliberately **outlive the turn** — a stop reason that vanished with + /// the process it described would be unreadable by the time anyone + /// asked — so they're cleared by the next `start` under the same name, + /// not by a turn ending. + goals: Mutex>, + stops: Mutex>, + reports: Mutex>, socket: PathBuf, + /// The prefix of every subagent's signal URL — this daemon's own `--http` + /// address with [`crate::mcp::SIGNAL_PATH`] on the end, and *not* a + /// reachable route by itself. It lives here because the daemon can only + /// learn it from its own `--http` argument — deriving it from a + /// convention would be the same inference the report path is careful not + /// to make. + signal_base: String, + signal_tokens: Mutex, +} + +/// Which opaque URL segment belongs to which session — the whole of a +/// subagent's identity, as far as `goal_reached`/`need_help` are concerned. +/// +/// A subagent is told one URL, in its own `--mcp-config`, and that URL is +/// what says who it is: it has no field to name a session in and no second +/// session's URL to reach for. Both directions live under one lock because a +/// half-updated pair is exactly the state in which a token could resolve to +/// a session that has since minted another one. +#[derive(Default)] +struct SignalTokens { + /// The token currently minted for a session, so a re-mint can retire it. + by_name: HashMap, + /// The resolution the route does: token -> the session it speaks for. + by_token: HashMap, +} + +/// What a session is being continued toward, and how far through its turn +/// budget it is. Present only for a session `start`ed with a `goal`; its +/// absence is what makes a session single-turn. +struct GoalState { + /// Verbatim from `start` — re-prompted at the subagent each turn rather + /// than paraphrased, since the caller wrote it for the subagent to read. + goal: String, + /// The cap this session runs under (`start`'s `max_turns`). + max_turns: u32, + /// Turns started so far, counting the first — so the very first turn is + /// `1 of max_turns`, not `0`. + turn: u32, } impl State { + /// `signal_base` is where the streamable-http endpoint a subagent's own + /// claude reaches `goal_reached`/`need_help` on *starts* — this daemon's + /// `--http` address with the signal route appended (see + /// `crate::mcp::serve_http`). Each session's actual URL is that plus its + /// own token; see `State::mint_signal_url`. #[must_use] - pub fn new(socket: PathBuf) -> Self { + pub fn new(socket: PathBuf, signal_base: String) -> Self { Self { running: Mutex::new(HashMap::new()), dirs: Mutex::new(HashMap::new()), killed: Mutex::new(HashMap::new()), last_event: Mutex::new(HashMap::new()), + goals: Mutex::new(HashMap::new()), + stops: Mutex::new(HashMap::new()), + reports: Mutex::new(HashMap::new()), socket, + signal_base, + signal_tokens: Mutex::new(SignalTokens::default()), + } + } + + /// Mint `name` a fresh signal URL: an unguessable token appended to + /// [`State::signal_base`], resolvable back to this one session and to no + /// other. Called once per spawned run, and the result goes into exactly + /// one place — that subagent's own `--mcp-config` (`crate::mcp_config`). + /// + /// A v4 UUID's 122 bits come from the OS CSPRNG, so the segment is not + /// derived from the name, the port or anything else a sibling subagent + /// could compute; a subagent that wants to signal as somebody else has + /// nothing to guess *from*. Minting replaces any token the name held + /// before, which is what stops a name's old URL surviving the run it was + /// issued for. + pub(crate) fn mint_signal_url(&self, name: &str) -> String { + let token = uuid::Uuid::new_v4().simple().to_string(); + let mut tokens = self + .signal_tokens + .lock() + .unwrap_or_else(PoisonError::into_inner); + if let Some(previous) = tokens.by_name.insert(name.to_owned(), token.clone()) { + tokens.by_token.remove(&previous); + } + tokens.by_token.insert(token.clone(), name.to_owned()); + drop(tokens); + format!("{}/{token}", self.signal_base) + } + + /// Which session an incoming signal request speaks for, or `None` for a + /// token this daemon never minted or has since revoked — which the route + /// answers with a bare 404 (see `crate::mcp::serve_http`). `None` is the + /// only failure shape there is: nothing about the answer distinguishes + /// "never existed" from "that run is over", so a caller holding a wrong + /// token learns nothing from being refused. + pub(crate) fn session_for_signal_token(&self, token: &str) -> Option { + self.signal_tokens + .lock() + .unwrap_or_else(PoisonError::into_inner) + .by_token + .get(token) + .cloned() + } + + /// Retire `name`'s signal URL — its run is over (or never started), so + /// the route it was handed stops resolving and answers 404 from here on. + /// This is the expiry half of "unknown or expired token ⇒ 404": without + /// it a finished subagent's config file would still name a live route. + fn revoke_signal_token(&self, name: &str) { + let mut tokens = self + .signal_tokens + .lock() + .unwrap_or_else(PoisonError::into_inner); + if let Some(token) = tokens.by_name.remove(name) { + tokens.by_token.remove(&token); } } /// `Some(true)` — a live process is tracked, interruptible. `Some(false)` - /// — reserved for an in-flight start/continue, not yet a confirmed - /// spawn. `None` — nothing tracked under `name` at all. + /// — the name is claimed but no process is confirmed under it: an + /// in-flight start/continue that hasn't spawned yet, or the gap between + /// one continued turn's child exiting and the next one spawning (see + /// `between_turns`). `None` — nothing tracked under `name` at all. fn occupancy(&self, name: &str) -> Option { self.running .lock() @@ -266,11 +466,23 @@ impl State { /// on the slow path between `reserve` and `Claude::spawn` succeeding). /// A no-op if the entry was already upgraded to `Some` — this only ever /// clears a still-`None` placeholder, never a live process. + /// + /// The signal token the failed call minted goes with it: no process ever + /// read that URL, and a token outliving the call that minted it is the + /// one way a route could resolve to a session that isn't there. fn release_reservation(&self, name: &str) { let mut running = self.running.lock().unwrap_or_else(PoisonError::into_inner); - if matches!(running.get(name), Some(None)) { + let released = matches!(running.get(name), Some(None)); + if released { running.remove(name); } + drop(running); + // Only alongside a reservation this actually released: a call that + // found a live process left it running, and revoking that run's URL + // would cut off a subagent which is still using it. + if released { + self.revoke_signal_token(name); + } } /// Retire the finished turn's tracking for `name` and remember how it @@ -287,7 +499,15 @@ impl State { /// The liveness clock goes with the `running` entry, for the same reason /// it's kept at all: it answers "is this turn still making progress", /// and a turn that has ended has no progress left to make. + /// + /// So does the session's signal token. Every caller of this is a point + /// where the *run* stops — a continued run's own turn boundary goes + /// through `between_turns` instead, and keeps its URL because the next + /// turn is the same subagent against the same rendered config. Revoking + /// here is what makes a signal for a session that already ended a 404 + /// rather than a late stop reason recorded against it. fn finish_turn(&self, name: &str, end: &TurnEnd) { + self.revoke_signal_token(name); let mut killed = self.killed.lock().unwrap_or_else(PoisonError::into_inner); match end { TurnEnd::Killed { signal } => killed.insert(name.to_owned(), *signal), @@ -332,13 +552,14 @@ impl State { /// is an elapsed age, which a clock adjustment must not be able to /// distort into a stall that never happened. /// - /// Also called once by `spawn_and_track` the moment the child exists, so - /// the clock starts at the spawn rather than at the first line. Without - /// that seed a subagent that wedged before emitting anything at all - /// would report no age forever — the one case where an age is most worth - /// having. The value therefore reads as "how long since the daemon last - /// heard anything from this child", counting the spawn itself as the - /// first thing it heard. + /// Also called by `spawn_and_track` the moment each turn's child exists, + /// so the clock starts at the spawn rather than at the first line. + /// Without that seed a subagent that wedged before emitting anything at + /// all would report no age forever — the one case where an age is most + /// worth having. The value therefore reads as "how long since the daemon + /// last heard anything from this child", counting the spawn itself as + /// the first thing it heard. A continued run re-seeds it per turn, for + /// the same reason: the age describes the turn in flight, not the run. fn note_event(&self, name: &str) { self.last_event .lock() @@ -408,6 +629,180 @@ impl State { .cloned(), } } + + /// Upgrade `name`'s `None` reservation to a real, interruptible process. + /// Same key as the reservation, so there is no window in which `name` + /// reads as unoccupied between the two. + fn track(&self, name: &str, cancel: Cancel) { + self.running + .lock() + .unwrap_or_else(PoisonError::into_inner) + .insert(name.to_owned(), Some(cancel)); + } + + /// Drop `name`'s cancel handle but keep the name claimed — the moment + /// between a continued turn's child exiting and its successor being + /// spawned. Without it the name would read as free mid-loop and a + /// concurrent `start` could take it out from under the continuation; + /// with it, `status` says "starting" and `interrupt` says "still + /// starting, retry shortly", both of which are true of the sub-second + /// gap it covers. + fn between_turns(&self, name: &str) { + self.running + .lock() + .unwrap_or_else(PoisonError::into_inner) + .insert(name.to_owned(), None); + } + + /// Set (or clear) `name`'s goal and start its turn budget over at turn + /// one. Called by `start` only: the goal is a property of the session + /// being created, and a later `continue` re-prompts toward whatever + /// `start` set rather than redefining it. + fn set_goal(&self, name: &str, goal: Option, max_turns: u32) { + let mut goals = self.goals.lock().unwrap_or_else(PoisonError::into_inner); + match goal { + Some(goal) => { + goals.insert( + name.to_owned(), + GoalState { + goal, + max_turns, + turn: 1, + }, + ); + } + None => { + goals.remove(name); + } + } + } + + /// Put `name` back at turn one, keeping whatever goal `start` set. A + /// `continue` is the parent's own deliberate turn, and the cap exists to + /// bound *unattended* continuation — so its allowance starts over rather + /// than a capped session being permanently un-continuable. The parent + /// was always the authority on whether more turns are worth spending. + fn restart_turns(&self, name: &str) { + if let Some(goal) = self + .goals + .lock() + .unwrap_or_else(PoisonError::into_inner) + .get_mut(name) + { + goal.turn = 1; + } + } + + /// `(turn, max_turns)` for `name`. `None` for a session with no goal, + /// which has no turn budget to be partway through — reporting `1 of 1` + /// there would invent a cap that isn't enforced. + fn turns(&self, name: &str) -> Option<(u32, u32)> { + self.goals + .lock() + .unwrap_or_else(PoisonError::into_inner) + .get(name) + .map(|g| (g.turn, g.max_turns)) + } + + /// Record why `name` stopped. Idempotent by design: the subagent's own + /// signal lands here mid-turn and the loop re-records the same reason at + /// the turn's end, so both paths can write without checking. + fn record_stop(&self, name: &str, stop: StopReason) { + self.stops + .lock() + .unwrap_or_else(PoisonError::into_inner) + .insert(name.to_owned(), stop); + } + + fn stop_reason(&self, name: &str) -> Option { + self.stops + .lock() + .unwrap_or_else(PoisonError::into_inner) + .get(name) + .cloned() + } + + /// Forget why `name` last stopped — called when a new turn is starting, + /// since the record describes the run before it and would otherwise make + /// a session that has since been given another turn still read as + /// blocked or out of turns. + fn clear_stop(&self, name: &str) { + self.stops + .lock() + .unwrap_or_else(PoisonError::into_inner) + .remove(name); + } + + /// Remember where `name` writes its report, so a stop reason can be + /// appended to the artifact a parent already reads rather than living + /// only in a todo. `None` leaves whatever was remembered alone, so a + /// signal tool that doesn't name a path doesn't erase the one `start` + /// gave. + /// + /// Never inferred: `start` carries what the brief named, and the signal + /// tools carry where the subagent says it actually wrote. A daemon that + /// derived this from a path convention would be guessing about someone + /// else's layout. + fn set_report_file(&self, name: &str, path: Option<&str>) { + if let Some(path) = path { + self.reports + .lock() + .unwrap_or_else(PoisonError::into_inner) + .insert(name.to_owned(), PathBuf::from(path)); + } + } + + /// Forget `name`'s report path — a fresh `start` under the same name is + /// a different piece of work, and inheriting the last one's artifact + /// path would append its stop reason to a file this run never wrote. + fn clear_report_file(&self, name: &str) { + self.reports + .lock() + .unwrap_or_else(PoisonError::into_inner) + .remove(name); + } + + fn report_file(&self, name: &str) -> Option { + self.reports + .lock() + .unwrap_or_else(PoisonError::into_inner) + .get(name) + .cloned() + } + + /// What the continuation loop does after a turn that ran to completion: + /// stop with a reason, or spend another turn re-prompting toward the + /// goal. Advances the turn counter itself, since deciding to continue + /// and consuming a turn are the same act. + /// + /// A signal the subagent raised mid-turn is already in `stops` and wins + /// outright — that is what "both stop goal continues" means, and it's + /// checked before the goal so a `goal_reached` on the last allowed turn + /// reads as reached rather than as capped. + fn plan_after_turn(&self, name: &str) -> Continuation { + if let Some(stop) = self.stop_reason(name) { + return Continuation::Stop(stop); + } + let mut goals = self.goals.lock().unwrap_or_else(PoisonError::into_inner); + let Some(state) = goals.get_mut(name) else { + return Continuation::Stop(StopReason::Done); + }; + if state.turn >= state.max_turns { + return Continuation::Stop(StopReason::TurnCap { turns: state.turn }); + } + state.turn += 1; + Continuation::Continue { + prompt: continuation_prompt(name, &state.goal, state.turn, state.max_turns), + } + } +} + +/// `plan_after_turn`'s answer: the loop either stops with a reason to +/// report, or has the next turn's prompt ready to spawn against. +#[derive(Debug, PartialEq, Eq)] +enum Continuation { + Stop(StopReason), + Continue { prompt: String }, } /// A caller-chosen name, validated the same way `hive-bash-mcp`'s task ids @@ -448,16 +843,27 @@ fn subagent_otel_attrs(name: &str) -> String { /// property is `strict_mcp_config: true` with no ambient MCP discovery, not /// an unconditional absence of `--mcp-config`: a subagent gets exactly the /// `hyperhive.extraMcpServers` entries an operator has explicitly opted in -/// via `availableToSubagents = true` (`crate::mcp_config::build`), nothing -/// implicit and nothing more. With no entry opted in — the default — that -/// resolves to `None` and the invocation is unchanged from before this -/// toggle existed: zero MCP servers, full stop. +/// via `availableToSubagents = true` (`crate::mcp_config::build`), plus this +/// daemon's own two-tool signal surface when `signal_url` is given — nothing +/// implicit and nothing more. +/// +/// `signal_url` is what makes `goal_reached`/`need_help` callable at all: a +/// subagent reaches them over the same streamable-http listener its parent +/// uses, on a route that serves those two tools and nothing else, so being +/// able to say "I'm done" never carries the ability to spawn a subagent of +/// its own. It is also *this* session's own URL — minted per run by +/// [`State::mint_signal_url`] and written only into this session's config +/// file — which is where the signal tools get the identity they no longer +/// ask the caller for. `None` — which only `status` passes, building a +/// config purely to resolve the session store — leaves the surface out +/// entirely. fn build_config( name: &str, model: Option, effort: Option, prompt_file: Option<&str>, dir: Option<&str>, + signal_url: Option<&str>, ) -> Config { let mut extra_args = vec!["--dangerously-skip-permissions".to_owned()]; if let Some(path) = prompt_file { @@ -468,7 +874,7 @@ fn build_config( model, effort: Some(effort.unwrap_or_else(|| "medium".to_owned())), cwd: dir.map(PathBuf::from), - mcp_config: crate::mcp_config::build(), + mcp_config: crate::mcp_config::build(name, signal_url), strict_mcp_config: true, extra_args, env: vec![( @@ -489,12 +895,44 @@ fn build_store(config: &Config) -> std::io::Result { )) } +/// Everything a `start` needs, as one struct rather than a parameter list: +/// the call already carried six mostly-optional values before goals were +/// added, and nine positional arguments is both unreadable at the call site +/// and a lint. +pub struct StartRequest { + /// Session name — the tracking key and claude's own session title. + pub name: String, + pub model: Option, + pub effort: Option, + /// File holding the subagent's task instructions. + pub prompt_file: String, + /// The first turn's prompt. A goal, when given, is appended to it. + pub trigger: String, + /// Working directory for the session; `None` inherits the daemon's. + pub dir: Option, + /// What this session is being continued *toward*. `None` keeps the + /// pre-continuation shape: one turn, one todo, no re-prompting. + pub goal: Option, + /// Turn cap for the continuation, defaulting to `DEFAULT_MAX_TURNS`. + /// Ignored without a `goal`, which is what continuation continues + /// toward — there is nothing to re-prompt against otherwise. + pub max_turns: Option, + /// Where this session's brief told it to write its report, so the stop + /// reason can be appended to that artifact. Never inferred — see + /// `State::set_report_file`. + pub report_file: Option, +} + /// Start a fresh subagent under `name`. A prior *finished* session under /// the same name is archived first (so this is a real fresh start, not a /// silent resume of old history) — a *currently running* one is refused /// outright, since `hive_claude::InfiniteSession`'s own docs warn that two /// concurrent runs against the same name corrupt both. /// +/// With a `goal` set this starts a whole *run*, not a single turn: see +/// `spawn_and_track` for the continuation loop. The return is unchanged +/// either way — it reports the first turn's spawn, not the run's outcome. +/// /// # Errors /// /// A name already running, an invalid name, an archive failure, or the @@ -502,28 +940,34 @@ fn build_store(config: &Config) -> std::io::Result { /// 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 /// succeeds. -pub fn start( - state: &Arc, - name: &str, - model: Option, - effort: Option, - prompt_file: &str, - trigger: String, - dir: Option<&str>, -) -> anyhow::Result { +pub fn start(state: &Arc, req: StartRequest) -> anyhow::Result { + let name = req.name.as_str(); validate_name(name)?; if !state.reserve(name) { anyhow::bail!("subagent `{name}` is already running — use `continue` or `interrupt`"); } // Only commit the remembered `dir` now that `reserve` has actually - // claimed `name` — see `resolve_dir`'s doc for why the order matters. - let dir = state.resolve_dir(name, dir); + // claimed `name` — see `resolve_dir`'s doc for why the order matters, + // and the same reasoning governs the three records below it. + let dir = state.resolve_dir(name, req.dir.as_deref()); + // A fresh start owns none of the previous run's records: its goal, its + // turn budget, why it stopped and where it wrote are all about work this + // call is deliberately replacing. + state.clear_stop(name); + state.clear_report_file(name); + state.set_report_file(name, req.report_file.as_deref()); + let max_turns = req.max_turns.unwrap_or(DEFAULT_MAX_TURNS).max(1); + state.set_goal(name, req.goal.clone(), max_turns); + let trigger = match req.goal.as_deref() { + None => req.trigger, + Some(goal) => format!("{}{}", req.trigger, goal_briefing(name, goal, max_turns)), + }; let result = start_reserved( state, name, - model, - effort, - prompt_file, + req.model, + req.effort, + &req.prompt_file, trigger, dir.as_deref(), ); @@ -546,7 +990,15 @@ fn start_reserved( trigger: String, dir: Option<&str>, ) -> anyhow::Result { - let config = build_config(name, model, effort, Some(prompt_file), dir); + let signal_url = state.mint_signal_url(name); + let config = build_config( + name, + model, + effort, + Some(prompt_file), + dir, + Some(&signal_url), + ); let store = build_store(&config)?; if store.find_by_title(name).is_some() { tracing::info!( @@ -593,6 +1045,9 @@ fn start_reserved( /// so, since a caller that never ran `status` and missed the todo would /// otherwise carry on from cut-off work believing it was finished work. /// +/// A goal session re-enters its continuation loop here with a fresh turn +/// budget — the cap bounds *unattended* re-prompting, not the parent's own. +/// /// # Errors /// /// An invalid name, one already running, `Claude::spawn` failing, or the @@ -617,6 +1072,10 @@ pub async fn continue_( // Only commit the remembered `dir` now that `reserve` has actually // claimed `name` — see `resolve_dir`'s doc for why the order matters. let dir = state.resolve_dir(name, dir); + // A new turn supersedes why the last run stopped, and gives a + // goal-continued session its allowance back — see `restart_turns`. + state.clear_stop(name); + state.restart_turns(name); let (tx, rx) = oneshot::channel(); let verdict: VerdictTx = Arc::new(Mutex::new(Some(tx))); let started = continue_reserved(state, name, prompt, model, effort, dir.as_deref(), &verdict); @@ -624,9 +1083,10 @@ pub async fn continue_( state.release_reservation(name); return started; } - // Nothing to release on this path: a turn that ended inside the grace - // has already been through `finish_turn`, which clears the tracking the - // spawn put there. + // Nothing to release on this path either way: a turn that ended inside + // the grace has already been through `finish_turn`, which clears the + // tracking the spawn put there, and one that a goal carried into another + // turn still owns the name via `between_turns`. await_resume(rx).await?; started.map(|msg| note_resumed_after_kill(&msg, killed)) } @@ -642,6 +1102,14 @@ pub async fn continue_( /// reported as started — which it is, with the end-of-turn todo left to say /// how it goes. /// +/// The third case is the channel closing with nothing ever sent — both the +/// sink and the background task dropping their `Arc` without calling +/// `settle`, which only a panic in the task can produce. That reads here as +/// "started", the same as `Underway`, and the fail-open is deliberate: the +/// one thing already known is that `Claude::spawn` returned a live pid, so +/// answering "the resume missed" would be a claim about the session that +/// nothing observed. The turn's real end still reaches the caller as a todo. +/// /// # Errors /// /// The turn's own failure message, verbatim: the caller asked claude to @@ -653,7 +1121,9 @@ async fn await_resume(rx: oneshot::Receiver) -> anyhow::Result<() // Everything else is a turn that started: it spoke (`Underway`), or // it ended on its own terms within the grace — `Complete`, or a // `Killed` that some concurrent `interrupt` asked for and whose todo - // says so — or it is still going when the grace runs out. + // says so — or it is still going when the grace runs out, or nobody + // ever sent at all, which only a panicked task produces and which + // this deliberately fails open on. See the doc above. _ => Ok(()), } } @@ -684,7 +1154,10 @@ fn continue_reserved( dir: Option<&str>, verdict: &VerdictTx, ) -> anyhow::Result { - let config = build_config(name, model, effort, None, dir); + // A fresh URL for the resumed run, not the one the last run was handed: + // a token is per run, and this is a new one. + let signal_url = state.mint_signal_url(name); + let config = build_config(name, model, effort, None, dir, Some(&signal_url)); // No existence pre-check: claude's own `--resume` is the authority on // whether the session is there, and it errors rather than quietly // starting a fresh one. `verdict` is how that answer gets back to the @@ -769,6 +1242,16 @@ impl hive_claude::Sink for LivenessSink { /// that is waiting for it (`await_resume`); `None` on a `start`, which keeps /// the immediate-return contract unchanged. /// +/// **The turn *continuation* loop lives in that background task, and only +/// there.** A session `start`ed with a goal runs turn after turn until +/// something stops it, and every one of those turns is a fresh +/// `Claude::spawn` against `Attach::Resume` — the driver's `wait` consumes +/// its child, so there is no other shape it could take. Keeping the loop +/// behind the same `tokio::spawn` is what leaves both tool-call contracts +/// untouched: `start` still returns at the first spawn, `continue` still +/// returns when its own turn is underway, and neither waits on turns two +/// through five. +/// /// Not `async` itself — `tokio::spawn` needs an active runtime to spawn /// *onto*, not an `async` caller to spawn *from*. fn spawn_and_track( @@ -781,14 +1264,7 @@ fn spawn_and_track( ) -> anyhow::Result { let running = Claude::spawn(config, attach) .map_err(|e| anyhow::anyhow!("starting the subagent process failed: {e}"))?; - // Upgrades the `None` reservation `reserve` already placed here to a - // real cancel handle — same key, so there's no window where `name` - // reads as unoccupied between the reservation and this insert. - state - .running - .lock() - .unwrap_or_else(PoisonError::into_inner) - .insert(name.to_owned(), Some(running.cancel_handle())); + state.track(name, running.cancel_handle()); // This turn supersedes whatever the previous one did, including having // been killed — the record is about the turn before this one. state.clear_kill(name); @@ -798,85 +1274,297 @@ fn spawn_and_track( // Resolved here, on the calling thread, while the config is still to // hand: only a resume can miss, and only `classify_end` finding a - // `SessionNotFound` ever uses it. - let searched = matches!(attach, Attach::Resume(_)) - .then(|| searched_location(config)) - .flatten(); + // `SessionNotFound` ever uses it. Every continuation turn is a resume, + // so this is worth having even when the *first* attach is a `Create`. + let resume_searched = searched_location(config); + let mut searched = if matches!(attach, Attach::Resume(_)) { + resume_searched.clone() + } else { + None + }; + let config = config.clone(); let state = Arc::clone(state); let task_name = name.to_owned(); tokio::spawn(async move { - let sink = LivenessSink { - state: Arc::clone(&state), - name: task_name.clone(), - verdict: verdict.clone(), - }; - let end = classify_end(running.wait(&prompt, &sink).await, searched.as_deref()); - match &end { - TurnEnd::Complete => {} - TurnEnd::Killed { signal } => { - tracing::warn!( - name = %task_name, - signal, - "subagent: turn killed — the child died on a signal, it did not finish" - ); + let mut running = running; + let mut prompt = prompt; + loop { + let sink = LivenessSink { + state: Arc::clone(&state), + name: task_name.clone(), + verdict: verdict.clone(), + }; + let end = classify_end(running.wait(&prompt, &sink).await, searched.as_deref()); + log_turn_end(&task_name, &end); + // The todo is how a turn's end reaches an agent that is no + // longer looking — so it is pushed for every end *except* the + // one the caller is being handed as a tool-call error right now. + // `settle` saying the verdict was delivered is what makes that + // certain: a `continue` whose grace had already run out gets + // `false` here and its todo, same as before. Only the first turn + // can ever win this — the sender is consumed — which is right, + // since only the first turn is one a caller is still waiting on. + let reported = settle(verdict.as_ref(), ResumeVerdict::Ended(end.clone())); + + if !matches!(end, TurnEnd::Complete) { + // A killed or failed turn ends the run, goal or not: there is + // nothing to re-prompt a child that isn't there any more, and + // these two ends already have records of their own. + state.finish_turn(&task_name, &end); + if !(matches!(end, TurnEnd::Failed(_)) && reported) { + push_turn_end_todo(&state.socket, &task_name, &end, None).await; + } + return; } - TurnEnd::Failed(e) => { - tracing::warn!(name = %task_name, error = %e, "subagent: turn failed"); + + match state.plan_after_turn(&task_name) { + Continuation::Stop(stop) => { + state.record_stop(&task_name, stop.clone()); + state.finish_turn(&task_name, &end); + write_stop_to_report(state.report_file(&task_name), &task_name, &stop).await; + push_turn_end_todo(&state.socket, &task_name, &end, Some(&stop)).await; + return; + } + Continuation::Continue { prompt: next } => { + // The name stays claimed across the gap — see + // `between_turns` for what a concurrent `start` would + // otherwise be able to do with it. + state.between_turns(&task_name); + match Claude::spawn(&config, &Attach::Resume(task_name.clone())) { + Ok(next_running) => { + state.track(&task_name, next_running.cancel_handle()); + state.note_event(&task_name); + running = next_running; + prompt = next; + searched = resume_searched.clone(); + } + Err(e) => { + let end = TurnEnd::Failed(format!( + "claude error: starting the next goal turn failed: {e}" + )); + log_turn_end(&task_name, &end); + state.finish_turn(&task_name, &end); + push_turn_end_todo(&state.socket, &task_name, &end, None).await; + return; + } + } + } } } - state.finish_turn(&task_name, &end); - let failed = matches!(end, TurnEnd::Failed(_)); - // The todo is how a turn's end reaches an agent that is no longer - // looking — so it is pushed for every end *except* the one the - // caller is being handed as a tool-call error right now. `settle` - // saying the verdict was delivered is what makes that certain: a - // `continue` whose grace had already run out gets `false` here and - // its todo, same as before. - let reported = settle(verdict.as_ref(), ResumeVerdict::Ended(end.clone())); - if !(failed && reported) { - push_turn_end_todo(&state.socket, &task_name, &end).await; - } }); Ok(format!("subagent `{name}` started")) } +/// Log a turn's end at the level its severity deserves: a completion is +/// unremarkable, the other two are not. +fn log_turn_end(name: &str, end: &TurnEnd) { + match end { + TurnEnd::Complete => {} + TurnEnd::Killed { signal } => { + tracing::warn!( + name = %name, + signal, + "subagent: turn killed — the child died on a signal, it did not finish" + ); + } + TurnEnd::Failed(e) => { + tracing::warn!(name = %name, error = %e, "subagent: turn failed"); + } + } +} + +/// Append the stop reason to the session's own report file, so the artifact +/// a parent already reads is where the run's ending is recorded too — rather +/// than the parent having to correlate a todo against a file. +/// +/// Best-effort and appended, never rewritten: the subagent wrote that file, +/// and this adds a line under what it wrote instead of taking a position on +/// the rest of it. A path this daemon can't write to is logged and dropped — +/// the todo still carries the same sentence, so nothing is only here. +async fn write_stop_to_report(path: Option, name: &str, stop: &StopReason) { + let Some(path) = path else { return }; + let line = format!("\n**Subagent `{name}` stopped:** {}\n", stop.sentence()); + let appended = tokio::fs::OpenOptions::new() + .create(true) + .append(true) + .open(&path) + .await; + let result = match appended { + Ok(mut file) => { + use tokio::io::AsyncWriteExt as _; + // `flush`, not just `write_all`: a `tokio::fs::File` buffers, and + // dropping one discards whatever hasn't been handed to the + // blocking pool — so without this the line is written to nothing + // and the failure is silent. A unit test caught exactly that. + match file.write_all(line.as_bytes()).await { + Ok(()) => file.flush().await, + Err(e) => Err(e), + } + } + Err(e) => Err(e), + }; + if let Err(e) = result { + tracing::warn!( + name, + path = %path.display(), + error = ?e, + "subagent: could not record the stop reason in the session's report file", + ); + } +} + +/// The goal contract appended to a `start`'s first prompt when a goal was +/// given. Spelled out to the subagent rather than left implicit: it is about +/// to be re-prompted by something it can't see, and the two tools that stop +/// that are the only way it has to say "done" or "stuck". +fn goal_briefing(name: &str, goal: &str, max_turns: u32) -> String { + format!( + "\n\nYou are the subagent session `{name}`.\n\nYour goal for this session: {goal}\n\nYou \ + have up to {max_turns} turns to reach it. When a turn of yours ends and you haven't \ + reported the goal reached, the harness starts another turn re-prompting you toward it. \ + Call the `goal_reached` tool once you've genuinely reached it, or `need_help` with what \ + is blocking you if you can't proceed — either one stops the re-prompting. Neither takes \ + a session name: the endpoint you call them on is yours alone, so they always apply to \ + this session and can't be aimed at another one. Running out of turns stops it too, with \ + the work left wherever it had got to." + ) +} + +/// The prompt a continuation turn opens with. It states the one fact the +/// subagent can't observe for itself — that its last turn ended without the +/// goal being reported reached — and says plainly that claiming the goal +/// isn't the same as reaching it, since being re-prompted is precisely the +/// pressure to claim it. +fn continuation_prompt(name: &str, goal: &str, turn: u32, max_turns: u32) -> String { + format!( + "Your previous turn ended and you have not reported the goal reached.\n\nYou are the \ + subagent session `{name}`.\n\nGoal: {goal}\n\nThis is turn {turn} of {max_turns}. Carry \ + on toward the goal. If you have in fact reached it, call `goal_reached`; if you are \ + blocked, call `need_help` with what is blocking you. Neither is a substitute for \ + the work: whoever spawned you reads what you actually changed, not what you claim about \ + it." + ) +} + +/// Record the subagent's own "I have reached the goal" signal and stop its +/// turn continuation. Called by the subagent, from inside its own turn, over +/// the signal route this daemon hands it (see `build_config`). +/// +/// **`name` is not a parameter of the tool.** It is whatever the route's +/// token resolved to (`State::session_for_signal_token`), so a subagent +/// records a stop against its own session because that is the only session +/// its URL can reach — not because it addressed the right one. +/// +/// **This verifies nothing**, and the answer it returns says so to the +/// subagent's face. It stops the loop and extends the done message; whether +/// the goal was actually reached is a question about the diff and the gate +/// output, which the parent reads for itself. +/// +/// `report_file` is the subagent saying where it wrote its report, which is +/// the only reason this daemon ever knows that path — see +/// `State::set_report_file`. +pub fn goal_reached( + state: &State, + name: &str, + msg: Option, + report_file: Option<&str>, +) -> String { + signal_stop(state, name, StopReason::GoalReached(msg), report_file); + format!( + "noted — `{name}`'s goal is recorded as reported reached, so this turn finishes and no \ + further goal turn is started. It is recorded as your claim, not as verification: whoever \ + spawned you still reads what you changed." + ) +} + +/// Record that the subagent can't proceed, and stop its turn continuation. +/// The blocking signal the run has otherwise no way to raise: without it a +/// stuck subagent would be re-prompted toward a goal it has already told +/// nobody it can't reach, until the turn cap. +/// +/// `msg` is required, unlike `goal_reached`'s — "I'm stuck" with no reason +/// gives the parent nothing to act on, and acting on it is the entire point. +/// +/// `name` comes from the route's token, exactly as in [`goal_reached`]. +pub fn need_help(state: &State, name: &str, msg: String, report_file: Option<&str>) -> String { + signal_stop(state, name, StopReason::NeedHelp(msg), report_file); + format!( + "noted — `{name}` is recorded as blocked, so this turn finishes and no further goal turn \ + is started. Write down what you have done so far where your brief told you to; whoever \ + spawned you sees the block in `status` and in this run's todo." + ) +} + +/// The half [`goal_reached`] and [`need_help`] share: remember where the +/// subagent says it wrote, and record the stop. +/// +/// It checks nothing, and has nothing left to check. `name` reached it by +/// being resolved from the route's own token, so "is this signal about the +/// session it claims" is answered before the request is dispatched at all — +/// a token that names no live session never reaches this function, it gets a +/// 404. What used to stand here was an `occupancy()` liveness check standing +/// in for identity, which two concurrently-running siblings could each +/// satisfy for the other's name. +fn signal_stop(state: &State, name: &str, stop: StopReason, report_file: Option<&str>) { + state.set_report_file(name, report_file); + state.record_stop(name, stop); +} + /// Report whether `name` is currently running — a zero-cost check that -/// never launches a process, unlike `continue`. Distinguishes five states: -/// running, starting (reserved, not yet a confirmed spawn — see `State`'s -/// doc), killed (its last turn died on a signal), idle (a session exists, -/// its last turn finished, nothing is in flight), and no such session at -/// all. +/// never launches a process, unlike `continue`. Distinguishes running, +/// starting (reserved, not yet a confirmed spawn — see `State`'s doc), +/// killed (its last turn died on a signal), each of the four ways a goal +/// run stops, idle (a session exists, its last turn finished, nothing is in +/// flight), and no such session at all. /// /// A *running* answer also carries how long since that turn last produced /// output, which is the part of this answer a caller can act on: "running" /// describes a wedged child and a busy one identically, and the age /// separates them (see `State`'s `last_event` doc). /// +/// A goal-continued session carries `turn N of M` alongside that, in every +/// state. With the age, it is what lets a caller tell *working* from +/// *wedged* from *out of turns* off one answer, without reaching for `ps` +/// or reading any file. +/// /// # Errors /// -/// An invalid name, or no session — running, killed or on disk — under -/// `name`. +/// An invalid name, or no session — running, killed, stopped or on disk — +/// under `name`. pub fn status(state: &State, name: &str, dir: Option<&str>) -> anyhow::Result { validate_name(name)?; // Read-only: an explicit `dir` here is a one-off "check this other // directory's session" per this fn's own doc, not a new remembered // default — `peek_dir` resolves the same way but never writes. let dir = state.peek_dir(name, dir); - let occupancy = state.occupancy(name); - let killed = state.killed_by(name); - let last_event_age = state.last_event_age(name); + let facts = StatusFacts { + occupancy: state.occupancy(name), + killed: state.killed_by(name), + last_event_age: state.last_event_age(name), + turns: state.turns(name), + stop: state.stop_reason(name), + ..StatusFacts::new(name) + }; // Nothing on disk to look for while something is tracked in memory — // those states answer on their own, and the store read is the only - // expensive part of this call. - let session_exists = if occupancy.is_none() && killed.is_none() { - let config = build_config(name, None, None, None, dir.as_deref()); - build_store(&config)?.find_by_title(name).is_some() - } else { - false - }; - describe_status(name, occupancy, killed, session_exists, last_event_age) + // expensive part of this call. A recorded stop reason counts: it is + // proof this daemon ran the session, which is what the lookup asks. + let session_exists = + if facts.occupancy.is_none() && facts.killed.is_none() && facts.stop.is_none() { + // No signal surface in this config: it exists only to resolve the + // session store, and rendering a subagent's MCP config for a + // read-only status check would be writing a file for nobody. + let config = build_config(name, None, None, None, dir.as_deref(), None); + build_store(&config)?.find_by_title(name).is_some() + } else { + false + }; + describe_status(&StatusFacts { + session_exists, + ..facts + }) } /// The liveness sentence appended to a *running* answer, and the whole point @@ -894,65 +1582,113 @@ fn describe_liveness(age: Option) -> String { Some(age) => format!( " Last event {}s ago — how long since this turn's claude process produced any output \ at all, whatever it was: a few seconds means it's working, an age that keeps \ - climbing into the minutes with no end-of-turn todo means it's wedged.", + climbing into the minutes means it's wedged. It resets at each turn's spawn, so on a \ + goal run it describes the turn in flight, not the run.", age.as_secs() ), } } -/// Render `status`'s answer from the facts it gathers. Split out from the -/// gathering so the killed-versus-idle distinction — and the liveness age on -/// a running turn — are exercisable without a real spawn, a real signal, a -/// real stream of events and a real on-disk claude session. +/// The progress sentence a goal-continued session carries in every state: +/// which turn of its budget it is on. Empty for a session with no goal, +/// which has no budget to be partway through. /// -/// A recorded kill outranks the on-disk session: the session file exists -/// either way, so "a session is there" is precisely the fact that cannot -/// tell the two apart. +/// On a *running* answer this is the turn in flight; on a stopped one it is +/// the turn it stopped on — the same number either way, since the counter +/// only advances when the loop decides to spend another turn. +fn describe_turns(turns: Option<(u32, u32)>) -> String { + match turns { + None => String::new(), + Some((turn, max_turns)) => format!(" Turn {turn} of {max_turns}."), + } +} + +/// Everything `status` gathers, as one value — seven separate parameters +/// read as noise at both the call site and the test sites, and every one of +/// them is a fact about the same session. +struct StatusFacts<'a> { + name: &'a str, + occupancy: Option, + killed: Option, + session_exists: bool, + last_event_age: Option, + turns: Option<(u32, u32)>, + stop: Option, +} + +impl<'a> StatusFacts<'a> { + /// The nothing-known baseline for `name`: no process, no records, no + /// session. Both the real gathering in `status` and the tests build on + /// it, so neither has to spell out the fields it isn't exercising. + fn new(name: &'a str) -> Self { + Self { + name, + occupancy: None, + killed: None, + session_exists: false, + last_event_age: None, + turns: None, + stop: None, + } + } +} + +/// Render `status`'s answer from the facts it gathers. Split out from the +/// gathering so the killed-versus-idle distinction — and the liveness age, +/// the turn counter and each stop reason — are exercisable without a real +/// spawn, a real signal, a real stream of events and a real on-disk claude +/// session. +/// +/// A recorded kill outranks both the stop reason and the on-disk session: +/// the session file exists either way and a stop reason may be left over +/// from the signal a subagent raised just before something killed it, so +/// neither can tell a killed turn from a finished one. The kill can. /// /// Every answer is self-contained — it names the one state the caller got /// and what to do next — because the tool description deliberately doesn't /// enumerate the state space (the operator's ruling on this surface: /// describe the tool, explain the state when returning it). Keep the split: /// a terser answer here has nowhere left to be explained from. -fn describe_status( - name: &str, - occupancy: Option, - killed: Option, - session_exists: bool, - last_event_age: Option, -) -> anyhow::Result { - match occupancy { +fn describe_status(facts: &StatusFacts<'_>) -> anyhow::Result { + let name = facts.name; + let turns = describe_turns(facts.turns); + match facts.occupancy { Some(true) => { return Ok(format!( "subagent `{name}` is running — its turn is still in flight, so there's nothing \ - to do but let it work: the daemon pushes a todo when the turn ends, or \ - `interrupt` it if you want it stopped early.{}", - describe_liveness(last_event_age) + to do but let it work: the daemon pushes a todo when the run ends, or \ + `interrupt` it if you want it stopped early.{turns}{}{}", + describe_liveness(facts.last_event_age), + describe_pending_signal(facts.stop.as_ref()), )); } Some(false) => { return Ok(format!( - "subagent `{name}` is starting — a `start`/`continue` has claimed the name but \ - its process isn't confirmed spawned yet, which is normally over in well under a \ - second: check again shortly rather than starting anything else under this name." + "subagent `{name}` is starting — the name is claimed but no process is confirmed \ + under it yet, either because a `start`/`continue` hasn't spawned one or because \ + a goal run is between turns. Normally over in well under a second: check again \ + shortly rather than starting anything else under this name.{turns}" )); } None => {} } - if let Some(signal) = killed { + if let Some(signal) = facts.killed { return Ok(format!( "subagent `{name}` was killed — its last turn died on {}, so its work stopped \ wherever it had got to rather than finishing. `continue` still resumes it, but \ whatever it was told to do is unfinished: check what it actually left behind before \ - trusting it.", + trusting it.{turns}", describe_signal(signal) )); } - if session_exists { + if let Some(stop) = &facts.stop { + return Ok(describe_stopped(name, stop, &turns)); + } + if facts.session_exists { Ok(format!( "subagent `{name}` is idle — its session exists, its last turn ended on its own \ rather than being cut off, and nothing is in flight: that turn's own todo says how \ - it went, and `continue` gives it another." + it went, and `continue` gives it another.{turns}" )) } else { anyhow::bail!( @@ -963,16 +1699,74 @@ fn describe_status( } } +/// The note a *running* answer carries when the subagent has already raised +/// a stop signal for the turn still in flight. Without it a parent polling +/// `status` would read plain "running" for the whole stretch between the +/// subagent saying it is blocked and its turn actually ending — the one +/// stretch where "it's working, leave it alone" is the wrong conclusion. +fn describe_pending_signal(stop: Option<&StopReason>) -> String { + match stop { + None | Some(StopReason::Done | StopReason::TurnCap { .. }) => String::new(), + Some(stop) => format!( + " It has already signalled how this run ends — {} — so this is its last turn.", + stop.sentence() + ), + } +} + +/// The answer for a session whose run has stopped, one per [`StopReason`]. +/// Each names the state, why the continuation stopped, and what `continue` +/// would do about it — and the `GoalReached` one is deliberately the least +/// reassuring of the four, because it is the one a caller is most likely to +/// read as "finished successfully" when it means "said so". +fn describe_stopped(name: &str, stop: &StopReason, turns: &str) -> String { + match stop { + StopReason::Done => format!( + "subagent `{name}` is idle — its session exists, its last turn ended on its own \ + rather than being cut off, and nothing is in flight: that turn's own todo says how \ + it went, and `continue` gives it another.{turns}" + ), + StopReason::GoalReached(msg) => format!( + "subagent `{name}` stopped: it reported its goal reached{}. Nothing is in flight and \ + no further goal turn will start.{turns} That report is the subagent's own claim, not \ + a verification of anything — read the diff and whatever gate the work was supposed \ + to pass before you treat the goal as met, exactly as you would a build report saying \ + the tests passed.", + msg.as_deref() + .map_or_else(String::new, |m| format!(": {m}")) + ), + StopReason::NeedHelp(msg) => format!( + "subagent `{name}` is BLOCKED and needs help: {msg}. It called `need_help`, which \ + stopped its goal continuation, and nothing is in flight — it stays blocked until you \ + answer it.{turns} `continue` is how you answer: give it what it asked for as the \ + next turn's prompt." + ), + StopReason::TurnCap { turns: spent } => format!( + "subagent `{name}` ran out of turns — the harness limit of {spent} was reached and it \ + never reported its goal reached, so the work stopped wherever it had got to rather \ + than finishing.{turns} Check what it actually left behind; `continue` gives it a \ + fresh allowance if carrying on is worth it." + ), + } +} + /// Signal `name`'s running process — `force` picks SIGKILL over SIGINT (see /// `hive_claude::Cancel::cancel`). Refuses a name with nothing running: no -/// entry at all, or one still in the brief `reserve`d-but-not-yet-spawned -/// window (nothing to signal yet — the reservation is put back so a -/// concurrent `start`/`continue` for the same name still gets refused). +/// entry at all, or one still in the brief not-yet-spawned window (nothing +/// to signal yet — the reservation is put back so a concurrent +/// `start`/`continue` for the same name still gets refused). +/// +/// This stops a goal run, not just the turn in it: the signalled child ends +/// as `TurnEnd::Killed`, which the continuation loop treats as the end of +/// the whole run rather than something to re-prompt past. That falls out of +/// there being no child left to continue, and it is the answer you want — +/// `interrupt` would be useless if the harness immediately started turn +/// three of five. /// /// # Errors /// -/// An invalid name, nothing tracked under `name`, or `name` is still -/// starting (reserved, not yet a confirmed spawn). +/// An invalid name, nothing tracked under `name`, or `name` has no confirmed +/// process right now (a spawn in flight, or a goal run between turns). pub fn interrupt(state: &State, name: &str, force: bool) -> anyhow::Result { validate_name(name)?; let mut running = state.running.lock().unwrap_or_else(PoisonError::into_inner); @@ -997,11 +1791,16 @@ pub fn interrupt(state: &State, name: &str, force: bool) -> anyhow::Result, +) { let req = hive_agent_sock::Request::UpsertTodo { subsystem: "subagent".to_owned(), key: Some(name.to_owned()), - summary: turn_end_summary(name, end), + summary: turn_end_summary(name, end, stop), source: None, reopen_if_acked: false, }; @@ -1010,12 +1809,20 @@ async fn push_turn_end_todo(socket: &std::path::Path, name: &str, end: &TurnEnd) } } -/// The todo text for a turn that has ended. A killed turn deliberately does +/// The todo text for a run that has ended. A killed turn deliberately does /// not use the "finished" wording the other two share: this todo is the only /// thing the owning agent is shown without asking, so it has to read as the /// interruption it is rather than as one more completed subagent. -fn turn_end_summary(name: &str, end: &TurnEnd) -> String { - match end { +/// +/// A `stop` **extends** that message, it never replaces it. Both halves are +/// load-bearing and neither substitutes for the other: the turn's own end is +/// what the daemon observed, the stop reason is why the run went no further +/// — and for `GoalReached` that second half is a claim, which would read as +/// a verdict if it were allowed to stand where the observed end belongs. +/// `StopReason::Done` adds nothing, since "there was no goal" is exactly +/// what the unextended message already describes. +fn turn_end_summary(name: &str, end: &TurnEnd, stop: Option<&StopReason>) -> String { + let base = match end { TurnEnd::Complete => format!("subagent `{name}` finished: turn complete"), TurnEnd::Failed(e) => format!("subagent `{name}` finished: {e}"), TurnEnd::Killed { signal } => format!( @@ -1024,6 +1831,10 @@ fn turn_end_summary(name: &str, end: &TurnEnd) -> String { resumes the session if you want it carried on.", describe_signal(*signal) ), + }; + match stop { + None | Some(StopReason::Done) => base, + Some(stop) => format!("{base} — and the run stopped there: {}", stop.sentence()), } } @@ -1036,9 +1847,42 @@ mod tests { // half of `State` directly — the actual TOCTOU-closure logic — rather // than the full start/spawn path. + /// A stand-in for the signal route a real daemon would hand its + /// subagents — the *prefix*, as `State::new` takes it. Nothing in these + /// tests dials it: what `State` does with it is append a minted token and + /// carry the result into `build_config`, both asserted on directly. + fn signal_url() -> String { + "http://127.0.0.1:1/signal/mcp".to_owned() + } + + /// The token out of a minted URL — what the route would have parsed out + /// of the path before resolving it. + fn token_of(url: &str) -> String { + url.rsplit('/') + .next() + .expect("a minted URL always has a last segment") + .to_owned() + } + + /// A `StartRequest` with only the fields a test cares about set — the + /// other six are the same "nothing asked for" every time. + fn start_request(name: &str) -> StartRequest { + StartRequest { + name: name.to_owned(), + model: None, + effort: None, + prompt_file: "/tmp/prompt.md".to_owned(), + trigger: "trigger".to_owned(), + dir: None, + goal: None, + max_turns: None, + report_file: None, + } + } + #[test] fn reserve_is_exclusive_for_the_same_name() { - let state = State::new(PathBuf::from("/dev/null")); + let state = State::new(PathBuf::from("/dev/null"), signal_url()); assert!(state.reserve("dup"), "first reservation should succeed"); assert!( !state.reserve("dup"), @@ -1049,7 +1893,7 @@ mod tests { #[test] fn reserve_does_not_cross_block_different_names() { - let state = State::new(PathBuf::from("/dev/null")); + let state = State::new(PathBuf::from("/dev/null"), signal_url()); assert!(state.reserve("a")); assert!( state.reserve("b"), @@ -1059,7 +1903,7 @@ mod tests { #[test] fn release_reservation_frees_the_name_for_reuse() { - let state = State::new(PathBuf::from("/dev/null")); + let state = State::new(PathBuf::from("/dev/null"), signal_url()); assert!(state.reserve("n")); state.release_reservation("n"); assert!( @@ -1070,7 +1914,7 @@ mod tests { #[test] fn occupancy_reflects_the_reserved_but_not_running_state() { - let state = State::new(PathBuf::from("/dev/null")); + let state = State::new(PathBuf::from("/dev/null"), signal_url()); assert_eq!(state.occupancy("never-reserved"), None); state.reserve("n"); assert_eq!( @@ -1109,14 +1953,14 @@ mod tests { #[test] fn build_config_only_appends_system_prompt_when_given() { - let with = build_config("n", None, None, Some("/tmp/p.md"), None); + let with = build_config("n", None, None, Some("/tmp/p.md"), None, None); assert!( with.extra_args .contains(&"--append-system-prompt-file".to_owned()) ); assert!(with.extra_args.contains(&"/tmp/p.md".to_owned())); - let without = build_config("n", None, None, None, None); + let without = build_config("n", None, None, None, None, None); assert!( !without .extra_args @@ -1126,7 +1970,7 @@ mod tests { #[test] fn resolve_dir_remembers_an_explicit_dir_and_falls_back_to_it_when_omitted() { - let state = State::new(PathBuf::from("/dev/null")); + let state = State::new(PathBuf::from("/dev/null"), signal_url()); assert_eq!( state.resolve_dir("n", None), None, @@ -1156,7 +2000,7 @@ mod tests { #[test] fn resolve_dir_does_not_cross_names() { - let state = State::new(PathBuf::from("/dev/null")); + let state = State::new(PathBuf::from("/dev/null"), signal_url()); state.resolve_dir("a", Some("/tmp/a")); assert_eq!( state.resolve_dir("b", None), @@ -1167,7 +2011,7 @@ mod tests { #[test] fn peek_dir_resolves_like_resolve_dir_but_never_writes() { - let state = State::new(PathBuf::from("/dev/null")); + let state = State::new(PathBuf::from("/dev/null"), signal_url()); state.resolve_dir("n", Some("/tmp/remembered")); assert_eq!( state.peek_dir("n", Some("/tmp/one-off")), @@ -1189,7 +2033,7 @@ mod tests { // to. Reordering `reserve` before `resolve_dir` in `start` closes it // — a name that's already reserved must never reach `resolve_dir` at // all, so `dirs` stays exactly as a caller left it. - let state = Arc::new(State::new(PathBuf::from("/dev/null"))); + let state = Arc::new(State::new(PathBuf::from("/dev/null"), signal_url())); state.resolve_dir("dup", Some("/tmp/original")); assert!( state.reserve("dup"), @@ -1197,12 +2041,10 @@ mod tests { ); let result = start( &state, - "dup", - None, - None, - "/tmp/prompt.md", - "trigger".to_owned(), - Some("/tmp/rejected"), + StartRequest { + dir: Some("/tmp/rejected".to_owned()), + ..start_request("dup") + }, ); assert!( result.is_err(), @@ -1251,9 +2093,17 @@ mod tests { fn status_reports_killed_not_idle_for_a_signalled_session() { // Both sessions exist on disk and neither is running: the only thing // that can tell them apart is the recorded signal. - let killed = - describe_status("n", None, Some(libc::SIGKILL), true, None).expect("killed status"); - let idle = describe_status("n", None, None, true, None).expect("idle status"); + let killed = describe_status(&StatusFacts { + killed: Some(libc::SIGKILL), + session_exists: true, + ..StatusFacts::new("n") + }) + .expect("killed status"); + let idle = describe_status(&StatusFacts { + session_exists: true, + ..StatusFacts::new("n") + }) + .expect("idle status"); assert!( killed.contains("killed") && killed.contains("SIGKILL (signal 9)"), "a killed session must say so, and name the signal: {killed}" @@ -1274,30 +2124,45 @@ mod tests { // The tool description no longer lists the states, so each answer // has to carry its own explanation — checked one state at a time, // which is all a caller ever gets back. - let running = describe_status("n", Some(true), None, false, None).expect("running status"); + let running = describe_status(&StatusFacts { + occupancy: Some(true), + ..StatusFacts::new("n") + }) + .expect("running status"); assert!( running.contains("running") && running.contains("`interrupt`"), "a running answer must say the turn is in flight and how to stop it: {running}" ); - let starting = - describe_status("n", Some(false), None, false, None).expect("starting status"); + let starting = describe_status(&StatusFacts { + occupancy: Some(false), + ..StatusFacts::new("n") + }) + .expect("starting status"); assert!( starting.contains("starting") && starting.contains("check again"), "a starting answer must say the spawn isn't confirmed yet and to retry: {starting}" ); - let idle = describe_status("n", None, None, true, None).expect("idle status"); + let idle = describe_status(&StatusFacts { + session_exists: true, + ..StatusFacts::new("n") + }) + .expect("idle status"); assert!( idle.contains("idle") && idle.contains("`continue`"), "an idle answer must say the last turn ended on its own and how to give it another: \ {idle}" ); - let killed = - describe_status("n", None, Some(libc::SIGKILL), true, None).expect("killed status"); + let killed = describe_status(&StatusFacts { + killed: Some(libc::SIGKILL), + session_exists: true, + ..StatusFacts::new("n") + }) + .expect("killed status"); assert!( killed.contains("killed") && killed.contains("`continue`"), "a killed answer must name the kill and say resuming is still possible: {killed}" ); - let missing = describe_status("n", None, None, false, None) + let missing = describe_status(&StatusFacts::new("n")) .expect_err("nothing tracked and nothing on disk is an error, not a state"); assert!( missing.to_string().contains("`start`"), @@ -1309,7 +2174,7 @@ mod tests { fn a_killed_turn_and_a_clean_one_do_not_land_in_the_same_state() { // The whole path a real turn takes, minus the process: what the // driver returned -> what the daemon records -> what `status` says. - let state = State::new(PathBuf::from("/dev/null")); + let state = State::new(PathBuf::from("/dev/null"), signal_url()); for (name, outcome) in [ ("gone", Err(signalled_exit(libc::SIGKILL))), ("done", Ok(())), @@ -1325,17 +2190,25 @@ mod tests { assert_eq!(state.killed_by("gone"), Some(9)); assert_eq!(state.killed_by("done"), None); - let gone = - describe_status("gone", None, state.killed_by("gone"), true, None).expect("status"); - let done = - describe_status("done", None, state.killed_by("done"), true, None).expect("status"); + let gone = describe_status(&StatusFacts { + killed: state.killed_by("gone"), + session_exists: true, + ..StatusFacts::new("gone") + }) + .expect("status"); + let done = describe_status(&StatusFacts { + killed: state.killed_by("done"), + session_exists: true, + ..StatusFacts::new("done") + }) + .expect("status"); assert!(gone.contains("killed"), "{gone}"); assert!(done.contains("idle"), "{done}"); } #[test] fn a_new_turn_clears_the_previous_turn_s_kill() { - let state = State::new(PathBuf::from("/dev/null")); + let state = State::new(PathBuf::from("/dev/null"), signal_url()); state.finish_turn("n", &TurnEnd::Killed { signal: 9 }); assert_eq!(state.killed_by("n"), Some(9)); // What `spawn_and_track` does once the next turn is confirmed @@ -1352,8 +2225,8 @@ mod tests { #[test] fn the_killed_todo_does_not_read_like_a_completion() { - let complete = turn_end_summary("n", &TurnEnd::Complete); - let killed = turn_end_summary("n", &TurnEnd::Killed { signal: 9 }); + let complete = turn_end_summary("n", &TurnEnd::Complete, None); + let killed = turn_end_summary("n", &TurnEnd::Killed { signal: 9 }, None); assert_eq!( complete, "subagent `n` finished: turn complete", "the ordinary completion todo is unchanged" @@ -1394,10 +2267,10 @@ mod tests { #[test] fn build_config_sets_cwd_only_when_a_dir_is_given() { - let with = build_config("n", None, None, None, Some("/tmp/some-worktree")); + let with = build_config("n", None, None, None, Some("/tmp/some-worktree"), None); assert_eq!(with.cwd, Some(PathBuf::from("/tmp/some-worktree"))); - let without = build_config("n", None, None, None, None); + let without = build_config("n", None, None, None, None, None); assert_eq!(without.cwd, None); } @@ -1409,6 +2282,7 @@ mod tests { Some("high".to_owned()), None, None, + None, ); assert_eq!(config.model, Some("opus".to_owned())); assert_eq!(config.effort, Some("high".to_owned())); @@ -1416,7 +2290,7 @@ mod tests { #[test] fn an_event_starts_the_liveness_clock_and_the_turn_ending_stops_it() { - let state = State::new(PathBuf::from("/dev/null")); + let state = State::new(PathBuf::from("/dev/null"), signal_url()); assert_eq!( state.last_event_age("n"), None, @@ -1440,7 +2314,7 @@ mod tests { fn a_later_event_resets_the_age_rather_than_letting_it_climb() { // The distinction the whole record exists for: a child still // producing output must not accumulate the age of a wedged one. - let state = State::new(PathBuf::from("/dev/null")); + let state = State::new(PathBuf::from("/dev/null"), signal_url()); state.note_event("n"); std::thread::sleep(Duration::from_millis(20)); let before = state.last_event_age("n").expect("clock started"); @@ -1454,7 +2328,7 @@ mod tests { #[test] fn the_liveness_clock_does_not_cross_names() { - let state = State::new(PathBuf::from("/dev/null")); + let state = State::new(PathBuf::from("/dev/null"), signal_url()); state.note_event("a"); assert_eq!( state.last_event_age("b"), @@ -1471,7 +2345,7 @@ mod tests { use hive_claude::Sink as _; fn fresh() -> (Arc, LivenessSink) { - let state = Arc::new(State::new(PathBuf::from("/dev/null"))); + let state = Arc::new(State::new(PathBuf::from("/dev/null"), signal_url())); let sink = LivenessSink { state: Arc::clone(&state), name: "n".to_owned(), @@ -1504,15 +2378,23 @@ mod tests { #[test] fn a_running_status_reports_the_age_and_an_idle_one_does_not() { - let running = describe_status("n", Some(true), None, false, Some(Duration::from_secs(4))) - .expect("running status"); + let running = describe_status(&StatusFacts { + occupancy: Some(true), + last_event_age: Some(Duration::from_secs(4)), + ..StatusFacts::new("n") + }) + .expect("running status"); assert!( running.contains("Last event 4s ago"), "a running answer must carry the age — the one part of it a caller can act on: \ {running}" ); - let wedged = describe_status("n", Some(true), None, false, Some(Duration::from_mins(15))) - .expect("running status"); + let wedged = describe_status(&StatusFacts { + occupancy: Some(true), + last_event_age: Some(Duration::from_mins(15)), + ..StatusFacts::new("n") + }) + .expect("running status"); assert!( wedged.contains("Last event 900s ago"), "the wedged case is the one this exists for: {wedged}" @@ -1523,7 +2405,11 @@ mod tests { the bug this reports" ); - let idle = describe_status("n", None, None, true, None).expect("idle status"); + let idle = describe_status(&StatusFacts { + session_exists: true, + ..StatusFacts::new("n") + }) + .expect("idle status"); assert!( !idle.contains("Last event"), "an idle session has no in-flight turn whose progress an age would describe: {idle}" @@ -1535,7 +2421,11 @@ mod tests { // `status` reads `running` and the clock under separate locks, so a // turn can finish between the two. The answer drops the age rather // than inventing one. - let raced = describe_status("n", Some(true), None, false, None).expect("running status"); + let raced = describe_status(&StatusFacts { + occupancy: Some(true), + ..StatusFacts::new("n") + }) + .expect("running status"); assert!( raced.contains("is running") && !raced.contains("Last event"), "a missing age must cost the sentence, not the answer: {raced}" @@ -1598,7 +2488,7 @@ mod tests { // re-deriving them: the location reported must be the one actually // searched, so it can't drift from `build_store`/the driver. let cwd = std::env::current_dir().expect("a cwd"); - let config = build_config("n", None, None, None, Some(&cwd.to_string_lossy())); + let config = build_config("n", None, None, None, Some(&cwd.to_string_lossy()), None); let Some(located) = searched_location(&config) else { // No `HOME` in this environment — nothing to compare against. return; @@ -1747,7 +2637,7 @@ mod tests { fn fresh() -> (oneshot::Receiver, LivenessSink) { let (tx, rx) = oneshot::channel(); let sink = LivenessSink { - state: Arc::new(State::new(PathBuf::from("/dev/null"))), + state: Arc::new(State::new(PathBuf::from("/dev/null"), signal_url())), name: "n".to_owned(), verdict: Some(Arc::new(Mutex::new(Some(tx)))), }; @@ -1786,7 +2676,525 @@ mod tests { // claude's own default (`high` on most models) — this daemon picks // `medium` itself, a deliberate cost-conscious choice for subagent // work. - let config = build_config("n", None, None, None, None); + let config = build_config("n", None, None, None, None, None); assert_eq!(config.effort, Some("medium".to_owned())); } + + // ---- turn continuation ------------------------------------------------- + + /// A state with `name` mid-run against `goal`, as `start` would have left + /// it: reserved, goal registered, on turn one of `max_turns`. + fn mid_run(name: &str, goal: &str, max_turns: u32) -> State { + let state = State::new(PathBuf::from("/dev/null"), signal_url()); + state.reserve(name); + state.set_goal(name, Some(goal.to_owned()), max_turns); + state + } + + #[test] + fn a_session_with_no_goal_stops_after_one_turn() { + // The pre-continuation shape, and the reason `goal` is what switches + // the loop on rather than a separate flag: nothing to continue + // toward is the same fact as nothing to continue. + let state = mid_run("n", "unused", 5); + state.set_goal("n", None, 5); + assert!( + matches!( + state.plan_after_turn("n"), + Continuation::Stop(StopReason::Done) + ), + "without a goal the first completed turn ends the run" + ); + assert_eq!( + state.turns("n"), + None, + "and there is no turn budget to report being partway through" + ); + } + + #[test] + fn a_goal_keeps_spending_turns_until_the_cap_and_then_stops() { + let state = mid_run("n", "make the gate pass", 3); + assert_eq!(state.turns("n"), Some((1, 3)), "the first turn is 1 of 3"); + + for expected in [2, 3] { + let Continuation::Continue { prompt } = state.plan_after_turn("n") else { + panic!("turn {expected} of 3 must still be spent"); + }; + assert!( + prompt.contains("make the gate pass"), + "the re-prompt continues toward the goal verbatim: {prompt}" + ); + assert!( + prompt.contains("have not reported the goal reached"), + "and says the one thing the subagent can't observe for itself: {prompt}" + ); + assert_eq!(state.turns("n"), Some((expected, 3))); + } + + assert_eq!( + state.plan_after_turn("n"), + Continuation::Stop(StopReason::TurnCap { turns: 3 }), + "the cap is the number of turns run, not one more" + ); + assert_eq!( + state.turns("n"), + Some((3, 3)), + "a capped run must not advance past its own cap" + ); + } + + #[test] + fn the_default_cap_is_five_turns() { + // The number is the feature's own, not a value tuned here — pinned so + // a later edit to `DEFAULT_MAX_TURNS` has to be deliberate. + assert_eq!(DEFAULT_MAX_TURNS, 5); + } + + #[test] + fn both_signals_stop_the_continuation_before_the_cap_is_reached() { + // The issue's own words: "both stop goal continues". Turn one of + // five, so only the signal can be what stopped it. + for stop in [ + StopReason::GoalReached(Some("wrote the fix".to_owned())), + StopReason::NeedHelp("no credential for the registry".to_owned()), + ] { + let state = mid_run("n", "a goal", 5); + state.record_stop("n", stop.clone()); + assert_eq!( + state.plan_after_turn("n"), + Continuation::Stop(stop.clone()), + "{stop:?} must end the run with turns still on the clock" + ); + assert_eq!( + state.turns("n"), + Some((1, 5)), + "and must not have spent one on the way out" + ); + } + } + + #[test] + fn a_signal_on_the_last_allowed_turn_outranks_the_cap() { + // Both are true at once, and which one is reported is the difference + // between "it says it finished" and "it ran out of road". + let state = mid_run("n", "a goal", 1); + state.record_stop("n", StopReason::GoalReached(None)); + assert_eq!( + state.plan_after_turn("n"), + Continuation::Stop(StopReason::GoalReached(None)) + ); + } + + #[test] + fn a_subagent_cannot_signal_a_different_session() { + // The requirement itself, and the reason it's a test: two siblings + // running concurrently, each holding exactly one signal URL. Half the + // answer is in `mcp.rs` — neither tool has a `name` argument to put a + // sibling's name in (`the_signal_tools_take_no_session_name` pins + // that). The other half is here: whether the only identity a subagent + // *does* hold, its token, can be made to resolve to anyone else. + let state = State::new(PathBuf::from("/dev/null"), signal_url()); + state.reserve("alpha"); + state.reserve("beta"); + let alpha = token_of(&state.mint_signal_url("alpha")); + let beta = token_of(&state.mint_signal_url("beta")); + assert_ne!(alpha, beta, "two sessions must not share a token"); + + assert_eq!( + state.session_for_signal_token(&alpha).as_deref(), + Some("alpha") + ); + assert_eq!( + state.session_for_signal_token(&beta).as_deref(), + Some("beta") + ); + + // `alpha` signals the only way it can: on its own endpoint, with the + // session resolved from the token rather than supplied by the caller. + let resolved = state + .session_for_signal_token(&alpha) + .expect("alpha's own route resolves"); + need_help(&state, &resolved, "no credential".to_owned(), None); + assert_eq!( + state.stop_reason("alpha"), + Some(StopReason::NeedHelp("no credential".to_owned())) + ); + assert_eq!( + state.stop_reason("beta"), + None, + "a sibling's run must be untouched — there is no route `alpha` holds that reaches it" + ); + + // A subagent does know its siblings' *names* (a brief can mention + // them) — and a name is not a token, which is the whole point. + assert_eq!(state.session_for_signal_token("beta"), None); + assert_eq!(state.session_for_signal_token(&format!("{alpha}0")), None); + } + + #[test] + fn a_token_stops_resolving_once_its_run_is_over() { + // The expiry half of "unknown or expired token ⇒ 404": a finished + // run's config file still names its URL, and that URL must be dead. + let state = State::new(PathBuf::from("/dev/null"), signal_url()); + state.reserve("n"); + let token = token_of(&state.mint_signal_url("n")); + state.finish_turn("n", &TurnEnd::Complete); + assert_eq!( + state.session_for_signal_token(&token), + None, + "the run ended, so the route it was issued must resolve to nothing" + ); + + // Same for a call that never reached a spawn at all. + state.reserve("n"); + let unspawned = token_of(&state.mint_signal_url("n")); + state.release_reservation("n"); + assert_eq!(state.session_for_signal_token(&unspawned), None); + } + + #[test] + fn a_re_minted_url_retires_the_previous_one() { + // A `continue` mints the resumed run its own token; the run before it + // is over, so the URL that run was handed must not still work. + let state = State::new(PathBuf::from("/dev/null"), signal_url()); + let first = token_of(&state.mint_signal_url("n")); + let second = token_of(&state.mint_signal_url("n")); + assert_eq!(state.session_for_signal_token(&first), None); + assert_eq!( + state.session_for_signal_token(&second).as_deref(), + Some("n") + ); + } + + #[test] + fn need_help_is_a_state_a_parent_can_see_without_reading_anything() { + // Requirement in full: it stops the session *and* shows up in + // `status` as its own state, distinct from idle and from killed. + let blocked = describe_status(&StatusFacts { + stop: Some(StopReason::NeedHelp( + "the brief contradicts the code".to_owned(), + )), + turns: Some((2, 5)), + ..StatusFacts::new("n") + }) + .expect("a blocked session is a state, not an error"); + assert!( + blocked.contains("BLOCKED") && blocked.contains("the brief contradicts the code"), + "the block and its reason must both be in the answer: {blocked}" + ); + assert!( + blocked.contains("Turn 2 of 5"), + "with the progress that says how far it got: {blocked}" + ); + assert!( + !blocked.contains("idle"), + "a blocked subagent must not also read as idle: {blocked}" + ); + } + + #[test] + fn a_reported_goal_never_reads_as_a_verified_one() { + // The failure this is built against: `goal_reached` is self-reported + // by a subagent that has just been told it hasn't reached the goal. + // Every surface that renders it has to say so. + let status = describe_status(&StatusFacts { + stop: Some(StopReason::GoalReached(Some( + "refactored the parser".to_owned(), + ))), + ..StatusFacts::new("n") + }) + .expect("status"); + assert!( + status.contains("refactored the parser") && status.contains("claim"), + "status must carry both the report and the fact it's only a report: {status}" + ); + let todo = turn_end_summary( + "n", + &TurnEnd::Complete, + Some(&StopReason::GoalReached(Some( + "refactored the parser".to_owned(), + ))), + ); + assert!( + todo.contains("self-reported, not verified"), + "and so must the todo, which is the half a parent reads unprompted: {todo}" + ); + } + + #[test] + fn a_stop_reason_extends_the_done_message_rather_than_replacing_it() { + // "Extend", not "replace": the observed end of the turn and the + // reason the run stopped are different facts, and dropping the first + // would let a self-reported claim stand where an observation was. + let plain = turn_end_summary("n", &TurnEnd::Complete, None); + for stop in [ + StopReason::GoalReached(None), + StopReason::NeedHelp("blocked".to_owned()), + StopReason::TurnCap { turns: 5 }, + ] { + let extended = turn_end_summary("n", &TurnEnd::Complete, Some(&stop)); + assert!( + extended.starts_with(&plain), + "{stop:?} must extend the done message, not rewrite it: {extended}" + ); + assert!( + extended.len() > plain.len(), + "{stop:?} must actually add something: {extended}" + ); + } + assert_eq!( + turn_end_summary("n", &TurnEnd::Complete, Some(&StopReason::Done)), + plain, + "`Done` has nothing to add — the unextended message already says exactly that" + ); + } + + #[test] + fn the_turn_cap_todo_says_the_harness_limit_was_what_stopped_it() { + // Not a silent stop: the one notification a parent gets unprompted + // has to distinguish "it finished" from "we stopped asking". + let todo = turn_end_summary( + "n", + &TurnEnd::Complete, + Some(&StopReason::TurnCap { turns: 5 }), + ); + assert!( + todo.contains("harness turn limit was reached (5 turns)"), + "the todo must name the limit as the cause: {todo}" + ); + assert!( + todo.contains("without the goal ever being reported reached"), + "and say what that means for the work: {todo}" + ); + } + + #[test] + fn status_reports_the_turn_counter_in_every_state_a_goal_run_reaches() { + for occupancy in [Some(true), Some(false), None] { + let answer = describe_status(&StatusFacts { + occupancy, + session_exists: true, + turns: Some((3, 5)), + ..StatusFacts::new("n") + }) + .expect("status"); + assert!( + answer.contains("Turn 3 of 5"), + "progress is the point of the counter — it can't be absent from {occupancy:?}: \ + {answer}" + ); + } + let goalless = describe_status(&StatusFacts { + occupancy: Some(true), + ..StatusFacts::new("n") + }) + .expect("status"); + assert!( + !goalless.contains("Turn "), + "a session with no goal has no budget to be partway through: {goalless}" + ); + } + + #[test] + fn a_running_turn_that_has_already_signalled_says_so() { + // The window a parent would otherwise misread: the subagent has said + // it is blocked, its turn hasn't ended yet, and plain "running" would + // tell the parent to leave it alone. + let answer = describe_status(&StatusFacts { + occupancy: Some(true), + stop: Some(StopReason::NeedHelp("no credential".to_owned())), + ..StatusFacts::new("n") + }) + .expect("status"); + assert!( + answer.contains("is running") && answer.contains("no credential"), + "both facts are true at once and both have to be in the answer: {answer}" + ); + let quiet = describe_status(&StatusFacts { + occupancy: Some(true), + stop: Some(StopReason::Done), + ..StatusFacts::new("n") + }) + .expect("status"); + assert!( + !quiet.contains("already signalled"), + "`Done` is not a signal the subagent raised — nothing to announce: {quiet}" + ); + } + + #[test] + fn a_kill_outranks_a_stop_reason_the_subagent_had_already_raised() { + // Both records can be set at once — a subagent calls `goal_reached` + // and something SIGKILLs it before the turn ends. The observed kill + // is the one that can't be a claim. + let answer = describe_status(&StatusFacts { + killed: Some(libc::SIGKILL), + stop: Some(StopReason::GoalReached(None)), + session_exists: true, + ..StatusFacts::new("n") + }) + .expect("status"); + assert!( + answer.contains("was killed"), + "the kill is what happened: {answer}" + ); + assert!( + !answer.contains("goal reached"), + "a claim made just before being killed must not be the headline: {answer}" + ); + } + + #[test] + fn a_continue_gives_a_capped_session_its_allowance_back() { + // `TurnCap` stops the loop, it doesn't retire the session — and the + // parent spending a turn on purpose is not what the cap bounds. + let state = mid_run("n", "a goal", 2); + state.record_stop("n", StopReason::TurnCap { turns: 2 }); + state.clear_stop("n"); + state.restart_turns("n"); + assert_eq!(state.turns("n"), Some((1, 2))); + assert_eq!(state.stop_reason("n"), None); + assert!( + matches!(state.plan_after_turn("n"), Continuation::Continue { .. }), + "with the allowance back, the loop has a turn to spend again" + ); + } + + #[test] + fn a_fresh_start_inherits_nothing_from_the_run_before_it() { + // `start` archives the prior session precisely so this is a fresh + // start; a leftover stop reason or report path would make the new run + // report the old one's ending, into the old one's file. + let state = Arc::new(State::new(PathBuf::from("/dev/null"), signal_url())); + state.record_stop("n", StopReason::NeedHelp("old block".to_owned())); + state.set_report_file("n", Some("/tmp/old-report.md")); + state.reserve("n"); + let refused = start(&state, start_request("n")); + assert!(refused.is_err(), "the reserved name is refused as before"); + + // The same clearing `start` does once it owns the name. + state.clear_stop("n"); + state.clear_report_file("n"); + assert_eq!(state.stop_reason("n"), None); + assert_eq!(state.report_file("n"), None); + } + + #[test] + fn the_report_path_is_taken_from_the_session_and_never_guessed() { + // Both halves of where it can come from: the brief `start` carried, + // and the subagent saying where it actually wrote. A signal that + // names no path leaves the remembered one alone rather than erasing + // it. + let state = State::new(PathBuf::from("/dev/null"), signal_url()); + assert_eq!( + state.report_file("n"), + None, + "a session nobody told about a report file has none — nothing is inferred" + ); + state.set_report_file("n", Some("/tmp/brief-said.md")); + goal_reached(&state, "n", None, None); + assert_eq!( + state.report_file("n"), + Some(PathBuf::from("/tmp/brief-said.md")), + "a signal with no path must not erase what the brief named" + ); + goal_reached(&state, "n", None, Some("/tmp/actually-wrote.md")); + assert_eq!( + state.report_file("n"), + Some(PathBuf::from("/tmp/actually-wrote.md")), + "and the subagent saying where it wrote is what wins" + ); + } + + #[tokio::test] + async fn the_stop_reason_is_appended_to_the_report_file_the_session_named() { + let dir = std::env::temp_dir().join(format!("hive-subagent-report-{}", std::process::id())); + std::fs::create_dir_all(&dir).expect("scratch dir"); + let path = dir.join("report.md"); + std::fs::write(&path, "# what the subagent wrote\n").expect("seed the report"); + + write_stop_to_report(Some(path.clone()), "n", &StopReason::TurnCap { turns: 5 }).await; + + let body = std::fs::read_to_string(&path).expect("report still readable"); + assert!( + body.starts_with("# what the subagent wrote\n"), + "appended, never rewritten — the subagent wrote that file: {body}" + ); + assert!( + body.contains("harness turn limit was reached (5 turns)"), + "and the stop reason lands in the artifact a parent already reads: {body}" + ); + std::fs::remove_dir_all(&dir).ok(); + } + + #[tokio::test] + async fn an_unwritable_report_path_costs_the_line_and_nothing_else() { + // Best-effort by design: the same sentence is in the todo, so a path + // this daemon can't write is a missing convenience, not a lost fact. + write_stop_to_report( + Some(PathBuf::from("/proc/definitely/not/writable/report.md")), + "n", + &StopReason::Done, + ) + .await; + write_stop_to_report(None, "n", &StopReason::Done).await; + } + + #[test] + fn a_goal_briefing_tells_the_subagent_what_it_cannot_otherwise_know() { + let briefing = goal_briefing("batch-1", "get the gate to pass", 5); + assert!( + briefing.contains("get the gate to pass") && briefing.contains("up to 5 turns"), + "the goal and the budget both have to reach the subagent: {briefing}" + ); + assert!( + briefing.contains("goal_reached") && briefing.contains("need_help"), + "as do the two ways it has to stop the re-prompting: {briefing}" + ); + assert!( + briefing.contains("batch-1"), + "and its own name, which is what those tools are called with: {briefing}" + ); + } + + #[test] + fn a_signal_url_reaches_the_subagent_and_a_status_check_renders_no_config() { + // `turn` actually writes an --mcp-config file (crate::mcp_config::build), + // which resolves `hive_agent_sock::paths::harness_dir` — normally the + // container's injected `HYPERHIVE_HARNESS_DIR`, unset in a plain `cargo + // test` sandbox. Point it at a scratch dir for just this test rather + // than depending on ambient environment; only this test's call path + // reads the var, so there's no cross-test race to sequence around + // (contrast `otel_attrs_append_ambient_value_or_stand_alone` above). + let dir = std::env::temp_dir().join(format!( + "hive-subagent-mcp-test-harness-dir-{}", + std::process::id() + )); + // SAFETY: test-only env mutation; no other test reads this var. + unsafe { + std::env::set_var("HYPERHIVE_HARNESS_DIR", &dir); + } + + // The signal surface is how `goal_reached` is callable at all, so a + // spawned turn's config has to carry it; `status` builds a config + // purely to resolve the store and has no subagent to hand it to. + let turn = build_config("n", None, None, None, None, Some(&signal_url())); + let checked = build_config("n", None, None, None, None, None); + + // SAFETY: see above. + unsafe { + std::env::remove_var("HYPERHIVE_HARNESS_DIR"); + } + let _ = std::fs::remove_dir_all(&dir); + + assert!( + turn.mcp_config.is_some(), + "a turn's config must carry an --mcp-config with the signal surface in it" + ); + assert!( + turn.strict_mcp_config && checked.strict_mcp_config, + "the safety property is unchanged: no ambient MCP discovery either way" + ); + } } diff --git a/nix/agent-modules/mcp.nix b/nix/agent-modules/mcp.nix index 70b6db16..f65ba05f 100644 --- a/nix/agent-modules/mcp.nix +++ b/nix/agent-modules/mcp.nix @@ -349,11 +349,17 @@ in # Subagent task runner daemon — independent of `hive-bash-daemon` (own # crate, own process): spawns nested claude sessions on request, serves # the `start`/`continue`/`status`/`interrupt` MCP tools directly over - # streamable-http on `hyperhive.mcp.subagentHttpPort`. No task files — - # this daemon's only state is an in-memory map of currently-running - # processes, live only as long as the process is (see - # `hive-subagent-mcp/src/session.rs`'s module doc); a restart stops - # whatever's running, the actual claude session survives independently. + # streamable-http on `hyperhive.mcp.subagentHttpPort`. The same port also + # serves a second, subagent-facing route (`/signal/mcp/`: + # `goal_reached`/`need_help`) — not something an agent's own config points + # at: the daemon mints each subagent it spawns its own token and writes + # that one URL into that subagent's own `--mcp-config`, which is how a + # signal's identity comes from the endpoint instead of from a `name` the + # caller could have filled in with a sibling's. No task files — + # this daemon's only state is in-memory, live only as long as the process + # is (see `hive-subagent-mcp/src/session.rs`'s module doc); a restart + # stops whatever's running, the actual claude session survives + # independently. systemd.services.hive-subagent-daemon = { description = "subagent task runner + MCP daemon for hive-subagent"; wantedBy = [ "multi-user.target" ];