feat(mcp): scaffold api/mcp module (T1 #648)#666
Conversation
… entry point Add the bare MCP server module (api/mcp/) using the official FastMCP SDK, wire the cgraph-mcp console script in pyproject.toml, and include a protocol smoke test that spawns the server over stdio and verifies list_tools returns an empty tool set. Also copies the MCP design docs into docs/. Closes #648 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
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 (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughScaffolds an MCP stdio server under Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 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 |
There was a problem hiding this comment.
Pull request overview
Scaffolds an MCP (Model Context Protocol) server module within the existing Python backend so external agents can connect via stdio, and establishes the packaging/test wiring needed to evolve into a full “code-graph” MCP server.
Changes:
- Add
api/mcp/with a bareFastMCP("code-graph")app plus a stdiomain()runner. - Wire a new
cgraph-mcpconsole script and add themcp>=1,<2dependency (with updated lockfile). - Add MCP stdio smoke tests and check in MCP design documentation assets.
Reviewed changes
Copilot reviewed 5 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
api/mcp/server.py |
Introduces the FastMCP app instance and stdio runner entry point. |
api/mcp/__init__.py |
Adds module-level documentation for the MCP server package. |
pyproject.toml |
Adds mcp dependency and cgraph-mcp console-script wiring. |
uv.lock |
Locks transitive dependencies introduced by mcp (and its dependency graph). |
tests/mcp/test_scaffold.py |
Adds a stdio protocol smoke test verifying handshake + empty tool list. |
tests/mcp/__init__.py |
Establishes the tests.mcp package. |
docs/MCP_SERVER_DESIGN.md |
Adds MCP server design summary and roadmap for Phase 1+. |
docs/code-graph-mcp-v4.docx |
Adds the full design document (binary). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [project.scripts] | ||
| cgraph = "api.cli:app" | ||
| cgraph-mcp = "api.mcp.server:main" | ||
|
|
There was a problem hiding this comment.
The design/issue text specifies the console-script entry point as api.mcp.server:app, but pyproject.toml wires cgraph-mcp to api.mcp.server:main. Please align the entry point target with the documented contract (either change the script target back to :app if supported by the MCP SDK, or update the docs/issue references to :main).
| "javatools>=1.6.0,<2.0.0", | ||
| "pygit2>=1.17.0,<2.0.0", | ||
| "typer>=0.24.0,<1.0.0", | ||
| "mcp>=1.0.0,<2.0.0", | ||
| ] |
There was a problem hiding this comment.
Adding mcp as a core dependency pulls in heavier transitive deps (notably pyjwt[crypto] → cryptography, plus python-multipart, sse-starlette, etc. per uv.lock). Please confirm this footprint is acceptable for all installs of the base package; if MCP is optional, consider moving it under an extra to avoid requiring compiled wheels for users who don’t use cgraph-mcp.
docs/MCP_SERVER_DESIGN.md
Outdated
| ## Key Decisions Made | ||
|
|
||
| - **Mono-repo**: Build inside `code-graph/api/mcp/`, not a separate project. One pip package, one repo. | ||
| - **Module path is `api/mcp/`, NOT top-level `mcp/`**: A top-level `mcp/` directory would shadow the installed `mcp` PyPI SDK and break `from mcp.server.fastmcp import FastMCP`. Entry point: `cgraph-mcp = "api.mcp.server:app"`. |
There was a problem hiding this comment.
This section documents the entry point as cgraph-mcp = "api.mcp.server:app", but the PR wires the console script to api.mcp.server:main. Please update this doc (and any other occurrences) so the documented entry point matches the actual packaging.
| - **Module path is `api/mcp/`, NOT top-level `mcp/`**: A top-level `mcp/` directory would shadow the installed `mcp` PyPI SDK and break `from mcp.server.fastmcp import FastMCP`. Entry point: `cgraph-mcp = "api.mcp.server:app"`. | |
| - **Module path is `api/mcp/`, NOT top-level `mcp/`**: A top-level `mcp/` directory would shadow the installed `mcp` PyPI SDK and break `from mcp.server.fastmcp import FastMCP`. Entry point: `cgraph-mcp = "api.mcp.server:main"`. |
| │ ├── test_ask.py # T11 — mocked LLM, real Cypher against fixture | ||
| │ ├── test_auto_init.py # T12 | ||
| │ └── test_init_agent.py # T13 | ||
| └── pyproject.toml # Adds `cgraph-mcp = "api.mcp.server:app"` and `mcp>=1.0,<2.0` | ||
| ``` | ||
|
|
There was a problem hiding this comment.
The directory structure example also states cgraph-mcp = "api.mcp.server:app", but the package currently exports cgraph-mcp = "api.mcp.server:main". Please keep the design doc consistent with the actual pyproject.toml script target.
docs/MCP_SERVER_DESIGN.md
Outdated
| - **GraphRAG SDK powers the ask tool**: `kg.ask()` does NL-to-Cypher. Code-graph's `api/llm.py` already integrates this — repackage for MCP. | ||
| - **Reuse the existing hand-coded ontology** from `api/llm.py:_define_ontology()` (lines 26–233) rather than auto-extracting via `Ontology.from_kg_graph()`. The hand-coded version has richer entity attributes and descriptions tuned for code. Refactor: rename `_define_ontology` → `define_ontology` so the MCP module can import it. | ||
| - **Ship with 3 languages** (Python/Java/C#), add tree-sitter for broad coverage in Phase 2. | ||
| - **No incremental indexing in v1**: Full re-indexing is sufficient. Deferred to Phase 3. |
There was a problem hiding this comment.
The “Key Decisions Made” list says “No incremental indexing in v1” (deferred), but later in this same document it describes incremental indexing as “default-on” in Phase 1. Please reconcile these statements so the design doc has a single, consistent plan.
| - **No incremental indexing in v1**: Full re-indexing is sufficient. Deferred to Phase 3. | |
| - **Incremental indexing in v1**: First-time indexing is full; once a `(project, branch)` graph exists, incremental indexing is the default. Allow explicit full re-index via `--full` (CLI) or `incremental=False` (MCP). |
tests/mcp/test_scaffold.py
Outdated
| params = StdioServerParameters(command=cgraph_mcp, args=[]) | ||
| async with stdio_client(params) as (read, write): | ||
| async with ClientSession(read, write) as session: | ||
| await session.initialize() | ||
| result = await session.list_tools() | ||
| assert result.tools == [] |
There was a problem hiding this comment.
This protocol smoke test can hang indefinitely if the spawned server fails to start or the MCP handshake stalls (no timeout around initialize() / list_tools()). Consider wrapping the stdio session block in an anyio.fail_after(...) (or equivalent pytest timeout) to keep CI runs from blocking forever on failures.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/MCP_SERVER_DESIGN.md`:
- Line 18: Update the documented console-script entry point from
"api.mcp.server:app" to the actual runtime target "api.mcp.server:main" wherever
it appears in the design doc (e.g., the references currently stating
api.mcp.server:app around the console-script/entrypoint description), so the doc
matches the pyproject wiring and avoid misleading setup instructions.
- Line 37: Several fenced code blocks in the document (the ones showing
"code-graph/ ..." the ASCII graph starting with "T1 ──┬─> T2 ..." and the cli
example "claude mcp add-json \"code-graph\" ...") are missing language
identifiers; update those backtick-fenced blocks to include appropriate language
tags (e.g., ```text for directory/listing and ASCII art, ```bash for the CLI
command) so markdownlint MD040 is satisfied and the blocks render correctly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 208a6980-6e4c-4d2e-92e2-51f4193a35af
⛔ Files ignored due to path filters (2)
docs/code-graph-mcp-v4.docxis excluded by!**/*.docxuv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
api/mcp/__init__.pyapi/mcp/server.pydocs/MCP_SERVER_DESIGN.mdpyproject.tomltests/mcp/__init__.pytests/mcp/test_scaffold.py
- Fix stale entry point references in design doc: api.mcp.server:app → :main - Remove contradicting decisions about tree-sitter/incremental indexing scope - Add language tags to fenced code blocks (MD040) - Add anyio.fail_after timeout to stdio smoke test to prevent CI hangs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
api/mcp/module with a bareFastMCP("code-graph")server instancecgraph-mcpconsole-script entry point inpyproject.tomlmcp>=1.0,<2.0dependencydocs/MCP_SERVER_DESIGN.md,docs/code-graph-mcp-v4.docx) into the repotests/mcp/test_scaffold.py) that spawns the server over stdio, completes the MCP handshake, and assertslist_toolsreturns an empty setTest plan
uv run pytest tests/mcp/test_scaffold.py -v— 3/3 pass (import, entry point, stdio round-trip)ruff check api/mcp/ tests/mcp/— cleancgraph-mcpresolves on PATH afteruv syncCloses #648
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
Chores