Skip to content

Implemented selective disable of tools to spare with the initial schema size#1035

Merged
carlos-alm merged 10 commits intooptave:mainfrom
kecsap:feat/mcp-disabled-tools
May 1, 2026
Merged

Implemented selective disable of tools to spare with the initial schema size#1035
carlos-alm merged 10 commits intooptave:mainfrom
kecsap:feat/mcp-disabled-tools

Conversation

@kecsap
Copy link
Copy Markdown
Contributor

@kecsap kecsap commented Apr 30, 2026

Fixed of #1023
Fixes #1022

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 30, 2026

Greptile Summary

This PR implements selective MCP tool disabling via a new mcp.disabledTools config array, allowing users to shrink the schema for small-context models. Tools are filtered at server startup using a normalized (lowercase, prefix-stripped) comparison; disabled tools are excluded from tools/list and rejected early in tools/call via a pre-built enabledToolNames Set. The list_repos dead-code branch in validateMultiRepoAccess is also cleaned up, and a comprehensive docs/guides/configuration.md is added.

Confidence Score: 5/5

This PR is safe to merge — the feature is well-scoped, correctly tested, and introduces no regressions.

No P0 or P1 issues found. The normalization logic is sound, the enabledToolNames gate is consistent with how TOOL_HANDLERS was already keyed (plain lowercase names), DEFAULTS initialize the array so deep-merge always yields a string[], and the updated tests cover both the list and call handlers end-to-end. Prior review threads have been addressed.

No files require special attention.

Important Files Changed

Filename Overview
src/mcp/tool-registry.ts Adds normalizeToolName and buildDisabledToolSet helpers; extends buildToolList with optional disabledTools parameter for config-driven tool exclusion — logic is correct and handles prefixed/cased names cleanly.
src/mcp/server.ts Plumbs disabledTools config into buildToolList at startup; passes the resulting enabledToolNames Set to createCallToolHandler for an early-rejection guard; cleans up the now-dead list_repos branch from validateMultiRepoAccess.
src/infrastructure/config.ts Adds disabledTools: [] as string[] to DEFAULTS.mcp — minimal, correct change that gives deep-merge a starting value.
src/types.ts Adds optional disabledTools?: string[] to CodegraphConfig.mcp — correct typing that allows user configs to omit the field.
tests/unit/mcp.test.ts Adds unit tests for disabled-tool filtering in both buildToolList and startMCPServer; updates the list_repos single-repo rejection test to match the new Unknown tool error path.
tests/unit/config.test.ts Adds a simple assertion that DEFAULTS.mcp.disabledTools starts as an empty array — adequate coverage for the default.
docs/guides/configuration.md New comprehensive configuration reference covering every config group; the mcp.disabledTools section accurately describes normalization rules and runtime behavior.
README.md Adds an MCP tool filtering quickstart section and a pointer to the new configuration guide — accurate and consistent with implementation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[startMCPServer] --> B[loadConfig]
    B --> C[config.mcp.disabledTools]
    C --> D[buildToolList multiRepo, disabledTools]
    D --> E[buildDisabledToolSet\nnormalizeToolName each entry]
    E --> F{includeTool?\n!disabled.has\nnormalizeToolName tool.name}
    F -- excluded --> G[tool omitted]
    F -- included --> H[enabledTools array]
    H --> I[enabledToolNames Set\ntool.name for each]
    H --> J[tools/list handler\nreturns enabledTools]
    I --> K[tools/call handler]
    K --> L{enabledToolNames.has name ?}
    L -- no --> M[isError: Unknown tool: name]
    L -- yes --> N[validateMultiRepoAccess\ncheck args.repo]
    N --> O[TOOL_HANDLERS.get name\ndispatch to handler]
Loading

Reviews (8): Last reviewed commit: "Merge branch 'main' into feat/mcp-disabl..." | Re-trigger Greptile

Comment thread src/mcp/server.ts Outdated
// Apply config-based MCP page-size overrides
const config = options.config || loadConfig();
initMcpDefaults(config.mcp?.defaults ? { ...config.mcp.defaults } : undefined);
const disabledTools = config.mcp?.disabledTools ? [...config.mcp.disabledTools] : undefined;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Ternary undefined branch is unreachable due to empty-array truthiness

DEFAULTS now sets disabledTools: [] as string[], so after deep-merging, config.mcp.disabledTools will always be an array (at worst []). An empty array is truthy in JavaScript, so config.mcp?.disabledTools ? [...config.mcp.disabledTools] : undefined will always take the truthy branch — the undefined branch can never be reached.

