From 4e768fe247931c360cb0f003a6493d46cf91d03e Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Sun, 19 Jul 2026 10:41:07 -0700 Subject: [PATCH] Release: gateway approval_id fallback (#6168) + update-check guard (#6180) + state-dir test isolation (#6305) (#6332) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: catch unhandled exception in POST /api/updates/check (defensive hardening) (#6180) * fix: generate non-empty approval_id when gateway approval.request omits it (#6008) (#6168) * chore: mark update-check try/except as defensive-only guard, drop #6086 linkage Per maintainer review, the try/except wrapper is defense-in-depth only — it does NOT fix #6086 (root cause is signal/process-group reaping). Updated log message and added inline comment to make this explicit. Leave #6086 open. * test: isolate state-dir probes from user state * docs(changelog): stamp #6168 approval_id, #6180 update-check guard, #6305 test isolation --------- Co-authored-by: webtecnica <75556242+webtecnica@users.noreply.github.com> Co-authored-by: webtecnica Co-authored-by: pxxD1998 <214340659+pxxD1998@users.noreply.github.com> Co-authored-by: nesquena-hermes --- CHANGELOG.md | 6 ++ api/gateway_chat.py | 3 + api/routes.py | 12 +++- tests/test_issue4449_state_dir_contract.py | 70 +++++++++++++++++----- 4 files changed, 76 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e453471..010a3db1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,12 @@ ### Fixed +- **A gateway command-approval card with no `approval_id` is now respondable.** When a gateway `approval.request` omits both `approval_id` and the legacy `id`, WebUI now synthesizes a non-empty `uuid4().hex` id instead of leaving it blank, so the approval card can be answered (matching the `#527` stable-uuid design). The downstream respond path resolves by `run_id` independently, so the synthetic id causes no mismatch; a provided or legacy id is preserved verbatim. Thanks @webtecnica. (#6168, #6008) + +- **The update check no longer lets an unexpected exception escape the route.** `POST /api/updates/check` now wraps `check_for_updates()` in a defensive guard that returns a clean `500` (and logs) instead of propagating, giving the endpoint the same exception protection as its siblings. Defensive hardening only — it does not change the success path. Thanks @webtecnica. (#6180) + +- **Test-infra: state-dir probe subprocesses can no longer touch real user state.** The `#4449` state-dir contract test spawned import-only probes that inherited the real user home, so an `import api.config` could reconcile pytest workspace defaults into the actual `settings.json`. The probes now pin `HOME`/`USERPROFILE`/`LOCALAPPDATA` to fixture-owned paths and clear inherited redirect vars, with byte-for-byte sentinel coverage (incl. native Windows paths). Test-only. Thanks @pxxD1998. (#6305) + - **A background terminal process that finishes after a WebUI restart now wakes its original session.** A `terminal(background=True, notify_on_complete=True)` child can outlive a WebUI restart, but the restarted WebUI previously started only the completion-queue drain and never recovered the checkpointed process registry — so its in-memory registry and session index started empty and the eventual `proc_*` completion had nowhere to route. WebUI startup now recovers checkpointed core background processes and rebuilds the session index (validating the owning session still exists) before the drain begins. Recovery is one-shot, lock-serialized, and fail-soft — a missing agent or corrupt checkpoint degrades gracefully without blocking startup. This complements the async-delegation delivery fix below; the ordinary `proc_*` and durable `async_delegation` paths stay separate. Thanks @allenliang2022. (#6287) - **The test suite's server fixture no longer fails on native Windows without symlink privilege.** The session-scoped test server seeds a `skills` directory by symlinking the real one; on native Windows without `SeCreateSymbolicLinkPrivilege` (Developer Mode off / non-admin) that raises `WinError 1314` and broke the whole suite. It now falls back to a read-only-preserving copy on that specific error, leaving the symlink path unchanged everywhere else. Test-infra only. Thanks @rodboev. (#6276, #6240) diff --git a/api/gateway_chat.py b/api/gateway_chat.py index be416b08..dbadfbaf 100644 --- a/api/gateway_chat.py +++ b/api/gateway_chat.py @@ -6,6 +6,7 @@ import logging import os import threading import time +import uuid import urllib.error import urllib.request from typing import Any @@ -336,6 +337,8 @@ def _gateway_runs_approval_event(payload: dict) -> dict | None: args = payload.get("args") if isinstance(payload.get("args"), (list, dict)) else [] run_id = str(payload.get("run_id") or "").strip() approval_id = str(payload.get("approval_id") or payload.get("id") or "").strip() + if not approval_id: + approval_id = uuid.uuid4().hex risk = str(payload.get("risk_level") or "high").strip() choices = payload.get("choices") if isinstance(payload.get("choices"), list) else [] allow_permanent = payload.get("allow_permanent") diff --git a/api/routes.py b/api/routes.py index 91d0b73a..4494619d 100644 --- a/api/routes.py +++ b/api/routes.py @@ -13907,7 +13907,17 @@ def handle_post(handler, parsed) -> bool: channel = settings.get("update_channel") from api.updates import check_for_updates - return j(handler, check_for_updates(force=force, include_agent=include_agent_updates, channel=channel)) + logger.info("checking for updates (force=%s, include_agent=%s, channel=%s)", force, include_agent_updates, channel) + # Defensive-only guard: wrap check_for_updates() for consistent + # exception protection across all route handlers. Does NOT fix #6086 + # (root cause is likely signal/process-group reaping, per maintainer analysis). + try: + payload = check_for_updates(force=force, include_agent=include_agent_updates, channel=channel) + except Exception: + logger.exception("update check failed unexpectedly (defensive guard caught exception)") + return bad(handler, "Update check failed, see server log for details", status=500) + logger.info("update check completed") + return j(handler, payload) if parsed.path == "/api/extensions/toggle": from api.extensions import ExtensionToggleError, set_extension_user_enabled diff --git a/tests/test_issue4449_state_dir_contract.py b/tests/test_issue4449_state_dir_contract.py index 82671698..a0f37730 100644 --- a/tests/test_issue4449_state_dir_contract.py +++ b/tests/test_issue4449_state_dir_contract.py @@ -12,6 +12,7 @@ truly isolated import and can't pollute the test process. from __future__ import annotations +import json import os import subprocess import sys @@ -19,20 +20,27 @@ from pathlib import Path REPO_ROOT = Path(__file__).resolve().parents[1] -_PROBE = ( - "import api.config as c; " - "print(c.STATE_DIR)" -) +_PROBE = "import api.config as c; print(c.STATE_DIR)" -def _state_dir_for_env(**env_overrides) -> Path: +def _state_dir_for_env(platform_home: Path, **env_overrides) -> Path: """Import api.config in a fresh subprocess under the given env and return the resolved STATE_DIR it computes. None-valued overrides unset the var.""" env = dict(os.environ) - # Start from a clean slate for the two vars under test so the parent - # process's pytest values don't bleed in. + # Keep every probe inside fixture-owned platform paths. Inheriting the real + # user home lets an import-only probe reconcile pytest workspace defaults + # into the user's settings.json before it prints STATE_DIR. + env["HOME"] = str(platform_home) + env["USERPROFILE"] = str(platform_home) + env["LOCALAPPDATA"] = str(platform_home / "AppData" / "Local") + # Start from a clean slate for the vars that can redirect state or trigger + # startup settings reconciliation in the child process. env.pop("HERMES_HOME", None) + env.pop("HERMES_BASE_HOME", None) env.pop("HERMES_WEBUI_STATE_DIR", None) + env.pop("HERMES_WEBUI_TEST_STATE_DIR", None) + env.pop("HERMES_WEBUI_DEFAULT_WORKSPACE", None) + env.pop("HERMES_CONFIG_PATH", None) for key, value in env_overrides.items(): if value is None: env.pop(key, None) @@ -50,20 +58,51 @@ def _state_dir_for_env(**env_overrides) -> Path: return Path(out.stdout.strip()).resolve() -def _platform_default_state_dir() -> Path: +def _platform_default_state_dir(platform_home: Path) -> Path: """What STATE_DIR resolves to with HERMES_HOME + STATE_DIR both unset.""" - return _state_dir_for_env() + return _state_dir_for_env(platform_home) + + +def test_state_dir_probe_does_not_mutate_platform_default_settings( + tmp_path, monkeypatch +): + """The subprocess probe must not reconcile pytest defaults into user state.""" + platform_home = tmp_path / "platform-home" + if os.name == "nt": + settings_file = ( + platform_home / "AppData" / "Local" / "hermes" / "webui" / "settings.json" + ) + else: + settings_file = platform_home / ".hermes" / "webui" / "settings.json" + settings_file.parent.mkdir(parents=True) + persisted_workspace = tmp_path / "persisted-workspace" + persisted_workspace.mkdir() + original = ( + json.dumps({"default_workspace": str(persisted_workspace)}) + "\n" + ).encode() + settings_file.write_bytes(original) + + monkeypatch.setenv("HOME", str(platform_home)) + monkeypatch.setenv("USERPROFILE", str(platform_home)) + monkeypatch.setenv( + "HERMES_WEBUI_DEFAULT_WORKSPACE", str(tmp_path / "pytest-workspace") + ) + + state_dir = _platform_default_state_dir(platform_home) + + assert settings_file.read_bytes() == original + assert state_dir == settings_file.parent.resolve() def test_config_state_dir_defaults_to_hermes_home_webui(tmp_path): hermes_home = tmp_path / ".hermes" / "profiles" / "isolated" hermes_home.mkdir(parents=True) - state_dir = _state_dir_for_env(HERMES_HOME=hermes_home) + state_dir = _state_dir_for_env(tmp_path / "platform-home", HERMES_HOME=hermes_home) assert state_dir == (hermes_home / "webui").resolve() -def test_config_state_dir_unchanged_for_normal_install_hermes_home_unset(): +def test_config_state_dir_unchanged_for_normal_install_hermes_home_unset(tmp_path): """Backward-compat: with HERMES_HOME unset, STATE_DIR stays at the platform default `<~/.hermes>/webui` — a normal install's state must NOT relocate (the #4449/#4454 state-dir move only affects an explicitly-set HERMES_HOME). @@ -71,12 +110,14 @@ def test_config_state_dir_unchanged_for_normal_install_hermes_home_unset(): Cross-check: the unset-HERMES_HOME result must NOT equal the result of pointing HERMES_HOME at an arbitrary other base — i.e. the default is genuinely the platform home, not whatever the test environment injected.""" - default = _platform_default_state_dir() + platform_home = tmp_path / "platform-home" + default = _platform_default_state_dir(platform_home) assert default.name == "webui" # Pointing HERMES_HOME elsewhere produces a DIFFERENT dir, proving the unset # case resolves to the platform default rather than echoing an injected base. - elsewhere = _state_dir_for_env(HERMES_HOME="/tmp/hermes-4449-elsewhere-base") - assert elsewhere == Path("/tmp/hermes-4449-elsewhere-base/webui").resolve() + elsewhere_base = tmp_path / "elsewhere-base" + elsewhere = _state_dir_for_env(platform_home, HERMES_HOME=elsewhere_base) + assert elsewhere == (elsewhere_base / "webui").resolve() assert default != elsewhere @@ -88,6 +129,7 @@ def test_config_state_dir_explicit_override_takes_precedence(tmp_path): explicit = tmp_path / "custom-state" state_dir = _state_dir_for_env( + tmp_path / "platform-home", HERMES_HOME=hermes_home, HERMES_WEBUI_STATE_DIR=explicit, )