emrg: connect — never delete a healthy daemon's token in cleanup_server (guard on fixed-port probe) - #1044
Conversation
8520fb4 to
18101deComparehow2how2how2-arch
commented
Aug 27, 2026
I tested this PR and it works as described: Validation performed:
Observations (non-blocking):
|
18101de to
ac271f0Compare
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 194103 (1/3)
Fresh code review of head ac271f0:
cleanup_server()now probes the fixed port viais_server_running_sync()and returns early when a daemon is listening — the token is only deleted when the port probe confirms no daemon accepts connections. Correct ground truth (fixed port = single-instance truth; stale token from a crashed daemon is harmless on next atomic rewrite).- Tests cover both states: probe False → token removed; probe True → token survives (
test_keeps_token_when_daemon_listening). Existing cleanup tests updated to mock the probe. - Doc-count consistent (Agent.md Python 1126 at head), CI dual-green (33064747323), MERGEABLE/CLEAN.
… protection) (#1046) Completes the upgrade_prompt structure-guard trilogy (#1045 guarded build steps + macOS re-seal chain; this adds the data-safety chain): - step 4 must back up BOTH app locations ({{ install_dir }}/emrg-gui/EMRG.app install source + ~/Applications/EMRG.app run copy) into {{ backup_dir }} - backup must precede the copy — replacing the working app without a backup leaves no rollback path - restore-from-backup must be instructed in both the step-5 verify failure path and the '## 4. Backup & rollback' section - the '## 4. Backup & rollback' section must contain backup + restore semantics Negative-state verified: removing the backup instruction makes the guard fail, restoring it passes. Python tests 1125 -> 1126, Agent.md synced. NOTE for reviewers: #1044 (connect token guard) also documents 1126 — whomever merges second must rebase and bump the count to 1127. Co-authored-by: argszero <argszero@gmail.com>
…er (guard on fixed-port probe)
…er (guard on fixed-port probe) Rebased onto master (Agent.md Python 1126→1127 after #1046 merged).
ac271f0 to
e83ec09Compare
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 195734 (1/3 on new head e83ec09)
Rebased onto master after #1046 merged: Agent.md Python 1126→1127 (master now collects 1126; this PR adds one more test → 1127, verified locally: collect 1127 == Agent.md 1127, connect tests 11/11). Code unchanged from the previously reviewed ac271f0: cleanup_server() fixed-port probe guard + dual-state tests. CI dual-green on the new head (33069839941), MERGEABLE/CLEAN.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 201611 (3/3)
Fresh re-verification on head e83ec09: connect tests 11/11, full suite collects 1127 == Agent.md 1127. CI dual-green (33069839941), MERGEABLE/CLEAN. Third consecutive ✅ (cycles 195734 → 200934 → 201611, no ❌) — merge condition met.
Uh oh!
There was an error while loading. Please reload this page.
Problem
cleanup_server()unconditionally deletes the daemon auth token file, and it's called before every daemon spawn attempt (start_daemoninemrg/client/daemon_manager.py,_start_daemon_backgroundinemrg/__main__.py). When a client reconnect triggers a spawn while a healthy daemon is actually alive on the fixed port, this deletes the healthy daemon's token. The doomed spawn then hitsEADDRINUSEand suicides before writing a token back (the daemon only writes the token after a successful bind). The result is a token-missing window where all new connections fail.Reported from a live incident (2026-08-27 14:42-14:44): a TUI reconnect caused 4 doomed spawn attempts (pids 32807/32825/32855/32872), each deleting the token before the spawn; the old TUI self-healed via the daemon's 60s keepalive, but a fresh TUI reading the token file got
FileNotFoundError: emrgd.token.Fix
Guard
cleanup_server()inemrg/connect.pyso it only deletes the token when the fixed-port probe confirms no daemon is accepting connections. The fixed port is the single-instance ground truth, so a mis-triggered spawn cleanup can never remove the token of a live daemon. A fresh daemon start rewrites the token atomically (_assert_token_file→atomic_write_bytes), so a stale token from a crashed daemon is harmless on the next successful start.All legitimate deletion contexts still work because in each the daemon is truly dead/not-listening:
daemon_manager.py:215, after PID-death wait) — probe returns Falseensure_connectedwhennot is_running()(daemon_manager.py:235) — probe returns Falseemrg server stop(__main__.py:212) — probe returns False after the daemon stopsTests
TestCleanupServercases to mockis_server_running_sync→ False (no daemon → deletion proceeds).test_keeps_token_when_daemon_listening— the token must survive when a daemon is listening on the fixed port (regression guard for this exact bug).Agent.md(1123 → 1124).Verification
pytest tests/ -q: 1123 passed, 1 skippedpython -c "from emrg.client.app import run_client"— OKpython -m emrg --help— OK