Skip to content

Improvements to push_files tool #1676

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.

Sign up for GitHub

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

Merged
almaleksia merged 7 commits into from
Jan 5, 2026
Merged

Improvements to push_files tool #1676

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6d3051d
Fallback to default branch in get_file_contents when main doesn't exist
almaleksia Dec 22, 2025
fda28d2
Addressing review comments
almaleksia Dec 23, 2025
bf28276
Improvements to push_files tool
almaleksia Dec 23, 2025
7ea39cc
Fixed copilot comments
almaleksia Dec 23, 2025
fc993d4
Merge branch 'main' of https://github.com/github/github-mcp-server in…
almaleksia Jan 5, 2026
da49890
Addressing review comments
almaleksia Jan 5, 2026
61748ab
Remove debug statement
almaleksia Jan 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
329 changes: 70 additions & 259 deletions pkg/github/repositories.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
ghErrors "github.com/github/github-mcp-server/pkg/errors"
"github.com/github/github-mcp-server/pkg/inventory"
"github.com/github/github-mcp-server/pkg/octicons"
"github.com/github/github-mcp-server/pkg/raw"
"github.com/github/github-mcp-server/pkg/translations"
"github.com/github/github-mcp-server/pkg/utils"
"github.com/google/go-github/v79/github"
Expand Down Expand Up @@ -1279,28 +1278,74 @@ func PushFiles(t translations.TranslationHelperFunc) inventory.ServerTool {
}

// Get the reference for the branch
var repositoryIsEmpty bool
var branchNotFound bool
ref, resp, err := client.Git.GetRef(ctx, owner, repo, "refs/heads/"+branch)
if err != nil {
return ghErrors.NewGitHubAPIErrorResponse(ctx,
"failed to get branch reference",
resp,
err,
), nil, nil
ghErr, isGhErr := err.(*github.ErrorResponse)
if isGhErr {
if ghErr.Response.StatusCode == http.StatusConflict && ghErr.Message == "Git Repository is empty." {
repositoryIsEmpty = true
} else if ghErr.Response.StatusCode == http.StatusNotFound {
branchNotFound = true
}
}

if !repositoryIsEmpty && !branchNotFound {
return ghErrors.NewGitHubAPIErrorResponse(ctx,
"failed to get branch reference",
resp,
err,
), nil, nil
}
}
// Only close resp if it's not nil and not an error case where resp might be nil
if resp != nil && resp.Body != nil {
defer func() { _ = resp.Body.Close() }()
}
defer func() { _ = resp.Body.Close() }()

// Get the commit object that the branch points to
baseCommit, resp, err := client.Git.GetCommit(ctx, owner, repo, *ref.Object.SHA)
if err != nil {
return ghErrors.NewGitHubAPIErrorResponse(ctx,
"failed to get base commit",
resp,
err,
), nil, nil
var baseCommit *github.Commit
if !repositoryIsEmpty {
if branchNotFound {
ref, err = createReferenceFromDefaultBranch(ctx, client, owner, repo, branch)
if err != nil {
return utils.NewToolResultError(fmt.Sprintf("failed to create branch from default: %v", err)), nil, nil
}
}

// Get the commit object that the branch points to
baseCommit, resp, err = client.Git.GetCommit(ctx, owner, repo, *ref.Object.SHA)
if err != nil {
return ghErrors.NewGitHubAPIErrorResponse(ctx,
"failed to get base commit",
resp,
err,
), nil, nil
}
if resp != nil && resp.Body != nil {
defer func() { _ = resp.Body.Close() }()
}
} else {
var base *github.Commit
// Repository is empty, need to initialize it first
ref, base, err = initializeRepository(ctx, client, owner, repo)
if err != nil {
return utils.NewToolResultError(fmt.Sprintf("failed to initialize repository: %v", err)), nil, nil
}

defaultBranch := strings.TrimPrefix(*ref.Ref, "refs/heads/")
if branch != defaultBranch {
// Create the requested branch from the default branch
ref, err = createReferenceFromDefaultBranch(ctx, client, owner, repo, branch)
if err != nil {
return utils.NewToolResultError(fmt.Sprintf("failed to create branch from default: %v", err)), nil, nil
}
}

baseCommit = base
}
defer func() { _ = resp.Body.Close() }()

// Create tree entries for all files
// Create tree entries for all files (or remaining files if empty repo)
var entries []*github.TreeEntry

for _, file := range filesObj {
Expand Down Expand Up @@ -1328,7 +1373,7 @@ func PushFiles(t translations.TranslationHelperFunc) inventory.ServerTool {
})
}

