Conversation
📝 WalkthroughWalkthroughAdds an MCP server mode: a new Changes
Sequence DiagramssequenceDiagram
participant Client as MCP Client
participant Stdio as Stdio Transport
participant Handler as Request Handler
participant Kernel as ZeptoKernel
Client->>Stdio: send line-delimited JSON-RPC
activate Stdio
Stdio->>Stdio: parse & validate jsonrpc/id/method
alt invalid
Stdio-->>Client: JSON-RPC error response
else valid
Stdio->>Handler: handle_request(method, params, id)
activate Handler
alt initialize
Handler-->>Stdio: initialization response
else tools/list
Handler->>Kernel: get_tools()
activate Kernel
Kernel-->>Handler: tool list
deactivate Kernel
Handler-->>Stdio: tools/list response
else tools/call
Handler->>Kernel: execute_tool(name, args)
activate Kernel
Kernel-->>Handler: execution result
deactivate Kernel
Handler-->>Stdio: tools/call response
end
deactivate Handler
Stdio-->>Client: JSON-RPC response + newline
end
deactivate Stdio
sequenceDiagram
participant Client as HTTP Client
participant Axum as HTTP Endpoint
participant Channel as Tokio Channel
participant Processor as Background Processor
participant Handler as Request Handler
participant Kernel as ZeptoKernel
Client->>Axum: POST JSON-RPC
activate Axum
Axum->>Axum: validate request
alt invalid
Axum-->>Client: error response
else valid
Axum->>Channel: enqueue request
Axum->>Processor: await oneshot response
activate Processor
Processor->>Handler: handle_request(...)
activate Handler
Handler->>Kernel: execute/list/etc
activate Kernel
Kernel-->>Handler: result
deactivate Kernel
Handler-->>Processor: McpResponse
deactivate Handler
Processor->>Axum: return response via channel
deactivate Processor
Axum-->>Client: JSON-RPC response
end
deactivate Axum
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli/mod.rs`:
- Around line 716-717: Config::load() performs blocking filesystem I/O but is
called directly inside the async cmd_mcp_server function, which can block Tokio
executor threads; fix this by calling Config::load inside
tokio::task::spawn_blocking (or tokio::spawn_blocking) and await the JoinHandle,
mapping any error into the same anyhow::Error shape as before so the resulting
let config = ... line remains equivalent; locate the call site in cmd_mcp_server
and replace the direct Config::load() invocation with a spawn_blocking wrapper
around Config::load (preserving the map_err(...) behavior).
In `@src/mcp_server/handler.rs`:
- Around line 29-34: handle_request currently takes id: Option<u64>, which drops
non-numeric JSON-RPC IDs; change the signature to accept and propagate the raw
JSON id (e.g., id: Option<serde_json::Value> or Option<Value>) so string,
number, or null IDs are preserved and echoed in McpResponse; update any internal
uses that rely on numeric-only behavior (remove .as_u64() conversions and adjust
logic in McpResponse construction and any call sites) so the response's id field
mirrors the incoming Value exactly.
In `@src/mcp_server/mod.rs`:
- Around line 64-69: The handler currently defaults missing/non-string "method"
to an empty string which yields a -32601 later; instead mirror the stdio
transport by validating "method" immediately: change the extraction of method
(the body.get("method") / .and_then(|v| v.as_str()) logic) to return an Invalid
Request (-32600) response when "method" is absent or not a string (do not
default to ""), i.e. check method presence and type up-front using the same
error construction used for jsonrpc validation and short-circuit the handler on
failure so the HTTP transport matches stdio behavior.
In `@src/mcp_server/stdio.rs`:
- Line 31: The debug call is logging the full MCP stdin payload via debug!(line
= %line, "MCP stdin: received"); change it to log only safe metadata: compute
the payload size (e.g., line.len()) and, if needed, extract only safe fields
like "method" and "id" from the parsed JSON (using serde_json to read Value and
.get("method")/.get("id") without logging the rest), then call debug! with those
fields (e.g., payload_size, method, id) instead of the raw line; ensure no full
payload or nested argument content is ever included in the log and update the
code around the debug! invocation in stdio.rs accordingly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/cli/mod.rssrc/lib.rssrc/mcp_server/handler.rssrc/mcp_server/mod.rssrc/mcp_server/stdio.rs
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/mcp_server/stdio.rs (1)
45-48: Preferif letoveris_none()+unwrap()pattern.While safe here, the current pattern could be more idiomatic. This also aligns with the coding guideline to avoid
unwrap()in production paths.♻️ Suggested refactor
- // JSON-RPC 2.0 spec: notifications (no id) MUST NOT receive a - // response. Detect via the method name extracted from the raw line. - if resp.is_none() { - continue; - } - let resp = resp.unwrap(); + // JSON-RPC 2.0 spec: notifications (no id) MUST NOT receive a + // response. Detect via the method name extracted from the raw line. + let Some(resp) = resp else { + continue; + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mcp_server/stdio.rs` around lines 45 - 48, Replace the is_none() + unwrap() pattern around the resp variable with an idiomatic if let binding: instead of checking if resp.is_none() then calling resp.unwrap(), use if let Some(resp) = resp { /* existing body that used resp */ } else { continue; } so the code in src/mcp_server/stdio.rs uses pattern matching and avoids unwrap().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/mcp_server/stdio.rs`:
- Around line 45-48: Replace the is_none() + unwrap() pattern around the resp
variable with an idiomatic if let binding: instead of checking if resp.is_none()
then calling resp.unwrap(), use if let Some(resp) = resp { /* existing body that
used resp */ } else { continue; } so the code in src/mcp_server/stdio.rs uses
pattern matching and avoids unwrap().
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/mcp_server/handler.rssrc/mcp_server/mod.rssrc/mcp_server/stdio.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/mcp_server/mod.rs
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Change McpResponse.id from Option<u64> to Option<serde_json::Value> so the server preserves whatever id type the client sends (number, string, or null) as required by JSON-RPC 2.0. Also: - Safe stdio logging (metadata only, no payloads) - Proper Invalid Request when method is missing in HTTP transport - spawn_blocking for Config::load() in CLI handler - New tests for string id preservation and log metadata Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…plies Issue 5: HTTP transport now accepts raw String body instead of Axum's Json extractor, manually parses JSON, and returns a proper JSON-RPC -32700 parse error response for malformed input. Previously, Axum would reject bad JSON with its own HTTP 422 before the handler ran. Issue 6: Notifications (methods starting with "notifications/") no longer produce a response per JSON-RPC 2.0 spec. The stdio transport skips writing to stdout, and the HTTP transport returns 204 No Content. - Add is_notification() helper in handler.rs - Change process_line() to return Option<McpResponse> - Update all existing tests for Option return type - Add tests: notification suppression, malformed JSON parse error, is_notification true/false cases Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
zeptoclaw mcp-servercommand exposing all registered tools via JSON-RPC 2.0--http :3000POST endpoint (requirespanelfeature for axum dependency)src/tools/mcp/protocol.rsCloses #217
Test plan
cargo test --libpasses (2848 tests, including 32 new MCP server tests)cargo clippy -- -D warningspassescargo fmt -- --checkpassesecho '{"jsonrpc":"2.0","id":1,"method":"initialize"}' | cargo run -- mcp-serverreturns server info--http :3000is passed (requirespanelfeature)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation