centralize tool plugin hooks + add agent to hook input#13521
Closed
ArmirKS wants to merge 5 commits intoanomalyco:devfrom
Closed
centralize tool plugin hooks + add agent to hook input#13521ArmirKS wants to merge 5 commits intoanomalyco:devfrom
ArmirKS wants to merge 5 commits intoanomalyco:devfrom
Conversation
Contributor
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
ArmirKS
force-pushed
the
fix/tool-invoke-hooks
branch
3 times, most recently
from
f573ba1 to
242ada3
Compare
ArmirKS
force-pushed
the
fix/tool-invoke-hooks
branch
2 times, most recently
from
d1440a3 to
d74de90
Compare
ArmirKS
changed the title
ArmirKS
force-pushed
the
fix/tool-invoke-hooks
branch
from
4db8f42 to
76b0b87
Compare
ArmirKS
changed the title
Contributor
|
Hey! Your PR title Please update it to start with one of:
Where See CONTRIBUTING.md for details. |
prompt.ts had duplicated inline Plugin.trigger calls for tool.execute.before/after at two call sites. Sub-tools inside batch never fired hooks, allowing plugin security policies to be silently bypassed. - Add Tool.invoke() to wrap execution with before/after hooks - Replace duplicated inline Plugin.trigger calls in prompt.ts - Add agent to hook input, status to after output - Fire after hook on error path (previously silent) - MCP tools kept inline (different result shape)
ArmirKS
force-pushed
the
fix/tool-invoke-hooks
branch
from
54d4993 to
312ade4
Compare
github-actions
bot
added
the
needs:compliance
Contributor
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
R44VC0RP
removed
the
needs:compliance
1 task
Author
|
Closing in favor of a smaller, focused PR that adds agent + parentAgent to hook input without mixing in the Tool.invoke() refactor. Will link to #15403. |
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.
What does this PR do?
prompt.tshad the same before/after hook logic inline in 3 places (there's a TODO at line 353 saying "centralize invoke tool logic"). None of them passedagentto the hooks, so plugins couldn't tell which agent made the call.This adds
Tool.invoke()that wraps tool execution with before/after hooks:Tool.invoke(). MCP wrapper kept inline to preserve backward compat (existing plugins accessoutput.content)agentto hook input,statusto after outputtry/catchinTool.invoke()(deviation from style guide — needed to fire the after hook on errors then re-throw,.catch()can't do that cleanly)batch.ts changes removed — batch tool is experimental and being deprecated, so the bypass there is moot.Fixes refactor: centralize tool plugin hooks + add agent to hook input #13524
How did you verify your code works?
1 test file, 4 tests:
test/tool/invoke.test.ts— before/after hooks fire, error status on throw, callID defaults, result passthroughTypecheck clean, turbo typecheck passed all 12 packages on push.