diff --git a/apps/server/src/core/comment/comment.service.ts b/apps/server/src/core/comment/comment.service.ts index 88122e96..85af14e0 100644 --- a/apps/server/src/core/comment/comment.service.ts +++ b/apps/server/src/core/comment/comment.service.ts @@ -638,14 +638,17 @@ export class CommentService { * inline `comment` anchor mark, then ATOMICALLY hard-delete the row only if it * is still childless. Shared by the apply/dismiss no-replies branches (#329). * - * ORDER MATTERS: the anchor mark is removed FIRST and FATALLY (mirrors - * applySuggestion, which mutates the doc before writing the DB). The row - * delete is irreversible, so if the mark removal fails — including the - * COLLAB_DISABLE_REDIS "no live instance" hard-error — we must NOT delete the - * row and report success, or the document is left with a permanent orphan - * anchor pointing at a comment that no longer exists (the exact data-integrity - * bug #329 targets). Let the exception propagate (→ 5xx); the operation is - * then repeatable with row + mark still consistent. + * ORDER MATTERS (updated #399 → #496): what runs FIRST and FATALLY here is the + * mark-removal ENQUEUE (a fast, durable Redis add), NOT the mark op itself. + * deleteCommentMark awaits only the enqueue, so a failed add throws BEFORE the + * irreversible row delete — the row + mark stay consistent and the operation is + * repeatable. The actual anchor strip then runs off the HTTP path in the worker + * (idempotent, 3 retries). Only an EXHAUSTED-retries job could leave the doc + * with an orphan anchor pointing at a hard-deleted comment (the data-integrity + * bug #329 targets); that residual divergence is now self-healed by the + * resolve/unresolve mark worker, which strips an orphan mark whenever its + * comment row is gone (#496), and it is meanwhile VISIBLE via BullMQ failed-job + * metrics rather than a silently-swallowed warn. * * RACE (#338 F4): the caller read `hasChildren` BEFORE the (slow) mark * removal, so a reply can land in that window. `comments.parent_comment_id` is diff --git a/apps/server/src/integrations/queue/processors/general-queue.processor.comment-mark.spec.ts b/apps/server/src/integrations/queue/processors/general-queue.processor.comment-mark.spec.ts index 7adf30ed..29c958c7 100644 --- a/apps/server/src/integrations/queue/processors/general-queue.processor.comment-mark.spec.ts +++ b/apps/server/src/integrations/queue/processors/general-queue.processor.comment-mark.spec.ts @@ -139,13 +139,19 @@ describe('GeneralQueueProcessor — COMMENT_MARK_UPDATE (#399)', () => { ); }); - it('skips (no throw) when the comment row has vanished', async () => { + it('reconcile (#496): comment row vanished → strips the orphan anchor mark', async () => { const { proc, collaborationGateway, commentRepo } = makeProc(); commentRepo.findById.mockResolvedValue(undefined); await expect( proc.process(job({ ...base, action: 'resolve', ts: 1000 })), ).resolves.toBeUndefined(); - expect(collaborationGateway.handleYjsEvent).not.toHaveBeenCalled(); + // A resolve/unresolve mark job whose comment row is gone leaves a silent + // orphan; the worker self-heals by stripping the anchor instead of returning. + expect(collaborationGateway.handleYjsEvent).toHaveBeenCalledWith( + 'deleteCommentMark', + 'page.page-1', + { commentId: 'c-1', user: { id: 'user-1' } }, + ); }); }); diff --git a/apps/server/src/integrations/queue/processors/general-queue.processor.ts b/apps/server/src/integrations/queue/processors/general-queue.processor.ts index e75cd8c7..d2dee503 100644 --- a/apps/server/src/integrations/queue/processors/general-queue.processor.ts +++ b/apps/server/src/integrations/queue/processors/general-queue.processor.ts @@ -95,7 +95,8 @@ export class GeneralQueueProcessor * #399: apply a comment's inline-mark mirror in the collab Y.Doc, off the HTTP * critical path. Runs the SAME gateway path the synchronous comment.service * code used (byte-identical mark op): - * - resolve / unresolve → resolveCommentMark (flip the `resolved` attribute); + * - resolve / unresolve → resolveCommentMark (flip the `resolved` attribute), + * OR strip an orphan anchor when the comment row has vanished (#496); * - delete → deleteCommentMark (strip the ephemeral-suggestion anchor #329). * The op is idempotent, so a BullMQ retry is safe. Throwing propagates to * WorkerHost → the job is retried and, on exhaustion, surfaces in failed-job @@ -133,7 +134,18 @@ export class GeneralQueueProcessor // of this resolve), skip it rather than flip the mark to a stale state. const comment = await this.commentRepo.findById(commentId); if (!comment) { - // The comment vanished (e.g. hard-deleted) → nothing left to mirror. + // #496 reconcile: the comment row is GONE (e.g. an ephemeral apply/dismiss + // hard-deleted it while this resolve/unresolve mark job sat in the queue), + // but its inline anchor may still live in the doc — a silent orphan mark + // pointing at a comment that no longer exists. Self-heal by stripping it + // instead of just returning: this closes the divergence the fire-and-forget + // resolve/unresolve enqueue (comment.service resolveComment) could leave. + // Idempotent — deleteCommentMark on an already-absent mark is a no-op. + await this.getCollaborationGateway().handleYjsEvent( + 'deleteCommentMark', + documentName, + { commentId, user }, + ); return; } const wantResolved = action === 'resolve';