diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d7f78699..5ac331cbe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ Docs: https://docs.openclaw.ai - Cron: deliver text-only output directly when `delivery.to` is set so cron recipients get full output instead of summaries. (#16360) Thanks @thewilloftheshadow. - Cron: prevent `cron list`/`cron status` from silently skipping past-due recurring jobs by using maintenance recompute semantics. (#16156) Thanks @zerone0x. - CLI/Dashboard: when `gateway.bind=lan`, generate localhost dashboard URLs to satisfy browser secure-context requirements while preserving non-LAN bind behavior. (#16434) Thanks @BinHPdev. +- CLI/Plugins: ensure `openclaw message send` exits after successful delivery across plugin-backed channels so one-shot sends do not hang. (#16491) Thanks @yinghaosang. - Cron: repair missing/corrupt `nextRunAtMs` for the updated job without globally recomputing unrelated due jobs during `cron update`. (#15750) - Discord: prefer gateway guild id when logging inbound messages so cached-miss guilds do not appear as `guild=dm`. Thanks @thewilloftheshadow. - TUI: refactor searchable select list description layout and add regression coverage for ANSI-highlight width bounds. diff --git a/src/cli/program/message/helpers.test.ts b/src/cli/program/message/helpers.test.ts new file mode 100644 index 000000000..5e5ed5c17 --- /dev/null +++ b/src/cli/program/message/helpers.test.ts @@ -0,0 +1,125 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const messageCommandMock = vi.fn(async () => {}); +vi.mock("../../../commands/message.js", () => ({ + messageCommand: (...args: unknown[]) => messageCommandMock(...args), +})); + +vi.mock("../../../globals.js", () => ({ + danger: (s: string) => s, + setVerbose: vi.fn(), +})); + +vi.mock("../../plugin-registry.js", () => ({ + ensurePluginRegistryLoaded: vi.fn(), +})); + +const exitMock = vi.fn((): never => { + throw new Error("exit"); +}); +const errorMock = vi.fn(); +const runtimeMock = { log: vi.fn(), error: errorMock, exit: exitMock }; +vi.mock("../../../runtime.js", () => ({ + defaultRuntime: runtimeMock, +})); + +vi.mock("../../deps.js", () => ({ + createDefaultDeps: () => ({}), +})); + +const { createMessageCliHelpers } = await import("./helpers.js"); + +describe("runMessageAction", () => { + beforeEach(() => { + vi.clearAllMocks(); + messageCommandMock.mockReset().mockResolvedValue(undefined); + exitMock.mockReset().mockImplementation((): never => { + throw new Error("exit"); + }); + }); + + it("calls exit(0) after successful message delivery", async () => { + const fakeCommand = { help: vi.fn() } as never; + const { runMessageAction } = createMessageCliHelpers(fakeCommand, "discord"); + + await expect( + runMessageAction("send", { channel: "discord", target: "123", message: "hi" }), + ).rejects.toThrow("exit"); + + expect(exitMock).toHaveBeenCalledOnce(); + expect(exitMock).toHaveBeenCalledWith(0); + }); + + it("calls exit(1) when message delivery fails", async () => { + messageCommandMock.mockRejectedValueOnce(new Error("send failed")); + const fakeCommand = { help: vi.fn() } as never; + const { runMessageAction } = createMessageCliHelpers(fakeCommand, "discord"); + + await expect( + runMessageAction("send", { channel: "discord", target: "123", message: "hi" }), + ).rejects.toThrow("exit"); + + expect(errorMock).toHaveBeenCalledWith("Error: send failed"); + expect(exitMock).toHaveBeenCalledOnce(); + expect(exitMock).toHaveBeenCalledWith(1); + }); + + it("does not call exit(0) when the action throws", async () => { + messageCommandMock.mockRejectedValueOnce(new Error("boom")); + const fakeCommand = { help: vi.fn() } as never; + const { runMessageAction } = createMessageCliHelpers(fakeCommand, "discord"); + + await expect( + runMessageAction("send", { channel: "discord", target: "123", message: "hi" }), + ).rejects.toThrow("exit"); + + // exit should only be called once with code 1, never with 0 + expect(exitMock).toHaveBeenCalledOnce(); + expect(exitMock).not.toHaveBeenCalledWith(0); + }); + + it("does not call exit(0) if the error path returns", async () => { + messageCommandMock.mockRejectedValueOnce(new Error("boom")); + exitMock.mockReset().mockImplementation(() => undefined as never); + const fakeCommand = { help: vi.fn() } as never; + const { runMessageAction } = createMessageCliHelpers(fakeCommand, "discord"); + + await expect( + runMessageAction("send", { channel: "discord", target: "123", message: "hi" }), + ).resolves.toBeUndefined(); + + expect(errorMock).toHaveBeenCalledWith("Error: boom"); + expect(exitMock).toHaveBeenCalledOnce(); + expect(exitMock).toHaveBeenCalledWith(1); + expect(exitMock).not.toHaveBeenCalledWith(0); + }); + + it("passes action and maps account to accountId", async () => { + const fakeCommand = { help: vi.fn() } as never; + const { runMessageAction } = createMessageCliHelpers(fakeCommand, "discord"); + + await expect( + runMessageAction("poll", { + channel: "discord", + target: "456", + account: "acct-1", + message: "hi", + }), + ).rejects.toThrow("exit"); + + expect(messageCommandMock).toHaveBeenCalledWith( + expect.objectContaining({ + action: "poll", + channel: "discord", + target: "456", + accountId: "acct-1", + message: "hi", + }), + expect.anything(), + expect.anything(), + ); + // account key should be stripped in favor of accountId + const passedOpts = messageCommandMock.mock.calls[0][0] as Record; + expect(passedOpts).not.toHaveProperty("account"); + }); +}); diff --git a/src/cli/program/message/helpers.ts b/src/cli/program/message/helpers.ts index 803c064e9..d71d1ec8b 100644 --- a/src/cli/program/message/helpers.ts +++ b/src/cli/program/message/helpers.ts @@ -14,6 +14,14 @@ export type MessageCliHelpers = { runMessageAction: (action: string, opts: Record) => Promise; }; +function normalizeMessageOptions(opts: Record): Record { + const { account, ...rest } = opts; + return { + ...rest, + accountId: typeof account === "string" ? account : undefined, + }; +} + export function createMessageCliHelpers( message: Command, messageChannelOptions: string, @@ -35,18 +43,13 @@ export function createMessageCliHelpers( setVerbose(Boolean(opts.verbose)); ensurePluginRegistryLoaded(); const deps = createDefaultDeps(); + let failed = false; await runCommandWithRuntime( defaultRuntime, async () => { await messageCommand( { - ...(() => { - const { account, ...rest } = opts; - return { - ...rest, - accountId: typeof account === "string" ? account : undefined, - }; - })(), + ...normalizeMessageOptions(opts), action, }, deps, @@ -54,10 +57,15 @@ export function createMessageCliHelpers( ); }, (err) => { + failed = true; defaultRuntime.error(danger(String(err))); defaultRuntime.exit(1); }, ); + if (failed) { + return; + } + defaultRuntime.exit(0); }; // `message` is only used for `message.help({ error: true })`, keep the