diff --git a/src/renderer/src/lib/worktree-creation-cancellation.test.ts b/src/renderer/src/lib/worktree-creation-cancellation.test.ts index 8d1b8285afa..39204299eaf 100644 --- a/src/renderer/src/lib/worktree-creation-cancellation.test.ts +++ b/src/renderer/src/lib/worktree-creation-cancellation.test.ts @@ -262,6 +262,49 @@ describe('worktree creation cancellation', () => { expect(toast.error).not.toHaveBeenCalled() }) + it('keeps blocking retry until a failed deferred rollback actually removes its workspace', async () => { + await withWorktreeCreationCancellation('creation', async (attempt) => { + attempt.worktree = worktree + }) + state.removeWorktree.mockResolvedValue({ ok: false, error: 'Host unavailable' }) + + const firstRetry = vi.fn() + await expect(withWorktreeCreationCancellation('creation', firstRetry)).rejects.toThrow( + 'Could not clean up' + ) + expect(firstRetry).not.toHaveBeenCalled() + + // The workspace still exists, so the obligation must survive the first retry + // rather than letting the next one create a second workspace beside it. + const secondRetry = vi.fn() + await expect(withWorktreeCreationCancellation('creation', secondRetry)).rejects.toThrow( + 'Could not clean up' + ) + expect(secondRetry).not.toHaveBeenCalled() + + state.removeWorktree.mockResolvedValue({ ok: true }) + const finalRetry = vi.fn() + await withWorktreeCreationCancellation('creation', finalRetry) + expect(finalRetry).toHaveBeenCalledOnce() + }) + + it('lets retry proceed once an unidentifiable workspace has been reported', async () => { + const unstamped = makeWorktree({ id: 'repo::/workspace', repoId: 'repo', hostId: 'ssh:owner' }) + await withWorktreeCreationCancellation('creation', async (attempt) => { + attempt.worktree = unstamped + }) + const blocked = vi.fn() + await expect(withWorktreeCreationCancellation('creation', blocked)).rejects.toThrow( + 'Could not clean up' + ) + // No removal was attempted, so retrying can never discharge it — do not + // block the user forever behind an obligation nothing can satisfy. + const retry = vi.fn() + await withWorktreeCreationCancellation('creation', retry) + expect(retry).toHaveBeenCalledOnce() + expect(state.removeWorktree).not.toHaveBeenCalled() + }) + it('lets retry proceed after the rollback found its workspace already replaced', async () => { await withWorktreeCreationCancellation('creation', async (attempt) => { attempt.worktree = worktree diff --git a/src/renderer/src/lib/worktree-creation-cancellation.ts b/src/renderer/src/lib/worktree-creation-cancellation.ts index e658c75e1d6..8f6ba04bca0 100644 --- a/src/renderer/src/lib/worktree-creation-cancellation.ts +++ b/src/renderer/src/lib/worktree-creation-cancellation.ts @@ -38,10 +38,20 @@ export async function withWorktreeCreationCancellation( await execute(attempt) } } finally { - const cleanup = (deferred: boolean): Promise => - removeCancelledCreation(attempt, deferred).finally(() => + const cleanup = async (deferred: boolean): Promise => { + const outcome = await removeCancelledCreation(attempt, deferred) + // Why: a deferred rollback whose removal call failed still owns a real + // workspace, and its attempt is the only record of it. Dropping that record + // would let the next retry create a second workspace beside the first, so + // keep the obligation until a removal actually discharges it. An unidentified + // workspace is exempt: no call was made, so retrying can never discharge it. + if (deferred && !outcome.ok && outcome.retryable) { + attempt.cleanupAfterSettlement = () => cleanup(deferred) + } else { releaseActiveWorktreeCreation(creationId, attempt) - ) + } + return outcome.ok + } if (!attempt.completed && attempt.isCancelled()) { await cleanup(false) } else if (!attempt.completed && attempt.worktree) { @@ -53,6 +63,9 @@ export async function withWorktreeCreationCancellation( } } +/** `retryable` is false when no removal was attempted, so retrying cannot help. */ +type CancelledCreationCleanup = { ok: boolean; retryable: boolean } + /** * `deferred` marks a rollback that outlived its attempt, waiting behind an error * panel. Only that one can sit long enough for the user to delete and recreate at @@ -62,11 +75,14 @@ export async function withWorktreeCreationCancellation( async function removeCancelledCreation( attempt: WorktreeCreationAttempt, deferred: boolean -): Promise { +): Promise { const { worktree } = attempt + let retryable = true try { if (worktree) { if (deferred && !worktree.instanceId) { + // No removal is attempted, so no later retry can discharge this. + retryable = false throw new Error( 'it could not be identified on the host, so it was left in place. Delete it manually if unwanted.' ) @@ -92,7 +108,7 @@ async function removeCancelledCreation( ) } await attempt.cleanupRuntime?.() - return true + return { ok: true, retryable } } catch (error) { const message = error instanceof Error ? error.message : String(error) console.error('worktree create: cancellation cleanup failed', worktree?.id, error) @@ -105,6 +121,6 @@ async function removeCancelledCreation( toast.error(`Could not remove the cancelled workspace: ${message}${runtimeHint}`, { duration: Infinity }) - return false + return { ok: false, retryable } } }