Conversation
WalkthroughThis PR implements per-window workspace tracking in the main process and comprehensively refactors MainScreen into a modular, hook-driven architecture. It adds window-scoped workspace management via new IPC channels, restructures the renderer into reusable hooks (useWorkspace, useTabs, useTasks, useWorktrees, useSidebar) and new components, replacing inline state and imperative panel refs with composable, testable abstractions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MainScreen as MainScreen<br/>(React)
participant useWorkspace as useWorkspace<br/>(Hook)
participant IPCRenderer as IPC Renderer
participant IPCMain as IPC Main
participant WindowMgr as window-manager
User ->> MainScreen: Component mounts
MainScreen ->> useWorkspace: useWorkspace()
Note over useWorkspace: On mount: load workspaces
useWorkspace ->> IPCRenderer: workspace-get-all
IPCRenderer ->> IPCMain: IPC call
IPCMain -->> IPCRenderer: workspaces[]
Note over useWorkspace: Read window-scoped workspace
useWorkspace ->> IPCRenderer: workspace-get-window-workspace-id
IPCRenderer ->> IPCMain: IPC call
IPCMain ->> WindowMgr: getWorkspaceForWindow()
WindowMgr -->> IPCMain: workspaceId | null
IPCMain -->> IPCRenderer: workspaceId | null
alt Workspace assigned
useWorkspace ->> useWorkspace: Show MainContentArea
else No workspace assigned
useWorkspace ->> useWorkspace: Show WorkspaceSelectionModal
end
User ->> User: Selects workspace in modal
User ->> MainScreen: onSelectWorkspace()
MainScreen ->> useWorkspace: handleWorkspaceSelect()
Note over useWorkspace: Persist window-scoped workspace
useWorkspace ->> IPCRenderer: workspace-set-window-workspace-id
IPCRenderer ->> IPCMain: IPC call
IPCMain ->> WindowMgr: setWorkspaceForWindow()
Note over useWorkspace: Update global active workspace
useWorkspace ->> IPCRenderer: workspace-set-active-workspace
IPCRenderer ->> IPCMain: IPC call
useWorkspace ->> useWorkspace: Update currentWorkspace state
MainScreen ->> MainScreen: Re-render MainContentArea
sequenceDiagram
participant User
participant TabUI as Tab UI<br/>(MainContentArea)
participant useTabs as useTabs<br/>(Hook)
participant IPCRenderer as IPC Renderer
participant IPCMain as IPC Main
User ->> TabUI: Select different tab
TabUI ->> useTabs: handleTabSelect(worktreeId, tabId)
useTabs ->> useTabs: Update selectedWorktreeId<br/>selectedTabId state
Note over useTabs: Persist active selection
useTabs ->> IPCRenderer: workspace-set-active-selection
IPCRenderer ->> IPCMain: IPC call
Note over useTabs: Refresh workspace state
useTabs ->> IPCRenderer: workspace-get (currentWorkspace)
IPCRenderer ->> IPCMain: IPC call
IPCMain -->> IPCRenderer: updated workspace
useTabs ->> useTabs: Update currentWorkspace<br/>state with new selection
TabUI ->> TabUI: Re-render with new tab
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/hooks/useWorktrees.ts (1)
170-170: Consider replacing alert with a user-friendly error notification.Using
alert()for error messages provides a poor user experience. Consider using a toast notification system or a custom error dialog component for better UX.apps/desktop/src/renderer/screens/main/hooks/useSidebar.ts (1)
25-27: Consider renaming for clarity.The function
handleSidebarCollapseonly hides the overlay and doesn't actually collapse the sidebar, which might be confusing given its name. Consider renaming tohandleHideOverlayorhandleOverlayDismissfor clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
apps/desktop/src/main/lib/window-manager.ts(2 hunks)apps/desktop/src/main/lib/workspace-ipcs.ts(2 hunks)apps/desktop/src/renderer/screens/main/MainScreen.tsx(5 hunks)apps/desktop/src/renderer/screens/main/components/MainContentArea/MainContentArea.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/MainContentArea/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/SidebarOverlay/SidebarOverlay.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/SidebarOverlay/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceSelectionModal/WorkspaceSelectionModal.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceSelectionModal/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/constants.ts(1 hunks)apps/desktop/src/renderer/screens/main/hooks/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/hooks/useSidebar.ts(1 hunks)apps/desktop/src/renderer/screens/main/hooks/useTabs.ts(1 hunks)apps/desktop/src/renderer/screens/main/hooks/useTasks.ts(1 hunks)apps/desktop/src/renderer/screens/main/hooks/useWorkspace.ts(1 hunks)apps/desktop/src/renderer/screens/main/hooks/useWorktrees.ts(1 hunks)apps/desktop/src/renderer/screens/main/types.ts(1 hunks)apps/desktop/src/renderer/screens/main/utils.ts(1 hunks)apps/desktop/src/shared/ipc-channels.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (13)
apps/desktop/src/renderer/screens/main/constants.ts (1)
apps/desktop/src/renderer/screens/main/types.ts (1)
UITask(4-14)
apps/desktop/src/renderer/screens/main/components/SidebarOverlay/SidebarOverlay.tsx (2)
apps/desktop/src/shared/types.ts (1)
Workspace(70-82)apps/desktop/src/renderer/screens/main/components/SidebarOverlay/index.ts (1)
SidebarOverlay(1-1)
apps/desktop/src/renderer/screens/main/utils.ts (3)
apps/desktop/src/shared/types.ts (2)
Tab(44-56)Worktree(58-68)apps/desktop/src/renderer/screens/main/types.ts (1)
PendingWorktree(17-28)apps/desktop/src/renderer/screens/main/constants.ts (1)
MOCK_TASKS(4-178)
apps/desktop/src/renderer/screens/main/hooks/useWorktrees.ts (1)
apps/desktop/src/shared/types.ts (2)
Workspace(70-82)Worktree(58-68)
apps/desktop/src/main/lib/window-manager.ts (1)
apps/desktop/src/main/windows/main.ts (1)
MainWindow(19-123)
apps/desktop/src/renderer/screens/main/hooks/useTabs.ts (2)
apps/desktop/src/shared/types.ts (2)
Workspace(70-82)Tab(44-56)apps/desktop/src/renderer/screens/main/utils.ts (1)
findTabRecursive(7-27)
apps/desktop/src/main/lib/workspace-ipcs.ts (1)
apps/desktop/src/main/lib/workspace-manager.ts (1)
workspaceManager(481-481)
apps/desktop/src/renderer/screens/main/hooks/useWorkspace.ts (1)
apps/desktop/src/shared/types.ts (1)
Workspace(70-82)
apps/desktop/src/renderer/screens/main/hooks/useSidebar.ts (1)
apps/desktop/src/renderer/screens/main/hooks/index.ts (1)
useSidebar(1-1)
apps/desktop/src/renderer/screens/main/hooks/useTasks.ts (3)
apps/desktop/src/shared/types.ts (1)
Worktree(58-68)apps/desktop/src/renderer/screens/main/types.ts (2)
PendingWorktree(17-28)UITask(4-14)apps/desktop/src/renderer/screens/main/constants.ts (1)
MOCK_TASKS(4-178)
apps/desktop/src/renderer/screens/main/components/WorkspaceSelectionModal/WorkspaceSelectionModal.tsx (1)
apps/desktop/src/shared/types.ts (1)
Workspace(70-82)
apps/desktop/src/renderer/screens/main/components/MainContentArea/MainContentArea.tsx (5)
apps/desktop/src/renderer/screens/main/types.ts (1)
AppMode(30-30)apps/desktop/src/shared/types.ts (3)
Workspace(70-82)Worktree(58-68)Tab(44-56)apps/desktop/src/renderer/screens/main/components/PlaceholderState.tsx (1)
PlaceholderState(7-44)apps/desktop/src/renderer/screens/main/components/MainContent/TabGroup.tsx (1)
TabGroup(23-313)apps/desktop/src/renderer/screens/main/components/MainContent/TabContent.tsx (1)
TabContent(32-158)
apps/desktop/src/renderer/screens/main/MainScreen.tsx (9)
apps/desktop/src/renderer/screens/main/types.ts (1)
AppMode(30-30)apps/desktop/src/renderer/screens/main/hooks/useWorkspace.ts (1)
useWorkspace(4-189)apps/desktop/src/renderer/screens/main/hooks/useTabs.ts (1)
useTabs(13-111)apps/desktop/src/renderer/screens/main/hooks/useSidebar.ts (1)
useSidebar(4-39)apps/desktop/src/renderer/screens/main/hooks/useWorktrees.ts (1)
useWorktrees(12-221)apps/desktop/src/renderer/screens/main/hooks/useTasks.ts (1)
useTasks(16-191)apps/desktop/src/renderer/screens/main/components/SidebarOverlay/SidebarOverlay.tsx (1)
SidebarOverlay(18-54)apps/desktop/src/renderer/screens/main/components/MainContentArea/MainContentArea.tsx (1)
MainContentArea(40-185)apps/desktop/src/renderer/screens/main/components/WorkspaceSelectionModal/WorkspaceSelectionModal.tsx (1)
WorkspaceSelectionModal(22-132)
🪛 GitHub Actions: CI
apps/desktop/src/renderer/screens/main/hooks/useWorktrees.ts
[error] 58-58: Argument of type '(prev: any) => any' is not assignable to parameter of type 'Workspace[]'.
[error] 58-58: Parameter 'prev' implicitly has an 'any' type.
[error] 60-60: Parameter 'ws' implicitly has an 'any' type.
apps/desktop/src/main/lib/window-manager.ts
[warning] 32-34: Formatter would have printed the following content: setWorkspaceForWindow(window: BrowserWindow, workspaceId: string | null): void
apps/desktop/src/renderer/screens/main/hooks/useWorkspace.ts
[error] 7-7: Cannot find name 'UseWorkspaceProps'.
apps/desktop/src/renderer/screens/main/hooks/useTasks.ts
[error] 2-2: Module '"shared/types"' has no exported member 'TaskStatus'.
🔇 Additional comments (11)
apps/desktop/src/renderer/screens/main/components/WorkspaceSelectionModal/index.ts (1)
1-1: LGTM!Standard barrel export pattern for the WorkspaceSelectionModal component.
apps/desktop/src/shared/ipc-channels.ts (2)
85-92: LGTM!The new IPC channels for per-window workspace tracking are well-typed and follow the existing naming conventions. They integrate seamlessly with the window-workspace management introduced in window-manager.ts.
397-398: LGTM!Channel names correctly added to the validation list.
apps/desktop/src/renderer/screens/main/components/WorkspaceSelectionModal/WorkspaceSelectionModal.tsx (2)
45-45: Verify that the modal should not be dismissible.The
onOpenChangehandler is an empty function andshowCloseButtonis set to false, preventing users from closing the modal. This appears intentional for forcing workspace selection on startup, but please confirm this is the desired UX behavior.
58-106: LGTM!The conditional rendering handles both empty and populated workspace states appropriately, with clear visual feedback and appropriate messaging.
apps/desktop/src/renderer/screens/main/hooks/useWorktrees.ts (1)
21-37: LGTM!The workspace refresh logic properly handles errors and maintains state consistency.
apps/desktop/src/renderer/screens/main/components/MainContentArea/index.ts (1)
1-1: LGTM!Standard barrel export pattern for the MainContentArea component.
apps/desktop/src/renderer/screens/main/components/SidebarOverlay/index.ts (1)
1-1: LGTM!Standard barrel export pattern for the SidebarOverlay component.
apps/desktop/src/renderer/screens/main/hooks/useSidebar.ts (1)
9-23: LGTM!The collapse and expand handlers properly manage both the panel state and the boolean flag.
apps/desktop/src/main/lib/window-manager.ts (2)
6-6: LGTM!The window-workspace mapping is properly initialized and will be cleaned up when windows close.
11-12: LGTM!Initializing new windows with no workspace (null) aligns with the user flow requiring explicit workspace selection via the WorkspaceSelectionModal.
| getWorkspaceForWindow(window: BrowserWindow): string | null { | ||
| return this.windowWorkspaces.get(window) ?? null; | ||
| } | ||
|
|
||
| setWorkspaceForWindow(window: BrowserWindow, workspaceId: string | null): void { | ||
| this.windowWorkspaces.set(window, workspaceId); | ||
| } |
There was a problem hiding this comment.
Fix formatting issue.
The formatter would reformat the setWorkspaceForWindow method. Run your formatter to resolve this warning.
Expected format:
setWorkspaceForWindow(window: BrowserWindow, workspaceId: string | null): void {
this.windowWorkspaces.set(window, workspaceId);
}🧰 Tools
🪛 GitHub Actions: CI
[warning] 32-34: Formatter would have printed the following content: setWorkspaceForWindow(window: BrowserWindow, workspaceId: string | null): void
🤖 Prompt for AI Agents
In apps/desktop/src/main/lib/window-manager.ts around lines 30 to 36, the
setWorkspaceForWindow method has a formatting discrepancy that the
linter/formatter flags; reformat the method to match project style (single-line
method signature with opening brace, indented body, and closing brace on its own
line) — i.e. ensure the method appears exactly as: setWorkspaceForWindow(window:
BrowserWindow, workspaceId: string | null): void
{ this.windowWorkspaces.set(window, workspaceId); } — then run the project's
formatter (e.g., prettier or eslint --fix) to apply and commit the corrected
formatting.
| import { useState } from "react"; | ||
| import type { TaskStatus, Worktree } from "shared/types"; | ||
| import type { UITask, PendingWorktree } from "../types"; | ||
| import { MOCK_TASKS } from "../constants"; |
There was a problem hiding this comment.
Fix TaskStatus import to unblock build
shared/types doesn’t export TaskStatus, so this import triggers the CI failure (Module '"shared/types"' has no exported member 'TaskStatus'). Either re-export TaskStatus from shared/types or import it from the module where it’s actually defined (the same place you import UITask). Without this fix the TypeScript build can’t succeed.
Apply this diff if TaskStatus lives in ../types:
-import type { TaskStatus, Worktree } from "shared/types";
-import type { UITask, PendingWorktree } from "../types";
+import type { Worktree } from "shared/types";
+import type { UITask, PendingWorktree, TaskStatus } from "../types";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { useState } from "react"; | |
| import type { TaskStatus, Worktree } from "shared/types"; | |
| import type { UITask, PendingWorktree } from "../types"; | |
| import { MOCK_TASKS } from "../constants"; | |
| import { useState } from "react"; | |
| import type { Worktree } from "shared/types"; | |
| import type { UITask, PendingWorktree, TaskStatus } from "../types"; | |
| import { MOCK_TASKS } from "../constants"; |
🧰 Tools
🪛 GitHub Actions: CI
[error] 2-2: Module '"shared/types"' has no exported member 'TaskStatus'.
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/screens/main/hooks/useTasks.ts around lines 1 to 4,
the current import pulls TaskStatus from "shared/types" which doesn't export it
and breaks the build; update the import to get TaskStatus from the correct
module (likely "../types") — i.e., remove TaskStatus from the "shared/types"
import and add it to the import list from "../types" (or alternatively re-export
TaskStatus from shared/types if you prefer), then run type-check to confirm the
error is resolved.
| export function useWorkspace({ | ||
| setSelectedWorktreeId, | ||
| setSelectedTabId, | ||
| }: UseWorkspaceProps = {}) { |
There was a problem hiding this comment.
Restore the missing UseWorkspaceProps definition.
UseWorkspaceProps is referenced but never declared or imported, triggering the CI failure (Cannot find name 'UseWorkspaceProps'). Please reintroduce the interface so the hook’s optional setters are properly typed.
[suggested fix]
+interface UseWorkspaceProps {
+ setSelectedWorktreeId?: (id: string | null) => void;
+ setSelectedTabId?: (id: string | null) => void;
+}
+
export function useWorkspace({
setSelectedWorktreeId,
setSelectedTabId,
}: UseWorkspaceProps = {}) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function useWorkspace({ | |
| setSelectedWorktreeId, | |
| setSelectedTabId, | |
| }: UseWorkspaceProps = {}) { | |
| interface UseWorkspaceProps { | |
| setSelectedWorktreeId?: (id: string | null) => void; | |
| setSelectedTabId?: (id: string | null) => void; | |
| } | |
| export function useWorkspace({ | |
| setSelectedWorktreeId, | |
| setSelectedTabId, | |
| }: UseWorkspaceProps = {}) { |
🧰 Tools
🪛 GitHub Actions: CI
[error] 7-7: Cannot find name 'UseWorkspaceProps'.
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/screens/main/hooks/useWorkspace.ts around lines 4
to 7, restore the missing UseWorkspaceProps type by adding an interface above
the hook like: declare UseWorkspaceProps with optional properties
setSelectedWorktreeId and setSelectedTabId typed as functions accepting a string
or null (e.g. (id: string | null) => void); place it before the useWorkspace
export so the hook parameter is properly typed and CI error is resolved.
| // Also update in workspaces array if available | ||
| setWorkspaces((prev) => { | ||
| if (!prev) return prev; | ||
| return prev.map((ws) => | ||
| ws.id === currentWorkspace.id ? updatedCurrentWorkspace : ws, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Fix type error with setWorkspaces callback.
The pipeline correctly flags a type mismatch. The setWorkspaces parameter is typed as (workspaces: Workspace[]) => void, but it's being invoked with a callback function (prev) => ..., which suggests it's a React state setter.
If setWorkspaces is a setState function from useState, update the prop type to:
setWorkspaces: React.Dispatch<React.SetStateAction<Workspace[]>>;Alternatively, if it's meant to be a plain setter, replace the callback with direct array manipulation.
Apply this diff to fix the type:
interface UseWorktreesProps {
currentWorkspace: Workspace | null;
setCurrentWorkspace: (workspace: Workspace) => void;
- setWorkspaces: (workspaces: Workspace[]) => void;
+ setWorkspaces: React.Dispatch<React.SetStateAction<Workspace[]>>;
loadAllWorkspaces: () => Promise<void>;
setSelectedWorktreeId: (id: string | null) => void;
setSelectedTabId: (id: string | null) => void;
}🧰 Tools
🪛 GitHub Actions: CI
[error] 58-58: Argument of type '(prev: any) => any' is not assignable to parameter of type 'Workspace[]'.
[error] 58-58: Parameter 'prev' implicitly has an 'any' type.
[error] 60-60: Parameter 'ws' implicitly has an 'any' type.
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/screens/main/hooks/useWorktrees.ts around lines 57
to 63, the code calls setWorkspaces with a callback but the prop is typed as
(workspaces: Workspace[]) => void causing a type mismatch; change the prop type
to React.Dispatch<React.SetStateAction<Workspace[]>> so it accepts the updater
callback, or if setWorkspaces is intentionally a plain setter, modify the call
to compute and pass the new Workspace[] directly instead of using a function
callback.
| onCreatePR={() => handleCreatePR(selectedWorktreeId)} | ||
| onMergePR={() => handleMergePR(selectedWorktreeId)} | ||
| worktrees={enrichWorktreesWithTasks( |
There was a problem hiding this comment.
Restore per-worktree PR callbacks
TaskTabs still invokes onCreatePR(worktree.id) / onMergePR(worktree.id) for tab-level actions. Replacing the props with () => handleCreatePR(selectedWorktreeId) drops the worktree id. When a user triggers the menu on a non-active worktree (or before selection state catches up) we’ll operate on the wrong worktree or silently bail when selectedWorktreeId is null—a functional regression on those PR actions. Please keep the callbacks accepting the worktree id (or pass the id through explicitly).
- onCreatePR={() => handleCreatePR(selectedWorktreeId)}
- onMergePR={() => handleMergePR(selectedWorktreeId)}
+ onCreatePR={handleCreatePR}
+ onMergePR={handleMergePR}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onCreatePR={() => handleCreatePR(selectedWorktreeId)} | |
| onMergePR={() => handleMergePR(selectedWorktreeId)} | |
| worktrees={enrichWorktreesWithTasks( | |
| onCreatePR={handleCreatePR} | |
| onMergePR={handleMergePR} | |
| worktrees={enrichWorktreesWithTasks( |
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/screens/main/MainScreen.tsx around lines 144 to
146, the TaskTabs prop callbacks were changed from accepting a worktree id to
using the selectedWorktreeId closure, causing actions on non-active tabs to
target the wrong worktree or fail; restore the original signature by passing the
worktree id through (e.g., onCreatePR={(id) => handleCreatePR(id)} and
onMergePR={(id) => handleMergePR(id)}) or otherwise ensure the id from the
invoked tab is forwarded explicitly to the handlers instead of using
selectedWorktreeId.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Refactor