fix(vm): keep failed cleanup retryable (#14476)

This commit is contained in:
Jinwoo Hong
2026-08-14 19:04:54 -04:00
committed by GitHub
parent 8b22f044f5
commit 32f46f9a24
19 changed files with 373 additions and 60 deletions
+35 -9
View File
@@ -12,12 +12,17 @@
"gates": [
{
"id": "ephemeral-vm-runtime.rollback-readable-sidecar",
"title": "VM lifecycle records remain readable across provisioned-root rollback",
"title": "VM lifecycle stays rollback-readable and cleanup-retryable",
"maturity": "experimental",
"protection": "partial",
"owner": "ephemeral-vm-runtime-store",
"layer": "desktop-main-persistence",
"surfaces": ["VM runtime sidecar", "recipe lifecycle", "provisioned-root direct SSH"],
"surfaces": [
"VM runtime sidecar",
"recipe lifecycle",
"cleanup retry",
"provisioned-root direct SSH"
],
"platforms": ["macos", "linux", "windows"],
"providers": ["orca-server", "direct-ssh"],
"coveredPlatforms": ["macos", "linux", "windows"],
@@ -28,11 +33,11 @@
"https://github.com/stablyai/orca/pull/14352",
"https://github.com/stablyai/orca/issues/13044"
],
"invariant": "A sidecar written by the new build cannot make the previous production build lose otherwise compatible VM runtimes. Downgrade lifecycle writes remain authoritative after re-upgrade, provisioned-root metadata remains recoverable, and corrupt or future feature metadata is preserved without poisoning the rollback-readable v1 store.",
"oracle": "Write one ordinary schema-v1 runtime and one provisioned-root runtime. Require the exact pre-#14352 reader to return both, run the old destroy lifecycle against the projected provisioned record, then require the candidate to retain the cleaned state while restoring schema-v2 and checkout-mode metadata. The same old reader must reject affected-main and fix-reverted bytes. Malformed, oversized, unknown-record, and future-version feature sidecars must leave rollback-compatible records accessible and must not be overwritten during compatible lifecycle mutations.",
"invariant": "A sidecar written by the new build cannot make the previous production build lose otherwise compatible VM runtimes. Downgrade lifecycle writes remain authoritative after re-upgrade, provisioned-root metadata remains recoverable, and corrupt or future feature metadata is preserved without poisoning the rollback-readable v1 store. Destroy attempts settle at a bounded deadline, retry starts fresh, and provider failure retains the SSH/project context needed for safe cleanup.",
"oracle": "Write one ordinary schema-v1 runtime and one provisioned-root runtime. Require the exact pre-#14352 reader to return both, run the old destroy lifecycle against the projected provisioned record, then require the candidate to retain the cleaned state while restoring schema-v2 and checkout-mode metadata. The same old reader must reject affected-main and fix-reverted bytes. Malformed, oversized, unknown-record, and future-version feature sidecars must leave rollback-compatible records accessible and must not be overwritten during compatible lifecycle mutations. Force destroy to outlive its deadline, require cleanup_failed plus a fresh successful retry, and prove provider or project-host rollback failure retains every ownership record required to retry.",
"commands": [
"node config/scripts/run-ephemeral-vm-runtime-store-rollback-repro.mjs config/scripts/ephemeral-vm-runtime-store-cross-version.test.ts",
"pnpm exec vitest run --config config/vitest.config.ts src/shared/ephemeral-vm-runtime-store.test.ts src/shared/ephemeral-vm-runtime-store-rollback.test.ts src/main/ephemeral-vm-runtime-service.test.ts src/main/ephemeral-vm-recipe-runner.test.ts src/main/ephemeral-vm-runtime-ssh-cleanup.test.ts src/main/ipc/ephemeral-vm-runtime-handler-cleanup.test.ts src/main/ipc/ephemeral-vm.test.ts src/main/provisioned-root-ssh-adoption.test.ts",
"pnpm exec vitest run --config config/vitest.config.ts src/shared/ephemeral-vm-runtime-store.test.ts src/shared/ephemeral-vm-runtime-store-rollback.test.ts src/shared/ephemeral-vm-recipe-process.test.ts src/main/ephemeral-vm-runtime-service.test.ts src/main/ephemeral-vm-recipe-runner.test.ts src/main/ephemeral-vm-runtime-ssh-cleanup.test.ts src/main/ipc/ephemeral-vm-runtime-handler-cleanup.test.ts src/main/ipc/ephemeral-vm.test.ts src/main/provisioned-root-ssh-adoption.test.ts src/renderer/src/lib/ephemeral-vm-failed-create-cleanup.test.ts src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts",
"ORCA_E2E_SSH_DOCKER=1 pnpm exec playwright test tests/e2e/ephemeral-vm-provisioned-root.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1",
"SKIP_BUILD=1 ORCA_E2E_SSH_DOCKER=1 pnpm exec playwright test tests/e2e/ephemeral-vm-provisioned-root.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1"
],
@@ -41,12 +46,15 @@
"config/scripts/run-ephemeral-vm-runtime-store-rollback-repro.mjs",
"src/shared/ephemeral-vm-runtime-store-rollback.test.ts",
"src/shared/ephemeral-vm-runtime-store.test.ts",
"src/shared/ephemeral-vm-recipe-process.test.ts",
"src/main/ephemeral-vm-runtime-service.test.ts",
"src/main/ephemeral-vm-recipe-runner.test.ts",
"src/main/ephemeral-vm-runtime-ssh-cleanup.test.ts",
"src/main/ipc/ephemeral-vm-runtime-handler-cleanup.test.ts",
"src/main/ipc/ephemeral-vm.test.ts",
"src/main/provisioned-root-ssh-adoption.test.ts",
"src/renderer/src/lib/ephemeral-vm-failed-create-cleanup.test.ts",
"src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts",
"tests/e2e/ephemeral-vm-provisioned-root.spec.ts"
],
"assertionRefs": [
@@ -74,6 +82,23 @@
"post-create persistence failure destroys the provider resource when possible",
"failed destroy persists a durable rollback-readable cleanup recovery record"
]
},
{
"file": "src/shared/ephemeral-vm-recipe-process.test.ts",
"assertions": [
"a destroy process that never closes is killed and settled at its bounded deadline"
]
},
{
"file": "src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts",
"assertions": [
"provider failure retains runtime-owned SSH targets and confirmed success releases them",
"unknown runtime inventory retains an explicitly requested target"
]
},
{
"file": "src/renderer/src/lib/ephemeral-vm-failed-create-cleanup.test.ts",
"assertions": ["failed or unconfirmed project-host rollback prevents provider destroy"]
}
],
"evidenceRuns": [
@@ -90,10 +115,10 @@
"date": "2026-08-14",
"runner": "local",
"platform": "macos",
"command": "pnpm exec vitest run --config config/vitest.config.ts src/shared/ephemeral-vm-runtime-store.test.ts src/shared/ephemeral-vm-runtime-store-rollback.test.ts src/main/ephemeral-vm-runtime-service.test.ts src/main/ephemeral-vm-recipe-runner.test.ts src/main/ephemeral-vm-runtime-ssh-cleanup.test.ts src/main/ipc/ephemeral-vm-runtime-handler-cleanup.test.ts src/main/ipc/ephemeral-vm.test.ts src/main/provisioned-root-ssh-adoption.test.ts",
"command": "pnpm exec vitest run --config config/vitest.config.ts src/shared/ephemeral-vm-runtime-store.test.ts src/shared/ephemeral-vm-runtime-store-rollback.test.ts src/shared/ephemeral-vm-recipe-process.test.ts src/main/ephemeral-vm-runtime-service.test.ts src/main/ephemeral-vm-recipe-runner.test.ts src/main/ephemeral-vm-runtime-ssh-cleanup.test.ts src/main/ipc/ephemeral-vm-runtime-handler-cleanup.test.ts src/main/ipc/ephemeral-vm.test.ts src/main/provisioned-root-ssh-adoption.test.ts src/renderer/src/lib/ephemeral-vm-failed-create-cleanup.test.ts src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts",
"result": "passed",
"durationSeconds": 2,
"summary": "Sixty-two focused store, recipe, lifecycle, cleanup-handler, provisioning-recovery, and provisioned-root adoption tests passed."
"summary": "Seventy-nine focused store, recipe-process, lifecycle, cleanup-handler, retry-context, provisioning-recovery, and provisioned-root adoption tests passed."
},
{
"date": "2026-08-14",
@@ -119,18 +144,19 @@
},
"performanceBudget": {
"required": true,
"evidence": "Ordinary writes remain byte-identical and create no feature sidecar. Reads add one bounded feature-file existence check; sorted feature comparisons avoid unchanged rewrites, and durable companion writes occur only when compatibility metadata changes. There is no polling, timer, renderer work, RPC traffic, or remote fanout."
"evidence": "Ordinary writes remain byte-identical and create no feature sidecar. Reads add one bounded feature-file existence check; sorted feature comparisons avoid unchanged rewrites, and durable companion writes occur only when compatibility metadata changes. Cleanup adds one unrefed timer per active destroy process and clears it on settlement; there is no polling, background scan, or remote fanout."
},
"promotionCriteria": [
"Keep the pinned baseline/latest/candidate/revert oracle green in CI.",
"Keep the Docker provisioned-root create/adopt/terminal/remove/destroy journey green.",
"Keep hung destroy, fresh retry, and retained-context failure tests green.",
"Collect soak history without unexplained lifecycle or sidecar flakes."
],
"knownGaps": [
"No packaged-build downgrade installer journey; the exact released source reader and service are executed in-process.",
"Live Docker SSH evidence runs locally on macOS and in the changed-file PR E2E lane on Ubuntu; Windows desktop behavior is covered by the platform-neutral filesystem codec and package job."
],
"demotionRule": "Demote if a new write fails the pinned rollback reader, a downgrade lifecycle mutation is lost after re-upgrade, malformed/future metadata overwrites recoverable bytes, ordinary v1 bytes change, or the focused gate flakes without an identified harness defect."
"demotionRule": "Demote if a new write fails the pinned rollback reader, a downgrade lifecycle mutation is lost after re-upgrade, malformed/future metadata overwrites recoverable bytes, ordinary v1 bytes change, a destroy can remain permanently in flight, cleanup loses retry ownership, or the focused gate flakes without an identified harness defect."
},
{
"id": "terminal-session.shell-ready-exec-prompt-fallback",
@@ -159,6 +159,61 @@ describe('ephemeral VM runtime service', () => {
expect(readFileSync(join(repoPath, 'cleanup-count.txt'), 'utf8')).toBe('x')
})
it('times out a hung destroy and starts a fresh retry', async () => {
const userDataPath = makeDir('orca-ephemeral-vm-service-user-data-')
const repoPath = makeDir('orca-ephemeral-vm-service-repo-')
const cleanupPath = join(repoPath, 'cleanup.js')
const countPath = join(repoPath, 'cleanup-count.txt')
writeFileSync(
cleanupPath,
`require('fs').appendFileSync(${JSON.stringify(countPath)}, 'x'); setInterval(() => {}, 1000)`
)
const recipe: OrcaVmRecipe = {
id: 'cloud-sandbox',
name: 'Cloud Sandbox',
create: 'unused',
destroy: nodeCommand(cleanupPath)
}
upsertEphemeralVmRuntime(userDataPath, {
id: 'runtime-1',
recipeId: recipe.id,
recipe,
status: 'running',
cleanupStatus: 'not_started',
createdAt: 1_000,
updatedAt: 1_000,
recipeResult: {
schemaVersion: 1,
connection: {
type: 'ssh',
projectRoot: '/workspace/repo',
target: { label: 'VM', host: 'host', port: 22, username: 'orca' }
}
}
})
const cleanupArgs = {
userDataPath,
repoPath,
recipe,
runtimeId: 'runtime-1',
destroyTimeoutMs: 100
}
await expect(cleanupEphemeralVmRuntime(cleanupArgs)).resolves.toMatchObject({
ok: false,
runtime: { status: 'cleanup_failed', cleanupStatus: 'failed' },
error: expect.stringContaining('timed out')
})
writeFileSync(cleanupPath, `require('fs').appendFileSync(${JSON.stringify(countPath)}, 'x')`)
await expect(
cleanupEphemeralVmRuntime({ ...cleanupArgs, destroyTimeoutMs: 2_000 })
).resolves.toMatchObject({
ok: true,
runtime: { status: 'cleaned', cleanupStatus: 'succeeded' }
})
expect(readFileSync(countPath, 'utf8')).toBe('xx')
})
it('does not persist a runtime when recipe output cannot be parsed', async () => {
const userDataPath = makeDir('orca-ephemeral-vm-service-user-data-')
const repoPath = makeDir('orca-ephemeral-vm-service-repo-')
+2
View File
@@ -55,6 +55,7 @@ export type CleanupEphemeralVmRuntimeArgs = {
recipe: OrcaVmRecipe
runtimeId: string
now?: number
destroyTimeoutMs?: number
signal?: AbortSignal
onStdout?: (chunk: string) => void
onStderr?: (chunk: string) => void
@@ -184,6 +185,7 @@ async function cleanupEphemeralVmRuntimeOnce(
recipe: args.recipe,
context: contextFromRuntime(args.repoPath, running),
recipeResult: running.recipeResult,
timeoutMs: args.destroyTimeoutMs,
signal: args.signal,
onStdout: args.onStdout,
onStderr: args.onStderr
@@ -47,7 +47,7 @@ afterEach(() => {
}
})
it('removes the hidden SSH target when recipe context is unavailable', async () => {
it('removes the hidden SSH target when provider cleanup cannot start', async () => {
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-vm-runtime-handler-'))
tempDirs.push(userDataPath)
getPathMock.mockReturnValue(userDataPath)
@@ -79,6 +79,7 @@ it('removes the hidden SSH target when recipe context is unavailable', async ()
expect(cleaned).toMatchObject({
status: 'cleanup_failed',
cleanupStatus: 'failed',
connectionMode: undefined,
sshTargetId: undefined
})
expect(removeRuntimeOwnedSshTargetMock).toHaveBeenCalledWith('runtime-ssh-missing-context')
@@ -111,8 +111,9 @@ export function registerEphemeralVmRuntimeHandlers(store: Store): void {
// environment row; users can still remove that manually.
}
}
// Remove even on cleanup_failed (removal is idempotent via the deterministic
// id) so a terminal cleanup never orphans the hidden SSH target.
if (!result.ok) {
return result.runtime
}
return removeEphemeralVmRuntimeSshTarget({
userDataPath,
runtime: result.runtime,
+4 -6
View File
@@ -498,7 +498,7 @@ describe('registerEphemeralVmHandlers', () => {
expect(listEnvironments(userDataPath)).toEqual([])
})
it('removes the runtime-owned SSH target on cleanup even when destroy fails', async () => {
it('retains the runtime-owned SSH target when destroy fails', async () => {
const userDataPath = makeDir('orca-ephemeral-vm-ipc-user-data-')
const repoPath = makeDir('orca-ephemeral-vm-ipc-repo-')
getPathMock.mockReturnValue(userDataPath)
@@ -523,8 +523,6 @@ describe('registerEphemeralVmHandlers', () => {
'}))'
].join('\n')
)
// Why: a failing destroy drives the cleanup_failed branch; the runtime-owned
// SSH target must still be torn down so it never orphans (see Fix D).
writeFileSync(destroyPath, 'process.exit(1)')
writeFileSync(
join(repoPath, 'orca.yaml'),
@@ -554,9 +552,9 @@ describe('registerEphemeralVmHandlers', () => {
expect(cleaned).toEqual(expect.objectContaining({ status: 'cleanup_failed' }))
expect(cleaned.cleanupLastError).toBeTruthy()
expect(removeRuntimeOwnedSshTargetMock).toHaveBeenCalledWith('runtime-ssh-orca-instance-1')
expect(cleaned.connectionMode).toBeUndefined()
expect(cleaned.sshTargetId).toBeUndefined()
expect(removeRuntimeOwnedSshTargetMock).not.toHaveBeenCalled()
expect(cleaned.connectionMode).toBe('ssh')
expect(cleaned.sshTargetId).toBe('runtime-ssh-orca-instance-1')
})
it('retries hidden SSH teardown without rerunning completed provider cleanup', async () => {
@@ -32,6 +32,7 @@ describe('cleanupFailedEphemeralVmWorkspace', () => {
await cleanupFailedEphemeralVmWorkspace(request(), {
deleteProjectHostSetup: vi.fn(async () => {
order.push('setup')
return { id: 'setup-1' }
}),
cleanupRuntime: vi.fn(async () => {
order.push('runtime')
@@ -43,7 +44,7 @@ describe('cleanupFailedEphemeralVmWorkspace', () => {
expect(order).toEqual(['setup', 'runtime'])
})
it('still destroys the VM when setup deletion fails', async () => {
it('retains the VM when setup deletion throws', async () => {
const cleanupRuntime = vi.fn().mockResolvedValue(undefined)
const reportSetupError = vi.fn()
await cleanupFailedEphemeralVmWorkspace(request(), {
@@ -54,6 +55,20 @@ describe('cleanupFailedEphemeralVmWorkspace', () => {
})
expect(reportSetupError).toHaveBeenCalledOnce()
expect(cleanupRuntime).toHaveBeenCalledWith('runtime-1')
expect(cleanupRuntime).not.toHaveBeenCalled()
})
it('retains the VM when setup deletion reports no result', async () => {
const cleanupRuntime = vi.fn().mockResolvedValue(undefined)
const reportSetupError = vi.fn()
await cleanupFailedEphemeralVmWorkspace(request(), {
deleteProjectHostSetup: vi.fn().mockResolvedValue(null),
cleanupRuntime,
reportSetupError,
reportRuntimeError: vi.fn()
})
expect(reportSetupError).toHaveBeenCalledOnce()
expect(cleanupRuntime).not.toHaveBeenCalled()
})
})
@@ -19,9 +19,16 @@ export async function cleanupFailedEphemeralVmWorkspace(
request.workspaceRunContext?.projectHostSetupId
) {
try {
await actions.deleteProjectHostSetup(request.workspaceRunContext.projectHostSetupId)
const deleted = await actions.deleteProjectHostSetup(
request.workspaceRunContext.projectHostSetupId
)
if (deleted == null) {
actions.reportSetupError(new Error('Could not confirm project host rollback.'))
return
}
} catch (error) {
actions.reportSetupError(error)
return
}
}
try {
@@ -28,7 +28,10 @@ describe('cleanupEphemeralVmRuntimesForDeleted', () => {
expect(cleanup).toHaveBeenCalledTimes(1)
expect(cleanup).toHaveBeenCalledWith({ runtimeId: 'rt-1' })
expect(destroyed).toEqual(['runtime-ssh-a'])
expect(destroyed).toEqual({
destroyedSshTargetIds: ['runtime-ssh-a'],
retainedSshTargetIds: []
})
})
it('cleans a runtime matched only by its runtime-owned SSH target id', async () => {
@@ -43,7 +46,10 @@ describe('cleanupEphemeralVmRuntimesForDeleted', () => {
})
expect(cleanup).toHaveBeenCalledWith({ runtimeId: 'rt-1' })
expect(destroyed).toEqual(['runtime-ssh-orca-1'])
expect(destroyed).toEqual({
destroyedSshTargetIds: ['runtime-ssh-orca-1'],
retainedSshTargetIds: []
})
})
it('ignores non-runtime-owned target ids and already-cleaned runtimes', async () => {
@@ -58,7 +64,7 @@ describe('cleanupEphemeralVmRuntimesForDeleted', () => {
})
expect(cleanup).not.toHaveBeenCalled()
expect(destroyed).toEqual([])
expect(destroyed).toEqual({ destroyedSshTargetIds: [], retainedSshTargetIds: [] })
})
it('retries a completed provider cleanup while its SSH target remains', async () => {
@@ -74,7 +80,7 @@ describe('cleanupEphemeralVmRuntimesForDeleted', () => {
cleanup.mockResolvedValue({ status: 'cleaned', cleanupStatus: 'succeeded' })
await expect(cleanupEphemeralVmRuntimesForDeleted({ workspaceIds: ['wt-1'] })).resolves.toEqual(
['runtime-ssh-a']
{ destroyedSshTargetIds: ['runtime-ssh-a'], retainedSshTargetIds: [] }
)
expect(cleanup).toHaveBeenCalledWith({ runtimeId: 'rt-1' })
})
@@ -90,14 +96,27 @@ describe('cleanupEphemeralVmRuntimesForDeleted', () => {
})
await expect(cleanupEphemeralVmRuntimesForDeleted({ workspaceIds: ['wt-1'] })).resolves.toEqual(
[]
{ destroyedSshTargetIds: [], retainedSshTargetIds: ['runtime-ssh-a'] }
)
})
it('swallows listRuntimes failures', async () => {
listRuntimes.mockRejectedValue(new Error('boom'))
await expect(cleanupEphemeralVmRuntimesForDeleted({ workspaceIds: ['wt-1'] })).resolves.toEqual(
[]
{ destroyedSshTargetIds: [], retainedSshTargetIds: [] }
)
})
it('retains an explicit runtime target when runtime listing fails', async () => {
listRuntimes.mockRejectedValue(new Error('boom'))
await expect(
cleanupEphemeralVmRuntimesForDeleted({
runtimeOwnedSshTargetIds: ['runtime-ssh-a']
})
).resolves.toEqual({
destroyedSshTargetIds: [],
retainedSshTargetIds: ['runtime-ssh-a']
})
})
})
@@ -10,16 +10,21 @@ import { isRuntimeOwnedSshTargetId } from '../../../shared/execution-host'
* routes through project removal, which must not leak the live Docker/VM and its
* hidden SSH target.
*
* Returns the runtime-owned SSH target ids that were destroyed so the caller can
* purge the now-orphaned project that pointed at them.
* Classifies runtime-owned SSH targets so callers purge only confirmed-destroyed projects.
*/
export type EphemeralVmCleanupSummary = {
destroyedSshTargetIds: string[]
retainedSshTargetIds: string[]
}
export async function cleanupEphemeralVmRuntimesForDeleted(args: {
workspaceIds?: readonly string[]
// Raw runtime-owned SSH target ids (e.g. a removed repo's connectionId) whose
// backing runtime should also be torn down, even if no workspace id matched.
runtimeOwnedSshTargetIds?: readonly string[]
}): Promise<string[]> {
const destroyedSshTargetIds: string[] = []
}): Promise<EphemeralVmCleanupSummary> {
const destroyedSshTargetIds = new Set<string>()
const retainedSshTargetIds = new Set<string>()
try {
const workspaceIdSet = new Set(args.workspaceIds ?? [])
const sshTargetIdSet = new Set(
@@ -33,13 +38,29 @@ 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 (runtime.sshTargetId && !cleaned.sshTargetId) {
destroyedSshTargetIds.push(runtime.sshTargetId)
try {
const cleaned = await window.api.ephemeralVm.cleanup({ runtimeId: runtime.id })
if (runtime.sshTargetId) {
const targetIds = cleaned.sshTargetId ? retainedSshTargetIds : destroyedSshTargetIds
targetIds.add(runtime.sshTargetId)
}
} catch (error) {
console.error('Failed to clean up ephemeral VM runtime for deleted workspace:', error)
if (runtime.sshTargetId) {
retainedSshTargetIds.add(runtime.sshTargetId)
}
}
}
} catch (error) {
console.error('Failed to clean up ephemeral VM runtime for deleted workspace:', error)
for (const targetId of args.runtimeOwnedSshTargetIds ?? []) {
if (isRuntimeOwnedSshTargetId(targetId)) {
retainedSshTargetIds.add(targetId)
}
}
}
return {
destroyedSshTargetIds: [...destroyedSshTargetIds],
retainedSshTargetIds: [...retainedSshTargetIds]
}
return destroyedSshTargetIds
}
@@ -0,0 +1,49 @@
import { expect, it, vi } from 'vitest'
import { toast } from 'sonner'
import type { Repo } from '../../../../shared/repo-types'
import {
ephemeralVmCleanup,
ephemeralVmListRuntimes,
installReposRuntimeRoutingHarness,
reposRemove,
sshRepo
} from './repos-runtime-routing-fixture'
import { createTestStore } from './store-test-helpers'
vi.mock('sonner', () => ({
toast: {
error: vi.fn(),
info: vi.fn(),
success: vi.fn(),
warning: vi.fn()
}
}))
installReposRuntimeRoutingHarness()
it('retains a runtime-owned SSH project when VM cleanup fails', async () => {
const runtimeRepo: Repo = { ...sshRepo, connectionId: 'runtime-ssh-runtime-1' }
ephemeralVmListRuntimes.mockResolvedValue([
{
id: 'runtime-1',
cleanupStatus: 'not_started',
sshTargetId: runtimeRepo.connectionId
}
])
ephemeralVmCleanup.mockResolvedValue({
status: 'cleanup_failed',
cleanupStatus: 'failed',
sshTargetId: runtimeRepo.connectionId
})
const store = createTestStore()
store.setState({ repos: [runtimeRepo], activeRepoId: runtimeRepo.id })
await store.getState().removeProject(runtimeRepo.id, { errorFeedback: 'toast' })
expect(store.getState().repos).toEqual([runtimeRepo])
expect(reposRemove).not.toHaveBeenCalled()
expect(toast.error).toHaveBeenCalledWith(
expect.any(String),
expect.objectContaining({ description: expect.stringContaining('Retry cleanup') })
)
})
@@ -56,6 +56,8 @@ export const runtimeEnvironmentCall: Mock = vi.fn()
export const runtimeEnvironmentTransportCall: Mock = vi.fn()
export const orcaProfileFindProjectProfiles: Mock = vi.fn()
export const uiSet: Mock = vi.fn()
export const ephemeralVmListRuntimes: Mock = vi.fn()
export const ephemeralVmCleanup: Mock = vi.fn()
// Registers the per-test reset + window stub. Call once inside the suite's module scope.
export function installReposRuntimeRoutingHarness(): void {
@@ -86,6 +88,8 @@ export function installReposRuntimeRoutingHarness(): void {
runtimeEnvironmentTransportCall.mockReset()
uiSet.mockReset()
uiSet.mockResolvedValue(undefined)
ephemeralVmListRuntimes.mockReset().mockResolvedValue([])
ephemeralVmCleanup.mockReset()
runtimeEnvironmentTransportCall.mockImplementation((args: RuntimeEnvironmentCallRequest) => {
return createCompatibleRuntimeStatusResponseIfNeeded(args) ?? runtimeEnvironmentCall(args)
})
@@ -117,6 +121,10 @@ export function installReposRuntimeRoutingHarness(): void {
},
pty: { kill: ptyKill },
runtimeEnvironments: { call: runtimeEnvironmentTransportCall },
ephemeralVm: {
listRuntimes: ephemeralVmListRuntimes,
cleanup: ephemeralVmCleanup
},
ui: { set: uiSet }
}
})
+7 -3
View File
@@ -3602,12 +3602,16 @@ export const createRepoSlice: StateCreator<AppState, [], [], RepoSlice> = (set,
return
}
const ownerHostId = getRepoExecutionHostId(ownerRepo)
const runtimeSshTargetId = ownerRepo.connectionId
// Why: an SSH per-workspace-env's workspace is the repo's main worktree, so removal routes here; tear down its ephemeral runtime first so it doesn't leak.
if (isRuntimeOwnedSshTargetId(ownerRepo.connectionId)) {
await cleanupEphemeralVmRuntimesForDeleted({
if (runtimeSshTargetId && isRuntimeOwnedSshTargetId(runtimeSshTargetId)) {
const cleanup = await cleanupEphemeralVmRuntimesForDeleted({
workspaceIds: getKnownRepoWorktreeIds(get(), projectId, ownerHostId),
runtimeOwnedSshTargetIds: [ownerRepo.connectionId as string]
runtimeOwnedSshTargetIds: [runtimeSshTargetId]
})
if (cleanup.retainedSshTargetIds.includes(runtimeSshTargetId)) {
throw new Error('The cloud VM could not be destroyed. Retry cleanup before removing it.')
}
}
// Why: derive the target from the owner's settings (via options.hostId) so an SSH host removal never routes repo.rm to the focused runtime.
const target = getActiveRuntimeTarget(settingsForRepoOwner(get(), projectId, options?.hostId))
@@ -10573,6 +10573,7 @@ describe('pending worktree creation state', () => {
it('removePendingWorktreeCreation cleans up a provisioned-root setup and VM runtime', async () => {
const store = createTestStore()
const deleteProjectHostSetup = vi.mocked(store.getState().deleteProjectHostSetup)
deleteProjectHostSetup.mockResolvedValue({ setup: { id: 'setup-1' } } as never)
store.getState().beginPendingWorktreeCreation(
makePendingCreation('c1', {
phase: 'fetching',
+2 -2
View File
@@ -4388,11 +4388,11 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
backendOwnsPtyTeardown: true
})
// Why: dispose the SSH relay AFTER terminal teardown so a still-mounted pane can't hit a gone relay and toast "SSH not active".
const destroyedRuntimeSshTargetIds = await cleanupEphemeralVmRuntimesForDeleted({
const runtimeCleanup = await cleanupEphemeralVmRuntimesForDeleted({
workspaceIds: [worktreeId]
})
// Remove the orphaned project for the destroyed SSH target so it can't surface as a dead project in the composer.
await purgeOrphanedRuntimeSshProjects(get, destroyedRuntimeSshTargetIds)
await purgeOrphanedRuntimeSshProjects(get, runtimeCleanup.destroyedSshTargetIds)
const tabs = get().tabsByWorktree[worktreeId] ?? []
const tabIds = new Set(tabs.map((t) => t.id))
@@ -0,0 +1,32 @@
import type { ProcessRunResult } from './ephemeral-vm-recipe-process'
export const EPHEMERAL_VM_RECIPE_DESTROY_TIMEOUT_MS = 5 * 60 * 1000
type FailedEphemeralVmRecipeDestroy = {
ok: false
skipped: false
error: string
} & ProcessRunResult
export function getEphemeralVmRecipeDestroyFailure(
result: ProcessRunResult,
timeoutMs: number
): FailedEphemeralVmRecipeDestroy | null {
if (result.timedOut) {
return {
ok: false,
skipped: false,
error: `Destroy timed out after ${timeoutMs}ms.`,
...result
}
}
if (result.exitCode !== 0) {
return {
ok: false,
skipped: false,
error: `Destroy exited with code ${result.exitCode ?? 'unknown'}.`,
...result
}
}
return null
}
+31 -1
View File
@@ -1,12 +1,15 @@
import { EventEmitter } from 'node:events'
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'
import { PassThrough } from 'node:stream'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { afterEach, describe, expect, it } from 'vitest'
import { afterEach, describe, expect, it, vi } from 'vitest'
import { runRecipeCommand } from './ephemeral-vm-recipe-process'
const tmpRoots: string[] = []
afterEach(() => {
vi.useRealTimers()
for (const root of tmpRoots.splice(0)) {
rmSync(root, { recursive: true, force: true })
}
@@ -23,6 +26,33 @@ function nodeCommand(scriptPath: string): string {
}
describe('runRecipeCommand', () => {
it('settles and kills a destroy process at its deadline even if close never arrives', async () => {
vi.useFakeTimers()
const child = Object.assign(new EventEmitter(), {
pid: undefined,
stdin: new PassThrough(),
stdout: new PassThrough(),
stderr: new PassThrough(),
kill: vi.fn(),
unref: vi.fn()
})
const resultPromise = runRecipeCommand({
command: 'destroy',
repoPath: makeRepo(),
mode: 'destroy',
resultSchemaVersion: 1,
context: { recipeId: 'cloud-sandbox', repoPath: makeRepo() },
timeoutMs: 1_000,
spawnCommand: vi.fn(() => child) as never
})
await vi.advanceTimersByTimeAsync(1_000)
await expect(resultPromise).resolves.toMatchObject({ timedOut: true, exitCode: null })
expect(child.kill).toHaveBeenCalledWith('SIGKILL')
expect(child.unref).toHaveBeenCalledOnce()
})
it.each([
{ output: 'abcdef', maxCaptureBytes: 4, expected: 'cdef' },
{ output: 'A😀B', maxCaptureBytes: 5, expected: '😀B' },
+53 -12
View File
@@ -8,6 +8,7 @@ export type ProcessRunResult = {
stderr: string
exitCode: number | null
signal: NodeJS.Signals | null
timedOut?: true
}
export function quoteShellToken(value: string): string {
@@ -29,6 +30,7 @@ export async function runRecipeCommand(args: {
stdin?: string
env?: NodeJS.ProcessEnv
maxCaptureBytes?: number
timeoutMs?: number
signal?: AbortSignal
onStdout?: (chunk: string) => void
onStderr?: (chunk: string) => void
@@ -36,6 +38,9 @@ export async function runRecipeCommand(args: {
}): Promise<ProcessRunResult> {
const maxBytes = args.maxCaptureBytes ?? DEFAULT_MAX_CAPTURE_BYTES
const spawnCommand = args.spawnCommand ?? spawn
if (args.timeoutMs !== undefined && (!Number.isFinite(args.timeoutMs) || args.timeoutMs <= 0)) {
throw new Error('Recipe command timeout must be a positive finite number.')
}
return new Promise((resolve, reject) => {
let child: ChildProcessWithoutNullStreams
@@ -55,6 +60,29 @@ export async function runRecipeCommand(args: {
let stdout = ''
let stderr = ''
let settled = false
let timeout: ReturnType<typeof setTimeout> | undefined
const finish = (result: ProcessRunResult): void => {
if (settled) {
return
}
settled = true
if (timeout) {
clearTimeout(timeout)
}
args.signal?.removeEventListener('abort', abort)
resolve(result)
}
const fail = (error: Error): void => {
if (settled) {
return
}
settled = true
if (timeout) {
clearTimeout(timeout)
}
args.signal?.removeEventListener('abort', abort)
reject(error)
}
const abort = (): void => {
if (settled) {
return
@@ -62,7 +90,11 @@ export async function runRecipeCommand(args: {
killRecipeProcess(child)
}
args.signal?.addEventListener('abort', abort, { once: true })
if (args.signal?.aborted) {
abort()
} else {
args.signal?.addEventListener('abort', abort, { once: true })
}
child.stdout.setEncoding('utf8')
child.stderr.setEncoding('utf8')
@@ -75,16 +107,24 @@ export async function runRecipeCommand(args: {
args.onStderr?.(chunk)
})
child.on('error', (error) => {
settled = true
args.signal?.removeEventListener('abort', abort)
reject(error)
fail(error)
})
child.on('close', (exitCode, signal) => {
settled = true
args.signal?.removeEventListener('abort', abort)
resolve({ stdout, stderr, exitCode, signal })
finish({ stdout, stderr, exitCode, signal })
})
if (args.timeoutMs !== undefined) {
timeout = setTimeout(() => {
finish({ stdout, stderr, exitCode: null, signal: null, timedOut: true })
killRecipeProcess(child, true)
child.stdin.destroy()
child.stdout.destroy()
child.stderr.destroy()
child.unref()
}, args.timeoutMs)
timeout.unref()
}
if (args.stdin) {
child.stdin.end(args.stdin)
} else {
@@ -93,7 +133,8 @@ export async function runRecipeCommand(args: {
})
}
function killRecipeProcess(child: ChildProcessWithoutNullStreams): void {
function killRecipeProcess(child: ChildProcessWithoutNullStreams, force = false): void {
const signal = force ? 'SIGKILL' : 'SIGTERM'
if (process.platform === 'win32') {
// Recipes run through `cmd.exe /c` (shell: true), so child.kill() would only
// terminate the wrapper and orphan the actual recipe subprocess (e.g. a cloud
@@ -103,22 +144,22 @@ function killRecipeProcess(child: ChildProcessWithoutNullStreams): void {
windowsHide: true,
stdio: 'ignore'
})
killer.on('error', () => child.kill())
killer.on('error', () => child.kill(signal))
return
}
child.kill()
child.kill(signal)
return
}
if (child.pid) {
try {
// Recipes run through a shell; kill the process group so shell children do not linger.
process.kill(-child.pid, 'SIGTERM')
process.kill(-child.pid, signal)
return
} catch {
// Fall back to killing the direct child if the process group is already gone.
}
}
child.kill()
child.kill(signal)
}
function buildRecipeEnv(
+10 -7
View File
@@ -8,6 +8,10 @@ import {
getEphemeralVmRecipeResultSchemaVersion
} from './ephemeral-vm-recipe-checkout-mode'
import { runRecipeCommand } from './ephemeral-vm-recipe-process'
import {
EPHEMERAL_VM_RECIPE_DESTROY_TIMEOUT_MS,
getEphemeralVmRecipeDestroyFailure
} from './ephemeral-vm-recipe-destroy-result'
import {
buildEphemeralVmRecipeCleanupPayload,
buildEphemeralVmRecipeLifecyclePayload
@@ -74,6 +78,7 @@ export type EphemeralVmRecipeCleanupArgs = {
recipeResult: EphemeralVmRecipeResult
env?: NodeJS.ProcessEnv
maxCaptureBytes?: number
timeoutMs?: number
signal?: AbortSignal
onStdout?: (chunk: string) => void
onStderr?: (chunk: string) => void
@@ -169,6 +174,7 @@ export async function runEphemeralVmRecipeCleanup(
}
const payload = buildEphemeralVmRecipeCleanupPayload(args)
const timeoutMs = args.timeoutMs ?? EPHEMERAL_VM_RECIPE_DESTROY_TIMEOUT_MS
const processResult = await runRecipeCommand({
command: args.recipe.destroy,
repoPath: args.repoPath,
@@ -178,19 +184,16 @@ export async function runEphemeralVmRecipeCleanup(
stdin: `${JSON.stringify(payload)}\n`,
env: args.env,
maxCaptureBytes: args.maxCaptureBytes,
timeoutMs,
signal: args.signal,
onStdout: args.onStdout,
onStderr: args.onStderr,
spawnCommand: args.spawnCommand
})
if (processResult.exitCode !== 0) {
return {
ok: false,
skipped: false,
error: `Destroy exited with code ${processResult.exitCode ?? 'unknown'}.`,
...processResult
}
const failure = getEphemeralVmRecipeDestroyFailure(processResult, timeoutMs)
if (failure) {
return failure
}
return { ok: true, skipped: false, ...processResult }