feat(cli/mcp): 'zeus mcp' subcommand scaffold#24
Conversation
Registers 'code mcp' (Zeus's 'zeus mcp') as a clap subcommand that
takes --transport (stdio|sse), --port, and --workspace flags. The
handler is a stub that prints which transport was selected; real
implementation lands in a follow-up PR.
'cargo check' and 'cargo build --bin code' pass locally. Smoke tested:
$ ./target/debug/code mcp --help
$ ./target/debug/code mcp
zeus mcp: stdio transport not yet implemented (workspace=...)
Design doc: docs/zeus-mcp-server.md
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR scaffolds a new ChangesMCP Command Scaffolding
🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a scaffold for the Model Context Protocol (MCP) server, adding a new mcp subcommand to the CLI with support for stdio and sse transports. The changes include argument parsing, a stub implementation for the command, and detailed documentation. Review feedback identifies a type mismatch in the main command dispatcher that would prevent compilation, recommends more robust error handling when resolving the current working directory, and suggests correcting documentation to ensure diagnostic output is consistently directed to stderr.
- cli/src/commands/mcp.rs: replace expect() on current_dir() with ? via wrap(). Prevents a panic if cwd is inaccessible. - docs/zeus-mcp-server.md: clarify that the chosen SSE port is printed on stderr (was 'stdout'). Stdout is reserved for protocol traffic on stdio transport; stderr is consistent across transports. cargo check passes.
|
Re. main.rs:107 type mismatch — checked locally and this is a false positive. All match arms in the
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request scaffolds the Model Context Protocol (MCP) server integration for the Zeus CLI, introducing the mcp subcommand and its associated configuration arguments. The feedback focuses on maintaining architectural consistency by passing CommandContext to the new command handler and ensuring the CLI implementation matches the design documentation. Specifically, suggestions were made to include missing network binding arguments, canonicalize workspace paths for security, and correct a contradiction in the documentation regarding whether authentication tokens are printed to stdout or stderr.
… canonicalize workspace - mcp() now takes CommandContext like every other command handler. Real implementation will need it for logging and workbench discovery; passing it from the dispatch site now keeps the signature stable. - McpArgs gains '--bind' (defaults to 127.0.0.1) and '--allow-non-loopback' (required when --bind is non-loopback). The docs already promised these — adding them now to keep the CLI surface in lockstep with the design. - Workspace path is canonicalized at entry. The 'refuse paths outside the workspace' security check can then use byte-prefix comparison instead of re-resolving relative segments per request. - docs/zeus-mcp-server.md: fix the stdout/stderr contradiction in the auth section. Bearer token is on stderr (stdout is reserved for protocol traffic on stdio transport).
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a scaffold for the Model Context Protocol (MCP) server as a new mcp subcommand in the CLI, including argument parsing for transport types and workspace paths. It also adds documentation detailing the server's design and tool surface. Feedback recommends deriving equality traits for the transport enum and implementing validation logic for the workspace directory and security flags.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a scaffold for the Model Context Protocol (MCP) server in the Zeus CLI, adding a new mcp subcommand with support for stdio and sse transports. The implementation includes workspace canonicalization and basic security checks for network binding. Feedback from the review suggests improving the CLI argument handling by using std::net::IpAddr for the bind parameter to leverage clap's built-in validation, which would also eliminate the need for manual parsing in the command implementation. Additionally, it was noted that the help text should be updated to clarify that only IP literals are supported, as localhost is not compatible with the current parsing logic.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a scaffold for the Model Context Protocol (MCP) server within the Zeus CLI, adding the mcp subcommand and its associated argument parsing. The implementation includes workspace path canonicalization and security validations for network binding. Feedback focuses on improving CLI error handling by returning an error code for unimplemented transports and ensuring the documentation synopsis accurately reflects all available command-line flags.
| match args.transport { | ||
| McpTransport::Stdio => { | ||
| eprintln!( | ||
| "zeus mcp: stdio transport not yet implemented (workspace={})", | ||
| workspace.display() | ||
| ); | ||
| } | ||
| McpTransport::Sse => { | ||
| // Refuse to bind to a non-loopback interface without explicit opt-in, | ||
| // to keep the default posture local-only. clap already parsed | ||
| // `--bind` into `IpAddr`, so no string-level validation needed here. | ||
| // See docs/zeus-mcp-server.md. | ||
| if !args.bind.is_loopback() && !args.allow_non_loopback { | ||
| return Err(SetupError(format!( | ||
| "refusing to bind SSE transport on non-loopback address {} \ | ||
| without --allow-non-loopback (use 127.0.0.1 / ::1 for local-only)", | ||
| args.bind | ||
| )) | ||
| .into()); | ||
| } | ||
|
|
||
| eprintln!( | ||
| "zeus mcp: sse transport not yet implemented (port={}, bind={}, workspace={})", | ||
| args.port, | ||
| args.bind, | ||
| workspace.display() | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| Ok(0) |
There was a problem hiding this comment.
The command currently returns Ok(0) after printing that the transport is not yet implemented. For a CLI tool, it is more idiomatic to return an error (e.g., using SetupError) when a requested feature is unavailable, rather than exiting with a success code. This prevents scripts from proceeding as if the server had successfully started.
There was a problem hiding this comment.
Agreed and changed in ec322b0. Both the stdio and sse arms now return SetupError for the unimplemented case, so the process exits non-zero. A script that pipes zeus mcp into an MCP client expecting it to work would otherwise see a silent exit 0 and parse an empty pipe as a successful (but malformed) MCP session.
| ## Subcommand surface | ||
|
|
||
| ```text | ||
| zeus mcp [--transport stdio|sse] [--port N] [--workspace PATH] |
There was a problem hiding this comment.
The synopsis in the documentation is missing the --bind and --allow-non-loopback flags, which are implemented in the CLI arguments and discussed later in the 'Authentication' section. Including them here would provide a more complete overview of the command surface.
| zeus mcp [--transport stdio|sse] [--port N] [--workspace PATH] | |
| zeus mcp [--transport stdio|sse] [--port N] [--bind IP] [--allow-non-loopback] [--workspace PATH] |
There was a problem hiding this comment.
Updated in ec322b0. The synopsis now reads zeus mcp [--transport stdio|sse] [--port N] [--bind IP] [--allow-non-loopback] [--workspace PATH], with bullets describing each flag and the loopback-default / acknowledgement-flag relationship.
…is in docs Address remaining reviewer feedback: - Replace Ok(0) returns for unimplemented stdio/sse transports with SetupError so a script that pipes 'zeus mcp' into a real MCP client doesn't appear to succeed silently. The eprintln!() lines were also redundant with the SetupError display, so they're folded in. - Docs synopsis now lists --bind and --allow-non-loopback alongside --transport / --port / --workspace; previously the flags were implemented and documented further down, but the at-a-glance command line was incomplete.
1 participant
Goal
Add
zeus mcp(and equivalentcode mcp) as a CLI subcommand. This is the entry point for exposing Zeus as an MCP server. Real implementation lands in a follow-up; this PR scaffolds the surface so other PRs can hang implementation off it.Design doc:
docs/zeus-mcp-server.mdWhat this PR ships
cli/src/commands/mcp.rs— stub command handlercli/src/commands/args.rs—McpArgs { --transport, --port, --workspace }+McpTransport { Stdio, Sse }cli/src/commands.rs— module registrationcli/src/bin/code/main.rs— dispatchdocs/zeus-mcp-server.md— design / tool surfaceVerified locally
cargo checkpassescargo build --bin codepasses./target/debug/code mcp --helpshows the new options./target/debug/code mcpand./target/debug/code mcp --transport sse --port 8000dispatch to the stub correctlyAcceptance criteria (when impl lands)
code mcpboots, prints transport info, accepts an MCPinitializerequestbuffer_get/buffer_setround-trip a small file in tests--workspaceDepends on:
feat/zeus-conventions(for.zeus/mcp.jsonshape, only loosely)Part of the MCP-first pivot. See
zeus-internal/ARCHITECTURE.md.Summary by CodeRabbit
New Features
zeus mcpsubcommand with configurable transport selection (Stdio and SSE)Documentation