- Notifications
You must be signed in to change notification settings - Fork3.2k
get_file_contents fetch refs improvements#1655
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull request overview
This PR enhances theget_file_contents tool's ref resolution by auto-detecting Git commit SHAs passed in theref 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 | AddslooksLikeSHA() helper function and integrates SHA detection intoresolveGitReference(); adds custom error messages for missing 'main' branch |
| pkg/github/repositories_test.go | Adds unit tests forlooksLikeSHA() function and integration tests for SHA-based ref resolution inTest_resolveGitReference() |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
8988b78 tof76807bCompareget_file_contents fetch refs improvements
mattdholloway left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
looks great!looksLikeSHA is a really good idea
SamMorrowDrums left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
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