fix(comments): orphan-anchor reconcile + docstring (#496)
deleteEphemeralSuggestion docstring обещал инвариант «метка снимается FIRST and FATALLY» синхронно — после #399 это уже не так: fatal только ENQUEUE снятия метки, сама операция идёт в воркере с ретраями. Docstring переписан под фактическое поведение. Reconcile: воркер COMMENT_MARK_UPDATE на resolve/unresolve, обнаружив что строки комментария больше нет (hard-delete гонкой с ephemeral apply/ dismiss), теперь СНИМАЕТ осиротевшую метку вместо тихого return. Это самозаживляет тихую дивергенцию и закрывает fire-and-forget resolve/ unresolve enqueue из resolveComment. Операция идемпотентна. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
+8
-2
@@ -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' } },
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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';
|
||||
|
||||
Reference in New Issue
Block a user