From c75c04eaae3eae7eb61dec4041dd00ca3c091398 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 27 Jul 2026 17:32:55 -0700 Subject: [PATCH] fix(runtime): reclaim orca-runtime.json when it stops describing this runtime (#10840) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(runtime): reclaim orca-runtime.json when it stops describing this runtime On macOS the Chromium single-instance lock is silently defeated whenever `SingletonSocket`/`SingletonCookie` go missing from the profile — and the socket they point at lives under `$TMPDIR` (`/var/folders/.../T`), which macOS purges after 3 days (`com.apple.bsd.dirhelper`, CLEAN_FILES_OLDER_THAN_DAYS=3). A launch that slips past the lock runs a full startup, republishes `orca-runtime.json` with its own pid, and leaves the CLI on a dead pid once it exits: `orca status` reports `stale_bootstrap` and every terminal command fails `runtime_unavailable` while the original app keeps serving. The owner now watches its own discovery record and republishes once no live runtime is described. Reclaiming only a dead pid is deliberate: two live runtimes sharing a profile would otherwise fight over the file. Reproduced on macOS with two real Orca main processes on one profile: the second instance took the lock and clobbered the record, and killing it left `stale_bootstrap` against the still-healthy first instance. With this change the owner reclaimed the record in ~2s and the CLI returned to `ready`. Refs #7848 * test(runtime): assert stop() clears the metadata ownership timer The republish guard alone kept the shutdown test green, so the watch teardown was unasserted. Also drop the doc claim of startup/activation callers that do not exist. * test(runtime): stand in a real live pid for the sibling-runtime case Windows never assigns pid 1, so the hardcoded sibling read as dead there and the watch would reclaim the record. Own a synthetic pid instead and let process.pid play the live sibling. --- .../runtime-metadata-ownership-watch.test.ts | 218 ++++++++++++++++++ .../runtime-metadata-ownership-watch.ts | 114 +++++++++ src/main/runtime/runtime-rpc.test.ts | 77 ++++++- src/main/runtime/runtime-rpc.ts | 39 +++- 4 files changed, 446 insertions(+), 2 deletions(-) create mode 100644 src/main/runtime/runtime-metadata-ownership-watch.test.ts create mode 100644 src/main/runtime/runtime-metadata-ownership-watch.ts diff --git a/src/main/runtime/runtime-metadata-ownership-watch.test.ts b/src/main/runtime/runtime-metadata-ownership-watch.test.ts new file mode 100644 index 000000000..d8eb41965 --- /dev/null +++ b/src/main/runtime/runtime-metadata-ownership-watch.test.ts @@ -0,0 +1,218 @@ +import { mkdtempSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { getRuntimeMetadataPath, type RuntimeMetadata } from '../../shared/runtime-bootstrap' +import { clearRuntimeMetadata, readRuntimeMetadata, writeRuntimeMetadata } from './runtime-metadata' +import { + shouldReclaimRuntimeMetadata, + watchRuntimeMetadataOwnership, + type RuntimeMetadataOwnershipWatch +} from './runtime-metadata-ownership-watch' + +const OWNED_PID = 4242 +const OWNED_RUNTIME_ID = 'rt_owner' +const FOREIGN_LIVE_PID = 5151 +const FOREIGN_DEAD_PID = 5252 + +function record(overrides: Partial = {}): RuntimeMetadata { + return { + runtimeId: OWNED_RUNTIME_ID, + pid: OWNED_PID, + transports: [{ kind: 'unix', endpoint: '/tmp/orca-owner.sock' }], + authToken: 'secret', + startedAt: 100, + ...overrides + } +} + +const isProcessRunning = (pid: number): boolean => pid === OWNED_PID || pid === FOREIGN_LIVE_PID + +describe('shouldReclaimRuntimeMetadata', () => { + it('leaves the record alone while it still describes this runtime', () => { + expect( + shouldReclaimRuntimeMetadata(record(), OWNED_PID, OWNED_RUNTIME_ID, isProcessRunning) + ).toBe(false) + }) + + it('reclaims a missing record', () => { + expect(shouldReclaimRuntimeMetadata(null, OWNED_PID, OWNED_RUNTIME_ID, isProcessRunning)).toBe( + true + ) + }) + + it('reclaims a record left behind by a dead runtime', () => { + expect( + shouldReclaimRuntimeMetadata( + record({ pid: FOREIGN_DEAD_PID, runtimeId: 'rt_second_instance' }), + OWNED_PID, + OWNED_RUNTIME_ID, + isProcessRunning + ) + ).toBe(true) + }) + + it('yields to another live runtime so two instances cannot ping-pong the record', () => { + expect( + shouldReclaimRuntimeMetadata( + record({ pid: FOREIGN_LIVE_PID, runtimeId: 'rt_second_instance' }), + OWNED_PID, + OWNED_RUNTIME_ID, + isProcessRunning + ) + ).toBe(false) + }) + + it('reclaims a foreign runtimeId stamped on this pid', () => { + // Why: only this process can be this pid, so the record is a recycled-pid leftover. + expect( + shouldReclaimRuntimeMetadata( + record({ runtimeId: 'rt_previous_process' }), + OWNED_PID, + OWNED_RUNTIME_ID, + isProcessRunning + ) + ).toBe(true) + }) +}) + +describe('watchRuntimeMetadataOwnership', () => { + const watches: RuntimeMetadataOwnershipWatch[] = [] + const userDataPaths: string[] = [] + + afterEach(() => { + for (const watch of watches.splice(0)) { + watch.stop() + } + for (const dir of userDataPaths.splice(0)) { + clearRuntimeMetadata(dir) + } + vi.useRealTimers() + }) + + function armWatch(userDataPath: string, pollIntervalMs = 10): RuntimeMetadataOwnershipWatch { + const watch = watchRuntimeMetadataOwnership({ + userDataPath, + ownedPid: OWNED_PID, + ownedRuntimeId: OWNED_RUNTIME_ID, + pollIntervalMs, + isProcessRunning, + republish: () => writeRuntimeMetadata(userDataPath, record()) + }) + watches.push(watch) + return watch + } + + function makeUserDataPath(): string { + const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-ownership-')) + userDataPaths.push(userDataPath) + return userDataPath + } + + it('republishes after a second instance clobbers the record and exits', () => { + const userDataPath = makeUserDataPath() + writeRuntimeMetadata(userDataPath, record()) + const watch = armWatch(userDataPath) + + writeRuntimeMetadata( + userDataPath, + record({ pid: FOREIGN_DEAD_PID, runtimeId: 'rt_second_instance' }) + ) + watch.check() + + expect(readRuntimeMetadata(userDataPath)).toMatchObject({ + pid: OWNED_PID, + runtimeId: OWNED_RUNTIME_ID + }) + }) + + it('republishes a record that was deleted underneath the runtime', () => { + const userDataPath = makeUserDataPath() + writeRuntimeMetadata(userDataPath, record()) + const watch = armWatch(userDataPath) + + clearRuntimeMetadata(userDataPath) + watch.check() + + expect(readRuntimeMetadata(userDataPath)).toMatchObject({ pid: OWNED_PID }) + }) + + it('replaces an unreadable record', () => { + const userDataPath = makeUserDataPath() + const watch = armWatch(userDataPath) + writeFileSync(getRuntimeMetadataPath(userDataPath), '{ truncated') + + watch.check() + + expect(readRuntimeMetadata(userDataPath)).toMatchObject({ pid: OWNED_PID }) + }) + + it('leaves a live sibling runtime in place', () => { + const userDataPath = makeUserDataPath() + const watch = armWatch(userDataPath) + writeRuntimeMetadata( + userDataPath, + record({ pid: FOREIGN_LIVE_PID, runtimeId: 'rt_second_instance' }) + ) + + watch.check() + + expect(readRuntimeMetadata(userDataPath)).toMatchObject({ pid: FOREIGN_LIVE_PID }) + }) + + it('reclaims on the poll interval without an explicit check', () => { + vi.useFakeTimers() + const userDataPath = makeUserDataPath() + armWatch(userDataPath, 1_000) + writeRuntimeMetadata( + userDataPath, + record({ pid: FOREIGN_DEAD_PID, runtimeId: 'rt_second_instance' }) + ) + + vi.advanceTimersByTime(1_000) + + expect(readRuntimeMetadata(userDataPath)).toMatchObject({ pid: OWNED_PID }) + }) + + it('stops reclaiming once the watch is stopped', () => { + vi.useFakeTimers() + const userDataPath = makeUserDataPath() + const watch = armWatch(userDataPath, 1_000) + + watch.stop() + writeRuntimeMetadata( + userDataPath, + record({ pid: FOREIGN_DEAD_PID, runtimeId: 'rt_second_instance' }) + ) + vi.advanceTimersByTime(5_000) + + expect(readRuntimeMetadata(userDataPath)).toMatchObject({ pid: FOREIGN_DEAD_PID }) + }) + + it('keeps polling after a republish failure', () => { + vi.useFakeTimers() + const userDataPath = makeUserDataPath() + const republish = vi + .fn() + .mockImplementationOnce(() => { + throw new Error('disk full') + }) + .mockImplementation(() => writeRuntimeMetadata(userDataPath, record())) + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + const watch = watchRuntimeMetadataOwnership({ + userDataPath, + ownedPid: OWNED_PID, + ownedRuntimeId: OWNED_RUNTIME_ID, + pollIntervalMs: 1_000, + isProcessRunning, + republish + }) + watches.push(watch) + + vi.advanceTimersByTime(2_000) + + expect(republish).toHaveBeenCalledTimes(2) + expect(readRuntimeMetadata(userDataPath)).toMatchObject({ pid: OWNED_PID }) + consoleError.mockRestore() + }) +}) diff --git a/src/main/runtime/runtime-metadata-ownership-watch.ts b/src/main/runtime/runtime-metadata-ownership-watch.ts new file mode 100644 index 000000000..ccdb8bf8f --- /dev/null +++ b/src/main/runtime/runtime-metadata-ownership-watch.ts @@ -0,0 +1,114 @@ +import { getRuntimeMetadataPath, type RuntimeMetadata } from '../../shared/runtime-bootstrap' +import { readRuntimeMetadata } from './runtime-metadata' + +/** + * Why: `orca-runtime.json` is the CLI's only pointer at a live runtime, and it + * can stop describing this process while this process is still serving RPC — + * a second instance that slipped past the single-instance lock publishes its + * own pid and then exits, leaving the CLI on a dead pid (`stale_bootstrap`) + * against a healthy app (#7848). Chromium's lock is defeated on macOS whenever + * `SingletonSocket`/`SingletonCookie` go missing, and its socket lives under + * `$TMPDIR` (`/var/folders/.../T`), which macOS itself purges after 3 days. + * + * The owner therefore watches its own record and reclaims it once no live + * runtime is described. Reclaiming only a dead pid is deliberate: two live + * runtimes sharing a profile would otherwise ping-pong the file forever. + */ +export const RUNTIME_METADATA_OWNERSHIP_POLL_MS = 10_000 + +export type RuntimeMetadataOwnershipWatch = { + /** Runs one ownership check immediately; exposed for tests and eager repair. */ + check: () => void + stop: () => void +} + +export type RuntimeMetadataOwnershipWatchOptions = { + userDataPath: string + ownedPid: number + ownedRuntimeId: string + republish: () => void + pollIntervalMs?: number + isProcessRunning?: (pid: number) => boolean + onReclaim?: (previous: RuntimeMetadata | null) => void +} + +export function shouldReclaimRuntimeMetadata( + current: RuntimeMetadata | null, + ownedPid: number, + ownedRuntimeId: string, + isProcessRunning: (pid: number) => boolean +): boolean { + if (!current) { + return true + } + if (current.pid === ownedPid && current.runtimeId === ownedRuntimeId) { + return false + } + // Why: only this process can legitimately claim this pid, so a foreign + // runtimeId on it is a leftover from a recycled pid, not a live sibling. + if (current.pid === ownedPid) { + return true + } + return !isProcessRunning(current.pid) +} + +export function watchRuntimeMetadataOwnership( + options: RuntimeMetadataOwnershipWatchOptions +): RuntimeMetadataOwnershipWatch { + const isProcessRunning = options.isProcessRunning ?? isPidRunning + const check = (): void => { + const current = tryReadRuntimeMetadata(options.userDataPath) + if ( + !shouldReclaimRuntimeMetadata( + current, + options.ownedPid, + options.ownedRuntimeId, + isProcessRunning + ) + ) { + return + } + try { + options.republish() + } catch (error) { + // Why: a transient write failure must not kill the watch; the next tick retries. + console.error('[runtime] Failed to reclaim runtime metadata ownership:', error) + return + } + options.onReclaim?.(current) + } + + const timer = setInterval(check, options.pollIntervalMs ?? RUNTIME_METADATA_OWNERSHIP_POLL_MS) + // Why: discovery bookkeeping must never be the reason the process stays alive. + timer.unref?.() + return { + check, + stop: () => clearInterval(timer) + } +} + +function tryReadRuntimeMetadata(userDataPath: string): RuntimeMetadata | null { + try { + return readRuntimeMetadata(userDataPath) + } catch (error) { + // Why: an unparseable record is as useless to the CLI as a missing one, so treat it as reclaimable. + console.warn( + `[runtime] Ignoring unreadable ${getRuntimeMetadataPath(userDataPath)}:`, + error instanceof Error ? error.message : String(error) + ) + return null + } +} + +function isPidRunning(pid: number): boolean { + if (!pid || pid <= 0) { + return false + } + try { + process.kill(pid, 0) + return true + } catch (error) { + // Why: only ESRCH proves the pid is gone; EPERM means a foreign owner holds it (same rule as the socket sweep). + return (error as NodeJS.ErrnoException).code !== 'ESRCH' + } +} diff --git a/src/main/runtime/runtime-rpc.test.ts b/src/main/runtime/runtime-rpc.test.ts index 2ed908bba..87d984b6f 100644 --- a/src/main/runtime/runtime-rpc.test.ts +++ b/src/main/runtime/runtime-rpc.test.ts @@ -11,7 +11,7 @@ import Database from '../sqlite/sync-database' import { OrcaRuntimeService } from './orca-runtime' import { OrchestrationDb } from './orchestration/db' import * as runtimeMetadataModule from './runtime-metadata' -import { readRuntimeMetadata } from './runtime-metadata' +import { readRuntimeMetadata, writeRuntimeMetadata } from './runtime-metadata' import { createRuntimeTransportMetadata, OrcaRuntimeRpcServer } from './runtime-rpc' import { parsePairingCode } from '../../shared/pairing' import { subscribeRemoteRuntimeRequest } from '../../shared/remote-runtime-client' @@ -362,6 +362,81 @@ describe('OrcaRuntimeRpcServer', () => { }) }) + it('reclaims runtime metadata clobbered by a second instance that has since died', async () => { + // Why: #7848 — a launch that slips past the single-instance lock republishes + // orca-runtime.json with its own pid, so the CLI reports stale_bootstrap + // against this still-serving runtime once that instance exits. + const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-rpc-')) + const runtime = new OrcaRuntimeService() + const server = new OrcaRuntimeRpcServer({ runtime, userDataPath }) + await server.start() + const published = readRuntimeMetadata(userDataPath) + + writeRuntimeMetadata(userDataPath, { + runtimeId: 'rt_second_instance', + pid: 99999999, + transports: [{ kind: 'unix', endpoint: join(userDataPath, 'o-99999999-rt2.sock') }], + authToken: 'second-instance-token', + startedAt: 1 + }) + server.checkRuntimeMetadataOwnership() + + expect(readRuntimeMetadata(userDataPath)).toEqual(published) + + await server.stop() + }) + + it('leaves runtime metadata owned by a live sibling runtime untouched', async () => { + const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-rpc-')) + // Why: a synthetic owned pid frees the always-alive process.pid to stand in for + // the sibling — Windows never assigns pid 1, so hardcoding it there reads as dead. + const server = new OrcaRuntimeRpcServer({ + runtime: new OrcaRuntimeService(), + userDataPath, + pid: 4242 + }) + await server.start() + + writeRuntimeMetadata(userDataPath, { + runtimeId: 'rt_live_sibling', + pid: process.pid, + transports: [{ kind: 'unix', endpoint: join(userDataPath, `o-${process.pid}-rt2.sock`) }], + authToken: 'sibling-token', + startedAt: 1 + }) + server.checkRuntimeMetadataOwnership() + + expect(readRuntimeMetadata(userDataPath)).toMatchObject({ runtimeId: 'rt_live_sibling' }) + + await server.stop() + }) + + it('stops reclaiming runtime metadata after the server is stopped', async () => { + const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-rpc-')) + const server = new OrcaRuntimeRpcServer({ runtime: new OrcaRuntimeService(), userDataPath }) + await server.start() + const watch = server['metadataOwnershipWatch'] + if (!watch) { + throw new Error('start() must arm the metadata ownership watch') + } + // Why: the republish guard alone would keep this test green, so assert the timer teardown itself. + const watchStop = vi.spyOn(watch, 'stop') + await server.stop() + + writeRuntimeMetadata(userDataPath, { + runtimeId: 'rt_second_instance', + pid: 99999999, + transports: [], + authToken: 'second-instance-token', + startedAt: 1 + }) + server.checkRuntimeMetadataOwnership() + + expect(watchStop).toHaveBeenCalledTimes(1) + expect(server['metadataOwnershipWatch']).toBeNull() + expect(readRuntimeMetadata(userDataPath)).toMatchObject({ runtimeId: 'rt_second_instance' }) + }) + it('creates a pairing offer for the active WebSocket transport', async () => { const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-rpc-')) const runtime = new OrcaRuntimeService() diff --git a/src/main/runtime/runtime-rpc.ts b/src/main/runtime/runtime-rpc.ts index a4ac64704..54734e693 100644 --- a/src/main/runtime/runtime-rpc.ts +++ b/src/main/runtime/runtime-rpc.ts @@ -6,6 +6,11 @@ import { join } from 'node:path' import type { RuntimeMetadata, RuntimeTransportMetadata } from '../../shared/runtime-bootstrap' import type { OrcaRuntimeService } from './orca-runtime' import { writeRuntimeMetadata } from './runtime-metadata' +import { + RUNTIME_METADATA_OWNERSHIP_POLL_MS, + watchRuntimeMetadataOwnership, + type RuntimeMetadataOwnershipWatch +} from './runtime-metadata-ownership-watch' import { RpcDispatcher } from './rpc/dispatcher' import type { RpcRequest, RpcResponse } from './rpc/core' import { errorResponse } from './rpc/errors' @@ -57,6 +62,8 @@ type OrcaRuntimeRpcServerOptions = { // Why: test-only overrides for the two constants below; production must not pass these (defaults set by §3.1). keepaliveIntervalMs?: number longPollCap?: number + // Why: test-only override for the ownership reclaim cadence. + metadataOwnershipPollMs?: number } export type PairingOfferUnavailableReason = @@ -450,6 +457,7 @@ export class OrcaRuntimeRpcServer { private readonly authToken = randomBytes(24).toString('hex') private readonly keepaliveIntervalMs: number private readonly longPollCap: number + private readonly metadataOwnershipPollMs: number private readonly askLongPollCap: number private readonly relayRevokeOutbox: RelayRevokeOutbox private deviceRegistry: DeviceRegistry | null = null @@ -458,6 +466,7 @@ export class OrcaRuntimeRpcServer { private tlsFingerprint: string | null = null private activeTransports: RpcTransport[] = [] private transports: RuntimeTransportMetadata[] = [] + private metadataOwnershipWatch: RuntimeMetadataOwnershipWatch | null = null private mobileSocketWiring: MobileSocketWiring | null = null private mobileRelayPairingProvider: MobileRelayPairingProvider | null = null private onUnpairedDeviceAuthFailure: (() => void) | null = null @@ -485,7 +494,8 @@ export class OrcaRuntimeRpcServer { preferPinnedWsPort = false, webClientRoot, keepaliveIntervalMs = KEEPALIVE_INTERVAL_MS, - longPollCap = LONG_POLL_CAP + longPollCap = LONG_POLL_CAP, + metadataOwnershipPollMs = RUNTIME_METADATA_OWNERSHIP_POLL_MS }: OrcaRuntimeRpcServerOptions) { this.runtime = runtime this.dispatcher = new RpcDispatcher({ runtime }) @@ -498,6 +508,7 @@ export class OrcaRuntimeRpcServer { this.webClientRoot = webClientRoot this.keepaliveIntervalMs = keepaliveIntervalMs this.longPollCap = longPollCap + this.metadataOwnershipPollMs = metadataOwnershipPollMs // Why: derived, not configurable — the reservation must hold for whatever cap a caller picks. this.askLongPollCap = Math.max(1, Math.floor(longPollCap * ASK_LONG_POLL_SHARE)) this.relayRevokeOutbox = new RelayRevokeOutbox(userDataPath) @@ -1003,12 +1014,38 @@ export class OrcaRuntimeRpcServer { await Promise.all(activeTransports.map((t) => t.stop().catch(() => {}))).catch(() => {}) throw error } + + this.metadataOwnershipWatch = watchRuntimeMetadataOwnership({ + userDataPath: this.userDataPath, + ownedPid: this.pid, + ownedRuntimeId: this.runtime.getRuntimeId(), + pollIntervalMs: this.metadataOwnershipPollMs, + republish: () => { + // Why: never advertise endpoints we already tore down. + if (this.activeTransports.length === 0) { + return + } + this.writeMetadata() + }, + onReclaim: (previous) => { + console.warn( + `[runtime] Reclaimed orca-runtime.json from a dead runtime (pid ${previous?.pid ?? 'none'}); republished pid ${this.pid}.` + ) + } + }) + } + + /** Why: test-only seam — runs one ownership check instead of waiting out the poll interval. */ + checkRuntimeMetadataOwnership(): void { + this.metadataOwnershipWatch?.check() } async stop(): Promise { const transports = this.activeTransports this.activeTransports = [] this.transports = [] + this.metadataOwnershipWatch?.stop() + this.metadataOwnershipWatch = null this.mobileSocketWiring = null if (transports.length === 0) { return