fix(backend): retire branch-agnostic field edges on object deletion (… by ajtmccarty · Pull Request #10312 · opsmill/infrahub

ajtmccarty · 2026-08-18T21:12:36Z

Why

Deleting a branch-aware object almost always left its branch-agnostic attributes and relationships undeleted.
Those values live on -global- edges and nothing closed them, so a deleted object went on
holding a live value. This would eventually become visible to users through uniqueness validation
failures due to unreachable, undeleteable agnostic values.

The difficult part of deleting branch-agnostic fields on branch-aware objects is that we need to catch the final delete action that removes the object from ever being visible on any branch going forward and only then delete the agnostic fields. If we close the agnostic fields too early, then we break the object on branches where it is still visible. If we close the agnostic field too late, then we will never see it again and it will remain undeleted.

Normally, I do not like to have chunks of cypher in different modules being accumulated into different queries, but, in this case, we want the same logic to be used in every place that we might delete an aware object with agnostic attributes: single object delete, branch delete, branch merge, or branch rebase. And we will have to handle attribute and relationship remove migrations b/c those can delete the schema field(s) on a branch without deleting the object, which adds an extra layer of complexity.

Goal: the first of six enforcement points — a value is released when no branch can still reach a
live owner over a live edge, decided on object deletion.

Non-goals: the other five enforcement points (branch merge, branch rebase, branch deletion, and
the two schema-removal paths) and the repair migration for orphans already in customer databases.
Each lands as its own slice; this one establishes the shared retention judgement they all compose.

Part of #9762 (does not close it — one of six points).

Base branch is retire-agnostic-edges-ifc-2843, so the spec revision is already in the base and this
diff is code only.

What changed

Behavioral

Deleting an object now closes the global edges of its branch-agnostic fields, once no branch
retains them.
A branch that forked while the object was live keeps reading the object and its value; the
release is deferred until that branch deletes it too.
A retirement failure now fails the delete, where the delete runs in a transaction. Retirement
runs after the tombstone and before the commit, so raising rolls the tombstone back and the caller
retries. A caller deleting in session mode gets no rollback — see Atomicity below.

Implementation notes

agnostic_retention.py holds the retention judgement as one shared Cypher fragment rather than
a component, so the five enforcement points still to come compose it instead of each re-deriving
it. Agnostic attributes and relationships on aware objects are only deleted when the object
is no longer visible on ANY branch.
A relationship needs two live peers, counted by uuid, so the copies a kind or inheritance
change leaves behind count once between them and cannot supply both ends.
Retirement is a time-close (SET e.to = ...), never a deleted-status edge on the global
branch, which would strip the field from every branch at once including the ones still holding the
owner live.
The anchor and the closure require from <= $at, so the query cannot write a to that precedes an
edge's own from.
git/tasks.py now wraps its repository-check cleanup in a transaction. It deleted in session mode,
where a part-way failure leaves earlier deletions committed — already true of its multi-node loop
before this PR, and now true of retirement as well.

What stayed the same

No GraphQL, REST, SDK, CLI or frontend change. No generated file affected.
No graph migration and no GRAPH_VERSION bump — those belong to the repair slice, which is
gated on maintainer sign-off.
No new dependency, no config or feature flag.

Suggested review order

backend/infrahub/core/query/agnostic_retention.py — the retention predicate. This is where a
mistake destroys live customer data; everything else is plumbing.
backend/infrahub/core/query/node_agnostic_retirement.py — the anchor and the closure.
backend/infrahub/core/node/__init__.py — 13 lines, the call site.
Tests: test_agnostic_retirement.py for behaviour through Node.delete, then
test_node_agnostic_retirement_query.py for graph shapes the delete path cannot produce.
backend/tests/helpers/agnostic_edges.py — shared readers, mechanical.

Atomicity depends on the caller

GraphQL requests run in session mode (graphql/app.py:125); mutations that want atomicity open a
transaction explicitly, and most do. Of the delete paths, mutations/main.py:511 (the ordinary delete)
and mutations/account.py:145 already had one, git/tasks.py:1174 did not and is wrapped here, and
ConvertObjectType still does not — object_conversion.py:163 and repository_conversion.py:87
delete in session mode. That one matters because it converts user objects, which are exactly the ones
that can carry a branch-agnostic attribute. It is left alone deliberately: it is a separate mutation
whose multi-step conversion was already non-atomic, and wrapping it belongs in its own change.

Known, pre-existing, not addressed here: a merged-but-undeleted branch retains a value until the
branch itself is deleted, so a value stays allocated after a proposed change merges. It predates this
work and is recorded as a follow-up in tasks.md.

Test summary

20 tests. Coverage includes: created and deleted on one branch; deleted on a branch while the default
holds it; deleted on the default with no fork; a branch forked between creation and deletion
(deferred, and the value still readable there); two retaining branches released one at a time; a
field removed on the only retaining branch; relationship peers live on different branches; a
relationship peer deleted; same-uuid copies after a kind rename; a branch-agnostic owner; idempotence;
failure propagation; and an edge that begins after the requested time being left alone.

The tests were checked by mutation rather than only by passing. Each of these breaks the production
predicate and fails the named number of tests:

Mutation	Tests that fail
drop the existence-axis gate	4
drop the field-edge status gate	2
require one live peer instead of two	2
make the existence match ignore its branch window	4
swap `HAS_SOURCE`/`HAS_OWNER` out of the closure	1
drop `from <= $at` from the closure	1

Impact & rollout

Backward compatibility: behaviour change on delete — branch-agnostic values are now released
rather than leaked.
Performance: one additional query per object deletion, anchored by index seek on the node uuid
and bounded by that node's fields. Plan shape verified; no volume measurement yet.
Config/env changes: none.
Deployment notes: safe to deploy on its own. It prevents new orphans; it does not repair the
backlog, which needs the repair slice and its migration.

Checklist

Tests added/updated
Changelog entry added (uv run towncrier create ...)
External docs updated (if user-facing or ops-facing change)
Internal .md docs updated (internal knowledge and AI code tools knowledge)
I have reviewed AI generated content

…9762) A branch-agnostic attribute or relationship keeps its value on edges carrying the global branch name. Deleting the owning object tombstoned the object but left those edges open and active, so the value stayed allocated with no owner until uniqueness validation failed on ids that no longer resolve. This is the first enforcement point: single object deletion. - agnostic_retention.py holds the retention judgement as one shared Cypher fragment, so the enforcement points still to come cannot each re-derive it slightly differently. A branch retains a field only where the same linked vertex is live and reaches the field over a live edge, under that branch's own view with isolation applied; retention across branches is a disjunction taken after that per-branch conjunction. A relationship needs two live peers, counted by uuid so that the copies a kind or inheritance change leaves behind count once between them. The branch windows are derived from (:Branch) inside the query rather than marshalled through Python, which removes a paginated branch read whose default limit would quietly turn the branches past it into branches that retain nothing. - node_agnostic_retirement.py anchors on one node's open, active global owning edges and time-closes every open, active global edge of each field vertex no branch retains. Never a deleted-status edge and never a vertex; IS_RESERVED is out of reach because it is never incident to an attribute or relationship vertex, and closing HAS_VALUE and HAS_ATTRIBUTE is what frees a pooled value. - Node.delete invokes it after the existence tombstone, inside the caller's still-open transaction, and a failure propagates. Raising rolls the tombstone back and the caller retries; swallowing would commit a deleted object still holding a live branch-agnostic value, which no user or operator action repairs. Still to come, each as its own slice: branch merge, branch rebase, branch deletion, the two schema-removal paths, and the repair migration for the backlog of orphans already in customer databases. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

