From aa27334d765ce0f26b8200ff5443125a1e367dd6 Mon Sep 17 00:00:00 2001 From: damocles Date: Fri, 11 Sep 2026 21:49:44 +0200 Subject: [PATCH] subagent tool: remember dir per session name so continue/status don't need to repeat it --- hive-subagent-mcp/src/mcp.rs | 24 +++++---- hive-subagent-mcp/src/session.rs | 84 +++++++++++++++++++++++++++++--- 2 files changed, 92 insertions(+), 16 deletions(-) diff --git a/hive-subagent-mcp/src/mcp.rs b/hive-subagent-mcp/src/mcp.rs index f424fad8..f0f3be86 100644 --- a/hive-subagent-mcp/src/mcp.rs +++ b/hive-subagent-mcp/src/mcp.rs @@ -40,10 +40,12 @@ struct StartArgs { /// Working directory for the subagent's session — e.g. a git worktree /// you've already prepared for it, so a parallel batch of subagents /// never race on the same working tree. Must exist. Omit to inherit - /// this daemon's own working directory (today's default). Claude - /// derives its per-project session storage from this path, so - /// `continue`/`status` against this name must pass this exact same - /// `dir` again to find the session — see those tools' own docs. + /// this daemon's own working directory (today's default). The daemon + /// remembers whichever `dir` you give here against `name`, so a later + /// `continue`/`status` for the same name doesn't need to repeat it — + /// only pass it there again if you want to point at a *different* + /// directory. Forgotten on a daemon restart, same as everything else + /// this daemon tracks in memory. #[serde(default)] dir: Option, } @@ -62,10 +64,9 @@ struct ContinueArgs { /// default — this does not have to match whatever model `start` used. #[serde(default)] model: Option, - /// The same `dir` given at `start`, if any — sessions are stored keyed - /// by directory, so a different (or omitted) `dir` here looks in the - /// wrong place and reports no session found under `name` even though - /// one exists. + /// Omit to reuse whatever `dir` `start` (or a prior `continue`) used for + /// this name — the daemon remembers it. Only pass this to point the + /// session at a *different* directory than last time. #[serde(default)] dir: Option, } @@ -74,8 +75,11 @@ struct ContinueArgs { struct StatusArgs { /// The subagent name to check. name: String, - /// The same `dir` given at `start`, if any — see `continue`'s `dir` doc - /// for why this has to match. + /// Omit to use whatever `dir` was last remembered for this name (see + /// `start`'s `dir` doc) — you only need this if nothing's running or + /// reserved for `name` right now (the common "is it running" case never + /// even looks at it) *and* you want to check a different directory's + /// session than the one last remembered. #[serde(default)] dir: Option, } diff --git a/hive-subagent-mcp/src/session.rs b/hive-subagent-mcp/src/session.rs index c7dd6b7a..8d116c3d 100644 --- a/hive-subagent-mcp/src/session.rs +++ b/hive-subagent-mcp/src/session.rs @@ -3,13 +3,17 @@ //! //! **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), live for exactly as long as the process +//! the `None`/`Some` split is for) plus a `name -> dir` memory so `start`'s +//! `dir` doesn't have to be repeated on every later `continue`/`status` +//! (`State::resolve_dir`) — both live 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. The durable +//! (`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 session, found again by name. `continue` is how a caller //! reattaches to it, whether that's "give it a new turn" or "the daemon -//! restarted and I want to pick this back up." +//! restarted and I want to pick this back up" — after a restart, re-supply +//! `dir` once if the session isn't in the daemon's own default directory. //! //! **No mid-turn compaction.** Building on `hive_claude::Claude::spawn` + //! `RunningClaude::wait` directly (not `InfiniteSession::run`) is what makes @@ -43,6 +47,7 @@ use hive_claude::{Attach, Cancel, Claude, Config, NoopSink, SessionStore}; /// unless the check *is* the reservation — see `reserve`. pub struct State { running: Mutex>>, + dirs: Mutex>, socket: PathBuf, } @@ -51,6 +56,7 @@ impl State { pub fn new(socket: PathBuf) -> Self { Self { running: Mutex::new(HashMap::new()), + dirs: Mutex::new(HashMap::new()), socket, } } @@ -92,6 +98,28 @@ impl State { running.remove(name); } } + + /// An explicit `dir` is remembered against `name` and returned as-is; an + /// omitted one (`None`) falls back to whatever was last remembered for + /// `name`, so a `start` that gave a `dir` doesn't force every later + /// `continue`/`status` against the same name to repeat it. Explicit + /// always wins and updates the memory — there's no way to say "forget + /// it, use the default" once something's been remembered short of + /// giving a genuinely different `dir`, a deliberate simplicity trade, + /// not an oversight. In-memory only, same durability envelope as + /// `running`: a daemon restart forgets it same as it forgets everything + /// else here — the caller re-supplies `dir` once, same as it always + /// could. + fn resolve_dir(&self, name: &str, dir: Option<&str>) -> Option { + let mut dirs = self.dirs.lock().unwrap_or_else(PoisonError::into_inner); + match dir { + Some(d) => { + dirs.insert(name.to_owned(), d.to_owned()); + Some(d.to_owned()) + } + None => dirs.get(name).cloned(), + } + } } /// A caller-chosen name, validated the same way `hive-bash-mcp`'s task ids @@ -179,10 +207,11 @@ pub fn start( dir: Option<&str>, ) -> anyhow::Result { validate_name(name)?; + let dir = state.resolve_dir(name, dir); if !state.reserve(name) { anyhow::bail!("subagent `{name}` is already running — use `continue` or `interrupt`"); } - let result = start_reserved(state, name, model, prompt_file, trigger, dir); + let result = start_reserved(state, name, model, prompt_file, trigger, dir.as_deref()); if result.is_err() { state.release_reservation(name); } @@ -239,12 +268,13 @@ pub fn continue_( dir: Option<&str>, ) -> anyhow::Result { validate_name(name)?; + let dir = state.resolve_dir(name, dir); if !state.reserve(name) { anyhow::bail!( "subagent `{name}` is already running — use `interrupt` first if you meant to redirect it" ); } - let result = continue_reserved(state, name, prompt, model, dir); + let result = continue_reserved(state, name, prompt, model, dir.as_deref()); if result.is_err() { state.release_reservation(name); } @@ -336,6 +366,7 @@ fn spawn_and_track( /// An invalid name, or no session — running or on disk — under `name`. pub fn status(state: &State, name: &str, dir: Option<&str>) -> anyhow::Result { validate_name(name)?; + let dir = state.resolve_dir(name, dir); match state.occupancy(name) { Some(true) => return Ok(format!("subagent `{name}` is running")), Some(false) => { @@ -345,7 +376,7 @@ pub fn status(state: &State, name: &str, dir: Option<&str>) -> anyhow::Result {} } - let config = build_config(name, None, None, dir); + let config = build_config(name, None, None, dir.as_deref()); let store = build_store(&config)?; if store.find_by_title(name).is_some() { Ok(format!( @@ -500,6 +531,47 @@ 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")); + assert_eq!( + state.resolve_dir("n", None), + None, + "nothing remembered yet — omitted dir has nothing to fall back to" + ); + assert_eq!( + state.resolve_dir("n", Some("/tmp/worktree")), + Some("/tmp/worktree".to_owned()), + "an explicit dir is returned as-is" + ); + assert_eq!( + state.resolve_dir("n", None), + Some("/tmp/worktree".to_owned()), + "a later omitted dir falls back to what was just remembered" + ); + assert_eq!( + state.resolve_dir("n", Some("/tmp/other")), + Some("/tmp/other".to_owned()), + "a later explicit dir overrides the memory, not just reads it" + ); + assert_eq!( + state.resolve_dir("n", None), + Some("/tmp/other".to_owned()), + "the fallback now reflects the override" + ); + } + + #[test] + fn resolve_dir_does_not_cross_names() { + let state = State::new(PathBuf::from("/dev/null")); + state.resolve_dir("a", Some("/tmp/a")); + assert_eq!( + state.resolve_dir("b", None), + None, + "an unrelated name's memory must stay empty" + ); + } + #[test] fn build_config_sets_cwd_only_when_a_dir_is_given() { let with = build_config("n", None, None, Some("/tmp/some-worktree"));