vinzenz/hyperhive
vinzenz/hyperhive
Watch
0
0
Fork
You've already forked hyperhive
 0
Code Activity

hive-subagent-mcp: refuse a continue whose session is not in the dir searched

Browse source 
resume_target no longer falls back to the bare name when the project dir
holds no session titled it: continue (and a goal run's next turn) refuses
before anything spawns, naming the directory searched. A store that cannot
be resolved refuses too, with the io error. The fallback notice is gone.
This commit is contained in:
atlas 2026-10-07 11:55:34 +02:00
commit 0e015197b6
3 changed files with 116 additions and 158 deletions
Download patch file Download diff file Expand all files Collapse all files

41
docs/tools/subagent.md
View file

@ -71,10 +71,8 @@ project directory, read off disk at every resume. claude looks a bare name
up across every git worktree of the repo and refuses when two sessions up across every git worktree of the repo and refuses when two sessions
match, and `start` archives a prior session only in its own directory, so match, and `start` archives a prior session only in its own directory, so
a name reused under a sibling worktree would otherwise stop resuming. When a name reused under a sibling worktree would otherwise stop resuming. When
`dir` holds no session under the name, the name goes to claude as-is, `dir` holds no session under the name, the daemon refuses and never hands
the daemon logs it, and `continue`'s reply — or its error, when claude refuses claude the bare name — see below.
— carries a note naming the directory searched; claude's answer stands, see
below.
Because the remembered directory goes with the rest of it, a `continue` Because the remembered directory goes with the rest of it, a `continue`
after a restart has to re-supply `dir` when the session lives anywhere after a restart has to re-supply `dir` when the session lives anywhere
@ -213,20 +211,33 @@ no progress left to describe.
## A `continue` that finds no session ## A `continue` that finds no session
<!-- vale write-good.Passive = NO --> `continue` looks for the session before it spawns anything. The lookup is
the live session titled the name in the project directory of the `dir` in
effect: the one passed, or else the one the daemon remembers for the name.
No such session refuses the call — nothing spawns and the name stays free
for a retry — and the error names the directory searched:
`continue` doesn't check for the session before spawning. claude's own ```
`--resume` is the authority, and it exits non-zero rather than quietly continue error: no session titled `n` under /home/agent/.claude/projects/-home-agent-work; pass the `dir` the session was started in
starting a fresh session, so the check could only duplicate the lookup ```
the driver was about to do — while answering as though the session were
gone. The usual truth is that the session exists somewhere else.
<!-- vale write-good.Passive = YES --> A name is never searched for in any other directory, because claude would
resolve a bare title across every worktree of the repo and could pick, or
refuse, the wrong session. A session started in another directory
needs that directory passed as `dir` — and after a daemon restart, which
forgets the remembered one, a session outside the daemon's own working
directory needs it again. A store location the daemon can't resolve
refuses the same way, with the io error.
`continue` waits for that answer instead. Where `start` returns the A goal run's next turn does the same lookup; a miss there ends the run as
instant the process exists — it creates its session, so the spawn a failed turn, with the same message in its end-of-turn todo.
succeeding is the whole story — a resumed turn can fail a moment _after_
a pid exists, and a pid is no proof that turn began. `continue` holds the The refusal covers a session missing from the directory searched.
It can't cover one that vanishes between the lookup and the spawn, so
`continue` still waits for claude's answer instead of returning the
instant the process exists. Where `start` returns at the spawn — it
creates its session, so the spawn succeeding is the whole story — a
resumed turn can fail a moment _after_ a pid exists. `continue` holds the
tool call open until the turn is underway or the resume has come back tool call open until the turn is underway or the resume has come back
missed, and reports a miss as the call's own error, carrying claude's missed, and reports a miss as the call's own error, carrying claude's
message plus the location this daemon searched: message plus the location this daemon searched:

11
hive-subagent-mcp/src/mcp.rs
View file

@ -164,8 +164,8 @@ struct ContinueArgs {
/// is exactly when you're most likely to be reaching for `continue`. So /// is exactly when you're most likely to be reaching for `continue`. So
/// pass it when pointing the session at a *different* directory than /// pass it when pointing the session at a *different* directory than
/// last time, and pass it again after a restart if the session lives /// last time, and pass it again after a restart if the session lives
/// anywhere other than the daemon's own working directory. A resume that /// anywhere other than the daemon's own working directory. A name with no
/// finds nothing says which directory it searched. /// session in the directory searched is refused, naming it.
#[serde(default)] #[serde(default)]
dir: Option<String>, dir: Option<String>,
} }
@ -245,9 +245,10 @@ impl SubagentMcp {
of the daemon that spawned it). Returns once the turn is underway rather than the \ of the daemon that spawned it). Returns once the turn is underway rather than the \
instant the process exists — a second or so, not the length of the turn — so a \ instant the process exists — a second or so, not the length of the turn — so a \
reply saying the turn started means it started; a name already running is refused. \ reply saying the turn started means it started; a name already running is refused. \
A name with no session to resume is not refused up front, because claude's own \ A name with no session in the directory searched is refused up front, before \
`--resume` decides that: a miss comes back as this call's own error, naming the \ anything spawns, and the error names that directory — the session is only found \
directory that was searched, so check `dir` before concluding the session is gone. \ under the `dir` it was started in (or the one this daemon remembers for the name, \
which a restart forgets), so pass `dir` before concluding the session is gone. \
Resuming a session whose last turn was killed is allowed — the reply says so, since \ Resuming a session whose last turn was killed is allowed — the reply says so, since \
that turn's work stopped wherever it had got to." that turn's work stopped wherever it had got to."
)] )]

