From d3ebcd4535d2e90661e09f151655b2246ca0faf8 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 02:15:08 -0700 Subject: [PATCH] fix(worktree): preserve retained creation progress on submit --- .../quick-creation-execution.ts | 3 +- .../quick-creation-retention.test.ts | 58 +++++++++++++++++-- .../retained-composer-creation.ts | 34 ++++++++--- .../lib/retained-worktree-progress.test.ts | 45 ++++++++++++++ .../src/lib/worktree-creation-flow.ts | 11 +++- 5 files changed, 135 insertions(+), 16 deletions(-) create mode 100644 src/renderer/src/lib/retained-worktree-progress.test.ts diff --git a/src/renderer/src/hooks/composer-state/quick-creation-execution.ts b/src/renderer/src/hooks/composer-state/quick-creation-execution.ts index d638514d4b5..52227f0f057 100644 --- a/src/renderer/src/hooks/composer-state/quick-creation-execution.ts +++ b/src/renderer/src/hooks/composer-state/quick-creation-execution.ts @@ -255,7 +255,8 @@ export function useQuickCreationExecution(input: QuickCreationExecutionInput) { clearNewWorkspaceDraft() } - runBackgroundWorktreeCreation(request, retainedCreation.take(request, selectedRepo)) + const retained = retainedCreation.take(request, selectedRepo) + runBackgroundWorktreeCreation(request, retained?.creation, retained) if (createMultiple) { retainedCreation.resetForNextCreate() diff --git a/src/renderer/src/hooks/composer-state/quick-creation-retention.test.ts b/src/renderer/src/hooks/composer-state/quick-creation-retention.test.ts index dcacc941d48..870e4138503 100644 --- a/src/renderer/src/hooks/composer-state/quick-creation-retention.test.ts +++ b/src/renderer/src/hooks/composer-state/quick-creation-retention.test.ts @@ -4,7 +4,10 @@ import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { Repo } from '../../../../shared/repo-types' import type { CreateWorktreeResult } from '../../../../shared/worktree/create-types' -import type { WorktreeCreationRequest } from '@/lib/pending-worktree-creation' +import type { + WorktreeCreationPhase, + WorktreeCreationRequest +} from '@/lib/pending-worktree-creation' const boundaries = vi.hoisted(() => ({ create: @@ -17,7 +20,13 @@ const boundaries = vi.hoisted(() => ({ >(), capabilities: vi.fn<() => Promise>(), activate: - vi.fn<(request: WorktreeCreationRequest, creation?: Promise) => void>() + vi.fn< + ( + request: WorktreeCreationRequest, + creation?: CreateWorktreeResult | Promise, + progress?: { creationId: string; phase?: string } + ) => void + >() })) vi.mock('@/lib/create-requested-worktree', () => ({ createRequestedWorktree: boundaries.create })) vi.mock('@/lib/worktree-creation-flow', () => ({ @@ -115,10 +124,21 @@ function deferred() { describe('quick composer retained creation', () => { let root: Root | null let input: Input + let progress: (event: { creationId: string; phase: WorktreeCreationPhase }) => void + const unsubscribe = vi.fn() beforeEach(() => { vi.clearAllMocks() + vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true) boundaries.create.mockResolvedValue(result) boundaries.capabilities.mockResolvedValue(['worktree.background-startup.v1']) + vi.stubGlobal('api', { + worktrees: { + onCreateProgress: (listener: typeof progress) => { + progress = listener + return unsubscribe + } + } + }) input = makeInput() const container = document.createElement('div') document.body.appendChild(container) @@ -128,6 +148,7 @@ describe('quick composer retained creation', () => { afterEach(() => { act(() => root?.unmount()) document.body.replaceChildren() + vi.unstubAllGlobals() }) const execute = (preparation?: { isCancelled: () => boolean }, selectedRepo = repo) => @@ -167,6 +188,9 @@ describe('quick composer retained creation', () => { }) expect(boundaries.create).toHaveBeenCalledTimes(1) expect(boundaries.activate).toHaveBeenCalledTimes(1) + expect(boundaries.activate.mock.calls[0][2]?.creationId).toBe( + boundaries.create.mock.calls[0][0] + ) const joined = boundaries.activate.mock.calls[0][1] expect(joined).toBeInstanceOf(Promise) pending.resolve(result) @@ -175,6 +199,23 @@ describe('quick composer retained creation', () => { expect(input.onCreated).toHaveBeenCalledTimes(1) }) + it('replays the latest matching phase and unsubscribes on unmount', async () => { + await act(async () => { + await execute({ isCancelled: () => false }) + }) + const creationId = boundaries.create.mock.calls[0][0] + progress({ creationId, phase: 'fetching' }) + progress({ creationId, phase: 'creating' }) + progress({ creationId: 'unrelated', phase: 'fetching' }) + await act(async () => { + await execute() + }) + expect(boundaries.activate.mock.calls[0][2]).toMatchObject({ creationId, phase: 'creating' }) + act(() => root!.unmount()) + root = null + expect(unsubscribe).toHaveBeenCalledTimes(1) + }) + it('uses ordinary Create when the host cannot promise background startup', async () => { boundaries.capabilities.mockResolvedValue([]) await act(async () => { @@ -184,7 +225,7 @@ describe('quick composer retained creation', () => { await act(async () => { await execute() }) - expect(boundaries.activate).toHaveBeenCalledWith(expect.any(Object), undefined) + expect(boundaries.activate).toHaveBeenCalledWith(expect.any(Object), undefined, undefined) }) it('allows one new preparation after an explicit Create more submission', async () => { @@ -202,6 +243,15 @@ describe('quick composer retained creation', () => { }) expect(boundaries.create).toHaveBeenCalledTimes(2) expect(boundaries.create.mock.calls[1][1].name).toBe('next') + expect(boundaries.create.mock.calls[1][0]).not.toBe(boundaries.create.mock.calls[0][0]) + progress({ creationId: boundaries.create.mock.calls[0][0], phase: 'creating' }) + await act(async () => { + await execute() + }) + expect(boundaries.activate.mock.calls[1][2]).toMatchObject({ + creationId: boundaries.create.mock.calls[1][0], + phase: undefined + }) }) it('does not create if preparation is canceled while preflight is pending', async () => { @@ -238,7 +288,7 @@ describe('quick composer retained creation', () => { await execute(undefined, selectedRepo) }) expect(boundaries.create).toHaveBeenCalledTimes(1) - expect(boundaries.activate).toHaveBeenCalledWith(expect.any(Object), undefined) + expect(boundaries.activate).toHaveBeenCalledWith(expect.any(Object), undefined, undefined) } ) diff --git a/src/renderer/src/hooks/composer-state/retained-composer-creation.ts b/src/renderer/src/hooks/composer-state/retained-composer-creation.ts index 36d66b0dc0a..539ca93f876 100644 --- a/src/renderer/src/hooks/composer-state/retained-composer-creation.ts +++ b/src/renderer/src/hooks/composer-state/retained-composer-creation.ts @@ -1,16 +1,24 @@ import type { Repo } from '../../../../shared/repo-types' -import { useMemo, useState } from 'react' +import { useEffect, useMemo, useState } from 'react' import { createRetainedWorktreeCreation } from '@/lib/retained-worktree-creation' import { createRequestedWorktree } from '@/lib/create-requested-worktree' import { createBrowserUuid } from '@/lib/browser-uuid' -import type { WorktreeCreationRequest } from '@/lib/pending-worktree-creation' +import type { + WorktreeCreationPhase, + WorktreeCreationRequest +} from '@/lib/pending-worktree-creation' export type ComposerPreparation = { isCancelled: () => boolean } function createComposerReservation() { - return createRetainedWorktreeCreation((request) => - createRequestedWorktree(createBrowserUuid(), request, true) - ) + const creationId = createBrowserUuid() + return { + creationId, + phase: undefined as WorktreeCreationPhase | undefined, + ...createRetainedWorktreeCreation((request) => + createRequestedWorktree(creationId, request, true) + ) + } } export function useRetainedComposerCreation( @@ -22,6 +30,15 @@ export function useRetainedComposerCreation( started: false, creation: createComposerReservation() })) + useEffect( + () => + window.api?.worktrees?.onCreateProgress?.((event) => { + if (event.creationId === state.creation.creationId) { + state.creation.phase = event.phase + } + }), + [state] + ) return useMemo( () => ({ begin(preparation?: ComposerPreparation): (() => boolean) | null { @@ -42,9 +59,10 @@ export function useRetainedComposerCreation( state.creation = createComposerReservation() }, take(request: WorktreeCreationRequest, repo: Repo) { - return ( - state.creation.take(request, JSON.stringify({ executionIdentity, repo })) ?? undefined - ) + const creation = state.creation.take(request, JSON.stringify({ executionIdentity, repo })) + return creation + ? { creation, creationId: state.creation.creationId, phase: state.creation.phase } + : undefined } }), [executionIdentity, isSubmissionCancelled, state] diff --git a/src/renderer/src/lib/retained-worktree-progress.test.ts b/src/renderer/src/lib/retained-worktree-progress.test.ts new file mode 100644 index 00000000000..5148aa7d873 --- /dev/null +++ b/src/renderer/src/lib/retained-worktree-progress.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it, vi } from 'vitest' +import type { PendingWorktreeCreation } from './pending-worktree-creation' +import type { CreateWorktreeResult } from '../../../shared/worktree/create-types' +import { makeRequest } from './worktree-creation-request.test-fixture' +import { runBackgroundWorktreeCreation } from './worktree-creation-flow' +import { executeWorktreeCreation } from './worktree-creation-flow-execute' + +const { store } = vi.hoisted(() => ({ + store: { + settings: {}, + pendingWorktreeCreations: {} as Record, + beginPendingWorktreeCreation: vi.fn(), + setActiveView: vi.fn(), + setSidebarOpen: vi.fn() + } +})) +vi.mock('@/store', () => ({ useAppStore: { getState: () => store } })) +vi.mock('@/runtime/runtime-rpc-client', () => ({ + getActiveRuntimeTarget: () => ({ kind: 'local' }) +})) +vi.mock('@/lib/browser-uuid', () => ({ createBrowserUuid: () => 'new-id' })) +vi.mock('@/lib/worktree-creation-flow-execute', () => ({ executeWorktreeCreation: vi.fn() })) +vi.mock('@/lib/worktree-creation-structured-recovery', () => ({ + retryStructuredWorktreeLaunch: vi.fn() +})) + +describe('retained checkout progress correlation', () => { + it('uses the backend reservation ID for both the pending panel and completion', () => { + const request = makeRequest() + const creation = new Promise(() => {}) + const id = runBackgroundWorktreeCreation(request, creation, { + creationId: 'reservation-id', + phase: 'creating' + }) + expect(id).toBe('reservation-id') + expect(store.beginPendingWorktreeCreation).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ creationId: 'reservation-id', phase: 'creating', request }) + ) + expect(executeWorktreeCreation).toHaveBeenCalledExactlyOnceWith( + 'reservation-id', + request, + creation + ) + }) +}) diff --git a/src/renderer/src/lib/worktree-creation-flow.ts b/src/renderer/src/lib/worktree-creation-flow.ts index 4aabb16f9bf..0371112236b 100644 --- a/src/renderer/src/lib/worktree-creation-flow.ts +++ b/src/renderer/src/lib/worktree-creation-flow.ts @@ -49,7 +49,8 @@ function revealPendingCreation( */ export function runBackgroundWorktreeCreation( request: WorktreeCreationRequest, - retainedCreation?: CreateWorktreeResult | Promise + retainedCreation?: CreateWorktreeResult | Promise, + retainedProgress?: { creationId: string; phase?: WorktreeCreationPhase } ): string { const store = useAppStore.getState() const existingCreationId = findPendingLinkedWorkItemCreationId( @@ -64,8 +65,12 @@ export function runBackgroundWorktreeCreation( } // Why: crypto.randomUUID is undefined in non-secure browser contexts (LAN web // client over plain HTTP). createBrowserUuid falls back to getRandomValues. - const creationId = createBrowserUuid() - revealPendingCreation(creationId, request, getInitialWorktreeCreationPhase(request)) + const creationId = retainedProgress?.creationId ?? createBrowserUuid() + revealPendingCreation( + creationId, + request, + retainedProgress?.phase ?? getInitialWorktreeCreationPhase(request) + ) void executeWorktreeCreation(creationId, request, retainedCreation) return creationId }