Skip to content

Commit 408efde

Browse files
DvirDukhanclaude
andcommitted
fix(mcp): address Copilot + CodeRabbit review comments on T1 PR
- 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>
1 parent 92d6805 commit 408efde

2 files changed

Lines changed: 16 additions & 12 deletions

File tree

docs/MCP_SERVER_DESIGN.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,13 @@ Phase 1 also bundles three foundational improvements to `api/` that the MCP serv
1515
## Key Decisions Made
1616

1717
- **Mono-repo**: Build inside `code-graph/api/mcp/`, not a separate project. One pip package, one repo.
18-
- **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"`.
18+
- **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"`.
1919
- **Python MCP server**: Use the official `mcp` Python SDK (`from mcp.server.fastmcp import FastMCP`), NOT standalone `fastmcp` or Node.js. Avoids language bridge.
2020
- **Reuse everything**: Most code already exists. The MCP tools are thin wrappers around `api/graph.py`, `api/project.py`, `api/cli.py`, and `api/llm.py`.
2121
- **GraphRAG SDK powers the ask tool**: `kg.ask()` does NL-to-Cypher. Code-graph's `api/llm.py` already integrates this — repackage for MCP.
2222
- **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.
23-
- **Ship with 3 languages** (Python/Java/C#), add tree-sitter for broad coverage in Phase 2.
24-
- **No incremental indexing in v1**: Full re-indexing is sufficient. Deferred to Phase 3.
23+
- **11 languages in Phase 1**: Python/JS/Kotlin via tree-sitter (refactored onto a shared base class in T15), Go/Rust/TypeScript/Ruby/C/C++ added in T16, Java/C# stay on multilspy.
24+
- **Incremental indexing in Phase 1**: file-hash-based skip-unchanged, default-on once a `(project, branch)` graph exists (T18). Full re-index via `--full` (CLI) or `incremental=False` (MCP).
2525
- **No Graphiti/memory or raw FalkorDB MCP in v1**: Out of scope. Available as separate servers. Architecture supports merging later.
2626
- **Auto-init for zero config**: ensure-db auto-starts FalkorDB Docker, auto-index on first tool call, auto-GraphRAG init.
2727
- **Expose Cypher in ask responses**: Transparency for the agent + learning patterns.
@@ -34,7 +34,7 @@ Phase 1 also bundles three foundational improvements to `api/` that the MCP serv
3434

3535
## Directory Structure
3636

37-
```
37+
```text
3838
code-graph/
3939
├── api/ # Existing Python backend (FastAPI, analyzers, graph, llm, cli)
4040
│ └── mcp/ # NEW — MCP server module (under api/ to avoid shadowing the installed `mcp` SDK)
@@ -69,7 +69,7 @@ code-graph/
6969
│ ├── test_ask.py # T11 — mocked LLM, real Cypher against fixture
7070
│ ├── test_auto_init.py # T12
7171
│ └── test_init_agent.py # T13
72-
└── pyproject.toml # Adds `cgraph-mcp = "api.mcp.server:app"` and `mcp>=1.0,<2.0`
72+
└── pyproject.toml # Adds `cgraph-mcp = "api.mcp.server:main"` and `mcp>=1.0,<2.0`
7373
```
7474

7575
**Note on test layout:** Each tool ticket ships its own integration + MCP-protocol round-trip tests in the same PR — there is no separate "integration tests" or "protocol tests" milestone. The previous `integration/` and `e2e/` subdirectories are removed in favor of per-tool test files. Real-LLM E2E is deferred to Phase 1.5.
@@ -276,7 +276,7 @@ Each tool ticket ships impl + unit + integration + protocol round-trip in a sing
276276
- Real-LLM nightly E2E with API-key secrets (was a row in the CI table)
277277

278278
### Dependency graph
279-
```
279+
```text
280280
T1 ──┬─> T2 ──> T3 ──> T17 ──> T4 ──┬─> T5
281281
│ ├─> T6
282282
│ ├─> T7
@@ -304,7 +304,7 @@ After T17 lands, multiple streams parallelize: structural tools (T4 → T5/T6/T7
304304
| MCP_PORT | HTTP transport port | 3000 |
305305

306306
Quick start (Claude Code):
307-
```
307+
```bash
308308
claude mcp add-json "code-graph" '{"command":"cgraph-mcp","env":{"FALKORDB_HOST":"localhost","LLM_API_KEY":"sk-..."}}'
309309
```
310310

tests/mcp/test_scaffold.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,13 @@
1515

1616
import shutil
1717

18+
import anyio
1819
import pytest
1920
from mcp import ClientSession, StdioServerParameters
2021
from mcp.client.stdio import stdio_client
2122

23+
STDIO_TIMEOUT = 30 # seconds — prevents CI from hanging if the server fails to start
24+
2225

2326
def test_app_is_importable() -> None:
2427
"""The FastMCP instance can be imported and is named ``code-graph``."""
@@ -48,8 +51,9 @@ async def test_stdio_server_lists_zero_tools() -> None:
4851
)
4952

5053
params = StdioServerParameters(command=cgraph_mcp, args=[])
51-
async with stdio_client(params) as (read, write):
52-
async with ClientSession(read, write) as session:
53-
await session.initialize()
54-
result = await session.list_tools()
55-
assert result.tools == []
54+
with anyio.fail_after(STDIO_TIMEOUT):
55+
async with stdio_client(params) as (read, write):
56+
async with ClientSession(read, write) as session:
57+
await session.initialize()
58+
result = await session.list_tools()
59+
assert result.tools == []

0 commit comments

Comments
 (0)