From f190139f032ff88ebdf1e32f98023a062da93544 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 9 Nov 2026 05:36:44 -0800 Subject: [PATCH] fix(vm-recipes): keep a VM's checkout out of the sidebar until its create finishes (#40355) A recipe VM create registers the VM's project before its environment checks run, so the VM's primary checkout showed as a normal workspace next to the pending row while the checks ran or were paused on a failure. Hide that checkout in both sidebar list models until the create adopts it (or makes its worktree). A cancelled or failed create keeps it hidden while its VM is cleaned up; a VM that could not be cleaned up is shown again so it can be removed. --- .../listing/use-main-list-model.ts | 18 ++- .../listing/use-visible-worktrees.ts | 12 +- .../lib/ephemeral-vm-failed-create-cleanup.ts | 20 ++- .../src/lib/ephemeral-vm-worktree-creation.ts | 22 ++- .../lib/vm-create-hidden-checkouts.test.ts | 112 +++++++++++++ .../src/lib/vm-create-hidden-checkouts.ts | 152 ++++++++++++++++++ .../worktree-creation-flow-execute.ts | 7 +- .../create/pending-worktree-creation.ts | 3 +- .../worktrees-pending-creation-state.test.ts | 44 ++++- .../e2e/ssh-orcad-vm-provisioned-root.spec.ts | 134 ++++++++++++--- 10 files changed, 486 insertions(+), 38 deletions(-) create mode 100644 src/renderer/src/lib/vm-create-hidden-checkouts.test.ts create mode 100644 src/renderer/src/lib/vm-create-hidden-checkouts.ts diff --git a/src/renderer/src/components/sidebar/worktree-list/listing/use-main-list-model.ts b/src/renderer/src/components/sidebar/worktree-list/listing/use-main-list-model.ts index c4faab59cdb..6c9fbe9abd5 100644 --- a/src/renderer/src/components/sidebar/worktree-list/listing/use-main-list-model.ts +++ b/src/renderer/src/components/sidebar/worktree-list/listing/use-main-list-model.ts @@ -9,6 +9,11 @@ import { } from '@/lib/sidebar-pending-layer' import { useSidebarListSession } from '@/lib/sidebar-list-session' import { readEndedCreations, subscribeEndedCreations } from '@/lib/sidebar-ended-creations' +import { + readHiddenVmCheckouts, + subscribeHiddenVmCheckouts, + withoutHiddenVmCheckouts +} from '@/lib/vm-create-hidden-checkouts' import '../../../../../../shared/folder-workspaces/folder-workspace-execution-host' import { getFolderWorkspaceHostId } from '../../../../../../shared/folder-workspaces/folder-workspace-host-id' import type { FolderWorkspace } from '../../../../../../shared/folder-workspaces/folder-workspace-types' @@ -73,6 +78,7 @@ export function useMainSidebarListModel(args: SidebarListModelArgs): SidebarList const feed = useDesktopSidebarFeed() const pending = useSyncExternalStore(subscribeSidebarPendingLayer, readSidebarPendingLayer) const endedCreationIds = useSyncExternalStore(subscribeEndedCreations, readEndedCreations) + const hiddenVmCheckouts = useSyncExternalStore(subscribeHiddenVmCheckouts, readHiddenVmCheckouts) const pendingCreationKeys = useAppStore( useShallow((s) => selectPendingWorktreeCreationKeys(s.pendingWorktreeCreations)) ) @@ -84,10 +90,13 @@ export function useMainSidebarListModel(args: SidebarListModelArgs): SidebarList const domains = useMemo( () => withWindowCreations( - overlaySidebarPendingLayer(feed?.domains ?? EMPTY_SIDEBAR_FEED_DOMAINS, pending, { - repos, - lineageById: args.worktreeLineageById - }), + withoutHiddenVmCheckouts( + overlaySidebarPendingLayer(feed?.domains ?? EMPTY_SIDEBAR_FEED_DOMAINS, pending, { + repos, + lineageById: args.worktreeLineageById + }), + hiddenVmCheckouts + ), pendingCreationKeys, endedCreationIds, hiddenCreationKeys @@ -98,6 +107,7 @@ export function useMainSidebarListModel(args: SidebarListModelArgs): SidebarList pendingCreationKeys, endedCreationIds, hiddenCreationKeys, + hiddenVmCheckouts, repos, args.worktreeLineageById ] diff --git a/src/renderer/src/components/sidebar/worktree-list/listing/use-visible-worktrees.ts b/src/renderer/src/components/sidebar/worktree-list/listing/use-visible-worktrees.ts index 8917a0d2b3e..9a83fe32d62 100644 --- a/src/renderer/src/components/sidebar/worktree-list/listing/use-visible-worktrees.ts +++ b/src/renderer/src/components/sidebar/worktree-list/listing/use-visible-worktrees.ts @@ -1,5 +1,5 @@ import { selectSidebarLineageParentIdentities } from '@/store/slices/worktrees/metadata/cross-store-lineage-parents' -import { useMemo, useState } from 'react' +import { useMemo, useState, useSyncExternalStore } from 'react' import { shallow } from 'zustand/shallow' import type { AppState } from '@/store/types' import { useAppStore } from '@/store' @@ -24,6 +24,11 @@ import type { SidebarWorktreeFilters } from './use-filters' import { useReusedArrayIdentity } from './use-reused-array-identity' import { useSleepingSweepRetention } from './use-sleeping-sweep-retention' import { selectWorktreesByRepo } from '@/store/workspaces/catalog/workspace-record-fields' +import { + readHiddenVmCheckouts, + subscribeHiddenVmCheckouts, + withoutHiddenVmWorktrees +} from '@/lib/vm-create-hidden-checkouts' const EMPTY_WORKTREE_ID_SET: ReadonlySet = new Set() @@ -118,8 +123,9 @@ export function useVisibleSidebarWorktrees(args: { worktreeIdsWithStructuredChat }) + const hiddenVmCheckouts = useSyncExternalStore(subscribeHiddenVmCheckouts, readHiddenVmCheckouts) const recomputedVisibleWorktrees = useMemo(() => { - return computeVisibleWorktrees(worktreesByRepo, sortedIds, { + const visible = computeVisibleWorktrees(worktreesByRepo, sortedIds, { filterRepoIds, showSleepingWorkspaces, tabsByWorktree, @@ -148,7 +154,9 @@ export function useVisibleSidebarWorktrees(args: { ? [args.agentSendTargetWorktreeId] : undefined }) + return withoutHiddenVmWorktrees(visible, hiddenVmCheckouts) }, [ + hiddenVmCheckouts, args.agentSendTargetWorktreeId, worktreeIdsWithLiveAgent, sleepingSweepExemptWorktreeIds, diff --git a/src/renderer/src/lib/ephemeral-vm-failed-create-cleanup.ts b/src/renderer/src/lib/ephemeral-vm-failed-create-cleanup.ts index 3b8e555a2b6..0c517c440b9 100644 --- a/src/renderer/src/lib/ephemeral-vm-failed-create-cleanup.ts +++ b/src/renderer/src/lib/ephemeral-vm-failed-create-cleanup.ts @@ -3,17 +3,19 @@ import type { ExecutionHostId } from '../../../shared/execution-host' type FailedCreateCleanupActions = { deleteProjectHostSetup: (setupId: string, hostId: ExecutionHostId) => Promise - cleanupRuntime: (runtimeId: string) => Promise + /** Resolves with the runtime record; a failed destroy resolves rather than rejects. */ + cleanupRuntime: (runtimeId: string) => Promise<{ cleanupStatus?: string } | undefined> reportSetupError: (error: unknown) => void reportRuntimeError: (error: unknown) => void } +/** Resolves false when any cleanup step failed and the VM or its project may remain. */ export async function cleanupFailedEphemeralVmWorkspace( request: WorktreeCreationRequest, actions: FailedCreateCleanupActions -): Promise { +): Promise { if (!request.ephemeralVmRuntimeId) { - return + return true } if ( request.ephemeralVmCheckoutMode === 'provisioned-root' && @@ -26,16 +28,22 @@ export async function cleanupFailedEphemeralVmWorkspace( ) if (deleted == null) { actions.reportSetupError(new Error('Could not confirm project host rollback.')) - return + return false } } catch (error) { actions.reportSetupError(error) - return + return false } } try { - await actions.cleanupRuntime(request.ephemeralVmRuntimeId) + const runtime = await actions.cleanupRuntime(request.ephemeralVmRuntimeId) + if (runtime?.cleanupStatus === 'failed') { + actions.reportRuntimeError(new Error('The VM recipe could not destroy the VM.')) + return false + } + return true } catch (error) { actions.reportRuntimeError(error) + return false } } diff --git a/src/renderer/src/lib/ephemeral-vm-worktree-creation.ts b/src/renderer/src/lib/ephemeral-vm-worktree-creation.ts index 5347af12a6e..fb6604f8eda 100644 --- a/src/renderer/src/lib/ephemeral-vm-worktree-creation.ts +++ b/src/renderer/src/lib/ephemeral-vm-worktree-creation.ts @@ -7,6 +7,10 @@ import type { Repo } from '../../../shared/repo-types' import { translate } from '@/i18n/i18n' import { cleanupFailedEphemeralVmWorkspace } from '@/lib/ephemeral-vm-failed-create-cleanup' import { gateVmCreateOnEnvironmentCheck } from '@/lib/ephemeral-vm-environment-check' +import { + hideVmCreateCheckout, + settleVmCreateCheckoutAfterCleanup +} from '@/lib/vm-create-hidden-checkouts' import { appendProvisioningLog, appendProvisioningWarnings @@ -60,7 +64,14 @@ async function prepareEphemeralVmRecipeRequest( } : {}), provisionId: creationId, - setupExistingFolder: store.setupProjectExistingFolder + setupExistingFolder: async (args) => { + const setup = await store.setupProjectExistingFolder(args) + if (setup) { + // Why: until the create adopts it, the VM's primary checkout is not a workspace; the pending row stands for it. + hideVmCreateCheckout(creationId, { repoId: setup.repo.id, path: setup.repo.path }) + } + return setup + } }) } finally { unsubscribeProvisionEvents?.() @@ -119,7 +130,7 @@ async function prepareEphemeralVmRecipeRequest( preparedTarget.setup.repo.path ))) if (!proceed) { - await cleanupEphemeralVmRuntimeForFailedCreate(preparedRequest) + await cleanupEphemeralVmRuntimeForFailedCreate(preparedRequest, creationId) return null } useAppStore.getState().updatePendingWorktreeCreation(creationId, { @@ -182,10 +193,12 @@ function resolvePortableEphemeralVmProjectId(repo: Repo | undefined): string | n return key.startsWith('github:') ? key : null } +/** A VM cleaned up stays out of the sidebar; one that could not be is shown again so the user can remove it. */ export async function cleanupEphemeralVmRuntimeForFailedCreate( - request: WorktreeCreationRequest + request: WorktreeCreationRequest, + creationId: string ): Promise { - await cleanupFailedEphemeralVmWorkspace(request, { + const cleanedUp = await cleanupFailedEphemeralVmWorkspace(request, { deleteProjectHostSetup: (setupId, hostId) => useAppStore.getState().deleteProjectHostSetup({ setupId, hostId }), cleanupRuntime: (runtimeId) => window.api.ephemeralVm.cleanup({ runtimeId }), @@ -197,6 +210,7 @@ export async function cleanupEphemeralVmRuntimeForFailedCreate( error ) }) + settleVmCreateCheckoutAfterCleanup(creationId, cleanedUp) } export function getProvisionedRootSparseCheckoutError(): string { diff --git a/src/renderer/src/lib/vm-create-hidden-checkouts.test.ts b/src/renderer/src/lib/vm-create-hidden-checkouts.test.ts new file mode 100644 index 00000000000..87f2a584548 --- /dev/null +++ b/src/renderer/src/lib/vm-create-hidden-checkouts.test.ts @@ -0,0 +1,112 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { EMPTY_SIDEBAR_FEED_DOMAINS } from '../../../shared/sidebar-feed-client/sidebar-feed-snapshot' +import type { SidebarFeedDomains } from '../../../shared/sidebar-list/sidebar-feed-domains' +import { + hideVmCreateCheckout, + readHiddenVmCheckouts, + releaseVmCreateCheckout, + settleVmCreateCheckoutAfterCleanup, + subscribeHiddenVmCheckouts, + withoutHiddenVmCheckouts, + withoutHiddenVmWorktrees +} from './vm-create-hidden-checkouts' + +type Row = { repoId: string; path: string; isMainWorktree?: boolean } + +function domainsWith(workspaces: Row[], pairedServerRows: Row[]): SidebarFeedDomains { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the filter reads only repoId, path and isMainWorktree. + return { + ...EMPTY_SIDEBAR_FEED_DOMAINS, + workspaces: new Map(workspaces.map((row, i) => [`w${i}`, row])), + pairedServerRows: new Map(pairedServerRows.map((row, i) => [`p${i}`, row])) + } as unknown as SidebarFeedDomains +} + +function isListed(repoId: string, path = '/srv/repo'): boolean { + return ( + withoutHiddenVmWorktrees([{ repoId, path, isMainWorktree: true }], readHiddenVmCheckouts()) + .length === 1 + ) +} + +afterEach(() => { + releaseVmCreateCheckout('c1') + releaseVmCreateCheckout('c2') + vi.useRealTimers() +}) + +describe('withoutHiddenVmCheckouts', () => { + it('drops only the VM primary checkout while its create is in progress', () => { + const vmPrimary = { repoId: 'vm-repo', path: '/srv/repo', isMainWorktree: true } + const vmLinked = { repoId: 'vm-repo', path: '/srv/repo-wt', isMainWorktree: false } + const sameRepoIdElsewhere = { repoId: 'vm-repo', path: '/other', isMainWorktree: true } + const local = { repoId: 'local', path: '/srv/repo', isMainWorktree: true } + const domains = domainsWith([local], [vmPrimary, vmLinked, sameRepoIdElsewhere]) + + hideVmCreateCheckout('c1', { repoId: 'vm-repo', path: '/srv/repo/' }) + const filtered = withoutHiddenVmCheckouts(domains, readHiddenVmCheckouts()) + + expect([...filtered.pairedServerRows.values()]).toEqual([vmLinked, sameRepoIdElsewhere]) + expect(filtered.workspaces).toBe(domains.workspaces) + }) + + it('keeps the same domains object when nothing is hidden or nothing matches', () => { + const domains = domainsWith([{ repoId: 'a', path: '/a' }], []) + expect(withoutHiddenVmCheckouts(domains, readHiddenVmCheckouts())).toBe(domains) + hideVmCreateCheckout('c1', { repoId: 'b', path: '/b' }) + expect(withoutHiddenVmCheckouts(domains, readHiddenVmCheckouts())).toBe(domains) + }) +}) + +describe('withoutHiddenVmWorktrees', () => { + it('drops the hidden VM primary checkout from the window list and keeps identity otherwise', () => { + const vmPrimary = { id: 'vm', repoId: 'vm-repo', path: '/srv/repo', isMainWorktree: true } + const other = { id: 'o', repoId: 'local', path: '/srv/repo', isMainWorktree: true } + const list = [vmPrimary, other] + expect(withoutHiddenVmWorktrees(list, readHiddenVmCheckouts())).toBe(list) + + hideVmCreateCheckout('c1', { repoId: 'vm-repo', path: '/srv/repo' }) + expect(withoutHiddenVmWorktrees(list, readHiddenVmCheckouts())).toEqual([other]) + releaseVmCreateCheckout('c1') + expect(withoutHiddenVmWorktrees(list, readHiddenVmCheckouts())).toBe(list) + }) +}) + +describe('hidden VM checkout lifecycle', () => { + it('notifies subscribers when a checkout is hidden and released', () => { + const listener = vi.fn() + const unsubscribe = subscribeHiddenVmCheckouts(listener) + hideVmCreateCheckout('c1', { repoId: 'vm', path: '/srv/repo' }) + releaseVmCreateCheckout('c1') + releaseVmCreateCheckout('c1') + expect(listener).toHaveBeenCalledTimes(2) + unsubscribe() + }) + + it('shows a VM again when its cleanup failed, but not after it was cleaned up', () => { + vi.useFakeTimers() + hideVmCreateCheckout('c1', { repoId: 'failed-vm', path: '/srv/repo' }) + settleVmCreateCheckoutAfterCleanup('c1', false) + expect(isListed('failed-vm')).toBe(true) + + hideVmCreateCheckout('c2', { repoId: 'cleaned-vm', path: '/srv/repo' }) + settleVmCreateCheckoutAfterCleanup('c2', true) + // A cancel racing the create cleans up twice; the second, failing pass must not un-hide it. + settleVmCreateCheckoutAfterCleanup('c2', false) + expect(isListed('cleaned-vm')).toBe(false) + vi.runAllTimers() + expect(isListed('cleaned-vm')).toBe(true) + }) + + it("keeps a retry's checkout hidden while the previous attempt's cleanup settles", () => { + vi.useFakeTimers() + hideVmCreateCheckout('c1', { repoId: 'vm', path: '/srv/repo' }) + settleVmCreateCheckoutAfterCleanup('c1', true) + // Same host and path again: the retry's hide outlives the old attempt's. + hideVmCreateCheckout('c1', { repoId: 'vm', path: '/srv/repo' }) + vi.runAllTimers() + expect(isListed('vm')).toBe(false) + releaseVmCreateCheckout('c1') + expect(isListed('vm')).toBe(true) + }) +}) diff --git a/src/renderer/src/lib/vm-create-hidden-checkouts.ts b/src/renderer/src/lib/vm-create-hidden-checkouts.ts new file mode 100644 index 00000000000..8cdc5080fab --- /dev/null +++ b/src/renderer/src/lib/vm-create-hidden-checkouts.ts @@ -0,0 +1,152 @@ +import type { SidebarFeedDomains } from '../../../shared/sidebar-list/sidebar-feed-domains' +import { normalizeRuntimePathForComparison } from '../../../shared/cross-platform-path' + +/** A VM's primary checkout, registered on its host before the create that owns it finishes. */ +export type HiddenVmCheckout = { repoId: string; path: string } +type CheckoutRow = { repoId: string; path: string; isMainWorktree?: boolean } + +/** What the sidebar filters by; `repoIds` lets most rows skip building a path key. */ +export type HiddenVmCheckouts = { + readonly repoIds: ReadonlySet + readonly keys: ReadonlySet +} + +// Upper bound for a cleaned-up VM's rows to leave the lists (its host is retired asynchronously). +const HIDDEN_AFTER_CLEANUP_MS = 60_000 + +const EMPTY: HiddenVmCheckouts = { repoIds: new Set(), keys: new Set() } +type Hide = HiddenVmCheckout & { key: string } +// Keyed per hide, not per creation: a retry re-provisions under the same creation id while the +// previous attempt's cleaned-up VM may still be listed. +let hides = new Map() +const activeHideByCreation = new Map() +const cleanedHides = new Set() +let nextHideId = 1 +let hidden: HiddenVmCheckouts = EMPTY +const listeners = new Set<() => void>() + +function checkoutKey(repoId: string, path: string): string { + return `${repoId}\0${normalizeRuntimePathForComparison(path)}` +} + +function publish(next: Map): void { + hides = next + const values = [...next.values()] + hidden = + values.length === 0 + ? EMPTY + : { + repoIds: new Set(values.map((hide) => hide.repoId)), + keys: new Set(values.map((hide) => hide.key)) + } + listeners.forEach((listener) => listener()) +} + +function removeHide(hideId: number): void { + cleanedHides.delete(hideId) + if (!hides.has(hideId)) { + return + } + const next = new Map(hides) + next.delete(hideId) + publish(next) +} + +/** + * Why: a VM create registers the VM's project before its checks run and before the checkout is + * adopted or a worktree is made; the pending row stands for it until the create finishes. + */ +export function hideVmCreateCheckout(creationId: string, checkout: HiddenVmCheckout): void { + const hideId = nextHideId++ + activeHideByCreation.set(creationId, hideId) + publish( + new Map(hides).set(hideId, { ...checkout, key: checkoutKey(checkout.repoId, checkout.path) }) + ) +} + +/** The create finished: the checkout is now a workspace (or its project) the sidebar may list. */ +export function releaseVmCreateCheckout(creationId: string): void { + const hideId = activeHideByCreation.get(creationId) + if (hideId === undefined) { + return + } + activeHideByCreation.delete(creationId) + removeHide(hideId) +} + +/** + * A failed or cancelled create's VM cleanup finished. A VM that could not be cleaned up shows again + * so the user can remove it; a cleaned-up one stays hidden while its rows leave the lists. A later + * cleanup of the same create (a cancel racing the create) cannot un-hide a VM already cleaned up. + */ +export function settleVmCreateCheckoutAfterCleanup(creationId: string, cleanedUp: boolean): void { + const hideId = activeHideByCreation.get(creationId) + if (hideId === undefined || cleanedHides.has(hideId)) { + return + } + if (!cleanedUp) { + releaseVmCreateCheckout(creationId) + return + } + activeHideByCreation.delete(creationId) + cleanedHides.add(hideId) + setTimeout(() => removeHide(hideId), HIDDEN_AFTER_CLEANUP_MS) +} + +export function subscribeHiddenVmCheckouts(listener: () => void): () => void { + listeners.add(listener) + return () => listeners.delete(listener) +} + +export function readHiddenVmCheckouts(): HiddenVmCheckouts { + return hidden +} + +function isHidden(checkouts: HiddenVmCheckouts, row: CheckoutRow): boolean { + return ( + row.isMainWorktree !== false && + checkouts.repoIds.has(row.repoId) && + checkouts.keys.has(checkoutKey(row.repoId, row.path)) + ) +} + +/** The window's sidebar list without the primary checkouts of VMs whose creates are in progress. */ +export function withoutHiddenVmWorktrees( + worktrees: W[], + checkouts: HiddenVmCheckouts +): W[] { + if (checkouts.keys.size === 0) { + return worktrees + } + const kept = worktrees.filter((worktree) => !isHidden(checkouts, worktree)) + return kept.length === worktrees.length ? worktrees : kept +} + +function withoutHiddenRows( + rows: ReadonlyMap, + checkouts: HiddenVmCheckouts +): ReadonlyMap { + let kept: Map | null = null + for (const [key, row] of rows) { + if (isHidden(checkouts, row)) { + kept ??= new Map(rows) + kept.delete(key) + } + } + return kept ?? rows +} + +/** Main's rows without the primary checkouts of VMs whose creates are still in progress. */ +export function withoutHiddenVmCheckouts( + domains: SidebarFeedDomains, + checkouts: HiddenVmCheckouts +): SidebarFeedDomains { + if (checkouts.keys.size === 0) { + return domains + } + const workspaces = withoutHiddenRows(domains.workspaces, checkouts) + const pairedServerRows = withoutHiddenRows(domains.pairedServerRows, checkouts) + return workspaces === domains.workspaces && pairedServerRows === domains.pairedServerRows + ? domains + : { ...domains, workspaces, pairedServerRows } +} diff --git a/src/renderer/src/lib/workspace/creation/worktree-creation-flow-execute.ts b/src/renderer/src/lib/workspace/creation/worktree-creation-flow-execute.ts index bf95b767dfb..23955624b38 100644 --- a/src/renderer/src/lib/workspace/creation/worktree-creation-flow-execute.ts +++ b/src/renderer/src/lib/workspace/creation/worktree-creation-flow-execute.ts @@ -9,6 +9,7 @@ import { prepareRequestForCreate } from '@/lib/ephemeral-vm-worktree-creation' import { getProvisionedRootCreateOptions } from '@/lib/provisioned-root-create-options' +import { releaseVmCreateCheckout } from '@/lib/vm-create-hidden-checkouts' import { formatWorkspaceCreateError, getWorkspaceCreateErrorToastMessage @@ -125,7 +126,7 @@ export async function executeWorktreeCreation( return } if (preparedRequest.ephemeralVmRuntimeId) { - await cleanupEphemeralVmRuntimeForFailedCreate(preparedRequest) + await cleanupEphemeralVmRuntimeForFailedCreate(preparedRequest, creationId) } if (recoverWorktreeLegacyPreparation(creationId, preparedRequest, error)) { return @@ -145,10 +146,12 @@ export async function executeWorktreeCreation( // Why: cancellation can race a successful backend adoption; clean up again after it settles so an adopted workspace cannot outlive its destroyed VM. if (!useAppStore.getState().pendingWorktreeCreations[creationId]) { if (preparedRequest.ephemeralVmRuntimeId) { - await cleanupEphemeralVmRuntimeForFailedCreate(preparedRequest) + await cleanupEphemeralVmRuntimeForFailedCreate(preparedRequest, creationId) } return } + // The VM's checkout is now this workspace (or its project), so the sidebar may list it. + releaseVmCreateCheckout(creationId) await attachEphemeralVmRuntimeToWorkspace(preparedRequest, worktree.id) const backendSpawned = result.startupTerminal?.spawned === true diff --git a/src/renderer/src/store/slices/worktrees/create/pending-worktree-creation.ts b/src/renderer/src/store/slices/worktrees/create/pending-worktree-creation.ts index 155cb969d2c..d60e0d19c0b 100644 --- a/src/renderer/src/store/slices/worktrees/create/pending-worktree-creation.ts +++ b/src/renderer/src/store/slices/worktrees/create/pending-worktree-creation.ts @@ -3,6 +3,7 @@ import type { WorktreeSliceGet, WorktreeSliceSet } from '../listing/worktree-sli import type { AppState } from '../../../types' import { cleanupFailedEphemeralVmWorkspace } from '@/lib/ephemeral-vm-failed-create-cleanup' import { endWindowCreation } from '@/lib/sidebar-ended-creations' +import { settleVmCreateCheckoutAfterCleanup } from '@/lib/vm-create-hidden-checkouts' import { cancelVmEnvironmentCheck } from '@/lib/ephemeral-vm-environment-check-decision' import { pendingCreationChatDraftScopeKey, @@ -137,7 +138,7 @@ export function createRemovePendingWorktreeCreation( console.error('Failed to remove cancelled provisioned-root project setup:', error), reportRuntimeError: (error) => console.error('Failed to clean up cancelled ephemeral VM runtime:', error) - }) + }).then((cleanedUp) => settleVmCreateCheckoutAfterCleanup(creationId, cleanedUp)) } } diff --git a/src/renderer/src/store/slices/worktrees/create/worktrees-pending-creation-state.test.ts b/src/renderer/src/store/slices/worktrees/create/worktrees-pending-creation-state.test.ts index b9cef02c940..0ce94475804 100644 --- a/src/renderer/src/store/slices/worktrees/create/worktrees-pending-creation-state.test.ts +++ b/src/renderer/src/store/slices/worktrees/create/worktrees-pending-creation-state.test.ts @@ -8,6 +8,12 @@ import { resetRemoteRuntimeMocks, resetWorktreeSliceModuleMemory } from '../worktrees-slice-test-harness' +import { + hideVmCreateCheckout, + readHiddenVmCheckouts, + releaseVmCreateCheckout, + withoutHiddenVmWorktrees +} from '@/lib/vm-create-hidden-checkouts' const requestWorktreeBaseFallbackNotice = vi.hoisted(() => vi.fn()) @@ -25,7 +31,18 @@ vi.mock('@/components/worktree-base-fallback-notice', () => ({ requestWorktreeBaseFallbackNotice })) -beforeEach(resetWorktreeSliceModuleMemory) +beforeEach(() => { + resetWorktreeSliceModuleMemory() + releaseVmCreateCheckout('c1') + releaseVmCreateCheckout('c2') +}) + +function isCheckoutHidden(repoId: string): boolean { + return ( + withoutHiddenVmWorktrees([{ repoId, path: '/workspace/repo' }], readHiddenVmCheckouts()) + .length === 0 + ) +} function makePendingCreation( creationId: string, @@ -197,6 +214,31 @@ describe('pending worktree creation state', () => { expect(store.getState().pendingWorktreeCreations.c1).toBeUndefined() }) + it('keeps a cancelled VM checkout hidden once cleaned up, and shows it if the VM was not destroyed', async () => { + const store = createTestStore() + const request = { ...makePendingCreation('c1').request, ephemeralVmRuntimeId: 'runtime-1' } + store.getState().beginPendingWorktreeCreation(makePendingCreation('c1', { request })) + hideVmCreateCheckout('c1', { repoId: 'vm-repo', path: '/workspace/repo' }) + store.getState().removePendingWorktreeCreation('c1') + await vi.waitFor(() => expect(mockApi.ephemeralVm.cleanup).toHaveBeenCalledTimes(1)) + // Let the cleanup chain settle its hide before checking it was kept. + for (let i = 0; i < 10; i++) { + await Promise.resolve() + } + expect(isCheckoutHidden('vm-repo')).toBe(true) + + // A recipe whose destroy fails resolves a record rather than rejecting. + mockApi.ephemeralVm.cleanup.mockResolvedValueOnce({ cleanupStatus: 'failed' }) + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + store + .getState() + .beginPendingWorktreeCreation(makePendingCreation('c2', { request: { ...request } })) + hideVmCreateCheckout('c2', { repoId: 'vm-repo-2', path: '/workspace/repo' }) + store.getState().removePendingWorktreeCreation('c2') + await vi.waitFor(() => expect(isCheckoutHidden('vm-repo-2')).toBe(false)) + errorSpy.mockRestore() + }) + it('setActiveWorktree dismisses the creation panel even when re-selecting the already-active worktree', () => { const store = createTestStore() const wt = makeWorktree({ id: 'repo1::/path/a', repoId: 'repo1' }) diff --git a/tests/e2e/ssh-orcad-vm-provisioned-root.spec.ts b/tests/e2e/ssh-orcad-vm-provisioned-root.spec.ts index f436745acea..1076b2e976d 100644 --- a/tests/e2e/ssh-orcad-vm-provisioned-root.spec.ts +++ b/tests/e2e/ssh-orcad-vm-provisioned-root.spec.ts @@ -38,24 +38,8 @@ test('adopts a recipe-provisioned SSH root without creating a linked worktree', await waitForSessionReady(orcaPage) const sourceRepoId = await addRecipeRepo(orcaPage, sourceRepo) - await openSidebarWorkspaceComposer(orcaPage) - const dialog = orcaPage.getByRole('dialog', { name: /Create (Workspace|Worktree)/i }) - await expect(dialog).toBeVisible() - await dialog.getByRole('combobox', { name: 'Run on' }).click() - await orcaPage.getByRole('option', { name: /Per-Workspace Environment/ }).click() - await orcaPage - .getByRole('listbox', { name: 'Per-Workspace Environment' }) - .getByText('Docker provisioned root', { exact: true }) - .click() - const workspaceName = `provisioned-root-${Date.now()}` - await dialog.getByPlaceholder(/Type a name/i).fill(workspaceName) - await dialog.getByRole('button', { name: /Create (Workspace|Worktree)/i }).click() - const trustDialog = orcaPage.getByRole('dialog', { name: /Run VM recipe/ }) - await expect(trustDialog).toBeVisible() - await trustDialog.getByRole('button', { name: 'Run hooks' }).click() - - await expect(dialog).toBeHidden({ timeout: 60_000 }) + await startRecipeCreate(orcaPage, workspaceName) await expect(orcaPage.getByRole('option', { name: new RegExp(workspaceName) })).toBeVisible({ timeout: 240_000 }) @@ -138,6 +122,113 @@ test('adopts a recipe-provisioned SSH root without creating a linked worktree', } }) +test('hides the VM checkout while a create is paused on failed checks, and leaves nothing after cancel', async ({ + orcaPage +}, testInfo) => { + // The first connect deploys the VM's managed server before the checks can run. + test.setTimeout(420_000) + let target: DockerSshHostTarget | null = null + const sourceRepo = mkdtempSync(path.join(tmpdir(), 'orca-provisioned-root-source-')) + try { + ensureDockerSshHostImage(process.cwd()) + target = startDockerSshHostTarget(testInfo) + // The VM keeps a remote it cannot push to, so its environment checks fail and the create pauses. + seedRecipeRepo(sourceRepo, target, { reachableRemote: false }) + await waitForSessionReady(orcaPage) + const sourceRepoId = await addRecipeRepo(orcaPage, sourceRepo) + const reposBefore = await orcaPage.evaluate(() => + window.__store!.getState().repos.map((repo) => repo.id) + ) + + const workspaceName = `provisioned-root-${Date.now()}` + await startRecipeCreate(orcaPage, workspaceName) + const sidebar = orcaPage.getByRole('listbox', { name: 'Worktrees' }) + const pendingRow = sidebar.getByRole('button', { + name: new RegExp(`${workspaceName} Environment checks failed`) + }) + await expect(pendingRow).toBeVisible({ timeout: 240_000 }) + // The VM's checkout is in the store by now; the sidebar shows only the pending row for it. + await expect + .poll( + () => + orcaPage.evaluate( + ({ sourceRepoId, workspaceName }) => + Object.values(window.__store!.getState().worktreesByRepo) + .flat() + .some((wt) => wt.repoId !== sourceRepoId && wt.branch.endsWith(workspaceName)), + { sourceRepoId, workspaceName } + ), + { timeout: 30_000 } + ) + .toBe(true) + await expect(sidebar.getByRole('option', { name: new RegExp(workspaceName) })).toHaveCount(0) + await testInfo.attach('paused-vm-create-sidebar', { + body: await orcaPage.screenshot(), + contentType: 'image/png' + }) + + await sidebar.getByRole('button', { name: 'Cancel worktree creation' }).click() + await expect(pendingRow).toBeHidden() + const cancelled = target + await expect + .poll( + () => { + try { + execDockerSshHostTargetCommand(cancelled, 'true') + return false + } catch { + return true + } + }, + { timeout: 60_000, message: 'the cancelled VM was never destroyed' } + ) + .toBe(true) + await expect + .poll( + () => + orcaPage.evaluate( + (before) => + window + .__store!.getState() + .repos.filter((repo) => !before.includes(repo.id)) + .map((repo) => repo.id), + reposBefore + ), + { timeout: 30_000, message: 'the cancelled VM project was left behind' } + ) + .toEqual([]) + await expect(sidebar.getByRole('option', { name: new RegExp(workspaceName) })).toHaveCount(0) + await testInfo.attach('cancelled-vm-create-sidebar', { + body: await orcaPage.screenshot(), + contentType: 'image/png' + }) + } finally { + cleanupDockerSshHostTarget(target) + rmSync(sourceRepo, { recursive: true, force: true }) + } +}) + +async function startRecipeCreate( + page: Parameters[0], + workspaceName: string +): Promise { + await openSidebarWorkspaceComposer(page) + const dialog = page.getByRole('dialog', { name: /Create (Workspace|Worktree)/i }) + await expect(dialog).toBeVisible() + await dialog.getByRole('combobox', { name: 'Run on' }).click() + await page.getByRole('option', { name: /Per-Workspace Environment/ }).click() + await page + .getByRole('listbox', { name: 'Per-Workspace Environment' }) + .getByText('Docker provisioned root', { exact: true }) + .click() + await dialog.getByPlaceholder(/Type a name/i).fill(workspaceName) + await dialog.getByRole('button', { name: /Create (Workspace|Worktree)/i }).click() + const trustDialog = page.getByRole('dialog', { name: /Run VM recipe/ }) + await expect(trustDialog).toBeVisible() + await trustDialog.getByRole('button', { name: 'Run hooks' }).click() + await expect(dialog).toBeHidden({ timeout: 60_000 }) +} + async function addRecipeRepo(page: Parameters[0], repoPath: string) { return page.evaluate(async (pathValue) => { const result = await window.api.repos.add({ path: pathValue }) @@ -152,7 +243,11 @@ async function addRecipeRepo(page: Parameters[0], re }, repoPath) } -function seedRecipeRepo(repoPath: string, target: DockerSshHostTarget): string { +function seedRecipeRepo( + repoPath: string, + target: DockerSshHostTarget, + options: { reachableRemote: boolean } = { reachableRemote: true } +): string { const createScript = path.join(repoPath, 'create.sh') const destroyScript = path.join(repoPath, 'destroy.sh') // The recipe's isolated HOME must still address the engine that owns the fixture container. @@ -218,6 +313,9 @@ ${docker} rm -f ${quotePosixShell(target.containerName)} >/dev/null ) // Why: VM creates run the environment checks first, and a VM that cannot fetch or push its // remote pauses the create. Give the VM a remote it can reach so the create proceeds unattended. + if (!options.reachableRemote) { + return expectedRefHead + } const vmOrigin = '/tmp/provisioned-root-origin.git' execDockerSshHostTargetCommand( target,