Fix update reload readiness race

[franksong2702]

Thinking Path

• Hermes WebUI self-update should restart the local server and reload only after the replacement process is ready.
• The existing helper already polls /health, but it accepted any healthy response.
• During the scheduled restart window, the old process can still answer /health with status: "ok".
• This PR captures the pre-update /health.uptime_seconds baseline and only reloads after a later healthy probe proves a new process by reporting a lower uptime.
• The result is a small readiness-contract hardening instead of a blind longer timeout.

What Changed

• Added a _readHealthUptimeSeconds() helper that reads the mount-relative /health endpoint.
• Updated applyUpdates() and forceUpdate() to pass a baseline uptime into _waitForServerThenReload().
• Updated _waitForServerThenReload() to keep polling while healthy probes still look like the old process.
• Added focused regression coverage, including a Node-driven behavioral test that verifies old-process health is ignored until a new-process uptime appears.
• Added a release-note-ready CHANGELOG.md entry.

Why It Matters

• Prevents the browser from reloading into an update/restart race where the old process answered /health just before exiting.
• Keeps the existing subpath-safe health URL behavior.
• Avoids masking the bug with a longer fixed sleep.

Verification

• python -m pytest tests/test_update_banner_fixes.py -k "wait_for_server_polls_health or wait_for_server_requires_new_uptime or wait_for_server_ignores_old_process_health_until_new_uptime or apply_and_force_updates_capture_uptime" -q
• python -m pytest tests/test_update_banner_fixes.py tests/test_update_apply_ui.py -q
• node --check static/ui.js
• git diff --check

Risks / Follow-ups

• If the baseline uptime probe is unavailable, the helper preserves the existing readiness behavior rather than blocking reload forever.
• Extremely fresh old-process baselines may require polling until timeout if the new process uptime is not lower yet, but this is preferable to reloading into the old process.

Model Used

• AI-assisted.
• Coordinator: OpenAI GPT-5 Codex.
• Worker sub-agent: OpenAI gpt-5.3-codex-spark (Hegel).
• Coordinator added the final behavior-level acceptance test and ran verification.

Fixes #3619

[greptile-apps]

Greptile Summary

This PR replaces the update-reload readiness guard from an uptime-seconds comparison to a stable process-identity comparison using server_started_at, a new field added to the /health response. The approach is sound: the backend exposes a constant epoch timestamp per process, and the client polls until the identity changes before triggering a reload.

• api/routes.py: Adds server_started_at: SERVER_START_TIME to the health payload so the client can distinguish old-process from new-process responses.
• static/ui.js: Replaces _readHealthUptimeSeconds / uptime_seconds logic with _readHealthServerIdentity / server_started_at; both applyUpdates and forceUpdate now correctly capture the baseline before triggering the restart API call.
• Tests: New behavioral Node-driven tests exercise the wait loop end-to-end; ordering assertions tighten the capture-before-trigger contract for both update paths.

Confidence Score: 5/5

Safe to merge; the core logic is correct and the previously reported baseline-ordering bug in forceUpdate is fixed.

Both update paths correctly capture the process identity before triggering the restart API call. The new server_started_at field is a stable constant per process, making it a reliable discriminator. The fallback behavior (null baseline → reload on first healthy probe) is preserved and exercised. The only gaps are in test coverage: behavioral tests simulate string-typed identity values while the server emits a numeric float, and the applyUpdates ordering assertion does not pin against the API call site — neither affects the runtime behavior shipped in this PR.

No files require special attention for merging; the test gaps in test_update_banner_fixes.py are worth addressing in a follow-up.

Important Files Changed

Filename	Overview
static/ui.js	Switches readiness guard from uptime-seconds to server_started_at identity; baseline capture ordering is correct in both applyUpdates and forceUpdate; null-identity fallback behavior unchanged from previous threads.
api/routes.py	Adds server_started_at (a constant float) to the health payload; straightforward one-liner addition with no logic changes.
tests/test_update_banner_fixes.py	Adds behavioral Node tests and ordering assertions; behavioral tests use string-typed server_started_at values while the real server emits a JSON float, leaving the numeric normalization branch untested end-to-end.
tests/test_update_apply_ui.py	Minor update to broaden the _waitForServerThenReload string search to match the new call-with-options form; no logic issues.

Sequence Diagram

sequenceDiagram
    participant Browser
    participant OldServer as Old Server Process
    participant NewServer as New Server Process

    Browser->>OldServer: GET /health (baseline capture)
    OldServer-->>Browser: "{status:'ok', server_started_at: T0}"
    note over Browser: baselineServerIdentity = normalize(T0)

    Browser->>OldServer: POST /api/updates/apply (or /force)
    OldServer-->>Browser: 200 OK (restart triggered)

    loop Poll until identity changes or timeout (15s)
        Browser->>OldServer: GET /health (old process still alive)
        OldServer-->>Browser: "{status:'ok', server_started_at: T0}"
        note over Browser: identity == baseline → keep polling
        Browser->>NewServer: GET /health
        NewServer-->>Browser: "{status:'ok', server_started_at: T1}"
        note over Browser: T1 ≠ T0 → reload!
    end

    Browser->>Browser: location.reload()

Loading

_{Reviews (2): Last reviewed commit: "Remove stray changelog whitespace" | Re-trigger Greptile}

[greptile-apps]

If the baseline was captured successfully (non-null) but the restarted server's health probe returns status: 'ok' without a uptime_seconds field, Number(undefined) yields NaN, Number.isFinite(NaN) is false, and the condition never fires. The browser will poll silently for the full 15 s then show "click Reload when ready" even though the server is already healthy. Any environment where the health schema omits uptime_seconds (even conditionally) will regress to a manual-reload UX after applying updates.

[nesquena-hermes]

Read the full diff (static/ui.js:5608-5610, 5680-5682, 5694-5728), the /health handler it depends on, and all three new tests. This is the right fix for #3619 — it implements exactly the uptime-baseline approach (Option 1) that came up in the issue thread, and it's the correct call over a longer blind delay.

The /health contract this depends on is real

_handle_health returns the field the client now reads, derived from a process-global start time:

# api/routes.py:4435
"uptime_seconds": round(time.time() - SERVER_START_TIME, 1),

and SERVER_START_TIME = time.time() is set once at import in api/config.py:4912, so it resets on every os.execv re-exec. The "new process reports a lower uptime" invariant the PR relies on holds. The status==='ok' gate in the poll also matches — _handle_health only emits ok when the stream-lock check passes, otherwise degraded/503. Good.

The behavioral test actually exercises the logic

test_wait_for_server_ignores_old_process_health_until_new_uptime extracts the real _waitForServerThenReload body, drives it under Node with a fake fetch returning uptime_seconds: 120 (old) then 2 (new) against baselineUptimeSeconds: 100, and asserts the old-process ok is ignored and exactly one reload fires after the lower uptime appears. That's a real regression guard, not just a string match.

Minor: baseline capture ordering is inconsistent between the two callers

applyUpdates() captures the baseline before the apply POST loop (ui.js:5609):

const baselineUptimeSeconds = await _readHealthUptimeSeconds();
for(const target of targets){ ... }

but forceUpdate() captures it after the force POST returns (ui.js:5681):

const res=await api('/api/updates/force',{...});
...
const baselineUptimeSeconds = await _readHealthUptimeSeconds();

In practice this is safe because the server delays the re-exec — _schedule_restart(delay=2.0) (api/updates.py:1015), gated further by _wait_until_restart_safe — so the post-POST read still hits the old process. But it's racier than the applyUpdates path for no benefit. Recommend moving the _readHealthUptimeSeconds() call above the force POST in forceUpdate for symmetry, so both paths capture the baseline while the old process is unambiguously still the one answering.

Residual edge the author already flagged (uptime-only has a fresh-baseline corner)

The PR notes "extremely fresh old-process baselines may require polling until timeout." Concretely: if the server was started seconds ago (baseline ≈ 1.5s) when the user clicks update, the new process can accumulate >1.5s of uptime before the client's first successful probe, so nextUptime < baseline stays false and the poll rides out to the 15s maxMs and shows "taking longer than expected." Falls back gracefully (manual reload works), but it's a real corner.