// Create a new tree with the file entries
// Create a new tree with the file entries (baseCommit is now guaranteed to exist)
newTree, resp, err := client.Git.CreateTree(ctx, owner, repo, *baseCommit.Tree.SHA, entries)
if err != nil {
return ghErrors.NewGitHubAPIErrorResponse(ctx,
Expand All @@ -1337,9 +1382,11 @@ func PushFiles(t translations.TranslationHelperFunc) inventory.ServerTool {
err,
), nil, nil
}
defer func() { _ = resp.Body.Close() }()
if resp != nil && resp.Body != nil {
defer func() { _ = resp.Body.Close() }()
}

// Create a new commit
// Create a new commit (baseCommit always has a value now)
commit := github.Commit{
Message: github.Ptr(message),
Tree: newTree,
Expand All @@ -1353,7 +1400,9 @@ func PushFiles(t translations.TranslationHelperFunc) inventory.ServerTool {
err,
), nil, nil
}
defer func() { _ = resp.Body.Close() }()
if resp != nil && resp.Body != nil {
defer func() { _ = resp.Body.Close() }()
}

// Update the reference to point to the new commit
ref.Object.SHA = newCommit.SHA
Expand Down Expand Up @@ -1770,244 +1819,6 @@ func GetReleaseByTag(t translations.TranslationHelperFunc) inventory.ServerTool
)
}

// matchFiles searches for files in the Git tree that match the given path.
// It's used when GetContents fails or returns unexpected results.
func matchFiles(ctx context.Context, client *github.Client, owner, repo, ref, path string, rawOpts *raw.ContentOpts, rawAPIResponseCode int) (*mcp.CallToolResult, any, error) {
// Step 1: Get Git Tree recursively
tree, response, err := client.Git.GetTree(ctx, owner, repo, ref, true)
if err != nil {
return ghErrors.NewGitHubAPIErrorResponse(ctx,
"failed to get git tree",
response,
err,
), nil, nil
}
defer func() { _ = response.Body.Close() }()

// Step 2: Filter tree for matching paths
const maxMatchingFiles = 3
matchingFiles := filterPaths(tree.Entries, path, maxMatchingFiles)
if len(matchingFiles) > 0 {
matchingFilesJSON, err := json.Marshal(matchingFiles)
if err != nil {
return utils.NewToolResultError(fmt.Sprintf("failed to marshal matching files: %s", err)), nil, nil
}
resolvedRefs, err := json.Marshal(rawOpts)
if err != nil {
return utils.NewToolResultError(fmt.Sprintf("failed to marshal resolved refs: %s", err)), nil, nil
}
if rawAPIResponseCode > 0 {
return utils.NewToolResultText(fmt.Sprintf("Resolved potential matches in the repository tree (resolved refs: %s, matching files: %s), but the content API returned an unexpected status code %d.", string(resolvedRefs), string(matchingFilesJSON), rawAPIResponseCode)), nil, nil
}
return utils.NewToolResultText(fmt.Sprintf("Resolved potential matches in the repository tree (resolved refs: %s, matching files: %s).", string(resolvedRefs), string(matchingFilesJSON))), nil, nil
}
return utils.NewToolResultError("Failed to get file contents. The path does not point to a file or directory, or the file does not exist in the repository."), nil, nil
}

