Skip to content

fix(mcp): make GitHub skill tools discoverable through omniroute_tool_search - #10575

Merged
diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.50from
branben:fix/mcp-github-tool-search-parity
Aug 18, 2026
Merged

diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.50from
branben:fix/mcp-github-tool-search-parity

Conversation

@branben

@branben branben commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

fix(mcp): make GitHub skill tools discoverable through omniroute_tool_search

Summary

The MCP server registers three GitHub skill tools (omniroute_github_skills_search, omniroute_github_skills_scan, omniroute_github_skills_install), and TOTAL_MCP_TOOL_COUNT already counts them — but getAllToolDefinitions() did not include the githubSkillTools collection. As a result, a client that already knew an exact tool name could call it, but a client relying on omniroute_tool_search could not discover it.

This PR adds githubSkillTools to the search catalog and adds regression coverage at both layers: catalog construction and the client-visible MCP search path.

Why this matters

This is a focused instance of a recurring MCP contract-drift pattern:

  • #6854 — MCP tool-count / collection drift
  • #7579 — client-visible missing tools
  • #10159 — MCP search-provider schema drift
  • #10209 — existing provider-enum fix, which this PR complements rather than duplicates

This PR fixes only the tool-search catalog omission. The linked issues describe related discovery and contract-parity layers; the REST catalog and observability layers should be handled separately.

Changes

  • Add githubSkillTools to getAllToolDefinitions() (one import + one collection entry, matching the existing pattern for memoryTools, skillTools, agentSkillTools, etc.).
  • Catalog test: assert all three GitHub skill tools are present in getAllToolDefinitions().
  • Client test: assert an in-memory MCP client can discover all three through omniroute_tool_search.
  • Changelog fragment under changelog.d/fixes/.

Scope boundaries

This PR does not change:

  • MCP server registration or tool handlers;
  • provider schemas or provider visibility;
  • /api/mcp/tools behavior;
  • MCP transport or stateless-session behavior;
  • startup, OAuth, or admission control;
  • observability or MCP status/doctor endpoints.

Validation

Focused tests and gates, run under Node 24.19.0:

npm run check:node-runtime
npm run test:vitest -- \
  open-sse/mcp-server/__tests__/toolSearch.catalog.test.ts \
  open-sse/mcp-server/__tests__/toolSearch.tool.test.ts
npm run check:open-sse-typecheck
npm run check:test-discovery
npm run check:changelog-integrity
npm run lint

Results:

  • Focused MCP tests: 2 files, 8 tests passed (hermetic temp DATA_DIR, no writes to the developer's ~/.omniroute).
  • Open SSE typecheck: passed (only pre-existing frozen baseline errors remain).
  • Test discovery / changelog integrity: passed.
  • Full repository lint: passed (0 errors, 0 warnings across 8,310 files).
  • git diff --check: passed.

Review notes

Every tool in the searchable static MCP collection must be discoverable through omniroute_tool_search; this PR adds the previously omitted GitHub skill collection.

This change is deliberately narrow: it does not claim every MCP projection must have an identical inventory, nor that it resolves the entire historical MCP visibility issue cluster.

@branben
branben requested a review from diegosouzapw as a code owner August 17, 2026 01:27
…_search

Add githubSkillTools to getAllToolDefinitions() so the searchable MCP
catalog matches TOTAL_MCP_TOOL_COUNT, which already counts them. The
GitHub skill tools were registered and counted but missing from the
catalog, so omniroute_tool_search could not surface them.

Adds regression tests at both layers: catalog aggregation and
client-visible discovery via the MCP client.
@branben
branben force-pushed the fix/mcp-github-tool-search-parity branch from 7545c17 to 95dbffe Compare August 17, 2026 01:27
@diegosouzapw

Copy link
Copy Markdown
Collaborator

This is exactly the tool-search catalog omission it describes, and it's well-tested — I re-ran both changed vitest files locally (8/8 pass) and linted the touched files (0 problems). I traced every CI red on this PR (onnxruntime dependency-contract test, CLI_TOOLS counts, Alibaba free-tier classification, repo-state sync gate) and they're all pre-existing base drift that landed on release/v3.8.50 after your branch point, not caused by this diff. We'll rebase onto the current tip before merging — no action needed from you.

@diegosouzapw
diegosouzapw merged commit 228ef6f into cheaper-inference:release/v3.8.50 Aug 18, 2026
5 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…_search (cheaper-inference#10575)

Add githubSkillTools to getAllToolDefinitions() so the searchable MCP
catalog matches TOTAL_MCP_TOOL_COUNT, which already counts them. The
GitHub skill tools were registered and counted but missing from the
catalog, so omniroute_tool_search could not surface them.

Adds regression tests at both layers: catalog aggregation and
client-visible discovery via the MCP client.

Co-authored-by: Brandon Bennett <brandonbennett@macbookair.myfiosgateway.com>
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