Automattic / Automattic/wordpress-atmosphere

Guard each reply's origin DID in the post-delete comment cascade

Open
#215 0 comments 0 reactions 0 assignees View on GitHub
enhancement
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.