// filterPaths filters the entries in a GitHub tree to find paths that
// match the given suffix.
// maxResults limits the number of results returned to first maxResults entries,
// a maxResults of -1 means no limit.
// It returns a slice of strings containing the matching paths.
// Directories are returned with a trailing slash.
func filterPaths(entries []*github.TreeEntry, path string, maxResults int) []string {
// Remove trailing slash for matching purposes, but flag whether we
// only want directories.
dirOnly := false
if strings.HasSuffix(path, "/") {
dirOnly = true
path = strings.TrimSuffix(path, "/")
}

matchedPaths := []string{}
for _, entry := range entries {
if len(matchedPaths) == maxResults {
break // Limit the number of results to maxResults
}
if dirOnly && entry.GetType() != "tree" {
continue // Skip non-directory entries if dirOnly is true
}
entryPath := entry.GetPath()
if entryPath == "" {
continue // Skip empty paths
}
if strings.HasSuffix(entryPath, path) {
if entry.GetType() == "tree" {
entryPath += "/" // Return directories with a trailing slash
}
matchedPaths = append(matchedPaths, entryPath)
}
}
return matchedPaths
}

// looksLikeSHA returns true if the string appears to be a Git commit SHA.
// A SHA is a 40-character hexadecimal string.
func looksLikeSHA(s string) bool {
if len(s) != 40 {
return false
}
for _, c := range s {
if (c < '0' || c > '9') && (c < 'a' || c > 'f') && (c < 'A' || c > 'F') {
return false
}
}
return true
}

