diff --git a/apps/cli/src/__tests__/lib/_collab-password-worker.ts b/apps/cli/src/__tests__/lib/_collab-password-worker.ts index 28d30d11e5..83da2eb063 100644 --- a/apps/cli/src/__tests__/lib/_collab-password-worker.ts +++ b/apps/cli/src/__tests__/lib/_collab-password-worker.ts @@ -32,6 +32,8 @@ mock.module('superdoc/super-editor', () => ({ getDocumentApiAdapters: () => ({}), markdownToPmDoc: () => null, initPartsRuntime: () => ({ dispose: () => {} }), + // SD-3214: bridge imports this to feed Y.Array entries into the store. + syncCommentEntitiesFromCollaboration: () => new Set(), })); mock.module('happy-dom', () => ({ diff --git a/apps/cli/src/lib/__tests__/sd-3214-browser-comment-metadata.regression.test.ts b/apps/cli/src/lib/__tests__/sd-3214-browser-comment-metadata.regression.test.ts new file mode 100644 index 0000000000..518a6ab573 --- /dev/null +++ b/apps/cli/src/lib/__tests__/sd-3214-browser-comment-metadata.regression.test.ts @@ -0,0 +1,506 @@ +/** + * SD-3214 regression: comment metadata authored on the browser side (Y.Array) + * must reach the headless SDK's CommentEntityStore so `doc.comments.list()` + * surfaces text / creatorName / creatorEmail / createdTime. + * + * Architectural gap (pre-fix): browser clients write to BOTH + * `ydoc.getArray('comments')` (metadata) and the PM XmlFragment (anchor marks). + * The headless CLI bridge only handled the WRITE direction. The fix wires + * `bridge.attachEditor(editor)` to observe Y.Array and feed entries through + * `syncCommentEntitiesFromCollaboration`. + * + * This test focuses on the ENTITY STORE flow: when a Y.Array entry exists + * pre-open OR arrives post-open, its metadata should be readable via + * `doc.comments.list()`. (PM anchor presence is orthogonal — it supplies + * target/anchoredText/status when the comment is anchored to text.) + */ +import { describe, expect, it } from 'vitest'; +import { Doc as YDoc, Map as YMap } from 'yjs'; +import { openDocument } from '../document'; + +function createIo() { + return { + stdout() {}, + stderr() {}, + async readStdinBytes() { + return new Uint8Array(); + }, + now() { + return Date.now(); + }, + }; +} + +function createProviderStub() { + const noop = () => {}; + return { + synced: true, + awareness: { + on: noop, + off: noop, + getStates: () => new Map(), + setLocalState: noop, + setLocalStateField: noop, + }, + on: noop, + off: noop, + connect: noop, + disconnect: noop, + destroy: noop, + }; +} + +/** + * Mirror the shape `addYComment` in collaboration-comments.js produces when + * a browser user creates a comment via SuperDoc.vue's normal flow. + */ +function pushBrowserAuthoredComment(ydoc: YDoc, comment: Record): void { + const yArray = ydoc.getArray('comments'); + const yComment = new YMap(Object.entries(comment)); + ydoc.transact( + () => { + yArray.push([yComment]); + }, + { user: { name: comment.creatorName, email: comment.creatorEmail } }, + ); +} + +describe('SD-3214: headless SDK reads browser-authored comment metadata', () => { + it('exposes creatorName, createdTime, and text from Y.Array entries pre-existing at open', async () => { + const ydoc = new YDoc(); + + // Browser authored this comment BEFORE the headless client connected. + pushBrowserAuthoredComment(ydoc, { + commentId: 'c-browser-pre', + commentText: 'Please review this clause.', + creatorName: 'Browser User', + creatorEmail: 'browser@example.com', + createdTime: 1700000000000, + isInternal: false, + }); + + const opened = await openDocument(undefined, createIo(), { + documentId: 'sd-3214-doc', + ydoc, + collaborationProvider: createProviderStub(), + isNewFile: true, + user: { name: 'Headless Agent', email: 'agent@superdoc.dev' }, + }); + + const result = opened.editor.doc.comments.list(); + opened.dispose(); + + const item = result.items.find((c) => c.id === 'c-browser-pre'); + expect(item, 'browser-authored comment should be listed by headless SDK').toBeDefined(); + expect(item?.text, 'commentText from Y.Array should reach SDK').toBe('Please review this clause.'); + expect(item?.creatorName, 'creatorName from Y.Array should reach SDK').toBe('Browser User'); + expect(item?.creatorEmail, 'creatorEmail from Y.Array should reach SDK').toBe('browser@example.com'); + expect(item?.createdTime, 'createdTime from Y.Array should reach SDK').toBe(1700000000000); + }); + + it('exposes metadata for browser-authored comments that arrive AFTER open via Y.Array observe', async () => { + const ydoc = new YDoc(); + + const opened = await openDocument(undefined, createIo(), { + documentId: 'sd-3214-doc-late', + ydoc, + collaborationProvider: createProviderStub(), + isNewFile: true, + user: { name: 'Headless Agent', email: 'agent@superdoc.dev' }, + }); + + // Browser authors the comment AFTER the headless client connected. + pushBrowserAuthoredComment(ydoc, { + commentId: 'c-browser-late', + commentText: 'A late comment.', + creatorName: 'Late Browser User', + creatorEmail: 'late@example.com', + createdTime: 1700000001000, + isInternal: false, + }); + + const result = opened.editor.doc.comments.list(); + opened.dispose(); + + const late = result.items.find((c) => c.id === 'c-browser-late'); + expect(late, 'late browser-authored comment should be listed').toBeDefined(); + expect(late?.text).toBe('A late comment.'); + expect(late?.creatorName).toBe('Late Browser User'); + expect(late?.creatorEmail).toBe('late@example.com'); + expect(late?.createdTime).toBe(1700000001000); + }); + + it('a remote update to an existing browser comment surfaces the new fields', async () => { + const ydoc = new YDoc(); + + pushBrowserAuthoredComment(ydoc, { + commentId: 'c-update', + commentText: 'v1', + creatorName: 'Browser User', + creatorEmail: 'browser@example.com', + createdTime: 1700000002000, + }); + + const opened = await openDocument(undefined, createIo(), { + documentId: 'sd-3214-doc-update', + ydoc, + collaborationProvider: createProviderStub(), + isNewFile: true, + user: { name: 'Headless Agent', email: 'agent@superdoc.dev' }, + }); + + // Browser edits the same comment. + const yArray = ydoc.getArray>('comments'); + ydoc.transact( + () => { + yArray.delete(0, 1); + yArray.push([ + new YMap( + Object.entries({ + commentId: 'c-update', + commentText: 'v2', + creatorName: 'Browser User', + creatorEmail: 'browser@example.com', + createdTime: 1700000002000, + }), + ), + ]); + }, + { user: { name: 'Browser User', email: 'browser@example.com' } }, + ); + + const result = opened.editor.doc.comments.list(); + opened.dispose(); + + const item = result.items.find((c) => c.id === 'c-update'); + expect(item?.text).toBe('v2'); + }); +}); + +// --------------------------------------------------------------------------- +// Two-client validation (SD-3214 follow-up). +// Option A: two Editors on a single shared Y.Doc — proves Yjs change broadcast +// reaches Session B's bridge through the real write path that a browser SuperDoc +// would use (editor.commands.addComment + bridge onCommentsUpdate → addYComment). +// Option B: two distinct Y.Docs synced by relaying updates — closer to +// wire-protocol reality. Verifies origin filtering survives applyUpdate hops. +// --------------------------------------------------------------------------- + +import { applyUpdate as yApplyUpdate, encodeStateAsUpdate as yEncodeStateAsUpdate } from 'yjs'; + +describe('SD-3214: two-client end-to-end', () => { + it('Option A — shared Y.Doc: Session B sees a comment authored by Session A', async () => { + const ydoc = new YDoc(); + + // Session A — author (the "browser" side, run as a headless Editor here so + // the test stays in Node; the code path it exercises is identical to what + // SuperDoc.vue runs). + const sessionA = await openDocument(undefined, createIo(), { + documentId: 'sd-3214-shared-doc', + ydoc, + collaborationProvider: createProviderStub(), + isNewFile: true, + user: { name: 'Browser User', email: 'browser@example.com' }, + }); + const insertA = sessionA.editor.doc.create.paragraph({ + at: { kind: 'documentEnd' }, + text: 'A clause about indemnification.', + }); + expect(insertA.success).toBe(true); + const matchA = sessionA.editor.doc.query.match({ + select: { type: 'text', pattern: 'indemnification' }, + require: 'first', + }); + const matchBlock = matchA.items[0].blocks[0]; + const create = sessionA.editor.doc.comments.create({ + target: { kind: 'text', blockId: matchBlock.blockId, range: matchBlock.range } as never, + text: 'Please review this clause.', + }); + expect(create.success).toBe(true); + + // Session B — agent connects to the SAME ydoc. + const sessionB = await openDocument(undefined, createIo(), { + documentId: 'sd-3214-shared-doc', + ydoc, + collaborationProvider: createProviderStub(), + isNewFile: false, + user: { name: 'Headless Agent', email: 'agent@superdoc.dev' }, + }); + + const listB = sessionB.editor.doc.comments.list(); + sessionA.dispose(); + sessionB.dispose(); + + expect(listB.items.length).toBeGreaterThanOrEqual(1); + const item = listB.items[0]; + // Anchor-derived fields survive via PM/Yjs sync. + expect(item.target).toBeDefined(); + // Y.Array-derived metadata — the field set the ticket flagged as empty. + expect(item.text).toBe('Please review this clause.'); + expect(item.creatorName).toBe('Browser User'); + expect(item.creatorEmail).toBe('browser@example.com'); + expect(item.createdTime).toBeTypeOf('number'); + }); + + it('Option A — origin filter: Session A does not double-sync its own writes through observe', async () => { + // After A writes a comment, A's bridge observer fires for its own write. + // The origin filter must skip — otherwise the entry could be processed + // twice (idempotent thanks to upsert, but wasted work and a hint of a + // broken filter that would matter under deletion). + const ydoc = new YDoc(); + const sessionA = await openDocument(undefined, createIo(), { + documentId: 'sd-3214-self-echo', + ydoc, + collaborationProvider: createProviderStub(), + isNewFile: true, + user: { name: 'Author', email: 'author@example.com' }, + }); + sessionA.editor.doc.create.paragraph({ at: { kind: 'documentEnd' }, text: 'A short clause.' }); + const matchBlock = sessionA.editor.doc.query.match({ + select: { type: 'text', pattern: 'clause' }, + require: 'first', + }).items[0].blocks[0]; + sessionA.editor.doc.comments.create({ + target: { kind: 'text', blockId: matchBlock.blockId, range: matchBlock.range } as never, + text: 'mine', + }); + + // Trigger any pending observers by reading. + const list = sessionA.editor.doc.comments.list(); + sessionA.dispose(); + + expect(list.items).toHaveLength(1); + expect(list.items[0].text).toBe('mine'); + expect(list.items[0].creatorName).toBe('Author'); + }); + + it('Option B — two Y.Docs synced via update relay: Session B sees the metadata', async () => { + const docA = new YDoc(); + const docB = new YDoc(); + + // Manual sync relay — simulates a network bridge. Origin tags prevent + // infinite echo loops. + const relayAtoB = (update: Uint8Array, origin: unknown) => { + if (origin === 'from-B') return; + yApplyUpdate(docB, update, 'from-A'); + }; + const relayBtoA = (update: Uint8Array, origin: unknown) => { + if (origin === 'from-A') return; + yApplyUpdate(docA, update, 'from-B'); + }; + docA.on('update', relayAtoB); + docB.on('update', relayBtoA); + + const sessionA = await openDocument(undefined, createIo(), { + documentId: 'sd-3214-two-docs', + ydoc: docA, + collaborationProvider: createProviderStub(), + isNewFile: true, + user: { name: 'Browser User', email: 'browser@example.com' }, + }); + sessionA.editor.doc.create.paragraph({ + at: { kind: 'documentEnd' }, + text: 'A clause on confidentiality.', + }); + const matchA = sessionA.editor.doc.query.match({ + select: { type: 'text', pattern: 'confidentiality' }, + require: 'first', + }); + const matchBlockA = matchA.items[0].blocks[0]; + sessionA.editor.doc.comments.create({ + target: { kind: 'text', blockId: matchBlockA.blockId, range: matchBlockA.range } as never, + text: 'Confidentiality should cover IP.', + }); + + // Seed docB from docA before opening Session B — mimics a fresh client + // joining the room after some history exists. + yApplyUpdate(docB, yEncodeStateAsUpdate(docA), 'from-A'); + + const sessionB = await openDocument(undefined, createIo(), { + documentId: 'sd-3214-two-docs', + ydoc: docB, + collaborationProvider: createProviderStub(), + isNewFile: false, + user: { name: 'Headless Agent', email: 'agent@superdoc.dev' }, + }); + + const listB = sessionB.editor.doc.comments.list(); + sessionA.dispose(); + sessionB.dispose(); + docA.off('update', relayAtoB); + docB.off('update', relayBtoA); + + expect(listB.items.length).toBeGreaterThanOrEqual(1); + const item = listB.items[0]; + expect(item.text).toBe('Confidentiality should cover IP.'); + expect(item.creatorName).toBe('Browser User'); + expect(item.creatorEmail).toBe('browser@example.com'); + }); + + it('Option A — remote delete: when a browser removes a comment from Y.Array, Session B prunes the entry', async () => { + // Scope: this validates Session B's READ-SIDE prune behavior. The browser + // delete is simulated by removing the entry from ydoc.getArray('comments') + // directly — exactly what packages/superdoc/.../collaboration-comments.js + // deleteYComment does. The CLI's own write-side delete bridging is tracked + // separately; SD-3214 covers metadata propagation from browser to agent. + const ydoc = new YDoc(); + + // Seed Y.Array with a browser-authored comment. + pushBrowserAuthoredComment(ydoc, { + commentId: 'c-to-delete', + commentText: 'Will be removed.', + creatorName: 'Browser User', + creatorEmail: 'browser@example.com', + createdTime: 1700000010000, + }); + + const opened = await openDocument(undefined, createIo(), { + documentId: 'sd-3214-delete-readside', + ydoc, + collaborationProvider: createProviderStub(), + isNewFile: true, + user: { name: 'Headless Agent', email: 'agent@superdoc.dev' }, + }); + + // Session sees the comment after attach. + const beforeList = opened.editor.doc.comments.list(); + expect(beforeList.items.find((c) => c.id === 'c-to-delete')).toBeDefined(); + + // Simulate browser deletion of the Y.Array entry. + const yArr = ydoc.getArray>('comments'); + const idx = (yArr.toJSON() as Array>).findIndex((c) => c.commentId === 'c-to-delete'); + ydoc.transact( + () => { + yArr.delete(idx, 1); + }, + { user: { name: 'Browser User', email: 'browser@example.com' } }, + ); + + const afterList = opened.editor.doc.comments.list(); + opened.dispose(); + + expect(afterList.items.find((c) => c.id === 'c-to-delete')).toBeUndefined(); + }); + + it('CLI write-side delete: agent.comments.delete propagates to Y.Array (customer "agent resolves comments" flow)', async () => { + // The customer's headless agent needs to mutate comments and have those + // mutations reach other browser collaborators. resolveComment / removeComment + // engine commands don't emit `commentsUpdate`, so SD-3214's bridge fix + // pairs with wrapper-level emits in `comments-wrappers.ts`. + const ydoc = new YDoc(); + const session = await openDocument(undefined, createIo(), { + documentId: 'sd-3214-cli-delete', + ydoc, + collaborationProvider: createProviderStub(), + isNewFile: true, + user: { name: 'Headless Agent', email: 'agent@superdoc.dev' }, + }); + session.editor.doc.create.paragraph({ at: { kind: 'documentEnd' }, text: 'A clause to delete.' }); + const matchBlock = session.editor.doc.query.match({ + select: { type: 'text', pattern: 'delete' }, + require: 'first', + }).items[0].blocks[0]; + session.editor.doc.comments.create({ + target: { kind: 'text', blockId: matchBlock!.blockId, range: matchBlock!.range } as never, + text: 'agent-authored', + }); + const yArr = ydoc.getArray('comments').toJSON() as Array>; + expect(yArr.length).toBe(1); + const targetId = yArr[0].commentId as string; + + // Agent deletes — this is the write-side that previously didn't propagate. + const del = session.editor.doc.comments.delete({ commentId: targetId }); + expect(del.success).toBe(true); + + // Y.Array now reflects the delete (other collaborators would observe this). + const afterYArr = ydoc.getArray('comments').toJSON() as Array>; + session.dispose(); + expect(afterYArr).toHaveLength(0); + }); + + it('CLI write-side resolve: agent.comments.patch({status:resolved}) propagates resolvedTime to Y.Array', async () => { + const ydoc = new YDoc(); + const session = await openDocument(undefined, createIo(), { + documentId: 'sd-3214-cli-resolve', + ydoc, + collaborationProvider: createProviderStub(), + isNewFile: true, + user: { name: 'Headless Agent', email: 'agent@superdoc.dev' }, + }); + session.editor.doc.create.paragraph({ at: { kind: 'documentEnd' }, text: 'A clause to resolve.' }); + const matchBlock = session.editor.doc.query.match({ + select: { type: 'text', pattern: 'resolve' }, + require: 'first', + }).items[0].blocks[0]; + session.editor.doc.comments.create({ + target: { kind: 'text', blockId: matchBlock!.blockId, range: matchBlock!.range } as never, + text: 'pending review', + }); + const initial = ydoc.getArray('comments').toJSON() as Array>; + const targetId = initial[0].commentId as string; + expect(initial[0].resolvedTime).toBeFalsy(); + + // Agent resolves via the public patch surface. + const patch = session.editor.doc.comments.patch({ commentId: targetId, status: 'resolved' }); + expect(patch.success).toBe(true); + + // Y.Array reflects the resolution. + const after = ydoc.getArray('comments').toJSON() as Array>; + session.dispose(); + expect(after).toHaveLength(1); + expect(after[0].isDone).toBe(true); + expect(typeof after[0].resolvedTime).toBe('number'); + }); + + it('Option B — post-open browser write: a comment authored AFTER the agent connects still propagates', async () => { + const docA = new YDoc(); + const docB = new YDoc(); + const relayAtoB = (update: Uint8Array, origin: unknown) => { + if (origin === 'from-B') return; + yApplyUpdate(docB, update, 'from-A'); + }; + const relayBtoA = (update: Uint8Array, origin: unknown) => { + if (origin === 'from-A') return; + yApplyUpdate(docA, update, 'from-B'); + }; + docA.on('update', relayAtoB); + docB.on('update', relayBtoA); + + // Agent connects FIRST. + const sessionB = await openDocument(undefined, createIo(), { + documentId: 'sd-3214-late-author', + ydoc: docB, + collaborationProvider: createProviderStub(), + isNewFile: true, + user: { name: 'Headless Agent', email: 'agent@superdoc.dev' }, + }); + + // Browser connects and authors a comment. + const sessionA = await openDocument(undefined, createIo(), { + documentId: 'sd-3214-late-author', + ydoc: docA, + collaborationProvider: createProviderStub(), + isNewFile: true, + user: { name: 'Browser User', email: 'browser@example.com' }, + }); + sessionA.editor.doc.create.paragraph({ at: { kind: 'documentEnd' }, text: 'A late clause.' }); + const matchBlock = sessionA.editor.doc.query.match({ + select: { type: 'text', pattern: 'late clause' }, + require: 'first', + }).items[0].blocks[0]; + sessionA.editor.doc.comments.create({ + target: { kind: 'text', blockId: matchBlock.blockId, range: matchBlock.range } as never, + text: 'Late comment.', + }); + + const listB = sessionB.editor.doc.comments.list(); + sessionA.dispose(); + sessionB.dispose(); + docA.off('update', relayAtoB); + docB.off('update', relayBtoA); + + const found = listB.items.find((c) => (c as { text?: string }).text === 'Late comment.'); + expect(found, 'browser-authored comment after agent connect should reach agent').toBeDefined(); + expect((found as { creatorName?: string }).creatorName).toBe('Browser User'); + }); +}); diff --git a/apps/cli/src/lib/document.ts b/apps/cli/src/lib/document.ts index 5b3e5faf2c..bd01d7347d 100644 --- a/apps/cli/src/lib/document.ts +++ b/apps/cli/src/lib/document.ts @@ -222,6 +222,11 @@ export async function openDocument( // afterCommit hooks are always wired, including in headless CLI sessions. initPartsRuntime(editor as never); + // SD-3214: bridge observes ydoc.getArray('comments') and feeds remote + // (browser-authored) metadata into the editor's CommentEntityStore so the + // headless SDK can read text/creatorName/createdTime via doc.comments.list(). + commentBridge?.attachEditor(editor as never); + // Apply content override post-init. // - markdown: DOM-free AST pipeline // - plainText: builds PM paragraphs directly, preserving all whitespace diff --git a/apps/cli/src/lib/headless-comment-bridge.ts b/apps/cli/src/lib/headless-comment-bridge.ts index adab125c2d..2f80b7fcc3 100644 --- a/apps/cli/src/lib/headless-comment-bridge.ts +++ b/apps/cli/src/lib/headless-comment-bridge.ts @@ -11,9 +11,16 @@ */ import { Map as YMap } from 'yjs'; -import type { Doc as YDoc, Array as YArray } from 'yjs'; +import type { Doc as YDoc, Array as YArray, YArrayEvent } from 'yjs'; +import { syncCommentEntitiesFromCollaboration } from 'superdoc/super-editor'; import type { UserIdentity } from './types'; +// Editor handle is intentionally typed as `unknown` here — the bridge only +// forwards it to `syncCommentEntitiesFromCollaboration`, which owns the +// engine-specific knowledge. Keeping the type opaque preserves the CLI's +// engine-agnostic boundary. +type EditorHandle = Parameters[0]; + // --------------------------------------------------------------------------- // Yjs write helpers (mirrors collaboration-comments.js) // --------------------------------------------------------------------------- @@ -144,7 +151,23 @@ export interface HeadlessCommentBridgeResult { onCommentsUpdate: (params: Record) => void; onCommentsLoaded: (params: { editor: unknown; comments: unknown[] }) => void; }; - /** Cleanup — clears internal registry */ + /** + * Wire the bridge to an Editor instance once `Editor.open()` resolves. + * + * Seeds the editor's CommentEntityStore from the current Y.Array contents, + * then installs a Y.Array observer that mirrors remote (other-client) + * comment additions, updates, and removals into the store. Without this, + * `editor.doc.comments.list()` only sees PM-anchor data for browser- + * authored comments and the text/creatorName/createdTime fields are empty. + * + * Origin filter: events whose `transaction.origin.user` matches the bridge + * user are skipped — those came from this client's own writes and are + * already in the store via the normal `onCommentsUpdate` path. + * + * Safe to call multiple times; only the most recent editor is observed. + */ + attachEditor(editor: EditorHandle): void; + /** Cleanup — clears internal registry and detaches Y.Array observer */ dispose(): void; } @@ -274,6 +297,51 @@ export function buildHeadlessCommentBridge(ydoc: unknown, user?: UserIdentity): ); } + // ---- Y.Array → CommentEntityStore observer (SD-3214) ---- + + let attachedEditor: EditorHandle | null = null; + let yArrayObserver: ((event: YArrayEvent>) => void) | null = null; + // Set of commentIds previously synced from Y.Array. The helper uses this + // to detect remote deletions and prune them from the store. + let previousSyncedIds: ReadonlySet = new Set(); + + function syncYArrayToStore(): void { + if (!attachedEditor) return; + const entries = yArray.toJSON() as Array>; + previousSyncedIds = syncCommentEntitiesFromCollaboration(attachedEditor, entries, { + previouslySynced: previousSyncedIds, + }); + } + + function detachYArrayObserver(): void { + if (yArrayObserver) { + yArray.unobserve(yArrayObserver); + yArrayObserver = null; + } + } + + function attachEditor(editor: EditorHandle): void { + detachYArrayObserver(); + attachedEditor = editor; + // Reset the prior-sync set so a re-attach (e.g. document re-open) doesn't + // prune entries that are genuinely fresh from this editor's perspective. + previousSyncedIds = new Set(); + + // Initial seed: pull whatever is already in the room. + syncYArrayToStore(); + + yArrayObserver = (event) => { + // Skip our own writes — they're already in the store via onCommentsUpdate. + const origin = (event.transaction.origin ?? null) as { user?: { name?: string; email?: string } } | null; + const originUser = origin?.user; + if (userOrigin && originUser && originUser.name === userOrigin.name && originUser.email === userOrigin.email) { + return; + } + syncYArrayToStore(); + }; + yArray.observe(yArrayObserver); + } + return { editorOptions: { isCommentsEnabled: true, @@ -281,7 +349,11 @@ export function buildHeadlessCommentBridge(ydoc: unknown, user?: UserIdentity): onCommentsUpdate: handleCommentsUpdate, onCommentsLoaded: handleCommentsLoaded, }, + attachEditor, dispose() { + detachYArrayObserver(); + attachedEditor = null; + previousSyncedIds = new Set(); registry.clear(); }, }; diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/comment-entity-store.test.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/comment-entity-store.test.ts index f435142c8f..2ccadb8cef 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/comment-entity-store.test.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/comment-entity-store.test.ts @@ -8,6 +8,7 @@ import { getCommentEntityStore, isCommentResolved, removeCommentEntityTree, + syncCommentEntitiesFromCollaboration, toCommentInfo, upsertCommentEntity, type CommentEntityRecord, @@ -252,3 +253,185 @@ describe('toCommentInfo', () => { expect(info.anchoredText).toBeUndefined(); }); }); + +describe('syncCommentEntitiesFromCollaboration (SD-3214)', () => { + it('upserts a new browser-authored comment into an empty store', () => { + const editor = makeEditorWithConverter(); + syncCommentEntitiesFromCollaboration(editor, [ + { + commentId: 'c1', + commentText: 'Please review this clause.', + creatorName: 'Browser User', + creatorEmail: 'browser@example.com', + createdTime: 1700000000000, + isInternal: false, + }, + ]); + + const store = getCommentEntityStore(editor); + expect(store).toHaveLength(1); + expect(store[0].commentId).toBe('c1'); + expect(store[0].commentText).toBe('Please review this clause.'); + expect(store[0].creatorName).toBe('Browser User'); + expect(store[0].creatorEmail).toBe('browser@example.com'); + expect(store[0].createdTime).toBe(1700000000000); + expect(store[0].isInternal).toBe(false); + }); + + it('accepts `text` as a fallback for `commentText`', () => { + // Some browser writers emit { text } instead of { commentText }; mirror + // the alias logic the browser-side loader uses. + const editor = makeEditorWithConverter(); + syncCommentEntitiesFromCollaboration(editor, [{ commentId: 'c1', text: 'short form' }]); + const store = getCommentEntityStore(editor); + expect(store[0].commentText).toBe('short form'); + }); + + it('skips entries flagged trackedChange:true (those belong to a separate domain)', () => { + const editor = makeEditorWithConverter(); + syncCommentEntitiesFromCollaboration(editor, [ + { commentId: 'tc-1', trackedChange: true, trackedChangeText: 'inserted', creatorName: 'A' }, + { commentId: 'c-1', commentText: 'real comment', creatorName: 'B' }, + ]); + const store = getCommentEntityStore(editor); + expect(store).toHaveLength(1); + expect(store[0].commentId).toBe('c-1'); + }); + + it('skips entries without a commentId', () => { + const editor = makeEditorWithConverter(); + syncCommentEntitiesFromCollaboration(editor, [{ creatorName: 'orphan' }, { commentId: 'c-ok', creatorName: 'X' }]); + const store = getCommentEntityStore(editor); + expect(store).toHaveLength(1); + expect(store[0].commentId).toBe('c-ok'); + }); + + it('falls back to importedId when commentId is missing', () => { + const editor = makeEditorWithConverter(); + syncCommentEntitiesFromCollaboration(editor, [{ importedId: 'imp-1', creatorName: 'X' }]); + const store = getCommentEntityStore(editor); + expect(store).toHaveLength(1); + expect(store[0].commentId).toBe('imp-1'); + expect(store[0].importedId).toBe('imp-1'); + }); + + it('merges an updated entry without clobbering unchanged fields', () => { + const editor = makeEditorWithConverter([ + { + commentId: 'c1', + commentText: 'v1', + creatorName: 'Author', + creatorEmail: 'author@example.com', + createdTime: 1, + }, + ]); + // Remote update bumps commentText only. + syncCommentEntitiesFromCollaboration(editor, [{ commentId: 'c1', commentText: 'v2' }]); + const store = getCommentEntityStore(editor); + expect(store).toHaveLength(1); + expect(store[0].commentText).toBe('v2'); + expect(store[0].creatorName).toBe('Author'); + expect(store[0].creatorEmail).toBe('author@example.com'); + expect(store[0].createdTime).toBe(1); + }); + + it('propagates resolution metadata', () => { + const editor = makeEditorWithConverter([{ commentId: 'c1', commentText: 'hi' }]); + syncCommentEntitiesFromCollaboration(editor, [ + { + commentId: 'c1', + commentText: 'hi', + isDone: true, + resolvedTime: 1700000005000, + resolvedByEmail: 'resolver@example.com', + resolvedByName: 'Resolver', + }, + ]); + const store = getCommentEntityStore(editor); + expect(store[0].isDone).toBe(true); + expect(store[0].resolvedTime).toBe(1700000005000); + expect(store[0].resolvedByEmail).toBe('resolver@example.com'); + expect(store[0].resolvedByName).toBe('Resolver'); + }); + + it('returns the set of synced comment ids (for caller-driven deletion sweep)', () => { + const editor = makeEditorWithConverter(); + const seen = syncCommentEntitiesFromCollaboration(editor, [ + { commentId: 'c1' }, + { commentId: 'c2' }, + { trackedChange: true, commentId: 'tc-1' }, + ]); + expect(seen).toEqual(new Set(['c1', 'c2'])); + }); + + it('is a no-op for empty input', () => { + const editor = makeEditorWithConverter([{ commentId: 'pre', commentText: 'kept' }]); + syncCommentEntitiesFromCollaboration(editor, []); + const store = getCommentEntityStore(editor); + expect(store).toHaveLength(1); + expect(store[0].commentText).toBe('kept'); + }); + + // Remote-deletion handling: when a prior collab-synced id disappears from + // the upstream Y.Array, the helper prunes the matching store entry. + it('prunes a previously-synced entry that is no longer in upstream entries', () => { + const editor = makeEditorWithConverter(); + const first = syncCommentEntitiesFromCollaboration(editor, [ + { commentId: 'a', commentText: 'a' }, + { commentId: 'b', commentText: 'b' }, + ]); + expect(getCommentEntityStore(editor)).toHaveLength(2); + expect(first).toEqual(new Set(['a', 'b'])); + + // Remote drops 'a'. + const second = syncCommentEntitiesFromCollaboration(editor, [{ commentId: 'b', commentText: 'b' }], { + previouslySynced: first, + }); + const store = getCommentEntityStore(editor); + expect(store).toHaveLength(1); + expect(store[0].commentId).toBe('b'); + expect(second).toEqual(new Set(['b'])); + }); + + it('does not prune locally-authored entries that were never collab-synced', () => { + // 'local' is in the store but never in `previouslySynced` — the helper + // must leave it alone even though it isn't in the upstream entries. + const editor = makeEditorWithConverter([{ commentId: 'local', commentText: 'cli-authored' }]); + syncCommentEntitiesFromCollaboration(editor, [{ commentId: 'remote', commentText: 'r' }], { + previouslySynced: new Set(), + }); + const store = getCommentEntityStore(editor); + expect(store).toHaveLength(2); + expect(store.map((e) => e.commentId).sort()).toEqual(['local', 'remote']); + }); + + it('prunes thread replies along with the deleted parent', () => { + // removeCommentEntityTree cascades to children — confirm via the helper. + const editor = makeEditorWithConverter(); + const first = syncCommentEntitiesFromCollaboration(editor, [ + { commentId: 'root' }, + { commentId: 'reply-1', parentCommentId: 'root' }, + { commentId: 'reply-2', parentCommentId: 'root' }, + { commentId: 'unrelated' }, + ]); + expect(getCommentEntityStore(editor)).toHaveLength(4); + + syncCommentEntitiesFromCollaboration(editor, [{ commentId: 'unrelated' }], { + previouslySynced: first, + }); + const store = getCommentEntityStore(editor); + expect(store.map((e) => e.commentId).sort()).toEqual(['unrelated']); + }); + + it('returns the new sync set unchanged when no removals occur', () => { + const editor = makeEditorWithConverter(); + const first = syncCommentEntitiesFromCollaboration(editor, [{ commentId: 'a' }, { commentId: 'b' }]); + const second = syncCommentEntitiesFromCollaboration( + editor, + [{ commentId: 'a' }, { commentId: 'b' }, { commentId: 'c' }], + { previouslySynced: first }, + ); + expect(second).toEqual(new Set(['a', 'b', 'c'])); + expect(getCommentEntityStore(editor)).toHaveLength(3); + }); +}); diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/comment-entity-store.ts b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/comment-entity-store.ts index c1f8712413..778ed125a2 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/helpers/comment-entity-store.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/helpers/comment-entity-store.ts @@ -182,6 +182,93 @@ export function isCommentResolved(entry: CommentEntityRecord): boolean { return Boolean(entry.isDone || entry.resolvedTime); } +/** + * Sync remote comment metadata from a collaboration channel (e.g. the Yjs + * `ydoc.getArray('comments')` used by browser SuperDoc clients) into the + * editor's CommentEntityStore. + * + * Without this sync, the headless SDK only sees PM-anchor-derived fields + * (id, target, anchoredText, status) for browser-authored comments — the + * Y.Array metadata (text, creatorName, creatorEmail, createdTime) never + * reaches `doc.comments.list()`. See SD-3214. + * + * Behavior: + * - Each entry with a `commentId` is upserted into the store. Existing + * entries are merged (collaborator-authored fields override locally + * captured ones; missing fields are left alone). + * - Entries flagged `trackedChange: true` are skipped — those belong to + * the tracked-changes domain, not the comments store. + * - When `options.previouslySynced` is provided, any id present in the + * prior set but absent from the current entries is treated as a remote + * deletion and pruned via `removeCommentEntityTree`. Locally-authored + * entries that were never collab-synced are left alone. + * - Returns the set of commentIds observed during the sync. Callers should + * pass this back as `previouslySynced` on the next call to detect + * subsequent remote deletions. + */ +export function syncCommentEntitiesFromCollaboration( + editor: Editor, + entries: ReadonlyArray>, + options: { previouslySynced?: ReadonlySet } = {}, +): Set { + const store = getCommentEntityStore(editor); + const seen = new Set(); + + for (const raw of entries) { + if (!raw || typeof raw !== 'object') continue; + if (raw.trackedChange === true) continue; + + const commentId = toNonEmptyString(raw.commentId) ?? toNonEmptyString(raw.importedId); + if (!commentId) continue; + seen.add(commentId); + + const patch: Partial = {}; + // Identity fields + if (typeof raw.importedId === 'string') patch.importedId = raw.importedId; + if (typeof raw.parentCommentId === 'string') patch.parentCommentId = raw.parentCommentId; + // Body + const commentText = + typeof raw.commentText === 'string' ? raw.commentText : typeof raw.text === 'string' ? raw.text : undefined; + if (commentText !== undefined) patch.commentText = commentText; + if (raw.commentJSON !== undefined) patch.commentJSON = raw.commentJSON; + if (raw.elements !== undefined) patch.elements = raw.elements; + // Authoring metadata + if (typeof raw.creatorName === 'string') patch.creatorName = raw.creatorName; + if (typeof raw.creatorEmail === 'string') patch.creatorEmail = raw.creatorEmail; + if (typeof raw.creatorImage === 'string') patch.creatorImage = raw.creatorImage; + if (typeof raw.createdTime === 'number') patch.createdTime = raw.createdTime; + // Status + if (typeof raw.isInternal === 'boolean') patch.isInternal = raw.isInternal; + if (typeof raw.isDone === 'boolean') patch.isDone = raw.isDone; + if (typeof raw.resolvedTime === 'number') patch.resolvedTime = raw.resolvedTime; + if (raw.resolvedTime === null) patch.resolvedTime = null; + if (typeof raw.resolvedByEmail === 'string') patch.resolvedByEmail = raw.resolvedByEmail; + if (typeof raw.resolvedByName === 'string') patch.resolvedByName = raw.resolvedByName; + + upsertCommentEntity(store, commentId, patch); + } + + // Prune entries previously known to come from collab sync but now absent + // from the upstream Y.Array. Locally-authored entries that were never in + // `previouslySynced` are intentionally left alone. + if (options.previouslySynced) { + for (const priorId of options.previouslySynced) { + if (!seen.has(priorId)) { + removeCommentEntityTree(store, priorId); + } + } + } + + return seen; +} + +/** Local helper: trim+narrow a value to a non-empty string. */ +function toNonEmptyString(value: unknown): string | undefined { + if (typeof value !== 'string') return undefined; + const trimmed = value.trim(); + return trimmed.length > 0 ? trimmed : undefined; +} + export function toCommentInfo( entry: CommentEntityRecord, options: { diff --git a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/comments-wrappers.ts b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/comments-wrappers.ts index a828ee85a4..b5f4ec71e0 100644 --- a/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/comments-wrappers.ts +++ b/packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/comments-wrappers.ts @@ -147,6 +147,29 @@ function listCommentAnchorsSafe(editor: Editor): ReturnType, +): void { + const emitter = (editor as unknown as { emit?: (event: string, payload: unknown) => void }).emit; + if (typeof emitter !== 'function') return; + emitter.call(editor, 'commentsUpdate', { type, comment }); +} + function applyTextSelection(editor: Editor, from: number, to: number): boolean { const setTextSelection = editor.commands?.setTextSelection; if (typeof setTextSelection === 'function') { @@ -762,6 +785,8 @@ function resolveCommentHandler(editor: Editor, input: ResolveCommentInput, optio }; } + let resolvedTimestamp: number | null = null; + const receipt = executeDomainCommand( editor, () => { @@ -770,10 +795,11 @@ function resolveCommentHandler(editor: Editor, input: ResolveCommentInput, optio importedId: identity.importedId, }); if (didResolve) { + resolvedTimestamp = Date.now(); upsertCommentEntity(store, identity.commentId, { importedId: identity.importedId, isDone: true, - resolvedTime: Date.now(), + resolvedTime: resolvedTimestamp, }); } return Boolean(didResolve); @@ -788,6 +814,18 @@ function resolveCommentHandler(editor: Editor, input: ResolveCommentInput, optio }; } + // SD-3214: the resolveComment engine command sets `tr.setMeta(CommentsPluginKey, { event: 'update' })` + // but does not emit `commentsUpdate`. The browser commentsStore handles its own resolve flow by + // emitting `comments-update` manually + writing to Y.Array. Document-API consumers (CLI, MCP) need + // the wrapper to fire the canonical event so the headless bridge can propagate to Y.Array via its + // existing `'update'` / `'resolved'` handler. + emitCommentLifecycleUpdate(editor, 'resolved', { + commentId: identity.commentId, + importedId: identity.importedId, + isDone: true, + resolvedTime: resolvedTimestamp, + }); + return { success: true, updated: [toCommentAddress(identity.commentId)] }; } @@ -850,6 +888,16 @@ function reopenCommentHandler(editor: Editor, input: ReopenCommentInput, options }; } + // SD-3214: reopenComment doesn't emit either — surface a canonical + // 'update' event so the bridge can mirror the cleared resolved markers + // into Y.Array. + emitCommentLifecycleUpdate(editor, 'update', { + commentId: identity.commentId, + importedId: identity.importedId, + isDone: false, + resolvedTime: null, + }); + return { success: true, updated: [toCommentAddress(identity.commentId)] }; } @@ -890,6 +938,17 @@ function removeCommentHandler(editor: Editor, input: RemoveCommentInput, options removedIds.add(identity.commentId); } + // SD-3214: removeComment engine command sets `tr.setMeta` but doesn't emit + // `commentsUpdate`. Emit here so the headless bridge propagates the delete + // to Y.Array (and the browser's existing DELETED branch refreshes its Vue + // list). Emits per removed id so thread-reply cascades reach subscribers. + for (const removedId of removedIds) { + emitCommentLifecycleUpdate(editor, 'deleted', { + commentId: removedId, + importedId: removedId === identity.commentId ? identity.importedId : undefined, + }); + } + return { success: true, removed: Array.from(removedIds).map((id) => toCommentAddress(id)), diff --git a/packages/super-editor/src/editors/v1/index.js b/packages/super-editor/src/editors/v1/index.js index 0979a94485..fcf98a5d4b 100644 --- a/packages/super-editor/src/editors/v1/index.js +++ b/packages/super-editor/src/editors/v1/index.js @@ -51,6 +51,7 @@ import { onCollaborationProviderSynced } from './core/helpers/collaboration-prov import { resolveSelectionTarget } from './document-api-adapters/helpers/selection-target-resolver.js'; import { resolveDefaultInsertTarget } from './document-api-adapters/helpers/adapter-utils.js'; import { resolveTrackedChangeInStory } from './document-api-adapters/helpers/tracked-change-resolver.js'; +import { syncCommentEntitiesFromCollaboration } from './document-api-adapters/helpers/comment-entity-store.js'; import { getTrackedChangeIndex } from './document-api-adapters/tracked-changes/tracked-change-index.js'; import { makeTrackedChangeAnchorKey, @@ -158,6 +159,8 @@ export { resolveDefaultInsertTarget, /** @internal */ resolveTrackedChangeInStory, + /** @internal SD-3214: feed collaboration-sourced comment metadata into the editor's CommentEntityStore. */ + syncCommentEntitiesFromCollaboration, // Story-aware tracked-change service /** @internal */