From 6f26e3fb34f4cfa21368d76b0c14c81141d73caa Mon Sep 17 00:00:00 2001 From: Brace Sproul Date: Mon, 21 Jul 2025 12:59:29 -0700 Subject: [PATCH] fix: Git issues (#455) * fix: Git issues * switch to using git api only for authed commands * cr * cr * cr * cr --- apps/open-swe/evals/evaluator.ts | 32 +- .../src/graphs/programmer/nodes/open-pr.ts | 1 + .../graphs/programmer/nodes/take-action.ts | 3 + .../reviewer/nodes/take-review-action.ts | 3 + .../src/graphs/shared/initialize-sandbox.ts | 58 +- apps/open-swe/src/security/auth.ts | 6 - apps/open-swe/src/utils/github/git.ts | 613 ++++-------------- apps/open-swe/src/utils/retry.ts | 10 +- apps/open-swe/src/utils/sandbox.ts | 11 +- 9 files changed, 151 insertions(+), 586 deletions(-) diff --git a/apps/open-swe/evals/evaluator.ts b/apps/open-swe/evals/evaluator.ts index 61e98565..5576967a 100644 --- a/apps/open-swe/evals/evaluator.ts +++ b/apps/open-swe/evals/evaluator.ts @@ -177,46 +177,24 @@ export async function evaluator(inputs: { } const daytonaInstance = new Daytona(); + const solutionBranch = output.branchName; logger.info("Creating sandbox...", { repo: openSWEInputs.repo, originalBranch: openSWEInputs.branch, - solutionBranch: output.branchName, + solutionBranch, user_input: openSWEInputs.user_input.substring(0, 100) + "...", }); const sandbox = await daytonaInstance.create(DEFAULT_SANDBOX_CREATE_PARAMS); try { - const res = await cloneRepo(sandbox, output.targetRepository, { + await cloneRepo(sandbox, output.targetRepository, { githubInstallationToken: githubToken, + stateBranchName: solutionBranch, }); - if (res.exitCode !== 0) { - logger.error("Failed to clone repository", { - targetRepository: output.targetRepository, - cloneResult: res, - }); - throw new Error("Failed to clone repository"); - } const absoluteRepoDir = getRepoAbsolutePath(output.targetRepository); - const solutionBranch = output.branchName; - logger.info(`Checking out agent's solution branch: ${solutionBranch}`); - - const checkoutBranchRes = await sandbox.process.executeCommand( - `git checkout ${solutionBranch}`, - absoluteRepoDir, - undefined, - TIMEOUT_SEC, - ); - if (checkoutBranchRes.exitCode !== 0) { - logger.error("Failed to checkout solution branch", { - solutionBranch, - checkoutResult: checkoutBranchRes, - }); - throw new Error(`Failed to checkout solution branch: ${solutionBranch}`); - } - const envSetupSuccess = await setupEnv(sandbox, absoluteRepoDir); if (!envSetupSuccess) { logger.error("Failed to setup environment"); @@ -238,7 +216,7 @@ export async function evaluator(inputs: { mypyScore: analysisResult.mypyScore, repo: openSWEInputs.repo, originalBranch: openSWEInputs.branch, - solutionBranch: output.branchName, + solutionBranch, }); return [ diff --git a/apps/open-swe/src/graphs/programmer/nodes/open-pr.ts b/apps/open-swe/src/graphs/programmer/nodes/open-pr.ts index c106fd06..b30ddca7 100644 --- a/apps/open-swe/src/graphs/programmer/nodes/open-pr.ts +++ b/apps/open-swe/src/graphs/programmer/nodes/open-pr.ts @@ -90,6 +90,7 @@ export async function openPullRequest( sandbox, { branchName, + githubInstallationToken, }, ); } diff --git a/apps/open-swe/src/graphs/programmer/nodes/take-action.ts b/apps/open-swe/src/graphs/programmer/nodes/take-action.ts index 2555b4af..b63ca4f3 100644 --- a/apps/open-swe/src/graphs/programmer/nodes/take-action.ts +++ b/apps/open-swe/src/graphs/programmer/nodes/take-action.ts @@ -31,6 +31,7 @@ import { createInstallDependenciesTool } from "../../../tools/install-dependenci import { createSearchTool } from "../../../tools/search.js"; import { getMcpTools } from "../../../utils/mcp-client.js"; import { shouldDiagnoseError } from "../../../utils/tool-message-error.js"; +import { getGitHubTokensFromConfig } from "../../../utils/github-tokens.js"; const logger = createLogger(LogLevel.INFO, "TakeAction"); @@ -167,12 +168,14 @@ export async function takeAction( logger.info(`Has ${changedFiles.length} changed files. Committing.`, { changedFiles, }); + const { githubInstallationToken } = getGitHubTokensFromConfig(config); branchName = await checkoutBranchAndCommit( config, state.targetRepository, sandbox, { branchName, + githubInstallationToken, }, ); } diff --git a/apps/open-swe/src/graphs/reviewer/nodes/take-review-action.ts b/apps/open-swe/src/graphs/reviewer/nodes/take-review-action.ts index 697eacde..213ee35c 100644 --- a/apps/open-swe/src/graphs/reviewer/nodes/take-review-action.ts +++ b/apps/open-swe/src/graphs/reviewer/nodes/take-review-action.ts @@ -27,6 +27,7 @@ import { getSandboxWithErrorHandling } from "../../../utils/sandbox.js"; import { Command } from "@langchain/langgraph"; import { shouldDiagnoseError } from "../../../utils/tool-message-error.js"; import { filterHiddenMessages } from "../../../utils/message/filter-hidden.js"; +import { getGitHubTokensFromConfig } from "../../../utils/github-tokens.js"; const logger = createLogger(LogLevel.INFO, "TakeReviewAction"); @@ -140,12 +141,14 @@ export async function takeReviewerActions( logger.info(`Has ${changedFiles.length} changed files. Committing.`, { changedFiles, }); + const { githubInstallationToken } = getGitHubTokensFromConfig(config); branchName = await checkoutBranchAndCommit( config, state.targetRepository, sandbox, { branchName, + githubInstallationToken, }, ); } diff --git a/apps/open-swe/src/graphs/shared/initialize-sandbox.ts b/apps/open-swe/src/graphs/shared/initialize-sandbox.ts index 8fe5785b..fc6f1bdf 100644 --- a/apps/open-swe/src/graphs/shared/initialize-sandbox.ts +++ b/apps/open-swe/src/graphs/shared/initialize-sandbox.ts @@ -8,12 +8,7 @@ import { } from "@open-swe/shared/open-swe/types"; import { createLogger, LogLevel } from "../../utils/logger.js"; import { daytonaClient } from "../../utils/sandbox.js"; -import { - checkoutBranch, - cloneRepo, - configureGitUserInRepo, - pullLatestChanges, -} from "../../utils/github/git.js"; +import { cloneRepo, pullLatestChanges } from "../../utils/github/git.js"; import { FAILED_TO_GENERATE_TREE_MESSAGE, getCodebaseTree, @@ -153,8 +148,11 @@ export async function initializeSandbox( const pullChangesRes = await pullLatestChanges( absoluteRepoDir, existingSandbox, + { + githubInstallationToken, + }, ); - if (!pullChangesRes || pullChangesRes.exitCode !== 0) { + if (!pullChangesRes) { emitStepEvent(basePullLatestChangesAction, "skipped"); throw new Error("Failed to pull latest changes."); } @@ -271,7 +269,7 @@ export async function initializeSandbox( { retries: 3, delay: 0 }, ); - if (cloneRepoRes.exitCode !== 0) { + if (cloneRepoRes instanceof Error) { emitStepEvent( baseCloneRepoAction, "error", @@ -279,31 +277,10 @@ export async function initializeSandbox( ); throw new Error("Failed to clone repository."); } + const newBranchName = + typeof cloneRepoRes === "string" ? cloneRepoRes : branchName; emitStepEvent(baseCloneRepoAction, "success"); - // Configuring git user - const configureGitUserActionId = uuidv4(); - const baseConfigureGitUserAction: CustomNodeEvent = { - nodeId: INITIALIZE_NODE_ID, - createdAt: new Date().toISOString(), - actionId: configureGitUserActionId, - action: "Configuring git user", - data: { - status: "pending", - sandboxSessionId: sandbox.id, - branch: branchName, - repo: repoName, - }, - }; - emitStepEvent(baseConfigureGitUserAction, "pending"); - - await configureGitUserInRepo(absoluteRepoDir, sandbox, { - githubInstallationToken, - owner: targetRepository.owner, - repo: targetRepository.repo, - }); - emitStepEvent(baseConfigureGitUserAction, "success"); - // Checking out branch const checkoutBranchActionId = uuidv4(); const baseCheckoutBranchAction: CustomNodeEvent = { @@ -314,24 +291,10 @@ export async function initializeSandbox( data: { status: "pending", sandboxSessionId: sandbox.id, - branch: branchName, + branch: newBranchName, repo: repoName, }, }; - emitStepEvent(baseCheckoutBranchAction, "pending"); - const checkoutBranchRes = await checkoutBranch( - absoluteRepoDir, - branchName, - sandbox, - ); - if (!checkoutBranchRes) { - emitStepEvent( - baseCheckoutBranchAction, - "error", - "Failed to checkout branch. Please check your branch name.", - ); - throw new Error("Failed to checkout branch."); - } emitStepEvent(baseCheckoutBranchAction, "success"); // Generating codebase tree @@ -344,7 +307,7 @@ export async function initializeSandbox( data: { status: "pending", sandboxSessionId: sandbox.id, - branch: branchName, + branch: newBranchName, repo: repoName, }, }; @@ -368,5 +331,6 @@ export async function initializeSandbox( messages: createEventsMessage(), dependenciesInstalled: false, customRules: await getCustomRules(sandbox, absoluteRepoDir), + branchName: newBranchName, }; } diff --git a/apps/open-swe/src/security/auth.ts b/apps/open-swe/src/security/auth.ts index 7472c59f..096ec02b 100644 --- a/apps/open-swe/src/security/auth.ts +++ b/apps/open-swe/src/security/auth.ts @@ -89,12 +89,6 @@ function isRunReq(reqUrl: string): boolean { export const auth = new Auth() .authenticate(async (request: Request) => { const isProd = process.env.NODE_ENV === "production"; - // Disable all requests to the server in prod for now. - if (isProd) { - return new HTTPException(504, { - message: "Open SWE temporarily disabled.", - }) as any; - } if (request.method === "OPTIONS") { return { diff --git a/apps/open-swe/src/utils/github/git.ts b/apps/open-swe/src/utils/github/git.ts index ad55bfe4..b510426f 100644 --- a/apps/open-swe/src/utils/github/git.ts +++ b/apps/open-swe/src/utils/github/git.ts @@ -6,36 +6,13 @@ import { getSandboxErrorFields } from "../sandbox-error-fields.js"; import { getRepoAbsolutePath } from "@open-swe/shared/git"; import { ExecuteResponse } from "@daytonaio/sdk/src/types/ExecuteResponse.js"; -class ExecuteCommandError extends Error { - command: string; - result: string; - exitCode: number; - constructor(command: string, error: ExecuteResponse) { - super("Failed to execute command"); - this.name = "ExecuteCommandError"; - this.command = ExecuteCommandError.cleanCommand(command); - this.result = error.result; - this.exitCode = error.exitCode; - } - - static cleanCommand(command: string): string { - if ( - command.includes("x-access-token:") && - command.includes("@github.com/") - ) { - return command.replace( - /(x-access-token:)([^@]+)(@github\.com\/)/, - "$1ACCESS_TOKEN_REDACTED$3", - ); - } - return command; - } -} - const logger = createLogger(LogLevel.INFO, "GitHub-Git"); -export function getBranchName(config: GraphConfig): string { - const threadId = config.configurable?.thread_id; +export function getBranchName(configOrThreadId: GraphConfig | string): string { + const threadId = + typeof configOrThreadId === "string" + ? configOrThreadId + : configOrThreadId.configurable?.thread_id; if (!threadId) { throw new Error("No thread ID provided"); } @@ -43,319 +20,6 @@ export function getBranchName(config: GraphConfig): string { return `open-swe/${threadId}`; } -export async function checkoutBranch( - absoluteRepoDir: string, - branchName: string, - sandbox: Sandbox, -): Promise { - logger.info(`Checking out branch '${branchName}'...`); - - try { - const getCurrentBranchOutput = await sandbox.process.executeCommand( - "git branch --show-current", - absoluteRepoDir, - undefined, - TIMEOUT_SEC, - ); - - if (getCurrentBranchOutput.exitCode !== 0) { - logger.error(`Failed to get current branch`, { - getCurrentBranchOutput, - }); - } else { - const currentBranch = getCurrentBranchOutput.result.trim(); - if (currentBranch === branchName) { - logger.info(`Already on branch '${branchName}'. No checkout needed.`); - return { - result: `Already on branch ${branchName}`, - exitCode: 0, - }; - } - } - } catch (e) { - const errorFields = getSandboxErrorFields(e); - logger.error(`Failed to get current branch`, { - ...(errorFields && { errorFields }), - ...(e instanceof Error && { - name: e.name, - message: e.message, - stack: e.stack, - }), - }); - return false; - } - - let checkoutCommand: string; - try { - logger.info( - `Checking if branch 'refs/heads/${branchName}' exists using 'git rev-parse --verify --quiet'`, - ); - // Check if branch exists using git rev-parse for robustness - const checkBranchExistsOutput = await sandbox.process.executeCommand( - `git rev-parse --verify --quiet "refs/heads/${branchName}"`, - absoluteRepoDir, - undefined, - TIMEOUT_SEC, - ); - - if (checkBranchExistsOutput.exitCode === 0) { - // Branch exists (rev-parse exit code 0 means success) - checkoutCommand = `git checkout "${branchName}"`; - } else { - // Branch does not exist (rev-parse non-zero exit code) or other error. - // Attempt to create it. - checkoutCommand = `git checkout -b "${branchName}"`; - } - } catch (e: unknown) { - const errorFields = getSandboxErrorFields(e); - if ( - errorFields && - errorFields.exitCode === 1 && - errorFields.result === "" - ) { - checkoutCommand = `git checkout -b "${branchName}"`; - } else { - logger.error(`Error checking if branch exists`, { - ...(e instanceof Error && { - name: e.name, - message: e.message, - stack: e.stack, - }), - }); - return false; - } - } - - try { - const gitCheckoutOutput = await sandbox.process.executeCommand( - checkoutCommand, - absoluteRepoDir, - undefined, - TIMEOUT_SEC, - ); - - if (gitCheckoutOutput.exitCode !== 0) { - logger.error(`Failed to checkout branch`, { - gitCheckoutOutput, - }); - return false; - } - - logger.info(`Checked out branch '${branchName}' successfully.`, { - gitCheckoutOutput, - }); - - return gitCheckoutOutput; - } catch (e) { - const errorFields = getSandboxErrorFields(e); - logger.error(`Error checking out branch`, { - ...(errorFields && { errorFields }), - ...(e instanceof Error && { - name: e.name, - message: e.message, - stack: e.stack, - }), - }); - return false; - } -} - -export async function configureGitUserInRepo( - absoluteRepoDir: string, - sandbox: Sandbox, - args: { - githubInstallationToken: string; - owner: string; - repo: string; - }, -): Promise { - const { githubInstallationToken, owner, repo } = args; - let needsGitConfig = false; - try { - const nameCheck = await sandbox.process.executeCommand( - "git config user.name", - absoluteRepoDir, - undefined, - TIMEOUT_SEC, - ); - const emailCheck = await sandbox.process.executeCommand( - "git config user.email", - absoluteRepoDir, - undefined, - TIMEOUT_SEC, - ); - - if ( - nameCheck.exitCode !== 0 || - nameCheck.result.trim() === "" || - emailCheck.exitCode !== 0 || - emailCheck.result.trim() === "" - ) { - needsGitConfig = true; - } - } catch (checkError) { - logger.warn(`Could not check existing git config, will attempt to set it`, { - ...(checkError instanceof Error && { - name: checkError.name, - message: checkError.message, - stack: checkError.stack, - }), - }); - needsGitConfig = true; - } - - // Configure git to use the token for authentication with GitHub by updating the remote URL - logger.info( - "Configuring git to use token for GitHub authentication via remote URL...", - ); - try { - // Set the remote URL with the token using the provided owner and repo - const setRemoteOutput = await sandbox.process.executeCommand( - `git remote set-url origin https://x-access-token:${githubInstallationToken}@github.com/${owner}/${repo}.git`, - absoluteRepoDir, - undefined, - TIMEOUT_SEC, - ); - - if (setRemoteOutput.exitCode !== 0) { - logger.error(`Failed to set remote URL with token`, { - exitCode: setRemoteOutput.exitCode, - }); - } else { - logger.info("Git remote URL updated with token successfully."); - } - } catch (authError) { - logger.error(`Error configuring git authentication for GitHub`, { - ...(authError instanceof Error && { - name: authError.name, - message: authError.message, - stack: authError.stack, - }), - }); - } - - if (needsGitConfig) { - const botAppName = process.env.GITHUB_APP_NAME; - if (!botAppName) { - logger.error("GITHUB_APP_NAME environment variable is not set."); - throw new Error("GITHUB_APP_NAME environment variable is not set."); - } - const userName = `${botAppName}[bot]`; - const userEmail = `${botAppName}@users.noreply.github.com`; - - const configUserNameOutput = await sandbox.process.executeCommand( - `git config user.name "${userName}"`, - absoluteRepoDir, - undefined, - TIMEOUT_SEC, - ); - if (configUserNameOutput.exitCode !== 0) { - logger.error(`Failed to set git user.name`, { - configUserNameOutput, - }); - } else { - logger.info(`Set git user.name to '${userName}' successfully.`); - } - - const configUserEmailOutput = await sandbox.process.executeCommand( - `git config user.email "${userEmail}"`, - absoluteRepoDir, - undefined, - TIMEOUT_SEC, - ); - if (configUserEmailOutput.exitCode !== 0) { - logger.error(`Failed to set git user.email`, { - configUserEmailOutput, - }); - } else { - logger.info(`Set git user.email to '${userEmail}' successfully.`); - } - } else { - logger.info( - "Git user.name and user.email are already configured in this repository.", - ); - } -} - -export async function commitAll( - absoluteRepoDir: string, - message: string, - sandbox: Sandbox, -): Promise { - try { - const gitAddOutput = await sandbox.process.executeCommand( - `git add -A && git commit -m "${message}"`, - absoluteRepoDir, - undefined, - TIMEOUT_SEC, - ); - - if (gitAddOutput.exitCode !== 0) { - logger.error(`Failed to commit all changes to git repository`, { - gitAddOutput, - }); - } - return gitAddOutput; - } catch (e) { - const errorFields = getSandboxErrorFields(e); - logger.error(`Failed to commit all changes to git repository`, { - ...(errorFields && { errorFields }), - ...(e instanceof Error && { - name: e.name, - message: e.message, - stack: e.stack, - }), - }); - return false; - } -} - -export async function commitAllAndPush( - absoluteRepoDir: string, - message: string, - sandbox: Sandbox, -): Promise { - try { - const commitOutput = await commitAll(absoluteRepoDir, message, sandbox); - logger.info( - "Committed changes to git repository successfully. Now pushing...", - ); - const pushCurrentBranchCmd = - "git push -u origin $(git rev-parse --abbrev-ref HEAD)"; - - if (!commitOutput || commitOutput.exitCode !== 0) { - return false; - } - - const gitPushOutput = await sandbox.process.executeCommand( - pushCurrentBranchCmd, - absoluteRepoDir, - undefined, - TIMEOUT_SEC, - ); - - if (gitPushOutput.exitCode !== 0) { - logger.error(`Failed to push changes to git repository`, { - gitPushOutput, - }); - return false; - } - - return gitPushOutput; - } catch (e) { - const errorFields = getSandboxErrorFields(e); - logger.error(`Failed to commit all and push changes to git repository`, { - ...(errorFields && { errorFields }), - ...(e instanceof Error && { - name: e.name, - message: e.message, - stack: e.stack, - }), - }); - return false; - } -} - export async function getChangedFilesStatus( absoluteRepoDir: string, sandbox: Sandbox, @@ -416,19 +80,37 @@ export async function checkoutBranchAndCommit( config: GraphConfig, targetRepository: TargetRepository, sandbox: Sandbox, - options?: { + options: { branchName?: string; + githubInstallationToken: string; }, ): Promise { - logger.info("Checking out branch and committing changes..."); const absoluteRepoDir = getRepoAbsolutePath(targetRepository); - const branchName = options?.branchName || getBranchName(config); - - await checkoutBranch(absoluteRepoDir, branchName, sandbox); + const branchName = options.branchName || getBranchName(config); logger.info(`Committing changes to branch ${branchName}`); - await commitAllAndPush(absoluteRepoDir, "Apply patch", sandbox); - logger.info("Successfully checked out & committed changes."); + // Commit the changes. We can use the sandbox executeCommand API for this since it doesn't require a token. + await sandbox.git.add(absoluteRepoDir, ["."]); + + const botAppName = process.env.GITHUB_APP_NAME; + if (!botAppName) { + logger.error("GITHUB_APP_NAME environment variable is not set."); + throw new Error("GITHUB_APP_NAME environment variable is not set."); + } + const userName = `${botAppName}[bot]`; + const userEmail = `${botAppName}@users.noreply.github.com`; + await sandbox.git.commit(absoluteRepoDir, "Apply patch", userName, userEmail); + + // Push the changes using the git API so it handles authentication for us. + await sandbox.git.push( + absoluteRepoDir, + "git", + options.githubInstallationToken, + ); + + logger.info("Successfully checked out & committed changes.", { + commitAuthor: userName, + }); return branchName; } @@ -436,15 +118,17 @@ export async function checkoutBranchAndCommit( export async function pullLatestChanges( absoluteRepoDir: string, sandbox: Sandbox, -): Promise { + args: { + githubInstallationToken: string; + }, +): Promise { try { - const gitPullOutput = await sandbox.process.executeCommand( - "git pull", + await sandbox.git.pull( absoluteRepoDir, - undefined, - TIMEOUT_SEC, + "git", + args.githubInstallationToken, ); - return gitPullOutput; + return true; } catch (e) { const errorFields = getSandboxErrorFields(e); logger.error(`Failed to pull latest changes`, { @@ -459,6 +143,10 @@ export async function pullLatestChanges( } } +/** + * Securely clones a GitHub repository using temporary credential helper. + * The GitHub installation token is never persisted in the Git configuration or remote URLs. + */ export async function cloneRepo( sandbox: Sandbox, targetRepository: TargetRepository, @@ -466,148 +154,91 @@ export async function cloneRepo( githubInstallationToken: string; stateBranchName?: string; }, -) { +): Promise { const absoluteRepoDir = getRepoAbsolutePath(targetRepository); - let cloneResult: ExecuteResponse | null = null; + const cloneUrl = `https://github.com/${targetRepository.owner}/${targetRepository.repo}.git`; + const branchName = args.stateBranchName || targetRepository.branch; try { - const gitCloneCommand = ["git", "clone"]; - - // Use x-access-token format for better GitHub authentication - const repoUrlWithToken = `https://x-access-token:${args.githubInstallationToken}@github.com/${targetRepository.owner}/${targetRepository.repo}.git`; - - const branchName = args.stateBranchName || targetRepository.branch; - if (branchName) { - gitCloneCommand.push("-b", branchName, repoUrlWithToken); - } else { - gitCloneCommand.push(repoUrlWithToken); - } - - logger.info("Cloning repository", { - repoPath: `${targetRepository.owner}/${targetRepository.repo}`, - branch: branchName, - baseCommit: targetRepository.baseCommit, - cloneCommand: ExecuteCommandError.cleanCommand(gitCloneCommand.join(" ")), - }); - - cloneResult = await sandbox.process.executeCommand( - gitCloneCommand.join(" "), - undefined, - undefined, - TIMEOUT_SEC * 2, // two min timeout since large repos can take a while to clone - ); - - if (!targetRepository.baseCommit) { - if (cloneResult.exitCode !== 0) { - if (!cloneResult.result.includes("not found in upstream origin")) { - logger.error("Failed to clone repository", { - targetRepository, - }); - throw new ExecuteCommandError(gitCloneCommand.join(" "), cloneResult); - } else { - const cloneDefaultBranchCommand = ["git", "clone", repoUrlWithToken]; - logger.info( - "Branch not found in upstream origin. Cloning default & checking out branch", - { - targetRepository, - cloneDefaultBranchCommand: ExecuteCommandError.cleanCommand( - cloneDefaultBranchCommand.join(" "), - ), - }, - ); - const cloneDefaultBranchResult = await sandbox.process.executeCommand( - cloneDefaultBranchCommand.join(" "), - undefined, - undefined, - TIMEOUT_SEC * 2, // two min timeout since large repos can take a while to clone - ); - if (cloneDefaultBranchResult.exitCode !== 0) { - logger.error("Failed to clone default branch", { - targetRepository, - cloneDefaultBranchCommand: ExecuteCommandError.cleanCommand( - cloneDefaultBranchCommand.join(" "), - ), - }); - throw new ExecuteCommandError( - cloneDefaultBranchCommand.join(" "), - cloneDefaultBranchResult, - ); - } - - cloneResult = cloneDefaultBranchResult; - - // Now checkout the branch. We're creating a new branch here since the above error indicated the branch doesn't exist. - const checkoutBranchCommand = ["git", "checkout", "-b", branchName]; - const checkoutBranchResult = await sandbox.process.executeCommand( - checkoutBranchCommand.join(" "), - absoluteRepoDir, - undefined, - TIMEOUT_SEC, - ); - if (checkoutBranchResult.exitCode !== 0) { - logger.error("Failed to checkout branch", { - targetRepository, - checkoutBranchCommand: checkoutBranchCommand.join(" "), - }); - throw new ExecuteCommandError( - checkoutBranchCommand.join(" "), - checkoutBranchResult, - ); - } - - logger.info("Successfully checked out branch", { - targetRepository, - checkoutBranchCommand: checkoutBranchCommand.join(" "), - }); - } - } - return cloneResult; - } - } catch (e) { - const errorFields = getSandboxErrorFields(e); - logger.error("Clone repo failed\n", errorFields ?? e); - throw e; - } - - try { - // If a baseCommit is specified, checkout that commit after cloning - logger.info("Checking out base commit", { - baseCommit: targetRepository.baseCommit, - repoPath: `${targetRepository.owner}/${targetRepository.repo}`, - }); - - const checkoutCommitCommand = [ - "git", - "checkout", - targetRepository.baseCommit, - ]; - const checkoutResult = await sandbox.process.executeCommand( - checkoutCommitCommand.join(" "), + // Attempt to clone the repository + return await performClone(sandbox, cloneUrl, { + branchName, + targetRepository, absoluteRepoDir, - undefined, - TIMEOUT_SEC, - ); - - if (checkoutResult.exitCode !== 0) { - logger.error("Failed to checkout base commit", { - baseCommit: targetRepository.baseCommit, - checkoutCommitCommand: checkoutCommitCommand.join(" "), - }); - throw new ExecuteCommandError( - checkoutCommitCommand.join(" "), - checkoutResult, - ); - } - - logger.info("Successfully checked out base commit", { - baseCommit: targetRepository.baseCommit, - checkoutCommitCommand: checkoutCommitCommand.join(" "), + githubInstallationToken: args.githubInstallationToken, }); - } catch (e) { - const errorFields = getSandboxErrorFields(e); - logger.error("Clone repo failed\n", errorFields ?? e); - throw e; + } catch (error) { + const errorFields = getSandboxErrorFields(error); + logger.error("Clone repo failed", errorFields ?? error); + throw error; + } +} + +/** + * Performs the actual Git clone operation, handling branch-specific logic. + * Returns the branch name that was cloned. + */ +async function performClone( + sandbox: Sandbox, + cloneUrl: string, + args: { + branchName: string | undefined; + targetRepository: TargetRepository; + absoluteRepoDir: string; + githubInstallationToken: string; + }, +): Promise { + const { + branchName, + targetRepository, + absoluteRepoDir, + githubInstallationToken, + } = args; + logger.info("Cloning repository", { + repoPath: `${targetRepository.owner}/${targetRepository.repo}`, + branch: branchName, + baseCommit: targetRepository.baseCommit, + }); + + await sandbox.git.clone( + cloneUrl, + absoluteRepoDir, + undefined, + targetRepository.baseCommit, + "git", + githubInstallationToken, + ); + logger.info("Successfully cloned repository", { + repoPath: `${targetRepository.owner}/${targetRepository.repo}`, + branch: branchName, + baseCommit: targetRepository.baseCommit, + }); + + if (targetRepository.baseCommit) { + return targetRepository.baseCommit; } - return cloneResult; + if (!branchName) { + throw new Error( + "Can not create new branch or checkout existing branch without branch name", + ); + } + + try { + await sandbox.git.createBranch(absoluteRepoDir, branchName); + logger.info("Created branch", { + branch: branchName, + }); + return branchName; + } catch { + logger.info("Failed to create branch, checking out branch", { + branch: branchName, + }); + } + + await sandbox.git.checkoutBranch(absoluteRepoDir, branchName); + logger.info("Checked out branch", { + branch: branchName, + }); + return branchName; } diff --git a/apps/open-swe/src/utils/retry.ts b/apps/open-swe/src/utils/retry.ts index d5156d53..f57f6fcd 100644 --- a/apps/open-swe/src/utils/retry.ts +++ b/apps/open-swe/src/utils/retry.ts @@ -12,10 +12,10 @@ interface RetryOptions { export async function withRetry( fn: () => Promise, options: RetryOptions = {}, -): Promise { +): Promise { const { retries = 3, delay = 0 } = options; - let lastError: Error; + let lastError: Error | undefined; for (let attempt = 0; attempt <= retries; attempt++) { try { @@ -24,7 +24,7 @@ export async function withRetry( lastError = error instanceof Error ? error : new Error(String(error)); if (attempt === retries) { - throw lastError; + return lastError; } if (delay > 0) { @@ -33,7 +33,7 @@ export async function withRetry( } } - throw lastError!; + return lastError; } /** @@ -45,6 +45,6 @@ export async function withRetry( export function createRetryWrapper( fn: (...args: T) => Promise, options: RetryOptions = {}, -): (...args: T) => Promise { +): (...args: T) => Promise { return (...args: T) => withRetry(() => fn(...args), options); } diff --git a/apps/open-swe/src/utils/sandbox.ts b/apps/open-swe/src/utils/sandbox.ts index 39a94848..dc048264 100644 --- a/apps/open-swe/src/utils/sandbox.ts +++ b/apps/open-swe/src/utils/sandbox.ts @@ -3,8 +3,7 @@ import { createLogger, LogLevel } from "./logger.js"; import { GraphConfig, TargetRepository } from "@open-swe/shared/open-swe/types"; import { DEFAULT_SANDBOX_CREATE_PARAMS } from "../constants.js"; import { getGitHubTokensFromConfig } from "./github-tokens.js"; -import { cloneRepo, configureGitUserInRepo } from "./github/git.js"; -import { getRepoAbsolutePath } from "@open-swe/shared/git"; +import { cloneRepo } from "./github/git.js"; import { FAILED_TO_GENERATE_TREE_MESSAGE, getCodebaseTree } from "./tree.js"; const logger = createLogger(LogLevel.INFO, "Sandbox"); @@ -155,14 +154,6 @@ export async function getSandboxWithErrorHandling( stateBranchName: branchName, }); - // Configure git user - const absoluteRepoDir = getRepoAbsolutePath(targetRepository); - await configureGitUserInRepo(absoluteRepoDir, sandbox, { - githubInstallationToken, - owner: targetRepository.owner, - repo: targetRepository.repo, - }); - // Get codebase tree const codebaseTree = await getCodebaseTree(sandbox.id, targetRepository); const codebaseTreeToReturn =