Summary

Fixes #19381

Plugin-registered tools that go through toToolDefinitions without being pre-wrapped by wrapToolWithBeforeToolCallHook receive undefined for both agentId and sessionKey in their before_tool_call and after_tool_call hooks, making per-session access control impossible.

Root cause

toToolDefinitions has a fallback path (line 103-108) for unwrapped tools that calls runBeforeToolCallHook without passing ctx. Additionally, after_tool_call hooks (both success and error paths) only pass { toolName } as context, never agentId/sessionKey.

Changes

• src/agents/pi-tool-definition-adapter.ts: Accept optional hookContext parameter in toToolDefinitions. Pass it to runBeforeToolCallHook for unwrapped tools and include agentId/sessionKey in all after_tool_call hook contexts.
• src/agents/pi-embedded-runner/tool-split.ts: Forward hookContext through splitSdkTools to toToolDefinitions.
• src/agents/pi-embedded-runner/run/attempt.ts: Pass sessionAgentId/sessionKey as hook context to splitSdkTools.
• src/agents/pi-embedded-runner/compact.ts: Pass sessionAgentId/sessionKey as hook context to splitSdkTools.
• src/agents/pi-tool-definition-adapter.hook-context.test.ts: 4 regression tests covering:
  • before_tool_call receives agentId/sessionKey for unwrapped tools
  • after_tool_call receives context on success
  • after_tool_call receives context on error
  • Backwards compatibility when no hookContext is provided

Test plan

• All 4 new regression tests pass
• All 395 existing agent tests pass (65 test files)
• pnpm format:check — clean
• pnpm tsgo — clean
• pnpm lint — 0 warnings, 0 errors

Greptile Summary

This PR correctly fixes the root cause described in issue #19381: plugin tools processed through toToolDefinitions without being pre-wrapped by wrapToolWithBeforeToolCallHook were receiving undefined for agentId and sessionKey in their before_tool_call and after_tool_call hooks, making per-session access control impossible. The fix threads an optional hookContext parameter through splitSdkTools → toToolDefinitions and wires it into all three hook invocation sites. Both call sites (attempt.ts and compact.ts) are updated consistently.

Key observations:

• The core fix is correct and the approach is clean and backward-compatible (optional parameter with undefined fallback).
• There is a pre-existing inconsistency now made more consequential: the after_tool_call success path passes the raw, un-normalized tool name as toolName in both event payload and context (lines 129, 134), while the error path passes normalizedName (lines 179, 184). Since plugins can now rely on toolName in the hook context for access control routing, receiving different values for the same tool depending on success vs. failure is a correctness hazard.
• The regression tests use all-lowercase tool names ("my_plugin_tool", "failing_tool"), which are unchanged by normalizeToolName, so the name vs. normalizedName discrepancy is not caught.

Confidence Score: 3/5

• Safe to merge with one logic issue that should be addressed before plugins rely on consistent toolName values in after_tool_call contexts.
• The fix correctly addresses the reported bug and is backward-compatible. However, a pre-existing inconsistency — raw name on the success path vs normalizedName on the error path in after_tool_call — becomes a real correctness hazard now that hookContext is plumbed through and plugins can perform access control using toolName. The regression tests don't cover this edge case. This warrants attention before the feature is relied upon in production plugins.
• src/agents/pi-tool-definition-adapter.ts (lines 129 and 134: success-path after_tool_call uses raw name instead of normalizedName)

_{Last reviewed commit: 4ec8a93}