Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions api/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
Expand Down
39 changes: 35 additions & 4 deletions static/ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -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){
Expand All @@ -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';}
Expand All @@ -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;
Expand All @@ -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';}
Expand All @@ -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';
Expand All @@ -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');
Expand All @@ -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 */ }
Expand Down
2 changes: 1 addition & 1 deletion tests/test_update_apply_ui.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
151 changes: 151 additions & 0 deletions tests/test_update_banner_fixes.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import os
import io
import json
import subprocess
import types

REPO = pathlib.Path(__file__).parent.parent
Expand Down Expand Up @@ -931,6 +932,156 @@ def test_wait_for_server_polls_health(self):
"_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
Expand Down
Loading