Simplify rename pairing and index scoping

bundle/configsync/blockindex.go

@@ -446,10 +446,8 @@ func (r *blockResolver) blocksDefiningSequence(sequencePath dyn.Path) []sourceBl
 		if err != nil {
 			continue
 		}
-		for _, location := range sequence.Locations() {
-			if location.File == block.file && !slices.Contains(blocks, block) {
-				blocks = append(blocks, block)
-			}
+		if slices.ContainsFunc(sequence.Locations(), func(l dyn.Location) bool { return l.File == block.file }) {
+			blocks = append(blocks, block)
 		}
 	}
 	return blocks

bundle/configsync/rename.go

@@ -54,6 +54,19 @@ func pairRenames(b *bundle.Bundle, blocks *blockResolver, resourceKey string, ch
 
 	removes, adds := keyedElementChanges(changes)
 	for _, remove := range removes {
+		// The remove half carries no value, so the old element is read from the
+		// merged configuration. Resolving it once also gives the sequence steps that
+		// multiBlockElement needs below.
+		resolved, err := resolveSelectors(resourceKey+"."+remove.path, b, OperationRemove)
+		if err != nil || !resolved.leaf.IsValid() {
+			continue
+		}
+		element, ok := resolved.leaf.AsAny().(map[string]any)
+		if !ok {
+			continue
+		}
+		oldFields := withoutKey(element, remove.keyField)
+
 		// Every add the removed element could equally well have become. Two
 		// identically-bodied elements renamed in one run produce two removes that
 		// each match both adds, and the pairing decides which key goes to which
@@ -69,7 +82,7 @@ func pairRenames(b *bundle.Bundle, blocks *blockResolver, resourceKey string, ch
 			if remove.parent != add.parent || remove.keyField != add.keyField {
 				continue
 			}
-			if sameElementApartFromKey(b, resourceKey, remove, add) {
+			if sameElementApartFromKey(oldFields, add) {
 				matches = append(matches, add)
 			}
 		}
@@ -112,14 +125,14 @@ func pairRenames(b *bundle.Bundle, blocks *blockResolver, resourceKey string, ch
 			if _, taken := set.addPaths[add.path]; taken {
 				continue
 			}
-			if sameFieldsApartFromKey(b, resourceKey, remove, add) {
+			if sameFieldsApartFromKey(oldFields, add) {
 				candidates = append(candidates, add.path)
 			}
 		}
 		if len(candidates) == 0 {
 			continue
 		}
-		if !multiBlockElement(b, blocks, resourceKey, remove.path) {
+		if !multiBlockElement(blocks, resolved) {
 			continue
 		}
 		const reason = "a split element cannot be removed and recreated in one run"
@@ -131,14 +144,13 @@ func pairRenames(b *bundle.Bundle, blocks *blockResolver, resourceKey string, ch
 	return set
 }
 
-// multiBlockElement reports whether the element at path is assembled from more
-// than one physical block.
-func multiBlockElement(b *bundle.Bundle, blocks *blockResolver, resourceKey, path string) bool {
-	resolved, err := resolveSelectors(resourceKey+"."+path, b, OperationRemove)
-	if err != nil || len(resolved.steps) == 0 {
+// multiBlockElement reports whether the element the change addresses is assembled
+// from more than one physical block.
+func multiBlockElement(blocks *blockResolver, change resolvedChange) bool {
+	if len(change.steps) == 0 {
 		return false
 	}
-	last := resolved.steps[len(resolved.steps)-1]
+	last := change.steps[len(change.steps)-1]
 	return len(blocks.blocksOf(last.element)) > 1
 }
 
@@ -184,42 +196,25 @@ func keyedElementChanges(changes ResourceChanges) (removes, adds []keyedElement)
 }
 
 // sameElementApartFromKey reports whether the added element is the removed one
-// with a different key. The remove half carries no value, so the old element is
-// read from the merged configuration.
-func sameElementApartFromKey(b *bundle.Bundle, resourceKey string, remove, add keyedElement) bool {
-	resolved, err := resolveSelectors(resourceKey+"."+remove.path, b, OperationRemove)
-	if err != nil || !resolved.leaf.IsValid() {
-		return false
-	}
-	oldValue, ok := resolved.leaf.AsAny().(map[string]any)
-	if !ok {
-		return false
-	}
+// with a different key. oldFields is the removed element without its key field.
+func sameElementApartFromKey(oldFields map[string]any, add keyedElement) bool {
 	newValue, ok := add.value.(map[string]any)
 	if !ok {
 		return false
 	}
-	return reflect.DeepEqual(withoutKey(oldValue, remove.keyField), withoutKey(newValue, add.keyField))
+	return reflect.DeepEqual(oldFields, withoutKey(newValue, add.keyField))
 }
 
 // sameFieldsApartFromKey reports whether the added element has the same fields as
 // the removed one, ignoring their values. A rename that also edited a field keeps
 // the element's shape; an unrelated new element generally does not.
-func sameFieldsApartFromKey(b *bundle.Bundle, resourceKey string, remove, add keyedElement) bool {
-	resolved, err := resolveSelectors(resourceKey+"."+remove.path, b, OperationRemove)
-	if err != nil || !resolved.leaf.IsValid() {
-		return false
-	}
-	oldValue, ok := resolved.leaf.AsAny().(map[string]any)
-	if !ok {
-		return false
-	}
+func sameFieldsApartFromKey(oldFields map[string]any, add keyedElement) bool {
 	newValue, ok := add.value.(map[string]any)
 	if !ok {
 		return false
 	}
 	return slices.Equal(
-		slices.Sorted(maps.Keys(withoutKey(oldValue, remove.keyField))),
+		slices.Sorted(maps.Keys(oldFields)),
 		slices.Sorted(maps.Keys(withoutKey(newValue, add.keyField))),
 	)
 }

bundle/configsync/resolve.go

@@ -164,6 +164,12 @@ func pathDepth(pathStr string) int {
 	return len(node.AsSlice())
 }
 
+// scopedParent keys index bookkeeping by the sequence a path addresses, within the
+// scope of one block, so operations on one block cannot shift indices in another.
+func scopedParent(scope string, path *structpath.PatternNode) string {
+	return scope + path.Parent().String()
+}
+
 // adjustArrayIndex adjusts the index in a PatternNode based on previous operations.
 // When operations are applied sequentially, removals and additions shift array indices.
 // This function adjusts the index to account for those shifts.
@@ -178,8 +184,7 @@ func adjustArrayIndex(path *structpath.PatternNode, scope string, operations map
 	}
 
 	parentPath := path.Parent()
-	parentPathStr := scope + parentPath.String()
-	ops := operations[parentPathStr]
+	ops := operations[scopedParent(scope, path)]
 
 	adjustment := 0
 	for _, op := range ops {
@@ -348,13 +353,13 @@ func ResolveChanges(ctx context.Context, b *bundle.Bundle, configChanges Changes
 				if destChange.Operation == OperationRemove {
 					freeIndex, ok := resolvedPath.Index()
 					if ok {
-						parentPath := scope + resolvedPath.Parent().String()
+						parentPath := scopedParent(scope, resolvedPath)
 						indicesToReplaceMap[parentPath] = append(indicesToReplaceMap[parentPath], freeIndex)
 					}
 				}
 
 				if destChange.Operation == OperationAdd && resolvedPath.BracketStar() {
-					parentPath := scope + resolvedPath.Parent().String()
+					parentPath := scopedParent(scope, resolvedPath)
 					indices, ok := indicesToReplaceMap[parentPath]
 					if ok && len(indices) > 0 {
 						index := indices[0]
@@ -367,7 +372,7 @@ func ResolveChanges(ctx context.Context, b *bundle.Bundle, configChanges Changes
 
 				// Track this operation for future index adjustments (only for array element operations)
 				if originalIndex, ok := resolvedPath.Index(); ok {
-					parentPath := scope + resolvedPath.Parent().String()
+					parentPath := scopedParent(scope, resolvedPath)
 					indexOperations[parentPath] = append(indexOperations[parentPath], struct {
 						index     int
 						operation OperationType