-
Notifications
You must be signed in to change notification settings - Fork 0
feat(hooks): add BeforeToolsEvent and AfterToolsEvent for batch-level tool execution #26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
… tool execution hooks Add batch-level hooks to complement existing per-tool hooks, enabling hook providers to observe and control tool execution at the batch level. This provides parity with the TypeScript SDK. - Add BeforeToolsEvent: triggered before a batch of tools execute, supports interrupts for approval workflows - Add AfterToolsEvent: triggered after all tools in a batch complete, uses reverse callback ordering - Update tool executors (sequential and concurrent) to trigger batch events - Add comprehensive test coverage for batch events and interrupts Resolves #25
| Events from the tool execution stream. | ||
| """ | ||
| # Skip batch events if no tools | ||
| if not tool_uses: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this out of the executors and do this wherever _execute is called from. Otherwise each executor has to duplicate this
I think after this, ToolExecutor should have no changes maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed! Kept the check in executors to avoid firing batch events when tool_uses is empty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. Move the logic of the firing of BeforeToolsEvent and AfterToolsEvent out of the executor - right now, this is duplicated between the two implementations which is not idea.
Do the event emitting whereever _execute is called from.
If you can't do that, tell me why
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed! Moved batch event firing to event loop. Executors now have no batch event logic.
|
|
||
| if interrupts: | ||
| # Use the first tool_use for the interrupt event (tools not executed yet) | ||
| yield ToolInterruptEvent(tool_uses[0], interrupts) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, add a new event for this - don't re-use ToolInterruptEvent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed! Created ToolsInterruptEvent for batch-level interrupts. Now using that instead.
|
/strands |
Address review feedback from PR #26: 1. Create ToolsInterruptEvent for batch-level interrupts (don't reuse ToolInterruptEvent) 2. Always fire AfterToolsEvent even when interrupted (maintain same contract as per-tool events) 3. Keep empty tool check in executors to avoid duplicate logic Changes: - Add ToolsInterruptEvent class for batch-level interrupts in types/_events.py - Update event loop to handle ToolsInterruptEvent - Update both executors to use ToolsInterruptEvent for batch interrupts - Ensure AfterToolsEvent always fires if BeforeToolsEvent fires - Update tests to reflect new contract (AfterToolsEvent fires even on interrupt) - All 233 tests passing
| def _execute( | ||
| self, | ||
| agent: "Agent", | ||
| message: Message, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove message - it's no longer needed
Same for the other one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed! Removed message parameter from _execute() signature.
| # Use ToolsInterruptEvent for batch-level interrupts | ||
| yield ToolsInterruptEvent(tool_uses, interrupts) | ||
| # Always fire AfterToolsEvent even if interrupted | ||
| after_event = AfterToolsEvent(agent=agent, message=message, tool_uses=tool_uses) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I lied, this shouldn't be fired when interrupted - sorry man, I'm the worst!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed! AfterToolsEvent now does NOT fire on batch interrupt. Only fires after successful batch completion.
| Events from the tool execution stream. | ||
| """ | ||
| # Skip batch events if no tools | ||
| if not tool_uses: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. Move the logic of the firing of BeforeToolsEvent and AfterToolsEvent out of the executor - right now, this is duplicated between the two implementations which is not idea.
Do the event emitting whereever _execute is called from.
If you can't do that, tell me why
|
/strands |
Address additional PR review feedback: 1. Move batch event firing out of executors to event loop - Eliminates duplication between sequential and concurrent executors - BeforeToolsEvent and AfterToolsEvent now fired in _handle_tool_execution 2. Remove message parameter from _execute() signature - No longer needed since batch events fire in event loop - Simplifies executor interface 3. AfterToolsEvent does NOT fire on batch-level interrupt - Only fires if BeforeToolsEvent completes without interrupt - Per-tool interrupts still allow AfterToolsEvent to fire All 233 tests passing with clean executor implementations.
Motivation
Hook providers need the ability to observe and control tool execution at the batch level, not just for individual tools. This enables use cases like batch approval workflows where a user must approve all tool calls before any execute, audit logging of tool batches, and batch-level resource management. Without batch-level hooks, hook providers must track state across individual tool callbacks or miss batch-level information entirely.
This change provides parity with the TypeScript SDK's batch-level hook events.
Public API Changes
New Events
BeforeToolsEvent — Triggered after the model returns tool use blocks but before any tools execute:
AfterToolsEvent — Triggered after all tools in a batch complete execution:
Event Characteristics
Fields
Both events include:
agent: Agent— The agent executing the toolsmessage: Message— The message from the model containing tool use blockstool_uses: list[ToolUse]— List of tools in the batchUse Cases
Batch Approval Workflows: Request user approval before executing any tools in the batch, enabling safety controls for sensitive operations.
Audit Logging: Log complete tool batches for compliance and debugging, capturing the full context of what tools were requested together.
Resource Management: Allocate/deallocate batch-level resources like database connections or API rate limits that span multiple tool calls.
Resolves #25