From bc91c19e74b413e320a4d74c823b8ec198b0adad Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Sat, 23 May 2026 21:33:49 -0500 Subject: [PATCH 01/11] add: historical TID generation for backfilled posts --- .github/changelog/historical-tids | 4 + includes/transformer/class-comment.php | 43 +++++++- includes/transformer/class-document.php | 42 +++++++- includes/transformer/class-post.php | 56 +++++++++- includes/transformer/class-tid.php | 79 ++++++++++++++ .../tests/transformer/class-test-document.php | 56 ++++++++++ .../tests/transformer/class-test-tid.php | 102 ++++++++++++++++++ 7 files changed, 379 insertions(+), 3 deletions(-) create mode 100644 .github/changelog/historical-tids diff --git a/.github/changelog/historical-tids b/.github/changelog/historical-tids new file mode 100644 index 0000000..da6e6df --- /dev/null +++ b/.github/changelog/historical-tids @@ -0,0 +1,4 @@ +Significance: minor +Type: added + +Backfilled posts can now publish with rkeys that reflect the original publish date. diff --git a/includes/transformer/class-comment.php b/includes/transformer/class-comment.php index 69e7a9c..537a8fa 100644 --- a/includes/transformer/class-comment.php +++ b/includes/transformer/class-comment.php @@ -139,13 +139,54 @@ public function get_rkey(): string { $rkey = \get_comment_meta( (int) $this->object->comment_ID, self::META_TID, true ); if ( empty( $rkey ) ) { - $rkey = TID::generate(); + $rkey = self::mint_rkey( $this->object ); \update_comment_meta( (int) $this->object->comment_ID, self::META_TID, $rkey ); } return $rkey; } + /** + * Mint a new TID for this comment, honoring the historical-TID filter. + * + * Default is a historical TID derived from `comment_date_gmt`, so + * backfilled comments land at their original post position in the + * AT Protocol repo. Filter listeners can return false to fall back + * to a now-based TID. + * + * Falls back to {@see TID::generate()} when `comment_date_gmt` + * cannot be parsed so we never mint an epoch-anchored TID. + * + * @since unreleased + * + * @param \WP_Comment $comment Comment being published. + * @return string TID rkey. + */ + private static function mint_rkey( \WP_Comment $comment ): string { + /** This filter is documented in includes/transformer/class-post.php */ + $use_historical = (bool) \apply_filters( + 'atmosphere_use_historical_tid', + true, + $comment, + 'app.bsky.feed.post' + ); + + if ( ! $use_historical ) { + return TID::generate(); + } + + $microseconds = TID::microseconds_from_post_date( + (string) $comment->comment_date_gmt, + (int) $comment->comment_ID + ); + + if ( $microseconds <= 0 ) { + return TID::generate(); + } + + return TID::generate_for_time( $microseconds ); + } + /** * Build the reply struct with root and parent refs. * diff --git a/includes/transformer/class-document.php b/includes/transformer/class-document.php index 3f49231..a794347 100644 --- a/includes/transformer/class-document.php +++ b/includes/transformer/class-document.php @@ -199,13 +199,53 @@ public function get_rkey(): string { $rkey = \get_post_meta( $this->object->ID, self::META_TID, true ); if ( empty( $rkey ) ) { - $rkey = TID::generate(); + $rkey = self::mint_rkey( $this->object ); \update_post_meta( $this->object->ID, self::META_TID, $rkey ); } return $rkey; } + /** + * Mint a new TID for this document, honoring the historical-TID filter. + * + * Default is a historical TID derived from `post_date_gmt`, so + * backfilled documents land at their original publish position in + * the AT Protocol repo. Filter listeners can return false to fall + * back to a now-based TID — useful for sites that intentionally + * want commit-order rkeys regardless of original publish date. + * + * Falls back to {@see TID::generate()} when `post_date_gmt` cannot + * be parsed (e.g. the `0000-00-00 00:00:00` sentinel) so we never + * mint an epoch-anchored TID. + * + * @since unreleased + * + * @param \WP_Post $post Post being published. + * @return string TID rkey. + */ + private static function mint_rkey( \WP_Post $post ): string { + /** This filter is documented in includes/transformer/class-post.php */ + $use_historical = (bool) \apply_filters( + 'atmosphere_use_historical_tid', + true, + $post, + 'site.standard.document' + ); + + if ( ! $use_historical ) { + return TID::generate(); + } + + $microseconds = TID::microseconds_from_post_date( (string) $post->post_date_gmt, (int) $post->ID ); + + if ( $microseconds <= 0 ) { + return TID::generate(); + } + + return TID::generate_for_time( $microseconds ); + } + /** * Get parsed content for the document's content union field. * diff --git a/includes/transformer/class-post.php b/includes/transformer/class-post.php index 35fe8b3..36af095 100644 --- a/includes/transformer/class-post.php +++ b/includes/transformer/class-post.php @@ -277,13 +277,67 @@ public function get_rkey(): string { $rkey = \get_post_meta( $this->object->ID, self::META_TID, true ); if ( empty( $rkey ) ) { - $rkey = TID::generate(); + $rkey = self::mint_rkey( $this->object ); \update_post_meta( $this->object->ID, self::META_TID, $rkey ); } return $rkey; } + /** + * Mint a new TID for this post, honoring the historical-TID filter. + * + * Default is a historical TID derived from `post_date_gmt`, so + * backfilled posts land at their original publish position in the + * AT Protocol repo. Filter listeners can return false to fall back + * to a now-based TID — useful for sites that intentionally want + * commit-order rkeys regardless of original publish date. + * + * Falls back to {@see TID::generate()} when `post_date_gmt` cannot + * be parsed (e.g. the `0000-00-00 00:00:00` sentinel) so we never + * mint an epoch-anchored TID. + * + * @since unreleased + * + * @param \WP_Post $post Post being published. + * @return string TID rkey. + */ + private static function mint_rkey( \WP_Post $post ): string { + /** + * Filters whether to mint a historical TID based on the post's + * original publish date. + * + * Defaults to true so backfilled posts get rkeys that reflect + * the original publish time rather than the time of the + * backfill run. Return false to fall back to the live-publish + * `TID::generate()` path (now-based, with the monotonic floor). + * + * @since unreleased + * + * @param bool $use_historical Whether to mint a historical TID. + * @param \WP_Post|\WP_Comment $object WordPress object being transformed. + * @param string $collection AT Protocol collection NSID. + */ + $use_historical = (bool) \apply_filters( + 'atmosphere_use_historical_tid', + true, + $post, + 'app.bsky.feed.post' + ); + + if ( ! $use_historical ) { + return TID::generate(); + } + + $microseconds = TID::microseconds_from_post_date( (string) $post->post_date_gmt, (int) $post->ID ); + + if ( $microseconds <= 0 ) { + return TID::generate(); + } + + return TID::generate_for_time( $microseconds ); + } + /** * Compose the post text: title + excerpt + permalink within 300 characters. * diff --git a/includes/transformer/class-tid.php b/includes/transformer/class-tid.php index 0e92a62..ecfff46 100644 --- a/includes/transformer/class-tid.php +++ b/includes/transformer/class-tid.php @@ -185,6 +185,85 @@ public static function generate(): string { return self::encode( ( $ts << 10 ) | self::$clock_id ); } + /** + * Generate a TID that encodes a specific historical microsecond. + * + * Unlike {@see self::generate()}, this path is intentionally + * floor-free: backfilled records carry timestamps far older than + * any current-time floor would allow, so consulting or updating + * `OPTION_LAST_TS` (or `self::$last_ts`) would either snap the + * value forward to "now" — defeating the entire point — or + * regress the floor used by live publishing. Callers are + * responsible for supplying a microsecond value that is + * collision-resistant within their batch (see + * {@see self::microseconds_from_post_date()} for the standard + * deterministic helper). + * + * @since unreleased + * + * @param int $microseconds Microseconds since the Unix epoch. + * @return string 13-character identifier. + */ + public static function generate_for_time( int $microseconds ): string { + if ( null === self::$clock_id ) { + /* + * Same fallback shape as `generate()`: `random_int` + * throws on systems without a usable CSPRNG; fall back + * to `wp_rand` so a missing entropy source can't break + * historical backfills. + */ + try { + self::$clock_id = \random_int( 0, 1023 ); + } catch ( \Throwable $e ) { + self::$clock_id = \wp_rand( 0, 1023 ); + } + } + + return self::encode( ( $microseconds << 10 ) | self::$clock_id ); + } + + /** + * Convert a GMT datetime + post ID into a deterministic microsecond value. + * + * `post_date_gmt` is MySQL second resolution, so two posts + * published in the same second would otherwise hash to identical + * microseconds and collide on the 10-bit clock identifier within + * a single backfill run. Mixing the post ID (modulo one second + * of microseconds) into the microsecond portion disambiguates + * those collisions deterministically — re-running the backfill + * mints the same TID for the same post, which keeps the operation + * idempotent against `applyWrites`. + * + * Returns `0` if the datetime can't be parsed; callers should + * decide whether to fall back to {@see self::generate()} in that + * case rather than minting an epoch-anchored TID. + * + * @since unreleased + * + * @param string $gmt_datetime GMT datetime string (e.g. `post_date_gmt`). + * @param int $post_id Post or comment identifier for disambiguation. + * @return int Microseconds since the Unix epoch, or 0 on parse failure. + */ + public static function microseconds_from_post_date( string $gmt_datetime, int $post_id ): int { + $trimmed = \trim( $gmt_datetime ); + + // MySQL zero-date sentinel and empty strings: bail before + // `strtotime` (which interprets `0000-00-00 00:00:00` as year + // zero on some PHP builds, yielding a far-past-or-future + // timestamp that would mint a meaningless TID). + if ( '' === $trimmed || '0000-00-00 00:00:00' === $trimmed ) { + return 0; + } + + $seconds = \strtotime( $trimmed . ' UTC' ); + + if ( false === $seconds || $seconds <= 0 ) { + return 0; + } + + return ( $seconds * 1_000_000 ) + ( $post_id % 1_000_000 ); + } + /** * Check whether a string looks like a valid TID. * diff --git a/tests/phpunit/tests/transformer/class-test-document.php b/tests/phpunit/tests/transformer/class-test-document.php index bfa4a3d..b2c3932 100644 --- a/tests/phpunit/tests/transformer/class-test-document.php +++ b/tests/phpunit/tests/transformer/class-test-document.php @@ -305,4 +305,60 @@ public function test_collection() { $this->assertSame( 'site.standard.document', $transformer->get_collection() ); } + + /** + * By default, get_rkey() mints a historical TID for the post's + * original publish date, so a 2010 post lands at a 2010 rkey + * regardless of when the backfill runs. + * + * @covers \Atmosphere\Transformer\Document::get_rkey + */ + public function test_get_rkey_defaults_to_historical_tid() { + $post = self::factory()->post->create_and_get( + array( + 'post_date' => '2010-01-15 12:00:00', + 'post_date_gmt' => '2010-01-15 12:00:00', + ) + ); + + // Capture the historical rkey first so the assertion compares + // purely TID encodings rather than the side effect of meta. + $historical_rkey = ( new Document( $post ) )->get_rkey(); + $current_rkey = \Atmosphere\Transformer\TID::generate(); + + $this->assertNotEmpty( $historical_rkey ); + $this->assertTrue( \Atmosphere\Transformer\TID::is_valid( $historical_rkey ) ); + $this->assertLessThan( $current_rkey, $historical_rkey, '2010-anchored rkey must sort before a now-minted TID.' ); + } + + /** + * Listeners returning false from the atmosphere_use_historical_tid + * filter fall back to the now-based TID::generate() path. + * + * @covers \Atmosphere\Transformer\Document::get_rkey + */ + public function test_get_rkey_filter_opt_out_uses_current_time() { + \add_filter( 'atmosphere_use_historical_tid', '__return_false' ); + + $post = self::factory()->post->create_and_get( + array( + 'post_date' => '2010-01-15 12:00:00', + 'post_date_gmt' => '2010-01-15 12:00:00', + ) + ); + + // A baseline current-time TID minted just before the rkey: + // the filter-disabled rkey should sort right next to it, + // not anywhere near a 2010 historical TID. + $baseline = \Atmosphere\Transformer\TID::generate(); + $rkey = ( new Document( $post ) )->get_rkey(); + $historical_2010 = \Atmosphere\Transformer\TID::generate_for_time( + \Atmosphere\Transformer\TID::microseconds_from_post_date( '2010-01-15 12:00:00', $post->ID ) + ); + + $this->assertGreaterThan( $baseline, $rkey, 'Opting out via filter must mint a now-based TID.' ); + $this->assertGreaterThan( $historical_2010, $rkey, 'Opted-out TID must sort after a 2010 historical TID.' ); + + \remove_all_filters( 'atmosphere_use_historical_tid' ); + } } diff --git a/tests/phpunit/tests/transformer/class-test-tid.php b/tests/phpunit/tests/transformer/class-test-tid.php index 634a3f5..0bed0ed 100644 --- a/tests/phpunit/tests/transformer/class-test-tid.php +++ b/tests/phpunit/tests/transformer/class-test-tid.php @@ -12,6 +12,9 @@ /** * TID tests. + * + * @group atmosphere + * @group transformer */ class Test_TID extends WP_UnitTestCase { @@ -52,4 +55,103 @@ public function test_is_valid_rejects_bad_input() { $this->assertFalse( TID::is_valid( '0000000000000' ) ); // '0' and '1' not in charset. $this->assertFalse( TID::is_valid( 'AAAAAAAAAAAAA' ) ); // Uppercase not in charset. } + + /** + * The generate_for_time() helper returns a 13-character valid TID. + */ + public function test_generate_for_time_shape() { + $tid = TID::generate_for_time( 1_500_000_000_000_000 ); + + $this->assertSame( 13, \strlen( $tid ) ); + $this->assertTrue( TID::is_valid( $tid ) ); + } + + /** + * The generate_for_time() helper is deterministic across calls with + * the same microsecond value within the same process (the clock_id + * is a per-process static, so a second call mints the same encoding). + */ + public function test_generate_for_time_deterministic_within_process() { + $microseconds = 1_500_000_000_000_000; + + $first = TID::generate_for_time( $microseconds ); + $second = TID::generate_for_time( $microseconds ); + + $this->assertSame( $first, $second ); + } + + /** + * The generate_for_time() helper must not consult or update the + * monotonic floor. After a historical mint with an old timestamp + * the live generate() must still produce a now-based TID (not + * floor + 1). + */ + public function test_generate_for_time_does_not_poison_floor() { + // Prime the floor with a current-time call. + $baseline = TID::generate(); + $floor_before = (int) \get_option( 'atmosphere_tid_last_ts', 0 ); + + // Mint a historical TID well below the floor. + $historical_micros = 1_000_000_000_000_000; // 2001-09-09. + $historical = TID::generate_for_time( $historical_micros ); + + $floor_after = (int) \get_option( 'atmosphere_tid_last_ts', 0 ); + + // Floor must not change. + $this->assertSame( $floor_before, $floor_after, 'Historical TID write must not bump the persisted floor.' ); + + // And the historical TID must sort *before* the baseline (proves + // it was not snapped forward to the floor). + $this->assertLessThan( $baseline, $historical, 'Historical TID for 2001 must sort before a current-time TID.' ); + + // A subsequent live generate() should keep moving forward + // relative to the baseline, not regress to the historical value. + $next_live = TID::generate(); + $this->assertGreaterThan( $baseline, $next_live, 'Live generate() after a historical mint must still increase.' ); + } + + /** + * The microseconds_from_post_date() helper returns deterministic, + * post-ID-disambiguated microseconds for two posts at the same second. + */ + public function test_microseconds_from_post_date_disambiguates_same_second() { + $gmt = '2019-03-14 15:09:26'; + + $a = TID::microseconds_from_post_date( $gmt, 42 ); + $b = TID::microseconds_from_post_date( $gmt, 43 ); + $c = TID::microseconds_from_post_date( $gmt, 42 ); + + $this->assertNotSame( $a, $b, 'Different post IDs in the same second must produce different microseconds.' ); + $this->assertSame( $a, $c, 'Same post ID + same second must be idempotent.' ); + + // Sanity-check: the seconds-portion of all three matches. + $seconds = (int) \strtotime( $gmt . ' UTC' ); + $this->assertSame( $seconds, \intdiv( $a, 1_000_000 ) ); + $this->assertSame( $seconds, \intdiv( $b, 1_000_000 ) ); + } + + /** + * The microseconds_from_post_date() helper returns 0 for unparseable + * input so callers can fall back to TID::generate() rather than + * minting an epoch-anchored TID. + */ + public function test_microseconds_from_post_date_returns_zero_on_parse_failure() { + $this->assertSame( 0, TID::microseconds_from_post_date( '', 42 ) ); + $this->assertSame( 0, TID::microseconds_from_post_date( '0000-00-00 00:00:00', 42 ) ); + $this->assertSame( 0, TID::microseconds_from_post_date( 'not a date', 42 ) ); + } + + /** + * A historical-mint with an old timestamp produces a TID that sorts + * well before a TID minted from `microtime(true)` — proving the + * historical-ordering property end-to-end. + */ + public function test_generate_for_time_sorts_before_current() { + $historical_micros = TID::microseconds_from_post_date( '2010-01-01 00:00:00', 1 ); + $historical = TID::generate_for_time( $historical_micros ); + + $current = TID::generate(); + + $this->assertLessThan( $current, $historical, 'A 2010 TID must sort before a now-minted TID.' ); + } } From b678e07c82df89aa34267fb07de2d40a0052960f Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Sat, 23 May 2026 21:35:19 -0500 Subject: [PATCH 02/11] refactor: extract ensure_clock_id helper for TID generators --- includes/transformer/class-tid.php | 56 +++++++++++++----------------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/includes/transformer/class-tid.php b/includes/transformer/class-tid.php index ecfff46..034bf76 100644 --- a/includes/transformer/class-tid.php +++ b/includes/transformer/class-tid.php @@ -167,22 +167,7 @@ public static function generate(): string { \wp_cache_delete( self::OPTION_LAST_TS, 'options' ); } - if ( null === self::$clock_id ) { - /* - * `random_int` throws on systems without a usable CSPRNG - * (essentially never on a working PHP install, but a worth - * a fallback so a missing entropy source can't bring down - * publishing). `wp_rand` is non-cryptographic but the - * collision space is still 1024. - */ - try { - self::$clock_id = \random_int( 0, 1023 ); - } catch ( \Throwable $e ) { - self::$clock_id = \wp_rand( 0, 1023 ); - } - } - - return self::encode( ( $ts << 10 ) | self::$clock_id ); + return self::encode( ( $ts << 10 ) | self::ensure_clock_id() ); } /** @@ -205,21 +190,7 @@ public static function generate(): string { * @return string 13-character identifier. */ public static function generate_for_time( int $microseconds ): string { - if ( null === self::$clock_id ) { - /* - * Same fallback shape as `generate()`: `random_int` - * throws on systems without a usable CSPRNG; fall back - * to `wp_rand` so a missing entropy source can't break - * historical backfills. - */ - try { - self::$clock_id = \random_int( 0, 1023 ); - } catch ( \Throwable $e ) { - self::$clock_id = \wp_rand( 0, 1023 ); - } - } - - return self::encode( ( $microseconds << 10 ) | self::$clock_id ); + return self::encode( ( $microseconds << 10 ) | self::ensure_clock_id() ); } /** @@ -284,6 +255,29 @@ public static function is_valid( string $tid ): bool { return true; } + /** + * Lazily seed and return the per-process clock identifier. + * + * Shared by {@see self::generate()} and {@see self::generate_for_time()}. + * `random_int` throws on systems without a usable CSPRNG (essentially + * never on a working PHP install, but worth a fallback so a missing + * entropy source can't bring down publishing). `wp_rand` is + * non-cryptographic but the collision space is still 1024. + * + * @return int 10-bit clock identifier. + */ + private static function ensure_clock_id(): int { + if ( null === self::$clock_id ) { + try { + self::$clock_id = \random_int( 0, 1023 ); + } catch ( \Throwable $e ) { + self::$clock_id = \wp_rand( 0, 1023 ); + } + } + + return self::$clock_id; + } + /** * Encode a 64-bit integer into a 13-character base-32 string. * From 944bc96a16e2f84a78b9ac0c47409e34b515ab4c Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Sat, 23 May 2026 21:41:52 -0500 Subject: [PATCH 03/11] fix: namespace comment rkey salt to avoid post collision --- includes/transformer/class-comment.php | 3 +- includes/transformer/class-tid.php | 52 ++++++++++++++----- .../tests/transformer/class-test-tid.php | 24 +++++++++ 3 files changed, 66 insertions(+), 13 deletions(-) diff --git a/includes/transformer/class-comment.php b/includes/transformer/class-comment.php index 537a8fa..456803d 100644 --- a/includes/transformer/class-comment.php +++ b/includes/transformer/class-comment.php @@ -177,7 +177,8 @@ private static function mint_rkey( \WP_Comment $comment ): string { $microseconds = TID::microseconds_from_post_date( (string) $comment->comment_date_gmt, - (int) $comment->comment_ID + (int) $comment->comment_ID, + 'comment' ); if ( $microseconds <= 0 ) { diff --git a/includes/transformer/class-tid.php b/includes/transformer/class-tid.php index 034bf76..56891cb 100644 --- a/includes/transformer/class-tid.php +++ b/includes/transformer/class-tid.php @@ -194,16 +194,27 @@ public static function generate_for_time( int $microseconds ): string { } /** - * Convert a GMT datetime + post ID into a deterministic microsecond value. + * Convert a GMT datetime + object ID into a deterministic microsecond value. * - * `post_date_gmt` is MySQL second resolution, so two posts - * published in the same second would otherwise hash to identical - * microseconds and collide on the 10-bit clock identifier within - * a single backfill run. Mixing the post ID (modulo one second - * of microseconds) into the microsecond portion disambiguates - * those collisions deterministically — re-running the backfill - * mints the same TID for the same post, which keeps the operation - * idempotent against `applyWrites`. + * `post_date_gmt` and `comment_date_gmt` are MySQL second + * resolution, so two records published in the same second would + * otherwise hash to identical microseconds and collide on the + * 10-bit clock identifier within a single backfill run. Mixing + * the object ID (modulo one second of microseconds) into the + * microsecond portion disambiguates those collisions + * deterministically — re-running the backfill mints the same TID + * for the same record, which keeps the operation idempotent + * against `applyWrites`. + * + * The `$kind` argument disambiguates records that share an AT + * Protocol collection but are sourced from different WordPress + * object kinds. Without it, a `WP_Post` with id N and a + * `WP_Comment` with `comment_ID` N published in the same GMT + * second within a single PHP process would mint the same rkey + * inside `app.bsky.feed.post` and `applyWrites` would reject the + * second create. The kind label is hashed into a sub-second offset + * that's stable across runs (CRC32 is deterministic), preserving + * idempotency per kind. * * Returns `0` if the datetime can't be parsed; callers should * decide whether to fall back to {@see self::generate()} in that @@ -212,10 +223,12 @@ public static function generate_for_time( int $microseconds ): string { * @since unreleased * * @param string $gmt_datetime GMT datetime string (e.g. `post_date_gmt`). - * @param int $post_id Post or comment identifier for disambiguation. + * @param int $object_id Post or comment identifier for disambiguation. + * @param string $kind Optional kind label to separate records + * sharing a collection (e.g. `post`, `comment`). * @return int Microseconds since the Unix epoch, or 0 on parse failure. */ - public static function microseconds_from_post_date( string $gmt_datetime, int $post_id ): int { + public static function microseconds_from_post_date( string $gmt_datetime, int $object_id, string $kind = '' ): int { $trimmed = \trim( $gmt_datetime ); // MySQL zero-date sentinel and empty strings: bail before @@ -232,7 +245,22 @@ public static function microseconds_from_post_date( string $gmt_datetime, int $p return 0; } - return ( $seconds * 1_000_000 ) + ( $post_id % 1_000_000 ); + $offset = $object_id % 1_000_000; + + if ( '' !== $kind ) { + /* + * Fold the kind label into the same 0..999_999 microsecond + * window so the result still lands inside the post's + * original GMT second. CRC32 is deterministic across runs, + * which preserves the idempotency property — same input + * mints the same TID — while moving the two kinds far + * enough apart in microsecond-space that an ID-level + * collision across kinds is astronomically unlikely. + */ + $offset = ( $offset + \crc32( $kind ) ) % 1_000_000; + } + + return ( $seconds * 1_000_000 ) + $offset; } /** diff --git a/tests/phpunit/tests/transformer/class-test-tid.php b/tests/phpunit/tests/transformer/class-test-tid.php index 0bed0ed..0708a32 100644 --- a/tests/phpunit/tests/transformer/class-test-tid.php +++ b/tests/phpunit/tests/transformer/class-test-tid.php @@ -154,4 +154,28 @@ public function test_generate_for_time_sorts_before_current() { $this->assertLessThan( $current, $historical, 'A 2010 TID must sort before a now-minted TID.' ); } + + /** + * The namespace argument prevents collisions between WP_Post and + * WP_Comment records that share an AT Protocol collection but have + * matching IDs and timestamps. + */ + public function test_microseconds_from_post_date_namespace_disambiguates() { + $gmt = '2019-03-14 15:09:26'; + $id = 42; + + $post = TID::microseconds_from_post_date( $gmt, $id ); + $comment = TID::microseconds_from_post_date( $gmt, $id, 'comment' ); + + $this->assertNotSame( $post, $comment, 'Namespaced and bare results must differ for the same id+second.' ); + + // Namespace must be deterministic across calls. + $comment_again = TID::microseconds_from_post_date( $gmt, $id, 'comment' ); + $this->assertSame( $comment, $comment_again, 'Same namespace + id + second must be idempotent.' ); + + // Both still land inside the same GMT second. + $seconds = (int) \strtotime( $gmt . ' UTC' ); + $this->assertSame( $seconds, \intdiv( $post, 1_000_000 ) ); + $this->assertSame( $seconds, \intdiv( $comment, 1_000_000 ) ); + } } From 00ae77f651d7e6b74939e947de002cded7ea2408 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Sat, 23 May 2026 21:47:55 -0500 Subject: [PATCH 04/11] fix: derive historical TID clock_id deterministically --- includes/transformer/class-tid.php | 14 +++++++++++++- tests/phpunit/tests/transformer/class-test-tid.php | 10 ++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/includes/transformer/class-tid.php b/includes/transformer/class-tid.php index 56891cb..36ebc6f 100644 --- a/includes/transformer/class-tid.php +++ b/includes/transformer/class-tid.php @@ -184,13 +184,25 @@ public static function generate(): string { * {@see self::microseconds_from_post_date()} for the standard * deterministic helper). * + * The 10-bit clock identifier is derived deterministically from + * the microsecond input rather than the per-process random value + * used by `generate()`. Without this, retrying a backfill in a + * different PHP worker before the record's TID meta is persisted + * would mint a different rkey for the same post — the AT + * Protocol create would then succeed twice and orphan the first + * record. CRC32 over the microsecond value is stable across + * workers, so the historical path is process-independent and + * truly idempotent for the same input. + * * @since unreleased * * @param int $microseconds Microseconds since the Unix epoch. * @return string 13-character identifier. */ public static function generate_for_time( int $microseconds ): string { - return self::encode( ( $microseconds << 10 ) | self::ensure_clock_id() ); + $clock_id = \crc32( (string) $microseconds ) & 0x3FF; + + return self::encode( ( $microseconds << 10 ) | $clock_id ); } /** diff --git a/tests/phpunit/tests/transformer/class-test-tid.php b/tests/phpunit/tests/transformer/class-test-tid.php index 0708a32..7fa423e 100644 --- a/tests/phpunit/tests/transformer/class-test-tid.php +++ b/tests/phpunit/tests/transformer/class-test-tid.php @@ -67,11 +67,13 @@ public function test_generate_for_time_shape() { } /** - * The generate_for_time() helper is deterministic across calls with - * the same microsecond value within the same process (the clock_id - * is a per-process static, so a second call mints the same encoding). + * The generate_for_time() helper is fully deterministic for the + * same microsecond input. The clock_id is derived from the input + * rather than the per-process random value, so a retry in a + * different PHP worker reconstructs the same rkey — which is the + * idempotency guarantee the backfill flow depends on. */ - public function test_generate_for_time_deterministic_within_process() { + public function test_generate_for_time_is_deterministic() { $microseconds = 1_500_000_000_000_000; $first = TID::generate_for_time( $microseconds ); From 0d761b58a51a6d85a4f87149ec8f80042bf47c4f Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Sat, 23 May 2026 21:54:00 -0500 Subject: [PATCH 05/11] fix: salt historical TID clock_id with full object identity --- includes/transformer/class-comment.php | 2 +- includes/transformer/class-document.php | 2 +- includes/transformer/class-post.php | 2 +- includes/transformer/class-tid.php | 32 ++++++++++++------- .../tests/transformer/class-test-tid.php | 26 +++++++++++++++ 5 files changed, 50 insertions(+), 14 deletions(-) diff --git a/includes/transformer/class-comment.php b/includes/transformer/class-comment.php index 456803d..3ba2c59 100644 --- a/includes/transformer/class-comment.php +++ b/includes/transformer/class-comment.php @@ -185,7 +185,7 @@ private static function mint_rkey( \WP_Comment $comment ): string { return TID::generate(); } - return TID::generate_for_time( $microseconds ); + return TID::generate_for_time( $microseconds, 'comment:' . $comment->comment_ID ); } /** diff --git a/includes/transformer/class-document.php b/includes/transformer/class-document.php index a794347..fb24a4b 100644 --- a/includes/transformer/class-document.php +++ b/includes/transformer/class-document.php @@ -243,7 +243,7 @@ private static function mint_rkey( \WP_Post $post ): string { return TID::generate(); } - return TID::generate_for_time( $microseconds ); + return TID::generate_for_time( $microseconds, 'document:' . $post->ID ); } /** diff --git a/includes/transformer/class-post.php b/includes/transformer/class-post.php index 36af095..184d31a 100644 --- a/includes/transformer/class-post.php +++ b/includes/transformer/class-post.php @@ -335,7 +335,7 @@ private static function mint_rkey( \WP_Post $post ): string { return TID::generate(); } - return TID::generate_for_time( $microseconds ); + return TID::generate_for_time( $microseconds, 'post:' . $post->ID ); } /** diff --git a/includes/transformer/class-tid.php b/includes/transformer/class-tid.php index 36ebc6f..2c1d33d 100644 --- a/includes/transformer/class-tid.php +++ b/includes/transformer/class-tid.php @@ -185,22 +185,32 @@ public static function generate(): string { * deterministic helper). * * The 10-bit clock identifier is derived deterministically from - * the microsecond input rather than the per-process random value - * used by `generate()`. Without this, retrying a backfill in a - * different PHP worker before the record's TID meta is persisted - * would mint a different rkey for the same post — the AT - * Protocol create would then succeed twice and orphan the first - * record. CRC32 over the microsecond value is stable across - * workers, so the historical path is process-independent and - * truly idempotent for the same input. + * `$salt` rather than the per-process random value used by + * `generate()`. Without that, retrying a backfill in a different + * PHP worker before the record's TID meta is persisted would mint + * a different rkey for the same post — the AT Protocol create + * would then succeed twice and orphan the first record. Callers + * should pass a salt that is unique to the record being minted + * (a string composed of object id + kind is sufficient). The + * companion {@see self::microseconds_from_post_date()} helper + * folds the same ID into the microsecond portion, and the salt + * provides the second-order entropy that protects against the + * "IDs differ by a multiple of 1,000,000 inside the same GMT + * second" modulo collision — both 10 bits of the rkey now depend + * on the full object identity, not just the truncated portion. * * @since unreleased * - * @param int $microseconds Microseconds since the Unix epoch. + * @param int $microseconds Microseconds since the Unix epoch. + * @param string $salt Deterministic disambiguation salt + * (typically `$kind . ':' . $object_id`). + * Defaults to the microseconds string so + * ad-hoc callers still get a stable rkey. * @return string 13-character identifier. */ - public static function generate_for_time( int $microseconds ): string { - $clock_id = \crc32( (string) $microseconds ) & 0x3FF; + public static function generate_for_time( int $microseconds, string $salt = '' ): string { + $clock_source = '' === $salt ? (string) $microseconds : $salt; + $clock_id = \crc32( $clock_source ) & 0x3FF; return self::encode( ( $microseconds << 10 ) | $clock_id ); } diff --git a/tests/phpunit/tests/transformer/class-test-tid.php b/tests/phpunit/tests/transformer/class-test-tid.php index 7fa423e..95e2de7 100644 --- a/tests/phpunit/tests/transformer/class-test-tid.php +++ b/tests/phpunit/tests/transformer/class-test-tid.php @@ -157,6 +157,32 @@ public function test_generate_for_time_sorts_before_current() { $this->assertLessThan( $current, $historical, 'A 2010 TID must sort before a now-minted TID.' ); } + /** + * The salt argument to generate_for_time() makes two records that + * map to the same microsecond (e.g. IDs that collide modulo 1M + * inside the same GMT second) mint distinct rkeys. + */ + public function test_generate_for_time_salt_disambiguates_modulo_collision() { + $gmt = '2019-03-14 15:09:26'; + + // IDs that collide on `% 1_000_000` produce identical microseconds. + $micros_a = TID::microseconds_from_post_date( $gmt, 42 ); + $micros_b = TID::microseconds_from_post_date( $gmt, 1_000_042 ); + $this->assertSame( $micros_a, $micros_b, 'Setup: ids differing by 1M must share microseconds.' ); + + // Without a salt, the rkey collides — that's the documented + // failure mode we are guarding against. + $rkey_no_salt_a = TID::generate_for_time( $micros_a ); + $rkey_no_salt_b = TID::generate_for_time( $micros_b ); + $this->assertSame( $rkey_no_salt_a, $rkey_no_salt_b, 'Sanity: no-salt path collides when microseconds match.' ); + + // With a per-record salt, the rkeys must differ even though + // the microsecond portion is identical. + $rkey_a = TID::generate_for_time( $micros_a, 'post:42' ); + $rkey_b = TID::generate_for_time( $micros_b, 'post:1000042' ); + $this->assertNotSame( $rkey_a, $rkey_b, 'Distinct salts must mint distinct rkeys.' ); + } + /** * The namespace argument prevents collisions between WP_Post and * WP_Comment records that share an AT Protocol collection but have From cd2ba18c79089a3695dd030d5ce7599c57d448d7 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Sat, 23 May 2026 22:13:55 -0500 Subject: [PATCH 06/11] refactor: extract mint_historical_rkey to Base, hoist salt constants --- includes/transformer/class-base.php | 70 ++++++++++++++++++++++ includes/transformer/class-comment.php | 78 +++++++++++-------------- includes/transformer/class-document.php | 62 +++++++------------- includes/transformer/class-post.php | 76 +++++++----------------- 4 files changed, 147 insertions(+), 139 deletions(-) diff --git a/includes/transformer/class-base.php b/includes/transformer/class-base.php index 99a3163..fc96e15 100644 --- a/includes/transformer/class-base.php +++ b/includes/transformer/class-base.php @@ -72,6 +72,76 @@ public function get_uri(): string { ); } + /** + * Mint an rkey that honors the historical-TID filter contract. + * + * Shared by `Post::get_rkey()`, `Document::get_rkey()`, and + * `Comment::get_rkey()`. Each subclass calls this with its own + * collection NSID, the relevant `*_date_gmt` string and ID, a + * per-class salt prefix (so `applyWrites` can never see two records + * collapse onto the same rkey within a single collection), and an + * optional kind label that separates records of different WordPress + * object kinds when they share an AT Protocol collection. + * + * The historical path falls back to `TID::generate()` when either + * the filter opts out or the GMT datetime cannot be parsed + * (empty string, MySQL `0000-00-00 00:00:00` sentinel, garbage), + * so callers never need to handle the "no usable date" case. + * + * @since unreleased + * + * @param string $collection AT Protocol collection NSID. + * @param string $gmt_datetime GMT datetime string from the WP object. + * @param int $object_id Post or comment identifier. + * @param string $salt_prefix Per-class salt prefix (e.g. `'post:'`). + * @param \WP_Post|\WP_Comment $wp_object WordPress object being transformed. + * @param string $kind Optional kind label for collection-sharing + * record types (e.g. `'comment'`). + * @return string Minted TID rkey. + */ + final protected function mint_historical_rkey( + string $collection, + string $gmt_datetime, + int $object_id, + string $salt_prefix, + \WP_Post|\WP_Comment $wp_object, + string $kind = '' + ): string { + /** + * Filters whether to mint a historical TID based on the post's + * original publish date. + * + * Defaults to true so backfilled posts get rkeys that reflect + * the original publish time rather than the time of the + * backfill run. Return false to fall back to the live-publish + * `TID::generate()` path (now-based, with the monotonic floor). + * + * @since unreleased + * + * @param bool $use_historical Whether to mint a historical TID. + * @param \WP_Post|\WP_Comment $object WordPress object being transformed. + * @param string $collection AT Protocol collection NSID. + */ + $use_historical = (bool) \apply_filters( + 'atmosphere_use_historical_tid', + true, + $wp_object, + $collection + ); + + if ( ! $use_historical ) { + return TID::generate(); + } + + $microseconds = TID::microseconds_from_post_date( $gmt_datetime, $object_id, $kind ); + + if ( $microseconds <= 0 ) { + return TID::generate(); + } + + return TID::generate_for_time( $microseconds, $salt_prefix . $object_id ); + } + /** * WordPress locale as BCP-47 language tag array. * diff --git a/includes/transformer/class-comment.php b/includes/transformer/class-comment.php index 3ba2c59..6fb1932 100644 --- a/includes/transformer/class-comment.php +++ b/includes/transformer/class-comment.php @@ -31,6 +31,33 @@ class Comment extends Base { * * @var string */ + /** + * Salt prefix passed to {@see TID::generate_for_time()} for comments. + * + * Combined with the comment ID it produces the deterministic + * clock-id input for the historical TID path. + * + * @since unreleased + * + * @var string + */ + public const TID_SALT_PREFIX = 'comment:'; + + /** + * Kind label passed to {@see TID::microseconds_from_post_date()}. + * + * Comments share the `app.bsky.feed.post` collection with posts, so + * a post-id-vs-comment-id collision inside a single GMT second would + * otherwise mint identical rkeys. The kind label folds an extra + * deterministic offset into the microsecond portion so posts and + * comments occupy different sub-second windows. + * + * @since unreleased + * + * @var string + */ + public const TID_KIND = 'comment'; + public const META_TID = '_atmosphere_bsky_tid'; /** @@ -139,55 +166,20 @@ public function get_rkey(): string { $rkey = \get_comment_meta( (int) $this->object->comment_ID, self::META_TID, true ); if ( empty( $rkey ) ) { - $rkey = self::mint_rkey( $this->object ); + $rkey = $this->mint_historical_rkey( + $this->get_collection(), + (string) $this->object->comment_date_gmt, + (int) $this->object->comment_ID, + self::TID_SALT_PREFIX, + $this->object, + self::TID_KIND + ); \update_comment_meta( (int) $this->object->comment_ID, self::META_TID, $rkey ); } return $rkey; } - /** - * Mint a new TID for this comment, honoring the historical-TID filter. - * - * Default is a historical TID derived from `comment_date_gmt`, so - * backfilled comments land at their original post position in the - * AT Protocol repo. Filter listeners can return false to fall back - * to a now-based TID. - * - * Falls back to {@see TID::generate()} when `comment_date_gmt` - * cannot be parsed so we never mint an epoch-anchored TID. - * - * @since unreleased - * - * @param \WP_Comment $comment Comment being published. - * @return string TID rkey. - */ - private static function mint_rkey( \WP_Comment $comment ): string { - /** This filter is documented in includes/transformer/class-post.php */ - $use_historical = (bool) \apply_filters( - 'atmosphere_use_historical_tid', - true, - $comment, - 'app.bsky.feed.post' - ); - - if ( ! $use_historical ) { - return TID::generate(); - } - - $microseconds = TID::microseconds_from_post_date( - (string) $comment->comment_date_gmt, - (int) $comment->comment_ID, - 'comment' - ); - - if ( $microseconds <= 0 ) { - return TID::generate(); - } - - return TID::generate_for_time( $microseconds, 'comment:' . $comment->comment_ID ); - } - /** * Build the reply struct with root and parent refs. * diff --git a/includes/transformer/class-document.php b/includes/transformer/class-document.php index fb24a4b..f8def3e 100644 --- a/includes/transformer/class-document.php +++ b/includes/transformer/class-document.php @@ -28,6 +28,20 @@ class Document extends Base { * * @var string */ + /** + * Salt prefix passed to {@see TID::generate_for_time()} for documents. + * + * Combined with the post ID it produces the deterministic clock-id + * input for the historical TID path; namespacing by class keeps + * `Document` and `Post` rkeys distinct even though both derive from + * the same `WP_Post`. + * + * @since unreleased + * + * @var string + */ + public const TID_SALT_PREFIX = 'document:'; + public const META_TID = '_atmosphere_doc_tid'; /** @@ -199,53 +213,19 @@ public function get_rkey(): string { $rkey = \get_post_meta( $this->object->ID, self::META_TID, true ); if ( empty( $rkey ) ) { - $rkey = self::mint_rkey( $this->object ); + $rkey = $this->mint_historical_rkey( + $this->get_collection(), + (string) $this->object->post_date_gmt, + (int) $this->object->ID, + self::TID_SALT_PREFIX, + $this->object + ); \update_post_meta( $this->object->ID, self::META_TID, $rkey ); } return $rkey; } - /** - * Mint a new TID for this document, honoring the historical-TID filter. - * - * Default is a historical TID derived from `post_date_gmt`, so - * backfilled documents land at their original publish position in - * the AT Protocol repo. Filter listeners can return false to fall - * back to a now-based TID — useful for sites that intentionally - * want commit-order rkeys regardless of original publish date. - * - * Falls back to {@see TID::generate()} when `post_date_gmt` cannot - * be parsed (e.g. the `0000-00-00 00:00:00` sentinel) so we never - * mint an epoch-anchored TID. - * - * @since unreleased - * - * @param \WP_Post $post Post being published. - * @return string TID rkey. - */ - private static function mint_rkey( \WP_Post $post ): string { - /** This filter is documented in includes/transformer/class-post.php */ - $use_historical = (bool) \apply_filters( - 'atmosphere_use_historical_tid', - true, - $post, - 'site.standard.document' - ); - - if ( ! $use_historical ) { - return TID::generate(); - } - - $microseconds = TID::microseconds_from_post_date( (string) $post->post_date_gmt, (int) $post->ID ); - - if ( $microseconds <= 0 ) { - return TID::generate(); - } - - return TID::generate_for_time( $microseconds, 'document:' . $post->ID ); - } - /** * Get parsed content for the document's content union field. * diff --git a/includes/transformer/class-post.php b/includes/transformer/class-post.php index 184d31a..a3317dd 100644 --- a/includes/transformer/class-post.php +++ b/includes/transformer/class-post.php @@ -27,6 +27,20 @@ class Post extends Base { * * @var string */ + /** + * Salt prefix passed to {@see TID::generate_for_time()} for posts. + * + * Combined with the post ID it produces the deterministic clock-id + * input for the historical TID path; namespacing by class keeps + * `Post` and `Document` rkeys distinct even though both derive from + * the same `WP_Post`. + * + * @since unreleased + * + * @var string + */ + public const TID_SALT_PREFIX = 'post:'; + public const META_TID = '_atmosphere_bsky_tid'; /** @@ -277,67 +291,19 @@ public function get_rkey(): string { $rkey = \get_post_meta( $this->object->ID, self::META_TID, true ); if ( empty( $rkey ) ) { - $rkey = self::mint_rkey( $this->object ); + $rkey = $this->mint_historical_rkey( + $this->get_collection(), + (string) $this->object->post_date_gmt, + (int) $this->object->ID, + self::TID_SALT_PREFIX, + $this->object + ); \update_post_meta( $this->object->ID, self::META_TID, $rkey ); } return $rkey; } - /** - * Mint a new TID for this post, honoring the historical-TID filter. - * - * Default is a historical TID derived from `post_date_gmt`, so - * backfilled posts land at their original publish position in the - * AT Protocol repo. Filter listeners can return false to fall back - * to a now-based TID — useful for sites that intentionally want - * commit-order rkeys regardless of original publish date. - * - * Falls back to {@see TID::generate()} when `post_date_gmt` cannot - * be parsed (e.g. the `0000-00-00 00:00:00` sentinel) so we never - * mint an epoch-anchored TID. - * - * @since unreleased - * - * @param \WP_Post $post Post being published. - * @return string TID rkey. - */ - private static function mint_rkey( \WP_Post $post ): string { - /** - * Filters whether to mint a historical TID based on the post's - * original publish date. - * - * Defaults to true so backfilled posts get rkeys that reflect - * the original publish time rather than the time of the - * backfill run. Return false to fall back to the live-publish - * `TID::generate()` path (now-based, with the monotonic floor). - * - * @since unreleased - * - * @param bool $use_historical Whether to mint a historical TID. - * @param \WP_Post|\WP_Comment $object WordPress object being transformed. - * @param string $collection AT Protocol collection NSID. - */ - $use_historical = (bool) \apply_filters( - 'atmosphere_use_historical_tid', - true, - $post, - 'app.bsky.feed.post' - ); - - if ( ! $use_historical ) { - return TID::generate(); - } - - $microseconds = TID::microseconds_from_post_date( (string) $post->post_date_gmt, (int) $post->ID ); - - if ( $microseconds <= 0 ) { - return TID::generate(); - } - - return TID::generate_for_time( $microseconds, 'post:' . $post->ID ); - } - /** * Compose the post text: title + excerpt + permalink within 300 characters. * From 28a7283f45571872430d2c4349e6b1d46c96269a Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Sat, 23 May 2026 22:15:11 -0500 Subject: [PATCH 07/11] fix: validate generate_for_time microsecond bounds --- includes/transformer/class-tid.php | 25 +++++++++- .../tests/transformer/class-test-tid.php | 48 +++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/includes/transformer/class-tid.php b/includes/transformer/class-tid.php index 2c1d33d..f38ecd3 100644 --- a/includes/transformer/class-tid.php +++ b/includes/transformer/class-tid.php @@ -201,7 +201,26 @@ public static function generate(): string { * * @since unreleased * - * @param int $microseconds Microseconds since the Unix epoch. + * Non-positive `$microseconds` (zero or negative) fall through to + * {@see self::generate()} rather than encoding garbage. This is a + * second line of defence: the standard caller path goes through + * {@see self::microseconds_from_post_date()} which already returns + * 0 for unparseable input, but a direct caller that hands in `0`, + * a negative pre-epoch timestamp, or a value large enough that + * `($microseconds << 10)` overflows into a negative 64-bit integer + * would otherwise mint an unsortable / negative-encoded TID and + * silently violate the "13-character sortable identifier" contract. + * + * The upper bound is `PHP_INT_MAX >> 10` — anything above that + * cannot survive the shift inside a signed 64-bit int. That bound + * corresponds to roughly the year 294,247, so the guard is purely + * defensive against caller bugs (a stray `* 1_000_000` applied + * twice, for instance). + * + * @param int $microseconds Microseconds since the Unix epoch. Must + * be in `(0, PHP_INT_MAX >> 10]` to mint + * a historical TID; out-of-range values + * fall back to {@see self::generate()}. * @param string $salt Deterministic disambiguation salt * (typically `$kind . ':' . $object_id`). * Defaults to the microseconds string so @@ -209,6 +228,10 @@ public static function generate(): string { * @return string 13-character identifier. */ public static function generate_for_time( int $microseconds, string $salt = '' ): string { + if ( $microseconds <= 0 || $microseconds > ( \PHP_INT_MAX >> 10 ) ) { + return self::generate(); + } + $clock_source = '' === $salt ? (string) $microseconds : $salt; $clock_id = \crc32( $clock_source ) & 0x3FF; diff --git a/tests/phpunit/tests/transformer/class-test-tid.php b/tests/phpunit/tests/transformer/class-test-tid.php index 95e2de7..616fdd4 100644 --- a/tests/phpunit/tests/transformer/class-test-tid.php +++ b/tests/phpunit/tests/transformer/class-test-tid.php @@ -143,6 +143,18 @@ public function test_microseconds_from_post_date_returns_zero_on_parse_failure() $this->assertSame( 0, TID::microseconds_from_post_date( 'not a date', 42 ) ); } + /** + * Pre-epoch dates are rejected so the historical path never mints a + * negative-encoded TID. Posts dated before 1970-01-01 UTC are + * vanishingly rare in WordPress but possible (manual `post_date_gmt` + * edits, imports from legacy systems); the explicit guard keeps the + * caller-side fallback to {@see TID::generate()} working. + */ + public function test_microseconds_from_post_date_returns_zero_for_pre_epoch_date() { + $this->assertSame( 0, TID::microseconds_from_post_date( '1969-12-31 23:59:00', 42 ) ); + $this->assertSame( 0, TID::microseconds_from_post_date( '1900-01-01 00:00:00', 42 ) ); + } + /** * A historical-mint with an old timestamp produces a TID that sorts * well before a TID minted from `microtime(true)` — proving the @@ -183,6 +195,42 @@ public function test_generate_for_time_salt_disambiguates_modulo_collision() { $this->assertNotSame( $rkey_a, $rkey_b, 'Distinct salts must mint distinct rkeys.' ); } + /** + * Non-positive microseconds fall back to {@see TID::generate()} + * rather than encoding a negative or epoch-anchored value. Mirrors + * the parse-failure guard in `microseconds_from_post_date()` and + * protects direct callers who hand the helper their own integer. + */ + public function test_generate_for_time_rejects_non_positive_microseconds() { + $zero_tid = TID::generate_for_time( 0, 'post:1' ); + $negative_tid = TID::generate_for_time( -1, 'post:1' ); + + $this->assertTrue( TID::is_valid( $zero_tid ), 'Zero microseconds must fall back to a valid TID.' ); + $this->assertTrue( TID::is_valid( $negative_tid ), 'Negative microseconds must fall back to a valid TID.' ); + + // Fall-back path goes through `generate()` which uses the live + // monotonic floor, so the fallback rkey must sort after a 2010 + // historical TID — the same ordering invariant that proves the + // fallback didn't accidentally mint a near-epoch garbage value. + $historical_2010 = TID::generate_for_time( + TID::microseconds_from_post_date( '2010-01-01 00:00:00', 1 ), + 'post:1' + ); + $this->assertGreaterThan( $historical_2010, $zero_tid ); + $this->assertGreaterThan( $historical_2010, $negative_tid ); + } + + /** + * Microsecond values large enough to overflow into negative when + * shifted left by 10 bits also fall back to `generate()`. Guards + * against caller bugs (e.g. an accidental double `* 1_000_000`). + */ + public function test_generate_for_time_rejects_oversized_microseconds() { + $overflow_tid = TID::generate_for_time( \PHP_INT_MAX, 'post:1' ); + + $this->assertTrue( TID::is_valid( $overflow_tid ), 'Oversized microseconds must fall back to a valid TID.' ); + } + /** * The namespace argument prevents collisions between WP_Post and * WP_Comment records that share an AT Protocol collection but have From 5b7a7ba7a8ea0d379f399666953ba4e0c5202ec4 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Sat, 23 May 2026 22:18:50 -0500 Subject: [PATCH 08/11] test: cover Post/Comment historical TID paths with decoded-rkey checks --- .../tests/transformer/class-test-comment.php | 148 ++++++++++++++++++ .../tests/transformer/class-test-document.php | 32 +++- .../tests/transformer/class-test-post.php | 63 ++++++++ .../tests/transformer/class-tid-decoder.php | 61 ++++++++ 4 files changed, 297 insertions(+), 7 deletions(-) create mode 100644 tests/phpunit/tests/transformer/class-tid-decoder.php diff --git a/tests/phpunit/tests/transformer/class-test-comment.php b/tests/phpunit/tests/transformer/class-test-comment.php index a878df2..747560c 100644 --- a/tests/phpunit/tests/transformer/class-test-comment.php +++ b/tests/phpunit/tests/transformer/class-test-comment.php @@ -13,6 +13,9 @@ use Atmosphere\Reaction_Sync; use Atmosphere\Transformer\Comment; use Atmosphere\Transformer\Post; +use Atmosphere\Transformer\TID; + +require_once __DIR__ . '/class-tid-decoder.php'; /** * Comment transformer tests. @@ -58,6 +61,7 @@ public function set_up(): void { */ public function tear_down(): void { \remove_all_filters( 'atmosphere_transform_comment' ); + \remove_all_filters( 'atmosphere_use_historical_tid' ); parent::tear_down(); } @@ -298,4 +302,148 @@ public function test_long_content_is_truncated() { $this->assertLessThanOrEqual( 300, \mb_strlen( $record['text'] ) ); } + + /** + * By default, Comment::get_rkey() mints a historical TID anchored + * on `comment_date_gmt`. The decoded microseconds must match the + * helper output (including the `comment` kind label that offsets + * comments away from posts in the shared collection). + * + * @covers ::get_rkey + */ + public function test_get_rkey_defaults_to_historical_tid() { + $comment_id = self::factory()->comment->create( + array( + 'comment_post_ID' => $this->post_id, + 'comment_date' => '2014-08-20 10:15:00', + 'comment_date_gmt' => '2014-08-20 10:15:00', + 'comment_content' => 'Historical comment.', + 'user_id' => 1, + ) + ); + + $comment = \get_comment( $comment_id ); + $historical_rkey = ( new Comment( $comment ) )->get_rkey(); + $current_rkey = TID::generate(); + + $this->assertNotEmpty( $historical_rkey ); + $this->assertTrue( TID::is_valid( $historical_rkey ) ); + $this->assertLessThan( $current_rkey, $historical_rkey, '2014-anchored rkey must sort before a now-minted TID.' ); + + $expected_microseconds = TID::microseconds_from_post_date( + '2014-08-20 10:15:00', + (int) $comment_id, + Comment::TID_KIND + ); + $this->assertSame( + $expected_microseconds, + TID_Decoder::tid_to_microseconds( $historical_rkey ), + 'Decoded rkey microseconds must match microseconds_from_post_date() including the comment kind.' + ); + } + + /** + * Listeners returning false from atmosphere_use_historical_tid + * fall back to the now-based TID::generate() path for comments + * just like posts. + * + * @covers ::get_rkey + */ + public function test_get_rkey_filter_opt_out_uses_current_time() { + \add_filter( 'atmosphere_use_historical_tid', '__return_false' ); + + $comment_id = self::factory()->comment->create( + array( + 'comment_post_ID' => $this->post_id, + 'comment_date' => '2014-08-20 10:15:00', + 'comment_date_gmt' => '2014-08-20 10:15:00', + 'user_id' => 1, + ) + ); + + $comment = \get_comment( $comment_id ); + $baseline = TID::generate(); + $rkey = ( new Comment( $comment ) )->get_rkey(); + $historical_2014 = TID::generate_for_time( + TID::microseconds_from_post_date( '2014-08-20 10:15:00', (int) $comment_id, Comment::TID_KIND ), + Comment::TID_SALT_PREFIX . $comment_id + ); + + $this->assertGreaterThan( $baseline, $rkey, 'Opting out via filter must mint a now-based TID.' ); + $this->assertGreaterThan( $historical_2014, $rkey, 'Opted-out TID must sort after a 2014 historical TID.' ); + } + + /** + * The headline test for the `$kind` argument: a Post with `ID == N` + * and a Comment with `comment_ID == N`, both with matching + * `*_date_gmt` values, must mint distinct rkeys even though both + * live in `app.bsky.feed.post`. Without the kind offset, both would + * collapse onto the same microsecond + salt-prefix-only-by-class + * encoding and `applyWrites` would reject the second create. + * + * @covers ::get_rkey + */ + public function test_post_and_comment_with_matching_id_and_date_mint_distinct_rkeys() { + $gmt = '2018-04-01 12:00:00'; + + // Force matching IDs by inserting the comment with an explicit + // `comment_ID` equal to a known post ID. WordPress doesn't make + // `comment_ID` directly settable through the factory, so insert + // the post, then the comment, and assert on the *salt+kind* + // collision shape that would otherwise produce identical rkeys: + // equal numeric IDs (the disambiguator inside one second) and + // equal GMT seconds. + $post = self::factory()->post->create_and_get( + array( + 'post_date' => $gmt, + 'post_date_gmt' => $gmt, + ) + ); + + $comment_id = self::factory()->comment->create( + array( + 'comment_post_ID' => $this->post_id, + 'comment_date' => $gmt, + 'comment_date_gmt' => $gmt, + 'comment_content' => 'Cross-kind test.', + 'user_id' => 1, + ) + ); + + // Force the salt collision: assert directly on the TID helper + // with id parity, because comment_ID auto-increments separately + // from post IDs and we cannot guarantee equality through the + // factory. The transformer wiring then proves get_rkey() goes + // through the namespaced helper in production. + $shared_id = 4242; + + $post_microseconds = TID::microseconds_from_post_date( $gmt, $shared_id ); + $comment_microseconds = TID::microseconds_from_post_date( $gmt, $shared_id, Comment::TID_KIND ); + + $this->assertNotSame( + $post_microseconds, + $comment_microseconds, + 'A post and a comment with identical id+date must map to different microseconds.' + ); + + $post_rkey = TID::generate_for_time( $post_microseconds, Post::TID_SALT_PREFIX . $shared_id ); + $comment_rkey = TID::generate_for_time( $comment_microseconds, Comment::TID_SALT_PREFIX . $shared_id ); + + $this->assertNotSame( + $post_rkey, + $comment_rkey, + 'Salt prefix + kind together must produce distinct rkeys for the collision case.' + ); + + // Sanity-check the production wiring: a real Post and Comment + // transformer with this same gmt still produce valid, distinct + // rkeys via get_rkey(). + $comment = \get_comment( $comment_id ); + $real_post_key = ( new Post( $post ) )->get_rkey(); + $real_comm_key = ( new Comment( $comment ) )->get_rkey(); + + $this->assertTrue( TID::is_valid( $real_post_key ) ); + $this->assertTrue( TID::is_valid( $real_comm_key ) ); + $this->assertNotSame( $real_post_key, $real_comm_key ); + } } diff --git a/tests/phpunit/tests/transformer/class-test-document.php b/tests/phpunit/tests/transformer/class-test-document.php index b2c3932..f8e840f 100644 --- a/tests/phpunit/tests/transformer/class-test-document.php +++ b/tests/phpunit/tests/transformer/class-test-document.php @@ -10,16 +10,26 @@ namespace Atmosphere\Tests\Transformer; require_once __DIR__ . '/class-stub-parser.php'; +require_once __DIR__ . '/class-tid-decoder.php'; use WP_UnitTestCase; use Atmosphere\Transformer\Document; use Atmosphere\Transformer\Post; +use Atmosphere\Transformer\TID; /** * Document transformer tests. */ class Test_Document extends WP_UnitTestCase { + /** + * Remove any filter overrides so they do not leak between tests. + */ + public function tear_down(): void { + \remove_all_filters( 'atmosphere_use_historical_tid' ); + parent::tear_down(); + } + /** * Test that content field is absent when no parser is registered. */ @@ -324,11 +334,20 @@ public function test_get_rkey_defaults_to_historical_tid() { // Capture the historical rkey first so the assertion compares // purely TID encodings rather than the side effect of meta. $historical_rkey = ( new Document( $post ) )->get_rkey(); - $current_rkey = \Atmosphere\Transformer\TID::generate(); + $current_rkey = TID::generate(); $this->assertNotEmpty( $historical_rkey ); - $this->assertTrue( \Atmosphere\Transformer\TID::is_valid( $historical_rkey ) ); + $this->assertTrue( TID::is_valid( $historical_rkey ) ); $this->assertLessThan( $current_rkey, $historical_rkey, '2010-anchored rkey must sort before a now-minted TID.' ); + + // Lock the encoding contract: decoding the rkey must return the + // same microseconds we'd get from the helper for this post. + $expected_microseconds = TID::microseconds_from_post_date( '2010-01-15 12:00:00', $post->ID ); + $this->assertSame( + $expected_microseconds, + TID_Decoder::tid_to_microseconds( $historical_rkey ), + 'Decoded rkey microseconds must match microseconds_from_post_date().' + ); } /** @@ -350,15 +369,14 @@ public function test_get_rkey_filter_opt_out_uses_current_time() { // A baseline current-time TID minted just before the rkey: // the filter-disabled rkey should sort right next to it, // not anywhere near a 2010 historical TID. - $baseline = \Atmosphere\Transformer\TID::generate(); + $baseline = TID::generate(); $rkey = ( new Document( $post ) )->get_rkey(); - $historical_2010 = \Atmosphere\Transformer\TID::generate_for_time( - \Atmosphere\Transformer\TID::microseconds_from_post_date( '2010-01-15 12:00:00', $post->ID ) + $historical_2010 = TID::generate_for_time( + TID::microseconds_from_post_date( '2010-01-15 12:00:00', $post->ID ), + Document::TID_SALT_PREFIX . $post->ID ); $this->assertGreaterThan( $baseline, $rkey, 'Opting out via filter must mint a now-based TID.' ); $this->assertGreaterThan( $historical_2010, $rkey, 'Opted-out TID must sort after a 2010 historical TID.' ); - - \remove_all_filters( 'atmosphere_use_historical_tid' ); } } diff --git a/tests/phpunit/tests/transformer/class-test-post.php b/tests/phpunit/tests/transformer/class-test-post.php index 5aaaec9..c4c567a 100644 --- a/tests/phpunit/tests/transformer/class-test-post.php +++ b/tests/phpunit/tests/transformer/class-test-post.php @@ -11,6 +11,9 @@ use WP_UnitTestCase; use Atmosphere\Transformer\Post; +use Atmosphere\Transformer\TID; + +require_once __DIR__ . '/class-tid-decoder.php'; /** * Post transformer tests. @@ -28,6 +31,7 @@ public function tear_down() { \remove_all_filters( 'atmosphere_teaser_thread_posts' ); \remove_all_filters( 'atmosphere_transform_bsky_post' ); \remove_all_filters( 'atmosphere_post_embed' ); + \remove_all_filters( 'atmosphere_use_historical_tid' ); \remove_all_actions( 'atmosphere_long_form_strategy_downgraded' ); parent::tear_down(); } @@ -2030,4 +2034,63 @@ public function test_get_attachment_aspect_ratio_returns_null_for_zero_dimension $this->assertNull( Post::get_attachment_aspect_ratio( $attachment_id ) ); } + + /** + * By default, Post::get_rkey() mints a historical TID anchored on + * the post's original publish date. The decoded microseconds must + * match microseconds_from_post_date(), locking the encoding + * contract end-to-end. + * + * @covers ::get_rkey + */ + public function test_get_rkey_defaults_to_historical_tid() { + $post = self::factory()->post->create_and_get( + array( + 'post_date' => '2012-06-15 09:30:00', + 'post_date_gmt' => '2012-06-15 09:30:00', + ) + ); + + $historical_rkey = ( new Post( $post ) )->get_rkey(); + $current_rkey = TID::generate(); + + $this->assertNotEmpty( $historical_rkey ); + $this->assertTrue( TID::is_valid( $historical_rkey ) ); + $this->assertLessThan( $current_rkey, $historical_rkey, '2012-anchored rkey must sort before a now-minted TID.' ); + + $expected_microseconds = TID::microseconds_from_post_date( '2012-06-15 09:30:00', $post->ID ); + $this->assertSame( + $expected_microseconds, + TID_Decoder::tid_to_microseconds( $historical_rkey ), + 'Decoded rkey microseconds must match microseconds_from_post_date().' + ); + } + + /** + * Listeners returning false from atmosphere_use_historical_tid + * fall back to the now-based TID::generate() path even when the + * post has a usable historical date. + * + * @covers ::get_rkey + */ + public function test_get_rkey_filter_opt_out_uses_current_time() { + \add_filter( 'atmosphere_use_historical_tid', '__return_false' ); + + $post = self::factory()->post->create_and_get( + array( + 'post_date' => '2012-06-15 09:30:00', + 'post_date_gmt' => '2012-06-15 09:30:00', + ) + ); + + $baseline = TID::generate(); + $rkey = ( new Post( $post ) )->get_rkey(); + $historical_2012 = TID::generate_for_time( + TID::microseconds_from_post_date( '2012-06-15 09:30:00', $post->ID ), + Post::TID_SALT_PREFIX . $post->ID + ); + + $this->assertGreaterThan( $baseline, $rkey, 'Opting out via filter must mint a now-based TID.' ); + $this->assertGreaterThan( $historical_2012, $rkey, 'Opted-out TID must sort after a 2012 historical TID.' ); + } } diff --git a/tests/phpunit/tests/transformer/class-tid-decoder.php b/tests/phpunit/tests/transformer/class-tid-decoder.php new file mode 100644 index 0000000..b050b4d --- /dev/null +++ b/tests/phpunit/tests/transformer/class-tid-decoder.php @@ -0,0 +1,61 @@ +> 10; + } +} From 8d2f8381195a80aa8001e0e22c7f66dbc0a3f102 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Sat, 23 May 2026 22:20:33 -0500 Subject: [PATCH 09/11] docs: tighten generate_for_time and microseconds_from_post_date docblocks --- includes/transformer/class-tid.php | 75 +++++++++++++++++------------- 1 file changed, 42 insertions(+), 33 deletions(-) diff --git a/includes/transformer/class-tid.php b/includes/transformer/class-tid.php index f38ecd3..367881a 100644 --- a/includes/transformer/class-tid.php +++ b/includes/transformer/class-tid.php @@ -180,9 +180,9 @@ public static function generate(): string { * value forward to "now" — defeating the entire point — or * regress the floor used by live publishing. Callers are * responsible for supplying a microsecond value that is - * collision-resistant within their batch (see + * collision-resistant within their batch; see * {@see self::microseconds_from_post_date()} for the standard - * deterministic helper). + * deterministic helper. * * The 10-bit clock identifier is derived deterministically from * `$salt` rather than the per-process random value used by @@ -190,39 +190,32 @@ public static function generate(): string { * PHP worker before the record's TID meta is persisted would mint * a different rkey for the same post — the AT Protocol create * would then succeed twice and orphan the first record. Callers - * should pass a salt that is unique to the record being minted - * (a string composed of object id + kind is sufficient). The - * companion {@see self::microseconds_from_post_date()} helper - * folds the same ID into the microsecond portion, and the salt - * provides the second-order entropy that protects against the - * "IDs differ by a multiple of 1,000,000 inside the same GMT - * second" modulo collision — both 10 bits of the rkey now depend - * on the full object identity, not just the truncated portion. + * should pass a salt that is unique to the record being minted; + * the conventional shape used in the bundled transformers is + * `"{kind}:{object_id}"` (e.g. `"post:42"`, `"comment:128"`), + * which together with the modulo-disambiguated microsecond + * portion makes both 10-bit fields depend on the full object + * identity — closing the "IDs differ by a multiple of 1,000,000 + * inside the same GMT second" modulo collision. * - * @since unreleased - * - * Non-positive `$microseconds` (zero or negative) fall through to - * {@see self::generate()} rather than encoding garbage. This is a - * second line of defence: the standard caller path goes through - * {@see self::microseconds_from_post_date()} which already returns - * 0 for unparseable input, but a direct caller that hands in `0`, - * a negative pre-epoch timestamp, or a value large enough that - * `($microseconds << 10)` overflows into a negative 64-bit integer - * would otherwise mint an unsortable / negative-encoded TID and - * silently violate the "13-character sortable identifier" contract. + * Non-positive `$microseconds` (zero or negative) and values + * above `PHP_INT_MAX >> 10` fall through to {@see self::generate()} + * rather than encoding garbage. The standard caller path through + * {@see self::microseconds_from_post_date()} already returns 0 + * for unparseable input, but this guard protects direct callers + * who hand the helper their own integer (a stray `* 1_000_000` + * applied twice, a negative pre-epoch timestamp) from silently + * minting an unsortable / negative-encoded TID. The upper bound + * corresponds to roughly the year 294,247. * - * The upper bound is `PHP_INT_MAX >> 10` — anything above that - * cannot survive the shift inside a signed 64-bit int. That bound - * corresponds to roughly the year 294,247, so the guard is purely - * defensive against caller bugs (a stray `* 1_000_000` applied - * twice, for instance). + * @since unreleased * * @param int $microseconds Microseconds since the Unix epoch. Must * be in `(0, PHP_INT_MAX >> 10]` to mint * a historical TID; out-of-range values * fall back to {@see self::generate()}. - * @param string $salt Deterministic disambiguation salt - * (typically `$kind . ':' . $object_id`). + * @param string $salt Deterministic disambiguation salt, + * conventionally `"{kind}:{object_id}"`. * Defaults to the microseconds string so * ad-hoc callers still get a stable rkey. * @return string 13-character identifier. @@ -261,17 +254,33 @@ public static function generate_for_time( int $microseconds, string $salt = '' ) * that's stable across runs (CRC32 is deterministic), preserving * idempotency per kind. * - * Returns `0` if the datetime can't be parsed; callers should - * decide whether to fall back to {@see self::generate()} in that - * case rather than minting an epoch-anchored TID. + * Bundled `$kind` values used by the plugin's transformers: + * + * - `''` (default) — `Post` and `Document`. Their AT collections + * (`app.bsky.feed.post` and `site.standard.document`) don't + * overlap so no kind disambiguation is needed. + * - `'comment'` — `Comment`. Shares `app.bsky.feed.post` with + * `Post`, so the kind offset is required to avoid the cross- + * kind ID collision described above. See `Comment::TID_KIND`. + * + * Returns `0` for any unparseable input — empty / whitespace-only + * strings, the MySQL `0000-00-00 00:00:00` sentinel, pre-epoch + * datetimes, or garbage `strtotime()` rejects. Callers must check + * for the zero return and fall back to {@see self::generate()} + * rather than passing the zero through to + * {@see self::generate_for_time()} (which would itself fall back, + * but the explicit check at the call site keeps the intent + * obvious). * * @since unreleased * * @param string $gmt_datetime GMT datetime string (e.g. `post_date_gmt`). * @param int $object_id Post or comment identifier for disambiguation. * @param string $kind Optional kind label to separate records - * sharing a collection (e.g. `post`, `comment`). - * @return int Microseconds since the Unix epoch, or 0 on parse failure. + * sharing a collection. See list above for + * the values used by bundled transformers. + * @return int Microseconds since the Unix epoch, or 0 on parse failure + * / pre-epoch / sentinel input. */ public static function microseconds_from_post_date( string $gmt_datetime, int $object_id, string $kind = '' ): int { $trimmed = \trim( $gmt_datetime ); From 15c05b0f2905cc2bb0e841e02426ef8c97b579c5 Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Sat, 23 May 2026 22:30:52 -0500 Subject: [PATCH 10/11] fix: restore META_TID docblocks and fix generate_for_time year reference --- includes/transformer/class-comment.php | 10 +- includes/transformer/class-document.php | 10 +- includes/transformer/class-post.php | 10 +- includes/transformer/class-tid.php | 2 +- .../tests/transformer/class-test-comment.php | 93 ++++++++++--------- 5 files changed, 65 insertions(+), 60 deletions(-) diff --git a/includes/transformer/class-comment.php b/includes/transformer/class-comment.php index 6fb1932..97b9bd8 100644 --- a/includes/transformer/class-comment.php +++ b/includes/transformer/class-comment.php @@ -26,11 +26,6 @@ */ class Comment extends Base { - /** - * Comment meta key for the bsky post TID (rkey). - * - * @var string - */ /** * Salt prefix passed to {@see TID::generate_for_time()} for comments. * @@ -58,6 +53,11 @@ class Comment extends Base { */ public const TID_KIND = 'comment'; + /** + * Comment meta key for the bsky post TID (rkey). + * + * @var string + */ public const META_TID = '_atmosphere_bsky_tid'; /** diff --git a/includes/transformer/class-document.php b/includes/transformer/class-document.php index f8def3e..eaeadc8 100644 --- a/includes/transformer/class-document.php +++ b/includes/transformer/class-document.php @@ -23,11 +23,6 @@ */ class Document extends Base { - /** - * Post meta key for the document TID. - * - * @var string - */ /** * Salt prefix passed to {@see TID::generate_for_time()} for documents. * @@ -42,6 +37,11 @@ class Document extends Base { */ public const TID_SALT_PREFIX = 'document:'; + /** + * Post meta key for the document TID. + * + * @var string + */ public const META_TID = '_atmosphere_doc_tid'; /** diff --git a/includes/transformer/class-post.php b/includes/transformer/class-post.php index a3317dd..f98a1ba 100644 --- a/includes/transformer/class-post.php +++ b/includes/transformer/class-post.php @@ -22,11 +22,6 @@ */ class Post extends Base { - /** - * Post meta key for the bsky post TID. - * - * @var string - */ /** * Salt prefix passed to {@see TID::generate_for_time()} for posts. * @@ -41,6 +36,11 @@ class Post extends Base { */ public const TID_SALT_PREFIX = 'post:'; + /** + * Post meta key for the bsky post TID. + * + * @var string + */ public const META_TID = '_atmosphere_bsky_tid'; /** diff --git a/includes/transformer/class-tid.php b/includes/transformer/class-tid.php index 367881a..7cb63c9 100644 --- a/includes/transformer/class-tid.php +++ b/includes/transformer/class-tid.php @@ -206,7 +206,7 @@ public static function generate(): string { * who hand the helper their own integer (a stray `* 1_000_000` * applied twice, a negative pre-epoch timestamp) from silently * minting an unsortable / negative-encoded TID. The upper bound - * corresponds to roughly the year 294,247. + * corresponds to roughly the year 2255. * * @since unreleased * diff --git a/tests/phpunit/tests/transformer/class-test-comment.php b/tests/phpunit/tests/transformer/class-test-comment.php index 747560c..70fe7cb 100644 --- a/tests/phpunit/tests/transformer/class-test-comment.php +++ b/tests/phpunit/tests/transformer/class-test-comment.php @@ -381,18 +381,17 @@ public function test_get_rkey_filter_opt_out_uses_current_time() { * collapse onto the same microsecond + salt-prefix-only-by-class * encoding and `applyWrites` would reject the second create. * + * Forces the ID collision via direct `$wpdb` insert because + * `comment_ID` and `post_ID` auto-increment from separate + * sequences — the test factories can't guarantee equality. + * * @covers ::get_rkey */ public function test_post_and_comment_with_matching_id_and_date_mint_distinct_rkeys() { + global $wpdb; + $gmt = '2018-04-01 12:00:00'; - // Force matching IDs by inserting the comment with an explicit - // `comment_ID` equal to a known post ID. WordPress doesn't make - // `comment_ID` directly settable through the factory, so insert - // the post, then the comment, and assert on the *salt+kind* - // collision shape that would otherwise produce identical rkeys: - // equal numeric IDs (the disambiguator inside one second) and - // equal GMT seconds. $post = self::factory()->post->create_and_get( array( 'post_date' => $gmt, @@ -400,50 +399,56 @@ public function test_post_and_comment_with_matching_id_and_date_mint_distinct_rk ) ); - $comment_id = self::factory()->comment->create( + // Insert a comment whose `comment_ID` literally equals the + // post ID. Bypass the factory + `wp_insert_comment` because + // neither honors a caller-supplied `comment_ID`; the AUTO_INCREMENT + // override is the whole point of the test. Use the same + // `$wpdb` direct-write pattern that lives in `class-tid.php` + // for its CAS write, with the same phpcs disable comment. + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.DirectDatabaseQuery.SchemaChange + $wpdb->insert( + $wpdb->comments, array( - 'comment_post_ID' => $this->post_id, - 'comment_date' => $gmt, - 'comment_date_gmt' => $gmt, - 'comment_content' => 'Cross-kind test.', - 'user_id' => 1, + 'comment_ID' => $post->ID, + 'comment_post_ID' => $this->post_id, + 'comment_author' => 'tester', + 'comment_author_email' => 'tester@example.com', + 'comment_date' => $gmt, + 'comment_date_gmt' => $gmt, + 'comment_content' => 'Cross-kind test.', + 'comment_approved' => '1', + 'user_id' => 1, ) ); - - // Force the salt collision: assert directly on the TID helper - // with id parity, because comment_ID auto-increments separately - // from post IDs and we cannot guarantee equality through the - // factory. The transformer wiring then proves get_rkey() goes - // through the namespaced helper in production. - $shared_id = 4242; - - $post_microseconds = TID::microseconds_from_post_date( $gmt, $shared_id ); - $comment_microseconds = TID::microseconds_from_post_date( $gmt, $shared_id, Comment::TID_KIND ); - - $this->assertNotSame( - $post_microseconds, - $comment_microseconds, - 'A post and a comment with identical id+date must map to different microseconds.' - ); - - $post_rkey = TID::generate_for_time( $post_microseconds, Post::TID_SALT_PREFIX . $shared_id ); - $comment_rkey = TID::generate_for_time( $comment_microseconds, Comment::TID_SALT_PREFIX . $shared_id ); - + \clean_comment_cache( $post->ID ); + + $comment = \get_comment( $post->ID ); + $this->assertNotNull( $comment, 'Sanity: the forced-ID comment must round-trip through get_comment().' ); + $this->assertSame( (int) $post->ID, (int) $comment->comment_ID, 'Setup: comment_ID must equal post ID.' ); + $this->assertSame( $gmt, $comment->comment_date_gmt, 'Setup: comment_date_gmt must match the post.' ); + + // End-to-end: each transformer mints through `get_rkey()` (which + // goes through `Base::mint_historical_rkey()` with the per-class + // salt + kind), and the resulting rkeys must differ even though + // both live inside `app.bsky.feed.post` with identical ID + date. + $post_rkey = ( new Post( $post ) )->get_rkey(); + $comment_rkey = ( new Comment( $comment ) )->get_rkey(); + + $this->assertTrue( TID::is_valid( $post_rkey ) ); + $this->assertTrue( TID::is_valid( $comment_rkey ) ); $this->assertNotSame( $post_rkey, $comment_rkey, - 'Salt prefix + kind together must produce distinct rkeys for the collision case.' + 'Post and Comment with identical id+date must mint distinct rkeys via the kind namespace.' ); - // Sanity-check the production wiring: a real Post and Comment - // transformer with this same gmt still produce valid, distinct - // rkeys via get_rkey(). - $comment = \get_comment( $comment_id ); - $real_post_key = ( new Post( $post ) )->get_rkey(); - $real_comm_key = ( new Comment( $comment ) )->get_rkey(); - - $this->assertTrue( TID::is_valid( $real_post_key ) ); - $this->assertTrue( TID::is_valid( $real_comm_key ) ); - $this->assertNotSame( $real_post_key, $real_comm_key ); + // Both transformers share the `app.bsky.feed.post` collection, + // so the rkeys must also differ at the microsecond level — not + // just at the 10-bit clock_id derived from the salt. + $this->assertNotSame( + TID_Decoder::tid_to_microseconds( $post_rkey ), + TID_Decoder::tid_to_microseconds( $comment_rkey ), + 'Microsecond portions must differ — the kind label offsets one of them inside the GMT second.' + ); } } From cabe0d2cc5462c1097a2f7fc583da0cfcab0e10b Mon Sep 17 00:00:00 2001 From: Brandon Kraft Date: Sat, 23 May 2026 22:43:13 -0500 Subject: [PATCH 11/11] fix: segregate microsecond ranges per kind to avoid rkey collisions --- includes/transformer/class-tid.php | 67 +++++++++++-------- .../tests/transformer/class-test-tid.php | 44 ++++++++++-- 2 files changed, 78 insertions(+), 33 deletions(-) diff --git a/includes/transformer/class-tid.php b/includes/transformer/class-tid.php index 7cb63c9..ffbf988 100644 --- a/includes/transformer/class-tid.php +++ b/includes/transformer/class-tid.php @@ -244,24 +244,35 @@ public static function generate_for_time( int $microseconds, string $salt = '' ) * for the same record, which keeps the operation idempotent * against `applyWrites`. * - * The `$kind` argument disambiguates records that share an AT - * Protocol collection but are sourced from different WordPress - * object kinds. Without it, a `WP_Post` with id N and a - * `WP_Comment` with `comment_ID` N published in the same GMT - * second within a single PHP process would mint the same rkey - * inside `app.bsky.feed.post` and `applyWrites` would reject the - * second create. The kind label is hashed into a sub-second offset - * that's stable across runs (CRC32 is deterministic), preserving - * idempotency per kind. + * Cross-kind rkey collisions in the same collection (e.g. a Post + * and a Comment both landing in `app.bsky.feed.post`) are + * impossible by construction: posts/documents occupy microseconds + * 0..499,999 within each second, comments occupy 500,000..999,999. + * Even if the salt-derived 10-bit `clock_id` collides across kinds + * (1/1024 chance through CRC32), the microsecond field differs and + * the rkeys differ. Within a single kind, two records published in + * the exact same second still need a matching `object_id % 500_000` + * AND a `clock_id` collision — astronomically unlikely in practice. + * + * An earlier shape folded `crc32($kind)` into the same 0..999_999 + * window via modulo addition. That preserved idempotency per kind + * but did not guarantee disjoint ranges, so specific cross-kind ID + * pairs (e.g. post id 1288 vs comment id 350044 at the same GMT + * second) could still collapse onto identical microseconds — and, + * when the salt-derived clock_id also collided, identical rkeys. + * Codex surfaced that pattern by construction during PR review; + * range segregation removes the failure mode at its root rather + * than relying on the hash space staying lucky. * * Bundled `$kind` values used by the plugin's transformers: * * - `''` (default) — `Post` and `Document`. Their AT collections * (`app.bsky.feed.post` and `site.standard.document`) don't - * overlap so no kind disambiguation is needed. + * overlap so no kind segregation is needed and they share the + * 0..499,999 range. * - `'comment'` — `Comment`. Shares `app.bsky.feed.post` with - * `Post`, so the kind offset is required to avoid the cross- - * kind ID collision described above. See `Comment::TID_KIND`. + * `Post`, so it occupies the 500,000..999,999 range. See + * `Comment::TID_KIND`. * * Returns `0` for any unparseable input — empty / whitespace-only * strings, the MySQL `0000-00-00 00:00:00` sentinel, pre-epoch @@ -299,22 +310,24 @@ public static function microseconds_from_post_date( string $gmt_datetime, int $o return 0; } - $offset = $object_id % 1_000_000; - - if ( '' !== $kind ) { - /* - * Fold the kind label into the same 0..999_999 microsecond - * window so the result still lands inside the post's - * original GMT second. CRC32 is deterministic across runs, - * which preserves the idempotency property — same input - * mints the same TID — while moving the two kinds far - * enough apart in microsecond-space that an ID-level - * collision across kinds is astronomically unlikely. - */ - $offset = ( $offset + \crc32( $kind ) ) % 1_000_000; - } + /* + * Reserve disjoint microsecond ranges per kind so cross-kind + * rkeys can never share the microsecond field — eliminating + * the failure mode regardless of whether the salt-derived + * `clock_id` ever collides: + * + * - `''` (post / document) → 0..499,999 within the second. + * - `'comment'` → 500,000..999,999 within the second. + * + * Within a single kind the `(object_id % 500_000)` term still + * disambiguates same-second records, paired with the + * salt-derived `clock_id` for the final 10 bits. + */ + $kind_offset = ( 'comment' === $kind ) ? 500_000 : 0; + $within_kind = $object_id % 500_000; + $micros_within_second = $kind_offset + $within_kind; - return ( $seconds * 1_000_000 ) + $offset; + return ( $seconds * 1_000_000 ) + $micros_within_second; } /** diff --git a/tests/phpunit/tests/transformer/class-test-tid.php b/tests/phpunit/tests/transformer/class-test-tid.php index 616fdd4..52fd98a 100644 --- a/tests/phpunit/tests/transformer/class-test-tid.php +++ b/tests/phpunit/tests/transformer/class-test-tid.php @@ -232,13 +232,15 @@ public function test_generate_for_time_rejects_oversized_microseconds() { } /** - * The namespace argument prevents collisions between WP_Post and - * WP_Comment records that share an AT Protocol collection but have - * matching IDs and timestamps. + * The kind argument segregates Post/Document records (microseconds + * 0..499,999 within each second) from Comment records (500,000.. + * 999,999), so they cannot share the microsecond field even with + * matching IDs and timestamps. Idempotency per kind still holds. */ public function test_microseconds_from_post_date_namespace_disambiguates() { - $gmt = '2019-03-14 15:09:26'; - $id = 42; + $gmt = '2019-03-14 15:09:26'; + $id = 42; + $seconds = (int) \strtotime( $gmt . ' UTC' ); $post = TID::microseconds_from_post_date( $gmt, $id ); $comment = TID::microseconds_from_post_date( $gmt, $id, 'comment' ); @@ -250,8 +252,38 @@ public function test_microseconds_from_post_date_namespace_disambiguates() { $this->assertSame( $comment, $comment_again, 'Same namespace + id + second must be idempotent.' ); // Both still land inside the same GMT second. - $seconds = (int) \strtotime( $gmt . ' UTC' ); $this->assertSame( $seconds, \intdiv( $post, 1_000_000 ) ); $this->assertSame( $seconds, \intdiv( $comment, 1_000_000 ) ); + + // Disjoint ranges by construction: posts/documents in 0..499_999, + // comments in 500_000..999_999. Lock the exact offsets for id 42 + // so a future refactor cannot silently shrink the segregation. + $this->assertSame( ( $seconds * 1_000_000 ) + 42, $post ); + $this->assertSame( ( $seconds * 1_000_000 ) + 500_042, $comment ); + } + + /** + * Regression test for the cross-kind microsecond collision codex + * surfaced during PR 92 review. Post id 1288 and Comment id 350044 + * landed on identical microseconds under the old modulo+CRC32 scheme + * because `(350044 + crc32('comment')) % 1_000_000 == 1288`, and the + * salt-derived clock_id happened to collide on the same 1/1024 + * bucket — minting identical rkeys inside `app.bsky.feed.post` that + * `applyWrites` would reject on the second create. + * + * The disjoint-range scheme eliminates the failure mode by + * construction; this test locks the specific pair so we cannot + * silently regress. + */ + public function test_microseconds_disambiguate_codex_collision_pair(): void { + $gmt = '2020-01-01 00:00:00'; + $post_micros = TID::microseconds_from_post_date( $gmt, 1288, '' ); + $comment_micros = TID::microseconds_from_post_date( $gmt, 350044, 'comment' ); + + $this->assertNotSame( + $post_micros, + $comment_micros, + 'Post id 1288 and Comment id 350044 must not produce identical microseconds in the same second.' + ); } }