From 878acaa72f2f7cb263d9e9100c591accde7e3b6b Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Thu, 23 Apr 2026 20:24:15 -0500 Subject: [PATCH 01/22] WIP: long-form teaser-thread strategy This branch introduces Atmosphere-side support for multi-post thread composition for long-form posts. Tracks the FOSSE SDD at https://github.com/Automattic/fosse/tree/trunk/sdd/long-form-bluesky-strategy. Opening as draft to start the async review window. Implementation lands in follow-up commits on this branch. Linear: https://linear.app/a8c/issue/DOTCOM-16810 From fa9408d22e0be854b3435dea21cef11c246c6ef0 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Thu, 23 Apr 2026 20:33:08 -0500 Subject: [PATCH 02/22] Add atmosphere_long_form_composition filter and long-form record composition methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New on Atmosphere\Transformer\Post: - `META_THREAD_RECORDS` constant for the per-WP-post ordered array of `{ uri, cid, tid }` triples that Publisher will populate once it knows about threads (next commit). - `build_long_form_records()` as the long-form counterpart to `transform()`. Branches on `atmosphere_long_form_composition` (default `'link-card'`, i.e. today's behavior unchanged) to produce one record for `'link-card'` / `'truncate-link'`, or 2+ records for `'teaser-thread'`. `transform()` is unchanged — legacy callers keep today's contract. - `build_teaser_thread()` (private) composes the default hook + CTA pair, filterable via `atmosphere_teaser_thread_posts`. Hook uses the post's excerpt when set, else the first ~280 chars of the body cut at a sentence boundary so the hook never ends mid-sentence right before the CTA. - `build_truncate_link_text()` (private) for the single-post body-plus-permalink strategy. - `truncate_to_budget()` (private) — budget-aware truncation that prefers a sentence break, falls back to word boundary, then hard-caps with an ellipsis. Uses mb_strlen, matching the existing `truncate_text()` helper. - `is_short_form_post()` (public) wraps the existing `atmosphere_is_short_form_post` filter + private `is_short_form()` discriminator so Publisher can branch on short vs. long without duplicating the filter call inside its own code. - Empty-body guard: `'teaser-thread'` and `'truncate-link'` silently degrade to `'link-card'` when the post has neither a 10+-char excerpt nor a 10+-char body. Logs a notice so ops can tell the fallback from an intentional link-card configuration. New on Atmosphere\API: - `atmosphere_pre_apply_writes` filter. Returning a non-null value short-circuits the PDS call, so PHPUnit and the FOSSE e2e harness can observe the write batch without a real DPoP round-trip (`pre_http_request` fires inside `wp_remote_request`, too late to mock past the DPoP-proof step in test environments without a real JWK). Default behavior is unchanged: `atmosphere_long_form_composition` defaults to `'link-card'`. Existing sites see exactly today's output unless a downstream projects a different value. Tests cover these additions in the next commit. Linear: https://linear.app/a8c/issue/DOTCOM-16810 --- includes/class-api.php | 24 +++ includes/transformer/class-post.php | 315 ++++++++++++++++++++++++++++ 2 files changed, 339 insertions(+) diff --git a/includes/class-api.php b/includes/class-api.php index bdb666c..b3c0346 100644 --- a/includes/class-api.php +++ b/includes/class-api.php @@ -167,6 +167,30 @@ public static function upload_blob( string $file_path, string $mime_type ): arra * @return array|\WP_Error */ public static function apply_writes( array $writes ): array|\WP_Error { + /** + * Short-circuits the applyWrites call before it reaches the PDS. + * + * Return a non-null array (success shape: `[ 'results' => [...] ]`) + * or a `WP_Error` to bypass the real HTTP round-trip. Used by + * the PHPUnit suite, the FOSSE end-to-end harness, and anything + * else that needs to observe or mock a write batch without + * actually hitting the PDS. + * + * A common use is `pre_http_request`, but that filter fires + * inside `wp_remote_request`, which is only reached after the + * DPoP proof has been built — so in test environments without + * a real DPoP JWK, the call errors out first. This filter runs + * before any of that. + * + * @param null|array|\WP_Error $short_circuit Short-circuit value. Return null to skip. + * @param array $writes The write batch about to be sent. + */ + $short_circuit = \apply_filters( 'atmosphere_pre_apply_writes', null, $writes ); + + if ( null !== $short_circuit ) { + return $short_circuit; + } + return self::post( '/xrpc/com.atproto.repo.applyWrites', array( diff --git a/includes/transformer/class-post.php b/includes/transformer/class-post.php index 4ec0d51..c797be8 100644 --- a/includes/transformer/class-post.php +++ b/includes/transformer/class-post.php @@ -43,6 +43,20 @@ class Post extends Base { */ public const META_CID = '_atmosphere_bsky_cid'; + /** + * Post meta key for the ordered list of bsky post + * { uri, cid, tid } triples written for this WordPress post. + * + * Populated by Publisher on every successful publish — even the + * single-record case — so readers can enumerate every Bluesky + * record tied to the post from this key alone. The legacy + * META_URI / META_TID / META_CID keys continue to mirror index 0 + * (the root post) for backwards compatibility. + * + * @var string + */ + public const META_THREAD_RECORDS = '_atmosphere_bsky_thread_records'; + /** * Transform the post. * @@ -271,4 +285,305 @@ private function is_short_form( \WP_Post $post ): bool { private function build_short_form_text(): string { return truncate_text( $this->render_post_content_plain( $this->object ), 300 ); } + + /** + * Whether this post should be treated as short-form for Bluesky. + * + * Thin public wrapper around the private discriminator plus the + * `atmosphere_is_short_form_post` filter. Callers such as + * Publisher branch on short vs. long without reaching into the + * transformer's private state. + * + * @return bool + */ + public function is_short_form_post(): bool { + return \wp_validate_boolean( + \apply_filters( + 'atmosphere_is_short_form_post', + $this->is_short_form( $this->object ), + $this->object + ) + ); + } + + /** + * Produce the record(s) to publish for a long-form post. + * + * Branches on `atmosphere_long_form_composition`: + * - `'link-card'` (default): 1 record, today's title + excerpt + + * permalink + app.bsky.embed.external card. + * - `'truncate-link'`: 1 record, body text + inline permalink, + * no embed card. + * - `'teaser-thread'`: 2+ records forming a reply chain + * (hook + CTA by default; filterable to 3 posts via + * `atmosphere_teaser_thread_posts`). + * - unknown values: treated as `'link-card'`. + * + * Empty-body guard: for `'teaser-thread'` and `'truncate-link'`, + * if neither the post body nor an excerpt has at least 10 + * characters of prose, the strategy silently degrades to + * `'link-card'` and an error_log notice is emitted so operators + * can tell the fallback from an intentional configuration. + * + * Publisher stamps `createdAt` (and `reply` for thread entries + * 1..N) at write time — they are intentionally absent from the + * returned records. + * + * `Post::transform()` is unchanged and remains the entry point + * for the short-form path and for any legacy caller on today's + * single-record contract. + * + * @return array[] Bsky post records, in thread order (index 0 is + * the root / parent of any replies). + */ + public function build_long_form_records(): array { + /** + * Filters the long-form composition strategy for this post. + * + * @param string $strategy Composition strategy key. + * @param \WP_Post $post The post being transformed. + */ + $strategy = (string) \apply_filters( 'atmosphere_long_form_composition', 'link-card', $this->object ); + + if ( \in_array( $strategy, array( 'teaser-thread', 'truncate-link' ), true ) + && ! $this->has_composable_body() + ) { + // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log + \error_log( + \sprintf( + '[atmosphere] post %d has no composable body/excerpt; downgrading "%s" to "link-card"', + $this->object->ID, + $strategy + ) + ); + $strategy = 'link-card'; + } + + switch ( $strategy ) { + case 'teaser-thread': + $records = array(); + foreach ( $this->build_teaser_thread() as $text ) { + $records[] = $this->record_for_thread_entry( (string) $text ); + } + return $records; + + case 'truncate-link': + return array( $this->record_for_thread_entry( $this->build_truncate_link_text() ) ); + + case 'link-card': + default: + return array( $this->record_for_link_card() ); + } + } + + /** + * Truncate text to a character budget, preferring a sentence break. + * + * Priority order: + * 1. Sentence boundary (`.`, `!`, `?`, optionally followed by a + * close-quote / close-paren / close-bracket) inside the + * budget, when `$prefer_sentence` is true. + * 2. Word boundary — the last whitespace before the budget. + * 3. Hard cap: `$max - 1` chars + trailing ellipsis (a single + * unbroken token longer than the budget). + * + * Character length uses `mb_strlen`, matching the convention of + * the existing `truncate_text()` helper. Preg offsets are byte + * offsets against the `mb_substr`-clamped string; substr on a + * match's byte-end is UTF-8-safe because matches end on valid + * sequence boundaries. + * + * @param string $text Input text. + * @param int $max Maximum character length (mb_strlen). + * @param bool $prefer_sentence Prefer a sentence boundary over a word boundary. + * @return string + */ + private function truncate_to_budget( string $text, int $max, bool $prefer_sentence = true ): string { + if ( \mb_strlen( $text ) <= $max ) { + return $text; + } + + $clamped = \mb_substr( $text, 0, $max ); + + if ( $prefer_sentence + && \preg_match_all( + '/[.!?][\"\')\]]?(?=\s|$)/u', + $clamped, + $matches, + \PREG_OFFSET_CAPTURE + ) + ) { + $last = \end( $matches[0] ); + $byte_to = $last[1] + \strlen( $last[0] ); + return \substr( $clamped, 0, $byte_to ); + } + + $word_cut = \preg_replace( '/\s+\S*$/u', '', $clamped ); + if ( \is_string( $word_cut ) && '' !== $word_cut ) { + return $word_cut; + } + + // Hard cap. Reserve one character for the ellipsis. + return \mb_substr( $text, 0, \max( 1, $max - 1 ) ) . '…'; + } + + /** + * Compose the single-post truncate-link text. + * + * Used when `atmosphere_long_form_composition` returns + * `'truncate-link'`. Body-as-text plus trailing permalink. + * Word-boundary truncation is fine — the permalink follows + * immediately in the same post. + * + * @return string + */ + private function build_truncate_link_text(): string { + $permalink = \get_permalink( $this->object ); + $plain = $this->render_post_content_plain( $this->object ); + $budget = 300 - \mb_strlen( "\n\n" ) - \mb_strlen( $permalink ); + + $body = $this->truncate_to_budget( $plain, $budget, false ); + + return $body . "\n\n" . $permalink; + } + + /** + * Compose the default 2-post teaser thread: hook + CTA-with-link. + * + * Hook precedence: + * 1. If the post has a `post_excerpt`, use it (plain-text + * normalized, clamped to 300 chars as a safety floor). + * Excerpts are curated strings — a mid-word cut is unlikely + * at this length, so word-boundary fallback is enough. + * 2. Otherwise, use the first ~280 chars of the body text, + * cut at a **sentence boundary**. The hook is the final + * prose shown before the CTA post, so we never end + * mid-sentence. 280 leaves ~20 chars of headroom for future + * variants that append trailing content. + * + * CTA is an internationalised `Continue reading: `. + * + * Filterable via `atmosphere_teaser_thread_posts`. Downstream + * filters may return 3 entries to extend the thread; in that + * case the intermediate body-to-body cut (entry 1 → entry 2) + * may be at a word boundary, but the final body entry before + * the CTA (entry 2 → entry 3) must still cut at a sentence + * boundary. The return contract does not capture this — it's + * the filter author's responsibility. + * + * @return string[] Text of each post in order. At least 2 entries. + */ + private function build_teaser_thread(): array { + if ( ! empty( $this->object->post_excerpt ) ) { + $hook = $this->truncate_to_budget( sanitize_text( $this->object->post_excerpt ), 300, false ); + } else { + $plain = $this->render_post_content_plain( $this->object ); + $hook = $this->truncate_to_budget( $plain, 280, true ); + } + + $cta = \sprintf( + /* translators: %s: the WordPress post permalink. */ + \__( 'Continue reading: %s', 'atmosphere' ), + \get_permalink( $this->object ) + ); + + /** + * Filters the default teaser-thread post texts. + * + * @param string[] $posts 2-entry array: [ hook, cta ]. + * @param \WP_Post $post The post being composed. + */ + return \apply_filters( 'atmosphere_teaser_thread_posts', array( $hook, $cta ), $this->object ); + } + + /** + * Whether the post has enough prose to be worth building a thread from. + * + * Used by the `build_long_form_records()` empty-body guard. 10 + * characters is a defensive floor — anything below is noise and + * would produce a stub hook post. + * + * @return bool + */ + private function has_composable_body(): bool { + if ( ! empty( $this->object->post_excerpt ) + && \mb_strlen( sanitize_text( $this->object->post_excerpt ) ) >= 10 + ) { + return true; + } + + return \mb_strlen( $this->render_post_content_plain( $this->object ) ) >= 10; + } + + /** + * Build one thread-entry record (hook, intermediate, or CTA). + * + * `createdAt` and `reply` are intentionally omitted — Publisher + * stamps both at write time so every record in a thread carries + * a fresh timestamp and a reply-ref to the already-written root. + * + * The `atmosphere_transform_bsky_post` filter fires **once per + * thread entry** for the thread strategies. Downstream consumers + * that assumed single-post semantics should branch on record + * shape (e.g. presence of `reply` is not yet set here, but the + * caller can check `$record['text']` against the CTA/permalink + * shape, or filter on `atmosphere_long_form_composition` + * upstream) to decide whether to transform every record. + * + * @param string $text Pre-composed post text. + * @return array Bsky post record (no createdAt, no reply). + */ + private function record_for_thread_entry( string $text ): array { + $record = array( + '$type' => 'app.bsky.feed.post', + 'text' => $text, + 'langs' => $this->get_langs(), + ); + + $facets = Facet::extract( $text ); + if ( ! empty( $facets ) ) { + $record['facets'] = $facets; + } + + /** This filter is documented in Post::transform() above. */ + return \apply_filters( 'atmosphere_transform_bsky_post', $record, $this->object ); + } + + /** + * Build the single link-card record (today's long-form output). + * + * Kept separate from `transform()` so `transform()` stays + * byte-compatible for legacy callers while `build_long_form_records()` + * can produce the same output when the composition filter + * resolves to `'link-card'` (the default) or an unknown value. + * + * @return array Bsky post record (no createdAt — Publisher stamps). + */ + private function record_for_link_card(): array { + $text = $this->build_text(); + $embed = $this->build_embed(); + + $record = array( + '$type' => 'app.bsky.feed.post', + 'text' => $text, + 'langs' => $this->get_langs(), + ); + + $facets = Facet::extract( $text ); + if ( ! empty( $facets ) ) { + $record['facets'] = $facets; + } + + if ( $embed ) { + $record['embed'] = $embed; + } + + $tags = $this->collect_tags( $this->object ); + if ( ! empty( $tags ) ) { + $record['tags'] = $tags; + } + + /** This filter is documented in Post::transform() above. */ + return \apply_filters( 'atmosphere_transform_bsky_post', $record, $this->object ); + } } From 5c9d20ae48a25d8c3d8e3befcdc7ef9bdfe80956 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Thu, 23 Apr 2026 20:47:31 -0500 Subject: [PATCH 03/22] Tests: cover build_long_form_records strategy branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds 16 unit tests under Atmosphere\Tests\Transformer\Test_Post: - 6 tests for `truncate_to_budget()` via reflection — sentence-preferred cut, trailing close-punctuation, word-boundary fallback, word-only when `prefer_sentence` is false, and hard-cap for a single long token. - 10 tests for `build_long_form_records()` — default (link-card) matches `transform()` output, `atmosphere_transform_bsky_post` fires per entry for threads, `truncate-link` branch, `teaser-thread` default 2-entry shape with sentence-cut hook and link-faceted CTA, word-boundary fallback when no sentence break exists, excerpt precedence, empty-body downgrade to link-card, 3-entry extension via `atmosphere_teaser_thread_posts`, `langs` consistency, per-record facet extraction, and unknown-strategy fallback. Also adds a small observability hook on the downgrade path: `atmosphere_long_form_strategy_downgraded` fires with the post, requested strategy, and effective strategy, so ops and the downgrade test can distinguish the fallback from an intentional `'link-card'` configuration. Linear: https://linear.app/a8c/issue/DOTCOM-16810 --- includes/transformer/class-post.php | 16 + .../tests/transformer/class-test-post.php | 438 ++++++++++++++++++ 2 files changed, 454 insertions(+) diff --git a/includes/transformer/class-post.php b/includes/transformer/class-post.php index c797be8..6fb13e5 100644 --- a/includes/transformer/class-post.php +++ b/includes/transformer/class-post.php @@ -356,6 +356,22 @@ public function build_long_form_records(): array { $strategy ) ); + + /** + * Fires when a long-form strategy is silently downgraded to + * `'link-card'` because the post has neither a usable excerpt + * nor enough body text to compose a thread hook from. + * + * Purpose is observability — the downgrade is not itself an + * error, but ops teams may want to distinguish a fallback + * from an intentional `'link-card'` configuration. + * + * @param \WP_Post $post The post being composed. + * @param string $requested The strategy the filter returned (e.g. 'teaser-thread'). + * @param string $effective The strategy actually used ('link-card'). + */ + \do_action( 'atmosphere_long_form_strategy_downgraded', $this->object, $strategy, 'link-card' ); + $strategy = 'link-card'; } diff --git a/tests/phpunit/tests/transformer/class-test-post.php b/tests/phpunit/tests/transformer/class-test-post.php index 0883830..4098d0b 100644 --- a/tests/phpunit/tests/transformer/class-test-post.php +++ b/tests/phpunit/tests/transformer/class-test-post.php @@ -24,9 +24,33 @@ class Test_Post extends WP_UnitTestCase { */ public function tear_down() { \remove_all_filters( 'atmosphere_is_short_form_post' ); + \remove_all_filters( 'atmosphere_long_form_composition' ); + \remove_all_filters( 'atmosphere_teaser_thread_posts' ); + \remove_all_filters( 'atmosphere_transform_bsky_post' ); + \remove_all_actions( 'atmosphere_long_form_strategy_downgraded' ); parent::tear_down(); } + /** + * Invoke `Post::truncate_to_budget()` via reflection. + * + * The helper is private because it's an implementation detail of + * composition; tests exercise it directly to lock in the + * sentence / word / hard-cap contract the hook builders depend on. + * + * @param string $text Input text. + * @param int $max Budget. + * @param bool $prefer_sentence Whether to prefer a sentence break. + * @return string + */ + private function truncate( string $text, int $max, bool $prefer_sentence = true ): string { + $post = self::factory()->post->create_and_get(); + $method = new \ReflectionMethod( Post::class, 'truncate_to_budget' ); + $method->setAccessible( true ); + + return $method->invoke( new Post( $post ), $text, $max, $prefer_sentence ); + } + /** * A titled post with no post format uses the long-form path: * title + excerpt + permalink as text, plus an external embed card. @@ -203,4 +227,418 @@ public function test_filter_receives_computed_default_and_post() { $this->assertFalse( $received_default, 'Default for titled-no-format post should be false (long-form).' ); $this->assertSame( $post->ID, $received_post_id, 'Filter should receive the post being transformed.' ); } + + /* + * ----------------------------------------------------------------- + * truncate_to_budget() — private helper covered via reflection. + * ----------------------------------------------------------------- + */ + + /** + * Text under budget returns unchanged. + */ + public function test_truncate_to_budget_returns_unchanged_when_under_budget() { + $this->assertSame( 'Hello world.', $this->truncate( 'Hello world.', 100 ) ); + } + + /** + * Prefers a sentence boundary inside the budget over a word boundary later. + */ + public function test_truncate_to_budget_prefers_sentence_when_enabled() { + $text = \str_repeat( 'Hi there. ', 35 ); // 350 chars, sentence every 10. + $cut = $this->truncate( $text, 280, true ); + $last = \substr( $cut, -1 ); + $this->assertLessThanOrEqual( 280, \mb_strlen( $cut ) ); + $this->assertSame( '.', $last, 'Sentence-preferred cut must end at sentence punctuation.' ); + // The text at the boundary is `"Hi there. " x N`, cut after the 28th period (byte 279). + $this->assertSame( 279, \strlen( $cut ) ); + } + + /** + * Cut includes optional trailing close-punctuation after the sentence stop. + */ + public function test_truncate_to_budget_allows_trailing_close_punctuation() { + // Clamp to 5 chars: `Hi!" ` — regex matches `!"` (close-quote allowed). Cut = `Hi!"`. + $cut = $this->truncate( 'Hi!" Then I left.', 5, true ); + $this->assertSame( 'Hi!"', $cut ); + } + + /** + * Falls back to the last word boundary when no sentence break is in range. + */ + public function test_truncate_to_budget_falls_back_to_word_boundary_when_no_sentence() { + $text = 'The quick brown fox jumps over the lazy dog'; + $cut = $this->truncate( $text, 20, true ); + // mb_substr 0,20 = "The quick brown fox ", word cut strips trailing space+token → "The quick brown fox". + $this->assertSame( 'The quick brown fox', $cut ); + } + + /** + * With prefer_sentence=false, ignores sentence breaks and uses word boundary. + */ + public function test_truncate_to_budget_word_boundary_only_when_prefer_sentence_false() { + // Sentence break at char 3 (`.`) would dominate if prefer_sentence were true. + $text = 'Hi. Then hello world goodbye.'; + $cut = $this->truncate( $text, 12, false ); + // Clamp "Hi. Then hel", word-cut strips " hel" → "Hi. Then". + $this->assertSame( 'Hi. Then', $cut ); + } + + /** + * Single token longer than budget: hard-cap with a trailing ellipsis. + */ + public function test_truncate_to_budget_hard_cap_for_single_long_word() { + $cut = $this->truncate( 'Supercalifragilisticexpialidocious', 10, true ); + $this->assertSame( 10, \mb_strlen( $cut ) ); + $this->assertSame( '…', \mb_substr( $cut, -1 ) ); + $this->assertNotSame( '', $cut ); + } + + /* + * ----------------------------------------------------------------- + * build_long_form_records() — strategy branches. + * ----------------------------------------------------------------- + */ + + /** + * No filter: long-form default is link-card. Single record, text and embed + * match today's transform() output byte-for-byte on the relevant fields. + * + * @covers ::build_long_form_records + */ + public function test_build_long_form_records_default_is_link_card() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'A Titled Post', + 'post_content' => 'Body.', + 'post_excerpt' => 'Teaser excerpt.', + ) + ); + + $transformer = new Post( $post ); + $records = $transformer->build_long_form_records(); + $oracle = $transformer->transform(); + + $this->assertCount( 1, $records ); + $this->assertSame( $oracle['text'], $records[0]['text'] ); + $this->assertArrayHasKey( 'embed', $records[0] ); + $this->assertSame( $oracle['embed'], $records[0]['embed'] ); + } + + /** + * The `atmosphere_transform_bsky_post` filter fires once per record + * in thread strategies — not once per WP post. + * + * @covers ::build_long_form_records + */ + public function test_build_long_form_records_applies_atmosphere_transform_bsky_post_per_entry() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'A Titled Post', + 'post_content' => 'Body sentence one. Body sentence two.', + ) + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + \add_filter( + 'atmosphere_transform_bsky_post', + static function ( $record ) { + $record['text'] .= ' __transformed__'; + return $record; + } + ); + + $records = ( new Post( $post ) )->build_long_form_records(); + + $this->assertCount( 2, $records ); + foreach ( $records as $record ) { + $this->assertStringEndsWith( ' __transformed__', $record['text'] ); + } + } + + /** + * Truncate-link branch: single record, no embed, text ends with permalink, + * and facets include a link covering the permalink. + * + * @covers ::build_long_form_records + */ + public function test_build_long_form_records_truncate_link_branch() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'Titled', + 'post_content' => \str_repeat( 'Some body content. ', 20 ), + ) + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'truncate-link' ); + + $records = ( new Post( $post ) )->build_long_form_records(); + + $this->assertCount( 1, $records ); + $this->assertArrayNotHasKey( 'embed', $records[0] ); + + $permalink = \get_permalink( $post ); + $this->assertStringEndsWith( "\n\n" . $permalink, $records[0]['text'] ); + + $has_link_facet = false; + foreach ( $records[0]['facets'] ?? array() as $facet ) { + foreach ( $facet['features'] as $feature ) { + if ( 'app.bsky.richtext.facet#link' === ( $feature['$type'] ?? '' ) + && ( $feature['uri'] ?? '' ) === $permalink + ) { + $has_link_facet = true; + } + } + } + $this->assertTrue( $has_link_facet, 'Permalink should be captured by a link facet.' ); + } + + /** + * Teaser-thread default: 2 entries, hook cut at sentence punctuation, + * CTA starts with `Continue reading: `. + * + * @covers ::build_long_form_records + */ + public function test_build_long_form_records_teaser_thread_default_two_entries() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'A Long Post', + // 35 sentences × 10 chars = 350 chars; body exceeds the 280 hook budget. + 'post_content' => \str_repeat( 'Hi there. ', 35 ), + ) + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + + $records = ( new Post( $post ) )->build_long_form_records(); + + $this->assertCount( 2, $records ); + + // Hook. + $hook = $records[0]['text']; + $this->assertLessThanOrEqual( 280, \mb_strlen( $hook ) ); + $this->assertContains( \substr( $hook, -1 ), array( '.', '!', '?' ), 'Hook should end at sentence punctuation.' ); + $this->assertStringNotContainsString( \get_permalink( $post ), $hook ); + $this->assertArrayNotHasKey( 'embed', $records[0] ); + + // CTA. + $cta = $records[1]['text']; + $this->assertMatchesRegularExpression( '~^Continue reading: https?://~', $cta ); + + $has_cta_link_facet = false; + foreach ( $records[1]['facets'] ?? array() as $facet ) { + foreach ( $facet['features'] as $feature ) { + if ( 'app.bsky.richtext.facet#link' === ( $feature['$type'] ?? '' ) ) { + $has_cta_link_facet = true; + } + } + } + $this->assertTrue( $has_cta_link_facet, 'CTA permalink should produce a link facet.' ); + } + + /** + * When no sentence boundary exists inside 280 chars the hook falls back + * to a word boundary — never ends mid-word. + * + * @covers ::build_long_form_records + */ + public function test_build_long_form_records_teaser_thread_hook_falls_back_to_word_boundary_when_no_sentence() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'Unpunctuated', + // 36 repetitions × 18 chars = 648 chars, no `.`/`!`/`?`. + 'post_content' => \str_repeat( 'abcdefgh ijklmnop ', 36 ), + ) + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + + $hook = ( new Post( $post ) )->build_long_form_records()[0]['text']; + + $this->assertLessThanOrEqual( 280, \mb_strlen( $hook ) ); + $this->assertDoesNotMatchRegularExpression( + '~\S$~', + $hook, + 'Hook should end at whitespace, not mid-word.' + ); + } + + /** + * Post excerpt, when set, takes precedence over body-derived hooks. + * + * @covers ::build_long_form_records + */ + public function test_build_long_form_records_teaser_thread_uses_excerpt_when_set() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'Titled', + 'post_content' => 'Body sentence one. Body sentence two.', + 'post_excerpt' => 'Custom-curated hook copy.', + ) + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + + $records = ( new Post( $post ) )->build_long_form_records(); + + $this->assertSame( 'Custom-curated hook copy.', $records[0]['text'] ); + } + + /** + * Empty body + empty excerpt: strategy silently degrades to link-card + * and fires the observability action so ops can distinguish fallback + * from intentional configuration. + * + * @covers ::build_long_form_records + */ + public function test_build_long_form_records_degrades_to_link_card_when_body_and_excerpt_empty() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'Almost Empty Post', + 'post_content' => 'Hi', // 2 chars — below the 10-char floor. + 'post_excerpt' => '', + ) + ); + + $events = array(); + \add_action( + 'atmosphere_long_form_strategy_downgraded', + function ( $downgrade_post, $requested, $effective ) use ( &$events ) { + $events[] = array( $downgrade_post->ID, $requested, $effective ); + }, + 10, + 3 + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + + $records = ( new Post( $post ) )->build_long_form_records(); + + $this->assertCount( 1, $records ); + $this->assertArrayHasKey( 'embed', $records[0] ); + $this->assertSame( 'app.bsky.embed.external', $records[0]['embed']['$type'] ); + + $this->assertCount( 1, $events, 'Downgrade action should fire exactly once.' ); + $this->assertSame( array( $post->ID, 'teaser-thread', 'link-card' ), $events[0] ); + } + + /** + * Downstream filters may extend the thread to 3 posts. + * + * @covers ::build_long_form_records + */ + public function test_build_long_form_records_teaser_thread_filter_extends_to_three() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'Titled', + 'post_content' => 'Body content.', + ) + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + \add_filter( + 'atmosphere_teaser_thread_posts', + fn() => array( 'Hook post', 'Key takeaway', 'Call to action link' ) + ); + + $records = ( new Post( $post ) )->build_long_form_records(); + + $this->assertCount( 3, $records ); + $this->assertSame( 'Hook post', $records[0]['text'] ); + $this->assertSame( 'Key takeaway', $records[1]['text'] ); + $this->assertSame( 'Call to action link', $records[2]['text'] ); + } + + /** + * Every record in a thread carries the same `langs` array. + * + * @covers ::build_long_form_records + */ + public function test_build_long_form_records_langs_consistent_across_thread() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'Titled', + 'post_content' => 'Body content with enough prose to form a hook.', + ) + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + + $records = ( new Post( $post ) )->build_long_form_records(); + + $this->assertGreaterThanOrEqual( 2, \count( $records ) ); + $root_langs = $records[0]['langs']; + $this->assertNotEmpty( $root_langs ); + foreach ( $records as $record ) { + $this->assertSame( $root_langs, $record['langs'] ); + } + } + + /** + * Facets are extracted against each record's own text — tag on the hook, + * link on the CTA. + * + * @covers ::build_long_form_records + */ + public function test_build_long_form_records_facets_extracted_per_entry() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'Titled', + 'post_content' => 'Read about #testing sensors in this detailed write-up on instrumentation.', + ) + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + + $records = ( new Post( $post ) )->build_long_form_records(); + + $hook_has_tag = false; + foreach ( $records[0]['facets'] ?? array() as $facet ) { + foreach ( $facet['features'] as $feature ) { + if ( 'app.bsky.richtext.facet#tag' === ( $feature['$type'] ?? '' ) + && 'testing' === ( $feature['tag'] ?? '' ) + ) { + $hook_has_tag = true; + } + } + } + $this->assertTrue( $hook_has_tag, 'Hook text should have a #testing tag facet.' ); + + $cta_has_link = false; + foreach ( $records[1]['facets'] ?? array() as $facet ) { + foreach ( $facet['features'] as $feature ) { + if ( 'app.bsky.richtext.facet#link' === ( $feature['$type'] ?? '' ) ) { + $cta_has_link = true; + } + } + } + $this->assertTrue( $cta_has_link, 'CTA text should have a link facet.' ); + } + + /** + * An unknown strategy value silently falls back to link-card. + * + * @covers ::build_long_form_records + */ + public function test_build_long_form_records_unknown_strategy_falls_back_to_link_card() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'Titled', + 'post_content' => 'Body.', + 'post_excerpt' => 'Teaser excerpt.', + ) + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'nonsense' ); + + $transformer = new Post( $post ); + $records = $transformer->build_long_form_records(); + + \remove_all_filters( 'atmosphere_long_form_composition' ); + $oracle = $transformer->transform(); + + $this->assertCount( 1, $records ); + $this->assertSame( $oracle['text'], $records[0]['text'] ); + $this->assertSame( $oracle['embed'], $records[0]['embed'] ); + } } From 6581a0f82047f473b6c2998d4f455829b583b15a Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Thu, 23 Apr 2026 21:15:56 -0500 Subject: [PATCH 04/22] Publisher: thread-aware publish/update/delete with partial-meta writes and rollback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reshapes Publisher around the idea that one WordPress post may map to N Bluesky feed posts (a thread) plus exactly one site.standard.document. **publish()** - Branches on `Post::is_short_form_post()`. Short-form stays on today's single-record path via `transform()`. - Long-form calls the new `build_long_form_records()`. 1-entry output (link-card / truncate-link) goes through `publish_single()`; N-entry output goes through `publish_thread()`. - `publish_single()` preserves today's atomic applyWrites (bsky + doc) and additionally populates `META_THREAD_RECORDS` as a 1-entry array so the new key is always present after a successful publish. - `publish_thread()` writes root + doc atomically, persists partial meta immediately, then writes each reply on its own `applyWrites`. Reply refs (`reply.root` / `reply.parent`) are filled from the already-persisted thread records since CIDs only arrive via the PDS response to the prior create — we can't assemble one atomic batch for the whole thread. **Rollback** On a mid-thread failure, `rollback_thread()` issues compensating deletes tail-first + a doc delete, clears all meta, and returns the original error. If rollback itself fails, the returned WP_Error wraps both errors and carries `partial_records` so an operator can clean up by hand. **update()** - Reads `META_THREAD_RECORDS` with legacy fallback to single-record meta. - Single stored + single new composition → in-place applyWrites#update on bsky + doc, matching today's behavior. - Any other shape (single ↔ thread, thread → thread with different lengths) → delete all existing records atomically, then republish. Documented side effect: followers see the thread updates as a fresh publish (new `createdAt`); external replies to the original thread are orphaned. **delete()** - Reads `META_THREAD_RECORDS` with legacy fallback; batches one applyWrites with N bsky deletes + 1 doc delete. Clears every meta key on success; leaves meta intact on failure so a retry can complete. **Meta shape** - New `META_THREAD_RECORDS` is the canonical list of ordered `{ uri, cid, tid }` triples. - Legacy `META_URI` / `META_TID` / `META_CID` continue to mirror the root record for backwards compatibility and because `Document::transform()` still reads them to compose the bskyPostRef. - `clear_all_record_meta()` helper keeps the six keys in sync on delete / rollback paths. **Not touched** - `delete_by_tids()` signature preserved for queued-cron backwards compatibility. `on_before_delete` in class-atmosphere.php still captures a single TID; force-deleting a thread-published post leaves thread tails on the PDS. Follow-up task will update the on_before_delete / cron hook to capture the full thread. - Short-form behavior is unchanged — `transform()`'s `createdAt` (from `post_date_gmt`) still flows through `publish_single()`. Linear: https://linear.app/a8c/issue/DOTCOM-16810 --- includes/class-publisher.php | 624 +++++++++++++++++++++++++++++++---- 1 file changed, 561 insertions(+), 63 deletions(-) diff --git a/includes/class-publisher.php b/includes/class-publisher.php index ffd48a7..6820a1a 100644 --- a/includes/class-publisher.php +++ b/includes/class-publisher.php @@ -2,9 +2,22 @@ /** * Orchestrates publishing WordPress posts to the AT Protocol. * - * Every publish creates both an app.bsky.feed.post and a - * site.standard.document atomically via a single applyWrites call. - * Updates and deletions follow the same pattern. + * A WordPress post corresponds to: + * - One or more `app.bsky.feed.post` records (a thread, for the + * long-form `teaser-thread` strategy). + * - Exactly one `site.standard.document` record. + * + * Single-record publishes (short-form, link-card, truncate-link) use + * one atomic `applyWrites` call with the bsky post + doc. Threads + * write the root + doc atomically, then each reply as its own + * `applyWrites` call so reply refs can carry the parent's CID (the + * PDS only returns CIDs in the response, so we can't assemble a + * single atomic batch for the full thread). + * + * Thread publishes persist partial meta after each successful write, + * so an interrupted thread can be surfaced for retry. A mid-thread + * failure issues compensating `applyWrites#delete` calls in reverse + * order to roll back to a "nothing published" state. * * @package Atmosphere */ @@ -16,6 +29,7 @@ use Atmosphere\Transformer\Document; use Atmosphere\Transformer\Post; use Atmosphere\Transformer\Publication; +use Atmosphere\Transformer\TID; /** * Publisher class. @@ -23,35 +37,78 @@ class Publisher { /** - * Publish a post to AT Protocol (both record types). + * Publish a post to AT Protocol (bsky record(s) + document). * * @param \WP_Post $post WordPress post. - * @return array|\WP_Error applyWrites response or error. + * @return array|\WP_Error applyWrites response(s) or error. */ public static function publish( \WP_Post $post ): array|\WP_Error { $bsky_transformer = new Post( $post ); $doc_transformer = new Document( $post ); // Ensure TIDs exist (generates if needed). - $bsky_rkey = $bsky_transformer->get_rkey(); - $doc_rkey = $doc_transformer->get_rkey(); + $bsky_transformer->get_rkey(); + $doc_transformer->get_rkey(); + + if ( $bsky_transformer->is_short_form_post() ) { + // Short-form path: single record via today's transform(). + return self::publish_single( + $post, + $bsky_transformer->transform(), + $bsky_transformer, + $doc_transformer + ); + } + + $records = $bsky_transformer->build_long_form_records(); + + if ( 1 === \count( $records ) ) { + return self::publish_single( $post, $records[0], $bsky_transformer, $doc_transformer ); + } - // Build records. - $bsky_record = $bsky_transformer->transform(); - $doc_record = $doc_transformer->transform(); + return self::publish_thread( $post, $records, $bsky_transformer, $doc_transformer ); + } + + /** + * Write a single bsky post + document atomically. + * + * Used for short-form (via `transform()`'s output) and for the + * `link-card` / `truncate-link` long-form strategies (via + * `build_long_form_records()`'s single-element output). + * + * `createdAt` defaults to `wp_date( 'c' )` when the record doesn't + * already carry one. `transform()` sets `createdAt` from the post's + * `post_date_gmt` (preserving today's short-form behavior); + * `build_long_form_records()` omits it on purpose. + * + * @param \WP_Post $post WordPress post. + * @param array $bsky_record Pre-composed bsky post record. + * @param Post $bsky_transformer Bsky transformer instance. + * @param Document $doc_transformer Document transformer instance. + * @return array|\WP_Error + */ + private static function publish_single( + \WP_Post $post, + array $bsky_record, + Post $bsky_transformer, + Document $doc_transformer + ): array|\WP_Error { + if ( empty( $bsky_record['createdAt'] ) ) { + $bsky_record['createdAt'] = \wp_date( 'c' ); + } $writes = array( array( '$type' => 'com.atproto.repo.applyWrites#create', 'collection' => 'app.bsky.feed.post', - 'rkey' => $bsky_rkey, + 'rkey' => $bsky_transformer->get_rkey(), 'value' => $bsky_record, ), array( '$type' => 'com.atproto.repo.applyWrites#create', 'collection' => 'site.standard.document', - 'rkey' => $doc_rkey, - 'value' => $doc_record, + 'rkey' => $doc_transformer->get_rkey(), + 'value' => $doc_transformer->transform(), ), ); @@ -61,46 +118,329 @@ public static function publish( \WP_Post $post ): array|\WP_Error { return $result; } - // Store URIs and CIDs from the response. self::store_results( $post->ID, $result, $bsky_transformer, $doc_transformer ); - - // Follow-up: update document with bsky post reference (now that we have the CID). + self::mirror_thread_records_meta( + $post->ID, + array( + self::build_triple_from_result( + $result, + 0, + $bsky_transformer->get_uri(), + $bsky_transformer->get_rkey() + ), + ) + ); self::update_document_bsky_ref( $post, $doc_transformer ); return $result; } /** - * Update both records for an existing post. + * Sequential-writes-with-rollback for thread-strategy publishes. + * + * Step 1 writes root + doc atomically. Partial meta is persisted + * immediately so crash recovery has a pointer to the root record. + * Step 2..N writes each reply on its own, with reply refs derived + * from the already-persisted thread records. Meta is updated after + * each successful create so an interrupted thread is visible. + * + * On any reply failure, compensating deletes run in reverse order + * (tail-first), all meta is cleared, and the original failing + * WP_Error is returned. If rollback also fails, the return wraps + * both errors and includes the partial thread state. + * + * @param \WP_Post $post WordPress post. + * @param array[] $records Records from build_long_form_records(). + * @param Post $bsky_transformer Bsky transformer instance. + * @param Document $doc_transformer Document transformer instance. + * @return array|\WP_Error + */ + private static function publish_thread( + \WP_Post $post, + array $records, + Post $bsky_transformer, + Document $doc_transformer + ): array|\WP_Error { + $root_record = $records[0]; + $root_record['createdAt'] = \wp_date( 'c' ); + $root_rkey = $bsky_transformer->get_rkey(); + + $root_result = API::apply_writes( + array( + array( + '$type' => 'com.atproto.repo.applyWrites#create', + 'collection' => 'app.bsky.feed.post', + 'rkey' => $root_rkey, + 'value' => $root_record, + ), + array( + '$type' => 'com.atproto.repo.applyWrites#create', + 'collection' => 'site.standard.document', + 'rkey' => $doc_transformer->get_rkey(), + 'value' => $doc_transformer->transform(), + ), + ) + ); + + if ( \is_wp_error( $root_result ) ) { + return $root_result; + } + + $root_triple = self::build_triple_from_result( + $root_result, + 0, + $bsky_transformer->get_uri(), + $root_rkey + ); + + if ( empty( $root_triple['cid'] ) ) { + return new \WP_Error( + 'atmosphere_missing_cid', + \__( 'Root post created but PDS response lacked a CID; cannot chain thread replies.', 'atmosphere' ) + ); + } + + $thread_records = array( $root_triple ); + + self::store_results( $post->ID, $root_result, $bsky_transformer, $doc_transformer ); + self::mirror_thread_records_meta( $post->ID, $thread_records ); + self::update_document_bsky_ref( $post, $doc_transformer ); + + $aggregated_results = $root_result['results'] ?? array(); + + $count = \count( $records ); + for ( $i = 1; $i < $count; $i++ ) { + $reply_rkey = TID::generate(); + $reply_record = $records[ $i ]; + + $reply_record['createdAt'] = \wp_date( 'c' ); + $reply_record['reply'] = array( + 'root' => array( + 'uri' => $thread_records[0]['uri'], + 'cid' => $thread_records[0]['cid'], + ), + 'parent' => array( + 'uri' => $thread_records[ $i - 1 ]['uri'], + 'cid' => $thread_records[ $i - 1 ]['cid'], + ), + ); + + $reply_result = API::apply_writes( + array( + array( + '$type' => 'com.atproto.repo.applyWrites#create', + 'collection' => 'app.bsky.feed.post', + 'rkey' => $reply_rkey, + 'value' => $reply_record, + ), + ) + ); + + if ( \is_wp_error( $reply_result ) ) { + return self::rollback_thread( $post, $thread_records, $doc_transformer, $reply_result ); + } + + $reply_triple = self::build_triple_from_result( + $reply_result, + 0, + build_at_uri( get_did(), 'app.bsky.feed.post', $reply_rkey ), + $reply_rkey + ); + + if ( empty( $reply_triple['cid'] ) ) { + return self::rollback_thread( + $post, + $thread_records, + $doc_transformer, + new \WP_Error( + 'atmosphere_missing_cid', + \__( 'Reply created but PDS response lacked a CID; rolling back thread.', 'atmosphere' ) + ) + ); + } + + $thread_records[] = $reply_triple; + self::mirror_thread_records_meta( $post->ID, $thread_records ); + + $aggregated_results = \array_merge( $aggregated_results, $reply_result['results'] ?? array() ); + } + + return array( 'results' => $aggregated_results ); + } + + /** + * Delete every already-written record in a partially-published thread. + * + * Posts are deleted tail-first so the root survives until last — + * replies pointing at the (still-live) root remain valid until their + * own delete lands. The document record is deleted last. + * + * Meta is always cleared — a failed rollback leaves orphans on the + * PDS but the local state stays consistent with "no published thread." + * When rollback itself fails, the returned `WP_Error` wraps both + * errors and carries `partial_records` so an operator retrying can + * clean up by hand. + * + * @param \WP_Post $post WordPress post. + * @param array[] $thread_records Already-written thread records (uri/cid/tid each). + * @param Document $doc_transformer Document transformer instance. + * @param \WP_Error $original_error The failure that triggered rollback. + * @return \WP_Error + */ + private static function rollback_thread( + \WP_Post $post, + array $thread_records, + Document $doc_transformer, + \WP_Error $original_error + ): \WP_Error { + $rollback_writes = array(); + + for ( $i = \count( $thread_records ) - 1; $i >= 0; $i-- ) { + $rollback_writes[] = array( + '$type' => 'com.atproto.repo.applyWrites#delete', + 'collection' => 'app.bsky.feed.post', + 'rkey' => $thread_records[ $i ]['tid'], + ); + } + $rollback_writes[] = array( + '$type' => 'com.atproto.repo.applyWrites#delete', + 'collection' => 'site.standard.document', + 'rkey' => $doc_transformer->get_rkey(), + ); + + $rollback_result = API::apply_writes( $rollback_writes ); + + self::clear_all_record_meta( $post->ID ); + + if ( \is_wp_error( $rollback_result ) ) { + // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log + \error_log( + \sprintf( + '[atmosphere] thread rollback failed for post %d: %s', + $post->ID, + $rollback_result->get_error_message() + ) + ); + + return new \WP_Error( + 'atmosphere_thread_rollback_failed', + \sprintf( + /* translators: %s: the original error message. */ + \__( 'Thread publish failed and rollback also failed: %s', 'atmosphere' ), + $original_error->get_error_message() + ), + array( + 'original_error' => $original_error, + 'rollback_error' => $rollback_result, + 'partial_records' => $thread_records, + ) + ); + } + + return $original_error; + } + + /** + * Update the bsky + doc records for an existing post. + * + * - Single record stored + single record composed: in-place update + * via `applyWrites#update` on both. + * - Any other shape (strategy change, thread ↔ single, or + * thread ↔ thread): delete every existing record and republish + * with the fresh composition. Thread updates therefore arrive to + * followers as a fresh publish (new `createdAt`), and any replies + * other Bluesky users posted become orphaned. * * @param \WP_Post $post WordPress post. * @return array|\WP_Error */ public static function update( \WP_Post $post ): array|\WP_Error { - $bsky_uri = \get_post_meta( $post->ID, Post::META_URI, true ); - $doc_uri = \get_post_meta( $post->ID, Document::META_URI, true ); + $stored = self::stored_thread_records( $post->ID ); - if ( ! $bsky_uri || ! $doc_uri ) { - // Not yet published — do a fresh publish instead. + if ( empty( $stored ) ) { + // Never successfully published — do a fresh publish. return self::publish( $post ); } - $bsky_tid = \get_post_meta( $post->ID, Post::META_TID, true ); - $doc_tid = \get_post_meta( $post->ID, Document::META_TID, true ); + foreach ( $stored as $entry ) { + if ( empty( $entry['tid'] ) ) { + return new \WP_Error( + 'atmosphere_missing_tid', + \__( 'Record URIs exist but TIDs are missing.', 'atmosphere' ) + ); + } + } + + $doc_uri = \get_post_meta( $post->ID, Document::META_URI, true ); + $doc_tid = \get_post_meta( $post->ID, Document::META_TID, true ); - if ( ! $bsky_tid || ! $doc_tid ) { - return new \WP_Error( 'atmosphere_missing_tid', \__( 'Record URIs exist but TIDs are missing.', 'atmosphere' ) ); + if ( ! $doc_uri ) { + // Partial state: bsky exists but doc never did. Safer to + // republish than to patch around missing doc. + return self::publish( $post ); + } + + if ( ! $doc_tid ) { + return new \WP_Error( + 'atmosphere_missing_tid', + \__( 'Record URIs exist but TIDs are missing.', 'atmosphere' ) + ); } $bsky_transformer = new Post( $post ); $doc_transformer = new Document( $post ); + $new_records = $bsky_transformer->is_short_form_post() + ? array( $bsky_transformer->transform() ) + : $bsky_transformer->build_long_form_records(); + + // In-place update: single stored + single new composition. + if ( 1 === \count( $stored ) && 1 === \count( $new_records ) ) { + return self::update_single( + $post, + $stored[0], + $new_records[0], + $bsky_transformer, + $doc_transformer, + $doc_tid + ); + } + + // Strategy change or thread shape — delete everything and republish. + return self::rewrite_thread( $post, $stored, $doc_tid ); + } + + /** + * In-place `applyWrites#update` for both bsky + doc, mirroring today's + * update path. Extended only to refresh `META_THREAD_RECORDS` with the + * post-update CID. + * + * @param \WP_Post $post WordPress post. + * @param array $stored_root The single stored {uri, cid, tid} triple. + * @param array $new_bsky_record Freshly composed bsky record. + * @param Post $bsky_transformer Bsky transformer instance. + * @param Document $doc_transformer Document transformer instance. + * @param string $doc_tid Document record TID. + * @return array|\WP_Error + */ + private static function update_single( + \WP_Post $post, + array $stored_root, + array $new_bsky_record, + Post $bsky_transformer, + Document $doc_transformer, + string $doc_tid + ): array|\WP_Error { + if ( empty( $new_bsky_record['createdAt'] ) ) { + $new_bsky_record['createdAt'] = \wp_date( 'c' ); + } + $writes = array( array( '$type' => 'com.atproto.repo.applyWrites#update', 'collection' => 'app.bsky.feed.post', - 'rkey' => $bsky_tid, - 'value' => $bsky_transformer->transform(), + 'rkey' => $stored_root['tid'], + 'value' => $new_bsky_record, ), array( '$type' => 'com.atproto.repo.applyWrites#update', @@ -117,37 +457,93 @@ public static function update( \WP_Post $post ): array|\WP_Error { } self::store_results( $post->ID, $result, $bsky_transformer, $doc_transformer ); - - // Update document with bsky post reference (CID may have changed). + self::mirror_thread_records_meta( + $post->ID, + array( + self::build_triple_from_result( + $result, + 0, + $stored_root['uri'], + $stored_root['tid'] + ), + ) + ); self::update_document_bsky_ref( $post, $doc_transformer ); return $result; } /** - * Delete both records for a post. + * Delete every stored bsky record + the doc atomically, then publish + * fresh. Used when the composition strategy changes (single ↔ thread) + * or when a thread updates to a thread with a different record count. + * + * The local meta is cleared between delete and publish so `publish()` + * sees a clean slate. + * + * @param \WP_Post $post WordPress post. + * @param array[] $stored Stored thread records (may be 1-entry). + * @param string $doc_tid Document record TID. + * @return array|\WP_Error + */ + private static function rewrite_thread( \WP_Post $post, array $stored, string $doc_tid ): array|\WP_Error { + $delete_writes = array(); + foreach ( $stored as $record ) { + $delete_writes[] = array( + '$type' => 'com.atproto.repo.applyWrites#delete', + 'collection' => 'app.bsky.feed.post', + 'rkey' => $record['tid'], + ); + } + $delete_writes[] = array( + '$type' => 'com.atproto.repo.applyWrites#delete', + 'collection' => 'site.standard.document', + 'rkey' => $doc_tid, + ); + + $delete_result = API::apply_writes( $delete_writes ); + + if ( \is_wp_error( $delete_result ) ) { + return $delete_result; + } + + self::clear_all_record_meta( $post->ID ); + + return self::publish( $post ); + } + + /** + * Delete every bsky record + the doc for a post. + * + * Handles thread posts (reads `META_THREAD_RECORDS`) and legacy + * single-record posts (falls back to the mirrored `META_URI` / + * `META_TID` / `META_CID` keys). * * @param \WP_Post $post WordPress post. * @return array|\WP_Error */ public static function delete( \WP_Post $post ): array|\WP_Error { - $bsky_tid = \get_post_meta( $post->ID, Post::META_TID, true ); - $doc_tid = \get_post_meta( $post->ID, Document::META_TID, true ); + $stored = self::stored_thread_records( $post->ID ); + $doc_tid = \get_post_meta( $post->ID, Document::META_TID, true ); - if ( ! $bsky_tid && ! $doc_tid ) { - return new \WP_Error( 'atmosphere_not_published', \__( 'Post has no AT Protocol records.', 'atmosphere' ) ); + if ( empty( $stored ) && ! $doc_tid ) { + return new \WP_Error( + 'atmosphere_not_published', + \__( 'Post has no AT Protocol records.', 'atmosphere' ) + ); } $writes = array(); - - if ( $bsky_tid ) { + foreach ( $stored as $record ) { + if ( empty( $record['tid'] ) ) { + continue; + } $writes[] = array( '$type' => 'com.atproto.repo.applyWrites#delete', 'collection' => 'app.bsky.feed.post', - 'rkey' => $bsky_tid, + 'rkey' => $record['tid'], ); } - if ( $doc_tid ) { $writes[] = array( '$type' => 'com.atproto.repo.applyWrites#delete', @@ -156,27 +552,34 @@ public static function delete( \WP_Post $post ): array|\WP_Error { ); } + if ( empty( $writes ) ) { + return new \WP_Error( + 'atmosphere_not_published', + \__( 'Post has no AT Protocol records.', 'atmosphere' ) + ); + } + $result = API::apply_writes( $writes ); if ( \is_wp_error( $result ) ) { + // Leave meta intact so a retry can complete. return $result; } - // Clean up post meta. - \delete_post_meta( $post->ID, Post::META_TID ); - \delete_post_meta( $post->ID, Post::META_URI ); - \delete_post_meta( $post->ID, Post::META_CID ); - \delete_post_meta( $post->ID, Document::META_TID ); - \delete_post_meta( $post->ID, Document::META_URI ); - \delete_post_meta( $post->ID, Document::META_CID ); + self::clear_all_record_meta( $post->ID ); return $result; } /** - * Delete AT Protocol records by TID, without requiring the post to exist. + * Delete AT Protocol records by TID without requiring the post to exist. * - * Used when a post is permanently deleted and post meta is no longer available. + * Used when a post is permanently deleted and its meta is no longer + * accessible to `delete()`. Kept at the single-TID signature for + * backwards compatibility with queued cron events; force-deletion of + * thread-strategy posts is handled by the caller by reading + * META_THREAD_RECORDS pre-deletion and scheduling a separate cron per + * thread record. * * @param string $bsky_tid Bluesky post TID (may be empty). * @param string $doc_tid Document TID (may be empty). @@ -205,13 +608,7 @@ public static function delete_by_tids( string $bsky_tid, string $doc_tid ): arra ); } - $result = API::apply_writes( $writes ); - - if ( \is_wp_error( $result ) ) { - return $result; - } - - return $result; + return API::apply_writes( $writes ); } /** @@ -225,7 +622,6 @@ public static function sync_publication(): array|\WP_Error { $existing_uri = \get_option( Publication::OPTION_URI ); if ( $existing_uri ) { - // Update existing. $result = API::post( '/xrpc/com.atproto.repo.putRecord', array( @@ -236,7 +632,6 @@ public static function sync_publication(): array|\WP_Error { ) ); } else { - // Create new. $result = API::post( '/xrpc/com.atproto.repo.createRecord', array( @@ -259,7 +654,13 @@ public static function sync_publication(): array|\WP_Error { } /** - * Extract URIs/CIDs from applyWrites response and store in post meta. + * Extract URIs/CIDs from applyWrites response and mirror into the + * legacy single-record meta keys (META_URI / META_TID / META_CID + * on Post and Document). + * + * Called for the root + doc write in every publish flow. Thread + * reply writes don't go through this helper — they're captured in + * `META_THREAD_RECORDS` by `publish_thread()` directly. * * @param int $post_id Post ID. * @param array $result applyWrites response. @@ -274,7 +675,6 @@ private static function store_results( int $post_id, array $result, Post $bsky_t $cid = $item['cid'] ?? ''; if ( 0 === $i ) { - // First write = bsky post. if ( $uri ) { \update_post_meta( $post_id, Post::META_URI, $uri ); } else { @@ -284,7 +684,6 @@ private static function store_results( int $post_id, array $result, Post $bsky_t \update_post_meta( $post_id, Post::META_CID, $cid ); } } elseif ( 1 === $i ) { - // Second write = document. if ( $uri ) { \update_post_meta( $post_id, Document::META_URI, $uri ); } else { @@ -298,10 +697,13 @@ private static function store_results( int $post_id, array $result, Post $bsky_t } /** - * Update the document record with the bsky post strong reference. + * Update the document record with the bsky root strong reference. * - * After the initial applyWrites, we now know the bsky post's CID, - * so we update the document to include the bskyPostRef field. + * After the initial applyWrites, the bsky root's CID is known, so + * we re-transform the document (which picks up the ref via its + * own read of `Post::META_URI` / `META_CID`) and persist via + * `putRecord`. Called once per publish — the doc always references + * the thread root, regardless of thread length. * * @param \WP_Post $post WordPress post. * @param Document $doc_transformer Document transformer. @@ -314,7 +716,6 @@ private static function update_document_bsky_ref( \WP_Post $post, Document $doc_ return; } - // Re-transform the document (now includes the bskyPostRef). $updated_doc = new Document( $post ); $record = $updated_doc->transform(); @@ -328,4 +729,101 @@ private static function update_document_bsky_ref( \WP_Post $post, Document $doc_ ) ); } + + /** + * Read the ordered thread records for a post. + * + * Prefers `Post::META_THREAD_RECORDS`. Falls back to legacy single-record + * meta so posts published before this key existed still delete/update + * correctly. + * + * @param int $post_id Post ID. + * @return array[] Array of { uri, cid, tid } triples, possibly empty. + */ + private static function stored_thread_records( int $post_id ): array { + $stored = \get_post_meta( $post_id, Post::META_THREAD_RECORDS, true ); + if ( \is_array( $stored ) && ! empty( $stored ) ) { + return $stored; + } + + $uri = \get_post_meta( $post_id, Post::META_URI, true ); + $tid = \get_post_meta( $post_id, Post::META_TID, true ); + $cid = \get_post_meta( $post_id, Post::META_CID, true ); + + if ( ! $uri && ! $tid ) { + return array(); + } + + return array( + array( + 'uri' => (string) $uri, + 'cid' => (string) $cid, + 'tid' => (string) $tid, + ), + ); + } + + /** + * Persist the thread-records meta and keep the root-mirrored single-record + * meta in sync with it. + * + * @param int $post_id Post ID. + * @param array[] $thread_records Ordered thread records. + */ + private static function mirror_thread_records_meta( int $post_id, array $thread_records ): void { + \update_post_meta( $post_id, Post::META_THREAD_RECORDS, $thread_records ); + + if ( empty( $thread_records ) ) { + return; + } + + $root = $thread_records[0]; + if ( ! empty( $root['uri'] ) ) { + \update_post_meta( $post_id, Post::META_URI, $root['uri'] ); + } + if ( ! empty( $root['tid'] ) ) { + \update_post_meta( $post_id, Post::META_TID, $root['tid'] ); + } + if ( ! empty( $root['cid'] ) ) { + \update_post_meta( $post_id, Post::META_CID, $root['cid'] ); + } + } + + /** + * Build a single { uri, cid, tid } triple from an `applyWrites` result + * entry, with sensible fallbacks when the PDS omits the URI (falling + * back to the transformer-computed URI) or when the caller needs a + * specific rkey (`Thread replies generate their own rkey so we can't + * derive it from `$item['uri']` reliably — we pass it in). + * + * @param array $result applyWrites response. + * @param int $index Zero-based index into `$result['results']`. + * @param string $fallback_uri AT-URI to use if the response omits one. + * @param string $tid Known rkey (generated client-side). + * @return array{ uri: string, cid: string, tid: string } + */ + private static function build_triple_from_result( array $result, int $index, string $fallback_uri, string $tid ): array { + $entry = $result['results'][ $index ] ?? array(); + + return array( + 'uri' => (string) ( $entry['uri'] ?? $fallback_uri ), + 'cid' => (string) ( $entry['cid'] ?? '' ), + 'tid' => $tid, + ); + } + + /** + * Clear every post-meta key tied to AT Protocol records for the post. + * + * @param int $post_id Post ID. + */ + private static function clear_all_record_meta( int $post_id ): void { + \delete_post_meta( $post_id, Post::META_THREAD_RECORDS ); + \delete_post_meta( $post_id, Post::META_URI ); + \delete_post_meta( $post_id, Post::META_TID ); + \delete_post_meta( $post_id, Post::META_CID ); + \delete_post_meta( $post_id, Document::META_URI ); + \delete_post_meta( $post_id, Document::META_TID ); + \delete_post_meta( $post_id, Document::META_CID ); + } } From dd2cc5323f2c3ad7ef7442fe8ad925bcb03f6c83 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Thu, 23 Apr 2026 21:20:40 -0500 Subject: [PATCH 05/22] Tests: Publisher thread writes, rollback, legacy fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds 10 unit tests covering the new long-form / thread flows, using the `atmosphere_pre_apply_writes` filter as the test seam (DPoP-dependent `pre_http_request` mocking can't reach the wp_remote_request layer in the test environment's auth setup). Publish path: - link-card writes one atomic applyWrites with 2 creates and mirrors the root into `META_THREAD_RECORDS` as a 1-entry array. - teaser-thread writes root + doc atomically, then the CTA reply as a second applyWrites with `reply.root` / `reply.parent` pointing at the root's {uri, cid}. - partial meta is persisted between the root write and the reply write — asserted via a meta snapshot taken inside the capture filter when the reply call fires. - happy-path final meta is an ordered 2-entry `META_THREAD_RECORDS` array, with legacy single-value keys mirroring the root. Rollback path: - second-write failure triggers compensating deletes (root bsky + doc), clears every meta key, and returns the original WP_Error. - when the rollback applyWrites itself also fails, the returned `WP_Error` has code `atmosphere_thread_rollback_failed` and data containing `original_error`, `rollback_error`, and `partial_records`. Update / delete paths: - single stored + single new composition uses in-place `applyWrites#update` on both bsky and doc (one applyWrites call), matching today's link-card update behavior. - strategy change (stored single → composed thread) issues a delete of the old record + doc, then publishes fresh as a thread. - thread delete batches every bsky delete + the doc delete into one atomic applyWrites and clears all meta. - legacy single-record meta (no `META_THREAD_RECORDS`) still deletes correctly via the fallback in `stored_thread_records()`. Linear: https://linear.app/a8c/issue/DOTCOM-16810 --- tests/phpunit/tests/class-test-publisher.php | 542 +++++++++++++++++++ 1 file changed, 542 insertions(+) diff --git a/tests/phpunit/tests/class-test-publisher.php b/tests/phpunit/tests/class-test-publisher.php index 0bafc8c..d390ecb 100644 --- a/tests/phpunit/tests/class-test-publisher.php +++ b/tests/phpunit/tests/class-test-publisher.php @@ -49,9 +49,109 @@ public function tear_down(): void { \delete_option( 'atmosphere_did' ); \delete_option( 'atmosphere_publication_tid' ); + \remove_all_filters( 'atmosphere_pre_apply_writes' ); + \remove_all_filters( 'atmosphere_long_form_composition' ); + \remove_all_filters( 'atmosphere_teaser_thread_posts' ); + \remove_all_filters( 'atmosphere_transform_bsky_post' ); + \remove_all_filters( 'atmosphere_is_short_form_post' ); + parent::tear_down(); } + /** + * Synthesize a plausible applyWrites response for a batch of writes. + * + * One result per write, with a stable URI + CID derived from the + * write's collection + rkey. Delete writes produce empty result + * entries so Publisher's `store_results()` treats them as no-ops. + * + * @param array $writes Write batch. + * @return array applyWrites response shape. + */ + private function mock_response( array $writes ): array { + $results = array(); + foreach ( $writes as $write ) { + $type = $write['$type'] ?? ''; + if ( 'com.atproto.repo.applyWrites#delete' === $type ) { + $results[] = array(); + continue; + } + + $collection = $write['collection'] ?? 'app.bsky.feed.post'; + $rkey = $write['rkey'] ?? ''; + $cid_seed = \md5( \wp_json_encode( $write['value'] ?? array() ) ); + + $results[] = array( + 'uri' => "at://did:plc:test123/{$collection}/{$rkey}", + 'cid' => 'bafyreib' . \substr( $cid_seed, 0, 20 ), + ); + } + + return array( 'results' => $results ); + } + + /** + * Register the `atmosphere_pre_apply_writes` capture filter. + * + * Every call is appended to `$this->captured_calls` with the write + * batch, the synthesized response, and a snapshot of + * `META_THREAD_RECORDS` at call time (useful for asserting partial + * meta between thread writes). Tests can flip individual calls to + * a `WP_Error` by pushing entries to `$this->fail_call_indexes` + * before invoking Publisher — any call whose 1-based index is in + * that array returns the associated error instead of a success + * response. + * + * @param int $post_id Post being published; used to snapshot meta. + */ + private function register_capture( int $post_id ): void { + $this->captured_calls = array(); + $this->fail_call_indexes = $this->fail_call_indexes ?? array(); + + \add_filter( + 'atmosphere_pre_apply_writes', + function ( $short_circuit, array $writes ) use ( $post_id ) { + $call_number = \count( $this->captured_calls ) + 1; + + $meta_snapshot = \get_post_meta( $post_id, Post::META_THREAD_RECORDS, true ); + + if ( isset( $this->fail_call_indexes[ $call_number ] ) ) { + $response = $this->fail_call_indexes[ $call_number ]; + $this->captured_calls[] = array( + 'writes' => $writes, + 'meta_snapshot' => $meta_snapshot, + 'response' => $response, + ); + return $response; + } + + $response = $this->mock_response( $writes ); + $this->captured_calls[] = array( + 'writes' => $writes, + 'meta_snapshot' => $meta_snapshot, + 'response' => $response, + ); + return $response; + }, + 10, + 2 + ); + } + + /** + * Captured applyWrites calls (set by register_capture()). + * + * @var array + */ + private array $captured_calls = array(); + + /** + * 1-indexed map of call number → WP_Error to return for that call. + * + * @var array + */ + private array $fail_call_indexes = array(); + /** * Test that update() falls back to publish() when no URIs exist. * @@ -224,4 +324,446 @@ public function test_delete_errors_without_tids() { $this->assertWPError( $result ); $this->assertSame( 'atmosphere_not_published', $result->get_error_code() ); } + + /* + * ----------------------------------------------------------------- + * Thread publish/update/delete flows (via atmosphere_pre_apply_writes). + * ----------------------------------------------------------------- + */ + + /** + * Default long-form path (link-card) issues exactly one applyWrites + * call with 2 creates, and `META_THREAD_RECORDS` is populated as a + * 1-entry array mirroring the root. + */ + public function test_publish_link_card_writes_single_atomic_applywrites() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'A Long-Form Post', + 'post_content' => 'Body.', + 'post_excerpt' => 'Teaser excerpt.', + ) + ); + + $this->fail_call_indexes = array(); + $this->register_capture( $post->ID ); + + $result = Publisher::publish( $post ); + + $this->assertIsArray( $result ); + $this->assertCount( 1, $this->captured_calls, 'link-card publish uses one applyWrites call.' ); + + $writes = $this->captured_calls[0]['writes']; + $this->assertCount( 2, $writes ); + $this->assertSame( 'com.atproto.repo.applyWrites#create', $writes[0]['$type'] ); + $this->assertSame( 'app.bsky.feed.post', $writes[0]['collection'] ); + $this->assertSame( 'com.atproto.repo.applyWrites#create', $writes[1]['$type'] ); + $this->assertSame( 'site.standard.document', $writes[1]['collection'] ); + + $thread_records = \get_post_meta( $post->ID, Post::META_THREAD_RECORDS, true ); + $this->assertIsArray( $thread_records ); + $this->assertCount( 1, $thread_records ); + $this->assertNotEmpty( $thread_records[0]['uri'] ); + $this->assertNotEmpty( $thread_records[0]['cid'] ); + $this->assertNotEmpty( $thread_records[0]['tid'] ); + + $this->assertSame( $thread_records[0]['uri'], \get_post_meta( $post->ID, Post::META_URI, true ) ); + $this->assertSame( $thread_records[0]['tid'], \get_post_meta( $post->ID, Post::META_TID, true ) ); + } + + /** + * Teaser-thread writes root + doc atomically first, then the CTA + * reply as its own applyWrites call, with reply refs pointing at + * the root. + */ + public function test_publish_teaser_thread_writes_root_first_then_reply_sequentially() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'A Long-Form Post', + 'post_content' => 'Body content that is enough to compose a hook from.', + ) + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + + $this->fail_call_indexes = array(); + $this->register_capture( $post->ID ); + + $result = Publisher::publish( $post ); + + $this->assertIsArray( $result ); + $this->assertCount( 2, $this->captured_calls ); + + // Call 1: root + doc creates, no reply refs. + $first_writes = $this->captured_calls[0]['writes']; + $this->assertCount( 2, $first_writes ); + $this->assertSame( 'com.atproto.repo.applyWrites#create', $first_writes[0]['$type'] ); + $this->assertSame( 'app.bsky.feed.post', $first_writes[0]['collection'] ); + $this->assertSame( 'site.standard.document', $first_writes[1]['collection'] ); + $this->assertArrayNotHasKey( 'reply', $first_writes[0]['value'] ); + $this->assertNotEmpty( $first_writes[0]['value']['createdAt'] ); + + // Call 2: single reply create with reply refs pointing at root. + $second_writes = $this->captured_calls[1]['writes']; + $this->assertCount( 1, $second_writes ); + $this->assertSame( 'com.atproto.repo.applyWrites#create', $second_writes[0]['$type'] ); + $this->assertSame( 'app.bsky.feed.post', $second_writes[0]['collection'] ); + + $reply = $second_writes[0]['value']; + $this->assertArrayHasKey( 'reply', $reply ); + + $root_response_uri = $this->captured_calls[0]['response']['results'][0]['uri']; + $root_response_cid = $this->captured_calls[0]['response']['results'][0]['cid']; + + $this->assertSame( $root_response_uri, $reply['reply']['root']['uri'] ); + $this->assertSame( $root_response_cid, $reply['reply']['root']['cid'] ); + // 2-post thread: parent is the root. + $this->assertSame( $root_response_uri, $reply['reply']['parent']['uri'] ); + $this->assertSame( $root_response_cid, $reply['reply']['parent']['cid'] ); + } + + /** + * After the root write succeeds but before the reply write runs, + * `META_THREAD_RECORDS` should be a 1-entry array — a crash-recovery + * anchor pointing at the already-written root. + */ + public function test_publish_teaser_thread_partial_meta_written_after_root() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'A Long-Form Post', + 'post_content' => 'Body content that is enough to compose a hook from.', + ) + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + + $this->fail_call_indexes = array(); + $this->register_capture( $post->ID ); + + Publisher::publish( $post ); + + // Call 1 snapshot: taken before root write, meta should be empty / unset. + $this->assertTrue( empty( $this->captured_calls[0]['meta_snapshot'] ) ); + + // Call 2 snapshot: taken just before the reply write, root should + // be already persisted. + $call2_snapshot = $this->captured_calls[1]['meta_snapshot']; + $this->assertIsArray( $call2_snapshot ); + $this->assertCount( 1, $call2_snapshot ); + $this->assertNotEmpty( $call2_snapshot[0]['uri'] ); + } + + /** + * On happy-path thread publish, META_THREAD_RECORDS is an ordered + * 2-entry array (root, reply) and the legacy single-record meta + * mirrors the root. + */ + public function test_publish_teaser_thread_final_meta_has_ordered_triples() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'A Long-Form Post', + 'post_content' => 'Body content that is enough to compose a hook from.', + ) + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + + $this->fail_call_indexes = array(); + $this->register_capture( $post->ID ); + + Publisher::publish( $post ); + + $thread_records = \get_post_meta( $post->ID, Post::META_THREAD_RECORDS, true ); + $this->assertIsArray( $thread_records ); + $this->assertCount( 2, $thread_records ); + + foreach ( $thread_records as $record ) { + $this->assertNotEmpty( $record['uri'] ); + $this->assertNotEmpty( $record['cid'] ); + $this->assertNotEmpty( $record['tid'] ); + } + + // Single-record meta mirrors the root. + $this->assertSame( $thread_records[0]['uri'], \get_post_meta( $post->ID, Post::META_URI, true ) ); + $this->assertSame( $thread_records[0]['tid'], \get_post_meta( $post->ID, Post::META_TID, true ) ); + $this->assertSame( $thread_records[0]['cid'], \get_post_meta( $post->ID, Post::META_CID, true ) ); + } + + /** + * When the reply write fails, issue compensating deletes for the root + * + doc, clear every meta key, and return the original WP_Error. + */ + public function test_publish_teaser_thread_rollback_on_second_write_failure() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'A Long-Form Post', + 'post_content' => 'Body content that is enough to compose a hook from.', + ) + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + + // Fail call #2 (the reply create). Rollback call #3 succeeds. + $this->fail_call_indexes = array( + 2 => new \WP_Error( 'atmosphere_reply_failed', 'Reply write failed.' ), + ); + $this->register_capture( $post->ID ); + + $result = Publisher::publish( $post ); + + $this->assertWPError( $result ); + $this->assertSame( 'atmosphere_reply_failed', $result->get_error_code() ); + + // 3 calls total: root+doc create, reply create (fail), rollback deletes. + $this->assertCount( 3, $this->captured_calls ); + + $rollback_writes = $this->captured_calls[2]['writes']; + + // Rollback deletes root bsky first, then doc: tail-first traversal + // over the thread records, which here is just the root (only the + // root made it before failure). + $this->assertCount( 2, $rollback_writes ); + $this->assertSame( 'com.atproto.repo.applyWrites#delete', $rollback_writes[0]['$type'] ); + $this->assertSame( 'app.bsky.feed.post', $rollback_writes[0]['collection'] ); + $this->assertSame( 'com.atproto.repo.applyWrites#delete', $rollback_writes[1]['$type'] ); + $this->assertSame( 'site.standard.document', $rollback_writes[1]['collection'] ); + + // Root TID appears in rollback. + $root_rkey = $rollback_writes[0]['rkey']; + $this->assertNotEmpty( $root_rkey ); + + // Meta fully cleared. + $this->assertSame( '', \get_post_meta( $post->ID, Post::META_THREAD_RECORDS, true ) ); + $this->assertSame( '', \get_post_meta( $post->ID, Post::META_URI, true ) ); + $this->assertSame( '', \get_post_meta( $post->ID, Post::META_TID, true ) ); + $this->assertSame( '', \get_post_meta( $post->ID, Post::META_CID, true ) ); + } + + /** + * When both the reply write AND the rollback fail, the returned + * WP_Error wraps both errors and carries `partial_records` data for + * manual cleanup. + */ + public function test_publish_teaser_thread_rollback_failing_surfaces_partial_state() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'A Long-Form Post', + 'post_content' => 'Body content that is enough to compose a hook from.', + ) + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + + $this->fail_call_indexes = array( + 2 => new \WP_Error( 'atmosphere_reply_failed', 'Reply write failed.' ), + 3 => new \WP_Error( 'atmosphere_rollback_pds', 'Rollback PDS error.' ), + ); + $this->register_capture( $post->ID ); + + $result = Publisher::publish( $post ); + + $this->assertWPError( $result ); + $this->assertSame( 'atmosphere_thread_rollback_failed', $result->get_error_code() ); + + $data = $result->get_error_data(); + $this->assertIsArray( $data ); + $this->assertArrayHasKey( 'partial_records', $data ); + $this->assertIsArray( $data['partial_records'] ); + $this->assertCount( 1, $data['partial_records'] ); + $this->assertArrayHasKey( 'original_error', $data ); + $this->assertArrayHasKey( 'rollback_error', $data ); + } + + /** + * Update with stored single-record + single-record composition uses + * applyWrites#update in place rather than delete + republish. + */ + public function test_update_link_card_unchanged_single_post_uses_in_place_applywrites_update() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'A Long-Form Post', + 'post_content' => 'Body.', + 'post_excerpt' => 'Teaser excerpt.', + ) + ); + + // Seed META_THREAD_RECORDS with a 1-entry stored root + legacy mirrors. + $root_uri = 'at://did:plc:test123/app.bsky.feed.post/stored-rkey-1'; + \update_post_meta( + $post->ID, + Post::META_THREAD_RECORDS, + array( + array( + 'uri' => $root_uri, + 'cid' => 'bafyreibstored', + 'tid' => 'stored-rkey-1', + ), + ) + ); + \update_post_meta( $post->ID, Post::META_URI, $root_uri ); + \update_post_meta( $post->ID, Post::META_TID, 'stored-rkey-1' ); + \update_post_meta( $post->ID, Post::META_CID, 'bafyreibstored' ); + \update_post_meta( $post->ID, Document::META_URI, 'at://did:plc:test123/site.standard.document/doc-rkey-1' ); + \update_post_meta( $post->ID, Document::META_TID, 'doc-rkey-1' ); + + $this->fail_call_indexes = array(); + $this->register_capture( $post->ID ); + + $result = Publisher::update( $post ); + + $this->assertIsArray( $result ); + $this->assertCount( 1, $this->captured_calls, 'single-record update uses one applyWrites.' ); + + $writes = $this->captured_calls[0]['writes']; + $this->assertCount( 2, $writes ); + $this->assertSame( 'com.atproto.repo.applyWrites#update', $writes[0]['$type'] ); + $this->assertSame( 'stored-rkey-1', $writes[0]['rkey'] ); + $this->assertSame( 'com.atproto.repo.applyWrites#update', $writes[1]['$type'] ); + $this->assertSame( 'doc-rkey-1', $writes[1]['rkey'] ); + } + + /** + * Update with a stored 1-entry link-card but a teaser-thread composition + * deletes the old record + doc atomically, then publishes fresh. + */ + public function test_update_thread_rewrites_on_strategy_change() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'A Long-Form Post', + 'post_content' => 'Body content that is enough to compose a hook from.', + ) + ); + + $root_uri = 'at://did:plc:test123/app.bsky.feed.post/stored-rkey-1'; + \update_post_meta( + $post->ID, + Post::META_THREAD_RECORDS, + array( + array( + 'uri' => $root_uri, + 'cid' => 'bafyreibstored', + 'tid' => 'stored-rkey-1', + ), + ) + ); + \update_post_meta( $post->ID, Post::META_URI, $root_uri ); + \update_post_meta( $post->ID, Post::META_TID, 'stored-rkey-1' ); + \update_post_meta( $post->ID, Post::META_CID, 'bafyreibstored' ); + \update_post_meta( $post->ID, Document::META_URI, 'at://did:plc:test123/site.standard.document/doc-rkey-1' ); + \update_post_meta( $post->ID, Document::META_TID, 'doc-rkey-1' ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + + $this->fail_call_indexes = array(); + $this->register_capture( $post->ID ); + + $result = Publisher::update( $post ); + + $this->assertIsArray( $result ); + + // Call 1: delete old (1 bsky + 1 doc). + $this->assertCount( 3, $this->captured_calls ); + $delete_writes = $this->captured_calls[0]['writes']; + $this->assertCount( 2, $delete_writes ); + $this->assertSame( 'com.atproto.repo.applyWrites#delete', $delete_writes[0]['$type'] ); + $this->assertSame( 'stored-rkey-1', $delete_writes[0]['rkey'] ); + $this->assertSame( 'com.atproto.repo.applyWrites#delete', $delete_writes[1]['$type'] ); + $this->assertSame( 'doc-rkey-1', $delete_writes[1]['rkey'] ); + + // Call 2: fresh publish — root + doc creates. + $publish_root_writes = $this->captured_calls[1]['writes']; + $this->assertCount( 2, $publish_root_writes ); + $this->assertSame( 'com.atproto.repo.applyWrites#create', $publish_root_writes[0]['$type'] ); + + // Call 3: reply create. + $reply_writes = $this->captured_calls[2]['writes']; + $this->assertCount( 1, $reply_writes ); + $this->assertSame( 'com.atproto.repo.applyWrites#create', $reply_writes[0]['$type'] ); + + $thread_records = \get_post_meta( $post->ID, Post::META_THREAD_RECORDS, true ); + $this->assertIsArray( $thread_records ); + $this->assertCount( 2, $thread_records ); + } + + /** + * Deleting a thread-published post issues one atomic applyWrites with + * every bsky delete + the doc delete, then clears all meta. + */ + public function test_delete_thread_removes_all_records() { + $post = self::factory()->post->create_and_get(); + + $thread = array( + array( + 'uri' => 'at://did:plc:test123/app.bsky.feed.post/t-root', + 'cid' => 'bafyreibroot', + 'tid' => 't-root', + ), + array( + 'uri' => 'at://did:plc:test123/app.bsky.feed.post/t-reply', + 'cid' => 'bafyreibrepy', + 'tid' => 't-reply', + ), + ); + \update_post_meta( $post->ID, Post::META_THREAD_RECORDS, $thread ); + \update_post_meta( $post->ID, Post::META_URI, $thread[0]['uri'] ); + \update_post_meta( $post->ID, Post::META_TID, 't-root' ); + \update_post_meta( $post->ID, Post::META_CID, 'bafyreibroot' ); + \update_post_meta( $post->ID, Document::META_URI, 'at://did:plc:test123/site.standard.document/doc-rkey-1' ); + \update_post_meta( $post->ID, Document::META_TID, 'doc-rkey-1' ); + + $this->fail_call_indexes = array(); + $this->register_capture( $post->ID ); + + $result = Publisher::delete( $post ); + + $this->assertIsArray( $result ); + $this->assertCount( 1, $this->captured_calls ); + + $writes = $this->captured_calls[0]['writes']; + $this->assertCount( 3, $writes ); // 2 bsky deletes + 1 doc delete. + $this->assertSame( 't-root', $writes[0]['rkey'] ); + $this->assertSame( 't-reply', $writes[1]['rkey'] ); + $this->assertSame( 'site.standard.document', $writes[2]['collection'] ); + $this->assertSame( 'doc-rkey-1', $writes[2]['rkey'] ); + + $this->assertSame( '', \get_post_meta( $post->ID, Post::META_THREAD_RECORDS, true ) ); + $this->assertSame( '', \get_post_meta( $post->ID, Post::META_URI, true ) ); + $this->assertSame( '', \get_post_meta( $post->ID, Post::META_TID, true ) ); + $this->assertSame( '', \get_post_meta( $post->ID, Post::META_CID, true ) ); + $this->assertSame( '', \get_post_meta( $post->ID, Document::META_URI, true ) ); + $this->assertSame( '', \get_post_meta( $post->ID, Document::META_TID, true ) ); + } + + /** + * A post with only legacy single-record meta (no META_THREAD_RECORDS) + * still deletes correctly via the fallback path. + */ + public function test_delete_legacy_single_post_meta() { + $post = self::factory()->post->create_and_get(); + + \update_post_meta( $post->ID, Post::META_URI, 'at://did:plc:test123/app.bsky.feed.post/legacy-rkey' ); + \update_post_meta( $post->ID, Post::META_TID, 'legacy-rkey' ); + \update_post_meta( $post->ID, Post::META_CID, 'bafyreiblegacy' ); + \update_post_meta( $post->ID, Document::META_URI, 'at://did:plc:test123/site.standard.document/doc-legacy' ); + \update_post_meta( $post->ID, Document::META_TID, 'doc-legacy' ); + // Deliberately no META_THREAD_RECORDS. + + $this->fail_call_indexes = array(); + $this->register_capture( $post->ID ); + + $result = Publisher::delete( $post ); + + $this->assertIsArray( $result ); + $this->assertCount( 1, $this->captured_calls ); + + $writes = $this->captured_calls[0]['writes']; + $this->assertCount( 2, $writes ); + $this->assertSame( 'com.atproto.repo.applyWrites#delete', $writes[0]['$type'] ); + $this->assertSame( 'legacy-rkey', $writes[0]['rkey'] ); + $this->assertSame( 'doc-legacy', $writes[1]['rkey'] ); + + $this->assertSame( '', \get_post_meta( $post->ID, Post::META_URI, true ) ); + $this->assertSame( '', \get_post_meta( $post->ID, Post::META_TID, true ) ); + $this->assertSame( '', \get_post_meta( $post->ID, Post::META_CID, true ) ); + } } From 821f38f1935e66cced05fea6565f80ca9aba5ca9 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Thu, 23 Apr 2026 21:21:30 -0500 Subject: [PATCH 06/22] add: changelog for atmosphere_long_form_composition + thread meta --- .github/changelog/long-form-teaser-thread | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 .github/changelog/long-form-teaser-thread diff --git a/.github/changelog/long-form-teaser-thread b/.github/changelog/long-form-teaser-thread new file mode 100644 index 0000000..cbe945b --- /dev/null +++ b/.github/changelog/long-form-teaser-thread @@ -0,0 +1,4 @@ +Significance: minor +Type: added + +Long-form post composition is now strategy-aware. New `atmosphere_long_form_composition` filter selects between `link-card` (default, unchanged), `truncate-link` (single post, body + permalink), and `teaser-thread` (2-post thread: hook + CTA). Threads persist as an ordered `{uri, cid, tid}` array under the new `_atmosphere_bsky_thread_records` post meta; legacy `_atmosphere_bsky_*` keys continue to mirror the root. `Publisher::publish/update/delete` rewritten with sequential-writes-with-rollback for multi-record threads and a new `atmosphere_pre_apply_writes` test seam. `Publisher::update()` for thread posts deletes and republishes the whole thread (known caveat: external replies to the original thread are orphaned; followers see the update as a fresh publish with new createdAt) — callers that need in-place updates should pin to a single-post strategy. From 72ac4740a6cd6e776da204c06ab0de9adede9bf3 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Fri, 24 Apr 2026 11:30:51 -0500 Subject: [PATCH 07/22] Address initial review: createdAt, force-delete, rollback orphans, in-place thread update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five changes informed by the early review feedback on #34: 1. Preserve the post's publish date as `createdAt` on every record representing the post itself (short-form transform(), link-card, truncate-link, and thread root). Previously the long-form paths fell through to `wp_date('c')`, which stamped the backfill-run time on re-synced posts and collapsed timeline ordering. 2. Thread-aware force-delete. `Atmosphere::on_before_delete` now reads `Post::META_THREAD_RECORDS` and schedules a delete covering every bsky tid plus the doc; `Publisher::delete_by_tids` accepts an array so one applyWrites covers the full thread. Legacy single-TID callers still work unchanged. 3. Persist rollback orphans to post meta. When a thread publish fails and the compensating rollback itself fails, the partial record list now lands in `Post::META_ORPHAN_RECORDS` (and error_log) instead of disappearing with the cron closure's WP_Error. 4. In-place applyWrites#update for thread posts when the record count is unchanged. Preserves TIDs/URIs and external replies; only CIDs change. Falls back to delete+republish when the thread shape changes (single ↔ thread, 2-post ↔ 3-post). 5. Cleanups: drop redundant get_rkey() side-effect calls from publish(); split legacy store_results() into a document-only helper so the root's single-record meta is mirrored from one place; document that `atmosphere_transform_bsky_post` fires per record (not per post) when the thread strategy is active. --- .github/changelog/long-form-teaser-thread | 2 +- includes/class-atmosphere.php | 36 +- includes/class-publisher.php | 336 +++++++++++++----- includes/transformer/class-post.php | 29 +- tests/phpunit/tests/class-test-publisher.php | 80 +++++ .../tests/transformer/class-test-post.php | 6 + 6 files changed, 389 insertions(+), 100 deletions(-) diff --git a/.github/changelog/long-form-teaser-thread b/.github/changelog/long-form-teaser-thread index cbe945b..d50a854 100644 --- a/.github/changelog/long-form-teaser-thread +++ b/.github/changelog/long-form-teaser-thread @@ -1,4 +1,4 @@ Significance: minor Type: added -Long-form post composition is now strategy-aware. New `atmosphere_long_form_composition` filter selects between `link-card` (default, unchanged), `truncate-link` (single post, body + permalink), and `teaser-thread` (2-post thread: hook + CTA). Threads persist as an ordered `{uri, cid, tid}` array under the new `_atmosphere_bsky_thread_records` post meta; legacy `_atmosphere_bsky_*` keys continue to mirror the root. `Publisher::publish/update/delete` rewritten with sequential-writes-with-rollback for multi-record threads and a new `atmosphere_pre_apply_writes` test seam. `Publisher::update()` for thread posts deletes and republishes the whole thread (known caveat: external replies to the original thread are orphaned; followers see the update as a fresh publish with new createdAt) — callers that need in-place updates should pin to a single-post strategy. +Long-form posts can now be published as a Bluesky thread. A new filter lets sites choose between a single link-card post (default, unchanged from today), a single post that combines body text with the permalink, or a 2-post teaser thread that leads with a hook and follows with a "continue reading" link. Post edits update every record in place when the shape of the post hasn't changed, so Bluesky URLs stay stable and people's replies don't get orphaned. If the shape of the post changes (for example, switching between a single post and a thread), the old records are replaced with fresh ones. diff --git a/includes/class-atmosphere.php b/includes/class-atmosphere.php index f0d9a69..39656d1 100644 --- a/includes/class-atmosphere.php +++ b/includes/class-atmosphere.php @@ -283,8 +283,11 @@ public function on_status_change( string $new_status, string $old_status, \WP_Po /** * Schedule AT Protocol record deletion before a post is permanently deleted. * - * Captures TIDs from post meta before they're lost, then schedules - * an async delete via cron. + * Captures every Bluesky TID (including thread replies) and the + * document TID from post meta before they're lost, then schedules + * an async delete via cron. Thread-strategy posts: reads + * `Post::META_THREAD_RECORDS` and batches every bsky tid into the + * cron event so a single delete covers root + replies. * * @param int $post_id Post ID being deleted. */ @@ -299,11 +302,28 @@ public function on_before_delete( int $post_id ): void { return; } - $bsky_tid = \get_post_meta( $post_id, Transformer\Post::META_TID, true ); - $doc_tid = \get_post_meta( $post_id, Transformer\Document::META_TID, true ); + $bsky_tids = array(); - if ( $bsky_tid || $doc_tid ) { - \wp_schedule_single_event( \time(), 'atmosphere_delete_records', array( $bsky_tid, $doc_tid ) ); + $thread_records = \get_post_meta( $post_id, Transformer\Post::META_THREAD_RECORDS, true ); + if ( \is_array( $thread_records ) && ! empty( $thread_records ) ) { + foreach ( $thread_records as $record ) { + if ( ! empty( $record['tid'] ) ) { + $bsky_tids[] = (string) $record['tid']; + } + } + } + + if ( empty( $bsky_tids ) ) { + $legacy_tid = \get_post_meta( $post_id, Transformer\Post::META_TID, true ); + if ( $legacy_tid ) { + $bsky_tids[] = (string) $legacy_tid; + } + } + + $doc_tid = (string) \get_post_meta( $post_id, Transformer\Document::META_TID, true ); + + if ( ! empty( $bsky_tids ) || '' !== $doc_tid ) { + \wp_schedule_single_event( \time(), 'atmosphere_delete_records', array( $bsky_tids, $doc_tid ) ); } } @@ -374,8 +394,8 @@ static function (): void { \add_action( 'atmosphere_delete_records', - static function ( string $bsky_tid, string $doc_tid ): void { - Publisher::delete_by_tids( $bsky_tid, $doc_tid ); + static function ( $bsky_tids, string $doc_tid ): void { + Publisher::delete_by_tids( $bsky_tids, $doc_tid ); }, 10, 2 diff --git a/includes/class-publisher.php b/includes/class-publisher.php index 6820a1a..93fc065 100644 --- a/includes/class-publisher.php +++ b/includes/class-publisher.php @@ -46,10 +46,6 @@ public static function publish( \WP_Post $post ): array|\WP_Error { $bsky_transformer = new Post( $post ); $doc_transformer = new Document( $post ); - // Ensure TIDs exist (generates if needed). - $bsky_transformer->get_rkey(); - $doc_transformer->get_rkey(); - if ( $bsky_transformer->is_short_form_post() ) { // Short-form path: single record via today's transform(). return self::publish_single( @@ -76,10 +72,13 @@ public static function publish( \WP_Post $post ): array|\WP_Error { * `link-card` / `truncate-link` long-form strategies (via * `build_long_form_records()`'s single-element output). * - * `createdAt` defaults to `wp_date( 'c' )` when the record doesn't - * already carry one. `transform()` sets `createdAt` from the post's - * `post_date_gmt` (preserving today's short-form behavior); - * `build_long_form_records()` omits it on purpose. + * `createdAt` defaults to the post's `post_date_gmt` when the record + * doesn't already carry one, so the Bluesky timeline mirrors the + * WordPress publish date (critical for backfill — otherwise every + * re-synced post would stamp with the backfill-run time and + * collapse chronological order). `transform()` already sets it; + * `build_long_form_records()` leaves it absent on purpose so a + * single policy lives here. * * @param \WP_Post $post WordPress post. * @param array $bsky_record Pre-composed bsky post record. @@ -94,7 +93,7 @@ private static function publish_single( Document $doc_transformer ): array|\WP_Error { if ( empty( $bsky_record['createdAt'] ) ) { - $bsky_record['createdAt'] = \wp_date( 'c' ); + $bsky_record['createdAt'] = to_iso8601( $post->post_date_gmt ); } $writes = array( @@ -118,7 +117,7 @@ private static function publish_single( return $result; } - self::store_results( $post->ID, $result, $bsky_transformer, $doc_transformer ); + self::store_document_meta( $post->ID, $result, $doc_transformer ); self::mirror_thread_records_meta( $post->ID, array( @@ -162,7 +161,7 @@ private static function publish_thread( Document $doc_transformer ): array|\WP_Error { $root_record = $records[0]; - $root_record['createdAt'] = \wp_date( 'c' ); + $root_record['createdAt'] = to_iso8601( $post->post_date_gmt ); $root_rkey = $bsky_transformer->get_rkey(); $root_result = API::apply_writes( @@ -202,7 +201,7 @@ private static function publish_thread( $thread_records = array( $root_triple ); - self::store_results( $post->ID, $root_result, $bsky_transformer, $doc_transformer ); + self::store_document_meta( $post->ID, $root_result, $doc_transformer ); self::mirror_thread_records_meta( $post->ID, $thread_records ); self::update_document_bsky_ref( $post, $doc_transformer ); @@ -275,11 +274,13 @@ private static function publish_thread( * replies pointing at the (still-live) root remain valid until their * own delete lands. The document record is deleted last. * - * Meta is always cleared — a failed rollback leaves orphans on the - * PDS but the local state stays consistent with "no published thread." - * When rollback itself fails, the returned `WP_Error` wraps both - * errors and carries `partial_records` so an operator retrying can - * clean up by hand. + * Active-record meta is always cleared so the local state stays + * consistent with "no published thread." When rollback itself fails, + * the partial thread is *also* persisted to `Post::META_ORPHAN_RECORDS` + * (and error-logged) so an operator or recovery worker can issue + * manual deletes later — the orphan manifest in the returned + * `WP_Error` data otherwise disappears the moment the cron closure + * returns. * * @param \WP_Post $post WordPress post. * @param array[] $thread_records Already-written thread records (uri/cid/tid each). @@ -293,6 +294,8 @@ private static function rollback_thread( Document $doc_transformer, \WP_Error $original_error ): \WP_Error { + $doc_rkey = $doc_transformer->get_rkey(); + $rollback_writes = array(); for ( $i = \count( $thread_records ) - 1; $i >= 0; $i-- ) { @@ -305,7 +308,7 @@ private static function rollback_thread( $rollback_writes[] = array( '$type' => 'com.atproto.repo.applyWrites#delete', 'collection' => 'site.standard.document', - 'rkey' => $doc_transformer->get_rkey(), + 'rkey' => $doc_rkey, ); $rollback_result = API::apply_writes( $rollback_writes ); @@ -313,13 +316,12 @@ private static function rollback_thread( self::clear_all_record_meta( $post->ID ); if ( \is_wp_error( $rollback_result ) ) { - // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log - \error_log( - \sprintf( - '[atmosphere] thread rollback failed for post %d: %s', - $post->ID, - $rollback_result->get_error_message() - ) + self::persist_orphan_records( + $post->ID, + $thread_records, + $doc_rkey, + $original_error, + $rollback_result ); return new \WP_Error( @@ -340,16 +342,70 @@ private static function rollback_thread( return $original_error; } + /** + * Record the partial thread state left on the PDS after a failed + * rollback so a human (or a future recovery worker) can find it. + * + * Writes `Post::META_ORPHAN_RECORDS` and error-logs a + * machine-parseable summary. The post meta is the source of truth — + * the log line is a convenience for ops grepping a filesystem tail. + * + * @param int $post_id WordPress post ID. + * @param array[] $thread_records Thread records that survived rollback. + * @param string $doc_rkey Document rkey that survived rollback. + * @param \WP_Error $original_error Publish-time error that triggered rollback. + * @param \WP_Error $rollback_error Rollback-time error. + */ + private static function persist_orphan_records( + int $post_id, + array $thread_records, + string $doc_rkey, + \WP_Error $original_error, + \WP_Error $rollback_error + ): void { + $entry = array( + 'stamp' => \gmdate( 'Y-m-d\TH:i:s.000\Z' ), + 'bsky_records' => $thread_records, + 'doc_rkey' => $doc_rkey, + 'original_error' => $original_error->get_error_message(), + 'rollback_error' => $rollback_error->get_error_message(), + ); + + $existing = \get_post_meta( $post_id, Post::META_ORPHAN_RECORDS, true ); + if ( ! \is_array( $existing ) ) { + $existing = array(); + } + + $existing[] = $entry; + + \update_post_meta( $post_id, Post::META_ORPHAN_RECORDS, $existing ); + + // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log + \error_log( + \sprintf( + '[atmosphere] thread rollback failed for post %d; orphans persisted to %s: %s', + $post_id, + Post::META_ORPHAN_RECORDS, + \wp_json_encode( $entry ) + ) + ); + } + /** * Update the bsky + doc records for an existing post. * - * - Single record stored + single record composed: in-place update - * via `applyWrites#update` on both. - * - Any other shape (strategy change, thread ↔ single, or - * thread ↔ thread): delete every existing record and republish - * with the fresh composition. Thread updates therefore arrive to - * followers as a fresh publish (new `createdAt`), and any replies - * other Bluesky users posted become orphaned. + * - Stored record count == new record count: in-place update via + * `applyWrites#update` on every bsky record + doc in one atomic + * batch. Preserves TIDs, URIs, and external replies; each record + * just gets a new CID from the PDS. Reply refs are rewired to + * the pre-update CIDs so each record's `reply.parent.cid` still + * resolves — clients treat the mismatch as "parent was edited." + * - Record counts differ (strategy change: link-card ↔ teaser-thread, + * or 2-post thread ↔ 3-post thread): delete every existing record + * and republish with the fresh composition. Thread updates via + * this path arrive to followers as a fresh publish (new + * `createdAt`) and any replies other Bluesky users posted become + * orphaned. * * @param \WP_Post $post WordPress post. * @return array|\WP_Error @@ -394,19 +450,29 @@ public static function update( \WP_Post $post ): array|\WP_Error { ? array( $bsky_transformer->transform() ) : $bsky_transformer->build_long_form_records(); - // In-place update: single stored + single new composition. - if ( 1 === \count( $stored ) && 1 === \count( $new_records ) ) { - return self::update_single( + // In-place update: matching record counts. + if ( \count( $stored ) === \count( $new_records ) ) { + if ( 1 === \count( $stored ) ) { + return self::update_single( + $post, + $stored[0], + $new_records[0], + $bsky_transformer, + $doc_transformer, + $doc_tid + ); + } + + return self::update_thread_in_place( $post, - $stored[0], - $new_records[0], - $bsky_transformer, + $stored, + $new_records, $doc_transformer, $doc_tid ); } - // Strategy change or thread shape — delete everything and republish. + // Strategy or shape change — delete everything and republish. return self::rewrite_thread( $post, $stored, $doc_tid ); } @@ -432,7 +498,7 @@ private static function update_single( string $doc_tid ): array|\WP_Error { if ( empty( $new_bsky_record['createdAt'] ) ) { - $new_bsky_record['createdAt'] = \wp_date( 'c' ); + $new_bsky_record['createdAt'] = to_iso8601( $post->post_date_gmt ); } $writes = array( @@ -456,7 +522,7 @@ private static function update_single( return $result; } - self::store_results( $post->ID, $result, $bsky_transformer, $doc_transformer ); + self::store_document_meta( $post->ID, $result, $doc_transformer ); self::mirror_thread_records_meta( $post->ID, array( @@ -473,6 +539,102 @@ private static function update_single( return $result; } + /** + * In-place `applyWrites#update` for every record in a thread + + * the document, in one atomic batch. + * + * Preserves URIs/TIDs/external reply integrity; each record's CID + * changes (since CID is a content hash). Reply refs are built from + * the *pre-update* CIDs stored in `META_THREAD_RECORDS` — + * structurally self-consistent at write time, and clients treat any + * post-update CID mismatch as "parent was edited" rather than + * broken. + * + * After the write, `META_THREAD_RECORDS` is refreshed with the new + * CIDs from the response so future updates chain from current CIDs. + * + * @param \WP_Post $post WordPress post. + * @param array[] $stored Current {uri, cid, tid} triples in order. + * @param array[] $new_records Freshly composed bsky records, same count. + * @param Document $doc_transformer Document transformer. + * @param string $doc_tid Document record TID. + * @return array|\WP_Error + */ + private static function update_thread_in_place( + \WP_Post $post, + array $stored, + array $new_records, + Document $doc_transformer, + string $doc_tid + ): array|\WP_Error { + $root = $stored[0]; + $created_at = to_iso8601( $post->post_date_gmt ); + $writes = array(); + $bsky_count = \count( $new_records ); + + foreach ( $new_records as $i => $record ) { + $record['createdAt'] = $created_at; + + if ( $i > 0 ) { + $record['reply'] = array( + 'root' => array( + 'uri' => $root['uri'], + 'cid' => $root['cid'], + ), + 'parent' => array( + 'uri' => $stored[ $i - 1 ]['uri'], + 'cid' => $stored[ $i - 1 ]['cid'], + ), + ); + } + + $writes[] = array( + '$type' => 'com.atproto.repo.applyWrites#update', + 'collection' => 'app.bsky.feed.post', + 'rkey' => $stored[ $i ]['tid'], + 'value' => $record, + ); + } + + $writes[] = array( + '$type' => 'com.atproto.repo.applyWrites#update', + 'collection' => 'site.standard.document', + 'rkey' => $doc_tid, + 'value' => $doc_transformer->transform(), + ); + + $result = API::apply_writes( $writes ); + + if ( \is_wp_error( $result ) ) { + return $result; + } + + $results = $result['results'] ?? array(); + $refreshed = array(); + foreach ( $stored as $i => $old ) { + $entry = $results[ $i ] ?? array(); + $refreshed[] = array( + 'uri' => $old['uri'], + 'cid' => (string) ( $entry['cid'] ?? $old['cid'] ), + 'tid' => $old['tid'], + ); + } + + self::mirror_thread_records_meta( $post->ID, $refreshed ); + + $doc_entry = $results[ $bsky_count ] ?? array(); + if ( ! empty( $doc_entry['uri'] ) ) { + \update_post_meta( $post->ID, Document::META_URI, $doc_entry['uri'] ); + } + if ( ! empty( $doc_entry['cid'] ) ) { + \update_post_meta( $post->ID, Document::META_CID, $doc_entry['cid'] ); + } + + self::update_document_bsky_ref( $post, $doc_transformer ); + + return $result; + } + /** * Delete every stored bsky record + the doc atomically, then publish * fresh. Used when the composition strategy changes (single ↔ thread) @@ -575,24 +737,31 @@ public static function delete( \WP_Post $post ): array|\WP_Error { * Delete AT Protocol records by TID without requiring the post to exist. * * Used when a post is permanently deleted and its meta is no longer - * accessible to `delete()`. Kept at the single-TID signature for - * backwards compatibility with queued cron events; force-deletion of - * thread-strategy posts is handled by the caller by reading - * META_THREAD_RECORDS pre-deletion and scheduling a separate cron per - * thread record. - * - * @param string $bsky_tid Bluesky post TID (may be empty). - * @param string $doc_tid Document TID (may be empty). + * accessible to `delete()`. Accepts either a single Bluesky TID + * (legacy single-record posts) or an array of TIDs + * (thread-strategy posts). All are issued in one atomic + * `applyWrites` call. + * + * @param string|string[] $bsky_tids Bluesky post TID or array of TIDs (may be empty). + * @param string $doc_tid Document TID (may be empty). * @return array|\WP_Error */ - public static function delete_by_tids( string $bsky_tid, string $doc_tid ): array|\WP_Error { - if ( ! $bsky_tid && ! $doc_tid ) { + public static function delete_by_tids( $bsky_tids, string $doc_tid ): array|\WP_Error { + if ( \is_string( $bsky_tids ) ) { + $bsky_tids = '' === $bsky_tids ? array() : array( $bsky_tids ); + } elseif ( ! \is_array( $bsky_tids ) ) { + $bsky_tids = array(); + } + + $bsky_tids = \array_values( \array_filter( \array_map( 'strval', $bsky_tids ), 'strlen' ) ); + + if ( empty( $bsky_tids ) && ! $doc_tid ) { return new \WP_Error( 'atmosphere_not_published', \__( 'No TIDs provided.', 'atmosphere' ) ); } $writes = array(); - if ( $bsky_tid ) { + foreach ( $bsky_tids as $bsky_tid ) { $writes[] = array( '$type' => 'com.atproto.repo.applyWrites#delete', 'collection' => 'app.bsky.feed.post', @@ -654,45 +823,32 @@ public static function sync_publication(): array|\WP_Error { } /** - * Extract URIs/CIDs from applyWrites response and mirror into the - * legacy single-record meta keys (META_URI / META_TID / META_CID - * on Post and Document). - * - * Called for the root + doc write in every publish flow. Thread - * reply writes don't go through this helper — they're captured in - * `META_THREAD_RECORDS` by `publish_thread()` directly. - * - * @param int $post_id Post ID. - * @param array $result applyWrites response. - * @param Post $bsky_transformer Bsky transformer. - * @param Document $doc_transformer Document transformer. + * Persist the document record's URI/CID from an applyWrites response. + * + * The document is always written at index 1 of the first applyWrites + * batch in every publish flow (root + doc, atomically). Post meta + * (`Post::META_URI` / `META_TID` / `META_CID`) is owned by + * `mirror_thread_records_meta()` and intentionally not touched here + * — single mirroring point keeps the two paths from drifting. + * + * @param int $post_id Post ID. + * @param array $result applyWrites response. + * @param Document $doc_transformer Document transformer. */ - private static function store_results( int $post_id, array $result, Post $bsky_transformer, Document $doc_transformer ): void { - $results = $result['results'] ?? array(); - - foreach ( $results as $i => $item ) { - $uri = $item['uri'] ?? ''; - $cid = $item['cid'] ?? ''; - - if ( 0 === $i ) { - if ( $uri ) { - \update_post_meta( $post_id, Post::META_URI, $uri ); - } else { - \update_post_meta( $post_id, Post::META_URI, $bsky_transformer->get_uri() ); - } - if ( $cid ) { - \update_post_meta( $post_id, Post::META_CID, $cid ); - } - } elseif ( 1 === $i ) { - if ( $uri ) { - \update_post_meta( $post_id, Document::META_URI, $uri ); - } else { - \update_post_meta( $post_id, Document::META_URI, $doc_transformer->get_uri() ); - } - if ( $cid ) { - \update_post_meta( $post_id, Document::META_CID, $cid ); - } - } + private static function store_document_meta( int $post_id, array $result, Document $doc_transformer ): void { + $doc_entry = $result['results'][1] ?? null; + + if ( null === $doc_entry ) { + return; + } + + $uri = $doc_entry['uri'] ?? ''; + $cid = $doc_entry['cid'] ?? ''; + + \update_post_meta( $post_id, Document::META_URI, $uri ?: $doc_transformer->get_uri() ); + + if ( $cid ) { + \update_post_meta( $post_id, Document::META_CID, $cid ); } } diff --git a/includes/transformer/class-post.php b/includes/transformer/class-post.php index 6fb13e5..c8d33ea 100644 --- a/includes/transformer/class-post.php +++ b/includes/transformer/class-post.php @@ -57,6 +57,22 @@ class Post extends Base { */ public const META_THREAD_RECORDS = '_atmosphere_bsky_thread_records'; + /** + * Post meta key for thread records left orphaned on the PDS after a + * rollback failure. + * + * Populated by Publisher only when a thread publish fails and the + * compensating-delete rollback also fails — the records listed here + * are alive on Bluesky but no longer tracked in META_THREAD_RECORDS + * (which Publisher clears to keep the local "active" state + * consistent with "not published"). Surfaced so an operator or + * recovery worker can issue manual deletes. Value shape mirrors + * META_THREAD_RECORDS with an added `stamp` key (ISO 8601 UTC). + * + * @var string + */ + public const META_ORPHAN_RECORDS = '_atmosphere_bsky_orphan_records'; + /** * Transform the post. * @@ -117,6 +133,17 @@ public function transform(): array { /** * Filters the app.bsky.feed.post record before publishing. * + * Fires once per record. For single-record strategies + * (`link-card`, `truncate-link`, and any short-form post) this + * is exactly one call per WordPress post — today's behavior. + * For `teaser-thread`, the filter fires for *every* thread + * entry (hook, intermediate posts, CTA). Listeners that + * accumulate state across calls (rate-limit counters, external + * lint hooks) should branch on record shape (presence of + * `reply` or permalink-in-`text`) or inspect + * `atmosphere_long_form_composition` to decide per-entry + * behavior. + * * @param array $record Bsky post record. * @param \WP_Post $post WordPress post. */ @@ -435,7 +462,7 @@ private function truncate_to_budget( string $text, int $max, bool $prefer_senten } $word_cut = \preg_replace( '/\s+\S*$/u', '', $clamped ); - if ( \is_string( $word_cut ) && '' !== $word_cut ) { + if ( \is_string( $word_cut ) && '' !== $word_cut && $word_cut !== $clamped ) { return $word_cut; } diff --git a/tests/phpunit/tests/class-test-publisher.php b/tests/phpunit/tests/class-test-publisher.php index d390ecb..c96960f 100644 --- a/tests/phpunit/tests/class-test-publisher.php +++ b/tests/phpunit/tests/class-test-publisher.php @@ -572,6 +572,16 @@ public function test_publish_teaser_thread_rollback_failing_surfaces_partial_sta $this->assertCount( 1, $data['partial_records'] ); $this->assertArrayHasKey( 'original_error', $data ); $this->assertArrayHasKey( 'rollback_error', $data ); + + // Orphan manifest is persisted to post meta so it outlives the cron closure. + $orphans = \get_post_meta( $post->ID, Post::META_ORPHAN_RECORDS, true ); + $this->assertIsArray( $orphans ); + $this->assertCount( 1, $orphans ); + $this->assertSame( $data['partial_records'], $orphans[0]['bsky_records'] ); + $this->assertArrayHasKey( 'stamp', $orphans[0] ); + $this->assertNotEmpty( $orphans[0]['doc_rkey'] ); + $this->assertSame( 'Reply write failed.', $orphans[0]['original_error'] ); + $this->assertSame( 'Rollback PDS error.', $orphans[0]['rollback_error'] ); } /** @@ -622,6 +632,76 @@ public function test_update_link_card_unchanged_single_post_uses_in_place_applyw $this->assertSame( 'doc-rkey-1', $writes[1]['rkey'] ); } + /** + * Update of a 2-post thread → 2-post thread uses in-place + * applyWrites#update for every record, preserving TIDs and URIs, + * and refreshes META_THREAD_RECORDS with the response CIDs. + */ + public function test_update_thread_in_place_when_record_counts_match() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'A Long-Form Post', + 'post_content' => 'Body content that is enough to compose a hook from.', + ) + ); + + $stored = array( + array( + 'uri' => 'at://did:plc:test123/app.bsky.feed.post/t-root', + 'cid' => 'bafyreibroot-old', + 'tid' => 't-root', + ), + array( + 'uri' => 'at://did:plc:test123/app.bsky.feed.post/t-reply', + 'cid' => 'bafyreibreply-old', + 'tid' => 't-reply', + ), + ); + \update_post_meta( $post->ID, Post::META_THREAD_RECORDS, $stored ); + \update_post_meta( $post->ID, Post::META_URI, $stored[0]['uri'] ); + \update_post_meta( $post->ID, Post::META_TID, 't-root' ); + \update_post_meta( $post->ID, Post::META_CID, 'bafyreibroot-old' ); + \update_post_meta( $post->ID, Document::META_URI, 'at://did:plc:test123/site.standard.document/doc-rkey-1' ); + \update_post_meta( $post->ID, Document::META_TID, 'doc-rkey-1' ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + + $this->fail_call_indexes = array(); + $this->register_capture( $post->ID ); + + $result = Publisher::update( $post ); + + $this->assertIsArray( $result ); + $this->assertCount( 1, $this->captured_calls, 'in-place thread update uses one applyWrites.' ); + + $writes = $this->captured_calls[0]['writes']; + $this->assertCount( 3, $writes, '2 bsky updates + 1 doc update.' ); + + $this->assertSame( 'com.atproto.repo.applyWrites#update', $writes[0]['$type'] ); + $this->assertSame( 'app.bsky.feed.post', $writes[0]['collection'] ); + $this->assertSame( 't-root', $writes[0]['rkey'] ); + $this->assertArrayNotHasKey( 'reply', $writes[0]['value'] ); + + $this->assertSame( 'com.atproto.repo.applyWrites#update', $writes[1]['$type'] ); + $this->assertSame( 't-reply', $writes[1]['rkey'] ); + $this->assertArrayHasKey( 'reply', $writes[1]['value'] ); + $this->assertSame( $stored[0]['uri'], $writes[1]['value']['reply']['root']['uri'] ); + $this->assertSame( $stored[0]['cid'], $writes[1]['value']['reply']['root']['cid'] ); + + $this->assertSame( 'site.standard.document', $writes[2]['collection'] ); + $this->assertSame( 'doc-rkey-1', $writes[2]['rkey'] ); + + // Thread meta was refreshed with the response CIDs; URIs and TIDs preserved. + $refreshed = \get_post_meta( $post->ID, Post::META_THREAD_RECORDS, true ); + $this->assertIsArray( $refreshed ); + $this->assertCount( 2, $refreshed ); + $this->assertSame( $stored[0]['uri'], $refreshed[0]['uri'] ); + $this->assertSame( 't-root', $refreshed[0]['tid'] ); + $this->assertNotSame( 'bafyreibroot-old', $refreshed[0]['cid'], 'Root CID should refresh to the response cid.' ); + $this->assertSame( $stored[1]['uri'], $refreshed[1]['uri'] ); + $this->assertSame( 't-reply', $refreshed[1]['tid'] ); + } + /** * Update with a stored 1-entry link-card but a teaser-thread composition * deletes the old record + doc atomically, then publishes fresh. diff --git a/tests/phpunit/tests/transformer/class-test-post.php b/tests/phpunit/tests/transformer/class-test-post.php index 4098d0b..e12b3df 100644 --- a/tests/phpunit/tests/transformer/class-test-post.php +++ b/tests/phpunit/tests/transformer/class-test-post.php @@ -405,6 +405,8 @@ public function test_build_long_form_records_teaser_thread_default_two_entries() 'post_title' => 'A Long Post', // 35 sentences × 10 chars = 350 chars; body exceeds the 280 hook budget. 'post_content' => \str_repeat( 'Hi there. ', 35 ), + // Force body-path hook; factory auto-fills "Post excerpt NNN" otherwise. + 'post_excerpt' => '', ) ); @@ -448,6 +450,8 @@ public function test_build_long_form_records_teaser_thread_hook_falls_back_to_wo 'post_title' => 'Unpunctuated', // 36 repetitions × 18 chars = 648 chars, no `.`/`!`/`?`. 'post_content' => \str_repeat( 'abcdefgh ijklmnop ', 36 ), + // Force body-path hook; factory auto-fills "Post excerpt NNN" otherwise. + 'post_excerpt' => '', ) ); @@ -585,6 +589,8 @@ public function test_build_long_form_records_facets_extracted_per_entry() { array( 'post_title' => 'Titled', 'post_content' => 'Read about #testing sensors in this detailed write-up on instrumentation.', + // Force body-path hook; factory auto-fills "Post excerpt NNN" otherwise. + 'post_excerpt' => '', ) ); From 75d7d28d776a247f56aa0b98e4fb1b3f8ed4dc6f Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Fri, 24 Apr 2026 11:35:46 -0500 Subject: [PATCH 08/22] Tests: assert hook ends at complete word, not just whitespace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The assertion `assertDoesNotMatch('~\S$~', $hook)` required the hook to end with whitespace, which contradicts `test_truncate_to_budget_falls_back_to_word_boundary_when_no_sentence` (expects 'The quick brown fox' with no trailing space). The test name and comment both say "not mid-word" — that's the actual intent. Since the corpus is built of 8-char words, a partial trailing word is any `\s\S{1,7}$` tail. --- tests/phpunit/tests/transformer/class-test-post.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/phpunit/tests/transformer/class-test-post.php b/tests/phpunit/tests/transformer/class-test-post.php index e12b3df..8b7c4c9 100644 --- a/tests/phpunit/tests/transformer/class-test-post.php +++ b/tests/phpunit/tests/transformer/class-test-post.php @@ -460,10 +460,12 @@ public function test_build_long_form_records_teaser_thread_hook_falls_back_to_wo $hook = ( new Post( $post ) )->build_long_form_records()[0]['text']; $this->assertLessThanOrEqual( 280, \mb_strlen( $hook ) ); + // Body is built of 8-char words, so a word-boundary cut must not + // leave a trailing run shorter than 8 chars. $this->assertDoesNotMatchRegularExpression( - '~\S$~', + '~\s\S{1,7}$~', $hook, - 'Hook should end at whitespace, not mid-word.' + 'Hook should end at a complete word, not mid-word.' ); } From 62bdee894d8d43a93f55a1206c7819b2187c82a1 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Fri, 24 Apr 2026 12:01:27 -0500 Subject: [PATCH 09/22] Pass 1 review fixes: reply createdAt, filter guards, URI index, tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidated findings from a 3-agent parallel review pass: - Unify reply `createdAt` across publish_thread and update_thread_in_place. Both now stamp every thread record with `post_date_gmt`, so backfill, fresh publish, and in-place update all agree on the post's publish date. Previously the publish path stamped replies with `wp_date('c')`, which would stamp replies with the backfill-run time. - Validate `atmosphere_pre_apply_writes` return. A filter that returns a scalar or object would fatal on the `array|\WP_Error` return type. Now normalized to a `WP_Error` with a clear code. - Harden `atmosphere_teaser_thread_posts`. Non-iterable or non-string entries fall back to the default pair; the list is capped at 5 to bound PDS rate-limit blast radius if a downstream returns something unreasonable. - Carry post tags on the thread root (index 0), matching the link-card and short-form record shape. Replies remain conversational and omit tags. - Index reply URIs for reaction sync. New `META_URI_INDEX` multi-row meta key mirrors every thread record URI; `find_post_by_bsky_uri` in Reaction_Sync now falls back to the index so a like or repost on a reply post resolves back to the originating WP post. - Cap `META_ORPHAN_RECORDS` at 10 entries so a crash-looping rollback can't grow the meta row past MySQL limits. - Align `has_composable_body` with `build_teaser_thread`: both now require an excerpt of ≥ 10 chars before preferring it over the body. - Memoize `render_post_content_plain()` per instance so thread composition doesn't re-run `the_content` multiple times per pass. - Tests: `delete_by_tids` with array/string/empty; `on_before_delete` thread-aware vs legacy vs no-meta scheduling; `META_URI_INDEX` is populated; `atmosphere_pre_apply_writes` malformed return surfaces as WP_Error; stale docblock references updated. --- includes/class-api.php | 12 ++- includes/class-publisher.php | 33 +++++-- includes/class-reaction-sync.php | 21 +++++ includes/transformer/class-base.php | 21 ++++- includes/transformer/class-post.php | 74 +++++++++++---- tests/phpunit/tests/class-test-publisher.php | 91 ++++++++++++++++++- .../tests/class-test-status-change.php | 84 +++++++++++++++++ 7 files changed, 309 insertions(+), 27 deletions(-) diff --git a/includes/class-api.php b/includes/class-api.php index b3c0346..e2f04cb 100644 --- a/includes/class-api.php +++ b/includes/class-api.php @@ -187,10 +187,20 @@ public static function apply_writes( array $writes ): array|\WP_Error { */ $short_circuit = \apply_filters( 'atmosphere_pre_apply_writes', null, $writes ); - if ( null !== $short_circuit ) { + if ( \is_array( $short_circuit ) || \is_wp_error( $short_circuit ) ) { return $short_circuit; } + if ( null !== $short_circuit ) { + // Malformed filter return (scalar / object / etc). Surface as a + // WP_Error instead of letting PHP fatal on the `array|\WP_Error` + // return type. + return new \WP_Error( + 'atmosphere_invalid_pre_apply_writes_return', + \__( 'atmosphere_pre_apply_writes must return null, an array, or a WP_Error.', 'atmosphere' ) + ); + } + return self::post( '/xrpc/com.atproto.repo.applyWrites', array( diff --git a/includes/class-publisher.php b/includes/class-publisher.php index 93fc065..c9b0b94 100644 --- a/includes/class-publisher.php +++ b/includes/class-publisher.php @@ -200,6 +200,7 @@ private static function publish_thread( } $thread_records = array( $root_triple ); + $created_at = to_iso8601( $post->post_date_gmt ); self::store_document_meta( $post->ID, $root_result, $doc_transformer ); self::mirror_thread_records_meta( $post->ID, $thread_records ); @@ -212,7 +213,7 @@ private static function publish_thread( $reply_rkey = TID::generate(); $reply_record = $records[ $i ]; - $reply_record['createdAt'] = \wp_date( 'c' ); + $reply_record['createdAt'] = $created_at; $reply_record['reply'] = array( 'root' => array( 'uri' => $thread_records[0]['uri'], @@ -378,6 +379,12 @@ private static function persist_orphan_records( $existing[] = $entry; + // Cap the manifest so a crash-looping cron can't grow the meta row + // past MySQL's max_allowed_packet. Most-recent entries win. + if ( \count( $existing ) > 10 ) { + $existing = \array_slice( $existing, -10 ); + } + \update_post_meta( $post_id, Post::META_ORPHAN_RECORDS, $existing ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log @@ -920,8 +927,9 @@ private static function stored_thread_records( int $post_id ): array { } /** - * Persist the thread-records meta and keep the root-mirrored single-record - * meta in sync with it. + * Persist the thread-records meta, mirror the root into the legacy + * single-record meta, and rebuild the flat per-URI index used by + * inbound reaction sync to resolve reply URIs back to the post. * * @param int $post_id Post ID. * @param array[] $thread_records Ordered thread records. @@ -929,6 +937,14 @@ private static function stored_thread_records( int $post_id ): array { private static function mirror_thread_records_meta( int $post_id, array $thread_records ): void { \update_post_meta( $post_id, Post::META_THREAD_RECORDS, $thread_records ); + // Rebuild the flat URI index so reaction sync can resolve replies. + \delete_post_meta( $post_id, Post::META_URI_INDEX ); + foreach ( $thread_records as $record ) { + if ( ! empty( $record['uri'] ) ) { + \add_post_meta( $post_id, Post::META_URI_INDEX, $record['uri'] ); + } + } + if ( empty( $thread_records ) ) { return; } @@ -946,11 +962,11 @@ private static function mirror_thread_records_meta( int $post_id, array $thread_ } /** - * Build a single { uri, cid, tid } triple from an `applyWrites` result - * entry, with sensible fallbacks when the PDS omits the URI (falling - * back to the transformer-computed URI) or when the caller needs a - * specific rkey (`Thread replies generate their own rkey so we can't - * derive it from `$item['uri']` reliably — we pass it in). + * Build a single { uri, cid, tid } triple from an `applyWrites` + * result entry. Falls back to the transformer-computed URI when the + * PDS response omits one. Callers pass the rkey in because thread + * replies are created with a freshly-generated TID that isn't + * recoverable from the response URI alone. * * @param array $result applyWrites response. * @param int $index Zero-based index into `$result['results']`. @@ -975,6 +991,7 @@ private static function build_triple_from_result( array $result, int $index, str */ private static function clear_all_record_meta( int $post_id ): void { \delete_post_meta( $post_id, Post::META_THREAD_RECORDS ); + \delete_post_meta( $post_id, Post::META_URI_INDEX ); \delete_post_meta( $post_id, Post::META_URI ); \delete_post_meta( $post_id, Post::META_TID ); \delete_post_meta( $post_id, Post::META_CID ); diff --git a/includes/class-reaction-sync.php b/includes/class-reaction-sync.php index a8299c6..d0918fe 100644 --- a/includes/class-reaction-sync.php +++ b/includes/class-reaction-sync.php @@ -594,6 +594,13 @@ private static function build_bsky_web_url( string $at_uri, string $handle ): st /** * Find a WordPress post by its Bluesky AT-URI. * + * Checks the single-record meta key first (fast, unique per post, + * covers every non-thread post) and falls back to the thread-URI + * index that Publisher populates for every record — root and + * every reply — under the `teaser-thread` strategy. Without the + * fallback, a like/repost targeting a reply post would silently + * fail to resolve back to the originating WordPress post. + * * @param string $uri AT-URI. * @return int|false */ @@ -612,6 +619,20 @@ private static function find_post_by_bsky_uri( string $uri ): int|false { ) ); + if ( ! empty( $posts ) ) { + return (int) $posts[0]; + } + + $posts = \get_posts( + array( + 'meta_key' => BskyPost::META_URI_INDEX, // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_key + 'meta_value' => $uri, // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_value + 'posts_per_page' => 1, + 'post_status' => 'publish', + 'fields' => 'ids', + ) + ); + return ! empty( $posts ) ? (int) $posts[0] : false; } diff --git a/includes/transformer/class-base.php b/includes/transformer/class-base.php index 422da1b..ddb40bb 100644 --- a/includes/transformer/class-base.php +++ b/includes/transformer/class-base.php @@ -134,19 +134,38 @@ protected function get_excerpt( \WP_Post $post, int $word_limit = 30 ): string { return \wp_trim_words( sanitize_text( $post->post_content ), $word_limit, '...' ); } + /** + * Cache of `render_post_content_plain()` output keyed by post ID. + * + * Per-instance memoization; `the_content` filter chains can be + * expensive, and long-form composition may touch a post's plain + * text from multiple helpers inside a single publish pass. + * + * @var array + */ + private array $plain_content_cache = array(); + /** * Render a post's content to plain text. * * Runs the_content filter, strips tags, decodes entities, and * collapses whitespace. Shared by short-form Bluesky post * composition and the document record's textContent field. + * Memoized per post ID to avoid re-running the filter chain. * * @param \WP_Post $post Post object. * @return string */ protected function render_post_content_plain( \WP_Post $post ): string { + if ( isset( $this->plain_content_cache[ $post->ID ] ) ) { + return $this->plain_content_cache[ $post->ID ]; + } + $content = \apply_filters( 'the_content', $post->post_content ); // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound -- Core WordPress filter. + $plain = sanitize_text( $content ); + + $this->plain_content_cache[ $post->ID ] = $plain; - return sanitize_text( $content ); + return $plain; } } diff --git a/includes/transformer/class-post.php b/includes/transformer/class-post.php index c8d33ea..aad14cc 100644 --- a/includes/transformer/class-post.php +++ b/includes/transformer/class-post.php @@ -57,6 +57,18 @@ class Post extends Base { */ public const META_THREAD_RECORDS = '_atmosphere_bsky_thread_records'; + /** + * Multi-row post meta key indexing every Bluesky record URI tied + * to the post — root and every reply — so inbound reaction sync + * can resolve a `subject.uri` that targets a reply post back to + * the parent WordPress post. `META_URI` still holds the root for + * backwards compatibility; this key adds one row per URI, + * populated by Publisher on every successful publish / update. + * + * @var string + */ + public const META_URI_INDEX = '_atmosphere_bsky_uri_index'; + /** * Post meta key for thread records left orphaned on the PDS after a * rollback failure. @@ -405,13 +417,13 @@ public function build_long_form_records(): array { switch ( $strategy ) { case 'teaser-thread': $records = array(); - foreach ( $this->build_teaser_thread() as $text ) { - $records[] = $this->record_for_thread_entry( (string) $text ); + foreach ( $this->build_teaser_thread() as $i => $text ) { + $records[] = $this->record_for_thread_entry( (string) $text, 0 === $i ); } return $records; case 'truncate-link': - return array( $this->record_for_thread_entry( $this->build_truncate_link_text() ) ); + return array( $this->record_for_thread_entry( $this->build_truncate_link_text(), true ) ); case 'link-card': default: @@ -517,8 +529,10 @@ private function build_truncate_link_text(): string { * @return string[] Text of each post in order. At least 2 entries. */ private function build_teaser_thread(): array { - if ( ! empty( $this->object->post_excerpt ) ) { - $hook = $this->truncate_to_budget( sanitize_text( $this->object->post_excerpt ), 300, false ); + $excerpt = sanitize_text( (string) $this->object->post_excerpt ); + + if ( \mb_strlen( $excerpt ) >= 10 ) { + $hook = $this->truncate_to_budget( $excerpt, 300, false ); } else { $plain = $this->render_post_content_plain( $this->object ); $hook = $this->truncate_to_budget( $plain, 280, true ); @@ -536,7 +550,29 @@ private function build_teaser_thread(): array { * @param string[] $posts 2-entry array: [ hook, cta ]. * @param \WP_Post $post The post being composed. */ - return \apply_filters( 'atmosphere_teaser_thread_posts', array( $hook, $cta ), $this->object ); + $filtered = \apply_filters( 'atmosphere_teaser_thread_posts', array( $hook, $cta ), $this->object ); + + // Defensive: a filter that returns a non-iterable or non-string + // entries would otherwise fatal on the caller's foreach. Fall + // back to the default pair on anything unexpected. + if ( ! \is_array( $filtered ) || empty( $filtered ) ) { + return array( $hook, $cta ); + } + + $texts = array(); + foreach ( $filtered as $entry ) { + if ( \is_string( $entry ) && '' !== $entry ) { + $texts[] = $entry; + } + } + + if ( empty( $texts ) ) { + return array( $hook, $cta ); + } + + // Cap at 5 to contain PDS rate-limit blast radius on mid-thread + // failure (which triggers N compensating deletes). + return \array_slice( $texts, 0, 5 ); } /** @@ -563,20 +599,19 @@ private function has_composable_body(): bool { * * `createdAt` and `reply` are intentionally omitted — Publisher * stamps both at write time so every record in a thread carries - * a fresh timestamp and a reply-ref to the already-written root. + * a timestamp pinned to the post's publish date and (for replies) + * a reply-ref to the already-written root. * - * The `atmosphere_transform_bsky_post` filter fires **once per - * thread entry** for the thread strategies. Downstream consumers - * that assumed single-post semantics should branch on record - * shape (e.g. presence of `reply` is not yet set here, but the - * caller can check `$record['text']` against the CTA/permalink - * shape, or filter on `atmosphere_long_form_composition` - * upstream) to decide whether to transform every record. + * The root entry (`$is_root === true`) carries the post's `tags`, + * mirroring `record_for_link_card()` and `transform()` — the root + * is the indexed representation of the WP post for the Bluesky + * algorithm. Non-root replies are conversational and omit tags. * - * @param string $text Pre-composed post text. + * @param string $text Pre-composed post text. + * @param bool $is_root Whether this record is the thread root. * @return array Bsky post record (no createdAt, no reply). */ - private function record_for_thread_entry( string $text ): array { + private function record_for_thread_entry( string $text, bool $is_root = false ): array { $record = array( '$type' => 'app.bsky.feed.post', 'text' => $text, @@ -588,6 +623,13 @@ private function record_for_thread_entry( string $text ): array { $record['facets'] = $facets; } + if ( $is_root ) { + $tags = $this->collect_tags( $this->object ); + if ( ! empty( $tags ) ) { + $record['tags'] = $tags; + } + } + /** This filter is documented in Post::transform() above. */ return \apply_filters( 'atmosphere_transform_bsky_post', $record, $this->object ); } diff --git a/tests/phpunit/tests/class-test-publisher.php b/tests/phpunit/tests/class-test-publisher.php index c96960f..5adc76f 100644 --- a/tests/phpunit/tests/class-test-publisher.php +++ b/tests/phpunit/tests/class-test-publisher.php @@ -63,7 +63,7 @@ public function tear_down(): void { * * One result per write, with a stable URI + CID derived from the * write's collection + rkey. Delete writes produce empty result - * entries so Publisher's `store_results()` treats them as no-ops. + * entries so Publisher's `store_document_meta()` treats them as no-ops. * * @param array $writes Write batch. * @return array applyWrites response shape. @@ -487,6 +487,14 @@ public function test_publish_teaser_thread_final_meta_has_ordered_triples() { $this->assertSame( $thread_records[0]['uri'], \get_post_meta( $post->ID, Post::META_URI, true ) ); $this->assertSame( $thread_records[0]['tid'], \get_post_meta( $post->ID, Post::META_TID, true ) ); $this->assertSame( $thread_records[0]['cid'], \get_post_meta( $post->ID, Post::META_CID, true ) ); + + // Flat URI index carries every record URI so reaction sync + // can resolve reply URIs back to this post. + $indexed = \get_post_meta( $post->ID, Post::META_URI_INDEX, false ); + $this->assertIsArray( $indexed ); + $this->assertCount( 2, $indexed ); + $this->assertContains( $thread_records[0]['uri'], $indexed ); + $this->assertContains( $thread_records[1]['uri'], $indexed ); } /** @@ -814,6 +822,87 @@ public function test_delete_thread_removes_all_records() { $this->assertSame( '', \get_post_meta( $post->ID, Document::META_TID, true ) ); } + /** + * Publisher::delete_by_tids accepts an array of bsky TIDs and + * issues one applyWrites covering every bsky delete + the doc. + */ + public function test_delete_by_tids_array_of_bsky_tids() { + $this->fail_call_indexes = array(); + $this->register_capture( 0 ); + + $result = \Atmosphere\Publisher::delete_by_tids( + array( 't-root', 't-r1', 't-r2' ), + 'doc-tid' + ); + + $this->assertIsArray( $result ); + $this->assertCount( 1, $this->captured_calls ); + + $writes = $this->captured_calls[0]['writes']; + $this->assertCount( 4, $writes ); + $this->assertSame( 't-root', $writes[0]['rkey'] ); + $this->assertSame( 't-r1', $writes[1]['rkey'] ); + $this->assertSame( 't-r2', $writes[2]['rkey'] ); + $this->assertSame( 'site.standard.document', $writes[3]['collection'] ); + $this->assertSame( 'doc-tid', $writes[3]['rkey'] ); + } + + /** + * Publisher::delete_by_tids with a legacy string argument still + * produces a single-bsky-delete batch — backwards compatibility for + * cron events queued before the signature change. + */ + public function test_delete_by_tids_legacy_string_argument() { + $this->fail_call_indexes = array(); + $this->register_capture( 0 ); + + $result = \Atmosphere\Publisher::delete_by_tids( 'legacy-tid', 'doc-tid' ); + + $this->assertIsArray( $result ); + $this->assertCount( 1, $this->captured_calls ); + + $writes = $this->captured_calls[0]['writes']; + $this->assertCount( 2, $writes ); + $this->assertSame( 'legacy-tid', $writes[0]['rkey'] ); + $this->assertSame( 'doc-tid', $writes[1]['rkey'] ); + } + + /** + * Publisher::delete_by_tids with empty inputs errors without + * making any API call. + */ + public function test_delete_by_tids_empty_inputs_error() { + $this->fail_call_indexes = array(); + $this->register_capture( 0 ); + + $result = \Atmosphere\Publisher::delete_by_tids( array(), '' ); + + $this->assertWPError( $result ); + $this->assertSame( 'atmosphere_not_published', $result->get_error_code() ); + $this->assertCount( 0, $this->captured_calls, 'No API call should be made.' ); + } + + /** + * A malformed atmosphere_pre_apply_writes return (scalar, object) + * surfaces as a WP_Error instead of fatal-ing on the return type. + */ + public function test_pre_apply_writes_malformed_return_surfaces_wp_error() { + \add_filter( 'atmosphere_pre_apply_writes', fn() => true ); + + $result = \Atmosphere\API::apply_writes( + array( + array( + '$type' => 'com.atproto.repo.applyWrites#delete', + 'collection' => 'x', + 'rkey' => 'y', + ), + ) + ); + + $this->assertWPError( $result ); + $this->assertSame( 'atmosphere_invalid_pre_apply_writes_return', $result->get_error_code() ); + } + /** * A post with only legacy single-record meta (no META_THREAD_RECORDS) * still deletes correctly via the fallback path. diff --git a/tests/phpunit/tests/class-test-status-change.php b/tests/phpunit/tests/class-test-status-change.php index e2f7534..1a4c98c 100644 --- a/tests/phpunit/tests/class-test-status-change.php +++ b/tests/phpunit/tests/class-test-status-change.php @@ -284,4 +284,88 @@ public function test_disconnected_state_prevents_scheduling() { 'Disconnected state must prevent scheduling.' ); } + + /** + * Force-deleting a thread-strategy post schedules a single + * atmosphere_delete_records cron event with every bsky TID in the + * thread, so the whole thread is cleaned up by one applyWrites. + */ + public function test_before_delete_thread_post_schedules_all_tids() { + $post = self::factory()->post->create_and_get( + array( 'post_status' => 'publish' ) + ); + + $thread = array( + array( + 'uri' => 'at://did:plc:test/app.bsky.feed.post/t-root', + 'cid' => 'c1', + 'tid' => 't-root', + ), + array( + 'uri' => 'at://did:plc:test/app.bsky.feed.post/t-r1', + 'cid' => 'c2', + 'tid' => 't-r1', + ), + array( + 'uri' => 'at://did:plc:test/app.bsky.feed.post/t-r2', + 'cid' => 'c3', + 'tid' => 't-r2', + ), + ); + \update_post_meta( $post->ID, Post::META_THREAD_RECORDS, $thread ); + \update_post_meta( $post->ID, Post::META_TID, 't-root' ); + \update_post_meta( $post->ID, Document::META_TID, 'doc-tid' ); + + $this->atmosphere->on_before_delete( $post->ID ); + + $scheduled = \wp_next_scheduled( + 'atmosphere_delete_records', + array( array( 't-root', 't-r1', 't-r2' ), 'doc-tid' ) + ); + $this->assertNotFalse( + $scheduled, + 'Expected atmosphere_delete_records to be scheduled with every thread tid.' + ); + } + + /** + * Force-deleting a legacy single-record post (no META_THREAD_RECORDS) + * schedules a cron event with a 1-element bsky tid array. + */ + public function test_before_delete_legacy_post_schedules_single_tid_array() { + $post = self::factory()->post->create_and_get( + array( 'post_status' => 'publish' ) + ); + + \update_post_meta( $post->ID, Post::META_TID, 'legacy-tid' ); + \update_post_meta( $post->ID, Document::META_TID, 'doc-tid' ); + + $this->atmosphere->on_before_delete( $post->ID ); + + $this->assertNotFalse( + \wp_next_scheduled( + 'atmosphere_delete_records', + array( array( 'legacy-tid' ), 'doc-tid' ) + ), + 'Expected atmosphere_delete_records to be scheduled with a 1-element tid array.' + ); + } + + /** + * A post with no AT Protocol meta schedules nothing on before-delete. + */ + public function test_before_delete_unpublished_post_schedules_nothing() { + $post = self::factory()->post->create_and_get( + array( 'post_status' => 'publish' ) + ); + + \wp_clear_scheduled_hook( 'atmosphere_delete_records' ); + + $this->atmosphere->on_before_delete( $post->ID ); + + $this->assertFalse( + \wp_next_scheduled( 'atmosphere_delete_records' ), + 'No atmosphere_delete_records event should be scheduled for a post with no AT Protocol meta.' + ); + } } From 2e2925e48fc244782793e1123a1b1de25ad79043 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Fri, 24 Apr 2026 15:13:20 -0500 Subject: [PATCH 10/22] Roll back thread publish when the root CID is missing from the PDS response Previously publish_thread returned a bare WP_Error on this path, leaving the root + doc records live on the PDS while the local TID was still stored from get_rkey(). A retry reused the same TID and the PDS rejected the create, stuck state. Route through rollback_thread so retry starts from a clean slate; on rollback failure the orphan manifest surfaces as for any other thread rollback. --- includes/class-publisher.php | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/includes/class-publisher.php b/includes/class-publisher.php index c9b0b94..eb07282 100644 --- a/includes/class-publisher.php +++ b/includes/class-publisher.php @@ -193,9 +193,17 @@ private static function publish_thread( ); if ( empty( $root_triple['cid'] ) ) { - return new \WP_Error( - 'atmosphere_missing_cid', - \__( 'Root post created but PDS response lacked a CID; cannot chain thread replies.', 'atmosphere' ) + // Root + doc were written, but without a CID we can't chain + // replies. Roll back so a retry starts from a clean slate + // instead of hitting "record already exists" on the same TID. + return self::rollback_thread( + $post, + array( $root_triple ), + $doc_transformer, + new \WP_Error( + 'atmosphere_missing_cid', + \__( 'Root post created but PDS response lacked a CID; rolling back thread.', 'atmosphere' ) + ) ); } From 46017140f67a250eba7666472e6a72647514a5fd Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Fri, 24 Apr 2026 15:29:44 -0500 Subject: [PATCH 11/22] Codex adversarial review: harden rewrite_thread and partial-state recovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two real findings from an adversarial pass that Pass 1/Pass 2 missed: 1. rewrite_thread deleted remote records before proving the replacement publish would succeed. If the republish failed (transient PDS error, rate limit, malformed payload), the old thread was gone from Bluesky, the new one was absent, and local meta was already cleared. The deleted records themselves are unrecoverable, but we now persist a rewrite-phase manifest to META_ORPHAN_RECORDS so operators have an audit trail, and retry of update() self-heals via publish() (stored records are cleared, so new TIDs are generated). 2. update()'s "missing document URI" repair branch called publish() directly, which reuses the existing bsky TID via get_rkey() — the PDS then rejected the create as "already exists." Self-heal was broken. Now routes through rewrite_thread with an empty doc_tid, which deletes the orphan bsky records and republishes fresh. rewrite_thread now also guards against empty tids and empty doc_tid so it can handle the partial-state recovery shape. Tests cover both paths. --- includes/class-publisher.php | 102 +++++++++++++--- tests/phpunit/tests/class-test-publisher.php | 118 +++++++++++++++++++ 2 files changed, 206 insertions(+), 14 deletions(-) diff --git a/includes/class-publisher.php b/includes/class-publisher.php index eb07282..72556d0 100644 --- a/includes/class-publisher.php +++ b/includes/class-publisher.php @@ -446,9 +446,13 @@ public static function update( \WP_Post $post ): array|\WP_Error { $doc_tid = \get_post_meta( $post->ID, Document::META_TID, true ); if ( ! $doc_uri ) { - // Partial state: bsky exists but doc never did. Safer to - // republish than to patch around missing doc. - return self::publish( $post ); + // Partial state: bsky exists but the document never did. + // Calling publish() directly here would reuse the existing + // bsky TID via get_rkey() and the PDS would reject the + // create as already-existing. Route through rewrite_thread + // with an empty doc_tid so the existing bsky records are + // deleted before we republish with fresh TIDs. + return self::rewrite_thread( $post, $stored, '' ); } if ( ! $doc_tid ) { @@ -656,37 +660,107 @@ private static function update_thread_in_place( * or when a thread updates to a thread with a different record count. * * The local meta is cleared between delete and publish so `publish()` - * sees a clean slate. + * sees a clean slate. If the republish step fails after the delete + * succeeded, the pre-rewrite manifest is persisted to + * `Post::META_ORPHAN_RECORDS` (marked `phase: rewrite`) so operators + * can see what was lost — a subsequent retry of `update()` sees + * empty stored records and goes straight to `publish()`, which + * self-heals with fresh TIDs. * * @param \WP_Post $post WordPress post. * @param array[] $stored Stored thread records (may be 1-entry). - * @param string $doc_tid Document record TID. + * @param string $doc_tid Document record TID (may be empty when + * recovering from a partial state where the + * bsky records exist but the doc never did). * @return array|\WP_Error */ private static function rewrite_thread( \WP_Post $post, array $stored, string $doc_tid ): array|\WP_Error { $delete_writes = array(); foreach ( $stored as $record ) { + if ( empty( $record['tid'] ) ) { + continue; + } $delete_writes[] = array( '$type' => 'com.atproto.repo.applyWrites#delete', 'collection' => 'app.bsky.feed.post', 'rkey' => $record['tid'], ); } - $delete_writes[] = array( - '$type' => 'com.atproto.repo.applyWrites#delete', - 'collection' => 'site.standard.document', - 'rkey' => $doc_tid, - ); + if ( '' !== $doc_tid ) { + $delete_writes[] = array( + '$type' => 'com.atproto.repo.applyWrites#delete', + 'collection' => 'site.standard.document', + 'rkey' => $doc_tid, + ); + } - $delete_result = API::apply_writes( $delete_writes ); + if ( ! empty( $delete_writes ) ) { + $delete_result = API::apply_writes( $delete_writes ); - if ( \is_wp_error( $delete_result ) ) { - return $delete_result; + if ( \is_wp_error( $delete_result ) ) { + return $delete_result; + } } self::clear_all_record_meta( $post->ID ); - return self::publish( $post ); + $publish_result = self::publish( $post ); + + if ( \is_wp_error( $publish_result ) ) { + self::persist_rewrite_failure( $post->ID, $stored, $doc_tid, $publish_result ); + } + + return $publish_result; + } + + /** + * Record a rewrite-thread failure in the orphan manifest so + * operators can trace what was deleted before the republish + * step failed. The deleted records are genuinely gone from the + * PDS (no recovery is possible), but the manifest gives a + * durable trail for audit / user communication. + * + * @param int $post_id Post ID. + * @param array[] $pre_delete The thread records as they existed before delete. + * @param string $doc_tid Document TID that was deleted (may be empty). + * @param \WP_Error $publish_error The republish-step failure. + */ + private static function persist_rewrite_failure( + int $post_id, + array $pre_delete, + string $doc_tid, + \WP_Error $publish_error + ): void { + $entry = array( + 'phase' => 'rewrite', + 'stamp' => \gmdate( 'Y-m-d\TH:i:s.000\Z' ), + 'deleted_bsky' => $pre_delete, + 'deleted_doc' => $doc_tid, + 'publish_error' => $publish_error->get_error_message(), + ); + + $existing = \get_post_meta( $post_id, Post::META_ORPHAN_RECORDS, true ); + if ( ! \is_array( $existing ) ) { + $existing = array(); + } + + $existing[] = $entry; + + if ( \count( $existing ) > 10 ) { + $existing = \array_slice( $existing, -10 ); + } + + \update_post_meta( $post_id, Post::META_ORPHAN_RECORDS, $existing ); + + // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log + \error_log( + \sprintf( + '[atmosphere] rewrite_thread republish failed for post %d; deleted records logged to %s: %s', + $post_id, + Post::META_ORPHAN_RECORDS, + $publish_error->get_error_message() + ) + ); } /** diff --git a/tests/phpunit/tests/class-test-publisher.php b/tests/phpunit/tests/class-test-publisher.php index 5adc76f..3a2aedc 100644 --- a/tests/phpunit/tests/class-test-publisher.php +++ b/tests/phpunit/tests/class-test-publisher.php @@ -773,6 +773,124 @@ public function test_update_thread_rewrites_on_strategy_change() { $this->assertCount( 2, $thread_records ); } + /** + * When rewrite_thread deletes the old records successfully but the + * subsequent republish fails, the pre-delete manifest is persisted + * to META_ORPHAN_RECORDS (phase=rewrite) so operators can audit + * what was lost. Meta is cleared so the next retry self-heals via + * publish(). + */ + public function test_rewrite_thread_persists_manifest_on_republish_failure() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'A Long-Form Post', + 'post_content' => 'Body content.', + 'post_excerpt' => 'Teaser excerpt.', + ) + ); + + $root_uri = 'at://did:plc:test123/app.bsky.feed.post/stored-rkey-1'; + \update_post_meta( + $post->ID, + Post::META_THREAD_RECORDS, + array( + array( + 'uri' => $root_uri, + 'cid' => 'bafyreibstored', + 'tid' => 'stored-rkey-1', + ), + ) + ); + \update_post_meta( $post->ID, Post::META_URI, $root_uri ); + \update_post_meta( $post->ID, Post::META_TID, 'stored-rkey-1' ); + \update_post_meta( $post->ID, Post::META_CID, 'bafyreibstored' ); + \update_post_meta( $post->ID, Document::META_URI, 'at://did:plc:test123/site.standard.document/doc-rkey-1' ); + \update_post_meta( $post->ID, Document::META_TID, 'doc-rkey-1' ); + + // Force a strategy change (1-entry stored → 2-entry new). + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + + // Call 1 (the delete batch) succeeds, call 2 (the republish + // create of root + doc) fails. + $this->fail_call_indexes = array( + 2 => new \WP_Error( 'atmosphere_republish_failed', 'Republish PDS error.' ), + ); + $this->register_capture( $post->ID ); + + $result = \Atmosphere\Publisher::update( $post ); + + $this->assertWPError( $result ); + $this->assertSame( 'atmosphere_republish_failed', $result->get_error_code() ); + + // Active-record meta cleared so a retry self-heals. + $this->assertSame( '', \get_post_meta( $post->ID, Post::META_THREAD_RECORDS, true ) ); + $this->assertSame( '', \get_post_meta( $post->ID, Post::META_TID, true ) ); + + // Manifest persisted for operator visibility. + $orphans = \get_post_meta( $post->ID, Post::META_ORPHAN_RECORDS, true ); + $this->assertIsArray( $orphans ); + $this->assertCount( 1, $orphans ); + $this->assertSame( 'rewrite', $orphans[0]['phase'] ); + $this->assertSame( 'doc-rkey-1', $orphans[0]['deleted_doc'] ); + $this->assertSame( 'Republish PDS error.', $orphans[0]['publish_error'] ); + $this->assertCount( 1, $orphans[0]['deleted_bsky'] ); + $this->assertSame( 'stored-rkey-1', $orphans[0]['deleted_bsky'][0]['tid'] ); + } + + /** + * When bsky records exist but the document URI is missing (an + * anomalous partial state), update() deletes the orphan bsky records + * via rewrite_thread with an empty doc_tid before republishing + * fresh. Previously this branch called publish() directly, which + * reused existing TIDs and triggered "already exists" on the PDS. + */ + public function test_update_partial_state_missing_doc_uri_rewrites() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'A Long-Form Post', + 'post_content' => 'Body.', + 'post_excerpt' => 'Teaser excerpt.', + ) + ); + + $root_uri = 'at://did:plc:test123/app.bsky.feed.post/stored-rkey-1'; + \update_post_meta( + $post->ID, + Post::META_THREAD_RECORDS, + array( + array( + 'uri' => $root_uri, + 'cid' => 'bafyreibstored', + 'tid' => 'stored-rkey-1', + ), + ) + ); + \update_post_meta( $post->ID, Post::META_URI, $root_uri ); + \update_post_meta( $post->ID, Post::META_TID, 'stored-rkey-1' ); + // Deliberately no Document::META_URI or META_TID. + + $this->fail_call_indexes = array(); + $this->register_capture( $post->ID ); + + $result = \Atmosphere\Publisher::update( $post ); + + $this->assertIsArray( $result ); + $this->assertGreaterThanOrEqual( 2, \count( $this->captured_calls ) ); + + // First batch: delete the orphan bsky record, no doc delete. + $delete_writes = $this->captured_calls[0]['writes']; + $this->assertCount( 1, $delete_writes ); + $this->assertSame( 'com.atproto.repo.applyWrites#delete', $delete_writes[0]['$type'] ); + $this->assertSame( 'app.bsky.feed.post', $delete_writes[0]['collection'] ); + $this->assertSame( 'stored-rkey-1', $delete_writes[0]['rkey'] ); + + // Second batch: fresh publish with newly-generated TIDs + // (not the stale stored-rkey-1). + $publish_writes = $this->captured_calls[1]['writes']; + $this->assertSame( 'com.atproto.repo.applyWrites#create', $publish_writes[0]['$type'] ); + $this->assertNotSame( 'stored-rkey-1', $publish_writes[0]['rkey'] ); + } + /** * Deleting a thread-published post issues one atomic applyWrites with * every bsky delete + the doc delete, then clears all meta. From fbe2ea0ad375cd97ee3cd6e6ecac08550b788f20 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Fri, 24 Apr 2026 15:41:36 -0500 Subject: [PATCH 12/22] Legacy-fallback: treat a bare TID without a URI as "nothing published" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A TID without a URI means Transformer::get_rkey() reserved the rkey locally but the create never landed on the PDS — most commonly after a failed publish or a rewrite_thread republish that clears meta mid-step. stored_thread_records() previously returned such a bare TID as a recoverable record, which caused the next update() to attempt a delete of a non-existent PDS record and the retry to loop. Now the legacy fallback requires a non-empty URI, matching how publish() actually persists state. The reserved ghost TID is harmless: the next publish() reuses it (get_rkey returns the stored value) and the PDS accepts the create normally. --- includes/class-publisher.php | 8 +++++++- tests/phpunit/tests/class-test-publisher.php | 9 +++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/includes/class-publisher.php b/includes/class-publisher.php index 72556d0..37ed462 100644 --- a/includes/class-publisher.php +++ b/includes/class-publisher.php @@ -995,7 +995,13 @@ private static function stored_thread_records( int $post_id ): array { $tid = \get_post_meta( $post_id, Post::META_TID, true ); $cid = \get_post_meta( $post_id, Post::META_CID, true ); - if ( ! $uri && ! $tid ) { + // A bare TID without a URI means the rkey was reserved via + // Transformer::get_rkey() but no create ever succeeded on the + // PDS (e.g. a prior publish failed mid-step, or a rewrite_thread + // republish failed). Treat that as "nothing published" so the + // caller falls back to a fresh publish and the reserved TID is + // reused on the next attempt. + if ( ! $uri ) { return array(); } diff --git a/tests/phpunit/tests/class-test-publisher.php b/tests/phpunit/tests/class-test-publisher.php index 3a2aedc..43b4016 100644 --- a/tests/phpunit/tests/class-test-publisher.php +++ b/tests/phpunit/tests/class-test-publisher.php @@ -822,9 +822,14 @@ public function test_rewrite_thread_persists_manifest_on_republish_failure() { $this->assertWPError( $result ); $this->assertSame( 'atmosphere_republish_failed', $result->get_error_code() ); - // Active-record meta cleared so a retry self-heals. + // Active-record meta cleared so a retry self-heals. A fresh TID + // may have been reserved by the failed publish's get_rkey() — that's + // a harmless ghost; stored_thread_records's legacy fallback ignores + // bare TIDs without a URI so the retry goes through publish() and + // reuses the reserved TID. $this->assertSame( '', \get_post_meta( $post->ID, Post::META_THREAD_RECORDS, true ) ); - $this->assertSame( '', \get_post_meta( $post->ID, Post::META_TID, true ) ); + $this->assertSame( '', \get_post_meta( $post->ID, Post::META_URI, true ) ); + $this->assertSame( '', \get_post_meta( $post->ID, Post::META_CID, true ) ); // Manifest persisted for operator visibility. $orphans = \get_post_meta( $post->ID, Post::META_ORPHAN_RECORDS, true ); From 0e1e9cc099aa9deb52f3b7f01605994f3d713759 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Tue, 28 Apr 2026 11:12:19 -0500 Subject: [PATCH 13/22] Harden long-form publish review fixes --- .github/changelog/long-form-teaser-thread | 2 +- includes/class-api.php | 61 ++++++- includes/class-publisher.php | 50 ++++-- includes/transformer/class-post.php | 156 ++++++++++++++---- tests/phpunit/tests/class-test-publisher.php | 127 +++++++++++++- .../tests/class-test-reaction-sync.php | 15 ++ .../tests/transformer/class-test-post.php | 105 ++++++++++++ 7 files changed, 464 insertions(+), 52 deletions(-) diff --git a/.github/changelog/long-form-teaser-thread b/.github/changelog/long-form-teaser-thread index d50a854..010607e 100644 --- a/.github/changelog/long-form-teaser-thread +++ b/.github/changelog/long-form-teaser-thread @@ -1,4 +1,4 @@ Significance: minor Type: added -Long-form posts can now be published as a Bluesky thread. A new filter lets sites choose between a single link-card post (default, unchanged from today), a single post that combines body text with the permalink, or a 2-post teaser thread that leads with a hook and follows with a "continue reading" link. Post edits update every record in place when the shape of the post hasn't changed, so Bluesky URLs stay stable and people's replies don't get orphaned. If the shape of the post changes (for example, switching between a single post and a thread), the old records are replaced with fresh ones. +Long-form posts can now be published to Bluesky as a short thread that points readers back to the full article. Sites can keep the existing single-post behavior, publish a shortened text version with a link, or use a two-post teaser thread. When a threaded post is edited, ATmosphere updates the existing Bluesky posts when possible so links and replies stay connected. If the publishing format changes, ATmosphere replaces the old Bluesky posts with new ones. diff --git a/includes/class-api.php b/includes/class-api.php index e2f04cb..a3a199b 100644 --- a/includes/class-api.php +++ b/includes/class-api.php @@ -170,8 +170,9 @@ public static function apply_writes( array $writes ): array|\WP_Error { /** * Short-circuits the applyWrites call before it reaches the PDS. * - * Return a non-null array (success shape: `[ 'results' => [...] ]`) - * or a `WP_Error` to bypass the real HTTP round-trip. Used by + * Return a non-null array (success shape: `[ 'results' => [...] ]`, + * with one array result per write) or a `WP_Error` to bypass the + * real HTTP round-trip. Used by * the PHPUnit suite, the FOSSE end-to-end harness, and anything * else that needs to observe or mock a write batch without * actually hitting the PDS. @@ -187,10 +188,14 @@ public static function apply_writes( array $writes ): array|\WP_Error { */ $short_circuit = \apply_filters( 'atmosphere_pre_apply_writes', null, $writes ); - if ( \is_array( $short_circuit ) || \is_wp_error( $short_circuit ) ) { + if ( \is_wp_error( $short_circuit ) ) { return $short_circuit; } + if ( \is_array( $short_circuit ) ) { + return self::validate_apply_writes_response( $short_circuit, $writes ); + } + if ( null !== $short_circuit ) { // Malformed filter return (scalar / object / etc). Surface as a // WP_Error instead of letting PHP fatal on the `array|\WP_Error` @@ -210,6 +215,56 @@ public static function apply_writes( array $writes ): array|\WP_Error { ); } + /** + * Validate a short-circuited applyWrites success response. + * + * @param array $response Short-circuited applyWrites response. + * @param array $writes Write batch the response represents. + * @return array|\WP_Error + */ + private static function validate_apply_writes_response( array $response, array $writes ): array|\WP_Error { + if ( ! isset( $response['results'] ) + || ! \is_array( $response['results'] ) + || ! \array_is_list( $response['results'] ) + || \count( $response['results'] ) !== \count( $writes ) + ) { + return self::invalid_apply_writes_response(); + } + + foreach ( $response['results'] as $i => $result ) { + if ( ! \is_array( $result ) ) { + return self::invalid_apply_writes_response(); + } + + $type = $writes[ $i ]['$type'] ?? ''; + if ( \in_array( + $type, + array( + 'com.atproto.repo.applyWrites#create', + 'com.atproto.repo.applyWrites#update', + ), + true + ) && ( empty( $result['uri'] ) || empty( $result['cid'] ) ) + ) { + return self::invalid_apply_writes_response(); + } + } + + return $response; + } + + /** + * Build a consistent malformed applyWrites response error. + * + * @return \WP_Error + */ + private static function invalid_apply_writes_response(): \WP_Error { + return new \WP_Error( + 'atmosphere_invalid_pre_apply_writes_response', + \__( 'atmosphere_pre_apply_writes success responses must include one results array entry for each write.', 'atmosphere' ) + ); + } + /** * Get a single record from the PDS. * diff --git a/includes/class-publisher.php b/includes/class-publisher.php index 37ed462..04a342c 100644 --- a/includes/class-publisher.php +++ b/includes/class-publisher.php @@ -129,7 +129,11 @@ private static function publish_single( ), ) ); - self::update_document_bsky_ref( $post, $doc_transformer ); + + $doc_ref_result = self::update_document_bsky_ref( $post, $doc_transformer ); + if ( \is_wp_error( $doc_ref_result ) ) { + return $doc_ref_result; + } return $result; } @@ -160,9 +164,11 @@ private static function publish_thread( Post $bsky_transformer, Document $doc_transformer ): array|\WP_Error { - $root_record = $records[0]; - $root_record['createdAt'] = to_iso8601( $post->post_date_gmt ); - $root_rkey = $bsky_transformer->get_rkey(); + $root_record = $records[0]; + if ( empty( $root_record['createdAt'] ) ) { + $root_record['createdAt'] = to_iso8601( $post->post_date_gmt ); + } + $root_rkey = $bsky_transformer->get_rkey(); $root_result = API::apply_writes( array( @@ -212,7 +218,11 @@ private static function publish_thread( self::store_document_meta( $post->ID, $root_result, $doc_transformer ); self::mirror_thread_records_meta( $post->ID, $thread_records ); - self::update_document_bsky_ref( $post, $doc_transformer ); + + $doc_ref_result = self::update_document_bsky_ref( $post, $doc_transformer ); + if ( \is_wp_error( $doc_ref_result ) ) { + return $doc_ref_result; + } $aggregated_results = $root_result['results'] ?? array(); @@ -221,8 +231,10 @@ private static function publish_thread( $reply_rkey = TID::generate(); $reply_record = $records[ $i ]; - $reply_record['createdAt'] = $created_at; - $reply_record['reply'] = array( + if ( empty( $reply_record['createdAt'] ) ) { + $reply_record['createdAt'] = $created_at; + } + $reply_record['reply'] = array( 'root' => array( 'uri' => $thread_records[0]['uri'], 'cid' => $thread_records[0]['cid'], @@ -258,7 +270,7 @@ private static function publish_thread( if ( empty( $reply_triple['cid'] ) ) { return self::rollback_thread( $post, - $thread_records, + \array_merge( $thread_records, array( $reply_triple ) ), $doc_transformer, new \WP_Error( 'atmosphere_missing_cid', @@ -553,7 +565,11 @@ private static function update_single( ), ) ); - self::update_document_bsky_ref( $post, $doc_transformer ); + + $doc_ref_result = self::update_document_bsky_ref( $post, $doc_transformer ); + if ( \is_wp_error( $doc_ref_result ) ) { + return $doc_ref_result; + } return $result; } @@ -592,7 +608,9 @@ private static function update_thread_in_place( $bsky_count = \count( $new_records ); foreach ( $new_records as $i => $record ) { - $record['createdAt'] = $created_at; + if ( empty( $record['createdAt'] ) ) { + $record['createdAt'] = $created_at; + } if ( $i > 0 ) { $record['reply'] = array( @@ -649,7 +667,10 @@ private static function update_thread_in_place( \update_post_meta( $post->ID, Document::META_CID, $doc_entry['cid'] ); } - self::update_document_bsky_ref( $post, $doc_transformer ); + $doc_ref_result = self::update_document_bsky_ref( $post, $doc_transformer ); + if ( \is_wp_error( $doc_ref_result ) ) { + return $doc_ref_result; + } return $result; } @@ -952,19 +973,20 @@ private static function store_document_meta( int $post_id, array $result, Docume * * @param \WP_Post $post WordPress post. * @param Document $doc_transformer Document transformer. + * @return array|\WP_Error|null */ - private static function update_document_bsky_ref( \WP_Post $post, Document $doc_transformer ): void { + private static function update_document_bsky_ref( \WP_Post $post, Document $doc_transformer ): array|\WP_Error|null { $bsky_uri = \get_post_meta( $post->ID, Post::META_URI, true ); $bsky_cid = \get_post_meta( $post->ID, Post::META_CID, true ); if ( ! $bsky_uri || ! $bsky_cid ) { - return; + return null; } $updated_doc = new Document( $post ); $record = $updated_doc->transform(); - API::post( + return API::post( '/xrpc/com.atproto.repo.putRecord', array( 'repo' => get_did(), diff --git a/includes/transformer/class-post.php b/includes/transformer/class-post.php index aad14cc..385707e 100644 --- a/includes/transformer/class-post.php +++ b/includes/transformer/class-post.php @@ -151,15 +151,23 @@ public function transform(): array { * For `teaser-thread`, the filter fires for *every* thread * entry (hook, intermediate posts, CTA). Listeners that * accumulate state across calls (rate-limit counters, external - * lint hooks) should branch on record shape (presence of - * `reply` or permalink-in-`text`) or inspect - * `atmosphere_long_form_composition` to decide per-entry - * behavior. + * lint hooks) should use the `$context` array to distinguish + * single-post output from teaser-thread entries. * * @param array $record Bsky post record. * @param \WP_Post $post WordPress post. + * @param array $context Additional composition context. */ - return \apply_filters( 'atmosphere_transform_bsky_post', $record, $this->object ); + return \apply_filters( + 'atmosphere_transform_bsky_post', + $record, + $this->object, + array( + 'strategy' => $is_short ? 'short-form' : 'link-card', + 'thread_index' => 0, + 'is_thread_reply' => false, + ) + ); } /** @@ -204,6 +212,12 @@ private function build_text(): string { $reserved = \mb_strlen( $permalink ) + 4; $available = 300 - $reserved; + if ( $available <= 0 ) { + $prose = \trim( $title . ( ! empty( $excerpt ) ? "\n\n" . $excerpt : '' ) ); + + return '' !== $prose ? truncate_text( $prose, 300 ) : truncate_text( $permalink, 300 ); + } + $prose = $title; if ( ! empty( $excerpt ) ) { $prose .= "\n\n" . $excerpt; @@ -364,9 +378,11 @@ public function is_short_form_post(): bool { * `'link-card'` and an error_log notice is emitted so operators * can tell the fallback from an intentional configuration. * - * Publisher stamps `createdAt` (and `reply` for thread entries - * 1..N) at write time — they are intentionally absent from the - * returned records. + * Records carry `createdAt` before `atmosphere_transform_bsky_post` + * runs so filters see the same timestamp shape as `transform()`. + * Publisher fills `createdAt` only if a filter removes it, and adds + * `reply` refs for thread entries 1..N at write time after parent + * CIDs are known. * * `Post::transform()` is unchanged and remains the entry point * for the short-form path and for any legacy caller on today's @@ -416,14 +432,40 @@ public function build_long_form_records(): array { switch ( $strategy ) { case 'teaser-thread': + if ( $this->requires_link_card_for_long_permalink() ) { + return array( $this->record_for_link_card() ); + } + $records = array(); foreach ( $this->build_teaser_thread() as $i => $text ) { - $records[] = $this->record_for_thread_entry( (string) $text, 0 === $i ); + $records[] = $this->record_for_thread_entry( + (string) $text, + 0 === $i, + array( + 'strategy' => 'teaser-thread', + 'thread_index' => $i, + 'is_thread_reply' => 0 !== $i, + ) + ); } return $records; case 'truncate-link': - return array( $this->record_for_thread_entry( $this->build_truncate_link_text(), true ) ); + if ( $this->requires_link_card_for_long_permalink() ) { + return array( $this->record_for_link_card() ); + } + + return array( + $this->record_for_thread_entry( + $this->build_truncate_link_text(), + true, + array( + 'strategy' => 'truncate-link', + 'thread_index' => 0, + 'is_thread_reply' => false, + ) + ), + ); case 'link-card': default: @@ -454,10 +496,18 @@ public function build_long_form_records(): array { * @return string */ private function truncate_to_budget( string $text, int $max, bool $prefer_sentence = true ): string { + if ( $max <= 0 ) { + return ''; + } + if ( \mb_strlen( $text ) <= $max ) { return $text; } + if ( 1 === $max ) { + return '…'; + } + $clamped = \mb_substr( $text, 0, $max ); if ( $prefer_sentence @@ -482,6 +532,15 @@ private function truncate_to_budget( string $text, int $max, bool $prefer_senten return \mb_substr( $text, 0, \max( 1, $max - 1 ) ) . '…'; } + /** + * Whether the permalink is too long to place safely in post text. + * + * @return bool + */ + private function requires_link_card_for_long_permalink(): bool { + return \mb_strlen( \get_permalink( $this->object ) ) >= 300; + } + /** * Compose the single-post truncate-link text. * @@ -493,13 +552,24 @@ private function truncate_to_budget( string $text, int $max, bool $prefer_senten * @return string */ private function build_truncate_link_text(): string { - $permalink = \get_permalink( $this->object ); - $plain = $this->render_post_content_plain( $this->object ); - $budget = 300 - \mb_strlen( "\n\n" ) - \mb_strlen( $permalink ); + $max_length = 300; + $separator = "\n\n"; + $permalink = \get_permalink( $this->object ); + $plain = $this->render_post_content_plain( $this->object ); - $body = $this->truncate_to_budget( $plain, $budget, false ); + if ( \mb_strlen( $permalink ) >= $max_length ) { + return $this->truncate_to_budget( $permalink, $max_length, false ); + } + + $budget = $max_length - \mb_strlen( $permalink ); + + if ( $budget <= \mb_strlen( $separator ) ) { + return $permalink; + } + + $body = $this->truncate_to_budget( $plain, $budget - \mb_strlen( $separator ), false ); - return $body . "\n\n" . $permalink; + return $body . $separator . $permalink; } /** @@ -561,8 +631,11 @@ private function build_teaser_thread(): array { $texts = array(); foreach ( $filtered as $entry ) { - if ( \is_string( $entry ) && '' !== $entry ) { - $texts[] = $entry; + if ( \is_string( $entry ) ) { + $entry = sanitize_text( $entry ); + if ( '' !== $entry ) { + $texts[] = $this->truncate_to_budget( $entry, 300, false ); + } } } @@ -597,10 +670,8 @@ private function has_composable_body(): bool { /** * Build one thread-entry record (hook, intermediate, or CTA). * - * `createdAt` and `reply` are intentionally omitted — Publisher - * stamps both at write time so every record in a thread carries - * a timestamp pinned to the post's publish date and (for replies) - * a reply-ref to the already-written root. + * `reply` is intentionally omitted — Publisher stamps it at write + * time for non-root entries after the parent CID is known. * * The root entry (`$is_root === true`) carries the post's `tags`, * mirroring `record_for_link_card()` and `transform()` — the root @@ -609,13 +680,15 @@ private function has_composable_body(): bool { * * @param string $text Pre-composed post text. * @param bool $is_root Whether this record is the thread root. - * @return array Bsky post record (no createdAt, no reply). + * @param array $context Additional filter context. + * @return array Bsky post record (no reply). */ - private function record_for_thread_entry( string $text, bool $is_root = false ): array { + private function record_for_thread_entry( string $text, bool $is_root = false, array $context = array() ): array { $record = array( - '$type' => 'app.bsky.feed.post', - 'text' => $text, - 'langs' => $this->get_langs(), + '$type' => 'app.bsky.feed.post', + 'text' => $text, + 'createdAt' => $this->to_iso8601( $this->object->post_date_gmt ), + 'langs' => $this->get_langs(), ); $facets = Facet::extract( $text ); @@ -630,8 +703,17 @@ private function record_for_thread_entry( string $text, bool $is_root = false ): } } + $context = \wp_parse_args( + $context, + array( + 'strategy' => 'teaser-thread', + 'thread_index' => 0, + 'is_thread_reply' => ! $is_root, + ) + ); + /** This filter is documented in Post::transform() above. */ - return \apply_filters( 'atmosphere_transform_bsky_post', $record, $this->object ); + return \apply_filters( 'atmosphere_transform_bsky_post', $record, $this->object, $context ); } /** @@ -642,16 +724,17 @@ private function record_for_thread_entry( string $text, bool $is_root = false ): * can produce the same output when the composition filter * resolves to `'link-card'` (the default) or an unknown value. * - * @return array Bsky post record (no createdAt — Publisher stamps). + * @return array Bsky post record. */ private function record_for_link_card(): array { $text = $this->build_text(); $embed = $this->build_embed(); $record = array( - '$type' => 'app.bsky.feed.post', - 'text' => $text, - 'langs' => $this->get_langs(), + '$type' => 'app.bsky.feed.post', + 'text' => $text, + 'createdAt' => $this->to_iso8601( $this->object->post_date_gmt ), + 'langs' => $this->get_langs(), ); $facets = Facet::extract( $text ); @@ -669,6 +752,15 @@ private function record_for_link_card(): array { } /** This filter is documented in Post::transform() above. */ - return \apply_filters( 'atmosphere_transform_bsky_post', $record, $this->object ); + return \apply_filters( + 'atmosphere_transform_bsky_post', + $record, + $this->object, + array( + 'strategy' => 'link-card', + 'thread_index' => 0, + 'is_thread_reply' => false, + ) + ); } } diff --git a/tests/phpunit/tests/class-test-publisher.php b/tests/phpunit/tests/class-test-publisher.php index 43b4016..504340d 100644 --- a/tests/phpunit/tests/class-test-publisher.php +++ b/tests/phpunit/tests/class-test-publisher.php @@ -14,6 +14,8 @@ use WP_UnitTestCase; use Atmosphere\Publisher; +use Atmosphere\OAuth\DPoP; +use Atmosphere\OAuth\Encryption; use Atmosphere\Transformer\Post; use Atmosphere\Transformer\Document; @@ -32,13 +34,16 @@ public function set_up(): void { \update_option( 'atmosphere_connection', array( - 'access_token' => 'encrypted-token', + 'access_token' => Encryption::encrypt( 'test-token' ), 'did' => 'did:plc:test123', 'pds_endpoint' => 'https://pds.example.com', - 'dpop_jwk' => 'encrypted-jwk', + 'dpop_jwk' => Encryption::encrypt( (string) \wp_json_encode( DPoP::generate_key() ) ), + 'expires_at' => \time() + HOUR_IN_SECONDS, ) ); \update_option( 'atmosphere_did', 'did:plc:test123' ); + + \add_filter( 'pre_http_request', array( $this, 'mock_document_ref_update' ), 10, 3 ); } /** @@ -54,10 +59,39 @@ public function tear_down(): void { \remove_all_filters( 'atmosphere_teaser_thread_posts' ); \remove_all_filters( 'atmosphere_transform_bsky_post' ); \remove_all_filters( 'atmosphere_is_short_form_post' ); + \remove_filter( 'pre_http_request', array( $this, 'mock_document_ref_update' ), 10 ); parent::tear_down(); } + /** + * Mock follow-up document putRecord calls. + * + * @param false|array|\WP_Error $response Preemptive HTTP response. + * @param array $args Request args. + * @param string $url Request URL. + * @return false|array|\WP_Error + */ + public function mock_document_ref_update( $response, array $args, string $url ) { + if ( false !== $response ) { + return $response; + } + + if ( false === \strpos( $url, 'com.atproto.repo.putRecord' ) ) { + return $response; + } + + return array( + 'response' => array( 'code' => 200 ), + 'body' => \wp_json_encode( + array( + 'uri' => 'at://did:plc:test123/site.standard.document/doc-ref', + 'cid' => 'bafyreibdocref', + ) + ), + ); + } + /** * Synthesize a plausible applyWrites response for a batch of writes. * @@ -497,6 +531,45 @@ public function test_publish_teaser_thread_final_meta_has_ordered_triples() { $this->assertContains( $thread_records[1]['uri'], $indexed ); } + /** + * If the follow-up document update fails after the initial applyWrites, + * publish returns the error while preserving meta for a retry. + */ + public function test_publish_surfaces_document_ref_update_failure() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'A Long-Form Post', + 'post_content' => 'Body content.', + ) + ); + + \add_filter( + 'pre_http_request', + static function ( $response, $args, $url ) { + if ( false !== \strpos( $url, 'com.atproto.repo.putRecord' ) ) { + return new \WP_Error( 'atmosphere_doc_ref_failed', 'Document ref update failed.' ); + } + + return $response; + }, + 5, + 3 + ); + + $this->fail_call_indexes = array(); + $this->register_capture( $post->ID ); + + $result = Publisher::publish( $post ); + + $this->assertWPError( $result ); + $this->assertSame( 'atmosphere_doc_ref_failed', $result->get_error_code() ); + + $thread_records = \get_post_meta( $post->ID, Post::META_THREAD_RECORDS, true ); + $this->assertIsArray( $thread_records ); + $this->assertCount( 1, $thread_records ); + $this->assertNotEmpty( \get_post_meta( $post->ID, Document::META_URI, true ) ); + } + /** * When the reply write fails, issue compensating deletes for the root * + doc, clear every meta key, and return the original WP_Error. @@ -1026,6 +1099,56 @@ public function test_pre_apply_writes_malformed_return_surfaces_wp_error() { $this->assertSame( 'atmosphere_invalid_pre_apply_writes_return', $result->get_error_code() ); } + /** + * A malformed atmosphere_pre_apply_writes success array must include + * a results list matching the write batch. + */ + public function test_pre_apply_writes_malformed_success_array_surfaces_wp_error() { + \add_filter( 'atmosphere_pre_apply_writes', fn() => array( 'ok' => true ) ); + + $result = \Atmosphere\API::apply_writes( + array( + array( + '$type' => 'com.atproto.repo.applyWrites#delete', + 'collection' => 'x', + 'rkey' => 'y', + ), + ) + ); + + $this->assertWPError( $result ); + $this->assertSame( 'atmosphere_invalid_pre_apply_writes_response', $result->get_error_code() ); + } + + /** + * A create/update short-circuit result must include the URI and CID + * shape returned by the PDS. + */ + public function test_pre_apply_writes_create_result_without_uri_and_cid_surfaces_wp_error() { + \add_filter( + 'atmosphere_pre_apply_writes', + fn() => array( + 'results' => array( + array(), + ), + ) + ); + + $result = \Atmosphere\API::apply_writes( + array( + array( + '$type' => 'com.atproto.repo.applyWrites#create', + 'collection' => 'app.bsky.feed.post', + 'rkey' => 'abc', + 'value' => array( 'text' => 'Hello' ), + ), + ) + ); + + $this->assertWPError( $result ); + $this->assertSame( 'atmosphere_invalid_pre_apply_writes_response', $result->get_error_code() ); + } + /** * A post with only legacy single-record meta (no META_THREAD_RECORDS) * still deletes correctly via the fallback path. diff --git a/tests/phpunit/tests/class-test-reaction-sync.php b/tests/phpunit/tests/class-test-reaction-sync.php index e268c99..d1c0543 100644 --- a/tests/phpunit/tests/class-test-reaction-sync.php +++ b/tests/phpunit/tests/class-test-reaction-sync.php @@ -33,6 +33,21 @@ public function test_find_post_by_bsky_uri() { $this->assertSame( $post_id, $method->invoke( null, $uri ) ); } + /** + * Test that find_post_by_bsky_uri falls back to the thread URI index. + */ + public function test_find_post_by_bsky_uri_uses_thread_uri_index() { + $post_id = self::factory()->post->create(); + $reply_uri = 'at://did:plc:test123/app.bsky.feed.post/reply123'; + + \add_post_meta( $post_id, BskyPost::META_URI_INDEX, $reply_uri ); + + $method = new \ReflectionMethod( Reaction_Sync::class, 'find_post_by_bsky_uri' ); + $method->setAccessible( true ); + + $this->assertSame( $post_id, $method->invoke( null, $reply_uri ) ); + } + /** * Test that find_post_by_bsky_uri returns false for unknown URI. */ diff --git a/tests/phpunit/tests/transformer/class-test-post.php b/tests/phpunit/tests/transformer/class-test-post.php index 8b7c4c9..4b652e9 100644 --- a/tests/phpunit/tests/transformer/class-test-post.php +++ b/tests/phpunit/tests/transformer/class-test-post.php @@ -356,6 +356,49 @@ static function ( $record ) { } } + /** + * Long-form filters receive records with `createdAt` plus context for + * distinguishing thread entries before Publisher adds final reply refs. + * + * @covers ::build_long_form_records + */ + public function test_build_long_form_records_filter_receives_created_at_and_context() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'A Titled Post', + 'post_content' => 'Body sentence one. Body sentence two.', + ) + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + + $seen = array(); + \add_filter( + 'atmosphere_transform_bsky_post', + static function ( $record, $filtered_post, $context = array() ) use ( &$seen ) { + $seen[] = array( + 'createdAt' => $record['createdAt'] ?? '', + 'context' => $context, + ); + + return $record; + }, + 10, + 3 + ); + + ( new Post( $post ) )->build_long_form_records(); + + $this->assertCount( 2, $seen ); + $this->assertNotEmpty( $seen[0]['createdAt'] ); + $this->assertNotEmpty( $seen[1]['createdAt'] ); + $this->assertSame( 'teaser-thread', $seen[0]['context']['strategy'] ?? '' ); + $this->assertSame( 0, $seen[0]['context']['thread_index'] ?? null ); + $this->assertFalse( $seen[0]['context']['is_thread_reply'] ?? true ); + $this->assertSame( 1, $seen[1]['context']['thread_index'] ?? null ); + $this->assertTrue( $seen[1]['context']['is_thread_reply'] ?? false ); + } + /** * Truncate-link branch: single record, no embed, text ends with permalink, * and facets include a link covering the permalink. @@ -393,6 +436,68 @@ public function test_build_long_form_records_truncate_link_branch() { $this->assertTrue( $has_link_facet, 'Permalink should be captured by a link facet.' ); } + /** + * Truncate-link branch: an unusually long permalink must not push the + * final post text over Bluesky's 300-character limit. + * + * @covers ::build_long_form_records + */ + public function test_build_long_form_records_truncate_link_long_permalink_stays_under_limit() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'Titled', + 'post_content' => \str_repeat( 'Some body content. ', 20 ), + ) + ); + + $permalink_filter = static fn() => 'https://example.com/' . \str_repeat( 'a', 320 ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'truncate-link' ); + \add_filter( 'post_link', $permalink_filter ); + + try { + $records = ( new Post( $post ) )->build_long_form_records(); + } finally { + \remove_filter( 'post_link', $permalink_filter ); + } + + $this->assertCount( 1, $records ); + $this->assertLessThanOrEqual( 300, \mb_strlen( $records[0]['text'] ) ); + $this->assertArrayHasKey( 'embed', $records[0], 'Overlong inline permalinks should fall back to a link card.' ); + } + + /** + * Filtered teaser-thread entries are sanitized and clamped before + * they are turned into Bluesky records. + * + * @covers ::build_long_form_records + */ + public function test_build_long_form_records_teaser_thread_filter_entries_are_sanitized_and_clamped() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'Titled', + 'post_content' => 'Body content with enough prose to form a hook.', + ) + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + \add_filter( + 'atmosphere_teaser_thread_posts', + fn() => array( + '' . \str_repeat( 'A', 400 ) . '', + \str_repeat( 'B', 400 ), + ) + ); + + $records = ( new Post( $post ) )->build_long_form_records(); + + $this->assertCount( 2, $records ); + foreach ( $records as $record ) { + $this->assertLessThanOrEqual( 300, \mb_strlen( $record['text'] ) ); + $this->assertStringNotContainsString( '', $record['text'] ); + } + } + /** * Teaser-thread default: 2 entries, hook cut at sentence punctuation, * CTA starts with `Continue reading: `. From b92fd1c94c22e0f6f38432c4e2c5fd2c03736745 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Tue, 28 Apr 2026 12:52:13 -0500 Subject: [PATCH 14/22] Address review: doc-ref half-publish + downgrade hook + docblock drift - publish_thread: doc-ref putRecord failure between root+doc and replies is now best-effort (log + continue). Returning early left META_THREAD_RECORDS at length=1, and a follow-up edit treated that as a shape change and rewrote the already-published root URI/TID, invalidating likes/reposts/external replies pointing at it. - build_long_form_records: fire atmosphere_long_form_strategy_downgraded when the long-permalink path falls back to link-card, matching the empty-body downgrade so observability stays consistent. - publish_single docblock: createdAt is set by transform() and the long-form record builders; Publisher only backfills when missing. - Test coverage: publish_thread continues to write replies (and meta reflects the full thread) when the doc-ref putRecord fails. --- includes/class-publisher.php | 23 +++++++-- includes/transformer/class-post.php | 2 + tests/phpunit/tests/class-test-publisher.php | 54 ++++++++++++++++++++ 3 files changed, 75 insertions(+), 4 deletions(-) diff --git a/includes/class-publisher.php b/includes/class-publisher.php index 04a342c..59b1dcf 100644 --- a/includes/class-publisher.php +++ b/includes/class-publisher.php @@ -76,9 +76,10 @@ public static function publish( \WP_Post $post ): array|\WP_Error { * doesn't already carry one, so the Bluesky timeline mirrors the * WordPress publish date (critical for backfill — otherwise every * re-synced post would stamp with the backfill-run time and - * collapse chronological order). `transform()` already sets it; - * `build_long_form_records()` leaves it absent on purpose so a - * single policy lives here. + * collapse chronological order). `transform()` and the long-form + * record builders normally set it already; Publisher only backfills + * here when a record arrives without `createdAt` (for example, if a + * filter stripped it). * * @param \WP_Post $post WordPress post. * @param array $bsky_record Pre-composed bsky post record. @@ -219,9 +220,23 @@ private static function publish_thread( self::store_document_meta( $post->ID, $root_result, $doc_transformer ); self::mirror_thread_records_meta( $post->ID, $thread_records ); + // Best-effort: a doc-ref update failure must not abort thread + // publishing after the root + document are already created. + // Bailing here leaves META_THREAD_RECORDS at length=1, and a + // subsequent edit treats that as a shape change and rewrites + // the (already-published) root — invalidating likes/reposts/ + // external replies pointing at it. Log and continue; the + // doc-ref is metadata, the replies are user-visible content. $doc_ref_result = self::update_document_bsky_ref( $post, $doc_transformer ); if ( \is_wp_error( $doc_ref_result ) ) { - return $doc_ref_result; + // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log + \error_log( + \sprintf( + '[atmosphere] post %d: doc-ref update failed during thread publish (%s); continuing with replies', + $post->ID, + $doc_ref_result->get_error_code() + ) + ); } $aggregated_results = $root_result['results'] ?? array(); diff --git a/includes/transformer/class-post.php b/includes/transformer/class-post.php index 385707e..cb70011 100644 --- a/includes/transformer/class-post.php +++ b/includes/transformer/class-post.php @@ -433,6 +433,7 @@ public function build_long_form_records(): array { switch ( $strategy ) { case 'teaser-thread': if ( $this->requires_link_card_for_long_permalink() ) { + \do_action( 'atmosphere_long_form_strategy_downgraded', $this->object, $strategy, 'link-card' ); return array( $this->record_for_link_card() ); } @@ -452,6 +453,7 @@ public function build_long_form_records(): array { case 'truncate-link': if ( $this->requires_link_card_for_long_permalink() ) { + \do_action( 'atmosphere_long_form_strategy_downgraded', $this->object, $strategy, 'link-card' ); return array( $this->record_for_link_card() ); } diff --git a/tests/phpunit/tests/class-test-publisher.php b/tests/phpunit/tests/class-test-publisher.php index 504340d..b178819 100644 --- a/tests/phpunit/tests/class-test-publisher.php +++ b/tests/phpunit/tests/class-test-publisher.php @@ -570,6 +570,60 @@ static function ( $response, $args, $url ) { $this->assertNotEmpty( \get_post_meta( $post->ID, Document::META_URI, true ) ); } + /** + * In a thread publish, a failure on the doc-ref `putRecord` between + * step 1 (root + doc) and step 2+ (replies) must not abort the thread + * — otherwise META_THREAD_RECORDS sticks at length=1 and the next + * edit triggers a rewrite that replaces the already-published root + * URI/TID, invalidating likes/reposts/external replies. + * + * Best-effort: log the doc-ref failure, then continue writing replies. + */ + public function test_publish_thread_continues_when_doc_ref_update_fails() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'A Long-Form Post', + 'post_excerpt' => 'A curated excerpt long enough to compose a hook from.', + 'post_content' => 'Body content that has plenty to teaser from for a thread.', + ) + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + + $put_record_failure = static function ( $response, $args, $url ) { + if ( false !== \strpos( $url, 'com.atproto.repo.putRecord' ) ) { + return new \WP_Error( 'atmosphere_doc_ref_failed', 'Document ref update failed.' ); + } + return $response; + }; + + \add_filter( 'pre_http_request', $put_record_failure, 5, 3 ); + + try { + $this->fail_call_indexes = array(); + $this->register_capture( $post->ID ); + + $result = Publisher::publish( $post ); + + // Doc-ref failure is swallowed — overall publish succeeds. + $this->assertIsArray( $result ); + $this->assertArrayHasKey( 'results', $result ); + + // Both root + reply applyWrites batches went through (call 1 = root+doc, call 2 = reply). + $this->assertCount( 2, $this->captured_calls ); + + $thread_records = \get_post_meta( $post->ID, Post::META_THREAD_RECORDS, true ); + $this->assertIsArray( $thread_records ); + $this->assertCount( 2, $thread_records ); + foreach ( $thread_records as $record ) { + $this->assertNotEmpty( $record['uri'] ); + $this->assertNotEmpty( $record['cid'] ); + } + } finally { + \remove_filter( 'pre_http_request', $put_record_failure, 5 ); + } + } + /** * When the reply write fails, issue compensating deletes for the root * + doc, clear every meta key, and return the original WP_Error. From c464fda35e96f128687df6e86f21a2eeaa9ae29f Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Tue, 28 Apr 2026 13:01:46 -0500 Subject: [PATCH 15/22] Address review: atmosphere_teaser_thread_posts minimum-entry guard The filter docblock promises a multi-entry array, but a filter returning a single string yielded a 1-element array that quietly routed to publish_single() and dropped the CTA. Enforce the contract with _doing_it_wrong() and fall back to the default hook + CTA pair when fewer than 2 valid string entries come back, so the contract is visible to filter authors before this becomes public API. --- includes/transformer/class-post.php | 10 ++++++- .../tests/transformer/class-test-post.php | 28 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/includes/transformer/class-post.php b/includes/transformer/class-post.php index cb70011..cb0dc7b 100644 --- a/includes/transformer/class-post.php +++ b/includes/transformer/class-post.php @@ -641,7 +641,15 @@ private function build_teaser_thread(): array { } } - if ( empty( $texts ) ) { + // A 1-entry return would silently route to publish_single() and + // drop the CTA — confusing for filter authors who expected a + // thread. Enforce the docblock contract instead. + if ( \count( $texts ) < 2 ) { + \_doing_it_wrong( + 'atmosphere_teaser_thread_posts', + \esc_html__( 'The atmosphere_teaser_thread_posts filter must return at least 2 string entries; falling back to the default hook + CTA pair.', 'atmosphere' ), + 'unreleased' + ); return array( $hook, $cta ); } diff --git a/tests/phpunit/tests/transformer/class-test-post.php b/tests/phpunit/tests/transformer/class-test-post.php index 4b652e9..5bbc861 100644 --- a/tests/phpunit/tests/transformer/class-test-post.php +++ b/tests/phpunit/tests/transformer/class-test-post.php @@ -660,6 +660,34 @@ public function test_build_long_form_records_teaser_thread_filter_extends_to_thr $this->assertSame( 'Call to action link', $records[2]['text'] ); } + /** + * Filter that returns fewer than 2 entries should trigger + * _doing_it_wrong and fall back to the default hook + CTA pair — + * a 1-entry return would silently route to publish_single() and + * drop the CTA. + * + * @covers ::build_long_form_records + */ + public function test_build_long_form_records_teaser_thread_filter_under_two_falls_back() { + $this->setExpectedIncorrectUsage( 'atmosphere_teaser_thread_posts' ); + + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'Titled', + 'post_content' => 'Body content with enough prose to form a hook.', + ) + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + \add_filter( 'atmosphere_teaser_thread_posts', fn() => array( 'Just one entry' ) ); + + $records = ( new Post( $post ) )->build_long_form_records(); + + $this->assertCount( 2, $records ); + $this->assertNotSame( 'Just one entry', $records[0]['text'] ); + $this->assertMatchesRegularExpression( '~^Continue reading: ~', $records[1]['text'] ); + } + /** * Every record in a thread carries the same `langs` array. * From c6a86e87c8383c997f828457ffd1a750019905f6 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Tue, 28 Apr 2026 13:04:23 -0500 Subject: [PATCH 16/22] Address review: stop leaking pre_http_request filter from publisher tests test_publish_surfaces_document_ref_update_failure added a pre_http_request filter and never removed it; set_up()'s own filter already exists in the suite, so the leak could affect later tests in the class. Wrap in try/finally with a referenced callback so the filter is removed even if assertions throw. test_update_sends_update_writes was using pre_http_request to capture the applyWrites payload, with conditional assertions that no-op'd when the auth/DPoP layer blocked the request before reaching the filter. Switch to register_capture() (atmosphere_pre_apply_writes) so the test runs deterministically and assertions are unconditional. --- tests/phpunit/tests/class-test-publisher.php | 109 ++++++------------- 1 file changed, 34 insertions(+), 75 deletions(-) diff --git a/tests/phpunit/tests/class-test-publisher.php b/tests/phpunit/tests/class-test-publisher.php index b178819..f3c7192 100644 --- a/tests/phpunit/tests/class-test-publisher.php +++ b/tests/phpunit/tests/class-test-publisher.php @@ -251,6 +251,10 @@ public function test_update_errors_with_uris_but_no_tids() { /** * Test that update() sends applyWrites#update when URIs and TIDs exist. + * + * Uses the `atmosphere_pre_apply_writes` short-circuit so the test + * runs to completion regardless of the DPoP/auth state in the test + * environment, and assertions are unconditional. */ public function test_update_sends_update_writes() { $post = self::factory()->post->create_and_get( @@ -262,65 +266,20 @@ public function test_update_sends_update_writes() { \update_post_meta( $post->ID, Post::META_URI, 'at://did:plc:test/app.bsky.feed.post/bsky-tid-123' ); \update_post_meta( $post->ID, Document::META_URI, 'at://did:plc:test/site.standard.document/doc-tid-456' ); - /* - * Intercept the HTTP request to verify the payload contains - * #update operations (not #create or #delete). - */ - $captured_body = null; - - \add_filter( - 'pre_http_request', - static function ( $response, $args, $url ) use ( &$captured_body ) { - if ( false !== \strpos( $url, 'applyWrites' ) ) { - $captured_body = \json_decode( $args['body'], true ); - - return array( - 'response' => array( 'code' => 200 ), - 'body' => \wp_json_encode( - array( - 'results' => array( - array( - 'uri' => 'at://did:plc:test/app.bsky.feed.post/bsky-tid-123', - 'cid' => 'bafyreib-new-bsky-cid', - ), - array( - 'uri' => 'at://did:plc:test/site.standard.document/doc-tid-456', - 'cid' => 'bafyreib-new-doc-cid', - ), - ), - ) - ), - ); - } - - return $response; - }, - 10, - 3 - ); + $this->fail_call_indexes = array(); + $this->register_capture( $post->ID ); $result = Publisher::update( $post ); - \remove_all_filters( 'pre_http_request' ); + $this->assertIsArray( $result ); + $this->assertCount( 1, $this->captured_calls ); - /* - * The API call requires a valid access token and DPoP proof, - * which we can't easily mock. If the request reached our - * filter, captured_body will be set. If it failed before - * reaching the HTTP layer (auth errors), we still verify the - * flow didn't crash. - */ - if ( null !== $captured_body ) { - $this->assertIsArray( $captured_body['writes'] ); - $this->assertCount( 2, $captured_body['writes'] ); - $this->assertSame( 'com.atproto.repo.applyWrites#update', $captured_body['writes'][0]['$type'] ); - $this->assertSame( 'com.atproto.repo.applyWrites#update', $captured_body['writes'][1]['$type'] ); - $this->assertSame( 'bsky-tid-123', $captured_body['writes'][0]['rkey'] ); - $this->assertSame( 'doc-tid-456', $captured_body['writes'][1]['rkey'] ); - } else { - // Auth layer blocked the request — still verify no crash. - $this->assertWPError( $result ); - } + $writes = $this->captured_calls[0]['writes']; + $this->assertCount( 2, $writes ); + $this->assertSame( 'com.atproto.repo.applyWrites#update', $writes[0]['$type'] ); + $this->assertSame( 'com.atproto.repo.applyWrites#update', $writes[1]['$type'] ); + $this->assertSame( 'bsky-tid-123', $writes[0]['rkey'] ); + $this->assertSame( 'doc-tid-456', $writes[1]['rkey'] ); } /** @@ -543,31 +502,31 @@ public function test_publish_surfaces_document_ref_update_failure() { ) ); - \add_filter( - 'pre_http_request', - static function ( $response, $args, $url ) { - if ( false !== \strpos( $url, 'com.atproto.repo.putRecord' ) ) { - return new \WP_Error( 'atmosphere_doc_ref_failed', 'Document ref update failed.' ); - } + $put_record_failure = static function ( $response, $args, $url ) { + if ( false !== \strpos( $url, 'com.atproto.repo.putRecord' ) ) { + return new \WP_Error( 'atmosphere_doc_ref_failed', 'Document ref update failed.' ); + } + return $response; + }; - return $response; - }, - 5, - 3 - ); + \add_filter( 'pre_http_request', $put_record_failure, 5, 3 ); - $this->fail_call_indexes = array(); - $this->register_capture( $post->ID ); + try { + $this->fail_call_indexes = array(); + $this->register_capture( $post->ID ); - $result = Publisher::publish( $post ); + $result = Publisher::publish( $post ); - $this->assertWPError( $result ); - $this->assertSame( 'atmosphere_doc_ref_failed', $result->get_error_code() ); + $this->assertWPError( $result ); + $this->assertSame( 'atmosphere_doc_ref_failed', $result->get_error_code() ); - $thread_records = \get_post_meta( $post->ID, Post::META_THREAD_RECORDS, true ); - $this->assertIsArray( $thread_records ); - $this->assertCount( 1, $thread_records ); - $this->assertNotEmpty( \get_post_meta( $post->ID, Document::META_URI, true ) ); + $thread_records = \get_post_meta( $post->ID, Post::META_THREAD_RECORDS, true ); + $this->assertIsArray( $thread_records ); + $this->assertCount( 1, $thread_records ); + $this->assertNotEmpty( \get_post_meta( $post->ID, Document::META_URI, true ) ); + } finally { + \remove_filter( 'pre_http_request', $put_record_failure, 5 ); + } } /** From 14f21d2751c256853447ebc2c5204370cb9ad3cb Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Tue, 28 Apr 2026 13:08:34 -0500 Subject: [PATCH 17/22] Address review: cosmetic cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - update(): add a comment that count-only shape detection means a truncate-link → teaser-thread (which empty-body-downgrades to link-card) takes the in-place path; output is structurally correct because both end up as a single record. - update_document_bsky_ref(): drop the redundant `new Document( $post )` and reuse the existing transformer; transform() reads URI/CID fresh from post meta on every call. - persist_orphan_records(): TODO comment pointing at issue 44 (surface the manifest in admin / Site Health / WP-CLI). --- includes/class-publisher.php | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/includes/class-publisher.php b/includes/class-publisher.php index 59b1dcf..cc24b73 100644 --- a/includes/class-publisher.php +++ b/includes/class-publisher.php @@ -386,6 +386,10 @@ private static function rollback_thread( * machine-parseable summary. The post meta is the source of truth — * the log line is a convenience for ops grepping a filesystem tail. * + * TODO: surface the manifest in admin / Site Health / WP-CLI so + * orphans don't require a manual `get_post_meta()` to discover. + * Tracked in issue 44. + * * @param int $post_id WordPress post ID. * @param array[] $thread_records Thread records that survived rollback. * @param string $doc_rkey Document rkey that survived rollback. @@ -496,7 +500,12 @@ public static function update( \WP_Post $post ): array|\WP_Error { ? array( $bsky_transformer->transform() ) : $bsky_transformer->build_long_form_records(); - // In-place update: matching record counts. + // In-place update: matching record counts. Strategy is not + // compared — a `truncate-link` (count=1) post that switches to + // a `teaser-thread` whose empty-body guard downgrades to + // `link-card` (count=1) takes this path. Output is structurally + // correct because both end up as a single-record post; URIs and + // TIDs are reused on the bsky side. if ( \count( $stored ) === \count( $new_records ) ) { if ( 1 === \count( $stored ) ) { return self::update_single( @@ -998,8 +1007,11 @@ private static function update_document_bsky_ref( \WP_Post $post, Document $doc_ return null; } - $updated_doc = new Document( $post ); - $record = $updated_doc->transform(); + // `transform()` reads Post::META_URI / META_CID fresh from post + // meta on every call, so the existing transformer picks up the + // values just persisted by mirror_thread_records_meta() above — + // no need for a new instance. + $record = $doc_transformer->transform(); return API::post( '/xrpc/com.atproto.repo.putRecord', From f441deee566792b65b25c0d32c4d0c6168479e97 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Tue, 28 Apr 2026 13:22:42 -0500 Subject: [PATCH 18/22] Test: assert downgrade action fires on long-permalink fallback Adds coverage for the new atmosphere_long_form_strategy_downgraded action fire from the long-permalink branch in build_long_form_records. Mirrors the existing empty-body downgrade test. --- .../tests/transformer/class-test-post.php | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/phpunit/tests/transformer/class-test-post.php b/tests/phpunit/tests/transformer/class-test-post.php index 5bbc861..cd9ea08 100644 --- a/tests/phpunit/tests/transformer/class-test-post.php +++ b/tests/phpunit/tests/transformer/class-test-post.php @@ -633,6 +633,48 @@ function ( $downgrade_post, $requested, $effective ) use ( &$events ) { $this->assertSame( array( $post->ID, 'teaser-thread', 'link-card' ), $events[0] ); } + /** + * Long-permalink fallback: when the permalink alone is >= 300 chars, + * teaser-thread / truncate-link both fall back to link-card and fire + * the observability action so the downgrade is distinguishable from + * an intentional link-card configuration. + * + * @covers ::build_long_form_records + */ + public function test_build_long_form_records_long_permalink_fires_downgrade_action() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'Titled', + 'post_content' => \str_repeat( 'Some body content. ', 20 ), + ) + ); + + $permalink_filter = static fn() => 'https://example.com/' . \str_repeat( 'a', 320 ); + + $events = array(); + \add_action( + 'atmosphere_long_form_strategy_downgraded', + function ( $downgrade_post, $requested, $effective ) use ( &$events ) { + $events[] = array( $downgrade_post->ID, $requested, $effective ); + }, + 10, + 3 + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + \add_filter( 'post_link', $permalink_filter ); + + try { + $records = ( new Post( $post ) )->build_long_form_records(); + } finally { + \remove_filter( 'post_link', $permalink_filter ); + } + + $this->assertCount( 1, $records ); + $this->assertCount( 1, $events ); + $this->assertSame( array( $post->ID, 'teaser-thread', 'link-card' ), $events[0] ); + } + /** * Downstream filters may extend the thread to 3 posts. * From 8374793f17b3cd895d7742d78e34836f316151b2 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Tue, 28 Apr 2026 13:55:08 -0500 Subject: [PATCH 19/22] Codex review: downgrade teaser-thread when full CTA exceeds 300 chars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous guard checked only the bare permalink (`mb_strlen >= 300`), but the actual CTA text is `Continue reading: `. A permalink of e.g. 290 chars passed the guard yet produced a composed CTA of ~309 chars; truncate_to_budget then word-cut the over-budget URL fragment off and shipped a thread whose final post said `Continue reading:` with no link. Add `requires_link_card_for_teaser_thread()` that builds the localized CTA via a new `teaser_thread_cta_text()` helper and tests its mb_strlen. build_teaser_thread reuses the same helper so the guard and the actual composition operate on identical strings (locale changes can't desync them). truncate-link keeps `requires_link_card_for_long_permalink()` — its overflow equation is different. --- includes/transformer/class-post.php | 44 +++++++++++++++--- .../tests/transformer/class-test-post.php | 46 +++++++++++++++++++ 2 files changed, 84 insertions(+), 6 deletions(-) diff --git a/includes/transformer/class-post.php b/includes/transformer/class-post.php index cb0dc7b..964e5ec 100644 --- a/includes/transformer/class-post.php +++ b/includes/transformer/class-post.php @@ -432,7 +432,7 @@ public function build_long_form_records(): array { switch ( $strategy ) { case 'teaser-thread': - if ( $this->requires_link_card_for_long_permalink() ) { + if ( $this->requires_link_card_for_teaser_thread() ) { \do_action( 'atmosphere_long_form_strategy_downgraded', $this->object, $strategy, 'link-card' ); return array( $this->record_for_link_card() ); } @@ -537,12 +537,48 @@ private function truncate_to_budget( string $text, int $max, bool $prefer_senten /** * Whether the permalink is too long to place safely in post text. * + * Used by the `truncate-link` strategy where the post text is just + * `\n\n` and the permalink is the load-bearing part. + * * @return bool */ private function requires_link_card_for_long_permalink(): bool { return \mb_strlen( \get_permalink( $this->object ) ) >= 300; } + /** + * Whether the teaser-thread CTA can't carry the full permalink. + * + * The CTA text is `Continue reading: ` (localized — the + * prefix length varies by locale). If the composed CTA exceeds the + * 300-char post limit, `truncate_to_budget()` would word-cut the URL + * fragment off and ship a thread whose final post says + * `Continue reading:` with no link. Detect that case and bail to + * link-card instead. + * + * @return bool + */ + private function requires_link_card_for_teaser_thread(): bool { + return \mb_strlen( $this->teaser_thread_cta_text() ) > 300; + } + + /** + * Compose the default teaser-thread CTA text. + * + * Centralised so the overflow guard (`requires_link_card_for_teaser_thread`) + * and the actual thread builder (`build_teaser_thread`) operate on + * identical strings. + * + * @return string + */ + private function teaser_thread_cta_text(): string { + return \sprintf( + /* translators: %s: the WordPress post permalink. */ + \__( 'Continue reading: %s', 'atmosphere' ), + \get_permalink( $this->object ) + ); + } + /** * Compose the single-post truncate-link text. * @@ -610,11 +646,7 @@ private function build_teaser_thread(): array { $hook = $this->truncate_to_budget( $plain, 280, true ); } - $cta = \sprintf( - /* translators: %s: the WordPress post permalink. */ - \__( 'Continue reading: %s', 'atmosphere' ), - \get_permalink( $this->object ) - ); + $cta = $this->teaser_thread_cta_text(); /** * Filters the default teaser-thread post texts. diff --git a/tests/phpunit/tests/transformer/class-test-post.php b/tests/phpunit/tests/transformer/class-test-post.php index cd9ea08..3675a39 100644 --- a/tests/phpunit/tests/transformer/class-test-post.php +++ b/tests/phpunit/tests/transformer/class-test-post.php @@ -633,6 +633,52 @@ function ( $downgrade_post, $requested, $effective ) use ( &$events ) { $this->assertSame( array( $post->ID, 'teaser-thread', 'link-card' ), $events[0] ); } + /** + * Teaser-thread downgrades to link-card whenever the localized CTA + * (`Continue reading: `) exceeds 300 chars — even when + * the bare permalink is below the 300-char limit. Otherwise the CTA + * gets word-truncated and the URL fragment is dropped, shipping a + * thread whose call-to-action has no link. + * + * @covers ::build_long_form_records + */ + public function test_build_long_form_records_teaser_thread_downgrades_when_cta_overflows() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'Titled', + 'post_content' => \str_repeat( 'Some body content. ', 20 ), + ) + ); + + // Bare permalink under 300 chars but CTA "Continue reading: " + // pushes the composed text past the 300-char limit. + $permalink_filter = static fn() => 'https://example.com/' . \str_repeat( 'a', 280 ); + + $events = array(); + \add_action( + 'atmosphere_long_form_strategy_downgraded', + function ( $downgrade_post, $requested, $effective ) use ( &$events ) { + $events[] = array( $downgrade_post->ID, $requested, $effective ); + }, + 10, + 3 + ); + + \add_filter( 'atmosphere_long_form_composition', fn() => 'teaser-thread' ); + \add_filter( 'post_link', $permalink_filter ); + + try { + $records = ( new Post( $post ) )->build_long_form_records(); + } finally { + \remove_filter( 'post_link', $permalink_filter ); + } + + $this->assertCount( 1, $records ); + $this->assertArrayHasKey( 'embed', $records[0] ); + $this->assertCount( 1, $events ); + $this->assertSame( array( $post->ID, 'teaser-thread', 'link-card' ), $events[0] ); + } + /** * Long-permalink fallback: when the permalink alone is >= 300 chars, * teaser-thread / truncate-link both fall back to link-card and fire From 694f3c55203ab1b2e13340b7cbe6d33057b17c55 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Tue, 28 Apr 2026 13:58:52 -0500 Subject: [PATCH 20/22] Codex review: persist deferred doc-ref failure to META_DOC_REF_PENDING MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The doc-ref `putRecord` is best-effort within `publish_thread`, but swallowing the WP_Error and only writing to `error_log` left the gap invisible to backfill, the admin UI, and any future Site Health surface. Backfill callers would mark the post synced and never retry the missing cross-reference. Add Post::META_DOC_REF_PENDING (`{ stamp, code, message }`) and a small `record_doc_ref_pending()` helper that publish_thread invokes when the follow-up `putRecord` fails. The next successful `update_document_bsky_ref` clears the marker — typical recovery path is the user re-saving the post, which routes through update_thread_in_place or update_single, both of which already end with `update_document_bsky_ref`. `clear_all_record_meta` clears the marker too so rollback / delete paths leave no stale state. TODO comment points at issue 44 to broaden its surfacing remit beyond META_ORPHAN_RECORDS. --- includes/class-publisher.php | 67 ++++++++++++++++---- includes/transformer/class-post.php | 19 ++++++ tests/phpunit/tests/class-test-publisher.php | 43 +++++++++++++ 3 files changed, 118 insertions(+), 11 deletions(-) diff --git a/includes/class-publisher.php b/includes/class-publisher.php index cc24b73..495f928 100644 --- a/includes/class-publisher.php +++ b/includes/class-publisher.php @@ -225,18 +225,13 @@ private static function publish_thread( // Bailing here leaves META_THREAD_RECORDS at length=1, and a // subsequent edit treats that as a shape change and rewrites // the (already-published) root — invalidating likes/reposts/ - // external replies pointing at it. Log and continue; the - // doc-ref is metadata, the replies are user-visible content. + // external replies pointing at it. Persist the gap to + // META_DOC_REF_PENDING so operators (and any admin/Site Health + // surface) can see it; the next edit retries the doc-ref via + // update_*'s normal call to update_document_bsky_ref. $doc_ref_result = self::update_document_bsky_ref( $post, $doc_transformer ); if ( \is_wp_error( $doc_ref_result ) ) { - // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log - \error_log( - \sprintf( - '[atmosphere] post %d: doc-ref update failed during thread publish (%s); continuing with replies', - $post->ID, - $doc_ref_result->get_error_code() - ) - ); + self::record_doc_ref_pending( $post->ID, $doc_ref_result ); } $aggregated_results = $root_result['results'] ?? array(); @@ -1013,7 +1008,7 @@ private static function update_document_bsky_ref( \WP_Post $post, Document $doc_ // no need for a new instance. $record = $doc_transformer->transform(); - return API::post( + $result = API::post( '/xrpc/com.atproto.repo.putRecord', array( 'repo' => get_did(), @@ -1022,6 +1017,55 @@ private static function update_document_bsky_ref( \WP_Post $post, Document $doc_ 'record' => $record, ) ); + + // Clear any previous deferred-failure marker on success so + // subsequent edits stop reporting the post as pending. + if ( ! \is_wp_error( $result ) ) { + \delete_post_meta( $post->ID, Post::META_DOC_REF_PENDING ); + } + + return $result; + } + + /** + * Persist a deferred `update_document_bsky_ref` failure. + * + * Called from `publish_thread()` when the follow-up `putRecord` + * fails after the bsky thread root and document records have + * already been created. The publish itself is treated as a success + * (replies still ship; rewriting the root on the next edit would + * be the worse failure mode), but the gap is recorded here so + * operators / admin surfaces can see that the document's + * `bskyPostRef` is stale and that the post should be re-saved to + * trigger a retry. + * + * Cleared by a subsequent successful `update_document_bsky_ref`. + * + * TODO: surface in admin / Site Health alongside META_ORPHAN_RECORDS + * (issue 44). + * + * @param int $post_id WordPress post ID. + * @param \WP_Error $error The doc-ref failure to record. + */ + private static function record_doc_ref_pending( int $post_id, \WP_Error $error ): void { + \update_post_meta( + $post_id, + Post::META_DOC_REF_PENDING, + array( + 'stamp' => \gmdate( 'Y-m-d\TH:i:s.000\Z' ), + 'code' => $error->get_error_code(), + 'message' => $error->get_error_message(), + ) + ); + + // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log + \error_log( + \sprintf( + '[atmosphere] post %d: doc-ref update failed during thread publish (%s); continuing with replies', + $post_id, + $error->get_error_code() + ) + ); } /** @@ -1132,6 +1176,7 @@ private static function clear_all_record_meta( int $post_id ): void { \delete_post_meta( $post_id, Post::META_URI ); \delete_post_meta( $post_id, Post::META_TID ); \delete_post_meta( $post_id, Post::META_CID ); + \delete_post_meta( $post_id, Post::META_DOC_REF_PENDING ); \delete_post_meta( $post_id, Document::META_URI ); \delete_post_meta( $post_id, Document::META_TID ); \delete_post_meta( $post_id, Document::META_CID ); diff --git a/includes/transformer/class-post.php b/includes/transformer/class-post.php index 964e5ec..0502e81 100644 --- a/includes/transformer/class-post.php +++ b/includes/transformer/class-post.php @@ -85,6 +85,25 @@ class Post extends Base { */ public const META_ORPHAN_RECORDS = '_atmosphere_bsky_orphan_records'; + /** + * Tracks a deferred `update_document_bsky_ref` failure. + * + * Set by Publisher when the doc-ref `putRecord` fails after the + * thread root + document have already been written, so the bsky + * post(s) and the document are both live on the PDS but the + * document's `bskyPostRef` is missing or stale. The publish itself + * is treated as successful (replies still ship; rewriting the root + * on the next edit would be worse) and this meta records the gap so + * an operator or admin/Site Health surface can spot it. + * + * Cleared the next time `update_document_bsky_ref` succeeds for the + * post (typical recovery path: any subsequent edit retries the + * follow-up putRecord). Value: `[ stamp, code, message ]`. + * + * @var string + */ + public const META_DOC_REF_PENDING = '_atmosphere_doc_ref_pending'; + /** * Transform the post. * diff --git a/tests/phpunit/tests/class-test-publisher.php b/tests/phpunit/tests/class-test-publisher.php index f3c7192..806d406 100644 --- a/tests/phpunit/tests/class-test-publisher.php +++ b/tests/phpunit/tests/class-test-publisher.php @@ -578,11 +578,54 @@ public function test_publish_thread_continues_when_doc_ref_update_fails() { $this->assertNotEmpty( $record['uri'] ); $this->assertNotEmpty( $record['cid'] ); } + + // Pending-doc-ref marker is persisted so admin / Site Health + // can surface the gap; logs are not the only signal. + $pending = \get_post_meta( $post->ID, Post::META_DOC_REF_PENDING, true ); + $this->assertIsArray( $pending ); + $this->assertSame( 'atmosphere_doc_ref_failed', $pending['code'] ); + $this->assertNotEmpty( $pending['stamp'] ); + $this->assertNotEmpty( $pending['message'] ); } finally { \remove_filter( 'pre_http_request', $put_record_failure, 5 ); } } + /** + * A successful `update_document_bsky_ref` clears any prior + * `META_DOC_REF_PENDING` marker — typical recovery path is the user + * re-saves the post (any `Publisher::update*` flow ends at + * `update_document_bsky_ref`). + */ + public function test_publish_clears_doc_ref_pending_on_successful_doc_ref() { + $post = self::factory()->post->create_and_get( + array( + 'post_title' => 'A Long-Form Post', + 'post_content' => 'Body.', + 'post_excerpt' => 'Teaser excerpt.', + ) + ); + + // Pretend a previous publish persisted a pending marker. + \update_post_meta( + $post->ID, + Post::META_DOC_REF_PENDING, + array( + 'stamp' => '2026-04-28T00:00:00.000Z', + 'code' => 'atmosphere_doc_ref_failed', + 'message' => 'previous failure', + ) + ); + + $this->fail_call_indexes = array(); + $this->register_capture( $post->ID ); + + $result = Publisher::publish( $post ); + + $this->assertIsArray( $result ); + $this->assertSame( '', \get_post_meta( $post->ID, Post::META_DOC_REF_PENDING, true ) ); + } + /** * When the reply write fails, issue compensating deletes for the root * + doc, clear every meta key, and return the original WP_Error. From 880af50eb8eddfbd553af7b25895a82f0b5d4255 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Tue, 28 Apr 2026 14:00:17 -0500 Subject: [PATCH 21/22] Codex review: include ambiguous reply rkey in thread rollback PDS create errors are ambiguous: the server may have committed the write even when WP got back a transport-level WP_Error (response timeout, connection drop after server-side commit). Previously the rollback only iterated already-stored thread_records, so the just-attempted reply could be a live record on the PDS that META_THREAD_RECORDS never saw and rollback never targeted. The reply rkey is generated locally before the create call, so it is known either way. Include a synthetic triple (uri/cid/tid with empty cid) in the rollback list when the create returns WP_Error. Two outcomes: - If the PDS never committed: the compensating delete is a no-op, or surfaces in the orphan manifest if the PDS treats it as an error. - If the PDS committed: rollback cleans the live record up. Either way we no longer leave a Bluesky reply that the WP side can't discover or delete. Existing rollback tests updated for the new 3-write rollback shape (reply + root + doc) and the partial_records count of 2. --- includes/class-publisher.php | 23 +++++++++++- tests/phpunit/tests/class-test-publisher.php | 38 ++++++++++++++------ 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/includes/class-publisher.php b/includes/class-publisher.php index 495f928..c0bff51 100644 --- a/includes/class-publisher.php +++ b/includes/class-publisher.php @@ -267,7 +267,28 @@ private static function publish_thread( ); if ( \is_wp_error( $reply_result ) ) { - return self::rollback_thread( $post, $thread_records, $doc_transformer, $reply_result ); + // `apply_writes` errors are ambiguous: the PDS may have + // committed the create even when WP got a transport-level + // failure back (server-side commit + response timeout / + // connection drop). The rkey is generated locally and is + // known regardless of commit state, so include a synthetic + // triple in the rollback list. If the record was never + // committed, the compensating delete is a no-op (or + // surfaces in the orphan manifest if the PDS rejects it). + // If it was committed, rollback cleans it up. Either way + // we don't leave a live reply that META_THREAD_RECORDS + // can't see. + $ambiguous_triple = array( + 'uri' => build_at_uri( get_did(), 'app.bsky.feed.post', $reply_rkey ), + 'cid' => '', + 'tid' => $reply_rkey, + ); + return self::rollback_thread( + $post, + \array_merge( $thread_records, array( $ambiguous_triple ) ), + $doc_transformer, + $reply_result + ); } $reply_triple = self::build_triple_from_result( diff --git a/tests/phpunit/tests/class-test-publisher.php b/tests/phpunit/tests/class-test-publisher.php index 806d406..604557b 100644 --- a/tests/phpunit/tests/class-test-publisher.php +++ b/tests/phpunit/tests/class-test-publisher.php @@ -627,8 +627,17 @@ public function test_publish_clears_doc_ref_pending_on_successful_doc_ref() { } /** - * When the reply write fails, issue compensating deletes for the root - * + doc, clear every meta key, and return the original WP_Error. + * When the reply write fails, issue compensating deletes for the + * (possibly committed) reply, the root, and the doc, clear every + * meta key, and return the original WP_Error. + * + * The reply rkey is generated locally before the create, so even + * when the WP_Error is genuinely "never committed" the delete still + * has the correct rkey to target — and when the failure is actually + * an ambiguous "PDS committed but response failed", that rkey is + * the only handle on the live record. Including it in rollback + * closes the gap where rollback used to leave a live reply + * untracked in META_THREAD_RECORDS. */ public function test_publish_teaser_thread_rollback_on_second_write_failure() { $post = self::factory()->post->create_and_get( @@ -646,6 +655,8 @@ public function test_publish_teaser_thread_rollback_on_second_write_failure() { ); $this->register_capture( $post->ID ); + // Capture the reply rkey from the (failed) create write so we can + // assert it appears in the rollback batch. $result = Publisher::publish( $post ); $this->assertWPError( $result ); @@ -654,19 +665,25 @@ public function test_publish_teaser_thread_rollback_on_second_write_failure() { // 3 calls total: root+doc create, reply create (fail), rollback deletes. $this->assertCount( 3, $this->captured_calls ); + $failed_reply_rkey = $this->captured_calls[1]['writes'][0]['rkey']; + $this->assertNotEmpty( $failed_reply_rkey ); + $rollback_writes = $this->captured_calls[2]['writes']; - // Rollback deletes root bsky first, then doc: tail-first traversal - // over the thread records, which here is just the root (only the - // root made it before failure). - $this->assertCount( 2, $rollback_writes ); + // Rollback deletes the (ambiguous) reply first, then the root, + // then the doc — tail-first traversal over thread_records with + // the failed-reply triple appended. + $this->assertCount( 3, $rollback_writes ); $this->assertSame( 'com.atproto.repo.applyWrites#delete', $rollback_writes[0]['$type'] ); $this->assertSame( 'app.bsky.feed.post', $rollback_writes[0]['collection'] ); + $this->assertSame( $failed_reply_rkey, $rollback_writes[0]['rkey'] ); $this->assertSame( 'com.atproto.repo.applyWrites#delete', $rollback_writes[1]['$type'] ); - $this->assertSame( 'site.standard.document', $rollback_writes[1]['collection'] ); + $this->assertSame( 'app.bsky.feed.post', $rollback_writes[1]['collection'] ); + $this->assertSame( 'com.atproto.repo.applyWrites#delete', $rollback_writes[2]['$type'] ); + $this->assertSame( 'site.standard.document', $rollback_writes[2]['collection'] ); - // Root TID appears in rollback. - $root_rkey = $rollback_writes[0]['rkey']; + // Root TID is the second delete (after the ambiguous reply). + $root_rkey = $rollback_writes[1]['rkey']; $this->assertNotEmpty( $root_rkey ); // Meta fully cleared. @@ -706,7 +723,8 @@ public function test_publish_teaser_thread_rollback_failing_surfaces_partial_sta $this->assertIsArray( $data ); $this->assertArrayHasKey( 'partial_records', $data ); $this->assertIsArray( $data['partial_records'] ); - $this->assertCount( 1, $data['partial_records'] ); + // Root + ambiguous failed-reply (rkey known, commit state unknown). + $this->assertCount( 2, $data['partial_records'] ); $this->assertArrayHasKey( 'original_error', $data ); $this->assertArrayHasKey( 'rollback_error', $data ); From 6bd6a3209f17ccd8cf6bafeeb82d469ca7ce5c2a Mon Sep 17 00:00:00 2001 From: Matthias Pfefferle Date: Wed, 29 Apr 2026 17:49:59 +0200 Subject: [PATCH 22/22] Settings UI for the long-form composition strategy (#42) --- .../add-long-form-composition-setting | 4 + includes/class-atmosphere.php | 45 ++++++++ includes/wp-admin/class-admin.php | 100 ++++++++++++++++ ...ass-test-long-form-composition-setting.php | 108 ++++++++++++++++++ 4 files changed, 257 insertions(+) create mode 100644 .github/changelog/add-long-form-composition-setting create mode 100644 tests/phpunit/tests/class-test-long-form-composition-setting.php diff --git a/.github/changelog/add-long-form-composition-setting b/.github/changelog/add-long-form-composition-setting new file mode 100644 index 0000000..68cc108 --- /dev/null +++ b/.github/changelog/add-long-form-composition-setting @@ -0,0 +1,4 @@ +Significance: minor +Type: added + +Choose how long-form posts publish to Bluesky from the ATmosphere settings page — link card (default), a single post combining body text with the permalink, or a two-post teaser thread. diff --git a/includes/class-atmosphere.php b/includes/class-atmosphere.php index c4c5b1e..f1b9005 100644 --- a/includes/class-atmosphere.php +++ b/includes/class-atmosphere.php @@ -21,6 +21,12 @@ */ class Atmosphere { + /** + * Allowed values for the long-form composition strategy filter and + * the matching `atmosphere_long_form_composition` option. + */ + public const LONG_FORM_STRATEGIES = array( 'link-card', 'truncate-link', 'teaser-thread' ); + /** * Wire up all hooks. */ @@ -34,6 +40,13 @@ public function init(): void { \add_action( 'init', array( Admin::class, 'register' ), 5 ); \add_action( 'init', array( Backfill::class, 'register' ), 5 ); + /* + * Seed the long-form composition strategy from the user's + * setting. Priority 1 so any downstream filter at the default + * priority can still override it per post. + */ + \add_filter( 'atmosphere_long_form_composition', array( self::class, 'seed_long_form_composition' ), 1 ); + // REST route (always active for client-metadata). \add_action( 'rest_api_init', array( Admin::class, 'register_rest_routes' ) ); @@ -354,6 +367,38 @@ public function cron_refresh_token(): void { Client::refresh(); } + /** + * Seed the `atmosphere_long_form_composition` filter from the option. + * + * Returns the configured strategy when valid; otherwise returns the + * incoming `$strategy` (so downstream filters and the `link-card` + * default still apply). An invalid stored value is logged at most + * once per hour so operators can spot config drift. + * + * @param string $strategy Strategy passed in by `apply_filters()`. + * @return string + */ + public static function seed_long_form_composition( string $strategy ): string { + $option = (string) \get_option( 'atmosphere_long_form_composition', 'link-card' ); + + if ( \in_array( $option, self::LONG_FORM_STRATEGIES, true ) ) { + return $option; + } + + if ( ! \get_transient( 'atmosphere_invalid_long_form_composition_logged' ) ) { + // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log + \error_log( + \sprintf( + '[atmosphere] invalid `atmosphere_long_form_composition` option value %s; falling through to default', + \wp_json_encode( $option ) + ) + ); + \set_transient( 'atmosphere_invalid_long_form_composition_logged', 1, \HOUR_IN_SECONDS ); + } + + return $strategy; + } + /** * Register async action hooks (called by WP-Cron). */ diff --git a/includes/wp-admin/class-admin.php b/includes/wp-admin/class-admin.php index 69915e8..8137438 100644 --- a/includes/wp-admin/class-admin.php +++ b/includes/wp-admin/class-admin.php @@ -9,6 +9,7 @@ \defined( 'ABSPATH' ) || exit; +use Atmosphere\Atmosphere; use Atmosphere\Backfill; use Atmosphere\OAuth\Client; use Atmosphere\Publisher; @@ -68,6 +69,22 @@ public static function register_settings(): void { ) ); + \register_setting( + 'atmosphere', + 'atmosphere_long_form_composition', + array( + 'type' => 'string', + 'description' => 'Composition strategy for long-form Bluesky posts.', + 'default' => 'link-card', + 'sanitize_callback' => array( self::class, 'sanitize_long_form_composition' ), + 'show_in_rest' => array( + 'schema' => array( + 'enum' => Atmosphere::LONG_FORM_STRATEGIES, + ), + ), + ) + ); + \register_setting( 'atmosphere', 'atmosphere_handle', @@ -116,6 +133,14 @@ public static function register_settings(): void { 'atmosphere_publishing' ); + \add_settings_field( + 'atmosphere_long_form_composition', + \__( 'Long-form posts', 'atmosphere' ), + array( self::class, 'render_long_form_composition_field' ), + 'atmosphere', + 'atmosphere_publishing' + ); + \add_settings_field( 'atmosphere_backfill', \__( 'Backfill', 'atmosphere' ), @@ -247,6 +272,81 @@ public static function render_auto_publish_field(): void { +
+ + + + +

