diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index 7b4f699cd62..509c7243eca 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -6935,6 +6935,169 @@ describe('registerWorktreeHandlers', () => { expect(provider.removeWorktree).toHaveBeenCalledWith('/remote/feature-wt', undefined) }) + it('uses the workspace host when duplicate repo ids exist across local and SSH', async () => { + const localRepo = { + id: 'repo-shared', + path: '/local/repo', + displayName: 'local', + badgeColor: '#000', + addedAt: 0, + worktreeBaseRef: null + } + const sshRepo = { + ...localRepo, + path: '/remote/repo', + displayName: 'ssh', + connectionId: 'conn-1' + } + const provider = { + listWorktrees: vi.fn().mockResolvedValue([ + { + path: sshRepo.path, + head: 'main', + branch: 'main', + isBare: false, + isMainWorktree: true + }, + { + path: '/remote/feature-wt', + head: 'feature', + branch: 'feature', + isBare: false, + isMainWorktree: false + } + ]), + removeWorktree: vi.fn().mockResolvedValue(undefined), + worktreeIsClean: vi.fn().mockResolvedValue({ clean: true }) + } + store.getRepo.mockReturnValue(localRepo) + store.getRepos.mockReturnValue([localRepo, sshRepo]) + getSshGitProviderMock.mockReturnValue(provider) + + await handlers['worktrees:remove'](null, { + worktreeId: 'repo-shared::/remote/feature-wt', + hostId: 'ssh:conn-1' + }) + + expect(provider.removeWorktree).toHaveBeenCalledWith('/remote/feature-wt', undefined) + expect(removeWorktreeMock).not.toHaveBeenCalled() + }) + + it('fails closed when duplicate repo ids are deleted without a host', async () => { + const localRepo = { + id: 'repo-shared', + path: '/local/repo', + displayName: 'local', + badgeColor: '#000', + addedAt: 0 + } + const sshRepo = { ...localRepo, path: '/remote/repo', connectionId: 'conn-1' } + store.getRepos.mockReturnValue([localRepo, sshRepo]) + + await expect( + handlers['worktrees:remove'](null, { + worktreeId: 'repo-shared::/remote/feature-wt' + }) + ).rejects.toThrow('Repo not found: repo-shared') + + expect(removeWorktreeMock).not.toHaveBeenCalled() + expect(getSshGitProviderMock).not.toHaveBeenCalled() + }) + + it('inspects hooks on the requested host when repo ids collide', async () => { + const localRepo = { + id: 'repo-shared', + path: '/local/repo', + displayName: 'local', + badgeColor: '#000', + addedAt: 0 + } + const sshRepo = { ...localRepo, path: '/remote/repo', connectionId: 'conn-1' } + const fsProvider = { + readFile: vi.fn().mockResolvedValue({ + content: 'scripts:\n archive: remote-cleanup', + isBinary: false + }) + } + store.getRepos.mockReturnValue([localRepo, sshRepo]) + getSshFilesystemProviderMock.mockReturnValue(fsProvider) + parseOrcaYamlMock.mockReturnValue({ scripts: { archive: 'remote-cleanup' } }) + + await expect( + handlers['hooks:check'](null, { + repoId: 'repo-shared', + hostId: 'ssh:conn-1' + }) + ).resolves.toEqual({ + status: 'ok', + hasHooks: true, + hooks: { scripts: { archive: 'remote-cleanup' } }, + mayNeedUpdate: false + }) + expect(fsProvider.readFile).toHaveBeenCalledWith('/remote/repo/orca.yaml') + expect(hasHooksFileMock).not.toHaveBeenCalled() + }) + + it('fails hook inspection closed when duplicate repo ids omit the host', async () => { + const localRepo = { + id: 'repo-shared', + path: '/local/repo', + displayName: 'local', + badgeColor: '#000', + addedAt: 0 + } + const sshRepo = { ...localRepo, path: '/remote/repo', connectionId: 'conn-1' } + store.getRepos.mockReturnValue([localRepo, sshRepo]) + + await expect(handlers['hooks:check'](null, { repoId: 'repo-shared' })).resolves.toEqual({ + status: 'error', + hasHooks: false, + hooks: null, + mayNeedUpdate: false + }) + expect(getSshFilesystemProviderMock).not.toHaveBeenCalled() + expect(hasHooksFileMock).not.toHaveBeenCalled() + }) + + it('does not coalesce forget requests for the same id on different hosts', async () => { + const localRepo = { + id: 'repo-shared', + path: '/local/repo', + displayName: 'local', + badgeColor: '#000', + addedAt: 0 + } + const sshRepo = { ...localRepo, path: '/remote/repo', connectionId: 'conn-1' } + store.getRepos.mockReturnValue([localRepo, sshRepo]) + let finishFirst!: () => void + killAllProcessesForWorktreeMock + .mockImplementationOnce( + () => + new Promise((resolve) => { + finishFirst = () => + resolve({ runtimeStopped: 0, providerStopped: 0, registryStopped: 0 }) + }) + ) + .mockResolvedValueOnce({ runtimeStopped: 0, providerStopped: 0, registryStopped: 0 }) + + const first = handlers['worktrees:forgetLocal'](null, { + worktreeId: 'repo-shared::/same/path', + hostId: 'local' + }) as Promise + await vi.waitFor(() => expect(killAllProcessesForWorktreeMock).toHaveBeenCalledTimes(1)) + + await expect( + handlers['worktrees:forgetLocal'](null, { + worktreeId: 'repo-shared::/same/path', + hostId: 'ssh:conn-1' + }) + ).resolves.toEqual({}) + expect(killAllProcessesForWorktreeMock).toHaveBeenCalledTimes(2) + + finishFirst() + await expect(first).resolves.toEqual({}) + }) + it('preserves the branch on remove for worktrees created from an existing local branch', async () => { mockKnownFeatureWorktree() removeWorktreeMock.mockResolvedValue(undefined) @@ -7257,14 +7420,16 @@ describe('registerWorktreeHandlers', () => { await mkdir(adminWorktreePath, { recursive: true }) await writeFile(join(orphanPath, '.git'), `gitdir: ${adminWorktreePath}\n`) await writeFile(join(adminWorktreePath, 'gitdir'), `${join(orphanPath, '.git')}\n`) - store.getRepo.mockReturnValue({ + const repo = { id: 'repo-1', path: repoPath, displayName: 'repo', badgeColor: '#000', addedAt: 0, worktreeBaseRef: null - }) + } + store.getRepo.mockReturnValue(repo) + store.getRepos.mockReturnValue([repo]) mockKnownFeatureWorktree(join(parentDir, 'real-feature'), repoPath) store.getWorktreeMeta.mockReturnValue(makeWorktreeMeta({ createdAt: Date.now() })) @@ -7298,14 +7463,16 @@ describe('registerWorktreeHandlers', () => { await mkdir(adminWorktreePath, { recursive: true }) await writeFile(join(orphanPath, '.git'), `gitdir: ${adminWorktreePath}\n`) await writeFile(join(adminWorktreePath, 'gitdir'), `${join(orphanPath, '.git')}\n`) - store.getRepo.mockReturnValue({ + const repo = { id: 'repo-1', path: repoPath, displayName: 'repo', badgeColor: '#000', addedAt: 0, worktreeBaseRef: null - }) + } + store.getRepo.mockReturnValue(repo) + store.getRepos.mockReturnValue([repo]) mockKnownFeatureWorktree(join(parentDir, 'real-feature'), repoPath) store.getWorktreeMeta.mockReturnValue( makeWorktreeMeta({ orcaCreatedAt: Date.now(), orcaCreationSource: 'runtime' }) @@ -7527,6 +7694,7 @@ describe('registerWorktreeHandlers', () => { }) as Promise const second = handlers['worktrees:remove'](null, { worktreeId: 'repo-1::/workspace/feature-wt', + hostId: 'local', force: true }) as Promise @@ -7564,6 +7732,7 @@ describe('registerWorktreeHandlers', () => { await expect( handlers['worktrees:remove'](null, { worktreeId: 'repo-1::/workspace/feature-wt', + hostId: 'local', force: true }) ).rejects.toThrow('Worktree deletion already in progress') diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index d3ed835646a..8f4e64e4207 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -29,6 +29,7 @@ import type { Worktree, WorktreeMeta } from '../../shared/types' +import { getRepoExecutionHostId, type ExecutionHostId } from '../../shared/execution-host' import { buildKnownOrcaWorkspaceLayouts, isLegacyRepoForExternalWorktreeVisibility, @@ -101,6 +102,35 @@ import { type CreateWorktreeArgsWithSystemProvenance = CreateWorktreeArgs & { automationProvenance?: AutomationWorkspaceProvenance } + +type RemoveWorktreeArgs = { + worktreeId: string + hostId?: ExecutionHostId + force?: boolean + skipArchive?: boolean +} + +function getRepoForWorktreeRemoval( + store: Store, + repoId: string, + hostId?: ExecutionHostId +): Repo | undefined { + const matches = store + .getRepos() + .filter((repo) => repo.id === repoId && (!hostId || getRepoExecutionHostId(repo) === hostId)) + // Why: deletion must never guess between host owners. Legacy unscoped calls + // remain compatible only while the repo id still has one unique owner. + if (matches.length === 1) { + return matches[0] + } + if (matches.length > 1) { + return undefined + } + const legacyMatch = store.getRepo(repoId) + return legacyMatch && (!hostId || getRepoExecutionHostId(legacyMatch) === hostId) + ? legacyMatch + : undefined +} import { classifyWorkspaceCreateError } from './workspace-create-error-classifier' import { advertisedUrlWatcher } from '../ports/advertised-url-watcher' import { localhostWorktreeLabelProxy } from '../localhost-worktree-label-proxy' @@ -300,6 +330,10 @@ function getWorktreeRemovalOptionsKey(args: { force?: boolean; skipArchive?: boo return `${forceKey}:${archiveKey}` } +function getWorktreeRemovalInFlightKey(worktreeId: string, hostId?: ExecutionHostId): string { + return `${hostId ?? ''}\0${worktreeId}` +} + async function getArchiveHooksForRemoval(repo: Repo): Promise { if (!repo.connectionId) { return getEffectiveHooks(repo) @@ -1345,9 +1379,18 @@ export function registerWorktreeHandlers( ipcMain.handle( 'worktrees:remove', - async (_event, args: { worktreeId: string; force?: boolean; skipArchive?: boolean }) => { + async (_event, args: RemoveWorktreeArgs): Promise => { + const { repoId, worktreePath } = parseWorktreeId(args.worktreeId) + const repo = getRepoForWorktreeRemoval(store, repoId, args.hostId) + if (!repo) { + throw new Error(`Repo not found: ${repoId}`) + } + const inFlightKey = getWorktreeRemovalInFlightKey( + args.worktreeId, + getRepoExecutionHostId(repo) + ) const optionsKey = getWorktreeRemovalOptionsKey(args) - const inFlightRemoval = worktreeRemovalsInFlight.get(args.worktreeId) + const inFlightRemoval = worktreeRemovalsInFlight.get(inFlightKey) if (inFlightRemoval) { if (inFlightRemoval.optionsKey === optionsKey) { return inFlightRemoval.promise @@ -1359,11 +1402,6 @@ export function registerWorktreeHandlers( // target the same worktree concurrently. Share the destructive backend // operation so only one path touches Git and the filesystem. const removal = (async (): Promise => { - const { repoId, worktreePath } = parseWorktreeId(args.worktreeId) - const repo = store.getRepo(repoId) - if (!repo) { - throw new Error(`Repo not found: ${repoId}`) - } if (isFolderRepo(repo)) { if (args.worktreeId === getFolderWorkspaceRootId(repo)) { throw new Error( @@ -1797,12 +1835,12 @@ export function registerWorktreeHandlers( notifyWorktreesChanged(mainWindow, repoId) return removalResult ?? {} })() - worktreeRemovalsInFlight.set(args.worktreeId, { optionsKey, promise: removal }) + worktreeRemovalsInFlight.set(inFlightKey, { optionsKey, promise: removal }) try { return await removal } finally { - if (worktreeRemovalsInFlight.get(args.worktreeId)?.promise === removal) { - worktreeRemovalsInFlight.delete(args.worktreeId) + if (worktreeRemovalsInFlight.get(inFlightKey)?.promise === removal) { + worktreeRemovalsInFlight.delete(inFlightKey) } } } @@ -1816,12 +1854,24 @@ export function registerWorktreeHandlers( // no branches, no files are deleted there. ipcMain.handle( 'worktrees:forgetLocal', - async (_event, args: { worktreeId: string }): Promise => { + async ( + _event, + args: Pick + ): Promise => { + const { repoId } = parseWorktreeId(args.worktreeId) + const repo = getRepoForWorktreeRemoval(store, repoId, args.hostId) + if (!repo) { + throw new Error(`Repo not found: ${repoId}`) + } // Why: share the removal in-flight map (not a separate one) so a concurrent // worktrees:remove and worktrees:forgetLocal on the same id cannot both // mutate metadata. A forget takes no force/skipArchive options. + const inFlightKey = getWorktreeRemovalInFlightKey( + args.worktreeId, + getRepoExecutionHostId(repo) + ) const optionsKey = 'forget-local' - const inFlight = worktreeRemovalsInFlight.get(args.worktreeId) + const inFlight = worktreeRemovalsInFlight.get(inFlightKey) if (inFlight) { if (inFlight.optionsKey === optionsKey) { return inFlight.promise @@ -1830,11 +1880,6 @@ export function registerWorktreeHandlers( } const forget = (async (): Promise => { - const { repoId } = parseWorktreeId(args.worktreeId) - const repo = store.getRepo(repoId) - if (!repo) { - throw new Error(`Repo not found: ${repoId}`) - } if (isFolderRepo(repo) && args.worktreeId === getFolderWorkspaceRootId(repo)) { throw new Error( 'Cannot delete the project root workspace. Remove the folder project instead.' @@ -1858,12 +1903,12 @@ export function registerWorktreeHandlers( notifyWorktreesChanged(mainWindow, repoId) return {} })() - worktreeRemovalsInFlight.set(args.worktreeId, { optionsKey, promise: forget }) + worktreeRemovalsInFlight.set(inFlightKey, { optionsKey, promise: forget }) try { return await forget } finally { - if (worktreeRemovalsInFlight.get(args.worktreeId)?.promise === forget) { - worktreeRemovalsInFlight.delete(args.worktreeId) + if (worktreeRemovalsInFlight.get(inFlightKey)?.promise === forget) { + worktreeRemovalsInFlight.delete(inFlightKey) } } } @@ -1996,49 +2041,63 @@ export function registerWorktreeHandlers( } }) - ipcMain.handle('hooks:check', async (_event, args: { repoId: string }) => { - const repo = store.getRepo(args.repoId) - if (!repo || isFolderRepo(repo)) { - return { status: 'ok', hasHooks: false, hooks: null, mayNeedUpdate: false } - } - - if (repo.connectionId) { - const fsProvider = getSshFilesystemProvider(repo.connectionId) - if (!fsProvider) { - return { status: 'error', hasHooks: false, hooks: null, mayNeedUpdate: false } - } - try { - const result = await fsProvider.readFile(joinWorktreeRelativePath(repo.path, 'orca.yaml')) + ipcMain.handle( + 'hooks:check', + async (_event, args: { repoId: string; hostId?: ExecutionHostId }) => { + const repo = getRepoForWorktreeRemoval(store, args.repoId, args.hostId) + if (!repo) { + const repoIdExists = store.getRepos().some((candidate) => candidate.id === args.repoId) + // Why: a requested or ambiguous host must not be reported as hook-free; + // callers treat inspection errors as "skip", which keeps hook execution fail closed. return { - status: 'ok', - hasHooks: !result.isBinary, - hooks: result.isBinary ? null : parseOrcaYaml(result.content), - mayNeedUpdate: false - } - } catch (error) { - return { - status: isENOENT(error) ? 'ok' : 'error', + status: args.hostId || repoIdExists ? 'error' : 'ok', hasHooks: false, hooks: null, mayNeedUpdate: false } } - } + if (isFolderRepo(repo)) { + return { status: 'ok', hasHooks: false, hooks: null, mayNeedUpdate: false } + } - const has = hasHooksFile(repo.path) - const hooks = has ? loadHooks(repo.path) : null - // Why: when a newer Orca version adds a top-level key to `orca.yaml`, older - // versions that don't recognise it return null and show "could not be parsed". - // Detecting well-formed but unrecognised keys lets the UI suggest updating - // instead of implying the file is broken. - const mayNeedUpdate = has && !hooks && hasUnrecognizedOrcaYamlKeys(repo.path) - return { - status: 'ok', - hasHooks: has, - hooks, - mayNeedUpdate + if (repo.connectionId) { + const fsProvider = getSshFilesystemProvider(repo.connectionId) + if (!fsProvider) { + return { status: 'error', hasHooks: false, hooks: null, mayNeedUpdate: false } + } + try { + const result = await fsProvider.readFile(joinWorktreeRelativePath(repo.path, 'orca.yaml')) + return { + status: 'ok', + hasHooks: !result.isBinary, + hooks: result.isBinary ? null : parseOrcaYaml(result.content), + mayNeedUpdate: false + } + } catch (error) { + return { + status: isENOENT(error) ? 'ok' : 'error', + hasHooks: false, + hooks: null, + mayNeedUpdate: false + } + } + } + + const has = hasHooksFile(repo.path) + const hooks = has ? loadHooks(repo.path) : null + // Why: when a newer Orca version adds a top-level key to `orca.yaml`, older + // versions that don't recognise it return null and show "could not be parsed". + // Detecting well-formed but unrecognised keys lets the UI suggest updating + // instead of implying the file is broken. + const mayNeedUpdate = has && !hooks && hasUnrecognizedOrcaYamlKeys(repo.path) + return { + status: 'ok', + hasHooks: has, + hooks, + mayNeedUpdate + } } - }) + ) ipcMain.handle( 'hooks:createIssueCommandRunner', diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 100b89b97f4..e278d863e21 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -1130,12 +1130,16 @@ export type PreloadApi = { > remove: (args: { worktreeId: string + hostId?: ExecutionHostId force?: boolean skipArchive?: boolean }) => Promise // Forget a workspace from Orca only — no remote Git/filesystem work. Used // for workspaces pinned to a removed/disconnected SSH host. - forgetLocal: (args: { worktreeId: string }) => Promise + forgetLocal: (args: { + worktreeId: string + hostId?: ExecutionHostId + }) => Promise forceDeletePreservedBranch: (args: { worktreeId: string branchName: string @@ -2131,7 +2135,7 @@ export type PreloadApi = { browser: BrowserApi emulator: EmulatorApi hooks: { - check: (args: { repoId: string }) => Promise<{ + check: (args: { repoId: string; hostId?: ExecutionHostId }) => Promise<{ status?: 'ok' | 'error' hasHooks: boolean hooks: OrcaHooks | null diff --git a/src/preload/index.ts b/src/preload/index.ts index 9f63a71d985..8f24dc18217 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -142,6 +142,7 @@ import type { TelemetryConsentState } from '../shared/telemetry-consent-types' import type { PreflightRuntimeContext, RefreshAgentsResult } from './api-types' import type { AgentKind, LaunchSource, RequestKind } from '../shared/telemetry-events' import type { AppStarSource } from '../shared/gh-star-source' +import type { ExecutionHostId } from '../shared/execution-host' import type { Automation, AutomationCreateInput, @@ -2478,6 +2479,7 @@ const api = { hooks: { check: (args: { repoId: string + hostId?: ExecutionHostId }): Promise<{ status?: 'ok' | 'error' hasHooks: boolean diff --git a/src/renderer/src/components/sidebar/delete-worktree-flow.ts b/src/renderer/src/components/sidebar/delete-worktree-flow.ts index cac60ca3727..02ff447dfc1 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-flow.ts +++ b/src/renderer/src/components/sidebar/delete-worktree-flow.ts @@ -1,6 +1,7 @@ import { toast } from 'sonner' import { useAppStore } from '@/store' import { getWorktreeMapFromState } from '@/store/selectors' +import { findRepoForHost } from '@/store/slices/repo-host-identity' import { activateAndRevealWorktree } from '@/lib/worktree-activation' import { prepareActiveWorktreeFocusAfterDelete } from './active-worktree-focus-after-delete' import { showDeleteWorktreeFailureToast } from './delete-worktree-failure-toast' @@ -262,7 +263,12 @@ export function runWorktreeDelete(worktreeId: string): void { // misclassify every SSH repo as a ghost, routing to a forget dialog whose // local-only backend is unavailable there. Their normal worktree.rm RPC path // already handles the delete against the desktop runtime. - const repo = state.repos.find((entry) => entry.id === target.repoId) ?? null + const matchingRepos = state.repos.filter((entry) => entry.id === target.repoId) + const repo = target.hostId + ? findRepoForHost(matchingRepos, target.repoId, { hostId: target.hostId }) + : matchingRepos.length === 1 + ? matchingRepos[0] + : null const sshResolution = isPairedWebClientWindow() ? { kind: 'not-ssh' as const } : resolveSshWorkspaceForget({ diff --git a/src/renderer/src/lib/ensure-hooks-confirmed.test.ts b/src/renderer/src/lib/ensure-hooks-confirmed.test.ts index e7367b0063a..da3d2028ab2 100644 --- a/src/renderer/src/lib/ensure-hooks-confirmed.test.ts +++ b/src/renderer/src/lib/ensure-hooks-confirmed.test.ts @@ -225,6 +225,28 @@ describe('ensureHooksConfirmed', () => { expect(pending).toHaveLength(0) }) + it('inspects the requested host when duplicate repo ids exist', async () => { + const { state } = createTestState({ + settings: { activeRuntimeEnvironmentId: 'env-1' }, + trustedOrcaHooks: { 'repo-1': { all: { approvedAt: 1 } } }, + repos: [ + { id: 'repo-1', displayName: 'Runtime', executionHostId: 'runtime:env-1' }, + { id: 'repo-1', displayName: 'SSH', connectionId: 'ssh-1' } + ] + } as unknown as Partial) + hooksCheckMock.mockResolvedValue({ + hasHooks: true, + hooks: { scripts: {} }, + mayNeedUpdate: false + }) + + const decision = await ensureHooksConfirmed(state, 'repo-1', 'archive', 'ssh:ssh-1') + + expect(decision).toBe('run') + expect(hooksCheckMock).toHaveBeenCalledWith({ repoId: 'repo-1', hostId: 'ssh:ssh-1' }) + expect(runtimeEnvironmentCallMock).not.toHaveBeenCalled() + }) + it('checks runtime-owned repo hooks through the repo owner runtime', async () => { const { state, pending } = createTestState({ settings: { activeRuntimeEnvironmentId: 'focused-env' }, diff --git a/src/renderer/src/lib/ensure-hooks-confirmed.ts b/src/renderer/src/lib/ensure-hooks-confirmed.ts index 856b45378bc..1ad0b7a7a77 100644 --- a/src/renderer/src/lib/ensure-hooks-confirmed.ts +++ b/src/renderer/src/lib/ensure-hooks-confirmed.ts @@ -4,6 +4,11 @@ import { resolveHookCommandSourcePolicy } from '../../../shared/hook-command-sou import { hashOrcaHookScript, type OrcaHookScriptKind } from './orca-hook-trust' import { checkRuntimeHooks, readRuntimeIssueCommand } from '@/runtime/runtime-hooks-client' import { getRuntimeEnvironmentIdForRepo } from './repo-runtime-owner' +import { + getRepoExecutionHostId, + parseExecutionHostId, + type ExecutionHostId +} from '../../../shared/execution-host' export type HookScriptKind = OrcaHookScriptKind @@ -56,8 +61,23 @@ function getVmRecipeTrustContent(yamlHooks: OrcaHooks | null): string { .join('\n\n') } -function settingsForHookRepoOwner(state: AppState, repoId: string): AppState['settings'] { - const runtimeEnvironmentId = getRuntimeEnvironmentIdForRepo(state, repoId) +function findHookRepo(state: AppState, repoId: string, hostId?: ExecutionHostId) { + return hostId + ? state.repos.find((repo) => repo.id === repoId && getRepoExecutionHostId(repo) === hostId) + : state.repos.find((repo) => repo.id === repoId) +} + +function settingsForHookRepoOwner( + state: AppState, + repoId: string, + hostId?: ExecutionHostId +): AppState['settings'] { + const parsedHost = hostId ? parseExecutionHostId(hostId) : null + const runtimeEnvironmentId = hostId + ? parsedHost?.kind === 'runtime' + ? parsedHost.environmentId + : null + : getRuntimeEnvironmentIdForRepo(state, repoId) // Why: hook inspection must follow the repo owner. SSH/local repos execute // through desktop IPC, while runtime repos may differ from the focused host. return state.settings @@ -68,10 +88,12 @@ function settingsForHookRepoOwner(state: AppState, repoId: string): AppState['se export async function ensureHooksConfirmed( state: AppState, repoId: string, - scriptKind: HookScriptKind + scriptKind: HookScriptKind, + hostId?: ExecutionHostId ): Promise<'run' | 'skip'> { return enqueueTrustPrompt(async () => { - if (state.trustedOrcaHooks[repoId]?.all) { + const hasDuplicateRepoId = state.repos.filter((repo) => repo.id === repoId).length > 1 + if (state.trustedOrcaHooks[repoId]?.all && !(hostId && hasDuplicateRepoId)) { return 'run' } @@ -80,7 +102,7 @@ export async function ensureHooksConfirmed( if (scriptKind === 'issueCommand') { // Local overrides are user-owned; only shared orca.yaml commands need repo trust. const result = await readRuntimeIssueCommand( - settingsForHookRepoOwner(state, repoId), + settingsForHookRepoOwner(state, repoId, hostId), repoId ) if (result.source === 'local') { @@ -94,7 +116,7 @@ export async function ensureHooksConfirmed( } scriptContent = (result.sharedContent ?? '').trim() } else { - const repo = state.repos.find((r) => r.id === repoId) + const repo = findHookRepo(state, repoId, hostId) const localScript = repo?.hookSettings?.scripts?.[scriptKind]?.trim() const sourcePolicy = resolveHookCommandSourcePolicy( repo?.hookSettings?.commandSourcePolicy, @@ -105,7 +127,11 @@ export async function ensureHooksConfirmed( if (sourcePolicy === 'local-only') { return 'run' } - const result = await checkRuntimeHooks(settingsForHookRepoOwner(state, repoId), repoId) + const result = await checkRuntimeHooks( + settingsForHookRepoOwner(state, repoId, hostId), + repoId, + hostId + ) if (result.status === 'error') { return 'skip' } @@ -132,7 +158,7 @@ export async function ensureHooksConfirmed( return 'run' } - const repo = state.repos.find((r) => r.id === repoId) + const repo = findHookRepo(state, repoId, hostId) const repoName = repo?.displayName ?? 'this repository' // A non-empty existingHash that didn't match means the user approved a previous // version of this script; the prompt is reappearing because orca.yaml changed. diff --git a/src/renderer/src/runtime/runtime-hooks-client.ts b/src/renderer/src/runtime/runtime-hooks-client.ts index 3d372188481..eff6f52f5b6 100644 --- a/src/renderer/src/runtime/runtime-hooks-client.ts +++ b/src/renderer/src/runtime/runtime-hooks-client.ts @@ -1,4 +1,5 @@ import type { GlobalSettings, OrcaHooks } from '../../../shared/types' +import type { ExecutionHostId } from '../../../shared/execution-host' import type { SetupScriptImportCandidate } from '../../../shared/setup-script-imports' import { callRuntimeRpc, getActiveRuntimeTarget } from './runtime-rpc-client' @@ -20,11 +21,12 @@ export type IssueCommandReadResult = { export async function checkRuntimeHooks( settings: Pick | null | undefined, - repoId: string + repoId: string, + hostId?: ExecutionHostId ): Promise { const target = getActiveRuntimeTarget(settings) if (target.kind !== 'environment') { - return window.api.hooks.check({ repoId }) + return window.api.hooks.check({ repoId, ...(hostId ? { hostId } : {}) }) } return callRuntimeRpc( target, diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index 8929f57594c..a3f85eec42c 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -4101,6 +4101,7 @@ describe('worktree remote runtime mutations', () => { expect(result).toEqual({ ok: true }) expect(mockApi.worktrees.remove).toHaveBeenCalledWith({ worktreeId: wt.id, + hostId: 'ssh:ssh-1', force: undefined, skipArchive: false }) @@ -4108,6 +4109,39 @@ describe('worktree remote runtime mutations', () => { expect(store.getState().worktreesByRepo['repo-ssh']).toEqual([]) }) + it('fails closed before deleting an exact worktree id owned by multiple hosts', async () => { + const store = createTestStore() + const worktreeId = 'repo-shared::/same/path' + store.setState({ + repos: [ + { id: 'repo-shared', path: '/local', displayName: 'Local', badgeColor: '#000', addedAt: 0 }, + { + id: 'repo-shared', + path: '/remote', + displayName: 'SSH', + badgeColor: '#111', + addedAt: 1, + connectionId: 'ssh-1' + } + ], + worktreesByRepo: { + 'repo-shared': [ + makeWorktree({ id: worktreeId, repoId: 'repo-shared', hostId: 'local' }), + makeWorktree({ id: worktreeId, repoId: 'repo-shared', hostId: 'ssh:ssh-1' }) + ] + } + } as Partial) + + const result = await store.getState().removeWorktree(worktreeId) + + expect(result).toEqual({ + ok: false, + error: 'Workspace identity is ambiguous across hosts. Refresh projects and try again.' + }) + expect(mockApi.worktrees.remove).not.toHaveBeenCalled() + expect(runtimeEnvironmentCall).not.toHaveBeenCalled() + }) + it('persists worktree metadata through the active remote runtime environment', async () => { const store = createTestStore() const wt = makeWorktree({ id: 'repo1::/path/wt1', repoId: 'repo1', path: '/path/wt1' }) diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index d6fb11a315b..f459f6f641e 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -71,6 +71,8 @@ export type { WorktreeSlice, WorktreeDeleteState } from './worktree-helpers' // Why: old runtime servers only have `worktree.list`; preserve the large-list // UI hydration parity this slice used before `worktree.detectedList` existed. const REMOTE_WORKTREE_LIST_PARITY_LIMIT = 10_000 +const WORKTREE_REMOVAL_AMBIGUOUS_ERROR = + 'Workspace identity is ambiguous across hosts. Refresh projects and try again.' const ACTIVE_WORKTREE_TERMINAL_PREP_DELAY_MS = 300 const ACTIVE_WORKTREE_TERMINAL_PREP_INPUT_QUIET_MS = 450 const ACTIVE_WORKTREE_TERMINAL_PREP_IDLE_TIMEOUT_MS = 180 @@ -1158,6 +1160,41 @@ function getWorktreeHostId( return repo ? getRepoExecutionHostId(repo) : null } +function resolveWorktreeRemovalHost( + state: Pick, + worktreeId: string +): { hostId: ExecutionHostId | null; ambiguous: boolean } { + const hostIds = new Set() + for (const worktrees of Object.values(state.worktreesByRepo)) { + for (const worktree of worktrees) { + if (worktree.id === worktreeId && worktree.hostId) { + hostIds.add(worktree.hostId) + } + } + } + for (const result of Object.values(state.detectedWorktreesByRepo)) { + for (const worktree of result.worktrees) { + if (worktree.id === worktreeId && worktree.hostId) { + hostIds.add(worktree.hostId) + } + } + } + if (hostIds.size > 1) { + return { hostId: null, ambiguous: true } + } + if (hostIds.size === 1) { + return { hostId: hostIds.values().next().value ?? null, ambiguous: false } + } + + const repoId = getRepoIdFromWorktreeId(worktreeId) + const repoHostIds = new Set( + state.repos.filter((repo) => repo.id === repoId).map(getRepoExecutionHostId) + ) + return repoHostIds.size > 1 + ? { hostId: null, ambiguous: true } + : { hostId: repoHostIds.values().next().value ?? null, ambiguous: false } +} + function mergeLineageForHost( state: Pick< AppState, @@ -3145,6 +3182,11 @@ export const createWorktreeSlice: StateCreator }, removeWorktree: async (worktreeId, force, options) => { + const removalOwner = resolveWorktreeRemovalHost(get(), worktreeId) + if (removalOwner.ambiguous) { + return { ok: false, error: WORKTREE_REMOVAL_AMBIGUOUS_ERROR } + } + const hostId = removalOwner.hostId ?? undefined const forgetLocalOnly = options?.mode === 'forget-local' set((s) => ({ deleteStateByWorktreeId: { @@ -3163,20 +3205,35 @@ export const createWorktreeSlice: StateCreator // and no need to prompt for hook trust. const skipArchive = forgetLocalOnly ? true - : (await ensureHooksConfirmed(get(), getRepoIdFromWorktreeId(worktreeId), 'archive')) === - 'skip' + : (await ensureHooksConfirmed( + get(), + getRepoIdFromWorktreeId(worktreeId), + 'archive', + hostId + )) === 'skip' const worktreeBeforeRemoval = get() .allWorktrees() .find((entry) => entry.id === worktreeId) + const currentOwner = resolveWorktreeRemovalHost(get(), worktreeId) + if ( + currentOwner.ambiguous || + (hostId && currentOwner.hostId && currentOwner.hostId !== hostId) + ) { + throw new Error(WORKTREE_REMOVAL_AMBIGUOUS_ERROR) + } // Why: forget-local always clears Orca's own records via the local IPC // handler regardless of the workspace's execution host — the whole point // is that the remote (SSH relay / runtime) is gone or unreachable. - const target = getActiveRuntimeTarget(settingsForWorktreeOwner(get(), worktreeId)) + const target = getActiveRuntimeTarget( + hostId + ? settingsForExecutionHostOwner(get().settings, hostId) + : settingsForWorktreeOwner(get(), worktreeId) + ) const removalResult = await (forgetLocalOnly - ? window.api.worktrees.forgetLocal({ worktreeId }) + ? window.api.worktrees.forgetLocal({ worktreeId, hostId }) : target.kind === 'local' - ? window.api.worktrees.remove({ worktreeId, force, skipArchive }) + ? window.api.worktrees.remove({ worktreeId, hostId, force, skipArchive }) : callRuntimeRpc( target, 'worktree.rm',