get_file_contents accepts a ref instead of just branch #558
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR expands get_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 out branch schema, added ref/sha handling and PR logic |
| pkg/github/repositories_test.go | Updated tests to check for ref/sha instead of branch |
| pkg/github/toolsnaps/get_file_contents.snap | Removed branch from snapshot and added ref/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)
|
Howdy 👋 Can we make the change backwards compatible because we depend on this tool: https://github.com/github/sweagentd/pull/3357 |
| @@ -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.
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.
Agree, just looking at the files commit history is pretty great way to check user faced changes.
There was a problem hiding this comment.
I think we will use this for 1p clients to help them understand the diff.
#get_file_contentsnow accepts:instead of just branches.
Closes: #539