Skip to content

fix(collaboration): sync comments to second collab client on export#3593

Open
mattConnHarbour wants to merge 2 commits into
mainfrom
SD-3338
Open

fix(collaboration): sync comments to second collab client on export#3593
mattConnHarbour wants to merge 2 commits into
mainfrom
SD-3338

Conversation

@mattConnHarbour
Copy link
Copy Markdown
Contributor

@mattConnHarbour mattConnHarbour commented Jun 1, 2026

Second collaboration client was missing comment metadata (text, author) because comments were not being pushed to Y.Array before the client exported.

CLI (SDK):

  • Set shouldLoadComments: true so #initComments() emiats commentsLoaded
  • Fix isNewFile logic: when seeding from a document, isNewFile must be false so #initComments() runs immediately (not deferred)

Browser:

  • Queue comment events arriving before collaboration is ready
  • Flush queued events when provider syncs

Second collaboration client was missing comment metadata (text, author)
because comments were not being pushed to Y.Array before the client
exported. Three fixes:

CLI:
- Set shouldLoadComments: true so #initComments() emits commentsLoaded
- Fix isNewFile logic: when seeding from a document, isNewFile must be
  false so #initComments() runs immediately (not deferred)

Browser:
- Queue comment events arriving before collaboration is ready
- Flush queued events when provider syncs

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mattConnHarbour mattConnHarbour requested a review from a team as a code owner June 1, 2026 03:05
@linear-code
Copy link
Copy Markdown

linear-code Bot commented Jun 1, 2026

SD-3338

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 63f5d60069

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

isNewFile: shouldSeed,
// When seeding from a document, we need isNewFile: false so that
// #initComments() runs and emits commentsLoaded, pushing comments to Y.Array.
isNewFile: shouldSeed && !docForEditor,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep isNewFile true when seeding from a doc

When decision.action === 'seed' and a document path is provided, docForEditor is the same truthy doc, so this now passes isNewFile: false for the normal seed-from-file path. The editor's collaboration bootstrap only inserts the parsed document into the shared Yjs fragment when options.isNewFile is true (initializeCollaborationData() returns early otherwise), so an empty room seeded from a real DOCX can be marked bootstrapped without actually publishing the document content to other clients.

Useful? React with 👍 / 👎.

* Queue for comment events that arrive before collaboration is ready.
* Flushed when provider syncs via flushPendingCommentEvents().
*/
let pendingCommentEvents = [];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Scope queued comment events per SuperDoc instance

This module-level queue is shared by every SuperDoc instance loaded in the page/process. If one non-collaborative or not-yet-collaborative editor emits a comment event, then another editor later reaches provider sync, flushPendingCommentEvents(superdoc) replays all queued events into that other editor's Y.Array, leaking or corrupting comments across documents; the queued events need to be tied to the originating instance or discarded when that instance is not actually being upgraded to collaboration.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cubic analysis

1 issue found across 3 files

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="packages/superdoc/src/core/collaboration/helpers.js">

<violation number="1" location="packages/superdoc/src/core/collaboration/helpers.js:11">
P1: Module-level `pendingCommentEvents` queue is shared across all SuperDoc instances. If multiple instances exist (e.g., multi-document workspace, or CLI processing multiple files), events queued by one instance will be flushed into a different instance's Y.Array, corrupting comment sync. The queue should be scoped per SuperDoc instance (e.g., stored on `superdoc` itself or in a WeakMap keyed by instance).</violation>
</file>

Linked issue analysis

Linked issue: SD-3338: Bug: Collaborative comment export fails for second client - Y.Array not seeded

Status Acceptance criteria Notes
Queue comment events that arrive before collaboration is ready so they aren't dropped helpers.js introduces a pendingCommentEvents queue and syncCommentsToClients now pushes events to the queue when superdoc.isCollaborative is false.
Flush queued comment events into the Y.Array when provider syncs (so Y.Array is seeded before loading comments) helpers.js adds flushPendingCommentEvents(), calls it in the provider 'synced' handler and if provider is already synced, ensuring queued events are applied before loadCommentsFromYdoc runs.
Ensure CLI opens editor so initComments runs when seeding from a document (isNewFile false) so commentsLoaded is emitted apps/cli change adjusts isNewFile to be false when seeding from a document (shouldSeed && !docForEditor), which causes #initComments() to run immediately during open.
Headless/CLI editorOptions set to load comments so commentsLoaded/onCommentsLoaded callbacks run The headless comment bridge now sets shouldLoadComments: true in editorOptions (two places), which enables comment loading flow used to seed Y.Array/store.
Overall outcome: second collaboration clients receive comment definitions (so exports have comment metadata) The PR implements queuing of early events, flushing them on provider sync, and ensures comment-loading is triggered in CLI/headless paths, which together seed Y.Array before loadCommentsFromYdoc runs — addressing the root cause described in the issue.

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

* Queue for comment events that arrive before collaboration is ready.
* Flushed when provider syncs via flushPendingCommentEvents().
*/
let pendingCommentEvents = [];
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Module-level pendingCommentEvents queue is shared across all SuperDoc instances. If multiple instances exist (e.g., multi-document workspace, or CLI processing multiple files), events queued by one instance will be flushed into a different instance's Y.Array, corrupting comment sync. The queue should be scoped per SuperDoc instance (e.g., stored on superdoc itself or in a WeakMap keyed by instance).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/superdoc/src/core/collaboration/helpers.js, line 11:

<comment>Module-level `pendingCommentEvents` queue is shared across all SuperDoc instances. If multiple instances exist (e.g., multi-document workspace, or CLI processing multiple files), events queued by one instance will be flushed into a different instance's Y.Array, corrupting comment sync. The queue should be scoped per SuperDoc instance (e.g., stored on `superdoc` itself or in a WeakMap keyed by instance).</comment>

<file context>
@@ -4,6 +4,25 @@ import { actorIdentitiesMatch } from '@superdoc/common';
+ * Queue for comment events that arrive before collaboration is ready.
+ * Flushed when provider syncs via flushPendingCommentEvents().
+ */
+let pendingCommentEvents = [];
+
+/**
</file context>
Fix with Cubic

@mattConnHarbour mattConnHarbour marked this pull request as ready for review June 1, 2026 03:13
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.25000% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ackages/superdoc/src/core/collaboration/helpers.js 81.25% 6 Missing ⚠️

📢 Thoughts on this report? Let us know!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants