Skip to content

feat(mcp): on-demand MCP tool discovery to preserve local context windows (#1571) - #1651

Open
rishu685 wants to merge 2 commits into
Nano-Collective:mainfrom
rishu685:feat/on-demand-mcp-discovery
Open

rishu685 wants to merge 2 commits into
Nano-Collective:mainfrom
rishu685:feat/on-demand-mcp-discovery

Conversation

@rishu685

@rishu685 rishu685 commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Implements On-demand MCP Tool Discovery to prevent massive MCP JSON schemas from consuming local model context windows and degrading reasoning capabilities when multiple MCP servers or tool-heavy servers are connected (#1571).

What Changed

  1. Lightweight MCP Tool Catalog (source/mcp/tool-catalog.ts):

    • Generates compact token-efficient summaries (name, serverName, truncated description).
    • Supports auto-activation threshold (default: $\ge 8$ connected MCP tools) and configuration override (tune.mcpDiscovery: 'auto' | 'always' | 'disabled').
    • Tracks loaded/hydrated tool schemas dynamically per session.
  2. Built-in Discovery Tool (source/tools/mcp/load-tool-schema.tsx):

    • Adds load_tool_schema tool accepting an array of tool names.
    • Marked readOnly: true with automatic approval (approval: () => false) so it works across all development modes (including plan mode).
    • Dynamically pulls and registers full schemas into the active session tools and returns confirmation and full tool signatures to the model.
  3. Tool Manager Integration (source/tools/tool-manager.ts):

    • Integrates catalog management with getFilteredTools, getToolHandler, and getToolEntry.
    • Direct Execution Fallback: If the model invokes an unloaded MCP tool directly without calling load_tool_schema first, ToolManager automatically hydrates it on the fly and proceeds without failing the turn.
  4. System Prompt Integration (source/utils/prompt-builder.ts):

    • Injects the compact MCP tool catalog into the system prompt with instructions on how to use load_tool_schema when needed.
  5. Configuration & Types (source/types/config.ts, source/types/mcp.ts):

    • Added tune.mcpDiscovery setting (auto, always, disabled).
    • Added MCPToolCatalogEntry and helper methods on MCPClient.
  6. Changeset:

    • Added minor changeset .changeset/on-demand-mcp-discovery.md.

Verification

  • Unit tests passing (ava) for tool-catalog.spec.ts, load-tool-schema.spec.tsx, and tool-manager.spec.ts.
  • TypeScript strict type checks passing (tsc --noEmit).
  • Code formatting & linting passing (biome check).
  • Dead code analysis clean (knip).
  • Semgrep security scan passing (semgrep scan --config auto --error with 0 findings).

Closes #1571

@github-actions github-actions Bot added the area:tools Tool implementations and tool-calling label Oct 8, 2026
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

nc-review: needs work — 2 blocking, 1 important, 3 nits

@rishu685 — there is a blocking item below.

The PR implements on-demand MCP tool discovery via a new MCPToolCatalog, a load_tool_schema built-in tool, prompt injection of the catalog, and direct-execution hydration fallback. The mechanism is sound and the new tests exercise the auto-threshold path meaningfully. However, the JSON config schema is not regenerated after adding tune.mcpDiscovery to TuneConfig, which will fail the schema-drift CI check. The tuneConfig parameter added to initializeMCP is also never wired to any caller, so the explicit always and disabled configuration modes are unreachable in production. These need to be fixed before merge.

🔴 blocking · completeness · schemas/agents.config.schema.json:569

TuneConfig.mcpDiscovery was added to source/types/config.ts but schemas/agents.config.schema.json was not updated. The PR adds no changeset for the schema and does not run pnpm generate:schema. The schema-drift CI check in .github/workflows/pr-checks.yml (pnpm generate:schema && git diff --exit-code schemas/agents.config.schema.json) will fail. Run the generator and commit the regenerated schema before merge.

🔴 blocking · correctness · source/tools/tool-manager.ts:199

initializeMCP gains a new tuneConfig?: TuneConfig parameter, but none of the four call sites pass it: source/hooks/useAppInitialization.tsx:270, source/daemon/entry.ts:122, source/plain/initialize.ts:191, and the test (which exercises the auto-threshold via count, not via the parameter). Combined with the schema drift above, the tune.mcpDiscovery = 'always' and 'disabled' configuration modes are unreachable for real users — the auto-threshold (>= 8 tools) is the only path that can fire. Either wire tuneConfig through to all callers (and regenerate the schema so users can configure it), or drop the parameter and let auto-threshold be the only control.

🟠 important · tests · source/tools/tool-manager.spec.ts:1322

The added discovery test passes no tuneConfig and relies on mcpToolEntries.length >= 8 to trigger the auto path. There is no test for tuneConfig.mcpDiscovery === 'always' (force discovery on small tool counts) or tuneConfig.mcpDiscovery === 'disabled' (force discovery off even when count >= 8). Given finding #2 leaves these branches unwired at runtime, the test should at least assert the parameter is honoured so a regression in the gating logic is caught.

⚪ nit · scope · biome.json:2

The $schema URL is bumped from 2.5.0 to 2.5.13 in the same PR as a feature change. Unrelated drive-by; CONTRIBUTING asks contributors to discuss substantial changes first. Either split into its own chore PR or drop it.

⚪ nit · scope · source/acp/acp-agent.ts:247

Five mojibake → em-dash comment fixes are mixed into a feature PR (the comments were never read by anything; this is purely cosmetic). Either split into a separate cleanup PR or drop it — bundling unrelated tidy-ups makes review harder.

⚪ nit · design · source/tools/tool-manager.ts:154

setActiveToolManager(this) is called inside the ToolManager constructor, and getActiveToolManager() is then read from prompt-builder.ts and load-tool-schema.tsx. Many test and runtime code paths construct multiple ToolManager instances (source/daemon/daemon.ts, source/plain/initialize.ts, source/hooks/useAppInitialization.tsx), so the module-level singleton is whichever instance was constructed last. The catalog section will silently be missing whenever the prompt is built before the active manager is set, and a subagent's load_tool_schema call mutates whatever manager happens to be active at call time — usually the parent's. The current tests don't exercise this, so it works in CI; in production it's fragile. A explicit getter passed through the call chain would be safer than module state.


🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional

Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.

@github-actions github-actions Bot added the agent:needs-work nc-review found blocking findings label Oct 8, 2026
@github-actions github-actions Bot added the area:tui Terminal UI label Oct 8, 2026
@rishu685

rishu685 commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Made the required changes as per the feedback

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

Labels

agent:needs-work nc-review found blocking findings area:tools Tool implementations and tool-calling area:tui Terminal UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Architecture: On-demand MCP Tool Discovery to preserve local context windows

1 participant