diff --git a/AGENTS.md b/AGENTS.md index 8a0686a..07667f2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -247,11 +247,16 @@ written, and the fold uses that same predicate to decide a reply is settled. screenful of dividers about work nobody was asking after, one of them a whole shell command. A subagent's report is its own transcript's closing text and is read in the subcard. +- **A backgrounded command has no subagent, so its report lands in the tool + card that launched it** — a `ToolUpdate` against the call's own id, replacing + the launch result that says it is still running. `/background [seconds]` in + an echo session is that shape end to end. - **Two replies that meet are separated by a `TurnBreak`** — a hairline, no words. The reply that follows a turn boundary is a **new** message: the fold refuses to grow a settled reply, and without that the two ran together - mid-sentence. `./ui-sandbox.sh` plus `/subagent 3` in an echo session is the - whole rig; the helpers stagger a second apart so each reply is its own. + mid-sentence. `./ui-sandbox.sh` plus `/subagent 3` or `/background 5` in an + echo session is the whole rig; the helpers stagger a second apart so each + reply is its own. - **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 diff --git a/PLAN.md b/PLAN.md index 6cd6170..a961dc8 100644 --- a/PLAN.md +++ b/PLAN.md @@ -676,14 +676,23 @@ them: a hairline rule, no words, no colour. It is made by the fold rather than sent by the server because it is not something that happened — it is the boundary between two things that did. -**Nothing else about a background task goes in the session's transcript.** That -was tried and was wrong: a row per finished subagent is a screenful of dividers -about work the reader was not asking after, and one of them turned out to be a -whole shell command drawn as centred prose, because its words came from -somewhere with no reason to keep them short. A subagent's closing report is -recorded as that subagent's own transcript's closing text and is read there. -The parent's transcript gets a row for a message a subagent genuinely *sends* -it, which arrives by the peer path and already has one. +**Nothing else about a background task gets a row of its own.** That was tried +and was wrong: a row per finished subagent is a screenful of dividers about work +the reader was not asking after, and one of them turned out to be a whole shell +command drawn as centred prose, because its words came from somewhere with no +reason to keep them short. The parent's transcript gets a row for a message a +subagent genuinely *sends* it, which arrives by the peer path and already has +one. + +**The report goes to whichever record is the only one of it**, and the two cases +are different places. A subagent has a transcript of its own, and its closing +words are that transcript's last line. A backgrounded *command* has none: its +own tool card is the only record of it anywhere, and until the notification +arrives that card is showing the launch result, which says the command is +running. So the card is updated (`Event::ToolUpdate` against the call's own id) +rather than left making a claim nothing will ever correct — including for the +endings that carry no summary, which are exactly the ones that went wrong and +the ones a stale "running in background" reads worst on. What the notification is still used for is the status: it is what closes a task in `Status::Waiting`'s bookkeeping. Handled once, however many of the two diff --git a/SUBAGENTS.md b/SUBAGENTS.md index acc4ab6..ad7e78e 100644 --- a/SUBAGENTS.md +++ b/SUBAGENTS.md @@ -86,7 +86,10 @@ transcript is still being written to and its process is the session's to stop. of dividers about work the reader was not asking after; the closing report is *this* transcript's last line and here is where somebody reads it. What the parent gets a row for is a message a subagent genuinely sends it, which - arrives by the peer path. The two lifecycle shapes are still handled once: + arrives by the peer path. A backgrounded *command* is the other half of + this and goes the other way: it has no transcript of its own, so its report + updates the tool card that launched it, which was still saying the command + was running. The two lifecycle shapes are still handled once: whichever gets there first is the one that finds the task still open, and `finish` below closes it. See PLAN.md's "Two turns must never be drawn as one". diff --git a/server/src/session/claude/translate.rs b/server/src/session/claude/translate.rs index 4ceb79f..753cca2 100644 --- a/server/src/session/claude/translate.rs +++ b/server/src/session/claude/translate.rs @@ -631,12 +631,33 @@ impl Translator { if !self.open_tasks.remove(&about) && !self.subagents.is_open(&about) { return Vec::new(); } - if let Some(summary) = summary { - self.subagents - .record(&about, Event::AssistantText { delta: summary }); + let mut events = Vec::new(); + // Where the report goes, and the two cases are not the same place. + // + // A subagent has a transcript of its own, and its closing words are + // that transcript's last line -- see `SUBAGENTS.md`. A backgrounded + // *command* has none: its own tool card is the only record of it + // anywhere, and until this arrives that card is still showing the + // launch result, which says the command is running. It stopped being + // true at this line, so the card is brought up to date rather than + // left making a claim nothing will ever correct. + if self.subagents.get(&about).is_some() { + if let Some(summary) = summary { + self.subagents + .record(&about, Event::AssistantText { delta: summary }); + } + } else { + events.push(Event::ToolUpdate { + id: about.clone(), + // A task that stopped without a word still has to say so: the + // states with no summary are exactly the ones that went + // wrong, and they are the ones a stale "running in + // background" reads worst on. + output: summary + .unwrap_or_else(|| format!("this background command {}", status_word(status))), + }); } self.subagents.finish(&about); - let mut events = Vec::new(); // 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 @@ -1037,6 +1058,15 @@ fn closes_a_turn(event: &Event) -> bool { ) } +/// A task's ending in a word, for a card that has to say what became of it. +/// +/// `ended` has already said this is an ending, so the fallback is not "still +/// going" -- it is the honest answer for an ending this build has no word for, +/// which is that nobody here knows which it was. +fn status_word(status: Option<&str>) -> &str { + status.unwrap_or("ended, and this build cannot say how") +} + /// 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 @@ -1488,6 +1518,73 @@ mod tests { ); } + /// A backgrounded command is a task with no subagent behind it, so there is + /// no second transcript for its report to live in -- its own tool card is + /// the only record of it anywhere, and until the notification arrives that + /// card is still showing the launch result saying it is running. + #[test] + fn a_backgrounded_command_reports_into_the_card_that_launched_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":"system","subtype":"task_started","task_id":"bg1","tool_use_id":"toolu_sh","is_backgrounded":true}"#, + ], + ); + assert_eq!( + translate_lines( + &mut translator, + &[ + r#"{"type":"system","subtype":"task_notification","task_id":"bg1","tool_use_id":"toolu_sh","status":"completed","summary":"Background command \"run the tests\" completed (exit code 0)"}"#, + ], + ), + vec![ + Event::ToolUpdate { + id: "toolu_sh".into(), + output: r#"Background command "run the tests" completed (exit code 0)"#.into(), + }, + Event::Status { + state: SessionStatus::Idle + }, + ] + ); + assert!( + subagents.get("toolu_sh").is_none(), + "a command is not a subagent and must not become one" + ); + } + + /// The states with no summary are exactly the ones that went wrong, and a + /// card left saying "running in background" is the worst thing to leave on + /// screen for them. + #[test] + fn a_backgrounded_command_that_failed_says_so_rather_than_saying_nothing() { + let dir = tempfile::tempdir().expect("tempdir"); + let subagents = test_subagents(&dir); + let mut translator = Translator::new(dir.path().to_path_buf(), subagents); + translate_lines( + &mut translator, + &[ + r#"{"type":"system","subtype":"task_started","task_id":"bg2","tool_use_id":"toolu_sh2","is_backgrounded":true}"#, + ], + ); + let events = translate_lines( + &mut translator, + &[ + r#"{"type":"system","subtype":"task_updated","task_id":"bg2","patch":{"status":"failed"}}"#, + ], + ); + assert_eq!( + events.first(), + Some(&Event::ToolUpdate { + id: "toolu_sh2".into(), + output: "this background command failed".into(), + }) + ); + } + /// 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 diff --git a/server/src/session/echo.rs b/server/src/session/echo.rs index 92596ef..96bb54b 100644 --- a/server/src/session/echo.rs +++ b/server/src/session/echo.rs @@ -57,6 +57,10 @@ //! parent then runs a turn answering it -- which is the whole of the shape a //! real background Task produces, and the one where two replies used to be //! drawn as one paragraph. +//! - `/background [seconds]` -- a backgrounded *command*: the same shape with +//! no subagent behind it, so its report has nowhere to go but the card that +//! launched it. Until then that card says the command is running, which is +//! the stale claim this exists to watch being corrected. //! //! `/slow` earns its place: a queued message, a Stop button and a spinner are //! states that only exist mid-turn, and the obvious way to get one -- ask a @@ -461,6 +465,72 @@ impl EchoDriver { return; } + // A backgrounded command: the task shape with no subagent behind it. + // Its own verb because it is the *other* half of what a task + // notification does -- a subagent's report goes into the subagent's + // transcript, and this one has nowhere to go but the card that + // launched it, which until then is still saying the command is + // running. + if let Some(rest) = text.strip_prefix("/background") { + let seconds = rest.trim().parse::().unwrap_or(4).clamp(1, 120); + if announce { + self.emit(Event::MessageTaken { + id: None, + text: text.clone(), + attachments, + }); + } + self.emit(Event::Status { + state: SessionStatus::Running, + }); + let id = format!("echo-background-{}", super::random_hex()); + let command = format!("sleep {seconds} && echo done"); + self.emit(Event::ToolStart { + id: id.clone(), + tool: "Bash".to_string(), + input: serde_json::json!({ + "command": command, + "description": "wait a moment", + "run_in_background": true, + }), + }); + self.emit(Event::ToolEnd { + id: id.clone(), + output: format!("Command running in background with ID: {id}"), + }); + for word in "Started it; I'll pick this up when it lands.".split_inclusive(' ') { + self.emit(Event::AssistantText { + delta: word.to_string(), + }); + } + // Not idle: the command is still going, and the session will speak + // again with nobody having typed anything. + self.emit(Event::Status { + state: SessionStatus::Waiting, + }); + let sink = self.sink.clone(); + tokio::spawn(async move { + tokio::time::sleep(Duration::from_secs(seconds)).await; + let _ = sink.send(Event::ToolUpdate { + id, + output: format!(r#"Background command "{command}" completed (exit code 0)"#), + }); + let _ = sink.send(Event::Status { + state: SessionStatus::Running, + }); + for word in "Done -- it finished cleanly.".split_inclusive(' ') { + let _ = sink.send(Event::AssistantText { + delta: word.to_string(), + }); + tokio::time::sleep(DELTA_DELAY).await; + } + let _ = sink.send(Event::Status { + state: SessionStatus::Idle, + }); + }); + return; + } + // The same word the real CLI takes, so a phone drives both the same way. // `Driver::compact` is what the manager's route calls; this is the typed // path onto it.