diff --git a/changelog.d/stop-macos-eperm-failing-qa-captures.md b/changelog.d/stop-macos-eperm-failing-qa-captures.md new file mode 100644 index 000000000..c90fc63ac --- /dev/null +++ b/changelog.d/stop-macos-eperm-failing-qa-captures.md @@ -0,0 +1 @@ +- **QA captures on macOS no longer record a finished command as incomplete.** After a command exited, the cleanup kill of its process group could get `EPERM` from macOS while the group was being torn down, and `gstack-qa-evidence capture` then recorded `status: incomplete`, `exitCode: 2`. On macOS, an `EPERM` now makes the capture check the group directly and count it as gone only when every remaining member is a zombie or is exiting (`ps` state starting with `Z` or carrying the `E` flag); a live member, or a check that cannot run, still fails closed, and `EPERM` on other platforms fails closed as before. The macOS teardown state is inferred from XNU and `ps`, not measured. Reported with measurements by @vilano93 (#4166). diff --git a/docs/reference-qa-deadlines.md b/docs/reference-qa-deadlines.md index 58f4ae743..b5f754c8a 100644 --- a/docs/reference-qa-deadlines.md +++ b/docs/reference-qa-deadlines.md @@ -73,6 +73,10 @@ including later report Writes and cleanup; it is not a completed-session measure or new-session descendants are outside that guarantee, so detached probes are unsupported. Force-killing the guard itself with SIGKILL also prevents its POSIX cleanup handler from running. +- A cleanup kill that fails with anything other than "no such process" is a guard + error (2), with one macOS exception: an EPERM while the group is being torn + down counts as clean only when every remaining member is a zombie or exiting + (ps state Z or flag E). A live member, or a failed check, still returns 2. - On Windows, dedicated nested Jobs contain the probe worker and its descendants, including children whose immediate parent exits. Failure to initialize this containment prevents the command from starting. diff --git a/lib/qa-deadline.ts b/lib/qa-deadline.ts index 9e65bfb72..246790ad8 100644 --- a/lib/qa-deadline.ts +++ b/lib/qa-deadline.ts @@ -1,7 +1,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { randomUUID } from 'node:crypto'; -import { spawn } from 'node:child_process'; +import { spawn, spawnSync } from 'node:child_process'; import { bunIsolationArgs } from './bun-isolation'; import { constants as osConstants } from 'node:os'; import { initializeWindowsReviewJob } from './claude-code-windows-job'; @@ -120,6 +120,16 @@ export interface QaCommandCapture { spawn?: { cwd?: string; env?: Record }; } +function processGroupEmpty(pgid: number): boolean { + try { process.kill(-pgid, 0); } catch (error) { if ((error as NodeJS.ErrnoException).code === 'ESRCH') return true; } + const listing = spawnSync('ps', ['-A', '-o', 'pgid=,stat='], { encoding: 'utf8', timeout: 5_000 }); + if (listing.status !== 0) return false; + return !listing.stdout.split('\n').some(line => { + const [group, state = ''] = line.trim().split(/\s+/); + return Number(group) === pgid && !state.startsWith('Z') && !state.includes('E'); + }); +} + export async function runQaDeadlineCommand(file: string, command: string, args: string[], emit: Emit, capture?: QaCommandCapture): Promise { if (!['linux', 'darwin', 'win32'].includes(process.platform)) throw new QaDeadlineError('Process containment is unavailable on this platform'); let status = qaDeadlineStatus(readQaDeadline(file)); @@ -152,7 +162,8 @@ export async function runQaDeadlineCommand(file: string, command: string, args: if (process.platform === 'win32') child.kill('SIGKILL'); else process.kill(-child.pid, 'SIGKILL'); } catch (error) { - if ((error as NodeJS.ErrnoException).code !== 'ESRCH') outcome = 2; + const code = (error as NodeJS.ErrnoException).code; + if (code !== 'ESRCH' && !(code === 'EPERM' && process.platform === 'darwin' && processGroupEmpty(child.pid))) outcome = 2; } }; const finish = (code: number, signal: NodeJS.Signals | null = null, completed = false) => { diff --git a/test/qa-evidence.test.ts b/test/qa-evidence.test.ts index 5cf292db7..1343bdb4d 100644 --- a/test/qa-evidence.test.ts +++ b/test/qa-evidence.test.ts @@ -569,3 +569,75 @@ test('materialize accepts a single limits string as a one-item list', () => { expect(report.status, report.stderr).toBe(0); expect(JSON.parse(fs.readFileSync(path.join(f.root, 'evidence.json'), 'utf8')).limits).toEqual(['One probe only.']); }); + +function refusedGroupKill(f: { root: string }, platform: NodeJS.Platform) { + const preload = path.join(f.root, 'refused-group-kill.ts'); + const refused = path.join(f.root, 'refused-group-kill'); + const group = path.join(f.root, 'refused-group'); + fs.writeFileSync(preload, `import { appendFileSync, writeFileSync } from 'node:fs'; +if (process.argv[1] === ${JSON.stringify(CLI)}) { + Object.defineProperty(process, 'platform', { value: ${JSON.stringify(platform)} }); + const kill = process.kill.bind(process); + process.kill = (pid, signal) => { + if (pid >= 0) return kill(pid, signal); + appendFileSync(${JSON.stringify(refused)}, String(signal ?? 'SIGTERM') + '\\n'); + writeFileSync(${JSON.stringify(group)}, String(-pid)); + throw Object.assign(new Error('kill EPERM'), { code: 'EPERM', errno: -1, syscall: 'kill' }); + }; +}`); + return { preload, refused, group }; +} + +test.skipIf(process.platform === 'win32')('a refused group kill after the command exits still records a complete capture once the group is empty', () => { + const f = fixture(); + const stub = refusedGroupKill(f, 'darwin'); + const result = spawnSync(process.execPath, ['--preload', stub.preload, CLI, 'capture', f.root, '001', '--timeout-ms', '4000', '--', process.execPath, '-e', 'console.log("{}")'], + { cwd: f.root, encoding: 'utf8', timeout: 10_000 }); + expect(fs.readFileSync(stub.refused, 'utf8')).toContain('SIGKILL'); + expect(result.status, result.stderr).toBe(0); + expect(receipt(result.stdout)).toMatchObject({ status: 'complete', exitCode: 0 }); +}); + +test.skipIf(process.platform === 'win32')('a refused group kill off macOS fails closed even once the group is empty', () => { + const f = fixture(); + const stub = refusedGroupKill(f, 'linux'); + const result = spawnSync(process.execPath, ['--preload', stub.preload, CLI, 'capture', f.root, '001', '--timeout-ms', '4000', '--', process.execPath, '-e', 'console.log("{}")'], + { cwd: f.root, encoding: 'utf8', timeout: 10_000 }); + expect(fs.readFileSync(stub.refused, 'utf8')).toContain('SIGKILL'); + expect(result.status).not.toBe(0); + expect(receipt(result.stdout)).toMatchObject({ status: 'incomplete', exitCode: 2 }); +}); + +test.skipIf(process.platform === 'win32')('a refused group kill fails closed while the group still has a live member', () => { + const f = fixture(); + const stub = refusedGroupKill(f, 'darwin'); + const marker = path.join(f.root, 'member.pid'); + const program = `const member = require('node:child_process').spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { stdio: 'ignore' }); require('node:fs').writeFileSync(${JSON.stringify(marker)}, String(member.pid)); member.unref(); console.log('{}');`; + let pid = 0; + try { + const result = spawnSync(process.execPath, ['--preload', stub.preload, CLI, 'capture', f.root, '001', '--timeout-ms', '4000', '--', process.execPath, '-e', program], + { cwd: f.root, encoding: 'utf8', timeout: 10_000 }); + pid = Number(fs.readFileSync(marker, 'utf8')); + expect(fs.readFileSync(stub.refused, 'utf8')).toContain('SIGKILL'); + expect(() => process.kill(pid, 0)).not.toThrow(); + expect(result.status).not.toBe(0); + expect(receipt(result.stdout)).toMatchObject({ status: 'incomplete', exitCode: 2 }); + } finally { + if (pid > 0) try { process.kill(pid, 'SIGKILL'); } catch {} + } +}); + +for (const [state, status, exitCode] of [['RE', 'complete', 0], ['UE', 'complete', 0], ['R', 'incomplete', 2], ['S', 'incomplete', 2], ['X', 'incomplete', 2], ['RX', 'incomplete', 2]] as const) { + test.skipIf(process.platform === 'win32')(`a refused macOS group kill reads a listed member in ps state ${state} as ${status}`, () => { + const f = fixture(); + const stub = refusedGroupKill(f, 'darwin'); + const bin = path.join(f.root, 'bin'); + fs.mkdirSync(bin); + fs.writeFileSync(path.join(bin, 'ps'), `#!/bin/sh\nprintf '%s ${state}\\n' "$(cat ${JSON.stringify(stub.group)})"\n`, { mode: 0o755 }); + const result = spawnSync(process.execPath, ['--preload', stub.preload, CLI, 'capture', f.root, '001', '--timeout-ms', '4000', '--', process.execPath, '-e', 'console.log("{}")'], + { cwd: f.root, encoding: 'utf8', timeout: 10_000, env: { ...process.env, PATH: `${bin}${path.delimiter}${process.env.PATH}` } }); + expect(fs.readFileSync(stub.refused, 'utf8')).toContain('SIGKILL'); + expect(result.status, result.stderr).toBe(exitCode); + expect(receipt(result.stdout)).toMatchObject({ status, exitCode }); + }); +}