From 14adbb61a4c25908917c627a7f072b31e4fc6b89 Mon Sep 17 00:00:00 2001 From: Frank Song Date: Tue, 14 Jul 2026 14:46:00 +0800 Subject: [PATCH] fix: pin agent update gateway restart profile --- api/agent_health.py | 10 ++-- api/gateway_restart.py | 31 ++++++++++-- api/updates.py | 10 ++-- tests/test_health_restart.py | 26 +++++++++- tests/test_update_banner_fixes.py | 79 +++++++++++++++++++++---------- 5 files changed, 119 insertions(+), 37 deletions(-) diff --git a/api/agent_health.py b/api/agent_health.py index 2c55a611..0aa20649 100644 --- a/api/agent_health.py +++ b/api/agent_health.py @@ -251,7 +251,7 @@ def _gateway_running_pid(gateway_status: Any, pid_path: Path | None) -> int | No return get_running_pid() -def get_active_profile_gateway_running_pid() -> int | None: +def get_active_profile_gateway_running_pid(profile: str | None = None) -> int | None: """Return the confirmed active-profile PID without state fallbacks. Update recovery needs process identity, not just a recent ``running`` state: @@ -260,10 +260,14 @@ def get_active_profile_gateway_running_pid() -> int | None: the default-root health path. Keep this fail-closed when status is unavailable. """ try: - from api.profiles import get_active_hermes_home + from api.profiles import get_active_hermes_home, get_hermes_home_for_profile gateway_status = _gateway_status_module() - gateway_pid_path = Path(get_active_hermes_home()) / _GATEWAY_PID_FILE + if profile is None: + gateway_home = get_active_hermes_home() + else: + gateway_home = get_hermes_home_for_profile(profile) + gateway_pid_path = Path(gateway_home) / _GATEWAY_PID_FILE return _gateway_running_pid(gateway_status, gateway_pid_path) except Exception: return None diff --git a/api/gateway_restart.py b/api/gateway_restart.py index 2cf54f08..d28cbed7 100644 --- a/api/gateway_restart.py +++ b/api/gateway_restart.py @@ -10,7 +10,13 @@ import sys import threading from pathlib import Path -from api.profiles import get_active_hermes_home +from api.profiles import ( + _PROFILE_ID_RE, + _is_root_profile, + get_active_hermes_home, + get_active_profile_name, + get_hermes_home_for_profile, +) logger = logging.getLogger(__name__) @@ -46,8 +52,23 @@ def _release_lock() -> None: pass +def _gateway_restart_profile_context(profile: str | None = None) -> tuple[Path, str]: + """Return the HERMES_HOME and CLI profile arg for a gateway restart.""" + if profile is None: + raw_profile = str(get_active_profile_name() or "default").strip() + active_home = Path(get_active_hermes_home()) + else: + raw_profile = str(profile or "default").strip() + active_home = Path(get_hermes_home_for_profile(raw_profile)) + + if not raw_profile or not _PROFILE_ID_RE.fullmatch(raw_profile) or _is_root_profile(raw_profile): + return active_home, "default" + return active_home, raw_profile + + def restart_active_profile_gateway( *, + profile: str | None = None, quick_timeout_seconds: float = 2.0, background_wait_seconds: float = 240.0, ) -> dict: @@ -66,18 +87,20 @@ def restart_active_profile_gateway( } try: - active_home = get_active_hermes_home() + active_home, cli_profile = _gateway_restart_profile_context(profile) env = os.environ.copy() env["HERMES_HOME"] = str(active_home) hermes_cmd = _resolve_hermes_command() + cmd = [hermes_cmd, "--profile", cli_profile, "gateway", "restart"] logger.info( - "Restarting gateway service via CLI command: %s gateway restart (HERMES_HOME=%s)", + "Restarting gateway service via CLI command: %s --profile %s gateway restart (HERMES_HOME=%s)", hermes_cmd, + cli_profile, active_home, ) proc = subprocess.Popen( - [hermes_cmd, "gateway", "restart"], + cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, diff --git a/api/updates.py b/api/updates.py index ed831d8a..e7258cb3 100644 --- a/api/updates.py +++ b/api/updates.py @@ -26,6 +26,7 @@ from urllib.parse import urlparse from api.agent_health import get_active_profile_gateway_running_pid from api.gateway_restart import restart_active_profile_gateway +from api.profiles import get_active_profile_name from api.config import REPO_ROOT, STREAMS, STREAMS_LOCK logger = logging.getLogger(__name__) @@ -1816,8 +1817,9 @@ def _ensure_gateway_restart_for_agent_update() -> tuple[bool, dict]: - ok is False when restart did not complete and callers must abort success. - restart_payload contains helper status fields for response shaping. """ - gateway_pid_before_restart = get_active_profile_gateway_running_pid() - restart_result = restart_active_profile_gateway() + target_profile = str(get_active_profile_name() or "default").strip() or "default" + gateway_pid_before_restart = get_active_profile_gateway_running_pid(profile=target_profile) + restart_result = restart_active_profile_gateway(profile=target_profile) status = str(restart_result.get("status") or "") if status in {"completed", "in_progress"}: return True, restart_result @@ -1829,7 +1831,7 @@ def _ensure_gateway_restart_for_agent_update() -> tuple[bool, dict]: # bounded delay so an already-applied Agent update is not reported as a # complete failure because of that transient process handoff. time.sleep(_AGENT_GATEWAY_RESTART_RETRY_DELAY_S) - retry_result = restart_active_profile_gateway() + retry_result = restart_active_profile_gateway(profile=target_profile) retry_status = str(retry_result.get("status") or "") if retry_status in {"completed", "in_progress"}: return True, { @@ -1848,7 +1850,7 @@ def _ensure_gateway_restart_for_agent_update() -> tuple[bool, dict]: # service. Only accept that recovery when the confirmed local PID changed; # a merely-alive old gateway has not loaded the updated Agent checkout. time.sleep(_AGENT_GATEWAY_RESTART_RETRY_DELAY_S) - gateway_pid_after_retry = get_active_profile_gateway_running_pid() + gateway_pid_after_retry = get_active_profile_gateway_running_pid(profile=target_profile) if ( gateway_pid_before_restart is not None and gateway_pid_after_retry is not None diff --git a/tests/test_health_restart.py b/tests/test_health_restart.py index bc112695..1c886de7 100644 --- a/tests/test_health_restart.py +++ b/tests/test_health_restart.py @@ -96,11 +96,35 @@ def test_restart_active_profile_gateway_success_uses_active_profile_home(monkeyp assert result["status"] == "completed" assert result["message"] == "Gateway service restarted successfully" - assert called["args"] == ["/mock/bin/hermes", "gateway", "restart"] + assert called["args"] == ["/mock/bin/hermes", "--profile", "default", "gateway", "restart"] assert called["env"]["HERMES_HOME"] == "/mock/hermes/home" assert gateway_restart._GATEWAY_RESTART_LOCK.locked() is False +def test_restart_active_profile_gateway_pins_explicit_default_profile(monkeypatch): + gateway_restart._GATEWAY_RESTART_LOCK = threading.Lock() + called = {} + + def fake_popen(args, stdout=None, stderr=None, text=True, env=None): + called["args"] = args + called["env"] = env + return MockPopen(args, stdout_text="ok", returncode=0, env=env) + + monkeypatch.setattr( + gateway_restart, + "get_hermes_home_for_profile", + lambda profile: "/mock/hermes/default" if profile == "default" else "/mock/hermes/profiles/work", + ) + monkeypatch.setattr(gateway_restart.shutil, "which", lambda cmd: "/mock/bin/hermes") + monkeypatch.setattr(gateway_restart.subprocess, "Popen", fake_popen) + + result = gateway_restart.restart_active_profile_gateway(profile="default") + + assert result["status"] == "completed" + assert called["args"] == ["/mock/bin/hermes", "--profile", "default", "gateway", "restart"] + assert called["env"]["HERMES_HOME"] == "/mock/hermes/default" + + def test_restart_active_profile_gateway_failure_preserves_empty_output_contract(monkeypatch): gateway_restart._GATEWAY_RESTART_LOCK = threading.Lock() diff --git a/tests/test_update_banner_fixes.py b/tests/test_update_banner_fixes.py index d8b7783d..52783d20 100644 --- a/tests/test_update_banner_fixes.py +++ b/tests/test_update_banner_fixes.py @@ -729,7 +729,7 @@ class TestSuccessfulUpdateReturnsRestartScheduled: monkeypatch.setattr(upd, '_schedule_restart', lambda delay=2.0: None) monkeypatch.setattr( 'api.updates.restart_active_profile_gateway', - lambda: {'status': 'completed', 'message': 'Gateway service restarted successfully'}, + lambda **kwargs: {'status': 'completed', 'message': 'Gateway service restarted successfully'}, ) result = upd.apply_update('agent') @@ -839,13 +839,13 @@ class TestAgentUpdateRequiresGatewayRestart: restart_calls = [] sleeps = [] - def fake_restart(): - restart_calls.append('called') + def fake_restart(*, profile=None): + restart_calls.append(profile) return next(restart_results) monkeypatch.setattr(upd, 'restart_active_profile_gateway', fake_restart) monkeypatch.setattr(upd.time, 'sleep', sleeps.append) - monkeypatch.setattr(upd, 'get_active_profile_gateway_running_pid', lambda: 101) + monkeypatch.setattr(upd, 'get_active_profile_gateway_running_pid', lambda *, profile=None: 101) ok, result = upd._ensure_gateway_restart_for_agent_update() @@ -853,7 +853,7 @@ class TestAgentUpdateRequiresGatewayRestart: assert result['status'] == 'completed' assert result['retry_attempted'] is True assert 'bad file descriptor' in result['initial_failure'] - assert restart_calls == ['called', 'called'] + assert restart_calls == ['default', 'default'] assert sleeps == [upd._AGENT_GATEWAY_RESTART_RETRY_DELAY_S] def test_agent_gateway_restart_retry_busy_stays_fail_closed(self, monkeypatch): @@ -866,12 +866,12 @@ class TestAgentUpdateRequiresGatewayRestart: sleeps = [] gateway_pid_calls = [] - monkeypatch.setattr(upd, 'restart_active_profile_gateway', lambda: next(restart_results)) + monkeypatch.setattr(upd, 'restart_active_profile_gateway', lambda **kwargs: next(restart_results)) monkeypatch.setattr(upd.time, 'sleep', sleeps.append) monkeypatch.setattr( upd, 'get_active_profile_gateway_running_pid', - lambda: gateway_pid_calls.append(101) or 101, + lambda *, profile=None: gateway_pid_calls.append(profile) or 101, ) ok, result = upd._ensure_gateway_restart_for_agent_update() @@ -881,7 +881,7 @@ class TestAgentUpdateRequiresGatewayRestart: assert result['retry_attempted'] is True assert 'first' in result['initial_failure'] assert sleeps == [upd._AGENT_GATEWAY_RESTART_RETRY_DELAY_S] - assert gateway_pid_calls == [101] + assert gateway_pid_calls == ['default'] def test_agent_gateway_restart_accepts_verified_process_replacement_after_retry_failure(self, monkeypatch): import api.updates as upd @@ -894,11 +894,11 @@ class TestAgentUpdateRequiresGatewayRestart: sleeps = [] gateway_pids = iter([101, 202]) - def fake_restart(): + def fake_restart(*, profile=None): timeline.append('restart') return next(restart_results) - def fake_gateway_pid(): + def fake_gateway_pid(*, profile=None): pid = next(gateway_pids) timeline.append(f'pid:{pid}') return pid @@ -931,13 +931,13 @@ class TestAgentUpdateRequiresGatewayRestart: restart_calls = [] sleeps = [] - def fake_restart(): - restart_calls.append('called') + def fake_restart(*, profile=None): + restart_calls.append(profile) return next(restart_results) monkeypatch.setattr(upd, 'restart_active_profile_gateway', fake_restart) monkeypatch.setattr(upd.time, 'sleep', sleeps.append) - monkeypatch.setattr(upd, 'get_active_profile_gateway_running_pid', lambda: 101) + monkeypatch.setattr(upd, 'get_active_profile_gateway_running_pid', lambda *, profile=None: 101) ok, result = upd._ensure_gateway_restart_for_agent_update() @@ -946,12 +946,41 @@ class TestAgentUpdateRequiresGatewayRestart: assert result['retry_attempted'] is True assert 'Restart failed: first' in result['message'] assert 'Restart failed: retry' in result['message'] - assert restart_calls == ['called', 'called'] + assert restart_calls == ['default', 'default'] assert sleeps == [ upd._AGENT_GATEWAY_RESTART_RETRY_DELAY_S, upd._AGENT_GATEWAY_RESTART_RETRY_DELAY_S, ] + def test_agent_gateway_restart_default_retry_cannot_use_sticky_named_profile(self, monkeypatch): + import api.updates as upd + + default_restart_results = iter([ + {'status': 'failed', 'message': 'Restart failed: default first'}, + {'status': 'failed', 'message': 'Restart failed: default retry'}, + ]) + restart_profiles = [] + + def fake_restart(*, profile=None): + effective_profile = profile or 'sticky-work' + restart_profiles.append(effective_profile) + if effective_profile == 'sticky-work': + return {'status': 'completed', 'message': 'wrong profile restarted'} + return next(default_restart_results) + + monkeypatch.setattr(upd, 'get_active_profile_name', lambda: 'default') + monkeypatch.setattr(upd, 'restart_active_profile_gateway', fake_restart) + monkeypatch.setattr(upd.time, 'sleep', lambda seconds: None) + monkeypatch.setattr(upd, 'get_active_profile_gateway_running_pid', lambda *, profile=None: 101) + + ok, result = upd._ensure_gateway_restart_for_agent_update() + + assert ok is False + assert restart_profiles == ['default', 'default'] + assert result['status'] == 'failed' + assert 'default first' in result['message'] + assert 'default retry' in result['message'] + def test_apply_update_agent_requires_gateway_restart(self, tmp_path, monkeypatch): import api.updates as upd @@ -973,8 +1002,8 @@ class TestAgentUpdateRequiresGatewayRestart: return 'Already up to date.', True return '', True - def fake_gateway_restart(): - gateway_restarts.append('called') + def fake_gateway_restart(*, profile=None): + gateway_restarts.append(profile) return {'status': 'completed', 'message': 'Gateway service restarted successfully'} monkeypatch.setattr(upd, '_run_git', fake_run) @@ -988,7 +1017,7 @@ class TestAgentUpdateRequiresGatewayRestart: assert result['target'] == 'agent' assert result['restart_scheduled'] is True assert result['gateway_restart'] == 'completed' - assert gateway_restarts == ['called'] + assert gateway_restarts == ['default'] def test_apply_update_agent_stash_conflict_success_invokes_gateway_restart(self, tmp_path, monkeypatch): import api.updates as upd @@ -1023,8 +1052,8 @@ class TestAgentUpdateRequiresGatewayRestart: return 'Updating', True return '', True - def fake_gateway_restart(): - gateway_restarts.append('called') + def fake_gateway_restart(*, profile=None): + gateway_restarts.append(profile) return {'status': 'in_progress', 'message': 'Gateway service restart initiated (in progress)'} monkeypatch.setattr(upd, '_run_git', fake_run) @@ -1039,7 +1068,7 @@ class TestAgentUpdateRequiresGatewayRestart: assert result['target'] == 'agent' assert result['restart_scheduled'] is True assert result['gateway_restart'] == 'in_progress' - assert gateway_restarts == ['called'] + assert gateway_restarts == ['default'] def test_apply_update_agent_without_gateway_restart_result_fails(self, tmp_path, monkeypatch): import api.updates as upd @@ -1066,8 +1095,8 @@ class TestAgentUpdateRequiresGatewayRestart: monkeypatch.setattr(upd, 'REPO_ROOT', tmp_path) monkeypatch.setattr(upd, '_AGENT_DIR', tmp_path) monkeypatch.setattr(upd, '_schedule_restart', lambda delay=2.0: (_ for _ in ()).throw(AssertionError('must not restart'))) - monkeypatch.setattr('api.updates.restart_active_profile_gateway', lambda: ( - restart_calls.append('called'), + monkeypatch.setattr('api.updates.restart_active_profile_gateway', lambda **kwargs: ( + restart_calls.append(kwargs.get('profile')), {'status': 'busy', 'message': 'Restart already in progress. Please wait a moment and try again.'}, )[1]) @@ -1077,7 +1106,7 @@ class TestAgentUpdateRequiresGatewayRestart: assert result['target'] == 'agent' assert result['gateway_restart'] == 'busy' assert 'hermes gateway restart' in result['message'] - assert restart_calls == ['called'] + assert restart_calls == ['default'] def test_apply_force_update_agent_uses_gateway_restart_status(self, tmp_path, monkeypatch): import api.updates as upd @@ -1101,7 +1130,7 @@ class TestAgentUpdateRequiresGatewayRestart: monkeypatch.setattr(upd, 'REPO_ROOT', tmp_path) monkeypatch.setattr(upd, '_AGENT_DIR', tmp_path) monkeypatch.setattr(upd, '_schedule_restart', lambda delay=2.0: None) - monkeypatch.setattr('api.updates.restart_active_profile_gateway', lambda: {'status': 'completed', 'message': 'Gateway service restarted successfully'}) + monkeypatch.setattr('api.updates.restart_active_profile_gateway', lambda **kwargs: {'status': 'completed', 'message': 'Gateway service restarted successfully'}) result = upd.apply_force_update('agent') assert result['ok'] is True @@ -1131,7 +1160,7 @@ class TestAgentUpdateRequiresGatewayRestart: monkeypatch.setattr(upd, '_schedule_restart', lambda delay=2.0: (_ for _ in ()).throw(AssertionError('must not restart'))) monkeypatch.setattr( 'api.updates.restart_active_profile_gateway', - lambda: {'status': 'busy', 'message': 'Restart already in progress. Please wait a moment and try again.'}, + lambda **kwargs: {'status': 'busy', 'message': 'Restart already in progress. Please wait a moment and try again.'}, ) result = upd.apply_force_update('agent')