diff --git a/client-core/src/durations.rs b/client-core/src/durations.rs new file mode 100644 index 0000000..9adb933 --- /dev/null +++ b/client-core/src/durations.rs @@ -0,0 +1,100 @@ +//! A span of milliseconds, written the way somebody reads it -- the port +//! of `Durations.kt`'s `formatMillis`/`formatMillisText`, with its tests. +//! +//! Only the tool-timeout half is here. `formatSpan` (the usage +//! countdown's rounding-up rule) belongs with whatever draws the usage +//! bar, and nothing in this crate needs it yet. + +/// A span of milliseconds, written the way somebody reads it. +/// +/// A tool's timeout arrives as `480000`, which nobody reads as eight +/// minutes. The rule has two halves, because a short span and a long one +/// are read for different things. Under a minute the question is "roughly +/// how long", so only the largest unit is shown and a fraction carries the +/// rest -- `2.5s`. At a minute or more the question is "how long exactly", +/// so every unit with something in it is written out -- `5d 12h 4m`. Empty +/// units are left out rather than written as zero. +/// +/// Sub-second precision is dropped past a minute: nothing that takes days +/// is measured in milliseconds. +pub fn format_millis(ms: i64) -> String { + if ms < 0 { + return format!("-{}", format_millis(-ms)); + } + if ms < 1000 { + return format!("{ms}ms"); + } + if ms < 60_000 { + let tenths = (ms + 50) / 100; + let (whole, rest) = (tenths / 10, tenths % 10); + return if rest == 0 { + format!("{whole}s") + } else { + format!("{whole}.{rest}s") + }; + } + let seconds = ms / 1000; + [ + ("d", seconds / 86_400), + ("h", seconds / 3600 % 24), + ("m", seconds / 60 % 60), + ("s", seconds % 60), + ] + .iter() + .filter(|(_, n)| *n > 0) + .map(|(unit, n)| format!("{n}{unit}")) + .collect::>() + .join(" ") +} + +/// `text` as a span when it is a whole number of milliseconds, and +/// unchanged when it is not. +pub fn format_millis_text(text: &str) -> String { + match text.trim().parse::() { + Ok(ms) => format_millis(ms), + Err(_) => text.to_string(), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The two ways a span of time is written here, and the rule each of + /// them follows -- ported from `DurationsTest.kt`, whose doc says why: + /// both are read off a screen to make a decision, so what matters is + /// that the shortest form that answers the question is what appears. + #[test] + fn under_a_minute_is_the_largest_unit_alone() { + assert_eq!(format_millis(30), "30ms"); + assert_eq!(format_millis(999), "999ms"); + assert_eq!(format_millis(1000), "1s"); + assert_eq!(format_millis(2500), "2.5s"); + // One decimal, rounded rather than cut: 2.46s is nearer two and a + // half than two and four. + assert_eq!(format_millis(2460), "2.5s"); + assert_eq!(format_millis(59_900), "59.9s"); + } + + #[test] + fn a_minute_or_more_is_every_unit_that_has_something_in_it() { + // The figure this rule was written for: a tool timeout, which + // arrives as milliseconds and is unreadable as 480000. + assert_eq!(format_millis(480_000), "8m"); + assert_eq!(format_millis(60_000), "1m"); + assert_eq!(format_millis(90_000), "1m 30s"); + assert_eq!(format_millis(475_440_000), "5d 12h 4m"); + // Empty units are left out rather than written as zero: the labels + // say which is which, and "5d 0h 4m" is only longer. + assert_eq!(format_millis(432_240_000), "5d 4m"); + } + + #[test] + fn only_a_whole_number_of_milliseconds_is_rewritten() { + assert_eq!(format_millis_text(" 480000 "), "8m"); + // A timeout a tool expressed some other way is its own words, + // passed through rather than guessed at. + assert_eq!(format_millis_text("2 minutes"), "2 minutes"); + assert_eq!(format_millis_text(""), ""); + } +} diff --git a/client-core/src/lib.rs b/client-core/src/lib.rs index d94e4f5..c7a91d8 100644 --- a/client-core/src/lib.rs +++ b/client-core/src/lib.rs @@ -5,11 +5,13 @@ pub mod ansi; pub mod api; pub mod config; +pub mod durations; pub mod event_stream; pub mod highlight; pub mod markdown_blocks; pub mod notifications; pub mod sse; +pub mod tool_summary; pub mod transcript_cache; pub mod transcript_fold; pub mod transcript_source; diff --git a/client-core/src/tool_summary.rs b/client-core/src/tool_summary.rs new file mode 100644 index 0000000..6bcb91a --- /dev/null +++ b/client-core/src/tool_summary.rs @@ -0,0 +1,244 @@ +//! A tool call's input, read rather than dumped -- the port of +//! `ToolInput.kt`'s `parseToolInput`, which is what both the collapsed +//! card's one-line summary and the expanded card's key/value list are +//! derived from. +//! +//! Every tool's input arrives as JSON, and showing it raw makes the reader +//! parse `{"command":"…","timeout":120000}` themselves to find the one +//! line they care about. So the fields that carry the meaning are pulled +//! out, and anything left over is still shown, because dropping a field +//! would be claiming the tool has no other input when it might. +//! +//! Pure, and here rather than in the widget crate, for the reason the rest +//! of this crate exists: the derivation is the same on a phone and on a +//! desktop, and it is testable without a renderer. + +use crate::durations::format_millis_text; +use crate::highlight::Language; +use serde_json::{Map, Value}; + +/// A tool call's input, split into the parts a card draws separately. +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub struct ToolInput { + /// The thing that will actually be run or read, if this tool has one. + pub subject: Option, + /// The language [`ToolInput::subject`] is written in, for + /// highlighting. + pub language: Option, + /// The tool's own one-line summary, when it wrote one. + pub description: Option, + /// How long the call may take, in the largest units it fits. Shown + /// apart because it is a limit on the call rather than part of what + /// the call does. + pub timeout: Option, + /// Everything else, as `name: value` lines. Never dropped. + pub rest: Vec, +} + +impl ToolInput { + /// The one line to show when there is only room for one: what this + /// call is for. + pub fn title(&self) -> Option<&str> { + self.description + .as_deref() + .or(self.subject.as_deref()) + // A subject that is only whitespace would draw as an empty + // summary line, which reads as a tool with nothing to say + // rather than as one whose subject was blank. + .filter(|t| !t.trim().is_empty()) + } +} + +/// Which field of which tool is the subject. +/// +/// A table rather than a chain of `if`s: adding a tool is a row, and the +/// shape stops any of them from being the special case that gets its own +/// code path. Unknown tools fall through to "no subject, everything is +/// rest". +const SUBJECTS: &[(&str, &str, Option)] = &[ + ("Bash", "command", Some(Language::Shell)), + ("Read", "file_path", None), + ("Write", "file_path", None), + ("Edit", "file_path", None), + ("Glob", "pattern", None), + ("Grep", "pattern", None), + ("WebFetch", "url", None), +]; + +/// Fields that are the tool's own prose about itself rather than input to +/// it. +const DESCRIPTIONS: &[&str] = &["description", "prompt"]; + +/// One JSON value as the Kotlin's `JSONObject.optString`/`get` wrote it: a +/// string is its own characters, anything else is its JSON form. +/// +/// One function rather than two, because the same coercion decides both +/// what a subject reads as and what a leftover field's value reads as, and +/// two copies would eventually disagree about a number. +fn as_text(value: &Value) -> String { + match value { + Value::String(s) => s.clone(), + other => other.to_string(), + } +} + +fn non_blank(value: Option<&Value>) -> Option { + let text = as_text(value?); + (!text.trim().is_empty()).then_some(text) +} + +/// Split `input` (a tool call's JSON) into the parts a card draws. +/// +/// Input that is not a JSON object -- older transcripts and some tools +/// send a bare string -- is still the input, so it is still shown, as the +/// whole of `rest`. +pub fn parse_tool_input(tool: &str, input: &str) -> ToolInput { + let Ok(Value::Object(json)) = serde_json::from_str::(input) else { + return ToolInput { + rest: match input.trim().is_empty() { + true => Vec::new(), + false => vec![input.to_string()], + }, + ..ToolInput::default() + }; + }; + parse_object(tool, &json) +} + +fn parse_object(tool: &str, json: &Map) -> ToolInput { + let (subject_key, language) = SUBJECTS + .iter() + .find(|(name, ..)| *name == tool) + .map(|(_, key, language)| (Some(*key), *language)) + .unwrap_or((None, None)); + let subject = subject_key.and_then(|key| non_blank(json.get(key))); + let description = DESCRIPTIONS + .iter() + .find_map(|key| non_blank(json.get(*key))); + let timeout = non_blank(json.get("timeout")).map(|t| format_millis_text(&t)); + + // Sorted, so the leftovers are in the same order every time this call + // is drawn rather than in whatever order the JSON happened to arrive + // in. A field is left out only when it is already drawn somewhere + // else on the card. + let mut keys: Vec<&String> = json + .keys() + .filter(|k| Some(k.as_str()) != subject_key || subject.is_none()) + .filter(|k| !DESCRIPTIONS.contains(&k.as_str()) || description.is_none()) + .filter(|k| k.as_str() != "timeout" || timeout.is_none()) + .collect(); + keys.sort(); + let rest = keys + .into_iter() + .map(|key| format!("{key}: {}", as_text(&json[key]))) + .collect(); + + ToolInput { + subject, + language, + description, + timeout, + rest, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn each_tool_in_the_table_has_its_own_subject() { + // One assertion per row of `SUBJECTS`, because the table is the + // whole of the rule and a row lost in an edit would otherwise + // only show up as a card with no summary line. + let cases = [ + ("Bash", r#"{"command":"ls -la"}"#, "ls -la"), + ("Read", r#"{"file_path":"/tmp/x.rs"}"#, "/tmp/x.rs"), + ("Write", r#"{"file_path":"/tmp/y.rs"}"#, "/tmp/y.rs"), + ("Edit", r#"{"file_path":"/tmp/z.rs"}"#, "/tmp/z.rs"), + ("Glob", r#"{"pattern":"**/*.rs"}"#, "**/*.rs"), + ("Grep", r#"{"pattern":"fn main"}"#, "fn main"), + ("WebFetch", r#"{"url":"https://x/y"}"#, "https://x/y"), + ]; + for (tool, input, expected) in cases { + let parsed = parse_tool_input(tool, input); + assert_eq!(parsed.subject.as_deref(), Some(expected), "{tool}"); + assert_eq!(parsed.title(), Some(expected), "{tool}"); + assert!(parsed.rest.is_empty(), "{tool}: {:?}", parsed.rest); + } + assert_eq!( + parse_tool_input("Bash", r#"{"command":"ls"}"#).language, + Some(Language::Shell), + "a Bash command is shell, and is the one row that names a language" + ); + } + + #[test] + fn a_tools_own_description_is_what_the_one_line_says() { + // The description wins over the subject: it is the tool's own + // prose about what this call is for, which is what a reader + // scanning a collapsed run is looking for. + let parsed = parse_tool_input( + "Bash", + r#"{"command":"cargo test -p iris","description":"Run the iris tests"}"#, + ); + assert_eq!(parsed.title(), Some("Run the iris tests")); + assert_eq!(parsed.subject.as_deref(), Some("cargo test -p iris")); + assert!(parsed.rest.is_empty(), "{:?}", parsed.rest); + } + + #[test] + fn a_timeout_is_read_as_a_span_and_kept_apart_from_the_rest() { + let parsed = parse_tool_input("Bash", r#"{"command":"sleep 500","timeout":480000}"#); + assert_eq!(parsed.timeout.as_deref(), Some("8m")); + assert!(parsed.rest.is_empty(), "{:?}", parsed.rest); + } + + #[test] + fn every_field_not_drawn_elsewhere_is_still_shown() { + // The half the "never dropped" promise is about: a tool this + // build has never heard of has no subject, so *everything* is + // rest -- and a known tool's extra fields are too. + let parsed = parse_tool_input( + "Edit", + r#"{"file_path":"/a.rs","old_string":"x","new_string":"y","replace_all":true}"#, + ); + assert_eq!( + parsed.rest, + vec![ + "new_string: y".to_string(), + "old_string: x".to_string(), + "replace_all: true".to_string(), + ], + "sorted, and a non-string value written as JSON" + ); + let unknown = parse_tool_input("SomeNewTool", r#"{"b":2,"a":"one"}"#); + assert_eq!(unknown.subject, None); + assert_eq!(unknown.rest, vec!["a: one".to_string(), "b: 2".to_string()]); + } + + #[test] + fn input_that_is_not_an_object_is_still_the_input() { + // Older transcripts and some tools send a bare string; a card + // that dropped it would claim the call had no input at all. + assert_eq!( + parse_tool_input("Bash", "just a string").rest, + vec!["just a string".to_string()] + ); + assert_eq!(parse_tool_input("Bash", " ").rest, Vec::::new()); + assert_eq!(parse_tool_input("Bash", "").title(), None); + } + + #[test] + fn a_blank_subject_is_no_subject_rather_than_an_empty_summary_line() { + let parsed = parse_tool_input("Bash", r#"{"command":" ","other":1}"#); + assert_eq!(parsed.subject, None); + assert_eq!(parsed.title(), None); + // Not dropped just because it was blank -- it is still a field + // the call carried. + assert_eq!( + parsed.rest, + vec!["command: ".to_string(), "other: 1".to_string()] + ); + } +} diff --git a/client-core/src/transcript_fold.rs b/client-core/src/transcript_fold.rs index a4cf03c..42030a4 100644 --- a/client-core/src/transcript_fold.rs +++ b/client-core/src/transcript_fold.rs @@ -78,6 +78,11 @@ pub enum TranscriptItem { input: String, output: String, done: bool, + /// Whether the result that arrived said the call failed + /// ([`Event::ToolEnd`]'s `is_error`). Meaningless while `done` is + /// false, and [`ToolState::of`] is the only thing that reads the + /// pair, so the two cannot be combined wrongly at a call site. + failed: bool, asks: Vec, images: Vec, }, @@ -352,6 +357,7 @@ pub fn join_pages(earlier: &[TranscriptItem], later: &[TranscriptItem]) -> Vec Vec Vec Vec { + Event::ToolEnd { + id, + output, + is_error, + } => { if items.iter().any(|i| i.as_tool_run() == Some(id.as_str())) { update_tool(items, id, |item| { if let TranscriptItem::ToolRun { - output: out, done, .. + output: out, + done, + failed, + .. } = item { *out = output.clone(); *done = true; + *failed = *is_error; } }) } else { @@ -594,6 +610,7 @@ pub fn fold_event(items: &[TranscriptItem], entry: &SeqEvent) -> Vec Vec { for ask in asks.iter_mut() { @@ -665,6 +683,7 @@ pub fn fold_event(items: &[TranscriptItem], entry: &SeqEvent) -> Vec Vec Option { + let TranscriptItem::ToolRun { + done, failed, asks, .. + } = item + else { + return None; + }; + debug_assert!( + !failed || *done, + "a call cannot have failed before its result arrived" + ); + Some(if asks.iter().any(|ask| ask.answers.is_empty()) { + // Ahead of `done`: a call waiting on permission has not + // finished either, and which of the two the reader is being + // told about is the one they can act on. + Self::Deciding + } else if !*done { + match session_working { + true => Self::Running, + false => Self::NoResult, + } + } else if *failed { + Self::Failed + } else { + Self::Succeeded + }) + } +} + /// One row as the transcript draws it: a run of consecutive tool calls, or /// anything else. Ported from `ToolRows.kt`'s `TranscriptRow` and /// `groupToolRuns` -- the Compose card rendering in that file is not part @@ -989,6 +1076,7 @@ mod tests { Event::ToolEnd { id: "x".to_string(), output: "done".to_string(), + is_error: false, }, )]); assert_eq!( @@ -1001,6 +1089,7 @@ mod tests { input: String::new(), output: "done".to_string(), done: true, + failed: false, asks: Vec::new(), images: Vec::new(), }] @@ -1166,6 +1255,7 @@ mod tests { Event::ToolEnd { id: id.to_string(), output: output.to_string(), + is_error: false, }, ) } @@ -1211,6 +1301,7 @@ mod tests { input: "{}".to_string(), output: "the result".to_string(), done: true, + failed: false, asks: Vec::new(), images: Vec::new(), }], @@ -1269,6 +1360,7 @@ mod tests { input: "{}".to_string(), output: String::new(), done: false, + failed: false, asks: Vec::new(), images: Vec::new(), }]; @@ -1279,3 +1371,150 @@ mod tests { } } } + +/// [`ToolState`] is what a card colours itself by, so each of its five +/// states is asserted from the events that actually produce it rather than +/// from a hand-built item -- a mapping that agreed with a fixture and +/// disagreed with the fold would be invisible until it was on screen. +#[cfg(test)] +mod tool_state_tests { + use super::*; + + fn event(seq: u64, e: Event) -> SeqEvent { + SeqEvent { + seq, + ts: 0.0, + event: e, + } + } + + fn fold_all(events: &[SeqEvent]) -> Vec { + events + .iter() + .fold(Vec::new(), |items, e| fold_event(&items, e)) + } + + fn start(id: &str) -> SeqEvent { + event( + 1, + Event::ToolStart { + id: id.to_string(), + tool: "Bash".to_string(), + input: serde_json::json!({"command": "ls"}), + }, + ) + } + + fn end(id: &str, output: &str, is_error: bool) -> SeqEvent { + event( + 2, + Event::ToolEnd { + id: id.to_string(), + output: output.to_string(), + is_error, + }, + ) + } + + fn state_of(events: &[SeqEvent], session_working: bool) -> ToolState { + let items = fold_all(events); + ToolState::of(&items[0], session_working).expect("the fixture's first item is a tool call") + } + + #[test] + fn a_result_that_arrived_is_read_from_is_error() { + assert_eq!( + state_of(&[start("a"), end("a", "ok", false)], false), + ToolState::Succeeded + ); + assert_eq!( + state_of(&[start("a"), end("a", "No such file", true)], false), + ToolState::Failed + ); + } + + /// The pair this enum exists for. Both calls have an empty `output` + /// and nothing else distinguishes them, so a card that only looked at + /// the text would draw the interrupted one as a call that ran fine and + /// printed nothing. + #[test] + fn a_call_that_printed_nothing_is_not_a_call_that_never_answered() { + assert_eq!( + state_of(&[start("a"), end("a", "", false)], false), + ToolState::Succeeded, + "a result arrived; it was empty" + ); + assert_eq!( + state_of(&[start("a")], false), + ToolState::NoResult, + "no result, and the session is not working any more" + ); + } + + /// The same call, mid-turn: still running rather than abandoned. The + /// only thing separating the two is the session's own status, which is + /// why `of` takes it. + #[test] + fn no_result_while_the_session_works_is_still_running() { + assert_eq!(state_of(&[start("a")], true), ToolState::Running); + } + + #[test] + fn an_unanswered_ask_is_the_readers_move_whatever_else_is_true() { + let asking = event( + 3, + Event::Question { + id: "q1".to_string(), + prompt: "Allow?".to_string(), + header: None, + options: vec![QuestionOption { + label: "Allow".to_string(), + description: None, + preview: None, + }], + multi_select: false, + about: Some("a".to_string()), + }, + ); + let answered = event( + 4, + Event::Answered { + id: "q1".to_string(), + answers: vec!["Allow".to_string()], + }, + ); + // Ahead of both "still running" and "no result": the reader can + // act on this one, and cannot act on either of those. + assert_eq!( + state_of(&[start("a"), asking.clone()], true), + ToolState::Deciding + ); + assert_eq!( + state_of(&[start("a"), asking.clone()], false), + ToolState::Deciding + ); + assert_eq!( + state_of( + &[start("a"), asking, answered, end("a", "ok", false)], + false + ), + ToolState::Succeeded, + "once it is answered the call is an ordinary one again" + ); + } + + #[test] + fn nothing_but_a_tool_call_has_a_tool_state() { + assert_eq!( + ToolState::of( + &TranscriptItem::UserMsg { + seq: 1, + text: "hi".to_string(), + attachments: Vec::new(), + }, + true + ), + None + ); + } +} diff --git a/event-model/src/lib.rs b/event-model/src/lib.rs index 4c40c11..f81f28e 100644 --- a/event-model/src/lib.rs +++ b/event-model/src/lib.rs @@ -150,6 +150,19 @@ pub enum Event { ToolEnd { id: String, output: String, + /// Whether the tool reported that the call *failed*, from the + /// CLI's own `is_error` on the `tool_result`. + /// + /// Added 2026-09-06 with the tool-call cards (RUST.md's P1b), + /// because without it a result is the only thing a card has and a + /// failed call is drawn as confidently as a successful one -- the + /// missing state, not a wrong one. `#[serde(default)]` so a + /// transcript written before this field, or a peer on an older + /// build, reads back as "not reported to have failed" rather than + /// failing to parse; that is the same claim the field's absence + /// used to make implicitly. + #[serde(default)] + is_error: bool, }, /// An image the session produced or was sent, saved under the session /// dir and referenced by id; the phone fetches it by URL. diff --git a/server/src/session/claude/translate.rs b/server/src/session/claude/translate.rs index 72964e4..d37bd2b 100644 --- a/server/src/session/claude/translate.rs +++ b/server/src/session/claude/translate.rs @@ -700,6 +700,7 @@ impl Translator { events.push(Event::ToolEnd { id: about.clone(), output: texts.join("\n"), + is_error: crate::session::import::tool_result_is_error(block), }); // Deliberately does *not* finish a subagent `about` might name: // the Task tool runs in the background by default, so this @@ -1015,7 +1016,44 @@ mod tests { }, Event::ToolEnd { id: "toolu_01".to_string(), - output: "probe-ok".to_string() + output: "probe-ok".to_string(), + is_error: false, + }, + ] + ); + } + + /// The other half of the test above, and the one it cannot stand in + /// for: a call the tool itself reported as failed. Both lines are + /// `tool_result`s and both carry output, so nothing but `is_error` + /// tells them apart -- which is why dropping the field made a broken + /// call draw exactly like one that worked. + #[test] + fn a_failed_tool_result_says_so() { + let dir = tempfile::tempdir().expect("tempdir"); + let mut translator = Translator::new(dir.path().to_path_buf(), test_subagents(&dir)); + let events = translate_lines( + &mut translator, + &[ + r#"{"type":"user","message":{"role":"user","content":[{"tool_use_id":"toolu_02","type":"tool_result","content":"No such file or directory","is_error":true}]},"parent_tool_use_id":null}"#, + // No `is_error` at all: every transcript written before + // the field was read looks like this, and it means the + // call was not reported to have failed. + r#"{"type":"user","message":{"role":"user","content":[{"tool_use_id":"toolu_03","type":"tool_result","content":"fine"}]},"parent_tool_use_id":null}"#, + ], + ); + assert_eq!( + events, + vec![ + Event::ToolEnd { + id: "toolu_02".to_string(), + output: "No such file or directory".to_string(), + is_error: true, + }, + Event::ToolEnd { + id: "toolu_03".to_string(), + output: "fine".to_string(), + is_error: false, }, ] ); @@ -1459,6 +1497,7 @@ mod tests { Event::ToolEnd { id: "toolu_05".to_string(), output: "took a screenshot".to_string(), + is_error: false, } ); } diff --git a/server/src/session/echo.rs b/server/src/session/echo.rs index c899524..3e055c8 100644 --- a/server/src/session/echo.rs +++ b/server/src/session/echo.rs @@ -134,6 +134,7 @@ impl EchoDriver { self.emit(Event::ToolEnd { id, output: format!("{label} step {index} finished"), + is_error: false, }); } } @@ -645,6 +646,7 @@ impl EchoDriver { send(Event::ToolEnd { id, output: format!("call {i} finished"), + is_error: false, }); } finish(); @@ -698,6 +700,7 @@ impl EchoDriver { send(Event::ToolEnd { id, output: format!("ran: {command}"), + is_error: false, }); } @@ -717,6 +720,7 @@ impl EchoDriver { send(Event::ToolEnd { id, output: format!("echoed: {input}"), + is_error: false, }); } @@ -817,6 +821,7 @@ async fn write_beat(sink: &EventSink, session_dir: &Path, beat: usize) { send(Event::ToolEnd { id, output: format!("beat {beat}: forty-two lines of nothing in particular"), + is_error: false, }); } // A run of three, which the app folds into one collapsed group -- the @@ -829,9 +834,20 @@ async fn write_beat(sink: &EventSink, session_dir: &Path, beat: usize) { tool: if i % 2 == 0 { "Bash" } else { "Grep" }.to_string(), input: serde_json::json!({ "command": format!("grep -rn 'beat {beat}' /tmp") }), }); + // The middle one fails, so this fixture carries a run in + // which the three calls are not all in the same state -- + // the case a card drawing every finished call the same way + // looks correct on. `is_error` is the CLI's own field + // (`import::tool_result_is_error`), and a driver that + // never sets it makes the failed appearance unreachable + // from the sandbox. send(Event::ToolEnd { id, - output: format!("beat {beat}, call {i} of 3"), + output: match i == 2 { + true => format!("beat {beat}, call {i} of 3: No such file or directory"), + false => format!("beat {beat}, call {i} of 3"), + }, + is_error: i == 2, }); } } @@ -856,6 +872,7 @@ async fn write_beat(sink: &EventSink, session_dir: &Path, beat: usize) { send(Event::ToolEnd { id, output: format!("beat {beat}: captured"), + is_error: false, }); } // Somebody else's voice, which is its own row shape. @@ -904,6 +921,7 @@ async fn run_helper(id: String, sink: EventSink, subagents: Arc) { Event::ToolEnd { id: tool_id, output: "helper done".to_string(), + is_error: false, }, ); let target = Duration::from_secs(3); @@ -915,6 +933,7 @@ async fn run_helper(id: String, sink: EventSink, subagents: Arc) { let _ = sink.send(Event::ToolEnd { id, output: "subagent finished".to_string(), + is_error: false, }); } @@ -1069,6 +1088,7 @@ impl Driver for EchoDriver { self.emit(Event::ToolEnd { id: call, output: format!("answered: {answer}"), + is_error: false, }); // The work carries on where it left off, which is what makes the // asked-here row a boundary with a group on each side rather than diff --git a/server/src/session/import.rs b/server/src/session/import.rs index b9bfc74..d5289da 100644 --- a/server/src/session/import.rs +++ b/server/src/session/import.rs @@ -363,6 +363,22 @@ fn is_hidden(record: &Value) -> bool { || record.get("isMeta").and_then(Value::as_bool) == Some(true) } +/// Whether a `tool_result` block says the call itself failed. +/// +/// One reader for the field rather than one per caller: the live +/// translator (`translate.rs`) and this replay of the CLI's own file look +/// at the same block shape, and a call drawn as failed in one and as +/// succeeded in the other would be the same conversation disagreeing with +/// itself. Absent means "not reported to have failed" -- which is what the +/// CLI writes for a call that went fine, and also what every transcript +/// written before this field was read says. +pub(crate) fn tool_result_is_error(block: &Value) -> bool { + block + .get("is_error") + .and_then(Value::as_bool) + .unwrap_or(false) +} + fn text_of(content: &Value) -> String { match content { Value::String(text) => text.clone(), @@ -549,6 +565,7 @@ fn push_user(events: &mut Vec, content: &Value, session_dir: &std::path:: events.push(Event::ToolEnd { id: id.to_string(), output: text_of(block.get("content").unwrap_or(&Value::Null)), + is_error: tool_result_is_error(block), }); } } diff --git a/server/src/session/transcript.rs b/server/src/session/transcript.rs index 2ac74fd..68f4469 100644 --- a/server/src/session/transcript.rs +++ b/server/src/session/transcript.rs @@ -744,6 +744,7 @@ mod tests { Event::ToolEnd { id: "t1".into(), output: "done".into(), + is_error: false, }, Event::Image { image: "img1".into(),