feat: allow tool-specific hooks and let onToolEnd override tool out…#978
Conversation
🦋 Changeset detectedLatest commit: 52809f2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This comment has been minimized.
This comment has been minimized.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds per-tool lifecycle hooks (onStart, onEnd) and enables tool.onEnd and agent onToolEnd hooks to return an { output } override; overrides are applied sequentially and re-validated against a tool output schema when present. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Agent as Agent
participant Tool as Tool
participant Schema as OutputSchema
Client->>Agent: executeTool(args)
Agent->>Agent: runToolStartHooks()
Agent->>Tool: tool.onStart?(args)
Agent->>Tool: execute(args)
Tool-->>Agent: originalOutput
Agent->>Tool: tool.onEnd?({ output: originalOutput, error: none })
Tool-->>Agent: toolEndResult (maybe { output })
Agent->>Agent: compute intermediateOutput
Agent->>Schema: validate(intermediateOutput)
Schema-->>Agent: validation result
Agent->>Agent: agent.onToolEnd({ output: intermediateOutput, error: none })
Agent-->>Agent: agentHookResult (maybe { output })
Agent->>Schema: validate(finalOutput)
Schema-->>Agent: validation result
Agent-->>Client: return finalOutput
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying voltagent with
|
| Latest commit: |
2e55636
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://28427b94.voltagent.pages.dev |
| Branch Preview URL: | https://feat-tool-hooks.voltagent.pages.dev |
There was a problem hiding this comment.
1 issue found across 10 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="website/docs/agents/tools.md">
<violation number="1" location="website/docs/agents/tools.md:49">
P2: This adds a second "## Tool Hooks" heading in the same document, which will generate duplicate anchors and confuse navigation/links. Rename this new section to a distinct title (e.g., "Tool-Specific Hooks") to avoid conflicts with the existing agent hook section.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/agent/agent.ts (1)
5158-5165: onToolEnd handler must return output override to match AgentHookOnToolEnd type signature.Lines 5162-5165 currently return
void, but theAgentHookOnToolEndtype requiresPromise<OnToolEndHookResult | undefined> | OnToolEndHookResult | undefined. This causes a TS2322 type error. Chain the option hook result into the agent hook and return the final override (agent wins) so output overrides propagate correctly.Suggested fix
- onToolEnd: async (...args) => { - await options.hooks?.onToolEnd?.(...args); - await this.hooks.onToolEnd?.(...args); - }, + onToolEnd: async (...args) => { + const [payload] = args; + let output = payload?.output; + let hasOverride = false; + + const optionResult = await options.hooks?.onToolEnd?.(...args); + if (optionResult && typeof optionResult === "object" && "output" in optionResult) { + output = (optionResult as { output?: unknown }).output; + hasOverride = true; + } + + const agentResult = await this.hooks.onToolEnd?.({ + ...(payload ?? {}), + output, + }); + if (agentResult && typeof agentResult === "object" && "output" in agentResult) { + return { output: (agentResult as { output?: unknown }).output }; + } + + return hasOverride ? { output } : undefined; + },
🤖 Fix all issues with AI agents
In `@packages/core/src/agent/agent.ts`:
- Around line 4665-4693: The resolveToolEndOutput flow currently only
revalidates when output !== currentOutput, which skips validation if a hook
explicitly returns or mutates to the same object; change resolveToolEndOutput to
track an explicit override flag when tool.hooks?.onEnd or hooks.onToolEnd
returns a value satisfying hasOutputOverride (e.g., set overrideProvided =
true), and call this.validateToolOutput(output, tool) whenever overrideProvided
is true (instead of only on inequality) so any explicit hook override goes
through outputSchema validation; adjust logic around hasOutputOverride,
toolHookResult, and agentHookResult to set and check overrideProvided before
calling validateToolOutput.
In `@website/docs/agents/tools.md`:
- Around line 49-81: This file contains a duplicate "## Tool Hooks" section;
merge the two sections or rename one to avoid split/contradictory guidance.
Locate the example block that defines summarizeTool via createTool (the snippet
with parameters, execute, and hooks: { onStart, onEnd }) and either consolidate
its content into the other "Tool Hooks" section or remove the redundant heading
and cross-reference the single canonical section so documentation remains
consistent.
e193b4d to
939d7ba
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/core/src/agent/agent.ts`:
- Around line 4638-4699: The runToolStartHooks implementation calls the
agent-level hook (hooks.onToolStart) before the tool-level hook
(tool.hooks.onStart), which contradicts the docs; modify runToolStartHooks so it
invokes tool.hooks?.onStart(...) first and then awaits hooks.onToolStart(...)
while keeping the same argument objects (tool, args, options: executionOptions,
and agent/context where applicable) to preserve behavior and ordering
consistency between the tool-level onStart and the agent-level onToolStart.
- Around line 4701-4709: handleToolSuccess and handleToolError both call
toolSpan.end(), and the outer finally calls
oc.traceContext.endChildSpan(toolSpan, "completed", {}), causing double-ending;
either remove the explicit toolSpan.end() calls from handleToolSuccess and
handleToolError (so span closing is centralized in the finally via
oc.traceContext.endChildSpan) or guard the finally span-close with a recording
check (use toolSpan.isRecording() before calling oc.traceContext.endChildSpan)
so endChildSpan is only invoked for active spans; update the functions
handleToolSuccess, handleToolError, and the finally block accordingly to use one
consistent span-end path.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/agent/agent.ts (1)
4769-4815: Yield validated stream chunks to preserve output normalization.
validateToolOutputreturns the parsed/normalized output, but the streaming loop yields the rawpendingOutput. This can make streamed chunks diverge from the validated output (e.g., defaults/coercions/stripping), while the final chunk uses the parsed result. Consider storing/yielding the validated value.🔧 Suggested fix
- while (true) { - const next = await oc.traceContext.withSpan(toolSpan, () => iterator.next()); - if (next.done) { - break; - } - - if (hasOutput) { - yield pendingOutput; - } - - pendingOutput = next.value; - hasOutput = true; - validatedResult = await this.validateToolOutput(pendingOutput, tool); - } + while (true) { + const next = await oc.traceContext.withSpan(toolSpan, () => iterator.next()); + if (next.done) { + break; + } + + const validatedNext = await this.validateToolOutput(next.value, tool); + if (hasOutput) { + yield pendingOutput; + } + + pendingOutput = validatedNext; + hasOutput = true; + validatedResult = validatedNext; + }
…put #975
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
What is the new behavior?
fixes (issue)
Notes for reviewers
Summary by cubic
Adds tool-level hooks and lets agent onToolEnd override tool outputs with validation to improve control over tool results. Addresses #975.
Written for commit 52809f2. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.