+ +
+ +

+ +
+

+ +

+ \__( 'Truncated post with link', 'atmosphere' ), + 'help' => \__( 'A single Bluesky post containing the body text followed by an inline permalink. No card.', 'atmosphere' ), + ); + case 'teaser-thread': + return array( + 'label' => \__( 'Teaser thread', 'atmosphere' ), + 'help' => \__( 'A two-post Bluesky thread: a hook followed by a "continue reading" reply with the permalink.', 'atmosphere' ), + ); + case 'link-card': + default: + return array( + 'label' => \__( 'Link card', 'atmosphere' ), + 'help' => \__( 'A single Bluesky post with the title, an excerpt, and a permalink card. (Default — unchanged behavior.)', 'atmosphere' ), + ); + } + } + + /** + * Sanitize the long-form composition setting. + * + * @param mixed $value Submitted value. + * @return string + */ + public static function sanitize_long_form_composition( $value ): string { + $value = \is_string( $value ) ? \sanitize_text_field( $value ) : ''; + + return \in_array( $value, Atmosphere::LONG_FORM_STRATEGIES, true ) ? $value : 'link-card'; + } + /** * Render the Backfill field. */ diff --git a/tests/phpunit/tests/class-test-long-form-composition-setting.php b/tests/phpunit/tests/class-test-long-form-composition-setting.php new file mode 100644 index 0000000..901e2fd --- /dev/null +++ b/tests/phpunit/tests/class-test-long-form-composition-setting.php @@ -0,0 +1,108 @@ +assertSame( 'link-card', Admin::sanitize_long_form_composition( 'link-card' ) ); + $this->assertSame( 'truncate-link', Admin::sanitize_long_form_composition( 'truncate-link' ) ); + $this->assertSame( 'teaser-thread', Admin::sanitize_long_form_composition( 'teaser-thread' ) ); + } + + /** + * Sanitize callback falls back to the default for unknown / non-string input. + */ + public function test_sanitize_rejects_unknown_values() { + $this->assertSame( 'link-card', Admin::sanitize_long_form_composition( 'something-else' ) ); + $this->assertSame( 'link-card', Admin::sanitize_long_form_composition( '' ) ); + $this->assertSame( 'link-card', Admin::sanitize_long_form_composition( null ) ); + $this->assertSame( 'link-card', Admin::sanitize_long_form_composition( array( 'teaser-thread' ) ) ); + } + + /** + * The option seeds the `atmosphere_long_form_composition` filter. + */ + public function test_option_seeds_filter() { + \update_option( 'atmosphere_long_form_composition', 'teaser-thread' ); + + $result = \apply_filters( 'atmosphere_long_form_composition', 'link-card', null ); + + $this->assertSame( 'teaser-thread', $result ); + } + + /** + * Downstream filters at the default priority override the option. + */ + public function test_downstream_filter_overrides_option() { + \update_option( 'atmosphere_long_form_composition', 'teaser-thread' ); + + \add_filter( + 'atmosphere_long_form_composition', + static function (): string { + return 'truncate-link'; + } + ); + + $result = \apply_filters( 'atmosphere_long_form_composition', 'link-card', null ); + + $this->assertSame( 'truncate-link', $result ); + } + + /** + * Unknown stored values are ignored; the default flows through. + */ + public function test_corrupt_option_falls_through_to_default() { + \update_option( 'atmosphere_long_form_composition', 'bogus-strategy' ); + + $result = \apply_filters( 'atmosphere_long_form_composition', 'link-card', null ); + + $this->assertSame( 'link-card', $result ); + } +}