From 1393403dd79d7e391e361a34405dc3dae6ee171e Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Tue, 22 Sep 2026 09:22:24 -0700 Subject: [PATCH 1/9] fix(process_registry): strip bash startup noise until real output, not only from the first read A tty-less `bash -lic` writes "cannot set terminal process group" and "no job control in this shell" as two separate write() calls. The reader cleaned shell noise from the FIRST chunk only, so whenever it woke between the two writes (loaded CI runners) the second line landed in output_buffer as the process's only "output". Symptoms on main: tests/hermes_cli/test_process_dock.py painted "last: bash: no job control..." instead of "starting" (3 main reds, Sep 21-22) and tests/tools/test_process_registry_list_exit.py's probe woke on that noise before the writer had printed, so the completion event lacked "owner-output" (2 main FLAKY frames). The same leak reaches users through process.list output_preview and the live-work dock. Keep stripping leading noise from every chunk until the process has produced non-blank output; after that, matching text is the process's own. The list_exit probe now waits for the writer's marker rather than for any bytes. Live repro (stand-in shell reproducing bash's two writes with a 50 ms gap, real spawn_local + select/read1 reader): base 10/10 buffers carry "bash: no job control in this shell\n"; fixed 0/10. --- tests/tools/test_process_registry.py | 13 +++++++++++++ tests/tools/test_process_registry_list_exit.py | 2 +- tools/process_registry.py | 11 +++++++---- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/tests/tools/test_process_registry.py b/tests/tools/test_process_registry.py index 1b4fad8087..19292a1a4d 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 55f933d30e..26f604a742 100644 --- a/tests/tools/test_process_registry_list_exit.py +++ b/tests/tools/test_process_registry_list_exit.py @@ -45,7 +45,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 2b417d618c..07599328ef 100644 --- a/tools/process_registry.py +++ b/tools/process_registry.py @@ -1328,7 +1328,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") @@ -1339,10 +1342,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 From ff848442db38b593cf6d65c5188e1b6c83ad5ace Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 23 Sep 2026 04:30:43 -0700 Subject: [PATCH 2/9] test: restore the process dock paint test #120071 deleted as flaky Its flake was the bash startup-noise leak this PR fixes (the dock painted 'last: bash: no job control...' instead of 'starting'). 5/5 green with the fix under load average ~150. --- tests/hermes_cli/test_process_dock.py | 40 +++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) 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(): From 6d885a296a9638e1cd5b71ca7dd170e2e59fd833 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Tue, 22 Sep 2026 09:23:51 -0700 Subject: [PATCH 3/9] test(mcp): callback-latch tests keep the listener up until the browser stand-in is done tests/tools/test_mcp_oauth_callback_latch.py failed on main four times in two days (Sep 20-21) with ConnectionRefusedError / ConnectionResetError on the stand-in's second request. The production waiter polls _result_taken every 500 ms and shuts the listener down as soon as the first terminal /callback lands; the test relied on all follow-up GETs (favicon, duplicate callbacks) landing inside that poll window, which a loaded runner does not guarantee. Gate the waiter thread's _result_taken poll on a "requests sent" event so the listener stays bound until every stand-in request has been answered. The handler's own reads are untouched, so the first-wins latch is still what is being exercised. Waiter timeout 4 s -> 30 s: the happy path finishes in one poll, and a stuck waiter still fails the test. Live repro: a 0.7 s gap between the stand-in's requests turns the CI signature deterministic on base (2/2 ConnectionRefusedError); with the fix the same gap passes 2/2, and the unmodified test passes 5/5. --- tests/tools/test_mcp_oauth_callback_latch.py | 107 +++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 tests/tools/test_mcp_oauth_callback_latch.py 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") From c500dc6d99e948c0726a039d10a369bb441c4adb Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 23 Sep 2026 04:45:08 -0700 Subject: [PATCH 4/9] fix: hermes update restarts only the gateways of the home it updates (#93349) hermes-gateway*/hermes-serve* units, ai.hermes.gateway* LaunchAgents and `gateway run` processes are account-wide namespaces shared by every Hermes install on the box. The restart phase enumerated them by name, so a scratch home's `hermes update` drained and restarted the account's real hermes-gateway.service and SIGTERMed sibling installs' gateways (live: #93349 comment, 2026-09-23), then warned "Fleet version check returned no rows" because none of those runtimes belonged to the updating home. Ownership is now judged from what a runtime actually runs on, never from the label: the live process environment (`_hermes_home_for_pid`), the unit's declared Environment=HERMES_HOME, or the plist's pinned HERMES_HOME, compared against the homes the update's plan inventories (the updating root and its profiles/). Foreign or unreadable ownership is named in the output and left alone; it is not a failed restart. Applies to the systemd fleet loop, the catch-up best-effort restart, the launchd derived-label loop, the manual gateway sweep, and the pre/post-restart PID snapshots that drive the fail-closed verdict. --- hermes_cli/AGENTS.md | 13 +- hermes_cli/update_cmd_fleet.py | 45 +++++- hermes_cli/update_fleet_scope.py | 148 ++++++++++++++++++ .../test_update_fleet_home_scope.py | 95 +++++++++++ website/docs/reference/cli-commands.md | 2 +- 5 files changed, 296 insertions(+), 7 deletions(-) create mode 100644 hermes_cli/update_fleet_scope.py create mode 100644 tests/hermes_cli/test_update_fleet_home_scope.py diff --git a/hermes_cli/AGENTS.md b/hermes_cli/AGENTS.md index 421af5da92..32e557dbac 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/update_cmd_fleet.py b/hermes_cli/update_cmd_fleet.py index 60f0f6df6a..e0ddca4d60 100644 --- a/hermes_cli/update_cmd_fleet.py +++ b/hermes_cli/update_cmd_fleet.py @@ -581,6 +581,8 @@ def _restart_systemd_gateway_units_best_effort(failed: list, listings) -> None: 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 @@ -867,6 +869,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``.""" @@ -1053,9 +1083,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. @@ -1107,7 +1143,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 @@ -1335,6 +1371,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 @@ -1541,6 +1579,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 @@ -1756,7 +1797,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/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/website/docs/reference/cli-commands.md b/website/docs/reference/cli-commands.md index 03d148f441..68050f5ed7 100644 --- a/website/docs/reference/cli-commands.md +++ b/website/docs/reference/cli-commands.md @@ -1965,7 +1965,7 @@ Pulls the latest `hermes-agent` code and reinstalls dependencies in the managed 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. From f43eef9e27ffce219804cf3dc37aa13e9214a893 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 23 Sep 2026 04:45:08 -0700 Subject: [PATCH 5/9] test: update restart leaves other homes' units and gateways alone (#93349) Two invariants, red on origin/main: a hermes-gateway unit whose MainPID runs on another HERMES_HOME is neither drained nor restarted (and the same unit on the updating home still is); of the gateway processes on the host only the one on the updating home is SIGTERMed, an unreadable one is spared. Existing fixtures that invent units on PIDs 42/4242 with no readable home pin `_systemd_unit_owned_by_update` to True so they keep testing budgets, the once-per-host collapse and the catch-up contract rather than home scoping. --- tests/hermes_cli/test_pending_supervisor_recovery.py | 7 +++++++ tests/hermes_cli/test_update_host_obligation.py | 8 ++++++++ tests/hermes_cli/test_update_unit_client_budget.py | 7 +++++++ 3 files changed, 22 insertions(+) diff --git a/tests/hermes_cli/test_pending_supervisor_recovery.py b/tests/hermes_cli/test_pending_supervisor_recovery.py index 6752040de4..bdc0c2485d 100644 --- a/tests/hermes_cli/test_pending_supervisor_recovery.py +++ b/tests/hermes_cli/test_pending_supervisor_recovery.py @@ -7,6 +7,13 @@ import pytest from hermes_cli import gateway, main, update_cmd_fleet as fleet, update_receipt +@pytest.fixture(autouse=True) +def _units_belong_to_this_update(monkeypatch): + """The fake ``hermes-gateway-one/two`` units carry no home; ownership (#93349, + ``test_update_fleet_home_scope.py``) is pinned so this file keeps testing the catch-up contract.""" + monkeypatch.setattr(fleet, "_systemd_unit_owned_by_update", lambda scope_cmd, svc_name: True) + + @pytest.mark.linux_only @pytest.mark.parametrize("failure", ["listing", "timeout", "missing", "restart", "inactive", "running", "missing-owned", None]) def test_pending_marker_requires_complete_systemd_recovery(monkeypatch, tmp_path, failure): diff --git a/tests/hermes_cli/test_update_host_obligation.py b/tests/hermes_cli/test_update_host_obligation.py index 1c554c83f4..f3adad4056 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.""" diff --git a/tests/hermes_cli/test_update_unit_client_budget.py b/tests/hermes_cli/test_update_unit_client_budget.py index 4ba823a52c..9963575b25 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), ("catchup", False)]) def test_unit_transaction_budget_preserves_scope_and_health(monkeypatch, graceful, retry): catchup = graceful == "catchup" From f27417fc40b33b60614cd25dff33124640d9bb39 Mon Sep 17 00:00:00 2001 From: Andrey <3605840+Diaspar4u@users.noreply.github.com> Date: Mon, 31 Aug 2026 20:12:58 -0400 Subject: [PATCH 6/9] fix(agent): re-anchor after post-tool compression --- agent/turn_preflight.py | 7 ++- agent/turn_tool_round.py | 8 ++- ...est_post_tool_compression_turn_boundary.py | 56 +++++++++++++++++++ 3 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 tests/agent/test_post_tool_compression_turn_boundary.py 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/tests/agent/test_post_tool_compression_turn_boundary.py b/tests/agent/test_post_tool_compression_turn_boundary.py new file mode 100644 index 0000000000..b100cc1388 --- /dev/null +++ b/tests/agent/test_post_tool_compression_turn_boundary.py @@ -0,0 +1,56 @@ +from types import SimpleNamespace + +from agent.turn_preflight import compress_after_tool_results + + +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 From 2ad268b3143860f02a06037415b38eff2b522924 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 23 Sep 2026 02:50:28 -0700 Subject: [PATCH 7/9] fix(agent): re-anchor current_turn_user_idx before every request after mid-turn compaction Mid-turn compaction rebuilds `messages` without always handing back a new current_turn_user_idx: the pre-API pressure gate (run_preflight_compression) returns "continue" with the pre-compaction index, and any future compaction site would do the same. The request builder splits the replay prefix at that index and canonicalizes messages[:idx]. With a stale index the split lands inside the current turn's tool rows: canonicalization drops the assistant tool_call whose result fell past the split, the sanitizer then drops the orphaned result, and the model silently loses the turn's earlier tool output even though state.db (and the in-memory history) still hold it. persisted != sent, and a resumed session sends a different history than the live one. prepare_iteration now validates the index before every request (it must land on this turn's user row, verbatim or via its user-originated view) and re-anchors otherwise, so every mid-turn compaction path is covered in one place. The post-tool gate's own re-anchor (previous commit) keeps _persist_user_message_idx aligned immediately; this check is a no-op there. Found by the compaction E2E suite (tests/e2e/core/compaction, persisted-prefix invariant): a seeded tool-heavy session that crosses the trigger after a tool round. --- agent/turn_iteration_prep.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) 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, From fd9498122e5135106ce388f8106a5912a6f76a2c Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 23 Sep 2026 04:59:25 -0700 Subject: [PATCH 8/9] test(agent): pre-API compression mid-turn keeps this turn's tool pair on the wire Drives the real AIAgent loop: a tool round, then the pre-API gate compacts two historical rows away before the next request. That request must still end with this turn's user row, the assistant tool_call and its result. Red on origin/main and with only the post-tool re-anchor; green with the prepare_iteration check. Folds the post-tool re-anchor invariant into the same file (renamed to cover both gates). Passes the now-required current_turn_user_idx to the existing post-tool prune-wiring test call (signature change from the salvaged commit; no assertion changed). --- .../test_mid_turn_compaction_turn_boundary.py | 154 ++++++++++++++++++ ...est_post_tool_compression_turn_boundary.py | 56 ------- .../agent/test_proactive_prune_loop_wiring.py | 1 + 3 files changed, 155 insertions(+), 56 deletions(-) create mode 100644 tests/agent/test_mid_turn_compaction_turn_boundary.py delete mode 100644 tests/agent/test_post_tool_compression_turn_boundary.py 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_post_tool_compression_turn_boundary.py b/tests/agent/test_post_tool_compression_turn_boundary.py deleted file mode 100644 index b100cc1388..0000000000 --- a/tests/agent/test_post_tool_compression_turn_boundary.py +++ /dev/null @@ -1,56 +0,0 @@ -from types import SimpleNamespace - -from agent.turn_preflight import compress_after_tool_results - - -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 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 From 0b74f322e2ccbf886a33124ff00181ea1833d4a4 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 23 Sep 2026 04:36:12 -0700 Subject: [PATCH 9/9] fix(profiles): --clone copies the active memory provider's config (#120115) --clone carried memory.provider (e.g. hindsight) in config.yaml but not the provider's own config under the profile home (hindsight/config.json, mem0.json, ...), so the clone booted with the provider selected and silently unavailable. Copy the ACTIVE provider's / dir and/or .json by the same convention the dashboard memory-provider routers read, guard the name against path traversal, tighten copies to 0600 (they can hold an API key), and say so in the CLI notice. Convention-based on purpose: hindsight is a catalog plugin now, so a hook the plugin must implement could not fix the reported case, and no provider module is imported during profile create. Supersedes #43107 (credit @bionicbutterfly13 for the direction). --- hermes_cli/profile_cmd.py | 4 ++ hermes_cli/profile_memory_config.py | 71 ++++++++++++++++++++++ hermes_cli/profiles.py | 3 + tests/hermes_cli/test_profiles.py | 39 ++++++++++++ website/docs/reference/profile-commands.md | 2 +- website/docs/user-guide/profiles.md | 2 +- 6 files changed, 119 insertions(+), 2 deletions(-) create mode 100644 hermes_cli/profile_memory_config.py 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 9b52b934d4..e161d15b5f 100644 --- a/hermes_cli/profiles.py +++ b/hermes_cli/profiles.py @@ -1196,6 +1196,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/tests/hermes_cli/test_profiles.py b/tests/hermes_cli/test_profiles.py index 3808c6f11e..93ab79825e 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().strip() == "KEY=val" assert (profile_dir / "SOUL.md").read_text() == "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/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 006e31941b..345b74f282 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`)