From 60dc3741c0362a5a64adf6c12634986fa430ed7e Mon Sep 17 00:00:00 2001 From: Sascha Reuter Date: Tue, 17 Feb 2026 18:23:54 +1100 Subject: [PATCH] fix: before_tool_call hook double-fires with abort signal (#16852) Merged via /review-pr -> /prepare-pr -> /merge-pr. Prepared head SHA: 6269d617f3ac811e03cd29d915f94657da922ba1 Co-authored-by: sreuter <550246+sreuter@users.noreply.github.com> Co-authored-by: obviyus <22031114+obviyus@users.noreply.github.com> Reviewed-by: @obviyus --- CHANGELOG.md | 1 + .../pi-tools.before-tool-call.e2e.test.ts | 19 +++++++++++++++++++ src/agents/pi-tools.before-tool-call.ts | 2 +- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0674d17276..bc068c5a66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ Docs: https://docs.openclaw.ai - Agents/Tools/exec: add a preflight guard that detects likely shell env var injection (e.g. `$DM_JSON`, `$TMPDIR`) in Python/Node scripts before execution, preventing recurring cron failures and wasted tokens when models emit mixed shell+language source. (#12836) - Agents/Tools/exec: treat normal non-zero exit codes as completed and append the exit code to tool output to avoid false tool-failure warnings. (#18425) - Agents/Tools: make loop detection progress-aware and phased by hard-blocking known `process(action=poll|log)` no-progress loops, warning on generic identical-call repeats, warning + no-progress-blocking ping-pong alternation loops (10/20), coalescing repeated warning spam into threshold buckets (including canonical ping-pong pairs), adding a global circuit breaker at 30 no-progress repeats, and emitting structured diagnostic `tool.loop` warning/error events for loop actions. (#16808) Thanks @akramcodez and @beca-oc. +- Agents/Hooks: preserve the `before_tool_call` wrapped-marker across abort-signal tool wrapping so the hook runs once per tool call in normal agent sessions. (#16852) Thanks @sreuter. - Agents/Tools: scope the `message` tool schema to the active channel so Telegram uses `buttons` and Discord uses `components`. (#18215) Thanks @obviyus. - Agents/Image tool: replace Anthropic-incompatible union schema with explicit `image` (single) and `images` (multi) parameters, keeping tool schemas `anyOf`/`oneOf`/`allOf`-free while preserving multi-image analysis support. (#18551, #18566) Thanks @aldoeliacim. - Agents/Models: probe the primary model when its auth-profile cooldown is near expiry (with per-provider throttling), so runs recover from temporary rate limits without staying on fallback models until restart. (#17478) Thanks @PlayerGhost. 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 4734ddeb32..b615245bc3 100644 --- a/src/agents/pi-tools.before-tool-call.e2e.test.ts +++ b/src/agents/pi-tools.before-tool-call.e2e.test.ts @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { resetDiagnosticSessionStateForTest } from "../logging/diagnostic-session-state.js"; import { getGlobalHookRunner } from "../plugins/hook-runner-global.js"; import { toClientToolDefinitions, toToolDefinitions } from "./pi-tool-definition-adapter.js"; +import { wrapToolWithAbortSignal } from "./pi-tools.abort.js"; import { wrapToolWithBeforeToolCallHook } from "./pi-tools.before-tool-call.js"; vi.mock("../plugins/hook-runner-global.js"); @@ -162,6 +163,24 @@ describe("before_tool_call hook deduplication (#15502)", () => { expect(hookRunner.runBeforeToolCall).toHaveBeenCalledTimes(1); }); + + it("fires hook exactly once when tool goes through wrap + abort + toToolDefinitions", async () => { + const execute = vi.fn().mockResolvedValue({ content: [], details: { ok: true } }); + // oxlint-disable-next-line typescript/no-explicit-any + const baseTool = { name: "Bash", execute, description: "bash", parameters: {} } as any; + + const abortController = new AbortController(); + const wrapped = wrapToolWithBeforeToolCallHook(baseTool, { + agentId: "main", + sessionKey: "main", + }); + const withAbort = wrapToolWithAbortSignal(wrapped, abortController.signal); + const [def] = toToolDefinitions([withAbort]); + + await def.execute("call-abort-dedup", { command: "ls" }, undefined, undefined, undefined); + + expect(hookRunner.runBeforeToolCall).toHaveBeenCalledTimes(1); + }); }); describe("before_tool_call hook integration for client tools", () => { diff --git a/src/agents/pi-tools.before-tool-call.ts b/src/agents/pi-tools.before-tool-call.ts index 1198c813fc..a0a5ca4cb1 100644 --- a/src/agents/pi-tools.before-tool-call.ts +++ b/src/agents/pi-tools.before-tool-call.ts @@ -227,7 +227,7 @@ export function wrapToolWithBeforeToolCallHook( }; Object.defineProperty(wrappedTool, BEFORE_TOOL_CALL_WRAPPED, { value: true, - enumerable: false, + enumerable: true, }); return wrappedTool; }