emrg: an upgrade tick fires only for a strictly newer target

emrg/server/upgrade.py

@@ -334,8 +334,17 @@ async def tick(self) -> None:
         if not target:
             return  # nothing eligible this round
         local = self._read_local_version()
-        if local == target.lstrip("v"):
-            return  # already at target — nothing to do
+        # **Strictly newer, not merely different** (issue #2404). `_find_target_tag`
+        # picks the highest release outside the delay window and does not look at what
+        # is installed, so an *equality* test lets every older target through: with a
+        # newer release still inside its delay window, the newest eligible tag is
+        # older than the host, and the tick would start a session asking to downgrade.
+        # Observed live at 0.2.58 offered `v0.2.54`; the agent refused, but that made
+        # the install's safety a property of the agent's judgement rather than of this
+        # guard. An unreadable or absent `version.txt` reads as `""` → `()` and is
+        # still "not yet installed", so the documented trigger-on-`""` path survives.
+        if not is_newer(parse_version(target), parse_version(local)):
+            return  # same version, or a target older than what is installed
         if self._is_waiting(target):
             return  # an attempt at THIS tag did not move version.txt; still paying it off
         await self._trigger(target)

tests/test_check_release_tag.py

@@ -163,12 +163,19 @@ def test_the_chain_still_compares_the_way_this_guard_assumes() -> None:
     If `UpgradeManager.tick` ever stops comparing `version.txt` with the tag, the
     consequence this guard is built on disappears — and the guard, not the chain,
     is what would then be wrong.
+
+    What the chain must keep doing is **ordering** the installed version against the
+    tag's, and it does that with the module's own `parse_version` / `is_newer` rather
+    than by string equality (issue #2404: equality also admitted targets *older* than
+    the host). The direction still matters and is the same direction this guard
+    rests on: a tag announcing a version **above** the one `build-runtime.sh` writes
+    keeps every host re-triggering, because the installed version can never reach it.
     """
     source = (REPO_ROOT / "emrg" / "server" / "upgrade.py").read_text(encoding="utf-8")
-    assert 'target.lstrip("v")' in source, (
-        "emrg/server/upgrade.py no longer compares version.txt with "
-        'target.lstrip("v") — re-derive what a mismatched tag costs before '
-        "trusting the guard's docstring"
+    assert "is_newer(parse_version(target), parse_version(local))" in source, (
+        "emrg/server/upgrade.py no longer orders the tag's version against the "
+        "installed one — re-derive what a mismatched tag costs before trusting the "
+        "guard's docstring"
     )
 
 

tests/test_upgrade.py

@@ -134,6 +134,43 @@ def test_tick_no_trigger_when_local_matches_target(monkeypatch, tmp_path):
     assert calls == [], "local == target → no trigger"
 
 
+def test_tick_no_trigger_when_the_eligible_target_is_older(monkeypatch, tmp_path):
+    """A downgrade is never an upgrade (issue #2404).
+
+    The reachable shape: the host already runs the newest release, but that release
+    is still inside the delay window, so `_find_target_tag` answers an *older* one.
+    An equality test passes it through and the tick starts a session asking to
+    downgrade; only a strictly-newer comparison stops it. Asserted on the trigger
+    calls, because a test that read the guard's own expression would be green for a
+    guard that is never reached.
+    """
+    releases = [_release("v0.2.56", 60 * 60 * 24 * 5)]
+    monkeypatch.setattr(
+        "emrg.server.upgrade.VERSION_FILE", tmp_path / "version.txt"
+    )
+    (tmp_path / "version.txt").write_text("0.2.58\n", encoding="utf-8")
+    calls = asyncio.run(_tick_with_releases(monkeypatch, releases))
+    assert calls == [], "an older target is not an upgrade target"
+
+
+def test_tick_still_triggers_when_version_txt_is_absent(monkeypatch, tmp_path):
+    """The strictly-newer guard must not eat the documented `""` path.
+
+    `_read_local_version` keeps `""` rather than refusing, which reads as "not yet
+    installed" — a trigger — and that is the semantics an absent `version.txt` has
+    had since the module was written. `parse_version("")` is `()`, which is never
+    newer, so the guard keeps the trigger only because it asks "is the target newer
+    than `()`" rather than "is the local version readable".
+    """
+    releases = [_release("v0.2.57", 60 * 60 * 24 * 2)]
+    monkeypatch.setattr(
+        "emrg.server.upgrade.VERSION_FILE", tmp_path / "version.txt"
+    )  # never written: the file does not exist
+    calls = asyncio.run(_tick_with_releases(monkeypatch, releases))
+    assert len(calls) == 1, "an absent version.txt is 'not yet installed', not 'up to date'"
+    assert "v0.2.57" in calls[0]["prompt"]
+
+
 def test_tick_disabled_by_config(monkeypatch, tmp_path):
     releases = [_release("v0.2.57", 60 * 60 * 24 * 2)]
     monkeypatch.setattr(