From 96f3a4e46161b15806e1fc20b96d9a795e8ba11d Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 11 Aug 2026 20:35:23 -0700 Subject: [PATCH] test(cli): make help-exit process failures self-diagnosing (#122205) CI run 31515400879 (checks-node-compact-small-1) failed the run-main import-failure test as an opaque "expected null to be 1": the deadlock guard had fired with the child still alive, but the guard rejection carried the captured child output only as hidden error properties and the tests asserted on failure.code blindly, discarding the startup trace lines that pinpoint where the child stopped. runCliProcess now takes expectedExitCode and throws self-describing errors for all failure identities (guard fired, signal death, wrong exit code) with bounded tails of the child stderr/stdout embedded in the message. The CliProcessFailure try/catch pattern is deleted file-wide; formatter regression tests protect the diagnostic contract. --- src/cli/help-exit.process.test.ts | 160 +++++++++++++++++++----------- 1 file changed, 102 insertions(+), 58 deletions(-) diff --git a/src/cli/help-exit.process.test.ts b/src/cli/help-exit.process.test.ts index 23ddc02bb3d6..e2574679725e 100644 --- a/src/cli/help-exit.process.test.ts +++ b/src/cli/help-exit.process.test.ts @@ -17,6 +17,8 @@ const tempDirs = useAutoCleanupTempDirTracker(afterEach); // to cold-load the CLI graph on shared hosted runners, while still exiting correctly. // Keep the default guard below the shared Vitest deadline so it always reports // captured child output before the framework can replace it with an opaque timeout. +// Guard, signal, and wrong-code failures embed both output tails so CI shows the +// child's last completed startup step. const DEFAULT_CHILD_PROCESS_TIMEOUT_MS = DEFAULT_VITEST_TEST_TIMEOUT_MS - 20_000; const SLOW_DOTENV_CHILD_PROCESS_TIMEOUT_MS = 240_000; const SLOW_DOTENV_TEST_TIMEOUT_MS = SLOW_DOTENV_CHILD_PROCESS_TIMEOUT_MS + 10_000; @@ -34,6 +36,21 @@ const LAZY_GROUP_HELP_CASES = [ { group: "update", usageCommand: "update", registry: "subcli" }, ] as const; +function formatCliProcessFailure(params: { + reason: string; + stdout: string; + stderr: string; +}): string { + const tail = (stream: string) => { + const tailLength = 8_000; + const truncatedLength = stream.length - tailLength; + return truncatedLength > 0 + ? `[... truncated ${truncatedLength} chars ...]\n${stream.slice(-tailLength)}` + : stream; + }; + return `${params.reason}\n--- child stderr (tail) ---\n${tail(params.stderr)}\n--- child stdout (tail) ---\n${tail(params.stdout)}`; +} + async function createHelpProcessFixture(config?: Record) { const root = tempDirs.make("openclaw-help-exit-"); const stateDir = path.join(root, "state"); @@ -93,6 +110,7 @@ async function runCliProcess(params: { allowRespawn?: boolean; stateEnv?: (stateDir: string) => Record; timeoutMs?: number; + expectedExitCode?: number; }) { const fixture = await createHelpProcessFixture(params.config); if (params.stateEnv) { @@ -155,6 +173,8 @@ async function runCliProcess(params: { const stdoutEnded = once(child.stdout, "end"); const stderrEnded = once(child.stderr, "end"); + const expectedExitCode = params.expectedExitCode ?? 0; + const timeoutMs = params.timeoutMs ?? DEFAULT_CHILD_PROCESS_TIMEOUT_MS; let timeout: NodeJS.Timeout | undefined; const exit = await Promise.race([ Promise.all([once(child, "exit"), stdoutEnded, stderrEnded]).then(([[code, signal]]) => ({ @@ -165,14 +185,15 @@ async function runCliProcess(params: { timeout = setTimeout(() => { child.kill("SIGKILL"); reject( - Object.assign(new Error("CLI process did not exit before the deadlock guard"), { - code: child.exitCode, - signal: child.signalCode, - stderr, - stdout, - }), + new Error( + formatCliProcessFailure({ + reason: `CLI process did not exit before the ${timeoutMs}ms deadlock guard (SIGKILL sent; exitCode=${child.exitCode} signalCode=${child.signalCode})`, + stderr, + stdout, + }), + ), ); - }, params.timeoutMs ?? DEFAULT_CHILD_PROCESS_TIMEOUT_MS); + }, timeoutMs); timeout.unref(); }), ]).finally(() => { @@ -180,12 +201,23 @@ async function runCliProcess(params: { clearTimeout(timeout); } }); - if (exit.code !== 0) { - throw Object.assign(new Error(`CLI process exited with code ${exit.code}`), { - ...exit, - stderr, - stdout, - }); + if (exit.signal) { + throw new Error( + formatCliProcessFailure({ + reason: `CLI process was killed by signal ${exit.signal} (expected exit code ${expectedExitCode})`, + stderr, + stdout, + }), + ); + } + if (exit.code !== expectedExitCode) { + throw new Error( + formatCliProcessFailure({ + reason: `CLI process exited with code ${exit.code} (expected ${expectedExitCode})`, + stderr, + stdout, + }), + ); } return { stderr, stdout }; } @@ -197,11 +229,33 @@ function parseJsonLines(stdout: string): Array> { .map((line) => JSON.parse(line) as Record); } -type CliProcessFailure = Error & { - code?: number | string; - stderr?: string; - stdout?: string; -}; +describe("formatCliProcessFailure", () => { + it("includes the failure identity and both captured output tails", () => { + const reason = + "CLI process did not exit before the 240000ms deadlock guard (SIGKILL sent; exitCode=null signalCode=null)"; + const message = formatCliProcessFailure({ + reason, + stderr: "startup trace: entry.bootstrap", + stdout: "partial command output", + }); + + expect(message).toContain(reason); + expect(message).toContain("startup trace: entry.bootstrap"); + expect(message).toContain("partial command output"); + }); + + it("keeps the end of streams longer than the output tail cap", () => { + const message = formatCliProcessFailure({ + reason: "wrong exit code", + stderr: "", + stdout: `${"x".repeat(8_005)}END`, + }); + + expect(message).toContain("[... truncated 8 chars ...]"); + expect(message).toMatch(/xEND$/u); + }); +}); + describe("CLI help process exit", () => { it("disables esbuild worker IPC for source CLI children", () => { expect(process.env.ESBUILD_WORKER_THREADS).toBe("0"); @@ -328,26 +382,21 @@ describe("JSON console style process output", () => { it( "captures exact exit code 2 after loading dotenv for entry validation diagnostics", async () => { - let failure: CliProcessFailure | undefined; - try { - await runCliProcess({ - args: ["--container"], - config: { - logging: { - consoleStyle: "${OPENCLAW_TEST_CONSOLE_STYLE}", - level: "silent", - }, + const result = await runCliProcess({ + args: ["--container"], + config: { + logging: { + consoleStyle: "${OPENCLAW_TEST_CONSOLE_STYLE}", + level: "silent", }, - env: { OPENCLAW_TEST_CONSOLE_STYLE: undefined }, - stateEnv: () => ({ OPENCLAW_TEST_CONSOLE_STYLE: "json" }), - timeoutMs: SLOW_DOTENV_CHILD_PROCESS_TIMEOUT_MS, - }); - } catch (error) { - failure = error as CliProcessFailure; - } + }, + env: { OPENCLAW_TEST_CONSOLE_STYLE: undefined }, + stateEnv: () => ({ OPENCLAW_TEST_CONSOLE_STYLE: "json" }), + timeoutMs: SLOW_DOTENV_CHILD_PROCESS_TIMEOUT_MS, + expectedExitCode: 2, + }); - expect(failure?.code).toBe(2); - expect(parseJsonLines(failure?.stderr ?? "")).toEqual([ + expect(parseJsonLines(result.stderr)).toEqual([ expect.objectContaining({ level: "error", message: expect.stringContaining("--container requires a value"), @@ -360,30 +409,25 @@ describe("JSON console style process output", () => { it( "loads eligible dotenv before formatting a run-main import failure", async () => { - let failure: CliProcessFailure | undefined; - try { - await runCliProcess({ - args: ["gateway", "status"], - config: { - logging: { - consoleStyle: "${OPENCLAW_TEST_CONSOLE_STYLE}", - level: "silent", - }, + const result = await runCliProcess({ + args: ["gateway", "status"], + config: { + logging: { + consoleStyle: "${OPENCLAW_TEST_CONSOLE_STYLE}", + level: "silent", }, - env: { - OPENCLAW_GATEWAY_STARTUP_TRACE: "1", - OPENCLAW_TEST_CONSOLE_STYLE: undefined, - }, - failRunMainImport: true, - stateEnv: () => ({ OPENCLAW_TEST_CONSOLE_STYLE: "json" }), - timeoutMs: SLOW_DOTENV_CHILD_PROCESS_TIMEOUT_MS, - }); - } catch (error) { - failure = error as CliProcessFailure; - } + }, + env: { + OPENCLAW_GATEWAY_STARTUP_TRACE: "1", + OPENCLAW_TEST_CONSOLE_STYLE: undefined, + }, + failRunMainImport: true, + stateEnv: () => ({ OPENCLAW_TEST_CONSOLE_STYLE: "json" }), + timeoutMs: SLOW_DOTENV_CHILD_PROCESS_TIMEOUT_MS, + expectedExitCode: 1, + }); - expect(failure?.code).toBe(1); - expect(parseJsonLines(failure?.stderr ?? "")).toEqual( + expect(parseJsonLines(result.stderr)).toEqual( expect.arrayContaining([ expect.objectContaining({ level: "info",