Watch
0
0
Fork
You've already forked hyperhive
0

hive-subagent-mcp: a refused continue leaves the session's records alone

`continue_` cleared the stop reason, reset the turn budget and remembered
the caller's `dir` before the session lookup ran, and nothing restored
them when the lookup refused. With the strict lookup a refusal is an
expected outcome (the caller omitted `dir`), so every such retry erased
why the session stopped from `status`, gave a capped goal session its
budget back, and made the wrong `dir` the remembered one for the next
bare `continue`.

The lookup now runs first, against the dir peeked rather than committed;
only a session that is found gets `State::begin_continue`, which does
all three. Same for the ACP path's recorded-session check.
This commit is contained in:
atlas 2026-10-07 12:20:26 +02:00
commit 22ce8b815a
2 changed files with 99 additions and 32 deletions

View file

@ -214,8 +214,10 @@ no progress left to describe.
`continue` looks for the session before it spawns anything. The lookup is `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 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. 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 No such session refuses the call — nothing spawns, the name stays free
for a retry — and the error names the directory searched: for a retry, and the session keeps its `status` (stop reason and turn
budget) and its remembered `dir` — and the error names the directory
searched:
``` ```
continue error: no session titled `n` under /home/agent/.claude/projects/-home-agent-work; pass the `dir` the session was started in continue error: no session titled `n` under /home/agent/.claude/projects/-home-agent-work; pass the `dir` the session was started in

View file

@ -678,13 +678,11 @@ impl State {
/// could. /// could.
/// ///
/// ⚠️ **Callers must only call this once the call is known to proceed** /// ⚠️ **Callers must only call this once the call is known to proceed**
/// (i.e. after `reserve` has already succeeded in `start`/`continue_`) — /// — after `reserve` in `start`, after the session lookup in a
/// see `peek_dir` for the non-committing variant a call that might still /// `continue` (`begin_continue`) — so a refused call never changes what
/// be refused (or that shouldn't persist its `dir` at all, like /// a later bare `continue` resolves to. `peek_dir` is the non-committing
/// `status`) needs instead. This one argus caught in review: calling it /// variant for a call that might still be refused, or that shouldn't
/// unconditionally before `reserve`'s check meant a *rejected* concurrent /// persist its `dir` at all, like `status`.
/// call still overwrote the remembered `dir` for the name it was refused
/// against.
fn resolve_dir(&self, name: &str, dir: Option<&str>) -> Option<String> { fn resolve_dir(&self, name: &str, dir: Option<&str>) -> Option<String> {
let mut dirs = self.dirs.lock().unwrap_or_else(PoisonError::into_inner); let mut dirs = self.dirs.lock().unwrap_or_else(PoisonError::into_inner);
match dir { match dir {
@ -778,6 +776,16 @@ impl State {
} }
} }
/// Commit a `continue` whose session has been found: remember its `dir`
/// (see `resolve_dir`), drop the stop reason that described the run
/// before it, and give a goal session its allowance back (see
/// `restart_turns`).
fn begin_continue(&self, name: &str, dir: Option<&str>) {
self.resolve_dir(name, dir);
self.clear_stop(name);
self.restart_turns(name);
}
/// `(turn, max_turns)` for `name`. `None` for a session with no goal, /// `(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` /// which has no turn budget to be partway through — reporting `1 of 1`
/// there would invent a cap that isn't enforced. /// there would invent a cap that isn't enforced.
@ -1389,23 +1397,15 @@ pub async fn continue_(
// Read before the spawn, which clears the record as soon as the new turn // Read before the spawn, which clears the record as soon as the new turn
// is confirmed — this reply is the last chance to mention it. // is confirmed — this reply is the last chance to mention it.
let killed = state.killed_by(name); let killed = state.killed_by(name);
// 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 (tx, rx) = oneshot::channel();
let verdict: VerdictTx = Arc::new(Mutex::new(Some(tx))); let verdict: VerdictTx = Arc::new(Mutex::new(Some(tx)));
let started = let started = match continue_reserved(state, name, prompt, model, effort, dir, &verdict) {
match continue_reserved(state, name, prompt, model, effort, dir.as_deref(), &verdict) { Ok(spawned) => spawned,
Ok(spawned) => spawned, Err(e) => {
Err(e) => { state.release_reservation(name);
state.release_reservation(name); return Err(e);
return Err(e); }
} };
};
// Nothing to release on this path either way: a turn that ended inside // Nothing to release on this path either way: a turn that ended inside
// 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
@ -1467,7 +1467,8 @@ fn note_resumed_after_kill(msg: &str, killed: Option<i32>) -> String {
/// 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`. `dir` is the caller's own, not yet resolved against the
/// remembered one.
fn continue_reserved( fn continue_reserved(
state: &Arc<State>, state: &Arc<State>,
name: &str, name: &str,
@ -1480,12 +1481,24 @@ fn continue_reserved(
// 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);
let config = build_config(name, model, effort, None, dir, Some(&signal_url)); let resolved = state.peek_dir(name, dir);
continue_on_runtime(state, name, config, prompt, verdict) let config = build_config(
name,
model,
effort,
None,
resolved.as_deref(),
Some(&signal_url),
);
continue_on_runtime(state, name, config, dir, prompt, verdict)
} }
/// 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.
/// ///
/// The session is looked up before any of `name`'s records change, so a
/// refused call leaves `status` and the remembered `dir` as they were; only
/// a session that is there gets `State::begin_continue`.
///
/// # Errors /// # Errors
/// ///
/// On claude, a name with no session in `config`'s project dir (see /// On claude, a name with no session in `config`'s project dir (see
@ -1496,6 +1509,7 @@ fn continue_on_runtime(
state: &Arc<State>, state: &Arc<State>,
name: &str, name: &str,
config: Config, config: Config,
dir: Option<&str>,
prompt: String, prompt: String,
verdict: &VerdictTx, verdict: &VerdictTx,
) -> anyhow::Result<String> { ) -> anyhow::Result<String> {
@ -1508,6 +1522,7 @@ fn continue_on_runtime(
session.display() session.display()
); );
} }
state.begin_continue(name, dir);
let runtime = acp_runtime(command.clone(), sessions, name); let runtime = acp_runtime(command.clone(), sessions, name);
return spawn_and_track_acp( return spawn_and_track_acp(
state, state,
@ -1523,6 +1538,7 @@ fn continue_on_runtime(
// for a session that vanishes between the lookup and the spawn. See this // for a session that vanishes between the lookup and the spawn. See this
// module's doc. // module's doc.
let id = resume_target(&config, name)?; let id = resume_target(&config, name)?;
state.begin_continue(name, dir);
spawn_and_track( spawn_and_track(
state, state,
name, name,
@ -4396,7 +4412,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)));
continue_on_runtime(&state, "n", config(&here), "again".into(), &verdict) continue_on_runtime(&state, "n", config(&here), None, "again".into(), &verdict)
.expect("the fake claude spawns"); .expect("the fake claude spawns");
await_resume(rx) await_resume(rx)
.await .await
@ -4448,8 +4464,9 @@ 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 refused = continue_on_runtime(&state, "n", config(&here), "again".into(), &verdict) let refused =
.expect_err("no session titled `n` under `here`"); continue_on_runtime(&state, "n", config(&here), None, "again".into(), &verdict)
.expect_err("no session titled `n` under `here`");
assert_eq!( assert_eq!(
refused.to_string(), refused.to_string(),
format!( format!(
@ -4472,6 +4489,46 @@ echo '{"type":"result","subtype":"success","is_error":false}'
std::fs::remove_dir_all(&root).ok(); std::fs::remove_dir_all(&root).ok();
} }
#[test]
fn a_refused_continue_leaves_the_sessions_records_as_they_were() {
// A capped goal session on its last turn, stopped on a signal, with a
// remembered `dir`; the `continue` names a dir holding no session.
let root = scratch_dir("refused-keeps-records");
let state = Arc::new(State::new(PathBuf::from("/dev/null"), signal_url()));
state.set_goal("n", Some("g".into()), 3);
state.plan_after_turn("n");
state.plan_after_turn("n");
state.record_stop("n", StopReason::NeedHelp("blocked on X".into()));
state.resolve_dir("n", Some("/remembered"));
assert_eq!(state.turns("n"), Some((3, 3)));
assert!(state.reserve("n"));
let (tx, _rx) = oneshot::channel();
let verdict: VerdictTx = Arc::new(Mutex::new(Some(tx)));
continue_on_runtime(
&state,
"n",
claude_config(&root),
root.to_str(),
"again".into(),
&verdict,
)
.expect_err("no session under the scratch dir");
assert_eq!(
state.stop_reason("n"),
Some(StopReason::NeedHelp("blocked on X".into())),
"`status` still says why the session stopped"
);
assert_eq!(state.turns("n"), Some((3, 3)), "the budget is not reset");
assert_eq!(
state.peek_dir("n", None),
Some("/remembered".to_owned()),
"the refused call's dir is not remembered"
);
std::fs::remove_dir_all(&root).ok();
}
#[tokio::test] #[tokio::test]
async fn an_acp_parent_runs_its_subagents_on_its_acp_agent() { async fn an_acp_parent_runs_its_subagents_on_its_acp_agent() {
let dir = scratch_dir("runtime-acp"); let dir = scratch_dir("runtime-acp");
@ -4509,6 +4566,7 @@ echo '{"type":"result","subtype":"success","is_error":false}'
&state, &state,
"n", "n",
claude_config(&dir), claude_config(&dir),
None,
"TRIGGER-TWO".into(), "TRIGGER-TWO".into(),
&verdict, &verdict,
) )
@ -4542,8 +4600,15 @@ echo '{"type":"result","subtype":"success","is_error":false}'
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 err = continue_on_runtime(&state, "n", claude_config(&dir), "go".into(), &verdict) let err = continue_on_runtime(
.expect_err("nothing to resume"); &state,
"n",
claude_config(&dir),
None,
"go".into(),
&verdict,
)
.expect_err("nothing to resume");
assert!(err.to_string().contains("no ACP session"), "{err}"); assert!(err.to_string().contains("no ACP session"), "{err}");
assert!( assert!(
!dir.join("prompts.methods").exists(), !dir.join("prompts.methods").exists(),