From 9affbc55e9adb27daa0c039fae0d136d9ed9aa88 Mon Sep 17 00:00:00 2001
From: Ashwin Bhat <ashwin@anthropic.com>
Date: Mon, 28 Apr 2025 10:49:15 -0700
Subject: [PATCH 1/7] feat: add DeleteFile tool to delete files from GitHub
 repositories
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
---
 pkg/github/repositories.go      |  90 +++++++++++++++++++++
 pkg/github/repositories_test.go | 136 ++++++++++++++++++++++++++++++++
 pkg/github/tools.go             |   1 +
 3 files changed, 227 insertions(+)

diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go
index fa69de558..ff0931470 100644
--- a/pkg/github/repositories.go
+++ b/pkg/github/repositories.go
@@ -556,6 +556,96 @@ func ForkRepository(getClient GetClientFn, t translations.TranslationHelperFunc)
 		}
 }
 
+// DeleteFile creates a tool to delete a file in a GitHub repository.
+func DeleteFile(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
+	return mcp.NewTool("delete_file",
+			mcp.WithDescription(t("TOOL_DELETE_FILE_DESCRIPTION", "Delete a file from a GitHub repository")),
+			mcp.WithString("owner",
+				mcp.Required(),
+				mcp.Description("Repository owner (username or organization)"),
+			),
+			mcp.WithString("repo",
+				mcp.Required(),
+				mcp.Description("Repository name"),
+			),
+			mcp.WithString("path",
+				mcp.Required(),
+				mcp.Description("Path to the file to delete"),
+			),
+			mcp.WithString("message",
+				mcp.Required(),
+				mcp.Description("Commit message"),
+			),
+			mcp.WithString("branch",
+				mcp.Required(),
+				mcp.Description("Branch to delete the file from"),
+			),
+			mcp.WithString("sha",
+				mcp.Required(),
+				mcp.Description("SHA of the file being deleted"),
+			),
+		),
+		func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
+			owner, err := requiredParam[string](request, "owner")
+			if err != nil {
+				return mcp.NewToolResultError(err.Error()), nil
+			}
+			repo, err := requiredParam[string](request, "repo")
+			if err != nil {
+				return mcp.NewToolResultError(err.Error()), nil
+			}
+			path, err := requiredParam[string](request, "path")
+			if err != nil {
+				return mcp.NewToolResultError(err.Error()), nil
+			}
+			message, err := requiredParam[string](request, "message")
+			if err != nil {
+				return mcp.NewToolResultError(err.Error()), nil
+			}
+			branch, err := requiredParam[string](request, "branch")
+			if err != nil {
+				return mcp.NewToolResultError(err.Error()), nil
+			}
+			sha, err := requiredParam[string](request, "sha")
+			if err != nil {
+				return mcp.NewToolResultError(err.Error()), nil
+			}
+
+			// Create the file options
+			opts := &github.RepositoryContentFileOptions{
+				Message: github.Ptr(message),
+				Branch:  github.Ptr(branch),
+				SHA:     github.Ptr(sha),
+			}
+
+			// Delete the file
+			client, err := getClient(ctx)
+			if err != nil {
+				return nil, fmt.Errorf("failed to get GitHub client: %w", err)
+			}
+			deleteResponse, resp, err := client.Repositories.DeleteFile(ctx, owner, repo, path, opts)
+			if err != nil {
+				return nil, fmt.Errorf("failed to delete file: %w", err)
+			}
+			defer func() { _ = resp.Body.Close() }()
+
+			if resp.StatusCode != 200 && resp.StatusCode != 204 {
+				body, err := io.ReadAll(resp.Body)
+				if err != nil {
+					return nil, fmt.Errorf("failed to read response body: %w", err)
+				}
+				return mcp.NewToolResultError(fmt.Sprintf("failed to delete file: %s", string(body))), nil
+			}
+
+			r, err := json.Marshal(deleteResponse)
+			if err != nil {
+				return nil, fmt.Errorf("failed to marshal response: %w", err)
+			}
+
+			return mcp.NewToolResultText(string(r)), nil
+		}
+}
+
 // CreateBranch creates a tool to create a new branch.
 func CreateBranch(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
 	return mcp.NewTool("create_branch",
diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go
index 59d19fc41..5dbbaeaa3 100644
--- a/pkg/github/repositories_test.go
+++ b/pkg/github/repositories_test.go
@@ -1529,6 +1529,142 @@ func Test_ListBranches(t *testing.T) {
 	}
 }
 
+func Test_DeleteFile(t *testing.T) {
+	// Verify tool definition once
+	mockClient := github.NewClient(nil)
+	tool, _ := DeleteFile(stubGetClientFn(mockClient), translations.NullTranslationHelper)
+
+	assert.Equal(t, "delete_file", tool.Name)
+	assert.NotEmpty(t, tool.Description)
+	assert.Contains(t, tool.InputSchema.Properties, "owner")
+	assert.Contains(t, tool.InputSchema.Properties, "repo")
+	assert.Contains(t, tool.InputSchema.Properties, "path")
+	assert.Contains(t, tool.InputSchema.Properties, "message")
+	assert.Contains(t, tool.InputSchema.Properties, "branch")
+	assert.Contains(t, tool.InputSchema.Properties, "sha")
+	assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "path", "message", "branch", "sha"})
+
+	// Setup mock delete response
+	mockDeleteResponse := &github.RepositoryContentResponse{
+		Content: &github.RepositoryContent{
+			Name:    github.Ptr("example.md"),
+			Path:    github.Ptr("docs/example.md"),
+			SHA:     github.Ptr("abc123def456"),
+			HTMLURL: github.Ptr("https://github.com/owner/repo/blob/main/docs/example.md"),
+		},
+		Commit: github.Commit{
+			SHA:     github.Ptr("def456abc789"),
+			Message: github.Ptr("Delete example file"),
+			Author: &github.CommitAuthor{
+				Name:  github.Ptr("Test User"),
+				Email: github.Ptr("test@example.com"),
+				Date:  &github.Timestamp{Time: time.Now()},
+			},
+			HTMLURL: github.Ptr("https://github.com/owner/repo/commit/def456abc789"),
+		},
+	}
+
+	tests := []struct {
+		name           string
+		mockedClient   *http.Client
+		requestArgs    map[string]interface{}
+		expectError    bool
+		expectedResult *github.RepositoryContentResponse
+		expectedErrMsg string
+	}{
+		{
+			name: "successful file deletion",
+			mockedClient: mock.NewMockedHTTPClient(
+				mock.WithRequestMatchHandler(
+					mock.DeleteReposContentsByOwnerByRepoByPath,
+					expectRequestBody(t, map[string]interface{}{
+						"message": "Delete example file",
+						"branch":  "main",
+						"sha":     "abc123def456",
+						"content": interface{}(nil),
+					}).andThen(
+						mockResponse(t, http.StatusOK, mockDeleteResponse),
+					),
+				),
+			),
+			requestArgs: map[string]interface{}{
+				"owner":   "owner",
+				"repo":    "repo",
+				"path":    "docs/example.md",
+				"message": "Delete example file",
+				"branch":  "main",
+				"sha":     "abc123def456",
+			},
+			expectError:    false,
+			expectedResult: mockDeleteResponse,
+		},
+		{
+			name: "file deletion fails",
+			mockedClient: mock.NewMockedHTTPClient(
+				mock.WithRequestMatchHandler(
+					mock.DeleteReposContentsByOwnerByRepoByPath,
+					http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+						w.WriteHeader(http.StatusNotFound)
+						_, _ = w.Write([]byte(`{"message": "Not Found"}`))
+					}),
+				),
+			),
+			requestArgs: map[string]interface{}{
+				"owner":   "owner",
+				"repo":    "repo",
+				"path":    "docs/nonexistent.md",
+				"message": "Delete nonexistent file",
+				"branch":  "main",
+				"sha":     "abc123def456",
+			},
+			expectError:    true,
+			expectedErrMsg: "failed to delete file",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			// Setup client with mock
+			client := github.NewClient(tc.mockedClient)
+			_, handler := DeleteFile(stubGetClientFn(client), translations.NullTranslationHelper)
+
+			// Create call request
+			request := createMCPRequest(tc.requestArgs)
+
+			// Call handler
+			result, err := handler(context.Background(), request)
+
+			// Verify results
+			if tc.expectError {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), tc.expectedErrMsg)
+				return
+			}
+
+			require.NoError(t, err)
+
+			// Parse the result and get the text content if no error
+			textContent := getTextResult(t, result)
+
+			// Unmarshal and verify the result
+			var returnedContent github.RepositoryContentResponse
+			err = json.Unmarshal([]byte(textContent.Text), &returnedContent)
+			require.NoError(t, err)
+
+			// Verify content
+			if tc.expectedResult.Content != nil {
+				assert.Equal(t, *tc.expectedResult.Content.Name, *returnedContent.Content.Name)
+				assert.Equal(t, *tc.expectedResult.Content.Path, *returnedContent.Content.Path)
+				assert.Equal(t, *tc.expectedResult.Content.SHA, *returnedContent.Content.SHA)
+			}
+
+			// Verify commit
+			assert.Equal(t, *tc.expectedResult.Commit.SHA, *returnedContent.Commit.SHA)
+			assert.Equal(t, *tc.expectedResult.Commit.Message, *returnedContent.Commit.Message)
+		})
+	}
+}
+
 func Test_ListTags(t *testing.T) {
 	// Verify tool definition once
 	mockClient := github.NewClient(nil)
diff --git a/pkg/github/tools.go b/pkg/github/tools.go
index 0d8099785..faef86ce7 100644
--- a/pkg/github/tools.go
+++ b/pkg/github/tools.go
@@ -36,6 +36,7 @@ func InitToolsets(passedToolsets []string, readOnly bool, getClient GetClientFn,
 			toolsets.NewServerTool(ForkRepository(getClient, t)),
 			toolsets.NewServerTool(CreateBranch(getClient, t)),
 			toolsets.NewServerTool(PushFiles(getClient, t)),
+			toolsets.NewServerTool(DeleteFile(getClient, t)),
 		)
 	issues := toolsets.NewToolset("issues", "GitHub Issues related tools").
 		AddReadTools(

From 3d5f6b76dac91ef1b78237d9a3fd77459be36507 Mon Sep 17 00:00:00 2001
From: Ashwin Bhat <ashwin@anthropic.com>
Date: Tue, 29 Apr 2025 07:06:46 -0700
Subject: [PATCH 2/7] feat: add ReadOnlyHint and DestructiveHint annotations to
 DeleteFile tool
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
---
 pkg/github/repositories.go | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go
index ff0931470..926812cf5 100644
--- a/pkg/github/repositories.go
+++ b/pkg/github/repositories.go
@@ -560,6 +560,11 @@ func ForkRepository(getClient GetClientFn, t translations.TranslationHelperFunc)
 func DeleteFile(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
 	return mcp.NewTool("delete_file",
 			mcp.WithDescription(t("TOOL_DELETE_FILE_DESCRIPTION", "Delete a file from a GitHub repository")),
+			mcp.WithToolAnnotation(mcp.ToolAnnotation{
+				Title:           t("TOOL_DELETE_FILE_USER_TITLE", "Delete file"),
+				ReadOnlyHint:    false,
+				DestructiveHint: true,
+			}),
 			mcp.WithString("owner",
 				mcp.Required(),
 				mcp.Description("Repository owner (username or organization)"),

From 4a5f2903cbf0fa00c5aadd94d1eb33c17d9894fb Mon Sep 17 00:00:00 2001
From: Ashwin Bhat <ashwin@anthropic.com>
Date: Thu, 1 May 2025 13:00:02 -0700
Subject: [PATCH 3/7] switch to approach that does commit signing

---
 pkg/github/repositories.go      |  74 ++++++++++++-----
 pkg/github/repositories_test.go | 143 ++++++++++++++++++++------------
 2 files changed, 143 insertions(+), 74 deletions(-)

diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go
index 926812cf5..7f53aee6a 100644
--- a/pkg/github/repositories.go
+++ b/pkg/github/repositories.go
@@ -585,10 +585,6 @@ func DeleteFile(getClient GetClientFn, t translations.TranslationHelperFunc) (to
 				mcp.Required(),
 				mcp.Description("Branch to delete the file from"),
 			),
-			mcp.WithString("sha",
-				mcp.Required(),
-				mcp.Description("SHA of the file being deleted"),
-			),
 		),
 		func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
 			owner, err := requiredParam[string](request, "owner")
@@ -611,38 +607,70 @@ func DeleteFile(getClient GetClientFn, t translations.TranslationHelperFunc) (to
 			if err != nil {
 				return mcp.NewToolResultError(err.Error()), nil
 			}
-			sha, err := requiredParam[string](request, "sha")
+
+			client, err := getClient(ctx)
 			if err != nil {
-				return mcp.NewToolResultError(err.Error()), nil
+				return nil, fmt.Errorf("failed to get GitHub client: %w", err)
 			}
 
-			// Create the file options
-			opts := &github.RepositoryContentFileOptions{
-				Message: github.Ptr(message),
-				Branch:  github.Ptr(branch),
-				SHA:     github.Ptr(sha),
+			// Get the reference for the branch
+			ref, resp, err := client.Git.GetRef(ctx, owner, repo, "refs/heads/"+branch)
+			if err != nil {
+				return nil, fmt.Errorf("failed to get branch reference: %w", err)
 			}
+			defer func() { _ = resp.Body.Close() }()
 
-			// Delete the file
-			client, err := getClient(ctx)
+			// Get the commit object that the branch points to
+			baseCommit, resp, err := client.Git.GetCommit(ctx, owner, repo, *ref.Object.SHA)
 			if err != nil {
-				return nil, fmt.Errorf("failed to get GitHub client: %w", err)
+				return nil, fmt.Errorf("failed to get base commit: %w", err)
 			}
-			deleteResponse, resp, err := client.Repositories.DeleteFile(ctx, owner, repo, path, opts)
+			defer func() { _ = resp.Body.Close() }()
+
+			// Create a tree entry for the file deletion by setting SHA to nil
+			treeEntries := []*github.TreeEntry{
+				{
+					Path: github.Ptr(path),
+					Mode: github.Ptr("100644"), // Regular file mode
+					Type: github.Ptr("blob"),
+					SHA:  nil, // Setting SHA to nil deletes the file
+				},
+			}
+
+			// Create a new tree with the deletion
+			newTree, resp, err := client.Git.CreateTree(ctx, owner, repo, *baseCommit.Tree.SHA, treeEntries)
 			if err != nil {
-				return nil, fmt.Errorf("failed to delete file: %w", err)
+				return nil, fmt.Errorf("failed to create tree: %w", err)
 			}
 			defer func() { _ = resp.Body.Close() }()
 
-			if resp.StatusCode != 200 && resp.StatusCode != 204 {
-				body, err := io.ReadAll(resp.Body)
-				if err != nil {
-					return nil, fmt.Errorf("failed to read response body: %w", err)
-				}
-				return mcp.NewToolResultError(fmt.Sprintf("failed to delete file: %s", string(body))), nil
+			// Create a new commit with the new tree
+			commit := &github.Commit{
+				Message: github.Ptr(message),
+				Tree:    newTree,
+				Parents: []*github.Commit{{SHA: baseCommit.SHA}},
+			}
+			newCommit, resp, err := client.Git.CreateCommit(ctx, owner, repo, commit, nil)
+			if err != nil {
+				return nil, fmt.Errorf("failed to create commit: %w", err)
+			}
+			defer func() { _ = resp.Body.Close() }()
+
+			// Update the branch reference to point to the new commit
+			ref.Object.SHA = newCommit.SHA
+			_, resp, err = client.Git.UpdateRef(ctx, owner, repo, ref, false)
+			if err != nil {
+				return nil, fmt.Errorf("failed to update reference: %w", err)
+			}
+			defer func() { _ = resp.Body.Close() }()
+
+			// Create a response similar to what the DeleteFile API would return
+			response := map[string]interface{}{
+				"commit": newCommit,
+				"content": nil,
 			}
 
-			r, err := json.Marshal(deleteResponse)
+			r, err := json.Marshal(response)
 			if err != nil {
 				return nil, fmt.Errorf("failed to marshal response: %w", err)
 			}
diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go
index 5dbbaeaa3..b0a90c0ff 100644
--- a/pkg/github/repositories_test.go
+++ b/pkg/github/repositories_test.go
@@ -1541,49 +1541,96 @@ func Test_DeleteFile(t *testing.T) {
 	assert.Contains(t, tool.InputSchema.Properties, "path")
 	assert.Contains(t, tool.InputSchema.Properties, "message")
 	assert.Contains(t, tool.InputSchema.Properties, "branch")
-	assert.Contains(t, tool.InputSchema.Properties, "sha")
-	assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "path", "message", "branch", "sha"})
+	// SHA is no longer required since we're using Git Data API
+	assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "path", "message", "branch"})
 
-	// Setup mock delete response
-	mockDeleteResponse := &github.RepositoryContentResponse{
-		Content: &github.RepositoryContent{
-			Name:    github.Ptr("example.md"),
-			Path:    github.Ptr("docs/example.md"),
-			SHA:     github.Ptr("abc123def456"),
-			HTMLURL: github.Ptr("https://github.com/owner/repo/blob/main/docs/example.md"),
+	// Setup mock objects for Git Data API
+	mockRef := &github.Reference{
+		Ref: github.Ptr("refs/heads/main"),
+		Object: &github.GitObject{
+			SHA: github.Ptr("abc123"),
 		},
-		Commit: github.Commit{
-			SHA:     github.Ptr("def456abc789"),
-			Message: github.Ptr("Delete example file"),
-			Author: &github.CommitAuthor{
-				Name:  github.Ptr("Test User"),
-				Email: github.Ptr("test@example.com"),
-				Date:  &github.Timestamp{Time: time.Now()},
-			},
-			HTMLURL: github.Ptr("https://github.com/owner/repo/commit/def456abc789"),
+	}
+
+	mockCommit := &github.Commit{
+		SHA: github.Ptr("abc123"),
+		Tree: &github.Tree{
+			SHA: github.Ptr("def456"),
 		},
 	}
 
+	mockTree := &github.Tree{
+		SHA: github.Ptr("ghi789"),
+	}
+
+	mockNewCommit := &github.Commit{
+		SHA:     github.Ptr("jkl012"),
+		Message: github.Ptr("Delete example file"),
+		HTMLURL: github.Ptr("https://github.com/owner/repo/commit/jkl012"),
+	}
+
 	tests := []struct {
-		name           string
-		mockedClient   *http.Client
-		requestArgs    map[string]interface{}
-		expectError    bool
-		expectedResult *github.RepositoryContentResponse
-		expectedErrMsg string
+		name             string
+		mockedClient     *http.Client
+		requestArgs      map[string]interface{}
+		expectError      bool
+		expectedCommitSHA string
+		expectedErrMsg   string
 	}{
 		{
-			name: "successful file deletion",
+			name: "successful file deletion using Git Data API",
 			mockedClient: mock.NewMockedHTTPClient(
+				// Get branch reference
+				mock.WithRequestMatch(
+					mock.GetReposGitRefByOwnerByRepoByRef,
+					mockRef,
+				),
+				// Get commit
+				mock.WithRequestMatch(
+					mock.GetReposGitCommitsByOwnerByRepoByCommitSha,
+					mockCommit,
+				),
+				// Create tree
+				mock.WithRequestMatchHandler(
+					mock.PostReposGitTreesByOwnerByRepo,
+					expectRequestBody(t, map[string]interface{}{
+						"base_tree": "def456",
+						"tree": []interface{}{
+							map[string]interface{}{
+								"path":    "docs/example.md",
+								"mode":    "100644",
+								"type":    "blob",
+								"sha":     nil,
+							},
+						},
+					}).andThen(
+						mockResponse(t, http.StatusCreated, mockTree),
+					),
+				),
+				// Create commit
 				mock.WithRequestMatchHandler(
-					mock.DeleteReposContentsByOwnerByRepoByPath,
+					mock.PostReposGitCommitsByOwnerByRepo,
 					expectRequestBody(t, map[string]interface{}{
 						"message": "Delete example file",
-						"branch":  "main",
-						"sha":     "abc123def456",
-						"content": interface{}(nil),
+						"tree":    "ghi789",
+						"parents": []interface{}{"abc123"},
+					}).andThen(
+						mockResponse(t, http.StatusCreated, mockNewCommit),
+					),
+				),
+				// Update reference
+				mock.WithRequestMatchHandler(
+					mock.PatchReposGitRefsByOwnerByRepoByRef,
+					expectRequestBody(t, map[string]interface{}{
+						"sha":   "jkl012",
+						"force": false,
 					}).andThen(
-						mockResponse(t, http.StatusOK, mockDeleteResponse),
+						mockResponse(t, http.StatusOK, &github.Reference{
+							Ref: github.Ptr("refs/heads/main"),
+							Object: &github.GitObject{
+								SHA: github.Ptr("jkl012"),
+							},
+						}),
 					),
 				),
 			),
@@ -1593,19 +1640,18 @@ func Test_DeleteFile(t *testing.T) {
 				"path":    "docs/example.md",
 				"message": "Delete example file",
 				"branch":  "main",
-				"sha":     "abc123def456",
 			},
-			expectError:    false,
-			expectedResult: mockDeleteResponse,
+			expectError:        false,
+			expectedCommitSHA: "jkl012",
 		},
 		{
-			name: "file deletion fails",
+			name: "file deletion fails - branch not found",
 			mockedClient: mock.NewMockedHTTPClient(
 				mock.WithRequestMatchHandler(
-					mock.DeleteReposContentsByOwnerByRepoByPath,
+					mock.GetReposGitRefByOwnerByRepoByRef,
 					http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 						w.WriteHeader(http.StatusNotFound)
-						_, _ = w.Write([]byte(`{"message": "Not Found"}`))
+						_, _ = w.Write([]byte(`{"message": "Reference not found"}`))
 					}),
 				),
 			),
@@ -1614,11 +1660,10 @@ func Test_DeleteFile(t *testing.T) {
 				"repo":    "repo",
 				"path":    "docs/nonexistent.md",
 				"message": "Delete nonexistent file",
-				"branch":  "main",
-				"sha":     "abc123def456",
+				"branch":  "nonexistent-branch",
 			},
 			expectError:    true,
-			expectedErrMsg: "failed to delete file",
+			expectedErrMsg: "failed to get branch reference",
 		},
 	}
 
@@ -1647,20 +1692,16 @@ func Test_DeleteFile(t *testing.T) {
 			textContent := getTextResult(t, result)
 
 			// Unmarshal and verify the result
-			var returnedContent github.RepositoryContentResponse
-			err = json.Unmarshal([]byte(textContent.Text), &returnedContent)
+			var response map[string]interface{}
+			err = json.Unmarshal([]byte(textContent.Text), &response)
 			require.NoError(t, err)
 
-			// Verify content
-			if tc.expectedResult.Content != nil {
-				assert.Equal(t, *tc.expectedResult.Content.Name, *returnedContent.Content.Name)
-				assert.Equal(t, *tc.expectedResult.Content.Path, *returnedContent.Content.Path)
-				assert.Equal(t, *tc.expectedResult.Content.SHA, *returnedContent.Content.SHA)
-			}
-
-			// Verify commit
-			assert.Equal(t, *tc.expectedResult.Commit.SHA, *returnedContent.Commit.SHA)
-			assert.Equal(t, *tc.expectedResult.Commit.Message, *returnedContent.Commit.Message)
+			// Verify the response contains the expected commit
+			commit, ok := response["commit"].(map[string]interface{})
+			require.True(t, ok)
+			commitSHA, ok := commit["sha"].(string)
+			require.True(t, ok)
+			assert.Equal(t, tc.expectedCommitSHA, commitSHA)
 		})
 	}
 }

From 880629f4d9eb74764349a69a43fe7e67cbc6c301 Mon Sep 17 00:00:00 2001
From: William Martin <williammartin@github.com>
Date: Mon, 12 May 2025 13:58:16 +0200
Subject: [PATCH 4/7] Add e2e tests for file and directory deletion

---
 e2e/e2e_test.go                 | 403 ++++++++++++++++++++++++++++++++
 pkg/github/repositories.go      |   4 +-
 pkg/github/repositories_test.go |  20 +-
 3 files changed, 415 insertions(+), 12 deletions(-)

diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go
index b6637191c..489681e96 100644
--- a/e2e/e2e_test.go
+++ b/e2e/e2e_test.go
@@ -4,6 +4,7 @@ package e2e_test
 
 import (
 	"context"
+	"encoding/base64"
 	"encoding/json"
 	"fmt"
 	"os"
@@ -369,3 +370,405 @@ func TestTags(t *testing.T) {
 	require.Equal(t, "v0.0.1", trimmedTag[0].Name, "expected tag name to match")
 	require.Equal(t, *ref.Object.SHA, trimmedTag[0].Commit.SHA, "expected tag SHA to match")
 }
+
+func TestFileDeletion(t *testing.T) {
+	t.Parallel()
+
+	mcpClient := setupMCPClient(t)
+
+	ctx := context.Background()
+
+	// First, who am I
+	getMeRequest := mcp.CallToolRequest{}
+	getMeRequest.Params.Name = "get_me"
+
+	t.Log("Getting current user...")
+	resp, err := mcpClient.CallTool(ctx, getMeRequest)
+	require.NoError(t, err, "expected to call 'get_me' tool successfully")
+	require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
+
+	require.False(t, resp.IsError, "expected result not to be an error")
+	require.Len(t, resp.Content, 1, "expected content to have one item")
+
+	textContent, ok := resp.Content[0].(mcp.TextContent)
+	require.True(t, ok, "expected content to be of type TextContent")
+
+	var trimmedGetMeText struct {
+		Login string `json:"login"`
+	}
+	err = json.Unmarshal([]byte(textContent.Text), &trimmedGetMeText)
+	require.NoError(t, err, "expected to unmarshal text content successfully")
+
+	currentOwner := trimmedGetMeText.Login
+
+	// Then create a repository with a README (via autoInit)
+	repoName := fmt.Sprintf("github-mcp-server-e2e-%s-%d", t.Name(), time.Now().UnixMilli())
+	createRepoRequest := mcp.CallToolRequest{}
+	createRepoRequest.Params.Name = "create_repository"
+	createRepoRequest.Params.Arguments = map[string]any{
+		"name":     repoName,
+		"private":  true,
+		"autoInit": true,
+	}
+	t.Logf("Creating repository %s/%s...", currentOwner, repoName)
+	_, err = mcpClient.CallTool(ctx, createRepoRequest)
+	require.NoError(t, err, "expected to call 'get_me' tool successfully")
+	require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
+
+	// Cleanup the repository after the test
+	t.Cleanup(func() {
+		// MCP Server doesn't support deletions, but we can use the GitHub Client
+		ghClient := gogithub.NewClient(nil).WithAuthToken(getE2EToken(t))
+		t.Logf("Deleting repository %s/%s...", currentOwner, repoName)
+		_, err := ghClient.Repositories.Delete(context.Background(), currentOwner, repoName)
+		require.NoError(t, err, "expected to delete repository successfully")
+	})
+
+	// Create a branch on which to create a new commit
+	createBranchRequest := mcp.CallToolRequest{}
+	createBranchRequest.Params.Name = "create_branch"
+	createBranchRequest.Params.Arguments = map[string]any{
+		"owner":       currentOwner,
+		"repo":        repoName,
+		"branch":      "test-branch",
+		"from_branch": "main",
+	}
+
+	t.Logf("Creating branch in %s/%s...", currentOwner, repoName)
+	resp, err = mcpClient.CallTool(ctx, createBranchRequest)
+	require.NoError(t, err, "expected to call 'create_branch' tool successfully")
+	require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
+
+	// Create a commit with a new file
+	commitRequest := mcp.CallToolRequest{}
+	commitRequest.Params.Name = "create_or_update_file"
+	commitRequest.Params.Arguments = map[string]any{
+		"owner":   currentOwner,
+		"repo":    repoName,
+		"path":    "test-file.txt",
+		"content": fmt.Sprintf("Created by e2e test %s", t.Name()),
+		"message": "Add test file",
+		"branch":  "test-branch",
+	}
+
+	t.Logf("Creating commit with new file in %s/%s...", currentOwner, repoName)
+	resp, err = mcpClient.CallTool(ctx, commitRequest)
+	require.NoError(t, err, "expected to call 'create_or_update_file' tool successfully")
+	require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
+
+	textContent, ok = resp.Content[0].(mcp.TextContent)
+	require.True(t, ok, "expected content to be of type TextContent")
+
+	var trimmedCommitText struct {
+		SHA string `json:"sha"`
+	}
+	err = json.Unmarshal([]byte(textContent.Text), &trimmedCommitText)
+	require.NoError(t, err, "expected to unmarshal text content successfully")
+
+	// Check the file exists
+	getFileContentsRequest := mcp.CallToolRequest{}
+	getFileContentsRequest.Params.Name = "get_file_contents"
+	getFileContentsRequest.Params.Arguments = map[string]any{
+		"owner":  currentOwner,
+		"repo":   repoName,
+		"path":   "test-file.txt",
+		"branch": "test-branch",
+	}
+
+	t.Logf("Getting file contents in %s/%s...", currentOwner, repoName)
+	resp, err = mcpClient.CallTool(ctx, getFileContentsRequest)
+	require.NoError(t, err, "expected to call 'get_file_contents' tool successfully")
+	require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
+
+	textContent, ok = resp.Content[0].(mcp.TextContent)
+	require.True(t, ok, "expected content to be of type TextContent")
+
+	var trimmedGetFileText struct {
+		Content string `json:"content"`
+	}
+	err = json.Unmarshal([]byte(textContent.Text), &trimmedGetFileText)
+	require.NoError(t, err, "expected to unmarshal text content successfully")
+	b, err := base64.StdEncoding.DecodeString(trimmedGetFileText.Content)
+	require.NoError(t, err, "expected to decode base64 content successfully")
+	require.Equal(t, fmt.Sprintf("Created by e2e test %s", t.Name()), string(b), "expected file content to match")
+
+	// Delete the file
+	deleteFileRequest := mcp.CallToolRequest{}
+	deleteFileRequest.Params.Name = "delete_file"
+	deleteFileRequest.Params.Arguments = map[string]any{
+		"owner":   currentOwner,
+		"repo":    repoName,
+		"path":    "test-file.txt",
+		"message": "Delete test file",
+		"branch":  "test-branch",
+	}
+
+	t.Logf("Deleting file in %s/%s...", currentOwner, repoName)
+	resp, err = mcpClient.CallTool(ctx, deleteFileRequest)
+	require.NoError(t, err, "expected to call 'delete_file' tool successfully")
+	require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
+
+	// See that there is a commit that removes the file
+	listCommitsRequest := mcp.CallToolRequest{}
+	listCommitsRequest.Params.Name = "list_commits"
+	listCommitsRequest.Params.Arguments = map[string]any{
+		"owner": currentOwner,
+		"repo":  repoName,
+		"sha":   "test-branch", // can be SHA or branch, which is an unfortunate API design
+	}
+
+	t.Logf("Listing commits in %s/%s...", currentOwner, repoName)
+	resp, err = mcpClient.CallTool(ctx, listCommitsRequest)
+	require.NoError(t, err, "expected to call 'list_commits' tool successfully")
+	require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
+
+	textContent, ok = resp.Content[0].(mcp.TextContent)
+	require.True(t, ok, "expected content to be of type TextContent")
+
+	var trimmedListCommitsText []struct {
+		SHA    string `json:"sha"`
+		Commit struct {
+			Message string `json:"message"`
+		}
+		Files []struct {
+			Filename  string `json:"filename"`
+			Deletions int    `json:"deletions"`
+		}
+	}
+	err = json.Unmarshal([]byte(textContent.Text), &trimmedListCommitsText)
+	require.NoError(t, err, "expected to unmarshal text content successfully")
+	require.GreaterOrEqual(t, len(trimmedListCommitsText), 1, "expected to find at least one commit")
+
+	deletionCommit := trimmedListCommitsText[0]
+	require.Equal(t, "Delete test file", deletionCommit.Commit.Message, "expected commit message to match")
+
+	// Now get the commit so we can look at the file changes because list_commits doesn't include them
+	getCommitRequest := mcp.CallToolRequest{}
+	getCommitRequest.Params.Name = "get_commit"
+	getCommitRequest.Params.Arguments = map[string]any{
+		"owner": currentOwner,
+		"repo":  repoName,
+		"sha":   deletionCommit.SHA,
+	}
+
+	t.Logf("Getting commit %s/%s:%s...", currentOwner, repoName, deletionCommit.SHA)
+	resp, err = mcpClient.CallTool(ctx, getCommitRequest)
+	require.NoError(t, err, "expected to call 'get_commit' tool successfully")
+	require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
+
+	textContent, ok = resp.Content[0].(mcp.TextContent)
+	require.True(t, ok, "expected content to be of type TextContent")
+
+	var trimmedGetCommitText struct {
+		Files []struct {
+			Filename  string `json:"filename"`
+			Deletions int    `json:"deletions"`
+		}
+	}
+	err = json.Unmarshal([]byte(textContent.Text), &trimmedGetCommitText)
+	require.NoError(t, err, "expected to unmarshal text content successfully")
+	require.Len(t, trimmedGetCommitText.Files, 1, "expected to find one file change")
+	require.Equal(t, "test-file.txt", trimmedGetCommitText.Files[0].Filename, "expected filename to match")
+	require.Equal(t, 1, trimmedGetCommitText.Files[0].Deletions, "expected one deletion")
+}
+
+func TestDirectoryDeletion(t *testing.T) {
+	t.Parallel()
+
+	mcpClient := setupMCPClient(t)
+
+	ctx := context.Background()
+
+	// First, who am I
+	getMeRequest := mcp.CallToolRequest{}
+	getMeRequest.Params.Name = "get_me"
+
+	t.Log("Getting current user...")
+	resp, err := mcpClient.CallTool(ctx, getMeRequest)
+	require.NoError(t, err, "expected to call 'get_me' tool successfully")
+	require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
+
+	require.False(t, resp.IsError, "expected result not to be an error")
+	require.Len(t, resp.Content, 1, "expected content to have one item")
+
+	textContent, ok := resp.Content[0].(mcp.TextContent)
+	require.True(t, ok, "expected content to be of type TextContent")
+
+	var trimmedGetMeText struct {
+		Login string `json:"login"`
+	}
+	err = json.Unmarshal([]byte(textContent.Text), &trimmedGetMeText)
+	require.NoError(t, err, "expected to unmarshal text content successfully")
+
+	currentOwner := trimmedGetMeText.Login
+
+	// Then create a repository with a README (via autoInit)
+	repoName := fmt.Sprintf("github-mcp-server-e2e-%s-%d", t.Name(), time.Now().UnixMilli())
+	createRepoRequest := mcp.CallToolRequest{}
+	createRepoRequest.Params.Name = "create_repository"
+	createRepoRequest.Params.Arguments = map[string]any{
+		"name":     repoName,
+		"private":  true,
+		"autoInit": true,
+	}
+	t.Logf("Creating repository %s/%s...", currentOwner, repoName)
+	_, err = mcpClient.CallTool(ctx, createRepoRequest)
+	require.NoError(t, err, "expected to call 'get_me' tool successfully")
+	require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
+
+	// Cleanup the repository after the test
+	t.Cleanup(func() {
+		// MCP Server doesn't support deletions, but we can use the GitHub Client
+		ghClient := gogithub.NewClient(nil).WithAuthToken(getE2EToken(t))
+		t.Logf("Deleting repository %s/%s...", currentOwner, repoName)
+		_, err := ghClient.Repositories.Delete(context.Background(), currentOwner, repoName)
+		require.NoError(t, err, "expected to delete repository successfully")
+	})
+
+	// Create a branch on which to create a new commit
+	createBranchRequest := mcp.CallToolRequest{}
+	createBranchRequest.Params.Name = "create_branch"
+	createBranchRequest.Params.Arguments = map[string]any{
+		"owner":       currentOwner,
+		"repo":        repoName,
+		"branch":      "test-branch",
+		"from_branch": "main",
+	}
+
+	t.Logf("Creating branch in %s/%s...", currentOwner, repoName)
+	resp, err = mcpClient.CallTool(ctx, createBranchRequest)
+	require.NoError(t, err, "expected to call 'create_branch' tool successfully")
+	require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
+
+	// Create a commit with a new file
+	commitRequest := mcp.CallToolRequest{}
+	commitRequest.Params.Name = "create_or_update_file"
+	commitRequest.Params.Arguments = map[string]any{
+		"owner":   currentOwner,
+		"repo":    repoName,
+		"path":    "test-dir/test-file.txt",
+		"content": fmt.Sprintf("Created by e2e test %s", t.Name()),
+		"message": "Add test file",
+		"branch":  "test-branch",
+	}
+
+	t.Logf("Creating commit with new file in %s/%s...", currentOwner, repoName)
+	resp, err = mcpClient.CallTool(ctx, commitRequest)
+	require.NoError(t, err, "expected to call 'create_or_update_file' tool successfully")
+	require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
+
+	textContent, ok = resp.Content[0].(mcp.TextContent)
+	require.True(t, ok, "expected content to be of type TextContent")
+
+	var trimmedCommitText struct {
+		SHA string `json:"sha"`
+	}
+	err = json.Unmarshal([]byte(textContent.Text), &trimmedCommitText)
+	require.NoError(t, err, "expected to unmarshal text content successfully")
+
+	// Check the file exists
+	getFileContentsRequest := mcp.CallToolRequest{}
+	getFileContentsRequest.Params.Name = "get_file_contents"
+	getFileContentsRequest.Params.Arguments = map[string]any{
+		"owner":  currentOwner,
+		"repo":   repoName,
+		"path":   "test-dir/test-file.txt",
+		"branch": "test-branch",
+	}
+
+	t.Logf("Getting file contents in %s/%s...", currentOwner, repoName)
+	resp, err = mcpClient.CallTool(ctx, getFileContentsRequest)
+	require.NoError(t, err, "expected to call 'get_file_contents' tool successfully")
+	require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
+
+	textContent, ok = resp.Content[0].(mcp.TextContent)
+	require.True(t, ok, "expected content to be of type TextContent")
+
+	var trimmedGetFileText struct {
+		Content string `json:"content"`
+	}
+	err = json.Unmarshal([]byte(textContent.Text), &trimmedGetFileText)
+	require.NoError(t, err, "expected to unmarshal text content successfully")
+	b, err := base64.StdEncoding.DecodeString(trimmedGetFileText.Content)
+	require.NoError(t, err, "expected to decode base64 content successfully")
+	require.Equal(t, fmt.Sprintf("Created by e2e test %s", t.Name()), string(b), "expected file content to match")
+
+	// Delete the directory containing the file
+	deleteFileRequest := mcp.CallToolRequest{}
+	deleteFileRequest.Params.Name = "delete_file"
+	deleteFileRequest.Params.Arguments = map[string]any{
+		"owner":   currentOwner,
+		"repo":    repoName,
+		"path":    "test-dir",
+		"message": "Delete test directory",
+		"branch":  "test-branch",
+	}
+
+	t.Logf("Deleting directory in %s/%s...", currentOwner, repoName)
+	resp, err = mcpClient.CallTool(ctx, deleteFileRequest)
+	require.NoError(t, err, "expected to call 'delete_file' tool successfully")
+	require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
+
+	// See that there is a commit that removes the directory
+	listCommitsRequest := mcp.CallToolRequest{}
+	listCommitsRequest.Params.Name = "list_commits"
+	listCommitsRequest.Params.Arguments = map[string]any{
+		"owner": currentOwner,
+		"repo":  repoName,
+		"sha":   "test-branch", // can be SHA or branch, which is an unfortunate API design
+	}
+
+	t.Logf("Listing commits in %s/%s...", currentOwner, repoName)
+	resp, err = mcpClient.CallTool(ctx, listCommitsRequest)
+	require.NoError(t, err, "expected to call 'list_commits' tool successfully")
+	require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
+
+	textContent, ok = resp.Content[0].(mcp.TextContent)
+	require.True(t, ok, "expected content to be of type TextContent")
+
+	var trimmedListCommitsText []struct {
+		SHA    string `json:"sha"`
+		Commit struct {
+			Message string `json:"message"`
+		}
+		Files []struct {
+			Filename  string `json:"filename"`
+			Deletions int    `json:"deletions"`
+		} `json:"files"`
+	}
+	err = json.Unmarshal([]byte(textContent.Text), &trimmedListCommitsText)
+	require.NoError(t, err, "expected to unmarshal text content successfully")
+	require.GreaterOrEqual(t, len(trimmedListCommitsText), 1, "expected to find at least one commit")
+
+	deletionCommit := trimmedListCommitsText[0]
+	require.Equal(t, "Delete test directory", deletionCommit.Commit.Message, "expected commit message to match")
+
+	// Now get the commit so we can look at the file changes because list_commits doesn't include them
+	getCommitRequest := mcp.CallToolRequest{}
+	getCommitRequest.Params.Name = "get_commit"
+	getCommitRequest.Params.Arguments = map[string]any{
+		"owner": currentOwner,
+		"repo":  repoName,
+		"sha":   deletionCommit.SHA,
+	}
+
+	t.Logf("Getting commit %s/%s:%s...", currentOwner, repoName, deletionCommit.SHA)
+	resp, err = mcpClient.CallTool(ctx, getCommitRequest)
+	require.NoError(t, err, "expected to call 'get_commit' tool successfully")
+	require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
+
+	textContent, ok = resp.Content[0].(mcp.TextContent)
+	require.True(t, ok, "expected content to be of type TextContent")
+
+	var trimmedGetCommitText struct {
+		Files []struct {
+			Filename  string `json:"filename"`
+			Deletions int    `json:"deletions"`
+		}
+	}
+	err = json.Unmarshal([]byte(textContent.Text), &trimmedGetCommitText)
+	require.NoError(t, err, "expected to unmarshal text content successfully")
+	require.Len(t, trimmedGetCommitText.Files, 1, "expected to find one file change")
+	require.Equal(t, "test-dir/test-file.txt", trimmedGetCommitText.Files[0].Filename, "expected filename to match")
+	require.Equal(t, 1, trimmedGetCommitText.Files[0].Deletions, "expected one deletion")
+}
diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go
index 7f53aee6a..795c4467f 100644
--- a/pkg/github/repositories.go
+++ b/pkg/github/repositories.go
@@ -287,7 +287,7 @@ func CreateOrUpdateFile(getClient GetClientFn, t translations.TranslationHelperF
 				return mcp.NewToolResultError(err.Error()), nil
 			}
 
-			// Convert content to base64
+			// json.Marshal encodes byte arrays with base64, which is required for the API.
 			contentBytes := []byte(content)
 
 			// Create the file options
@@ -666,7 +666,7 @@ func DeleteFile(getClient GetClientFn, t translations.TranslationHelperFunc) (to
 
 			// Create a response similar to what the DeleteFile API would return
 			response := map[string]interface{}{
-				"commit": newCommit,
+				"commit":  newCommit,
 				"content": nil,
 			}
 
diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go
index b0a90c0ff..6bb97da53 100644
--- a/pkg/github/repositories_test.go
+++ b/pkg/github/repositories_test.go
@@ -1570,12 +1570,12 @@ func Test_DeleteFile(t *testing.T) {
 	}
 
 	tests := []struct {
-		name             string
-		mockedClient     *http.Client
-		requestArgs      map[string]interface{}
-		expectError      bool
+		name              string
+		mockedClient      *http.Client
+		requestArgs       map[string]interface{}
+		expectError       bool
 		expectedCommitSHA string
-		expectedErrMsg   string
+		expectedErrMsg    string
 	}{
 		{
 			name: "successful file deletion using Git Data API",
@@ -1597,10 +1597,10 @@ func Test_DeleteFile(t *testing.T) {
 						"base_tree": "def456",
 						"tree": []interface{}{
 							map[string]interface{}{
-								"path":    "docs/example.md",
-								"mode":    "100644",
-								"type":    "blob",
-								"sha":     nil,
+								"path": "docs/example.md",
+								"mode": "100644",
+								"type": "blob",
+								"sha":  nil,
 							},
 						},
 					}).andThen(
@@ -1641,7 +1641,7 @@ func Test_DeleteFile(t *testing.T) {
 				"message": "Delete example file",
 				"branch":  "main",
 			},
-			expectError:        false,
+			expectError:       false,
 			expectedCommitSHA: "jkl012",
 		},
 		{

From 36833ff573541e6939802d323dc5fe512fe161f5 Mon Sep 17 00:00:00 2001
From: Ashwin Bhat <ashwin@anthropic.com>
Date: Mon, 12 May 2025 09:12:00 -0700
Subject: [PATCH 5/7] add comment

---
 pkg/github/repositories.go | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go
index 795c4467f..d3d51facd 100644
--- a/pkg/github/repositories.go
+++ b/pkg/github/repositories.go
@@ -557,6 +557,11 @@ func ForkRepository(getClient GetClientFn, t translations.TranslationHelperFunc)
 }
 
 // DeleteFile creates a tool to delete a file in a GitHub repository.
+// This tool uses a more roundabout way of deleting a file than just using the client.Repositories.DeleteFile.
+// This is because REST file deletion endpoint (and client.Repositories.DeleteFile) don't add commit signing to the deletion commit, 
+// unlike how the endpoint backing the create_or_update_files tool does. This appears to be a quirk of the API. 
+// The approach implemented here gets automatic commit signing when used with either the github-actions user or as an app, 
+// both of which suit an LLM well.
 func DeleteFile(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
 	return mcp.NewTool("delete_file",
 			mcp.WithDescription(t("TOOL_DELETE_FILE_DESCRIPTION", "Delete a file from a GitHub repository")),

From 33d04023cc7a63f63b7f3361dabd5de819514bbd Mon Sep 17 00:00:00 2001
From: William Martin <williammartin@github.com>
Date: Tue, 13 May 2025 14:05:56 +0200
Subject: [PATCH 6/7] fix mcp-go bump breaking change

---
 pkg/github/repositories.go | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go
index d3d51facd..8740fc828 100644
--- a/pkg/github/repositories.go
+++ b/pkg/github/repositories.go
@@ -558,17 +558,17 @@ func ForkRepository(getClient GetClientFn, t translations.TranslationHelperFunc)
 
 // DeleteFile creates a tool to delete a file in a GitHub repository.
 // This tool uses a more roundabout way of deleting a file than just using the client.Repositories.DeleteFile.
-// This is because REST file deletion endpoint (and client.Repositories.DeleteFile) don't add commit signing to the deletion commit, 
-// unlike how the endpoint backing the create_or_update_files tool does. This appears to be a quirk of the API. 
-// The approach implemented here gets automatic commit signing when used with either the github-actions user or as an app, 
+// This is because REST file deletion endpoint (and client.Repositories.DeleteFile) don't add commit signing to the deletion commit,
+// unlike how the endpoint backing the create_or_update_files tool does. This appears to be a quirk of the API.
+// The approach implemented here gets automatic commit signing when used with either the github-actions user or as an app,
 // both of which suit an LLM well.
 func DeleteFile(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) {
 	return mcp.NewTool("delete_file",
 			mcp.WithDescription(t("TOOL_DELETE_FILE_DESCRIPTION", "Delete a file from a GitHub repository")),
 			mcp.WithToolAnnotation(mcp.ToolAnnotation{
 				Title:           t("TOOL_DELETE_FILE_USER_TITLE", "Delete file"),
-				ReadOnlyHint:    false,
-				DestructiveHint: true,
+				ReadOnlyHint:    toBoolPtr(false),
+				DestructiveHint: toBoolPtr(true),
 			}),
 			mcp.WithString("owner",
 				mcp.Required(),

From 8c5af0e555b0c650da4402e5891026fee339947e Mon Sep 17 00:00:00 2001
From: William Martin <williammartin@github.com>
Date: Tue, 13 May 2025 14:14:53 +0200
Subject: [PATCH 7/7] Add additional error handling on delete_file

---
 pkg/github/repositories.go | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go
index 8740fc828..4403e2a19 100644
--- a/pkg/github/repositories.go
+++ b/pkg/github/repositories.go
@@ -632,6 +632,14 @@ func DeleteFile(getClient GetClientFn, t translations.TranslationHelperFunc) (to
 			}
 			defer func() { _ = resp.Body.Close() }()
 
+			if resp.StatusCode != http.StatusOK {
+				body, err := io.ReadAll(resp.Body)
+				if err != nil {
+					return nil, fmt.Errorf("failed to read response body: %w", err)
+				}
+				return mcp.NewToolResultError(fmt.Sprintf("failed to get commit: %s", string(body))), nil
+			}
+
 			// Create a tree entry for the file deletion by setting SHA to nil
 			treeEntries := []*github.TreeEntry{
 				{
@@ -649,6 +657,14 @@ func DeleteFile(getClient GetClientFn, t translations.TranslationHelperFunc) (to
 			}
 			defer func() { _ = resp.Body.Close() }()
 
+			if resp.StatusCode != http.StatusCreated {
+				body, err := io.ReadAll(resp.Body)
+				if err != nil {
+					return nil, fmt.Errorf("failed to read response body: %w", err)
+				}
+				return mcp.NewToolResultError(fmt.Sprintf("failed to create tree: %s", string(body))), nil
+			}
+
 			// Create a new commit with the new tree
 			commit := &github.Commit{
 				Message: github.Ptr(message),
@@ -661,6 +677,14 @@ func DeleteFile(getClient GetClientFn, t translations.TranslationHelperFunc) (to
 			}
 			defer func() { _ = resp.Body.Close() }()
 
+			if resp.StatusCode != http.StatusCreated {
+				body, err := io.ReadAll(resp.Body)
+				if err != nil {
+					return nil, fmt.Errorf("failed to read response body: %w", err)
+				}
+				return mcp.NewToolResultError(fmt.Sprintf("failed to create commit: %s", string(body))), nil
+			}
+
 			// Update the branch reference to point to the new commit
 			ref.Object.SHA = newCommit.SHA
 			_, resp, err = client.Git.UpdateRef(ctx, owner, repo, ref, false)
@@ -669,6 +693,14 @@ func DeleteFile(getClient GetClientFn, t translations.TranslationHelperFunc) (to
 			}
 			defer func() { _ = resp.Body.Close() }()
 
+			if resp.StatusCode != http.StatusOK {
+				body, err := io.ReadAll(resp.Body)
+				if err != nil {
+					return nil, fmt.Errorf("failed to read response body: %w", err)
+				}
+				return mcp.NewToolResultError(fmt.Sprintf("failed to update reference: %s", string(body))), nil
+			}
+
 			// Create a response similar to what the DeleteFile API would return
 			response := map[string]interface{}{
 				"commit":  newCommit,