test(startup): prove the lock-loss guard against a real Linux reproduction

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.
This commit is contained in:
Brennan Benson
2026-07-26 15:10:11 -07:00
parent abc8a6e529
commit ef33bb754a
3 changed files with 26 additions and 2 deletions
+3 -2
View File
@@ -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
}
@@ -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.
@@ -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<RuntimeMetadata>
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<boolean> {
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<RuntimeStatus>('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()}`