Conversation
Connects to any MCP server, reads its tool/resource/prompt schemas, and writes a standalone Python CLI script with typed subcommands.
jlowin
added
the
feature
marvin-context-protocol
Bot
added
the
cli
![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: 2c705faa87
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if is_required: | ||
| annotation = ( | ||
| f'Annotated[{py_type}, cyclopts.Parameter(help="{help_escaped}")]' | ||
| ) | ||
| param_lines.append(f" {prop_name}: {annotation},") |
There was a problem hiding this comment.
The generator emits JSON Schema property names directly as Python keyword parameters. MCP/JSON Schema property names are arbitrary strings, so names containing hyphens, spaces, or Python keywords (e.g., from) will produce invalid syntax in the generated script and make compile() fail. This is triggered whenever a server exposes such property names; consider mapping to safe identifiers (and translating back when building the argument dict) or using **kwargs handling.
Useful? React with 👍 / 👎.
| # Function name: use tool name directly (preserve underscores) | ||
| fn_name = tool.name.replace("-", "_") |
There was a problem hiding this comment.
Tool names are only normalized by replacing - with _ when generating function names. MCP tool names can include other non-identifier characters (e.g., ., /, spaces) or start with digits, which will produce invalid Python function definitions and break the generated CLI. This will surface for any server that uses such tool names; consider a fuller identifier sanitization step while still preserving the original name in the command decorator.
Useful? React with 👍 / 👎.
WalkthroughAdds a top-level CLI command Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
![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
🧹 Nitpick comments (1)
src/fastmcp/cli/generate.py (1)
494-495: Edge case: trailing colon yields empty name.If
server_specis"source:", the split returns an empty string, which could cause issues in generated code (e.g., empty app name).♻️ Proposed fix to handle edge case
# Bare name or qualified name if ":" in server_spec: - return server_spec.split(":", 1)[1] + name = server_spec.split(":", 1)[1] + return name if name else server_spec.split(":", 1)[0] return server_spec
Test Failure AnalysisSummary: The Root Cause: The test at Suggested Solution: Skip this test on Windows, as the executable bit concept doesn't apply there. Python files are executable on Windows by default when they have the proper shebang and file association. Add the following decorator to the test: @pytest.mark.skipif(
sys.platform == "win32",
reason="Executable permissions don't exist on Windows"
)
async def test_file_is_executable(self, tmp_path: Path):Don't forget to add Detailed AnalysisThe failure occurs in the Windows test job (Python 3.10 on windows-latest): The The Related Files
|
- Sanitize tool and parameter names to valid Python identifiers - Replace bare except Exception with specific exception types - Escape server name in generated string literals - Handle trailing colon edge case in _derive_server_name - Clarify in docs that generated CLI is a client, not a bundled server
- Use single-quoted docstrings to avoid triple-quote escaping issues - Escape quotes in app_name derived from server_name - Add tests for descriptions with quotes and server names with quotes Addresses CodeRabbit review comments about insufficient escaping.
- Simple types (str, int, float, bool): Direct typed flags - Arrays of simple types (list[str], list[int]): Repeatable flags via cyclopts - Complex types (objects, nested arrays): Accept JSON strings with parsing - JSON schema shown in help text for complex parameters - Proper escaping of newlines and quotes in help text - Filter out None and empty list defaults when calling tools This gives typed, discoverable CLIs for common cases while handling complex schemas via JSON input.
- Document simple types as direct typed flags - Document arrays of simple types as repeatable flags - Document complex types as JSON strings with schema in help - Add examples showing all three patterns
High priority fixes: - Complex type defaults: Serialize dict/list defaults to JSON strings - List params: Preserve help metadata with Annotated wrapper - Name collisions: Detect and error on sanitized name conflicts - JSON parsing: Use isinstance check for safety with defaults Added tests for: - Complex types with default values - Parameter name collision detection - Updated existing tests to match new format
- Generator now uses pydantic_core.to_json() instead of json.dumps() - Consistent with rest of fastmcp codebase - Generated CLI still uses plain json module (standalone script)
| def serialize_transport( | ||
| resolved: str | dict[str, Any] | ClientTransport, | ||
| ) -> tuple[str, set[str]]: | ||
| """Serialize a resolved transport to a Python expression string. | ||
|
|
||
| Returns ``(expression, extra_imports)`` where *extra_imports* is a set of | ||
| import lines needed by the expression. | ||
| """ | ||
| if isinstance(resolved, str): | ||
| return repr(resolved), set() | ||
|
|
||
| if isinstance(resolved, StdioTransport): | ||
| parts = [f"command={resolved.command!r}", f"args={resolved.args!r}"] | ||
| if resolved.env: | ||
| parts.append(f"env={resolved.env!r}") | ||
| if resolved.cwd: | ||
| parts.append(f"cwd={resolved.cwd!r}") | ||
| expr = f"StdioTransport({', '.join(parts)})" | ||
| imports = {"from fastmcp.client.transports import StdioTransport"} | ||
| return expr, imports | ||
|
|
||
| if isinstance(resolved, dict): | ||
| return repr(resolved), set() | ||
|
|
||
| # Fallback: try repr | ||
| return repr(resolved), set() |
There was a problem hiding this comment.
Preserve non-default StdioTransport fields during serialization.
log_file (notably for .py specs) and keep_alive are currently dropped, and empty env/cwd are ignored via truthiness checks. This can change runtime behavior in the generated CLI.
🛠️ Proposed fix
if isinstance(resolved, StdioTransport):
- parts = [f"command={resolved.command!r}", f"args={resolved.args!r}"]
- if resolved.env:
- parts.append(f"env={resolved.env!r}")
- if resolved.cwd:
- parts.append(f"cwd={resolved.cwd!r}")
+ imports = {"from fastmcp.client.transports import StdioTransport"}
+ parts = [f"command={resolved.command!r}", f"args={resolved.args!r}"]
+ if resolved.env is not None:
+ parts.append(f"env={resolved.env!r}")
+ if resolved.cwd is not None:
+ parts.append(f"cwd={resolved.cwd!r}")
+ if resolved.keep_alive is not True:
+ parts.append(f"keep_alive={resolved.keep_alive!r}")
+ if resolved.log_file is not None:
+ if isinstance(resolved.log_file, Path):
+ parts.append(f"log_file=Path({str(resolved.log_file)!r})")
+ imports.add("from pathlib import Path")
+ else:
+ parts.append(f"log_file={resolved.log_file!r}")
expr = f"StdioTransport({', '.join(parts)})"
- imports = {"from fastmcp.client.transports import StdioTransport"}
return expr, imports
1 participant
Any MCP server — FastMCP or otherwise — already describes its capabilities through tool schemas.
generate-clitakes that schema and turns it into a real CLI you can run, edit, and distribute.The generated script embeds the resolved transport (URL or stdio command) and has proper typed subcommands:
Tool parameters are mapped from JSON Schema to Python type annotations with
cyclopts.Parametermetadata, so you get help text, type coercion, and required/optional validation for free. Tool names are preserved as-is (underscores, not kebab-cased).