fix: attribute source/owner on relationships in update_node

[PhillSimonds]

Problem

update_node (infrahub_sync/adapters/infrahub.py) sets source/owner attribution on updated attributes but not on updated relationships. A relationship changed by a sync therefore carries no lineage back to the sync's source/owner — most visible for update-only flows, where an object is matched and only its relationships/attributes are updated.

Fixes #142.

Fix

Apply source/owner attribution to both relationship update paths:

# cardinality one: retain the resolved peer for SDK resource-pool detection
setattr(node, attr_name, peer_node)
relationship = getattr(node, attr_name)
relationship.source = source
relationship.owner = owner

# cardinality many: attribute each newly added edge
attr_manager.add(_relationship_input_data(new_id, source, owner))

Retaining the peer object in the cardinality-one path is important: the SDK uses it to recognize CoreResourcePool nodes and emit from_pool allocation input. source/owner are omitted when unset, so behavior is unchanged when no attribution is configured.

This change attributes newly linked cardinality-one peers and newly added cardinality-many edges. It intentionally does not rewrite or backfill unchanged pre-existing edges. It also does not change relationship protection behavior.

Tests

tests/adapters/test_infrahub_update_node_attribution.py now contains 13 focused regression tests covering:

• exact source/owner attribution for attributes and both relationship cardinalities
• behavior when attribution is unset
• real InfrahubNodeSync / RelatedNodeSync serialization for cardinality-one
• preservation of resource-pool from_pool allocation
• stale-peer removal behavior in the existing cardinality-many path

Validation on the current branch:

• focused regression suite: 13 passed
• full unit suite: 123 passed / 3 skipped
• Ruff, YAML, and ty checks pass (ty reports three pre-existing unused-ignore warnings)
• documentation generation and Docusaurus build pass
• live Infrahub end-to-end test passes for cardinality-one and cardinality-many updates, exact persisted source/owner IDs, the documented no-backfill boundary, and a repeated idempotent update
• targeted infrahub-sync generate against the live test schema passes
• GitHub Actions: all required checks green

The branch includes the current main history via a merge commit, per the repository's no-force-push policy.

🤖 Updated with Codex

Summary by CodeRabbit

• Bug Fixes
  • Relationship updates now carry attribution details such as source and owner when provided.
  • One-to-one and one-to-many relationship changes preserve attribution while handling added and removed peers correctly.
  • Relationship updates without attribution continue to work normally using only the peer identifier.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0296e77d-162d-4754-a77e-999008ea00e8

📥 Commits

Reviewing files that changed from the base of the PR and between ca9f9f5 and 79aeda9.

📒 Files selected for processing (2)

• infrahub_sync/adapters/infrahub.py
• tests/adapters/test_infrahub_update_node_attribution.py

---

Walkthrough

Added a _relationship_input_data() helper in the Infrahub sync adapter that builds relationship payloads containing peer id plus optional source and owner fields. Updated update_node() to use this helper for one-to-one relationship assignment and one-to-many relationship peer additions. Preserved resolved-peer assignment for resource-pool handling. Updated the update_node() docstring. Added regression tests for attribute attribution and relationship updates with both cardinalities.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The implementation and tests address issue #142 for cardinality-one and cardinality-many relationship attribution while preserving behavior without attribution.
Out of Scope Changes check	✅ Passed	The implementation and regression tests are directly related to relationship attribution in update_node, with no unrelated changes identified.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: applying relationship source and owner attribution in update_node.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[cloudflare-workers-and-pages]

Deploying infrahub-sync with    Cloudflare Pages

Latest commit:	caaa32c
Status:	✅  Deploy successful!
Preview URL:	https://052a913d.infrahub-sync.pages.dev
Branch Preview URL:	https://fix-update-node-relationship.infrahub-sync.pages.dev

View logs

[estivate]

Approve. Verified end-to-end against a live Infrahub 1.10.6 (Docker stack, throwaway schema, real GraphQL):

	attribute	rel (one)	rel (many)
main	source+owner	—	—
this PR	source+owner	source+owner	source+owner

Idempotent across re-runs. Protected relationships (created via generate_payload_create(is_protected=True)) still update fine — no new failure mode. Post-merge with main: 122 passed / 3 skipped, ruff clean; the 3 ty warnings are pre-existing on main.

The approach is right: generate_payload_create emits exactly this dict shape for relationships, and RelatedNodeBase.__init__ nulls all properties when handed a peer node — which is precisely the bug.

Please fix

1. Docstring is false. It claims parity with the create path, but create also sets is_protected=True. Contradicts your own Note — drop the clause.
2. Trim the comments. 7-line docstring + 2 duplicate call-site comments state the same thing 3×. See 9c571f7 ("drop narrating comments where code is self-explanatory") on this file.
3. Document: no backfill. Only newly-linked peers get attribution. Pre-existing links never converge on re-sync — operators will think the fix didn't work.
4. Rebase — 26 commits behind (merges clean).

Nits

5. test_update_node_attribute_gets_source_and_owner asserts is not None — a source/owner swap stays green. Pin exact values like the relationship tests do.
6. Cardinality-one tests assert on FakeNode.__dict__; real InfrahubNodeSync.__setattr__ intercepts that, so they can't fail for the intended reason. The two real-RelatedNodeSync tests carry the real coverage.
7. FakeAttr/FakeSchema/FakeNode + ty wrapper duplicate test_infrahub_node_to_diffsync.py → tests/adapters/conftest.py.
8. Line 173 list(attr_value) has no None guard, unlike diffsync_to_infrahub. Generated fields default to [], so hardening only.

Behavior change worth a line in the description

9. Cardinality-one loses resource-pool handling: dict → _peer=None → is_resource_pool always False (verified: dict False, peer True), so allocate_from_pool never fires and a pool peer gets linked rather than allocated from. Unreachable today (no pool kinds in examples/), and create already has this shape — so this makes update consistent with an already-broken create.

Separate issue, not this PR

10. existing_peer_ids is read before the conditional fetch() (line 170). For default-Generic-kind many relationships — which generate_query_data doesn't prefetch — the manager arrives initialized=False with peer_ids == [], so existing_only is empty and stale peers are never removed. Pre-existing on main, independent of attribution. (New peers still get attributed correctly, so it doesn't undermine this fix.)

Note on coverage: the e2e above used kind: Attribute relationships, so it did not exercise the Generic path in item 10.

🤖 Review prepared with Claude Code

[estivate]

Review feedback addressed in the latest branch:\n\n- corrected the cardinality-one implementation to retain the resolved peer and preserve resource-pool allocation\n- replaced fake cardinality-one coverage with real SDK nodes and pinned exact source/owner values\n- trimmed inaccurate/redundant narration and documented the no-backfill boundary\n- merged current main without force-pushing\n- kept the unrelated cardinality-many fetch-order bug and None hardening out of this PR\n\nValidation: focused tests 13 passed; full suite 123 passed / 3 skipped; Ruff, YAML, and ty pass (three pre-existing ty warnings); docs generation/build pass; live Infrahub E2E passed for both cardinalities, exact persisted attribution, unchanged-edge behavior, and repeat-update idempotency. All GitHub checks are green. Ready for final review.