Fix #197: auto-expand fineGrainedHashExternalRepos through bzlmod wrappers

cli/src/main/kotlin/com/bazel_diff/bazel/ModuleGraphParser.kt

@@ -69,6 +69,107 @@ class ModuleGraphParser {
     }
   }
 
+  /**
+   * Result of parsing the bzlmod dependency graph as edges keyed by `apparentName`.
+   *
+   * @property edges Map from a module's `apparentName` to the set of `apparentName`s of its direct
+   *   bzlmod dependencies. Populated for every module reached when walking the JSON tree.
+   * @property rootApparentNames `apparentName`s for the root module(s) (key `<root>`). The root is
+   *   the main repo and is always queried via `//...:all-targets`, so dependents-expansion should
+   *   never add it to a fine-grained set.
+   */
+  data class GraphEdges(
+      val edges: Map<String, Set<String>>,
+      val rootApparentNames: Set<String>,
+  )
+
+  /**
+   * Parses `bazel mod graph --output=json` into apparent-name dependency edges. Tolerates a
+   * non-JSON prefix (e.g. leaked stderr) using the same recovery as [parseModuleGraph].
+   *
+   * Each module in the JSON tree contributes an edge for every entry in its `dependencies` array.
+   * `unexpanded` dependency stubs (modules that the bzlmod resolver already described elsewhere
+   * in the graph) still contribute the edge but are not recursed into to avoid duplicate work.
+   */
+  fun parseModuleGraphEdges(json: String): GraphEdges {
+    val edges = mutableMapOf<String, MutableSet<String>>()
+    val rootApparentNames = mutableSetOf<String>()
+    try {
+      val root =
+          try {
+            JsonParser.parseString(json).asJsonObject
+          } catch (_: Exception) {
+            val start = json.indexOf('{')
+            if (start < 0) return GraphEdges(emptyMap(), emptySet())
+            JsonParser.parseString(json.substring(start)).asJsonObject
+          }
+      walkEdges(root, edges, rootApparentNames, isRoot = true)
+    } catch (_: Exception) {
+      return GraphEdges(emptyMap(), emptySet())
+    }
+    return GraphEdges(edges.mapValues { it.value.toSet() }, rootApparentNames.toSet())
+  }
+
+  private fun walkEdges(
+      obj: JsonObject,
+      edges: MutableMap<String, MutableSet<String>>,
+      rootApparentNames: MutableSet<String>,
+      isRoot: Boolean,
+  ) {
+    val apparentName = obj.get("apparentName")?.asString ?: return
+    if (isRoot) rootApparentNames.add(apparentName)
+    val mySet = edges.getOrPut(apparentName) { mutableSetOf() }
+    val deps = obj.get("dependencies")?.asJsonArray ?: return
+    for (dep in deps) {
+      if (!dep.isJsonObject) continue
+      val depObj = dep.asJsonObject
+      val depApparent = depObj.get("apparentName")?.asString ?: continue
+      mySet.add(depApparent)
+      val unexpanded = depObj.get("unexpanded")?.asBoolean ?: false
+      if (!unexpanded) {
+        walkEdges(depObj, edges, rootApparentNames, isRoot = false)
+      }
+    }
+  }
+
+  /**
+   * Computes the set of `apparentName`s that transitively depend on any module in [targets] by
+   * traversing [edges] in reverse. Excludes [rootApparentNames] from the result.
+   *
+   * Background: `--fineGrainedHashExternalRepos` opts a leaf module (e.g. `@inner_repo`) into
+   * per-target hashing. When another bzlmod module (`@middle_repo`) wraps it via alias or
+   * re-export and the main repo depends only on the wrapper, the wrapper's targets must also be
+   * queried for the hash chain to reach the leaf. This is the engine for that auto-expansion
+   * (issue #197). The set returned here is what the caller should add to the user-supplied set.
+   */
+  fun findTransitiveDependents(
+      edges: Map<String, Set<String>>,
+      targets: Set<String>,
+      rootApparentNames: Set<String>,
+  ): Set<String> {
+    if (edges.isEmpty() || targets.isEmpty()) return emptySet()
+    val reverse = mutableMapOf<String, MutableSet<String>>()
+    for ((from, deps) in edges) {
+      for (dep in deps) {
+        reverse.getOrPut(dep) { mutableSetOf() }.add(from)
+      }
+    }
+    val out = mutableSetOf<String>()
+    val visited = targets.toMutableSet()
+    val queue: ArrayDeque<String> = ArrayDeque(targets)
+    while (queue.isNotEmpty()) {
+      val cur = queue.removeFirst()
+      val parents = reverse[cur] ?: continue
+      for (p in parents) {
+        if (p in visited) continue
+        visited.add(p)
+        if (p !in rootApparentNames) out.add(p)
+        queue.addLast(p)
+      }
+    }
+    return out
+  }
+
   /**
    * Compares two module graphs and returns the keys of modules that changed.
    *

cli/src/main/kotlin/com/bazel_diff/di/Modules.kt

@@ -3,6 +3,7 @@ package com.bazel_diff.di
 import com.bazel_diff.bazel.BazelClient
 import com.bazel_diff.bazel.BazelModService
 import com.bazel_diff.bazel.BazelQueryService
+import com.bazel_diff.bazel.ModuleGraphParser
 import com.bazel_diff.hash.*
 import com.bazel_diff.io.ContentHashProvider
 import com.bazel_diff.log.Logger
@@ -40,11 +41,26 @@ fun hasherModule(
         "Error: fineGrainedHashExternalReposFile and fineGrainedHashExternalRepos are mutually exclusive - please provide only one of them")
     System.exit(1)
   }
-  val updatedFineGrainedHashExternalRepos =
+  val userSuppliedFineGrainedHashExternalRepos =
       fineGrainedHashExternalReposFile?.let { file ->
         file.readLines().filter { it.isNotBlank() }.toSet()
       } ?: fineGrainedHashExternalRepos
 
+  // Auto-expand the user-supplied fine-grained set with any bzlmod module that transitively
+  // wraps one of those repos (issue #197). Without this, a main-repo target consuming
+  // `@middle_repo//:wrapped` -- itself an alias for `@inner_repo//:all_files` -- collapses
+  // `@middle_repo` into an opaque `//external:middle_repo` blob, so source changes inside
+  // `@inner_repo` never reach the main consumer. Adding the wrapper to the fine-grained set
+  // keeps `@middle_repo//:wrapped` as a real ruleInput whose `actual` attribute carries the
+  // chain down to the leaf.
+  val updatedFineGrainedHashExternalRepos =
+      expandFineGrainedHashExternalReposWithBzlmodDependents(
+          userSuppliedFineGrainedHashExternalRepos,
+          workingDirectory,
+          bazelPath,
+          startupOptions,
+      )
+
   val cmd: MutableList<String> =
       ArrayList<String>().apply {
         add(bazelPath.toString())
@@ -87,6 +103,65 @@ fun hasherModule(
   single { ContentHashProvider(contentHashPath) }
 }
 
+/**
+ * Returns [userSupplied] together with every bzlmod module that transitively depends on a repo in
+ * [userSupplied]. Each entry is the apparent-name form prefixed with `@`, matching the format
+ * consumed by [BazelClient]/[com.bazel_diff.bazel.BazelRule] and the user-facing CLI flag.
+ *
+ * If `bazel mod graph --output=json` fails (e.g. bzlmod disabled) or the user supplied nothing,
+ * this is a no-op and returns [userSupplied] unchanged. This intentionally degrades gracefully
+ * for WORKSPACE-only setups where the bzlmod graph is unavailable -- the wrapped-repo bug
+ * (issue #197) is bzlmod-specific anyway.
+ */
+@OptIn(ExperimentalCoroutinesApi::class)
+private fun expandFineGrainedHashExternalReposWithBzlmodDependents(
+    userSupplied: Set<String>,
+    workingDirectory: Path,
+    bazelPath: Path,
+    startupOptions: List<String>,
+): Set<String> {
+  if (userSupplied.isEmpty()) return userSupplied
+
+  val cmd =
+      mutableListOf<String>().apply {
+        add(bazelPath.toString())
+        addAll(startupOptions)
+        add("mod")
+        add("graph")
+        add("--output=json")
+      }
+  val result =
+      runBlocking {
+        process(
+            *cmd.toTypedArray(),
+            stdout = Redirect.CAPTURE,
+            stderr = Redirect.SILENT,
+            workingDirectory = workingDirectory.toFile(),
+            destroyForcibly = true,
+        )
+      }
+  if (result.resultCode != 0) return userSupplied
+  val json = result.output.joinToString("\n").trim()
+  if (json.isEmpty()) return userSupplied
+
+  val parser = ModuleGraphParser()
+  val graph = parser.parseModuleGraphEdges(json)
+  if (graph.edges.isEmpty()) return userSupplied
+
+  // The user-supplied set uses the `@apparentName` form; strip leading `@`/`@@` to match the
+  // apparent names emitted by `bazel mod graph`.
+  val apparentTargets =
+      userSupplied
+          .map { it.removePrefix("@@").removePrefix("@") }
+          .filter { it.isNotEmpty() }
+          .toSet()
+  val dependents =
+      parser.findTransitiveDependents(graph.edges, apparentTargets, graph.rootApparentNames)
+  if (dependents.isEmpty()) return userSupplied
+
+  return userSupplied + dependents.map { "@$it" }
+}
+
 fun loggingModule(verbose: Boolean) = module { single<Logger> { StderrLogger(verbose) } }
 
 fun serialisationModule() = module {

cli/src/test/kotlin/com/bazel_diff/bazel/ModuleGraphParserTest.kt

@@ -274,6 +274,128 @@ class ModuleGraphParserTest {
     assertThat(result).containsExactlyInAnyOrder("root", "abseil-cpp@20240116.2")
   }
 
+  // Edge / transitive-dependents tests are regression coverage for issue #197: when
+  // `@inner_repo` is the user-listed fine-grained repo and `@middle_repo` wraps it, the
+  // expansion has to follow `bazel mod graph` backwards to add `@middle_repo` automatically.
+
+  @Test
+  fun parseModuleGraphEdges_realIssue197Shape_extractsEdgesAndRoot() {
+    // Mirrors `bazel mod graph --output=json` for the `wrapped_external_repo` fixture:
+    // <root> -> inner_repo, middle_repo; middle_repo -> inner_repo (unexpanded reference).
+    val json =
+        """
+      {
+        "key": "<root>",
+        "name": "wrapped_external_repo_test",
+        "version": "0.0.0",
+        "apparentName": "wrapped_external_repo_test",
+        "dependencies": [
+          {
+            "key": "inner_repo@_",
+            "name": "inner_repo",
+            "version": "0.0.0",
+            "apparentName": "inner_repo",
+            "dependencies": []
+          },
+          {
+            "key": "middle_repo@_",
+            "name": "middle_repo",
+            "version": "0.0.0",
+            "apparentName": "middle_repo",
+            "dependencies": [
+              {
+                "key": "inner_repo@_",
+                "name": "inner_repo",
+                "version": "0.0.0",
+                "apparentName": "inner_repo",
+                "unexpanded": true
+              }
+            ]
+          }
+        ]
+      }
+    """
+            .trimIndent()
+
+    val graph = parser.parseModuleGraphEdges(json)
+
+    assertThat(graph.rootApparentNames).containsExactlyInAnyOrder("wrapped_external_repo_test")
+    assertThat(graph.edges["wrapped_external_repo_test"])
+        .isNotNull()
+        .containsExactlyInAnyOrder("inner_repo", "middle_repo")
+    assertThat(graph.edges["middle_repo"])
+        .isNotNull()
+        .containsExactlyInAnyOrder("inner_repo")
+    // inner_repo has no outgoing deps.
+    assertThat(graph.edges["inner_repo"]).isNotNull().isEmpty()
+  }
+
+  @Test
+  fun findTransitiveDependents_issue197_addsMiddleRepoButNotRoot() {
+    // Same shape as parseModuleGraphEdges_realIssue197Shape: <root> wraps both, middle_repo wraps
+    // inner_repo. Listing `inner_repo` as a target must surface `middle_repo` (so wrapper-only
+    // consumers in the main repo get correctly invalidated) and must NOT surface the root
+    // (the root is the main repo, already queried via `//...:all-targets`).
+    val edges =
+        mapOf(
+            "wrapped_external_repo_test" to setOf("inner_repo", "middle_repo"),
+            "middle_repo" to setOf("inner_repo"),
+            "inner_repo" to emptySet(),
+        )
+
+    val result =
+        parser.findTransitiveDependents(
+            edges,
+            targets = setOf("inner_repo"),
+            rootApparentNames = setOf("wrapped_external_repo_test"))
+
+    assertThat(result).containsExactlyInAnyOrder("middle_repo")
+  }
+
+  @Test
+  fun findTransitiveDependents_multiLevelWrapping_followsChain() {
+    // inner -> middle -> outer -> root. Listing inner must add both middle and outer.
+    val edges =
+        mapOf(
+            "root_app" to setOf("outer"),
+            "outer" to setOf("middle"),
+            "middle" to setOf("inner"),
+            "inner" to emptySet(),
+        )
+
+    val result =
+        parser.findTransitiveDependents(
+            edges, targets = setOf("inner"), rootApparentNames = setOf("root_app"))
+
+    assertThat(result).containsExactlyInAnyOrder("middle", "outer")
+  }
+
+  @Test
+  fun findTransitiveDependents_unrelatedRepos_returnsEmpty() {
+    // A repo with no chain to the targets should not be expanded into.
+    val edges =
+        mapOf(
+            "root_app" to setOf("inner", "unrelated"),
+            "inner" to emptySet(),
+            "unrelated" to emptySet(),
+        )
+
+    val result =
+        parser.findTransitiveDependents(
+            edges, targets = setOf("inner"), rootApparentNames = setOf("root_app"))
+
+    assertThat(result).isEmpty()
+  }
+
+  @Test
+  fun findTransitiveDependents_emptyTargets_returnsEmpty() {
+    val edges = mapOf("a" to setOf("b"), "b" to emptySet())
+
+    val result = parser.findTransitiveDependents(edges, targets = emptySet(), rootApparentNames = emptySet())
+
+    assertThat(result).isEmpty()
+  }
+
   @Test
   fun findChangedModules_withNewGraphEmpty_returnsAllOldModuleKeys() {
     val oldGraph =