fix: retry a refused release check within the hour, so a rate-limited startup no longer hides updates for a day

app/main.py

@@ -2072,13 +2072,15 @@ def _on_job_event(event):
         coalesce=True,
         misfire_grace_time=300,
     )
-    # Once a day. A release is not urgent, and a self-hosted app polling a third
-    # party more often than that is rude. Every failure mode inside refresh()
-    # lands on UNKNOWN, so a firewalled install produces no noise at all.
+    # Frequent tick, daily fetch: refresh() asks GitHub once a day after a check
+    # that worked and an hour after one that failed, so a rate-limited startup
+    # check no longer leaves the release unknown for a whole day. A tick that
+    # is not due fetches nothing. Every failure mode inside refresh() lands on
+    # UNKNOWN, so a firewalled install produces no noise at all.
     scheduler.add_job(
         update_check.refresh,
         "interval",
-        hours=24,
+        seconds=update_check.SCHEDULER_TICK_SECONDS,
         id="update_check",
         max_instances=1,
         coalesce=True,
@@ -2087,13 +2089,13 @@ def _on_job_event(event):
         # convention a test enforces uniformly.
         misfire_grace_time=300,
     )
-    # The Android client's own release track, on the same daily cadence and with
+    # The Android client's own release track, on the same cadence and with
     # the same failure contract. Separate job rather than folded into the one
     # above so a GitHub hiccup fetching one cannot suppress the other.
     scheduler.add_job(
         update_check.refresh_android,
         "interval",
-        hours=24,
+        seconds=update_check.SCHEDULER_TICK_SECONDS,
         id="update_check_android",
         max_instances=1,
         coalesce=True,

app/update_check.py

@@ -55,18 +55,44 @@
 #: party more often than that is rude.
 CHECK_INTERVAL_SECONDS = 24 * 60 * 60
 
+#: After a FAILED check, the next try comes an hour later instead of a day. A
+#: home connection shares GitHub's unauthenticated limit (60 requests an hour
+#: per public IP) with everything else in the house, so the one check made at
+#: startup is often refused with a 403; waiting a full day after that left the
+#: release unknown, and every phone unjudged, for 24 hours after each restart.
+#: A reply with no usable release tag counts as a failure too, and keeps the
+#: tag already known. The scheduler calls refresh() every
+#: SCHEDULER_TICK_SECONDS; refresh() decides whether a fetch is due, so a
+#: working install still asks GitHub once a day.
+RETRY_AFTER_FAILURE_SECONDS = 60 * 60
+
+#: How often the scheduler calls refresh(). Well under the retry window, so a
+#: tick that lands a few seconds before a cooldown ends does not push the retry
+#: back by a whole extra window. A tick that is not due fetches nothing.
+SCHEDULER_TICK_SECONDS = 15 * 60
+
 _HTTP_TIMEOUT = 10.0
 
 #: Cached result. `tag` is None for UNKNOWN -- never for "up to date", which is a
 #: different statement and is carried by `known`.
 _latest_tag: str | None = None
 _checked_at: float = 0.0
+_last_failed: bool = False
 
 #: Same shape, for the Android client. Kept separate rather than folded into a
 #: dict so the existing server path cannot be perturbed by a failure fetching
 #: the Android one.
 _android_tag: str | None = None
 _android_checked_at: float = 0.0
+_android_last_failed: bool = False
+
+
+def _due(checked_at: float, last_failed: bool) -> bool:
+    """Whether the cooldown since the last attempt has run out."""
+    if not checked_at:
+        return True
+    wait = RETRY_AFTER_FAILURE_SECONDS if last_failed else CHECK_INTERVAL_SECONDS
+    return (time.time() - checked_at) >= wait
 
 
 def enabled() -> bool:
@@ -99,30 +125,33 @@ async def refresh(*, force: bool = False) -> str | None:
     that is not JSON, a tag that is not a version -- lands on the same answer:
     UNKNOWN. That is what makes the banner safe on an air-gapped install.
     """
-    global _latest_tag, _checked_at
+    global _latest_tag, _checked_at, _last_failed
 
     if not enabled():
         return None
-    if not force and _checked_at and (time.time() - _checked_at) < CHECK_INTERVAL_SECONDS:
+    if not force and not _due(_checked_at, _last_failed):
         return _latest_tag
 
     try:
         async with httpx.AsyncClient(timeout=_HTTP_TIMEOUT) as client:
             response = await client.get(LATEST_URL, headers={"Accept": "application/vnd.github+json"})
             response.raise_for_status()
             tag = _parse_tag(response.json())
+        if tag is None:
+            raise ValueError("no release tag in the reply")
     except Exception as exc:  # noqa: BLE001 - every failure means the same thing
         # DEBUG, not WARNING. A firewalled install would otherwise log a warning
         # every day forever about a feature its operator never asked for, and a
         # log full of harmless warnings is a log nobody reads.
         logger.debug("Update check could not reach %s: %s", LATEST_URL, exc)
         _checked_at = time.time()  # do not hammer a host that is refusing us
+        _last_failed = True
         return _latest_tag
 
     _latest_tag = tag
     _checked_at = time.time()
-    if tag:
-        logger.debug("Newest published release is %s", tag)
+    _last_failed = False
+    logger.debug("Newest published release is %s", tag)
     return tag
 
 
@@ -134,25 +163,29 @@ async def refresh_android(*, force: bool = False) -> str | None:
     value feeds a WARNING on the fleet page -- an unreachable GitHub must make
     the warning disappear, never make every phone look out of date.
     """
-    global _android_tag, _android_checked_at
+    global _android_tag, _android_checked_at, _android_last_failed
 
     if not enabled():
         return None
-    if not force and _android_checked_at and (time.time() - _android_checked_at) < CHECK_INTERVAL_SECONDS:
+    if not force and not _due(_android_checked_at, _android_last_failed):
         return _android_tag
 
     try:
         async with httpx.AsyncClient(timeout=_HTTP_TIMEOUT) as client:
             response = await client.get(ANDROID_LATEST_URL, headers={"Accept": "application/vnd.github+json"})
             response.raise_for_status()
             tag = _parse_tag(response.json())
+        if tag is None:
+            raise ValueError("no release tag in the reply")
     except Exception as exc:  # noqa: BLE001 - every failure means the same thing
         logger.debug("Android update check could not reach %s: %s", ANDROID_LATEST_URL, exc)
         _android_checked_at = time.time()
+        _android_last_failed = True
         return _android_tag
 
     _android_tag = tag
     _android_checked_at = time.time()
+    _android_last_failed = False
     return _android_tag
 
 
@@ -202,6 +235,6 @@ def _tuple(series_text: str) -> tuple[int, ...]:
 
 def _reset_for_tests() -> None:
     """Clear the module cache. Tests only."""
-    global _latest_tag, _checked_at, _android_tag, _android_checked_at
-    _latest_tag, _checked_at = None, 0.0
-    _android_tag, _android_checked_at = None, 0.0
+    global _latest_tag, _checked_at, _last_failed, _android_tag, _android_checked_at, _android_last_failed
+    _latest_tag, _checked_at, _last_failed = None, 0.0, False
+    _android_tag, _android_checked_at, _android_last_failed = None, 0.0, False

tests/test_main_routes.py

@@ -2269,6 +2269,13 @@ async def _run():
                         assert job.max_instances == 1, f"{job.id} max_instances"
                         assert job.coalesce is True, f"{job.id} coalesce"
                         assert job.misfire_grace_time == 300, f"{job.id} misfire_grace_time"
+                    # The release checks tick well inside the retry window so
+                    # a refused check is retried about an hour later;
+                    # refresh() itself keeps a working install to one GitHub
+                    # call a day.
+                    for job_id in ("update_check", "update_check_android"):
+                        tick = jobs[job_id].trigger.interval.total_seconds()
+                        assert tick <= main_mod.update_check.RETRY_AFTER_FAILURE_SECONDS / 2, job_id
                     # The error/missed listener is attached.
                     assert len(sched._listeners) >= 1
 

tests/test_update_check_retry_after_failure.py

@@ -0,0 +1,162 @@
+"""A refused release check is retried within the hour, not the next day.
+
+A home connection shares GitHub's unauthenticated limit (60 requests an hour per
+public IP) with everything else in the house. The one check CashPilot makes at
+startup was refused with a 403 on a live dashboard, and the old cooldown treated
+that failure like a success: nothing asked again for 24 hours, so the update
+card said "unknown" and no phone was judged against the Android release for a
+whole day after every restart.
+
+The contract now: a check that worked is not repeated for a day; one that failed
+is repeated after RETRY_AFTER_FAILURE_SECONDS, and not before.
+"""
+
+from __future__ import annotations
+
+import asyncio
+from unittest.mock import AsyncMock, patch
+
+import httpx
+import pytest
+
+from app import update_check
+
+
+@pytest.fixture(autouse=True)
+def _clean():
+    update_check._reset_for_tests()
+    yield
+    update_check._reset_for_tests()
+
+
+def _client(*outcomes):
+    """An AsyncClient stand-in whose get() yields each outcome in turn."""
+    client = AsyncMock()
+    client.get.side_effect = list(outcomes)
+    client.__aenter__.return_value = client
+    return client
+
+
+def _ok(url, tag):
+    return httpx.Response(200, json={"tag_name": tag}, request=httpx.Request("GET", url))
+
+
+def _rate_limited(url):
+    response = httpx.Response(403, json={"message": "API rate limit exceeded"}, request=httpx.Request("GET", url))
+    return response
+
+
+class _Clock:
+    def __init__(self, start: float = 1_000_000.0):
+        self.now = start
+
+    def __call__(self) -> float:
+        return self.now
+
+
+@pytest.mark.parametrize(
+    ("refresh_name", "url", "tag"),
+    [
+        ("refresh", update_check.LATEST_URL, "v2.0.1"),
+        ("refresh_android", update_check.ANDROID_LATEST_URL, "v1.0.1"),
+    ],
+)
+class TestARefusedCheckIsRetriedSoon:
+    def _run(self, refresh_name, client, clock):
+        with (
+            patch.dict("os.environ", {"CASHPILOT_VERSION": "2.0.0"}, clear=False),
+            patch("app.update_check.httpx.AsyncClient", return_value=client),
+            patch("app.update_check.time.time", clock),
+        ):
+            return asyncio.run(getattr(update_check, refresh_name)())
+
+    def test_a_403_at_startup_is_retried_after_an_hour_not_a_day(self, refresh_name, url, tag):
+        clock = _Clock()
+        client = _client(_rate_limited(url), _ok(url, tag))
+
+        assert self._run(refresh_name, client, clock) is None  # startup: refused
+
+        clock.now += update_check.RETRY_AFTER_FAILURE_SECONDS - 1
+        assert self._run(refresh_name, client, clock) is None
+        assert client.get.await_count == 1, "retried before the failure cooldown ran out"
+
+        clock.now += 1
+        assert self._run(refresh_name, client, clock) == tag
+        assert client.get.await_count == 2, "a refused check waited longer than the retry window"
+
+    def test_a_check_that_worked_still_waits_a_day(self, refresh_name, url, tag):
+        clock = _Clock()
+        client = _client(_ok(url, tag), _ok(url, "v9.9.9"))
+
+        assert self._run(refresh_name, client, clock) == tag
+        clock.now += update_check.RETRY_AFTER_FAILURE_SECONDS * 5
+        assert self._run(refresh_name, client, clock) == tag
+        assert client.get.await_count == 1, "a working install asked GitHub more than once a day"
+
+        clock.now = 1_000_000.0 + update_check.CHECK_INTERVAL_SECONDS
+        assert self._run(refresh_name, client, clock) == "v9.9.9"
+        assert client.get.await_count == 2
+
+    def test_a_failure_after_a_success_keeps_the_known_tag(self, refresh_name, url, tag):
+        """A refused retry must not blank what was already known."""
+        clock = _Clock()
+        client = _client(_ok(url, tag), _rate_limited(url))
+
+        assert self._run(refresh_name, client, clock) == tag
+        clock.now += update_check.CHECK_INTERVAL_SECONDS
+        assert self._run(refresh_name, client, clock) == tag
+        assert client.get.await_count == 2
+
+
+def test_the_retry_window_is_shorter_than_a_day_and_not_a_hammer():
+    assert 15 * 60 <= update_check.RETRY_AFTER_FAILURE_SECONDS < update_check.CHECK_INTERVAL_SECONDS
+
+
+@pytest.mark.parametrize(
+    ("refresh_name", "url"),
+    [
+        ("refresh", update_check.LATEST_URL),
+        ("refresh_android", update_check.ANDROID_LATEST_URL),
+    ],
+)
+def test_a_reply_without_a_usable_tag_keeps_the_known_tag_and_retries_soon(refresh_name, url):
+    """A 200 whose tag is not a version is a failed check, not "no release"."""
+    clock = _Clock()
+    bad = httpx.Response(200, json={"tag_name": "nightly"}, request=httpx.Request("GET", url))
+    client = _client(_ok(url, "v1.2.3"), bad, _ok(url, "v1.2.4"))
+    with (
+        patch.dict("os.environ", {"CASHPILOT_VERSION": "2.0.0"}, clear=False),
+        patch("app.update_check.httpx.AsyncClient", return_value=client),
+        patch("app.update_check.time.time", clock),
+    ):
+        refresh = getattr(update_check, refresh_name)
+        assert asyncio.run(refresh()) == "v1.2.3"
+        clock.now += update_check.CHECK_INTERVAL_SECONDS
+        assert asyncio.run(refresh()) == "v1.2.3", "an unusable reply blanked the known tag"
+        clock.now += update_check.RETRY_AFTER_FAILURE_SECONDS
+        assert asyncio.run(refresh()) == "v1.2.4", "an unusable reply waited a day before the next try"
+    assert client.get.await_count == 3
+
+
+def test_a_tick_just_before_the_cooldown_ends_does_not_cost_a_whole_extra_window():
+    """Ticks run every SCHEDULER_TICK_SECONDS from scheduling time; the startup
+    check fails a few seconds later, so the first tick at the hour mark is just
+    early. The retry must not slip by a whole extra window."""
+    clock = _Clock()
+    url = update_check.LATEST_URL
+    client = _client(_rate_limited(url), _ok(url, "v2.0.1"))
+    scheduled_at = clock.now
+    with (
+        patch.dict("os.environ", {"CASHPILOT_VERSION": "2.0.0"}, clear=False),
+        patch("app.update_check.httpx.AsyncClient", return_value=client),
+        patch("app.update_check.time.time", clock),
+    ):
+        clock.now += 5  # the startup check runs a few seconds after the jobs are added
+        assert asyncio.run(update_check.refresh()) is None
+        tick = 0
+        while client.get.await_count < 2:
+            tick += 1
+            clock.now = scheduled_at + tick * update_check.SCHEDULER_TICK_SECONDS
+            asyncio.run(update_check.refresh())
+    waited = clock.now - (scheduled_at + 5)
+    assert waited < update_check.RETRY_AFTER_FAILURE_SECONDS * 1.5, f"retry came {waited:.0f} s after the failure"