Skip to content

Commit 07dfdd4

Browse files
zenglingbiaozenglingbiao
authored
fix(agents): prevent duplicate before-tool-call hooks (#93009)
Prevent duplicate `before_tool_call` execution when an already wrapped tool passes through schema normalization and coding-tool assembly. Preserve the normalized schema while replacing stale wrapper context with the current agent/session/run context. Fixes #92973. Co-authored-by: zengLingbiao <[email protected]>
1 parent 96f786d commit 07dfdd4

6 files changed

Lines changed: 91 additions & 27 deletions

‎src/agents/agent-tools.before-tool-call.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1357,11 +1357,20 @@ export function rewrapToolWithBeforeToolCallHook(
13571357
wrappedContext && typeof wrappedContext === "object"
13581358
? (wrappedContext as HookContext)
13591359
: undefined;
1360-
return wrapToolWithBeforeToolCallHook(
1361-
source && typeof source === "object" ? (source as AnyAgentTool) : tool,
1362-
ctx ?? preservedContext,
1363-
options,
1364-
);
1360+
const sourceTool = source && typeof source === "object" ? (source as AnyAgentTool) : tool;
1361+
if (sourceTool === tool) {
1362+
return wrapToolWithBeforeToolCallHook(tool, ctx ?? preservedContext, options);
1363+
}
1364+
// Keep schema and metadata replacements applied after the original wrap while
1365+
// restoring the unwrapped execute function for the new hook context.
1366+
const rewrapSource: AnyAgentTool = {
1367+
...tool,
1368+
execute: sourceTool.execute,
1369+
};
1370+
delete (rewrapSource as unknown as Record<symbol, unknown>)[BEFORE_TOOL_CALL_WRAPPED];
1371+
copyPluginToolMeta(tool, rewrapSource);
1372+
copyChannelAgentToolMeta(tool as never, rewrapSource as never);
1373+
return wrapToolWithBeforeToolCallHook(rewrapSource, ctx ?? preservedContext, options);
13651374
}
13661375

13671376
/** Copy before_tool_call marker metadata when another wrapper replaces a tool. */

‎src/agents/agent-tools.create-openclaw-coding-tools.test.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { createMockPluginRegistry } from "../plugins/hooks.test-helpers.js";
2020
import "./test-helpers/fast-bash-tools.js";
2121
import "./test-helpers/fast-coding-tools.js";
2222
import "./test-helpers/fast-openclaw-tools.js";
23+
import { wrapToolWithBeforeToolCallHook } from "./agent-tools.before-tool-call.js";
2324
import { createOpenClawCodingTools } from "./agent-tools.js";
2425
import type { AuthProfileStore } from "./auth-profiles/types.js";
2526
import * as openClawPluginTools from "./openclaw-plugin-tools.js";
@@ -213,6 +214,36 @@ describe("createOpenClawCodingTools", () => {
213214
);
214215
});
215216

217+
it("re-wraps existing before_tool_call hooks once with the current context", async () => {
218+
const beforeToolCall = vi.fn();
219+
initializeGlobalHookRunner(
220+
createMockPluginRegistry([{ hookName: "before_tool_call", handler: beforeToolCall }]),
221+
);
222+
const execute = vi.fn().mockResolvedValue({ content: [], details: { ok: true } });
223+
const wrapped = wrapToolWithBeforeToolCallHook(
224+
{
225+
name: "already_wrapped",
226+
label: "Already wrapped",
227+
description: "Already wrapped tool",
228+
parameters: {},
229+
execute,
230+
},
231+
{ agentId: "main", sessionId: "session-original" },
232+
);
233+
vi.mocked(createOpenClawTools).mockReturnValueOnce([wrapped as never]);
234+
235+
const tools = createOpenClawCodingTools({ agentId: "main", sessionId: "session-new" });
236+
const tool = requireTool(tools, "already_wrapped");
237+
await requireToolExecute(tool)("call-wrapped", {});
238+
239+
expect(beforeToolCall).toHaveBeenCalledTimes(1);
240+
expect(beforeToolCall.mock.calls[0]?.[1]).toEqual(
241+
expect.objectContaining({ agentId: "main", sessionId: "session-new" }),
242+
);
243+
expect(execute).toHaveBeenCalledTimes(1);
244+
expect(tool.parameters).toEqual({ type: "object", properties: {} });
245+
});
246+
216247
it("adds Tool Search control tools when explicitly requested", () => {
217248
const tools = createOpenClawCodingTools({
218249
includeToolSearchControls: true,

‎src/agents/agent-tools.schema.test.ts‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@ import { runAgentLoop, type AgentEvent, type StreamFn } from "openclaw/plugin-sd
77
import { createAssistantMessageEventStream, validateToolArguments } from "openclaw/plugin-sdk/llm";
88
import { Type, type TSchema } from "typebox";
99
import { describe, expect, it, vi } from "vitest";
10-
import { wrapToolWithBeforeToolCallHook } from "./agent-tools.before-tool-call.js";
10+
import {
11+
isToolWrappedWithBeforeToolCallHook,
12+
testing as beforeToolCallTesting,
13+
wrapToolWithBeforeToolCallHook,
14+
} from "./agent-tools.before-tool-call.js";
1115
import {
1216
cleanToolSchemaForGemini,
1317
normalizeToolParameterSchema,
@@ -655,6 +659,19 @@ function makeTool(parameters: TSchema): AnyAgentTool {
655659
}
656660

657661
describe("normalizeToolParameters", () => {
662+
it("preserves before_tool_call wrapper metadata", () => {
663+
const source = makeTool(Type.Object({ value: Type.String() }));
664+
const hookContext = { agentId: "main", sessionId: "session-before-normalize" };
665+
const wrapped = wrapToolWithBeforeToolCallHook(source, hookContext);
666+
667+
const normalized = normalizeToolParameters(wrapped);
668+
const tagged = normalized as unknown as Record<symbol, unknown>;
669+
670+
expect(isToolWrappedWithBeforeToolCallHook(normalized)).toBe(true);
671+
expect(tagged[beforeToolCallTesting.BEFORE_TOOL_CALL_SOURCE_TOOL]).toBe(source);
672+
expect(tagged[beforeToolCallTesting.BEFORE_TOOL_CALL_HOOK_CONTEXT]).toBe(hookContext);
673+
});
674+
658675
it("normalizes truly empty schemas to type:object with properties:{} (MCP parameter-free tools)", () => {
659676
const tool: AnyAgentTool = {
660677
name: "get_flux_instance",

‎src/agents/agent-tools.schema.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
normalizeToolParameterSchema,
99
type ToolParameterSchemaOptions,
1010
} from "./agent-tools-parameter-schema.js";
11+
import { copyBeforeToolCallHookMarker } from "./agent-tools.before-tool-call.js";
1112
import type { AnyAgentTool } from "./agent-tools.types.js";
1213
import { copyChannelAgentToolMeta } from "./channel-tools.js";
1314

@@ -72,6 +73,7 @@ export function normalizeToolParameters(
7273
function preserveToolMeta(target: AnyAgentTool): AnyAgentTool {
7374
copyPluginToolMeta(tool, target);
7475
copyChannelAgentToolMeta(tool as never, target as never);
76+
copyBeforeToolCallHookMarker(tool, target);
7577
return target;
7678
}
7779
const schema =

‎src/agents/agent-tools.ts‎

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ import { resolveGatewayMessageChannel } from "../utils/message-channel.js";
3131
import { resolveAgentConfig } from "./agent-scope.js";
3232
import { wrapToolWithAbortSignal } from "./agent-tools.abort.js";
3333
import {
34+
isToolWrappedWithBeforeToolCallHook,
35+
rewrapToolWithBeforeToolCallHook,
3436
type ToolOutcomeObserver,
3537
wrapToolWithBeforeToolCallHook,
3638
} from "./agent-tools.before-tool-call.js";
@@ -1173,28 +1175,28 @@ export function createOpenClawCodingTools(options?: {
11731175
}),
11741176
);
11751177
options?.recordToolPrepStage?.("schema-normalization");
1178+
const hookContext = {
1179+
agentId,
1180+
...(options?.config ? { config: options.config } : {}),
1181+
cwd: codingRoot,
1182+
workspaceDir: workspaceRoot,
1183+
...(options?.skillsSnapshot ? { skillsSnapshot: options.skillsSnapshot } : {}),
1184+
...(sandboxRoot && allowWorkspaceWrites
1185+
? { sandbox: { root: sandboxRoot, bridge: sandboxFsBridge! } }
1186+
: {}),
1187+
sessionKey: options?.sessionKey,
1188+
sessionId: options?.sessionId,
1189+
runId: options?.runId,
1190+
channelId: options?.hookChannelId ?? options?.currentChannelId,
1191+
...(options?.trace ? { trace: options.trace } : {}),
1192+
loopDetection: resolveToolLoopDetectionConfig({ cfg: options?.config, agentId }),
1193+
onToolOutcome: options?.onToolOutcome,
1194+
};
1195+
const hookOptions = { emitDiagnostics: options?.emitBeforeToolCallDiagnostics };
11761196
const withHooks = normalized.map((tool) =>
1177-
wrapToolWithBeforeToolCallHook(
1178-
tool,
1179-
{
1180-
agentId,
1181-
...(options?.config ? { config: options.config } : {}),
1182-
cwd: codingRoot,
1183-
workspaceDir: workspaceRoot,
1184-
...(options?.skillsSnapshot ? { skillsSnapshot: options.skillsSnapshot } : {}),
1185-
...(sandboxRoot && allowWorkspaceWrites
1186-
? { sandbox: { root: sandboxRoot, bridge: sandboxFsBridge! } }
1187-
: {}),
1188-
sessionKey: options?.sessionKey,
1189-
sessionId: options?.sessionId,
1190-
runId: options?.runId,
1191-
channelId: options?.hookChannelId ?? options?.currentChannelId,
1192-
...(options?.trace ? { trace: options.trace } : {}),
1193-
loopDetection: resolveToolLoopDetectionConfig({ cfg: options?.config, agentId }),
1194-
onToolOutcome: options?.onToolOutcome,
1195-
},
1196-
{ emitDiagnostics: options?.emitBeforeToolCallDiagnostics },
1197-
),
1197+
isToolWrappedWithBeforeToolCallHook(tool)
1198+
? rewrapToolWithBeforeToolCallHook(tool, hookContext, hookOptions)
1199+
: wrapToolWithBeforeToolCallHook(tool, hookContext, hookOptions),
11981200
);
11991201
options?.recordToolPrepStage?.("tool-hooks");
12001202
const withAbort = options?.abortSignal

‎src/agents/tools/image-tool.test.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,9 @@ vi.mock("../apply-patch.js", () => ({
131131
}));
132132

133133
vi.mock("../agent-tools.before-tool-call.js", () => ({
134+
copyBeforeToolCallHookMarker: vi.fn(),
135+
isToolWrappedWithBeforeToolCallHook: vi.fn(() => false),
136+
rewrapToolWithBeforeToolCallHook: vi.fn((tool) => tool),
134137
wrapToolWithBeforeToolCallHook: vi.fn((tool) => tool),
135138
}));
136139

0 commit comments

Comments
 (0)