Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitafe34d8

Browse files
authored
Fix create_or_update SHA-related failures (#1621)
* Add repos toolset instructions* 1. Make sha optional (if not supplied - GetContents is used to retrieve original sha)2. Instruct LLM to supply actual sha using git command and move instructions to tool description.* Update toolsnaps and docs* Better error handling* Add tests* Clearing resources in time* Addressing review comments && checking etag* Test fixes
1 parent5a4338c commitafe34d8

File tree

4 files changed

+260
-6
lines changed

4 files changed

+260
-6
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1061,7 +1061,7 @@ The following sets of tools are available:
10611061
-`owner`: Repository owner (username or organization) (string, required)
10621062
-`path`: Path where to create/update the file (string, required)
10631063
-`repo`: Repository name (string, required)
1064-
-`sha`:Required if updating an existing file.The blob SHA of the file being replaced. (string, optional)
1064+
-`sha`: The blob SHA of the file being replaced. (string, optional)
10651065

10661066
-**create_repository** - Create repository
10671067
-`autoInit`: Initialize with README (boolean, optional)

‎pkg/github/__toolsnaps__/create_or_update_file.snap‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"annotations": {
33
"title":"Create or update file"
44
},
5-
"description":"Create or update a single file in a GitHub repository.If updating, youmust 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.",
5+
"description":"Create or update a single file in a GitHub repository.\nIf updating, youshould 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 ls-tree HEAD\u003cpath to file\u003e\n\nIf the SHA is not provided, the tool will attempt to acquire it by fetching the current file contents from the repository, which may lead to rewriting latest committed changes if the file has changed since last retrieval.\n",
66
"inputSchema": {
77
"type":"object",
88
"required": [
@@ -40,7 +40,7 @@
4040
},
4141
"sha": {
4242
"type":"string",
43-
"description":"Required if updating an existing file.The blob SHA of the file being replaced."
43+
"description":"The blob SHA of the file being replaced."
4444
}
4545
}
4646
},

‎pkg/github/repositories.go‎

