Always include SHA in get_file_contents responses #676

yonaka15 · 2025-07-12T16:06:35Z

Summary

This PR provides a simpler solution to #595 by always including SHA information in the get_file_contents tool responses without requiring any parameter changes.

Unlike #605 which adds an include_sha parameter, this approach transparently enhances all responses with SHA information while preserving the existing MCP server behavior.

Approach

The implementation adds a preliminary Contents API call to retrieve SHA information before fetching the actual content:

Call Contents API to get file metadata (including SHA)
Call Raw Content API to get the actual content (as before)
Include SHA in both the resource URI and success message

This dual-API approach ensures:

Text files continue to return raw text content
Binary files continue to return base64-encoded content
SHA information is always available for create_or_update_file operations

Changes

Modified pkg/github/repositories.go to add Contents API call before Raw Content API
Updated tests in pkg/github/repositories_test.go to include Contents API mocks
No changes to tool schema or parameters - fully backward compatible

Benefits over #605

No parameter required - SHA is always included automatically
Simpler for users (AI) - No need to remember to set include_sha=true
Better default behavior - Addresses the core issue directly
Minimal performance impact - The additional API call is lightweight

Testing

Verified with both unit tests and MCP Inspector against real repositories:

Text files return raw content with SHA ✅
Binary files return base64 content with SHA ✅
Directories return JSON with SHA for each item ✅
Graceful fallback when Contents API fails ✅

Closes #595

…ub#595) Enhance get_file_contents to include SHA information without changing the existing MCP server response format. Changes: - Add Contents API call to retrieve SHA before fetching raw content - Include SHA in resourceURI (repo://owner/repo/sha/{SHA}/contents/path) - Add SHA to success messages - Update tests to verify SHA inclusion - Maintain original behavior: text files return raw text, binaries return base64 This preserves backward compatibility while providing SHA information for better file versioning support. Closes github#595

Copilot

Pull Request Overview

This PR enhances the get_file_contents tool to always include SHA information in responses by adding a preliminary Contents API call before fetching raw content. This provides a simpler, backward-compatible solution that automatically includes SHA data without requiring parameter changes.

Key changes:

Added Contents API call to retrieve file metadata (including SHA) before fetching raw content
Updated resource URI format to include SHA instead of branch reference
Enhanced success messages to include SHA information

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
pkg/github/repositories.go	Added Contents API call to retrieve SHA before raw content fetch and updated URI/message formatting
pkg/github/repositories_test.go	Updated test mocks to include Contents API responses and adjusted expected URIs to use SHA format

Comments suppressed due to low confidence (1)

pkg/github/repositories.go:513

The variable name 'errContents' is inconsistent with Go naming conventions. It should be 'err' or follow the existing pattern used elsewhere in the function.

				fileContent, _, respContents, errContents := client.Repositories.GetContents(ctx, owner, repo, path, opts)

pkg/github/repositories.go

Ensure response body is properly closed even when an error occurs by moving the defer statement before the error check. This prevents potential resource leaks when the Contents API returns an error with a non-nil response. Changes: - Move defer respContents.Body.Close() before error checking - Rename errContents to err for consistency - Add nil check for respContents before attempting to close body This follows Go best practices for handling HTTP responses and prevents potential goroutine/memory leaks.

LuluBeatson

Thank you so much @yonaka15 for reworking this based on our feedback. It's great to see this working!

Screenshot

get_file_contents being used before create_or_update_file to get the blob SHA of file to update

My only minor comments (which I will action):

Changing the resource URI is not necessary. I will revert this.
Using the contents API to get the file's blob SHA is not optimal. I will look into using a GraphQL query to get just the SHA.

Thank you again, I'm excited to get your work merged :)

…nventions

…-include-sh

LuluBeatson · 2025-07-15T15:40:49Z

Examples

File from Default

Get repositories.go from github/github-mcp-server

File from Branch

Get repositories.go from the add-pagination-graphql branch of github/github-mcp-server

File from Commit

Get repositories.go from the f47e5542e419e6d0bea8d3b54849ae5613344af0 commit of github/github-mcp-server

File from PR

Get repositories.go from PR 676 of github/github-mcp-server

Dir from Default (unchanged by PR)

get the pkg/github dir from github/github-mcp-server

Image from Default

Get 10mb.png from lulubeatson/test-image

LuluBeatson · 2025-07-16T22:16:24Z

