From a32fba63ec711418b904be489d9ad87aeae0f6e6 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Fri, 3 Jul 2026 18:29:03 +0300 Subject: [PATCH 1/7] feat(comment): db columns for comment suggestions (#315 phase 1) Add suggested_text / suggestion_applied_at / suggestion_applied_by_id to the comments table (migration) and mirror them in the hand-curated db.d.ts Comments interface. suggested_text holds a proposed replacement for the comment's anchored selection; the applied_* columns record who applied it and when. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../20260703T190000-comment-suggestions.ts | 26 +++++++++++++++++++ apps/server/src/database/types/db.d.ts | 3 +++ 2 files changed, 29 insertions(+) create mode 100644 apps/server/src/database/migrations/20260703T190000-comment-suggestions.ts diff --git a/apps/server/src/database/migrations/20260703T190000-comment-suggestions.ts b/apps/server/src/database/migrations/20260703T190000-comment-suggestions.ts new file mode 100644 index 00000000..2f0523f8 --- /dev/null +++ b/apps/server/src/database/migrations/20260703T190000-comment-suggestions.ts @@ -0,0 +1,26 @@ +import { type Kysely } from 'kysely'; + +export async function up(db: Kysely): Promise { + // Agent comment suggestions (#315): a comment may carry a proposed replacement + // for its anchored `selection`, which a human applies via the comment UI. + await db.schema + .alterTable('comments') + // The proposed replacement text (plain text). NULL for ordinary comments. + .addColumn('suggested_text', 'text') + // When the suggestion was applied (NULL until applied). + .addColumn('suggestion_applied_at', 'timestamptz') + // Who applied it (NULL until applied). + .addColumn('suggestion_applied_by_id', 'uuid', (col) => + col.references('users.id').onDelete('set null'), + ) + .execute(); +} + +export async function down(db: Kysely): Promise { + await db.schema + .alterTable('comments') + .dropColumn('suggested_text') + .dropColumn('suggestion_applied_at') + .dropColumn('suggestion_applied_by_id') + .execute(); +} diff --git a/apps/server/src/database/types/db.d.ts b/apps/server/src/database/types/db.d.ts index f4b868cc..dab98b3d 100644 --- a/apps/server/src/database/types/db.d.ts +++ b/apps/server/src/database/types/db.d.ts @@ -173,6 +173,9 @@ export interface Comments { resolvedSource: string | null; selection: string | null; spaceId: string; + suggestedText: string | null; + suggestionAppliedAt: Timestamp | null; + suggestionAppliedById: string | null; type: string | null; updatedAt: Generated; workspaceId: string; From 7c0664d2b3527636fd2fbdcd14688b10be80bb62 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Fri, 3 Jul 2026 18:41:32 +0300 Subject: [PATCH 2/7] =?UTF-8?q?feat(collab):=20replaceYjsMarkedText=20?= =?UTF-8?q?=E2=80=94=20atomic=20check-and-replace=20of=20comment-marked=20?= =?UTF-8?q?text=20(#315=20phase=202)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The primitive behind "Apply comment suggestion": walk the XmlFragment, collect the delta segments carrying the `comment` mark for a commentId, and replace them with new text ONLY if the run is intact (single Y.XmlText, contiguous, and the joined text still equals the expected anchor). Otherwise return a verdict { applied:false, currentText } — null when the anchor is gone, else the current text — so the caller can report "someone changed it". On apply it deletes the run and re-inserts the new text re-attaching the same comment mark (thread stays anchored). Mutates in place for the caller's connection.transact(); opens no transaction of its own. Non-string inserts (embeds) advance the offset by their 1-unit index length so a marked segment after an embed gets the right position and an embed inside a run is correctly rejected as a changed anchor. Tests (yjs.util.spec.ts): happy path (mark preserved, surrounding text and no mark-bleed), resolved-mark match, changed text, deleted anchor, paragraph split, interleaved unmarked text, and embed before/inside the run. 17 passed. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../server/src/collaboration/yjs.util.spec.ts | 228 ++++++++++++++++++ apps/server/src/collaboration/yjs.util.ts | 131 ++++++++++ 2 files changed, 359 insertions(+) diff --git a/apps/server/src/collaboration/yjs.util.spec.ts b/apps/server/src/collaboration/yjs.util.spec.ts index 29511d6d..eb0eb0f0 100644 --- a/apps/server/src/collaboration/yjs.util.spec.ts +++ b/apps/server/src/collaboration/yjs.util.spec.ts @@ -10,6 +10,7 @@ import { setYjsMark, removeYjsMarkByAttribute, updateYjsMarkAttribute, + replaceYjsMarkedText, type YjsSelection, } from './yjs.util'; @@ -276,3 +277,230 @@ describe('updateYjsMarkAttribute', () => { expect(text.toDelta()).toEqual(before); }); }); + +describe('replaceYjsMarkedText', () => { + // Build a single-paragraph XmlText from runs. Insert the whole string as + // plain text FIRST, then format only the marked ranges — otherwise text + // inserted right after a marked run inherits its comment mark (Yjs carries + // formatting from the left insertion boundary). + function buildRuns( + runs: Array<{ + text: string; + comment?: { commentId: string; resolved: boolean }; + }>, + ): { fragment: Y.XmlFragment; text: Y.XmlText } { + const ydoc = new Y.Doc(); + const fragment = ydoc.getXmlFragment('default'); + const para = new Y.XmlElement('paragraph'); + fragment.insert(0, [para]); + const text = new Y.XmlText(); + para.insert(0, [text]); + text.insert(0, runs.map((r) => r.text).join('')); + let offset = 0; + for (const run of runs) { + if (run.comment) { + text.format(offset, run.text.length, { comment: run.comment }); + } + offset += run.text.length; + } + return { fragment, text }; + } + + // Two paragraphs, each with its own XmlText, both marked with the same + // commentId — mirrors a suggestion anchor that got split across blocks. + function buildTwoParagraphs( + a: { text: string; comment?: { commentId: string; resolved: boolean } }, + b: { text: string; comment?: { commentId: string; resolved: boolean } }, + ): { fragment: Y.XmlFragment; textA: Y.XmlText; textB: Y.XmlText } { + const ydoc = new Y.Doc(); + const fragment = ydoc.getXmlFragment('default'); + const build = (seg: typeof a) => { + const para = new Y.XmlElement('paragraph'); + const text = new Y.XmlText(); + para.insert(0, [text]); + text.insert(0, seg.text); + if (seg.comment) { + text.format(0, seg.text.length, { comment: seg.comment }); + } + return { para, text }; + }; + const pa = build(a); + const pb = build(b); + fragment.insert(0, [pa.para, pb.para]); + return { fragment, textA: pa.text, textB: pb.text }; + } + + it('happy path: replaces marked text with newText and keeps the comment mark', () => { + const { fragment, text } = buildRuns([ + { text: 'Hello ' }, + { text: 'world', comment: { commentId: 'c1', resolved: false } }, + { text: '!' }, + ]); + + const result = replaceYjsMarkedText(fragment, 'c1', 'world', 'planet'); + + expect(result).toEqual({ applied: true, currentText: 'planet' }); + // New text carries the SAME comment mark; surrounding text is untouched. + expect(text.toDelta()).toEqual([ + { insert: 'Hello ' }, + { + insert: 'planet', + attributes: { comment: { commentId: 'c1', resolved: false } }, + }, + { insert: '!' }, + ]); + }); + + it('matches by commentId even when the mark is resolved', () => { + const { fragment, text } = buildWithComments([ + { text: 'foo', comment: { commentId: 'c9', resolved: true } }, + ]); + + const result = replaceYjsMarkedText(fragment, 'c9', 'foo', 'bar'); + + expect(result).toEqual({ applied: true, currentText: 'bar' }); + expect(text.toDelta()).toEqual([ + { + insert: 'bar', + attributes: { comment: { commentId: 'c9', resolved: true } }, + }, + ]); + }); + + it('changed text: marked text differs from expected → no-op, doc unchanged', () => { + const { fragment, text } = buildWithComments([ + { text: 'abc', comment: { commentId: 'c1', resolved: false } }, + ]); + const before = text.toDelta(); + + const result = replaceYjsMarkedText(fragment, 'c1', 'expected', 'new'); + + expect(result).toEqual({ applied: false, currentText: 'abc' }); + expect(text.toDelta()).toEqual(before); + }); + + it('anchor deleted: no mark with that commentId → { applied: false, currentText: null }', () => { + const { fragment, text } = buildWithComments([ + { text: 'abc', comment: { commentId: 'c1', resolved: false } }, + ]); + const before = text.toDelta(); + + const result = replaceYjsMarkedText(fragment, 'missing', 'abc', 'new'); + + expect(result).toEqual({ applied: false, currentText: null }); + expect(text.toDelta()).toEqual(before); + }); + + it('paragraph split: same commentId in two XmlText nodes → no-op, doc unchanged', () => { + const { fragment, textA, textB } = buildTwoParagraphs( + { text: 'Hello ', comment: { commentId: 'c1', resolved: false } }, + { text: 'world', comment: { commentId: 'c1', resolved: false } }, + ); + const beforeA = textA.toDelta(); + const beforeB = textB.toDelta(); + + const result = replaceYjsMarkedText(fragment, 'c1', 'Hello world', 'new'); + + expect(result).toEqual({ applied: false, currentText: 'Hello world' }); + expect(textA.toDelta()).toEqual(beforeA); + expect(textB.toDelta()).toEqual(beforeB); + }); + + it('interleaved unmarked text: marked run not contiguous → no-op, doc unchanged', () => { + const { fragment, text } = buildRuns([ + { text: 'abc', comment: { commentId: 'c1', resolved: false } }, + { text: 'X' }, + { text: 'def', comment: { commentId: 'c1', resolved: false } }, + ]); + const before = text.toDelta(); + + const result = replaceYjsMarkedText(fragment, 'c1', 'abcdef', 'new'); + + // Joined marked text ("abcdef") is returned, but the run is not contiguous. + expect(result).toEqual({ applied: false, currentText: 'abcdef' }); + expect(text.toDelta()).toEqual(before); + }); + + it('preserves surrounding text and merges adjacent marked segments on apply', () => { + // The marked run itself is split into two adjacent delta segments; they must + // be treated as one contiguous run and replaced as a whole. + const { fragment, text } = buildRuns([ + { text: 'pre ' }, + { text: 'ab', comment: { commentId: 'c1', resolved: false } }, + { text: 'cd', comment: { commentId: 'c1', resolved: false } }, + { text: ' post' }, + ]); + + const result = replaceYjsMarkedText(fragment, 'c1', 'abcd', 'Z'); + + expect(result).toEqual({ applied: true, currentText: 'Z' }); + expect(text.toDelta()).toEqual([ + { insert: 'pre ' }, + { + insert: 'Z', + attributes: { comment: { commentId: 'c1', resolved: false } }, + }, + { insert: ' post' }, + ]); + }); + + it('embed before the marked run: offset accounts for the embed unit → replaces the right text, embed intact', () => { + // "AB", then a Yjs embed (1 index unit), then marked "world". Before the + // fix the embed was skipped WITHOUT advancing offset, so the computed start + // for "world" was too low by 1 → delete/insert would have hit the embed/text + // instead of "world", mangling the embed. With the fix offset is correct. + const ydoc = new Y.Doc(); + const fragment = ydoc.getXmlFragment('default'); + const para = new Y.XmlElement('paragraph'); + fragment.insert(0, [para]); + const text = new Y.XmlText(); + para.insert(0, [text]); + text.insert(0, 'AB'); + text.insertEmbed(2, { image: { src: 'x' } }); + text.insert(3, 'world'); + text.format(3, 'world'.length, { + comment: { commentId: 'c1', resolved: false }, + }); + + const result = replaceYjsMarkedText(fragment, 'c1', 'world', 'planet'); + + expect(result).toEqual({ applied: true, currentText: 'planet' }); + // "AB" untouched, embed still present and intact, "world" → "planet" + // carrying the SAME comment mark. + expect(text.toDelta()).toEqual([ + { insert: 'AB' }, + { insert: { image: { src: 'x' } } }, + { + insert: 'planet', + attributes: { comment: { commentId: 'c1', resolved: false } }, + }, + ]); + }); + + it('embed inside the marked run: embed splits the run → non-contiguous → no-op, doc unchanged', () => { + // marked "abc", an embed, marked "def" — same commentId. The embed occupies + // one index unit between the two marked segments, so they are not contiguous + // → the guard rejects it and nothing is mutated (embed intact). + const ydoc = new Y.Doc(); + const fragment = ydoc.getXmlFragment('default'); + const para = new Y.XmlElement('paragraph'); + fragment.insert(0, [para]); + const text = new Y.XmlText(); + para.insert(0, [text]); + text.insert(0, 'abc'); + text.insertEmbed(3, { image: { src: 'y' } }); + text.insert(4, 'def'); + text.format(0, 'abc'.length, { + comment: { commentId: 'c1', resolved: false }, + }); + text.format(4, 'def'.length, { + comment: { commentId: 'c1', resolved: false }, + }); + const before = text.toDelta(); + + const result = replaceYjsMarkedText(fragment, 'c1', 'abcdef', 'new'); + + expect(result).toEqual({ applied: false, currentText: 'abcdef' }); + expect(text.toDelta()).toEqual(before); + }); +}); diff --git a/apps/server/src/collaboration/yjs.util.ts b/apps/server/src/collaboration/yjs.util.ts index c79e5331..bea28dfc 100644 --- a/apps/server/src/collaboration/yjs.util.ts +++ b/apps/server/src/collaboration/yjs.util.ts @@ -133,6 +133,137 @@ export function removeYjsMarkByAttribute( } } +/** + * A single marked delta segment collected during the walk, together with the + * Y.XmlText node that owns it, the segment's start offset within that node, + * and the full `comment` mark attributes object (needed to re-attach the mark + * to the replacement text). + */ +type MarkedSegment = { + node: Y.XmlText; + offset: number; + length: number; + text: string; + markAttrs: Record; +}; + +/** + * Atomically check-and-replace the text currently under a comment mark. + * + * Walks the fragment collecting every delta segment whose `comment` mark has the + * given commentId. The replacement is applied ONLY if the marked run is intact: + * it lives in a single Y.XmlText node, is contiguous (no unmarked text spliced + * into the middle), and its joined text still equals `expectedText`. On success + * the run is deleted and `newText` is inserted at the same offset carrying the + * SAME comment attributes, so the comment thread stays anchored to the new text. + * + * This mutates the passed fragment/text directly and does NOT open its own Y + * transaction — the caller is expected to wrap the call in connection.transact() + * so the delete+insert are atomic (mirrors updateYjsMarkAttribute's direct + * mutation style). + * + * @returns `{ applied: true, currentText: newText }` on replacement, otherwise + * `{ applied: false, currentText }` where currentText is the text currently + * under the mark (or null when the mark/anchor no longer exists). + */ +export function replaceYjsMarkedText( + fragment: Y.XmlFragment, + commentId: string, + expectedText: string, + newText: string, +): { applied: boolean; currentText: string | null } { + // 1. Collect every marked segment in document order. + const segments: MarkedSegment[] = []; + + const processItem = (item: any) => { + if (item instanceof Y.XmlText) { + const deltas = item.toDelta(); + let offset = 0; + + for (const delta of deltas) { + const insert = delta.insert; + // Non-string inserts (embeds) carry no text length we can splice on. + if (typeof insert !== 'string') { + // A Yjs embed occupies one unit in the index space used by delete/ + // insert/format — advance offset so a marked segment after an embed + // gets the right position (and an embed inside a marked run creates a + // gap → the contiguity guard rejects it as a changed anchor). + offset += 1; + continue; + } + const length = insert.length; + const attributes = delta.attributes ?? {}; + const markAttr = attributes['comment']; + + if (markAttr && markAttr.commentId === commentId) { + segments.push({ + node: item, + offset, + length, + text: insert, + markAttrs: markAttr, + }); + } + offset += length; + } + } else if (item instanceof Y.XmlElement) { + for (let i = 0; i < item.length; i++) { + processItem(item.get(i)); + } + } + }; + + for (let i = 0; i < fragment.length; i++) { + processItem(fragment.get(i)); + } + + const joinedText = segments.map((s) => s.text).join(''); + + // 2a. No segments — the mark/anchor was deleted. + if (segments.length === 0) { + return { applied: false, currentText: null }; + } + + // 2b. Segments span more than one Y.XmlText node (paragraph split by Enter, + // or the mark bled across blocks) — treat as changed. + const node = segments[0].node; + const sameNode = segments.every((s) => s.node === node); + if (!sameNode) { + return { applied: false, currentText: joinedText }; + } + + // 2c. Non-contiguous within the single node: unmarked text is spliced between + // the first and last marked segment. Since collected segments are in document + // order, contiguity holds iff each segment starts where the previous ended. + let contiguous = true; + for (let i = 1; i < segments.length; i++) { + if (segments[i].offset !== segments[i - 1].offset + segments[i - 1].length) { + contiguous = false; + break; + } + } + if (!contiguous) { + return { applied: false, currentText: joinedText }; + } + + // 2d. The text under the mark changed. + if (joinedText !== expectedText) { + return { applied: false, currentText: joinedText }; + } + + // 3. All guards passed: delete the marked run and re-insert newText with the + // same comment attributes at the same offset. Atomic within the caller's + // transaction. + const start = segments[0].offset; + const len = segments.reduce((sum, s) => sum + s.length, 0); + const markAttrs = segments[0].markAttrs; + + node.delete(start, len); + node.insert(start, newText, { comment: markAttrs }); + + return { applied: true, currentText: newText }; +} + /** * Updates a mark's attributes for all text that has the specified attribute value. * Useful for resolving/unresolving comments by commentId. From a9da8f7f15466f109e29d84a272dc158f206298b Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Fri, 3 Jul 2026 18:52:44 +0300 Subject: [PATCH 3/7] feat(collab): applyCommentSuggestion event + no-Redis local fallback (#315 phase 3) New custom collab event applyCommentSuggestion runs replaceYjsMarkedText inside the document's Yjs transaction on the owning instance and returns the { applied, currentText } verdict to the API-server caller (cross-process via the Redis bridge, whose customEventComplete/replyId already carries handler return values). - withYdocConnection is now generic and returns the callback's result (captured in a closure, since hocuspocus connection.transact does not forward it). The callback is typed synchronous-only: transact runs fn synchronously without awaiting, so an async fn would mutate outside the transaction and lose atomicity. - collaboration.gateway.handleYjsEvent: when Redis is disabled (COLLAB_DISABLE_REDIS), dispatch the handler locally against the single hocuspocus instance and return its verdict instead of silently returning undefined (which would make apply a no-op). Also fixes the pre-existing silent no-op of setCommentMark/resolveCommentMark without Redis. Tests: handler spec (applied mutates doc + returns verdict; changed-text returns {applied:false} without mutating; args forwarded; withYdocConnection returns the value) and gateway spec (no-Redis path dispatches locally, returns the verdict, not undefined). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../collaboration.gateway.spec.ts | 60 ++++++++ .../collaboration/collaboration.gateway.ts | 37 ++++- .../collaboration.handler.spec.ts | 132 ++++++++++++++++++ .../collaboration/collaboration.handler.ts | 54 ++++++- 4 files changed, 276 insertions(+), 7 deletions(-) create mode 100644 apps/server/src/collaboration/collaboration.gateway.spec.ts create mode 100644 apps/server/src/collaboration/collaboration.handler.spec.ts diff --git a/apps/server/src/collaboration/collaboration.gateway.spec.ts b/apps/server/src/collaboration/collaboration.gateway.spec.ts new file mode 100644 index 00000000..5af89679 --- /dev/null +++ b/apps/server/src/collaboration/collaboration.gateway.spec.ts @@ -0,0 +1,60 @@ +import { CollaborationGateway } from './collaboration.gateway'; +import { CollaborationHandler } from './collaboration.handler'; + +/** + * Focused test for the COLLAB_DISABLE_REDIS fallback in handleYjsEvent. + * + * With Redis disabled the gateway builds no RedisSyncExtension, so the old code + * (`return this.redisSync?.handleEvent(...)`) returned undefined and every + * doc-mutation event silently no-opped. The fallback must instead invoke the + * handler locally against the single hocuspocus instance and return its verdict. + * + * We construct the gateway with stub extensions and an EnvironmentService whose + * isCollabDisableRedis() returns true (redisSync stays null, real hocuspocus is + * still built), then spy getHandlers so no real direct connection is opened. + */ + +const stubExtension = {} as any; + +function makeEnv() { + return { + getRedisUrl: () => 'redis://localhost:6379', + isCollabDisableRedis: () => true, + } as any; +} + +describe('CollaborationGateway.handleYjsEvent (no-Redis fallback)', () => { + it('invokes the handler locally and returns its verdict instead of undefined', async () => { + const collabHandler = new CollaborationHandler(); + const verdict = { applied: true, currentText: 'new' }; + const fakeHandler = jest.fn().mockResolvedValue(verdict); + // Bypass the real direct-connection code path — assert dispatch only. + jest + .spyOn(collabHandler, 'getHandlers') + .mockReturnValue({ applyCommentSuggestion: fakeHandler } as any); + + const gateway = new CollaborationGateway( + stubExtension, + stubExtension, + stubExtension, + makeEnv(), + collabHandler, + ); + + const payload = { + commentId: 'c1', + expectedText: 'old', + newText: 'new', + user: { id: 'u1' } as any, + }; + const result = await gateway.handleYjsEvent( + 'applyCommentSuggestion' as any, + 'doc-1', + payload as any, + ); + + expect(fakeHandler).toHaveBeenCalledWith('doc-1', payload); + expect(result).toEqual(verdict); + expect(result).not.toBeUndefined(); + }); +}); diff --git a/apps/server/src/collaboration/collaboration.gateway.ts b/apps/server/src/collaboration/collaboration.gateway.ts index b11f14b1..ced36083 100644 --- a/apps/server/src/collaboration/collaboration.gateway.ts +++ b/apps/server/src/collaboration/collaboration.gateway.ts @@ -147,8 +147,41 @@ export class CollaborationGateway { eventName: TName, documentName: string, payload: Parameters[1], - ) { - return this.redisSync?.handleEvent(eventName, documentName, payload); + ): ReturnType { + if (this.redisSync) { + // Normal path: the Redis bridge routes the event to the instance that owns + // the document (local or another worker) and carries the handler's return + // value back to us (customEventComplete + replyId). + return this.redisSync.handleEvent( + eventName, + documentName, + payload, + ) as ReturnType; + } + + // COLLAB_DISABLE_REDIS: there is no cross-process bridge, so a single local + // hocuspocus instance owns every document. Invoke the handler directly + // against it instead of returning undefined — otherwise doc-mutation events + // (setCommentMark / resolveCommentMark / applyCommentSuggestion) would + // silently no-op and, for suggestions, the caller could never learn the + // verdict. openDirectConnection loads the doc via the persistence extension + // if it is not already in memory. + if (this.hocuspocus) { + const handlers = this.collabEventsService.getHandlers(this.hocuspocus); + const handler = handlers[eventName] as ( + documentName: string, + payload: unknown, + ) => ReturnType; + return handler(documentName, payload); + } + + // Collaboration was never initialized (no live instance). Fail loudly rather + // than silently dropping a mutation; phase 4's caller maps this to a 5xx. + throw new Error( + `Cannot handle collaboration event "${String( + eventName, + )}": requires a live collaboration instance`, + ); } openDirectConnection(documentName: string, context?: any) { diff --git a/apps/server/src/collaboration/collaboration.handler.spec.ts b/apps/server/src/collaboration/collaboration.handler.spec.ts new file mode 100644 index 00000000..21f70325 --- /dev/null +++ b/apps/server/src/collaboration/collaboration.handler.spec.ts @@ -0,0 +1,132 @@ +import * as Y from 'yjs'; +import { CollaborationHandler } from './collaboration.handler'; +import * as yjsUtil from './yjs.util'; +import { User } from '@docmost/db/types/entity.types'; + +/** + * Unit tests for the `applyCommentSuggestion` collab handler (phase 3 of #315). + * + * The handler runs `replaceYjsMarkedText` inside the owning instance's Y + * transaction and returns the verdict to the caller. We exercise it against a + * REAL in-memory Y.Doc carrying a marked comment run, driven through a FAKE + * hocuspocus whose openDirectConnection().transact(fn) simply runs fn(doc) — + * mirroring how the real hocuspocus DirectConnection invokes the callback with + * the shared document (it does not forward the callback's return value, which is + * exactly why withYdocConnection captures it via a closure). + */ + +// Build a Y.Doc with a single paragraph whose text carries a `comment` mark for +// the given commentId — the shape `replaceYjsMarkedText` walks in production. +function buildDocWithComment(text: string, commentId: string) { + const doc = new Y.Doc(); + const fragment = doc.getXmlFragment('default'); + const paragraph = new Y.XmlElement('paragraph'); + const xmlText = new Y.XmlText(); + xmlText.insert(0, text); + xmlText.format(0, text.length, { comment: { commentId, resolved: false } }); + paragraph.insert(0, [xmlText]); + fragment.insert(0, [paragraph]); + return doc; +} + +// Fake hocuspocus exposing only what withYdocConnection needs: a direct +// connection whose transact() runs the callback against `doc`. +function fakeHocuspocus(doc: Y.Doc) { + const connection = { + transact: jest.fn(async (fn: (d: Y.Doc) => void) => { + fn(doc); + }), + disconnect: jest.fn(async () => {}), + }; + const hocuspocus = { + openDirectConnection: jest.fn(async () => connection), + } as any; + return { hocuspocus, connection }; +} + +const user = { id: 'u1' } as unknown as User; + +describe('CollaborationHandler.applyCommentSuggestion', () => { + it('applies the replacement and returns the verdict when the marked text matches', async () => { + const doc = buildDocWithComment('Hello world', 'c1'); + const { hocuspocus, connection } = fakeHocuspocus(doc); + const handler = new CollaborationHandler(); + const handlers = handler.getHandlers(hocuspocus); + + const result = await handlers.applyCommentSuggestion('doc-1', { + commentId: 'c1', + expectedText: 'Hello world', + newText: 'Goodbye world', + user, + }); + + expect(result).toEqual({ applied: true, currentText: 'Goodbye world' }); + // The mutation ran inside the transaction and hit the real doc. + expect(connection.transact).toHaveBeenCalledTimes(1); + expect(connection.disconnect).toHaveBeenCalledTimes(1); + expect(doc.getXmlFragment('default').toString()).toContain( + 'Goodbye world', + ); + }); + + it('rejects (applied=false) and returns the current text when it changed', async () => { + const doc = buildDocWithComment('Hello world', 'c1'); + const { hocuspocus } = fakeHocuspocus(doc); + const handler = new CollaborationHandler(); + const handlers = handler.getHandlers(hocuspocus); + + const result = await handlers.applyCommentSuggestion('doc-1', { + commentId: 'c1', + expectedText: 'Stale expected text', + newText: 'Goodbye world', + user, + }); + + expect(result).toEqual({ applied: false, currentText: 'Hello world' }); + // Nothing was replaced. + expect(doc.getXmlFragment('default').toString()).toContain( + 'Hello world', + ); + }); + + it('forwards the exact args to replaceYjsMarkedText and returns its result', async () => { + const doc = buildDocWithComment('abc', 'c9'); + const { hocuspocus } = fakeHocuspocus(doc); + const spy = jest + .spyOn(yjsUtil, 'replaceYjsMarkedText') + .mockReturnValue({ applied: true, currentText: 'xyz' }); + const handler = new CollaborationHandler(); + const handlers = handler.getHandlers(hocuspocus); + + const result = await handlers.applyCommentSuggestion('doc-1', { + commentId: 'c9', + expectedText: 'abc', + newText: 'xyz', + user, + }); + + expect(spy).toHaveBeenCalledWith( + doc.getXmlFragment('default'), + 'c9', + 'abc', + 'xyz', + ); + expect(result).toEqual({ applied: true, currentText: 'xyz' }); + spy.mockRestore(); + }); + + it('withYdocConnection returns the callback result (transact does not forward it)', async () => { + const doc = new Y.Doc(); + const { hocuspocus } = fakeHocuspocus(doc); + const handler = new CollaborationHandler(); + + const value = await handler.withYdocConnection( + hocuspocus, + 'doc-1', + {}, + () => 42, + ); + + expect(value).toBe(42); + }); +}); diff --git a/apps/server/src/collaboration/collaboration.handler.ts b/apps/server/src/collaboration/collaboration.handler.ts index 2d4ae58f..263adec4 100644 --- a/apps/server/src/collaboration/collaboration.handler.ts +++ b/apps/server/src/collaboration/collaboration.handler.ts @@ -5,7 +5,12 @@ import { prosemirrorNodeToYElement, tiptapExtensions, } from './collaboration.util'; -import { setYjsMark, updateYjsMarkAttribute, YjsSelection } from './yjs.util'; +import { + replaceYjsMarkedText, + setYjsMark, + updateYjsMarkAttribute, + YjsSelection, +} from './yjs.util'; import * as Y from 'yjs'; import { User } from '@docmost/db/types/entity.types'; @@ -73,6 +78,35 @@ export class CollaborationHandler { }, ); }, + applyCommentSuggestion: async ( + documentName: string, + payload: { + commentId: string; + expectedText: string; + newText: string; + user: User; + }, + ): Promise<{ applied: boolean; currentText: string | null }> => { + const { commentId, expectedText, newText, user } = payload; + // Run the check-and-replace inside the owning instance's Y transaction so + // the delete+insert are atomic. The verdict from replaceYjsMarkedText is + // returned to the API-server caller (cross-process via the Redis bridge, + // or locally when Redis is disabled — see collaboration.gateway.ts). + return this.withYdocConnection( + hocuspocus, + documentName, + { user }, + (doc) => { + const fragment = doc.getXmlFragment('default'); + return replaceYjsMarkedText( + fragment, + commentId, + expectedText, + newText, + ); + }, + ); + }, updatePageContent: async ( documentName: string, payload: { @@ -115,18 +149,28 @@ export class CollaborationHandler { }; } - async withYdocConnection( + async withYdocConnection( hocuspocus: Hocuspocus, documentName: string, context: any = {}, - fn: (doc: Document) => void, - ): Promise { + // `fn` MUST be synchronous: hocuspocus `connection.transact(fn)` runs fn + // synchronously and does NOT await it, so any mutations after an `await` + // inside fn would execute OUTSIDE the Yjs transaction and lose atomicity. + fn: (doc: Document) => T, + ): Promise { const connection = await hocuspocus.openDirectConnection( documentName, context, ); try { - await connection.transact(fn); + // hocuspocus `connection.transact(fn)` invokes fn(document) but does NOT + // forward fn's return value, so we capture it in a closure and return it + // after the transaction (and its storeDocument hooks) resolve. + let result: T; + await connection.transact((doc) => { + result = fn(doc); + }); + return result!; } finally { await connection.disconnect(); } From ec542a924b05e6fba67e11941c709e286c68a4f0 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Fri, 3 Jul 2026 19:09:23 +0300 Subject: [PATCH 4/7] feat(comment): store suggestedText + POST /comments/apply-suggestion (#315 phase 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Server side of agent comment suggestions. - CreateCommentDto gains optional suggestedText (<=2000). CommentService.create accepts it ONLY for a top-level inline comment with a non-empty selection, requires it be non-empty and differ from selection (else BadRequest), and stores it. - POST /comments/apply-suggestion (ApplySuggestionDto { commentId }): authorizes with validateCanEdit (applying edits page text) BEFORE any structural check or mutation, then CommentService.applySuggestion: - runs the phase-3 collab event applyCommentSuggestion on `page.` to atomically check-and-replace the marked text, returning { applied, currentText }; - applied → stamp suggestion_applied_at/by, auto-resolve the thread, ws commentUpdated, audit COMMENT_SUGGESTION_APPLIED; - already-applied (DB) → idempotent success (no re-apply), self-healing the resolve if it was missed — satisfies the issue's double-click / two-user race requirement; - collab verdict applied:false && currentText===suggestedText → idempotent success (crash between doc mutation and DB write); - text changed → 409 ConflictException carrying currentText; - gateway undefined/throw → hard error, never a silent success. - audit-events: COMMENT_SUGGESTION_APPLIED. Tests: create validation (reply/no-selection/equal-to-selection rejected; valid stored) + applySuggestion verdict branches incl. both idempotent paths. jest src/core/comment: 33 passed. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/server/src/common/events/audit-events.ts | 1 + .../src/core/comment/comment.controller.ts | 37 +++ .../comment.service.apply-suggestion.spec.ts | 245 ++++++++++++++++++ .../comment/comment.service.behavior.spec.ts | 93 +++++++ ...mment.service.provenance-broadcast.spec.ts | 3 + .../src/core/comment/comment.service.spec.ts | 1 + .../src/core/comment/comment.service.ts | 192 +++++++++++++- .../core/comment/dto/apply-suggestion.dto.ts | 6 + .../core/comment/dto/create-comment.dto.ts | 18 +- 9 files changed, 594 insertions(+), 2 deletions(-) create mode 100644 apps/server/src/core/comment/comment.service.apply-suggestion.spec.ts create mode 100644 apps/server/src/core/comment/dto/apply-suggestion.dto.ts diff --git a/apps/server/src/common/events/audit-events.ts b/apps/server/src/common/events/audit-events.ts index d8be76f8..b7cfabe5 100644 --- a/apps/server/src/common/events/audit-events.ts +++ b/apps/server/src/common/events/audit-events.ts @@ -51,6 +51,7 @@ export const AuditEvent = { COMMENT_UPDATED: 'comment.updated', COMMENT_RESOLVED: 'comment.resolved', COMMENT_REOPENED: 'comment.reopened', + COMMENT_SUGGESTION_APPLIED: 'comment.suggestion_applied', // Page PAGE_CREATED: 'page.created', diff --git a/apps/server/src/core/comment/comment.controller.ts b/apps/server/src/core/comment/comment.controller.ts index 96e8d5d2..f04f32e5 100644 --- a/apps/server/src/core/comment/comment.controller.ts +++ b/apps/server/src/core/comment/comment.controller.ts @@ -14,6 +14,7 @@ import { CommentService } from './comment.service'; import { CreateCommentDto } from './dto/create-comment.dto'; import { UpdateCommentDto } from './dto/update-comment.dto'; import { ResolveCommentDto } from './dto/resolve-comment.dto'; +import { ApplySuggestionDto } from './dto/apply-suggestion.dto'; import { PageIdDto, CommentIdDto } from './dto/comments.input'; import { AuthUser } from '../../common/decorators/auth-user.decorator'; import { AuthWorkspace } from '../../common/decorators/auth-workspace.decorator'; @@ -197,6 +198,42 @@ export class CommentController { return updated; } + @HttpCode(HttpStatus.OK) + @Post('apply-suggestion') + async applySuggestion( + @Body() dto: ApplySuggestionDto, + @AuthUser() user: User, + @AuthWorkspace() workspace: Workspace, + @AuthProvenance() provenance: AuthProvenanceData, + ) { + const comment = await this.commentRepo.findById(dto.commentId, { + includeCreator: true, + includeResolvedBy: true, + }); + if (!comment) { + throw new NotFoundException('Comment not found'); + } + + const page = await this.pageRepo.findById(comment.pageId); + if (!page || page.deletedAt) { + throw new NotFoundException('Page not found'); + } + + // Authorize BEFORE revealing any structural detail about the comment + // (metadata-disclosure hygiene). Applying a suggestion rewrites the page + // text, so require edit access (NOT just comment access). Running this + // first means a cross-workspace user with a guessed comment UUID gets a + // uniform 403 regardless of the comment's type or suggestion state — it can + // never distinguish those before the access check. The structural 400s + // (top-level / has-a-suggested-edit) are re-checked by the service below. + await this.pageAccessService.validateCanEdit(page, user); + + // The service re-validates the comment's state, returns idempotent success + // for an already-applied suggestion, and lets ConflictException (409, with + // currentText in the payload) propagate untouched. + return this.commentService.applySuggestion(comment, user, provenance); + } + @HttpCode(HttpStatus.OK) @Post('delete') async delete(@Body() input: CommentIdDto, @AuthUser() user: User, @AuthWorkspace() workspace: Workspace) { diff --git a/apps/server/src/core/comment/comment.service.apply-suggestion.spec.ts b/apps/server/src/core/comment/comment.service.apply-suggestion.spec.ts new file mode 100644 index 00000000..aa11f41e --- /dev/null +++ b/apps/server/src/core/comment/comment.service.apply-suggestion.spec.ts @@ -0,0 +1,245 @@ +import { + BadRequestException, + ConflictException, + InternalServerErrorException, +} from '@nestjs/common'; +import { CommentService } from './comment.service'; +import { AuditEvent, AuditResource } from '../../common/events/audit-events'; + +/** + * Focused coverage for CommentService.applySuggestion (comment.service.ts). + * The service is constructed directly with jest-mocked deps (the @InjectQueue + * tokens can't be resolved by Test.createTestingModule — see the sibling specs). + * + * The collaboration gateway verdict is the pivot of the whole flow, so each test + * pins a specific { applied, currentText } and asserts the DB persistence, + * auto-resolve, audit, ws broadcast, and error mapping that follow from it. + */ +describe('CommentService — applySuggestion', () => { + const UPDATED = { id: 'c-1', __updated: true } as any; + + function makeService(verdict: unknown) { + const commentRepo: any = { + // Both the applied-stamp re-read and resolveComment's re-read go through + // findById; return a recognizable enriched row. + findById: jest.fn(async () => UPDATED), + updateComment: jest.fn(async () => undefined), + }; + const pageRepo: any = {}; + const wsService: any = { emitCommentEvent: jest.fn() }; + const collaborationGateway: any = { + handleYjsEvent: jest.fn(async () => verdict), + }; + const generalQueue: any = { add: jest.fn(() => Promise.resolve()) }; + const notificationQueue: any = { add: jest.fn(async () => undefined) }; + const auditService: any = { log: jest.fn() }; + + const service = new CommentService( + commentRepo, + pageRepo, + wsService, + collaborationGateway, + generalQueue, + notificationQueue, + auditService, + ); + + return { + service, + commentRepo, + wsService, + collaborationGateway, + auditService, + }; + } + + const suggestionComment = (over?: Partial): any => ({ + id: 'c-1', + pageId: 'page-1', + spaceId: 'space-1', + workspaceId: 'ws-1', + creatorId: 'user-1', + parentCommentId: null, + selection: 'old text', + suggestedText: 'new text', + suggestionAppliedAt: null, + resolvedAt: null, + ...over, + }); + const user = (over?: Partial): any => ({ id: 'user-1', ...over }); + + // Pull the updateComment patch that carries the applied stamps. + const appliedPatch = (commentRepo: any) => + commentRepo.updateComment.mock.calls + .map((c: any[]) => c[0]) + .find((patch: any) => 'suggestionAppliedAt' in patch); + + it('applied=true → replaces text, persists applied stamps, auto-resolves, audits, returns updated', async () => { + const { service, commentRepo, wsService, collaborationGateway, auditService } = + makeService({ applied: true, currentText: 'new text' }); + + const result = await service.applySuggestion(suggestionComment(), user()); + + // The atomic replace was requested against the exact marked text. + expect(collaborationGateway.handleYjsEvent).toHaveBeenCalledWith( + 'applyCommentSuggestion', + 'page.page-1', + expect.objectContaining({ + commentId: 'c-1', + expectedText: 'old text', + newText: 'new text', + user: expect.objectContaining({ id: 'user-1' }), + }), + ); + + // Applied stamps persisted. + const patch = appliedPatch(commentRepo); + expect(patch.suggestionAppliedAt).toBeInstanceOf(Date); + expect(patch.suggestionAppliedById).toBe('user-1'); + + // Auto-resolved: resolveComment writes a resolvedAt/resolvedById patch too. + const resolvePatch = commentRepo.updateComment.mock.calls + .map((c: any[]) => c[0]) + .find((p: any) => 'resolvedAt' in p); + expect(resolvePatch.resolvedAt).toBeInstanceOf(Date); + expect(resolvePatch.resolvedById).toBe('user-1'); + + // Audit + broadcast + return. + expect(auditService.log).toHaveBeenCalledWith( + expect.objectContaining({ + event: AuditEvent.COMMENT_SUGGESTION_APPLIED, + resourceType: AuditResource.COMMENT, + resourceId: 'c-1', + spaceId: 'space-1', + metadata: { pageId: 'page-1' }, + }), + ); + expect(wsService.emitCommentEvent).toHaveBeenCalledWith( + 'space-1', + 'page-1', + expect.objectContaining({ operation: 'commentUpdated', comment: UPDATED }), + ); + expect(result).toBe(UPDATED); + }); + + it('applied=false but currentText === suggestedText → idempotent success (no 409)', async () => { + const { service, commentRepo, auditService } = makeService({ + applied: false, + currentText: 'new text', + }); + + const result = await service.applySuggestion(suggestionComment(), user()); + + // The stamps are still persisted (reconciling a crash between the doc + // mutation and the DB write) and the call succeeds. + const patch = appliedPatch(commentRepo); + expect(patch.suggestionAppliedAt).toBeInstanceOf(Date); + expect(patch.suggestionAppliedById).toBe('user-1'); + expect(auditService.log).toHaveBeenCalledTimes(1); + expect(result).toBe(UPDATED); + }); + + it('applied=false and currentText differs → ConflictException with currentText in payload', async () => { + const { service, commentRepo, auditService } = makeService({ + applied: false, + currentText: 'someone else edited this', + }); + + const err = await service + .applySuggestion(suggestionComment(), user()) + .catch((e) => e); + + expect(err).toBeInstanceOf(ConflictException); + expect(err.getResponse()).toMatchObject({ + currentText: 'someone else edited this', + }); + // No persistence and no audit on a conflict. + expect(appliedPatch(commentRepo)).toBeUndefined(); + expect(auditService.log).not.toHaveBeenCalled(); + }); + + it('already-applied AND already-resolved → idempotent success, no collab call, no re-resolve (#315 double-click)', async () => { + const { service, collaborationGateway, commentRepo, auditService } = + makeService({ applied: true, currentText: 'new text' }); + + const result = await service.applySuggestion( + suggestionComment({ + suggestionAppliedAt: new Date(), + resolvedAt: new Date(), + resolvedById: 'user-1', + }), + user(), + ); + + // Idempotent SUCCESS, not a 409. The suggestion is already applied, so the + // collaborative document is never touched again and nothing is re-stamped + // or re-resolved. + expect(result).toBe(UPDATED); + expect(collaborationGateway.handleYjsEvent).not.toHaveBeenCalled(); + expect(commentRepo.updateComment).not.toHaveBeenCalled(); + // Same success shape as the applied path (broadcast + audit). + expect(auditService.log).toHaveBeenCalledTimes(1); + }); + + it('already-applied but NOT resolved (crash window) → idempotent success, self-heals resolve, no re-apply', async () => { + const { service, collaborationGateway, commentRepo } = makeService({ + applied: true, + currentText: 'new text', + }); + + const result = await service.applySuggestion( + suggestionComment({ suggestionAppliedAt: new Date(), resolvedAt: null }), + user(), + ); + + expect(result).toBe(UPDATED); + + // The suggestion is NOT re-applied to the document… + expect(collaborationGateway.handleYjsEvent).not.toHaveBeenCalledWith( + 'applyCommentSuggestion', + expect.anything(), + expect.anything(), + ); + // …but the open thread is self-healed to resolved via resolveComment, which + // writes the resolve patch and updates the resolve mark. + const resolvePatch = commentRepo.updateComment.mock.calls + .map((c: any[]) => c[0]) + .find((p: any) => 'resolvedAt' in p); + expect(resolvePatch.resolvedAt).toBeInstanceOf(Date); + expect(resolvePatch.resolvedById).toBe('user-1'); + expect(collaborationGateway.handleYjsEvent).toHaveBeenCalledWith( + 'resolveCommentMark', + 'page.page-1', + expect.objectContaining({ commentId: 'c-1', resolved: true }), + ); + // The applied stamps are NOT re-written (already stamped). + expect(appliedPatch(commentRepo)).toBeUndefined(); + }); + + it('rejects a comment with no suggestedText', async () => { + const { service, collaborationGateway } = makeService({ + applied: true, + currentText: 'x', + }); + + await expect( + service.applySuggestion( + suggestionComment({ suggestedText: null }), + user(), + ), + ).rejects.toThrow(BadRequestException); + expect(collaborationGateway.handleYjsEvent).not.toHaveBeenCalled(); + }); + + it('gateway returning undefined → hard error, not a silent success', async () => { + const { service, commentRepo, auditService } = makeService(undefined); + + await expect( + service.applySuggestion(suggestionComment(), user()), + ).rejects.toThrow(InternalServerErrorException); + + // Nothing persisted, nothing audited. + expect(appliedPatch(commentRepo)).toBeUndefined(); + expect(auditService.log).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/server/src/core/comment/comment.service.behavior.spec.ts b/apps/server/src/core/comment/comment.service.behavior.spec.ts index 91572496..00f2aaaf 100644 --- a/apps/server/src/core/comment/comment.service.behavior.spec.ts +++ b/apps/server/src/core/comment/comment.service.behavior.spec.ts @@ -60,6 +60,7 @@ describe('CommentService — behavior', () => { }; const generalQueue: any = { add: jest.fn(() => Promise.resolve()) }; const notificationQueue: any = { add: jest.fn(async () => undefined) }; + const auditService: any = { log: jest.fn() }; const service = new CommentService( commentRepo, @@ -68,14 +69,17 @@ describe('CommentService — behavior', () => { collaborationGateway, generalQueue, notificationQueue, + auditService, ); return { service, commentRepo, wsService, + collaborationGateway, generalQueue, notificationQueue, + auditService, }; } @@ -181,6 +185,95 @@ describe('CommentService — behavior', () => { }); }); + describe('create — suggested edit validation & storage', () => { + it('rejects a suggestedText on a reply (not a top-level comment)', async () => { + const parentComment = { + id: 'parent-1', + pageId: 'page-1', + parentCommentId: null, + }; + const { service, commentRepo } = makeService({ parentComment }); + + await expect( + service.create( + { page: page(), workspaceId: 'ws-1', user: user() }, + { + content: JSON.stringify(docMentioning()), + parentCommentId: 'parent-1', + selection: 'hello world', + suggestedText: 'goodbye world', + } as any, + ), + ).rejects.toThrow(BadRequestException); + + expect(commentRepo.insertComment).not.toHaveBeenCalled(); + }); + + it('rejects a suggestedText without a selection', async () => { + const { service, commentRepo } = makeService(); + + await expect( + service.create( + { page: page(), workspaceId: 'ws-1', user: user() }, + { + content: JSON.stringify(docMentioning()), + suggestedText: 'new text', + } as any, + ), + ).rejects.toThrow(BadRequestException); + + expect(commentRepo.insertComment).not.toHaveBeenCalled(); + }); + + it('rejects a suggestedText identical to the selection (no-op)', async () => { + const { service, commentRepo } = makeService(); + + await expect( + service.create( + { page: page(), workspaceId: 'ws-1', user: user() }, + { + content: JSON.stringify(docMentioning()), + selection: 'same text', + // Only differs by surrounding whitespace → still a no-op after trim. + suggestedText: ' same text ', + } as any, + ), + ).rejects.toThrow(BadRequestException); + + expect(commentRepo.insertComment).not.toHaveBeenCalled(); + }); + + it('stores a valid suggestedText (trimmed) on the inserted row', async () => { + const { service, commentRepo } = makeService(); + + await service.create( + { page: page(), workspaceId: 'ws-1', user: user() }, + { + content: JSON.stringify(docMentioning()), + selection: 'old text', + type: 'inline', + suggestedText: ' new text ', + } as any, + ); + + const insertArg = commentRepo.insertComment.mock.calls[0][0]; + expect(insertArg.suggestedText).toBe('new text'); + expect(insertArg.selection).toBe('old text'); + }); + + it('leaves suggestedText null for an ordinary comment', async () => { + const { service, commentRepo } = makeService(); + + await service.create( + { page: page(), workspaceId: 'ws-1', user: user() }, + { content: JSON.stringify(docMentioning()) } as any, + ); + + const insertArg = commentRepo.insertComment.mock.calls[0][0]; + expect(insertArg.suggestedText).toBeNull(); + }); + }); + describe('resolveComment — provenance & resolve notifications', () => { it('stamps resolvedSource:"agent" when an agent resolves', async () => { const { service, commentRepo } = makeService(); diff --git a/apps/server/src/core/comment/comment.service.provenance-broadcast.spec.ts b/apps/server/src/core/comment/comment.service.provenance-broadcast.spec.ts index 2d962f1a..e2c828d5 100644 --- a/apps/server/src/core/comment/comment.service.provenance-broadcast.spec.ts +++ b/apps/server/src/core/comment/comment.service.provenance-broadcast.spec.ts @@ -61,6 +61,8 @@ describe('CommentService — broadcast carries the agent avatar stack', () => { const generalQueue: any = { add: jest.fn(() => Promise.resolve()) }; const notificationQueue: any = { add: jest.fn(async () => undefined) }; + const auditService: any = { log: jest.fn() }; + const service = new CommentService( commentRepo, pageRepo, @@ -68,6 +70,7 @@ describe('CommentService — broadcast carries the agent avatar stack', () => { collaborationGateway, generalQueue, notificationQueue, + auditService, ); return { service, commentRepo, wsService }; diff --git a/apps/server/src/core/comment/comment.service.spec.ts b/apps/server/src/core/comment/comment.service.spec.ts index 9384a2b8..102bb5fd 100644 --- a/apps/server/src/core/comment/comment.service.spec.ts +++ b/apps/server/src/core/comment/comment.service.spec.ts @@ -14,6 +14,7 @@ describe('CommentService', () => { {} as any, // collaborationGateway {} as any, // generalQueue {} as any, // notificationQueue + {} as any, // auditService ); }); diff --git a/apps/server/src/core/comment/comment.service.ts b/apps/server/src/core/comment/comment.service.ts index 98244b6a..3a251b58 100644 --- a/apps/server/src/core/comment/comment.service.ts +++ b/apps/server/src/core/comment/comment.service.ts @@ -1,7 +1,10 @@ import { BadRequestException, + ConflictException, ForbiddenException, + Inject, Injectable, + InternalServerErrorException, Logger, NotFoundException, } from '@nestjs/common'; @@ -26,6 +29,11 @@ import { AuthProvenanceData, agentSourceFields, } from '../../common/decorators/auth-provenance.decorator'; +import { AuditEvent, AuditResource } from '../../common/events/audit-events'; +import { + AUDIT_SERVICE, + IAuditService, +} from '../../integrations/audit/audit.service'; @Injectable() export class CommentService { @@ -40,6 +48,7 @@ export class CommentService { private generalQueue: Queue, @InjectQueue(QueueName.NOTIFICATION_QUEUE) private notificationQueue: Queue, + @Inject(AUDIT_SERVICE) private auditService: IAuditService, ) {} async findById(commentId: string) { @@ -78,15 +87,50 @@ export class CommentService { } } + const selection = createCommentDto?.selection?.substring(0, 250) ?? null; + + // A suggested edit rewrites the exact text under an inline comment mark, so + // it is only meaningful on a top-level inline comment that carries a + // selection, and only if the suggestion actually changes that text. + let suggestedText: string | null = null; + if ( + createCommentDto.suggestedText !== undefined && + createCommentDto.suggestedText !== null + ) { + if (createCommentDto.parentCommentId) { + throw new BadRequestException( + 'A suggested edit can only be attached to a top-level comment, not a reply', + ); + } + if (!selection || selection.trim().length === 0) { + throw new BadRequestException( + 'A suggested edit requires an inline comment with a non-empty text selection', + ); + } + const trimmed = createCommentDto.suggestedText.trim(); + if (trimmed.length === 0) { + throw new BadRequestException('A suggested edit cannot be empty'); + } + // A no-op suggestion (identical to the selection) is meaningless and would + // make "apply" indistinguishable from "already applied". + if (trimmed === selection.trim()) { + throw new BadRequestException( + 'A suggested edit must differ from the selected text', + ); + } + suggestedText = trimmed; + } + const inserted = await this.commentRepo.insertComment({ pageId: page.id, content: commentContent, - selection: createCommentDto?.selection?.substring(0, 250) ?? null, + selection, type: createCommentDto.type ?? 'page', parentCommentId: createCommentDto?.parentCommentId, creatorId: user.id, workspaceId: workspaceId, spaceId: page.spaceId, + suggestedText, // Agent-edit provenance: the user stays creatorId; this only annotates the // source. Normal user requests leave the column default ('user'). ...agentSourceFields(provenance, 'createdSource', 'aiChatId'), @@ -299,6 +343,152 @@ export class CommentService { return updatedComment; } + /** + * Apply the suggested edit carried by a top-level inline comment: atomically + * replace the text under the comment mark in the collaborative document with + * the comment's suggestedText, then stamp the applied fields and auto-resolve + * the thread. The controller authorizes (validateCanEdit); this re-checks the + * comment's own state so the invariant holds regardless of caller. + */ + async applySuggestion( + comment: Comment, + user: User, + provenance?: AuthProvenanceData, + ): Promise { + // Structural guards. + if (comment.parentCommentId) { + throw new BadRequestException( + 'Only a top-level comment can carry a suggested edit', + ); + } + if (!comment.suggestedText) { + throw new BadRequestException('This comment has no suggested edit to apply'); + } + // State guards. Order matters — the already-applied check precedes the + // resolved check because an applied comment is normally also resolved. + // + // Already applied → IDEMPOTENT SUCCESS (issue #315 DoD: double-click / + // two-user race → idempotent "already applied", NOT a 409). The suggestion + // is already in the document, so do NOT call the collab gateway again. + // finalizeAppliedSuggestion re-fetches/broadcasts the same success shape as + // the applied branch and, when the thread is still open (the rare "applied + // but not resolved" crash window), self-heals it via resolveComment. + if (comment.suggestionAppliedAt) { + return this.finalizeAppliedSuggestion(comment, user, provenance); + } + // Not-yet-applied on a resolved thread → reject. The client hides the apply + // button once a thread is resolved; this is the defensive server check. + if (comment.resolvedAt) { + throw new BadRequestException( + 'Cannot apply a suggested edit on a resolved comment thread', + ); + } + + // Derive the document name the same way create()/resolveComment() do for + // the comment marks: `page.${pageId}`. + const documentName = `page.${comment.pageId}`; + + let verdict: { applied: boolean; currentText: string | null } | undefined; + try { + verdict = await this.collaborationGateway.handleYjsEvent( + 'applyCommentSuggestion', + documentName, + { + commentId: comment.id, + expectedText: comment.selection, + newText: comment.suggestedText, + user, + }, + ); + } catch (error) { + // A throwing gateway (or the phase-3 fallback failing) is a hard error — + // never silently succeed, the document may or may not have changed. + this.logger.error( + `Failed to apply suggested edit for comment ${comment.id}`, + error, + ); + throw new InternalServerErrorException('Failed to apply the suggested edit'); + } + + if (!verdict) { + // Should not happen given the phase-3 fallback; treat as a hard error + // rather than assuming success. + throw new InternalServerErrorException('Failed to apply the suggested edit'); + } + + if (verdict.applied === true) { + return this.finalizeAppliedSuggestion(comment, user, provenance); + } + + // Idempotent branch: the mutation didn't run now, but the text under the + // mark is ALREADY the suggested text (double-click, two-user race, or a + // crash between the doc mutation and the DB write). Reconcile the DB / + // resolved state and report success — do NOT 409. + if ( + verdict.applied === false && + verdict.currentText === comment.suggestedText + ) { + return this.finalizeAppliedSuggestion(comment, user, provenance); + } + + // The commented text changed since the suggestion was made. Surface the + // current text so the client can tell the user what it is now. + throw new ConflictException({ + message: + 'The commented text changed since this suggestion was made; it was not applied.', + currentText: verdict.currentText, + }); + } + + /** + * Persist the applied stamps (idempotently), auto-resolve the thread and + * broadcast + audit the applied suggestion. Shared by the applied and the + * idempotent "already-applied" branches of applySuggestion. + */ + private async finalizeAppliedSuggestion( + comment: Comment, + user: User, + provenance?: AuthProvenanceData, + ): Promise { + if (!comment.suggestionAppliedAt) { + await this.commentRepo.updateComment( + { + suggestionAppliedAt: new Date(), + suggestionAppliedById: user.id, + }, + comment.id, + ); + } + + // Auto-resolve the thread. resolveComment handles the resolve mark, its ws + // broadcast and the resolve notification. The guard above guarantees the + // thread was open when we entered, but stay defensive on re-entry. + if (!comment.resolvedAt) { + await this.resolveComment(comment, true, user, provenance); + } + + const updatedComment = await this.commentRepo.findById(comment.id, { + includeCreator: true, + includeResolvedBy: true, + }); + + this.wsService.emitCommentEvent(comment.spaceId, comment.pageId, { + operation: 'commentUpdated', + pageId: comment.pageId, + comment: updatedComment, + }); + + this.auditService.log({ + event: AuditEvent.COMMENT_SUGGESTION_APPLIED, + resourceType: AuditResource.COMMENT, + resourceId: comment.id, + spaceId: comment.spaceId, + metadata: { pageId: comment.pageId }, + }); + + return updatedComment; + } + private async queueCommentNotification( content: any, oldMentionIds: string[], diff --git a/apps/server/src/core/comment/dto/apply-suggestion.dto.ts b/apps/server/src/core/comment/dto/apply-suggestion.dto.ts new file mode 100644 index 00000000..0c93f19e --- /dev/null +++ b/apps/server/src/core/comment/dto/apply-suggestion.dto.ts @@ -0,0 +1,6 @@ +import { IsUUID } from 'class-validator'; + +export class ApplySuggestionDto { + @IsUUID() + commentId: string; +} diff --git a/apps/server/src/core/comment/dto/create-comment.dto.ts b/apps/server/src/core/comment/dto/create-comment.dto.ts index c82ae187..b556c627 100644 --- a/apps/server/src/core/comment/dto/create-comment.dto.ts +++ b/apps/server/src/core/comment/dto/create-comment.dto.ts @@ -1,4 +1,12 @@ -import { IsIn, IsJSON, IsObject, IsOptional, IsString, IsUUID } from 'class-validator'; +import { + IsIn, + IsJSON, + IsObject, + IsOptional, + IsString, + IsUUID, + MaxLength, +} from 'class-validator'; import { z } from 'zod'; const yjsIdSchema = z.object({ @@ -43,4 +51,12 @@ export class CreateCommentDto { anchor: any; head: any; }; + + // Optional suggested replacement for the selected text (a "suggested edit"). + // Only valid on a top-level inline comment that carries a non-empty selection; + // enforced in CommentService.create. + @IsOptional() + @IsString() + @MaxLength(2000) + suggestedText?: string; } From b62db917dedb3776395089533159033ef323b813 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Fri, 3 Jul 2026 19:19:36 +0300 Subject: [PATCH 5/7] feat(comment): suggestion diff block + Apply button + mutation (#315 phase 5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Client UI for agent comment suggestions. - IComment gains suggestedText / suggestionAppliedAt / suggestionAppliedById. - comment-list-item shows a "было → стало" block (old selection struck/red, new suggestedText green) for a top-level comment with a suggestion, plus an Apply button — gated by canShowApply(comment, canEdit): edit permission AND a suggestion AND not applied AND not resolved AND top-level. Once applied, an "Applied" badge replaces the button. - canEdit comes from page.permissions.canEdit (real edit permission, NOT the looser canComment) and is threaded through CommentListItem and nested ChildComments; fail-closed when undefined. - useApplySuggestionMutation posts to /comments/apply-suggestion; on success it writes the applied + server auto-resolve fields into the react-query cache (UI flips to Applied + resolved without a refetch); on 409 it shows a specific message with the server's currentText, else a generic error. - i18n keys added in en-US + ru-RU. Tests (comment-list-item.test.tsx + canShowApply unit suite): Apply visibility across canEdit/applied/resolved/reply, click dispatches the mutation, diff rendering. 34 passed. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../public/locales/en-US/translation.json | 7 +- .../public/locales/ru-RU/translation.json | 7 +- .../components/comment-list-item.test.tsx | 101 +++++++++++++++++- .../comment/components/comment-list-item.tsx | 63 ++++++++++- .../components/comment-list-with-tabs.tsx | 19 +++- .../comment/components/comment.module.css | 32 ++++++ .../features/comment/queries/comment-query.ts | 58 ++++++++++ .../comment/services/comment-service.ts | 7 ++ .../features/comment/types/comment.types.ts | 7 ++ .../src/features/comment/utils/suggestion.ts | 14 +++ 10 files changed, 307 insertions(+), 8 deletions(-) create mode 100644 apps/client/src/features/comment/utils/suggestion.ts diff --git a/apps/client/public/locales/en-US/translation.json b/apps/client/public/locales/en-US/translation.json index 5dec73ff..9562d814 100644 --- a/apps/client/public/locales/en-US/translation.json +++ b/apps/client/public/locales/en-US/translation.json @@ -1373,5 +1373,10 @@ "Updated to the latest version": "Updated to the latest version", "This role is no longer in the catalog": "This role is no longer in the catalog", "This language is no longer available in the catalog": "This language is no longer available in the catalog", - "Connecting… (read-only)": "Connecting… (read-only)" + "Connecting… (read-only)": "Connecting… (read-only)", + "Apply": "Apply", + "Applied": "Applied", + "Suggestion applied": "Suggestion applied", + "Failed to apply suggestion": "Failed to apply suggestion", + "The commented text changed since this suggestion was made; it was not applied.": "The commented text changed since this suggestion was made; it was not applied." } diff --git a/apps/client/public/locales/ru-RU/translation.json b/apps/client/public/locales/ru-RU/translation.json index be16c5c9..e7a614c7 100644 --- a/apps/client/public/locales/ru-RU/translation.json +++ b/apps/client/public/locales/ru-RU/translation.json @@ -1229,5 +1229,10 @@ "Updated to the latest version": "Обновлено до последней версии", "This role is no longer in the catalog": "Эта роль больше не представлена в каталоге", "This language is no longer available in the catalog": "Этот язык больше не доступен в каталоге", - "Connecting… (read-only)": "Подключение… (только чтение)" + "Connecting… (read-only)": "Подключение… (только чтение)", + "Apply": "Применить", + "Applied": "Применено", + "Suggestion applied": "Предложение применено", + "Failed to apply suggestion": "Не удалось применить предложение", + "The commented text changed since this suggestion was made; it was not applied.": "Прокомментированный текст изменился после создания предложения; оно не было применено." } diff --git a/apps/client/src/features/comment/components/comment-list-item.test.tsx b/apps/client/src/features/comment/components/comment-list-item.test.tsx index 1826e4ae..8b75b337 100644 --- a/apps/client/src/features/comment/components/comment-list-item.test.tsx +++ b/apps/client/src/features/comment/components/comment-list-item.test.tsx @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from "vitest"; -import { render, screen } from "@testing-library/react"; +import { render, screen, fireEvent } from "@testing-library/react"; import { MantineProvider } from "@mantine/core"; import { IComment } from "@/features/comment/types/comment.types"; @@ -7,10 +7,15 @@ import { IComment } from "@/features/comment/types/comment.types"; // The comment mutation hooks reach out to react-query/network — stub them so the // component renders in isolation. We only assert the AI-badge rendering branch. +const applyMutateAsync = vi.fn(); vi.mock("@/features/comment/queries/comment-query", () => ({ useDeleteCommentMutation: () => ({ mutateAsync: vi.fn() }), useResolveCommentMutation: () => ({ mutateAsync: vi.fn() }), useUpdateCommentMutation: () => ({ mutateAsync: vi.fn() }), + useApplySuggestionMutation: () => ({ + mutateAsync: applyMutateAsync, + isPending: false, + }), })); // CommentEditor pulls in the full TipTap editor stack; replace it with a stub. @@ -19,6 +24,7 @@ vi.mock("@/features/comment/components/comment-editor", () => ({ })); import CommentListItem from "./comment-list-item"; +import { canShowApply } from "@/features/comment/utils/suggestion"; const baseComment = (over?: Partial): IComment => ({ @@ -32,10 +38,15 @@ const baseComment = (over?: Partial): IComment => ...over, }) as IComment; -function renderItem(comment: IComment) { +function renderItem(comment: IComment, canEdit = true) { return render( - + , ); } @@ -87,3 +98,87 @@ describe("CommentListItem — agent avatar stack", () => { // are covered directly in agent-avatar-stack.test.tsx; this integration suite // only guards the insertion gate (agent → stack, user → no stack). }); + +describe("CommentListItem — suggested edit (#315)", () => { + const suggestion = (over?: Partial): IComment => + baseComment({ + selection: "old wording here", + suggestedText: "new wording here", + ...over, + }); + + it("renders the было→стало diff and an Apply button when canEdit and not applied/resolved", () => { + renderItem(suggestion(), true); + // Old text appears both as the selection quote and as the struck diff row. + expect(screen.getAllByText("old wording here").length).toBeGreaterThan(0); + expect(screen.getByText("new wording here")).toBeDefined(); + // Apply button is present. + expect(screen.getByRole("button", { name: "Apply" })).toBeDefined(); + // No Applied badge yet. + expect(screen.queryByText("Applied")).toBeNull(); + }); + + it("hides the Apply button when canEdit is false", () => { + renderItem(suggestion(), false); + // Diff still renders... + expect(screen.getByText("new wording here")).toBeDefined(); + // ...but no Apply button. + expect(screen.queryByRole("button", { name: "Apply" })).toBeNull(); + }); + + it("shows an Applied badge (no Apply button) once suggestionAppliedAt is set", () => { + renderItem(suggestion({ suggestionAppliedAt: new Date() }), true); + expect(screen.getByText("Applied")).toBeDefined(); + expect(screen.queryByRole("button", { name: "Apply" })).toBeNull(); + }); + + it("hides the Apply button once the thread is resolved", () => { + renderItem(suggestion({ resolvedAt: new Date() }), true); + expect(screen.queryByRole("button", { name: "Apply" })).toBeNull(); + }); + + it("calls the apply mutation when the Apply button is clicked", () => { + applyMutateAsync.mockClear(); + renderItem(suggestion(), true); + fireEvent.click(screen.getByRole("button", { name: "Apply" })); + expect(applyMutateAsync).toHaveBeenCalledWith({ + commentId: "c-1", + pageId: "page-1", + }); + }); + + it("does not render the diff block for a reply (child) comment", () => { + renderItem( + suggestion({ parentCommentId: "c-0" }), + true, + ); + expect(screen.queryByText("new wording here")).toBeNull(); + expect(screen.queryByRole("button", { name: "Apply" })).toBeNull(); + }); +}); + +describe("canShowApply predicate", () => { + const c = (over?: Partial): IComment => + ({ suggestedText: "x", ...over }) as IComment; + + it("true when suggestion present, editable, not applied/resolved, top-level", () => { + expect(canShowApply(c(), true)).toBe(true); + }); + it("false without edit permission", () => { + expect(canShowApply(c(), false)).toBe(false); + }); + it("false when no suggestion", () => { + expect(canShowApply(c({ suggestedText: null }), true)).toBe(false); + }); + it("false when already applied", () => { + expect(canShowApply(c({ suggestionAppliedAt: new Date() }), true)).toBe( + false, + ); + }); + it("false when resolved", () => { + expect(canShowApply(c({ resolvedAt: new Date() }), true)).toBe(false); + }); + it("false for a reply comment", () => { + expect(canShowApply(c({ parentCommentId: "p" }), true)).toBe(false); + }); +}); diff --git a/apps/client/src/features/comment/components/comment-list-item.tsx b/apps/client/src/features/comment/components/comment-list-item.tsx index 072281bb..0d4b5e02 100644 --- a/apps/client/src/features/comment/components/comment-list-item.tsx +++ b/apps/client/src/features/comment/components/comment-list-item.tsx @@ -1,4 +1,4 @@ -import { Group, Text, Box } from "@mantine/core"; +import { Group, Text, Box, Badge, Button } from "@mantine/core"; import { AgentAvatarStack } from "@/components/ui/agent-avatar-stack.tsx"; import React, { useEffect, useRef, useState } from "react"; import classes from "./comment.module.css"; @@ -11,11 +11,13 @@ import CommentMenu from "@/features/comment/components/comment-menu"; import ResolveComment from "@/features/comment/components/resolve-comment"; import { useHover } from "@mantine/hooks"; import { + useApplySuggestionMutation, useDeleteCommentMutation, useResolveCommentMutation, useUpdateCommentMutation, } from "@/features/comment/queries/comment-query"; import { IComment } from "@/features/comment/types/comment.types"; +import { canShowApply } from "@/features/comment/utils/suggestion"; import { CustomAvatar } from "@/components/ui/custom-avatar.tsx"; import { currentUserAtom } from "@/features/user/atoms/current-user-atom.ts"; import { useTranslation } from "react-i18next"; @@ -24,6 +26,10 @@ interface CommentListItemProps { comment: IComment; pageId: string; canComment: boolean; + // Real page-edit permission (page.permissions.canEdit) — gates the suggestion + // "Apply" button. Distinct from `canComment`, which may be looser (viewers + // allowed to comment cannot apply edits). + canEdit?: boolean; userSpaceRole?: string; } @@ -31,6 +37,7 @@ function CommentListItem({ comment, pageId, canComment, + canEdit, userSpaceRole, }: CommentListItemProps) { const { t } = useTranslation(); @@ -43,6 +50,7 @@ function CommentListItem({ const updateCommentMutation = useUpdateCommentMutation(); const deleteCommentMutation = useDeleteCommentMutation(comment.pageId); const resolveCommentMutation = useResolveCommentMutation(); + const applySuggestionMutation = useApplySuggestionMutation(); const [currentUser] = useAtom(currentUserAtom); const createdAtAgo = useTimeAgo(comment.createdAt); @@ -95,6 +103,18 @@ function CommentListItem({ } } + async function handleApplySuggestion() { + try { + await applySuggestionMutation.mutateAsync({ + commentId: comment.id, + pageId: comment.pageId, + }); + } catch (error) { + // Errors surface via the mutation's onError notification (incl. 409). + console.error("Failed to apply suggestion:", error); + } + } + function handleCommentClick(comment: IComment) { const el = document.querySelector( `.comment-mark[data-comment-id="${comment.id}"]`, @@ -211,6 +231,47 @@ function CommentListItem({ )} + {/* Suggested-edit (#315): "было → стало" diff for a top-level comment + carrying a suggestion. Old text struck-through/red, new text green. */} + {!comment.parentCommentId && comment.suggestedText && ( + + {comment.selection && ( + + {comment.selection} + + )} + + {comment.suggestedText} + + + {comment.suggestionAppliedAt ? ( + + {t("Applied")} + + ) : ( + canShowApply(comment, canEdit) && ( + + ) + )} + + )} + {!isEditing ? ( ) : ( diff --git a/apps/client/src/features/comment/components/comment-list-with-tabs.tsx b/apps/client/src/features/comment/components/comment-list-with-tabs.tsx index a29d3da8..af9d0783 100644 --- a/apps/client/src/features/comment/components/comment-list-with-tabs.tsx +++ b/apps/client/src/features/comment/components/comment-list-with-tabs.tsx @@ -49,8 +49,10 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) { const [isLoading, setIsLoading] = useState(false); const { data: space } = useGetSpaceBySlugQuery(page?.space?.slug); + const canEdit = page?.permissions?.canEdit ?? false; + const canComment = - (page?.permissions?.canEdit ?? false) || + canEdit || (space?.settings?.comments?.allowViewerComments === true); // Separate active and resolved comments @@ -137,6 +139,7 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) { comment={comment} pageId={page?.id} canComment={canComment} + canEdit={canEdit} userSpaceRole={space?.membership?.role} /> @@ -160,7 +164,14 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) { )} ), - [comments, handleAddReply, isLoading, space?.membership?.role, canComment], + [ + comments, + handleAddReply, + isLoading, + space?.membership?.role, + canComment, + canEdit, + ], ); if (isCommentsLoading) { @@ -300,6 +311,7 @@ interface ChildCommentsProps { parentId: string; pageId: string; canComment: boolean; + canEdit?: boolean; userSpaceRole?: string; } const ChildComments = ({ @@ -307,6 +319,7 @@ const ChildComments = ({ parentId, pageId, canComment, + canEdit, userSpaceRole, }: ChildCommentsProps) => { const getChildComments = useCallback( @@ -325,6 +338,7 @@ const ChildComments = ({ comment={childComment} pageId={pageId} canComment={canComment} + canEdit={canEdit} userSpaceRole={userSpaceRole} /> diff --git a/apps/client/src/features/comment/components/comment.module.css b/apps/client/src/features/comment/components/comment.module.css index 36362338..2a4b9397 100644 --- a/apps/client/src/features/comment/components/comment.module.css +++ b/apps/client/src/features/comment/components/comment.module.css @@ -21,6 +21,38 @@ box-sizing: border-box; } +/* Suggested-edit (#315) "было → стало" diff block. */ +.suggestionBlock { + margin-top: 8px; + margin-left: 6px; + padding: 6px; + border-radius: var(--mantine-radius-sm); + border: 1px solid var(--mantine-color-default-border); + overflow-wrap: break-word; + word-break: break-word; + max-width: 100%; + box-sizing: border-box; + display: flex; + flex-direction: column; + align-items: flex-start; +} + +.suggestionOld { + text-decoration: line-through; + color: var(--mantine-color-red-7); + background: var(--mantine-color-red-light); + border-radius: 2px; + padding: 1px 3px; +} + +.suggestionNew { + color: var(--mantine-color-green-9); + background: var(--mantine-color-green-light); + border-radius: 2px; + padding: 1px 3px; + margin-top: 4px; +} + .commentEditor { &[data-editable][data-surface="muted"] .ProseMirror:not(.focused) { diff --git a/apps/client/src/features/comment/queries/comment-query.ts b/apps/client/src/features/comment/queries/comment-query.ts index 5d637610..a1942532 100644 --- a/apps/client/src/features/comment/queries/comment-query.ts +++ b/apps/client/src/features/comment/queries/comment-query.ts @@ -5,6 +5,7 @@ import { InfiniteData, } from "@tanstack/react-query"; import { + applySuggestion, createComment, deleteComment, getPageComments, @@ -176,6 +177,63 @@ function updateCommentInCache( }; } +export function useApplySuggestionMutation() { + const queryClient = useQueryClient(); + const { t } = useTranslation(); + + return useMutation({ + // No optimistic update: apply can fail with 409 (the commented text drifted), + // so we only mutate the cache once the server confirms. + mutationFn: ({ commentId }) => applySuggestion(commentId), + onSuccess: (data, variables) => { + const cache = queryClient.getQueryData( + RQ_KEY(variables.pageId), + ) as InfiniteData> | undefined; + + if (cache) { + queryClient.setQueryData( + RQ_KEY(variables.pageId), + updateCommentInCache(cache, variables.commentId, (comment) => ({ + ...comment, + suggestionAppliedAt: data.suggestionAppliedAt, + suggestionAppliedById: data.suggestionAppliedById, + // The server auto-resolves the thread on apply — carry that through. + resolvedAt: data.resolvedAt, + resolvedById: data.resolvedById, + resolvedBy: data.resolvedBy, + })), + ); + } + + notifications.show({ message: t("Suggestion applied") }); + }, + onError: (err: any) => { + // 409 => the commented text changed since the suggestion was made. Surface + // a specific message (with the current text) rather than a generic error. + const status = err?.response?.status; + const currentText = err?.response?.data?.currentText; + if (status === 409 && typeof currentText === "string") { + const shortText = + currentText.length > 80 + ? `${currentText.slice(0, 80)}…` + : currentText; + notifications.show({ + title: t( + "The commented text changed since this suggestion was made; it was not applied.", + ), + message: shortText, + color: "red", + }); + return; + } + notifications.show({ + message: t("Failed to apply suggestion"), + color: "red", + }); + }, + }); +} + export function useResolveCommentMutation() { const queryClient = useQueryClient(); const { t } = useTranslation(); diff --git a/apps/client/src/features/comment/services/comment-service.ts b/apps/client/src/features/comment/services/comment-service.ts index f1512469..f536519a 100644 --- a/apps/client/src/features/comment/services/comment-service.ts +++ b/apps/client/src/features/comment/services/comment-service.ts @@ -18,6 +18,13 @@ export async function resolveComment(data: IResolveComment): Promise { return req.data; } +export async function applySuggestion(commentId: string): Promise { + // Mirrors resolveComment: let axios reject on non-2xx so the mutation can read + // the 409 body (`{ message, currentText }`) off err.response.data. + const req = await api.post("/comments/apply-suggestion", { commentId }); + return req.data.data ?? req.data; +} + export async function updateComment( data: Partial, ): Promise { diff --git a/apps/client/src/features/comment/types/comment.types.ts b/apps/client/src/features/comment/types/comment.types.ts index 31ff67be..ac4eac64 100644 --- a/apps/client/src/features/comment/types/comment.types.ts +++ b/apps/client/src/features/comment/types/comment.types.ts @@ -28,6 +28,13 @@ export interface IComment { createdSource?: string; aiChatId?: string | null; resolvedSource?: string | null; + // Suggested-edit (#315): when an agent proposes a replacement for the + // commented `selection`, `suggestedText` holds the "стало" text. Once a user + // applies it server-side the backend stamps `suggestionAppliedAt` / + // `suggestionAppliedById` and auto-resolves the thread. + suggestedText?: string | null; + suggestionAppliedAt?: Date | string | null; + suggestionAppliedById?: string | null; // Server-normalized "agent avatar stack" provenance (#300), present only when // createdSource === "agent": `agent` is the front identity, `launcher` the // human behind it (null for an external MCP agent). diff --git a/apps/client/src/features/comment/utils/suggestion.ts b/apps/client/src/features/comment/utils/suggestion.ts new file mode 100644 index 00000000..d14dea6e --- /dev/null +++ b/apps/client/src/features/comment/utils/suggestion.ts @@ -0,0 +1,14 @@ +import { IComment } from "@/features/comment/types/comment.types"; + +// Whether the suggested-edit (#315) "Apply" button should be shown for a +// comment: it must carry a suggestion, not already be applied or resolved, be a +// top-level comment, and the viewer must be able to edit the page. +export function canShowApply(comment: IComment, canEdit?: boolean): boolean { + return Boolean( + canEdit && + comment.suggestedText && + !comment.suggestionAppliedAt && + !comment.resolvedAt && + !comment.parentCommentId, + ); +} From cd539558ed4efcb422f2c1dc6f7b3d3ffae8c146 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Fri, 3 Jul 2026 19:35:47 +0300 Subject: [PATCH 6/7] feat(agent-tools): suggestedText on create_comment with strict anchor uniqueness (#315 phase 6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agents can attach a suggested replacement when creating an inline comment, via both the MCP create_comment tool and the AI-chat createComment tool. Because applying a suggestion edits the EXACT anchored text, an ambiguous anchor would let Apply corrupt the wrong occurrence. So when suggestedText is set the selection must occur EXACTLY ONCE: - new countAnchorMatches(doc, selection) counts occurrences across all blocks (same normalization/traversal as canAnchorInDoc), counting occurrences (2 in one block => 2) — stricter than block-count, never under-counting distinct occurrences (false-unique is the dangerous direction). - client.createComment gains suggestedText: a pre-check (getPageJson + countAnchorMatches: 0 => not-found, >=2 => ambiguity error) before create, and an AUTHORITATIVE live check inside the anchoring mutation that recomputes on the live doc and, if != 1, aborts and rolls back the just-created comment (reusing the existing safeDeleteComment "anchor not found" path). Ordinary comments keep first-occurrence behavior unchanged. - suggestedText is rejected on a reply or without selection in all three layers (MCP handler, MCP client, AI-chat tool), mirroring the server DTO/service. - filterComment surfaces suggestedText/suggestionAppliedAt/suggestionAppliedById. - DocmostClientLike.createComment signature updated. MCP build/ rebuilt. Tests: countAnchorMatches (0/1/N, within/across/nested block, span nodes, quote normalization); createComment (ambiguous refused pre-create, reply and no-selection rejected, unique succeeds and forwards suggestedText, filterComment surfaces it); ai-chat schema accepts suggestedText. MCP 443 pass; ai-chat 601 pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../tools/ai-chat-tools.service.spec.ts | 14 ++ .../ai-chat/tools/ai-chat-tools.service.ts | 51 +++- .../ai-chat/tools/docmost-client.loader.ts | 1 + packages/mcp/build/client.js | 75 +++++- packages/mcp/build/index.js | 27 ++- packages/mcp/build/lib/comment-anchor.js | 71 ++++++ packages/mcp/build/lib/filters.js | 5 + packages/mcp/src/client.ts | 88 ++++++- packages/mcp/src/index.ts | 32 ++- packages/mcp/src/lib/comment-anchor.ts | 68 ++++++ packages/mcp/src/lib/filters.ts | 5 + .../mcp/test/mock/create-comment.test.mjs | 223 ++++++++++++++++++ .../mcp/test/unit/comment-anchor.test.mjs | 66 ++++++ 13 files changed, 694 insertions(+), 32 deletions(-) diff --git a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.spec.ts b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.spec.ts index 33960a3f..53d38895 100644 --- a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.spec.ts +++ b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.spec.ts @@ -518,6 +518,20 @@ describe('AiChatToolsService model-friendly input validation (#190)', () => { }); }); + it('createComment: accepts an optional suggestedText alongside a selection', async () => { + const tools = await buildTools(); + const result = await inputSchemaOf(tools.createComment).validate({ + pageId: '019efe44-0000-0000-0000-000000000000', + content: 'A remark', + selection: 'титановый проводник', + suggestedText: 'медный проводник', + }); + expect(result.success).toBe(true); + expect(result.value).toMatchObject({ + suggestedText: 'медный проводник', + }); + }); + it('sharedTool-built tools (getOutline) also get the friendly message on a dropped pageId', async () => { const tools = await buildTools(); const result = await inputSchemaOf(tools.getOutline).validate({}); diff --git a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts index 91ed2efd..8444aebc 100644 --- a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts +++ b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts @@ -450,8 +450,10 @@ export class AiChatToolsService { "new top-level comment REQUIRES a `selection`. Replies inherit the " + "parent's anchor and take no selection. If the call fails with a " + '"selection not found" error, retry with a corrected EXACT selection ' + - 'copied verbatim from a single paragraph/block. Reversible via the ' + - 'comment UI.', + 'copied verbatim from a single paragraph/block. You may also attach a ' + + '`suggestedText` proposing a replacement for the `selection` (a human ' + + 'applies it from the UI); when set, the `selection` must occur exactly ' + + 'once in the page. Reversible via the comment UI.', inputSchema: modelFriendlyInput({ pageId: z.string().describe('The id of the page to comment on.'), content: z.string().describe('The comment body as Markdown.'), @@ -473,24 +475,57 @@ export class AiChatToolsService { 'Optional id of a TOP-LEVEL comment to reply to (one level ' + 'of replies only).', ), + suggestedText: z + .string() + .min(1) + .max(2000) + .optional() + .describe( + 'Optional proposed replacement (PLAIN TEXT) for the `selection`, ' + + 'applied by a human via the UI (never auto-applied). REQUIRES a ' + + '`selection`; NOT allowed on a reply. When set, the `selection` ' + + 'must be UNIQUE in the page — expand it with surrounding context ' + + '(still <=250 chars) if it occurs more than once, or the call is ' + + 'refused.', + ), }), - execute: async ({ pageId, content, selection, parentCommentId }) => { - // createComment(pageId, content, type, selection?, parentCommentId?). - // Top-level comments are inline and must carry a selection to anchor - // on; replies inherit the parent's anchor (no selection). Throwing - // here surfaces a tool error to the model (Vercel `ai` SDK) so the - // agent retries with a better selection — do not catch/suppress it. + execute: async ({ + pageId, + content, + selection, + parentCommentId, + suggestedText, + }) => { + // createComment(pageId, content, type, selection?, parentCommentId?, + // suggestedText?). Top-level comments are inline and must carry a + // selection to anchor on; replies inherit the parent's anchor (no + // selection). Throwing here surfaces a tool error to the model (Vercel + // `ai` SDK) so the agent retries with a better selection — do not + // catch/suppress it. if (!parentCommentId && (!selection || !selection.trim())) { throw new Error( "createComment requires a 'selection' (exact text to anchor on) for a new top-level comment.", ); } + if (suggestedText !== undefined) { + if (parentCommentId) { + throw new Error( + "createComment: 'suggestedText' cannot be attached to a reply; it applies only to a top-level inline comment.", + ); + } + if (!selection || !selection.trim()) { + throw new Error( + "createComment: 'suggestedText' requires a 'selection' to anchor and rewrite.", + ); + } + } const result = await client.createComment( pageId, content, 'inline', selection, parentCommentId, + suggestedText, ); const data = (result?.data ?? {}) as { id?: string }; return { commentId: data.id, pageId }; diff --git a/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts b/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts index 63625fc3..7b5a9a4e 100644 --- a/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts +++ b/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts @@ -177,6 +177,7 @@ export interface DocmostClientLike { type?: 'page' | 'inline', selection?: string, parentCommentId?: string, + suggestedText?: string, ): Promise<{ data: Record; success: boolean }>; resolveComment( commentId: string, diff --git a/packages/mcp/build/client.js b/packages/mcp/build/client.js index fd144690..d1cdc528 100644 --- a/packages/mcp/build/client.js +++ b/packages/mcp/build/client.js @@ -17,7 +17,7 @@ import { withPageLock } from "./lib/page-lock.js"; import { applyTextEdits, } from "./lib/json-edit.js"; import { getCollabToken, performLogin } from "./lib/auth-utils.js"; import { diffDocs, summarizeChange } from "./lib/diff.js"; -import { applyAnchorInDoc, canAnchorInDoc } from "./lib/comment-anchor.js"; +import { applyAnchorInDoc, canAnchorInDoc, countAnchorMatches, } from "./lib/comment-anchor.js"; import { blockText, walk, getList, insertMarkerAfter, setCalloutRange, noteItem, mdToInlineNodes, commentsToFootnotes, canonicalizeFootnotes, insertInlineFootnote, } from "./lib/transforms.js"; import vm from "node:vm"; // Supported image types, kept as two lookup tables so both a local file @@ -1912,9 +1912,21 @@ export class DocmostClient { * an orphan, unanchored comment behind. Replies (parentCommentId set) inherit * their parent's anchor: they take NO selection and are not anchored. */ - async createComment(pageId, content, type = "page", selection, parentCommentId) { + async createComment(pageId, content, type = "page", selection, parentCommentId, suggestedText) { await this.ensureAuthenticated(); const isReply = !!parentCommentId; + const hasSuggestion = suggestedText !== undefined && suggestedText !== null; + // Defense in depth mirroring the server DTO/service: a suggested edit rewrites + // the exact anchored text, so it is only meaningful on a top-level inline + // comment that carries a selection. + if (hasSuggestion) { + if (isReply) { + throw new Error("create_comment: a suggested edit (suggestedText) cannot be attached to a reply; it applies only to a top-level inline comment."); + } + if (!selection || !selection.trim()) { + throw new Error("create_comment: a suggested edit (suggestedText) requires a 'selection' to anchor and rewrite."); + } + } // Only top-level comments are inline-anchored, so they are stored as // "inline". Replies carry no inline selection, so they keep the historical // general ("page") type — both backward-compatible and semantically correct. @@ -1931,16 +1943,33 @@ export class DocmostClient { if (!isReply && selection) { try { const page = await this.getPageJson(pageId); - if (!canAnchorInDoc(page.content, selection)) { + if (hasSuggestion) { + // A suggestion's anchor MUST be unambiguous: applying it rewrites the + // exact anchored text, and ordinary anchoring silently takes the first + // occurrence, so 0 matches -> not found and >=2 -> ambiguous, both + // rejected BEFORE creating the comment. + const matches = countAnchorMatches(page.content, selection); + if (matches === 0) { + throw new Error("create_comment: could not find the selection text in the page to anchor the comment. " + + "Provide the EXACT contiguous text from a single paragraph/block (<=250 chars)."); + } + if (matches >= 2) { + throw new Error(`create_comment: the suggestion's selection is ambiguous — it occurs ${matches} times in the page. ` + + "A suggested edit must anchor to a UNIQUE location; expand the selection with surrounding context " + + "(still <=250 chars) so it appears exactly once."); + } + } + else if (!canAnchorInDoc(page.content, selection)) { throw new Error("create_comment: could not find the selection text in the page to anchor the comment. " + "Provide the EXACT contiguous text from a single paragraph/block (<=250 chars)."); } } catch (e) { - // Rethrow our own "not found" error; swallow read/network errors so the - // live anchor step can still try (and enforce) the anchoring. + // Rethrow our own "not found"/"ambiguous" errors; swallow read/network + // errors so the live anchor step can still try (and enforce) anchoring. if (e instanceof Error && - e.message.startsWith("create_comment: could not find the selection")) { + (e.message.startsWith("create_comment: could not find the selection") || + e.message.startsWith("create_comment: the suggestion's selection is ambiguous"))) { throw e; } if (process.env.DEBUG) { @@ -1962,6 +1991,10 @@ export class DocmostClient { payload.selection = selection; if (parentCommentId) payload.parentCommentId = parentCommentId; + // Only a top-level inline comment (with a selection) may carry a suggestion. + if (!isReply && selection && hasSuggestion) { + payload.suggestedText = suggestedText; + } const response = await this.client.post("/comments/create", payload); const comment = response.data.data || response.data; const markdown = comment.content @@ -1988,15 +2021,34 @@ export class DocmostClient { throw new Error("create_comment: the server returned no comment id, so the comment could not be anchored"); } let anchored = false; + // Set inside the transform when a suggestion's live anchor is ambiguous + // (>=2 occurrences), so the rollback path can surface the right error. + let ambiguousInLiveDoc = false; try { const collabToken = await this.getCollabTokenWithReauth(); // Open the collab doc by the canonical UUID, never the slugId (#260). The // /comments/create REST call above keeps the agent-supplied id. const pageUuid = await this.resolvePageId(pageId); - const mutation = await mutatePageContent(pageUuid, collabToken, this.apiUrl, (liveDoc) => { + // Route through the mutatePage seam (not the free function) so this + // wrapper's uniqueness gate + rollback can be unit-tested without a live + // Hocuspocus collab socket. + const mutation = await this.mutatePage(pageUuid, collabToken, this.apiUrl, (liveDoc) => { const doc = liveDoc && liveDoc.type === "doc" ? liveDoc : { type: "doc", content: [] }; + if (hasSuggestion) { + // Authoritative uniqueness check against the LIVE document: a + // suggestion must anchor to EXACTLY ONE occurrence, otherwise + // "Apply" would rewrite the wrong/ambiguous text. If the live doc + // no longer has exactly one occurrence (it changed since the + // pre-check), abort so the just-created comment is rolled back + // rather than mis-anchored to the first occurrence. + const liveCount = countAnchorMatches(doc, selection); + if (liveCount !== 1) { + ambiguousInLiveDoc = liveCount >= 2; + return null; + } + } if (applyAnchorInDoc(doc, selection, newCommentId)) { anchored = true; return doc; @@ -2014,10 +2066,13 @@ export class DocmostClient { throw e; } if (!anchored) { - // Mutation aborted because the selection was not found in the live - // document. Roll back the comment and surface a hard error. + // Mutation aborted because the selection was not found (or, for a + // suggestion, was ambiguous) in the live document. Roll back the comment + // and surface a hard error. await this.safeDeleteComment(newCommentId); - throw new Error("create_comment: failed to anchor the comment (selection not found in the live document); the comment was rolled back"); + throw new Error(ambiguousInLiveDoc + ? "create_comment: the suggestion's selection is ambiguous in the live document (multiple occurrences); the comment was rolled back. Expand the selection with surrounding context so it is unique." + : "create_comment: failed to anchor the comment (selection not found in the live document); the comment was rolled back"); } result.anchored = true; return result; diff --git a/packages/mcp/build/index.js b/packages/mcp/build/index.js index 1ba52bba..360745e5 100644 --- a/packages/mcp/build/index.js +++ b/packages/mcp/build/index.js @@ -520,7 +520,10 @@ export function createDocmostMcpServer(config) { "A top-level comment REQUIRES an exact `selection`; if the selection " + "cannot be found in the page the call fails (no orphan comment is left). " + "Replies (parentCommentId set) inherit the parent's anchor and take no " + - "selection.", + "selection. You may also attach a `suggestedText` proposing a replacement " + + "for the `selection`; a human applies (or rejects) it from the UI. When " + + "`suggestedText` is set the `selection` MUST occur exactly once in the " + + "page — expand it with surrounding context if it is ambiguous.", inputSchema: { pageId: z.string().describe("ID of the page to comment on"), content: z.string().min(1).describe("Comment content in Markdown format"), @@ -537,12 +540,30 @@ export function createDocmostMcpServer(config) { .string() .optional() .describe("Parent comment ID to create a reply (max 2 nesting levels)"), + suggestedText: z + .string() + .min(1) + .max(2000) + .optional() + .describe("Optional proposed replacement (PLAIN TEXT) for the `selection`, " + + "applied by a human via the UI (never auto-applied). REQUIRES a " + + "`selection`; NOT allowed on a reply. When set, the `selection` must " + + "be UNIQUE in the page — expand it with surrounding context (still " + + "<=250 chars) if it occurs more than once, or the call is refused."), }, - }, async ({ pageId, content, selection, parentCommentId }) => { + }, async ({ pageId, content, selection, parentCommentId, suggestedText }) => { if (!parentCommentId && (!selection || !selection.trim())) { throw new Error("create_comment: a 'selection' (exact text to anchor on) is required for a top-level comment; omit it only when replying via parentCommentId."); } - const result = await docmostClient.createComment(pageId, content, "inline", selection, parentCommentId); + if (suggestedText !== undefined) { + if (parentCommentId) { + throw new Error("create_comment: 'suggestedText' cannot be attached to a reply; it applies only to a top-level inline comment."); + } + if (!selection || !selection.trim()) { + throw new Error("create_comment: 'suggestedText' requires a 'selection' to anchor and rewrite."); + } + } + const result = await docmostClient.createComment(pageId, content, "inline", selection, parentCommentId, suggestedText); return jsonContent(result); }); // Tool: update_comment diff --git a/packages/mcp/build/lib/comment-anchor.js b/packages/mcp/build/lib/comment-anchor.js index 50e113b2..03a50931 100644 --- a/packages/mcp/build/lib/comment-anchor.js +++ b/packages/mcp/build/lib/comment-anchor.js @@ -210,6 +210,77 @@ function spliceCommentMark(blockContent, match, commentId) { } blockContent.splice(startChild, endChild - startChild + 1, ...fragments); } +/** + * Count how many times `selection` occurs across the whole document, using the + * same normalization and run-matching as findAnchorInBlock but WITHOUT stopping + * at the first hit: every non-overlapping occurrence within each block's text + * runs is counted and summed across all blocks (depth-first, the same traversal + * as canAnchorInDoc). + * + * This is the uniqueness gate for SUGGESTIONS: because applying a suggestion + * rewrites the exact anchored text, an ambiguous anchor (>1 occurrence) would + * silently edit the wrong place, so a suggestion is only allowed when this + * returns exactly 1. Ordinary comments keep first-occurrence anchoring and do + * not use this. (Note: counts OCCURRENCES, not just matching blocks, so two + * occurrences inside one block are correctly reported as 2.) + */ +export function countAnchorMatches(doc, selection) { + const normSel = normalizeForMatch(selection).norm.trim(); + if (normSel.length === 0) + return 0; + // Count non-overlapping occurrences of the normalized selection within a + // single block's direct content, matching findAnchorInBlock's run building. + const countInBlock = (blockContent) => { + if (!Array.isArray(blockContent)) + return 0; + let count = 0; + let i = 0; + while (i < blockContent.length) { + const node = blockContent[i]; + if (!node || typeof node !== "object" || node.type !== "text") { + i++; + continue; + } + // Accumulate a maximal run of consecutive text nodes. + let rawRun = ""; + let j = i; + while (j < blockContent.length) { + const n = blockContent[j]; + if (!n || typeof n !== "object" || n.type !== "text") + break; + rawRun += typeof n.text === "string" ? n.text : ""; + j++; + } + const norm = normalizeForMatch(rawRun).norm; + // Count every non-overlapping occurrence in this run. + let from = 0; + for (;;) { + const idx = norm.indexOf(normSel, from); + if (idx === -1) + break; + count++; + from = idx + normSel.length; + } + i = j > i ? j : i + 1; + } + return count; + }; + let total = 0; + const visit = (node, depth) => { + if (depth > MAX_DEPTH || !node || typeof node !== "object") + return; + if (!Array.isArray(node.content)) + return; + total += countInBlock(node.content); + for (const child of node.content) { + if (child && typeof child === "object" && Array.isArray(child.content)) { + visit(child, depth + 1); + } + } + }; + visit(doc, 0); + return total; +} /** * Depth-first (same order as canAnchorInDoc) over `doc`; on the FIRST block * whose content matches `selection`, splice the comment mark across the matched diff --git a/packages/mcp/build/lib/filters.js b/packages/mcp/build/lib/filters.js index 63a6a55e..eb056968 100644 --- a/packages/mcp/build/lib/filters.js +++ b/packages/mcp/build/lib/filters.js @@ -70,6 +70,11 @@ export function filterComment(comment, markdownContent) { editedAt: comment.editedAt || null, resolvedAt: comment.resolvedAt || null, resolvedById: comment.resolvedById || null, + // Suggestion state: the proposed replacement text (if any) and, once a human + // applies it via the UI, when and by whom. + suggestedText: comment.suggestedText || null, + suggestionAppliedAt: comment.suggestionAppliedAt || null, + suggestionAppliedById: comment.suggestionAppliedById || null, }; } export function filterSearchResult(result) { diff --git a/packages/mcp/src/client.ts b/packages/mcp/src/client.ts index 14fd45ae..9e9eb421 100644 --- a/packages/mcp/src/client.ts +++ b/packages/mcp/src/client.ts @@ -56,7 +56,11 @@ import { } from "./lib/json-edit.js"; import { getCollabToken, performLogin } from "./lib/auth-utils.js"; import { diffDocs, summarizeChange } from "./lib/diff.js"; -import { applyAnchorInDoc, canAnchorInDoc } from "./lib/comment-anchor.js"; +import { + applyAnchorInDoc, + canAnchorInDoc, + countAnchorMatches, +} from "./lib/comment-anchor.js"; import { blockText, walk, @@ -2395,10 +2399,28 @@ export class DocmostClient { type: "page" | "inline" = "page", selection?: string, parentCommentId?: string, + suggestedText?: string, ) { await this.ensureAuthenticated(); const isReply = !!parentCommentId; + const hasSuggestion = + suggestedText !== undefined && suggestedText !== null; + // Defense in depth mirroring the server DTO/service: a suggested edit rewrites + // the exact anchored text, so it is only meaningful on a top-level inline + // comment that carries a selection. + if (hasSuggestion) { + if (isReply) { + throw new Error( + "create_comment: a suggested edit (suggestedText) cannot be attached to a reply; it applies only to a top-level inline comment.", + ); + } + if (!selection || !selection.trim()) { + throw new Error( + "create_comment: a suggested edit (suggestedText) requires a 'selection' to anchor and rewrite.", + ); + } + } // Only top-level comments are inline-anchored, so they are stored as // "inline". Replies carry no inline selection, so they keep the historical // general ("page") type — both backward-compatible and semantically correct. @@ -2418,18 +2440,40 @@ export class DocmostClient { if (!isReply && selection) { try { const page = await this.getPageJson(pageId); - if (!canAnchorInDoc(page.content, selection)) { + if (hasSuggestion) { + // A suggestion's anchor MUST be unambiguous: applying it rewrites the + // exact anchored text, and ordinary anchoring silently takes the first + // occurrence, so 0 matches -> not found and >=2 -> ambiguous, both + // rejected BEFORE creating the comment. + const matches = countAnchorMatches(page.content, selection); + if (matches === 0) { + throw new Error( + "create_comment: could not find the selection text in the page to anchor the comment. " + + "Provide the EXACT contiguous text from a single paragraph/block (<=250 chars).", + ); + } + if (matches >= 2) { + throw new Error( + `create_comment: the suggestion's selection is ambiguous — it occurs ${matches} times in the page. ` + + "A suggested edit must anchor to a UNIQUE location; expand the selection with surrounding context " + + "(still <=250 chars) so it appears exactly once.", + ); + } + } else if (!canAnchorInDoc(page.content, selection)) { throw new Error( "create_comment: could not find the selection text in the page to anchor the comment. " + "Provide the EXACT contiguous text from a single paragraph/block (<=250 chars).", ); } } catch (e) { - // Rethrow our own "not found" error; swallow read/network errors so the - // live anchor step can still try (and enforce) the anchoring. + // Rethrow our own "not found"/"ambiguous" errors; swallow read/network + // errors so the live anchor step can still try (and enforce) anchoring. if ( e instanceof Error && - e.message.startsWith("create_comment: could not find the selection") + (e.message.startsWith("create_comment: could not find the selection") || + e.message.startsWith( + "create_comment: the suggestion's selection is ambiguous", + )) ) { throw e; } @@ -2454,6 +2498,10 @@ export class DocmostClient { }; if (!isReply && selection) payload.selection = selection; if (parentCommentId) payload.parentCommentId = parentCommentId; + // Only a top-level inline comment (with a selection) may carry a suggestion. + if (!isReply && selection && hasSuggestion) { + payload.suggestedText = suggestedText; + } const response = await this.client.post("/comments/create", payload); const comment = response.data.data || response.data; @@ -2485,12 +2533,18 @@ export class DocmostClient { ); } let anchored = false; + // Set inside the transform when a suggestion's live anchor is ambiguous + // (>=2 occurrences), so the rollback path can surface the right error. + let ambiguousInLiveDoc = false; try { const collabToken = await this.getCollabTokenWithReauth(); // Open the collab doc by the canonical UUID, never the slugId (#260). The // /comments/create REST call above keeps the agent-supplied id. const pageUuid = await this.resolvePageId(pageId); - const mutation = await mutatePageContent( + // Route through the mutatePage seam (not the free function) so this + // wrapper's uniqueness gate + rollback can be unit-tested without a live + // Hocuspocus collab socket. + const mutation = await this.mutatePage( pageUuid, collabToken, this.apiUrl, @@ -2499,6 +2553,19 @@ export class DocmostClient { liveDoc && liveDoc.type === "doc" ? liveDoc : { type: "doc", content: [] }; + if (hasSuggestion) { + // Authoritative uniqueness check against the LIVE document: a + // suggestion must anchor to EXACTLY ONE occurrence, otherwise + // "Apply" would rewrite the wrong/ambiguous text. If the live doc + // no longer has exactly one occurrence (it changed since the + // pre-check), abort so the just-created comment is rolled back + // rather than mis-anchored to the first occurrence. + const liveCount = countAnchorMatches(doc, selection as string); + if (liveCount !== 1) { + ambiguousInLiveDoc = liveCount >= 2; + return null; + } + } if (applyAnchorInDoc(doc, selection as string, newCommentId)) { anchored = true; return doc; @@ -2517,11 +2584,14 @@ export class DocmostClient { } if (!anchored) { - // Mutation aborted because the selection was not found in the live - // document. Roll back the comment and surface a hard error. + // Mutation aborted because the selection was not found (or, for a + // suggestion, was ambiguous) in the live document. Roll back the comment + // and surface a hard error. await this.safeDeleteComment(newCommentId); throw new Error( - "create_comment: failed to anchor the comment (selection not found in the live document); the comment was rolled back", + ambiguousInLiveDoc + ? "create_comment: the suggestion's selection is ambiguous in the live document (multiple occurrences); the comment was rolled back. Expand the selection with surrounding context so it is unique." + : "create_comment: failed to anchor the comment (selection not found in the live document); the comment was rolled back", ); } diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts index 456e851b..4d0a9375 100644 --- a/packages/mcp/src/index.ts +++ b/packages/mcp/src/index.ts @@ -722,7 +722,10 @@ server.registerTool( "A top-level comment REQUIRES an exact `selection`; if the selection " + "cannot be found in the page the call fails (no orphan comment is left). " + "Replies (parentCommentId set) inherit the parent's anchor and take no " + - "selection.", + "selection. You may also attach a `suggestedText` proposing a replacement " + + "for the `selection`; a human applies (or rejects) it from the UI. When " + + "`suggestedText` is set the `selection` MUST occur exactly once in the " + + "page — expand it with surrounding context if it is ambiguous.", inputSchema: { pageId: z.string().describe("ID of the page to comment on"), content: z.string().min(1).describe("Comment content in Markdown format"), @@ -741,20 +744,45 @@ server.registerTool( .string() .optional() .describe("Parent comment ID to create a reply (max 2 nesting levels)"), + suggestedText: z + .string() + .min(1) + .max(2000) + .optional() + .describe( + "Optional proposed replacement (PLAIN TEXT) for the `selection`, " + + "applied by a human via the UI (never auto-applied). REQUIRES a " + + "`selection`; NOT allowed on a reply. When set, the `selection` must " + + "be UNIQUE in the page — expand it with surrounding context (still " + + "<=250 chars) if it occurs more than once, or the call is refused.", + ), }, }, - async ({ pageId, content, selection, parentCommentId }) => { + async ({ pageId, content, selection, parentCommentId, suggestedText }) => { if (!parentCommentId && (!selection || !selection.trim())) { throw new Error( "create_comment: a 'selection' (exact text to anchor on) is required for a top-level comment; omit it only when replying via parentCommentId.", ); } + if (suggestedText !== undefined) { + if (parentCommentId) { + throw new Error( + "create_comment: 'suggestedText' cannot be attached to a reply; it applies only to a top-level inline comment.", + ); + } + if (!selection || !selection.trim()) { + throw new Error( + "create_comment: 'suggestedText' requires a 'selection' to anchor and rewrite.", + ); + } + } const result = await docmostClient.createComment( pageId, content, "inline", selection, parentCommentId, + suggestedText, ); return jsonContent(result); }, diff --git a/packages/mcp/src/lib/comment-anchor.ts b/packages/mcp/src/lib/comment-anchor.ts index 79dbb469..7deb6620 100644 --- a/packages/mcp/src/lib/comment-anchor.ts +++ b/packages/mcp/src/lib/comment-anchor.ts @@ -242,6 +242,74 @@ function spliceCommentMark( blockContent.splice(startChild, endChild - startChild + 1, ...fragments); } +/** + * Count how many times `selection` occurs across the whole document, using the + * same normalization and run-matching as findAnchorInBlock but WITHOUT stopping + * at the first hit: every non-overlapping occurrence within each block's text + * runs is counted and summed across all blocks (depth-first, the same traversal + * as canAnchorInDoc). + * + * This is the uniqueness gate for SUGGESTIONS: because applying a suggestion + * rewrites the exact anchored text, an ambiguous anchor (>1 occurrence) would + * silently edit the wrong place, so a suggestion is only allowed when this + * returns exactly 1. Ordinary comments keep first-occurrence anchoring and do + * not use this. (Note: counts OCCURRENCES, not just matching blocks, so two + * occurrences inside one block are correctly reported as 2.) + */ +export function countAnchorMatches(doc: any, selection: string): number { + const normSel = normalizeForMatch(selection).norm.trim(); + if (normSel.length === 0) return 0; + + // Count non-overlapping occurrences of the normalized selection within a + // single block's direct content, matching findAnchorInBlock's run building. + const countInBlock = (blockContent: any[]): number => { + if (!Array.isArray(blockContent)) return 0; + let count = 0; + let i = 0; + while (i < blockContent.length) { + const node = blockContent[i]; + if (!node || typeof node !== "object" || node.type !== "text") { + i++; + continue; + } + // Accumulate a maximal run of consecutive text nodes. + let rawRun = ""; + let j = i; + while (j < blockContent.length) { + const n = blockContent[j]; + if (!n || typeof n !== "object" || n.type !== "text") break; + rawRun += typeof n.text === "string" ? n.text : ""; + j++; + } + const norm = normalizeForMatch(rawRun).norm; + // Count every non-overlapping occurrence in this run. + let from = 0; + for (;;) { + const idx = norm.indexOf(normSel, from); + if (idx === -1) break; + count++; + from = idx + normSel.length; + } + i = j > i ? j : i + 1; + } + return count; + }; + + let total = 0; + const visit = (node: any, depth: number): void => { + if (depth > MAX_DEPTH || !node || typeof node !== "object") return; + if (!Array.isArray(node.content)) return; + total += countInBlock(node.content); + for (const child of node.content) { + if (child && typeof child === "object" && Array.isArray(child.content)) { + visit(child, depth + 1); + } + } + }; + visit(doc, 0); + return total; +} + /** * Depth-first (same order as canAnchorInDoc) over `doc`; on the FIRST block * whose content matches `selection`, splice the comment mark across the matched diff --git a/packages/mcp/src/lib/filters.ts b/packages/mcp/src/lib/filters.ts index f1104d50..c789ec3a 100644 --- a/packages/mcp/src/lib/filters.ts +++ b/packages/mcp/src/lib/filters.ts @@ -75,6 +75,11 @@ export function filterComment(comment: any, markdownContent?: string) { editedAt: comment.editedAt || null, resolvedAt: comment.resolvedAt || null, resolvedById: comment.resolvedById || null, + // Suggestion state: the proposed replacement text (if any) and, once a human + // applies it via the UI, when and by whom. + suggestedText: comment.suggestedText || null, + suggestionAppliedAt: comment.suggestionAppliedAt || null, + suggestionAppliedById: comment.suggestionAppliedById || null, }; } diff --git a/packages/mcp/test/mock/create-comment.test.mjs b/packages/mcp/test/mock/create-comment.test.mjs index c0d6859e..d854bdb0 100644 --- a/packages/mcp/test/mock/create-comment.test.mjs +++ b/packages/mcp/test/mock/create-comment.test.mjs @@ -229,3 +229,226 @@ test("a reply creates without selection or anchoring and is stored as type 'page "a reply must skip the pre-check/anchoring (no /pages/info read)", ); }); + +// ----------------------------------------------------------------------------- +// 4) suggestedText + a DUPLICATE selection is refused BEFORE creating anything: +// a suggestion must anchor to a unique location, so >=2 occurrences throws the +// ambiguity error (the /pages/info pre-check short-circuits before create). +// ----------------------------------------------------------------------------- +test("suggestedText with an ambiguous selection is refused before creating", async () => { + let createCalls = 0; + let infoCalls = 0; + + const { baseURL } = await spawn(async (req, res) => { + await readBody(req); + if (req.url === "/api/auth/login") { + sendJson(res, 200, { success: true }, { + "Set-Cookie": "authToken=t; Path=/; HttpOnly", + }); + return; + } + if (req.url === "/api/pages/info") { + infoCalls++; + // "target" appears in two blocks -> ambiguous for a suggestion. + sendJson(res, 200, { + data: { + id: "page-1", + content: { + type: "doc", + content: [ + { type: "paragraph", content: [{ type: "text", text: "first target here" }] }, + { type: "paragraph", content: [{ type: "text", text: "second target here" }] }, + ], + }, + }, + }); + return; + } + if (req.url === "/api/comments/create") { + createCalls++; + sendJson(res, 200, { data: { id: "should-not-happen" } }); + return; + } + sendJson(res, 404, { message: "not found" }); + }); + + const client = new DocmostClient(baseURL, "user@example.com", "pw"); + + await assert.rejects( + () => + client.createComment( + "page-1", + "body", + "inline", + "target", + undefined, + "TARGET", + ), + /ambiguous/i, + "an ambiguous suggestion selection must reject with the ambiguity error", + ); + assert.ok(infoCalls >= 1, "the pre-check must read the page via /pages/info"); + assert.equal( + createCalls, + 0, + "/comments/create must NEVER be called for an ambiguous suggestion", + ); +}); + +// ----------------------------------------------------------------------------- +// 5) suggestedText on a reply is refused immediately (before any HTTP). +// ----------------------------------------------------------------------------- +test("suggestedText on a reply is rejected", async () => { + let anyCall = 0; + const { baseURL } = await spawn(async (req, res) => { + await readBody(req); + if (req.url === "/api/auth/login") { + sendJson(res, 200, { success: true }, { + "Set-Cookie": "authToken=t; Path=/; HttpOnly", + }); + return; + } + anyCall++; + sendJson(res, 200, { data: { id: "x" } }); + }); + + const client = new DocmostClient(baseURL, "user@example.com", "pw"); + + await assert.rejects( + () => + client.createComment( + "page-1", + "body", + "inline", + undefined, + "parent-1", + "replacement", + ), + /reply/i, + "suggestedText on a reply must be rejected", + ); + assert.equal(anyCall, 0, "no create/info call for a rejected reply suggestion"); +}); + +// ----------------------------------------------------------------------------- +// 6) suggestedText without a selection is refused immediately. +// ----------------------------------------------------------------------------- +test("suggestedText without a selection is rejected", async () => { + const { baseURL } = await spawn(async (req, res) => { + await readBody(req); + if (req.url === "/api/auth/login") { + sendJson(res, 200, { success: true }, { + "Set-Cookie": "authToken=t; Path=/; HttpOnly", + }); + return; + } + sendJson(res, 200, { data: { id: "x" } }); + }); + + const client = new DocmostClient(baseURL, "user@example.com", "pw"); + + await assert.rejects( + () => + client.createComment( + "page-1", + "body", + "inline", + undefined, + undefined, + "replacement", + ), + /selection/i, + "suggestedText without a selection must be rejected", + ); +}); + +// ----------------------------------------------------------------------------- +// 7) suggestedText + a UNIQUE selection succeeds: the pre-check passes, the +// create payload carries suggestedText, and the live anchoring step (stubbed +// via the mutatePage seam) writes the comment mark exactly once. +// ----------------------------------------------------------------------------- +test("suggestedText with a unique selection succeeds and forwards the payload", async () => { + let createPayload = null; + + const { baseURL } = await spawn(async (req, res) => { + const raw = await readBody(req); + if (req.url === "/api/auth/login") { + sendJson(res, 200, { success: true }, { + "Set-Cookie": "authToken=t; Path=/; HttpOnly", + }); + return; + } + if (req.url === "/api/pages/info") { + // "brave" is unique in the page. + sendJson(res, 200, { + data: { + id: "11111111-1111-1111-1111-111111111111", + content: { + type: "doc", + content: [ + { type: "paragraph", content: [{ type: "text", text: "Hello brave world" }] }, + ], + }, + }, + }); + return; + } + if (req.url === "/api/comments/create") { + createPayload = JSON.parse(raw); + sendJson(res, 200, { + data: { + id: "cmt-ok-1", + content: createPayload.content, + selection: createPayload.selection, + suggestedText: createPayload.suggestedText, + type: createPayload.type, + }, + }); + return; + } + sendJson(res, 404, { message: "not found" }); + }); + + // Subclass to stub the collab write seam: no live Hocuspocus socket, but the + // wrapper's uniqueness gate + applyAnchorInDoc still run against `doc`. + class TestClient extends DocmostClient { + async getCollabTokenWithReauth() { + return "collab-token"; + } + async resolvePageId(pageId) { + return "11111111-1111-1111-1111-111111111111"; + } + async mutatePage(pageId, collabToken, apiUrl, transform) { + const doc = { + type: "doc", + content: [ + { type: "paragraph", content: [{ type: "text", text: "Hello brave world" }] }, + ], + }; + const out = transform(doc); + return { doc: out, verify: { ok: true } }; + } + } + + const client = new TestClient(baseURL, "user@example.com", "pw"); + + const result = await client.createComment( + "11111111-1111-1111-1111-111111111111", + "please rename", + "inline", + "brave", + undefined, + "bold", + ); + + assert.equal(result.success, true, "a unique suggestion must resolve"); + assert.equal(result.anchored, true, "the comment must be anchored"); + assert.ok(createPayload, "/comments/create must have been called"); + assert.equal( + createPayload.suggestedText, + "bold", + "the create payload must carry suggestedText for a top-level inline comment", + ); + assert.equal(createPayload.selection, "brave"); + assert.equal(result.data.suggestedText, "bold", "filterComment surfaces suggestedText"); +}); diff --git a/packages/mcp/test/unit/comment-anchor.test.mjs b/packages/mcp/test/unit/comment-anchor.test.mjs index 490cba96..c35d853b 100644 --- a/packages/mcp/test/unit/comment-anchor.test.mjs +++ b/packages/mcp/test/unit/comment-anchor.test.mjs @@ -6,6 +6,7 @@ import { findAnchorInBlock, canAnchorInDoc, applyAnchorInDoc, + countAnchorMatches, } from "../../build/lib/comment-anchor.js"; const COMMENT_ID = "cmt-123"; @@ -208,3 +209,68 @@ test("anchoring works inside a nested block (e.g. list item) via DFS recursion", assert.equal(marked.length, 1); assert.equal(marked[0].text, "target"); }); + +// --------------------------------------------------------------------------- +// countAnchorMatches — the uniqueness gate for suggestions. Counts every +// non-overlapping occurrence across the whole document (0 / 1 / N). +// --------------------------------------------------------------------------- +test("countAnchorMatches returns 0 when the selection is absent", () => { + const doc = paragraphDoc([{ type: "text", text: "hello world" }]); + assert.equal(countAnchorMatches(doc, "missing"), 0); +}); + +test("countAnchorMatches returns 1 for a unique selection", () => { + const doc = paragraphDoc([{ type: "text", text: "Hello brave world" }]); + assert.equal(countAnchorMatches(doc, "brave"), 1); +}); + +test("countAnchorMatches counts multiple occurrences within one block", () => { + const doc = paragraphDoc([{ type: "text", text: "ab ab ab" }]); + assert.equal(countAnchorMatches(doc, "ab"), 3); +}); + +test("countAnchorMatches sums occurrences across separate blocks", () => { + const doc = { + type: "doc", + content: [ + { type: "paragraph", content: [{ type: "text", text: "first target here" }] }, + { type: "paragraph", content: [{ type: "text", text: "second target here" }] }, + ], + }; + assert.equal(countAnchorMatches(doc, "target"), 2); +}); + +test("countAnchorMatches counts a match spanning adjacent text nodes as one", () => { + const doc = paragraphDoc([ + { type: "text", text: "запуска ", marks: [{ type: "italic" }] }, + { type: "text", text: "перед блоком", marks: [{ type: "italic" }] }, + ]); + assert.equal(countAnchorMatches(doc, "запуска перед"), 1); +}); + +test("countAnchorMatches counts matches inside nested (recursed) blocks", () => { + const doc = { + type: "doc", + content: [ + { type: "paragraph", content: [{ type: "text", text: "outer target" }] }, + { + type: "bulletList", + content: [ + { + type: "listItem", + content: [ + { type: "paragraph", content: [{ type: "text", text: "nested target" }] }, + ], + }, + ], + }, + ], + }; + assert.equal(countAnchorMatches(doc, "target"), 2); +}); + +test("countAnchorMatches applies the same normalization as anchoring", () => { + // Smart quotes in the doc match ASCII quotes in the selection. + const doc = paragraphDoc([{ type: "text", text: "say “hi” now" }]); + assert.equal(countAnchorMatches(doc, '"hi"'), 1); +}); From 48c1ec46f71446b4957aedf8e69de06f22b80df7 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Fri, 3 Jul 2026 20:29:42 +0300 Subject: [PATCH 7/7] fix(comment): store the real anchored substring as expectedText + pin authz (#318 F1/F2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F1 [blocking]: a suggestion whose anchor matched via normalization could never be applied (spurious 409). The comment mark lands on the doc's ACTUAL text (Docmost auto-converts to typographic quotes/dashes/nbsp), but the stored selection — used as expectedText at apply — was the raw ASCII agent input (+substring(0,250)). So replaceYjsMarkedText's strict joined!==expectedText always failed and threw "text changed" though nobody edited. Fix: new pure getAnchoredText(doc, selection) reconstructs the exact raw doc substring the mark covers (slicing identical to spliceCommentMark); on the suggestion path client.createComment stores THAT as selection, so expectedText equals the marked text and apply returns applied:true. Live anchoring still uses the raw agent selection (normalization still finds the anchor). Truncation raised 250->2000 (+ DTO @MaxLength(2000)) so the anchored substring is never cut below the mark span. Ordinary comments unchanged. AI-chat shares client.createComment, so covered. Regression tests: getAnchoredText raw-vs-ASCII; create payload selection is the typographic substring; apply with typographic expectedText -> applied. F2 [blocking]: added comment.controller.spec.ts pinning that validateCanEdit runs before applySuggestion (Forbidden -> applySuggestion never called; happy path -> called; missing comment -> 404 without authorizing). MCP 448 pass; server comment+yjs 54 pass. MCP build/ rebuilt. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../server/src/collaboration/yjs.util.spec.ts | 26 ++++ .../core/comment/comment.controller.spec.ts | 119 ++++++++++++++++++ .../src/core/comment/comment.service.ts | 10 +- .../core/comment/dto/create-comment.dto.ts | 7 ++ packages/mcp/build/client.js | 24 +++- packages/mcp/build/lib/comment-anchor.js | 61 +++++++++ packages/mcp/src/client.ts | 25 +++- packages/mcp/src/lib/comment-anchor.ts | 59 +++++++++ .../mcp/test/mock/create-comment.test.mjs | 96 ++++++++++++++ .../mcp/test/unit/comment-anchor.test.mjs | 34 +++++ 10 files changed, 457 insertions(+), 4 deletions(-) create mode 100644 apps/server/src/core/comment/comment.controller.spec.ts diff --git a/apps/server/src/collaboration/yjs.util.spec.ts b/apps/server/src/collaboration/yjs.util.spec.ts index eb0eb0f0..8f6fc2e4 100644 --- a/apps/server/src/collaboration/yjs.util.spec.ts +++ b/apps/server/src/collaboration/yjs.util.spec.ts @@ -379,6 +379,32 @@ describe('replaceYjsMarkedText', () => { expect(text.toDelta()).toEqual(before); }); + // F1 regression: the marked doc text is TYPOGRAPHIC (smart quotes / em-dash) + // and expectedText equals that raw typographic text — as it now does, because + // the MCP client stores the RAW anchored substring (getAnchoredText) rather + // than the agent's ASCII input. The strict `joinedText !== expectedText` + // compare must therefore MATCH and the suggestion apply (not a spurious 409). + it('typographic marked text applies when expectedText is the raw typographic text', () => { + const marked = '“hello”—world'; + const { fragment, text } = buildRuns([ + { text: 'say ' }, + { text: marked, comment: { commentId: 'c1', resolved: false } }, + { text: '!' }, + ]); + + const result = replaceYjsMarkedText(fragment, 'c1', marked, 'bye'); + + expect(result).toEqual({ applied: true, currentText: 'bye' }); + expect(text.toDelta()).toEqual([ + { insert: 'say ' }, + { + insert: 'bye', + attributes: { comment: { commentId: 'c1', resolved: false } }, + }, + { insert: '!' }, + ]); + }); + it('anchor deleted: no mark with that commentId → { applied: false, currentText: null }', () => { const { fragment, text } = buildWithComments([ { text: 'abc', comment: { commentId: 'c1', resolved: false } }, diff --git a/apps/server/src/core/comment/comment.controller.spec.ts b/apps/server/src/core/comment/comment.controller.spec.ts new file mode 100644 index 00000000..707e9687 --- /dev/null +++ b/apps/server/src/core/comment/comment.controller.spec.ts @@ -0,0 +1,119 @@ +import { + ForbiddenException, + NotFoundException, +} from '@nestjs/common'; +import { CommentController } from './comment.controller'; + +/** + * Authz-gate tests for the apply-suggestion route. Applying a suggestion + * rewrites the page text, so the route MUST call + * pageAccessService.validateCanEdit BEFORE handing off to + * commentService.applySuggestion (which performs the document mutation + stamp). + * That ordering is a security boundary: an unauthorized user must never reach + * the mutation. These tests pin it against a fully mocked controller so any + * regression that drops the gate (or reorders it after the mutation) fails here. + */ +describe('CommentController apply-suggestion authz', () => { + function makeController() { + const commentService = { + applySuggestion: jest.fn(async () => ({ id: 'c-1', applied: true })), + }; + const commentRepo = { findById: jest.fn() }; + const pageRepo = { findById: jest.fn() }; + const spaceAbility = {} as any; + const pageAccessService = { + validateCanEdit: jest.fn(async () => undefined), + }; + const wsService = {} as any; + const auditService = { log: jest.fn() }; + + const controller = new CommentController( + commentService as any, + commentRepo as any, + pageRepo as any, + spaceAbility, + pageAccessService as any, + wsService, + auditService as any, + ); + return { + controller, + commentService, + commentRepo, + pageRepo, + pageAccessService, + }; + } + + const user: any = { id: 'u-1' }; + const workspace: any = { id: 'ws-1' }; + const provenance: any = undefined; + const dto: any = { commentId: 'c-1' }; + + const comment = { + id: 'c-1', + pageId: 'p-1', + spaceId: 'sp-1', + suggestedText: 'new text', + selection: 'old text', + }; + const page = { id: 'p-1', spaceId: 'sp-1', deletedAt: null }; + + it('validateCanEdit throwing Forbidden rejects AND applySuggestion is never called', async () => { + const { controller, commentRepo, pageRepo, pageAccessService, commentService } = + makeController(); + commentRepo.findById.mockResolvedValue(comment); + pageRepo.findById.mockResolvedValue(page); + pageAccessService.validateCanEdit.mockRejectedValue( + new ForbiddenException('no edit access'), + ); + + await expect( + controller.applySuggestion(dto, user, workspace, provenance), + ).rejects.toBeInstanceOf(ForbiddenException); + + // The security boundary: the mutation/stamp must NOT run for an + // unauthorized user. + expect(pageAccessService.validateCanEdit).toHaveBeenCalledWith(page, user); + expect(commentService.applySuggestion).not.toHaveBeenCalled(); + }); + + it('happy path: validateCanEdit resolves → applySuggestion is called and its result returned', async () => { + const { controller, commentRepo, pageRepo, pageAccessService, commentService } = + makeController(); + commentRepo.findById.mockResolvedValue(comment); + pageRepo.findById.mockResolvedValue(page); + const applied = { id: 'c-1', applied: true }; + commentService.applySuggestion.mockResolvedValue(applied); + + const result = await controller.applySuggestion( + dto, + user, + workspace, + provenance, + ); + + // Authorization ran before the mutation, then the service was invoked. + expect(pageAccessService.validateCanEdit).toHaveBeenCalledWith(page, user); + expect(commentService.applySuggestion).toHaveBeenCalledWith( + comment, + user, + provenance, + ); + expect(result).toBe(applied); + }); + + it('missing comment: NotFound is thrown without authorizing or applying', async () => { + const { controller, commentRepo, pageRepo, pageAccessService, commentService } = + makeController(); + commentRepo.findById.mockResolvedValue(null); + + await expect( + controller.applySuggestion(dto, user, workspace, provenance), + ).rejects.toBeInstanceOf(NotFoundException); + + expect(pageRepo.findById).not.toHaveBeenCalled(); + expect(pageAccessService.validateCanEdit).not.toHaveBeenCalled(); + expect(commentService.applySuggestion).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/server/src/core/comment/comment.service.ts b/apps/server/src/core/comment/comment.service.ts index 3a251b58..cc46a002 100644 --- a/apps/server/src/core/comment/comment.service.ts +++ b/apps/server/src/core/comment/comment.service.ts @@ -87,7 +87,15 @@ export class CommentService { } } - const selection = createCommentDto?.selection?.substring(0, 250) ?? null; + // Do NOT lossily truncate at 250: for a suggestion the client sends the RAW + // anchored document substring (the exact text under the comment mark) as the + // selection, which can be LONGER than the agent's <=250-char typed input + // (normalization collapses whitespace/typographic runs, so the raw span can + // exceed the normalized selection). Truncating it shorter than the mark span + // would break the apply-time equality check and make the suggestion + // un-appliable. Keep a generous 2000-char safety bound (matching + // suggestedText) so a legitimate anchored substring is never cut. + const selection = createCommentDto?.selection?.substring(0, 2000) ?? null; // A suggested edit rewrites the exact text under an inline comment mark, so // it is only meaningful on a top-level inline comment that carries a diff --git a/apps/server/src/core/comment/dto/create-comment.dto.ts b/apps/server/src/core/comment/dto/create-comment.dto.ts index b556c627..dd75494b 100644 --- a/apps/server/src/core/comment/dto/create-comment.dto.ts +++ b/apps/server/src/core/comment/dto/create-comment.dto.ts @@ -33,8 +33,15 @@ export class CreateCommentDto { @IsJSON() content: any; + // The agent tool caps what it TYPES at 250 chars, but for a suggestion the + // client resolves and sends the RAW anchored document substring (the exact + // text under the mark), which can be longer once normalization is undone. Bound + // the stored value at 2000 (matching suggestedText) so a legitimate anchored + // substring is never rejected — the service used to lossily truncate at 250, + // which broke the apply-time equality check. @IsOptional() @IsString() + @MaxLength(2000) selection: string; @IsOptional() diff --git a/packages/mcp/build/client.js b/packages/mcp/build/client.js index d1cdc528..1b4b31d5 100644 --- a/packages/mcp/build/client.js +++ b/packages/mcp/build/client.js @@ -17,7 +17,7 @@ import { withPageLock } from "./lib/page-lock.js"; import { applyTextEdits, } from "./lib/json-edit.js"; import { getCollabToken, performLogin } from "./lib/auth-utils.js"; import { diffDocs, summarizeChange } from "./lib/diff.js"; -import { applyAnchorInDoc, canAnchorInDoc, countAnchorMatches, } from "./lib/comment-anchor.js"; +import { applyAnchorInDoc, canAnchorInDoc, countAnchorMatches, getAnchoredText, } from "./lib/comment-anchor.js"; import { blockText, walk, getList, insertMarkerAfter, setCalloutRange, noteItem, mdToInlineNodes, commentsToFootnotes, canonicalizeFootnotes, insertInlineFootnote, } from "./lib/transforms.js"; import vm from "node:vm"; // Supported image types, kept as two lookup tables so both a local file @@ -1936,6 +1936,16 @@ export class DocmostClient { if (!isReply && (!selection || !selection.trim())) { throw new Error("create_comment: an inline 'selection' (exact text to anchor on) is required for a top-level comment"); } + // For a SUGGESTION, the value we store as the comment's `selection` must be + // the RAW document substring the mark lands on (typographic quotes/dashes, + // nbsp, collapsed whitespace), NOT the agent's ASCII input. The anchor is + // placed via normalization, so when the doc was auto-converted to + // typographic the raw substring differs from the agent input; apply-time + // compares the stored selection to the marked doc text STRICTLY, so storing + // the raw substring is what makes "Apply" succeed instead of a spurious 409. + // Captured in the pre-check below (which already reads the page) and used as + // payload.selection. Ordinary comments keep sending the raw agent selection. + let anchoredSelection = null; // For a top-level comment, fail BEFORE creating anything when the selection // is not present in the persisted document — this avoids leaving an orphan // comment + notification behind. A read failure (network) is non-fatal: the @@ -1958,6 +1968,11 @@ export class DocmostClient { "A suggested edit must anchor to a UNIQUE location; expand the selection with surrounding context " + "(still <=250 chars) so it appears exactly once."); } + // Exactly one match: capture the RAW anchored substring to store as the + // comment selection (so apply-time equality holds). If this returns + // null despite countAnchorMatches===1 (shouldn't happen), fall back to + // the raw agent selection below rather than crash. + anchoredSelection = getAnchoredText(page.content, selection); } else if (!canAnchorInDoc(page.content, selection)) { throw new Error("create_comment: could not find the selection text in the page to anchor the comment. " + @@ -1987,8 +2002,13 @@ export class DocmostClient { content: JSON.stringify(jsonContent), type: effectiveType, }; + // For a suggestion, store the RAW anchored substring (anchoredSelection) so + // the stored selection === the text under the mark === apply-time + // expectedText. Ordinary comments (and the null fallback) keep the raw + // agent selection — their selection is only display/anchor and never used + // by apply, so their behavior is unchanged. if (!isReply && selection) - payload.selection = selection; + payload.selection = anchoredSelection ?? selection; if (parentCommentId) payload.parentCommentId = parentCommentId; // Only a top-level inline comment (with a selection) may carry a suggestion. diff --git a/packages/mcp/build/lib/comment-anchor.js b/packages/mcp/build/lib/comment-anchor.js index 03a50931..b5c26dc4 100644 --- a/packages/mcp/build/lib/comment-anchor.js +++ b/packages/mcp/build/lib/comment-anchor.js @@ -148,6 +148,67 @@ export function findAnchorInBlock(blockContent, selection) { } return null; } +/** + * Reconstruct the RAW text spanned by an AnchorMatch inside one block's + * `content` array. `startChild..endChild` are all text nodes (guaranteed by + * findAnchorInBlock, which only builds runs of `text` nodes), so concatenate + * each node's text slice: from `startOffset` on the first node, up to + * `endOffset` on the last, and the whole `.text` for any node fully inside the + * range. Mirrors spliceCommentMark's per-node slicing so the string returned + * here is EXACTLY the characters the comment mark will cover. + */ +function reconstructRawText(blockContent, match) { + const { startChild, startOffset, endChild, endOffset } = match; + let out = ""; + for (let k = startChild; k <= endChild; k++) { + const n = blockContent[k]; + const text = typeof n.text === "string" ? n.text : ""; + const sliceStart = k === startChild ? startOffset : 0; + const sliceEnd = k === endChild ? endOffset : text.length; + out += text.slice(sliceStart, sliceEnd); + } + return out; +} +/** + * Return the RAW document substring that `selection` would anchor to — the exact + * characters the comment mark will cover — or `null` when the selection cannot + * be anchored anywhere in `doc`. + * + * This mirrors canAnchorInDoc / applyAnchorInDoc EXACTLY (same depth-first, + * document-order traversal and the same findAnchorInBlock match on the FIRST + * matching block), but instead of a boolean / an in-place mutation it + * reconstructs the raw text spanned by the matched range. Because + * findAnchorInBlock maps the normalized selection back to raw text-node + * positions, the returned string is the document's ORIGINAL characters (smart + * quotes, em-dashes, nbsp, collapsed whitespace) — NOT the normalized ASCII + * agent input. + * + * Callers store THIS as the comment's `selection` so the stored value equals the + * text actually under the mark, which is what the apply-suggestion equality + * check (replaceYjsMarkedText's `joinedText !== expectedText`) compares against. + * Without it a suggestion whose anchor only matched via normalization would be + * un-appliable (spurious 409). + */ +export function getAnchoredText(doc, selection) { + const visit = (node, depth) => { + if (depth > MAX_DEPTH || !node || typeof node !== "object") + return null; + if (!Array.isArray(node.content)) + return null; + const match = findAnchorInBlock(node.content, selection); + if (match) + return reconstructRawText(node.content, match); + for (const child of node.content) { + if (child && typeof child === "object" && Array.isArray(child.content)) { + const found = visit(child, depth + 1); + if (found !== null) + return found; + } + } + return null; + }; + return visit(doc, 0); +} /** * Depth-first, document-order check for whether `selection` can be anchored * anywhere in `doc`. At each node with an array `content`, first try to match diff --git a/packages/mcp/src/client.ts b/packages/mcp/src/client.ts index 9e9eb421..cd29d494 100644 --- a/packages/mcp/src/client.ts +++ b/packages/mcp/src/client.ts @@ -60,6 +60,7 @@ import { applyAnchorInDoc, canAnchorInDoc, countAnchorMatches, + getAnchoredText, } from "./lib/comment-anchor.js"; import { blockText, @@ -2433,6 +2434,17 @@ export class DocmostClient { ); } + // For a SUGGESTION, the value we store as the comment's `selection` must be + // the RAW document substring the mark lands on (typographic quotes/dashes, + // nbsp, collapsed whitespace), NOT the agent's ASCII input. The anchor is + // placed via normalization, so when the doc was auto-converted to + // typographic the raw substring differs from the agent input; apply-time + // compares the stored selection to the marked doc text STRICTLY, so storing + // the raw substring is what makes "Apply" succeed instead of a spurious 409. + // Captured in the pre-check below (which already reads the page) and used as + // payload.selection. Ordinary comments keep sending the raw agent selection. + let anchoredSelection: string | null = null; + // For a top-level comment, fail BEFORE creating anything when the selection // is not present in the persisted document — this avoids leaving an orphan // comment + notification behind. A read failure (network) is non-fatal: the @@ -2459,6 +2471,11 @@ export class DocmostClient { "(still <=250 chars) so it appears exactly once.", ); } + // Exactly one match: capture the RAW anchored substring to store as the + // comment selection (so apply-time equality holds). If this returns + // null despite countAnchorMatches===1 (shouldn't happen), fall back to + // the raw agent selection below rather than crash. + anchoredSelection = getAnchoredText(page.content, selection); } else if (!canAnchorInDoc(page.content, selection)) { throw new Error( "create_comment: could not find the selection text in the page to anchor the comment. " + @@ -2496,7 +2513,13 @@ export class DocmostClient { content: JSON.stringify(jsonContent), type: effectiveType, }; - if (!isReply && selection) payload.selection = selection; + // For a suggestion, store the RAW anchored substring (anchoredSelection) so + // the stored selection === the text under the mark === apply-time + // expectedText. Ordinary comments (and the null fallback) keep the raw + // agent selection — their selection is only display/anchor and never used + // by apply, so their behavior is unchanged. + if (!isReply && selection) + payload.selection = anchoredSelection ?? selection; if (parentCommentId) payload.parentCommentId = parentCommentId; // Only a top-level inline comment (with a selection) may carry a suggestion. if (!isReply && selection && hasSuggestion) { diff --git a/packages/mcp/src/lib/comment-anchor.ts b/packages/mcp/src/lib/comment-anchor.ts index 7deb6620..cb494b1b 100644 --- a/packages/mcp/src/lib/comment-anchor.ts +++ b/packages/mcp/src/lib/comment-anchor.ts @@ -171,6 +171,65 @@ export function findAnchorInBlock( return null; } +/** + * Reconstruct the RAW text spanned by an AnchorMatch inside one block's + * `content` array. `startChild..endChild` are all text nodes (guaranteed by + * findAnchorInBlock, which only builds runs of `text` nodes), so concatenate + * each node's text slice: from `startOffset` on the first node, up to + * `endOffset` on the last, and the whole `.text` for any node fully inside the + * range. Mirrors spliceCommentMark's per-node slicing so the string returned + * here is EXACTLY the characters the comment mark will cover. + */ +function reconstructRawText(blockContent: any[], match: AnchorMatch): string { + const { startChild, startOffset, endChild, endOffset } = match; + let out = ""; + for (let k = startChild; k <= endChild; k++) { + const n = blockContent[k]; + const text: string = typeof n.text === "string" ? n.text : ""; + const sliceStart = k === startChild ? startOffset : 0; + const sliceEnd = k === endChild ? endOffset : text.length; + out += text.slice(sliceStart, sliceEnd); + } + return out; +} + +/** + * Return the RAW document substring that `selection` would anchor to — the exact + * characters the comment mark will cover — or `null` when the selection cannot + * be anchored anywhere in `doc`. + * + * This mirrors canAnchorInDoc / applyAnchorInDoc EXACTLY (same depth-first, + * document-order traversal and the same findAnchorInBlock match on the FIRST + * matching block), but instead of a boolean / an in-place mutation it + * reconstructs the raw text spanned by the matched range. Because + * findAnchorInBlock maps the normalized selection back to raw text-node + * positions, the returned string is the document's ORIGINAL characters (smart + * quotes, em-dashes, nbsp, collapsed whitespace) — NOT the normalized ASCII + * agent input. + * + * Callers store THIS as the comment's `selection` so the stored value equals the + * text actually under the mark, which is what the apply-suggestion equality + * check (replaceYjsMarkedText's `joinedText !== expectedText`) compares against. + * Without it a suggestion whose anchor only matched via normalization would be + * un-appliable (spurious 409). + */ +export function getAnchoredText(doc: any, selection: string): string | null { + const visit = (node: any, depth: number): string | null => { + if (depth > MAX_DEPTH || !node || typeof node !== "object") return null; + if (!Array.isArray(node.content)) return null; + const match = findAnchorInBlock(node.content, selection); + if (match) return reconstructRawText(node.content, match); + for (const child of node.content) { + if (child && typeof child === "object" && Array.isArray(child.content)) { + const found = visit(child, depth + 1); + if (found !== null) return found; + } + } + return null; + }; + return visit(doc, 0); +} + /** * Depth-first, document-order check for whether `selection` can be anchored * anywhere in `doc`. At each node with an array `content`, first try to match diff --git a/packages/mcp/test/mock/create-comment.test.mjs b/packages/mcp/test/mock/create-comment.test.mjs index d854bdb0..bcf70c44 100644 --- a/packages/mcp/test/mock/create-comment.test.mjs +++ b/packages/mcp/test/mock/create-comment.test.mjs @@ -452,3 +452,99 @@ test("suggestedText with a unique selection succeeds and forwards the payload", assert.equal(createPayload.selection, "brave"); assert.equal(result.data.suggestedText, "bold", "filterComment surfaces suggestedText"); }); + +// ----------------------------------------------------------------------------- +// 8) suggestedText where the DOC has TYPOGRAPHIC text and the agent selection is +// ASCII: the stored selection sent to /comments/create MUST be the doc's RAW +// typographic substring (what the mark covers), NOT the agent's ASCII input. +// This is the F1 contract that makes "Apply" succeed instead of a spurious +// 409 (apply compares the stored selection to the marked doc text strictly). +// ----------------------------------------------------------------------------- +test("suggestedText: the stored selection is the doc's RAW typographic substring, not the ASCII input", async () => { + let createPayload = null; + + const { baseURL } = await spawn(async (req, res) => { + const raw = await readBody(req); + if (req.url === "/api/auth/login") { + sendJson(res, 200, { success: true }, { + "Set-Cookie": "authToken=t; Path=/; HttpOnly", + }); + return; + } + if (req.url === "/api/pages/info") { + // The doc holds SMART quotes; the agent will select the ASCII form. + sendJson(res, 200, { + data: { + id: "22222222-2222-2222-2222-222222222222", + content: { + type: "doc", + content: [ + { + type: "paragraph", + content: [{ type: "text", text: "he said “hello” loudly" }], + }, + ], + }, + }, + }); + return; + } + if (req.url === "/api/comments/create") { + createPayload = JSON.parse(raw); + sendJson(res, 200, { + data: { + id: "cmt-typo-1", + content: createPayload.content, + selection: createPayload.selection, + suggestedText: createPayload.suggestedText, + type: createPayload.type, + }, + }); + return; + } + sendJson(res, 404, { message: "not found" }); + }); + + class TestClient extends DocmostClient { + async getCollabTokenWithReauth() { + return "collab-token"; + } + async resolvePageId() { + return "22222222-2222-2222-2222-222222222222"; + } + async mutatePage(pageId, collabToken, apiUrl, transform) { + const doc = { + type: "doc", + content: [ + { + type: "paragraph", + content: [{ type: "text", text: "he said “hello” loudly" }], + }, + ], + }; + const out = transform(doc); + return { doc: out, verify: { ok: true } }; + } + } + + const client = new TestClient(baseURL, "user@example.com", "pw"); + + const result = await client.createComment( + "22222222-2222-2222-2222-222222222222", + "please change", + "inline", + '"hello"', // ASCII quotes — the doc has smart quotes + undefined, + "goodbye", + ); + + assert.equal(result.success, true); + assert.equal(result.anchored, true); + assert.ok(createPayload, "/comments/create must have been called"); + assert.equal( + createPayload.selection, + "“hello”", + "the stored selection must be the doc's RAW typographic substring, not the ASCII input", + ); + assert.equal(createPayload.suggestedText, "goodbye"); +}); diff --git a/packages/mcp/test/unit/comment-anchor.test.mjs b/packages/mcp/test/unit/comment-anchor.test.mjs index c35d853b..87e61def 100644 --- a/packages/mcp/test/unit/comment-anchor.test.mjs +++ b/packages/mcp/test/unit/comment-anchor.test.mjs @@ -7,6 +7,7 @@ import { canAnchorInDoc, applyAnchorInDoc, countAnchorMatches, + getAnchoredText, } from "../../build/lib/comment-anchor.js"; const COMMENT_ID = "cmt-123"; @@ -274,3 +275,36 @@ test("countAnchorMatches applies the same normalization as anchoring", () => { const doc = paragraphDoc([{ type: "text", text: "say “hi” now" }]); assert.equal(countAnchorMatches(doc, '"hi"'), 1); }); + +// ----------------------------------------------------------------------------- +// getAnchoredText: returns the RAW document substring the mark would cover (the +// doc's original typographic characters), not the normalized ASCII selection. +// This is what makes a suggestion's stored selection equal the apply-time +// expectedText, so the strict equality in replaceYjsMarkedText holds. +// ----------------------------------------------------------------------------- +test("getAnchoredText returns the RAW (typographic) doc substring for an ASCII selection", () => { + // Doc holds smart quotes; agent selection is the ASCII form. + const doc = paragraphDoc([{ type: "text", text: "he said “hello” loudly" }]); + assert.equal(getAnchoredText(doc, '"hello"'), "“hello”"); +}); + +test("getAnchoredText undoes whitespace/dash normalization to the raw span", () => { + // Em-dash + nbsp in the doc; ASCII hyphen + single space in the selection. + const doc = paragraphDoc([{ type: "text", text: "a—b c" }]); + // selection "a-b c" (ascii dash) matches, raw substring keeps the em-dash+nbsp. + assert.equal(getAnchoredText(doc, "a-b c"), "a—b c"); +}); + +test("getAnchoredText spans consecutive text nodes and returns their raw slices", () => { + const doc = paragraphDoc([ + { type: "text", text: "Hello " }, + { type: "text", text: "“brave”", marks: [{ type: "bold" }] }, + { type: "text", text: " world" }, + ]); + assert.equal(getAnchoredText(doc, '"brave" wor'), "“brave” wor"); +}); + +test("getAnchoredText returns null when the selection does not anchor", () => { + const doc = paragraphDoc([{ type: "text", text: "hello world" }]); + assert.equal(getAnchoredText(doc, "not present"), null); +});