fix: dedupe before_tool_call in embedded runtime (#15635) (thanks @lailoo)

CHANGELOG.md

@@ -27,7 +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.
+- Plugins/Hooks: fire `before_tool_call` hook exactly once per tool invocation in embedded runs by removing duplicate dispatch paths while preserving parameter mutation semantics. (#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.

src/agents/pi-embedded-subscribe.handlers.tools.ts

@@ -1,8 +1,5 @@
 import type { AgentEvent } from "@mariozechner/pi-agent-core";
-import type {
-  PluginHookAfterToolCallEvent,
-  PluginHookBeforeToolCallEvent,
-} from "../plugins/types.js";
+import type { PluginHookAfterToolCallEvent } from "../plugins/types.js";
 import type { EmbeddedPiSubscribeContext } from "./pi-embedded-subscribe.handlers.types.js";
 import { emitAgentEvent } from "../infra/agent-events.js";
 import { getGlobalHookRunner } from "../plugins/hook-runner-global.js";
@@ -61,20 +58,6 @@ export async function handleToolExecutionStart(
   // Track start time and args for after_tool_call hook
   toolStartData.set(toolCallId, { startTime: Date.now(), args });
 
-  // Call before_tool_call hook
-  const hookRunner = ctx.hookRunner ?? getGlobalHookRunner();
-  if (hookRunner?.hasHooks?.("before_tool_call")) {
-    try {
-      const hookEvent: PluginHookBeforeToolCallEvent = {
-        toolName,
-        params: args && typeof args === "object" ? (args as Record<string, unknown>) : {},
-      };
-      await hookRunner.runBeforeToolCall(hookEvent, { toolName });
-    } catch (err) {
-      ctx.log.debug(`before_tool_call hook failed: tool=${toolName} error=${String(err)}`);
-    }
-  }
-
   if (toolName === "read") {
     const record = args && typeof args === "object" ? (args as Record<string, unknown>) : {};
     const filePath = typeof record.path === "string" ? record.path.trim() : "";

src/agents/pi-tool-definition-adapter.after-tool-call.e2e.test.ts

@@ -7,6 +7,8 @@ const hookMocks = vi.hoisted(() => ({
     hasHooks: vi.fn(() => false),
     runAfterToolCall: vi.fn(async () => {}),
   },
+  isToolWrappedWithBeforeToolCallHook: vi.fn(() => false),
+  consumeAdjustedParamsForToolCall: vi.fn(() => undefined),
   runBeforeToolCallHook: vi.fn(async ({ params }: { params: unknown }) => ({
     blocked: false,
     params,
@@ -18,6 +20,8 @@ vi.mock("../plugins/hook-runner-global.js", () => ({
 }));
 
 vi.mock("./pi-tools.before-tool-call.js", () => ({
+  consumeAdjustedParamsForToolCall: hookMocks.consumeAdjustedParamsForToolCall,
+  isToolWrappedWithBeforeToolCallHook: hookMocks.isToolWrappedWithBeforeToolCallHook,
   runBeforeToolCallHook: hookMocks.runBeforeToolCallHook,
 }));
 
@@ -26,6 +30,10 @@ describe("pi tool definition adapter after_tool_call", () => {
     hookMocks.runner.hasHooks.mockReset();
     hookMocks.runner.runAfterToolCall.mockReset();
     hookMocks.runner.runAfterToolCall.mockResolvedValue(undefined);
+    hookMocks.isToolWrappedWithBeforeToolCallHook.mockReset();
+    hookMocks.isToolWrappedWithBeforeToolCallHook.mockReturnValue(false);
+    hookMocks.consumeAdjustedParamsForToolCall.mockReset();
+    hookMocks.consumeAdjustedParamsForToolCall.mockReturnValue(undefined);
     hookMocks.runBeforeToolCallHook.mockReset();
     hookMocks.runBeforeToolCallHook.mockImplementation(async ({ params }) => ({
       blocked: false,
@@ -35,6 +43,10 @@ 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",
@@ -48,13 +60,42 @@ 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: { path: "/tmp/file" },
+        params: { mode: "safe" },
+        result,
+      },
+      { toolName: "read" },
+    );
+  });
+
+  it("uses wrapped-tool adjusted params for after_tool_call payload", async () => {
+    hookMocks.runner.hasHooks.mockImplementation((name: string) => name === "after_tool_call");
+    hookMocks.isToolWrappedWithBeforeToolCallHook.mockReturnValue(true);
+    hookMocks.consumeAdjustedParamsForToolCall.mockReturnValue({ mode: "safe" });
+    const tool = {
+      name: "read",
+      label: "Read",
+      description: "reads",
+      parameters: {},
+      execute: vi.fn(async () => ({ content: [], details: { ok: true } })),
+    } satisfies AgentTool<unknown, unknown>;
+
+    const defs = toToolDefinitions([tool]);
+    const result = await defs[0].execute(
+      "call-ok-wrapped",
+      { path: "/tmp/file" },
+      undefined,
+      undefined,
+    );
+
+    expect(result.details).toMatchObject({ ok: true });
+    expect(hookMocks.runBeforeToolCallHook).not.toHaveBeenCalled();
+    expect(hookMocks.runner.runAfterToolCall).toHaveBeenCalledWith(
+      {
+        toolName: "read",
+        params: { mode: "safe" },
         result,
       },
       { toolName: "read" },

src/agents/pi-tool-definition-adapter.ts

@@ -8,7 +8,11 @@ import type { ClientToolDefinition } from "./pi-embedded-runner/run/params.js";
 import { logDebug, logError } from "../logger.js";
 import { getGlobalHookRunner } from "../plugins/hook-runner-global.js";
 import { isPlainObject } from "../utils.js";
-import { runBeforeToolCallHook } from "./pi-tools.before-tool-call.js";
+import {
+  consumeAdjustedParamsForToolCall,
+  isToolWrappedWithBeforeToolCallHook,
+  runBeforeToolCallHook,
+} from "./pi-tools.before-tool-call.js";
 import { normalizeToolName } from "./tool-policy.js";
 import { jsonResult } from "./tools/common.js";
 
@@ -83,6 +87,7 @@ export function toToolDefinitions(tools: AnyAgentTool[]): ToolDefinition[] {
   return tools.map((tool) => {
     const name = tool.name || "tool";
     const normalizedName = normalizeToolName(name);
+    const beforeHookWrapped = isToolWrappedWithBeforeToolCallHook(tool);
     return {
       name,
       label: tool.label ?? name,
@@ -90,12 +95,23 @@ export function toToolDefinitions(tools: AnyAgentTool[]): ToolDefinition[] {
       parameters: tool.parameters,
       execute: async (...args: ToolExecuteArgs): Promise<AgentToolResult<unknown>> => {
         const { toolCallId, params, onUpdate, signal } = splitToolExecuteArgs(args);
+        let executeParams = params;
         try {
-          // 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);
+          if (!beforeHookWrapped) {
+            const hookOutcome = await runBeforeToolCallHook({
+              toolName: name,
+              params,
+              toolCallId,
+            });
+            if (hookOutcome.blocked) {
+              throw new Error(hookOutcome.reason);
+            }
+            executeParams = hookOutcome.params;
+          }
+          const result = await tool.execute(toolCallId, executeParams, signal, onUpdate);
+          const afterParams = beforeHookWrapped
+            ? (consumeAdjustedParamsForToolCall(toolCallId) ?? executeParams)
+            : executeParams;
 
           // Call after_tool_call hook
           const hookRunner = getGlobalHookRunner();
@@ -104,7 +120,7 @@ export function toToolDefinitions(tools: AnyAgentTool[]): ToolDefinition[] {
               await hookRunner.runAfterToolCall(
                 {
                   toolName: name,
-                  params: isPlainObject(params) ? params : {},
+                  params: isPlainObject(afterParams) ? afterParams : {},
                   result,
                 },
                 { toolName: name },
@@ -128,6 +144,9 @@ export function toToolDefinitions(tools: AnyAgentTool[]): ToolDefinition[] {
           if (name === "AbortError") {
             throw err;
           }
+          if (beforeHookWrapped) {
+            consumeAdjustedParamsForToolCall(toolCallId);
+          }
           const described = describeToolExecutionError(err);
           if (described.stack && described.stack !== described.message) {
             logDebug(`tools: ${normalizedName} failed stack:\n${described.stack}`);

src/agents/pi-tools.before-tool-call.ts

@@ -12,6 +12,9 @@ type HookContext = {
 type HookOutcome = { blocked: true; reason: string } | { blocked: false; params: unknown };
 
 const log = createSubsystemLogger("agents/tools");
+const BEFORE_TOOL_CALL_WRAPPED = Symbol("beforeToolCallWrapped");
+const adjustedParamsByToolCallId = new Map<string, unknown>();
+const MAX_TRACKED_ADJUSTED_PARAMS = 1024;
 
 export async function runBeforeToolCallHook(args: {
   toolName: string;
@@ -71,7 +74,7 @@ export function wrapToolWithBeforeToolCallHook(
     return tool;
   }
   const toolName = tool.name || "tool";
-  return {
+  const wrappedTool: AnyAgentTool = {
     ...tool,
     execute: async (toolCallId, params, signal, onUpdate) => {
       const outcome = await runBeforeToolCallHook({
@@ -83,12 +86,39 @@ export function wrapToolWithBeforeToolCallHook(
       if (outcome.blocked) {
         throw new Error(outcome.reason);
       }
+      if (toolCallId) {
+        adjustedParamsByToolCallId.set(toolCallId, outcome.params);
+        if (adjustedParamsByToolCallId.size > MAX_TRACKED_ADJUSTED_PARAMS) {
+          const oldest = adjustedParamsByToolCallId.keys().next().value;
+          if (oldest) {
+            adjustedParamsByToolCallId.delete(oldest);
+          }
+        }
+      }
       return await execute(toolCallId, outcome.params, signal, onUpdate);
     },
   };
+  Object.defineProperty(wrappedTool, BEFORE_TOOL_CALL_WRAPPED, {
+    value: true,
+    enumerable: false,
+  });
+  return wrappedTool;
+}
+
+export function isToolWrappedWithBeforeToolCallHook(tool: AnyAgentTool): boolean {
+  const taggedTool = tool as unknown as Record<symbol, unknown>;
+  return taggedTool[BEFORE_TOOL_CALL_WRAPPED] === true;
+}
+
+export function consumeAdjustedParamsForToolCall(toolCallId: string): unknown {
+  const params = adjustedParamsByToolCallId.get(toolCallId);
+  adjustedParamsByToolCallId.delete(toolCallId);
+  return params;
 }
 
 export const __testing = {
+  BEFORE_TOOL_CALL_WRAPPED,
+  adjustedParamsByToolCallId,
   runBeforeToolCallHook,
   isPlainObject,
 };

src/plugins/wired-hooks-after-tool-call.e2e.test.ts

@@ -6,6 +6,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
 const hookMocks = vi.hoisted(() => ({
   runner: {
     hasHooks: vi.fn(() => false),
+    runBeforeToolCall: vi.fn(async () => {}),
     runAfterToolCall: vi.fn(async () => {}),
   },
 }));
@@ -23,6 +24,8 @@ describe("after_tool_call hook wiring", () => {
   beforeEach(() => {
     hookMocks.runner.hasHooks.mockReset();
     hookMocks.runner.hasHooks.mockReturnValue(false);
+    hookMocks.runner.runBeforeToolCall.mockReset();
+    hookMocks.runner.runBeforeToolCall.mockResolvedValue(undefined);
     hookMocks.runner.runAfterToolCall.mockReset();
     hookMocks.runner.runAfterToolCall.mockResolvedValue(undefined);
   });
@@ -84,6 +87,7 @@ describe("after_tool_call hook wiring", () => {
     );
 
     expect(hookMocks.runner.runAfterToolCall).toHaveBeenCalledTimes(1);
+    expect(hookMocks.runner.runBeforeToolCall).not.toHaveBeenCalled();
 
     const [event, context] = hookMocks.runner.runAfterToolCall.mock.calls[0];
     expect(event.toolName).toBe("read");