Note appears as being attached to a deleted block when two users add them at exactly the same moment using real-time collaboration

[smithjw1]

Description

This may be hard to replicate, but two Notes were placed on the same block within 0.02 seconds of each other.

The first Note is being shown as being attached to a block that is deleted, even though the block remains, and is showing the second comment.

Step-by-step reproduction instructions

1. Open a post up with real-time collaboration enabled.
2. Have another collaborator join the post.
3. Attempt to add a Note on the same block at the same time.

It may be necessarily to purposely delay replication of real-time collaboration changes to allow for better testing.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

• Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

• Yes

Please confirm which theme type you used for testing.

• Block
• Classic
• Hybrid (e.g. classic with theme.json)
• Not sure

[smithjw1]

I chatted with Co-pilot about this bug, and here's the result.

Yes, the inconsistency in clientId values between peers could potentially contribute to the behavior described in the issue. Here's why and how this could happen:

---

1. Understanding the Issue

The issue describes a scenario where two Notes were created on the same block within a small time difference (0.02 seconds), but one of the Notes is shown as being attached to a block that is "deleted" (even though the block still exists and is correctly showing the second Note).

2. Role of clientId in Notes Association

• Attachment Mechanism:
  Notes associate themselves with blocks using the clientId of the block at the time of creation. This is evident from the code snippet:

  const onCreate = async ( { content, parent } ) => {
      const savedRecord = await saveEntityRecord(
          'root',
          'comment',
          {
              post: getCurrentPostId(),
              content,
              status: 'hold',
              type: 'note',
          }
      );
  };

  Here, the parent likely represents the block's clientId, which is used to associate the Note with the block.

• Real-Time Collaboration and clientId:
  As noted earlier, clientId is not globally synchronized:

  • When peers interact with the same block, each peer may have a different clientId for that block.
  • If the Notes are created on different peers, they may become associated with different clientId values that do not match the "final" clientId after synchronization.

---

3. Possible Scenario Leading to the Bug

1. Initial State:

   • Two users (User A and User B) are editing the same block in real time.
   • The block has a unique clientId for each peer (clientId-A for User A and clientId-B for User B).
   • Each peer's editor is unaware of the other's clientId.

2. Simultaneous Additions:

   • User A adds a Note and associates it with clientId-A.
   • User B adds a Note and associates it with clientId-B.

3. Synchronization:

   • The block's content is synchronized across peers, but since the clientId is locally generated and transient, it may diverge or conflict between peers.
   • A block client ID might get regenerated or reassigned during the collaboration session, particularly if the synchronization mechanism detects a conflict in clientId values.

4. Resulting Inconsistency:

   • On one peer, clientId-A might now point to a different block or be marked as "deleted" or non-existent, leading to the Note associated with it being shown as attached to a non-existent or deleted block.
   • On the other peer, clientId-B may still correctly point to the intended block, so the second Note appears properly.

---

4. Factors Contributing to the Bug

• Non-Synchronized clientIds:
  Since clientId values are local and transient, there is no guarantee that the clientId used to create a Note on one peer will map to the corresponding block on another peer.

• Timing/Concurrency:
  The issue mentions that the two Notes were created within 0.02 seconds of each other. This introduces concurrency challenges:

  • Both peers may not have had time to sync the block data before creating the Notes.
  • The block state at the time of Note creation might differ between peers.

• Conflict Resolution in Block State:
  If the same block is edited or manipulated by both users, the conflict resolution mechanism (e.g., mergeCrdtBlocks) may introduce changes to the block's metadata, including the clientId.

---

5. Why This Leads to the Issue's Symptoms

• When a peer updates its state after synchronization, it may:
  • Mark one clientId as "orphaned" or "deleted" because it does not match the data received from the other peer.
  • Reassign a clientId to another block or regenerate it for consistency.
• Notes referencing the "orphaned" or stale clientId would then appear to be associated with a "deleted" block, even though the block itself still exists.

---

6. Potential Fixes/Improvements

To avoid this issue, the following improvements could be explored:

1. Global Block Identifiers:

   • Use a globally unique identifier (e.g., uuid) for blocks, rather than local clientIds.
   • Ensure the uuid remains consistent across all peers, so Notes can reliably reference the same block.

2. Delay Note Creation Until Synchronization:

   • Introduce a mechanism to delay Note creation until block synchronization is complete, ensuring consistent state across peers.

3. Fallback for Stale clientIds:

   • Implement a fallback mechanism to reconcile Notes associated with stale clientIds. For example:
     • Check if the underlying block content matches.
     • Reattach Notes to a matching block based on content or other metadata.

4. Explicit Mapping of clientIds:

   • Use an explicit mapping layer to translate clientIds between peers during synchronization, ensuring that Notes remain correctly associated.

---

7. Conclusion

Yes, the inconsistency in clientId synchronization can directly lead to the behavior described in the issue. When two Notes are created in real-time on separate peers, the clientId discrepancies and timing conflicts can result in one peer associating a Note with what it perceives as a "deleted" block. Addressing this requires aligning block identifiers or implementing additional reconciliation strategies during real-time collaboration.

[adamsilverstein]

Hey @smithjw1 - thanks for the detailed bug report and copilot analysis.

Unfortunately copilot seems a bit confused about how the notes are associated with blocks, so its points are mostly off.

Notes associate themselves with blocks using the clientId of the block at the time of creation. This is evident from the code snippet:

That is incorrect. blocks associate to notes by storing the note (or comment) id as a meta attribute on the block. The unique block id or clientId as it is called here is not persistent and changes on every load, but is not related to notes storage or association.

5. Why This Leads to the Issue's Symptoms
   When a peer updates its state after synchronization, it may:
   Mark one clientId as "orphaned" or "deleted" because it does not match the data received from the other peer.
   Reassign a clientId to another block or regenerate it for consistency.
   Notes referencing the "orphaned" or stale clientId would then appear to be associated with a "deleted" block, even though the block itself still exists.

Not quite - the second note becomes orphaned because blocks can (currently) only be associated with one (parent) note. if two saves occur very close together, one might overwrite the other?

6. Potential Fixes/Improvements

Global Block Identifiers:
We have decided not to do this for a host of reason; besides it will not help with the bug.

Delay Note Creation Until Synchronization:

Something like this could work, but again: only one note can be the parent currently. the other could become a reply but that would be unexpected.

Fallback for Stale clientIds:

client ids are not used to tie blocks to notes, note ids are.

Explicit Mapping of clientIds:

same.

Solutions

My suggestion would be to consider supporting multiple top level notes per block. We can do this by enabling the noteId meta to be an array of note ids. That way, if two notes were added close to the same time, they could easily co-exist attached to the same block.

cc: @Mamaduka Curious on your take here. What do you think of my suggestion for multiple notes per block? If it seems interesting I am happy to work on a PR.