Merge pull request #1566 from nesquena/stage-287
v0.50.287 — Self-update active-stream guard (#1565 by @ai-ag2026)
This commit is contained in:
commit
70f86d56f4
5 changed files with 116 additions and 4 deletions
18
CHANGELOG.md
18
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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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: <repo>/*
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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."""
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue