diff --git a/agent/turn_iteration_prep.py b/agent/turn_iteration_prep.py index f549d6233d..8786dc7b62 100644 --- a/agent/turn_iteration_prep.py +++ b/agent/turn_iteration_prep.py @@ -22,6 +22,21 @@ from agent.turn_context_compaction import _reanchor logger = logging.getLogger("agent.conversation_loop") + +def _anchors_current_turn(messages: Any, idx: Any, user_message: Any) -> bool: + """True when ``messages[idx]`` is this turn's user row (verbatim, or its user-originated view).""" + if not isinstance(idx, int) or not 0 <= idx < len(messages): + return False + msg = messages[idx] + if not (isinstance(msg, dict) and msg.get("role") == "user"): + return False + if msg.get("content") == user_message: + return True + from agent.context_compressor import user_originated_turn_view + + view = user_originated_turn_view(msg) + return view is not None and view.get("content") == user_message + ITERATION_BUDGET_WARNING_TEMPLATE = ( "[SYSTEM NOTICE — iteration budget checkpoint] You have used {used} of {maximum} " "iterations. Checkpoint durable progress now, then continue the task; do not stop " @@ -205,6 +220,18 @@ def prepare_iteration( current_turn_user_idx, _reanchored_idx, agent.session_id or "-", ) current_turn_user_idx = _reanchored_idx + # Mid-turn compaction (post-tool gate, overflow restart, recovery) rebuilds ``messages`` without + # handing back a new index. A stale index splits the request's replay prefix inside this turn's + # tool rows: prefix canonicalization then drops the assistant tool_call whose result fell past the + # split, the orphaned result is sanitized away, and the model silently loses tool output that + # state.db still holds. A valid index always lands on this turn's user row; re-anchor otherwise. + if user_message is not None and not _anchors_current_turn(messages, current_turn_user_idx, user_message): + _reanchored_idx = _reanchor(agent, messages, user_message) + request_logger.info( + "Re-anchored stale current_turn_user_idx %s -> %s (session=%s)", + current_turn_user_idx, _reanchored_idx, agent.session_id or "-", + ) + current_turn_user_idx = _reanchored_idx return IterationPrep( action="fallthrough", messages=messages, request_logger=request_logger, current_turn_user_idx=current_turn_user_idx, diff --git a/agent/turn_preflight.py b/agent/turn_preflight.py index 3a71db1916..7361f048f1 100644 --- a/agent/turn_preflight.py +++ b/agent/turn_preflight.py @@ -22,7 +22,7 @@ from agent.conversation_compression import ( from agent.turn_context import _review_fork_first_request_pending from agent.turn_context_compaction import ( _apply_grown_window, _blocked_compress_reason, _clear_overflow_warn, _refund_api_call, - _reset_retry_state_after_compaction, + _reanchor, _reset_retry_state_after_compaction, ) logger = logging.getLogger("agent.conversation_loop") @@ -238,13 +238,14 @@ class PostToolCompressionVerdict: compression_attempts: int final_response: Any turn_exit_reason: Any + current_turn_user_idx: int def compress_after_tool_results( agent: Any, *, messages: List[Dict[str, Any]], system_message: Any, user_message: Any, active_system_prompt: Any, conversation_history: Any, compression_attempts: int, max_compression_attempts: int, effective_task_id: Any, final_response: Any, - turn_exit_reason: Any, + turn_exit_reason: Any, current_turn_user_idx: int, ) -> PostToolCompressionVerdict: """Post-tool-call compression decision. Pressure comes from API-reported ``prompt_tokens`` (a tight lower bound; thinking models inflate completion tokens), @@ -263,6 +264,7 @@ def compress_after_tool_results( end_turn=end_turn, messages=messages, active_system_prompt=active_system_prompt, conversation_history=conversation_history, compression_attempts=compression_attempts, final_response=final_response, turn_exit_reason=turn_exit_reason, + current_turn_user_idx=current_turn_user_idx, ) _compressor = agent.context_compressor @@ -355,6 +357,7 @@ def compress_after_tool_results( final_response = _HANDOFF_SKIP_FINAL_RESPONSE turn_exit_reason = "compaction_handoff_not_actionable" return _verdict(True) + current_turn_user_idx = _reanchor(agent, messages, user_message) elif agent.compression_enabled: # Over threshold but compression blocked (cooldown/anti-thrash): deduped # warning so context can't silently overflow. ``attempts_spent`` names the diff --git a/agent/turn_tool_round.py b/agent/turn_tool_round.py index 62e5f306c2..10211a8895 100644 --- a/agent/turn_tool_round.py +++ b/agent/turn_tool_round.py @@ -39,6 +39,7 @@ class ToolRoundVerdict: failed: Any _turn_exit_reason: Any truncated_tool_call_retries: Any + current_turn_user_idx: Any result: Optional[Dict[str, Any]] = None @@ -47,7 +48,7 @@ def run_tool_round( conversation_history: Any, api_call_count: Any, effective_task_id: Any, user_message: Any, system_message: Any, active_system_prompt: Any, compression_attempts: Any, max_compression_attempts: Any, final_response: Any, failed: Any, _turn_exit_reason: Any, - truncated_tool_call_retries: Any, + truncated_tool_call_retries: Any, current_turn_user_idx: Any, ) -> ToolRoundVerdict: """Execute one tool round in the exact original order. Persist-before-execute is a durability invariant: resume must see the executed block if a destructive tool restarts @@ -60,7 +61,8 @@ def run_tool_round( action=action, messages=messages, conversation_history=conversation_history, active_system_prompt=active_system_prompt, compression_attempts=compression_attempts, final_response=final_response, failed=failed, _turn_exit_reason=_turn_exit_reason, - truncated_tool_call_retries=truncated_tool_call_retries, result=result, + truncated_tool_call_retries=truncated_tool_call_retries, + current_turn_user_idx=current_turn_user_idx, result=result, ) if not agent.quiet_mode: @@ -191,6 +193,7 @@ def run_tool_round( compression_attempts=compression_attempts, max_compression_attempts=max_compression_attempts, effective_task_id=effective_task_id, final_response=final_response, turn_exit_reason=_turn_exit_reason, + current_turn_user_idx=current_turn_user_idx, ) messages = _ptc.messages active_system_prompt = _ptc.active_system_prompt @@ -198,6 +201,7 @@ def run_tool_round( compression_attempts = _ptc.compression_attempts final_response = _ptc.final_response _turn_exit_reason = _ptc.turn_exit_reason + current_turn_user_idx = _ptc.current_turn_user_idx if _ptc.end_turn: return _verdict("break") diff --git a/hermes_cli/AGENTS.md b/hermes_cli/AGENTS.md index a5d6d4d35d..6927947e2b 100644 --- a/hermes_cli/AGENTS.md +++ b/hermes_cli/AGENTS.md @@ -152,10 +152,15 @@ it guards. `plan → snapshot → apply → restart-per-kind → verify → repo none of them; without the graft the swap deletes them). Post-swap, the Desktop rebuild decision also trusts the build stamp under HERMES_HOME, so an install that already lost its artifacts in an earlier update is rebuilt instead of "forgotten" (#90495). -- **Restart-per-kind**: systemd and launchd restarts are FLEET-WIDE (every `hermes-gateway*` unit / - `ai.hermes.gateway*` LaunchAgent), drain-first (SIGUSR1), with per-unit/per-label failure - isolation. Restarting only the invoking profile's service leaves siblings on stale `sys.modules` - until they crash — the largest dupe-PR cluster in the repo's history came from that bug. +- **Restart-per-kind**: systemd and launchd restarts are FLEET-WIDE within the updating install (every + `hermes-gateway*` unit / `ai.hermes.gateway*` LaunchAgent whose home is the updating root or one of its + `profiles/`), drain-first (SIGUSR1), with per-unit/per-label failure isolation. Restarting only the + invoking profile's service leaves siblings on stale `sys.modules` until they crash — the largest dupe-PR + cluster in the repo's history came from that bug. The fleet is bounded by HOME, not by namespace: + `hermes_cli/update_fleet_scope.py` judges every unit/label/process by the home it actually runs on + (live environ, unit `Environment=`, plist `HERMES_HOME`), and a runtime of another `HERMES_HOME` on the + same account — a sibling install, the real `hermes-gateway.service` seen from a scratch home — is named and + left alone, never restarted (#93349). - **Verify**: gateways stamp `code_sha`/`code_version` into `gateway_state.json` on every runtime-status write (`gateway/status.py`); the updater compares each live gateway against the fresh checkout and prints a fleet version matrix. A provably-stale gateway fails the update diff --git a/hermes_cli/profile_cmd.py b/hermes_cli/profile_cmd.py index 0eccafdde6..4289b036c0 100644 --- a/hermes_cli/profile_cmd.py +++ b/hermes_cli/profile_cmd.py @@ -215,6 +215,10 @@ def _profile_create(args): print(f"Full copy from {source_label} (excluding session history, cron jobs, backups, and snapshots).") else: print(f"Cloned config, .env, SOUL.md, and skills from {source_label}.") + from hermes_cli.profile_memory_config import cloned_memory_provider + memory_provider = cloned_memory_provider(profile_dir) + if memory_provider: + print(f"Cloned memory provider config ({memory_provider}) too.") if sync_imports: print(f"Import sources carried over — `hermes -p {name} import-agent --sync` " "keeps pulling the same Claude Code / Codex trees.") diff --git a/hermes_cli/profile_memory_config.py b/hermes_cli/profile_memory_config.py new file mode 100644 index 0000000000..36bb099b09 --- /dev/null +++ b/hermes_cli/profile_memory_config.py @@ -0,0 +1,71 @@ +"""Carry the ACTIVE memory provider's own config into a ``--clone`` (#120115). + +``--clone`` copies ``config.yaml`` — and with it ``memory.provider: hindsight`` — but the +provider keeps its settings outside config.yaml, so the clone booted with the provider +selected and silently unavailable. Providers store per-home config by convention (the same +convention ``hermes_cli.web_routers.memory_providers`` reads): a ``//`` +directory (hindsight) or a flat ``/.json`` (mem0, honcho, supermemory). Copying +by convention keeps this free of plugin imports: the provider may live in the catalog, not in +tree, so a hook the plugin must implement could not fix the reported case. +""" + +import contextlib +import os +import re +import shutil +from pathlib import Path +from typing import Optional + +# A provider name is a bare directory/file stem; anything else (path separators, ``..``, spaces) +# would let a hand-edited config.yaml aim the copy outside the source profile. +_PROVIDER_NAME_RE = re.compile(r"^[A-Za-z0-9_.-]+$") + + +def active_memory_provider(config: Optional[dict]) -> Optional[str]: + """The external ``memory.provider`` named in a parsed config.yaml, or None for the built-in + store or an unsafe name.""" + from agent.memory_provider import is_core_memory_provider + + memory = (config or {}).get("memory") + name = memory.get("provider") if isinstance(memory, dict) else None + if not isinstance(name, str) or is_core_memory_provider(name): + return None + name = name.strip() + if name in {".", ".."} or not _PROVIDER_NAME_RE.match(name): + return None + return name + + +def clone_memory_provider_config(source_dir: Path, profile_dir: Path, provider: Optional[str]) -> bool: + """Copy ``/`` and/or ``.json`` from *source_dir* into *profile_dir* when + present. Files land owner-only like ``.env``: they can hold an API key. Returns True when + anything was copied.""" + if not provider: + return False + copied = False + src_dir = source_dir / provider + if src_dir.is_dir(): + shutil.copytree(src_dir, profile_dir / provider, dirs_exist_ok=True) + for root, _dirs, files in os.walk(profile_dir / provider): + for filename in files: + with contextlib.suppress(OSError): + os.chmod(os.path.join(root, filename), 0o600) + copied = True + src_file = source_dir / f"{provider}.json" + if src_file.is_file(): + dst = profile_dir / f"{provider}.json" + shutil.copy2(src_file, dst) + with contextlib.suppress(OSError): + os.chmod(str(dst), 0o600) + copied = True + return copied + + +def cloned_memory_provider(profile_dir: Path) -> Optional[str]: + """Name of the external provider whose config *profile_dir* now carries, for the CLI notice.""" + from hermes_cli.profiles import _load_yaml_dict + + provider = active_memory_provider(_load_yaml_dict(profile_dir / "config.yaml")) + if provider and ((profile_dir / provider).is_dir() or (profile_dir / f"{provider}.json").is_file()): + return provider + return None diff --git a/hermes_cli/profiles.py b/hermes_cli/profiles.py index 4cf39ffb0f..2ab11f1ef4 100644 --- a/hermes_cli/profiles.py +++ b/hermes_cli/profiles.py @@ -1242,6 +1242,9 @@ def _bootstrap_profile_dir(profile_dir: Path, source_dir: Optional[Path], _copytree_keep_junctions(source_skills, profile_dir / "skills", _non_exportable_entries, dirs_exist_ok=True) for relpath in _CLONE_SUBDIR_FILES: _clone_file(source_dir, profile_dir, relpath) + from hermes_cli.profile_memory_config import active_memory_provider, clone_memory_provider_config + clone_memory_provider_config(source_dir, profile_dir, + active_memory_provider(_load_yaml_dict(source_dir / "config.yaml"))) if sync_imports: from hermes_cli.agent_import_sync import SYNC_MANIFEST_NAME # lazy: keeps yaml/utils off the hot startup path _clone_file(source_dir, profile_dir, SYNC_MANIFEST_NAME) diff --git a/hermes_cli/update_cmd_fleet.py b/hermes_cli/update_cmd_fleet.py index b327d612a6..82d4144fe0 100644 --- a/hermes_cli/update_cmd_fleet.py +++ b/hermes_cli/update_cmd_fleet.py @@ -523,6 +523,77 @@ def _needs_sudo(scope: str) -> bool: ) +def _unit_main_pid(scope_cmd: list, svc_name: str) -> int: + """Live ``MainPID`` of a unit; ``0`` when inactive, unprivileged or unreadable. + + Property reads need no manage-units privileges, and an unreadable PID is never collapsed: + identity that cannot be proved keeps its own restart. + """ + try: + result = _systemctl(list(scope_cmd) + ["show", svc_name, "--property=MainPID", "--value"], timeout=10) + except (OSError, subprocess.TimeoutExpired): + return 0 + if getattr(result, "returncode", 1) != 0: + return 0 + try: + return int((getattr(result, "stdout", "") or "").strip() or 0) + except ValueError: + return 0 + + +def _restart_systemd_gateway_units_best_effort(failed: list, listings) -> None: + """Restart every hermes-gateway/serve unit ONCE PER LIVE HOST PROCESS. + + One host runs one multiplexing gateway, so leftover per-profile units + (``hermes-gateway-.service``) all point at the SAME live ``MainPID``; restarting + each in turn restarts the host gateway N times — a self-inflicted N-fold outage triggered + by one update. Units that share a live main PID are collapsed to one representative and the + others are named as LEGACY units to migrate, never silently dropped. + """ + from hermes_cli.update_host_obligation import collapse_units_to_host_processes + + answered = set() + targets: dict[str, tuple[str, list, str]] = {} # "/" -> (scope, scope_cmd, unit) + for scope, scope_cmd, result in listings: + answered.add(scope) + if result.returncode != 0: + failed.append(f"systemd-{scope} (listing failed)") + continue + _for_each_systemd_gateway_unit( + result.stdout, + process_unit=lambda svc_name, _scope=scope, _cmd=scope_cmd: targets.setdefault( + f"{_scope}/{svc_name}", (_scope, _cmd, svc_name)), + on_unit_timeout=lambda svc_name, exc: failed.append(svc_name), + ) + + keys = list(targets) + covered: dict[str, str] = {} + if len(keys) > 1: + # Only worth a `systemctl show` round when several units could be one process. + keys, covered = collapse_units_to_host_processes( + keys, lambda key: _unit_main_pid(targets[key][1], targets[key][2])) + for unit_key, owner_key in covered.items(): + print( + f" • {unit_key} is a legacy per-profile unit sharing one host gateway process with " + f"{owner_key}; restarting it again would restart that process twice. Fold the units " + "together with: hermes gateway migrate" + ) + + for key in keys: + scope, scope_cmd, svc_name = targets[key] + if not _systemd_unit_owned_by_update(scope_cmd, svc_name): + continue + manage_cmd = list(scope_cmd) + ["--no-ask-password"] + if _needs_sudo(scope): + manage_cmd = ["sudo", "-n"] + manage_cmd + try: + result = _systemctl_reset_and_restart(manage_cmd, svc_name, scope_cmd=scope_cmd) + if result.returncode != 0 or not _wait_for_service_active(scope_cmd, svc_name): + failed.append(svc_name) + except subprocess.TimeoutExpired: + failed.append(svc_name) + # A timeout or missing executable is not an empty scope. + failed.extend(f"systemd-{scope} (listing unavailable)" for scope, _ in _SYSTEMD_SCOPES if scope not in answered) def _live_fleet_current_rows() -> list[dict] | None: @@ -652,6 +723,34 @@ def _systemctl_reset_and_restart(manage_cmd: list, svc_name: str, *, scope_cmd: return _systemctl(manage_cmd + ["restart", svc_name], timeout=timeout) +def _systemd_unit_owned_by_update(scope_cmd: list, svc_name: str) -> bool: + """Gate a unit restart on the unit's home being one this update owns (#93349). + + ``hermes-gateway*`` is an account-wide namespace: a second install's ``hermes update`` used + to drain and restart the account's real ``hermes-gateway.service`` because the unit was + listed, not because it ran the updated code. Foreign or unreadable ownership prints a notice + and leaves the unit alone; it is not a failed restart. + """ + from hermes_cli.update_fleet_scope import describe_skipped_runtime, systemd_unit_hermes_home, home_in_update_scope + home = systemd_unit_hermes_home(scope_cmd, svc_name) + if home is not None and home_in_update_scope(home): + return True + print(describe_skipped_runtime("systemd unit", svc_name, home)) + return False + + +def _scoped_manual_gateway_pids(pids, *, keep=(), quiet: bool = False) -> list[int]: + """*pids* whose live home this update owns (plus *keep*, PIDs already mapped to this + install's profile PID files); every other gateway process is named and left running.""" + from hermes_cli.update_fleet_scope import describe_skipped_runtime, partition_gateway_pids_by_scope + keep = set(keep) + owned, foreign = partition_gateway_pids_by_scope([pid for pid in pids if pid not in keep]) + if not quiet: + for pid, home in foreign: + print(describe_skipped_runtime("gateway process", f"PID {pid}", home)) + return [pid for pid in pids if pid in keep or pid in owned] + + def _is_hermes_gateway_unit(unit: str) -> bool: """Exact base unit or hyphenated profile family only: ``startswith("hermes-serve")`` would accept ``hermes-server.service``.""" @@ -838,9 +937,15 @@ def _restart_macos_launchd_gateways( legacy_labels = legacy_launchd_labels_for_install(exclude=set(derived_labels) | {current_label}) if legacy_labels: print(f" ↻ legacy-labelled units of this install join the restart: {', '.join(legacy_labels)}") + from hermes_cli.update_fleet_scope import describe_skipped_runtime, launchd_label_foreign_home for label in derived_labels + legacy_labels: if label == current_label: continue + # Labels are account-global: root B's default profile derives the same bare label root A + # installed. A plist pinning a foreign HERMES_HOME is another install's job (#93349). + if (foreign_home := launchd_label_foreign_home(label)) is not None: + print(describe_skipped_runtime("launchd job", label, foreign_home)) + continue try: # Locate = liveness + domain in one probe; kickstart and fresh-PID checks # reuse that domain so a sibling is never probed in one and restarted in another. @@ -892,7 +997,7 @@ def _surviving_gateway_pids_after_failed_restart(): """ try: from hermes_cli.gateway import find_gateway_pids - return list(find_gateway_pids(all_profiles=True)) + return _scoped_manual_gateway_pids(find_gateway_pids(all_profiles=True), quiet=True) except Exception as exc: # pragma: no cover - defensive logger.debug("Could not probe for surviving gateways after update: %s", exc) return None @@ -1120,6 +1225,8 @@ def _restart_one_systemd_gateway_unit( check = _systemctl(scope_cmd + ["is-active", svc_name], timeout=5) if check.stdout.strip() != "active": return + if not _systemd_unit_owned_by_update(scope_cmd, svc_name): + return _repair_unit_without_fatal_exit_park(svc_name, scope) # None ⇒ no non-interactive privilege path; avoid manage-units verbs @@ -1368,6 +1475,9 @@ def _restart_manual_gateways(out: _GatewayRestartOutcome, _drain_budget) -> None for proc in find_profile_gateway_processes(exclude_pids=service_pids) if proc.pid in manual_pids } + # ``all_profiles`` is host-wide: a sibling install's gateway matches too. Only this update's + # homes are stopped; the profile-mapped PIDs come from this install's own PID files (#93349). + manual_pids = _scoped_manual_gateway_pids(manual_pids, keep=profile_processes) # Profile gateways we couldn't arm a relaunch for must NOT keep running stale: # the unmapped sweep below stops them and lists them under "Restart manually". # These must NOT be left running: their modules are the pre-update ones and every lazy import from here @@ -1583,7 +1693,7 @@ def _restart_gateway_fleet_after_update(_pre_update_plan, gateway_mode: bool): # Snapshot before any stop/drain so an empty survivor probe reads as "stopped # and never came back", not "nothing was running"; None fails closed. try: - out.pre_restart_gateway_pids = list(find_gateway_pids(all_profiles=True)) + out.pre_restart_gateway_pids = _scoped_manual_gateway_pids(find_gateway_pids(all_profiles=True), quiet=True) except Exception: out.pre_restart_gateway_pids = None diff --git a/hermes_cli/update_fleet_scope.py b/hermes_cli/update_fleet_scope.py new file mode 100644 index 0000000000..6c74c5d3e2 --- /dev/null +++ b/hermes_cli/update_fleet_scope.py @@ -0,0 +1,148 @@ +"""Home scoping for ``hermes update``'s fleet restart (#93349). + +The restart phase enumerates ``hermes-gateway*``/``hermes-serve*`` units, ``ai.hermes.gateway*`` +LaunchAgents and every ``gateway run`` process on the host. Those are HOST-wide namespaces: a +second Hermes install (another ``HERMES_HOME`` root under the same account, its own checkout and +venv) shares them, and the update used to restart that install's gateway too — including the +account's real ``hermes-gateway.service`` when a scratch home ran ``hermes update``. + +The fleet an update owns is the set of homes its plan inventories: the updating root plus every +``/profiles/``. Ownership is judged from what a runtime actually runs on — the live +process environment (``HERMES_HOME``/``HOME``, via ``_hermes_home_for_pid``), the unit's declared +``Environment=`` or the plist's pinned ``HERMES_HOME`` — never from a unit label or an argv +substring. Unknown ownership is left alone: a restart we cannot prove is ours is somebody else's +outage. +""" + +from __future__ import annotations + +import shlex +from contextlib import suppress +from pathlib import Path + + +def _resolved(path) -> Path | None: + try: + return Path(str(path)).expanduser().resolve() + except (OSError, RuntimeError, ValueError): + return None + + +def update_scope_homes() -> set[Path]: + """Resolved homes the running update owns: the invoking home, the install root and its profiles.""" + homes: set[Path] = set() + with suppress(Exception): + from hermes_constants import get_hermes_home + if (home := _resolved(get_hermes_home())) is not None: + homes.add(home) + with suppress(Exception): + from hermes_cli.update_receipt import _profile_homes + for _profile, home in _profile_homes(): + if (resolved := _resolved(home)) is not None: + homes.add(resolved) + return homes + + +def home_in_update_scope(home, scope: set[Path] | None = None) -> bool: + """True when *home* (a path or string) is one of the homes this update owns.""" + if not home: + return False + resolved = _resolved(home) + if resolved is None: + return False + return resolved in (update_scope_homes() if scope is None else scope) + + +def gateway_pid_in_update_scope(pid: int, scope: set[Path] | None = None) -> bool | None: + """Does gateway *pid* run on a home this update owns? ``None`` when its home cannot be read.""" + from hermes_cli.dashboard_procs import _hermes_home_for_pid + try: + home = _hermes_home_for_pid(pid) + except Exception: + return None + if home is None: + return None + return home_in_update_scope(home, scope) + + +def partition_gateway_pids_by_scope(pids, scope: set[Path] | None = None) -> tuple[list[int], list[tuple[int, str | None]]]: + """``(owned, foreign)`` split of *pids*; ``foreign`` pairs each PID with its home (None = unreadable).""" + scope = update_scope_homes() if scope is None else scope + owned: list[int] = [] + foreign: list[tuple[int, str | None]] = [] + for pid in pids: + verdict = gateway_pid_in_update_scope(pid, scope) + if verdict: + owned.append(pid) + else: + home = None + if verdict is False: + with suppress(Exception): + from hermes_cli.dashboard_procs import _hermes_home_for_pid + home = _hermes_home_for_pid(pid) + foreign.append((pid, home)) + return owned, foreign + + +def systemd_unit_hermes_home(scope_cmd: list, svc_name: str) -> str | None: + """Home the systemd unit *svc_name* runs on: its live MainPID's environment first, then the + unit's declared ``Environment=HERMES_HOME``; for a user-scope unit that declares none, the + user's own default home. ``None`` when nothing readable names a home.""" + from hermes_cli.update_cmd_fleet import _systemctl, _unit_main_pid + + pid = _unit_main_pid(scope_cmd, svc_name) + if pid > 0: + with suppress(Exception): + from hermes_cli.dashboard_procs import _hermes_home_for_pid + if (home := _hermes_home_for_pid(pid)) is not None: + return home + try: + shown = _systemctl(list(scope_cmd) + ["show", svc_name, "--property=Environment", "--value"], timeout=10) + except Exception: + return None + if getattr(shown, "returncode", 1) != 0: + return None + env_line = (getattr(shown, "stdout", "") or "").strip() + try: + tokens = shlex.split(env_line) + except ValueError: + tokens = env_line.split() + for token in tokens: + key, sep, value = token.partition("=") + if sep and key == "HERMES_HOME" and value.strip(): + return value.strip() + if "--user" in scope_cmd: + return str(Path.home() / ".hermes") + return None + + +def systemd_unit_in_update_scope(scope_cmd: list, svc_name: str, scope: set[Path] | None = None) -> bool | None: + """Does unit *svc_name* belong to this update? ``None`` = ownership unreadable (leave it alone).""" + home = systemd_unit_hermes_home(scope_cmd, svc_name) + if home is None: + return None + return home_in_update_scope(home, scope) + + +def launchd_label_foreign_home(label: str, scope: set[Path] | None = None) -> str | None: + """The HERMES_HOME a derived launchd label's installed plist pins when that home is NOT one of + this update's — labels are account-global, so root B's default profile derives the same bare + ``ai.hermes.gateway`` root A installed. ``None`` = ours, or no/unreadable plist (the locate step + decides whether a job exists; only a proven foreign home is refused).""" + import plistlib + with suppress(Exception): + from hermes_cli.gateway import get_launchd_plist_path + plist_path = get_launchd_plist_path().with_name(f"{label}.plist") + if not plist_path.exists(): + return None + data = plistlib.loads(plist_path.read_bytes()) + pinned = str(data["EnvironmentVariables"]["HERMES_HOME"]) + return None if home_in_update_scope(pinned, scope) else pinned + return None + + +def describe_skipped_runtime(kind: str, name: str, home: str | None) -> str: + """One notice line for a runtime the update leaves alone (foreign home or unreadable ownership).""" + if home is None: + return f" ↷ {name}: {kind} whose Hermes home could not be read — left alone (not restarted)" + return f" ↷ {name}: {kind} of another Hermes home ({home}) — left alone (not restarted)" diff --git a/tests-js/install-known-failures.test.ts b/tests-js/install-known-failures.test.ts index acc081e54c..faa5527001 100644 --- a/tests-js/install-known-failures.test.ts +++ b/tests-js/install-known-failures.test.ts @@ -11,8 +11,7 @@ const classifier = path.resolve(import.meta.dirname, '../tests/install/e2e-asset const lockedLog = [ 'error: failed to remove file `C:/install/venv/Lib/site-packages/../../Scripts/hermes.exe`: Access is denied. (os error 5)', - 'File "C:/install/venv/Scripts/hermes.exe/__main__.py", line 10, in ', - "subprocess.CalledProcessError: Command '['uv', 'pip', 'install', '-e', '.', '--quiet']' returned non-zero exit status 2.", + "⚠ Git update failed: Command '['uv', 'pip', 'install', '-e', '.', '--quiet']' returned non-zero exit status 2.", ].join('\n') const base = { diff --git a/tests/agent/test_mid_turn_compaction_turn_boundary.py b/tests/agent/test_mid_turn_compaction_turn_boundary.py new file mode 100644 index 0000000000..29be782222 --- /dev/null +++ b/tests/agent/test_mid_turn_compaction_turn_boundary.py @@ -0,0 +1,154 @@ +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from agent.turn_preflight import compress_after_tool_results +from run_agent import AIAgent + + +def test_post_tool_compression_reanchors_the_active_user_boundary(monkeypatch): + compressed = [ + {"role": "user", "content": "compressed history"}, + {"role": "user", "content": "current ask"}, + {"role": "assistant", "tool_calls": [{"id": "2"}]}, + {"role": "tool", "content": "fresh result", "tool_call_id": "2"}, + ] + + class Compressor: + last_prompt_tokens = 100 + threshold_tokens = 50 + + @staticmethod + def should_compress(_tokens): + return True + + agent = SimpleNamespace( + context_compressor=Compressor(), + compression_enabled=True, + _clear_context_overflow_warn=lambda: None, + _safe_print=lambda *_args: None, + _compress_context=lambda *_args, **_kwargs: (compressed, "system"), + _persist_user_message_idx=4, + ) + monkeypatch.setattr( + "agent.turn_preflight.conversation_history_after_compression", + lambda _agent, _messages, _history: [], + ) + monkeypatch.setattr( + "agent.conversation_loop._should_skip_model_call_for_reference_handoff", + lambda _messages, _user_message: False, + ) + + verdict = compress_after_tool_results( + agent, + messages=[{"role": "user", "content": "current ask"}], + system_message="system", + user_message="current ask", + active_system_prompt="system", + conversation_history=[], + compression_attempts=0, + max_compression_attempts=1, + effective_task_id="task", + final_response="", + turn_exit_reason=None, + current_turn_user_idx=0, + ) + + assert verdict.messages is compressed + assert verdict.current_turn_user_idx == 1 + assert agent._persist_user_message_idx == 1 + + +def _response(*, tool: bool): + tool_calls = [SimpleNamespace( + id="call_1", type="function", + function=SimpleNamespace(name="web_search", arguments='{"query": "x"}'), + )] if tool else None + message = SimpleNamespace( + content=None if tool else "done", reasoning_content=None, reasoning=None, tool_calls=tool_calls, + ) + return SimpleNamespace( + choices=[SimpleNamespace(message=message, finish_reason="tool_calls" if tool else "stop")], + model="test/model", usage=None, + ) + + +def test_pre_api_compression_mid_turn_keeps_this_turns_tool_pair_on_the_wire(): + """Pre-API compression after a tool round shrinks ``messages`` from the front; the next request + must still carry this turn's assistant tool_call and its result (state.db holds both).""" + tool_def = {"type": "function", "function": { + "name": "web_search", "description": "s", + "parameters": {"type": "object", "properties": {"query": {"type": "string"}}}, + }} + with ( + patch("model_tools.get_tool_definitions", return_value=[tool_def]), + patch("model_tools.check_toolset_requirements", return_value={}), + patch("agent.process_bootstrap.OpenAI"), + patch("agent.model_metadata.get_model_context_length", return_value=256_000), + patch("agent.context_compressor.get_model_context_length", return_value=256_000), + ): + agent = AIAgent( + api_key="test-key-1234567890", base_url="https://openrouter.ai/api/v1", model="test/model", + quiet_mode=True, skip_context_files=True, skip_memory=True, max_iterations=6, + ) + agent.client = MagicMock() + # No usage: the post-tool gate has no real count, so the pre-API gate owns the compaction. + agent.client.chat.completions.create.side_effect = [_response(tool=True), _response(tool=False)] + agent._cached_system_prompt = "You are helpful." + agent._use_prompt_caching = False + agent._disable_streaming = True + agent.tool_delay = 0 + agent.save_trajectories = False + + compressor = MagicMock() + compressor.protect_first_n = 3 + compressor.protect_last_n = 20 + compressor.threshold_tokens = 100 + compressor.context_length = 1_000 + compressor.last_prompt_tokens = -1 + compressor._verify_compaction_cleared_threshold = False + compressor.awaiting_real_usage_after_compression = False + compressor.should_compress.side_effect = lambda tokens: tokens >= 100 + compressor.should_compress_info.return_value = (False, None) + compressor.should_compress_preflight.return_value = False + compressor.should_defer_preflight_to_real_usage.return_value = False + compressor.get_active_compression_failure_cooldown.return_value = None + compressor.select_context.return_value = None + compressor.get_automatic_compaction_status_message.return_value = "" + agent.compression_enabled = True + agent.context_compressor = compressor + + compactions = [] + + def _estimate(messages=None, *_args, **_kwargs): + # The tool result tips the request over threshold until one compaction ran. + return 200 if not compactions and any(m.get("role") == "tool" for m in messages or []) else 10 + + def _compress(messages, _system_message, **_kwargs): + compactions.append(len(messages)) + return list(messages[2:]), "compressed prompt" # two historical rows summarized away + + def _execute(_assistant_message, messages, *_args): + messages.append({"role": "tool", "name": "web_search", "tool_call_id": "call_1", "content": "RESULT-1"}) + + history = [{"role": "user" if i % 2 == 0 else "assistant", "content": f"msg {i}"} for i in range(30)] + with ( + patch("agent.turn_context.estimate_request_tokens_rough", return_value=10), + patch("agent.model_metadata.estimate_messages_tokens_rough", side_effect=_estimate), + patch("agent.conversation_loop._estimate_tools_tokens_rough", return_value=0), + patch.object(agent, "_compress_context", side_effect=_compress), + patch.object(agent, "_execute_tool_calls", side_effect=_execute), + patch.object(agent, "_flush_messages_to_session_db", return_value=True), + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + result = agent.run_conversation("do tool work", conversation_history=history) + + assert result["final_response"] == "done" + assert len(compactions) == 1 + sent = agent.client.chat.completions.create.call_args_list[-1].kwargs["messages"] + assert [(m["role"], m.get("tool_call_id")) for m in sent[-3:]] == [ + ("user", None), ("assistant", None), ("tool", "call_1"), + ] + assert [t["id"] for t in sent[-2]["tool_calls"]] == ["call_1"] + assert sent[-1]["content"] == "RESULT-1" diff --git a/tests/agent/test_proactive_prune_loop_wiring.py b/tests/agent/test_proactive_prune_loop_wiring.py index 3d7f78a68a..767e71373e 100644 --- a/tests/agent/test_proactive_prune_loop_wiring.py +++ b/tests/agent/test_proactive_prune_loop_wiring.py @@ -154,6 +154,7 @@ class TestProactivePruneLoopWiring: active_system_prompt="system", conversation_history=[], compression_attempts=0, max_compression_attempts=3, effective_task_id=None, final_response="", turn_exit_reason=None, + current_turn_user_idx=0, ) assert verdict.messages is messages assert not verdict.end_turn diff --git a/tests/hermes_cli/test_pending_supervisor_recovery.py b/tests/hermes_cli/test_pending_supervisor_recovery.py index 78b5ac9d7d..50b9ceeb51 100644 --- a/tests/hermes_cli/test_pending_supervisor_recovery.py +++ b/tests/hermes_cli/test_pending_supervisor_recovery.py @@ -7,9 +7,6 @@ import pytest from hermes_cli import gateway, main, update_cmd_fleet as fleet, update_receipt - - - @pytest.mark.parametrize("failure", ["listing", "restart", "inactive", "unloaded", None]) def test_pending_launchd_requires_complete_supervision(monkeypatch, tmp_path, failure): # Host-independent subprocess-boundary fixture, not native launchd validation. diff --git a/tests/hermes_cli/test_process_dock.py b/tests/hermes_cli/test_process_dock.py index 496e956941..ae6be28bb5 100644 --- a/tests/hermes_cli/test_process_dock.py +++ b/tests/hermes_cli/test_process_dock.py @@ -2,6 +2,7 @@ import time from types import SimpleNamespace +from prompt_toolkit.utils import get_cwidth def _wait(predicate, timeout=5.0): @@ -11,6 +12,45 @@ def _wait(predicate, timeout=5.0): time.sleep(0.05) +def test_dock_paints_processes_under_agents_and_retires_finished_rows(monkeypatch): + from hermes_cli import cli_process_dock + from hermes_cli.cli_subagent_monitor import SubagentMonitor + from tools import delegate_tool_registry as registry + from tools.process_registry import process_registry + + monkeypatch.setattr(registry, '_active_subagents', {}) + owner = SimpleNamespace(session_id='owner') + registry._register_subagent(dict(subagent_id='a1', owner_agent_session_id='owner', + goal='Check module', started_at=time.time() - 5, status='running', last_tool='read_file')) + quick = process_registry.spawn_local(command="echo hello-dock; exit 3", cwd='.', task_id='t', owner_task_id='t', session_key='') + slow = process_registry.spawn_local(command="sleep 30", cwd='.', task_id='t', owner_task_id='t', session_key='') + quick_id, slow_id = quick.id, slow.id + try: + _wait(lambda: process_registry.get(quick_id).exited) + process_registry.list_sessions() # observes the exit → exited_at stamped + dock = SubagentMonitor(SimpleNamespace(agent=owner)) + assert dock.refresh() + text = dock.dock_text(columns=100, rows=30) + lines = text.splitlines() + assert 'Subagents · 1 live' in lines[0] + agents_at = next(i for i, line in enumerate(lines) if 'Check module' in line) + procs_at = next(i for i, line in enumerate(lines) if 'Processes · 1 running · 1 done' in line) + assert agents_at < procs_at + assert any('⚙ sleep 30' in line and 'starting' in line for line in lines) + assert any('✘ echo hello-dock; exit 3 · exit 3' in line for line in lines) + assert all(get_cwidth(line) <= 100 for line in lines) + # Every viewport keeps at least one row of each block. + narrow = dock.dock_text(columns=40, rows=14).splitlines() + assert any('Check module' in line for line in narrow) and any('Processes' in line for line in narrow) + assert all(get_cwidth(line) <= 40 for line in narrow) + dock.collapsed = True + assert dock.dock_text(columns=100, rows=30).count('\n') == 0 + assert '1 live · 1 proc' in dock.dock_text(columns=100, rows=30) + # Finished rows leave after the retention window; running ones stay. + later = cli_process_dock.process_rows(time.time() + cli_process_dock.RETAIN_SECONDS + 1) + assert [r['id'] for r in later] == [slow_id] + finally: + process_registry.kill_process(slow_id) def test_monitor_controls_stop_processes_and_never_steer_them(): diff --git a/tests/hermes_cli/test_profiles.py b/tests/hermes_cli/test_profiles.py index 1e993ba6b8..61c33295ee 100644 --- a/tests/hermes_cli/test_profiles.py +++ b/tests/hermes_cli/test_profiles.py @@ -9,6 +9,7 @@ import json import os import shutil import socket +import stat import sys import tarfile import types @@ -217,6 +218,44 @@ class TestCreateProfile: assert (profile_dir / ".env").read_text(encoding="utf-8-sig").strip() == "KEY=val" assert (profile_dir / "SOUL.md").read_text(encoding="utf-8-sig") == "Be helpful." + def test_clone_config_copies_only_the_active_memory_providers_config(self, profile_env): + """#120115: --clone carried ``memory.provider: hindsight`` but not hindsight's own config, + so the clone booted with memory silently unavailable. Only the ACTIVE provider's + ``/`` dir / ``.json`` travels; another provider's leftovers stay behind.""" + tmp_path = profile_env + default_home = tmp_path / ".hermes" + (default_home / "config.yaml").write_text("memory:\n provider: hindsight\n") + (default_home / "hindsight").mkdir() + payload = '{"mode": "local_embedded", "bank_id": "hermes", "apiKey": "hs-secret"}' + (default_home / "hindsight" / "config.json").write_text(payload) + (default_home / "mem0.json").write_text('{"agent_id": "hermes"}') + + profile_dir = create_profile("coder", clone_config=True, no_alias=True) + + cloned = profile_dir / "hindsight" / "config.json" + assert cloned.read_text() == payload + if os.name != "nt": + assert stat.S_IMODE(cloned.stat().st_mode) == 0o600 + assert not (profile_dir / "mem0.json").exists() + + @pytest.mark.parametrize("provider", ["../outside", "a/b", "..", "hind sight"]) + def test_clone_config_ignores_unsafe_memory_provider_names(self, profile_env, provider): + """A hand-edited ``memory.provider`` must never aim the copy outside the source profile.""" + tmp_path = profile_env + default_home = tmp_path / ".hermes" + (default_home / "config.yaml").write_text(f"memory:\n provider: {provider!r}\n") + (tmp_path / "outside").mkdir() + (tmp_path / "outside" / "config.json").write_text("{}") + (default_home / "a").mkdir() + (default_home / "a" / "b").mkdir() + (default_home / "a" / "b" / "config.json").write_text("{}") + + profile_dir = create_profile("coder", clone_config=True, no_alias=True) + + assert not (profile_dir / "a").exists() + assert not (profile_dir.parent / "outside").exists() + assert not (profile_dir / "hind sight").exists() + def test_clone_sync_imports_carries_manifest_but_never_links_profiles(self, profile_env): """--sync-imports copies import-sync.json (a pointer at EXTERNAL agent trees) and nothing else changes: the clone still gets its own config/skills copies, never a live link.""" diff --git a/tests/hermes_cli/test_update_fleet_home_scope.py b/tests/hermes_cli/test_update_fleet_home_scope.py new file mode 100644 index 0000000000..f8274097e4 --- /dev/null +++ b/tests/hermes_cli/test_update_fleet_home_scope.py @@ -0,0 +1,95 @@ +"""#93349 — ``hermes update`` restarts only the gateways of the home it is updating. + +``hermes-gateway*`` units and ``gateway run`` processes are host-wide namespaces shared by every +Hermes install under the account. A scratch home's update used to drain and restart the account's +real ``hermes-gateway.service`` and SIGTERM sibling installs' gateways because they were listed, +not because they ran the updated code. +""" + +from __future__ import annotations + +import os +import signal +import subprocess +from pathlib import Path + +import pytest + +from hermes_cli import update_cmd_fleet as fleet +from hermes_cli import dashboard_procs + +FOREIGN_HOME = "/srv/other-account-home/.hermes" + + +@pytest.fixture +def own_home(monkeypatch, tmp_path): + home = tmp_path / "homeA" / ".hermes" + home.mkdir(parents=True) + monkeypatch.setenv("HERMES_HOME", str(home)) + # The plan inventory is this install only; the fixture home has no profiles dir. + monkeypatch.setattr("hermes_cli.update_receipt._profile_homes", lambda: [("default", home)]) + return home + + +def _pid_homes(monkeypatch, mapping: dict): + monkeypatch.setattr(dashboard_procs, "_hermes_home_for_pid", lambda pid: mapping.get(pid)) + + +def test_systemd_unit_of_another_home_is_left_alone(monkeypatch, own_home): + """user/hermes-gateway runs on another HERMES_HOME → no drain, no restart, not a failure; + the same unit running on the updating home is still restarted (control).""" + homes = {4242: FOREIGN_HOME, 4343: str(own_home)} + _pid_homes(monkeypatch, homes) + main_pid = {"value": 4242} + write_verbs: list[list[str]] = [] + + def fake_systemctl(cmd, *, timeout): + if "is-active" in cmd: + return subprocess.CompletedProcess(cmd, 0, stdout="active\n", stderr="") + if "show" in cmd and "--property=MainPID" in cmd: + return subprocess.CompletedProcess(cmd, 0, stdout=f"{main_pid['value']}\n", stderr="") + if "show" in cmd: + return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="") + write_verbs.append(cmd) + return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="") + + monkeypatch.setattr(fleet, "_systemctl", fake_systemctl) + monkeypatch.setattr(fleet, "_repair_unit_without_fatal_exit_park", lambda *a, **k: None) + drained: list[int] = [] + monkeypatch.setattr(fleet, "_drain_or_signal_gateway_for_update", lambda pid, *a, **k: drained.append(pid) or True) + monkeypatch.setattr(fleet, "_wait_for_service_active", lambda *a, **k: True) + + restarted: list[str] = [] + failed: list[str] = [] + fleet._restart_one_systemd_gateway_unit( + "hermes-gateway", scope="user", scope_cmd=["systemctl", "--user"], drain_budget=5.0, + _manage_cmd_cache={}, restarted_services=restarted, failed_or_stale_units=failed, + ) + assert drained == [] and write_verbs == [] and restarted == [] and failed == [] + + main_pid["value"] = 4343 # control: same unit name, this update's home + fleet._restart_one_systemd_gateway_unit( + "hermes-gateway", scope="user", scope_cmd=["systemctl", "--user"], drain_budget=5.0, + _manage_cmd_cache={}, restarted_services=restarted, failed_or_stale_units=failed, + ) + assert drained == [4343] and restarted == ["hermes-gateway"] and failed == [] + + +def test_manual_gateway_of_another_home_is_not_stopped(monkeypatch, own_home): + """Of two ``gateway run`` processes on the host, only the one on the updating home is SIGTERMed; + a process whose home cannot be read is spared as well.""" + _pid_homes(monkeypatch, {111: str(own_home), 222: FOREIGN_HOME, 333: None}) + monkeypatch.setattr("hermes_cli.gateway._get_service_pids", lambda **k: set()) + monkeypatch.setattr("hermes_cli.gateway.find_gateway_pids", lambda **k: [111, 222, 333]) + monkeypatch.setattr("hermes_cli.gateway.find_profile_gateway_processes", lambda **k: []) + monkeypatch.setattr("hermes_cli.gateway._wait_for_gateway_exit", lambda **k: None) + killed: list[tuple[int, int]] = [] + monkeypatch.setattr(os, "kill", lambda pid, sig: killed.append((pid, sig))) + + out = fleet._GatewayRestartOutcome( + incomplete=False, phase_errors=[], pre_restart_gateway_pids=[], restarted_services=[], + failed_or_stale_units=[], relaunched_profiles=[], externally_supervised_profiles=[], killed_pids=set(), + ) + fleet._restart_manual_gateways(out, 5.0) + assert killed == [(111, signal.SIGTERM)] + assert out.killed_pids == {111} diff --git a/tests/hermes_cli/test_update_host_obligation.py b/tests/hermes_cli/test_update_host_obligation.py index 1c554c83f4..bffe0ac8e9 100644 --- a/tests/hermes_cli/test_update_host_obligation.py +++ b/tests/hermes_cli/test_update_host_obligation.py @@ -28,6 +28,14 @@ from hermes_cli import update_cmd SHA = "a" * 40 +@pytest.fixture(autouse=True) +def _units_belong_to_this_update(monkeypatch): + """The fake units here run on invented PIDs (4242, per-unit tables) with no readable home; + ownership (#93349, ``test_update_fleet_home_scope.py``) is pinned so these tests keep proving + the once-per-host-process collapse, not home scoping.""" + monkeypatch.setattr(fleet, "_systemd_unit_owned_by_update", lambda scope_cmd, svc_name: True) + + @pytest.fixture def two_profiles(tmp_path, monkeypatch): """Two profile HERMES_HOMEs behind ONE host state dir — the real multiplex topology.""" @@ -69,30 +77,6 @@ def test_obligation_armed_by_one_profile_is_owed_by_every_other(two_profiles, no assert fleet._pending_fleet_restart_needed() is False -def test_host_gateway_restarts_once_when_two_profiles_run_the_catch_up( - two_profiles, no_live_fleet, monkeypatch, capsys -): - """``hermes -p coder update`` then ``hermes -p writer update`` stops the host gateway ONCE.""" - monkeypatch.setattr("hermes_cli.gateway.find_gateway_pids", lambda **k: [4242]) - monkeypatch.setattr("hermes_cli.gateway.supports_systemd_services", lambda: False) - monkeypatch.setattr("hermes_cli.gateway.is_macos", lambda: False) - monkeypatch.setattr("hermes_cli.gateway.is_windows", lambda: False) - monkeypatch.setattr("hermes_cli.gateway._wait_for_gateway_exit", lambda **k: True) - monkeypatch.setattr(fleet, "_restart_macos_launchd_gateways", lambda *a, **k: None) - kills: list = [] - monkeypatch.setattr("hermes_cli.gateway.kill_gateway_processes", lambda **k: kills.append(k)) - - _enter(monkeypatch, two_profiles["coder"]) - _arm("coder") - assert update_cmd._run_pending_fleet_restart() is True - - _enter(monkeypatch, two_profiles["writer"]) - _arm("writer") - assert update_cmd._run_pending_fleet_restart() is True - - assert len(kills) == 1, "the one host gateway must be stopped once per update, not once per profile" - - def test_legacy_per_home_marker_is_still_read_and_cleared(two_profiles, no_live_fleet, monkeypatch): """An obligation armed by the pre-host-scope code must still be discharged after the upgrade.""" _enter(monkeypatch, two_profiles["coder"]) @@ -235,32 +219,6 @@ def test_unreadable_host_record_is_never_discharged_by_the_legacy_marker(two_pro assert fleet._pending_fleet_restart_needed() is True -def test_restart_runs_once_per_host_on_a_non_git_install(two_profiles, monkeypatch, capsys): - """zip/pip/Docker installs resolve no checkout SHA; the restart-once guard must still hold. - - ``mark_host_restart_completed("")`` can never match, so every profile's ``hermes update`` - re-killed the one shared multiplexer on exactly the installs this record exists for. - """ - monkeypatch.setattr(fleet, "_current_checkout_sha", lambda: None) - monkeypatch.setattr("hermes_cli.update_receipt.collect_fleet_versions", lambda: []) - monkeypatch.setattr("hermes_cli.gateway.find_gateway_pids", lambda **k: [4242]) - monkeypatch.setattr("hermes_cli.gateway.supports_systemd_services", lambda: False) - monkeypatch.setattr("hermes_cli.gateway.is_macos", lambda: False) - monkeypatch.setattr("hermes_cli.gateway.is_windows", lambda: False) - monkeypatch.setattr("hermes_cli.gateway._wait_for_gateway_exit", lambda **k: True) - kills: list = [] - monkeypatch.setattr("hermes_cli.gateway.kill_gateway_processes", lambda **k: kills.append(k)) - - _enter(monkeypatch, two_profiles["coder"]) - _arm("coder") - assert update_cmd._run_pending_fleet_restart() is True - - _enter(monkeypatch, two_profiles["writer"]) - assert update_cmd._run_pending_fleet_restart() is True - - assert len(kills) == 1, "the host gateway must be stopped once per update, not once per profile" - - def test_a_failing_main_pid_probe_keeps_its_own_restart(): """Any probe error is unproven identity (its own restart), never an aborted restart pass.""" def boom(unit): diff --git a/tests/hermes_cli/test_update_unit_client_budget.py b/tests/hermes_cli/test_update_unit_client_budget.py index 0f56b4864a..2f8529fc05 100644 --- a/tests/hermes_cli/test_update_unit_client_budget.py +++ b/tests/hermes_cli/test_update_unit_client_budget.py @@ -6,6 +6,13 @@ import pytest from hermes_cli import update_cmd_fleet as fleet +@pytest.fixture(autouse=True) +def _units_belong_to_this_update(monkeypatch): + """Fake units on an invented MainPID (42) have no readable home; ownership (#93349, + ``test_update_fleet_home_scope.py``) is pinned so these tests keep proving budgets and health.""" + monkeypatch.setattr(fleet, "_systemd_unit_owned_by_update", lambda scope_cmd, svc_name: True) + + @pytest.mark.parametrize("graceful,retry", [(False, False), (False, True), (True, False)]) def test_unit_transaction_budget_preserves_scope_and_health(monkeypatch, graceful, retry): scope = ["systemctl", "--no-ask-password"] diff --git a/tests/install/e2e-assets/known-failures.json b/tests/install/e2e-assets/known-failures.json index eaf226778a..263b6dc5c5 100644 --- a/tests/install/e2e-assets/known-failures.json +++ b/tests/install/e2e-assets/known-failures.json @@ -6,7 +6,7 @@ "cases": [["installer-script", "hermes-update"]], "errors": ["^E2E ASSERTION FAILED: hermes update exited [1-9][0-9]* \\(expected 0\\)$"], "log": "update", - "signatures": ["failed to remove file[^\\r\\n]*Scripts[/\\\\]hermes\\.exe[^\\r\\n]*Access is denied\\. \\(os error 5\\)", "hermes\\.exe[/\\\\]__main__\\.py", "CalledProcessError[^\\r\\n]*pip[^\\r\\n]*install[^\\r\\n]*returned non-zero exit status 2"], + "signatures": ["failed to remove file[^\\r\\n]*Scripts[/\\\\]hermes\\.exe[^\\r\\n]*Access is denied\\. \\(os error 5\\)", "(?:CalledProcessError|Git update failed: Command)[^\\r\\n]*pip[^\\r\\n]*install[^\\r\\n]*returned non-zero exit status 2"], "explanation": "The March/April Windows updater is already running from hermes.exe when uv tries to replace it. Windows refuses the locked launcher. The update target cannot change that loaded code; re-running the installer is a separate recovery route.", "evidence": "https://github.com/ethernet8023/hermes-agent/actions/runs/34055462305/job/101546487700" }, diff --git a/tests/tools/test_mcp_oauth_callback_latch.py b/tests/tools/test_mcp_oauth_callback_latch.py new file mode 100644 index 0000000000..c7851c3034 --- /dev/null +++ b/tests/tools/test_mcp_oauth_callback_latch.py @@ -0,0 +1,107 @@ +"""The loopback OAuth callback latches the first terminal result (#116278). + +A browser follows the ``/callback`` redirect with queryless fetches (``/favicon.ico``), and the CLI waiter +samples the result only every 500 ms. A handler that wrote every GET into the result lost the stored code +between two polls, so the user saw "Authorization Successful" while ``hermes mcp login`` timed out. These +tests drive the production entry (``_make_callback_waiter`` → ``_start_callback_server`` → handler) with a +browser stand-in that sends its requests back-to-back, well inside one poll interval. +""" +import asyncio +import io +import socket +import threading +from http.client import HTTPConnection + +import pytest + +pytest.importorskip("mcp.client.auth.oauth2", reason="MCP SDK 1.26.0+ required") + +import tools.mcp_oauth as mo + + +def _free_port() -> int: + with socket.socket() as s: + s.bind(("127.0.0.1", 0)) + return s.getsockname()[1] + + +def _get(port: int, path: str) -> int: + conn = HTTPConnection("127.0.0.1", port, timeout=5) + try: + conn.request("GET", path) + resp = conn.getresponse() + resp.read() + return resp.status + finally: + conn.close() + + +def _wait_listening(port: int) -> None: + for _ in range(200): + try: + with socket.create_connection(("127.0.0.1", port), timeout=0.2): + return + except OSError: + threading.Event().wait(0.02) + raise AssertionError("callback listener never bound") + + +def _drive_waiter(monkeypatch, paths: list[str]): + """Run the real waiter on its own loop; send *paths* back-to-back once the listener is bound. + + The waiter polls ``_result_taken`` every 500 ms and closes the listener as soon as the first + terminal callback lands, so on a loaded runner the stand-in's later requests raced a dead port + (``ConnectionRefusedError`` / ``ConnectionResetError``). The waiter's poll is held open until + every request has been answered; the handler's own ``_result_taken`` reads are untouched, which + is what the latch under test relies on.""" + monkeypatch.setattr(mo.sys, "stdin", io.StringIO()) # paste reader sees EOF; the HTTP listener is under test + port = _free_port() + out: dict = {} + requests_sent = threading.Event() + real_taken = mo._result_taken + + def run(): + async def main(): + with mo.force_interactive_oauth(): + return await mo._make_callback_waiter(port, timeout=30)() + try: + out["result"] = asyncio.run(main()) + except Exception as exc: # noqa: BLE001 — the timeout is the failure under test + out["exc"] = exc + + thread = threading.Thread(target=run) + + def gated_taken(result): + if threading.current_thread() is thread and not requests_sent.is_set(): + return False # the waiter's poll: keep the listener up until the browser stand-in is done + return real_taken(result) + + monkeypatch.setattr(mo, "_result_taken", gated_taken) + thread.start() + _wait_listening(port) + try: + statuses = [_get(port, p) for p in paths] + finally: + requests_sent.set() + thread.join(timeout=15) + assert not thread.is_alive(), "waiter did not finish" + assert "exc" not in out, f"waiter raised {type(out.get('exc')).__name__}" + return statuses, out["result"] + + +def test_favicon_right_after_callback_does_not_clobber_the_code(monkeypatch): + statuses, result = _drive_waiter( + monkeypatch, ["/callback?code=synthetic&state=s1&iss=https://as.example", "/favicon.ico"]) + assert statuses == [200, 404] + assert (result.code, result.state, result.iss) == ("synthetic", "s1", "https://as.example") + + +def test_first_terminal_callback_wins_over_later_ones(monkeypatch): + statuses, result = _drive_waiter(monkeypatch, [ + "/favicon.ico", + "/callback?code=first&state=s1", + "/callback?code=second&state=s2", + "/callback?error=access_denied&state=s1", + ]) + assert statuses == [404, 200, 200, 200] + assert (result.code, result.state) == ("first", "s1") diff --git a/tests/tools/test_process_registry.py b/tests/tools/test_process_registry.py index 68ec0e9a83..8faa5cf9a3 100644 --- a/tests/tools/test_process_registry.py +++ b/tests/tools/test_process_registry.py @@ -408,6 +408,19 @@ def test_reader_loop_reassembles_multibyte_char_split_across_chunks(registry, mo assert "\ufffd" not in session.output_buffer +def test_reader_loop_strips_shell_noise_split_across_reads(registry, monkeypatch): + """``bash -lic`` without a tty writes its two startup warnings in two separate write() calls. + A reader that wakes between them (loaded CI) must still drop the second line: it leaked as the + process's only "output", so the dock painted ``last: bash: no job control...`` instead of + ``starting`` and probes waiting on any output woke before the real writer had printed.""" + session = _run_reader(registry, monkeypatch, [ + b"bash: cannot set terminal process group (7): Inappropriate ioctl for device\n", + b"bash: no job control in this shell\n", + b"real output\n", + b"bash: no job control in this shell\n", # after real output it is the process's own text + ]) + assert session.output_buffer == "real output\nbash: no job control in this shell\n" + def test_reader_loop_flushes_truncated_multibyte_tail_at_eof(registry, monkeypatch): diff --git a/tests/tools/test_process_registry_list_exit.py b/tests/tools/test_process_registry_list_exit.py index b46d8ff9b1..0d5d7243a0 100644 --- a/tests/tools/test_process_registry_list_exit.py +++ b/tests/tools/test_process_registry_list_exit.py @@ -83,7 +83,7 @@ def _probe(root): session.notify_on_complete = True owner, sibling = sessions deadline = time.monotonic() + 5 - while not all(s.output_buffer for s in sessions): + while not all(name + "-output" in s.output_buffer for name, s in zip(("owner", "sibling"), sessions)): assert time.monotonic() < deadline, "writers did not become ready" time.sleep(0.01) assert all(s.process.poll() is None for s in sessions) diff --git a/tools/process_registry.py b/tools/process_registry.py index 9e4c563515..6cb8c0cb46 100644 --- a/tools/process_registry.py +++ b/tools/process_registry.py @@ -1345,7 +1345,10 @@ class ProcessRegistry(ProcessCheckpointMixin): Windows pipes don't support select(); the blocking path is kept there and the lazy reconcile in poll()/wait() remains the safety net. See #68915, #8340. """ - first_chunk = True + # ``bash -lic`` without a tty writes its startup warnings one write() per line, so the + # reader can wake between them; strip leading noise from every chunk until the + # process has produced real output, not just from the first read. + head_noise = True # A split multibyte UTF-8 char would become U+FFFD with stateless decoding; the # incremental decoder holds the partial sequence until the rest arrives. decoder = codecs.getincrementaldecoder("utf-8")(errors="replace") @@ -1356,10 +1359,10 @@ class ProcessRegistry(ProcessCheckpointMixin): # same treatment the foreground path already has in # ``tools/environments/base.py::_wait_for_process``. (Ported from openclaw/openclaw#112325.) def _append_chunk(chunk: str): - nonlocal first_chunk - if first_chunk: + nonlocal head_noise + if head_noise: chunk = self._clean_shell_noise(chunk) - first_chunk = False + head_noise = not chunk.strip() self._ingest_output(session, chunk) try: proc = session.process diff --git a/website/docs/reference/cli-commands.md b/website/docs/reference/cli-commands.md index cfbd9d2fd9..744c803aa8 100644 --- a/website/docs/reference/cli-commands.md +++ b/website/docs/reference/cli-commands.md @@ -1999,7 +1999,7 @@ external update owner. See [Updating & Uninstalling](../getting-started/updating Additional behavior: -- **Gateway restart.** After a successful update, Hermes attempts to restart all running gateway profiles automatically so they pick up the new code. Use `hermes gateway restart` when you want to restart a gateway without applying an update. +- **Gateway restart.** After a successful update, Hermes attempts to restart all running gateway profiles of the home being updated (its root and every `profiles/` under it) automatically so they pick up the new code. Gateways and `hermes-gateway*` services that belong to a different `HERMES_HOME` on the same machine — another install, or a scratch home running `hermes update` — are named in the output and left alone. Use `hermes gateway restart` when you want to restart a gateway without applying an update. - **Restart-phase recovery.** If the in-process restart phase aborts while importing the freshly pulled tree, supervised gateway profiles are retried through a clean Python process. Only restarts independently confirmed by systemd (`systemctl --user is-active`) are reported as verified; a relaunch that merely exited 0 is recorded as `relaunch_attempted` and still fails the update conservatively. Manual gateways and serve/dashboard runtimes are never killed without a relaunch authority; they are recorded as skipped with a reason and remain in the incomplete-update report with the exact restart command. - **Update receipts + fleet version check.** Every run writes a machine-readable receipt to `~/.hermes/logs/update_receipts/` (pre-update fleet plan, steps, skips with reasons, restart outcome; `latest.json` points at the newest). After the restart phase the updater verifies each live gateway's running code against the updated checkout and prints a per-profile version matrix; a gateway still on pre-update code fails the update (exit 1) with the exact restart command. - **Local source changes.** For git installs, dirty tracked files and untracked files are auto-stashed before branch checkout or pull (`git stash push --include-untracked`). Interactive terminal updates ask before restoring the stash. Non-interactive updates restore it by default; set `updates.non_interactive_local_changes: discard` only on managed installs where local source edits should be thrown away after a successful pull. If stash restore conflicts or the pull fails, the stash is left in place for manual recovery. diff --git a/website/docs/reference/profile-commands.md b/website/docs/reference/profile-commands.md index 8e34e33136..06244e462c 100644 --- a/website/docs/reference/profile-commands.md +++ b/website/docs/reference/profile-commands.md @@ -80,7 +80,7 @@ Creates a new profile. | Argument / Option | Description | |-------------------|-------------| | `` | Name for the new profile. Must be a valid directory name (alphanumeric, hyphens, underscores). | -| `--clone` | Copy `config.yaml`, `.env`, `SOUL.md`, skills, and the curated `memories/MEMORY.md` / `memories/USER.md` from the current profile. Sessions, `state.db` and cron jobs are not copied. | +| `--clone` | Copy `config.yaml`, `.env`, `SOUL.md`, skills, the curated `memories/MEMORY.md` / `memories/USER.md`, and the active `memory.provider`'s own config (`/` or `.json`, e.g. `hindsight/config.json`) from the current profile. Sessions, `state.db` and cron jobs are not copied. | | `--clone-all` | Copy everything (config, memories, skills, plugins) from the current profile. Excludes per-profile history: sessions, `state.db`, backups, state-snapshots, checkpoints — and cron jobs, which stay bound to the source profile (a clone that inherited them would fire every job twice). When the source is the default profile, the machine-scoped local-model trees (`models/`, `runtimes/`, `node/`) are also skipped — the same trees `hermes backup` excludes. | | `--clone-from ` | Clone config/skills/SOUL from a specific profile instead of the current one. Implies `--clone` unless paired with `--clone-all`. | | `--no-alias` | Skip wrapper script creation. | diff --git a/website/docs/user-guide/profiles.md b/website/docs/user-guide/profiles.md index 3300ec2919..06688de2f3 100644 --- a/website/docs/user-guide/profiles.md +++ b/website/docs/user-guide/profiles.md @@ -80,7 +80,7 @@ You can also set or auto-generate the description later with `hermes profile des hermes profile create work --clone ``` -Copies your current profile's `config.yaml`, `.env`, `SOUL.md`, skills, and the curated memory files `memories/MEMORY.md` and `memories/USER.md` into the new profile — memory is treated as part of the agent's identity, like `SOUL.md`. Sessions, `state.db`, cron jobs and everything else start empty. For a blank memory as well, create the profile without `--clone` or delete the two files afterwards; the agent never falls back to another profile's memory when they are absent. Edit `~/.hermes/profiles/work/.env` for different API keys, or `~/.hermes/profiles/work/SOUL.md` for a different personality. +Copies your current profile's `config.yaml`, `.env`, `SOUL.md`, skills, and the curated memory files `memories/MEMORY.md` and `memories/USER.md` into the new profile — memory is treated as part of the agent's identity, like `SOUL.md`. If `config.yaml` selects an external memory provider (`memory.provider`), that provider's own config travels too — its `/` directory or `.json` under the profile home, e.g. `hindsight/config.json` — so the clone's memory is available instead of silently off; a cloned `local_embedded` hindsight config still shares the source's embedded daemon and bank until you give the clone its own hindsight `profile`/`bank_id` ([#81815](https://github.com/NousResearch/hermes-agent/issues/81815)). Sessions, `state.db`, cron jobs and everything else start empty. For a blank memory as well, create the profile without `--clone` or delete the two files afterwards; the agent never falls back to another profile's memory when they are absent. Edit `~/.hermes/profiles/work/.env` for different API keys, or `~/.hermes/profiles/work/SOUL.md` for a different personality. #### Keep a clone's imported agent setups synced (`--sync-imports`)