End a subagent on its own end_turn, not the parent's tool_result, and give the expander a touch-sized row
The Agent tool runs subagents in the background, so the parent's result arrives at launch while the subagent works on for minutes; finishing on it read a running agent as finished with a transcript cut off at launch. A subagent now ends on its own message_delta end_turn, and a later line for a finished one reopens it, since a background agent can be messaged again. The card's expander row was only the chevron's height, so a tap for it landed on the first subcard; it is the platform's 48dp minimum now. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
1 parent
9fa09b0af1
commit
cf10b17c5b
4 files changed
+226
-25
No files matched your search
+25
-7
@@ -50,17 +50,35 @@ There is no separate delete.
|
||||
2. Every child line is translated by that subagent's own `Translator`
|
||||
(one per subagent: tool ids are unique but streaming deltas are by
|
||||
content-block index, and parallel subagents interleave).
|
||||
3. When the parent's `tool_result` for the Task id arrives, the parent gets
|
||||
its `ToolEnd` as before, and the subagent gets `Status Exited`.
|
||||
4. When the parent session's process exits (`Status Exited` on the
|
||||
3. **The parent's `tool_result` never finishes a subagent.** The Task tool
|
||||
runs in the background by default: the `tool_result` -- "Async agent
|
||||
launched..." -- arrives the moment it *starts*, while the subagent goes
|
||||
on working for however long its own turn takes, sometimes minutes. What
|
||||
ends it is its own turn ending: the raw API's `message_delta` on its
|
||||
stream carrying `stop_reason: "end_turn"` (a `stop_reason` of `tool_use`
|
||||
is the model about to call one, not an end), or a `result` line for its
|
||||
own turn if a future CLI version ever sends one. Either maps to
|
||||
`Status Exited`; the subagent's vocabulary has no `Idle`, so the
|
||||
equivalent event `dispatch` produces for an ordinary session is dropped
|
||||
rather than written. A shipped version of this finished on the
|
||||
`tool_result` instead, which read a running background agent as
|
||||
"finished" with its transcript truncated at the moment it launched.
|
||||
4. **A child line for a subagent that already finished reopens it**
|
||||
(`Status Running`) rather than being dropped: a background Task can be
|
||||
sent another message long after its first turn ended, and that is
|
||||
exactly what a further line for it means. Same transcript, same child
|
||||
`Translator`, just picking back up.
|
||||
5. When the parent session's process exits (`Status Exited` on the
|
||||
session), every subagent still `Running` gets `Status Exited` too: its
|
||||
process was the parent's.
|
||||
|
||||
A subagent that was mid-flight when the backend restarted keeps working:
|
||||
the registry reopens the existing transcript on the next child line, and
|
||||
the file continues its sequence. If its Task call finished while the backend
|
||||
was down nothing ever closes it -- its last status stays `Running`, which
|
||||
the list reports as **unknown** rather than as running (see the wire shape).
|
||||
the file continues its sequence -- the same reopening #4 describes, whether
|
||||
what closed it was a restart or its own `end_turn`. If its turn ended while
|
||||
the backend was down nothing recorded that until the next line arrives, so
|
||||
its last status stays `Running`, which the list reports as **unknown**
|
||||
rather than as running (see the wire shape) until then.
|
||||
|
||||
Title: the Task call's `description` input, then ` (<subagent_type>)` when
|
||||
one is given; falling back to the tool's name when the child arrives before
|
||||
@@ -71,7 +89,7 @@ one is given; falling back to the tool's name when the child arrives before
|
||||
- `session/subagent.rs` -- the registry: `Subagents` (per session, in
|
||||
`Shared`), `Subagent` (its `Transcript` behind a mutex plus a
|
||||
`broadcast::Sender<SeqEvent>`), `record(id, event)`, `start(id, title,
|
||||
prompt)`, `finish(id)`, `finish_all()`, `list()` from disk. Drivers get an
|
||||
prompt)`, `finish(id)`, `reopen(id)`, `finish_all()`, `list()` from disk. Drivers get an
|
||||
`Arc<Subagents>` beside their `EventSink`; llama ignores it.
|
||||
- `session/claude/translate.rs` -- routes child lines by parent id, holds
|
||||
one child `Translator` per subagent, remembers pending Task calls'
|
||||
|
||||
@@ -11,6 +11,7 @@ import androidx.compose.foundation.layout.Spacer
|
||||
import androidx.compose.foundation.layout.fillMaxSize
|
||||
import androidx.compose.foundation.layout.fillMaxWidth
|
||||
import androidx.compose.foundation.layout.height
|
||||
import androidx.compose.foundation.layout.heightIn
|
||||
import androidx.compose.foundation.layout.padding
|
||||
import androidx.compose.foundation.layout.width
|
||||
import androidx.compose.foundation.lazy.LazyColumn
|
||||
@@ -396,10 +397,15 @@ private fun SessionCard(
|
||||
// already on screen -- see UI_RULES on a control not displacing the text beside it.
|
||||
if (session.subagents > 0) {
|
||||
Spacer(Modifier.height(8.dp))
|
||||
// The platform's minimum touch height, not the chevron's own ten or so dp:
|
||||
// at the chevron's height a tap meant for it landed on the first subcard
|
||||
// beneath and opened a subagent instead.
|
||||
Row(
|
||||
horizontalArrangement = Arrangement.Center,
|
||||
verticalAlignment = Alignment.CenterVertically,
|
||||
modifier =
|
||||
Modifier.fillMaxWidth()
|
||||
.heightIn(min = 48.dp)
|
||||
.clickable(enabled = !deleting, onClick = onToggleSubagents)
|
||||
.semantics {
|
||||
contentDescription =
|
||||
|
||||
@@ -151,10 +151,12 @@ impl Translator {
|
||||
fn translate_child(&mut self, id: &str, message: &Value) -> Vec<Event> {
|
||||
match self.subagents.get(id) {
|
||||
Some(subagent) if !subagent.is_open() => {
|
||||
// The Task call already ended (or this line is stale from a
|
||||
// resumed conversation) -- see `SUBAGENTS.md`'s lifecycle.
|
||||
tracing::debug!("dropping a line for subagent {id}, which has already finished");
|
||||
return Vec::new();
|
||||
// Not stale: the Task tool runs in the background by
|
||||
// default, so a finished subagent can still be sent another
|
||||
// message later (SendMessage) and start working again. A
|
||||
// line arriving after `finish` means exactly that, not a
|
||||
// conversation that is over -- see `SUBAGENTS.md`.
|
||||
self.subagents.reopen(id);
|
||||
}
|
||||
Some(_) => {}
|
||||
None => {
|
||||
@@ -178,8 +180,29 @@ impl Translator {
|
||||
.clone();
|
||||
let events = child.lock().unwrap().dispatch(message);
|
||||
for event in events {
|
||||
// The subagent's own vocabulary is Running/Exited/Unknown, never
|
||||
// Idle -- a background Task is either working or it has ended,
|
||||
// never merely "between turns" the way a session is. Dropped
|
||||
// here rather than never produced, so a `result` line's own
|
||||
// `Idle` (dispatch's ordinary end-of-turn event, for a subagent
|
||||
// dialect that ever sends one) is caught the same way a
|
||||
// `message_delta` would be.
|
||||
if !matches!(
|
||||
event,
|
||||
Event::Status {
|
||||
state: SessionStatus::Idle
|
||||
}
|
||||
) {
|
||||
self.subagents.record(id, event);
|
||||
}
|
||||
}
|
||||
// What actually ends a subagent's turn: not the parent's
|
||||
// `tool_result`, which for a background Task arrives at launch
|
||||
// ("Async agent launched...") long before the work is done -- see
|
||||
// `SUBAGENTS.md`.
|
||||
if ends_a_turn(message) {
|
||||
self.subagents.finish(id);
|
||||
}
|
||||
Vec::new()
|
||||
}
|
||||
|
||||
@@ -678,11 +701,11 @@ impl Translator {
|
||||
id: about.clone(),
|
||||
output: texts.join("\n"),
|
||||
});
|
||||
// A no-op unless `about` is a subagent's own id -- see
|
||||
// `SUBAGENTS.md`'s lifecycle #3: the parent gets this `ToolEnd`
|
||||
// like any other tool result, and the subagent it names (if it
|
||||
// names one) gets its `Status::Exited`.
|
||||
self.subagents.finish(&about);
|
||||
// Deliberately does *not* finish a subagent `about` might name:
|
||||
// the Task tool runs in the background by default, so this
|
||||
// `tool_result` -- "Async agent launched..." -- arrives at
|
||||
// launch, long before the subagent's own work is done. What
|
||||
// ends it is its own turn ending, handled in `translate_child`.
|
||||
}
|
||||
events
|
||||
}
|
||||
@@ -705,6 +728,32 @@ fn fallback_title(message: &Value) -> String {
|
||||
.to_string()
|
||||
}
|
||||
|
||||
/// Whether this line is a subagent's *own* turn ending -- the only thing
|
||||
/// that does, per `SUBAGENTS.md`: not the parent's `tool_result`, which for
|
||||
/// a background Task arrives at launch rather than at completion.
|
||||
///
|
||||
/// Checked on the raw line rather than on what `dispatch` returns, so this
|
||||
/// never has to touch the shared `translate_stream_event`/`dispatch` code a
|
||||
/// top-level session's own turn-ending also goes through -- a subagent's
|
||||
/// idea of "ended" must not change when a real session's does.
|
||||
///
|
||||
/// `message_delta` is the raw API's own signal, carrying the stop reason:
|
||||
/// `end_turn` is genuinely done, `tool_use` means the model is about to call
|
||||
/// one and there is more coming. A `result` line is the CLI's own shape for
|
||||
/// a top-level turn; a subagent has not been observed to send one, but
|
||||
/// SUBAGENTS.md counts it too in case a future CLI version does.
|
||||
fn ends_a_turn(message: &Value) -> bool {
|
||||
match message.get("type").and_then(Value::as_str) {
|
||||
Some("stream_event") => {
|
||||
let event = &message["event"];
|
||||
event.get("type").and_then(Value::as_str) == Some("message_delta")
|
||||
&& event["delta"].get("stop_reason").and_then(Value::as_str) == Some("end_turn")
|
||||
}
|
||||
Some("result") => true,
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether a failed turn failed because the account is out of quota, and when
|
||||
/// the CLI said the limit lifts.
|
||||
///
|
||||
@@ -1043,7 +1092,13 @@ mod tests {
|
||||
/// The parent's `tool_result` for the Task id is what ends the
|
||||
/// subagent -- SUBAGENTS.md's lifecycle #3 -- and nothing else does.
|
||||
#[test]
|
||||
fn the_parents_tool_result_finishes_the_subagent() {
|
||||
fn the_parents_tool_result_does_not_finish_the_subagent() {
|
||||
// The Task tool runs in the background by default: this
|
||||
// `tool_result` is "Async agent launched...", arriving the moment
|
||||
// the subagent *starts*, while it goes on working for however long
|
||||
// its own turn takes. Finishing it here was the bug -- a running
|
||||
// background agent read as "finished" with its transcript truncated
|
||||
// at launch.
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let subagents = test_subagents(&dir);
|
||||
let mut translator = Translator::new(dir.path().to_path_buf(), Arc::clone(&subagents));
|
||||
@@ -1058,10 +1113,115 @@ mod tests {
|
||||
translate_lines(
|
||||
&mut translator,
|
||||
&[
|
||||
r#"{"type":"user","message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"toolu_task2","content":"done","is_error":false}]},"parent_tool_use_id":null}"#,
|
||||
r#"{"type":"user","message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"toolu_task2","content":"Async agent launched","is_error":false}]},"parent_tool_use_id":null}"#,
|
||||
],
|
||||
);
|
||||
assert!(subagent.is_open());
|
||||
}
|
||||
|
||||
/// What actually ends a subagent: the raw API's own `message_delta`
|
||||
/// saying its turn stopped with `end_turn`. Never written into the
|
||||
/// subagent's own transcript as `Idle` -- its vocabulary has no such
|
||||
/// state.
|
||||
#[test]
|
||||
fn the_subagents_own_end_turn_finishes_it() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let subagents = test_subagents(&dir);
|
||||
let mut translator = Translator::new(dir.path().to_path_buf(), Arc::clone(&subagents));
|
||||
translate_lines(
|
||||
&mut translator,
|
||||
&[
|
||||
r#"{"type":"assistant","message":{"content":[{"type":"tool_use","id":"toolu_task3","name":"Task","input":{"description":"helper"}}]},"parent_tool_use_id":null}"#,
|
||||
r#"{"type":"stream_event","event":{"type":"message_delta","delta":{"stop_reason":"end_turn"}},"parent_tool_use_id":"toolu_task3"}"#,
|
||||
],
|
||||
);
|
||||
let subagent = subagents.get("toolu_task3").expect("subagent started");
|
||||
assert!(!subagent.is_open());
|
||||
let lines = crate::session::transcript::read_after(&subagent.transcript_path(), 0)
|
||||
.expect("read subagent transcript");
|
||||
assert!(
|
||||
!lines
|
||||
.iter()
|
||||
.any(|entry| matches!(&entry.event, Event::Status { state } if *state == SessionStatus::Idle)),
|
||||
"a subagent's transcript must never carry Idle: {lines:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
lines.last().unwrap().event,
|
||||
Event::Status {
|
||||
state: SessionStatus::Exited
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
/// `stop_reason: "tool_use"` is the model about to call a tool, with
|
||||
/// more of the turn still coming -- not an end.
|
||||
#[test]
|
||||
fn a_stop_reason_of_tool_use_does_not_finish_the_subagent() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let subagents = test_subagents(&dir);
|
||||
let mut translator = Translator::new(dir.path().to_path_buf(), Arc::clone(&subagents));
|
||||
translate_lines(
|
||||
&mut translator,
|
||||
&[
|
||||
r#"{"type":"assistant","message":{"content":[{"type":"tool_use","id":"toolu_task4","name":"Task","input":{"description":"helper"}}]},"parent_tool_use_id":null}"#,
|
||||
r#"{"type":"stream_event","event":{"type":"message_delta","delta":{"stop_reason":"tool_use"}},"parent_tool_use_id":"toolu_task4"}"#,
|
||||
],
|
||||
);
|
||||
assert!(
|
||||
subagents
|
||||
.get("toolu_task4")
|
||||
.expect("subagent started")
|
||||
.is_open()
|
||||
);
|
||||
}
|
||||
|
||||
/// A background Task can be sent another message long after its first
|
||||
/// turn ended -- a further child line for it reopens rather than being
|
||||
/// dropped, and the same transcript and child translator carry on.
|
||||
#[test]
|
||||
fn a_line_after_finish_reopens_the_subagent_rather_than_being_dropped() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let subagents = test_subagents(&dir);
|
||||
let mut translator = Translator::new(dir.path().to_path_buf(), Arc::clone(&subagents));
|
||||
translate_lines(
|
||||
&mut translator,
|
||||
&[
|
||||
r#"{"type":"assistant","message":{"content":[{"type":"tool_use","id":"toolu_task5","name":"Task","input":{"description":"helper"}}]},"parent_tool_use_id":null}"#,
|
||||
r#"{"type":"stream_event","event":{"type":"message_delta","delta":{"stop_reason":"end_turn"}},"parent_tool_use_id":"toolu_task5"}"#,
|
||||
],
|
||||
);
|
||||
let subagent = subagents.get("toolu_task5").expect("subagent started");
|
||||
assert!(!subagent.is_open());
|
||||
|
||||
translate_lines(
|
||||
&mut translator,
|
||||
&[
|
||||
r#"{"type":"assistant","message":{"content":[{"type":"tool_use","id":"toolu_more","name":"Bash","input":{}}]},"parent_tool_use_id":"toolu_task5"}"#,
|
||||
],
|
||||
);
|
||||
assert!(subagent.is_open());
|
||||
let lines = crate::session::transcript::read_after(&subagent.transcript_path(), 0)
|
||||
.expect("read subagent transcript");
|
||||
// Running, [prompt], Exited, Running (reopened), then the new line's
|
||||
// own ToolStart -- the same transcript throughout, not a new one.
|
||||
assert!(
|
||||
lines.iter().any(
|
||||
|entry| matches!(&entry.event, Event::ToolStart { tool, .. } if tool == "Bash")
|
||||
)
|
||||
);
|
||||
assert_eq!(
|
||||
lines
|
||||
.iter()
|
||||
.filter(|entry| matches!(
|
||||
&entry.event,
|
||||
Event::Status {
|
||||
state: SessionStatus::Running
|
||||
}
|
||||
))
|
||||
.count(),
|
||||
2,
|
||||
"expected one Running at creation and one at the reopen: {lines:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Two subagents running at once keep two separate transcripts: tool ids
|
||||
|
||||
@@ -89,9 +89,9 @@ impl Subagent {
|
||||
}
|
||||
|
||||
/// Whether this subagent's last recorded status is not `Exited` --
|
||||
/// what decides whether a further child line still belongs in its
|
||||
/// transcript. See `SUBAGENTS.md`'s lifecycle: "a child line whose
|
||||
/// subagent finished already... is ignored".
|
||||
/// what decides whether a further child line reopens it (see
|
||||
/// `Subagents::reopen`) rather than continuing straight through. See
|
||||
/// `SUBAGENTS.md`'s lifecycle.
|
||||
pub fn is_open(&self) -> bool {
|
||||
*self.status.lock().unwrap() != SessionStatus::Exited
|
||||
}
|
||||
@@ -269,10 +269,12 @@ impl Subagents {
|
||||
}
|
||||
}
|
||||
|
||||
/// The parent's `tool_result` for this Task id arrived: the subagent's
|
||||
/// own `Status::Exited`. A no-op for an id that is not a subagent's, so
|
||||
/// callers can call this for every `tool_result` without first checking
|
||||
/// whether it belongs to one.
|
||||
/// The subagent's own turn ended: its `Status::Exited`. Called from
|
||||
/// `translate_child` on the subagent's own `end_turn`, never on the
|
||||
/// parent's `tool_result` -- a background Task's `tool_result` arrives
|
||||
/// at launch, not at completion, so it says nothing about whether this
|
||||
/// is over. A no-op for an id that is not a subagent's or is already
|
||||
/// closed.
|
||||
pub fn finish(&self, id: &str) {
|
||||
if let Some(subagent) = self.live.lock().unwrap().get(id).cloned()
|
||||
&& subagent.is_open()
|
||||
@@ -283,6 +285,21 @@ impl Subagents {
|
||||
}
|
||||
}
|
||||
|
||||
/// A line arrived for a subagent that had already finished: it is
|
||||
/// working again, not stale -- a background Task can be sent another
|
||||
/// message long after its first turn ended. Appends `Status::Running`
|
||||
/// so the list stops reporting it as finished; a no-op if it was not
|
||||
/// actually closed, so a caller need not check first.
|
||||
pub fn reopen(&self, id: &str) {
|
||||
if let Some(subagent) = self.live.lock().unwrap().get(id).cloned()
|
||||
&& !subagent.is_open()
|
||||
{
|
||||
subagent.append(Event::Status {
|
||||
state: SessionStatus::Running,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/// The parent session's process is gone, so nothing still open here has
|
||||
/// a process behind it either -- see `SUBAGENTS.md`'s lifecycle #4.
|
||||
pub fn finish_all(&self) {
|
||||
|
||||
Reference in new issue
Block a user