From ef33bb754ab766ef529164a60df5a2bbbcba2bd6 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 26 Jul 2026 15:10:11 -0700 Subject: [PATCH] test(startup): prove the lock-loss guard against a real Linux reproduction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the STA-1513 guard: the mechanism is now reproduced rather than inferred, and the fix is verified against it. Electron's app.quit() before the ready event terminates the process on macOS but is deferred on Linux. So a lock-losing launch reaches app.whenReady() on Linux only, runs the full startup path, and republishes orca-runtime.json with its own pid before exiting. A minimal two-process Electron probe reproduces it exactly, and an A/B against the same build confirms the guard: without guard: final record = LOSER pid=119 (owner alive at pid 25) with guard: final record = OWNER pid=183 ("GUARD: bailing out") That matches the reported artifact — a runtime record naming a nonexistent pid while the live app is bound to a different socket — and explains why the reporter who captured it was on Linux. Changes: - Correct the guard comment: name Linux's deferred quit as the reason, and note the branch is a no-op on macOS, which quits before ready. - Assert the guard body actually returns, not merely that it is positioned first; ordering alone would still pass if the body were emptied. - Assert in the headless-serve E2E that the on-disk runtime record still names the owner after the lock-losing launch exits. status.get answers from the owner's live server, so it cannot observe a clobbered file and would have stayed green through this bug. That suite runs on ubuntu-latest, which is where the clobber is reachable. --- src/main/index.ts | 5 +++-- .../startup/desktop-startup-ordering.test.ts | 4 ++++ .../headless-serve-desktop-activation.spec.ts | 19 +++++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/main/index.ts b/src/main/index.ts index 0bbc207b913..95bb57f23ee 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -1777,8 +1777,9 @@ function shouldSuppressCodexAutoApprovalSyntheticTitleFromHook(args: { } app.whenReady().then(async () => { - // Why: app.quit() above is async, so a lock-losing launch still reaches ready; returning keeps it from - // republishing orca-runtime.json / agent-hooks endpoint files over the live instance's (STA-1513). + // Why: Linux defers the app.quit() above until after ready, so a lock-losing launch still runs this + // handler and republishes orca-runtime.json / agent-hooks endpoints over the live instance's, leaving + // the CLI pointed at a dead pid. macOS quits before ready, so this is a no-op there. (STA-1513) if (!hasSingleInstanceLock) { return } diff --git a/src/main/startup/desktop-startup-ordering.test.ts b/src/main/startup/desktop-startup-ordering.test.ts index 26b4b9e8239..a6330bf91cc 100644 --- a/src/main/startup/desktop-startup-ordering.test.ts +++ b/src/main/startup/desktop-startup-ordering.test.ts @@ -87,11 +87,15 @@ describe('startup ordering', () => { const source = readFileSync(join(process.cwd(), 'src/main/index.ts'), 'utf8') const readyStart = source.indexOf('app.whenReady().then(async () => {') const guard = source.indexOf('if (!hasSingleInstanceLock) {', readyStart) + const guardEnd = source.indexOf('}', guard) const rpcStart = source.indexOf('runtimeRpc.start()', readyStart) const startupServices = source.indexOf('startTerminalRuntimeStartupServices()', readyStart) expect(readyStart).toBeGreaterThanOrEqual(0) expect(guard).toBeGreaterThan(readyStart) + // Why: ordering alone would still pass if the body were emptied to a no-op. + expect(guardEnd).toBeGreaterThan(guard) + expect(source.slice(guard, guardEnd)).toContain('return') expect(rpcStart).toBeGreaterThan(guard) expect(startupServices).toBeGreaterThan(guard) // Why: the bail must precede every side effect in the handler, not merely the RPC publish. diff --git a/tests/e2e/headless-serve-desktop-activation.spec.ts b/tests/e2e/headless-serve-desktop-activation.spec.ts index 77b85929d70..0a59024db3b 100644 --- a/tests/e2e/headless-serve-desktop-activation.spec.ts +++ b/tests/e2e/headless-serve-desktop-activation.spec.ts @@ -29,6 +29,7 @@ import type { RuntimeTerminalRead } from '../../src/shared/runtime-types' import { PROTOCOL_VERSION } from '../../src/main/daemon/types' +import { getRuntimeMetadataPath, type RuntimeMetadata } from '../../src/shared/runtime-bootstrap' const electronPackageDir = path.join(process.cwd(), 'node_modules', 'electron') const electronPath = path.join( @@ -67,6 +68,18 @@ function readDaemonPid(userDataDir: string): number { return parsed.pid } +// Why: status.get answers from the owner's live server, so only the file on disk can show whether a +// lock-losing launch republished its own pid over the owner's. Guards Linux CI, where the quit is +// deferred past ready and the clobber is reachable; macOS quits earlier, so it is a no-op. (STA-1513) +function readRuntimeMetadataFromDisk(userDataDir: string): RuntimeMetadata { + const metadataPath = getRuntimeMetadataPath(userDataDir) + const parsed = JSON.parse(readFileSync(metadataPath, 'utf8')) as Partial + if (typeof parsed.pid !== 'number' || typeof parsed.runtimeId !== 'string') { + throw new Error(`Runtime metadata lacked a pid/runtimeId: ${metadataPath}`) + } + return parsed as RuntimeMetadata +} + async function waitForProcessExit(child: ChildProcess, timeoutMs: number): Promise { if (child.exitCode !== null || child.signalCode !== null) { return true @@ -127,6 +140,7 @@ test('promotes the headless owner without replacing its daemon terminal', async const beforeStatus = await client.call('status.get') const daemonPidBefore = readDaemonPid(userDataDir) + const runtimeMetadataBefore = readRuntimeMetadataFromDisk(userDataDir) await client.call('repo.add', { path: repoPath, kind: 'git' }) const created = await client.call<{ terminal: RuntimeTerminalCreate }>('terminal.create', { worktree: `path:${repoPath}`, @@ -194,6 +208,11 @@ test('promotes the headless owner without replacing its daemon terminal', async expect(promotedPtyId).toBe(terminal.ptyId) expect(readDaemonPid(userDataDir)).toBe(daemonPidBefore) expect(await waitForProcessExit(activatingProcess, 10_000)).toBe(true) + // Why: the loser publishes during its own startup, so only a post-exit read proves the + // owner's discovery record survived the whole second launch. + const runtimeMetadataAfter = readRuntimeMetadataFromDisk(userDataDir) + expect(runtimeMetadataAfter.pid).toBe(runtimeMetadataBefore.pid) + expect(runtimeMetadataAfter.runtimeId).toBe(runtimeMetadataBefore.runtimeId) await waitForTerminalOutput(page, beforeMarker, 30_000) const afterMarker = `SERVE_PROMOTION_AFTER_${Date.now()}`