From 9c7795e5ddb908d7a87d6dd929a559b2a49360a5 Mon Sep 17 00:00:00 2001 From: thossullivan Date: Tue, 11 Aug 2026 09:05:36 -0500 Subject: [PATCH] fix: drain stdio then hard-exit - piped JSON truncated at 64KiB check.mjs called process.exit() straight after console.log(JSON.stringify(...)), and Node discards unflushed pipe writes on exit, so any --json output past the 64KiB pipe buffer was silently truncated (repro: check on a large repo | jq). File redirection was unaffected, which is why CI logs never showed it. The CLI body now runs in main() returning the exit code; top level drains stdout and stderr through an error-capturing zero-byte write, then hard process.exit()s. Three review rounds shaped this: bare process.exitCode waited on the whole event loop (stray timer hangs the CLI, beforeExit rewrites the code), and a naive drain discarded stream errors (EPIPE truncation exited 0; a dead stderr plus a usage error crashed 1 instead of 2). Exit codes: 0/1/2 unchanged; new exit 3 when output fails on a clean run for any reason other than EPIPE, which stays 0 (consumer closed the pipe). Regression tests cover piped >64KiB stdout and stderr, setInterval preload, beforeExit preload, early-closed stdout, ended stderr with a usage error, and callback-level EBADF. Also converts the two badge tests from live feed dates, which rotted when claude-opus-4-1 retired on 2026-08-05, to synthetic fixtures. Implemented by Codex (gpt-5.6-luna) over three review rounds; verified locally. --- check.mjs | 104 +++++++++++++++++++++++++++++------------ test/run.mjs | 129 +++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 198 insertions(+), 35 deletions(-) diff --git a/check.mjs b/check.mjs index abbd55a..14d8d19 100755 --- a/check.mjs +++ b/check.mjs @@ -34,6 +34,7 @@ import { } from './lib/reports.mjs' import { addedLinesForTargets, filterFindingsToChanged, incompleteScanNotes, scanTargets } from './lib/scanner.mjs' +const main = () => { const COMMANDS = new Set(['check', 'inventory', 'schedule', 'alert', 'plan', 'apply', 'help']) const args = process.argv.slice(2) @@ -62,7 +63,7 @@ try { }) } catch (error) { console.error(error.message) - process.exit(2) + return 2 } const { values, positionals } = parsed @@ -107,43 +108,49 @@ Commands: Scopes: all Check every tracked model ID found. This preserves the original behavior. direct Check direct API and generic model references; leave cloud/gateway refs in inventory. + +Exit codes: + 0 Clean, or report generated successfully. + 1 Findings, alert errors, or refused apply items. + 2 Usage, feed, scan, or plan errors. + 3 Output stream failure other than EPIPE. `) - process.exit(0) + return 0 } if (command === 'apply') { if (!PLAN_FILE || positionals.length) { console.error('apply requires --plan plan.json and accepts no target paths') - process.exit(2) + return 2 } try { const result = applyPlan({ planPath: PLAN_FILE, dryRun: DRY_RUN, rootDir: process.cwd() }) - process.exit(result.failed ? 1 : 0) + return result.failed ? 1 : 0 } catch (e) { console.error(`failed to apply plan ${PLAN_FILE}: ${e.message}`) - process.exit(2) + return 2 } } if (!Number.isFinite(DAYS) || !Number.isInteger(DAYS) || DAYS < 0) { console.error('--days must be a finite non-negative integer') - process.exit(2) + return 2 } if (!['all', 'direct'].includes(SCOPE)) { console.error('--scope must be all or direct') - process.exit(2) + return 2 } if (CHANGED_BASE !== null && command !== 'check') { console.error('--changed is only supported by the check command') - process.exit(2) + return 2 } if (command === 'alert' && FORMAT !== null && !['github', 'markdown', 'badge', 'json'].includes(FORMAT)) { console.error('--format must be github, markdown, badge, or json for alert') - process.exit(2) + return 2 } if (command === 'inventory' && FORMAT !== null && !['json', 'cyclonedx', 'text'].includes(FORMAT)) { console.error('--format must be json, cyclonedx, or text for inventory') - process.exit(2) + return 2 } let feedData @@ -151,11 +158,11 @@ try { feedData = loadFeeds(FEEDS_DIR) } catch (e) { console.error(`failed to load feeds from ${FEEDS_DIR}: ${e.message}`) - process.exit(2) + return 2 } if (feedData.entries.size === 0) { console.error(`no feed entries loaded from ${FEEDS_DIR}`) - process.exit(2) + return 2 } let scan @@ -168,7 +175,7 @@ try { }) } catch (e) { console.error(`scan failed: ${e.message}`) - process.exit(2) + return 2 } const incompleteNotes = incompleteScanNotes(scan.notes) @@ -179,23 +186,22 @@ if (incompleteNotes.length && (command === 'check' || command === 'plan')) { const location = note.file ? ` (${note.file})` : '' console.error(` ${note.reason}${location}${note.message ? `: ${note.message}` : ''}`) } - if (!ALLOW_INCOMPLETE) process.exit(2) + if (!ALLOW_INCOMPLETE) return 2 } const findings = scan.modelRefs.map(ref => findingFromRef(ref, { days: DAYS, via: VIA })) const checkFindings = SCOPE === 'direct' ? findings.filter(f => f.usage === 'direct-api' || f.usage === 'model-reference') : findings -const changedFindings = CHANGED_BASE === null - ? checkFindings - : (() => { - try { - return filterFindingsToChanged(checkFindings, addedLinesForTargets(targets, CHANGED_BASE)) - } catch (e) { - console.error(`--changed failed: ${e.message}`) - process.exit(2) - } - })() +let changedFindings = checkFindings +if (CHANGED_BASE !== null) { + try { + changedFindings = filterFindingsToChanged(checkFindings, addedLinesForTargets(targets, CHANGED_BASE)) + } catch (e) { + console.error(`--changed failed: ${e.message}`) + return 2 + } +} const bad = changedFindings.filter(isBad) const inventory = () => buildInventory({ scan, findings, days: DAYS, via: VIA, scope: SCOPE, targets }) const schedule = () => buildSchedule(inventory()) @@ -210,7 +216,7 @@ if (command === 'plan') { via: VIA, scope: SCOPE, }), null, 2)) - process.exit(0) + return 0 } if (command === 'check') { @@ -219,7 +225,7 @@ if (command === 'check') { } else { console.log(formatCheck({ findings: changedFindings, bad, scannedFiles: scan.files.length, days: DAYS, scope: SCOPE })) } - process.exit(bad.length ? 1 : 0) + return bad.length ? 1 : 0 } if (command === 'inventory') { @@ -232,7 +238,7 @@ if (command === 'inventory') { } else { console.log(formatInventory(inv, DAYS)) } - process.exit(0) + return 0 } if (command === 'schedule') { @@ -242,7 +248,7 @@ if (command === 'schedule') { } else { console.log(formatSchedule(sched, DAYS)) } - process.exit(0) + return 0 } if (command === 'alert') { @@ -256,8 +262,46 @@ if (command === 'alert') { } else { console.log(formatAlertGithub(payload, DAYS)) } - process.exit(payload.errors.length ? 1 : 0) + return payload.errors.length ? 1 : 0 } console.error(`unknown command: ${command}`) -process.exit(2) +return 2 +} + +const writeAndCapture = (stream, chunk) => new Promise(resolve => { + let error = null + let settling = false + const settle = nextError => { + if (nextError && !error) error = nextError + if (settling) return + settling = true + setImmediate(() => { + stream.off('error', onError) + resolve(error) + }) + } + const onError = nextError => settle(nextError) + stream.on('error', onError) + try { + stream.write(chunk, settle) + } catch (writeError) { + settle(writeError) + } +}) +const drainStream = stream => writeAndCapture(stream, '') + +const exitCode = main() +const [stdoutError, stderrError] = await Promise.all([ + drainStream(process.stdout), + drainStream(process.stderr), +]) +const outputError = [stdoutError, stderrError].find(error => error && error.code !== 'EPIPE') +let finalExitCode = exitCode +if (exitCode === 0 && outputError) { + finalExitCode = 3 + const diagnosticStream = stderrError ? (stdoutError ? null : process.stdout) : process.stderr + const detail = String(outputError.code ?? outputError.message ?? 'unknown error').replace(/[\r\n]+/g, ' ') + if (diagnosticStream) await writeAndCapture(diagnosticStream, `model-eol: output failed: ${detail}\n`) +} +process.exit(finalExitCode) diff --git a/test/run.mjs b/test/run.mjs index eca0913..b522afc 100644 --- a/test/run.mjs +++ b/test/run.mjs @@ -3,7 +3,7 @@ // o3-deep-research's shutdown (2026-07-23) is in the past forever, and // claude-opus-4-1's (2026-08-05) is either retiring or retired - both flag. import crypto from 'node:crypto' -import { spawnSync } from 'node:child_process' +import { spawn, spawnSync } from 'node:child_process' import fs from 'node:fs' import os from 'node:os' import path from 'node:path' @@ -18,6 +18,35 @@ const run = (args, options = {}) => { const result = spawnSync('node', [path.join(root, 'check.mjs'), ...args], { encoding: 'utf8', ...options }) return { out: result.stdout ?? '', err: result.stderr ?? '', code: result.status } } +const runPiped = (args, { closeStdoutAfterData = false, env = process.env, timeout = 5000 } = {}) => new Promise((resolve, reject) => { + const child = spawn('node', [path.join(root, 'check.mjs'), ...args], { env, stdio: ['ignore', 'pipe', 'pipe'] }) + let out = '' + let err = '' + let timedOut = false + let stdoutClosed = false + const timer = setTimeout(() => { + timedOut = true + child.kill('SIGKILL') + }, timeout) + child.stdout.setEncoding('utf8') + child.stderr.setEncoding('utf8') + child.stdout.on('data', chunk => { + out += chunk + if (closeStdoutAfterData && !stdoutClosed) { + stdoutClosed = true + child.stdout.destroy() + } + }) + child.stderr.on('data', chunk => { err += chunk }) + child.once('error', error => { + clearTimeout(timer) + reject(error) + }) + child.once('close', code => { + clearTimeout(timer) + resolve({ out, err, code, timedOut }) + }) +}) let failures = 0 const assert = (cond, msg) => { @@ -29,6 +58,9 @@ const assert = (cond, msg) => { } } +const help = run(['--help']) +assert(help.code === 0 && help.out.includes('3 Output stream failure other than EPIPE.'), 'help documents exit 3 for non-EPIPE output failures') + const unknownFlag = run([path.join(root, 'test/fixture'), '--dyas', '90']) assert(unknownFlag.code === 2 && unknownFlag.err.includes('--dyas') && unknownFlag.err.includes('--help'), 'unknown check flags exit 2 with the bad flag and help hint') @@ -176,13 +208,100 @@ assert(property(cyclonedxComponent, 'model-eol:status') === 'retired', 'CycloneD assert(cyclonedxComponent?.evidence?.occurrences.some(item => item.location.endsWith('direct.py#8')), 'CycloneDX carries model reference occurrences') assert(!cyclonedx.components.some(item => item.name === 'gpt-9-ultra-20990101'), 'CycloneDX omits candidate model references') -const badgeRun = run(['alert', path.join(root, 'test/fixture'), '--days', '30', '--format', 'badge']) +const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'model-eol-test-')) + +const badgeDir = path.join(tempRoot, 'badge') +const badgeFeeds = path.join(badgeDir, 'feeds') +fs.mkdirSync(badgeFeeds, { recursive: true }) +fs.writeFileSync(path.join(badgeDir, 'models.py'), 'RETIRED = "badge-retired-model"\nRETIRING = "badge-retiring-model"\n') +fs.writeFileSync(path.join(badgeFeeds, 'badge.json'), JSON.stringify({ + spec: 'model-eol/0.1', + publisher: 'test', + generated: '2026-08-01T00:00:00Z', + source: 'https://example.invalid/badge', + models: [ + { id: 'badge-retired-model', shutdown: '2000-01-01' }, + { id: 'badge-retiring-model', shutdown: '9999-12-31' }, + ], +})) +const badgeThresholdDays = '4000000' +const badgeRun = run(['alert', badgeDir, '--feeds', badgeFeeds, '--days', badgeThresholdDays, '--format', 'badge']) const badge = JSON.parse(badgeRun.out) assert(badgeRun.code === 1, 'badge alert keeps existing alert exit semantics') assert(badge.schemaVersion === 1 && badge.label === 'model-eol', 'badge emits Shields endpoint keys') assert(badge.color === 'red' && badge.message.includes('retired') && badge.message.includes('retiring'), 'badge counts retired and retiring errors') -const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'model-eol-test-')) +const largePipeDir = path.join(tempRoot, 'large-pipe-output') +fs.mkdirSync(largePipeDir) +const largePipeFindingCount = 512 +fs.writeFileSync(path.join(largePipeDir, 'models.py'), Array.from( + { length: largePipeFindingCount }, + (_, index) => `MODEL_${index} = "o3-deep-research"`, +).join('\n')) +const largePipeRun = await runPiped(['check', largePipeDir, '--days', '90', '--scope', 'all', '--json']) +let largePipeJson = null +try { + largePipeJson = JSON.parse(largePipeRun.out) +} catch {} +assert(largePipeRun.out.length > 64 * 1024 && largePipeJson?.findings.length === largePipeFindingCount, 'piped check JSON over 64KiB is complete and parseable') +assert(largePipeRun.code === 1, 'large piped check keeps finding exit semantics') + +const preloadDir = path.join(tempRoot, 'preloads') +fs.mkdirSync(preloadDir) +const intervalPreload = path.join(preloadDir, 'interval.cjs') +const beforeExitPreload = path.join(preloadDir, 'before-exit.cjs') +const endedStderrPreload = path.join(preloadDir, 'ended-stderr.cjs') +const badStdoutPreload = path.join(preloadDir, 'bad-stdout.cjs') +fs.writeFileSync(intervalPreload, 'setInterval(() => {}, 1000)\n') +fs.writeFileSync(beforeExitPreload, 'process.on("beforeExit", () => { process.exitCode = 7 })\n') +fs.writeFileSync(endedStderrPreload, 'process.stderr.end()\n') +fs.writeFileSync(badStdoutPreload, 'process.stdout.write = (_chunk, encoding, callback) => { const done = typeof encoding === "function" ? encoding : callback; process.nextTick(() => done?.(Object.assign(new Error("bad file descriptor"), { code: "EBADF" }))); return false }\n') +const preloadEnv = preload => ({ + ...process.env, + NODE_OPTIONS: [process.env.NODE_OPTIONS, `--require=${preload}`].filter(Boolean).join(' '), +}) +const intervalRun = await runPiped( + ['check', largePipeDir, '--days', '90', '--scope', 'all', '--json'], + { env: preloadEnv(intervalPreload) }, +) +let intervalJson = null +try { + intervalJson = JSON.parse(intervalRun.out) +} catch {} +assert(!intervalRun.timedOut && intervalJson?.findings.length === largePipeFindingCount, 'CLI drains complete output and exits despite a preloaded interval') +assert(intervalRun.code === 1, 'preloaded interval does not change the check exit code') +const beforeExitRun = await runPiped(['--help'], { env: preloadEnv(beforeExitPreload) }) +assert(!beforeExitRun.timedOut && beforeExitRun.code === 0, 'beforeExit hooks cannot rewrite the CLI exit code') +const closedCheckRun = await runPiped( + ['check', largePipeDir, '--days', '90', '--scope', 'all', '--json'], + { closeStdoutAfterData: true }, +) +assert(!closedCheckRun.timedOut && closedCheckRun.code === 1, 'early-closed stdout keeps the intended nonzero exit code') +const closedInventoryRun = await runPiped( + ['inventory', largePipeDir, '--json'], + { closeStdoutAfterData: true }, +) +assert(!closedInventoryRun.timedOut && closedInventoryRun.code === 0, 'EPIPE keeps a successful command exit code') +const endedStderrRun = await runPiped(['--invalid-review-flag'], { env: preloadEnv(endedStderrPreload) }) +assert(!endedStderrRun.timedOut && endedStderrRun.code === 2 && !endedStderrRun.err.includes('Unhandled'), 'ended stderr cannot replace a usage exit with an unhandled error') +const badStdoutRun = await runPiped(['--help'], { env: preloadEnv(badStdoutPreload) }) +assert(!badStdoutRun.timedOut && badStdoutRun.code === 3 && badStdoutRun.err.includes('model-eol: output failed: EBADF'), 'non-EPIPE output failure exits 3 with a best-effort diagnostic') + +const largeStderrFeeds = path.join(tempRoot, 'large-stderr-feeds') +fs.mkdirSync(largeStderrFeeds) +const largeStderrErrorCount = 512 +fs.writeFileSync(path.join(largeStderrFeeds, 'invalid.json'), JSON.stringify({ + spec: 'model-eol/0.1', + publisher: 'test', + generated: '2026-08-01T00:00:00Z', + models: Array.from({ length: largeStderrErrorCount }, (_, index) => ({ + id: `invalid-stderr-model-${index}`, + shutdown: '2000-01-01', + })), +})) +const largeStderrRun = await runPiped(['check', largePipeDir, '--feeds', largeStderrFeeds, '--json']) +assert(largeStderrRun.err.length > 64 * 1024 && largeStderrRun.err.includes(`model invalid-stderr-model-${largeStderrErrorCount - 1}`), 'piped stderr over 64KiB arrives complete') +assert(largeStderrRun.code === 2, 'large piped feed errors keep usage/feed exit semantics') const extensionCoverageDir = path.join(tempRoot, 'extension-coverage') const extensionCoverageFeeds = path.join(tempRoot, 'extension-coverage-feeds') @@ -226,8 +345,8 @@ assert(incompleteInventoryText.code === 0 && incompleteInventoryText.out.include const orangeBadgeDir = path.join(tempRoot, 'orange-badge') fs.mkdirSync(orangeBadgeDir) -fs.writeFileSync(path.join(orangeBadgeDir, 'app.py'), 'MODEL = "claude-opus-4-1-20250805"\n') -const orangeBadge = JSON.parse(run(['alert', orangeBadgeDir, '--format', 'badge']).out) +fs.writeFileSync(path.join(orangeBadgeDir, 'app.py'), 'MODEL = "badge-retiring-model"\n') +const orangeBadge = JSON.parse(run(['alert', orangeBadgeDir, '--feeds', badgeFeeds, '--days', badgeThresholdDays, '--format', 'badge']).out) assert(orangeBadge.color === 'orange' && orangeBadge.message === '1 retiring', 'badge is orange for only retiring errors') const greenBadgeDir = path.join(tempRoot, 'green-badge') fs.mkdirSync(greenBadgeDir)