Never run two turns into one, and say when a session waits on its own work
A turn started by something with no row of its own -- a subagent reporting back, a peer message the CLI only owns up to at the end -- met the previous reply with nothing between it, and the fold grew that reply rather than starting a new one. Two answers were drawn as one paragraph, running together mid-sentence with not even a space between them. The fold now refuses to grow a settled reply, and `joinPages` carries the same rule across a page boundary. The other half is the row. `Event::TaskNote` records a background task reporting back -- a subagent that finished, or a backgrounded command -- with its title, how it ended and what it said; `TaskNoteRow` draws it as a card, since somebody said this, and its own row rather than an update to the Task call's, which is above everything the session has said since. Reported once however many of the CLI's two lifecycle shapes arrive. `SessionStatus::Waiting` is a session whose own turn is over while work it started is not. `Idle` means "waiting for a person" and this means the opposite, so reporting it as idle sent a "finished" notification at the one moment that was untrue. Drawn as "waiting" in `waitingColor`; the queue and the held-command boundary release on either end-of-turn status, so a message sent while a subagent runs is not held until it finishes. And a usage limit the account hits inside a subagent now reaches the session as well as the subagent's transcript. `resume.rs` can only schedule against a session, and a background Task outliving its parent's turn is the ordinary case, so auto-resume was doing nothing at all for it. The status word and its colour were two `when`s on two screens, and the second missed `waiting` silently; they are `sessionStatusWord`/`sessionStatusColour` now. Echo's `/subagent n` reproduces the whole shape, staggered a second apart. Verified on the emulator against the sandbox: 169 server tests, ktfmt, clippy, rustfmt, Android lint and the JVM unit tests all clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
74c07d687a
commit
5711c2568a
17 files changed
+891
-85
No files matched your search
@@ -108,12 +108,29 @@ pub(super) struct Translator {
|
||||
/// that only the change into that state is reported -- see
|
||||
/// [`Translator::translate_rate_limit`].
|
||||
rate_limited: bool,
|
||||
/// Which Task call each running task belongs to: the CLI's `task_id`
|
||||
/// Which Task call each *unfinished* task belongs to: the CLI's `task_id`
|
||||
/// against the `tool_use_id` this side names a subagent by.
|
||||
///
|
||||
/// Needed because the line that says a task *ended* comes in two shapes
|
||||
/// and only one of them carries the tool id -- see [`Translator::translate_task`].
|
||||
/// Needed because the line that says a task ended comes in two shapes and
|
||||
/// only one of them carries the tool id -- see [`Translator::translate_task`].
|
||||
///
|
||||
/// Emptied entry by entry as tasks report back, which makes it the answer
|
||||
/// to two further questions: whether an ending has already been reported
|
||||
/// (the two shapes can both arrive for one task, and the row belongs in
|
||||
/// the transcript once), and whether the session still has work outstanding
|
||||
/// when its own turn ends, which is the difference between `Idle` and
|
||||
/// [`SessionStatus::Waiting`].
|
||||
tasks: HashMap<String, String>,
|
||||
/// Whether a turn is open, judged from this translator's own output: the
|
||||
/// events that [`super::proves_a_turn`] accepts open one, and the status
|
||||
/// that ends a turn closes it.
|
||||
///
|
||||
/// The same rule the driver uses to announce `Running`, read from the same
|
||||
/// side of the translation, because two rules for "is a turn running"
|
||||
/// would disagree the first time either moved. What it decides here is
|
||||
/// narrow: whether a task reporting back means the session has gone idle,
|
||||
/// or is merely one of several things happening inside a turn.
|
||||
in_turn: bool,
|
||||
}
|
||||
|
||||
impl Translator {
|
||||
@@ -129,6 +146,7 @@ impl Translator {
|
||||
children: HashMap::new(),
|
||||
rate_limited: false,
|
||||
tasks: HashMap::new(),
|
||||
in_turn: false,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -191,22 +209,29 @@ impl Translator {
|
||||
})
|
||||
.clone();
|
||||
let events = child.lock().unwrap().dispatch(message);
|
||||
// A limit the account hit while this subagent was working. It belongs
|
||||
// in the subagent's transcript, which is where it happened -- and it
|
||||
// also has to reach the session, which is the only thing auto-resume
|
||||
// can schedule against: a background subagent can run on long after
|
||||
// its parent's own turn ended, so "the main agent was idle when the
|
||||
// limit hit" is the ordinary case rather than an edge of one, and
|
||||
// swallowing it here left that session waiting for a person for ever.
|
||||
let mut hoisted = Vec::new();
|
||||
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
|
||||
// Idle or Waiting -- 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 end-of-turn status (dispatch's ordinary one, 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);
|
||||
if closes_a_turn(&event) {
|
||||
continue;
|
||||
}
|
||||
if matches!(event, Event::LimitReached { .. }) {
|
||||
hoisted.push(event.clone());
|
||||
}
|
||||
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
|
||||
@@ -215,10 +240,26 @@ impl Translator {
|
||||
if ends_a_turn(message) {
|
||||
self.subagents.finish(id);
|
||||
}
|
||||
Vec::new()
|
||||
hoisted
|
||||
}
|
||||
|
||||
/// One line of this translator's own session, with [`Translator::in_turn`]
|
||||
/// kept up to date from what came out of it.
|
||||
///
|
||||
/// Here rather than in each arm because it has to hold for every line
|
||||
/// there is: the set of events that prove a turn is running is
|
||||
/// [`super::proves_a_turn`]'s, and no arm should have to remember it.
|
||||
fn dispatch(&mut self, message: &Value) -> Vec<Event> {
|
||||
let events = self.translate_line(message);
|
||||
if events.iter().any(closes_a_turn) {
|
||||
self.in_turn = false;
|
||||
} else if events.iter().any(super::proves_a_turn) {
|
||||
self.in_turn = true;
|
||||
}
|
||||
events
|
||||
}
|
||||
|
||||
fn translate_line(&mut self, message: &Value) -> Vec<Event> {
|
||||
match message.get("type").and_then(Value::as_str) {
|
||||
Some("system") => self.translate_system(message),
|
||||
// The CLI's own announcement that `/clear` took effect, sent just
|
||||
@@ -327,8 +368,17 @@ impl Translator {
|
||||
if tokens > 0 {
|
||||
events.push(Event::UsageDelta { tokens, context });
|
||||
}
|
||||
// Idle means "waiting for a person", and a session with a
|
||||
// backgrounded subagent or command still running is not doing
|
||||
// that -- it is waiting for itself, and will speak again with
|
||||
// nobody having typed anything. Reported as what it is, so
|
||||
// that nothing tells the reader the work has finished.
|
||||
events.push(Event::Status {
|
||||
state: SessionStatus::Idle,
|
||||
state: if self.tasks.is_empty() {
|
||||
SessionStatus::Idle
|
||||
} else {
|
||||
SessionStatus::Waiting
|
||||
},
|
||||
});
|
||||
events
|
||||
}
|
||||
@@ -477,20 +527,18 @@ impl Translator {
|
||||
if let (Some(task), Some(tool)) = (task_id, tool_use_id) {
|
||||
self.tasks.insert(task.to_string(), tool.to_string());
|
||||
}
|
||||
// The tool call this line is about, from the line itself or from
|
||||
// whichever earlier line did carry it.
|
||||
let about = tool_use_id
|
||||
.map(str::to_string)
|
||||
.or_else(|| task_id.and_then(|task| self.tasks.get(task).cloned()));
|
||||
match message.get("subtype").and_then(Value::as_str) {
|
||||
Some("task_notification") => {
|
||||
let Some(id) = tool_use_id else {
|
||||
return Vec::new();
|
||||
};
|
||||
if !ended(message.get("status").and_then(Value::as_str)) {
|
||||
return Vec::new();
|
||||
}
|
||||
if let Some(summary) = text_field(message, "summary") {
|
||||
self.subagents
|
||||
.record(id, Event::AssistantText { delta: summary });
|
||||
}
|
||||
self.subagents.finish(id);
|
||||
}
|
||||
Some("task_notification") => self.task_ended(
|
||||
task_id,
|
||||
about,
|
||||
message.get("status").and_then(Value::as_str),
|
||||
text_field(message, "summary"),
|
||||
),
|
||||
Some("task_updated") => {
|
||||
let status = message
|
||||
.get("patch")
|
||||
@@ -504,21 +552,81 @@ impl Translator {
|
||||
// failed or was cancelled has not been observed here, and the
|
||||
// failure to avoid is the one this whole function exists for:
|
||||
// a subagent that nothing ever finishes.
|
||||
if status != Some("completed")
|
||||
&& ended(status)
|
||||
&& let Some(id) = task_id.and_then(|task| self.tasks.get(task))
|
||||
{
|
||||
let id = id.clone();
|
||||
self.subagents.finish(&id);
|
||||
if status == Some("completed") {
|
||||
return Vec::new();
|
||||
}
|
||||
self.task_ended(task_id, about, status, None)
|
||||
}
|
||||
// `task_started` and `task_progress`: the mapping above is the
|
||||
// whole of what they are for. The subagent itself is created by
|
||||
// the Task `tool_use` in the parent's own message, which arrives
|
||||
// first and carries the title this side shows.
|
||||
_ => {}
|
||||
_ => Vec::new(),
|
||||
}
|
||||
Vec::new()
|
||||
}
|
||||
|
||||
/// A task reporting back, from whichever of the two lines got here first.
|
||||
///
|
||||
/// Reported once. The two shapes can both arrive for one task, and the
|
||||
/// [`Translator::tasks`] entry is what says which of them is the first --
|
||||
/// it is removed here, so a second line for the same task finds nothing
|
||||
/// and says nothing. That is also what stops a task being counted as
|
||||
/// outstanding for ever.
|
||||
///
|
||||
/// Three things come out of it, and the third is the one that is easy to
|
||||
/// leave out. The summary goes into the subagent's own transcript, which
|
||||
/// is the only place its closing words ever appear; the subagent is
|
||||
/// finished; and the *parent* gets an [`Event::TaskNote`], because a
|
||||
/// message arriving is something that happened to this session and the
|
||||
/// turn it wakes up and runs would otherwise begin with nothing in front
|
||||
/// of it.
|
||||
fn task_ended(
|
||||
&mut self,
|
||||
task_id: Option<&str>,
|
||||
about: Option<String>,
|
||||
status: Option<&str>,
|
||||
summary: Option<String>,
|
||||
) -> Vec<Event> {
|
||||
if !ended(status) {
|
||||
return Vec::new();
|
||||
}
|
||||
let Some(about) = about else {
|
||||
return Vec::new();
|
||||
};
|
||||
// Nothing under that id: either this task has already been reported,
|
||||
// or its `task_started` was never seen. Both are "say nothing"; the
|
||||
// first would be a duplicate row and the second a row for a task this
|
||||
// translator cannot say anything about.
|
||||
if task_id.is_none_or(|task| self.tasks.remove(task).is_none()) {
|
||||
return Vec::new();
|
||||
}
|
||||
if let Some(summary) = &summary {
|
||||
self.subagents.record(
|
||||
&about,
|
||||
Event::AssistantText {
|
||||
delta: summary.clone(),
|
||||
},
|
||||
);
|
||||
}
|
||||
self.subagents.finish(&about);
|
||||
let mut events = vec![Event::TaskNote {
|
||||
title: self.subagents.title_of(&about),
|
||||
about,
|
||||
// Present by construction: `ended` says no to a line with no
|
||||
// status at all.
|
||||
status: status.unwrap_or_default().to_string(),
|
||||
summary,
|
||||
}];
|
||||
// The last outstanding task, with the session's own turn already
|
||||
// over: it has stopped being `Waiting` and nothing else will say so.
|
||||
// Inside a turn there is nothing to announce -- the turn's own
|
||||
// `result` will decide between the two statuses when it lands.
|
||||
if self.tasks.is_empty() && !self.in_turn {
|
||||
events.push(Event::Status {
|
||||
state: SessionStatus::Idle,
|
||||
});
|
||||
}
|
||||
events
|
||||
}
|
||||
|
||||
/// A null `status` is the leaving edge, and it carries how the thing went.
|
||||
@@ -894,6 +1002,21 @@ fn is_known_refusal(status: &str) -> bool {
|
||||
matches!(status, "rejected" | "blocked" | "exceeded" | "limited")
|
||||
}
|
||||
|
||||
/// Whether this event is the end of a turn: the two statuses a turn can
|
||||
/// finish in, and no others.
|
||||
///
|
||||
/// A sibling of [`super::proves_a_turn`] and deliberately shaped like it --
|
||||
/// see [`Translator::in_turn`]. `Exited` is not here: a process that has gone
|
||||
/// ends the session rather than the turn, and nothing after it can start one.
|
||||
fn closes_a_turn(event: &Event) -> bool {
|
||||
matches!(
|
||||
event,
|
||||
Event::Status {
|
||||
state: SessionStatus::Idle | SessionStatus::Waiting
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
/// Whether a task status word means the task is over.
|
||||
///
|
||||
/// Written as "not one of the words that mean it is still going" rather than
|
||||
@@ -1286,6 +1409,143 @@ mod tests {
|
||||
assert!(subagent.is_open());
|
||||
}
|
||||
|
||||
/// The end of a turn is not the end of the work when the session
|
||||
/// backgrounded something, and `Idle` says it is. Everything that reads a
|
||||
/// status hangs off this: the phone's word for the row, whether a
|
||||
/// "finished" notification goes out, and what auto-resume is looking at.
|
||||
#[test]
|
||||
fn a_turn_that_ends_with_a_task_still_running_is_waiting_rather_than_idle() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let subagents = test_subagents(&dir);
|
||||
let mut translator = Translator::new(dir.path().to_path_buf(), Arc::clone(&subagents));
|
||||
let result = r#"{"type":"result","subtype":"success","is_error":false,"usage":{}}"#;
|
||||
translate_lines(
|
||||
&mut translator,
|
||||
&[
|
||||
r#"{"type":"assistant","message":{"content":[{"type":"tool_use","id":"toolu_bg","name":"Task","input":{"description":"the Dev Updater agent"}}]},"parent_tool_use_id":null}"#,
|
||||
r#"{"type":"system","subtype":"task_started","task_id":"t1","tool_use_id":"toolu_bg","is_backgrounded":true}"#,
|
||||
],
|
||||
);
|
||||
assert_eq!(
|
||||
translate_lines(&mut translator, &[result]).last(),
|
||||
Some(&Event::Status {
|
||||
state: SessionStatus::Waiting
|
||||
})
|
||||
);
|
||||
|
||||
// The task reports back: the message the session received, and only
|
||||
// now the session is genuinely waiting for a person.
|
||||
assert_eq!(
|
||||
translate_lines(
|
||||
&mut translator,
|
||||
&[
|
||||
r#"{"type":"system","subtype":"task_notification","task_id":"t1","tool_use_id":"toolu_bg","status":"completed","summary":"pushed as c41c36f"}"#,
|
||||
],
|
||||
),
|
||||
vec![
|
||||
Event::TaskNote {
|
||||
about: "toolu_bg".into(),
|
||||
title: Some("the Dev Updater agent".into()),
|
||||
status: "completed".into(),
|
||||
summary: Some("pushed as c41c36f".into()),
|
||||
},
|
||||
Event::Status {
|
||||
state: SessionStatus::Idle
|
||||
},
|
||||
]
|
||||
);
|
||||
|
||||
// Reported once. The two lifecycle shapes can both arrive for one
|
||||
// task, and a second row for it would be a message that never came.
|
||||
assert!(
|
||||
translate_lines(
|
||||
&mut translator,
|
||||
&[
|
||||
r#"{"type":"system","subtype":"task_updated","task_id":"t1","patch":{"status":"failed"}}"#,
|
||||
],
|
||||
)
|
||||
.is_empty()
|
||||
);
|
||||
|
||||
// And with nothing outstanding, the next turn ends idle as before.
|
||||
assert_eq!(
|
||||
translate_lines(&mut translator, &[result]).last(),
|
||||
Some(&Event::Status {
|
||||
state: SessionStatus::Idle
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
/// A task ending *inside* a turn says nothing about the session's status:
|
||||
/// the turn is still running, and its own `result` decides. Without the
|
||||
/// `in_turn` guard this reported the session idle in the middle of one,
|
||||
/// which releases the message queue and tells every phone the work is
|
||||
/// over.
|
||||
#[test]
|
||||
fn a_task_ending_during_a_turn_does_not_report_the_session_idle() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let subagents = test_subagents(&dir);
|
||||
let mut translator = Translator::new(dir.path().to_path_buf(), Arc::clone(&subagents));
|
||||
let events = translate_lines(
|
||||
&mut translator,
|
||||
&[
|
||||
r#"{"type":"assistant","message":{"content":[{"type":"tool_use","id":"toolu_fg","name":"Task","input":{"description":"a helper"}}]},"parent_tool_use_id":null}"#,
|
||||
r#"{"type":"system","subtype":"task_started","task_id":"t2","tool_use_id":"toolu_fg","is_backgrounded":false}"#,
|
||||
r#"{"type":"stream_event","event":{"type":"content_block_delta","index":0,"delta":{"type":"text_delta","text":"still going"}},"parent_tool_use_id":null}"#,
|
||||
r#"{"type":"system","subtype":"task_notification","task_id":"t2","tool_use_id":"toolu_fg","status":"completed","summary":"done"}"#,
|
||||
],
|
||||
);
|
||||
assert!(
|
||||
!events.iter().any(closes_a_turn),
|
||||
"the turn has not ended: {events:?}"
|
||||
);
|
||||
assert!(
|
||||
events
|
||||
.iter()
|
||||
.any(|event| matches!(event, Event::TaskNote { .. }))
|
||||
);
|
||||
}
|
||||
|
||||
/// A background subagent can still be working long after its parent's own
|
||||
/// turn ended, so the account running out while one is mid-flight is the
|
||||
/// ordinary shape of the problem rather than an edge of it. The limit
|
||||
/// belongs in the subagent's transcript *and* has to reach the session,
|
||||
/// which is the only thing `crate::resume` can schedule against.
|
||||
#[test]
|
||||
fn a_limit_a_subagent_hits_reaches_the_session_as_well_as_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_lim","name":"Task","input":{"description":"a helper"}}]},"parent_tool_use_id":null}"#,
|
||||
],
|
||||
);
|
||||
let hit = translate_lines(
|
||||
&mut translator,
|
||||
&[
|
||||
r#"{"type":"result","subtype":"error","is_error":true,"result":"Claude usage limit reached|1788726600","usage":{},"parent_tool_use_id":"toolu_lim"}"#,
|
||||
],
|
||||
);
|
||||
assert_eq!(
|
||||
hit,
|
||||
vec![Event::LimitReached {
|
||||
resets_at: Some(1788726600.0)
|
||||
}],
|
||||
"the session has to hear about it, or nothing resumes"
|
||||
);
|
||||
let subagent = subagents.get("toolu_lim").expect("subagent started");
|
||||
let lines =
|
||||
crate::session::transcript::read_after(&subagent.transcript_path(), 0).expect("read");
|
||||
assert!(
|
||||
lines
|
||||
.iter()
|
||||
.any(|entry| matches!(entry.event, Event::LimitReached { .. })),
|
||||
"and so does the transcript it happened in: {lines:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// The second limit detector, on the shape the CLI actually sends. The
|
||||
/// `allowed` line is copied from a real 2.1.237 run on 2026-09-06; the
|
||||
/// refused one is the same line with the status changed, which is the
|
||||
|
||||
Reference in new issue
Block a user