fix(hooks): fire before_tool_call hook exactly once per tool invocation (#15502)#15635
Merged
steipete merged 2 commits intoopenclaw:mainfrom Feb 14, 2026
Merged
Conversation
Contributor
Additional Comments (1)
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/agents/pi-tool-definition-adapter.ts
Line: 82:85
Comment:
**Missing hook enforcement**
`toToolDefinitions` no longer calls `runBeforeToolCallHook`, so any tools that reach this adapter *without* being wrapped by `wrapToolWithBeforeToolCallHook` will silently skip `before_tool_call` semantics (blocking and param adjustment). This is a functional/security behavior change that depends on all `toToolDefinitions(...)` inputs being pre-wrapped; `splitSdkTools` currently calls `toToolDefinitions(tools)` directly (`src/agents/pi-embedded-runner/tool-split.ts:15`). Please ensure all call paths constructing that `tools` array always apply `wrapToolWithBeforeToolCallHook` (or restore hook invocation here for unwrapped tools).
How can I resolve this? If you propose a fix, please make it concise. |
7374a0a to
21c7b02
Compare
steipete
added a commit
to lailoo/openclaw
that referenced
this pull request
Feb 14, 2026
21c7b02 to
ea41da6
Compare
Contributor
Closed
4 tasks
mverrilli
pushed a commit
to mverrilli/openclaw
that referenced
this pull request
Feb 14, 2026
GwonHyeok
pushed a commit
to learners-superpumped/openclaw
that referenced
this pull request
Feb 15, 2026
jiulingyun
added a commit
to jiulingyun/openclaw-cn
that referenced
this pull request
Feb 15, 2026
16 tasks
6 tasks
zooqueen
pushed a commit
to hanzoai/bot
that referenced
this pull request
Mar 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Remove the duplicate
runBeforeToolCallHookcall fromtoToolDefinitionsso thebefore_tool_callhook fires exactly once per tool invocation instead of 2–3 times.Root Cause
Tools go through two layers that both independently call
runBeforeToolCallHook:wrapToolWithBeforeToolCallHook(pi-tools.ts:460) — wrapstool.executewith hook call, includesagentId/sessionKeycontexttoToolDefinitions(pi-tool-definition-adapter.ts:95) — wraps again with another hook call, but without contextSince
tool-split.ts:15callstoToolDefinitions(tools)on tools that are already wrapped by step 1, the hook fires twice per invocation.Fix
Remove the
runBeforeToolCallHookcall fromtoToolDefinitions(layer 2). The hook is already invoked bywrapToolWithBeforeToolCallHook(layer 1) which has the fullagentId/sessionKeycontext.toClientToolDefinitionsis not changed — client tools don't go throughwrapToolWithBeforeToolCallHook, so they correctly call the hook directly.execute: async (...args) => { const { toolCallId, params, onUpdate, signal } = splitToolExecuteArgs(args); try { - const hookOutcome = await runBeforeToolCallHook({ toolName, params, toolCallId }); - if (hookOutcome.blocked) { throw new Error(hookOutcome.reason); } - const adjustedParams = hookOutcome.params; - const result = await tool.execute(toolCallId, adjustedParams, signal, onUpdate); + // before_tool_call hook is already invoked by wrapToolWithBeforeToolCallHook + // (applied in pi-tools.ts) before tools reach toToolDefinitions (#15502). + const result = await tool.execute(toolCallId, params, signal, onUpdate);Before (on main — real environment, zero mocks)
Registered a real
before_tool_callhook viainitializeGlobalHookRunnerwith a realPluginRegistry, then executed a tool through the fullwrapToolWithBeforeToolCallHook→toToolDefinitions→runBeforeToolCallHook→createHookRunner.runBeforeToolCallpipeline:After (on fix branch)
Changes
src/agents/pi-tool-definition-adapter.ts: remove duplicaterunBeforeToolCallHookcall fromtoToolDefinitionssrc/agents/pi-tools.before-tool-call.e2e.test.ts: add regression test verifying hook fires exactly once through the full wrap + toToolDefinitions pipelinesrc/agents/pi-tool-definition-adapter.after-tool-call.e2e.test.ts: updateafter_tool_calltest to reflect that params are no longer adjusted insidetoToolDefinitionsCHANGELOG.md: add entryCloses #15502