If you want to close it definitively rather than relying on uptime monotonicity, add a stable boot_id (or PID) to the /health payload next to uptime_seconds, capture it as the baseline, and reload as soon as the probed boot_id differs. That removes the dependence on the new process having a smaller clock value and is robust regardless of how fresh the old process was. Not a blocker for this PR — the uptime approach handles the normal case (post-restart uptime_seconds near zero vs. a baseline of minutes/hours) cleanly — but worth a follow-up note.

Otherwise this is clean and well-tested. LGTM with the forceUpdate ordering tweak.

[nesquena-hermes]

Re-reviewed in a fresh release gate. Thanks @franksong2702 — the goal (don't reload until the replacement server is actually up) is exactly right, and the /health.uptime_seconds baseline idea is a sound approach. But the regression gate (Codex, verified by running the code) surfaced a chain of issues, and the last one is a design-level concern that needs a backend /health change, so keeping hold + changes-requested for a rework.

I shipped the PDF-preview half of the original batch separately (v0.51.278); this PR is held on its own.

The issues found (in order of depth)

1. null baseline was defeated by numeric coercion. _readHealthUptimeSeconds() returns null when pre-update /health is unreadable, but _waitForServerThenReload() did Number.isFinite(Number(opts.baselineUptimeSeconds)) ? Number(...) : null — and Number(null) === 0 (finite), so the baseline became 0, making the intended baselineUptimeSeconds===null fallback-reload branch unreachable. Since no real uptime is < 0, the poll times out and the page never auto-reloads. (Fix: preserve null/undefined before the numeric coercion.)

2. forceUpdate() read the baseline on the wrong side of the POST. It captured baselineUptimeSeconds after await api('/api/updates/force', …) returned — but the backend schedules the restart while handling that request, so the follow-up /health can hit the new process, making the baseline reflect the new proc → the < gate never trips → hang. (Fix: capture the baseline before the POST, the way applyUpdates() does.)

3. The core design flaw — raw-uptime comparison is not a reliable "new process" signal. Even with 1 and 2 fixed, the gate nextUptimeSeconds < baselineUptimeSeconds fails when the old process was very fresh. Verified by running _waitForServerThenReload({baselineUptimeSeconds: 0.2}) against a healthy uptime_seconds: 1.0 response: the new process legitimately answers at 1.0s, 1.0 < 0.2 is false, so it never reloads — both applyUpdates() and forceUpdate() sit on the restart banner until the 15s timeout / manual reload. /health only exposes a rounded uptime_seconds (api/routes.py ~4340), so uptime alone can't distinguish "old proc, low uptime" from "new proc, low uptime."

Suggested rework — compare process identity, not raw uptime

Add a stable per-process boot identity to /health and gate the reload on that changing:

• Backend: expose a server_started_at (epoch, set once at process start) or a random boot_id on /health (api/routes.py).
• Frontend: capture that identity before the apply/force POST (both paths), and reload when a later healthy probe reports a different identity. Keep the existing null-baseline fallback (reload on first healthy response) for when the pre-update probe failed.

That makes the readiness signal correct regardless of how fresh either process is. Items 1 and 2 fold into this naturally once the comparison is identity-based. Happy to review again as soon as the boot-id approach is in — the rest of the structure (_waitForServerThenReload, the banner, the poll loop) is good and can stay.

[franksong2702]

Coordinator update:

• Reworked the restart-readiness gate to compare /health.server_started_at process identity instead of raw uptime.
• Captures the baseline identity before both applyUpdates() and forceUpdate() POSTs.
• Preserves null/failed-baseline fallback: reload on the first healthy response.
• Cleaned the branch onto latest origin/master (v0.51.283 / a7241f7); no changelog-only noise remains.
• Local verification: /Users/xuefusong/.hermes/hermes-agent/venv/bin/python -m pytest tests/test_update_banner_fixes.py tests/test_update_apply_ui.py -q => 82 passed.
• JS syntax: node --check static/ui.js.
• Whitespace: git diff --check origin/master..HEAD.
• Coordinator behavioral acceptance also executed the extracted UI functions to verify old-process same-identity health is ignored until a changed identity appears, and apply/force baseline reads happen before their POSTs.