-
Notifications
You must be signed in to change notification settings - Fork 478
feat: add external terminal support with cross-platform detection #565
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add external terminal support with cross-platform detection #565
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds an "Open in Terminal" feature: backend routes and platform launchers for external terminals, UI hooks/icons/settings, state/store plumbing for terminal modes and branch context, and client/server/electron integrations to open worktrees in integrated or external terminals. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Dropdown as Worktree Dropdown
participant Handler as UI Handler
participant Electron as Electron API
participant Server as Server Routes
participant Platform as Platform Services
participant System as OS
User->>Dropdown: Select "Open in External Terminal"
Dropdown->>Handler: handleOpenInExternalTerminal(path, terminalId?)
Handler->>Electron: worktree.openInExternalTerminal(path, terminalId?)
Electron->>Server: POST /api/worktree/open-in-external-terminal
Server->>Platform: openInExternalTerminal(path, terminalId?)
Platform->>Platform: detect/find terminal, build command
Platform->>System: spawn detached terminal process in cwd
System-->>Platform: process started
Platform-->>Server: { terminalName }
Server-->>Electron: success + terminalName
Electron->>Handler: return result
Handler->>User: show confirmation/toast
sequenceDiagram
actor User
participant Dropdown as Worktree Dropdown
participant Handler as UI Handler
participant Nav as Router
participant TerminalView as Terminal View
participant Store as App Store
User->>Dropdown: Select "Open in Integrated Terminal" (mode)
Dropdown->>Handler: handleOpenInIntegratedTerminal(path, mode)
Handler->>Nav: navigate to /terminal?cwd=...&mode=...&nonce=...
Nav->>TerminalView: mount with search params
TerminalView->>Store: createTerminal({ cwd, mode, branch, nonce })
Store->>Store: add terminal session (attach branchName)
TerminalView->>Nav: clear URL params (prevent re-create)
TerminalView->>User: focus terminal UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Summary of ChangesHello @stefandevo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience by integrating robust external terminal support, allowing users to seamlessly open worktree directories in their preferred terminal applications. It provides cross-platform detection for a wide array of terminals and introduces new settings for customization, offering greater flexibility and a more tailored workflow. Additionally, the integrated terminal now provides better contextual information by displaying the current Git branch. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a significant new feature: support for external terminals. The implementation is comprehensive, covering cross-platform detection for over 20 terminals and integrating cleanly with the existing UI through a new split-button pattern and settings page. The backend API and frontend hooks are well-structured. I've provided a few suggestions to improve code consistency, maintainability, and test robustness.
apps/ui/src/components/layout/project-switcher/components/project-switcher-item.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/ui/src/store/app-store.ts (1)
2674-2773: PreservebranchNamewhen adding to an empty tab layout.In
addTerminalToLayout, the!activeTab.layoutbranch omitsbranchName, creating an inconsistency with the first tab initialization (line 2693) and the similaraddTerminalToTabfunction (line 3254), both of which preserve it for display context.🔧 Suggested fix
- if (!activeTab.layout) { - newLayout = { type: 'terminal', sessionId, size: 100 }; + if (!activeTab.layout) { + newLayout = { type: 'terminal', sessionId, size: 100, branchName }; } else if (targetSessionId) {apps/ui/src/components/views/terminal-view.tsx (1)
1320-1349: Preserve branch label when a terminal is maximized.
branchNameis passed to regular panels but not to the maximized panel, causing the header to lose branch context. Create a utility function similar tofindTerminalFontSizeto look up the branch name bysessionIdin the active tab's layout tree, then pass it to the maximizedTerminalPanel.
🤖 Fix all issues with AI agents
In `@apps/server/src/routes/worktree/routes/open-in-terminal.ts`:
- Around line 31-49: The request currently asserts worktreePath but doesn't
check its runtime type, so non-string truthy values will reach
isAbsolute(worktreePath) and crash; update the handler in open-in-terminal.ts to
validate typeof worktreePath === 'string' before calling isAbsolute (and before
the existing falsy check), and if it's not a string respond with
res.status(400).json({ success: false, error: 'worktreePath must be a string'
}); keep the existing absolute-path check using isAbsolute(worktreePath) after
this string validation.
In
`@apps/ui/src/components/layout/project-switcher/components/project-switcher-item.tsx`:
- Around line 40-47: Replace the unstable name-only test id by incorporating the
stable project identifier: keep the sanitized slug (sanitizedName) for
readability but change the data-testid located on the button (and generated from
sanitizedName) to include project.id (for example
`project-switcher-project-${project.id}-${sanitizedName}`) so selectors are
stable and unique; update the code that computes sanitizedName
(project.name.toLowerCase().replace(/\s+/g, '-')) and the data-testid string in
the ProjectSwitcherItem component to use project.id alongside the slug.
In
`@apps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-terminals.ts`:
- Around line 81-98: In useEffectiveDefaultTerminal, treat both null and
undefined defaultTerminalId as the "integrated" preference by changing the
strict null check to a loose null check (use defaultTerminalId == null) so
undefined from settings sync/deserialization is handled the same as null; keep
the subsequent logic that looks up a saved preference (terminals.find by id) and
falls back to terminals[0] or null if not found, and reference the useAppStore
selector defaultTerminalId and the useEffectiveDefaultTerminal function when
making the change.
In `@libs/platform/src/editor.ts`:
- Around line 415-434: The xterm entry in the terminals array builds its args
unsafely; update the xterm args in the terminals array so the targetPath is
passed through the existing escapeShellArg(targetPath) utility and invoke a
shell with -lc so the cd and shell exec run in a proper shell context (e.g., use
sh -lc 'cd <escapedPath> && exec "$SHELL"'). Modify the xterm terminal object
(the entry with command 'xterm') to use escapeShellArg(targetPath) and run the
command via a shell with -lc instead of the current `cd "${targetPath}" &&
$SHELL` string.
- Around line 376-412: The Windows branch currently tries to spawn 'wt' with
shell:true and relies on child error/timeout, which won't catch a missing 'wt';
instead, use the existing commandExists('wt') helper to check availability
before attempting to spawn. Update the isWindows branch in the function (where
spawn is called for 'wt' and for 'cmd') to: if commandExists('wt') is true, then
spawn 'wt' as before and resolve with terminalName 'Windows Terminal'; otherwise
skip to the cmd fallback code that spawns 'cmd' and resolves with terminalName
'Command Prompt'. Ensure you reference and call commandExists('wt') (same helper
used for Linux) to determine whether to attempt the Windows Terminal spawn.
In `@libs/platform/src/terminal.ts`:
- Around line 557-560: The xterm branch currently injects targetPath directly
into the -e command; update the case 'xterm' to wrap the command in sh -c and
pass an escaped targetPath using the existing escapeShellArg helper to prevent
shell injection—i.e., construct the args for spawnDetached as ['-e', 'sh', '-c',
`cd ${escapeShellArg(targetPath)} && $SHELL`] (use the escapeShellArg function
and keep calling spawnDetached as before).
🧹 Nitpick comments (4)
libs/types/src/settings.ts (1)
478-481: Consider consolidating terminal settings.The
openTerminalModesetting is defined here under "Terminal Configuration", butdefaultTerminalId(lines 610-612) is defined separately under another "Terminal Configuration" comment. Consider moving these related settings adjacent to each other for better maintainability.apps/ui/src/hooks/use-settings-sync.ts (1)
531-542: Terminal state refresh logic is correct but could be more robust.The conditional spread for terminal settings works correctly. However, the condition uses
||which means if only one setting exists, both will be spread. This is intentional and works because of the inner conditionals, but consider a minor improvement for clarity:♻️ Optional: Slightly clearer structure
// Terminal settings (nested in terminalState) - ...((serverSettings.terminalFontFamily || serverSettings.openTerminalMode) && { - terminalState: { - ...currentAppState.terminalState, - ...(serverSettings.terminalFontFamily && { - fontFamily: serverSettings.terminalFontFamily, - }), - ...(serverSettings.openTerminalMode && { - openTerminalMode: serverSettings.openTerminalMode, - }), - }, - }), + ...(serverSettings.terminalFontFamily || serverSettings.openTerminalMode + ? { + terminalState: { + ...currentAppState.terminalState, + ...(serverSettings.terminalFontFamily && { + fontFamily: serverSettings.terminalFontFamily, + }), + ...(serverSettings.openTerminalMode && { + openTerminalMode: serverSettings.openTerminalMode, + }), + }, + } + : {}),apps/ui/src/components/views/board-view/worktree-panel/hooks/use-worktree-actions.ts (1)
175-177: Missing user-facing error feedback in catch block.The catch block logs the error but doesn't notify the user, unlike similar handlers (e.g.,
handleOpenInEditorwhich also doesn't show a toast, so this is consistent). However, for a better UX, consider adding a toast here since external terminal launching can fail for various reasons.♻️ Suggested improvement
} catch (error) { logger.error('Open in external terminal failed:', error); + toast.error('Failed to open in external terminal'); }apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx (1)
384-384: Consider adding a label for external terminals section.When there are external terminals, it might be helpful to add a label for clarity, especially if many terminals are detected.
♻️ Optional enhancement
{/* External terminals */} - {terminals.length > 0 && <DropdownMenuSeparator />} + {terminals.length > 0 && ( + <> + <DropdownMenuSeparator /> + <DropdownMenuLabel className="text-xs text-muted-foreground"> + External Terminals + </DropdownMenuLabel> + </> + )} {terminals.map((terminal) => {
apps/ui/src/components/layout/project-switcher/components/project-switcher-item.tsx
Outdated
Show resolved
Hide resolved
apps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-terminals.ts
Show resolved
Hide resolved
Add utility function to open a terminal in a specified directory: - macOS: Uses Terminal.app via AppleScript - Windows: Tries Windows Terminal, falls back to cmd - Linux: Tries common terminal emulators (gnome-terminal, konsole, xfce4-terminal, xterm, x-terminal-emulator) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add POST /open-in-terminal endpoint to open a system terminal in the worktree directory using the cross-platform openInTerminal utility. The endpoint validates that worktreePath is provided and is an absolute path for security. Extracted from PR AutoMaker-Org#558.
Add "Open in Terminal" option to the worktree actions dropdown menu. This opens the system terminal in the worktree directory. Changes: - Add openInTerminal method to http-api-client - Add Terminal icon and menu item to worktree-actions-dropdown - Add onOpenInTerminal prop to WorktreeTab component - Add handleOpenInTerminal handler to use-worktree-actions hook - Wire up handler in worktree-panel for both mobile and desktop views Extracted from PR AutoMaker-Org#558.
Instead of opening the system terminal, the "Open in Terminal" action now opens Automaker's built-in terminal with the worktree directory: - Add pendingTerminalCwd state to app store - Update use-worktree-actions to set pending cwd and navigate to /terminal - Add effect in terminal-view to create session with pending cwd This matches the original PR AutoMaker-Org#558 behavior.
Add a setting to choose how "Open in Terminal" behaves:
- New Tab: Creates a new tab named after the branch (default)
- Split: Adds to current tab as a split view
Changes:
- Add openTerminalMode setting to terminal state ('newTab' | 'split')
- Update terminal-view to respect the setting
- Add UI in Terminal Settings to toggle the behavior
- Rename pendingTerminalCwd to pendingTerminal with branch name
The new tab mode names tabs after the branch for easy identification.
The split mode is useful for comparing terminals side by side.
- Move branch name display from tab name to terminal header - Show full branch name (no truncation) with GitBranch icon - Display branch name for both 'new tab' and 'split' modes - Persist openTerminalMode setting to server and include in import/export - Update settings dropdown to simplified "New Tab" label
Add support for opening worktree directories in external terminals (iTerm2, Warp, Ghostty, System Terminal, etc.) while retaining the integrated terminal as the default option. Changes: - Add terminal detection for macOS, Windows, and Linux - Add "Open in Terminal" split-button in worktree dropdown - Add external terminal selection in Settings > Terminal - Add default open mode setting (new tab vs split) - Display branch name in terminal panel header - Support 20+ terminals across platforms Part of AutoMaker-Org#558, Closes AutoMaker-Org#550
- Add nonce parameter to terminal navigation to allow reopening same worktree multiple times - Fix shell path escaping in editor.ts using single-quote wrapper - Add validatePathParams middleware to open-in-external-terminal route - Remove redundant validation block from createOpenInExternalTerminalHandler - Remove unused pendingTerminal state and setPendingTerminal action - Remove unused getTerminalInfo function from editor.ts
- Add runtime type check for worktreePath in open-in-terminal handler - Fix Windows Terminal detection using commandExists before spawn - Fix xterm shell injection by using sh -c with escapeShellArg - Use loose equality for null/undefined in useEffectiveDefaultTerminal - Consolidate duplicate imports from open-in-terminal.js
0b89661 to
c42b786
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx (1)
130-346: Default open mode should fall back to “New Tab” when setting is missing.If
openTerminalModeisundefined(e.g., older persisted settings), the current ternary defaults tosplit. If the intended default is “New Tab,” this flips behavior unexpectedly. A defensive mapping avoids that.🛠️ Proposed fix
- const mode = openTerminalMode === 'newTab' ? 'tab' : 'split'; + const mode = openTerminalMode === 'split' ? 'split' : 'tab';
🤖 Fix all issues with AI agents
In `@apps/server/src/routes/worktree/routes/open-in-terminal.ts`:
- Around line 144-165: The createOpenInExternalTerminalHandler currently trusts
the middleware but should explicitly validate worktreePath like the other
handler: inside the returned async handler (where worktreePath and terminalId
are destructured and before calling openInExternalTerminal) add checks that
worktreePath exists and is a string and that path.isAbsolute(worktreePath) is
true; if either check fails, return res.status(400).json({ success: false,
error: 'worktreePath required and must be a string' }) or res.status(400).json({
success: false, error: 'worktreePath must be an absolute path' }) respectively;
ensure isAbsolute is imported from 'path' and keep the call to
openInExternalTerminal(worktreePath, terminalId) only after these validations.
In `@apps/ui/src/components/views/settings-view/terminal/terminal-section.tsx`:
- Around line 52-54: The refresh icon button in TerminalSection is missing an
accessible label and the empty-state message is shown prematurely while loading;
update the icon-only refresh button (the element that calls refresh from
useAvailableTerminals) to include an accessible label (e.g., aria-label="Refresh
terminals" or a visually-hidden label) and adjust the empty-state rendering
logic (the code checking terminals length) to only show the "no terminals"
message when isRefreshing is false (i.e., show a loader or nothing while
isRefreshing is true), and apply the same fixes to the other occurrences noted
around the component (lines referencing terminals, isRefreshing, refresh).
In `@apps/ui/src/components/views/terminal-view.tsx`:
- Around line 555-629: The effect sets initialCwdHandledRef.current before the
async createTerminalWithCwd call and never clears it on failure, preventing
retries; update createTerminalWithCwd (or its try/catch/finally) so that on any
failure path (data.success false or caught exception) you reset
initialCwdHandledRef.current back to undefined (or the previous value) so the
same initialCwd/initialMode/nonce key can be retried; reference
initialCwdHandledRef, createTerminalWithCwd, and the error branches where
logger.error/toast.error are called to locate where to clear the flag.
In `@apps/ui/src/routes/terminal.tsx`:
- Around line 3-19: Update the search schema to coerce URL string params to
numbers and ensure the runtime has zod available: change the nonce field in
terminalSearchSchema from z.number().optional() to z.coerce.number().optional()
(so Route.useSearch() yields a numeric nonce) and add "zod" to the apps/ui
package.json dependencies so the import { z } from 'zod' in this file (and
symbols terminalSearchSchema, Route, RouteComponent, TerminalView) resolves at
runtime.
In `@apps/ui/src/store/app-store.ts`:
- Around line 1224-1228: When adding a split to an active tab that has no
layout, the created newLayout omits branchName causing the terminal header to
not show the branch; ensure branchName is preserved by populating
newLayout.branchName (fallback to the current active tab's branchName or the
session's branchName) before dispatching the add/update call. Update the places
that call the function with signature including sessionId, direction,
targetSessionId, branchName so they pass the branchName through (or set it on
newLayout) — apply the same change in the other similar block referenced (the
second occurrence around the other range) to keep behavior consistent.
In `@apps/ui/src/types/electron.d.ts`:
- Around line 963-972: Update the getDefaultTerminal return type so the result
may be null when no external terminal is detected: change the result type in
getDefaultTerminal's Promise return from an optional object to either that
object or null (e.g., result?: { terminalId: string; terminalName: string;
terminalCommand: string; } | null), keeping the success and error fields intact
so consumers correctly handle the server returning result: null; locate the
declaration by the getDefaultTerminal symbol in the electron.d.ts types.
🧹 Nitpick comments (1)
libs/platform/src/terminal.ts (1)
340-357: Return cached terminal lists defensively to avoid accidental mutation.Callers can mutate the returned array and corrupt the cache. Returning a shallow copy prevents this.
♻️ Proposed refactor
if (isCacheValid() && cachedTerminals) { - return cachedTerminals; + return [...cachedTerminals]; } @@ - cachedTerminals = terminals; + cachedTerminals = [...terminals]; @@ - return terminals; + return [...terminals];
apps/ui/src/components/views/settings-view/terminal/terminal-section.tsx
Show resolved
Hide resolved
Replace response.body() with response.json() in open-existing-project.spec.ts to fix the "Response has been disposed" error. This matches the pattern used in other test files.
…n E2E test" This reverts commit 36bdf8c.
- Add explicit validation for worktreePath in createOpenInExternalTerminalHandler - Add aria-label to refresh button in terminal settings for accessibility - Only show "no terminals" message when not refreshing - Reset initialCwdHandledRef on failure to allow retries - Use z.coerce.number() for nonce URL param to handle string coercion - Preserve branchName when creating layout for empty tab - Update getDefaultTerminal return type to allow null result
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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/ui/src/components/views/terminal-view.tsx (1)
1324-1353: Keep branch name visible when a terminal is maximized.Branch name is now passed for standard panels, but the maximized panel path doesn’t receive it, so the header loses the branch indicator when maximized. Consider plumbing
branchNamethere as well (e.g., lookup by sessionId).
🤖 Fix all issues with AI agents
In `@apps/server/src/routes/worktree/routes/open-in-terminal.ts`:
- Around line 144-175: The handler createOpenInExternalTerminalHandler must
validate terminalId's runtime type before calling openInExternalTerminal: if
terminalId is present but not a string, respond with 400 and an error like
"terminalId must be a string" to avoid silent mismatches in findTerminalById;
add this lightweight check immediately after validating worktreePath and before
invoking openInExternalTerminal so only string terminalId values are delegated.
♻️ Duplicate comments (1)
apps/ui/src/components/views/terminal-view.tsx (1)
555-633: Reset the dedupe ref tonull, notundefined.
initialCwdHandledRefis typed asstring | null, but the error paths assignundefined, which is a type mismatch understrictNullChecksand makes the ref state inconsistent. Usenullinstead.🩹 Proposed fix
- initialCwdHandledRef.current = undefined; + initialCwdHandledRef.current = null; ... - initialCwdHandledRef.current = undefined; + initialCwdHandledRef.current = null;
🧹 Nitpick comments (1)
apps/ui/src/components/views/settings-view/terminal/terminal-section.tsx (1)
52-54: Handle missing default terminal IDs gracefully.If a saved
defaultTerminalIdisn’t detected, the Select can render blank. Consider adding a disabled “Unavailable” entry (or auto‑reset to integrated) so users understand why it’s missing.♻️ Suggested tweak
- const { terminals, isRefreshing, refresh } = useAvailableTerminals(); + const { terminals, isRefreshing, refresh } = useAvailableTerminals(); + const terminalIds = new Set(terminals.map((t) => t.id)); + const isDefaultMissing = + defaultTerminalId !== null && !terminalIds.has(defaultTerminalId); ... <SelectContent> <SelectItem value="integrated"> <span className="flex items-center gap-2"> <Terminal className="w-4 h-4" /> Integrated Terminal </span> </SelectItem> + {isDefaultMissing && ( + <SelectItem value={defaultTerminalId} disabled> + <span className="flex items-center gap-2"> + <Terminal className="w-4 h-4" /> + Unavailable (not detected) + </span> + </SelectItem> + )} {terminals.map((terminal) => { const TerminalIcon = getTerminalIcon(terminal.id); return (Also applies to: 96-129
| export function createOpenInExternalTerminalHandler() { | ||
| return async (req: Request, res: Response): Promise<void> => { | ||
| try { | ||
| const { worktreePath, terminalId } = req.body as { | ||
| worktreePath: string; | ||
| terminalId?: string; | ||
| }; | ||
|
|
||
| if (!worktreePath || typeof worktreePath !== 'string') { | ||
| res.status(400).json({ | ||
| success: false, | ||
| error: 'worktreePath required and must be a string', | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| if (!isAbsolute(worktreePath)) { | ||
| res.status(400).json({ | ||
| success: false, | ||
| error: 'worktreePath must be an absolute path', | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| const result = await openInExternalTerminal(worktreePath, terminalId); | ||
| res.json({ | ||
| success: true, | ||
| result: { | ||
| message: `Opened ${worktreePath} in ${result.terminalName}`, | ||
| terminalName: result.terminalName, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for openInExternalTerminal function in platform terminal.ts
rg -n -C5 "openInExternalTerminal" libs/platform/src/terminal.tsRepository: AutoMaker-Org/automaker
Length of output: 543
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get more context on the openInExternalTerminal implementation
rg -n -A 30 "export async function openInExternalTerminal" libs/platform/src/terminal.tsRepository: AutoMaker-Org/automaker
Length of output: 996
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the findTerminalById function to see how it handles the terminalId parameter
rg -n -B2 -A15 "function findTerminalById|export.*findTerminalById" libs/platform/src/terminal.tsRepository: AutoMaker-Org/automaker
Length of output: 904
Validate terminalId type before delegating.
If a client sends a non-string terminalId, it would pass the truthiness check but silently fail in findTerminalById() due to string comparison mismatch, causing unexpected fallback behavior instead of returning a clear 400 error. Add a lightweight runtime check.
🛠️ Proposed fix
const { worktreePath, terminalId } = req.body as {
worktreePath: string;
terminalId?: string;
};
if (!worktreePath || typeof worktreePath !== 'string') {
res.status(400).json({
success: false,
error: 'worktreePath required and must be a string',
});
return;
}
+ if (terminalId !== undefined && typeof terminalId !== 'string') {
+ res.status(400).json({
+ success: false,
+ error: 'terminalId must be a string',
+ });
+ return;
+ }
+
if (!isAbsolute(worktreePath)) {
res.status(400).json({
success: false,
error: 'worktreePath must be an absolute path',
});
return;
}🤖 Prompt for AI Agents
In `@apps/server/src/routes/worktree/routes/open-in-terminal.ts` around lines 144
- 175, The handler createOpenInExternalTerminalHandler must validate
terminalId's runtime type before calling openInExternalTerminal: if terminalId
is present but not a string, respond with 400 and an error like "terminalId must
be a string" to avoid silent mismatches in findTerminalById; add this
lightweight check immediately after validating worktreePath and before invoking
openInExternalTerminal so only string terminalId values are delegated.
Summary
Adds support for opening worktree directories in external terminals (iTerm2, Warp, Ghostty, Windows Terminal, etc.) while retaining Automaker's integrated terminal as the default option.
Related:
Features
1. External Terminal Detection
Cross-platform terminal detection supporting 20+ terminals:
2. Worktree Dropdown Integration
3. Settings
Two new settings in Settings > Terminal:
4. Branch Name Display
When opening a terminal from a worktree, the branch name is displayed in the terminal panel header next to the shell name (e.g., "zsh feature/my-branch").
File Structure
New Files
libs/types/src/terminal.ts- TerminalInfo type definitionlibs/platform/src/terminal.ts- Terminal detection & launching (~500 lines)apps/ui/src/components/icons/terminal-icons.tsx- SVG icons for 15+ terminalsapps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-terminals.ts- React hooks for terminal stateModified Files
apps/server/src/routes/worktree/routes/open-in-terminal.ts- Added 4 new API handlersapps/server/src/routes/worktree/index.ts- Registered new routesapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx- Terminal submenu UIapps/ui/src/components/views/settings-view/terminal/terminal-section.tsx- Settings UI for terminal preferencesapps/ui/src/store/app-store.ts- AddedbranchNameto terminal layout,openTerminalModesettingapps/ui/src/hooks/use-settings-sync.ts- SyncopenTerminalModesettingAPI Endpoints
GET/api/worktree/available-terminalsGET/api/worktree/default-terminalPOST/api/worktree/refresh-terminalsPOST/api/worktree/open-in-external-terminalTesting
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.