mirror of
https://github.com/openclaw/openclaw.git
synced 2026-02-19 18:39:20 -05:00
fix: ensure CLI exits after command completion (#12906)
* 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 <noreply@anthropic.com> * fix: refactor CLI lifecycle and lazy outbound deps (#12906) (thanks @DrCrinkle) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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<void> {
|
||||
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<void>((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<Uint8Array>;
|
||||
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);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
|
||||
93
src/cli/deps.test.ts
Normal file
93
src/cli/deps.test.ts
Normal file
@@ -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<unknown>;
|
||||
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<unknown>;
|
||||
|
||||
await sendDiscord("channel", "first", { verbose: false });
|
||||
await sendDiscord("channel", "second", { verbose: false });
|
||||
|
||||
expect(moduleLoads.discord).toHaveBeenCalledTimes(1);
|
||||
expect(sendFns.discord).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
@@ -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";
|
||||
|
||||
49
src/cli/run-main.exit.test.ts
Normal file
49
src/cli/run-main.exit.test.ts
Normal file
@@ -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();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user