subagent tool: remember dir per session name so continue/status don't need to repeat it
This commit is contained in:
parent
cfddb1bd40
commit
aa27334d76
2 changed files with 92 additions and 16 deletions
|
|
@ -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<String>,
|
||||
}
|
||||
|
|
@ -62,10 +64,9 @@ struct ContinueArgs {
|
|||
/// default — this does not have to match whatever model `start` used.
|
||||
#[serde(default)]
|
||||
model: Option<String>,
|
||||
/// 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<String>,
|
||||
}
|
||||
|
|
@ -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<String>,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,13 +3,17 @@
|
|||
//!
|
||||
//! **No task files, no restart recovery.** The daemon's only state is an
|
||||
//! in-memory `name -> Option<Cancel>` 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<HashMap<String, Option<Cancel>>>,
|
||||
dirs: Mutex<HashMap<String, String>>,
|
||||
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<String> {
|
||||
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<String> {
|
||||
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<String> {
|
||||
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<String> {
|
||||
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<St
|
|||
}
|
||||
None => {}
|
||||
}
|
||||
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"));
|
||||
|
|
|
|||
Loading…
Reference in a new issue