diff --git a/src/main/runtime/rpc/methods/worktree-rm-host-qualification.test.ts b/src/main/runtime/rpc/methods/worktree-rm-host-qualification.test.ts index c706c3a05be..25ecd00a471 100644 --- a/src/main/runtime/rpc/methods/worktree-rm-host-qualification.test.ts +++ b/src/main/runtime/rpc/methods/worktree-rm-host-qualification.test.ts @@ -82,4 +82,41 @@ describe('worktree.rm host qualification', () => { }) expect(runtime.removeManagedWorktree).not.toHaveBeenCalled() }) + // Why: a delete legitimately arrives after Git stops listing the path (the + // caller is clearing a stale row). Pre-resolving the host must not turn that + // into a hard failure for a client that sends no hostId — mobile could delete + // such a row before host qualification and must still be able to. + it('still deletes a stale workspace for an old client once Git no longer lists it', async () => { + const runtime = makeRuntime() + vi.mocked(runtime.showManagedWorktree).mockRejectedValue(new Error('selector_not_found')) + const dispatcher = new RpcDispatcher({ runtime, methods: WORKTREE_METHODS }) + + const response = await dispatcher.dispatch( + makeRequest({ worktree: 'id:wt-gone', force: true, runHooks: false }) + ) + + expect(response).toMatchObject({ ok: true, result: { removed: true } }) + // Unqualified on purpose: removeManagedWorktree owns the stale-row path and + // still refuses on its own if the id turns out to have two owners. + expect(runtime.removeManagedWorktree).toHaveBeenCalledWith( + 'id:wt-gone', + true, + false, + false, + undefined + ) + }) + + it('propagates a non-missing lookup failure instead of deleting unqualified', async () => { + const runtime = makeRuntime() + vi.mocked(runtime.showManagedWorktree).mockRejectedValue(new Error('selector_ambiguous')) + const dispatcher = new RpcDispatcher({ runtime, methods: WORKTREE_METHODS }) + + const response = await dispatcher.dispatch( + makeRequest({ worktree: 'id:wt-1', force: true, runHooks: false }) + ) + + expect(response).toMatchObject({ ok: false, error: { message: 'selector_ambiguous' } }) + expect(runtime.removeManagedWorktree).not.toHaveBeenCalled() + }) }) diff --git a/src/main/runtime/rpc/methods/worktree.ts b/src/main/runtime/rpc/methods/worktree.ts index 5630868365b..2dd7ef3f46f 100644 --- a/src/main/runtime/rpc/methods/worktree.ts +++ b/src/main/runtime/rpc/methods/worktree.ts @@ -201,10 +201,25 @@ export const WORKTREE_METHODS: RpcMethod[] = [ name: 'worktree.rm', params: WorktreeRemove, handler: async (params, { runtime }) => { - // Older mobile clients omit hostId, so resolve through the ambiguity gate before pinning removal. - const hostId = params.hostId ?? (await runtime.showManagedWorktree(params.worktree)).hostId - if (!hostId) { - throw new Error('worktree.rm could not resolve the workspace host') + // Older mobile clients omit hostId, so resolve through the ambiguity gate + // before pinning removal. An ambiguous selector still fails closed: two + // hosts own the id and an unqualified client cannot say which it meant. + let resolvedHostId = params.hostId + if (!resolvedHostId) { + try { + resolvedHostId = (await runtime.showManagedWorktree(params.worktree)).hostId + if (!resolvedHostId) { + throw new Error('worktree.rm could not resolve the workspace host') + } + } catch (error) { + // 'selector_not_found' is not a failure to attribute — Git simply no + // longer lists the path. A delete legitimately arrives in that state and + // `removeManagedWorktree` handles it, so a stale workspace stays + // deletable by a client that sends no host. Anything else propagates. + if (!(error instanceof Error) || error.message !== 'selector_not_found') { + throw error + } + } } const removalArgs = [ params.worktree, @@ -212,7 +227,7 @@ export const WORKTREE_METHODS: RpcMethod[] = [ params.runHooks === true, params.allowUnverifiedPtyStop === true ] as const - const result = await runtime.removeManagedWorktree(...removalArgs, hostId) + const result = await runtime.removeManagedWorktree(...removalArgs, resolvedHostId) return { removed: true, ...result } } }), diff --git a/src/renderer/src/store/slices/worktrees/teardown/host-qualified-worktree-removal.ts b/src/renderer/src/store/slices/worktrees/teardown/host-qualified-worktree-removal.ts index 544bd8beec3..ad46ef61455 100644 --- a/src/renderer/src/store/slices/worktrees/teardown/host-qualified-worktree-removal.ts +++ b/src/renderer/src/store/slices/worktrees/teardown/host-qualified-worktree-removal.ts @@ -23,6 +23,8 @@ import { WORKTREE_REMOVAL_HOST_CHANGED_ERROR } from '../listing/worktree-slice-constants' import { requestVirtualizedScrollAnchorRecord } from '@/hooks/requestVirtualizedScrollAnchorRecord' +import { cleanupEphemeralVmRuntimesForDeleted } from '@/lib/ephemeral-vm-runtime-cleanup' +import { purgeOrphanedRuntimeSshProjects } from './orphaned-runtime-ssh-project-purge' import { showPreservedBranchToast } from '@/components/sidebar/preserved-branch-toast' import { preservedBranchCleanupKey } from '../../../../../../shared/preserved-branch-cleanup' @@ -199,8 +201,15 @@ function dropConfirmedHostRow( /** * Finish a removal whose id still exists on another host: prune just the * confirmed host's row and keep the preserved-branch follow-up pinned to it. + * + * Tears down the ephemeral VM even though the shared renderer state survives. A + * VM is NOT shared: it belongs to the workspace just deleted. The likeliest real + * collision is one machine reachable as both `runtime:env` and `ssh:target`, so + * skipping it left the VM running and billing after its row was removed. Safe to + * do here without the full path's ordering care, because this path tears down no + * terminals — there is no still-mounted pane to race a disposed relay. */ -export function completeSameIdHostScopedRemoval(args: { +export async function completeSameIdHostScopedRemoval(args: { set: WorktreeSliceSet get: WorktreeSliceGet worktreeId: string @@ -210,7 +219,7 @@ export function completeSameIdHostScopedRemoval(args: { target: ReturnType worktreeBeforeRemoval: PreservedBranchWorktree suppressPreservedBranchToast: boolean -}): RemoveWorktreeSliceResult { +}): Promise> { const { set, get, @@ -222,6 +231,10 @@ export function completeSameIdHostScopedRemoval(args: { worktreeBeforeRemoval, suppressPreservedBranchToast } = args + const runtimeCleanup = await cleanupEphemeralVmRuntimesForDeleted({ + workspaceIds: [worktreeId] + }) + await purgeOrphanedRuntimeSshProjects(get, runtimeCleanup.destroyedSshTargetIds) dropConfirmedHostRow(set, worktreeId, requiredExecutionHostId) const preservedBranch = removalResult?.preservedBranch if (!preservedBranch) { diff --git a/src/renderer/src/store/slices/worktrees/teardown/remove-worktree.ts b/src/renderer/src/store/slices/worktrees/teardown/remove-worktree.ts index fae6ddf6472..cb921befdab 100644 --- a/src/renderer/src/store/slices/worktrees/teardown/remove-worktree.ts +++ b/src/renderer/src/store/slices/worktrees/teardown/remove-worktree.ts @@ -193,6 +193,8 @@ export function createRemoveWorktree( // Why (STA-4343): another host still owns this id, so the shared renderer // state (tabs, terminals, browsers) belongs to that live workspace — drop // only the confirmed host's row instead of tearing all of it down. + // + // The ephemeral VM is the exception — see completeSameIdHostScopedRemoval. if (sameIdSurvivesOnAnotherHost && requiredExecutionHostId) { return completeSameIdHostScopedRemoval({ set,