Skip to content

feat: add configurable tool search and generic invocation - #19

Open
calvinmclean wants to merge 34 commits into
obot-platform:mainfrom
calvinmclean:feat/8065-vmcp-tool-search
Open

calvinmclean wants to merge 34 commits into
obot-platform:mainfrom
calvinmclean:feat/8065-vmcp-tool-search

Conversation

@calvinmclean

@calvinmclean calvinmclean commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Add opt-in toolSearch configuration that exposes search and generic invocation tools in place of direct component tool calls.
  • Index the effective tool catalog with Bleve for ranked, paginated search; return tool schemas and references for invocation.
  • Route invocation through the current catalog, reject unavailable or changed tool references, and refresh search catalogs after tool-change notifications.
  • Add coverage for search, invocation, refresh behavior, and indexing performance.

Addresses obot-platform/obot#8065 for the mmmcp search and invocation layer.

Add Bleve-backed discovery, generic tool invocation, and off/search/hybrid modes. Keep calls bound to current catalog grants and cover behavior with unit and Everything integration tests.
Move Bleve search and synthetic tool definitions into toolsearch, rename the public tools with the mmmcp prefix, and let searches wait for background indexing with retryable timeouts. Add concurrency tests and document the exported interface.
Compile a client-visible tool list and call lookup for off, search, and hybrid
modes. Keep the allowed component routes in the same snapshot for reference
invocation, while the catalog enforces which names clients may call.

Resolve search and generic calls in the catalog and let the handler use the
existing downstream invocation path for resolved component routes. This removes
mode-specific dispatch and the extra executeTool wrapper from the handler.

Build search documents from effective tool definitions, indexing configured
names and descriptions while excluding disabled tools. Include the visible
view and mode in catalog cursor identity, remove unreleased compatibility
aliases, and cover routing, overrides, and cursor isolation in tests.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The required hybrid mode is absent, and unresolved build-state and registry concurrency defects remain.

Review effort: Balanced
Findings: 2 High severity · 1 Medium severity

Open (3)
What changed in this PR

Adds Bleve-backed tool discovery and generic invocation to the composite MCP catalog, with refresh handling and protocol integration.

Changes:

  • Adds configurable search-only tool exposure and generic invocation.
  • Adds ranked, paginated indexing with stale-reference validation.
  • Expands refresh, protocol, integration, and performance coverage.
File Description
README.md Documents tool-search behavior and readiness semantics.
catalog/​catalog.go Tracks search mode and visible tools.
catalog/​compiler.go Builds search-enabled catalogs and reserves synthetic names.
catalog/​compiler_test.go Tests routing with duplicate component names.
catalog/​discovery.go Builds indexes for directly compiled catalogs.
catalog/​pagination.go Paginates mode-visible tools.
catalog/​refresh_test.go Tests blocking and failed refresh behavior.
catalog/​registry.go Adds index lifecycle, staleness, and automatic refresh.
catalog/​routes.go Adds exposed names and searchable references.
catalog/​search.go Integrates indexing and reference lookup.
catalog/​search_test.go Tests search, overrides, revisions, and collisions.
catalog/​tool_calls.go Resolves synthetic search and invocation calls.
config/​config.go Adds the tool-search configuration field.
config/​load.go Loads tool-search configuration.
config/​load_test.go Tests configuration parsing and defaults.
go.mod Adds Bleve and its dependencies.
go.sum Records dependency checksums.
handler.go Lists visible tools and dispatches resolved calls.
health.go Updates readiness recovery behavior.
health_test.go Tests readiness after refresh recovery.
http_cache_test.go Updates tool-list cache-scope expectations.
http_result_translation_test.go Tests synthetic result protocol translation.
integration/​everything/​everything_test.go Exercises direct and search modes end-to-end.
notifications.go Tracks mode changes in tool notifications.
notifications_test.go Updates subscription tests for search mode.
tool_search_test.go Tests search, invocation, revocation, and staleness.
toolsearch/​search.go Implements indexing, search, and tool definitions.
toolsearch/​search_bench_test.go Benchmarks catalog indexing.
toolsearch/​search_test.go Tests pagination and asynchronous index behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread catalog/registry.go
Comment thread config/config.go
Comment thread toolsearch/search.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Tool revisions do not identify backend routes, and search indexing and documented HTTP failure behavior have unresolved issues.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (3)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Enforce schema term cap while appending property names

