ADE-96: Auto-create lane loading and error states break in new chat pane#513
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThis PR introduces a user-facing clipboard reminder toggle preference, adds infrastructure for persisting draft launch job state across component remounts, and refactors AgentChatPane to use scoped storage with stale detection and lifecycle guards to prevent post-unmount state mutations. ChangesClipboard reminder and draft job persistence
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
|
@copilot review but do not make fixes |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/components/chat/AgentChatPane.tsx (1)
2373-2395:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the raw draft kind in the job scope key.
draftLaunchJobsScopeKeycurrently reusesworkDraftStorageKind, which intentionally collapses both"chat"and"cli"into"work-start". That makes chat and CLI draft jobs share the same store bucket again, so a launch started from one surface can appear or be dismissed from the other. This PR’s isolation fix needs the actualworkDraftKindhere, not the normalized storage key.Suggested fix
const draftLaunchJobsScopeKey = useMemo( () => [ "draft-launch-jobs", projectRoot?.trim() || "project", laneId ?? "no-lane", surfaceProfile, - workDraftStorageKind, + workDraftKind, ].map(encodeURIComponent).join(":"), - [laneId, projectRoot, surfaceProfile, workDraftStorageKind], + [laneId, projectRoot, surfaceProfile, workDraftKind], );🤖 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 `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx` around lines 2373 - 2395, draftLaunchJobsScopeKey is using the normalized workDraftStorageKind which collapses "chat"/"cli" into "work-start" and causes shared buckets; change the array building draftLaunchJobsScopeKey to use the raw workDraftKind (the incoming prop/variable) instead of workDraftStorageKind, and update the useMemo dependency array to include workDraftKind (remove or keep workDraftStorageKind only if still referenced elsewhere) so chat and CLI draft jobs are isolated; target the draftLaunchJobsScopeKey definition and its dependencies in AgentChatPane.tsx.
🤖 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 `@apps/desktop/src/renderer/components/chat/AgentChatPane.test.tsx`:
- Around line 3774-3784: The test is asserting the "creating" copy while
suggestLaneName is still unresolved; instead, assert the naming-phase UI first
(i.e., check the UI state for the new "naming-lane" phase while suggestLaneName
is pending) using the same render/unmount flow (renderAutoCreateDraftPane /
rendered.unmount), then resolve or wait for createLane to start and only after
that assert the "creating-lane" copy; target the mocked suggestLaneName and
createLane flows so you explicitly check the naming-phase text before resolving
suggestLaneName and the creating-phase text after createLane begins.
In `@apps/desktop/src/renderer/lib/draftLaunchJobs.ts`:
- Around line 96-106: pruneDraftLaunchJobs currently forces at least one
terminal slot when any active job exists, which can let the returned array
exceed MAX_DRAFT_LAUNCH_JOBS; to enforce a hard cap, change the
remainingTerminalSlots calculation in pruneDraftLaunchJobs to use
Math.max(MAX_DRAFT_LAUNCH_JOBS - active.length, 0) (so when active.length === 0
you get the full MAX, otherwise you reserve zero or more terminal slots up to
the remaining capacity), remove the forced minimum of 1, and add a brief comment
next to MAX_DRAFT_LAUNCH_JOBS or the pruneDraftLaunchJobs function documenting
the intended total-cap semantics so callers like AgentChatPane aren’t surprised.
---
Outside diff comments:
In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx`:
- Around line 2373-2395: draftLaunchJobsScopeKey is using the normalized
workDraftStorageKind which collapses "chat"/"cli" into "work-start" and causes
shared buckets; change the array building draftLaunchJobsScopeKey to use the raw
workDraftKind (the incoming prop/variable) instead of workDraftStorageKind, and
update the useMemo dependency array to include workDraftKind (remove or keep
workDraftStorageKind only if still referenced elsewhere) so chat and CLI draft
jobs are isolated; target the draftLaunchJobsScopeKey definition and its
dependencies in AgentChatPane.tsx.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4dda871f-4c3b-4319-9794-50915972704e
⛔ Files ignored due to path filters (5)
docs/features/chat/README.mdis excluded by!docs/**docs/features/chat/composer-and-ui.mdis excluded by!docs/**docs/features/lanes/README.mdis excluded by!docs/**docs/features/linear-integration/README.mdis excluded by!docs/**docs/features/onboarding-and-settings/README.mdis excluded by!docs/**
📒 Files selected for processing (12)
apps/desktop/src/renderer/components/app/App.tsxapps/desktop/src/renderer/components/app/App.workKeepAlive.test.tsxapps/desktop/src/renderer/components/chat/AgentChatComposer.test.tsxapps/desktop/src/renderer/components/chat/AgentChatComposer.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.test.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.tsxapps/desktop/src/renderer/components/settings/AppearanceSection.tsxapps/desktop/src/renderer/lib/draftLaunchJobs.tsapps/desktop/src/renderer/lib/launchedLanesHighlight.test.tsapps/desktop/src/renderer/lib/launchedLanesHighlight.tsapps/desktop/src/renderer/state/appStore.test.tsapps/desktop/src/renderer/state/appStore.ts
62789c9 to
5a27222
Compare
Fixes ADE-96
Summary
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Linked Linear issues
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Greptile Summary
This PR fixes scoping bugs where draft launch job state (loading/error strips in the chat pane) was incorrectly shared across lane panes or lost on remount. It moves
draftLaunchJobsfrom component-localuseStateinto the globalappStorekeyed byprojectRoot + laneId + surfaceProfile + draftKind, adds a "naming-lane" status step, a 2-minute stale threshold with a dismiss button, and apaneMountedRefguard to prevent stale async continuations from affecting unmounted panes.draftLaunchJobsByScopeinappStorereplaces per-paneuseState, letting jobs survive remounts and preventing cross-pane bleed.paneMountedRefguard prevents background async from resettingselectedSessionIdafter the job is dismissed.launchPromptClipboardNoticeEnabledpreference lets users hide the "prompt will be copied" footer notice without disabling the copy behavior itself.Confidence Score: 4/5
Safe to merge with one fix: the job-still-visible guard reads from the wrong store instance in production, silently preventing foreground session auto-open and error display.
The
draftLaunchJobExistscallback usesuseAppStore.getState(), which is statically bound torootAppStore.getStateregardless of React context. In production everyAgentChatPaneruns inside anAppStoreProviderbacked by a per-project store, sodraftLaunchJobsByScopeis always empty when read this way. This makesjobStillVisiblepermanentlyfalse, so the guard exits every successful launch before callingopenLaunchedDraftSession. The new tests all run againstrootAppStoredirectly and do not catch this divergence.apps/desktop/src/renderer/components/chat/AgentChatPane.tsx — the draftLaunchJobExists callback around the patchDraftLaunchJob success/failure guards
Important Files Changed
Comments Outside Diff (1)
apps/desktop/src/renderer/components/chat/AgentChatPane.tsx, line 6109-6120 (link)The dismiss button is intentionally hidden for in-flight jobs (
!isActiveJob). The current catch/finally block instartDraftLaunchis thorough, so a stuck active job requires a hanging IPC call (e.g.,suggestLaneNamenever resolving). If that ever happens, the job will spin forever in the store with no user escape hatch —draftLaunchJobsByScopeis in-memory only, so a full app restart is the only remedy.Consider adding a timeout or a global "clear all stale active jobs" path (e.g., triggered on project open) as a safety valve for cases where the backing async is silently abandoned.
Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (5): Last reviewed commit: "Guard hidden draft launch completions" | Re-trigger Greptile