mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-06 14:01:24 +08:00
fix(agents): guard harness hook errors
This commit is contained in:
57
src/agents/harness/hook-helpers.test.ts
Normal file
57
src/agents/harness/hook-helpers.test.ts
Normal file
@@ -0,0 +1,57 @@
|
||||
// Regression tests for non-fatal agent harness tool hook failure handling.
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const hookRunnerMocks = vi.hoisted(() => ({
|
||||
getGlobalHookRunner: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../../plugins/hook-runner-global.js", () => ({
|
||||
getGlobalHookRunner: hookRunnerMocks.getGlobalHookRunner,
|
||||
}));
|
||||
|
||||
import { runAgentHarnessAfterToolCallHook } from "./hook-helpers.js";
|
||||
|
||||
function createHostileThrownValue(): unknown {
|
||||
return new Proxy(
|
||||
{},
|
||||
{
|
||||
get() {
|
||||
throw new Error("property trap");
|
||||
},
|
||||
getPrototypeOf() {
|
||||
throw new Error("prototype trap");
|
||||
},
|
||||
ownKeys() {
|
||||
throw new Error("ownKeys trap");
|
||||
},
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
describe("agent harness hook helpers", () => {
|
||||
afterEach(() => {
|
||||
hookRunnerMocks.getGlobalHookRunner.mockReset();
|
||||
});
|
||||
|
||||
it("keeps hostile after_tool_call hook failures non-fatal", async () => {
|
||||
const runAfterToolCall = vi.fn(async () => {
|
||||
throw createHostileThrownValue();
|
||||
});
|
||||
hookRunnerMocks.getGlobalHookRunner.mockReturnValue({
|
||||
hasHooks: vi.fn((hookName: string) => hookName === "after_tool_call"),
|
||||
runAfterToolCall,
|
||||
});
|
||||
|
||||
await expect(
|
||||
runAgentHarnessAfterToolCallHook({
|
||||
toolName: "demo",
|
||||
toolCallId: "call-1",
|
||||
runId: "run-1",
|
||||
sessionKey: "agent:main:session-1",
|
||||
startArgs: {},
|
||||
}),
|
||||
).resolves.toBeUndefined();
|
||||
|
||||
expect(runAfterToolCall).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -4,6 +4,7 @@
|
||||
* Harnesses use this to dispatch after-tool-call and before-message-write hooks
|
||||
* while isolating hook failures from the runtime path.
|
||||
*/
|
||||
import { formatErrorMessage } from "../../infra/errors.js";
|
||||
import { createSubsystemLogger } from "../../logging/subsystem.js";
|
||||
import { getGlobalHookRunner } from "../../plugins/hook-runner-global.js";
|
||||
import { consumeAdjustedParamsForToolCall } from "../agent-tools.before-tool-call.js";
|
||||
@@ -57,7 +58,9 @@ export async function runAgentHarnessAfterToolCallHook(params: {
|
||||
},
|
||||
);
|
||||
} catch (error) {
|
||||
log.warn(`after_tool_call hook failed: tool=${params.toolName} error=${String(error)}`);
|
||||
log.warn(
|
||||
`after_tool_call hook failed: tool=${params.toolName} error=${formatErrorMessage(error)}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -28,6 +28,28 @@ const EVENT = {
|
||||
success: true,
|
||||
};
|
||||
|
||||
function createHostileThrownValue(): unknown {
|
||||
return new Proxy(
|
||||
{},
|
||||
{
|
||||
get() {
|
||||
throw new Error("property trap");
|
||||
},
|
||||
getPrototypeOf() {
|
||||
throw new Error("prototype trap");
|
||||
},
|
||||
ownKeys() {
|
||||
throw new Error("ownKeys trap");
|
||||
},
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
async function flushMicrotasks(): Promise<void> {
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
}
|
||||
|
||||
describe("agent harness lifecycle hook helpers", () => {
|
||||
afterEach(() => {
|
||||
clearAgentHarnessFinalizeRetryBudget();
|
||||
@@ -116,6 +138,51 @@ describe("agent harness lifecycle hook helpers", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps hostile llm hook failures non-fatal", async () => {
|
||||
const hookRunner = {
|
||||
hasHooks: vi.fn((hookName: string) => hookName === "llm_input" || hookName === "llm_output"),
|
||||
runLlmInput: vi.fn(async () => {
|
||||
throw createHostileThrownValue();
|
||||
}),
|
||||
runLlmOutput: vi.fn(async () => {
|
||||
throw createHostileThrownValue();
|
||||
}),
|
||||
};
|
||||
|
||||
runAgentHarnessLlmInputHook({
|
||||
ctx: { runId: "run-1", sessionKey: "agent:main:session-1" },
|
||||
event: {},
|
||||
hookRunner: hookRunner as never,
|
||||
});
|
||||
runAgentHarnessLlmOutputHook({
|
||||
ctx: { runId: "run-1", sessionKey: "agent:main:session-1" },
|
||||
event: {},
|
||||
hookRunner: hookRunner as never,
|
||||
});
|
||||
|
||||
await flushMicrotasks();
|
||||
|
||||
expect(hookRunner.runLlmInput).toHaveBeenCalledTimes(1);
|
||||
expect(hookRunner.runLlmOutput).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("keeps hostile agent_end hook failures non-fatal", async () => {
|
||||
const hookRunner = {
|
||||
hasHooks: vi.fn((hookName: string) => hookName === "agent_end"),
|
||||
runAgentEnd: vi.fn(async () => {
|
||||
throw createHostileThrownValue();
|
||||
}),
|
||||
};
|
||||
|
||||
await expect(
|
||||
awaitAgentHarnessAgentEndHook({
|
||||
ctx: { runId: "run-1", sessionKey: "agent:main:session-1" },
|
||||
event: EVENT,
|
||||
hookRunner: hookRunner as never,
|
||||
}),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("continues when legacy hook runners advertise before_agent_finalize without a runner method", async () => {
|
||||
await expect(
|
||||
runAgentHarnessBeforeAgentFinalizeHook({
|
||||
@@ -126,6 +193,23 @@ describe("agent harness lifecycle hook helpers", () => {
|
||||
).resolves.toEqual({ action: "continue" });
|
||||
});
|
||||
|
||||
it("continues when before_agent_finalize throws a hostile value", async () => {
|
||||
const hookRunner = {
|
||||
hasHooks: vi.fn((hookName: string) => hookName === "before_agent_finalize"),
|
||||
runBeforeAgentFinalize: vi.fn(async () => {
|
||||
throw createHostileThrownValue();
|
||||
}),
|
||||
};
|
||||
|
||||
await expect(
|
||||
runAgentHarnessBeforeAgentFinalizeHook({
|
||||
ctx: { runId: "run-1", sessionKey: "agent:main:session-1" },
|
||||
event: EVENT,
|
||||
hookRunner: hookRunner as never,
|
||||
}),
|
||||
).resolves.toEqual({ action: "continue" });
|
||||
});
|
||||
|
||||
it("clears finalize retry budgets by run id", async () => {
|
||||
const hookRunner = {
|
||||
hasHooks: () => true,
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
*/
|
||||
import { createHash } from "node:crypto";
|
||||
import { normalizeOptionalString as normalizeTrimmedString } from "@openclaw/normalization-core/string-coerce";
|
||||
import { formatErrorMessage } from "../../infra/errors.js";
|
||||
import { createSubsystemLogger } from "../../logging/subsystem.js";
|
||||
import { getGlobalHookRunner } from "../../plugins/hook-runner-global.js";
|
||||
import type {
|
||||
@@ -87,7 +88,7 @@ export function runAgentHarnessLlmInputHook(params: {
|
||||
void hookRunner
|
||||
.runLlmInput(params.event, buildAgentHookContext(params.ctx))
|
||||
.catch((error: unknown) => {
|
||||
log.warn(`llm_input hook failed: ${String(error)}`);
|
||||
log.warn(`llm_input hook failed: ${formatErrorMessage(error)}`);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -104,7 +105,7 @@ export function runAgentHarnessLlmOutputHook(params: {
|
||||
void hookRunner
|
||||
.runLlmOutput(params.event, buildAgentHookContext(params.ctx))
|
||||
.catch((error: unknown) => {
|
||||
log.warn(`llm_output hook failed: ${String(error)}`);
|
||||
log.warn(`llm_output hook failed: ${formatErrorMessage(error)}`);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -122,7 +123,7 @@ async function executeAgentHarnessAgentEndHook(params: {
|
||||
const options: VoidHookRunOptions = { unrefTimeout: params.unrefTimeout ?? false };
|
||||
await hookRunner.runAgentEnd(params.event, buildAgentHookContext(params.ctx), options);
|
||||
} catch (error) {
|
||||
log.warn(`agent_end hook failed: ${String(error)}`);
|
||||
log.warn(`agent_end hook failed: ${formatErrorMessage(error)}`);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -176,7 +177,7 @@ export async function runAgentHarnessBeforeAgentFinalizeHook(params: {
|
||||
eventForNormalization,
|
||||
);
|
||||
} catch (error) {
|
||||
log.warn(`before_agent_finalize hook failed: ${String(error)}`);
|
||||
log.warn(`before_agent_finalize hook failed: ${formatErrorMessage(error)}`);
|
||||
return { action: "continue" };
|
||||
}
|
||||
}
|
||||
|
||||
101
src/agents/harness/prompt-compaction-hook-helpers.test.ts
Normal file
101
src/agents/harness/prompt-compaction-hook-helpers.test.ts
Normal file
@@ -0,0 +1,101 @@
|
||||
// Regression tests for non-fatal prompt and compaction hook failure handling.
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const hookRunnerMocks = vi.hoisted(() => ({
|
||||
getGlobalHookRunner: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../../plugins/hook-runner-global.js", () => ({
|
||||
getGlobalHookRunner: hookRunnerMocks.getGlobalHookRunner,
|
||||
}));
|
||||
|
||||
import {
|
||||
resolveAgentHarnessBeforePromptBuildResult,
|
||||
runAgentHarnessAfterCompactionHook,
|
||||
runAgentHarnessBeforeCompactionHook,
|
||||
} from "./prompt-compaction-hook-helpers.js";
|
||||
|
||||
function createHostileThrownValue(): unknown {
|
||||
return new Proxy(
|
||||
{},
|
||||
{
|
||||
get() {
|
||||
throw new Error("property trap");
|
||||
},
|
||||
getPrototypeOf() {
|
||||
throw new Error("prototype trap");
|
||||
},
|
||||
ownKeys() {
|
||||
throw new Error("ownKeys trap");
|
||||
},
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
const CTX = {
|
||||
runId: "run-1",
|
||||
sessionKey: "agent:main:session-1",
|
||||
};
|
||||
|
||||
describe("agent harness prompt and compaction hook helpers", () => {
|
||||
afterEach(() => {
|
||||
hookRunnerMocks.getGlobalHookRunner.mockReset();
|
||||
});
|
||||
|
||||
it("preserves prompt fields when prompt hooks throw hostile values", async () => {
|
||||
hookRunnerMocks.getGlobalHookRunner.mockReturnValue({
|
||||
hasHooks: vi.fn(
|
||||
(hookName: string) =>
|
||||
hookName === "before_prompt_build" || hookName === "before_agent_start",
|
||||
),
|
||||
runBeforePromptBuild: vi.fn(async () => {
|
||||
throw createHostileThrownValue();
|
||||
}),
|
||||
runBeforeAgentStart: vi.fn(async () => {
|
||||
throw createHostileThrownValue();
|
||||
}),
|
||||
});
|
||||
|
||||
await expect(
|
||||
resolveAgentHarnessBeforePromptBuildResult({
|
||||
prompt: "base prompt",
|
||||
developerInstructions: "base instructions",
|
||||
messages: [],
|
||||
ctx: CTX,
|
||||
}),
|
||||
).resolves.toEqual({
|
||||
prompt: "base prompt",
|
||||
developerInstructions: "base instructions",
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps hostile compaction hook failures non-fatal", async () => {
|
||||
hookRunnerMocks.getGlobalHookRunner.mockReturnValue({
|
||||
hasHooks: vi.fn(
|
||||
(hookName: string) => hookName === "before_compaction" || hookName === "after_compaction",
|
||||
),
|
||||
runBeforeCompaction: vi.fn(async () => {
|
||||
throw createHostileThrownValue();
|
||||
}),
|
||||
runAfterCompaction: vi.fn(async () => {
|
||||
throw createHostileThrownValue();
|
||||
}),
|
||||
});
|
||||
|
||||
await expect(
|
||||
runAgentHarnessBeforeCompactionHook({
|
||||
sessionFile: "session.jsonl",
|
||||
messages: [],
|
||||
ctx: CTX,
|
||||
}),
|
||||
).resolves.toBeUndefined();
|
||||
await expect(
|
||||
runAgentHarnessAfterCompactionHook({
|
||||
sessionFile: "session.jsonl",
|
||||
messages: [],
|
||||
compactedCount: 0,
|
||||
ctx: CTX,
|
||||
}),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -4,6 +4,7 @@
|
||||
* Harness runtimes use this to run plugin hooks around prompt construction and
|
||||
* compaction while keeping hook failures non-fatal.
|
||||
*/
|
||||
import { formatErrorMessage } from "../../infra/errors.js";
|
||||
import { createSubsystemLogger } from "../../logging/subsystem.js";
|
||||
import { getGlobalHookRunner } from "../../plugins/hook-runner-global.js";
|
||||
import type {
|
||||
@@ -47,13 +48,15 @@ export async function resolveAgentHarnessBeforePromptBuildResult(params: {
|
||||
// before_agent_start hook during the prompt-build migration window.
|
||||
const promptBuildResult = hookRunner.hasHooks("before_prompt_build")
|
||||
? await hookRunner.runBeforePromptBuild(promptEvent, hookCtx).catch((error: unknown) => {
|
||||
log.warn(`before_prompt_build hook failed: ${String(error)}`);
|
||||
log.warn(`before_prompt_build hook failed: ${formatErrorMessage(error)}`);
|
||||
return undefined;
|
||||
})
|
||||
: undefined;
|
||||
const beforeAgentStartResult = hookRunner.hasHooks("before_agent_start")
|
||||
? await hookRunner.runBeforeAgentStart(promptEvent, hookCtx).catch((error: unknown) => {
|
||||
log.warn(`deprecated before_agent_start hook failed during prompt build: ${String(error)}`);
|
||||
log.warn(
|
||||
`deprecated before_agent_start hook failed during prompt build: ${formatErrorMessage(error)}`,
|
||||
);
|
||||
return undefined;
|
||||
})
|
||||
: undefined;
|
||||
@@ -115,7 +118,7 @@ export async function runAgentHarnessBeforeCompactionHook(params: {
|
||||
buildAgentHookContext(params.ctx),
|
||||
);
|
||||
} catch (error) {
|
||||
log.warn(`before_compaction hook failed: ${String(error)}`);
|
||||
log.warn(`before_compaction hook failed: ${formatErrorMessage(error)}`);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -140,6 +143,6 @@ export async function runAgentHarnessAfterCompactionHook(params: {
|
||||
buildAgentHookContext(params.ctx),
|
||||
);
|
||||
} catch (error) {
|
||||
log.warn(`after_compaction hook failed: ${String(error)}`);
|
||||
log.warn(`after_compaction hook failed: ${formatErrorMessage(error)}`);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user