fix(workspaces): complete a worktree create when a post-create step throws

executeWorktreeCreation's try/catch ends once createWorktree resolves, and all three callers fire it with a bare void and no .catch. completeWorktreeCreation is the only thing that removes the pending creation, so a throw in that tail left pendingWorktreeCreations and activePendingCreationId set: the creation surface stayed up, workspaceChromeActive went false, and the finished workspace rendered no tab chrome while its panes mounted invisibly behind the panel. It only cleared when the user switched workspaces, because setActiveWorktree nulls the pointer. Silently -- no toast, no error state. activateAndRevealWorktree, ensureWorktreeHasInitialTerminal and ensureWebRuntimeWorktreeTerminalAfterWake are all synchronous with no internal guard; launchStructuredWorktreeSession guards only its awaited launch, and that catch's comment already names this stranding hazard. The worktree exists past that point, so each follow-up step is now guarded individually and falls back to the values the skip paths already used; control flow always reaches completion. The structured-launch cancelled/visibility returns keep their semantics, and a throw there is treated as a failed launch, matching what that module already returns for 'failed'. A .catch backstop on the three call sites turns anything that still escapes -- including prepareRequestForCreate, whose VM await has try/finally with no catch -- into a visible error state plus toast. Ablated: with the guards removed the new suite fails 4 of 5, the survivor being the no-throw control.
2026-09-28 00:02:41 +00:00 · 2026-09-11 16:24:00 -07:00
parent 6252f8149b
commit 2144e81fb1
3 changed files with 408 additions and 53 deletions
@@ -19,7 +19,10 @@ import type { WorktreeCreationRequest } from '@/lib/pending-worktree-creation'
 import { createBrowserUuid } from '@/lib/browser-uuid'
 import { resolveBackendDraftStartup } from '@/lib/worktree-draft-startup-view-mode'
 import { buildWorktreeCreationStartupOpt } from '@/lib/worktree-creation-flow-startup'
-import { launchStructuredWorktreeSession } from '@/lib/worktree-creation-structured-session'
+import {
+  launchStructuredWorktreeSession,
+  type WorktreeCreationStructuredSessionResult
+} from '@/lib/worktree-creation-structured-session'
 import { completeWorktreeCreation } from '@/lib/worktree-creation-completion'
 import { markStructuredWorktreeLaunchUnconfirmed } from '@/lib/worktree-creation-structured-recovery'
 import { ensureWebRuntimeWorktreeTerminalAfterWake } from '@/lib/web-runtime-worktree-terminal-after-wake'
@@ -164,46 +167,60 @@ export async function executeWorktreeCreation(
      (completionState.activeView === 'terminal' &&
        completionState.activePendingCreationId === null))

+  // Why: the worktree exists past this point and nothing awaits this caller, so
+  // each follow-up step is best-effort — an escaped throw would strand the
+  // creation surface over the finished workspace instead of reaching completion.
  let activation: ActivateAndRevealResult | false = false
