fix(serve): fetch unreachable revisions by SHA so PR-head commits res…

cli/src/main/kotlin/com/bazel_diff/server/GitClient.kt

@@ -19,8 +19,10 @@ open class GitClientException(message: String, cause: Throwable? = null) :
  * absent from the object database.
  *
  * The query service treats this as *retryable*: it only fetches at startup, so a commit that landed
- * on the remote afterwards is simply not here yet. Callers refetch and retry once before surfacing
- * the failure (see [com.bazel_diff.server.ImpactedTargetsService]).
+ * on the remote afterwards is simply not here yet. Callers refetch and retry before surfacing the
+ * failure -- first a broad [GitClient.fetch], then, for a commit not reachable from any fetched ref
+ * (e.g. a PR-head SHA), a targeted [GitClient.fetchRevision] (see
+ * [com.bazel_diff.server.ImpactedTargetsService]).
  */
 class MissingRevisionException(val revision: String, cause: Throwable? = null) :
     GitClientException("revision '$revision' is missing from the local clone", cause)
@@ -36,6 +38,21 @@ interface GitClient {
   /** Fetches the latest refs from all remotes. Throws [GitClientException] on failure. */
   fun fetch()
 
+  /**
+   * Best-effort targeted fetch of a single [revision] (a SHA or ref name) directly from the
+   * remote(s), for a commit a broad [fetch] does not bring in because it is reachable from no ref
+   * this clone fetches -- e.g. a GitHub PR-head SHA (advertised only under `refs/pull/<n>/head`),
+   * or a commit force-pushed off its branch. Works against servers that permit fetching a reachable
+   * object by SHA (`uploadpack.allowReachableSHA1InWant`, enabled by default on GitHub).
+   *
+   * Returns true if a fetch reported success, false if no remote could supply it (server disallows
+   * by-SHA fetch, or the revision is genuinely unknown). Never throws for an ordinary miss: callers
+   * re-run [resolveSha], the authoritative check, which raises [MissingRevisionException] if the
+   * object is still absent. Defaults to a no-op returning false for clients that cannot target an
+   * individual revision.
+   */
+  fun fetchRevision(revision: String): Boolean = false
+
   /**
    * Resolves a revision (branch, tag, short or full SHA) to a full 40-char commit SHA, verifying
    * that the commit actually exists in the local clone. Throws [MissingRevisionException] if the
@@ -63,6 +80,35 @@ class ProcessGitClient(
     run("fetch", "--all", "--prune")
   }
 
+  override fun fetchRevision(revision: String): Boolean {
+    // A broad `fetch --all` only downloads objects reachable from the refs the refspec covers, so a
+    // commit named by a SHA that no fetched ref reaches -- a GitHub PR-head commit (advertised only
+    // under refs/pull/*), or a commit force-pushed off its branch -- never arrives. Ask each remote
+    // for that exact object: servers that allow reachable-SHA fetches
+    // (uploadpack.allowReachableSHA1InWant, GitHub's default) return it even though no local ref
+    // ends up pointing at it. Best-effort per remote; a refusal or unknown SHA moves on, and the
+    // authoritative miss is left to the caller's resolveSha.
+    for (remote in remoteNames()) {
+      try {
+        logger.i { "git fetch $remote $revision in $workspacePath" }
+        run("fetch", remote, revision)
+        return true
+      } catch (e: GitClientException) {
+        logger.i { "targeted fetch of '$revision' from '$remote' failed: ${e.message}" }
+      }
+    }
+    return false
+  }
+
+  /** Configured remote names (`git remote`); empty when none are configured or the query fails. */
+  private fun remoteNames(): List<String> =
+      try {
+        run("remote").map(String::trim).filter(String::isNotEmpty)
+      } catch (e: GitClientException) {
+        logger.i { "could not list git remotes in $workspacePath: ${e.message}" }
+        emptyList()
+      }
+
   override fun resolveSha(revision: String): String {
     // `rev-parse --verify <rev>^{commit}` both resolves the revision to a commit SHA and confirms
     // that commit is present in the local clone. A bare `rev-parse <full-sha>` echoes any

cli/src/main/kotlin/com/bazel_diff/server/ImpactedTargetsService.kt

@@ -138,34 +138,69 @@ class ImpactedTargetsService(
   }
 
   /**
-   * Resolves both revisions to commit SHAs, refetching once if either is missing from the local
-   * clone. The service only fetches at startup, so a commit that landed on the remote afterwards
-   * (or a branch this clone has not yet seen) would otherwise fail with a stale-clone
-   * [MissingRevisionException]; a single on-demand `git fetch` brings it in.
+   * Resolves both revisions to commit SHAs, fetching on demand if either is missing from the local
+   * clone. The service only fetches at startup, so a revision that appeared on the remote
+   * afterwards would otherwise fail with a stale-clone [MissingRevisionException]. Recovery
+   * escalates in two steps: a broad [GitClient.fetch] (refreshes branch/tag tips -- the common
+   * "commit just landed" case), then, for a revision still missing because it is reachable from no
+   * ref this clone fetches (a GitHub PR-head SHA, or a commit force-pushed off its branch), a
+   * targeted [GitClient.fetchRevision] of that exact object.
    *
    * Fetches are serialized and double-checked through [fetchLock]: concurrent requests for the same
    * new commit wait on the lock, and whoever holds it re-resolves first, so an earlier fetch that
    * already made the revision resolvable saves the rest a redundant fetch. A revision still missing
-   * after the refetch is genuinely unknown (bad SHA, or a ref this clone's refspec doesn't fetch)
-   * and propagates as [MissingRevisionException] -> HTTP 400.
+   * after both steps is genuinely unknown (bad SHA, or an object the remote will not serve) and
+   * propagates as [MissingRevisionException] -> HTTP 400.
    */
   private fun resolveBoth(fromRev: String, toRev: String): Pair<String, String> {
-    try {
-      return gitClient.resolveSha(fromRev) to gitClient.resolveSha(toRev)
-    } catch (missing: MissingRevisionException) {
-      logger.i { "Revision '${missing.revision}' not in local clone; refetching and retrying" }
+    resolveBothOrNull(fromRev, toRev)?.let {
+      return it
     }
     synchronized(fetchLock) {
-      // A concurrent request may have refetched while we waited on the lock; retry before fetching.
-      try {
-        return gitClient.resolveSha(fromRev) to gitClient.resolveSha(toRev)
-      } catch (stillMissing: MissingRevisionException) {
-        gitClient.fetch()
+      // A concurrent request may have fetched while we waited on the lock; retry before fetching.
+      resolveBothOrNull(fromRev, toRev)?.let {
+        return it
+      }
+      // Broad fetch first: refreshes branch/tag tips, covering the common case of a commit that
+      // landed on the remote after startup.
+      logger.i { "Revision missing from local clone; fetching all refs and retrying" }
+      gitClient.fetch()
+      resolveBothOrNull(fromRev, toRev)?.let {
+        return it
+      }
+      // A broad fetch only downloads objects reachable from the refs this clone's refspec covers. A
+      // revision still missing is not among them -- a GitHub PR-head SHA (advertised under
+      // refs/pull/*), or a commit force-pushed off its branch. Ask the remote for each such object
+      // by SHA directly before giving up.
+      for (rev in linkedSetOf(fromRev, toRev)) {
+        if (!resolvable(rev)) {
+          logger.i { "Revision '$rev' unreachable via broad fetch; fetching it directly" }
+          gitClient.fetchRevision(rev)
+        }
       }
     }
+    // Authoritative: a revision still unresolved now is genuinely unknown and propagates as a
+    // MissingRevisionException -> HTTP 400.
     return gitClient.resolveSha(fromRev) to gitClient.resolveSha(toRev)
   }
 
+  /** Resolves both revisions, or null if either is still missing from the local clone. */
+  private fun resolveBothOrNull(fromRev: String, toRev: String): Pair<String, String>? =
+      try {
+        gitClient.resolveSha(fromRev) to gitClient.resolveSha(toRev)
+      } catch (missing: MissingRevisionException) {
+        null
+      }
+
+  /** True if [rev] resolves to a commit present in the local clone. */
+  private fun resolvable(rev: String): Boolean =
+      try {
+        gitClient.resolveSha(rev)
+        true
+      } catch (missing: MissingRevisionException) {
+        false
+      }
+
   // Synthetic //external:* labels are not buildable in bzlmod-only mode; mirror the
   // get-impacted-targets default of excluding them when Bzlmod is enabled (issue #326).
   private fun excludeExternalTargets(): Boolean = bazelModService.isBzlmodEnabled

cli/src/main/kotlin/com/bazel_diff/server/JGitClient.kt

@@ -74,6 +74,21 @@ class JGitClient(
     }
   }
 
+  override fun fetchRevision(revision: String): Boolean {
+    // Fetching an object by SHA is a negotiation JGit 5.13 does not reliably perform (it resolves
+    // the ref spec against the remote's advertised refs, and a PR-head or force-pushed SHA is not
+    // advertised). Route it through native git -- the same reason fetch() falls back -- which
+    // supports `git fetch <remote> <sha>` against servers that allow reachable-SHA fetches.
+    // With the fallback disabled the in-process engine cannot supply it, so report that honestly.
+    if (!nativeFetchFallback) {
+      logger.i {
+        "JGit cannot fetch revision '$revision' by SHA and the native fallback is disabled"
+      }
+      return false
+    }
+    return nativeGit.fetchRevision(revision)
+  }
+
   /**
    * Retries a failed JGit fetch with the native `git` binary (unless [nativeFetchFallback] is off,
    * in which case the original JGit failure is surfaced). If native git also fails the two failures

cli/src/test/kotlin/com/bazel_diff/server/GitClientTest.kt

@@ -3,7 +3,9 @@ package com.bazel_diff.server
 import assertk.assertThat
 import assertk.assertions.hasLength
 import assertk.assertions.isEqualTo
+import assertk.assertions.isFalse
 import assertk.assertions.isNotEqualTo
+import assertk.assertions.isTrue
 import com.bazel_diff.SilentLogger
 import com.bazel_diff.log.Logger
 import java.io.File
@@ -26,9 +28,11 @@ class GitClientTest : KoinTest {
   @get:Rule val temp: TemporaryFolder = TemporaryFolder()
 
   /** Runs git for test setup, returning trimmed stdout; fails the test on a non-zero exit. */
-  private fun git(vararg args: String): String {
-    val proc =
-        ProcessBuilder(listOf("git") + args).directory(temp.root).redirectErrorStream(true).start()
+  private fun git(vararg args: String): String = runGit(temp.root, *args)
+
+  /** Like [git] but in an explicit working directory (for multi-repo fetch tests). */
+  private fun runGit(dir: File, vararg args: String): String {
+    val proc = ProcessBuilder(listOf("git") + args).directory(dir).redirectErrorStream(true).start()
     val output = proc.inputStream.readBytes().decodeToString()
     val code = proc.waitFor()
     check(code == 0) { "git ${args.joinToString(" ")} failed ($code): $output" }
@@ -94,4 +98,60 @@ class GitClientTest : KoinTest {
     // `git fetch --all` is a no-op (exit 0) when there are no remotes, so it must not throw.
     ProcessGitClient(temp.root.toPath()).fetch()
   }
+
+  @Test
+  fun fetchRevisionBringsInACommitNoBranchReaches() {
+    // A commit advertised only under refs/pull/* (a GitHub PR head) is reachable from no branch, so
+    // a broad `fetch --all` never brings it in: resolveSha stays missing until fetchRevision pulls
+    // that exact object. file:// forces a real pack transfer (a local-path clone would hardlink
+    // every object and mask the miss).
+    val origin = File(temp.root, "origin").apply { mkdirs() }
+    runGit(origin, "init", "-q")
+    runGit(origin, "config", "user.email", "test@example.com")
+    runGit(origin, "config", "user.name", "test")
+    runGit(origin, "config", "uploadpack.allowReachableSHA1InWant", "true")
+    File(origin, "file.txt").writeText("one")
+    runGit(origin, "add", ".")
+    runGit(origin, "commit", "-q", "-m", "first")
+    val branch = runGit(origin, "rev-parse", "--abbrev-ref", "HEAD")
+
+    // A commit reachable only from refs/pull/7/head, not from any branch.
+    runGit(origin, "checkout", "-q", "--detach")
+    File(origin, "pr.txt").writeText("pr")
+    runGit(origin, "add", ".")
+    runGit(origin, "commit", "-q", "-m", "pr head")
+    val prHead = runGit(origin, "rev-parse", "HEAD")
+    runGit(origin, "update-ref", "refs/pull/7/head", prHead)
+    // Re-attach HEAD to the branch so the clone checks out the branch, not the detached PR commit.
+    runGit(origin, "checkout", "-q", branch)
+
+    val workspace = File(temp.root, "workspace")
+    runGit(temp.root, "clone", "-q", "file://${origin.absolutePath}", workspace.absolutePath)
+    val client = ProcessGitClient(workspace.toPath())
+
+    // A broad fetch does not reach the PR head; resolveSha must still report it missing.
+    client.fetch()
+    assertThrows(MissingRevisionException::class.java) { client.resolveSha(prHead) }
+
+    // The targeted fetch pulls the exact object, after which it resolves.
+    assertThat(client.fetchRevision(prHead)).isTrue()
+    assertThat(client.resolveSha(prHead)).isEqualTo(prHead)
+  }
+
+  @Test
+  fun fetchRevisionReturnsFalseForUnknownSha() {
+    initRepoWithTwoCommits()
+    git("remote", "add", "origin", temp.root.absolutePath)
+    // A SHA no remote can supply: the best-effort targeted fetch reports failure rather than
+    // throwing, leaving the authoritative miss to resolveSha.
+    val absent = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"
+    assertThat(ProcessGitClient(temp.root.toPath()).fetchRevision(absent)).isFalse()
+  }
+
+  @Test
+  fun fetchRevisionReturnsFalseWithoutRemotes() {
+    initRepoWithTwoCommits()
+    // No remotes to ask -> nothing to fetch from -> false, and no throw.
+    assertThat(ProcessGitClient(temp.root.toPath()).fetchRevision("HEAD")).isFalse()
+  }
 }

cli/src/test/kotlin/com/bazel_diff/server/ImpactedTargetsServiceTest.kt

@@ -83,14 +83,20 @@ class ImpactedTargetsServiceTest : KoinTest {
     override fun checkout(revision: String) = Unit
   }
 
-  /** [GitClient] whose [missing] revisions never resolve, even after a fetch. */
+  /** [GitClient] whose [missing] revisions never resolve, even after a broad or targeted fetch. */
   private class AlwaysMissingGitClient(private val missing: Set<String>) : GitClient {
     var fetchCount = 0
+    var fetchRevisionCount = 0
 
     override fun fetch() {
       fetchCount++
     }
 
+    override fun fetchRevision(revision: String): Boolean {
+      fetchRevisionCount++
+      return false
+    }
+
     override fun resolveSha(revision: String): String {
       if (revision in missing) throw MissingRevisionException(revision)
       return revision
@@ -99,6 +105,36 @@ class ImpactedTargetsServiceTest : KoinTest {
     override fun checkout(revision: String) = Unit
   }
 
+  /**
+   * [GitClient] modeling a PR-head SHA: a broad [fetch] never brings [prHead] in (it is on no
+   * fetched ref); only a targeted [fetchRevision] does.
+   */
+  private class DirectFetchGitClient(private val prHead: String) : GitClient {
+    var fetchCount = 0
+    var fetchRevisionCount = 0
+    private var directlyFetched = false
+
+    override fun fetch() {
+      fetchCount++
+    }
+
+    override fun fetchRevision(revision: String): Boolean {
+      fetchRevisionCount++
+      if (revision == prHead) {
+        directlyFetched = true
+        return true
+      }
+      return false
+    }
+
+    override fun resolveSha(revision: String): String {
+      if (revision == prHead && !directlyFetched) throw MissingRevisionException(revision)
+      return revision
+    }
+
+    override fun checkout(revision: String) = Unit
+  }
+
   /**
    * [HashProvider] returning canned data. By default the module-change workspace path is treated as
    * an error (the pure hash-diff path must not touch the workspace); set [allowWorkspaceAt] to
@@ -285,8 +321,8 @@ class ImpactedTargetsServiceTest : KoinTest {
 
   @Test
   fun propagatesMissingRevisionAfterRefetchStillFails() {
-    // A genuinely unknown revision: one refetch is attempted, then the miss propagates (the HTTP
-    // layer maps it to a 400).
+    // A genuinely unknown revision: a broad refetch AND a targeted by-SHA fetch are both attempted,
+    // then the miss propagates (the HTTP layer maps it to a 400).
     val git = AlwaysMissingGitClient(missing = setOf("to-sha"))
     val service = ImpactedTargetsService(git, FakeHashProvider(emptyMap()))
 
@@ -296,5 +332,23 @@ class ImpactedTargetsServiceTest : KoinTest {
         }
     assertThat(ex.revision).isEqualTo("to-sha")
     assertThat(git.fetchCount).isEqualTo(1)
+    assertThat(git.fetchRevisionCount).isEqualTo(1)
+  }
+
+  @Test
+  fun fetchesUnreachableRevisionDirectlyWhenBroadFetchDoesNotBringItIn() {
+    // The `to` SHA is a PR-head commit reachable from no fetched ref, so a broad `git fetch --all`
+    // never brings it in. Rather than 400, the service must fall back to a targeted by-SHA fetch.
+    val from = HashFileData(mapOf("//:a" to TargetHash("Rule", "h1", "d1")), null)
+    val to = HashFileData(mapOf("//:a" to TargetHash("Rule", "h2", "d2")), null)
+    val git = DirectFetchGitClient(prHead = "to-sha")
+    val service =
+        ImpactedTargetsService(git, FakeHashProvider(mapOf("from-sha" to from, "to-sha" to to)))
+
+    val result = service.getImpactedTargets("from-sha", "to-sha", null)
+
+    assertThat(result.impactedTargets).isEqualTo(listOf("//:a"))
+    assertThat(git.fetchCount).isEqualTo(1) // broad fetch tried first
+    assertThat(git.fetchRevisionCount).isEqualTo(1) // then the targeted by-SHA fetch
   }
 }