- 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
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 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 |
| @@ -18,9 +14,17 @@ | |||
| "description": "Path to file/directory (directories must end with a slash '/')", | |||
| "type": "string" | |||
| }, | |||
| "ref": { | |||
| "description": "Accepts optional git refs such as `refs/tags/\u003ctag\u003e`, `refs/heads/\u003cbranch\u003e` or `refs/pull/\u003cpr_number\u003e/head`", | |||
SamMorrowDrumsJun 23, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
| "description":"Accepts optional git refs such as `refs/tags/\u003ctag\u003e`, `refs/heads/\u003cbranch\u003e` or `refs/pull/\u003cpr_number\u003e/head`", | |
| "description":"Accepts optional git refs such as `refs/tags/<tag>`, `refs/heads/<branch>` or `refs/pull/<pr_number>/head`", |
This seems broken, these should just be regular angled brackets no? They shouldn't need Unicode markup.
Uh oh!
There was an error while loading.Please reload this page.
#get_file_contentsnow accepts:instead of just branches.
Closes:#539