← Back to PRs

#16028: feat/before-tool-result

by ambushalgorithm open 2026-02-14 05:54 View on GitHub →
agents size: XL
### AI/Vibe-Coded Disclosure 🤖 - [x] **AI-assisted:** Built with Kimi K2.5 (via Ollama Cloud) + OpenClaw v2026.2.4 - [x] **Testing level:** Fully tested (28 unit/integration tests, lint/format verified) - [ ] **Prompts/session logs:** Available on request (Discord conversation history) - [x] **Code understanding:** Yes — reviewed for compliance with OpenClaw standards # Summary This PR implements the **before_tool_result** hook — a new plugin hook that allows intercepting and modifying tool results after execution but before they're returned to the AI. ## What Introduces a new hook type before_tool_result that fires for every tool result (success or error), allowing plugins to: • Modify tool result content • Block results from being returned (with error message) • Inspect execution metadata (duration, params, errors) ## Why This enables powerful plugin use cases: • Result sanitization — strip sensitive data and sanitize prompt injections from tool outputs • Policy enforcement — block certain result types based on content • Result transformation — reformat or augment tool outputs • Audit/logging — capture tool results for compliance ## Technical Changes | File | Change | | --------------------------------------------------------- | --------------------------------------------------------- | | `src/plugins/types.ts` | Add `before_tool_result` to hook union + event/result types | | `src/plugins/hooks.ts` | Register hook in hook runner | | `src/agents/pi-tools.before-tool-result.ts` | New helper: `runBeforeToolResultHook()` | | `src/agents/pi-tool-definition-adapter.ts` | Integrate hook into tool execution flow | | `src/agents/pi-tools.before-tool-result.test.ts` | Unit tests for hook helper | | `src/plugins/hooks.before-tool-result.test.ts` | Hook integration tests | | `src/agents/pi-tool-definition-adapter.integration.test.ts` | Integration tests with real tool execution | | `src/test/docker-e2e.before-tool-result.test.ts` | Docker E2E tests | ## Hook API ```ts // Event passed to hook handlers { toolName: string; toolCallId: string; params: Record<string, unknown>; result: AgentToolResult<unknown>; isError: boolean; durationMs?: number; ctx?: { agentId?: string; sessionKey?: string }; } ``` ```ts // Return value from hook { content?: unknown; // Modified result content block?: boolean; // Block this result blockReason?: string; // Reason shown if blocked } ``` ## Testing • ✅ Unit tests for hook runner helper • ✅ Integration tests in tool execution adapter • ✅ Docker E2E tests with test plugin • ✅ Error handling (hook failures don't break tool execution) ## Breaking Changes None. This is an additive feature. Existing plugins without this hook are unaffected. ## Related Complements the existing before_tool_call hook — together they provide full lifecycle access to tool execution (request + response). Ready for review. 🚀 <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR adds a new `before_tool_result` plugin hook that fires after tool execution but before results are returned to the LLM, enabling plugins to modify, block, or audit tool results. The implementation follows established patterns from the existing `before_tool_call` hook, with type definitions in `src/plugins/types.ts`, hook runner registration in `src/plugins/hooks.ts`, a helper in `src/agents/pi-tools.before-tool-result.ts`, and integration into the tool execution adapter in `src/agents/pi-tool-definition-adapter.ts`. The `splitSdkTools` function is updated to accept and forward an optional `hookContext`. - **Bug: variable shadowing causes wrong tool name in error path** — In `pi-tool-definition-adapter.ts`, the `catch` block on line 166 declares `const name` (the error class name, e.g. `"Error"`), which shadows the outer `name` (the tool name). The `before_tool_result` hook invocation on line 210 then passes this error name as `toolName` instead of the actual tool name. The success path (line 124) is correct. - **Inconsistent hook ordering** — In the success path, `before_tool_result` runs before `after_tool_call`; in the error path the order is reversed. This means plugins that modify results via `before_tool_result` will see different behavior depending on success vs error. - **Callsites not yet passing `hookContext`** — The production callsites in `compact.ts:538` and `attempt.ts:540` call `splitSdkTools` without the new `hookContext` parameter, so the hook will not have agent/session context at runtime. This is not a bug since the parameter is optional, but it means the context fields will always be `undefined` until these callsites are updated. - The new helper uses `console.warn` instead of the project's structured `createSubsystemLogger`, unlike its sibling `pi-tools.before-tool-call.ts`. <h3>Confidence Score: 2/5</h3> - The variable shadowing bug in the error path will cause incorrect tool names to be passed to the `before_tool_result` hook, which could break plugin filtering and audit logic. - Score of 2 reflects one clear logic bug (variable shadowing causing wrong tool name in error path) and one design concern (inconsistent hook ordering between success and error paths). The feature is additive and won't break existing functionality, but the bugs in the new code should be fixed before merge. - `src/agents/pi-tool-definition-adapter.ts` needs attention for the variable shadowing bug on line 166/210 and the hook ordering inconsistency. <sub>Last reviewed commit: a346314</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs