feat(plugins): improve plugin creation devex with @hook and @tool decorators#1740
feat(plugins): improve plugin creation devex with @hook and @tool decorators#1740
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
d652f8e to
00f08ea
Compare
00f08ea to
655ffd3
Compare
- Create @hook decorator for declarative hook registration in plugins - Convert Plugin from Protocol to base class (breaking change) - Add auto-discovery of @hook and @tool decorated methods in Plugin.__init__() - Add auto-registration of hooks and tools in Plugin.init_plugin() - Support union types for multiple event types (e.g., BeforeModelCallEvent | AfterModelCallEvent) - Export hook from strands.plugins and strands namespaces - Update existing tests to use inheritance-based approach - Add comprehensive test coverage for new functionality BREAKING CHANGE: Plugin is now a base class instead of a Protocol. Existing plugins must inherit from Plugin instead of just implementing the protocol.
655ffd3 to
f2f74e4
Compare
66c8ff3 to
d4bac68
Compare
d4bac68 to
2e8a268
Compare
mkmeral
left a comment
There was a problem hiding this comment.
Also couple of pointers from my agent 😅
My main things are the other two reviews though, making tools/hooks public and moving the logic to registry to simplify init plugin devx (getting rid of the extra super call for plumbing)
Type annotation bug in _WrappedHookCallable
src/strands/plugins/decorator.py:25 — _hook_event_types is list[TEvent] but stores classes, not instances. Should be list[type[TEvent]]. Confirmed at runtime: type(handler._hook_event_types[0]) is <class 'type'>.
# Fix
_hook_event_types: list[type[TEvent]]@hook not exported from top-level strands package
from strands import tool works but from strands import hook does not. These are symmetrical decorators plugin authors will use together. Add to src/strands/__init__.py:
from .plugins import Plugin, hookHook discovery order is alphabetical, not definition order
plugin.py:82 uses dir(self) which returns alphabetically. A plugin with z_validate and a_log hooks on the same event registers a_log first. Surprising when ordering matters. Fix with __mro__ + vars() (Python 3.7+ guarantees dict insertion order), or at minimum document the behavior.
No public API for plugin.hooks / plugin.tools
Discovered hooks/tools live in _hooks/_tools — private mutable lists with no public accessors. Users can mutate them (p._hooks.clear(), p._hooks.append(...)) but it's undocumented and fragile. Either:
- Document that customization goes through
init_pluginoverride (minimum for this PR) - Add read-only properties returning tuples (follow-up)
Docstring examples use @tool without showing its import
plugins/__init__.py:19 and plugin.py:43 show @tool alongside @hook but never import it. Users copy-pasting get a NameError. Add from strands.tools.decorator import tool to the examples.
Docstring typo in _WrappedHookCallable
decorator.py:24 says "includes a _hook_event_types argument" — should be "attribute".
DevX before/after samples
Simple plugin:
# BEFORE — 3 lines of boilerplate
class LoggingPlugin(Plugin):
name = "logging"
def init_plugin(self, agent):
agent.add_hook(self._log, BeforeModelCallEvent)
def _log(self, event: BeforeModelCallEvent) -> None:
print(f"Model call for {event.agent.name}")
# AFTER
class LoggingPlugin(Plugin):
name = "logging"
@hook
def log(self, event: BeforeModelCallEvent) -> None:
print(f"Model call for {event.agent.name}")Multiple hooks — no registration list to keep in sync:
class AuditPlugin(Plugin):
name = "audit"
@hook
def before_model(self, event: BeforeModelCallEvent) -> None: ...
@hook
def after_model(self, event: AfterModelCallEvent) -> None: ...
@hook
def before_tool(self, event: BeforeToolCallEvent) -> None: ...Union types — one annotation, two registrations:
class MonitorPlugin(Plugin):
name = "monitor"
@hook
def on_model(self, event: BeforeModelCallEvent | AfterModelCallEvent) -> None:
print(f"Model event: {type(event).__name__}")Hooks + tools — both auto-discovered:
class MetricsPlugin(Plugin):
name = "metrics"
def __init__(self):
super().__init__()
self.calls = 0
@hook
def count(self, event: BeforeModelCallEvent) -> None:
self.calls += 1
@tool
def get_metrics(self) -> str:
"""Get call count."""
return f"Calls: {self.calls}"Hybrid — decorated + conditional manual hooks:
class ConfigPlugin(Plugin):
name = "config"
def __init__(self, verbose=False):
super().__init__()
self.verbose = verbose
@hook
def always_on(self, event: BeforeModelCallEvent) -> None: ...
def verbose_hook(self, event: BeforeInvocationEvent) -> None: ...
def init_plugin(self, agent):
super().init_plugin(agent) # registers @hook methods
if self.verbose:
agent.add_hook(self.verbose_hook, BeforeInvocationEvent)| Attributes: | ||
| name: A stable string identifier for the plugin | ||
| name: A stable string identifier for the plugin (must be provided by subclass) | ||
| _hooks: List of discovered @hook decorated methods (populated in __init__) |
There was a problem hiding this comment.
I think these should be public. That allows users to be able to customize their plugins before registering with the agent.
If we want plugins to be reusable components across projects, then we need to think about how it can be applied to different use cases. And there are user who might want to disable/update some of these hooks.
| """ | ||
| ... | ||
| # Register discovered hooks with the agent's hook registry | ||
| for hook_callback in self._hooks: |
There was a problem hiding this comment.
I think we can move this logic to plugin registry and make init plugin abstract. what do you think?
Motivation
Currently, plugin authors must manually register hooks in their
init_plugin()method, which is verbose and error-prone:This PR enables declarative hook registration using a
@hookdecorator, making plugin development more intuitive and reducing boilerplate:Resolves: #1739
Public API Changes
New
@hookdecoratorThe
@hookdecorator marks methods for automatic registration: