Repository navigation
fix(desktop): repair terminal preset launches - #4926
Conversation
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR introduces "sequential" as a first-class terminal execution mode. The execution pipeline is refactored to preserve sequential mode through normalization, properly handle preset cwd via quoted cd commands, and support pane reuse when a suitable active terminal exists. New helpers centralize launch planning and pane naming logic, while the UI updated to reflect sequential mode labels. Integration through pane openers and hotkeys ensures presets control terminal-tab creation throughout the workspace. ChangesSequential Execution Mode Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
|
Ready to review this PR? Stage has broken it down into 7 individual chapters for you: Chapters generated by Stage for commit 3b39797 on May 27, 2026 9:36pm UTC. |
Greptile SummaryThis PR repairs several regressions in desktop terminal preset launching:
Confidence Score: 4/5The changes are well-scoped bug fixes with matching tests; the only notable gap is that All core sequential paths (active-terminal, new-tab-single, hotkey, applyOnNewTab) are covered by new tests. The
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/stores/tabs/preset-launch.ts | Adds active-terminal plan type and shouldApplyPresetPaneName helper; sequential mode early-returns before the shared shouldUseActiveTab path, which is correct. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2PresetExecution/useV2PresetExecution.ts | Largest change: adds sequential execution, worktree-aware cwd, writeInput, and newTabPresets; writeInput (useMutation result) in the useCallback dep array may cause unnecessary callback recreation on mutation state changes. |
| apps/desktop/src/renderer/stores/tabs/useTabsWithPresets.ts | Adds sequential path, buildFocusedTerminalCommand with cwd+cd, and preset pane labeling; openPresetInCurrentTerminal still writes raw commands without applying cwd, diverging from the new behavior in executePreset's active-terminal case. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspacePaneOpeners/useWorkspacePaneOpeners.ts | Adds addTerminalTab that runs applyOnNewTab presets on new tab creation; former blank-tab logic preserved as addBlankTerminalTab fallback. |
| packages/local-db/src/schema/zod.ts | Adds sequential to EXECUTION_MODES and normalizeExecutionMode; parallel still maps to split-pane, correct. |
| apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetRow/PresetRow.tsx | Adds sequential display label ("All in current tab" / "Split pane"); the single-command sequential branch showing "Split pane" is unreachable from current callers but semantically inconsistent. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Trigger preset] --> B{target?}
B -- new-tab forced\napplyOnNewTab / hotkey --> C[getPresetLaunchPlan\nmode=sequential\ntarget=new-tab]
B -- active-tab\ndefault for sequential --> D{hasActiveTerminal?}
D -- yes --> E[getPresetLaunchPlan\nreturns active-terminal]
D -- no --> F[getPresetLaunchPlan\nreturns new-tab-single]
C --> F
E --> G[writeInput.mutateAsync\ncd cwd && combined-cmd\nto active terminal]
G --> H{titleOverride set?}
H -- no --> I[setPaneTitleOverride\npreset.name]
H -- yes --> J[leave label unchanged]
F --> K[launcher.create\ncommand + cwd\nnew terminal]
K --> L[state.addTab\nnew tab with pane]
Comments Outside Diff (1)
-
apps/desktop/src/renderer/stores/tabs/useTabsWithPresets.ts, line 466-480 (link)cwd not applied in
openPresetInCurrentTerminalThis PR adds
buildFocusedTerminalCommand(prependingcd <cwd> &&) to theactive-terminalpath inexecutePreset, fixing cwd preservation for sequential auto-launch. However,openPresetInCurrentTerminal— the explicit "run in current terminal" user action — still passespreset.commandsdirectly towriteCommandsInPanewithout acdprefix. A user with a preset that has acwdset will find the directory silently ignored when they trigger this action, even after this PR.Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/renderer/stores/tabs/useTabsWithPresets.ts Line: 466-480 Comment: **cwd not applied in `openPresetInCurrentTerminal`** This PR adds `buildFocusedTerminalCommand` (prepending `cd <cwd> &&`) to the `active-terminal` path in `executePreset`, fixing cwd preservation for sequential auto-launch. However, `openPresetInCurrentTerminal` — the explicit "run in current terminal" user action — still passes `preset.commands` directly to `writeCommandsInPane` without a `cd` prefix. A user with a preset that has a `cwd` set will find the directory silently ignored when they trigger this action, even after this PR. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/desktop/src/renderer/stores/tabs/useTabsWithPresets.ts:466-480
**cwd not applied in `openPresetInCurrentTerminal`**
This PR adds `buildFocusedTerminalCommand` (prepending `cd <cwd> &&`) to the `active-terminal` path in `executePreset`, fixing cwd preservation for sequential auto-launch. However, `openPresetInCurrentTerminal` — the explicit "run in current terminal" user action — still passes `preset.commands` directly to `writeCommandsInPane` without a `cd` prefix. A user with a preset that has a `cwd` set will find the directory silently ignored when they trigger this action, even after this PR.
### Issue 2 of 2
apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetRow/PresetRow.tsx:111-114
**Misleading label for single-command sequential preset**
When `modeValue === "sequential"` and `commands.length === 1`, the label resolves to `"Split pane"`, which is semantically wrong — sequential mode writes to the active terminal and never splits panes. The branch is unreachable today because both `PresetsSection` and `V2PresetsSection` normalise single-command sequential to `"split-pane"` before passing `modeValue`, but a future caller would display the wrong label. Consider using `"Current tab"` here instead.
Reviews (1): Last reviewed commit: "fix(desktop): preserve preset terminal l..." | Re-trigger Greptile
| : modeValue === "sequential" | ||
| ? commands.length > 1 | ||
| ? "All in current tab" | ||
| : "Split pane" |
There was a problem hiding this comment.
Misleading label for single-command sequential preset
When modeValue === "sequential" and commands.length === 1, the label resolves to "Split pane", which is semantically wrong — sequential mode writes to the active terminal and never splits panes. The branch is unreachable today because both PresetsSection and V2PresetsSection normalise single-command sequential to "split-pane" before passing modeValue, but a future caller would display the wrong label. Consider using "Current tab" here instead.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetRow/PresetRow.tsx
Line: 111-114
Comment:
**Misleading label for single-command sequential preset**
When `modeValue === "sequential"` and `commands.length === 1`, the label resolves to `"Split pane"`, which is semantically wrong — sequential mode writes to the active terminal and never splits panes. The branch is unreachable today because both `PresetsSection` and `V2PresetsSection` normalise single-command sequential to `"split-pane"` before passing `modeValue`, but a future caller would display the wrong label. Consider using `"Current tab"` here instead.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
1 issue found across 19 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Description
Fixes regressions in desktop terminal presets that were discussed with Kiet before opening this PR.
Scope:
Directory/cwdwhen launching terminal commands.sequentialas a valid execution mode instead of normalizing it away.All in current tab.applyOnNewTabpresets when creating a new terminal tab, including the new-tab hotkey path.Related Issues
No GitHub issue. Discussed with Kiet in Slack; he confirmed this was a regression and said a PR would be welcome.
Type of Change
Bug fix.
Testing
bun test apps/desktop/src/renderer/stores/tabs/preset-launch.test.ts apps/desktop/src/lib/trpc/routers/settings/preset-execution-mode.test.ts apps/desktop/src/renderer/lib/terminal/launch-command.test.ts 'apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalSessionDropdown/TerminalSessionDropdown.utils.test.ts'bun run typecheck --filter=@superset/desktop --filter=@superset/api --filter=@superset/authbun run lintDemo
terminal-preset.mov
Reviewer Notes
Summary by cubic
Fixes terminal preset launches in desktop: preserves preset cwd, restores sequential execution, auto-applies presets on new tabs, and keeps preset/user titles visible when reusing terminals.
Written for commit 3b39797. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Improvements
Tests