diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ef32abd..2ce43aee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,23 @@ # Hermes Web UI -- Changelog +## [v0.50.287] — 2026-05-03 + +### Fixed (1 PR — closes another vector for the pending-message-loss class) + +- **Self-update refuses to re-exec while chat streams are active** (#1565, @ai-ag2026) — closes the last known vector for the pending-message-loss class fixed in #1471/#1543/#1558. The WebUI self-update path schedules an in-process `os.execv()` re-exec after applying updates. That restart-equivalent path is independent of systemd, so when a browser user clicks "Update Now" while a chat is streaming, the process can be replaced mid-stream — same data-loss class as the stale-stream/pending-message work in v0.50.279/v0.50.284. **Fix:** new `_active_stream_count()` helper reads `len(STREAMS)` under `STREAMS_LOCK`; both `apply_update(target)` and `apply_force_update(target)` short-circuit at function entry with a structured `{ok: False, restart_blocked: True, active_streams: N, message: "Cannot update {target} while {N} active chat stream{s} is running. Wait for the response to finish, then retry the update."}` response — **before** any git command runs and **before** scheduling restart. Frontend integration: `_showUpdateError` in `static/ui.js:2882` already routes `res.message` to the persistent error element, and the "Force update" button only reveals on `res.conflict || res.diverged` (neither set for `restart_blocked`), so the user gets a clean error and correctly cannot escalate to force-update (which has the same restart problem and is also blocked by the same guard). 2 new regression tests in `tests/test_update_banner_fixes.py::TestApplyUpdateRestartSafety` pin the refusal shape AND the absence of side effects (`_run_git` never called; `_schedule_restart` raises if invoked). Pre-release Opus advisor: SHIP AS-IS — verified that the residual race window (between guard release and `_apply_lock` acquire) is bounded by design and recoverable via the #1543 pending-message recovery path. Closing the window would require holding `STREAMS_LOCK` across the whole git+restart sequence, which would block every new chat for the duration of an update — worse UX than the residual race. + +### Tests + +4051 → **4053 passing** (+2 from PR #1565). 0 regressions. Full suite in 120s. + +### Pre-release verification + +- All 31 update-banner tests pass standalone in 3.5s (29 existing + 2 new). +- All 4053 tests pass in the full suite. +- Browser sanity (HTTP API checks against port 8789): 11/11 endpoints verified. +- Pre-release Opus advisor: SHIP AS-IS — all 5 verification questions resolved (race-window bounded, lock ordering safe, no deadlock, frontend integration clean, test isolation robust against assertion failures). + + ## [v0.50.286] — 2026-05-03 ### Fixed (1 PR — closes #1560) diff --git a/ROADMAP.md b/ROADMAP.md index 7351ff5b..a4a83eb5 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ > Web companion to the Hermes Agent CLI. Same workflows, browser-native. > -> Last updated: v0.50.286 (May 03, 2026) — 4051 tests collected +> Last updated: v0.50.287 (May 03, 2026) — 4053 tests collected > Test source: `pytest tests/ --collect-only -q` > Per-version detail: see [CHANGELOG.md](./CHANGELOG.md) diff --git a/TESTING.md b/TESTING.md index 465a0740..7ac8ffd9 100644 --- a/TESTING.md +++ b/TESTING.md @@ -1835,8 +1835,8 @@ Bridged CLI sessions: --- -*Last updated: v0.50.286, May 03, 2026* -*Total automated tests collected: 4051* +*Last updated: v0.50.287, May 03, 2026* +*Total automated tests collected: 4053* *Regression gate: tests/test_regressions.py* *Run: pytest tests/ -v --timeout=60* *Source: /* diff --git a/api/updates.py b/api/updates.py index 1c286970..77c1aca3 100644 --- a/api/updates.py +++ b/api/updates.py @@ -13,7 +13,7 @@ import threading import time from pathlib import Path -from api.config import REPO_ROOT +from api.config import REPO_ROOT, STREAMS, STREAMS_LOCK # Lazy -- may be None if agent not found try: @@ -28,6 +28,32 @@ _apply_lock = threading.Lock() # prevents concurrent stash/pull/pop on same re CACHE_TTL = 1800 # 30 minutes +def _active_stream_count() -> int: + """Return the current in-memory chat stream count. + + Self-update schedules an in-process re-exec after git pull/reset. That is + restart-equivalent for live streams, even when systemd does not see a unit + restart. Refuse update/force-update while a stream exists so a browser + update click cannot recreate the pending-message loss class fixed in #1543. + """ + with STREAMS_LOCK: + return len(STREAMS) + + +def _restart_blocked_response(target: str, active_streams: int) -> dict: + plural = "s" if active_streams != 1 else "" + return { + 'ok': False, + 'message': ( + f'Cannot update {target} while {active_streams} active chat stream{plural} ' + 'is running. Wait for the response to finish, then retry the update.' + ), + 'target': target, + 'restart_blocked': True, + 'active_streams': active_streams, + } + + def _run_git(args, cwd, timeout=10): """Run a git command and return (useful output, ok). @@ -249,6 +275,10 @@ def apply_force_update(target: str) -> dict: response with ``conflict: True`` or ``diverged: True`` and the user has confirmed they want to discard local changes. """ + active_streams = _active_stream_count() + if active_streams: + return _restart_blocked_response(target, active_streams) + if not _apply_lock.acquire(blocking=False): return {'ok': False, 'message': 'Update already in progress'} try: @@ -299,6 +329,10 @@ def apply_force_update(target: str) -> dict: def apply_update(target): """Stash, pull --ff-only, pop for the given target repo.""" + active_streams = _active_stream_count() + if active_streams: + return _restart_blocked_response(target, active_streams) + if not _apply_lock.acquire(blocking=False): return {'ok': False, 'message': 'Update already in progress'} try: diff --git a/tests/test_update_banner_fixes.py b/tests/test_update_banner_fixes.py index 5532ecdc..31687b0f 100644 --- a/tests/test_update_banner_fixes.py +++ b/tests/test_update_banner_fixes.py @@ -117,6 +117,66 @@ class TestScheduleRestart: assert execv_called, "_schedule_restart must eventually call os.execv" +class TestApplyUpdateRestartSafety: + """Self-update must not re-exec while chat streams are active.""" + + def test_apply_update_refuses_when_stream_active(self, tmp_path, monkeypatch): + import queue + import api.updates as upd + from api.config import STREAMS, STREAMS_LOCK + + (tmp_path / '.git').mkdir() + monkeypatch.setattr(upd, 'REPO_ROOT', tmp_path) + monkeypatch.setattr(upd, '_AGENT_DIR', tmp_path) + called = [] + monkeypatch.setattr(upd, '_run_git', lambda *a, **k: (called.append(a) or ('', True))) + monkeypatch.setattr(upd, '_schedule_restart', lambda delay=2.0: (_ for _ in ()).throw(AssertionError('must not restart'))) + + with STREAMS_LOCK: + old = dict(STREAMS) + STREAMS.clear() + STREAMS['stream_active'] = queue.Queue() + try: + result = upd.apply_update('webui') + finally: + with STREAMS_LOCK: + STREAMS.clear() + STREAMS.update(old) + + assert result['ok'] is False + assert result.get('active_streams') == 1 + assert result.get('restart_blocked') is True + assert 'active chat stream' in result['message'] + assert called == [] + + def test_force_update_refuses_when_stream_active(self, tmp_path, monkeypatch): + import queue + import api.updates as upd + from api.config import STREAMS, STREAMS_LOCK + + (tmp_path / '.git').mkdir() + monkeypatch.setattr(upd, 'REPO_ROOT', tmp_path) + monkeypatch.setattr(upd, '_AGENT_DIR', tmp_path) + monkeypatch.setattr(upd, '_run_git', lambda *a, **k: (_ for _ in ()).throw(AssertionError('must not run git'))) + monkeypatch.setattr(upd, '_schedule_restart', lambda delay=2.0: (_ for _ in ()).throw(AssertionError('must not restart'))) + + with STREAMS_LOCK: + old = dict(STREAMS) + STREAMS.clear() + STREAMS['stream_active'] = queue.Queue() + try: + result = upd.apply_force_update('agent') + finally: + with STREAMS_LOCK: + STREAMS.clear() + STREAMS.update(old) + + assert result['ok'] is False + assert result.get('active_streams') == 1 + assert result.get('restart_blocked') is True + assert 'active chat stream' in result['message'] + + class TestSuccessfulUpdateReturnsRestartScheduled: """#814 — successful apply_update must return restart_scheduled: True."""