From 8217d77ece6dedcd3cab81d980144cf0cdfb9a35 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Sat, 14 Feb 2026 17:33:08 -0500 Subject: [PATCH] fix(cli): run plugin gateway_stop hooks before message exit (#16580) Merged via /review-pr -> /prepare-pr -> /merge-pr. Prepared head SHA: 8542ac77ae183e19a0700c3bb0304ab06bb7d568 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras --- CHANGELOG.md | 1 + src/cli/program/message/helpers.test.ts | 82 +++++++++++++++++++++++++ src/cli/program/message/helpers.ts | 16 +++-- src/gateway/server.impl.ts | 20 ++---- src/plugins/hook-runner-global.ts | 21 +++++++ 5 files changed, 121 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf944e1126..e4caf037dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,7 @@ Docs: https://docs.openclaw.ai - 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. +- CLI/Plugins: run registered plugin `gateway_stop` hooks before `openclaw message` exits (success and failure paths), so plugin-backed channels can clean up one-shot CLI resources. (#16580) Thanks @gumadeiras. - 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 index 5e5ed5c17b..7188c26b4f 100644 --- a/src/cli/program/message/helpers.test.ts +++ b/src/cli/program/message/helpers.test.ts @@ -14,6 +14,28 @@ vi.mock("../../plugin-registry.js", () => ({ ensurePluginRegistryLoaded: vi.fn(), })); +const hasHooksMock = vi.fn(() => false); +const runGatewayStopMock = vi.fn(async () => {}); +const runGlobalGatewayStopSafelyMock = vi.fn( + async (params: { + event: { reason?: string }; + ctx: Record; + onError?: (err: unknown) => void; + }) => { + if (!hasHooksMock("gateway_stop")) { + return; + } + try { + await runGatewayStopMock(params.event, params.ctx); + } catch (err) { + params.onError?.(err); + } + }, +); +vi.mock("../../../plugins/hook-runner-global.js", () => ({ + runGlobalGatewayStopSafely: (...args: unknown[]) => runGlobalGatewayStopSafelyMock(...args), +})); + const exitMock = vi.fn((): never => { throw new Error("exit"); }); @@ -33,6 +55,9 @@ describe("runMessageAction", () => { beforeEach(() => { vi.clearAllMocks(); messageCommandMock.mockReset().mockResolvedValue(undefined); + hasHooksMock.mockReset().mockReturnValue(false); + runGatewayStopMock.mockReset().mockResolvedValue(undefined); + runGlobalGatewayStopSafelyMock.mockClear(); exitMock.mockReset().mockImplementation((): never => { throw new Error("exit"); }); @@ -50,6 +75,19 @@ describe("runMessageAction", () => { expect(exitMock).toHaveBeenCalledWith(0); }); + it("runs gateway_stop hooks before exit when registered", async () => { + hasHooksMock.mockReturnValueOnce(true); + 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(runGatewayStopMock).toHaveBeenCalledWith({ reason: "cli message action complete" }, {}); + 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; @@ -64,6 +102,50 @@ describe("runMessageAction", () => { expect(exitMock).toHaveBeenCalledWith(1); }); + it("runs gateway_stop hooks on failure before exit(1)", async () => { + hasHooksMock.mockReturnValueOnce(true); + 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(runGatewayStopMock).toHaveBeenCalledWith({ reason: "cli message action complete" }, {}); + expect(exitMock).toHaveBeenCalledWith(1); + }); + + it("logs gateway_stop failure and still exits with success code", async () => { + hasHooksMock.mockReturnValueOnce(true); + runGatewayStopMock.mockRejectedValueOnce(new Error("hook 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("gateway_stop hook failed: Error: hook failed"); + expect(exitMock).toHaveBeenCalledWith(0); + }); + + it("logs gateway_stop failure and preserves failure exit code when send fails", async () => { + hasHooksMock.mockReturnValueOnce(true); + messageCommandMock.mockRejectedValueOnce(new Error("send failed")); + runGatewayStopMock.mockRejectedValueOnce(new Error("hook 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).toHaveBeenNthCalledWith(1, "Error: send failed"); + expect(errorMock).toHaveBeenNthCalledWith(2, "gateway_stop hook failed: Error: hook failed"); + 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; diff --git a/src/cli/program/message/helpers.ts b/src/cli/program/message/helpers.ts index d71d1ec8b3..cb94498e86 100644 --- a/src/cli/program/message/helpers.ts +++ b/src/cli/program/message/helpers.ts @@ -2,6 +2,7 @@ import type { Command } from "commander"; import { messageCommand } from "../../../commands/message.js"; import { danger, setVerbose } from "../../../globals.js"; import { CHANNEL_TARGET_DESCRIPTION } from "../../../infra/outbound/channel-target.js"; +import { runGlobalGatewayStopSafely } from "../../../plugins/hook-runner-global.js"; import { defaultRuntime } from "../../../runtime.js"; import { runCommandWithRuntime } from "../../cli-utils.js"; import { createDefaultDeps } from "../../deps.js"; @@ -22,6 +23,14 @@ function normalizeMessageOptions(opts: Record): Record { + await runGlobalGatewayStopSafely({ + event: { reason: "cli message action complete" }, + ctx: {}, + onError: (err) => defaultRuntime.error(danger(`gateway_stop hook failed: ${String(err)}`)), + }); +} + export function createMessageCliHelpers( message: Command, messageChannelOptions: string, @@ -59,13 +68,10 @@ export function createMessageCliHelpers( (err) => { failed = true; defaultRuntime.error(danger(String(err))); - defaultRuntime.exit(1); }, ); - if (failed) { - return; - } - defaultRuntime.exit(0); + await runPluginStopHooks(); + defaultRuntime.exit(failed ? 1 : 0); }; // `message` is only used for `message.help({ error: true })`, keep the diff --git a/src/gateway/server.impl.ts b/src/gateway/server.impl.ts index bf6746ef19..553131377e 100644 --- a/src/gateway/server.impl.ts +++ b/src/gateway/server.impl.ts @@ -44,7 +44,7 @@ import { import { scheduleGatewayUpdateCheck } from "../infra/update-startup.js"; import { startDiagnosticHeartbeat, stopDiagnosticHeartbeat } from "../logging/diagnostic.js"; import { createSubsystemLogger, runtimeForLogger } from "../logging/subsystem.js"; -import { getGlobalHookRunner } from "../plugins/hook-runner-global.js"; +import { getGlobalHookRunner, runGlobalGatewayStopSafely } from "../plugins/hook-runner-global.js"; import { getTotalQueueSize } from "../process/command-queue.js"; import { runOnboardingWizard } from "../wizard/onboarding.js"; import { createAuthRateLimiter, type AuthRateLimiter } from "./auth-rate-limit.js"; @@ -720,19 +720,11 @@ export async function startGatewayServer( return { close: async (opts) => { // Run gateway_stop plugin hook before shutdown - { - const hookRunner = getGlobalHookRunner(); - if (hookRunner?.hasHooks("gateway_stop")) { - try { - await hookRunner.runGatewayStop( - { reason: opts?.reason ?? "gateway stopping" }, - { port }, - ); - } catch (err) { - log.warn(`gateway_stop hook failed: ${String(err)}`); - } - } - } + await runGlobalGatewayStopSafely({ + event: { reason: opts?.reason ?? "gateway stopping" }, + ctx: { port }, + onError: (err) => log.warn(`gateway_stop hook failed: ${String(err)}`), + }); if (diagnosticsEnabled) { stopDiagnosticHeartbeat(); } diff --git a/src/plugins/hook-runner-global.ts b/src/plugins/hook-runner-global.ts index 28d741c79c..38e2a57ae9 100644 --- a/src/plugins/hook-runner-global.ts +++ b/src/plugins/hook-runner-global.ts @@ -6,6 +6,7 @@ */ import type { PluginRegistry } from "./registry.js"; +import type { PluginHookGatewayContext, PluginHookGatewayStopEvent } from "./types.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { createHookRunner, type HookRunner } from "./hooks.js"; @@ -58,6 +59,26 @@ export function hasGlobalHooks(hookName: Parameters[0]): return globalHookRunner?.hasHooks(hookName) ?? false; } +export async function runGlobalGatewayStopSafely(params: { + event: PluginHookGatewayStopEvent; + ctx: PluginHookGatewayContext; + onError?: (err: unknown) => void; +}): Promise { + const hookRunner = getGlobalHookRunner(); + if (!hookRunner?.hasHooks("gateway_stop")) { + return; + } + try { + await hookRunner.runGatewayStop(params.event, params.ctx); + } catch (err) { + if (params.onError) { + params.onError(err); + return; + } + log.warn(`gateway_stop hook failed: ${String(err)}`); + } +} + /** * Reset the global hook runner (for testing). */