subagent: report turn liveness, and stop pre-checking continue
`status` could only answer running / starting / idle / killed / none, because every turn ran against `&NoopSink` and the whole stream-json stream was discarded. "Running" describes a wedged subagent exactly as well as a busy one, leaving a caller to tell them apart from `ps` output and CPU-time deltas. So the daemon now keeps a `name -> last_event_at` clock, bumped by `LivenessSink` on every line of every stream — stream-json events, plain stdout chatter and stderr alike — and `status` reports its age on a running answer: a few seconds means working, an age climbing into the minutes with no end-of-turn todo means wedged. Nothing is read out of the content; classifying *what* a subagent is doing is a separate question and waits on its own driver work. In memory with the rest of this daemon's state, dropped when the turn ends, no persistence. The clock is seeded at the spawn rather than at the first line, so a subagent that wedged before emitting anything still reports a climbing age rather than no age at all — the case an age is worth most in. Separately, `continue`'s existence pre-check is gone. It could only repeat the lookup `Claude::spawn` was about to do, and its message — "no session named `x` exists" — was false in the common failure: the session existed, just not under the claude home + cwd `build_store` resolved from. claude's own `--resume` is the authority and exits non-zero (`does not match any session title`) rather than quietly starting a fresh session, so the turn fails on its own. `classify_end` appends the one fact the CLI's message lacks — the directory searched: claude error: no session matched the requested id or title (searched <claude_home> for cwd <cwd>; if the session was started elsewhere, pass `dir`) The `dirs` map's durability is untouched; whether to persist it stays an open operator decision. Module doc, `docs/tools/subagent.md`, the `continue`/`status` tool descriptions and the `base:claude-subagents` skill all updated — including `continue`'s `dir` doc, which said "the daemon remembers it" without saying that a restart is both when it forgets and when you most want it. Refs #4330 Refs #4405
This commit is contained in:
parent
30fa54cbc6
commit
307df77948
4 changed files with 490 additions and 56 deletions
|
|
@ -33,7 +33,10 @@ interrupt(name, force?)
|
||||||
`start` and `continue` return as soon as the process is confirmed
|
`start` and `continue` return as soon as the process is confirmed
|
||||||
running, not once it finishes — a completion lands as a todo
|
running, not once it finishes — a completion lands as a todo
|
||||||
(`get_loose_ends`), same as any other producer. Use `status` for a
|
(`get_loose_ends`), same as any other producer. Use `status` for a
|
||||||
zero-cost "is it still going" check; reach for `continue` only once you
|
zero-cost "is it still going" check — for a running subagent it also
|
||||||
|
reports how long since that turn last produced output, so a few seconds
|
||||||
|
means it's working and an age climbing into the minutes means it's
|
||||||
|
wedged and worth an `interrupt`. Reach for `continue` only once you
|
||||||
actually have a new instruction for it, since that spends a turn.
|
actually have a new instruction for it, since that spends a turn.
|
||||||
`interrupt` genuinely stops a running turn (`force: true` for SIGKILL).
|
`interrupt` genuinely stops a running turn (`force: true` for SIGKILL).
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -26,12 +26,57 @@ Served under the `subagent` MCP server (`mcp__subagent__<tool>`): `start`,
|
||||||
|
|
||||||
## State
|
## State
|
||||||
|
|
||||||
In-memory only: a map of currently running processes, live only as long
|
In-memory only: what's running now, where each name's session lives, how
|
||||||
as the daemon process is. A daemon restart stops whatever was running
|
each name's last turn ended, and when each running turn last produced
|
||||||
rather than adopting it. The durable record of a subagent's existence is
|
output. All of it lives only as long as the daemon process does. A daemon
|
||||||
claude's own on-disk session (`hive_claude::SessionStore`), which
|
restart stops whatever was running rather than adopting it. The durable
|
||||||
`continue` reattaches to independent of the daemon's own lifetime — a
|
record of a subagent's existence is claude's own on-disk session
|
||||||
restart loses the _in-flight turn_, not the subagent's history.
|
(`hive_claude::SessionStore`), which `continue` reattaches to independent
|
||||||
|
of the daemon's own lifetime — a restart loses the _in-flight turn_, not
|
||||||
|
the subagent's history.
|
||||||
|
|
||||||
|
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
|
||||||
|
other than the daemon's own working directory — and a restart is the
|
||||||
|
situation you reach for `continue` in most often.
|
||||||
|
|
||||||
|
## Is it working, or is it wedged?
|
||||||
|
|
||||||
|
`status` reporting **running** says a process is tracked, which a wedged
|
||||||
|
subagent satisfies as fully as a busy one. A running answer therefore
|
||||||
|
carries the age of that turn's last event too: seconds means it's working,
|
||||||
|
an age climbing into the minutes with no end-of-turn todo means it's
|
||||||
|
stuck. That one number replaces inferring the same thing from `ps` output
|
||||||
|
and CPU-time deltas.
|
||||||
|
|
||||||
|
Every line the subagent's `claude` process writes bumps the timestamp —
|
||||||
|
stream-json events, plain stdout chatter and stderr alike — and nothing
|
||||||
|
about the content is inspected. The record says the child is alive, not
|
||||||
|
what it's doing. The clock starts at the spawn, so a subagent that wedged
|
||||||
|
before it ever emitted anything still reports a climbing age rather than
|
||||||
|
no age at all. It's dropped when the turn ends, since a finished turn has
|
||||||
|
no progress left to describe.
|
||||||
|
|
||||||
|
## A `continue` that finds no session
|
||||||
|
|
||||||
|
`continue` doesn't check for the session before spawning. claude's own
|
||||||
|
`--resume` is the authority, and it exits non-zero rather than quietly
|
||||||
|
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.
|
||||||
|
|
||||||
|
`continue` returns as soon as the process is confirmed running, same as
|
||||||
|
always, so the failure lands where every other failed turn lands: that
|
||||||
|
turn's end-of-turn todo, carrying claude's own message plus the location
|
||||||
|
this daemon searched.
|
||||||
|
|
||||||
|
```
|
||||||
|
claude error: no session matched the requested id or title (searched /home/agent/.claude for cwd /home/agent/work; if the session was started elsewhere, pass `dir`)
|
||||||
|
```
|
||||||
|
|
||||||
|
The directory is the part claude's own message never names, and the part
|
||||||
|
that resolves the confusion: pass `dir` to point `continue` at the
|
||||||
|
directory the session was started in.
|
||||||
|
|
||||||
## A killed turn
|
## A killed turn
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -79,8 +79,12 @@ struct ContinueArgs {
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
effort: Option<String>,
|
effort: Option<String>,
|
||||||
/// Omit to reuse whatever `dir` `start` (or a prior `continue`) used for
|
/// Omit to reuse whatever `dir` `start` (or a prior `continue`) used for
|
||||||
/// this name — the daemon remembers it. Only pass this to point the
|
/// this name — the daemon remembers it until it restarts, and a restart
|
||||||
/// session at a *different* directory than last time.
|
/// is exactly when you're most likely to be reaching for `continue`. So
|
||||||
|
/// pass it when pointing the session at a *different* directory than
|
||||||
|
/// 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
|
||||||
|
/// finds nothing says which directory it searched.
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
dir: Option<String>,
|
dir: Option<String>,
|
||||||
}
|
}
|
||||||
|
|
@ -146,9 +150,11 @@ impl SubagentMcp {
|
||||||
because its previous turn finished and you have a follow-up instruction, or you're \
|
because its previous turn finished and you have a follow-up instruction, or you're \
|
||||||
reattaching after this daemon restarted (the session itself survives independently \
|
reattaching after this daemon restarted (the session itself survives independently \
|
||||||
of the daemon that spawned it). Returns as soon as confirmed running, same as \
|
of the daemon that spawned it). Returns as soon as confirmed running, same as \
|
||||||
`start`. Refuses a name with no session on disk at all, or one already running. \
|
`start`; a name already running is refused. A name with no session to resume is not \
|
||||||
Resuming a session whose last turn was killed is allowed — the reply says so, since \
|
refused up front — claude's own `--resume` decides that, and a miss arrives as a \
|
||||||
that turn's work stopped wherever it had got to."
|
failed turn naming the directory that was searched, so check `dir` before concluding \
|
||||||
|
the session is gone. 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."
|
||||||
)]
|
)]
|
||||||
fn r#continue(&self, Parameters(args): Parameters<ContinueArgs>) -> String {
|
fn r#continue(&self, Parameters(args): Parameters<ContinueArgs>) -> String {
|
||||||
match session::continue_(
|
match session::continue_(
|
||||||
|
|
@ -181,7 +187,9 @@ impl SubagentMcp {
|
||||||
#[tool(
|
#[tool(
|
||||||
description = "Report whether a subagent is currently running — a zero-cost check that \
|
description = "Report whether a subagent is currently running — a zero-cost check that \
|
||||||
never launches a process, unlike `continue`. The answer says what state it found \
|
never launches a process, unlike `continue`. The answer says what state it found \
|
||||||
and what to do about it."
|
and what to do about it. For a running one it also reports how long since that turn \
|
||||||
|
last produced any output, which is how you tell a subagent that's working from one \
|
||||||
|
that has wedged without resorting to `ps`."
|
||||||
)]
|
)]
|
||||||
fn status(&self, Parameters(args): Parameters<StatusArgs>) -> String {
|
fn status(&self, Parameters(args): Parameters<StatusArgs>) -> String {
|
||||||
match session::status(&self.state, &args.name, args.dir.as_deref()) {
|
match session::status(&self.state, &args.name, args.dir.as_deref()) {
|
||||||
|
|
|
||||||
|
|
@ -4,17 +4,30 @@
|
||||||
//!
|
//!
|
||||||
//! **No task files, no restart recovery.** The daemon's only state is an
|
//! **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
|
//! in-memory `name -> Option<Cancel>` map (see `State`'s own doc for what
|
||||||
//! the `None`/`Some` split is for) plus a `name -> dir` memory so `start`'s
|
//! the `None`/`Some` split is for), a `name -> dir` memory so `start`'s
|
||||||
//! `dir` doesn't have to be repeated on every later `continue`/`status`
|
//! `dir` doesn't have to be repeated on every later `continue`/`status`
|
||||||
//! (`State::resolve_dir`) — both live for exactly as long as the process
|
//! (`State::resolve_dir`), and a `name -> last_event_at` liveness clock
|
||||||
//! is — a daemon restart means whatever was running gets killed with it
|
//! (`State::note_event`) — all of it living for exactly as long as the
|
||||||
//! (`tokio`'s own child-process drop semantics), not adopted, and the
|
//! process is. A daemon restart means whatever was running gets killed
|
||||||
//! remembered `dir` is gone too. The durable
|
//! with it (`tokio`'s own child-process drop semantics), not adopted, and
|
||||||
//! record of a subagent's existence is `hive_claude::SessionStore` — claude's
|
//! the remembered `dir` is gone too. The durable record of a subagent's
|
||||||
//! own on-disk session, found again by name. `continue` is how a caller
|
//! existence is `hive_claude::SessionStore` — claude's own on-disk
|
||||||
//! reattaches to it, whether that's "give it a new turn" or "the daemon
|
//! session, found again by name. `continue` is how a caller reattaches to
|
||||||
//! restarted and I want to pick this back up" — after a restart, re-supply
|
//! it, whether that's "give it a new turn" or "the daemon restarted and I
|
||||||
//! `dir` once if the session isn't in the daemon's own default directory.
|
//! 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.
|
||||||
|
//!
|
||||||
|
//! **A running turn is not necessarily a working turn.** A wedged child
|
||||||
|
//! satisfies "running" as fully as a busy one, so every line of the
|
||||||
|
//! child's streams bumps `last_event_at` (`LivenessSink`) and `status`
|
||||||
|
//! reports its age. Deliberately *unclassified* — the timestamp says the
|
||||||
|
//! child is alive, not what it's doing.
|
||||||
|
//!
|
||||||
|
//! **`continue` doesn't pre-check the session's existence.** claude's own
|
||||||
|
//! `--resume` is the authority and errors rather than silently starting a
|
||||||
|
//! fresh session; `classify_end` appends the one fact its message lacks —
|
||||||
|
//! the directory searched (`searched_location`), which is where this
|
||||||
|
//! actually goes wrong. See `docs/tools/subagent.md` for both.
|
||||||
|
|
||||||
//! **A killed turn is not a finished turn.** A child that died on a signal
|
//! **A killed turn is not a finished turn.** A child that died on a signal
|
||||||
//! arrives as a `hive_claude::Error::Exit` carrying its `ExitStatus`, so the
|
//! arrives as a `hive_claude::Error::Exit` carrying its `ExitStatus`, so the
|
||||||
|
|
@ -40,8 +53,9 @@ use std::collections::HashMap;
|
||||||
use std::os::unix::process::ExitStatusExt as _;
|
use std::os::unix::process::ExitStatusExt as _;
|
||||||
use std::path::PathBuf;
|
use std::path::PathBuf;
|
||||||
use std::sync::{Arc, Mutex, PoisonError};
|
use std::sync::{Arc, Mutex, PoisonError};
|
||||||
|
use std::time::{Duration, Instant};
|
||||||
|
|
||||||
use hive_claude::{Attach, Cancel, Claude, Config, NoopSink, SessionStore};
|
use hive_claude::{Attach, Cancel, Claude, Config, SessionStore};
|
||||||
|
|
||||||
/// How a subagent's turn ended, as far as the daemon can tell from what
|
/// How a subagent's turn ended, as far as the daemon can tell from what
|
||||||
/// [`hive_claude::RunningClaude::wait`] returned.
|
/// [`hive_claude::RunningClaude::wait`] returned.
|
||||||
|
|
@ -80,7 +94,15 @@ fn describe_signal(signal: i32) -> String {
|
||||||
/// `signal()` — the driver's own docs point at exactly this check — so the
|
/// `signal()` — the driver's own docs point at exactly this check — so the
|
||||||
/// "how" is preserved by the time it reaches here rather than having to be
|
/// "how" is preserved by the time it reaches here rather than having to be
|
||||||
/// recovered.
|
/// recovered.
|
||||||
fn classify_end(outcome: hive_claude::Result<()>) -> TurnEnd {
|
///
|
||||||
|
/// `searched` is the resume lookup's location, present only for a turn that
|
||||||
|
/// attached with [`Attach::Resume`] (see `searched_location`). A missed
|
||||||
|
/// resume arrives as [`hive_claude::Error::SessionNotFound`], whose message
|
||||||
|
/// names the *value* that matched nothing but not the *directory* it was
|
||||||
|
/// looked for in — and "the session lives in another directory" is the way
|
||||||
|
/// this actually fails in practice, so the caller gets told where the daemon
|
||||||
|
/// looked rather than left to conclude the session is gone.
|
||||||
|
fn classify_end(outcome: hive_claude::Result<()>, searched: Option<&str>) -> TurnEnd {
|
||||||
let Err(error) = outcome else {
|
let Err(error) = outcome else {
|
||||||
return TurnEnd::Complete;
|
return TurnEnd::Complete;
|
||||||
};
|
};
|
||||||
|
|
@ -89,9 +111,31 @@ fn classify_end(outcome: hive_claude::Result<()>) -> TurnEnd {
|
||||||
{
|
{
|
||||||
return TurnEnd::Killed { signal };
|
return TurnEnd::Killed { signal };
|
||||||
}
|
}
|
||||||
|
if matches!(error, hive_claude::Error::SessionNotFound)
|
||||||
|
&& let Some(searched) = searched
|
||||||
|
{
|
||||||
|
return TurnEnd::Failed(format!("claude error: {error} {searched}"));
|
||||||
|
}
|
||||||
TurnEnd::Failed(format!("claude error: {error}"))
|
TurnEnd::Failed(format!("claude error: {error}"))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Where a `--resume` against `config` would have looked for the session,
|
||||||
|
/// phrased to append to claude's own not-found message. Both halves come
|
||||||
|
/// from the same resolution `build_store` and the driver itself use, so this
|
||||||
|
/// names the directory that was genuinely searched rather than a guess at
|
||||||
|
/// it. `None` when either half fails to resolve (no `HOME`, a `cwd` that
|
||||||
|
/// doesn't exist) — an unresolvable location is worse than none, and the
|
||||||
|
/// underlying error still reaches the caller unadorned.
|
||||||
|
fn searched_location(config: &Config) -> Option<String> {
|
||||||
|
let home = config.resolved_claude_home().ok()?;
|
||||||
|
let cwd = config.resolved_cwd().ok()?;
|
||||||
|
Some(format!(
|
||||||
|
"(searched {} for cwd {}; if the session was started elsewhere, pass `dir`)",
|
||||||
|
home.display(),
|
||||||
|
cwd.display()
|
||||||
|
))
|
||||||
|
}
|
||||||
|
|
||||||
/// This daemon's whole state: which names currently have a live process or
|
/// This daemon's whole state: which names currently have a live process or
|
||||||
/// a reservation in flight, and where to push the completion todo.
|
/// a reservation in flight, and where to push the completion todo.
|
||||||
/// `Arc`-wrapped so the background task that drives a turn to completion
|
/// `Arc`-wrapped so the background task that drives a turn to completion
|
||||||
|
|
@ -111,6 +155,10 @@ fn classify_end(outcome: hive_claude::Result<()>) -> TurnEnd {
|
||||||
/// killed session is indistinguishable from a finished one the moment its
|
/// killed session is indistinguishable from a finished one the moment its
|
||||||
/// entry leaves `running` — the whole point of this record.
|
/// entry leaves `running` — the whole point of this record.
|
||||||
///
|
///
|
||||||
|
/// `last_event` splits `running` a second way — *in flight* against
|
||||||
|
/// *making progress*, since a wedged child is tracked exactly like a busy
|
||||||
|
/// one (`note_event` / `last_event_age`).
|
||||||
|
///
|
||||||
/// ⚠️ This concurrency guard is keyed by `name` alone — a `start`/`continue`
|
/// ⚠️ This concurrency guard is keyed by `name` alone — a `start`/`continue`
|
||||||
/// for `name` with a *different* `dir` than one already in flight under
|
/// for `name` with a *different* `dir` than one already in flight under
|
||||||
/// that name is refused as "already running," even though the two would
|
/// that name is refused as "already running," even though the two would
|
||||||
|
|
@ -122,6 +170,7 @@ pub struct State {
|
||||||
running: Mutex<HashMap<String, Option<Cancel>>>,
|
running: Mutex<HashMap<String, Option<Cancel>>>,
|
||||||
dirs: Mutex<HashMap<String, String>>,
|
dirs: Mutex<HashMap<String, String>>,
|
||||||
killed: Mutex<HashMap<String, i32>>,
|
killed: Mutex<HashMap<String, i32>>,
|
||||||
|
last_event: Mutex<HashMap<String, Instant>>,
|
||||||
socket: PathBuf,
|
socket: PathBuf,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -132,6 +181,7 @@ impl State {
|
||||||
running: Mutex::new(HashMap::new()),
|
running: Mutex::new(HashMap::new()),
|
||||||
dirs: Mutex::new(HashMap::new()),
|
dirs: Mutex::new(HashMap::new()),
|
||||||
killed: Mutex::new(HashMap::new()),
|
killed: Mutex::new(HashMap::new()),
|
||||||
|
last_event: Mutex::new(HashMap::new()),
|
||||||
socket,
|
socket,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -184,6 +234,10 @@ impl State {
|
||||||
/// a `status` landing between the two would otherwise read the session
|
/// a `status` landing between the two would otherwise read the session
|
||||||
/// as plainly idle, which is the exact confusion this record exists to
|
/// as plainly idle, which is the exact confusion this record exists to
|
||||||
/// remove.
|
/// remove.
|
||||||
|
///
|
||||||
|
/// The liveness clock goes with the `running` entry, for the same reason
|
||||||
|
/// it's kept at all: it answers "is this turn still making progress",
|
||||||
|
/// and a turn that has ended has no progress left to make.
|
||||||
fn finish_turn(&self, name: &str, end: &TurnEnd) {
|
fn finish_turn(&self, name: &str, end: &TurnEnd) {
|
||||||
let mut killed = self.killed.lock().unwrap_or_else(PoisonError::into_inner);
|
let mut killed = self.killed.lock().unwrap_or_else(PoisonError::into_inner);
|
||||||
match end {
|
match end {
|
||||||
|
|
@ -195,6 +249,10 @@ impl State {
|
||||||
.lock()
|
.lock()
|
||||||
.unwrap_or_else(PoisonError::into_inner)
|
.unwrap_or_else(PoisonError::into_inner)
|
||||||
.remove(name);
|
.remove(name);
|
||||||
|
self.last_event
|
||||||
|
.lock()
|
||||||
|
.unwrap_or_else(PoisonError::into_inner)
|
||||||
|
.remove(name);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The signal `name`'s last turn died on, if it died on one.
|
/// The signal `name`'s last turn died on, if it died on one.
|
||||||
|
|
@ -216,6 +274,42 @@ impl State {
|
||||||
.remove(name);
|
.remove(name);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// `name`'s turn just produced output — record *when*, and nothing else.
|
||||||
|
/// Called from `LivenessSink` for every line of every stream, so it runs
|
||||||
|
/// far more often than anything else in this type and stays a single
|
||||||
|
/// map write for that reason.
|
||||||
|
///
|
||||||
|
/// A monotonic [`Instant`], not a wall clock: what's ever read back out
|
||||||
|
/// is an elapsed age, which a clock adjustment must not be able to
|
||||||
|
/// distort into a stall that never happened.
|
||||||
|
///
|
||||||
|
/// Also called once by `spawn_and_track` the moment the child exists, so
|
||||||
|
/// the clock starts at the spawn rather than at the first line. Without
|
||||||
|
/// that seed a subagent that wedged before emitting anything at all
|
||||||
|
/// would report no age forever — the one case where an age is most worth
|
||||||
|
/// having. The value therefore reads as "how long since the daemon last
|
||||||
|
/// heard anything from this child", counting the spawn itself as the
|
||||||
|
/// first thing it heard.
|
||||||
|
fn note_event(&self, name: &str) {
|
||||||
|
self.last_event
|
||||||
|
.lock()
|
||||||
|
.unwrap_or_else(PoisonError::into_inner)
|
||||||
|
.insert(name.to_owned(), Instant::now());
|
||||||
|
}
|
||||||
|
|
||||||
|
/// How long since `name`'s turn last produced output. `None` once the
|
||||||
|
/// turn is over (`finish_turn` drops the entry) or for a name that never
|
||||||
|
/// reached a spawn — in both cases there's no *running* turn whose
|
||||||
|
/// progress the age would describe, and a leftover age from a turn that
|
||||||
|
/// has already ended would read as a stall that isn't one.
|
||||||
|
fn last_event_age(&self, name: &str) -> Option<Duration> {
|
||||||
|
self.last_event
|
||||||
|
.lock()
|
||||||
|
.unwrap_or_else(PoisonError::into_inner)
|
||||||
|
.get(name)
|
||||||
|
.map(Instant::elapsed)
|
||||||
|
}
|
||||||
|
|
||||||
/// An explicit `dir` is remembered against `name` and returned as-is; an
|
/// An explicit `dir` is remembered against `name` and returned as-is; an
|
||||||
/// omitted one (`None`) falls back to whatever was last remembered for
|
/// omitted one (`None`) falls back to whatever was last remembered for
|
||||||
/// `name`, so a `start` that gave a `dir` doesn't force every later
|
/// `name`, so a `start` that gave a `dir` doesn't force every later
|
||||||
|
|
@ -425,10 +519,18 @@ fn start_reserved(
|
||||||
|
|
||||||
/// Give an existing named session a new turn — resuming it whether that
|
/// Give an existing named session a new turn — resuming it whether that
|
||||||
/// 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 with no session on disk
|
/// daemon restarted, reattaching." Refuses a name already running (same
|
||||||
/// at all (nothing to continue) or one already running (same
|
|
||||||
/// concurrent-run hazard as `start`).
|
/// concurrent-run hazard as `start`).
|
||||||
///
|
///
|
||||||
|
/// A name with **no** session to resume is not refused here: claude's own
|
||||||
|
/// `--resume` answers that, and its answer arrives with the turn rather
|
||||||
|
/// than before it — as a `SessionNotFound` failure carrying the directory
|
||||||
|
/// that was searched (`classify_end`), reaching the caller through the
|
||||||
|
/// end-of-turn todo and `status` like any other failed turn. The pre-check
|
||||||
|
/// that used to live here could only repeat the lookup the driver was about
|
||||||
|
/// to do anyway, and told the caller a session didn't exist when the true
|
||||||
|
/// answer was almost always that it exists somewhere else.
|
||||||
|
///
|
||||||
/// 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
|
||||||
/// so, since a caller that never ran `status` and missed the todo would
|
/// so, since a caller that never ran `status` and missed the todo would
|
||||||
|
|
@ -436,8 +538,7 @@ fn start_reserved(
|
||||||
///
|
///
|
||||||
/// # Errors
|
/// # Errors
|
||||||
///
|
///
|
||||||
/// No session under `name`, an invalid name, one already running, or
|
/// An invalid name, one already running, or `Claude::spawn` failing.
|
||||||
/// `Claude::spawn` failing.
|
|
||||||
pub fn continue_(
|
pub fn continue_(
|
||||||
state: &Arc<State>,
|
state: &Arc<State>,
|
||||||
name: &str,
|
name: &str,
|
||||||
|
|
@ -491,13 +592,9 @@ fn continue_reserved(
|
||||||
dir: Option<&str>,
|
dir: Option<&str>,
|
||||||
) -> anyhow::Result<String> {
|
) -> anyhow::Result<String> {
|
||||||
let config = build_config(name, model, effort, None, dir);
|
let config = build_config(name, model, effort, None, dir);
|
||||||
let store = build_store(&config)?;
|
// No existence pre-check: claude's own `--resume` is the authority on
|
||||||
if store.find_by_title(name).is_none() {
|
// whether the session is there, and it errors rather than quietly
|
||||||
anyhow::bail!(
|
// starting a fresh one. See this module's doc.
|
||||||
"no session named `{name}` exists — `continue` resumes an existing subagent, `start` \
|
|
||||||
creates one"
|
|
||||||
);
|
|
||||||
}
|
|
||||||
spawn_and_track(
|
spawn_and_track(
|
||||||
state,
|
state,
|
||||||
name,
|
name,
|
||||||
|
|
@ -507,6 +604,41 @@ fn continue_reserved(
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Bumps `name`'s liveness clock on every line of the turn's output, and
|
||||||
|
/// does nothing else with it. Replaces the `NoopSink` this daemon used to
|
||||||
|
/// run turns against, which discarded the stream wholesale and left `status`
|
||||||
|
/// unable to tell a working child from a wedged one.
|
||||||
|
///
|
||||||
|
/// **All three callbacks, deliberately.** A stderr line or a stdout line
|
||||||
|
/// that didn't parse as JSON is proof the child is alive every bit as much
|
||||||
|
/// as a stream-json event is, and the failure that matters here is reporting
|
||||||
|
/// a live subagent as wedged — so anything the child says counts. Nothing is
|
||||||
|
/// read out of the content: classifying *what* the subagent is doing is a
|
||||||
|
/// separate question from whether it's doing anything.
|
||||||
|
///
|
||||||
|
/// Sink methods are called synchronously from the driver's stream readers as
|
||||||
|
/// lines arrive, so the body has to stay cheap — one uncontended map write
|
||||||
|
/// is, and forwarding to a channel to do the same write elsewhere would cost
|
||||||
|
/// more than it saved.
|
||||||
|
struct LivenessSink {
|
||||||
|
state: Arc<State>,
|
||||||
|
name: String,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl hive_claude::Sink for LivenessSink {
|
||||||
|
fn on_event(&self, _event: &serde_json::Value) {
|
||||||
|
self.state.note_event(&self.name);
|
||||||
|
}
|
||||||
|
|
||||||
|
fn on_stdout_line(&self, _line: &str) {
|
||||||
|
self.state.note_event(&self.name);
|
||||||
|
}
|
||||||
|
|
||||||
|
fn on_stderr_line(&self, _line: &str) {
|
||||||
|
self.state.note_event(&self.name);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Spawn the child (synchronous — returns with a real pid the instant the
|
/// Spawn the child (synchronous — returns with a real pid the instant the
|
||||||
/// process exists, which *is* "confirmed running": there is no stronger
|
/// process exists, which *is* "confirmed running": there is no stronger
|
||||||
/// signal to wait for without slowing every call down for no reason), track
|
/// signal to wait for without slowing every call down for no reason), track
|
||||||
|
|
@ -534,11 +666,25 @@ fn spawn_and_track(
|
||||||
// This turn supersedes whatever the previous one did, including having
|
// This turn supersedes whatever the previous one did, including having
|
||||||
// been killed — the record is about the turn before this one.
|
// been killed — the record is about the turn before this one.
|
||||||
state.clear_kill(name);
|
state.clear_kill(name);
|
||||||
|
// Start the liveness clock at the spawn, so the age is already an answer
|
||||||
|
// before the child's first line — see `note_event`.
|
||||||
|
state.note_event(name);
|
||||||
|
|
||||||
|
// Resolved here, on the calling thread, while the config is still to
|
||||||
|
// hand: only a resume can miss, and only `classify_end` finding a
|
||||||
|
// `SessionNotFound` ever uses it.
|
||||||
|
let searched = matches!(attach, Attach::Resume(_))
|
||||||
|
.then(|| searched_location(config))
|
||||||
|
.flatten();
|
||||||
|
|
||||||
let state = Arc::clone(state);
|
let state = Arc::clone(state);
|
||||||
let task_name = name.to_owned();
|
let task_name = name.to_owned();
|
||||||
tokio::spawn(async move {
|
tokio::spawn(async move {
|
||||||
let end = classify_end(running.wait(&prompt, &NoopSink).await);
|
let sink = LivenessSink {
|
||||||
|
state: Arc::clone(&state),
|
||||||
|
name: task_name.clone(),
|
||||||
|
};
|
||||||
|
let end = classify_end(running.wait(&prompt, &sink).await, searched.as_deref());
|
||||||
match &end {
|
match &end {
|
||||||
TurnEnd::Complete => {}
|
TurnEnd::Complete => {}
|
||||||
TurnEnd::Killed { signal } => {
|
TurnEnd::Killed { signal } => {
|
||||||
|
|
@ -566,6 +712,11 @@ fn spawn_and_track(
|
||||||
/// its last turn finished, nothing is in flight), and no such session at
|
/// its last turn finished, nothing is in flight), and no such session at
|
||||||
/// all.
|
/// all.
|
||||||
///
|
///
|
||||||
|
/// A *running* answer also carries how long since that turn last produced
|
||||||
|
/// output, which is the part of this answer a caller can act on: "running"
|
||||||
|
/// describes a wedged child and a busy one identically, and the age
|
||||||
|
/// separates them (see `State`'s `last_event` doc).
|
||||||
|
///
|
||||||
/// # Errors
|
/// # Errors
|
||||||
///
|
///
|
||||||
/// An invalid name, or no session — running, killed or on disk — under
|
/// An invalid name, or no session — running, killed or on disk — under
|
||||||
|
|
@ -578,6 +729,7 @@ pub fn status(state: &State, name: &str, dir: Option<&str>) -> anyhow::Result<St
|
||||||
let dir = state.peek_dir(name, dir);
|
let dir = state.peek_dir(name, dir);
|
||||||
let occupancy = state.occupancy(name);
|
let occupancy = state.occupancy(name);
|
||||||
let killed = state.killed_by(name);
|
let killed = state.killed_by(name);
|
||||||
|
let last_event_age = state.last_event_age(name);
|
||||||
// Nothing on disk to look for while something is tracked in memory —
|
// Nothing on disk to look for while something is tracked in memory —
|
||||||
// those states answer on their own, and the store read is the only
|
// those states answer on their own, and the store read is the only
|
||||||
// expensive part of this call.
|
// expensive part of this call.
|
||||||
|
|
@ -587,12 +739,34 @@ pub fn status(state: &State, name: &str, dir: Option<&str>) -> anyhow::Result<St
|
||||||
} else {
|
} else {
|
||||||
false
|
false
|
||||||
};
|
};
|
||||||
describe_status(name, occupancy, killed, session_exists)
|
describe_status(name, occupancy, killed, session_exists, last_event_age)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Render `status`'s answer from the three facts it gathers. Split out from
|
/// The liveness sentence appended to a *running* answer, and the whole point
|
||||||
/// the gathering so the killed-versus-idle distinction is exercisable
|
/// of recording the timestamp: an age in seconds a caller can read a verdict
|
||||||
/// without a real spawn, a real signal and a real on-disk claude session.
|
/// off directly, rather than one it has to go and measure itself with `ps`
|
||||||
|
/// and CPU-time deltas.
|
||||||
|
///
|
||||||
|
/// Empty when there's no age to report — a turn that ended between `status`
|
||||||
|
/// reading `running` and reading the clock. Saying nothing is right there:
|
||||||
|
/// the alternative is an age that describes a turn which has already
|
||||||
|
/// finished, which reads as a stall that never happened.
|
||||||
|
fn describe_liveness(age: Option<Duration>) -> String {
|
||||||
|
match age {
|
||||||
|
None => String::new(),
|
||||||
|
Some(age) => format!(
|
||||||
|
" Last event {}s ago — how long since this turn's claude process produced any output \
|
||||||
|
at all, whatever it was: a few seconds means it's working, an age that keeps \
|
||||||
|
climbing into the minutes with no end-of-turn todo means it's wedged.",
|
||||||
|
age.as_secs()
|
||||||
|
),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Render `status`'s answer from the facts it gathers. Split out from the
|
||||||
|
/// gathering so the killed-versus-idle distinction — and the liveness age on
|
||||||
|
/// a running turn — are exercisable without a real spawn, a real signal, a
|
||||||
|
/// real stream of events and a real on-disk claude session.
|
||||||
///
|
///
|
||||||
/// A recorded kill outranks the on-disk session: the session file exists
|
/// A recorded kill outranks the on-disk session: the session file exists
|
||||||
/// either way, so "a session is there" is precisely the fact that cannot
|
/// either way, so "a session is there" is precisely the fact that cannot
|
||||||
|
|
@ -608,13 +782,15 @@ fn describe_status(
|
||||||
occupancy: Option<bool>,
|
occupancy: Option<bool>,
|
||||||
killed: Option<i32>,
|
killed: Option<i32>,
|
||||||
session_exists: bool,
|
session_exists: bool,
|
||||||
|
last_event_age: Option<Duration>,
|
||||||
) -> anyhow::Result<String> {
|
) -> anyhow::Result<String> {
|
||||||
match occupancy {
|
match occupancy {
|
||||||
Some(true) => {
|
Some(true) => {
|
||||||
return Ok(format!(
|
return Ok(format!(
|
||||||
"subagent `{name}` is running — its turn is still in flight, so there's nothing \
|
"subagent `{name}` is running — its turn is still in flight, so there's nothing \
|
||||||
to do but let it work: the daemon pushes a todo when the turn ends, or \
|
to do but let it work: the daemon pushes a todo when the turn ends, or \
|
||||||
`interrupt` it if you want it stopped early."
|
`interrupt` it if you want it stopped early.{}",
|
||||||
|
describe_liveness(last_event_age)
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
Some(false) => {
|
Some(false) => {
|
||||||
|
|
@ -915,19 +1091,19 @@ mod tests {
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn classify_end_separates_a_signalled_child_from_a_clean_one() {
|
fn classify_end_separates_a_signalled_child_from_a_clean_one() {
|
||||||
assert_eq!(classify_end(Ok(())), TurnEnd::Complete);
|
assert_eq!(classify_end(Ok(()), None), TurnEnd::Complete);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
classify_end(Err(signalled_exit(libc::SIGKILL))),
|
classify_end(Err(signalled_exit(libc::SIGKILL)), None),
|
||||||
TurnEnd::Killed { signal: 9 },
|
TurnEnd::Killed { signal: 9 },
|
||||||
"a SIGKILLed child must not read as a turn that ended on its own"
|
"a SIGKILLed child must not read as a turn that ended on its own"
|
||||||
);
|
);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
classify_end(Err(signalled_exit(libc::SIGTERM))),
|
classify_end(Err(signalled_exit(libc::SIGTERM)), None),
|
||||||
TurnEnd::Killed { signal: 15 }
|
TurnEnd::Killed { signal: 15 }
|
||||||
);
|
);
|
||||||
assert!(
|
assert!(
|
||||||
matches!(
|
matches!(
|
||||||
classify_end(Err(hive_claude::Error::PromptTooLong)),
|
classify_end(Err(hive_claude::Error::PromptTooLong), None),
|
||||||
TurnEnd::Failed(_)
|
TurnEnd::Failed(_)
|
||||||
),
|
),
|
||||||
"claude exiting on its own is a failure, not a kill"
|
"claude exiting on its own is a failure, not a kill"
|
||||||
|
|
@ -938,8 +1114,9 @@ mod tests {
|
||||||
fn status_reports_killed_not_idle_for_a_signalled_session() {
|
fn status_reports_killed_not_idle_for_a_signalled_session() {
|
||||||
// Both sessions exist on disk and neither is running: the only thing
|
// Both sessions exist on disk and neither is running: the only thing
|
||||||
// that can tell them apart is the recorded signal.
|
// that can tell them apart is the recorded signal.
|
||||||
let killed = describe_status("n", None, Some(libc::SIGKILL), true).expect("killed status");
|
let killed =
|
||||||
let idle = describe_status("n", None, None, true).expect("idle status");
|
describe_status("n", None, Some(libc::SIGKILL), true, None).expect("killed status");
|
||||||
|
let idle = describe_status("n", None, None, true, None).expect("idle status");
|
||||||
assert!(
|
assert!(
|
||||||
killed.contains("killed") && killed.contains("SIGKILL (signal 9)"),
|
killed.contains("killed") && killed.contains("SIGKILL (signal 9)"),
|
||||||
"a killed session must say so, and name the signal: {killed}"
|
"a killed session must say so, and name the signal: {killed}"
|
||||||
|
|
@ -960,28 +1137,30 @@ mod tests {
|
||||||
// The tool description no longer lists the states, so each answer
|
// The tool description no longer lists the states, so each answer
|
||||||
// has to carry its own explanation — checked one state at a time,
|
// has to carry its own explanation — checked one state at a time,
|
||||||
// which is all a caller ever gets back.
|
// which is all a caller ever gets back.
|
||||||
let running = describe_status("n", Some(true), None, false).expect("running status");
|
let running = describe_status("n", Some(true), None, false, None).expect("running status");
|
||||||
assert!(
|
assert!(
|
||||||
running.contains("running") && running.contains("`interrupt`"),
|
running.contains("running") && running.contains("`interrupt`"),
|
||||||
"a running answer must say the turn is in flight and how to stop it: {running}"
|
"a running answer must say the turn is in flight and how to stop it: {running}"
|
||||||
);
|
);
|
||||||
let starting = describe_status("n", Some(false), None, false).expect("starting status");
|
let starting =
|
||||||
|
describe_status("n", Some(false), None, false, None).expect("starting status");
|
||||||
assert!(
|
assert!(
|
||||||
starting.contains("starting") && starting.contains("check again"),
|
starting.contains("starting") && starting.contains("check again"),
|
||||||
"a starting answer must say the spawn isn't confirmed yet and to retry: {starting}"
|
"a starting answer must say the spawn isn't confirmed yet and to retry: {starting}"
|
||||||
);
|
);
|
||||||
let idle = describe_status("n", None, None, true).expect("idle status");
|
let idle = describe_status("n", None, None, true, None).expect("idle status");
|
||||||
assert!(
|
assert!(
|
||||||
idle.contains("idle") && idle.contains("`continue`"),
|
idle.contains("idle") && idle.contains("`continue`"),
|
||||||
"an idle answer must say the last turn ended on its own and how to give it another: \
|
"an idle answer must say the last turn ended on its own and how to give it another: \
|
||||||
{idle}"
|
{idle}"
|
||||||
);
|
);
|
||||||
let killed = describe_status("n", None, Some(libc::SIGKILL), true).expect("killed status");
|
let killed =
|
||||||
|
describe_status("n", None, Some(libc::SIGKILL), true, None).expect("killed status");
|
||||||
assert!(
|
assert!(
|
||||||
killed.contains("killed") && killed.contains("`continue`"),
|
killed.contains("killed") && killed.contains("`continue`"),
|
||||||
"a killed answer must name the kill and say resuming is still possible: {killed}"
|
"a killed answer must name the kill and say resuming is still possible: {killed}"
|
||||||
);
|
);
|
||||||
let missing = describe_status("n", None, None, false)
|
let missing = describe_status("n", None, None, false, None)
|
||||||
.expect_err("nothing tracked and nothing on disk is an error, not a state");
|
.expect_err("nothing tracked and nothing on disk is an error, not a state");
|
||||||
assert!(
|
assert!(
|
||||||
missing.to_string().contains("`start`"),
|
missing.to_string().contains("`start`"),
|
||||||
|
|
@ -999,7 +1178,7 @@ mod tests {
|
||||||
("done", Ok(())),
|
("done", Ok(())),
|
||||||
] {
|
] {
|
||||||
assert!(state.reserve(name));
|
assert!(state.reserve(name));
|
||||||
state.finish_turn(name, &classify_end(outcome));
|
state.finish_turn(name, &classify_end(outcome, None));
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
state.occupancy(name),
|
state.occupancy(name),
|
||||||
None,
|
None,
|
||||||
|
|
@ -1009,8 +1188,10 @@ mod tests {
|
||||||
assert_eq!(state.killed_by("gone"), Some(9));
|
assert_eq!(state.killed_by("gone"), Some(9));
|
||||||
assert_eq!(state.killed_by("done"), None);
|
assert_eq!(state.killed_by("done"), None);
|
||||||
|
|
||||||
let gone = describe_status("gone", None, state.killed_by("gone"), true).expect("status");
|
let gone =
|
||||||
let done = describe_status("done", None, state.killed_by("done"), true).expect("status");
|
describe_status("gone", None, state.killed_by("gone"), true, None).expect("status");
|
||||||
|
let done =
|
||||||
|
describe_status("done", None, state.killed_by("done"), true, None).expect("status");
|
||||||
assert!(gone.contains("killed"), "{gone}");
|
assert!(gone.contains("killed"), "{gone}");
|
||||||
assert!(done.contains("idle"), "{done}");
|
assert!(done.contains("idle"), "{done}");
|
||||||
}
|
}
|
||||||
|
|
@ -1096,6 +1277,203 @@ mod tests {
|
||||||
assert_eq!(config.effort, Some("high".to_owned()));
|
assert_eq!(config.effort, Some("high".to_owned()));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn an_event_starts_the_liveness_clock_and_the_turn_ending_stops_it() {
|
||||||
|
let state = State::new(PathBuf::from("/dev/null"));
|
||||||
|
assert_eq!(
|
||||||
|
state.last_event_age("n"),
|
||||||
|
None,
|
||||||
|
"a name that never spawned has no clock to read"
|
||||||
|
);
|
||||||
|
state.note_event("n");
|
||||||
|
assert!(
|
||||||
|
state.last_event_age("n").is_some(),
|
||||||
|
"the first event must give `status` an age to report"
|
||||||
|
);
|
||||||
|
state.finish_turn("n", &TurnEnd::Complete);
|
||||||
|
assert_eq!(
|
||||||
|
state.last_event_age("n"),
|
||||||
|
None,
|
||||||
|
"the age describes a turn in flight — a finished turn's leftover age would read as a \
|
||||||
|
stall that never happened"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_later_event_resets_the_age_rather_than_letting_it_climb() {
|
||||||
|
// The distinction the whole record exists for: a child still
|
||||||
|
// producing output must not accumulate the age of a wedged one.
|
||||||
|
let state = State::new(PathBuf::from("/dev/null"));
|
||||||
|
state.note_event("n");
|
||||||
|
std::thread::sleep(Duration::from_millis(20));
|
||||||
|
let before = state.last_event_age("n").expect("clock started");
|
||||||
|
state.note_event("n");
|
||||||
|
let after = state.last_event_age("n").expect("clock still running");
|
||||||
|
assert!(
|
||||||
|
after < before,
|
||||||
|
"a fresh event must reset the age, not extend it: {after:?} vs {before:?}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn the_liveness_clock_does_not_cross_names() {
|
||||||
|
let state = State::new(PathBuf::from("/dev/null"));
|
||||||
|
state.note_event("a");
|
||||||
|
assert_eq!(
|
||||||
|
state.last_event_age("b"),
|
||||||
|
None,
|
||||||
|
"one subagent's output says nothing about another's"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn every_sink_callback_counts_as_liveness() {
|
||||||
|
// All three, deliberately: a stderr line or a non-JSON stdout line is
|
||||||
|
// proof the child is alive exactly as much as a stream-json event,
|
||||||
|
// and reporting a live subagent as wedged is the failure that costs.
|
||||||
|
use hive_claude::Sink as _;
|
||||||
|
|
||||||
|
fn fresh() -> (Arc<State>, LivenessSink) {
|
||||||
|
let state = Arc::new(State::new(PathBuf::from("/dev/null")));
|
||||||
|
let sink = LivenessSink {
|
||||||
|
state: Arc::clone(&state),
|
||||||
|
name: "n".to_owned(),
|
||||||
|
};
|
||||||
|
(state, sink)
|
||||||
|
}
|
||||||
|
|
||||||
|
let (state, sink) = fresh();
|
||||||
|
sink.on_event(&serde_json::json!({"type": "system"}));
|
||||||
|
assert!(
|
||||||
|
state.last_event_age("n").is_some(),
|
||||||
|
"a stream-json event is liveness"
|
||||||
|
);
|
||||||
|
|
||||||
|
let (state, sink) = fresh();
|
||||||
|
sink.on_stdout_line("not json");
|
||||||
|
assert!(
|
||||||
|
state.last_event_age("n").is_some(),
|
||||||
|
"so is a stdout line that didn't parse as JSON"
|
||||||
|
);
|
||||||
|
|
||||||
|
let (state, sink) = fresh();
|
||||||
|
sink.on_stderr_line("some chatter");
|
||||||
|
assert!(
|
||||||
|
state.last_event_age("n").is_some(),
|
||||||
|
"so is a stderr line — anything the child says at all counts"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_running_status_reports_the_age_and_an_idle_one_does_not() {
|
||||||
|
let running = describe_status("n", Some(true), None, false, Some(Duration::from_secs(4)))
|
||||||
|
.expect("running status");
|
||||||
|
assert!(
|
||||||
|
running.contains("Last event 4s ago"),
|
||||||
|
"a running answer must carry the age — the one part of it a caller can act on: \
|
||||||
|
{running}"
|
||||||
|
);
|
||||||
|
let wedged = describe_status("n", Some(true), None, false, Some(Duration::from_mins(15)))
|
||||||
|
.expect("running status");
|
||||||
|
assert!(
|
||||||
|
wedged.contains("Last event 900s ago"),
|
||||||
|
"the wedged case is the one this exists for: {wedged}"
|
||||||
|
);
|
||||||
|
assert_ne!(
|
||||||
|
running, wedged,
|
||||||
|
"4s-ago and 900s-ago must not render identically — collapsing them back together is \
|
||||||
|
the bug this reports"
|
||||||
|
);
|
||||||
|
|
||||||
|
let idle = describe_status("n", None, None, true, None).expect("idle status");
|
||||||
|
assert!(
|
||||||
|
!idle.contains("Last event"),
|
||||||
|
"an idle session has no in-flight turn whose progress an age would describe: {idle}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_running_status_still_answers_when_the_turn_ended_mid_call() {
|
||||||
|
// `status` reads `running` and the clock under separate locks, so a
|
||||||
|
// turn can finish between the two. The answer drops the age rather
|
||||||
|
// than inventing one.
|
||||||
|
let raced = describe_status("n", Some(true), None, false, None).expect("running status");
|
||||||
|
assert!(
|
||||||
|
raced.contains("is running") && !raced.contains("Last event"),
|
||||||
|
"a missing age must cost the sentence, not the answer: {raced}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_missed_resume_names_the_directory_that_was_searched() {
|
||||||
|
// The failure that cost an afternoon: the session existed, just not
|
||||||
|
// where this daemon looked. claude's own message names the value it
|
||||||
|
// failed to match but never the directory, which is the fact that
|
||||||
|
// was missing.
|
||||||
|
let searched = "(searched /home/agent/.claude for cwd /home/agent/work; if the session \
|
||||||
|
was started elsewhere, pass `dir`)";
|
||||||
|
let end = classify_end(Err(hive_claude::Error::SessionNotFound), Some(searched));
|
||||||
|
let TurnEnd::Failed(msg) = end else {
|
||||||
|
panic!("a missed resume is a failed turn");
|
||||||
|
};
|
||||||
|
assert!(
|
||||||
|
msg.contains("no session matched"),
|
||||||
|
"claude's own diagnosis must survive: {msg}"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
msg.contains("/home/agent/.claude") && msg.contains("/home/agent/work"),
|
||||||
|
"and the search location must be appended to it: {msg}"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
msg.contains("pass `dir`"),
|
||||||
|
"naming the directory is only half of it — say what to do about it: {msg}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn only_a_missed_resume_gets_the_search_location() {
|
||||||
|
// A `start` never passes one (nothing to resume), and an unrelated
|
||||||
|
// failure must not be decorated with a location that had no part in
|
||||||
|
// it.
|
||||||
|
let no_hint = classify_end(Err(hive_claude::Error::SessionNotFound), None);
|
||||||
|
assert_eq!(
|
||||||
|
no_hint,
|
||||||
|
TurnEnd::Failed(
|
||||||
|
"claude error: no session matched the requested id or title".to_owned()
|
||||||
|
),
|
||||||
|
"with no resolvable location the underlying error still reaches the caller, plain"
|
||||||
|
);
|
||||||
|
|
||||||
|
let unrelated = classify_end(Err(hive_claude::Error::PromptTooLong), Some("(searched …)"));
|
||||||
|
let TurnEnd::Failed(msg) = unrelated else {
|
||||||
|
panic!("an overflowed context is a failed turn");
|
||||||
|
};
|
||||||
|
assert!(
|
||||||
|
!msg.contains("searched"),
|
||||||
|
"a context overflow has nothing to do with where the session lives: {msg}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn searched_location_names_the_same_place_the_store_would_look() {
|
||||||
|
// The point of reading both halves off the `Config` rather than
|
||||||
|
// re-deriving them: the location reported must be the one actually
|
||||||
|
// searched, so it can't drift from `build_store`/the driver.
|
||||||
|
let cwd = std::env::current_dir().expect("a cwd");
|
||||||
|
let config = build_config("n", None, None, None, Some(&cwd.to_string_lossy()));
|
||||||
|
let Some(located) = searched_location(&config) else {
|
||||||
|
// No `HOME` in this environment — nothing to compare against.
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
let home = config.resolved_claude_home().expect("home resolved");
|
||||||
|
let resolved_cwd = config.resolved_cwd().expect("cwd resolved");
|
||||||
|
assert!(
|
||||||
|
located.contains(&home.display().to_string())
|
||||||
|
&& located.contains(&resolved_cwd.display().to_string()),
|
||||||
|
"both halves must come from the config's own resolution: {located}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn build_config_defaults_effort_to_medium_when_omitted() {
|
fn build_config_defaults_effort_to_medium_when_omitted() {
|
||||||
// Unlike `model`, an omitted `effort` does not fall through to
|
// Unlike `model`, an omitted `effort` does not fall through to
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue