From 686d92712e176aa8ee3e295136b72a71461cabd5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 22:25:41 +0000 Subject: [PATCH 01/30] Bug analysis: per-file BUG-ANALYSIS.md docs (first completed batches) Each source file gets a sibling BUG-ANALYSIS.md documenting its business logic, functions, edge-case analysis, and bugs found. These docs are working artifacts: a final commit will remove them all once the bug fixes are in. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq --- .../background.BUG-ANALYSIS.md | 170 ++++++++++++++++ .../chrome-extension/check.BUG-ANALYSIS.md | 88 +++++++++ .../chrome-extension/content.BUG-ANALYSIS.md | 168 ++++++++++++++++ .../chrome-extension/options.BUG-ANALYSIS.md | 83 ++++++++ .../framework/dashboard/App.BUG-ANALYSIS.md | 119 +++++++++++ .../ActionsRunNotice.BUG-ANALYSIS.md | 34 ++++ .../ActionsRunNotice.test.BUG-ANALYSIS.md | 44 +++++ .../AddDeviceDialog.BUG-ANALYSIS.md | 67 +++++++ .../AddDeviceDialog.test.BUG-ANALYSIS.md | 48 +++++ .../AddProjectPanel.BUG-ANALYSIS.md | 76 +++++++ .../AddProjectPanel.test.BUG-ANALYSIS.md | 48 +++++ .../components/AgentActionBar.BUG-ANALYSIS.md | 50 +++++ .../AgentActionsMenu.BUG-ANALYSIS.md | 104 ++++++++++ .../components/AgentComposer.BUG-ANALYSIS.md | 99 ++++++++++ .../AgentComposer.test.BUG-ANALYSIS.md | 64 ++++++ .../components/AgentDetails.BUG-ANALYSIS.md | 58 ++++++ .../AgentErrorCount.BUG-ANALYSIS.md | 40 ++++ .../AgentErrorCount.test.BUG-ANALYSIS.md | 36 ++++ .../components/AgentFeed.BUG-ANALYSIS.md | 42 ++++ .../components/AgentHandoff.BUG-ANALYSIS.md | 103 ++++++++++ .../AgentHandoff.test.BUG-ANALYSIS.md | 62 ++++++ .../components/AgentHistory.BUG-ANALYSIS.md | 98 +++++++++ .../AgentHistory.test.BUG-ANALYSIS.md | 63 ++++++ .../components/AgentOverview.BUG-ANALYSIS.md | 56 ++++++ .../components/AgentView.BUG-ANALYSIS.md | 91 +++++++++ .../components/AgentView.test.BUG-ANALYSIS.md | 56 ++++++ .../components/Agents.BUG-ANALYSIS.md | 45 +++++ .../components/Agents.test.BUG-ANALYSIS.md | 40 ++++ .../components/AiQueue.BUG-ANALYSIS.md | 65 ++++++ .../components/AiQueue.test.BUG-ANALYSIS.md | 56 ++++++ .../components/AnsweredChoice.BUG-ANALYSIS.md | 38 ++++ .../components/AppFrame.BUG-ANALYSIS.md | 38 ++++ .../components/BrandLink.BUG-ANALYSIS.md | 34 ++++ .../components/BrandLink.test.BUG-ANALYSIS.md | 43 ++++ .../components/BridgeSettings.BUG-ANALYSIS.md | 67 +++++++ .../BridgeSettings.test.BUG-ANALYSIS.md | 64 ++++++ .../components/BrowserPanel.BUG-ANALYSIS.md | 87 ++++++++ .../BrowserPanel.test.BUG-ANALYSIS.md | 61 ++++++ .../components/ChoicePanel.BUG-ANALYSIS.md | 119 +++++++++++ .../CloudAgentNotice.BUG-ANALYSIS.md | 124 ++++++++++++ .../CloudAgentNotice.test.BUG-ANALYSIS.md | 64 ++++++ .../components/Composer.BUG-ANALYSIS.md | 91 +++++++++ .../components/Composer.test.BUG-ANALYSIS.md | 73 +++++++ .../ConnectionIndicator.BUG-ANALYSIS.md | 45 +++++ .../components/ContextFiles.BUG-ANALYSIS.md | 25 +++ .../ContextFiles.test.BUG-ANALYSIS.md | 22 +++ .../components/ContextMenu.BUG-ANALYSIS.md | 38 ++++ .../ContextMenu.test.BUG-ANALYSIS.md | 32 +++ .../components/DashboardPage.BUG-ANALYSIS.md | 41 ++++ .../DevicesSettings.BUG-ANALYSIS.md | 40 ++++ .../DevicesSettings.test.BUG-ANALYSIS.md | 33 ++++ .../components/DiffView.BUG-ANALYSIS.md | 62 ++++++ .../DisclosureToggle.BUG-ANALYSIS.md | 22 +++ .../DisclosureToggle.test.BUG-ANALYSIS.md | 24 +++ .../components/DiscordDialogs.BUG-ANALYSIS.md | 71 +++++++ .../DiscordDialogs.test.BUG-ANALYSIS.md | 39 ++++ .../components/DocsPanel.BUG-ANALYSIS.md | 41 ++++ .../DriverModelMenu.BUG-ANALYSIS.md | 46 +++++ .../DriverModelMenu.test.BUG-ANALYSIS.md | 35 ++++ .../components/EventList.BUG-ANALYSIS.md | 101 ++++++++++ .../components/EventList.test.BUG-ANALYSIS.md | 54 +++++ .../components/FilePreview.BUG-ANALYSIS.md | 44 +++++ .../FilePreview.test.BUG-ANALYSIS.md | 47 +++++ .../components/FileTree.BUG-ANALYSIS.md | 75 +++++++ .../components/FileTree.test.BUG-ANALYSIS.md | 43 ++++ .../components/GitStatusBar.BUG-ANALYSIS.md | 65 ++++++ .../GitStatusBar.test.BUG-ANALYSIS.md | 48 +++++ .../components/HotTickets.BUG-ANALYSIS.md | 48 +++++ .../HotTickets.test.BUG-ANALYSIS.md | 47 +++++ .../components/InlineBrowser.BUG-ANALYSIS.md | 43 ++++ .../InlineBrowser.test.BUG-ANALYSIS.md | 35 ++++ .../dashboard/components/Logo.BUG-ANALYSIS.md | 44 +++++ .../components/Logo.test.BUG-ANALYSIS.md | 38 ++++ .../components/Markdown.BUG-ANALYSIS.md | 67 +++++++ .../components/Markdown.test.BUG-ANALYSIS.md | 40 ++++ .../components/NotFound.BUG-ANALYSIS.md | 23 +++ .../NotificationsMenu.BUG-ANALYSIS.md | 54 +++++ .../NotificationsMenu.test.BUG-ANALYSIS.md | 49 +++++ .../OnboardingChecklist.BUG-ANALYSIS.md | 65 ++++++ .../OnboardingChecklist.test.BUG-ANALYSIS.md | 48 +++++ .../components/OpenQuestions.BUG-ANALYSIS.md | 69 +++++++ .../OpenQuestions.test.BUG-ANALYSIS.md | 49 +++++ .../components/OptionsMenu.BUG-ANALYSIS.md | 56 ++++++ .../OptionsMenu.test.BUG-ANALYSIS.md | 48 +++++ .../PresetCreatePanel.BUG-ANALYSIS.md | 49 +++++ .../PresetCreatePanel.test.BUG-ANALYSIS.md | 39 ++++ .../components/PresetsMenu.BUG-ANALYSIS.md | 77 ++++++++ .../PresetsMenu.test.BUG-ANALYSIS.md | 59 ++++++ .../components/ProjectActions.BUG-ANALYSIS.md | 34 ++++ .../components/ProjectDocs.BUG-ANALYSIS.md | 56 ++++++ .../ProjectDocs.test.BUG-ANALYSIS.md | 49 +++++ .../ProjectErrorBanner.BUG-ANALYSIS.md | 46 +++++ .../ProjectErrorBanner.test.BUG-ANALYSIS.md | 46 +++++ .../components/ProjectHome.BUG-ANALYSIS.md | 52 +++++ .../components/PromptEditor.BUG-ANALYSIS.md | 186 ++++++++++++++++++ .../PromptEditor.test.BUG-ANALYSIS.md | 63 ++++++ .../components/Quota.BUG-ANALYSIS.md | 128 ++++++++++++ .../components/Quota.test.BUG-ANALYSIS.md | 77 ++++++++ .../RemoteAgentNotice.BUG-ANALYSIS.md | 31 +++ .../ResolvedOptions.BUG-ANALYSIS.md | 52 +++++ .../ResolvedOptions.test.BUG-ANALYSIS.md | 58 ++++++ .../components/RightRail.BUG-ANALYSIS.md | 78 ++++++++ .../components/RightRail.test.BUG-ANALYSIS.md | 62 ++++++ .../components/StartAgentForm.BUG-ANALYSIS.md | 87 ++++++++ .../StartAgentForm.test.BUG-ANALYSIS.md | 64 ++++++ .../SystemPromptDisclosure.BUG-ANALYSIS.md | 88 +++++++++ ...ystemPromptDisclosure.test.BUG-ANALYSIS.md | 54 +++++ .../components/ThemeToggle.BUG-ANALYSIS.md | 34 ++++ .../ThemeToggle.test.BUG-ANALYSIS.md | 39 ++++ .../TicketDetailPage.BUG-ANALYSIS.md | 67 +++++++ .../TicketDetailPage.test.BUG-ANALYSIS.md | 52 +++++ .../TicketFilterBar.BUG-ANALYSIS.md | 71 +++++++ .../TicketFilterBar.test.BUG-ANALYSIS.md | 57 ++++++ .../components/TicketPlanPage.BUG-ANALYSIS.md | 39 ++++ .../TicketPlanPage.test.BUG-ANALYSIS.md | 33 ++++ .../components/TicketsPage.BUG-ANALYSIS.md | 83 ++++++++ .../TicketsPage.test.BUG-ANALYSIS.md | 57 ++++++ .../components/TicketsPanel.BUG-ANALYSIS.md | 71 +++++++ .../TicketsPanel.test.BUG-ANALYSIS.md | 51 +++++ .../components/ViewsRail.BUG-ANALYSIS.md | 61 ++++++ .../WorkspaceActions.BUG-ANALYSIS.md | 53 +++++ .../WorkspaceActions.test.BUG-ANALYSIS.md | 46 +++++ .../SuggestionList.BUG-ANALYSIS.md | 59 ++++++ .../prompt-editor/suggestion.BUG-ANALYSIS.md | 68 +++++++ .../prompt-editor/tokenize.BUG-ANALYSIS.md | 52 +++++ .../prompt-editor/tokens.BUG-ANALYSIS.md | 62 ++++++ .../components/ui/badge.BUG-ANALYSIS.md | 19 ++ .../components/ui/button.BUG-ANALYSIS.md | 30 +++ .../components/ui/card.BUG-ANALYSIS.md | 25 +++ .../components/ui/checkbox.BUG-ANALYSIS.md | 27 +++ .../ui/confirm-dialog.BUG-ANALYSIS.md | 53 +++++ .../components/ui/copy-button.BUG-ANALYSIS.md | 43 ++++ .../components/ui/dialog.BUG-ANALYSIS.md | 32 +++ .../ui/dropdown-menu.BUG-ANALYSIS.md | 45 +++++ .../components/ui/input.BUG-ANALYSIS.md | 20 ++ .../ui/message-scroller.BUG-ANALYSIS.md | 44 +++++ .../ui/message-scroller.test.BUG-ANALYSIS.md | 39 ++++ .../ui/option-label.BUG-ANALYSIS.md | 20 ++ .../components/ui/popover.BUG-ANALYSIS.md | 30 +++ .../components/ui/scroll-area.BUG-ANALYSIS.md | 38 ++++ .../ui/scroll-area.test.BUG-ANALYSIS.md | 38 ++++ .../components/ui/separator.BUG-ANALYSIS.md | 19 ++ .../components/ui/sheet.BUG-ANALYSIS.md | 36 ++++ .../components/ui/sidebar.BUG-ANALYSIS.md | 72 +++++++ .../components/ui/skeleton.BUG-ANALYSIS.md | 17 ++ .../components/ui/slider.BUG-ANALYSIS.md | 34 ++++ .../components/ui/tooltip.BUG-ANALYSIS.md | 32 +++ .../dashboard/design/build.BUG-ANALYSIS.md | 50 +++++ .../dashboard/design/previews.BUG-ANALYSIS.md | 53 +++++ .../design/vite.config.css.BUG-ANALYSIS.md | 24 +++ .../dashboard/lib/agent-label.BUG-ANALYSIS.md | 29 +++ .../lib/agent-option-rows.BUG-ANALYSIS.md | 49 +++++ .../agent-option-rows.test.BUG-ANALYSIS.md | 41 ++++ .../lib/agent-settings.BUG-ANALYSIS.md | 33 ++++ .../dashboard/lib/live-state.BUG-ANALYSIS.md | 31 +++ .../lib/live-state.test.BUG-ANALYSIS.md | 26 +++ .../notification-permission.BUG-ANALYSIS.md | 22 +++ .../lib/notify-channels.BUG-ANALYSIS.md | 25 +++ .../lib/notify-channels.test.BUG-ANALYSIS.md | 19 ++ .../dashboard/lib/preferences.BUG-ANALYSIS.md | 35 ++++ .../lib/preferences.test.BUG-ANALYSIS.md | 30 +++ .../dashboard/lib/profiles.BUG-ANALYSIS.md | 35 ++++ .../lib/profiles.test.BUG-ANALYSIS.md | 22 +++ .../dashboard/lib/queue-entry.BUG-ANALYSIS.md | 23 +++ .../lib/queue-entry.test.BUG-ANALYSIS.md | 19 ++ .../dashboard/lib/quota-bar.BUG-ANALYSIS.md | 31 +++ .../lib/quota-bar.test.BUG-ANALYSIS.md | 24 +++ .../dashboard/lib/quota.BUG-ANALYSIS.md | 17 ++ .../dashboard/lib/quota.test.BUG-ANALYSIS.md | 20 ++ .../lib/remote-target.BUG-ANALYSIS.md | 24 +++ .../lib/resume-command.BUG-ANALYSIS.md | 15 ++ .../lib/resume-command.test.BUG-ANALYSIS.md | 17 ++ .../dashboard/lib/route.BUG-ANALYSIS.md | 24 +++ .../dashboard/lib/route.test.BUG-ANALYSIS.md | 19 ++ .../dashboard/lib/rpc.BUG-ANALYSIS.md | 28 +++ .../lib/session-link.BUG-ANALYSIS.md | 16 ++ .../lib/session-link.test.BUG-ANALYSIS.md | 19 ++ .../dashboard/lib/status-tone.BUG-ANALYSIS.md | 15 ++ .../lib/ticket-filter.BUG-ANALYSIS.md | 38 ++++ .../lib/ticket-filter.test.BUG-ANALYSIS.md | 27 +++ .../lib/ticket-priority.BUG-ANALYSIS.md | 16 ++ .../lib/ticket-priority.test.BUG-ANALYSIS.md | 17 ++ .../dashboard/lib/use-action.BUG-ANALYSIS.md | 23 +++ .../lib/use-action.test.BUG-ANALYSIS.md | 19 ++ .../lib/use-agent-handoff.BUG-ANALYSIS.md | 29 +++ .../dashboard/lib/use-agents.BUG-ANALYSIS.md | 21 ++ .../lib/use-agents.test.BUG-ANALYSIS.md | 23 +++ .../dashboard/lib/use-async.BUG-ANALYSIS.md | 32 +++ .../lib/use-async.test.BUG-ANALYSIS.md | 18 ++ .../lib/use-context-set.BUG-ANALYSIS.md | 24 +++ .../lib/use-daemon-health.BUG-ANALYSIS.md | 23 +++ .../use-daemon-health.test.BUG-ANALYSIS.md | 20 ++ .../lib/use-device-status.BUG-ANALYSIS.md | 25 +++ 193 files changed, 9485 insertions(+) create mode 100644 packages/chrome-extension/background.BUG-ANALYSIS.md create mode 100644 packages/chrome-extension/check.BUG-ANALYSIS.md create mode 100644 packages/chrome-extension/content.BUG-ANALYSIS.md create mode 100644 packages/chrome-extension/options.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/App.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ActionsRunNotice.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ActionsRunNotice.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AddDeviceDialog.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AddDeviceDialog.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AddProjectPanel.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AddProjectPanel.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AgentActionBar.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AgentActionsMenu.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AgentComposer.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AgentComposer.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AgentDetails.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AgentErrorCount.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AgentErrorCount.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AgentFeed.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AgentHandoff.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AgentHandoff.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AgentHistory.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AgentHistory.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AgentOverview.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AgentView.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AgentView.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/Agents.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/Agents.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AiQueue.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AiQueue.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AnsweredChoice.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/AppFrame.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/BrandLink.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/BrandLink.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/BridgeSettings.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/BridgeSettings.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/BrowserPanel.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/BrowserPanel.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ChoicePanel.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/CloudAgentNotice.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/CloudAgentNotice.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/Composer.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/Composer.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ConnectionIndicator.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ContextFiles.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ContextFiles.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ContextMenu.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ContextMenu.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/DashboardPage.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/DevicesSettings.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/DevicesSettings.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/DiffView.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/DisclosureToggle.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/DisclosureToggle.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/DiscordDialogs.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/DiscordDialogs.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/DocsPanel.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/DriverModelMenu.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/DriverModelMenu.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/EventList.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/EventList.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/FilePreview.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/FilePreview.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/FileTree.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/FileTree.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/GitStatusBar.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/GitStatusBar.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/HotTickets.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/HotTickets.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/InlineBrowser.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/InlineBrowser.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/Logo.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/Logo.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/Markdown.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/Markdown.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/NotFound.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/NotificationsMenu.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/NotificationsMenu.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/OnboardingChecklist.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/OnboardingChecklist.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/OpenQuestions.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/OpenQuestions.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/OptionsMenu.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/OptionsMenu.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/PresetCreatePanel.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/PresetCreatePanel.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/PresetsMenu.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/PresetsMenu.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ProjectActions.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ProjectDocs.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ProjectDocs.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ProjectErrorBanner.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ProjectErrorBanner.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ProjectHome.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/PromptEditor.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/PromptEditor.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/Quota.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/Quota.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/RemoteAgentNotice.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ResolvedOptions.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ResolvedOptions.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/RightRail.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/RightRail.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/StartAgentForm.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/StartAgentForm.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/SystemPromptDisclosure.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/SystemPromptDisclosure.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ThemeToggle.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ThemeToggle.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/TicketDetailPage.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/TicketDetailPage.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/TicketFilterBar.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/TicketFilterBar.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/TicketPlanPage.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/TicketPlanPage.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/TicketsPage.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/TicketsPage.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/TicketsPanel.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/TicketsPanel.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ViewsRail.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/WorkspaceActions.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/WorkspaceActions.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/prompt-editor/SuggestionList.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/prompt-editor/suggestion.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/prompt-editor/tokenize.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/prompt-editor/tokens.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/badge.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/button.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/card.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/checkbox.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/confirm-dialog.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/copy-button.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/dialog.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/dropdown-menu.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/input.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/message-scroller.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/message-scroller.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/option-label.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/popover.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/scroll-area.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/scroll-area.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/separator.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/sheet.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/sidebar.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/skeleton.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/slider.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/components/ui/tooltip.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/design/build.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/design/previews.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/design/vite.config.css.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/agent-label.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/agent-option-rows.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/agent-option-rows.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/agent-settings.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/live-state.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/live-state.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/notification-permission.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/notify-channels.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/notify-channels.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/preferences.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/preferences.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/profiles.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/profiles.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/queue-entry.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/queue-entry.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/quota-bar.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/quota-bar.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/quota.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/quota.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/remote-target.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/resume-command.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/resume-command.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/route.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/route.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/rpc.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/session-link.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/session-link.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/status-tone.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/ticket-filter.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/ticket-filter.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/ticket-priority.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/ticket-priority.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/use-action.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/use-action.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/use-agent-handoff.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/use-agents.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/use-agents.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/use-async.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/use-async.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/use-context-set.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/use-daemon-health.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/use-daemon-health.test.BUG-ANALYSIS.md create mode 100644 packages/framework/dashboard/lib/use-device-status.BUG-ANALYSIS.md diff --git a/packages/chrome-extension/background.BUG-ANALYSIS.md b/packages/chrome-extension/background.BUG-ANALYSIS.md new file mode 100644 index 000000000..4862105ab --- /dev/null +++ b/packages/chrome-extension/background.BUG-ANALYSIS.md @@ -0,0 +1,170 @@ +# Bug analysis: packages/chrome-extension/background.js + +## Business logic (high-level) + +The extension's service worker: the only half that holds the bridge token and talks to the daemon. +Responsibilities per `background.SPEC.md`: + +- Relay content-script messages to the daemon (`tf-question` → `POST /_bridge/question`, + `tf-events` → `POST /_bridge/events`, `tf-hello` → `POST /_bridge/hello`), deduplicating + repeated questions per session in memory (`lastSent`). +- Poll `GET /_bridge/answer` per watched session (alarm every 30s + immediately after a question is + accepted), hand the text to the session's tab, and ack via `POST /_bridge/answered` — typing + strictly before acking, with `deliveredAnswers` as the in-worker claim set so an answer is never + handed to a tab twice "for as long as the service worker lives" (SPEC). Failed acks are retried + from `pendingAcks` at the start of every poll; a 400 settles an ack. +- Claim `GET /_bridge/start`, open the new-session page in a pinned inactive tab, have the content + script drive it, report `POST /_bridge/started`. SPEC: "Sessions are created one at a time … + a second request waits for the first to finish", guarded by the `creating` flag. +- Sweep `GET /_bridge/sessions` once a minute (+ on worker start + on demand from the options + page): open one pinned, inactive tab per watched session, skip dismissed sessions, close tabs it + opened for sessions no longer watched, and record every outcome under `lastOpen`. +- Bookkeeping in `chrome.storage.local`: `openedTabs` (tabId → sessionId for tabs *we* opened), + `dismissedSessionsV2` (sessions whose tab the user closed; capped at 50), `lastOpen`. + +Invariants the SPEC states and the code must uphold: +1. token never enters a page — holds: token only read here and in options. +2. version lockstep — holds: `VERSION_HEADER` on every fetch. +3. type-then-ack ordering — holds within one call path. +4. an answer already handed to a tab is not handed again while the worker lives — **violated by a + TOCTOU race, see Bugs 1**. +5. one creation at a time — **violated by a race on `creating`, see Bug 3**. +6. a user-closed tab dismisses exactly that session; extension-closed tabs never dismiss — holds + in the single-event case (`closeStaleTabs` drops the record before removing), **but concurrent + close events lose dismissals, see Bug 4**. + +Concurrency model — the central weakness. An MV3 worker woken by an alarm first evaluates the +whole script (running the top-level `void openWatchedTabs(); void pollAnswers(); void pollStarts()` +at L445-447) and then dispatches the alarm event (running the same functions again from the +listener at L437-444). So on every wake from idle, `openWatchedTabs`, `pollAnswers` and +`pollStarts` each run twice, near-simultaneously. None of these functions is re-entrant: their +guards (`deliveredAnswers`, `creating`) are checked before `await` points and set after, and all +storage bookkeeping is non-atomic read-modify-write of whole objects. Since Chrome idle-terminates +the worker after ~30s and the fastest alarm fires every 30s, wake-by-alarm (and hence the double +run) is a routine occurrence, not an edge case. The daemon side compounds this: `bridge-store.ts` +`pendingAnswer()` keeps returning the same queued answer until it is *acked* (collection does not +consume it), and `handleStart` *does* consume a start request per GET — so concurrent answer polls +both receive the same answer, and concurrent start polls claim two different requests. + +The deliberate design trade the SPEC accepts — an answer typed but unacked before worker death is +typed a second time later — is out of scope (documented cost). Everything the claim set is +supposed to prevent *within* one worker life is in scope. + +## Functions (low-level) + +- **`onMessage` listener (L22-48)** — routes `tf-open-now`, `tf-hello`, `tf-events`, + `tf-question`; returns `true` to keep the channel open; every promise chain has a `.catch` that + still calls `sendResponse`. Correct. +- **`report(question)` (L50-75)** — validates sessionId, requires token, dedupes on + `JSON.stringify([title, options, recommended])` per session, posts, remembers only successes, + then probes for an answer on the same beat. Edge cases: fingerprint omits `multi`/option flags — + irrelevant in practice since a changed question changes title/options; `lastSent` is in-memory so + a daemon restart is healed as soon as the worker idles out (~30s) and forgets. Correct. +- **`deliverAnswers(sessionIds)` (L99-146)** — per session: fetch answer, validate, find the tab by + session id in the URL, claim in `deliveredAnswers`, sendMessage, reload-and-retry only for tabs + we opened, release the claim on failure, ack. The claim check (L114) happens *before* the + `chrome.tabs.query` await (L116) while the claim insert (L121) happens after it — a re-entrancy + hole (Bug 1). The reload path waits a fixed 5s; a claude.ai page routinely takes longer to + re-inject, but the failure is acked, the claim released, and the next 30s poll retries — self- + healing, fine. `outcome` undefined → synthesized failure. Verdict: **bug found** (race). +- **`ack(base, token, body)` (L150-165)** — ok/400 settles, anything else (or throw) queues in + `pendingAcks` keyed by answer id. 401/426 acks retry forever but the set is tiny; bounded. + Correct. +- **`pollAnswers()` (L168-183)** — replays pending acks, lists sessions, delivers. Not re-entrant + but all the harm funnels into `deliverAnswers` (Bug 1). Correct in isolation. +- **`tabLoaded(tabId)` (L200-212)** — resolves on `status === 'complete'` or after 30s; listener + removed on either path; double `done()` harmless. Correct. +- **`askPage(tabId, start)` (L215-227)** — 10 tries × 2s; returns first truthy outcome; synthesizes + a failure note from the last error. Correct. +- **`pollStarts()` (L229-277)** — `creating` guard checked at entry (L230) but set only at L244, + after two awaits (storage get, claim fetch). Two overlapping invocations both pass the guard and + both *claim* a start from the daemon (the daemon dequeues per GET — bridge-endpoints.ts + `handleStart`), then drive two creations concurrently (Bug 3). Also: the function is not + try/finally around `creating = false`; an exception from `chrome.storage.local.set` (L258) or + `note()` (L275) would wedge `creating` at true — but only until the worker is idle-terminated, + so not reported. Field validation of `start`, tab cleanup on failure, and the report body are + correct. Verdict: **bug found** (race). +- **`post(path, body)` / `postEvents(msg)` (L280-305)** — straightforward authenticated POSTs with + error text capped at 200 chars. `postEvents` forwards `sessionId`/`events` unchanged (the daemon + re-validates). Correct. +- **`dismissed()` / `openedTabs()` (L325-334)** — storage reads with defaults. Correct. +- **`note(state)` (L342-345)** — records `lastOpen` with a timestamp. Shared by the tab sweep and + `pollStarts` (a creation outcome overwrites the last sweep outcome), but the options page only + reports fresh on-demand sweeps, so no user-visible harm. Correct. +- **`openWatchedTabs()` (L347-394)** — token/opt-out gates, session listing with reasons, dedupe + against open tabs by session id, dismissed skip, create pinned+inactive, record `openedTabs`, + close stale, record outcome. Three problems: (a) not re-entrant — two concurrent sweeps compute + `already` from the same pre-create `tabs.query` snapshot and both open a tab for the same + session (Bug 2); (b) `openedTabs` update is a read-modify-write of the whole object racing other + writers (folds into Bugs 2/4); (c) the comment at L350 calls the switch "opt-in" while + `autoOpen === false` makes it default-on — the SPECs disagree with each other on this (see + options.js analysis; the doc bug is filed there). A `tabs.create` failure aborts the loop before + `closeStaleTabs`, but it is reported, and the next sweep retries — acceptable. Verdict: **bug + found** (re-entrancy). +- **`closeStaleTabs(watched)` (L402-412)** — drops records first so the remove is not mistaken for + a user dismissal, then removes with errors swallowed. Ordering is right. Correct. +- **`tabs.onRemoved` listener (L419-429)** — attributes a close to a session, drops the record, + appends to the dismissed list (capped 50). Non-atomic read-modify-write: two closes at once + (window close with several pinned tabs) each read the same snapshot and the last write wins, + losing one dismissal and resurrecting the other tab's `openedTabs` entry (Bug 4). Verdict: + **bug found** (race). +- **Alarms + startup kicks (L433-447)** — alarms are the right instrument; the top-level kicks are + the "once when the service worker starts" behavior the SPEC asks for, but combined with the + alarm dispatch on the same wake they are what makes every function above run twice concurrently. + +## Bugs found + +1. `L114`/`L121`: **The same answer can be typed into the session twice while the worker lives.** + `deliveredAnswers.has(answer.id)` is checked at L114, but the claim is only inserted at L121, + after the `await chrome.tabs.query(...)` at L116. Two concurrent `deliverAnswers` runs for the + same session — which happen routinely: on every alarm wake of an idle-terminated worker the + top-level `void pollAnswers()` (L446) and the alarm handler's `pollAnswers()` (L440) run + back-to-back, and `report()` also kicks `deliverAnswers` (L73) while a poll may be in flight — + both fetch the same still-`queued` answer (the daemon's `pendingAnswer()` does not consume on + read), both pass the `has` check before either inserts, and both `sendMessage` the text, so the + content script fills and submits the composer twice. This contradicts the SPEC's explicit + guarantee ("An answer already handed to a tab is not handed to another one for as long as the + service worker lives") and the module's own comment (L94-97). Severity: major. Fix: make the + check-and-claim atomic — insert into `deliveredAnswers` synchronously right after the `has` + check (before any await) and delete the claim when no tab is found; or serialize + `deliverAnswers` behind a module-level in-flight promise. + +2. `L445`: **Duplicate pinned tabs for a watched session (and lost `openedTabs` records) from the + sweep double-running on every alarm wake.** When the `tf-sessions` alarm wakes an + idle-terminated worker, the script's top level runs `void openWatchedTabs()` (L445) and the + alarm listener runs it again (L438) a few milliseconds later. Both invocations query open tabs + (L367) before either has created one, so both see the same `already` set and both create a + pinned tab for every watched session that lacks one — the user gets two pinned tabs per new + cloud session, and `closeStaleTabs` never removes the duplicate because its session is still + watched. The interleaved `chrome.storage.local.set({openedTabs: {...(await openedTabs()), …}})` + writes (L386) also lose one tab's record (last write wins), leaving a tab the extension opened + unattributed: it is never auto-closed, and the user closing it no longer dismisses the session. + Severity: major. Fix: make `openWatchedTabs` non-reentrant (return the in-flight promise when + one exists), which also covers the top-level-plus-message (`tf-open-now`) overlap. + +3. `L230`/`L244`: **Two session creations can run concurrently, violating the serial-creation + contract.** `pollStarts` checks `creating` at entry (L230) but sets it only at L244, after + `await chrome.storage.local.get` and the claiming `await fetch('/_bridge/start')`. The + top-level kick (L447) and the alarm handler (L442) run `pollStarts` twice on every wake; both + pass the guard, and since the daemon dequeues one start request per GET, each claims a + *different* request when two or more are queued (exactly the fan-out case: several web runs + started together) and both drive new-session tabs at the same time — contrary to the SPEC + ("Sessions are created one at a time … a second request waits for the first to finish") and the + code's own rationale (L188-189). Whichever finishes first also resets `creating` while the + other is still driving its page. Their concurrent `openedTabs` writes (L258) can additionally + lose a record as in Bug 2. Severity: minor (each creation drives its own tab, so the damage is + spec-contract, bookkeeping, and unthrottled parallel tab driving rather than a wrong session). + Fix: set `creating = true` synchronously right after the entry check (before any await) and + reset it on every early return. + +4. `L419-L429`: **Closing several extension-opened tabs at once loses dismissals, so a + user-closed session's tab is reopened.** The `onRemoved` handler does a non-atomic + read-modify-write of both `openedTabs` and `dismissedSessionsV2`. When the user closes a window + holding two pinned session tabs (or hits "close other tabs"), both handlers read the same + pre-close snapshot; the second `chrome.storage.local.set` overwrites the first, so one + session's dismissal is dropped and the other closed tab's `openedTabs` entry is resurrected. + The next sweep sees the un-dismissed session with no open tab and reopens it — exactly the + user-hostile behavior the SPEC rules out ("A tab the user closed is not reopened"). Severity: + minor. Fix: serialize the storage updates (a module-level promise queue shared by every + `openedTabs`/`dismissedSessionsV2` writer), or key the records individually instead of as one + object. diff --git a/packages/chrome-extension/check.BUG-ANALYSIS.md b/packages/chrome-extension/check.BUG-ANALYSIS.md new file mode 100644 index 000000000..a34a9b217 --- /dev/null +++ b/packages/chrome-extension/check.BUG-ANALYSIS.md @@ -0,0 +1,88 @@ +# Bug analysis: packages/chrome-extension/check.mjs + +## Business logic (high-level) + +Offline harness for the content script: runs `content.js` inside jsdom pages built to look like +claude.ai session and new-session pages, with no browser and no extension runtime (`chrome` is +undefined, so the script exposes `__tfBridgeQuestion`, `__tfBridgeTranscript`, +`__tfBridgeDeliverAnswer`, `__tfBridgeCreateSession`, `__tfBridgeProbeNewSession`). Exit code 1 +when any case fails. What it pins, per `check.SPEC.md`: + +- **Extraction** (14 table cases): the block found in fenced code, `pre` without `code`, bare + `code`, wide indentation, highlighter-split spans, prose-surrounded, inside a shadow root; a + no-question page reports none; the protocol spec block loses to a real question and never wins + alone; the two literal examples (browser-handoff, "Ship this?") never count; decoys inside the + opening turn (`data-index="0"`) are ignored while a later real question wins; a question-shaped + block only in the opening turn is never reported. Each case also asserts the composer was found + and (for positive cases) that the panel shows the real title — so the assertions genuinely + check the panel output, not just absence of a crash. +- **Wire shape**: `multi`, per-option `default`/`stop`, labels and details reach + `__tfBridgeQuestion` exactly, unknown keys dropped, sessionId from the URL. +- **Mirror**: one entry per turn keyed by the page's `data-index`, roles mapped, markers skipped, + glyphs/blank lines removed, the opening prompt head-capped at 8000; tail-only rendering keeps + page positions; a page with no turn rows mirrors nothing and the panel says so. +- **Delivery**: composer filled and send button clicked (verified via a real click listener and + the composer's text content); Enter fallback verified via a keydown listener; a composer-less + page is refused with the reason (composer wait shortened via `__tfComposerWaitMs`). +- **Creation**: a synthetic new-session page (combobox chips repo→branch→add, searchable pickers + as dialogs, send button that pushes a session URL) exercised in the three remembered states, + with glyph-decorated labels, a missing branch (nothing sent, branch named), a missing repo + picker (named, probe describes controls untouched). +- **Panel fold**: folds to a "TF" tab (title in tooltip, `aria-expanded=false`) and restores. + +Do the tests verify what they claim? Yes — each case asserts on observable effects (panel text, +exposed wire objects, DOM state, listener flags, result notes) and each `ok` feeds the `failed` +counter and the process exit code. The panel-text probes are regex-based over concatenated +`textContent` (`/question found\s*yes/` etc.); the key/value spans concatenate without +whitespace, which `\s*` tolerates, and a false positive would require those phrases to appear +elsewhere in the panel, which they do not. The harness deliberately does not cover the live +page's real DOM (stated in its SPEC) nor the worker half — background.js is entirely untested +here, which is where the real races live (see background.BUG-ANALYSIS.md), but that is a scope +decision recorded in the SPEC, not a defect of this file. + +## Functions (low-level) + +- **jsdom resolution (L18-19)** — resolves `jsdom` through `../framework`'s dependencies since + this directory has no package.json; fails loudly if the repo install is missing. Correct. +- **Fixture builders (L21-67)** — `block`/`wideBlock`/`highlighted` (escaping `<` in the + highlighted variant), `esc` for the spec/example blocks (escapes `<`/`>` so placeholders + survive innerHTML), `row`/`feed` mirroring claude.ai's `transcript-row` markup. `block` itself + is inserted un-escaped, but it contains no `<`, so nothing is lost. Correct. +- **Case table + runner (L69-130)** — builds each page (shadow case via `attachShadow`), evals the + script, reads the panel from `documentElement`'s direct `div` children, computes + `found`/`composerOk`/`titleOk`, counts failures. `titleOk` is only enforced for positive cases — + intentional, negative pages have no title to show. One subtlety: `dom.window.eval(script)` runs + the top-level panel render synchronously, so no waiting is needed; the mutation observer and the + 60s heartbeat are torn down by `dom.window.close()`. Correct. +- **Wire-shape block (L136-162)** — feeds a shaped block in turn 1 (with turn 0 as intro so the + opening-turn exclusion is active) and compares `__tfBridgeQuestion` to the exact expected JSON, + including key order via `JSON.stringify` — stable because both objects are built in source + order. Correct. +- **`mirrorOf(body)` (L169-179)** — evals, grabs `__tfBridgeTranscript` and panel text, closes the + dom. Correct. +- **Mirror cases (L181-217)** — expected prompt text is `prompt.trim().slice(0, 8000)`: matches + `cleanText` (single line trimmed, then head-capped). The ` Copy` line pins glyph + stripping and line-trimming. Tail-rendering and no-rows cases assert positions and the named + panel state. Correct. +- **`deliver(body, prepare)` + delivery cases (L225-279)** — jsdom has no `execCommand`, so these + exercise the fallback fill; the click case additionally asserts the composer text equals the + answer, and the no-composer case shortens the wait. `deliver` returns the dom for the caller to + close; every caller does. Correct. +- **Collapse case (L287-309)** — finds the toggle by `aria-expanded` (the other panel buttons + carry none), asserts expanded → folded (title gone, "TF" present, aria state) → restored. + Correct. +- **`newSessionPage(opts)` (L318-373)** — synthetic new-session page: remembered chip or select + trigger, dialogs with `role=option` entries removed on pick, branch chip created after a repo + pick, send handler pushes `/code/session_01NEW`, waits shortened via the `__tf*Ms` knobs. + `seen.searched` records search input values. Entries stay in the DOM until picked, mirroring + the live page's closed-picker behavior closely enough for `usable` (no `checkVisibility` in + jsdom — treated as visible, which the content script documents as the intended degradation). + Correct. +- **Creation cases (L377-446)** — remembered/other/none remembered, glyph labels, wrong branch + (asserts `!seen.sent`), missing picker + probe. The wrong-branch case is the safety-critical + pin: nothing is sent when the branch list lacks the ref. Correct. +- **Exit (L448-449)** — prints and exits non-zero on failure. Correct. + +## Bugs found + +None found. diff --git a/packages/chrome-extension/content.BUG-ANALYSIS.md b/packages/chrome-extension/content.BUG-ANALYSIS.md new file mode 100644 index 000000000..640984515 --- /dev/null +++ b/packages/chrome-extension/content.BUG-ANALYSIS.md @@ -0,0 +1,168 @@ +# Bug analysis: packages/chrome-extension/content.js + +## Business logic (high-level) + +The page half of the bridge, injected into every `https://claude.ai/*` page and frame. It: + +- extracts the parked question (a JSON object carrying `options`) from code elements — shadow + roots included — falling back to whole-page text, discards the protocol's own decoys (anything + in the opening turn, placeholder titles/labels, the two literal examples), takes the last + survivor in page order, and reports it to the worker keyed by the session id from the URL; +- mirrors the transcript one `transcript-row` at a time under the page's own `data-index`, + re-sending only turns whose text changed and remembering only what the daemon accepted; +- on `tf-deliver-answer` (top frame only) waits up to 20s for a composer, fills it, waits 400ms, + submits via the last enabled send button or synthetic Enter, and reports which path it used; +- on `tf-create-session` drives the new-session page: repo chip/picker, branch chip (verified to + read the requested branch before anything is sent), prompt, send, then reads the session id from + the URL; +- draws a diagnostic panel (top frame) with fold state kept in extension storage; child frames post + their surveys to the top frame instead; +- re-surveys on DOM mutations (debounced 250ms) plus a 60s heartbeat, and shuts down when its + extension context dies. + +Invariants: no token or daemon fetch here (holds — everything goes through +`chrome.runtime.sendMessage`); only the top frame delivers/creates (holds — listener gated on +`IS_TOP`); the typed text comes only from the daemon (holds — `deliverAnswer` types +`message.text` verbatim; but see Bug 1: it can end up *appended to* pre-existing user text, so +what is submitted is not only the daemon's text); reports are trimmed "to what the daemon +accepts" (mostly holds — title/detail/option-count yes, option labels and `recommended` no, +Bug 3; event batches no, Bug 2). + +Concurrency/ordering: the mutation observer, the 60s heartbeat, and the panel's re-renders all +funnel through `survey()` → `findPendingChoice()` → `reportToDaemon()`/`reportTranscript()`; +duplicates are cheap because the worker and daemon both dedupe. `sentEvents` is keyed by `seq` +only, not by session — an SPA navigation to a different session in the same tab reuses the map, +but a colliding (same seq, same text) turn across sessions is not realistic, and the daemon keys +by session anyway; noted, not a bug. The `message` listener for child-frame surveys accepts any +origin and posts with `'*'` — a hostile iframe inside claude.ai could spoof the panel display and +a hostile embedder could read a child frame's survey, but claude.ai does not host third-party +frames and is not embeddable in practice, the data never reaches the daemon (child frame URLs +carry no session id), and rendering uses `textContent` (no XSS); noted, not reported. + +## Functions (low-level) + +- **`sessionIdFromUrl()` (L24-27)** — regex over `location.href`; child frames naturally yield + nothing, which is what keeps them read-only reporters. Correct. +- **`reportToDaemon(parsed)` (L39-84)** — builds the wire shape: title sliced to 500, options + mapped (string or object), detail sliced to 500, `default`/`stop` only when `=== true`, + label-less options dropped, capped at 20; drops the whole report when no title/options. Gaps: + option `label` is *not* sliced to the daemon's 300-char `MAX_LABEL` and `recommended` is not + sliced/validated against the sent labels, so an out-of-bounds label or a `recommended` that + names a dropped/26th option makes the daemon 400 the entire question forever (Bug 3). Records + the worker's verdict in `bridgeStatus` for the panel. Verdict: **bug found** (minor). +- **`transcript()` / `turnRows()` / `openingMessage()` (L105-129)** — rows by + `data-testid="transcript-row"`, role map human/assistant, integer non-negative `seq`, text + cleaned and sliced to 8000 (matches daemon `MAX_EVENT_TEXT`). `seq > 10_000` would be rejected + daemon-side but a 10k-turn session is not a real input. Correct. +- **`cleanText(text)` (L135-143)** — strips private-use glyphs, trims lines, drops blanks. Correct. +- **`sayHello(sessionId, note)` (L152-160)** — best-effort self-report; reads `lastError` to keep + Chrome quiet. Correct. +- **`reportTranscript()` (L167-206)** — sends only changed turns; remembers only on daemon accept, + so refused batches retry. Flaw: the changed set is sent as **one** batch with no chunking while + the daemon rejects batches over `MAX_EVENT_BATCH = 50` entries — a first pass over a page with + more than 50 rendered turn rows would 400 forever, since nothing is remembered and the batch + never shrinks (Bug 2). `sayHello` reports the *previous* pass's `transcriptStatus`, which is + what the SPEC says ("the outcome of the last transcript report"). Verdict: **bug found** + (low confidence on reachability). +- **`deepQueryAll` / `countShadowRoots` / `deepText` (L209-237)** — recursive shadow traversal + with a `seen` set; `deepText` may double-count slotted text across nested roots, which only + inflates diagnostics counters. Ordering caveat: shadow-root matches are appended after their + host root's light-DOM matches, so "last in page order" is approximate across mixed light/shadow + transcripts; on the real page all messages render in one container, so the tiebreak holds. + Correct. +- **`isTemplate(parsed)` (L252-266)** — placeholder title, punctuation-joined placeholders (letters + check is Unicode-aware), all-placeholder labels, and the two literal examples. The literal + match uses labels joined with `|`; a label containing `|` could in principle collide, but the + literal sets are fixed strings. Correct. +- **`inFirstMessage(el, first)` (L275-282)** — walks `parentNode ?? host` so shadowed highlighters + resolve to their turn row. Correct. +- **`extractChoices` / `collectChoices` (L285-350)** — brace matcher tracking strings and escapes; + on a parse success skips past the object (`i = j`), on failure rescans from the next `{` so + nested candidates are still found; stats separate "never found" from "found, unparseable". + Quadratic worst case on pathological text — performance, not correctness. Correct. +- **`findPendingChoice()` (L357-386)** — element-scoped scan excluding the opening turn, page-text + fallback with by-value subtraction of prompt blocks, template filter, last survivor wins, then + reports question and transcript. The by-value subtraction can drop a real question identical to + a prompt block — the SPEC accepts this cost explicitly. Correct. +- **`findComposer()` (L389-395)** — first contenteditable, else first textarea. Correct. +- **`fillComposer(composer, text)` (L401-421)** — textarea path *replaces* (`value = text`); + contenteditable path focuses and `execCommand('insertText')`, which *inserts at the selection* + without clearing existing content, falling back to `textContent = text` (which does replace). + So the two supported paths disagree about pre-existing content, and the primary live path + (execCommand on claude.ai's rich editor) appends to any draft already in the composer (Bug 1). + Verdict: **bug found**. +- **`findSendButton()` (L424-430)** — last enabled `aria-label*=send` button, else last + `button[type=submit]` (that fallback skips the disabled check; acceptable guesswork the SPEC + endorses). Correct. +- **`deliverAnswer(text)` (L440-470)** — waits up to 20s for a composer, fills, 400ms settle, + button else synthetic Enter; honest failure when no composer. It reports `ok: true` without + verifying the message actually posted — if a rich editor ignored both fill paths the send + button stays disabled, the Enter fallback fires into an empty composer, and the daemon marks + the question resolved though nothing was typed. The SPEC only requires reporting *which path + was used*, and the live page accepts `execCommand`, so: suspicious-but-unproven, not filed. +- **`controlText` / `usable` / `menuTriggers` / `chips` / `menuEntries` / `pickerSearch` / + `typeInto` / `waitFor` (L487-555)** — visibility-aware control discovery; `checkVisibility` + absent (jsdom) counts as visible by design; `typeInto` uses the native value setter for React. + Correct. +- **`chooseFrom(trigger, wanted, hint)` (L566-600)** — opens, waits for entries, exact-match on + lower-cased glyph-stripped text, search-box filter with clear-and-rescan fallback, Escape and a + rich diagnostic on failure; never clicks the trigger or its descendants as an entry. Correct. +- **`probeNewSession()` (L607-619)** — read-only description, capped. Correct. +- **`createSession({repo, branch, prompt})` (L632-691)** — the three remembered-repo states, late- + render re-read, chip verification after picking, branch verified to read the requested ref + before anything is sent (a mistaken chip — e.g. `chips()[1]` catching the "add repository" chip + before the branch chip renders — fails safe through `readsBranch`), send, session id from URL + with 60s wait. Failure notes name the missing control. Correct. +- **runtime `onMessage` listener (L695-713)** — top frame only; probe answered synchronously; + create/deliver async with catch-to-failure. Correct. +- **offline exports (L718-722)** — only when `chrome` is undefined; nothing leaks onto the real + page. Correct. +- **`diagnostics()` / `survey()` (L728-782)** — structure-only counters, no message text; matches + the SPEC's paste-into-an-issue promise. Correct. +- **child-frame sender / top-frame panel (L786-939)** — child frames post surveys up (only when + they saw something), top frame renders panel rows with `textContent`, fold state in extension + storage restored asynchronously, "Fill composer (does not send)" fills without submitting (same + append caveat as Bug 1, but the button is explicitly a non-sending probe, so harmless). + Correct. +- **`watch(run)` (L949-977)** — 250ms-debounced MutationObserver plus 60s heartbeat; `alive()` + disconnects both when the extension context dies, wrapped against `chrome.runtime` itself + throwing. Correct. + +## Bugs found + +1. `L407-L419`: **A delivered answer is appended to whatever the user already typed, and the + combined text is submitted.** The contenteditable fill path focuses the editor and uses + `document.execCommand('insertText')`, which inserts at the current selection without clearing + existing content (the textarea path, by contrast, replaces via `value = text`). Scenario: the + user starts typing a message to the cloud session in its (watched or self-opened) tab, walks + away, then answers the parked question from the dashboard; `deliverAnswer` inserts the + daemon-composed text into the draft and submits draft+answer as one message. That contradicts + the SPEC's core promise that "the extension never speaks for me beyond the option I picked" / + "what is typed … is built from what the session offered, never free text from the browser". + Severity: minor (needs a pre-existing draft, but that is exactly the state the worker's + never-reload-user-tabs rule anticipates). Fix: clear or select the editor's content before + inserting — e.g. `execCommand('selectAll')` (or a Range over `composer.el`) ahead of + `insertText`, mirroring the replace semantics of the textarea path. + +2. `L184-L202`: **A changed-turn set larger than 50 is sent as one batch, which the daemon + rejects forever.** `reportTranscript` sends every changed turn in a single `tf-events` message + and the worker forwards it as one `POST /_bridge/events`; the daemon rejects any batch over + `MAX_EVENT_BATCH = 50` (bridge-endpoints.ts L309) and rejects it wholesale. Because nothing is + remembered on rejection, the identical oversized batch is re-sent on every mutation and the + session's transcript never mirrors at all — the panel shows the 400 but the failure is + permanent. Trigger: a session page whose virtualized feed renders more than 50 turn rows on + first survey (long transcript in a tall window). Severity: minor. Confidence: low (whether + claude.ai ever renders >50 rows at once is unverified). Fix: chunk `events` into slices of at + most 50 before sending (or have the worker split them). + +3. `L53`/`L61`: **Option labels and `recommended` are not trimmed to what the daemon accepts, so + one oversized or dangling field kills the whole question.** The daemon requires each label + ≤300 chars and `recommended` to match a *sent* label; `reportToDaemon` slices title (500) and + detail (500) but sends labels and `recommended` untrimmed, and the 20-option cap can drop the + very option `recommended` names. Scenario: an agent emits a question whose recommended label is + over 300 characters (or sits past the 20th option) — `POST /_bridge/question` answers 400 and, + since failures are never remembered, every DOM change retries the same rejected payload; the + question never reaches the dashboard, contradicting this file's SPEC ("It is trimmed to what + the daemon accepts before being sent"). Severity: minor. Confidence: low (agent-authored labels + are normally short). Fix: slice labels to 300 in the option mapping, and drop `recommended` + when it does not match one of the labels that survived trimming. diff --git a/packages/chrome-extension/options.BUG-ANALYSIS.md b/packages/chrome-extension/options.BUG-ANALYSIS.md new file mode 100644 index 000000000..716d33dcf --- /dev/null +++ b/packages/chrome-extension/options.BUG-ANALYSIS.md @@ -0,0 +1,83 @@ +# Bug analysis: packages/chrome-extension/options.js + +## Business logic (high-level) + +The extension's settings page: loads/stores the three settings (`daemonUrl` — default +`http://localhost:4200`, trailing slashes stripped; `token`; `autoOpen` — checkbox default +checked, written out explicitly on save), and proves the connection immediately after every save +by walking the diagnosis ladder from `options.SPEC.md`: + +1. host grants actually held (`chrome.permissions.contains` for the daemon origin and claude.ai) — + checked first because a missing grant is otherwise indistinguishable from a wrong token; +2. 401 → token rejected; 3. 404 → bridge switched off; 4. 426 → daemon's version-skew refusal + relayed verbatim; 5. other non-ok → status shown; 6. 200 with a body other than `ok` → a + dashboard build with no bridge route (the SPA answers 200 HTML for unknown paths); +7. fetch throw → dashboard unreachable. Only a clean `ok` counts as connected, and success goes + on to list `/_bridge/sessions` and say whether tabs will open (honoring the checkbox state + just saved). + +A second button asks the worker (`tf-open-now`) to run the tab sweep immediately and renders the +worker's structured result: did-nothing reason, nothing-to-open with skip reasons, or +opened-X-of-Y. A worker that never answers (`lastError` / no reply) is named with the reload fix. + +Ordering concern: settings are saved *before* the test, so even a failing test leaves the values +stored — intended ("Saved. Testing…"). The save handler is not guarded against double clicks, but +a second run is idempotent (re-save, re-test). The version header is built per save from the +manifest. All failure text lands in `#status` via `textContent` — no injection surface. + +One cross-document inconsistency worth recording: `packages/chrome-extension/SPEC.md` calls tab +opening "(opt-in)" ("Opening tabs is opt-in from the options page"), and background.js L350 +comments "Opt-in: opening tabs on someone's behalf should be asked for, not assumed" — but this +file (checkbox defaults checked, `autoOpen !== false`), `options.SPEC.md` ("on unless the user +turns it off"), and background.js's actual guard (`autoOpen === false`) all implement default-on, +i.e. opt-out. Until the user first opens this page nothing has "asked", yet the worker opens tabs. +Two intent sources against two; the behavior is coherent and useful either way, but the documents +contradict each other (Bug 1). + +## Functions (low-level) + +- **Initial load (L10-15)** — `chrome.storage.local.get(...).then(...)`: populates the inputs; + `autoOpenEl.checked = autoOpen !== false` (unset ⇒ checked). Promise-form storage API is fine + under MV3. Race with the user editing before load resolves is theoretical (load is ~ms). + Correct. +- **`say(message, isError)` (L17-20)** — status line with error styling via class. Correct. +- **`missingHosts(daemonUrl)` (L31-39)** — checks `${daemonUrl}/*` and `https://claude.ai/*` via + `chrome.permissions.contains`, treating an API error as "missing". Origin patterns with an + explicit port (`http://localhost:4200/*`) are valid Chromium match patterns and are contained + by the manifest's port-less `http://localhost/*` grant, so the default URL passes. A daemon URL + on a non-declared host (e.g. `http://192.168.1.5:4200` for a remote dashboard) is correctly + reported as ungranted — though the advice ("switch those on under Site access") cannot succeed + there because the origin is not declared in the manifest at all; that is the documented + localhost-only scope of the extension, so a doc-accurate dead end, not a bug. A URL pasted with + a path (`http://host:4200/dash`) yields pattern `http://host:4200/dash/*`, still contained by a + host-wide grant. Correct. +- **Save handler (L41-88)** — trims the token (refuses empty), normalizes the URL, stores all + three keys explicitly, then runs the ladder exactly as the SPEC orders it. `res.text()` is read + once per branch (426 branch reads it, `body !== 'ok'` branch reads it separately on a different + path) — no double-read of one response. The sessions probe tolerates its own failure with + "Could not list sessions." while still reporting connected — matches the SPEC's "one step + further" framing. Uses `autoOpenEl.checked` (the just-saved value) for the tabs message. + Correct. +- **Open-now handler (L93-102)** — sends `tf-open-now`; distinguishes lastError / no reply / + `ok:false` (reason) / `opened===0` (reason and skip list) / success ("Opened X of Y"). The + worker's `closed` count is not surfaced, and the stored `lastOpen` record is never displayed — + the button always triggers a fresh sweep instead, which satisfies the SPEC's user story; + noted, not a bug. Correct. + +## Bugs found + +1. `L14` (with `packages/chrome-extension/SPEC.md` and background.js L350-351): **The specs + contradict each other — and the code comment contradicts the code — on whether tab opening is + opt-in.** The directory SPEC says "the extension keeps one pinned, inactive tab per session + (opt-in)" / "Opening tabs is opt-in from the options page", and background.js's guard is + commented "Opt-in: opening tabs … should be asked for, not assumed"; but the implemented + behavior everywhere (this file's `autoOpen !== false` default-checked checkbox, + `options.SPEC.md`'s "on unless the user turns it off", background.js's `autoOpen === false` + early-return) is default-on before the user has ever been asked. Scenario: a user installs the + extension, configures nothing, has a token-less setup — no tabs (token gate) — but the moment + a token is saved with the checkbox untouched, tabs open although per the directory SPEC they + only should after an explicit opt-in. Whichever way it is resolved, one intent source is + wrong. Severity: minor. Fix: pick the implemented semantics (default-on, switchable off) and + reword `packages/chrome-extension/SPEC.md`'s two "opt-in" phrases and the background.js L350 + comment to match — or, if opt-in is truly intended, flip the defaults here (unchecked) and in + background.js (`autoOpen !== true` gate). diff --git a/packages/framework/dashboard/App.BUG-ANALYSIS.md b/packages/framework/dashboard/App.BUG-ANALYSIS.md new file mode 100644 index 000000000..a8dfa811e --- /dev/null +++ b/packages/framework/dashboard/App.BUG-ANALYSIS.md @@ -0,0 +1,119 @@ +# Bug analysis: packages/framework/dashboard/App.tsx + +## Business logic (high-level) + +The dashboard shell. Responsibilities per `App.SPEC.md`: + +- **URL = selection**: `useRoute()` parses `/`, `/{projectId}`, `/{projectId}/{agentId}`, + `/settings`, `/tickets`, `/{projectId}/tickets/{slug}[/plan]`; every navigation helper + (`selectAgent`, `selectProject`, `selectAgentInProject`, `newAgentInProject`, `showDashboard`, + `showSettings`, `showTickets`, `openTicket`, `openTicketPlan`) is a history push and also ends + the adopt-follow. Verified against `lib/route.ts`: `settings`/`tickets` cannot collide with + project ids, so the route branches in `renderMain` are exhaustive and ordered correctly + (plan → detail → list → overview → unknown project → home/adopting → missing agent → agent). +- **Just-started agent**: `agentStart` {tick, intent, id, runsOn} drives the optimistic sidebar + row and the live `AgentView` before `agent.json` exists; a continuation of the agent already on + screen keeps the tick so the transcript is not blanked. "Gone" is only shown for an agent absent + from a list that was actually read (`agentsLoaded` from `useAgents`) and that is not the + just-started id — matches the SPEC's three-way distinction. +- **No-worktree fallback**: `adopting` follows live output and adopts the running agent with a + `replace` navigation once the poll surfaces it; every explicit selection clears it. Gap: it is + not scoped to the project it started in (Bug 2). +- **Per-project context set**: reset on `projectId` change (covers Back/Forward, keyed off the + route), on start (`agentStarted`), and on sidebar "New" in the same project (`newAgentInProject` + resets explicitly because the project-change effect would not fire). Complete. +- **Shared polls**: files (10s, scoped to project+agent), interventions (15s, unconditional — + feeds badge, Overview card, title, notifier), projects (30s, reloadable via `projectsKey`), + activity (15s, polled only while its notification could fire — matches SPEC), recents (10s, + Overview only). Notifications gated on category+browser prefs; the notifier receives the whole + `ProjectionRead` so unreachable projects are not mistaken for all-new (#1625). +- **Tab title/icon**: `useDocumentTitle(interventions.length, projectName)`, `useFavicon(working)`. +- **Dead daemon banner**: `useDaemonHealth()` gates a `role="alert"` banner. Matches SPEC. +- **Dead ends**: unknown project → `NotFound` (guarded by `projects.length > 0` — Bug 1); gone + agent → `NotFound` with a way back; `onDeleted` returns to the project home and reloads the + rail. + +Layout: one `SidebarProvider` frame; `AgentHistory` sidebar; main pane; `RightRail` on every +view except tickets. Note: the inline comment at L415-417 claims Settings has no rail either +("the way Settings takes the whole main pane with none either"), but the code renders the rail on +`/settings` and the SPEC backs the code ("the tickets page drops the right rail entirely" — only +tickets) — a stale comment, recorded here rather than filed as a bug since behavior follows the +SPEC. `docsInMain` is computed exactly for the ProjectHome case (project, no agent, not adopting, +not unknown), mirroring `renderMain`'s branch — the `view !== 'settings'` term is redundant +(settings always has `projectId === null`) but harmless. + +## Functions (low-level) + +- **`App()`** — everything below is inside it; hooks are called unconditionally in a stable + order (no conditional hooks — the conditionality lives in `load: null` / ternaries). Correct. +- **`agentStart` state + `agentStarted(inProject, intent, startedId, runsOn)` (L70, L150-166)** — + continuation detection compares both id and project; tick bump only for non-continuations + (prevents the #762 transcript blanking); `runsOn` only kept when passed (a later plain start + drops the previous `runsOn` — correct, the spread omits it); `setAdopting(startedId === + undefined)`; context reset; `go` (no-op when already there — relevant for continuations, whose + URL is unchanged, so no history spam); `reload()` so the real row lands fast. Edge: `go` with + `agentId: startedId ?? null` before the row exists is exactly what the `agentStart.id` check in + `renderMain` covers. Correct. +- **context reset effect (L87-91)** — depends on `projectId` only; `resetContext` is stable + enough in practice (fresh closure each render, deliberately excluded from deps). Runs on mount + too (resets an empty set — harmless). Correct. +- **files poll (L97-102)** — `null` load when no project; deps `[projectId, agentId]` match the + closure. Correct. +- **interventions/projects/activity/recents polls (L107-146)** — deps match closures + (`[projectsKey]`, `[browserActivity]`, `[projectId]`); stable initials avoid churn. Note: + bumping `projectsKey` resets `projects` to the empty initial until the re-read lands + (keepPrevious is not set), so the sidebar's project list and any `unknownProject` verdict blank + for a beat after "project added" — cosmetic flicker, noted not filed. Correct. +- **`unknownProject` (L126)** — requires a non-empty projects list before declaring a project + unknown. Wrong discriminator: emptiness conflates "not read yet" with "read and there are no + projects" (Bug 1). Verdict: **bug found**. +- **adopt effect (L173-181)** — fires whenever `adopting` with any running agent in the + *currently routed* project; replace-navigation so no extra history entry. Not scoped to the + project the fallback started in (Bug 2). Verdict: **bug found** (edge). +- **navigation helpers (L185-246)** — each clears `adopting` and pushes a route; `openTicket`/ + `openTicketPlan` carry slug/plan; `newAgentInProject` resets context explicitly. Correct. +- **stream ownership (L252-258)** — `useLiveEvents(projectId, agentId, agentStart.tick)` opened + once here and handed to both panes; `currentAgentEvents` scopes the rail's views to the newest + session segment while the feed keeps the whole journal — matches the SPEC's resume-appends + intent. Correct. +- **`renderMain()` (L276-367)** — branch order analyzed above. The gone-agent branch renders only + when `agentsLoaded && agentId !== agentStart.id` — so a bookmarked link never flashes "gone" + during the first read, per SPEC. The finished/live branch derives `live` from + `selectedAgent.status === 'running'` so the frame is never rebuilt across the transition + (same `AgentView` element position either way). `remoteLabel` for the just-started case is + gated on `agentId === agentStart.id` so a *different* not-yet-listed agent (list not read yet) + does not borrow the started one's device label. Correct. +- **render tree (L369-435)** — daemon banner; `relative` on the workspace row (the #904 + sr-only/phantom-scrollbar fix) with the reasoning preserved in the comment; sidebar props wire + the optimistic row (`startTick`/`startIntent`), follow flag, working mark, intervention count; + `RightRail` gets `hasBrowser` only for a running agent with a `browserStreamPort`. Correct. + +## Bugs found + +1. `L126`: **With zero registered projects, a URL naming a project renders that ghost project's + home instead of the "No such project" page.** `unknownProject` demands `projects.length > 0`, + using non-emptiness as the proxy for "the projects have been read" — but on a machine with no + projects registered the poll legitimately answers `[]`, the proxy never turns true, and + `renderMain` falls through to `ProjectHome` for a project the daemon does not know (its reads + fail or return nothing forever). The NotFound copy itself names the trigger — "the link may be + from another machine" — which is precisely the fresh-machine, zero-project case. The SPEC pins + the intent on read-completion ("only once the projects have actually been read"), and + `usePolled` already exposes the `loaded` flag built for exactly this distinction (its doc cites + the same #784 concern), which App.tsx uses for agents but not projects. Severity: minor. + Fix: destructure `loaded` from the projects poll and gate on it — `projectId !== null && + projectsLoaded && !projects.some(...)` (add `keepPrevious` or drop `loaded`'s reset if the + `projectsKey` bump would otherwise flash the home view for a beat). + +2. `L173-L181`: **The adopt-follow can hijack a Back/Forward navigation into a different project.** + `adopting` is cleared only by explicit selections (clicks); a popstate navigation does not + clear it, and the adopt effect matches any running agent in whatever project the URL now names. + Scenario: Start on a no-worktree project A (from Settings or Overview, `adopting = true`), + then press Back before A's poll surfaces the running agent, landing on previously visited + project B; B's agents load, one is running, and the effect replace-navigates to that agent — + the user is bounced off B's launcher onto an unrelated agent page and the history entry they + backed into is overwritten. The SPEC allows the one-agent inference only "in the no-worktree + fallback" project where the daemon's busy guard makes it safe; in project B it is a guess. + Severity: minor. Confidence: medium (needs the Back to happen inside the adoption window). + Fix: store the project alongside the flag (`setAdopting` → `{ projectId: inProject }`) and + have the effect (and the `renderMain` adopting branch) act only when the routed project + matches, clearing the flag otherwise. diff --git a/packages/framework/dashboard/components/ActionsRunNotice.BUG-ANALYSIS.md b/packages/framework/dashboard/components/ActionsRunNotice.BUG-ANALYSIS.md new file mode 100644 index 000000000..b12b0ad61 --- /dev/null +++ b/packages/framework/dashboard/components/ActionsRunNotice.BUG-ANALYSIS.md @@ -0,0 +1,34 @@ +# Bug analysis: packages/framework/dashboard/components/ActionsRunNotice.tsx + +## Business logic (high-level) + +A one-purpose banner for agents on the `actions` run target: because a GitHub Actions run replays +its whole transcript in one burst at the end, the live feed looks stalled, so this says the wait +is expected ("updates arrive when the run finishes.") while `live`, drops that clause once +finished, and links out to the Actions run once its URL is known. For every other target +(`local`, `remote`, `web`, or unset) it renders `null`, so `AgentView` can mount it +unconditionally — the whole gate is the first line. + +The URL comes from `actionsRunUrl(events)` (lib/live-state.ts L174-182): the last driver +`action` event whose label matches `^run (https?:\/\/\S+)$`. Last-match-wins means a resumed +agent that produced a second run points at the newest one; absence before the driver reports the +run renders no link, which the SPEC calls out. The events prop is the same shared stream the +shell owns, so a tab opened mid-run still finds the URL in the replay. + +Edge cases: an empty events array → no link (correct); malformed labels are filtered by the +anchored regex; the link opens in a new tab with `rel="noreferrer"` (no opener leak); +`role="status"` makes the notice announce politely. There is no state, effect, or subscription — +nothing to leak and no ordering to get wrong. + +## Functions (low-level) + +- **`ActionsRunNotice({ target, events, live })`** — gate on `target !== 'actions'` → `null`; + derive `url`; render the sentence with the live-only clause; conditionally render the anchor. + Inputs: `target` optional union, `events` readonly array, `live` boolean. Output: `null` or a + status row. Edge analysis: `live` toggling from true to false mid-mount simply re-renders the + shorter sentence (no remount, matching the app's live-and-finished-look-the-same rule); a URL + arriving in a later event batch re-renders the link in place. Verdict: correct. + +## Bugs found + +None found. diff --git a/packages/framework/dashboard/components/ActionsRunNotice.test.BUG-ANALYSIS.md b/packages/framework/dashboard/components/ActionsRunNotice.test.BUG-ANALYSIS.md new file mode 100644 index 000000000..a1fa93c84 --- /dev/null +++ b/packages/framework/dashboard/components/ActionsRunNotice.test.BUG-ANALYSIS.md @@ -0,0 +1,44 @@ +# Bug analysis: packages/framework/dashboard/components/ActionsRunNotice.test.tsx + +## Business logic (high-level) + +Pins the four behaviors its SPEC names: (1) a live `actions` agent shows the +"updates arrive when the run finishes" line; (2) the link appears once a driver `action` event +labelled `run ` is in the stream, and (3) is absent before then; (4) a finished agent drops +the updates line but keeps the link; plus the null renders for `target="local"` and for an unset +target. + +Do the tests verify what they claim? Yes: +- The `runAction` helper builds the exact event shape `actionsRunUrl` parses + (`kind: 'driver'`, `event.type: 'action'`, label `run `), so the link tests exercise the + real extraction rather than a mock. +- Assertions are on rendered output (`getByRole('status')` text, `getByRole('link')` href, + `container.firstChild === null`), each of which fails if the component regresses: dropping the + gate breaks both null tests, losing the live clause breaks test 1, rendering the link + unconditionally breaks test 3, keeping the clause when finished breaks test 4 (a negative + match, but paired with the positive match in test 1 on the same string, so the pair cannot both + pass vacuously). +- All renders are synchronous; nothing async is left un-awaited. `afterEach(cleanup)` prevents + cross-test DOM bleed (needed since `getByRole('status')` would otherwise match a previous + render). + +Coverage gap worth noting, not filing: no test that a `remote`/`web` target renders nothing (the +component's union names them), and no test that the *last* `run ` event wins — both +behaviors live in `actionsRunUrl`/the gate and are low-risk. + +## Functions (low-level) + +- **`runAction(url)`** — event factory; matches the production regex's expected label format. + Correct. +- **test "explains the burst wait"** — live + no events → status text matches. Correct. +- **test "links through"** — href equality against the exact URL, name-scoped role query. + Correct. +- **test "no link before"** — `queryByRole('link')` null. Correct. +- **test "finished drops the line, keeps the link"** — negative text match plus positive link + presence. Correct. +- **tests "renders nothing" (local / unset)** — `container.firstChild` null; the unset variant + also pins that the prop is optional. Correct. + +## Bugs found + +None found. diff --git a/packages/framework/dashboard/components/AddDeviceDialog.BUG-ANALYSIS.md b/packages/framework/dashboard/components/AddDeviceDialog.BUG-ANALYSIS.md new file mode 100644 index 000000000..ae2896465 --- /dev/null +++ b/packages/framework/dashboard/components/AddDeviceDialog.BUG-ANALYSIS.md @@ -0,0 +1,67 @@ +# Bug analysis: packages/framework/dashboard/components/AddDeviceDialog.tsx + +## Business logic (high-level) + +The "Add a device" modal (#1052): one paste of the `http://host:port/?token=…` URL another +machine's daemon printed on its network bind, an optional name, Save/Cancel. Responsibilities per +`AddDeviceDialog.SPEC.md`: + +- **One paste, not a form** — `parseDeviceUrl` (lib/profiles.ts) splits the paste into origin and + token; the user never transcribes a token. +- **A tokenless URL cannot be saved** — `valid = parsed !== null && parsed.token !== ''` disables + "Add device"; once the user typed something, the reason is spelled out (not a URL vs URL with + no token). The `save()` function double-guards (`if (!parsed || !valid) return`), so Ctrl-Enter + cannot bypass the disabled state. +- **The name is optional** — placeholder shows the host that will be used; `addProfile` itself + trims the label and falls back to the host, so the dialog passing the untrimmed `label` is fine + (the `label.trim() ? { label } : {}` spread only decides presence; profiles.ts normalizes). +- **Per-browser storage** — `addProfile` writes localStorage; nothing daemon-side. Saving calls + `onAdded()` then `onClose()`; Cancel and the dialog's own dismiss (`onOpenChange(false)`) close + without saving. Cmd/Ctrl-Enter saves from either field via a keydown handler on the wrapper. + +The failure mode that breaks this: `parseDeviceUrl` accepts any string the `URL` constructor +accepts, and the URL constructor accepts scheme-less pastes like `localhost:4200/?token=abc` +by treating `localhost` as the *scheme* — yielding an opaque URL whose `origin` is the literal +string `"null"` while `searchParams` still finds the token. Two consequences chain off that +(confirmed with node): the dialog's placeholder computes `new URL(parsed.url).host` = +`new URL("null")`, which **throws during render**, and `valid` is `true`, so were it not for the +crash a device with url `"null"` would be saved (its `connectTo` would navigate to the relative +path `/null`). Dropping the scheme is one of the most likely paste mistakes for this exact input +(`192.168.1.5:4200/…` happens to throw in `URL` and is handled; `localhost:4200/…` and any +hostname-first paste are not). See Bug 1. + +## Functions (low-level) + +- **`AddDeviceDialog({ onClose, onAdded })`** — state: `url`, `label`; derived: `parsed`, + `valid`. Renders help line, URL input (autofocus, monospace), name input with dynamic + placeholder, conditional warning (only when the user typed something and it is not savable — + distinguishing the two reasons per SPEC), Cancel and the gated Add button. Edge cases: empty + input → no warning, disabled button (correct); URL with empty `?token=` → "no token" branch + (correct, `token: ''`); whitespace-only URL → `url.trim() !== ''` false → no warning while the + button stays disabled (correct); the dynamic placeholder re-derives `new URL(parsed.url)` every + render — the crash vector when `parsed.url` is `"null"` (Bug 1). Verdict: **bug found** (root + cause in `parseDeviceUrl`, crash manifests here). +- **`save()`** — guards, `addProfile({url, token, ...label})`, `onAdded()`, `onClose()`. Ordering + is right (list refresh callback before close). Correct. +- **`onKeyDown(e)`** — Cmd/Ctrl+Enter → preventDefault + save; plain Enter does nothing (matches + the SPEC's "Ctrl-Enter … saves from anywhere in the dialog"; no accidental submit). Correct. + +## Bugs found + +1. `L50` (root cause: `parseDeviceUrl`, packages/framework/dashboard/lib/profiles.ts L89-96): + **A scheme-less paste like `localhost:4200/?token=abc` crashes the dialog render — and would + otherwise save a device with URL `"null"`.** `new URL('localhost:4200/?token=abc')` parses + with `localhost` as the scheme, so `u.origin` is the string `"null"` while + `searchParams.get('token')` still returns `abc`; `parseDeviceUrl` therefore returns + `{ url: 'null', token: 'abc' }` instead of `null`. The dialog's name-field placeholder then + evaluates `new URL('null').host`, which throws `Invalid URL` in the middle of render, taking + down the React tree to the nearest boundary — triggered by exactly the paste mistake this + dialog exists to absorb (dropping the `http://` prefix; verified in node, and note the + near-identical paste `192.168.1.5:4200/?token=…` happens to be rejected cleanly, making the + behavior arbitrary). Even without the placeholder crash, `valid` would be `true` and "Add + device" would store an unusable profile whose `connectTo` navigates to the relative path + `/null`, contradicting the SPEC's promise that only "a valid URL that carries a token" is + savable. Severity: major. Fix (in `parseDeviceUrl`): reject opaque/non-http origins — e.g. + `if (u.origin === 'null' || (u.protocol !== 'http:' && u.protocol !== 'https:')) return null` + — so the paste falls into the existing "That is not a valid URL." branch; optionally also + guard the placeholder with `hostLabel`-style try/catch as defense. diff --git a/packages/framework/dashboard/components/AddDeviceDialog.test.BUG-ANALYSIS.md b/packages/framework/dashboard/components/AddDeviceDialog.test.BUG-ANALYSIS.md new file mode 100644 index 000000000..f3dd86f97 --- /dev/null +++ b/packages/framework/dashboard/components/AddDeviceDialog.test.BUG-ANALYSIS.md @@ -0,0 +1,48 @@ +# Bug analysis: packages/framework/dashboard/components/AddDeviceDialog.test.tsx + +## Business logic (high-level) + +Pins the three behaviors its SPEC names, against the real `profiles.ts` storage (no mocks — +assertions read `listProfiles()` back out of the actual localStorage, cleared in `afterEach` +along with `cleanup()`, so tests are isolated and the origin/token split is tested end to end): + +1. Pasting `http://192.168.1.5:4200/?token=abc123` and clicking "Add device" stores exactly + `{ id, label (host default), url (bare origin), token }` and fires both `onAdded` and + `onClose` — this genuinely pins the origin normalization (query stripped), the host-default + label, and the close-after-save contract. +2. A URL without a token leaves the button disabled and shows the "no token" explanation — pins + the savability gate and the *which-reason* messaging. +3. A typed name overrides the host default — pins the optional-label path. + +The queries are sound: `getByPlaceholderText(/host:port/)` uniquely matches the URL input (the +name field's placeholder becomes "Name (optional) — defaults to …" once a URL parses, never +containing "host:port"), and `getByPlaceholderText(/Name/)` uniquely matches the name field. +Every assertion can fail if the behavior regresses (disabled flag read off the real button, +storage read back, spy call counts). All interactions are synchronous `fireEvent`s — nothing +async to await. + +Coverage gaps (noted, not bugs): no test for the "not a valid URL" branch of the warning, none +for Cmd/Ctrl-Enter save or Cancel, and — most relevantly — none for a scheme-less paste like +`localhost:4200/?token=abc`, which currently *crashes* the component render (see +AddDeviceDialog.BUG-ANALYSIS.md Bug 1); a regression test there would have caught it, and should +be added alongside the fix in `parseDeviceUrl`. + +## Functions (low-level) + +- **`afterEach`** — `cleanup()` plus `localStorage.clear()`; the second is required because + profiles.ts caches a snapshot keyed to a notify counter — clearing storage without notify is + fine here since each test re-renders fresh and `listProfiles()` reads storage directly. + Correct. +- **test "pasting a ?token= URL saves a profile and closes"** — change → click → deep-equal on + stored list → spies. The deep equality is strict (id/label/url/token all pinned), so a change + to normalization or labeling fails it. Correct. +- **test "a URL without a token cannot be saved"** — disabled assertion via the concrete + `HTMLButtonElement`, message presence via `getByText(/no token/i)`. Correct. +- **test "an optional label overrides the host default"** — stores then reads `label`. Uses + `listProfiles()[0]!` — safe because the click preceded it; if saving broke, the assertion + throws (test fails) rather than passing vacuously. Correct. + +## Bugs found + +None found. (The missing scheme-less-paste regression case is a coverage gap tied to the +component bug filed in AddDeviceDialog.BUG-ANALYSIS.md, not a defect in the existing tests.) diff --git a/packages/framework/dashboard/components/AddProjectPanel.BUG-ANALYSIS.md b/packages/framework/dashboard/components/AddProjectPanel.BUG-ANALYSIS.md new file mode 100644 index 000000000..35d5be451 --- /dev/null +++ b/packages/framework/dashboard/components/AddProjectPanel.BUG-ANALYSIS.md @@ -0,0 +1,76 @@ +# Bug analysis: packages/framework/dashboard/components/AddProjectPanel.tsx + +## Business logic (high-level) + +The "Add project" modal (#396/#1150/#439): mounting it immediately asks the daemon to open the OS +folder picker (`sendPickProjectDirectory`), because a browser page cannot learn an absolute path +on its own. Phases are derived from state: *picking* (`path === null`, no error), *pick error* +(`pickError` set — reason plus Try again/Cancel), *trust* (`path` set — path echoed, plain +prompt-injection warning, Choose again / "I trust it, add it"), *done* (`added` set — "Project +added" / "Already added", auto-close after 2.5s or Done). Per the SPEC: + +- **The system dialog is the form** — pick on mount; a dismissed picker (`path: null`) closes the + modal (`onClose`); a picker that cannot open shows its reason with Try again. Holds. +- **Trust gate before installing** — nothing calls `sendAddProject` until the trust button; + `confirmAdd` guards on `busy` and `path`; a failed add is routed by `useAction` into `error` + and rendered in place on the trust step (state stays, so the user can retry or re-pick). Holds. +- **Says what happened** — `alreadyActivated` selects the wording; auto-close timer cleaned up on + unmount; Done closes sooner. Holds. +- **Dialog contract** — Esc closes, Tab cycles inside, focus returns to the opener, click-away + closes. Partially holds: the trap/Esc handler is a `keydown` listener on the modal's own + subtree, so it only works while focus is *inside* the panel — and the first two phases render + no autofocused element, so on open focus stays wherever it was (the now-gone menu item → body, + or a focus-restoring dropdown trigger), where Esc and the Tab trap do nothing (Bug 1). + +Concurrency/ordering: `pick()` is not covered by `busy`, so rapid double-clicks on "Choose +again"/"Try again" fire two concurrent `sendPickProjectDirectory` calls (two native dialogs or a +daemon-side refusal — daemon behavior not verified; suspicious-but-unproven, not filed). State +updates after unmount (user closes the modal while the OS dialog is open, then picks/dismisses) +are React no-ops; the dismissal path would call `onClose()` a second time, which parents treat +idempotently. Closing mid-`confirmAdd` lets the add complete daemon-side without `onAdded()` — +the sidebar then catches up on the 30s projects poll; accepted, not filed. + +## Functions (low-level) + +- **mount focus effect (L24-27)** — captures `document.activeElement` at mount, refocuses it on + unmount. Whether the captured element is the dropdown trigger (comment's claim) or `body` + depends on the opener's own focus handling; either way the *return* half is best-effort and + harmless. Correct-ish; the *initial* focus placement is the gap (Bug 1). +- **`pick()` (L31-44)** — clears `pickError`, calls the RPC with a catch that synthesizes + `{ok:false, error:'Could not reach the daemon.'}` (so a transport failure lands in the same + error phase — good); `!picked.path` → `onClose()` (dismissed); success → `reset()` (clears a + stale add error from a previous trust round) then `setPath`. The `reset()` placement matters + and is right: a re-pick after a failed add starts the new trust step clean. Correct. +- **mount pick effect (L45-48)** — fires once. Correct. +- **auto-close effect (L51-56)** — timer keyed on `added`, cleared on unmount/re-run; `onClose` + deliberately out of deps (fresh closure). Correct. +- **`confirmAdd()` (L59-66)** — busy/path guard; `run` maps `{ok:false,error}` and throws into + `error` with the fallback; success records `alreadyActivated` and calls `onAdded()` before the + timed close. Correct. +- **`onKeyDown(e)` (L69-89)** — Esc → preventDefault + close; Tab → cycle among + `button, input, [tabindex]:not([tabindex="-1"])` minus disabled, wrapping at both ends. The + query is computed per keypress (fresh across phases — good). Only receives events from inside + the panel subtree (Bug 1 window aside, the trap itself is correct; during `busy` both buttons + are disabled, `focusable` can be empty, and the `!first || !last` guard bails cleanly). + Correct in itself. +- **render (L91-162)** — backdrop div closes on click; panel `role="dialog" aria-modal aria-label`; + the four phases ordered `added` → `pickError` → `path` → picking, which resolves the state + combinations correctly (an `added` result keeps showing even though `path` is still set; a + `pickError` can only coexist with `path` from a failed re-pick — error wins, and Try again + leads back). `autoFocus` on the trust and Done primary buttons pulls focus into the dialog from + those phases on. Correct. + +## Bugs found + +1. `L92` (with L147-158, the picking phase): **Esc (and the Tab trap) do not work when the dialog + opens, because nothing inside the modal is focused in the picking and pick-error phases.** The + dialog contract is implemented as a `keydown` handler on the modal wrapper, which only sees + events dispatched to its own subtree; the picking phase (and the pick-error phase on first + entry) renders no `autoFocus` element, so after the opener menu item unmounts, focus sits on + `body` (or on a dropdown trigger that restored focus to itself) — outside the wrapper. A user + who opens "Add project" and presses Esc gets nothing; Tab walks the page underneath the modal + overlay. This contradicts the SPEC's explicit "Esc, Cancel, and clicking outside the modal + close it without adding; keyboard focus stays inside the modal". The trust step masks the bug + later via its autofocused button. Severity: minor. Fix: give the picking phase's Cancel button + (and the pick-error phase's Try again) `autoFocus` — or attach the Escape handler to + `document` for the dialog's lifetime. diff --git a/packages/framework/dashboard/components/AddProjectPanel.test.BUG-ANALYSIS.md b/packages/framework/dashboard/components/AddProjectPanel.test.BUG-ANALYSIS.md new file mode 100644 index 000000000..681e10793 --- /dev/null +++ b/packages/framework/dashboard/components/AddProjectPanel.test.BUG-ANALYSIS.md @@ -0,0 +1,48 @@ +# Bug analysis: packages/framework/dashboard/components/AddProjectPanel.test.tsx + +## Business logic (high-level) + +Pins the six flows its SPEC lists, with the two RPCs mocked (`vi.hoisted` + `vi.mock` before a +top-level `await import` of the component — the correct vitest pattern) and everything else real: + +1. Picker asked on mount (`toHaveBeenCalledTimes(1)` immediately after `render` — valid because + RTL flushes effects inside `act`), path echoed on the trust step, `sendAddProject` **not** + called until the trust click, then called with exactly the picked path — this genuinely pins + the trust gate ordering, the SPEC's central promise. +2. `alreadyActivated: true` → "Already added" wording. +3. Dismissed picker (`path: null`) → `onClose` awaited via `waitFor`, and nothing added. +4. Pick failure → reason rendered; "Try again" → second mock answer → trust step. The + `mockResolvedValueOnce` chaining makes the retry observable. +5. Failed add → daemon's error text rendered *and* the trust button still present (stays on the + trust step) — asserts both halves of the SPEC sentence. +6. "Choose again" → picker called again, new path replaces the old (asserted by finding + '/Users/dev/second' and the call count). + +Every async transition is awaited (`findByText`/`findByRole`/`waitFor`); `afterEach` cleans up +DOM and resets both mocks, so no cross-test leakage of `mockResolvedValueOnce` queues (reset, not +clear — implementations are re-established per test). None of the tests can pass vacuously: each +asserts either a rendered phase change or a mock call pattern that the component must produce. + +Coverage gaps (noted, not defects): no tests for Esc/backdrop/Cancel closing, the focus trap, the +2.5s auto-close, `onAdded` being invoked, or the "Could not reach the daemon." catch path. The +missing Esc coverage is what lets the phase-1 dead-Esc bug (see AddProjectPanel.BUG-ANALYSIS.md +Bug 1) go unnoticed; a `fireEvent.keyDown` test would have to target a focused in-dialog element +to be faithful, which is exactly the condition the component fails to establish. + +## Functions (low-level) + +- **module setup (L4-8)** — hoisted mock fns; `vi.mock` of `../rpc/projects.js`; dynamic import + after mocks. Correct. +- **`afterEach` (L10-14)** — `cleanup` + `mockReset` on both fns. Correct. +- **test 1 (L17-29)** — the not-called-yet assertion sits after `findByText` (trust step + reached), so it proves ordering, not just timing luck. Correct. +- **test 2 (L31-37)** — clicks via `await findByRole` (waits for the trust step). Correct. +- **test 3 (L39-45)** — `waitFor(onClose called)`; also asserts no add. Correct. +- **test 4 (L47-54)** — once/once mock sequencing; asserts both the message and the recovery. + Correct. +- **test 5 (L56-63)** — error text plus still-on-trust-step. Correct. +- **test 6 (L65-73)** — re-pick replaces the path; call count 2. Correct. + +## Bugs found + +None found. diff --git a/packages/framework/dashboard/components/AgentActionBar.BUG-ANALYSIS.md b/packages/framework/dashboard/components/AgentActionBar.BUG-ANALYSIS.md new file mode 100644 index 000000000..70a008d1c --- /dev/null +++ b/packages/framework/dashboard/components/AgentActionBar.BUG-ANALYSIS.md @@ -0,0 +1,50 @@ +# Bug analysis: packages/framework/dashboard/components/AgentActionBar.tsx + +## Business logic (high-level) + +One agent's action bar, a pure composition with no state of its own. Left: identity and git +context via `GitStatusBar` (project/session breadcrumb, branch as a disclosure when `onToggle` +is given, dirty flag, disk size, PR, `summary`) with the one status pill injected beside the +branch facts as `agentState`. Right, in the SPEC's order: `AgentErrorCount` (unresolved error +count), the caller-supplied `actions` node (the handoff's next visible step), and the +`AgentActionsMenu` overflow. Serves running and finished agents alike so controls stay put at +Done — nothing here branches on liveness; the children derive everything from `events`. + +SPEC conformance checked point by point: +- **Identity first, git second** — label/projectName/summary/expanded/onToggle forwarded to + `GitStatusBar`; the bar itself adds nothing. Holds (delegated). +- **One status word** — `agentStatusPill(events)` (lib/agent-status.ts) is ranked exactly as the + SPEC lists: failed (with reason) > stopped > publishing > ready for merge > building (only + while active) > finished, and null until the agent named itself/reached a state/ended. + Verified against the implementation; the pill renders with `title={status.label}` so the + truncated (max-w-40) failure reason is still reachable, and the dot is `aria-hidden`. Holds. + (The inline comment at L53-54 lists only five states, omitting "publishing" — stale comment, + not behavior; noted only.) +- **Errors counted next to the controls** — `AgentErrorCount` sits inside the `shrink-0` + controls cluster, so the count cannot be truncated away. Holds. +- **Only the next step is a button** — `actions` is a slot; the menu gets + `retainedWorktree`/`onWorktreeRemoved`/`onDeleted` so remove-worktree and delete are offered + per the caller's knowledge. Holds (the caller decides what `actions` holds; this file cannot + enforce "only the next step", by design). +- **The row never wraps** — `flex items-center` with `overflow-hidden`, a `grow shrink-0` + spacer, and a `shrink-0` controls cluster; only `GitStatusBar` (min-width-able) gives up + width. A pathologically long status label is capped at `max-w-40`. Holds. + +Edge cases: `agentId` absent (project-root control log fallback) is simply forwarded; `status` +null renders no pill (the `agentState` expression short-circuits to `false`, which GitStatusBar +must treat as absent — consistent with its `ReactNode` prop); `events` empty → null pill, zero +errors, menu still usable. No effects, subscriptions, or timers — nothing to leak; re-renders +are driven entirely by the parent's `events` identity. + +## Functions (low-level) + +- **`AgentActionBar(props)`** — computes `status = agentStatusPill(events)` once per render and + lays out the row as described. Inputs: ids, events, presentation props, callbacks. Output: one + flex row. Failure modes: none of its own — no async, no state; incorrect `events` ordering + would surface in the children, which own that logic. The `agentState` prop passes `status && + ()`, so a falsy status contributes `null`-ish content rather than an empty wrapper. + Verdict: correct. + +## Bugs found + +None found. diff --git a/packages/framework/dashboard/components/AgentActionsMenu.BUG-ANALYSIS.md b/packages/framework/dashboard/components/AgentActionsMenu.BUG-ANALYSIS.md new file mode 100644 index 000000000..395e5f965 --- /dev/null +++ b/packages/framework/dashboard/components/AgentActionsMenu.BUG-ANALYSIS.md @@ -0,0 +1,104 @@ +# Bug analysis: packages/framework/dashboard/components/AgentActionsMenu.tsx + +## Business logic (high-level) + +The ⋮ overflow menu for one agent. Per its SPEC: open on GitHub (when the remote URL is known), +open folder / open in editor (the agent's own checkout while it has one — `active || +retainedWorktree` — else the project root, renamed and explained on hover), preferred-editor +submenu (detected editors + "Default", the remembered-but-undetected editor still listed, saved +via `updatePreferences` without closing the menu), the driver-session link, the session-id item +that copies a resume command (`mkdir -p && cd && --resume `, +or just the id when no workspace was recorded) and flashes "Copied" for 1.5s, Stop and "Merge +when finished" while live, Remove worktree and Delete (confirmed in a dialog) once ended. + +State machine notes: +- `hasOwnFolder = active || retainedWorktree` matches the SPEC's definition of "while it has + one" (#737/E5); the naming and hover explanation follow it. Correct. +- Stop latch: `stopRequested` set only after `sendStop` resolves; `stopping = busy || + (stopRequested && active)`; reset only on `agentId` change. Two flaws: (a) `sendStop` resolves + void even when the relay to a remote device failed (`relayOr(..., undefined)` in + src/dashboard-rpc/control.ts swallows the unreachable case), so `.then(() => true)` records a + landed stop that never landed (Bug 1); (b) the latch is never cleared when the agent ends, so + a stopped-then-resumed agent (same id, same mount) renders Stop as a disabled "Stopping…" + forever (Bug 2 — `isAgentActive` is deliberately segment-scoped, so `active` flips true again + on resume while `stopRequested` is still true). +- Merge latch: `mergeRequested` set on `{ok:true}`; persisting across a stop/resume is *correct* + because arming is run-level config daemon-side (`handoff-armed`, latest wins across segments). +- Deletion: menu item only opens the controlled `ConfirmDialog` (a menu item cannot be the + dialog trigger); `onConfirm` maps `{ok:false}` to a rejection so the dialog shows the error; + `onSuccess` = `onDeleted`. Matches the SPEC's naming/warning copy. +- Errors from `useAction` render inside `DropdownMenuContent`. Since ordinary items close the + menu on click (Base UI default), a failure surfaces only when the menu is reopened — the error + state persists, so it is discoverable, but the moment of failure shows nothing. Borderline + against "Both report their failure inside the menu"; recorded as a concern, not filed, since + the text does live inside the menu and reopening shows it. +- The header comment (L43-44) still describes a Serve item ("Serve keeps its state (Serve → + Open/Stop…)") that the menu no longer renders and the SPEC no longer lists — stale comment, + noted only. + +## Functions (low-level) + +- **derivations (L63-78)** — `active` (segment-scoped), `info = sessionInfo(events)`, `session = + describeSessionLink(info)`, `resumeCommand = buildResumeCommand(info)`. Correct. +- **copy feedback (L81-91)** — `copied` + ref-held timer, cleared on unmount and on re-click. + `navigator.clipboard?.writeText(...)` — the optional chain short-circuits the whole + `.then(...)` when `clipboard` is undefined, so no crash, but also no copy and no feedback: on a + dashboard reached over plain http from another machine (a first-class feature: non-loopback + bind behind a token), `navigator.clipboard` does not exist outside a secure context and the + item silently does nothing (Bug 3). Verdict: **bug found**. +- **`githubUrl` (L93)** — `useLoaded` with `keepPrevious`: holds the previous project's URL while + the next loads (deliberate anti-flicker; a beat of a wrong link on project switch is the + documented trade). Correct. +- **editor rows (L95-98)** — appends the remembered-but-undetected editor; tick via opacity on + the current choice; "Default" documented as `$FRAMEWORK_EDITOR, or code`. Matches SPEC. + Correct. +- **`openApp(target)` (L114)** — one `useAction` instance shared by open/stop/merge/remove: a + slow open disables Stop/Merge/Remove via `busy` — conservative, acceptable. Correct. +- **`stopSession()` (L115-118)** — maps void to `true`; failure routes to `error` and leaves the + latch unset. Correct locally; the remote-relay swallow makes it lie (Bug 1, fix daemon-side). +- **`mergeAgent()` (L119-124)** — guards on `agentId`; sets the armed latch only on `{ok:true}` + (the unreachable-device relay answer is `{ok:false, error}`, which correctly surfaces). + Correct. +- **`removeWorktree()` (L125-130)** — `result !== undefined` is the success test; `sendRemoveWorktree` + returns a `RemoveWorktreeResult` object on success and `{ok:false}` failures are mapped to + `undefined` by `useAction`, so the check is sound. Correct. +- **render (L134-279)** — items and separators gated as the SPEC orders; the between-section + separator's condition mirrors the two ended-agent items exactly; the resume item requires both + `resumeCommand` and `info?.sessionId`; the session-id slice (8 chars) matches the SPEC's + "first characters". The delete dialog is mounted outside the dropdown so it survives the + menu closing. Correct. + +## Bugs found + +1. `L116` (root cause: `sendStop`, packages/framework/src/dashboard-rpc/control.ts L50-54 with + src/dashboard-rpc/relay-agent.ts): **Stopping a remote agent whose device is unreachable + reports success — the menu reads "Stopping…" while the agent keeps running, with no error.** + `sendStop` is relayed with `relayOr(..., undefined)`: an unreachable device resolves the void + fallback, indistinguishable from a landed stop. The menu maps any resolution to `true` + (`.then(() => true)`) and latches `stopRequested`, so the item shows a permanent, disabled + "Stopping…" and the SPEC's "Both report their failure inside the menu" is unmet for Stop + (contrast `sendMerge`, whose fallback is `{ok:false, error: 'could not reach the device'}` + and does surface). Scenario: a saved-device agent is running, the device goes to sleep, the + user clicks Stop — nothing stops and nothing says so. Severity: major. Fix: give `sendStop` + a result shape like `sendMerge`'s (`{ok:true}` locally, `{ok:false, error: 'could not reach + the device'}` as the relay fallback) and have `stopSession` latch only on `ok`. + +2. `L103-L105`: **A stopped-then-resumed agent cannot be stopped again — Stop is stuck at + "Stopping…".** `stopRequested` is reset only on `agentId` change, but resuming a stopped + agent keeps the same id and the same mounted menu: the stop's `end` event flips `active` + false (item hidden), the resume opens a new segment flipping `active` true again, and + `stopping = stopRequested && active` re-disables the item with the "Stopping…" label even + though no stop is pending. Resuming a stopped agent is a first-class flow + (FEATURES-SPEC: "Resume a stopped agent"). Workaround is only to navigate away and back + (remount resets the state), which nothing suggests. Severity: minor. Confidence: high. + Fix: clear the latch when the agent stops being active — e.g. + `useEffect(() => { if (!active) setStopRequested(false) }, [active])`. + +3. `L86`: **The copy-resume item silently does nothing on a dashboard reached over plain http + from another machine.** `navigator.clipboard` exists only in secure contexts (https or + localhost); reaching the daemon at `http://:4200` — the documented remote-access + feature — leaves it undefined, the optional chain skips the write *and* the `.then`, so no + command is copied and no "Copied"/error feedback appears; the SPEC promises "clicking that + item copies a shell command … and flashes 'Copied'". Severity: minor. Fix: fall back to a + temporary textarea + `document.execCommand('copy')` when `navigator.clipboard` is absent, or + flash a "copy unavailable — select the command from the tooltip" state instead of nothing. diff --git a/packages/framework/dashboard/components/AgentComposer.BUG-ANALYSIS.md b/packages/framework/dashboard/components/AgentComposer.BUG-ANALYSIS.md new file mode 100644 index 000000000..7c5563585 --- /dev/null +++ b/packages/framework/dashboard/components/AgentComposer.BUG-ANALYSIS.md @@ -0,0 +1,99 @@ +# Bug analysis: packages/framework/dashboard/components/AgentComposer.tsx + +## Business logic (high-level) + +One message box per agent, alive for the agent's whole life (#1026). Three send modes chosen at +submit time: live → `sendMessage` (queued control entry); ended+resumable (`!live && sessionId`) +→ `sendStart` with `resumeSession`+`continueAgentId` (a continuation of the same agent, same +branch, same driver); ended without a session id → plain `sendStart` (new agent). A +`newAgent: true` preset always starts its own agent regardless of state. The empty box's submit +slot doubles as the agent control: Stop while live, Resume once stopped-with-an-id, nothing +otherwise (SPEC: "ended any other way, or with no session id, leaves the slot empty"). + +Key invariants checked against `AgentComposer.SPEC.md`: + +- **Continuation keeps driver, never re-chooses model** — `driverFromImpl(driver)` maps the + recorded impl id (`claude-code`/`claude-web`/`github-actions` → `claude`) and only passes a + `driver` option when it is not the default `claude`; no `model` option is ever passed. Matches + SPEC and #831 tests. +- **Stop latch** — `stopping = stopBusy || (stopRequested && live)`; `stopRequested` resets when + `live` drops or the agent id changes, so a resumed same-id agent gets a working Stop again + (#762 regression covered by test). +- **Resume latch (#1460)** — `resuming` holds the busy Resume between the RPC resolving and the + first live event; released on `live` or agent-id change. The slot condition + `resumable && (outcome?.stopped || resuming)` keeps the Resume rendered while `outcome` + momentarily reads undefined. Correct against the flicker scenario in the test. +- **Failure keeps text** — both `useAction.run` and `useStartAgent.start` return `undefined` on + failure; `composerRef.clear()` only runs on success, so a failed send keeps the user's text as + the SPEC requires. +- **Queued echo (#948)** — `queued` is set after a successful live send and rendered by `Note` + while `live`. See bug 1: it is never cleared on agent change or resume, unlike the two latches + next to it. + +Concurrency/ordering: `send` guards on `busy || starting` so double submits are dropped; +`useAction` serializes state via its own busy flag. The three latches all reset via effects keyed +to `agentId`/`live`, which matters because `AgentView` (and `App.tsx`) never remount this +component across agent switches — the whole design leans on prop-keyed resets, so a piece of +state without one goes stale (bug 1). + +Error surfacing: `error ?? startError ?? stopError` — one alert line; each `useAction` clears its +own error on the next run, but a *different* action's stale error is only replaced when that +action re-runs. E.g. a failed Stop's message stays on screen after a later successful message +send (the send clears `error`, not `stopError`). Cosmetic-only; noted, not reported. + +## Functions (low-level) + +- **`RESUME_MESSAGE` (const, L18)** — the stock resume text. Matches the SPEC's three required + points (stopped-not-done, look at what was done and carry on, setReadyForMerge lifecycle). + Correct. +- **`AgentComposer` (L40)** — see above. Props of note: `agentId` optional (falls back to project + control log in `sendMessage`/`sendStop` via `agentId ?? undefined`); `sessionId` gates + resumability; `outcome` drives the note wording. Correct except bug 1. +- **`send(text, _kind, opts)` (L101)** — + - `opts.newAgent || (!live && !resumable)` → new agent via `start(projectId, text, 'prompt', {})`; + clears+refocuses on success, navigates via `onAgentStarted`. Note the new-agent branch keeps + kind `'prompt'` even when the Composer submitted `'build'` — same as the continuation branch; + the mocked test submits `'build'` for `submit-normal` and the tests assert `'prompt'` reaches + `sendStart`, so this coercion is intended (a continuation is a prompt run). Correct. + - live branch: maps `sendMessage`'s void resolve to `true` so `useAction`'s failure + `undefined` is tellable — correct; on success sets `queued` and clears. + - continuation branch: only reachable when `!live && resumable`, so `sessionId as string` is + safe; passes `continueAgentId` only when `agentId` is set; driver mapped per #831. Correct. +- **`stopSession` (L152)** — fire-and-forget wrapper with `.then(result => setStopRequested(true))` + on success. `sendStop` resolves void → mapped to `true`. Correct. +- **`resume` (L160)** — guards `starting || !sessionId`; does not guard `resuming`, but the button + is disabled while `resuming`, so unreachable through the UI. Sets `resuming` only on success — + a refused resume keeps the slot pressable and surfaces the error (test-covered). Correct. +- **`idleControl` (L183)** — live → Stop (disabled while `stopping`, spinner + "Stopping…"); + stopped-with-id or latched `resuming` → Resume (disabled while `starting || resuming`); else + `undefined` so the Composer collapses the empty slot to the send arrow. `starting` is shared + with the typed-continuation flow, so a typed send also flips the Resume tooltip to "Resuming…" — + consistent with `submitBusyLabel="Resuming…"`. Correct. +- **`Note` (L264)** — live: renders the queued echo unless muted by an error; not live: nothing + when not resumable (the placeholder `NOT_CONTINUABLE` covers that state exactly once — asserted + by the test that no `