// resolveGitReference takes a user-provided ref and sha and resolves them into a
// definitive commit SHA and its corresponding fully-qualified reference.
//
// The resolution logic follows a clear priority:
//
// 1. If a specific commit `sha` is provided, it takes precedence and is used directly,
// and all reference resolution is skipped.
//
// 1a. If `sha` is empty but `ref` looks like a commit SHA (40 hexadecimal characters),
// it is returned as-is without any API calls or reference resolution.
//
// 2. If no `sha` is provided and `ref` does not look like a SHA, the function resolves
// the `ref` string into a fully-qualified format (e.g., "refs/heads/main") by trying
// the following steps in order:
// a). **Empty Ref:** If `ref` is empty, the repository's default branch is used.
// b). **Fully-Qualified:** If `ref` already starts with "refs/", it's considered fully
// qualified and used as-is.
// c). **Partially-Qualified:** If `ref` starts with "heads/" or "tags/", it is
// prefixed with "refs/" to make it fully-qualified.
// d). **Short Name:** Otherwise, the `ref` is treated as a short name. The function
// first attempts to resolve it as a branch ("refs/heads/<ref>"). If that
// returns a 404 Not Found error, it then attempts to resolve it as a tag
// ("refs/tags/<ref>").
//
// 3. **Final Lookup:** Once a fully-qualified ref is determined, a final API call
// is made to fetch that reference's definitive commit SHA.
//
// Any unexpected (non-404) errors during the resolution process are returned
// immediately. All API errors are logged with rich context to aid diagnostics.
func resolveGitReference(ctx context.Context, githubClient *github.Client, owner, repo, ref, sha string) (*raw.ContentOpts, bool, error) {
// 1) If SHA explicitly provided, it's the highest priority.
if sha != "" {
return &raw.ContentOpts{Ref: "", SHA: sha}, false, nil
}

// 1a) If sha is empty but ref looks like a SHA, return it without changes
if looksLikeSHA(ref) {
return &raw.ContentOpts{Ref: "", SHA: ref}, false, nil
}

originalRef := ref // Keep original ref for clearer error messages down the line.

// 2) If no SHA is provided, we try to resolve the ref into a fully-qualified format.
var reference *github.Reference
var resp *github.Response
var err error
var fallbackUsed bool

switch {
case originalRef == "":
// 2a) If ref is empty, determine the default branch.
reference, err = resolveDefaultBranch(ctx, githubClient, owner, repo)
if err != nil {
return nil, false, err // Error is already wrapped in resolveDefaultBranch.
}
ref = reference.GetRef()
case strings.HasPrefix(originalRef, "refs/"):
// 2b) Already fully qualified. The reference will be fetched at the end.
case strings.HasPrefix(originalRef, "heads/") || strings.HasPrefix(originalRef, "tags/"):
// 2c) Partially qualified. Make it fully qualified.
ref = "refs/" + originalRef
default:
// 2d) It's a short name, so we try to resolve it to either a branch or a tag.
branchRef := "refs/heads/" + originalRef
reference, resp, err = githubClient.Git.GetRef(ctx, owner, repo, branchRef)

if err == nil {
ref = branchRef // It's a branch.
} else {
// The branch lookup failed. Check if it was a 404 Not Found error.
ghErr, isGhErr := err.(*github.ErrorResponse)
if isGhErr && ghErr.Response.StatusCode == http.StatusNotFound {
tagRef := "refs/tags/" + originalRef
reference, resp, err = githubClient.Git.GetRef(ctx, owner, repo, tagRef)
if err == nil {
ref = tagRef // It's a tag.
} else {
// The tag lookup also failed. Check if it was a 404 Not Found error.
ghErr2, isGhErr2 := err.(*github.ErrorResponse)
if isGhErr2 && ghErr2.Response.StatusCode == http.StatusNotFound {
if originalRef == "main" {
reference, err = resolveDefaultBranch(ctx, githubClient, owner, repo)
if err != nil {
return nil, false, err // Error is already wrapped in resolveDefaultBranch.
}
// Update ref to the actual default branch ref so the note can be generated
ref = reference.GetRef()
fallbackUsed = true
break
}
return nil, false, fmt.Errorf("could not resolve ref %q as a branch or a tag", originalRef)
}

// The tag lookup failed for a different reason.
_, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get reference (tag)", resp, err)
return nil, false, fmt.Errorf("failed to get reference for tag '%s': %w", originalRef, err)
}
} else {
// The branch lookup failed for a different reason.
_, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get reference (branch)", resp, err)
return nil, false, fmt.Errorf("failed to get reference for branch '%s': %w", originalRef, err)
}
}
}

if reference == nil {
reference, resp, err = githubClient.Git.GetRef(ctx, owner, repo, ref)
if err != nil {
if ref == "refs/heads/main" {
reference, err = resolveDefaultBranch(ctx, githubClient, owner, repo)
if err != nil {
return nil, false, err // Error is already wrapped in resolveDefaultBranch.
}
// Update ref to the actual default branch ref so the note can be generated
ref = reference.GetRef()
fallbackUsed = true
} else {
_, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get final reference", resp, err)
return nil, false, fmt.Errorf("failed to get final reference for %q: %w", ref, err)
}
}
}

sha = reference.GetObject().GetSHA()
return &raw.ContentOpts{Ref: ref, SHA: sha}, fallbackUsed, nil
}

func resolveDefaultBranch(ctx context.Context, githubClient *github.Client, owner, repo string) (*github.Reference, error) {
repoInfo, resp, err := githubClient.Repositories.Get(ctx, owner, repo)
if err != nil {
_, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get repository info", resp, err)
return nil, fmt.Errorf("failed to get repository info: %w", err)
}

if resp != nil && resp.Body != nil {
_ = resp.Body.Close()
}

defaultBranch := repoInfo.GetDefaultBranch()

defaultRef, resp, err := githubClient.Git.GetRef(ctx, owner, repo, "heads/"+defaultBranch)
if err != nil {
_, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get default branch reference", resp, err)
return nil, fmt.Errorf("failed to get default branch reference: %w", err)
}

if resp != nil && resp.Body != nil {
defer func() { _ = resp.Body.Close() }()
}

return defaultRef, nil
}

// ListStarredRepositories creates a tool to list starred repositories for the authenticated user or a specified user.
func ListStarredRepositories(t translations.TranslationHelperFunc) inventory.ServerTool {
return NewTool(
Expand Down
Loading