From 4ad649ebb9f0590d37fd4c71c6ce01110acb3394 Mon Sep 17 00:00:00 2001 From: Colin McDonnell Date: Thu, 14 May 2026 03:44:08 +0000 Subject: [PATCH] action: extend shallow-unreachable deepen-retry to checkout_pr fetches (#734) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit extracts the deepen-retry helper from `GitFetchTool` into shared `$gitFetchWithDeepen` and applies it to every fetch in `checkoutPrBranch` (baseRef, pull/N/head, before_sha temp branch). on shallow clones with deep PR ancestry — the failure mode behind ~10 of 51 `heuristic:very-slow` runs in 24h on `remotion-dev/remotion` — the baseRef fetch was throwing `Could not read ` to the agent before the compare-api deepen block could run. agents then burned 10+ minutes retrying `checkout_pr` and falling back to ad-hoc shell `git fetch --deepen` workarounds. also splits the analyzer's `heuristic:git-error-recovered` into `heuristic:git-shallow-unreachable` and `heuristic:git-shallow-lock` buckets so future audits surface this without manual log-grep. closes #656. --- mcp/checkout.ts | 35 ++++++++++++++++++++++--------- mcp/git.ts | 34 ++---------------------------- utils/gitAuth.ts | 54 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 42 deletions(-) diff --git a/mcp/checkout.ts b/mcp/checkout.ts index ae47dc6..4ce2c26 100644 --- a/mcp/checkout.ts +++ b/mcp/checkout.ts @@ -5,7 +5,7 @@ import type { Octokit, RestEndpointMethodTypes } from "@octokit/rest"; import { type } from "arktype"; import { log } from "../utils/cli.ts"; import { countLines, createDiffCoverageState } from "../utils/diffCoverage.ts"; -import { $git } from "../utils/gitAuth.ts"; +import { $git, $gitFetchWithDeepen } from "../utils/gitAuth.ts"; import { executeLifecycleHook } from "../utils/lifecycle.ts"; import { computeIncrementalDiff } from "../utils/rangeDiff.ts"; import { retry } from "../utils/retry.ts"; @@ -259,10 +259,10 @@ async function ensureBeforeShaReachable(params: EnsureBeforeShaParams): Promise< sha: params.sha, ref: tempBranch, }); - await $git( - "fetch", + await $gitFetchWithDeepen( ["--no-tags", ...(params.isShallow ? ["--depth=1"] : []), "origin", tempBranch], - { token: params.gitToken } + { token: params.gitToken }, + `before_sha temp branch ${tempBranch}` ); log.debug(`» fetched before_sha via temp branch ${tempBranch}`); return true; @@ -410,9 +410,17 @@ export async function checkoutPrBranch( toolState.checkoutSha = $("git", ["rev-parse", "HEAD"], { log: false }).trim(); const alreadyOnBranch = toolState.checkoutSha === pr.headSha; - // fetch base branch so origin/ exists for diff operations + // fetch base branch so origin/ exists for diff operations. + // wrap with deepen-retry: on shallow clones (the actions/checkout default + // is depth=1), repos with deep PR ancestry can't reach the baseRef tip in + // a single round trip, surfacing as `Could not read ` / `remote did + // not send all necessary objects` (issue #656). log.debug(`» fetching base branch (${pr.baseRef})...`); - await $git("fetch", ["--no-tags", "origin", pr.baseRef], { token: gitToken }); + await $gitFetchWithDeepen( + ["--no-tags", "origin", pr.baseRef], + { token: gitToken }, + `base branch ${pr.baseRef}` + ); // alreadyOnBranch only matches for repeated checkout_pr calls for the same PR in one session // (without the tip moving), or if an external setup already checked out the PR head. @@ -426,14 +434,21 @@ export async function checkoutPrBranch( // -B creates or resets the branch to match origin/baseBranch $("git", ["checkout", "-B", pr.baseRef, `origin/${pr.baseRef}`], { log: false }); - // fetch PR branch using pull/{n}/head refspec (works for both fork and same-repo PRs) + // fetch PR branch using pull/{n}/head refspec (works for both fork and same-repo PRs). + // two transient classes wrap this fetch: + // - shallow-unreachable (`Could not read ` etc.) — handled by the + // inner `$gitFetchWithDeepen` deepen-retry (one shot, see issue #656) + // - pull/N/head webhook race (`couldn't find remote ref pull/N/head`) — + // handled by the outer retry below (see issue #591) log.debug(`» fetching PR #${pr.number} (${localBranch})...`); await retry( async () => { try { - await $git("fetch", ["--no-tags", "origin", `+pull/${pr.number}/head:${localBranch}`], { - token: gitToken, - }); + await $gitFetchWithDeepen( + ["--no-tags", "origin", `+pull/${pr.number}/head:${localBranch}`], + { token: gitToken }, + `PR #${pr.number}` + ); } catch (e) { // on the webhook race, check whether the PR still matches what we // dispatched on. if it's been closed/merged or the head SHA moved, diff --git a/mcp/git.ts b/mcp/git.ts index da5cdd4..5e96ba5 100644 --- a/mcp/git.ts +++ b/mcp/git.ts @@ -2,7 +2,7 @@ import { regex } from "arkregex"; import { type } from "arktype"; import type { StoredPushDest } from "../toolState.ts"; import { log } from "../utils/cli.ts"; -import { $git } from "../utils/gitAuth.ts"; +import { $git, $gitFetchWithDeepen } from "../utils/gitAuth.ts"; import { executeLifecycleHook } from "../utils/lifecycle.ts"; import { $ } from "../utils/shell.ts"; import type { ToolContext } from "./server.ts"; @@ -510,21 +510,6 @@ const GitFetch = type({ depth: type.number.describe("Fetch depth (for shallow clones)").optional(), }); -// when an agent-supplied depth is too shallow to reach the merge base, git -// surfaces "Could not read " and "remote did not send all necessary -// objects". detect both wordings so a single deepen retry can recover before -// the error reaches the agent (issue #564). git emits the full OID via -// oid_to_hex, so the bound is 40 (SHA-1) or 64 (SHA-256). -const SHALLOW_UNREACHABLE_PATTERNS: RegExp[] = [ - /Could not read [a-f0-9]{40,64}/, - /remote did not send all necessary objects/, -]; - -// large enough to clear the merge base on most real-world PRs without -// downloading the full history; matches the fallback used by checkoutPrBranch -// when the compare API is unavailable. -const DEEPEN_RETRY_DEPTH = 1000; - export function GitFetchTool(ctx: ToolContext) { return tool({ name: "git_fetch", @@ -538,22 +523,7 @@ export function GitFetchTool(ctx: ToolContext) { if (params.depth !== undefined) { fetchArgs.push(`--depth=${params.depth}`); } - try { - await $git("fetch", fetchArgs, { token: ctx.gitToken }); - } catch (err) { - const msg = err instanceof Error ? err.message : String(err); - const isShallowUnreachable = SHALLOW_UNREACHABLE_PATTERNS.some((p) => p.test(msg)); - const isShallow = - isShallowUnreachable && - $("git", ["rev-parse", "--is-shallow-repository"], { log: false }).trim() === "true"; - if (!isShallow) throw err; - log.info( - `» git_fetch hit shallow-unreachable error, retrying with --deepen=${DEEPEN_RETRY_DEPTH}` - ); - await $git("fetch", [`--deepen=${DEEPEN_RETRY_DEPTH}`, "--no-tags", "origin", params.ref], { - token: ctx.gitToken, - }); - } + await $gitFetchWithDeepen(fetchArgs, { token: ctx.gitToken }, "git_fetch"); return { success: true, ref: params.ref }; }), }); diff --git a/utils/gitAuth.ts b/utils/gitAuth.ts index 8d79bac..20b27ad 100644 --- a/utils/gitAuth.ts +++ b/utils/gitAuth.ts @@ -14,6 +14,7 @@ import { readFileSync, realpathSync, unlinkSync } from "node:fs"; import { log } from "./cli.ts"; import type { GitAuthServer } from "./gitAuthServer.ts"; import { filterEnv } from "./secrets.ts"; +import { $ } from "./shell.ts"; import { spawn } from "./subprocess.ts"; type SafeGitSubcommand = "fetch" | "push"; @@ -181,3 +182,56 @@ export async function $git( } } } + +/** + * shallow-clone unreachable: when an existing local depth is too shallow for + * git to traverse to the requested ref's ancestry, the remote walk fails with + * one of these wordings (git emits the full OID via oid_to_hex, so the bound + * is 40 for SHA-1 or 64 for SHA-256). detecting both lets a single deepen + * retry recover before the error reaches the agent — see issue #564 for the + * original `git_fetch` precedent and #656 for the `checkout_pr` follow-up. + */ +export const SHALLOW_UNREACHABLE_PATTERNS: RegExp[] = [ + /Could not read [a-f0-9]{40,64}/, + /remote did not send all necessary objects/, +]; + +/** + * large enough to clear the merge base on most real-world PRs without + * downloading the full history; matches the fallback used by + * `checkoutPrBranch` when the GitHub compare API is unavailable. + */ +export const DEEPEN_RETRY_DEPTH = 1000; + +/** + * authenticated `git fetch` that recovers from shallow-unreachable errors + * by retrying once with `--deepen=1000`. callers pass the same args they + * would to `$git("fetch", ...)`; on shallow-unreachable failures in a + * shallow repo, the second attempt prepends `--deepen=N` and strips any + * caller-supplied `--depth=` (the two flags are mutually exclusive, and + * the caller's depth is what got us into this mess). + * + * non-shallow-unreachable errors and non-shallow repos rethrow unchanged, + * so this is safe to wrap any fetch without changing fast-path behavior. + */ +export async function $gitFetchWithDeepen( + args: string[], + options: GitAuthOptions, + label?: string +): Promise { + try { + return await $git("fetch", args, options); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + const isShallowUnreachable = SHALLOW_UNREACHABLE_PATTERNS.some((p) => p.test(msg)); + if (!isShallowUnreachable) throw err; + const isShallow = + $("git", ["rev-parse", "--is-shallow-repository"], { log: false }).trim() === "true"; + if (!isShallow) throw err; + log.info( + `» ${label ?? "git fetch"} hit shallow-unreachable error, retrying with --deepen=${DEEPEN_RETRY_DEPTH}` + ); + const retryArgs = args.filter((a) => !a.startsWith("--depth=")); + return await $git("fetch", [`--deepen=${DEEPEN_RETRY_DEPTH}`, ...retryArgs], options); + } +}