feat(core): add global tool registry#2003
Merged
senamakel merged 3 commits intoMay 18, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a read-only global tool registry: new module exposing ChangesTool Registry Implementation
Sequence Diagram(s)sequenceDiagram
participant Agent as Agent/Client
participant RPC as JSON-RPC Server
participant handle_list as handle_list()
participant list_tools as list_tools()
participant registry_entries as registry_entries()
participant MCP as MCP Tool Specs
participant Controllers as Controller Schemas
Agent->>RPC: POST openhuman.tool_registry_list
RPC->>handle_list: invoke handler
handle_list->>list_tools: request ToolRegistryList
list_tools->>registry_entries: aggregate sources
registry_entries->>MCP: collect tool_specs
registry_entries->>Controllers: collect schemas
registry_entries-->>list_tools: sorted entries
list_tools-->>handle_list: RpcOutcome<ToolRegistryList>
handle_list-->>RPC: ToolRegistryList (tools array)
RPC-->>Agent: JSON response with tools
Agent->>RPC: POST openhuman.tool_registry_get {tool_id: "tools.web_search"}
RPC->>handle_list: invoke handler with tool_id
handle_list->>handle_list: required_tool_id validation
handle_list->>list_tools: lookup specific tool
list_tools-->>handle_list: ToolRegistryEntry
handle_list-->>RPC: entry with transport/route/schemas
RPC-->>Agent: ToolRegistryEntry JSON
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/openhuman/tool_registry/schemas.rs (1)
80-85: ⚡ Quick winAdd debug logging on
tool_registry.geterror paths.
required_tool_id(¶ms)?andops::get_tool(tool_id)?can return early without any error log, making RPC failures harder to trace in production.Proposed fix
fn handle_get(params: Map<String, Value>) -> ControllerFuture { Box::pin(async move { - let tool_id = required_tool_id(¶ms)?; + let tool_id = match required_tool_id(¶ms) { + Ok(value) => value, + Err(err) => { + log::debug!("[tool_registry] rpc get rejected invalid tool_id: {err}"); + return Err(err); + } + }; log::debug!("[tool_registry] rpc get requested tool_id={tool_id}"); - to_json(crate::openhuman::tool_registry::ops::get_tool(tool_id)?) + match crate::openhuman::tool_registry::ops::get_tool(tool_id) { + Ok(outcome) => to_json(outcome), + Err(err) => { + log::debug!("[tool_registry] rpc get failed tool_id={tool_id} error={err}"); + Err(err) + } + } }) }As per coding guidelines: "Use
log/tracingatdebugortracelevel on RPC entry and exit, error paths..."Also applies to: 88-95
🤖 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 `@src/openhuman/tool_registry/schemas.rs` around lines 80 - 85, The RPC handler handle_get currently uses the `?` operator on `required_tool_id(¶ms)` and `crate::openhuman::tool_registry::ops::get_tool(tool_id)` which can return early without logging; change the implementation to explicitly handle errors (e.g., match or .map_err/.and_then) so you log a debug/error message with the error details before returning the error, keeping the existing successful debug log and returning the original error after logging; do the same pattern for the analogous RPC handler in this module that handles the other tool-registry RPC (the sibling handler implemented just after handle_get) so all entry, success and error paths emit debug/tracing logs referencing the function names (`handle_get`, `required_tool_id`, `ops::get_tool`, `to_json`) for easier tracing.
🤖 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 `@docs/TEST-COVERAGE-MATRIX.md`:
- Line 397: The new feature row 11.1.5 was added to the matrix but the summary
totals at the bottom were not updated; update the summary counts (the "✅
Covered" count and the "Total leaves" count) in docs/TEST-COVERAGE-MATRIX.md to
reflect inclusion of feature 11.1.5 (increment the covered leaves and total
leaves by one as appropriate) and ensure any percentage or derived totals are
recalculated to remain consistent with the new row.
In `@src/openhuman/tool_registry/ops.rs`:
- Around line 47-55: The registry build currently uses entries.insert(...) for
entries created by mcp_tool_entry(spec) and controller_tool_entry(&schema),
which silently overwrites existing entries on duplicate tool_id; change both
insertion points to detect collisions and fail fast by checking
entries.contains_key(&entry.tool_id) (or using
entries.entry(entry.tool_id.clone()).or_insert_with and matching on Occupied)
and return/panic with a clear error including the duplicate tool_id and the
source (e.g., from mcp_tool_entry(spec) vs controller_tool_entry(&schema)); keep
the same entry creation (mcp_tool_entry and controller_tool_entry) but prevent
silent overwrites by aborting on duplicate tool_id.
---
Nitpick comments:
In `@src/openhuman/tool_registry/schemas.rs`:
- Around line 80-85: The RPC handler handle_get currently uses the `?` operator
on `required_tool_id(¶ms)` and
`crate::openhuman::tool_registry::ops::get_tool(tool_id)` which can return early
without logging; change the implementation to explicitly handle errors (e.g.,
match or .map_err/.and_then) so you log a debug/error message with the error
details before returning the error, keeping the existing successful debug log
and returning the original error after logging; do the same pattern for the
analogous RPC handler in this module that handles the other tool-registry RPC
(the sibling handler implemented just after handle_get) so all entry, success
and error paths emit debug/tracing logs referencing the function names
(`handle_get`, `required_tool_id`, `ops::get_tool`, `to_json`) for easier
tracing.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9963d1dd-979e-4457-b775-8ba372bea530
📒 Files selected for processing (14)
docs/TEST-COVERAGE-MATRIX.mdgitbooks/developing/mcp-server.mdscripts/feature-ids.jsonsrc/core/all.rssrc/core/all_tests.rssrc/openhuman/about_app/catalog.rssrc/openhuman/about_app/catalog_tests.rssrc/openhuman/mcp_server/mod.rssrc/openhuman/mod.rssrc/openhuman/tool_registry/mod.rssrc/openhuman/tool_registry/ops.rssrc/openhuman/tool_registry/schemas.rssrc/openhuman/tool_registry/types.rstests/json_rpc_e2e.rs
This was referenced May 18, 2026
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
tool_registrycore domain withopenhuman.tool_registry_listandopenhuman.tool_registry_getJSON-RPC controllers.toolsnamespace controller-backed tools into one read-only discovery registry.tool_id, transport, route, version, input/output schemas, allowed agents, tags, enabled state, and health metadata for each entry.Problem
Solution
src/openhuman/tool_registry/as a small read-only domain.mcp_server::tool_specs()for MCP tools andtools::all_tools_controller_schemas()for controller-backed tools.tools/call, and controller-backed tools still call their existing JSON-RPC methods.Submission Checklist
11.1.5 Global tool registrytodocs/TEST-COVERAGE-MATRIX.mdandscripts/feature-ids.json.## RelatedCloses #NNN— this PR is the core discovery MVP for Feature Request : Global MCP tool registry - centralised discovery, versioning, and routing for all MCP tools across the swarm #1848 and should not close the broader dashboard/routing/policy issue.Impact
tools.*controller methods keep their current routes.Related
11.1.5AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
codex/OH-1848-tool-registrya4de942ffc708eec7ba169e8fdac845a8fe0884cValidation Run
pnpm --filter openhuman-app format:checkpnpm typecheckpnpm debug rust -- --lib tool_registry -- --nocapture(13 passed after CodeRabbit fix);pnpm debug rust -- --test json_rpc_e2e json_rpc_tool_registry_lists_and_gets_entries -- --nocapture;pnpm debug rust -- --lib namespace_description_known_namespaces;pnpm debug rust -- --lib catalog;node scripts/check-coverage-matrix.mjscargo fmt --manifest-path Cargo.toml --all --check;cargo check --manifest-path Cargo.toml;git diff --check;git diff --cached --checkpnpm --filter openhuman-app format:checkand the pre-push hook also rancargo fmt --manifest-path app/src-tauri/Cargo.toml --all --checkandcargo check --manifest-path app/src-tauri/Cargo.toml.Validation Blocked
command:pnpm debug rusterror:full Rust suite failed in this environment with three failures outside this PR surface:openhuman::composio::action_tool::tests::factory_routes_through_direct_when_mode_is_direct(composio backend mode unavailable: no backend session token),openhuman::tools::implementations::network::url_guard::tests::dns_check_passes_for_public_domain(google.comresolved to private/local198.18.0.75), andopenhuman::wallet::execution::tests::execute_prepared_broadcasts_erc20_transfer_using_default_token_catalog(quote ... not found). Summary:7568 passed; 3 failed; 7 ignored.impact:Focused tests for the changed registry surface pass; full-suite failures appear unrelated/environmental and should be checked separately from this PR.Behavior Changes
openhuman.tool_registry_listandopenhuman.tool_registry_get.Parity Contract
tools/callrouting andtools.*JSON-RPC methods are unchanged.Duplicate / Superseded PR Handling
#1848/tool registry; only unrelated open MCP PR feat(mcp): add SearXNG search tool #1988 matched a broad keyword search.Summary by CodeRabbit
New Features
Documentation
Tests