Conversation
Contributor
WalkthroughThe MainScreen is refactored to use modular external components (PlanView, AddTaskModal, TaskTabs) instead of inline logic. New types (UITask, PendingWorktree) and utilities (enrichWorktreesWithTasks, tab management functions) are introduced. The NewLayoutMain component is removed. A DroppableMainContent component with drag-and-drop support is added. The ENABLE_NEW_UI environment flag is removed from configuration and type definitions. Changes
Sequence DiagramsequenceDiagram
participant User
participant MainScreen
participant AddTaskModal
participant PlanView
participant enrichWorktreesWithTasks as Enrichment Util
participant Backend
User->>MainScreen: Open app / set mode=plan
MainScreen->>MainScreen: Load currentWorkspace & worktrees
rect rgb(200, 220, 240)
Note over User,Backend: Task Creation Flow (Optimistic)
User->>AddTaskModal: Open add task form
User->>AddTaskModal: Submit new task
AddTaskModal->>MainScreen: handleCreateTask(task)
MainScreen->>MainScreen: Create PendingWorktree (isPending=true)
MainScreen->>enrichWorktreesWithTasks: Merge pending + real worktrees
enrichWorktreesWithTasks->>enrichWorktreesWithTasks: Mark pending as isPending, enrich real by branch
enrichWorktreesWithTasks-->>MainScreen: WorktreeWithTask[] (with pending)
MainScreen->>PlanView: Render with enriched worktrees
PlanView-->>User: Show pending task in Kanban
MainScreen->>Backend: Create worktree (async)
Backend-->>MainScreen: Worktree created
MainScreen->>MainScreen: Refresh worktrees, reconcile pending
end
rect rgb(240, 220, 200)
Note over User,Backend: Tab Selection & View Switch
User->>PlanView: Select task / switch to edit mode
PlanView->>MainScreen: handleSelectTask(task)
MainScreen->>MainScreen: findTabRecursive to locate tab
MainScreen->>Backend: IPC setActiveTab
MainScreen->>MainScreen: Switch mode to edit
MainScreen-->>User: Show workspace/terminal view
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
🧹 Nitpick comments (3)
apps/desktop/src/renderer/screens/main/components/NewLayout/mock-data.ts (1)
3-3: TODO: Backend integration for task data.The TODO comment appropriately flags that this mock data needs to be replaced with actual backend data.
When you're ready to integrate with a real backend, I can help generate the API integration code and update the consuming components. Would you like me to open an issue to track this work?
apps/desktop/src/renderer/screens/main/components/NewLayout/utils.ts (1)
35-62: Consider parameterizing the tasks data source for flexibility.The function is currently tightly coupled to
MOCK_TASKS(Line 3, Line 37), which will require refactoring during backend integration. Consider accepting tasks as a parameter to make this more flexible and testable.Apply this refactor to accept tasks as a parameter:
export function enrichWorktreesWithTasks( worktrees: Worktree[], pendingWorktrees: PendingWorktree[], + tasks: UITask[] = MOCK_TASKS, ): WorktreeWithTask[] { // ... const enrichedWorktrees = worktrees.map((worktree) => { - const matchingTask = MOCK_TASKS.find( + const matchingTask = tasks.find( (task) => task.branch === worktree.branch, ); // ... }); // ... }This allows callers to pass real task data while maintaining backward compatibility with the default parameter.
apps/desktop/src/renderer/screens/main/components/MainContent/utils.ts (1)
50-133: Mosaic tree utilities are well-implemented with good safety checks.The functions handle edge cases appropriately:
removeTabFromMosaicTreecorrectly prunes empty branches (Lines 63-74)addTabToMosaicTreeprevents duplicate tab IDs with helpful warnings (Lines 97-101, 122-125)The duplicate detection logic (Lines 114-126) is particularly valuable for preventing UI inconsistencies.
The console warnings are helpful for debugging but could be replaced with a proper logging utility for production environments:
// Add at top of file const logger = { warn: (message: string) => { if (import.meta.env.DEV) { console.warn(message); } // Could send to monitoring service in production } }; // Use in functions logger.warn(`[MainScreen] Attempted to add duplicate tab ID "${tabId}" to mosaic tree`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/desktop/src/renderer/screens/main/MainScreen.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/MainContent/components/DroppableMainContent/DroppableMainContent.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/MainContent/components/DroppableMainContent/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/MainContent/utils.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/NewLayout/NewLayoutMain.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/NewLayout/mock-data.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/NewLayout/types.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/NewLayout/utils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
apps/desktop/src/renderer/screens/main/components/NewLayout/utils.ts (3)
apps/desktop/src/shared/types.ts (1)
Worktree(58-68)apps/desktop/src/renderer/screens/main/components/NewLayout/types.ts (1)
PendingWorktree(17-28)apps/desktop/src/renderer/screens/main/components/NewLayout/mock-data.ts (1)
MOCK_TASKS(4-178)
apps/desktop/src/renderer/screens/main/components/NewLayout/mock-data.ts (1)
apps/desktop/src/renderer/screens/main/components/NewLayout/types.ts (1)
UITask(4-14)
apps/desktop/src/renderer/screens/main/components/MainContent/utils.ts (1)
apps/desktop/src/shared/types.ts (2)
Tab(44-56)MosaicNode(35-35)
apps/desktop/src/renderer/screens/main/components/MainContent/components/DroppableMainContent/DroppableMainContent.tsx (1)
apps/desktop/src/renderer/screens/main/components/MainContent/components/DroppableMainContent/index.ts (1)
DroppableMainContent(1-1)
🔇 Additional comments (8)
apps/desktop/src/renderer/screens/main/components/NewLayout/types.ts (1)
1-29: LGTM! Clean type definitions.The type aliases are well-structured and provide clear contracts for task and pending worktree data. The literal type
isPending: trueon Line 19 ensures type safety for discriminating between pending and real worktrees.apps/desktop/src/renderer/screens/main/components/MainContent/components/DroppableMainContent/index.ts (1)
1-1: LGTM! Standard re-export pattern.apps/desktop/src/renderer/screens/main/components/NewLayout/mock-data.ts (1)
4-178: Mock data structure looks good.The 15 mock tasks provide comprehensive test data covering all status types and various scenarios. The data structure correctly matches the UITask type definition.
apps/desktop/src/renderer/screens/main/MainScreen.tsx (1)
32-38: Excellent refactor: Shared utilities improve maintainability.Extracting
DroppableMainContentand the mosaic/tab utilities (findTabRecursive,findTabById,addTabToMosaicTree,removeTabFromMosaicTree) into shared modules eliminates code duplication and establishes clearer module boundaries. This makes the codebase more maintainable.apps/desktop/src/renderer/screens/main/components/MainContent/components/DroppableMainContent/DroppableMainContent.tsx (1)
1-41: LGTM! Clean droppable component implementation.The component provides clear visual feedback with the blue ring (Line 27) and overlay message (Lines 31-37) when users drag tabs over the drop zone. The integration with
@dnd-kit/coreis straightforward and follows best practices.apps/desktop/src/renderer/screens/main/components/NewLayout/NewLayoutMain.tsx (1)
15-24: LGTM! Successfully migrated to shared utilities.The component now leverages external types (
UITask,PendingWorktree) and utilities (findTabRecursive,enrichWorktreesWithTasks,MOCK_TASKS), reducing inline definitions and improving code organization. The refactor maintains functionality while establishing better module boundaries.apps/desktop/src/renderer/screens/main/components/NewLayout/utils.ts (1)
9-32: Pending worktree handling looks good.The conversion of
PendingWorktreeitems toWorktreeWithTaskformat (Lines 14-32) is clean and handles the optionaltaskDataappropriately.apps/desktop/src/renderer/screens/main/components/MainContent/utils.ts (1)
11-45: LGTM! Well-implemented tab search utilities.Both
findTabRecursiveandfindTabByIdhandle the tab hierarchy appropriately. The functions correctly handle group tabs and their children (Lines 22-28, 39-42), andfindTabRecursiveprovides parent tracking which is useful for context-aware operations.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
apps/desktop/src/renderer/screens/main/components/mock-data.ts (1)
3-3: Track the TODO for backend integration.The TODO comment indicates this mock data should be replaced with actual backend data.
Do you want me to open a new issue to track the backend integration task?
apps/desktop/src/renderer/screens/main/MainScreen.tsx (2)
48-51: Memoize theopenTaskscomputation.Lines 49-51 compute
openTasksby filteringMOCK_TASKSon every render. This is inefficient and should be memoized.Apply this diff:
+ const openTasks = useMemo( + () => + MOCK_TASKS.filter((task) => + currentWorkspace?.worktrees?.some((wt) => wt.branch === task.branch), + ), + [currentWorkspace?.worktrees], + ); - // Compute which tasks have worktrees (are "open") - const openTasks = MOCK_TASKS.filter((task) => - currentWorkspace?.worktrees?.some((wt) => wt.branch === task.branch), - );
675-687: Consider extracting the pending prefix as a constant.Line 677 uses the magic string
"pending-"to identify pending worktrees. This same prefix is used when creating pending worktrees on lines 343 and 409.Extract to a constant for better maintainability:
+const PENDING_WORKTREE_PREFIX = "pending-"; + export function MainScreen() { // ... rest of component // In handleSelectTask: - const pendingId = `pending-${Date.now()}`; + const pendingId = `${PENDING_WORKTREE_PREFIX}${Date.now()}`; // In handleCreateTask: - const pendingId = `pending-${Date.now()}`; + const pendingId = `${PENDING_WORKTREE_PREFIX}${Date.now()}`; // In onWorktreeSelect: - if (worktreeId.startsWith("pending-")) return; + if (worktreeId.startsWith(PENDING_WORKTREE_PREFIX)) return;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/desktop/electron.vite.config.ts(0 hunks)apps/desktop/index.d.ts(0 hunks)apps/desktop/src/renderer/env.d.ts(1 hunks)apps/desktop/src/renderer/screens/main/MainScreen.tsx(9 hunks)apps/desktop/src/renderer/screens/main/components/NewLayout/NewLayoutMain.tsx(0 hunks)apps/desktop/src/renderer/screens/main/components/mock-data.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/types.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/utils.ts(1 hunks)
💤 Files with no reviewable changes (3)
- apps/desktop/index.d.ts
- apps/desktop/electron.vite.config.ts
- apps/desktop/src/renderer/screens/main/components/NewLayout/NewLayoutMain.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
apps/desktop/src/renderer/screens/main/components/utils.ts (5)
apps/desktop/src/shared/types.ts (1)
Worktree(58-68)apps/desktop/src/renderer/screens/main/components/types.ts (1)
PendingWorktree(17-28)apps/desktop/src/renderer/screens/main/components/TaskTabs/types.ts (1)
WorktreeWithTask(4-18)apps/desktop/src/renderer/screens/main/components/mock-data.ts (1)
MOCK_TASKS(4-178)apps/desktop/src/renderer/screens/main/components/NewLayout/NewLayoutMain.tsx (1)
enrichWorktreesWithTasks(229-286)
apps/desktop/src/renderer/screens/main/components/mock-data.ts (1)
apps/desktop/src/renderer/screens/main/components/types.ts (1)
UITask(4-14)
apps/desktop/src/renderer/screens/main/components/types.ts (1)
apps/desktop/src/renderer/screens/main/components/StatusIndicator.tsx (1)
TaskStatus(3-9)
apps/desktop/src/renderer/screens/main/MainScreen.tsx (9)
apps/desktop/src/shared/types.ts (3)
Workspace(70-82)Tab(44-56)Worktree(58-68)apps/desktop/src/renderer/screens/main/components/types.ts (2)
PendingWorktree(17-28)UITask(4-14)apps/desktop/src/renderer/screens/main/components/mock-data.ts (1)
MOCK_TASKS(4-178)apps/desktop/src/renderer/screens/main/components/MainContent/utils.ts (1)
findTabRecursive(11-31)apps/desktop/src/renderer/screens/main/components/Sidebar/Sidebar.tsx (1)
Sidebar(28-361)apps/desktop/src/renderer/screens/main/components/TaskTabs/TaskTabs.tsx (1)
TaskTabs(9-71)apps/desktop/src/renderer/screens/main/components/utils.ts (1)
enrichWorktreesWithTasks(9-66)apps/desktop/src/renderer/screens/main/components/TabContent/components/DiffTab/DiffTab.tsx (1)
DiffTab(18-165)apps/desktop/src/renderer/screens/main/components/AddTaskModal.tsx (1)
AddTaskModal(62-457)
🔇 Additional comments (6)
apps/desktop/src/renderer/env.d.ts (1)
3-5: LGTM! Removal is complete with no dangling references.Verification confirms no code still references the removed
ENABLE_NEW_UIenvironment variable across the codebase. The emptyImportMetaEnvinterface is valid TypeScript and aligns with the broader refactor to eliminate this feature flag.apps/desktop/src/renderer/screens/main/components/utils.ts (2)
9-66: LGTM - Clean functional approach.The function follows good practices: it's pure, returns a new array without mutations, and has clear comments documenting each step. This is ideal for React components.
35-62: Task matching logic is correct—no issues found.Verification confirms that all 15 tasks in MOCK_TASKS have unique branch names with no duplicates. The
.find()approach is appropriate and the one-to-one branch-to-task mapping is enforced by the mock data structure.apps/desktop/src/renderer/screens/main/components/types.ts (1)
1-29: LGTM - Well-defined type aliases.The type definitions are clean and correctly structured. Using
isPending: trueas a literal type inPendingWorktreeenables effective type discrimination in TypeScript.apps/desktop/src/renderer/screens/main/MainScreen.tsx (2)
468-541: LGTM - Good error handling in PR operations.Both
handleCreatePRandhandleMergePRproperly handle errors and notify users via alerts. The error messages are clear and informative.
1-831: Excellent refactoring to modular components.The refactoring successfully extracts inline logic into modular external components (PlanView, AddTaskModal, TaskTabs). The mode-based rendering provides a clean separation between planning and editing workflows.
Comment on lines
+13
to
+175
| assigneeAvatarUrl: "https://i.pravatar.cc/150?img=1", | ||
| lastUpdated: "2 hours ago", | ||
| }, | ||
| { | ||
| id: "2", | ||
| slug: "SSET-2", | ||
| name: "API Integration", | ||
| status: "needs-feedback", | ||
| branch: "feature/api-integration", | ||
| description: "Integrate new REST API endpoints for user management", | ||
| assignee: "Bob", | ||
| assigneeAvatarUrl: "https://i.pravatar.cc/150?img=12", | ||
| lastUpdated: "1 day ago", | ||
| }, | ||
| { | ||
| id: "3", | ||
| slug: "SSET-3", | ||
| name: "Bug Fixes", | ||
| status: "planning", | ||
| branch: "fix/various-bugs", | ||
| description: "Collection of bug fixes reported by users", | ||
| assignee: "Charlie", | ||
| assigneeAvatarUrl: "https://i.pravatar.cc/150?img=33", | ||
| lastUpdated: "3 days ago", | ||
| }, | ||
| { | ||
| id: "4", | ||
| slug: "SSET-4", | ||
| name: "Performance Optimization", | ||
| status: "ready-to-merge", | ||
| branch: "perf/optimize-queries", | ||
| description: "Optimize database queries for faster page loads", | ||
| assignee: "Diana", | ||
| assigneeAvatarUrl: "https://i.pravatar.cc/150?img=9", | ||
| lastUpdated: "5 minutes ago", | ||
| }, | ||
| { | ||
| id: "5", | ||
| slug: "SSET-5", | ||
| name: "User Authentication System", | ||
| status: "working", | ||
| branch: "feature/auth-system", | ||
| description: | ||
| "Implement OAuth2 and JWT-based authentication system with refresh tokens", | ||
| assignee: "Eve", | ||
| assigneeAvatarUrl: "https://i.pravatar.cc/150?img=5", | ||
| lastUpdated: "3 hours ago", | ||
| }, | ||
| { | ||
| id: "6", | ||
| slug: "SSET-6", | ||
| name: "Dark Mode Support", | ||
| status: "planning", | ||
| branch: "feature/dark-mode", | ||
| description: "Add dark mode theme support across the entire application", | ||
| assignee: "Frank", | ||
| assigneeAvatarUrl: "https://i.pravatar.cc/150?img=13", | ||
| lastUpdated: "2 days ago", | ||
| }, | ||
| { | ||
| id: "7", | ||
| slug: "SSET-7", | ||
| name: "Database Migration Scripts", | ||
| status: "ready-to-merge", | ||
| branch: "db/migration-scripts", | ||
| description: | ||
| "Create automated migration scripts for production database updates", | ||
| assignee: "Grace", | ||
| assigneeAvatarUrl: "https://i.pravatar.cc/150?img=20", | ||
| lastUpdated: "1 hour ago", | ||
| }, | ||
| { | ||
| id: "8", | ||
| slug: "SSET-8", | ||
| name: "Email Notification Service", | ||
| status: "needs-feedback", | ||
| branch: "feature/email-notifications", | ||
| description: | ||
| "Build email notification service using SendGrid for transactional emails", | ||
| assignee: "Henry", | ||
| assigneeAvatarUrl: "https://i.pravatar.cc/150?img=8", | ||
| lastUpdated: "4 hours ago", | ||
| }, | ||
| { | ||
| id: "9", | ||
| slug: "SSET-9", | ||
| name: "Mobile Responsive Design", | ||
| status: "working", | ||
| branch: "feature/mobile-responsive", | ||
| description: | ||
| "Make the application fully responsive for mobile and tablet devices", | ||
| assignee: "Iris", | ||
| assigneeAvatarUrl: "https://i.pravatar.cc/150?img=16", | ||
| lastUpdated: "6 hours ago", | ||
| }, | ||
| { | ||
| id: "10", | ||
| slug: "SSET-10", | ||
| name: "Analytics Dashboard", | ||
| status: "planning", | ||
| branch: "feature/analytics-dashboard", | ||
| description: | ||
| "Create admin dashboard with charts and metrics for user analytics", | ||
| assignee: "Jack", | ||
| assigneeAvatarUrl: "https://i.pravatar.cc/150?img=11", | ||
| lastUpdated: "1 week ago", | ||
| }, | ||
| { | ||
| id: "11", | ||
| slug: "SSET-11", | ||
| name: "CI/CD Pipeline", | ||
| status: "ready-to-merge", | ||
| branch: "devops/ci-cd-pipeline", | ||
| description: | ||
| "Set up automated CI/CD pipeline with GitHub Actions and Docker", | ||
| assignee: "Kate", | ||
| assigneeAvatarUrl: "https://i.pravatar.cc/150?img=25", | ||
| lastUpdated: "30 minutes ago", | ||
| }, | ||
| { | ||
| id: "12", | ||
| slug: "SSET-12", | ||
| name: "Search Functionality", | ||
| status: "working", | ||
| branch: "feature/search", | ||
| description: "Implement full-text search with Elasticsearch integration", | ||
| assignee: "Liam", | ||
| assigneeAvatarUrl: "https://i.pravatar.cc/150?img=14", | ||
| lastUpdated: "5 hours ago", | ||
| }, | ||
| { | ||
| id: "13", | ||
| slug: "SSET-13", | ||
| name: "File Upload System", | ||
| status: "needs-feedback", | ||
| branch: "feature/file-uploads", | ||
| description: | ||
| "Build secure file upload system with S3 storage and virus scanning", | ||
| assignee: "Mia", | ||
| assigneeAvatarUrl: "https://i.pravatar.cc/150?img=27", | ||
| lastUpdated: "2 hours ago", | ||
| }, | ||
| { | ||
| id: "14", | ||
| slug: "SSET-14", | ||
| name: "API Rate Limiting", | ||
| status: "planning", | ||
| branch: "feature/rate-limiting", | ||
| description: "Implement rate limiting and throttling for API endpoints", | ||
| assignee: "Noah", | ||
| assigneeAvatarUrl: "https://i.pravatar.cc/150?img=17", | ||
| lastUpdated: "4 days ago", | ||
| }, | ||
| { | ||
| id: "15", | ||
| slug: "SSET-15", | ||
| name: "Internationalization", | ||
| status: "working", | ||
| branch: "feature/i18n", | ||
| description: | ||
| "Add multi-language support with i18next for English, Spanish, and French", | ||
| assignee: "Olivia", | ||
| assigneeAvatarUrl: "https://i.pravatar.cc/150?img=32", |
Contributor
There was a problem hiding this comment.
External avatar URLs may be unreliable.
The mock data uses pravatar.cc URLs for avatars. These third-party URLs may fail if the service is down or blocked by network policies, resulting in broken images.
Consider using placeholder data URLs or local assets for more reliable mock data:
- assigneeAvatarUrl: "https://i.pravatar.cc/150?img=1",
+ assigneeAvatarUrl: "data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='150' height='150'%3E%3Crect width='150' height='150' fill='%23ddd'/%3E%3C/svg%3E",
Comment on lines
+14
to
+32
| const pendingAsWorktrees: WorktreeWithTask[] = pendingWorktrees.map( | ||
| (pending) => ({ | ||
| id: pending.id, | ||
| branch: pending.branch, | ||
| path: "", // Pending worktrees don't have a path yet | ||
| tabs: [], | ||
| createdAt: new Date().toISOString(), | ||
| isPending: true, // Mark as pending for UI | ||
| task: pending.taskData | ||
| ? { | ||
| id: pending.id, | ||
| slug: pending.taskData.slug, | ||
| title: pending.taskData.name, | ||
| status: pending.taskData.status, | ||
| description: pending.description || "", | ||
| } | ||
| : undefined, | ||
| }), | ||
| ); |
Contributor
There was a problem hiding this comment.
Consider caching timestamps to prevent unnecessary re-renders.
Line 20 creates a new timestamp with new Date().toISOString() on every function call. When this function is called during render (as it is in MainScreen.tsx line 670), it causes the returned object references to change each time, potentially triggering unnecessary re-renders.
Consider one of these approaches:
- Accept a timestamp parameter (if the pending worktree should have a creation time):
export function enrichWorktreesWithTasks(
worktrees: Worktree[],
pendingWorktrees: PendingWorktree[],
): WorktreeWithTask[] {
const pendingAsWorktrees: WorktreeWithTask[] = pendingWorktrees.map(
(pending) => ({
id: pending.id,
branch: pending.branch,
path: "",
tabs: [],
- createdAt: new Date().toISOString(),
+ createdAt: pending.createdAt || new Date().toISOString(),
isPending: true,- Use a fixed placeholder (if the timestamp doesn't matter for pending worktrees):
- createdAt: new Date().toISOString(),
+ createdAt: "",Also, ensure this function is memoized at the call site (MainScreen.tsx line 670) using useMemo.
📝 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.
Suggested change
| const pendingAsWorktrees: WorktreeWithTask[] = pendingWorktrees.map( | |
| (pending) => ({ | |
| id: pending.id, | |
| branch: pending.branch, | |
| path: "", // Pending worktrees don't have a path yet | |
| tabs: [], | |
| createdAt: new Date().toISOString(), | |
| isPending: true, // Mark as pending for UI | |
| task: pending.taskData | |
| ? { | |
| id: pending.id, | |
| slug: pending.taskData.slug, | |
| title: pending.taskData.name, | |
| status: pending.taskData.status, | |
| description: pending.description || "", | |
| } | |
| : undefined, | |
| }), | |
| ); | |
| const pendingAsWorktrees: WorktreeWithTask[] = pendingWorktrees.map( | |
| (pending) => ({ | |
| id: pending.id, | |
| branch: pending.branch, | |
| path: "", // Pending worktrees don't have a path yet | |
| tabs: [], | |
| createdAt: "", | |
| isPending: true, // Mark as pending for UI | |
| task: pending.taskData | |
| ? { | |
| id: pending.id, | |
| slug: pending.taskData.slug, | |
| title: pending.taskData.name, | |
| status: pending.taskData.status, | |
| description: pending.description || "", | |
| } | |
| : undefined, | |
| }), | |
| ); |
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/screens/main/components/utils.ts around lines 14 to
32, the code creates a fresh timestamp via new Date().toISOString() for every
pending worktree which changes object references on each call and can trigger
unnecessary re-renders; fix by either (1) adding an optional createdAt parameter
to the function and use that value when provided (so callers can pass a stable
timestamp), or (2) replace the dynamic timestamp with a fixed placeholder (e.g.,
an empty string or constant) if the actual creation time is irrelevant for
pending items, and update the callsite in MainScreen.tsx (line ~670) to memoize
the mapping with useMemo and pass a stable timestamp when using option (1).
Comment on lines
+361
to
+395
| void (async () => { | ||
| try { | ||
| const result = await window.ipcRenderer.invoke("worktree-create", { | ||
| workspaceId: currentWorkspace.id, | ||
| title: task.name, | ||
| branch: task.branch, | ||
| createBranch: false, // Branch should already exist | ||
| description: task.description, | ||
| }); | ||
|
|
||
| if (activeSelection?.worktreeId && activeSelection?.tabId) { | ||
| setSelectedWorktreeId(activeSelection.worktreeId); | ||
| setSelectedTabId(activeSelection.tabId); | ||
| if (result.success && result.worktree) { | ||
| // Remove pending worktree | ||
| setPendingWorktrees((prev) => | ||
| prev.filter((wt) => wt.id !== pendingId), | ||
| ); | ||
| // Refresh workspace to get the real worktree | ||
| await handleWorktreeCreated(); | ||
| setSelectedWorktreeId(result.worktree.id); | ||
| if (result.worktree.tabs && result.worktree.tabs.length > 0) { | ||
| handleTabSelect(result.worktree.id, result.worktree.tabs[0].id); | ||
| } | ||
| } else { | ||
| // Remove pending on failure | ||
| setPendingWorktrees((prev) => | ||
| prev.filter((wt) => wt.id !== pendingId), | ||
| ); | ||
| } | ||
| } catch (error) { | ||
| console.error("Failed to create worktree for task:", error); | ||
| // Remove pending on error | ||
| setPendingWorktrees((prev) => | ||
| prev.filter((wt) => wt.id !== pendingId), | ||
| ); | ||
| } | ||
| } catch (err) { | ||
| setError(err instanceof Error ? err.message : String(err)); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| loadActiveWorkspace(); | ||
| }, []); | ||
|
|
||
| // Listen for workspace-opened event from menu | ||
| useEffect(() => { | ||
| const handler = async (workspace: Workspace) => { | ||
| console.log("[MainScreen] Workspace opened event received:", workspace); | ||
| setLoading(false); | ||
| // Persist the active workspace | ||
| await window.ipcRenderer.invoke( | ||
| "workspace-set-active-workspace-id", | ||
| workspace.id, | ||
| ); | ||
| // Scan for existing worktrees FIRST | ||
| await scanWorktrees(workspace.id); | ||
| // Refresh workspaces list (after scanning to get updated worktrees) | ||
| await loadAllWorkspaces(); | ||
| // Fetch the updated workspace after scanning (this includes imported worktrees) | ||
| const refreshedWorkspace = await window.ipcRenderer.invoke( | ||
| "workspace-get", | ||
| workspace.id, | ||
| ); | ||
| if (refreshedWorkspace) { | ||
| setCurrentWorkspace(refreshedWorkspace); | ||
| } | ||
| }; | ||
|
|
||
| console.log("[MainScreen] Setting up workspace-opened listener"); | ||
| window.ipcRenderer.on("workspace-opened", handler); | ||
| return () => { | ||
| console.log("[MainScreen] Removing workspace-opened listener"); | ||
| window.ipcRenderer.off("workspace-opened", handler); | ||
| }; | ||
| }, []); | ||
|
|
||
| // Listen for terminal exit events and auto-close the tab | ||
| useEffect(() => { | ||
| const handleTerminalExit = async (data: { | ||
| id: string; | ||
| exitCode: number; | ||
| }) => { | ||
| console.log( | ||
| `[MainScreen] Terminal ${data.id} exited with code ${data.exitCode}`, | ||
| ); | ||
|
|
||
| if (!currentWorkspace || !selectedWorktreeId) return; | ||
|
|
||
| // Find which tab contains this terminal | ||
| const worktree = currentWorkspace.worktrees.find( | ||
| (wt) => wt.id === selectedWorktreeId, | ||
| ); | ||
| if (!worktree) return; | ||
|
|
||
| // Check if the exited terminal is the currently selected tab | ||
| const isCurrentTab = selectedTabId === data.id; | ||
|
|
||
| // Find the tab to determine its context (top-level or in group) | ||
| const tabResult = findTabRecursive(worktree.tabs, data.id); | ||
| if (!tabResult) return; | ||
|
|
||
| const parentGroup = tabResult.parent; | ||
| const isInGroup = !!parentGroup; | ||
|
|
||
| // Get the tabs array (either from group or top-level) | ||
| const tabs = isInGroup ? parentGroup?.tabs || [] : worktree.tabs; | ||
| const currentIndex = tabs.findIndex((t) => t.id === data.id); | ||
|
|
||
| // Update mosaic tree if in a group | ||
| if (isInGroup && parentGroup && parentGroup.mosaicTree) { | ||
| const updatedMosaicTree = removeTabFromMosaicTree( | ||
| parentGroup.mosaicTree as MosaicNode<string>, | ||
| data.id, | ||
| ); | ||
|
|
||
| await window.ipcRenderer.invoke("tab-update-mosaic-tree", { | ||
| workspaceId: currentWorkspace.id, | ||
| worktreeId: selectedWorktreeId, | ||
| tabId: parentGroup.id, | ||
| mosaicTree: updatedMosaicTree, | ||
| }); | ||
| } | ||
|
|
||
| // Delete the tab | ||
| const result = await window.ipcRenderer.invoke("tab-delete", { | ||
| workspaceId: currentWorkspace.id, | ||
| worktreeId: selectedWorktreeId, | ||
| tabId: data.id, | ||
| }); | ||
|
|
||
| if (!result.success) { | ||
| console.error("Failed to close exited terminal tab:", result.error); | ||
| return; | ||
| } | ||
|
|
||
| // Temporarily clear selection to force unmount if it's the current tab | ||
| const savedTabId = selectedTabId; | ||
| const savedWorktreeId = selectedWorktreeId; | ||
| if (isCurrentTab) { | ||
| setSelectedTabId(null); | ||
| } | ||
|
|
||
| // Refresh workspace | ||
| const refreshedWorkspace = await window.ipcRenderer.invoke( | ||
| "workspace-get", | ||
| currentWorkspace.id, | ||
| ); | ||
|
|
||
| if (refreshedWorkspace) { | ||
| setCurrentWorkspace(refreshedWorkspace); | ||
| await loadAllWorkspaces(); | ||
|
|
||
| // Wait for next tick to ensure state updates | ||
| await new Promise((resolve) => setTimeout(resolve, 0)); | ||
|
|
||
| // Only select adjacent tab if the exited terminal was the current one | ||
| if (isCurrentTab) { | ||
| const updatedWorktree = refreshedWorkspace.worktrees.find( | ||
| (wt) => wt.id === savedWorktreeId, | ||
| ); | ||
|
|
||
| if (updatedWorktree) { | ||
| if (isInGroup && parentGroup) { | ||
| // Select adjacent tab within the group | ||
| const updatedGroupTab = findTabById( | ||
| updatedWorktree.tabs, | ||
| parentGroup.id, | ||
| ); | ||
| if ( | ||
| updatedGroupTab && | ||
| updatedGroupTab.tabs && | ||
| updatedGroupTab.tabs.length > 0 | ||
| ) { | ||
| const newIndex = Math.min( | ||
| currentIndex, | ||
| updatedGroupTab.tabs.length - 1, | ||
| ); | ||
| handleTabSelect( | ||
| savedWorktreeId, | ||
| updatedGroupTab.tabs[newIndex].id, | ||
| ); | ||
| } else { | ||
| setSelectedTabId(null); | ||
| } | ||
| } else if (updatedWorktree.tabs.length > 0) { | ||
| // Select adjacent top-level tab | ||
| const newIndex = Math.min( | ||
| currentIndex, | ||
| updatedWorktree.tabs.length - 1, | ||
| ); | ||
| handleTabSelect( | ||
| savedWorktreeId, | ||
| updatedWorktree.tabs[newIndex].id, | ||
| ); | ||
| } else { | ||
| setSelectedTabId(null); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| window.ipcRenderer.on("terminal-exited", handleTerminalExit); | ||
| return () => { | ||
| window.ipcRenderer.off("terminal-exited", handleTerminalExit); | ||
| }; | ||
| }, [currentWorkspace, selectedWorktreeId, selectedTabId]); | ||
|
|
||
| // Helper: recursively find a tab by ID | ||
| const findTabById = (tabs: Tab[], tabId: string): Tab | null => { | ||
| for (const tab of tabs) { | ||
| if (tab.id === tabId) return tab; | ||
| if (tab.type === "group" && tab.tabs) { | ||
| const found = findTabById(tab.tabs, tabId); | ||
| if (found) return found; | ||
| } | ||
| } | ||
| return null; | ||
| }; | ||
|
|
||
| // Helper: Remove tab ID from mosaic tree | ||
| const removeTabFromMosaicTree = ( | ||
| tree: MosaicNode<string>, | ||
| tabId: string, | ||
| ): MosaicNode<string> | null => { | ||
| if (typeof tree === "string") { | ||
| // If this is the tab to remove, return null | ||
| return tree === tabId ? null : tree; | ||
| } | ||
|
|
||
| // Recursively remove from branches | ||
| const newFirst = removeTabFromMosaicTree(tree.first, tabId); | ||
| const newSecond = removeTabFromMosaicTree(tree.second, tabId); | ||
|
|
||
| // If both branches are gone, return null | ||
| if (!newFirst && !newSecond) { | ||
| return null; | ||
| } | ||
|
|
||
| // If one branch is gone, return the other | ||
| if (!newFirst) { | ||
| return newSecond; | ||
| } | ||
| if (!newSecond) { | ||
| return newFirst; | ||
| })(); |
Contributor
There was a problem hiding this comment.
Improve error notification to user.
The error handling in lines 388-394 only logs errors and removes the pending worktree. Users won't know that worktree creation failed.
Apply this diff to notify users of errors:
} catch (error) {
console.error("Failed to create worktree for task:", error);
+ const errorMessage =
+ error instanceof Error ? error.message : String(error);
+ alert(`Failed to open task: ${errorMessage}`);
// Remove pending on error
setPendingWorktrees((prev) =>
prev.filter((wt) => wt.id !== pendingId),
);
}📝 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.
Suggested change
| void (async () => { | |
| try { | |
| const result = await window.ipcRenderer.invoke("worktree-create", { | |
| workspaceId: currentWorkspace.id, | |
| title: task.name, | |
| branch: task.branch, | |
| createBranch: false, // Branch should already exist | |
| description: task.description, | |
| }); | |
| if (activeSelection?.worktreeId && activeSelection?.tabId) { | |
| setSelectedWorktreeId(activeSelection.worktreeId); | |
| setSelectedTabId(activeSelection.tabId); | |
| if (result.success && result.worktree) { | |
| // Remove pending worktree | |
| setPendingWorktrees((prev) => | |
| prev.filter((wt) => wt.id !== pendingId), | |
| ); | |
| // Refresh workspace to get the real worktree | |
| await handleWorktreeCreated(); | |
| setSelectedWorktreeId(result.worktree.id); | |
| if (result.worktree.tabs && result.worktree.tabs.length > 0) { | |
| handleTabSelect(result.worktree.id, result.worktree.tabs[0].id); | |
| } | |
| } else { | |
| // Remove pending on failure | |
| setPendingWorktrees((prev) => | |
| prev.filter((wt) => wt.id !== pendingId), | |
| ); | |
| } | |
| } catch (error) { | |
| console.error("Failed to create worktree for task:", error); | |
| // Remove pending on error | |
| setPendingWorktrees((prev) => | |
| prev.filter((wt) => wt.id !== pendingId), | |
| ); | |
| } | |
| } catch (err) { | |
| setError(err instanceof Error ? err.message : String(err)); | |
| } finally { | |
| setLoading(false); | |
| } | |
| }; | |
| loadActiveWorkspace(); | |
| }, []); | |
| // Listen for workspace-opened event from menu | |
| useEffect(() => { | |
| const handler = async (workspace: Workspace) => { | |
| console.log("[MainScreen] Workspace opened event received:", workspace); | |
| setLoading(false); | |
| // Persist the active workspace | |
| await window.ipcRenderer.invoke( | |
| "workspace-set-active-workspace-id", | |
| workspace.id, | |
| ); | |
| // Scan for existing worktrees FIRST | |
| await scanWorktrees(workspace.id); | |
| // Refresh workspaces list (after scanning to get updated worktrees) | |
| await loadAllWorkspaces(); | |
| // Fetch the updated workspace after scanning (this includes imported worktrees) | |
| const refreshedWorkspace = await window.ipcRenderer.invoke( | |
| "workspace-get", | |
| workspace.id, | |
| ); | |
| if (refreshedWorkspace) { | |
| setCurrentWorkspace(refreshedWorkspace); | |
| } | |
| }; | |
| console.log("[MainScreen] Setting up workspace-opened listener"); | |
| window.ipcRenderer.on("workspace-opened", handler); | |
| return () => { | |
| console.log("[MainScreen] Removing workspace-opened listener"); | |
| window.ipcRenderer.off("workspace-opened", handler); | |
| }; | |
| }, []); | |
| // Listen for terminal exit events and auto-close the tab | |
| useEffect(() => { | |
| const handleTerminalExit = async (data: { | |
| id: string; | |
| exitCode: number; | |
| }) => { | |
| console.log( | |
| `[MainScreen] Terminal ${data.id} exited with code ${data.exitCode}`, | |
| ); | |
| if (!currentWorkspace || !selectedWorktreeId) return; | |
| // Find which tab contains this terminal | |
| const worktree = currentWorkspace.worktrees.find( | |
| (wt) => wt.id === selectedWorktreeId, | |
| ); | |
| if (!worktree) return; | |
| // Check if the exited terminal is the currently selected tab | |
| const isCurrentTab = selectedTabId === data.id; | |
| // Find the tab to determine its context (top-level or in group) | |
| const tabResult = findTabRecursive(worktree.tabs, data.id); | |
| if (!tabResult) return; | |
| const parentGroup = tabResult.parent; | |
| const isInGroup = !!parentGroup; | |
| // Get the tabs array (either from group or top-level) | |
| const tabs = isInGroup ? parentGroup?.tabs || [] : worktree.tabs; | |
| const currentIndex = tabs.findIndex((t) => t.id === data.id); | |
| // Update mosaic tree if in a group | |
| if (isInGroup && parentGroup && parentGroup.mosaicTree) { | |
| const updatedMosaicTree = removeTabFromMosaicTree( | |
| parentGroup.mosaicTree as MosaicNode<string>, | |
| data.id, | |
| ); | |
| await window.ipcRenderer.invoke("tab-update-mosaic-tree", { | |
| workspaceId: currentWorkspace.id, | |
| worktreeId: selectedWorktreeId, | |
| tabId: parentGroup.id, | |
| mosaicTree: updatedMosaicTree, | |
| }); | |
| } | |
| // Delete the tab | |
| const result = await window.ipcRenderer.invoke("tab-delete", { | |
| workspaceId: currentWorkspace.id, | |
| worktreeId: selectedWorktreeId, | |
| tabId: data.id, | |
| }); | |
| if (!result.success) { | |
| console.error("Failed to close exited terminal tab:", result.error); | |
| return; | |
| } | |
| // Temporarily clear selection to force unmount if it's the current tab | |
| const savedTabId = selectedTabId; | |
| const savedWorktreeId = selectedWorktreeId; | |
| if (isCurrentTab) { | |
| setSelectedTabId(null); | |
| } | |
| // Refresh workspace | |
| const refreshedWorkspace = await window.ipcRenderer.invoke( | |
| "workspace-get", | |
| currentWorkspace.id, | |
| ); | |
| if (refreshedWorkspace) { | |
| setCurrentWorkspace(refreshedWorkspace); | |
| await loadAllWorkspaces(); | |
| // Wait for next tick to ensure state updates | |
| await new Promise((resolve) => setTimeout(resolve, 0)); | |
| // Only select adjacent tab if the exited terminal was the current one | |
| if (isCurrentTab) { | |
| const updatedWorktree = refreshedWorkspace.worktrees.find( | |
| (wt) => wt.id === savedWorktreeId, | |
| ); | |
| if (updatedWorktree) { | |
| if (isInGroup && parentGroup) { | |
| // Select adjacent tab within the group | |
| const updatedGroupTab = findTabById( | |
| updatedWorktree.tabs, | |
| parentGroup.id, | |
| ); | |
| if ( | |
| updatedGroupTab && | |
| updatedGroupTab.tabs && | |
| updatedGroupTab.tabs.length > 0 | |
| ) { | |
| const newIndex = Math.min( | |
| currentIndex, | |
| updatedGroupTab.tabs.length - 1, | |
| ); | |
| handleTabSelect( | |
| savedWorktreeId, | |
| updatedGroupTab.tabs[newIndex].id, | |
| ); | |
| } else { | |
| setSelectedTabId(null); | |
| } | |
| } else if (updatedWorktree.tabs.length > 0) { | |
| // Select adjacent top-level tab | |
| const newIndex = Math.min( | |
| currentIndex, | |
| updatedWorktree.tabs.length - 1, | |
| ); | |
| handleTabSelect( | |
| savedWorktreeId, | |
| updatedWorktree.tabs[newIndex].id, | |
| ); | |
| } else { | |
| setSelectedTabId(null); | |
| } | |
| } | |
| } | |
| } | |
| }; | |
| window.ipcRenderer.on("terminal-exited", handleTerminalExit); | |
| return () => { | |
| window.ipcRenderer.off("terminal-exited", handleTerminalExit); | |
| }; | |
| }, [currentWorkspace, selectedWorktreeId, selectedTabId]); | |
| // Helper: recursively find a tab by ID | |
| const findTabById = (tabs: Tab[], tabId: string): Tab | null => { | |
| for (const tab of tabs) { | |
| if (tab.id === tabId) return tab; | |
| if (tab.type === "group" && tab.tabs) { | |
| const found = findTabById(tab.tabs, tabId); | |
| if (found) return found; | |
| } | |
| } | |
| return null; | |
| }; | |
| // Helper: Remove tab ID from mosaic tree | |
| const removeTabFromMosaicTree = ( | |
| tree: MosaicNode<string>, | |
| tabId: string, | |
| ): MosaicNode<string> | null => { | |
| if (typeof tree === "string") { | |
| // If this is the tab to remove, return null | |
| return tree === tabId ? null : tree; | |
| } | |
| // Recursively remove from branches | |
| const newFirst = removeTabFromMosaicTree(tree.first, tabId); | |
| const newSecond = removeTabFromMosaicTree(tree.second, tabId); | |
| // If both branches are gone, return null | |
| if (!newFirst && !newSecond) { | |
| return null; | |
| } | |
| // If one branch is gone, return the other | |
| if (!newFirst) { | |
| return newSecond; | |
| } | |
| if (!newSecond) { | |
| return newFirst; | |
| })(); | |
| void (async () => { | |
| try { | |
| const result = await window.ipcRenderer.invoke("worktree-create", { | |
| workspaceId: currentWorkspace.id, | |
| title: task.name, | |
| branch: task.branch, | |
| createBranch: false, // Branch should already exist | |
| description: task.description, | |
| }); | |
| if (result.success && result.worktree) { | |
| // Remove pending worktree | |
| setPendingWorktrees((prev) => | |
| prev.filter((wt) => wt.id !== pendingId), | |
| ); | |
| // Refresh workspace to get the real worktree | |
| await handleWorktreeCreated(); | |
| setSelectedWorktreeId(result.worktree.id); | |
| if (result.worktree.tabs && result.worktree.tabs.length > 0) { | |
| handleTabSelect(result.worktree.id, result.worktree.tabs[0].id); | |
| } | |
| } else { | |
| // Remove pending on failure | |
| setPendingWorktrees((prev) => | |
| prev.filter((wt) => wt.id !== pendingId), | |
| ); | |
| } | |
| } catch (error) { | |
| console.error("Failed to create worktree for task:", error); | |
| const errorMessage = | |
| error instanceof Error ? error.message : String(error); | |
| alert(`Failed to open task: ${errorMessage}`); | |
| // Remove pending on error | |
| setPendingWorktrees((prev) => | |
| prev.filter((wt) => wt.id !== pendingId), | |
| ); | |
| } | |
| })(); |
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/screens/main/MainScreen.tsx around lines 361-395,
the catch block only logs errors and removes the pending worktree so the user is
unaware the creation failed; update the catch and the failure branch to display
a user-facing error notification (use the app's existing notification/toast API
— e.g., notifyError/toast/error — or fallback to window.alert) including the
error message or result.error, while preserving the existing removal of the
pending worktree and logging; ensure both the else branch (result.success false)
and the catch block show the notification with a clear message like "Failed to
create worktree" plus the error details.
Comment on lines
+427
to
+465
| void (async () => { | ||
| try { | ||
| // Create a worktree for this task | ||
| const result = await window.ipcRenderer.invoke("worktree-create", { | ||
| workspaceId: currentWorkspace.id, | ||
| title: taskData.name, | ||
| branch: taskData.branch, | ||
| createBranch: true, | ||
| description: taskData.description, | ||
| }); | ||
|
|
||
| // Refresh workspace to show the updated structure | ||
| const refreshedWorkspace = await window.ipcRenderer.invoke( | ||
| "workspace-get", | ||
| currentWorkspace.id, | ||
| if (result.success && result.worktree) { | ||
| // Remove pending worktree | ||
| setPendingWorktrees((prev) => | ||
| prev.filter((wt) => wt.id !== pendingId), | ||
| ); | ||
| if (refreshedWorkspace) { | ||
| setCurrentWorkspace(refreshedWorkspace); | ||
| } | ||
| } catch (error) { | ||
| console.error("[MainScreen] Error adding tab to group:", error); | ||
| } | ||
| } | ||
| // Case 2: Currently viewing a single tab - create a new group with both tabs | ||
| else if (selectedTab) { | ||
| try { | ||
| // If dragging a tab onto itself, create a new duplicate tab for the split | ||
| let secondTabId = draggedTabId; | ||
| const parentTabId = activeData.parentTabId; | ||
|
|
||
| if (draggedTabId === selectedTab.id) { | ||
| console.log( | ||
| "[MainScreen] Dragging tab onto itself - creating duplicate tab for split", | ||
| ); | ||
|
|
||
| // Create a new tab with the same type and name | ||
| const newTab = await createTab( | ||
| currentWorkspace.id, | ||
| selectedWorktreeId, | ||
| selectedTab.name, | ||
| selectedTab.type, | ||
| ); | ||
| // Reload workspace to get the new worktree | ||
| await handleWorktreeCreated(); | ||
|
|
||
| if (!newTab) return; | ||
| // Switch to the new worktree | ||
| setSelectedWorktreeId(result.worktree.id); | ||
|
|
||
| secondTabId = newTab.id; | ||
| // Select first tab if available | ||
| if (result.worktree.tabs && result.worktree.tabs.length > 0) { | ||
| handleTabSelect(result.worktree.id, result.worktree.tabs[0].id); | ||
| } | ||
|
|
||
| // Create a new group tab | ||
| const groupTab = await createTab( | ||
| currentWorkspace.id, | ||
| selectedWorktreeId, | ||
| "Tab Group", | ||
| "group", | ||
| ); | ||
|
|
||
| if (!groupTab) return; | ||
|
|
||
| const groupTabId = groupTab.id; | ||
|
|
||
| // Move both tabs into the group | ||
| // First, move the currently selected tab | ||
| await window.ipcRenderer.invoke("tab-move", { | ||
| workspaceId: currentWorkspace.id, | ||
| worktreeId: selectedWorktreeId, | ||
| tabId: selectedTab.id, | ||
| sourceParentTabId: undefined, | ||
| targetParentTabId: groupTabId, | ||
| targetIndex: 0, | ||
| }); | ||
|
|
||
| // Then, move the second tab (either the dragged tab or the newly created one) | ||
| await window.ipcRenderer.invoke("tab-move", { | ||
| workspaceId: currentWorkspace.id, | ||
| worktreeId: selectedWorktreeId, | ||
| tabId: secondTabId, | ||
| sourceParentTabId: | ||
| secondTabId === draggedTabId ? parentTabId : undefined, | ||
| targetParentTabId: groupTabId, | ||
| targetIndex: 1, | ||
| }); | ||
|
|
||
| // Create a simple mosaic tree with both tabs | ||
| const mosaicTree: MosaicNode<string> = { | ||
| direction: "row", | ||
| first: selectedTab.id, | ||
| second: secondTabId, | ||
| splitPercentage: 50, | ||
| }; | ||
|
|
||
| await window.ipcRenderer.invoke("tab-update-mosaic-tree", { | ||
| workspaceId: currentWorkspace.id, | ||
| worktreeId: selectedWorktreeId, | ||
| tabId: groupTabId, | ||
| mosaicTree, | ||
| }); | ||
|
|
||
| // Select the new group tab to show the mosaic | ||
| setSelectedTabId(groupTabId); | ||
| await window.ipcRenderer.invoke("workspace-set-active-selection", { | ||
| workspaceId: currentWorkspace.id, | ||
| worktreeId: selectedWorktreeId, | ||
| tabId: groupTabId, | ||
| }); | ||
|
|
||
| // Refresh workspace to show the updated structure | ||
| const refreshedWorkspace = await window.ipcRenderer.invoke( | ||
| "workspace-get", | ||
| currentWorkspace.id, | ||
| } else { | ||
| // Remove pending on failure | ||
| setPendingWorktrees((prev) => | ||
| prev.filter((wt) => wt.id !== pendingId), | ||
| ); | ||
| if (refreshedWorkspace) { | ||
| setCurrentWorkspace(refreshedWorkspace); | ||
| } | ||
| } catch (error) { | ||
| console.error("[MainScreen] Error creating tab group:", error); | ||
| } | ||
| } catch (error) { | ||
| console.error("Failed to create task/worktree:", error); | ||
| // Remove pending on error | ||
| setPendingWorktrees((prev) => prev.filter((wt) => wt.id !== pendingId)); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| // Handle sidebar drag operations (reordering, moving between groups) | ||
| const draggedWorktreeId = activeData.worktreeId as string; | ||
| const draggedTabId = active.id as string; | ||
| const activeParentTabId = activeData.parentTabId; | ||
| const overParentTabId = overData?.parentTabId; | ||
| })(); |
Contributor
There was a problem hiding this comment.
Add user notification for task creation errors.
Similar to handleSelectTask, lines 460-464 only log errors without notifying the user.
Apply this diff:
} catch (error) {
console.error("Failed to create task/worktree:", error);
+ const errorMessage =
+ error instanceof Error ? error.message : String(error);
+ alert(`Failed to create task: ${errorMessage}`);
// Remove pending on error
setPendingWorktrees((prev) => prev.filter((wt) => wt.id !== pendingId));
}📝 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.
Suggested change
| void (async () => { | |
| try { | |
| // Create a worktree for this task | |
| const result = await window.ipcRenderer.invoke("worktree-create", { | |
| workspaceId: currentWorkspace.id, | |
| title: taskData.name, | |
| branch: taskData.branch, | |
| createBranch: true, | |
| description: taskData.description, | |
| }); | |
| // Refresh workspace to show the updated structure | |
| const refreshedWorkspace = await window.ipcRenderer.invoke( | |
| "workspace-get", | |
| currentWorkspace.id, | |
| if (result.success && result.worktree) { | |
| // Remove pending worktree | |
| setPendingWorktrees((prev) => | |
| prev.filter((wt) => wt.id !== pendingId), | |
| ); | |
| if (refreshedWorkspace) { | |
| setCurrentWorkspace(refreshedWorkspace); | |
| } | |
| } catch (error) { | |
| console.error("[MainScreen] Error adding tab to group:", error); | |
| } | |
| } | |
| // Case 2: Currently viewing a single tab - create a new group with both tabs | |
| else if (selectedTab) { | |
| try { | |
| // If dragging a tab onto itself, create a new duplicate tab for the split | |
| let secondTabId = draggedTabId; | |
| const parentTabId = activeData.parentTabId; | |
| if (draggedTabId === selectedTab.id) { | |
| console.log( | |
| "[MainScreen] Dragging tab onto itself - creating duplicate tab for split", | |
| ); | |
| // Create a new tab with the same type and name | |
| const newTab = await createTab( | |
| currentWorkspace.id, | |
| selectedWorktreeId, | |
| selectedTab.name, | |
| selectedTab.type, | |
| ); | |
| // Reload workspace to get the new worktree | |
| await handleWorktreeCreated(); | |
| if (!newTab) return; | |
| // Switch to the new worktree | |
| setSelectedWorktreeId(result.worktree.id); | |
| secondTabId = newTab.id; | |
| // Select first tab if available | |
| if (result.worktree.tabs && result.worktree.tabs.length > 0) { | |
| handleTabSelect(result.worktree.id, result.worktree.tabs[0].id); | |
| } | |
| // Create a new group tab | |
| const groupTab = await createTab( | |
| currentWorkspace.id, | |
| selectedWorktreeId, | |
| "Tab Group", | |
| "group", | |
| ); | |
| if (!groupTab) return; | |
| const groupTabId = groupTab.id; | |
| // Move both tabs into the group | |
| // First, move the currently selected tab | |
| await window.ipcRenderer.invoke("tab-move", { | |
| workspaceId: currentWorkspace.id, | |
| worktreeId: selectedWorktreeId, | |
| tabId: selectedTab.id, | |
| sourceParentTabId: undefined, | |
| targetParentTabId: groupTabId, | |
| targetIndex: 0, | |
| }); | |
| // Then, move the second tab (either the dragged tab or the newly created one) | |
| await window.ipcRenderer.invoke("tab-move", { | |
| workspaceId: currentWorkspace.id, | |
| worktreeId: selectedWorktreeId, | |
| tabId: secondTabId, | |
| sourceParentTabId: | |
| secondTabId === draggedTabId ? parentTabId : undefined, | |
| targetParentTabId: groupTabId, | |
| targetIndex: 1, | |
| }); | |
| // Create a simple mosaic tree with both tabs | |
| const mosaicTree: MosaicNode<string> = { | |
| direction: "row", | |
| first: selectedTab.id, | |
| second: secondTabId, | |
| splitPercentage: 50, | |
| }; | |
| await window.ipcRenderer.invoke("tab-update-mosaic-tree", { | |
| workspaceId: currentWorkspace.id, | |
| worktreeId: selectedWorktreeId, | |
| tabId: groupTabId, | |
| mosaicTree, | |
| }); | |
| // Select the new group tab to show the mosaic | |
| setSelectedTabId(groupTabId); | |
| await window.ipcRenderer.invoke("workspace-set-active-selection", { | |
| workspaceId: currentWorkspace.id, | |
| worktreeId: selectedWorktreeId, | |
| tabId: groupTabId, | |
| }); | |
| // Refresh workspace to show the updated structure | |
| const refreshedWorkspace = await window.ipcRenderer.invoke( | |
| "workspace-get", | |
| currentWorkspace.id, | |
| } else { | |
| // Remove pending on failure | |
| setPendingWorktrees((prev) => | |
| prev.filter((wt) => wt.id !== pendingId), | |
| ); | |
| if (refreshedWorkspace) { | |
| setCurrentWorkspace(refreshedWorkspace); | |
| } | |
| } catch (error) { | |
| console.error("[MainScreen] Error creating tab group:", error); | |
| } | |
| } catch (error) { | |
| console.error("Failed to create task/worktree:", error); | |
| // Remove pending on error | |
| setPendingWorktrees((prev) => prev.filter((wt) => wt.id !== pendingId)); | |
| } | |
| return; | |
| } | |
| // Handle sidebar drag operations (reordering, moving between groups) | |
| const draggedWorktreeId = activeData.worktreeId as string; | |
| const draggedTabId = active.id as string; | |
| const activeParentTabId = activeData.parentTabId; | |
| const overParentTabId = overData?.parentTabId; | |
| })(); | |
| void (async () => { | |
| try { | |
| // Create a worktree for this task | |
| const result = await window.ipcRenderer.invoke("worktree-create", { | |
| workspaceId: currentWorkspace.id, | |
| title: taskData.name, | |
| branch: taskData.branch, | |
| createBranch: true, | |
| description: taskData.description, | |
| }); | |
| if (result.success && result.worktree) { | |
| // Remove pending worktree | |
| setPendingWorktrees((prev) => | |
| prev.filter((wt) => wt.id !== pendingId), | |
| ); | |
| // Reload workspace to get the new worktree | |
| await handleWorktreeCreated(); | |
| // Switch to the new worktree | |
| setSelectedWorktreeId(result.worktree.id); | |
| // Select first tab if available | |
| if (result.worktree.tabs && result.worktree.tabs.length > 0) { | |
| handleTabSelect(result.worktree.id, result.worktree.tabs[0].id); | |
| } | |
| } else { | |
| // Remove pending on failure | |
| setPendingWorktrees((prev) => | |
| prev.filter((wt) => wt.id !== pendingId), | |
| ); | |
| } | |
| } catch (error) { | |
| console.error("Failed to create task/worktree:", error); | |
| const errorMessage = | |
| error instanceof Error ? error.message : String(error); | |
| alert(`Failed to create task: ${errorMessage}`); | |
| // Remove pending on error | |
| setPendingWorktrees((prev) => prev.filter((wt) => wt.id !== pendingId)); | |
| } | |
| })(); |
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/screens/main/MainScreen.tsx around lines 427-465,
the worktree/task creation flow only logs failures to console and removes
pending worktrees but does not notify the user; add user-facing error
notifications in both the failure branch (result.success false) and the catch
block using the same notification/toast helper used by handleSelectTask so users
are informed of the failure, include a concise message like "Failed to create
task/worktree" and append brief error details if available, and keep the
existing pendingWorktrees removal behavior.
Comment on lines
+670
to
+673
| worktrees={enrichWorktreesWithTasks( | ||
| currentWorkspace?.worktrees || [], | ||
| pendingWorktrees, | ||
| )} |
Contributor
There was a problem hiding this comment.
Memoize enrichWorktreesWithTasks call to prevent unnecessary re-renders.
The call to enrichWorktreesWithTasks on lines 670-673 runs on every render. Combined with the timestamp creation inside that function (utils.ts line 20), this causes new object references on each render, potentially triggering unnecessary re-renders of child components.
Add this memoized value before the return statement:
+ const enrichedWorktrees = useMemo(
+ () =>
+ enrichWorktreesWithTasks(
+ currentWorkspace?.worktrees || [],
+ pendingWorktrees,
+ ),
+ [currentWorkspace?.worktrees, pendingWorktrees],
+ );
+
return (Then use it in the JSX:
<TaskTabs
onCollapseSidebar={handleCollapseSidebar}
onExpandSidebar={handleExpandSidebar}
isSidebarOpen={isSidebarOpen}
onAddTask={handleOpenAddTaskModal}
onCreatePR={handleCreatePR}
onMergePR={handleMergePR}
- worktrees={enrichWorktreesWithTasks(
- currentWorkspace?.worktrees || [],
- pendingWorktrees,
- )}
+ worktrees={enrichedWorktrees}
selectedWorktreeId={selectedWorktreeId}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/screens/main/MainScreen.tsx around lines 670 to
673, the call to enrichWorktreesWithTasks is executed on every render which (due
to timestamp creation inside that helper) produces new object references and
causes unnecessary child re-renders; memoize the result with React.useMemo by
creating a variable (e.g. enrichedWorktrees) before the return that calls
enrichWorktreesWithTasks only when its inputs change, include
currentWorkspace?.worktrees and pendingWorktrees in the dependency array,
replace the inline call in JSX with the memoized variable, and ensure useMemo is
imported from React.
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 by CodeRabbit
New Features
Refactor
Chores