mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-07-16 20:50:18 +00:00
* Fix update reload readiness race — poll /health server identity before reload (#3654) Replaces the raw-uptime comparison (couldn't distinguish a fresh old process from the restarted one) with a stable server_started_at identity read before the update POST; reloads only when the identity changes. Both the force-update and regular apply paths read + pass the baseline. (#874, #3654) Co-authored-by: Frank Song <franksong2702@gmail.com> * docs(changelog): v0.51.285 — Release JA (stage-r19) --------- Co-authored-by: Frank Song <franksong2702@gmail.com> Co-authored-by: nesquena-hermes <[email protected]>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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),
|
||||
}
|
||||
|
||||
+35
-4
@@ -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 */ }
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user