repo_search_commits: removing client-side-filters#1360
Conversation
| server.tool( | ||
| REPO_TOOLS.search_commits, | ||
| "Search for commits in a repository with comprehensive filtering capabilities. Supports searching by description/comment text, time range, author, committer, specific commit IDs, and more. This is the unified tool for all commit search operations.", | ||
| "Search for commits in a repository with comprehensive filtering capabilities. Supports searching by description/comment text, time range, author, committer, specific commit IDs, and more. NOTE: searchText, authorEmail, committer, and committerEmail are client-side filters — they run against the commits already fetched from the API (limited by 'skip' and 'top'), not against the full repository history. Use API-level filters (fromDate, toDate, author, version) to narrow results before these filters apply.", |
There was a problem hiding this comment.
This is a big strange actually. I'm not sure we want to expose so many details through the tool description.
There was a problem hiding this comment.
I had looked at commitSearchResults api as you asked. I found out that it doesn't support some of the existing filter parameters in the tool.
It only supports this:
{
"searchText": "fix bug",
"$skip": 0,
"$top": 50,
"filters": {
"Project": ["MyProject"],
"Repository": ["MyRepo"],
"Branch": ["refs/heads/main"],
"Author": ["Alice Doe <[email protected]>"]
},
"$orderBy": [
{
"field": "authored_date",
"sortOrder": "DESC"
}
],
"includeFacets": false
}
If we want only server side-filtering to be applicable, we might have to implement a seperate tool call.
There was a problem hiding this comment.
I think we should align with the search API, rather than try to invent something new here that our APIs don't support.
@danhellem how would you feel about dropping some of the search parameters for search commits to align it with the search API that we have available. IMO, having client side filtering in the MCP server layer confuses the situation. Everything works better if we stick to what our REST APIs support
There was a problem hiding this comment.
@dpaquette I agree 100%, the tool should match API. We should not be adding extra stuff.
There was a problem hiding this comment.
@krid-583 Let's go ahead and change this tool to map to the inputs available for the commit search API
There was a problem hiding this comment.
I think you aligned with the wrong API. We should alight with the more advanced search API that supports searchText. The one you outlined in this comment thread
krid-583
changed the title
3 participants
Updated the repo_search_commits tool to remove all the added client-side-filters. Also added one more server side filter namely
userwhich replacescommitter. UpdatedhistorySimplificationModeto be included inside the searchCriteria as it was previously a dummy parameter for filtering.GitHub issue number
Closes #1327
Associated Risks
None
✅ PR Checklist
🧪 How did you test it?
Manually tested the tool. No changes in behaviour.