fix(serve): resolve missing revisions via on-demand refetch

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

@@ -10,9 +10,21 @@ import org.koin.core.component.KoinComponent
 import org.koin.core.component.inject
 
 /** Thrown when a `git` subprocess exits non-zero or produces unusable output. */
-class GitClientException(message: String, cause: Throwable? = null) :
+open class GitClientException(message: String, cause: Throwable? = null) :
     RuntimeException(message, cause)
 
+/**
+ * Thrown when a revision cannot be found in the local clone -- either it did not resolve at all, or
+ * it resolved (a full 40-char SHA always parses to an ObjectId) but the underlying commit object is
+ * 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]).
+ */
+class MissingRevisionException(val revision: String, cause: Throwable? = null) :
+    GitClientException("revision '$revision' is missing from the local clone", cause)
+
 /**
  * Thin wrapper over a `git` binary (assumed on `$PATH`) scoped to a single workspace clone. The
  * query service checks out the workspace at the revisions it is asked about; see
@@ -25,8 +37,10 @@ interface GitClient {
   fun fetch()
 
   /**
-   * Resolves a revision (branch, tag, short or full SHA) to a full 40-char commit SHA. Throws
-   * [GitClientException] if the revision cannot be resolved.
+   * 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
+   * revision does not resolve or its object is absent (retryable via [fetch]); throws
+   * [GitClientException] for other failures (e.g. the revision names a non-commit object).
    */
   fun resolveSha(revision: String): String
 
@@ -50,9 +64,19 @@ class ProcessGitClient(
   }
 
   override fun resolveSha(revision: String): String {
-    val output = run("rev-parse", revision)
+    // `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
+    // well-formed 40-char SHA straight back without consulting the object database, so a commit
+    // that isn't here would "resolve" and only fail later at checkout. A non-zero exit means the
+    // revision is absent (or not a commit) locally -- retryable via a refetch.
+    val output =
+        try {
+          run("rev-parse", "--verify", "$revision^{commit}")
+        } catch (e: GitClientException) {
+          throw MissingRevisionException(revision, e)
+        }
     return output.firstOrNull()?.trim()?.takeIf { it.isNotEmpty() }
-        ?: throw GitClientException("git rev-parse $revision produced no output")
+        ?: throw MissingRevisionException(revision)
   }
 
   override fun checkout(revision: String) {

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

@@ -72,13 +72,17 @@ class ImpactedTargetsService(
   private val bazelModService: BazelModService by inject()
   private val logger: Logger by inject()
 
+  // Serializes on-demand refetches so a burst of requests for a just-landed
+  // commit triggers at most one `git fetch` at a time (and skips it entirely
+  // once an earlier fetch has already brought the commit in).
+  private val fetchLock = Any()
+
   override fun getImpactedTargets(
       fromRev: String,
       toRev: String,
       targetTypes: Set<String>?
   ): ImpactedTargetsResult {
-    val fromSha = gitClient.resolveSha(fromRev)
-    val toSha = gitClient.resolveSha(toRev)
+    val (fromSha, toSha) = resolveBoth(fromRev, toRev)
     logger.i { "Computing impacted targets $fromSha -> $toSha" }
 
     val fromData = hashProvider.getHashes(fromSha)
@@ -110,8 +114,7 @@ class ImpactedTargetsService(
       throw DistancesUnavailableException(
           "distances unavailable: server started without --trackDeps")
     }
-    val fromSha = gitClient.resolveSha(fromRev)
-    val toSha = gitClient.resolveSha(toRev)
+    val (fromSha, toSha) = resolveBoth(fromRev, toRev)
     logger.i { "Computing impacted targets with distances $fromSha -> $toSha" }
 
     val fromData = hashProvider.getHashes(fromSha)
@@ -134,6 +137,35 @@ class ImpactedTargetsService(
     return ImpactedTargetsWithDistancesResult(fromSha, toSha, impacted)
   }
 
+  /**
+   * 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.
+   *
+   * 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.
+   */
+  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" }
+    }
+    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()
+      }
+    }
+    return gitClient.resolveSha(fromRev) to gitClient.resolveSha(toRev)
+  }
+
   // 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

@@ -3,6 +3,9 @@ package com.bazel_diff.server
 import com.bazel_diff.log.Logger
 import java.nio.file.Path
 import org.eclipse.jgit.api.Git
