diff --git a/packages/core/src/mcp/mcp-runtime-runner.ts b/packages/core/src/mcp/mcp-runtime-runner.ts index 21765fc..fa682d7 100644 --- a/packages/core/src/mcp/mcp-runtime-runner.ts +++ b/packages/core/src/mcp/mcp-runtime-runner.ts @@ -249,16 +249,23 @@ export async function cancelManagedTree ( if (!(await waitFor(closed, deadline))) { return { confirmed: false, error: 'npm process did not exit after SIGKILL' } } + let probeError: string | undefined while (Date.now() < deadline) { try { process.kill(groupId, 0) - await sleep(25) // group still exists (e.g. a grandchild) — keep polling + probeError = undefined } catch (err) { - if ((err as NodeJS.ErrnoException).code === 'ESRCH') return { confirmed: true } - return { confirmed: false, error: `process group check failed (${(err as NodeJS.ErrnoException).code})` } + const code = (err as NodeJS.ErrnoException).code + if (code === 'ESRCH') return { confirmed: true } + probeError = `process group check failed (${code})` + // Darwin skips zombies when probing a group and can return EPERM + // until they are reaped. Keep waiting for ESRCH within the same + // deadline; EPERM alone never proves that cleanup is safe. + if (platform !== 'darwin' || code !== 'EPERM') return { confirmed: false, error: probeError } } + await sleep(25) // group still exists or its state is unconfirmed — keep polling } - return { confirmed: false, error: 'managed npm process group still exists after the confirmation deadline' } + return { confirmed: false, error: probeError ?? 'managed npm process group still exists after the confirmation deadline' } } catch (err) { return { confirmed: false, error: (err as Error).message } } diff --git a/packages/core/test/unit/mcp/mcp-runtime-runner.test.ts b/packages/core/test/unit/mcp/mcp-runtime-runner.test.ts index c0736e4..5df607d 100644 --- a/packages/core/test/unit/mcp/mcp-runtime-runner.test.ts +++ b/packages/core/test/unit/mcp/mcp-runtime-runner.test.ts @@ -45,6 +45,64 @@ function killTree (pid: number): void { } } +describe('cancelManagedTree (Unix termination confirmation)', () => { + it('waits for a transient Darwin EPERM to become ESRCH before confirming termination', async (t) => { + let probes = 0 + const calls: Array<[number, NodeJS.Signals | number]> = [] + t.mock.method(process, 'kill', (pid: number, signal: NodeJS.Signals | number) => { + calls.push([pid, signal]) + if (signal === 0) { + probes++ + throw Object.assign(new Error('group probe'), { code: probes === 1 ? 'EPERM' : 'ESRCH' }) + } + return true + }) + + const outcome = await cancelManagedTree({ pid: 1234 }, Promise.resolve(), { + platform: 'darwin', + confirmMs: 5_000, + }) + + assert.deepStrictEqual(outcome, { confirmed: true }) + assert.strictEqual(probes, 2, 'confirmation requires a later ESRCH, not EPERM alone') + assert.deepStrictEqual(calls, [[-1234, 'SIGTERM'], [-1234, 'SIGKILL'], [-1234, 0], [-1234, 0]]) + }) + + it('does not confirm termination when Darwin EPERM persists to the deadline', async (t) => { + t.mock.method(process, 'kill', (pid: number, signal: NodeJS.Signals | number) => { + assert.strictEqual(pid, -1234) + if (signal === 0) throw Object.assign(new Error('permission denied'), { code: 'EPERM' }) + return true + }) + const start = Date.now() + const outcome = await cancelManagedTree({ pid: 1234 }, Promise.resolve(), { + platform: 'darwin', + confirmMs: 50, + }) + + assert.strictEqual(outcome.confirmed, false) + assert.match(outcome.error ?? '', /process group check failed \(EPERM\)/) + assert.ok(Date.now() - start < 5_000, 'confirmation remains bounded') + }) + + for (const { platform, code } of [ + { platform: 'linux', code: 'EPERM' }, + { platform: 'darwin', code: 'EACCES' }, + ] as const) { + it(`does not reinterpret ${code} on ${platform} as confirmed termination`, async (t) => { + t.mock.method(process, 'kill', (_pid: number, signal: NodeJS.Signals | number) => { + if (signal === 0) throw Object.assign(new Error('probe failed'), { code }) + return true + }) + + const outcome = await cancelManagedTree({ pid: 1234 }, Promise.resolve(), { platform }) + + assert.strictEqual(outcome.confirmed, false) + assert.match(outcome.error ?? '', new RegExp(`process group check failed \\(${code}\\)`)) + }) + } +}) + describe('cancelManagedTree (Windows termination branch)', () => { it('returns a bounded terminationError when taskkill never exits', async () => { const killer = fakeKiller()