fix(itg): stabilize cycle hashing for BUG-028

core/itg/graph/BUILD.bazel

@@ -29,5 +29,6 @@ go_test(
         "@com_github_bazelbuild_buildtools//build_proto",
         "@com_github_stretchr_testify//assert",
         "@com_github_stretchr_testify//require",
+        "@net_pgregory_rapid//:rapid",
     ],
 )

core/itg/graph/update.go

@@ -94,19 +94,44 @@ func (g *OptimizedGraph) UpdateGraph(
 		return err
 	}
 
-	// prioritize computing hashes of targets that could create cycles
+	return g.computeInvalidatedHashes(ctx, allInvalidated)
+}
+
+// computeInvalidatedHashes computes invalidated target hashes in the same
+// deterministic traversal order used by full graph hashing.
+func (g *OptimizedGraph) computeInvalidatedHashes(ctx context.Context, invalidated IntSet) error {
+	// Prioritize configured targets that could create cycles.
 	for _, target := range sequentialHashTargets {
 		id, ok := g.TargetNameToID[target]
-		if !ok || !allInvalidated.Contains(id) {
+		if !ok || !invalidated.Contains(id) {
+			continue
+		}
+		if _, err := g.computeHashes(ctx, id); err != nil {
+			return err
+		}
+	}
+
+	ids := invalidated.UnsortedList()
+	slices.SortStableFunc(ids, func(i, j int) int {
+		return strings.Compare(g.TargetIDToString[i], g.TargetIDToString[j])
+	})
+
+	// Full graph hashing starts from lexicographically sorted topological roots.
+	// Doing the same here preserves its cycle entry points for rooted cycles.
+	for _, id := range ids {
+		target, ok := g.OptimizedTargets[id]
+		if !ok || len(target.ReverseDeps) != 0 {
 			continue
 		}
 		if _, err := g.computeHashes(ctx, id); err != nil {
 			return err
 		}
 	}
 
-	// update hashes for invalidated targets
-	for id := range allInvalidated {
+	// Any remaining invalidated targets are either already hashed dependencies or
+	// root-unreachable cycles. The first lexicographic member is their canonical
+	// cycle breaker, matching full graph hashing's cyclic-target fallback.
+	for _, id := range ids {
 		if _, err := g.computeHashes(ctx, id); err != nil {
 			return err
 		}

core/itg/graph/update_test.go

@@ -23,6 +23,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/uber/tango/core/targethasher"
+	"pgregory.net/rapid"
 )
 
 // fakeSourceHasher is a test double for targethasher.SourceHasher.
@@ -232,3 +233,96 @@ func TestComputeHashes(t *testing.T) {
 		assert.Equal(t, h.Sum(nil), got)
 	})
 }
+
+func TestComputeInvalidatedHashesCycleOrderInvariance(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name       string
+		newTargets func() map[string]*targethasher.Target
+		fullRoot   string
+	}{
+		{
+			name: "rooted cycle matches full recomputation",
+			newTargets: func() map[string]*targethasher.Target {
+				return map[string]*targethasher.Target{
+					"//pkg:a": {
+						Name:            "//pkg:a",
+						RuleType:        "go_library",
+						HashWithoutDeps: []byte("a"),
+						Deps:            []string{"//pkg:b"},
+					},
+					"//pkg:b": {
+						Name:            "//pkg:b",
+						RuleType:        "go_library",
+						HashWithoutDeps: []byte("b"),
+						Deps:            []string{"//pkg:a"},
+					},
+					"//pkg:root": {
+						Name:            "//pkg:root",
+						RuleType:        "go_library",
+						HashWithoutDeps: []byte("root"),
+						Deps:            []string{"//pkg:b"},
+					},
+				}
+			},
+			fullRoot: "//pkg:root",
+		},
+		{
+			name: "rootless cycle uses canonical member",
+			newTargets: func() map[string]*targethasher.Target {
+				return map[string]*targethasher.Target{
+					"//pkg:a": {
+						Name:            "//pkg:a",
+						RuleType:        "go_library",
+						HashWithoutDeps: []byte("a"),
+						Deps:            []string{"//pkg:b"},
+					},
+					"//pkg:b": {
+						Name:            "//pkg:b",
+						RuleType:        "go_library",
+						HashWithoutDeps: []byte("b"),
+						Deps:            []string{"//pkg:a"},
+					},
+				}
+			},
+			fullRoot: "//pkg:a",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			expectedTargets := tt.newTargets()
+			_, err := targethasher.HashRecursively(t.Context(), targethasher.HashParam{
+				Targets:    expectedTargets,
+				TargetName: tt.fullRoot,
+			})
+			require.NoError(t, err)
+
+			names := make([]string, 0, len(expectedTargets))
+			expectedHashes := make(map[string][]byte, len(expectedTargets))
+			for name, target := range expectedTargets {
+				names = append(names, name)
+				expectedHashes[name] = target.Hash
+			}
+
+			ctx := t.Context()
+			rapid.Check(t, func(rt *rapid.T) {
+				graph := OptimizeGraph(tt.newTargets())
+				invalidated := NewIntSet()
+				for _, name := range rapid.Permutation(names).Draw(rt, "invalidation-order") {
+					invalidated.Insert(graph.TargetNameToID[name])
+				}
+
+				err := graph.computeInvalidatedHashes(ctx, invalidated)
+				require.NoError(rt, err)
+				for name, expected := range expectedHashes {
+					actual := graph.OptimizedTargets[graph.TargetNameToID[name]].Hash
+					assert.Equal(rt, expected, actual, "hash mismatch for %s", name)
+				}
+			})
+		})
+	}
+}