From 02e61ba56068a00d6934ef7747adabdb24348783 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 28 Aug 2026 07:43:30 -0700 Subject: [PATCH] test(daemon): pin the utility-fork arms the descriptor-hygiene premise rests on A gate ablation found three arms in the utility-process fork path that could each be reverted alone with CI green, including the PR's own premise: - `stdio: 'ignore'` on the shim fork. Changing it to `'pipe'` hands the shim the Chromium descriptors this hop exists to remove, and with nobody draining the pipe it also blocks exit. - the shim-path bundle-root-vs-`out/main` `existsSync` ternary. - the named throw when no fork port is installed (otherwise a bare TypeError). Seven neighbouring arms are pinned in the same pass: the fatal-error cause suffix, the post-release `daemon-error` suppression, the `daemonExited` conjunct on shim exit, the `disconnect()` post-message catch, the installed-port fallback, the shim's exit-relay linger, and its `child.connected` release guard. Every arm above goes red with that arm reverted alone and stays green with all the others reverted. --- .../daemon-utility-launcher-shim.test.ts | 23 +++- .../daemon-utility-process-fork.test.ts | 127 ++++++++++++++++-- 2 files changed, 138 insertions(+), 12 deletions(-) diff --git a/src/main/daemon/daemon-utility-launcher-shim.test.ts b/src/main/daemon/daemon-utility-launcher-shim.test.ts index ab6cb255450..43415e52a2c 100644 --- a/src/main/daemon/daemon-utility-launcher-shim.test.ts +++ b/src/main/daemon/daemon-utility-launcher-shim.test.ts @@ -172,12 +172,33 @@ describe('daemon-utility-launcher-shim', () => { it('relays the daemon exit code from a real child', async () => { const port = createFakePort() - runDaemonUtilityLauncherShim(port, undefined, () => {}) + const exit = vi.fn() + runDaemonUtilityLauncherShim(port, undefined, exit) port.deliver({ kind: 'spawn', spec: specFor(EXITING_FIXTURE) }) const spawned = await port.waitFor('spawned') spawnedPids.push(spawned.pid) const exited = await port.waitFor('daemon-exit') expect(exited).toEqual({ kind: 'daemon-exit', code: 7, signal: null }) + // Exiting inside the relay would race process.exit against delivery of the + // exit code the launcher reports as the startup-failure cause. + expect(exit).not.toHaveBeenCalled() + }) + + it('releases a shim whose daemon already exited without posting a phantom daemon error', async () => { + const port = createFakePort() + const exit = vi.fn() + runDaemonUtilityLauncherShim(port, undefined, exit) + port.deliver({ kind: 'spawn', spec: specFor(EXITING_FIXTURE) }) + + const spawned = await port.waitFor('spawned') + spawnedPids.push(spawned.pid) + await port.waitFor('daemon-exit') + + // Release still arrives here: the parent kills this shim on the exit relay, + // but its own cleanup path calls disconnect() and kills are not instant. + port.deliver({ kind: 'release' }) + expect(exit).toHaveBeenCalledWith(0) + expect(port.posted.filter((message) => message.kind === 'daemon-error')).toEqual([]) }) }) diff --git a/src/main/daemon/daemon-utility-process-fork.test.ts b/src/main/daemon/daemon-utility-process-fork.test.ts index 289a9c582f0..bb6a638acc4 100644 --- a/src/main/daemon/daemon-utility-process-fork.test.ts +++ b/src/main/daemon/daemon-utility-process-fork.test.ts @@ -1,4 +1,7 @@ import { EventEmitter } from 'node:events' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { setAppEnvironment, type AppEnvironment } from '../../shared/app-environment' import { @@ -34,8 +37,11 @@ const SPEC: UtilityDaemonForkSpec = { let shim: FakeShim let forkedPaths: string[] -const forkFn: UtilityProcessForkFn = (modulePath) => { +let forkOptions: (Record | undefined)[] + +const forkFn: UtilityProcessForkFn = (modulePath, _args, options) => { forkedPaths.push(modulePath) + forkOptions.push(options) return shim } @@ -47,11 +53,9 @@ async function forkSettledChild() { return await promise } -beforeEach(() => { - shim = new FakeShim() - forkedPaths = [] +function setFakeAppEnvironment(appPath: string): void { setAppEnvironment({ - getAppPath: () => '/fake/app', + getAppPath: () => appPath, getPath: () => '/fake/userData', getVersion: () => '1.2.3', isPackaged: () => false, @@ -59,6 +63,20 @@ beforeEach(() => { exit: () => {}, getAppMetrics: () => [] } as unknown as AppEnvironment) +} + +/** Runs a body against a throwaway bundle root so shim-path resolution is real. */ +function withBundleRoot(body: (appPath: string) => Promise): Promise { + const appPath = mkdtempSync(join(tmpdir(), 'orca-shim-path-')) + setFakeAppEnvironment(appPath) + return body(appPath).finally(() => rmSync(appPath, { recursive: true, force: true })) +} + +beforeEach(() => { + shim = new FakeShim() + forkedPaths = [] + forkOptions = [] + setFakeAppEnvironment('/fake/app') }) afterEach(() => { @@ -186,16 +204,65 @@ describe('forkDaemonThroughUtilityProcess', () => { child.on('error', (error) => errors.push(error)) shim.emit('exit', 1) expect(errors).toHaveLength(1) - expect(errors[0].message).toContain('before the daemon settled') + // Exact: with no fatal error recorded the message must not trail a cause. + expect(errors[0].message).toBe('Daemon utility launcher exited before the daemon settled') + }) + + it('carries a post-launch fatal shim error into the child error as the cause', async () => { + const child = await forkSettledChild() + const errors: Error[] = [] + child.on('error', (error) => errors.push(error)) + // Electron emits 'error' then 'exit'; failing the settled launch again would + // drop the cause, leaving only "exited before the daemon settled" to triage. + shim.emit('error', 'FatalError', 'v8::internal::Heap', '{}') + shim.emit('exit', 1) + expect(errors).toHaveLength(1) + expect(errors[0].message).toBe( + 'Daemon utility launcher exited before the daemon settled: Daemon utility launcher hit a fatal error: FatalError at v8::internal::Heap' + ) }) it('suppresses late daemon-error relays after release: no listener remains to catch them', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + try { + const child = await forkSettledChild() + child.disconnect() + // Would be an uncaught exception if emitted with no 'error' listener. + expect(() => + shim.emit('message', { kind: 'daemon-error', message: 'late failure' }) + ).not.toThrow() + // The launch already succeeded and the daemon is detached; degrading this + // to the no-listener warn would log a launch failure that did not happen. + expect(warnSpy).not.toHaveBeenCalled() + } finally { + warnSpy.mockRestore() + } + }) + + it('treats the shim exit that follows a relayed daemon exit as shutdown, not a launch failure', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + try { + await forkSettledChild() + shim.emit('message', { kind: 'daemon-exit', code: 1, signal: null }) + expect(shim.killed).toBe(true) + // Electron emits 'exit' after that kill(), by which point the launcher has + // already dropped its startup listeners and reported the real exit code. + shim.emit('exit', 0) + expect(warnSpy).not.toHaveBeenCalled() + } finally { + warnSpy.mockRestore() + } + }) + + it('releases a shim that is already gone without throwing out of disconnect', async () => { const child = await forkSettledChild() - child.disconnect() - // Would be an uncaught exception if emitted with no 'error' listener. - expect(() => - shim.emit('message', { kind: 'daemon-error', message: 'late failure' }) - ).not.toThrow() + // Reachable: a daemon that dies during startup makes the exit relay kill the + // shim, and the launcher's cleanup path still calls disconnect() afterwards. + shim.postMessage = () => { + throw new Error('Utility process is not running') + } + expect(() => child.disconnect()).not.toThrow() + expect(child.connected).toBe(false) }) // Pre-guard, every leg below re-raised as [main_uncaught_exception]: the relay @@ -246,6 +313,44 @@ describe('forkDaemonThroughUtilityProcess', () => { }) }) + it('forks through the port the desktop installed when called with a spec alone', async () => { + // The only shape production uses: daemon-init passes no fork argument. + setDaemonUtilityProcessFork(forkFn) + const promise = forkDaemonThroughUtilityProcess(SPEC) + shim.emit('message', { kind: 'shim-ready' }) + shim.emit('message', { kind: 'spawned', pid: 777 }) + await expect(promise).resolves.toMatchObject({ pid: 777 }) + expect(forkedPaths).toHaveLength(1) + }) + + it('gives the shim no inheritable stdio and a named service entry', async () => { + await forkSettledChild() + // 'ignore': nobody drains the shim's output, and an unread pipe from main + // both blocks exit and hands the shim descriptors this hop exists to avoid. + expect(forkOptions[0]).toEqual({ stdio: 'ignore', serviceName: 'orca-daemon-launcher' }) + }) + + it('rejects by name when no port is installed rather than throwing a bare TypeError', async () => { + await expect(forkDaemonThroughUtilityProcess(SPEC)).rejects.toThrow( + 'No utility-process fork is installed on this host' + ) + }) + + it('resolves the shim under out/main, the layout every host taking the hop ships', async () => { + await withBundleRoot(async (appPath) => { + await forkSettledChild() + expect(forkedPaths[0]).toBe(join(appPath, 'out', 'main', 'daemon-utility-launcher-shim.js')) + }) + }) + + it('prefers a shim sitting directly in the bundle root when one is there', async () => { + await withBundleRoot(async (appPath) => { + writeFileSync(join(appPath, 'daemon-utility-launcher-shim.js'), '') + await forkSettledChild() + expect(forkedPaths[0]).toBe(join(appPath, 'daemon-utility-launcher-shim.js')) + }) + }) + it('disconnect releases the shim instead of killing the daemon', async () => { const child = await forkSettledChild() const errors: Error[] = []