diff --git a/.github/workflows/computer-e2e.yml b/.github/workflows/computer-e2e.yml index dbd62e77200..e6b16fab7a7 100644 --- a/.github/workflows/computer-e2e.yml +++ b/.github/workflows/computer-e2e.yml @@ -10,6 +10,7 @@ on: - 'config/scripts/build-windows-cli-launcher.test.mjs' - 'config/scripts/computer-e2e-workflow.test.mjs' - 'config/scripts/macos-computer-helper-owner-loss-benchmark.mjs' + - 'config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs' - 'config/scripts/macos-computer-helper-owner-loss-metrics.mjs' - 'config/scripts/macos-computer-helper-owner-loss-processes.mjs' - 'config/scripts/macos-computer-helper-owner-loss-processes.test.mjs' @@ -92,6 +93,7 @@ jobs: config/scripts/build-windows-cli-launcher.test.mjs src/main/ssh/ssh-remote-cli-launcher.test.ts config/scripts/computer-e2e-workflow.test.mjs + config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs config/scripts/macos-computer-helper-owner-loss-processes.test.mjs config/scripts/computer-use-modifier-safety.test.mjs config/scripts/computer-use-skill-guidance.test.mjs @@ -165,7 +167,10 @@ jobs: run_install: false - run: pnpm install --frozen-lockfile - name: Owner-loss benchmark process cleanup - run: pnpm vitest run config/scripts/macos-computer-helper-owner-loss-processes.test.mjs + run: >- + pnpm vitest run + config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs + config/scripts/macos-computer-helper-owner-loss-processes.test.mjs - name: Authenticated helper owner-loss smoke run: pnpm bench:macos-computer-helper-owner-loss --expect reaped --trials 1 - name: Swift tests and signed universal helper verification diff --git a/config/scripts/computer-e2e-workflow.test.mjs b/config/scripts/computer-e2e-workflow.test.mjs index 17972e32031..a4230668bc7 100644 --- a/config/scripts/computer-e2e-workflow.test.mjs +++ b/config/scripts/computer-e2e-workflow.test.mjs @@ -47,6 +47,7 @@ describe('computer-use e2e workflow', () => { expect(triggerPaths).toEqual( expect.arrayContaining([ 'config/scripts/computer-e2e-workflow.test.mjs', + 'config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs', 'config/scripts/computer-use-modifier-safety.test.mjs', 'config/scripts/computer-use-skill-guidance.test.mjs', 'config/scripts/computer-use-smoke.mjs', @@ -76,6 +77,7 @@ describe('computer-use e2e workflow', () => { const regressionRun = nativeSmokeRuns.find((run) => run.includes('pnpm vitest run')) const expectedRegressionFiles = [ 'config/scripts/computer-e2e-workflow.test.mjs', + 'config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs', 'config/scripts/macos-computer-helper-owner-loss-processes.test.mjs', 'config/scripts/computer-use-modifier-safety.test.mjs', 'config/scripts/computer-use-skill-guidance.test.mjs', @@ -129,14 +131,18 @@ describe('computer-use e2e workflow', () => { expect(job['runs-on']).toBe('macos-15') expect(checkout.with['persist-credentials']).toBe(false) expect(runs).toContain('pnpm bench:macos-computer-helper-owner-loss --expect reaped --trials 1') - expect(runs).toContain( - 'pnpm vitest run config/scripts/macos-computer-helper-owner-loss-processes.test.mjs' + const cleanupRun = runs.find((run) => + run.includes('config/scripts/macos-computer-helper-owner-loss-processes.test.mjs') + ) + expect(cleanupRun).toContain( + 'config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs' ) expect(runs).toContain('pnpm verify:computer-native') expect(runs.join('\n')).not.toContain('test:e2e:computer') expect(workflow.on.pull_request.paths).toEqual( expect.arrayContaining([ 'config/scripts/macos-computer-helper-owner-loss-benchmark.mjs', + 'config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs', 'config/scripts/macos-computer-helper-owner-loss-metrics.mjs', 'config/scripts/macos-computer-helper-owner-loss-processes.mjs', 'config/scripts/macos-computer-helper-owner-loss-processes.test.mjs', @@ -158,6 +164,11 @@ describe('computer-use e2e workflow', () => { expect(benchmark).toContain('spawnBenchmarkProcess(executable, [launcherDir]') expect(benchmark).toContain("stdio: ['ignore', stdoutDescriptor, stderrDescriptor]") expect(benchmark).toContain('cleanupOwnerLossTrial({') + const parseIndex = benchmark.indexOf('parseBenchmarkTrialResult(serializedResult)') + const cleanupIndex = benchmark.indexOf('cleanupOwnerLossTrial({') + expect(parseIndex).toBeGreaterThanOrEqual(0) + expect(cleanupIndex).toBeGreaterThanOrEqual(0) + expect(parseIndex).toBeLessThan(cleanupIndex) expect(benchmark).toContain('trialCleanupSha256: artifactSha256(trialCleanupPath)') expect(cleanup).toContain('killRecordedAndMatchingProcesses(options.recordPath') expect(cleanup).toContain("signalValidatedProcessGroup(options.pid, options.marker, 'SIGKILL'") diff --git a/config/scripts/macos-computer-helper-owner-loss-benchmark.mjs b/config/scripts/macos-computer-helper-owner-loss-benchmark.mjs index 5cb99835fd1..8724cf8faf4 100644 --- a/config/scripts/macos-computer-helper-owner-loss-benchmark.mjs +++ b/config/scripts/macos-computer-helper-owner-loss-benchmark.mjs @@ -13,6 +13,8 @@ import { sampleProcess } from './macos-computer-helper-owner-loss-metrics.mjs' import { + benchmarkTrialNeedsCleanup, + parseBenchmarkTrialResult, processIdentityIsCurrent, signalProcessIdentity, spawnBenchmarkProcess, @@ -434,31 +436,39 @@ function artifactSha256(artifactPath) { } function runTrial(executable, expectation) { - const launcherDir = mkdtempSync(path.join(tmpdir(), 'orca-helper-owner-bench-launcher-')) - const trialTempDir = mkdtempSync(path.join(path.sep, 'tmp', 'orca-owner-bench-')) - const helperRecordPath = path.join(launcherDir, 'helper.json') - const resultPath = path.join(launcherDir, 'result.json') - const stderrPath = path.join(launcherDir, 'stderr.log') - const stdoutPath = path.join(launcherDir, 'stdout.log') - writeFileSync( - path.join(launcherDir, 'package.json'), - JSON.stringify({ name: 'orca-helper-owner-benchmark', main: 'main.cjs' }) - ) - writeFileSync( - path.join(launcherDir, 'main.cjs'), - `import(${JSON.stringify(pathToFileURL(scriptPath).href)}).catch((error) => { - console.error(error) - process.exitCode = 1 -})\n` - ) + let launcherDir + let trialTempDir + let helperRecordPath + let resultPath + let stderrPath + let stdoutPath let result let serializedResult + let parsedResult + let parsedResultAvailable = false let trialError let cleanupError let stderrDescriptor let stdoutDescriptor let trialOutput = '' try { + launcherDir = mkdtempSync(path.join(tmpdir(), 'orca-helper-owner-bench-launcher-')) + trialTempDir = mkdtempSync(path.join(path.sep, 'tmp', 'orca-owner-bench-')) + helperRecordPath = path.join(launcherDir, 'helper.json') + resultPath = path.join(launcherDir, 'result.json') + stderrPath = path.join(launcherDir, 'stderr.log') + stdoutPath = path.join(launcherDir, 'stdout.log') + writeFileSync( + path.join(launcherDir, 'package.json'), + JSON.stringify({ name: 'orca-helper-owner-benchmark', main: 'main.cjs' }) + ) + writeFileSync( + path.join(launcherDir, 'main.cjs'), + `import(${JSON.stringify(pathToFileURL(scriptPath).href)}).catch((error) => { + console.error(error) + process.exitCode = 1 +})\n` + ) const env = { ...process.env, TMPDIR: trialTempDir, @@ -478,12 +488,14 @@ function runTrial(executable, expectation) { }) if (result.status === 0 && existsSync(resultPath)) { serializedResult = readFileSync(resultPath, 'utf8') + parsedResult = parseBenchmarkTrialResult(serializedResult) + parsedResultAvailable = true } } catch (error) { trialError = error } finally { - const failedTrial = result?.status !== 0 || !serializedResult - const trialMarker = `TMPDIR=${trialTempDir}` + const failedTrial = benchmarkTrialNeedsCleanup(result, parsedResultAvailable) + const trialMarker = trialTempDir ? `TMPDIR=${trialTempDir}` : undefined const cleanup = cleanupOwnerLossTrial({ failed: failedTrial, pid: result?.pid, @@ -508,7 +520,7 @@ function runTrial(executable, expectation) { trialError = new Error(`Electron trial did not write a result:\n${trialOutput}`) } throwBenchmarkTrialFailures(trialError, cleanupError) - return JSON.parse(serializedResult) + return parsedResult } function runBenchmark() { diff --git a/config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs b/config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs new file mode 100644 index 00000000000..512a663cddb --- /dev/null +++ b/config/scripts/macos-computer-helper-owner-loss-group-recovery.test.mjs @@ -0,0 +1,136 @@ +import { describe, expect, it } from 'vitest' +import { signalValidatedProcessGroup } from './macos-computer-helper-owner-loss-processes.mjs' + +describe('macOS helper owner-loss benchmark group recovery', () => { + it('retains uncertain stop state across cleanup stage failures', () => { + const marker = 'ORCA_OWNER_GROUP=trial' + const members = [{ pid: 41, pgid: 41, command: `/launcher ${marker}` }] + const groupState = { stopped: false } + let scanCount = 0 + let continueAttempts = 0 + const operations = { + processIdentities: () => { + scanCount += 1 + if (scanCount >= 3) { + throw new Error( + scanCount === 3 ? 'post-stop inspection failed' : 'final inspection failed' + ) + } + return members + }, + signalProcess: (pid, signal) => { + if (pid === -41 && signal === 'SIGCONT') { + continueAttempts += 1 + if (continueAttempts === 1) { + throw new Error('first compensation failed') + } + } + } + } + + expect(() => + signalValidatedProcessGroup(41, marker, 'SIGSTOP', groupState, operations) + ).toThrow('Benchmark process group signal recovery failed') + expect(groupState.stopped).toBe(true) + expect(() => + signalValidatedProcessGroup(41, marker, 'SIGKILL', groupState, operations) + ).toThrow('final inspection failed') + expect(continueAttempts).toBe(2) + expect(groupState.stopped).toBe(false) + }) + + it('retains an uncertain anchor stop across cleanup stage failures', () => { + const marker = 'ORCA_OWNER_GROUP=trial' + const members = [{ pid: 41, pgid: 41, command: `/launcher ${marker}` }] + const groupState = { stopped: false, anchorPid: null } + let scanCount = 0 + let continueAttempts = 0 + const operations = { + processIdentities: () => { + scanCount += 1 + if (scanCount >= 2) { + throw new Error( + scanCount === 2 ? 'post-anchor inspection failed' : 'final inspection failed' + ) + } + return members + }, + signalProcess: (pid, signal) => { + if (pid === 41 && signal === 'SIGCONT') { + continueAttempts += 1 + if (continueAttempts === 1) { + throw new Error('first anchor compensation failed') + } + } + } + } + + expect(() => + signalValidatedProcessGroup(41, marker, 'SIGSTOP', groupState, operations) + ).toThrow('Benchmark process group signal recovery failed') + expect(groupState.anchorPid).toBe(41) + expect(() => + signalValidatedProcessGroup(41, marker, 'SIGKILL', groupState, operations) + ).toThrow('final inspection failed') + expect(continueAttempts).toBe(2) + expect(groupState.anchorPid).toBeNull() + }) + + it('recovers a retained anchor before selecting a new one', () => { + const marker = 'ORCA_OWNER_GROUP=trial' + const firstAnchor = { pid: 41, pgid: 41, command: `/launcher ${marker}` } + const finalAnchor = { pid: 42, pgid: 41, command: `/child ${marker}` } + const groupState = { stopped: false, anchorPid: null } + let scanCount = 0 + let firstAnchorContinues = 0 + const operations = { + processIdentities: () => { + scanCount += 1 + if (scanCount === 2) { + throw new Error('post-anchor inspection failed') + } + return scanCount === 1 ? [firstAnchor] : [finalAnchor] + }, + signalProcess: (pid, signal) => { + if (pid === 41 && signal === 'SIGCONT') { + firstAnchorContinues += 1 + if (firstAnchorContinues === 1) { + throw new Error('first anchor compensation failed') + } + } + } + } + + expect(() => + signalValidatedProcessGroup(41, marker, 'SIGSTOP', groupState, operations) + ).toThrow('Benchmark process group signal recovery failed') + expect(signalValidatedProcessGroup(41, marker, 'SIGKILL', groupState, operations)).toBe(true) + expect(firstAnchorContinues).toBe(2) + expect(groupState).toEqual({ stopped: false, anchorPid: null }) + }) + + it('preserves group authority errors when compensation fails', () => { + const marker = 'ORCA_OWNER_GROUP=trial' + const ownershipError = 'Benchmark process group no longer belongs to this trial' + let thrown + + try { + signalValidatedProcessGroup( + 41, + marker, + 'SIGKILL', + { stopped: true }, + { + processIdentities: () => [{ pid: 41, pgid: 41, command: '/unrelated' }], + signalProcess: () => { + throw new Error('resume denied') + } + } + ) + } catch (error) { + thrown = error + } + expect(thrown).toBeInstanceOf(AggregateError) + expect(thrown.errors.map((error) => error.message)).toEqual([ownershipError, 'resume denied']) + }) +}) diff --git a/config/scripts/macos-computer-helper-owner-loss-processes.mjs b/config/scripts/macos-computer-helper-owner-loss-processes.mjs index 522bc4e92f8..952d726d28d 100644 --- a/config/scripts/macos-computer-helper-owner-loss-processes.mjs +++ b/config/scripts/macos-computer-helper-owner-loss-processes.mjs @@ -185,16 +185,45 @@ export function throwBenchmarkTrialFailures(trialError, cleanupError) { } } +export function parseBenchmarkTrialResult(serializedResult) { + return JSON.parse(serializedResult) +} + +export function benchmarkTrialNeedsCleanup(spawnResult, parsedResultAvailable) { + return spawnResult?.status !== 0 || !parsedResultAvailable +} + const processGroupSignalOperations = { processIdentities, signalProcess: process.kill.bind(process) } +function compensateStoppedGroup(pgid, groupState, operations) { + const errors = [] + const targets = [ + groupState.stopped ? [-pgid, 'stopped'] : null, + groupState.anchorPid ? [groupState.anchorPid, 'anchorPid'] : null + ].filter(Boolean) + for (const [pid, stateKey] of targets) { + try { + operations.signalProcess(pid, 'SIGCONT') + groupState[stateKey] = stateKey === 'stopped' ? false : null + } catch (error) { + if (error?.code === 'ESRCH') { + groupState[stateKey] = stateKey === 'stopped' ? false : null + } else { + errors.push(error) + } + } + } + return errors +} + export function signalValidatedProcessGroup( pgid, environmentFragment, signal, - groupState = { stopped: false }, + groupState = { stopped: false, anchorPid: null }, operations = processGroupSignalOperations ) { if (!Number.isInteger(pgid) || pgid <= 0) { @@ -204,52 +233,49 @@ export function signalValidatedProcessGroup( try { members = operations.processIdentities(true).filter((identity) => identity.pgid === pgid) } catch (error) { - if (!groupState.stopped) { - throw error - } - try { - operations.signalProcess(-pgid, 'SIGCONT') - groupState.stopped = false - } catch (resumeError) { - if (resumeError?.code === 'ESRCH') { - groupState.stopped = false - } else { - throw new AggregateError( - [error, resumeError], - 'Benchmark process group recovery failed before validation' - ) - } + const recoveryErrors = compensateStoppedGroup(pgid, groupState, operations) + if (recoveryErrors.length > 0) { + throw new AggregateError( + [error, ...recoveryErrors], + 'Benchmark process group recovery failed before validation' + ) } throw error } if (members.length === 0) { + const recoveryErrors = compensateStoppedGroup(pgid, groupState, operations) + if (recoveryErrors.length > 0) { + throw new AggregateError(recoveryErrors, 'Benchmark missing process group recovery failed') + } return false } if (members.some((identity) => !identity.command.includes(environmentFragment))) { const ownershipError = new Error('Benchmark process group no longer belongs to this trial') - if (groupState.stopped) { - try { - operations.signalProcess(-pgid, 'SIGCONT') - groupState.stopped = false - } catch (resumeError) { - if (resumeError?.code === 'ESRCH') { - groupState.stopped = false - } else { - throw new AggregateError( - [ownershipError, resumeError], - 'Benchmark process group authority recovery failed' - ) - } - } + const recoveryErrors = compensateStoppedGroup(pgid, groupState, operations) + if (recoveryErrors.length > 0) { + throw new AggregateError( + [ownershipError, ...recoveryErrors], + 'Benchmark process group authority recovery failed' + ) } throw ownershipError } + if (groupState.anchorPid) { + try { + operations.signalProcess(groupState.anchorPid, 'SIGCONT') + groupState.anchorPid = null + } catch (error) { + if (error?.code === 'ESRCH') { + groupState.anchorPid = null + } else { + throw new AggregateError([error], 'Benchmark pending anchor recovery failed') + } + } + } const anchor = members[0] - let anchorNeedsResume = false - let groupNeedsResume = groupState.stopped try { operations.signalProcess(anchor.pid, 'SIGSTOP') - anchorNeedsResume = true + groupState.anchorPid = anchor.pid const stoppedAnchor = operations .processIdentities(true) .find((identity) => identity.pid === anchor.pid) @@ -257,9 +283,8 @@ export function signalValidatedProcessGroup( throw new Error('Benchmark process group anchor changed before signaling') } operations.signalProcess(-pgid, 'SIGSTOP') - groupNeedsResume = true groupState.stopped = true - anchorNeedsResume = false + groupState.anchorPid = null const stoppedMembers = operations .processIdentities(true) .filter((identity) => identity.pgid === pgid) @@ -274,33 +299,12 @@ export function signalValidatedProcessGroup( if (signal !== 'SIGKILL') { operations.signalProcess(-pgid, 'SIGCONT') } - groupNeedsResume = false groupState.stopped = false + groupState.anchorPid = null } return true } catch (error) { - const recoveryErrors = [] - if (groupNeedsResume) { - try { - operations.signalProcess(-pgid, 'SIGCONT') - groupState.stopped = false - } catch (caught) { - if (caught?.code === 'ESRCH') { - groupState.stopped = false - } else { - recoveryErrors.push(caught) - } - } - } - if (anchorNeedsResume) { - try { - operations.signalProcess(anchor.pid, 'SIGCONT') - } catch (caught) { - if (caught?.code !== 'ESRCH') { - recoveryErrors.push(caught) - } - } - } + const recoveryErrors = compensateStoppedGroup(pgid, groupState, operations) if (recoveryErrors.length > 0) { throw new AggregateError( [error, ...recoveryErrors], diff --git a/config/scripts/macos-computer-helper-owner-loss-processes.test.mjs b/config/scripts/macos-computer-helper-owner-loss-processes.test.mjs index ae799b02fc5..764eb73009f 100644 --- a/config/scripts/macos-computer-helper-owner-loss-processes.test.mjs +++ b/config/scripts/macos-computer-helper-owner-loss-processes.test.mjs @@ -1,12 +1,14 @@ import { execFileSync, spawn } from 'node:child_process' -import { mkdtempSync, readFileSync, rmSync } from 'node:fs' +import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs' import { tmpdir } from 'node:os' import path from 'node:path' import { afterEach, describe, expect, it } from 'vitest' import { + benchmarkTrialNeedsCleanup, killProcessMatchingCommand, killRecordedAndMatchingProcesses, killRecordedProcess, + parseBenchmarkTrialResult, processIdentity, runBenchmarkCleanupStages, signalProcessIdentity, @@ -15,6 +17,7 @@ import { throwBenchmarkTrialFailures, writeProcessRecord } from './macos-computer-helper-owner-loss-processes.mjs' +import { cleanupOwnerLossTrial } from './macos-computer-helper-owner-loss-trial-cleanup.mjs' const describeMacOS = process.platform === 'darwin' ? describe : describe.skip const spawnedPids = new Set() @@ -73,8 +76,13 @@ describeMacOS('macOS helper owner-loss benchmark process cleanup', () => { expect(thrown.errors.map((error) => error.message)).toEqual(['first failure', 'last failure']) }) - it('preserves both trial and cleanup failures', () => { - const trialError = new Error('trial failed with captured output') + it('preserves malformed-result and cleanup failures', () => { + let trialError + try { + parseBenchmarkTrialResult('{malformed') + } catch (error) { + trialError = error + } const cleanupError = new Error('cleanup failed') let thrown @@ -83,10 +91,33 @@ describeMacOS('macOS helper owner-loss benchmark process cleanup', () => { } catch (error) { thrown = error } + expect(trialError).toBeInstanceOf(SyntaxError) expect(thrown).toBeInstanceOf(AggregateError) expect(thrown.errors).toEqual([trialError, cleanupError]) }) + it('cleans up a status-zero trial whose result could not be parsed', () => { + expect(benchmarkTrialNeedsCleanup(undefined, false)).toBe(true) + expect(benchmarkTrialNeedsCleanup({ status: 0 }, false)).toBe(true) + expect(benchmarkTrialNeedsCleanup({ status: 0 }, true)).toBe(false) + expect(benchmarkTrialNeedsCleanup({ status: 1 }, true)).toBe(true) + }) + + it('removes a launcher directory after partial trial setup', () => { + const launcherDir = mkdtempSync(path.join(tmpdir(), 'orca-owner-partial-setup-test-')) + temporaryDirectories.add(launcherDir) + + const cleanup = cleanupOwnerLossTrial({ + failed: true, + launcherDir, + outputPaths: [] + }) + + expect(cleanup.error).toBeUndefined() + expect(existsSync(launcherDir)).toBe(false) + temporaryDirectories.delete(launcherDir) + }) + it('kills a timed-out trial group only after validating its environment', async () => { const temporaryDirectory = mkdtempSync(path.join(tmpdir(), 'orca-owner-benchmark-group-test-')) temporaryDirectories.add(temporaryDirectory) @@ -286,68 +317,6 @@ describeMacOS('macOS helper owner-loss benchmark process cleanup', () => { ]) }) - it('retains uncertain stop state across cleanup stage failures', () => { - const marker = 'ORCA_OWNER_GROUP=trial' - const members = [{ pid: 41, pgid: 41, command: `/launcher ${marker}` }] - const groupState = { stopped: false } - let scanCount = 0 - let continueAttempts = 0 - const operations = { - processIdentities: () => { - scanCount += 1 - if (scanCount >= 3) { - throw new Error( - scanCount === 3 ? 'post-stop inspection failed' : 'final inspection failed' - ) - } - return members - }, - signalProcess: (pid, signal) => { - if (pid === -41 && signal === 'SIGCONT') { - continueAttempts += 1 - if (continueAttempts === 1) { - throw new Error('first compensation failed') - } - } - } - } - - expect(() => - signalValidatedProcessGroup(41, marker, 'SIGSTOP', groupState, operations) - ).toThrow('Benchmark process group signal recovery failed') - expect(groupState.stopped).toBe(true) - expect(() => - signalValidatedProcessGroup(41, marker, 'SIGKILL', groupState, operations) - ).toThrow('final inspection failed') - expect(continueAttempts).toBe(2) - expect(groupState.stopped).toBe(false) - }) - - it('preserves group authority errors when compensation fails', () => { - const marker = 'ORCA_OWNER_GROUP=trial' - const ownershipError = 'Benchmark process group no longer belongs to this trial' - let thrown - - try { - signalValidatedProcessGroup( - 41, - marker, - 'SIGKILL', - { stopped: true }, - { - processIdentities: () => [{ pid: 41, pgid: 41, command: '/unrelated' }], - signalProcess: () => { - throw new Error('resume denied') - } - } - ) - } catch (error) { - thrown = error - } - expect(thrown).toBeInstanceOf(AggregateError) - expect(thrown.errors.map((error) => error.message)).toEqual([ownershipError, 'resume denied']) - }) - it('kills a recorded helper in a separate process group', async () => { const temporaryDirectory = mkdtempSync( path.join(tmpdir(), 'orca-owner-benchmark-cleanup-test-') diff --git a/config/scripts/macos-computer-helper-owner-loss-trial-cleanup.mjs b/config/scripts/macos-computer-helper-owner-loss-trial-cleanup.mjs index 61e0fef6f14..c3fb4aaf384 100644 --- a/config/scripts/macos-computer-helper-owner-loss-trial-cleanup.mjs +++ b/config/scripts/macos-computer-helper-owner-loss-trial-cleanup.mjs @@ -6,18 +6,18 @@ import { } from './macos-computer-helper-owner-loss-processes.mjs' export function cleanupOwnerLossTrial(options) { - const groupState = { stopped: false } + const groupState = { stopped: false, anchorPid: null } let error let output = '' try { runBenchmarkCleanupStages([ () => { - if (options.failed) { + if (options.failed && Number.isInteger(options.pid) && options.marker) { signalValidatedProcessGroup(options.pid, options.marker, 'SIGSTOP', groupState) } }, () => { - if (options.failed) { + if (options.failed && options.recordPath && options.tempDir) { killRecordedAndMatchingProcesses(options.recordPath, options.helperPath, [ options.helperPath, options.tempDir @@ -25,7 +25,7 @@ export function cleanupOwnerLossTrial(options) { } }, () => { - if (options.failed) { + if (options.failed && Number.isInteger(options.pid) && options.marker) { signalValidatedProcessGroup(options.pid, options.marker, 'SIGKILL', groupState) } }, @@ -40,13 +40,21 @@ export function cleanupOwnerLossTrial(options) { } }, () => { - output = options.outputPaths - .filter(existsSync) + output = (options.outputPaths ?? []) + .filter((outputPath) => outputPath && existsSync(outputPath)) .map((outputPath) => readFileSync(outputPath, 'utf8')) .join('') }, - () => rmSync(options.launcherDir, { recursive: true, force: true }), - () => rmSync(options.tempDir, { recursive: true, force: true }) + () => { + if (options.launcherDir) { + rmSync(options.launcherDir, { recursive: true, force: true }) + } + }, + () => { + if (options.tempDir) { + rmSync(options.tempDir, { recursive: true, force: true }) + } + } ]) } catch (caught) { error = caught