+import org.eclipse.jgit.errors.IncorrectObjectTypeException
+import org.eclipse.jgit.errors.MissingObjectException
+import org.eclipse.jgit.revwalk.RevWalk
 import org.eclipse.jgit.storage.file.FileRepositoryBuilder
 import org.eclipse.jgit.transport.RemoteConfig
 import org.koin.core.component.KoinComponent
@@ -53,10 +56,22 @@ class JGitClient(private val workspacePath: Path) : GitClient, KoinComponent {
 
   override fun resolveSha(revision: String): String {
     open().use { git ->
-      val objectId =
-          git.repository.resolve(revision)
-              ?: throw GitClientException("could not resolve revision '$revision'")
-      return objectId.name
+      val objectId = git.repository.resolve(revision) ?: throw MissingRevisionException(revision)
+      // Repository.resolve() turns a full 40-char SHA into an ObjectId by parsing the
+      // hex WITHOUT consulting the object database, so a commit absent from this clone
+      // still "resolves". Parse it through a RevWalk to confirm the object is present
+      // (peeling annotated tags to their commit), so an absent commit becomes a
+      // retryable MissingRevisionException here instead of an opaque "Missing unknown
+      // <sha>" failure deferred to the later checkout.
+      val commit =
+          try {
+            RevWalk(git.repository).use { walk -> walk.parseCommit(objectId) }
+          } catch (e: MissingObjectException) {
+            throw MissingRevisionException(revision, e)
+          } catch (e: IncorrectObjectTypeException) {
+            throw GitClientException("revision '$revision' does not refer to a commit", e)
+          }
+      return commit.name
     }
   }
 

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

@@ -78,6 +78,16 @@ class GitClientTest : KoinTest {
     assertThrows(GitClientException::class.java) { client.resolveSha("no-such-ref") }
   }
 
+  @Test
+  fun resolveShaThrowsMissingRevisionForAbsentFullSha() {
+    initRepoWithTwoCommits()
+    val client = ProcessGitClient(temp.root.toPath())
+    // `git rev-parse <full-sha>` echoes any well-formed SHA back even when absent; resolveSha must
+    // use `--verify` so a commit that isn't in the clone is rejected as a retryable miss.
+    val absent = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"
+    assertThrows(MissingRevisionException::class.java) { client.resolveSha(absent) }
+  }
+
   @Test
   fun fetchDoesNotThrowWithoutRemotes() {
     initRepoWithTwoCommits()

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

@@ -45,6 +45,60 @@ class ImpactedTargetsServiceTest : KoinTest {
     override fun checkout(revision: String) = Unit
   }
 
+  /** [GitClient] whose [missingUntilFetch] revisions resolve only after [fetch] has run once. */
+  private class RefetchGitClient(private val missingUntilFetch: Set<String>) : GitClient {
+    var fetchCount = 0
+    private var fetched = false
+
+    override fun fetch() {
+      fetchCount++
+      fetched = true
+    }
+
+    override fun resolveSha(revision: String): String {
+      if (!fetched && revision in missingUntilFetch) throw MissingRevisionException(revision)
+      return revision
+    }
+
+    override fun checkout(revision: String) = Unit
+  }
+
+  /**
+   * [GitClient] that fails the first resolve of [flaky] and succeeds on the next WITHOUT a fetch --
+   * models another request having refetched while this one waited on the fetch lock.
+   */
+  private class ResolvesOnSecondAttemptGitClient(private val flaky: String) : GitClient {
+    var fetchCount = 0
+    private var attempts = 0
+
+    override fun fetch() {
+      fetchCount++
+    }
+
+    override fun resolveSha(revision: String): String {
+      if (revision == flaky && ++attempts == 1) throw MissingRevisionException(revision)
+      return revision
+    }
+
+    override fun checkout(revision: String) = Unit
+  }
+
+  /** [GitClient] whose [missing] revisions never resolve, even after a fetch. */
+  private class AlwaysMissingGitClient(private val missing: Set<String>) : GitClient {
+    var fetchCount = 0
+
+    override fun fetch() {
+      fetchCount++
+    }
+
+    override fun resolveSha(revision: String): String {
+      if (revision in missing) 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
@@ -196,4 +250,51 @@ class ImpactedTargetsServiceTest : KoinTest {
     assertThat(result.impactedTargets).containsExactly(ImpactedTargetWithDistance("//:a", 0, 0))
     assertThat(provider.workspaceAtCalls).isEqualTo(listOf("to-sha"))
   }
+
+  @Test
+  fun refetchesOnceWhenRevisionMissingThenResolves() {
+    // The `to` commit isn't in the local clone yet (it landed after the last fetch); the service
+    // must fetch on demand and retry rather than surfacing the miss as a 400.
+    val from = HashFileData(mapOf("//:a" to TargetHash("Rule", "h1", "d1")), null)
+    val to = HashFileData(mapOf("//:a" to TargetHash("Rule", "h2", "d2")), null)
+    val git = RefetchGitClient(missingUntilFetch = setOf("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)
+  }
+
+  @Test
+  fun skipsRefetchWhenRevisionAppearsUnderTheLock() {
+    // A concurrent request already refetched: the double-check re-resolve under the lock succeeds,
+    // so this request must not issue its own redundant 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 = ResolvesOnSecondAttemptGitClient(flaky = "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(0)
+  }
+
+  @Test
+  fun propagatesMissingRevisionAfterRefetchStillFails() {
+    // A genuinely unknown revision: one refetch is 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()))
+
+    val ex =
+        org.junit.Assert.assertThrows(MissingRevisionException::class.java) {
+          service.getImpactedTargets("from-sha", "to-sha", null)
+        }
+    assertThat(ex.revision).isEqualTo("to-sha")
+    assertThat(git.fetchCount).isEqualTo(1)
+  }
 }

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

@@ -4,6 +4,7 @@ import assertk.assertThat
 import assertk.assertions.hasLength
 import assertk.assertions.isEqualTo
 import assertk.assertions.isNotEqualTo
+import assertk.assertions.messageContains
 import com.bazel_diff.SilentLogger
 import com.bazel_diff.log.Logger
 import java.io.File
@@ -26,9 +27,11 @@ class JGitClientTest : 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" }
@@ -109,4 +112,51 @@ class JGitClientTest : KoinTest {
     initRepoWithTwoCommits()
     assertThrows(GitClientException::class.java) { client().checkout("does-not-exist") }
   }
+
+  @Test
+  fun resolveShaThrowsMissingRevisionForAbsentFullSha() {
+    initRepoWithTwoCommits()
+    // A well-formed 40-char SHA that is not in the object database. Repository.resolve() parses the
+    // hex without a DB lookup, so resolveSha must detect the absence itself and flag it retryable.
+    val absent = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"
+    assertThrows(MissingRevisionException::class.java) { client().resolveSha(absent) }
+  }
+
+  @Test
+  fun resolveShaThrowsWhenRevisionIsNotACommit() {
+    initRepoWithTwoCommits()
+    // A tree object is present but is not a commit; resolveSha must reject it as a hard error, not
+    // as a retryable MissingRevisionException (a refetch would never make a tree checkoutable).
+    val treeSha = git("rev-parse", "HEAD^{tree}")
+    val ex = assertThrows(GitClientException::class.java) { client().resolveSha(treeSha) }
+    assertThat(ex).messageContains("does not refer to a commit")
+  }
+
+  @Test
+  fun resolveShaSeesCommitOnlyAfterFetch() {
+    // Reproduces the reported serve failure: a commit that lands on the remote AFTER the clone is
+    // absent locally, so resolveSha rejects it (rather than deferring an opaque checkout error)
+    // until an on-demand fetch brings it in.
+    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")
+    File(origin, "file.txt").writeText("one")
+    runGit(origin, "add", ".")
+    runGit(origin, "commit", "-q", "-m", "first")
+
+    val workspace = File(temp.root, "workspace")
+    runGit(temp.root, "clone", "-q", origin.absolutePath, workspace.absolutePath)
+
+    // A new commit lands in origin after the clone; the workspace has not fetched it yet.
+    File(origin, "file.txt").writeText("two")
+    runGit(origin, "add", ".")
+    runGit(origin, "commit", "-q", "-m", "second")
+    val sha2 = runGit(origin, "rev-parse", "HEAD")
+
+    val client = JGitClient(workspace.toPath())
+    assertThrows(MissingRevisionException::class.java) { client.resolveSha(sha2) }
+    client.fetch()
+    assertThat(client.resolveSha(sha2)).isEqualTo(sha2)
+  }
 }