emrg: TUI daemon spawn throttle — complete the anti-storm fix - #594
Conversation
…026-08-09T13:16:36) PRs #592 (CREATE_NO_WINDOW + GUI spawn throttle + scheduler backoff) and #593 (port-file self-heal + G43 PID guard) contain the Windows v0.2.15 storm fix, but the TUI client had the same storm pattern the GUI had: app.py _reconnect() loops every 1s calling ensure_connected() → with a down daemon each iteration calls start_daemon() → spawns a NEW daemon process every second. On Windows each spawn was a cmd-window source (pre-#592); even windowless it is process churn + PID-lock exit races post-#592. This completes rant acceptance item ① ("启动 GUI/TUI 后零 cmd 窗口弹出"): - daemon_manager.start_daemon: spawn throttle — max 3 attempts per connect lifecycle, then raise with a clear 'run emrg server manually' message instead of spawning forever; counter resets on successful connect in ensure_connected (mirrors GUI daemon_client.js MAX_SPAWN_ATTEMPTS). - app.py _reconnect: on throttle, surface a one-time system message + status hint so the host knows to start the daemon manually (recovery path intact: host starts daemon → is_running True → connect succeeds → counter resets). Tests: +2 (start_daemon throttles after 3 attempts, no 4th spawn; counter resets on success). 641→643 py, doc counts synced (#511).
|
Tested this PR on Windows (win32, Python 3.13.4): |
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 2026-08-09T17:07 (Round 188)
Reviewed the full diff on the PR branch:
daemon_manager.start_daemonspawn throttle (_MAX_SPAWN_ATTEMPTS=3) with reset inensure_connectedon success mirrors the GUI's daemon_client.js semantics exactly — consistent anti-storm design across both clients.app.py _reconnect()surfaces the throttle as a one-time system message + status hint (daemon down — run 'emrg server') so the host knows to intervene; recovery path intact (host starts daemon → connect succeeds → counter resets → loop returns).- Tests cover both states: 3 spawns then no 4th (
await_countasserted), counter reset on successful connect; module-level autouse fixture prevents cross-test leakage.
Verified on the PR branch: uv run pytest tests/ → 643 passed; import + --help OK. GUI tests untouched (96). Doc counts synced to the branch base (#511).
… 2026-08-09T13:16:36) After the spawn throttle exhausts, scheduleReconnect keeps retrying with backoff capped at 60s — each retry hit the throttle and re-sent daemon_stopped status, so the renderer appended a duplicate 'run emrg server' system message every minute forever. Add daemonStoppedNotified: the warning is sent once per connect lifecycle (reset on successful connect), symmetric with the TUI _throttle_warned guard (PR #594).
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 2026-08-09T17:22 (Round 191)
Second independent review of the TUI spawn throttle (re-verified fresh this cycle):
- daemon_manager _MAX_SPAWN_ATTEMPTS=3 + _spawn_attempts — mirrors the GUI daemon_client.js semantics exactly (module-level counter, reset on successful connect in ensure_connected). Consistent anti-storm design across both clients.
- app.py _reconnect() surfaces the throttle as a one-time system message + status hint (daemon down — run 'emrg server'); recovery path intact (host starts daemon → connect succeeds → counter resets → loop returns).
- Message-match verified: raise "daemon failed to start after N attempts" ↔ _reconnect catches "failed to start after" — exact alignment.
- Tests cover both states: 3 spawns then no 4th (await_count asserted), counter reset on success; module-level autouse fixture prevents cross-test leakage.
Fresh verification on the PR branch: uv run pytest tests/ → 643 passed; import + --help OK. GUI tests untouched (96). Doc counts synced to the branch base (#511).
…egression hotfix) (#592) * emrg: fix Windows cmd-window storm + daemon spawn throttle (v0.2.15 regression hotfix) Rant 2026-08-09T13:16:36 (emergency, highest priority): Windows v0.2.15 host observed continuous cmd popups (had to reboot) + daemon startup failure leaving GUI/scheduler unable to connect. Root causes: 1. Zero CREATE_NO_WINDOW anywhere in the codebase — every subprocess spawn (git/gh/bash tool/scheduler/daemon spawn) popped a console window on Windows. 2. GUI reconnect loop respawned emrgd.cmd every ~5s forever; each spawn raced the previous daemon's startup (G43 stale-port unlink deleted a healthy daemon's port file → its scheduler logged 'cannot connect' 93 times while the GUI kept spawning). 3. Scheduler had no connect-failure backoff. Fixes: - NEW emrg/_win.py: win32_no_window_kwargs() = {creationflags: CREATE_NO_WINDOW} on Windows, {} elsewhere. Splatted into all 34 subprocess call sites (bash_tool, scheduler x12, daemon x5, git_utils, daemon_manager, installer, __main__ x3, client/app x9). - GUI daemon_client.js: spawn throttle (max 3 attempts per connect lifecycle, then throw with emrgd.log tail instead of respawning); reset counter on successful auth. Both spawn-timeout errors now surface the real emrgd.log tail (readLogTail). - GUI main.js: reconnect exponential backoff 1s→2s→4s→…cap 60s; daemon_stopped status surfaces the real failure to the renderer (zh/en i18n) instead of infinite 5s respawns. - scheduler.py: connect-failure exponential backoff max(30s, interval*2^n) capped at 10 min — no more per-tick retry storm while the daemon is down. Tests: +6 Python (win32_no_window_kwargs POSIX/Windows/splat; backoff zero/exp/cap/floor; 641→647), +2 GUI (spawn throttle + counter reset; 96→98). Docs synced (#511 guard). macOS/Linux unaffected (win32 branch is a no-op empty dict). * emrg: GUI daemon_stopped warning one-time per connect lifecycle (rant 2026-08-09T13:16:36) After the spawn throttle exhausts, scheduleReconnect keeps retrying with backoff capped at 60s — each retry hit the throttle and re-sent daemon_stopped status, so the renderer appended a duplicate 'run emrg server' system message every minute forever. Add daemonStoppedNotified: the warning is sent once per connect lifecycle (reset on successful connect), symmetric with the TUI _throttle_warned guard (PR #594).
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 2026-08-09T17:30 (Round 192)
Reviewed the branch diff against current master (post-#593 merge):
_MAX_SPAWN_ATTEMPTS=3guard indaemon_manager.start_daemonis the client-side total gate for the v0.2.15 cmd-storm root cause — it caps respawns per connection lifecycle and surfaces a clearRuntimeErrorwhen exhausted; counter resets onensure_connectedsuccess (verified the reset only happens after a successful connect).app.py _reconnectcatches the throttle error once (_throttle_warned) and shows a one-time system message + 'daemon down — run emrg server' status hint — correct user-facing degradation instead of silent 1s respawn loop.- Positive/negative states covered by tests: 3 spawns then no 4th (await_count asserted), counter reset on successful connect, autouse fixture prevents cross-test leakage.
tests/test_daemon_manager.py29 passed locally. - Matches GUI
daemon_client.jsspawn-throttle semantics (#593 branch) — consistent anti-storm design across both clients.
…T13:16:36) (#596) Carries the emergency Windows v0.2.15 regression hotfix (host observed cmd-window storm + daemon startup failure, had to reboot): - #592: CREATE_NO_WINDOW across all 34 subprocess sites (emrg/_win.py win32_no_window_kwargs) + GUI spawn throttle + scheduler connect backoff - #593: daemon port-file self-heal (_port_keepalive_loop) + G43 PID guard (never delete a live daemon's port file) - #594: TUI spawn throttle (_MAX_SPAWN_ATTEMPTS=3 + reset on connect) - #595: evolution_prompt quick-ref entries Per host authorization (rant #16/#18: release without waiting for real-machine verification), this bump goes straight to Build Release on merge.
Summary
Completes the Windows v0.2.15 storm fix (rant 2026-08-09T13:16:36) by closing the TUI client's respawn storm path — the last remaining piece of rant acceptance item ① ("启动 GUI/TUI 后零 cmd 窗口弹出").
PR #592 (CREATE_NO_WINDOW + GUI spawn throttle + scheduler backoff) and PR #593 (port-file self-heal + G43 PID guard) cover the daemon, scheduler, and GUI. But
emrg/client/app.py_reconnect()had the same pattern the GUI had:With a down daemon, every iteration spawns a new daemon process every 1 second — on Windows each spawn was a cmd-window source (pre-#592); even windowless it is process churn + PID-lock exit races (post-#592).
Changes
emrg/client/daemon_manager.pyMAX_SPAWN_ATTEMPTS): max 3 spawn attempts per connect lifecycle, thenstart_daemon()raises a cleardaemon failed to start after 3 attempts — please run 'emrg server' manuallyerror instead of spawning forever.ensure_connected()after a successful connect (same semantics as the GUI's auth-success reset).emrg/client/app.py_reconnect(): on throttle, surface a one-time system message + status hint (daemon down — run 'emrg server') so the host knows to intervene. Recovery path stays intact: host starts the daemon →is_running()true → connect succeeds → counter resets → loop returns.Tests
tests/test_daemon_manager.py):start_daemonspawns exactly 3 times then throws the throttle error (4th call does not spawn — asserted viaawait_count); counter resets to 0 after a successfulensure_connected. Module-level autouse fixture prevents cross-test state leakage.Verification
uv run pytest tests/: 643 passed--helpOK