feat(desktop): add auto-run toggle for agent task launches#2271
Merged
Conversation
Decouple noExecute from task file existence and add a localStorage-persisted toggle so users can choose between auto-running agent commands or staging them for review. Terminal agents stage the command without pressing Enter; chat agents pre-fill the input with @task:SLUG for manual send.
Contributor
📝 WalkthroughWalkthroughAdds file-backed task prompt persistence under workspace Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as RunInWorkspacePopover
participant Orchestrator as AgentOrchestrator
participant Backend as TerminalTRPC
participant Terminal as TerminalPane
participant Chat as ChatManager
User->>UI: Select tasks, agent, project, auto-run (or draft)
UI->>Orchestrator: buildLaunchRequest(task, agent, autoRun)
Orchestrator->>Backend: createWorkspace (if needed)
Backend-->>Orchestrator: workspaceId / existing workspace
alt chat launch
Orchestrator->>Chat: launchAgentSession(chatConfig{initialPrompt/draftInput, autoExecute, taskSlug})
Chat-->>Orchestrator: session created / result
else terminal launch
Orchestrator->>Backend: createOrAttach(terminalConfig{taskPromptContent, taskPromptFileName, autoExecute})
Backend-->>Orchestrator: wrote file / terminal attached
Orchestrator->>Terminal: launchCommandInPane(noExecute, command, task files)
Terminal-->>Orchestrator: command written / executed (or not executed if noExecute)
end
Orchestrator->>UI: update per-task status
UI->>User: display final statuses
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
4 issues found across 14 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/lib/terminal/launch-command.ts">
<violation number="1" location="apps/desktop/src/renderer/lib/terminal/launch-command.ts:57">
P1: When `noExecute` is true, the command is passed through without stripping any existing trailing `\n`. If a caller passes a command that already ends with `\n`, the terminal will still execute it, silently defeating the purpose of the flag. Strip trailing newlines in the `noExecute` path to make the behavior robust regardless of input.</violation>
</file>
<file name="packages/shared/src/agent-command.ts">
<violation number="1" location="packages/shared/src/agent-command.ts:99">
P1: File paths containing spaces will break these commands. Inside `$(cat ${filePath})`, the path is subject to shell word-splitting even though the outer result is double-quoted. Wrap the path in single quotes: `"$(cat '${filePath}')"` across all entries.</violation>
</file>
<file name="apps/desktop/src/lib/trpc/routers/terminal/terminal.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/terminal/terminal.ts:96">
P1: Path traversal: `taskPromptFileName` is not sanitised, so a value like `../../evil.sh` escapes the `.superset/` directory and writes to an arbitrary path. Apply the same `/`, `\`, `..` check that `SAFE_ID` already enforces, or use `path.basename()` to strip directory components.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsx:235">
P2: Log the caught error so failed workspace creations can be diagnosed. Without this, failures are visible in the UI but the root cause is lost.
(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| write, | ||
| noExecute, | ||
| }: WriteCommandInPaneOptions): Promise<void> { | ||
| const data = noExecute ? command : normalizeTerminalCommand(command); |
Contributor
There was a problem hiding this comment.
P1: When noExecute is true, the command is passed through without stripping any existing trailing \n. If a caller passes a command that already ends with \n, the terminal will still execute it, silently defeating the purpose of the flag. Strip trailing newlines in the noExecute path to make the behavior robust regardless of input.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/lib/terminal/launch-command.ts, line 57:
<comment>When `noExecute` is true, the command is passed through without stripping any existing trailing `\n`. If a caller passes a command that already ends with `\n`, the terminal will still execute it, silently defeating the purpose of the flag. Strip trailing newlines in the `noExecute` path to make the behavior robust regardless of input.</comment>
<file context>
@@ -46,10 +52,12 @@ export async function writeCommandInPane({
write,
+ noExecute,
}: WriteCommandInPaneOptions): Promise<void> {
+ const data = noExecute ? command : normalizeTerminalCommand(command);
await write({
paneId,
</file context>
Suggested change
| const data = noExecute ? command : normalizeTerminalCommand(command); | |
| const data = noExecute ? command.replace(/\n+$/, "") : normalizeTerminalCommand(command); |
- Security: validate taskPromptFileName has no path traversal (../ or separators) - Fix TOCTOU race in writeTaskFile using exclusive write flag (wx) - Add max attempt bound (100) to prevent infinite loop - Use unique HTML ids for Switch components to avoid DOM conflicts
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![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: 2
🧹 Nitpick comments (5)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsx (3)
235-242: Consider logging the caught error for debugging.The error is discarded silently. While the UI shows "failed" status, logging the error would aid debugging.
♻️ Suggested improvement
- } catch { + } catch (err) { + console.error(`Failed to create workspace for task ${task.slug}:`, err); setTaskStatuses((prev) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsx` around lines 235 - 242, The catch block in RunInWorkspacePopover (where setTaskStatuses is called and failCount is incremented) currently swallows errors; modify the catch to accept the error (e.g., catch (err)) and log it using the existing logger or console (include task.id and contextual message) before calling setTaskStatuses and incrementing failCount so failures are recorded for debugging.
98-98: Abort mechanism is scaffolded but not wired.
abortRefis checked in the loop (line 184) but never set totrue. If cancellation is intended, consider adding a cancel button that setsabortRef.current = true. If not needed now, consider removing the unused abort logic to reduce confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsx` at line 98, The abortRef used in RunInWorkspacePopover is never set so the cancellation check in the loop (where abortRef is inspected) is ineffective; either wire up a cancellation control (e.g., add a Cancel button or similar UI inside RunInWorkspacePopover that sets abortRef.current = true when invoked and ensure its handler is reachable where the popover runs) or remove the unused abortRef and related conditional checks to avoid dead code; update any related state/cleanup in the run handler (e.g., stop triggering further iterations or close the popover) when abortRef is set.
47-58: Minor:BatchStatusIconshould be in its own file.As per coding guidelines, maintain one component per file. Consider moving
BatchStatusIcontoBatchStatusIcon/BatchStatusIcon.tsxwith a barrel export.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsx` around lines 47 - 58, The BatchStatusIcon component is defined inline and should be moved to its own file to follow the one-component-per-file guideline; create a new component file named BatchStatusIcon/BatchStatusIcon.tsx that exports the BatchStatusIcon function/component (preserving the same props and return UI for statuses "pending"|"creating"|"done"|"failed"), update the original RunInWorkspacePopover file to import BatchStatusIcon from the new barrel export (e.g., BatchStatusIcon/index.ts exports BatchStatusIcon), and ensure any used icons (LuCircle, Spinner, HiCheck, HiXMark) and types (TaskStatus) imports are included in the new file and tests/build still pass.apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/PropertiesSidebar/components/OpenInWorkspace/OpenInWorkspace.tsx (2)
94-137: Consider extracting sharedbuildLaunchRequestlogic.The
buildLaunchRequestfunction andtaskInputconstruction are nearly identical to the implementation inRunInWorkspacePopover.tsx(lines 120-166). Consider extracting this to a shared utility to reduce duplication and ensure consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/PropertiesSidebar/components/OpenInWorkspace/OpenInWorkspace.tsx around lines 94 - 137, The buildLaunchRequest duplication should be refactored into a shared utility: extract the logic that constructs taskInput and returns AgentLaunchRequest into a reusable function (e.g., buildAgentLaunchRequest or createLaunchRequest) and replace the local buildLaunchRequest in this file and the near-duplicate implementation in RunInWorkspacePopover.tsx with calls to that util; the util should accept the task object, selectedAgent, autoRun (and workspaceId when needed) and reuse buildAgentTaskPrompt and buildAgentFileCommand to produce the same "chat" or "terminal" shapes so both components remain consistent.
120-130: Consider adding defensive validation oftask.slugbefore embedding in file path.While task slugs are currently constrained to safe characters (locally generated slugs use
[a-z0-9\-]only, and Linear identifiers followKEY-###format), the unquoted filePath in the command substitution$(cat ${filePath})is a theoretically vulnerable pattern. If a new slug source is added without equivalent sanitization in the future, this could enable injection. Consider explicitly validating or escaping the slug value here, or using quoted expansion$(cat "${filePath}")for additional safety.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/PropertiesSidebar/components/OpenInWorkspace/OpenInWorkspace.tsx around lines 120 - 130, The code builds a file path using task.slug (taskPromptFileName) and passes it into buildAgentFileCommand, which may allow injection if slug contains unsafe characters; update the OpenInWorkspace logic to defensively validate or sanitize task.slug before constructing taskPromptFileName (e.g., allow only [a-z0-9-] or replace/encode any disallowed chars) and ensure buildAgentFileCommand receives a filePath that will be safely quoted when expanded (i.e., use a quoted expansion pattern or otherwise escape/encode the path); locate the construction of taskPromptFileName and the call to buildAgentFileCommand in OpenInWorkspace.tsx and apply the validation/escaping there (also ensure any downstream command generation in buildAgentFileCommand treats filePath as a literal, not unquoted input).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/terminal/terminal.ts`:
- Around line 127-137: The call to writeTaskFile (in the handler that currently
checks workspacePath, input.taskPromptContent, and input.taskPromptFileName)
ignores its return value so the command is built using the original filename and
will fail on collisions; capture the returned finalFileName from writeTaskFile
and use that finalFileName when constructing or returning the command (or return
finalFileName to the client so it can update the command), ensuring the handler
passes workspacePath, input.taskPromptFileName, input.taskPromptContent to
writeTaskFile and then replaces usages of the original filename with the
returned finalFileName.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsx`:
- Around line 149-159: The code builds a file path using task.slug
(taskPromptFileName) and passes it to buildAgentFileCommand inside
RunInWorkspacePopover, which mirrors the shell-injection risk noted in
OpenInWorkspace.tsx; replace this ad-hoc interpolation with a shared, sanitized
path builder utility (e.g., sanitizeTaskFileName or buildSafeAgentFilePath) that
validates/normalizes task.slug to a safe subset of characters (or encodes it),
then use that utility in both RunInWorkspacePopover and OpenInWorkspace to
produce `.superset/{safeName}` before calling buildAgentFileCommand so no
unsanitized slug reaches the command builder.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/PropertiesSidebar/components/OpenInWorkspace/OpenInWorkspace.tsx:
- Around line 94-137: The buildLaunchRequest duplication should be refactored
into a shared utility: extract the logic that constructs taskInput and returns
AgentLaunchRequest into a reusable function (e.g., buildAgentLaunchRequest or
createLaunchRequest) and replace the local buildLaunchRequest in this file and
the near-duplicate implementation in RunInWorkspacePopover.tsx with calls to
that util; the util should accept the task object, selectedAgent, autoRun (and
workspaceId when needed) and reuse buildAgentTaskPrompt and
buildAgentFileCommand to produce the same "chat" or "terminal" shapes so both
components remain consistent.
- Around line 120-130: The code builds a file path using task.slug
(taskPromptFileName) and passes it into buildAgentFileCommand, which may allow
injection if slug contains unsafe characters; update the OpenInWorkspace logic
to defensively validate or sanitize task.slug before constructing
taskPromptFileName (e.g., allow only [a-z0-9-] or replace/encode any disallowed
chars) and ensure buildAgentFileCommand receives a filePath that will be safely
quoted when expanded (i.e., use a quoted expansion pattern or otherwise
escape/encode the path); locate the construction of taskPromptFileName and the
call to buildAgentFileCommand in OpenInWorkspace.tsx and apply the
validation/escaping there (also ensure any downstream command generation in
buildAgentFileCommand treats filePath as a literal, not unquoted input).
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsx`:
- Around line 235-242: The catch block in RunInWorkspacePopover (where
setTaskStatuses is called and failCount is incremented) currently swallows
errors; modify the catch to accept the error (e.g., catch (err)) and log it
using the existing logger or console (include task.id and contextual message)
before calling setTaskStatuses and incrementing failCount so failures are
recorded for debugging.
- Line 98: The abortRef used in RunInWorkspacePopover is never set so the
cancellation check in the loop (where abortRef is inspected) is ineffective;
either wire up a cancellation control (e.g., add a Cancel button or similar UI
inside RunInWorkspacePopover that sets abortRef.current = true when invoked and
ensure its handler is reachable where the popover runs) or remove the unused
abortRef and related conditional checks to avoid dead code; update any related
state/cleanup in the run handler (e.g., stop triggering further iterations or
close the popover) when abortRef is set.
- Around line 47-58: The BatchStatusIcon component is defined inline and should
be moved to its own file to follow the one-component-per-file guideline; create
a new component file named BatchStatusIcon/BatchStatusIcon.tsx that exports the
BatchStatusIcon function/component (preserving the same props and return UI for
statuses "pending"|"creating"|"done"|"failed"), update the original
RunInWorkspacePopover file to import BatchStatusIcon from the new barrel export
(e.g., BatchStatusIcon/index.ts exports BatchStatusIcon), and ensure any used
icons (LuCircle, Spinner, HiCheck, HiXMark) and types (TaskStatus) imports are
included in the new file and tests/build still pass.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b5679135-07a4-4a5d-9fc5-86b5444f867e
📒 Files selected for processing (14)
apps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/renderer/lib/agent-session-orchestrator/adapters/chat-adapter.tsapps/desktop/src/renderer/lib/agent-session-orchestrator/adapters/terminal-adapter.tsapps/desktop/src/renderer/lib/agent-session-orchestrator/types.tsapps/desktop/src/renderer/lib/terminal/launch-command.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/PropertiesSidebar/components/OpenInWorkspace/OpenInWorkspace.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/TasksView.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/TasksTopBar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsxapps/desktop/src/shared/tabs-types.tspackages/shared/src/agent-command.tspackages/shared/src/agent-launch.ts
Comment on lines
+127
to
+137
| if ( | ||
| workspacePath && | ||
| input.taskPromptContent && | ||
| input.taskPromptFileName | ||
| ) { | ||
| await writeTaskFile( | ||
| workspacePath, | ||
| input.taskPromptFileName, | ||
| input.taskPromptContent, | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how taskPromptFileName is used in command generation
rg -n -B3 -A10 'taskPromptFileName' --type=ts -g '!*.test.ts' | head -80Repository: superset-sh/superset
Length of output: 9500
🏁 Script executed:
rg -n 'function writeTaskFile|const writeTaskFile' --type=ts -A 20 | head -100Repository: superset-sh/superset
Length of output: 1878
🏁 Script executed:
sed -n '27,80p' apps/desktop/src/lib/trpc/routers/terminal/terminal.tsRepository: superset-sh/superset
Length of output: 1353
🏁 Script executed:
sed -n '147,165p' apps/desktop/src/lib/trpc/routers/terminal/terminal.tsRepository: superset-sh/superset
Length of output: 475
🏁 Script executed:
# Check the full context of how the terminal command is constructed and used
sed -n '110,170p' apps/desktop/src/lib/trpc/routers/terminal/terminal.tsRepository: superset-sh/superset
Length of output: 1512
🏁 Script executed:
sed -n '140,170p' apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsxRepository: superset-sh/superset
Length of output: 730
Capture and use the return value from writeTaskFile to handle filename collisions.
The writeTaskFile function implements collision handling that appends -{attempt} to filenames when they already exist (e.g., task-slug.md becomes task-slug-1.md), and it returns the actual filename written. However, the return value is discarded in the caller, while the command was pre-built using the original input filename. If a collision occurs, the command will reference a non-existent file, causing execution failure.
Store the returned finalFileName from writeTaskFile and either: (1) pass it back to the client so it can adjust the command, or (2) ensure collision handling at write time is extremely rare by using unique identifiers (e.g., timestamp, UUID) instead of simple collision counters.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/terminal/terminal.ts` around lines 127 -
137, The call to writeTaskFile (in the handler that currently checks
workspacePath, input.taskPromptContent, and input.taskPromptFileName) ignores
its return value so the command is built using the original filename and will
fail on collisions; capture the returned finalFileName from writeTaskFile and
use that finalFileName when constructing or returning the command (or return
finalFileName to the client so it can update the command), ensuring the handler
passes workspacePath, input.taskPromptFileName, input.taskPromptContent to
writeTaskFile and then replaces usages of the original filename with the
returned finalFileName.
Comment on lines
+149
to
+159
| const taskPromptFileName = `task-${task.slug}.md`; | ||
| return { | ||
| kind: "terminal", | ||
| workspaceId, | ||
| agentType: selectedAgent, | ||
| source: "open-in-workspace", | ||
| terminal: { | ||
| command: buildAgentFileCommand({ | ||
| filePath: `.superset/${taskPromptFileName}`, | ||
| agent: selectedAgent, | ||
| }), |
Contributor
There was a problem hiding this comment.
Same shell injection concern as OpenInWorkspace.tsx.
The task.slug is interpolated into the file path without sanitization, creating the same potential shell injection vulnerability discussed in OpenInWorkspace.tsx. Both components should use a shared, sanitized path builder.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsx`
around lines 149 - 159, The code builds a file path using task.slug
(taskPromptFileName) and passes it to buildAgentFileCommand inside
RunInWorkspacePopover, which mirrors the shell-injection risk noted in
OpenInWorkspace.tsx; replace this ad-hoc interpolation with a shared, sanitized
path builder utility (e.g., sanitizeTaskFileName or buildSafeAgentFilePath) that
validates/normalizes task.slug to a safe subset of characters (or encodes it),
then use that utility in both RunInWorkspacePopover and OpenInWorkspace to
produce `.superset/{safeName}` before calling buildAgentFileCommand so no
unsanitized slug reaches the command builder.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsx (2)
235-242: Capture error for better diagnostics.The catch block discards the error object. While the status is tracked, logging or using the error would aid debugging failed task launches.
🔧 Proposed fix
- } catch { + } catch (err) { + console.error(`Failed to create workspace for task ${task.slug}:`, err); setTaskStatuses((prev) => { const next = new Map(prev); next.set(task.id, "failed"); return next; }); failCount++; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsx` around lines 235 - 242, The catch block currently swallows the thrown error; update it to capture the error (e.g., catch (err) or catch (error)) and log or record it for diagnostics before updating state and incrementing failCount. Specifically, inside RunInWorkspacePopover's try/catch where setTaskStatuses and failCount are used, change the catch to accept an error variable and call a logger (console.error or the app logger) with contextual info (task.id and the error) and then perform the existing setTaskStatuses((prev) => { const next = new Map(prev); next.set(task.id, "failed"); return next; }); and failCount++.
98-98: Unused abort mechanism.
abortRefis initialized and checked in the loop (line 184), but there's no UI control to set it totrue. Users cannot cancel an in-progress batch run despite the popover preventing dismissal while running.Consider either:
- Adding a cancel button that sets
abortRef.current = true- Removing the unused abort logic if cancellation isn't planned
Also applies to: 168-184
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsx` at line 98, The abort mechanism (abortRef) in RunInWorkspacePopover is unused — either wire it up to the UI or remove it: if you want cancellation, add a visible Cancel button (or icon) in the popover that, when clicked, sets abortRef.current = true and ensure the running state and popover blocking logic allow early exit and cleanup so the loop that checks abortRef actually stops; otherwise remove abortRef and its conditional checks from the batch-run loop and simplify the popover dismissal/disabled logic so there’s no dead cancellation code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsx`:
- Around line 235-242: The catch block currently swallows the thrown error;
update it to capture the error (e.g., catch (err) or catch (error)) and log or
record it for diagnostics before updating state and incrementing failCount.
Specifically, inside RunInWorkspacePopover's try/catch where setTaskStatuses and
failCount are used, change the catch to accept an error variable and call a
logger (console.error or the app logger) with contextual info (task.id and the
error) and then perform the existing setTaskStatuses((prev) => { const next =
new Map(prev); next.set(task.id, "failed"); return next; }); and failCount++.
- Line 98: The abort mechanism (abortRef) in RunInWorkspacePopover is unused —
either wire it up to the UI or remove it: if you want cancellation, add a
visible Cancel button (or icon) in the popover that, when clicked, sets
abortRef.current = true and ensure the running state and popover blocking logic
allow early exit and cleanup so the loop that checks abortRef actually stops;
otherwise remove abortRef and its conditional checks from the batch-run loop and
simplify the popover dismissal/disabled logic so there’s no dead cancellation
code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4ee3fc2f-7a0b-4fa3-a773-6412f09e0898
📒 Files selected for processing (2)
apps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/lib/trpc/routers/terminal/terminal.ts
- Quote filePath in $(cat ...) to prevent word-splitting on spaces - Log caught errors in RunInWorkspacePopover instead of silently swallowing - Simplify writeTaskFile to overwrite instead of collision-renaming, fixing filename mismatch when return value was ignored
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/shared/src/agent-command.ts">
<violation number="1" location="packages/shared/src/agent-command.ts:100">
P2: Single-quoting `filePath` inside `$(cat ...)` doesn't handle paths containing single quotes, which would break the shell quoting and could allow unintended command execution. Escape single quotes in `filePath` before interpolation — the standard shell idiom is `'\''` to break out and re-enter single quoting.
The safest place is in `buildAgentFileCommand` so every agent entry benefits:</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsx (1)
98-98:abortRefis declared but never set totrue.The ref is checked in
handleRun(line 184) but nothing ever setsabortRef.current = true. Consider either:
- Adding a cancel button that sets
abortRef.current = true- Removing the unused abort logic if cancellation isn't needed
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsx` at line 98, abortRef is created with useRef(false) but never updated, so the cancellation check in handleRun never triggers; either wire a cancel action to set abortRef.current = true (e.g., add a Cancel/Close button in the RunInWorkspacePopover that calls a handler which sets abortRef.current = true and ensures any running loop in handleRun checks it) or remove the abortRef and related checks in handleRun if cancellation is not supported; search for abortRef and handleRun in RunInWorkspacePopover to apply the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/shared/src/agent-command.ts`:
- Around line 98-107: AGENT_FILE_COMMANDS builds shell commands by interpolating
filePath into "$(cat '${filePath}')" which allows a single quote in filePath to
break out and enable command injection; fix by centralizing a sanitizer (e.g.,
escapeSingleQuote or shellEscapePath) and apply it to every mapping in
AGENT_FILE_COMMANDS so that every filePath has single quotes replaced with the
safe sequence '\'' (or otherwise safely quoted), or validate/whitelist allowed
characters before interpolation; update each lambda (claude, codex, gemini,
opencode, copilot, "cursor-agent") to call that sanitizer instead of directly
interpolating filePath.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsx`:
- Line 98: abortRef is created with useRef(false) but never updated, so the
cancellation check in handleRun never triggers; either wire a cancel action to
set abortRef.current = true (e.g., add a Cancel/Close button in the
RunInWorkspacePopover that calls a handler which sets abortRef.current = true
and ensures any running loop in handleRun checks it) or remove the abortRef and
related checks in handleRun if cancellation is not supported; search for
abortRef and handleRun in RunInWorkspacePopover to apply the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9ba4850d-d42e-4c24-85c7-0f8ebf182255
📒 Files selected for processing (3)
apps/desktop/src/lib/trpc/routers/terminal/terminal.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopover/RunInWorkspacePopover.tsxpackages/shared/src/agent-command.ts
Escape single quotes in filePath before interpolation into $(cat '...') to prevent shell quoting breakage on paths containing single quotes.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/shared/src/agent-command.ts (1)
98-107: Derive file-mode commands from the same agent config to avoid drift.This adds a third place to maintain per-agent flags, and
codexis already out of sync across the maps in this file. A later flag change can silently make preset, heredoc, and file launches behave differently. Consider a shared per-agent descriptor and generate both prompt/file builders from it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared/src/agent-command.ts` around lines 98 - 107, AGENT_FILE_COMMANDS is hardcoded separately and can drift from other per-agent command maps (e.g., the codex flags); refactor by introducing a single per-agent descriptor (e.g., an AGENT_DESCRIPTOR record mapping AgentType -> descriptor object containing base command and common flags) and then derive AGENT_FILE_COMMANDS and the prompt/heredoc builders from that descriptor (create small helpers like generateFileCommand(descriptor, filePath) and generatePromptCommand(descriptor, prompt) to build the final strings), ensuring the codex descriptor contains the single source of truth for its flags so all generated commands stay in sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/shared/src/agent-command.ts`:
- Around line 98-107: AGENT_FILE_COMMANDS is hardcoded separately and can drift
from other per-agent command maps (e.g., the codex flags); refactor by
introducing a single per-agent descriptor (e.g., an AGENT_DESCRIPTOR record
mapping AgentType -> descriptor object containing base command and common flags)
and then derive AGENT_FILE_COMMANDS and the prompt/heredoc builders from that
descriptor (create small helpers like generateFileCommand(descriptor, filePath)
and generatePromptCommand(descriptor, prompt) to build the final strings),
ensuring the codex descriptor contains the single source of truth for its flags
so all generated commands stay in sync.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eed3a55f-7ce5-4d29-abc0-4b57867008a4
📒 Files selected for processing (1)
packages/shared/src/agent-command.ts
This was referenced Mar 10, 2026
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
@task:SLUGso the user can review and send manuallynoExecutefrom task file existence — it's now driven by theautoExecuteflag on the launch config.mdfiles to the workspace via the terminal router (moved from client-side)agentAutoRunlocalStorage key shared between both components for consistent preferenceTest plan
@task:SLUGpre-filled in chat input, user sends manuallySummary by cubic
Adds an Auto-run command toggle for agent task launches in desktop. When off, terminal commands are staged and chat input is drafted for review; adds a batch Run in Workspace popover and switches terminal agents to file-based commands that write prompt files to
.superset.New Features
localStorage(agentAutoRun), shared by single-task OpenInWorkspace and the new batch RunInWorkspace popover..mdto.supersetvia the terminal router and run a quoted file-based command; when off, the command is staged without pressing Enter.@task:SLUGwhen off; when on, the initial prompt auto-sends.Refactors
noExecuteis now driven by anautoExecuteflag, decoupled from task file existence.autoExecute,taskPromptContent,taskPromptFileName, and chattaskSlug; addedbuildAgentFileCommandand chatdraftInput.taskPromptFileName(no path traversal), quote file paths in$(cat ...), escape single quotes in paths to avoid shell breakage, log errors in the batch runner, and simplify file writes to overwrite.Written for commit d1a5459. Summary will update on new commits.
Summary by CodeRabbit
New Features
Batch Actions
UI