218
hive-subagent-mcp/src/session.rs
View file

@ -23,11 +23,11 @@
//! age — the timestamp says the child is alive, never what it said. What it //! age — the timestamp says the child is alive, never what it said. What it
//! said goes to the swarm as the subagent's terminal (`crate::swarm_term`). //! said goes to the swarm as the subagent's terminal (`crate::swarm_term`).
//! //!
//! **`continue` doesn't pre-check the session's existence — it waits for //! **`continue` refuses a name with no session in the `dir` in effect
//! the answer instead.** claude's own `--resume` is the authority, so //! (`resume_target`), and waits for the driver's answer on the rest.** A
//! `continue` holds its tool call open for up to `RESUME_GRACE` and reports //! turn that still misses is the call's own error, naming the directory
//! a missed resume as its own error, naming the directory searched //! searched (`classify_end`), within `RESUME_GRACE`. See
//! (`classify_end`). See `docs/tools/subagent.md`. //! `docs/tools/subagent.md`.
//! **One `start` can be more than one turn.** With a `goal` set, a turn that //! **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 //! ends without a stop signal is followed by another re-prompting the
@ -1120,53 +1120,32 @@ fn build_store(config: &Config) -> std::io::Result<SessionStore> {
)) ))
} }
/// What a resume hands `--resume`, and whether that was a fallback the /// The session id a resume of `name` hands `--resume`: the live session
/// caller should be told about. /// titled `name` in `config`'s own project dir.
struct ResumeTarget {
/// The `--resume` value.
value: String,
/// Set when `value` is the bare name because the project dir held no
/// session titled it: what the caller needs to know to recover.
fallback_notice: Option<String>,
}
/// What a resume of `name` hands `--resume`: the id of the live session
/// titled `name` in `config`'s own project dir, or `name` itself when that
/// dir has none, so a miss is still claude's to report. The by-name fallback
/// is logged and carries a notice naming the dir searched.
/// ///
/// claude resolves a *title* across the project dirs of every git worktree /// claude resolves a *title* across the project dirs of every git worktree
/// of the repo, and refuses when more than one matches; `start` archives only /// of the repo, and refuses when more than one matches; `start` archives only
/// in its own cwd, so a name reused under a sibling worktree's `dir` stays /// in its own cwd, so a name reused under a sibling worktree's `dir` stays
/// live there. An id names one file. /// live there. An id names one file, and a name is never handed over in its
fn resume_target(config: &Config, name: &str) -> ResumeTarget { /// place: a session that is not in this dir is refused, not looked for
let store = build_store(config).ok(); /// elsewhere.
let found = store ///
.as_ref() /// # Errors
.and_then(|store| store.find_by_title(name)) ///
.and_then(|path| path.file_stem().map(|id| id.to_string_lossy().into_owned())); /// A store that cannot be resolved, or a project dir with no session titled
if let Some(id) = found { /// `name`; either message names what was searched.
return ResumeTarget { fn resume_target(config: &Config, name: &str) -> anyhow::Result<String> {
value: id, let store = build_store(config)
fallback_notice: None, .map_err(|e| anyhow::anyhow!("no session titled `{name}` can be looked for: {e}"))?;
}; store
} .find_by_title(name)
let searched = store.map_or_else( .and_then(|path| path.file_stem().map(|id| id.to_string_lossy().into_owned()))
|| "a location that could not be resolved".to_owned(), .ok_or_else(|| {
|store| store.project_dir().display().to_string(), anyhow::anyhow!(
); "no session titled `{name}` under {}; pass the `dir` the session was started in",
tracing::warn!( store.project_dir().display()
name = %name, )
searched = %searched, })
"subagent: no session found to resume by id; resuming by name"
);
ResumeTarget {
value: name.to_owned(),
fallback_notice: Some(format!(
"no session titled `{name}` was found under {searched}, so it was resumed by name — \
if it was started in another directory, pass the `dir` it was started in"
)),
}
} }
/// Everything a `start` needs, as one struct rather than a parameter list: /// Everything a `start` needs, as one struct rather than a parameter list:
@ -1368,18 +1347,15 @@ fn start_on_runtime(
/// means "the previous turn finished, here's the next instruction" or "the /// means "the previous turn finished, here's the next instruction" or "the
/// daemon restarted, reattaching." Refuses a name already running. /// daemon restarted, reattaching." Refuses a name already running.
/// ///
/// A name with **no** session to resume is still not *pre-checked* here: /// A name with no session titled it in the project dir of the `dir` in
/// claude's own `--resume` answers that, and the pre-check that used to live /// effect is refused before anything spawns, naming the directory searched
/// here could only repeat the lookup the driver was about to do anyway, /// (`resume_target`): the session is only ever resumed by its id, from that
/// while telling the caller a session didn't exist when the true answer was /// dir. A turn that still misses — the session vanished between the lookup
/// almost always that it exists somewhere else. What changed is that /// and the spawn — comes back as this call's own `Err` too, because
/// `continue` now waits for the driver's answer instead of returning ahead /// `continue` waits for the driver's answer instead of returning ahead of it
/// of it: a missed resume comes back as this call's own `Err`, carrying /// (`classify_end`). A `continue` that reports "started" therefore means the
/// claude's message and the directory that was searched (`classify_end`), /// turn started. See `await_resume` for the bound, and `spawn_and_track`'s
/// rather than only as an end-of-turn todo the caller had already stopped /// doc for why `start` keeps the older, unconditional contract.
/// looking for. A `continue` that reports "started" therefore means the turn
/// started. See `await_resume` for the bound, and `spawn_and_track`'s doc for
/// why `start` keeps the older, unconditional contract.
/// ///
/// Resuming a session whose last turn was *killed* is allowed — that is /// Resuming a session whose last turn was *killed* is allowed — that is
/// often exactly what the caller wants — but never silent: the reply says /// often exactly what the caller wants — but never silent: the reply says
@ -1392,8 +1368,9 @@ fn start_on_runtime(
/// # Errors /// # Errors
/// ///
/// An invalid name, one already running, a `model` the operator hasn't made /// An invalid name, one already running, a `model` the operator hasn't made
/// available (see `check_model`), `Claude::spawn` failing, or the resumed /// available (see `check_model`), no session titled `name` in the project
/// turn failing within `RESUME_GRACE` — in practice a missed resume. /// dir searched, `Claude::spawn` failing, or the resumed turn failing within
/// `RESUME_GRACE`.
pub async fn continue_( pub async fn continue_(
state: &Arc<State>, state: &Arc<State>,
name: &str, name: &str,
@ -1421,7 +1398,7 @@ pub async fn continue_(
state.restart_turns(name); state.restart_turns(name);
let (tx, rx) = oneshot::channel(); let (tx, rx) = oneshot::channel();
let verdict: VerdictTx = Arc::new(Mutex::new(Some(tx))); let verdict: VerdictTx = Arc::new(Mutex::new(Some(tx)));
let (started, notice) = let started =
match continue_reserved(state, name, prompt, model, effort, dir.as_deref(), &verdict) { match continue_reserved(state, name, prompt, model, effort, dir.as_deref(), &verdict) {
Ok(spawned) => spawned, Ok(spawned) => spawned,
Err(e) => { Err(e) => {
@ -1433,15 +1410,8 @@ pub async fn continue_(
// the grace has already been through `finish_turn`, which clears the // the grace has already been through `finish_turn`, which clears the
// tracking the spawn put there, and one that a goal carried into another // tracking the spawn put there, and one that a goal carried into another
// turn still owns the name via `between_turns`. // turn still owns the name via `between_turns`.
// The notice rides on the error too: a by-name resume is where claude await_resume(rx).await?;
// answers "matches 2 sessions", and the caller needs to know to pass `dir`. Ok(note_resumed_after_kill(&started, killed))
if let Err(e) = await_resume(rx).await {
anyhow::bail!("{}", with_resume_notice(&e.to_string(), notice.as_deref()));
}
Ok(with_resume_notice(
&note_resumed_after_kill(&started, killed),
notice.as_deref(),
))
} }
/// Hold a `continue` open until its resume is known to have landed — or /// Hold a `continue` open until its resume is known to have landed — or
@ -1495,15 +1465,6 @@ fn note_resumed_after_kill(msg: &str, killed: Option<i32>) -> String {
} }
} }
/// Append the by-name-resume notice (see `ResumeTarget`) to a `continue`'s
/// reply or error; pass `msg` through unchanged when there is none.
fn with_resume_notice(msg: &str, notice: Option<&str>) -> String {
match notice {
None => msg.to_owned(),
Some(notice) => format!("{msg} — note: {notice}"),
}
}
/// The slow, fallible part of `continue_`, run only after `reserve` has /// The slow, fallible part of `continue_`, run only after `reserve` has
/// already closed the TOCTOU window — same split rationale as /// already closed the TOCTOU window — same split rationale as
/// `start_reserved`. /// `start_reserved`.
@ -1515,7 +1476,7 @@ fn continue_reserved(
effort: Option<String>, effort: Option<String>,
dir: Option<&str>, dir: Option<&str>,
verdict: &VerdictTx, verdict: &VerdictTx,
) -> anyhow::Result<(String, Option<String>)> { ) -> anyhow::Result<String> {
// A fresh URL for the resumed run, not the one the last run was handed: // 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. // a token is per run, and this is a new one.
let signal_url = state.mint_signal_url(name); let signal_url = state.mint_signal_url(name);
@ -1525,20 +1486,19 @@ fn continue_reserved(
/// Resume `name`'s session on the daemon's runtime for one more run. /// Resume `name`'s session on the daemon's runtime for one more run.
/// ///
/// Returns the reply and, when the session could not be found by id, the
/// notice that goes with it.
///
/// # Errors /// # Errors
/// ///
/// On ACP, a name with no recorded session: the ACP runtime would start a /// On claude, a name with no session in `config`'s project dir (see
/// new one rather than fail, so the check has to happen here. /// `resume_target`). On ACP, a name with no recorded session: the ACP
/// runtime would start a new one rather than fail, so the check has to
/// happen here.
fn continue_on_runtime( fn continue_on_runtime(
state: &Arc<State>, state: &Arc<State>,
name: &str, name: &str,
config: Config, config: Config,
prompt: String, prompt: String,
verdict: &VerdictTx, verdict: &VerdictTx,
) -> anyhow::Result<(String, Option<String>)> { ) -> anyhow::Result<String> {
if let SubagentRuntime::Acp { command, sessions } = &state.runtime { if let SubagentRuntime::Acp { command, sessions } = &state.runtime {
let session = acp_session_file(sessions, name); let session = acp_session_file(sessions, name);
if !session.exists() { if !session.exists() {
@ -1556,23 +1516,21 @@ fn continue_on_runtime(
config, config,
prompt, prompt,
Some(Arc::clone(verdict)), Some(Arc::clone(verdict)),
) );
.map(|reply| (reply, None));
} }
// No existence pre-check: claude's own `--resume` is the authority on // The lookup in `resume_target` refuses before anything spawns. `verdict`
// whether the session is there, and it errors rather than quietly // still carries claude's own answer back in time to be the call's error,
// starting a fresh one. `verdict` is how that answer gets back to the // for a session that vanishes between the lookup and the spawn. See this
// caller in time to be its error. See this module's doc. // module's doc.
let target = resume_target(&config, name); let id = resume_target(&config, name)?;
let reply = spawn_and_track( spawn_and_track(
state, state,
name, name,
&config, &config,
&Attach::Resume(target.value), &Attach::Resume(id),
prompt, prompt,
Some(Arc::clone(verdict)), Some(Arc::clone(verdict)),
)?; )
Ok((reply, target.fallback_notice))
} }
/// Bumps `name`'s liveness clock on every line of the turn's output, and /// Bumps `name`'s liveness clock on every line of the turn's output, and
@ -1714,10 +1672,9 @@ fn spawn_and_track(
let Some(next) = after_turn(&state, &task_name, end, verdict.as_ref()).await else { let Some(next) = after_turn(&state, &task_name, end, verdict.as_ref()).await else {
return; return;
}; };
match Claude::spawn( match resume_target(&config, &task_name)
&config, .and_then(|id| Ok(Claude::spawn(&config, &Attach::Resume(id))?))
&Attach::Resume(resume_target(&config, &task_name).value), {
) {
Ok(next_running) => { Ok(next_running) => {
state.track(&task_name, Stopper::Claude(next_running.cancel_handle())); state.track(&task_name, Stopper::Claude(next_running.cancel_handle()));
state.note_event(&task_name); state.note_event(&task_name);
@ -4439,7 +4396,7 @@ echo '{"type":"result","subtype":"success","is_error":false}'
assert!(state.reserve("n")); assert!(state.reserve("n"));
let (tx, rx) = oneshot::channel(); let (tx, rx) = oneshot::channel();
let verdict: VerdictTx = Arc::new(Mutex::new(Some(tx))); let verdict: VerdictTx = Arc::new(Mutex::new(Some(tx)));
let (_, notice) = continue_on_runtime(&state, "n", config(&here), "again".into(), &verdict) continue_on_runtime(&state, "n", config(&here), "again".into(), &verdict)
.expect("the fake claude spawns"); .expect("the fake claude spawns");
await_resume(rx) await_resume(rx)
.await .await
@ -4459,16 +4416,14 @@ echo '{"type":"result","subtype":"success","is_error":false}'
], ],
"the continue must add its own resume of the session, not of the title" "the continue must add its own resume of the session, not of the title"
); );
assert_eq!(notice, None, "an id was found, so nothing was a fallback");
std::fs::remove_dir_all(&root).ok(); std::fs::remove_dir_all(&root).ok();
} }
#[tokio::test] #[tokio::test]
async fn resuming_where_no_session_is_on_disk_falls_back_to_the_name_and_says_so() { async fn resuming_where_no_session_is_on_disk_is_refused_without_spawning() {
// `n` lives only in the sibling worktree; resuming from `here` finds // `n` lives only in the sibling worktree, where claude would resolve
// no id and hands claude the bare name, which claude resolves across // the bare title; resuming from `here` must refuse instead.
// worktrees. let root = scratch_dir("resume-refused");
let root = scratch_dir("resume-fallback");
let (here, sibling) = (root.join("here"), root.join("sibling")); let (here, sibling) = (root.join("here"), root.join("sibling"));
let home = root.join("claude-home"); let home = root.join("claude-home");
for dir in [&here, &sibling, &home] { for dir in [&here, &sibling, &home] {
@ -4489,40 +4444,31 @@ echo '{"type":"result","subtype":"success","is_error":false}'
.expect("the fake claude spawns"); .expect("the fake claude spawns");
until("the run ends", || state.last_event_age("n").is_none()).await; until("the run ends", || state.last_event_age("n").is_none()).await;
let target = resume_target(&config(&here), "n");
assert_eq!(target.value, "n", "no session here, so the bare name");
let searched = SessionStore::new(&home, &here).project_dir(); let searched = SessionStore::new(&home, &here).project_dir();
let notice = target.fallback_notice.expect("a fallback says so");
assert!(
notice.contains("no session titled `n`")
&& notice.contains(&searched.display().to_string())
&& notice.contains("resumed by name")
&& notice.contains("pass the `dir`"),
"{notice}"
);
assert!(state.reserve("n")); assert!(state.reserve("n"));
let (tx, rx) = oneshot::channel(); let (tx, rx) = oneshot::channel();
let verdict: VerdictTx = Arc::new(Mutex::new(Some(tx))); let verdict: VerdictTx = Arc::new(Mutex::new(Some(tx)));
let (_, carried) = let refused = continue_on_runtime(&state, "n", config(&here), "again".into(), &verdict)
continue_on_runtime(&state, "n", config(&here), "again".into(), &verdict) .expect_err("no session titled `n` under `here`");
.expect("the fake claude spawns"); assert_eq!(
await_resume(rx).await.expect("one title match resumes"); refused.to_string(),
until("the continue's turn ends", || { format!(
state.last_event_age("n").is_none() "no session titled `n` under {}; pass the `dir` the session was started in",
}) searched.display()
.await; )
);
drop(rx);
assert_eq!( assert_eq!(
lines(&home.join("attaches")), lines(&home.join("attaches")),
["create session-0", "resume session-0"], ["create session-0"],
"the name was handed to claude, which found the sibling's session" "claude was not spawned, so nothing attached"
); );
assert_eq!(carried.as_deref(), Some(notice.as_str())); assert!(state.last_event_age("n").is_none(), "no turn is tracked");
assert_eq!(
with_resume_notice("subagent `n` started", carried.as_deref()), assert!(
format!("subagent `n` started — note: {notice}") resume_target(&config(&sibling), "n").is_ok(),
"the directory it was started in still resumes"
); );
assert_eq!(with_resume_notice("m", None), "m");
std::fs::remove_dir_all(&root).ok(); std::fs::remove_dir_all(&root).ok();
} }