From 874ff7089cc116682baed7aaca55b95ae5ff59e8 Mon Sep 17 00:00:00 2001 From: Taylor Asplund <62564740+DrCrinkle@users.noreply.github.com> Date: Fri, 13 Feb 2026 15:34:33 -0800 Subject: [PATCH] fix: ensure CLI exits after command completion (#12906) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: ensure CLI exits after command completion The CLI process would hang indefinitely after commands like `openclaw gateway restart` completed successfully. Two root causes: 1. `runCli()` returned without calling `process.exit()` after `program.parseAsync()` resolved, and Commander.js does not force-exit the process. 2. `daemon-cli/register.ts` eagerly called `createDefaultDeps()` which imported all messaging-provider modules, creating persistent event-loop handles that prevented natural Node exit. Changes: - Add `flushAndExit()` helper that drains stdout/stderr before calling `process.exit()`, preventing truncated piped output in CI/scripts. - Call `flushAndExit()` after both `tryRouteCli()` and `program.parseAsync()` resolve. - Remove unnecessary `void createDefaultDeps()` from daemon-cli registration — daemon lifecycle commands never use messaging deps. - Make `serveAcpGateway()` return a promise that resolves on intentional shutdown (SIGINT/SIGTERM), so `openclaw acp` blocks `parseAsync` for the bridge lifetime and exits cleanly on signal. - Handle the returned promise in the standalone main-module entry point to avoid unhandled rejections. Fixes #12904 Co-Authored-By: Claude Opus 4.6 * fix: refactor CLI lifecycle and lazy outbound deps (#12906) (thanks @DrCrinkle) --------- Co-authored-by: Claude Opus 4.6 Co-authored-by: Peter Steinberger --- CHANGELOG.md | 1 + src/acp/server.ts | 34 ++++++++++++- src/cli/acp-cli.ts | 4 +- src/cli/daemon-cli/register.ts | 4 -- src/cli/deps.test.ts | 93 ++++++++++++++++++++++++++++++++++ src/cli/deps.ts | 44 +++++++++++----- src/cli/run-main.exit.test.ts | 49 ++++++++++++++++++ 7 files changed, 208 insertions(+), 21 deletions(-) create mode 100644 src/cli/deps.test.ts create mode 100644 src/cli/run-main.exit.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index f4c55aa8f8..56a5d758c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- CLI: lazily load outbound provider dependencies and remove forced success-path exits so commands terminate naturally without killing intentional long-running foreground actions. (#12906) Thanks @DrCrinkle. - Clawdock: avoid Zsh readonly variable collisions in helper scripts. (#15501) Thanks @nkelner. - Discord: route autoThread replies to existing threads instead of the root channel. (#8302) Thanks @gavinbmoore, @thewilloftheshadow. - Agents/Image tool: cap image-analysis completion `maxTokens` by model capability (`min(4096, model.maxTokens)`) to avoid over-limit provider failures while still preventing truncation. (#11770) Thanks @detecti1. diff --git a/src/acp/server.ts b/src/acp/server.ts index 4a2c835b54..93acc4a523 100644 --- a/src/acp/server.ts +++ b/src/acp/server.ts @@ -11,7 +11,7 @@ import { isMainModule } from "../infra/is-main.js"; import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js"; import { AcpGatewayAgent } from "./translator.js"; -export function serveAcpGateway(opts: AcpServerOptions = {}): void { +export function serveAcpGateway(opts: AcpServerOptions = {}): Promise { const cfg = loadConfig(); const connection = buildGatewayConnectionDetails({ config: cfg, @@ -34,6 +34,12 @@ export function serveAcpGateway(opts: AcpServerOptions = {}): void { auth.password; let agent: AcpGatewayAgent | null = null; + let onClosed!: () => void; + const closed = new Promise((resolve) => { + onClosed = resolve; + }); + let stopped = false; + const gateway = new GatewayClient({ url: connection.url, token: token || undefined, @@ -50,9 +56,29 @@ export function serveAcpGateway(opts: AcpServerOptions = {}): void { }, onClose: (code, reason) => { agent?.handleGatewayDisconnect(`${code}: ${reason}`); + // Resolve only on intentional shutdown (gateway.stop() sets closed + // which skips scheduleReconnect, then fires onClose). Transient + // disconnects are followed by automatic reconnect attempts. + if (stopped) { + onClosed(); + } }, }); + const shutdown = () => { + if (stopped) { + return; + } + stopped = true; + gateway.stop(); + // If no WebSocket is active (e.g. between reconnect attempts), + // gateway.stop() won't trigger onClose, so resolve directly. + onClosed(); + }; + + process.once("SIGINT", shutdown); + process.once("SIGTERM", shutdown); + const input = Writable.toWeb(process.stdout); const output = Readable.toWeb(process.stdin) as unknown as ReadableStream; const stream = ndJsonStream(input, output); @@ -64,6 +90,7 @@ export function serveAcpGateway(opts: AcpServerOptions = {}): void { }, stream); gateway.start(); + return closed; } function parseArgs(args: string[]): AcpServerOptions { @@ -140,5 +167,8 @@ Options: if (isMainModule({ currentFile: fileURLToPath(import.meta.url) })) { const opts = parseArgs(process.argv.slice(2)); - serveAcpGateway(opts); + serveAcpGateway(opts).catch((err) => { + console.error(String(err)); + process.exit(1); + }); } diff --git a/src/cli/acp-cli.ts b/src/cli/acp-cli.ts index 1be77e71fc..c86deb48f2 100644 --- a/src/cli/acp-cli.ts +++ b/src/cli/acp-cli.ts @@ -22,9 +22,9 @@ export function registerAcpCli(program: Command) { "after", () => `\n${theme.muted("Docs:")} ${formatDocsLink("/cli/acp", "docs.openclaw.ai/cli/acp")}\n`, ) - .action((opts) => { + .action(async (opts) => { try { - serveAcpGateway({ + await serveAcpGateway({ gatewayUrl: opts.url as string | undefined, gatewayToken: opts.token as string | undefined, gatewayPassword: opts.password as string | undefined, diff --git a/src/cli/daemon-cli/register.ts b/src/cli/daemon-cli/register.ts index d1599a206a..47e3dd09bd 100644 --- a/src/cli/daemon-cli/register.ts +++ b/src/cli/daemon-cli/register.ts @@ -1,7 +1,6 @@ import type { Command } from "commander"; import { formatDocsLink } from "../../terminal/links.js"; import { theme } from "../../terminal/theme.js"; -import { createDefaultDeps } from "../deps.js"; import { runDaemonInstall, runDaemonRestart, @@ -83,7 +82,4 @@ export function registerDaemonCli(program: Command) { .action(async (opts) => { await runDaemonRestart(opts); }); - - // Build default deps (parity with other commands). - void createDefaultDeps(); } diff --git a/src/cli/deps.test.ts b/src/cli/deps.test.ts new file mode 100644 index 0000000000..34c28cece5 --- /dev/null +++ b/src/cli/deps.test.ts @@ -0,0 +1,93 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { createDefaultDeps } from "./deps.js"; + +const moduleLoads = vi.hoisted(() => ({ + whatsapp: vi.fn(), + telegram: vi.fn(), + discord: vi.fn(), + slack: vi.fn(), + signal: vi.fn(), + imessage: vi.fn(), +})); + +const sendFns = vi.hoisted(() => ({ + whatsapp: vi.fn(async () => ({ messageId: "w1", toJid: "whatsapp:1" })), + telegram: vi.fn(async () => ({ messageId: "t1", chatId: "telegram:1" })), + discord: vi.fn(async () => ({ messageId: "d1", channelId: "discord:1" })), + slack: vi.fn(async () => ({ messageId: "s1", channelId: "slack:1" })), + signal: vi.fn(async () => ({ messageId: "sg1", conversationId: "signal:1" })), + imessage: vi.fn(async () => ({ messageId: "i1", chatId: "imessage:1" })), +})); + +vi.mock("../channels/web/index.js", () => { + moduleLoads.whatsapp(); + return { sendMessageWhatsApp: sendFns.whatsapp }; +}); + +vi.mock("../telegram/send.js", () => { + moduleLoads.telegram(); + return { sendMessageTelegram: sendFns.telegram }; +}); + +vi.mock("../discord/send.js", () => { + moduleLoads.discord(); + return { sendMessageDiscord: sendFns.discord }; +}); + +vi.mock("../slack/send.js", () => { + moduleLoads.slack(); + return { sendMessageSlack: sendFns.slack }; +}); + +vi.mock("../signal/send.js", () => { + moduleLoads.signal(); + return { sendMessageSignal: sendFns.signal }; +}); + +vi.mock("../imessage/send.js", () => { + moduleLoads.imessage(); + return { sendMessageIMessage: sendFns.imessage }; +}); + +describe("createDefaultDeps", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("does not load provider modules until a dependency is used", async () => { + const deps = createDefaultDeps(); + + expect(moduleLoads.whatsapp).not.toHaveBeenCalled(); + expect(moduleLoads.telegram).not.toHaveBeenCalled(); + expect(moduleLoads.discord).not.toHaveBeenCalled(); + expect(moduleLoads.slack).not.toHaveBeenCalled(); + expect(moduleLoads.signal).not.toHaveBeenCalled(); + expect(moduleLoads.imessage).not.toHaveBeenCalled(); + + const sendTelegram = deps.sendMessageTelegram as unknown as ( + ...args: unknown[] + ) => Promise; + await sendTelegram("chat", "hello", { verbose: false }); + + expect(moduleLoads.telegram).toHaveBeenCalledTimes(1); + expect(sendFns.telegram).toHaveBeenCalledTimes(1); + expect(moduleLoads.whatsapp).not.toHaveBeenCalled(); + expect(moduleLoads.discord).not.toHaveBeenCalled(); + expect(moduleLoads.slack).not.toHaveBeenCalled(); + expect(moduleLoads.signal).not.toHaveBeenCalled(); + expect(moduleLoads.imessage).not.toHaveBeenCalled(); + }); + + it("reuses module cache after first dynamic import", async () => { + const deps = createDefaultDeps(); + const sendDiscord = deps.sendMessageDiscord as unknown as ( + ...args: unknown[] + ) => Promise; + + await sendDiscord("channel", "first", { verbose: false }); + await sendDiscord("channel", "second", { verbose: false }); + + expect(moduleLoads.discord).toHaveBeenCalledTimes(1); + expect(sendFns.discord).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/cli/deps.ts b/src/cli/deps.ts index 1f0e8e587f..a3c3c72ac4 100644 --- a/src/cli/deps.ts +++ b/src/cli/deps.ts @@ -1,10 +1,10 @@ +import type { sendMessageWhatsApp } from "../channels/web/index.js"; +import type { sendMessageDiscord } from "../discord/send.js"; +import type { sendMessageIMessage } from "../imessage/send.js"; import type { OutboundSendDeps } from "../infra/outbound/deliver.js"; -import { logWebSelfId, sendMessageWhatsApp } from "../channels/web/index.js"; -import { sendMessageDiscord } from "../discord/send.js"; -import { sendMessageIMessage } from "../imessage/send.js"; -import { sendMessageSignal } from "../signal/send.js"; -import { sendMessageSlack } from "../slack/send.js"; -import { sendMessageTelegram } from "../telegram/send.js"; +import type { sendMessageSignal } from "../signal/send.js"; +import type { sendMessageSlack } from "../slack/send.js"; +import type { sendMessageTelegram } from "../telegram/send.js"; export type CliDeps = { sendMessageWhatsApp: typeof sendMessageWhatsApp; @@ -17,12 +17,30 @@ export type CliDeps = { export function createDefaultDeps(): CliDeps { return { - sendMessageWhatsApp, - sendMessageTelegram, - sendMessageDiscord, - sendMessageSlack, - sendMessageSignal, - sendMessageIMessage, + sendMessageWhatsApp: async (...args) => { + const { sendMessageWhatsApp } = await import("../channels/web/index.js"); + return await sendMessageWhatsApp(...args); + }, + sendMessageTelegram: async (...args) => { + const { sendMessageTelegram } = await import("../telegram/send.js"); + return await sendMessageTelegram(...args); + }, + sendMessageDiscord: async (...args) => { + const { sendMessageDiscord } = await import("../discord/send.js"); + return await sendMessageDiscord(...args); + }, + sendMessageSlack: async (...args) => { + const { sendMessageSlack } = await import("../slack/send.js"); + return await sendMessageSlack(...args); + }, + sendMessageSignal: async (...args) => { + const { sendMessageSignal } = await import("../signal/send.js"); + return await sendMessageSignal(...args); + }, + sendMessageIMessage: async (...args) => { + const { sendMessageIMessage } = await import("../imessage/send.js"); + return await sendMessageIMessage(...args); + }, }; } @@ -38,4 +56,4 @@ export function createOutboundSendDeps(deps: CliDeps): OutboundSendDeps { }; } -export { logWebSelfId }; +export { logWebSelfId } from "../web/auth-store.js"; diff --git a/src/cli/run-main.exit.test.ts b/src/cli/run-main.exit.test.ts new file mode 100644 index 0000000000..86d74f0964 --- /dev/null +++ b/src/cli/run-main.exit.test.ts @@ -0,0 +1,49 @@ +import process from "node:process"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const tryRouteCliMock = vi.hoisted(() => vi.fn()); +const loadDotEnvMock = vi.hoisted(() => vi.fn()); +const normalizeEnvMock = vi.hoisted(() => vi.fn()); +const ensurePathMock = vi.hoisted(() => vi.fn()); +const assertRuntimeMock = vi.hoisted(() => vi.fn()); + +vi.mock("./route.js", () => ({ + tryRouteCli: tryRouteCliMock, +})); + +vi.mock("../infra/dotenv.js", () => ({ + loadDotEnv: loadDotEnvMock, +})); + +vi.mock("../infra/env.js", () => ({ + normalizeEnv: normalizeEnvMock, +})); + +vi.mock("../infra/path-env.js", () => ({ + ensureOpenClawCliOnPath: ensurePathMock, +})); + +vi.mock("../infra/runtime-guard.js", () => ({ + assertSupportedRuntime: assertRuntimeMock, +})); + +const { runCli } = await import("./run-main.js"); + +describe("runCli exit behavior", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("does not force process.exit after successful routed command", async () => { + tryRouteCliMock.mockResolvedValueOnce(true); + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`unexpected process.exit(${String(code)})`); + }) as typeof process.exit); + + await runCli(["node", "openclaw", "status"]); + + expect(tryRouteCliMock).toHaveBeenCalledWith(["node", "openclaw", "status"]); + expect(exitSpy).not.toHaveBeenCalled(); + exitSpy.mockRestore(); + }); +});