From 76d480f8081e0ef5f6529db1506bc989f42e6cef Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 6 Oct 2026 21:42:02 -0700 Subject: [PATCH] Fix unsafe test fixtures and the Bun version pin (#26051) * Keep test interruption signals within owned processes * Pin Bun and add optional unit runner shutdown diagnostics * Unblock CI lint without changing session host runtime * Avoid duplicating runtime import-check dependency bundles * Leave unit runner diagnostics disabled by default * test: keep runner incident follow-up focused on durable guards * test: apply transcript replacements as authoritative snapshots --- .github/CONTRIBUTING.md | 2 +- .github/workflows/pr.yml | 2 +- .github/workflows/unit-tests.yml | 2 +- config/{bun-version => .bun-version} | 0 ...ck-runtime-electron-ratchet-graph.test.mjs | 114 ++++++++++++++++ config/scripts/run-vitest.mjs | 2 +- ...ime-auth-drain-interruption-source.test.ts | 82 ++++++++++++ ...-runtime-auth-drain-interruption-source.ts | 25 ++++ ...y-wsl-runtime-auth-drain-script-harness.ts | 17 +-- ...me-auth-drain-script-interference-shims.ts | 4 +- ...ranscript-watch-inplace-drain-race.test.ts | 126 +++++++++++------- .../provider-process-teardown.test.ts | 51 +++---- 12 files changed, 341 insertions(+), 86 deletions(-) rename config/{bun-version => .bun-version} (100%) create mode 100644 config/scripts/check-runtime-electron-ratchet-graph.test.mjs create mode 100644 src/main/codex-accounts/legacy-wsl-runtime-auth-drain-interruption-source.test.ts create mode 100644 src/main/codex-accounts/legacy-wsl-runtime-auth-drain-interruption-source.ts diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 7fc5200404b..43339372bcb 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -16,7 +16,7 @@ Thanks for contributing to Orca. ## Local Setup -Install Node 24, pnpm, and the Bun version in [`config/bun-version`](../config/bun-version). +Install Node 24, pnpm, and the Bun version in [`config/.bun-version`](../config/.bun-version). `pnpm test` runs Vitest on Bun, with Node workers for runtime contracts such as SQLite, native PTYs, socket liveness, and V8 memory behavior. `pnpm test:node` runs the same suites entirely on Node. Builds and dependency installation still use Node and pnpm. diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 658c5eb6ee8..cd6d63011f3 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -201,7 +201,7 @@ jobs: if: needs.code_paths.outputs.static_analysis == 'true' uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 with: - bun-version-file: config/bun-version + bun-version-file: config/.bun-version # Keep each check in its own log while sharing this runner. - name: Lint diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index efb236d0c0e..c4ff51dd1e1 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -50,7 +50,7 @@ jobs: - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 with: - bun-version-file: config/bun-version + bun-version-file: config/.bun-version - uses: actions/download-artifact@v8 continue-on-error: true diff --git a/config/bun-version b/config/.bun-version similarity index 100% rename from config/bun-version rename to config/.bun-version diff --git a/config/scripts/check-runtime-electron-ratchet-graph.test.mjs b/config/scripts/check-runtime-electron-ratchet-graph.test.mjs new file mode 100644 index 00000000000..943b24fcd78 --- /dev/null +++ b/config/scripts/check-runtime-electron-ratchet-graph.test.mjs @@ -0,0 +1,114 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import path from 'node:path' +import { collectElectronImporters } from './check-runtime-electron-ratchet.mjs' + +const roots = [] +afterEach(() => { + for (const root of roots.splice(0)) { + rmSync(root, { recursive: true, force: true }) + } +}) + +function fixture(files) { + const root = mkdtempSync(path.join(tmpdir(), 'orca-electron-ratchet-graph-')) + roots.push(root) + for (const [file, source] of Object.entries(files)) { + const absolute = path.join(root, file) + mkdirSync(path.dirname(absolute), { recursive: true }) + writeFileSync(absolute, source) + } + return root +} + +async function inspectGraph(entries, { separateEntries = false, plugins = [] } = {}) { + let result + const inspect = { + name: 'inspect-electron-ratchet-graph', + setup(pluginBuild) { + if (separateEntries) { + delete pluginBuild.initialOptions.stdin + pluginBuild.initialOptions.entryPoints = entries + } + pluginBuild.onEnd((built) => { + result = built + }) + } + } + const importers = await collectElectronImporters(entries, { plugins: [...plugins, inspect] }) + return { importers, ...result } +} + +function fixtureInputs(result, root) { + return Object.fromEntries( + Object.entries(result.metafile.inputs) + .filter(([file]) => file.includes(path.basename(root))) + .sort(([left], [right]) => left.localeCompare(right)) + ) +} + +describe('the runtime Electron import graph', () => { + it('retains every original CommonJS input and Electron edge when sharing root dependencies', async () => { + const root = fixture({ + 'entry.mjs': ` + export { value } from './shared.cjs' + export { value as imported } from 'conditional-package' + export const load = () => import('./dynamic.mjs') + `, + 'future.cjs': ` + module.exports = { + value: require('conditional-package').value, + bare: () => require.resolve('electron'), + subpath: () => require.resolve('electron/main'), + addon: () => require('./missing.node'), + desktop: () => require('desktop-package') + } + `, + 'shared.cjs': 'exports.value = 42', + 'dynamic.mjs': "import 'electron/renderer'; export const value = 1", + 'node_modules/conditional-package/package.json': JSON.stringify({ + exports: { '.': { import: './import.mjs', require: './require.cjs' } } + }), + 'node_modules/conditional-package/import.mjs': "import 'electron'; export const value = 2", + 'node_modules/conditional-package/require.cjs': "require('electron/main'); exports.value = 3", + 'node_modules/desktop-package/package.json': '{"main":"index.cjs"}', + 'node_modules/desktop-package/index.cjs': "require('electron'); exports.value = 4" + }) + const injectElectron = { + name: 'inject-future-electron-import', + setup(pluginBuild) { + pluginBuild.onLoad({ filter: /future\.cjs$/ }, (args) => ({ + contents: `require('electron/utility')\n${readFileSync(args.path, 'utf8')}`, + loader: 'js' + })) + } + } + const entries = ['entry.mjs', 'future.cjs', 'shared.cjs'].map((file) => path.join(root, file)) + const original = await inspectGraph(entries, { + separateEntries: true, + plugins: [injectElectron] + }) + const shared = await inspectGraph(entries, { plugins: [injectElectron] }) + + expect(fixtureInputs(shared, root)).toEqual(fixtureInputs(original, root)) + expect(shared.importers).toEqual(original.importers) + expect(shared.importers.map((file) => file.slice(file.indexOf(path.basename(root))))).toEqual([ + `${path.basename(root)}/dynamic.mjs`, + `${path.basename(root)}/future.cjs`, + `${path.basename(root)}/node_modules/conditional-package/import.mjs`, + `${path.basename(root)}/node_modules/conditional-package/require.cjs`, + `${path.basename(root)}/node_modules/desktop-package/index.cjs` + ]) + const future = Object.entries(shared.metafile.inputs).find(([file]) => + file.endsWith('/future.cjs') + ) + expect(future[1].imports).toEqual( + expect.arrayContaining([ + expect.objectContaining({ path: 'electron', kind: 'require-resolve', external: true }), + expect.objectContaining({ path: 'electron/main', kind: 'require-resolve', external: true }), + expect.objectContaining({ path: './missing.node', external: true }) + ]) + ) + }) +}) diff --git a/config/scripts/run-vitest.mjs b/config/scripts/run-vitest.mjs index 21cb3ae4482..35bb720259a 100644 --- a/config/scripts/run-vitest.mjs +++ b/config/scripts/run-vitest.mjs @@ -19,6 +19,6 @@ try { process.exitCode = result.code ?? 1 } catch (error) { - console.error('Could not start Vitest. Install the Bun version in config/bun-version.', error) + console.error('Could not start Vitest. Install the Bun version in config/.bun-version.', error) process.exitCode = 1 } diff --git a/src/main/codex-accounts/legacy-wsl-runtime-auth-drain-interruption-source.test.ts b/src/main/codex-accounts/legacy-wsl-runtime-auth-drain-interruption-source.test.ts new file mode 100644 index 00000000000..5e6cd9af9b1 --- /dev/null +++ b/src/main/codex-accounts/legacy-wsl-runtime-auth-drain-interruption-source.test.ts @@ -0,0 +1,82 @@ +import { runInNewContext } from 'node:vm' +import { describe, expect, it, vi } from 'vitest' +import { DRAIN_APPLY_INTERRUPTION_JS } from './legacy-wsl-runtime-auth-drain-interruption-source' + +type CensusResult = { + status: number | null + stdout?: string + signal?: string + error?: Error +} + +function fixture(rows: CensusResult[], ownedPid = '100') { + const kill = vi.fn() + const census = vi.fn() + for (const row of rows) { + census.mockReturnValueOnce(row) + } + const context = { + process: { pid: 300, env: { ORCA_DRAIN_APPLY_PID: ownedPid }, kill }, + require: () => ({ spawnSync: census }) + } + return { + kill, + census, + interrupt: (depth = 2) => + runInNewContext(`${DRAIN_APPLY_INTERRUPTION_JS}\ninterruptDrainApply(${depth})`, context) + } +} + +describe('owned drain interruption', () => { + it.each([1, 2])('signals only the owned apply shell at ancestor depth %s', (depth) => { + const rows = depth === 1 ? ['100'] : ['200', '100'] + const target = fixture(rows.map((stdout) => ({ status: 0, stdout }))) + target.interrupt(depth) + expect(target.kill).toHaveBeenCalledExactlyOnceWith(100, 'SIGKILL') + expect(target.census.mock.calls.map(([, args]) => args)).toEqual( + depth === 1 + ? [['-o', 'ppid=', '-p', '300']] + : [ + ['-o', 'ppid=', '-p', '300'], + ['-o', 'ppid=', '-p', '200'] + ] + ) + }) + + it.each([ + { status: 0, stdout: '' }, + { status: 0, stdout: ' \n' }, + { status: 0, stdout: '0' }, + { status: 0, stdout: '-1' }, + { status: 0, stdout: '1' }, + { status: 0, stdout: '9007199254740992' }, + { status: 0, stdout: '100\n200' }, + { status: 1, stdout: '100' }, + { status: null, stdout: '100', signal: 'SIGTERM' }, + { status: 0, stdout: '100', error: new Error('census failed') }, + { status: 0 } + ])('refuses an invalid census without sending a signal: %j', (row) => { + const target = fixture([{ status: 0, stdout: '200' }, row]) + expect(() => target.interrupt()).toThrow() + expect(target.kill).not.toHaveBeenCalled() + }) + + it('refuses a valid ancestor that belongs to another process', () => { + const target = fixture([ + { status: 0, stdout: '200' }, + { status: 0, stdout: '99' } + ]) + expect(() => target.interrupt()).toThrow('Apply ancestor is not owned') + expect(target.kill).not.toHaveBeenCalled() + }) + + it.each(['', '0', '-1', '1', '300', '9007199254740992'])( + 'refuses an invalid owned shell PID %j before running the census', + (pid) => { + const target = fixture([], pid) + expect(() => target.interrupt()).toThrow() + expect(target.census).not.toHaveBeenCalled() + expect(target.kill).not.toHaveBeenCalled() + } + ) +}) diff --git a/src/main/codex-accounts/legacy-wsl-runtime-auth-drain-interruption-source.ts b/src/main/codex-accounts/legacy-wsl-runtime-auth-drain-interruption-source.ts new file mode 100644 index 00000000000..fc6799712b9 --- /dev/null +++ b/src/main/codex-accounts/legacy-wsl-runtime-auth-drain-interruption-source.ts @@ -0,0 +1,25 @@ +/** Interruption fixtures must never signal the runner's inherited process group. */ +export const DRAIN_APPLY_INTERRUPTION_JS = String.raw` +function interruptDrainApply(ancestorDepth) { + const expectedText = process.env.ORCA_DRAIN_APPLY_PID || '' + if (!/^[1-9][0-9]*$/.test(expectedText)) throw new Error('Missing owned apply PID') + const expectedPid = Number(expectedText) + if (!Number.isSafeInteger(expectedPid) || expectedPid <= 1 || expectedPid === process.pid) { + throw new Error('Invalid owned apply PID') + } + let ancestor = process.pid + for (let depth = 0; depth < ancestorDepth; depth++) { + const result = require('node:child_process').spawnSync( + '/bin/ps', ['-o', 'ppid=', '-p', String(ancestor)], { encoding: 'utf8' } + ) + const text = typeof result.stdout === 'string' ? result.stdout.trim() : '' + if (result.status !== 0 || result.signal || result.error || !/^[1-9][0-9]*$/.test(text)) { + throw new Error('Unverified apply ancestry') + } + ancestor = Number(text) + if (!Number.isSafeInteger(ancestor) || ancestor <= 1) throw new Error('Invalid apply ancestor') + } + if (ancestor !== expectedPid) throw new Error('Apply ancestor is not owned') + process.kill(ancestor, 'SIGKILL') +} +` diff --git a/src/main/codex-accounts/legacy-wsl-runtime-auth-drain-script-harness.ts b/src/main/codex-accounts/legacy-wsl-runtime-auth-drain-script-harness.ts index 1f35d4fcda0..fde9c5b37ec 100644 --- a/src/main/codex-accounts/legacy-wsl-runtime-auth-drain-script-harness.ts +++ b/src/main/codex-accounts/legacy-wsl-runtime-auth-drain-script-harness.ts @@ -15,6 +15,7 @@ import { import { tmpdir } from 'node:os' import { join } from 'node:path' import { installDrainInterferenceShims } from './legacy-wsl-runtime-auth-drain-script-interference-shims' +import { DRAIN_APPLY_INTERRUPTION_JS } from './legacy-wsl-runtime-auth-drain-interruption-source' import { INTRUDER_AUTH, NEWER_AUTH, @@ -69,6 +70,7 @@ export function runApplyScript(options: DrainApplyInterference = {}): DrainApply `#!/usr/bin/env node const { spawnSync } = require('node:child_process') const fs = require('node:fs') +${DRAIN_APPLY_INTERRUPTION_JS} const args = process.argv.slice(2) const result = spawnSync('/bin/rm', args, { stdio: 'inherit' }) const target = args.at(-1) ?? '' @@ -78,10 +80,7 @@ if ( target.includes('/account/sessions/') && target.includes('.orca-bridge-') ) { - const parent = spawnSync('/bin/ps', ['-o', 'ppid=', '-p', String(process.ppid)], { - encoding: 'utf8' - }) - process.kill(Number(parent.stdout.trim()), 'SIGKILL') + interruptDrainApply(2) } process.exit(result.status ?? 1) ` @@ -103,12 +102,13 @@ process.exit(result.status ?? 1) `#!/usr/bin/env node const { spawnSync } = require('node:child_process') const fs = require('node:fs') +${DRAIN_APPLY_INTERRUPTION_JS} const args = process.argv.slice(2) if ( process.env.KILL_DESTINATION_RECOVERY === '1' && args.at(-1)?.endsWith('.orca-drain-destination') ) { - process.kill(process.ppid, 'SIGKILL') + interruptDrainApply(1) process.exit(1) } if ( @@ -125,10 +125,7 @@ if ( args.at(-1)?.endsWith('/retired.jsonl') ) { if (process.env.KILL_SESSION_LINK === '1') { - const parent = spawnSync('/bin/ps', ['-o', 'ppid=', '-p', String(process.ppid)], { - encoding: 'utf8' - }) - process.kill(Number(parent.stdout.trim()), 'SIGKILL') + interruptDrainApply(2) } else if (process.env.REWRITE_AFTER_SESSION_LINK === '1') { if (process.env.REPLACE_TARGET === '1') { const replacement = process.env.REWRITE_SESSION_AUTH + '.replacement' @@ -154,7 +151,7 @@ process.exit(result.status ?? 1) '/bin/sh', [ '-c', - _internals.applyLegacyAuthScript, + `ORCA_DRAIN_APPLY_PID=$$; export ORCA_DRAIN_APPLY_PID;\n${_internals.applyLegacyAuthScript}`, 'sh', legacyHome, join(root, 'absent-active-home'), diff --git a/src/main/codex-accounts/legacy-wsl-runtime-auth-drain-script-interference-shims.ts b/src/main/codex-accounts/legacy-wsl-runtime-auth-drain-script-interference-shims.ts index 735e95bec86..8fe8561e11d 100644 --- a/src/main/codex-accounts/legacy-wsl-runtime-auth-drain-script-interference-shims.ts +++ b/src/main/codex-accounts/legacy-wsl-runtime-auth-drain-script-interference-shims.ts @@ -2,6 +2,7 @@ // interfered with at exact points - a hash read, an install rename, a source removal. import { chmodSync, writeFileSync } from 'node:fs' import { join } from 'node:path' +import { DRAIN_APPLY_INTERRUPTION_JS } from './legacy-wsl-runtime-auth-drain-interruption-source' export function installDrainInterferenceShims( binDir: string, @@ -39,6 +40,7 @@ export function installDrainInterferenceShims( `#!/usr/bin/env node const { spawnSync } = require('node:child_process') const fs = require('node:fs') + ${DRAIN_APPLY_INTERRUPTION_JS} const args = process.argv.slice(2) const result = spawnSync('/bin/mv', args, { stdio: 'inherit' }) const from = args.at(-2) ?? '' @@ -52,7 +54,7 @@ export function installDrainInterferenceShims( from.includes('/account/auth.json.orca-drain-snapshot-') && to.endsWith('/account/auth.json') if (result.status === 0 && (sourceInstalled || destinationInstalled)) { - process.kill(process.ppid, 'SIGKILL') + interruptDrainApply(1) } process.exit(result.status ?? 1) ` diff --git a/src/main/native-chat/transcript-watch-inplace-drain-race.test.ts b/src/main/native-chat/transcript-watch-inplace-drain-race.test.ts index c52de9a9896..0a09797b0be 100644 --- a/src/main/native-chat/transcript-watch-inplace-drain-race.test.ts +++ b/src/main/native-chat/transcript-watch-inplace-drain-race.test.ts @@ -1,4 +1,4 @@ -import { writeFileSync } from 'node:fs' +import { utimesSync, writeFileSync } from 'node:fs' import { appendFile, mkdtemp, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -15,52 +15,84 @@ function record(id: string, text: string): string { })}\n` } -it.each(['append', 'initial snapshot', 'replacement snapshot'] as const)( - 'delivers a larger rewrite while publishing a completed %s', - async (mode) => { - const before = getActiveNativeChatWatcherCount() - const root = await mkdtemp(join(tmpdir(), 'orca-transcript-drain-race-')) - const filePath = join(root, 'rollout.jsonl') - const seen: string[] = [] - let stop = (): void => {} - try { - await writeFile(filePath, record('old', 'old')) - const publish = (messages: NativeChatMessage[]): void => { - seen.push(...messages.map((message) => message.id)) - if (messages.some((message) => message.id === 'old')) { - writeFileSync( - filePath, - record( - mode === 'replacement snapshot' ? 'middle' : 'new', - 'a larger replacement transcript' - ) - ) - } - if (messages.some((message) => message.id === 'middle')) { - writeFileSync(filePath, record('new', 'a second still larger replacement transcript')) - } +it.each([ + 'append', + 'initial snapshot', + 'replacement snapshot', + 'metadata-only replacement replay' +] as const)('delivers a larger rewrite while publishing a completed %s', async (mode) => { + const before = getActiveNativeChatWatcherCount() + const root = await mkdtemp(join(tmpdir(), 'orca-transcript-drain-race-')) + const filePath = join(root, 'rollout.jsonl') + const seen: string[] = [] + const appendDeliveries: string[] = [] + let newReplacementCount = 0 + const usesReplacement = + mode === 'replacement snapshot' || mode === 'metadata-only replacement replay' + let stop = (): void => {} + try { + await writeFile(filePath, record('old', 'old')) + const publish = (messages: NativeChatMessage[], replacement: boolean): void => { + const ids = messages.map((message) => message.id) + if (replacement) { + // Full snapshots replace the client's view and may replay an existing message. + seen.splice(0, seen.length, ...ids) + } else { + seen.push(...ids) + appendDeliveries.push(...ids) + } + if (messages.some((message) => message.id === 'old')) { + writeFileSync( + filePath, + record(usesReplacement ? 'middle' : 'new', 'a larger replacement transcript') + ) + } + if (messages.some((message) => message.id === 'middle')) { + writeFileSync(filePath, record('new', 'a second still larger replacement transcript')) } - const sub = await subscribeNativeChatTranscript({ - agent: 'claude', - sessionId: 'ignored', - filePath, - debounceMs: 0, - reconciliationIntervalMs: 20, - initialLimit: mode === 'append' ? undefined : 50, - onAppend: publish, - ...(mode === 'initial snapshot' ? { onInitialSnapshot: publish } : {}), - ...(mode === 'replacement snapshot' ? { onReplace: publish } : {}) - }) - stop = sub.unsubscribe - await expect.poll(() => seen, { timeout: 1_000 }).toContain('new') - await appendFile(filePath, record('followup', 'normal append')) - await expect.poll(() => seen, { timeout: 1_000 }).toContain('followup') - expect(seen.filter((id) => id === 'new')).toHaveLength(1) - expect(seen.filter((id) => id === 'followup')).toHaveLength(1) - } finally { - stop() - await rm(root, { recursive: true, force: true }) - expect(getActiveNativeChatWatcherCount()).toBe(before) } + const sub = await subscribeNativeChatTranscript({ + agent: 'claude', + sessionId: 'ignored', + filePath, + debounceMs: 0, + reconciliationIntervalMs: 20, + initialLimit: mode === 'append' ? undefined : 50, + onAppend: (messages) => publish(messages, false), + ...(mode === 'initial snapshot' + ? { onInitialSnapshot: (messages) => publish(messages, true) } + : {}), + ...(usesReplacement + ? { + onReplace: (messages) => { + if (messages.some((message) => message.id === 'new')) { + newReplacementCount += 1 + } + publish(messages, true) + } + } + : {}) + }) + stop = sub.unsubscribe + await expect.poll(() => seen, { timeout: 1_000 }).toContain('new') + if (mode === 'metadata-only replacement replay') { + const beforeReplay = newReplacementCount + expect(beforeReplay).toBeGreaterThanOrEqual(1) + const updated = new Date(Date.now() + 2_000) + utimesSync(filePath, updated, updated) + await expect.poll(() => newReplacementCount, { timeout: 1_000 }).toBeGreaterThan(beforeReplay) + expect(seen).toEqual(['new']) + expect(appendDeliveries).not.toContain('new') + } + await appendFile(filePath, record('followup', 'normal append')) + await expect.poll(() => seen, { timeout: 1_000 }).toContain('followup') + expect(seen.filter((id) => id === 'new')).toHaveLength(1) + expect(seen.filter((id) => id === 'followup')).toHaveLength(1) + expect(appendDeliveries.filter((id) => id === 'new')).toHaveLength(usesReplacement ? 0 : 1) + expect(appendDeliveries.filter((id) => id === 'followup').length).toBeLessThanOrEqual(1) + } finally { + stop() + await rm(root, { recursive: true, force: true }) + expect(getActiveNativeChatWatcherCount()).toBe(before) } -) +}) diff --git a/src/main/provider-process/provider-process-teardown.test.ts b/src/main/provider-process/provider-process-teardown.test.ts index 62aeb53ccba..41ad22387d7 100644 --- a/src/main/provider-process/provider-process-teardown.test.ts +++ b/src/main/provider-process/provider-process-teardown.test.ts @@ -7,7 +7,6 @@ import { import type { DescendantTreeVerdict } from '../pty-descendant-exit-verification' import { terminateProviderProcessTree } from './provider-process-teardown' -/** Above pid_max on every supported POSIX host, so the group signal is a real ESRCH. */ const UNREACHABLE_PGID = 2_147_483_647 function child() { @@ -68,6 +67,7 @@ describe('terminateProviderProcessTree', () => { it('waits for an owned POSIX snapshot before killing the wrapper', async () => { const target = child() + const signalProcessGroup = vi.fn() const snapshot = { rootPgid: 1234, descendants: [], capturedAtMs: 1 } const release = Promise.withResolvers() @@ -75,7 +75,8 @@ describe('terminateProviderProcessTree', () => { site: 'codex-app-server-teardown', platform: 'darwin', captureDescendants: async () => snapshot, - terminateDescendants: () => release.promise + terminateDescendants: () => release.promise, + signalProcessGroup }) await vi.waitFor(() => expect(target.kill).toHaveBeenCalledWith('SIGSTOP')) expect(target.kill).not.toHaveBeenCalledWith('SIGKILL') @@ -83,6 +84,7 @@ describe('terminateProviderProcessTree', () => { await expect(teardown).resolves.toBe('exited') expect(target.kill).toHaveBeenLastCalledWith('SIGKILL') + expect(signalProcessGroup).toHaveBeenCalledWith(1234, 'SIGKILL') }) it('signals a proven dedicated POSIX process group without scanning descendants', async () => { @@ -125,31 +127,32 @@ describe('terminateProviderProcessTree', () => { expect(target.kill).not.toHaveBeenCalled() }) - /** - * `selfInitiatedTreeKillCount` decides whether a `render-process-gone` was - * ours. A group that had already exited was killed by nobody, so crediting it - * puts a suspect in the five-second window that Orca never issued. Exercised - * through the real `process.kill(-pgid)` because the swallow being tested - * lives in the production default, not in an injectable seam. - */ + // An intercepted ESRCH must exercise the default signal path without creating a false kill breadcrumb. it('does not claim a snapshot group that was already gone', async () => { const target = { pid: UNREACHABLE_PGID, kill: vi.fn(() => true) } + const kill = vi.spyOn(process, 'kill').mockImplementation(() => { + throw Object.assign(new Error('already exited'), { code: 'ESRCH' }) + }) + try { + await expect( + terminateProviderProcessTree(target, { + site: 'codex-app-server-teardown', + platform: 'darwin', + captureDescendants: async () => ({ + rootPgid: UNREACHABLE_PGID, + descendants: [], + capturedAtMs: 1 + }), + terminateDescendants: async () => 'exited' + }) + ).resolves.toBe('exited') - await expect( - terminateProviderProcessTree(target, { - site: 'codex-app-server-teardown', - platform: 'darwin', - captureDescendants: async () => ({ - rootPgid: UNREACHABLE_PGID, - descendants: [], - capturedAtMs: 1 - }), - terminateDescendants: async () => 'exited' - }) - ).resolves.toBe('exited') - - expect(target.kill).toHaveBeenLastCalledWith('SIGKILL') - expect(findSelfInitiatedTreeKills(Date.now())).toEqual([]) + expect(kill).toHaveBeenCalledWith(-UNREACHABLE_PGID, 'SIGKILL') + expect(target.kill).toHaveBeenLastCalledWith('SIGKILL') + expect(findSelfInitiatedTreeKills(Date.now())).toEqual([]) + } finally { + kill.mockRestore() + } }) it('claims a snapshot group the signal actually reached', async () => {