toolsearch/​search.go:329

The intended 256-term bound is bypassed by a flat schema: this loop appends every property name without rechecking len(parts), so an untrusted component can make indexing consume memory and CPU proportional to an arbitrarily large property map. Stop the loop once the cap is reached.

Low severity Expose CATALOG_UNAVAILABLE through the HTTP frontend

README.md:99

This documented error is not observable through the HTTP frontend. selectFrontendImplementation calls Registry.Get before dispatching every POST and converts this error to HTTP 500 with the body frontend identity unavailable (http.go:62-70), so HTTP clients never receive CATALOG_UNAVAILABLE. Either preserve this stable reason through that pre-dispatch path or document the actual transport-specific behavior.

Comment thread catalog/search.go Outdated
@calvinmclean
calvinmclean requested a balanced review from Copilot October 2, 2026 20:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Client-visible revisions expose hashes of secret-bearing configuration, and argument validation conflicts with the advertised schemas.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (1)
Previously missed (3)

In code that hasn't changed since last review

Medium severity Reject non-object arguments instead of forwarding invalid JSON

toolsearch/​search.go:107

An explicitly supplied arguments: null (or an array/string) has a nonzero RawMessage length and is forwarded downstream, despite this synthetic tool declaring arguments as an object. Validate the provided JSON shape here; only a truly omitted field should default to {}.

Medium severity Distinguish explicit zero limit from an omitted limit

toolsearch/​search.go:508

The published schema and README require a positive limit, but an explicit limit: 0 is silently treated as if the field were omitted. Decode limit as an optional value so only absence selects the default and zero returns INVALID_ARGUMENTS; update the zero-limit test accordingly.

Low severity Correct the error message for optional arguments

catalog/​tool_calls.go:47

This error says arguments is required even though the advertised schema makes it optional and ParseCallArguments defaults omission to {}. Report that tool and revision are required and that arguments must be an object when provided, so clients can correct the actual invalid field.

Comment thread catalog/search.go Outdated
@calvinmclean
calvinmclean marked this pull request as ready for review October 5, 2026 16:30
@calvinmclean
calvinmclean requested a balanced review from Copilot October 5, 2026 16:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Concurrent callers can hang when an initial search-catalog compilation fails.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (1)

Comment thread catalog/registry.go

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Explicit refreshes can temporarily serve stale catalogs and allow revoked tools to remain callable.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (1)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Generic invocation scans and re-marshals the full tool catalog

catalog/​search.go:56

Every generic invocation linearly scans the full tool catalog and then re-marshals the matched definition to recompute its revision. This makes the hot call path scale with catalog size and schema size—the same feature is benchmarked with up to 5,000 tools. Store routes and precomputed revisions by exposed reference when the immutable catalog is built so invocation is an O(1) lookup.

Medium severity Term cap bypassed while appending wide schema properties

toolsearch/​search.go:344

The intended term cap is checked only when entering walk; this loop continues appending every property name after nested calls start returning. A tool with a very wide schema can therefore create an arbitrarily large terms string and index workload despite the 256-term guard.

Comment thread catalog/registry.go

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Explicit zero limits currently bypass the documented positive-limit validation and silently use the default.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Reject explicit zero limit instead of defaulting to five

toolsearch/​search.go:554

An explicit "limit": 0 is treated as if the field were omitted, even though this tool's schema declares minimum: 1 and the README requires a positive limit. This silently converts invalid client input into a five-result request (and the added test currently codifies that mismatch). Represent the field as a pointer so omission can default to 5 while an explicit zero is rejected.

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