mirror of
https://github.com/farcasclaudiu/openclaw.git
synced 2026-06-28 15:01:41 +03:00
fix: wire 9 unwired plugin hooks to core code (openclaw#14882) thanks @shtse8
Verified: - GitHub CI checks green (non-skipped) Co-authored-by: shtse8 <8020099+shtse8@users.noreply.github.com>
This commit is contained in:
Kyle Tse
@@ -0,0 +1,200 @@
|
||||
/**
|
||||
* Test: after_tool_call hook wiring (pi-embedded-subscribe.handlers.tools.ts)
|
||||
*/
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const hookMocks = vi.hoisted(() => ({
|
||||
runner: {
|
||||
hasHooks: vi.fn(() => false),
|
||||
runAfterToolCall: vi.fn(async () => {}),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("../plugins/hook-runner-global.js", () => ({
|
||||
getGlobalHookRunner: () => hookMocks.runner,
|
||||
}));
|
||||
|
||||
// Mock agent events (used by handlers)
|
||||
vi.mock("../infra/agent-events.js", () => ({
|
||||
emitAgentEvent: vi.fn(),
|
||||
}));
|
||||
|
||||
describe("after_tool_call hook wiring", () => {
|
||||
beforeEach(() => {
|
||||
hookMocks.runner.hasHooks.mockReset();
|
||||
hookMocks.runner.hasHooks.mockReturnValue(false);
|
||||
hookMocks.runner.runAfterToolCall.mockReset();
|
||||
hookMocks.runner.runAfterToolCall.mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
it("calls runAfterToolCall in handleToolExecutionEnd when hook is registered", async () => {
|
||||
hookMocks.runner.hasHooks.mockReturnValue(true);
|
||||
|
||||
const { handleToolExecutionEnd, handleToolExecutionStart } =
|
||||
await import("../agents/pi-embedded-subscribe.handlers.tools.js");
|
||||
|
||||
const ctx = {
|
||||
params: {
|
||||
runId: "test-run-1",
|
||||
session: { messages: [] },
|
||||
agentId: "main",
|
||||
sessionKey: "test-session",
|
||||
onBlockReplyFlush: undefined,
|
||||
},
|
||||
state: {
|
||||
toolMetaById: new Map<string, string | undefined>(),
|
||||
toolMetas: [] as Array<{ toolName?: string; meta?: string }>,
|
||||
toolSummaryById: new Set<string>(),
|
||||
lastToolError: undefined,
|
||||
pendingMessagingTexts: new Map<string, string>(),
|
||||
pendingMessagingTargets: new Map<string, unknown>(),
|
||||
messagingToolSentTexts: [] as string[],
|
||||
messagingToolSentTextsNormalized: [] as string[],
|
||||
messagingToolSentTargets: [] as unknown[],
|
||||
blockBuffer: "",
|
||||
},
|
||||
log: { debug: vi.fn(), warn: vi.fn() },
|
||||
flushBlockReplyBuffer: vi.fn(),
|
||||
shouldEmitToolResult: () => false,
|
||||
shouldEmitToolOutput: () => false,
|
||||
emitToolSummary: vi.fn(),
|
||||
emitToolOutput: vi.fn(),
|
||||
trimMessagingToolSent: vi.fn(),
|
||||
};
|
||||
|
||||
await handleToolExecutionStart(
|
||||
ctx as never,
|
||||
{
|
||||
type: "tool_execution_start",
|
||||
toolName: "read",
|
||||
toolCallId: "call-1",
|
||||
args: { path: "/tmp/file.txt" },
|
||||
} as never,
|
||||
);
|
||||
|
||||
handleToolExecutionEnd(
|
||||
ctx as never,
|
||||
{
|
||||
type: "tool_execution_end",
|
||||
toolName: "read",
|
||||
toolCallId: "call-1",
|
||||
isError: false,
|
||||
result: { content: [{ type: "text", text: "file contents" }] },
|
||||
} as never,
|
||||
);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(hookMocks.runner.runAfterToolCall).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
const [event, context] = hookMocks.runner.runAfterToolCall.mock.calls[0];
|
||||
expect(event.toolName).toBe("read");
|
||||
expect(event.params).toEqual({ path: "/tmp/file.txt" });
|
||||
expect(event.error).toBeUndefined();
|
||||
expect(typeof event.durationMs).toBe("number");
|
||||
expect(context.toolName).toBe("read");
|
||||
});
|
||||
|
||||
it("includes error in after_tool_call event on tool failure", async () => {
|
||||
hookMocks.runner.hasHooks.mockReturnValue(true);
|
||||
|
||||
const { handleToolExecutionEnd, handleToolExecutionStart } =
|
||||
await import("../agents/pi-embedded-subscribe.handlers.tools.js");
|
||||
|
||||
const ctx = {
|
||||
params: {
|
||||
runId: "test-run-2",
|
||||
session: { messages: [] },
|
||||
onBlockReplyFlush: undefined,
|
||||
},
|
||||
state: {
|
||||
toolMetaById: new Map<string, string | undefined>(),
|
||||
toolMetas: [] as Array<{ toolName?: string; meta?: string }>,
|
||||
toolSummaryById: new Set<string>(),
|
||||
lastToolError: undefined,
|
||||
pendingMessagingTexts: new Map<string, string>(),
|
||||
pendingMessagingTargets: new Map<string, unknown>(),
|
||||
messagingToolSentTexts: [] as string[],
|
||||
messagingToolSentTextsNormalized: [] as string[],
|
||||
messagingToolSentTargets: [] as unknown[],
|
||||
blockBuffer: "",
|
||||
},
|
||||
log: { debug: vi.fn(), warn: vi.fn() },
|
||||
flushBlockReplyBuffer: vi.fn(),
|
||||
shouldEmitToolResult: () => false,
|
||||
shouldEmitToolOutput: () => false,
|
||||
emitToolSummary: vi.fn(),
|
||||
emitToolOutput: vi.fn(),
|
||||
trimMessagingToolSent: vi.fn(),
|
||||
};
|
||||
|
||||
await handleToolExecutionStart(
|
||||
ctx as never,
|
||||
{
|
||||
type: "tool_execution_start",
|
||||
toolName: "exec",
|
||||
toolCallId: "call-err",
|
||||
args: { command: "fail" },
|
||||
} as never,
|
||||
);
|
||||
|
||||
handleToolExecutionEnd(
|
||||
ctx as never,
|
||||
{
|
||||
type: "tool_execution_end",
|
||||
toolName: "exec",
|
||||
toolCallId: "call-err",
|
||||
isError: true,
|
||||
result: { status: "error", error: "command failed" },
|
||||
} as never,
|
||||
);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(hookMocks.runner.runAfterToolCall).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
const [event] = hookMocks.runner.runAfterToolCall.mock.calls[0];
|
||||
expect(event.error).toBeDefined();
|
||||
});
|
||||
|
||||
it("does not call runAfterToolCall when no hooks registered", async () => {
|
||||
hookMocks.runner.hasHooks.mockReturnValue(false);
|
||||
|
||||
const { handleToolExecutionEnd } =
|
||||
await import("../agents/pi-embedded-subscribe.handlers.tools.js");
|
||||
|
||||
const ctx = {
|
||||
params: { runId: "r", session: { messages: [] } },
|
||||
state: {
|
||||
toolMetaById: new Map<string, string | undefined>(),
|
||||
toolMetas: [] as Array<{ toolName?: string; meta?: string }>,
|
||||
toolSummaryById: new Set<string>(),
|
||||
lastToolError: undefined,
|
||||
pendingMessagingTexts: new Map<string, string>(),
|
||||
pendingMessagingTargets: new Map<string, unknown>(),
|
||||
messagingToolSentTexts: [] as string[],
|
||||
messagingToolSentTextsNormalized: [] as string[],
|
||||
messagingToolSentTargets: [] as unknown[],
|
||||
},
|
||||
log: { debug: vi.fn(), warn: vi.fn() },
|
||||
shouldEmitToolResult: () => false,
|
||||
shouldEmitToolOutput: () => false,
|
||||
emitToolSummary: vi.fn(),
|
||||
emitToolOutput: vi.fn(),
|
||||
trimMessagingToolSent: vi.fn(),
|
||||
};
|
||||
|
||||
handleToolExecutionEnd(
|
||||
ctx as never,
|
||||
{
|
||||
type: "tool_execution_end",
|
||||
toolName: "exec",
|
||||
toolCallId: "call-2",
|
||||
isError: false,
|
||||
result: {},
|
||||
} as never,
|
||||
);
|
||||
|
||||
expect(hookMocks.runner.runAfterToolCall).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,113 @@
|
||||
/**
|
||||
* Test: before_compaction & after_compaction hook wiring
|
||||
*/
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const hookMocks = vi.hoisted(() => ({
|
||||
runner: {
|
||||
hasHooks: vi.fn(() => false),
|
||||
runBeforeCompaction: vi.fn(async () => {}),
|
||||
runAfterCompaction: vi.fn(async () => {}),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("../plugins/hook-runner-global.js", () => ({
|
||||
getGlobalHookRunner: () => hookMocks.runner,
|
||||
}));
|
||||
|
||||
vi.mock("../infra/agent-events.js", () => ({
|
||||
emitAgentEvent: vi.fn(),
|
||||
}));
|
||||
|
||||
describe("compaction hook wiring", () => {
|
||||
beforeEach(() => {
|
||||
hookMocks.runner.hasHooks.mockReset();
|
||||
hookMocks.runner.hasHooks.mockReturnValue(false);
|
||||
hookMocks.runner.runBeforeCompaction.mockReset();
|
||||
hookMocks.runner.runBeforeCompaction.mockResolvedValue(undefined);
|
||||
hookMocks.runner.runAfterCompaction.mockReset();
|
||||
hookMocks.runner.runAfterCompaction.mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
it("calls runBeforeCompaction in handleAutoCompactionStart", async () => {
|
||||
hookMocks.runner.hasHooks.mockReturnValue(true);
|
||||
|
||||
const { handleAutoCompactionStart } =
|
||||
await import("../agents/pi-embedded-subscribe.handlers.lifecycle.js");
|
||||
|
||||
const ctx = {
|
||||
params: { runId: "r1", session: { messages: [1, 2, 3] } },
|
||||
state: { compactionInFlight: false },
|
||||
log: { debug: vi.fn(), warn: vi.fn() },
|
||||
incrementCompactionCount: vi.fn(),
|
||||
ensureCompactionPromise: vi.fn(),
|
||||
};
|
||||
|
||||
handleAutoCompactionStart(ctx as never);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(hookMocks.runner.runBeforeCompaction).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
const [event] = hookMocks.runner.runBeforeCompaction.mock.calls[0];
|
||||
expect(event.messageCount).toBe(3);
|
||||
});
|
||||
|
||||
it("calls runAfterCompaction when willRetry is false", async () => {
|
||||
hookMocks.runner.hasHooks.mockReturnValue(true);
|
||||
|
||||
const { handleAutoCompactionEnd } =
|
||||
await import("../agents/pi-embedded-subscribe.handlers.lifecycle.js");
|
||||
|
||||
const ctx = {
|
||||
params: { runId: "r2", session: { messages: [1, 2] } },
|
||||
state: { compactionInFlight: true },
|
||||
log: { debug: vi.fn(), warn: vi.fn() },
|
||||
maybeResolveCompactionWait: vi.fn(),
|
||||
getCompactionCount: () => 1,
|
||||
};
|
||||
|
||||
handleAutoCompactionEnd(
|
||||
ctx as never,
|
||||
{
|
||||
type: "auto_compaction_end",
|
||||
willRetry: false,
|
||||
} as never,
|
||||
);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(hookMocks.runner.runAfterCompaction).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
const [event] = hookMocks.runner.runAfterCompaction.mock.calls[0];
|
||||
expect(event.messageCount).toBe(2);
|
||||
expect(event.compactedCount).toBe(1);
|
||||
});
|
||||
|
||||
it("does not call runAfterCompaction when willRetry is true", async () => {
|
||||
hookMocks.runner.hasHooks.mockReturnValue(true);
|
||||
|
||||
const { handleAutoCompactionEnd } =
|
||||
await import("../agents/pi-embedded-subscribe.handlers.lifecycle.js");
|
||||
|
||||
const ctx = {
|
||||
params: { runId: "r3", session: { messages: [] } },
|
||||
state: { compactionInFlight: true },
|
||||
log: { debug: vi.fn(), warn: vi.fn() },
|
||||
noteCompactionRetry: vi.fn(),
|
||||
resetForCompactionRetry: vi.fn(),
|
||||
getCompactionCount: () => 0,
|
||||
};
|
||||
|
||||
handleAutoCompactionEnd(
|
||||
ctx as never,
|
||||
{
|
||||
type: "auto_compaction_end",
|
||||
willRetry: true,
|
||||
} as never,
|
||||
);
|
||||
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
expect(hookMocks.runner.runAfterCompaction).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,64 @@
|
||||
/**
|
||||
* Test: gateway_start & gateway_stop hook wiring (server.impl.ts)
|
||||
*
|
||||
* Since startGatewayServer is heavily integrated, we test the hook runner
|
||||
* calls at the unit level by verifying the hook runner functions exist
|
||||
* and validating the integration pattern.
|
||||
*/
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { PluginRegistry } from "./registry.js";
|
||||
import { createHookRunner } from "./hooks.js";
|
||||
|
||||
function createMockRegistry(
|
||||
hooks: Array<{ hookName: string; handler: (...args: unknown[]) => unknown }>,
|
||||
): PluginRegistry {
|
||||
return {
|
||||
hooks: hooks as never[],
|
||||
typedHooks: hooks.map((h) => ({
|
||||
pluginId: "test-plugin",
|
||||
hookName: h.hookName,
|
||||
handler: h.handler,
|
||||
priority: 0,
|
||||
source: "test",
|
||||
})),
|
||||
tools: [],
|
||||
httpHandlers: [],
|
||||
httpRoutes: [],
|
||||
channelRegistrations: [],
|
||||
gatewayHandlers: {},
|
||||
cliRegistrars: [],
|
||||
services: [],
|
||||
providers: [],
|
||||
commands: [],
|
||||
} as unknown as PluginRegistry;
|
||||
}
|
||||
|
||||
describe("gateway hook runner methods", () => {
|
||||
it("runGatewayStart invokes registered gateway_start hooks", async () => {
|
||||
const handler = vi.fn();
|
||||
const registry = createMockRegistry([{ hookName: "gateway_start", handler }]);
|
||||
const runner = createHookRunner(registry);
|
||||
|
||||
await runner.runGatewayStart({ port: 18789 }, { port: 18789 });
|
||||
|
||||
expect(handler).toHaveBeenCalledWith({ port: 18789 }, { port: 18789 });
|
||||
});
|
||||
|
||||
it("runGatewayStop invokes registered gateway_stop hooks", async () => {
|
||||
const handler = vi.fn();
|
||||
const registry = createMockRegistry([{ hookName: "gateway_stop", handler }]);
|
||||
const runner = createHookRunner(registry);
|
||||
|
||||
await runner.runGatewayStop({ reason: "test shutdown" }, { port: 18789 });
|
||||
|
||||
expect(handler).toHaveBeenCalledWith({ reason: "test shutdown" }, { port: 18789 });
|
||||
});
|
||||
|
||||
it("hasHooks returns true for registered gateway hooks", () => {
|
||||
const registry = createMockRegistry([{ hookName: "gateway_start", handler: vi.fn() }]);
|
||||
const runner = createHookRunner(registry);
|
||||
|
||||
expect(runner.hasHooks("gateway_start")).toBe(true);
|
||||
expect(runner.hasHooks("gateway_stop")).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,98 @@
|
||||
/**
|
||||
* Test: message_sending & message_sent hook wiring
|
||||
*
|
||||
* Tests the hook runner methods directly since outbound delivery is deeply integrated.
|
||||
*/
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { PluginRegistry } from "./registry.js";
|
||||
import { createHookRunner } from "./hooks.js";
|
||||
|
||||
function createMockRegistry(
|
||||
hooks: Array<{ hookName: string; handler: (...args: unknown[]) => unknown }>,
|
||||
): PluginRegistry {
|
||||
return {
|
||||
hooks: hooks as never[],
|
||||
typedHooks: hooks.map((h) => ({
|
||||
pluginId: "test-plugin",
|
||||
hookName: h.hookName,
|
||||
handler: h.handler,
|
||||
priority: 0,
|
||||
source: "test",
|
||||
})),
|
||||
tools: [],
|
||||
httpHandlers: [],
|
||||
httpRoutes: [],
|
||||
channelRegistrations: [],
|
||||
gatewayHandlers: {},
|
||||
cliRegistrars: [],
|
||||
services: [],
|
||||
providers: [],
|
||||
commands: [],
|
||||
} as unknown as PluginRegistry;
|
||||
}
|
||||
|
||||
describe("message_sending hook runner", () => {
|
||||
it("runMessageSending invokes registered hooks and returns modified content", async () => {
|
||||
const handler = vi.fn().mockReturnValue({ content: "modified content" });
|
||||
const registry = createMockRegistry([{ hookName: "message_sending", handler }]);
|
||||
const runner = createHookRunner(registry);
|
||||
|
||||
const result = await runner.runMessageSending(
|
||||
{ to: "user-123", content: "original content" },
|
||||
{ channelId: "telegram" },
|
||||
);
|
||||
|
||||
expect(handler).toHaveBeenCalledWith(
|
||||
{ to: "user-123", content: "original content" },
|
||||
{ channelId: "telegram" },
|
||||
);
|
||||
expect(result?.content).toBe("modified content");
|
||||
});
|
||||
|
||||
it("runMessageSending can cancel message delivery", async () => {
|
||||
const handler = vi.fn().mockReturnValue({ cancel: true });
|
||||
const registry = createMockRegistry([{ hookName: "message_sending", handler }]);
|
||||
const runner = createHookRunner(registry);
|
||||
|
||||
const result = await runner.runMessageSending(
|
||||
{ to: "user-123", content: "blocked" },
|
||||
{ channelId: "telegram" },
|
||||
);
|
||||
|
||||
expect(result?.cancel).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("message_sent hook runner", () => {
|
||||
it("runMessageSent invokes registered hooks with success=true", async () => {
|
||||
const handler = vi.fn();
|
||||
const registry = createMockRegistry([{ hookName: "message_sent", handler }]);
|
||||
const runner = createHookRunner(registry);
|
||||
|
||||
await runner.runMessageSent(
|
||||
{ to: "user-123", content: "hello", success: true },
|
||||
{ channelId: "telegram" },
|
||||
);
|
||||
|
||||
expect(handler).toHaveBeenCalledWith(
|
||||
{ to: "user-123", content: "hello", success: true },
|
||||
{ channelId: "telegram" },
|
||||
);
|
||||
});
|
||||
|
||||
it("runMessageSent invokes registered hooks with error on failure", async () => {
|
||||
const handler = vi.fn();
|
||||
const registry = createMockRegistry([{ hookName: "message_sent", handler }]);
|
||||
const runner = createHookRunner(registry);
|
||||
|
||||
await runner.runMessageSent(
|
||||
{ to: "user-123", content: "hello", success: false, error: "timeout" },
|
||||
{ channelId: "telegram" },
|
||||
);
|
||||
|
||||
expect(handler).toHaveBeenCalledWith(
|
||||
{ to: "user-123", content: "hello", success: false, error: "timeout" },
|
||||
{ channelId: "telegram" },
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,74 @@
|
||||
/**
|
||||
* Test: session_start & session_end hook wiring
|
||||
*
|
||||
* Tests the hook runner methods directly since session init is deeply integrated.
|
||||
*/
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { PluginRegistry } from "./registry.js";
|
||||
import { createHookRunner } from "./hooks.js";
|
||||
|
||||
function createMockRegistry(
|
||||
hooks: Array<{ hookName: string; handler: (...args: unknown[]) => unknown }>,
|
||||
): PluginRegistry {
|
||||
return {
|
||||
hooks: hooks as never[],
|
||||
typedHooks: hooks.map((h) => ({
|
||||
pluginId: "test-plugin",
|
||||
hookName: h.hookName,
|
||||
handler: h.handler,
|
||||
priority: 0,
|
||||
source: "test",
|
||||
})),
|
||||
tools: [],
|
||||
httpHandlers: [],
|
||||
httpRoutes: [],
|
||||
channelRegistrations: [],
|
||||
gatewayHandlers: {},
|
||||
cliRegistrars: [],
|
||||
services: [],
|
||||
providers: [],
|
||||
commands: [],
|
||||
} as unknown as PluginRegistry;
|
||||
}
|
||||
|
||||
describe("session hook runner methods", () => {
|
||||
it("runSessionStart invokes registered session_start hooks", async () => {
|
||||
const handler = vi.fn();
|
||||
const registry = createMockRegistry([{ hookName: "session_start", handler }]);
|
||||
const runner = createHookRunner(registry);
|
||||
|
||||
await runner.runSessionStart(
|
||||
{ sessionId: "abc-123", resumedFrom: "old-session" },
|
||||
{ sessionId: "abc-123", agentId: "main" },
|
||||
);
|
||||
|
||||
expect(handler).toHaveBeenCalledWith(
|
||||
{ sessionId: "abc-123", resumedFrom: "old-session" },
|
||||
{ sessionId: "abc-123", agentId: "main" },
|
||||
);
|
||||
});
|
||||
|
||||
it("runSessionEnd invokes registered session_end hooks", async () => {
|
||||
const handler = vi.fn();
|
||||
const registry = createMockRegistry([{ hookName: "session_end", handler }]);
|
||||
const runner = createHookRunner(registry);
|
||||
|
||||
await runner.runSessionEnd(
|
||||
{ sessionId: "abc-123", messageCount: 42 },
|
||||
{ sessionId: "abc-123", agentId: "main" },
|
||||
);
|
||||
|
||||
expect(handler).toHaveBeenCalledWith(
|
||||
{ sessionId: "abc-123", messageCount: 42 },
|
||||
{ sessionId: "abc-123", agentId: "main" },
|
||||
);
|
||||
});
|
||||
|
||||
it("hasHooks returns true for registered session hooks", () => {
|
||||
const registry = createMockRegistry([{ hookName: "session_start", handler: vi.fn() }]);
|
||||
const runner = createHookRunner(registry);
|
||||
|
||||
expect(runner.hasHooks("session_start")).toBe(true);
|
||||
expect(runner.hasHooks("session_end")).toBe(false);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user