From 6dbe04042b9e21c8ae119a4e7cf10b3025bedb91 Mon Sep 17 00:00:00 2001 From: Neil Date: Sat, 12 Sep 2026 23:06:05 -0700 Subject: [PATCH] Only retain a rollback obligation a retry could still discharge Dismissal removes the pending creation entry before invoking the deferred rollback, so a re-armed hook there could never run again and would strand the attempt, and the workspace it references, for the session. Retry keeps the entry, so it is the only caller that can re-attempt. Gate the retained obligation on the entry still being present. --- .../src/lib/worktree-creation-cancellation.test.ts | 13 +++++++++++++ .../src/lib/worktree-creation-cancellation.ts | 9 ++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/renderer/src/lib/worktree-creation-cancellation.test.ts b/src/renderer/src/lib/worktree-creation-cancellation.test.ts index 39204299eaf..a93fbada41a 100644 --- a/src/renderer/src/lib/worktree-creation-cancellation.test.ts +++ b/src/renderer/src/lib/worktree-creation-cancellation.test.ts @@ -288,6 +288,19 @@ describe('worktree creation cancellation', () => { expect(finalRetry).toHaveBeenCalledOnce() }) + it('does not arm an unreachable cleanup when a dismissal rollback fails', async () => { + await withWorktreeCreationCancellation('creation', async (attempt) => { + attempt.worktree = worktree + }) + state.removeWorktree.mockResolvedValue({ ok: false, error: 'Host unavailable' }) + // Dismissal removes the pending entry first, so nothing could ever invoke a + // re-armed hook again — holding the attempt would strand it for the session. + delete state.pendingWorktreeCreations.creation + expect(cancelActiveWorktreeCreation('creation')).toBe(true) + await vi.waitFor(() => expect(state.removeWorktree).toHaveBeenCalledOnce()) + await vi.waitFor(() => expect(cancelActiveWorktreeCreation('creation')).toBe(false)) + }) + 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) => { diff --git a/src/renderer/src/lib/worktree-creation-cancellation.ts b/src/renderer/src/lib/worktree-creation-cancellation.ts index 8f6ba04bca0..5e47075355d 100644 --- a/src/renderer/src/lib/worktree-creation-cancellation.ts +++ b/src/renderer/src/lib/worktree-creation-cancellation.ts @@ -45,7 +45,14 @@ export async function withWorktreeCreationCancellation( // 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) { + // The entry check separates retry (kept, so it can re-attempt) from dismissal + // (already gone, so a re-armed hook could never run again). + if ( + deferred && + !outcome.ok && + outcome.retryable && + useAppStore.getState().pendingWorktreeCreations[creationId] + ) { attempt.cleanupAfterSettlement = () => cleanup(deferred) } else { releaseActiveWorktreeCreation(creationId, attempt)