Skip to content

fix(tools): hidden tools promoted in the parent registry rather than the subagent's registry#2793

Closed
bogdanovich wants to merge 2 commits intosipeed:mainfrom
bogdanovich:fix/deferred-tool-search-clone
Closed

fix(tools): hidden tools promoted in the parent registry rather than the subagent's registry#2793
bogdanovich wants to merge 2 commits intosipeed:mainfrom
bogdanovich:fix/deferred-tool-search-clone

Conversation

@bogdanovich
Copy link
Copy Markdown
Contributor

Summary

Fix deferred tool discovery inside cloned tool registries, especially subagent turns.

ToolRegistry.Clone() previously shallow-copied every tool entry. Discovery tools such as BM25SearchTool and RegexSearchTool store a pointer to the registry they search and promote tools in. After cloning a registry for a subagent, the cloned search tool still pointed at the parent registry.

That meant a subagent could call tool_search_tool_bm25, get a successful "promoted tools" result, but the hidden tools were promoted in the parent registry rather than the subagent's cloned registry. The next subagent LLM iteration still could not call the discovered tools.

Changes

  • Recreate BM25 and regex discovery tools during ToolRegistry.Clone() so they point at the cloned registry.
  • Add a regression test that verifies searching in a clone promotes hidden tools only in that clone, not in the parent registry.

Validation

  • go test ./pkg/tools -run 'TestToolRegistry_Clone_RebindsDiscoveryToolsToClone|TestToolRegistry_Clone' -count=1
  • go test ./pkg/tools ./pkg/agent

@bogdanovich bogdanovich changed the title fix(tools): rebind discovery tools when cloning registries fix(tools): hidden tools promoted in the parent registry rather than the subagent's registry May 7, 2026
Comment thread pkg/tools/registry.go
@bogdanovich bogdanovich force-pushed the fix/deferred-tool-search-clone branch from efee12b to 883c3f4 Compare May 7, 2026 20:29
@bogdanovich
Copy link
Copy Markdown
Contributor Author

Addressed the review point.

I removed the concrete-type switch from ToolRegistry.Clone() and replaced it with a tool-owned optional contract: discovery tools now implement CloneForRegistry(*ToolRegistry) Tool, and registry cloning dispatches through that interface. That keeps the registry generic and makes future registry-bound tool types responsible for cloning their own internal state.

Also rebased the branch on current upstream/main and reran:

  • go test ./pkg/tools -run 'TestToolRegistry_Clone_RebindsDiscoveryToolsToClone|TestToolRegistry_Clone' -count=1

Please take another look when convenient.

@bogdanovich bogdanovich force-pushed the fix/deferred-tool-search-clone branch from 883c3f4 to cb606cd Compare May 8, 2026 06:11
@bogdanovich
Copy link
Copy Markdown
Contributor Author

Closing this as superseded by upstream. The requested architectural shape is now already present on upstream/main: ToolRegistry.Clone() dispatches through a tool-owned CloneForRegistry(*ToolRegistry) contract, and the discovery-tool clone regression coverage is already in tree. This branch is no longer adding distinct value beyond what upstream already has.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants