diff --git a/CHANGELOG.md b/CHANGELOG.md index a9aaa93a41..0d76c24431 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ Docs: https://docs.openclaw.ai - Discord: route autoThread replies to existing threads instead of the root channel. (#8302) Thanks @gavinbmoore, @thewilloftheshadow. - Discord/Agents: apply channel/group `historyLimit` during embedded-runner history compaction to prevent long-running channel sessions from bypassing truncation and overflowing context windows. (#11224) Thanks @shadril238. - Telegram: scope skill commands to the resolved agent for default accounts so `setMyCommands` no longer triggers `BOT_COMMANDS_TOO_MUCH` when multiple agents are configured. (#15599) +- Plugins/Hooks: fire `before_tool_call` hook exactly once per tool invocation instead of twice when tools pass through both `wrapToolWithBeforeToolCallHook` and `toToolDefinitions`. (#15635) Thanks @lailoo. - Agents/Image tool: cap image-analysis completion `maxTokens` by model capability (`min(4096, model.maxTokens)`) to avoid over-limit provider failures while still preventing truncation. (#11770) Thanks @detecti1. - TUI/Streaming: preserve richer streamed assistant text when final payload drops pre-tool-call text blocks, while keeping non-empty final payload authoritative for plain-text updates. (#15452) Thanks @TsekaLuk. - Inbound/Web UI: preserve literal `\n` sequences when normalizing inbound text so Windows paths like `C:\\Work\\nxxx\\README.md` are not corrupted. (#11547) Thanks @mcaxtr. diff --git a/src/agents/pi-tool-definition-adapter.after-tool-call.e2e.test.ts b/src/agents/pi-tool-definition-adapter.after-tool-call.e2e.test.ts index 7e7c74a35e..8cfdade34c 100644 --- a/src/agents/pi-tool-definition-adapter.after-tool-call.e2e.test.ts +++ b/src/agents/pi-tool-definition-adapter.after-tool-call.e2e.test.ts @@ -35,10 +35,6 @@ describe("pi tool definition adapter after_tool_call", () => { it("dispatches after_tool_call once on successful adapter execution", async () => { hookMocks.runner.hasHooks.mockImplementation((name: string) => name === "after_tool_call"); - hookMocks.runBeforeToolCallHook.mockResolvedValue({ - blocked: false, - params: { mode: "safe" }, - }); const tool = { name: "read", label: "Read", @@ -52,10 +48,13 @@ describe("pi tool definition adapter after_tool_call", () => { expect(result.details).toMatchObject({ ok: true }); expect(hookMocks.runner.runAfterToolCall).toHaveBeenCalledTimes(1); + // after_tool_call receives the params as passed to toToolDefinitions; + // before_tool_call adjustment happens in wrapToolWithBeforeToolCallHook + // (upstream), not inside toToolDefinitions (#15502). expect(hookMocks.runner.runAfterToolCall).toHaveBeenCalledWith( { toolName: "read", - params: { mode: "safe" }, + params: { path: "/tmp/file" }, result, }, { toolName: "read" }, diff --git a/src/agents/pi-tool-definition-adapter.ts b/src/agents/pi-tool-definition-adapter.ts index 159b12cf3c..3d6cceedf7 100644 --- a/src/agents/pi-tool-definition-adapter.ts +++ b/src/agents/pi-tool-definition-adapter.ts @@ -91,17 +91,11 @@ export function toToolDefinitions(tools: AnyAgentTool[]): ToolDefinition[] { execute: async (...args: ToolExecuteArgs): Promise> => { const { toolCallId, params, onUpdate, signal } = splitToolExecuteArgs(args); try { - // Call before_tool_call hook - const hookOutcome = await runBeforeToolCallHook({ - toolName: name, - params, - toolCallId, - }); - if (hookOutcome.blocked) { - throw new Error(hookOutcome.reason); - } - const adjustedParams = hookOutcome.params; - const result = await tool.execute(toolCallId, adjustedParams, signal, onUpdate); + // NOTE: before_tool_call hook is NOT called here — it is already + // invoked by wrapToolWithBeforeToolCallHook (applied in pi-tools.ts) + // before the tool reaches toToolDefinitions. Calling it again would + // fire the hook twice per invocation (#15502). + const result = await tool.execute(toolCallId, params, signal, onUpdate); // Call after_tool_call hook const hookRunner = getGlobalHookRunner(); @@ -110,7 +104,7 @@ export function toToolDefinitions(tools: AnyAgentTool[]): ToolDefinition[] { await hookRunner.runAfterToolCall( { toolName: name, - params: isPlainObject(adjustedParams) ? adjustedParams : {}, + params: isPlainObject(params) ? params : {}, result, }, { toolName: name }, diff --git a/src/agents/pi-tools.before-tool-call.e2e.test.ts b/src/agents/pi-tools.before-tool-call.e2e.test.ts index efc6c01104..f6a81bf1fc 100644 --- a/src/agents/pi-tools.before-tool-call.e2e.test.ts +++ b/src/agents/pi-tools.before-tool-call.e2e.test.ts @@ -1,6 +1,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { getGlobalHookRunner } from "../plugins/hook-runner-global.js"; -import { toClientToolDefinitions } from "./pi-tool-definition-adapter.js"; +import { toClientToolDefinitions, toToolDefinitions } from "./pi-tool-definition-adapter.js"; import { wrapToolWithBeforeToolCallHook } from "./pi-tools.before-tool-call.js"; vi.mock("../plugins/hook-runner-global.js"); @@ -108,6 +108,44 @@ describe("before_tool_call hook integration", () => { }); }); +describe("before_tool_call hook deduplication (#15502)", () => { + let hookRunner: { + hasHooks: ReturnType; + runBeforeToolCall: ReturnType; + }; + + beforeEach(() => { + hookRunner = { + hasHooks: vi.fn(() => true), + runBeforeToolCall: vi.fn(async () => undefined), + }; + // oxlint-disable-next-line typescript/no-explicit-any + mockGetGlobalHookRunner.mockReturnValue(hookRunner as any); + }); + + it("fires hook exactly once when tool goes through wrap + toToolDefinitions", async () => { + const execute = vi.fn().mockResolvedValue({ content: [], details: { ok: true } }); + // oxlint-disable-next-line typescript/no-explicit-any + const baseTool = { name: "web_fetch", execute, description: "fetch", parameters: {} } as any; + + const wrapped = wrapToolWithBeforeToolCallHook(baseTool, { + agentId: "main", + sessionKey: "main", + }); + const [def] = toToolDefinitions([wrapped]); + + await def.execute( + "call-dedup", + { url: "https://example.com" }, + undefined, + undefined, + undefined, + ); + + expect(hookRunner.runBeforeToolCall).toHaveBeenCalledTimes(1); + }); +}); + describe("before_tool_call hook integration for client tools", () => { let hookRunner: { hasHooks: ReturnType;