-
Notifications
You must be signed in to change notification settings - Fork 528
feat: add default IDE setting and multi-editor support with icons #423
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
Conversation
Add comprehensive editor detection and selection system that allows users to configure their preferred IDE for opening branches and worktrees. ## Server-side Changes - Add `/api/worktree/available-editors` endpoint to detect installed editors - Support detection via CLI commands (cursor, code, zed, subl, etc.) - Support detection via macOS app bundles in /Applications and ~/Applications - Detect editors: Cursor, VS Code, Zed, Sublime Text, Windsurf, Trae, Rider, WebStorm, Xcode, Android Studio, Antigravity, and file managers ## UI Changes ### Editor Icons - Add new `editor-icons.tsx` with SVG icons for all supported editors - Icons: Cursor, VS Code, Zed, Sublime Text, Windsurf, Trae, Rider, WebStorm, Xcode, Android Studio, Antigravity, Finder - `getEditorIcon()` helper maps editor commands to appropriate icons ### Default IDE Setting - Add "Default IDE" selector in Settings > Account section - Options: Auto-detect (Cursor > VS Code > first available) or explicit choice - Setting persists via `defaultEditorCommand` in global settings ### Worktree Dropdown Improvements - Implement split-button UX for "Open In" action - Click main area: opens directly in default IDE (single click) - Click chevron: shows submenu with other editors + Copy Path - Each editor shows with its branded icon ## Type & Store Changes - Add `defaultEditorCommand: string | null` to GlobalSettings - Add to app-store with `setDefaultEditorCommand` action - Add to SETTINGS_FIELDS_TO_SYNC for persistence - Add `useAvailableEditors` hook for fetching detected editors Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
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 📝 WalkthroughWalkthroughAdds backend editor detection and a GET /available-editors endpoint; expands open-in-editor to accept an optional editorCommand and safe execution with fallbacks. UI: new editor icons, hooks, settings picker, split "Open in" menu, persisted Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant Server
participant FS
Note over UI,Server: Detect & list editors
User->>UI: Open editor picker
UI->>Server: GET /available-editors
Server->>FS: Check CLI commands & macOS app bundles
FS-->>Server: Existence results
Server->>Server: Cache & return editor list
Server-->>UI: [{name, command}, ...]
Note over UI,Server: Open in selected editor
User->>UI: Select editor (or default)
UI->>Server: POST /open-in-editor {path, editorCommand?}
Server->>Server: Resolve EditorInfo
Server->>FS: Exec editor command (safe execFile)
alt launch succeeds
FS-->>Server: Process started
Server-->>UI: {success:true, editorName}
else launch fails
FS-->>Server: Error
Server->>FS: Launch file-manager fallback
FS-->>Server: Process started
Server-->>UI: {success:true, editorName:"File Manager"}
end
UI->>User: Show result/notification
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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. 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 improves the user experience by introducing a comprehensive and user-configurable system for interacting with various code editors. It allows users to seamlessly open worktrees in their preferred development environment, offering both a default choice and the flexibility to select from all available options, all while providing clear visual cues through branded icons. 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 comprehensive editor detection and selection system, which is a great enhancement for developer experience. The server-side detection is robust, covering both CLI commands and macOS app bundles. The UI changes, including the new editor icons, the 'Default IDE' setting, and the split-button for opening worktrees, are well-implemented and improve usability.
My review includes a few suggestions to improve performance and maintainability:
- Parallelizing the editor detection on the server for a faster initial response.
- Refactoring some duplicated logic in the UI components into a shared hook.
- A minor simplification for rendering icons in React components.
Overall, this is a solid feature addition with well-thought-out details.
apps/ui/src/components/views/settings-view/account/account-section.tsx
Outdated
Show resolved
Hide resolved
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/server/src/routes/worktree/routes/open-in-editor.ts (1)
178-203: Potential command injection vulnerability withworktreePathandeditorCommand.User-controlled input is interpolated directly into shell commands at Line 203:
await execAsync(\${editor.command} "${worktreePath}"`). WhileworktreePathis quoted, a path containing"$(malicious)"or backticks could still execute arbitrary commands. TheeditorCommandis validated against detected editors (Line 196), which mitigates direct injection, butworktreePath` remains unvalidated.Consider sanitizing the path or using
spawnwith argument arrays instead of string interpolation.Proposed fix using spawn
-import { exec } from 'child_process'; +import { exec, spawn } from 'child_process'; import { promisify } from 'util';Then for the command execution:
- await execAsync(`${editor.command} "${worktreePath}"`); + // Use spawn with arguments to avoid shell injection + const [cmd, ...args] = editor.command.split(' '); + await new Promise<void>((resolve, reject) => { + const child = spawn(cmd, [...args, worktreePath], { stdio: 'ignore', detached: true }); + child.on('error', reject); + child.unref(); + resolve(); + });
🤖 Fix all issues with AI agents
In @apps/server/src/routes/worktree/routes/open-in-editor.ts:
- Around line 38-57: The check in findMacApp uses a literal '~' when
process.env.HOME is missing which doesn't expand in the shell; replace the
fallback with a real home directory (e.g., use os.homedir() or
require('os').homedir()) and build the app path with path.join (or string
concat) so execAsync(`test -d "${homeDir}/Applications/${appName}.app"`)
receives an absolute path; update the findMacApp function to import/use
os.homedir() (and optionally path.join) instead of the '~' fallback when calling
execAsync.
In @apps/ui/src/components/views/settings-view/account/account-section.tsx:
- Around line 23-42: The saved defaultEditorCommand may point to an editor not
present in editors, and getEffectiveDefaultEditor() uses editors[0] without
checking for emptiness; update getEffectiveDefaultEditor to first check if
editors.length === 0 and return null (or a safe fallback), and when
defaultEditorCommand is set but not found in editors normalize the bound Select
value to 'auto' so the UI stays consistent; specifically adjust
getEffectiveDefaultEditor, the logic that computes effectiveEditor, and where
the Select value is derived from defaultEditorCommand to map missing commands to
'auto'.
In @apps/ui/src/lib/electron.ts:
- Around line 1628-1638: The mock openInEditor currently maps only 'cursor' to
"Cursor" and defaults everything else to "VS Code", causing commands like 'open'
(Finder) to be reported incorrectly; update the openInEditor implementation to
map editorCommand values to their correct display names (e.g., 'cursor' ->
"Cursor", 'open' -> "Finder", 'code'|'vscode' -> "VS Code") using a small lookup
or switch, defaulting to a sensible fallback like the raw editorCommand or
"Unknown Editor", and then use that derived editorName in the returned message
and console.log.
🧹 Nitpick comments (4)
apps/ui/src/store/app-store.ts (1)
964-966: Consider aligning persistence behavior with other setters (sync vs no-sync).
setDefaultEditorCommandonly updates Zustand state. If persistence is solely viauseSettingsSync, that’s fine—but it’s inconsistent with many other setters here that callsyncSettingsToServer(). Consider either (a) adding a brief comment noting it relies onuseSettingsSync, or (b) mirroring the existing “sync + revert on failure” pattern if you want immediate durability.Also applies to: 1970-1972
apps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-editors.ts (1)
12-46: Avoid setState-after-unmount in async hook.
fetchAvailableEditors()can resolve after unmount and still callsetEditors/setIsLoading.One simple pattern
export function useAvailableEditors() { const [editors, setEditors] = useState<EditorInfo[]>([]); const [isLoading, setIsLoading] = useState(true); + const [isMounted, setIsMounted] = useState(true); const fetchAvailableEditors = useCallback(async () => { try { const api = getElectronAPI(); if (!api?.worktree?.getAvailableEditors) { - setIsLoading(false); + if (isMounted) setIsLoading(false); return; } const result = await api.worktree.getAvailableEditors(); - if (result.success && result.result?.editors) { + if (isMounted && result.success && result.result?.editors) { setEditors(result.result.editors); } } catch (error) { logger.error('Failed to fetch available editors:', error); } finally { - setIsLoading(false); + if (isMounted) setIsLoading(false); } - }, []); + }, [isMounted]); useEffect(() => { fetchAvailableEditors(); + return () => setIsMounted(false); }, [fetchAvailableEditors]);apps/server/src/routes/worktree/routes/open-in-editor.ts (1)
140-155: Consider emitting events for editor detection operations.As per coding guidelines, server operations should use
createEventEmitter()fromlib/events.tsto emit events that stream to the frontend via WebSocket. This endpoint could benefit from emitting events for editor detection progress, especially if the detection becomes slow.apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx (1)
86-109: Consider memoizinggetEffectiveDefaultEditorto avoid recalculation on every render.The function performs array searches on each render. While not a performance bottleneck with a small editor list, wrapping with
useMemowould be cleaner and prevent unnecessary work when unrelated props change.Proposed refactor
+import { useMemo } from 'react'; - // Calculate effective default editor based on user setting or auto-detect (Cursor > VS Code > first) - const getEffectiveDefaultEditor = () => { + // Calculate effective default editor based on user setting or auto-detect (Cursor > VS Code > first) + const effectiveDefaultEditor = useMemo(() => { if (defaultEditorCommand) { const found = editors.find((e) => e.command === defaultEditorCommand); if (found) return found; } // Auto-detect: prefer Cursor, then VS Code, then first available const cursor = editors.find((e) => e.command === 'cursor'); if (cursor) return cursor; const vscode = editors.find((e) => e.command === 'code'); if (vscode) return vscode; return editors[0]; - }; - - const effectiveDefaultEditor = getEffectiveDefaultEditor(); + }, [editors, defaultEditorCommand]);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/server/src/routes/worktree/index.tsapps/server/src/routes/worktree/routes/open-in-editor.tsapps/ui/src/components/icons/editor-icons.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxapps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-editors.tsapps/ui/src/components/views/board-view/worktree-panel/hooks/use-worktree-actions.tsapps/ui/src/components/views/settings-view/account/account-section.tsxapps/ui/src/hooks/use-settings-sync.tsapps/ui/src/lib/electron.tsapps/ui/src/lib/http-api-client.tsapps/ui/src/store/app-store.tsapps/ui/src/types/electron.d.tslibs/types/src/settings.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxapps/ui/src/components/views/board-view/worktree-panel/hooks/use-worktree-actions.tslibs/types/src/settings.tsapps/ui/src/lib/http-api-client.tsapps/ui/src/hooks/use-settings-sync.tsapps/server/src/routes/worktree/routes/open-in-editor.tsapps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-editors.tsapps/ui/src/store/app-store.tsapps/server/src/routes/worktree/index.tsapps/ui/src/types/electron.d.tsapps/ui/src/components/icons/editor-icons.tsxapps/ui/src/components/views/settings-view/account/account-section.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxapps/ui/src/lib/electron.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
resolveModelString()from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Files:
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxapps/ui/src/components/views/board-view/worktree-panel/hooks/use-worktree-actions.tslibs/types/src/settings.tsapps/ui/src/lib/http-api-client.tsapps/ui/src/hooks/use-settings-sync.tsapps/server/src/routes/worktree/routes/open-in-editor.tsapps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-editors.tsapps/ui/src/store/app-store.tsapps/server/src/routes/worktree/index.tsapps/ui/src/types/electron.d.tsapps/ui/src/components/icons/editor-icons.tsxapps/ui/src/components/views/settings-view/account/account-section.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxapps/ui/src/lib/electron.ts
apps/server/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
createEventEmitter()fromlib/events.tsfor all server operations to emit events that stream to frontend via WebSocket
Files:
apps/server/src/routes/worktree/routes/open-in-editor.tsapps/server/src/routes/worktree/index.ts
🧠 Learnings (1)
📚 Learning: 2025-12-30T01:02:07.114Z
Learnt from: illia1f
Repo: AutoMaker-Org/automaker PR: 324
File: apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx:122-131
Timestamp: 2025-12-30T01:02:07.114Z
Learning: Tailwind CSS v4 uses postfix syntax for the important modifier: append ! at the end of the utility class (e.g., backdrop-blur-[0px]! or hover:bg-red-500!). The older v3 style used a prefix (!) at the start (e.g., !backdrop-blur-[0px]); prefer the postfix form for consistency across TSX files.
Applied to files:
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxapps/ui/src/components/icons/editor-icons.tsxapps/ui/src/components/views/settings-view/account/account-section.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx
🧬 Code graph analysis (6)
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsx (3)
apps/ui/src/types/electron.d.ts (1)
WorktreeInfo(613-618)libs/types/src/settings.ts (1)
WorktreeInfo(665-676)apps/ui/src/components/views/board-view/worktree-panel/types.ts (1)
WorktreeInfo(9-18)
apps/ui/src/components/views/board-view/worktree-panel/hooks/use-worktree-actions.ts (4)
apps/ui/src/lib/electron.ts (2)
WorktreeInfo(191-191)getElectronAPI(888-897)apps/ui/src/types/electron.d.ts (1)
WorktreeInfo(613-618)libs/types/src/settings.ts (1)
WorktreeInfo(665-676)apps/ui/src/components/views/board-view/worktree-panel/types.ts (1)
WorktreeInfo(9-18)
apps/server/src/routes/worktree/routes/open-in-editor.ts (1)
apps/ui/src/lib/utils.ts (1)
isMac(121-126)
apps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-editors.ts (1)
apps/ui/src/lib/electron.ts (1)
getElectronAPI(888-897)
apps/server/src/routes/worktree/index.ts (1)
apps/server/src/routes/worktree/routes/open-in-editor.ts (1)
createGetAvailableEditorsHandler(140-155)
apps/ui/src/components/views/settings-view/account/account-section.tsx (2)
apps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-editors.ts (1)
useAvailableEditors(12-46)apps/ui/src/components/icons/editor-icons.tsx (1)
getEditorIcon(150-193)
🔇 Additional comments (20)
libs/types/src/settings.ts (2)
779-827: Good defaulting for new field.Initializing
DEFAULT_GLOBAL_SETTINGS.defaultEditorCommandtonullmatches the “auto-detect” behavior described in the PR summary and keeps the default state explicit.
568-571: No schema version bump needed. TheSettingsService.getGlobalSettings()method uses a spread-merge pattern that automatically fills missing fields fromDEFAULT_GLOBAL_SETTINGS. Oldsettings.jsonfiles withoutdefaultEditorCommandwill receivenullon read, ensuring backward compatibility without explicit version-gated migration logic. This is the documented design pattern (see line 127-128: "Missing fields are filled in from DEFAULT_GLOBAL_SETTINGS for forward/backward compatibility") and is validated by the test suite (lines 62-74).apps/ui/src/hooks/use-settings-sync.ts (2)
29-66: Sync coverage looks complete fordefaultEditorCommand.Adding
defaultEditorCommandtoSETTINGS_FIELDS_TO_SYNCensures it participates in diffing + debounced updates.
374-422: Backward-compatible hydration fordefaultEditorCommand.
defaultEditorCommand: serverSettings.defaultEditorCommand ?? nullis the right fallback for older settings payloads.apps/ui/src/store/app-store.ts (1)
588-590: State/default wiring for editor preference looks consistent.
defaultEditorCommand: nullas the initial value aligns with the auto-detect behavior.Also applies to: 1205-1206
apps/ui/src/lib/http-api-client.ts (1)
1557-1612: API additions are straightforward; verify shared typings and server validation.
openInEditor(..., editorCommand?)andgetAvailableEditors()wiring looks good. Please verifyWorktreeAPI/ Electron types were updated accordingly and that the server validates/handles unsupportededitorCommandvalues gracefully.apps/server/src/routes/worktree/index.ts (1)
24-28: No action needed; auth protection and caching are already in place.The
/available-editorsendpoint is protected by the globalauthMiddleware(applied to all/api/*routes in the main app setup), and the handler implements module-level caching of the editor detection results. The UI hook fetches editors once on component mount with no polling loop, so the cached response prevents repeated system calls.apps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsx (1)
40-40: Prop type widening looks correct.
This matches the new “Open In” split-button flow where callers may pass a specific editor command.apps/ui/src/lib/electron.ts (1)
1651-1663: Mock getAvailableEditors LGTM.
Minimal set is fine, and the{ name, command }shape matches the UI hook.apps/ui/src/components/views/board-view/worktree-panel/hooks/use-worktree-actions.ts (1)
128-145: WorktreeAPI signature is properly aligned end-to-end—no action needed.The optional
editorCommandparameter is consistently defined across:
- WorktreeAPI interface (
electron.d.ts):openInEditor(worktreePath: string, editorCommand?: string)- Mock implementation (
electron.ts): matching signature- HTTP client (
http-api-client.ts): matching signature and parameter pass-throughThe code change safely passes
editorCommandthrough all layers with no regression risk.apps/ui/src/types/electron.d.ts (2)
875-885: LGTM!The
openInEditorsignature update with the optionaleditorCommandparameter is well-typed and aligns with the server-side handler that accepts an optional editor command in the request body.
897-907: LGTM!The new
getAvailableEditorsmethod is properly typed with a consistent response structure matching other API methods in this interface.apps/server/src/routes/worktree/routes/open-in-editor.ts (3)
25-32: LGTM!The
commandExistshelper properly abstracts the platform-specific command check (wherefor Windows,whichfor Unix-like systems).
62-82: LGTM!The
tryAddEditorhelper cleanly encapsulates the priority logic (CLI first, then macOS app bundle) and handles theopen -acommand format correctly for apps discovered via bundle path.
84-123: LGTM!The
detectAllEditorsfunction provides comprehensive editor detection with appropriate caching and platform-specific file manager fallbacks.apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx (2)
9-12: LGTM!The new imports are well-organized: submenu components from the dropdown-menu UI library, Copy icon from lucide-react, and the necessary hooks/store for editor functionality.
Also applies to: 26-27, 31-33
234-277: The split-button submenu UI is well-implemented.The pattern of having the main click area open the default editor while the chevron triggers a submenu with alternatives is intuitive. The conditional rendering based on
effectiveDefaultEditorand dynamic icon rendering viagetEditorIconare clean.One minor UX note: the "Copy Path" action at Line 269 doesn't provide visual feedback. Consider adding a toast notification.
apps/ui/src/components/icons/editor-icons.tsx (3)
1-6: LGTM!Clean type definitions using React's built-in types. The
IconComponenttype alias provides good reusability for the icon mapping.
10-145: LGTM!The SVG icon components are well-structured with consistent patterns:
- All accept and spread
IconPropsfor flexibility- Use
fill="currentColor"for theme compatibility- Proper viewBox attributes for scalability
The icon paths appear to be sourced from reputable icon libraries (LobeHub, Simple Icons).
150-193: LGTM!The
getEditorIconfunction provides comprehensive command-to-icon mapping with good fallback logic:
- Direct CLI command lookup
- Pattern matching for macOS
open -astyle commands- Default
FolderOpenfallbackThe case-insensitive matching via
toLowerCase()at Line 176 is appropriate for handling varied app name formats.
Addresses feedback from gemini-code-assist and coderabbitai reviewers: ## Duplicate Code (High Priority) - Extract `getEffectiveDefaultEditor` logic into shared `useEffectiveDefaultEditor` hook - Both account-section.tsx and worktree-actions-dropdown.tsx now use the shared hook ## Performance (Medium Priority) - Refactor `detectAllEditors` to use `Promise.all` for parallel editor detection - Replace sequential `await tryAddEditor()` calls with parallel `findEditor()` checks ## Code Quality (Medium Priority) - Remove verbose IIFE pattern for editor icon rendering - Pre-compute icon components before JSX return statement ## Bug Fixes - Use `os.homedir()` instead of `~` fallback which doesn't expand in shell - Normalize Select value to 'auto' when saved editor command not found in editors - Add defensive check for empty editors array in useEffectiveDefaultEditor - Improve mock openInEditor to correctly map all editor commands to display names Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/server/src/routes/worktree/routes/open-in-editor.ts (1)
196-241: Potential command injection viaworktreePath.The
worktreePathfrom user input is directly interpolated into shell commands without sanitization. A malicious path like"; rm -rf / #could execute arbitrary commands.Consider validating that
worktreePathis an absolute path and doesn't contain shell metacharacters, or useexecFilewith argument arrays instead of string interpolation.Safer approach using execFile
+import { execFile } from 'child_process'; +import { promisify } from 'util'; + +const execFileAsync = promisify(execFile); // In the handler: -await execAsync(`${editor.command} "${worktreePath}"`); +// For simple commands like 'code', 'cursor': +await execFileAsync(editor.command, [worktreePath]);For
open -a "AppName"style commands, you'd need to parse and handle them separately:if (editor.command.startsWith('open -a')) { const appPath = editor.command.replace('open -a ', '').replace(/"/g, ''); await execFileAsync('open', ['-a', appPath, worktreePath]); } else { await execFileAsync(editor.command, [worktreePath]); }
🧹 Nitpick comments (2)
apps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-editors.ts (1)
8-11: Consider using a shared type definition.The
EditorInfointerface is also defined server-side inopen-in-editor.ts. To prevent drift between client and server definitions, consider moving this to a shared types package (e.g.,@automaker/types).apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx (1)
37-37: Remove unuseddefaultEditorNameprop from the component interface.The prop is declared but never used in the component. The refactor to use
useEffectiveDefaultEditorhook replaced its functionality entirely.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/server/src/routes/worktree/routes/open-in-editor.tsapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxapps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-editors.tsapps/ui/src/components/views/settings-view/account/account-section.tsxapps/ui/src/lib/electron.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
apps/server/src/routes/worktree/routes/open-in-editor.tsapps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-editors.tsapps/ui/src/components/views/settings-view/account/account-section.tsxapps/ui/src/lib/electron.tsapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx
apps/server/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
createEventEmitter()fromlib/events.tsfor all server operations to emit events that stream to frontend via WebSocket
Files:
apps/server/src/routes/worktree/routes/open-in-editor.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
resolveModelString()from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Files:
apps/server/src/routes/worktree/routes/open-in-editor.tsapps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-editors.tsapps/ui/src/components/views/settings-view/account/account-section.tsxapps/ui/src/lib/electron.tsapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-30T01:02:07.114Z
Learnt from: illia1f
Repo: AutoMaker-Org/automaker PR: 324
File: apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx:122-131
Timestamp: 2025-12-30T01:02:07.114Z
Learning: Tailwind CSS v4 uses postfix syntax for the important modifier: append ! at the end of the utility class (e.g., backdrop-blur-[0px]! or hover:bg-red-500!). The older v3 style used a prefix (!) at the start (e.g., !backdrop-blur-[0px]); prefer the postfix form for consistency across TSX files.
Applied to files:
apps/ui/src/components/views/settings-view/account/account-section.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx
🧬 Code graph analysis (3)
apps/server/src/routes/worktree/routes/open-in-editor.ts (2)
apps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-editors.ts (1)
EditorInfo(8-11)apps/ui/src/lib/utils.ts (1)
isMac(121-126)
apps/ui/src/components/views/settings-view/account/account-section.tsx (3)
apps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-editors.ts (2)
useAvailableEditors(13-47)useEffectiveDefaultEditor(53-74)apps/ui/src/store/app-store.ts (1)
useAppStore(1250-3143)apps/ui/src/components/icons/editor-icons.tsx (1)
getEditorIcon(150-193)
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx (4)
apps/ui/src/types/electron.d.ts (1)
WorktreeInfo(613-618)apps/ui/src/components/views/board-view/worktree-panel/types.ts (1)
WorktreeInfo(9-18)apps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-editors.ts (2)
useAvailableEditors(13-47)useEffectiveDefaultEditor(53-74)apps/ui/src/components/icons/editor-icons.tsx (1)
getEditorIcon(150-193)
🔇 Additional comments (13)
apps/ui/src/lib/electron.ts (2)
1628-1655: LGTM!The mock implementation correctly handles the optional
editorCommandparameter with a proper display name mapping and sensible fallback to VS Code when no command is specified or the command is unrecognized.
1668-1679: LGTM!The mock
getAvailableEditorsimplementation provides a minimal but functional set of editors for development/testing purposes, matching the expected response structure from the real server endpoint.apps/server/src/routes/worktree/routes/open-in-editor.ts (5)
26-33: LGTM!Simple and effective command existence check using platform-appropriate commands (
wherefor Windows,whichfor Unix).
39-58: LGTM!Good approach checking both
/Applicationsand~/Applications(used by JetBrains Toolbox). Usinghomedir()from theosmodule is the correct way to handle home directory resolution.
64-84: LGTM!Clean abstraction that prioritizes CLI commands over macOS app bundles. The
open -acommand with the full path handles apps installed in non-standard locations.
86-128: Parallel editor detection is well-designed.Good use of
Promise.allfor performance and proper filtering of null results. The file manager fallback ensures there's always at least one option available.
145-160: LGTM!Clean handler implementation for the new endpoint with proper error handling.
apps/ui/src/components/views/settings-view/account/account-section.tsx (2)
26-40: LGTM!Clean state management using the shared
useEffectiveDefaultEditorhook. The normalization ofselectValueto'auto'when the saved editor isn't found ensures a consistent UI state.
75-128: LGTM!Well-structured IDE selector UI with:
- Proper disabled states during loading
- Dynamic icon rendering for each editor option
- Clear auto-detect option as the first choice
- Visual indication when auto-detect is active via the "(Auto)" suffix
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx (2)
85-98: LGTM!Good use of the shared hooks and computing values outside JSX. The
otherEditorsfilter correctly excludes the effective default to avoid duplication in the submenu.
222-262: LGTM!The split-button UX is well-implemented:
- Main click area opens in the effective default editor
- Chevron reveals submenu with other editors and Copy Path
- Conditional separator prevents visual oddities when no other editors exist
apps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-editors.ts (2)
13-47: LGTM!Clean hook implementation with:
- Proper loading state management
- Defensive API availability check
- Error logging without crashing
- Useful convenience properties (
hasMultipleEditors,defaultEditor)
53-74: LGTM!Well-designed fallback logic with clear priority order: user preference → Cursor → VS Code → first available. The
useMemocorrectly optimizes recomputation based on dependencies.
## Security Fix (Command Injection) - Use `execFile` with argument arrays instead of string interpolation - Add `safeOpenInEditor` helper that properly handles `open -a` commands - Validate that worktreePath is an absolute path before execution - Prevents shell metacharacter injection attacks ## Shared Type Definition - Move `EditorInfo` interface to `@automaker/types` package - Server and UI now import from shared package to prevent drift - Re-export from use-available-editors.ts for convenience ## Remove Unused Code - Remove unused `defaultEditorName` prop from WorktreeActionsDropdown - Remove prop from WorktreeTab component interface - Remove useDefaultEditor hook usage from WorktreePanel - Export new hooks from hooks/index.ts Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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: 3
🤖 Fix all issues with AI agents
In @apps/server/src/routes/worktree/routes/open-in-editor.ts:
- Around line 23-30: The commandExists function uses execAsync with a
shell-interpolated string; replace this with a shell-safe execFile call (or a
promisified child_process.execFile) to avoid command injection. Update
commandExists to call execFile/execFileAsync with the executable name ('which'
on POSIX or 'where' on Windows) and pass the command to check as an argument
rather than interpolating it into a shell string; adjust imports to use the
promisified execFile utility and keep the try/catch logic and return values the
same.
- Around line 36-55: The findMacApp function currently shells out with
execAsync("test -d ...") which is unsafe and unnecessary; replace those
execAsync checks with Node fs methods: use path.join(homedir(), 'Applications',
`${appName}.app`) and path.join('/Applications', `${appName}.app`) and verify
existence with fs.promises.access or fs.promises.stat (catching errors) instead
of spawning a shell, keeping the same return values (the absolute app path or
null) and preserving the process.platform === 'darwin' guard.
In
@apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx:
- Around line 251-257: The onClick handler directly calls
navigator.clipboard.writeText(worktree.path) without error handling; change this
to an async handler (e.g., handleCopyPath) that awaits
navigator.clipboard.writeText(worktree.path) inside a try/catch, and on success
show a user-facing confirmation (toast/notification) and on failure show an
error notification and optionally console.error the caught error; update the
DropdownMenuItem onClick to call this new handler so clipboard failures are
handled gracefully.
🧹 Nitpick comments (2)
apps/server/src/routes/worktree/routes/open-in-editor.ts (2)
17-18: Consider adding cache invalidation or TTL.The cached editor information never expires. If a user installs a new editor, they must restart the server to detect it. Consider adding a TTL or manual refresh endpoint.
Example with TTL
let cachedEditor: EditorInfo | null = null; let cachedEditors: EditorInfo[] | null = null; let cacheTimestamp: number = 0; const CACHE_TTL_MS = 5 * 60 * 1000; // 5 minutes function isCacheValid(): boolean { return Date.now() - cacheTimestamp < CACHE_TTL_MS; } async function detectAllEditors(): Promise<EditorInfo[]> { if (cachedEditors && isCacheValid()) { return cachedEditors; } // ... detection logic ... cachedEditors = editors; cacheTimestamp = Date.now(); return editors; }
142-268: Consider emitting events for open-in-editor operations.Per coding guidelines, server operations should use
createEventEmitter()fromlib/events.tsto stream events to the frontend via WebSocket. This would enable real-time feedback for editor launch success/failure.As per coding guidelines: "Use
createEventEmitter()fromlib/events.tsfor all server operations to emit events that stream to frontend via WebSocket"
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/server/src/routes/worktree/routes/open-in-editor.tsapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxapps/ui/src/components/views/board-view/worktree-panel/hooks/index.tsapps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-editors.tsapps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsxlibs/types/src/editor.tslibs/types/src/index.ts
💤 Files with no reviewable changes (1)
- apps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx
✅ Files skipped from review due to trivial changes (1)
- libs/types/src/editor.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/ui/src/components/views/board-view/worktree-panel/hooks/use-available-editors.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
libs/types/src/index.tsapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxapps/server/src/routes/worktree/routes/open-in-editor.tsapps/ui/src/components/views/board-view/worktree-panel/hooks/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
resolveModelString()from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Files:
libs/types/src/index.tsapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsxapps/server/src/routes/worktree/routes/open-in-editor.tsapps/ui/src/components/views/board-view/worktree-panel/hooks/index.ts
apps/server/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
createEventEmitter()fromlib/events.tsfor all server operations to emit events that stream to frontend via WebSocket
Files:
apps/server/src/routes/worktree/routes/open-in-editor.ts
🧠 Learnings (1)
📚 Learning: 2025-12-30T01:02:07.114Z
Learnt from: illia1f
Repo: AutoMaker-Org/automaker PR: 324
File: apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx:122-131
Timestamp: 2025-12-30T01:02:07.114Z
Learning: Tailwind CSS v4 uses postfix syntax for the important modifier: append ! at the end of the utility class (e.g., backdrop-blur-[0px]! or hover:bg-red-500!). The older v3 style used a prefix (!) at the start (e.g., !backdrop-blur-[0px]); prefer the postfix form for consistency across TSX files.
Applied to files:
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsxapps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsx
🧬 Code graph analysis (1)
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsx (3)
apps/ui/src/types/electron.d.ts (1)
WorktreeInfo(613-618)libs/types/src/settings.ts (1)
WorktreeInfo(665-676)apps/ui/src/components/views/board-view/worktree-panel/types.ts (1)
WorktreeInfo(9-18)
🔇 Additional comments (6)
libs/types/src/index.ts (1)
207-208: LGTM!The EditorInfo type export is correctly placed and follows the established pattern for type exports in this file.
apps/server/src/routes/worktree/routes/open-in-editor.ts (2)
181-190: LGTM! Excellent use ofexecFilefor security.The
safeOpenInEditorfunction correctly usesexecFileinstead ofexecto prevent command injection. The handling of both macOSopen -acommands and direct CLI commands is well implemented.
208-215: LGTM! Essential security validation.Validating that
worktreePathis an absolute path prevents path traversal and other path-based attacks.apps/ui/src/components/views/board-view/worktree-panel/hooks/index.ts (1)
6-6: LGTM!The updated exports correctly expose the new editor-related hooks.
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-tab.tsx (1)
39-39: LGTM!The interface update correctly adds the optional
editorCommandparameter, enabling per-invocation editor selection.apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx (1)
220-260: LGTM! Excellent split-button UX pattern.The implementation of the split-button with submenu provides a clean UX: default editor on main click, other editors in the chevron submenu. The dynamic icon rendering and integration with
useAvailableEditorsis well structured.
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx
Show resolved
Hide resolved
|
@stefandevo Hey, thanks for creating the PR. The E2E might fail in our v0.10.0-rc branch, but that’s fine. If you could just address the latest CodeRabbit comments, that would be great. |
Security improvements in open-in-editor.ts: - Use execFile with argument arrays instead of shell interpolation in commandExists() to prevent command injection - Replace shell `test -d` commands with Node.js fs/promises access() in findMacApp() for safer file system checks - Add cache TTL (5 minutes) for editor detection to prevent stale data UX improvements in worktree-actions-dropdown.tsx: - Add error handling for clipboard copy operation - Show success toast when path is copied - Show error toast if clipboard write fails Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
@coderabbitai can you review again with latest changes? |
|
@stefandevo: I'll review the latest changes in this PR. ✅ Actions performedReview triggered.
|
@Shironex it seems that Coderabbit will not re-review my latest changes. |
|
Tip For best results, initiate chat on the files or code changes. @stefandevo: I understand your concern! Let me clarify how CodeRabbit's review system works: CodeRabbit uses an incremental review approach, which means:
When you used the "review again" command, I triggered a review of any new commits. If all commits in the PR have already been reviewed, the incremental review won't produce new comments. If you'd like a complete re-review of all changes from scratch, I can do that using the full review command. This will:
Would you like me to perform a full re-review of all changes in this PR? Just let me know, and I'll trigger it for you! 🐰 |
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/server/src/routes/worktree/routes/open-in-editor.ts (1)
248-273: Inconsistent command execution and missing timeout.Line 265 directly uses
execFileAsyncinstead ofsafeOpenInEditor, creating inconsistency in how commands are executed. Additionally, no timeout is specified, which could cause the fallback to hang.♻️ Proposed fix
if (platform === 'darwin') { fallbackCommand = 'open'; fallbackName = 'Finder'; } else if (platform === 'win32') { fallbackCommand = 'explorer'; fallbackName = 'Explorer'; } else { fallbackCommand = 'xdg-open'; fallbackName = 'File Manager'; } - await execFileAsync(fallbackCommand, [worktreePath]); + await execFileAsync(fallbackCommand, [worktreePath], { timeout: 10000 }); res.json({ success: true, result: { message: `Opened ${worktreePath} in ${fallbackName}`, editorName: fallbackName, }, });Note: Consider using
safeOpenInEditorfor consistency, though in this case the fallback commands are simple and don't need special parsing.
🤖 Fix all issues with AI agents
In @apps/server/src/routes/worktree/routes/open-in-editor.ts:
- Around line 17-25: The cache uses a single cacheTimestamp for both
cachedEditor and cachedEditors causing stale/invalid reads; change cache
management so cachedEditors is the primary source and its freshness is validated
via isCacheValid() (or give each cached value its own timestamp), update
detectDefaultEditor() to not trust cachedEditor directly but instead call
detectAllEditors() or read from cachedEditors (refreshing it when isCacheValid()
is false) and then compute the default editor from that list; ensure functions
referenced—cachedEditor, cachedEditors, cacheTimestamp, isCacheValid(),
detectDefaultEditor(), detectAllEditors()—are updated so cachedEditor is derived
from cachedEditors (or removed) and TTL checks use the appropriate timestamp.
- Around line 228-237: When editorCommand is provided but detectAllEditors()
doesn't find a matching EditorInfo (the specifiedEditor is undefined), don't
silently fall back to detectDefaultEditor(); instead surface the issue by either
returning an error response or logging a warning with details. Update the
open-in-editor route logic that sets editor (where specifiedEditor is computed)
to: if specifiedEditor is undefined, log a clear warning (including the
requested editorCommand and the list of available editors from
detectAllEditors()) or respond with a 400/422 error informing the caller that
the requested editor is unavailable; only call detectDefaultEditor() as an
explicit fallback or after notifying the user.
🧹 Nitpick comments (5)
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx (2)
91-91: Consider memoizing derived editor list.The
otherEditorsarray is recomputed on every render. Since it only depends oneditorsandeffectiveDefaultEditor, consider wrapping it inuseMemoto avoid unnecessary recalculations.♻️ Proposed optimization
- // Get other editors (excluding the default) for the submenu - const otherEditors = editors.filter((e) => e.command !== effectiveDefaultEditor?.command); + // Get other editors (excluding the default) for the submenu + const otherEditors = React.useMemo( + () => editors.filter((e) => e.command !== effectiveDefaultEditor?.command), + [editors, effectiveDefaultEditor] + );
222-268: Consider UX when no editors are detected.The entire "Open In" menu is hidden when
effectiveDefaultEditoris null. While this is functionally correct (can't open in an editor if none are detected), consider whether users should see a disabled menu item or a message explaining why the option is unavailable.This is a minor UX consideration—the current approach is acceptable if the expectation is that at least one editor should always be detected in typical development environments.
apps/server/src/routes/worktree/routes/open-in-editor.ts (3)
31-39: Add timeout to prevent indefinite hangs.The
execFilecall has no timeout, which could cause the detection to hang if the system command stalls.♻️ Proposed fix
async function commandExists(cmd: string): Promise<boolean> { try { const whichCmd = process.platform === 'win32' ? 'where' : 'which'; - await execFileAsync(whichCmd, [cmd]); + await execFileAsync(whichCmd, [cmd], { timeout: 5000 }); return true; } catch { return false; } }
71-91: Consider a more structured approach for macOS app commands.Line 86 constructs the command as a string with embedded quotes (
open -a "${appPath}"), which is then parsed insafeOpenInEditor()using string replacement. This approach is brittle and relies on string manipulation that could fail with unexpected app paths.♻️ Alternative: Store command components separately
Consider extending
EditorInfoto support structured commands:export interface EditorInfo { name: string; command: string; args?: string[]; // Optional arguments }Then update
findEditor:if (appPath) { - // Use 'open -a' with full path for apps not in /Applications - return { name, command: `open -a "${appPath}"` }; + return { name, command: 'open', args: ['-a', appPath] }; }And update
safeOpenInEditorto use the args if present:async function safeOpenInEditor(command: string, targetPath: string): Promise<void> { - if (command.startsWith('open -a ')) { - const appPath = command.replace('open -a ', '').replace(/"/g, ''); - await execFileAsync('open', ['-a', appPath, targetPath]); - } else { - await execFileAsync(command, [targetPath]); - } + if (editor.args) { + await execFileAsync(editor.command, [...editor.args, targetPath]); + } else { + await execFileAsync(editor.command, [targetPath]); + } }Note: This would require updating the
EditorInfotype inlibs/types/src/editor.ts.
192-201: Add timeout to prevent indefinite hangs.Both
execFileAsynccalls lack a timeout, which could cause the operation to hang indefinitely if the editor fails to launch.♻️ Proposed fix
async function safeOpenInEditor(command: string, targetPath: string): Promise<void> { // Handle 'open -a "AppPath"' style commands (macOS) if (command.startsWith('open -a ')) { const appPath = command.replace('open -a ', '').replace(/"/g, ''); - await execFileAsync('open', ['-a', appPath, targetPath]); + await execFileAsync('open', ['-a', appPath, targetPath], { timeout: 10000 }); } else { // Simple commands like 'code', 'cursor', 'zed', etc. - await execFileAsync(command, [targetPath]); + await execFileAsync(command, [targetPath], { timeout: 10000 }); } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/server/src/routes/worktree/routes/open-in-editor.tsapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always import from shared packages (@automaker/*), never from old relative paths
Files:
apps/server/src/routes/worktree/routes/open-in-editor.tsapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx
apps/server/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
createEventEmitter()fromlib/events.tsfor all server operations to emit events that stream to frontend via WebSocket
Files:
apps/server/src/routes/worktree/routes/open-in-editor.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
resolveModelString()from @automaker/model-resolver to convert model aliases (haiku, sonnet, opus) to full model names
Files:
apps/server/src/routes/worktree/routes/open-in-editor.tsapps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-30T01:02:07.114Z
Learnt from: illia1f
Repo: AutoMaker-Org/automaker PR: 324
File: apps/ui/src/components/views/board-view/components/kanban-card/kanban-card.tsx:122-131
Timestamp: 2025-12-30T01:02:07.114Z
Learning: Tailwind CSS v4 uses postfix syntax for the important modifier: append ! at the end of the utility class (e.g., backdrop-blur-[0px]! or hover:bg-red-500!). The older v3 style used a prefix (!) at the start (e.g., !backdrop-blur-[0px]); prefer the postfix form for consistency across TSX files.
Applied to files:
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx
🧬 Code graph analysis (1)
apps/server/src/routes/worktree/routes/open-in-editor.ts (2)
libs/types/src/editor.ts (1)
EditorInfo(8-13)libs/types/src/index.ts (1)
EditorInfo(208-208)
🔇 Additional comments (8)
apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx (5)
49-49: LGTM: Backward-compatible API extension.The optional
editorCommandparameter allows specifying which editor to use while maintaining backward compatibility with existing callers.
253-260: LGTM: Solid clipboard implementation.The async clipboard copy includes proper error handling and user feedback via toast notifications for both success and error cases.
224-235: LGTM: Clean split-button pattern.The split-button implementation with main action area and chevron submenu trigger provides good UX. The flex layout and visual border separation between the main item and trigger are well-executed.
239-246: No changes needed. The code is correct as written.getEditorIcon()always returns a valid React component (withFolderOpenas the fallback), so the direct rendering ofEditorIconon line 246 is safe. The conditional rendering ofDefaultEditorIconon line 230 exists becauseeffectiveDefaultEditoritself can be null, not becausegetEditorIcon()returns null.
30-30: The relative import at line 30 is correct and necessary.The types
WorktreeInfo,DevServerInfo,PRInfo, andGitRepoStatusare not available in@automaker/types. While aWorktreeInfointerface exists in the shared types package, it is structurally different (lacksisCurrent,hasWorktree, andprproperties required by this component).DevServerInfo,PRInfo, andGitRepoStatusexist only as local domain-specific types for the worktree panel. The relative import is appropriate for local, component-scoped types.apps/server/src/routes/worktree/routes/open-in-editor.ts (3)
45-65: LGTM!The function correctly uses filesystem methods to detect macOS apps, checking both system and user directories without shell commands.
93-136: LGTM!The parallel editor detection (lines 102-118) and platform-specific file manager fallback are well-implemented.
153-168: LGTM!The handler correctly exposes the editor detection functionality via API with proper error handling.
Cache management improvements: - Remove separate cachedEditor variable; derive default from cachedEditors - Update isCacheValid() to check cachedEditors existence - detectDefaultEditor() now always goes through detectAllEditors() to ensure cache TTL is respected consistently Editor fallback improvements: - Log warning when requested editorCommand is not found in available editors - Include list of available editor commands in warning message - Make fallback to default editor explicit rather than silent Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CI Test Failure ExplanationThe failing test ( Root CauseThis PR targets // libs/types/src/enhancement.ts in v0.10.0rc
export type EnhancementMode = 'improve' | 'technical' | 'simplify' | 'acceptance' | 'ux-reviewer';However, the test was not updated to reflect this change:
Suggested Resolution The test in v0.10.0rc should be updated to expect 5 modes and include 'ux-reviewer' in the valid modes check. This fix should be made in a separate PR targeting v0.10.0rc, as it's unrelated to the editor/IDE selection feature implemented here. |
…ionality - Add libs/platform/src/editor.ts with cross-platform editor detection and launching - Handles Windows .cmd batch scripts (cursor.cmd, code.cmd, etc.) - Supports macOS app bundles in /Applications and ~/Applications - Includes caching with 5-minute TTL for performance - Refactor open-in-editor.ts to use @automaker/platform utilities - Add POST /api/worktree/refresh-editors endpoint to clear cache - Add refresh button to Settings > Account for IDE selection - Update useAvailableEditors hook with refresh() and isRefreshing Fixes Windows issue where "Open in Editor" was falling back to Explorer because execFile cannot run .cmd scripts without shell:true. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The xed command requires full Xcode.app, not just Command Line Tools. This fix adds validation to ensure Xcode is properly configured before offering it as an editor option. Changes: - Added isXcodeFullyInstalled() to check xcode-select points to Xcode.app - Added helpful warning when Xcode is installed but xcode-select points to CLT - Users see clear instructions on how to fix the configuration Fixes issue where xed would fail with "tool 'xed' requires Xcode" error when only Command Line Tools are configured via xcode-select.
Added support for two new editors: - VS Code Insiders (code-insiders command) - Kiro (kido command) - VS Code fork Changes: - Added editor definitions to SUPPORTED_EDITORS list - Added VSCodeInsidersIcon (reuses VS Code icon) - Added KiroIcon with custom SVG logo - Updated getEditorIcon() to handle both new commands - Fixed logger initialization to be lazy-loaded, preventing circular dependency error with isBrowser variable during module initialization Both editors were tested and successfully open directories on macOS.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace createLogger with console.warn to prevent circular import between @automaker/platform and @automaker/utils. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add comprehensive editor detection and selection system that allows users to configure their preferred IDE for opening branches and worktrees.
Server-side Changes
/api/worktree/available-editorsendpoint to detect installed editorsUI Changes
Editor Icons
editor-icons.tsxwith SVG icons for all supported editorsgetEditorIcon()helper maps editor commands to appropriate iconsDefault IDE Setting
defaultEditorCommandin global settingsWorktree Dropdown Improvements
Type & Store Changes
defaultEditorCommand: string | nullto GlobalSettingssetDefaultEditorCommandactionuseAvailableEditorshook for fetching detected editorsUI
Within the Kanban Board:
Added default setting in Account:
Summary by CodeRabbit
New Features
Other
✏️ Tip: You can customize this high-level summary in your review settings.