emrg: GUI daemon liveness — fixed-port TCP probe, never delete healthy daemon's token (rant 2026-08-21T15:26:42) - #918
Conversation
…, never delete healthy daemon's token (rant 2026-08-21T15:26:42)
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 1209. Port-based liveness verified: positive (port open → keep token) and negative (port closed → delete+respawn) states covered by mocked net.connect tests; pid-file presence no longer influences the judgment. Test counts unchanged (GUI 249+8, pytest 994+1), CI test + test-windows PASS.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle
2nd review (head 77f4de5, MERGEABLE/CLEAN, CI test + test-windows PASS):
- Fresh local verification on this branch: pytest 994 passed + 1 skipped (995 collected), GUI npm test 257 (249 pass / 0 fail / 8 skipped), import + CLI OK.
- Root cause confirmed: _daemonProcessAlive() read ~/.emrg/emrgd.pid + process.kill(pid,0); missing/stale pid → false → G43 stale-token path unlinked a LIVE daemon's token (emrgd.log showed repeated 're-asserted (external deletion?)' + TUI FileNotFoundError on startup).
- Fix: async fixed-port TCP probe (net.connect 127.0.0.1:56031, mirroring connect.py is_server_running_sync): port open = alive = never delete token; port closed → only then delete+respawn. emrgd.pid demoted to pure diagnostics (HOME_PID_FILE constant removed).
- Tests: 5 G43 liveness cases rewritten with mocked net.connect covering port-open (keep) / port-closed (delete+respawn) / pid-present-but-port-closed (dead) + adapted skipStart/spawn-throttle tests; net.connect restored in finally.
- Matches the port-is-ground-truth model from rant 2026-08-19 (fixed-port admission) and the earlier #861/#884 fixed-port work.
2/3.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 1210. Fresh review: fixed-port TCP probe mirrors connect.py is_server_running_sync semantics (port = ground truth); call site awaits properly; tests cover positive (port open → keep token) + negative (port closed → delete+respawn) + pid-file-independent states. HOME_PID_FILE removed cleanly, no leftover references. CI test + test-windows PASS, head unchanged.
Problem (rant 2026-08-21T15:26:42)
The GUI's daemon-liveness check could delete a healthy daemon's auth token:
EMRGD_PORT= 56031) since rant 2026-08-19T08:05:21 — the port is the ground truth, andemrg/connect.pyalready probes it (is_server_running_sync).DaemonClient._daemonProcessAlive()still read~/.emrg/emrgd.pid+process.kill(pid, 0). Whenemrgd.pidis missing/stale (stop_all cleanup, crash, external deletion), it returnedfalse→ misjudged "daemon dead" → the G43 stale-token path ranfs.unlinkSync(TOKEN_FILE())on a live daemon's token._port_keepalive_loop, daemon.py) — emrgd.log showed repeated"emrgd.token was missing — re-asserted (external deletion?)"(14:08/14:09/14:20/15:07/15:16), and TUI startup kept failing withFileNotFoundError: ~/.emrg/emrgd.token.Fix
_daemonProcessAlive()→ fixed-port TCP probe (net.connect to127.0.0.1:56031, mirroringconnect.py is_server_running_syncsemantics): port open = daemon alive = never delete the token; port closed → only then allow the stale-token delete + respawn path. Now async; call site awaits it.emrgd.piddemoted to pure diagnostics — the daemon still writes/deletes it, but no GUI code reads it for liveness anymore (HOME_PID_FILEconstant removed).connect.py/daemon_manager.py) already uses the port probe — no change needed there.Tests
Rewrote the G43 liveness tests (5 cases) to mock
net.connect(port-open → keep token / port-closed → delete + respawn / pid file present but port closed → still dead), plus adapted the stale-token spawn-throttle and skipStart tests to the new semantics. Test count unchanged: GUI 249 pass + 8 skip; pytest 994 pass + 1 skip.Verification (per rant)
After this fix, the GUI no longer deletes
emrgd.tokenwhile the daemon's port is alive — the daemon's periodic"re-asserted (external deletion?)"log entries and TUIFileNotFoundErroron startup should stop.