feat(desktop): add Start Working flow to launch Claude from tasks#1243
Conversation
Adds a dialog that lets users select a project, create a workspace with a task-derived branch name, and launch Claude autonomously with full task context. Accessible from the task detail page header and the task table context menu.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a "Start Working" feature: modal state store, StartWorkingDialog component, task integrations (page button + context menu), table row selection and selection-mode UI, a task-context formatter, layout route to host the dialog, and a hook option to transform initial workspace commands. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant UI as Task Page / Menu
participant Store as StartWorking Modal Store
participant Dialog as StartWorkingDialog
participant Projects as Projects Service
participant Workspaces as Workspaces Service
User->>UI: Click "Start Working" (button or menu)
UI->>Store: openStartWorkingModal(task(s))
Store->>Dialog: modal opens with tasks
Dialog->>Projects: request recent projects / import repo
Projects-->>Dialog: return projects / import result
User->>Dialog: select project, add context, confirm
Dialog->>Workspaces: createWorkspace / createBatchWorkspace (resolveInitialCommands used if provided)
Workspaces-->>Dialog: success / per-task progress
Dialog->>Store: closeModal()
Dialog-->>User: show completion toasts / update UI
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 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 |
Kitenite
left a comment
There was a problem hiding this comment.
Code Review: feat(desktop): add Start Working flow to launch Claude from tasks
Nice feature! The "Start Working" flow that creates a workspace and launches Claude with full task context is well-structured and follows existing codebase patterns closely. Here's my detailed review:
Strengths
- Store pattern —
start-working-modal.tsperfectly mirrors the existingnew-workspace-modal.tsandconfig-modal.tsstores:devtoolsmiddleware,isOpen/taskstate, action methods, convenience selector hooks. Exactly right. - Dialog structure —
StartWorkingDialogclosely followsNewWorkspaceModalfor project selection, import repo flow, form reset, error handling with toast, and keyboard shortcut. Consistent and clean. - Co-location —
StartWorkingDialog/undertasks/components/,formatTaskContext.tsundertasks/utils/, and the store atrenderer/stores/all follow the project structure conventions. - Object params —
formatTaskContext({ task, additionalContext })correctly uses object signature for 2+ parameters per AGENTS.md. - Heredoc safety — Using a random UUID delimiter (
SUPERSET_PROMPT_<uuid>) for the heredoc is smart and prevents injection from task content containingEOFor similar strings.
Issues
1. addPendingTerminalSetup override is fragile coupling (Medium)
StartWorkingDialog.tsx:119-123
useCreateWorkspace internally calls addPendingTerminalSetup in its onSuccess callback, and then this dialog calls it again immediately after mutateAsync resolves to override the initial commands with the Claude prompt. This works because the store uses workspaceId as a key and the second write overwrites the first, but it's a fragile implicit dependency — if useCreateWorkspace ever changes its timing or the store deduplication changes, this breaks silently.
Suggestion: Consider passing a skipTerminalSetup: true option to useCreateWorkspace (or using onSuccess override) so the intent is explicit rather than relying on overwrite ordering. Alternatively, add a code comment explaining the deliberate override.
2. Priority "none" leaks into Claude prompt metadata (Low)
formatTaskContext.ts:12
const metadata = [
`Priority: ${task.priority}`,
// ...This always includes priority, even when it's "none". The dialog UI correctly filters this with task.priority !== "none" in the badge, but the generated prompt will include Priority: none which is noise for Claude. Should filter consistently:
task.priority && task.priority !== "none" && `Priority: ${task.priority}`,3. Missing baseBranch parameter (Low)
StartWorkingDialog.tsx:109-113
NewWorkspaceModal passes baseBranch to createWorkspace.mutateAsync, but StartWorkingDialog does not. This means the workspace always branches from the project's default branch. This is probably fine for the "quick start" use case, but worth confirming this is intentional.
4. task.id passed to prompt may not match Superset task ID (Low)
formatTaskContext.ts:42
5. When done, use the Superset MCP \`update_task\` tool to update task "${task.id}" with a summary of what was doneIs task.id the Superset/local task ID that the MCP update_task tool expects? If it's a Linear sync ID or some other identifier, the MCP call in the generated prompt will fail silently. Worth verifying.
Nits
formatTaskContext.ts— The utility has no correspondingindex.tsbarrel file, unlike the component folders. Minor inconsistency but follows howTasksView/utils/sorting/does it (hasindex.ts). Consider adding one for consistency.- Dialog rendered in two places —
StartWorkingDialogis rendered in bothtasks/page.tsxandtasks/$taskId/page.tsx. These are separate leaf routes so they're never mounted simultaneously, so this is fine functionally. But if a tasks layout route is ever added, this should be consolidated.
Summary
Clean, well-structured PR that follows established patterns. The main thing I'd want addressed is the addPendingTerminalSetup override coupling (issue #1) — either make it explicit or add a comment. The priority filtering (issue #2) is a quick fix. The rest are minor considerations.
LGTM with minor changes.
…unting - Add `resolveInitialCommands` option to `useCreateWorkspace` so callers can override server commands in a single `addPendingTerminalSetup` call - Move `StartWorkingDialog` to a shared `tasks/layout.tsx` instead of mounting it in both the list and detail pages
Add checkboxes to task list rows for multi-selection, with a selection action bar in the top bar and batch workspace creation in the Start Working dialog.
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)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/hooks/useTasksTable/useTasksTable.tsx (1)
107-123:⚠️ Potential issue | 🟡 MinorRow selection not reset when
searchQuerychanges.When the user types a search query, rows that no longer match are hidden but remain in
rowSelection. This means the selection bar could show "3 selected" while only 1 of those rows is visible, and "Start Working" would operate on invisible tasks.Consider adding
searchQueryto the reset trigger, or resetting selection in a separate effect:Proposed fix
useEffect(() => { const newColumnFilters: ColumnFiltersState = []; if (filterTab !== "all") { newColumnFilters.push({ id: "status", value: filterTab, }); } if (assigneeFilter !== null) { newColumnFilters.push({ id: "assigneeId", value: assigneeFilter, }); } setColumnFilters(newColumnFilters); setRowSelection({}); - }, [filterTab, assigneeFilter]); + }, [filterTab, assigneeFilter, searchQuery]);
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/StartWorkingDialog/StartWorkingDialog.tsx`:
- Around line 175-218: The batch-create loop in handleBatchCreate continues
after handleClose, so either prevent closing while batch runs or abort the loop:
add a ref like abortBatchRef (initially false) and set abortBatchRef.current =
true inside handleClose; in handleBatchCreate check abortBatchRef.current before
each iteration (and after await) and break if true, and reset
abortBatchRef.current = false when starting; alternatively disable the Dialog's
onOpenChange/close triggers while batchProgress is active (e.g., pass a prop
like disabledClose={batchProgress.current>0} or guard the onOpenChange handler)
so users cannot dismiss during processing. Ensure references to
currentBatchTaskRef and createBatchWorkspace.mutateAsync remain unchanged and
reset state appropriately after abort or completion.
🧹 Nitpick comments (3)
apps/desktop/src/renderer/stores/start-working-modal.ts (1)
1-1: Consider extractingTaskWithStatusto a shared types module.This global store imports a type from a deeply nested route-specific hook. While it's a type-only import (no runtime coupling), it creates an architectural dependency from
renderer/stores/→ route internals. ExtractingTaskWithStatusto a shared types location (e.g.,renderer/types/or a shared schema) would improve cohesion.apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/StartWorkingDialog/StartWorkingDialog.tsx (1)
335-347: Nit:.filter((project) => project.id)silently drops projects with falsy IDs.This is likely defensive, but if a project ever has an empty-string or
nullID due to a data issue, it would silently vanish from the dropdown with no indication. Consider whether a stricter type on the query result would be more appropriate, so this filter becomes unnecessary.apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/TasksView.tsx (1)
43-53:useMemodependency ontablelikely defeats memoization.
useReactTablereturns a new object reference on each render, so[rowSelection, table]causesselectedTasksto recompute every render. This is fine for typical task list sizes, but if you want effective memoization, consider deriving from the data array androwSelectiondirectly:Alternative approach
const selectedTasks = useMemo(() => { const selectedIds = Object.keys(rowSelection).filter( (id) => rowSelection[id], ); if (selectedIds.length === 0) return []; - - return table - .getRowModel() - .rows.filter((row) => row.getIsSelected() && !row.getIsGrouped()) - .map((row) => row.original); - }, [rowSelection, table]); + const selectedSet = new Set(selectedIds); + return table + .getRowModel() + .rows.filter((row) => selectedSet.has(row.id) && !row.getIsGrouped()) + .map((row) => row.original); + }, [rowSelection, table]);Even with this,
tableinstability remains. For true memoization, you'd need to pull the flat data from the hook and filter it byrowSelectionkeys without depending ontable.
- Prevent dialog close during batch operation (Escape, click outside) - Clear row selection when Start Working modal closes - Exclude group rows from selection at data-model level - Add aria-label to clear selection button - Simplify selectedTasks memo (remove unused variable) - Fix JSX indentation inside scrollable dialog wrapper
There was a problem hiding this comment.
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/routes/_authenticated/_dashboard/tasks/components/TasksView/hooks/useTasksTable/useTasksTable.tsx (1)
107-123:⚠️ Potential issue | 🟡 MinorRow selection not reset when
searchQuerychanges.When the search query changes, previously selected rows may no longer be visible in the filtered results, but
rowSelectionretains their IDs. This can lead to stale selections—e.g., theselectedTasksderivation inTasksViewwould still find them intable.getRowModel().rowsonly if they happen to match the new query, but the selection count could be confusing. Consider addingsearchQueryto the dependency list (or clearing selection in the parent when search changes).Proposed fix
}, [filterTab, assigneeFilter]); + + // Also reset selection when search query changes + useEffect(() => { + setRowSelection({}); + }, [searchQuery]);Alternatively, merge into the existing effect:
- }, [filterTab, assigneeFilter]); + }, [filterTab, assigneeFilter, searchQuery]);
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/TasksView.tsx (1)
47-54:tablereference identity defeats memoization.
useReactTablereturns a new object on every render, sotablein the dependency array causesselectedTasksto recompute every render regardless. The computation is lightweight, so this isn't a bug, but if you want genuine memoization you could depend ontable.getRowModel().rowsor extract the row model into a variable first. Alternatively, droppinguseMemoentirely (just a plain derivation) would be more honest about what's happening.
- Preserve active tab (backlog/active) when navigating to task detail and back via URL search params on the tasks layout route - Show per-task progress indicators during batch workspace creation (pending/creating/done/failed icons for each task) - Persist row selection across navigation using zustand store - Skip clearing row selection on initial mount
- Combine wasExisting guard with resolveInitialCommands in useCreateWorkspace - Adopt virtualized TasksTableView from main, preserve selection highlight - Update handleImportRepo to use new multi-result openNew API
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/hooks/useTasksTable/useTasksTable.tsx`:
- Around line 223-234: The checkbox currently uses
row.getToggleSelectedHandler() (meant for native input events) causing a type
mismatch with Radix UI's onCheckedChange; replace the onCheckedChange handler to
call row.toggleSelected(...) directly (e.g., onCheckedChange={(checked) =>
row.toggleSelected(Boolean(checked))} or handle the "indeterminate" case
explicitly) while keeping the existing row.getIsGrouped() guard,
row.getIsSelected() checked state, onClick stopPropagation and aria props so
selection toggles correctly via row.toggleSelected.
🧹 Nitpick comments (4)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/hooks/useEscapeToNavigate/useEscapeToNavigate.ts (1)
4-9: Potential unnecessary effect re-runs ifoptions.searchis an unstable reference.
searchis compared by reference in the dependency array. If the caller passes an inline object (e.g.,{ search: { tab } }), the effect re-subscribes the keydown listener on every render. Consider documenting that callers should memoize thesearchobject, or stabilize it internally (e.g., viaJSON.stringifycomparison oruseMemo).Low risk if current callers always pass a stable reference.
apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/hooks/useTasksTable/useTasksTable.tsx (1)
45-59: Module-level Zustand store makes row selection a process-wide singleton.This is intentional for persisting selection across navigations, but be aware that if
useTasksTableis ever instantiated in multiple views simultaneously, they will share the same selection state. A brief inline comment would help future maintainers understand the design choice.apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/TasksView.tsx (2)
50-57:selectedTasksmemo may not update correctly when table row model changes.The
tableobject reference fromuseReactTableis referentially stable across renders, so the memo won't recompute when the underlying row model changes (e.g., after filtering). SincerowSelectionis in the deps and is the primary trigger for selection changes, this likely works in practice for selection toggles, but could go stale if rows are removed by a filter while selected.The
isFirstMountguard inuseTasksTablealready clears selection on filter changes, which mitigates this. Just noting for awareness.
68-74: Handler functions are not memoized.
handleTabChange,handleStartWorking,handleClearSelection, andhandleTaskClickare re-created every render and passed as props. Consider wrapping withuseCallbackifTasksTopBarorTasksTableVieware memoized (or might be in the future), to avoid unnecessary child re-renders.
…for Radix checkbox getToggleSelectedHandler returns an event-based handler meant for native inputs, causing a type mismatch with Radix UI's onCheckedChange callback.
…n with Claude" - Add xs and icon-xs sizes to Button component - Use proper Button size variants instead of manual overrides - Memoize backSearch to prevent useEscapeToNavigate effect churn - Rename "Start Working" to "Run with Claude" across all surfaces - Add dialog description explaining workspace creation flow
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
The dialog already shows per-task progress icons (spinner/check/X). Individual error toasts were noisy — keep only the summary toast.
Summary
--dangerously-skip-permissionsfor fully autonomous executionTest plan
Summary by CodeRabbit