diff --git a/AGENTS.md b/AGENTS.md index 38487c6..8d81866 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -241,12 +241,24 @@ sent while a subagent runs is not held until the subagent finishes; and `sessionWorking("waiting")` is deliberately **false** — nothing is being written, and the fold uses that same predicate to decide a reply is settled. -- **A task reporting back is a row** (`Event::TaskNote`, `TaskNoteRow`), and - the reply that answers it is a **new** message. The fold refuses to grow a - settled reply; without that, two turns with nothing recorded between them - were folded into one and ran together mid-sentence. `./ui-sandbox.sh` plus - `/subagent 8` in an echo session is the whole rig — the helpers stagger a - second apart so each report and the reply to it are legible. +- **A task reporting back is a divider** (`Event::TaskNote`, `TaskNoteRow`), + and the reply that answers it is a **new** message. The fold refuses to grow + a settled reply; without that, two turns with nothing recorded between them + were folded into one and ran together mid-sentence. The divider is **closed** + and does not say what the subagent said: that is recorded as the subagent's + own transcript's closing text and belongs there, not repeated in its + parent's. Opening it shows the report anyway, and a backgrounded *command* + — which has no transcript of its own — names itself from its summary and has + nothing left to open. `./ui-sandbox.sh` plus `/subagent 8` in an echo session + is the whole rig; the helpers stagger a second apart so each report and the + reply to it are legible. +- **Whether work is outstanding has two sources and needs both.** The + translator's `open_tasks` is what it watched start — the only thing that + knows about a backgrounded command — and `Subagents::any_open` reads the + directory, which is the only thing that knows about a subagent started + before this translator existed. That second one is every subagent a session + has when the backend is updated under it: adoption reads stdout from a + recorded offset, so those `task_started` lines are already behind it. - **A usage limit a subagent hits reaches the session**, not just the subagent's own transcript; auto-resume can only schedule against a session. That is the case where the main agent is idle and a background Task is diff --git a/PLAN.md b/PLAN.md index ab8a65b..7756dab 100644 --- a/PLAN.md +++ b/PLAN.md @@ -678,22 +678,35 @@ mid-sentence with not even a space between them. Both halves were wrong and both are fixed. The fold now refuses to grow a *settled* reply, so a turn boundary is always a message boundary whatever caused it (`joinPages` carries the same rule across a page boundary). And the -notification is recorded as `Event::TaskNote`, drawn as a card naming who -reported and what they said — a card rather than a divider, because 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 and would change where no reader -is looking. +notification is recorded as `Event::TaskNote`, on its own row rather than as an +update to the Task call's — that row is wherever the call was made, above +everything the session has said since, and it would change where no reader is +looking. + +**Drawn as a divider, closed, and not carrying the subagent's words.** It +marks a boundary, which is what the reader needs from it; the subagent's +closing report is recorded as that subagent's own transcript's closing text, +and repeating it in the parent puts the same paragraph in two places for +somebody who did not ask for it. Opening the divider shows it anyway, because +leaving the conversation to read one line has its own cost — and because a +backgrounded *command* has no transcript of its own, so this is the only place +its report exists at all. That one names itself from its summary and has +nothing left to open. `status` is carried beside `summary` rather than folded into it because the summary is absent exactly when things went wrong, and "finished" is the wrong -word for a task that was killed. `title` is the subagent's; a backgrounded -command has none and its summary names itself, so the card says "a background -task" rather than inventing one. +word for a task that was killed. Reported once. The two lifecycle shapes (`task_notification` and -`task_updated`) can both arrive for one task, and the translator's `tasks` map -is what says which got there first — removing the entry is also what stops the -task counting as outstanding, which is what decides `Status::Waiting`. +`task_updated`) can both arrive for one task, and whichever gets here first is +the one that finds the task open — in the translator's own `open_tasks`, or +failing that in the registry, which is what makes an **adopted** session work. +A backend restart picks a session's stdout back up from a recorded offset, so +the `task_started` lines for anything already running are behind it and the +translator never sees them; `Subagents::any_open` is the measurement that +covers those, and `open_tasks` covers the backgrounded command, which has no +subagent to be found in the registry at all. Both are needed and neither +subsumes the other. ### A limit a subagent hits is the session's (2026-09-06) diff --git a/SUBAGENTS.md b/SUBAGENTS.md index c0f391e..f10cbff 100644 --- a/SUBAGENTS.md +++ b/SUBAGENTS.md @@ -81,19 +81,22 @@ transcript is still being written to and its process is the session's to stop. `Waiting`, so the end-of-turn status `dispatch` produces for an ordinary session is dropped rather than written. - **The ending is also reported to the parent** (2026-09-06), as - `Event::TaskNote { about, title, status, summary }`: the notification is a - message the session received, and the turn it wakes up and runs would - otherwise begin with nothing in front of it -- which drew two replies as - one paragraph. Reported once however many of the two lifecycle shapes - arrive; the `task_id -> tool_use_id` entry is removed as it is reported, - which is what says the first one got there. See PLAN.md's "A task + **The ending is also marked in the parent** (2026-09-06), as + `Event::TaskNote { about, title, status, summary }`: the turn the session + wakes up and runs would otherwise begin with nothing in front of it, which + drew two replies as one paragraph. It is a **divider**, closed, and does + not repeat the summary -- that is this subagent's own closing text, and + here is not where somebody reads it. Reported once however many of the two + lifecycle shapes arrive: whichever gets there first is the one that finds + the task still open, and `finish` below closes it. See PLAN.md's "A task reporting back". **While any task is outstanding the session's turn ends in - `Status Waiting` rather than `Idle`** -- the same `tasks` map, asked - whether it is empty. `Idle` means "waiting for a person", and a session - with a backgrounded subagent is not doing that. + `Status Waiting` rather than `Idle`.** `Idle` means "waiting for a person", + and a session with a backgrounded subagent is not doing that. Two sources: + the translator's `open_tasks`, and `Subagents::any_open` -- which is what + covers a subagent launched before a backend restart adopted the session, + whose `task_started` is behind the offset its stdout is read from. **A limit the account hits inside a subagent is hoisted to the session** as well as recorded here, because `resume.rs` can only schedule against a diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/Dividers.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/Dividers.kt index 57600e1..f4c5113 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/Dividers.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/Dividers.kt @@ -26,9 +26,18 @@ import java.time.format.FormatStyle * two it was is said by the words and the colour. * * The rules take [color] too, so the whole divider reads as one mark of one kind. + * + * [trailing] is drawn inside the rules, beside the words -- the one divider that opens onto + * something needs its chevron there, and giving it its own copy of this layout is how the two would + * come to sit at different heights. */ @Composable -fun TranscriptDivider(text: String, color: Color, modifier: Modifier = Modifier) { +fun TranscriptDivider( + text: String, + color: Color, + modifier: Modifier = Modifier, + trailing: (@Composable () -> Unit)? = null, +) { Row( verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(8.dp), @@ -36,6 +45,7 @@ fun TranscriptDivider(text: String, color: Color, modifier: Modifier = Modifier) ) { HorizontalDivider(Modifier.weight(1f), color = color) Text(text, style = MaterialTheme.typography.bodySmall, color = color) + trailing?.invoke() HorizontalDivider(Modifier.weight(1f), color = color) } } diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt index a545cdd..28726e1 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -288,6 +288,10 @@ fun SessionScreen( // Which messages from other agents are open, by the seq that identifies their row. Closed by // default, which is the rule for anything new in this transcript. var expandedNotes by remember { mutableStateOf(setOf()) } + // Which task reports are open, by the seq that identifies their row. Closed by default, which + // is the rule for anything new in this transcript -- and doubly so here, since what one opens + // onto is already in the subagent's own transcript. + var expandedTaskNotes by remember { mutableStateOf(setOf()) } // Which memory notes are open, by the note's own text. Held here rather than in the card so a // note opened and scrolled past is still open on the way back. var openMemories by remember { mutableStateOf(setOf()) } @@ -1603,7 +1607,25 @@ fun SessionScreen( is TranscriptItem.CompactedNote -> CompactedRow(item) is TranscriptItem.LimitNote -> LimitRow(item) - is TranscriptItem.TaskNote -> TaskNoteRow(item) + is TranscriptItem.TaskNote -> + TaskNoteRow( + item, + open = item.seq in expandedTaskNotes, + // Anchored, so the edge the reader pressed + // stays where it was. + onToggle = { + toggleAnchored(row) { + expandedTaskNotes = + if ( + item.seq in + expandedTaskNotes + ) + expandedTaskNotes - item.seq + else + expandedTaskNotes + item.seq + } + }, + ) // Never reached: a peer message is flattened into // its own units. Here because a `when` over the // item kinds has to stay exhaustive. diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/TaskNote.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/TaskNote.kt index e48aac9..46de340 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/TaskNote.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/TaskNote.kt @@ -1,89 +1,110 @@ package com.example.aiapp +import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding -import androidx.compose.material3.CardDefaults import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier -import androidx.compose.ui.graphics.Color +import androidx.compose.ui.semantics.contentDescription +import androidx.compose.ui.semantics.semantics import androidx.compose.ui.unit.dp /** - * A background task reporting back: a subagent that finished, or a backgrounded command. + * The mark a background task reporting back leaves: a subagent that finished, or a backgrounded + * command. * - * A card rather than a divider, and for the same reason a peer message is one -- somebody said - * this. A divider is a fact about the conversation ("everything above is out of context"); this is - * a message that arrived, and the turn under it is the session answering it. + * A divider, drawn like a clear or a compaction, because what it marks is a *boundary*. The turn + * below it is the session answering something that arrived, and without a row there the reply that + * ended the previous turn and the reply that answers this one met with nothing between them -- the + * fold grew the older message and drew two answers as one paragraph, running together mid-sentence. * - * The card is also what separates the two turns. Before it existed, a reply woken by one of these - * met the previous reply with nothing between them and the transcript ran them into one paragraph, - * mid-sentence. The row being *there* is most of the fix; what it says is the rest. - * - * Drawn whole rather than in [cardPiece] slices, unlike a peer message: a summary is one sentence - * the CLI wrote, so there is no unbounded case to bound. If one ever arrives long enough to be - * worth splitting, it belongs in the same flatten a peer message goes through. + * **Closed, and the subagent's words are not what it says.** A subagent's closing report is + * recorded as its own transcript's closing text, which is where somebody who wants it looks; + * putting it in the parent by default is the same paragraph in two places for a reader who did not + * ask for it. Opened, this shows it anyway, because having to leave the conversation to read one + * line is its own cost -- and because a backgrounded *command* has no transcript of its own, so + * here is the only place its report exists at all. */ @Composable -fun TaskNoteRow(item: TranscriptItem.TaskNote, modifier: Modifier = Modifier) { +fun TaskNoteRow( + item: TranscriptItem.TaskNote, + open: Boolean, + onToggle: () -> Unit, + modifier: Modifier = Modifier, +) { + // Blue is the session's own background work -- the same thing `waiting` means in the status + // row, so "this is the session getting on with something it started" is learned once. Red only + // where something actually went wrong; a cancelled task is a choice somebody made. + val colour = if (item.status == "failed") failedColor else waitingColor + val line = taskNoteSummary(item.title, item.status, item.summary) + // Nothing behind the line: a task that ended without a word, or one whose report *is* the line + // already. A control that opens onto nothing, or onto a copy of what is above it, teaches the + // reader that the control means nothing. + val expandable = item.summary != null && item.summary != line Column( - modifier.cardPiece( - top = true, - bottom = true, - fill = CardDefaults.cardColors().containerColor, - ) + modifier + .fillMaxWidth() + .then(if (expandable) Modifier.clickable(onClick = onToggle) else Modifier) ) { - Text( - taskNoteHeading(item.title, item.status), - style = MaterialTheme.typography.titleSmall, - color = taskNoteColor(item.status), + TranscriptDivider( + line, + colour, + trailing = + if (!expandable) null + else { + { + Chevron( + if (open) Pointing.Up else Pointing.Down, + colour = colour, + // The row is the control and the chevron is all of its marking, so the + // name belongs here: it is the only thing a screen reader has to read. + modifier = + Modifier.semantics { + contentDescription = + if (open) "Hide the report" else "Show the report" + }, + ) + } + }, ) - if (item.summary != null) { + if (open && expandable) { + // Not the divider's colour: this is what the task said rather than a mark we drew, and + // colouring a quotation as if it were part of the rule around it makes the rule look + // like it is carrying some of the meaning. Text( - item.summary, + item.summary.orEmpty(), style = MaterialTheme.typography.bodyMedium, - modifier = Modifier.padding(top = 4.dp), + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(bottom = 8.dp), ) } } } /** - * What the card says: who reported, and how it went. + * What the divider says: what reported, and how it went. * * Its own function so the wording is testable without a screen, and because the case that decides * whether this is any good is the one nobody builds a screen for -- a task that failed or was - * killed. "Message from" is the right sentence for exactly one of the endings; using it for all of - * them would report a task that died as one that had something to say. + * killed. "Reported back" is the right phrase for exactly one of the endings; using it for all of + * them would announce a task that died as one that had something to say. * * A status word this build has never seen is said as itself rather than mapped onto the nearest - * one, since the nearest one would read as a decision somebody made. + * one, since the nearest one would read as a fact somebody established. */ -fun taskNoteHeading(title: String?, status: String): String { - // A backgrounded command has no title of its own -- its summary names it -- so the card says - // what it was rather than inventing a name for it. - val who = title ?: "a background task" +fun taskNoteSummary(title: String?, status: String, summary: String?): String { + // A backgrounded command has no title of its own and its summary is already a whole sentence -- + // `Background command "..." completed (exit code 0)`. Naming it from that beats "a background + // task", which says nothing, and there is nothing left behind the line to open. + if (title == null && summary != null && status == "completed") return summary + val who = title ?: "A background task" return when (status) { - "completed" -> "Message from $who" + "completed" -> "$who reported back" "failed" -> "$who failed" "cancelled" -> "$who was cancelled" else -> "$who: $status" } } - -/** - * The heading's colour: coloured only where something went wrong. - * - * A task that finished and said something is the ordinary case and takes the ordinary text colour; - * the accent is spent on the one ending a reader would want to find by scanning. Cancelled is - * neither -- somebody chose it, and a deliberate choice is not a problem to report -- and a word - * this build does not recognise is not coloured as a failure, because it is not one. It says - * itself, which is the difference in *kind* that no colour can carry. - */ -@Composable -private fun taskNoteColor(status: String): Color = - when (status) { - "failed" -> failedColor - else -> MaterialTheme.colorScheme.onSurface - } diff --git a/app/androidApp/src/test/kotlin/com/example/aiapp/TranscriptItemsTest.kt b/app/androidApp/src/test/kotlin/com/example/aiapp/TranscriptItemsTest.kt index e4843f2..7123cdf 100644 --- a/app/androidApp/src/test/kotlin/com/example/aiapp/TranscriptItemsTest.kt +++ b/app/androidApp/src/test/kotlin/com/example/aiapp/TranscriptItemsTest.kt @@ -83,14 +83,21 @@ class TranscriptItemsTest { assertEquals(listOf("One turn.", "The next."), texts(whole)) } - /** The endings nobody builds a screen for -- see [taskNoteHeading]. */ + /** The endings nobody builds a screen for -- see [taskNoteSummary]. */ @Test fun a_task_note_says_which_ending_it_was() { - assertEquals("Message from helper 1", taskNoteHeading("helper 1", "completed")) - assertEquals("Message from a background task", taskNoteHeading(null, "completed")) - assertEquals("helper 1 failed", taskNoteHeading("helper 1", "failed")) - assertEquals("helper 1 was cancelled", taskNoteHeading("helper 1", "cancelled")) + val said = "it said hello" + // A subagent is named, and its own words stay in its own transcript rather than being + // repeated here. + assertEquals("helper 1 reported back", taskNoteSummary("helper 1", "completed", said)) + assertEquals("helper 1 failed", taskNoteSummary("helper 1", "failed", null)) + assertEquals("helper 1 was cancelled", taskNoteSummary("helper 1", "cancelled", said)) // A word this build has never seen is said as itself, not mapped onto the nearest one. - assertEquals("helper 1: evicted", taskNoteHeading("helper 1", "evicted")) + assertEquals("helper 1: evicted", taskNoteSummary("helper 1", "evicted", said)) + // A backgrounded command has no title and no transcript of its own, so its summary is the + // only record of it there is -- and it is already a sentence. + val command = """Background command "build the kernel" completed (exit code 0)""" + assertEquals(command, taskNoteSummary(null, "completed", command)) + assertEquals("A background task failed", taskNoteSummary(null, "failed", null)) } } diff --git a/server/src/session/claude/translate.rs b/server/src/session/claude/translate.rs index a54f583..97b6606 100644 --- a/server/src/session/claude/translate.rs +++ b/server/src/session/claude/translate.rs @@ -9,7 +9,7 @@ //! session directory; everything else is pure, which is what makes the mapping //! testable without a process. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; @@ -108,19 +108,23 @@ 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 *unfinished* task belongs to: the CLI's `task_id` - /// against the `tool_use_id` this side names a subagent by. + /// Which Task call each 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`]. - /// - /// 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, + /// The backgrounded tasks this translator has seen start and not seen + /// finish, by `tool_use_id`. + /// + /// Half of the answer to "does this session still have work outstanding", + /// which is the difference between `Idle` and [`SessionStatus::Waiting`]. + /// The other half is `Subagents::any_open`, and both are needed: this one + /// covers a backgrounded *command*, which has no subagent behind it at + /// all, and the registry covers a subagent launched before this + /// translator existed, which is every one of them after a backend + /// restart adopts a running session. + open_tasks: HashSet, /// 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. @@ -146,6 +150,7 @@ impl Translator { children: HashMap::new(), rate_limited: false, tasks: HashMap::new(), + open_tasks: HashSet::new(), in_turn: false, } } @@ -374,10 +379,10 @@ impl Translator { // nobody having typed anything. Reported as what it is, so // that nothing tells the reader the work has finished. events.push(Event::Status { - state: if self.tasks.is_empty() { - SessionStatus::Idle - } else { + state: if self.work_outstanding() { SessionStatus::Waiting + } else { + SessionStatus::Idle }, }); events @@ -534,7 +539,6 @@ impl Translator { .or_else(|| task_id.and_then(|task| self.tasks.get(task).cloned())); match message.get("subtype").and_then(Value::as_str) { Some("task_notification") => self.task_ended( - task_id, about, message.get("status").and_then(Value::as_str), text_field(message, "summary"), @@ -555,16 +559,39 @@ impl Translator { if status == Some("completed") { return Vec::new(); } - self.task_ended(task_id, about, status, None) + self.task_ended(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. + Some("task_started") => { + // What makes the session `Waiting` when its turn ends. 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. + if let Some(about) = about { + self.open_tasks.insert(about); + } + Vec::new() + } + // `task_progress`: the mapping above is the whole of what it is + // for. _ => Vec::new(), } } + /// Whether the session has work of its own still running: the difference + /// between `Idle` and [`SessionStatus::Waiting`]. + /// + /// Two sources because neither covers the other. `open_tasks` holds what + /// this translator watched start, which is the only thing that knows + /// about a backgrounded *command* -- it has no subagent. The registry + /// holds what is on disk, which is the only thing that knows about a + /// subagent that started before this translator did. + /// + /// `session_running` is true by construction: this is only ever asked + /// while translating a line the session's process just wrote. + fn work_outstanding(&self) -> bool { + !self.open_tasks.is_empty() || self.subagents.any_open(true) + } + /// 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 @@ -582,7 +609,6 @@ impl Translator { /// of it. fn task_ended( &mut self, - task_id: Option<&str>, about: Option, status: Option<&str>, summary: Option, @@ -593,11 +619,16 @@ impl Translator { 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()) { + // Reported once. The two lifecycle shapes can both arrive for one + // task, and whichever gets here first is the one that finds it open. + // + // The registry is asked as well as this translator's own set, and + // that is what makes an adopted session work: a subagent launched + // before a backend restart has no entry here, because its + // `task_started` is behind the offset its session's stdout is read + // from. `finish` below closes it either way, so a second line for the + // same task still finds nothing. + if !self.open_tasks.remove(&about) && !self.subagents.is_open(&about) { return Vec::new(); } if let Some(summary) = &summary { @@ -621,7 +652,7 @@ impl Translator { // 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 { + if !self.work_outstanding() && !self.in_turn { events.push(Event::Status { state: SessionStatus::Idle, }); @@ -1476,6 +1507,62 @@ mod tests { ); } + /// The case a backend restart produces, which is every subagent a session + /// has when the server is updated under it. Adoption picks the session's + /// stdout back up from a recorded offset, so the `task_started` lines for + /// anything already running are behind it and this translator never sees + /// them: it starts empty, and asking only itself would report the session + /// idle with a subagent plainly still working. + #[test] + fn a_subagent_that_started_before_this_translator_still_counts_as_outstanding() { + let dir = tempfile::tempdir().expect("tempdir"); + let subagents = test_subagents(&dir); + // Started by somebody else, exactly as a previous run of the server + // would have left it on disk. + subagents.start("toolu_old", "the Dev Updater agent", None); + + let mut translator = Translator::new(dir.path().to_path_buf(), Arc::clone(&subagents)); + let result = r#"{"type":"result","subtype":"success","is_error":false,"usage":{}}"#; + assert_eq!( + translate_lines(&mut translator, &[result]).last(), + Some(&Event::Status { + state: SessionStatus::Waiting + }), + "the registry knows about it even though this translator does not" + ); + + // And its ending is reported, though nothing here saw it begin. + assert_eq!( + translate_lines( + &mut translator, + &[ + r#"{"type":"system","subtype":"task_notification","task_id":"old","tool_use_id":"toolu_old","status":"completed","summary":"pushed"}"#, + ], + ), + vec![ + Event::TaskNote { + about: "toolu_old".into(), + title: Some("the Dev Updater agent".into()), + status: "completed".into(), + summary: Some("pushed".into()), + }, + Event::Status { + state: SessionStatus::Idle + }, + ] + ); + // Once: `finish` closed it, so the second shape finds nothing. + assert!( + translate_lines( + &mut translator, + &[ + r#"{"type":"system","subtype":"task_updated","task_id":"old","patch":{"status":"failed"}}"#, + ], + ) + .is_empty() + ); + } + /// 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, diff --git a/server/src/session/driver.rs b/server/src/session/driver.rs index 415f1f3..80bf9a1 100644 --- a/server/src/session/driver.rs +++ b/server/src/session/driver.rs @@ -211,11 +211,15 @@ pub enum Event { /// A task the session started in the background reporting back: a /// subagent that has finished, or a backgrounded command. /// - /// Recorded because it is a message the session *received*, and without - /// it the turn it wakes up and runs has nothing in front of it. Two - /// replies then met with no row between them and were folded into one, - /// so a phone drew the answer to a question nobody could see as a - /// continuation of the previous sentence. + /// Recorded because the turn the session wakes up and runs would + /// otherwise have nothing in front of it: two replies met with no row + /// between them and were folded into one, so a phone drew the answer to + /// a question nobody could see as a continuation of the previous + /// sentence. It is drawn as a **divider** rather than as a message -- + /// what it marks is the boundary, and the subagent's own words are in + /// the subagent's own transcript, which is where somebody who wants them + /// looks. Repeating them here would be the same text in two places, and + /// the copy is the one that goes stale. /// /// Its own kind rather than an update to the Task call's row: that row /// is wherever the call was made, which is above everything the session @@ -236,6 +240,11 @@ pub enum Event { /// "finished" is the wrong word for a task that was killed. status: String, /// What it said on the way out, where it said anything. + /// + /// Only ever *shown* for a task with no [`TaskNote::title`], which is + /// a backgrounded command: it has no transcript of its own, so this + /// is the only record there is of it. A subagent's report is recorded + /// as that subagent's own closing text and is not repeated here. #[serde(default, skip_serializing_if = "Option::is_none")] summary: Option, }, diff --git a/server/src/session/echo.rs b/server/src/session/echo.rs index 98a52f6..cee7ce8 100644 --- a/server/src/session/echo.rs +++ b/server/src/session/echo.rs @@ -944,15 +944,24 @@ async fn run_helper( if elapsed < target { tokio::time::sleep(target - elapsed).await; } + // The subagent's closing report, in the subagent's own transcript, which + // is where a real one's goes and the only place it belongs -- recorded + // before the ending, so it is not below it. + let summary = format!("{title} finished and had nothing to report."); + subagents.record( + &id, + Event::AssistantText { + delta: summary.clone(), + }, + ); subagents.finish(&id); let _ = sink.send(Event::ToolEnd { id: id.clone(), output: "subagent finished".to_string(), }); - // The message the session receives, and then the turn it runs because of - // it: the parent has to say something afterwards, since the defect this + // The boundary the session's next turn begins at, and then that turn: the + // parent has to say something afterwards, since the defect this // reproduces is two replies meeting with nothing between them. - let summary = format!("{title} finished and had nothing to report."); let _ = sink.send(Event::TaskNote { about: id, title: Some(title.clone()), diff --git a/server/src/session/subagent.rs b/server/src/session/subagent.rs index bc82149..4f4e0cc 100644 --- a/server/src/session/subagent.rs +++ b/server/src/session/subagent.rs @@ -280,6 +280,35 @@ impl Subagents { .filter(|title| !title.is_empty()) } + /// Whether the subagent named `id` exists and has not finished. `false` + /// for an id that is not a subagent's at all -- a backgrounded command's + /// tool call reaches here with the same shape. + pub fn is_open(&self, id: &str) -> bool { + self.get(id).is_some_and(|subagent| subagent.is_open()) + } + + /// Whether this session has any subagent still working, read from the + /// directory rather than from what this process has seen. + /// + /// That is the whole point of it. A backend restart adopts a session's + /// process and picks its stdout back up from a recorded offset, so the + /// `task_started` lines for subagents launched before the restart are + /// already behind that offset and the translator never sees them -- it + /// starts with an empty set and reports the session `Idle` at the end of + /// a turn it should have called [`SessionStatus::Waiting`]. Asking the + /// registry is a measurement instead of bookkeeping, so it is right for + /// a session this process did not start. + /// + /// Measured rather than cached because the wrong answer has to be able + /// to correct itself: a subagent left `Running` by a previous run is + /// finished by the session's own exit (see `finish_all`), and the next + /// turn to end then reads the truth. + pub fn any_open(&self, session_running: bool) -> bool { + self.list(session_running) + .iter() + .any(|info| info.status == SessionStatus::Running) + } + /// Appends one event to a subagent's own transcript. A no-op, with a /// debug log, for an id nothing was started under -- a child line for a /// subagent this registry never opened is dropped rather than guessed