From b48ed73b4e50f6e7d291f8567ae0b4ce1c8d3908 Mon Sep 17 00:00:00 2001 From: Matt Edholm Date: Fri, 8 May 2026 12:41:40 -0400 Subject: [PATCH] fix(rotation): isDue() compares wakeTime boundary in UTC, not device-local tz MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Symptom: wakeTimes schedules silently never fire on non-UTC devices. Reported live by Matt's EDT frame: wakeTimes=[12:30 PM NY] saved, 12:30 came and went, no rotation. Same bug pattern would fire *every* poll on east-of-UTC tzs. Root cause: device_image_history.served_at is `timestamp without time zone`, written by `new DateTimeImmutable()` so it stores UTC components ("2026-05-08 16:28:50"). The boundary in isDue() was bound through Doctrine with the device's local tz still attached, so Doctrine's format() emitted local-tz components ("12:30:00"). Postgres compared the strings literally — for west-of-UTC tzs the UTC timestamp is numerically larger than the local-tz boundary, so every same-day row falsely satisfied `servedAt >= :wakeTime` and isDue returned false. Fix: $boundary->setTimezone(UTC) before binding. Both sides now format in UTC components, so Postgres's literal compare is correct. Regression test ID-TZ-01: device in America/New_York, wakeTimes [12:30 PM NY], history at 12:00 PM NY (= 16:00 UTC). With the fix isDue returns true; without it the test falsely-matches and fails. Skipped before 13:00 NY since the assertion needs the wake slot to have already passed today. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Service/RotationService.php | 13 +++++- tests/Unit/Service/RotationServiceTest.php | 47 ++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/src/Service/RotationService.php b/src/Service/RotationService.php index 8b35841..ba463ab 100644 --- a/src/Service/RotationService.php +++ b/src/Service/RotationService.php @@ -53,13 +53,24 @@ class RotationService return false; } + // device_image_history.served_at is `timestamp without time zone` + // and entries are written by `new DateTimeImmutable()` (UTC). The + // boundary above carries the device's local tz, which Doctrine + // would format with local-tz components — leading to a string + // comparison against UTC-formatted rows in PostgreSQL. For + // anything other than UTC that mismatch quietly breaks `isDue`: + // west-of-UTC tzs falsely match every same-day row (rotation + // never fires); east-of-UTC tzs falsely miss every row (rotation + // fires every poll). Bind in UTC so both sides agree. + $boundaryUtc = $boundary->setTimezone(new \DateTimeZone('UTC')); + $entry = $this->em->createQueryBuilder() ->select('h') ->from(DeviceImageHistory::class, 'h') ->where('h.device = :device') ->andWhere('h.servedAt >= :wakeTime') ->setParameter('device', $device) - ->setParameter('wakeTime', $boundary) + ->setParameter('wakeTime', $boundaryUtc) ->setMaxResults(1) ->getQuery() ->getOneOrNullResult(); diff --git a/tests/Unit/Service/RotationServiceTest.php b/tests/Unit/Service/RotationServiceTest.php index 9254f17..e6fb68e 100644 --- a/tests/Unit/Service/RotationServiceTest.php +++ b/tests/Unit/Service/RotationServiceTest.php @@ -322,6 +322,53 @@ class RotationServiceTest extends AppKernelTestCase self::em()->flush(); } + // ID-TZ-01: Regression — isDue() in a non-UTC device timezone. + // + // Bug: device_image_history.served_at is `timestamp without time zone` + // storing UTC components, while the boundary in isDue() was bound with + // the device's local-tz components. Postgres compared the strings + // literally, so for tzs west of UTC every same-day row falsely matched + // the `>= boundary` predicate and isDue returned false — wakeTimes + // schedules silently never fired in EDT/PST/CST/etc. + // + // Real-world repro (Matt's EDT frame, 2026-05-08): wakeTimes=[12:30 PM + // NY], history at 12:28:50 EDT = 16:28:50 UTC. Boundary serialized as + // "12:30:00" (NY components), history stored as "16:28:50" (UTC): + // "16:28:50" >= "12:30:00" lexically → false-match → bug. Fix: bind + // the boundary in UTC so both sides agree. + // + // Test windowing: needs NY local to be past 12:30 PM (so the wake slot + // qualifies as "today's most-recent past"). Skipped before 13:00 NY. + public function test_id_tz_01_isDue_correct_in_non_utc_timezone(): void + { + $nyHour = (int) (new \DateTimeImmutable('now', new \DateTimeZone('America/New_York')))->format('G'); + if ($nyHour < 13) { + $this->markTestSkipped('Test exercises a 12:30 PM NY-local boundary; skip until after 13:00 NY'); + } + + [$device, $images] = $this->setupDeviceAndImages('tz-edt@example.com', [ + '2024-01-01', + ]); + $device->setTimezone('America/New_York'); + $device->setWakeTimes([12 * 60 + 30]); // 12:30 PM NY-local + self::em()->flush(); + + // Record a history row at 12:00 PM NY today, stored as UTC components + // (16:00:00 in EDT, 17:00:00 in EST). The buggy comparison sees + // "16:00:00" >= "12:30:00" (boundary was NY-formatted) → falsely + // matches → isDue returns false. The fixed comparison binds the + // boundary as "16:30:00" UTC → "16:00:00" < "16:30:00" → no match → + // isDue returns true. + $historyUtc = (new \DateTimeImmutable('today 12:00:00', new \DateTimeZone('America/New_York'))) + ->setTimezone(new \DateTimeZone('UTC')); + $this->recordHistoryAt($device, $images[0], $historyUtc->format('Y-m-d H:i:s')); + + $this->assertTrue( + $this->rotation->isDue($device), + 'isDue must use a tz-consistent comparison; the regression is invisible on UTC-tz devices but breaks every other tz', + ); + } + // RM-01: NewestUpload picks the most recent upload. public function test_newest_upload_mode_picks_newest(): void {