I will look into using a GraphQL query to get just the SHA.

The GraphQL query was slow so I've stuck with your original method using the contents method of the GitHub API.

GraphQL (~350ms)

You can run the following query in https://docs.github.com/en/graphql/overview/explorer and see the response time in the network panel

query {
  repository(owner: "github", name:"github-mcp-server") {
    object(expression: "main:README.md") {
      ... on Blob {
        oid
      }
    }
  }
}

Contents (~30ms)

https://api.github.com/repos/github/github-mcp-server/contents/README.md

https://api.github.com/repos/{owner}/{repo}/contents/{path}

Docs: https://docs.github.com/en/rest/repos/contents?apiVersion=2022-11-28#get-repository-content

Raw (~30ms)

https://raw.githubusercontent.com/github/github-mcp-server/d15026b0eb2a2e5d3265a2601798ab28017dc719/README.md

https://raw.githubusercontent.com/{owner}/{repo}/{sha}/{path}

tommaso-moro

Nice one! The approach to contents api vs raw api vs graphql makes sense to me!

@tonytrg

* Create 'add sub-issue' tool * Fix hardcoded API host * Create 'list sub-issues' tool * Create 'remove sub-issue' tool * Fix Test_GetIssue mock data - add missing User field The assertion was already checking User.Login but the mock was incomplete * Create 'reprioritize sub-issue' tool * fixes * use go github pck to add sub-issues * Update to use go github package * update description * update to use go github v73 * lint, docs * refactor: tests to use go-github-mock * add toolsnaps * make RemoveSubIssue use NewGitHubAPIErrorResponse, update docstring * Always include SHA in get_file_contents responses (#676) * fix: Add SHA to get_file_contents while preserving MCP behavior (#595) Enhance get_file_contents to include SHA information without changing the existing MCP server response format. Changes: - Add Contents API call to retrieve SHA before fetching raw content - Include SHA in resourceURI (repo://owner/repo/sha/{SHA}/contents/path) - Add SHA to success messages - Update tests to verify SHA inclusion - Maintain original behavior: text files return raw text, binaries return base64 This preserves backward compatibility while providing SHA information for better file versioning support. Closes #595 * fix: Improve error handling for Contents API response Ensure response body is properly closed even when an error occurs by moving the defer statement before the error check. This prevents potential resource leaks when the Contents API returns an error with a non-nil response. Changes: - Move defer respContents.Body.Close() before error checking - Rename errContents to err for consistency - Add nil check for respContents before attempting to close body This follows Go best practices for handling HTTP responses and prevents potential goroutine/memory leaks. * revert changes to resource URI * use GraphQL API to get file SHA * refactor: mock GQL client instead of getFileSHA function to follow conventions * lint * revert GraphQL --------- Co-authored-by: LuluBeatson <lulubeatson@github.com> * Reorganize README, add dedicated install guides, include policies and governance info for the github server (#695) * Refactor README and add host installation guides, governance docs - Reorganized README for clarity and navigation - Added dedicated installation guides for Claude, Cursor, Windsurf, JetBrains, and more - Clarified contribution guidelines and approval criteria - Added policies and governance documentation * Update README.md * Update README with configuration section for remote GitHub MCP Server * Update MCP access policy description in README Removing coding agent from the policy note, as the GitHub server is unaffected by this policy * Update configuration steps for GitHub Copilot in JetBrains IDEs... ...to reflect changes in accessing settings and configuring MCP. * Update install-other-copilot-ides.md * Update Eclipse MCP support version and configuration steps... ...for GitHub Copilot plugin in installation guide. * Update docs/installation-guides/install-cursor.md * Update docs/installation-guides/install-windsurf.md * Apply suggestion from @tonytrg * Apply suggestion from @tonytrg * Apply suggestion from @tonytrg * Apply suggestion from @tonytrg * Apply suggestion from @tonytrg * Apply suggestion from @tonytrg * Apply suggestion from @tonytrg --------- Co-authored-by: Tony Truong <tonytrg@github.com> * fix: shorten long tool name for adding pr review comments (#697) * shorten tool name * update function name to match tool name * adjust wording of descriptions * Update installation guide for GitHub MCP Server (#699) * Update installation guide for GitHub MCP Server Removed reference to GitHub.com in the installation guide. The GitHub server is available to Coding Agent by default, without installation needed. * Rename section to 'Install in Other MCP Hosts' Updating title for consistency and adding a link to the "other Copilot IDEs" install guide. * Revise installation guide for Cursor MCP setup Updated installation guide for Cursor with steps clarified, remote server installation, and one-click install deeplinks to open Cursor and add the github server to the config file. * fix: make mcpcurl support "integer" type (#688) - FYI:https://json-schema.org/understanding-json-schema/reference/numeric#integer * Added installation instructions for mcpcurl (#719) * Added installation instructions for mcpcurl * Update cmd/mcpcurl/README.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Add pagination support to GraphQL-based tools (#683) * initial pagination for `ListDiscussions` * redo category id var cast * add GraphQL pagination support for discussion comments and categories * remove pageinfo returns * fix out ref for linter * update docs * move to unified pagination for consensus on params * update docs * refactor pagination handling * update docs * linter fix * conv rest to gql params for safe lint * add nolint * add error handling for perPage value in ToGraphQLParams * refactor pagination error handling * unified params for rest andn graphql and rennamed to be uniform for golang * add 'after' for pagination * update docs * Update pkg/github/discussions.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update pkg/github/discussions.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update pkg/github/discussions_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * update default page size const * reduce default pagination size from 100 to 30 in discussion tests * update pagination for reverse and total * update pagination to remove from discussions * updated README * improve the `ToGraphQLParams` function --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * shorten param descriptions * fix: resp nil check in error handling in RemoveSubIssue function --------- Co-authored-by: LuluBeatson <lulubeatson@github.com> Co-authored-by: tommaso-moro <tommaso-moro@github.com>

Copilot AI review requested due to automatic review settings July 12, 2025 16:06

yonaka15 requested a review from a team as a code owner July 12, 2025 16:06

Copilot AI reviewed Jul 12, 2025

View reviewed changes

pkg/github/repositories.go Outdated Show resolved Hide resolved

pkg/github/repositories.go Outdated Show resolved Hide resolved

yonaka15 changed the title ~~fix: Always include SHA in get_file_contents responses (#595)~~ fix: Always include SHA in get_file_contents responses Jul 12, 2025

yonaka15 mentioned this pull request Jul 12, 2025

feat: add include_sha parameter to get_file_contents tool #605

Closed

yonaka15 changed the title ~~fix: Always include SHA in get_file_contents responses~~ Always include SHA in get_file_contents responses Jul 12, 2025

revert changes to resource URI

e650bb0

LuluBeatson reviewed Jul 15, 2025

View reviewed changes

LuluBeatson added 4 commits July 15, 2025 16:15

use GraphQL API to get file SHA

f1f225d

refactor: mock GQL client instead of getFileSHA function to follow co…

505f361

…nventions

lint

af760f2

Merge remote-tracking branch 'origin/main' into fix/get-file-contents…

db04b21

…-include-sh

revert GraphQL

d1b6c32

LuluBeatson requested a review from a team July 17, 2025 08:32

tommaso-moro approved these changes Jul 17, 2025

View reviewed changes

LuluBeatson merged commit 0568187 into github:main Jul 17, 2025
10 checks passed

LuluBeatson mentioned this pull request Jul 17, 2025

how does create_update_files get sha when get_file_content dose not return this parameter? #691

Closed

BrewTestBot mentioned this pull request Jul 18, 2025

github-mcp-server 0.8.0 Homebrew/homebrew-core#230514

Merged

yonaka15 deleted the fix/get-file-contents-include-sh branch July 22, 2025 01:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Always include SHA in get_file_contents responses #676

Always include SHA in get_file_contents responses #676

Uh oh!

yonaka15 commented Jul 12, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

LuluBeatson left a comment •

edited

Loading

Uh oh!

LuluBeatson commented Jul 15, 2025 •

edited

Loading

Uh oh!

LuluBeatson commented Jul 16, 2025

Uh oh!

tommaso-moro left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Always include SHA in get_file_contents responses #676

Always include SHA in get_file_contents responses #676

Uh oh!

Conversation

yonaka15 commented Jul 12, 2025

Summary

Approach

Changes

Benefits over #605

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

LuluBeatson left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LuluBeatson commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Examples

Uh oh!

LuluBeatson commented Jul 16, 2025

Uh oh!

tommaso-moro left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

LuluBeatson left a comment •

edited

Loading

LuluBeatson commented Jul 15, 2025 •

edited

Loading

tommaso-moro left a comment •

edited

Loading