emrg: GUI restart-to-apply — full-chain stop via _stop_all --skip-gui + app.relaunch (rant 2026-08-21T12:44:34) - #915
Merged
Conversation
… + app.relaunch (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 restart-to-apply fix (rant 2026-08-21T12:44:34):
_stop_all.py --skip-gui: step plan omits the GUI stop step while keeping TUI → daemon; POSIX verify excludes the GUI's own pids (EMRG.app/AppImage) from residuals — the GUI is the caller and must survive to relaunch. Windows verify skips the EMRG.exe category the same way. Pure stdlib preserved (module stays importable by the standalone installer runtime).main.jsemrg:restartDaemon: spawnpython -m emrg._stop_all --skip-gui→ await exit 0 →app.relaunch()+app.exit(0). No more bareshutdownthat the TUI would immediately undo. stderr tail surfaced on failure for diagnostics.- Tests: +5 Python (step-plan POSIX/Windows skip + default, verify GUI-pid exclusion both with and without the flag, existing verify stubs updated for the new kwarg), +1 GUI source-level test (asserts
_stop_all --skip-guispawn + relaunch + no bare shutdown). - Verified locally: Python pytest 992 passed + 1 skipped (993 collected, matches Agent.md), GUI npm test 257 (249+8, matches Agent.md),
node --check main.jsOK, import + CLI OK.
1/3.
pm25coder
commented
Aug 21, 2026
Collaborator
Independent test report (Contributor, pm25coder): |
…start-skip-gui # Conflicts: # Agent.md
argszero
commented
Aug 21, 2026
argszero
left a comment
OwnerAuthor
There was a problem hiding this comment.
✅ LGTM — cycle
2nd review (head 0f5753a — conflict-resolved merge of master, CI test + test-windows PASS on the resolved tree):
- Fresh local verification on the merged tree: pytest 993 passed + 1 skipped (994 collected — doc-count guard satisfied), import + CLI OK, GUI 257 tests (249 pass / 0 fail / 8 skipped).
- Conflict resolution: master had moved (bd3797e = #913 upgrade-banner); Agent.md Python count resolved to 994 (993 from this branch + 1 from #913's pong test), GUI 257/128 renderer smoke; no conflict markers left.
- Code (from earlier review, unchanged in substance): _stop_all.py --skip-gui omits the GUI stop step while keeping TUI → daemon stop; GUI restart button runs the full-chain stop then app.relaunch — sound restart-to-apply flow.
2/3.
argszero
commented
Aug 21, 2026
argszero
left a comment
OwnerAuthor
There was a problem hiding this comment.
✅ LGTM — cycle
3rd review (head 0f5753a — conflict-resolved tree with #913 merged in; CI test + test-windows PASS on run 32451470122):
- Fresh local verification: pytest 993 passed + 1 skipped (994 collected, matches Agent.md 994), GUI npm test 257 (249 pass + 8 skipped), import + CLI OK.
- Master has since advanced (#914 v0.2.62 bump) — no overlap with this branch's files (version bump vs _stop_all/main.js/tests), merge remains CLEAN.
- Code (unchanged in substance from 2/3):
_stop_all --skip-guiomits the GUI stop step and excludes the GUI from the residual verify (it is the caller and must survive to relaunch); GUI restart button spawns the full-chain stop, awaits exit 0, thenapp.relaunch()+app.exit(0).
3/3 — merging.
Uh oh!
There was an error while loading. Please reload this page.
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.
GUI "Restart to apply" — full-chain stop via
_stop_all --skip-gui+app.relaunchRant 2026-08-21T12:44:34: the GUI "Restart to apply" button previously only
sent
shutdownto the daemon. The daemon then got pulled back by the TUIclient's auto-reconnect/spawn (
emrg/client/app.py:378-394), while the GUIitself never reconnected — observed:
emrg-gui.log2026-08-21 11:41, GUIkilled the daemon, no reconnect for 36 minutes, status dot stuck at the old
connected:truesnapshot.Fix
emrg/_stop_all.py— new--skip-guimode (pure stdlib, same module):python -m emrg._stop_all --skip-guiruns the full-chain stop(GUI → TUI → daemon, clients first so nothing respawns the daemon) but
skips the GUI stop step and excludes the GUI from the residual
verify — the GUI is the caller and intentionally stays alive to
relaunch itself. Without the skip,
stop_gui(taskkill/IM EMRG.exe/ps-scan
EMRG.app) would kill the GUI main process and the relaunchwould never execute; verify() would also report the GUI as a residual
and exit 1.
_is_gui_cmdline()helper extracted frommatch_cmdlinefor thePOSIX verify filter;
_iter_ps_lines()parses ps output into(pid, cmdline)pairs.emrg/gui/main.js—emrg:restartDaemonrewritten:python -m emrg._stop_all --skip-gui(TUI + daemon torn down)app.relaunch()+app.exit(0)→ new GUI process starts →ensureDaemonspawns a fresh daemon from the newly installed codeTUI clients are killed outright (no reconnect loop); the old "TUI receives
restart command and auto-exits" design is avoided entirely.
Tests / verification
tests/test_stop_all.py: +5 — step plan omits GUI when--skip-gui(POSIX + Windows), default plan keeps GUI, POSIX verify excludes GUI pid
(TUI residual still reported), verify without the flag still flags GUI.
Existing
verifystubs updated to accept the kwarg.renderer.smoke.test.js: +1 source-level test asserting the new handlerspawns
_stop_all --skip-gui+app.relaunch()and no longer sends abare
shutdown.257 (249 pass + 8 skipped),
node --check main.jsOK, import + CLI OK.