← Back to PRs

#17930: fix: evaluate tool_result_persist hooks lazily to avoid race condition

by TheArkifaneVashtorr open 2026-02-16 09:35 View on GitHub →
agents stale size: XS
## Problem `guardSessionManager()` in `session-tool-result-guard-wrapper.ts` captures `hookRunner?.hasHooks("tool_result_persist")` **eagerly at setup time** as a ternary condition. If plugins register their `tool_result_persist` hooks after this function runs (which is the common case since plugin loading is async), the check permanently returns `false` and the transform callback is set to `undefined`. This means `tool_result_persist` hooks registered via `api.on("tool_result_persist", ...)` **never fire**, even though they appear as registered in the hook runner and `openclaw hooks check` shows them as ready. ## Root Cause ```typescript // BEFORE: eager capture — hookRunner is evaluated once const hookRunner = getGlobalHookRunner(); const transform = hookRunner?.hasHooks("tool_result_persist") ? (message, meta) => { /* ... */ } : undefined; // <-- permanently undefined if plugins load after this ``` ## Fix Move `getGlobalHookRunner()` and `hasHooks()` inside the callback so they are evaluated lazily on each tool result: ```typescript // AFTER: lazy evaluation — checks on every tool result const transform = (message, meta) => { const hookRunner = getGlobalHookRunner(); if (!hookRunner?.hasHooks("tool_result_persist")) return undefined; return hookRunner.runToolResultPersist(/* ... */)?.message ?? message; }; ``` ## Impact - **Before:** Any plugin using `api.on("tool_result_persist", ...)` has its handler silently ignored - **After:** `tool_result_persist` hooks fire correctly regardless of plugin load order ## Testing Verified on OpenClaw 2026.2.3-1 (Linux): 1. Created a plugin registering a `tool_result_persist` hook via `api.on()` 2. With `hooks.internal.enabled: true`, the hook was registered but never called 3. After applying this patch, the hook fires on every tool result ## Related - #11229 (Plugin hooks never fire — this fixes one of the root causes) - The same eager-capture pattern may exist for other hook types (`before_tool_call`, `before_agent_start`) — worth auditing ## Notes This is a one-file, 23-line change with no behavioral difference when no `tool_result_persist` hooks are registered (the `hasHooks()` check still gates execution, just lazily). <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR fixes a race condition in `guardSessionManager()` where the `tool_result_persist` hook availability was checked eagerly at setup time. Since plugin loading is async, hooks registered after guard installation were permanently missed. The fix moves `getGlobalHookRunner()` and `hasHooks()` inside the transform callback so they are evaluated lazily on each tool result. The PR also cleans up dead `before_message_write` hook wiring that referenced a non-existent hook type. - Lazy evaluation of `getGlobalHookRunner()` and `hasHooks("tool_result_persist")` ensures plugins loaded after `guardSessionManager()` are correctly detected - Removed dead `beforeMessageWrite` hook code — the `before_message_write` hook type does not exist in the current codebase and `installSessionToolResultGuard` does not accept a `beforeMessageWriteHook` option - A previously flagged issue regarding `return undefined` on the no-hooks path (line 37) still needs to be addressed before merging — see existing thread <h3>Confidence Score: 2/5</h3> - This PR should not be merged until the previously flagged `return undefined` issue is resolved — it will silently drop tool result messages when no hooks are registered. - The core idea (lazy hook evaluation) is correct and well-motivated. However, the existing thread identified that the transform function now always exists (never `undefined`), causing `persistToolResult` to always invoke it. When no hooks are registered, `return undefined` on line 37 propagates as `undefined` through `originalAppend()`, replacing the tool result message with `undefined`. This is a functional regression for the common no-hooks case. The cleanup of dead `beforeMessageWrite` code is fine. - `src/agents/session-tool-result-guard-wrapper.ts` — the `return undefined` on line 37 needs to be changed to `return message` to preserve tool results when no hooks are registered <sub>Last reviewed commit: 1639ae3</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs