Split publish/cleanup gates so allowlist changes don't orphan remote records#38
Merged
Split publish/cleanup gates so allowlist changes don't orphan remote records#38
Conversation
Merged
6 tasks
There was a problem hiding this comment.
Pull request overview
This PR fixes a rollback hazard in Atmosphere’s post lifecycle hooks where narrowing the syncable post-type allowlist could prevent cleanup of already-synced remote AT Protocol records, leaving orphaned records behind.
Changes:
- Split
on_status_change()gating so the allowlist applies only to publish/update scheduling, while unpublish cleanup schedules deletion based on existing stored TIDs (regardless of current allowlist). - Remove the allowlist check from
on_before_delete()so permanent deletes always capture TIDs and schedule remote record cleanup when publication metadata exists. - Add PHPUnit coverage for the new cleanup behavior and for the regression that new publishes still respect the allowlist.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
includes/class-atmosphere.php |
Separates publish/update allowlist gating from cleanup paths; ensures unpublish + permanent delete still clean up remote records based on stored TIDs. |
tests/phpunit/tests/class-test-status-change.php |
Adds regression tests verifying cleanup bypasses allowlist for previously-synced posts, while new publish remains allowlist-gated. |
.github/changelog/fix-cleanup-gate-split |
Adds changelog entry documenting the fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pfefferle
reviewed
Apr 24, 2026
1 task
pfefferle
approved these changes
Apr 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the rollback hazard where narrowing `atmosphere_syncable_post_types` (directly or via the `activitypub_support_post_types` option projected into that filter, as FOSSE's `Post_Types` projector does) orphans the remote AT Protocol records of posts that were already synced under the wider configuration.
Proposed changes:
Rationale
The bug is a gate-conflation issue. `Backfill::syncable_post_types()` answers "what post types is this site willing to publish to Bluesky?" — a publish-time eligibility question. Both cleanup paths were reusing it as a proxy for "was this post ever synced?", which only coincides when the allowlist never narrows. A site owner unchecking a post type on the AP side (where FOSSE's new projector surfaces that control in a way most sites already used) becomes a normal operation, not an edge case — at which point the conflation silently leaks orphaned records.
FOSSE PR Automattic/fosse#31 surfaces this path by making the allowlist editable via AP's existing checkbox UI, but the bug pre-exists in every standalone Atmosphere site that filters `atmosphere_syncable_post_types` in PHP. Fixing it here — not in a FOSSE-side workaround — is correct per Atmosphere's existing upstream-first positioning: post-type-agnostic correctness lives here.
Other information:
Three new PHPUnit cases in `tests/phpunit/tests/class-test-status-change.php`:
Full suite runs clean (97/97).
Testing instructions:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Preserve remote cleanup of already-synced posts when their post type is removed from the syncable allowlist.
Companion references: