Repository navigation
feat(desktop): add Open in workspace button to task detail - #1473
Conversation
📝 WalkthroughWalkthroughThis PR introduces workspace integration for tasks. It adds a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/`$taskId/components/WorkOnTaskButton/WorkOnTaskButton.tsx:
- Around line 89-93: In the catch block inside the WorkOnTaskButton component
(the error handler that currently calls toast.error), add a console.error call
before showing the toast to log the full error and contextual info (e.g.,
"WorkOnTaskButton: failed to create workspace" plus the caught err and err.stack
when available) so the error is not swallowed; keep the toast behavior unchanged
after logging.
🧹 Nitpick comments (2)
packages/shared/src/claude-command.ts (1)
1-9: ExportTaskInputso consumers can reference the type.The
buildClaudeCommandfunction acceptstask: TaskInput, butTaskInputis not exported. Consumers (e.g.,WorkOnTaskButton, MCP tools) cannot type-check their task objects against this interface without relying on structural compatibility alone.Proposed fix
-interface TaskInput { +export interface TaskInput {apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/WorkOnTaskButton/WorkOnTaskButton.tsx (1)
158-165: Consider showing a loading indicator while workspace creation is pending.The button is correctly disabled during
createWorkspace.isPending, but users have no visual feedback that an action is in progress. A spinner or similar indicator would improve the experience, especially if workspace creation takes a noticeable amount of time.Suggested change
<Button size="icon" className="h-8 w-8 shrink-0" disabled={!effectiveProjectId || createWorkspace.isPending} onClick={handleOpen} > - <HiArrowRight className="w-3.5 h-3.5" /> + {createWorkspace.isPending ? ( + <span className="animate-spin w-3.5 h-3.5 border-2 border-current border-t-transparent rounded-full" /> + ) : ( + <HiArrowRight className="w-3.5 h-3.5" /> + )} </Button>
| } catch (err) { | ||
| toast.error( | ||
| err instanceof Error ? err.message : "Failed to create workspace", | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add console.error logging before the toast.
The catch block displays a user-facing toast but doesn't log the error. This makes debugging harder in production. Per the project guidelines: "Never swallow errors silently; at minimum log errors with context before rethrowing or handling them explicitly."
Proposed fix
} catch (err) {
+ console.error("[WorkOnTaskButton/handleSelectProject]", err);
toast.error(
err instanceof Error ? err.message : "Failed to create workspace",
);
}🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/WorkOnTaskButton/WorkOnTaskButton.tsx
around lines 89 - 93, In the catch block inside the WorkOnTaskButton component
(the error handler that currently calls toast.error), add a console.error call
before showing the toast to log the full error and contextual info (e.g.,
"WorkOnTaskButton: failed to create workspace" plus the caught err and err.stack
when available) so the error is not swallowed; keep the toast behavior unchanged
after logging.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Adds a project selector + go button in the task detail properties sidebar that creates a workspace and launches an autonomous Claude session with full task context. Consolidates the Claude command prompt template into @superset/shared so MCP and desktop share a single source of truth.
cdb1a4f to
f8d143e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/shared/src/claude-command.ts`:
- Around line 1-9: The TaskInput interface is not exported, so callers cannot
import the primary input type for buildClaudeCommand; update the module to
export TaskInput (e.g., change the declaration to export interface TaskInput or
export type TaskInput = { ... }) so consumers can import TaskInput alongside
buildClaudeCommand and get explicit type safety when constructing task objects.
🧹 Nitpick comments (4)
packages/shared/src/claude-command.ts (1)
48-48: Minor: consider validating thatrandomIdproduces a safe shell delimiter.
replaceAll("-", "")on a UUID is fine for the expected input, but if a non-UUID string were passed (e.g., containing newlines or quotes), it could break the heredoc. Since callers currently usecrypto.randomUUID(), this is low risk — just worth a defensive note or a doc comment on the expected format.apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/PropertiesSidebar/components/OpenInWorkspace/OpenInWorkspace.tsx (3)
37-42: TheuseEffectis redundant given theeffectiveProjectIdfallback on line 32.Line 32 already computes
effectiveProjectIdas the fallback to the first recent project. TheuseEffecton lines 38–42 then setsselectedProjectIdto the same value, causing a needless re-render. SinceeffectiveProjectIdalready handles the "no selection yet" case, the effect adds no user-visible value — it only triggers an extra render cycle.Consider removing it unless there's a downstream need for
selectedProjectIdto be explicitly set in state (e.g., for controlled form submission).
44-47:handleOpentriggers workspace creation immediately — is this intentional UX?Clicking the arrow button calls
handleSelectProject, which both selects and creates/opens a workspace. But clicking a project in the dropdown only updates the selection (line 140). The asymmetry seems intentional (dropdown = pick, arrow = go), but the function namehandleSelectProjectis misleading since it actually creates a workspace. A rename to something likehandleCreateOrOpenWorkspacewould reduce confusion.
15-17: Use path aliases instead of deep relative imports.Lines 11–14 use the
renderer/alias, but these three imports use relative paths with deep../../../../traversals. This is inconsistent and fragile.Proposed fix
-import type { TaskWithStatus } from "../../../../../components/TasksView/hooks/useTasksTable"; -import { buildClaudeCommand } from "../../../../utils/buildClaudeCommand"; -import { deriveBranchName } from "../../../../utils/deriveBranchName"; +import type { TaskWithStatus } from "renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/hooks/useTasksTable"; +import { buildClaudeCommand } from "renderer/routes/_authenticated/_dashboard/tasks/$taskId/utils/buildClaudeCommand"; +import { deriveBranchName } from "renderer/routes/_authenticated/_dashboard/tasks/$taskId/utils/deriveBranchName";As per coding guidelines: "Use alias as defined in
tsconfig.jsonwhen possible."
| interface TaskInput { | ||
| id: string; | ||
| slug: string; | ||
| title: string; | ||
| description: string | null; | ||
| priority: string; | ||
| statusName: string | null; | ||
| labels: string[] | null; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/shared/src/claude-command.tsRepository: superset-sh/superset
Length of output: 1997
🏁 Script executed:
rg "import.*TaskInput|from.*claude-command" --type=ts --type=tsxRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
rg "TaskInput" --type=ts --type=tsx -B 2 -A 2Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
rg "TaskInput" --type=ts -B 2 -A 2Repository: superset-sh/superset
Length of output: 1317
🏁 Script executed:
fd "\.ts$|\.tsx$" packages/mcp packages/desktop-app 2>/dev/null | head -20Repository: superset-sh/superset
Length of output: 1171
🏁 Script executed:
rg "import.*from.*claude-command" --type=tsRepository: superset-sh/superset
Length of output: 209
🏁 Script executed:
cat -n packages/mcp/src/tools/devices/start-claude-session/start-claude-session.tsRepository: superset-sh/superset
Length of output: 5510
🏁 Script executed:
cat -n packages/mcp/src/tools/tasks/create-task/create-task.ts | head -40Repository: superset-sh/superset
Length of output: 1699
Export TaskInput from the claude-command module.
buildClaudeCommand is exported, but TaskInput — its primary input type — is not. Consumers in packages/mcp and elsewhere must construct the task object without importing the type, relying on structural typing instead of explicit type safety.
Proposed fix
-interface TaskInput {
+export interface TaskInput {
id: string;
slug: string;
title: string;
description: string | null;
priority: string;
statusName: string | null;
labels: string[] | null;
}🤖 Prompt for AI Agents
In `@packages/shared/src/claude-command.ts` around lines 1 - 9, The TaskInput
interface is not exported, so callers cannot import the primary input type for
buildClaudeCommand; update the module to export TaskInput (e.g., change the
declaration to export interface TaskInput or export type TaskInput = { ... }) so
consumers can import TaskInput alongside buildClaudeCommand and get explicit
type safety when constructing task objects.
Summary
@superset/shared/claude-commandso both the MCP server and desktop renderer share a single source of truth instead of duplicated copiesTest plan
bun run typecheckpasses for both@superset/mcpand@superset/desktopSummary by CodeRabbit