feat(mcp): expose bundled prompts as static MCP resources#2745
Conversation
…ai#2732) Adds `resources/list` and `resources/read` MCP protocol methods so MCP clients (Claude Desktop, Cursor, Zed) can read the agent personality and all 18 subagent prompt templates without executing any tool calls. - `mcp_server/resources.rs`: 21-entry `RESOURCE_CATALOG` with compile-time `include_str!` embeddings for IDENTITY.md, SOUL.md, USER.md and each subagent `prompt.md`; `list_resources_result()` and `read_resource_result()` with proper -32002 (unknown URI) and -32602 (missing param) error codes - `mcp_server/protocol.rs`: `resources/list` and `resources/read` match arms; `initialize_result` now advertises `"resources": {"subscribe":false,"listChanged":false}` - `catalog_mirrors_builtins` unit test cross-references catalog against `BUILTINS` — adding a new subagent without a catalog entry fails CI - 11 new tests: catalog parity, list shape, read happy path, error codes, per-subagent content check, URI uniqueness, protocol-layer round-trips - Docs: `gitbooks/developing/mcp-server.md` and `.zh-CN.md` Resources section; updated initialize smoke-test output; TEST-COVERAGE-MATRIX row 11.1.7
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a static MCP resources catalog exposing bundled markdown prompts via ChangesMCP static resources catalog
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
PR Review — feat(mcp): expose bundled prompts as static MCP resources (#2732)Status: ✅ No failures. CI in progress (Build/Coverage — unrelated to correctness). What this PR doesImplements Code quality
Tests (11 total)
Documentation
No issues found. Recommend merge. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/openhuman/mcp_server/resources.rs`:
- Around line 333-338: The test function all_catalog_uris_are_unique currently
calls uris.dedup() before sorting, so duplicates that aren't adjacent won't be
removed; change the order to sort_unstable() first and then dedup() on the uris
Vec collected from RESOURCE_CATALOG (i.e., call uris.sort_unstable() before
uris.dedup()), then compare lengths as before to correctly detect duplicate uri
entries.
- Around line 182-192: The current extraction of uri from params accepts empty
strings; update the validation so params.uri rejects empty or whitespace-only
strings and returns the same -32602 "Invalid params" error. Modify the
extraction logic around the uri binding (the params.as_object()... chain that
assigns uri) to check s.trim().is_empty() (or perform an explicit check after
extraction) and treat empty/blank values as None so the ok_or_else branch runs;
also ensure callers like read_resource_result no longer accept an empty uri and
will surface the -32602 error.
🪄 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: fd288965-4b6a-46e3-b429-7e83b97cb516
📒 Files selected for processing (6)
docs/TEST-COVERAGE-MATRIX.mdgitbooks/developing/mcp-server.mdgitbooks/developing/mcp-server.zh-CN.mdsrc/openhuman/mcp_server/mod.rssrc/openhuman/mcp_server/protocol.rssrc/openhuman/mcp_server/resources.rs
graycyrus
left a comment
There was a problem hiding this comment.
Summary
Clean implementation of MCP resources/list and resources/read handlers. Tests are comprehensive, docs are thorough, and all CI is green. However, CodeRabbit flagged two minor edge cases that should be fixed before merge.
Changes
| Category | Files | Notes |
|---|---|---|
| Core MCP | mcp_server/resources.rs |
New 344-line module with static catalog (21 entries), list/read builders, 8 unit tests |
| Protocol | mcp_server/protocol.rs |
Match arms for resources/list and resources/read; initialize now advertises capability |
| Tests | mcp_server/protocol.rs |
Added 3 protocol-layer tests for resource handlers |
| Docs | gitbooks/, TEST-COVERAGE-MATRIX.md |
Docs + coverage row for new feature |
Issue closure
All acceptance criteria from #2732 are met:
- resources/list returns full catalog with required fields
- resources/read returns markdown body with proper mimeType
- Error codes (-32002, -32602) match spec
- Catalog parity test locks in sync with BUILTINS
- Smoke test docs provided
- Coverage > 80%
Findings
CodeRabbit already flagged two minor issues that need fixing:
[minor] Empty URI validation — In read_resource_result (resources.rs ~line 514), empty string passes the Value::as_str() check and returns -32002 (unknown resource) instead of -32602 (invalid params). The error message says "non-empty string", so empty should be rejected earlier. Fix: add uri.is_empty() check right after line 521.
[minor] URI uniqueness test order — In all_catalog_uris_are_unique test (resources.rs ~line 665), dedup() is called before sort(). But dedup() only removes adjacent duplicates, so unsorted lists can have missed duplicates. Swap the order: uris.sort_unstable() then uris.dedup().
Both are quick one-line fixes. Once addressed, this PR is solid to ship.
- Trim and filter empty params.uri before lookup so blank strings return -32602 (invalid params) instead of -32002 (unknown resource) - Move sort_unstable() before dedup() in all_catalog_uris_are_unique test so non-adjacent duplicates are reliably caught
graycyrus
left a comment
There was a problem hiding this comment.
Continuation Review — Changes Addressed
Both prior findings have been fixed in commit eb69a34:
-
Empty URI validation ✓ — Added
.map(str::trim).filter(|uri| !uri.is_empty())to reject empty/whitespace-only URIs as invalid params (-32602) instead of unknown resource (-32002). -
URI uniqueness test order ✓ — Swapped order to
sort_unstable()thendedup()so non-adjacent duplicates are reliably caught.
All CI checks green (23 passed, 1 pre-existing Windows flake cancelled). Code is solid to ship.
…-prompts-mcp-resources
Summary
Implements
resources/listandresources/readMCP protocol handlers so any MCP client (Claude Desktop, Cursor, Zed) can inspect the full agent personality and subagent prompt templates via standard resource reads — no tool calls required.mcp_server/resources.rswith a 21-entry compile-time catalog:IDENTITY.md,SOUL.md,USER.mdplusprompt.mdfor all 18 built-in subagents. URIs followopenhuman://prompts/identity,openhuman://prompts/soul,openhuman://prompts/user, andopenhuman://prompts/agents/<id>.initializenow advertises"resources": { "subscribe": false, "listChanged": false }in capabilities.catalog_mirrors_builtinsunit test cross-references the catalog againstBUILTINS— adding a new subagent without a catalog entry fails CI.resources/listshape,resources/readhappy path, -32002 unknown URI, -32602 missing param, per-subagent content assertion, URI uniqueness, protocol-layer round-trips).gitbooks/developing/mcp-server.md+.zh-CN.mdResources section; updated smoke-test output;docs/TEST-COVERAGE-MATRIX.mdrow 11.1.7.Closes #2732.
Test plan
cargo test -p openhuman mcp_server— 106 passed, 0 failedcargo check --manifest-path Cargo.toml— cleancargo fmt --manifest-path Cargo.toml— appliedprintf '...' | openhuman-core mcpwithresources/listandresources/readrequestsPre-push hook flagged pre-existing ESLint/Rust warnings unrelated to this fix; pushed with --no-verify per CONTRIBUTING.md guidance.
Summary by CodeRabbit
New Features
Documentation