Deferred (lazy) tool loading: tool_search + auto threshold for large MCP tool sets - #147
Closed
gnanam1990 wants to merge 31 commits into
Closed
Deferred (lazy) tool loading: tool_search + auto threshold for large MCP tool sets#147gnanam1990 wants to merge 31 commits into
gnanam1990 wants to merge 31 commits into
Conversation
…tion + reminder Both reactive-compaction retry sites in the agent loop rebuilt the retry request via the bare toolDefinitions(), which routes through partitionTools with an empty loaded set. When deferral was ACTIVE and a context-limit error triggered a mid-run compaction+retry, that retried turn re-hid every already-loaded deferred tool and sent no <system-reminder>. Reuse the exposed/reminder already computed for the turn (they depend on registry+loaded, not the messages, so they survive compaction) and re-append the reminder to the per-turn retry copy only, matching the main path and keeping the reminder-not-persisted invariant. The inactive path is unchanged. Also gofmt deferred_loop_test.go (receiver-column alignment).
…isable deferral
A bare ToolsConfig{DeferThreshold: 0} leaves the unexported presence flag
unset, so applyOverrides treats it as unset and silently drops an
override-to-disable. ToolsOverride(n) carries the presence flag, so
ToolsOverride(0) now correctly overrides a non-zero base down to 0
(disabled) while ToolsOverride(7) overrides to 7. applyOverrides is
unchanged; the file-layer explicit-0 behavior is untouched.
The old name implied the absence of the deferred-tool search on the --list-tools path was caused by the threshold gate, but --list-tools short-circuits and returns before resolveConfig and the eligibility check ever run. Rename to ...AdvertisesMCPToolsWithoutToolSearch and add a comment pointing at the unit/E2E tests that actually exercise the threshold gate. Also apply a pre-existing gofmt alignment fix in the same block so the package is gofmt-clean.
…CP server by name - Export ToolSearchToolName and use it for the tool's Name() so every gate that must treat the loader specially references one constant. - Add EnabledTools/DisabledTools to RunOptions and honor them in tool_search: an operator-hidden deferred tool is now invisible to select:, keyword ranking, and the no-match listing (no disclosure, no wasted load). - DeferredLine prefers an optional MCPServerName() over the name-derived token so a multi-token server name is labeled correctly in the reminder.
…ervers registryTool now implements the optional tools.mcpServerNamed interface, returning the true configured server name. This fixes the reminder 'server:' label for a server whose sanitized name token contains an underscore (e.g. 'git hub'), which the name-only parser would truncate. Label-only; resolution is unaffected.
…tor filters - partitionTools always exposes tool_search on the ACTIVE path (looked up by ToolSearchToolName) even when a non-empty EnabledTools allowlist omits it; the inactive path stays byte-identical (tool_search still dropped below threshold). - The tool-call dispatch gate exempts tool_search from the allowlist so a call to the loader is never rejected as 'not enabled', while an explicit DisabledTools entry for tool_search is still honored. Together this closes the dead-end where the reminder pointed the model at an absent/rejected loader. - Forward EnabledTools/DisabledTools into the tool RunOptions so tool_search is filter-aware about operator-hidden deferred tools. - Tests: FIX 1+2 dead-end regression (allowlisted deferred + tool_search reachable and dispatch-allowed), disabled-tool_search still hidden/rejected, connect-time reactive-retry keeps the loaded tool + reminder, filter x deferral interactions, and a byte-identical DeepEqual reference for the inactive partition.
… as the loop - deferredEligibleCount/registerToolSearchIfEligible now count only deferred tools that pass agent.ToolVisible (permission-mode advertising + operator filters), so registration and the loop's partition activation agree. Both exec and TUI call sites pass the run's permission mode and enabled/disabled filters; the TUI now resolves its effective Ask mode BEFORE the gate. - validateExecToolFilters treats tool_search as always-valid (it is registered later, only when deferral activates) so --enabled-tools/--disabled-tools tool_search no longer errors 'Unknown tool'. - Tests: a disabled deferred tool drops the visible count below threshold and suppresses registration; tool_search passes filter validation while a genuinely unknown name still errors.
WalkthroughThis PR implements deferred loading of MCP tool schemas. When the count of deferred-eligible MCP tools meets a configurable threshold, full schemas are withheld from initial exposure and advertised via a new ChangesDeferred MCP Tool Loading
Sequence Diagram(s)sequenceDiagram
participant Agent as Agent Loop
participant Registry as Registry
participant ToolSearch as tool_search
participant Deferred as Deferred Tool
participant LLM as LLM
Agent->>Registry: partitionTools(loaded={})
alt Deferred tools meet threshold
Registry-->>Agent: exposed=[tool_search], reminder="..."
else Below threshold
Registry-->>Agent: exposed=[all tools], reminder=""
end
Agent->>LLM: Request with exposed tools + reminder
LLM->>Agent: Call tool_search with "select:weather"
Agent->>ToolSearch: Execute (weather_lookup not yet loaded)
ToolSearch->>Registry: Find deferred tools
Registry-->>ToolSearch: [weather_lookup schema]
ToolSearch-->>Agent: Result with Meta["load_tools"]="weather_lookup"
Agent->>Agent: loaded["weather_lookup"] = true
Agent->>Registry: partitionTools(loaded={weather_lookup})
Registry-->>Agent: exposed=[tool_search, weather_lookup], reminder="hidden:..."
Agent->>LLM: Next turn with loaded tool exposed
LLM->>Agent: Call weather_lookup directly
Agent->>Deferred: Execute weather_lookup
Deferred-->>Agent: Result (no reminder in history)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/agent/loop.go (2)
469-479:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOnly honor
load_toolsfromtool_search.Right now every tool result is allowed to drive
LoadedTools. That means any MCP tool that returnsMeta["load_tools"]can force hidden deferred schemas to be exposed on the next turn, bypassing the intendedtool_searchgate.Proposed fix
return ToolResult{ ToolCallID: call.ID, Name: call.Name, Status: result.Status, Output: result.Output, Meta: result.Meta, Redacted: result.Redacted, ChangedFiles: result.ChangedFiles, Display: result.Display, - LoadedTools: loadedToolsFromResult(result.Meta), + LoadedTools: func() []string { + if call.Name != tools.ToolSearchToolName { + return nil + } + return loadedToolsFromResult(result.Meta) + }(), }, nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/agent/loop.go` around lines 469 - 479, The current construction of ToolResult always sets LoadedTools via loadedToolsFromResult(result.Meta), allowing any tool to expose deferred schemas; change it so LoadedTools is populated only when the originating tool call is the tool_search call. Specifically, in the code that returns ToolResult (the block constructing ToolResult with ToolCallID, Name, Status, etc.), conditionally call loadedToolsFromResult(result.Meta) only if call.Name (or the identifier used for the tool) equals "tool_search" (or the project's canonical tool_search identifier); otherwise set LoadedTools to nil/empty. Keep the rest of the fields unchanged.
95-97:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMeasure the actual request payload.
Line 96 reports context for
messages, but the sent request may also include the deferred reminder appended torequest.Messages. That undercounts the active turn's prompt size whenever deferral is on.Proposed fix
if options.OnContext != nil { - options.OnContext(MeasureContext(messages, request.Tools, options.ContextWindow)) + options.OnContext(MeasureContext(request.Messages, request.Tools, options.ContextWindow)) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/agent/loop.go` around lines 95 - 97, The current OnContext call measures the original messages variable but misses any deferred reminder appended into request.Messages; update the call so MeasureContext is invoked with the actual request payload (request.Messages) and the same request.Tools and options.ContextWindow instead of messages; locate the call to options.OnContext(MeasureContext(messages, request.Tools, options.ContextWindow)) and replace the messages argument with request.Messages (ensuring request is non-nil if necessary) so context measurement includes deferred reminders.
🧹 Nitpick comments (1)
internal/tools/tool_search.go (1)
98-102: ⚡ Quick winRemove the unused
deferredToolsmethod.This method is flagged by static analysis as unused and has no callers in the codebase. It's a thin wrapper around
visibleDeferredTools(nil, nil), but since no code needs the "all deferred tools without filters" convenience, it should be removed.🧹 Proposed cleanup
-// deferredTools returns the registry's deferred-eligible tools sorted by name so -// keyword ranking and listings are deterministic. -func (tool toolSearchTool) deferredTools() []Tool { - return tool.visibleDeferredTools(nil, nil) -} -🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/tools/tool_search.go` around lines 98 - 102, Remove the unused deferredTools method on type toolSearchTool: delete the deferredTools() []Tool wrapper that simply calls visibleDeferredTools(nil, nil), since static analysis reports no callers; ensure no references remain to deferredTools and leave visibleDeferredTools intact for existing usages.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/agent/loop.go`:
- Around line 894-953: The deferral "active" flag should be disabled whenever
the loader tool_search is unavailable or explicitly disabled; update the active
computation (currently using options.DeferThreshold and eligible) to also
require that tools.ToolSearchToolName is not present in options.DisabledTools
and that registry.Get(tools.ToolSearchToolName) returns ok, so that if the
loader is denied or missing you don't hide deferred tools, don't build
hiddenLines, and don't produce the reminder; adjust uses of active accordingly
(symbols: options.DeferThreshold, eligible, tools.ToolSearchToolName,
options.DisabledTools, containsToolName, registry.Get, hiddenLines,
BuildDeferredReminder).
---
Outside diff comments:
In `@internal/agent/loop.go`:
- Around line 469-479: The current construction of ToolResult always sets
LoadedTools via loadedToolsFromResult(result.Meta), allowing any tool to expose
deferred schemas; change it so LoadedTools is populated only when the
originating tool call is the tool_search call. Specifically, in the code that
returns ToolResult (the block constructing ToolResult with ToolCallID, Name,
Status, etc.), conditionally call loadedToolsFromResult(result.Meta) only if
call.Name (or the identifier used for the tool) equals "tool_search" (or the
project's canonical tool_search identifier); otherwise set LoadedTools to
nil/empty. Keep the rest of the fields unchanged.
- Around line 95-97: The current OnContext call measures the original messages
variable but misses any deferred reminder appended into request.Messages; update
the call so MeasureContext is invoked with the actual request payload
(request.Messages) and the same request.Tools and options.ContextWindow instead
of messages; locate the call to options.OnContext(MeasureContext(messages,
request.Tools, options.ContextWindow)) and replace the messages argument with
request.Messages (ensuring request is non-nil if necessary) so context
measurement includes deferred reminders.
---
Nitpick comments:
In `@internal/tools/tool_search.go`:
- Around line 98-102: Remove the unused deferredTools method on type
toolSearchTool: delete the deferredTools() []Tool wrapper that simply calls
visibleDeferredTools(nil, nil), since static analysis reports no callers; ensure
no references remain to deferredTools and leave visibleDeferredTools intact for
existing usages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f662afae-3270-4edd-8fe9-08f237379ad0
📒 Files selected for processing (20)
internal/agent/deferred_loop_test.gointernal/agent/loop.gointernal/agent/types.gointernal/cli/app.gointernal/cli/deferred_wiring_test.gointernal/cli/exec.gointernal/cli/exec_tools.gointernal/config/resolver.gointernal/config/resolver_test.gointernal/config/types.gointernal/config/types_test.gointernal/mcp/registry.gointernal/mcp/registry_test.gointernal/tools/deferred.gointernal/tools/deferred_test.gointernal/tools/registry.gointernal/tools/registry_test.gointernal/tools/tool_search.gointernal/tools/tool_search_test.gointernal/tools/types.go
Comment on lines
+894
to
+953
| active := options.DeferThreshold > 0 && eligible >= options.DeferThreshold | ||
|
|
||
| definitions := make([]zeroruntime.ToolDefinition, 0, len(visible)) | ||
| exposedNames := make(map[string]bool, len(visible)) | ||
| var hiddenLines []string | ||
| for _, tool := range visible { | ||
| name := tool.Name() | ||
| deferred := tools.IsDeferred(tool) | ||
|
|
||
| if !active { | ||
| // Inactive: byte-identical to legacy, but tool_search is never advertised. | ||
| if name == tools.ToolSearchToolName { | ||
| continue | ||
| } | ||
| definitions = append(definitions, zeroruntime.ToolDefinition{ | ||
| Name: name, | ||
| Description: tool.Description(), | ||
| Parameters: schemaToRuntimeMap(tool.Parameters()), | ||
| }) | ||
| exposedNames[name] = true | ||
| continue | ||
| } | ||
|
|
||
| // Active path. | ||
| if deferred && !loaded[name] { | ||
| hiddenLines = append(hiddenLines, tools.DeferredLine(tool)) | ||
| continue | ||
| } | ||
| definitions = append(definitions, zeroruntime.ToolDefinition{ | ||
| Name: tool.Name(), | ||
| Name: name, | ||
| Description: tool.Description(), | ||
| Parameters: schemaToRuntimeMap(tool.Parameters()), | ||
| }) | ||
| exposedNames[name] = true | ||
| } | ||
|
|
||
| // On the ACTIVE path, tool_search is the gateway to the allowlisted deferred | ||
| // tools, so it must ALWAYS be reachable — even when a non-empty EnabledTools | ||
| // allowlist omits it (the operator allowlisted the deferred tools, not the | ||
| // loader). Expose its full definition unless an explicit DisabledTools entry | ||
| // turns the loader itself off. This never runs on the inactive path, so the | ||
| // byte-identical below-threshold output is preserved. | ||
| if active && !exposedNames[tools.ToolSearchToolName] && !containsToolName(options.DisabledTools, tools.ToolSearchToolName) { | ||
| if loader, ok := registry.Get(tools.ToolSearchToolName); ok { | ||
| definitions = append(definitions, zeroruntime.ToolDefinition{ | ||
| Name: loader.Name(), | ||
| Description: loader.Description(), | ||
| Parameters: schemaToRuntimeMap(loader.Parameters()), | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| sort.Slice(definitions, func(left int, right int) bool { | ||
| return definitions[left].Name < definitions[right].Name | ||
| }) | ||
| return definitions | ||
|
|
||
| reminder := "" | ||
| if active { | ||
| reminder = tools.BuildDeferredReminder(hiddenLines) | ||
| } |
There was a problem hiding this comment.
Disable deferral when the loader is unavailable.
If tool_search is explicitly disabled, this branch still activates deferral, hides unloaded deferred tools, and builds the reminder. The model then gets instructions for a loader it cannot call.
Proposed fix
- active := options.DeferThreshold > 0 && eligible >= options.DeferThreshold
+ toolSearchEnabled := !containsToolName(options.DisabledTools, tools.ToolSearchToolName)
+ active := toolSearchEnabled && options.DeferThreshold > 0 && eligible >= options.DeferThresholdThat preserves the operator's denylist without making deferred tools unreachable.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| active := options.DeferThreshold > 0 && eligible >= options.DeferThreshold | |
| definitions := make([]zeroruntime.ToolDefinition, 0, len(visible)) | |
| exposedNames := make(map[string]bool, len(visible)) | |
| var hiddenLines []string | |
| for _, tool := range visible { | |
| name := tool.Name() | |
| deferred := tools.IsDeferred(tool) | |
| if !active { | |
| // Inactive: byte-identical to legacy, but tool_search is never advertised. | |
| if name == tools.ToolSearchToolName { | |
| continue | |
| } | |
| definitions = append(definitions, zeroruntime.ToolDefinition{ | |
| Name: name, | |
| Description: tool.Description(), | |
| Parameters: schemaToRuntimeMap(tool.Parameters()), | |
| }) | |
| exposedNames[name] = true | |
| continue | |
| } | |
| // Active path. | |
| if deferred && !loaded[name] { | |
| hiddenLines = append(hiddenLines, tools.DeferredLine(tool)) | |
| continue | |
| } | |
| definitions = append(definitions, zeroruntime.ToolDefinition{ | |
| Name: tool.Name(), | |
| Name: name, | |
| Description: tool.Description(), | |
| Parameters: schemaToRuntimeMap(tool.Parameters()), | |
| }) | |
| exposedNames[name] = true | |
| } | |
| // On the ACTIVE path, tool_search is the gateway to the allowlisted deferred | |
| // tools, so it must ALWAYS be reachable — even when a non-empty EnabledTools | |
| // allowlist omits it (the operator allowlisted the deferred tools, not the | |
| // loader). Expose its full definition unless an explicit DisabledTools entry | |
| // turns the loader itself off. This never runs on the inactive path, so the | |
| // byte-identical below-threshold output is preserved. | |
| if active && !exposedNames[tools.ToolSearchToolName] && !containsToolName(options.DisabledTools, tools.ToolSearchToolName) { | |
| if loader, ok := registry.Get(tools.ToolSearchToolName); ok { | |
| definitions = append(definitions, zeroruntime.ToolDefinition{ | |
| Name: loader.Name(), | |
| Description: loader.Description(), | |
| Parameters: schemaToRuntimeMap(loader.Parameters()), | |
| }) | |
| } | |
| } | |
| sort.Slice(definitions, func(left int, right int) bool { | |
| return definitions[left].Name < definitions[right].Name | |
| }) | |
| return definitions | |
| reminder := "" | |
| if active { | |
| reminder = tools.BuildDeferredReminder(hiddenLines) | |
| } | |
| toolSearchEnabled := !containsToolName(options.DisabledTools, tools.ToolSearchToolName) | |
| active := toolSearchEnabled && options.DeferThreshold > 0 && eligible >= options.DeferThreshold | |
| definitions := make([]zeroruntime.ToolDefinition, 0, len(visible)) | |
| exposedNames := make(map[string]bool, len(visible)) | |
| var hiddenLines []string | |
| for _, tool := range visible { | |
| name := tool.Name() | |
| deferred := tools.IsDeferred(tool) | |
| if !active { | |
| // Inactive: byte-identical to legacy, but tool_search is never advertised. | |
| if name == tools.ToolSearchToolName { | |
| continue | |
| } | |
| definitions = append(definitions, zeroruntime.ToolDefinition{ | |
| Name: name, | |
| Description: tool.Description(), | |
| Parameters: schemaToRuntimeMap(tool.Parameters()), | |
| }) | |
| exposedNames[name] = true | |
| continue | |
| } | |
| // Active path. | |
| if deferred && !loaded[name] { | |
| hiddenLines = append(hiddenLines, tools.DeferredLine(tool)) | |
| continue | |
| } | |
| definitions = append(definitions, zeroruntime.ToolDefinition{ | |
| Name: name, | |
| Description: tool.Description(), | |
| Parameters: schemaToRuntimeMap(tool.Parameters()), | |
| }) | |
| exposedNames[name] = true | |
| } | |
| // On the ACTIVE path, tool_search is the gateway to the allowlisted deferred | |
| // tools, so it must ALWAYS be reachable — even when a non-empty EnabledTools | |
| // allowlist omits it (the operator allowlisted the deferred tools, not the | |
| // loader). Expose its full definition unless an explicit DisabledTools entry | |
| // turns the loader itself off. This never runs on the inactive path, so the | |
| // byte-identical below-threshold output is preserved. | |
| if active && !exposedNames[tools.ToolSearchToolName] && !containsToolName(options.DisabledTools, tools.ToolSearchToolName) { | |
| if loader, ok := registry.Get(tools.ToolSearchToolName); ok { | |
| definitions = append(definitions, zeroruntime.ToolDefinition{ | |
| Name: loader.Name(), | |
| Description: loader.Description(), | |
| Parameters: schemaToRuntimeMap(loader.Parameters()), | |
| }) | |
| } | |
| } | |
| sort.Slice(definitions, func(left int, right int) bool { | |
| return definitions[left].Name < definitions[right].Name | |
| }) | |
| reminder := "" | |
| if active { | |
| reminder = tools.BuildDeferredReminder(hiddenLines) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/agent/loop.go` around lines 894 - 953, The deferral "active" flag
should be disabled whenever the loader tool_search is unavailable or explicitly
disabled; update the active computation (currently using options.DeferThreshold
and eligible) to also require that tools.ToolSearchToolName is not present in
options.DisabledTools and that registry.Get(tools.ToolSearchToolName) returns
ok, so that if the loader is denied or missing you don't hide deferred tools,
don't build hiddenLines, and don't produce the reminder; adjust uses of active
accordingly (symbols: options.DeferThreshold, eligible,
tools.ToolSearchToolName, options.DisabledTools, containsToolName, registry.Get,
hiddenLines, BuildDeferredReminder).
4 tasks
Collaborator
Author
gnanam1990
added a commit
that referenced
this pull request
Jun 9, 2026
…MCP tool sets Adds deferred tool loading so a large MCP tool set no longer sends every tool's full JSON schema each turn. When the number of visible MCP tools crosses a threshold (config tools.deferThreshold, default 10; 0 disables), their schemas are withheld and advertised compactly in a per-turn system-reminder; the model pulls a tool's full schema on demand via a new tool_search tool. Strictly additive: below the threshold behaviour is byte-identical to today. No new module dependencies. - Eligibility: MCP tools opt in via an optional Deferred() bool interface (tools.IsDeferred); built-ins stay eager. No change to the core Tool interface. - Formatting (internal/tools/deferred.go): compact one-liners wrapped in a <system-reminder> by BuildDeferredReminder; server label via MCPServerName(). - tool_search (internal/tools/tool_search.go): SideEffectNone/PermissionAllow/AdvertiseInAuto; select:Name1,Name2 (exact) or keyword (ranked); returns full schemas and signals the loop via Meta["load_tools"]. Operator-filter-aware (never lists tools hidden by --enabled-tools/--disabled-tools). - Agent loop: partitionTools is the single source of truth; eligible counts only ToolVisible deferred tools; Meta["load_tools"] lifts to ToolResult.LoadedTools and unions into a per-run loaded-set advertised next turn. Reminder appended only to the per-turn request copy (never persisted), including both reactive-compaction retry paths. tool_search stays reachable when deferral is active (exempt from the EnabledTools allowlist). - Config + wiring: tools.deferThreshold (default 10; 0 disables; negative rejected); tool_search registered and DeferThreshold threaded in exec and TUI, gated on the same visible-deferred count the loop uses. Recreated on top of current main; resolves the SideEffectNone clash with #146 (escalate_model). Supersedes #147.
gnanam1990
added a commit
that referenced
this pull request
Jun 9, 2026
…MCP tool sets (#148) * Deferred (lazy) tool loading: tool_search + auto threshold for large MCP tool sets Adds deferred tool loading so a large MCP tool set no longer sends every tool's full JSON schema each turn. When the number of visible MCP tools crosses a threshold (config tools.deferThreshold, default 10; 0 disables), their schemas are withheld and advertised compactly in a per-turn system-reminder; the model pulls a tool's full schema on demand via a new tool_search tool. Strictly additive: below the threshold behaviour is byte-identical to today. No new module dependencies. - Eligibility: MCP tools opt in via an optional Deferred() bool interface (tools.IsDeferred); built-ins stay eager. No change to the core Tool interface. - Formatting (internal/tools/deferred.go): compact one-liners wrapped in a <system-reminder> by BuildDeferredReminder; server label via MCPServerName(). - tool_search (internal/tools/tool_search.go): SideEffectNone/PermissionAllow/AdvertiseInAuto; select:Name1,Name2 (exact) or keyword (ranked); returns full schemas and signals the loop via Meta["load_tools"]. Operator-filter-aware (never lists tools hidden by --enabled-tools/--disabled-tools). - Agent loop: partitionTools is the single source of truth; eligible counts only ToolVisible deferred tools; Meta["load_tools"] lifts to ToolResult.LoadedTools and unions into a per-run loaded-set advertised next turn. Reminder appended only to the per-turn request copy (never persisted), including both reactive-compaction retry paths. tool_search stays reachable when deferral is active (exempt from the EnabledTools allowlist). - Config + wiring: tools.deferThreshold (default 10; 0 disables; negative rejected); tool_search registered and DeferThreshold threaded in exec and TUI, gated on the same visible-deferred count the loop uses. Recreated on top of current main; resolves the SideEffectNone clash with #146 (escalate_model). Supersedes #147. * deferred-tools: activate deferral only when tool_search is runnable; drop dead deferredTools() Addresses #148 review: - partitionTools gates 'active' on a registered, non-disabled, mode-advertised tool_search (mirrors the executeToolCall dispatch gate). When the loader is unavailable (explicitly disabled, missing, or not advertised e.g. spec-draft), fall back to the eager/inactive path instead of hiding deferred tools behind a loader the model can never call (the dead-end). - Flip TestDisabledToolSearch... to assert the eager fallback; register a real tool_search in the active-path tests that previously relied on activation without a usable loader. - Remove unused toolSearchTool.deferredTools(). * deferred-tools: disable deferral in exec when tool_search disabled; drop unused toolDefinitions shim Further #148 review: - exec computes one effective DeferThreshold, forced to 0 when tool_search is in --disabled-tools, and feeds it to both registerToolSearchIfEligible and agent.Options.DeferThreshold — so the registration gate and the loop partition gate agree, and a disabled loader never enables deferral. - Remove the unused toolDefinitions shim (golangci-lint unused; no callers).
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.
Summary
Adds deferred (lazy) tool loading so a large MCP tool set no longer sends every tool's full JSON schema on every turn. When the number of visible MCP tools crosses a threshold, their schemas are withheld, advertised compactly in a per-turn
<system-reminder>, and the model pulls a tool's full schema on demand via atool_searchtool. Strictly additive — below the threshold (default 10) behaviour is byte-identical to today. No new module dependencies.internal/tools,internal/mcp): MCP tools are deferred-eligible via an optionalDeferred() boolinterface (tools.IsDeferred); built-ins never implement it, so they stay eager. No change to the coreToolinterface.internal/tools/deferred.go): compact one-liners —name: <short-desc> | server: <X> | inputs (* required): a (string)*, …; +N more— wrapped in a<system-reminder>block byBuildDeferredReminder. Server label uses the MCP server name directly (MCPServerName()), falling back to a name parse.internal/tools/tool_search.go):tool_search(SideEffectNone/PermissionAllow/AdvertiseInAuto). Queryselect:Name1,Name2(exact) or keywords (ranked); returns each loaded tool's full schema and signals the loop viaMeta["load_tools"]. It is operator-filter-aware — it never resolves, ranks, or lists a tool hidden by--enabled-tools/--disabled-tools.internal/agent):partitionToolsis the single source of truth —eligiblecounts onlyToolVisibledeferred tools; wheneligible >= DeferThreshold, unloaded deferred tools are withheld into the reminder whiletool_search+ built-ins + already-loaded tools stay exposed.Meta["load_tools"]is lifted toToolResult.LoadedToolsand unioned into a per-run loaded-set so the next turn advertises them. The reminder is appended only to the per-turn request copy (never persisted history), including on both reactive-compaction retry paths.tool_searchis always reachable when deferral is active (exempt from theEnabledToolsallowlist) so an allowlist of deferred tools can never strand the loader.internal/config,internal/cli):tools.deferThreshold(default 10;0disables; negative rejected; explicit0preserved vs unset).tool_searchis registered — andDeferThresholdthreaded — in exec and the TUI, gated on the same visible-deferred count the loop uses, so registration and activation never disagree.Loaded-set scope is within a single
agent.Run(v1, fully covers headlessexec); TUI cross-turn persistence is a documented follow-up.Test Plan
go build ./.../go vet ./...clean;gofmtclean (touched files)go test ./...green (new tests acrosstools,mcp,agent,config,cli)go test -race ./internal/{tools,mcp,agent,config,cli}/...greenGOOS=windows GOARCH=amd64 go build ./...greenReviewer focus
partitionToolsinactive output isreflect.DeepEqualto the legacytoolDefinitions(onlytool_searchdropped, which isn't registered when inactive).ToolVisibledeferred population — prompt-gated MCP tools that aren't advertised in auto mode don't spuriously registertool_search.--enabled-toolsallowlist of deferred tools keepstool_searchreachable (regression-tested); a deferred tool hidden by--disabled-toolsis invisible totool_search.Merge note
This branch adds
tools.SideEffectNone(fortool_search); PR #146 (model escalation) adds the same constant forescalate_model— whichever merges second will need a one-line conflict resolution (keep one declaration).tool_searchis safe under the sandbox here regardless (itsPermissionAllowshort-circuits before any risk gate); #146 additionally makesnonefirst-class in the sandbox classifier.Summary by CodeRabbit
Release Notes
New Features
tool_search: a new capability to dynamically load and discover deferred tools using keyword search or exact tool selection.Configuration
deferThresholdsetting controls when tool deferral activates (default: 10 deferred tools).