From 669c16a0161514fbb10682764bd9442eb5567327 Mon Sep 17 00:00:00 2001 From: Sean Callan Date: Thu, 23 Jul 2026 11:57:13 -0400 Subject: [PATCH 01/14] feat(chat): capture Workflow tool progress from the agent stream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude Code's `Workflow` tool runs as a background task, so the transcript saw a single tool_use carrying a multi-hundred-line JS script, then silence for minutes, then a task-notification. The run's phase/agent tree was already on the wire and we were discarding it. The CLI emits it on `system`/`task_progress` as `workflow_progress` — a full replacement snapshot of the tree, attached only on real state transitions. Throttled no-transition ticks omit the key, which means "unchanged"; the stream handler only assigns when the key is present so a mid-run tick can't blank the tree. Everything on `WorkflowAgentProgress` is `#[serde(default)]` and unknown entry kinds map to `Unknown`, so one renamed field upstream costs an attribute rather than the whole progress tick. Persisted as `workflow_progress_json` alongside `agent_tool_calls_json` so a finished run stays readable after a reload, in replayed history, and in forked sessions. No UI yet — this is the plumbing. --- src/agent/codex_app_server.rs | 4 + src/agent/mod.rs | 3 +- src/agent/types.rs | 232 ++++++++++++++++++ src/db/checkpoint.rs | 17 +- src/fork.rs | 3 + ...3_turn_tool_activity_workflow_progress.sql | 10 + src/migrations/mod.rs | 5 + src/model/checkpoint.rs | 6 + src/ui/src/hooks/useAgentStream.ts | 10 + src/ui/src/stores/slices/chatSlice.ts | 6 + src/ui/src/types/agent-events.ts | 8 + src/ui/src/types/checkpoint.ts | 3 + src/ui/src/types/workflow.ts | 173 +++++++++++++ src/ui/src/utils/reconstructTurns.test.ts | 2 + src/ui/src/utils/reconstructTurns.ts | 31 +++ 15 files changed, 506 insertions(+), 7 deletions(-) create mode 100644 src/migrations/20260723155203_turn_tool_activity_workflow_progress.sql create mode 100644 src/ui/src/types/workflow.ts diff --git a/src/agent/codex_app_server.rs b/src/agent/codex_app_server.rs index d7fc34744..2a79e595b 100644 --- a/src/agent/codex_app_server.rs +++ b/src/agent/codex_app_server.rs @@ -2854,6 +2854,7 @@ fn codex_compacting_status_event() -> AgentEvent { description: None, last_tool_name: None, usage: None, + workflow_progress: None, status: Some("compacting".to_string()), compact_result: None, compact_metadata: None, @@ -2884,6 +2885,7 @@ fn codex_context_compaction_event() -> AgentEvent { description: None, last_tool_name: None, usage: None, + workflow_progress: None, status: None, compact_result: None, compact_metadata: Some(crate::agent::types::CompactMetadata { @@ -2948,6 +2950,7 @@ fn codex_command_execution_status_event(item: &Value, status: &str) -> AgentEven description: None, last_tool_name: None, usage: None, + workflow_progress: None, status: Some(terminal_status.to_string()), compact_result: None, compact_metadata: None, @@ -3069,6 +3072,7 @@ pub fn codex_turn_start_events(thread_id: &str) -> Vec { description: None, last_tool_name: None, usage: None, + workflow_progress: None, status: None, compact_result: None, compact_metadata: None, diff --git a/src/agent/mod.rs b/src/agent/mod.rs index 3ee0548be..5c1caa6f4 100644 --- a/src/agent/mod.rs +++ b/src/agent/mod.rs @@ -37,7 +37,8 @@ pub use session::PersistentSession; pub use types::{ AssistantMessage, CompactMetadata, ContentBlock, ControlRequestInner, ControlResponsePayload, Delta, FileAttachment, InnerStreamEvent, StartContentBlock, StreamEvent, TokenUsage, - TokenUsageIteration, UserContentBlock, UserEventMessage, UserMessageContent, parse_stream_line, + TokenUsageIteration, UserContentBlock, UserEventMessage, UserMessageContent, + WorkflowAgentProgress, WorkflowProgressEntry, parse_stream_line, }; /// Per-turn settings that control CLI flags for the agent subprocess. diff --git a/src/agent/types.rs b/src/agent/types.rs index 615749899..8b1cb426c 100644 --- a/src/agent/types.rs +++ b/src/agent/types.rs @@ -72,6 +72,95 @@ pub struct TaskUsage { pub duration_ms: Option, } +/// One entry in the `workflow_progress` array carried by +/// `subtype: "task_progress"` events for Claude Code's `Workflow` tool. +/// +/// The array is a *full replacement snapshot* of the run's progress tree, +/// not a delta — but the CLI only includes it on meaningful state +/// transitions (queued → progress → done/error). Pure "still working" +/// ticks are throttled and arrive with the field absent, which means +/// "unchanged", never "empty". Consumers must not clear on `None`. +/// +/// Field names are camelCase inside these entries even though the +/// enclosing `system` event uses snake_case — that asymmetry is upstream's, +/// not ours. +/// +/// Unrecognized `type` values deserialize to [`WorkflowProgressEntry::Unknown`] +/// rather than failing the whole line. Note this is lossy on re-serialize: +/// an unknown entry persists as `{"type":"Unknown"}`. That is deliberate — +/// an entry kind we can't name is one we can't render either, and keeping +/// the surrounding progress tree parseable matters more than round-tripping +/// a payload nothing reads. `workflow_log` entries are the known example: +/// the CLI strips them before they ever reach the SDK stream. +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] +#[serde(tag = "type")] +pub enum WorkflowProgressEntry { + /// Declares a phase heading. Emitted once per `phase()` call in the + /// workflow script, ahead of the agents that belong to it. + #[serde(rename = "workflow_phase")] + Phase { + #[serde(default)] + index: i64, + #[serde(default)] + title: String, + }, + /// One subagent's current state. Re-emitted on each transition, so the + /// latest entry for a given `index` supersedes earlier ones. + #[serde(rename = "workflow_agent")] + Agent(Box), + #[serde(other)] + Unknown, +} + +/// Payload of [`WorkflowProgressEntry::Agent`]. Boxed at the variant because +/// — unlike [`StreamEvent`], which is built one at a time — these are held +/// by the hundred in a `Vec`, so an unboxed variant would size every entry +/// (including bare `Phase` headings) to this struct's footprint. +/// +/// Every field is `#[serde(default)]` on purpose. A missing or renamed +/// field upstream would otherwise fail the entire enclosing `StreamEvent`, +/// costing us the whole progress tick rather than one attribute of one row. +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, Default)] +#[serde(rename_all = "camelCase", default)] +pub struct WorkflowAgentProgress { + /// Position in the run's global agent ordering. Stable across + /// re-emissions, so the UI keys rows on it. + pub index: i64, + /// Display label — the script's `opts.label`, or a summary of the prompt. + pub label: String, + pub phase_index: Option, + pub phase_title: Option, + pub agent_id: Option, + /// Custom subagent type (`opts.agentType`), when the script set one. + pub agent_type: Option, + /// `"worktree"` or `"remote"` when the agent runs isolated. + pub isolation: Option, + pub remote_session_id: Option, + pub model: Option, + pub fallback_model: Option, + /// Observed: `"queued"`, `"progress"`, `"done"`, `"error"`. Kept as a + /// `String` rather than an enum so a future state can't break parsing — + /// same rationale as [`CompactMetadata::trigger`]. + pub state: String, + pub started_at: Option, + pub queued_at: Option, + pub last_progress_at: Option, + /// Retry counter, 1-based. `> 1` means an earlier attempt failed. + pub attempt: Option, + pub last_attempt_reason: Option, + pub last_tool_name: Option, + pub last_tool_summary: Option, + pub prompt_preview: Option, + pub result_preview: Option, + pub tokens: Option, + pub tool_calls: Option, + pub duration_ms: Option, + /// `true` when a resumed run served this agent from its prior result + /// instead of re-running it. + pub cached: Option, + pub error: Option, +} + /// Top-level JSON line from Claude CLI stdout. #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(tag = "type")] @@ -118,6 +207,14 @@ pub enum StreamEvent { /// Present on `task_progress` / `task_notification`. #[serde(default)] usage: Option, + /// Present on `task_progress` events for the `Workflow` tool: a full + /// snapshot of the run's phase/agent tree. Absent on the throttled + /// no-transition ticks, where it means "unchanged" — see + /// [`WorkflowProgressEntry`]. `skip_serializing_if` keeps it off the + /// re-emitted Tauri payload for the many system events that never + /// carry it. + #[serde(default, skip_serializing_if = "Option::is_none")] + workflow_progress: Option>, /// Only present on `subtype: "status"` events. Values observed: /// `"requesting"` (normal API call), `"compacting"` (compaction in /// flight), or `null` (compaction complete). @@ -214,6 +311,7 @@ impl StreamEvent { description: None, last_tool_name: None, usage: None, + workflow_progress: None, status: None, compact_result: None, compact_metadata: None, @@ -1342,3 +1440,137 @@ mod compaction_tests { } } } + +#[cfg(test)] +mod workflow_progress_tests { + use super::*; + + /// Shape captured from a real `Workflow` run's `task_progress` event. + /// Field names inside `workflow_progress` are camelCase while the + /// enclosing event is snake_case — that split is what this pins. + #[test] + fn parses_task_progress_workflow_tree() { + let line = r#"{ + "type": "system", + "subtype": "task_progress", + "task_id": "w4stpeffj", + "tool_use_id": "toolu_01AxY9PTrqaJ6k9apBCi3kue", + "description": "Review: review:completeness", + "last_tool_name": "StructuredOutput", + "usage": {"total_tokens": 84669, "tool_uses": 38, "duration_ms": 180027}, + "workflow_progress": [ + {"type": "workflow_phase", "index": 1, "title": "Review"}, + {"type": "workflow_agent", "index": 1, "label": "review:completeness", + "phaseIndex": 1, "phaseTitle": "Review", "agentId": "ae1e803ca4fd797bf", + "model": "claude-opus-4-8", "state": "done", "startedAt": 1782843838042, + "queuedAt": 1782843838028, "attempt": 1, "lastToolName": "StructuredOutput", + "lastToolSummary": "No functional completeness gaps", + "promptPreview": "CONTEXT - a code change", "lastProgressAt": 1782844018069, + "tokens": 84669, "toolCalls": 38, "durationMs": 180027, + "resultPreview": "{\"summary\":\"...\"}", "cached": false} + ] + }"#; + let ev: StreamEvent = serde_json::from_str(line).unwrap(); + let StreamEvent::System { + subtype, + workflow_progress, + .. + } = ev + else { + panic!("expected System event"); + }; + assert_eq!(subtype, "task_progress"); + let entries = workflow_progress.expect("workflow_progress parsed"); + assert_eq!(entries.len(), 2); + assert_eq!( + entries[0], + WorkflowProgressEntry::Phase { + index: 1, + title: "Review".to_string(), + } + ); + let WorkflowProgressEntry::Agent(agent) = &entries[1] else { + panic!("expected Agent entry, got {:?}", entries[1]); + }; + assert_eq!(agent.label, "review:completeness"); + assert_eq!(agent.state, "done"); + assert_eq!(agent.phase_title.as_deref(), Some("Review")); + assert_eq!(agent.tokens, Some(84669)); + assert_eq!(agent.tool_calls, Some(38)); + assert_eq!(agent.duration_ms, Some(180027)); + assert_eq!(agent.cached, Some(false)); + } + + /// An entry kind we don't model must not cost us the rest of the tree — + /// the surrounding phases and agents still have to parse. + #[test] + fn unknown_workflow_progress_entry_does_not_fail_the_line() { + let line = r#"{"type":"system","subtype":"task_progress","workflow_progress":[ + {"type":"workflow_log","message":"3/10 found"}, + {"type":"workflow_phase","index":2,"title":"Verify"} + ]}"#; + let ev: StreamEvent = serde_json::from_str(line).unwrap(); + let StreamEvent::System { + workflow_progress, .. + } = ev + else { + panic!("expected System event"); + }; + let entries = workflow_progress.expect("workflow_progress parsed"); + assert_eq!(entries.len(), 2); + assert_eq!(entries[0], WorkflowProgressEntry::Unknown); + assert_eq!( + entries[1], + WorkflowProgressEntry::Phase { + index: 2, + title: "Verify".to_string(), + } + ); + } + + /// A `workflow_agent` stripped of every optional field must still parse. + /// Everything is `#[serde(default)]` precisely so one renamed attribute + /// upstream can't cost us the whole progress tick. + #[test] + fn workflow_agent_tolerates_missing_fields() { + let line = r#"{"type":"system","subtype":"task_progress","workflow_progress":[ + {"type":"workflow_agent"} + ]}"#; + let ev: StreamEvent = serde_json::from_str(line).unwrap(); + let StreamEvent::System { + workflow_progress, .. + } = ev + else { + panic!("expected System event"); + }; + let entries = workflow_progress.expect("workflow_progress parsed"); + let WorkflowProgressEntry::Agent(agent) = &entries[0] else { + panic!("expected Agent entry"); + }; + assert_eq!(agent.label, ""); + assert_eq!(agent.state, ""); + assert_eq!(agent.tokens, None); + } + + /// Throttled no-transition ticks omit `workflow_progress` entirely. + /// It must deserialize to `None` (meaning "unchanged"), and must not be + /// re-emitted as an explicit `null` on the many system events that never + /// carry it. + #[test] + fn task_progress_without_workflow_tree_is_none_and_not_reserialized() { + let line = r#"{"type":"system","subtype":"task_progress","task_id":"w4stpeffj"}"#; + let ev: StreamEvent = serde_json::from_str(line).unwrap(); + let StreamEvent::System { + workflow_progress, .. + } = &ev + else { + panic!("expected System event"); + }; + assert!(workflow_progress.is_none()); + let re = serde_json::to_string(&ev).unwrap(); + assert!( + !re.contains("workflow_progress"), + "absent tree should not be re-emitted: {re}" + ); + } +} diff --git a/src/db/checkpoint.rs b/src/db/checkpoint.rs index 5d5946bc0..3cfacc48a 100644 --- a/src/db/checkpoint.rs +++ b/src/db/checkpoint.rs @@ -796,9 +796,9 @@ impl Database { result_text, summary, sort_order, assistant_message_ordinal, agent_task_id, agent_description, agent_last_tool_name, agent_tool_use_count, agent_status, agent_tool_calls_json, - agent_thinking_blocks_json, agent_result_text + agent_thinking_blocks_json, agent_result_text, workflow_progress_json ) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17)", + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18)", )?; for a in activities { stmt.execute(params![ @@ -819,6 +819,7 @@ impl Database { a.agent_tool_calls_json, a.agent_thinking_blocks_json, a.agent_result_text, + a.workflow_progress_json, ])?; } } @@ -857,9 +858,9 @@ impl Database { result_text, summary, sort_order, assistant_message_ordinal, agent_task_id, agent_description, agent_last_tool_name, agent_tool_use_count, agent_status, agent_tool_calls_json, - agent_thinking_blocks_json, agent_result_text + agent_thinking_blocks_json, agent_result_text, workflow_progress_json ) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17)", + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18)", )?; for a in activities { stmt.execute(params![ @@ -880,6 +881,7 @@ impl Database { a.agent_tool_calls_json, a.agent_thinking_blocks_json, a.agent_result_text, + a.workflow_progress_json, ])?; } } @@ -903,7 +905,7 @@ impl Database { ta.assistant_message_ordinal, ta.agent_task_id, ta.agent_description, ta.agent_last_tool_name, ta.agent_tool_use_count, ta.agent_status, ta.agent_tool_calls_json, - ta.agent_thinking_blocks_json, ta.agent_result_text + ta.agent_thinking_blocks_json, ta.agent_result_text, ta.workflow_progress_json FROM turn_tool_activities ta JOIN conversation_checkpoints cp ON ta.checkpoint_id = cp.id WHERE cp.workspace_id = ?1 @@ -929,6 +931,7 @@ impl Database { agent_tool_calls_json: row.get(14)?, agent_thinking_blocks_json: row.get(15)?, agent_result_text: row.get(16)?, + workflow_progress_json: row.get(17)?, }) })? .collect::, _>>()?; @@ -1012,7 +1015,7 @@ impl Database { ta.assistant_message_ordinal, ta.agent_task_id, ta.agent_description, ta.agent_last_tool_name, ta.agent_tool_use_count, ta.agent_status, ta.agent_tool_calls_json, - ta.agent_thinking_blocks_json, ta.agent_result_text + ta.agent_thinking_blocks_json, ta.agent_result_text, ta.workflow_progress_json FROM turn_tool_activities ta JOIN conversation_checkpoints cp ON ta.checkpoint_id = cp.id WHERE cp.chat_session_id = ?1 @@ -1038,6 +1041,7 @@ impl Database { agent_tool_calls_json: row.get(14)?, agent_thinking_blocks_json: row.get(15)?, agent_result_text: row.get(16)?, + workflow_progress_json: row.get(17)?, }) })? .collect::, _>>()?; @@ -1122,6 +1126,7 @@ mod tests { agent_tool_calls_json: "[]".into(), agent_thinking_blocks_json: "[]".into(), agent_result_text: None, + workflow_progress_json: "[]".into(), } } diff --git a/src/fork.rs b/src/fork.rs index 463758c38..781fe5717 100644 --- a/src/fork.rs +++ b/src/fork.rs @@ -462,6 +462,7 @@ fn copy_history( agent_tool_calls_json: a.agent_tool_calls_json.clone(), agent_thinking_blocks_json: a.agent_thinking_blocks_json.clone(), agent_result_text: a.agent_result_text.clone(), + workflow_progress_json: a.workflow_progress_json.clone(), }) .collect(); db.insert_turn_tool_activities(&remapped)?; @@ -746,6 +747,7 @@ mod tests { agent_tool_calls_json: "[]".into(), agent_thinking_blocks_json: "[]".into(), agent_result_text: None, + workflow_progress_json: "[]".into(), }]) .unwrap(); db.insert_turn_tool_activities(&[TurnToolActivity { @@ -766,6 +768,7 @@ mod tests { agent_tool_calls_json: "[]".into(), agent_thinking_blocks_json: "[]".into(), agent_result_text: None, + workflow_progress_json: "[]".into(), }]) .unwrap(); db diff --git a/src/migrations/20260723155203_turn_tool_activity_workflow_progress.sql b/src/migrations/20260723155203_turn_tool_activity_workflow_progress.sql new file mode 100644 index 000000000..480bbca85 --- /dev/null +++ b/src/migrations/20260723155203_turn_tool_activity_workflow_progress.sql @@ -0,0 +1,10 @@ +-- Stores the phase/agent tree that Claude Code's `Workflow` tool reports +-- on `subtype: "task_progress"` stream events, so a completed run stays +-- readable after a reload and in DB-replayed history (see +-- `reconstructTurns.ts`). Mirrors the `agent_tool_calls_json` column: +-- opaque JSON to SQLite, parsed on the way out. +-- +-- Single statement so the runner's "already exists" leniency applies +-- cleanly if the column is ever hand-applied on a dev DB. +ALTER TABLE turn_tool_activities + ADD COLUMN workflow_progress_json TEXT NOT NULL DEFAULT '[]'; diff --git a/src/migrations/mod.rs b/src/migrations/mod.rs index 2017c9cf9..b0e5dfd5d 100644 --- a/src/migrations/mod.rs +++ b/src/migrations/mod.rs @@ -269,4 +269,9 @@ pub const MIGRATIONS: &[Migration] = &[ sql: include_str!("20260611120000_agent_conclusions.sql"), legacy_version: None, }, + Migration { + id: "20260723155203_turn_tool_activity_workflow_progress", + sql: include_str!("20260723155203_turn_tool_activity_workflow_progress.sql"), + legacy_version: None, + }, ]; diff --git a/src/model/checkpoint.rs b/src/model/checkpoint.rs index 7a132db62..4a2640e93 100644 --- a/src/model/checkpoint.rs +++ b/src/model/checkpoint.rs @@ -61,6 +61,12 @@ pub struct TurnToolActivity { pub agent_thinking_blocks_json: String, #[serde(default)] pub agent_result_text: Option, + /// Serialized `Vec` — the + /// phase/agent tree of a `Workflow` tool run. `"[]"` for every other + /// tool. Defaulted on deserialize so activity payloads persisted before + /// the column existed still load. + #[serde(default = "empty_json_array")] + pub workflow_progress_json: String, } fn empty_json_array() -> String { diff --git a/src/ui/src/hooks/useAgentStream.ts b/src/ui/src/hooks/useAgentStream.ts index 7834691e1..f3794a339 100644 --- a/src/ui/src/hooks/useAgentStream.ts +++ b/src/ui/src/hooks/useAgentStream.ts @@ -319,6 +319,15 @@ export function useAgentStream() { if (typeof streamEvent.usage?.tool_uses === "number") { updates.agentToolUseCount = streamEvent.usage.tool_uses; } + // Full replacement snapshot of a Workflow run's phase/agent + // tree. The CLI only attaches it on real state transitions; + // the throttled in-between ticks omit the key entirely, and + // that absence means "unchanged". Assigning unconditionally + // would blank the card every ~10s mid-run, so only touch + // `workflowProgress` when the key is actually present. + if (Array.isArray(streamEvent.workflow_progress)) { + updates.workflowProgress = streamEvent.workflow_progress; + } if (streamEvent.status) { updates.agentStatus = streamEvent.status; } else if (streamEvent.subtype === "task_progress") { @@ -1011,6 +1020,7 @@ export function useAgentStream() { a.agentThinkingBlocks ?? [], ), agent_result_text: a.agentResultText ?? null, + workflow_progress_json: JSON.stringify(a.workflowProgress ?? []), })); return saveTurnToolActivities(checkpoint.id, messageCount, activities); })() diff --git a/src/ui/src/stores/slices/chatSlice.ts b/src/ui/src/stores/slices/chatSlice.ts index 3eb2c1310..201e83a42 100644 --- a/src/ui/src/stores/slices/chatSlice.ts +++ b/src/ui/src/stores/slices/chatSlice.ts @@ -8,6 +8,7 @@ import type { import type { StoredAttachment } from "../../types/chat"; import { debugChat } from "../../utils/chatDebug"; import type { CompactionEvent } from "../../utils/compactionSentinel"; +import type { WorkflowProgressEntry } from "../../types/workflow"; import type { AppState } from "../useAppStore"; export interface ToolActivity { @@ -27,6 +28,10 @@ export interface ToolActivity { agentToolCalls?: AgentToolCall[]; agentThinkingBlocks?: string[]; agentResultText?: string | null; + /** Phase/agent tree for a `Workflow` tool activity. Undefined for every + * other tool, and also for a workflow whose first progress tick hasn't + * landed yet. */ + workflowProgress?: WorkflowProgressEntry[]; } export interface AgentToolCall { @@ -585,6 +590,7 @@ export const createChatSlice: StateCreator = ( agentToolCalls: a.agentToolCalls, agentThinkingBlocks: a.agentThinkingBlocks, agentResultText: a.agentResultText, + workflowProgress: a.workflowProgress, })), messageCount, collapsed: true, diff --git a/src/ui/src/types/agent-events.ts b/src/ui/src/types/agent-events.ts index 2af874878..fd3dd4000 100644 --- a/src/ui/src/types/agent-events.ts +++ b/src/ui/src/types/agent-events.ts @@ -1,3 +1,5 @@ +import type { WorkflowProgressEntry } from "./workflow"; + /** Payload shape emitted from the Rust backend via Tauri events. */ export interface AgentStreamPayload { workspace_id: string; @@ -41,6 +43,12 @@ export type StreamEvent = tool_uses?: number | null; duration_ms?: number | null; } | null; + /** Present on `subtype: "task_progress"` events for the `Workflow` + * tool: a full replacement snapshot of the run's phase/agent tree. + * Absent (not `null` — Rust skips the key) on the throttled + * no-transition ticks, where it means "unchanged". Consumers must + * treat `undefined` as "keep previous", never as "clear". */ + workflow_progress?: WorkflowProgressEntry[]; /** Only present on the end-of-compaction status event. Rust * serializes `Option` as `null` (no `skip_serializing_if`), * so the wire payload carries `null` when absent. */ diff --git a/src/ui/src/types/checkpoint.ts b/src/ui/src/types/checkpoint.ts index 22518e85c..77dd90608 100644 --- a/src/ui/src/types/checkpoint.ts +++ b/src/ui/src/types/checkpoint.ts @@ -27,6 +27,9 @@ export interface TurnToolActivityData { agent_tool_calls_json: string; agent_thinking_blocks_json: string; agent_result_text: string | null; + /** Serialized `WorkflowProgressEntry[]` for a `Workflow` tool activity; + * `"[]"` for every other tool. */ + workflow_progress_json: string; } export interface CompletedTurnData { diff --git a/src/ui/src/types/workflow.ts b/src/ui/src/types/workflow.ts new file mode 100644 index 000000000..9e28a5e79 --- /dev/null +++ b/src/ui/src/types/workflow.ts @@ -0,0 +1,173 @@ +/** The phase/agent tree Claude Code's `Workflow` tool reports on + * `subtype: "task_progress"` stream events. + * + * Mirrors `WorkflowProgressEntry` / `WorkflowAgentProgress` in + * `src/agent/types.rs`. Field names are camelCase because that is what the + * CLI emits inside these entries — the enclosing `system` event is + * snake_case, and Rust re-serializes the inner struct with + * `rename_all = "camelCase"` to preserve that. + * + * Optional fields are typed `| null` (not just `?`) because Rust's + * `Option` serializes as an explicit `null` here — only the top-level + * `workflow_progress` array itself carries `skip_serializing_if`. + */ +export interface WorkflowPhaseEntry { + type: "workflow_phase"; + index: number; + title: string; +} + +export interface WorkflowAgentEntry { + type: "workflow_agent"; + /** Position in the run's global agent ordering. Stable across + * re-emissions, so it is safe to use as a React key. */ + index: number; + label: string; + phaseIndex?: number | null; + phaseTitle?: string | null; + agentId?: string | null; + agentType?: string | null; + /** `"worktree"` or `"remote"` when the agent runs isolated. */ + isolation?: string | null; + remoteSessionId?: string | null; + model?: string | null; + fallbackModel?: string | null; + /** Observed: `"queued"`, `"progress"`, `"done"`, `"error"`. Deliberately + * a bare `string` — the Rust side keeps it unconstrained so a new + * upstream state can't break parsing, and the UI must not assume the + * set is closed either. */ + state: string; + startedAt?: number | null; + queuedAt?: number | null; + lastProgressAt?: number | null; + /** Retry counter, 1-based. `> 1` means an earlier attempt failed. */ + attempt?: number | null; + lastAttemptReason?: string | null; + lastToolName?: string | null; + lastToolSummary?: string | null; + promptPreview?: string | null; + resultPreview?: string | null; + tokens?: number | null; + toolCalls?: number | null; + durationMs?: number | null; + /** `true` when a resumed run served this agent from cache rather than + * re-running it. */ + cached?: boolean | null; + error?: string | null; +} + +/** Entry kinds we don't model. Rust maps any unrecognized `type` here so a + * future upstream addition can't cost us the rest of the tree. */ +export interface WorkflowUnknownEntry { + type: "Unknown"; +} + +export type WorkflowProgressEntry = + | WorkflowPhaseEntry + | WorkflowAgentEntry + | WorkflowUnknownEntry; + +export function isWorkflowPhase( + entry: WorkflowProgressEntry, +): entry is WorkflowPhaseEntry { + return entry.type === "workflow_phase"; +} + +export function isWorkflowAgent( + entry: WorkflowProgressEntry, +): entry is WorkflowAgentEntry { + return entry.type === "workflow_agent"; +} + +/** Terminal states — an agent in one of these will not change again. + * Anything else (including a state we've never seen) counts as in-flight, + * so an unrecognized value degrades to "still running" rather than + * silently reporting a run as finished. */ +const TERMINAL_AGENT_STATES = new Set(["done", "error"]); + +export function isAgentTerminal(agent: WorkflowAgentEntry): boolean { + return TERMINAL_AGENT_STATES.has(agent.state); +} + +export interface WorkflowRunSummary { + phases: WorkflowPhaseEntry[]; + agents: WorkflowAgentEntry[]; + /** Agents in a terminal state. */ + doneCount: number; + /** Agents that ended in `error`. */ + errorCount: number; + totalCount: number; + /** True while any agent is still queued or in flight. */ + running: boolean; + totalTokens: number; + totalToolCalls: number; + /** Phase title of the newest non-terminal agent, else the last phase seen. + * Drives the compact pill's "Phase: Verify" text. */ + currentPhaseTitle: string | null; +} + +/** + * Collapse a raw progress array into render-ready totals. + * + * Two things this has to get right: + * + * 1. **De-duplication.** The CLI re-emits a full `workflow_agent` entry on + * every state transition, so the same `index` appears repeatedly within + * one snapshot. Last-write-wins per index, which matches the upstream + * semantics ("the latest entry supersedes earlier ones") and keeps + * counts from inflating past `totalCount`. + * 2. **Order.** Agents come back in first-seen order, not index order, so + * a phase-2 agent that started before a slow phase-1 agent finished + * doesn't jump the list on a later tick. + */ +export function summarizeWorkflowProgress( + entries: WorkflowProgressEntry[] | undefined, +): WorkflowRunSummary { + const phases: WorkflowPhaseEntry[] = []; + const agentsByIndex = new Map(); + const order: number[] = []; + + for (const entry of entries ?? []) { + if (isWorkflowPhase(entry)) { + phases.push(entry); + } else if (isWorkflowAgent(entry)) { + if (!agentsByIndex.has(entry.index)) order.push(entry.index); + agentsByIndex.set(entry.index, entry); + } + } + + const agents = order.map((index) => agentsByIndex.get(index)!); + + let doneCount = 0; + let errorCount = 0; + let totalTokens = 0; + let totalToolCalls = 0; + let currentPhaseTitle: string | null = null; + + for (const agent of agents) { + if (isAgentTerminal(agent)) doneCount++; + else if (currentPhaseTitle === null) currentPhaseTitle = agent.phaseTitle ?? null; + if (agent.state === "error") errorCount++; + totalTokens += agent.tokens ?? 0; + totalToolCalls += agent.toolCalls ?? 0; + } + + // No in-flight agent to borrow a phase from (run finished, or the first + // tick arrived before any agent did) — fall back to the last declared + // phase so the header still says something useful. + if (currentPhaseTitle === null && phases.length > 0) { + currentPhaseTitle = phases[phases.length - 1].title; + } + + return { + phases, + agents, + doneCount, + errorCount, + totalCount: agents.length, + running: agents.length > 0 && doneCount < agents.length, + totalTokens, + totalToolCalls, + currentPhaseTitle, + }; +} diff --git a/src/ui/src/utils/reconstructTurns.test.ts b/src/ui/src/utils/reconstructTurns.test.ts index 2313e3bbb..4fedce329 100644 --- a/src/ui/src/utils/reconstructTurns.test.ts +++ b/src/ui/src/utils/reconstructTurns.test.ts @@ -53,6 +53,7 @@ function makeTurnData( agent_tool_calls_json: "[]", agent_thinking_blocks_json: "[]", agent_result_text: null, + workflow_progress_json: "[]", })), }; } @@ -342,6 +343,7 @@ describe("reconstructCompletedTurns", () => { agent_tool_calls_json: "[]", agent_thinking_blocks_json: "[]", agent_result_text: null, + workflow_progress_json: "[]", }, ], }, diff --git a/src/ui/src/utils/reconstructTurns.ts b/src/ui/src/utils/reconstructTurns.ts index e460ecde7..6f54cd6a6 100644 --- a/src/ui/src/utils/reconstructTurns.ts +++ b/src/ui/src/utils/reconstructTurns.ts @@ -1,6 +1,7 @@ import type { CompletedTurn } from "../stores/useAppStore"; import type { ChatMessage } from "../types/chat"; import type { CompletedTurnData } from "../types/checkpoint"; +import type { WorkflowProgressEntry } from "../types/workflow"; import { debugChat } from "./chatDebug"; /** @@ -94,6 +95,7 @@ export function reconstructCompletedTurns( agentToolCalls: parseAgentToolCalls(a.agent_tool_calls_json), agentThinkingBlocks: parseStringArray(a.agent_thinking_blocks_json), agentResultText: a.agent_result_text, + workflowProgress: parseWorkflowProgress(a.workflow_progress_json), })), messageCount: td.message_count, collapsed: true, @@ -118,6 +120,35 @@ function parseAgentToolCalls(value: string | null | undefined) { } } +/** Rehydrate a persisted `Workflow` progress tree. + * + * Returns `undefined` rather than `[]` for the empty case so a non-workflow + * activity (every one of which stores `"[]"`) doesn't come back looking + * like a workflow that produced no agents — `WorkflowCard` keys off + * `workflowProgress` being present at all. + * + * Entries are not validated beyond "is an object with a `type`". The Rust + * side already normalized unrecognized kinds to `{"type":"Unknown"}` on + * the way in, and the renderer switches on `type` with a default of + * "skip", so a malformed row degrades to an omitted line rather than a + * thrown render. */ +function parseWorkflowProgress( + value: string | null | undefined, +): WorkflowProgressEntry[] | undefined { + if (!value) return undefined; + try { + const parsed = JSON.parse(value); + if (!Array.isArray(parsed) || parsed.length === 0) return undefined; + const entries = parsed.filter( + (item): item is WorkflowProgressEntry => + typeof item === "object" && item !== null && typeof item.type === "string", + ); + return entries.length > 0 ? entries : undefined; + } catch { + return undefined; + } +} + function parseStringArray(value: string | null | undefined) { if (!value) return undefined; try { From f9875dfbb46e9ea1ed7fb4c770fb2a6042fecd03 Mon Sep 17 00:00:00 2001 From: Sean Callan Date: Thu, 23 Jul 2026 12:09:17 -0400 Subject: [PATCH 02/14] feat(chat): render Workflow runs as a live phase/agent card MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the single tool row — whose summary was a truncated dump of the minified script — with a card showing the run's phases, per-agent state, model, token/tool counts, elapsed time, and latest tool summary, plus a progress rail. Two workflow-specific traps the card has to avoid: - "finished" is NOT `resultText.length > 0`, the idiom every other tool uses. A workflow's tool_result lands a second after launch while the run takes minutes; completion arrives later as a task-notification, which `useAgentStream` maps onto `agentStatus`. - Elapsed time is interpolated from `startedAt` only on the live path. Progress ticks are throttled to 10s, so reading elapsed off the last tick would look frozen — but a run interrupted by an app restart replays with agents still marked `progress`, and interpolating those against the current clock would report days of work that never happened. Workflows default to expanded, inverting the "collapsed is quiet" stance used for agents and tool groups: the card is the only indication that a backgrounded run is progressing at all. The script stays reachable behind a disclosure — it was visible in ToolActivityRow's expanded view before, and the card replaces that path. `extractToolSummary` gets a Workflow case so chat search stops matching against minified JavaScript. --- .../components/chat/ToolActivitiesSection.tsx | 60 +++ src/ui/src/components/chat/TurnSummary.tsx | 21 +- .../components/chat/WorkflowCard.module.css | 262 ++++++++++++ .../src/components/chat/WorkflowCard.test.tsx | 266 ++++++++++++ src/ui/src/components/chat/WorkflowCard.tsx | 388 ++++++++++++++++++ src/ui/src/components/chat/chatHelpers.ts | 2 + .../components/chat/collapsedToolGroupKey.ts | 20 +- .../src/components/chat/toolActivityGroups.ts | 33 +- .../src/components/chat/workflowMeta.test.ts | 135 ++++++ src/ui/src/components/chat/workflowMeta.ts | 152 +++++++ src/ui/src/hooks/toolSummary.ts | 13 + src/ui/src/types/workflow.test.ts | 137 +++++++ 12 files changed, 1467 insertions(+), 22 deletions(-) create mode 100644 src/ui/src/components/chat/WorkflowCard.module.css create mode 100644 src/ui/src/components/chat/WorkflowCard.test.tsx create mode 100644 src/ui/src/components/chat/WorkflowCard.tsx create mode 100644 src/ui/src/components/chat/workflowMeta.test.ts create mode 100644 src/ui/src/components/chat/workflowMeta.ts create mode 100644 src/ui/src/types/workflow.test.ts diff --git a/src/ui/src/components/chat/ToolActivitiesSection.tsx b/src/ui/src/components/chat/ToolActivitiesSection.tsx index eeb5b9b87..d5cd9032d 100644 --- a/src/ui/src/components/chat/ToolActivitiesSection.tsx +++ b/src/ui/src/components/chat/ToolActivitiesSection.tsx @@ -7,6 +7,7 @@ import styles from "./ChatPanel.module.css"; import { EMPTY_ACTIVITIES } from "./chatConstants"; import { activityMatchesSearch } from "./agentToolCallRendering"; import { AgentToolCallGroup } from "./AgentToolCallGroup"; +import { WorkflowCard } from "./WorkflowCard"; import { ToolActivityRow } from "./ToolActivityRow"; import { groupToolActivitiesForDisplay } from "./toolActivityGroups"; import { collapsedToolGroupKey } from "./collapsedToolGroupKey"; @@ -55,6 +56,21 @@ export const ToolActivitiesSection = memo(function ToolActivitiesSection({ worktreePath={worktreePath} inline={toolDisplayMode === "inline"} /> + ) : group.kind === "workflow" && group.activities[0] ? ( + toolDisplayMode === "inline" ? ( + + ) : ( + + ) ) : group.kind === "agent" && group.activities[0] ? ( // Inline mode keeps the legacy always-expanded Agent rendering; // grouped mode wraps the same component with a chevron+toggle @@ -218,6 +234,50 @@ function GroupedToolActivityRows({ ); } +/** + * Live wrapper around a single Workflow activity. + * + * Unlike agents and tool groups, workflows default to **expanded**. A + * workflow's tool_result lands within a second ("launched in background") + * while the run itself takes minutes, so the usual "collapsed is quiet" + * argument inverts here: collapsing by default would hide the only + * indication that anything is happening for the entire run. The header, + * progress rail, and agent tree are what make a background workflow legible + * at all. + * + * Collapse state persists through the same `collapsedToolGroupsBySession` + * slice as every other group, keyed `workflow:`, so the user's + * choice survives the running→completed transition into `TurnSummary`. + */ +function GroupedWorkflowActivity({ + sessionId, + activity, +}: { + sessionId: string; + activity: ToolActivity; +}) { + const groupKey = collapsedToolGroupKey([activity]); + const userOverride = useAppStore((s) => + groupKey ? s.collapsedToolGroupsBySession[sessionId]?.[groupKey] : undefined, + ); + const setCollapsedToolGroup = useAppStore((s) => s.setCollapsedToolGroup); + + const collapsed = userOverride ?? false; + const toggle = () => { + if (!groupKey) return; + setCollapsedToolGroup(sessionId, groupKey, !collapsed); + }; + + return ( + + ); +} + /** * Live (running-turn) wrapper around a single Agent activity that * adds a chevron + collapse toggle while leaving the existing diff --git a/src/ui/src/components/chat/TurnSummary.tsx b/src/ui/src/components/chat/TurnSummary.tsx index 4115b3213..5e507602a 100644 --- a/src/ui/src/components/chat/TurnSummary.tsx +++ b/src/ui/src/components/chat/TurnSummary.tsx @@ -8,8 +8,9 @@ import { TaskProgressBar } from "./TaskProgressBar"; import { activityMatchesSearch } from "./agentToolCallRendering"; import { toolColor } from "./chatHelpers"; import { AgentToolCallGroup } from "./AgentToolCallGroup"; +import { WorkflowCard } from "./WorkflowCard"; import { ToolActivityRow } from "./ToolActivityRow"; -import { isAgentActivity } from "./toolActivityGroups"; +import { isAgentActivity, isWorkflowActivity } from "./toolActivityGroups"; import { TurnEditSummaryCard } from "./EditChangeSummary"; import { type EditPreviewLine, @@ -17,15 +18,15 @@ import { summarizeTurnEdits, } from "./editActivitySummary"; -/// Split the leading "Agent" / "Skill" prefix on a turn label into a -/// colored span so the finalized summary matches the accent color used -/// while the turn was still running. Handles three label shapes: -/// • bare — "Agent" / "Skill" -/// • prefixed — "Agent " / "Skill " +/// Split the leading "Agent" / "Skill" / "Workflow" prefix on a turn label +/// into a colored span so the finalized summary matches the accent color +/// used while the turn was still running. Handles three label shapes: +/// • bare — "Agent" / "Skill" / "Workflow" +/// • prefixed — "Agent " / "Workflow " /// • anything else — rendered untouched /// Kept inline rather than promoted to a helper module — the only /// other consumer of `toolColor` already lives in TurnSummary. -const COLORED_PREFIX = /^(Agent|Skill)(?:\s+(.+))?$/; +const COLORED_PREFIX = /^(Agent|Skill|Workflow)(?:\s+(.+))?$/; function renderTurnLabel(label: string) { const match = COLORED_PREFIX.exec(label); if (!match) return label; @@ -133,6 +134,12 @@ export function TurnSummary({ ); const isExpanded = inline || !collapsed || queryHasMatch; const renderedActivities = visibleActivities.map((act: ToolActivity) => { + if (isWorkflowActivity(act)) { + // Always expanded here — the enclosing TurnSummary header already + // provides the collapse affordance for a finished run, and nesting a + // second chevron inside it would give the same card two of them. + return ; + } if (isAgentActivity(act)) { return ( { + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + mountedRoots.push(root); + mountedContainers.push(container); + await act(async () => { + root.render(node); + }); + return container; +} + +afterEach(async () => { + await act(async () => { + for (const root of mountedRoots.splice(0)) root.unmount(); + }); + for (const container of mountedContainers.splice(0)) container.remove(); + vi.useRealTimers(); +}); + +const SCRIPT = `export const meta = { + name: 'review-changes', + description: 'Review the diff across dimensions', + phases: [{ title: 'Review' }, { title: 'Verify' }], +} +phase('Review')`; + +function makeActivity(overrides: Partial = {}): ToolActivity { + return { + toolUseId: "toolu_wf1", + toolName: "Workflow", + inputJson: JSON.stringify({ script: SCRIPT }), + // A workflow's tool_result lands at LAUNCH, not completion — the real + // text the CLI returns. Several assertions below depend on the card + // not treating this as "finished". + resultText: "Workflow launched in background. Task ID: w4stpeffj", + collapsed: false, + summary: "review-changes", + agentStatus: "running", + ...overrides, + }; +} + +function agent( + index: number, + state: string, + extra: Record = {}, +): WorkflowProgressEntry { + return { + type: "workflow_agent", + index, + label: `agent-${index}`, + state, + ...extra, + } as WorkflowProgressEntry; +} + +describe("WorkflowCard", () => { + it("shows the meta name and description instead of the script", async () => { + const container = await render(); + expect(container.textContent).toContain("review-changes"); + expect(container.textContent).toContain("Review the diff across dimensions"); + // The script is behind a disclosure, but must never be the headline. + expect(container.querySelector("[class*=name]")?.textContent).toBe( + "review-changes", + ); + }); + + it("renders agents grouped under their phase headings", async () => { + const container = await render( + , + ); + const text = container.textContent ?? ""; + expect(text).toContain("Review"); + expect(text).toContain("Verify"); + expect(text).toContain("review:bugs"); + expect(text).toContain("verify:bugs"); + expect(text).toContain("1/2 agents"); + }); + + it("reports progress on the rail with accessible bounds", async () => { + const container = await render( + , + ); + const rail = container.querySelector("[role=progressbar]"); + expect(rail?.getAttribute("aria-valuenow")).toBe("2"); + expect(rail?.getAttribute("aria-valuemax")).toBe("3"); + }); + + // Regression guard for the trap this component sits on: every other tool + // treats a non-empty resultText as "finished", but a workflow's result + // arrives seconds after launch while the run continues for minutes. + it("does not treat the launch tool_result as completion", async () => { + const container = await render( + , + ); + expect(container.textContent).not.toContain("No agent activity"); + // Rail must not be in its completed styling while the run is live. + const fill = container.querySelector("[role=progressbar] > div"); + expect(fill?.className).not.toContain("railFillDone"); + }); + + it("marks the run complete once a terminal task status arrives", async () => { + const container = await render( + , + ); + const fill = container.querySelector("[role=progressbar] > div"); + expect(fill?.className).toContain("railFillDone"); + expect(container.textContent).toContain("2/2 agents"); + }); + + it("surfaces a failed agent's error and counts it", async () => { + const container = await render( + , + ); + const text = container.textContent ?? ""; + expect(text).toContain("1 failed"); + expect(text).toContain("schema validation failed"); + }); + + it("collapses the agent tree but keeps the header readable", async () => { + const container = await render( + {}} + />, + ); + expect(container.textContent).toContain("review-changes"); + expect(container.textContent).toContain("0/1 agents"); + expect(container.textContent).not.toContain("review:bugs"); + }); + + it("shows a starting state before the first progress tick", async () => { + const container = await render( + , + ); + expect(container.textContent).toContain("Starting workflow"); + expect(container.querySelector("[role=progressbar]")).toBeNull(); + }); + + it("keeps the script reachable behind a disclosure", async () => { + const container = await render(); + const details = container.querySelector("details"); + expect(details).not.toBeNull(); + expect(details?.hasAttribute("open")).toBe(false); + expect(details?.textContent).toContain("export const meta"); + expect(details?.querySelector("summary")?.textContent).toContain("6 lines"); + }); + + it("omits the script disclosure for a workflow launched by name", async () => { + const container = await render( + , + ); + expect(container.querySelector("details")).toBeNull(); + expect(container.textContent).toContain("saved-review"); + }); + + // A run interrupted by an app restart replays with agents still marked + // `progress` and a `startedAt` from whenever it happened. Interpolating + // against the current clock would report days of elapsed work, and would + // keep climbing on every reload. + it("shows no fabricated elapsed time for a replayed (non-live) agent", async () => { + const sixDaysAgo = Date.now() - 6 * 24 * 60 * 60 * 1000; + const container = await render( + , + ); + expect(container.textContent).not.toMatch(/\d+[dhm]/); + }); + + it("still shows a recorded duration when replayed", async () => { + const container = await render( + , + ); + expect(container.textContent).toContain("3m 0s"); + }); + + it("does not run the clock for a replayed (non-live) card", async () => { + vi.useFakeTimers(); + const container = await render( + , + ); + const before = container.textContent ?? ""; + await act(async () => { + vi.advanceTimersByTime(5_000); + }); + expect(container.textContent).toBe(before); + }); + + it("interpolates elapsed time for a live in-flight agent", async () => { + const container = await render( + , + ); + expect(container.textContent).toContain("42s"); + }); +}); diff --git a/src/ui/src/components/chat/WorkflowCard.tsx b/src/ui/src/components/chat/WorkflowCard.tsx new file mode 100644 index 000000000..65923e179 --- /dev/null +++ b/src/ui/src/components/chat/WorkflowCard.tsx @@ -0,0 +1,388 @@ +import { useEffect, useMemo, useState, type KeyboardEvent } from "react"; +import type { ToolActivity } from "../../stores/useAppStore"; +import { + isAgentTerminal, + summarizeWorkflowProgress, + type WorkflowAgentEntry, +} from "../../types/workflow"; +import { formatDurationMs } from "./chatHelpers"; +import { formatTokens } from "./formatTokens"; +import { + isWorkflowResume, + workflowDescription, + workflowDisplayName, +} from "./workflowMeta"; +import styles from "./WorkflowCard.module.css"; + +/** Grouping key for an agent with no phase — workflows can call `agent()` + * before any `phase()`, and those still need a home in the tree. */ +const UNPHASED = "__claudette_unphased__"; + +/** Read the inline `script` back out of the tool input. + * + * Kept reachable because it used to be: before this card existed a + * workflow rendered through `ToolActivityRow`, whose expanded view showed + * the script. Returns null for workflows launched by `name` or + * `scriptPath`, where there is no inline source to show. */ +function workflowScript(inputJson: string): string | null { + try { + const parsed: unknown = JSON.parse(inputJson || "{}"); + if (!parsed || typeof parsed !== "object") return null; + const script = (parsed as { script?: unknown }).script; + return typeof script === "string" && script.trim() ? script : null; + } catch { + return null; + } +} + +/** Statuses that mean the background task itself has ended. + * + * These arrive on the `task-notification` the CLI emits when a workflow + * finishes — `useAgentStream` copies the notification's `status` onto + * `agentStatus`. While the run is in flight the same field reads + * `"running"` (set by the `task_progress` branch). */ +const TERMINAL_WORKFLOW_STATUSES = new Set([ + "completed", + "failed", + "error", + "killed", + "cancelled", + "canceled", +]); + +function isTerminalWorkflowStatus(status: string | null | undefined): boolean { + return TERMINAL_WORKFLOW_STATUSES.has((status ?? "").toLowerCase()); +} + +function stateIcon(state: string): string { + switch (state) { + case "done": + return "●"; + case "error": + return "✕"; + case "progress": + return "◐"; + default: + // Covers "queued" and any state we don't recognize — both mean + // "hasn't produced anything yet" as far as the reader is concerned. + return "○"; + } +} + +function stateClass(state: string): string { + switch (state) { + case "done": + return styles.stateDone; + case "error": + return styles.stateError; + case "progress": + return styles.stateProgress; + default: + return styles.stateQueued; + } +} + +/** + * Elapsed milliseconds for one agent. + * + * A finished agent reports `durationMs` and we use it verbatim. + * + * A running one doesn't — and its `lastProgressAt` only advances when the + * CLI sends a tick, which is throttled to once every 10s for a workflow + * that is otherwise just working. Reading elapsed off the last tick would + * make every in-flight row look frozen for ten seconds at a stretch, so a + * live row interpolates from `startedAt` against the local clock instead. + * + * `live` gates that interpolation, and the gate is load-bearing: a run + * interrupted by an app restart replays with agents still marked + * `progress` and a `startedAt` from whenever it happened. Interpolating + * those against *now* would report an agent as having run for six days. + * Off the live path we show nothing rather than a fabricated duration. + */ +function agentElapsedMs( + agent: WorkflowAgentEntry, + now: number, + live: boolean, +): number | null { + if (typeof agent.durationMs === "number") return agent.durationMs; + if (!live || isAgentTerminal(agent)) return null; + if (typeof agent.startedAt !== "number") return null; + return Math.max(0, now - agent.startedAt); +} + +/** Ticks once a second while `active`, so interpolated elapsed times + * advance. Returns a stable value when inactive so a finished card never + * re-renders on a timer. */ +function useSecondTick(active: boolean): number { + const [now, setNow] = useState(() => Date.now()); + useEffect(() => { + if (!active) return; + setNow(Date.now()); + const id = setInterval(() => setNow(Date.now()), 1000); + return () => clearInterval(id); + }, [active]); + return now; +} + +function AgentRow({ + agent, + now, + live, +}: { + agent: WorkflowAgentEntry; + now: number; + live: boolean; +}) { + const elapsedMs = agentElapsedMs(agent, now, live); + const detail = + agent.lastToolSummary?.trim() || + (agent.lastToolName ? `${agent.lastToolName}…` : ""); + + return ( +
+ + {stateIcon(agent.state)} + + + {agent.label || `agent ${agent.index}`} + + + {agent.cached && cached} + {typeof agent.attempt === "number" && agent.attempt > 1 && ( + + retry {agent.attempt} + + )} + {agent.isolation && {agent.isolation}} + {typeof agent.tokens === "number" && agent.tokens > 0 && ( + {formatTokens(agent.tokens)} + )} + {typeof agent.toolCalls === "number" && agent.toolCalls > 0 && ( + + {agent.toolCalls} tool{agent.toolCalls !== 1 ? "s" : ""} + + )} + {elapsedMs !== null && {formatDurationMs(elapsedMs)}} + + {agent.error ? ( + + {agent.error} + + ) : ( + detail && ( + + {detail} + + ) + )} +
+ ); +} + +/** + * Renders a `Workflow` tool call as its live phase/agent tree. + * + * Before this existed, a workflow showed up as one tool row whose summary + * was a truncated dump of the minified script, followed by several minutes + * of nothing while the run happened out of view. + * + * The tree arrives on `system`/`task_progress` events (see + * `useAgentStream`) and is persisted with the turn, so a completed run + * renders identically after a reload. + */ +export function WorkflowCard({ + activity, + collapsed, + onToggle, + inline = false, + live = false, +}: { + activity: ToolActivity; + /** Omit (with `onToggle`) for the always-expanded inline display mode. */ + collapsed?: boolean; + onToggle?: () => void; + inline?: boolean; + /** True when rendered from the in-flight turn's activities rather than + * replayed from a completed turn. Only a live card runs the clock. */ + live?: boolean; +}) { + const name = useMemo( + () => workflowDisplayName(activity.inputJson), + [activity.inputJson], + ); + const description = useMemo( + () => workflowDescription(activity.inputJson), + [activity.inputJson], + ); + const isResume = useMemo( + () => isWorkflowResume(activity.inputJson), + [activity.inputJson], + ); + const summary = useMemo( + () => summarizeWorkflowProgress(activity.workflowProgress), + [activity.workflowProgress], + ); + const script = useMemo( + () => workflowScript(activity.inputJson), + [activity.inputJson], + ); + const scriptLineCount = useMemo( + () => (script ? script.split("\n").length : 0), + [script], + ); + + // NOT `resultText.length > 0` — the idiom used for every other tool. A + // workflow's tool_result lands within a second of launch ("Workflow + // launched in background. Task ID: …"); the run itself takes minutes. + // Completion arrives much later as a task-notification, which + // `useAgentStream` maps onto `agentStatus`. + const finished = isTerminalWorkflowStatus(activity.agentStatus); + // `live` gates the interpolated clock. A run that was still going when + // the app closed replays with non-terminal agents forever; without this + // its elapsed times would tick upward on every reload, reporting minutes + // of work that never happened. + const isRunning = live && !finished; + const now = useSecondTick(isRunning); + + const collapsible = !inline && typeof onToggle === "function"; + const isCollapsed = collapsible && collapsed === true; + + // Group agents under their phase, preserving the order phases were + // declared in and falling back to first-appearance order for agents whose + // phase was never declared via `phase()`. + const grouped = useMemo(() => { + const byPhase = new Map(); + for (const agent of summary.agents) { + const key = agent.phaseTitle ?? UNPHASED; + const bucket = byPhase.get(key); + if (bucket) bucket.push(agent); + else byPhase.set(key, [agent]); + } + return [...byPhase.entries()].map(([title, agents]) => ({ + title: title === UNPHASED ? null : title, + agents, + })); + }, [summary.agents]); + + const headerInteractiveProps = collapsible + ? { + role: "button" as const, + tabIndex: 0, + "aria-expanded": !isCollapsed, + "aria-label": `${isCollapsed ? "Expand" : "Collapse"} workflow ${name}`, + onClick: onToggle, + onKeyDown: (e: KeyboardEvent) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + onToggle(); + } + }, + } + : undefined; + + const progressPct = + summary.totalCount > 0 + ? Math.round((summary.doneCount / summary.totalCount) * 100) + : 0; + + return ( +
+
+ {collapsible && ( + + )} + Workflow + + {name} + + + {isResume && resumed} + {summary.errorCount > 0 && ( + + {summary.errorCount} failed + + )} + {summary.totalCount > 0 && ( + + {summary.doneCount}/{summary.totalCount} agents + + )} + {summary.totalTokens > 0 && ( + {formatTokens(summary.totalTokens)} + )} + +
+ + {summary.totalCount > 0 && ( +
+
0 + ? styles.railFillError + : finished + ? styles.railFillDone + : "" + }`} + style={{ width: `${progressPct}%` }} + /> +
+ )} + + {!isCollapsed && ( +
+ {description &&
{description}
} + {summary.totalCount === 0 ? ( +
+ {finished + ? "No agent activity was reported for this run." + : "Starting workflow…"} +
+ ) : ( + grouped.map(({ title, agents }, i) => ( +
+ {title &&
{title}
} + {agents.map((agent) => ( + + ))} +
+ )) + )} + {script && ( +
+ + Script ({scriptLineCount} line{scriptLineCount !== 1 ? "s" : ""}) + + 
{script}
+
+ )} +
+ )} +
+ ); +} diff --git a/src/ui/src/components/chat/chatHelpers.ts b/src/ui/src/components/chat/chatHelpers.ts index 97420873d..21d6c4789 100644 --- a/src/ui/src/components/chat/chatHelpers.ts +++ b/src/ui/src/components/chat/chatHelpers.ts @@ -37,6 +37,8 @@ export const TOOL_COLORS: Record = { WebSearch: "var(--tool-web)", WebFetch: "var(--tool-web)", Agent: "var(--tool-agent)", + // Same family as Agent — a workflow is a fan-out of them. + Workflow: "var(--tool-agent)", AskUserQuestion: "var(--accent-primary)", }; diff --git a/src/ui/src/components/chat/collapsedToolGroupKey.ts b/src/ui/src/components/chat/collapsedToolGroupKey.ts index b9416a245..1416bbaf3 100644 --- a/src/ui/src/components/chat/collapsedToolGroupKey.ts +++ b/src/ui/src/components/chat/collapsedToolGroupKey.ts @@ -32,17 +32,19 @@ export function collapsedToolGroupKey( const first = activities[0]; if (!first) return null; // Match the discriminator `groupToolActivitiesForDisplay` uses for - // its own React `key` (`agent:` for Agent tool, `mcp:` for an MCP tool, - // `tools:` otherwise) so the slice key collides with nothing - // user-meaningful and reads sensibly in dev tools. Both render paths - // (live `GroupedToolActivityRows` and post-turn `TurnSummary`) call this - // with the same first activity, so the key stays stable across the - // running→completed transition. + // its own React `key` (`agent:` for Agent tool, `workflow:` for the + // Workflow tool, `mcp:` for an MCP tool, `tools:` otherwise) so the + // slice key collides with nothing user-meaningful and reads sensibly in + // dev tools. Both render paths (live `GroupedToolActivityRows` and + // post-turn `TurnSummary`) call this with the same first activity, so + // the key stays stable across the running→completed transition. const kind = first.toolName === "Agent" ? "agent" - : isMcpToolName(first.toolName) - ? "mcp" - : "tools"; + : first.toolName === "Workflow" + ? "workflow" + : isMcpToolName(first.toolName) + ? "mcp" + : "tools"; return `${kind}:${first.toolUseId}`; } diff --git a/src/ui/src/components/chat/toolActivityGroups.ts b/src/ui/src/components/chat/toolActivityGroups.ts index 15400a353..5fe4cea3e 100644 --- a/src/ui/src/components/chat/toolActivityGroups.ts +++ b/src/ui/src/components/chat/toolActivityGroups.ts @@ -1,10 +1,11 @@ import type { ToolDisplayMode } from "../../stores/slices/settingsSlice"; import type { ToolActivity } from "../../stores/useAppStore"; import { parseMcpToolName } from "./mcpToolName"; +import { workflowDisplayName } from "./workflowMeta"; export type ToolActivityDisplayGroup = { key: string; - kind: "tools" | "agent" | "skill" | "mcp"; + kind: "tools" | "agent" | "skill" | "mcp" | "workflow"; label: string; activities: ToolActivity[]; }; @@ -48,11 +49,16 @@ export function groupToolActivitiesForDisplay( // `directTools` and `mcpRun` are mutually exclusive accumulators: pushing to // either flushes the other first, so at most one is non-empty at a time. for (const activity of activities) { - // Agents and skills are first-class transcript entries, not tool - // calls — they break a run of direct tools and render on their own - // (agents as a collapsible group, skills as a flat "activated" - // marker) rather than getting bundled into the "N tool calls" pill. - if (isAgentActivity(activity) || isSkillActivity(activity)) { + // Agents, skills, and workflows are first-class transcript entries, + // not tool calls — they break a run of direct tools and render on + // their own (agents and workflows as collapsible groups, skills as a + // flat "activated" marker) rather than getting bundled into the + // "N tool calls" pill. + if ( + isAgentActivity(activity) || + isSkillActivity(activity) || + isWorkflowActivity(activity) + ) { flushDirectTools(); flushMcpRun(); groups.push(activityDisplayGroup(activity)); @@ -88,6 +94,10 @@ export function isSkillActivity(activity: ToolActivity): boolean { return activity.toolName === "Skill"; } +export function isWorkflowActivity(activity: ToolActivity): boolean { + return activity.toolName === "Workflow"; +} + /** Skill name from the `Skill` tool input (`{ "skill": "..." }`), falling * back to the first token of the activity summary, then "Skill". Shared by * the group label and the rendered " activated" marker. */ @@ -145,6 +155,17 @@ function activityDisplayGroup(activity: ToolActivity): ToolActivityDisplayGroup activities: [activity], }; } + if (isWorkflowActivity(activity)) { + return { + key: `workflow:${activity.toolUseId}`, + kind: "workflow", + // Name comes from the tool input (stable from the moment the + // tool_use arrives), not the streamed description — which the CLI + // repoints at whichever agent is currently running. + label: `Workflow ${workflowDisplayName(activity.inputJson)}`, + activities: [activity], + }; + } return { key: `tool:${activity.toolUseId}`, kind: "tools", diff --git a/src/ui/src/components/chat/workflowMeta.test.ts b/src/ui/src/components/chat/workflowMeta.test.ts new file mode 100644 index 000000000..101d02da2 --- /dev/null +++ b/src/ui/src/components/chat/workflowMeta.test.ts @@ -0,0 +1,135 @@ +import { describe, expect, it } from "vitest"; +import { + isWorkflowResume, + workflowDescription, + workflowDisplayName, +} from "./workflowMeta"; + +/** Shape captured from a real Workflow tool_use. */ +const INLINE_SCRIPT = `export const meta = { + name: 'verify-sonnet-5-transition', + description: 'Adversarially verify the Sonnet 5 model-transition diff', + phases: [ + { title: 'Review' }, + { title: 'Synthesize' }, + ], +} + +const DIMENSIONS = [{ name: 'completeness' }] +phase('Review') +await agent('do the thing', { label: 'review:completeness' }) +`; + +function input(obj: Record): string { + return JSON.stringify(obj); +} + +describe("workflowDisplayName", () => { + it("reads meta.name from an inline script", () => { + expect(workflowDisplayName(input({ script: INLINE_SCRIPT }))).toBe( + "verify-sonnet-5-transition", + ); + }); + + it("prefers an explicit name over the script's meta", () => { + expect( + workflowDisplayName(input({ name: "saved-review", script: INLINE_SCRIPT })), + ).toBe("saved-review"); + }); + + it("does not mistake a later `name:` in the script body for the workflow name", () => { + const script = `export const meta = { description: 'no name here' } +const DIMENSIONS = [{ name: 'completeness' }]`; + // No name in meta and no scriptPath — must fall back, not pick up + // the DIMENSIONS entry. + expect(workflowDisplayName(input({ script }))).toBe("Workflow"); + }); + + it("survives a brace inside a meta string value", () => { + const script = `export const meta = { + description: 'emit {n} findings per {dimension}', + name: 'braced', +} +const DIMENSIONS = [{ name: 'wrong' }]`; + expect(workflowDisplayName(input({ script }))).toBe("braced"); + }); + + it("reads through nested phase objects to a name declared last", () => { + const script = `export const meta = { + phases: [{ title: 'Review' }, { title: 'Verify' }], + name: 'nested-last', +} +const DIMENSIONS = [{ name: 'wrong' }]`; + expect(workflowDisplayName(input({ script }))).toBe("nested-last"); + }); + + it("falls back rather than hanging when the meta literal never closes", () => { + expect( + workflowDisplayName(input({ script: "export const meta = { name: 'x'" })), + ).toBe("Workflow"); + }); + + it("falls back to the script path basename, stripping the run-id suffix", () => { + expect( + workflowDisplayName( + input({ + scriptPath: + "/Users/x/.claude/projects/p/s/workflows/scripts/safety-review-wf_e170b89e-638.js", + }), + ), + ).toBe("safety-review"); + }); + + it("handles a Windows-style script path", () => { + expect( + workflowDisplayName(input({ scriptPath: "C:\\claude\\scripts\\audit.js" })), + ).toBe("audit"); + }); + + it("handles double-quoted and backtick meta values", () => { + expect( + workflowDisplayName(input({ script: `export const meta = { name: "dq-flow" }` })), + ).toBe("dq-flow"); + expect( + workflowDisplayName(input({ script: "export const meta = { name: `bt-flow` }" })), + ).toBe("bt-flow"); + }); + + it("returns a usable label for malformed or empty input", () => { + expect(workflowDisplayName("not json")).toBe("Workflow"); + expect(workflowDisplayName("")).toBe("Workflow"); + expect(workflowDisplayName(input({}))).toBe("Workflow"); + expect(workflowDisplayName(input({ name: " " }))).toBe("Workflow"); + // A JSON array parses fine but isn't an input object. + expect(workflowDisplayName("[1,2]")).toBe("Workflow"); + }); +}); + +describe("workflowDescription", () => { + it("reads meta.description from an inline script", () => { + expect(workflowDescription(input({ script: INLINE_SCRIPT }))).toBe( + "Adversarially verify the Sonnet 5 model-transition diff", + ); + }); + + it("is null when the workflow was launched by name with no script", () => { + expect(workflowDescription(input({ name: "saved-review" }))).toBeNull(); + }); + + it("is null when the script has no meta block", () => { + expect(workflowDescription(input({ script: "phase('Review')" }))).toBeNull(); + }); +}); + +describe("isWorkflowResume", () => { + it("detects a resumed run", () => { + expect( + isWorkflowResume(input({ scriptPath: "/tmp/x.js", resumeFromRunId: "wf_abc123" })), + ).toBe(true); + }); + + it("is false for a fresh run", () => { + expect(isWorkflowResume(input({ script: INLINE_SCRIPT }))).toBe(false); + expect(isWorkflowResume("not json")).toBe(false); + }); +}); diff --git a/src/ui/src/components/chat/workflowMeta.ts b/src/ui/src/components/chat/workflowMeta.ts new file mode 100644 index 000000000..a87c6e90d --- /dev/null +++ b/src/ui/src/components/chat/workflowMeta.ts @@ -0,0 +1,152 @@ +/** + * Pull a stable display name and description for a `Workflow` tool call out + * of its input JSON. + * + * Why not use the streamed `description`? On `task_progress` the CLI sets it + * to the *currently running agent* (`"Review: review:completeness"`), so it + * churns every few seconds. That makes a good live subtitle and a terrible + * title. The tool input, by contrast, is available the moment the tool_use + * arrives — before any progress event — and never changes. + * + * Resolution order: an explicit `name` (saved/named workflow) → the `meta` + * block inside an inline `script` → the `scriptPath` basename → `"Workflow"`. + */ + +/** Upper bound on the `meta` literal scan, so a script with an unbalanced + * brace (or one that simply never closes the object) can't walk the whole + * file. Real `meta` blocks are a few hundred chars; scripts run to + * hundreds of KB. */ +const META_SCAN_CHARS = 4000; + +interface WorkflowToolInput { + name?: unknown; + script?: unknown; + scriptPath?: unknown; + resumeFromRunId?: unknown; +} + +function parseInput(inputJson: string): WorkflowToolInput | null { + try { + const parsed: unknown = JSON.parse(inputJson || "{}"); + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return null; + return parsed as WorkflowToolInput; + } catch { + return null; + } +} + +function nonEmptyString(value: unknown): string | null { + return typeof value === "string" && value.trim() ? value.trim() : null; +} + +/** + * Slice out the `meta = { … }` object literal, brace-matched. + * + * Bounding the search to the literal itself is what stops a `name:` further + * down the script (an agent option, a schema property, an array of named + * dimensions) from being read as the workflow's name. A fixed character + * window can't do that — `meta` is often followed immediately by exactly + * such a declaration. + * + * The scan skips over quoted strings so a brace inside a description + * (`'emit {n} findings'`) doesn't close the object early. `meta` is + * contractually a pure literal, so there are no comments, regexes, or + * template interpolations to worry about. + * + * Returns null if the literal is absent or never closes within + * `META_SCAN_CHARS`. + */ +function extractMetaLiteral(script: string): string | null { + const header = /export\s+const\s+meta\s*=\s*\{/.exec(script); + if (!header) return null; + + const open = header.index + header[0].length - 1; + const limit = Math.min(script.length, open + META_SCAN_CHARS); + let depth = 0; + let quote: string | null = null; + + for (let i = open; i < limit; i++) { + const ch = script[i]; + if (quote) { + if (ch === "\\") i++; + else if (ch === quote) quote = null; + continue; + } + if (ch === "'" || ch === '"' || ch === "`") { + quote = ch; + continue; + } + if (ch === "{") depth++; + else if (ch === "}") { + depth--; + if (depth === 0) return script.slice(open, i + 1); + } + } + return null; +} + +/** Read one string property out of the workflow's `meta` literal. + * + * A regex rather than a parser, applied to the brace-matched literal only: + * `meta` is contractually pure, and a failed match degrades to the next + * fallback rather than to a wrong answer. */ +function readMetaString(script: string, key: string): string | null { + const meta = extractMetaLiteral(script); + if (!meta) return null; + // Single, double, or backtick quotes — `meta` literals in the wild use all + // three. The value class excludes all three quote characters, so an + // interpolated backtick value (`${...}`) simply fails to match and falls + // through instead of resolving to something wrong. + const match = new RegExp(`\\b${key}\\s*:\\s*(['"\`])([^'"\`]*)\\1`).exec(meta); + const value = match?.[2]?.trim(); + return value ? value : null; +} + +/** Basename of a script path, minus the `-wf_` suffix and extension + * that `Workflow` appends when it persists an inline script. */ +function scriptPathName(scriptPath: string): string | null { + const base = scriptPath.split(/[/\\]/).pop() ?? ""; + const withoutExt = base.replace(/\.[cm]?js$/, ""); + const withoutRunId = withoutExt.replace(/-wf_[A-Za-z0-9-]+$/, ""); + return withoutRunId.trim() || null; +} + +/** Stable title for a workflow tool call. Never empty. */ +export function workflowDisplayName(inputJson: string): string { + const input = parseInput(inputJson); + if (!input) return "Workflow"; + + const explicitName = nonEmptyString(input.name); + if (explicitName) return explicitName; + + const script = nonEmptyString(input.script); + if (script) { + const metaName = readMetaString(script, "name"); + if (metaName) return metaName; + } + + const scriptPath = nonEmptyString(input.scriptPath); + if (scriptPath) { + const fromPath = scriptPathName(scriptPath); + if (fromPath) return fromPath; + } + + return "Workflow"; +} + +/** One-line description from `meta.description`, or null when the workflow + * was launched by name / path (where we have no script to read). */ +export function workflowDescription(inputJson: string): string | null { + const input = parseInput(inputJson); + const script = input ? nonEmptyString(input.script) : null; + if (!script) return null; + return readMetaString(script, "description"); +} + +/** True when this invocation resumed a prior run rather than starting fresh. + * Worth surfacing because a resumed run's cached agents complete instantly, + * which otherwise looks like an implausibly fast workflow. */ +export function isWorkflowResume(inputJson: string): boolean { + const input = parseInput(inputJson); + return !!input && nonEmptyString(input.resumeFromRunId) !== null; +} diff --git a/src/ui/src/hooks/toolSummary.ts b/src/ui/src/hooks/toolSummary.ts index 1f34454dc..bc7979ac1 100644 --- a/src/ui/src/hooks/toolSummary.ts +++ b/src/ui/src/hooks/toolSummary.ts @@ -1,4 +1,8 @@ import { resolveToolSummary } from "../components/chat/toolMetadata"; +import { + workflowDescription, + workflowDisplayName, +} from "../components/chat/workflowMeta"; /** * Extract a short human-readable summary from a tool's input JSON. @@ -56,6 +60,15 @@ export function extractToolSummary( return input.name ?? input.prompt ?? ""; case "LSP": return input.action ?? ""; + case "Workflow": { + // Without this case the registry's longest-string heuristic picks + // `script` — so a workflow rendered as a truncated dump of minified + // JavaScript, and chat search matched against it. Resolve the same + // stable name the card's header shows. + const name = workflowDisplayName(inputJson); + const description = workflowDescription(inputJson); + return description ? `${name} — ${description}` : name; + } } } catch { return ""; diff --git a/src/ui/src/types/workflow.test.ts b/src/ui/src/types/workflow.test.ts new file mode 100644 index 000000000..1530b5385 --- /dev/null +++ b/src/ui/src/types/workflow.test.ts @@ -0,0 +1,137 @@ +import { describe, expect, it } from "vitest"; +import { + isAgentTerminal, + summarizeWorkflowProgress, + type WorkflowAgentEntry, + type WorkflowProgressEntry, +} from "./workflow"; + +function agent( + index: number, + state: string, + extra: Partial = {}, +): WorkflowAgentEntry { + return { + type: "workflow_agent", + index, + label: `agent-${index}`, + state, + ...extra, + }; +} + +function phase(index: number, title: string): WorkflowProgressEntry { + return { type: "workflow_phase", index, title }; +} + +describe("summarizeWorkflowProgress", () => { + it("returns an empty, not-running summary for no entries", () => { + for (const input of [undefined, [] as WorkflowProgressEntry[]]) { + const summary = summarizeWorkflowProgress(input); + expect(summary.totalCount).toBe(0); + expect(summary.running).toBe(false); + expect(summary.currentPhaseTitle).toBeNull(); + } + }); + + // The CLI re-emits a full entry per state transition, so one snapshot + // routinely carries the same agent three times. Counting them separately + // would report "5/3 agents". + it("collapses repeated entries for the same agent index, last write winning", () => { + const summary = summarizeWorkflowProgress([ + agent(1, "queued"), + agent(1, "progress"), + agent(1, "done", { tokens: 100, toolCalls: 4 }), + ]); + expect(summary.totalCount).toBe(1); + expect(summary.doneCount).toBe(1); + expect(summary.running).toBe(false); + expect(summary.totalTokens).toBe(100); + expect(summary.totalToolCalls).toBe(4); + }); + + it("keeps agents in first-seen order, not index order", () => { + // A phase-2 agent can start before a slow phase-1 agent finishes; a + // later tick for the slow one must not reorder the list under the user. + const summary = summarizeWorkflowProgress([ + agent(2, "progress"), + agent(1, "progress"), + agent(2, "done"), + ]); + expect(summary.agents.map((a) => a.index)).toEqual([2, 1]); + }); + + it("sums tokens and tool calls across agents, tolerating missing values", () => { + const summary = summarizeWorkflowProgress([ + agent(1, "done", { tokens: 100, toolCalls: 3 }), + agent(2, "done", { tokens: 50 }), + agent(3, "progress"), + ]); + expect(summary.totalTokens).toBe(150); + expect(summary.totalToolCalls).toBe(3); + }); + + it("counts errors separately but still as terminal", () => { + const summary = summarizeWorkflowProgress([ + agent(1, "done"), + agent(2, "error", { error: "boom" }), + ]); + expect(summary.doneCount).toBe(2); + expect(summary.errorCount).toBe(1); + expect(summary.running).toBe(false); + }); + + it("is running while any agent is queued or in flight", () => { + expect( + summarizeWorkflowProgress([agent(1, "done"), agent(2, "queued")]).running, + ).toBe(true); + expect( + summarizeWorkflowProgress([agent(1, "done"), agent(2, "progress")]).running, + ).toBe(true); + }); + + it("reports the phase of the first non-terminal agent", () => { + const summary = summarizeWorkflowProgress([ + phase(1, "Review"), + phase(2, "Verify"), + agent(1, "done", { phaseTitle: "Review" }), + agent(2, "progress", { phaseTitle: "Verify" }), + ]); + expect(summary.currentPhaseTitle).toBe("Verify"); + }); + + it("falls back to the last declared phase once everything is terminal", () => { + const summary = summarizeWorkflowProgress([ + phase(1, "Review"), + phase(2, "Synthesize"), + agent(1, "done", { phaseTitle: "Review" }), + ]); + expect(summary.currentPhaseTitle).toBe("Synthesize"); + }); + + it("ignores unknown entry kinds without disturbing the counts", () => { + const summary = summarizeWorkflowProgress([ + { type: "Unknown" }, + agent(1, "done"), + { type: "Unknown" }, + ]); + expect(summary.totalCount).toBe(1); + expect(summary.doneCount).toBe(1); + }); +}); + +describe("isAgentTerminal", () => { + it("treats done and error as terminal", () => { + expect(isAgentTerminal(agent(1, "done"))).toBe(true); + expect(isAgentTerminal(agent(1, "error"))).toBe(true); + }); + + // A state we've never seen must read as "still going", so a run can't + // report itself finished on the strength of a value we don't understand. + it("treats queued, progress, and unrecognized states as in-flight", () => { + expect(isAgentTerminal(agent(1, "queued"))).toBe(false); + expect(isAgentTerminal(agent(1, "progress"))).toBe(false); + expect(isAgentTerminal(agent(1, "some_future_state"))).toBe(false); + expect(isAgentTerminal(agent(1, ""))).toBe(false); + }); +}); From 8052dfd0bdde2731bcf316d39881f85bdc06ffe2 Mon Sep 17 00:00:00 2001 From: Sean Callan Date: Thu, 23 Jul 2026 12:13:38 -0400 Subject: [PATCH 03/14] fix(chat): keep Workflow progress flowing after its launching turn ends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A backgrounded workflow normally outlives the turn that started it: the tool_result ("launched in background") lands within a second, the agent finishes its turn, and the run keeps emitting task_progress for minutes before its completion notification arrives. `updateToolActivity` only ever touched `toolActivities[sessionId]`, so every one of those updates was dropped once `finalizeTurn` moved the activity into `completedTurns` — the card froze at whatever the tree looked like at turn end and never showed the run finishing. It now falls back to searching completed turns (newest first) when the live list has no match. Purely additive: those updates were previously discarded, so no currently-working path changes. Persistence has the same problem — `save_turn_tool_activities` ran minutes before the run ended. Adds `update_turn_tool_activity_progress`, a targeted UPDATE by tool_use_id, called once on the terminal notification. COALESCE on the status column so a progress-only write can't blank a resolved status. --- src-tauri/src/commands/chat/checkpoint.rs | 21 ++ src-tauri/src/main.rs | 1 + src/db/checkpoint.rs | 90 +++++++++ src/ui/src/hooks/useAgentStream.ts | 25 +++ src/ui/src/services/tauri/checkpoints.ts | 15 ++ src/ui/src/stores/slices/chatSlice.ts | 52 ++++- .../useAppStore.workflowProgress.test.ts | 181 ++++++++++++++++++ 7 files changed, 377 insertions(+), 8 deletions(-) create mode 100644 src/ui/src/stores/useAppStore.workflowProgress.test.ts diff --git a/src-tauri/src/commands/chat/checkpoint.rs b/src-tauri/src/commands/chat/checkpoint.rs index d6b3e5485..265b14b22 100644 --- a/src-tauri/src/commands/chat/checkpoint.rs +++ b/src-tauri/src/commands/chat/checkpoint.rs @@ -295,6 +295,27 @@ pub async fn save_turn_tool_activities( Ok(()) } +/// Persist the final workflow progress tree for an activity whose turn has +/// already been checkpointed. See +/// [`claudette::db::Database::update_turn_tool_activity_progress`] for why a +/// targeted update is needed rather than a re-save of the whole turn. +#[tauri::command] +pub async fn update_turn_tool_activity_progress( + tool_use_id: String, + workflow_progress_json: String, + agent_status: Option, + state: State<'_, AppState>, +) -> Result<(), String> { + let db = Database::open(&state.db_path).map_err(|e| e.to_string())?; + db.update_turn_tool_activity_progress( + &tool_use_id, + &workflow_progress_json, + agent_status.as_deref(), + ) + .map_err(|e| e.to_string())?; + Ok(()) +} + #[tauri::command] pub async fn load_completed_turns( session_id: String, diff --git a/src-tauri/src/main.rs b/src-tauri/src/main.rs index a7ea47dfa..7858d92af 100644 --- a/src-tauri/src/main.rs +++ b/src-tauri/src/main.rs @@ -1123,6 +1123,7 @@ fn main() { commands::chat::checkpoint::rollback_to_checkpoint, commands::chat::checkpoint::clear_conversation, commands::chat::checkpoint::save_turn_tool_activities, + commands::chat::checkpoint::update_turn_tool_activity_progress, commands::chat::checkpoint::load_completed_turns, commands::chat::session::list_chat_sessions, commands::chat::session::get_chat_session, diff --git a/src/db/checkpoint.rs b/src/db/checkpoint.rs index 3cfacc48a..0d79284d2 100644 --- a/src/db/checkpoint.rs +++ b/src/db/checkpoint.rs @@ -889,6 +889,37 @@ impl Database { Ok(()) } + /// Update the persisted workflow progress tree (and agent status) for a + /// single already-saved tool activity, addressed by `tool_use_id`. + /// + /// Exists because a `Workflow` run routinely outlives the turn that + /// launched it: the tool returns "launched in background" within a + /// second, the agent finishes its turn, and the run keeps going for + /// minutes before its task-notification arrives. By then the activity + /// has already been written by `save_turn_tool_activities`, so the + /// final tree has nowhere to land without a targeted update. + /// + /// Returns the number of rows updated — zero is normal and not an + /// error (the activity may belong to a session whose turn was never + /// checkpointed). + pub fn update_turn_tool_activity_progress( + &self, + tool_use_id: &str, + workflow_progress_json: &str, + agent_status: Option<&str>, + ) -> Result { + // COALESCE keeps the stored status when the caller has nothing + // better to say, so a progress-only update can't blank a status + // that a prior notification already resolved. + self.conn.execute( + "UPDATE turn_tool_activities + SET workflow_progress_json = ?1, + agent_status = COALESCE(?2, agent_status) + WHERE tool_use_id = ?3", + params![workflow_progress_json, agent_status, tool_use_id], + ) + } + /// Load all completed turns for a workspace: checkpoints joined with their /// tool activities, grouped by checkpoint and ordered by turn_index. pub fn list_completed_turns( @@ -1399,6 +1430,65 @@ mod tests { assert_eq!(turns[1].activities[0].tool_name, "Bash"); } + /// A backgrounded `Workflow` finishes long after its turn was saved, so + /// the final tree has to be written straight at the activity row. + #[test] + fn test_update_turn_tool_activity_progress_writes_tree_and_status() { + let db = setup_db_with_workspace(); + db.insert_chat_message(&make_chat_msg(&db, "m1", "w1", ChatRole::Assistant, "a1")) + .unwrap(); + db.insert_checkpoint(&make_checkpoint(&db, "cp1", "w1", "m1", 0)) + .unwrap(); + db.insert_turn_tool_activities(&[make_tool_activity("a1", "cp1", "Workflow", 0)]) + .unwrap(); + + let tree = r#"[{"type":"workflow_agent","index":1,"label":"review","state":"done"}]"#; + let updated = db + .update_turn_tool_activity_progress("tu_a1", tree, Some("completed")) + .unwrap(); + assert_eq!(updated, 1); + + let turns = db.list_completed_turns("w1").unwrap(); + let activity = &turns[0].activities[0]; + assert_eq!(activity.workflow_progress_json, tree); + assert_eq!(activity.agent_status.as_deref(), Some("completed")); + } + + /// `COALESCE` on the status column: a progress-only update must not + /// blank a status a prior notification already resolved. + #[test] + fn test_update_turn_tool_activity_progress_preserves_status_when_none() { + let db = setup_db_with_workspace(); + db.insert_chat_message(&make_chat_msg(&db, "m1", "w1", ChatRole::Assistant, "a1")) + .unwrap(); + db.insert_checkpoint(&make_checkpoint(&db, "cp1", "w1", "m1", 0)) + .unwrap(); + db.insert_turn_tool_activities(&[make_tool_activity("a1", "cp1", "Workflow", 0)]) + .unwrap(); + + db.update_turn_tool_activity_progress("tu_a1", "[]", Some("completed")) + .unwrap(); + db.update_turn_tool_activity_progress("tu_a1", r#"[{"type":"Unknown"}]"#, None) + .unwrap(); + + let turns = db.list_completed_turns("w1").unwrap(); + assert_eq!( + turns[0].activities[0].agent_status.as_deref(), + Some("completed") + ); + } + + /// An unknown `tool_use_id` is normal (the turn may never have been + /// checkpointed), so it reports zero rows rather than erroring. + #[test] + fn test_update_turn_tool_activity_progress_unknown_id_is_not_an_error() { + let db = setup_db_with_workspace(); + let updated = db + .update_turn_tool_activity_progress("tu_missing", "[]", Some("completed")) + .unwrap(); + assert_eq!(updated, 0); + } + #[test] fn test_list_messages_up_to_includes_boundary() { let db = setup_db_with_workspace(); diff --git a/src/ui/src/hooks/useAgentStream.ts b/src/ui/src/hooks/useAgentStream.ts index f3794a339..acfda4c7c 100644 --- a/src/ui/src/hooks/useAgentStream.ts +++ b/src/ui/src/hooks/useAgentStream.ts @@ -4,6 +4,7 @@ import { useAppStore } from "../stores/useAppStore"; import { loadChatHistory, saveTurnToolActivities, + updateTurnToolActivityProgress, setSessionCliInvocation, } from "../services/tauri"; import type { AgentStreamPayload } from "../types/agent-events"; @@ -334,6 +335,30 @@ export function useAgentStream() { updates.agentStatus = "running"; } updateToolActivity(sessionId, streamEvent.tool_use_id, updates); + // A backgrounded Workflow usually finishes after the turn + // that launched it has already been checkpointed, so its + // final tree has nowhere to land — `save_turn_tool_activities` + // ran minutes ago. Write it directly at the activity row. + // + // Only on the terminal notification, not on every progress + // tick: one write per run instead of dozens, and the + // completed state is what replay actually needs. A reload + // mid-run still shows the tree as of the turn's end. + if ( + streamEvent.subtype === "task_notification" && + updates.workflowProgress !== undefined + ) { + void updateTurnToolActivityProgress( + streamEvent.tool_use_id, + JSON.stringify(updates.workflowProgress), + updates.agentStatus ?? null, + ).catch((err) => { + debugChat("stream", "persist workflow progress failed", { + toolUseId: streamEvent.tool_use_id, + error: String(err), + }); + }); + } if (streamEvent.task_id) { const pending = pendingAgentToolCallsRef.current[streamEvent.task_id] || []; const remaining = pending.filter( diff --git a/src/ui/src/services/tauri/checkpoints.ts b/src/ui/src/services/tauri/checkpoints.ts index 2b1861df3..19f857f5b 100644 --- a/src/ui/src/services/tauri/checkpoints.ts +++ b/src/ui/src/services/tauri/checkpoints.ts @@ -46,6 +46,21 @@ export function saveTurnToolActivities( }); } +/** Persist the workflow progress tree for a single already-checkpointed + * activity. Used when a backgrounded `Workflow` finishes after the turn + * that launched it was already saved. */ +export function updateTurnToolActivityProgress( + toolUseId: string, + workflowProgressJson: string, + agentStatus: string | null, +): Promise { + return invoke("update_turn_tool_activity_progress", { + toolUseId, + workflowProgressJson, + agentStatus, + }); +} + export function loadCompletedTurns( sessionId: string, ): Promise { diff --git a/src/ui/src/stores/slices/chatSlice.ts b/src/ui/src/stores/slices/chatSlice.ts index 201e83a42..2d37810e1 100644 --- a/src/ui/src/stores/slices/chatSlice.ts +++ b/src/ui/src/stores/slices/chatSlice.ts @@ -474,14 +474,50 @@ export const createChatSlice: StateCreator = ( }, })), updateToolActivity: (sessionId, toolUseId, updates) => - set((s) => ({ - toolActivities: { - ...s.toolActivities, - [sessionId]: (s.toolActivities[sessionId] || []).map((a) => - a.toolUseId === toolUseId ? { ...a, ...updates } : a, - ), - }, - })), + set((s) => { + const live = s.toolActivities[sessionId] || []; + if (live.some((a) => a.toolUseId === toolUseId)) { + return { + toolActivities: { + ...s.toolActivities, + [sessionId]: live.map((a) => + a.toolUseId === toolUseId ? { ...a, ...updates } : a, + ), + }, + }; + } + + // Not in the live turn. Normally that means the update is for an + // activity we've already forgotten and there's nothing to do — but a + // backgrounded `Workflow` breaks that assumption: its tool_result + // ("launched in background") arrives within a second, the agent + // finishes its turn, and the run keeps emitting `task_progress` for + // minutes afterwards. Those updates arrive after `finalizeTurn` has + // moved the activity into `completedTurns`, so without this fallback + // the card would freeze at whatever the tree looked like when the + // turn ended and never show the run completing. + // + // Searched newest-first because `toolUseId` is unique per turn and + // the relevant activity is almost always in the most recent one. + // Purely additive: today these updates are silently dropped, so no + // currently-working path changes behavior. + const turns = s.completedTurns[sessionId]; + if (!turns) return {}; + for (let i = turns.length - 1; i >= 0; i--) { + const idx = turns[i].activities.findIndex( + (a) => a.toolUseId === toolUseId, + ); + if (idx < 0) continue; + const nextTurns = [...turns]; + const nextActivities = [...turns[i].activities]; + nextActivities[idx] = { ...nextActivities[idx], ...updates }; + nextTurns[i] = { ...turns[i], activities: nextActivities }; + return { + completedTurns: { ...s.completedTurns, [sessionId]: nextTurns }, + }; + } + return {}; + }), upsertAgentToolCall: (sessionId, agentId, call) => { let matched = false; set((s) => { diff --git a/src/ui/src/stores/useAppStore.workflowProgress.test.ts b/src/ui/src/stores/useAppStore.workflowProgress.test.ts new file mode 100644 index 000000000..f5be68a9a --- /dev/null +++ b/src/ui/src/stores/useAppStore.workflowProgress.test.ts @@ -0,0 +1,181 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { useAppStore } from "./useAppStore"; +import type { CompletedTurn, ToolActivity } from "./useAppStore"; +import type { WorkflowProgressEntry } from "../types/workflow"; + +const SESSION = "session-1"; + +function workflowActivity( + toolUseId = "toolu_wf1", + overrides: Partial = {}, +): ToolActivity { + return { + toolUseId, + toolName: "Workflow", + inputJson: JSON.stringify({ script: "export const meta = { name: 'x' }" }), + resultText: "Workflow launched in background. Task ID: w4stpeffj", + collapsed: false, + summary: "x", + agentStatus: "running", + ...overrides, + }; +} + +function completedTurn( + id: string, + activities: ToolActivity[], +): CompletedTurn { + return { + id, + activities, + messageCount: 1, + collapsed: true, + afterMessageIndex: 1, + }; +} + +const TREE: WorkflowProgressEntry[] = [ + { type: "workflow_phase", index: 1, title: "Review" }, + { + type: "workflow_agent", + index: 1, + label: "review:bugs", + state: "done", + }, +]; + +beforeEach(() => { + useAppStore.setState({ toolActivities: {}, completedTurns: {} }); +}); + +describe("updateToolActivity — workflow progress after the turn ends", () => { + it("updates the live activity when one matches", () => { + useAppStore.setState({ + toolActivities: { [SESSION]: [workflowActivity()] }, + }); + + useAppStore + .getState() + .updateToolActivity(SESSION, "toolu_wf1", { + workflowProgress: TREE, + agentStatus: "completed", + }); + + const activity = useAppStore.getState().toolActivities[SESSION][0]; + expect(activity.workflowProgress).toEqual(TREE); + expect(activity.agentStatus).toBe("completed"); + }); + + // The case this fallback exists for: a Workflow's tool_result lands at + // launch, the agent finishes its turn, and the run keeps emitting + // progress for minutes. Those updates arrive after `finalizeTurn` has + // moved the activity into `completedTurns`. + it("falls back to a completed turn when the activity is no longer live", () => { + useAppStore.setState({ + toolActivities: { [SESSION]: [] }, + completedTurns: { + [SESSION]: [completedTurn("turn-1", [workflowActivity()])], + }, + }); + + useAppStore + .getState() + .updateToolActivity(SESSION, "toolu_wf1", { + workflowProgress: TREE, + agentStatus: "completed", + }); + + const activity = + useAppStore.getState().completedTurns[SESSION][0].activities[0]; + expect(activity.workflowProgress).toEqual(TREE); + expect(activity.agentStatus).toBe("completed"); + }); + + it("finds the activity in an older turn, not just the newest", () => { + useAppStore.setState({ + completedTurns: { + [SESSION]: [ + completedTurn("turn-1", [workflowActivity()]), + completedTurn("turn-2", [ + workflowActivity("toolu_other", { toolName: "Bash" }), + ]), + ], + }, + }); + + useAppStore + .getState() + .updateToolActivity(SESSION, "toolu_wf1", { agentStatus: "completed" }); + + const turns = useAppStore.getState().completedTurns[SESSION]; + expect(turns[0].activities[0].agentStatus).toBe("completed"); + expect(turns[1].activities[0].agentStatus).toBe("running"); + }); + + it("prefers the live activity over a same-id completed one", () => { + useAppStore.setState({ + toolActivities: { [SESSION]: [workflowActivity()] }, + completedTurns: { + [SESSION]: [completedTurn("turn-1", [workflowActivity()])], + }, + }); + + useAppStore + .getState() + .updateToolActivity(SESSION, "toolu_wf1", { agentStatus: "completed" }); + + expect(useAppStore.getState().toolActivities[SESSION][0].agentStatus).toBe( + "completed", + ); + expect( + useAppStore.getState().completedTurns[SESSION][0].activities[0] + .agentStatus, + ).toBe("running"); + }); + + it("is a no-op when the id matches nothing anywhere", () => { + const before = { + toolActivities: { [SESSION]: [workflowActivity()] }, + completedTurns: { + [SESSION]: [completedTurn("turn-1", [workflowActivity("toolu_a")])], + }, + }; + useAppStore.setState(before); + + useAppStore + .getState() + .updateToolActivity(SESSION, "toolu_missing", { agentStatus: "completed" }); + + expect(useAppStore.getState().toolActivities[SESSION][0].agentStatus).toBe( + "running", + ); + expect( + useAppStore.getState().completedTurns[SESSION][0].activities[0] + .agentStatus, + ).toBe("running"); + }); + + it("does not disturb sibling activities in the same completed turn", () => { + useAppStore.setState({ + completedTurns: { + [SESSION]: [ + completedTurn("turn-1", [ + workflowActivity("toolu_a", { toolName: "Read" }), + workflowActivity(), + workflowActivity("toolu_b", { toolName: "Edit" }), + ]), + ], + }, + }); + + useAppStore + .getState() + .updateToolActivity(SESSION, "toolu_wf1", { workflowProgress: TREE }); + + const activities = + useAppStore.getState().completedTurns[SESSION][0].activities; + expect(activities[0].workflowProgress).toBeUndefined(); + expect(activities[1].workflowProgress).toEqual(TREE); + expect(activities[2].workflowProgress).toBeUndefined(); + }); +}); From f1753a82f21258017aebd63078a9a21a677ef37c Mon Sep 17 00:00:00 2001 From: Sean Callan Date: Thu, 23 Jul 2026 12:16:19 -0400 Subject: [PATCH 04/14] feat(chat): add a live status pill for backgrounded Workflow runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The card is the real surface, but a workflow's launching turn ends within seconds of starting it — so for nearly the whole multi-minute run the card is buried under whatever the agent did next. The pill pins name, current phase, and agent counts above the composer, and clicking it scrolls the card back into view. Sourced from completed turns as well as the live turn, for the same reason: `toolActivities` holds the activity only until the turn is finalized, which is a small fraction of a run's lifetime. Card lookup goes through a shared `data-workflow-tool-use-id` attribute rather than a ref — the two components sit in unrelated subtrees, and threading a ref would route it through MessagesWithTurns and TurnSummary, both god files. The spinner honors prefers-reduced-motion and only spins while agents are actually in flight. --- .../components/chat/ChatPanelSessionView.tsx | 6 + src/ui/src/components/chat/WorkflowCard.tsx | 4 +- .../chat/WorkflowStatusPill.module.css | 85 +++++++ .../chat/WorkflowStatusPill.test.tsx | 208 ++++++++++++++++++ .../components/chat/WorkflowStatusPill.tsx | 81 +++++++ src/ui/src/components/chat/workflowAnchor.ts | 12 + src/ui/src/hooks/useLiveWorkflows.ts | 79 +++++++ 7 files changed, 474 insertions(+), 1 deletion(-) create mode 100644 src/ui/src/components/chat/WorkflowStatusPill.module.css create mode 100644 src/ui/src/components/chat/WorkflowStatusPill.test.tsx create mode 100644 src/ui/src/components/chat/WorkflowStatusPill.tsx create mode 100644 src/ui/src/components/chat/workflowAnchor.ts create mode 100644 src/ui/src/hooks/useLiveWorkflows.ts diff --git a/src/ui/src/components/chat/ChatPanelSessionView.tsx b/src/ui/src/components/chat/ChatPanelSessionView.tsx index c05612b80..5bad9dbd5 100644 --- a/src/ui/src/components/chat/ChatPanelSessionView.tsx +++ b/src/ui/src/components/chat/ChatPanelSessionView.tsx @@ -32,6 +32,7 @@ import { setPlanModeAndPersist } from "./planModePersistence"; import { QueuedMessagesPopover } from "./QueuedMessagesPopover"; import { ScrollContext } from "./ScrollContext"; import { ScrollToBottomPill } from "./ScrollToBottomPill"; +import { WorkflowStatusPill } from "./WorkflowStatusPill"; import { SetupScriptBanner } from "./SetupScriptBanner"; import { StreamingMessage } from "./StreamingMessage"; import { StreamingThinkingBlock } from "./StreamingThinkingBlock"; @@ -424,6 +425,11 @@ export function ChatPanelSessionView({
+ {/* Sits above the scroll pill: a backgrounded workflow's card leaves + the viewport within seconds (its launching turn ends almost + immediately), so this is what keeps a multi-minute run visible. */} + + 0} onClick={scrollToBottom} diff --git a/src/ui/src/components/chat/WorkflowCard.tsx b/src/ui/src/components/chat/WorkflowCard.tsx index 65923e179..33607c143 100644 --- a/src/ui/src/components/chat/WorkflowCard.tsx +++ b/src/ui/src/components/chat/WorkflowCard.tsx @@ -12,6 +12,7 @@ import { workflowDescription, workflowDisplayName, } from "./workflowMeta"; +import { WORKFLOW_CARD_ANCHOR_ATTR } from "./workflowAnchor"; import styles from "./WorkflowCard.module.css"; /** Grouping key for an agent with no phase — workflows can call `agent()` @@ -298,7 +299,8 @@ export function WorkflowCard({ : 0; return ( -
+ // Anchor for the status pill's scroll-into-view — see `workflowAnchor`. +
{collapsible && (
)} diff --git a/src/ui/src/components/chat/workflowMeta.ts b/src/ui/src/components/chat/workflowMeta.ts index a87c6e90d..bc91cbaf4 100644 --- a/src/ui/src/components/chat/workflowMeta.ts +++ b/src/ui/src/components/chat/workflowMeta.ts @@ -14,8 +14,9 @@ /** Upper bound on the `meta` literal scan, so a script with an unbalanced * brace (or one that simply never closes the object) can't walk the whole - * file. Real `meta` blocks are a few hundred chars; scripts run to - * hundreds of KB. */ + * file. Real `meta` blocks are a few hundred chars; measured against a + * corpus of real runs, whole scripts are a median of ~10 KB and up to + * ~30 KB. */ const META_SCAN_CHARS = 4000; interface WorkflowToolInput { From f818663fbcca3b47b0ddae4cbb20dd1d55f3de6f Mon Sep 17 00:00:00 2001 From: Sean Callan Date: Fri, 24 Jul 2026 17:02:10 -0400 Subject: [PATCH 10/14] fix(chat): make the persisted workflow-entry type guard sound MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `parseWorkflowProgress` filtered with a predicate asserting `WorkflowProgressEntry` while only checking that `type` was a string, so `{type: "bogus"}` was admitted and typed as a fully-validated union member. Harmless today — every consumer re-discriminates on `type` and silently skips what it doesn't recognize — but it is a trap for the first consumer that trusts the type instead: an exhaustive switch with a `never` default, or reading `.label` off a presumed agent. Replaced with `isWorkflowProgressEntry`, colocated with the union it validates and admitting only the kinds Rust can persist (including `Unknown`, which Rust writes for any incoming kind it doesn't recognize). Anything else is foreign or corrupt column data and is now dropped at the boundary rather than carried through the type system. Tests cover the guard directly and the persistence boundary: modeled kinds rehydrate, unmodeled ones are dropped, and a column with nothing valid returns undefined rather than an empty array. --- src/ui/src/types/workflow.test.ts | 25 ++++++++ src/ui/src/types/workflow.ts | 27 ++++++++ src/ui/src/utils/reconstructTurns.test.ts | 78 +++++++++++++++++++++++ src/ui/src/utils/reconstructTurns.ts | 20 +++--- 4 files changed, 140 insertions(+), 10 deletions(-) diff --git a/src/ui/src/types/workflow.test.ts b/src/ui/src/types/workflow.test.ts index 15abcf023..e3a522f49 100644 --- a/src/ui/src/types/workflow.test.ts +++ b/src/ui/src/types/workflow.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { isAgentTerminal, + isWorkflowProgressEntry, phaseTitleOf, summarizeWorkflowProgress, type WorkflowAgentEntry, @@ -159,6 +160,30 @@ describe("summarizeWorkflowProgress", () => { }); }); +describe("isWorkflowProgressEntry", () => { + it("accepts every kind the union models", () => { + expect(isWorkflowProgressEntry(phase(1, "Review"))).toBe(true); + expect(isWorkflowProgressEntry(agent(1, "done"))).toBe(true); + // Rust writes this for any incoming kind it doesn't recognize, so it's + // a legitimate stored value, not a parse failure. + expect(isWorkflowProgressEntry({ type: "Unknown" })).toBe(true); + }); + + // The predicate asserts the union type, so admitting an unmodeled `type` + // would be unsound — safe only while every consumer re-discriminates. + it("rejects an object whose type is a string but not a known kind", () => { + expect(isWorkflowProgressEntry({ type: "workflow_log" })).toBe(false); + expect(isWorkflowProgressEntry({ type: "bogus", label: "x" })).toBe(false); + expect(isWorkflowProgressEntry({ type: "" })).toBe(false); + }); + + it("rejects non-objects and objects without a string type", () => { + for (const value of [null, undefined, 42, "workflow_agent", [], {}, { type: 1 }]) { + expect(isWorkflowProgressEntry(value)).toBe(false); + } + }); +}); + describe("phaseTitleOf", () => { it("returns the trimmed title when present", () => { expect(phaseTitleOf(agent(1, "done", { phaseTitle: " Review " }))).toBe( diff --git a/src/ui/src/types/workflow.ts b/src/ui/src/types/workflow.ts index 588807a5e..6a4a7cb9f 100644 --- a/src/ui/src/types/workflow.ts +++ b/src/ui/src/types/workflow.ts @@ -67,6 +67,33 @@ export type WorkflowProgressEntry = | WorkflowAgentEntry | WorkflowUnknownEntry; +/** The complete set of entry kinds Rust can persist. `Unknown` is included + * deliberately: the Rust side maps any unrecognized incoming kind to it, so + * it is a legitimate stored value rather than a parse failure. */ +const KNOWN_ENTRY_TYPES = new Set([ + "workflow_phase", + "workflow_agent", + "Unknown", +]); + +/** + * Boundary check for a single entry decoded from persisted JSON. + * + * Narrow enough to justify the type predicate: it accepts only the kinds + * the union actually models. A looser "has a string `type`" check would + * assert `WorkflowProgressEntry` for `{type: "bogus"}`, which is unsound — + * harmless while every consumer re-discriminates on `type`, but a trap for + * the first one that trusts the type instead (an exhaustive switch with a + * `never` default, say, or reading `.label` off a presumed agent). + */ +export function isWorkflowProgressEntry( + value: unknown, +): value is WorkflowProgressEntry { + if (typeof value !== "object" || value === null) return false; + const type = (value as { type?: unknown }).type; + return typeof type === "string" && KNOWN_ENTRY_TYPES.has(type); +} + export function isWorkflowPhase( entry: WorkflowProgressEntry, ): entry is WorkflowPhaseEntry { diff --git a/src/ui/src/utils/reconstructTurns.test.ts b/src/ui/src/utils/reconstructTurns.test.ts index 4fedce329..4bf21d7af 100644 --- a/src/ui/src/utils/reconstructTurns.test.ts +++ b/src/ui/src/utils/reconstructTurns.test.ts @@ -361,4 +361,82 @@ describe("reconstructCompletedTurns", () => { expect(act.summary).toBe("list files"); expect(act.collapsed).toBe(true); }); + + describe("workflow progress rehydration", () => { + function turnWithProgress(json: string): CompletedTurnData[] { + return [ + { + checkpoint_id: "cp1", + message_id: "m1", + turn_index: 0, + message_count: 1, + commit_hash: null, + activities: [ + { + id: "act-1", + checkpoint_id: "cp1", + tool_use_id: "tu-wf", + tool_name: "Workflow", + input_json: "{}", + result_text: "launched", + summary: "wf", + sort_order: 0, + assistant_message_ordinal: 0, + agent_task_id: null, + agent_description: null, + agent_last_tool_name: null, + agent_tool_use_count: null, + agent_status: null, + agent_tool_calls_json: "[]", + agent_thinking_blocks_json: "[]", + agent_result_text: null, + workflow_progress_json: json, + }, + ], + }, + ]; + } + + const messages = [makeMsg("m1")]; + + it("rehydrates the modeled entry kinds", () => { + const json = JSON.stringify([ + { type: "workflow_phase", index: 1, title: "Review" }, + { type: "workflow_agent", index: 1, label: "a", state: "done" }, + { type: "Unknown" }, + ]); + const result = reconstructCompletedTurns(messages, turnWithProgress(json)); + expect(result[0].activities[0].workflowProgress).toHaveLength(3); + }); + + // The filter's type predicate asserts the union, so an unmodeled `type` + // must be dropped at the boundary rather than asserted into the type + // and skipped later by each consumer in turn. + it("drops entries whose type is not a modeled kind", () => { + const json = JSON.stringify([ + { type: "workflow_phase", index: 1, title: "Review" }, + { type: "workflow_log", message: "3/10 found" }, + { type: "bogus" }, + "not an object", + null, + ]); + const result = reconstructCompletedTurns(messages, turnWithProgress(json)); + const progress = result[0].activities[0].workflowProgress; + expect(progress).toHaveLength(1); + expect(progress?.[0]).toEqual({ + type: "workflow_phase", + index: 1, + title: "Review", + }); + }); + + // `undefined`, not `[]` — a non-workflow activity stores "[]", and the + // card keys off `workflowProgress` being present at all. + it("returns undefined when nothing survives or the column is empty", () => { + for (const json of ["[]", "[{\"type\":\"bogus\"}]", "not json", ""]) { + const result = reconstructCompletedTurns(messages, turnWithProgress(json)); + expect(result[0].activities[0].workflowProgress).toBeUndefined(); + } + }); + }); }); diff --git a/src/ui/src/utils/reconstructTurns.ts b/src/ui/src/utils/reconstructTurns.ts index 6f54cd6a6..9f05dc80a 100644 --- a/src/ui/src/utils/reconstructTurns.ts +++ b/src/ui/src/utils/reconstructTurns.ts @@ -1,7 +1,10 @@ import type { CompletedTurn } from "../stores/useAppStore"; import type { ChatMessage } from "../types/chat"; import type { CompletedTurnData } from "../types/checkpoint"; -import type { WorkflowProgressEntry } from "../types/workflow"; +import { + isWorkflowProgressEntry, + type WorkflowProgressEntry, +} from "../types/workflow"; import { debugChat } from "./chatDebug"; /** @@ -127,11 +130,11 @@ function parseAgentToolCalls(value: string | null | undefined) { * like a workflow that produced no agents — `WorkflowCard` keys off * `workflowProgress` being present at all. * - * Entries are not validated beyond "is an object with a `type`". The Rust - * side already normalized unrecognized kinds to `{"type":"Unknown"}` on - * the way in, and the renderer switches on `type` with a default of - * "skip", so a malformed row degrades to an omitted line rather than a - * thrown render. */ + * Entries are filtered through `isWorkflowProgressEntry`, which admits only + * the kinds the union models — including `Unknown`, which Rust writes for + * any incoming kind it doesn't recognize. Anything else in the column is + * foreign or corrupt data and is dropped here rather than being asserted + * into the type and skipped later by each consumer in turn. */ function parseWorkflowProgress( value: string | null | undefined, ): WorkflowProgressEntry[] | undefined { @@ -139,10 +142,7 @@ function parseWorkflowProgress( try { const parsed = JSON.parse(value); if (!Array.isArray(parsed) || parsed.length === 0) return undefined; - const entries = parsed.filter( - (item): item is WorkflowProgressEntry => - typeof item === "object" && item !== null && typeof item.type === "string", - ); + const entries = parsed.filter(isWorkflowProgressEntry); return entries.length > 0 ? entries : undefined; } catch { return undefined; From 30c80577a78396bd4a60f498bd49641cb05a4fff Mon Sep 17 00:00:00 2001 From: Sean Callan Date: Fri, 24 Jul 2026 17:12:01 -0400 Subject: [PATCH 11/14] fix(chat): validate required fields on persisted workflow entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to f818663f. The guard checked only that `type` was a known kind, so a corrupted entry of a known kind still passed and was asserted into the union — and `phaseTitleOf` calls `.trim()`, which throws on a non-string. An unhandled throw there takes out the whole transcript render, not one row. The guard now validates the fields each kind declares as required, making the predicate honest about everything the interface marks non-optional. Optional fields are deliberately still unchecked — per-field validation of ~25 attributes to guard data we serialized from Rust ourselves is not worth it. That trade has a consequence, so it is handled rather than assumed: `readOptionalString` / `readOptionalNumber` back every accessor that calls a method on, or accumulates, an optional field. Note the two layers are both needed — {type, index, label, state, phaseTitle: 1} passes required- field validation and would still have thrown. readOptionalNumber also rejects NaN and infinities, which would otherwise poison a running total as surely as a string does. Tests: required-field rejection per kind, the helpers directly, and the user-visible outcome — a card whose agent has four corrupt optional fields renders instead of throwing. Verified both accessor tests fail without the guards. --- .../src/components/chat/WorkflowCard.test.tsx | 27 ++++++ src/ui/src/components/chat/WorkflowCard.tsx | 8 +- src/ui/src/types/workflow.test.ts | 93 +++++++++++++++++++ src/ui/src/types/workflow.ts | 74 +++++++++++---- 4 files changed, 180 insertions(+), 22 deletions(-) diff --git a/src/ui/src/components/chat/WorkflowCard.test.tsx b/src/ui/src/components/chat/WorkflowCard.test.tsx index 7469bd6e7..f60600373 100644 --- a/src/ui/src/components/chat/WorkflowCard.test.tsx +++ b/src/ui/src/components/chat/WorkflowCard.test.tsx @@ -236,6 +236,33 @@ describe("WorkflowCard", () => { ).toBeNull(); }); + // The persistence guard checks required fields only, so a corrupt + // optional one reaches the renderer. `.trim()` on a non-string throws, + // and an unhandled throw here takes out the whole transcript, not just + // this row. + it("renders rather than throwing when optional fields are corrupt", async () => { + const container = await render( + , + ); + expect(container.textContent).toContain("review:bugs"); + expect(container.textContent).toContain("0/1 agents"); + }); + it("shows a starting state before the first progress tick", async () => { const container = await render( , diff --git a/src/ui/src/components/chat/WorkflowCard.tsx b/src/ui/src/components/chat/WorkflowCard.tsx index b6fc2f8b7..b2748b385 100644 --- a/src/ui/src/components/chat/WorkflowCard.tsx +++ b/src/ui/src/components/chat/WorkflowCard.tsx @@ -3,6 +3,7 @@ import type { ToolActivity } from "../../stores/useAppStore"; import { isAgentTerminal, phaseTitleOf, + readOptionalString, summarizeWorkflowProgress, type WorkflowAgentEntry, } from "../../types/workflow"; @@ -136,9 +137,12 @@ function AgentRow({ live: boolean; }) { const elapsedMs = agentElapsedMs(agent, now, live); + // Both fields are optional, so the persistence boundary guard didn't + // type-check them — read through `readOptionalString` before `.trim()`. + const lastToolName = readOptionalString(agent.lastToolName); const detail = - agent.lastToolSummary?.trim() || - (agent.lastToolName ? `${agent.lastToolName}…` : ""); + readOptionalString(agent.lastToolSummary)?.trim() || + (lastToolName ? `${lastToolName}…` : ""); return (
diff --git a/src/ui/src/types/workflow.test.ts b/src/ui/src/types/workflow.test.ts index e3a522f49..05df6f83a 100644 --- a/src/ui/src/types/workflow.test.ts +++ b/src/ui/src/types/workflow.test.ts @@ -3,6 +3,8 @@ import { isAgentTerminal, isWorkflowProgressEntry, phaseTitleOf, + readOptionalNumber, + readOptionalString, summarizeWorkflowProgress, type WorkflowAgentEntry, type WorkflowProgressEntry, @@ -149,6 +151,24 @@ describe("summarizeWorkflowProgress", () => { ).toBe("Verify"); }); + // A stray non-number would make `+=` concatenate or produce NaN — a + // silently wrong total rather than a loud failure. + it("ignores corrupt token and tool-call values instead of corrupting totals", () => { + const summary = summarizeWorkflowProgress([ + agent(1, "done", { tokens: 100, toolCalls: 3 }), + { + type: "workflow_agent", + index: 2, + label: "b", + state: "done", + tokens: "abc", + toolCalls: NaN, + } as unknown as WorkflowAgentEntry, + ]); + expect(summary.totalTokens).toBe(100); + expect(summary.totalToolCalls).toBe(3); + }); + it("ignores unknown entry kinds without disturbing the counts", () => { const summary = summarizeWorkflowProgress([ { type: "Unknown" }, @@ -182,6 +202,64 @@ describe("isWorkflowProgressEntry", () => { expect(isWorkflowProgressEntry(value)).toBe(false); } }); + + it("rejects a known kind that is missing or mistypes a required field", () => { + // phase requires index:number + title:string + expect(isWorkflowProgressEntry({ type: "workflow_phase" })).toBe(false); + expect( + isWorkflowProgressEntry({ type: "workflow_phase", index: "1", title: "R" }), + ).toBe(false); + expect( + isWorkflowProgressEntry({ type: "workflow_phase", index: 1, title: 2 }), + ).toBe(false); + + // agent requires index:number + label:string + state:string + expect(isWorkflowProgressEntry({ type: "workflow_agent" })).toBe(false); + expect( + isWorkflowProgressEntry({ type: "workflow_agent", index: 1, label: "a" }), + ).toBe(false); + expect( + isWorkflowProgressEntry({ + type: "workflow_agent", + index: 1, + label: "a", + state: 3, + }), + ).toBe(false); + }); + + // Documents the deliberate limit of this guard: optional fields are not + // type-checked, so a corrupt one survives. That is why accessors reaching + // for string methods go through `readOptionalString` — see the + // "tolerates a corrupt optional field" test below. + it("admits a well-formed entry even when an optional field is corrupt", () => { + expect( + isWorkflowProgressEntry({ + type: "workflow_agent", + index: 1, + label: "a", + state: "done", + phaseTitle: 1, + }), + ).toBe(true); + }); +}); + +describe("readOptionalString / readOptionalNumber", () => { + it("passes through valid values and nulls everything else", () => { + expect(readOptionalString("x")).toBe("x"); + expect(readOptionalString("")).toBe(""); + for (const bad of [1, null, undefined, {}, [], true]) { + expect(readOptionalString(bad)).toBeNull(); + } + + expect(readOptionalNumber(0)).toBe(0); + expect(readOptionalNumber(42)).toBe(42); + // NaN and infinities would poison a running total just as a string does. + for (const bad of ["1", null, undefined, NaN, Infinity, -Infinity, {}]) { + expect(readOptionalNumber(bad)).toBeNull(); + } + }); }); describe("phaseTitleOf", () => { @@ -199,6 +277,21 @@ describe("phaseTitleOf", () => { expect(phaseTitleOf(agent(1, "done", { phaseTitle: "" }))).toBeNull(); expect(phaseTitleOf(agent(1, "done", { phaseTitle: " " }))).toBeNull(); }); + + // The guard admits this shape (optional fields go unchecked), so the + // accessor is the layer that has to survive it. `.trim()` on a number + // throws, which would take down the whole card render. + it("tolerates a corrupt optional field instead of throwing", () => { + const corrupt = { + type: "workflow_agent", + index: 1, + label: "a", + state: "done", + phaseTitle: 1, + } as unknown as WorkflowAgentEntry; + expect(() => phaseTitleOf(corrupt)).not.toThrow(); + expect(phaseTitleOf(corrupt)).toBeNull(); + }); }); describe("isAgentTerminal", () => { diff --git a/src/ui/src/types/workflow.ts b/src/ui/src/types/workflow.ts index 6a4a7cb9f..a5cc6dac1 100644 --- a/src/ui/src/types/workflow.ts +++ b/src/ui/src/types/workflow.ts @@ -67,31 +67,60 @@ export type WorkflowProgressEntry = | WorkflowAgentEntry | WorkflowUnknownEntry; -/** The complete set of entry kinds Rust can persist. `Unknown` is included - * deliberately: the Rust side maps any unrecognized incoming kind to it, so - * it is a legitimate stored value rather than a parse failure. */ -const KNOWN_ENTRY_TYPES = new Set([ - "workflow_phase", - "workflow_agent", - "Unknown", -]); - /** * Boundary check for a single entry decoded from persisted JSON. * - * Narrow enough to justify the type predicate: it accepts only the kinds - * the union actually models. A looser "has a string `type`" check would - * assert `WorkflowProgressEntry` for `{type: "bogus"}`, which is unsound — - * harmless while every consumer re-discriminates on `type`, but a trap for - * the first one that trusts the type instead (an exhaustive switch with a - * `never` default, say, or reading `.label` off a presumed agent). + * Validates the kind *and* the fields that kind declares as required, so + * the type predicate is honest about everything the interface says is + * non-optional. `Unknown` is an accepted kind on purpose: Rust maps any + * incoming kind it doesn't recognize to it, making it a legitimate stored + * value rather than a parse failure. + * + * **Optional fields are not type-checked here.** Doing so would mean + * per-field validation of ~25 attributes to guard against data we + * ourselves serialized from Rust, where the types are already enforced. + * The cost of that trade is real but bounded: a corrupt optional field + * survives this check, so accessors that call *methods* on one must + * tolerate a wrong type. `readOptionalString` exists for exactly that and + * is used at every such site — see `phaseTitleOf`. Numeric accumulators + * take the same care via `readOptionalNumber`, where a stray string would + * otherwise turn `+=` into concatenation and silently corrupt a total + * rather than throwing. */ export function isWorkflowProgressEntry( value: unknown, ): value is WorkflowProgressEntry { if (typeof value !== "object" || value === null) return false; - const type = (value as { type?: unknown }).type; - return typeof type === "string" && KNOWN_ENTRY_TYPES.has(type); + const entry = value as Record; + switch (entry.type) { + case "workflow_phase": + return typeof entry.index === "number" && typeof entry.title === "string"; + case "workflow_agent": + return ( + typeof entry.index === "number" && + typeof entry.label === "string" && + typeof entry.state === "string" + ); + case "Unknown": + return true; + default: + return false; + } +} + +/** Read an optional string field that survived `isWorkflowProgressEntry` + * without being type-checked. Returns null for anything that isn't a + * string, so a corrupt value degrades to "absent" instead of throwing + * when a caller reaches for a string method. */ +export function readOptionalString(value: unknown): string | null { + return typeof value === "string" ? value : null; +} + +/** Numeric counterpart to `readOptionalString`. Rejects NaN and infinities + * as well as non-numbers, so a corrupt value contributes 0 to a running + * total rather than poisoning it into NaN. */ +export function readOptionalNumber(value: unknown): number | null { + return typeof value === "number" && Number.isFinite(value) ? value : null; } export function isWorkflowPhase( @@ -129,7 +158,10 @@ export function isAgentTerminal(agent: WorkflowAgentEntry): boolean { * agents count as unphased. */ export function phaseTitleOf(agent: WorkflowAgentEntry): string | null { - const title = agent.phaseTitle?.trim(); + // Read through `readOptionalString` rather than trusting the declared + // type: `phaseTitle` is optional, so the boundary guard didn't verify it, + // and `.trim()` on a non-string throws. + const title = readOptionalString(agent.phaseTitle)?.trim(); return title ? title : null; } @@ -202,8 +234,10 @@ export function summarizeWorkflowProgress( if (currentPhaseTitle === null) currentPhaseTitle = phaseTitleOf(agent); } if (agent.state === "error") errorCount++; - totalTokens += agent.tokens ?? 0; - totalToolCalls += agent.toolCalls ?? 0; + // `?? 0` alone would let a corrupt non-number through `+=`, turning the + // total into a concatenated string or NaN rather than failing loudly. + totalTokens += readOptionalNumber(agent.tokens) ?? 0; + totalToolCalls += readOptionalNumber(agent.toolCalls) ?? 0; } // Fall back to the last declared phase only when nothing is in flight — From 8688639de4f7f0ce4b35d087e0354705befa3d9e Mon Sep 17 00:00:00 2001 From: Sean Callan Date: Fri, 24 Jul 2026 18:19:58 -0400 Subject: [PATCH 12/14] fix(chat): let the Workflow card own collapse in completed turns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `workflow:` is deliberately shared between the live card and the completed-turn group so a collapse choice survives the turn ending, but the two sides were spending it differently. While live, collapse hid only the agent tree and kept the header and progress rail. Once the turn ended, the enclosing `TurnSummary` chevron unmounted the whole card — so the finished run's agent count, failure badge, and token total vanished, which is exactly the summary someone collapsing a completed run wants to keep. `TurnSummary` now forwards a grouped workflow's `collapsed`/`onToggle` to the `WorkflowCard` instead of to its own chevron. The card is already a single-activity group with its own header, chevron, and rail, so the key means one thing on both sides and the run renders one chevron, not two. Keeping the card inside `TurnSummary` (rather than early-returning it the way skill markers do) preserves the turn footer and `TaskProgressBar`, which the standalone footer path does not render. Workflow groups also seed from `false` rather than `turn.collapsed`, which defaults to true and would otherwise fold an untouched card the instant its turn ended, reversing what the user was just watching. They no longer sync the legacy `turn.collapsed` flag, since they never read it. Four tests cover the collapsed header/badges/rail, the expanded default under a collapsed turn, the single collapse control, and persistence under the shared live key; all four fail against the previous behavior. --- site/src/content/docs/features/workflows.mdx | 2 +- .../chat/MessagesWithTurns.test.tsx | 132 ++++++++++++++++++ .../src/components/chat/MessagesWithTurns.tsx | 20 ++- .../components/chat/ToolActivitiesSection.tsx | 5 +- src/ui/src/components/chat/TurnSummary.tsx | 40 +++++- 5 files changed, 191 insertions(+), 8 deletions(-) diff --git a/site/src/content/docs/features/workflows.mdx b/site/src/content/docs/features/workflows.mdx index 63fa5385c..b240e7517 100644 --- a/site/src/content/docs/features/workflows.mdx +++ b/site/src/content/docs/features/workflows.mdx @@ -16,7 +16,7 @@ A workflow appears in the transcript as a card, in place of the tool call that l Rows carry extra badges where relevant: `cached` when a resumed run served an agent from its prior result instead of re-running it, `retry N` when an agent needed more than one attempt, and `worktree` or `remote` when an agent ran isolated. -The card is expanded by default. Collapsing it hides the agent tree but keeps the header and progress rail visible, and your choice persists for that run. +The card is expanded by default. Collapsing it hides the agent tree but keeps the header and progress rail visible, so a collapsed run still shows its agent count, failures, and tokens at a glance. That holds both while the run is live and after the launching turn has finished, and your choice carries across that boundary — collapse a running workflow and it stays collapsed, in the same shape, once the turn ends. ## The status pill diff --git a/src/ui/src/components/chat/MessagesWithTurns.test.tsx b/src/ui/src/components/chat/MessagesWithTurns.test.tsx index c08fc76a9..30f270ce3 100644 --- a/src/ui/src/components/chat/MessagesWithTurns.test.tsx +++ b/src/ui/src/components/chat/MessagesWithTurns.test.tsx @@ -844,3 +844,135 @@ describe("MessagesWithTurns conclusion gating", () => { expect(container.textContent).not.toContain("Shipped the migration."); }); }); + +describe("MessagesWithTurns workflow groups", () => { + const WORKFLOW_SCRIPT = `export const meta = { + name: 'review-changes', + description: 'Review the diff across dimensions', + phases: [{ title: 'Review' }], +}`; + + // One done + one errored: both are terminal, so the badge reads "2/2 agents" + // alongside "1 failed". Two agents (rather than one) keeps the count badge + // distinguishable from the failure badge in the collapsed assertions below. + function workflowActivity(): ToolActivity { + return { + toolUseId: "toolu_wf1", + toolName: "Workflow", + inputJson: JSON.stringify({ script: WORKFLOW_SCRIPT }), + // A workflow's tool_result lands at LAUNCH, not completion. + resultText: "Workflow launched in background. Task ID: w4stpeffj", + collapsed: false, + summary: "review-changes", + agentStatus: "completed", + workflowProgress: [ + { type: "workflow_phase", index: 1, title: "Review" }, + { + type: "workflow_agent", + index: 1, + label: "review:bugs", + state: "done", + phaseTitle: "Review", + }, + { + type: "workflow_agent", + index: 2, + label: "review:perf", + state: "error", + phaseTitle: "Review", + }, + ], + }; + } + + const workflowMessages = [ + message("user-1", "User", "Review the diff"), + message("assistant-1", "Assistant", "Launched a workflow."), + ]; + + async function renderWorkflowTurn(turn: CompletedTurn): Promise { + useAppStore.setState({ completedTurns: { [SESSION_ID]: [turn] } }); + return render( + , + ); + } + + it("renders a finished workflow expanded even when its turn is collapsed", async () => { + // `turn.collapsed` defaults true for ordinary tool groups. A workflow must + // not inherit that: the card was expanded the whole time it was running, + // and folding it the instant the turn ends reverses what the user saw. + const container = await renderWorkflowTurn({ + ...completedTurn([workflowActivity()]), + collapsed: true, + }); + + expect(container.textContent).toContain("review:bugs"); + expect(container.textContent).toContain("review:perf"); + }); + + it("keeps the header, badges, and rail visible when the card is collapsed", async () => { + // The regression this guards: collapse used to be spent on the enclosing + // TurnSummary chevron, which unmounted the entire card — so the finished + // run's agent count and failure badge vanished, which is precisely the + // summary someone collapsing a completed workflow wants to keep. + useAppStore.setState({ + collapsedToolGroupsBySession: { + [SESSION_ID]: { "workflow:toolu_wf1": true }, + }, + }); + const container = await renderWorkflowTurn( + completedTurn([workflowActivity()]), + ); + + expect(container.textContent).toContain("2/2 agents"); + expect(container.textContent).toContain("1 failed"); + expect(container.querySelector('[role="progressbar"]')).not.toBeNull(); + // Only the agent tree is hidden. + expect(container.textContent).not.toContain("review:bugs"); + }); + + it("gives the group exactly one collapse control, on the card itself", async () => { + const container = await renderWorkflowTurn( + completedTurn([workflowActivity()]), + ); + + // No generic TurnSummary chevron wrapping the card — otherwise the run + // would render two stacked chevrons meaning two different things. + expect(container.querySelectorAll("[class*=turnHeader]")).toHaveLength(0); + const cardHeader = container.querySelector( + '[role="button"][aria-label*="workflow review-changes"]', + ); + expect(cardHeader).not.toBeNull(); + expect(cardHeader?.getAttribute("aria-expanded")).toBe("true"); + }); + + it("persists a card collapse under the same key the live card uses", async () => { + const container = await renderWorkflowTurn( + completedTurn([workflowActivity()]), + ); + const cardHeader = container.querySelector( + '[role="button"][aria-label*="workflow review-changes"]', + ) as HTMLElement; + + await act(async () => { + cardHeader.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + // `workflow:` — the key `GroupedWorkflowActivity` writes while + // the run is live, so the choice carries across the turn boundary. + expect( + useAppStore.getState().collapsedToolGroupsBySession[SESSION_ID]?.[ + "workflow:toolu_wf1" + ], + ).toBe(true); + expect(container.textContent).not.toContain("review:bugs"); + expect(container.querySelector('[role="progressbar"]')).not.toBeNull(); + }); +}); diff --git a/src/ui/src/components/chat/MessagesWithTurns.tsx b/src/ui/src/components/chat/MessagesWithTurns.tsx index 55e9e6bde..6154e2230 100644 --- a/src/ui/src/components/chat/MessagesWithTurns.tsx +++ b/src/ui/src/components/chat/MessagesWithTurns.tsx @@ -749,7 +749,15 @@ export const MessagesWithTurns = memo(function MessagesWithTurns({ // accidentally colliding with a real tool group. `tools:__empty__:${turn.id}`; const userOverride = collapsedToolGroups?.[groupKey]; - const collapsed = userOverride ?? turn.collapsed; + // Workflows default to expanded on both sides of the turn + // boundary. `turn.collapsed` defaults to true, which would fold + // an untouched workflow card the instant its turn ended — + // reversing what the user was just watching, and for the one + // group kind whose whole point is the phase/agent tree (see + // `GroupedWorkflowActivity` for the same default while live). + const isWorkflowGroup = kind === "workflow"; + const collapsed = + userOverride ?? (isWorkflowGroup ? false : turn.collapsed); // Single-group turns also flip the legacy `turn.collapsed` // flag so persistence-aware code (Cmd-A "collapse all" etc.) // sees the same state without needing to consult the override @@ -763,7 +771,15 @@ export const MessagesWithTurns = memo(function MessagesWithTurns({ const onToggle = () => { const next = !collapsed; setCollapsedToolGroup(sessionId, groupKey, next); - if (isSingleGroupTurn && next !== turn.collapsed) { + // Workflow groups read the override against their own `false` + // default and never consult `turn.collapsed`, so syncing the + // legacy flag here would drift it away from what the card + // actually shows. + if ( + !isWorkflowGroup && + isSingleGroupTurn && + next !== turn.collapsed + ) { toggleCompletedTurn(sessionId, globalIdx); } }; diff --git a/src/ui/src/components/chat/ToolActivitiesSection.tsx b/src/ui/src/components/chat/ToolActivitiesSection.tsx index d5cd9032d..6fbec2e98 100644 --- a/src/ui/src/components/chat/ToolActivitiesSection.tsx +++ b/src/ui/src/components/chat/ToolActivitiesSection.tsx @@ -247,7 +247,10 @@ function GroupedToolActivityRows({ * * Collapse state persists through the same `collapsedToolGroupsBySession` * slice as every other group, keyed `workflow:`, so the user's - * choice survives the running→completed transition into `TurnSummary`. + * choice survives the running→completed transition into `TurnSummary` — + * which forwards that same state to this card rather than to its own + * chevron, so collapse means "hide the agent tree, keep the header and + * rail" on both sides of the boundary. */ function GroupedWorkflowActivity({ sessionId, diff --git a/src/ui/src/components/chat/TurnSummary.tsx b/src/ui/src/components/chat/TurnSummary.tsx index 5e507602a..54ded1747 100644 --- a/src/ui/src/components/chat/TurnSummary.tsx +++ b/src/ui/src/components/chat/TurnSummary.tsx @@ -133,11 +133,35 @@ export function TurnSummary({ activityMatchesSearch(activity, searchQuery, worktreePath), ); const isExpanded = inline || !collapsed || queryHasMatch; + + // In grouped mode a workflow is always a group of exactly one activity + // (`groupToolActivitiesForDisplay` gives it its own group), and the card it + // renders already owns a header, chevron, and progress rail. So hand this + // group's `collapsed`/`onToggle` to the card rather than spending them on + // the generic TurnSummary chevron. + // + // That keeps one chevron on screen instead of two, but the real reason is + // that `workflow:` is shared with the live card on purpose (see + // `collapsedToolGroupKey`) — and wrapping made the key mean two different + // things across the running→completed boundary. While live, collapse hid + // only the agent tree and kept the header/rail; once the turn ended the + // TurnSummary chevron unmounted the whole card, so the final agent count, + // failure badge, and token total silently disappeared — exactly the summary + // a user collapsing a finished run wants to keep. Forwarding makes collapse + // mean "hide the agent tree" on both sides. + const soleActivity = + visibleActivities.length === 1 ? visibleActivities[0] : undefined; + const workflowCollapseOwner = + !inline && soleActivity && isWorkflowActivity(soleActivity) + ? soleActivity + : null; + const renderedActivities = visibleActivities.map((act: ToolActivity) => { if (isWorkflowActivity(act)) { - // Always expanded here — the enclosing TurnSummary header already - // provides the collapse affordance for a finished run, and nesting a - // second chevron inside it would give the same card two of them. + // Reached only in inline display mode (grouped mode routes the card + // through `workflowCollapseOwner` above). Inline mode renders every + // activity flush with no enclosing chevron, so the card has no collapse + // affordance to inherit and stays expanded. return ; } if (isAgentActivity(act)) { @@ -166,7 +190,15 @@ export function TurnSummary({ return (
- {inline ? ( + {workflowCollapseOwner ? ( +
+ +
+ ) : inline ? (
{renderedActivities}
) : (
From 62b4eae0c2a4aa0e7a3a61ac5ba9cb97636b99d2 Mon Sep 17 00:00:00 2001 From: Sean Callan Date: Fri, 24 Jul 2026 20:04:53 -0400 Subject: [PATCH 13/14] fix(chat): persist a workflow's final tree from the store, not the event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The terminal-notification write was gated on the notification carrying `workflow_progress`. It never does. Decompiling the Claude CLI (byte- identical across 2.1.208/218/219) shows one `task_notification` emitter, and its payload is `{task_id, tool_use_id, status, output_file, summary, usage, skip_transcript}` — the CLI's own zod schema for the subtype enumerates the same closed set. The only emitter attaching a tree is the `task_progress` one; the workflow completion path receives the final tree but spends it on `` counts in the notification's body text rather than putting it on the wire. So the gate never opened and `updateTurnToolActivityProgress` was dead code. The row stayed as `save_turn_tool_activities` wrote it seconds after launch — `"[]"` with a null status, because the tool_result ("launched in background") lands before any agent has run. Every finished run replayed as "Starting workflow…" with no rail, behind a status pill that spun forever because the stale row still said "running", one per historical run, stacking uncapped. That contradicts workflows.mdx's "The pill disappears when the run finishes". It was not reload-only: `hydrateCompletedTurns` clobbers correct in-memory state on scroll-up pagination, `/clear`, rollback, and first session open. Read the tree from the store instead. The CLI flushes a last `task_progress` carrying the complete all-done tree before the task leaves `running`, so the store holds the final state when the notification arrives. By then the activity has migrated into `completedTurns`, hence `findToolActivity`, which mirrors the search `updateToolActivity` already does on the write side. Also decouples the two columns: the guard previously tied status persistence to tree presence, so both orderings lost data. Both now COALESCE, and the caller passes `null` for a tree it doesn't have, so a status-only write can't blank a good row. Five hook tests and one db test; the three behavioral hook tests fail against the old gate. --- src-tauri/src/commands/chat/checkpoint.rs | 4 +- src/db/checkpoint.rs | 49 +++- src/ui/src/hooks/useAgentStream.ts | 66 +++-- ...seAgentStream.workflowPersistence.test.tsx | 271 ++++++++++++++++++ src/ui/src/services/tauri/checkpoints.ts | 9 +- src/ui/src/stores/findToolActivity.ts | 42 +++ 6 files changed, 411 insertions(+), 30 deletions(-) create mode 100644 src/ui/src/hooks/useAgentStream.workflowPersistence.test.tsx create mode 100644 src/ui/src/stores/findToolActivity.ts diff --git a/src-tauri/src/commands/chat/checkpoint.rs b/src-tauri/src/commands/chat/checkpoint.rs index 265b14b22..eaa3d2ba8 100644 --- a/src-tauri/src/commands/chat/checkpoint.rs +++ b/src-tauri/src/commands/chat/checkpoint.rs @@ -302,14 +302,14 @@ pub async fn save_turn_tool_activities( #[tauri::command] pub async fn update_turn_tool_activity_progress( tool_use_id: String, - workflow_progress_json: String, + workflow_progress_json: Option, agent_status: Option, state: State<'_, AppState>, ) -> Result<(), String> { let db = Database::open(&state.db_path).map_err(|e| e.to_string())?; db.update_turn_tool_activity_progress( &tool_use_id, - &workflow_progress_json, + workflow_progress_json.as_deref(), agent_status.as_deref(), ) .map_err(|e| e.to_string())?; diff --git a/src/db/checkpoint.rs b/src/db/checkpoint.rs index 0d79284d2..18b2c9cfc 100644 --- a/src/db/checkpoint.rs +++ b/src/db/checkpoint.rs @@ -905,15 +905,19 @@ impl Database { pub fn update_turn_tool_activity_progress( &self, tool_use_id: &str, - workflow_progress_json: &str, + workflow_progress_json: Option<&str>, agent_status: Option<&str>, ) -> Result { - // COALESCE keeps the stored status when the caller has nothing - // better to say, so a progress-only update can't blank a status - // that a prior notification already resolved. + // COALESCE on both columns keeps the stored value when the caller + // has nothing better to say, so neither field can be blanked by an + // update that only knows about the other. Status-only updates are + // the reason the tree is optional: a run that ends without ever + // reporting agents still has to resolve its status, and passing + // `None` there must not overwrite a tree the run did report. self.conn.execute( "UPDATE turn_tool_activities - SET workflow_progress_json = ?1, + SET workflow_progress_json = + COALESCE(?1, workflow_progress_json), agent_status = COALESCE(?2, agent_status) WHERE tool_use_id = ?3", params![workflow_progress_json, agent_status, tool_use_id], @@ -1444,7 +1448,7 @@ mod tests { let tree = r#"[{"type":"workflow_agent","index":1,"label":"review","state":"done"}]"#; let updated = db - .update_turn_tool_activity_progress("tu_a1", tree, Some("completed")) + .update_turn_tool_activity_progress("tu_a1", Some(tree), Some("completed")) .unwrap(); assert_eq!(updated, 1); @@ -1466,9 +1470,9 @@ mod tests { db.insert_turn_tool_activities(&[make_tool_activity("a1", "cp1", "Workflow", 0)]) .unwrap(); - db.update_turn_tool_activity_progress("tu_a1", "[]", Some("completed")) + db.update_turn_tool_activity_progress("tu_a1", Some("[]"), Some("completed")) .unwrap(); - db.update_turn_tool_activity_progress("tu_a1", r#"[{"type":"Unknown"}]"#, None) + db.update_turn_tool_activity_progress("tu_a1", Some(r#"[{"type":"Unknown"}]"#), None) .unwrap(); let turns = db.list_completed_turns("w1").unwrap(); @@ -1478,13 +1482,40 @@ mod tests { ); } + /// `COALESCE` on the tree column: a status-only update must not blank a + /// tree the run already reported. This is the terminal-notification + /// path — the CLI never carries `workflow_progress` on + /// `task_notification`, so a run whose store entry has no tree resolves + /// its status with `None` here and the stored tree has to survive. + #[test] + fn test_update_turn_tool_activity_progress_preserves_tree_when_none() { + let db = setup_db_with_workspace(); + db.insert_chat_message(&make_chat_msg(&db, "m1", "w1", ChatRole::Assistant, "a1")) + .unwrap(); + db.insert_checkpoint(&make_checkpoint(&db, "cp1", "w1", "m1", 0)) + .unwrap(); + db.insert_turn_tool_activities(&[make_tool_activity("a1", "cp1", "Workflow", 0)]) + .unwrap(); + + let tree = r#"[{"type":"workflow_agent","index":1,"label":"review","state":"done"}]"#; + db.update_turn_tool_activity_progress("tu_a1", Some(tree), None) + .unwrap(); + db.update_turn_tool_activity_progress("tu_a1", None, Some("failed")) + .unwrap(); + + let turns = db.list_completed_turns("w1").unwrap(); + let activity = &turns[0].activities[0]; + assert_eq!(activity.workflow_progress_json, tree); + assert_eq!(activity.agent_status.as_deref(), Some("failed")); + } + /// An unknown `tool_use_id` is normal (the turn may never have been /// checkpointed), so it reports zero rows rather than erroring. #[test] fn test_update_turn_tool_activity_progress_unknown_id_is_not_an_error() { let db = setup_db_with_workspace(); let updated = db - .update_turn_tool_activity_progress("tu_missing", "[]", Some("completed")) + .update_turn_tool_activity_progress("tu_missing", Some("[]"), Some("completed")) .unwrap(); assert_eq!(updated, 0); } diff --git a/src/ui/src/hooks/useAgentStream.ts b/src/ui/src/hooks/useAgentStream.ts index acfda4c7c..416402f6b 100644 --- a/src/ui/src/hooks/useAgentStream.ts +++ b/src/ui/src/hooks/useAgentStream.ts @@ -1,6 +1,7 @@ import { useEffect, useRef } from "react"; import { listen } from "@tauri-apps/api/event"; import { useAppStore } from "../stores/useAppStore"; +import { findToolActivity } from "../stores/findToolActivity"; import { loadChatHistory, saveTurnToolActivities, @@ -338,26 +339,57 @@ export function useAgentStream() { // A backgrounded Workflow usually finishes after the turn // that launched it has already been checkpointed, so its // final tree has nowhere to land — `save_turn_tool_activities` - // ran minutes ago. Write it directly at the activity row. + // ran minutes ago, writing `"[]"` and a null status because + // the tool_result ("launched in background") arrives within a + // second of launch, before any progress exists. Write the + // finished state directly at the activity row instead. // - // Only on the terminal notification, not on every progress - // tick: one write per run instead of dozens, and the - // completed state is what replay actually needs. A reload - // mid-run still shows the tree as of the turn's end. - if ( - streamEvent.subtype === "task_notification" && - updates.workflowProgress !== undefined - ) { - void updateTurnToolActivityProgress( + // Read the tree back out of the store rather than off this + // event: the CLI attaches `workflow_progress` to `task_progress` + // ONLY — its `task_notification` payload carries just + // `task_id` / `tool_use_id` / `status` / `output_file` / + // `summary` / `usage`, with no tree on any code path. Gating + // this write on the event carrying one therefore never fired, + // leaving every finished run to replay as "Starting workflow…" + // behind a status pill that spun forever. + // + // The store is the right source: `updateToolActivity` above has + // already applied the terminal status, and the last + // `task_progress` tick left the completed tree there. By now + // the activity has migrated into `completedTurns`, so the + // lookup has to check both places — hence `findToolActivity`. + // + // Still only on the terminal notification, not on every tick: + // one write per run instead of dozens, and the completed state + // is what replay actually needs. + if (streamEvent.subtype === "task_notification") { + const activity = findToolActivity( + useAppStore.getState(), + sessionId, streamEvent.tool_use_id, - JSON.stringify(updates.workflowProgress), - updates.agentStatus ?? null, - ).catch((err) => { - debugChat("stream", "persist workflow progress failed", { - toolUseId: streamEvent.tool_use_id, - error: String(err), + ); + // Persist for workflows even when no tree was ever reported + // (a run that failed before its first `task_progress`): the + // terminal status alone still has to land, or the pill keeps + // spinning. `null` leaves the stored tree untouched in that + // case rather than blanking it — both columns COALESCE. + // `agentStatus` is read back off the activity so a + // notification without an explicit `status` still writes + // whatever the store settled on. + if (activity?.toolName === "Workflow") { + void updateTurnToolActivityProgress( + streamEvent.tool_use_id, + activity.workflowProgress + ? JSON.stringify(activity.workflowProgress) + : null, + activity.agentStatus ?? null, + ).catch((err) => { + debugChat("stream", "persist workflow progress failed", { + toolUseId: streamEvent.tool_use_id, + error: String(err), + }); }); - }); + } } if (streamEvent.task_id) { const pending = pendingAgentToolCallsRef.current[streamEvent.task_id] || []; diff --git a/src/ui/src/hooks/useAgentStream.workflowPersistence.test.tsx b/src/ui/src/hooks/useAgentStream.workflowPersistence.test.tsx new file mode 100644 index 000000000..8b85a0880 --- /dev/null +++ b/src/ui/src/hooks/useAgentStream.workflowPersistence.test.tsx @@ -0,0 +1,271 @@ +// @vitest-environment happy-dom + +/** + * Regression tests for persisting a backgrounded `Workflow` run's final + * state at its activity row. + * + * A workflow's `tool_result` ("launched in background") lands within a + * second of launch, so `save_turn_tool_activities` checkpoints the turn + * with `workflow_progress_json: "[]"` and a null status long before any + * agent has run. The completed tree therefore has to be written later, + * when the terminal `task_notification` arrives — `update_turn_tool_ + * activity_progress` is the only UPDATE path in the schema. + * + * The bug these pin: that write used to be gated on the notification + * itself carrying `workflow_progress`. The CLI attaches that field to + * `task_progress` events ONLY — its `task_notification` payload is + * `{task_id, tool_use_id, status, output_file, summary, usage}` with no + * tree on any code path — so the gate never opened and the write never + * happened. Every finished run replayed from the DB as "Starting + * workflow…" behind a status pill that spun forever, because the stale + * row still said the run was going. + * + * Strategy matches `useAgentStream.sessionRenamed.test.tsx`: mock + * `listen()` to capture the `agent-stream` handler, mount the hook, and + * push raw stream payloads through it. + */ + +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +type HandlerRecord = (event: { payload: unknown }) => void; +const registeredHandlers = new Map(); + +vi.mock("@tauri-apps/api/event", () => ({ + listen: (eventName: string, handler: HandlerRecord) => { + const list = registeredHandlers.get(eventName) ?? []; + list.push(handler); + registeredHandlers.set(eventName, list); + return Promise.resolve(() => { + const after = + registeredHandlers.get(eventName)?.filter((h) => h !== handler) ?? []; + registeredHandlers.set(eventName, after); + }); + }, +})); + +const persistSpy = vi.hoisted(() => vi.fn().mockResolvedValue(undefined)); + +vi.mock("../services/tauri", async () => { + const actual = await vi.importActual( + "../services/tauri", + ); + return { + ...actual, + loadChatHistory: vi.fn().mockResolvedValue([]), + saveTurnToolActivities: vi.fn().mockResolvedValue(undefined), + setSessionCliInvocation: vi.fn().mockResolvedValue(undefined), + updateTurnToolActivityProgress: persistSpy, + }; +}); + +import { useAgentStream } from "./useAgentStream"; +import { useAppStore, type ToolActivity } from "../stores/useAppStore"; +import type { WorkflowProgressEntry } from "../types/workflow"; + +const WS_ID = "ws-1"; +const SESSION_ID = "session-1"; +const WF_ID = "toolu_wf1"; + +const mountedRoots: Root[] = []; +const mountedContainers: HTMLElement[] = []; + +async function mountHook(): Promise { + function Probe() { + useAgentStream(); + return null; + } + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + mountedRoots.push(root); + mountedContainers.push(container); + await act(async () => { + root.render(); + }); +} + +async function fireStream(streamEvent: Record): Promise { + const handlers = registeredHandlers.get("agent-stream") ?? []; + expect(handlers.length).toBeGreaterThan(0); + await act(async () => { + for (const handler of handlers) { + handler({ + payload: { + workspace_id: WS_ID, + chat_session_id: SESSION_ID, + event: { Stream: streamEvent }, + }, + }); + } + }); +} + +const TREE: WorkflowProgressEntry[] = [ + { type: "workflow_phase", index: 1, title: "Review" }, + { + type: "workflow_agent", + index: 1, + label: "review:bugs", + state: "done", + phaseTitle: "Review", + }, +]; + +function workflowActivity( + overrides: Partial = {}, +): ToolActivity { + return { + toolUseId: WF_ID, + toolName: "Workflow", + inputJson: JSON.stringify({ script: "export const meta = {}" }), + resultText: "Workflow launched in background. Task ID: w4stpeffj", + collapsed: false, + summary: "review-changes", + agentStatus: "running", + workflowProgress: TREE, + ...overrides, + }; +} + +/** The terminal event the CLI actually emits — note the absent tree. */ +function terminalNotification(): Record { + return { + type: "system", + subtype: "task_notification", + task_id: "wf_1", + tool_use_id: WF_ID, + status: "completed", + output_file: "", + summary: "Workflow completed", + }; +} + +beforeEach(() => { + registeredHandlers.clear(); + persistSpy.mockClear(); + useAppStore.setState({ + agentQuestions: {}, + toolActivities: {}, + completedTurns: {}, + streamingContent: {}, + streamingThinking: {}, + promptStartTime: {}, + sessionsByWorkspace: {}, + }); +}); + +afterEach(async () => { + vi.useRealTimers(); + for (const root of mountedRoots.splice(0).reverse()) { + await act(async () => { + root.unmount(); + }); + } + for (const container of mountedContainers.splice(0)) { + container.remove(); + } +}); + +describe("useAgentStream — workflow progress persistence", () => { + it("persists the last-known tree when the terminal notification omits it", async () => { + useAppStore.setState({ + toolActivities: { [SESSION_ID]: [workflowActivity()] }, + }); + await mountHook(); + + await fireStream(terminalNotification()); + + expect(persistSpy).toHaveBeenCalledTimes(1); + const [toolUseId, treeJson, status] = persistSpy.mock.calls[0]; + expect(toolUseId).toBe(WF_ID); + expect(JSON.parse(treeJson)).toEqual(TREE); + expect(status).toBe("completed"); + }); + + it("finds the activity after its turn has already been checkpointed", async () => { + // The normal case for a backgrounded run: the launching turn ended + // minutes before the workflow did, so the activity now lives in + // `completedTurns`, not `toolActivities`. + useAppStore.setState({ + toolActivities: { [SESSION_ID]: [] }, + completedTurns: { + [SESSION_ID]: [ + { + id: "turn-1", + activities: [workflowActivity()], + messageCount: 2, + collapsed: false, + afterMessageIndex: 2, + }, + ], + }, + }); + await mountHook(); + + await fireStream(terminalNotification()); + + expect(persistSpy).toHaveBeenCalledTimes(1); + expect(JSON.parse(persistSpy.mock.calls[0][1])).toEqual(TREE); + expect(persistSpy.mock.calls[0][2]).toBe("completed"); + }); + + it("writes the terminal status without blanking the tree when none is known", async () => { + // A run that failed before its first `task_progress`. The status has + // to land or the pill spins forever against a stale "running" row — + // but the tree must go as `null`, not `"[]"`, so the COALESCE leaves + // whatever is stored intact instead of overwriting a good row. + useAppStore.setState({ + toolActivities: { + [SESSION_ID]: [ + workflowActivity({ workflowProgress: undefined, agentStatus: undefined }), + ], + }, + }); + await mountHook(); + + await fireStream({ ...terminalNotification(), status: "failed" }); + + expect(persistSpy).toHaveBeenCalledTimes(1); + expect(persistSpy.mock.calls[0][1]).toBeNull(); + expect(persistSpy.mock.calls[0][2]).toBe("failed"); + }); + + it("does not persist on progress ticks — one write per run", async () => { + useAppStore.setState({ + toolActivities: { [SESSION_ID]: [workflowActivity()] }, + }); + await mountHook(); + + await fireStream({ + type: "system", + subtype: "task_progress", + task_id: "wf_1", + tool_use_id: WF_ID, + workflow_progress: TREE, + }); + + expect(persistSpy).not.toHaveBeenCalled(); + }); + + it("ignores terminal notifications for non-workflow activities", async () => { + // Backgrounded Bash and Agent tasks emit the same notification; only + // Workflow rows carry a progress tree worth writing back. + useAppStore.setState({ + toolActivities: { + [SESSION_ID]: [ + workflowActivity({ + toolName: "Agent", + workflowProgress: undefined, + }), + ], + }, + }); + await mountHook(); + + await fireStream(terminalNotification()); + + expect(persistSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/src/ui/src/services/tauri/checkpoints.ts b/src/ui/src/services/tauri/checkpoints.ts index 19f857f5b..0d77fafb9 100644 --- a/src/ui/src/services/tauri/checkpoints.ts +++ b/src/ui/src/services/tauri/checkpoints.ts @@ -48,10 +48,15 @@ export function saveTurnToolActivities( /** Persist the workflow progress tree for a single already-checkpointed * activity. Used when a backgrounded `Workflow` finishes after the turn - * that launched it was already saved. */ + * that launched it was already saved. + * + * Both fields are optional and COALESCE against the stored row: pass + * `null` for either to leave it untouched. A run that ends without ever + * reporting agents still needs its status resolved, and must not blank + * the tree on the way. */ export function updateTurnToolActivityProgress( toolUseId: string, - workflowProgressJson: string, + workflowProgressJson: string | null, agentStatus: string | null, ): Promise { return invoke("update_turn_tool_activity_progress", { diff --git a/src/ui/src/stores/findToolActivity.ts b/src/ui/src/stores/findToolActivity.ts new file mode 100644 index 000000000..9ba7cfaea --- /dev/null +++ b/src/ui/src/stores/findToolActivity.ts @@ -0,0 +1,42 @@ +import type { CompletedTurn, ToolActivity } from "./useAppStore"; + +/** + * Find a tool activity by `toolUseId` across both places one can live. + * + * A tool activity starts in `toolActivities[sessionId]` while its turn is + * running and migrates into `completedTurns[sessionId][N].activities` when + * the turn ends — the `toolUseId` is preserved verbatim across that move. + * Anything reading an activity outside the turn that created it therefore + * has to check both, in that order. + * + * This mirrors the search `updateToolActivity` performs on the write side + * (`chatSlice.ts`), including the newest-first scan of completed turns: + * `toolUseId` is unique per turn, so the newest match is the only match, + * and the relevant activity is almost always in the most recent turn. + * + * The backgrounded-`Workflow` case is why this exists as a read helper. + * A workflow's turn is checkpointed within a second of launch, so by the + * time its terminal `task_notification` arrives — minutes later — the + * activity has long since moved into `completedTurns`. + */ +export function findToolActivity( + state: { + toolActivities: Record; + completedTurns: Record; + }, + sessionId: string, + toolUseId: string, +): ToolActivity | undefined { + const live = state.toolActivities[sessionId]?.find( + (a) => a.toolUseId === toolUseId, + ); + if (live) return live; + + const turns = state.completedTurns[sessionId]; + if (!turns) return undefined; + for (let i = turns.length - 1; i >= 0; i--) { + const match = turns[i].activities.find((a) => a.toolUseId === toolUseId); + if (match) return match; + } + return undefined; +} From 7d764c8d19cee964f0681929f38fae3d5f1bd050 Mon Sep 17 00:00:00 2001 From: Sean Callan Date: Fri, 24 Jul 2026 20:48:08 -0400 Subject: [PATCH 14/14] docs(chat): correct the rationale for undefined vs [] progress trees MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The doc comment on `parseWorkflowProgress` and its matching test comment both claimed `WorkflowCard` keys off `workflowProgress` being present. It doesn't: `summarizeWorkflowProgress` coerces `undefined` and `[]` to the same empty summary, and whether a card renders is decided by `toolName === "Workflow"` via `isWorkflowActivity`. A non-workflow activity was never going to render as a workflow regardless. The distinction is still load-bearing, but for a reason introduced in 62b4eae0: the terminal-notification write sends `null` for an absent tree so the stored column is COALESCEd rather than overwritten with `"[]"`. Collapsing the two here would let a run rehydrated without a tree blank a good row. Comments only — no behavior change. --- src/ui/src/utils/reconstructTurns.test.ts | 6 ++++-- src/ui/src/utils/reconstructTurns.ts | 16 ++++++++++++---- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/ui/src/utils/reconstructTurns.test.ts b/src/ui/src/utils/reconstructTurns.test.ts index 4bf21d7af..10c23a00b 100644 --- a/src/ui/src/utils/reconstructTurns.test.ts +++ b/src/ui/src/utils/reconstructTurns.test.ts @@ -430,8 +430,10 @@ describe("reconstructCompletedTurns", () => { }); }); - // `undefined`, not `[]` — a non-workflow activity stores "[]", and the - // card keys off `workflowProgress` being present at all. + // `undefined`, not `[]` — the distinction is load-bearing for the + // terminal-notification write in `useAgentStream`, which sends `null` + // for an absent tree so the stored column is COALESCEd rather than + // blanked. Rendering treats the two identically. it("returns undefined when nothing survives or the column is empty", () => { for (const json of ["[]", "[{\"type\":\"bogus\"}]", "not json", ""]) { const result = reconstructCompletedTurns(messages, turnWithProgress(json)); diff --git a/src/ui/src/utils/reconstructTurns.ts b/src/ui/src/utils/reconstructTurns.ts index 9f05dc80a..9814bd25c 100644 --- a/src/ui/src/utils/reconstructTurns.ts +++ b/src/ui/src/utils/reconstructTurns.ts @@ -125,10 +125,18 @@ function parseAgentToolCalls(value: string | null | undefined) { /** Rehydrate a persisted `Workflow` progress tree. * - * Returns `undefined` rather than `[]` for the empty case so a non-workflow - * activity (every one of which stores `"[]"`) doesn't come back looking - * like a workflow that produced no agents — `WorkflowCard` keys off - * `workflowProgress` being present at all. + * Returns `undefined` rather than `[]` when nothing valid is stored, so + * "no tree was ever recorded" stays distinguishable from "a tree that + * reported no agents". Rendering does not need that distinction — + * `summarizeWorkflowProgress` coerces both to the same empty summary, and + * whether a card renders at all is decided by `toolName === "Workflow"` + * (`isWorkflowActivity`), not by this field. + * + * The terminal-notification write in `useAgentStream` is what needs it: + * it sends `null` for an absent tree so the stored column is COALESCEd + * instead of overwritten with `"[]"`. Collapsing the two here would let a + * run whose activity was rehydrated without a tree blank the good row that + * a prior write had already put in the database. * * Entries are filtered through `isWorkflowProgressEntry`, which admits only * the kinds the union models — including `Unknown`, which Rust writes for