fix: fetch many-relationship peers before reconciliation

changelog/+168-many-relationship-fetch.fixed.md

@@ -0,0 +1 @@
+Fetch uninitialized cardinality-many relationships before comparing their existing peers during an Infrahub update, so stale remote peers are removed correctly.

infrahub_sync/adapters/infrahub.py

@@ -165,6 +165,8 @@ def update_node(
 
                 elif rel_schema.cardinality == "many":
                     attr_manager: RelationshipManagerSync = getattr(node, attr_name)
+                    if not attr_manager.initialized:
+                        attr_manager.fetch()
                     existing_peer_ids = attr_manager.peer_ids
                     new_peer_ids = []
 
@@ -182,9 +184,6 @@ def update_node(
 
                     _, existing_only, new_only = compare_lists(existing_peer_ids, new_peer_ids)
 
-                    if not attr_manager.initialized:
-                        attr_manager.fetch()
-
                     for existing_id in existing_only:
                         attr_manager.remove(existing_id)
 

tests/adapters/test_infrahub_update_node_attribution.py

@@ -93,6 +93,32 @@ def remove(self, peer_id: str) -> None:
         self.removed.append(peer_id)
 
 
+class LazyFakeRelManager(FakeRelManager):
+    """Manager whose destination peers are unavailable until the SDK fetches them."""
+
+    def __init__(self, remote_ids: list[str]) -> None:
+        super().__init__()
+        self.initialized = False
+        self._remote_ids = list(remote_ids)
+        self.fetch_count = 0
+
+    def fetch(self) -> None:
+        self.fetch_count += 1
+        self.peer_ids = list(self._remote_ids)
+        self.initialized = True
+
+    def add(self, data: object) -> None:
+        super().add(data)
+        if isinstance(data, dict):
+            peer_id = cast("dict[str, object]", data).get("id")
+            if isinstance(peer_id, str) and peer_id not in self.peer_ids:
+                self.peer_ids.append(peer_id)
+
+    def remove(self, peer_id: str) -> None:
+        super().remove(peer_id)
+        self.peer_ids.remove(peer_id)
+
+
 class FakeNode:
     """Stand-in for ``InfrahubNodeSync`` exposing only what ``update_node`` reads."""
 
@@ -315,3 +341,22 @@ def test_update_node_relationship_many_no_attribution_when_unset(patch_resolve_p
     _run_update(node, {"tags": ["t1-uid"]})
 
     assert manager.added == [{"id": "t1-uid"}]
+
+
+def test_update_node_fetches_many_relationship_before_reconciling_peers(patch_resolve_peer: None) -> None:  # noqa: ARG001
+    """SYNC-38: existing peers must be fetched before current and desired IDs are compared."""
+    rel = FakeRelSchema(name="tags", peer="BuiltinTag", cardinality="many")
+    schema = FakeSchema(relationships=[rel], relationship_names=["tags"])
+    manager = LazyFakeRelManager(remote_ids=["a-uid", "b-uid"])
+    node = FakeNode(
+        schema=schema,
+        client=FakeClient(peers={"BuiltinTag": object()}),
+        many_managers={"tags": manager},
+    )
+
+    _run_update(node, {"tags": ["a-uid", "c-uid"]}, source=SOURCE_ID, owner=OWNER_ID)
+
+    assert manager.fetch_count == 1
+    assert manager.peer_ids == ["a-uid", "c-uid"]
+    assert manager.removed == ["b-uid"]
+    assert manager.added == [{"id": "c-uid", "source": SOURCE_ID, "owner": OWNER_ID}]