From 283e7bb684036dc95dfc97c47b64438dce4f638d Mon Sep 17 00:00:00 2001 From: David Christensen Date: Fri, 29 May 2026 05:11:01 -0700 Subject: [PATCH] refactor(server): gate tool_catalog behind test-support `mcp::tool_catalog` was `pub mod` and `TOOL_DEFS` a `pub static`, exposed on the (doc-hidden) library surface solely so the binary's test-support `dump-tool-catalog` subcommand could reach the catalog. Every production caller is in-crate, routing through `dispatch` and `server`. Demote the module to `pub(crate)` and re-export `TOOL_DEFS` at `mcp::TOOL_DEFS` under `#[cfg(any(test, feature = "test-support"))]`, mirroring how `execute_tool_for_test` is gated in `server.rs`. The binary's `dump_tool_catalog` cli imports the gated re-export. In-crate callers keep the canonical `crate::mcp::tool_catalog` path. Also fix a stale `output_schema` doc reference in the wire-conformance test left by #336's rename. Closes #339 --- .../rimap-server/src/cli/dump_tool_catalog.rs | 2 +- crates/rimap-server/src/mcp/mod.rs | 17 ++++++++++++----- crates/rimap-server/src/mcp/tool_catalog.rs | 8 ++++---- .../rimap-server/tests/mcp_wire_conformance.rs | 2 +- 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/crates/rimap-server/src/cli/dump_tool_catalog.rs b/crates/rimap-server/src/cli/dump_tool_catalog.rs index ce836f9a..b1776113 100644 --- a/crates/rimap-server/src/cli/dump_tool_catalog.rs +++ b/crates/rimap-server/src/cli/dump_tool_catalog.rs @@ -7,7 +7,7 @@ use std::io::Write; use rimap_core::tool::ToolName; -use rimap_server::mcp::tool_catalog::TOOL_DEFS; +use rimap_server::mcp::TOOL_DEFS; /// Print each entry of the static `TOOL_DEFS` map as one line of /// JSON to the given writer. Iteration order follows `ToolName::all()` diff --git a/crates/rimap-server/src/mcp/mod.rs b/crates/rimap-server/src/mcp/mod.rs index 8565de53..c90e5184 100644 --- a/crates/rimap-server/src/mcp/mod.rs +++ b/crates/rimap-server/src/mcp/mod.rs @@ -8,11 +8,18 @@ pub mod preinit; pub mod response; pub(crate) mod result_provenance; pub mod server; -// `tool_catalog` is `pub` (doc-hidden via the parent `#[doc(hidden)] pub mod -// mcp` in `lib.rs`) so the binary's test-support `dump-tool-catalog` -// subcommand (#264) can reach `TOOL_DEFS`. Production callers route through -// `dispatch` and `server` and do not import this module directly. -pub mod tool_catalog; +// `tool_catalog` is in-crate only: production callers route through +// `dispatch` and `server`. `TOOL_DEFS` is re-exported below under +// `test-support` so the binary's `dump-tool-catalog` subcommand (#264) +// can reach it without widening the module's surface for production. +pub(crate) mod tool_catalog; + +/// Re-export of the static tool catalog for the binary's test-support +/// `dump-tool-catalog` subcommand (#264). Gated to keep `TOOL_DEFS` out +/// of the production library surface, mirroring `execute_tool_for_test` +/// in [`server`]. +#[cfg(any(test, feature = "test-support"))] +pub use tool_catalog::TOOL_DEFS; pub(crate) mod tool_name; pub mod wire_validator; diff --git a/crates/rimap-server/src/mcp/tool_catalog.rs b/crates/rimap-server/src/mcp/tool_catalog.rs index cc501a7b..23384ac4 100644 --- a/crates/rimap-server/src/mcp/tool_catalog.rs +++ b/crates/rimap-server/src/mcp/tool_catalog.rs @@ -294,10 +294,10 @@ fn build_tool_defs() -> HashMap { /// Memoized MCP tool definitions. Built once at first access; each /// `list_tools` call reuses the same `Arc` for schemas. /// -/// `pub` so the binary's test-support `dump-tool-catalog` subcommand -/// (#264) can iterate the catalog from outside the library crate. The -/// parent `mcp` module is `#[doc(hidden)]` so this does not become a -/// stable library API. +/// `pub` so the gated `mcp::TOOL_DEFS` re-export can expose it to the +/// binary's test-support `dump-tool-catalog` subcommand (#264); the +/// `tool_catalog` module itself is `pub(crate)`, so this is not a stable +/// library API. In-crate callers reach it via `crate::mcp::tool_catalog`. pub static TOOL_DEFS: std::sync::LazyLock> = std::sync::LazyLock::new(build_tool_defs); diff --git a/crates/rimap-server/tests/mcp_wire_conformance.rs b/crates/rimap-server/tests/mcp_wire_conformance.rs index 30aee8ea..7dd9465e 100644 --- a/crates/rimap-server/tests/mcp_wire_conformance.rs +++ b/crates/rimap-server/tests/mcp_wire_conformance.rs @@ -261,7 +261,7 @@ async fn wire_published_output_schema_matches_fixture() { // under tests/fixtures/rimap-tool-schemas/. Both are generated from // the same Rust types via schemars — drift signals either a // regenerate-fixtures miss or a divergence between dump_tool_schemas - // and tool_catalog::output_schema. + // and tool_catalog::tool_def_parts. // // Scope: the zero-account harness exercises exactly two tools // (`list_accounts`, `use_account`). Full-catalog coverage lives in