fix(core): preserve user callTool/searchTools with tool routing#1117
Merged
Conversation
🦋 Changeset detectedLatest commit: f850d08 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 |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReworks agent tool-routing conflict detection to prefer user-defined tools over internal routing helpers by detecting internal routing tools via a symbol property instead of reserved names; adds tests and a release note documenting the behavior change so user tools named Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
This comment has been minimized.
This comment has been minimized.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/core/src/agent/agent.spec.ts (1)
3031-3041: Avoid introducing newas anyin these regression tests to maintain type safety.The new tests at lines 3039-3040 and 3079-3080 use
(agent as any)to access internal methods, which violates the TypeScript type safety guideline. Replace with a narrow test-only interface viaunknowncast for better compile-time safety:Suggested refactor
+type AgentPrepareToolsTestApi = { + createOperationContext: (input: string) => { + traceContext: { end: (status: string) => void }; + }; + prepareTools: ( + runtimeTools: Tool[], + operationContext: { traceContext: { end: (status: string) => void } }, + maxSteps: number, + options: { toolRouting?: false | Record<string, unknown> }, + ) => Promise<Record<string, { execute: (args: Record<string, string>) => Promise<unknown> }>>; +}; - const operationContext = (agent as any).createOperationContext("input message"); - const prepared = await (agent as any).prepareTools([], operationContext, 3, {}); + const testAgent = agent as unknown as AgentPrepareToolsTestApi; + const operationContext = testAgent.createOperationContext("input message"); + const prepared = await testAgent.prepareTools([], operationContext, 3, {});Apply the same change at lines 3079-3080.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/agent/agent.spec.ts` around lines 3031 - 3041, The tests currently use unsafe `(agent as any)` casts to call internal methods; define a narrow test-only interface (e.g., `interface TestAgentInternals { createOperationContext(input: string): ReturnType; prepareTools(args: any[], ctx: any, n: number, opts: any): Promise<any>; }`) that declares only the internal members you need, then replace `(agent as any)` with `agent as unknown as TestAgentInternals` when calling `createOperationContext` and `prepareTools` (apply the same change for both call sites), removing the `as any` usages to restore type safety while limiting exposure to internals.packages/core/src/agent/agent.ts (1)
7189-7197: Prefer marker-based filtering before API projection (avoid name-based fallback).Line 7197 still excludes by name. If internal and user-defined same-name tools ever coexist in the pool, this can hide the user tool from
toolRouting.poolstate output.♻️ Suggested refactor
- const internalPoolSupportNames = new Set( - this.toolPoolManager - .getAllTools() - .filter((tool) => this.isToolRoutingSupportTool(tool)) - .map((tool) => tool.name), - ); - const poolApiTools = this.toolPoolManager - .getToolsForApi() - .filter((tool) => !internalPoolSupportNames.has(tool.name)); + const poolToolsWithoutInternalSupport = this.toolPoolManager + .getAllTools() + .filter((tool) => !this.isToolRoutingSupportTool(tool)); + const poolApiTools = new ToolManager( + poolToolsWithoutInternalSupport, + this.logger, + ).getToolsForApi();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/agent/agent.ts` around lines 7189 - 7197, The current exclusion uses tool.name which can hide user tools if names collide; instead derive the support-tool identity from the marker-based predicate and exclude by a stable identifier before producing the API projection: create a Set of support tool ids from this.toolPoolManager.getAllTools().filter(tool => this.isToolRoutingSupportTool(tool)).map(t => t.id) and then build poolApiTools by filtering this.toolPoolManager.getToolsForApi() by !supportIds.has(tool.id) (not by name), so toolRouting.pool uses marker-aware filtering rather than name-based fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/src/agent/agent.spec.ts`:
- Around line 3031-3041: The tests currently use unsafe `(agent as any)` casts
to call internal methods; define a narrow test-only interface (e.g., `interface
TestAgentInternals { createOperationContext(input: string): ReturnType;
prepareTools(args: any[], ctx: any, n: number, opts: any): Promise<any>; }`)
that declares only the internal members you need, then replace `(agent as any)`
with `agent as unknown as TestAgentInternals` when calling
`createOperationContext` and `prepareTools` (apply the same change for both call
sites), removing the `as any` usages to restore type safety while limiting
exposure to internals.
In `@packages/core/src/agent/agent.ts`:
- Around line 7189-7197: The current exclusion uses tool.name which can hide
user tools if names collide; instead derive the support-tool identity from the
marker-based predicate and exclude by a stable identifier before producing the
API projection: create a Set of support tool ids from
this.toolPoolManager.getAllTools().filter(tool =>
this.isToolRoutingSupportTool(tool)).map(t => t.id) and then build poolApiTools
by filtering this.toolPoolManager.getToolsForApi() by !supportIds.has(tool.id)
(not by name), so toolRouting.pool uses marker-aware filtering rather than
name-based fallback.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/soft-dots-bow.mdpackages/core/src/agent/agent.spec.tspackages/core/src/agent/agent.ts
Deploying voltagent with
|
| Latest commit: |
f850d08
|
| Status: | ⚡️ Build in progress... |
omeraplak
deleted the
fix/core-tool-routing-user-tool-precedence-1116
branch
February 28, 2026 04:15
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
1 participant
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.
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
When
toolRoutingis enabled, user-defined tools namedcallToolandsearchToolsare treated as internal routing tools by name and get replaced/filtered, so they are not actually registered as user tools.What is the new behavior?
callTool/searchTools, those user-defined tools take precedence and internal support tools are not inserted over them.toolRouting: falseper-request checks true internal support tools instead of reserved names, so user-defined same-name tools are allowed.fixes #1116
Notes for reviewers
packages/core/src/agent/agent.spec.ts:should prefer user-defined callTool/searchTools when tool routing is enabledshould allow user-defined callTool/searchTools when tool routing is disabled per requestpnpm --filter @voltagent/core exec vitest run src/agent/agent.spec.ts -t "callTool/searchTools"Summary by cubic
Preserves user-defined callTool and searchTools with tool routing and removes name-based conflicts. Internal tools use a symbol marker, pool filtering is tighter, and per-request toolRouting: false works with same-name user tools.
Written for commit f850d08. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests