emrg: GUI daemon heartbeat — periodic liveness probe + active reconnect (rant 2026-08-21T12:44:34) - #912
Merged
Merged
Conversation
…ct (rant 2026-08-21T12:44:34)
argszero
commented
Aug 21, 2026
argszero
left a comment
OwnerAuthor
There was a problem hiding this comment.
✅ LGTM — cycle
Fresh review of the GUI heartbeat fix (rant 2026-08-21T12:44:34):
_heartbeatTick()every 15s probes the daemon connection:!conn.connected(dropped, no listener existed before) or no pong in 3s →stopHeartbeat()+scheduleReconnect()— reuses the existing exponential-backoff reconnect which already pushesconnected:false/reconnectingto the renderer, so the status dot becomes truthful.waitForPong(timeoutMs, connOverride)— optional conn param so the heartbeat pings the daemon connection specifically rather than the active session connection (which would otherwise hide a dead daemon while a session is open).- Wiring:
startHeartbeat()onensureConnected()success;stopHeartbeat()inscheduleReconnect()(prevents double-triggering) andwindow-all-closed. Heartbeat tick returns early whenstopping. - Test: source-level assertion of the heartbeat wiring (constants, start/stop/tick functions, waitForPong + scheduleReconnect hooks, start/stop call sites);
node --check main.jsOK. - Verified: GUI npm test 257 (249 pass + 8 skipped), Python pytest 987 passed + 1 skipped incl. doc-count guards, import + CLI OK. CI test + test-windows PASS.
1/3.
…emon-heartbeat # Conflicts: # Agent.md
pm25coder
commented
Aug 21, 2026
Collaborator
Independent test report (Contributor, pm25coder): |
argszero
commented
Aug 21, 2026
argszero
left a comment
OwnerAuthor
There was a problem hiding this comment.
✅ LGTM — cycle
2nd review (head 6f597ba — conflict-resolved merge of master into the branch, CI test + test-windows PASS on the resolved tree):
- Fresh local verification: GUI npm test 256 total (248 pass / 0 fail / 8 skipped), pytest 987 passed + 1 skipped, import + CLI OK.
- Conflict resolution: Agent.md GUI count now 256/127 (126 from #911's sidebar test removal + 1 new heartbeat test); no conflict markers left.
- Code: _heartbeatTick() every 15s — detects dropped daemon-level connections (previously no listener existed) and triggers scheduleReconnect; pings via waitForPong(3000, conn); no-pong path closes the conn and reconnects; stopHeartbeat() on reconnect + window close prevents timer stacking. Sound.
2/3.
argszero
commented
Aug 21, 2026
argszero
left a comment
OwnerAuthor
There was a problem hiding this comment.
✅ LGTM — cycle
Re-review (head updated to 6f597ba — master merged in by a parallel cycle):
- Heartbeat implementation intact after merge:
startHeartbeat()/stopHeartbeat()/_heartbeatTick()— 15s ping on the daemon connection viawaitForPong(3000, conn); no-pong or dropped connection →stopHeartbeat()+scheduleReconnect()(existing exponential backoff, renderer status dot updated truthfully). waitForPongconn override targets the daemon connection specifically; heartbeat guarded againststopping; started onensureConnectedsuccess, stopped onscheduleReconnectentry andwindow-all-closed.- Merge brought in #911 (sidebar removal) — no conflict; both features coexist. Agent.md GUI count on merged head = 256 (127 renderer smoke) matches actual
npm test(256: 248 pass + 8 skipped). - Verified on merged head: GUI npm test 256, Python pytest 987 passed + 1 skipped, import OK. CI test + test-windows PASS (run 32449393467).
2/3.
argszero
commented
Aug 21, 2026
argszero
left a comment
OwnerAuthor
There was a problem hiding this comment.
✅ LGTM — cycle
3rd review (head 6f597ba, unchanged since 2/3):
- Fresh local verification on this branch: GUI npm test 256 total (248 pass / 0 fail / 8 skipped), Python pytest 987 passed + 1 skipped, import + CLI OK.
- Heartbeat implementation intact:
_heartbeatTick()every 15s pings the daemon connection viawaitForPong(3000, conn); dropped connection or no pong →stopHeartbeat()+scheduleReconnect()(exponential backoff, renderer status dot updated truthfully). Guarded againststopping; started onensureConnectedsuccess; stopped onscheduleReconnectentry andwindow-all-closed. - CI test + test-windows PASS (run 32449393467).
3/3 — merging.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes rant
2026-08-21T12:44:34(heartbeat item) — the GUI had no periodic liveness probe:daemon_client.jsonly sends one ping after connect ("不做轮询"), and the daemon connection'sdisconnectedevent was never listened to inmain.js(the 2026-08-21 11:41 incident: GUI killed the daemon, never reconnected for 36 minutes, stale green status dot).Changes
emrg/gui/main.jsstartHeartbeat()/stopHeartbeat()/_heartbeatTick(): every 15s, ping the daemon connection viawaitForPong(3000, conn); on no-pong or a dropped connection →scheduleReconnect()(existing exponential backoff, already pushesconnected:false/reconnectingstatus to the renderer so the status dot is truthful).waitForPong(timeoutMs, connOverride)— optional connection override so the heartbeat probes the daemon connection specifically (not the active session connection).ensureConnected()success; stopped whenscheduleReconnect()is entered (no double-trigger) and onwindow-all-closed.emrg/gui/test/renderer.smoke.test.js— source-level test asserting the heartbeat wiring exists (constants, functions, waitForPong reuse, scheduleReconnect hook, start/stop wiring).Agent.md— GUI test breakdown synced (renderer smoke 127→128, total 256→257).Verification
npm test— 257 tests, 249 pass, 8 skipped (live daemon owns fixed port), 0 failnode --check main.jsOKuv run pytest tests/— 987 passed + 1 skipped (incl. doc-count guards)python -m emrg --help+ import OK