feat: worktree panel enhancements and bug fixes #558
+1,029
−185
Conversation
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
Sanitize featureId using the same pattern as worktree creation (replace non-alphanumeric chars with dashes) to ensure consistent path resolution and prevent path traversal issues. Affected routes: diffs, file-diff, info, status Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Remove complex ConPTY/winpty fallback logic on Windows - Improve ANSI escape code stripping to handle OSC, CSI, and private mode sequences more comprehensively - Simplify authentication error detection - Fix reset time parsing for edge cases - Add tests for ANSI stripping edge cases Co-Authored-By: Claude Opus 4.5 <[email protected]>
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 two new worktree API endpoints: - POST /open-in-terminal: Opens system terminal in worktree directory using the cross-platform openInTerminal utility - POST /discard-changes: Discards all uncommitted changes in a worktree (git reset, checkout, clean) Both endpoints validate worktreePath and use appropriate middleware. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add multiple new features to the worktree panel: - Open in Terminal: Opens system terminal in worktree directory, navigates to terminal view with pending cwd - View Changes: New dialog to view git diffs for a worktree - Discard Changes: Discard all uncommitted changes with confirmation - Improved conflict resolution: Choose between pulling from worktree's remote branch or the main branch - Branch tracking: Show "new" badge for branches without remote - Target branch display: Show actual target branch name in merge UI Also adds: - pendingTerminalCwd to app store for terminal navigation - hasRemoteTracking to branch management hook - API client methods for new endpoints - TypeScript types for new functionality Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Increase command input font size to 16px to prevent iOS zoom - Increase branch switch dropdown input font size - Make dev server logs panel full-screen on mobile with proper header spacing for close button Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Include branchName in running agents API response - Pass branchName to agent output modal for correct diff display - Also includes featureId sanitization in auto-mode-service for worktree path resolution Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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. 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 |
Contributor
Summary of ChangesHello @Shironex, 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 delivers significant enhancements and crucial bug fixes to the worktree panel, aiming to streamline developer workflows and improve the overall user experience. New capabilities such as direct terminal access, comprehensive change viewing, and a robust discard changes mechanism empower users with more control over their worktrees. The refined conflict resolution and clearer branch tracking status contribute to a more intuitive Git interaction. Additionally, a critical security vulnerability has been addressed, and the UI has been made more responsive across devices, ensuring a stable and user-friendly environment. 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
|
Shironex
added
Enhancement
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces significant enhancements to the worktree panel, including new features like opening in terminal, viewing changes, and discarding changes, alongside several bug fixes and improvements. The code is generally well-structured. My review focuses on a critical security vulnerability in the new "Open in Terminal" feature and a suggestion to refactor duplicated code for better maintainability.
Comment on lines
384
to
464
| export async function openInTerminal(targetPath: string): Promise<{ terminalName: string }> { | ||
| if (isMac) { | ||
| // Use AppleScript to open Terminal.app in the specified directory | ||
| const script = ` | ||
| tell application "Terminal" | ||
| do script "cd ${targetPath.replace(/"/g, '\\"').replace(/\$/g, '\\$')}" | ||
| activate | ||
| end tell | ||
| `; | ||
| await execFileAsync('osascript', ['-e', script]); | ||
| return { terminalName: 'Terminal' }; | ||
| } else if (isWindows) { | ||
| // Try Windows Terminal first | ||
| try { | ||
| return await new Promise((resolve, reject) => { | ||
| const child: ChildProcess = spawn('wt', ['-d', targetPath], { | ||
| shell: true, | ||
| stdio: 'ignore', | ||
| detached: true, | ||
| }); | ||
| child.unref(); | ||
|
|
||
| child.on('error', () => { | ||
| reject(new Error('Windows Terminal not available')); | ||
| }); | ||
|
|
||
| setTimeout(() => resolve({ terminalName: 'Windows Terminal' }), 100); | ||
| }); | ||
| } catch { | ||
| // Fall back to cmd | ||
| return await new Promise((resolve, reject) => { | ||
| const child: ChildProcess = spawn( | ||
| 'cmd', | ||
| ['/c', 'start', 'cmd', '/k', `cd /d "${targetPath}"`], | ||
| { | ||
| shell: true, | ||
| stdio: 'ignore', | ||
| detached: true, | ||
| } | ||
| ); | ||
| child.unref(); | ||
|
|
||
| child.on('error', (err) => { | ||
| reject(err); | ||
| }); | ||
|
|
||
| setTimeout(() => resolve({ terminalName: 'Command Prompt' }), 100); | ||
| }); | ||
| } | ||
| } else { | ||
| // Linux: Try common terminal emulators in order | ||
| const terminals = [ | ||
| { | ||
| name: 'GNOME Terminal', | ||
| command: 'gnome-terminal', | ||
| args: ['--working-directory', targetPath], | ||
| }, | ||
| { name: 'Konsole', command: 'konsole', args: ['--workdir', targetPath] }, | ||
| { | ||
| name: 'xfce4-terminal', | ||
| command: 'xfce4-terminal', | ||
| args: ['--working-directory', targetPath], | ||
| }, | ||
| { name: 'xterm', command: 'xterm', args: ['-e', `cd "${targetPath}" && $SHELL`] }, | ||
| { | ||
| name: 'x-terminal-emulator', | ||
| command: 'x-terminal-emulator', | ||
| args: ['--working-directory', targetPath], | ||
| }, | ||
| ]; | ||
|
|
||
| for (const terminal of terminals) { | ||
| if (await commandExists(terminal.command)) { | ||
| await execFileAsync(terminal.command, terminal.args); | ||
| return { terminalName: terminal.name }; | ||
| } | ||
| } | ||
|
|
||
| throw new Error('No terminal emulator found'); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
The openInTerminal function has multiple command injection vulnerabilities across different platforms due to unsafe path interpolation into shell commands.
- macOS: The AppleScript
do scriptcommand executes thecdcommand with an unquoted path, allowing for arbitrary command execution. - Windows: The fallback to
cmdusescd /d "${targetPath}", which is vulnerable iftargetPathcontains double quotes. - Linux: The
xtermcommand constructs a shell command stringcd "${targetPath}" && $SHELL, which is also vulnerable.
A malicious targetPath could lead to arbitrary code execution on the user's machine.
To fix this, the function should be refactored to avoid shell interpolation and use platform-specific features for setting the working directory safely. For example:
- On macOS, pass the path as an argument to
osascriptand usequoted form of. - On Windows, use the
cwdoption ofspawnor the/Dflag of thestartcommand. - On Linux, use the appropriate argument for each terminal emulator, like
-cdforxtermor--working-directoryforgnome-terminal.
export async function openInTerminal(targetPath: string): Promise<{ terminalName: string }> {
if (isMac) {
// Use AppleScript to safely open Terminal.app in the specified directory.
// Passing the path as an argument to osascript and using "quoted form of"
// prevents command injection vulnerabilities.
const script = `
on run argv
tell application "Terminal"
if not running then
activate
end if
do script "cd " & quoted form of (item 1 of argv) in window 1
activate
end tell
end run
`;
try {
await execFileAsync('osascript', ['-e', script, targetPath]);
return { terminalName: 'Terminal' };
} catch (error) {
// Fallback for when AppleScript fails (e.g., permissions)
// 'open -a Terminal /path' is safer but opens a new window.
await execFileAsync('open', ['-a', 'Terminal', targetPath]);
return { terminalName: 'Terminal' };
}
} else if (isWindows) {
// Try Windows Terminal first, passing the directory as a safe argument.
try {
return await new Promise((resolve, reject) => {
const child: ChildProcess = spawn('wt.exe', ['-d', targetPath], {
shell: false, // Set to false for security
stdio: 'ignore',
detached: true,
});
child.unref();
child.on('error', (err) => {
// ENOENT means wt.exe is not in PATH.
if ((err as NodeJS.ErrnoException).code === 'ENOENT') {
reject(new Error('Windows Terminal not available'));
} else {
reject(err);
}
});
// Assume success if it doesn't error out immediately.
setTimeout(() => resolve({ terminalName: 'Windows Terminal' }), 200);
});
} catch {
// Fall back to cmd, using the cwd option for safety.
return await new Promise((resolve, reject) => {
const child: ChildProcess = spawn('cmd.exe', ['/c', 'start', 'cmd.exe', '/k'], {
cwd: targetPath,
shell: false,
stdio: 'ignore',
detached: true,
});
child.unref();
child.on('error', reject);
setTimeout(() => resolve({ terminalName: 'Command Prompt' }), 200);
});
}
} else {
// Linux: Try common terminal emulators in order, using safe arguments.
const terminals = [
{
name: 'GNOME Terminal',
command: 'gnome-terminal',
args: ['--working-directory', targetPath],
},
{ name: 'Konsole', command: 'konsole', args: ['--workdir', targetPath] },
{
name: 'xfce4-terminal',
command: 'xfce4-terminal',
args: ['--working-directory', targetPath],
},
{ name: 'xterm', command: 'xterm', args: ['-cd', targetPath] }, // Use -cd for safety
{
name: 'x-terminal-emulator',
command: 'x-terminal-emulator',
args: ['--working-directory', targetPath],
},
];
for (const terminal of terminals) {
if (await commandExists(terminal.command)) {
await execFileAsync(terminal.command, terminal.args);
return { terminalName: terminal.name };
}
}
throw new Error('No supported terminal emulator found on your system.');
}
}
Comment on lines
+42
to
+45
| // Sanitize featureId the same way it's sanitized when creating worktrees | ||
| // (see create.ts: branchName.replace(/[^a-zA-Z0-9_-]/g, '-')) | ||
| const sanitizedFeatureId = featureId.replace(/[^a-zA-Z0-9_-]/g, '-'); | ||
| const worktreePath = path.join(projectPath, '.worktrees', sanitizedFeatureId); |
Contributor
There was a problem hiding this comment.
This featureId sanitization logic is duplicated in multiple places. This reduces maintainability, as any change to the sanitization rule would need to be updated in all locations. Consider creating a shared helper function to centralize this logic.
This logic is repeated in:
apps/server/src/routes/worktree/routes/file-diff.tsapps/server/src/routes/worktree/routes/info.tsapps/server/src/routes/worktree/routes/status.tsapps/server/src/services/auto-mode-service.ts
A shared function like getWorktreePath(projectPath, featureId) could be created in a common utility file.
- Fix Windows crash: Remove SIGTERM argument from pty.kill() since Windows doesn't support Unix signals - Fix command injection in openInTerminal: - macOS: Use AppleScript's "quoted form of" for safe path handling - Windows: Use shell: false and cwd option instead of shell interpolation - Linux: Use -cd flag for xterm instead of shell command Addresses PR review comments from #558. Co-Authored-By: Claude Opus 4.5 <[email protected]>
ConPTY requires AttachConsole which fails when running in Electron or service mode without a console. Force winpty on Windows by setting useConpty: false in PTY spawn options. Co-Authored-By: Claude Opus 4.5 <[email protected]>
stefandevo
added a commit
to stefandevo/automaker
that referenced
this pull request
Jan 17, 2026
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.
stefandevo
added a commit
to stefandevo/automaker
that referenced
this pull request
Jan 17, 2026
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.
stefandevo
added a commit
to stefandevo/automaker
that referenced
this pull request
Jan 17, 2026
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.
stefandevo
added a commit
to stefandevo/automaker
that referenced
this pull request
Jan 17, 2026
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
stefandevo
added a commit
to stefandevo/automaker
that referenced
this pull request
Jan 18, 2026
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.
stefandevo
added a commit
to stefandevo/automaker
that referenced
this pull request
Jan 18, 2026
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.
stefandevo
added a commit
to stefandevo/automaker
that referenced
this pull request
Jan 18, 2026
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.
stefandevo
added a commit
to stefandevo/automaker
that referenced
this pull request
Jan 18, 2026
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
stefandevo
added a commit
to stefandevo/automaker
that referenced
this pull request
Jan 18, 2026
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.
stefandevo
added a commit
to stefandevo/automaker
that referenced
this pull request
Jan 18, 2026
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.
stefandevo
added a commit
to stefandevo/automaker
that referenced
this pull request
Jan 18, 2026
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.
stefandevo
added a commit
to stefandevo/automaker
that referenced
this pull request
Jan 18, 2026
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
stefandevo
added a commit
that referenced
this pull request
Jan 19, 2026
* feat(platform): add cross-platform openInTerminal utility 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]> * feat(server): add open-in-terminal endpoint 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 #558. * feat(ui): add Open in Terminal action to worktree dropdown 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 #558. * fix(ui): open in terminal navigates to Automaker terminal view 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 #558 behavior. * feat(ui): add terminal open mode setting (new tab vs split) 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. * feat(ui): display branch name in terminal header with git icon - 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 * feat: add external terminal support with cross-platform detection 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 #558, Closes #550 * fix: address PR review comments - 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 * fix: address PR review security and validation issues - 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 * chore: update package-lock.json * fix: use response.json() to prevent disposal race condition in E2E test 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. * Revert "fix: use response.json() to prevent disposal race condition in E2E test" This reverts commit 36bdf8c. * fix: address PR review feedback for terminal feature - 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 --------- Co-authored-by: Kacper <[email protected]> Co-authored-by: Claude Opus 4.5 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
This PR adds several enhancements to the worktree panel and fixes various bugs:
New Features
Bug Fixes
Other Improvements
branchNameto running agents API for correct diff displayTest plan
Closes