Skip to content

Commit e6201f4

Browse files
ashwin-antashwin-ant
committed
switch to approach that does commit signing
1 parent 5c4181a commit e6201f4

File tree

2 files changed

+143
-74
lines changed

2 files changed

+143
-74
lines changed

‎pkg/github/repositories.go‎

Lines changed: 51 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -585,10 +585,6 @@ func DeleteFile(getClient GetClientFn, t translations.TranslationHelperFunc) (to
585585
mcp.Required(),
586586
mcp.Description("Branch to delete the file from"),
587587
),
588-
mcp.WithString("sha",
589-
mcp.Required(),
590-
mcp.Description("SHA of the file being deleted"),
591-
),
592588
),
593589
func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
594590
owner, err := requiredParam[string](request, "owner")
@@ -611,38 +607,70 @@ func DeleteFile(getClient GetClientFn, t translations.TranslationHelperFunc) (to
611607
if err != nil {
612608
return mcp.NewToolResultError(err.Error()), nil
613609
}
614-
sha, err := requiredParam[string](request, "sha")
610+
611+
client, err := getClient(ctx)
615612
if err != nil {
616-
return mcp.NewToolResultError(err.Error()), nil
613+
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
617614
}
618615

619-
// Create the file options
620-
opts := &github.RepositoryContentFileOptions{
621-
Message: github.Ptr(message),
622-
Branch: github.Ptr(branch),
623-
SHA: github.Ptr(sha),
616+
// Get the reference for the branch
617+
ref, resp, err := client.Git.GetRef(ctx, owner, repo, "refs/heads/"+branch)
618+
if err != nil {
619+
return nil, fmt.Errorf("failed to get branch reference: %w", err)
624620
}
621+
defer func() { _ = resp.Body.Close() }()
625622

626-
// Delete the file
627-
client, err := getClient(ctx)
623+
// Get the commit object that the branch points to
624+
baseCommit, resp, err := client.Git.GetCommit(ctx, owner, repo, *ref.Object.SHA)
628625
if err != nil {
629-
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
626+
return nil, fmt.Errorf("failed to get base commit: %w", err)
630627
}
631-
deleteResponse, resp, err := client.Repositories.DeleteFile(ctx, owner, repo, path, opts)
628+
defer func() { _ = resp.Body.Close() }()
629+
630+
// Create a tree entry for the file deletion by setting SHA to nil
631+
treeEntries := []*github.TreeEntry{
632+
{
633+
Path: github.Ptr(path),
634+
Mode: github.Ptr("100644"), // Regular file mode
635+
Type: github.Ptr("blob"),
636+
SHA: nil, // Setting SHA to nil deletes the file
637+
},
638+
}
639+
640+
// Create a new tree with the deletion
641+
newTree, resp, err := client.Git.CreateTree(ctx, owner, repo, *baseCommit.Tree.SHA, treeEntries)
632642
if err != nil {
633-
return nil, fmt.Errorf("failed to delete file: %w", err)
643+
return nil, fmt.Errorf("failed to create tree: %w", err)
634644
}
635645
defer func() { _ = resp.Body.Close() }()
636646

637-
if resp.StatusCode != 200 && resp.StatusCode != 204 {
638-
body, err := io.ReadAll(resp.Body)
639-
if err != nil {
640-
return nil, fmt.Errorf("failed to read response body: %w", err)
641-
}
642-
return mcp.NewToolResultError(fmt.Sprintf("failed to delete file: %s", string(body))), nil
647+
// Create a new commit with the new tree
648+
commit := &github.Commit{
649+
Message: github.Ptr(message),
650+
Tree: newTree,
651+
Parents: []*github.Commit{{SHA: baseCommit.SHA}},
652+
}
653+
newCommit, resp, err := client.Git.CreateCommit(ctx, owner, repo, commit, nil)
654+
if err != nil {
655+
return nil, fmt.Errorf("failed to create commit: %w", err)
656+
}
657+
defer func() { _ = resp.Body.Close() }()
658+
659+
// Update the branch reference to point to the new commit
660+
ref.Object.SHA = newCommit.SHA
661+
_, resp, err = client.Git.UpdateRef(ctx, owner, repo, ref, false)
662+
if err != nil {
663+
return nil, fmt.Errorf("failed to update reference: %w", err)
664+
}
665+
defer func() { _ = resp.Body.Close() }()
666+
667+
// Create a response similar to what the DeleteFile API would return
668+
response := map[string]interface{}{
669+
"commit": newCommit,
670+
"content": nil,
643671
}
644672

645-
r, err := json.Marshal(deleteResponse)
673+
r, err := json.Marshal(response)
646674
if err != nil {
647675
return nil, fmt.Errorf("failed to marshal response: %w", err)
648676
}

‎pkg/github/repositories_test.go‎

Lines changed: 92 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1541,49 +1541,96 @@ func Test_DeleteFile(t *testing.T) {
15411541
assert.Contains(t, tool.InputSchema.Properties, "path")
15421542
assert.Contains(t, tool.InputSchema.Properties, "message")
15431543
assert.Contains(t, tool.InputSchema.Properties, "branch")
1544-
assert.Contains(t, tool.InputSchema.Properties, "sha")
1545-
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "path", "message", "branch", "sha"})
1544+
// SHA is no longer required since we're using Git Data API
1545+
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "path", "message", "branch"})
15461546

