fix(profile): detect service-managed gateway status - #20488
Conversation
austinpickett
left a comment
There was a problem hiding this comment.
Please use PULL_REQUEST_TEMPLATE.md
There was a problem hiding this comment.
Pull request overview
Fixes #20254 by teaching profile status reporting to recognize service-managed (systemd/launchd) gateways. The existing per-profile gateway.pid lookup is kept as a fast path; when no pid file is found, _check_gateway_running temporarily scopes HERMES_HOME to the profile being inspected and consults get_gateway_runtime_snapshot() so service-managed gateways are correctly reported by hermes profile list/show.
Changes:
- Add a service-manager fallback in
hermes_cli/profiles._check_gateway_runningwhen the profile'sgateway.pidis absent. - Temporarily scope
HERMES_HOMEto the target profile around the snapshot call soget_service_name()resolves the right systemd unit. - Add regression test (
test_gateway_running_check_uses_runtime_snapshot_fallback) and update the existing tests to mock the new snapshot dependency.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| hermes_cli/profiles.py | Adds the service-manager runtime-snapshot fallback inside _check_gateway_running, with HERMES_HOME save/restore. |
| tests/hermes_cli/test_profiles.py | Mocks get_gateway_runtime_snapshot in the existing tests and adds a regression test verifying the no-pid-file service fallback path and HERMES_HOME restoration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| from hermes_cli.gateway import get_gateway_runtime_snapshot | ||
| previous_home = os.environ.get("HERMES_HOME") | ||
| os.environ["HERMES_HOME"] = str(profile_dir) | ||
| try: | ||
| snapshot = get_gateway_runtime_snapshot() | ||
| return bool(getattr(snapshot, "service_running", False)) | ||
| finally: | ||
| if previous_home is None: | ||
| os.environ.pop("HERMES_HOME", None) | ||
| else: | ||
| os.environ["HERMES_HOME"] = previous_home | ||
| except Exception: | ||
| return False |
| previous_home = os.environ.get("HERMES_HOME") | ||
| os.environ["HERMES_HOME"] = str(profile_dir) | ||
| try: | ||
| snapshot = get_gateway_runtime_snapshot() | ||
| return bool(getattr(snapshot, "service_running", False)) | ||
| finally: | ||
| if previous_home is None: | ||
| os.environ.pop("HERMES_HOME", None) | ||
| else: | ||
| os.environ["HERMES_HOME"] = previous_home |
| try: | ||
| from gateway.status import get_running_pid | ||
| return get_running_pid(profile_dir / "gateway.pid", cleanup_stale=False) is not None | ||
| if ( | ||
| get_running_pid(profile_dir / "gateway.pid", cleanup_stale=False) | ||
| is not None | ||
| ): | ||
| return True | ||
| except Exception: | ||
| pass | ||
| try: | ||
| from hermes_cli.gateway import get_gateway_runtime_snapshot | ||
| previous_home = os.environ.get("HERMES_HOME") | ||
| os.environ["HERMES_HOME"] = str(profile_dir) | ||
| try: | ||
| snapshot = get_gateway_runtime_snapshot() | ||
| return bool(getattr(snapshot, "service_running", False)) | ||
| finally: | ||
| if previous_home is None: | ||
| os.environ.pop("HERMES_HOME", None) | ||
| else: | ||
| os.environ["HERMES_HOME"] = previous_home | ||
| except Exception: | ||
| return False |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting the service-managed no-PID path; current main still has a related systemd gap.
Problems
hermes_cli/profiles.py:368temporarily writesos.environ["HERMES_HOME"]. That environment is process-global, so another dashboard/request thread can observe the displayed profile and use the wrong home.hermes_constants.py:23-35providesset_hermes_home_override()specifically for per-task scoping without mutating the environment.- The new test validates that global mutation rather than proving the profile-specific service probe is isolated.
Suggested changes
- Scope
get_gateway_runtime_snapshot()withset_hermes_home_override(str(profile_dir))and reset its token infinally. - Add coverage that the snapshot resolves the target profile while an independent context retains its original home.
Automated hermes-sweeper review.
| from hermes_cli.gateway import get_gateway_runtime_snapshot | ||
| previous_home = os.environ.get("HERMES_HOME") | ||
| os.environ["HERMES_HOME"] = str(profile_dir) | ||
| try: |
There was a problem hiding this comment.
os.environ is shared by every thread. Please use hermes_constants.set_hermes_home_override(str(profile_dir)) and reset its token in finally; this scopes the snapshot to this operation without exposing another profile to concurrent dashboard/request work.
Summary
Closes #20254
Verification
scripts/run_tests.sh tests/hermes_cli/test_profiles.py -k gateway_running_checkscripts/run_tests.sh tests/hermes_cli/test_profiles.pyNon-goals