diff --git a/emrg/server/upgrade.py b/emrg/server/upgrade.py index 9351f7af..b6265ead 100644 --- a/emrg/server/upgrade.py +++ b/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) diff --git a/tests/test_check_release_tag.py b/tests/test_check_release_tag.py index e86d0484..02dd2756 100644 --- a/tests/test_check_release_tag.py +++ b/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" ) diff --git a/tests/test_upgrade.py b/tests/test_upgrade.py index b2a9c987..46e4adea 100644 --- a/tests/test_upgrade.py +++ b/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(