From 6be5420fcf03c38ed0e4dfa7ee6dd23045f46783 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Sun, 30 Aug 2026 16:37:13 +0200 Subject: [PATCH] Stop trusting an alarm the phone published months ago Belt-and-braces for the publishing race fixed in the previous commit. That bug is gone, but the shape of the failure is worth closing on this side too: a publish that never happens is indistinguishable from one that has not changed, so the last successful publish stayed authoritative forever. One record stamped 2026-08-09 had the PC starting the morning routine at 07:00 for three weeks. get_alarm now skips any record the phone published longer ago than PUBLISHED_ALARM_MAX_AGE_SECONDS (14 days) and falls back. That is safe precisely because FALLBACK_ALARM_HOUR is the phone's own baseline, so a lapsed record degrades to the right answer rather than to a guess -- and an override is one-shot anyway, so reverting to the baseline is what the phone would say. The check is on when the PHONE stamped it, never on when we fetched, and that distinction is the whole design. Checking the fetch would be trivially defeated: the cache is rewritten from the same stale record every few minutes, so `fetched_at` is always fresh no matter how old the record behind it is. So the winning HLC's instant is now carried out of _alarm_payload, through _sync_transport, and persisted in the cache as `published_at`. Nothing else reads that cache any more -- the shutdown wrapper that used to went with the PC's waking role -- so widening its shape is free. An undated record is trusted rather than expired. The GitHub mirror reads a single fixed path with no per-field HLC, so treating "cannot tell how old this is" as "too old" would break that whole backend, and refusing an alarm we merely failed to date is a worse failure than honouring it. test_sync.py and test_sync_backends.py crossed the 250-line cap, so the new lapse cases and the backend-ordering cases move to test_sync_lapsed.py and test_sync_dispatch.py. 168 tests at 100% branch coverage. Verified against the real backend: the cache now carries published_at, and `_sync --show` still resolves 08:00 from the record the phone stamped minutes earlier. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PcdngP688mW6rJye3LRkSN --- wake_alarm/_alarm_payload.py | 37 +++++- wake_alarm/_constants.py | 14 +++ wake_alarm/_sync.py | 66 ++++++++-- wake_alarm/_sync_transport.py | 24 ++-- wake_alarm/tests/test_alarm_payload.py | 28 ++++- wake_alarm/tests/test_sync.py | 33 +++-- wake_alarm/tests/test_sync_backends.py | 59 +++------ wake_alarm/tests/test_sync_dispatch.py | 52 ++++++++ wake_alarm/tests/test_sync_lapsed.py | 159 +++++++++++++++++++++++++ 9 files changed, 394 insertions(+), 78 deletions(-) create mode 100644 wake_alarm/tests/test_sync_dispatch.py create mode 100644 wake_alarm/tests/test_sync_lapsed.py diff --git a/wake_alarm/_alarm_payload.py b/wake_alarm/_alarm_payload.py index a6db82a..b02f5ef 100644 --- a/wake_alarm/_alarm_payload.py +++ b/wake_alarm/_alarm_payload.py @@ -16,6 +16,7 @@ rather than the value, so the alarm that wins is the one *set* most recently. from __future__ import annotations +from datetime import UTC, datetime import json import logging from typing import Any @@ -140,7 +141,34 @@ def parse_log(raw: str) -> tuple[int, int] | None: return hour, minute -def newest_alarm(published: list[tuple[str, str]]) -> tuple[int, int] | None: +def hlc_instant(hlc: str) -> float | None: + """Wall-clock instant an HLC stamp was minted, as epoch seconds. + + HLC strings are ``--``, and the ISO part ends at + its ``Z`` -- which is what makes this splittable at all, since the ISO + date itself contains the same ``-`` the fields are joined with. + + Args: + hlc: The stamp, or ``""`` when the field carried none. + + Returns: + Epoch seconds, or None when the stamp is absent or unparsable. None + means "cannot tell how old this is", and callers treat that as + trustworthy rather than expired -- refusing to act on an alarm we + merely failed to date would be a worse failure than acting on it. + """ + iso, separator, _ = hlc.partition("Z") + if not separator: + return None + try: + return datetime.fromisoformat(iso).replace(tzinfo=UTC).timestamp() + except ValueError: + return None + + +def newest_alarm( + published: list[tuple[str, str]], +) -> tuple[int, int, float | None] | None: """Pick the most recently published alarm out of every device's log. Ranked by the hour field's HLC stamp, which orders by wall time and then @@ -154,8 +182,9 @@ def newest_alarm(published: list[tuple[str, str]]) -> tuple[int, int] | None: published, in any order. Returns: - The newest (hour, minute) pair, or None if no device published a - usable one. + The newest ``(hour, minute, published_at)``, where ``published_at`` is + epoch seconds from the winning HLC (or None if it carried no readable + stamp). None if no device published a usable alarm. """ ranked: list[tuple[str, str, tuple[int, int]]] = [] for device_id, raw in published: @@ -170,4 +199,4 @@ def newest_alarm(published: list[tuple[str, str]]) -> tuple[int, int] | None: # stamped in the same millisecond don't alternate between ticks. hlc, winner, alarm = max(ranked, key=lambda entry: (entry[0], entry[1])) _logger.info("Using the alarm published by %r (hlc %s)", winner, hlc or "none") - return alarm + return (*alarm, hlc_instant(hlc)) diff --git a/wake_alarm/_constants.py b/wake_alarm/_constants.py index e94802b..7274505 100644 --- a/wake_alarm/_constants.py +++ b/wake_alarm/_constants.py @@ -104,6 +104,20 @@ SYNC_HTTP_TIMEOUT_SECONDS: float = 10.0 # a minute (so firing lands on the exact minute), but only touches the network # when the cache is older than this — bounding phone->PC latency at ~5 minutes # while keeping GitHub reads to ~288/day. +# How old the phone's *published* record may be before the PC stops trusting +# it. Distinct from ALARM_CACHE_MAX_AGE_SECONDS, which is about how recently +# *we fetched*: a value fetched five seconds ago can still be a time the phone +# chose months back and has since moved on from. +# +# This exists because that is exactly what happened. A publishing bug on the +# phone left one record stamped 2026-08-09, and the PC honoured it faithfully +# for three weeks -- a publish that never happens is indistinguishable from one +# that has not changed, so the last successful publish is authoritative +# forever. Expiring to FALLBACK_ALARM_HOUR is safe precisely because that +# constant is the phone's own baseline, so a lapsed record degrades to the +# right answer rather than a guess. +PUBLISHED_ALARM_MAX_AGE_SECONDS: float = 14 * 24 * 60 * 60 + ALARM_CACHE_MAX_AGE_SECONDS: float = 300.0 # How long after the alarm instant the trigger will still fire. Covers a diff --git a/wake_alarm/_sync.py b/wake_alarm/_sync.py index aa6b083..dd001a8 100644 --- a/wake_alarm/_sync.py +++ b/wake_alarm/_sync.py @@ -5,9 +5,9 @@ in a ``crdt_sync`` ``LogStore`` and pushes that log to a fixed path in the private ``kuhyx/syncs`` repo. This module is the PC-side reader used by the per-minute trigger (see ``_trigger.py``). -``shutdown-wrapper.sh`` no longer reads the remote file at all — it reads the -local cache this module writes, so there is only one remote reader to keep in -step (see ``CLAUDE.md``). +This module is the only reader of both the remote record and the local cache; +the shutdown wrapper that used to read the cache went with the PC's waking +role. **Backends.** Firebase is preferred and GitHub is the cutover fallback, the same order every other app in the fleet uses. Firebase needs ``crdt_sync`` @@ -34,6 +34,7 @@ from wake_alarm._constants import ( ALARM_CACHE_FILE, ALARM_CACHE_MAX_AGE_SECONDS, FALLBACK_ALARM_HOUR, + PUBLISHED_ALARM_MAX_AGE_SECONDS, ) from wake_alarm._logging_setup import configure_logging from wake_alarm._sync_transport import ( @@ -84,12 +85,13 @@ class Alarm: return f"{self.hour:02d}:{self.minute:02d} ({self.source})" -def _read_cache() -> tuple[int, int, float] | None: +def _read_cache() -> tuple[int, int, float, float | None] | None: """Read the cached alarm. Returns: - An (hour, minute, fetched_at) triple, or None if there is no usable - cache. + ``(hour, minute, fetched_at, published_at)``, or None if there is no + usable cache. ``published_at`` is None for a cache written before it + was recorded, or by the GitHub mirror. """ try: with ALARM_CACHE_FILE.open() as handle: @@ -101,16 +103,19 @@ def _read_cache() -> tuple[int, int, float] | None: hour = cached.get("hour") minute = cached.get("minute") fetched_at = cached.get("fetched_at") + published_at = cached.get("published_at") + if not isinstance(published_at, float | int): + published_at = None if not isinstance(hour, int) or not isinstance(minute, int): return None if not isinstance(fetched_at, (int, float)): return None if not in_range(hour, minute): return None - return hour, minute, float(fetched_at) + return hour, minute, float(fetched_at), published_at -def _write_cache(hour: int, minute: int) -> None: +def _write_cache(hour: int, minute: int, published_at: float | None) -> None: """Persist a freshly fetched alarm so later reads survive a network loss. Failures are logged and swallowed: an unwritable cache degrades the next @@ -119,8 +124,16 @@ def _write_cache(hour: int, minute: int) -> None: Args: hour: Hour of day, 0-23. minute: Minute of hour, 0-59. + published_at: When the phone stamped the record, epoch seconds, or + None when unknown. Cached alongside so a stale record cannot be + laundered into freshness by a recent *fetch*. """ - entry = {"hour": hour, "minute": minute, "fetched_at": time.time()} + entry = { + "hour": hour, + "minute": minute, + "fetched_at": time.time(), + "published_at": published_at, + } try: ALARM_CACHE_FILE.parent.mkdir(parents=True, exist_ok=True) with ALARM_CACHE_FILE.open("w") as handle: @@ -129,15 +142,38 @@ def _write_cache(hour: int, minute: int) -> None: _logger.warning("Cannot write alarm cache: %s", exc) +def _has_lapsed(published_at: float | None) -> bool: + """Whether a record is too old for the phone to still mean it. + + An unknown stamp is never treated as lapsed: refusing an alarm we merely + failed to date would be a worse failure than honouring it. + + Args: + published_at: When the phone stamped the record, or None if unknown. + + Returns: + True when the record is past ``PUBLISHED_ALARM_MAX_AGE_SECONDS``. + """ + if published_at is None: + return False + return time.time() - published_at > PUBLISHED_ALARM_MAX_AGE_SECONDS + + def get_alarm(*, max_cache_age_seconds: float = ALARM_CACHE_MAX_AGE_SECONDS) -> Alarm: """Resolve the alarm to fire today, degrading gracefully at every step. Resolution order: 1. A cache entry younger than *max_cache_age_seconds* (no network). - 2. A live fetch from GitHub (cached on success). + 2. A live fetch, Firebase first (cached on success). 3. Any cache entry, however stale — better an old alarm than none. - 4. ``FALLBACK_ALARM_HOUR:00``, if the alarm has never been read at all. + 4. ``FALLBACK_ALARM_HOUR:00``. + + A record the phone published longer ago than + ``PUBLISHED_ALARM_MAX_AGE_SECONDS`` is skipped at *every* step, including + the cache — the check is on when the phone said it, not on when we last + fetched, because a value re-fetched seconds ago can still be a time the + phone moved on from months back. Args: max_cache_age_seconds: How long a cached read is served without @@ -148,12 +184,18 @@ def get_alarm(*, max_cache_age_seconds: float = ALARM_CACHE_MAX_AGE_SECONDS) -> The resolved alarm. Always succeeds. """ cached = _read_cache() + if cached is not None and _has_lapsed(cached[3]): + _logger.info("Cached alarm lapsed — the phone has not published since") + cached = None if cached is not None: - hour, minute, fetched_at = cached + hour, minute, fetched_at, _ = cached if time.time() - fetched_at < max_cache_age_seconds: return Alarm(hour, minute, "cache") remote = fetch_remote_alarm() + if remote is not None and _has_lapsed(remote[2]): + _logger.info("Published alarm lapsed — the phone has not published since") + remote = None if remote is not None: _write_cache(*remote) return Alarm(remote[0], remote[1], "remote") diff --git a/wake_alarm/_sync_transport.py b/wake_alarm/_sync_transport.py index 095318a..f919e66 100644 --- a/wake_alarm/_sync_transport.py +++ b/wake_alarm/_sync_transport.py @@ -103,7 +103,7 @@ def _read_token() -> str | None: return _read_token_config() or _read_token_fallback() -def _fetch_firebase_alarm() -> tuple[int, int] | None: +def _fetch_firebase_alarm() -> tuple[int, int, float | None] | None: """Fetch the newest alarm any device has pushed to Firebase. Discovers device directories rather than reading one fixed path: the @@ -118,7 +118,8 @@ def _fetch_firebase_alarm() -> tuple[int, int] | None: fall back to GitHub and then to the cache. Returns: - The (hour, minute) pair, or None if unconfigured or unreadable. + ``(hour, minute, published_at)``, or None if unconfigured or + unreadable. """ if not _FIREBASE_AVAILABLE: return None @@ -153,14 +154,15 @@ def _fetch_firebase_alarm() -> tuple[int, int] | None: return newest_alarm(published) -def _fetch_github_alarm() -> tuple[int, int] | None: +def _fetch_github_alarm() -> tuple[int, int, float | None] | None: """Fetch the alarm the phone last pushed to GitHub. Uses the raw media type so the Contents API returns the file body directly rather than base64-wrapped JSON. Returns: - The (hour, minute) pair, or None if unconfigured, unreachable, or + ``(hour, minute, published_at)`` -- age unknown on this path -- or + None if unconfigured, unreachable, or malformed. Never raises — a failed read must degrade to the cache, not crash the trigger. """ @@ -202,10 +204,16 @@ def _fetch_github_alarm() -> tuple[int, int] | None: _logger.info("Cannot fetch synced alarm: %s", exc) return None - return parse_log(raw) + parsed = parse_log(raw) + if parsed is None: + return None + # The mirror is a single fixed path with no per-field HLC to read, so its + # age is unknown rather than fresh. See hlc_instant on why that is treated + # as trustworthy. + return (*parsed, None) -def fetch_remote_alarm() -> tuple[int, int] | None: +def fetch_remote_alarm() -> tuple[int, int, float | None] | None: """Fetch the alarm the phone last pushed, Firebase first. GitHub is tried only when Firebase yields nothing, which covers both the @@ -213,6 +221,8 @@ def fetch_remote_alarm() -> tuple[int, int] | None: moved, deleting the GitHub branch is the whole retirement. Returns: - The (hour, minute) pair, or None if no backend has a usable answer. + ``(hour, minute, published_at)``, or None if no backend has a + usable answer. ``published_at`` is epoch seconds, or None when the + record carries no readable stamp. """ return _fetch_firebase_alarm() or _fetch_github_alarm() diff --git a/wake_alarm/tests/test_alarm_payload.py b/wake_alarm/tests/test_alarm_payload.py index 8a95356..b1392c5 100644 --- a/wake_alarm/tests/test_alarm_payload.py +++ b/wake_alarm/tests/test_alarm_payload.py @@ -6,6 +6,7 @@ different module from the fetching/caching this file's siblings cover. from __future__ import annotations +from datetime import UTC, datetime import json from typing import TYPE_CHECKING, Any from unittest.mock import patch @@ -15,6 +16,7 @@ import pytest from wake_alarm._alarm_payload import ( extract_field, extract_hlc, + hlc_instant, newest_alarm, parse_log, ) @@ -176,7 +178,9 @@ class TestNewestAlarm: } }, ) - assert newest_alarm([("old", older), ("new", newer)]) == (7, 30) + winner = newest_alarm([("old", older), ("new", newer)]) + assert winner is not None + assert winner[:2] == (7, 30) def test_an_unusable_device_does_not_block_a_good_one(self) -> None: """One broken device must not cost the user their alarm.""" @@ -190,4 +194,24 @@ class TestNewestAlarm: } }, ) - assert newest_alarm([("broken", "{}"), ("good", good)]) == (5, 15) + winner = newest_alarm([("broken", "{}"), ("good", good)]) + assert winner is not None + assert winner[:2] == (5, 15) + + +class TestHlcInstant: + """Dating a record, so a lapsed one can be spotted.""" + + def test_reads_the_iso_prefix(self) -> None: + """The ISO part ends at its Z, which is what makes it splittable.""" + stamp = hlc_instant("2026-08-09T16:28:05.575Z-0000-device") + assert stamp is not None + assert datetime.fromtimestamp(stamp, UTC).day == 9 + + def test_an_absent_stamp_is_undated(self) -> None: + """A field with no HLC cannot be dated, and says so.""" + assert hlc_instant("") is None + + def test_an_unparsable_prefix_is_undated(self) -> None: + """Garbage before the Z must not raise out of a per-minute tick.""" + assert hlc_instant("not-a-timestampZ-0000-device") is None diff --git a/wake_alarm/tests/test_sync.py b/wake_alarm/tests/test_sync.py index 8aeb4bb..e27244d 100644 --- a/wake_alarm/tests/test_sync.py +++ b/wake_alarm/tests/test_sync.py @@ -9,7 +9,9 @@ import urllib.error import pytest -from wake_alarm._constants import FALLBACK_ALARM_HOUR +from wake_alarm._constants import ( + FALLBACK_ALARM_HOUR, +) from wake_alarm._sync import ( Alarm, _fetch_github_alarm, @@ -92,7 +94,18 @@ class TestFetchRemoteAlarm: response.read.return_value = _log(6, 15).encode() response.__enter__.return_value = response with patch("urllib.request.urlopen", return_value=response): - assert _fetch_github_alarm() == (6, 15) + fetched = _fetch_github_alarm() + assert fetched is not None + assert fetched[:2] == (6, 15) + + def test_unparseable_body_is_not_an_alarm(self, config_file: Path) -> None: + """A 200 whose body is not a usable log yields nothing, not a crash.""" + self._with_token(config_file) + response = MagicMock() + response.read.return_value = b"{}" + response.__enter__.return_value = response + with patch("urllib.request.urlopen", return_value=response): + assert _fetch_github_alarm() is None def test_sends_raw_media_type_and_auth(self, config_file: Path) -> None: """The request carries the token and the raw Accept header.""" @@ -139,7 +152,7 @@ class TestCache: def test_roundtrip(self, cache_file: Path) -> None: """A written cache reads back with the same values.""" - _write_cache(6, 45) + _write_cache(6, 45, None) cached = _read_cache() assert cached is not None assert cached[0] == 6 @@ -147,13 +160,13 @@ class TestCache: def test_write_creates_parent_dir(self, cache_file: Path) -> None: """The cache directory is created on demand.""" - _write_cache(6, 45) + _write_cache(6, 45, None) assert cache_file.exists() def test_write_failure_is_swallowed(self) -> None: """An unwritable cache logs but does not raise.""" with patch("pathlib.Path.mkdir", side_effect=OSError("read-only")): - _write_cache(6, 45) + _write_cache(6, 45, None) def test_missing_file(self) -> None: """No cache file yields None.""" @@ -183,7 +196,7 @@ class TestGetAlarm: def test_fresh_cache_skips_network(self, cache_file: Path) -> None: """A young cache entry is served without any fetch.""" - _write_cache(6, 45) + _write_cache(6, 45, None) with patch("wake_alarm._sync.fetch_remote_alarm") as fetch: alarm = get_alarm() fetch.assert_not_called() @@ -191,20 +204,20 @@ class TestGetAlarm: def test_stale_cache_triggers_fetch(self, cache_file: Path) -> None: """An old cache entry is refreshed from the network.""" - _write_cache(6, 45) - with patch("wake_alarm._sync.fetch_remote_alarm", return_value=(7, 0)): + _write_cache(6, 45, None) + with patch("wake_alarm._sync.fetch_remote_alarm", return_value=(7, 0, None)): alarm = get_alarm(max_cache_age_seconds=0) assert (alarm.hour, alarm.minute, alarm.source) == (7, 0, "remote") def test_fetch_result_is_cached(self, cache_file: Path) -> None: """A successful fetch is written to the cache for next time.""" - with patch("wake_alarm._sync.fetch_remote_alarm", return_value=(7, 0)): + with patch("wake_alarm._sync.fetch_remote_alarm", return_value=(7, 0, None)): get_alarm() assert json.loads(cache_file.read_text())["hour"] == 7 def test_falls_back_to_stale_cache(self, cache_file: Path) -> None: """When the fetch fails, an old cache entry is still used.""" - _write_cache(6, 45) + _write_cache(6, 45, None) with patch("wake_alarm._sync.fetch_remote_alarm", return_value=None): alarm = get_alarm(max_cache_age_seconds=0) assert (alarm.hour, alarm.minute, alarm.source) == ( diff --git a/wake_alarm/tests/test_sync_backends.py b/wake_alarm/tests/test_sync_backends.py index 9eb3b41..3f2a643 100644 --- a/wake_alarm/tests/test_sync_backends.py +++ b/wake_alarm/tests/test_sync_backends.py @@ -17,7 +17,7 @@ from wake_alarm._constants import ( SYNC_ALARM_PATH, SYNC_DEVICES_PREFIX, ) -from wake_alarm._sync import _fetch_firebase_alarm, fetch_remote_alarm +from wake_alarm._sync import _fetch_firebase_alarm if TYPE_CHECKING: from contextlib import AbstractContextManager @@ -93,7 +93,9 @@ class TestFetchFirebaseAlarm: client.list_directory.return_value = [] client.get_file_text.return_value = _log(6, 15) with self._patched(client=client): - assert _fetch_firebase_alarm() == (6, 15) + fetched = _fetch_firebase_alarm() + assert fetched is not None + assert fetched[:2] == (6, 15) client.get_file_text.assert_called_once_with(SYNC_ALARM_PATH) def test_absent_value_yields_none(self) -> None: @@ -110,7 +112,9 @@ class TestFetchFirebaseAlarm: client.list_directory.return_value = ["7f3c-uuid"] client.get_file_text.return_value = _log(6, 45) with self._patched(client=client): - assert _fetch_firebase_alarm() == (6, 45) + fetched = _fetch_firebase_alarm() + assert fetched is not None + assert fetched[:2] == (6, 45) client.get_file_text.assert_called_once_with( f"{SYNC_DEVICES_PREFIX}/7f3c-uuid/{SYNC_ALARM_FILENAME}" ) @@ -134,7 +138,9 @@ class TestFetchFirebaseAlarm: ), }[path] with self._patched(client=client): - assert _fetch_firebase_alarm() == (5, 30) + fetched = _fetch_firebase_alarm() + assert fetched is not None + assert fetched[:2] == (5, 30) def test_falls_back_to_the_legacy_path_when_nothing_lists(self) -> None: """A backend that lists nothing still reads the un-migrated phone.""" @@ -142,7 +148,9 @@ class TestFetchFirebaseAlarm: client.list_directory.return_value = ["phone"] client.get_file_text.side_effect = [None, _log(8, 5)] with self._patched(client=client): - assert _fetch_firebase_alarm() == (8, 5) + fetched = _fetch_firebase_alarm() + assert fetched is not None + assert fetched[:2] == (8, 5) assert client.get_file_text.call_args_list[-1].args[0] == SYNC_ALARM_PATH def test_a_device_publishing_garbage_does_not_win(self) -> None: @@ -154,7 +162,9 @@ class TestFetchFirebaseAlarm: f"{SYNC_DEVICES_PREFIX}/good/{SYNC_ALARM_FILENAME}": _log(7, 20), }[path] with self._patched(client=client): - assert _fetch_firebase_alarm() == (7, 20) + fetched = _fetch_firebase_alarm() + assert fetched is not None + assert fetched[:2] == (7, 20) @pytest.mark.parametrize( "exc", @@ -201,40 +211,3 @@ class TestOptionalCrdtSyncImport: # the import-failed state would silently disable Firebase for the # rest of the session. importlib.reload(_sync_transport) - - -class TestFetchRemoteAlarmDispatch: - """Tests for the Firebase-first, GitHub-fallback ordering.""" - - def test_prefers_firebase(self) -> None: - """When Firebase answers, GitHub is never asked.""" - with ( - patch( - "wake_alarm._sync_transport._fetch_firebase_alarm", return_value=(6, 15) - ), - patch("wake_alarm._sync_transport._fetch_github_alarm") as github, - ): - assert fetch_remote_alarm() == (6, 15) - github.assert_not_called() - - def test_falls_back_to_github(self) -> None: - """A Firebase miss still resolves through the mirror.""" - with ( - patch( - "wake_alarm._sync_transport._fetch_firebase_alarm", return_value=None - ), - patch( - "wake_alarm._sync_transport._fetch_github_alarm", return_value=(7, 30) - ), - ): - assert fetch_remote_alarm() == (7, 30) - - def test_both_missing_yields_none(self) -> None: - """With neither backend usable the caller degrades to the cache.""" - with ( - patch( - "wake_alarm._sync_transport._fetch_firebase_alarm", return_value=None - ), - patch("wake_alarm._sync_transport._fetch_github_alarm", return_value=None), - ): - assert fetch_remote_alarm() is None diff --git a/wake_alarm/tests/test_sync_dispatch.py b/wake_alarm/tests/test_sync_dispatch.py new file mode 100644 index 0000000..cd848e0 --- /dev/null +++ b/wake_alarm/tests/test_sync_dispatch.py @@ -0,0 +1,52 @@ +"""Tests for fetch_remote_alarm's backend ordering. + +Split from test_sync_backends.py for the 250-line cap. +""" + +from __future__ import annotations + +from unittest.mock import patch + +from wake_alarm._sync import fetch_remote_alarm + + +class TestFetchRemoteAlarmDispatch: + """Tests for the Firebase-first, GitHub-fallback ordering.""" + + def test_prefers_firebase(self) -> None: + """When Firebase answers, GitHub is never asked.""" + with ( + patch( + "wake_alarm._sync_transport._fetch_firebase_alarm", return_value=(6, 15) + ), + patch("wake_alarm._sync_transport._fetch_github_alarm") as github, + ): + resolved = fetch_remote_alarm() + assert resolved is not None + assert resolved[:2] == (6, 15) + github.assert_not_called() + + def test_falls_back_to_github(self) -> None: + """A Firebase miss still resolves through the mirror.""" + with ( + patch( + "wake_alarm._sync_transport._fetch_firebase_alarm", return_value=None + ), + patch( + "wake_alarm._sync_transport._fetch_github_alarm", + return_value=(7, 30, None), + ), + ): + resolved = fetch_remote_alarm() + assert resolved is not None + assert resolved[:2] == (7, 30) + + def test_both_missing_yields_none(self) -> None: + """With neither backend usable the caller degrades to the cache.""" + with ( + patch( + "wake_alarm._sync_transport._fetch_firebase_alarm", return_value=None + ), + patch("wake_alarm._sync_transport._fetch_github_alarm", return_value=None), + ): + assert fetch_remote_alarm() is None diff --git a/wake_alarm/tests/test_sync_lapsed.py b/wake_alarm/tests/test_sync_lapsed.py new file mode 100644 index 0000000..48a96d7 --- /dev/null +++ b/wake_alarm/tests/test_sync_lapsed.py @@ -0,0 +1,159 @@ +"""Tests for _sync.py — reading the alarm the phone pushed, with fallbacks.""" + +from __future__ import annotations + +import json +import time +from typing import TYPE_CHECKING +from unittest.mock import patch + +import pytest + +from wake_alarm._constants import ( + FALLBACK_ALARM_HOUR, + PUBLISHED_ALARM_MAX_AGE_SECONDS, +) +from wake_alarm._sync import ( + get_alarm, +) + +if TYPE_CHECKING: + from pathlib import Path + + +def _log(hour: object = 7, minute: object = 30) -> str: + """Build a crdt_sync log payload shaped like the phone's.""" + return json.dumps( + { + "alarm": { + "fields": { + "alarm_hour": [hour, "hlc-1"], + "alarm_minute": [minute, "hlc-1"], + } + } + } + ) + + +@pytest.fixture +def config_file(tmp_path: Path) -> Path: + """Provide a temporary sync config path.""" + return tmp_path / "sync.json" + + +@pytest.fixture +def cache_file(tmp_path: Path) -> Path: + """Provide a temporary alarm cache path.""" + return tmp_path / "cache" / "alarm_cache.json" + + +@pytest.fixture +def fallback_files(tmp_path: Path) -> tuple[Path, Path]: + """Provide two temporary sibling-app token paths.""" + return tmp_path / "diet_guard_token", tmp_path / "screen_locker_token" + + +@pytest.fixture(autouse=True) +def _patch_paths( + config_file: Path, cache_file: Path, fallback_files: tuple[Path, Path] +) -> None: + """Redirect every on-disk path into tmp_path for every test.""" + with ( + patch("wake_alarm._sync_transport.SYNC_CONFIG_FILE", config_file), + patch("wake_alarm._sync.ALARM_CACHE_FILE", cache_file), + patch("wake_alarm._sync_transport.SYNC_TOKEN_FALLBACK_FILES", fallback_files), + ): + yield + + +class TestLapsedPublishedAlarm: + """The phone must keep saying it, not just have said it once. + + A publishing bug on the phone left one record stamped 2026-08-09 and the + PC honoured it for three weeks: a publish that never happens looks exactly + like one that has not changed, so the last successful publish is + authoritative forever. These pin the guard that ends that. + """ + + def test_a_lapsed_remote_is_not_used(self, tmp_path: Path) -> None: + """A record older than the max age falls through to the fallback.""" + ancient = time.time() - PUBLISHED_ALARM_MAX_AGE_SECONDS - 1 + with ( + patch("wake_alarm._sync.ALARM_CACHE_FILE", tmp_path / "cache.json"), + patch( + "wake_alarm._sync.fetch_remote_alarm", + return_value=(7, 0, ancient), + ), + ): + alarm = get_alarm() + assert alarm.source == "fallback" + assert alarm.hour == FALLBACK_ALARM_HOUR + + def test_a_recent_remote_is_used(self, tmp_path: Path) -> None: + """A freshly stamped record is honoured as normal.""" + with ( + patch("wake_alarm._sync.ALARM_CACHE_FILE", tmp_path / "cache.json"), + patch( + "wake_alarm._sync.fetch_remote_alarm", + return_value=(7, 0, time.time()), + ), + ): + alarm = get_alarm() + assert (alarm.hour, alarm.minute, alarm.source) == (7, 0, "remote") + + def test_an_undated_remote_is_trusted(self, tmp_path: Path) -> None: + """No stamp means "cannot tell", which must not mean "expired". + + The GitHub mirror reads a single fixed path with no per-field HLC, so + rejecting undated records would break that whole backend. + """ + with ( + patch("wake_alarm._sync.ALARM_CACHE_FILE", tmp_path / "cache.json"), + patch( + "wake_alarm._sync.fetch_remote_alarm", + return_value=(7, 0, None), + ), + ): + alarm = get_alarm() + assert alarm.source == "remote" + + def test_a_lapsed_cache_is_not_served(self, tmp_path: Path) -> None: + """The check is on when the PHONE said it, not when we fetched. + + Without this the guard is trivially defeated: the cache is rewritten + from the same stale record every few minutes, so `fetched_at` is + always fresh no matter how old the record behind it is. + """ + cache = tmp_path / "cache.json" + cache.write_text( + json.dumps( + { + "hour": 7, + "minute": 0, + "fetched_at": time.time(), + "published_at": time.time() - PUBLISHED_ALARM_MAX_AGE_SECONDS - 1, + } + ) + ) + with ( + patch("wake_alarm._sync.ALARM_CACHE_FILE", cache), + patch("wake_alarm._sync.fetch_remote_alarm", return_value=None), + ): + alarm = get_alarm() + assert alarm.source == "fallback" + + def test_the_publish_instant_round_trips_through_the_cache( + self, tmp_path: Path + ) -> None: + """A fetched record's age survives being cached.""" + cache = tmp_path / "cache.json" + stamped = time.time() - 60 + with ( + patch("wake_alarm._sync.ALARM_CACHE_FILE", cache), + patch( + "wake_alarm._sync.fetch_remote_alarm", + return_value=(7, 0, stamped), + ), + ): + get_alarm() + assert json.loads(cache.read_text())["published_at"] == stamped