#17930: fix: evaluate tool_result_persist hooks lazily to avoid race condition
agents
stale
size: XS
Cluster:
Plugin Hook Enhancements
## 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
#19422: fix: pass session context to plugin tool hooks in toToolDefinitions
by namabile · 2026-02-17
82.7%
#17346: feat(hooks): add message_persist hook for all transcript messages
by clawee-vanguard · 2026-02-15
82.1%
#16028: feat/before-tool-result
by ambushalgorithm · 2026-02-14
79.3%
#11778: fix(plugins): enforce monotonic hook deny merges
by coygeek · 2026-02-08
79.2%
#11071: Plugins: add tool_result_received hook for output interception
by ThomasLWang · 2026-02-07
78.5%
#9011: fix(session): auto-recovery for corrupted tool responses [AI-assisted]
by cheenu1092-oss · 2026-02-04
78.3%
#20580: feat(hooks): bridge after_tool_call to internal hook handler system
by CryptoKrad · 2026-02-19
77.9%
#23019: fix(hooks): use globalThis singleton for internal hooks handlers Map
by karmafeast · 2026-02-21
77.8%
#19094: Fix empty tool_call_id and function names in provider transcript pa...
by yxshee · 2026-02-17
77.8%
#22011: fix(transcript): drop empty toolCallId toolResults during persisten...
by sauerdaniel · 2026-02-20
77.6%