- Notifications
You must be signed in to change notification settings - Fork1.2k
get_file_contents accepts a ref instead of just branch#558
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
Uh oh!
There was an error while loading.Please reload this page.
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 expandsget_file_contents to accept arbitrary git refs and SHAs rather than only branches.
- Replaces the
branchparameter withrefandshain the tool’s schema - Updates
GetFileContentslogic to handleref,sha, and special pull-request refs - Refreshes tests and snapshots to assert
ref/shainstead ofbranch
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/github/repositories.go | Swapped outbranch schema, addedref/sha handling and PR logic |
| pkg/github/repositories_test.go | Updated tests to check forref/sha instead ofbranch |
| pkg/github/toolsnaps/get_file_contents.snap | Removedbranch from snapshot and addedref/sha properties |
Comments suppressed due to low confidence (2)
pkg/github/repositories_test.go:84
- There are no tests covering the new
shaparameter or the pull-request ref branch. Consider adding a test case that passes ashadirectly and one for arefs/pull/<n>/headto verify the override logic.
"ref": "refs/heads/main",pkg/github/repositories.go:473
- The code calls fmt.Errorf but I don't see an import for "fmt" in this diff. Please ensure that "fmt" is imported to avoid a compile error.
return nil, fmt.Errorf("failed to get GitHub client: %w", err)Uh oh!
There was an error while loading.Please reload this page.
almaleksia commentedJun 23, 2025
Howdy 👋 Can we make the change backwards compatible because we depend on this tool:https://github.com/github/sweagentd/pull/3357 |
Uh oh!
There was an error while loading.Please reload this page.
| @@ -6,10 +6,6 @@ | |||
| "description": "Get the contents of a file or directory from a GitHub repository", | |||
| "inputSchema": { | |||
| "properties": { | |||
| "branch": { | |||
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.
Just as an aside, I think it's really nice to look at the snapshot to see the structural changes that have occurred without looking at the imperative tool handler.
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.
Agree, just looking at the files commit history is pretty great way to check user faced changes.
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.
I think we will use this for 1p clients to help them understand the diff.
b2901e1 intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
#get_file_contentsnow accepts:instead of just branches.
Closes:#539