-  let primaryTabId: string | null
+  let primaryTabId: string | null = null
  if (shouldActivateOnCompletion && !structuredLaunch) {
-    activation = activateAndRevealWorktree(worktree.id, {
-      sidebarRevealBehavior: 'auto',
-      ...(preparedRequest.agent !== null ? { agent: preparedRequest.agent } : {}),
-      ...(result.setup ? { setup: result.setup } : {}),
-      ...(result.defaultTabs ? { defaultTabs: result.defaultTabs } : {}),
-      ...(startupOpt ? { startup: startupOpt } : {}),
-      ...(preparedRequest.issueCommand ? { issueCommand: preparedRequest.issueCommand } : {}),
-      ...(backendSpawned ? { backendStartupTerminalSpawned: true } : {})
-    })
+    try {
+      activation = activateAndRevealWorktree(worktree.id, {
+        sidebarRevealBehavior: 'auto',
+        ...(preparedRequest.agent !== null ? { agent: preparedRequest.agent } : {}),
+        ...(result.setup ? { setup: result.setup } : {}),
+        ...(result.defaultTabs ? { defaultTabs: result.defaultTabs } : {}),
+        ...(startupOpt ? { startup: startupOpt } : {}),
+        ...(preparedRequest.issueCommand ? { issueCommand: preparedRequest.issueCommand } : {}),
+        ...(backendSpawned ? { backendStartupTerminalSpawned: true } : {})
+      })
+    } catch (error) {
+      console.error('worktree create: activate-and-reveal failed', worktree.id, error)
+    }
    primaryTabId = activation === false ? null : activation.primaryTabId
  } else {
    // Keep chat creation on its pending surface until the session is ready.
    const hasExplicitTerminalWork = Boolean(
      startupOpt || result.setup || preparedRequest.issueCommand || result.defaultTabs
    )
-    primaryTabId =
-      preparedRequest.agent !== null && !hasExplicitTerminalWork
-        ? null
-        : ensureWorktreeHasInitialTerminal(
-            useAppStore.getState(),
-            worktree.id,
-            startupOpt,
-            result.setup,
-            preparedRequest.issueCommand,
-            result.defaultTabs,
-            {
-              activateCreatedTabs: false,
-              ...(preparedRequest.agent !== null ? { callerProvidesSurface: true } : {}),
-              ...(backendSpawned ? { backendStartupTerminalSpawned: true } : {})
-            }
-          )
+    if (preparedRequest.agent === null || hasExplicitTerminalWork) {
+      try {
+        primaryTabId = ensureWorktreeHasInitialTerminal(
+          useAppStore.getState(),
+          worktree.id,
+          startupOpt,
+          result.setup,
+          preparedRequest.issueCommand,
+          result.defaultTabs,
+          {
+            activateCreatedTabs: false,
+            ...(preparedRequest.agent !== null ? { callerProvidesSurface: true } : {}),
+            ...(backendSpawned ? { backendStartupTerminalSpawned: true } : {})
+          }
+        )
+      } catch (error) {
+        console.error('worktree create: initial terminal seeding failed', worktree.id, error)
+      }
+    }
    if (!structuredLaunch && !backendSpawned) {
-      ensureWebRuntimeWorktreeTerminalAfterWake(worktree.id, {
-        startup: startupOpt,
-        agent: preparedRequest.agent,
-        activate: false
-      })
+      try {
+        ensureWebRuntimeWorktreeTerminalAfterWake(worktree.id, {
+          startup: startupOpt,
+          agent: preparedRequest.agent,
+          activate: false
+        })
+      } catch (error) {
+        console.error('worktree create: after-wake terminal seeding failed', worktree.id, error)
+      }
    }
  }

