feat: Add IDE opening functionality to Worktrees UI #369
Conversation
- Add openPath and openInIde IPC channels for folder/IDE opening - Implement shell handlers for Cursor, VS Code, and Finder - Update Worktrees component with buttons to open in Cursor, VS Code, or Finder - Add proper TypeScript types and browser mock implementations - Includes fallback to macOS 'open -a' command if CLI not available
📝 WalkthroughWalkthroughAdds two new IPC handlers and corresponding shell API methods to enable opening folder paths in different IDEs (Cursor, VS Code, Finder) or system file browser. Updates Worktrees component UI with IDE-specific action buttons and expands mock implementations for browser compatibility. Changes
Sequence DiagramsequenceDiagram
participant Renderer as Renderer (Worktrees)
participant Preload as Preload API
participant Main as Main Process
participant Shell as System Shell
participant App as IDE/Finder
Renderer->>Preload: openInIde(path, 'cursor')
Preload->>Main: IPC: shell:openInIde
Main->>Main: Validate path
Main->>Shell: Attempt cursor command
alt Command Success
Shell-->>App: Open in IDE
App-->>Renderer: Success
else Fallback (macOS)
Main->>Shell: open -a Cursor path
Shell-->>App: Launch via app
App-->>Renderer: Success
else Final Fallback
Main->>Shell: Open in file browser
Shell-->>App: Finder/Explorer
App-->>Renderer: Success/Error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thanks for your first PR!
A maintainer will review it soon. Please make sure:
- Your branch is synced with
develop - CI checks pass
- You've followed our contribution guide
Welcome to the Auto Claude community!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/frontend/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.json
📒 Files selected for processing (6)
apps/frontend/src/main/ipc-handlers/settings-handlers.tsapps/frontend/src/preload/api/modules/shell-api.tsapps/frontend/src/renderer/components/Worktrees.tsxapps/frontend/src/renderer/lib/mocks/settings-mock.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/shared/types/ipc.ts
🧰 Additional context used
📓 Path-based instructions (2)
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/frontend/src/**/*.{ts,tsx}: Always use translation keys withuseTranslation()for all user-facing text in React/TypeScript frontend components - use formatnamespace:section.key(e.g.,navigation:items.githubPRs)
Never use hardcoded strings in JSX/TSX files for user-facing text - always reference translation keys fromapps/frontend/src/shared/i18n/locales/
Files:
apps/frontend/src/renderer/components/Worktrees.tsxapps/frontend/src/main/ipc-handlers/settings-handlers.tsapps/frontend/src/renderer/lib/mocks/settings-mock.tsapps/frontend/src/shared/types/ipc.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/preload/api/modules/shell-api.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/renderer/components/Worktrees.tsxapps/frontend/src/main/ipc-handlers/settings-handlers.tsapps/frontend/src/renderer/lib/mocks/settings-mock.tsapps/frontend/src/shared/types/ipc.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/preload/api/modules/shell-api.ts
🧬 Code graph analysis (2)
apps/frontend/src/renderer/components/Worktrees.tsx (1)
.design-system/src/components/Button.tsx (1)
Button(10-44)
apps/frontend/src/main/ipc-handlers/settings-handlers.ts (1)
apps/frontend/src/shared/constants/ipc.ts (1)
IPC_CHANNELS(6-367)
🔇 Additional comments (5)
apps/frontend/src/shared/constants/ipc.ts (1)
120-121: LGTM!The new IPC channel constants follow the existing naming conventions and are correctly placed in the Shell operations section.
apps/frontend/src/shared/types/ipc.ts (1)
451-452: LGTM!The method signatures are correctly typed and consistent with the rest of the ElectronAPI interface. The distinction between
openPathreturningvoidandopenInIdereturningIPCResultis appropriate for their respective error handling needs.apps/frontend/src/renderer/components/Worktrees.tsx (1)
15-17: LGTM!The icon imports are appropriate for the new IDE-opening actions.
apps/frontend/src/renderer/lib/mocks/settings-mock.ts (1)
27-40: LGTM!The mock implementations appropriately simulate shell operations in browser mode. The warning messages and fallback behaviors (opening URL in new tab, returning failure for IDE operations) are correct for a browser environment.
apps/frontend/src/preload/api/modules/shell-api.ts (1)
4-27: LGTM!The IDEType definition, interface extensions, and implementation correctly wire the shell operations to their respective IPC channels. The types are consistent with the ElectronAPI interface definitions.
| ipcMain.handle( | ||
| IPC_CHANNELS.SHELL_OPEN_PATH, | ||
| async (_, folderPath: string): Promise<void> => { | ||
| await shell.openPath(folderPath); | ||
| } | ||
| ); |
There was a problem hiding this comment.
Add error handling and path validation.
The SHELL_OPEN_PATH handler lacks error handling and path validation, making it inconsistent with other IPC handlers and potentially unsafe.
🔎 Proposed fix
ipcMain.handle(
IPC_CHANNELS.SHELL_OPEN_PATH,
- async (_, folderPath: string): Promise<void> => {
- await shell.openPath(folderPath);
+ async (_, folderPath: string): Promise<IPCResult> => {
+ try {
+ // Validate path exists
+ if (!existsSync(folderPath)) {
+ return { success: false, error: `Path does not exist: ${folderPath}` };
+ }
+
+ const result = await shell.openPath(folderPath);
+
+ // shell.openPath returns a string with an error message if it fails
+ if (result) {
+ return { success: false, error: result };
+ }
+
+ return { success: true };
+ } catch (error) {
+ return {
+ success: false,
+ error: error instanceof Error ? error.message : 'Failed to open path'
+ };
+ }
}
);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/frontend/src/main/ipc-handlers/settings-handlers.ts around lines
303–308, the SHELL_OPEN_PATH handler currently calls shell.openPath without
validating the folderPath or handling errors; update it to first validate the
input (ensure folderPath is a non-empty string and a valid absolute/path that
exists via fs.stat or fs.existsSync), then wrap the shell.openPath call in a
try/catch, await its result and treat a non-empty result string as a failure
(throw or return an error), and log errors (or return a standardized IPC error
response) so the renderer gets consistent success/failure behavior.
| try { | ||
| // Use execSync to run the command - this opens the IDE | ||
| execSync(`${cmd} ${args.join(' ')}`, { | ||
| cwd: folderPath, | ||
| stdio: 'ignore', | ||
| timeout: 5000 | ||
| }); | ||
| return { success: true }; |
There was a problem hiding this comment.
CRITICAL: Command injection vulnerability.
Using string interpolation with execSync creates a command injection vulnerability. If folderPath contains special shell characters (e.g., ; rm -rf /), it could execute arbitrary commands.
🔎 Proposed fix
Use an array-based approach or properly escape arguments:
try {
- // Use execSync to run the command - this opens the IDE
- execSync(`${cmd} ${args.join(' ')}`, {
+ // Use spawn for safer command execution
+ const { spawnSync } = require('child_process');
+ const result = spawnSync(cmd, [...args, folderPath], {
- cwd: folderPath,
stdio: 'ignore',
timeout: 5000
});
+
+ if (result.error) {
+ throw result.error;
+ }
+
return { success: true };Alternatively, if you must use execSync, properly escape the path:
+ const { execSync } = require('child_process');
+ // Escape the folder path to prevent command injection
+ const escapedPath = folderPath.replace(/"/g, '\\"');
+
- execSync(`${cmd} ${args.join(' ')}`, {
- cwd: folderPath,
+ execSync(`${cmd} "${escapedPath}"`, {
stdio: 'ignore',
timeout: 5000
});But spawnSync with an array of arguments is the safer approach.
Committable suggestion skipped: line range outside the PR's diff.
| for (const appName of apps) { | ||
| try { | ||
| execSync(`open -a "${appName}" "${folderPath}"`, { | ||
| stdio: 'ignore', | ||
| timeout: 5000 | ||
| }); | ||
| return { success: true }; | ||
| } catch { | ||
| // Try next app name | ||
| continue; | ||
| } | ||
| } |
There was a problem hiding this comment.
CRITICAL: Command injection vulnerability in macOS fallback.
The same command injection vulnerability exists in the macOS fallback path. The folderPath variable is interpolated directly into the shell command without escaping.
🔎 Proposed fix
const apps = appNames[ideToUse] || appNames.cursor;
for (const appName of apps) {
try {
- execSync(`open -a "${appName}" "${folderPath}"`, {
+ const { spawnSync } = require('child_process');
+ const result = spawnSync('open', ['-a', appName, folderPath], {
stdio: 'ignore',
timeout: 5000
});
+
+ if (result.error) {
+ continue;
+ }
+
return { success: true };
} catch {
// Try next app name
continue;
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/frontend/src/main/ipc-handlers/settings-handlers.ts around lines 356 to
367, the macOS fallback uses execSync with a shell-interpolated command
containing folderPath, which creates a command-injection risk; replace the
execSync shell call with a non-shell child process API (e.g. execFileSync or
spawnSync) that passes the executable and each argument separately (['-a',
appName, folderPath']) so folderPath is never interpreted by a shell, and
optionally validate/normalize appName against the known apps list before
invoking.
| // Fallback: open in Finder/Explorer | ||
| console.warn(`[SHELL_OPEN_IN_IDE] Falling back to system file browser`); | ||
| await shell.openPath(folderPath); | ||
| return { | ||
| success: true, | ||
| error: `${ideToUse} not found. Opened in file browser instead.` | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify the success/error return pattern.
Returning success: true with an error message is confusing. Consider either returning success: false or using a different field like warning to indicate the fallback behavior.
🔎 Proposed refactor
// Fallback: open in Finder/Explorer
console.warn(`[SHELL_OPEN_IN_IDE] Falling back to system file browser`);
await shell.openPath(folderPath);
return {
success: true,
- error: `${ideToUse} not found. Opened in file browser instead.`
+ data: {
+ warning: `${ideToUse} not found. Opened in file browser instead.`
+ }
};Or if you want to treat it as a partial failure:
// Fallback: open in Finder/Explorer
console.warn(`[SHELL_OPEN_IN_IDE] Falling back to system file browser`);
await shell.openPath(folderPath);
return {
- success: true,
+ success: false,
error: `${ideToUse} not found. Opened in file browser instead.`
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Fallback: open in Finder/Explorer | |
| console.warn(`[SHELL_OPEN_IN_IDE] Falling back to system file browser`); | |
| await shell.openPath(folderPath); | |
| return { | |
| success: true, | |
| error: `${ideToUse} not found. Opened in file browser instead.` | |
| }; | |
| } | |
| // Fallback: open in Finder/Explorer | |
| console.warn(`[SHELL_OPEN_IN_IDE] Falling back to system file browser`); | |
| await shell.openPath(folderPath); | |
| return { | |
| success: false, | |
| error: `${ideToUse} not found. Opened in file browser instead.` | |
| }; |
| onClick={async () => { | ||
| // Open in Cursor (default IDE) | ||
| const result = await window.electronAPI.openInIde(worktree.path, 'cursor'); | ||
| if (!result.success) { | ||
| setError(result.error || 'Failed to open in Cursor'); | ||
| } | ||
| }} | ||
| > | ||
| <Code2 className="h-3.5 w-3.5 mr-1.5" /> | ||
| Open in Cursor | ||
| </Button> | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| onClick={async () => { | ||
| // Open in VS Code | ||
| const result = await window.electronAPI.openInIde(worktree.path, 'vscode'); | ||
| if (!result.success) { | ||
| setError(result.error || 'Failed to open in VS Code'); | ||
| } | ||
| }} | ||
| > | ||
| <ExternalLink className="h-3.5 w-3.5 mr-1.5" /> | ||
| VS Code | ||
| </Button> | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| onClick={async () => { | ||
| // Open in Finder | ||
| await window.electronAPI.openInIde(worktree.path, 'finder'); | ||
| }} | ||
| > | ||
| <FolderOpen className="h-3.5 w-3.5 mr-1.5" /> | ||
| Copy Path | ||
| Finder | ||
| </Button> |
There was a problem hiding this comment.
Replace hardcoded strings with translation keys.
All user-facing button text must use translation keys as per coding guidelines. Additionally, the Finder button lacks error handling that the other IDE buttons have.
🔎 Proposed fixes
First, ensure you have translation keys defined in apps/frontend/src/shared/i18n/locales/. For example, in your English locale file:
{
"worktrees": {
"actions": {
"openInCursor": "Open in Cursor",
"openInVSCode": "VS Code",
"openInFinder": "Finder",
"failedToOpen": "Failed to open in {{ide}}"
}
}
}Then update the component:
+import { useTranslation } from 'react-i18next';
+
export function Worktrees({ projectId }: WorktreesProps) {
+ const { t } = useTranslation();
const projects = useProjectStore((state) => state.projects);
// ... rest of component
// In the JSX:
<Button
variant="outline"
size="sm"
onClick={async () => {
- // Open in Cursor (default IDE)
const result = await window.electronAPI.openInIde(worktree.path, 'cursor');
if (!result.success) {
- setError(result.error || 'Failed to open in Cursor');
+ setError(result.error || t('worktrees:actions.failedToOpen', { ide: 'Cursor' }));
}
}}
>
<Code2 className="h-3.5 w-3.5 mr-1.5" />
- Open in Cursor
+ {t('worktrees:actions.openInCursor')}
</Button>
<Button
variant="outline"
size="sm"
onClick={async () => {
- // Open in VS Code
const result = await window.electronAPI.openInIde(worktree.path, 'vscode');
if (!result.success) {
- setError(result.error || 'Failed to open in VS Code');
+ setError(result.error || t('worktrees:actions.failedToOpen', { ide: 'VS Code' }));
}
}}
>
<ExternalLink className="h-3.5 w-3.5 mr-1.5" />
- VS Code
+ {t('worktrees:actions.openInVSCode')}
</Button>
<Button
variant="outline"
size="sm"
onClick={async () => {
- // Open in Finder
- await window.electronAPI.openInIde(worktree.path, 'finder');
+ const result = await window.electronAPI.openInIde(worktree.path, 'finder');
+ if (!result.success) {
+ setError(result.error || t('worktrees:actions.failedToOpen', { ide: 'Finder' }));
+ }
}}
>
<FolderOpen className="h-3.5 w-3.5 mr-1.5" />
- Finder
+ {t('worktrees:actions.openInFinder')}
</Button>As per coding guidelines: "Always use translation keys with useTranslation() for all user-facing text in React/TypeScript frontend components - use format namespace:section.key (e.g., navigation:items.githubPRs). Never use hardcoded strings in JSX/TSX files for user-facing text."
Committable suggestion skipped: line range outside the PR's diff.
MikeeBuilds
added
feature
|
|
4 participants
Summary
Adds the ability to open worktree folders directly in IDEs (Cursor, VS Code) or Finder from the Worktrees view.
Changes:
SHELL_OPEN_PATHandSHELL_OPEN_IN_IDEIPC channelscursor .,code .)open -acommandScreenshots
The Worktrees card now shows:
Test Plan
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.