Skip to content

Commit a2212c0

Browse files
committed
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 <branch>:<path>`. 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.
1 parent 64a49f3 commit a2212c0

3 files changed

Lines changed: 21 additions & 11 deletions

File tree

pkg/github/__toolsnaps__/create_or_update_file.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"readOnlyHint": false,
55
"title": "Create or update file"
66
},
7-
"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",
7+
"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",
88
"inputSchema": {
99
"properties": {
1010
"allow_symlink_write": {

pkg/github/repositories.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -412,8 +412,7 @@ func CreateOrUpdateFile(t translations.TranslationHelperFunc) inventory.ServerTo
412412
Description: t("TOOL_CREATE_OR_UPDATE_FILE_DESCRIPTION", `Create or update a single file in a GitHub repository.
413413
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.
414414
415-
In order to obtain the SHA of original file version before updating, use the following git command:
416-
git rev-parse <branch>:<path to file>
415+
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.
417416
418417
SHA MUST be provided for existing file updates.
419418
`),
@@ -549,8 +548,8 @@ SHA MUST be provided for existing file updates.
549548
if currentSHA != sha {
550549
return utils.NewToolResultError(fmt.Sprintf(
551550
"SHA mismatch: provided SHA %s is stale. Current file SHA is %s. "+
552-
"Pull the latest changes and use git rev-parse %s:%s to get the current SHA.",
553-
sha, currentSHA, branch, path)), nil, nil
551+
"Re-read the file with get_file_contents if you need its latest content, then retry with the sha parameter set to %s.",
552+
sha, currentSHA, currentSHA)), nil, nil
554553
}
555554
if !allowSymlinkWrite {
556555
if existingFile.GetType() == "symlink" {
@@ -594,8 +593,8 @@ SHA MUST be provided for existing file updates.
594593
// File exists but no SHA was provided - reject to prevent blind overwrites
595594
return utils.NewToolResultError(fmt.Sprintf(
596595
"File already exists at %s. You must provide the current file's SHA when updating. "+
597-
"Use git rev-parse %s:%s to get the blob SHA, then retry with the sha parameter.",
598-
path, branch, path)), nil, nil
596+
"The current SHA is %s; retry with the sha parameter set to that value.",
597+
path, existingFile.GetSHA())), nil, nil
599598
}
600599
// If file not found, no previous SHA needed (new file creation)
601600
}

pkg/github/repositories_test.go

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2058,6 +2058,8 @@ func Test_CreateOrUpdateFile(t *testing.T) {
20582058

20592059
assert.Equal(t, "create_or_update_file", tool.Name)
20602060
assert.NotEmpty(t, tool.Description)
2061+
assert.NotContains(t, tool.Description, "git rev-parse")
2062+
assert.Contains(t, tool.Description, "get_file_contents")
20612063
assert.Contains(t, schema.Properties, "owner")
20622064
assert.Contains(t, schema.Properties, "repo")
20632065
assert.Contains(t, schema.Properties, "path")
@@ -2468,8 +2470,11 @@ func Test_CreateOrUpdateFile(t *testing.T) {
24682470
"branch": "main",
24692471
"sha": "oldsha123456",
24702472
},
2471-
expectError: true,
2472-
expectedErrMsg: "SHA mismatch: provided SHA oldsha123456 is stale. Current file SHA is newsha999888",
2473+
expectError: true,
2474+
expectedErrMsgs: []string{
2475+
"SHA mismatch: provided SHA oldsha123456 is stale. Current file SHA is newsha999888",
2476+
"retry with the sha parameter set to newsha999888",
2477+
},
24732478
expectedRequestCount: 1,
24742479
},
24752480
{
@@ -2531,8 +2536,11 @@ func Test_CreateOrUpdateFile(t *testing.T) {
25312536
"message": "Update without SHA",
25322537
"branch": "main",
25332538
},
2534-
expectError: true,
2535-
expectedErrMsg: "File already exists at docs/example.md",
2539+
expectError: true,
2540+
expectedErrMsgs: []string{
2541+
"File already exists at docs/example.md",
2542+
"The current SHA is existing123; retry with the sha parameter set to that value.",
2543+
},
25362544
expectedRequestCount: 1,
25372545
},
25382546
{
@@ -2607,6 +2615,9 @@ func Test_CreateOrUpdateFile(t *testing.T) {
26072615
for _, expectedErrMsg := range tc.expectedErrMsgs {
26082616
assert.Contains(t, errorText.String(), expectedErrMsg)
26092617
}
2618+
// The caller of this tool works over the API and has no working
2619+
// tree, so errors must never ask it to run a local git command.
2620+
assert.NotContains(t, errorText.String(), "git rev-parse")
26102621
return
26112622
}
26122623

0 commit comments

Comments
 (0)