cubic-dev-ai

3 issues found across 8 files

Confidence score: 2/5

backend/infrahub/core/node/__init__.py — A retirement failure during session-backed Node.delete() can leave earlier delete tombstones committed, including the path used by git/tasks.py; make the deletion atomic so the tombstones roll back on failure.
backend/infrahub/core/query/agnostic_retention.py — Retention can preserve a non-isolated branch’s origin snapshot even though its reads use the current default branch, allowing objects needed by that branch to be retired after deletion from the default branch; align the retention window with non-isolated read semantics.
backend/infrahub/core/query/agnostic_retention.py — Self-referencing relationships can be counted as one distinct peer instead of two, causing a valid node to be retired incorrectly; handle self-referential edges when calculating required live peers.

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="backend/infrahub/core/query/agnostic_retention.py">

<violation number="1" location="backend/infrahub/core/query/agnostic_retention.py:37">
P2: When a non-isolated branch exists, this window treats its origin snapshot as retained even though non-isolated reads use the current default branch. After the default branch deletes an object, the branch can no longer read that owner, but this keeps its agnostic fields open; include `branch.is_isolated` in the snapshot condition.</violation>

<violation number="2" location="backend/infrahub/core/query/agnostic_retention.py:106">
P2: For a self-referencing relationship (a node related to itself) both `IS_RELATED` edges target the same uuid, so `count(DISTINCT node.uuid)` is 1 and `live_peers (1) < required_live_peers (2)` holds, wrongly retiring a relationship that is still live on a branch. The distinct-uuid collapse is needed only to fold kind-migration duplicates; counting live peer edge-ends instead would keep genuine self-loops retained.</violation>
</file>

