diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f7b1fe28..7ff4f966b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,11 @@ ## [Unreleased] +## [v0.51.285] — 2026-06-06 — Release JA (stage-r19 — update-reload server-identity race fix) + +### Fixed +- **Don't reload the page until the *replacement* server is actually up after an update.** The post-update reload previously compared raw `/health` uptime, which couldn't distinguish a still-running old process from the restarted one (it could reload against the old process or hang). The client now reads a stable `server_started_at` identity before the update POST and reloads only once `/health` reports a *different* identity (with a null-baseline fallback). Both the force-update and regular apply paths read and pass the baseline. (#3654, @franksong2702; #874) + ## [v0.51.284] — 2026-06-05 — Release IZ (stage-w4 — sidebar status labels + cron-sessions toggle) ### Added diff --git a/api/routes.py b/api/routes.py index 52f48950d..1baaa04e2 100644 --- a/api/routes.py +++ b/api/routes.py @@ -4337,6 +4337,7 @@ def _handle_health(handler, parsed): "active_runs": int(run_check.get("active_runs") or 0), "runs": run_check.get("runs", []), "last_run_finished_at": run_check.get("last_run_finished_at"), + "server_started_at": SERVER_START_TIME, "uptime_seconds": round(time.time() - SERVER_START_TIME, 1), "accept_loop": _accept_loop_health(handler), } diff --git a/static/ui.js b/static/ui.js index 0f6ba1236..ac9d73b85 100644 --- a/static/ui.js +++ b/static/ui.js @@ -5647,6 +5647,7 @@ async function applyUpdates(){ } try{ const stashConflictMessages=[]; + const baselineServerIdentity = await _readHealthServerIdentity(); for(const target of targets){ const res=await api('/api/updates/apply',{method:'POST',body:JSON.stringify({target}),timeoutMs:120000}); if(!res.ok){ @@ -5663,7 +5664,7 @@ async function applyUpdates(){ showToast(stashConflictMessage||'Update applied — restarting…',stashConflictMessages.length?10000:undefined,stashConflictMessages.length?'warning':undefined); sessionStorage.removeItem('hermes-update-checked'); sessionStorage.removeItem('hermes-update-dismissed'); - _waitForServerThenReload(); + _waitForServerThenReload({baselineServerIdentity}); }catch(e){ const msg=_formatUpdateApplyExceptionMessage(e); if(errEl){errEl.textContent=msg;errEl.style.display='block';} @@ -5687,6 +5688,26 @@ function _showUpdateError(target,res){ forceBtn.style.display='inline-block'; } } +function _normalizeHealthServerIdentity(rawIdentity){ + if(rawIdentity===undefined||rawIdentity===null) return null; + if(typeof rawIdentity==='string'){ + const value=rawIdentity.trim(); + return value ? value : null; + } + const numeric=Number(rawIdentity); + return Number.isFinite(numeric) ? String(numeric) : null; +} + +async function _readHealthServerIdentity() { + try { + const r=await fetch(new URL('health', document.baseURI||location.href).href,{cache:'no-store'}); + if(!r.ok) return null; + const data=await r.json(); + return _normalizeHealthServerIdentity(data&&data.server_started_at); + } catch (_) { + return null; + } +} async function forceUpdate(btn){ const target=btn&&btn.dataset.target; if(!target) return; @@ -5702,6 +5723,7 @@ async function forceUpdate(btn){ const errEl=$('updateError'); if(errEl){errEl.style.display='none';} try{ + const baselineServerIdentity = await _readHealthServerIdentity(); const res=await api('/api/updates/force',{method:'POST',body:JSON.stringify({target}),timeoutMs:120000}); if(!res.ok){ if(errEl){errEl.textContent='Force update failed: '+(res.message||'unknown error');errEl.style.display='block';} @@ -5711,7 +5733,7 @@ async function forceUpdate(btn){ showToast('Force update applied — restarting…'); sessionStorage.removeItem('hermes-update-checked'); sessionStorage.removeItem('hermes-update-dismissed'); - _waitForServerThenReload(); + _waitForServerThenReload({baselineServerIdentity}); }catch(e){ if(errEl){errEl.textContent='Force update failed: '+e.message;errEl.style.display='block';} btn.disabled=false;btn.textContent='Force update'; @@ -5726,6 +5748,7 @@ async function _waitForServerThenReload(opts){ opts=opts||{}; const interval=opts.interval||500; const maxMs=opts.maxMs||15000; + const baselineServerIdentity=_normalizeHealthServerIdentity(opts.baselineServerIdentity); window._restartingForUpdate=true; const msgEl=$('reconnectMsg'); const banner=$('reconnectBanner'); @@ -5742,8 +5765,16 @@ async function _waitForServerThenReload(opts){ let data={}; try{ data=await r.json(); }catch(_){} if(data && data.status==='ok'){ - location.reload(); - return; + const nextServerIdentity=_normalizeHealthServerIdentity(data&&data.server_started_at); + if (baselineServerIdentity===null){ + location.reload(); + return; + } + if (nextServerIdentity!==null && nextServerIdentity !== baselineServerIdentity){ + location.reload(); + return; + } + // Keep polling while the server keeps reporting the same (pre-restart) process identity } } }catch(_){ /* socket closed during restart — retry */ } diff --git a/tests/test_update_apply_ui.py b/tests/test_update_apply_ui.py index 11a945f5b..4aa8b54f5 100644 --- a/tests/test_update_apply_ui.py +++ b/tests/test_update_apply_ui.py @@ -43,7 +43,7 @@ def test_update_apply_successful_stash_conflict_displays_recovery_message(): message_push = body.index("stashConflictMessages.push('Update applied ('+target+'):", stash_branch) persistent_display = body.index("errEl.textContent=stashConflictMessages.join('\\n\\n')", message_push) message_join = body.index("const stashConflictMessage=stashConflictMessages.join('\\n\\n');", persistent_display) - restart_wait = body.index("_waitForServerThenReload();", message_join) + restart_wait = body.index("_waitForServerThenReload", message_join) assert messages_decl < stash_branch < message_push < persistent_display < message_join < restart_wait assert "showToast(stashConflictMessage||'Update applied" in body diff --git a/tests/test_update_banner_fixes.py b/tests/test_update_banner_fixes.py index eda213c0e..5ba6861da 100644 --- a/tests/test_update_banner_fixes.py +++ b/tests/test_update_banner_fixes.py @@ -20,6 +20,7 @@ import sys import os import io import json +import subprocess import types REPO = pathlib.Path(__file__).parent.parent @@ -931,6 +932,156 @@ class TestUiJsUpdateBanner: "_waitForServerThenReload must call location.reload() once the server is ready" ) + def test_wait_for_server_requires_new_process_identity(self): + src = read('static/ui.js') + m = re.search(r'function\s+_waitForServerThenReload\b.*?\n\}', src, re.DOTALL) + assert m, "_waitForServerThenReload() not found" + fn = m.group(0) + assert 'baselineServerIdentity' in fn, ( + "_waitForServerThenReload() should capture and compare a baseline process identity" + ) + assert 'nextServerIdentity!==null&&nextServerIdentity!==baselineServerIdentity' in fn.replace(' ', ''), ( + "_waitForServerThenReload() should only reload when health process identity changes" + ) + assert 'baselineServerIdentity===null' in fn.replace(' ', ''), ( + "_waitForServerThenReload() should fallback to existing behavior when baseline is unavailable" + ) + + def test_wait_for_server_fallbacks_to_ready_on_missing_baseline(self): + """Healthy /health should reload immediately when baseline identity is missing.""" + src = read('static/ui.js') + start = src.index("async function _waitForServerThenReload") + brace = src.index("{", start) + depth = 0 + end = None + for idx in range(brace, len(src)): + ch = src[idx] + if ch == "{": + depth += 1 + elif ch == "}": + depth -= 1 + if depth == 0: + end = idx + 1 + break + assert end is not None, "_waitForServerThenReload body was not balanced" + fn = src[start:end] + + script = f""" +let now = 0; +let reloads = 0; +let fetches = 0; +const responses = [ + {{ ok: true, data: {{ status: 'ok', server_started_at: null, uptime_seconds: 120 }} }}, +]; +const _normalizeHealthServerIdentity = (rawIdentity) => rawIdentity===undefined||rawIdentity===null ? null : + (typeof rawIdentity==='string' ? (rawIdentity.trim() ? rawIdentity.trim() : null) : + Number.isFinite(Number(rawIdentity)) ? String(Number(rawIdentity)) : null); +global.window = {{}}; +global.document = {{ baseURI: 'http://127.0.0.1:8788/' }}; +global.location = {{ reload: () => {{ reloads += 1; }} }}; +global.$ = () => null; +global.Date = {{ now: () => now }}; +global.setTimeout = (cb, ms) => {{ now += ms || 0; cb(); return 0; }}; +global.fetch = async () => {{ + fetches += 1; + const next = responses.shift(); + if (!next) throw new Error('unexpected extra fetch'); + return {{ + ok: next.ok, + json: async () => next.data, + }}; +}}; +{fn} +(async () => {{ + await _waitForServerThenReload({{ interval: 1, maxMs: 10, baselineServerIdentity: null }}); + if (fetches !== 1) throw new Error('expected fallback baseline to reload on first healthy probe, got '+fetches); + if (reloads !== 1) throw new Error('expected exactly one reload on first healthy probe, got '+reloads); +}})().catch(err => {{ console.error(err.stack || err.message); process.exit(1); }}); +""" + subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True) + + def test_wait_for_server_ignores_old_identity_and_reloads_on_new_identity(self): + """Healthy /health from the old process should not reload until identity changes.""" + src = read('static/ui.js') + start = src.index("async function _waitForServerThenReload") + brace = src.index("{", start) + depth = 0 + end = None + for idx in range(brace, len(src)): + ch = src[idx] + if ch == "{": + depth += 1 + elif ch == "}": + depth -= 1 + if depth == 0: + end = idx + 1 + break + assert end is not None, "_waitForServerThenReload body was not balanced" + fn = src[start:end] + + script = f""" +let now = 0; +let reloads = 0; +let fetches = 0; +const responses = [ + {{ ok: true, data: {{ status: 'ok', server_started_at: '1001.234', uptime_seconds: 1000 }} }}, + {{ ok: true, data: {{ status: 'ok', server_started_at: '1001.234', uptime_seconds: 2000 }} }}, + {{ ok: true, data: {{ status: 'ok', server_started_at: '1001.235', uptime_seconds: 2010 }} }}, +]; +const _normalizeHealthServerIdentity = (rawIdentity) => rawIdentity===undefined||rawIdentity===null ? null : + (typeof rawIdentity==='string' ? (rawIdentity.trim() ? rawIdentity.trim() : null) : + Number.isFinite(Number(rawIdentity)) ? String(Number(rawIdentity)) : null); +global.window = {{}}; +global.document = {{ baseURI: 'http://127.0.0.1:8788/' }}; +global.location = {{ reload: () => {{ reloads += 1; }} }}; +global.$ = () => null; +global.Date = {{ now: () => now }}; +global.setTimeout = (cb, ms) => {{ now += ms || 0; cb(); return 0; }}; +global.fetch = async () => {{ + fetches += 1; + const next = responses.shift(); + if (!next) throw new Error('unexpected extra fetch'); + return {{ + ok: next.ok, + json: async () => next.data, + }}; +}}; +{fn} +(async () => {{ + await _waitForServerThenReload({{ interval: 1, maxMs: 20, baselineServerIdentity: '1001.234' }}); + if (fetches !== 3) throw new Error('expected old-process health to be ignored before identity changes, got '+fetches); + if (reloads !== 1) throw new Error('expected exactly one reload after new identity, got '+reloads); +}})().catch(err => {{ console.error(err.stack || err.message); process.exit(1); }}); +""" + subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True) + + def test_apply_and_force_updates_capture_identity(self): + src = read('static/ui.js') + apply_fn = re.search(r'function\s+applyUpdates\b.*?\n\}', src, re.DOTALL) + force_fn = re.search(r'function\s+forceUpdate\b.*?\n\}', src, re.DOTALL) + assert apply_fn, "applyUpdates() not found" + assert force_fn, "forceUpdate() not found" + apply_body = apply_fn.group(0) + force_body = force_fn.group(0) + assert '_readHealthServerIdentity()' in apply_body, ( + "applyUpdates() must call _readHealthServerIdentity() before reload wait" + ) + assert '_readHealthServerIdentity()' in force_body, ( + "forceUpdate() must call _readHealthServerIdentity() before reload wait" + ) + assert '_waitForServerThenReload({baselineServerIdentity})' in apply_body, ( + "applyUpdates() must pass baselineServerIdentity to _waitForServerThenReload()" + ) + assert '_waitForServerThenReload({baselineServerIdentity})' in force_body, ( + "forceUpdate() must pass baselineServerIdentity to _waitForServerThenReload()" + ) + assert apply_body.index('_readHealthServerIdentity()') < apply_body.index('_waitForServerThenReload({baselineServerIdentity})'), ( + "applyUpdates() must capture baseline before reload scheduling" + ) + assert force_body.index('_readHealthServerIdentity()') < force_body.index("const res=await api('/api/updates/force'"), ( + "forceUpdate() must capture baseline before force POST" + ) + def test_refresh_session_handles_restart_mode(self): """When _restartingForUpdate flag is set, refreshSession() must do a full page reload rather than hit /api/session (which will 502 while