From a2212c06e6dfd8de9069e20ad7bfaf0137eded6f Mon Sep 17 00:00:00 2001 From: Dylan Pulver Date: Fri, 21 Aug 2026 00:41:32 +0300 Subject: [PATCH 1/2] fix(repos): give create_or_update_file callers a SHA they can actually get The create_or_update_file tool description and both of its SHA errors told the caller to run `git rev-parse :`. The caller is an MCP client talking to the GitHub API, and the same description tells it not to use this tool for local file operations, so it has no working tree to run that command against. Point the description at get_file_contents instead, which returns the blob SHA over the API. In the already-exists error the server has just fetched the file, so return that SHA directly rather than asking for a round trip. The stale-SHA error already interpolates the current SHA, so it only needed the impossible instruction removed. --- .../__toolsnaps__/create_or_update_file.snap | 2 +- pkg/github/repositories.go | 11 +++++------ pkg/github/repositories_test.go | 19 +++++++++++++++---- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/pkg/github/__toolsnaps__/create_or_update_file.snap b/pkg/github/__toolsnaps__/create_or_update_file.snap index 37bf3b46bf..272bc12438 100644 --- a/pkg/github/__toolsnaps__/create_or_update_file.snap +++ b/pkg/github/__toolsnaps__/create_or_update_file.snap @@ -4,7 +4,7 @@ "readOnlyHint": false, "title": "Create or update file" }, - "description": "Create or update a single file in a GitHub repository. \nIf updating, you should provide the SHA of the file you want to update. Use this tool to create or update a file in a GitHub repository remotely; do not use it for local file operations.\n\nIn order to obtain the SHA of original file version before updating, use the following git command:\ngit rev-parse \u003cbranch\u003e:\u003cpath to file\u003e\n\nSHA MUST be provided for existing file updates.\n", + "description": "Create or update a single file in a GitHub repository. \nIf updating, you should provide the SHA of the file you want to update. Use this tool to create or update a file in a GitHub repository remotely; do not use it for local file operations.\n\nTo obtain the SHA of the current file version before updating, call the get_file_contents tool for the same path and ref; it reports the blob SHA of the file it returns.\n\nSHA MUST be provided for existing file updates.\n", "inputSchema": { "properties": { "allow_symlink_write": { diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 446d7ca551..e308b81439 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -412,8 +412,7 @@ func CreateOrUpdateFile(t translations.TranslationHelperFunc) inventory.ServerTo Description: t("TOOL_CREATE_OR_UPDATE_FILE_DESCRIPTION", `Create or update a single file in a GitHub repository. If updating, you should provide the SHA of the file you want to update. Use this tool to create or update a file in a GitHub repository remotely; do not use it for local file operations. -In order to obtain the SHA of original file version before updating, use the following git command: -git rev-parse : +To obtain the SHA of the current file version before updating, call the get_file_contents tool for the same path and ref; it reports the blob SHA of the file it returns. SHA MUST be provided for existing file updates. `), @@ -549,8 +548,8 @@ SHA MUST be provided for existing file updates. if currentSHA != sha { return utils.NewToolResultError(fmt.Sprintf( "SHA mismatch: provided SHA %s is stale. Current file SHA is %s. "+ - "Pull the latest changes and use git rev-parse %s:%s to get the current SHA.", - sha, currentSHA, branch, path)), nil, nil + "Re-read the file with get_file_contents if you need its latest content, then retry with the sha parameter set to %s.", + sha, currentSHA, currentSHA)), nil, nil } if !allowSymlinkWrite { if existingFile.GetType() == "symlink" { @@ -594,8 +593,8 @@ SHA MUST be provided for existing file updates. // File exists but no SHA was provided - reject to prevent blind overwrites return utils.NewToolResultError(fmt.Sprintf( "File already exists at %s. You must provide the current file's SHA when updating. "+ - "Use git rev-parse %s:%s to get the blob SHA, then retry with the sha parameter.", - path, branch, path)), nil, nil + "The current SHA is %s; retry with the sha parameter set to that value.", + path, existingFile.GetSHA())), nil, nil } // If file not found, no previous SHA needed (new file creation) } diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index e45c3c4f47..9db9dea8c3 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -2058,6 +2058,8 @@ func Test_CreateOrUpdateFile(t *testing.T) { assert.Equal(t, "create_or_update_file", tool.Name) assert.NotEmpty(t, tool.Description) + assert.NotContains(t, tool.Description, "git rev-parse") + assert.Contains(t, tool.Description, "get_file_contents") assert.Contains(t, schema.Properties, "owner") assert.Contains(t, schema.Properties, "repo") assert.Contains(t, schema.Properties, "path") @@ -2468,8 +2470,11 @@ func Test_CreateOrUpdateFile(t *testing.T) { "branch": "main", "sha": "oldsha123456", }, - expectError: true, - expectedErrMsg: "SHA mismatch: provided SHA oldsha123456 is stale. Current file SHA is newsha999888", + expectError: true, + expectedErrMsgs: []string{ + "SHA mismatch: provided SHA oldsha123456 is stale. Current file SHA is newsha999888", + "retry with the sha parameter set to newsha999888", + }, expectedRequestCount: 1, }, { @@ -2531,8 +2536,11 @@ func Test_CreateOrUpdateFile(t *testing.T) { "message": "Update without SHA", "branch": "main", }, - expectError: true, - expectedErrMsg: "File already exists at docs/example.md", + expectError: true, + expectedErrMsgs: []string{ + "File already exists at docs/example.md", + "The current SHA is existing123; retry with the sha parameter set to that value.", + }, expectedRequestCount: 1, }, { @@ -2607,6 +2615,9 @@ func Test_CreateOrUpdateFile(t *testing.T) { for _, expectedErrMsg := range tc.expectedErrMsgs { assert.Contains(t, errorText.String(), expectedErrMsg) } + // The caller of this tool works over the API and has no working + // tree, so errors must never ask it to run a local git command. + assert.NotContains(t, errorText.String(), "git rev-parse") return } From 4e9f5194a5abeb99c5a92e2008ff41a1b3a2381a Mon Sep 17 00:00:00 2001 From: Dylan Pulver Date: Fri, 21 Aug 2026 14:15:13 +0300 Subject: [PATCH 2/2] fix(repos): stop disclosing the blob SHA when no sha was supplied The already-exists path is reached only when the caller sent no sha, so it has not read the file. Returning the current blob SHA there let it overwrite content it never saw on the next call, which is the race the SHA gate exists to prevent. Send the caller to get_file_contents for the path and ref instead, so obtaining the SHA still requires reading the file. The stale-SHA path is unaffected: the caller did supply a sha, and that message already reported the current SHA before this change. Only its recovery step moved off git rev-parse, and it is now imperative rather than conditional, since a stale SHA means the file definitely changed. Assert the already-exists error does not contain the blob SHA so the gate cannot be loosened again without a test failing. --- pkg/github/repositories.go | 10 ++++++---- pkg/github/repositories_test.go | 13 +++++++++++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index e308b81439..a2da0df91d 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -548,8 +548,9 @@ SHA MUST be provided for existing file updates. if currentSHA != sha { return utils.NewToolResultError(fmt.Sprintf( "SHA mismatch: provided SHA %s is stale. Current file SHA is %s. "+ - "Re-read the file with get_file_contents if you need its latest content, then retry with the sha parameter set to %s.", - sha, currentSHA, currentSHA)), nil, nil + "The file changed since you read it, so re-read it with get_file_contents for this path and ref, "+ + "rebuild your content against what it returns, and retry with the sha parameter set to the SHA that call reports.", + sha, currentSHA)), nil, nil } if !allowSymlinkWrite { if existingFile.GetType() == "symlink" { @@ -593,8 +594,9 @@ SHA MUST be provided for existing file updates. // File exists but no SHA was provided - reject to prevent blind overwrites return utils.NewToolResultError(fmt.Sprintf( "File already exists at %s. You must provide the current file's SHA when updating. "+ - "The current SHA is %s; retry with the sha parameter set to that value.", - path, existingFile.GetSHA())), nil, nil + "Call get_file_contents for this path and ref to read the file you are about to overwrite, "+ + "then retry with the sha parameter set to the blob SHA that call reports.", + path)), nil, nil } // If file not found, no previous SHA needed (new file creation) } diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 9db9dea8c3..72f7a82188 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -2138,6 +2138,7 @@ func Test_CreateOrUpdateFile(t *testing.T) { expectedContent *github.RepositoryContentResponse expectedErrMsg string expectedErrMsgs []string + unexpectedErrMsgs []string expectedRequestCount int }{ { @@ -2473,7 +2474,8 @@ func Test_CreateOrUpdateFile(t *testing.T) { expectError: true, expectedErrMsgs: []string{ "SHA mismatch: provided SHA oldsha123456 is stale. Current file SHA is newsha999888", - "retry with the sha parameter set to newsha999888", + "re-read it with get_file_contents", + "retry with the sha parameter set to the SHA that call reports", }, expectedRequestCount: 1, }, @@ -2539,8 +2541,12 @@ func Test_CreateOrUpdateFile(t *testing.T) { expectError: true, expectedErrMsgs: []string{ "File already exists at docs/example.md", - "The current SHA is existing123; retry with the sha parameter set to that value.", + "Call get_file_contents for this path and ref", + "retry with the sha parameter set to the blob SHA that call reports", }, + // A caller that never supplied a SHA has not read this file, so the + // error must not hand it one to overwrite with. + unexpectedErrMsgs: []string{"existing123"}, expectedRequestCount: 1, }, { @@ -2615,6 +2621,9 @@ func Test_CreateOrUpdateFile(t *testing.T) { for _, expectedErrMsg := range tc.expectedErrMsgs { assert.Contains(t, errorText.String(), expectedErrMsg) } + for _, unexpectedErrMsg := range tc.unexpectedErrMsgs { + assert.NotContains(t, errorText.String(), unexpectedErrMsg) + } // The caller of this tool works over the API and has no working // tree, so errors must never ask it to run a local git command. assert.NotContains(t, errorText.String(), "git rev-parse")