From 75a7aaf88702afb8c812d73b3a58f2abb6a42cd1 Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 6 Oct 2026 14:43:28 -0700 Subject: [PATCH 1/6] fix: retire removed runtime host transports Co-authored-by: CC --- ...e-environment-cli-removal.electron.test.ts | 216 ++++++++++++++++++ .../runtime-environment-removal-watch.test.ts | 188 +++++++++++++++ .../ipc/runtime-environment-removal-watch.ts | 103 +++++++++ ...me-environment-request-connections.test.ts | 46 ++++ ...runtime-environment-request-connections.ts | 13 ++ src/main/ipc/runtime-environments.ts | 5 + ...-runtime-shared-control-connection.test.ts | 155 +++++++++++++ ...emote-runtime-shared-control-connection.ts | 57 +++-- ...ntime-shared-control-diagnostics-inputs.ts | 18 ++ .../remote-runtime-shared-control-open.ts | 9 + ...e-runtime-shared-control-reconnect.test.ts | 65 ++++++ ...remote-runtime-shared-control-reconnect.ts | 9 + ...mote-runtime-shared-control-test-server.ts | 5 + .../remote-runtime-shared-control-types.ts | 3 + 14 files changed, 867 insertions(+), 25 deletions(-) create mode 100644 src/main/ipc/runtime-environment-cli-removal.electron.test.ts create mode 100644 src/main/ipc/runtime-environment-removal-watch.test.ts create mode 100644 src/main/ipc/runtime-environment-removal-watch.ts create mode 100644 src/shared/remote-runtime-shared-control-diagnostics-inputs.ts diff --git a/src/main/ipc/runtime-environment-cli-removal.electron.test.ts b/src/main/ipc/runtime-environment-cli-removal.electron.test.ts new file mode 100644 index 000000000000..2363e2c33b30 --- /dev/null +++ b/src/main/ipc/runtime-environment-cli-removal.electron.test.ts @@ -0,0 +1,216 @@ +import { + spawnProcess, + runProcess, + type ChildProcessHandle +} from '../../shared/child-process/run-process' +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + symlinkSync, + writeFileSync +} from 'node:fs' +import { builtinModules, createRequire } from 'node:module' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterAll, describe, expect, it } from 'vitest' +import { build as buildVite } from 'vite' +import { resolveElectronProbeLaunch } from '../browser/electron-probe-display-launch' +import { REMOTE_RUNTIME_SOCKET_PING_INTERVAL_MS } from '../../shared/remote-runtime-socket-liveness' +import { REMOTE_RUNTIME_SHARED_CONTROL_CAPABILITY } from '../../shared/protocol-version' +import { encodePairingOffer } from '../../shared/pairing' +import { addEnvironmentFromPairingCode } from '../../shared/runtime-environment-store' +import { + closeSharedControlTestServers, + createSharedControlTestServer +} from '../../shared/remote-runtime-shared-control-test-server' + +// Why a real Electron process: the registration in `runtime-environments.ts` that hands the removal +// watch its user-data path and its teardown only runs inside the shipped main process, and the +// retirement rides the socket's own liveness timer. A vitest-only harness can reach neither, so +// #20995 -- the CLI removes an environment and the app keeps the socket -- could only be reproduced +// here. The store rewrite is issued from this process, which is what `orca environment rm` is. + +const electronBinary = resolveElectronBinary() +const roots: string[] = [] +let child: ChildProcessHandle | null = null + +afterAll(async () => { + child?.kill('SIGKILL') + await closeSharedControlTestServers() + if (process.env.ORCA_KEEP_FIXTURE === '1') { + return + } + for (const root of roots) { + rmSync(root, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }) + } +}) + +function resolveElectronBinary(): string { + const resolved: unknown = createRequire(import.meta.url)('electron') + if (typeof resolved !== 'string') { + throw new Error('The electron package did not resolve to a binary path') + } + return resolved +} + +function buildFixtureMain(bundlePath: string, logPath: string, userDataPath: string): string { + return [ + `const { app } = require('electron')`, + `app.setPath('userData', ${JSON.stringify(userDataPath)})`, + `const { appendFileSync } = require('node:fs')`, + `const log = (message) => appendFileSync(${JSON.stringify(logPath)}, message + '\\n')`, + `process.on('uncaughtException', (error) => log('uncaught ' + (error && error.stack)))`, + `process.on('unhandledRejection', (error) => log('unhandled ' + (error && error.stack)))`, + `const runtime = require(${JSON.stringify(bundlePath)})`, + `runtime.applyBackgroundActivationPolicy()`, + `app.whenReady().then(() => {`, + ` log('ready ' + app.getPath('userData'))`, + ` runtime.registerRuntimeEnvironmentHandlers({ getSettings: () => ({}) })`, + ` log('registered')`, + `}).catch((error) => log('ready-failed ' + (error && error.stack)))` + ].join('\n') +} + +async function waitFor( + predicate: () => boolean, + timeoutMs: number, + what: () => string +): Promise { + const startedAt = Date.now() + while (!predicate()) { + if (Date.now() - startedAt > timeoutMs) { + throw new Error(`Timed out waiting for ${what()}`) + } + await new Promise((resolve) => setTimeout(resolve, 100)) + } +} + +function readFixtureLog(root: string): string { + const path = join(root, 'fixture.log') + return existsSync(path) ? readFileSync(path, 'utf8') : '(no fixture log)' +} + +const delay = (ms: number): Promise => new Promise((resolve) => setTimeout(resolve, ms)) + +describe('runtime environment removed by the CLI', () => { + it('drops the live socket the desktop app was holding and never reconnects', async () => { + const root = mkdtempSync(join(tmpdir(), 'orca-cli-removal-')) + roots.push(root) + const userDataPath = join(root, 'profile') + mkdirSync(userDataPath, { recursive: true }) + const fixtureDir = join(root, 'fixture') + mkdirSync(fixtureDir, { recursive: true }) + symlinkSync( + join(process.cwd(), 'node_modules'), + join(fixtureDir, 'node_modules'), + process.platform === 'win32' ? 'junction' : 'dir' + ) + + // The app only keeps a standing shared-control socket for a host that advertises the capability. + const server = await createSharedControlTestServer({ + resultForRequest: (method) => + method === 'status.get' + ? { capabilities: [REMOTE_RUNTIME_SHARED_CONTROL_CAPABILITY], runtimeId: 'runtime-test' } + : { method } + }) + const environment = addEnvironmentFromPairingCode(userDataPath, { + name: 'removal-host', + pairingCode: encodePairingOffer(server.pairing) + }) + + const entryPath = join(fixtureDir, 'entry.ts') + writeFileSync( + entryPath, + `export { registerRuntimeEnvironmentHandlers } from ${JSON.stringify(join(process.cwd(), 'src/main/ipc/runtime-environments.ts'))}\nexport { applyBackgroundActivationPolicy } from ${JSON.stringify(join(process.cwd(), 'src/main/window/foreground-activation-policy.ts'))}` + ) + await buildVite({ + configFile: false, + logLevel: 'silent', + // Why `node` conditions and the bare-builtin externals: the default browser resolution stubs + // `crypto` and the bundle throws on load instead of running the shipped code. + resolve: { conditions: ['node'] }, + build: { + emptyOutDir: false, + lib: { entry: entryPath, formats: ['cjs'], fileName: () => 'runtime-environments.cjs' }, + outDir: fixtureDir, + target: 'node20', + rollupOptions: { external: ['electron', /^node:/, ...builtinModules] } + } + }) + + writeFileSync( + join(fixtureDir, 'package.json'), + '{ "name": "orca-cli-removal-fixture", "main": "main.js" }' + ) + writeFileSync( + join(fixtureDir, 'main.js'), + buildFixtureMain( + join(fixtureDir, 'runtime-environments.cjs'), + join(root, 'fixture.log'), + userDataPath + ) + ) + + const { ELECTRON_RUN_AS_NODE: _runAsNode, ...env } = process.env + const electronArgs = [fixtureDir, `--user-data-dir=${userDataPath}`] + const { executable, args } = resolveElectronProbeLaunch({ + electronBinary, + electronArgs, + platform: process.platform, + display: env.DISPLAY + }) + child = spawnProcess({ + program: executable, + args, + env: { ...env, ORCA_BACKGROUND_LAUNCH: '1' }, + stdio: ['ignore', 'pipe', 'pipe'] + }) + let stderr = '' + child.stderr?.on('data', (chunk: Buffer) => { + stderr += chunk.toString() + }) + + // The app connects on its own at startup, exactly as it does for a stored environment. + await waitFor( + () => + server.connectionCount() >= 2 && server.openClientCount() === 1 && server.auths.length >= 2, + 60_000, + () => + `the app to connect (connections=${server.connectionCount()} open=${server.openClientCount()} methods=${server.requests.map((request) => request.method).join(',')} stderr=${stderr} fixture=${readFixtureLog(root)})` + ) + const connectionsWhileStored = server.connectionCount() + + // What `orca environment rm` does, from a process the app knows nothing about. + const cli = await runProcess({ + program: process.execPath, + args: [ + join(process.cwd(), 'out/cli/index.js'), + 'environment', + 'rm', + '--environment', + environment.id, + '--json' + ], + env: { ...process.env, ORCA_USER_DATA_PATH: userDataPath }, + timeoutMs: 30_000 + }) + expect(cli.code, cli.stderr || cli.stdout).toBe(0) + + await waitFor( + () => server.openClientCount() === 0, + REMOTE_RUNTIME_SOCKET_PING_INTERVAL_MS * 3, + () => `the removed environment to lose its socket (open=${server.openClientCount()})` + ) + await delay(REMOTE_RUNTIME_SOCKET_PING_INTERVAL_MS + 2_000) + expect(server.openClientCount()).toBe(0) + expect(server.connectionCount()).toBe(connectionsWhileStored) + console.info('CLI removal socket proof', { + before: connectionsWhileStored, + openAfter: server.openClientCount(), + totalAfter: server.connectionCount() + }) + }, 180_000) +}) diff --git a/src/main/ipc/runtime-environment-removal-watch.test.ts b/src/main/ipc/runtime-environment-removal-watch.test.ts new file mode 100644 index 000000000000..f621ab258bba --- /dev/null +++ b/src/main/ipc/runtime-environment-removal-watch.test.ts @@ -0,0 +1,188 @@ +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join, sep } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { encodePairingOffer } from '../../shared/pairing' +import { + addEnvironmentFromPairingCode, + getEnvironmentStorePath, + removeEnvironment +} from '../../shared/runtime-environment-store' +import { + isRuntimeEnvironmentRemoved, + noteRuntimeEnvironmentStored, + retireRemovedRuntimeEnvironment, + setRuntimeEnvironmentRemovalWatch +} from './runtime-environment-removal-watch' + +const tempDirs: string[] = [] + +function createStore(): { userDataPath: string; environmentId: string } { + const userDataPath = mkdtempSync(join(tmpdir(), 'orca-removal-watch-')) + tempDirs.push(userDataPath) + const environment = addEnvironmentFromPairingCode(userDataPath, { + name: 'desk', + pairingCode: encodePairingOffer({ + v: 2, + endpoint: 'ws://127.0.0.1:6768', + deviceToken: 'device-token', + publicKeyB64: Buffer.from(new Uint8Array(32).fill(1)).toString('base64') + }) + }) + return { userDataPath, environmentId: environment.id } +} + +afterEach(() => { + setRuntimeEnvironmentRemovalWatch(null) + for (const dir of tempDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }) + } +}) + +describe('runtime environment removal watch', () => { + it('reports a stored environment as present', () => { + const { userDataPath, environmentId } = createStore() + setRuntimeEnvironmentRemovalWatch({ getUserDataPath: () => userDataPath, retire: vi.fn() }) + + expect(isRuntimeEnvironmentRemoved(environmentId)).toBe(false) + }) + + it('never judges an environment the watched store has never listed', () => { + const { userDataPath } = createStore() + setRuntimeEnvironmentRemovalWatch({ getUserDataPath: () => userDataPath, retire: vi.fn() }) + + // Why: an id resolved from another user-data path is absent here for reasons unrelated to removal. + expect(isRuntimeEnvironmentRemoved('environment-from-another-path')).toBe(false) + }) + + it('reports an environment the CLI removed from the store', () => { + const { userDataPath, environmentId } = createStore() + setRuntimeEnvironmentRemovalWatch({ getUserDataPath: () => userDataPath, retire: vi.fn() }) + noteRuntimeEnvironmentStored(environmentId) + removeEnvironment(userDataPath, environmentId) + + expect(isRuntimeEnvironmentRemoved(environmentId)).toBe(true) + }) + + it('judges a removal seen only through repeated checks, with no resolve recorded', () => { + const { userDataPath, environmentId } = createStore() + setRuntimeEnvironmentRemovalWatch({ getUserDataPath: () => userDataPath, retire: vi.fn() }) + + // Why no note call: a liveness tick observing the id still listed is the only evidence a + // connection gets when its construction-time observation never landed. + expect(isRuntimeEnvironmentRemoved(environmentId)).toBe(false) + removeEnvironment(userDataPath, environmentId) + + expect(isRuntimeEnvironmentRemoved(environmentId)).toBe(true) + }) + + it('records a resolve that already succeeded, even once the store no longer lists it', () => { + const { userDataPath, environmentId } = createStore() + setRuntimeEnvironmentRemovalWatch({ getUserDataPath: () => userDataPath, retire: vi.fn() }) + // Why removed first: this is the race -- the caller resolved, the CLI removed, and only then + // does the observation get recorded. Re-reading the store here would find nothing. + removeEnvironment(userDataPath, environmentId) + noteRuntimeEnvironmentStored(environmentId, userDataPath) + + expect(isRuntimeEnvironmentRemoved(environmentId)).toBe(true) + }) + + it('ignores a resolve that came from a different user-data path', () => { + const { userDataPath, environmentId } = createStore() + const foreignUserDataPath = mkdtempSync(join(tmpdir(), 'orca-removal-watch-foreign-')) + tempDirs.push(foreignUserDataPath) + setRuntimeEnvironmentRemovalWatch({ getUserDataPath: () => userDataPath, retire: vi.fn() }) + removeEnvironment(userDataPath, environmentId) + noteRuntimeEnvironmentStored(environmentId, foreignUserDataPath) + + expect(isRuntimeEnvironmentRemoved(environmentId)).toBe(false) + }) + + it('accepts a watched path written with a trailing separator', () => { + const { userDataPath, environmentId } = createStore() + setRuntimeEnvironmentRemovalWatch({ getUserDataPath: () => userDataPath, retire: vi.fn() }) + removeEnvironment(userDataPath, environmentId) + noteRuntimeEnvironmentStored(environmentId, `${userDataPath}${sep}`) + + expect(isRuntimeEnvironmentRemoved(environmentId)).toBe(true) + }) + + it('forgets recorded environments when the watch is replaced', () => { + const { userDataPath, environmentId } = createStore() + setRuntimeEnvironmentRemovalWatch({ getUserDataPath: () => userDataPath, retire: vi.fn() }) + noteRuntimeEnvironmentStored(environmentId) + removeEnvironment(userDataPath, environmentId) + setRuntimeEnvironmentRemovalWatch({ getUserDataPath: () => userDataPath, retire: vi.fn() }) + + expect(isRuntimeEnvironmentRemoved(environmentId)).toBe(false) + }) + + it('treats a missing store as unknown rather than removed', () => { + const userDataPath = mkdtempSync(join(tmpdir(), 'orca-removal-watch-')) + tempDirs.push(userDataPath) + setRuntimeEnvironmentRemovalWatch({ getUserDataPath: () => userDataPath, retire: vi.fn() }) + + expect(isRuntimeEnvironmentRemoved('environment-a')).toBe(false) + }) + + it('treats an unreadable store as unknown rather than removed', () => { + const { userDataPath, environmentId } = createStore() + // Why: an unclean exit can leave the store NUL-filled (#18766); that must not retire anything. + writeFileSync(getEnvironmentStorePath(userDataPath), Buffer.alloc(64)) + setRuntimeEnvironmentRemovalWatch({ getUserDataPath: () => userDataPath, retire: vi.fn() }) + + expect(isRuntimeEnvironmentRemoved(environmentId)).toBe(false) + }) + + it('does not carry positive observations to a different user-data catalog', () => { + const first = createStore() + const second = createStore() + let userDataPath = first.userDataPath + setRuntimeEnvironmentRemovalWatch({ getUserDataPath: () => userDataPath, retire: vi.fn() }) + noteRuntimeEnvironmentStored(first.environmentId, userDataPath) + userDataPath = second.userDataPath + expect(isRuntimeEnvironmentRemoved(first.environmentId)).toBe(false) + }) + + it('keeps a previously observed host when its catalog becomes corrupt, unsupported or disappears', () => { + const { userDataPath, environmentId } = createStore() + setRuntimeEnvironmentRemovalWatch({ getUserDataPath: () => userDataPath, retire: vi.fn() }) + noteRuntimeEnvironmentStored(environmentId, userDataPath) + for (const content of ['broken json', JSON.stringify({ version: 2, environments: [] })]) { + writeFileSync(getEnvironmentStorePath(userDataPath), content) + expect(isRuntimeEnvironmentRemoved(environmentId)).toBe(false) + } + rmSync(getEnvironmentStorePath(userDataPath)) + expect(isRuntimeEnvironmentRemoved(environmentId)).toBe(false) + }) + + it('answers present and retires nothing before a watch is installed', () => { + expect(isRuntimeEnvironmentRemoved('environment-a')).toBe(false) + expect(() => retireRemovedRuntimeEnvironment('environment-a')).not.toThrow() + }) + + it('retires a removed environment through the injected teardown', () => { + const { userDataPath } = createStore() + const retire = vi.fn().mockResolvedValue(undefined) + setRuntimeEnvironmentRemovalWatch({ getUserDataPath: () => userDataPath, retire }) + + retireRemovedRuntimeEnvironment('environment-a') + + expect(retire).toHaveBeenCalledWith('environment-a') + }) + + it('survives a teardown that rejects', async () => { + const { userDataPath } = createStore() + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + setRuntimeEnvironmentRemovalWatch({ + getUserDataPath: () => userDataPath, + retire: () => Promise.reject(new Error('teardown failed')) + }) + + retireRemovedRuntimeEnvironment('environment-a') + await Promise.resolve() + + expect(warn).toHaveBeenCalled() + warn.mockRestore() + }) +}) diff --git a/src/main/ipc/runtime-environment-removal-watch.ts b/src/main/ipc/runtime-environment-removal-watch.ts new file mode 100644 index 000000000000..4c4d2b1a6577 --- /dev/null +++ b/src/main/ipc/runtime-environment-removal-watch.ts @@ -0,0 +1,103 @@ +import { resolve } from 'node:path' +import { listEnvironments } from '../../shared/runtime-environment-store' + +// Why: `orca environment rm` runs in a separate CLI process and only rewrites the store, so the +// app's cached transports are the only thing left describing an environment that no longer exists. +type RuntimeEnvironmentRemovalWatch = { + getUserDataPath: () => string + retire: (environmentId: string) => Promise +} + +let removalWatch: RuntimeEnvironmentRemovalWatch | null = null +const storedEnvironmentIds = new Set() +let storedEnvironmentPath: string | null = null + +export function setRuntimeEnvironmentRemovalWatch( + watch: RuntimeEnvironmentRemovalWatch | null +): void { + removalWatch = watch + storedEnvironmentPath = null + storedEnvironmentIds.clear() +} + +/** + * Records the watched store listing this environment, so a later absence becomes evidence. + * + * Why a caller's `userDataPath` beats reading again: the caller already resolved the environment + * out of that store, and a removal landing between its read and ours would lose the observation + * for good. The path still has to be the watched one, or a foreign store's id would be recorded. + */ +export function noteRuntimeEnvironmentStored(environmentId: string, userDataPath?: string): void { + const watch = removalWatch + if (userDataPath !== undefined) { + if (watch && resolve(watch.getUserDataPath()) === resolve(userDataPath)) { + scopeObservationsToPath(userDataPath) + storedEnvironmentIds.add(environmentId) + } + return + } + if (watchedStoreListsEnvironment(environmentId) === true) { + storedEnvironmentIds.add(environmentId) + } +} + +/** + * Why positive observation only: a missing store reads as zero environments rather than throwing, + * so anything less than "this store was read and the id is not in it" would retire every + * environment at once. An unknown store keeps the pre-fix answer, which is to stay connected. + */ +export function isRuntimeEnvironmentRemoved(environmentId: string): boolean { + const listed = watchedStoreListsEnvironment(environmentId) + if (listed === null) { + return false + } + if (listed) { + storedEnvironmentIds.add(environmentId) + return false + } + // Why: callers may resolve an environment from another user-data path (dev redirect, or the + // canonical path captured before app.setName). Absence from a store that never listed the id + // says nothing about it, so only an id this store once held can be judged removed. + return storedEnvironmentIds.has(environmentId) +} + +/** `null` when the store cannot be read, so the caller keeps the pre-fix answer. */ +function watchedStoreListsEnvironment(environmentId: string): boolean | null { + const watch = removalWatch + if (!watch) { + return null + } + try { + const userDataPath = watch.getUserDataPath() + scopeObservationsToPath(userDataPath) + return listEnvironments(userDataPath, { requireStoreFile: true }).some( + (entry) => entry.id === environmentId + ) + } catch { + return null + } +} + +export function retireRemovedRuntimeEnvironment(environmentId: string): void { + const watch = removalWatch + if (!watch) { + return + } + try { + void watch.retire(environmentId).catch(warnRetirementFailed) + } catch (error) { + warnRetirementFailed(error) + } +} + +function warnRetirementFailed(error: unknown): void { + console.warn('[runtime-environments] removed environment retirement failed:', error) +} + +function scopeObservationsToPath(userDataPath: string): void { + const path = resolve(userDataPath) + if (storedEnvironmentPath !== path) { + storedEnvironmentPath = path + storedEnvironmentIds.clear() + } +} diff --git a/src/main/ipc/runtime-environment-request-connections.test.ts b/src/main/ipc/runtime-environment-request-connections.test.ts index b02d1b06fb55..f03c871a0634 100644 --- a/src/main/ipc/runtime-environment-request-connections.test.ts +++ b/src/main/ipc/runtime-environment-request-connections.test.ts @@ -1,8 +1,17 @@ +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import { afterEach, describe, expect, it, vi } from 'vitest' +import { encodePairingOffer, type PairingOffer } from '../../shared/pairing' +import { + addEnvironmentFromPairingCode, + removeEnvironment +} from '../../shared/runtime-environment-store' import { closeSharedControlTestServers, createSharedControlTestServer } from '../../shared/remote-runtime-shared-control-test-server' +import { setRuntimeEnvironmentRemovalWatch } from './runtime-environment-removal-watch' import { applyRuntimeEnvironmentCapabilityVerdict, captureRuntimeEnvironmentCapabilityEvidence, @@ -23,11 +32,26 @@ import { } from './runtime-environment-request-connections' const ENVIRONMENT_ID = 'standing-intent-test' +const tempUserDataPaths: string[] = [] + +function storeEnvironment(pairing: PairingOffer): { userDataPath: string; environmentId: string } { + const userDataPath = mkdtempSync(join(tmpdir(), 'orca-request-connections-')) + tempUserDataPaths.push(userDataPath) + const environment = addEnvironmentFromPairingCode(userDataPath, { + name: 'desk', + pairingCode: encodePairingOffer(pairing) + }) + return { userDataPath, environmentId: environment.id } +} afterEach(async () => { closeRemoteRuntimeRequestConnection(ENVIRONMENT_ID) clearRuntimeEnvironmentManualDisconnect(ENVIRONMENT_ID) resetRuntimeEnvironmentCapabilityEvidence() + setRuntimeEnvironmentRemovalWatch(null) + for (const dir of tempUserDataPaths.splice(0)) { + rmSync(dir, { recursive: true, force: true }) + } await closeSharedControlTestServers() }) @@ -77,6 +101,28 @@ describe('runtime environment shared-control connection cache', () => { expect(getRemoteRuntimeSharedControlDiagnostics(ENVIRONMENT_ID)).toBeNull() }) + it('never retries a connection whose environment the CLI removed from the store', async () => { + const server = await createSharedControlTestServer() + const { userDataPath, environmentId } = storeEnvironment(server.pairing) + setRuntimeEnvironmentRemovalWatch({ + getUserDataPath: () => userDataPath, + retire: async (id) => closeRemoteRuntimeRequestConnection(id) + }) + ensureRemoteRuntimeSharedControlConnection(environmentId, server.pairing) + reconnectRemoteRuntimeSharedControlConnection(environmentId) + await waitFor(() => server.connectionCount() === 1) + + removeEnvironment(userDataPath, environmentId) + server.closeClients() + // Why the cache entry disappears rather than settling on 'closed': the close observes the + // removal and retires the transport, which is what leaves nothing behind to retry. + await waitFor(() => getRemoteRuntimeSharedControlDiagnostics(environmentId) === null) + await delay(400) + + expect(server.connectionCount()).toBe(1) + closeRemoteRuntimeRequestConnection(environmentId) + }) + it('lets a bypass request finish but never retries it after manual disconnect', async () => { const server = await createSharedControlTestServer() markRuntimeEnvironmentManuallyDisconnected(ENVIRONMENT_ID) diff --git a/src/main/ipc/runtime-environment-request-connections.ts b/src/main/ipc/runtime-environment-request-connections.ts index 32b7b348d0ba..adbeea23057b 100644 --- a/src/main/ipc/runtime-environment-request-connections.ts +++ b/src/main/ipc/runtime-environment-request-connections.ts @@ -17,6 +17,11 @@ import type { } from '../../shared/remote-runtime-shared-control-types' import { isRuntimeEnvironmentCapabilityPaused } from './runtime-environment-capability-evidence' import { isRuntimeEnvironmentManuallyDisconnected } from './runtime-environment-manual-disconnect' +import { + isRuntimeEnvironmentRemoved, + noteRuntimeEnvironmentStored, + retireRemovedRuntimeEnvironment +} from './runtime-environment-removal-watch' import { publishRuntimeEnvironmentDiagnostics } from './runtime-environment-diagnostics-broadcast' import { advanceRuntimeEnvironmentTransportGeneration, @@ -42,6 +47,9 @@ export function getRuntimeEnvironmentStatusOwner( selector: string ): RuntimeHostStatusOwner { const environment = resolveEnvironment(userDataPath, selector) + // Why here: this resolve is the store read that proves the environment exists, and re-reading + // later to learn the same thing loses the answer to a removal that lands in between. + noteRuntimeEnvironmentStored(environment.id, userDataPath) const pairing = getPreferredPairingOffer(environment) const key = `${userDataPath}\0${environment.pairingRevision ?? environment.createdAt}\0${getPairingKey(pairing)}` let cached = statusOwners.get(environment.id) @@ -208,6 +216,9 @@ function getSharedControlConnection( let cached = sharedControlConnections.get(environmentId) if (!cached || cached.pairingKey !== pairingKey) { advanceRuntimeEnvironmentTransportGeneration(environmentId) + // Why: the caller just resolved this environment, so record it before removal can race the + // first liveness tick -- otherwise its later absence would never count as evidence. + noteRuntimeEnvironmentStored(environmentId) cached?.connection.close() const transportGeneration = getRuntimeEnvironmentTransportGeneration(environmentId) cached = { @@ -217,6 +228,8 @@ function getSharedControlConnection( clientCapabilities: ELECTRON_REMOTE_RUNTIME_CLIENT_CAPABILITIES, isManuallyDisconnected: () => isRuntimeEnvironmentManuallyDisconnected(environmentId), isCapabilityPaused: () => isRuntimeEnvironmentCapabilityPaused(environmentId), + isEnvironmentRemoved: () => isRuntimeEnvironmentRemoved(environmentId), + onEnvironmentRemoved: () => retireRemovedRuntimeEnvironment(environmentId), onDiagnosticsChanged: (diagnostics) => { if (getRuntimeEnvironmentTransportGeneration(environmentId) !== transportGeneration) { return diff --git a/src/main/ipc/runtime-environments.ts b/src/main/ipc/runtime-environments.ts index 9e3506eb5fb7..c05e41d552d8 100644 --- a/src/main/ipc/runtime-environments.ts +++ b/src/main/ipc/runtime-environments.ts @@ -18,6 +18,7 @@ import { import { registerRuntimeEnvironmentRecoveryHandler } from './runtime-environment-recovery-handler' import { advanceRuntimeEnvironmentTransportGeneration } from './runtime-environment-transport-generation' import { resetSharedControlSupport } from './runtime-environment-transport-routing' +import { setRuntimeEnvironmentRemovalWatch } from './runtime-environment-removal-watch' import { RUNTIME_ENVIRONMENT_HANDLER_CHANNELS } from './runtime-environment-handler-channels' import { registerOrcadRuntimeLifecycleHandlers } from './orcad-runtime-lifecycle-handlers' import { registerOrcadRuntimeConversionHandlers } from './orcad-runtime-conversion-handlers' @@ -112,6 +113,10 @@ export function registerRuntimeEnvironmentHandlers(store: Store): void { }) registerRuntimeEnvironmentRecoveryHandler() registerRuntimeEnvironmentPassiveHandlers(getUserDataPath) + setRuntimeEnvironmentRemovalWatch({ + getUserDataPath, + retire: invalidateRuntimeEnvironmentTransport + }) for (const environment of listEnvironments(getUserDataPath())) { if (!isRuntimeEnvironmentManuallyDisconnected(environment.id)) { getRuntimeEnvironmentStatusOwner(getUserDataPath(), environment.id).activate() diff --git a/src/shared/remote-runtime-shared-control-connection.test.ts b/src/shared/remote-runtime-shared-control-connection.test.ts index bebf444e1f65..3b6c74ee1d23 100644 --- a/src/shared/remote-runtime-shared-control-connection.test.ts +++ b/src/shared/remote-runtime-shared-control-connection.test.ts @@ -671,6 +671,30 @@ describe('RemoteRuntimeSharedControlConnection', () => { }) }) + it('retires a live socket on the liveness cadence once its environment is removed', async () => { + const server = await createServer() + let environmentRemoved = false + let connection: RemoteRuntimeSharedControlConnection | null = null + const onEnvironmentRemoved = vi.fn(() => connection?.close()) + connection = new RemoteRuntimeSharedControlConnection(server.pairing, { + liveness: { pingIntervalMs: 20, livenessTimeoutMs: 10_000 }, + isEnvironmentRemoved: () => environmentRemoved, + onEnvironmentRemoved + }) + + await connection.request('worktree.ps', undefined, 1000) + expect(server.connectionCount()).toBe(1) + + environmentRemoved = true + await vi.waitFor(() => expect(onEnvironmentRemoved).toHaveBeenCalled()) + + expect(connection.getDiagnostics()).toMatchObject({ state: 'closed' }) + await new Promise((resolve) => setTimeout(resolve, 100)) + expect(server.connectionCount()).toBe(1) + // Why: closing the socket must stop its liveness timer, or retirement repeats forever. + expect(onEnvironmentRemoved).toHaveBeenCalledTimes(1) + }) + it('rejects pending requests and schedules standing recovery when the socket closes', async () => { const server = await createServer({ closeBeforeResponse: true }) const connection = new RemoteRuntimeSharedControlConnection(server.pairing) @@ -687,6 +711,137 @@ describe('RemoteRuntimeSharedControlConnection', () => { expect(connection.getDiagnostics()).toMatchObject({ state: 'closed' }) connection.close() }) + + it('declines a request-driven reopen once its environment is removed', async () => { + const server = await createServer() + let environmentRemoved = false + const connection = new RemoteRuntimeSharedControlConnection(server.pairing, { + isEnvironmentRemoved: () => environmentRemoved + }) + + await connection.request('worktree.ps', undefined, 1000) + expect(server.connectionCount()).toBe(1) + + // Removal landing while the socket is down leaves the cached connection inert: the scheduler + // declines, so no timer is armed and this is the state a later request finds. + environmentRemoved = true + server.closeClients() + await vi.waitFor(() => + expect(connection.getDiagnostics()).toMatchObject({ + state: 'closed', + lastClose: { code: 4001, reason: 'test close' } + }) + ) + + await expect(connection.request('worktree.ps', undefined, 1000)).rejects.toThrow( + 'Remote Orca runtime closed the connection' + ) + // Why the server-side count and not just the rejection: the defect was a *new socket* dialled + // to a removed environment, which a rejected caller would not otherwise reveal. + expect(server.connectionCount()).toBe(1) + + connection.close() + }) + + it('retires a removed environment when a request finds the socket closed', async () => { + const server = await createServer() + let environmentRemoved = false + let connection: RemoteRuntimeSharedControlConnection | null = null + const onEnvironmentRemoved = vi.fn(() => connection?.close()) + connection = new RemoteRuntimeSharedControlConnection(server.pairing, { + isEnvironmentRemoved: () => environmentRemoved, + onEnvironmentRemoved + }) + + await connection.request('worktree.ps', undefined, 1000) + + // Why the socket closes while the environment is still stored and the retry is then paused: + // that leaves no live socket, no armed timer and no close-time observation of the removal, so + // the next request is the only thing that can retire it. + server.closeClients() + await vi.waitFor(() => + expect(connection?.getDiagnostics()).toMatchObject({ state: 'reconnecting' }) + ) + connection.pauseStandingRetry() + environmentRemoved = true + expect(onEnvironmentRemoved).not.toHaveBeenCalled() + + await expect(connection.request('worktree.ps', undefined, 1000)).rejects.toThrow( + 'Remote Orca runtime closed the connection' + ) + expect(onEnvironmentRemoved).toHaveBeenCalledTimes(1) + + // Why a second request: retirement closes the connection, so later callers finding the same + // retired transport must not re-fire it. + await expect(connection.request('worktree.ps', undefined, 1000)).rejects.toThrow() + expect(onEnvironmentRemoved).toHaveBeenCalledTimes(1) + }) + + it('closes a live removed socket once even when the retirement callback does not close it', async () => { + const server = await createServer() + let removed = false + const onEnvironmentRemoved = vi.fn() + const connection = new RemoteRuntimeSharedControlConnection(server.pairing, { + isEnvironmentRemoved: () => removed, + onEnvironmentRemoved, + liveness: { pingIntervalMs: 20, livenessTimeoutMs: 200 } + }) + try { + await connection.request('worktree.ps', undefined, 1000) + removed = true + await vi.waitFor(() => expect(server.openClientCount()).toBe(0)) + expect(onEnvironmentRemoved).toHaveBeenCalledTimes(1) + await expect(connection.request('worktree.ps', undefined, 1000)).rejects.toThrow() + expect(onEnvironmentRemoved).toHaveBeenCalledTimes(1) + expect(server.connectionCount()).toBe(1) + } finally { + connection.close() + } + }) + + it('retires a removed environment when its socket closes', async () => { + const server = await createServer() + let environmentRemoved = false + let connection: RemoteRuntimeSharedControlConnection | null = null + const onEnvironmentRemoved = vi.fn(() => connection?.close()) + connection = new RemoteRuntimeSharedControlConnection(server.pairing, { + isEnvironmentRemoved: () => environmentRemoved, + onEnvironmentRemoved + }) + + await connection.request('worktree.ps', undefined, 1000) + + // Why this ordering: the removal is already known when the socket drops, so no reconnect is + // armed and the closed socket has no liveness tick left to retire it. + environmentRemoved = true + server.closeClients() + + await vi.waitFor(() => expect(onEnvironmentRemoved).toHaveBeenCalledTimes(1), { timeout: 5000 }) + expect(server.connectionCount()).toBe(1) + }) + + it('retires a removed environment when its reconnect timer fires', async () => { + const server = await createServer() + let environmentRemoved = false + let connection: RemoteRuntimeSharedControlConnection | null = null + const onEnvironmentRemoved = vi.fn(() => connection?.close()) + connection = new RemoteRuntimeSharedControlConnection(server.pairing, { + isEnvironmentRemoved: () => environmentRemoved, + onEnvironmentRemoved, + // Why flip from here: the removal has to land after the scheduler armed its timer and before + // it fires, the one window no request passes through. + onDiagnosticsChanged: ({ state }) => { + environmentRemoved ||= state === 'reconnecting' + } + }) + + await connection.request('worktree.ps', undefined, 1000) + server.closeClients() + + await vi.waitFor(() => expect(onEnvironmentRemoved).toHaveBeenCalledTimes(1), { timeout: 5000 }) + // Why the server-side count: retiring must not cost a dial to an environment that is gone. + expect(server.connectionCount()).toBe(1) + }) }) it('cancels a pending subscription setup without canceling a sibling on the shared connection', async () => { diff --git a/src/shared/remote-runtime-shared-control-connection.ts b/src/shared/remote-runtime-shared-control-connection.ts index 3356be044b35..d7c3b6b9abdf 100644 --- a/src/shared/remote-runtime-shared-control-connection.ts +++ b/src/shared/remote-runtime-shared-control-connection.ts @@ -1,3 +1,4 @@ +import { sharedControlDiagnosticsInputs } from './remote-runtime-shared-control-diagnostics-inputs' import type WebSocket from 'ws' import type { PairingOffer } from './pairing' import type { RemoteRuntimeClientError } from './remote-runtime-client-error' @@ -10,7 +11,7 @@ import { SharedControlReconnectScheduler } from './remote-runtime-shared-control import { requestSharedControl } from './remote-runtime-shared-control-requests' import { SharedControlRetiredRequestIds } from './remote-runtime-shared-control-retired-request-ids' import { SharedControlReadyStableResetTimer } from './remote-runtime-shared-control-stability' -import * as sharedControlState from './remote-runtime-shared-control-state' +import { rejectSharedControlReadyWaiters } from './remote-runtime-shared-control-state' import { closeSharedControlSocket } from './remote-runtime-shared-control-socket-close' import { startSharedControlSubscription } from './remote-runtime-shared-control-subscription-start' import { SharedControlSocketGeneration } from './remote-runtime-shared-control-socket-generation' @@ -118,24 +119,16 @@ export class RemoteRuntimeSharedControlConnection { } } - private publishDiagnostics(): void { - this.diagnostics.publish({ - state: this.state, - reconnecting: this.reconnect.isScheduled, - pendingRequestCount: this.pendingRequests.size, - subscriptionCount: this.subscriptions.size, - reconnectAttempt: this.reconnect.attemptCount - }) - } - getDiagnostics(): SharedControlTypes.RemoteRuntimeSharedConnectionDiagnostics { - return this.diagnostics.get({ - state: this.state, - reconnecting: this.reconnect.isScheduled, - pendingRequestCount: this.pendingRequests.size, - subscriptionCount: this.subscriptions.size, - reconnectAttempt: this.reconnect.attemptCount - }) - } + private diagnosticsInputs = () => + sharedControlDiagnosticsInputs( + this.state, + this.reconnect, + this.pendingRequests, + this.subscriptions + ) + private publishDiagnostics = (): void => this.diagnostics.publish(this.diagnosticsInputs()) + getDiagnostics = (): SharedControlTypes.RemoteRuntimeSharedConnectionDiagnostics => + this.diagnostics.get(this.diagnosticsInputs()) reconnectNow(): void { refreshRemoteRuntimeSharedControl({ intentionallyClosed: this.intentionallyClosed, @@ -165,12 +158,23 @@ export class RemoteRuntimeSharedControlConnection { }) } + private retireRemovedEnvironment(): void { + if (!this.intentionallyClosed) { + this.close(remoteRuntimeUnavailableError()) + this.options.onEnvironmentRemoved?.() + } + } + private open(): void { - if (this.intentionallyClosed) { - sharedControlState.rejectSharedControlReadyWaiters( - this.readyWaiters, - remoteRuntimeUnavailableError() - ) + // Why removal is re-asked here, not only in the scheduler: a request reaching a closed socket + // opens one directly through `ensureReadyWithTimeout`, bypassing the backoff gate entirely. + if (this.intentionallyClosed || this.options.isEnvironmentRemoved?.()) { + rejectSharedControlReadyWaiters(this.readyWaiters, remoteRuntimeUnavailableError()) + // Why retire from here: declining the dial leaves no liveness monitor, the only other caller + // of this callback, so the cached transport would outlive the environment it describes. + if (!this.intentionallyClosed) { + this.retireRemovedEnvironment() + } return } this.reconnect.clear() @@ -187,7 +191,9 @@ export class RemoteRuntimeSharedControlConnection { onTextFrame: (frame) => this.handleTextFrame(frame, socketGeneration), liveness: { options: this.options.liveness, - onDead: (error) => this.handleSocketClosed(error, socketGeneration) + onDead: (error) => this.handleSocketClosed(error, socketGeneration), + isRetired: () => this.options.isEnvironmentRemoved?.() ?? false, + onRetired: () => this.retireRemovedEnvironment() } }) if (!opened.ok) { @@ -280,6 +286,7 @@ export class RemoteRuntimeSharedControlConnection { this.reconnect.scheduleAfterSocketClose({ intentionallyClosed: this.intentionallyClosed, manuallyDisconnected: this.options.isManuallyDisconnected?.() ?? false, + environmentRemoved: () => this.options.isEnvironmentRemoved?.() ?? false, capabilityPaused: this.options.isCapabilityPaused?.() ?? false, subscriptionCount: this.subscriptions.size, open: () => this.open() diff --git a/src/shared/remote-runtime-shared-control-diagnostics-inputs.ts b/src/shared/remote-runtime-shared-control-diagnostics-inputs.ts new file mode 100644 index 000000000000..c32deb6d6bfb --- /dev/null +++ b/src/shared/remote-runtime-shared-control-diagnostics-inputs.ts @@ -0,0 +1,18 @@ +import type { SharedControlConnectionState } from './remote-runtime-shared-control-types' +import type { SharedControlDiagnosticsTracker } from './remote-runtime-shared-control-diagnostics' +import type { SharedControlReconnectScheduler } from './remote-runtime-shared-control-reconnect' + +export function sharedControlDiagnosticsInputs( + state: SharedControlConnectionState, + reconnect: SharedControlReconnectScheduler, + pendingRequests: ReadonlyMap, + subscriptions: ReadonlyMap +): Parameters[0] { + return { + state, + reconnecting: reconnect.isScheduled, + pendingRequestCount: pendingRequests.size, + subscriptionCount: subscriptions.size, + reconnectAttempt: reconnect.attemptCount + } +} diff --git a/src/shared/remote-runtime-shared-control-open.ts b/src/shared/remote-runtime-shared-control-open.ts index ddb3eb0cd789..9650ecd8ef6d 100644 --- a/src/shared/remote-runtime-shared-control-open.ts +++ b/src/shared/remote-runtime-shared-control-open.ts @@ -22,9 +22,14 @@ export function openSharedControlSocket( // Why: reconnect is edge-triggered on `close`, but a half-open tunnel // never delivers one. Liveness pings and declares the socket dead when // the server goes silent, so the close/reconnect path can run (#7718). + // Why: the liveness cadence is the only level-triggered hook an open socket has, so a + // transport whose environment was removed out-of-process is retired on the same tick + // instead of surviving until the next disconnect (#20995). liveness?: { options?: RemoteRuntimeSocketLivenessOptions onDead: (error: RemoteRuntimeClientError) => void + isRetired?: () => boolean + onRetired?: () => void } } ): { ok: true; socket: RemoteRuntimeWebSocket } | { ok: false; error: RemoteRuntimeClientError } { @@ -68,6 +73,10 @@ export function openSharedControlSocket( const liveness = callbacks.liveness const monitor = startRemoteRuntimeSocketLiveness({ ping: () => { + if (liveness.isRetired?.() === true) { + liveness.onRetired?.() + return + } if (ws.readyState === 1) { ws.ping() } diff --git a/src/shared/remote-runtime-shared-control-reconnect.test.ts b/src/shared/remote-runtime-shared-control-reconnect.test.ts index bb816ef86b19..ae7710dc4d87 100644 --- a/src/shared/remote-runtime-shared-control-reconnect.test.ts +++ b/src/shared/remote-runtime-shared-control-reconnect.test.ts @@ -20,6 +20,71 @@ describe('SharedControlReconnectScheduler', () => { expect(open).toHaveBeenCalledTimes(1) }) + it('stops reconnecting once the environment is gone from the store', () => { + vi.useFakeTimers() + const scheduler = new SharedControlReconnectScheduler() + const open = vi.fn() + + scheduler.scheduleAfterSocketClose({ + intentionallyClosed: false, + manuallyDisconnected: false, + environmentRemoved: () => true, + capabilityPaused: false, + subscriptionCount: 1, + open + }) + + expect(scheduler.isScheduled).toBe(false) + // Why once rather than never: the close is handed to open(), whose guard retires the transport + // instead of dialling. Nothing is armed, so the backoff adds no second call. + expect(open).toHaveBeenCalledTimes(1) + vi.advanceTimersByTime(300_000) + expect(open).toHaveBeenCalledTimes(1) + }) + + it('keeps reconnecting a subscribed socket while the environment is still stored', () => { + vi.useFakeTimers() + const scheduler = new SharedControlReconnectScheduler() + const open = vi.fn() + + scheduler.scheduleAfterSocketClose({ + intentionallyClosed: false, + manuallyDisconnected: false, + environmentRemoved: () => false, + capabilityPaused: false, + subscriptionCount: 1, + open + }) + + expect(scheduler.isScheduled).toBe(true) + vi.advanceTimersByTime(300_000) + expect(open).toHaveBeenCalledTimes(1) + }) + + it('fires a pending reconnect after a mid-backoff removal so open() can retire it', () => { + vi.useFakeTimers() + const scheduler = new SharedControlReconnectScheduler() + const open = vi.fn() + let environmentRemoved = false + + scheduler.scheduleAfterSocketClose({ + intentionallyClosed: false, + manuallyDisconnected: false, + environmentRemoved: () => environmentRemoved, + capabilityPaused: false, + subscriptionCount: 1, + open + }) + expect(scheduler.isScheduled).toBe(true) + + // Why mid-wait: the verdict taken before the backoff said "still stored". The fire is handed + // to open(), which re-asks and retires there; declining here would strand the transport. + environmentRemoved = true + vi.advanceTimersByTime(300_000) + + expect(open).toHaveBeenCalledTimes(1) + }) + it('does not advance cleared or intentionally closed work', () => { vi.useFakeTimers() const scheduler = new SharedControlReconnectScheduler() diff --git a/src/shared/remote-runtime-shared-control-reconnect.ts b/src/shared/remote-runtime-shared-control-reconnect.ts index 6704db6cdcdf..66914a2af4b9 100644 --- a/src/shared/remote-runtime-shared-control-reconnect.ts +++ b/src/shared/remote-runtime-shared-control-reconnect.ts @@ -55,6 +55,7 @@ export class SharedControlReconnectScheduler { scheduleAfterSocketClose(args: { intentionallyClosed: boolean manuallyDisconnected: boolean + environmentRemoved: () => boolean capabilityPaused: boolean subscriptionCount: number open: () => void @@ -66,6 +67,14 @@ export class SharedControlReconnectScheduler { ) { return } + // Why open() instead of a bare return: its guard declines the dial *and* retires, and a socket + // closing after the removal has no liveness tick left to do that. + if (args.environmentRemoved()) { + args.open() + return + } + // Why the fire is not gated on removal either: open() re-asks the same predicate on every entry, + // so short-circuiting the timer would strand the transport it must evict. if (args.subscriptionCount > 0) { this.scheduleWithDefaultBackoff(args.intentionallyClosed, args.open) return diff --git a/src/shared/remote-runtime-shared-control-test-server.ts b/src/shared/remote-runtime-shared-control-test-server.ts index 701ea0e58935..3c813a52dd04 100644 --- a/src/shared/remote-runtime-shared-control-test-server.ts +++ b/src/shared/remote-runtime-shared-control-test-server.ts @@ -15,6 +15,9 @@ export type SharedControlTestServer = { requests: { id: string; method: string; params?: unknown }[] auths: unknown[] connectionCount: () => number + /** Sockets still open right now, unlike the cumulative `connectionCount`. */ + openClientCount: () => number + endpointPort: () => number flushDelayedResponses: () => void closeClients: () => void } @@ -144,6 +147,8 @@ export async function createSharedControlTestServer( requests, auths, connectionCount: () => connectionCount, + openClientCount: () => wss.clients.size, + endpointPort: () => address.port, flushDelayedResponses: () => delayedResponses.splice(0).forEach((send) => send()), closeClients: () => wss.clients.forEach((client) => client.close(4001, 'test close')) } diff --git a/src/shared/remote-runtime-shared-control-types.ts b/src/shared/remote-runtime-shared-control-types.ts index 39d1b01f6133..c59b41750c29 100644 --- a/src/shared/remote-runtime-shared-control-types.ts +++ b/src/shared/remote-runtime-shared-control-types.ts @@ -77,6 +77,9 @@ export type RemoteRuntimeSharedControlConnectionOptions = { clientCapabilities?: readonly RuntimeCapability[] isManuallyDisconnected?: () => boolean isCapabilityPaused?: () => boolean + isEnvironmentRemoved?: () => boolean + /** Runs once a live socket observes its environment is gone; the owner retires the transport. */ + onEnvironmentRemoved?: () => void /** Publishes local transport diagnostics after a meaningful state transition. */ onDiagnosticsChanged?: (diagnostics: RemoteRuntimeSharedConnectionDiagnostics) => void reconnectStableResetMs?: number From 55055ada8e9e7583dcdcb0a90fe9362579327463 Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 6 Oct 2026 15:42:54 -0700 Subject: [PATCH 2/6] Make CLI removal regression independent of prebuilt artifacts --- config/scripts/vitest-node-runtime-files.mjs | 1 + ...e-environment-cli-removal.electron.test.ts | 27 ++++++++++++++----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/config/scripts/vitest-node-runtime-files.mjs b/config/scripts/vitest-node-runtime-files.mjs index 791e38f209f7..0cacccc638c5 100644 --- a/config/scripts/vitest-node-runtime-files.mjs +++ b/config/scripts/vitest-node-runtime-files.mjs @@ -2,6 +2,7 @@ import { SQLITE_RUNTIME_INCLUDE } from './vitest-sqlite-runtime-files.mjs' // SQLite publication, native bindings, worker IPC, socket liveness, and V8 retention use Node. export const NODE_RUNTIME_INCLUDE = [ + 'src/main/ipc/runtime-environment-cli-removal.electron.test.ts', 'config/scripts/vitest-sqlite-runtime-boundary.test.ts', 'config/scripts/cloud-security-history-workflow.test.mjs', 'config/scripts/skill-recipe-shell.test.mjs', diff --git a/src/main/ipc/runtime-environment-cli-removal.electron.test.ts b/src/main/ipc/runtime-environment-cli-removal.electron.test.ts index 2363e2c33b30..08a747651fef 100644 --- a/src/main/ipc/runtime-environment-cli-removal.electron.test.ts +++ b/src/main/ipc/runtime-environment-cli-removal.electron.test.ts @@ -14,7 +14,7 @@ import { } from 'node:fs' import { builtinModules, createRequire } from 'node:module' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { isAbsolute, join } from 'node:path' import { afterAll, describe, expect, it } from 'vitest' import { build as buildVite } from 'vite' import { resolveElectronProbeLaunch } from '../browser/electron-probe-display-launch' @@ -27,11 +27,7 @@ import { createSharedControlTestServer } from '../../shared/remote-runtime-shared-control-test-server' -// Why a real Electron process: the registration in `runtime-environments.ts` that hands the removal -// watch its user-data path and its teardown only runs inside the shipped main process, and the -// retirement rides the socket's own liveness timer. A vitest-only harness can reach neither, so -// #20995 -- the CLI removes an environment and the app keeps the socket -- could only be reproduced -// here. The store rewrite is issued from this process, which is what `orca environment rm` is. +// The real CLI and a hidden Electron process exercise registration and socket retirement together. const electronBinary = resolveElectronBinary() const roots: string[] = [] @@ -141,6 +137,23 @@ describe('runtime environment removed by the CLI', () => { } }) + await buildVite({ + configFile: false, + logLevel: 'silent', + resolve: { conditions: ['node'] }, + build: { + emptyOutDir: false, + lib: { + entry: join(process.cwd(), 'src/cli/index.ts'), + formats: ['cjs'], + fileName: () => 'orca-cli.cjs' + }, + outDir: fixtureDir, + target: 'node20', + rollupOptions: { external: (id) => !isAbsolute(id) && !id.startsWith('.') } + } + }) + writeFileSync( join(fixtureDir, 'package.json'), '{ "name": "orca-cli-removal-fixture", "main": "main.js" }' @@ -187,7 +200,7 @@ describe('runtime environment removed by the CLI', () => { const cli = await runProcess({ program: process.execPath, args: [ - join(process.cwd(), 'out/cli/index.js'), + join(fixtureDir, 'orca-cli.cjs'), 'environment', 'rm', '--environment', From 65a5338fb8770ff0270237b0ab22a662347ec157 Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 6 Oct 2026 15:50:49 -0700 Subject: [PATCH 3/6] test: route new orchestration SQLite fixtures through Node --- config/scripts/vitest-sqlite-database-runtime-files.mjs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/scripts/vitest-sqlite-database-runtime-files.mjs b/config/scripts/vitest-sqlite-database-runtime-files.mjs index 9098a6f566db..cec9f43adefd 100644 --- a/config/scripts/vitest-sqlite-database-runtime-files.mjs +++ b/config/scripts/vitest-sqlite-database-runtime-files.mjs @@ -1,5 +1,7 @@ // Database opening and lifecycle fixtures exercise Node SQLite behavior. export const SQLITE_DATABASE_RUNTIME_INCLUDE = [ + 'src/main/runtime/orchestration/orchestration-party-location.test.ts', + 'src/main/runtime/runtime-orchestration-sender-names.test.ts', 'src/main/ai-vault-search/session-search-file-write.test.ts', 'src/main/ai-vault-search/session-search-live-transcript.test.ts', 'src/main/ai-vault/session-scanner-devin-contention.test.ts', From b5f8eb3948abcdc5d6fb88f246c1217de08256ba Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 6 Oct 2026 17:44:44 -0700 Subject: [PATCH 4/6] Use upstream SQLite runtime registrations without duplicates --- config/scripts/vitest-sqlite-database-runtime-files.mjs | 2 -- 1 file changed, 2 deletions(-) diff --git a/config/scripts/vitest-sqlite-database-runtime-files.mjs b/config/scripts/vitest-sqlite-database-runtime-files.mjs index cec9f43adefd..9098a6f566db 100644 --- a/config/scripts/vitest-sqlite-database-runtime-files.mjs +++ b/config/scripts/vitest-sqlite-database-runtime-files.mjs @@ -1,7 +1,5 @@ // Database opening and lifecycle fixtures exercise Node SQLite behavior. export const SQLITE_DATABASE_RUNTIME_INCLUDE = [ - 'src/main/runtime/orchestration/orchestration-party-location.test.ts', - 'src/main/runtime/runtime-orchestration-sender-names.test.ts', 'src/main/ai-vault-search/session-search-file-write.test.ts', 'src/main/ai-vault-search/session-search-live-transcript.test.ts', 'src/main/ai-vault/session-scanner-devin-contention.test.ts', From d82ac49bd7f3e4eee07ec9a5ee285c23b1021de4 Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 6 Oct 2026 18:20:38 -0700 Subject: [PATCH 5/6] Run the upstream agent launch database fixture in Node --- config/scripts/vitest-sqlite-database-runtime-files.mjs | 1 + 1 file changed, 1 insertion(+) diff --git a/config/scripts/vitest-sqlite-database-runtime-files.mjs b/config/scripts/vitest-sqlite-database-runtime-files.mjs index 9098a6f566db..e837c26efb27 100644 --- a/config/scripts/vitest-sqlite-database-runtime-files.mjs +++ b/config/scripts/vitest-sqlite-database-runtime-files.mjs @@ -34,6 +34,7 @@ export const SQLITE_DATABASE_RUNTIME_INCLUDE = [ 'src/main/runtime/claude-structured-session-integration.test.ts', 'src/main/runtime/claude-structured-startup-fault-is-not-an-exit.test.ts', 'src/main/runtime/exit-provenance-audit.test.ts', + 'src/main/runtime/rpc/methods/agent-launch-instant-tab.test.ts', 'src/main/runtime/orchestration-cli-subprocess.test.ts', 'src/main/runtime/orchestration-dispatch-mailbox-delivery.test.ts', 'src/main/runtime/orchestration-mailbox-crash-recovery.test.ts', From 1371f3eda0ad2ed11412bd05ca4ccc23eaaefb91 Mon Sep 17 00:00:00 2001 From: Neil Date: Wed, 7 Oct 2026 13:16:22 -0700 Subject: [PATCH 6/6] Keep the CLI removal fixture compatible with managed SSH registration --- src/main/ipc/runtime-environment-cli-removal.electron.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/ipc/runtime-environment-cli-removal.electron.test.ts b/src/main/ipc/runtime-environment-cli-removal.electron.test.ts index 08a747651fef..b64320e8b48b 100644 --- a/src/main/ipc/runtime-environment-cli-removal.electron.test.ts +++ b/src/main/ipc/runtime-environment-cli-removal.electron.test.ts @@ -64,7 +64,7 @@ function buildFixtureMain(bundlePath: string, logPath: string, userDataPath: str `runtime.applyBackgroundActivationPolicy()`, `app.whenReady().then(() => {`, ` log('ready ' + app.getPath('userData'))`, - ` runtime.registerRuntimeEnvironmentHandlers({ getSettings: () => ({}) })`, + ` runtime.registerRuntimeEnvironmentHandlers({ getSettings: () => ({}), getWorkspaceSessionHostIds: () => [] })`, ` log('registered')`, `}).catch((error) => log('ready-failed ' + (error && error.stack)))` ].join('\n') @@ -133,7 +133,7 @@ describe('runtime environment removed by the CLI', () => { lib: { entry: entryPath, formats: ['cjs'], fileName: () => 'runtime-environments.cjs' }, outDir: fixtureDir, target: 'node20', - rollupOptions: { external: ['electron', /^node:/, ...builtinModules] } + rollupOptions: { external: ['electron', 'ssh2', /^node:/, ...builtinModules] } } })