Plugins: add tool_result_received hook for output interception by ThomasLWang · Pull Request #11071 · openclaw/openclaw

ThomasLWang · 2026-02-07T10:49:34Z

Summary

Add a new tool_result_received plugin hook that runs after tool execution but before results reach the agent, enabling plugins to modify or block tool outputs for security guardrails and content filtering.
Runs sequentially with async support, complementing the existing fire-and-forget after_tool_call hook.
Includes review fixes: preserve original params type (unknown), fix runModifyingHook type cast, clarify durationMs semantics.

Based on #10328 by @ThomasLWang with review feedback from greptile-apps addressed.

Test plan

pnpm build passes
pnpm check (tsgo + oxlint + oxfmt) passes
pnpm test — 219 tests pass

🤖 Generated with Claude Code

Co-Authored-By: Thomas thomas@openguardrails.com

Greptile Overview

Greptile Summary

This PR adds a new plugin lifecycle hook, tool_result_received, which runs after a tool executes but before its output is delivered back into the agent loop. Tool execution is now wrapped so plugins can (a) modify tool params pre-execution via before_tool_call and (b) modify/block tool results post-execution via the new sequential hook, while after_tool_call remains parallel fire-and-forget. The hook runner (src/plugins/hooks.ts) gains a sequential “modifying hook” runner that supports both sync and async handlers and merges results in priority order, including preserving explicit null overrides for tool results. Documentation and changelog are updated to describe the new interception point in the agent loop.

Confidence Score: 5/5

This PR is safe to merge with minimal risk.
Changes are localized to plugin hook plumbing and tool wrapping, with clear semantics, idempotent wrapping, and explicit handling for null overrides; no remaining issues from prior review threads were found in the current commit.
No files require special attention

greptile-apps

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-07T10:50:38Z

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

+      // Execute the tool
+      const result = await execute(toolCallId, beforeOutcome.params, signal, onUpdate);
+      // Pure tool execution time (excludes hook overhead)


Hook skips tool errors

tool_result_received is declared with error?: string (PluginHookToolResultReceivedEvent), but this wrapper only calls the hook after a successful await execute(...). If the tool throws, the hook never runs, and even on success error is always undefined because it’s never passed. This makes it impossible for plugins to filter/block error outputs via this hook. Consider either (a) running the hook in a try/catch/finally path and populating error on failure, or (b) removing error from the event contract if errors are intentionally out of scope.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/agents/pi-tools.before-tool-call.ts Line: 142:144 Comment: **Hook skips tool errors** `tool_result_received` is declared with `error?: string` (`PluginHookToolResultReceivedEvent`), but this wrapper only calls the hook after a successful `await execute(...)`. If the tool throws, the hook never runs, and even on success `error` is always `undefined` because it’s never passed. This makes it impossible for plugins to filter/block error outputs via this hook. Consider either (a) running the hook in a `try/catch/finally` path and populating `error` on failure, or (b) removing `error` from the event contract if errors are intentionally out of scope. How can I resolve this? If you propose a fix, please make it concise.

ThomasLWang · 2026-02-07T10:56:00Z

Re: Hook skips tool errors — Good catch. This hook is designed to intercept successful tool outputs only (the security use case is filtering content like emails/web pages for prompt injection, not error messages). Removed the unused error field from PluginHookToolResultReceivedEvent in 00681dd to avoid misleading plugin authors.

…law#10328) Add a new plugin hook that runs after tool execution but before results reach the agent, enabling plugins to modify or block tool outputs for security guardrails and content filtering. Fixes from review: - Pass original params (unknown) instead of normalizing to {} - Fix runModifyingHook type cast to match handler signatures - Clarify durationMs measures pure tool execution time Co-Authored-By: Thomas <ThomasLWang@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ThomasLWang · 2026-02-08T06:57:01Z

Hello, can this pr be merged?

ThomasLWang · 2026-02-10T01:15:24Z

@greptileai can you help review this?

