feat(tools): MCP sampling Phase 2 — spec coverage, governance & audit - #366
feat(tools): MCP sampling Phase 2 — spec coverage, governance & audit#366eren-karakus0 wants to merge 1 commit into
Conversation
…d audit Extend MCP sampling with full spec compliance, per-server governance controls, and structured audit observability. Spec coverage: - Fix _map_stop_reason: tool_calls now correctly maps to "toolUse" - Add tools/toolChoice forwarding to LLM (OpenAI function-calling format) - Handle tool_use and tool_result content blocks in message conversion - Return tool_use content blocks in CreateMessageResult for tool responses - Graceful fallback for malformed JSON in tool_calls arguments - Handle mixed content lists (text + tool_result) without silent data loss Governance: - allowed_models: per-server model whitelist (empty = allow all) - max_tool_rounds: cap consecutive tool use rounds (default: 5, 0 = disable) - Both return structured ErrorData on violation Audit: - _sampling_metrics: per-server request/error/token/tool_use counters - Metrics exposed via get_mcp_status() for observability - log_level config: control audit verbosity (debug/info/warning) All new config fields have defaults — zero breaking changes for existing configs. Tests: 31 new tests (151 total passed, 3 pre-existing fail from missing mcp pkg)
Add MCP sampling/createMessage capability allowing MCP servers to request
LLM completions through the Hermes agent during tool execution. Enables
agent-in-the-loop workflows (data analysis, content generation, decision
making) where servers can leverage the LLM as needed.
Implementation as SamplingHandler class (per-server instance, no globals):
- Text-only sampling: server asks LLM a question, gets text back
- Tool use in sampling: server provides tools, LLM can use them in a
multi-turn loop with configurable max_tool_rounds governance
- Rate limiting (sliding window, configurable max_rpm per server)
- Model resolution (config override > server hint > default)
- Model whitelist (allowed_models per server)
- Token cap (max_tokens_cap per server)
- LLM timeout with asyncio.wait_for
- Credential stripping on responses
- Per-server audit metrics (requests, errors, tokens_used, tool_use_count)
- Configurable log_level for audit verbosity
- Non-blocking: LLM calls offloaded via asyncio.to_thread()
- Proper MCP SDK types: CreateMessageResult for text responses,
CreateMessageResultWithTools + ToolUseContent for tool use responses
- SamplingCapability with SamplingToolsCapability advertised to servers
- Backward compatible: silently disabled if MCP SDK lacks sampling types
Config (all optional, zero breaking changes):
mcp_servers:
my_server:
sampling:
enabled: true # default
model: 'gemini-3-flash'
max_tokens_cap: 4096
timeout: 30
max_rpm: 10
allowed_models: []
max_tool_rounds: 5
log_level: 'info'
Based on the sampling concept from PR #366 by eren-karakus0. Restructured
as a class-based design, fixed critical bugs (wrong return types for tool
use, missing capability advertisement, broken Pydantic validation), and
added tests using real MCP SDK types.
50 new tests, full suite passes (2600 tests).
|
Thanks for this work! The sampling concept and the thorough documentation were valuable. We've incorporated the feature in PR #753, restructured as a class-based Closing this PR since the branch was 388 commits behind main and couldn't be merged directly, but the feature is landing. Thanks again for the contribution! |
Add MCP sampling/createMessage capability via SamplingHandler class. Text-only sampling + tool use in sampling with governance (rate limits, model whitelist, token caps, tool loop limits). Per-server audit metrics. Based on concept from PR #366 by eren-karakus0. Restructured as class-based design with bug fixes and tests using real MCP SDK types. 50 new tests, 2600 total passing.
|
Glad to see MCP sampling landed in #753! Looks like it covers similar ground to what I proposed here. Happy to have contributed to the direction. |
ManagedServer in this branch passes tools= to apply_chat_template(), enabling proper tool calling for Phase 2 (RL training with logprobs).
SafariShow
left a comment
There was a problem hiding this comment.
Hermes Agent Review
Found 1 critical issue, 1 warning, and a few minor suggestions.
🔴 Critical
apps/dashboard/server/api/ai/personalize-template.post.tsandapps/dashboard/server/api/ai/template-events.post.ts: Missing authentication checks. The comment says "Auth: requires the same Bearer/cookie token", but neither endpoint actually verifies the user. An attacker can hitpersonalize-templateto drain Anthropic API tokens via Claude generation, and hittemplate-eventsto spam thebuilder_eventstable with junk. You need to callgetTemplateAccessContextor similar auth middleware before processing the body.
⚠️ Warnings
apps/dashboard/server/utils/templateEvents.ts(Lines 76-88):emitTemplateEventtriggers adb.insert()without returning a Promise, and it is not passed toevent.waitUntil(). In serverless environments (like Vercel), if the HTTP response is returned before the background DB insert completes, the process will be suspended and the event will be dropped. Either pass theH3Eventto useevent.waitUntil(), or simplyawaitit with a.catch()in the endpoint.- Request Body Validation:
readBody(event) as RequestBodyis used without Zod parsing. Ifbody.productsis passed as a string or object instead of an array, the application will crash at runtime when calling.map()or.length.
💡 Suggestions
apps/dashboard/server/ai/template-personalizers/listicle.ts: The file header comment says "Per-item copy rewrite is intentionally deferred to Phase 2", but the code actually does implementrewriteListicleItemsWithLLM! The docstring is stale and should be updated to match the Phase 3 implementation.apps/dashboard/scripts/seed-global-templates.ts: Contains severalconsole.logstatements for progress reporting. Perfectly fine for a CLI script, but noting it here in case you intended to use a structured logger.
✅ Looks Good
- The architectural split between the generic substitution (Floor) and the LLM rewrite (Ceiling) is very clean and provides a great fallback mechanism.
- Great job adding the
Authorization: Bearerfallback inresolveAuthTokento support SDK/MCP callers! - Zod schemas in
llm-copy.tsfor structured outputs are well-described and strict.
…Research#753) Add MCP sampling/createMessage capability via SamplingHandler class. Text-only sampling + tool use in sampling with governance (rate limits, model whitelist, token caps, tool loop limits). Per-server audit metrics. Based on concept from PR NousResearch#366 by eren-karakus0. Restructured as class-based design with bug fixes and tests using real MCP SDK types. 50 new tests, 2600 total passing.
ManagedServer in this branch passes tools= to apply_chat_template(), enabling proper tool calling for Phase 2 (RL training with logprobs).
Add MCP sampling/createMessage capability allowing MCP servers to request
LLM completions through the Hermes agent during tool execution. Enables
agent-in-the-loop workflows (data analysis, content generation, decision
making) where servers can leverage the LLM as needed.
Implementation as SamplingHandler class (per-server instance, no globals):
- Text-only sampling: server asks LLM a question, gets text back
- Tool use in sampling: server provides tools, LLM can use them in a
multi-turn loop with configurable max_tool_rounds governance
- Rate limiting (sliding window, configurable max_rpm per server)
- Model resolution (config override > server hint > default)
- Model whitelist (allowed_models per server)
- Token cap (max_tokens_cap per server)
- LLM timeout with asyncio.wait_for
- Credential stripping on responses
- Per-server audit metrics (requests, errors, tokens_used, tool_use_count)
- Configurable log_level for audit verbosity
- Non-blocking: LLM calls offloaded via asyncio.to_thread()
- Proper MCP SDK types: CreateMessageResult for text responses,
CreateMessageResultWithTools + ToolUseContent for tool use responses
- SamplingCapability with SamplingToolsCapability advertised to servers
- Backward compatible: silently disabled if MCP SDK lacks sampling types
Config (all optional, zero breaking changes):
mcp_servers:
my_server:
sampling:
enabled: true # default
model: 'gemini-3-flash'
max_tokens_cap: 4096
timeout: 30
max_rpm: 10
allowed_models: []
max_tool_rounds: 5
log_level: 'info'
Based on the sampling concept from PR NousResearch#366 by eren-karakus0. Restructured
as a class-based design, fixed critical bugs (wrong return types for tool
use, missing capability advertisement, broken Pydantic validation), and
added tests using real MCP SDK types.
50 new tests, full suite passes (2600 tests).
…Research#753) Add MCP sampling/createMessage capability via SamplingHandler class. Text-only sampling + tool use in sampling with governance (rate limits, model whitelist, token caps, tool loop limits). Per-server audit metrics. Based on concept from PR NousResearch#366 by eren-karakus0. Restructured as class-based design with bug fixes and tests using real MCP SDK types. 50 new tests, 2600 total passing.
ManagedServer in this branch passes tools= to apply_chat_template(), enabling proper tool calling for Phase 2 (RL training with logprobs).
…Research#753) Add MCP sampling/createMessage capability via SamplingHandler class. Text-only sampling + tool use in sampling with governance (rate limits, model whitelist, token caps, tool loop limits). Per-server audit metrics. Based on concept from PR NousResearch#366 by eren-karakus0. Restructured as class-based design with bug fixes and tests using real MCP SDK types. 50 new tests, 2600 total passing.
Add MCP sampling/createMessage capability allowing MCP servers to request
LLM completions through the Hermes agent during tool execution. Enables
agent-in-the-loop workflows (data analysis, content generation, decision
making) where servers can leverage the LLM as needed.
Implementation as SamplingHandler class (per-server instance, no globals):
- Text-only sampling: server asks LLM a question, gets text back
- Tool use in sampling: server provides tools, LLM can use them in a
multi-turn loop with configurable max_tool_rounds governance
- Rate limiting (sliding window, configurable max_rpm per server)
- Model resolution (config override > server hint > default)
- Model whitelist (allowed_models per server)
- Token cap (max_tokens_cap per server)
- LLM timeout with asyncio.wait_for
- Credential stripping on responses
- Per-server audit metrics (requests, errors, tokens_used, tool_use_count)
- Configurable log_level for audit verbosity
- Non-blocking: LLM calls offloaded via asyncio.to_thread()
- Proper MCP SDK types: CreateMessageResult for text responses,
CreateMessageResultWithTools + ToolUseContent for tool use responses
- SamplingCapability with SamplingToolsCapability advertised to servers
- Backward compatible: silently disabled if MCP SDK lacks sampling types
Config (all optional, zero breaking changes):
mcp_servers:
my_server:
sampling:
enabled: true # default
model: 'gemini-3-flash'
max_tokens_cap: 4096
timeout: 30
max_rpm: 10
allowed_models: []
max_tool_rounds: 5
log_level: 'info'
Based on the sampling concept from PR NousResearch#366 by eren-karakus0. Restructured
as a class-based design, fixed critical bugs (wrong return types for tool
use, missing capability advertisement, broken Pydantic validation), and
added tests using real MCP SDK types.
50 new tests, full suite passes (2600 tests).
Summary
Extends the MCP
sampling/createMessageimplementation with full MCP spec compliance, per-server governance controls, and structured audit observability. Built on top of Phase 1 (PR #64).Spec Coverage
_map_stop_reasonnow correctly mapstool_calls→"toolUse"(was incorrectly"endTurn")toolChoicemode mapping (auto/required/none)tool_useandtool_resultcontent blocks in_convert_sampling_messages— converts to OpenAItool_callsandrole: "tool"messages respectivelytool_calls, returnsCreateMessageResultwithstopReason: "toolUse"and tool use content blocksjson.loads()on tool call arguments is now wrapped in try/except — malformed JSON falls back to raw string instead of crashing the callbacktool_resultblocks are now handled correctly (previously tool_results were silently dropped as "unsupported")Governance
allowed_models: Per-server model whitelist. Empty list (default) permits any model. Non-listed models return structuredErrorDatamax_tool_rounds: Caps consecutive tool use rounds per server (default: 5). Prevents infinite tool loops between LLM and server. Set to 0 to disable tool loops entirely. Counter resets on normal text responseAudit Observability
_sampling_metrics: Per-server counters trackingrequests,errors,tokens_used, andtool_use_countget_mcp_status(): Metrics exposed in server status for monitoring/debugginglog_level: Config-driven audit verbosity ("debug","info","warning")Config Schema (all backward-compatible)
All new fields have defaults — zero breaking changes for existing configurations.
Files Changed
tools/mcp_tool.pytests/tools/test_mcp_tool.pydocs/mcp.mdskills/mcp/native-mcp/SKILL.mdTest Plan
pytest tests/tools/test_mcp_tool.py -q→ 151 passed, 3 pre-existing fail (mcp pkg not installed)pytest tests/tools/test_mcp_tool.py -k Sampling -W error::RuntimeWarning→ 0 warningsRelated
sampling/createMessage