-
Notifications
You must be signed in to change notification settings - Fork 1.3k
get_file_content
Match Paths in Git Tree if Full Path Unknown
#650
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 our terms of service and privacy 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 the get_file_content
function to handle cases where the provided path isn't found by checking the Git tree for possible matching paths that end with the provided path. The implementation includes comprehensive error handling and path resolution improvements.
- Refactored Git reference resolution logic into a dedicated function for better maintainability
- Added fuzzy path matching using Git tree traversal when exact paths fail
- Enhanced test coverage for the new functionality with proper mocking
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
pkg/github/repositories.go | Refactored Git reference resolution, added fuzzy path matching via Git tree, and improved error handling in GetFileContents |
pkg/github/repositories_test.go | Added comprehensive unit tests for new functions and updated existing tests to include Git reference mocking |
rawOpts := &raw.ContentOpts{} | ||
|
||
if strings.HasPrefix(ref, "refs/pull/") { | ||
prNumber := strings.TrimSuffix(strings.TrimPrefix(ref, "refs/pull/"), "/head") | ||
if len(prNumber) > 0 { | ||
// fetch the PR from the API to get the latest commit and use SHA | ||
githubClient, err := getClient(ctx) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to get GitHub client: %w", err) | ||
} | ||
prNum, err := strconv.Atoi(prNumber) | ||
if err != nil { | ||
return nil, fmt.Errorf("invalid pull request number: %w", err) | ||
} | ||
pr, _, err := githubClient.PullRequests.Get(ctx, owner, repo, prNum) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to get pull request: %w", err) | ||
} | ||
sha = pr.GetHead().GetSHA() | ||
ref = "" | ||
} | ||
client, err := getClient(ctx) | ||
if err != nil { | ||
return mcp.NewToolResultError("failed to get GitHub client"), nil | ||
} | ||
|
||
rawOpts.SHA = sha | ||
rawOpts.Ref = ref |
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.
Reason for large diff: Removed this block - resolving refs is now handled by resolveGitReference
if err != nil { | ||
return mcp.NewToolResultError("failed to get file contents"), nil | ||
} | ||
defer func() { _ = resp.Body.Close() }() | ||
|
||
if resp.StatusCode != 200 { | ||
body, err := io.ReadAll(resp.Body) | ||
if err == nil && resp.StatusCode == http.StatusOK { | ||
defer func() { _ = resp.Body.Close() }() | ||
r, err := json.Marshal(dirContent) | ||
if err != nil { | ||
return mcp.NewToolResultError("failed to read response body"), nil | ||
return mcp.NewToolResultError("failed to marshal response"), nil | ||
} | ||
return mcp.NewToolResultError(fmt.Sprintf("failed to get file contents: %s", string(body))), nil |
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.
Reason for large diff: Inverted this logic - err or bad response status will now continue to the new stage of git tree path matching
Adds additional steps to the end of
get_file_content
so that if the provided path wasn't found, then we check the Git Tree for possible matching paths. A path matches if it ends with the provided path.Changes
get_file_content
so that if previous attempts to get the file/dir fail, then:path
arg. New funcfilterTree
.ref
andsha
into a functionresolveGitReference
. This still handles refs of the formrefs/tags/{tag}
,refs/heads/{branch}
,refs/pull/{pr_number}/head
and finds their commit SHA.filterTree
,resolveGitReference
.GetFileContent
to include mocking github client for resolving the git refs.Examples
File (Unknown Path) from Branch with 2 Matches
File (Unknown Path) from Last Release
File from Pull Request
File from Branch
TODO
ref
is not empty when we callclient.Git.GetTree
/
are handled properly byfilterPaths
. Have unit tests.