diff --git a/src/main/ephemeral-vm/runtime-environment-release.test.ts b/src/main/ephemeral-vm/runtime-environment-release.test.ts index efba558d388..d74f0a034db 100644 --- a/src/main/ephemeral-vm/runtime-environment-release.test.ts +++ b/src/main/ephemeral-vm/runtime-environment-release.test.ts @@ -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() diff --git a/src/main/ephemeral-vm/runtime-environment-release.ts b/src/main/ephemeral-vm/runtime-environment-release.ts index 31f3299703c..c2dbe4ce7e5 100644 --- a/src/main/ephemeral-vm/runtime-environment-release.ts +++ b/src/main/ephemeral-vm/runtime-environment-release.ts @@ -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 + runtime: Pick, + // Same retirement as a Settings removal, so the host's session partition and transport go too. + retireRemovedEnvironment: (environmentId: string) => void ): Promise { // 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) } } diff --git a/src/main/ipc/ephemeral-vm-runtime-handler-cleanup.test.ts b/src/main/ipc/ephemeral-vm-runtime-handler-cleanup.test.ts index 0dbef129556..2a271058a9b 100644 --- a/src/main/ipc/ephemeral-vm-runtime-handler-cleanup.test.ts +++ b/src/main/ipc/ephemeral-vm-runtime-handler-cleanup.test.ts @@ -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 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()), + 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') +}) diff --git a/src/main/ipc/ephemeral-vm-runtime-handlers.ts b/src/main/ipc/ephemeral-vm-runtime-handlers.ts index 898e997259a..fada16a9e16 100644 --- a/src/main/ipc/ephemeral-vm-runtime-handlers.ts +++ b/src/main/ipc/ephemeral-vm-runtime-handlers.ts @@ -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. diff --git a/src/main/ipc/runtime-environments/runtime-environments.ts b/src/main/ipc/runtime-environments/runtime-environments.ts index 253e18fb639..118ab3ab940 100644 --- a/src/main/ipc/runtime-environments/runtime-environments.ts +++ b/src/main/ipc/runtime-environments/runtime-environments.ts @@ -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() } diff --git a/src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts b/src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts index 86f8e4c6dbf..5fb01b2febc 100644 --- a/src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts +++ b/src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts @@ -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): Record { 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({ diff --git a/src/renderer/src/lib/ephemeral-vm-runtime-cleanup.ts b/src/renderer/src/lib/ephemeral-vm-runtime-cleanup.ts index 70d3204dc72..f03ecc82a88 100644 --- a/src/renderer/src/lib/ephemeral-vm-runtime-cleanup.ts +++ b/src/renderer/src/lib/ephemeral-vm-runtime-cleanup.ts @@ -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 ?? []) { diff --git a/src/renderer/src/runtime/runtime-environment-pairing-refresh.test.ts b/src/renderer/src/runtime/runtime-environment-pairing-refresh.test.ts index f8789d69284..8487f805d57 100644 --- a/src/renderer/src/runtime/runtime-environment-pairing-refresh.test.ts +++ b/src/renderer/src/runtime/runtime-environment-pairing-refresh.test.ts @@ -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() + 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']) + }) +}) diff --git a/src/renderer/src/runtime/runtime-environment-pairing-refresh.ts b/src/renderer/src/runtime/runtime-environment-pairing-refresh.ts index b27bcfb80c2..56c82192840 100644 --- a/src/renderer/src/runtime/runtime-environment-pairing-refresh.ts +++ b/src/renderer/src/runtime/runtime-environment-pairing-refresh.ts @@ -5,6 +5,7 @@ type CatalogRefresher = () => Promise let refreshCatalog: CatalogRefresher | null = null let inFlight: Promise | null = null +let trailing: Promise | null = null export function setRuntimeEnvironmentCatalogRefresher(refresher: CatalogRefresher | null): void { refreshCatalog = refresher @@ -31,7 +32,15 @@ export function refreshRuntimeEnvironmentCatalog(): Promise { 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:',