hive-subagent-mcp: resume a subagent by session id, not by name
`continue`, and every later turn of a goal run, passed `--resume <name>`. claude 2.1.287 resolves a title across the project dirs of every git worktree of the repo and refuses when two sessions match, while `start` archives a prior session only in its own cwd's project dir. A name first started under one worktree and then started again under a sibling stayed live in both, and every resume of it failed with `--resume "<name>" matches 2 sessions`. That is the #5000 incident: build-4921a/b were first run under /home/atlas/wt/4901-build, then under their own worktrees. Both resume sites now look up the live session titled `name` in the config's own project dir (the same `SessionStore::find_by_title` lookup `start` archives with) and pass its id. The id comes off disk at each resume, so it survives a daemon restart. When that dir holds no session under the name, the name is passed as before and claude's answer stands. The test drives a fake claude that keeps sessions on disk and resolves titles across project dirs the way claude does. Its script is written by a child process, so no write fd on it exists in the test binary for a concurrent fork to inherit (the ETXTBSY of #4898).
This commit is contained in:
parent
40687ab4a5
commit
8f2069486d
2 changed files with 149 additions and 2 deletions
|
|
@ -65,6 +65,15 @@ record of a subagent's existence is claude's own on-disk session
|
||||||
of the daemon's own lifetime — a restart loses the _in-flight turn_, not
|
of the daemon's own lifetime — a restart loses the _in-flight turn_, not
|
||||||
the subagent's history.
|
the subagent's history.
|
||||||
|
|
||||||
|
`continue`, and each later turn of a goal run, hands claude a session id,
|
||||||
|
not the name: the id of the live session carrying that name in `dir`'s
|
||||||
|
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
|
||||||
|
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
|
||||||
|
`dir` holds no session under the name, the name goes to claude as-is and
|
||||||
|
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
|
||||||
other than the daemon's own working directory — and a restart is the
|
other than the daemon's own working directory — and a restart is the
|
||||||
|
|
|
||||||
|
|
@ -1120,6 +1120,22 @@ fn build_store(config: &Config) -> std::io::Result<SessionStore> {
|
||||||
))
|
))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// 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.
|
||||||
|
///
|
||||||
|
/// 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
|
||||||
|
/// in its own cwd, so a name reused under a sibling worktree's `dir` stays
|
||||||
|
/// live there. An id names one file.
|
||||||
|
fn resume_target(config: &Config, name: &str) -> String {
|
||||||
|
build_store(config)
|
||||||
|
.ok()
|
||||||
|
.and_then(|store| store.find_by_title(name))
|
||||||
|
.and_then(|path| path.file_stem().map(|id| id.to_string_lossy().into_owned()))
|
||||||
|
.unwrap_or_else(|| name.to_owned())
|
||||||
|
}
|
||||||
|
|
||||||
/// Everything a `start` needs, as one struct rather than a parameter list:
|
/// Everything a `start` needs, as one struct rather than a parameter list:
|
||||||
/// the call already carried six mostly-optional values before goals were
|
/// the call already carried six mostly-optional values before goals were
|
||||||
/// added, and nine positional arguments is both unreadable at the call site
|
/// added, and nine positional arguments is both unreadable at the call site
|
||||||
|
|
@ -1495,7 +1511,7 @@ fn continue_on_runtime(
|
||||||
state,
|
state,
|
||||||
name,
|
name,
|
||||||
&config,
|
&config,
|
||||||
&Attach::Resume(name.to_owned()),
|
&Attach::Resume(resume_target(&config, name)),
|
||||||
prompt,
|
prompt,
|
||||||
Some(Arc::clone(verdict)),
|
Some(Arc::clone(verdict)),
|
||||||
)
|
)
|
||||||
|
|
@ -1640,7 +1656,7 @@ 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(&config, &Attach::Resume(task_name.clone())) {
|
match Claude::spawn(&config, &Attach::Resume(resume_target(&config, &task_name))) {
|
||||||
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);
|
||||||
|
|
@ -4256,6 +4272,128 @@ done
|
||||||
std::fs::remove_dir_all(&dir).ok();
|
std::fs::remove_dir_all(&dir).ok();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A stand-in for claude's session store. `--name` writes
|
||||||
|
/// `$CLAUDE_CONFIG_DIR/projects/<slug(cwd)>/session-<n>.jsonl` carrying a
|
||||||
|
/// `custom-title` event. `--resume` takes an id from the cwd's project
|
||||||
|
/// dir, or a title matched against every live session under `projects/`
|
||||||
|
/// — each project dir standing for a worktree of one repo, which is how
|
||||||
|
/// far claude 2.1.287 looks for a title — and refuses unless exactly one
|
||||||
|
/// matches. Each attach is logged to `$CLAUDE_CONFIG_DIR/attaches`.
|
||||||
|
const SESSION_CLAUDE: &str = r#"#!/bin/sh
|
||||||
|
cat >/dev/null
|
||||||
|
projects="$CLAUDE_CONFIG_DIR/projects"
|
||||||
|
here="$projects/$(pwd -P | tr '/.' '--')"
|
||||||
|
while [ $# -gt 0 ]; do
|
||||||
|
case "$1" in
|
||||||
|
--name) mode=create; arg=$2; shift ;;
|
||||||
|
--resume) mode=resume; arg=$2; shift ;;
|
||||||
|
esac
|
||||||
|
shift
|
||||||
|
done
|
||||||
|
if [ "$mode" = create ]; then
|
||||||
|
n=$(cat "$CLAUDE_CONFIG_DIR/count" 2>/dev/null || echo 0)
|
||||||
|
echo $((n + 1)) > "$CLAUDE_CONFIG_DIR/count"
|
||||||
|
id="session-$n"
|
||||||
|
mkdir -p "$here"
|
||||||
|
printf '{"type":"custom-title","customTitle":"%s"}\n' "$arg" > "$here/$id.jsonl"
|
||||||
|
elif [ -f "$here/$arg.jsonl" ]; then
|
||||||
|
id=$arg
|
||||||
|
else
|
||||||
|
matches=$(grep -l "\"customTitle\":\"$arg\"" "$projects"/*/*.jsonl 2>/dev/null)
|
||||||
|
count=$(printf '%s' "$matches" | grep -c .)
|
||||||
|
if [ "$count" -ne 1 ]; then
|
||||||
|
echo "Error: --resume \"$arg\" matches $count sessions" >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
id=$(basename "$matches" .jsonl)
|
||||||
|
fi
|
||||||
|
echo "$mode $id" >> "$CLAUDE_CONFIG_DIR/attaches"
|
||||||
|
echo '{"type":"system","subtype":"init"}'
|
||||||
|
echo '{"type":"result","subtype":"success","is_error":false}'
|
||||||
|
"#;
|
||||||
|
|
||||||
|
/// Write an executable script from a child process rather than from
|
||||||
|
/// this one: a write fd on it held here could be inherited by a child
|
||||||
|
/// another test thread forks, and exec'ing a file some process still
|
||||||
|
/// holds open for writing fails with `ETXTBSY`.
|
||||||
|
fn write_executable(path: &Path, body: &str) {
|
||||||
|
let status = std::process::Command::new("/bin/sh")
|
||||||
|
.args([
|
||||||
|
"-c",
|
||||||
|
r#"printf '%s' "$1" > "$2" && chmod 755 "$2""#,
|
||||||
|
"sh",
|
||||||
|
body,
|
||||||
|
])
|
||||||
|
.arg(path)
|
||||||
|
.status()
|
||||||
|
.expect("run /bin/sh");
|
||||||
|
assert!(status.success(), "writing {} failed", path.display());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn resuming_a_name_reused_in_a_sibling_worktree_attaches_to_its_newest_session() {
|
||||||
|
// `n` started once in a sibling worktree, then twice here — the
|
||||||
|
// second archiving the first — leaves two live sessions titled `n`,
|
||||||
|
// which claude refuses to resume by title. Both a goal run's next
|
||||||
|
// turn and a `continue` must resume the session the last `start`
|
||||||
|
// here created.
|
||||||
|
let root = scratch_dir("resume-newest");
|
||||||
|
let (here, sibling) = (root.join("here"), root.join("sibling"));
|
||||||
|
let home = root.join("claude-home");
|
||||||
|
for dir in [&here, &sibling, &home] {
|
||||||
|
std::fs::create_dir_all(dir).expect("scratch subdir");
|
||||||
|
}
|
||||||
|
let program = root.join("session-claude");
|
||||||
|
write_executable(&program, SESSION_CLAUDE);
|
||||||
|
let config = |cwd: &Path| Config {
|
||||||
|
program: Some(program.clone()),
|
||||||
|
cwd: Some(cwd.to_path_buf()),
|
||||||
|
claude_config_dir: Some(home.clone()),
|
||||||
|
..Config::default()
|
||||||
|
};
|
||||||
|
let attaches = || lines(&home.join("attaches"));
|
||||||
|
let state = Arc::new(State::new(PathBuf::from("/dev/null"), signal_url()));
|
||||||
|
|
||||||
|
for (cwd, goal) in [(&sibling, None), (&here, None), (&here, Some("g"))] {
|
||||||
|
assert!(state.reserve("n"));
|
||||||
|
state.clear_stop("n");
|
||||||
|
state.set_goal("n", goal.map(str::to_owned), 2);
|
||||||
|
start_on_runtime(&state, "n", config(cwd), None, "go".into())
|
||||||
|
.expect("the fake claude spawns");
|
||||||
|
until("the run ends", || state.last_event_age("n").is_none()).await;
|
||||||
|
}
|
||||||
|
assert_eq!(
|
||||||
|
attaches(),
|
||||||
|
[
|
||||||
|
"create session-0",
|
||||||
|
"create session-1",
|
||||||
|
"create session-2",
|
||||||
|
"resume session-2"
|
||||||
|
],
|
||||||
|
"the goal run's second turn must resume its own session, not the title"
|
||||||
|
);
|
||||||
|
|
||||||
|
state.set_goal("n", None, 1);
|
||||||
|
state.clear_stop("n");
|
||||||
|
assert!(state.reserve("n"));
|
||||||
|
let (tx, rx) = oneshot::channel();
|
||||||
|
let verdict: VerdictTx = Arc::new(Mutex::new(Some(tx)));
|
||||||
|
continue_on_runtime(&state, "n", config(&here), "again".into(), &verdict)
|
||||||
|
.expect("the fake claude spawns");
|
||||||
|
await_resume(rx)
|
||||||
|
.await
|
||||||
|
.expect("a title shared with a sibling worktree must not fail the continue");
|
||||||
|
until("the continue's turn ends", || {
|
||||||
|
state.last_event_age("n").is_none()
|
||||||
|
})
|
||||||
|
.await;
|
||||||
|
assert_eq!(
|
||||||
|
attaches().last().map(String::as_str),
|
||||||
|
Some("resume session-2")
|
||||||
|
);
|
||||||
|
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");
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue