From 61b8bd7125612d13b9cb03681bd6eb8483bd37e8 Mon Sep 17 00:00:00 2001 From: rUv Date: Sat, 26 Sep 2026 07:01:24 +0000 Subject: [PATCH] fix(npm): preserve failed native child completion (#18) --- docs/v2/adrs/ADR-207-npm-child-completion.md | 61 ++++++++++++++++ qudag-npm/bin/qudag.js | 8 ++- qudag-npm/src/index.ts | 7 +- qudag-npm/tests/process-status.test.ts | 75 ++++++++++++++++++++ 4 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 docs/v2/adrs/ADR-207-npm-child-completion.md create mode 100644 qudag-npm/tests/process-status.test.ts diff --git a/docs/v2/adrs/ADR-207-npm-child-completion.md b/docs/v2/adrs/ADR-207-npm-child-completion.md new file mode 100644 index 00000000..96f7fe33 --- /dev/null +++ b/docs/v2/adrs/ADR-207-npm-child-completion.md @@ -0,0 +1,61 @@ +# ADR 207: Preserve failed child completion in npm consumers + +Status: proposed, 2026-09-26. Related release qualification: issue #17. + +## Context + +Both the shipped CLI and programmatic `execute` wrapper map a native child's +missing numeric exit code to zero. Node reports a null exit code when a child +terminates from a signal. A cancelled or killed command can therefore appear +successful to shell automation and API callers. This is independent of the +existing release asset and full workspace qualification gaps. + +## Decision + +Preserve every numeric exit code unchanged. Map an absent numeric code to the +existing generic failure code 1. Wait for `close`, which follows process exit +and stdio closure, before resolving captured output. Keep spawn errors on the +existing failure/rejection path. No signal handler, release, workflow, native +binary, authorization, or dependency policy is changed. + +The API continues to return a numeric `code`. It does not claim to expose the +original signal identity or POSIX signal-derived exit numbers. This avoids an +API shape change and works with the package's existing Node >=16 support. + +## Authoritative source and applicability + +The current Node child-process contract, retrieved 2026-09-26, distinguishes +numeric exit codes from signal termination and documents that streams may +remain open at `exit`: https://nodejs.org/api/child_process.html#event-close +and https://nodejs.org/api/child_process.html#event-exit. + +This is a stable process contract, not a new algorithmic SOTA claim. Current +documentation also offers newer signal-conversion helpers; using a new runtime +API would break the declared Node support floor, so the compatible null check +is the smallest applicable repair. + +## Validation and limits + +From `qudag-npm`, install the committed dependencies without lifecycle scripts, +build, and compile the TypeScript regression: + +```sh +npm ci --ignore-scripts --no-audit --no-fund +npm run build +./node_modules/.bin/tsc --module commonjs --target es2020 --esModuleInterop --skipLibCheck --outDir test-dist tests/process-status.test.ts +node --test test-dist/process-status.test.js +``` + +The fixture copies the actual built package to a temporary directory and +substitutes only the native executable with a harmless local Node process. +It tests ordinary exits 0 and 7, SIGTERM/SIGINT/SIGKILL in both consumers, +complete 256 KiB stdout and stderr, and a non-executable native file. No live +network, production process, or credential is used. `QUDAG_TEST_PACKAGE` may +point at a separately installed packed package for final-consumer replay. + +The baseline passed 6 of 12 cases: all six signal cases falsely succeeded. +The acceptance threshold is all 12 cases passing in the source build and a +cleanly installed packed artifact, without modifying the test expectations. +These executable-fixture tests target POSIX systems; Windows and native binary +qualification remain separate. Passing this focused repair does not qualify +QuDAG for release or resolve the dependency audit and missing release assets. diff --git a/qudag-npm/bin/qudag.js b/qudag-npm/bin/qudag.js index 6826b997..56cb1225 100644 --- a/qudag-npm/bin/qudag.js +++ b/qudag-npm/bin/qudag.js @@ -37,8 +37,10 @@ async function main() { }); // Forward the exit code - child.on('exit', (code) => { - process.exit(code || 0); + child.on('close', (code) => { + // Node reports null after signal termination; absence of an exit code + // must never turn an interrupted native command into a successful CLI. + process.exit(code ?? 1); }); // Handle errors @@ -66,4 +68,4 @@ async function main() { main().catch((err) => { console.error('Unexpected error:', err); process.exit(1); -}); \ No newline at end of file +}); diff --git a/qudag-npm/src/index.ts b/qudag-npm/src/index.ts index 1120d0ad..95cd407d 100644 --- a/qudag-npm/src/index.ts +++ b/qudag-npm/src/index.ts @@ -43,9 +43,10 @@ export async function execute( child.on('error', reject); - child.on('exit', (code) => { + // Wait for captured streams to close as well as for the process to exit. + child.on('close', (code) => { resolve({ - code: code || 0, + code: code ?? 1, stdout: stdout.join(''), stderr: stderr.join('') }); @@ -162,4 +163,4 @@ export interface PlatformInfo { targetTriple: string; binaryName: string; binaryPath: string; -} \ No newline at end of file +} diff --git a/qudag-npm/tests/process-status.test.ts b/qudag-npm/tests/process-status.test.ts new file mode 100644 index 00000000..8cbde53c --- /dev/null +++ b/qudag-npm/tests/process-status.test.ts @@ -0,0 +1,75 @@ +import { test } from 'node:test'; +import * as assert from 'node:assert/strict'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { spawnSync } from 'node:child_process'; + +if (process.platform === 'win32') { + test.skip('POSIX executable and signal fixture; Windows requires native qualification'); +} else { +// Copy the real built consumer into isolation. Only the native executable is a +// synthetic fixture; no downloader, production process, or live network is used. +const source = process.env.QUDAG_TEST_PACKAGE || path.resolve(__dirname, '..'); +const fixture = fs.mkdtempSync(path.join(os.tmpdir(), 'qudag-process-status-')); +for (const entry of ['bin', 'dist', 'package.json']) { + fs.cpSync(path.join(source, entry), path.join(fixture, entry), { recursive: true }); +} +// npm may hoist a packed package's dependencies into its parent node_modules. +const dependencies = fs.existsSync(path.join(source, 'node_modules')) + ? path.join(source, 'node_modules') : path.dirname(source); +fs.symlinkSync(dependencies, path.join(fixture, 'node_modules'), 'junction'); +const binary = path.join(fixture, 'bin', 'platform', 'qudag'); +fs.mkdirSync(path.dirname(binary), { recursive: true }); +fs.writeFileSync(binary, `#!${process.execPath} +const [mode, value] = process.argv.slice(2); +if (mode === 'signal') process.kill(process.pid, value); +else if (mode === 'output') { + process.stdout.write('O'.repeat(262144)); + process.stderr.write('E'.repeat(262144)); + process.exitCode = 0; +} else process.exit(Number(value)); +`, { mode: 0o755 }); +const api = require(path.join(fixture, 'dist', 'index.js')); +process.on('exit', () => fs.rmSync(fixture, { recursive: true, force: true })); + +for (const code of [0, 7]) { + test(`CLI preserves ordinary exit ${code}`, () => { + const result = spawnSync(process.execPath, [path.join(fixture, 'bin', 'qudag.js'), 'exit', String(code)], { timeout: 5000 }); + assert.equal(result.error, undefined); + assert.equal(result.status, code); + }); + test(`API preserves ordinary exit ${code}`, async () => { + assert.equal((await api.execute(['exit', String(code)])).code, code); + }); +} + +for (const signal of ['SIGTERM', 'SIGINT', 'SIGKILL']) { + test(`CLI fails on ${signal}`, () => { + const result = spawnSync(process.execPath, [path.join(fixture, 'bin', 'qudag.js'), 'signal', signal], { timeout: 5000 }); + assert.equal(result.error, undefined); + assert.ok(result.status !== null && result.status > 0, JSON.stringify(result)); + }); + test(`API fails on ${signal}`, async () => { + assert.ok((await api.execute(['signal', signal])).code > 0); + }); +} + +test('API collects complete stdout and stderr', async () => { + const result = await api.execute(['output']); + assert.equal(result.code, 0); + assert.equal(result.stdout, 'O'.repeat(262144)); + assert.equal(result.stderr, 'E'.repeat(262144)); +}); + +test('CLI and API fail when the installed file is not executable', async () => { + fs.chmodSync(binary, 0o644); + try { + const result = spawnSync(process.execPath, [path.join(fixture, 'bin', 'qudag.js')], { timeout: 5000 }); + assert.equal(result.status, 1); + await assert.rejects(api.execute([]), { code: 'EACCES' }); + } finally { + fs.chmodSync(binary, 0o755); + } +}); +}