mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 06:39:38 +00:00
fix(cli): run plugin gateway_stop hooks before message exit (#16580)
Merged via /review-pr -> /prepare-pr -> /merge-pr.
Prepared head SHA: 8542ac77ae
Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com>
Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com>
Reviewed-by: @gumadeiras
This commit is contained in:
committed by
GitHub
parent
3821d74019
commit
8217d77ece
@@ -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.
|
- 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/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: 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)
|
- 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.
|
- 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.
|
- TUI: refactor searchable select list description layout and add regression coverage for ANSI-highlight width bounds.
|
||||||
|
|||||||
@@ -14,6 +14,28 @@ vi.mock("../../plugin-registry.js", () => ({
|
|||||||
ensurePluginRegistryLoaded: vi.fn(),
|
ensurePluginRegistryLoaded: vi.fn(),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
const hasHooksMock = vi.fn(() => false);
|
||||||
|
const runGatewayStopMock = vi.fn(async () => {});
|
||||||
|
const runGlobalGatewayStopSafelyMock = vi.fn(
|
||||||
|
async (params: {
|
||||||
|
event: { reason?: string };
|
||||||
|
ctx: Record<string, unknown>;
|
||||||
|
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 => {
|
const exitMock = vi.fn((): never => {
|
||||||
throw new Error("exit");
|
throw new Error("exit");
|
||||||
});
|
});
|
||||||
@@ -33,6 +55,9 @@ describe("runMessageAction", () => {
|
|||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
vi.clearAllMocks();
|
vi.clearAllMocks();
|
||||||
messageCommandMock.mockReset().mockResolvedValue(undefined);
|
messageCommandMock.mockReset().mockResolvedValue(undefined);
|
||||||
|
hasHooksMock.mockReset().mockReturnValue(false);
|
||||||
|
runGatewayStopMock.mockReset().mockResolvedValue(undefined);
|
||||||
|
runGlobalGatewayStopSafelyMock.mockClear();
|
||||||
exitMock.mockReset().mockImplementation((): never => {
|
exitMock.mockReset().mockImplementation((): never => {
|
||||||
throw new Error("exit");
|
throw new Error("exit");
|
||||||
});
|
});
|
||||||
@@ -50,6 +75,19 @@ describe("runMessageAction", () => {
|
|||||||
expect(exitMock).toHaveBeenCalledWith(0);
|
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 () => {
|
it("calls exit(1) when message delivery fails", async () => {
|
||||||
messageCommandMock.mockRejectedValueOnce(new Error("send failed"));
|
messageCommandMock.mockRejectedValueOnce(new Error("send failed"));
|
||||||
const fakeCommand = { help: vi.fn() } as never;
|
const fakeCommand = { help: vi.fn() } as never;
|
||||||
@@ -64,6 +102,50 @@ describe("runMessageAction", () => {
|
|||||||
expect(exitMock).toHaveBeenCalledWith(1);
|
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 () => {
|
it("does not call exit(0) when the action throws", async () => {
|
||||||
messageCommandMock.mockRejectedValueOnce(new Error("boom"));
|
messageCommandMock.mockRejectedValueOnce(new Error("boom"));
|
||||||
const fakeCommand = { help: vi.fn() } as never;
|
const fakeCommand = { help: vi.fn() } as never;
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ import type { Command } from "commander";
|
|||||||
import { messageCommand } from "../../../commands/message.js";
|
import { messageCommand } from "../../../commands/message.js";
|
||||||
import { danger, setVerbose } from "../../../globals.js";
|
import { danger, setVerbose } from "../../../globals.js";
|
||||||
import { CHANNEL_TARGET_DESCRIPTION } from "../../../infra/outbound/channel-target.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 { defaultRuntime } from "../../../runtime.js";
|
||||||
import { runCommandWithRuntime } from "../../cli-utils.js";
|
import { runCommandWithRuntime } from "../../cli-utils.js";
|
||||||
import { createDefaultDeps } from "../../deps.js";
|
import { createDefaultDeps } from "../../deps.js";
|
||||||
@@ -22,6 +23,14 @@ function normalizeMessageOptions(opts: Record<string, unknown>): Record<string,
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async function runPluginStopHooks(): Promise<void> {
|
||||||
|
await runGlobalGatewayStopSafely({
|
||||||
|
event: { reason: "cli message action complete" },
|
||||||
|
ctx: {},
|
||||||
|
onError: (err) => defaultRuntime.error(danger(`gateway_stop hook failed: ${String(err)}`)),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
export function createMessageCliHelpers(
|
export function createMessageCliHelpers(
|
||||||
message: Command,
|
message: Command,
|
||||||
messageChannelOptions: string,
|
messageChannelOptions: string,
|
||||||
@@ -59,13 +68,10 @@ export function createMessageCliHelpers(
|
|||||||
(err) => {
|
(err) => {
|
||||||
failed = true;
|
failed = true;
|
||||||
defaultRuntime.error(danger(String(err)));
|
defaultRuntime.error(danger(String(err)));
|
||||||
defaultRuntime.exit(1);
|
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
if (failed) {
|
await runPluginStopHooks();
|
||||||
return;
|
defaultRuntime.exit(failed ? 1 : 0);
|
||||||
}
|
|
||||||
defaultRuntime.exit(0);
|
|
||||||
};
|
};
|
||||||
|
|
||||||
// `message` is only used for `message.help({ error: true })`, keep the
|
// `message` is only used for `message.help({ error: true })`, keep the
|
||||||
|
|||||||
@@ -44,7 +44,7 @@ import {
|
|||||||
import { scheduleGatewayUpdateCheck } from "../infra/update-startup.js";
|
import { scheduleGatewayUpdateCheck } from "../infra/update-startup.js";
|
||||||
import { startDiagnosticHeartbeat, stopDiagnosticHeartbeat } from "../logging/diagnostic.js";
|
import { startDiagnosticHeartbeat, stopDiagnosticHeartbeat } from "../logging/diagnostic.js";
|
||||||
import { createSubsystemLogger, runtimeForLogger } from "../logging/subsystem.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 { getTotalQueueSize } from "../process/command-queue.js";
|
||||||
import { runOnboardingWizard } from "../wizard/onboarding.js";
|
import { runOnboardingWizard } from "../wizard/onboarding.js";
|
||||||
import { createAuthRateLimiter, type AuthRateLimiter } from "./auth-rate-limit.js";
|
import { createAuthRateLimiter, type AuthRateLimiter } from "./auth-rate-limit.js";
|
||||||
@@ -720,19 +720,11 @@ export async function startGatewayServer(
|
|||||||
return {
|
return {
|
||||||
close: async (opts) => {
|
close: async (opts) => {
|
||||||
// Run gateway_stop plugin hook before shutdown
|
// Run gateway_stop plugin hook before shutdown
|
||||||
{
|
await runGlobalGatewayStopSafely({
|
||||||
const hookRunner = getGlobalHookRunner();
|
event: { reason: opts?.reason ?? "gateway stopping" },
|
||||||
if (hookRunner?.hasHooks("gateway_stop")) {
|
ctx: { port },
|
||||||
try {
|
onError: (err) => log.warn(`gateway_stop hook failed: ${String(err)}`),
|
||||||
await hookRunner.runGatewayStop(
|
});
|
||||||
{ reason: opts?.reason ?? "gateway stopping" },
|
|
||||||
{ port },
|
|
||||||
);
|
|
||||||
} catch (err) {
|
|
||||||
log.warn(`gateway_stop hook failed: ${String(err)}`);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if (diagnosticsEnabled) {
|
if (diagnosticsEnabled) {
|
||||||
stopDiagnosticHeartbeat();
|
stopDiagnosticHeartbeat();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -6,6 +6,7 @@
|
|||||||
*/
|
*/
|
||||||
|
|
||||||
import type { PluginRegistry } from "./registry.js";
|
import type { PluginRegistry } from "./registry.js";
|
||||||
|
import type { PluginHookGatewayContext, PluginHookGatewayStopEvent } from "./types.js";
|
||||||
import { createSubsystemLogger } from "../logging/subsystem.js";
|
import { createSubsystemLogger } from "../logging/subsystem.js";
|
||||||
import { createHookRunner, type HookRunner } from "./hooks.js";
|
import { createHookRunner, type HookRunner } from "./hooks.js";
|
||||||
|
|
||||||
@@ -58,6 +59,26 @@ export function hasGlobalHooks(hookName: Parameters<HookRunner["hasHooks"]>[0]):
|
|||||||
return globalHookRunner?.hasHooks(hookName) ?? false;
|
return globalHookRunner?.hasHooks(hookName) ?? false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export async function runGlobalGatewayStopSafely(params: {
|
||||||
|
event: PluginHookGatewayStopEvent;
|
||||||
|
ctx: PluginHookGatewayContext;
|
||||||
|
onError?: (err: unknown) => void;
|
||||||
|
}): Promise<void> {
|
||||||
|
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).
|
* Reset the global hook runner (for testing).
|
||||||
*/
|
*/
|
||||||
|
|||||||
Reference in New Issue
Block a user