diff --git a/.github/workflows/windows-recovery.yml b/.github/workflows/windows-recovery.yml index 55969b7b940..ce46dd16036 100644 --- a/.github/workflows/windows-recovery.yml +++ b/.github/workflows/windows-recovery.yml @@ -94,14 +94,17 @@ 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-owner-death.test.ts' - 'packages/runtime/src/builtin-tools.ts' - 'packages/runtime/src/file-stable-write.ts' - '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 +115,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' @@ -249,6 +251,27 @@ 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 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-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 ` + 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 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 + } + - name: Verify Runtime Host owner-death recovery shell: pwsh run: | diff --git a/docs/windows-test-inventory.md b/docs/windows-test-inventory.md index e3327820ce2..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 | 40 | +| platform-contract | 46 | -Total Windows-excluded declarations: **120** +Total Windows-excluded declarations: **126** ## Inventory @@ -87,8 +87,14 @@ 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` 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` ${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` | | 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__/fixtures/shell-owner.ts b/packages/runtime/src/__tests__/fixtures/shell-owner.ts new file mode 100644 index 00000000000..a5a6a3bd4e5 --- /dev/null +++ b/packages/runtime/src/__tests__/fixtures/shell-owner.ts @@ -0,0 +1,55 @@ +/* + * 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 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 }, + 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..ae91f0c4f0c --- /dev/null +++ b/packages/runtime/src/__tests__/owned-child-process.test.ts @@ -0,0 +1,419 @@ +/* + * 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 { 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'; + +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'); +}); + +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. + 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('a supervisor that loses its lease stops the command', { + 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); + // 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 { + if (child.pid && process.platform !== 'win32') { + try { + process.kill(-child.pid, 'SIGKILL'); + } catch { + /* already exited */ + } + } + await rm(directory, { recursive: true, force: true }); + } +}); + +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'); + 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'); + 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', + // 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, + 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' }); + await assert.rejects(readFile(escapedLate), { 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('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, +}, 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, +}, 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..22aeb3a50dd 100644 --- a/packages/runtime/src/__tests__/pipe-process-driver.test.ts +++ b/packages/runtime/src/__tests__/pipe-process-driver.test.ts @@ -63,9 +63,59 @@ 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(); } }); + +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..4e8945c7863 --- /dev/null +++ b/packages/runtime/src/__tests__/shell-owner-death.test.ts @@ -0,0 +1,161 @@ +/* + * 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'; + +// `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-')); + 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 }); + } + }); + } +} + +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/__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/__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 new file mode 100644 index 00000000000..bbc806bc5ac --- /dev/null +++ b/packages/runtime/src/owned-child-process.ts @@ -0,0 +1,223 @@ +/* + * 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. 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, + }, + ); + 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(() => {}); + }; + // 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' + // 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(); + }; + /** + * `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 (stop === 'tree') terminate(); + else if (stop === 'group') terminateOrphanedGroup(); + child.emit('error', error); + } + child.once('error', (error) => { + clearTimeout(timer); + rejectReady(error); + releaseOutput(); + }); + 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); + // 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') { + // A result already arrived; a later report cannot replace it. + if (completed) return; + const error = Object.assign(new Error(message.message), { code: message.code }); + 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) => { + 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) 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 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')); + } + } + 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..6932d4efad1 --- /dev/null +++ b/packages/runtime/src/owned-process-main.ts @@ -0,0 +1,189 @@ +/* + * 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; +let finishing = 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), + ); +} + +/** 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; + // 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; + } + // 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', 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 +// 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; + // 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 { + finishing = true; + 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 (!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'); + 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..e3ff4d7cf2d 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 @@ -65,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; @@ -165,19 +165,18 @@ function runSpawnedProcessWithBoundedTail( }); } return new Promise((resolvePromise, reject) => { - let child: ReturnType; + let child: ChildProcess; + let admitted: Promise; try { - child = spawn(program, [...args], { + ({ child, ready: admitted } = 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); } @@ -210,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(); 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 e54ed659d3d..b7f6feab2f3 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.mjs', + '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.mjs', + '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'