feat: MCP server for documentation retrieval - #73
Conversation
The reusable asset in this repo is the folder-index retrieval layer. "Which docs cover this code?" is a query any agent wants. Expose it as a read-only MCP server with three tools: find_docs_for_code, get_doc_index, and check_doc_drift. Read-only by design; no write tools in this commit. Includes docs/mcp.md with setup instructions for Claude Code and other MCP clients.
|
🤖 Finished Review · ✅ Success · Started 5:56 AM UTC · Completed 6:11 AM UTC Commit: |
ReviewFindingsCritical
High
Medium
Low
Labels: PR adds a new MCP server feature in Python Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| results = [] | ||
|
|
||
| for folder, info in manifest.get("folders", {}).items(): | ||
| index_text = info.get("index", "") |
There was a problem hiding this comment.
[critical] api-contract
Both _find_docs_for_code and _get_doc_index read index content via info.get("index", "") from the manifest. The manifest never stores an "index" key — it stores {"built": ..., "doc_hashes": ...} per folder. Actual index text lives in separate .index.md files loaded via load_index() or load_all_indexes(). Two of three MCP tools always return empty/not-found responses.
Suggested fix: Replace manifest-based lookup with load_all_indexes(docs_root) in _find_docs_for_code and load_index(folder, docs_root) in _get_doc_index. Import these from doc_index.
| async def _check_doc_drift(doc_path): | ||
| """Assess staleness of a single doc file.""" | ||
| docs_root = get_docs_root().resolve() | ||
| full_path = docs_root / doc_path |
There was a problem hiding this comment.
[high] path-traversal
_check_doc_drift constructs full_path = docs_root / doc_path from user-supplied input without validating the resolved path stays within docs root. A path like ../../etc/passwd leaks file metadata (existence, line count, code-block presence) of arbitrary files. The codebase provides validate_file_path() in security_utils.py for this purpose.
Suggested fix: Resolve and validate: full_path = (docs_root / doc_path).resolve(); if not full_path.is_relative_to(docs_root): return error. Alternatively, import and use validate_file_path from security_utils.
| ), | ||
| ) | ||
| ] | ||
| except Exception as e: |
There was a problem hiding this comment.
[medium] data-exposure
Exception handler exposes raw exception message in MCP response. Python file-operation exceptions can include full filesystem paths. The codebase consistently uses sanitize_output() from security_utils for error output.
Suggested fix: Import sanitize_output from security_utils and wrap: f"Error reading {doc_path}: {sanitize_output(str(e))}"
| from mcp.server.stdio import stdio_server | ||
| from mcp.types import TextContent, Tool | ||
|
|
||
| sys.path.insert(0, str(Path(__file__).resolve().parent)) |
There was a problem hiding this comment.
[medium] pattern-inconsistency
sys.path.insert(0, ...) is not used in any other module in the codebase. As a standalone entry point, consider using uv run python -m invocation or configuring a package entry point instead.
| return await _get_doc_index(arguments.get("folder", "")) | ||
| elif name == "check_doc_drift": | ||
| return await _check_doc_drift(arguments.get("doc_path", "")) | ||
| return [TextContent(type="text", text=f"Unknown tool: {name}")] |
There was a problem hiding this comment.
[low] fail-open
call_tool dispatcher passes arguments to handlers without input validation. Low risk given read-only design and MCP protocol-level schema validation.
fullsend-ai-review
Bot
added
feature
python
1 participant
Summary
Exposes the folder-index retrieval layer as a read-only MCP server. Any MCP-compatible agent can query which docs cover a given source file.
find_docs_for_code,get_doc_index,check_doc_driftdocs/mcp.md: setup for Claude Code and other MCP clientsmcppackage (already in dependencies)Test plan
uv run pytest -vpasses (417 tests)uv run python src/mcp_server.pystarts and responds to MCP protocol