Revert "feat: defer remote branch creation until first commit (#244)" (#278)

This reverts commit cefe963a6b.
This commit is contained in:
Ashwin Bhat
2025-07-15 16:05:30 -07:00
committed by GitHub
parent a9d9ad3612
commit 018533dc9a
8 changed files with 71 additions and 249 deletions

View File

@@ -12,6 +12,7 @@ import { checkHumanActor } from "../github/validation/actor";
import { checkWritePermissions } from "../github/validation/permissions";
import { createInitialComment } from "../github/operations/comments/create-initial";
import { setupBranch } from "../github/operations/branch";
import { updateTrackingComment } from "../github/operations/comments/update-with-branch";
import { configureGitAuth } from "../github/operations/git-config";
import { prepareMcpConfig } from "../mcp/install-mcp-server";
import { createPrompt } from "../create-prompt";
@@ -66,7 +67,17 @@ async function run() {
// Step 8: Setup branch
const branchInfo = await setupBranch(octokit, githubData, context);
// Step 9: Configure git authentication if not using commit signing
// Step 9: Update initial comment with branch link (only for issues that created a new branch)
if (branchInfo.claudeBranch) {
await updateTrackingComment(
octokit,
context,
commentId,
branchInfo.claudeBranch,
);
}
// Step 10: Configure git authentication if not using commit signing
if (!context.inputs.useCommitSigning) {
try {
await configureGitAuth(githubToken, context, commentData.user);
@@ -76,7 +87,7 @@ async function run() {
}
}
// Step 10: Create prompt file
// Step 11: Create prompt file
await createPrompt(
commentId,
branchInfo.baseBranch,
@@ -85,7 +96,7 @@ async function run() {
context,
);
// Step 11: Get MCP configuration
// Step 12: Get MCP configuration
const additionalMcpConfig = process.env.MCP_CONFIG || "";
const mcpConfig = await prepareMcpConfig({
githubToken,

View File

@@ -201,7 +201,7 @@ async function run() {
jobUrl,
branchLink,
prLink,
branchName: shouldDeleteBranch || !branchLink ? undefined : claudeBranch,
branchName: shouldDeleteBranch ? undefined : claudeBranch,
triggerUsername,
errorDetails,
};

View File

@@ -14,31 +14,6 @@ export async function checkAndCommitOrDeleteBranch(
let shouldDeleteBranch = false;
if (claudeBranch) {
// First check if the branch exists remotely
let branchExistsRemotely = false;
try {
await octokit.rest.repos.getBranch({
owner,
repo,
branch: claudeBranch,
});
branchExistsRemotely = true;
} catch (error: any) {
if (error.status === 404) {
console.log(`Branch ${claudeBranch} does not exist remotely`);
} else {
console.error("Error checking if branch exists:", error);
}
}
// Only proceed if branch exists remotely
if (!branchExistsRemotely) {
console.log(
`Branch ${claudeBranch} does not exist remotely, no branch link will be added`,
);
return { shouldDeleteBranch: false, branchLink: "" };
}
// Check if Claude made any commits to the branch
try {
const { data: comparison } =
@@ -106,8 +81,8 @@ export async function checkAndCommitOrDeleteBranch(
branchLink = `\n[View branch](${branchUrl})`;
}
} catch (error) {
console.error("Error comparing commits on Claude branch:", error);
// If we can't compare but the branch exists remotely, include the branch link
console.error("Error checking for commits on Claude branch:", error);
// If we can't check, assume the branch has commits to be safe
const branchUrl = `${GITHUB_SERVER_URL}/${owner}/${repo}/tree/${claudeBranch}`;
branchLink = `\n[View branch](${branchUrl})`;
}

View File

@@ -84,7 +84,7 @@ export async function setupBranch(
sourceBranch = repoResponse.data.default_branch;
}
// Generate branch name for either an issue or closed/merged PR
// Creating a new branch for either an issue or closed/merged PR
const entityType = isPR ? "pr" : "issue";
// Create Kubernetes-compatible timestamp: lowercase, hyphens only, shorter format
@@ -100,7 +100,7 @@ export async function setupBranch(
const newBranch = branchName.toLowerCase().substring(0, 50);
try {
// Get the SHA of the source branch to verify it exists
// Get the SHA of the source branch
const sourceBranchRef = await octokits.rest.git.getRef({
owner,
repo,
@@ -108,34 +108,23 @@ export async function setupBranch(
});
const currentSHA = sourceBranchRef.data.object.sha;
console.log(`Source branch SHA: ${currentSHA}`);
// For commit signing, defer branch creation to the file ops server
if (context.inputs.useCommitSigning) {
console.log(
`Branch name generated: ${newBranch} (will be created by file ops server on first commit)`,
);
console.log(`Current SHA: ${currentSHA}`);
// Set outputs for GitHub Actions
core.setOutput("CLAUDE_BRANCH", newBranch);
core.setOutput("BASE_BRANCH", sourceBranch);
return {
baseBranch: sourceBranch,
claudeBranch: newBranch,
currentBranch: sourceBranch, // Stay on source branch for now
};
}
// Create branch using GitHub API
await octokits.rest.git.createRef({
owner,
repo,
ref: `refs/heads/${newBranch}`,
sha: currentSHA,
});
// For non-signing case, create and checkout the branch locally only
console.log(
`Creating local branch ${newBranch} for ${entityType} #${entityNumber} from source branch: ${sourceBranch}...`,
);
// Create and checkout the new branch locally
await $`git checkout -b ${newBranch}`;
// Checkout the new branch (shallow fetch for performance)
await $`git fetch origin --depth=1 ${newBranch}`;
await $`git checkout ${newBranch}`;
console.log(
`Successfully created and checked out local branch: ${newBranch}`,
`Successfully created and checked out new branch: ${newBranch}`,
);
// Set outputs for GitHub Actions
@@ -147,7 +136,7 @@ export async function setupBranch(
currentBranch: newBranch,
};
} catch (error) {
console.error("Error in branch setup:", error);
console.error("Error creating branch:", error);
process.exit(1);
}
}

View File

@@ -52,120 +52,6 @@ const server = new McpServer({
version: "0.0.1",
});
// Helper function to get or create branch reference
async function getOrCreateBranchRef(
owner: string,
repo: string,
branch: string,
githubToken: string,
): Promise<string> {
// Try to get the branch reference
const refUrl = `${GITHUB_API_URL}/repos/${owner}/${repo}/git/refs/heads/${branch}`;
const refResponse = await fetch(refUrl, {
headers: {
Accept: "application/vnd.github+json",
Authorization: `Bearer ${githubToken}`,
"X-GitHub-Api-Version": "2022-11-28",
},
});
if (refResponse.ok) {
const refData = (await refResponse.json()) as GitHubRef;
return refData.object.sha;
}
if (refResponse.status !== 404) {
throw new Error(`Failed to get branch reference: ${refResponse.status}`);
}
// Branch doesn't exist, need to create it
console.log(`Branch ${branch} does not exist, creating it...`);
// Get base branch from environment or determine it
const baseBranch = process.env.BASE_BRANCH || "main";
// Get the SHA of the base branch
const baseRefUrl = `${GITHUB_API_URL}/repos/${owner}/${repo}/git/refs/heads/${baseBranch}`;
const baseRefResponse = await fetch(baseRefUrl, {
headers: {
Accept: "application/vnd.github+json",
Authorization: `Bearer ${githubToken}`,
"X-GitHub-Api-Version": "2022-11-28",
},
});
let baseSha: string;
if (!baseRefResponse.ok) {
// If base branch doesn't exist, try default branch
const repoUrl = `${GITHUB_API_URL}/repos/${owner}/${repo}`;
const repoResponse = await fetch(repoUrl, {
headers: {
Accept: "application/vnd.github+json",
Authorization: `Bearer ${githubToken}`,
"X-GitHub-Api-Version": "2022-11-28",
},
});
if (!repoResponse.ok) {
throw new Error(`Failed to get repository info: ${repoResponse.status}`);
}
const repoData = (await repoResponse.json()) as {
default_branch: string;
};
const defaultBranch = repoData.default_branch;
// Try default branch
const defaultRefUrl = `${GITHUB_API_URL}/repos/${owner}/${repo}/git/refs/heads/${defaultBranch}`;
const defaultRefResponse = await fetch(defaultRefUrl, {
headers: {
Accept: "application/vnd.github+json",
Authorization: `Bearer ${githubToken}`,
"X-GitHub-Api-Version": "2022-11-28",
},
});
if (!defaultRefResponse.ok) {
throw new Error(
`Failed to get default branch reference: ${defaultRefResponse.status}`,
);
}
const defaultRefData = (await defaultRefResponse.json()) as GitHubRef;
baseSha = defaultRefData.object.sha;
} else {
const baseRefData = (await baseRefResponse.json()) as GitHubRef;
baseSha = baseRefData.object.sha;
}
// Create the new branch
const createRefUrl = `${GITHUB_API_URL}/repos/${owner}/${repo}/git/refs`;
const createRefResponse = await fetch(createRefUrl, {
method: "POST",
headers: {
Accept: "application/vnd.github+json",
Authorization: `Bearer ${githubToken}`,
"X-GitHub-Api-Version": "2022-11-28",
"Content-Type": "application/json",
},
body: JSON.stringify({
ref: `refs/heads/${branch}`,
sha: baseSha,
}),
});
if (!createRefResponse.ok) {
const errorText = await createRefResponse.text();
throw new Error(
`Failed to create branch: ${createRefResponse.status} - ${errorText}`,
);
}
console.log(`Successfully created branch ${branch}`);
return baseSha;
}
// Commit files tool
server.tool(
"commit_files",
@@ -195,13 +81,24 @@ server.tool(
return filePath;
});
// 1. Get the branch reference (create if doesn't exist)
const baseSha = await getOrCreateBranchRef(
owner,
repo,
branch,
githubToken,
// 1. Get the branch reference
const refUrl = `${GITHUB_API_URL}/repos/${owner}/${repo}/git/refs/heads/${branch}`;
const refResponse = await fetch(refUrl, {
headers: {
Accept: "application/vnd.github+json",
Authorization: `Bearer ${githubToken}`,
"X-GitHub-Api-Version": "2022-11-28",
},
});
if (!refResponse.ok) {
throw new Error(
`Failed to get branch reference: ${refResponse.status}`,
);
}
const refData = (await refResponse.json()) as GitHubRef;
const baseSha = refData.object.sha;
// 2. Get the base commit
const commitUrl = `${GITHUB_API_URL}/repos/${owner}/${repo}/git/commits/${baseSha}`;
@@ -363,6 +260,7 @@ server.tool(
// Only retry on 403 errors - these are the intermittent failures we're targeting
if (updateRefResponse.status === 403) {
console.log("Received 403 error, will retry...");
throw error;
}
@@ -455,13 +353,24 @@ server.tool(
return filePath;
});
// 1. Get the branch reference (create if doesn't exist)
const baseSha = await getOrCreateBranchRef(
owner,
repo,
branch,
githubToken,
// 1. Get the branch reference
const refUrl = `${GITHUB_API_URL}/repos/${owner}/${repo}/git/refs/heads/${branch}`;
const refResponse = await fetch(refUrl, {
headers: {
Accept: "application/vnd.github+json",
Authorization: `Bearer ${githubToken}`,
"X-GitHub-Api-Version": "2022-11-28",
},
});
if (!refResponse.ok) {
throw new Error(
`Failed to get branch reference: ${refResponse.status}`,
);
}
const refData = (await refResponse.json()) as GitHubRef;
const baseSha = refData.object.sha;
// 2. Get the base commit
const commitUrl = `${GITHUB_API_URL}/repos/${owner}/${repo}/git/commits/${baseSha}`;

View File

@@ -100,7 +100,6 @@ export async function prepareMcpConfig(
REPO_OWNER: owner,
REPO_NAME: repo,
BRANCH_NAME: branch,
BASE_BRANCH: process.env.BASE_BRANCH || "",
REPO_DIR: process.env.GITHUB_WORKSPACE || process.cwd(),
GITHUB_EVENT_NAME: process.env.GITHUB_EVENT_NAME || "",
IS_PR: process.env.IS_PR || "false",

View File

@@ -21,7 +21,6 @@ describe("checkAndCommitOrDeleteBranch", () => {
const createMockOctokit = (
compareResponse?: any,
deleteRefError?: Error,
branchExists: boolean = true,
): Octokits => {
return {
rest: {
@@ -29,14 +28,6 @@ describe("checkAndCommitOrDeleteBranch", () => {
compareCommitsWithBasehead: async () => ({
data: compareResponse || { total_commits: 0 },
}),
getBranch: async () => {
if (!branchExists) {
const error: any = new Error("Not Found");
error.status = 404;
throw error;
}
return { data: {} };
},
},
git: {
deleteRef: async () => {
@@ -111,7 +102,6 @@ describe("checkAndCommitOrDeleteBranch", () => {
compareCommitsWithBasehead: async () => {
throw new Error("API error");
},
getBranch: async () => ({ data: {} }), // Branch exists
},
git: {
deleteRef: async () => ({ data: {} }),
@@ -133,7 +123,7 @@ describe("checkAndCommitOrDeleteBranch", () => {
`\n[View branch](${GITHUB_SERVER_URL}/owner/repo/tree/claude/issue-123-20240101-1234)`,
);
expect(consoleErrorSpy).toHaveBeenCalledWith(
"Error comparing commits on Claude branch:",
"Error checking for commits on Claude branch:",
expect.any(Error),
);
});
@@ -158,30 +148,4 @@ describe("checkAndCommitOrDeleteBranch", () => {
deleteError,
);
});
test("should return no branch link when branch doesn't exist remotely", async () => {
const mockOctokit = createMockOctokit(
{ total_commits: 0 },
undefined,
false, // branch doesn't exist
);
const result = await checkAndCommitOrDeleteBranch(
mockOctokit,
"owner",
"repo",
"claude/issue-123-20240101-1234",
"main",
false,
);
expect(result.shouldDeleteBranch).toBe(false);
expect(result.branchLink).toBe("");
expect(consoleLogSpy).toHaveBeenCalledWith(
"Branch claude/issue-123-20240101-1234 does not exist remotely",
);
expect(consoleLogSpy).toHaveBeenCalledWith(
"Branch claude/issue-123-20240101-1234 does not exist remotely, no branch link will be added",
);
});
});

View File

@@ -1,8 +1,5 @@
import { describe, it, expect } from "bun:test";
import {
updateCommentBody,
type CommentUpdateInput,
} from "../src/github/operations/comment-logic";
import { updateCommentBody } from "../src/github/operations/comment-logic";
describe("updateCommentBody", () => {
const baseInput = {
@@ -420,27 +417,5 @@ describe("updateCommentBody", () => {
"• [Create PR ➔](https://github.com/owner/repo/compare/main...claude/issue-123-20240101-1200)",
);
});
it("should not show branch name when branch doesn't exist remotely", () => {
const input: CommentUpdateInput = {
currentBody: "@claude can you help with this?",
actionFailed: false,
executionDetails: { duration_ms: 90000 },
jobUrl: "https://github.com/owner/repo/actions/runs/123",
branchLink: "", // Empty branch link means branch doesn't exist remotely
branchName: undefined, // Should be undefined when branchLink is empty
triggerUsername: "claude",
prLink: "",
};
const result = updateCommentBody(input);
expect(result).toContain("Claude finished @claude's task in 1m 30s");
expect(result).toContain(
"[View job](https://github.com/owner/repo/actions/runs/123)",
);
expect(result).not.toContain("claude/issue-123");
expect(result).not.toContain("tree/claude/issue-123");
});
});
});