Skip to content

GO-7167 Soft-delete discussion comments - #3324

Merged
requilence merged 19 commits into
developfrom
go-7167-soft-delete-for-discussion-comments
Oct 12, 2026
Merged

requilence merged 19 commits into
developfrom
go-7167-soft-delete-for-discussion-comments

Conversation

@requilence

Copy link
Copy Markdown
Contributor

Problem

Deleting a discussion comment removes it from the store and clients get Event.Chat.Delete with only the id. There is nothing left to render the designed "Comment deleted" placeholder from, and every reply under the comment disappears: clients drop a reply whose replyToMessageId chain hits a missing message. Android replies to the tapped message and API agents reply to the message they answer, so this applies to any comment in a thread, not just a post. The replies stay in storage and keep syncing.

Design: docs/superpowers/specs/2026-10-12-discussion-soft-delete-design.md.

Change

Storage (discussions only). The store Delete op is unchanged on the wire. In a discussion, ChatHandler.BeforeDelete turns the message into a tombstone and keeps it (new storestate.DeleteModeKeep).

  • The tombstone keeps id, author, createdAt, replyToMessageId and its place (_o).
  • It sets deleted: true (new ChatMessage.deleted = 18, sparse index) and is always read and never a mention.
  • Content, attachments, blocks, reactions and the pin are cleared, and modifiedAt is reset (all clients show "(edited)" otherwise).
  • Space chats still hard-delete.

Apply rules.

  • An edit, reaction or pin of a tombstone is dropped at apply time, so delete wins whatever the order changes arrive in.
  • A second delete is a no-op.
  • A tombstone write failure aborts the apply (ErrCritical).
  • A full replay builds the same tombstones, including for deletes made before this change.
  • Chat modifies are now update-only. Before, a reaction or pin that arrived after a hard delete re-created a stub message with no author, counted as unread.

Counters and events.

  • SoftDelete sends Event.Chat.Update carrying the tombstone and drops the message from the unread, mention and message counters at once, plus the post-commit reload.
  • messageCount and GetLastMessage (lastMessageDate) skip tombstones.
  • Subscription windows and stream backfill keep them, so replies keep their thread.
  • When a full window evicts a message, its pending update is kept, and a later status change in the batch reaches it.

Local guards.

  • Edit, reaction and pin need a live, locally stored target (ErrMessageDeleted / ErrMessageNotFound, BAD_INPUT).
  • A new reply to a tombstone is refused (ErrReplyToDeletedMessage).
  • Deleting a missing or deleted message succeeds without pushing a change.
  • SetMessagesPinned checks every target before pinning any.

Search. Full-text hits on tombstones are dropped, and browse reads live messages only. The full-text index removes the comment and never re-adds it.

API v2.

  • Messages serve deleted: true with empty text.
  • Edit, reaction and delete of a tombstone return 404, and a reply to one returns 400 on /reply_to, dry runs included.
  • The middleware refusals map to the same statuses when a delete races the check.
  • No new response codes. message_count descriptions updated; OpenAPI and the wrapper golden regenerated.

API v1. deleted field; a reaction to a tombstone returns 400 instead of 500.

Compatibility

  • Old apps see no difference. Their heart applies the same Delete op as a hard delete, exactly as today.
  • A new heart with a UI that doesn't know deleted. It renders a tombstone as a comment with author and date and no body (checked on desktop, iOS and Android; none crash), and the replies stay attached. Client work:
    • render the placeholder;
    • hide react, reply and edit on a tombstone;
    • desktop: stop removing your own deleted comment locally (post.tsx:439), otherwise the deleter sees the comment and its replies vanish until reload; follow the reply chain to the root;
    • iOS: IOS-5944.
  • Same-profile downgrade. A profile a new heart already wrote tombstones into shows them as empty comments on an older heart (documented, no reindex bump).

Not in this PR

  • No migration. Comments hard-deleted before this change stay gone on existing stores.
  • Two pre-existing bugs, to file separately:
    • a reply applied in the same transaction as its parent loses its mention (the parent lookup reads outside the apply transaction);
    • BeforeCreate skips a message when its full-text enqueue fails.

Tests

  • New softdelete_test.go files in chatobject, chatrepository, chatsubscription and chats, and chat_softdelete_test.go in api/v2/service, plus storestate, chatmodel, indexer, v1 handler and service tests.
  • They cover the tombstone shape against the pre-delete message, delete-wins in both orders, replay, moderator deletes, counters (including a flush before the reload), events, window eviction, guards, search, the index used by each query, and the exact served JSON.
  • go test ./core/... ./pkg/... ./space/... ./util/... passes.