` exists); resumable: failed (`!ok && !stopped`) → "Session failed…", + `stopped` → "Session stopped…", else "Agent ended…". A crash is never called "ended" — matches + SPEC. Correct except that the queued echo it renders can be stale (bug 1). +- **`NOT_CONTINUABLE` (L260)** — placeholder text for the ended-no-session-id state. Correct. + +Edge cases considered and fine: empty `files` (passed through), absent `agentId` (project-log +fallback is deliberate, used by the adopting view in `App.tsx`), `driver` undefined +(`driverFromImpl(undefined)` → undefined → no driver option), `outcome` undefined while resumable +(→ "Agent ended — your next message continues it", the honest wording when the end event is not +in view yet). + +## Bugs found + +1. **L98: `queued` echo is never reset — stale "Queued — …" note on agent switch and on resume.** + Scenario A: watch live agent A, send "do X" (note appears), then select live agent B from the + rail — `AgentView`/`AgentComposer` stay mounted (no `key` anywhere from `App.tsx` down), and + B's composer shows "Queued — the session reads it between turns: 'do X'" although nothing was + ever queued for B. Scenario B: send "do X" to a live agent, it stops, press Resume — the + moment the resumed leg reads live the old "Queued — do X" note reappears even though that + message was drained (or died with) the previous leg. Contradicts the SPEC ("the dashboard + repeats it **until then**" — until the agent reads it, for **that** send) and the file's own + convention: the two sibling latches (`stopRequested` L82, `resuming` L91) both reset on + `[agentId]` / `live` transitions precisely because the component is never remounted. Severity: + minor (misleading UI text, no data loss). Fix: `useEffect(() => setQueued(null), [agentId])` + plus clear it when `live` flips false (agent ended ⇒ the queue is gone), e.g. in the existing + `if (!live)` effect. diff --git a/packages/framework/dashboard/components/AgentComposer.test.BUG-ANALYSIS.md b/packages/framework/dashboard/components/AgentComposer.test.BUG-ANALYSIS.md new file mode 100644 index 000000000..f48649a6a --- /dev/null +++ b/packages/framework/dashboard/components/AgentComposer.test.BUG-ANALYSIS.md @@ -0,0 +1,64 @@ +# Bug analysis: packages/framework/dashboard/components/AgentComposer.test.tsx + +## Business logic (high-level) + +Tests for `AgentComposer` against `AgentComposer.test.SPEC.md`. The Composer is mocked to two +buttons (`submit-normal` → `('hello','build',{newAgent:false})`, `submit-new-session` → +`('Import tickets from GitHub','prompt',{newAgent:true})`), a props echo span, and a passthrough +of `idleControl` so the slot's Stop/Resume can be pressed. RPCs (`sendMessage`, `sendStart`, +`sendStop`) are mocked; preferences are mocked so the #831 tests can set a conflicting driver +pref and prove the continuation ignores it. + +Coverage vs. the test SPEC, block by block: + +- **Slot control (#1455)**: live Stop (fires `sendStop('p1','run-1')`, stays disabled), latch + release on `live` false→true rerender (the #762 resumed-same-id regression), stopped Resume + (asserts the full `sendStart` tuple: RESUME_MESSAGE, kind `prompt`, `resumeSession`, + `continueAgentId`, then navigation), refused Resume (error surfaced, no navigation), + finished-clean offers neither, no-session-id offers no Resume, and the #1460 no-flicker hold + (busy Resume across an `outcome: undefined` rerender, handover to Stop on `live`). All claims + in the SPEC's first bullet are genuinely asserted. The "surfaces a refusal" test matches on + `/agent is already active/i`, which is the wording `useStartAgent` maps `busy: true` to — the + real mapping code runs (only the RPC layer is mocked), so the assertion is honest. +- **Live (#714)**: ordinary send → `sendMessage` with the agent id, never `sendStart`, no + navigation; new-session preset → `sendStart` without `resumeSession`/`continueAgentId`, never + `sendMessage`, navigates to the started agent; refused start → error, no navigation. Matches. +- **Finished (#720/#831)**: resumable note text, continuation tuple (session id + continueAgentId + + no `sendMessage`), driver mapping both ways (claude-code → no driver option; codex → codex, + against an opposing pref), no driver/model select, busy-guard refusal. Matches. +- **No session id (#1026)**: placeholder says can't-be-continued, said only there + (`document.querySelector('p')` null — a tight assertion that also pins "no note above the + box"), composer still present, busy label "Starting…"; a send starts fresh with no resume + options. Matches. +- **Options gear (#1172)**: `agentEnded` false live, true ended. Matches. + +What the tests do *not* cover (gaps, not bugs; the test SPEC does not claim them): the queued +echo note after a live send (which is where the source's stale-`queued` bug hides), the failed +`sendMessage` path keeping text, and the `files`/context plumbing (owned by Composer's own +tests). + +## Functions (low-level) + +- **Mocks (L7-41)** — `vi.hoisted` fns reset in `beforeEach`; the Composer mock's buttons are + disabled by `props.busy`, mirroring the real gating closely enough for the double-submit + guards to stay out of scope. The mock renders `idleControl` unconditionally — the real + Composer only shows it when the box is empty, but that display rule is Composer's own tested + contract, so exercising the control here is sound. Correct. +- **`renderComposer(over)` (L45)** — defaults `live` true, `agentId 'run-1'`; returns the + `onAgentStarted` spy. Correct. +- **`props()` (L53)** — parses the echo span. Correct. +- **Per-test analysis** — each async test awaits its `waitFor`s; negative assertions + (`not.toHaveBeenCalled`) run after a positive `waitFor` on the same flow, so they cannot pass + vacuously by racing. The latch tests drive `live` via `rerender`, matching how the real parent + feeds the poll's verdict. `sendStop.mockResolvedValue(undefined)` exercises the real + void→`true` mapping in `stopSession`. No test can pass with the behavior it pins broken (I + checked each against the source paths): e.g. removing the `continueAgentId` spread fails two + tests; dropping the `resuming` latch fails the #1460 test at the `outcome: undefined` rerender. +- **Typing** — `sendStart.mockResolvedValue({ ok: true })` (no agentId) in the finished block + matches `useStartAgent`'s `{ agentId?: string }` success shape; the assertion + `onAgentStarted` called with `('hello', undefined)` pins that a missing agent id is passed + through rather than faked. Correct. + +## Bugs found + +None found. diff --git a/packages/framework/dashboard/components/AgentDetails.BUG-ANALYSIS.md b/packages/framework/dashboard/components/AgentDetails.BUG-ANALYSIS.md new file mode 100644 index 000000000..59be668ff --- /dev/null +++ b/packages/framework/dashboard/components/AgentDetails.BUG-ANALYSIS.md @@ -0,0 +1,58 @@ +# Bug analysis: packages/framework/dashboard/components/AgentDetails.tsx + +## Business logic (high-level) + +The "about this agent" strip behind the action bar disclosure: driver label, model, and the spend +readout folded from `usage` events (#322). Pure projection of the `events` prop (the same `shown` +feed `AgentView` renders), no state, no effects — nothing to leak or race. Per its SPEC it must +not repeat branch/PR/changes (it doesn't), must abbreviate token counts, and must say "No spend +reported yet" before the first usage event (it does). + +Data contracts verified against `src/events.ts` and `src/agent-view.ts`: + +- `usage` event: `costUsd?` optional (guarded with `!== undefined` — correct, since `$0.00` for a + driver that reports no price would be wrong, per #540), `inputTokens`/`outputTokens`/ + `cacheReadTokens`/`cacheCreationTokens`/`turns` always present — unconditional access is safe. +- `sessionInfo` folds the latest `session` + `session-update`; `driverFromImpl` maps impl ids + (`claude-code`→`claude` etc.), label falls back to the raw impl id (so a `fake` run shows + "fake") and finally to "Agent" when no session event exists yet. Reasonable for every reachable + state. + +Resumed-agent nuance (recorded, not asserted as a bug): each leg runs a fresh `UsageMeter` +(`createDriverEventHandler` in `src/agent-telemetry.ts`), so `usage` events are cumulative **per +leg**. `lastUsage` takes the last event in the whole journal, i.e. a resumed agent's strip shows +only the newest leg's spend while the SPEC says "what it has spent so far". Whether "it" is the +agent (all legs) or the current session leg is not pinned anywhere; every other surface (terminal, +overview) reads the same last event, so this is a consistent product choice rather than a defect +of this file. Suspicious-but-unproven; reported at low confidence since the SPEC wording leans +toward the whole agent. + +## Functions (low-level) + +- **`lastUsage(events)` (L13)** — backwards scan for the last `usage` event. Correct for empty + input (undefined → "No spend reported yet"). Scans the whole feed rather than + `currentAgentEvents`; see the per-leg note above (the last event of the whole feed *is* the last + event of the current leg, so segment-slicing would change nothing). Correct. +- **`compact(n)` (L22)** — `>= 1M` → `X.XM`, `>= 1k` → `X.Xk`, else raw. Edge: 999_950–999_999 + renders "1000.0k" rather than "1.0M"; cosmetic rounding seam, tokens are never negative or + fractional. No `NaN` sources (fields are typed numbers from the meter). Correct (cosmetic edge + noted, not a bug). +- **`Fact` (L28)** — label/value span. Correct. +- **`AgentDetails` (L37)** — renders Agent always; Model only when the latest `session` event + recorded one (a leg that recorded none clears it — matches `sessionInfo`'s per-leg fold); + Spent only with `costUsd`; Tokens+Turns whenever any usage exists; Cache row only when either + cache counter is > 0. All conditionals match the event shapes. Correct. + +## Bugs found + +1. **L13/L41: a resumed agent's spend strip shows only the current leg's totals.** Scenario: + an agent spends $0.80, is stopped, and is resumed; the continuation spends $0.10 — the strip + reads "Spent $0.10 / Turns N(leg2)", although the SPEC for this strip says it shows "what it + has spent so far" and the product's unit of work is the agent (one entry across + continuations, MEMORY.md D5, #762). Root cause is that each leg's `UsageMeter` restarts at + zero (`src/agent-telemetry.ts` `createDriverEventHandler`) and this strip renders the last + `usage` event as the whole truth. Severity: minor (under-reported spend readout; the budget + cap has the same per-leg scope by design). Confidence: low — every surface shares this + reading, so it may be the intended meaning of "spent so far". Fix sketch (if intended to be + whole-agent): sum the final `usage` event of each `session` segment in `lastUsage` (or fold a + running offset when a new `session` boundary follows a `usage` event). diff --git a/packages/framework/dashboard/components/AgentErrorCount.BUG-ANALYSIS.md b/packages/framework/dashboard/components/AgentErrorCount.BUG-ANALYSIS.md new file mode 100644 index 000000000..9e5274cab --- /dev/null +++ b/packages/framework/dashboard/components/AgentErrorCount.BUG-ANALYSIS.md @@ -0,0 +1,40 @@ +# Bug analysis: packages/framework/dashboard/components/AgentErrorCount.tsx + +## Business logic (high-level) + +A pure, stateless projection: fold `error` events via `agentErrors` (src/agent-view.ts — a simple +filter/map that always returns an array), show nothing for zero, otherwise a shrink-proof +count + optional latest headline, with every headline in the tooltip. Checked against +`AgentErrorCount.SPEC.md`: + +- "Nothing when none" — `errors.length === 0 → null`. ✓ +- "A count, not the errors" — only headlines are surfaced; `detail` stays in the log. ✓ +- "Latest headline inline where the row has room" — `headline` prop gates it; `latest` is + `errors[errors.length - 1]`, non-null-asserted only after the length check, so safe. ✓ +- "Never truncated" — `shrink-0` on the container and icon; the inline headline alone carries + `min-w-0 truncate`, so the count keeps full width while the headline clips. ✓ +- Tooltip lists *every* headline joined by newlines under `whitespace-pre-line`. ✓ + +Scope note: the fold runs over the whole `events` prop, not `currentAgentEvents` — so a resumed +agent's count includes the earlier leg's errors. That matches the SPEC ("how many errors an +agent reported" — the agent, not the leg) and the callers pass the agent's own feed/archive, so +cross-agent contamination cannot occur through this component (the segment-slicing problem lives +with the channel owner, not here). + +Edge cases: empty `events` → null; an error event with an empty-string headline renders "· " — +producers always set a headline (typed as required `headline: string`), so not reachable; +duplicate headlines join fine. No state, no listeners, no keys. Rerenders recompute the fold — +cheap linear scan. + +## Functions (low-level) + +- **`AgentErrorCount({ events, headline })` (L10)** — described above. Inputs: the agent's event + feed and the room flag. Output: null or the alert span wrapped in a Tooltip. The + `TooltipTrigger render={...}` pattern matches the ui/tooltip primitive used across the + codebase. `role="alert"` on a statically rendered span is an a11y nicety more than a live + region (the count updates in place while the agent streams, so it does announce on change). + Verdict: correct. + +## Bugs found + +None found. diff --git a/packages/framework/dashboard/components/AgentErrorCount.test.BUG-ANALYSIS.md b/packages/framework/dashboard/components/AgentErrorCount.test.BUG-ANALYSIS.md new file mode 100644 index 000000000..4170060a9 --- /dev/null +++ b/packages/framework/dashboard/components/AgentErrorCount.test.BUG-ANALYSIS.md @@ -0,0 +1,36 @@ +# Bug analysis: packages/framework/dashboard/components/AgentErrorCount.test.tsx + +## Business logic (high-level) + +Four synchronous rendering tests, no mocks (the component and the real `agentErrors` fold both +run). Coverage matches `AgentErrorCount.test.SPEC.md` exactly: + +1. No errors → empty container. Uses a non-error event (`ready-for-merge`) rather than `[]`, so + the fold's filter is actually exercised. Can fail if the component rendered anything. ✓ +2. One error with `headline` prop → "1 error" (singular pinned) + headline text visible. ✓ +3. One error without `headline` prop → count alone; `queryByText` proves the headline is absent + from the DOM (not merely visually truncated — a stronger, honest assertion of the "tight row" + rule). ✓ +4. Two errors → "2 errors" (plural) + latest headline shown, earlier one asserted absent from + the inline row. ✓ + +Each test would fail if its behavior regressed (wrong pluralization, first-instead-of-last +headline, headline leaking into the tight row, or a stray zero-state render). Nothing async, so +no missing awaits; `cleanup` in `afterEach` prevents cross-test DOM bleed. + +Note on test 4's last assertion: `queryByText(/first thing broke/)` is null because the tooltip +content (which joins *all* headlines) is not mounted until the trigger is hovered — the +assertion therefore pins the inline row only, which is what the SPEC claims. If the tooltip +primitive ever rendered its content eagerly, this test would start failing for the wrong reason; +acceptable coupling, recorded here. + +## Functions (low-level) + +- **Test data** — events are cast through `FrameworkEvent[]` with the real `error` shape + (`kind`, `headline`); `detail` unused, matching the component's surface. Correct. +- **Assertions** — `getByText('1 error')`/`'2 errors'` are exact-match (headline lives in a + sibling span, so text-node splitting does not break the exact match). Correct. + +## Bugs found + +None found. diff --git a/packages/framework/dashboard/components/AgentFeed.BUG-ANALYSIS.md b/packages/framework/dashboard/components/AgentFeed.BUG-ANALYSIS.md new file mode 100644 index 000000000..921adb6bd --- /dev/null +++ b/packages/framework/dashboard/components/AgentFeed.BUG-ANALYSIS.md @@ -0,0 +1,42 @@ +# Bug analysis: packages/framework/dashboard/components/AgentFeed.tsx + +## Business logic (high-level) + +Thin presentational wrapper around `EventList`: choose between placeholder and log, and prepend +the lost-stream banner. Checked against `AgentFeed.SPEC.md`: + +- **Lost stream announced** — `lost` renders the warning banner in both the empty and populated + branches, wording separates "connection died" from "agent went quiet" and says the agent keeps + running. ✓ +- **Live follows / finished static** — pure prop plumbing: `stick` defaults true (live), + `openAt` forwarded only when set; `AgentView` passes `{stick:false, openAt:'end'}` for a + non-live feed. The actual scroll contract lives in `EventList`; nothing here can break it. ✓ +- **Gates stay answerable** — `projectId` is required and `agentId` forwarded, so `EventList` + can always render a `choice` event as an answerable panel; the type makes the #846 downgrade + impossible for any caller. ✓ +- **Empty states** — `emptyLabel` defaults to the live wording; the finished caller overrides it + ("This agent has no events."). ✓ + +Edge cases: `events` flipping empty→populated swaps placeholder for list (remount of EventList — +its own openAt/stick logic initializes then, which is exactly when it should); `tail` is only +forwarded in the populated branch, so a web agent's mirror box is absent while the feed is empty +— defensible ("rides the tail of the scroller" needs a scroller; the CloudAgentNotice above the +feed still points at the cloud session), transient (a web run always has opening events), so not +reported as a bug. The `{...(tail ? { tail } : {})}` guard is against `undefined`, not against a +self-nulling element (the `CloudMirrorRow` element is always truthy and nulls itself) — correct +either way. + +No state, no effects, no subscriptions — nothing to leak; `lost=true` with an empty feed shows +banner + placeholder together, which is coherent. + +## Functions (low-level) + +- **`AgentFeed(props)` (L11)** — inputs as typed; output: banner? + (placeholder | EventList). + Off-by-one/none: `events.length === 0` is the only branch point and both sides render the + banner. Conditional spread keeps `EventList`'s own defaults for `openAt`/`tail` when unset + (passing `openAt: undefined` explicitly could otherwise defeat a default parameter — the + spread pattern avoids that correctly). Verdict: correct. + +## Bugs found + +None found. diff --git a/packages/framework/dashboard/components/AgentHandoff.BUG-ANALYSIS.md b/packages/framework/dashboard/components/AgentHandoff.BUG-ANALYSIS.md new file mode 100644 index 000000000..3ce9f39e2 --- /dev/null +++ b/packages/framework/dashboard/components/AgentHandoff.BUG-ANALYSIS.md @@ -0,0 +1,103 @@ +# Bug analysis: packages/framework/dashboard/components/AgentHandoff.tsx + +## Business logic (high-level) + +Four exports composing the end-of-work handoff in the agent's action bar (SPEC: +`AgentHandoff.SPEC.md`): `HandoffSummary` (one-line verdict), `HandoffArm` (the live +pre-commitment checkbox), `HandoffActions` (the settled agent's next step / reason), and +`AgentHandoffDetails` (+`handoffExpandable`) behind the disclosure. Data comes from +`useAgentHandoff` (polled `onAgentHandoff` → `readAgentHandoff` in +`src/dashboard/agent-handoff.ts`) and, for the arm, from the event fold `handoffState` reduced +to a rung by `handoffFromStages`. + +Verified against the SPEC: + +- **Verdict** — "branch gone" vs "no changes" told apart; counts + DiffStat; "pushed" only when + no PR (the bar links the PR); see bug 1 for "merged". +- **Arming** — one checkbox, label names the rung in force (`Push branch` / `Open PR` / + `Open PR & merge`), untick → `local`, re-tick → `pr` (never restores merge) — exactly the + SPEC's ladder semantics; optimistic `pending` holds the click until the events echo it + (bounce-free, test-pinned). See bug 2 for the pending latch across agent switches. +- **Next step** — `prPending` → nothing (the "second PR" guard, #1028); open unmerged PR → + Merge PR; closed/merged PR → nothing; else Open PR; branch gone / empty (+named uncommitted + paths, first two + count, full list in `title`) / no remote → Reason. Order of the guards + matches the SPEC's own order. In-flight labels ("Merging…", "Opening PR…") keyed off + `pending`, failures surface through `useAgentHandoff`'s error (rendered by the callers). +- **Details** — commit list capped at 6, files and uncommitted at 10, each with "and N more"; + empty sections omitted; `handoffExpandable` refuses branch-gone and + nothing-changed-nothing-pending. All ✓. + +Concurrency/ordering: the arm's RPC failure path reverts `pending` (`.catch`), success waits +for the event round-trip; `busy` prevents double-fire during the RPC only, which is fine since a +second click during the pending window just re-sends. A failed `sendSetHandoff` reverts +silently — no error line — the SPEC only demands failure reporting for the next-step actions, +so noted here, not filed. `HandoffActions` is stateless; its `busy`/`pending` live in +`useAgentHandoff`, reset by `act`. + +Cross-file observation (fix would belong in `src/dashboard/agent-handoff.ts` L343-346): +`readAgentHandoff` computes `merged` purely as `git branch --list --merged base branch` +(ancestor check), while the framework's own merges are squash merges (`ghMergePr` uses +`--squash`), whose branch tip is never an ancestor — so `merged: true` occurs only for +externally merge-committed branches, and for those `base..branch` is empty ⇒ `commits.length +=== 0` ⇒ `empty: true`. Both facts feed bug 1. + +## Functions (low-level) + +- **`handoffExpandable(handoff)` (L29)** — `exists && (!empty || pendingFiles?.length)`. + Nullable-safe. Note: an `empty`-because-bookkeeping-only handoff can still carry non-empty + `commits`/`files`; when it also has `pendingFiles` the details pane then lists the + bookkeeping files — informative rather than wrong. Correct. +- **`HandoffSummary` (L34)** — null → null; `!exists` → "branch gone"; `empty` → "no changes"; + else counts + optional markers. Pluralization correct. Bug 1: the `merged` marker (L51) is + unreachable — see below. Otherwise correct. +- **`HandoffArm` (L67)** — `armed = handoffFromStages(state)`; `shown = pending ?? armed`; + effect clears `pending` when the events agree. `set` is optimistic with revert-on-reject. + The un/re-tick mapping `on ? 'pr' : 'local'` matches the SPEC. Bug 2: `pending`/`busy` are + never reset when `agentId` changes under the mounted component. Otherwise correct. +- **`Arm` (L134)** — label-as-hit-target checkbox inside a tooltip trigger; `onCheckedChange` + normalizes the indeterminate type to boolean. Correct. +- **`HandoffActions` (L168)** — guard order: no handoff → null; `prPending` → null; PR present + → Merge only for OPEN-and-not-merged; `!exists` → reason; `empty` → reason (+pending files + named via `namePending`); `!hasRemote` → reason; else Open PR. Edge: a PR with state + 'UNKNOWN' (offline `resolveAgentPr` fallback) offers nothing — conservative and safe against + the double-PR mistake. Edge: `merged` true with no PR falls through to the `empty` reason + ("Nothing committed — no PR to open."), slightly untrue for landed work but never offers a + wrong action; folded into bug 1's fix. Correct except as noted. +- **`namePending(paths)` (L234)** — first two joined, rest counted; `rest` arithmetic safe for + 1/2/3 paths (0 → no suffix). Correct. +- **`Reason` (L241)** — width-capped truncating span, optional `title` hover. Correct. +- **`AgentHandoffDetails` (L252)** — re-checks `handoffExpandable`; two-column grid only with + >1 section. Correct. +- **`Commits`/`Files`/`PendingFiles` (L271/L312/L293)** — slice + count remainder; keys by + sha/path (unique within a handoff); binary files say "binary" instead of a diffstat. All + correct. + +## Bugs found + +1. **L39/L51: a merged branch's verdict reads "no changes" — the "· merged" marker is dead + code.** `readAgentHandoff` can only report `merged: true` when the branch tip is an ancestor + of base (`git branch --list --merged`), and then `base..branch` is necessarily empty ⇒ + `commits.length === 0` ⇒ `empty: true` — so `HandoffSummary` early-returns "no changes" at + L39 and never reaches the `handoff.merged && '· merged'` span at L51. Scenario: an agent's + PR is merged on GitHub with a merge commit (or the user merges the branch locally); the + agent's bar then says "no changes", which reads as "the agent produced nothing" — exactly + the confusion the SPEC forbids ("work that landed says 'merged'"; branch-gone/no-changes/ + merged are "different facts"). The framework's own squash merges (`--squash` in + `ghMergePr`) sidestep the marker too (git `merged` stays false), so no path ever renders it. + Severity: minor. Fix: in `HandoffSummary`, check merged before the empty early-return — + e.g. `if (handoff.empty) return {handoff.merged ? 'merged' : 'no changes'}` — + and (cross-file, `src/dashboard/agent-handoff.ts` L346) consider `pr.state === 'MERGED'` as + `merged` so squash-merged work says it too. + +2. **L76-87: `HandoffArm`'s optimistic `pending` (and `busy`) leak across agent switches.** + The component is rendered at a stable position inside the always-mounted `AgentView` + (`working ? : …`, no `key`), so switching the rail from working agent A to + working agent B keeps the same instance and state. Scenario: untick A's box (pending + `'local'`), switch to B before A's event echo lands — B's box renders unticked ("hands off + nothing") although B is armed `pr`, and the latch never clears because the clearing effect + waits for `pending === armed` (`'local' === 'pr'` never holds); B's arming display stays + wrong until the user clicks it. The sibling `AgentComposer` resets its equivalents on + `[agentId]` for exactly this reason. Severity: minor (wrong pre-commitment display; the + daemon state is untouched). Confidence: medium (needs the switch inside the echo window, + but the wrong state then persists indefinitely). Fix: + `useEffect(() => setPending(null), [agentId])` in `HandoffArm`. diff --git a/packages/framework/dashboard/components/AgentHandoff.test.BUG-ANALYSIS.md b/packages/framework/dashboard/components/AgentHandoff.test.BUG-ANALYSIS.md new file mode 100644 index 000000000..83d4693d6 --- /dev/null +++ b/packages/framework/dashboard/components/AgentHandoff.test.BUG-ANALYSIS.md @@ -0,0 +1,62 @@ +# Bug analysis: packages/framework/dashboard/components/AgentHandoff.test.tsx + +## Business logic (high-level) + +Tests for the four handoff pieces, driven through a `Harness` that composes them the way +`AgentView` does — importantly, it runs the real `useAgentHandoff` hook (only the RPC modules +are mocked), so the busy/pending/error/reload plumbing is exercised, not stubbed. Coverage +matches `AgentHandoff.test.SPEC.md` bullet for bullet: + +- **Verdict/detail**: counts summarised; subjects/paths only when expanded (the harness gates + the details on `open && handoffExpandable`, mirroring the real bar's disclosure); branch name + never repeated; nothing rendered before the first read (never-resolving promise → empty + container — a genuine pin of the `loaded` flash guard). +- **Nothing to hand off**: "no changes" + no button + reason; `handoffExpandable` false; + uncommitted work named ("a.ts, b.ts and 2 more"), hover carries the full newline-joined list, + disclosure lists them; branch gone and no-remote reasons. +- **Next step**: only Open PR (never Push branch, also asserted when already pushed); click → + `sendOpenPullRequest('p1','run-1')` and never `sendPushBranch`; failure surfaces the RPC's + error string (through the real `useAction` failure branch — `{ok:false,error}` → error state); + open PR → Merge PR wired to `sendMerge`; merged/closed PR → neither button. +- **Arming**: exactly one checkbox, ticked; untick → `sendSetHandoff(...,'local')`; tick from + local → `'pr'`; push-only label; merge label ("Open PR & merge", and no plain "Open PR" — + `queryByText` is exact-match so the substring does not false-positive); the pending latch + (click, await RPC, rerender with the stale armed prop, box still unticked via `data-checked` + null). + +All async tests await their assertions; mocks are reset per test including re-priming resolved +values; `cleanup` runs. Each test fails if its pinned behavior regresses (I traced each +assertion to the branch it guards; none is vacuous). + +Weak spots (recorded, none rises to a bug): + +1. The merged-PR fixture (`{...worked, pushed: true, merged: true, empty: false, pr: {state: + 'MERGED'}}`) is a state the real reader cannot produce — `readAgentHandoff` derives + `merged: true` only when the branch is an ancestor of base, which forces `empty: true`. The + test still proves the button logic it targets (PR-state gating), but it is why the summary's + unreachable "· merged" marker (see `AgentHandoff.BUG-ANALYSIS.md` bug 1) went unnoticed: no + test renders a *producible* merged handoff, and no test asserts the "merged"/"pushed" + markers at all. +2. "push is offered only while the branch is unpushed" (L130) asserts `Push branch` absent — + but that string is absent in every state of the current one-button design; the test title + is a leftover from the two-button era. It cannot fail for the reason its name gives + (harmless: its other assertion, Open PR present for a pushed branch, is real). +3. The Harness renders `state.error` itself rather than through the real bar, so the failure + test pins the message's existence, not its placement — placement is AgentView's, tested + there. Acceptable division. + +## Functions (low-level) + +- **Mocks (L4-10)** — `onAgentHandoff` (reads) and the four control RPCs; shapes match the + real signatures (`sendSetHandoff` → void, actions → `{ok, error?}`). `sendPushBranch` is + mocked though nothing should call it — that is the point of the never-called assertion. +- **`worked` fixture (L16)** — a coherent producible handoff (exists, non-empty, unpushed, no + PR). Correct. +- **`Harness` (L32)** — described above; `open` defaults true so detail tests need no click. + Correct. +- **Per-test verdicts** — all correct; see weak spots for the two soft assertions. + +## Bugs found + +None found. (The unproducible merged fixture and the stale test name are recorded above as +test-quality notes, not defects — every assertion still pins a true behavior.) diff --git a/packages/framework/dashboard/components/AgentHistory.BUG-ANALYSIS.md b/packages/framework/dashboard/components/AgentHistory.BUG-ANALYSIS.md new file mode 100644 index 000000000..7a9dbf8c6 --- /dev/null +++ b/packages/framework/dashboard/components/AgentHistory.BUG-ANALYSIS.md @@ -0,0 +1,98 @@ +# Bug analysis: packages/framework/dashboard/components/AgentHistory.tsx + +## Business logic (high-level) + +The left rail on every route: brand, New launcher, Overview/Tickets/Projects nav, the recent +agents list (project-scoped or, on the Overview, pooled `recentAgents`), footer chrome. Checked +against `AgentHistory.SPEC.md` section by section. + +**Optimistic "starting…" row** — one state atom `{intent, known}` snapshotted when `startTick` +bumps (deliberately depending on `startTick` alone so `known` is the list *at Start*, which the +"survives an older run" test pins). Retired by: `landed` (any agent not in `known` — covers a +run that starts and fails inside one poll interval), a project switch, or a 20s deadline. +`showOptimistic` also re-checks `landed` so the stand-in and the real row never paint together +for the effect-queued frame — sound. The extra `!hasRunning` gate is consistent with the +daemon's one-run-per-project busy guard (a successful Start implies nothing else is running); +its only cost is a ≤2s window after a previous agent ends where the poll still says `running` +and the new start's row is briefly suppressed — transient, noted, not filed. Deadline effect +cleans its timeout; correct. + +**Highlight rules** — SPEC: "Exactly one of these carries the active highlight." In-project +launcher → New (`atProjectLauncher`); Overview → `projectId === null && !ticketsActive`; +Tickets → `ticketsActive`; agent row → selected or (followLive → newest running). The follow +case highlights only the newest running row (`agents` newest-first). Overview rows are never +active. **Bug 1**: on a ticket's own page (`/{projectId}/tickets/{slug}`, also `/plan`) the +route has a non-null `projectId` with `agentId === null`, so `atProjectLauncher` is true while +`ticketsActive` is also true — New and Tickets are both lit at once. Related (ambiguous, see +bug 3): on the Settings page (`projectId === null`, view `settings`) the Overview row is lit +although the Overview is not the current view. + +**Rows** — status word + dot: running pulses, parked (settled while running, or cloud +`waiting`) reads "waiting" with the still dot *only when `status === 'running'`* — a cloud- +waiting run has status `done`, so it gets no dot at all (bug 2); publishing (ended clean, +armed push, no report — `isMetaPublishing`) pulses green and is outranked by any cloud word +(`publishingNow = publishing && !cloud`, matching the "in cloud outranks publishing" comment); +cloud words from `cloudRunState` (waiting/in-cloud/merged, `done` falls through to the stored +status — correct, `status` is `done` in every `cloudState === 'done'` case by construction). +Meta line: project name leads only on the Overview; relative time; right cluster of glyphs +(other-host laptop, device, cloud, driver logo) each with tooltip + aria-label; driver via +`driverFromImpl` so `claude-web` rows still say Claude. Title overflow measured +(`scrollWidth > clientWidth + 1`) on `[intent]` — fine for a fixed-width rail; tooltip only +when overflowing, plain span otherwise (test-pinned). + +**New button** — in project / single project → direct start; none → add-project dialog; +several → picker with activated dots. Matches SPEC. `projects[0]!` guarded by the +`length === 1` branch condition. Correct. + +**Projects nav** — expandable list, red dot with `projectErrorTitle`-joined tooltip + sr-only +text for errors, activated/muted dots, Add project. Matches SPEC. + +Lifecycle/cleanup: the one `setTimeout` is cleaned up; no subscriptions. State that could go +stale across prop changes: `optimistic` (reset on projectId change — handled), `ProjectsNav +open`/`adding` (harmless persistence), `overflowing` (recomputed per intent). + +## Functions (low-level) + +- **`AgentHistory` (L50)** — see above. Edge: `crossProject` requires `recentAgents !== + undefined`, so a caller that omits it (the relay) falls back to the project list — safe. + `hasRecents` includes the optimistic row so "No agents yet." never sits under a starting row. + Verdict: bug 1 aside, correct. +- **`TicketsButton` (L297)** — plain nav row; stale comment ("Only rendered once a project is + selected" — actually gated on `onTickets`), behavior correct. +- **`OverviewButton` (L317)** — badge only when count > 0, singular/plural tooltip. Correct + in itself; its `active` input is wrong on the settings route (bug 3, fix at the callsite + expression L207 or in `App.tsx`). +- **`ProjectsNav` (L353)** — chevron rotation off `open`; error dot precedence (errors > + activated > muted) matches SPEC; tooltip joins every error. Correct. +- **`NewButton` (L441)** — three-way behavior; `start()` falls back to `onSelect(null)` when no + `onNewAgentInProject` (test-only path). Correct. +- **`AgentHistoryRow` (L521)** — `parked`, `inCloud`, `cloudWord`, `publishingNow` derivations + as analyzed; the badge chain `parked → in cloud → merged → publishing… → status` gives the + SPEC's precedence. Dot rendering: bug 2. Overflow measurement effect: correct, no cleanup + needed. Verdict: bug 2 aside, correct. + +## Bugs found + +1. **L177: two nav items highlighted at once on a ticket's detail/plan page.** Scenario: open + Tickets, click into a ticket — the route becomes `{view:'tickets', projectId:'p1', + agentId:null, ticketSlug}` (`App.tsx` `openTicket`); `atProjectLauncher = projectId !== + null && selectedAgentId === null && !followLive` is true, so New carries the active fill + while TicketsButton does too (`ticketsActive`). Contradicts the SPEC's "Exactly one of + these carries the active highlight" and mislabels the current view as the project + launcher. Severity: minor. Fix: `const atProjectLauncher = projectId !== null && + selectedAgentId === null && !followLive && !ticketsActive`. +2. **L603: a cloud-waiting run gets no still dot.** The dot block renders only for `status === + 'running'`, but a web run parked on a bridge question has status `done` with `cloudState + === 'waiting'` — the row reads "waiting" with no dot, while the SPEC says "'waiting' with a + still dot when the browser bridge holds a question its session is parked on" (and the local + parked row does show one, so the two waiting states render differently). Severity: minor. + Fix: render the still dot when `parked` regardless of status, e.g. change the guard to + `(status === 'running' || cloudState === 'waiting')`. +3. **L207 (with `App.tsx`): the Overview row stays highlighted on the Settings page.** + `active={projectId === null && !ticketsActive}` is true on `/settings` (route has + `projectId: null`, view `settings`), so Overview reads as the current view while the + Settings page is open — the SPEC ties each highlight to its view being open ("Overview + while the Overview is"). Severity: minor. Confidence: low — the SPEC's "exactly one" could + be read as requiring some row to stay lit, and Settings is footer chrome without an active + state of its own. Fix sketch: pass a `settingsActive` (or reuse a `view` prop) and exclude + it, as `ticketsActive` already does. diff --git a/packages/framework/dashboard/components/AgentHistory.test.BUG-ANALYSIS.md b/packages/framework/dashboard/components/AgentHistory.test.BUG-ANALYSIS.md new file mode 100644 index 000000000..aa1f08231 --- /dev/null +++ b/packages/framework/dashboard/components/AgentHistory.test.BUG-ANALYSIS.md @@ -0,0 +1,63 @@ +# Bug analysis: packages/framework/dashboard/components/AgentHistory.test.tsx + +## Business logic (high-level) + +Rail tests, mounted under the required `SidebarProvider`; RPC-bearing chrome (ThemeToggle, +NotificationsMenu, ConnectionIndicator) and the projects RPC stubs are mocked so jsdom never +fetches. Coverage vs. `AgentHistory.test.SPEC.md`: + +- **Status words** — running+pulse; settled → "waiting", pulse gone (asserts both the word and + the absence of `.animate-pulse` — honest, since the still dot has no pulse class); + publishing… (armed, no report) with pulse; plain done for reported/never-armed (two rows, + `getAllByText('done').length === 2` — cannot pass if either mislabels); terminal status never + relabelled waiting despite a stale `settledAt`. All genuinely pin the logic. ✓ +- **Starting placeholder** — highlight lands on the "starting…" row (checks `bg-accent` on it + AND its absence on New — the #784 regression pair); retires on a landed agent that was never + seen running (the `landed` rule; asserts both the placeholder's absence and the failed row's + presence); survives a pre-existing agent (the `known`-snapshot rule; done via rerender with + `startTick` 0→1, matching the real tick flow). ✓ +- **Where it runs** — device glyph label with name; other-host laptop glyph shown only for + `otherHost` rows (both positive and negative asserted, cross-project variant); cloud runs: + in-cloud vs stopped vs waiting vs merged/done trio (fresh/old `startedAt` drive the + 12h-window branch of `cloudRunState` both ways) plus glyph + driver label ("claude-web is + still Claude"). These pin `cloudRunState` integration, not just the component. ✓ +- **Scope** — empty Overview shows New + "No agents yet."; pooled recents name projects and + `onSelectRecent` gets `(projectId, agentId)`. ✓ +- **New** — one project → starts there; in-project → starts there; several+none → + `aria-haspopup` (menu, not immediate start). ✓ +- **Tickets** — offered/absent per `onTickets`; opens; `aria-current` active with Overview not + claiming it (the `projectId === null` shared-state regression). ✓ (Does not cover the + ticket-*detail* route where New and Tickets both light up — that needs `projectId` non-null + with `ticketsActive`, a combination no test renders; the bug filed in + `AgentHistory.BUG-ANALYSIS.md` bug 1 slips through exactly here.) +- **Title tooltip** — overflow simulated by stubbing `scrollWidth`/`clientWidth` prototype + getters (jsdom measures 0), full prompt in tooltip via the shared `hoverTooltip` util; the + fits case fires hover events and asserts no tooltip role appears — a real negative since the + plain-span branch has no trigger wiring. Spies restored in `finally`. ✓ +- **Project health** — red dot + `projectErrorTitle` wording + raw message in tooltip; healthy + project keeps "Activated:" sr-only text and no `.bg-danger`. ✓ + +Async hygiene: the two tooltip tests await `hoverTooltip`/`findByText`; everything else is +synchronous rendering. `cleanup` after each; hoisted mocks initialized before the dynamic +import of the component under test (the documented reason for the `await import`). + +Gaps (no SPEC claim broken): no test for the cloud-waiting row's dot (the SPEC-required still +dot is missing in the source — nothing here would catch it), none for the optimistic row's 20s +deadline (timer-based; reasonable omission), none for the two-highlight ticket-detail case +noted above. + +## Functions (low-level) + +- **`agent(over)` (L27)** — minimal valid `AgentMeta`; `updatedAt`/`startedAt` fixed ISO + strings keep `formatRelative` deterministic enough (it renders a relative phrase, never + asserted on). Correct. +- **`renderRail` (L39)** — SidebarProvider wrapper; rerenders in tests re-wrap manually — + consistent. Correct. +- **`proj(id,name)` (L223)** — activated ProjectSummary. Correct. +- **Assertions on class names** (`bg-accent`, `.animate-pulse`, `.bg-danger`) — couple the + tests to styling tokens, but each token *is* the behavior under test (highlight, pulse, error + dot); acceptable and stable within this design system. Correct. + +## Bugs found + +None found. diff --git a/packages/framework/dashboard/components/AgentOverview.BUG-ANALYSIS.md b/packages/framework/dashboard/components/AgentOverview.BUG-ANALYSIS.md new file mode 100644 index 000000000..3be33650e --- /dev/null +++ b/packages/framework/dashboard/components/AgentOverview.BUG-ANALYSIS.md @@ -0,0 +1,56 @@ +# Bug analysis: packages/framework/dashboard/components/AgentOverview.tsx + +## Business logic (high-level) + +Stateless at-a-glance strip above a feed (rendered by `ProjectHome` when the project has +events): status pill + session name, error count with latest headline, session deep link. +Checked against `AgentOverview.SPEC.md`: + +- **Each part appears only once its fact arrived; all absent → render nothing** — the + `!sessionLink && !status && errors.length === 0 → null` guard plus per-part conditionals. ✓ + (`agentStatusPill` is null until the agent has a name/state/ending, `describeSessionLink` + null until a deep link exists, errors empty until one lands.) +- **Errors: count kept here, rows stay in the log** — delegates to `AgentErrorCount` with + `headline`. ✓ +- **Session link labelled for what it opens** — partially: `describeSessionLink` only ever + returns a *per-session* deep link ("Open session (id) ↗") and returns null for the generic + app entry — it never "offers the driver's app", which both this SPEC and this file's own + comment (L19-21: "the generic app entry (claude.ai/code) is shown as 'Open Claude Code' + with the id surfaced separately") say it should. See bug 1 (cross-file: the behavior lives + in `dashboard/lib/session-link.ts`). + +Data-shape checks: `sessionInfo`/`agentProgress`/`agentErrors`/`agentStatusPill` all +total functions over the events array (empty → null/empty), so no reachable crash. The +status row shows the dot+label even when `progress.sessionName` is absent (name conditionally +rendered inside) — consistent with "each part appears only once its fact has arrived". The +link opens in a new tab with `rel="noreferrer"` — no window.opener leak. No state, effects, or +subscriptions. + +One structural note: `errors.length > 0` gates a wrapper around `AgentErrorCount`, which +re-checks the same condition internally — harmless duplication that prevents an empty wrapper +div from adding grid gap. Fine. + +## Functions (low-level) + +- **`AgentOverview({ events })` (L13)** — inputs: the (possibly foreign-project-scoped) live + feed; outputs: the strip or null. Folds are all linear scans; recomputed per render, cheap. + Edge cases: empty events → all folds empty → null ✓; a failed agent's pill carries + `failed — ` with danger tones (from `agentStatusPill`, ranked ending-over-progress) + ✓; the grid gives every child `md:col-span-2`, so the two-column template is inert — layout + quirk, not behavior. Verdict: correct except the delegated link behavior (bug 1). + +## Bugs found + +1. **Cross-file (fix in `packages/framework/dashboard/lib/session-link.ts` L27, or this SPEC): + the generic session link is dropped instead of being offered as the driver's app.** Scenario: + a headless Claude run records the default generic `claude.ai/code` session link (a literal + `sessionLink` with no `{sessionId}`); `describeSessionLink` returns null for any href that + does not embed the session id, so the overview shows no link at all — while + `AgentOverview.SPEC.md` says "where the driver has no per-session address, it offers the + driver's app rather than pretending to be a deep link to that session", and this component's + own JSDoc describes the same "Open Claude Code" fallback. Severity: minor. Confidence: low — + `session-link.ts`'s header comment deliberately argues the generic entry "isn't worth an + action", so either the lib regressed against the SPEC or the SPEC and this file's comment + are both stale; the orchestrator should decide which document wins. Fix sketch: have + `describeSessionLink` return `{ href, label: 'Open ↗' }` for a literal link (id + shown separately), or update the SPEC + comment to the show-nothing rule. diff --git a/packages/framework/dashboard/components/AgentView.BUG-ANALYSIS.md b/packages/framework/dashboard/components/AgentView.BUG-ANALYSIS.md new file mode 100644 index 000000000..64a5db298 --- /dev/null +++ b/packages/framework/dashboard/components/AgentView.BUG-ANALYSIS.md @@ -0,0 +1,91 @@ +# Bug analysis: packages/framework/dashboard/components/AgentView.tsx + +## Business logic (high-level) + +One agent's page — stable frame for running and finished (#1026), per `AgentView.SPEC.md`. Key +mechanisms, each checked: + +**Event source selection** — `shown = live ? events : archived?.length && !feedAhead ? archived +: events`. Empty archive never replaces on-screen events (#1383) ✓; stale archive never hides a +resumed leg — `feedAhead` prefers the channel when it holds more (#1460) ✓; foreign-journal +guard: `sameJournal` matches the archive's first event as fingerprint (JSON.stringify equality +— events are plain JSONL rows, so structural equality is sound; an unloaded/empty archive keeps +the show-the-feed fallback, transiently showing the root journal until the archive lands, which +the SPEC accepts) ✓. `archiveBehind` bumps a dep to re-read the archive while the feed is ahead, +which is how archive-only epilogue events (clean run's `handoff`) reach the screen ✓ — but see +bug 2 for a residual stall. + +**`feedLive`** — `live || (feedAhead && isAgentActive(events))`: the feed's own verdict drives +the scroll contract and the composer slot during the ≤2s poll window after a Resume ✓ (the +composer's Stop takes over the moment the first event lands, per SPEC). + +**working vs live (#1173)** — `working = live && !agentSettled(events)`; the arm checkboxes +show while working, the handoff actions once settled; the handoff read enabled `!working`; +`showHandoff` additionally waits for `handoff.loaded` so the summary swaps exactly once ✓. +`working` reads the *channel* events (not `shown`) as the comment demands — correct: while live +they are the same events, and once not live `working` is false regardless. + +**outcome** — `live ? undefined : agentOutcome(shown)`; during the resume window the newest +segment has no `end`, so `agentOutcome` is undefined — the composer's `resuming` latch covers +the flicker (#1460) ✓. + +**armed handoff** — `handoffState(shown, armedDefault)` seeded from the agent record's mirror +(#1376) so a tab opened mid-run reads the same arming ✓; the failed auto-handoff is surfaced on +the bar (`armed.result?.outcome === 'failed'`) ✓. + +**Retained worktree / removal** — `retained` refetches per `[projectId, agentId, live]` +(`useLoaded` resets to `[]` on dep change), `removed` clears the offer locally after a removal +✓ for the same agent — but `removed` never resets on an agent switch (bug 1). + +**Target notices** — ActionsRunNotice / CloudAgentNotice (+ CloudMirrorRow tail) / +RemoteAgentNotice each rendered unconditionally and self-gating on `target` ✓; `lost` banner +delegated to AgentFeed ✓; empty states split "Loading agent…" (finished, archive pending) vs +the feed's own waiting label ✓. + +Statefulness across agent switches (the component is never remounted — `App.tsx` renders it +unkeyed): `archived`/`retained` reset via dep-keyed hooks; `removed` (bug 1), `archiveBehind` +(bug 2), `open` (deliberate-looking persistence of the disclosure; harmless), and `changes` +(stale counts can flash on the bar for up to one poll after switching between two working +agents before the new `AgentChanges` reports — self-correcting, noted only) do not. + +## Functions (low-level) + +- **`AgentView(props)` (L30)** — orchestration only; every derived value analyzed above. +- **`archived` read (L89)** — `!live && agentId ? () => onAgent(projectId, agentId) : null`; + dep list includes `archiveBehind` for the re-read. Correct. +- **`retained` read (L97)** — enabled on the same condition; `includes(agentId)` guarded by + null/undefined checks. Correct. +- **`onChangesSummary` (L117)** — identity-stable via useCallback, equality guard prevents + render loops with `AgentChanges`. Correct. +- **`sameJournal`/`feedAhead`/`shown` (L141-144)** — analyzed above; the `!archived?.length` + clause makes an empty archive count as same-journal, which combined with `shown`'s + `archived?.length &&` guard keeps the live events on screen. Correct. +- **archive-behind effect (L145)** — fires only when `!live && archived !== null && feedAhead`; + see bug 2. Otherwise correct. +- **`session`/`progress`/`outcome`/`armed` folds (L153-162)** — all over `shown`, so a finished + agent reads its archive. Correct. +- **Render (L168-263)** — bar summary swap (`showHandoff`), details/changes/handoff panes gated + on `open`/`working`, feed props for finished (`stick:false, openAt:'end'`, empty label), + composer receives `feedLive`/sessionId/driver/outcome. Matches SPEC. Correct. + +## Bugs found + +1. **L98: `removed` is never reset when the agent changes — the next agent's Remove offer is + wrongly hidden.** Scenario: open finished agent A (worktree retained), press Remove + (`onWorktreeRemoved` → `removed = true`), then select finished agent B which also retained + its worktree — `AgentView` stays mounted (no `key` in `App.tsx`), `retained` refetches and + includes B, but `hasWorktree = !live && !removed && …` stays false, so the bar never offers + to remove B's worktree (until a full page reload). Contradicts the SPEC ("Whether a finished + agent still has its worktree decides whether the bar offers to remove it"). Severity: minor. + Confidence: high. Fix: `useEffect(() => setRemoved(false), [agentId])` (mirroring the reset + pattern AgentComposer uses for its latches). +2. **L88/L145: `archiveBehind` carries over across agent switches and can stall the archive + re-read on an exact length collision.** It is only ever set to `events.length` and never + reset, so after leaving agent A with `archiveBehind = N`, opening agent B whose channel feed + also holds exactly N events while its archive is behind makes `setArchiveBehind(N)` a no-op + — no re-read fires, and B's archive-only epilogue events (the `handoff`/PR line) never reach + the screen until some new event changes the length or the user refreshes; the SPEC promises + they arrive "without the user refreshing". Severity: minor. Confidence: low (requires the + equal-length coincidence). Fix: reset it on agent change (`useEffect(() => + setArchiveBehind(0), [agentId])`) or make the trigger a fresh object token instead of the + length. diff --git a/packages/framework/dashboard/components/AgentView.test.BUG-ANALYSIS.md b/packages/framework/dashboard/components/AgentView.test.BUG-ANALYSIS.md new file mode 100644 index 000000000..82096e494 --- /dev/null +++ b/packages/framework/dashboard/components/AgentView.test.BUG-ANALYSIS.md @@ -0,0 +1,56 @@ +# Bug analysis: packages/framework/dashboard/components/AgentView.test.tsx + +## Business logic (high-level) + +Focused suite for AgentView's one hard decision — which event log a finished agent shows — +matching `AgentView.test.SPEC.md`'s six bullets one-to-one. The frame is stubbed +(AgentActionBar reduced to its `actions` slot so the handoff cluster stays reachable; +AgentComposer to null); reads/control RPCs and the preferences module are mocked so jsdom never +fetches. The six tests: + +1. **Swap to archive once it has events** — archived text appears, channel text gone. Pins the + `shown` preference for a caught-up archive. ✓ +2. **Empty archive never replaces (#1383)** — `onAgent` resolves `[]`; asserts the channel line + stays and the "no events" label does not appear. Awaits the read first, so the negative + assertion is not vacuous. ✓ +3. **Nothing anywhere → "This agent has no events."** — also proves the empty label is the + caller-supplied finished-state one. ✓ +4. **Stale archive never hides a resumed leg (#1460)** — events = archive + new `session` + boundary + new line; asserts the streamed line renders while `live` is false. Pins + `feedAhead` + `feedLive`. ✓ +5. **Foreign journal never beats the archive (#1460)** — a longer feed whose first event + differs from the archive's; asserts the archive line renders and the foreign line does not. + Pins the fingerprint guard. ✓ +6. **Archive catches up and takes back over** — `onAgent` resolves stale-then-full + (`mockResolvedValueOnce` then `mockResolvedValue`); waits for ≥2 calls (the + `archiveBehind`-triggered re-read) and for the epilogue "branch pushed" text, which is + rendered by the real `HandoffSummary`... actually by the feed's handoff row — either way it + only exists in the full archive, so the assertion genuinely requires the re-read + takeover. + ✓ + +Each test drives the logic through public props and mocked RPC answers — no internal state is +poked, so the tests would survive a refactor of the mechanism while still failing on behavior +regressions. All `waitFor`s awaited; `vi.clearAllMocks()` plus re-priming of the two reads whose +default (`undefined`) would otherwise break `useAgentHandoff`/retained. + +Coverage notes (consistent with the test SPEC's narrow scope): the Resume-offer rules are +explicitly delegated to AgentComposer's suite (closing comment); `working`-vs-`live` gating, +the removed-worktree flow (where `AgentView.BUG-ANALYSIS.md` bug 1 lives), the notices, and the +`armedDefault` seeding are untested here — none claimed by the test SPEC. + +## Functions (low-level) + +- **Mocks (L5-32)** — every `rpc/reads` symbol AgentView's tree imports is provided; the + bridge/choice mocks exist because the feed renders inline gates. Shapes match the real + signatures. Correct. +- **Fixtures (L36-41)** — `view()` defaults to a finished agent (`live={false}`) with one + channel line; overrides per test. `LIVE_EVENTS`/`ARCHIVED` differ in first event, which is + what makes them distinct journals for the fingerprint — deliberate and load-bearing in test + 5; in tests 4/6 the resumed/ahead feeds are built by *extending* `ARCHIVED`, keeping the + fingerprint equal — also load-bearing. Correct. +- **beforeEach (L43)** — `mockResolvedValue` after `clearAllMocks` restores the two reads used + unconditionally by hooks. Correct. + +## Bugs found + +None found. diff --git a/packages/framework/dashboard/components/Agents.BUG-ANALYSIS.md b/packages/framework/dashboard/components/Agents.BUG-ANALYSIS.md new file mode 100644 index 000000000..69fdf5567 --- /dev/null +++ b/packages/framework/dashboard/components/Agents.BUG-ANALYSIS.md @@ -0,0 +1,45 @@ +# Bug analysis: packages/framework/dashboard/components/Agents.tsx + +## Business logic (high-level) + +The Overview's Agents card (#1139): props-only projection of `ActiveAgent[]` (the daemon's +"working right now" pool, cloud sides included). Checked against `Agents.SPEC.md`: + +- **Only what is working now** — the component renders exactly what `working` holds; the + filtering (running + cloud in-cloud/waiting, finished excluded) is the server projection's + job (`src/dashboard/overview.ts`), and the card adds nothing. Loading state distinct from + empty ("Loading…" vs "No agents working right now.") ✓ — the SPEC's "while the list is still + being fetched it says so". +- **A row opens the agent itself** — `onOpen` calls `onSelectAgent(projectId, agentId)`, both + ids, never just the project (#1189). The whole line is the button; no per-row hint (the + comment explains the tooltip-collision rationale). ✓ +- **Row contents** — label via `activeLabel`, cloud word ("waiting"/"in cloud"), project name, + "from " only when `host` set (the projection only sets it for other machines' runs), + age with exact timestamp on hover (`formatAge`/`formatDateTime`), rendered only when + `updatedAt` exists (it is optional on `ActiveAgent`). ✓ + +Edge cases: key `${projectId}:${agentId}` unique across projects ✓; `activeLabel` trims each +candidate so whitespace-only intent falls through (test-pinned) and ends at `projectName` +(always present) so a row is never blank ✓ — the SPEC's exact fallback chain +(intent → session name → scope → project). `cloud` chip text maps the two values explicitly; +an impossible third value would render nothing rather than lie. No state, effects, or +subscriptions; re-render safe. + +One SPEC nuance: the SPEC's row description mentions "how long ago it was last active" — +`updatedAt` is optional in the type, and a row without it simply omits the age instead of +rendering "Invalid Date"; correct handling of the optional. + +## Functions (low-level) + +- **`Agents({ working, loading, onSelectAgent })` (L17)** — three-way body: loading / empty / + list. Order matters (loading checked first) and matches "loading is not the same as empty". + Correct. +- **`AgentRow` (L63)** — one `