diff --git a/src/main/ipc/worktrees-windows.test.ts b/src/main/ipc/worktrees-windows.test.ts index 5a7c2789260..0298baa40bd 100644 --- a/src/main/ipc/worktrees-windows.test.ts +++ b/src/main/ipc/worktrees-windows.test.ts @@ -427,7 +427,10 @@ describe('registerWorktreeHandlers – Windows path handling', () => { knownRemovedWorktree: registeredWorktree }) ) - expect(store.removeWorktreeMeta).toHaveBeenCalledWith('repo-1::C:/workspaces/improve-dashboard') + expect(store.removeWorktreeMeta).toHaveBeenCalledWith( + 'repo-1::C:/workspaces/improve-dashboard', + 'local' + ) // Windows history lives under a path-derived hash, so scheduling must not regress on this path only. expect(deleteWorktreeHistoryDirMock).toHaveBeenCalledWith( 'repo-1::C:/workspaces/improve-dashboard' diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index 2dec930beaf..7e50c874f34 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -257,7 +257,11 @@ import { __resetSshWorktreeCreateFetchCacheForTests, notifyWorktreesChanged } from './worktree-remote' -import { invalidateAuthorizedRootsCache, resolveRegisteredWorktreePath } from './filesystem-auth' +import { + invalidateAuthorizedRootsCache, + registerWorktreeRootsForRepo, + resolveRegisteredWorktreePath +} from './filesystem-auth' import { _resetTracerForTests, setActiveSink } from '../observability/tracer' import type { RedactableSpan } from '../observability/redactor' import { @@ -304,6 +308,7 @@ describe('registerWorktreeHandlers', () => { setWorktreeMeta: vi.fn(), getProjectHostSetups: vi.fn(), removeWorktreeMeta: vi.fn(), + removeWorkspaceSessionStateForWorktree: vi.fn(), getAllWorktreeLineage: vi.fn(), removeWorktreeLineage: vi.fn(), getAllWorkspaceLineage: vi.fn(), @@ -385,6 +390,7 @@ describe('registerWorktreeHandlers', () => { store.setWorktreeMeta, store.getProjectHostSetups, store.removeWorktreeMeta, + store.removeWorkspaceSessionStateForWorktree, store.getAllWorktreeLineage, store.removeWorktreeLineage, store.getAllWorkspaceLineage, @@ -8005,7 +8011,7 @@ describe('registerWorktreeHandlers', () => { expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['worktree', 'prune'], { cwd: '/workspace/repo' }) - expect(store.removeWorktreeMeta).toHaveBeenCalledWith('repo-1::/workspace/feature-wt') + expect(store.removeWorktreeMeta).toHaveBeenCalledWith('repo-1::/workspace/feature-wt', 'local') expect(deleteWorktreeHistoryDirMock).toHaveBeenCalledWith('repo-1::/workspace/feature-wt') expect(mainWindow.webContents.send).toHaveBeenCalledWith('worktrees:changed', { repoId: 'repo-1' @@ -8046,7 +8052,7 @@ describe('registerWorktreeHandlers', () => { expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['worktree', 'prune'], { cwd: '/workspace/repo' }) - expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId) + expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId, 'local') expect(mainWindow.webContents.send).toHaveBeenCalledWith('worktrees:changed', { repoId: 'repo-1' }) @@ -8160,7 +8166,7 @@ describe('registerWorktreeHandlers', () => { expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['worktree', 'prune'], { cwd: '/workspace/repo' }) - expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId) + expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId, 'local') }) it('preserves a locked missing registration even with force', async () => { @@ -8240,7 +8246,7 @@ describe('registerWorktreeHandlers', () => { expect(killAllProcessesForWorktreeMock.mock.invocationCallOrder[0]).toBeLessThan( store.removeWorktreeMeta.mock.invocationCallOrder[0] ) - expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId) + expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId, 'local') expect(advertisedUrlWatcherForgetWorktreeMock).toHaveBeenCalledWith(worktreeId) expect(deleteWorktreeHistoryDirMock).toHaveBeenCalledWith(worktreeId) expect(mainWindow.webContents.send).toHaveBeenCalledWith('worktrees:changed', { @@ -8873,6 +8879,53 @@ describe('registerWorktreeHandlers', () => { expect(removeWorktreeMock).not.toHaveBeenCalled() }) + it('tears down the remote session when an ownerless remote worktree is deleted', async () => { + const sshRepo = { + id: 'repo-1', + path: '/remote/repo', + displayName: 'ssh', + badgeColor: '#000', + addedAt: 0, + connectionId: 'conn-1', + worktreeBaseRef: null + } + 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(sshRepo) + store.getRepos.mockReturnValue([sshRepo]) + getSshGitProviderMock.mockReturnValue(provider) + // Why: no WorktreeMeta means removal cannot read the owner back; the repo is the only host source. + store.getWorktreeMeta.mockReturnValue(undefined) + + await handlers['worktrees:remove'](null, { + worktreeId: 'repo-1::/remote/feature-wt' + }) + + // The whole point: without the repo's host the ownerless row would clear the local session instead. + expect(store.removeWorktreeMeta).toHaveBeenCalledWith( + 'repo-1::/remote/feature-wt', + 'ssh:conn-1' + ) + }) + it('fails closed when duplicate repo ids are deleted without a host', async () => { const localRepo = { id: 'repo-shared', @@ -9283,7 +9336,10 @@ describe('registerWorktreeHandlers', () => { expect(runtimeStub.clearOptimisticReconcileToken).toHaveBeenCalledWith( 'repo-1::/workspace/already-deleted-wt' ) - expect(store.removeWorktreeMeta).toHaveBeenCalledWith('repo-1::/workspace/already-deleted-wt') + expect(store.removeWorktreeMeta).toHaveBeenCalledWith( + 'repo-1::/workspace/already-deleted-wt', + 'local' + ) expect(deleteWorktreeHistoryDirMock).toHaveBeenCalledWith( 'repo-1::/workspace/already-deleted-wt' ) @@ -9306,7 +9362,7 @@ describe('registerWorktreeHandlers', () => { expect(runHookMock).not.toHaveBeenCalled() expect(removeWorktreeMock).not.toHaveBeenCalled() expect(runtimeStub.clearOptimisticReconcileToken).toHaveBeenCalledWith(worktreeId) - expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId) + expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId, 'local') expect(deleteWorktreeHistoryDirMock).toHaveBeenCalledWith(worktreeId) expect(mainWindow.webContents.send).toHaveBeenCalledWith('worktrees:changed', { repoId: 'repo-1' @@ -9327,7 +9383,10 @@ describe('registerWorktreeHandlers', () => { expect(runtimeStub.clearOptimisticReconcileToken).toHaveBeenCalledWith( 'repo-1::/workspace/already-deleted-wt' ) - expect(store.removeWorktreeMeta).toHaveBeenCalledWith('repo-1::/workspace/already-deleted-wt') + expect(store.removeWorktreeMeta).toHaveBeenCalledWith( + 'repo-1::/workspace/already-deleted-wt', + 'local' + ) expect(deleteWorktreeHistoryDirMock).toHaveBeenCalledWith( 'repo-1::/workspace/already-deleted-wt' ) @@ -9373,7 +9432,7 @@ describe('registerWorktreeHandlers', () => { expect(runHookMock).not.toHaveBeenCalled() expect(removeWorktreeMock).not.toHaveBeenCalled() expect(runtimeStub.clearOptimisticReconcileToken).toHaveBeenCalledWith(worktreeId) - expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId) + expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId, 'local') expect(deleteWorktreeHistoryDirMock).toHaveBeenCalledWith(worktreeId) expect(mainWindow.webContents.send).toHaveBeenCalledWith('worktrees:changed', { repoId: 'repo-1' @@ -9468,7 +9527,7 @@ describe('registerWorktreeHandlers', () => { expect(runHookMock).not.toHaveBeenCalled() expect(removeWorktreeMock).not.toHaveBeenCalled() expect(runtimeStub.clearOptimisticReconcileToken).toHaveBeenCalledWith(worktreeId) - expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId) + expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId, 'local') expect(deleteWorktreeHistoryDirMock).toHaveBeenCalledWith(worktreeId) expect(mainWindow.webContents.send).toHaveBeenCalledWith('worktrees:changed', { repoId: 'repo-1' @@ -10249,21 +10308,29 @@ describe('registerWorktreeHandlers', () => { } const ptyProvider = {} as never const worktreeId = 'repo-1::/workspace/feature-wt' + store.getRepos.mockReturnValue([repo]) store.getRepo.mockReturnValue(repo) getLocalPtyProviderMock.mockReturnValue(ptyProvider) // Why: a removed/disconnected SSH target has no live provider; forgetLocal must not reach for one. getSshGitProviderMock.mockReturnValue(undefined) + getSshPtyProviderMock.mockReturnValue(undefined) const result = await handlers['worktrees:forgetLocal'](null, { worktreeId }) expect(result).toEqual({}) expect(killAllProcessesForWorktreeMock).toHaveBeenCalledWith(worktreeId, { runtime: runtimeStub, + // Without the exact id the sweep resolves a selector that no longer exists and stops nothing. + resolvedWorktreeId: worktreeId, + resolvedConnectionId: 'ssh-dead', localProvider: ptyProvider, - onPtyStopped: clearProviderPtyStateMock + onPtyStopped: clearProviderPtyStateMock, + includeProviderInventory: false, + includeLocalRegistry: false }) expect(runtimeStub.clearOptimisticReconcileToken).toHaveBeenCalledWith(worktreeId) - expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId) + // The purge must be scoped to the same owner the sweep used, or the ssh:* partition keeps this worktree's session state. + expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId, 'ssh:ssh-dead') expect(advertisedUrlWatcherForgetWorktreeMock).toHaveBeenCalledWith(worktreeId) expect(deleteWorktreeHistoryDirMock).toHaveBeenCalledWith(worktreeId) expect(mainWindow.webContents.send).toHaveBeenCalledWith('worktrees:changed', { @@ -10276,17 +10343,115 @@ describe('registerWorktreeHandlers', () => { expect(removeWorktreeMock).not.toHaveBeenCalled() }) - it('throws when the repo is missing', async () => { + it('sweeps a connected SSH owner through its PTY provider', async () => { + const worktreeId = 'repo-gone::/workspace/feature-wt' + const sshProvider = {} as never + store.getRepos.mockReturnValue([]) + store.getRepo.mockReturnValue(undefined) + store.getWorktreeMeta.mockReturnValue({ hostId: 'ssh:ssh-live' }) + getSshPtyProviderMock.mockReturnValue(sshProvider) + + await expect( + handlers['worktrees:forgetLocal'](null, { worktreeId, hostId: 'ssh:ssh-live' }) + ).resolves.toEqual({}) + + expect(getSshPtyProviderMock).toHaveBeenCalledWith('ssh-live') + expect(killAllProcessesForWorktreeMock).toHaveBeenCalledWith(worktreeId, { + runtime: runtimeStub, + resolvedWorktreeId: worktreeId, + resolvedConnectionId: 'ssh-live', + localProvider: sshProvider, + onPtyStopped: clearProviderPtyStateMock, + includeProviderInventory: true, + includeLocalRegistry: false + }) + expect(getLocalPtyProviderMock).not.toHaveBeenCalled() + }) + + it('forgets an ownerless workspace after its repo is already gone', async () => { + const worktreeId = 'repo-gone::/workspace/feature-wt' + const worktreePath = '/workspace/feature-wt' + // Seed authorized roots while the owning repo still exists, then let it disappear. + store.getRepos.mockReturnValue([ + { + id: 'repo-gone', + path: '/workspace/gone', + displayName: 'gone', + badgeColor: '#000', + addedAt: 0 + } + ]) + registerWorktreeRootsForRepo(store as never, 'repo-gone', [worktreePath]) + await expect(resolveRegisteredWorktreePath(worktreePath, store as never)).resolves.toBe( + resolve(worktreePath) + ) + store.getRepos.mockReturnValue([]) store.getRepo.mockReturnValue(undefined) await expect( handlers['worktrees:forgetLocal'](null, { - worktreeId: 'repo-gone::/workspace/feature-wt' + worktreeId, + hostId: 'runtime:env-1' }) - ).rejects.toThrow(/Repo not found/) + ).resolves.toEqual({}) - expect(store.removeWorktreeMeta).not.toHaveBeenCalled() + // The whole point: a forgotten workspace must not stay filesystem-authorized via cached roots. + await expect(resolveRegisteredWorktreePath(worktreePath, store as never)).rejects.toThrow( + 'Access denied: unknown repository or worktree path' + ) + expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId, 'runtime:env-1') + expect(runtimeStub.clearOptimisticReconcileToken).toHaveBeenCalledWith(worktreeId) + expect(mainWindow.webContents.send).toHaveBeenCalledWith('worktrees:changed', { + repoId: 'repo-gone' + }) expect(getSshGitProviderMock).not.toHaveBeenCalled() + expect(removeWorktreeMock).not.toHaveBeenCalled() + }) + + it('purges the SSH owner partition for a hostId-less forget whose worktreeMeta row is already gone', async () => { + const repo = { + id: 'repo-ssh', + path: '/workspace/repo', + displayName: 'repo', + badgeColor: '#000', + addedAt: 0, + executionHostId: 'ssh:ssh-live' as const, + connectionId: 'ssh-live', + worktreeBaseRef: null + } + const worktreeId = 'repo-ssh::/workspace/feature-wt' + store.getRepos.mockReturnValue([repo]) + store.getRepo.mockReturnValue(repo) + // The orphan case forget-local exists for: the meta row is lost, so only the repo still records ownership. + store.getWorktreeMeta.mockReturnValue(undefined) + getSshPtyProviderMock.mockReturnValue(undefined) + getLocalPtyProviderMock.mockReturnValue({} as never) + + await expect(handlers['worktrees:forgetLocal'](null, { worktreeId })).resolves.toEqual({}) + + // Without the resolved owner the purge resolves to [local] only and ssh:ssh-live keeps tabsByWorktree et al. forever. + expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId, 'ssh:ssh-live') + }) + + it('scopes the purge to a local folder workspace owner', async () => { + const repo = { + id: 'repo-folder-child', + path: '/workspace/folder', + displayName: 'folder', + badgeColor: '#000', + addedAt: 0, + kind: 'folder' as const + } + const worktreeId = 'repo-folder-child::/workspace/folder/child' + store.getRepos.mockReturnValue([repo]) + store.getRepo.mockReturnValue(repo) + store.getWorktreeMeta.mockReturnValue(undefined) + getLocalPtyProviderMock.mockReturnValue({} as never) + + await expect(handlers['worktrees:forgetLocal'](null, { worktreeId })).resolves.toEqual({}) + + expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId, 'local') + expect(getSshPtyProviderMock).not.toHaveBeenCalled() }) it('rejects forgetting a folder project root', async () => { diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index baa5f496e58..fe2d6d5fff3 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -287,9 +287,31 @@ async function mapWithConcurrency( return results } -function removeWorktreeMetadataAndTransientState(store: Store, worktreeId: string): void { +// Why: the worktree's own persisted host outranks the repo fallback; teardown and metadata purge must resolve the same owner +// or the purge lands on the local partition while the SSH/runtime one keeps the workspace's tabs forever. +function resolveWorktreeRemovalOwnerHostId( + store: Store, + worktreeId: string, + repo: Repo | undefined, + fallbackHostId?: ExecutionHostId +): ExecutionHostId | undefined { + return ( + store.getWorktreeMeta(worktreeId)?.hostId ?? + (repo ? getRepoExecutionHostId(repo) : fallbackHostId) + ) +} + +function removeWorktreeMetadataAndTransientState( + store: Store, + worktreeId: string, + hostId?: ExecutionHostId +): void { // Why: worktree IDs are path-derived and reusable; drop process-local caches before the same ID can map to a new workspace. - store.removeWorktreeMeta(worktreeId) + if (hostId) { + store.removeWorktreeMeta(worktreeId, hostId) + } else { + store.removeWorktreeMeta(worktreeId) + } advertisedUrlWatcher.forgetWorktree(worktreeId) // Why: drop this worktree's localhost label routes so they don't accumulate in the proxy's route maps all session. localhostWorktreeLabelProxy.unregisterWorktree(worktreeId) @@ -2230,10 +2252,9 @@ export function registerWorktreeHandlers( if (!repo) { throw new Error(`Repo not found: ${repoId}`) } - const inFlightKey = getWorktreeRemovalInFlightKey( - args.worktreeId, - getRepoExecutionHostId(repo) - ) + // The resolved repo supplies host ownership when legacy callers omit args.hostId. + const removalHostId = getRepoExecutionHostId(repo) + const inFlightKey = getWorktreeRemovalInFlightKey(args.worktreeId, removalHostId) const optionsKey = getWorktreeRemovalOptionsKey(args) const inFlightRemoval = worktreeRemovalsInFlight.get(inFlightKey) if (inFlightRemoval) { @@ -2264,7 +2285,7 @@ export function registerWorktreeHandlers( }) }) await withWorktreeRemoveStageSpan('metadata_purge', 'folder', async () => { - removeWorktreeMetadataAndTransientState(store, args.worktreeId) + removeWorktreeMetadataAndTransientState(store, args.worktreeId, removalHostId) }) preservedBranchCleanupByWorktreeId.delete(args.worktreeId) notifyWorktreesChanged(mainWindow, repoId) @@ -2373,7 +2394,7 @@ export function registerWorktreeHandlers( invalidateAuthorizedRootsCache() } runtime.clearOptimisticReconcileToken(args.worktreeId) - removeWorktreeMetadataAndTransientState(store, args.worktreeId) + removeWorktreeMetadataAndTransientState(store, args.worktreeId, removalHostId) preservedBranchCleanupByWorktreeId.delete(args.worktreeId) notifyWorktreesChanged(mainWindow, repoId) return {} @@ -2416,7 +2437,7 @@ export function registerWorktreeHandlers( localWorktreeGitOptions ) runtime.clearOptimisticReconcileToken(args.worktreeId) - removeWorktreeMetadataAndTransientState(store, args.worktreeId) + removeWorktreeMetadataAndTransientState(store, args.worktreeId, removalHostId) preservedBranchCleanupByWorktreeId.delete(args.worktreeId) invalidateAuthorizedRootsCache() notifyWorktreesChanged(mainWindow, repoId) @@ -2448,7 +2469,7 @@ export function registerWorktreeHandlers( invalidateAuthorizedRootsCache() } runtime.clearOptimisticReconcileToken(args.worktreeId) - removeWorktreeMetadataAndTransientState(store, args.worktreeId) + removeWorktreeMetadataAndTransientState(store, args.worktreeId, removalHostId) preservedBranchCleanupByWorktreeId.delete(args.worktreeId) notifyWorktreesChanged(mainWindow, repoId) return {} @@ -2502,7 +2523,7 @@ export function registerWorktreeHandlers( removedPushTarget ) runtime.clearOptimisticReconcileToken(args.worktreeId) - removeWorktreeMetadataAndTransientState(store, args.worktreeId) + removeWorktreeMetadataAndTransientState(store, args.worktreeId, removalHostId) invalidateAuthorizedRootsCache() notifyWorktreesChanged(mainWindow, repoId) return removalResult ?? {} @@ -2601,7 +2622,7 @@ export function registerWorktreeHandlers( ) runtime.clearOptimisticReconcileToken(args.worktreeId) await withWorktreeRemoveStageSpan('metadata_purge', 'remote', async () => { - removeWorktreeMetadataAndTransientState(store, args.worktreeId) + removeWorktreeMetadataAndTransientState(store, args.worktreeId, removalHostId) }) notifyWorktreesChanged(mainWindow, repoId) return removalResult ?? {} @@ -2745,7 +2766,7 @@ export function registerWorktreeHandlers( localWorktreeGitOptions ) runtime.clearOptimisticReconcileToken(args.worktreeId) - removeWorktreeMetadataAndTransientState(store, args.worktreeId) + removeWorktreeMetadataAndTransientState(store, args.worktreeId, removalHostId) preservedBranchCleanupByWorktreeId.delete(args.worktreeId) invalidateAuthorizedRootsCache() notifyWorktreesChanged(mainWindow, repoId) @@ -2776,7 +2797,7 @@ export function registerWorktreeHandlers( ) runtime.clearOptimisticReconcileToken(args.worktreeId) await withWorktreeRemoveStageSpan('metadata_purge', 'local', async () => { - removeWorktreeMetadataAndTransientState(store, args.worktreeId) + removeWorktreeMetadataAndTransientState(store, args.worktreeId, removalHostId) }) await withWorktreeRemoveStageSpan('cache_invalidation', 'local', async () => { invalidateAuthorizedRootsCache() @@ -2812,13 +2833,12 @@ export function registerWorktreeHandlers( ): 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 so concurrent remove and forgetLocal on the same id can't both mutate metadata. + // Repo-first (unlike owner resolution below) so this key matches worktrees:remove's; meta only covers ownerless forgets. const inFlightKey = getWorktreeRemovalInFlightKey( args.worktreeId, - getRepoExecutionHostId(repo) + repo + ? getRepoExecutionHostId(repo) + : (store.getWorktreeMeta(args.worktreeId)?.hostId ?? args.hostId) ) const optionsKey = 'forget-local' const inFlight = worktreeRemovalsInFlight.get(inFlightKey) @@ -2830,23 +2850,50 @@ export function registerWorktreeHandlers( } const forget = (async (): Promise => { - if (isFolderRepo(repo) && args.worktreeId === getFolderWorkspaceRootId(repo)) { + // Ambiguous repo lookup must not bypass a live folder root's deletion guard. + const isFolderRootOf = (candidate: Repo): boolean => + isFolderRepo(candidate) && args.worktreeId === getFolderWorkspaceRootId(candidate) + if (repo ? isFolderRootOf(repo) : store.getRepos().some(isFolderRootOf)) { throw new Error( 'Cannot delete the project root workspace. Remove the folder project instead.' ) } - // Why: best-effort PTY sweep; resolves synchronously for a dead SSH relay (tombstoned lease) so it never hangs. + const ownerHostId = resolveWorktreeRemovalOwnerHostId( + store, + args.worktreeId, + repo, + args.hostId + ) + const ownerHost = parseExecutionHostId(ownerHostId) + const sshPtyProvider = + ownerHost?.kind === 'ssh' ? getSshPtyProvider(ownerHost.targetId) : undefined + const externalHost = ownerHost?.kind === 'ssh' || ownerHost?.kind === 'runtime' + // External host inventories must never sweep a same-id local workspace. await killAllProcessesForWorktree(args.worktreeId, { runtime, - localProvider: getLocalPtyProvider(), - onPtyStopped: clearProviderPtyState + resolvedWorktreeId: args.worktreeId, + ...(ownerHost?.kind === 'ssh' ? { resolvedConnectionId: ownerHost.targetId } : {}), + ...(ownerHost?.kind === 'runtime' + ? { resolvedRuntimeEnvironmentId: ownerHost.environmentId } + : {}), + localProvider: sshPtyProvider ?? getLocalPtyProvider(), + onPtyStopped: clearProviderPtyState, + ...(externalHost + ? { + includeProviderInventory: ownerHost?.kind === 'ssh' && Boolean(sshPtyProvider), + includeLocalRegistry: false + } + : {}) }).catch((err) => { console.warn(`[worktree-teardown] forget-local failed for ${args.worktreeId}:`, err) }) runtime.clearOptimisticReconcileToken(args.worktreeId) - removeWorktreeMetadataAndTransientState(store, args.worktreeId) + // The resolved owner, not args.hostId: an orphan forget with no hostId still has to purge its SSH/runtime partition. + removeWorktreeMetadataAndTransientState(store, args.worktreeId, ownerHost?.id) + // Why: cached roots outlive the forgotten workspace, so an ownerless path stays filesystem-authorized until a rebuild. + invalidateAuthorizedRootsCache() preservedBranchCleanupByWorktreeId.delete(args.worktreeId) notifyWorktreesChanged(mainWindow, repoId) return {} diff --git a/src/main/persistence.test.ts b/src/main/persistence.test.ts index 8fb3ad85904..81faa5f6297 100644 --- a/src/main/persistence.test.ts +++ b/src/main/persistence.test.ts @@ -11699,6 +11699,197 @@ describe('Store host-partitioned workspace sessions', () => { expect(reloaded.getWorkspaceSession('runtime:env-a').activeRepoId).toBe('repo-a') }) + it('removes one orphaned worktree with a host-scoped topology fence', async () => { + const store = await createStore() + const worktreeId = 'repo-gone::/workspace/stale' + const session = { + ...makeHostSession('repo-gone'), + activeWorktreeId: worktreeId, + activeWorktreeIdsOnShutdown: [worktreeId], + lastVisitedAtByWorktreeId: { [worktreeId]: 123 }, + tabsByWorktree: { + [worktreeId]: [makeTerminalTab({ id: 'stale-tab', worktreeId })] + }, + terminalTopologyRevisionByRepoId: { 'repo-gone': 3 } + } + store.setWorkspaceSession(session, 'local') + store.setWorkspaceSession(session, 'runtime:env-a') + store.setWorkspaceSession(session, 'runtime:env-b') + + store.removeWorkspaceSessionStateForWorktree(worktreeId, 'runtime:env-a') + store.setWorkspaceSession(session, 'runtime:env-a') + store.flush() + + const reloaded = await createStore() + expect(reloaded.getWorkspaceSession('runtime:env-a')).toMatchObject({ + tabsByWorktree: {}, + terminalTopologyRevisionByRepoId: { 'repo-gone': 4 } + }) + expect(reloaded.getWorkspaceSession('runtime:env-b')).toMatchObject({ + activeWorktreeId: worktreeId, + activeWorktreeIdsOnShutdown: [worktreeId], + lastVisitedAtByWorktreeId: { [worktreeId]: 123 }, + terminalTopologyRevisionByRepoId: { 'repo-gone': 3 } + }) + // The local blob is a co-owner surface for every remote host, since the renderer parks state + // there whenever worktree ownership is unresolved; leaving it behind leaks the removed worktree. + expect(reloaded.getWorkspaceSession('local')).toMatchObject({ + tabsByWorktree: {}, + activeWorktreeId: null, + activeWorktreeIdsOnShutdown: [], + lastVisitedAtByWorktreeId: {}, + terminalTopologyRevisionByRepoId: { 'repo-gone': 4 } + }) + }) + + it('uses persisted worktree ownership when removing metadata', async () => { + const store = await createStore() + const worktreeId = 'repo-gone::/workspace/stale' + const session = { + ...makeHostSession('repo-gone'), + tabsByWorktree: { + [worktreeId]: [makeTerminalTab({ id: 'stale-tab', worktreeId })] + } + } + store.setWorkspaceSession(session, 'local') + store.setWorkspaceSession(session, 'runtime:env-a') + store.setWorkspaceSession(session, 'runtime:env-b') + store.setWorktreeMeta(worktreeId, { hostId: 'runtime:env-a' }) + + store.removeWorktreeMeta(worktreeId) + + expect(store.getWorkspaceSession('runtime:env-a').tabsByWorktree[worktreeId]).toBeUndefined() + // Only the owning host's partition and the local blob it may spill into are cleaned; a same-id + // worktree on another host is a different workspace and must survive. + expect(store.getWorkspaceSession('runtime:env-b').tabsByWorktree[worktreeId]).toHaveLength(1) + expect(store.getWorkspaceSession('local').tabsByWorktree[worktreeId]).toBeUndefined() + }) + + it('cleans both surfaces that hold an ssh-owned worktree', async () => { + const store = await createStore() + const worktreeId = 'repo-ssh::/srv/stale' + const makeSession = (): ReturnType => ({ + ...makeHostSession('repo-ssh'), + activeWorktreeId: worktreeId, + lastVisitedAtByWorktreeId: { [worktreeId]: 123 }, + tabsByWorktree: { + [worktreeId]: [makeTerminalTab({ id: 'stale-tab', worktreeId })] + } + }) + // The renderer persists SSH workspaces in the local blob; the main process writes their + // terminal state to the host's own `ssh:*` partition. Removal must clear both. + store.setWorkspaceSession(makeSession(), 'local') + store.setWorkspaceSession(makeSession(), 'ssh:conn-1') + store.setWorktreeMeta(worktreeId, { hostId: 'ssh:conn-1' }) + + store.removeWorktreeMeta(worktreeId, 'ssh:conn-1') + + for (const hostId of ['local', 'ssh:conn-1'] as const) { + expect(store.getWorkspaceSession(hostId)).toMatchObject({ + tabsByWorktree: {}, + activeWorktreeId: null, + lastVisitedAtByWorktreeId: {}, + terminalTopologyRevisionByRepoId: { 'repo-ssh': 1 } + }) + } + }) + + it('does not bump the topology fence in a partition that never held the worktree', async () => { + const store = await createStore() + const worktreeId = 'repo-split::/workspace/stale' + const otherWorktreeId = 'repo-split::/workspace/live' + // Why this matters: the fence is keyed by repo, so a bump here would make every later write + // for repo-split rebase onto main's copy and silently drop the live worktree's unsaved tabs. + store.setWorkspaceSession( + { + ...makeHostSession('repo-split'), + tabsByWorktree: { + [otherWorktreeId]: [makeTerminalTab({ id: 'live-tab', worktreeId: otherWorktreeId })] + } + }, + 'local' + ) + store.setWorkspaceSession( + { + ...makeHostSession('repo-split'), + tabsByWorktree: { + [worktreeId]: [makeTerminalTab({ id: 'stale-tab', worktreeId })] + } + }, + 'runtime:env-a' + ) + store.setWorktreeMeta(worktreeId, { hostId: 'runtime:env-a' }) + + store.removeWorktreeMeta(worktreeId, 'local') + + expect( + store.getWorkspaceSession('runtime:env-a').terminalTopologyRevisionByRepoId?.['repo-split'] + ).toBe(1) + expect( + store.getWorkspaceSession('local').terminalTopologyRevisionByRepoId?.['repo-split'] + ).toBeUndefined() + expect(store.getWorkspaceSession('local').tabsByWorktree[otherWorktreeId]).toHaveLength(1) + }) + + it('trusts persisted ownership over a stale caller hostId', async () => { + const store = await createStore() + const worktreeId = 'repo-split::/workspace/stale' + const session = { + ...makeHostSession('repo-split'), + tabsByWorktree: { + [worktreeId]: [makeTerminalTab({ id: 'stale-tab', worktreeId })] + } + } + store.setWorkspaceSession(session, 'runtime:env-a') + store.setWorkspaceSession(session, 'runtime:env-b') + store.setWorktreeMeta(worktreeId, { hostId: 'runtime:env-a' }) + + // A caller's hostId comes from live routing and can go stale mid-removal; the same + // repoId::path can name a live worktree on env-b, whose tabs must survive. + store.removeWorktreeMeta(worktreeId, 'runtime:env-b') + + expect(store.getWorkspaceSession('runtime:env-a').tabsByWorktree[worktreeId]).toBeUndefined() + expect(store.getWorkspaceSession('runtime:env-b').tabsByWorktree[worktreeId]).toHaveLength(1) + }) + + it('falls back to the caller hostId only when no ownership was recorded', async () => { + const store = await createStore() + const worktreeId = 'repo-gone::/workspace/stale' + const session = { + ...makeHostSession('repo-gone'), + tabsByWorktree: { + [worktreeId]: [makeTerminalTab({ id: 'stale-tab', worktreeId })] + } + } + store.setWorkspaceSession(session, 'runtime:env-a') + store.setWorkspaceSession(session, 'local') + + store.removeWorktreeMeta(worktreeId, 'runtime:env-a') + + expect(store.getWorkspaceSession('runtime:env-a').tabsByWorktree[worktreeId]).toBeUndefined() + expect(store.getWorkspaceSession('local').tabsByWorktree[worktreeId]).toBeUndefined() + }) + + // Trade-off: the fence is repo-wide, so claiming authority over a partition main never wrote would + // rebase every sibling worktree of that repo onto an empty copy. A delayed write wins here instead. + it('does not fence a delayed session write when its host partition was never persisted', async () => { + const store = await createStore() + const worktreeId = 'repo-gone::/workspace/stale' + const delayedSession = { + ...makeHostSession('repo-gone'), + tabsByWorktree: { + [worktreeId]: [makeTerminalTab({ id: 'stale-tab', worktreeId })] + } + } + + store.removeWorkspaceSessionStateForWorktree(worktreeId, 'runtime:env-a') + store.setWorkspaceSession(delayedSession, 'runtime:env-a') + + const session = store.getWorkspaceSession('runtime:env-a') + expect(session.tabsByWorktree[worktreeId]?.[0]?.id).toBe('stale-tab') + expect(session.terminalTopologyRevisionByRepoId?.['repo-gone']).toBeUndefined() + }) + it('drops a corrupt host partition to defaults without failing the others', async () => { writeDataFile({ schemaVersion: 1, diff --git a/src/main/persistence.ts b/src/main/persistence.ts index 966686bfbdf..40660eb8a1c 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -2614,6 +2614,20 @@ function deleteScannedSessionFieldsForOwners( ) } +// Remote (ssh:/runtime:) workspace state can exist in both the renderer's local blob and main's host +// partition, because the renderer falls back to 'local' whenever worktree ownership is unresolved. +function workspaceSessionPartitionIdsForHost(hostId: string | null | undefined): ExecutionHostId[] { + const parsed = parseExecutionHostId(hostId) + return parsed && parsed.id !== LOCAL_EXECUTION_HOST_ID + ? [LOCAL_EXECUTION_HOST_ID, parsed.id] + : [LOCAL_EXECUTION_HOST_ID] +} + +/** The partition the host actually owns; the others are only spill surfaces for it. */ +function workspaceSessionOwnerPartitionForHost(hostId: string | null | undefined): ExecutionHostId { + return parseExecutionHostId(hostId)?.id ?? LOCAL_EXECUTION_HOST_ID +} + function removeWorkspaceSessionOwner( session: WorkspaceSessionState | undefined, ownerKey: string, @@ -5332,15 +5346,34 @@ export class Store { return updated } - removeWorktreeMeta(worktreeId: string): void { + removeWorktreeMeta(worktreeId: string, hostId?: ExecutionHostId | null): void { + // Persisted ownership beats stale live routing; hostId is only an ownerless fallback. + const owner = this.state.worktreeMeta[worktreeId]?.hostId ?? hostId + // Skip partitions main never wrote: materializing one fences every sibling worktree of the repo. + const partitions = new Set( + workspaceSessionPartitionIdsForHost(owner).filter((partition) => + this.hasPersistedWorkspaceSession(partition) + ) + ) + // A repo-wide fence must not rebase a sibling's unpersisted tabs onto main's copy, and a spill + // partition that never held this worktree has no claim on the repo at all. + const ownerPartition = workspaceSessionOwnerPartitionForHost(owner) + const fencedPartitions = new Set( + [...partitions].filter( + (partition) => + this.partitionOwnsWorktreeTabs(worktreeId, partition) || + (partition === ownerPartition && + !this.partitionHasOtherRepoWorktreeTabs(worktreeId, partition)) + ) + ) delete this.state.worktreeMeta[worktreeId] delete this.state.worktreeLineageById[worktreeId] delete this.state.workspaceLineageByChildKey[worktreeWorkspaceKey(worktreeId)] - this.state.workspaceSession = removeWorkspaceSessionOwner( - this.state.workspaceSession, - worktreeId, - { advanceTerminalTopologyRevision: true } - )! + for (const partition of partitions) { + this.removeWorkspaceSessionOwnerInPartition(worktreeId, partition, { + advanceTerminalTopologyRevision: fencedPartitions.has(partition) + }) + } this.scheduleSave() } @@ -6107,6 +6140,14 @@ export class Store { return this.state.workspaceSessionsByHostId?.[resolved] ?? getDefaultWorkspaceSession() } + /** Whether a partition was ever written; `getWorkspaceSession` defaults absent ones and cannot tell them apart. */ + private hasPersistedWorkspaceSession(hostId: ExecutionHostId): boolean { + return ( + hostId === LOCAL_EXECUTION_HOST_ID || + this.state.workspaceSessionsByHostId?.[hostId] !== undefined + ) + } + getWorkspaceSessionHostIds(): ExecutionHostId[] { const hostIds = new Set([LOCAL_EXECUTION_HOST_ID]) for (const key of Object.keys(this.state.workspaceSessionsByHostId ?? {})) { @@ -6136,6 +6177,58 @@ export class Store { this.setHostWorkspaceSession(resolved, session) } + removeWorkspaceSessionStateForWorktree( + worktreeId: string, + hostId?: ExecutionHostId | null, + options: { advanceTerminalTopologyRevision?: boolean } = {} + ): void { + for (const resolved of workspaceSessionPartitionIdsForHost(hostId)) { + this.removeWorkspaceSessionOwnerInPartition(worktreeId, resolved, options) + } + } + + private removeWorkspaceSessionOwnerInPartition( + worktreeId: string, + resolved: ExecutionHostId, + options: { advanceTerminalTopologyRevision?: boolean } + ): void { + if (!this.hasPersistedWorkspaceSession(resolved)) { + return + } + const current = this.getWorkspaceSession(resolved) + const session = removeWorkspaceSessionOwner(current, worktreeId, { + advanceTerminalTopologyRevision: options.advanceTerminalTopologyRevision ?? true + }) + if (!session) { + return + } + if (resolved === LOCAL_EXECUTION_HOST_ID) { + this.state.workspaceSession = session + } else { + // Host scoping matters because identical repo/path ids may exist on two servers. + this.state.workspaceSessionsByHostId = { + ...this.state.workspaceSessionsByHostId, + [resolved]: session + } + } + this.scheduleSave() + } + + /** Whether a partition still holds terminal membership for `worktreeId`. */ + private partitionOwnsWorktreeTabs(worktreeId: string, hostId: ExecutionHostId): boolean { + return this.getWorkspaceSession(hostId).tabsByWorktree?.[worktreeId] !== undefined + } + + /** Whether fencing this partition would rebase a sibling worktree of the same repo. */ + private partitionHasOtherRepoWorktreeTabs(worktreeId: string, hostId: ExecutionHostId): boolean { + const repoId = getRepoIdFromWorktreeId(worktreeId) + const tabsByWorktree = this.getWorkspaceSession(hostId).tabsByWorktree ?? {} + return Object.entries(tabsByWorktree).some( + ([id, tabs]) => + id !== worktreeId && getRepoIdFromWorktreeId(id) === repoId && (tabs?.length ?? 0) > 0 + ) + } + stageWorkspaceSessionBeforeUnload( session: PersistedState['workspaceSession'], hostId?: string | null diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index e89bf7fb214..51c7b252470 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -96,6 +96,10 @@ import { unregisterSshFilesystemProvider } from '../providers/ssh-filesystem-dispatch' import { registerSshGitProvider, unregisterSshGitProvider } from '../providers/ssh-git-dispatch' +import { + registerPty as registerLocalPtyMemoryRow, + unregisterPty as unregisterLocalPtyMemoryRow +} from '../memory/pty-registry' import { inspectPtyProviderProcess } from '../providers/pty-process-inspection' import type { IPtyProvider } from '../providers/types' import * as worktreePathComparison from '../ipc/worktree-path-comparison' @@ -33597,6 +33601,68 @@ describe('OrcaRuntimeService', () => { expect(killed).toBe(false) }) + it('stops by exact id when the selector no longer resolves', async () => { + const runtime = new OrcaRuntimeService(store) + const kill = vi.fn(() => true) + runtime.setPtyController({ + write: () => true, + kill, + stopAndWait: vi.fn(async () => true), + getForegroundProcess: async () => null + }) + syncSinglePty(runtime) + + // An orphaned workspace's repo is gone, so only the caller's id can identify it. + await expect( + runtime.stopTerminalsForWorktree('id:repo-gone::/tmp/gone', { + resolvedWorktreeId: TEST_WORKTREE_ID + }) + ).resolves.toEqual({ stopped: 1 }) + expect(kill).toHaveBeenCalledWith('pty-1') + }) + + it('does not sweep a sibling workspace sharing the checkout dir of an exact id', async () => { + const runtime = new OrcaRuntimeService(store) + const kill = vi.fn(() => true) + runtime.setPtyController({ + write: () => true, + kill, + stopAndWait: vi.fn(async () => true), + getForegroundProcess: async () => null + }) + syncSinglePty(runtime) + + // Regression for #10252: folder-workspace instances share one checkout dir, so comparing + // filesystem paths (which strip `::workspace:`) would match the root and its siblings + // and kill their live terminals. + await expect( + runtime.stopTerminalsForWorktree('id:repo-gone::/tmp/gone', { + resolvedWorktreeId: `${TEST_WORKTREE_ID}::workspace:11111111-1111-1111-1111-111111111111` + }) + ).resolves.toEqual({ stopped: 0 }) + expect(kill).not.toHaveBeenCalled() + }) + + it('does not sweep a same-id terminal owned by another connection', async () => { + const runtime = new OrcaRuntimeService(store) + const kill = vi.fn(() => true) + runtime.setPtyController({ + write: () => true, + kill, + stopAndWait: vi.fn(async () => true), + getForegroundProcess: async () => null + }) + syncSinglePty(runtime) + + await expect( + runtime.stopTerminalsForWorktree('id:repo-gone::/tmp/gone', { + resolvedWorktreeId: TEST_WORKTREE_ID, + resolvedConnectionId: 'ssh-1' + }) + ).resolves.toEqual({ stopped: 0 }) + expect(kill).not.toHaveBeenCalled() + }) + it('awaits physical PTY stop when destructive teardown supplies shared dedupe', async () => { const runtime = new OrcaRuntimeService(store) const physicalStop = makeDeferred() @@ -41617,6 +41683,194 @@ describe('OrcaRuntimeService', () => { expect(removeWorktreeLinkedPathsMock).toHaveBeenCalledWith(TEST_WORKTREE_PATH, ['node_modules']) }) + it('forgets exact-id orphan metadata when the parent repo is already gone', async () => { + const { runtimeStore, removeWorktreeMeta } = createStaleRuntimeWorktreeStore(TEST_WORKTREE_ID, { + hostId: 'runtime:env-1' + }) + const orphanStore = { + ...runtimeStore, + getRepos: () => [], + getRepo: () => undefined + } + const runtime = createWorktreeRemovalRuntime(orphanStore) + + // Nothing left the disk, so non-desktop callers must be able to tell "forgotten" from "deleted". + await expect(runtime.removeManagedWorktree(TEST_WORKTREE_ID)).resolves.toEqual({ + warning: expect.stringContaining(TEST_WORKTREE_PATH) + }) + + expect(removeWorktreeMeta).toHaveBeenCalledWith(TEST_WORKTREE_ID, 'runtime:env-1') + expect(deleteWorktreeHistoryDirMock).toHaveBeenCalledWith(TEST_WORKTREE_ID) + expect(invalidateAuthorizedRootsCacheMock).toHaveBeenCalled() + expect(removeWorktree).not.toHaveBeenCalled() + // Nothing is deleted on disk here, so the surviving directory's watchers must still be released. + expect(closeLocalWatcherForWorktreePathMock).toHaveBeenCalledWith( + TEST_WORKTREE_PATH, + expect.objectContaining({ remainingMs: expect.any(Function) }) + ) + // Regression: the directory survives, so its watchers must be restored rather than forgotten. + expect(restoreLocalWatcherAfterFailedRemovalMock).toHaveBeenCalledWith(TEST_WORKTREE_PATH) + expect(forgetLocalWatcherRemovalSnapshotMock).not.toHaveBeenCalled() + }) + + it('scopes a runtime-host orphan PTY sweep to its environment, not the local host', async () => { + const { runtimeStore } = createStaleRuntimeWorktreeStore(TEST_WORKTREE_ID, { + hostId: 'runtime:env-1' + }) + const orphanStore = { + ...runtimeStore, + getRepos: () => [], + getRepo: () => undefined + } + const localProvider = { + listProcesses: vi.fn(async () => [{ id: `${TEST_WORKTREE_ID}@@1` }]), + shutdown: vi.fn(async () => {}) + } + const runtime = new OrcaRuntimeService(orphanStore as never, undefined, { + getLocalProvider: () => localProvider as never + }) + const stopAndWait = vi.fn().mockResolvedValue(true) + runtime.setPtyController({ + write: () => true, + kill: () => true, + stopAndWait, + getForegroundProcess: async () => null + }) + syncSinglePty(runtime, 'local-pty-1') + runtime.registerPty('local-pty-1', TEST_WORKTREE_ID) + registerLocalPtyMemoryRow({ + ptyId: 'local-pty-1', + worktreeId: TEST_WORKTREE_ID, + sessionId: null, + paneKey: null, + pid: null + }) + + try { + await runtime.removeManagedWorktree(TEST_WORKTREE_ID) + } finally { + unregisterLocalPtyMemoryRow('local-pty-1') + } + + // Regression: `repoId::path` collides across hosts, so a runtime-owned orphan + // must never kill the live PTYs of a same-id LOCAL workspace. + expect(localProvider.listProcesses).not.toHaveBeenCalled() + expect(localProvider.shutdown).not.toHaveBeenCalled() + expect(stopAndWait).not.toHaveBeenCalled() + }) + + it('still sweeps the local host for an ownerless orphan', async () => { + const { runtimeStore } = createStaleRuntimeWorktreeStore(TEST_WORKTREE_ID) + const orphanStore = { + ...runtimeStore, + getRepos: () => [], + getRepo: () => undefined + } + const localProvider = { + listProcesses: vi.fn(async () => []), + shutdown: vi.fn(async () => {}) + } + const runtime = new OrcaRuntimeService(orphanStore as never, undefined, { + getLocalProvider: () => localProvider as never + }) + const stopAndWait = vi.fn().mockResolvedValue(true) + runtime.setPtyController({ + write: () => true, + kill: () => true, + stopAndWait, + getForegroundProcess: async () => null + }) + syncSinglePty(runtime, 'local-pty-1') + runtime.registerPty('local-pty-1', TEST_WORKTREE_ID) + + await runtime.removeManagedWorktree(TEST_WORKTREE_ID) + + expect(localProvider.listProcesses).toHaveBeenCalled() + expect(stopAndWait).toHaveBeenCalledWith('local-pty-1', expect.anything()) + }) + + it('restores rather than forgets watchers for the orphan directory that survives', async () => { + const { runtimeStore } = createStaleRuntimeWorktreeStore(TEST_WORKTREE_ID, { + hostId: 'runtime:env-1' + }) + const orphanStore = { + ...runtimeStore, + getRepos: () => [], + getRepo: () => undefined + } + const runtime = createWorktreeRemovalRuntime(orphanStore) + + await runtime.removeManagedWorktree(TEST_WORKTREE_ID) + + // Regression: the gate must finish(false) — forgetting watchers would silently deafen a + // folder workspace or File Explorer pane rooted at this still-present directory. + expect(closeLocalWatcherForWorktreePathMock).toHaveBeenCalledWith( + TEST_WORKTREE_PATH, + expect.objectContaining({ remainingMs: expect.any(Function) }) + ) + expect(restoreLocalWatcherAfterFailedRemovalMock).toHaveBeenCalledWith(TEST_WORKTREE_PATH) + expect(forgetLocalWatcherRemovalSnapshotMock).not.toHaveBeenCalled() + expect(removeWorktree).not.toHaveBeenCalled() + }) + + it('warns that a missing-repo removal only forgot the workspace', async () => { + const { runtimeStore } = createStaleRuntimeWorktreeStore(TEST_WORKTREE_ID, { + hostId: 'runtime:env-1' + }) + const orphanStore = { + ...runtimeStore, + getRepos: () => [], + getRepo: () => undefined + } + const runtime = createWorktreeRemovalRuntime(orphanStore) + + const result = await runtime.removeManagedWorktree(TEST_WORKTREE_ID) + + // Regression: CLI and mobile share this method, so success alone must not read as "deleted". + expect(result.warning).toEqual(expect.stringContaining(TEST_WORKTREE_PATH)) + expect(result.warning).toEqual(expect.stringContaining(TEST_REPO_ID)) + expect(removeWorktree).not.toHaveBeenCalled() + }) + + it('sweeps an orphaned SSH worktree through its host provider', async () => { + const { runtimeStore } = createStaleRuntimeWorktreeStore(TEST_WORKTREE_ID, { + hostId: 'ssh:ssh-1' + }) + const orphanStore = { + ...runtimeStore, + getRepos: () => [], + getRepo: () => undefined + } + const localProvider = { + listProcesses: vi.fn(async () => []), + shutdown: vi.fn(async () => {}) + } + const sshProvider = { + listProcesses: vi.fn(async () => []), + shutdown: vi.fn(async () => {}) + } + const getSshProvider = vi.fn(() => sshProvider as never) + const runtime = new OrcaRuntimeService(orphanStore as never, undefined, { + getLocalProvider: () => localProvider as never, + getSshProvider + }) + + await expect(runtime.removeManagedWorktree(TEST_WORKTREE_ID)).resolves.toEqual({ + warning: expect.stringContaining(TEST_WORKTREE_PATH) + }) + + expect(getSshProvider).toHaveBeenCalledWith('ssh-1') + expect(sshProvider.listProcesses).toHaveBeenCalled() + expect(localProvider.listProcesses).not.toHaveBeenCalled() + expect(closeRemoteWatcherForWorktreePathMock).toHaveBeenCalledWith('ssh-1', TEST_WORKTREE_PATH) + // The remote directory survives, so its watchers are restored, not forgotten. + expect(restoreRemoteWatcherAfterFailedRemovalMock).toHaveBeenCalledWith( + 'ssh-1', + TEST_WORKTREE_PATH + ) + expect(forgetRemoteWatcherRemovalSnapshotMock).not.toHaveBeenCalled() + }) + it('does not remove a runtime worktree when watcher teardown cannot release it', async () => { const repo = { ...store.getRepos()[0], symlinkPaths: ['node_modules'] } const runtimeStore = { ...store, getRepos: () => [repo], getRepo: () => repo } diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 18273e3a6ca..698fd80f564 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -23076,7 +23076,13 @@ export class OrcaRuntimeService { private removeWorktreeMetadataAndHistory(store: RuntimeStore, worktreeId: string): void { // Why: worktree IDs are path-derived and can be recreated, so removal must // purge history and process-local caches before the ID points at new state. - store.removeWorktreeMeta(worktreeId) + const hostId = store.getWorktreeMeta(worktreeId)?.hostId + if (hostId) { + store.removeWorktreeMeta(worktreeId, hostId) + } else { + store.removeWorktreeMeta(worktreeId) + } + this.mobileSessionTabsByWorktree.delete(worktreeId) advertisedUrlWatcher.forgetWorktree(worktreeId) deleteWorktreeHistoryDir(worktreeId) this.closeHeadlessBrowserPagesForWorktree(worktreeId) @@ -23229,7 +23235,61 @@ export class OrcaRuntimeService { return withWorktreeSpan({ stage: 'remove', path: removalTarget.path }, async () => { const repo = store.getRepo(removalTarget.repoId) if (!repo) { - throw new Error('repo_not_found') + const orphanHost = parseExecutionHostId(store.getWorktreeMeta(removalTarget.id)?.hostId) + const sshPtyProvider = + orphanHost?.kind === 'ssh' ? this.getSshProviderFn?.(orphanHost.targetId) : undefined + const ptyProvider = sshPtyProvider ?? this.getLocalProvider() + const externalOrphanHost = orphanHost?.kind === 'ssh' || orphanHost?.kind === 'runtime' + if (ptyProvider) { + // External host inventories must never sweep a same-id local workspace. + await killAllProcessesForWorktree(removalTarget.id, { + runtime: this, + resolvedWorktreeId: removalTarget.id, + ...(orphanHost?.kind === 'ssh' ? { resolvedConnectionId: orphanHost.targetId } : {}), + ...(orphanHost?.kind === 'runtime' + ? { resolvedRuntimeEnvironmentId: orphanHost.environmentId } + : {}), + localProvider: ptyProvider, + onPtyStopped: this.onPtyStopped ?? undefined, + ...(externalOrphanHost + ? { + includeProviderInventory: orphanHost?.kind === 'ssh' && Boolean(sshPtyProvider), + includeLocalRegistry: false + } + : {}) + }).catch((error) => { + console.warn( + `[worktree-teardown] orphan cleanup failed for ${removalTarget.id}:`, + error + ) + }) + } + // Why: nothing is deleted on disk here, so watchers must be restored — a folder + // workspace or explorer pane rooted at the same path stays live. + const orphanFullPath = splitWorktreeId(removalTarget.id)?.worktreePath + const orphanWatcherPath = + splitWorktreeIdForFilesystem(removalTarget.id)?.worktreePath === orphanFullPath + ? orphanFullPath + : undefined + if (orphanWatcherPath) { + await this.acquireFileWatcherRemoval( + orphanWatcherPath, + orphanHost?.kind === 'ssh' ? orphanHost.targetId : undefined + ) + .then((gate) => gate.finish(false)) + .catch(() => {}) + } + this.clearOptimisticReconcileToken(removalTarget.id) + this.removeWorktreeMetadataAndHistory(store, removalTarget.id) + this.preservedBranchCleanupByWorktreeId.delete(removalTarget.id) + this.invalidateResolvedWorktreeCache() + this.invalidateWorktreeScanCacheForRepo(removalTarget.repoId) + invalidateAuthorizedRootsCache() + this.notifyWorktreesChanged(removalTarget.repoId) + // Why: non-desktop callers must be able to tell "forgotten" from "deleted"; nothing left the disk. + return { + warning: `Project ${removalTarget.repoId} is no longer tracked, so ${removalTarget.path} was forgotten without deleting the directory or its Git worktree registration.` + } } if (isFolderRepo(repo)) { if (removalTarget.id === getRuntimeFolderWorkspaceRootId(repo)) { @@ -26019,23 +26079,57 @@ export class OrcaRuntimeService { ptyId: string, stop: () => boolean | Promise ) => Promise<{ stopped: boolean; owner: boolean }> + /** Authoritative id for an orphan whose selector no longer resolves. */ + resolvedWorktreeId?: string + resolvedConnectionId?: string + resolvedRuntimeEnvironmentId?: string } = {} ): Promise<{ stopped: number }> { // Why: this mutates live PTYs, so reject while the graph is reloading rather than act on cached leaf ownership. const graphEpoch = this.captureReadyGraphEpoch() - const worktree = await this.resolveWorktreeSelector(worktreeSelector) + const worktree = options.resolvedWorktreeId + ? { id: options.resolvedWorktreeId } + : await this.resolveWorktreeSelector(worktreeSelector) this.assertStableReadyGraph(graphEpoch) if (options.deadline !== undefined && Date.now() >= options.deadline) { return { stopped: 0 } } + // Preserve folder-instance suffixes while normalizing cross-platform path spelling. + const parsedTarget = splitWorktreeId(worktree.id) + const ownsWorktree = options.resolvedWorktreeId + ? (candidate: string | undefined): boolean => { + if (!candidate) { + return false + } + const parsedCandidate = splitWorktreeId(candidate) + return parsedCandidate && parsedTarget + ? parsedCandidate.repoId === parsedTarget.repoId && + runtimePathsEqual(parsedCandidate.worktreePath, parsedTarget.worktreePath) + : candidate === worktree.id + } + : (candidate: string | undefined): boolean => candidate === worktree.id + const ownsHost = (ptyId: string, connectionId?: string | null): boolean => { + if (options.resolvedRuntimeEnvironmentId !== undefined) { + return ptyId.startsWith( + `remote:${encodeURIComponent(options.resolvedRuntimeEnvironmentId)}@@` + ) + } + return ( + options.resolvedConnectionId === undefined || connectionId === options.resolvedConnectionId + ) + } const ptyIds = new Set() for (const leaf of this.leaves.values()) { - if (leaf.worktreeId === worktree.id && leaf.ptyId) { + if ( + ownsWorktree(leaf.worktreeId) && + leaf.ptyId && + ownsHost(leaf.ptyId, this.ptysById.get(leaf.ptyId)?.connectionId) + ) { ptyIds.add(leaf.ptyId) } } for (const pty of this.ptysById.values()) { - if (pty.worktreeId === worktree.id && pty.connected) { + if (ownsWorktree(pty.worktreeId) && pty.connected && ownsHost(pty.ptyId, pty.connectionId)) { ptyIds.add(pty.ptyId) } } diff --git a/src/main/runtime/worktree-teardown.test.ts b/src/main/runtime/worktree-teardown.test.ts index eac1820a047..b51f292c29c 100644 --- a/src/main/runtime/worktree-teardown.test.ts +++ b/src/main/runtime/worktree-teardown.test.ts @@ -424,6 +424,56 @@ describe('killAllProcessesForWorktree', () => { expect(result.runtimeStopped).toBe(3) }) + it('forwards resolvedWorktreeId so an orphan sweep skips selector resolution', async () => { + const stopTerminalsForWorktree = vi.fn().mockResolvedValue({ stopped: 2 }) + const runtime = { + stopTerminalsForWorktree + } as unknown as Parameters[1]['runtime'] + + const localProvider = createProviderStub(async () => []) + listRegisteredPtysMock.mockReturnValue([]) + + const result = await killAllProcessesForWorktree('w1', { + runtime, + localProvider, + resolvedWorktreeId: 'w1' + }) + + // Without this the sweep resolves a selector whose repo is gone and throws selector_not_found. + expect(stopTerminalsForWorktree).toHaveBeenCalledWith('w1', { + deadline: expect.any(Number), + stopPty: expect.any(Function), + resolvedWorktreeId: 'w1' + }) + expect(result.runtimeStopped).toBe(2) + }) + + it('can restrict an orphan runtime sweep to its SSH connection', async () => { + const stopTerminalsForWorktree = vi.fn().mockResolvedValue({ stopped: 1 }) + const runtime = { + stopTerminalsForWorktree + } as unknown as Parameters[1]['runtime'] + const localProvider = createProviderStub(async () => []) + listRegisteredPtysMock.mockReturnValue([]) + + await killAllProcessesForWorktree('w1', { + runtime, + localProvider, + resolvedWorktreeId: 'w1', + resolvedConnectionId: 'ssh-1', + includeProviderInventory: false, + includeLocalRegistry: false + }) + + expect(stopTerminalsForWorktree).toHaveBeenCalledWith('w1', { + deadline: expect.any(Number), + stopPty: expect.any(Function), + resolvedWorktreeId: 'w1', + resolvedConnectionId: 'ssh-1' + }) + expect(localProvider.listProcesses).not.toHaveBeenCalled() + }) + it('claims duplicate provider and registry PTY ids only once', async () => { const localProvider = createProviderStub(async () => [ { id: 'w1@@same', cwd: '/tmp/w1', title: 'shell' } diff --git a/src/main/runtime/worktree-teardown.ts b/src/main/runtime/worktree-teardown.ts index 7f25d67937a..64b6a09703f 100644 --- a/src/main/runtime/worktree-teardown.ts +++ b/src/main/runtime/worktree-teardown.ts @@ -11,10 +11,17 @@ const WORKTREE_TEARDOWN_CONCURRENCY = 32 export type WorktreeTeardownDeps = { runtime?: OrcaRuntimeService + /** Authoritative id for callers whose selector no longer resolves (orphaned workspace). */ + resolvedWorktreeId?: string + /** SSH connection owning `resolvedWorktreeId`; prevents same-id cross-host graph matches. */ + resolvedConnectionId?: string + /** Runtime environment owning a mirrored `resolvedWorktreeId`. */ + resolvedRuntimeEnvironmentId?: string localProvider: IPtyProvider onPtyStopped?: (ptyId: string) => void timeoutMs?: number requirePhysicalStop?: boolean + includeProviderInventory?: boolean includeLocalRegistry?: boolean } @@ -62,11 +69,6 @@ export async function killAllProcessesForWorktree( worktreeId: string, deps: WorktreeTeardownDeps ): Promise { - const result: WorktreeTeardownResult = { - runtimeStopped: 0, - providerStopped: 0, - registryStopped: 0 - } const deadline = Date.now() + Math.max(1, deps.timeoutMs ?? WORKTREE_PROCESS_SWEEP_TIMEOUT_MS) const deadlineError = new Error(`Timed out waiting for physical PTY teardown: ${worktreeId}`) const stopAttempts = new Map>() @@ -97,7 +99,18 @@ export async function killAllProcessesForWorktree( // destructive removal closed. const runtimeSweep = deps.runtime ? settleBeforeDeadline( - () => deps.runtime!.stopTerminalsForWorktree(worktreeId, { deadline, stopPty }), + () => + deps.runtime!.stopTerminalsForWorktree(worktreeId, { + deadline, + stopPty, + ...(deps.resolvedWorktreeId ? { resolvedWorktreeId: deps.resolvedWorktreeId } : {}), + ...(deps.resolvedConnectionId + ? { resolvedConnectionId: deps.resolvedConnectionId } + : {}), + ...(deps.resolvedRuntimeEnvironmentId + ? { resolvedRuntimeEnvironmentId: deps.resolvedRuntimeEnvironmentId } + : {}) + }), { stopped: 0 }, deadline, deps.requirePhysicalStop ? deadlineError : undefined, @@ -108,20 +121,23 @@ export async function killAllProcessesForWorktree( ) ) : Promise.resolve({ stopped: 0 }) - const providerSweep = settleBeforeDeadline( - () => - sweepProviderByPrefix( - worktreeId, - deps.localProvider, - deadline, - stopPty, - deps.onPtyStopped, - deps.requirePhysicalStop - ), - 0, - deadline, - deps.requirePhysicalStop ? deadlineError : undefined - ) + const providerSweep = + deps.includeProviderInventory === false + ? Promise.resolve(0) + : settleBeforeDeadline( + () => + sweepProviderByPrefix( + worktreeId, + deps.localProvider, + deadline, + stopPty, + deps.onPtyStopped, + deps.requirePhysicalStop + ), + 0, + deadline, + deps.requirePhysicalStop ? deadlineError : undefined + ) const registrySweep = deps.includeLocalRegistry === false ? Promise.resolve(0) @@ -143,9 +159,6 @@ export async function killAllProcessesForWorktree( providerSweep, registrySweep ]) - result.runtimeStopped = runtimeResult.stopped - result.providerStopped = providerStopped - result.registryStopped = registryStopped if (deps.requirePhysicalStop) { const stopResults = await Promise.all( [...stopAttempts].map(async ([ptyId, stopped]) => [ptyId, await stopped] as const) @@ -162,7 +175,7 @@ export async function killAllProcessesForWorktree( } } - return result + return { runtimeStopped: runtimeResult.stopped, providerStopped, registryStopped } } async function verifyFailedPtysExited( diff --git a/src/main/stats/stats-snapshot-writer.ts b/src/main/stats/stats-snapshot-writer.ts index 593a554b636..f118a7ec5af 100644 --- a/src/main/stats/stats-snapshot-writer.ts +++ b/src/main/stats/stats-snapshot-writer.ts @@ -65,7 +65,10 @@ export class StatsSnapshotWriter { return this.pendingWrite ?? Promise.resolve() } - private preparePayload(finalPath: string, serialize: () => string): { + private preparePayload( + finalPath: string, + serialize: () => string + ): { tmpFile: string json: string generation: number diff --git a/src/main/worktree-removal-session-partition-fencing.test.ts b/src/main/worktree-removal-session-partition-fencing.test.ts new file mode 100644 index 00000000000..53b4dc805a6 --- /dev/null +++ b/src/main/worktree-removal-session-partition-fencing.test.ts @@ -0,0 +1,233 @@ +// Why this file exists: worktree removal writes to host session partitions, and the two hazards below +// are only visible across a removal followed by a renderer session write — persistence.test.ts covers +// removal and partitioning separately, so neither suite catches the interaction. +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { rmSync, mkdtempSync } from 'node:fs' +import { join } from 'node:path' +import { tmpdir } from 'node:os' +import type { TerminalTab } from '../shared/types' +import { getDefaultWorkspaceSession } from '../shared/constants' +import { toRuntimeExecutionHostId } from '../shared/execution-host' + +const testState = { dir: '' } + +vi.mock('./ssh/ssh-config-parser', () => ({ + loadUserSshConfig: vi.fn(), + sshConfigHostsToTargets: vi.fn() +})) + +vi.mock('electron', () => ({ + app: { + getPath: () => testState.dir + }, + safeStorage: { + isEncryptionAvailable: () => false, + encryptString: (plaintext: string) => Buffer.from(plaintext, 'utf-8'), + decryptString: (ciphertext: Buffer) => ciphertext.toString('utf-8') + } +})) + +vi.mock('./telemetry/client', () => ({ track: vi.fn() })) +vi.mock('./telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn().mockReturnValue({}) })) + +async function createStore() { + vi.resetModules() + const { Store, initDataPath } = await import('./persistence') + initDataPath() + return new Store() +} + +const makeTerminalTab = (overrides: Partial = {}): TerminalTab => ({ + id: 'tab1', + ptyId: 'pty1', + worktreeId: 'repo::/worktree', + title: 'Terminal', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1, + ...overrides +}) + +const ENV_A = toRuntimeExecutionHostId('env-a') +const STALE = 'repo-gone::/workspace/stale' +const LIVE = 'repo-gone::/workspace/live' + +describe('worktree removal across host session partitions', () => { + beforeEach(() => { + testState.dir = mkdtempSync(join(tmpdir(), 'orca-test-')) + }) + + afterEach(() => { + rmSync(testState.dir, { recursive: true, force: true }) + }) + + // Regression: removal fenced the never-written partition, so main's empty copy became authoritative + // and rebased away every sibling worktree's tabs on the first renderer write for that host. + it('keeps sibling tabs when a worktree is removed before its host partition was ever persisted', async () => { + const store = await createStore() + store.setWorktreeMeta(STALE, { hostId: ENV_A }) + + store.removeWorktreeMeta(STALE) + + store.setWorkspaceSession( + { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { [LIVE]: [makeTerminalTab({ id: 'live-tab', worktreeId: LIVE })] } + }, + ENV_A + ) + + expect(store.getWorkspaceSession(ENV_A).tabsByWorktree[LIVE]?.map((tab) => tab.id)).toEqual([ + 'live-tab' + ]) + }) + + it('does not materialize an unwritten host partition when removing a worktree', async () => { + const store = await createStore() + store.setWorktreeMeta(STALE, { hostId: ENV_A }) + + store.removeWorktreeMeta(STALE) + + expect(store.getWorkspaceSessionHostIds()).toEqual(['local']) + }) + + it('does not materialize an unwritten host partition when removing session state directly', async () => { + const store = await createStore() + + store.removeWorkspaceSessionStateForWorktree(STALE, ENV_A) + + expect(store.getWorkspaceSessionHostIds()).toEqual(['local']) + }) + + // A persisted partition must still be fenced so main's copy wins over the renderer's stale tabs. + it('still fences a host partition that was persisted before the removal', async () => { + const store = await createStore() + store.setWorktreeMeta(STALE, { hostId: ENV_A }) + store.setWorkspaceSession( + { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { [STALE]: [makeTerminalTab({ id: 'stale-tab', worktreeId: STALE })] } + }, + ENV_A + ) + + store.removeWorktreeMeta(STALE) + + expect(store.getWorkspaceSession(ENV_A).tabsByWorktree[STALE]).toBeUndefined() + expect(store.getWorkspaceSession(ENV_A).terminalTopologyRevisionByRepoId?.['repo-gone']).toBe(1) + + store.setWorkspaceSession( + { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { [STALE]: [makeTerminalTab({ id: 'stale-tab', worktreeId: STALE })] } + }, + ENV_A + ) + expect(store.getWorkspaceSession(ENV_A).tabsByWorktree[STALE]).toEqual([]) + }) + + // Regression: runtime removal purged only the host partition, so tabs the renderer had parked in the + // local blob (its fallback whenever worktree ownership is unresolved) leaked forever. + it('purges a runtime-owned worktree from the local blob as well as its host partition', async () => { + const store = await createStore() + store.setWorktreeMeta(STALE, { hostId: ENV_A }) + store.setWorkspaceSession({ + ...getDefaultWorkspaceSession(), + tabsByWorktree: { + [STALE]: [makeTerminalTab({ id: 'local-fallback-tab', worktreeId: STALE })] + } + }) + store.setWorkspaceSession( + { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { [STALE]: [makeTerminalTab({ id: 'host-tab', worktreeId: STALE })] } + }, + ENV_A + ) + + store.removeWorktreeMeta(STALE) + + expect(store.getWorkspaceSession().tabsByWorktree[STALE]).toBeUndefined() + expect(store.getWorkspaceSession(ENV_A).tabsByWorktree[STALE]).toBeUndefined() + }) + + // Regression: an empty local blob counted as "no sibling to rebase", so a runtime removal claimed + // repo-wide authority there and the renderer's next ownership-unresolved write lost its tabs. + it('does not fence the local blob for a runtime removal it never held tabs for', async () => { + const store = await createStore() + store.setWorktreeMeta(STALE, { hostId: ENV_A }) + store.setWorkspaceSession( + { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { [STALE]: [makeTerminalTab({ id: 'host-tab', worktreeId: STALE })] } + }, + ENV_A + ) + + store.removeWorktreeMeta(STALE) + + store.setWorkspaceSession({ + ...getDefaultWorkspaceSession(), + tabsByWorktree: { [LIVE]: [makeTerminalTab({ id: 'live-tab', worktreeId: LIVE })] } + }) + + expect(store.getWorkspaceSession().tabsByWorktree[LIVE]?.map((tab) => tab.id)).toEqual([ + 'live-tab' + ]) + expect( + store.getWorkspaceSession().terminalTopologyRevisionByRepoId?.['repo-gone'] + ).toBeUndefined() + }) + + // The owner's own partition still fences on emptiness: nothing of the repo lives there to rebase. + it('fences the owning partition that holds no tabs for the removed worktree', async () => { + const store = await createStore() + const otherRepoWorktree = 'repo-other::/workspace/live' + store.setWorktreeMeta(STALE, { hostId: ENV_A }) + store.setWorkspaceSession( + { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { + [otherRepoWorktree]: [ + makeTerminalTab({ id: 'other-repo-tab', worktreeId: otherRepoWorktree }) + ] + } + }, + ENV_A + ) + + store.removeWorktreeMeta(STALE) + + expect(store.getWorkspaceSession(ENV_A).terminalTopologyRevisionByRepoId?.['repo-gone']).toBe(1) + + store.setWorkspaceSession( + { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { [STALE]: [makeTerminalTab({ id: 'delayed-tab', worktreeId: STALE })] } + }, + ENV_A + ) + + expect(store.getWorkspaceSession(ENV_A).tabsByWorktree[STALE]).toEqual([]) + }) + + // The widened local purge must not fence the local blob when only siblings live there. + it('does not fence the local blob that holds only sibling worktrees of the removed runtime worktree', async () => { + const store = await createStore() + store.setWorktreeMeta(STALE, { hostId: ENV_A }) + store.setWorkspaceSession({ + ...getDefaultWorkspaceSession(), + tabsByWorktree: { [LIVE]: [makeTerminalTab({ id: 'live-tab', worktreeId: LIVE })] } + }) + + store.removeWorktreeMeta(STALE) + + expect( + store.getWorkspaceSession().terminalTopologyRevisionByRepoId?.['repo-gone'] + ).toBeUndefined() + expect(store.getWorkspaceSession().tabsByWorktree[LIVE]?.map((tab) => tab.id)).toEqual([ + 'live-tab' + ]) + }) +}) diff --git a/src/renderer/src/runtime/runtime-rpc-client.ts b/src/renderer/src/runtime/runtime-rpc-client.ts index 1087c3a5a18..57ba17b75a0 100644 --- a/src/renderer/src/runtime/runtime-rpc-client.ts +++ b/src/renderer/src/runtime/runtime-rpc-client.ts @@ -14,7 +14,11 @@ export { settingsForRuntimeOwner, type RuntimeClientTarget } from './runtime-client-target' -export { RuntimeRpcCallError, unwrapRuntimeRpcResult } from './runtime-rpc-result' +export { + hasRuntimeRpcErrorCode, + RuntimeRpcCallError, + unwrapRuntimeRpcResult +} from './runtime-rpc-result' const RUNTIME_COMPATIBILITY_CACHE_MAX = 32 const RECENT_RUNTIME_COMPATIBILITY_FAILURE_TTL_MS = 60_000 diff --git a/src/renderer/src/runtime/runtime-rpc-result.test.ts b/src/renderer/src/runtime/runtime-rpc-result.test.ts new file mode 100644 index 00000000000..f63cb1fb7b0 --- /dev/null +++ b/src/renderer/src/runtime/runtime-rpc-result.test.ts @@ -0,0 +1,117 @@ +import { describe, expect, it } from 'vitest' +import { hasRuntimeRpcErrorCode, RuntimeRpcCallError } from './runtime-rpc-result' + +describe('hasRuntimeRpcErrorCode', () => { + it('matches structured runtime failures through wrapped causes', () => { + const failure = new RuntimeRpcCallError({ + id: 'rpc-1', + ok: false, + error: { code: 'selector_not_found', message: 'Selector not found' } + }) + + expect( + hasRuntimeRpcErrorCode(new Error('remove failed', { cause: failure }), failure.code) + ).toBe(true) + }) + + it('supports legacy response envelopes and exact plain errors', () => { + expect( + hasRuntimeRpcErrorCode( + { response: { error: { message: 'repo_not_found' } } }, + 'repo_not_found' + ) + ).toBe(true) + expect(hasRuntimeRpcErrorCode('repo_not_found', 'repo_not_found')).toBe(true) + }) + + // Why: Electron IPC and relay envelopes re-wrap the token into a longer message and drop the cause. + it('matches codes re-wrapped into a transport message with no cause', () => { + expect( + hasRuntimeRpcErrorCode( + new Error("Error invoking remote method 'worktrees:updateMeta': Error: selector_not_found"), + 'selector_not_found' + ) + ).toBe(true) + expect(hasRuntimeRpcErrorCode('Error: selector_not_found', 'selector_not_found')).toBe(true) + expect( + hasRuntimeRpcErrorCode( + { message: 'relay call failed: selector_not_found\n' }, + 'selector_not_found' + ) + ).toBe(true) + expect( + hasRuntimeRpcErrorCode( + new Error('worktree.set failed', { + cause: new Error('Error invoking remote method: Error: selector_not_found') + }), + 'selector_not_found' + ) + ).toBe(true) + }) + + // Why: the destructive consumer forgets the workspace locally, so only a real message boundary may classify. + it('rejects a trailing token that is not after a message boundary', () => { + expect( + hasRuntimeRpcErrorCode(new Error('failed to reach host for repo_not_found'), 'repo_not_found') + ).toBe(false) + expect( + hasRuntimeRpcErrorCode( + new Error('remote is unreachable, selector_not_found'), + 'selector_not_found' + ) + ).toBe(false) + expect( + hasRuntimeRpcErrorCode({ message: 'timed out waiting for repo_not_found' }, 'repo_not_found') + ).toBe(false) + expect( + hasRuntimeRpcErrorCode(new Error('host offline (selector_not_found'), 'selector_not_found') + ).toBe(false) + }) + + it('rejects an unrelated failure re-wrapped around a token-shaped tail', () => { + expect( + hasRuntimeRpcErrorCode( + new Error('worktree.rm failed while probing selector_not_found', { + cause: new Error('connection reset by peer while resolving repo_not_found') + }), + 'selector_not_found' + ) + ).toBe(false) + expect( + hasRuntimeRpcErrorCode( + { + response: { + error: { code: 'permission_denied', message: 'denied lookup of repo_not_found' } + } + }, + 'repo_not_found' + ) + ).toBe(false) + }) + + it('matches a token placed on its own line by a multi-line transport message', () => { + expect( + hasRuntimeRpcErrorCode( + new Error('relay envelope rejected the call\nselector_not_found'), + 'selector_not_found' + ) + ).toBe(true) + }) + + it('rejects diagnostic mentions and cyclic causes', () => { + const cycle: { cause?: unknown; message: string } = { message: 'permission_denied' } + cycle.cause = cycle + + expect( + hasRuntimeRpcErrorCode( + new Error('Access denied after a prior selector_not_found diagnostic'), + 'selector_not_found' + ) + ).toBe(false) + expect(hasRuntimeRpcErrorCode(cycle, 'selector_not_found')).toBe(false) + // A longer identifier that merely ends in the token is a different code. + expect( + hasRuntimeRpcErrorCode(new Error('stale_selector_not_found'), 'selector_not_found') + ).toBe(false) + }) +}) diff --git a/src/renderer/src/runtime/runtime-rpc-result.ts b/src/renderer/src/runtime/runtime-rpc-result.ts index e342c80c5bb..c168b3ab116 100644 --- a/src/renderer/src/runtime/runtime-rpc-result.ts +++ b/src/renderer/src/runtime/runtime-rpc-result.ts @@ -12,6 +12,55 @@ export class RuntimeRpcCallError extends Error { } } +// Why: transports re-wrap the token into a longer message and drop the cause (Electron IPC's +// "Error invoking remote method 'x': Error: selector_not_found", relay envelope re-throws), so the +// token classifies only as the whole message or after a real message boundary (": " or a newline) — +// prose that merely trails off in the token must not reach the destructive forget-local fallback. +const CODE_TOKEN_BOUNDARY = /(?:: |\n)[ \t]*$/ + +function endsWithCodeToken(text: string, expectedCode: string): boolean { + const trimmed = text.trimEnd() + if (!trimmed.endsWith(expectedCode)) { + return false + } + const prefix = trimmed.slice(0, -expectedCode.length) + return prefix.trim() === '' || CODE_TOKEN_BOUNDARY.test(prefix) +} + +export function hasRuntimeRpcErrorCode(error: unknown, expectedCode: string): boolean { + const seen = new Set() + let current = error + while (!seen.has(current)) { + if (typeof current === 'string') { + return endsWithCodeToken(current, expectedCode) + } + if (!current || typeof current !== 'object') { + return false + } + seen.add(current) + const candidate = current as { + cause?: unknown + code?: unknown + message?: unknown + response?: { error?: { code?: unknown; message?: unknown } } + } + // Machine tokens are checked before messages so a subclass with a human-readable message still classifies by code. + if (candidate.code === expectedCode || candidate.response?.error?.code === expectedCode) { + return true + } + const messages = [candidate.message, candidate.response?.error?.message] + if ( + messages.some( + (message) => typeof message === 'string' && endsWithCodeToken(message, expectedCode) + ) + ) { + return true + } + current = candidate.cause + } + return false +} + export function unwrapRuntimeRpcResult(response: RuntimeRpcResponse): TResult { if (response.ok === false) { throw new RuntimeRpcCallError(response) diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index 443032a5e4d..1b670957ddb 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -717,6 +717,8 @@ export type TerminalSlice = { shutdownReason?: AgentStatusWorktreeShutdownReason sleepingPaneKeys?: string[] expectedRuntimePtyIds?: string[] + /** Opt-in for callers that already tore the workspace down backend-side; omitting it keeps the runtime stop. */ + backendOwnsPtyTeardown?: boolean } ) => Promise shutdownCompletedAgentPaneForHibernation: ( @@ -3145,7 +3147,8 @@ export const createTerminalSlice: StateCreator markShutdownPending() handlerSnapshots = unregisterPtyDataHandlers(rendererShutdownPtyIds) ?? [] try { - if (runtimeEnvironmentId) { + // Why an opt-in, not the reason: defaulting to the stop keeps callers without backend teardown from leaking remote PTYs. + if (runtimeEnvironmentId && opts?.backendOwnsPtyTeardown !== true) { await (shutdownReason === 'manual-sleep' ? requestRemoteWorktreeSleep({ environmentId: runtimeEnvironmentId, @@ -3173,6 +3176,7 @@ export const createTerminalSlice: StateCreator } } + // Exact-stop callers own physical teardown and must not use the remove-worktree reason. if (expectedRuntimePtyIds.length > 0) { if (!runtimeEnvironmentId) { throw new Error('missing_runtime_for_exact_terminal_stop') diff --git a/src/renderer/src/store/slices/worktree-terminal-removal-teardown.test.ts b/src/renderer/src/store/slices/worktree-terminal-removal-teardown.test.ts new file mode 100644 index 00000000000..8cee0962adb --- /dev/null +++ b/src/renderer/src/store/slices/worktree-terminal-removal-teardown.test.ts @@ -0,0 +1,112 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const unregisterPtyDataHandlers = vi.hoisted(() => vi.fn(() => [])) + +vi.mock('sonner', () => ({ + toast: { info: vi.fn(), success: vi.fn(), error: vi.fn(), warning: vi.fn() } +})) + +vi.mock('@/components/terminal-pane/pty-dispatcher', () => ({ + restorePtyDataHandlersAfterFailedShutdown: vi.fn(), + unregisterPtyDataHandlers +})) + +const runtimeCall = vi.fn() +const killPty = vi.fn().mockResolvedValue(undefined) + +globalThis.window = { + api: { + pty: { kill: killPty }, + runtimeEnvironments: { call: runtimeCall } + } +} as never + +import { getDefaultSettings } from '../../../../shared/constants' +import { createCompatibleRuntimeStatusResponseIfNeeded } from '../../runtime/runtime-compatibility-test-fixture' +import { clearRuntimeCompatibilityCacheForTests } from '../../runtime/runtime-rpc-client' +import { createTestStore, makeRuntimeOwnedWorktree, makeTab, seedStore } from './store-test-helpers' + +const worktreeId = 'repo1::/srv/worktree' +const ptyId = 'remote:env-1@@pty-1' + +function seedRuntimeOwnedWorktree(store: ReturnType): void { + seedStore(store, { + settings: { ...getDefaultSettings('/tmp'), activeRuntimeEnvironmentId: 'env-1' }, + worktreesByRepo: { + repo1: [ + makeRuntimeOwnedWorktree( + { id: worktreeId, repoId: 'repo1', path: '/srv/worktree' }, + 'env-1' + ) + ] + }, + tabsByWorktree: { + [worktreeId]: [makeTab({ id: 'tab-1', worktreeId, ptyId })] + }, + ptyIdsByTabId: { 'tab-1': [ptyId] } + }) +} + +describe('worktree terminal removal teardown', () => { + beforeEach(() => { + vi.clearAllMocks() + clearRuntimeCompatibilityCacheForTests() + runtimeCall.mockImplementation((args: { method: string }) => + Promise.resolve( + createCompatibleRuntimeStatusResponseIfNeeded(args) ?? { + id: 'rpc-default', + ok: true, + result: {}, + _meta: { runtimeId: 'remote-runtime' } + } + ) + ) + }) + + it('retires renderer bindings without stopping the already-removed remote workspace again', async () => { + const store = createTestStore() + seedRuntimeOwnedWorktree(store) + + await store.getState().shutdownWorktreeTerminals(worktreeId, { + shutdownReason: 'remove-worktree', + backendOwnsPtyTeardown: true + }) + + expect(runtimeCall.mock.calls.filter(([args]) => args.method === 'terminal.stop')).toHaveLength( + 0 + ) + expect(killPty).not.toHaveBeenCalled() + expect(store.getState().ptyIdsByTabId['tab-1']).toEqual([]) + expect(store.getState().pendingPtyShutdownIds[ptyId]).toBeUndefined() + }) + + // Why: the skip is an opt-in for backend-paired removal; a bare call must still kill remote PTYs or they leak with no local trace. + it('still stops remote terminals when the caller passes no options', async () => { + const store = createTestStore() + seedRuntimeOwnedWorktree(store) + + await store.getState().shutdownWorktreeTerminals(worktreeId) + + expect(runtimeCall).toHaveBeenCalledWith( + expect.objectContaining({ + selector: 'env-1', + method: 'terminal.stop', + params: { worktree: `id:${worktreeId}` } + }) + ) + expect(store.getState().ptyIdsByTabId['tab-1']).toEqual([]) + }) + + it('still stops remote terminals for an explicit remove reason without the opt-in', async () => { + const store = createTestStore() + seedRuntimeOwnedWorktree(store) + + await store + .getState() + .shutdownWorktreeTerminals(worktreeId, { shutdownReason: 'remove-worktree' }) + + expect(runtimeCall.mock.calls.filter(([args]) => args.method === 'terminal.stop')).toHaveLength( + 1 + ) + }) +}) diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index cda7f0de246..595e0828c92 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -114,6 +114,7 @@ const mockApi = { cancelListDetected: vi.fn().mockResolvedValue(undefined), listLineage: vi.fn().mockResolvedValue({}), remove: vi.fn().mockResolvedValue(undefined), + forgetLocal: vi.fn().mockResolvedValue({}), forceDeletePreservedBranch: vi.fn().mockResolvedValue({ deleted: true }), resolvePrBase: vi.fn(), resolveMrBase: vi.fn(), @@ -5437,6 +5438,10 @@ describe('worktree remote runtime mutations', () => { timeoutMs: 60_000 }) expect(mockApi.worktrees.remove).not.toHaveBeenCalled() + expect(store.getState().shutdownWorktreeTerminals).toHaveBeenCalledWith(wt.id, { + shutdownReason: 'remove-worktree', + backendOwnsPtyTeardown: true + }) expect(store.getState().worktreesByRepo.repo1).toEqual([]) }) @@ -5504,6 +5509,108 @@ describe('worktree remote runtime mutations', () => { expect(mockApi.worktrees.remove).not.toHaveBeenCalled() }) + it.each(['repo_not_found', 'selector_not_found'])( + 'forgets a mirrored row when the remote returns %s', + async (errorCode) => { + const store = createTestStore() + const wt = makeWorktree({ + id: 'repo-gone::/path/stale', + repoId: 'repo-gone', + path: '/path/stale', + hostId: 'runtime:env-1' + }) + runtimeEnvironmentCall.mockResolvedValue({ + id: 'rpc-rm', + ok: false, + error: { code: errorCode, message: errorCode }, + _meta: { runtimeId: 'runtime-remote' } + }) + store.setState({ + settings: { activeRuntimeEnvironmentId: 'env-1' } as never, + worktreesByRepo: { 'repo-gone': [wt] } + } as Partial) + + const result = await store.getState().removeWorktree(wt.id) + + expect(result).toEqual({ ok: true }) + expect(mockApi.worktrees.forgetLocal).toHaveBeenCalledWith({ + worktreeId: wt.id, + hostId: 'runtime:env-1' + }) + expect(store.getState().worktreesByRepo['repo-gone']).toEqual([]) + } + ) + + it('does not forget a row that becomes ambiguous while remote removal is in flight', async () => { + const store = createTestStore() + const worktreeId = 'repo-shared::/path/stale' + const original = makeWorktree({ + id: worktreeId, + repoId: 'repo-shared', + hostId: 'runtime:env-1' + }) + const rival = makeWorktree({ + id: worktreeId, + repoId: 'repo-shared', + hostId: 'runtime:env-2' + }) + runtimeEnvironmentCall.mockImplementationOnce(async () => { + store.setState({ worktreesByRepo: { 'repo-shared': [original, rival] } }) + return { + id: 'rpc-rm', + ok: false, + error: { code: 'selector_not_found', message: 'selector_not_found' }, + _meta: { runtimeId: 'runtime-remote' } + } + }) + store.setState({ + settings: { activeRuntimeEnvironmentId: 'env-1' } as never, + trustedOrcaHooks: { 'repo-shared': { all: { approvedAt: 1 } } }, + worktreesByRepo: { 'repo-shared': [original] } + } as Partial) + + const result = await store.getState().removeWorktree(worktreeId) + + expect(result).toEqual({ + ok: false, + error: 'selector_not_found' + }) + expect(mockApi.worktrees.forgetLocal).not.toHaveBeenCalled() + expect(store.getState().worktreesByRepo['repo-shared']).toEqual([original, rival]) + }) + + it('does not forget a mirrored row when diagnostics merely mention a missing code', async () => { + const store = createTestStore() + const wt = makeWorktree({ + id: 'repo1::/path/wt1', + repoId: 'repo1', + path: '/path/wt1', + hostId: 'runtime:env-1' + }) + runtimeEnvironmentCall.mockResolvedValue({ + id: 'rpc-rm', + ok: false, + error: { + code: 'permission_denied', + message: 'Access denied while checking a prior repo_not_found diagnostic.' + }, + _meta: { runtimeId: 'runtime-remote' } + }) + store.setState({ + settings: { activeRuntimeEnvironmentId: 'env-1' } as never, + worktreesByRepo: { repo1: [wt] } + } as Partial) + + const result = await store.getState().removeWorktree(wt.id) + + expect(result).toEqual({ + ok: false, + error: 'Access denied while checking a prior repo_not_found diagnostic.' + }) + expect(mockApi.worktrees.forgetLocal).not.toHaveBeenCalled() + expect(store.getState().worktreesByRepo.repo1).toEqual([wt]) + }) + it('removes SSH-owned worktrees through local IPC even when a runtime is focused', async () => { const store = createTestStore() const wt = makeWorktree({ @@ -6655,6 +6762,68 @@ describe('worktree remote runtime mutations', () => { } }) + // Why: Electron IPC re-wraps the main-process message and drops the cause, so the quiet + // "row is gone" handling must survive a transport that only leaves the token in the text. + it('stays quiet when a transport-wrapped selector miss rejects the activity write', async () => { + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + const store = createTestStore() + const wt = makeWorktree({ id: 'repo1::/path/wt1', repoId: 'repo1', path: '/path/wt1' }) + mockApi.worktrees.updateMeta.mockRejectedValueOnce( + new Error("Error invoking remote method 'worktrees:updateMeta': Error: selector_not_found") + ) + store.setState({ worktreesByRepo: { repo1: [wt] } } as Partial) + + try { + store.getState().bumpWorktreeActivity(wt.id) + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(mockApi.worktrees.updateMeta).toHaveBeenCalled() + expect(errorSpy).not.toHaveBeenCalled() + expect(worktreeListMock).not.toHaveBeenCalled() + } finally { + errorSpy.mockRestore() + } + }) + + it.each([ + [ + 'updateWorktreeMeta', + (store: ReturnType, worktreeId: string) => + store.getState().updateWorktreeMeta(worktreeId, { isUnread: true }) + ], + [ + 'updateWorktreesMeta', + (store: ReturnType, worktreeId: string) => + store.getState().updateWorktreesMeta(new Map([[worktreeId, { isUnread: true }]])) + ], + [ + 'markWorktreeUnread', + async (store: ReturnType, worktreeId: string) => { + store.getState().markWorktreeUnread(worktreeId) + await new Promise((resolve) => setTimeout(resolve, 0)) + } + ] + ])('swallows a transport-wrapped selector miss in %s', async (_name, run) => { + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + const store = createTestStore() + const wt = makeWorktree({ id: 'repo1::/path/wt1', repoId: 'repo1', path: '/path/wt1' }) + mockApi.worktrees.updateMeta.mockRejectedValue( + new Error("Error invoking remote method 'worktrees:updateMeta': Error: selector_not_found") + ) + store.setState({ worktreesByRepo: { repo1: [wt] } } as Partial) + + try { + await run(store, wt.id) + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(mockApi.worktrees.updateMeta).toHaveBeenCalled() + expect(errorSpy).not.toHaveBeenCalled() + } finally { + errorSpy.mockRestore() + mockApi.worktrees.updateMeta.mockReset().mockResolvedValue({}) + } + }) + it('does not persist activity for a missing worktree', async () => { const store = createTestStore() store.setState({ diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index cf0474554a1..b80d5d4e87b 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -46,6 +46,7 @@ import { callRuntimeRpc, assertRuntimeEnvironmentCapability, getActiveRuntimeTarget, + hasRuntimeRpcErrorCode, isRuntimeScopeForbiddenError, RuntimeRpcCallError } from '../../runtime/runtime-rpc-client' @@ -84,6 +85,7 @@ import { import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' import { resolveWorktreeOperationRoute, + resolveWorktreeOperationRouteResult, settingsForWorktreeOperationRoute } from '@/lib/worktree-operation-route' import { captureWorktreeOperationGenerationGuard } from '@/lib/worktree-operation-generation' @@ -951,46 +953,11 @@ function getFolderWorkspaceMetaUpdates( } function isRuntimeSelectorNotFoundError(error: unknown): boolean { - if ( - error && - typeof error === 'object' && - 'cause' in error && - isRuntimeSelectorNotFoundError((error as { cause?: unknown }).cause) - ) { - return true - } - const code = - error && - typeof error === 'object' && - 'code' in error && - typeof (error as { code: unknown }).code === 'string' - ? (error as { code: string }).code - : null - const responseCode = - error && - typeof error === 'object' && - 'response' in error && - typeof (error as { response?: { error?: { code?: unknown } } }).response?.error?.code === - 'string' - ? (error as { response: { error: { code: string } } }).response.error.code - : null - const responseMessage = - error && - typeof error === 'object' && - 'response' in error && - typeof (error as { response?: { error?: { message?: unknown } } }).response?.error?.message === - 'string' - ? (error as { response: { error: { message: string } } }).response.error.message - : null - const message = error instanceof Error ? error.message : String(error) - return ( - message === 'selector_not_found' || - message.includes('selector_not_found') || - code === 'selector_not_found' || - responseCode === 'selector_not_found' || - responseMessage === 'selector_not_found' || - String(error).includes('selector_not_found') - ) + return hasRuntimeRpcErrorCode(error, 'selector_not_found') +} + +function isRuntimeRepoNotFoundError(error: unknown): boolean { + return hasRuntimeRpcErrorCode(error, 'repo_not_found') } function replaceWorktreeInRepoLists( @@ -4008,22 +3975,51 @@ export const createWorktreeSlice: StateCreator ? { ...get().settings, activeRuntimeEnvironmentId: null } : { activeRuntimeEnvironmentId: null } ) - const removalResult = await (forgetLocalOnly - ? window.api.worktrees.forgetLocal({ worktreeId, hostId }) - : target.kind === 'local' - ? (removalGenerationGuard?.assertCurrent(), - window.api.worktrees.remove({ worktreeId, hostId, force, skipArchive })) - : (removalGenerationGuard?.assertCurrent(), - callRuntimeRpc( - target, - 'worktree.rm', - { - worktree: toRuntimeWorktreeSelector(worktreeId), - force, - runHooks: !skipArchive - }, - { timeoutMs: 60_000 } - ))) + let removalResult: RemoveWorktreeResult + try { + removalResult = await (forgetLocalOnly + ? window.api.worktrees.forgetLocal({ worktreeId, hostId }) + : target.kind === 'local' + ? (removalGenerationGuard?.assertCurrent(), + window.api.worktrees.remove({ worktreeId, hostId, force, skipArchive })) + : (removalGenerationGuard?.assertCurrent(), + callRuntimeRpc( + target, + 'worktree.rm', + { + worktree: toRuntimeWorktreeSelector(worktreeId), + force, + runHooks: !skipArchive + }, + { timeoutMs: 60_000 } + ))) + } catch (error) { + if ( + !forgetLocalOnly && + target.kind !== 'local' && + (isRuntimeRepoNotFoundError(error) || isRuntimeSelectorNotFoundError(error)) + ) { + // Missing means stale mirror; ambiguous or changed ownership must fail closed. + const currentResolution = resolveWorktreeOperationRouteResult(get(), worktreeId) + if (currentResolution.kind === 'ambiguous') { + throw error + } + if (currentResolution.kind === 'resolved') { + removalGenerationGuard?.assertCurrent() + } + try { + removalResult = await window.api.worktrees.forgetLocal({ worktreeId, hostId }) + } catch (fallbackError) { + // Preserve the remote verdict as fallback failure context. + throw new Error( + fallbackError instanceof Error ? fallbackError.message : String(fallbackError), + { cause: error } + ) + } + } else { + throw error + } + } // Why: invalidate stale probes once deletion is authoritative, so an old toast can't mutate a same-path replacement. forgetHugeRepoWarningDismissalsForWorktrees([worktreeId]) @@ -4044,7 +4040,11 @@ export const createWorktreeSlice: StateCreator // Why: renderer state follows the successful backend result, so blocked dirty deletes keep their terminals intact. // Why browsers first: unregister Chromium guests before other teardown can intercept them (avoids a browser-state race). await get().shutdownWorktreeBrowsers(worktreeId) - await get().shutdownWorktreeTerminals(worktreeId) + await get().shutdownWorktreeTerminals(worktreeId, { + shutdownReason: 'remove-worktree', + // The backend removal above already killed the workspace's PTYs. + backendOwnsPtyTeardown: true + }) // Why: dispose the SSH relay AFTER terminal teardown so a still-mounted pane can't hit a gone relay and toast "SSH not active". const destroyedRuntimeSshTargetIds = await cleanupEphemeralVmRuntimesForDeleted({ workspaceIds: [worktreeId]