Discussion comments become tombstones on delete instead of disappearing,
so replies keep their thread. The wire op stays the existing Delete;
only discussions materialize it differently.
Separate tombstone-inclusive reads (windows, backfill) from live-only
ones (search browse, last-message date); require local targets before
pushing modifies or deletes; abort the apply on a tombstone write
failure; adjust counters at once instead of only after the reload;
pin batches preflight; v2/v1 error mapping for the races; record the
inherited pre-commit side-effect limitation.
Read the reply parent through the apply transaction so replayed
replies keep their mention; keep pending events of entries a full
window evicts; tell the two SSE streams apart for a create and delete
in one batch; serve author only when resolved; document the
same-profile downgrade boundary; add ChatSearch service tests.
It is a pre-existing bug unrelated to soft delete; record it under the
accepted limitations for its own ticket and qualify the mention claim.
ChatMessage.deleted marks a soft-deleted discussion comment. The model
stores it only when true and can turn a message into its tombstone
form; a create never arrives deleted.
DeleteModeKeep tells applyDelete the handler already rewrote the
document inside the apply transaction and it must stay.
CountMessages subtracts tombstones (a sparse index on deleted counts
them), mark-unread skips them, the fulltext iterator skips them, and
GetLastLiveMessages / GetLastMessagesByCreators return live messages
only. GetLastMessages keeps them: it seeds subscription windows.
A discussion's Delete op rewrites the message into its tombstone and
keeps it (DeleteModeKeep): author, time, reply target and place stay,
content, attachments, reactions and the pin go. The manager's
SoftDelete sends the tombstone as an update and drops the message from
the counters at once; the post-commit reload still runs. Modifies of a
tombstone are dropped at apply time, so delete wins in either order,
and chat modifies become update-only so a reaction or pin after a hard
delete no longer re-creates a stub. A failed tombstone write is
critical.
…s as last message

When a full window evicts its oldest entry, unflushed updates (a
tombstone, an edit, a reaction, a pin) move to the out-of-window
events instead of being dropped; an unflushed Add is still dropped.
GetLastMessage returns the newest live message.
Edit, reaction and pin need a live, locally stored target
(ErrMessageDeleted / ErrMessageNotFound); a new reply to a deleted
comment is refused (ErrReplyToDeletedMessage); deleting a missing or
deleted message succeeds without a change. SetMessagesPinned checks
every target before pushing any pin. The RPCs report the three as
BAD_INPUT.
A fulltext hit whose message is now a tombstone is dropped, and browse
without creators reads live messages only (GetLastLiveMessages).
Messages carry deleted: true on a tombstone. Edit, reaction and delete
of a deleted comment are a 404 and a reply to one a 400 on reply_to,
dry runs included; the middleware's deleted/missing refusals map to
the same statuses when a delete races the check. The message_count
description says a deleted comment stays but isn't counted.
Messages carry deleted on a tombstone, in reads and stream events, and
a reaction the middleware refuses as bad input (a deleted comment) is
a 400 instead of a 500.
Status events (read, mention read, synced, reaction read) exist only
for in-window messages; a kept one could not be corrected by a later
status change in the same batch and was sent stale.
Expected tombstones come from the message as it was before the delete,
and its stored order object is compared; Owner and Admin both delete;
a repeat delete makes no observer change and no fulltext work; the
in-batch counter test covers the mention counter.
The tombstone-aware repository queries are built in one place and the
index tests explain those exact queries; tests cover the v1 reaction
400 at the handler, the last-message fallback past a tombstone-only
window, an indexer backfill skipping a tombstone, and creatorless
browse only (the creator read is live-only in the repository). The
served message_count field says a deleted comment isn't counted.
…atus changes

A status change (read, mention read, synced, reaction read) of a
message outside the window sends no event of its own, but a pending
full update of that message is still sent: it now carries the new
value instead of the one it was queued with. The unflushed-Add
eviction test now gives the evicted message an update too, so it
guards the Add check.
@requilence
requilence merged commit 55e43ac into develop Oct 12, 2026
13 of 16 checks passed
@requilence
requilence deleted the go-7167-soft-delete-for-discussion-comments branch October 12, 2026 13:52
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 12, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant