Track hooks in a HookRegistry, rather than in globals#213
Merged
Conversation
Storing it in globals is a smell in general, and I found a specific case where it can cause problems: when using vercel workflow, we sometimes have the same workflow running on two threads at the same time, and they will have the same hook names. The current HookRegistry is tracked in a contextvar. By default `Agent.run` will use the current registry and will create a fresh registry if one does not exist. It can also take a hook_registry to use as an argument. This means that subagents by default will inherit the parent's hook context. I could go either way on this? I also dropped the runtime tracking of hooks, since the cleanup will happen when the HookRegistry gets torn down. That is a bit of a change for subagents, though not a big one. hook_impl deregisters the hook on exceptions now, though, and not just success. Everything that signals hooks gains an optional `registry` argument. This does require changes in some client code, if the hook signalling isn't from inside the `agent.run`.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
anbuzin
approved these changes
Jul 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Storing it in globals is a smell in general, and I found a specific
case where it can cause problems: when using vercel workflow, we
sometimes have the same workflow running on two threads at the same
time, and they will have the same hook names.
The current HookRegistry is tracked in a contextvar. By default
Agent.runwill use the current registry and will create a freshregistry if one does not exist. It can also take a hook_registry to
use as an argument.
This means that subagents by default will inherit the parent's hook
context. I could go either way on this?
I also dropped the runtime tracking of hooks, since the cleanup will
happen when the HookRegistry gets torn down.
That is a bit of a change for subagents, though not a big one.
hook_impl deregisters the hook on exceptions now, though, and not just
success.
Everything that signals hooks gains an optional
registryargument.This does require changes in some client code, if the hook signalling
isn't from inside the
agent.run.