Lines changed: 75 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -310,8 +310,15 @@ func ListBranches(getClient GetClientFn, t translations.TranslationHelperFunc) (
310310
// CreateOrUpdateFile creates a tool to create or update a file in a GitHub repository.
311311
funcCreateOrUpdateFile(getClientGetClientFn,t translations.TranslationHelperFunc) (mcp.Tool, mcp.ToolHandlerFor[map[string]any,any]) {
312312
tool:= mcp.Tool{
313-
Name:"create_or_update_file",
314-
Description:t("TOOL_CREATE_OR_UPDATE_FILE_DESCRIPTION","Create or update a single file in a GitHub repository. If updating, you must 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."),
313+
Name:"create_or_update_file",
314+
Description:t("TOOL_CREATE_OR_UPDATE_FILE_DESCRIPTION",`Create or update a single file in a GitHub repository.
315+
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.
316+
317+
In order to obtain the SHA of original file version before updating, use the following git command:
318+
git ls-tree HEAD <path to file>
319+
320+
If the SHA is not provided, the tool will attempt to acquire it by fetching the current file contents from the repository, which may lead to rewriting latest committed changes if the file has changed since last retrieval.
321+
`),
315322
Annotations:&mcp.ToolAnnotations{
316323
Title:t("TOOL_CREATE_OR_UPDATE_FILE_USER_TITLE","Create or update file"),
317324
ReadOnlyHint:false,
@@ -345,7 +352,7 @@ func CreateOrUpdateFile(getClient GetClientFn, t translations.TranslationHelperF
345352
},
346353
"sha": {
347354
Type:"string",
348-
Description:"Required if updating an existing file.The blob SHA of the file being replaced.",
355+
Description:"The blob SHA of the file being replaced.",
349356
},
350357
},
351358
Required: []string{"owner","repo","path","content","message","branch"},
@@ -404,6 +411,58 @@ func CreateOrUpdateFile(getClient GetClientFn, t translations.TranslationHelperF
404411
}
405412

406413
path=strings.TrimPrefix(path,"/")
414+
415+
// SHA validation using conditional HEAD request (efficient - no body transfer)
416+
varpreviousSHAstring
417+
contentURL:=fmt.Sprintf("repos/%s/%s/contents/%s",owner,repo,url.PathEscape(path))
418+
ifbranch!="" {
419+
contentURL+="?ref="+url.QueryEscape(branch)
420+
}
421+
422+
ifsha!="" {
423+
// User provided SHA - validate it's still current
424+
req,err:=client.NewRequest("HEAD",contentURL,nil)
425+
iferr==nil {
426+
req.Header.Set("If-None-Match",fmt.Sprintf(`"%s"`,sha))
427+
resp,_:=client.Do(ctx,req,nil)
428+
ifresp!=nil {
429+
deferresp.Body.Close()
430+
431+
switchresp.StatusCode {
432+
casehttp.StatusNotModified:
433+
// SHA matches current - proceed
434+
opts.SHA=github.Ptr(sha)
435+
casehttp.StatusOK:
436+
// SHA is stale - reject with current SHA so user can check diff
437+
currentSHA:=strings.Trim(resp.Header.Get("ETag"),`"`)
438+
returnutils.NewToolResultError(fmt.Sprintf(
439+
"SHA mismatch: provided SHA %s is stale. Current file SHA is %s. "+
440+
"Use get_file_contents or compare commits to review changes before updating.",
441+
sha,currentSHA)),nil,nil
442+
casehttp.StatusNotFound:
443+
// File doesn't exist - this is a create, ignore provided SHA
444+
}
445+
}
446+
}
447+
}else {
448+
// No SHA provided - check if file exists to warn about blind update
449+
req,err:=client.NewRequest("HEAD",contentURL,nil)
450+
iferr==nil {
451+
resp,_:=client.Do(ctx,req,nil)
452+
ifresp!=nil {
453+
deferresp.Body.Close()
454+
ifresp.StatusCode==http.StatusOK {
455+
previousSHA=strings.Trim(resp.Header.Get("ETag"),`"`)
456+
}
457+
// 404 = new file, no previous SHA needed
458+
}
459+
}
460+
}
461+
462+
ifpreviousSHA!="" {
463+
opts.SHA=github.Ptr(previousSHA)
464+
}
465+
407466
fileContent,resp,err:=client.Repositories.CreateFile(ctx,owner,repo,path,opts)
408467
iferr!=nil {
409468
returnghErrors.NewGitHubAPIErrorResponse(ctx,
@@ -427,6 +486,19 @@ func CreateOrUpdateFile(getClient GetClientFn, t translations.TranslationHelperF
427486
returnnil,nil,fmt.Errorf("failed to marshal response: %w",err)
428487
}
429488

489+
// Warn if file was updated without SHA validation (blind update)
490+
ifsha==""&&previousSHA!="" {
491+
returnutils.NewToolResultText(fmt.Sprintf(
492+
"Warning: File updated without SHA validation. Previous file SHA was %s. "+
493+
`Verify no unintended changes were overwritten:
494+
1. Extract the SHA of the local version using git ls-tree HEAD %s.
495+
2. Compare with the previous SHA above.
496+
3. Revert changes if shas do not match.
497+
498+
%s`,
499+
previousSHA,path,string(r))),nil,nil
500+
}
501+
430502
returnutils.NewToolResultText(string(r)),nil,nil
431503
})
432504

‎pkg/github/repositories_test.go‎

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,6 +1121,182 @@ func Test_CreateOrUpdateFile(t *testing.T) {
11211121
expectError:true,
11221122
expectedErrMsg:"failed to create/update file",
11231123
},
1124+
{
1125+
name:"sha validation - current sha matches (304 Not Modified)",
1126+
mockedClient:mock.NewMockedHTTPClient(
1127+
mock.WithRequestMatchHandler(
1128+
mock.EndpointPattern{
1129+
Pattern:"/repos/owner/repo/contents/docs/example.md",
1130+
Method:"HEAD",
1131+
},
1132+
http.HandlerFunc(func(w http.ResponseWriter,req*http.Request) {
1133+
// Verify If-None-Match header is set correctly
1134+
ifNoneMatch:=req.Header.Get("If-None-Match")
1135+
ififNoneMatch==`"abc123def456"` {
1136+
w.WriteHeader(http.StatusNotModified)
1137+
}else {
1138+
w.WriteHeader(http.StatusOK)
1139+
w.Header().Set("ETag",`"abc123def456"`)
1140+
}
1141+
}),
1142+
),
1143+
mock.WithRequestMatchHandler(
1144+
mock.PutReposContentsByOwnerByRepoByPath,
1145+
expectRequestBody(t,map[string]interface{}{
1146+
"message":"Update example file",
1147+
"content":"IyBVcGRhdGVkIEV4YW1wbGUKClRoaXMgZmlsZSBoYXMgYmVlbiB1cGRhdGVkLg==",
1148+
"branch":"main",
1149+
"sha":"abc123def456",
1150+
}).andThen(
1151+
mockResponse(t,http.StatusOK,mockFileResponse),
1152+
),
1153+
),
1154+
),
1155+
requestArgs:map[string]interface{}{
1156+
"owner":"owner",
1157+
"repo":"repo",
1158+
"path":"docs/example.md",
1159+
"content":"# Updated Example\n\nThis file has been updated.",
1160+
"message":"Update example file",
1161+
"branch":"main",
1162+
"sha":"abc123def456",
1163+
},
1164+
expectError:false,
1165+
expectedContent:mockFileResponse,
1166+
},
1167+
{
1168+
name:"sha validation - stale sha detected (200 OK with different ETag)",
1169+
mockedClient:mock.NewMockedHTTPClient(
1170+
mock.WithRequestMatchHandler(
1171+
mock.EndpointPattern{
1172+
Pattern:"/repos/owner/repo/contents/docs/example.md",
1173+
Method:"HEAD",
1174+
},
1175+
http.HandlerFunc(func(w http.ResponseWriter,_*http.Request) {
1176+
// SHA doesn't match - return 200 with current ETag
1177+
w.Header().Set("ETag",`"newsha999888"`)
1178+
w.WriteHeader(http.StatusOK)
1179+
}),
1180+
),
1181+
),
1182+
requestArgs:map[string]interface{}{
1183+
"owner":"owner",
1184+
"repo":"repo",
1185+
"path":"docs/example.md",
1186+
"content":"# Updated Example\n\nThis file has been updated.",
1187+
"message":"Update example file",
1188+
"branch":"main",
1189+
"sha":"oldsha123456",
1190+
},
1191+
expectError:true,
1192+
expectedErrMsg:"SHA mismatch: provided SHA oldsha123456 is stale. Current file SHA is newsha999888",
1193+
},
1194+
{
1195+
name:"sha validation - file doesn't exist (404), proceed with create",
1196+
mockedClient:mock.NewMockedHTTPClient(
1197+
mock.WithRequestMatchHandler(
1198+
mock.EndpointPattern{
1199+
Pattern:"/repos/owner/repo/contents/docs/example.md",
1200+
Method:"HEAD",
1201+
},
1202+
http.HandlerFunc(func(w http.ResponseWriter,_*http.Request) {
1203+
w.WriteHeader(http.StatusNotFound)
1204+
}),
1205+
),
1206+
mock.WithRequestMatchHandler(
1207+
mock.PutReposContentsByOwnerByRepoByPath,
1208+
expectRequestBody(t,map[string]interface{}{
1209+
"message":"Create new file",
1210+
"content":"IyBOZXcgRmlsZQoKVGhpcyBpcyBhIG5ldyBmaWxlLg==",
1211+
"branch":"main",
1212+
"sha":"ignoredsha",// SHA is sent but GitHub API ignores it for new files
1213+
}).andThen(
1214+
mockResponse(t,http.StatusCreated,mockFileResponse),
1215+
),
1216+
),
1217+
),
1218+
requestArgs:map[string]interface{}{
1219+
"owner":"owner",
1220+
"repo":"repo",
1221+
"path":"docs/example.md",
1222+
"content":"# New File\n\nThis is a new file.",
1223+
"message":"Create new file",
1224+
"branch":"main",
1225+
"sha":"ignoredsha",
1226+
},
1227+
expectError:false,
1228+
expectedContent:mockFileResponse,
1229+
},
1230+
{
1231+
name:"no sha provided - file exists, returns warning",
1232+
mockedClient:mock.NewMockedHTTPClient(
1233+
mock.WithRequestMatchHandler(
1234+
mock.EndpointPattern{
1235+
Pattern:"/repos/owner/repo/contents/docs/example.md",
1236+
Method:"HEAD",
1237+
},
1238+
http.HandlerFunc(func(w http.ResponseWriter,_*http.Request) {
1239+
w.Header().Set("ETag",`"existing123"`)
1240+
w.WriteHeader(http.StatusOK)
1241+
}),
1242+
),
1243+
mock.WithRequestMatchHandler(
1244+
mock.PutReposContentsByOwnerByRepoByPath,
1245+
expectRequestBody(t,map[string]interface{}{
1246+
"message":"Update without SHA",
1247+
"content":"IyBVcGRhdGVkCgpVcGRhdGVkIHdpdGhvdXQgU0hBLg==",
1248+
"branch":"main",
1249+
"sha":"existing123",// SHA is automatically added from ETag
1250+
}).andThen(
1251+
mockResponse(t,http.StatusOK,mockFileResponse),
1252+
),
1253+
),
1254+
),
1255+
requestArgs:map[string]interface{}{
1256+
"owner":"owner",
1257+
"repo":"repo",
1258+
"path":"docs/example.md",
1259+
"content":"# Updated\n\nUpdated without SHA.",
1260+
"message":"Update without SHA",
1261+
"branch":"main",
1262+
},
1263+
expectError:false,
1264+
expectedErrMsg:"Warning: File updated without SHA validation. Previous file SHA was existing123",
1265+
},
1266+
{
1267+
name:"no sha provided - file doesn't exist, no warning",
1268+
mockedClient:mock.NewMockedHTTPClient(
1269+
mock.WithRequestMatchHandler(
1270+
mock.EndpointPattern{
1271+
Pattern:"/repos/owner/repo/contents/docs/example.md",
1272+
Method:"HEAD",
1273+
},
1274+
http.HandlerFunc(func(w http.ResponseWriter,_*http.Request) {
1275+
w.WriteHeader(http.StatusNotFound)
1276+
}),
1277+
),
1278+
mock.WithRequestMatchHandler(
1279+
mock.PutReposContentsByOwnerByRepoByPath,
1280+
expectRequestBody(t,map[string]interface{}{
1281+
"message":"Create new file",
1282+
"content":"IyBOZXcgRmlsZQoKQ3JlYXRlZCB3aXRob3V0IFNIQQ==",
1283+
"branch":"main",
1284+
}).andThen(
1285+
mockResponse(t,http.StatusCreated,mockFileResponse),
1286+
),
1287+
),
1288+
),
1289+
requestArgs:map[string]interface{}{
1290+
"owner":"owner",
1291+
"repo":"repo",
1292+
"path":"docs/example.md",
1293+
"content":"# New File\n\nCreated without SHA",
1294+
"message":"Create new file",
1295+
"branch":"main",
1296+
},
1297+
expectError:false,
1298+
expectedContent:mockFileResponse,
1299+
},
11241300
}
11251301

11261302
for_,tc:=rangetests {
@@ -1150,6 +1326,12 @@ func Test_CreateOrUpdateFile(t *testing.T) {
11501326
// Parse the result and get the text content if no error
11511327
textContent:=getTextResult(t,result)
11521328

1329+
// If expectedErrMsg is set (but expectError is false), this is a warning case
1330+
iftc.expectedErrMsg!="" {
1331+
assert.Contains(t,textContent.Text,tc.expectedErrMsg)
1332+
return
1333+
}
1334+
11531335
// Unmarshal and verify the result
11541336
varreturnedContent github.RepositoryContentResponse
11551337
err=json.Unmarshal([]byte(textContent.Text),&returnedContent)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp