Automattic / Automattic/wordpress-atmosphere
Guard each reply's origin DID in the post-delete comment cascade
- Dominant language
- PHP
- Stars
- 52
- Forks
- 3
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 22
Description
Follow-up from #70 / #71.
Those PRs added the wrong-repo-delete guard to the dedicated comment cleanup paths (`delete_comment`, `delete_comment_by_tid`) and to the post root/document records in the hard-delete cascade (`delete_post_by_tids`). One case is still uncovered: the **comment batch inside the post hard-delete cascade** only checks the post's own origin DID, never each reply's.
## The gap
`on_before_delete()` collects reply TIDs via `array_column( collect_published_comment_tids(), 'tid' )`, dropping the per-comment DID, and `delete_post_by_tids()` only gates the comment batch on the post's `bsky_origin_did` / `doc_origin_did`. So a reply published under a since-abandoned account, riding along with a post whose own records are same-account (or predate provenance), gets an `applyWrites#delete` fired against the current repo. It no-ops idempotently and the reply stays stranded on the old account.
The same gap exists in the already-shipped `delete_post()` (the trash path), which likewise never applies its `$bsky_skip` / `$doc_skip` check to `$comment_writes`. So this should fix both to keep the two paths symmetric.
## Why it is low severity today
On permanent deletion the reply also gets its own guarded `atmosphere_delete_comment_record` event (`on_comment_before_delete` fires when WP cascades the comment deletion), so the mismatch is already logged. The cascade just issues a redundant wrong-repo no-op alongside it. Net cost is a pointless write, not a lost breadcrumb. That is why it was split out rather than blocking #70 / #71.
## Fix sketch
- Have `collect_published_comment_tids()` return the origin DID per comment.
- Filter foreign-DID replies out of the comment batch in both `delete_post_by_tids()` and `delete_post()` (skip the individual write, do not abort the whole cascade, since the comment batch is already decoupled from the root batch).
- The data is already there: `Comment::META_DID` is written on every publish as of #70.
Contributor guide
Research direction
Start with on_before_delete(), collect_published_comment_tids(), delete_post_by_tids(), and delete_post(), then inspect Comment::META_DID and the existing delete-comment guards. Keep the two post-deletion paths symmetric by excluding foreign-DID replies from the comment batch while preserving the root and individual comment cleanup; done means no wrong-repo comment delete writes are issued.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php, wordpress
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 73/100