greptile-apps

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-10T01:19:32Z

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

    execute: async (toolCallId, params, signal, onUpdate) => {
-      const outcome = await runBeforeToolCallHook({
+      const startTime = Date.now();
+
+      // Before hook - can modify params or block the call
+      const beforeOutcome = await runBeforeToolCallHook({
        toolName,
        params,
        toolCallId,
        ctx,
      });
-      if (outcome.blocked) {
-        throw new Error(outcome.reason);
+      if (beforeOutcome.blocked) {
+        throw new Error(beforeOutcome.reason);
+      }
+
+      // Execute the tool
+      const result = await execute(toolCallId, beforeOutcome.params, signal, onUpdate);
+      // Pure tool execution time (excludes hook overhead)
+      const durationMs = Date.now() - startTime;
+
+      // After hook - can modify result or block it
+      const afterOutcome = await runToolResultReceivedHook({
+        toolName,
+        params: beforeOutcome.params,
+        result,
+        toolCallId,
+        ctx,
+        durationMs,
+      });


Incorrect durationMs semantics

durationMs is described as “Pure tool execution time (excludes hook overhead)”, but it’s measured from startTime taken before runBeforeToolCallHook(...), so it includes before-hook time (and any other overhead prior to execute(...)). This makes the tool_result_received event’s durationMs inaccurate for plugins that rely on it (e.g., latency guardrails). Consider starting the timer immediately before execute(...) (or explicitly documenting that durationMs includes before-hook time).

Prompt To Fix With AI

This is a comment left during a code review. Path: src/agents/pi-tools.before-tool-call.ts Line: 128:155 Comment: **Incorrect `durationMs` semantics** `durationMs` is described as “Pure tool execution time (excludes hook overhead)”, but it’s measured from `startTime` taken *before* `runBeforeToolCallHook(...)`, so it includes before-hook time (and any other overhead prior to `execute(...)`). This makes the `tool_result_received` event’s `durationMs` inaccurate for plugins that rely on it (e.g., latency guardrails). Consider starting the timer immediately before `execute(...)` (or explicitly documenting that `durationMs` includes before-hook time). How can I resolve this? If you propose a fix, please make it concise.

Move startTime capture to immediately before tool execution (after before-hook completes) to ensure durationMs accurately represents pure tool execution time, excluding before-hook overhead. Addresses review feedback from greptile-apps on PR openclaw#11071.

ThomasLWang · 2026-02-10T08:50:50Z

Fixed the durationMs timing issue in commit dd15753.

The startTime is now captured immediately before tool execution (after the before-hook completes), ensuring durationMs accurately represents pure tool execution time, excluding hook overhead.

Changes:

Moved const startTime = Date.now(); to line 142 (immediately before execute())
Previously it was at line 129 (before runBeforeToolCallHook())

✅ All tests pass (239 tests)
✅ Build and type-check pass

The fix is now on main and addresses the review feedback from @greptile-apps.

greptile-apps · 2026-02-10T08:51:13Z

Perfect! The timing fix looks correct. Moving startTime capture to immediately before execute() ensures durationMs accurately reflects pure tool execution time, excluding both before-hook and after-hook overhead.

The implementation now matches the documented semantics:

durationMs passed to tool_result_received represents actual tool execution duration
Plugins can rely on accurate timing for performance monitoring/guardrails
Hook overhead is properly excluded from measurements

✅ This addresses the correctness issue from the review.

- Accept upstream README changes - Regenerate contributors list with update-clawtributors script (485 entries) - Includes timing fix for tool_result_received hook

ThomasLWang · 2026-02-10T08:56:19Z

Also resolved the README merge conflict:

✅ Merged upstream main branch
✅ Resolved README contributors list conflict
✅ Regenerated contributors using update-clawtributors.ts (485 entries)

The fork is now up to date with upstream and includes all fixes.

ThomasLWang · 2026-02-10T10:35:24Z

@greptileai can you help review this ?

greptile-apps

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-10T10:36:57Z

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

    execute: async (toolCallId, params, signal, onUpdate) => {
-      const outcome = await runBeforeToolCallHook({
+      // Before hook - can modify params or block the call
+      const beforeOutcome = await runBeforeToolCallHook({
        toolName,


Double-wrap causes hooks twice

wrapToolWithBeforeToolCallHook always wraps tool.execute with hook logic and returns a new object. If this wrapper is applied more than once to the same tool instance (e.g., tools are composed/registered in multiple places), hooks will run multiple times per call and may block/modify results unexpectedly. Consider marking wrapped tools (e.g., via a Symbol/flag) and returning early when already wrapped to make wrapping idempotent.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/agents/pi-tools.before-tool-call.ts Line: 125:128 Comment: **Double-wrap causes hooks twice** `wrapToolWithBeforeToolCallHook` always wraps `tool.execute` with hook logic and returns a new object. If this wrapper is applied more than once to the same tool instance (e.g., tools are composed/registered in multiple places), hooks will run multiple times per call and may block/modify results unexpectedly. Consider marking wrapped tools (e.g., via a Symbol/flag) and returning early when already wrapped to make wrapping idempotent. How can I resolve this? If you propose a fix, please make it concise.

Add Symbol-based marker to prevent double-wrapping of tools with hooks. Multiple calls to wrapToolWithBeforeToolCallHook now return early if the tool is already wrapped, preventing hooks from running multiple times. Addresses review feedback from greptile-apps on PR openclaw#11071.

ThomasLWang · 2026-02-10T10:44:13Z

Also fixed the double-wrap issue:

Issue: wrapToolWithBeforeToolCallHook could wrap the same tool multiple times, causing hooks to run multiple times per call.

Solution: Added Symbol-based marker (HOOK_WRAPPED_SYMBOL) to make wrapping idempotent:

Tools are marked when wrapped
Function returns early if tool is already wrapped
Prevents duplicate hook execution

All three review issues are now resolved:
✅ Timing fix (durationMs accuracy)
✅ README conflicts resolved
✅ Double-wrap prevention (idempotent wrapping)

Commits:

dd15753: timing fix
ec4c833: idempotent wrapping

ThomasLWang · 2026-02-10T12:15:27Z

@greptileai code updated, please help to review it.

greptile-apps

_{2 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-10T12:18:46Z

src/plugins/hooks.ts

    for (const hook of hooks) {
      try {
        const handlerResult = await (
-          hook.handler as (event: unknown, ctx: unknown) => Promise<TResult>
+          hook.handler as (event: unknown, ctx: unknown) => Promise<TResult | void>
        )(event, ctx);


Dropping sync hook returns

runModifyingHook casts handlers as async-only ((event, ctx) => Promise<TResult | void>), so any hook handler that returns a plain object synchronously (which the PluginHookHandlerMap allows for modifying hooks like before_tool_call and tool_result_received) will be ignored because await on a non-Promise yields the value but the cast prevents non-Promise call sites in TS and can mask incorrect handler invocation types. This should call handlers as possibly-sync (Promise<TResult|void> | TResult | void) like the map declares, otherwise synchronous plugin hooks won’t be able to modify/block anything.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/plugins/hooks.ts Line: 152:156 Comment: **Dropping sync hook returns** `runModifyingHook` casts handlers as async-only (`(event, ctx) => Promise<TResult | void>`), so any hook handler that returns a plain object synchronously (which the `PluginHookHandlerMap` allows for modifying hooks like `before_tool_call` and `tool_result_received`) will be ignored because `await` on a non-Promise yields the value but the cast prevents non-Promise call sites in TS and can mask incorrect handler invocation types. This should call handlers as possibly-sync (`Promise<TResult|void> | TResult | void`) like the map declares, otherwise synchronous plugin hooks won’t be able to modify/block anything. How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-10T12:18:47Z

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

+      // After hook - can modify result or block it
+      const afterOutcome = await runToolResultReceivedHook({
+        toolName,
+        params: beforeOutcome.params,
+        result,
+        toolCallId,
+        ctx,
+        durationMs,
+      });
+      if (afterOutcome.blocked) {
+        throw new Error(afterOutcome.reason);
+      }
+
+      // oxlint-disable-next-line typescript/no-explicit-any
+      return afterOutcome.result as any;


Null result cannot be set

In runToolResultReceivedHook, the wrapper only applies a modified result when hookResult?.result !== undefined. That makes it impossible for a plugin to intentionally replace a tool result with null (or explicitly clear it) because null will be treated the same as “no override” and the original result will pass through. If null is a valid/desired override value for redaction, you’ll need a different sentinel (e.g., check for property existence rather than value).

Prompt To Fix With AI

This is a comment left during a code review. Path: src/agents/pi-tools.before-tool-call.ts Line: 150:164 Comment: **Null result cannot be set** In `runToolResultReceivedHook`, the wrapper only applies a modified result when `hookResult?.result !== undefined`. That makes it impossible for a plugin to intentionally replace a tool result with `null` (or explicitly clear it) because `null` will be treated the same as “no override” and the original result will pass through. If `null` is a valid/desired override value for redaction, you’ll need a different sentinel (e.g., check for property existence rather than value). How can I resolve this? If you propose a fix, please make it concise.

Two fixes based on greptile-apps review: 1. Allow synchronous hook handlers: Changed runModifyingHook type cast to support both sync and async returns (Promise<T> | T | void), fixing before_tool_call and tool_result_received handlers that return plain objects synchronously. 2. Allow null as valid result override: Changed result check from 'result \!== undefined' to property existence check ('result' in obj), enabling plugins to intentionally set null results for redaction. Addresses review feedback from greptile-apps on PR openclaw#11071.

ThomasLWang · 2026-02-10T12:31:48Z

Fixed two additional issues from latest Greptile review:

1. Synchronous hook handlers now supported ✅

Issue: runModifyingHook cast handlers as async-only, ignoring synchronous returns.

Solution: Updated type cast to Promise<T> | T | void to support both sync and async handlers.

Impact: Plugins can now return plain objects synchronously from before_tool_call and tool_result_received hooks.

2. Null results now allowed ✅

Issue: Check hookResult?.result !== undefined prevented setting null as a result.

Solution: Changed to property existence check: "result" in hookResult

Impact: Plugins can now intentionally set null results for redaction/clearing.

All review issues resolved:

✅ Timing accuracy (durationMs)
✅ README conflicts
✅ Double-wrap prevention (idempotent)
✅ Sync handler support
✅ Null result support

Commit: d162fc5

ThomasLWang · 2026-02-10T12:37:16Z

@greptileai Please help review it.

greptile-apps

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-10T12:41:32Z

src/plugins/hooks.ts

+      event,
+      ctx,
+      (acc, next) => ({
+        result: next.result ?? acc?.result,
+        block: next.block ?? acc?.block,
+        blockReason: next.blockReason ?? acc?.blockReason,
+      }),


Null tool result lost

In runToolResultReceived, the merge uses result: next.result ?? acc?.result, which drops an explicit null override (and any other intentional falsy-but-not-undefined values) from a hook and falls back to the previous result. Since wrapToolWithBeforeToolCallHook now explicitly supports null overrides by checking property existence, this merge step reintroduces the inability for plugins to redact by setting result: null.

A fix is to merge based on key presence (e.g. ("result" in next) ? next.result : acc?.result) rather than nullish coalescing.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/plugins/hooks.ts Line: 331:337 Comment: **Null tool result lost** In `runToolResultReceived`, the merge uses `result: next.result ?? acc?.result`, which drops an explicit `null` override (and any other intentional falsy-but-not-undefined values) from a hook and falls back to the previous result. Since `wrapToolWithBeforeToolCallHook` now explicitly supports `null` overrides by checking property existence, this merge step reintroduces the inability for plugins to redact by setting `result: null`. A fix is to merge based on key presence (e.g. `("result" in next) ? next.result : acc?.result`) rather than nullish coalescing. How can I resolve this? If you propose a fix, please make it concise.

Changed merge logic from nullish coalescing (??) to property existence check ('result' in next) to preserve explicit null overrides from hooks. Without this fix, hooks that intentionally set result: null for redaction would have their null values dropped and replaced with the accumulated result, defeating the purpose of the null override. Addresses review feedback from greptile-apps on PR openclaw#11071.

ThomasLWang · 2026-02-10T12:44:52Z

Fixed the merge logic issue:

Null result preservation in merge ✅

Issue: The merge function in runToolResultReceived used nullish coalescing:

result: next.result ?? acc?.result,  // ❌ Drops null

This caused explicit null overrides to be lost and replaced with the accumulated result.

Solution: Changed to property existence check:

result: "result" in next ? next.result : acc?.result,  // ✅ Preserves null

Impact: Plugins can now successfully redact tool results by setting result: null, even when multiple hooks are chained.

This completes the null-handling fix across all layers:

✅ wrapToolWithBeforeToolCallHook - property existence check
✅ runToolResultReceived merge - property existence check

Commit: af1349a

ThomasLWang · 2026-02-10T12:59:20Z

@greptileai please review

ThomasLWang · 2026-02-10T13:09:37Z

@greptileai can you merge this pr?

greptile-apps · 2026-02-10T13:10:13Z

I don't have the ability to merge pull requests. Only users with write access to the repository can merge PRs.

However, I can confirm that this PR appears ready to merge based on:

All tests passing (219 tests)
Build and checks (pnpm build, pnpm check) passing
Review feedback addressed
Greptile confidence score: 5/5 (safe to merge with minimal risk)

If you have merge permissions, you can merge it yourself through the GitHub interface. If you need someone else to merge it, you'll need to ask a maintainer or someone with write access to the openclaw/openclaw repository.

ThomasLWang · 2026-02-10T13:12:12Z

@greptileai Which maintainer do you think is suitable for merge this PR?

greptile-apps · 2026-02-10T13:13:00Z

Based on the repository's PR workflow and recent commit activity, here are the most suitable maintainers for this PR:

Primary recommendation: @quotentiroler - Most active maintainer in the recent commits with extensive work on plugin infrastructure, CI/CD improvements, and codebase refactoring. Has demonstrated deep knowledge of the agent runtime and plugin system.

Alternative recommendations:

@vignesh07 - Active maintainer with recent commits on memory systems and config management
@thewilloftheshadow - Active maintainer with work on Discord integrations and CI workflows
@Takhoffman - Active maintainer with focus on agent runtime and sanitization

This PR adds plugin lifecycle hooks (specifically tool_result_received) which aligns well with @quotentiroler's recent work on system architecture. The PR is tagged with the agents label, indicating it touches the agent runtime, and has already passed all checks with a 5/5 confidence score.

openclaw-barnacle · 2026-03-08T04:11:37Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

openclaw-barnacle bot added docs Improvements or additions to documentation agents Agent runtime and tooling labels Feb 7, 2026

greptile-apps bot reviewed Feb 7, 2026

View reviewed changes

Thomas and others added 3 commits February 7, 2026 19:06

README: add ThomasLWang to clawtributors

1358e41

Plugins: remove unused error field from tool_result_received event

18ee398

ThomasLWang force-pushed the main branch from 00681dd to 18ee398 Compare February 7, 2026 11:06

fix(lint): preserve caught error cause in memory-lancedb

157d50c

openclaw-barnacle bot added the extensions: memory-lancedb Extension: memory-lancedb label Feb 7, 2026

Merge branch 'main' into main

713f147

openclaw-barnacle bot removed the extensions: memory-lancedb Extension: memory-lancedb label Feb 8, 2026

Reapor-Yurnero mentioned this pull request Feb 9, 2026

feat(gateway): support modular guardrails extensions for securing against indirect prompt injections and other agentic threats #6095

Closed

greptile-apps bot reviewed Feb 10, 2026

View reviewed changes

Merge upstream main and resolve README conflicts

bcafeda

- Accept upstream README changes - Regenerate contributors list with update-clawtributors script (485 entries) - Includes timing fix for tool_result_received hook

greptile-apps bot reviewed Feb 10, 2026

View reviewed changes

thewilloftheshadow force-pushed the main branch from bfc1ccb to f92900f Compare February 15, 2026 18:46

openclaw-barnacle bot added the stale Marked as stale due to inactivity label Mar 8, 2026

Uh oh!

Conversation

ThomasLWang commented Feb 7, 2026 • edited by greptile-apps bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Greptile Overview

Greptile Summary

Confidence Score: 5/5

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

ThomasLWang commented Feb 7, 2026

Uh oh!

ThomasLWang commented Feb 8, 2026

Uh oh!

ThomasLWang commented Feb 10, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

ThomasLWang commented Feb 10, 2026

Uh oh!

greptile-apps bot commented Feb 10, 2026

Uh oh!

ThomasLWang commented Feb 10, 2026

Uh oh!

ThomasLWang commented Feb 10, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

ThomasLWang commented Feb 10, 2026

Uh oh!

ThomasLWang commented Feb 10, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

ThomasLWang commented Feb 10, 2026

1. Synchronous hook handlers now supported ✅

2. Null results now allowed ✅

Uh oh!

ThomasLWang commented Feb 10, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

ThomasLWang commented Feb 10, 2026

Null result preservation in merge ✅

Uh oh!

ThomasLWang commented Feb 10, 2026

Uh oh!

ThomasLWang commented Feb 10, 2026

Uh oh!

greptile-apps bot commented Feb 10, 2026

Uh oh!

ThomasLWang commented Feb 10, 2026

Uh oh!

greptile-apps bot commented Feb 10, 2026

Uh oh!

ThomasLWang commented Feb 7, 2026 •

edited by greptile-apps bot

Loading