From f41b1b083cf4b0ab879115018b4676d46756c465 Mon Sep 17 00:00:00 2001 From: Belal Taher Date: Fri, 26 Jun 2026 13:50:32 -0400 Subject: [PATCH 1/2] WIP: make CCA finalize push resilient to non-fast-forward and local hooks The Engine SDK finalize push (commitAndPush -> finalizeChanges) ran a bare "git push --set-upstream origin HEAD" with no rebase/retry and surfaced only Node's generic "Command failed" message, dropping git's rejection reason. On follow-up CCA jobs the runner can check out a base behind the live branch tip, so the finalize push is rejected non-fast-forward and fails silently. This change: - rebases the agent's commit onto origin/ and retries the push when a push is rejected non-fast-forward (fetch first / behind); - surfaces git's captured stdout/stderr so the real reason is logged instead of a bare "Command failed"; - adds --no-verify to commit/push as defense-in-depth so automated CCA pushes are not blocked by the repository's local git hooks (matches the runtime error path, which already pushes with noVerify). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/git.ts | 88 +++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 81 insertions(+), 7 deletions(-) diff --git a/src/git.ts b/src/git.ts index 2111850..e1683b0 100644 --- a/src/git.ts +++ b/src/git.ts @@ -176,25 +176,99 @@ export function cloneRepo(options: CloneRepoOptions): string { // Commit and Push // ============================================================================= +/** + * Runs a git command capturing stdout/stderr. On failure, throws an Error whose + * message includes git's combined output so the real reason (e.g. a rejected + * push) is preserved rather than the bare "Command failed: git ..." string. + */ +function git(args: string[], repoLocation: string): string { + try { + return execFileSync("git", args, { + cwd: repoLocation, + encoding: "utf-8", + stdio: ["pipe", "pipe", "pipe"], + }); + } catch (error) { + throw new Error(`git ${args.join(" ")} failed: ${gitErrorOutput(error)}`); + } +} + +/** Extracts git's combined stdout/stderr (falling back to the error message) from a thrown exec error. */ +function gitErrorOutput(error: unknown): string { + const e = error as { stderr?: string | Buffer; stdout?: string | Buffer; message?: string } | null; + const stderr = e?.stderr ? e.stderr.toString().trim() : ""; + const stdout = e?.stdout ? e.stdout.toString().trim() : ""; + const combined = [stdout, stderr].filter(Boolean).join("\n"); + return combined || e?.message || String(error); +} + +/** True when a push was rejected because the remote branch has advanced past our local tip. */ +function isNonFastForwardError(output: string): boolean { + return /\(fetch first\)|\(non-fast-forward\)|\[rejected\]|Updates were rejected|tip of your current branch is behind/i.test( + output, + ); +} + +/** + * Pushes the current HEAD to origin. If the push is rejected because the remote + * branch is ahead (non-fast-forward), fetches and rebases onto the remote tip, + * then retries. Any other failure is rethrown with git's output preserved. + * + * `--no-verify` bypasses the repository's local pre-push/pre-commit hooks. + * CCA finalization is a non-interactive safety net and should not run arbitrary + * repo hooks (e.g. husky / lint-staged); this matches the runtime error path, + * which already pushes with `noVerify`. It is defense-in-depth — the primary + * resilience here is the non-fast-forward rebase/retry below. + */ +function pushWithRebaseFallback(repoLocation: string): void { + try { + git(["push", "--no-verify", "--set-upstream", "origin", "HEAD"], repoLocation); + return; + } catch (error) { + const output = error instanceof Error ? error.message : String(error); + if (!isNonFastForwardError(output)) { + throw error; + } + console.log("[Engine SDK] Push rejected because the remote branch is ahead; fetching and rebasing..."); + } + + const branch = git(["rev-parse", "--abbrev-ref", "HEAD"], repoLocation).trim(); + git(["fetch", "origin", branch], repoLocation); + + try { + git(["rebase", "--no-verify", `origin/${branch}`], repoLocation); + } catch (rebaseError) { + try { + execFileSync("git", ["rebase", "--abort"], { cwd: repoLocation, stdio: "pipe" }); + } catch { + // best-effort cleanup + } + throw rebaseError; + } + + git(["push", "--no-verify", "--set-upstream", "origin", "HEAD"], repoLocation); +} + /** * Stages all changes, commits with the given message, and pushes to origin. * If there are no changes to commit, only pushes (to catch any prior local commits). + * + * Commits and pushes use `--no-verify` so the repository's local git hooks do not + * block automated CCA pushes, and the push transparently rebases onto the remote + * branch if it has advanced. */ export function commitAndPush(repoLocation: string, commitMessage: string): CommitAndPushResult { - const status = execFileSync("git", ["status", "--porcelain"], { - cwd: repoLocation, - encoding: "utf-8", - }).trim(); + const status = git(["status", "--porcelain"], repoLocation).trim(); let hadChanges = false; if (status) { hadChanges = true; - execFileSync("git", ["add", "."], { cwd: repoLocation }); - execFileSync("git", ["commit", "-m", commitMessage], { cwd: repoLocation }); + git(["add", "."], repoLocation); + git(["commit", "--no-verify", "-m", commitMessage], repoLocation); } - execFileSync("git", ["push", "--set-upstream", "origin", "HEAD"], { cwd: repoLocation }); + pushWithRebaseFallback(repoLocation); return { success: true, From 52629e02fb69c152e7074458932315cb02f36724 Mon Sep 17 00:00:00 2001 From: Belal Taher Date: Sat, 27 Jun 2026 20:44:58 -0400 Subject: [PATCH 2/2] remove no-verify --- src/git.ts | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/git.ts b/src/git.ts index e1683b0..29ad399 100644 --- a/src/git.ts +++ b/src/git.ts @@ -214,15 +214,14 @@ function isNonFastForwardError(output: string): boolean { * branch is ahead (non-fast-forward), fetches and rebases onto the remote tip, * then retries. Any other failure is rethrown with git's output preserved. * - * `--no-verify` bypasses the repository's local pre-push/pre-commit hooks. - * CCA finalization is a non-interactive safety net and should not run arbitrary - * repo hooks (e.g. husky / lint-staged); this matches the runtime error path, - * which already pushes with `noVerify`. It is defense-in-depth — the primary - * resilience here is the non-fast-forward rebase/retry below. + * The repository's local git hooks (e.g. pre-push) are intentionally left + * enabled so we never silently bypass checks the repository owner configured. + * The resilience here is the non-fast-forward rebase/retry below, not skipping + * hooks. */ function pushWithRebaseFallback(repoLocation: string): void { try { - git(["push", "--no-verify", "--set-upstream", "origin", "HEAD"], repoLocation); + git(["push", "--set-upstream", "origin", "HEAD"], repoLocation); return; } catch (error) { const output = error instanceof Error ? error.message : String(error); @@ -236,7 +235,7 @@ function pushWithRebaseFallback(repoLocation: string): void { git(["fetch", "origin", branch], repoLocation); try { - git(["rebase", "--no-verify", `origin/${branch}`], repoLocation); + git(["rebase", `origin/${branch}`], repoLocation); } catch (rebaseError) { try { execFileSync("git", ["rebase", "--abort"], { cwd: repoLocation, stdio: "pipe" }); @@ -246,16 +245,16 @@ function pushWithRebaseFallback(repoLocation: string): void { throw rebaseError; } - git(["push", "--no-verify", "--set-upstream", "origin", "HEAD"], repoLocation); + git(["push", "--set-upstream", "origin", "HEAD"], repoLocation); } /** * Stages all changes, commits with the given message, and pushes to origin. * If there are no changes to commit, only pushes (to catch any prior local commits). * - * Commits and pushes use `--no-verify` so the repository's local git hooks do not - * block automated CCA pushes, and the push transparently rebases onto the remote - * branch if it has advanced. + * The push transparently rebases onto the remote branch if it has advanced. The + * repository's local git hooks are left enabled so automated CCA pushes do not + * bypass checks the repository owner configured. */ export function commitAndPush(repoLocation: string, commitMessage: string): CommitAndPushResult { const status = git(["status", "--porcelain"], repoLocation).trim(); @@ -265,7 +264,7 @@ export function commitAndPush(repoLocation: string, commitMessage: string): Comm if (status) { hadChanges = true; git(["add", "."], repoLocation); - git(["commit", "--no-verify", "-m", commitMessage], repoLocation); + git(["commit", "-m", commitMessage], repoLocation); } pushWithRebaseFallback(repoLocation);