<file name="backend/infrahub/core/node/__init__.py">

<violation number="1" location="backend/infrahub/core/node/__init__.py:1255">
P1: When `Node.delete()` receives a session-backed `db`, a retirement failure leaves the earlier delete tombstones committed. `git/tasks.py` already calls this path from `start_session()`, so run the whole deletion in an explicit transaction or enforce that requirement before propagating this exception.</violation>
</file>

_{Shadow auto-approve: would not auto-approve because issues were found.

Re-trigger cubic}

cubic-dev-ai · 2026-08-18T21:19:08Z

        await query.execute(db=db)

+        retirement_query = await RetireNodeAgnosticFieldsQuery.init(db=db, node_uuid=self.get_id(), at=delete_at)
+        await retirement_query.execute(db=db)


P1: When Node.delete() receives a session-backed db, a retirement failure leaves the earlier delete tombstones committed. git/tasks.py already calls this path from start_session(), so run the whole deletion in an explicit transaction or enforce that requirement before propagating this exception.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At backend/infrahub/core/node/__init__.py, line 1255: <comment>When `Node.delete()` receives a session-backed `db`, a retirement failure leaves the earlier delete tombstones committed. `git/tasks.py` already calls this path from `start_session()`, so run the whole deletion in an explicit transaction or enforce that requirement before propagating this exception.</comment> <file context> @@ -1250,6 +1251,18 @@ async def delete(self, db: InfrahubDatabase, user_id: str = SYSTEM_USER_ID, at: await query.execute(db=db) + retirement_query = await RetireNodeAgnosticFieldsQuery.init(db=db, node_uuid=self.get_id(), at=delete_at) + await retirement_query.execute(db=db) + retired = retirement_query.get_data() + if retired.edges_closed: </file context>

updated run_check_merge_conflicts() in git/tasks.py to use a transaction instead of a session. this is a pre-existing bug b/c NodeManager.delete() will run multiple Node.delete() calls in sequence, so it is possible for one to fail partway through, leaving the group semi-deleted. the move to use a transaction should fix this

cubic-dev-ai · 2026-08-18T21:19:08Z

+        origin_name: CASE WHEN branch.is_default THEN NULL ELSE branch.origin_branch END,
+        origin_at: CASE
+            WHEN branch.is_default THEN NULL
+            WHEN branch.branched_from < $at THEN branch.branched_from


P2: When a non-isolated branch exists, this window treats its origin snapshot as retained even though non-isolated reads use the current default branch. After the default branch deletes an object, the branch can no longer read that owner, but this keeps its agnostic fields open; include branch.is_isolated in the snapshot condition.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At backend/infrahub/core/query/agnostic_retention.py, line 37: <comment>When a non-isolated branch exists, this window treats its origin snapshot as retained even though non-isolated reads use the current default branch. After the default branch deletes an object, the branch can no longer read that owner, but this keeps its agnostic fields open; include `branch.is_isolated` in the snapshot condition.</comment> <file context> @@ -0,0 +1,112 @@ + origin_name: CASE WHEN branch.is_default THEN NULL ELSE branch.origin_branch END, + origin_at: CASE + WHEN branch.is_default THEN NULL + WHEN branch.branched_from < $at THEN branch.branched_from + ELSE $at + END </file context>

non-isolated branches do not exist anymore. the references to them are dead code

cubic-dev-ai · 2026-08-18T21:19:08Z

+    // Peers are counted by uuid. Kind/inheritance migration leaves multiple Node vertices with
+    // the same uuid for a single entity
+    // ----------------------
+    WITH branch_name, count(DISTINCT node.uuid) AS live_peer_count


P2: For a self-referencing relationship (a node related to itself) both IS_RELATED edges target the same uuid, so count(DISTINCT node.uuid) is 1 and live_peers (1) < required_live_peers (2) holds, wrongly retiring a relationship that is still live on a branch. The distinct-uuid collapse is needed only to fold kind-migration duplicates; counting live peer edge-ends instead would keep genuine self-loops retained.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At backend/infrahub/core/query/agnostic_retention.py, line 106: <comment>For a self-referencing relationship (a node related to itself) both `IS_RELATED` edges target the same uuid, so `count(DISTINCT node.uuid)` is 1 and `live_peers (1) < required_live_peers (2)` holds, wrongly retiring a relationship that is still live on a branch. The distinct-uuid collapse is needed only to fold kind-migration duplicates; counting live peer edge-ends instead would keep genuine self-loops retained.</comment> <file context> @@ -0,0 +1,112 @@ + // Peers are counted by uuid. Kind/inheritance migration leaves multiple Node vertices with + // the same uuid for a single entity + // ---------------------- + WITH branch_name, count(DISTINCT node.uuid) AS live_peer_count + RETURN max(live_peer_count) AS most_live_peers +} </file context>

we don't support an object referencing itself in a relationship

codspeed-hq · 2026-08-18T21:22:02Z

Merging this PR will regress 1 benchmark

⚠️

Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
❌ 1 regressed benchmark
✅ 11 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Benchmark	`BASE`	`HEAD`	Efficiency
❌	`test_query_rel_many`	2.5 s	2.8 s	-10.16%
⚡	`test_base_schema_duplicate_CoreProposedChange`	5.8 ms	4.2 ms	+38.48%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

_{Comparing retire-agnostic-edges-ifc-2843-slice1 (6a86225) with stable (0cb07cb)¹}

No successful run was found on retire-agnostic-edges-ifc-2843 (0240e6d) during the generation of this report, so stable (0cb07cb) was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

…n time (#9762) Review follow-ups on the object-deletion enforcement point. `git/tasks.py` deleted repository check nodes in session mode, so a retirement failure left the earlier deletions committed — the partial state this feature exists to prevent. The cleanup now runs in a transaction. The session was not a decision against one: history shows it was applied by a 2024 sweep that replaced an unscoped `db` across eight files, and the multi-node delete loop it wraps was already non-atomic. The retirement query now requires `from <= $at` on both the anchor and the closure, so it cannot write a `to` that precedes an edge's own `from`. In practice `at` is the deletion's own timestamp and this cannot arise; one predicate makes the inverted interval unrepresentable. The test builds a candidate holding one edge dated later than the requested time, and removing the closure bound fails it. Removing the anchor bound fails nothing, because the closure bound already prevents the write, so it is a candidate-set narrowing rather than verified behaviour. Atomicity still depends on the caller elsewhere. `ConvertObjectType` deletes in session mode and is left alone deliberately, being a separate mutation whose multi-step conversion was already non-atomic. Making `Node.delete` refuse to retire outside transaction mode would settle it generally. Two further review points are unchanged by decision: non-isolated branches, where the API deprecates the flag, and self-referencing relationships, which are not supported. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

cubic-dev-ai

0 issues found across 3 files (changes from recent commits).

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

_{Shadow auto-approve: would not auto-approve. Auto-approval blocked by 1 unresolved issue from previous reviews.

Re-trigger cubic}

…lue is deleted (#9762) Allocate a branch-agnostic value from a number pool, delete the object holding it, allocate again, and assert the same value comes back — along with the graph state that accompanies it: the global HAS_VALUE and HAS_ATTRIBUTE closed, and the IS_RESERVED edge deliberately untouched, since a reservation is the pool's to manage and allocation joins it to the live node by identifier rather than by edge validity. The test disproved the premise it was written under. Re-allocation does not depend on retirement: `BaseAttribute.get_branch_for_delete` returns the node's branch for an agnostic attribute on an aware node, so an ordinary delete already writes branch-scoped `deleted` HAS_VALUE and HAS_ATTRIBUTE edges, and the pool's used-value queries run `branch_agnostic=True`, which collapses the filter to a pure time predicate under which those tombstones win. Neutralising the closure leaves re-allocation working. What retirement fixes is the uniqueness-validation leak, which reads the graph differently. So the test keeps its place through the graph assertions rather than through SC-007, and it is mutation-proven on those: neutralising the closure fails it. The success criterion is amended separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

cubic-dev-ai

0 issues found across 2 files (changes from recent commits).

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

_{Shadow auto-approve: would not auto-approve. Auto-approval blocked by 1 unresolved issue from previous reviews.

Re-trigger cubic}

github-actions Bot added the group/backend Issue related to the backend (API Server, Git Agent) label Aug 18, 2026

cubic-dev-ai Bot reviewed Aug 18, 2026

View reviewed changes

cubic-dev-ai Bot reviewed Aug 19, 2026

View reviewed changes

ajtmccarty marked this pull request as ready for review August 19, 2026 04:18

ajtmccarty requested a review from a team as a code owner August 19, 2026 04:18

Navigation Menu

Uh oh!

fix(backend): retire branch-agnostic field edges on object deletion (… - #10312

fix(backend): retire branch-agnostic field edges on object deletion (…#10312
ajtmccarty wants to merge 3 commits into
retire-agnostic-edges-ifc-2843from
retire-agnostic-edges-ifc-2843-slice1

ajtmccarty commented Aug 18, 2026 •

edited

Loading

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

cubic-dev-ai Bot Aug 18, 2026

Uh oh!

ajtmccarty Aug 18, 2026

Uh oh!

cubic-dev-ai Bot Aug 18, 2026

Uh oh!

ajtmccarty Aug 18, 2026

Uh oh!

Uh oh!

cubic-dev-ai Bot Aug 18, 2026

Uh oh!

ajtmccarty Aug 18, 2026

Uh oh!

codspeed-hq Bot commented Aug 18, 2026 •

edited

Loading

Uh oh!

cubic-dev-ai Bot left a comment

Uh oh!

cubic-dev-ai Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

ajtmccarty commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What changed

Suggested review order

Atomicity depends on the caller

Test summary

Impact & rollout

Checklist

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

ajtmccarty Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

ajtmccarty Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cubic-dev-ai Bot Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

ajtmccarty Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

codspeed-hq Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will regress 1 benchmark

Performance Changes

Footnotes

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ajtmccarty commented Aug 18, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

codspeed-hq Bot commented Aug 18, 2026 •

edited

Loading