From 03316a68f1591af4b6d3a766ea6fc080ab2984fe Mon Sep 17 00:00:00 2001 From: chihumyum Date: Thu, 8 Oct 2026 00:45:30 +0800 Subject: [PATCH 01/10] fix(runtime): stop shell commands when their Runtime Host dies Pipe-mode commands are spawned detached, so each leads its own process group and nothing stops it when the Host dies without running cleanup (SIGKILL, crash, OOM kill). The command is reparented and keeps writing to the workspace with no owner left to stop it or record its result. Start each bounded and background pipe command through a small owning supervisor (owned-process-main.js, launched with process.execPath, as Electron in Node mode under Desktop). The supervisor leads the process group, spawns the command into it with the Host's stdio and descriptor payloads inherited directly, and holds a private IPC channel to the Host as a lifetime lease; when the channel disconnects it SIGKILLs its tree. Host-initiated signals still reach the group once, and the command's exit code or signal is mirrored, including SIGPIPE and SIGUSR1, which Node would otherwise ignore or repurpose. Keep the shell run's reported pid the command's own, so `kill ` behaves as before; group signalling uses the supervisor. Require the supervisor entry in CLI release artifacts. Each running pipe command now costs one extra process: about 26 ms per command under Node and 75 ms under Electron on an idle Apple Silicon machine, and 13-15 MB of physical footprint while it runs. Generated-by: Codex Generated-by: Claude Code --- docs/windows-test-inventory.md | 7 +- .../src/__tests__/fixtures/shell-owner.ts | 48 ++++ .../src/__tests__/owned-child-process.test.ts | 207 ++++++++++++++++++ .../src/__tests__/pipe-process-driver.test.ts | 46 ++++ .../src/__tests__/shell-owner-death.test.ts | 90 ++++++++ .../src/__tests__/shell-run-manager.test.ts | 34 +++ packages/runtime/src/owned-child-process.ts | 160 ++++++++++++++ packages/runtime/src/owned-process-main.ts | 135 ++++++++++++ packages/runtime/src/pipe-process-driver.ts | 55 ++--- packages/runtime/src/shell-exec.ts | 26 +-- packages/runtime/src/shell-run-manager.ts | 3 +- scripts/release-cli-package.mjs | 2 + 12 files changed, 760 insertions(+), 53 deletions(-) create mode 100644 packages/runtime/src/__tests__/fixtures/shell-owner.ts create mode 100644 packages/runtime/src/__tests__/owned-child-process.test.ts create mode 100644 packages/runtime/src/__tests__/shell-owner-death.test.ts create mode 100644 packages/runtime/src/owned-child-process.ts create mode 100644 packages/runtime/src/owned-process-main.ts diff --git a/docs/windows-test-inventory.md b/docs/windows-test-inventory.md index e3327820ce2..e0e6d0e8f16 100644 --- a/docs/windows-test-inventory.md +++ b/docs/windows-test-inventory.md @@ -17,9 +17,9 @@ Locations intentionally omit line numbers so unrelated edits do not invalidate t |---|---:| | windows-backend-gap | 27 | | portable-candidate | 53 | -| platform-contract | 40 | +| platform-contract | 43 | -Total Windows-excluded declarations: **120** +Total Windows-excluded declarations: **123** ## Inventory @@ -87,6 +87,9 @@ Total Windows-excluded declarations: **120** | platform-contract | `packages/runtime/src/__tests__/filesystem-worker-smoke.test.ts` macOS filesystem worker smoke | `process.platform !== 'darwin'` | | platform-contract | `packages/runtime/src/__tests__/glob-search.test.ts` both Glob paths report permission failures and recover after permissions are restored | `process.platform === 'win32' \|\| process.getuid?.() === 0` | | portable-candidate | `packages/runtime/src/__tests__/node-pty-write-lifecycle.test.ts` does not carry queued Unix PTY writes past native exit | `process.platform === 'win32' ? 'Unix PTY file-descriptor lifecycle only' : false` | +| platform-contract | `packages/runtime/src/__tests__/owned-child-process.test.ts` POSIX cancellation delivers SIGTERM exactly once to the owned command | `process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false` | +| platform-contract | `packages/runtime/src/__tests__/owned-child-process.test.ts` supervisor mirrors a command that dies from ${signal} | `process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false` | +| platform-contract | `packages/runtime/src/__tests__/owned-child-process.test.ts` unexpected POSIX supervisor death terminates its command and cannot report success | `process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false` | | portable-candidate | `packages/runtime/src/__tests__/shell-exec.test.ts` writes a legacy WSL Bash command through stdin | `process.platform === 'win32' ? 'uses /bin/sh as a portable stdin probe' : false` | | platform-contract | `packages/runtime/src/__tests__/shell-exec.test.ts` bounds output drain after the root exits while a detached descendant retains stdout | `process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false` | | platform-contract | `packages/runtime/src/__tests__/shell-run-manager.test.ts` latches timeout when the root exits during POSIX process discovery | `process.platform === 'win32' ? 'POSIX process discovery only' : false` | diff --git a/packages/runtime/src/__tests__/fixtures/shell-owner.ts b/packages/runtime/src/__tests__/fixtures/shell-owner.ts new file mode 100644 index 00000000000..510f823d6ad --- /dev/null +++ b/packages/runtime/src/__tests__/fixtures/shell-owner.ts @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { runProcessWithBoundedTail } from '../../shell-exec.js'; +import { PipeProcessDriver } from '../../pipe-process-driver.js'; +const [mode, directory, script] = process.argv.slice(2) as [string, string, string]; +if (mode === 'bounded') { + const controller = new AbortController(); + process.on('message', () => controller.abort()); + const result = await runProcessWithBoundedTail(process.execPath, [script], { + cwd: directory, + timeoutMs: 30_000, + killGraceMs: 5000, + abortSignal: controller.signal, + }); + process.send?.(result); + process.disconnect?.(); +} else { + const driver = new PipeProcessDriver({ + plan: { file: process.execPath, args: [script], useShellOption: false }, + cwd: directory, + outputDrainMs: 100, + onData: () => {}, + onRootExit: () => {}, + onExit: () => {}, + onFailure: (error) => { + throw error; + }, + }); + process.on('message', () => driver.kill('SIGTERM')); + await driver.ready; +} diff --git a/packages/runtime/src/__tests__/owned-child-process.test.ts b/packages/runtime/src/__tests__/owned-child-process.test.ts new file mode 100644 index 00000000000..ae0f0928a92 --- /dev/null +++ b/packages/runtime/src/__tests__/owned-child-process.test.ts @@ -0,0 +1,207 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { openSync } from 'node:fs'; +import { mkdtemp, writeFile, readFile, rm } from 'node:fs/promises'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { setTimeout as delay } from 'node:timers/promises'; +import { runProcessWithBoundedTail } from '../shell-exec.js'; +import { spawnOwnedProcess } from '../owned-child-process.js'; + +test('owned command retains argv, environment, binary descriptor payloads and nonzero exit status', async () => { + const directory = await mkdtemp(join(tmpdir(), 'maka-owned-descriptors-')); + try { + const source = join(directory, 'payload.bin'); + await writeFile(source, new Uint8Array([0, 255, 13, 10, 128])); + const result = await runProcessWithBoundedTail( + process.execPath, + [ + '-e', + ` + const fs=require('node:fs'); + process.stdout.write(JSON.stringify({env:process.env.OWNED_TEST, arg:process.argv[1], + third:[...fs.readFileSync(3)], seventh:[...fs.readFileSync(7)]})); + process.stderr.write('stderr stays separate');process.exit(23); + `, + 'literal $HOME and \"quotes\"', + ], + { + cwd: directory, + timeoutMs: 5000, + env: { ...process.env, OWNED_TEST: 'forwarded' }, + fdInputs: [ + { fd: 3, data: new Uint8Array([7, 0, 255]) }, + { fd: 7, sourceFd: openSync(source, 'r') }, + ], + }, + ); + assert.equal(result.exitCode, 23); + assert.deepEqual(JSON.parse(result.stdout), { + env: 'forwarded', + arg: 'literal $HOME and \"quotes\"', + third: [7, 0, 255], + seventh: [0, 255, 13, 10, 128], + }); + assert.equal(result.stderr, 'stderr stays separate'); + } finally { + await rm(directory, { recursive: true, force: true }); + } +}); + +test('missing owned executable rejects with its actual spawn error', async () => { + await assert.rejects( + runProcessWithBoundedTail(join(tmpdir(), 'maka-missing-program-43972'), [], { + cwd: tmpdir(), + timeoutMs: 5000, + }), + { code: 'ENOENT' }, + ); +}); + +test('command NODE_OPTIONS applies once to the command, not to its supervisor', async () => { + const directory = await mkdtemp(join(tmpdir(), 'maka-owned-environment-')); + try { + const preload = join(directory, 'preload.cjs'); + await writeFile(preload, "process.stdout.write('PRELOAD\\n');"); + const result = await runProcessWithBoundedTail( + process.execPath, + ['-e', "process.stdout.write('COMMAND\\n')"], + { + cwd: directory, + timeoutMs: 5000, + env: { ...process.env, NODE_OPTIONS: `--require=${preload}` }, + }, + ); + assert.equal(result.exitCode, 0); + assert.equal(result.stdout, 'PRELOAD\nCOMMAND\n'); + } finally { + await rm(directory, { recursive: true, force: true }); + } +}); + +test('POSIX cancellation delivers SIGTERM exactly once to the owned command', { + skip: process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false, +}, async () => { + const abort = new AbortController(); + const result = await runProcessWithBoundedTail( + process.execPath, + [ + '-e', + ` + let count=0, timer; + process.on('SIGTERM',()=>{count++; timer??=setTimeout(()=>{ + console.log('signals='+count);process.exit(0); + },100);}); + console.log('ready');setInterval(()=>{},1000); + `, + ], + { + cwd: tmpdir(), + timeoutMs: 5000, + abortSignal: abort.signal, + emitOutput: (_stream, data) => { + if (data.includes('ready')) abort.abort(); + }, + }, + ); + assert.equal(result.aborted, true); + assert.equal(result.stdout, 'ready\nsignals=1\n'); +}); + +for (const signal of ['SIGPIPE', 'SIGUSR1'] as const) { + // Node ignores SIGPIPE and reserves SIGUSR1 for its inspector, so a supervisor + // that merely re-raised these would outlive its command. + test(`supervisor mirrors a command that dies from ${signal}`, { + skip: process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false, + timeout: 10_000, + }, async () => { + const { child, ready } = spawnOwnedProcess({ + program: '/bin/sh', + args: ['-c', `kill -${signal.slice(3)} $$`], + cwd: tmpdir(), + shell: false, + stdin: 'ignore', + }); + let stderr = ''; + child.stderr?.on('data', (chunk) => { + stderr += String(chunk); + }); + const exited = new Promise((resolve) => + child.once('exit', (code, exitSignal) => resolve({ code, signal: exitSignal })), + ); + await ready; + assert.deepEqual(await exited, { code: null, signal }); + assert.equal(stderr, ''); + }); +} + +test('unexpected POSIX supervisor death terminates its command and cannot report success', { + skip: process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false, + timeout: 10_000, +}, async () => { + const directory = await mkdtemp(join(tmpdir(), 'maka-owned-supervisor-death-')); + const marker = join(directory, 'started'); + const late = join(directory, 'late'); + const { child, ready } = spawnOwnedProcess({ + program: process.execPath, + args: [ + '-e', + `require('node:fs').writeFileSync(${JSON.stringify(marker)}, 'ready'); + setTimeout(() => require('node:fs').writeFileSync(${JSON.stringify(late)}, 'late'), 1000); + setInterval(() => {}, 1000);`, + ], + cwd: directory, + shell: false, + stdin: 'ignore', + }); + child.on('error', () => {}); + const exited = new Promise((resolve) => + child.once('exit', (code, signal) => resolve({ code, signal })), + ); + try { + await ready; + const deadline = Date.now() + 5000; + for (;;) { + try { + await readFile(marker); + break; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + assert.ok(Date.now() < deadline, 'command did not start'); + await delay(10); + } + } + child.kill('SIGKILL'); + assert.deepEqual(await exited, { code: null, signal: 'SIGKILL' }); + await delay(1200); + await assert.rejects(readFile(late), { code: 'ENOENT' }); + } finally { + if (child.pid) { + try { + process.kill(-child.pid, 'SIGKILL'); + } catch { + /* already exited */ + } + } + await rm(directory, { recursive: true, force: true }); + } +}); diff --git a/packages/runtime/src/__tests__/pipe-process-driver.test.ts b/packages/runtime/src/__tests__/pipe-process-driver.test.ts index 10acb549aa2..7baa426d02b 100644 --- a/packages/runtime/src/__tests__/pipe-process-driver.test.ts +++ b/packages/runtime/src/__tests__/pipe-process-driver.test.ts @@ -69,3 +69,49 @@ test('reports a partial stdin delivery failure before the child exit', async () driver.dispose(); } }); + +test('reports the command PID apart from its supervisor and lets a signal to it end the command', async () => { + let resolveExit!: (exit: PipeProcessExit) => void; + const exited = new Promise((resolve) => { + resolveExit = resolve; + }); + let resolveReported!: (pid: number) => void; + const reported = new Promise((resolve) => { + resolveReported = resolve; + }); + let stdout = ''; + const driver = new PipeProcessDriver({ + plan: { + file: process.execPath, + args: ['-e', "process.stdout.write(process.pid + '\\n'); setInterval(() => {}, 1000);"], + useShellOption: false, + }, + cwd: process.cwd(), + outputDrainMs: 1_000, + onData(stream, data) { + if (stream !== 'stdout') return; + stdout += data; + if (stdout.endsWith('\n')) resolveReported(Number.parseInt(stdout, 10)); + }, + onRootExit() {}, + onFailure(error) { + throw error; + }, + onExit: resolveExit, + }); + + try { + driver.writeInputs(); + await driver.ready; + const commandPid = await reported; + assert.equal(driver.commandPid, commandPid); + assert.notEqual(driver.commandPid, driver.pid); + // Signalling only the reported PID, as `kill ` would, reaches the command. + process.kill(commandPid, 'SIGTERM'); + const exit = await exited; + if (process.platform !== 'win32') assert.equal(exit.signal, 'SIGTERM'); + } finally { + driver.kill('SIGKILL'); + driver.dispose(); + } +}); diff --git a/packages/runtime/src/__tests__/shell-owner-death.test.ts b/packages/runtime/src/__tests__/shell-owner-death.test.ts new file mode 100644 index 00000000000..40cd9901576 --- /dev/null +++ b/packages/runtime/src/__tests__/shell-owner-death.test.ts @@ -0,0 +1,90 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { fork } from 'node:child_process'; +import { mkdtemp, readFile, writeFile, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { setTimeout as delay } from 'node:timers/promises'; + +for (const mode of ['bounded', 'pipes']) { + for (const beginStop of [false, true]) { + test(`${mode} (stop already requested: ${beginStop}): owner SIGKILL terminates an admitted command before its delayed write`, { + timeout: 15_000, + }, async () => { + const directory = await mkdtemp(join(tmpdir(), 'maka-owner-death-')); + const marker = join(directory, 'started.json'); + const late = join(directory, 'late.txt'); + const script = join(directory, 'writer.cjs'); + await writeFile( + script, + `const fs=require('node:fs');\nprocess.on('SIGTERM',()=>{});\nfs.writeFileSync(${JSON.stringify(marker)},JSON.stringify({pid:process.pid,ppid:process.ppid}));\nsetTimeout(()=>fs.writeFileSync(${JSON.stringify(late)},'late'),1500);\nsetInterval(()=>{},1000);`, + ); + const owner = fork( + new URL('./fixtures/shell-owner.js', import.meta.url), + [mode, directory, script], + { silent: true, execArgv: [] }, + ); + let command: { pid: number; ppid: number } | undefined; + let diagnostic = ''; + owner.stderr?.on('data', (chunk) => { + diagnostic += String(chunk); + }); + const exited = new Promise((resolve) => owner.once('exit', resolve)); + try { + const deadline = Date.now() + 8_000; + while (!command) { + try { + command = JSON.parse(await readFile(marker, 'utf8')); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + } + assert.ok(Date.now() < deadline, diagnostic || 'Command did not start'); + await delay(10); + } + if (beginStop) { + owner.send('stop'); + await delay(75); + } + assert.equal(owner.kill('SIGKILL'), true); + await exited; + await delay(1800); + await assert.rejects( + readFile(late, 'utf8'), + { code: 'ENOENT' }, + 'Command wrote after its owner was killed', + ); + } finally { + owner.kill('SIGKILL'); + if (command) { + for (const pid of [command.pid, ...(command.ppid !== owner.pid ? [command.ppid] : [])]) { + try { + process.kill(pid, 'SIGKILL'); + } catch { + /* already exited */ + } + } + } + await rm(directory, { recursive: true, force: true }); + } + }); + } +} diff --git a/packages/runtime/src/__tests__/shell-run-manager.test.ts b/packages/runtime/src/__tests__/shell-run-manager.test.ts index 312e3f4bcfb..6b1cfe8245f 100644 --- a/packages/runtime/src/__tests__/shell-run-manager.test.ts +++ b/packages/runtime/src/__tests__/shell-run-manager.test.ts @@ -431,6 +431,40 @@ describe('ShellRunProcessManager', () => { }); } + test('records the pipe command PID, not the supervisor that owns its process group', async () => { + const cwd = await workspace(); + const rootPidPath = join(cwd, 'root.pid'); + const manager = await createTestManager(); + try { + const initial = await manager.runBackgroundBash( + shellInput({ + cwd, + command: 'report the command process id', + argv: [ + process.execPath, + '-e', + `require('node:fs').writeFileSync(${JSON.stringify(rootPidPath)}, String(process.pid)); + setInterval(() => {}, 1000);`, + ], + timeoutMs: 30_000, + }), + ); + assertShellRun(initial); + await waitUntil(async () => { + try { + return (await readFile(rootPidPath, 'utf8')).length > 0; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return false; + throw error; + } + }); + // A model can act on this PID directly, e.g. `kill `. + assert.equal(initial.pid, Number.parseInt(await readFile(rootPidPath, 'utf8'), 10)); + } finally { + await manager.terminateAll().catch(() => undefined); + } + }); + test('hands off a long pipe command without output and publishes monotonic revisions', async () => { const updates: ShellRunUpdate[] = []; const store = sqliteShellRunStore(await workspace()); diff --git a/packages/runtime/src/owned-child-process.ts b/packages/runtime/src/owned-child-process.ts new file mode 100644 index 00000000000..42393e65634 --- /dev/null +++ b/packages/runtime/src/owned-child-process.ts @@ -0,0 +1,160 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { spawn, type ChildProcess } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; +import { buildSpawnStdio, type ChildFdInput } from './child-fd-input.js'; +import { terminateProcessTree } from './process-tree-terminator.js'; + +export interface OwnedProcessInput { + program: string; + args: readonly string[]; + cwd: string; + env?: NodeJS.ProcessEnv; + shell: boolean; + stdin: 'ignore' | 'pipe'; + fdInputs?: readonly ChildFdInput[]; +} + +export interface OwnedProcessLaunch { + kind: 'launch'; + program: string; + args: readonly string[]; + cwd: string; + env: NodeJS.ProcessEnv; + shell: boolean; + inheritedFds: number[]; +} + +type SupervisorMessage = + | { kind: 'started'; pid?: number } + | { kind: 'completed' } + | { kind: 'failed'; message: string; code?: string }; + +/** The returned PID owns the command group. Its private IPC channel is a + * lifetime lease: only the supervisor can spawn the command, and losing the + * owner tears down the group even when no Host cleanup callback can run. + * Output and descriptor payloads flow directly through inherited OS handles. + * `ready` resolves with the command's own PID once the supervisor admits it. + */ +export function spawnOwnedProcess(input: OwnedProcessInput): { + child: ChildProcess; + ready: Promise; +} { + const stdio = buildSpawnStdio(input.fdInputs, input.stdin); + const child = spawn( + process.execPath, + [fileURLToPath(new URL('./owned-process-main.js', import.meta.url))], + { + cwd: input.cwd, + // Caller-supplied environment belongs to the command, not this trusted supervisor. + env: { + ...process.env, + NODE_OPTIONS: '', + ...(process.versions.electron ? { ELECTRON_RUN_AS_NODE: '1' } : {}), + }, + stdio: [...stdio, 'ipc'], + // POSIX: the supervisor leads a new process group that the command joins. + // Termination signals the group and removes descendants visible outside + // it at each process-table snapshot. Windows has no process groups; + // taskkill /T owns the equivalent cleanup. + detached: process.platform !== 'win32', + windowsHide: true, + }, + ); + let started = false; + let completed = false; + let failureReported = false; + let resolveReady!: (commandPid: number | undefined) => void; + let rejectReady!: (error: Error) => void; + const ready = new Promise((resolve, reject) => { + resolveReady = resolve; + rejectReady = reject; + }); + // Some callers observe ChildProcess errors instead of awaiting admission. + void ready.catch(() => {}); + const timer = setTimeout(() => fail(new Error('Command supervisor startup timed out')), 10_000); + timer.unref(); + const terminate = () => { + if (child.pid) void terminateProcessTree({ pid: child.pid, signal: 'SIGKILL' }).catch(() => {}); + }; + function fail(error: Error): void { + if (failureReported) return; + failureReported = true; + clearTimeout(timer); + rejectReady(error); + terminate(); + child.emit('error', error); + } + child.once('error', (error) => { + clearTimeout(timer); + rejectReady(error); + }); + child.once('spawn', () => { + const request: OwnedProcessLaunch = { + kind: 'launch', + program: input.program, + args: input.args, + cwd: input.cwd, + env: input.env ?? process.env, + shell: input.shell, + inheritedFds: (input.fdInputs ?? []).map(({ fd }) => fd), + }; + child.send(request, (error) => { + if (error && child.exitCode === null && child.signalCode === null) fail(error); + }); + }); + child.on('message', (value) => { + const message = value as SupervisorMessage; + if (message.kind === 'started') { + started = true; + clearTimeout(timer); + resolveReady(message.pid); + } else if (message.kind === 'completed') { + completed = true; + } else if (message.kind === 'failed') { + completed = true; + const error = Object.assign(new Error(message.message), { code: message.code }); + fail(error); + } + }); + child.once('exit', (code) => { + clearTimeout(timer); + if (!started) rejectReady(new Error('Command supervisor exited before admission')); + if (!completed) { + // The group can outlive its leader. An unexpected supervisor exit must + // not leave an admitted command running while the Host is still alive. + terminate(); + if (code === 0) child.emit('error', new Error('Command supervisor lost its result')); + } + }); + return { child, ready }; +} + +/** A supervisor PID denotes the whole command group, including during startup. */ +export function signalOwnedProcess(child: ChildProcess, signal: 'SIGTERM' | 'SIGKILL'): boolean { + if (process.platform === 'win32' || !child.pid) return child.kill(signal); + try { + process.kill(-child.pid, signal); + return true; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ESRCH') return false; + throw error; + } +} diff --git a/packages/runtime/src/owned-process-main.ts b/packages/runtime/src/owned-process-main.ts new file mode 100644 index 00000000000..329c2515890 --- /dev/null +++ b/packages/runtime/src/owned-process-main.ts @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { spawn, type ChildProcess } from 'node:child_process'; +import { terminateProcessTree } from './process-tree-terminator.js'; +import type { OwnedProcessLaunch } from './owned-child-process.js'; + +let command: ChildProcess | undefined; +let stopping = false; +let completed = false; +let ownerLost = false; + +function loseOwner(): void { + if (completed || ownerLost) return; + ownerLost = true; + stopping = true; + // This process is the POSIX group leader; the command joins its group. The + // Windows implementation uses taskkill /T. Discovery also includes descendants + // that have moved into another process group while still attached to this tree. + void terminateProcessTree({ + pid: process.pid, + signal: 'SIGKILL', + fallback: () => process.exit(1), + }).then( + () => process.exit(1), + () => process.exit(1), + ); +} + +process.on('disconnect', loseOwner); +process.on('uncaughtException', loseOwner); +process.on('unhandledRejection', loseOwner); +for (const signal of ['SIGTERM', 'SIGINT'] as const) { + process.on(signal, () => { + stopping = true; + // On POSIX the Host signals our entire group, including the command. Do not + // forward the signal a second time. Before admission, stop without spawning. + if (!command) finish(null, signal); + }); +} + +function send(message: object, then?: () => void): void { + if (!process.connected) { + loseOwner(); + return; + } + process.send!(message, (error: Error | null) => { + if (error) loseOwner(); + else then?.(); + }); +} + +function finish(code: number | null, signal: NodeJS.Signals | null): void { + send({ kind: 'completed' }, () => { + completed = true; + if (signal) exitWithSignal(signal); + else process.exit(code ?? 1); + }); +} + +/** Mirror the command's terminating signal. Node ignores SIGPIPE and reserves + * SIGUSR1 for its inspector even without listeners; removing a listener resets + * the handler to the default action, so the re-raised signal ends this process. + */ +function exitWithSignal(signal: NodeJS.Signals): void { + process.removeAllListeners(signal); + try { + process.on(signal, () => {}); + process.removeAllListeners(signal); + } catch { + // SIGKILL and SIGSTOP accept no listener; their default action already applies. + } + process.kill(process.pid, signal); + // Never outlive the command if the signal did not end this process. + setTimeout(() => process.exit(1), 1_000); +} + +if (!process.connected) process.exit(1); +process.once('message', (message) => { + if (stopping || !process.connected) { + loseOwner(); + return; + } + const input = message as OwnedProcessLaunch; + if (input.kind !== 'launch') { + loseOwner(); + return; + } + const stdio: Array = [0, 1, 2]; + for (const fd of input.inheritedFds) { + while (stdio.length <= fd) stdio.push('ignore'); + stdio[fd] = fd; + } + try { + command = spawn(input.program, [...input.args], { + cwd: input.cwd, + env: input.env, + shell: input.shell, + stdio, + detached: false, + windowsHide: true, + }); + command.once('spawn', () => send({ kind: 'started', pid: command?.pid })); + command.once('error', failed); + command.once('exit', finish); + } catch (error) { + failed(error as Error); + } +}); + +function failed(error: Error): void { + send( + { kind: 'failed', message: error.message, code: (error as NodeJS.ErrnoException).code }, + () => { + completed = true; + process.exit(1); + }, + ); +} diff --git a/packages/runtime/src/pipe-process-driver.ts b/packages/runtime/src/pipe-process-driver.ts index cbe91d6d870..901d181a2ba 100644 --- a/packages/runtime/src/pipe-process-driver.ts +++ b/packages/runtime/src/pipe-process-driver.ts @@ -17,16 +17,12 @@ * under the License. */ -import { spawn, type ChildProcess } from 'node:child_process'; +import type { ChildProcess } from 'node:child_process'; import type { Readable, Writable } from 'node:stream'; +import { signalOwnedProcess, spawnOwnedProcess } from './owned-child-process.js'; import type { ShellSpawnPlan } from './shell-detect.js'; -import { - buildSpawnStdio, - closeChildFdSources, - writeChildFdInputs, - type ChildFdInput, -} from './child-fd-input.js'; +import { closeChildFdSources, writeChildFdInputs, type ChildFdInput } from './child-fd-input.js'; import { trackCapturedOutputDrain, type CapturedOutputDrain, @@ -55,6 +51,7 @@ export interface PipeProcessDriverOptions { } export class PipeProcessDriver { + /** Supervisor PID; it leads the command's process group, so signals target it. */ readonly pid: number | undefined; readonly ready: Promise; @@ -69,19 +66,24 @@ export class PipeProcessDriver { private rootExit: Omit | undefined; private stdinSettled: boolean; private stdinError: Error | undefined; + private admittedPid: number | undefined; constructor(private readonly options: PipeProcessDriverOptions) { try { - this.child = spawn(options.plan.file, options.plan.args, { + const spawned = spawnOwnedProcess({ + program: options.plan.file, + args: options.plan.args, cwd: options.cwd, env: options.env, shell: options.plan.useShellOption, - stdio: buildSpawnStdio( - options.fdInputs, - options.plan.stdin === undefined ? 'ignore' : 'pipe', - ), - detached: process.platform !== 'win32', + stdin: options.plan.stdin === undefined ? 'ignore' : 'pipe', + fdInputs: options.fdInputs, + }); + this.child = spawned.child; + this.ready = spawned.ready.then((pid) => { + this.admittedPid = pid; }); + void this.ready.catch(() => {}); } finally { closeChildFdSources(options.fdInputs); } @@ -117,7 +119,11 @@ export class PipeProcessDriver { this.child.on('exit', this.onRootExit); this.child.on('close', this.onCloseFallback); this.child.on('error', this.onError); - this.ready = waitForSpawn(this.child); + } + + /** The command's own PID, known once `ready` resolves; this is what users see. */ + get commandPid(): number | undefined { + return this.admittedPid; } writeInputs(): void { @@ -128,7 +134,7 @@ export class PipeProcessDriver { } kill(signal: 'SIGTERM' | 'SIGKILL'): boolean { - return this.child.kill(signal); + return signalOwnedProcess(this.child, signal); } dispose(): void { @@ -200,22 +206,3 @@ export class PipeProcessDriver { this.settleAfterDrain(); }; } - -function waitForSpawn(child: ChildProcess): Promise { - return new Promise((resolve, reject) => { - const onSpawn = () => { - cleanup(); - resolve(); - }; - const onError = (error: Error) => { - cleanup(); - reject(error); - }; - const cleanup = () => { - child.off('spawn', onSpawn); - child.off('error', onError); - }; - child.once('spawn', onSpawn); - child.once('error', onError); - }); -} diff --git a/packages/runtime/src/shell-exec.ts b/packages/runtime/src/shell-exec.ts index ea2fce39822..6b23d88c791 100644 --- a/packages/runtime/src/shell-exec.ts +++ b/packages/runtime/src/shell-exec.ts @@ -27,7 +27,8 @@ // the process cannot be spawned at all. Each caller maps those facts to its own // contract. -import { spawn } from 'node:child_process'; +import type { ChildProcess } from 'node:child_process'; +import { spawnOwnedProcess } from './owned-child-process.js'; import { buildShellSpawnPlan, defaultShellPlan, type ShellPlan } from './shell-detect.js'; import { BashTailBuffer } from './bash-tail-buffer.js'; import { DEFAULT_PROCESS_TERMINATION_GRACE_MS } from './process-tree-terminator.js'; @@ -36,12 +37,7 @@ import { manageChildProcessLifecycle, type ChildProcessLifecycleResult, } from './child-process-lifecycle.js'; -import { - buildSpawnStdio, - closeChildFdSources, - writeChildFdInputs, - type ChildFdInput, -} from './child-fd-input.js'; +import { closeChildFdSources, writeChildFdInputs, type ChildFdInput } from './child-fd-input.js'; // Per-stream cap on the output RETAINED for the result (~1MB). This only bounds // what is kept to return. The tool layer preserves this result for durable @@ -165,19 +161,17 @@ function runSpawnedProcessWithBoundedTail( }); } return new Promise((resolvePromise, reject) => { - let child: ReturnType; + let child: ChildProcess; try { - child = spawn(program, [...args], { + ({ child } = spawnOwnedProcess({ + program, + args, cwd: options.cwd, env: options.env, shell: useShellOption, - stdio: buildSpawnStdio(options.fdInputs, stdin === undefined ? 'ignore' : 'pipe'), - // POSIX: make the shell its own process-group leader (setsid). Termination - // signals the group and removes descendants visible outside it at each - // process-table snapshot. - // Windows has no process groups; taskkill /T owns the equivalent cleanup. - detached: process.platform !== 'win32', - }); + stdin: stdin === undefined ? 'ignore' : 'pipe', + fdInputs: options.fdInputs, + })); } finally { closeChildFdSources(options.fdInputs); } diff --git a/packages/runtime/src/shell-run-manager.ts b/packages/runtime/src/shell-run-manager.ts index 3128992b9e6..a0882fd9d7d 100644 --- a/packages/runtime/src/shell-run-manager.ts +++ b/packages/runtime/src/shell-run-manager.ts @@ -1015,7 +1015,8 @@ export class ShellRunProcessManager } private processPidPatch(live: LiveShellRun): Pick { - const pid = live.driver.pid; + // A pipe command runs under an owning supervisor; report the command itself. + const pid = live.mode === 'pipes' ? live.driver.commandPid : live.driver.pid; return pid !== undefined && Number.isSafeInteger(pid) && pid > 0 ? { pid } : {}; } diff --git a/scripts/release-cli-package.mjs b/scripts/release-cli-package.mjs index c9a43f2c2ae..1e728d67a1c 100644 --- a/scripts/release-cli-package.mjs +++ b/scripts/release-cli-package.mjs @@ -750,6 +750,7 @@ function validateStaging(publishable) { 'RUNTIME_HOST_PEER_DEPENDENCIES.rust.tsv', 'RUNTIME_HOST_PEER_THIRD_PARTY_NOTICES.txt', 'node_modules/@maka/runtime/dist/workers/filesystem-worker.js', + 'node_modules/@maka/runtime/dist/owned-process-main.js', 'node_modules/@maka/runtime-host/dist/execution-candidate-main.js', 'node_modules/@maka/eval/dist/harbor-external-subject.js', 'node_modules/@maka/eval/harbor/relay_agent.py', @@ -863,6 +864,7 @@ function validatePackedFiles(files, expectedDependencyManifests, publishable) { 'dist/cli.js', 'DISCLAIMER-WIP', 'node_modules/@maka/runtime/dist/workers/filesystem-worker.js', + 'node_modules/@maka/runtime/dist/owned-process-main.js', 'node_modules/@maka/runtime-host/dist/execution-candidate-main.js', 'node_modules/@maka/eval/harbor/relay_agent.py', ...(publishable || privateRuntimeHostTarget !== 'none' From f7c35ffbd6bdffdb74010110e0958fa50e42754c Mon Sep 17 00:00:00 2001 From: chihumyum Date: Thu, 8 Oct 2026 08:42:18 +0800 Subject: [PATCH 02/10] ci(windows): follow the win32 branch into owned-child-process The Windows recovery path filter must equal the win32-branching closure of the suites that lane runs. The platform branch for spawning pipe-mode commands moved from shell-exec.ts and pipe-process-driver.ts into owned-child-process.ts, so the filter now lists that module instead. --- .github/workflows/windows-recovery.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/windows-recovery.yml b/.github/workflows/windows-recovery.yml index 27f19c06777..3a44f50eeb4 100644 --- a/.github/workflows/windows-recovery.yml +++ b/.github/workflows/windows-recovery.yml @@ -101,7 +101,7 @@ on: - 'packages/runtime/src/filesystem-worker/client.ts' - 'packages/runtime/src/filesystem-worker/launch-spec.ts' - 'packages/runtime/src/filesystem-worker/process-runner.ts' - - 'packages/runtime/src/pipe-process-driver.ts' + - 'packages/runtime/src/owned-child-process.ts' - 'packages/runtime/src/process-tree-terminator.ts' - 'packages/runtime/src/pty-process-driver.ts' - 'packages/runtime/src/ripgrep-guidance.ts' @@ -112,7 +112,6 @@ on: - 'packages/runtime/src/sandbox/windows-profile.ts' - 'packages/runtime/src/sandbox/windows-sandbox.ts' - 'packages/runtime/src/shell-detect.ts' - - 'packages/runtime/src/shell-exec.ts' - 'packages/storage/src/__tests__/managed-dependency-environment.test.ts' - 'packages/storage/src/__tests__/root-authority.test.ts' - 'packages/storage/src/artifact-store.ts' From bab002a9e86a47182b0b997b7d07751e5bf3dd68 Mon Sep 17 00:00:00 2001 From: chihumyum Date: Thu, 8 Oct 2026 09:00:06 +0800 Subject: [PATCH 03/10] fix(runtime): hold supervised command output until admission A supervised command writes to the Host's pipes directly, while its admission arrives over the supervisor's IPC channel, so the Host could read output before `ready` resolved. A foreground Bash aborted on its first output then failed startup with an AbortError instead of returning a cancelled result, as seen on Linux CI. With a direct spawn the spawn event always preceded any output. Pause stdout and stderr until admission has been observed. Release them in the check phase after `ready` resolves, or when startup fails or the supervisor exits. The workspace executor abort test now aborts on the command's first output instead of after a fixed 100 ms, which also had to cover the supervisor's startup on a loaded runner. --- .../src/__tests__/owned-child-process.test.ts | 30 +++++++++++++++++++ .../src/__tests__/workspace-executor.test.ts | 9 ++++-- packages/runtime/src/owned-child-process.ts | 18 +++++++++++ 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/packages/runtime/src/__tests__/owned-child-process.test.ts b/packages/runtime/src/__tests__/owned-child-process.test.ts index ae0f0928a92..583cb700c9d 100644 --- a/packages/runtime/src/__tests__/owned-child-process.test.ts +++ b/packages/runtime/src/__tests__/owned-child-process.test.ts @@ -127,6 +127,36 @@ test('POSIX cancellation delivers SIGTERM exactly once to the owned command', { assert.equal(result.stdout, 'ready\nsignals=1\n'); }); +test('command output never reaches the caller before admission', async () => { + const { child, ready } = spawnOwnedProcess({ + program: process.execPath, + args: ['-e', "process.stdout.write('out'); process.stderr.write('err')"], + cwd: tmpdir(), + shell: false, + stdin: 'ignore', + }); + let admitted = false; + void ready.then(() => { + admitted = true; + }); + const seen: Array<{ stream: string; admitted: boolean }> = []; + child.stdout?.on('data', () => seen.push({ stream: 'stdout', admitted })); + child.stderr?.on('data', () => seen.push({ stream: 'stderr', admitted })); + // Output travels on inherited pipes and admission on the IPC channel. Keep + // the loop busy after the launch request is written, so both are pending + // when it resumes; without the hold, output is delivered first. + child.once('spawn', () => { + const until = Date.now() + 250; + while (Date.now() < until) {} + }); + await new Promise((resolve) => child.once('close', resolve)); + assert.deepEqual([...new Set(seen.map(({ stream }) => stream))].sort(), ['stderr', 'stdout']); + assert.ok( + seen.every((entry) => entry.admitted), + JSON.stringify(seen), + ); +}); + for (const signal of ['SIGPIPE', 'SIGUSR1'] as const) { // Node ignores SIGPIPE and reserves SIGUSR1 for its inspector, so a supervisor // that merely re-raised these would outlive its command. diff --git a/packages/runtime/src/__tests__/workspace-executor.test.ts b/packages/runtime/src/__tests__/workspace-executor.test.ts index 5b55ea7f5e3..19017bfeb26 100644 --- a/packages/runtime/src/__tests__/workspace-executor.test.ts +++ b/packages/runtime/src/__tests__/workspace-executor.test.ts @@ -132,14 +132,17 @@ describe('LocalWorkspaceExecutor exec', () => { const executor = new LocalWorkspaceExecutor(); const controller = new AbortController(); - const resultPromise = executor.exec({ + const result = await executor.exec({ command: 'printf "before-abort"; sleep 5; printf "after-abort"', cwd, timeoutMs: 5_000, abortSignal: controller.signal, + // Abort once the output exists rather than after a fixed delay that + // must also cover process startup on a loaded runner. + emitOutput: (stream, chunk) => { + if (stream === 'stdout' && chunk.includes('before-abort')) controller.abort(); + }, }); - setTimeout(() => controller.abort(), 100); - const result = await resultPromise; assert.strictEqual(result.exitCode, 130); assert.strictEqual(result.aborted, true); diff --git a/packages/runtime/src/owned-child-process.ts b/packages/runtime/src/owned-child-process.ts index 42393e65634..bcacfc466d7 100644 --- a/packages/runtime/src/owned-child-process.ts +++ b/packages/runtime/src/owned-child-process.ts @@ -94,17 +94,31 @@ export function spawnOwnedProcess(input: OwnedProcessInput): { const terminate = () => { if (child.pid) void terminateProcessTree({ pid: child.pid, signal: 'SIGKILL' }).catch(() => {}); }; + // Command output reaches these pipes directly, racing the IPC admission + // message. Hold it until callers have observed `ready`, so output never + // precedes admission, as with a direct spawn. Callers attach 'data' + // listeners synchronously; an explicit pause keeps those from resuming. + const outputs = [child.stdout, child.stderr].filter((stream) => stream !== null); + for (const stream of outputs) stream.pause(); + let outputReleased = false; + const releaseOutput = () => { + if (outputReleased) return; + outputReleased = true; + for (const stream of outputs) stream.resume(); + }; function fail(error: Error): void { if (failureReported) return; failureReported = true; clearTimeout(timer); rejectReady(error); + releaseOutput(); terminate(); child.emit('error', error); } child.once('error', (error) => { clearTimeout(timer); rejectReady(error); + releaseOutput(); }); child.once('spawn', () => { const request: OwnedProcessLaunch = { @@ -126,6 +140,9 @@ export function spawnOwnedProcess(input: OwnedProcessInput): { started = true; clearTimeout(timer); resolveReady(message.pid); + // Promise continuations of `ready` run before the check phase; a + // nextTick resume would deliver output ahead of them. + setImmediate(releaseOutput); } else if (message.kind === 'completed') { completed = true; } else if (message.kind === 'failed') { @@ -136,6 +153,7 @@ export function spawnOwnedProcess(input: OwnedProcessInput): { }); child.once('exit', (code) => { clearTimeout(timer); + releaseOutput(); if (!started) rejectReady(new Error('Command supervisor exited before admission')); if (!completed) { // The group can outlive its leader. An unexpected supervisor exit must From 2cad001bb94798f5baa85858182029548331f08d Mon Sep 17 00:00:00 2001 From: chihumyum Date: Thu, 8 Oct 2026 09:28:01 +0800 Subject: [PATCH 04/10] fix(runtime): harden the command supervisor's exit, signals and launch After the supervisor exits without reporting, its PID may be reaped and reused, so walking its tree could reach unrelated processes. Signal only its POSIX process group, which cannot be reallocated while members remain, and skip taskkill on Windows. Decide such an exit once the IPC channel has drained rather than on `exit`, and report a normal exit without a result as an integrity error instead of the command's status. The command shares the supervisor's process group, so a script that signals its own group could end the supervisor or start its inspector. Ignore HUP, USR1, USR2, QUIT and ALRM in the supervisor on POSIX. Spawn the command in the check phase after the launch message, so a group stop pending in the same turn is handled before the command starts. Tests cover a script trapping and sending group signals, a lost lease, and a setsid descendant of a live command on owner death. --- docs/windows-test-inventory.md | 6 +- .../src/__tests__/owned-child-process.test.ts | 74 +++++++++++++++++++ .../src/__tests__/shell-owner-death.test.ts | 61 +++++++++++++++ packages/runtime/src/owned-child-process.ts | 33 +++++++-- packages/runtime/src/owned-process-main.ts | 29 +++++++- 5 files changed, 193 insertions(+), 10 deletions(-) diff --git a/docs/windows-test-inventory.md b/docs/windows-test-inventory.md index e0e6d0e8f16..c540debf0ae 100644 --- a/docs/windows-test-inventory.md +++ b/docs/windows-test-inventory.md @@ -17,9 +17,9 @@ Locations intentionally omit line numbers so unrelated edits do not invalidate t |---|---:| | windows-backend-gap | 27 | | portable-candidate | 53 | -| platform-contract | 43 | +| platform-contract | 45 | -Total Windows-excluded declarations: **123** +Total Windows-excluded declarations: **125** ## Inventory @@ -89,9 +89,11 @@ Total Windows-excluded declarations: **123** | portable-candidate | `packages/runtime/src/__tests__/node-pty-write-lifecycle.test.ts` does not carry queued Unix PTY writes past native exit | `process.platform === 'win32' ? 'Unix PTY file-descriptor lifecycle only' : false` | | platform-contract | `packages/runtime/src/__tests__/owned-child-process.test.ts` POSIX cancellation delivers SIGTERM exactly once to the owned command | `process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false` | | platform-contract | `packages/runtime/src/__tests__/owned-child-process.test.ts` supervisor mirrors a command that dies from ${signal} | `process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false` | +| platform-contract | `packages/runtime/src/__tests__/owned-child-process.test.ts` group-wide signals from the command do not end its supervisor | `process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false` | | platform-contract | `packages/runtime/src/__tests__/owned-child-process.test.ts` unexpected POSIX supervisor death terminates its command and cannot report success | `process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false` | | portable-candidate | `packages/runtime/src/__tests__/shell-exec.test.ts` writes a legacy WSL Bash command through stdin | `process.platform === 'win32' ? 'uses /bin/sh as a portable stdin probe' : false` | | platform-contract | `packages/runtime/src/__tests__/shell-exec.test.ts` bounds output drain after the root exits while a detached descendant retains stdout | `process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false` | +| platform-contract | `packages/runtime/src/__tests__/shell-owner-death.test.ts` owner SIGKILL also terminates a descendant that left the command group with setsid | `process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false` | | platform-contract | `packages/runtime/src/__tests__/shell-run-manager.test.ts` latches timeout when the root exits during POSIX process discovery | `process.platform === 'win32' ? 'POSIX process discovery only' : false` | | platform-contract | `packages/runtime/src/__tests__/shell-run-manager.test.ts` preserves cancellation when timeout fires during POSIX process discovery | `process.platform === 'win32' ? 'POSIX process discovery only' : false` | | platform-contract | `packages/runtime/src/__tests__/shell-run-manager.test.ts` ignores a Stop abort that occurs after another admitted Stop commits termination | `process.platform === 'win32' ? 'Windows termination has no asynchronous POSIX snapshot window' : false` | diff --git a/packages/runtime/src/__tests__/owned-child-process.test.ts b/packages/runtime/src/__tests__/owned-child-process.test.ts index 583cb700c9d..4e566684bca 100644 --- a/packages/runtime/src/__tests__/owned-child-process.test.ts +++ b/packages/runtime/src/__tests__/owned-child-process.test.ts @@ -184,6 +184,80 @@ for (const signal of ['SIGPIPE', 'SIGUSR1'] as const) { }); } +test('a supervisor that loses its lease stops the command and never reports a command status', { + timeout: 10_000, +}, async () => { + const directory = await mkdtemp(join(tmpdir(), 'maka-owned-lost-lease-')); + const late = join(directory, 'late'); + const { child, ready } = spawnOwnedProcess({ + program: process.execPath, + args: [ + '-e', + `setTimeout(() => require('node:fs').writeFileSync(${JSON.stringify(late)}, 'late'), 1000); + setInterval(() => {}, 1000);`, + ], + cwd: directory, + shell: false, + stdin: 'ignore', + }); + const errors: Error[] = []; + child.on('error', (error) => errors.push(error)); + const exited = new Promise<{ code: number | null; signal: NodeJS.Signals | null }>((resolve) => + child.once('exit', (code, signal) => resolve({ code, signal })), + ); + try { + await ready; + // Losing the lease channel makes the supervisor stop its whole tree + // without reporting the command's own status. + child.disconnect(); + const exit = await exited; + await delay(50); + if (process.platform === 'win32') { + // taskkill ends the supervisor with an ordinary exit code, which must + // surface as a lost result rather than as the command's status. + assert.deepEqual( + errors.map((error) => error.message), + ['Command supervisor lost its result'], + ); + } else { + // The supervisor ends with its own group's SIGKILL, which the Host + // already reports as a forced termination. + assert.deepEqual(exit, { code: null, signal: 'SIGKILL' }); + assert.deepEqual(errors, []); + } + await delay(1200); + await assert.rejects(readFile(late), { code: 'ENOENT' }); + } finally { + if (child.pid && process.platform !== 'win32') { + try { + process.kill(-child.pid, 'SIGKILL'); + } catch { + /* already exited */ + } + } + await rm(directory, { recursive: true, force: true }); + } +}); + +test('group-wide signals from the command do not end its supervisor', { + skip: process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false, + timeout: 10_000, +}, async () => { + // A script may signal its own process group and trap the signal; the + // supervisor shares that group and must not die or start an inspector. + const result = await runProcessWithBoundedTail( + '/bin/sh', + [ + '-c', + "trap '' HUP USR1 USR2 QUIT ALRM; kill -HUP 0; kill -USR2 0; kill -QUIT 0; kill -ALRM 0; kill -USR1 0; sleep 0.3; echo survived", + ], + { cwd: tmpdir(), timeoutMs: 5000 }, + ); + assert.equal(result.exitCode, 0); + assert.equal(result.stdout, 'survived\n'); + assert.doesNotMatch(result.stderr, /Debugger listening/u); +}); + test('unexpected POSIX supervisor death terminates its command and cannot report success', { skip: process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false, timeout: 10_000, diff --git a/packages/runtime/src/__tests__/shell-owner-death.test.ts b/packages/runtime/src/__tests__/shell-owner-death.test.ts index 40cd9901576..739619c44ad 100644 --- a/packages/runtime/src/__tests__/shell-owner-death.test.ts +++ b/packages/runtime/src/__tests__/shell-owner-death.test.ts @@ -88,3 +88,64 @@ for (const mode of ['bounded', 'pipes']) { }); } } + +test('owner SIGKILL also terminates a descendant that left the command group with setsid', { + skip: process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false, + timeout: 15_000, +}, async () => { + const directory = await mkdtemp(join(tmpdir(), 'maka-owner-death-setsid-')); + const marker = join(directory, 'escaped.json'); + const late = join(directory, 'late.txt'); + const script = join(directory, 'escaper.cjs'); + const escapee = `const fs=require('node:fs');process.on('SIGTERM',()=>{});fs.writeFileSync(${JSON.stringify(marker)},JSON.stringify({pid:process.pid,pgid:process.pid}));setTimeout(()=>fs.writeFileSync(${JSON.stringify(late)},'late'),1500);setInterval(()=>{},1000);`; + // The command stays alive while its child leads a new session, outside the + // supervisor's process group but still inside its process tree. + await writeFile( + script, + `require('node:child_process').spawn(process.execPath,['-e',${JSON.stringify(escapee)}],{detached:true,stdio:'ignore'}).unref();\nsetInterval(()=>{},1000);`, + ); + const owner = fork( + new URL('./fixtures/shell-owner.js', import.meta.url), + ['bounded', directory, script], + { + silent: true, + execArgv: [], + }, + ); + let escaped: { pid: number } | undefined; + let diagnostic = ''; + owner.stderr?.on('data', (chunk) => { + diagnostic += String(chunk); + }); + const exited = new Promise((resolve) => owner.once('exit', resolve)); + try { + const deadline = Date.now() + 8_000; + while (!escaped) { + try { + escaped = JSON.parse(await readFile(marker, 'utf8')); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + } + assert.ok(Date.now() < deadline, diagnostic || 'Escaped descendant did not start'); + await delay(10); + } + assert.equal(owner.kill('SIGKILL'), true); + await exited; + await delay(1800); + await assert.rejects( + readFile(late, 'utf8'), + { code: 'ENOENT' }, + 'An escaped descendant wrote after its owner was killed', + ); + } finally { + owner.kill('SIGKILL'); + if (escaped) { + try { + process.kill(escaped.pid, 'SIGKILL'); + } catch { + /* already exited */ + } + } + await rm(directory, { recursive: true, force: true }); + } +}); diff --git a/packages/runtime/src/owned-child-process.ts b/packages/runtime/src/owned-child-process.ts index bcacfc466d7..c64577e22eb 100644 --- a/packages/runtime/src/owned-child-process.ts +++ b/packages/runtime/src/owned-child-process.ts @@ -94,6 +94,18 @@ export function spawnOwnedProcess(input: OwnedProcessInput): { const terminate = () => { if (child.pid) void terminateProcessTree({ pid: child.pid, signal: 'SIGKILL' }).catch(() => {}); }; + // Once the supervisor is reaped its PID can be reused, so a tree walk from it + // could reach unrelated processes. Its POSIX process group cannot be + // reallocated while members remain, so it alone still names the command. + // Windows has no such handle; taskkill /T on a reaped PID is skipped. + const terminateOrphanedGroup = () => { + if (process.platform === 'win32' || !child.pid) return; + try { + process.kill(-child.pid, 'SIGKILL'); + } catch { + // The group is already empty. + } + }; // Command output reaches these pipes directly, racing the IPC admission // message. Hold it until callers have observed `ready`, so output never // precedes admission, as with a direct spawn. Callers attach 'data' @@ -154,14 +166,23 @@ export function spawnOwnedProcess(input: OwnedProcessInput): { child.once('exit', (code) => { clearTimeout(timer); releaseOutput(); + // `exit` is not ordered after IPC messages still being read; the channel's + // close is. Decide a missing result only once the channel has drained. + if (completed || !child.connected) settleExit(code); + else child.once('disconnect', () => settleExit(code)); + }); + function settleExit(code: number | null): void { if (!started) rejectReady(new Error('Command supervisor exited before admission')); - if (!completed) { - // The group can outlive its leader. An unexpected supervisor exit must - // not leave an admitted command running while the Host is still alive. - terminate(); - if (code === 0) child.emit('error', new Error('Command supervisor lost its result')); + if (completed) return; + // The group can outlive its leader. An unexpected supervisor exit must + // not leave an admitted command running while the Host is still alive. + terminateOrphanedGroup(); + // A signal exit is the Host's own forced termination. A normal exit without + // `completed` lost the command's result and must not pass as its status. + if (code !== null && !failureReported) { + child.emit('error', new Error('Command supervisor lost its result')); } - }); + } return { child, ready }; } diff --git a/packages/runtime/src/owned-process-main.ts b/packages/runtime/src/owned-process-main.ts index 329c2515890..c97df289467 100644 --- a/packages/runtime/src/owned-process-main.ts +++ b/packages/runtime/src/owned-process-main.ts @@ -46,6 +46,15 @@ function loseOwner(): void { process.on('disconnect', loseOwner); process.on('uncaughtException', loseOwner); process.on('unhandledRejection', loseOwner); +// The command shares this process group, so a group-wide signal its own script +// sends (`kill -HUP 0`) reaches the supervisor too. Ignore the ones whose +// default action would end it, or for SIGUSR1 start the inspector, so the +// command alone decides how to handle them. Windows cannot listen for these. +if (process.platform !== 'win32') { + for (const signal of ['SIGHUP', 'SIGUSR1', 'SIGUSR2', 'SIGQUIT', 'SIGALRM'] as const) { + process.on(signal, () => {}); + } +} for (const signal of ['SIGTERM', 'SIGINT'] as const) { process.on(signal, () => { stopping = true; @@ -93,15 +102,31 @@ function exitWithSignal(signal: NodeJS.Signals): void { if (!process.connected) process.exit(1); process.once('message', (message) => { - if (stopping || !process.connected) { + if (!process.connected) { loseOwner(); return; } + // A stop that arrived before the launch is already finishing. + if (stopping) return; const input = message as OwnedProcessLaunch; if (input.kind !== 'launch') { loseOwner(); return; } + // A group signal can be pending in this loop turn alongside the launch. + // Spawn in the check phase, after its handler has run, so a stop that + // arrived first ends the supervisor without starting the command. + setImmediate(() => { + if (stopping) return; + if (!process.connected) { + loseOwner(); + return; + } + launch(input); + }); +}); + +function launch(input: OwnedProcessLaunch): void { const stdio: Array = [0, 1, 2]; for (const fd of input.inheritedFds) { while (stdio.length <= fd) stdio.push('ignore'); @@ -122,7 +147,7 @@ process.once('message', (message) => { } catch (error) { failed(error as Error); } -}); +} function failed(error: Error): void { send( From 92bd3bc177354e5da16dcfe16806eadac1f831d3 Mon Sep 17 00:00:00 2001 From: chihumyum Date: Thu, 8 Oct 2026 09:28:02 +0800 Subject: [PATCH 05/10] ci(windows): run the shell command ownership suites on Windows Every Windows Bash command now runs under the owning supervisor, but no Windows lane ran its suites. Add owned-child-process, shell-owner-death, pipe-process-driver and shell-exec to windows_recovery, and list their Windows-branching test sources in the lane's path filter. --- .github/workflows/windows-recovery.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.github/workflows/windows-recovery.yml b/.github/workflows/windows-recovery.yml index 3a44f50eeb4..e998df7bf2c 100644 --- a/.github/workflows/windows-recovery.yml +++ b/.github/workflows/windows-recovery.yml @@ -94,8 +94,12 @@ on: - 'packages/runtime-host/src/server/host-resource-probe.ts' - 'packages/runtime-host/src/server/skill-catalog-repository.ts' - 'packages/runtime-host/src/server/skill-catalog-transaction.ts' + - 'packages/runtime/src/__tests__/owned-child-process.test.ts' + - 'packages/runtime/src/__tests__/pipe-process-driver.test.ts' - 'packages/runtime/src/__tests__/runtime-continuation-crash.test.ts' - 'packages/runtime/src/__tests__/runtime-resume-crash.test.ts' + - 'packages/runtime/src/__tests__/shell-exec.test.ts' + - 'packages/runtime/src/__tests__/shell-owner-death.test.ts' - 'packages/runtime/src/builtin-tools.ts' - 'packages/runtime/src/file-stable-write.ts' - 'packages/runtime/src/filesystem-worker/client.ts' @@ -247,6 +251,17 @@ jobs: packages/runtime/dist/__tests__/runtime-resume-crash.test.js ` packages/runtime/dist/__tests__/runtime-continuation-crash.test.js + # Every Windows Bash command runs under the owning supervisor. Run its + # owner-death, lifecycle and bounded-shell suites on a real Windows host. + - name: Verify shell command ownership + shell: pwsh + run: | + node.exe --test --test-concurrency=1 ` + packages/runtime/dist/__tests__/owned-child-process.test.js ` + packages/runtime/dist/__tests__/shell-owner-death.test.js ` + packages/runtime/dist/__tests__/pipe-process-driver.test.js ` + packages/runtime/dist/__tests__/shell-exec.test.js + - name: Verify Runtime Host owner-death recovery shell: pwsh run: | From b587b7680e8cb27c235919e3cab1c9450b6db35b Mon Sep 17 00:00:00 2001 From: chihumyum Date: Thu, 8 Oct 2026 09:40:34 +0800 Subject: [PATCH 06/10] fix(runtime): detach the Windows supervisor and keep forced exits quiet The first Windows run showed two problems. On Windows the Host's forced stop is taskkill, which ends the supervisor with an ordinary exit code. Treating every non-zero exit without `completed` as a lost result turned timeouts and aborts into integrity errors. Only a clean exit without a result is reported now. libuv puts non-detached children in a job that is killed with their parent, but not their own children. A non-detached supervisor therefore died with the Host before it could stop anything, and a shell root's child survived, as on main. Detach the supervisor on every platform. It now outlives the Host long enough to stop its whole tree; the command stays in the supervisor's job. Add a `wrapped` owner-death case, a shell root running the script as its child, which fails on main. Skip the stop-requested variant on Windows, where a stop is an immediate forced kill. Accept EOF for a broken stdin pipe on Windows. Drop shell-exec from the Windows lane: its suite uses POSIX shell syntax throughout and fails on Windows without this change. --- .github/workflows/windows-recovery.yml | 7 +++---- docs/windows-test-inventory.md | 5 +++-- .../src/__tests__/fixtures/shell-owner.ts | 7 +++++++ .../src/__tests__/owned-child-process.test.ts | 19 +++++-------------- .../src/__tests__/pipe-process-driver.test.ts | 6 +++++- .../src/__tests__/shell-owner-death.test.ts | 12 +++++++++++- packages/runtime/src/owned-child-process.ts | 14 +++++++++----- 7 files changed, 43 insertions(+), 27 deletions(-) diff --git a/.github/workflows/windows-recovery.yml b/.github/workflows/windows-recovery.yml index e998df7bf2c..e038bec84f4 100644 --- a/.github/workflows/windows-recovery.yml +++ b/.github/workflows/windows-recovery.yml @@ -98,7 +98,6 @@ on: - 'packages/runtime/src/__tests__/pipe-process-driver.test.ts' - 'packages/runtime/src/__tests__/runtime-continuation-crash.test.ts' - 'packages/runtime/src/__tests__/runtime-resume-crash.test.ts' - - 'packages/runtime/src/__tests__/shell-exec.test.ts' - 'packages/runtime/src/__tests__/shell-owner-death.test.ts' - 'packages/runtime/src/builtin-tools.ts' - 'packages/runtime/src/file-stable-write.ts' @@ -252,15 +251,15 @@ jobs: packages/runtime/dist/__tests__/runtime-continuation-crash.test.js # Every Windows Bash command runs under the owning supervisor. Run its - # owner-death, lifecycle and bounded-shell suites on a real Windows host. + # owner-death and lifecycle suites on a real Windows host. shell-exec's + # suite is written in POSIX shell syntax and is not portable yet. - name: Verify shell command ownership shell: pwsh run: | node.exe --test --test-concurrency=1 ` packages/runtime/dist/__tests__/owned-child-process.test.js ` packages/runtime/dist/__tests__/shell-owner-death.test.js ` - packages/runtime/dist/__tests__/pipe-process-driver.test.js ` - packages/runtime/dist/__tests__/shell-exec.test.js + packages/runtime/dist/__tests__/pipe-process-driver.test.js - name: Verify Runtime Host owner-death recovery shell: pwsh diff --git a/docs/windows-test-inventory.md b/docs/windows-test-inventory.md index c540debf0ae..3fe2cdf1f90 100644 --- a/docs/windows-test-inventory.md +++ b/docs/windows-test-inventory.md @@ -17,9 +17,9 @@ Locations intentionally omit line numbers so unrelated edits do not invalidate t |---|---:| | windows-backend-gap | 27 | | portable-candidate | 53 | -| platform-contract | 45 | +| platform-contract | 46 | -Total Windows-excluded declarations: **125** +Total Windows-excluded declarations: **126** ## Inventory @@ -93,6 +93,7 @@ Total Windows-excluded declarations: **125** | platform-contract | `packages/runtime/src/__tests__/owned-child-process.test.ts` unexpected POSIX supervisor death terminates its command and cannot report success | `process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false` | | portable-candidate | `packages/runtime/src/__tests__/shell-exec.test.ts` writes a legacy WSL Bash command through stdin | `process.platform === 'win32' ? 'uses /bin/sh as a portable stdin probe' : false` | | platform-contract | `packages/runtime/src/__tests__/shell-exec.test.ts` bounds output drain after the root exits while a detached descendant retains stdout | `process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false` | +| platform-contract | `packages/runtime/src/__tests__/shell-owner-death.test.ts` ${mode} (stop already requested: ${beginStop}): owner SIGKILL terminates an admitted command before its delayed write | `beginStop && process.platform === 'win32' ? 'POSIX graceful SIGTERM window required' : false` | | platform-contract | `packages/runtime/src/__tests__/shell-owner-death.test.ts` owner SIGKILL also terminates a descendant that left the command group with setsid | `process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false` | | platform-contract | `packages/runtime/src/__tests__/shell-run-manager.test.ts` latches timeout when the root exits during POSIX process discovery | `process.platform === 'win32' ? 'POSIX process discovery only' : false` | | platform-contract | `packages/runtime/src/__tests__/shell-run-manager.test.ts` preserves cancellation when timeout fires during POSIX process discovery | `process.platform === 'win32' ? 'POSIX process discovery only' : false` | diff --git a/packages/runtime/src/__tests__/fixtures/shell-owner.ts b/packages/runtime/src/__tests__/fixtures/shell-owner.ts index 510f823d6ad..a5a6a3bd4e5 100644 --- a/packages/runtime/src/__tests__/fixtures/shell-owner.ts +++ b/packages/runtime/src/__tests__/fixtures/shell-owner.ts @@ -31,6 +31,13 @@ if (mode === 'bounded') { }); process.send?.(result); process.disconnect?.(); +} else if (mode === 'wrapped') { + // A shell root whose child runs the script, as `sh -c` or `cmd /c` commands do. + const [program, args]: [string, string[]] = + process.platform === 'win32' + ? [process.env.ComSpec ?? 'cmd.exe', ['/d', '/c', process.execPath, script]] + : ['/bin/sh', ['-c', '"$0" "$1"; exit $?', process.execPath, script]]; + await runProcessWithBoundedTail(program, args, { cwd: directory, timeoutMs: 30_000 }); } else { const driver = new PipeProcessDriver({ plan: { file: process.execPath, args: [script], useShellOption: false }, diff --git a/packages/runtime/src/__tests__/owned-child-process.test.ts b/packages/runtime/src/__tests__/owned-child-process.test.ts index 4e566684bca..3d58ca2733e 100644 --- a/packages/runtime/src/__tests__/owned-child-process.test.ts +++ b/packages/runtime/src/__tests__/owned-child-process.test.ts @@ -184,7 +184,7 @@ for (const signal of ['SIGPIPE', 'SIGUSR1'] as const) { }); } -test('a supervisor that loses its lease stops the command and never reports a command status', { +test('a supervisor that loses its lease stops the command', { timeout: 10_000, }, async () => { const directory = await mkdtemp(join(tmpdir(), 'maka-owned-lost-lease-')); @@ -212,19 +212,10 @@ test('a supervisor that loses its lease stops the command and never reports a co child.disconnect(); const exit = await exited; await delay(50); - if (process.platform === 'win32') { - // taskkill ends the supervisor with an ordinary exit code, which must - // surface as a lost result rather than as the command's status. - assert.deepEqual( - errors.map((error) => error.message), - ['Command supervisor lost its result'], - ); - } else { - // The supervisor ends with its own group's SIGKILL, which the Host - // already reports as a forced termination. - assert.deepEqual(exit, { code: null, signal: 'SIGKILL' }); - assert.deepEqual(errors, []); - } + // The supervisor stops its tree and ends by a forced kill (POSIX group + // SIGKILL, Windows taskkill), never with the command's own clean status. + assert.ok(exit.signal !== null || exit.code !== 0, JSON.stringify(exit)); + assert.deepEqual(errors, []); await delay(1200); await assert.rejects(readFile(late), { code: 'ENOENT' }); } finally { diff --git a/packages/runtime/src/__tests__/pipe-process-driver.test.ts b/packages/runtime/src/__tests__/pipe-process-driver.test.ts index 7baa426d02b..22aeb3a50dd 100644 --- a/packages/runtime/src/__tests__/pipe-process-driver.test.ts +++ b/packages/runtime/src/__tests__/pipe-process-driver.test.ts @@ -63,7 +63,11 @@ test('reports a partial stdin delivery failure before the child exit', async () const exit = await exited; assert.equal(exit.exitCode, 0); assert.equal(failures.length, 1); - assert.match(String((failures[0] as NodeJS.ErrnoException).code), /EPIPE|ERR_STREAM_DESTROYED/); + // Windows reports a pipe whose last reader exited as EOF. + assert.match( + String((failures[0] as NodeJS.ErrnoException).code), + /EPIPE|EOF|ERR_STREAM_DESTROYED/, + ); assert.deepEqual(events, ['failure', 'exit']); } finally { driver.dispose(); diff --git a/packages/runtime/src/__tests__/shell-owner-death.test.ts b/packages/runtime/src/__tests__/shell-owner-death.test.ts index 739619c44ad..4e8945c7863 100644 --- a/packages/runtime/src/__tests__/shell-owner-death.test.ts +++ b/packages/runtime/src/__tests__/shell-owner-death.test.ts @@ -25,9 +25,19 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { setTimeout as delay } from 'node:timers/promises'; -for (const mode of ['bounded', 'pipes']) { +// `wrapped` runs the script as the child of a shell root. Its child is outside +// the root's own libuv job on Windows, so only the supervisor's tree stop +// reaches it there. +for (const mode of ['bounded', 'pipes', 'wrapped']) { for (const beginStop of [false, true]) { + if (mode === 'wrapped' && beginStop) continue; test(`${mode} (stop already requested: ${beginStop}): owner SIGKILL terminates an admitted command before its delayed write`, { + // A Windows stop is an immediate forced kill, so there is no window in + // which a stopping command still runs when its owner dies. + skip: + beginStop && process.platform === 'win32' + ? 'POSIX graceful SIGTERM window required' + : false, timeout: 15_000, }, async () => { const directory = await mkdtemp(join(tmpdir(), 'maka-owner-death-')); diff --git a/packages/runtime/src/owned-child-process.ts b/packages/runtime/src/owned-child-process.ts index c64577e22eb..6a52021b317 100644 --- a/packages/runtime/src/owned-child-process.ts +++ b/packages/runtime/src/owned-child-process.ts @@ -73,8 +73,11 @@ export function spawnOwnedProcess(input: OwnedProcessInput): { // POSIX: the supervisor leads a new process group that the command joins. // Termination signals the group and removes descendants visible outside // it at each process-table snapshot. Windows has no process groups; - // taskkill /T owns the equivalent cleanup. - detached: process.platform !== 'win32', + // taskkill /T owns the equivalent cleanup. There, detaching also keeps + // the supervisor out of the Host's libuv job, which would kill it with + // the Host before it could stop the command's descendants; the command + // itself stays in the supervisor's job. + detached: true, windowsHide: true, }, ); @@ -177,9 +180,10 @@ export function spawnOwnedProcess(input: OwnedProcessInput): { // The group can outlive its leader. An unexpected supervisor exit must // not leave an admitted command running while the Host is still alive. terminateOrphanedGroup(); - // A signal exit is the Host's own forced termination. A normal exit without - // `completed` lost the command's result and must not pass as its status. - if (code !== null && !failureReported) { + // A clean exit without `completed` lost the command's result. Any other + // exit may be the Host's own forced termination, which on Windows + // (taskkill) also ends with an ordinary exit code. + if (code === 0 && !failureReported) { child.emit('error', new Error('Command supervisor lost its result')); } } From a2ff3fb22069e51408e514aaa9ba7d5c73a52164 Mon Sep 17 00:00:00 2001 From: chihumyum Date: Thu, 8 Oct 2026 11:24:11 +0800 Subject: [PATCH 07/10] fix(runtime): report supervisor faults and pin the Windows ownership lane A `failed` message means the supervisor is already exiting on its own, so a tree walk from its PID could start after it is reaped. Signal only its process group on that path, as after an exit. On POSIX the Host's own stops end the supervisor by signal, so an exit code without `completed` can only be a supervisor fault. Report it as a lost result there, rather than as the command exiting with that code. Windows keeps only a clean exit, since taskkill also ends with a code. The Windows ownership step now asserts its TAP counts, like the other gates in that job, so a widened POSIX-only skip cannot leave it green while it tests less. --- .github/workflows/windows-recovery.yml | 14 ++++++++++++-- packages/runtime/src/owned-child-process.ts | 19 ++++++++++++------- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/.github/workflows/windows-recovery.yml b/.github/workflows/windows-recovery.yml index e038bec84f4..ff9c98ee5e7 100644 --- a/.github/workflows/windows-recovery.yml +++ b/.github/workflows/windows-recovery.yml @@ -256,10 +256,20 @@ jobs: - name: Verify shell command ownership shell: pwsh run: | - node.exe --test --test-concurrency=1 ` + node.exe --test --test-reporter=tap --test-concurrency=1 ` packages/runtime/dist/__tests__/owned-child-process.test.js ` packages/runtime/dist/__tests__/shell-owner-death.test.js ` - packages/runtime/dist/__tests__/pipe-process-driver.test.js + packages/runtime/dist/__tests__/pipe-process-driver.test.js ` + 2>&1 | Tee-Object -FilePath "$env:RUNNER_TEMP/shell-command-ownership.tap" + $exitCode = $LASTEXITCODE + if ($exitCode -ne 0) { exit $exitCode } + $output = Get-Content "$env:RUNNER_TEMP/shell-command-ownership.tap" + # Most of these suites carry POSIX-only skips; pin how many still run + # here so a widened skip cannot leave the lane green while testing less. + if ($output -notcontains '# tests 18' -or $output -notcontains '# pass 10' -or $output -notcontains '# skipped 8') { + Write-Error 'Shell command ownership gate did not run exactly 10 passing Windows tests' + exit 1 + } - name: Verify Runtime Host owner-death recovery shell: pwsh diff --git a/packages/runtime/src/owned-child-process.ts b/packages/runtime/src/owned-child-process.ts index 6a52021b317..b65e1a1cb09 100644 --- a/packages/runtime/src/owned-child-process.ts +++ b/packages/runtime/src/owned-child-process.ts @@ -121,13 +121,16 @@ export function spawnOwnedProcess(input: OwnedProcessInput): { outputReleased = true; for (const stream of outputs) stream.resume(); }; - function fail(error: Error): void { + /** `supervisorExiting`: the supervisor reported and is exiting on its own, + * so its PID may be reaped before a tree walk could start. */ + function fail(error: Error, supervisorExiting = false): void { if (failureReported) return; failureReported = true; clearTimeout(timer); rejectReady(error); releaseOutput(); - terminate(); + if (supervisorExiting) terminateOrphanedGroup(); + else terminate(); child.emit('error', error); } child.once('error', (error) => { @@ -163,7 +166,7 @@ export function spawnOwnedProcess(input: OwnedProcessInput): { } else if (message.kind === 'failed') { completed = true; const error = Object.assign(new Error(message.message), { code: message.code }); - fail(error); + fail(error, true); } }); child.once('exit', (code) => { @@ -180,10 +183,12 @@ export function spawnOwnedProcess(input: OwnedProcessInput): { // The group can outlive its leader. An unexpected supervisor exit must // not leave an admitted command running while the Host is still alive. terminateOrphanedGroup(); - // A clean exit without `completed` lost the command's result. Any other - // exit may be the Host's own forced termination, which on Windows - // (taskkill) also ends with an ordinary exit code. - if (code === 0 && !failureReported) { + // A clean exit without `completed` lost the command's result. On POSIX the + // Host's own stops end the supervisor by signal, so any exit code there is + // a supervisor fault too. On Windows taskkill, the Host's forced stop, also + // ends with an ordinary code. + const lost = code === 0 || (code !== null && process.platform !== 'win32'); + if (lost && !failureReported) { child.emit('error', new Error('Command supervisor lost its result')); } } From 2d0c55019aa48d8f5281a6195468a58d855145f3 Mon Sep 17 00:00:00 2001 From: chihumyum Date: Thu, 8 Oct 2026 13:18:09 +0800 Subject: [PATCH 08/10] fix(runtime): report a supervisor fault before stopping its tree An uncaught exception or rejection in the supervisor stopped its tree with its own group's SIGKILL, so the Host saw a signal exit it could not tell from a forced stop, and reported it as the command being killed. While the lease is still up, report the fault as a failure first, then stop the tree. A one-second fallback stops it even if the report hangs. The test plays the Host against the real supervisor, injecting the fault through a preload rather than a product hook. The Windows ownership step now expects 19 tests, 11 passed, 8 skipped. --- .github/workflows/windows-recovery.yml | 4 +- .../src/__tests__/owned-child-process.test.ts | 62 +++++++++++++++++++ packages/runtime/src/owned-process-main.ts | 25 +++++++- 3 files changed, 87 insertions(+), 4 deletions(-) diff --git a/.github/workflows/windows-recovery.yml b/.github/workflows/windows-recovery.yml index ff9c98ee5e7..8f8c44cf460 100644 --- a/.github/workflows/windows-recovery.yml +++ b/.github/workflows/windows-recovery.yml @@ -266,8 +266,8 @@ jobs: $output = Get-Content "$env:RUNNER_TEMP/shell-command-ownership.tap" # Most of these suites carry POSIX-only skips; pin how many still run # here so a widened skip cannot leave the lane green while testing less. - if ($output -notcontains '# tests 18' -or $output -notcontains '# pass 10' -or $output -notcontains '# skipped 8') { - Write-Error 'Shell command ownership gate did not run exactly 10 passing Windows tests' + if ($output -notcontains '# tests 19' -or $output -notcontains '# pass 11' -or $output -notcontains '# skipped 8') { + Write-Error 'Shell command ownership gate did not run exactly 11 passing Windows tests' exit 1 } diff --git a/packages/runtime/src/__tests__/owned-child-process.test.ts b/packages/runtime/src/__tests__/owned-child-process.test.ts index 3d58ca2733e..6fd18e55fb2 100644 --- a/packages/runtime/src/__tests__/owned-child-process.test.ts +++ b/packages/runtime/src/__tests__/owned-child-process.test.ts @@ -19,11 +19,13 @@ import assert from 'node:assert/strict'; import { test } from 'node:test'; +import { fork } from 'node:child_process'; import { openSync } from 'node:fs'; import { mkdtemp, writeFile, readFile, rm } from 'node:fs/promises'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; import { setTimeout as delay } from 'node:timers/promises'; +import { fileURLToPath } from 'node:url'; import { runProcessWithBoundedTail } from '../shell-exec.js'; import { spawnOwnedProcess } from '../owned-child-process.js'; @@ -230,6 +232,66 @@ test('a supervisor that loses its lease stops the command', { } }); +test('a supervisor fault is reported as a failure before its tree is stopped', { + timeout: 10_000, +}, async () => { + const directory = await mkdtemp(join(tmpdir(), 'maka-owned-fault-')); + const late = join(directory, 'late'); + // Throw inside the real supervisor once it has admitted the command; the + // test plays the Host on the other end of its lease channel. + const preload = join(directory, 'fault.cjs'); + await writeFile( + preload, + `const send = process.send.bind(process); +process.send = (message, ...rest) => { + const result = send(message, ...rest); + if (message && message.kind === 'started') setImmediate(() => { throw new Error('injected fault'); }); + return result; +};`, + ); + const supervisor = fork(fileURLToPath(new URL('../owned-process-main.js', import.meta.url)), [], { + execArgv: ['--require', preload], + stdio: ['ignore', 'pipe', 'pipe', 'ipc'], + detached: true, + }); + const messages: Array<{ kind: string; message?: string }> = []; + supervisor.on('message', (message) => messages.push(message as (typeof messages)[number])); + const exited = new Promise((resolve) => supervisor.once('exit', resolve)); + try { + supervisor.send({ + kind: 'launch', + program: process.execPath, + args: [ + '-e', + `setTimeout(() => require('node:fs').writeFileSync(${JSON.stringify(late)}, 'late'), 1000); + setInterval(() => {}, 1000);`, + ], + cwd: directory, + env: process.env, + shell: false, + inheritedFds: [], + }); + await exited; + assert.deepEqual( + messages.map((message) => message.kind), + ['started', 'failed'], + ); + assert.equal(messages[1]?.message, 'Command supervisor failed: injected fault'); + await delay(1200); + await assert.rejects(readFile(late), { code: 'ENOENT' }); + } finally { + if (supervisor.pid && process.platform !== 'win32') { + try { + process.kill(-supervisor.pid, 'SIGKILL'); + } catch { + /* already exited */ + } + } + supervisor.kill('SIGKILL'); + await rm(directory, { recursive: true, force: true }); + } +}); + test('group-wide signals from the command do not end its supervisor', { skip: process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false, timeout: 10_000, diff --git a/packages/runtime/src/owned-process-main.ts b/packages/runtime/src/owned-process-main.ts index c97df289467..d1792f903bf 100644 --- a/packages/runtime/src/owned-process-main.ts +++ b/packages/runtime/src/owned-process-main.ts @@ -43,9 +43,30 @@ function loseOwner(): void { ); } +/** An unexpected supervisor error. Stopping the tree ends this process with + * its own group's SIGKILL, which the Host cannot tell from a forced stop, so + * report the fault first while the lease is still up. */ +function fault(error: unknown): void { + if (completed || ownerLost) return; + if (!process.connected) { + loseOwner(); + return; + } + // The tree stops whether or not the report goes through. + setTimeout(loseOwner, 1_000); + const message = error instanceof Error ? error.message : String(error); + try { + process.send!({ kind: 'failed', message: `Command supervisor failed: ${message}` }, () => + loseOwner(), + ); + } catch { + loseOwner(); + } +} + process.on('disconnect', loseOwner); -process.on('uncaughtException', loseOwner); -process.on('unhandledRejection', loseOwner); +process.on('uncaughtException', fault); +process.on('unhandledRejection', fault); // The command shares this process group, so a group-wide signal its own script // sends (`kill -HUP 0`) reaches the supervisor too. Ignore the ones whose // default action would end it, or for SIGUSR1 start the inspector, so the From 816dc0d007f3c5a5c705888c9c2177b48a7e8434 Mon Sep 17 00:00:00 2001 From: chihumyum Date: Thu, 8 Oct 2026 13:33:43 +0800 Subject: [PATCH 09/10] fix(runtime): start a bounded command's timeout at its admission The bounded runner started its timeout when it spawned the supervisor, so the supervisor's startup consumed part of the command's budget. On a loaded Linux runner a 100 ms timeout fired before the command had run at all, and the escalation test found no heartbeat file. With a direct spawn the budget belonged to the command, and the background runner already arms its timeout after admission. Start the timer once the supervisor admits the command. A supervisor that never admits it still fails through its own startup timeout. --- packages/runtime/src/shell-exec.ts | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/packages/runtime/src/shell-exec.ts b/packages/runtime/src/shell-exec.ts index 6b23d88c791..e3ff4d7cf2d 100644 --- a/packages/runtime/src/shell-exec.ts +++ b/packages/runtime/src/shell-exec.ts @@ -61,7 +61,11 @@ export const LIVE_OUTPUT_SUPPRESSED_MARKER = export interface BoundedShellOptions { cwd: string; - /** Hard wall-clock cap; the child is SIGTERM'd and `timedOut` is set. */ + /** + * Hard wall-clock cap on the command, counted from its admission so the + * owning supervisor's startup does not consume it; the child is SIGTERM'd + * and `timedOut` is set. + */ timeoutMs: number; /** Per-stream retained-tail cap in characters. Defaults to BASH_MAX_RETAINED_CHARS. */ maxRetainedChars?: number; @@ -162,8 +166,9 @@ function runSpawnedProcessWithBoundedTail( } return new Promise((resolvePromise, reject) => { let child: ChildProcess; + let admitted: Promise; try { - ({ child } = spawnOwnedProcess({ + ({ child, ready: admitted } = spawnOwnedProcess({ program, args, cwd: options.cwd, @@ -204,7 +209,17 @@ function runSpawnedProcessWithBoundedTail( ); void lifecycle.completion.then(resolveOnce, rejectOnce); - const timer = setTimeout(() => beginTermination({ timedOut: true }), options.timeoutMs); + // The budget is the command's, as with a direct spawn: start it once the + // supervisor has admitted the command. A supervisor that never admits it + // fails through its own startup timeout. + let timer: NodeJS.Timeout | undefined; + void admitted.then( + () => { + if (settled || termination) return; + timer = setTimeout(() => beginTermination({ timedOut: true }), options.timeoutMs); + }, + () => {}, + ); const abort = () => beginTermination({ aborted: true }); if (options.abortSignal) { if (options.abortSignal.aborted) abort(); From 0e242642f8325ceefd120c242e756cf9057ff5f8 Mon Sep 17 00:00:00 2001 From: chihumyum Date: Thu, 8 Oct 2026 15:18:55 +0800 Subject: [PATCH 10/10] fix(runtime): let a faulting supervisor finish its own tree stop Since a fault after admission is reported as `failed`, the Host's immediate group kill on that message hit the supervisor while its own tree walk was still reading the process table, so descendants that had left the group survived. On `failed` after admission the Host now only reports the error and leaves the stop to the supervisor. The group kill on its exit stays the backstop. A spawn failure before admission still signals the group at once. A fault while the `completed` report is in flight no longer sends `failed`, and the Host ignores a `failed` that follows `completed`, so a valid result is never turned into an error. Tests cover the Host leaving a reporting supervisor alive with the backstop still stopping the command, and the supervisor's walk reaching a setsid descendant on a fault. The Windows ownership step now expects 20 tests, 12 passed, 8 skipped. --- .github/workflows/windows-recovery.yml | 4 +- .../src/__tests__/owned-child-process.test.ts | 57 ++++++++++++++++++- packages/runtime/src/owned-child-process.ts | 29 +++++++--- packages/runtime/src/owned-process-main.ts | 8 +++ 4 files changed, 88 insertions(+), 10 deletions(-) diff --git a/.github/workflows/windows-recovery.yml b/.github/workflows/windows-recovery.yml index 8f8c44cf460..4e9fcb2abb8 100644 --- a/.github/workflows/windows-recovery.yml +++ b/.github/workflows/windows-recovery.yml @@ -266,8 +266,8 @@ jobs: $output = Get-Content "$env:RUNNER_TEMP/shell-command-ownership.tap" # Most of these suites carry POSIX-only skips; pin how many still run # here so a widened skip cannot leave the lane green while testing less. - if ($output -notcontains '# tests 19' -or $output -notcontains '# pass 11' -or $output -notcontains '# skipped 8') { - Write-Error 'Shell command ownership gate did not run exactly 11 passing Windows tests' + if ($output -notcontains '# tests 20' -or $output -notcontains '# pass 12' -or $output -notcontains '# skipped 8') { + Write-Error 'Shell command ownership gate did not run exactly 12 passing Windows tests' exit 1 } diff --git a/packages/runtime/src/__tests__/owned-child-process.test.ts b/packages/runtime/src/__tests__/owned-child-process.test.ts index 6fd18e55fb2..ae91f0c4f0c 100644 --- a/packages/runtime/src/__tests__/owned-child-process.test.ts +++ b/packages/runtime/src/__tests__/owned-child-process.test.ts @@ -237,6 +237,7 @@ test('a supervisor fault is reported as a failure before its tree is stopped', { }, async () => { const directory = await mkdtemp(join(tmpdir(), 'maka-owned-fault-')); const late = join(directory, 'late'); + const escapedLate = join(directory, 'escaped-late'); // Throw inside the real supervisor once it has admitted the command; the // test plays the Host on the other end of its lease channel. const preload = join(directory, 'fault.cjs'); @@ -263,7 +264,11 @@ process.send = (message, ...rest) => { program: process.execPath, args: [ '-e', - `setTimeout(() => require('node:fs').writeFileSync(${JSON.stringify(late)}, 'late'), 1000); + // The command also starts a descendant in its own session, outside the + // supervisor's process group: only the supervisor's tree walk reaches it. + `const { spawn } = require('node:child_process'); + spawn(process.execPath, ['-e', ${JSON.stringify(`setTimeout(() => require('node:fs').writeFileSync(${JSON.stringify(escapedLate)}, 'late'), 1000); setInterval(() => {}, 1000);`)}], { detached: true, stdio: 'ignore' }).unref(); + setTimeout(() => require('node:fs').writeFileSync(${JSON.stringify(late)}, 'late'), 1000); setInterval(() => {}, 1000);`, ], cwd: directory, @@ -279,6 +284,7 @@ process.send = (message, ...rest) => { assert.equal(messages[1]?.message, 'Command supervisor failed: injected fault'); await delay(1200); await assert.rejects(readFile(late), { code: 'ENOENT' }); + await assert.rejects(readFile(escapedLate), { code: 'ENOENT' }); } finally { if (supervisor.pid && process.platform !== 'win32') { try { @@ -292,6 +298,55 @@ process.send = (message, ...rest) => { } }); +test('a fault reported after admission leaves the stop to the supervisor, with a group kill as backstop', { + timeout: 10_000, +}, async () => { + const directory = await mkdtemp(join(tmpdir(), 'maka-owned-fault-host-')); + const late = join(directory, 'late'); + const { child, ready } = spawnOwnedProcess({ + program: process.execPath, + args: [ + '-e', + `setTimeout(() => require('node:fs').writeFileSync(${JSON.stringify(late)}, 'late'), 1000); + setInterval(() => {}, 1000);`, + ], + cwd: directory, + shell: false, + stdin: 'ignore', + }); + const errors: Error[] = []; + child.on('error', (error) => errors.push(error)); + const exited = new Promise((resolve) => child.once('exit', resolve)); + try { + await ready; + // What a faulting supervisor reports before walking its own tree. + child.emit('message', { kind: 'failed', message: 'Command supervisor failed: injected' }); + await delay(150); + assert.deepEqual( + errors.map((error) => error.message), + ['Command supervisor failed: injected'], + ); + assert.equal(child.exitCode, null, 'the Host does not kill a supervisor mid-walk'); + assert.equal(child.signalCode, null, 'the Host does not kill a supervisor mid-walk'); + // If the supervisor then dies without stopping its command, the Host's + // exit backstop still does. + child.kill('SIGKILL'); + await exited; + await delay(1200); + await assert.rejects(readFile(late), { code: 'ENOENT' }); + assert.equal(errors.length, 1, 'the fault is reported once'); + } finally { + if (child.pid && process.platform !== 'win32') { + try { + process.kill(-child.pid, 'SIGKILL'); + } catch { + /* already exited */ + } + } + await rm(directory, { recursive: true, force: true }); + } +}); + test('group-wide signals from the command do not end its supervisor', { skip: process.platform === 'win32' ? 'POSIX detached process-group semantics required' : false, timeout: 10_000, diff --git a/packages/runtime/src/owned-child-process.ts b/packages/runtime/src/owned-child-process.ts index b65e1a1cb09..bbc806bc5ac 100644 --- a/packages/runtime/src/owned-child-process.ts +++ b/packages/runtime/src/owned-child-process.ts @@ -121,16 +121,20 @@ export function spawnOwnedProcess(input: OwnedProcessInput): { outputReleased = true; for (const stream of outputs) stream.resume(); }; - /** `supervisorExiting`: the supervisor reported and is exiting on its own, - * so its PID may be reaped before a tree walk could start. */ - function fail(error: Error, supervisorExiting = false): void { + /** + * `stop` says who stops the tree: `tree` walks it from the live supervisor; + * `group` signals only its process group, because the supervisor is exiting + * on its own and its PID may be reaped before a walk could start; `none` + * leaves the stop to the supervisor, which walks its own tree. + */ + function fail(error: Error, stop: 'tree' | 'group' | 'none' = 'tree'): void { if (failureReported) return; failureReported = true; clearTimeout(timer); rejectReady(error); releaseOutput(); - if (supervisorExiting) terminateOrphanedGroup(); - else terminate(); + if (stop === 'tree') terminate(); + else if (stop === 'group') terminateOrphanedGroup(); child.emit('error', error); } child.once('error', (error) => { @@ -164,9 +168,20 @@ export function spawnOwnedProcess(input: OwnedProcessInput): { } else if (message.kind === 'completed') { completed = true; } else if (message.kind === 'failed') { - completed = true; + // A result already arrived; a later report cannot replace it. + if (completed) return; const error = Object.assign(new Error(message.message), { code: message.code }); - fail(error, true); + if (!started) { + // The command never spawned; the supervisor is exiting on its own. + completed = true; + fail(error, 'group'); + } else { + // A supervisor fault after admission: it stops its own tree, including + // descendants that left its group, before exiting. Signalling the group + // now would kill it mid-walk. The group kill on its exit stays the + // backstop, so `completed` remains unset. + fail(error, 'none'); + } } }); child.once('exit', (code) => { diff --git a/packages/runtime/src/owned-process-main.ts b/packages/runtime/src/owned-process-main.ts index d1792f903bf..6932d4efad1 100644 --- a/packages/runtime/src/owned-process-main.ts +++ b/packages/runtime/src/owned-process-main.ts @@ -25,6 +25,7 @@ let command: ChildProcess | undefined; let stopping = false; let completed = false; let ownerLost = false; +let finishing = false; function loseOwner(): void { if (completed || ownerLost) return; @@ -48,6 +49,12 @@ function loseOwner(): void { * report the fault first while the lease is still up. */ function fault(error: unknown): void { if (completed || ownerLost) return; + // The command has exited and its result is already on the way; a fault + // now must not turn it into a failure. Only make sure this process ends. + if (finishing) { + setTimeout(() => process.exit(1), 1_000); + return; + } if (!process.connected) { loseOwner(); return; @@ -97,6 +104,7 @@ function send(message: object, then?: () => void): void { } function finish(code: number | null, signal: NodeJS.Signals | null): void { + finishing = true; send({ kind: 'completed' }, () => { completed = true; if (signal) exitWithSignal(signal);