feat(server): rfc 0027 green transport — /mcp behind querier.mcp.enabled - #413
Conversation
The rmcp streamable-HTTP service (official MCP Rust SDK, server-side features only) nests at /mcp on the querier listener when querier.mcp.enabled / OURIOS_QUERIER_MCP_ENABLED is set (default off), behind the RFC 0026 bearer gate as an axum layer — 401 before any MCP dispatch, open mode passes through. The handler announces the server and the treat-log-bodies-as-data instruction; tools and the grammar resource land in the next slices. RFC0027.1 goes green. Also bumps crossbeam-epoch past RUSTSEC-2026-0204 (pre-existing advisory caught by the deny run). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 28 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds an opt-in RFC 0027 MCP (Model Context Protocol) transport surface served at ChangesMCP Surface Implementation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Router as router_from_querier
participant Middleware as require_bearer
participant McpHandler as OuriosMcp
Client->>Router: POST /mcp (initialize)
alt mcp_enabled = false
Router-->>Client: 404 NOT_FOUND
else mcp_enabled = true
Router->>Middleware: forward request
alt auth configured, missing/invalid bearer
Middleware-->>Client: 401 UNAUTHORIZED
else authenticated or no auth required
Middleware->>McpHandler: dispatch initialize
McpHandler-->>Client: 200 OK (serverInfo, protocolVersion)
end
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
Adds the RFC 0027 “green transport” slice for an MCP endpoint in the querier, gated behind a new querier.mcp.enabled config/env flag so existing routers remain unchanged unless explicitly opted in.
Changes:
- Introduces a new
/mcpsub-router usingrmcpstreamable HTTP, wrapped with the RFC 0026 bearer authentication gate. - Adds config plumbing for
querier.mcp.enabled(file schema + env var) and wires it into querier router construction andserve. - Converts scenario RFC0027.1 from an ignored stub into an async integration test covering 404 (off), initialize handshake (on), and bearer gating behavior.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/ourios-server/tests/it/rfc0027_mcp.rs | Implements RFC0027.1 integration test for MCP gating/placement and bearer behavior. |
| crates/ourios-server/src/querier.rs | Adds mcp_enabled to config and nests /mcp when enabled; introduces router_with_mcp. |
| crates/ourios-server/src/mcp.rs | New MCP transport module: streamable-HTTP service + bearer middleware layer. |
| crates/ourios-server/src/main.rs | Adds env/file parsing for OURIOS_QUERIER_MCP_ENABLED / querier.mcp.enabled and passes it into querier config. |
| crates/ourios-server/src/lib.rs | Registers the new mcp module within the crate. |
| crates/ourios-server/src/config/file.rs | Extends the file schema with querier.mcp.enabled and includes it in env-substitution. |
| crates/ourios-server/Cargo.toml | Adds the rmcp dependency with server + streamable-http-server features only. |
| Cargo.lock | Locks rmcp and transitive deps; bumps crossbeam-epoch to 0.9.20. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
crates/ourios-server/src/main.rs (1)
364-398: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a unit test for the
mcp_enabled_rawtruthy branch.Every updated
build_querier_configtest call passesNonefor the newmcp_enabled_rawargument; none assertQuerierParams.mcp_enabled == trueforSome("1")/Some("true")/Some("yes"). The only positive-path coverage lives in the separate integration test, which exercisesrouter_with_mcpdirectly and bypasses this parsing function.✅ Suggested test addition
+ #[test] + fn build_querier_config_derives_mcp_enabled_from_truthy_values() { + for raw in [Some("1"), Some("true"), Some("yes")] { + let params = build_querier_config(Some("1"), None, None, raw) + .expect("ok") + .expect("enabled"); + assert!(params.mcp_enabled, "mcp enabled for mcp_enabled_raw = {raw:?}"); + } + for raw in [None, Some("0"), Some("false"), Some("nope")] { + let params = build_querier_config(Some("1"), None, None, raw) + .expect("ok") + .expect("enabled"); + assert!(!params.mcp_enabled, "mcp disabled for mcp_enabled_raw = {raw:?}"); + } + }As per coding guidelines, "Unit tests must be next to the code and are mandatory for anything non-trivial."
Also applies to: 1345-1406
🤖 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 `@crates/ourios-server/src/main.rs` around lines 364 - 398, Add a unit test for build_querier_config that exercises the mcp_enabled_raw truthy path. Update the existing build_querier_config test cases to pass the new argument explicitly, then add assertions that QuerierParams.mcp_enabled is true when mcp_enabled_raw is Some("1"), Some("true"), and Some("yes"). Keep the test next to build_querier_config so the parsing behavior is covered directly rather than only through router_with_mcp integration tests.Source: Coding guidelines
crates/ourios-server/src/mcp.rs (1)
52-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo unit tests colocated with
require_bearer/mcp_router.This file has no
#[cfg(test)]module; the bearer-gating branch logic is only exercised indirectly via the integration test intests/it/rfc0027_mcp.rs. As per coding guidelines, "Unit tests must be next to the code and are mandatory for anything non-trivial," so a focused unit test onrequire_bearer(open mode passes through, missing/invalid bearer → 401, valid bearer → passthrough) would satisfy the guideline directly rather than relying solely on full-router integration coverage.🤖 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 `@crates/ourios-server/src/mcp.rs` around lines 52 - 81, Add a local #[cfg(test)] module next to require_bearer and mcp_router with focused unit coverage for the bearer gate instead of relying only on tests/it/rfc0027_mcp.rs. Exercise require_bearer directly for the open/pass-through case, missing or invalid Authorization returning 401, and a valid bearer flowing through to next.run, using the existing authenticate_bearer behavior and mcp_router setup only as needed to locate the logic.Source: Coding guidelines
🤖 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 `@crates/ourios-server/src/mcp.rs`:
- Around line 70-81: The MCP router is using
StreamableHttpServerConfig::default(), which keeps the host allowlist limited to
loopback and causes /mcp to be rejected on non-loopback deployments. Update
mcp_router to accept an allowed_hosts configuration (or equivalent trusted-proxy
toggle) and pass it into StreamableHttpService::new instead of relying on the
default config; keep the require_bearer middleware unchanged and make sure the
new config is threaded from the caller that builds the router.
- Around line 35-45: Use struct initialization instead of creating ServerInfo
with ServerInfo::default() and then reassigning fields in get_info. Update the
get_info method in mcp.rs to construct ServerInfo directly with the desired
capabilities and instructions, likely using struct update syntax, so it no
longer triggers clippy::field_reassign_with_default.
- Around line 1-81: The /mcp route in mcp_router currently has no metrics or
tracing instrumentation, so add request-level observability around the
StreamableHttpService and bearer middleware. Update mcp_router and/or
require_bearer to emit a request span and Prometheus counter/histogram for MCP
requests, including success/failure outcomes and latency. If this is
intentionally deferred to a later slice, add an explicit note in the module docs
or the mcp_router comment stating that MCP metrics and tracing are pending and
where they will be implemented.
In `@crates/ourios-server/src/querier.rs`:
- Around line 254-264: The
`Router::layer(DefaultBodyLimit::max(MAX_BODY_BYTES))` in `querier.rs` is
applied before `router.nest("/mcp", crate::mcp::mcp_router(auth))`, so the
nested MCP routes bypass the body-size limit. Move the `DefaultBodyLimit`
application to after all routes are added, or apply the same limit within
`crate::mcp::mcp_router` so both `/v1/query` and `/mcp` are covered. Use the
`router` construction and `mcp_router(auth)` call as the reference points when
updating the layering order.
In `@crates/ourios-server/tests/it/rfc0027_mcp.rs`:
- Around line 45-109: The MCP router currently uses
StreamableHttpServerConfig::default(), which only permits loopback hosts and can
cause a 403 before auth or MCP handling. Update querier::router_with_mcp to
accept and pass through allowed_hosts from config into the MCP server config, or
explicitly constrain the test to loopback-only behavior. Use the router_with_mcp
setup in rfc0027_1_gating_and_placement to verify the intended host policy.
---
Nitpick comments:
In `@crates/ourios-server/src/main.rs`:
- Around line 364-398: Add a unit test for build_querier_config that exercises
the mcp_enabled_raw truthy path. Update the existing build_querier_config test
cases to pass the new argument explicitly, then add assertions that
QuerierParams.mcp_enabled is true when mcp_enabled_raw is Some("1"),
Some("true"), and Some("yes"). Keep the test next to build_querier_config so the
parsing behavior is covered directly rather than only through router_with_mcp
integration tests.
In `@crates/ourios-server/src/mcp.rs`:
- Around line 52-81: Add a local #[cfg(test)] module next to require_bearer and
mcp_router with focused unit coverage for the bearer gate instead of relying
only on tests/it/rfc0027_mcp.rs. Exercise require_bearer directly for the
open/pass-through case, missing or invalid Authorization returning 401, and a
valid bearer flowing through to next.run, using the existing authenticate_bearer
behavior and mcp_router setup only as needed to locate the logic.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c5a28866-4800-452d-87d4-8056fca960e2
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
crates/ourios-server/Cargo.tomlcrates/ourios-server/src/config/file.rscrates/ourios-server/src/lib.rscrates/ourios-server/src/main.rscrates/ourios-server/src/mcp.rscrates/ourios-server/src/querier.rscrates/ourios-server/tests/it/rfc0027_mcp.rs
…abilities The nested router now carries the same DefaultBodyLimit (Router::layer does not reach routes nested after it); allowed_hosts opens (rmcp's loopback-only default would 403 real deployments pre-auth; the bearer layer is the gate, matching the JSON API's Host posture); capabilities stay empty until the tools slice attaches them; the mcp flag parsing gains its unit test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What
RFC 0027 green slice 1 (transport) — Scenario RFC0027.1:
rmcp2.1.0 (the official MCP Rust SDK —modelcontextprotocol/rust-sdk), server-side + streamable-HTTP features only, inourios-serveralone (§3.1: no new crate).cargo denyclean (the run also surfaced pre-existing RUSTSEC-2026-0204 oncrossbeam-epoch; patch-bumped here).src/mcp.rs: the/mcpsub-router —StreamableHttpServiceover aLocalSessionManager, wrapped in the RFC 0026 bearer gate as an axum layer (one undifferentiated 401 before any MCP dispatch; open mode passes through — the JSON API's exact contract). The handler announces the server with the §3.3 treat-log-bodies-as-data instruction; the §3.2 tools and the grammar resource attach in the next slices.querier.mcp.enabledin the RFC 0020 file schema (with${env:…}substitution) andOURIOS_QUERIER_MCP_ENABLEDon the env path, default off;router/router_with_authare unchanged (= off),router_with_mcpis the optioned constructor tests drive.Scenario mapping
rfc0027_1_gating_and_placement(stub → green): flag off ⇒/mcp404 with the existing constructors untouched (the RFC 0016/0026 suites in this harness are the JSON-API-unchanged assertion); flag on ⇒ the MCPinitializehandshake answers withserverInfoon the same router; auth enabled ⇒ no bearer 401 before dispatch, valid bearer served. The.2tenant-denial arm needs a tool to probe and lands with the tools slice (its stub notes the transport label — the authn arm is covered here).Test note: rmcp validates the
Hostheader (DNS-rebinding protection); the in-process requests carry one explicitly, as any real client does.Invariants / hazards
authenticate_bearerthe JSON API and OTLP listeners use — one bearer implementation across every surface.Checks run locally
cargo fmt --all --check,cargo clippy -p ourios-server --all-targets --all-features -- -D warnings,cargo test -p ourios-server --all-features,cargo deny check— all green.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
/mcpwhen enabled.Bug Fixes
404instead of exposing the endpoint.