Suggested change
const disabledTools = config.mcp?.disabledTools ? [...config.mcp.disabledTools] : undefined;
const disabledTools = [...(config.mcp?.disabledTools ?? [])];

Fix in Claude Code

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 242944b — replaced the ternary with [...(config.mcp?.disabledTools ?? [])] so disabledTools is always a string[]. Verified the only consumer (buildToolListbuildDisabledToolSet) already normalizes undefined/[] identically (disabledTools || [] produces an empty Set), so behavior is unchanged.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 30, 2026

Codegraph Impact Analysis

8 functions changed7 callers affected across 3 files

  • validateMultiRepoAccess in src/mcp/server.ts:101 (3 transitive callers)
  • createCallToolHandler in src/mcp/server.ts:159 (2 transitive callers)
  • startMCPServer in src/mcp/server.ts:206 (1 transitive callers)
  • normalizeToolName in src/mcp/tool-registry.ts:32 (5 transitive callers)
  • buildDisabledToolSet in src/mcp/tool-registry.ts:39 (4 transitive callers)
  • buildToolList in src/mcp/tool-registry.ts:863 (3 transitive callers)
  • includeTool in src/mcp/tool-registry.ts:865 (4 transitive callers)
  • CodegraphConfig.mcp in src/types.ts:1202 (0 transitive callers)

carlos-alm added a commit to kecsap/ops-codegraph-tool that referenced this pull request Apr 30, 2026
@carlos-alm
Copy link
Copy Markdown
Contributor

@greptileai

carlos-alm added a commit to kecsap/ops-codegraph-tool that referenced this pull request Apr 30, 2026
The 'list_repos' tool is excluded from 'enabledToolNames' when 'multiRepo'
is false, so 'createCallToolHandler' rejects single-repo 'list_repos'
calls with 'Unknown tool' before they ever reach 'validateMultiRepoAccess'.
That made the 'name === "list_repos"' branch unreachable. Drop the dead
branch and the now-unused 'name' parameter; existing test
'rejects list_repos in single-repo mode' covers the early-rejection path.
@carlos-alm
Copy link
Copy Markdown
Contributor

Addressed Greptile's summary callout about the unreachable 'list_repos' branch in 'validateMultiRepoAccess'. Cleaned up in b37d464 — dropped the dead branch and the now-unused 'name' parameter. The existing test 'rejects list_repos in single-repo mode' (tests/unit/mcp.test.ts:1058) already covers the early-rejection path via 'enabledToolNames'.

@carlos-alm
Copy link
Copy Markdown
Contributor

@greptileai

kecsap and others added 9 commits May 1, 2026 00:17
- Reorder createCallToolHandler so the disabled-tool check runs before
  validateMultiRepoAccess, giving config-disabled tools a consistent
  "Unknown tool" response regardless of multi-repo state.
- Mark CodegraphConfig.mcp.disabledTools optional to match the runtime
  guard (config.mcp?.disabledTools) and tolerate partial configs.
- Update list_repos single-repo test to reflect the unified
  "Unknown tool" path.
- Apply biome formatting to normalizeToolName.
- README: short subsection under Configuration with example and link.
- docs/guides/mcp-tool-filtering.md: extensive guide covering motivation,
  name normalization, runtime behavior, full tool catalog, and recipes.
Per review feedback, the previous guide was too narrowly scoped. Replace
with docs/guides/configuration.md covering every key in DEFAULTS:
file selection, build, query, embeddings, llm, search, ci, manifesto,
check, coChange, analysis, community, structure, risk, display, mcp.

README points to the new guide and keeps the short MCP tool-filtering
example for the PR's headline feature.
The 'list_repos' tool is excluded from 'enabledToolNames' when 'multiRepo'
is false, so 'createCallToolHandler' rejects single-repo 'list_repos'
calls with 'Unknown tool' before they ever reach 'validateMultiRepoAccess'.
That made the 'name === "list_repos"' branch unreachable. Drop the dead
branch and the now-unused 'name' parameter; existing test
'rejects list_repos in single-repo mode' covers the early-rejection path.
@carlos-alm carlos-alm force-pushed the feat/mcp-disabled-tools branch from 0df784b to 8842024 Compare May 1, 2026 06:17
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 1, 2026

Want your agent to iterate on Greptile's feedback? Try greploops.

@carlos-alm carlos-alm merged commit e9fd336 into optave:main May 1, 2026
21 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators May 1, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Add option to remove specific tools from the MCP server schema

2 participants