mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 00:57:45 +00:00
fix(runtime-environments): drop a recipe VM's rows and session partition when cleanup forgets its server (#33805) (#34101)
Recipe cleanup removed the saved server without the retirement a Settings removal runs, so the host's session partition stayed (listings kept it under omittedHostIds) and the sidebar kept the VM's primary checkout until the next catalog read. Cleanup now retires the server through the same routine as a Settings removal, the renderer re-reads the catalog once cleanup returns, a catalog read that started before a change is followed by one more, and registry edits made outside Settings (CLI environment rm, recipe cleanup) also drop the removed host's session partitions.
This commit is contained in:
@@ -8,10 +8,6 @@ const mocks = vi.hoisted(() => ({
|
||||
closeTunnel: vi.fn(async () => {})
|
||||
}))
|
||||
|
||||
vi.mock('../runtime/desktop-chat-creation-history', () => ({
|
||||
desktopChatCreationHistory: () => ({ removeEnvironment: mocks.retire })
|
||||
}))
|
||||
|
||||
vi.mock('../../shared/runtime-environment-store', () => ({
|
||||
listEnvironments: mocks.list,
|
||||
removeEnvironment: mocks.remove
|
||||
@@ -35,10 +31,11 @@ describe('releaseEphemeralVmRuntimeEnvironments', () => {
|
||||
{ id: 'env-vm', orcadDeployment: { sshTargetId: 'runtime-ssh-vm' } },
|
||||
{ id: 'env-other', orcadDeployment: { sshTargetId: 'ssh-builder' } }
|
||||
])
|
||||
await releaseEphemeralVmRuntimeEnvironments('/user-data', {
|
||||
runtimeEnvironmentId: 'env-vm',
|
||||
sshTargetId: 'runtime-ssh-vm'
|
||||
})
|
||||
await releaseEphemeralVmRuntimeEnvironments(
|
||||
'/user-data',
|
||||
{ runtimeEnvironmentId: 'env-vm', sshTargetId: 'runtime-ssh-vm' },
|
||||
mocks.retire
|
||||
)
|
||||
expect(mocks.closeTunnel).toHaveBeenCalledTimes(1)
|
||||
expect(mocks.closeTunnel).toHaveBeenCalledWith('env-vm')
|
||||
expect(mocks.removeManaged).toHaveBeenCalledTimes(1)
|
||||
@@ -54,15 +51,21 @@ describe('releaseEphemeralVmRuntimeEnvironments', () => {
|
||||
mocks.list.mockReturnValue([
|
||||
{ id: 'env-converted', orcadDeployment: { sshTargetId: 'runtime-ssh-vm' } }
|
||||
])
|
||||
await releaseEphemeralVmRuntimeEnvironments('/user-data', { sshTargetId: 'runtime-ssh-vm' })
|
||||
await releaseEphemeralVmRuntimeEnvironments(
|
||||
'/user-data',
|
||||
{ sshTargetId: 'runtime-ssh-vm' },
|
||||
mocks.retire
|
||||
)
|
||||
expect(mocks.removeManaged).toHaveBeenCalledWith('/user-data', 'env-converted')
|
||||
})
|
||||
|
||||
it('removes a paired recipe server the ordinary way', async () => {
|
||||
mocks.list.mockReturnValue([{ id: 'env-paired' }])
|
||||
await releaseEphemeralVmRuntimeEnvironments('/user-data', {
|
||||
runtimeEnvironmentId: 'env-paired'
|
||||
})
|
||||
await releaseEphemeralVmRuntimeEnvironments(
|
||||
'/user-data',
|
||||
{ runtimeEnvironmentId: 'env-paired' },
|
||||
mocks.retire
|
||||
)
|
||||
expect(mocks.remove).toHaveBeenCalledWith('/user-data', 'env-paired')
|
||||
expect(mocks.retire).toHaveBeenCalledWith('env-paired')
|
||||
expect(mocks.remove.mock.invocationCallOrder[0]).toBeLessThan(
|
||||
@@ -73,7 +76,11 @@ describe('releaseEphemeralVmRuntimeEnvironments', () => {
|
||||
|
||||
it('ignores a server that is already gone', async () => {
|
||||
mocks.list.mockReturnValue([])
|
||||
await releaseEphemeralVmRuntimeEnvironments('/user-data', { runtimeEnvironmentId: 'env-gone' })
|
||||
await releaseEphemeralVmRuntimeEnvironments(
|
||||
'/user-data',
|
||||
{ runtimeEnvironmentId: 'env-gone' },
|
||||
mocks.retire
|
||||
)
|
||||
expect(mocks.remove).not.toHaveBeenCalled()
|
||||
expect(mocks.removeManaged).not.toHaveBeenCalled()
|
||||
expect(mocks.retire).not.toHaveBeenCalled()
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
import { desktopChatCreationHistory } from '../runtime/desktop-chat-creation-history'
|
||||
import type { EphemeralVmRuntimeRecord } from '../../shared/ephemeral-vm-runtimes'
|
||||
import { removeManagedOrcadEnvironment } from '../../shared/runtime-environment-managed-orcad-store'
|
||||
import { listEnvironments, removeEnvironment } from '../../shared/runtime-environment-store'
|
||||
@@ -7,7 +6,9 @@ import { getOrcadSshDeployment } from '../../shared/runtime-environments'
|
||||
/** Remove destroyed VM servers so managed tunnels stop reconnecting. */
|
||||
export async function releaseEphemeralVmRuntimeEnvironments(
|
||||
userDataPath: string,
|
||||
runtime: Pick<EphemeralVmRuntimeRecord, 'runtimeEnvironmentId' | 'sshTargetId'>
|
||||
runtime: Pick<EphemeralVmRuntimeRecord, 'runtimeEnvironmentId' | 'sshTargetId'>,
|
||||
// Same retirement as a Settings removal, so the host's session partition and transport go too.
|
||||
retireRemovedEnvironment: (environmentId: string) => void
|
||||
): Promise<void> {
|
||||
// Relay-era conversions may have recorded only the SSH target, not the server ID.
|
||||
const environments = listEnvironments(userDataPath).filter(
|
||||
@@ -19,13 +20,13 @@ export async function releaseEphemeralVmRuntimeEnvironments(
|
||||
for (const environment of environments) {
|
||||
if (!environment.orcadDeployment) {
|
||||
removeEnvironment(userDataPath, environment.id)
|
||||
void desktopChatCreationHistory(userDataPath).removeEnvironment(environment.id)
|
||||
retireRemovedEnvironment(environment.id)
|
||||
continue
|
||||
}
|
||||
const { closeOrcadManagedTunnel } =
|
||||
await import('../managed-server/lifecycle/orcad-managed-tunnel')
|
||||
await closeOrcadManagedTunnel(environment.id).catch(() => undefined)
|
||||
removeManagedOrcadEnvironment(userDataPath, environment.id)
|
||||
void desktopChatCreationHistory(userDataPath).removeEnvironment(environment.id)
|
||||
retireRemovedEnvironment(environment.id)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, beforeEach, expect, it, vi } from 'vitest'
|
||||
import { upsertEphemeralVmRuntime } from '../../shared/ephemeral-vm-runtime-store'
|
||||
import type * as RuntimeEnvironmentStore from '../../shared/runtime-environment-store'
|
||||
|
||||
const handlers = new Map<string, (_event: unknown, args: { runtimeId: string }) => unknown>()
|
||||
const { getPathMock, handleMock, removeRuntimeOwnedSshTargetMock, removeHandlerMock } = vi.hoisted(
|
||||
@@ -19,6 +20,21 @@ vi.mock('electron', () => ({
|
||||
ipcMain: { handle: handleMock, removeHandler: removeHandlerMock }
|
||||
}))
|
||||
|
||||
const { listEnvironmentsMock, removeEnvironmentMock } = vi.hoisted(() => ({
|
||||
listEnvironmentsMock: vi.fn((): { id: string }[] => []),
|
||||
removeEnvironmentMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('../../shared/runtime-environment-store', async (importOriginal) => ({
|
||||
...(await importOriginal<typeof RuntimeEnvironmentStore>()),
|
||||
listEnvironments: listEnvironmentsMock,
|
||||
removeEnvironment: removeEnvironmentMock
|
||||
}))
|
||||
|
||||
vi.mock('./runtime-environments/runtime-environments', () => ({
|
||||
invalidateRuntimeEnvironmentTransport: vi.fn(async () => {})
|
||||
}))
|
||||
|
||||
vi.mock('../ephemeral-vm/runtime-ssh', () => ({
|
||||
connectRuntimeOwnedSshTarget: vi.fn(),
|
||||
disconnectRuntimeOwnedSshTarget: vi.fn(),
|
||||
@@ -148,3 +164,55 @@ it('stops in-flight cleanup and retains the runtime for retry', async () => {
|
||||
})
|
||||
await expect(cleanup).resolves.toMatchObject({ status: 'cleanup_failed' })
|
||||
})
|
||||
|
||||
it("retires a recipe server's session partition once its VM is destroyed (#33805)", async () => {
|
||||
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-vm-runtime-handler-'))
|
||||
const repoPath = mkdtempSync(join(tmpdir(), 'orca-vm-runtime-repo-'))
|
||||
tempDirs.push(userDataPath, repoPath)
|
||||
getPathMock.mockReturnValue(userDataPath)
|
||||
const destroyPath = join(repoPath, 'destroy.js')
|
||||
writeFileSync(destroyPath, 'process.exit(0)')
|
||||
listEnvironmentsMock.mockReturnValue([{ id: 'env-recipe' }])
|
||||
upsertEphemeralVmRuntime(userDataPath, {
|
||||
id: 'runtime-paired',
|
||||
recipeId: 'cloud-sandbox',
|
||||
recipe: {
|
||||
id: 'cloud-sandbox',
|
||||
name: 'Cloud Sandbox',
|
||||
create: 'unused',
|
||||
destroy: nodeCommand(destroyPath)
|
||||
},
|
||||
repoId: 'repo-1',
|
||||
workspaceId: 'test1',
|
||||
runtimeEnvironmentId: 'env-recipe',
|
||||
status: 'running',
|
||||
cleanupStatus: 'not_started',
|
||||
createdAt: 1,
|
||||
updatedAt: 1,
|
||||
recipeResult: {
|
||||
schemaVersion: 1,
|
||||
connection: { type: 'orca-server', pairingCode: 'unused', projectRoot: '/workspace/repo' }
|
||||
}
|
||||
})
|
||||
const removeWorkspaceSessionHost = vi.fn()
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: cleanup reads only getRepo and the removed-server store methods this fake implements.
|
||||
registerEphemeralVmRuntimeHandlers({
|
||||
getRepo: vi.fn(() => ({
|
||||
id: 'repo-1',
|
||||
path: repoPath,
|
||||
displayName: 'Repo',
|
||||
badgeColor: '#000',
|
||||
addedAt: 0
|
||||
})),
|
||||
removeWorkspaceSessionHost,
|
||||
getRemoteLayoutSnapshot: vi.fn(),
|
||||
setRemoteLayoutSnapshot: vi.fn()
|
||||
} as never)
|
||||
|
||||
await expect(
|
||||
handlers.get('ephemeralVm:cleanup')?.(null, { runtimeId: 'runtime-paired' })
|
||||
).resolves.toMatchObject({ cleanupStatus: 'succeeded' })
|
||||
|
||||
expect(removeEnvironmentMock).toHaveBeenCalledWith(userDataPath, 'env-recipe')
|
||||
expect(removeWorkspaceSessionHost).toHaveBeenCalledWith('runtime:env-recipe')
|
||||
})
|
||||
|
||||
@@ -27,6 +27,7 @@ import {
|
||||
} from '../ephemeral-vm/runtime-ssh'
|
||||
import { getRuntimeRecipeContext } from './ephemeral-vm-recipe-context'
|
||||
import { invalidateRuntimeEnvironmentTransport } from './runtime-environments/runtime-environments'
|
||||
import { retireRemovedRuntimeEnvironment } from './runtime-environments/runtime-environment-removal-cleanup'
|
||||
import { attachEphemeralVmRuntimeToWorkspace } from '../ephemeral-vm/runtime-attachment'
|
||||
|
||||
export type EphemeralVmCleanupCommandResult = {
|
||||
@@ -102,7 +103,14 @@ export function registerEphemeralVmRuntimeHandlers(store: Store): void {
|
||||
}
|
||||
if (result.ok) {
|
||||
try {
|
||||
await releaseEphemeralVmRuntimeEnvironments(userDataPath, runtime)
|
||||
await releaseEphemeralVmRuntimeEnvironments(userDataPath, runtime, (environmentId) => {
|
||||
void retireRemovedRuntimeEnvironment(
|
||||
environmentId,
|
||||
invalidateRuntimeEnvironmentTransport,
|
||||
store,
|
||||
userDataPath
|
||||
)
|
||||
})
|
||||
} catch {
|
||||
// Cleanup of provider resources matters more than hiding a stale local
|
||||
// environment row; users can still remove that manually.
|
||||
|
||||
@@ -117,11 +117,11 @@ let stopPreferenceWatch: (() => void) | undefined
|
||||
|
||||
export function registerRuntimeEnvironmentHandlers(store: Store): void {
|
||||
stopPreferenceWatch?.()
|
||||
stopPreferenceWatch = watchRuntimeEnvironmentPreference(
|
||||
store,
|
||||
getUserDataPath(),
|
||||
publishRuntimeEnvironmentCatalogChanged
|
||||
)
|
||||
stopPreferenceWatch = watchRuntimeEnvironmentPreference(store, getUserDataPath(), () => {
|
||||
// CLI and recipe removals skip the Settings path; drop their session partitions here too.
|
||||
reconcileOrphanedRuntimeSessions(store, getUserDataPath())
|
||||
publishRuntimeEnvironmentCatalogChanged()
|
||||
})
|
||||
for (const pending of pendingSubscriptions.values()) {
|
||||
pending.close()
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@ globalThis.window = { api: { ephemeralVm: { listRuntimes, cleanup } } }
|
||||
|
||||
import { cleanupEphemeralVmRuntimesForDeleted } from './ephemeral-vm-runtime-cleanup'
|
||||
import { toSshExecutionHostId } from '../../../shared/execution-host'
|
||||
import { setRuntimeEnvironmentCatalogRefresher } from '@/runtime/runtime-environment-pairing-refresh'
|
||||
|
||||
function runtime(overrides: Record<string, unknown>): Record<string, unknown> {
|
||||
return { id: 'rt', cleanupStatus: 'not_started', ...overrides }
|
||||
@@ -35,6 +36,24 @@ describe('cleanupEphemeralVmRuntimesForDeleted', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it("re-reads the server catalog once a cleanup may have forgotten the VM's server (#33805)", async () => {
|
||||
listRuntimes.mockResolvedValue([
|
||||
runtime({ id: 'rt-a', workspaceId: 'wt-1', runtimeEnvironmentId: 'env-a' })
|
||||
])
|
||||
const refresher = vi.fn(async () => {})
|
||||
setRuntimeEnvironmentCatalogRefresher(refresher)
|
||||
try {
|
||||
await cleanupEphemeralVmRuntimesForDeleted({ workspaceIds: ['wt-1'] })
|
||||
} finally {
|
||||
setRuntimeEnvironmentCatalogRefresher(null)
|
||||
}
|
||||
|
||||
expect(refresher).toHaveBeenCalledTimes(1)
|
||||
expect(refresher.mock.invocationCallOrder[0]).toBeGreaterThan(
|
||||
cleanup.mock.invocationCallOrder[0]
|
||||
)
|
||||
})
|
||||
|
||||
it('cleans only the confirmed host runtime when another host owns the same workspace id', async () => {
|
||||
listRuntimes.mockResolvedValue([
|
||||
runtime({
|
||||
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
type ExecutionHostId
|
||||
} from '../../../shared/execution-host'
|
||||
import { composeWorktreeHostIdentity } from '../../../shared/worktree/host-qualified-identity'
|
||||
import { refreshRuntimeEnvironmentCatalog } from '@/runtime/runtime-environment-pairing-refresh'
|
||||
|
||||
/** Destroy runtimes for removed workspaces and runtime-owned SSH targets. Share with project removal because SSH-mode main worktrees delete through that path; classify targets so callers purge only confirmed destruction. */
|
||||
export type EphemeralVmCleanupSummary = {
|
||||
@@ -56,9 +57,12 @@ export async function cleanupEphemeralVmRuntimesForDeleted(args: {
|
||||
)))) ||
|
||||
(runtime.sshTargetId !== undefined && sshTargetIdSet.has(runtime.sshTargetId)))
|
||||
)
|
||||
let releasedServer = false
|
||||
for (const runtime of matchingRuntimes) {
|
||||
try {
|
||||
const cleaned = await window.api.ephemeralVm.cleanup({ runtimeId: runtime.id })
|
||||
releasedServer ||=
|
||||
runtime.runtimeEnvironmentId !== undefined || runtime.sshTargetId !== undefined
|
||||
if (runtime.sshTargetId) {
|
||||
const targetIds = cleaned.sshTargetId ? retainedSshTargetIds : destroyedSshTargetIds
|
||||
targetIds.add(runtime.sshTargetId)
|
||||
@@ -70,6 +74,10 @@ export async function cleanupEphemeralVmRuntimesForDeleted(args: {
|
||||
}
|
||||
}
|
||||
}
|
||||
if (releasedServer) {
|
||||
// Cleanup can forget the VM's server; purge its rows now instead of on the next catalog read.
|
||||
await refreshRuntimeEnvironmentCatalog()
|
||||
}
|
||||
} catch (error) {
|
||||
console.error('Failed to clean up ephemeral VM runtime for deleted workspace:', error)
|
||||
for (const targetId of args.runtimeOwnedSshTargetIds ?? []) {
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
replaceRuntimeEnvironmentRevisions
|
||||
} from './runtime-environment-revision'
|
||||
import {
|
||||
refreshRuntimeEnvironmentCatalog,
|
||||
setRuntimeEnvironmentCatalogRefresher,
|
||||
subscribeRuntimeEnvironment
|
||||
} from './runtime-environment-pairing-refresh'
|
||||
@@ -93,3 +94,31 @@ describe('a pairing rotation under a live subscriber', () => {
|
||||
await expect(request()).resolves.toMatchObject({ ok: true })
|
||||
})
|
||||
})
|
||||
|
||||
describe('catalog refresh coalescing', () => {
|
||||
it('reads again after an in-flight read that may predate the change', async () => {
|
||||
let catalog = ['env-1', 'env-recipe']
|
||||
const published: string[][] = []
|
||||
const releaseFirstRead = Promise.withResolvers<void>()
|
||||
let reads = 0
|
||||
setRuntimeEnvironmentCatalogRefresher(async () => {
|
||||
const snapshot = [...catalog]
|
||||
reads += 1
|
||||
if (reads === 1) {
|
||||
await releaseFirstRead.promise
|
||||
}
|
||||
published.push(snapshot)
|
||||
})
|
||||
|
||||
const first = refreshRuntimeEnvironmentCatalog()
|
||||
// Recipe cleanup forgets the server while the earlier read is still out.
|
||||
catalog = ['env-1']
|
||||
const afterRemoval = refreshRuntimeEnvironmentCatalog()
|
||||
const sameBurst = refreshRuntimeEnvironmentCatalog()
|
||||
releaseFirstRead.resolve()
|
||||
await Promise.all([first, afterRemoval, sameBurst])
|
||||
|
||||
expect(reads).toBe(2)
|
||||
expect(published.at(-1)).toEqual(['env-1'])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -5,6 +5,7 @@ type CatalogRefresher = () => Promise<void>
|
||||
|
||||
let refreshCatalog: CatalogRefresher | null = null
|
||||
let inFlight: Promise<void> | null = null
|
||||
let trailing: Promise<void> | null = null
|
||||
|
||||
export function setRuntimeEnvironmentCatalogRefresher(refresher: CatalogRefresher | null): void {
|
||||
refreshCatalog = refresher
|
||||
@@ -31,7 +32,15 @@ export function refreshRuntimeEnvironmentCatalog(): Promise<void> {
|
||||
if (!refreshCatalog) {
|
||||
return Promise.resolve()
|
||||
}
|
||||
inFlight ??= refreshCatalog()
|
||||
if (inFlight) {
|
||||
// The running read may predate this caller's change, so queue one more for the whole burst.
|
||||
trailing ??= inFlight.then(() => {
|
||||
trailing = null
|
||||
return refreshRuntimeEnvironmentCatalog()
|
||||
})
|
||||
return trailing
|
||||
}
|
||||
inFlight = refreshCatalog()
|
||||
.catch((refreshError: unknown) => {
|
||||
console.warn(
|
||||
'[runtime-environments] Could not re-read a re-paired environment:',
|
||||
|
||||
Reference in New Issue
Block a user