fix(vm): address provisioned root review findings

This commit is contained in:
Jinwoo-H
2026-08-11 22:35:50 -04:00
parent 1f8974ecaf
commit 4d5659d84e
11 changed files with 114 additions and 22 deletions
+3 -1
View File
@@ -410,7 +410,9 @@ vercel_args=(); [ -n "$scope" ] && vercel_args+=(--scope "$scope"); [ -n "$proje
[ -n "$repo_url" ] && [ -n "$repo_ref" ] && [ -n "$repo_branch" ] \
|| { echo "provisioned-root requires repo URL, start ref, and workspace branch" >&2; exit 1; }
gh_token="${GH_TOKEN:-${GITHUB_TOKEN:-$(command -v gh >/dev/null 2>&1 && gh auth token 2>/dev/null || true)}}"
name="orca-${ORCA_RECIPE_ID:-vercel-sandbox}-${ORCA_VM_INSTANCE_ID:-$(date +%s)}" # sanitize+cap to 63 chars
raw_name="orca-${ORCA_RECIPE_ID:-vercel-sandbox}-${ORCA_VM_INSTANCE_ID:-$(date +%s)}"
name="$(printf '%s' "$raw_name" | tr '[:upper:]_' '[:lower:]-' | sed -E 's/[^a-z0-9-]+/-/g; s/^-+//; s/-+$//' | cut -c1-63 | sed -E 's/-+$//')"
[ -n "$name" ] || { echo "sandbox name is empty after normalization" >&2; exit 1; }
# Arm cleanup BEFORE create so a failing create can't leak a half-built paid sandbox.
cleanup_on_error() { [ "$?" -ne 0 ] && vercel sandbox remove "$name" "${vercel_args[@]}" >/dev/null 2>&1 || true; }
File diff suppressed because one or more lines are too long
@@ -8,8 +8,15 @@ const mocks = vi.hoisted(() => ({
modalData: { repoId: 'repo-1', displayName: 'Cloud workspace' } as Record<string, unknown>,
closeModal: vi.fn(),
removeProject: vi.fn(),
repos: [{ id: 'repo-1', connectionId: null as string | null }],
worktreesByRepo: {} as Record<string, { ephemeralVmCheckoutMode?: string }[]>,
repos: [
{
id: 'repo-1',
connectionId: null as string | null,
executionHostId: undefined as string | undefined
}
],
worktreesByRepo: {} as Record<string, { hostId?: string; ephemeralVmCheckoutMode?: string }[]>,
settings: { activeRuntimeEnvironmentId: null as string | null },
sshTargetLabels: new Map<string, string>(),
removedSshTargetLabels: new Map<string, string>()
}
@@ -46,7 +53,8 @@ vi.mock('@/i18n/i18n', () => ({
describe('RemoveFolderDialog', () => {
beforeEach(() => {
vi.clearAllMocks()
mocks.state.repos = [{ id: 'repo-1', connectionId: null }]
mocks.state.modalData = { repoId: 'repo-1', displayName: 'Cloud workspace' }
mocks.state.repos = [{ id: 'repo-1', connectionId: null, executionHostId: undefined }]
mocks.state.worktreesByRepo = {}
})
@@ -69,4 +77,30 @@ describe('RemoveFolderDialog', () => {
expect(markup).toContain('still on your disk')
})
it('keeps local retention copy when a same-id sibling host is provisioned', async () => {
mocks.state.modalData = {
repoId: 'repo-1',
displayName: 'Local workspace',
hostId: 'local'
}
mocks.state.repos = [
{ id: 'repo-1', connectionId: null, executionHostId: undefined },
{ id: 'repo-1', connectionId: null, executionHostId: 'runtime:env-1' }
]
mocks.state.worktreesByRepo = {
'repo-1': [
{
hostId: 'runtime:env-1',
ephemeralVmCheckoutMode: 'provisioned-root'
}
]
}
const { default: RemoveFolderDialog } = await import('./RemoveFolderDialog')
const markup = renderToStaticMarkup(<RemoveFolderDialog />)
expect(markup).toContain('still on your disk')
expect(markup).not.toContain('cleans up its provisioned environment')
})
})
@@ -10,6 +10,12 @@ import {
import { Button } from '@/components/ui/button'
import { useAppStore } from '@/store'
import { translate } from '@/i18n/i18n'
import { findRepoForHost } from '@/store/slices/repo-host-identity'
import {
getRepoExecutionHostId,
getWorktreeExecutionHostId,
type ExecutionHostId
} from '../../../../shared/execution-host'
// Why: interpolated into the sentence so locales control where the name sits;
// U+0000 cannot appear in a real project name, so the split is unambiguous.
@@ -24,18 +30,29 @@ const RemoveFolderDialog = React.memo(function RemoveFolderDialog() {
const isOpen = activeModal === 'confirm-remove-folder'
const repoId = typeof modalData.repoId === 'string' ? modalData.repoId : ''
const displayName = typeof modalData.displayName === 'string' ? modalData.displayName : ''
const isProvisionedRoot = useAppStore((s) =>
(s.worktreesByRepo[repoId] ?? []).some(
(worktree) => worktree.ephemeralVmCheckoutMode === 'provisioned-root'
)
const requestedHostId =
typeof modalData.hostId === 'string' ? (modalData.hostId as ExecutionHostId) : undefined
const ownerRepo = useAppStore((s) =>
findRepoForHost(s.repos, repoId, { hostId: requestedHostId, settings: s.settings })
)
const ownerHostId = ownerRepo ? getRepoExecutionHostId(ownerRepo) : requestedHostId
const isProvisionedRoot = useAppStore((s) => {
if (!ownerHostId) {
return false
}
return (s.worktreesByRepo[repoId] ?? []).some(
(worktree) =>
getWorktreeExecutionHostId(worktree, ownerRepo ?? undefined, ownerHostId) === ownerHostId &&
worktree.ephemeralVmCheckoutMode === 'provisioned-root'
)
})
// Why: for an SSH project the files live on the remote host's disk, not the
// user's — "still on your disk" would be misleading. Name the host (using the
// removed-target label when it's a ghost) so the user knows where it remains
// and that re-adding that host recovers it.
const sshHostLabel = useAppStore((s) => {
const connectionId = s.repos.find((r) => r.id === repoId)?.connectionId?.trim()
const connectionId = ownerRepo?.connectionId?.trim()
if (!connectionId) {
return null
}
@@ -70,10 +87,10 @@ const RemoveFolderDialog = React.memo(function RemoveFolderDialog() {
const handleConfirm = useCallback(() => {
if (repoId) {
void removeProject(repoId, { errorFeedback: 'toast' })
void removeProject(repoId, { hostId: ownerHostId, errorFeedback: 'toast' })
}
closeModal()
}, [closeModal, removeProject, repoId])
}, [closeModal, ownerHostId, removeProject, repoId])
const handleOpenChange = useCallback(
(open: boolean) => {
@@ -6189,7 +6189,8 @@ const WorktreeList = React.memo(function WorktreeList({
(repo: Repo) => {
openModal('confirm-remove-folder', {
repoId: repo.id,
displayName: repo.displayName
displayName: repo.displayName,
hostId: getRepoExecutionHostId(repo)
})
},
[openModal]
@@ -493,7 +493,8 @@ describe('delete worktree flow', () => {
expect(mocks.state.removeWorktree).not.toHaveBeenCalled()
expect(mocks.state.openModal).toHaveBeenCalledWith('confirm-remove-folder', {
repoId: 'repo-1',
displayName: 'orca'
displayName: 'orca',
hostId: 'local'
})
})
@@ -9,6 +9,10 @@ import { getWorkspaceDeleteLineage } from './workspace-delete-lineage'
import { resolveSshWorkspaceForget } from './ssh-workspace-forget-resolution'
import { isPairedWebClientWindow } from '@/lib/desktop-window-chrome'
import { parseWorkspaceKey } from '../../../../shared/workspace-scope'
import {
getRepoExecutionHostId,
getSettingsFocusedExecutionHostId
} from '../../../../shared/execution-host'
import {
resolveWorktreeBatchDeleteTargets,
toWorktreeDeleteIdentities,
@@ -44,11 +48,18 @@ export function runWorktreeDelete(worktreeId: string, options: WorktreeDeleteOpt
return
}
if (target.isMainWorktree) {
const repo = state.repos.find((entry) => entry.id === target.repoId)
const repo = findRepoForHost(state.repos, target.repoId, {
hostId: target.hostId,
settings: state.settings
})
const hostId = repo
? getRepoExecutionHostId(repo)
: (target.hostId ?? getSettingsFocusedExecutionHostId(state.settings))
// Why: git refuses to delete the primary checkout; users can still remove the owning project from Orca (disk contents kept).
state.openModal('confirm-remove-folder', {
repoId: target.repoId,
displayName: repo?.displayName ?? target.displayName
displayName: repo?.displayName ?? target.displayName,
hostId
})
return
}
@@ -1,7 +1,7 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const listRuntimes = vi.fn()
const cleanup = vi.fn().mockResolvedValue({})
const cleanup = vi.fn().mockResolvedValue({ status: 'cleaned' })
// @ts-expect-error -- test shim for the preload bridge
globalThis.window = { api: { ephemeralVm: { listRuntimes, cleanup } } }
@@ -15,7 +15,7 @@ function runtime(overrides: Record<string, unknown>): Record<string, unknown> {
describe('cleanupEphemeralVmRuntimesForDeleted', () => {
beforeEach(() => {
listRuntimes.mockReset()
cleanup.mockClear()
cleanup.mockReset().mockResolvedValue({ status: 'cleaned' })
})
it('cleans runtimes matched by workspace id and returns destroyed SSH target ids', async () => {
@@ -70,4 +70,26 @@ describe('cleanupEphemeralVmRuntimesForDeleted', () => {
{ runtimeIds: [], sshTargetIds: [] }
)
})
it('does not report runtime or SSH target ids when cleanup fails', async () => {
listRuntimes.mockResolvedValue([
runtime({ id: 'rt-1', workspaceId: 'wt-1', sshTargetId: 'runtime-ssh-a' })
])
cleanup.mockResolvedValue({ status: 'cleanup_failed', cleanupStatus: 'failed' })
await expect(cleanupEphemeralVmRuntimesForDeleted({ workspaceIds: ['wt-1'] })).resolves.toEqual(
{ runtimeIds: [], sshTargetIds: [] }
)
})
it('does not report runtime or SSH target ids when cleanup rejects', async () => {
listRuntimes.mockResolvedValue([
runtime({ id: 'rt-1', workspaceId: 'wt-1', sshTargetId: 'runtime-ssh-a' })
])
cleanup.mockRejectedValue(new Error('destroy failed'))
await expect(cleanupEphemeralVmRuntimesForDeleted({ workspaceIds: ['wt-1'] })).resolves.toEqual(
{ runtimeIds: [], sshTargetIds: [] }
)
})
})
@@ -40,11 +40,14 @@ export async function cleanupEphemeralVmRuntimesForDeleted(args: {
(runtime.sshTargetId !== undefined && sshTargetIdSet.has(runtime.sshTargetId)))
)
for (const runtime of matchingRuntimes) {
const cleaned = await window.api.ephemeralVm.cleanup({ runtimeId: runtime.id })
if (cleaned.status !== 'cleaned') {
continue
}
matches.runtimeIds.push(runtime.id)
if (runtime.sshTargetId) {
matches.sshTargetIds.push(runtime.sshTargetId)
}
await window.api.ephemeralVm.cleanup({ runtimeId: runtime.id })
}
} catch (error) {
console.error('Failed to clean up ephemeral VM runtime for deleted workspace:', error)
@@ -88,7 +88,7 @@ export function installReposRuntimeRoutingHarness(): void {
runtimeEnvironmentTransportCall.mockReset()
uiSet.mockReset()
ephemeralVmListRuntimes.mockReset().mockResolvedValue([])
ephemeralVmCleanup.mockReset().mockResolvedValue(undefined)
ephemeralVmCleanup.mockReset().mockResolvedValue({ status: 'cleaned' })
uiSet.mockResolvedValue(undefined)
runtimeEnvironmentTransportCall.mockImplementation((args: RuntimeEnvironmentCallRequest) => {
return createCompatibleRuntimeStatusResponseIfNeeded(args) ?? runtimeEnvironmentCall(args)
@@ -155,7 +155,7 @@ const mockApi = {
},
ephemeralVm: {
cancelProvision: vi.fn().mockResolvedValue({ cancelled: true }),
cleanup: vi.fn().mockResolvedValue({}),
cleanup: vi.fn().mockResolvedValue({ status: 'cleaned' }),
listRuntimes: vi.fn().mockResolvedValue([])
}
}
@@ -199,6 +199,7 @@ function resetRemoteRuntimeMocks() {
// earlier describe would silently suppress a row here. Reset for every case, not just the fetch suites.
beforeEach(() => {
resetAuthoritativelyRemovedWorktreeMemoryForTests()
mockApi.ephemeralVm.cleanup.mockResolvedValue({ status: 'cleaned' })
})
function createTestStore() {
@@ -405,7 +406,7 @@ describe('setActiveWorktree focus handling', () => {
vi.clearAllMocks()
resetRemoteRuntimeMocks()
mockApi.ephemeralVm.cancelProvision.mockResolvedValue({ cancelled: true })
mockApi.ephemeralVm.cleanup.mockResolvedValue({})
mockApi.ephemeralVm.cleanup.mockResolvedValue({ status: 'cleaned' })
})
it('moves focus out of a registered webview before switching worktrees', () => {