fix: auto-restart dashboard after update when argv is recoverable - #41508
fix: auto-restart dashboard after update when argv is recoverable#41508Variable85 wants to merge 2 commits into
Conversation
Preserve recoverable dashboard argv before stopping stale dashboard processes, then restart those dashboards after update while retaining the manual fallback when argv recovery or restart fails. Harden stale dashboard detection against shell-wrapper false positives and cover quoted Windows paths.
56b75f7 to
93d997e
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tackling the update/dashboard mismatch; current main still stops stale processes and leaves a manual restart hint (hermes_cli/main.py:6094-6210), so the underlying problem is valid.
Problems
- The new matcher only recognizes
dashboard(hermes_cli/main.py:5649in this PR). Current main also intentionally reapshermes servebackends (hermes_cli/main.py:5881-5886), so this would regress staleservecleanup. - The systemd path ignores
user.slice(hermes_cli/main.py:5799; see the explicit expectation intests/hermes_cli/test_update_stale_dashboard.py:507-510). It then falls back to detached spawning, which can race a user service configured withRestart=always. Existing gateway service support has explicitsystemctl --userhandling inhermes_cli/gateway.py:1999-2002. - Restart output is sent to
Path.home() / '.hermes'athermes_cli/main.py:5857, bypassing the active profile'sHERMES_HOME.
Suggested changes
- Preserve
servematching and test it. - Handle user- and system-scoped units via the scope-aware service helpers instead of treating user units as unmanaged.
- Use
get_hermes_home()for the restart log path.
Automated hermes-sweeper review.
| # `python -m hermes_cli.main dashboard` and `hermes dashboard` both | ||
| # place the subcommand after this token. Permit global flags before | ||
| # the subcommand (e.g. `hermes --profile taste dashboard`). | ||
| if "dashboard" in tail: |
There was a problem hiding this comment.
This drops current-main support for hermes serve: main currently treats hermes serve, hermes_cli.main serve, and hermes_cli/main.py serve as stale backends too (hermes_cli/main.py:5881-5886). Preserve that command family here and add a regression test before replacing the existing matcher.
| # cgroup v2: 0::/system.slice/hermes-dashboard.service | ||
| # cgroup v1/systemd: 1:name=systemd:/system.slice/foo.service | ||
| path = line.rstrip("\n").split(":", 2)[-1] | ||
| if not path.startswith("/system.slice/"): |
There was a problem hiding this comment.
A per-user systemd service appears under user.slice, so this branch makes it fall through to detached Popen rather than service restart. That can race a Restart=always unit. Please detect user scope and use systemctl --user; hermes_cli/gateway.py:1999-2002 already centralizes its environment setup.
|
|
||
| restarted: list[list[str]] = [] | ||
| failed: list[tuple[list[str], str]] = [] | ||
| log_path = Path.home() / ".hermes" / "logs" / "gui.log" |
There was a problem hiding this comment.
Use get_hermes_home() / 'logs' / 'gui.log' here. Path.home() / '.hermes' sends logs from an active named profile into the default profile, violating profile isolation.
…update Capture each manually-started dashboard/serve process's argv before the stale-process kill (/proc/<pid>/cmdline on Linux, ps -o command= on macOS), then respawn it detached after the update — headless (--no-open) with output to logs/dashboard-restart.log under the active profile's HERMES_HOME. Supervised PIDs keep their systemd-unit restart; --stop stays a plain stop. Salvaged from PR #41508 with scope fixes: serve matching preserved, profile- aware log path, restart only on the update path (restart_managed=True).
…update Capture each manually-started dashboard/serve process's argv before the stale-process kill (/proc/<pid>/cmdline on Linux, ps -o command= on macOS), then respawn it detached after the update — headless (--no-open) with output to logs/dashboard-restart.log under the active profile's HERMES_HOME. Supervised PIDs keep their systemd-unit restart; --stop stays a plain stop. Salvaged from PR #41508 with scope fixes: serve matching preserved, profile- aware log path, restart only on the update path (restart_managed=True).
|
Merged via PR #72192 — your argv-capture/respawn work was applied onto current main with your authorship preserved in git history (8e1fb9e), with the review-noted adjustments folded in (serve matching kept, profile-aware log path via |
…update Capture each manually-started dashboard/serve process's argv before the stale-process kill (/proc/<pid>/cmdline on Linux, ps -o command= on macOS), then respawn it detached after the update — headless (--no-open) with output to logs/dashboard-restart.log under the active profile's HERMES_HOME. Supervised PIDs keep their systemd-unit restart; --stop stays a plain stop. Salvaged from PR NousResearch#41508 with scope fixes: serve matching preserved, profile- aware log path, restart only on the update path (restart_managed=True).
Summary
hermes update--no-openfor headless/SSH safetysystemctl restart <unit>instead of spawning an unmanaged clone that races the service for the portTest Plan
git diff --checkpython -m py_compile hermes_cli/main.py tests/hermes_cli/test_update_stale_dashboard.pyuv run --with pytest --with pytest-xdist --with pyyaml python -m pytest tests/hermes_cli/test_update_stale_dashboard.py tests/hermes_cli/test_dashboard_lifecycle_flags.py -q -o 'addopts='→44 passed in 0.88shermes-dashboard.service:/proc/<MainPID>/cgroupresolves tohermes-dashboard.service; helper returnshermes-dashboard.servicecurl http://127.0.0.1:9119/loginreturned dashboard HTMLNotes