From 263ad629c161ecf33f17482aa3480eff81d36192 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 7 Aug 2026 21:28:52 -0700 Subject: [PATCH] fix(build): keep CLI help fallback asynchronous (#120454) --- scripts/write-cli-startup-metadata.ts | 43 +++++----------- .../write-cli-startup-metadata.test.ts | 51 ++++++++++++------- 2 files changed, 46 insertions(+), 48 deletions(-) diff --git a/scripts/write-cli-startup-metadata.ts b/scripts/write-cli-startup-metadata.ts index fffa9ef392c3..b85f2c1e5219 100644 --- a/scripts/write-cli-startup-metadata.ts +++ b/scripts/write-cli-startup-metadata.ts @@ -665,9 +665,9 @@ export async function renderBundledRootHelpText( }); } -function renderSourceRootHelpText(renderContext?: RootHelpRenderContext): string { +async function renderSourceRootHelpText(renderContext?: RootHelpRenderContext): Promise { if (!renderContext) { - return withIsolatedRootHelpRenderContext(extensionsDir, renderSourceRootHelpText); + return await withIsolatedRootHelpRenderContext(extensionsDir, renderSourceRootHelpText); } const moduleUrl = pathToFileURL(path.join(rootDir, "src/cli/program/root-help.ts")).href; const renderOptions = { @@ -684,28 +684,12 @@ function renderSourceRootHelpText(renderContext?: RootHelpRenderContext): string "process.stdout.write(output);", "process.exit(0);", ].join("\n"); - const result = spawnSync( - process.execPath, - ["--import", "tsx", "--input-type=module", "--eval", inlineModule], - { - cwd: rootDir, - encoding: "utf8", - env: renderContext.env, - killSignal: "SIGKILL", - timeout: ROOT_HELP_RENDER_TIMEOUT_MS, - }, - ); - if (result.error) { - throw result.error; - } - if (result.status !== 0) { - const stderr = result.stderr?.trim(); - throw new Error( - "Failed to render source root help" + - (stderr ? `: ${stderr}` : result.signal ? `: terminated by ${result.signal}` : ""), - ); - } - return result.stdout ?? ""; + return await spawnText(["--import", "tsx", "--input-type=module", "--eval", inlineModule], { + cwd: rootDir, + env: renderContext.env ?? process.env, + failureMessage: "Failed to render source root help", + timeoutMs: ROOT_HELP_RENDER_TIMEOUT_MS, + }); } async function renderSourceBrowserHelpText(renderContext: RootHelpRenderContext): Promise { @@ -777,7 +761,7 @@ export async function writeCliStartupMetadata(options?: { extensionsDir?: string; sourceRootDir?: string; renderBundledRootHelpText?: typeof renderBundledRootHelpText; - renderSourceRootHelpText?: typeof renderSourceRootHelpText; + renderSourceRootHelpText?: (renderContext: RootHelpRenderContext) => Awaitable; renderSourceBrowserHelpText?: (renderContext: RootHelpRenderContext) => Awaitable; renderSourceSecretsHelpText?: (renderContext: RootHelpRenderContext) => Awaitable; renderSourceNodesHelpText?: (renderContext: RootHelpRenderContext) => Awaitable; @@ -877,10 +861,11 @@ export async function writeCliStartupMetadata(options?: { renderContext, ); } catch { - // The spawnSync source fallback blocks the event loop; that is fine for - // this rare recovery path (missing/broken bundle) and only delays - // draining sibling render output, not its correctness. - return (options?.renderSourceRootHelpText ?? renderSourceRootHelpText)(renderContext); + // Keep the fallback asynchronous: sibling help renders share this + // event loop, so blocking here can turn completed children into false timeouts. + return await (options?.renderSourceRootHelpText ?? renderSourceRootHelpText)( + renderContext, + ); } })(); const hasCustomCommandRenderer = diff --git a/test/scripts/write-cli-startup-metadata.test.ts b/test/scripts/write-cli-startup-metadata.test.ts index d1a6c3269227..1546cf547f26 100644 --- a/test/scripts/write-cli-startup-metadata.test.ts +++ b/test/scripts/write-cli-startup-metadata.test.ts @@ -1,5 +1,5 @@ // Write Cli Startup Metadata tests cover write cli startup metadata script behavior. -import { spawn, spawnSync } from "node:child_process"; +import { spawn } from "node:child_process"; import { EventEmitter } from "node:events"; import fs, { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import path from "node:path"; @@ -12,7 +12,7 @@ import { createScriptTestHarness } from "./test-helpers.js"; vi.mock("node:child_process", async (importOriginal) => { const actual = await importOriginal(); - return { ...actual, spawnSync: vi.fn(actual.spawnSync) }; + return { ...actual, spawn: vi.fn(actual.spawn) }; }); // These subprocess tests use explicit ready/close signals; timeout only catches broken fixtures. @@ -119,25 +119,38 @@ async function waitForChildClose( describe("write-cli-startup-metadata", () => { const { createTempDir } = createScriptTestHarness(); - it("hard-kills synchronous source root help after its timeout", () => { - const spawnSyncMock = vi.mocked(spawnSync); - const successfulRender = { - error: undefined, - output: [null, "Usage: openclaw\n", ""], - pid: 123, - signal: null, - status: 0, - stderr: "", - stdout: "Usage: openclaw\n", - }; - spawnSyncMock.mockReturnValueOnce(successfulRender); + it("renders source root help without blocking sibling child events", async () => { + const child = createSpawnTextChild(); + const spawnMock = vi.mocked(spawn); + spawnMock.mockImplementationOnce(() => child as unknown as ReturnType); + let siblingEventObserved = false; + const siblingEvent = new Promise((resolve) => { + setImmediate(() => { + siblingEventObserved = true; + resolve(); + }); + }); - expect(__testing.renderSourceRootHelpText()).toBe("Usage: openclaw\n"); + const render = __testing.renderSourceRootHelpText(); + child.stdout.write("Usage: openclaw\n"); + setImmediate(() => { + child.emit("close", 0, null); + }); - expect(spawnSyncMock).toHaveBeenCalledOnce(); - expect(spawnSyncMock.mock.calls[0]?.[2]).toMatchObject({ - killSignal: "SIGKILL", - timeout: 120_000, + await siblingEvent; + expect(siblingEventObserved).toBe(true); + await expect(render).resolves.toBe("Usage: openclaw\n"); + expect(spawnMock).toHaveBeenCalledOnce(); + expect(spawnMock.mock.calls[0]?.[1]).toEqual([ + "--import", + "tsx", + "--input-type=module", + "--eval", + expect.any(String), + ]); + expect(spawnMock.mock.calls[0]?.[2]).toMatchObject({ + detached: process.platform !== "win32", + stdio: ["ignore", "pipe", "pipe"], }); });