feat(chat): surface failed project switch inline (5-2) - #459
Merged
Conversation
Story 5-2 (Drawer Project Switcher) was a verify-and-close-gaps story: the drawer, its MobileChatShell mounting, the switch_project WS handler, the project registry mirror, projects_changed live-sync, and the queuedProjectSwitchId "Queued" badge were already delivered (PRs #449, #456). This commit verifies them against the 5-2 ACs and closes the remaining gaps. Gap closed (T2.3, option a): a rejected/failed project switch was toast-only and the "Queued" badge just disappeared. Add a transient inline "Failed" badge: - useAcpStore gains a failedProjectSwitchId field (mirrors queuedProjectSwitchId; no second state mechanism) + a setFailedProjectSwitch action. - applyFailedProjectSwitch sets failedProjectSwitchId on the project_switch_failed event (alongside clearing queued + toast). - switchProject clears it at the start (retry self-heals); applyCompletedProjectSwitch clears it on success. - ProjectSwitcherDrawer renders a red AlertCircle + "Failed" label (switching > queued > failed > active), keeps the failed row retryable, and clears the badge when the drawer closes. Test gaps closed: - ProjectSwitcherDrawer.test.tsx: add failedProjectSwitchId/ setFailedProjectSwitch to the store mock, a sonner toast spy, and two tests (rejected sync switch → inline Failed + toast + stays open + retryable; queued→failed transition replaces Queued with Failed). - MobileChatShell.test.tsx: flip isTauriContext via a mutable ref so a web-mode test asserts the "Switch project" trigger toggles the drawer open/closed, and a Tauri-mode test asserts the trigger is hidden (desktop non-regression). No new dependencies, no server-side changes, no second drawer/switch/ data-path. bun run lint/typecheck/test green (one unrelated WorkspaceLayout waitFor flake under heavy load; passes in isolation).
✅ PR description satisfies the templateThanks for filling it in. |
Contributor
📝 WalkthroughWalkthroughProject switching now records failed attempts, clears failure state across retries and drawer closure, and displays a retryable ChangesProject switch failure state
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ProjectSwitcherDrawer
participant acp-store
participant ACP
participant sonner
ProjectSwitcherDrawer->>acp-store: Start project switch
acp-store->>ACP: Process switch request
ACP-->>acp-store: project_switch_failed
acp-store-->>ProjectSwitcherDrawer: failedProjectSwitchId
ProjectSwitcherDrawer->>sonner: Show rejection message
ProjectSwitcherDrawer-->>ProjectSwitcherDrawer: Render Failed badge and retry action
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/renderer/components/chat/ProjectSwitcherDrawer.tsx`:
- Around line 45-47: Update the cleanup effect in ProjectSwitcherDrawer to also
react to failedProjectSwitchId, clearing the stored failure when the drawer is
closed even if the failure arrives after closure. Add a regression test covering
a queued project switch that fails after closing, ensuring no stale failure
badge appears when reopening.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b66cee09-42c8-4a95-abc8-81c4c93e8e2e
📒 Files selected for processing (4)
src/renderer/components/chat/ProjectSwitcherDrawer.test.tsxsrc/renderer/components/chat/ProjectSwitcherDrawer.tsxsrc/renderer/components/mobile/MobileChatShell.test.tsxsrc/renderer/stores/acp-store.ts
gnoviawan
added a commit
that referenced
this pull request
Jul 29, 2026
Address the CodeRabbit review on #459: the inline-Failed-badge cleanup effect now also depends on `failedProjectSwitchId`, so a failure that arrives AFTER the drawer closes (e.g. a queued switch rejected while closed) is cleared immediately — no stale red badge resurfaces on reopen. The guard now only fires when there is an actual failure to clear (avoids redundant `setFailedProjectSwitch(null)` calls) and references the dep so `useExhaustiveDependencies` is satisfied. Adds a regression test: a queued switch that fails after closure leaves no stale "Failed" badge on reopen.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/components/chat/ProjectSwitcherDrawer.test.tsx (1)
5-12: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake the failure-state mock update its observed state.
setFailedProjectSwitchis only a spy, while Line 212 manually setsfailedRef.current = null. This means the test can pass even if the cleanup action does not actually clear the store state. Make the mock action updatefailedRef.current, remove the manual reset, and assert that the ref is null afterwaitFor.Also applies to: 189-217
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/components/chat/ProjectSwitcherDrawer.test.tsx` around lines 5 - 12, The failure-state mock does not update the observed ref, allowing cleanup tests to pass without clearing store state. Update setFailedProjectSwitch in the hoisted mock to assign failedRef.current, remove the manual failedRef.current reset in the affected test, and assert that failedRef.current is null after waitFor.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pr-body-current.md`:
- Line 74: Update the checklist entry to report five new tests, reflecting three
ProjectSwitcherDrawer tests and two MobileChatShell tests instead of four.
---
Outside diff comments:
In `@src/renderer/components/chat/ProjectSwitcherDrawer.test.tsx`:
- Around line 5-12: The failure-state mock does not update the observed ref,
allowing cleanup tests to pass without clearing store state. Update
setFailedProjectSwitch in the hoisted mock to assign failedRef.current, remove
the manual failedRef.current reset in the affected test, and assert that
failedRef.current is null after waitFor.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 91938a64-6f23-4c9a-b8aa-d52859e4cb0c
📒 Files selected for processing (3)
pr-body-current.mdsrc/renderer/components/chat/ProjectSwitcherDrawer.test.tsxsrc/renderer/components/chat/ProjectSwitcherDrawer.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/components/chat/ProjectSwitcherDrawer.tsx
Address the CodeRabbit review on #459: the inline-Failed-badge cleanup effect now also depends on `failedProjectSwitchId`, so a failure that arrives AFTER the drawer closes (e.g. a queued switch rejected while closed) is cleared immediately — no stale red badge resurfaces on reopen. The guard now only fires when there is an actual failure to clear (avoids redundant `setFailedProjectSwitch(null)` calls) and references the dep so `useExhaustiveDependencies` is satisfied. Adds a regression test: a queued switch that fails after closure leaves no stale "Failed" badge on reopen.
gnoviawan
force-pushed
the
feat/drawer-project-switcher-5-2
branch
from
July 29, 2026 08:41
0545bd4 to
788aa1c
Compare
Owner
Author
|
Addressed the remaining CodeRabbit test-mock finding in e6be5ea: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Story 5-2 (Drawer Project Switcher) was a verify-and-close-gaps story. The drawer (
ProjectSwitcherDrawer.tsx), itsMobileChatShellmounting, theswitch_projectWS handler, the project registry mirror, theprojects_changedlive-sync, thequeuedProjectSwitchIdstore field, and the "Queued" badge were already delivered by #449 (mobile ACP chat layout + web/remote project-list mirror, switch & live-sync) and #456 (multi-project switching and session persistence). The BMAD sprint still marked 5-2backlog/ready-for-devbecause the formal story was never "started" even though its deliverables shipped.This PR verifies the existing implementation against the 5-2 ACs and closes the remaining gaps — it does not create a second drawer, switch mechanism, or data path.
The real problem it solves: a rejected/failed project switch was toast-only — the "Queued" badge just disappeared and the only failure signal was a transient toast, which is easy to miss on mobile. AC2 requires the drawer to surface the queued/rejected state clearly. This PR adds a transient inline "Failed" badge on the failed project row, plus closes the test-coverage gaps for the shell trigger and the failed mid-turn state.
Related Issue
No related issue — this is BMAD sprint story 5-2 (Epic 5: Mobile/Responsive ACP Chat). Builds directly on #449 and #456 (the PRs that delivered the existing drawer + switch pipeline). No prior PR addresses the inline failed-state gap or the two test gaps closed here (searched open + closed PRs to
dev).Type of Change
What Changed
1. Inline "Failed" indicator for a rejected/failed project switch (T2.3, option a)
The existing
queuedProjectSwitchIdfield surfaced the queued state inline (Clock3+ "Queued"). Failure was toast-only. Added a symmetricfailedProjectSwitchIdfield onuseAcpStore(no second state mechanism — mirrors the existing queued pattern):src/renderer/stores/acp-store.tsfailedProjectSwitchId: string | null+ asetFailedProjectSwitch(projectId)action.applyFailedProjectSwitch(theproject_switch_failedevent handler) now setsfailedProjectSwitchId: event.projectIdalongside clearingqueuedProjectSwitchId+toast.error(covers the async queued-then-failed path).switchProjectclearsfailedProjectSwitchIdat the start (a retry self-heals the stale badge);applyCompletedProjectSwitchclears it on success.src/renderer/components/chat/ProjectSwitcherDrawer.tsxfailedProjectSwitchId/setFailedProjectSwitch; on a sync rejection (switchProjectthrows — e.g.no_agentor a rejected reply) thehandleSwitchcatch now sets the failed id + still toasts.AlertCircle+ "Failed" label on that row (priority: switching > queued > failed > active). The failed row stays enabled so it's retryable.useEffectclears the badge when the drawer closes, so a stale red indicator doesn't reappear on the next open.Reuses existing
lucide-reactAlertCircleand thetext-destructiveTailwind token — no new dependencies, no new UI library.2. Test-coverage gaps closed (AC3)
src/renderer/components/chat/ProjectSwitcherDrawer.test.tsxuseAcpStoremock withfailedProjectSwitchId/setFailedProjectSwitchand added asonnertoast.errorspy.setFailedProjectSwitch(id)+toast.error+ drawer stays open + inline "Failed" badge on a retryable (non-disabled) row.src/renderer/components/mobile/MobileChatShell.test.tsxisTauriContextmutable per-test (via atauriRef) and stubbedProjectSwitcherDrawerso the shell test focuses on trigger wiring.FolderGit2"Switch project" trigger exists, opens the drawer on click, andonOpenChange(false)closes it.Unchanged (verified against ACs, no edits):
MobileChatShell.tsx,use-mobile-web-shell.ts,WorkspaceLayout.tsx,use-projects-persistence.ts,ChatInputBar.tsx,AgentChatPanel.tsx,web-projects.types.ts, allsrc-tauri/server code.How It Was Tested
bun run ci(Biome lint + format + imports, strict mode) — changed files pass Biomeci --diagnostic-level=error(the pre-commit hook also confirmed "Biome check passed" on staged files). The repo has pre-existing diagnostics in untouched files, none introduced here.bun run typecheck— green (typecheck:node+typecheck:web+typecheck:test).bun run test— green: 2478 passed, 43 skipped. One unrelatedWorkspaceLayout.test.tsx"Color theme picker"waitFortimeout under heavy parallel load; it passes in isolation (15 passed) and is unrelated to project switching.cargo clippy --all-targets -- -D warnings(in src-tauri) — N/A, no Rust changes (the optional server-side fail-first, T2.4, was explicitly skipped / supervisor-gated).cargo test(in src-tauri) — N/A, no Rust changes.useMobileWebShell()isfalsein Tauri,FolderGit2hidden in Tauri) and the web bundle (dist-web/) builds. The inline "Failed" badge is web/remote-only (!isTauriContext()), so a browser click-through at ≤767px via the in-process Remote Access server is pending; behavior is covered by the new renderer tests.Screenshots or Recordings
The "Failed" badge is web/remote-only (mounted only
!isTauriContext()at ≤767px) and requires the in-process Remote Access server + a mobile-width browser to view. Will add a recording on request; the render logic + transitions are asserted inProjectSwitcherDrawer.test.tsx.CI & Review Gate
I'll wait for all CI checks (PR Validation, Rust Checks, Build Verification, Security Scans) and the CodeRabbit/Claude review to complete, then address every finding before this is ready for merge.
Checklist
feat(chat): …)Summary by CodeRabbit
New Features
Bug Fixes
Tests