feat: MCP write tools -- add_repository, index, directories, delete (OPE-165)#287
Merged
Merged
Conversation
…OPE-165)
4 new MCP tools so we can manage repos without leaving the conversation:
- add_repository: POST /repos (name, git_url, branch)
- get_repo_directories: GET /repos/{repo_id}/directories
- index_repository: POST /repos/{repo_id}/index (with optional include_paths)
- delete_repository: DELETE /repos/{repo_id}
Also adds api_delete to the HTTP client.
All 45 tests pass, lint clean.
|
@DevanshuNEU is attempting to deploy a commit to the Dev's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded repository-management tools and handlers plus an API client DELETE helper: new tool schemas and tests for add/get directories/index/delete repository operations; handlers wired into the dispatch map call the API client (including new Changes
Sequence Diagram(s)sequenceDiagram
participant User as Client
participant Handler as MCP Handler
participant APIClient as api_client.get_client()/api_delete
participant Backend as Repo Backend
User->>Handler: invoke tool (e.g., delete_repository repo_id)
Handler->>APIClient: DELETE /repos/{repo_id}
APIClient->>Backend: HTTP DELETE /repos/{repo_id}
Backend-->>APIClient: 200 OK / 204 No Content (JSON or empty)
APIClient-->>Handler: parsed JSON or {}
Handler-->>User: formatted status string/result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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)
mcp-server/api_client.py (1)
61-66: Consider handling DELETE responses that return no body.The backend currently returns JSON, but DELETE endpoints sometimes return 204 No Content with an empty body. Calling
response.json()on an empty response would raise an exception.🛡️ Defensive fix for empty responses
async def api_delete(path: str, **kwargs: Any) -> dict: """Make a DELETE request to the backend API.""" client = await get_client() response = await client.delete(path, **kwargs) response.raise_for_status() - return response.json() + if response.status_code == 204 or not response.content: + return {} + return response.json()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcp-server/api_client.py` around lines 61 - 66, The api_delete function currently calls response.json() which will raise if the DELETE returns 204 No Content or an empty body; update api_delete (and keep using get_client and response.raise_for_status) to defensively handle empty responses by checking response.status_code (or response.content/response.text) and returning an empty dict (or None per contract) when there's no body, otherwise parse and return response.json(); ensure the behavior is consistent with other API helpers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mcp-server/handlers.py`:
- Around line 84-110: The handler _handle_add_repository reads the wrong
response key for the repository identifier: it uses result.get("id", ...) which
the backend returns as "repo_id"; update the code to read repo_id =
result.get("repo_id", "unknown") (and ensure any other references to
result.get("id") in that function are replaced) so the returned guidance string
(including the index_repository(repo_id='...') hint) contains the real repo id.
- Around line 130-156: The handler _handle_index_repository is reading the wrong
response field and is sending an unsupported include_paths body to the sync
index endpoint; change the response mapping to use result.get("functions",
result.get("functions_indexed", 0)) for fn_count (and keep file_count fallback
as-is), and stop sending include_paths to POST /repos/{repo_id}/index (remove
include_paths from params) — if include_paths support is required, call the
async endpoint "/repos/{repo_id}/index/async" instead and pass include_paths
inside the JSON payload (e.g. use api_post(f"/repos/{repo_id}/index/async",
json={ "include_paths": include_paths }) when include_paths is present).
In `@mcp-server/tests/test_tools.py`:
- Around line 18-21: The test test_repo_tools_require_repo_id is missing the new
tools in its repo_tools list; update the repo_tools variable inside
test_repo_tools_require_repo_id to include "get_repo_directories",
"index_repository", and "delete_repository" alongside the existing
"add_repository" so the schema validation covers these tools that require
repo_id.
---
Nitpick comments:
In `@mcp-server/api_client.py`:
- Around line 61-66: The api_delete function currently calls response.json()
which will raise if the DELETE returns 204 No Content or an empty body; update
api_delete (and keep using get_client and response.raise_for_status) to
defensively handle empty responses by checking response.status_code (or
response.content/response.text) and returning an empty dict (or None per
contract) when there's no body, otherwise parse and return response.json();
ensure the behavior is consistent with other API helpers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 45ca7132-f732-485c-baf8-d4f78c3ed173
📒 Files selected for processing (4)
mcp-server/api_client.pymcp-server/handlers.pymcp-server/tests/test_tools.pymcp-server/tools.py
…defensive delete
- handlers.py: add_repository reads result.get('repo_id') not 'id'
(backend returns 'repo_id')
- handlers.py: index_repository uses /index/async when include_paths
set (sync endpoint doesn't accept include_paths), reads 'functions'
not 'function_count'
- api_client.py: api_delete handles 204/empty body gracefully
- test_tools.py: repo_tools list includes new write tools
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mcp-server/handlers.py`:
- Around line 132-150: The code currently treats an explicit empty include_paths
list as omitted and falls through to the full-repo sync index; change the check
so we distinguish None (omitted) from an explicit empty list: use a presence
check (e.g. if "include_paths" in args or if include_paths is not None) and if
include_paths == [] return/reject with a BadRequest-style response indicating
empty include_paths is invalid; otherwise proceed to call the async subset
endpoint via api_post(f"/repos/{repo_id}/index/async", json={"include_paths":
include_paths}) and keep the existing status/response handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2d98e752-14e9-4614-b054-5ad6fc078060
📒 Files selected for processing (3)
mcp-server/api_client.pymcp-server/handlers.pymcp-server/tests/test_tools.py
🚧 Files skipped from review as they are similar to previous changes (2)
- mcp-server/tests/test_tools.py
- mcp-server/api_client.py
[] is falsy in Python, so 'if include_paths:' treated an explicit empty list the same as None (omitted). Now returns a clear error for empty list, preserving the distinction between 'index everything' (omit include_paths) and 'index nothing' (empty list).
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
4 new MCP tools for managing repositories directly from Claude conversations:
Why
MCP only had read tools. To add/index a repo, you had to go to the dashboard. Now we can do everything from Claude -- true dogfooding.
Changes
api_client.py: Addedapi_deletemethodtools.py: 4 new tool schemas with LLM-friendly descriptionshandlers.py: 4 new handler functions with formatted responsestests/test_tools.py: Updated EXPECTED_TOOLS setTesting
45 tests pass, flake8 clean.
Workflow after merge
Summary by CodeRabbit