1547-
// Setup mock delete response
1548-
mockDeleteResponse := &github.RepositoryContentResponse{
1549-
Content: &github.RepositoryContent{
1550-
Name: github.Ptr("example.md"),
1551-
Path: github.Ptr("docs/example.md"),
1552-
SHA: github.Ptr("abc123def456"),
1553-
HTMLURL: github.Ptr("https://github.com/owner/repo/blob/main/docs/example.md"),
1547+
// Setup mock objects for Git Data API
1548+
mockRef := &github.Reference{
1549+
Ref: github.Ptr("refs/heads/main"),
1550+
Object: &github.GitObject{
1551+
SHA: github.Ptr("abc123"),
15541552
},
1555-
Commit: github.Commit{
1556-
SHA: github.Ptr("def456abc789"),
1557-
Message: github.Ptr("Delete example file"),
1558-
Author: &github.CommitAuthor{
1559-
Name: github.Ptr("Test User"),
1560-
Email: github.Ptr("[email protected]"),
1561-
Date: &github.Timestamp{Time: time.Now()},
1562-
},
1563-
HTMLURL: github.Ptr("https://github.com/owner/repo/commit/def456abc789"),
1553+
}
1554+
1555+
mockCommit := &github.Commit{
1556+
SHA: github.Ptr("abc123"),
1557+
Tree: &github.Tree{
1558+
SHA: github.Ptr("def456"),
15641559
},
15651560
}
15661561

1562+
mockTree := &github.Tree{
1563+
SHA: github.Ptr("ghi789"),
1564+
}
1565+
1566+
mockNewCommit := &github.Commit{
1567+
SHA: github.Ptr("jkl012"),
1568+
Message: github.Ptr("Delete example file"),
1569+
HTMLURL: github.Ptr("https://github.com/owner/repo/commit/jkl012"),
1570+
}
1571+
15671572
tests := []struct {
1568-
name string
1569-
mockedClient *http.Client
1570-
requestArgs map[string]interface{}
1571-
expectError bool
1572-
expectedResult *github.RepositoryContentResponse
1573-
expectedErrMsg string
1573+
name string
1574+
mockedClient *http.Client
1575+
requestArgs map[string]interface{}
1576+
expectError bool
1577+
expectedCommitSHA string
1578+
expectedErrMsg string
15741579
}{
15751580
{
1576-
name: "successful file deletion",
1581+
name: "successful file deletion using Git Data API",
15771582
mockedClient: mock.NewMockedHTTPClient(
1583+
// Get branch reference
1584+
mock.WithRequestMatch(
1585+
mock.GetReposGitRefByOwnerByRepoByRef,
1586+
mockRef,
1587+
),
1588+
// Get commit
1589+
mock.WithRequestMatch(
1590+
mock.GetReposGitCommitsByOwnerByRepoByCommitSha,
1591+
mockCommit,
1592+
),
1593+
// Create tree
1594+
mock.WithRequestMatchHandler(
1595+
mock.PostReposGitTreesByOwnerByRepo,
1596+
expectRequestBody(t, map[string]interface{}{
1597+
"base_tree": "def456",
1598+
"tree": []interface{}{
1599+
map[string]interface{}{
1600+
"path": "docs/example.md",
1601+
"mode": "100644",
1602+
"type": "blob",
1603+
"sha": nil,
1604+
},
1605+
},
1606+
}).andThen(
1607+
mockResponse(t, http.StatusCreated, mockTree),
1608+
),
1609+
),
1610+
// Create commit
15781611
mock.WithRequestMatchHandler(
1579-
mock.DeleteReposContentsByOwnerByRepoByPath,
1612+
mock.PostReposGitCommitsByOwnerByRepo,
15801613
expectRequestBody(t, map[string]interface{}{
15811614
"message": "Delete example file",
1582-
"branch": "main",
1583-
"sha": "abc123def456",
1584-
"content": interface{}(nil),
1615+
"tree": "ghi789",
1616+
"parents": []interface{}{"abc123"},
1617+
}).andThen(
1618+
mockResponse(t, http.StatusCreated, mockNewCommit),
1619+
),
1620+
),
1621+
// Update reference
1622+
mock.WithRequestMatchHandler(
1623+
mock.PatchReposGitRefsByOwnerByRepoByRef,
1624+
expectRequestBody(t, map[string]interface{}{
1625+
"sha": "jkl012",
1626+
"force": false,
15851627
}).andThen(
1586-
mockResponse(t, http.StatusOK, mockDeleteResponse),
1628+
mockResponse(t, http.StatusOK, &github.Reference{
1629+
Ref: github.Ptr("refs/heads/main"),
1630+
Object: &github.GitObject{
1631+
SHA: github.Ptr("jkl012"),
1632+
},
1633+
}),
15871634
),
15881635
),
15891636
),
@@ -1593,19 +1640,18 @@ func Test_DeleteFile(t *testing.T) {
15931640
"path": "docs/example.md",
15941641
"message": "Delete example file",
15951642
"branch": "main",
1596-
"sha": "abc123def456",
15971643
},
1598-
expectError: false,
1599-
expectedResult: mockDeleteResponse,
1644+
expectError: false,
1645+
expectedCommitSHA: "jkl012",
16001646
},
16011647
{
1602-
name: "file deletion fails",
1648+
name: "file deletion fails - branch not found",
16031649
mockedClient: mock.NewMockedHTTPClient(
16041650
mock.WithRequestMatchHandler(
1605-
mock.DeleteReposContentsByOwnerByRepoByPath,
1651+
mock.GetReposGitRefByOwnerByRepoByRef,
16061652
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
16071653
w.WriteHeader(http.StatusNotFound)
1608-
_, _ = w.Write([]byte(`{"message": "Not Found"}`))
1654+
_, _ = w.Write([]byte(`{"message": "Reference not found"}`))
16091655
}),
16101656
),
16111657
),
@@ -1614,11 +1660,10 @@ func Test_DeleteFile(t *testing.T) {
16141660
"repo": "repo",
16151661
"path": "docs/nonexistent.md",
16161662
"message": "Delete nonexistent file",
1617-
"branch": "main",
1618-
"sha": "abc123def456",
1663+
"branch": "nonexistent-branch",
16191664
},
16201665
expectError: true,
1621-
expectedErrMsg: "failed to delete file",
1666+
expectedErrMsg: "failed to get branch reference",
16221667
},
16231668
}
16241669

@@ -1647,20 +1692,16 @@ func Test_DeleteFile(t *testing.T) {
16471692
textContent := getTextResult(t, result)
16481693

16491694
// Unmarshal and verify the result
1650-
var returnedContent github.RepositoryContentResponse
1651-
err = json.Unmarshal([]byte(textContent.Text), &returnedContent)
1695+
var response map[string]interface{}
1696+
err = json.Unmarshal([]byte(textContent.Text), &response)
16521697
require.NoError(t, err)
16531698

1654-
// Verify content
1655-
if tc.expectedResult.Content != nil {
1656-
assert.Equal(t, *tc.expectedResult.Content.Name, *returnedContent.Content.Name)
1657-
assert.Equal(t, *tc.expectedResult.Content.Path, *returnedContent.Content.Path)
1658-
assert.Equal(t, *tc.expectedResult.Content.SHA, *returnedContent.Content.SHA)
1659-
}
1660-
1661-
// Verify commit
1662-
assert.Equal(t, *tc.expectedResult.Commit.SHA, *returnedContent.Commit.SHA)
1663-
assert.Equal(t, *tc.expectedResult.Commit.Message, *returnedContent.Commit.Message)
1699+
// Verify the response contains the expected commit
1700+
commit, ok := response["commit"].(map[string]interface{})
1701+
require.True(t, ok)
1702+
commitSHA, ok := commit["sha"].(string)
1703+
require.True(t, ok)
1704+
assert.Equal(t, tc.expectedCommitSHA, commitSHA)
16641705
})
16651706
}
16661707
}

0 commit comments

Comments
 (0)