@@ -213,25 +230,34 @@ export async function executeWorktreeCreation(
    agentLaunchRoute === 'structured-native-chat' &&
    isAgentSessionHandleProvider(preparedRequest.agent)
  ) {
-    const structuredSession = await launchStructuredWorktreeSession({
-      creationId,
-      request: preparedRequest,
-      agentLaunchRoute,
-      worktreeId: worktree.id,
-      shouldActivateOnCompletion,
-      fallbackStartupOpt,
-      activation,
-      primaryTabId
-    })
-    structuredLaunchAccepted = structuredSession.accepted
-    activation = structuredSession.activation
-    primaryTabId = structuredSession.primaryTabId
-    if (structuredSession.cancelled) {
-      return
+    let structuredSession: WorktreeCreationStructuredSessionResult | null = null
+    try {
+      structuredSession = await launchStructuredWorktreeSession({
+        creationId,
+        request: preparedRequest,
+        agentLaunchRoute,
+        worktreeId: worktree.id,
+        shouldActivateOnCompletion,
+        fallbackStartupOpt,
+        activation,
+        primaryTabId
+      })
+    } catch (error) {
+      // Why: plan.launch is guarded inside, but its sync prologue is not; treat
+      // an escaped throw like a failed launch (accepted) and still complete.
+      console.error('worktree create: structured session launch failed', worktree.id, error)
    }
-    if (structuredSession.visibilityUnknown) {
-      markStructuredWorktreeLaunchUnconfirmed(creationId, worktree.id)
-      return
+    if (structuredSession) {
+      structuredLaunchAccepted = structuredSession.accepted
+      activation = structuredSession.activation
+      primaryTabId = structuredSession.primaryTabId
+      if (structuredSession.cancelled) {
+        return
+      }
+      if (structuredSession.visibilityUnknown) {
+        markStructuredWorktreeLaunchUnconfirmed(creationId, worktree.id)
+        return
+      }
    }
  }

@@ -0,0 +1,309 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import type {
+  PendingWorktreeCreation,
+  WorktreeCreationRequest
+} from '@/lib/pending-worktree-creation'
+import { shouldShowWorktreeCreationSurface } from '@/lib/worktree-creation-surface'
+
+// Guards executeWorktreeCreation's post-create tail: callers fire and forget,
+// so a throw after createWorktree succeeds must be contained per-step and the
+// creation must still reach completeWorktreeCreation, which tears the creation
+// surface down. Also covers the caller-side .catch() backstop: a rejection that
+// still escapes (e.g. pre-create preparation) becomes a visible error state
+// plus toast instead of a panel silently stuck at "creating".
+
+type TestActiveView = 'terminal' | 'tasks'
+
+const store = {
+  settings: {
+    activeRuntimeEnvironmentId: null as string | null,
+    experimentalNativeChat: undefined as boolean | undefined,
+    openAgentTabsInChatByDefault: undefined as boolean | undefined
+  },
+  activeView: 'terminal' as TestActiveView,
+  activePendingCreationId: 'creation-1' as string | null,
+  repos: [] as { id: string; connectionId: string | null }[],
+  pendingWorktreeCreations: {} as Record<string, PendingWorktreeCreation>,
+  beginPendingWorktreeCreation: vi.fn((entry: PendingWorktreeCreation) => {
+    store.pendingWorktreeCreations[entry.creationId] = entry
+    store.activePendingCreationId = entry.creationId
+  }),
+  updatePendingWorktreeCreation: vi.fn(
+    (creationId: string, patch: Partial<PendingWorktreeCreation>) => {
+      const entry = store.pendingWorktreeCreations[creationId]
+      if (entry) {
+        store.pendingWorktreeCreations[creationId] = { ...entry, ...patch }
+      }
+    }
+  ),
+  // Mirrors pending-worktree-creation.ts: drop the entry and the active pointer.
+  removePendingWorktreeCreation: vi.fn((creationId: string) => {
+    delete store.pendingWorktreeCreations[creationId]
+    if (store.activePendingCreationId === creationId) {
+      store.activePendingCreationId = null
+    }
+  }),
+  setActivePendingWorktreeCreation: vi.fn((creationId: string | null) => {
+    store.activePendingCreationId = creationId
+  }),
+  setActiveView: vi.fn((view: TestActiveView) => {
+    store.activeView = view
+  }),
+  setSidebarOpen: vi.fn(),
+  updateWorktreeMeta: vi.fn(),
+  createWorktree: vi.fn(),
+  tabsByWorktree: {} as Record<string, { id: string }[]>,
+  unifiedTabsByWorktree: {}
+}
+
+vi.mock('@/store', () => ({
+  useAppStore: {
+    getState: () => store
+  }
+}))
+
+vi.mock('sonner', () => ({
+  toast: { error: vi.fn() }
+}))
+
+vi.mock('@/lib/worktree-activation', () => ({
+  activateAndRevealWorktree: vi.fn()
+}))
+
+vi.mock('@/lib/worktree-initial-terminal-seeding', () => ({
+  ensureWorktreeHasInitialTerminal: vi.fn()
+}))
+
+vi.mock('@/lib/web-runtime-worktree-terminal-after-wake', () => ({
+  ensureWebRuntimeWorktreeTerminalAfterWake: vi.fn()
+}))
+
+vi.mock('@/lib/workspace-activation-terminal-focus', () => ({
+  queueWorkspaceActivationTerminalFocus: vi.fn()
+}))
+
+vi.mock('@/lib/new-workspace', () => ({
+  ensureAgentStartupInTerminal: vi.fn()
+}))
+
+vi.mock('@/lib/worktree-creation-agent-seeds', () => ({
+  seedAgentTabStateAfterWorktreeCreate: vi.fn()
+}))
+
+vi.mock('@/lib/ephemeral-vm-workspace-target', () => ({
+  prepareEphemeralVmWorkspaceTarget: vi.fn()
+}))
+
+vi.mock('@/lib/ephemeral-vm-worktree-creation', () => ({
+  prepareRequestForCreate: vi.fn(
+    async (_creationId: string, request: WorktreeCreationRequest) => request
+  ),
+  attachEphemeralVmRuntimeToWorkspace: vi.fn(async () => undefined),
+  cleanupEphemeralVmRuntimeForFailedCreate: vi.fn(async () => undefined)
+}))
+
+vi.mock('@/lib/worktree-creation-structured-recovery', () => ({
+  markStructuredWorktreeLaunchUnconfirmed: vi.fn(),
+  retryStructuredWorktreeLaunch: vi.fn()
+}))
+
+import { toast } from 'sonner'
+import { activateAndRevealWorktree } from '@/lib/worktree-activation'
+import { ensureWorktreeHasInitialTerminal } from '@/lib/worktree-initial-terminal-seeding'
+import { ensureWebRuntimeWorktreeTerminalAfterWake } from '@/lib/web-runtime-worktree-terminal-after-wake'
+import { prepareRequestForCreate } from '@/lib/ephemeral-vm-worktree-creation'
+import { executeWorktreeCreation } from './worktree-creation-flow-execute'
+import { runBackgroundWorktreeCreation } from './worktree-creation-flow'
+
+function makeRequest(overrides: Partial<WorktreeCreationRequest> = {}): WorktreeCreationRequest {
+  return {
+    repoId: 'repo-1',
+    name: 'feature',
+    setupDecision: 'inherit',
+    agent: null,
+    pendingFirstAgentMessageRename: false,
+    note: '',
+    startupPlan: null,
+    quickPrompt: '',
+    quickTelemetry: null,
+    ...overrides
+  } as WorktreeCreationRequest
+}
+
+function seedPendingCreation(request: WorktreeCreationRequest): void {
+  store.pendingWorktreeCreations = {
+    'creation-1': {
+      creationId: 'creation-1',
+      phase: 'fetching',
+      status: 'creating',
+      startedAt: 1,
+      indeterminate: false,
+      loaderVisible: true,
+      request
+    }
+  }
+  store.activePendingCreationId = 'creation-1'
+}
+
+function surfaceInput(activeView: TestActiveView): {
+  activeView: TestActiveView
+  activePendingCreationId: string | null
+  hasActivePendingCreation: boolean
+} {
+  return {
+    activeView,
+    activePendingCreationId: store.activePendingCreationId,
+    hasActivePendingCreation:
+      store.activePendingCreationId !== null &&
+      store.pendingWorktreeCreations[store.activePendingCreationId] !== undefined
+  }
+}
+
+beforeEach(() => {
+  // resetAllMocks: implementations from prior tests (the injected throws) must not leak.
+  vi.resetAllMocks()
+  vi.spyOn(console, 'error').mockImplementation(() => undefined)
+  store.activeView = 'terminal'
+  store.repos = [{ id: 'repo-1', connectionId: null }]
+  store.tabsByWorktree = {}
+  store.pendingWorktreeCreations = {}
+  store.activePendingCreationId = null
+  store.createWorktree.mockResolvedValue({
+    worktree: { id: 'wt-1', repoId: 'repo-1' }
+  })
+})
+
+describe('a throw after createWorktree succeeds no longer strands the creation surface', () => {
+  it('activating branch: a throw in activateAndRevealWorktree is contained and the create completes', async () => {
+    const request = makeRequest()
+    seedPendingCreation(request)
+    vi.mocked(activateAndRevealWorktree).mockImplementation(() => {
+      throw new Error('activation exploded')
+    })
+
+    await executeWorktreeCreation('creation-1', request)
+
+    expect(console.error).toHaveBeenCalledWith(
+      'worktree create: activate-and-reveal failed',
+      'wt-1',
+      expect.any(Error)
+    )
+    // Contained: completion still tears the surface down.
+    expect(store.removePendingWorktreeCreation).toHaveBeenCalledWith('creation-1', {
+      cleanupVm: false
+    })
+    expect(store.pendingWorktreeCreations['creation-1']).toBeUndefined()
+    expect(store.activePendingCreationId).toBeNull()
+    expect(shouldShowWorktreeCreationSurface(surfaceInput('terminal'))).toBe(false)
+    // The activating branch never seeds through the background path.
+    expect(ensureWorktreeHasInitialTerminal).not.toHaveBeenCalled()
+  })
+
+  it('background branch: a throw in after-wake seeding is contained after tabs are seeded', async () => {
+    // User left the terminal view mid-create, so the non-activating branch runs.
+    store.activeView = 'tasks'
+    const request = makeRequest()
+    seedPendingCreation(request)
+    vi.mocked(ensureWorktreeHasInitialTerminal).mockReturnValue('tab-1')
+    vi.mocked(ensureWebRuntimeWorktreeTerminalAfterWake).mockImplementation(() => {
+      throw new Error('after-wake exploded')
+    })
+
+    await executeWorktreeCreation('creation-1', request)
+
+    // Tabs were seeded for the new worktree...
+    expect(ensureWorktreeHasInitialTerminal).toHaveBeenCalledWith(
+      store,
+      'wt-1',
+      undefined,
+      undefined,
+      undefined,
+      undefined,
+      expect.objectContaining({ activateCreatedTabs: false })
+    )
+    expect(console.error).toHaveBeenCalledWith(
+      'worktree create: after-wake terminal seeding failed',
+      'wt-1',
+      expect.any(Error)
+    )
+    // ...and the creation still completed instead of stranding the entry.
+    expect(store.removePendingWorktreeCreation).toHaveBeenCalledWith('creation-1', {
+      cleanupVm: false
+    })
+    expect(store.pendingWorktreeCreations['creation-1']).toBeUndefined()
+    expect(store.activePendingCreationId).toBeNull()
+    expect(shouldShowWorktreeCreationSurface(surfaceInput('terminal'))).toBe(false)
+  })
+
+  it('concurrent create: a throw completing a backgrounded creation still tears its entry down', async () => {
+    // A second submitted create repointed activePendingCreationId, so this
+    // creation's completion takes the non-activating branch on the terminal view.
+    store.activeView = 'terminal'
+    const request = makeRequest()
+    seedPendingCreation(request)
+    store.activePendingCreationId = 'creation-2'
+    store.createWorktree.mockResolvedValue({
+      worktree: { id: 'wt-1', repoId: 'repo-1' },
+      setup: { runnerScriptPath: '/tmp/setup.sh' }
+    })
+    vi.mocked(ensureWorktreeHasInitialTerminal).mockReturnValue('tab-1')
+    vi.mocked(ensureWebRuntimeWorktreeTerminalAfterWake).mockImplementation(() => {
+      throw new Error('after-wake exploded')
+    })
+
+    await executeWorktreeCreation('creation-1', request)
+
+    // Blank terminal + Setup tab are seeded by this one synchronous call.
+    expect(activateAndRevealWorktree).not.toHaveBeenCalled()
+    expect(ensureWorktreeHasInitialTerminal).toHaveBeenCalledWith(
+      store,
+      'wt-1',
+      undefined,
+      { runnerScriptPath: '/tmp/setup.sh' },
+      undefined,
+      undefined,
+      expect.objectContaining({ activateCreatedTabs: false })
+    )
+    // The entry is gone; the pointer stays on the other in-flight creation.
+    expect(store.pendingWorktreeCreations['creation-1']).toBeUndefined()
+    expect(store.activePendingCreationId).toBe('creation-2')
+    expect(shouldShowWorktreeCreationSurface(surfaceInput('terminal'))).toBe(false)
+  })
+
+  it('control: with no throw the same flow completes and tears the surface down', async () => {
+    store.activeView = 'tasks'
+    const request = makeRequest()
+    seedPendingCreation(request)
+    vi.mocked(ensureWorktreeHasInitialTerminal).mockReturnValue('tab-1')
+
+    await executeWorktreeCreation('creation-1', request)
+
+    expect(store.removePendingWorktreeCreation).toHaveBeenCalledWith('creation-1', {
+      cleanupVm: false
+    })
+    expect(store.pendingWorktreeCreations['creation-1']).toBeUndefined()
+    expect(store.activePendingCreationId).toBeNull()
+    expect(shouldShowWorktreeCreationSurface(surfaceInput('terminal'))).toBe(false)
+  })
+
+  it('backstop: a rejection that escapes the execute promise becomes a visible error, not a stuck panel', async () => {
+    // Pre-create preparation runs before the in-function try/catch.
+    vi.mocked(prepareRequestForCreate).mockRejectedValue(new Error('prepare exploded'))
+
+    const creationId = runBackgroundWorktreeCreation(makeRequest())
+
+    await vi.waitFor(() => {
+      expect(toast.error).toHaveBeenCalledWith('prepare exploded')
+    })
+    expect(store.pendingWorktreeCreations[creationId]).toMatchObject({
+      status: 'error',
+      error: 'prepare exploded'
+    })
+    expect(store.removePendingWorktreeCreation).not.toHaveBeenCalled()
+    expect(console.error).toHaveBeenCalledWith(
+      'worktree create: unhandled failure',
+      creationId,
+      expect.any(Error)
+    )
+  })
+})
@@ -1,3 +1,4 @@
+import { toast } from 'sonner'
 import { useAppStore } from '@/store'
 import {
  findPendingLinkedWorkItemCreationId,
@@ -11,11 +12,30 @@ import {
  getWorktreeCreationIndeterminate
 } from '@/lib/worktree-creation-flow-startup'
 import { retryStructuredWorktreeLaunch } from '@/lib/worktree-creation-structured-recovery'
+import {
+  formatWorkspaceCreateError,
+  getWorkspaceCreateErrorToastMessage
+} from '@/lib/workspace-create-error-format'

 type ContinueBackgroundWorktreeCreationOptions = {
  revealCreationSurface?: boolean
 }

+// Why: nothing awaits these creations, so an escaped rejection would otherwise
+// strand the pending entry — and the creation surface — with no error shown.
+function startWorktreeCreation(creationId: string, request: WorktreeCreationRequest): void {
+  executeWorktreeCreation(creationId, request).catch((error: unknown) => {
+    console.error('worktree create: unhandled failure', creationId, error)
+    const store = useAppStore.getState()
+    if (!store.pendingWorktreeCreations[creationId]) {
+      return
+    }
+    const message = getWorkspaceCreateErrorToastMessage(formatWorkspaceCreateError(error))
+    store.updatePendingWorktreeCreation(creationId, { status: 'error', error: message })
+    toast.error(message)
+  })
+}
+
 function revealPendingCreation(
  creationId: string,
  request: WorktreeCreationRequest,
@@ -62,7 +82,7 @@ export function runBackgroundWorktreeCreation(request: WorktreeCreationRequest):
  // client over plain HTTP). createBrowserUuid falls back to getRandomValues.
  const creationId = createBrowserUuid()
  revealPendingCreation(creationId, request, getInitialWorktreeCreationPhase(request))
-  void executeWorktreeCreation(creationId, request)
+  startWorktreeCreation(creationId, request)
  return creationId
 }

@@ -101,7 +121,7 @@ export function continueBackgroundWorktreeCreation(
    store.setActiveView('terminal')
    store.setSidebarOpen(true)
  }
-  void executeWorktreeCreation(creationId, request)
+  startWorktreeCreation(creationId, request)
  return true
 }

@@ -133,5 +153,5 @@ export function retryBackgroundWorktreeCreation(creationId: string): void {
    )
    return
  }
-  void executeWorktreeCreation(creationId, entry.request)
+  startWorktreeCreation(creationId, entry.request)
 }