test: dedupe plugin hook merger suites

This commit is contained in:
Peter Steinberger
2026-03-28 03:08:10 +00:00
parent 218a711d5e
commit e11a74843e
6 changed files with 96 additions and 45 deletions

View File

@@ -28,6 +28,15 @@ function expectTestOnlyArtifactsExcluded(artifacts: readonly string[]) {
}
}
function expectGeneratedPathResolution(tempRoot: string, expectedRelativePath: string) {
expect(
resolveBundledPluginGeneratedPath(tempRoot, {
source: "plugin/index.ts",
built: "plugin/index.js",
}),
).toBe(path.join(tempRoot, expectedRelativePath));
}
async function writeGeneratedMetadataModule(params: {
repoRoot: string;
outputPath?: string;
@@ -79,20 +88,10 @@ describe("bundled plugin metadata", () => {
fs.mkdirSync(path.join(tempRoot, "plugin"), { recursive: true });
fs.writeFileSync(path.join(tempRoot, "plugin", "index.ts"), "export {};\n", "utf8");
expect(
resolveBundledPluginGeneratedPath(tempRoot, {
source: "plugin/index.ts",
built: "plugin/index.js",
}),
).toBe(path.join(tempRoot, "plugin", "index.ts"));
expectGeneratedPathResolution(tempRoot, path.join("plugin", "index.ts"));
fs.writeFileSync(path.join(tempRoot, "plugin", "index.js"), "export {};\n", "utf8");
expect(
resolveBundledPluginGeneratedPath(tempRoot, {
source: "plugin/index.ts",
built: "plugin/index.js",
}),
).toBe(path.join(tempRoot, "plugin", "index.js"));
expectGeneratedPathResolution(tempRoot, path.join("plugin", "index.js"));
});
it("supports check mode for stale generated artifacts", async () => {

View File

@@ -47,6 +47,19 @@ describe("before_agent_start hook merger", () => {
return result;
};
const expectMergedBeforeAgentStart = async (
hooks: Array<{
pluginId: string;
result: PluginHookBeforeAgentStartResult;
priority?: number;
}>,
expected: Partial<PluginHookBeforeAgentStartResult>,
) => {
const result = await runWithHooks(hooks);
expect(result).toEqual(expect.objectContaining(expected));
return result;
};
const runWithHooks = async (
hooks: Array<{
pluginId: string;
@@ -101,16 +114,17 @@ describe("before_agent_start hook merger", () => {
},
],
] as const)("%s", async (_name, hookResult, expected) => {
const result = await runWithHooks([{ pluginId: "plugin-a", result: hookResult }]);
expect(result).toEqual(expect.objectContaining(expected));
await expectMergedBeforeAgentStart([{ pluginId: "plugin-a", result: hookResult }], expected);
});
it("higher-priority plugin wins for modelOverride", async () => {
const result = await runWithHooks([
{ pluginId: "low-priority", result: { modelOverride: "gpt-5.4" }, priority: 1 },
{ pluginId: "high-priority", result: { modelOverride: "llama3.3:8b" }, priority: 10 },
]);
const result = await expectMergedBeforeAgentStart(
[
{ pluginId: "low-priority", result: { modelOverride: "gpt-5.4" }, priority: 1 },
{ pluginId: "high-priority", result: { modelOverride: "llama3.3:8b" }, priority: 10 },
],
{ modelOverride: "llama3.3:8b" },
);
expect(result?.modelOverride).toBe("llama3.3:8b");
});
@@ -154,12 +168,7 @@ describe("before_agent_start hook merger", () => {
});
it("backward compat: plugin returning only prependContext produces no modelOverride", async () => {
addBeforeAgentStartHook(registry, "legacy-plugin", () => ({
prependContext: "legacy context",
}));
const runner = createHookRunner(registry);
const result = await runner.runBeforeAgentStart({ prompt: "hello" }, stubCtx);
const result = await runWithSingleHook({ prependContext: "legacy context" });
expect(result?.prependContext).toBe("legacy context");
expect(result?.modelOverride).toBeUndefined();

View File

@@ -41,6 +41,23 @@ async function runBeforeToolCallWithHooks(
return await runner.runBeforeToolCall({ toolName: "bash", params: {} }, stubCtx);
}
function expectRequireApprovalResult(
result: PluginHookBeforeToolCallResult | undefined,
expected: {
block?: boolean;
blockReason?: string;
params?: Record<string, unknown>;
requireApproval?: Record<string, unknown>;
},
) {
expect(result?.block).toBe(expected.block);
expect(result?.blockReason).toBe(expected.blockReason);
expect(result?.params).toEqual(expected.params);
expect(result?.requireApproval).toEqual(
expected.requireApproval ? expect.objectContaining(expected.requireApproval) : undefined,
);
}
describe("before_tool_call hook merger — requireApproval", () => {
let registry: PluginRegistry;
@@ -139,7 +156,7 @@ describe("before_tool_call hook merger — requireApproval", () => {
},
] as const)("$name", async ({ hooks, expectedApproval }) => {
const result = await runBeforeToolCallWithHooks(registry, hooks);
expect(result?.requireApproval).toEqual(expect.objectContaining(expectedApproval));
expectRequireApprovalResult(result, { requireApproval: expectedApproval });
});
it("merges block and requireApproval from different plugins", async () => {
@@ -235,9 +252,6 @@ describe("before_tool_call hook merger — requireApproval", () => {
},
] as const)("$name", async ({ hooks, expected }) => {
const result = await runBeforeToolCallWithHooks(registry, hooks);
expect(result?.block).toBe(expected.block);
expect(result?.blockReason).toBe(expected.blockReason);
expect(result?.params).toEqual(expected.params);
expect(result?.requireApproval).toEqual(expect.objectContaining(expected.requireApproval));
expectRequireApprovalResult(result, expected);
});
});

View File

@@ -51,6 +51,19 @@ describe("phase hooks merger", () => {
return await runner.runBeforePromptBuild({ prompt: "test", messages: [] }, {});
}
async function expectPhaseHookMerge(params: {
hookName: "before_model_resolve" | "before_prompt_build";
hooks: ReadonlyArray<{
pluginId: string;
result: PluginHookBeforeModelResolveResult | PluginHookBeforePromptBuildResult;
priority?: number;
}>;
expected: Record<string, unknown>;
}) {
const result = await runPhaseHook(params);
expect(result).toEqual(expect.objectContaining(params.expected));
}
it.each([
{
name: "before_model_resolve keeps higher-priority override values",
@@ -118,7 +131,6 @@ describe("phase hooks merger", () => {
},
},
] as const)("$name", async ({ hookName, hooks, expected }) => {
const result = await runPhaseHook({ hookName, hooks });
expect(result).toEqual(expect.objectContaining(expected));
await expectPhaseHookMerge({ hookName, hooks, expected });
});
});

View File

@@ -77,6 +77,23 @@ async function runMessageSendingWithHooks(
return await runner.runMessageSending(messageEvent, messageCtx);
}
function expectTerminalHookState<
TResult extends { block?: boolean; blockReason?: string; cancel?: boolean; content?: string },
>(result: TResult | undefined, expected: Partial<TResult>) {
if ("block" in expected) {
expect(result?.block).toBe(expected.block);
}
if ("blockReason" in expected) {
expect(result?.blockReason).toBe(expected.blockReason);
}
if ("cancel" in expected) {
expect(result?.cancel).toBe(expected.cancel);
}
if ("content" in expected) {
expect(result?.content).toBe(expected.content);
}
}
describe("before_tool_call terminal block semantics", () => {
let registry: PluginRegistry;
@@ -117,11 +134,7 @@ describe("before_tool_call terminal block semantics", () => {
},
] as const)("$name", async ({ hooks, expected }) => {
const result = await runBeforeToolCallWithHooks(registry, hooks);
if (expected.block === undefined) {
expect(result?.block).toBeUndefined();
return;
}
expect(result).toEqual(expect.objectContaining(expected));
expectTerminalHookState(result, expected);
});
it("short-circuits lower-priority hooks after block=true", async () => {
@@ -223,11 +236,7 @@ describe("message_sending terminal cancel semantics", () => {
},
] as const)("$name", async ({ hooks, expected }) => {
const result = await runMessageSendingWithHooks(registry, hooks);
if (expected.cancel === undefined) {
expect(result?.cancel).toBeUndefined();
return;
}
expect(result).toEqual(expect.objectContaining(expected));
expectTerminalHookState(result, expected);
});
it("short-circuits lower-priority hooks after cancel=true", async () => {

View File

@@ -47,6 +47,16 @@ function expectServiceContext(
expect(typeof ctx.logger.error).toBe("function");
}
function expectServiceContexts(
contexts: OpenClawPluginServiceContext[],
config: Parameters<typeof startPluginServices>[0]["config"],
) {
expect(contexts).not.toHaveLength(0);
for (const ctx of contexts) {
expectServiceContext(ctx, config);
}
}
function createTrackingService(
id: string,
params: {
@@ -107,9 +117,7 @@ describe("startPluginServices", () => {
expect(starts).toEqual(["a", "b", "c"]);
expect(stops).toEqual(["c", "a"]);
expect(contexts).toHaveLength(3);
for (const ctx of contexts) {
expectServiceContext(ctx, config);
}
expectServiceContexts(contexts, config);
});
it("logs start/stop failures and continues", async () => {