Skip to content

merge_nodes_in_tx can leave a survivor with two has_child parents #2945

Closed

Description

@mstomar125

SqliteStore::merge_nodes_in_tx's edge repoint (packages/core/src/db/sqlite_store/conflicts.rs) re-points every edge touching a merge's loser onto the survivor, including has_child edges. Its only collision guard is a raw (in_node, out_node, relationship_type) unique-index check, which does not catch two DIFFERENT parents landing on the same child: if the survivor already has parent A (A -> has_child -> survivor) and the loser has a different parent B (B -> has_child -> loser), the repoint produces B -> has_child -> survivor alongside the survivor's existing A -> has_child -> survivor — a legitimate two-parent state that violates the outline's single-parent invariant (normally enforced by NodeService::create_relationship_in_tx's explicit get_parent_id_in_tx check, which the store-level merge repoint bypasses entirely).

This is pre-existing (the has_child repoint logic is unchanged by PR #2937 — only its return type changed) and out of scope for #2908, which is scoped to declared cardinality/reverse_cardinality relationships, not built-in structural constraints like has_child single-parent. Surfaced during #2908's review (both the embedded pragmatic-code-reviewer pass and a second independent high-effort review) as a real, reachable, untested gap.

packages/core/tests/merge_nodes_test.rs's existing merge_repoints_has_child_and_preserves_order test does not cover this two-parent scenario (it only covers a loser with a single existing parent).

Suggested acceptance criteria

  • A test: survivor already has a has_child parent; loser has a different has_child parent; after merging loser into survivor, the survivor has exactly one parent (either raise/reject the merge, or pick one parent deterministically and drop the other edge — whichever the codebase's ADR-068/069 conflict-resolution semantics call for)
  • merge_nodes_in_tx's has_child repoint gains an explicit single-parent guard, or NodeService::merge_nodes resolves it post-repoint the same way merge_nodes_in_tx bypasses cardinality/reverse_cardinality enforcement #2908 does for declared cardinality-one relationships

Activity

  1. malibio commented on Sep 26, 2026

    @malibio
    Collaborator

    Fixed by #3094 (#2938). The survivor now keeps its own parent and the loser's parent edge is dropped and counted. Covered by merge_of_two_children_keeps_the_survivors_parent_only in packages/core/tests/merge_nodes_test.rs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions