Skip to content

emrg: stop_all stops clients (GUI/TUI) before daemon - #820

Merged
argszero merged 1 commit into
masterfrom
feature/stop-order-clients-first
Aug 17, 2026
Merged

emrg: stop_all stops clients (GUI/TUI) before daemon#820
argszero merged 1 commit into
masterfrom
feature/stop-order-clients-first

Conversation

@argszero

Copy link
Copy Markdown
Owner

Fix host rant 2026-08-17T14:15:33 — stop order was reversed.

Problem: stop_all() stopped the daemon FIRST, then GUI/TUI. Both clients auto-spawn the daemon when they detect it missing (GUI daemon_client.js ensureConnected → spawnDaemon; TUI client/app.py reconnect). So stopping the daemon first with clients alive made them immediately re-spawn it — the installer still hit locked files under ~/.emrg/install.

Fix: reorder to clients-first, daemon-last:

  1. stop_gui()
  2. stop_tui()
  3. stop_daemon()
  4. [Windows] stop_bundled_git()
  5. verify()

With the daemon stopped last, no client remains to bring it back, verify() sees the true final state, and the installer overwrite is safe.

Tests: +2 order tests in test_stop_all.py (clients before daemon on Windows incl. bundled-git; POSIX skips bundled-git). Doc sync: Agent.md Python count 869→871. Full suite: 870 passed + 1 skipped.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle 576: reordered stop_all to clients-first/daemon-last; order tests added (daemon index after both clients); CI test+test-windows green. Matches rant 2026-08-17T14:15:33.

@pm25coder

Copy link
Copy Markdown
Contributor

I tested this PR end-to-end (Windows, branch feature/stop-order-clients-first):

  • tests/test_stop_all.py: 20 passed, including the new TestStopAllOrder tests — the order assertions (stop_guistop_tuistop_daemonstop_bundled_git on win32; daemon strictly after both clients) hold, and the POSIX branch correctly skips stop_bundled_git.
  • Full suite: 811 passed, 60 skipped (871 collected) — matches the updated Agent.md count (869 → 871).
  • Import check (from emrg.client.app import run_client) and emrg --help CLI both OK.

The reordering rationale is sound and matches the auto-spawn behavior: both the GUI (daemon_client.js spawn path) and the TUI (daemon_manager.py start_daemon) will bring the daemon back if it disappears while they are alive, so stopping the daemon first would leave the installer hitting locked files. Stopping clients first and the daemon last gives verify() a true final state.

One non-blocking observation: stop_daemon()'s ws-protocol shutdown path relies on the daemon still being reachable; since the daemon is now stopped last, this path is exercised in the most favorable state (clients gone, no one re-spawning), which is exactly right. No issues found from my side.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle 577: re-verified diff (Agent.md count 871 + _stop_all reorder + order tests); CI test+test-windows green, mergeable CLEAN; pm25coder Windows end-to-end pass confirms. 2/3.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle 578: 3rd consecutive approval (576/577/578). Re-verified: order fix correct, tests green, CI pass, mergeable CLEAN. Merging.

@argszero
argszero merged commit f9edaff into master Aug 17, 2026
2 checks passed
@argszero
argszero deleted the feature/stop-order-clients-first branch August 17, 2026 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants