get_file_contents fetch refs improvements#1655
Conversation
…to push-files-improvements
There was a problem hiding this comment.
Pull request overview
This PR enhances the get_file_contents tool's ref resolution by auto-detecting Git commit SHAs passed in the ref parameter and providing clearer error messages when the 'main' branch is not found.
- Adds
looksLikeSHA()function to detect full 40-character commit SHAs and bypass branch/tag resolution - Implements custom error messages suggesting 'master' when 'main' branch is not found
- Adds comprehensive test coverage for the SHA detection logic
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| pkg/github/repositories.go | Adds looksLikeSHA() helper function and integrates SHA detection into resolveGitReference(); adds custom error messages for missing 'main' branch |
| pkg/github/repositories_test.go | Adds unit tests for looksLikeSHA() function and integration tests for SHA-based ref resolution in Test_resolveGitReference() |
8988b78 to
f76807b
Compare
get_file_contents fetch refs improvements
mattdholloway
left a comment
There was a problem hiding this comment.
looks great! looksLikeSHA is a really good idea
SamMorrowDrums
left a comment
There was a problem hiding this comment.
Very nice. Out of interest, do we have a way of looking up the default ref? And should we just try master or versa and just say what we did as part of the response? I guess it's just how things are that we need heuristics like this. But higher % success of achieving actual intent feels reasonable.
|
@SamMorrowDrums We can actually do that using |
|
Yeah agree that seems the correct call |
Summary
This PR improves ref resolution handling in
get_file_contentstool.refparameter which results in failure. This PR autocorrects it allowing shas to be passed as refs.mainbranch and failed - suggest to trymasterinstead.Why
We have lots of errors like
failed to resolve git reference: could not resolve ref "50e4e8b9178ca5c4e3c60d0022b3b041cc58c188" as a branch or a tagWhich means sha is passed as ref, this change aims to decrease amounts of such failures.
What changed
MCP impact
Prompts tested (tool changes only)
Security / limits
Lint & tests
./script/lint./script/testDocs