From 35e13a04b15dceb704f0529a6b0253fa35d1560d Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Fri, 15 May 2026 11:05:21 -0700 Subject: [PATCH] Add worktree Open in editor button (#1929) * Add worktree open-in editor button Implements the Open in menu flow from docs/open-in-worktree-button.md, including shell IPC validation and sidebar menu integration. * Move worktree open action into menu Updates the design to remove the card button and show VS Code under a single Open in context-menu item. * Shorten worktree file manager labels --- docs/open-in-worktree-button.md | 132 +++++++++ src/main/ipc/shell.test.ts | 260 +++++++++++++++++- src/main/ipc/shell.ts | 110 +++++++- src/preload/api-types.ts | 5 + src/preload/index.ts | 7 + .../src/components/sidebar/WorktreeCard.tsx | 10 +- .../sidebar/WorktreeContextMenu.tsx | 24 +- .../sidebar/WorktreeOpenInMenu.test.tsx | 151 ++++++++++ .../components/sidebar/WorktreeOpenInMenu.tsx | 145 ++++++++++ src/renderer/src/web/web-preload-api.ts | 3 + src/shared/shell-open-types.ts | 5 + 11 files changed, 813 insertions(+), 39 deletions(-) create mode 100644 docs/open-in-worktree-button.md create mode 100644 src/renderer/src/components/sidebar/WorktreeOpenInMenu.test.tsx create mode 100644 src/renderer/src/components/sidebar/WorktreeOpenInMenu.tsx create mode 100644 src/shared/shell-open-types.ts diff --git a/docs/open-in-worktree-button.md b/docs/open-in-worktree-button.md new file mode 100644 index 000000000..4084bbee2 --- /dev/null +++ b/docs/open-in-worktree-button.md @@ -0,0 +1,132 @@ +# Open In Menu Item on Worktrees + +## Scope + +Add an `Open in` item to the worktree context menu for local worktree paths. + +Actions: + +- VS Code (directory target) +- Open in OS file manager (directory reveal/open) + +Blocked: + +- Any SSH-backed worktree (`repo.connectionId` set) +- Any non-local runtime context (`activeRuntimeEnvironmentId` set) + +Use `VS Code` as the visible editor label. Keep implementation identifiers generic so launcher behavior can evolve without renaming APIs. + +## Current State (Code-Verified) + +- `WorktreeContextMenu.tsx` has `Open in Finder` (macOS-specific label) and calls `window.api.shell.openPath(worktree.path)`. +- `shell:openPath` in `src/main/ipc/shell.ts` calls `shell.showItemInFolder(path)` directly with no absolute-path validation, no existence check, and no structured return. +- Preload contract exposes `openPath/openFilePath/openFileUri` as `Promise`. +- `isLocalPathOpenBlocked` exists and is tested (`local-path-open-guard.test.ts`), but enforcement is renderer-side only. +- `shell.test.ts` currently does not test `shell:openPath`, `shell:openFilePath`, or `shell:openFileUri` behavior. + +## Required Design Changes + +### 1) IPC: split intent, return structured results + +Add explicit IPC handlers (main + preload + type contract): + +- `shell:openInFileManager(path)` +- `shell:openInExternalEditor(path)` + +Return: + +- `{ ok: true }` +- `{ ok: false, reason: 'not-absolute' | 'not-found' | 'launch-failed' }` + +Why: `Promise` hides failures and forces renderer to assume success. + +### 2) Main-process validation must be authoritative + +For both new handlers: + +- Normalize path. +- Require absolute path. +- Check existence at call time (`stat`). +- Map launcher failure to `launch-failed`. + +Renderer checks are UX only; security/correctness must not depend on renderer state. + +### 3) External editor launch must be feasible and generic + +Launch a generic editor CLI command constant with the normalized directory path as an argv entry. + +Important correction: do not claim this uses the OS default app for a directory. The launcher is platform-dependent and may fail per host config; failure must be surfaced via result union + toast. + +### 4) File manager action semantics + +Keep file-manager action separate from editor action. + +Platform-aware label in renderer: + +- macOS: `Finder` +- Windows: `File Explorer` +- Linux: `File Manager` + +If using reveal semantics, document that behavior explicitly and keep it consistent in the context menu. + +### 5) UI integration + +- Add a reusable sidebar open-in menu component used by `WorktreeContextMenu`. +- The context menu should show a single `Open in` item with nested choices for VS Code and OS file manager. + +Interaction constraints: + +- Disable while deleting (`isDeleting`). +- Preserve existing keyboard/focus behavior of menu primitives. + +### 6) Remote/SSH blocking policy + +Keep existing `isLocalPathOpenBlocked` gate in renderer for immediate UX and consistent toasts. + +Policy nuance: + +- Continue blocking by runtime/SSH context (existing behavior). +- Main process still validates path existence/shape for all requests. +- If later hardening is needed, add connection/runtime provenance in IPC args and enforce remote block in main too. + +## Edge Cases That Must Be Explicitly Handled + +- Worktree deleted/moved between render and click: return `not-found`. +- Multi-window stale data: each click is independently validated in main. +- Rapid repeated clicks: requests are independent; no Orca-side dedupe required. +- External FS mutation races: normalized absolute path + existence recheck per call. +- Host launcher missing/misconfigured: return `launch-failed`, show actionable toast. + +## Testing Requirements + +### `src/main/ipc/shell.test.ts` + +Add tests for both new handlers: + +- rejects relative path (`not-absolute`) +- rejects missing path (`not-found`) +- maps launcher error/failure (`launch-failed`) +- success path invokes expected Electron shell call + +Also add coverage for existing `shell:openFilePath`/`shell:openFileUri` failure branches if retained. + +### Renderer tests + +Add/adjust tests near sidebar components for: + +- submenu disabled state while deleting +- blocked-path toast path +- platform label mapping + +## Rollout Order + +1. Add new main IPC handlers + result union types in preload contract. +2. Add shared sidebar open-in menu component and wire into context menu. +3. Replace direct `openPath` usage for worktree actions with new handlers. +4. Add tests and run targeted test/typecheck. + +## Non-goals + +- Editor picker UI +- Per-worktree app preference +- Enabling local OS open for SSH/remote runtime worktrees diff --git a/src/main/ipc/shell.test.ts b/src/main/ipc/shell.test.ts index cd7ce56d8..5f034fb90 100644 --- a/src/main/ipc/shell.test.ts +++ b/src/main/ipc/shell.test.ts @@ -1,8 +1,25 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' +import { normalize, resolve } from 'node:path' +import { pathToFileURL } from 'node:url' -const { handleMock, showOpenDialogMock } = vi.hoisted(() => ({ +const { + getSpawnArgsForWindowsMock, + handleMock, + openPathMock, + resolveCliCommandMock, + showItemInFolderMock, + showOpenDialogMock, + spawnMock, + statMock +} = vi.hoisted(() => ({ + getSpawnArgsForWindowsMock: vi.fn(), handleMock: vi.fn(), - showOpenDialogMock: vi.fn() + openPathMock: vi.fn(), + resolveCliCommandMock: vi.fn(), + showItemInFolderMock: vi.fn(), + showOpenDialogMock: vi.fn(), + spawnMock: vi.fn(), + statMock: vi.fn() })) vi.mock('electron', () => ({ @@ -10,21 +27,71 @@ vi.mock('electron', () => ({ handle: handleMock }, shell: { - showItemInFolder: vi.fn(), + showItemInFolder: showItemInFolderMock, openExternal: vi.fn(), - openPath: vi.fn() + openPath: openPathMock }, dialog: { showOpenDialog: showOpenDialogMock } })) -import { registerShellHandlers } from './shell' +vi.mock('node:fs/promises', () => ({ + constants: { COPYFILE_EXCL: 1 }, + copyFile: vi.fn(), + stat: statMock +})) + +vi.mock('node:child_process', () => ({ + spawn: spawnMock +})) + +vi.mock('../codex-cli/command', () => ({ + resolveCliCommand: resolveCliCommandMock +})) + +vi.mock('../win32-utils', () => ({ + getSpawnArgsForWindows: getSpawnArgsForWindowsMock +})) + +import { EXTERNAL_EDITOR_CLI_COMMAND, registerShellHandlers } from './shell' + +function createSpawnedProcess(result: 'spawn' | 'error' = 'spawn'): { + once: ReturnType + unref: ReturnType +} { + const child = { + once: vi.fn((eventName: string, callback: (error?: Error) => void) => { + if (eventName === result) { + queueMicrotask(() => { + callback(result === 'error' ? new Error('launcher unavailable') : undefined) + }) + } + return child + }), + unref: vi.fn() + } + return child +} describe('registerShellHandlers', () => { beforeEach(() => { handleMock.mockReset() + getSpawnArgsForWindowsMock.mockReset() + openPathMock.mockReset() + resolveCliCommandMock.mockReset() + showItemInFolderMock.mockReset() showOpenDialogMock.mockReset() + spawnMock.mockReset() + statMock.mockReset() + openPathMock.mockResolvedValue('') + resolveCliCommandMock.mockReturnValue('editor-cli') + getSpawnArgsForWindowsMock.mockImplementation((command: string, args: string[]) => ({ + spawnCmd: command, + spawnArgs: args + })) + spawnMock.mockReturnValue(createSpawnedProcess()) + statMock.mockResolvedValue({ isDirectory: () => true }) }) function getHandler(channel: string): (event: unknown, args?: unknown) => Promise { @@ -59,4 +126,187 @@ describe('registerShellHandlers', () => { const handler = getHandler('shell:pickAudio') await expect(handler({})).resolves.toBeNull() }) + + describe('shell:openInFileManager', () => { + it('rejects relative paths', async () => { + const handler = getHandler('shell:openInFileManager') + + await expect(handler({}, 'relative/workspace')).resolves.toEqual({ + ok: false, + reason: 'not-absolute' + }) + expect(statMock).not.toHaveBeenCalled() + expect(showItemInFolderMock).not.toHaveBeenCalled() + }) + + it('rejects missing paths', async () => { + statMock.mockRejectedValueOnce(new Error('missing')) + const workspacePath = resolve('missing-workspace') + const handler = getHandler('shell:openInFileManager') + + await expect(handler({}, workspacePath)).resolves.toEqual({ + ok: false, + reason: 'not-found' + }) + expect(statMock).toHaveBeenCalledWith(normalize(workspacePath)) + expect(showItemInFolderMock).not.toHaveBeenCalled() + }) + + it('maps launcher errors to launch-failed', async () => { + showItemInFolderMock.mockImplementationOnce(() => { + throw new Error('launcher unavailable') + }) + const workspacePath = resolve('workspace') + const handler = getHandler('shell:openInFileManager') + + await expect(handler({}, workspacePath)).resolves.toEqual({ + ok: false, + reason: 'launch-failed' + }) + expect(showItemInFolderMock).toHaveBeenCalledWith(normalize(workspacePath)) + }) + + it('opens existing absolute paths in the OS file manager', async () => { + const workspacePath = resolve('workspace') + const handler = getHandler('shell:openInFileManager') + + await expect(handler({}, workspacePath)).resolves.toEqual({ ok: true }) + expect(showItemInFolderMock).toHaveBeenCalledWith(normalize(workspacePath)) + }) + }) + + describe('shell:openInExternalEditor', () => { + it('rejects relative paths', async () => { + const handler = getHandler('shell:openInExternalEditor') + + await expect(handler({}, 'relative/workspace')).resolves.toEqual({ + ok: false, + reason: 'not-absolute' + }) + expect(statMock).not.toHaveBeenCalled() + expect(openPathMock).not.toHaveBeenCalled() + expect(spawnMock).not.toHaveBeenCalled() + }) + + it('rejects missing paths', async () => { + statMock.mockRejectedValueOnce(new Error('missing')) + const workspacePath = resolve('missing-workspace') + const handler = getHandler('shell:openInExternalEditor') + + await expect(handler({}, workspacePath)).resolves.toEqual({ + ok: false, + reason: 'not-found' + }) + expect(statMock).toHaveBeenCalledWith(normalize(workspacePath)) + expect(openPathMock).not.toHaveBeenCalled() + expect(spawnMock).not.toHaveBeenCalled() + }) + + it('maps launcher failures to launch-failed', async () => { + spawnMock.mockReturnValueOnce(createSpawnedProcess('error')) + const workspacePath = resolve('workspace') + const handler = getHandler('shell:openInExternalEditor') + + await expect(handler({}, workspacePath)).resolves.toEqual({ + ok: false, + reason: 'launch-failed' + }) + expect(resolveCliCommandMock).toHaveBeenCalledWith(EXTERNAL_EDITOR_CLI_COMMAND) + expect(getSpawnArgsForWindowsMock).toHaveBeenCalledWith('editor-cli', [ + normalize(workspacePath) + ]) + expect(spawnMock).toHaveBeenCalledWith('editor-cli', [normalize(workspacePath)], { + detached: true, + stdio: 'ignore', + windowsHide: true + }) + expect(openPathMock).not.toHaveBeenCalled() + }) + + it('opens existing absolute paths with the editor launcher', async () => { + const workspacePath = resolve('workspace') + const handler = getHandler('shell:openInExternalEditor') + + await expect(handler({}, workspacePath)).resolves.toEqual({ ok: true }) + expect(resolveCliCommandMock).toHaveBeenCalledWith(EXTERNAL_EDITOR_CLI_COMMAND) + expect(getSpawnArgsForWindowsMock).toHaveBeenCalledWith('editor-cli', [ + normalize(workspacePath) + ]) + expect(spawnMock).toHaveBeenCalledWith('editor-cli', [normalize(workspacePath)], { + detached: true, + stdio: 'ignore', + windowsHide: true + }) + expect(openPathMock).not.toHaveBeenCalled() + }) + + it('uses platform-safe launcher command arguments', async () => { + getSpawnArgsForWindowsMock.mockReturnValueOnce({ + spawnCmd: 'platform-runner', + spawnArgs: ['platform-arg'] + }) + const workspacePath = resolve('workspace') + const handler = getHandler('shell:openInExternalEditor') + + await expect(handler({}, workspacePath)).resolves.toEqual({ ok: true }) + expect(getSpawnArgsForWindowsMock).toHaveBeenCalledWith('editor-cli', [ + normalize(workspacePath) + ]) + expect(spawnMock).toHaveBeenCalledWith('platform-runner', ['platform-arg'], { + detached: true, + stdio: 'ignore', + windowsHide: true + }) + expect(openPathMock).not.toHaveBeenCalled() + }) + }) + + describe('legacy file open handlers', () => { + it('does not open relative file paths', async () => { + const handler = getHandler('shell:openFilePath') + + await expect(handler({}, 'relative/file.md')).resolves.toBeUndefined() + expect(openPathMock).not.toHaveBeenCalled() + }) + + it('does not open missing file paths', async () => { + statMock.mockRejectedValueOnce(new Error('missing')) + const handler = getHandler('shell:openFilePath') + + await expect(handler({}, resolve('missing.md'))).resolves.toBeUndefined() + expect(openPathMock).not.toHaveBeenCalled() + }) + + it('swallows host launcher failures for file paths', async () => { + openPathMock.mockRejectedValueOnce(new Error('launcher unavailable')) + const filePath = resolve('note.md') + const handler = getHandler('shell:openFilePath') + + await expect(handler({}, filePath)).resolves.toBeUndefined() + expect(openPathMock).toHaveBeenCalledWith(normalize(filePath)) + }) + + it('does not open non-file URIs', async () => { + const handler = getHandler('shell:openFileUri') + + await expect(handler({}, 'https://example.com/file.md')).resolves.toBeUndefined() + expect(openPathMock).not.toHaveBeenCalled() + }) + + it('does not open remote file URIs', async () => { + const handler = getHandler('shell:openFileUri') + + await expect(handler({}, 'file://server/share/file.md')).resolves.toBeUndefined() + expect(openPathMock).not.toHaveBeenCalled() + }) + + it('swallows host launcher failures for file URIs', async () => { + openPathMock.mockRejectedValueOnce(new Error('launcher unavailable')) + const filePath = resolve('note.md') + const handler = getHandler('shell:openFileUri') + + await expect(handler({}, pathToFileURL(filePath).toString())).resolves.toBeUndefined() + expect(openPathMock).toHaveBeenCalledWith(normalize(filePath)) + }) + }) }) diff --git a/src/main/ipc/shell.ts b/src/main/ipc/shell.ts index 51dff5ae5..cb6fba2d9 100644 --- a/src/main/ipc/shell.ts +++ b/src/main/ipc/shell.ts @@ -1,7 +1,13 @@ import { ipcMain, shell, dialog } from 'electron' +import { spawn } from 'node:child_process' import { constants, copyFile, stat } from 'node:fs/promises' import { isAbsolute, normalize } from 'node:path' import { fileURLToPath } from 'node:url' +import type { ShellOpenLocalPathResult } from '../../shared/shell-open-types' +import { resolveCliCommand } from '../codex-cli/command' +import { getSpawnArgsForWindows } from '../win32-utils' + +export const EXTERNAL_EDITOR_CLI_COMMAND = 'code' async function pathExists(pathValue: string): Promise { try { @@ -12,11 +18,91 @@ async function pathExists(pathValue: string): Promise { } } +async function validateLocalPathTarget( + pathValue: string +): Promise<{ ok: true; path: string } | { ok: false; reason: 'not-absolute' | 'not-found' }> { + const normalizedPath = normalize(pathValue) + if (!isAbsolute(normalizedPath)) { + return { ok: false, reason: 'not-absolute' } + } + if (!(await pathExists(normalizedPath))) { + return { ok: false, reason: 'not-found' } + } + return { ok: true, path: normalizedPath } +} + +async function openInFileManager(pathValue: string): Promise { + const target = await validateLocalPathTarget(pathValue) + if (!target.ok) { + return target + } + try { + // Why: the file-manager action uses reveal semantics, matching the + // previous sidebar behavior while still validating the path per click. + shell.showItemInFolder(target.path) + return { ok: true } + } catch { + return { ok: false, reason: 'launch-failed' } + } +} + +async function launchExternalEditor(pathValue: string): Promise { + const editorCommand = resolveCliCommand(EXTERNAL_EDITOR_CLI_COMMAND) + const { spawnCmd, spawnArgs } = getSpawnArgsForWindows(editorCommand, [pathValue]) + + await new Promise((resolvePromise, rejectPromise) => { + const child = spawn(spawnCmd, spawnArgs, { + detached: true, + stdio: 'ignore', + windowsHide: true + }) + let settled = false + const settle = (callback: () => void): void => { + if (settled) { + return + } + settled = true + callback() + } + + child.once('error', (error) => { + settle(() => rejectPromise(error)) + }) + child.once('spawn', () => { + child.unref() + settle(resolvePromise) + }) + }) +} + +async function openInExternalEditor(pathValue: string): Promise { + const target = await validateLocalPathTarget(pathValue) + if (!target.ok) { + return target + } + try { + await launchExternalEditor(target.path) + return { ok: true } + } catch { + return { ok: false, reason: 'launch-failed' } + } +} + export function registerShellHandlers(): void { ipcMain.handle('shell:openPath', (_event, path: string) => { shell.showItemInFolder(path) }) + ipcMain.handle( + 'shell:openInFileManager', + (_event, path: string): Promise => openInFileManager(path) + ) + + ipcMain.handle( + 'shell:openInExternalEditor', + (_event, path: string): Promise => openInExternalEditor(path) + ) + ipcMain.handle('shell:openUrl', (_event, rawUrl: string) => { let parsed: URL try { @@ -33,14 +119,15 @@ export function registerShellHandlers(): void { }) ipcMain.handle('shell:openFilePath', async (_event, filePath: string) => { - if (!isAbsolute(filePath)) { + const target = await validateLocalPathTarget(filePath) + if (!target.ok) { return } - const normalizedPath = normalize(filePath) - if (!(await pathExists(normalizedPath))) { - return + try { + await shell.openPath(target.path) + } catch { + // Why: legacy file-open IPC is best-effort; callers already treat failure as a no-op. } - await shell.openPath(normalizedPath) }) ipcMain.handle('shell:openFileUri', async (_event, rawUri: string) => { @@ -67,15 +154,16 @@ export function registerShellHandlers(): void { return } - const normalizedPath = normalize(filePath) - if (!isAbsolute(normalizedPath)) { - return - } - if (!(await pathExists(normalizedPath))) { + const target = await validateLocalPathTarget(filePath) + if (!target.ok) { return } - await shell.openPath(normalizedPath) + try { + await shell.openPath(target.path) + } catch { + // Why: legacy file-open IPC is best-effort; callers already treat failure as a no-op. + } }) ipcMain.handle('shell:pathExists', async (_event, filePath: string): Promise => { diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index cecd0f29d..a874def4f 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -141,6 +141,9 @@ import type { E2EConfig } from '../shared/e2e-config' import type { AgentHookInstallStatus } from '../shared/agent-hook-types' import type { AgentStatusIpcPayload } from '../shared/agent-status-types' import type { RuntimeStatus, RuntimeSyncWindowGraph } from '../shared/runtime-types' +import type { ShellOpenLocalPathResult } from '../shared/shell-open-types' + +export type { ShellOpenLocalPathResult } from '../shared/shell-open-types' type RuntimeEnvironmentSubscriptionHandle = { unsubscribe: () => void @@ -992,6 +995,8 @@ export type PreloadApi = { } shell: { openPath: (path: string) => Promise + openInFileManager: (path: string) => Promise + openInExternalEditor: (path: string) => Promise openUrl: (url: string) => Promise openFilePath: (path: string) => Promise openFileUri: (uri: string) => Promise diff --git a/src/preload/index.ts b/src/preload/index.ts index d87d80f2e..014996c28 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -32,6 +32,7 @@ import type { WorktreeBaseStatusEvent, WorktreeRemoteBranchConflictEvent } from '../shared/types' +import type { ShellOpenLocalPathResult } from '../shared/shell-open-types' import type { RuntimeStatus, RuntimeSyncWindowGraph } from '../shared/runtime-types' import type { RuntimeRpcResponse } from '../shared/runtime-rpc-envelope' import type { PublicKnownRuntimeEnvironment } from '../shared/runtime-environments' @@ -1179,6 +1180,12 @@ const api = { shell: { openPath: (path: string): Promise => ipcRenderer.invoke('shell:openPath', path), + openInFileManager: (path: string): Promise => + ipcRenderer.invoke('shell:openInFileManager', path), + + openInExternalEditor: (path: string): Promise => + ipcRenderer.invoke('shell:openInExternalEditor', path), + openUrl: (url: string): Promise => ipcRenderer.invoke('shell:openUrl', url), openFilePath: (path: string): Promise => ipcRenderer.invoke('shell:openFilePath', path), diff --git a/src/renderer/src/components/sidebar/WorktreeCard.tsx b/src/renderer/src/components/sidebar/WorktreeCard.tsx index a444bf39e..8690617c2 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.tsx @@ -465,9 +465,9 @@ const WorktreeCard = React.memo(function WorktreeCard({ )} - {/* CI Checks & PR state on the right */} - {cardProps.includes('ci') && hostedReview && hostedReview.status !== 'neutral' && ( -
+
+ {/* CI Checks & PR state on the right */} + {cardProps.includes('ci') && hostedReview && hostedReview.status !== 'neutral' && ( @@ -486,8 +486,8 @@ const WorktreeCard = React.memo(function WorktreeCard({ CI checks {checksLabel(hostedReview.status).toLowerCase()} -
- )} + )} +
{/* Subtitle row: Repo badge + Branch */} diff --git a/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx b/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx index dd522a58a..d8eccb5c8 100644 --- a/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx +++ b/src/renderer/src/components/sidebar/WorktreeContextMenu.tsx @@ -13,7 +13,6 @@ import { } from '@/components/ui/dropdown-menu' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { - FolderOpen, Copy, Bell, BellOff, @@ -33,8 +32,8 @@ import type { Worktree } from '../../../../shared/types' import { isFolderRepo } from '../../../../shared/repo-kind' import { runWorktreeBatchDelete, runWorktreeDelete } from './delete-worktree-flow' import { runSleepWorktrees } from './sleep-worktree-flow' -import { isLocalPathOpenBlocked, showLocalPathOpenBlockedToast } from '@/lib/local-path-open-guard' import { getWorkspaceStatus, getWorkspaceStatusVisualMeta } from './workspace-status' +import { WorktreeOpenInSubMenu } from './WorktreeOpenInMenu' type Props = { worktree: Worktree @@ -118,18 +117,6 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({ return () => window.removeEventListener(CLOSE_ALL_CONTEXT_MENUS_EVENT, closeMenu) }, []) - const handleOpenInFinder = useCallback(() => { - if ( - isLocalPathOpenBlocked(useAppStore.getState().settings, { - connectionId: repo?.connectionId ?? null - }) - ) { - showLocalPathOpenBlockedToast() - return - } - window.api.shell.openPath(worktree.path) - }, [repo?.connectionId, worktree.path]) - const handleCopyPath = useCallback(() => { window.api.ui.writeClipboardText(worktree.path) }, [worktree.path]) @@ -282,10 +269,11 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({ {!isMultiContext && ( <> - - - Open in Finder - + Copy Path diff --git a/src/renderer/src/components/sidebar/WorktreeOpenInMenu.test.tsx b/src/renderer/src/components/sidebar/WorktreeOpenInMenu.test.tsx new file mode 100644 index 000000000..691899bbd --- /dev/null +++ b/src/renderer/src/components/sidebar/WorktreeOpenInMenu.test.tsx @@ -0,0 +1,151 @@ +import React from 'react' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { DropdownMenuSubContent, DropdownMenuSubTrigger } from '@/components/ui/dropdown-menu' +import { + getLocalFileManagerLabel, + openWorktreePath, + WorktreeOpenInSubMenu +} from './WorktreeOpenInMenu' + +type ReactElementLike = { + type: unknown + props: Record +} + +const { mockState, openInExternalEditorMock, openInFileManagerMock, toastErrorMock } = vi.hoisted( + () => ({ + mockState: { + settings: { activeRuntimeEnvironmentId: null as string | null } + }, + openInExternalEditorMock: vi.fn(), + openInFileManagerMock: vi.fn(), + toastErrorMock: vi.fn() + }) +) + +vi.mock('sonner', () => ({ + toast: { + error: toastErrorMock + } +})) + +vi.mock('@/store', () => { + const useAppStore = Object.assign( + (selector: (state: { settings: typeof mockState.settings }) => unknown) => + selector({ settings: mockState.settings }), + { + getState: () => ({ settings: mockState.settings }) + } + ) + return { useAppStore } +}) + +function visit(node: unknown, cb: (node: ReactElementLike) => void): void { + if (node == null || typeof node === 'string' || typeof node === 'number') { + return + } + if (Array.isArray(node)) { + node.forEach((entry) => visit(entry, cb)) + return + } + const element = node as ReactElementLike + cb(element) + if (element.props?.children) { + visit(element.props.children, cb) + } +} + +function findByType(node: unknown, type: unknown): ReactElementLike { + let found: ReactElementLike | null = null + visit(node, (entry) => { + if (entry.type === type) { + found = entry + } + }) + if (!found) { + throw new Error('element not found') + } + return found +} + +describe('WorktreeOpenInMenu', () => { + beforeEach(() => { + mockState.settings = { activeRuntimeEnvironmentId: null } + toastErrorMock.mockReset() + openInFileManagerMock.mockReset() + openInExternalEditorMock.mockReset() + openInFileManagerMock.mockResolvedValue({ ok: true }) + openInExternalEditorMock.mockResolvedValue({ ok: true }) + Object.defineProperty(globalThis, 'window', { + configurable: true, + value: { + api: { + shell: { + openInFileManager: openInFileManagerMock, + openInExternalEditor: openInExternalEditorMock + } + } + } + }) + }) + + it('maps file manager labels by platform', () => { + expect(getLocalFileManagerLabel('Mozilla/5.0 Mac OS X')).toBe('Finder') + expect(getLocalFileManagerLabel('Mozilla/5.0 Windows NT 10.0')).toBe('File Explorer') + expect(getLocalFileManagerLabel('Mozilla/5.0 X11 Linux x86_64')).toBe('File Manager') + }) + + it('disables the Open in submenu while deleting', () => { + const tree = WorktreeOpenInSubMenu({ + worktreePath: '/tmp/workspace', + connectionId: null, + disabled: true + }) + + expect(findByType(tree, DropdownMenuSubTrigger).props.disabled).toBe(true) + }) + + it('stops menu item click propagation', () => { + const tree = WorktreeOpenInSubMenu({ + worktreePath: '/tmp/workspace', + connectionId: null + }) + const menuContent = findByType(tree, DropdownMenuSubContent) + + const stopPropagation = vi.fn() + const handler = menuContent.props.onClick as ((event: React.SyntheticEvent) => void) | null + handler?.({ stopPropagation } as unknown as React.SyntheticEvent) + expect(stopPropagation).toHaveBeenCalled() + }) + + it('uses the blocked-path toast without calling main IPC', async () => { + mockState.settings = { activeRuntimeEnvironmentId: 'runtime-1' } + + await openWorktreePath({ + target: 'file-manager', + worktreePath: '/tmp/workspace', + connectionId: null + }) + + expect(toastErrorMock).toHaveBeenCalledWith( + 'Opening remote paths in the local OS is not available.' + ) + expect(openInFileManagerMock).not.toHaveBeenCalled() + expect(openInExternalEditorMock).not.toHaveBeenCalled() + }) + + it('shows an actionable toast when the host launcher fails', async () => { + openInExternalEditorMock.mockResolvedValueOnce({ ok: false, reason: 'launch-failed' }) + + await openWorktreePath({ + target: 'external-editor', + worktreePath: '/tmp/workspace', + connectionId: null + }) + + expect(openInExternalEditorMock).toHaveBeenCalledWith('/tmp/workspace') + expect(toastErrorMock).toHaveBeenCalledWith('Could not open workspace folder.', { + description: 'Check the editor command or file manager configuration on this machine.' + }) + }) +}) diff --git a/src/renderer/src/components/sidebar/WorktreeOpenInMenu.tsx b/src/renderer/src/components/sidebar/WorktreeOpenInMenu.tsx new file mode 100644 index 000000000..5624984dc --- /dev/null +++ b/src/renderer/src/components/sidebar/WorktreeOpenInMenu.tsx @@ -0,0 +1,145 @@ +import React, { useCallback } from 'react' +import { ExternalLink, FolderOpen } from 'lucide-react' +import { toast } from 'sonner' +import { + DropdownMenuItem, + DropdownMenuSub, + DropdownMenuSubContent, + DropdownMenuSubTrigger +} from '@/components/ui/dropdown-menu' +import { useAppStore } from '@/store' +import { isLocalPathOpenBlocked, showLocalPathOpenBlockedToast } from '@/lib/local-path-open-guard' +import type { ShellOpenLocalPathFailureReason } from '../../../../shared/shell-open-types' + +type WorktreeOpenInMenuItemsProps = { + worktreePath: string + connectionId?: string | null + disabled?: boolean +} + +export function getLocalFileManagerLabel(userAgent?: string): string { + const resolvedUserAgent = + userAgent ?? (typeof navigator === 'undefined' ? '' : navigator.userAgent) + if (resolvedUserAgent.includes('Mac')) { + return 'Finder' + } + if (resolvedUserAgent.includes('Windows')) { + return 'File Explorer' + } + return 'File Manager' +} + +function showOpenFailureToast(reason: ShellOpenLocalPathFailureReason): void { + if (reason === 'not-absolute') { + toast.error('Workspace path is not a valid local path.') + return + } + if (reason === 'not-found') { + toast.error('Workspace folder was not found.', { + description: 'It may have been moved or deleted. Refresh workspaces or remove it from Orca.' + }) + return + } + toast.error('Could not open workspace folder.', { + description: 'Check the editor command or file manager configuration on this machine.' + }) +} + +function stopMenuPropagation(event: React.SyntheticEvent): void { + event.stopPropagation() +} + +export async function openWorktreePath(args: { + target: 'file-manager' | 'external-editor' + worktreePath: string + connectionId?: string | null +}): Promise { + if ( + isLocalPathOpenBlocked(useAppStore.getState().settings, { + connectionId: args.connectionId ?? null + }) + ) { + showLocalPathOpenBlockedToast() + return + } + + const result = + args.target === 'file-manager' + ? await window.api.shell.openInFileManager(args.worktreePath) + : await window.api.shell.openInExternalEditor(args.worktreePath) + if (!result.ok) { + showOpenFailureToast(result.reason) + } +} + +function useOpenInWorktreePath({ + worktreePath, + connectionId +}: WorktreeOpenInMenuItemsProps): (target: 'file-manager' | 'external-editor') => Promise { + return useCallback( + async (target) => { + await openWorktreePath({ target, worktreePath, connectionId }) + }, + [connectionId, worktreePath] + ) +} + +export function WorktreeOpenInMenuItems({ + worktreePath, + connectionId, + disabled +}: WorktreeOpenInMenuItemsProps): React.JSX.Element { + const openInWorktreePath = useOpenInWorktreePath({ worktreePath, connectionId }) + const fileManagerLabel = getLocalFileManagerLabel() + + return ( + <> + { + void openInWorktreePath('external-editor') + }} + disabled={disabled} + > + + VS Code + + { + void openInWorktreePath('file-manager') + }} + disabled={disabled} + > + + {fileManagerLabel} + + + ) +} + +export function WorktreeOpenInSubMenu({ + worktreePath, + connectionId, + disabled +}: WorktreeOpenInMenuItemsProps): React.JSX.Element { + return ( + + + + Open in + + + + + + ) +} diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index b8bb0242a..3d88e85d7 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -908,9 +908,12 @@ function createUpdaterApi(): NonNullable['updater']> { } function createShellApi(): NonNullable['shell']> { + const openResult = { ok: true } as const return { openPath: (path) => Promise.resolve(window.open(path, '_blank', 'noopener,noreferrer') as never), + openInFileManager: () => Promise.resolve(openResult), + openInExternalEditor: () => Promise.resolve(openResult), openUrl: (url) => Promise.resolve(window.open(url, '_blank', 'noopener,noreferrer') as never), openFilePath: () => Promise.resolve(), openFileUri: (uri) => diff --git a/src/shared/shell-open-types.ts b/src/shared/shell-open-types.ts new file mode 100644 index 000000000..a2d477e9c --- /dev/null +++ b/src/shared/shell-open-types.ts @@ -0,0 +1,5 @@ +export type ShellOpenLocalPathFailureReason = 'not-absolute' | 'not-found' | 'launch-failed' + +export type ShellOpenLocalPathResult = + | { ok: true } + | { ok: false; reason: ShellOpenLocalPathFailureReason }