feat(mcp): surface a server's own instructions when it has no description - #5321
feat(mcp): surface a server's own instructions when it has no description#5321yh928 wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughMCP connections now retain initialization instructions from stdio and HTTP servers. Connected-server overviews expose them. Orchestrator prompts use sanitized, length-bounded instructions when registry descriptions are unavailable, while preserving description and tool-count fallback precedence. ChangesMCP instruction flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant MCPServer
participant Connection
participant ConnectedServerOverview
participant OrchestratorPrompt
MCPServer->>Connection: Return initialize instructions
Connection->>ConnectedServerOverview: Store and expose instructions
ConnectedServerOverview->>OrchestratorPrompt: Provide server metadata
OrchestratorPrompt->>OrchestratorPrompt: Sanitize, bound, and apply fallback precedence
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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
added
agent
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/agent_registry/agents/orchestrator/prompt.rs`:
- Around line 669-686: Extend
connected_mcp_block_sanitizes_untrusted_instructions to use instructions
containing newline and tab characters plus content exceeding the 600-byte limit.
Assert the rendered server item is capped at the defined 600-byte boundary and
contains no newline or tab characters, while preserving the existing
instruction-fence sanitization and server-name assertions.
In `@src/openhuman/mcp_registry/connections.rs`:
- Around line 174-178: Add stdio and HTTP connection tests covering the
initialization-to-overview propagation: have each initialize branch return
distinct instructions, then assert the resulting connected_overview retains the
corresponding value from Connection.instructions. Update the existing connection
test coverage near the initialize and overview construction paths without
changing production behavior.
In `@src/openhuman/mcp_registry/types.rs`:
- Around line 75-78: Add focused tests for the Transport::is_http_remote
discriminator, asserting Transport::Stdio returns false and
Transport::HttpRemote returns true. Place the coverage alongside the existing
Transport tests and construct each variant with the required fields.
🪄 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 Plus
Run ID: 64b7d89f-789d-449f-bb72-a85d2f13c6f5
📒 Files selected for processing (3)
src/openhuman/agent_registry/agents/orchestrator/prompt.rssrc/openhuman/mcp_registry/connections.rssrc/openhuman/mcp_registry/types.rs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a80330cee1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
| Filename | Overview |
|---|---|
| src/openhuman/agent/registry/agents/orchestrator/prompt.rs | Adds instruction fallback and three tests; the instructions sanitization call references a non-existent module path (mcp_client::sanitize vs util::sanitize), causing a compile failure. |
| src/openhuman/mcp/registry/connections.rs | Correctly captures init.instructions from both Stdio and HttpRemote branches and threads it through to Connection and connected_overview. |
| src/openhuman/mcp/registry/types.rs | Adds instructions: Option<String> field to ConnectedServerOverview with clear documentation; straightforward and correct. |
Reviews (3): Last reviewed commit: "feat(mcp): surface a server's own instru..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
a80330c to
2073e7f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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_registry/types.rs`:
- Around line 195-208: Add coverage in the mcp_registry test module for
format_connected_mcp_block() and serializable ConnectedServerOverview data:
verify a non-empty description takes precedence over instructions, instruction
text is sanitized before rendering, and the rendered instruction fallback is
capped at 600 bytes.
🪄 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 Plus
Run ID: ff9b3c43-077e-4751-8fb5-184232aec0ee
📒 Files selected for processing (3)
src/openhuman/agent_registry/agents/orchestrator/prompt.rssrc/openhuman/mcp_registry/connections.rssrc/openhuman/mcp_registry/types.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/openhuman/agent_registry/agents/orchestrator/prompt.rs
- src/openhuman/mcp_registry/connections.rs
2073e7f to
3c96df9
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
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/agent/registry/agents/orchestrator/prompt.rs`:
- Around line 207-232: Split the oversized files by extracting the connected-MCP
block formatting around prompt.rs lines 207-232 into a focused module, and move
its formatter tests from prompt.rs lines 613-686 alongside it; update prompt.rs
to call the extracted formatter while preserving current description,
instructions sanitization, and fallback behavior. Extract the connection
initialization workflow from connections.rs lines 404-481 into a focused MCP
registry module, updating callers and imports without changing its behavior.
Ensure the resulting files remain at or below 500 lines.
In `@src/openhuman/mcp/registry/connections.rs`:
- Around line 417-418: Update the initialize call in the active client setup
flow around stdio.initialize() and the corresponding additional transport path
to emit [rpc] debug events immediately before and after each RPC. Include
server_id and transport on both events, and include instructions_present on the
completion event without logging instruction contents; preserve the existing
initialization result and error propagation.
🪄 Autofix
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 Plus
Run ID: a970da01-e469-4e8c-badc-9331f2b0fe6e
📒 Files selected for processing (3)
src/openhuman/agent/registry/agents/orchestrator/prompt.rssrc/openhuman/mcp/registry/connections.rssrc/openhuman/mcp/registry/types.rs
…tion An MCP server returns `instructions` in its `initialize` response — the server's own statement of what it is for and how to drive it. We asked for it, threw it away, and told the agent nothing. That was survivable while every connected server came from the registry inventory, which carries a curated description. Hand-entered custom servers have no such entry, so the orchestrator prompt listed them by name and tool count alone. `Connection` now keeps the `instructions` from `initialize`, and `ConnectedServerOverview` carries them through. The prompt block falls back to them only when the registry has no description — an existing description still wins, so nothing that reads well today changes — and the text is untrusted input from a third-party server, so it goes through `sanitize_for_llm` with a 600-character cap and flattened newlines before it can reach the prompt. Three tests cover the ladder: instructions used when there is no description, description preferred when there is one, and untrusted instructions sanitized. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SRSNnqQsokuGmkbpLoLCGy
3c96df9 to
7e4201c
Compare
There was a problem hiding this comment.
yh928 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
…boundaries Adds the one uncovered property of the new fallback: instructions are remote free-form text with no length contract, so a verbose or hostile server must not be able to spend the orchestrator's prompt budget. The test asserts the rendered server line stays near the 600-byte bound for input several times that size. Description precedence, the no-description fallback, and instruction sanitization were already pinned. Adds the `[rpc]` boundary events around both `initialize` calls with `server_id`, transport, and `instructions_present`. The instruction content stays out of the log — it is untrusted remote text, and the block already scrubs it before the prompt sees it. orchestrator::prompt connected_mcp 8 pass. Reported by CodeRabbit on tinyhumansai#5321. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SRSNnqQsokuGmkbpLoLCGy
There was a problem hiding this comment.
yh928 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
1 participant
Summary
Connectionnow keeps theinstructionsan MCP server returns frominitialize, andConnectedServerOverviewcarries them through to the prompt.Problem
An MCP server's
initializeresponse carriesinstructions— the server's own statement of what it is for and how to drive it. We asked for it, discarded it, and told the agent nothing.That was survivable while every connected server came from the registry inventory, which carries a curated
description. Hand-entered custom servers (#5033) have no inventory entry, so the orchestrator prompt listed them by name and tool count alone and the agent had to infer the server's purpose from tool names.Solution
A three-step ladder in
format_connected_mcp_block: registry description → server instructions → tool count. The description keeps priority deliberately — it is curated, the instructions are whatever the server author wrote.Instructions pass through
sanitize_for_llmwith a 600-character cap and flattened newlines. This is a third party's text landing in our system prompt, so it is treated as untrusted the same way skill descriptions are.Submission Checklist
Closes #NNNin the## RelatedsectionImpact
Related
Closes #5317
Summary by CodeRabbit
New Features
Bug Fixes