From b9ef2b30ee9a48d9d32dab98708215955ef7e9d1 Mon Sep 17 00:00:00 2001 From: EMRG Evolution Date: Tue, 20 Oct 2026 14:19:05 +0800 Subject: [PATCH 1/2] emrg: an upgrade tick fires only for a strictly newer target (#2404) `tick()` tested the target for equality with the installed version, and `_find_target_tag()` never looks at what is installed, so every older target passed the guard: with a newer release still inside its delay window, the newest eligible tag is older than the host and the tick started a session asking to downgrade. The module already defines parse_version() and is_newer() fifty lines above the guard and used neither at the call site. Now the tick returns unless the target is strictly newer. An unreadable or absent version.txt still triggers: it reads as "", parse_version("") is (), and "not yet installed" is exactly what that path has always meant. --- emrg/server/upgrade.py | 13 +++++++++++-- tests/test_upgrade.py | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) 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_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( From 82429bd3a096a493d84759c3c88bc6bc3232c9bd Mon Sep 17 00:00:00 2001 From: EMRG Evolution Date: Tue, 20 Oct 2026 14:29:40 +0800 Subject: [PATCH 2/2] emrg: the release-tag guard reads the comparison the chain now makes (#2404) `tests/test_check_release_tag.py` pins the premise its own subject rests on, by looking for the chain's comparison in `upgrade.py`. The tick no longer string-compares, so the literal it probed for is gone; what the guard needs to keep holding is that the installed version is *ordered* against the tag's, and the direction that costs a host a forever-retrying upgrade session is a tag above the built tree's version. The probe now reads that call. --- tests/test_check_release_tag.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) 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" )