diff --git a/src/mcp/github-file-ops-server.ts b/src/mcp/github-file-ops-server.ts index 4d61621b..a8d5c72b 100644 --- a/src/mcp/github-file-ops-server.ts +++ b/src/mcp/github-file-ops-server.ts @@ -8,8 +8,8 @@ import { resolve } from "path"; import { constants } from "fs"; import fetch from "node-fetch"; import { GITHUB_API_URL } from "../github/api/config"; -import { retryWithBackoff } from "../utils/retry"; import { validatePathWithinRepo } from "./path-validation"; +import { updateGitReference } from "./update-git-reference"; type GitHubRef = { object: { @@ -365,57 +365,13 @@ server.tool( const newCommitData = (await newCommitResponse.json()) as GitHubNewCommit; // 6. Update the reference to point to the new commit - const updateRefUrl = `${GITHUB_API_URL}/repos/${owner}/${repo}/git/refs/heads/${branch}`; - - // We're seeing intermittent 403 "Resource not accessible by integration" errors - // on certain repos when updating git references. These appear to be transient - // GitHub API issues that succeed on retry. - await retryWithBackoff( - async () => { - const updateRefResponse = await fetch(updateRefUrl, { - method: "PATCH", - headers: { - Accept: "application/vnd.github+json", - Authorization: `Bearer ${githubToken}`, - "X-GitHub-Api-Version": "2022-11-28", - "Content-Type": "application/json", - }, - body: JSON.stringify({ - sha: newCommitData.sha, - force: false, - }), - }); - - if (!updateRefResponse.ok) { - const errorText = await updateRefResponse.text(); - - // Provide a more helpful error message for 403 permission errors - if (updateRefResponse.status === 403) { - const permissionError = new Error( - `Permission denied: Unable to push commits to branch '${branch}'. ` + - `Please rebase your branch from the main/master branch to allow Claude to commit.\n\n` + - `Original error: ${errorText}`, - ); - throw permissionError; - } - - // For other errors, use the original message - const error = new Error( - `Failed to update reference: ${updateRefResponse.status} - ${errorText}`, - ); - - // For non-403 errors, fail immediately without retry - console.error("Non-retryable error:", updateRefResponse.status); - throw error; - } - }, - { - maxAttempts: 3, - initialDelayMs: 1000, // Start with 1 second delay - maxDelayMs: 5000, // Max 5 seconds delay - backoffFactor: 2, // Double the delay each time - }, - ); + await updateGitReference({ + owner, + repo, + branch, + sha: newCommitData.sha, + githubToken, + }); const simplifiedResult = { commit: { @@ -580,58 +536,13 @@ server.tool( const newCommitData = (await newCommitResponse.json()) as GitHubNewCommit; // 6. Update the reference to point to the new commit - const updateRefUrl = `${GITHUB_API_URL}/repos/${owner}/${repo}/git/refs/heads/${branch}`; - - // We're seeing intermittent 403 "Resource not accessible by integration" errors - // on certain repos when updating git references. These appear to be transient - // GitHub API issues that succeed on retry. - await retryWithBackoff( - async () => { - const updateRefResponse = await fetch(updateRefUrl, { - method: "PATCH", - headers: { - Accept: "application/vnd.github+json", - Authorization: `Bearer ${githubToken}`, - "X-GitHub-Api-Version": "2022-11-28", - "Content-Type": "application/json", - }, - body: JSON.stringify({ - sha: newCommitData.sha, - force: false, - }), - }); - - if (!updateRefResponse.ok) { - const errorText = await updateRefResponse.text(); - - // Provide a more helpful error message for 403 permission errors - if (updateRefResponse.status === 403) { - console.log("Received 403 error, will retry..."); - const permissionError = new Error( - `Permission denied: Unable to push commits to branch '${branch}'. ` + - `Please rebase your branch from the main/master branch to allow Claude to commit.\n\n` + - `Original error: ${errorText}`, - ); - throw permissionError; - } - - // For other errors, use the original message - const error = new Error( - `Failed to update reference: ${updateRefResponse.status} - ${errorText}`, - ); - - // For non-403 errors, fail immediately without retry - console.error("Non-retryable error:", updateRefResponse.status); - throw error; - } - }, - { - maxAttempts: 3, - initialDelayMs: 1000, // Start with 1 second delay - maxDelayMs: 5000, // Max 5 seconds delay - backoffFactor: 2, // Double the delay each time - }, - ); + await updateGitReference({ + owner, + repo, + branch, + sha: newCommitData.sha, + githubToken, + }); const simplifiedResult = { commit: { diff --git a/src/mcp/update-git-reference.ts b/src/mcp/update-git-reference.ts new file mode 100644 index 00000000..9b3bc993 --- /dev/null +++ b/src/mcp/update-git-reference.ts @@ -0,0 +1,90 @@ +import fetch, { type RequestInit, type Response } from "node-fetch"; +import { GITHUB_API_URL } from "../github/api/config"; +import { retryWithBackoff, type RetryOptions } from "../utils/retry"; + +type GitHubFetch = ( + url: string, + init: RequestInit, +) => Promise>; + +type UpdateGitReferenceOptions = { + owner: string; + repo: string; + branch: string; + sha: string; + githubToken: string; + fetchFn?: GitHubFetch; + retryOptions?: Omit; +}; + +class GitReferenceUpdateError extends Error { + constructor( + readonly status: number, + message: string, + ) { + super(message); + this.name = "GitReferenceUpdateError"; + } +} + +function shouldRetryGitReferenceUpdate(error: Error): boolean { + if (!(error instanceof GitReferenceUpdateError)) { + return true; + } + + return error.status === 403 || error.status === 429 || error.status >= 500; +} + +export async function updateGitReference({ + owner, + repo, + branch, + sha, + githubToken, + fetchFn = fetch, + retryOptions, +}: UpdateGitReferenceOptions): Promise { + const updateRefUrl = `${GITHUB_API_URL}/repos/${owner}/${repo}/git/refs/heads/${branch}`; + + await retryWithBackoff( + async () => { + const response = await fetchFn(updateRefUrl, { + method: "PATCH", + headers: { + Accept: "application/vnd.github+json", + Authorization: `Bearer ${githubToken}`, + "X-GitHub-Api-Version": "2022-11-28", + "Content-Type": "application/json", + }, + body: JSON.stringify({ sha, force: false }), + }); + + if (response.ok) { + return; + } + + const errorText = await response.text(); + if (response.status === 403) { + throw new GitReferenceUpdateError( + response.status, + `Permission denied: Unable to push commits to branch '${branch}'. ` + + `Please rebase your branch from the main/master branch to allow Claude to commit.\n\n` + + `Original error: ${errorText}`, + ); + } + + throw new GitReferenceUpdateError( + response.status, + `Failed to update reference: ${response.status} - ${errorText}`, + ); + }, + { + maxAttempts: 3, + initialDelayMs: 1000, + maxDelayMs: 5000, + backoffFactor: 2, + ...retryOptions, + shouldRetry: shouldRetryGitReferenceUpdate, + }, + ); +} diff --git a/test/update-git-reference.test.ts b/test/update-git-reference.test.ts new file mode 100644 index 00000000..4f57cba7 --- /dev/null +++ b/test/update-git-reference.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it } from "bun:test"; +import { GITHUB_API_URL } from "../src/github/api/config"; +import { updateGitReference } from "../src/mcp/update-git-reference"; + +const reference = { + owner: "owner", + repo: "repo", + branch: "feature", + sha: "abc123", + githubToken: "token", +}; + +function response(status: number) { + return { + ok: status >= 200 && status < 300, + status, + text: async () => "response body", + }; +} + +describe("updateGitReference", () => { + it("should patch the branch reference", async () => { + await updateGitReference({ + ...reference, + fetchFn: async (url, init) => { + expect(url).toBe( + `${GITHUB_API_URL}/repos/owner/repo/git/refs/heads/feature`, + ); + expect(init.method).toBe("PATCH"); + expect(init.headers).toMatchObject({ Authorization: "Bearer token" }); + expect(JSON.parse(String(init.body))).toEqual({ + sha: "abc123", + force: false, + }); + return response(200); + }, + }); + }); + + it("should not retry deterministic client errors", async () => { + for (const status of [400, 404, 409, 422]) { + let attempts = 0; + + await expect( + updateGitReference({ + ...reference, + fetchFn: async () => { + attempts++; + return response(status); + }, + retryOptions: { initialDelayMs: 1 }, + }), + ).rejects.toThrow(`Failed to update reference: ${status}`); + + expect(attempts).toBe(1); + } + }); + + it("should retry transient HTTP errors", async () => { + for (const status of [403, 429, 500]) { + let attempts = 0; + + await updateGitReference({ + ...reference, + fetchFn: async () => response(attempts++ === 0 ? status : 200), + retryOptions: { initialDelayMs: 1 }, + }); + + expect(attempts).toBe(2); + } + }); + + it("should retry network errors", async () => { + let attempts = 0; + + await updateGitReference({ + ...reference, + fetchFn: async () => { + attempts++; + if (attempts === 1) { + throw new Error("network error"); + } + return response(200); + }, + retryOptions: { initialDelayMs: 1 }, + }); + + expect(attempts).toBe(2); + }); +});