diff --git a/.story_kit/work/2_current/147_bug_activity_indicator_still_only_shows_thinking_despite_bug_140_fix.md b/.story_kit/work/2_current/147_bug_activity_indicator_still_only_shows_thinking_despite_bug_140_fix.md new file mode 100644 index 0000000..0ce5219 --- /dev/null +++ b/.story_kit/work/2_current/147_bug_activity_indicator_still_only_shows_thinking_despite_bug_140_fix.md @@ -0,0 +1,80 @@ +--- +name: "Activity indicator still only shows Thinking despite bug 140 fix" +--- + +# Bug 147: Activity indicator still only shows Thinking despite bug 140 fix + +## Description + +Bug 140 fixed the frontend display condition but activity labels still never appear. The full data path has been traced and the suspected failure point identified. + +## End-to-End Data Path + +### 1. Frontend display (FIXED by bug 140) +- `frontend/src/components/Chat.tsx` line 686: `{loading && (activityStatus != null || !streamingContent) && (` +- `frontend/src/components/Chat.tsx` line 697: `{activityStatus ?? "Thinking..."}` +- `frontend/src/components/Chat.tsx` line 204: `setActivityStatus(formatToolActivity(toolName))` — called by `onActivity` callback + +### 2. WebSocket client receives event +- `frontend/src/api/client.ts` line 350: `if (data.type === "tool_activity") this.onActivity?.(data.tool_name)` + +### 3. Server sends ToolActivity over WebSocket (WIRED CORRECTLY) +- `server/src/http/ws.rs` line 251-254: activity callback sends `WsResponse::ToolActivity { tool_name }` +- This callback is passed to `chat::chat()` as the `on_activity` closure + +### 4. chat::chat passes callback to Claude Code provider +- `server/src/llm/chat.rs`: passes `on_activity` through to `claude_code::chat_stream` +- `server/src/llm/providers/claude_code.rs` line 47: `mut on_activity: A` parameter +- `server/src/llm/providers/claude_code.rs` line 70: creates internal `activity_tx` channel +- `server/src/llm/providers/claude_code.rs` line 94: drains channel and calls `on_activity(&name)` + +### 5. PTY event processing (SUSPECTED FAILURE POINT) +- `server/src/llm/providers/claude_code.rs` line 327: `process_json_event()` dispatches parsed JSON +- Line 348-353: matches `"stream_event"` type → extracts inner `event` → calls `handle_stream_event()` +- `server/src/llm/providers/claude_code.rs` line 486: `handle_stream_event()` matches on event type +- Line 494-500: matches `"content_block_start"` with `content_block.type == "tool_use"` → sends to `activity_tx` + +### 6. The problem +`handle_stream_event` only matches `content_block_start` — this is the **raw Anthropic streaming API format**. But Claude Code's `--output-format stream-json` may NOT emit raw Anthropic events wrapped in `stream_event`. It likely uses its own event types for tool calls (e.g. `tool_use_begin`, `tool_use`, or similar). + +The existing `process_json_event` also matches `"assistant"` (line 355) and `"user"` (line 363) event types from stream-json, but these are complete messages — they arrive after the tool call is done, not when it starts. So there's no event being caught at tool-call-start time. + +## Investigation Steps + +1. Add logging in `process_json_event` (line 334) to print every `event_type` received from the PTY during a chat session with tool use +2. Identify which event type Claude Code emits when it starts a tool call +3. Add matching for that event type to fire `activity_tx.send(tool_name)` + +## Key Files +- `server/src/llm/providers/claude_code.rs` line 327: `process_json_event` — event dispatcher +- `server/src/llm/providers/claude_code.rs` line 486: `handle_stream_event` — only handles Anthropic API format +- `server/src/http/ws.rs` line 251: activity callback wiring to WebSocket +- `frontend/src/components/Chat.tsx` line 203: `onActivity` handler that sets display state + +## How to Reproduce + +1. Rebuild both frontend and backend from master (which includes story 86 and bug 140) +2. Open web UI chat +3. Send a message that causes tool use (e.g. ask agent to read a file) +4. Watch the activity indicator + +## Actual Result + +Indicator always shows "Thinking..." and never changes to tool activity labels like "Reading file..." or "Executing command..." + +## Expected Result + +Indicator should cycle through tool activity labels as the agent calls tools + +## Hints for the Coder + +- **Check external docs**: The Claude Code CLI `--output-format stream-json` format may be documented at https://docs.anthropic.com or in the Claude Code repo. Search for the actual event schema before guessing. +- **Add logging as an intermediate step**: If unsure about the event format, add a `slog!` or `eprintln!` in `process_json_event` (line 334) to log every `event_type` received. Rebuild, run a web UI chat with tool use, and inspect the output to see exactly what events arrive. +- **Run the CLI directly**: You can run `claude -p "read /etc/hosts" --output-format stream-json` in a terminal to see the raw stream-json output and identify the event types for tool calls. +- **Don't assume the Anthropic API format**: The existing `content_block_start` matching was likely copied from the Anthropic provider. Claude Code's stream-json is a different format. + +## Acceptance Criteria + +- [ ] Activity indicator shows tool names (e.g. "Reading file...", "Executing command...") when the web UI agent calls tools +- [ ] Indicator still falls back to "Thinking..." when no tool activity is in progress +- [ ] Works for all tool types (Read, Write, Bash, Glob, Grep, etc.) diff --git a/frontend/src/components/Chat.test.tsx b/frontend/src/components/Chat.test.tsx index 420fc96..1858d20 100644 --- a/frontend/src/components/Chat.test.tsx +++ b/frontend/src/components/Chat.test.tsx @@ -473,4 +473,85 @@ describe("Chat activity status indicator (Bug 140)", () => { // The activity indicator should NOT be visible (just streaming bubble) expect(screen.queryByTestId("activity-indicator")).not.toBeInTheDocument(); }); + + it("shows activity label for Claude Code tool names (Read, Bash, etc.)", async () => { + render(); + + await waitFor(() => expect(capturedWsHandlers).not.toBeNull()); + + // Simulate sending a message to set loading=true + const input = screen.getByPlaceholderText("Send a message..."); + await act(async () => { + fireEvent.change(input, { target: { value: "Read my file" } }); + }); + await act(async () => { + fireEvent.keyDown(input, { key: "Enter", shiftKey: false }); + }); + + // Simulate tokens arriving + act(() => { + capturedWsHandlers?.onToken("Let me read that."); + }); + + // Claude Code sends tool name "Read" (not "read_file") + act(() => { + capturedWsHandlers?.onActivity("Read"); + }); + + const indicator = await screen.findByTestId("activity-indicator"); + expect(indicator).toBeInTheDocument(); + expect(indicator).toHaveTextContent("Reading file..."); + }); + + it("shows activity label for Claude Code Bash tool", async () => { + render(); + + await waitFor(() => expect(capturedWsHandlers).not.toBeNull()); + + const input = screen.getByPlaceholderText("Send a message..."); + await act(async () => { + fireEvent.change(input, { target: { value: "Run the tests" } }); + }); + await act(async () => { + fireEvent.keyDown(input, { key: "Enter", shiftKey: false }); + }); + + act(() => { + capturedWsHandlers?.onToken("Running tests now."); + }); + + act(() => { + capturedWsHandlers?.onActivity("Bash"); + }); + + const indicator = await screen.findByTestId("activity-indicator"); + expect(indicator).toBeInTheDocument(); + expect(indicator).toHaveTextContent("Executing command..."); + }); + + it("shows generic label for unknown tool names", async () => { + render(); + + await waitFor(() => expect(capturedWsHandlers).not.toBeNull()); + + const input = screen.getByPlaceholderText("Send a message..."); + await act(async () => { + fireEvent.change(input, { target: { value: "Do something" } }); + }); + await act(async () => { + fireEvent.keyDown(input, { key: "Enter", shiftKey: false }); + }); + + act(() => { + capturedWsHandlers?.onToken("Working on it."); + }); + + act(() => { + capturedWsHandlers?.onActivity("SomeCustomTool"); + }); + + const indicator = await screen.findByTestId("activity-indicator"); + expect(indicator).toBeInTheDocument(); + expect(indicator).toHaveTextContent("Using SomeCustomTool..."); + }); }); diff --git a/frontend/src/components/Chat.tsx b/frontend/src/components/Chat.tsx index 8e7728b..d6eb79e 100644 --- a/frontend/src/components/Chat.tsx +++ b/frontend/src/components/Chat.tsx @@ -16,16 +16,34 @@ const NARROW_BREAKPOINT = 900; function formatToolActivity(toolName: string): string { switch (toolName) { + // Built-in provider tool names case "read_file": + case "Read": return "Reading file..."; case "write_file": + case "Write": + case "Edit": return "Writing file..."; case "list_directory": - return "Listing directory..."; + case "Glob": + return "Listing files..."; case "search_files": + case "Grep": return "Searching files..."; case "exec_shell": + case "Bash": return "Executing command..."; + // Claude Code additional tool names + case "Task": + return "Running task..."; + case "WebFetch": + return "Fetching web content..."; + case "WebSearch": + return "Searching the web..."; + case "NotebookEdit": + return "Editing notebook..."; + case "TodoWrite": + return "Updating tasks..."; default: return `Using ${toolName}...`; } diff --git a/server/src/llm/providers/claude_code.rs b/server/src/llm/providers/claude_code.rs index 4edbfd8..7bc5493 100644 --- a/server/src/llm/providers/claude_code.rs +++ b/server/src/llm/providers/claude_code.rs @@ -96,6 +96,13 @@ impl ClaudeCodeProvider { } } + // Drain any remaining activity messages that were buffered when the + // token channel closed. The select! loop breaks on token_rx → None, + // but activity_rx may still hold signals sent in the same instant. + while let Ok(name) = activity_rx.try_recv() { + on_activity(&name); + } + pty_handle .await .map_err(|e| format!("PTY task panicked: {e}"))??; @@ -154,6 +161,11 @@ fn run_pty_session( cmd.arg("--output-format"); cmd.arg("stream-json"); cmd.arg("--verbose"); + // Enable partial streaming events so we receive stream_event messages + // containing raw API events (content_block_start, content_block_delta, + // etc.). Without this flag, only complete assistant/user/result events + // are emitted and tool-start activity signals never fire. + cmd.arg("--include-partial-messages"); // Delegate permission decisions to the MCP prompt_permission tool. // Claude Code will call this tool via the story-kit MCP server when // a tool requires user approval, instead of using PTY stdin/stdout. @@ -166,7 +178,7 @@ fn run_pty_session( cmd.env("CLAUDECODE", ""); slog!( - "[pty-debug] Spawning: claude -p \"{}\" {} --output-format stream-json --verbose --permission-prompt-tool mcp__story-kit__prompt_permission", + "[pty-debug] Spawning: claude -p \"{}\" {} --output-format stream-json --verbose --include-partial-messages --permission-prompt-tool mcp__story-kit__prompt_permission", user_message, resume_session_id .map(|s| format!("--resume {s}")) @@ -255,39 +267,18 @@ fn run_pty_session( Err(std::sync::mpsc::RecvTimeoutError::Timeout) => { // Check if child has exited if let Ok(Some(_status)) = child.try_wait() { - // Drain remaining lines + // Drain remaining lines through the same dispatch path + // (process_json_event) so activity signals fire correctly. while let Ok(Some(line)) = line_rx.try_recv() { let trimmed = line.trim(); - if let Ok(json) = serde_json::from_str::(trimmed) - && let Some(event_type) = - json.get("type").and_then(|t| t.as_str()) - { - match event_type { - "stream_event" => { - if let Some(event) = json.get("event") { - handle_stream_event(event, &token_tx, &activity_tx); - } - } - "assistant" => { - if let Some(message) = json.get("message") - && let Some(content) = message - .get("content") - .and_then(|c| c.as_array()) - { - parse_assistant_message(content, &msg_tx); - } - } - "user" => { - if let Some(message) = json.get("message") - && let Some(content) = message - .get("content") - .and_then(|c| c.as_array()) - { - parse_tool_results(content, &msg_tx); - } - } - _ => {} - } + if let Ok(json) = serde_json::from_str::(trimmed) { + process_json_event( + &json, + &token_tx, + &activity_tx, + &msg_tx, + &mut sid_tx, + ); } } break; @@ -356,6 +347,17 @@ fn process_json_event( if let Some(message) = json.get("message") && let Some(content) = message.get("content").and_then(|c| c.as_array()) { + // Fire activity signals for tool_use blocks as a fallback path. + // The primary path is via stream_event → content_block_start (real-time), + // but assistant events also carry tool_use blocks and serve as a reliable + // backup if stream_event delivery is delayed or missed. + for block in content { + if block.get("type").and_then(|t| t.as_str()) == Some("tool_use") + && let Some(name) = block.get("name").and_then(|n| n.as_str()) + { + let _ = activity_tx.send(name.to_string()); + } + } parse_assistant_message(content, msg_tx); } false @@ -943,6 +945,105 @@ mod tests { assert_eq!(tokens, vec!["word"]); } + #[test] + fn process_json_event_stream_event_tool_use_fires_activity() { + // This is the primary activity path: stream_event wrapping content_block_start + // with a tool_use block. Requires --include-partial-messages to be enabled. + let (tok_tx, _tok_rx, act_tx, mut act_rx, msg_tx, _msg_rx) = make_channels(); + let mut sid_tx = None::>; + let json = json!({ + "type": "stream_event", + "session_id": "s1", + "event": { + "type": "content_block_start", + "index": 1, + "content_block": {"type": "tool_use", "id": "toolu_abc", "name": "Bash", "input": {}} + } + }); + assert!(!process_json_event(&json, &tok_tx, &act_tx, &msg_tx, &mut sid_tx)); + drop(act_tx); + let activities: Vec = { + let mut v = vec![]; + while let Ok(a) = act_rx.try_recv() { + v.push(a); + } + v + }; + assert_eq!(activities, vec!["Bash"]); + } + + #[test] + fn process_json_event_assistant_with_tool_use_fires_activity() { + let (tok_tx, _tok_rx, act_tx, mut act_rx, msg_tx, _msg_rx) = make_channels(); + let mut sid_tx = None::>; + let json = json!({ + "type": "assistant", + "message": { + "content": [ + {"type": "text", "text": "Let me read that file."}, + {"type": "tool_use", "id": "toolu_1", "name": "Read", "input": {"file_path": "/foo.rs"}} + ] + } + }); + assert!(!process_json_event(&json, &tok_tx, &act_tx, &msg_tx, &mut sid_tx)); + drop(act_tx); + let activities: Vec = { + let mut v = vec![]; + while let Ok(a) = act_rx.try_recv() { + v.push(a); + } + v + }; + assert_eq!(activities, vec!["Read"]); + } + + #[test] + fn process_json_event_assistant_with_multiple_tool_uses_fires_all_activities() { + let (tok_tx, _tok_rx, act_tx, mut act_rx, msg_tx, _msg_rx) = make_channels(); + let mut sid_tx = None::>; + let json = json!({ + "type": "assistant", + "message": { + "content": [ + {"type": "tool_use", "id": "id1", "name": "Glob", "input": {}}, + {"type": "tool_use", "id": "id2", "name": "Bash", "input": {}} + ] + } + }); + assert!(!process_json_event(&json, &tok_tx, &act_tx, &msg_tx, &mut sid_tx)); + drop(act_tx); + let activities: Vec = { + let mut v = vec![]; + while let Ok(a) = act_rx.try_recv() { + v.push(a); + } + v + }; + assert_eq!(activities, vec!["Glob", "Bash"]); + } + + #[test] + fn process_json_event_assistant_text_only_no_activity() { + let (tok_tx, _tok_rx, act_tx, mut act_rx, msg_tx, _msg_rx) = make_channels(); + let mut sid_tx = None::>; + let json = json!({ + "type": "assistant", + "message": { + "content": [{"type": "text", "text": "Just text, no tools."}] + } + }); + assert!(!process_json_event(&json, &tok_tx, &act_tx, &msg_tx, &mut sid_tx)); + drop(act_tx); + let activities: Vec = { + let mut v = vec![]; + while let Ok(a) = act_rx.try_recv() { + v.push(a); + } + v + }; + assert!(activities.is_empty()); + } + #[test] fn process_json_event_assistant_event_parses_message() { let (tok_tx, _tok_rx, act_tx, _act_rx, msg_tx, msg_rx) = make_channels();