diff --git a/CLAUDE.md b/CLAUDE.md index f4953d6..5de7524 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -52,6 +52,7 @@ All new features, fixes, and significant changes should include tests whenever p ### Mocking Strategies #### Console Mocking + When testing code that intentionally logs errors (like error handling paths), mock console methods to prevent stderr output that makes tests appear flaky: ```typescript @@ -70,6 +71,7 @@ it("handles errors gracefully", async () => { ``` #### E2E Integration Testing for Cross-Platform Code + For code that calls platform-specific system APIs (like Windows PowerShell or Linux notify-send), use helper functions that build the command structure without execution. This allows CI to verify cross-platform compatibility on Linux-only containers: ```rust @@ -99,6 +101,7 @@ fn test_e2e_notify_send_command_structure() { ``` This approach: + - Verifies command structure, argument order, and escaping logic - Tests cross-platform code paths without requiring the target platform - Allows CI to catch regressions in Windows-specific code whilst running on Linux diff --git a/src-tauri/src/wsl_bridge.rs b/src-tauri/src/wsl_bridge.rs index 9294fff..1c18302 100644 --- a/src-tauri/src/wsl_bridge.rs +++ b/src-tauri/src/wsl_bridge.rs @@ -678,6 +678,34 @@ fn handle_stderr( } } + // Check if this is a SubagentStop hook message + if line.contains("[SubagentStop Hook]") { + if let Some(stop_data) = parse_subagent_stop_hook(&line) { + tracing::debug!("Parsed SubagentStop hook: tool_use_id={:?}", + stop_data.parent_tool_use_id); + + // Emit agent-end event if we have a tool_use_id + if let Some(tool_use_id) = stop_data.parent_tool_use_id { + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_millis() as u64; + + let _ = app.emit( + "claude:agent-end", + AgentEndEvent { + tool_use_id, + ended_at: now, + is_error: false, + conversation_id: conversation_id.clone(), + duration_ms: None, + num_turns: None, + }, + ); + } + } + } + // Still emit the stderr line as output let _ = app.emit( "claude:output", @@ -732,6 +760,30 @@ fn parse_subagent_start_hook(line: &str) -> Option { }) } +#[derive(Debug)] +struct SubagentStopData { + parent_tool_use_id: Option, +} + +fn parse_subagent_stop_hook(line: &str) -> Option { + // Parse: [SubagentStop Hook] ... parent_tool_use_id=Some("toolu_xxx"), ... + + // Extract parent_tool_use_id if present + let parent_tool_use_id = if line.contains("parent_tool_use_id=Some") { + line.split("parent_tool_use_id=Some(\"") + .nth(1)? + .split('"') + .next() + .map(|s| s.to_string()) + } else { + None + }; + + Some(SubagentStopData { + parent_tool_use_id, + }) +} + fn process_json_line( line: &str, app: &AppHandle, @@ -1823,4 +1875,98 @@ mod tests { let manager = shared.lock(); assert!(manager.get_active_conversations().is_empty()); } + + // SubagentStart hook parsing tests + #[test] + fn test_parse_subagent_start_hook_with_parent() { + let line = r#"[SubagentStart Hook] agent_id=agent-abc123, parent_tool_use_id=Some("toolu_01XYZ789"), session_id=123"#; + let result = parse_subagent_start_hook(line); + + assert!(result.is_some()); + let data = result.unwrap(); + assert_eq!(data.agent_id, "agent-abc123"); + assert_eq!(data.parent_tool_use_id, Some("toolu_01XYZ789".to_string())); + } + + #[test] + fn test_parse_subagent_start_hook_without_parent() { + let line = r#"[SubagentStart Hook] agent_id=agent-xyz789, parent_tool_use_id=None, session_id=456"#; + let result = parse_subagent_start_hook(line); + + assert!(result.is_some()); + let data = result.unwrap(); + assert_eq!(data.agent_id, "agent-xyz789"); + assert_eq!(data.parent_tool_use_id, None); + } + + #[test] + fn test_parse_subagent_start_hook_invalid() { + let line = "[SubagentStart Hook] invalid data"; + let result = parse_subagent_start_hook(line); + + assert!(result.is_none()); + } + + #[test] + fn test_parse_subagent_start_hook_with_extra_fields() { + let line = r#"[SubagentStart Hook] agent_id=agent-test, parent_tool_use_id=Some("toolu_test"), session_id=789, cwd=/home/user"#; + let result = parse_subagent_start_hook(line); + + assert!(result.is_some()); + let data = result.unwrap(); + assert_eq!(data.agent_id, "agent-test"); + assert_eq!(data.parent_tool_use_id, Some("toolu_test".to_string())); + } + + // SubagentStop hook parsing tests + #[test] + fn test_parse_subagent_stop_hook_with_parent() { + let line = r#"[SubagentStop Hook] stop_hook_active=true, parent_tool_use_id=Some("toolu_01ABC123"), session_id=123"#; + let result = parse_subagent_stop_hook(line); + + assert!(result.is_some()); + let data = result.unwrap(); + assert_eq!(data.parent_tool_use_id, Some("toolu_01ABC123".to_string())); + } + + #[test] + fn test_parse_subagent_stop_hook_without_parent() { + let line = r#"[SubagentStop Hook] stop_hook_active=true, parent_tool_use_id=None, session_id=456"#; + let result = parse_subagent_stop_hook(line); + + assert!(result.is_some()); + let data = result.unwrap(); + assert_eq!(data.parent_tool_use_id, None); + } + + #[test] + fn test_parse_subagent_stop_hook_minimal() { + let line = r#"[SubagentStop Hook] parent_tool_use_id=Some("toolu_minimal")"#; + let result = parse_subagent_stop_hook(line); + + assert!(result.is_some()); + let data = result.unwrap(); + assert_eq!(data.parent_tool_use_id, Some("toolu_minimal".to_string())); + } + + #[test] + fn test_parse_subagent_stop_hook_with_extra_fields() { + let line = r#"[SubagentStop Hook] stop_hook_active=false, parent_tool_use_id=Some("toolu_extra"), session_id=789, transcript_path=/path/to/transcript"#; + let result = parse_subagent_stop_hook(line); + + assert!(result.is_some()); + let data = result.unwrap(); + assert_eq!(data.parent_tool_use_id, Some("toolu_extra".to_string())); + } + + #[test] + fn test_parse_subagent_stop_hook_empty() { + let line = "[SubagentStop Hook]"; + let result = parse_subagent_stop_hook(line); + + // Should still return Some with None parent_tool_use_id + assert!(result.is_some()); + let data = result.unwrap(); + assert_eq!(data.parent_tool_use_id, None); + } } diff --git a/src/lib/components/AgentMonitorPanel.svelte b/src/lib/components/AgentMonitorPanel.svelte index 9c3dde7..9fcd85b 100644 --- a/src/lib/components/AgentMonitorPanel.svelte +++ b/src/lib/components/AgentMonitorPanel.svelte @@ -118,6 +118,8 @@ try { await invoke("interrupt_claude", { conversationId: currentConversationId }); + // Mark all running agents as errored after killing the process + agentStore.markAllErrored(currentConversationId); } catch (error) { console.error("Failed to kill Claude process:", error); } diff --git a/src/lib/stores/agents.test.ts b/src/lib/stores/agents.test.ts new file mode 100644 index 0000000..c82d649 --- /dev/null +++ b/src/lib/stores/agents.test.ts @@ -0,0 +1,325 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { agentStore, getAgentsForConversation, runningAgentCount } from "./agents"; +import { get } from "svelte/store"; +import type { AgentInfo } from "$lib/types/agents"; + +describe("agents store", () => { + const conversationId = "test-conversation-1"; + const otherConversationId = "test-conversation-2"; + + const createMockAgent = (overrides?: Partial): AgentInfo => ({ + toolUseId: "toolu_test123", + description: "Test agent", + subagentType: "Explore", + startedAt: Date.now(), + status: "running", + ...overrides, + }); + + beforeEach(() => { + // Clear all conversations by subscribing and getting state + let state: Record = {}; + const unsub = agentStore.subscribe((s) => { + state = s; + }); + unsub(); + + // Clear each conversation + for (const convId of Object.keys(state)) { + agentStore.clearConversation(convId); + } + }); + + describe("addAgent", () => { + it("adds an agent to a conversation", () => { + const agent = createMockAgent(); + agentStore.addAgent(conversationId, agent); + + const agents = get(getAgentsForConversation(conversationId)); + expect(agents).toHaveLength(1); + expect(agents[0]).toEqual(agent); + }); + + it("adds multiple agents to the same conversation", () => { + const agent1 = createMockAgent({ toolUseId: "tool1" }); + const agent2 = createMockAgent({ toolUseId: "tool2" }); + + agentStore.addAgent(conversationId, agent1); + agentStore.addAgent(conversationId, agent2); + + const agents = get(getAgentsForConversation(conversationId)); + expect(agents).toHaveLength(2); + expect(agents[0]).toEqual(agent1); + expect(agents[1]).toEqual(agent2); + }); + + it("keeps agents in different conversations separate", () => { + const agent1 = createMockAgent({ toolUseId: "tool1" }); + const agent2 = createMockAgent({ toolUseId: "tool2" }); + + agentStore.addAgent(conversationId, agent1); + agentStore.addAgent(otherConversationId, agent2); + + const agents1 = get(getAgentsForConversation(conversationId)); + const agents2 = get(getAgentsForConversation(otherConversationId)); + + expect(agents1).toHaveLength(1); + expect(agents2).toHaveLength(1); + expect(agents1[0]).toEqual(agent1); + expect(agents2[0]).toEqual(agent2); + }); + }); + + describe("updateAgentId", () => { + it("updates the agent_id for a specific agent", () => { + const agent = createMockAgent({ agentId: undefined }); + agentStore.addAgent(conversationId, agent); + + agentStore.updateAgentId(conversationId, agent.toolUseId, "agent-abc123"); + + const agents = get(getAgentsForConversation(conversationId)); + expect(agents[0].agentId).toBe("agent-abc123"); + }); + + it("does nothing if conversation doesn't exist", () => { + agentStore.updateAgentId("nonexistent", "tool1", "agent1"); + // Should not throw + expect(true).toBe(true); + }); + + it("does nothing if tool_use_id doesn't exist", () => { + const agent = createMockAgent(); + agentStore.addAgent(conversationId, agent); + + agentStore.updateAgentId(conversationId, "nonexistent-tool", "agent1"); + + const agents = get(getAgentsForConversation(conversationId)); + expect(agents[0].agentId).toBeUndefined(); + }); + }); + + describe("endAgent", () => { + it("marks an agent as completed", () => { + const agent = createMockAgent({ status: "running" }); + agentStore.addAgent(conversationId, agent); + + const endTime = Date.now(); + agentStore.endAgent(conversationId, agent.toolUseId, endTime, false); + + const agents = get(getAgentsForConversation(conversationId)); + expect(agents[0].status).toBe("completed"); + expect(agents[0].endedAt).toBe(endTime); + expect(agents[0].durationMs).toBeGreaterThanOrEqual(0); // Duration can be 0 if timestamps are the same + }); + + it("marks an agent as errored", () => { + const agent = createMockAgent({ status: "running" }); + agentStore.addAgent(conversationId, agent); + + const endTime = Date.now(); + agentStore.endAgent(conversationId, agent.toolUseId, endTime, true); + + const agents = get(getAgentsForConversation(conversationId)); + expect(agents[0].status).toBe("errored"); + expect(agents[0].endedAt).toBe(endTime); + }); + + it("calculates duration correctly", () => { + const startTime = Date.now() - 5000; // 5 seconds ago + const agent = createMockAgent({ startedAt: startTime, status: "running" }); + agentStore.addAgent(conversationId, agent); + + const endTime = Date.now(); + agentStore.endAgent(conversationId, agent.toolUseId, endTime, false); + + const agents = get(getAgentsForConversation(conversationId)); + expect(agents[0].durationMs).toBeGreaterThanOrEqual(5000); + expect(agents[0].durationMs).toBeLessThanOrEqual(6000); // Allow some buffer + }); + + it("does nothing if conversation doesn't exist", () => { + agentStore.endAgent("nonexistent", "tool1", Date.now(), false); + // Should not throw + expect(true).toBe(true); + }); + + it("does nothing if agent doesn't exist", () => { + const agent = createMockAgent(); + agentStore.addAgent(conversationId, agent); + + agentStore.endAgent(conversationId, "nonexistent-tool", Date.now(), false); + + const agents = get(getAgentsForConversation(conversationId)); + expect(agents[0].status).toBe("running"); // Status unchanged + }); + }); + + describe("markAllErrored", () => { + it("marks all running agents as errored", () => { + const agent1 = createMockAgent({ toolUseId: "tool1", status: "running" }); + const agent2 = createMockAgent({ toolUseId: "tool2", status: "running" }); + const agent3 = createMockAgent({ toolUseId: "tool3", status: "completed" }); + + agentStore.addAgent(conversationId, agent1); + agentStore.addAgent(conversationId, agent2); + agentStore.addAgent(conversationId, agent3); + + agentStore.markAllErrored(conversationId); + + const agents = get(getAgentsForConversation(conversationId)); + expect(agents[0].status).toBe("errored"); + expect(agents[0].endedAt).toBeGreaterThan(0); + expect(agents[1].status).toBe("errored"); + expect(agents[1].endedAt).toBeGreaterThan(0); + expect(agents[2].status).toBe("completed"); // Already completed, unchanged + }); + + it("does nothing if conversation doesn't exist", () => { + agentStore.markAllErrored("nonexistent"); + // Should not throw + expect(true).toBe(true); + }); + + it("does nothing if conversation has no running agents", () => { + const agent = createMockAgent({ status: "completed" }); + agentStore.addAgent(conversationId, agent); + + agentStore.markAllErrored(conversationId); + + const agents = get(getAgentsForConversation(conversationId)); + expect(agents[0].status).toBe("completed"); // Unchanged + }); + }); + + describe("clearCompleted", () => { + it("removes completed and errored agents", () => { + const agent1 = createMockAgent({ toolUseId: "tool1", status: "running" }); + const agent2 = createMockAgent({ toolUseId: "tool2", status: "completed" }); + const agent3 = createMockAgent({ toolUseId: "tool3", status: "errored" }); + + agentStore.addAgent(conversationId, agent1); + agentStore.addAgent(conversationId, agent2); + agentStore.addAgent(conversationId, agent3); + + agentStore.clearCompleted(conversationId); + + const agents = get(getAgentsForConversation(conversationId)); + expect(agents).toHaveLength(1); + expect(agents[0].toolUseId).toBe("tool1"); // Only running agent remains + }); + + it("does nothing if conversation doesn't exist", () => { + agentStore.clearCompleted("nonexistent"); + // Should not throw + expect(true).toBe(true); + }); + + it("clears all agents if all are completed", () => { + const agent1 = createMockAgent({ toolUseId: "tool1", status: "completed" }); + const agent2 = createMockAgent({ toolUseId: "tool2", status: "errored" }); + + agentStore.addAgent(conversationId, agent1); + agentStore.addAgent(conversationId, agent2); + + agentStore.clearCompleted(conversationId); + + const agents = get(getAgentsForConversation(conversationId)); + expect(agents).toHaveLength(0); + }); + }); + + describe("clearConversation", () => { + it("removes all agents from a conversation", () => { + const agent1 = createMockAgent({ toolUseId: "tool1" }); + const agent2 = createMockAgent({ toolUseId: "tool2" }); + + agentStore.addAgent(conversationId, agent1); + agentStore.addAgent(conversationId, agent2); + + agentStore.clearConversation(conversationId); + + const agents = get(getAgentsForConversation(conversationId)); + expect(agents).toHaveLength(0); + }); + + it("only removes agents from the specified conversation", () => { + const agent1 = createMockAgent({ toolUseId: "tool1" }); + const agent2 = createMockAgent({ toolUseId: "tool2" }); + + agentStore.addAgent(conversationId, agent1); + agentStore.addAgent(otherConversationId, agent2); + + agentStore.clearConversation(conversationId); + + const agents1 = get(getAgentsForConversation(conversationId)); + const agents2 = get(getAgentsForConversation(otherConversationId)); + + expect(agents1).toHaveLength(0); + expect(agents2).toHaveLength(1); + expect(agents2[0]).toEqual(agent2); + }); + + it("does nothing if conversation doesn't exist", () => { + agentStore.clearConversation("nonexistent"); + // Should not throw + expect(true).toBe(true); + }); + }); + + describe("runningAgentCount", () => { + it("counts running agents across all conversations", () => { + const agent1 = createMockAgent({ toolUseId: "tool1", status: "running" }); + const agent2 = createMockAgent({ toolUseId: "tool2", status: "running" }); + const agent3 = createMockAgent({ toolUseId: "tool3", status: "completed" }); + const agent4 = createMockAgent({ toolUseId: "tool4", status: "running" }); + + agentStore.addAgent(conversationId, agent1); + agentStore.addAgent(conversationId, agent2); + agentStore.addAgent(conversationId, agent3); + agentStore.addAgent(otherConversationId, agent4); + + const count = get(runningAgentCount); + expect(count).toBe(3); // 2 from first conversation + 1 from second + }); + + it("returns 0 when no agents are running", () => { + const agent1 = createMockAgent({ status: "completed" }); + const agent2 = createMockAgent({ status: "errored" }); + + agentStore.addAgent(conversationId, agent1); + agentStore.addAgent(otherConversationId, agent2); + + const count = get(runningAgentCount); + expect(count).toBe(0); + }); + + it("updates when agents complete", () => { + const agent = createMockAgent({ status: "running" }); + agentStore.addAgent(conversationId, agent); + + let count = get(runningAgentCount); + expect(count).toBe(1); + + agentStore.endAgent(conversationId, agent.toolUseId, Date.now(), false); + + count = get(runningAgentCount); + expect(count).toBe(0); + }); + + it("updates when conversation is cleared", () => { + const agent1 = createMockAgent({ toolUseId: "tool1", status: "running" }); + const agent2 = createMockAgent({ toolUseId: "tool2", status: "running" }); + + agentStore.addAgent(conversationId, agent1); + agentStore.addAgent(conversationId, agent2); + + let count = get(runningAgentCount); + expect(count).toBe(2); + + agentStore.clearConversation(conversationId); + + count = get(runningAgentCount); + expect(count).toBe(0); + }); + }); +}); diff --git a/src/lib/stores/conversations.ts b/src/lib/stores/conversations.ts index 8c55e5d..3b9be8f 100644 --- a/src/lib/stores/conversations.ts +++ b/src/lib/stores/conversations.ts @@ -10,6 +10,7 @@ import type { CharacterState } from "$lib/types/states"; import { cleanupConversationTracking } from "$lib/tauri"; import { characterState } from "$lib/stores/character"; import { sessionsStore } from "$lib/stores/sessions"; +import { agentStore } from "$lib/stores/agents"; export interface ConversationSummary { generatedAt: Date; @@ -333,6 +334,10 @@ function createConversationsStore() { // Clean up tracking for this conversation (including temp files) await cleanupConversationTracking(id); + // Clean up agent tracking for this conversation + // This prevents the badge from persisting after tab close + agentStore.clearConversation(id); + conversations.update((c) => { c.delete(id); return c; diff --git a/src/lib/tauri.ts b/src/lib/tauri.ts index 7e42c1f..1d5d767 100644 --- a/src/lib/tauri.ts +++ b/src/lib/tauri.ts @@ -183,6 +183,9 @@ export async function initializeTauriListeners() { // (permission prompts trigger reconnects and agents may complete before reconnect) if (!skipNextGreeting && targetConversationId) { agentStore.markAllErrored(targetConversationId); + // Clear the conversation's agents from the store on real disconnect + // This prevents agents from persisting across sessions + agentStore.clearConversation(targetConversationId); } // Only remove from connected set if we're not about to reconnect