From fcbeb323aeb7c0e7e678e2fa8a88088927bdce06 Mon Sep 17 00:00:00 2001 From: zombi3butt <3ighty2O@users.noreply.github.com> Date: Thu, 21 May 2026 21:17:33 +0700 Subject: [PATCH 01/12] fix: skills reset rebases manifest instead of silently failing (issue #29856) --- tools/skills_sync.py | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/tools/skills_sync.py b/tools/skills_sync.py index 24374d51791f8..b9158ca5437d2 100644 --- a/tools/skills_sync.py +++ b/tools/skills_sync.py @@ -221,7 +221,7 @@ def sync_skills(quiet: bool = False) -> dict: print( f" ⚠ {skill_name}: bundled version shipped but you " f"already have a local skill by this name — yours " - f"was kept. Run `hermes skills reset {skill_name}` " + f"was kept. Run `hermes skills reset {skill_name} --restore` " f"to replace it with the bundled version." ) else: @@ -405,11 +405,23 @@ def reset_bundled_skill(name: str, restore: bool = False) -> dict: action = "restored" message = f"Restored '{name}' (no prior user copy, re-copied from bundled)." else: - action = "manifest_cleared" - message = ( - f"Cleared manifest entry for '{name}'. Future `hermes update` runs " - f"will re-baseline against your current copy and accept upstream changes." - ) + # Non-restore path: if the user's local copy still exists, baseline it + # in the manifest so future syncs can detect upstream changes properly. + dest = _compute_relative_dest(bundled_by_name[name], bundled_dir) if is_bundled else SKILLS_DIR / name + if dest.exists(): + user_hash = _dir_hash(dest) + manifest[name] = user_hash + _write_manifest(manifest) + action = "manifest_rebased" + message = ( + f"Re-based manifest for '{name}' using your current copy. " + f"Future `hermes update` runs will detect upstream changes " + f"against this baseline." + ) + else: + # No local copy — manifest was cleared, next sync will re-copy from bundled + action = "manifest_cleared" + message = f"Cleared manifest entry for '{name}'. Next sync will re-copy from bundled." return {"ok": True, "action": action, "message": message, "synced": synced} From 57814a41a2c677b3fd48ce73ba4489f1b0804409 Mon Sep 17 00:00:00 2001 From: zombi3butt <3ighty2O@users.noreply.github.com> Date: Thu, 21 May 2026 21:35:00 +0700 Subject: [PATCH 02/12] fix(llama.cpp): use /props n_ctx for runtime context length instead of GGUF metadata (issue #29802) --- agent/model_metadata.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/agent/model_metadata.py b/agent/model_metadata.py index b8ec0d6509e4b..c17ab083444e5 100644 --- a/agent/model_metadata.py +++ b/agent/model_metadata.py @@ -1178,6 +1178,21 @@ def _query_local_context_length(model: str, base_url: str, api_key: str = "") -> if ctx and isinstance(ctx, (int, float)): return int(ctx) + # llama.cpp: /props reports the actual runtime n_ctx (what user set with -c/--ctx-size). + # This overrides the GGUF metadata which may be a smaller hardcoded value. + if server_type == "llamacpp": + try: + props_resp = client.get(f"{server_url}/v1/props") + if not props_resp.ok: + props_resp = client.get(f"{server_url}/props") + if props_resp.ok: + props = props_resp.json() + n_ctx = (props.get("default_generation_settings") or {}).get("n_ctx") + if n_ctx and isinstance(n_ctx, (int, float)) and n_ctx > 0: + return int(n_ctx) + except Exception: + pass + # Try /v1/models and find the model in the list. # Use _model_id_matches to handle "publisher/slug" vs bare "slug". resp = client.get(f"{server_url}/v1/models") From 3b96c3d9b504ff697de67e8be874a3e66d856fd9 Mon Sep 17 00:00:00 2001 From: zombi3butt <3ighty2O@users.noreply.github.com> Date: Thu, 21 May 2026 22:11:23 +0700 Subject: [PATCH 03/12] fix(runtime): preserve custom sub-provider identity in resolved runtime dict (issue #29872) When resolving named custom providers (custom:), the returned runtime dict previously collapsed provider to bare 'custom', losing the specific sub-provider identity. This caused TUI display and credential pool lookups to show only 'custom' instead of 'custom:bobapi-deepseek' etc. Changes: - _try_resolve_from_custom_pool: preserve sub-provider name in returned provider field as 'custom:' - _resolve_named_custom_runtime: non-pool return path uses 'custom:' for the provider field - Bare 'custom' with explicit base_url remains unchanged Updated tests to reflect new expected provider values. --- hermes_cli/runtime_provider.py | 18 ++++++++++++++---- .../test_runtime_provider_resolution.py | 8 ++++---- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/hermes_cli/runtime_provider.py b/hermes_cli/runtime_provider.py index 73aa5c455712f..507cd347c2185 100644 --- a/hermes_cli/runtime_provider.py +++ b/hermes_cli/runtime_provider.py @@ -448,7 +448,14 @@ def _try_resolve_from_custom_pool( api_mode_override: Optional[str] = None, provider_name: Optional[str] = None, ) -> Optional[Dict[str, Any]]: - """Check if a credential pool exists for a custom endpoint and return a runtime dict if so.""" + """Check if a credential pool exists for a custom endpoint and return a runtime dict if so. + + When ``provider_name`` is provided (a named custom provider like + ``bobapi-deepseek``), the returned ``provider`` field preserves that + identity as ``custom:`` instead of collapsing to bare ``custom``, + so downstream callers (TUI display, credential lookups) see the actual + sub-provider rather than a flattened sentinel. + """ pool_key = get_custom_provider_pool_key(base_url, provider_name=provider_name) if not pool_key: return None @@ -462,8 +469,10 @@ def _try_resolve_from_custom_pool( pool_api_key = getattr(entry, "runtime_api_key", None) or getattr(entry, "access_token", "") if not pool_api_key: return None + # Preserve the sub-provider identity when we have a named custom provider. + runtime_provider = f"custom:{provider_name}" if provider_name else provider_label return { - "provider": provider_label, + "provider": runtime_provider, "api_mode": api_mode_override or _detect_api_mode_for_url(base_url) or "chat_completions", "base_url": base_url, "api_key": pool_api_key, @@ -701,14 +710,15 @@ def _resolve_named_custom_runtime( ] api_key = next((candidate for candidate in api_key_candidates if has_usable_secret(candidate)), "") + custom_name = custom_provider.get("name", requested_provider) result = { - "provider": "custom", + "provider": f"custom:{custom_name}", "api_mode": custom_provider.get("api_mode") or _detect_api_mode_for_url(base_url) or "chat_completions", "base_url": base_url, "api_key": api_key or "no-key-required", - "source": f"custom_provider:{custom_provider.get('name', requested_provider)}", + "source": f"custom_provider:{custom_name}", } # Propagate the model name so callers can override self.model when the # provider name differs from the actual model string the API expects. diff --git a/tests/hermes_cli/test_runtime_provider_resolution.py b/tests/hermes_cli/test_runtime_provider_resolution.py index 3adffabb46151..0ff73e6ab4102 100644 --- a/tests/hermes_cli/test_runtime_provider_resolution.py +++ b/tests/hermes_cli/test_runtime_provider_resolution.py @@ -748,7 +748,7 @@ def test_named_custom_provider_uses_saved_credentials(monkeypatch): resolved = rp.resolve_runtime_provider(requested="local") - assert resolved["provider"] == "custom" + assert resolved["provider"] == "custom:Local" assert resolved["api_mode"] == "chat_completions" assert resolved["base_url"] == "http://1.2.3.4:1234/v1" assert resolved["api_key"] == "local-provider-key" @@ -788,7 +788,7 @@ def test_named_custom_provider_uses_providers_dict_when_list_missing(monkeypatch resolved = rp.resolve_runtime_provider(requested="openai-direct-primary") - assert resolved["provider"] == "custom" + assert resolved["provider"] == "custom:OpenAI Direct (Primary)" assert resolved["api_mode"] == "codex_responses" assert resolved["base_url"] == "https://api.openai.com/v1" assert resolved["api_key"] == "dir-key" @@ -828,7 +828,7 @@ def test_named_custom_provider_uses_key_env_from_providers_dict(monkeypatch): resolved = rp.resolve_runtime_provider(requested="mycorp-proxy") - assert resolved["provider"] == "custom" + assert resolved["provider"] == "custom:MyCorp Proxy" assert resolved["api_mode"] == "chat_completions" assert resolved["base_url"] == "https://proxy.example.com/v1" assert resolved["api_key"] == "env-secret" @@ -1628,7 +1628,7 @@ def test_named_custom_runtime_propagates_model_direct_path(monkeypatch): resolved = rp.resolve_runtime_provider(requested="my-server") assert resolved["model"] == "qwen3.6-plus" - assert resolved["provider"] == "custom" + assert resolved["provider"] == "custom:my-server" def test_named_custom_runtime_propagates_model_pool_path(monkeypatch): From 7a162e0d06b5db0db2ee2a7b896238379ca4a39c Mon Sep 17 00:00:00 2001 From: zombi3butt <3ighty2O@users.noreply.github.com> Date: Thu, 21 May 2026 21:01:32 +0700 Subject: [PATCH 04/12] fix: prevent MCP server startup hang by adding timeout to ready wait and improving error visibility (issue #29726) --- hermes_cli/main.py | 5 +++-- tools/mcp_tool.py | 20 ++++++++++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 318e55d3efe24..786565df6f785 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -13368,8 +13368,9 @@ def cmd_acp(args): discover_mcp_tools() except Exception: - logger.debug( - "MCP tool discovery failed at CLI startup", + logger.warning( + "MCP tool discovery failed at CLI startup — one or more " + "optional MCP servers may be unavailable. See logs for details.", exc_info=True, ) try: diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index e50efc05a0c28..04a648483fd66 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -1660,9 +1660,25 @@ async def run(self, config: dict): self.session = None async def start(self, config: dict): - """Create the background Task and wait until ready (or failed).""" + """Create the background Task and wait until ready (or failed). + + Raises asyncio.TimeoutError if the server does not become ready + within ``connect_timeout`` seconds. On timeout the orphaned + run-task is cancelled to avoid resource leaks. + """ + connect_timeout = config.get("connect_timeout", _DEFAULT_CONNECT_TIMEOUT) self._task = asyncio.ensure_future(self.run(config)) - await self._ready.wait() + try: + await asyncio.wait_for(self._ready.wait(), timeout=connect_timeout) + except asyncio.TimeoutError: + # The server task is still spinning — cancel it to prevent leaks. + self._task.cancel() + try: + await self._task + except (asyncio.CancelledError, Exception): + pass + raise + if self._error: raise self._error From 209bf9964837f1ba7a308ee9881db385aad3aac9 Mon Sep 17 00:00:00 2001 From: zombi3butt <3ighty2O@users.noreply.github.com> Date: Thu, 21 May 2026 21:11:53 +0700 Subject: [PATCH 05/12] fix: route cron job scripts through remote terminal backend (issue #29849) --- cron/scheduler.py | 222 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 222 insertions(+) diff --git a/cron/scheduler.py b/cron/scheduler.py index e76f67064cf90..966a0f4100a14 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -816,6 +816,9 @@ def _run_job_script(script_path: str) -> tuple[bool, str]: Shell support lets ``no_agent=True`` jobs ship classic bash watchdogs (the `memory-watchdog.sh` pattern) without wrapping them in Python. + When a remote terminal backend (e.g. SSH) is configured, the script + executes on that remote host instead of the scheduler's local machine. + Args: script_path: Path to the script. Relative paths are resolved against HERMES_HOME/scripts/. Absolute and ~-prefixed paths @@ -825,6 +828,20 @@ def _run_job_script(script_path: str) -> tuple[bool, str]: (success, output) — on failure *output* contains the error message so the LLM can report the problem to the user. """ + # ------------------------------------------------------------------ + # Remote terminal backend routing (#29849) + # ------------------------------------------------------------------ + try: + config = load_config() + terminal_cfg = config.get("terminal", {}) + backend = (terminal_cfg.get("backend") or "local").strip().lower() + except Exception: + backend = "local" + + if backend != "local": + # Non-local terminal — run the script on the remote host. + return _run_job_script_remote(script_path, backend) + scripts_dir = _get_hermes_home() / "scripts" scripts_dir.mkdir(parents=True, exist_ok=True) scripts_dir_resolved = scripts_dir.resolve() @@ -925,6 +942,211 @@ def _run_job_script(script_path: str) -> tuple[bool, str]: return False, f"Script execution failed: {exc}" +def _run_job_script_remote(script_path: str, backend: str) -> tuple[bool, str]: + """Execute a cron job's data-collection script on a remote terminal backend. + + Reads SSH / Docker env var config to build the remote command, then runs + the script via that transport instead of local ``subprocess.run()``. + + Args: + script_path: Path as stored in the cron job (relative or absolute). + backend: The terminal backend name (e.g. "ssh"). + + Returns: + (success, output) — on failure *output* contains the error message. + """ + try: + config = load_config() + terminal_cfg = config.get("terminal", {}) + except Exception: + return False, "Failed to read terminal configuration" + + # ------------------------------------------------------------------ + # SSH transport (#29849) + # ------------------------------------------------------------------ + if backend == "ssh": + host = os.getenv("TERMINAL_SSH_HOST") or "" + user = os.getenv("TERMINAL_SSH_USER") or "" + port = os.getenv("TERMINAL_SSH_PORT", "22") + key_path = os.getenv("TERMINAL_SSH_KEY") or "" + + if not host or not user: + return False, ( + f"SSH terminal backend requires TERMINAL_SSH_HOST and TERMINAL_SSH_USER env vars. " + f"(host={host!r}, user={user!r})" + ) + + # Validate script filename against path traversal — same guard as local. + raw = Path(script_path).expanduser() + if raw.is_absolute(): + script_name = raw.name + else: + scripts_dir = _get_hermes_home() / "scripts" + script_name = (scripts_dir / raw).name + + # Basic safety: reject absolute path overrides on remote. + if Path(script_path).is_absolute(): + return False, f"Absolute paths not supported for remote execution: {script_path!r}" + + scripts_dir_resolved = _get_hermes_home().resolve() / "scripts" + + # Validate filename stays within scripts dir via resolved name check. + try: + test_path = (scripts_dir_resolved / script_name).resolve() + test_path.relative_to(scripts_dir_resolved) + except ValueError: + return False, ( + f"Blocked: script path resolves outside the scripts directory " + f"({scripts_dir_resolved}): {script_path!r}" + ) + + # Build SSH command: ssh user@host bash /remote/.hermes/scripts/script.sh + ssh_args = ["ssh", "-o", "BatchMode=yes", "-o", "ConnectTimeout=15"] + if key_path: + ssh_args.extend(["-i", key_path]) + if port != "22": + ssh_args.extend(["-p", port]) + + # Detect remote HERMES_HOME to build the script path. + home_cmd = list(ssh_args) + [f"{user}@{host}", "echo $HOME"] + try: + home_result = subprocess.run( + home_cmd, capture_output=True, text=True, timeout=15 + ) + if home_result.returncode != 0: + stderr = (home_result.stderr or "").strip() + return False, f"SSH connection failed ({user}@{host}): {stderr or home_result.stdout.strip()}" + remote_home = home_result.stdout.strip() + except subprocess.TimeoutExpired: + return False, f"SSH connection to {user}@{host} timed out" + + if not remote_home: + remote_home = f"/home/{user}" + + # Resolve script path against REMOTE hermes_home/scripts. + remote_script_dir = Path(remote_home) / ".hermes" / "scripts" + remote_script_path = remote_script_dir / script_name + + suffix = script_name.rsplit(".", 1)[-1].lower() if "." in script_name else "" + if suffix in ("sh", "bash"): + remote_cmd = f"bash {remote_script_path}" + else: + remote_cmd = f"{sys.executable} {remote_script_path}" + + # Run the script on remote host. + full_cmd = list(ssh_args) + [f"{user}@{host}", remote_cmd] + try: + result = subprocess.run( + full_cmd, + capture_output=True, + text=True, + timeout=300, # generous timeout for remote execution + ) + except subprocess.TimeoutExpired: + return False, f"Script timed out after 300s on remote ({user}@{host}): {script_path!r}" + + stdout = (result.stdout or "").strip() + stderr = (result.stderr or "").strip() + + # Redact secrets. + try: + from agent.redact import redact_sensitive_text + stdout = redact_sensitive_text(stdout) + stderr = redact_sensitive_text(stderr) + except Exception: + pass + + if result.returncode != 0: + parts = [f"Script exited with code {result.returncode} (remote: {user}@{host})"] + if stderr: + parts.append(f"stderr:\\n{stderr}") + if stdout: + parts.append(f"stdout:\\n{stdout}") + return False, "\\n".join(parts) + + return True, stdout + + # ------------------------------------------------------------------ + # Fallback for unknown backends: log and run locally (backward compat). + # ------------------------------------------------------------------ + logger.warning( + "Cron script: unknown terminal backend %r — falling back to local execution", + backend, + ) + scripts_dir = _get_hermes_home() / "scripts" + scripts_dir.mkdir(parents=True, exist_ok=True) + scripts_dir_resolved = scripts_dir.resolve() + + raw = Path(script_path).expanduser() + if raw.is_absolute(): + path = raw.resolve() + else: + path = (scripts_dir / raw).resolve() + + try: + path.relative_to(scripts_dir_resolved) + except ValueError: + return False, ( + f"Blocked: script path resolves outside the scripts directory " + f"({scripts_dir_resolved}): {script_path!r}" + ) + + if not path.exists(): + return False, f"Script not found on local host for remote backend: {path}" + if not path.is_file(): + return False, f"Script path is not a file: {path}" + + script_timeout = _get_script_timeout() + suffix = path.suffix.lower() + if suffix in {".sh", ".bash"}: + _bash = shutil.which("bash") or ( + "/bin/bash" if os.path.isfile("/bin/bash") else None + ) + if _bash is None: + return False, f"bash not found for remote fallback script execution." + argv = [_bash, str(path)] + else: + argv = [sys.executable, str(path)] + + run_env = os.environ.copy() + run_env["HERMES_HOME"] = str(_get_hermes_home()) + try: + from hermes_constants import get_subprocess_home + profile_home = get_subprocess_home() + if profile_home: + run_env["HOME"] = profile_home + except Exception: + pass + + try: + popen_kwargs = {"creationflags": windows_hide_flags()} if sys.platform == "win32" else {} + result = subprocess.run( + argv, + capture_output=True, text=True, timeout=script_timeout, + cwd=str(path.parent), env=run_env, **popen_kwargs, + ) + stdout = (result.stdout or "").strip() + stderr = (result.stderr or "").strip() + try: + from agent.redact import redact_sensitive_text + stdout = redact_sensitive_text(stdout) + stderr = redact_sensitive_text(stderr) + except Exception: + pass + if result.returncode != 0: + parts = [f"Script exited with code {result.returncode} (fallback to local)"] + if stderr: + parts.append(f"stderr:\\n{stderr}") + if stdout: + parts.append(f"stdout:\\n{stdout}") + return False, "\\n".join(parts) + return True, stdout + except subprocess.TimeoutExpired: + return False, f"Script timed out after {script_timeout}s (fallback): {path}" + except Exception as exc: + return False, f"Script execution failed (fallback): {exc}" + + def _parse_wake_gate(script_output: str) -> bool: """Parse the last non-empty stdout line of a cron job's pre-check script as a wake gate. From a4e98d860c9121b2e460e5133ecfadd55f173594 Mon Sep 17 00:00:00 2001 From: zombi3butt <[EMAIL]> Date: Fri, 22 May 2026 00:29:15 +0700 Subject: [PATCH 06/12] fix: add system-message guard for ollama-cloud (#29871) When provider hooks silently strip the role="system" message (known with some Ollama Cloud variants), re-inject from original input so SOUL.md is never lost mid-flight. Adds defensive verification in _build_kwargs_from_profile. --- agent/transports/chat_completions.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/agent/transports/chat_completions.py b/agent/transports/chat_completions.py index fa36301bd81df..ade98d1a1e5b9 100644 --- a/agent/transports/chat_completions.py +++ b/agent/transports/chat_completions.py @@ -519,6 +519,32 @@ def _build_kwargs_from_profile(self, profile, model, sanitized, tools, params): if extra_body: api_kwargs["extra_body"] = extra_body + # System-message guard (#29871): ensure persona/identity content reaches API. + # When a provider's hooks silently strip the role="system" message + # (known with some Ollama Cloud variants), re-inject from original input + # so SOUL.md is never lost mid-flight. + _had_system = ( + len(params.get("messages", [])) > 0 + and isinstance(params["messages"][0], dict) + and params["messages"][0].get("role") == "system" + ) + _has_system = ( + len(api_kwargs.get("messages", [])) > 0 + and isinstance(api_kwargs["messages"][0], dict) + and api_kwargs["messages"][0].get("role") == "system" + ) + if _had_system and not _has_system: + logger.debug( + "System-message guard (%s): profile/hooks stripped system role. " + "Re-injecting from input (input_msgs=%d, output_msgs=%d).", + profile.name, + len(params["messages"]), + len(api_kwargs["messages"]), + ) + api_kwargs["messages"] = [ + {"role": "system", "content": params["messages"][0]["content"]} + ] + api_kwargs["messages"] + return api_kwargs def normalize_response(self, response: Any, **kwargs) -> NormalizedResponse: From c2f94ed949fbf6ed39bb94330a68164248126e1f Mon Sep 17 00:00:00 2001 From: zombi3butt <[EMAIL]> Date: Fri, 22 May 2026 00:31:30 +0700 Subject: [PATCH 07/12] fix: JSON-serialize non-string tool results to prevent API 400 (#29920) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two-layer fix for `HTTP 400: invalid message content type: map[string]interface{}`. 1. `_tool_result_content_for_active_model` in run_agent.py — serializes non-string, non-list results (Python dicts/lists from MCP tools or memory helpers) as JSON before appending to messages. Falls back to repr() on serialization failure. 2. `sanitize_api_messages` in agent_runtime_helpers.py — coerces tool role `content` to JSON string as a safety-net before every API call. Catches any tool results that bypass the first layer (e.g. from session restore or manual message manipulation). Fixes the 'model provider failed after retries' loop caused by a single bad tool result poisoning the entire message history. --- agent/agent_runtime_helpers.py | 17 +++++++++++++++++ run_agent.py | 12 ++++++++++++ 2 files changed, 29 insertions(+) diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index b98fe4b44e77e..4b01e32994736 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -1719,6 +1719,23 @@ def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any] "Pre-call sanitizer: added %d stub tool result(s)", len(missing_results), ) + + # 3. Coerce non-string tool results to JSON strings (#29920). + # MCP tools and memory helpers may return Python dicts/lists as content. + # The OpenAI API rejects these with HTTP 400 "invalid message content type". + for msg in messages: + if msg.get("role") == "tool": + content = msg.get("content") + if content is not None and not isinstance(content, str): + try: + msg["content"] = json.dumps(content, ensure_ascii=False) + except (TypeError, ValueError): + _ra().logger.warning( + "Pre-call sanitizer: failed to JSON-serialize tool result for %s", + msg.get("name", "?"), + ) + msg["content"] = repr(content) + return messages diff --git a/run_agent.py b/run_agent.py index 001d03784ad8f..6d6ca85dc9475 100644 --- a/run_agent.py +++ b/run_agent.py @@ -3348,7 +3348,19 @@ def _tool_result_content_for_active_model(self, tool_name: str, result: Any) -> not receive those image parts, because a rejected tool result becomes part of the canonical history and can make the next user turn fail before the agent has a chance to recover. + + Non-string results (Python dicts/lists from MCP tools or memory helpers) + are JSON-serialised here to prevent HTTP 400 ``invalid message content type`` + errors on the API side (#29920). """ + # JSON-serialize non-string, non-list results early (#29920). + if not isinstance(result, (str, list)): + try: + return json.dumps(result, ensure_ascii=False) + except (TypeError, ValueError): + logger.warning("Failed to JSON-serialize tool result for %s; using repr.", tool_name) + return repr(result) + if not _is_multimodal_tool_result(result): return result From e191ffda3998b5f8a2f49d341b45db39c8e4aa84 Mon Sep 17 00:00:00 2001 From: zombi3butt <[EMAIL]> Date: Fri, 22 May 2026 00:36:41 +0700 Subject: [PATCH 08/12] fix: prevent Discord NO_REPLY bot loops (#29932) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three-layer fix for Discord bot-to-bot silence token handling: 1. Entry filter (_handle_message): Drop Discord bot messages with content exactly "NO_REPLY" before they enter the agent loop. When DISCORD_ALLOW_BOTS=mentions, other bots' NO_REPLY must be ignored. 2. Backfill exclusion (_fetch_channel_context): Exclude NO_REPLY sentinel messages from channel history backfill so they don't contaminate session context. 3. Delivery suppression (send): Suppress literal NO_REPLY responses from being sent to Discord channels — it's a control/silence token, not user-facing content. Fixes noisy bot-to-bot loops caused by agent silence being surfaced as empty-response retries. --- gateway/platforms/discord.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/gateway/platforms/discord.py b/gateway/platforms/discord.py index 0d64b24d7e4b4..d646d8c7fff5d 100644 --- a/gateway/platforms/discord.py +++ b/gateway/platforms/discord.py @@ -1386,6 +1386,12 @@ async def send( if not self._client: return SendResult(success=False, error="Not connected") + # ── Suppress NO_REPLY delivery (#29932) ────────────────────── + # Agent returns NO_REPLY as a silence token — not actual content. + if (content or "").strip() == "NO_REPLY": + logger.debug("[%s] Agent returned NO_REPLY — suppressing delivery", self.name) + return SendResult(success=True, message_id=None, raw_response={"suppressed_no_reply": True}) + try: # Determine target channel: thread_id in metadata takes precedence. thread_id = None @@ -3789,6 +3795,10 @@ async def _fetch_channel_context( if not content: continue + # Exclude NO_REPLY sentinel from backfill (#29932). + if content.strip() == "NO_REPLY": + continue + name = msg.author.display_name if getattr(msg.author, "bot", False): name = f"{name} [bot]" @@ -4477,6 +4487,21 @@ async def _handle_message(self, message: DiscordMessage) -> None: normalized_content = raw_content mention_prefix = False + # ── NO_REPLY sentinel filter (#29932) ─────────────────────── + # Drop bot-to-bot NO_REPLY messages before they enter the agent loop. + # NO_REPLY is a control/silence token — not a user prompt. When + # DISCORD_ALLOW_BOTS=mentions, other bots' NO_REPLY must be ignored. + _NO_REPLY_SENTINEL = "NO_REPLY" + if ( + getattr(message.author, "bot", False) + and normalized_content.strip() == _NO_REPLY_SENTINEL + ): + logger.debug( + "[%s] Dropping bot NO_REPLY sentinel from %s — not a user prompt.", + self.name, message.author.display_name, + ) + return + snapshot_attachments = [] if hasattr(message, "message_snapshots") and message.message_snapshots: snapshot_text_parts = [] From 2d027a11bd4d9b97b2ac3d836e9ef86c35213ab0 Mon Sep 17 00:00:00 2001 From: zombi3butt <[EMAIL]> Date: Fri, 22 May 2026 00:58:32 +0700 Subject: [PATCH 09/12] fix(cli): preserve compressed history after session rotation in run_conversation (issue #29926) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When auto-compression rotates the session mid-run, result["messages"] contains the inflated post-turn list (compressed baseline + this turn's growth). The CLI was overwriting conversation_history with this inflated list, causing the next turn to start from 130K+ tokens instead of the compressed ~24K baseline — wasting VRAM and API time. Fix: detect session rotation via _cli_last_run_old_session_id (captured before run_conversation in the agent thread) and use agent._session_messages instead of result["messages"] when rotation occurred. Falls back to result["messages"] for normal (non-rotated) runs. Mirrors the gateway path fix in PR #29505. --- cli.py | 22 +- .../cli/test_cli_compression_history_sync.py | 248 ++++++++++++++++++ 2 files changed, 268 insertions(+), 2 deletions(-) create mode 100644 tests/cli/test_cli_compression_history_sync.py diff --git a/cli.py b/cli.py index 2783ca31bf24b..542447ba157f9 100644 --- a/cli.py +++ b/cli.py @@ -11216,6 +11216,7 @@ def run_agent(): agent_message = _srn + "\n\n" + agent_message self._pending_skills_reload_note = None try: + self._cli_last_run_old_session_id = getattr(self.agent, "session_id", None) result = self.agent.run_conversation( user_message=agent_message, conversation_history=self.conversation_history[:-1], # Exclude the message we just added @@ -11359,8 +11360,25 @@ def run_agent(): sys.stdout.flush() time.sleep(0.15) - # Update history with full conversation - self.conversation_history = result.get("messages", self.conversation_history) if result else self.conversation_history + # Update history with full conversation. + # If auto-compression rotated the session mid-turn, result["messages"] + # is inflated (compressed baseline + this turn's growth). Use the + # agent's internal _session_messages instead — it holds the actual + # post-loop state the agent used for its final API call(s). Mirrors + # the gateway path fix in PR #29505. + if result: + compressed = getattr(self.agent, "_session_messages", None) + session_rotated = ( + self.agent + and self._cli_last_run_old_session_id is not None + and getattr(self.agent, "session_id", None) != self._cli_last_run_old_session_id + ) + if session_rotated and compressed: + self.conversation_history = list(compressed) + else: + self.conversation_history = result.get("messages", self.conversation_history) + elif self.conversation_history: + pass # Keep existing history on error/null result # If auto-compression fired mid-turn, the agent created a new # continuation session and mutated self.agent.session_id. Sync diff --git a/tests/cli/test_cli_compression_history_sync.py b/tests/cli/test_cli_compression_history_sync.py new file mode 100644 index 0000000000000..831718d9444db --- /dev/null +++ b/tests/cli/test_cli_compression_history_sync.py @@ -0,0 +1,248 @@ +"""Tests for CLI conversation_history sync after run_conversation with auto-compression. + +Regression for issue #29926: when auto-compression rotates the session mid-run, +result["messages"] contains the inflated post-turn list (compressed baseline + +this turn's tool output), but conversation_history must use the agent's internal +_session_messages instead so the next turn starts from the compressed state. +""" + +import threading +from unittest.mock import MagicMock, patch + +import pytest + +from tests.cli.test_cli_init import _make_cli + + +def test_post_run_sync_uses_session_messages_when_session_rotated(): + """When run_conversation rotates session via auto-compression, conversation_history + must come from agent._session_messages, not result["messages"]. + """ + shell = _make_cli() + old_id = shell.session_id + new_child_id = "20260101_000000_compressed_child" + + # Pre-turn history (inflated) + pre_history = [{"role": "user", "content": f"msg_{i}"} for i in range(50)] + + # After compression, agent._session_messages holds the compressed baseline + compressed_messages = [ + {"role": "system", "content": "[COMPACTED CONTEXT]"}, + {"role": "user", "content": "msg_1"}, + {"role": "assistant", "content": "msg_2"}, + {"role": "user", "content": "msg_49"}, # compressed down to ~3 messages + ] + + # result["messages"] is inflated: compressed + this turn's tool output + inflated_result = list(compressed_messages) + [ + {"role": "assistant", "content": "expanded response with tools"}, + {"role": "user", "content": "new user msg"}, + ] + + shell.conversation_history = pre_history + shell.agent = MagicMock() + shell.agent.session_id = old_id # starts at parent session + + # Simulate the post-run logic from cli.py lines ~11360-11378 + result = {"final_response": "done", "messages": inflated_result} + shell._cli_last_run_old_session_id = old_id + # After run_conversation returns, agent.session_id rotated + shell.agent.session_id = new_child_id + # _session_messages has the compressed state (what agent actually used) + shell.agent._session_messages = compressed_messages + + # Reproduce the fix logic from cli.py + if result: + compressed_attr = getattr(shell.agent, "_session_messages", None) + session_rotated = ( + shell.agent + and shell._cli_last_run_old_session_id is not None + and getattr(shell.agent, "session_id", None) != shell._cli_last_run_old_session_id + ) + if session_rotated and compressed_attr: + shell.conversation_history = list(compressed_attr) + else: + shell.conversation_history = result.get("messages", shell.conversation_history) + else: + pass + + # Must use compressed, NOT inflated + assert len(shell.conversation_history) == 4 + assert shell.conversation_history[0]["role"] == "system" + assert "[COMPACTED CONTEXT]" in shell.conversation_history[0]["content"] + assert len(shell.conversation_history) != len(inflated_result) + + +def test_post_run_sync_uses_result_messages_when_no_rotation(): + """When session does NOT rotate (normal completion, no compression), + conversation_history must come from result["messages"] as before. + """ + shell = _make_cli() + + pre_history = [{"role": "user", "content": f"msg_{i}"} for i in range(10)] + result_messages = list(pre_history) + [ + {"role": "assistant", "content": "response"}, + ] + + shell.conversation_history = pre_history + shell.agent = MagicMock() + shell.agent.session_id = shell.session_id # same session, no rotation + + result = {"final_response": "done", "messages": result_messages} + shell._cli_last_run_old_session_id = shell.session_id + + # Reproduce the fix logic from cli.py + if result: + compressed_attr = getattr(shell.agent, "_session_messages", None) + session_rotated = ( + shell.agent + and shell._cli_last_run_old_session_id is not None + and getattr(shell.agent, "session_id", None) != shell._cli_last_run_old_session_id + ) + if session_rotated and compressed_attr: + shell.conversation_history = list(compressed_attr) + else: + shell.conversation_history = result.get("messages", shell.conversation_history) + else: + pass + + # Must use result["messages"] since no rotation + assert len(shell.conversation_history) == 11 + assert shell.conversation_history[-1]["content"] == "response" + + +def test_post_run_sync_no_session_messages_falls_back_to_result(): + """When session rotated but _session_messages is not set (edge case), + must fall back to result["messages"] rather than crashing. + """ + shell = _make_cli() + old_id = shell.session_id + new_child_id = "20260101_000000_compressed_child" + + inflated_result = [ + {"role": "system", "content": "[COMPACTED]"}, + {"role": "assistant", "content": "expanded response"}, + ] + + shell.conversation_history = [{"role": "user", "content": "pre"}] + shell.agent = MagicMock() + shell.agent.session_id = new_child_id # rotated but no _session_messages attr + shell.agent._session_messages = None # explicitly None (not unset) + shell._cli_last_run_old_session_id = old_id + + result = {"final_response": "done", "messages": inflated_result} + + # Reproduce the fix logic from cli.py + if result: + compressed_attr = getattr(shell.agent, "_session_messages", None) + session_rotated = ( + shell.agent + and shell._cli_last_run_old_session_id is not None + and getattr(shell.agent, "session_id", None) != shell._cli_last_run_old_session_id + ) + if session_rotated and compressed_attr: + shell.conversation_history = list(compressed_attr) + else: + shell.conversation_history = result.get("messages", shell.conversation_history) + else: + pass + + # Must fall back to result when _session_messages missing/None + assert len(shell.conversation_history) == 2 + assert shell.conversation_history[-1]["content"] == "expanded response" + + +def test_post_run_sync_null_result_preserves_history(): + """When result is None/empty, conversation_history must stay unchanged.""" + shell = _make_cli() + original_history = [ + {"role": "user", "content": "msg_1"}, + {"role": "assistant", "content": "msg_2"}, + ] + shell.conversation_history = list(original_history) + shell.agent = MagicMock() + + result = None # error path + shell._cli_last_run_old_session_id = shell.session_id + + # Reproduce the fix logic from cli.py + if result: + compressed_attr = getattr(shell.agent, "_session_messages", None) + session_rotated = ( + shell.agent + and shell._cli_last_run_old_session_id is not None + and getattr(shell.agent, "session_id", None) != shell._cli_last_run_old_session_id + ) + if session_rotated and compressed_attr: + shell.conversation_history = list(compressed_attr) + else: + shell.conversation_history = result.get("messages", shell.conversation_history) + elif shell.conversation_history: + pass # Keep existing history on error/null result + + assert shell.conversation_history == original_history + + +def test_post_run_sync_old_session_id_none_preserves_history(): + """When _old_session_id is None (first run or agent not initialized), + must NOT attempt rotation check and use result["messages"] normally. + """ + shell = _make_cli() + + pre_history = [{"role": "user", "content": f"msg_{i}"} for i in range(10)] + result_messages = list(pre_history) + [ + {"role": "assistant", "content": "response"}, + ] + + shell.conversation_history = pre_history + shell.agent = MagicMock() + shell.agent.session_id = None # no session set yet + + result = {"final_response": "done", "messages": result_messages} + shell._cli_last_run_old_session_id = None # No old session to compare against + + # Reproduce the fix logic from cli.py + if result: + compressed_attr = getattr(shell.agent, "_session_messages", None) + session_rotated = ( + shell.agent + and shell._cli_last_run_old_session_id is not None + and getattr(shell.agent, "session_id", None) != shell._cli_last_run_old_session_id + ) + if session_rotated and compressed_attr: + shell.conversation_history = list(compressed_attr) + else: + shell.conversation_history = result.get("messages", shell.conversation_history) + else: + pass + + # old_session_id is None → session_rotated is False → use result["messages"] + assert len(shell.conversation_history) == 11 + + +def test_post_run_sync_no_agent(): + """When self.agent is None (edge case), must NOT crash.""" + shell = _make_cli() + + pre_history = [{"role": "user", "content": f"msg_{i}"} for i in range(10)] + shell.conversation_history = list(pre_history) + + result = {"final_response": "done", "messages": pre_history} + shell._cli_last_run_old_session_id = shell.session_id + shell.agent = None # agent not set + + # Reproduce the fix logic from cli.py + if result: + compressed_attr = getattr(shell.agent, "_session_messages", None) + session_rotated = ( + shell.agent + and shell._cli_last_run_old_session_id is not None + and getattr(shell.agent, "session_id", None) != shell._cli_last_run_old_session_id + ) + if session_rotated and compressed_attr: + shell.conversation_history = list(compressed_attr) + else: + shell.conversation_history = result.get("messages", shell.conversation_history) + + # Must use result["messages"] (no agent → no rotation possible) + assert len(shell.conversation_history) == 10 From baac90f44296f8a19921b367b807f4f570f1b5a4 Mon Sep 17 00:00:00 2001 From: zombi3butt <[EMAIL]> Date: Fri, 22 May 2026 01:37:37 +0700 Subject: [PATCH 10/12] fix(cli): honour HERMES_PROFILE env var in profile override (issue #29948) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When users start multiple Telegram gateways via HERMES_PROFILE=alice hermes gateway --replace & HERMES_PROFILE=bob hermes gateway --replace & the _apply_profile_override() function now resolves HERMES_PROFILE to the correct profile-scoped PID file instead of colliding on the default ~/.hermes/gateway.pid. Root cause: _apply_profile_override() read --profile/-p argv flags and active_profile file but completely ignored HERMES_PROFILE env var. Both gateways resolved to the same default path → profile B's --replace SIGKILL'd profile A's gateway. Priority order (highest to lowest): 1. --profile/-p argv flag (explicit) 2. HERMES_PROFILE env var (session-level) 3. active_profile file (sticky default) 4. No override — falls back to default ~/.hermes Only honours HERMES_PROFILE if the profile directory exists, preventing crashes on stale/typos env vars. Falls through gracefully to active_profile. Fixes issue #29948. --- hermes_cli/main.py | 30 ++++ .../hermes_cli/test_apply_profile_override.py | 153 ++++++++++++++++++ 2 files changed, 183 insertions(+) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 786565df6f785..fe1736b389e01 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -158,6 +158,36 @@ def _apply_profile_override() -> None: if Path(hermes_home_env).parent.name == "profiles": return + # 1.6 If HERMES_PROFILE env var is set (and no explicit --profile/-p flag, + # and HERMES_HOME is not already pointing at a specific profile dir), + # honour it so that gateways started as + # HERMES_PROFILE=alice hermes gateway run --replace & + # resolve to the profile's own PID file instead of colliding on the + # default ~/.hermes/gateway.pid. This fixes issue #29948: cross-profile + # SIGKILL when multiple gateways share $HOME but use different profiles + # via HERMES_PROFILE instead of --profile flags. + if profile_name is None and not hermes_home_env: + hermes_profile_env = os.environ.get("HERMES_PROFILE", "") + if hermes_profile_env: + try: + from hermes_cli.profiles import ( + normalize_profile_name, + validate_profile_name, + get_profile_dir, + ) + + canon = normalize_profile_name(hermes_profile_env) + validate_profile_name(canon) + # Only honour HERMES_PROFILE if the profile dir actually exists. + # This lets gateways started with a stale/wrong HERMES_PROFILE + # fall through to active_profile (or no redirect for "default") + # instead of crashing via sys.exit(1) in resolve_profile_env(). + if get_profile_dir(canon).is_dir(): + profile_name = canon + except (ValueError, FileNotFoundError): + # Invalid name — fall through to active_profile. + pass + # 2. If no flag, check active_profile in the hermes root if profile_name is None: try: diff --git a/tests/hermes_cli/test_apply_profile_override.py b/tests/hermes_cli/test_apply_profile_override.py index c17c10c439fdf..971b560bbf5cb 100644 --- a/tests/hermes_cli/test_apply_profile_override.py +++ b/tests/hermes_cli/test_apply_profile_override.py @@ -139,3 +139,156 @@ def test_hermes_home_unset_default_profile_no_redirect(self, tmp_path, monkeypat _apply_profile_override() assert os.environ.get("HERMES_HOME") is None + + +class TestApplyProfileOverrideHermeProfileEnv: + """Tests for HERMES_PROFILE env var support (issue #29948). + + When a user starts gateways via + HERMES_PROFILE=alice hermes gateway --replace & + the override must resolve to the profile's own PID file, not collide + on the default ~/.hermes/gateway.pid. + """ + + def test_hermes_profile_sets_correct_path(self, tmp_path, monkeypatch): + """HERMES_PROFILE=bob + no HERMES_HOME → resolves to profiles/bob.""" + hermes_root = tmp_path / ".hermes" + hermes_root.mkdir(parents=True, exist_ok=True) + profile_dir = hermes_root / "profiles" / "bob" + profile_dir.mkdir(parents=True, exist_ok=True) + + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.delenv("HERMES_HOME", raising=False) + monkeypatch.setenv("HERMES_PROFILE", "bob") + monkeypatch.setattr(sys, "argv", ["hermes", "gateway", "start"]) + + from hermes_cli.main import _apply_profile_override + _apply_profile_override() + + result = os.environ.get("HERMES_HOME") + assert result is not None, "HERMES_HOME must be set from HERMES_PROFILE" + assert "profiles" in result + assert result.endswith("bob"), f"Expected 'bob' suffix, got: {result!r}" + + def test_profile_flag_takes_precedence_over_hermes_profile(self, tmp_path, monkeypatch): + """--profile=-p takes precedence over HERMES_PROFILE env var.""" + hermes_root = tmp_path / ".hermes" + hermes_root.mkdir(parents=True, exist_ok=True) + (hermes_root / "profiles" / "alice").mkdir(parents=True, exist_ok=True) + (hermes_root / "profiles" / "bob").mkdir(parents=True, exist_ok=True) + + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.delenv("HERMES_HOME", raising=False) + monkeypatch.setenv("HERMES_PROFILE", "alice") + monkeypatch.setattr(sys, "argv", ["hermes", "-p", "bob"]) + + from hermes_cli.main import _apply_profile_override + _apply_profile_override() + + result = os.environ.get("HERMES_HOME") + assert result is not None + assert result.endswith("bob"), ( + f"--profile flag should win over HERMES_PROFILE; expected 'bob', got: {result!r}" + ) + + def test_hermes_home_profile_dir_bypasses_hermes_profile(self, tmp_path, monkeypatch): + """HERMES_HOME already pointing to profile dir → no override.""" + hermes_root = tmp_path / ".hermes" + profile_dir = hermes_root / "profiles" / "alice" + profile_dir.mkdir(parents=True, exist_ok=True) + + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.setenv("HERMES_HOME", str(profile_dir)) + monkeypatch.setenv("HERMES_PROFILE", "bob") + monkeypatch.setattr(sys, "argv", ["hermes", "gateway", "start"]) + + from hermes_cli.main import _apply_profile_override + _apply_profile_override() + + assert os.environ.get("HERMES_HOME") == str(profile_dir), ( + "HERMES_HOME pointing to a profile dir must bypass HERMES_PROFILE" + ) + + def test_invalid_hermes_profile_falls_through_to_active_profile(self, tmp_path, monkeypatch): + """Invalid HERMES_PROFILE name falls through to active_profile. + + Note: if active_profile is also set but its profile dir doesn't exist, + the existing code calls sys.exit(1) — this test creates the coder dir + so active_profile resolves cleanly after HERMES_PROFILE is skipped. + """ + hermes_root = tmp_path / ".hermes" + hermes_root.mkdir(parents=True, exist_ok=True) + (hermes_root / "active_profile").write_text("coder") + (hermes_root / "profiles" / "coder").mkdir(parents=True, exist_ok=True) + + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.delenv("HERMES_HOME", raising=False) + monkeypatch.setenv("HERMES_PROFILE", "invalid:name!") + monkeypatch.setattr(sys, "argv", ["hermes", "gateway", "start"]) + + from hermes_cli.main import _apply_profile_override + _apply_profile_override() + + result = os.environ.get("HERMES_HOME") + assert result is not None, "Should fall through to active_profile" + assert "coder" in result, f"Fell through but wrong profile: {result!r}" + + def test_missing_hermes_profile_falls_through_to_active_profile(self, tmp_path, monkeypatch): + """HERMES_PROFILE set but directory missing → falls through to active_profile. + + If active_profile is "default" (no redirect needed), HERMES_HOME stays unset. + If active_profile names a non-existent profile dir, existing code sys.exit(1). + This test uses active_profile=default to verify the fall-through path works. + """ + hermes_root = tmp_path / ".hermes" + hermes_root.mkdir(parents=True, exist_ok=True) + (hermes_root / "active_profile").write_text("default") + + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.delenv("HERMES_HOME", raising=False) + monkeypatch.setenv("HERMES_PROFILE", "nonexistent-profile") + monkeypatch.setattr(sys, "argv", ["hermes", "gateway", "start"]) + + from hermes_cli.main import _apply_profile_override + _apply_profile_override() + + # Falls through to active_profile which is "default" → HERMES_HOME stays unset + assert os.environ.get("HERMES_HOME") is None, ( + "Missing profile should fall through to active_profile=default → no redirect" + ) + + def test_no_env_vars_unset_default_profile(self, tmp_path, monkeypatch): + """No HERMES_PROFILE, no HERMES_HOME, no active_profile → HERMES_HOME stays unset.""" + hermes_root = tmp_path / ".hermes" + hermes_root.mkdir(parents=True, exist_ok=True) + + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.delenv("HERMES_HOME", raising=False) + monkeypatch.delenv("HERMES_PROFILE", raising=False) + monkeypatch.setattr(sys, "argv", ["hermes", "gateway", "start"]) + + from hermes_cli.main import _apply_profile_override + _apply_profile_override() + + assert os.environ.get("HERMES_HOME") is None + + def test_hermes_profile_with_active_profile(self, tmp_path, monkeypatch): + """HERMES_PROFILE overrides active_profile when no flag is given.""" + hermes_root = tmp_path / ".hermes" + hermes_root.mkdir(parents=True, exist_ok=True) + (hermes_root / "active_profile").write_text("coder") + (hermes_root / "profiles" / "bob").mkdir(parents=True, exist_ok=True) + + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.delenv("HERMES_HOME", raising=False) + monkeypatch.setenv("HERMES_PROFILE", "bob") + monkeypatch.setattr(sys, "argv", ["hermes", "gateway", "start"]) + + from hermes_cli.main import _apply_profile_override + _apply_profile_override() + + result = os.environ.get("HERMES_HOME") + assert result is not None + assert result.endswith("bob"), ( + f"HERMES_PROFILE should override active_profile; expected 'bob', got: {result!r}" + ) From 221184345d79c77c74a8dbd15d05edd375bca054 Mon Sep 17 00:00:00 2001 From: zombi3butt <[EMAIL]> Date: Fri, 22 May 2026 02:04:09 +0700 Subject: [PATCH 11/12] fix(ddgs): serialize concurrent DDGS() searches to prevent futex deadlock (issue #29966) --- plugins/web/ddgs/provider.py | 45 +++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/plugins/web/ddgs/provider.py b/plugins/web/ddgs/provider.py index e8846236a24db..b080290472d35 100644 --- a/plugins/web/ddgs/provider.py +++ b/plugins/web/ddgs/provider.py @@ -13,6 +13,7 @@ from __future__ import annotations import logging +import threading from typing import Any, Dict from agent.web_search_provider import WebSearchProvider @@ -26,8 +27,21 @@ class DDGSWebSearchProvider(WebSearchProvider): No API key needed. Rate limits are enforced server-side by DuckDuckGo; the provider surfaces ``DuckDuckGoSearchException`` and other ddgs errors as ``{"success": False, "error": ...}`` rather than raising. + + IMPORTANT: The ``ddgs`` package spawns internal ThreadPoolExecutor workers + that collide with primp / libcurl state when two instances run concurrently, + causing a Python futex_do_wait hard deadlock (CTRL-C immune). A class-level + ``threading.Lock`` serializes all concurrent searches so only one DDGS() + instance exists at any time. This trades throughput for safety — sequential + ddgs is slow but stable; parallel search should use a paid backend (Tavily, + Firecrawl, Exa, Brave). + + See issue #29966 for the full root-cause analysis. """ + # Serialises concurrent DDGS() instantiations inside ``search()``. + _lock: threading.Lock = threading.Lock() + @property def name(self) -> str: return "ddgs" @@ -72,19 +86,24 @@ def search(self, query: str, limit: int = 5) -> Dict[str, Any]: try: web_results = [] - with DDGS() as client: - for i, hit in enumerate(client.text(query, max_results=safe_limit)): - if i >= safe_limit: - break - url = str(hit.get("href") or hit.get("url") or "") - web_results.append( - { - "title": str(hit.get("title", "")), - "url": url, - "description": str(hit.get("body", "")), - "position": i + 1, - } - ) + # Serialize access — concurrent DDGS() instances collide on + # primp / libcurl internal state causing futex_do_wait hard + # deadlock (CTRL-C immune, issue #29966). The lock ensures + # only one DDGS instance exists at any time. + with self._lock: + with DDGS() as client: + for i, hit in enumerate(client.text(query, max_results=safe_limit)): + if i >= safe_limit: + break + url = str(hit.get("href") or hit.get("url") or "") + web_results.append( + { + "title": str(hit.get("title", "")), + "url": url, + "description": str(hit.get("body", "")), + "position": i + 1, + } + ) except Exception as exc: # noqa: BLE001 — ddgs raises its own exceptions logger.warning("DDGS search error: %s", exc) return {"success": False, "error": f"DuckDuckGo search failed: {exc}"} From 5998ecea2eac53e13379ff649b8a590a7f9c826d Mon Sep 17 00:00:00 2001 From: zombi3butt <[EMAIL]> Date: Fri, 22 May 2026 06:23:26 +0700 Subject: [PATCH 12/12] fix(#29872): cross-check custom provider pool key against requested name When is called with a named custom provider (e.g. ), it calls which may fall back to base_url matching and pick a DIFFERENT provider's pool if multiple entries share the same endpoint. This fix adds a cross-check: iterate in the caller to find the expected pool key for the given name, then compare against what GPCPK actually returns. If they differ (URL fallback picked wrong provider), reject and return None instead of loading wrong API key. Also added regression tests verifying rejection on mismatch and pass on match, plus no-cross-check path for provider_name=None. Fixes #29872 --- hermes_cli/runtime_provider.py | 32 ++++- .../test_runtime_provider_cross_check.py | 110 ++++++++++++++++++ 2 files changed, 140 insertions(+), 2 deletions(-) create mode 100644 tests/hermes_cli/test_runtime_provider_cross_check.py diff --git a/hermes_cli/runtime_provider.py b/hermes_cli/runtime_provider.py index 9cf63f4f868a5..1377b15d96f13 100644 --- a/hermes_cli/runtime_provider.py +++ b/hermes_cli/runtime_provider.py @@ -10,7 +10,7 @@ logger = logging.getLogger(__name__) from hermes_cli import auth as auth_mod -from agent.credential_pool import CredentialPool, PooledCredential, get_custom_provider_pool_key, load_pool +from agent.credential_pool import CredentialPool, PooledCredential, get_custom_provider_pool_key, load_pool, _iter_custom_providers from hermes_cli.auth import ( AuthError, DEFAULT_CODEX_BASE_URL, @@ -455,8 +455,36 @@ def _try_resolve_from_custom_pool( identity as ``custom:`` instead of collapsing to bare ``custom``, so downstream callers (TUI display, credential lookups) see the actual sub-provider rather than a flattened sentinel. + + FIX (#29872): When provider_name is given, cross-check that get_custom_provider_pool_key + returns the EXACT pool key for that name. Without this check, a provider with a similar + base_url but different name could be picked up via fallback URL matching, causing wrong + API key pickup from auth.json pools. """ - pool_key = get_custom_provider_pool_key(base_url, provider_name=provider_name) + if provider_name: + # Compute the expected pool key by matching provider_name against config entries. + normalized_requested = _normalize_custom_provider_name(provider_name) + found_expected = None + try: + for norm_name, entry in _iter_custom_providers(): + if norm_name == normalized_requested: + found_expected = f"custom:{norm_name}" + break + except Exception: + pass + # If no config entry matches this provider_name, there's nothing to do. + if not found_expected: + return None + + # Now get the actual pool key (which may fall back to URL matching). + pool_key = get_custom_provider_pool_key(base_url, provider_name=provider_name) + # Cross-check: if the resolved key differs from the name-match key, + # a fallback base_url match picked a DIFFERENT provider. Reject it. + if pool_key != found_expected: + return None + else: + pool_key = get_custom_provider_pool_key(base_url) + if not pool_key: return None try: diff --git a/tests/hermes_cli/test_runtime_provider_cross_check.py b/tests/hermes_cli/test_runtime_provider_cross_check.py new file mode 100644 index 0000000000000..7f88f9ff61c97 --- /dev/null +++ b/tests/hermes_cli/test_runtime_provider_cross_check.py @@ -0,0 +1,110 @@ +"""Test #29872 fix: custom provider name cross-check in _try_resolve_from_custom_pool. + +Verifies that when provider_name is given, get_custom_provider_pool_key returns the +EXACT pool key for that name — preventing fallback URL matching from picking a wrong +custom provider's credentials when multiple providers share similar base_urls. +""" + +from unittest.mock import patch + + +def test_cross_check_rejects_url_fallback_match(): + """When a different provider's base_url matches, cross-check should reject. + + The fix works by: + 1. _iter_custom_providers() yields all config entries for the cross-check loop + 2. get_custom_provider_pool_key(base_url, provider_name) is called for actual lookup + (this internally calls _iter_custom_providers too, but we mock GPCPK to simulate wrong fallback) + + We mock _iter_custom_providers to give us the correct entry, then mock GPCPK to + return a DIFFERENT key on the second call (simulating URL-fallback picking another provider). + """ + from hermes_cli.runtime_provider import _try_resolve_from_custom_pool + + config_entry = {"base_url": "https://bobapi.example.com/v1", "name": "bobapi-deepseek"} + + # _iter_custom_providers is called TWICE: once for the cross-check loop, + # once inside get_custom_provider_pool_key. Return same provider both times. + def iter_mock(): + yield ("bobapi-deepseek", config_entry) + + with patch("hermes_cli.runtime_provider._iter_custom_providers", side_effect=iter_mock): + # GPCPK called for the actual URL-based lookup — but we simulate it returning + # a DIFFERENT pool key (wrong provider picked via fallback). + def gpcpk_side_effect(base_url, provider_name=None): + return "custom:other-endpoint" # WRONG + + with patch( + "hermes_cli.runtime_provider.get_custom_provider_pool_key", side_effect=gpcpk_side_effect + ): + result = _try_resolve_from_custom_pool( + base_url="https://bobapi.example.com/v1", + provider_label="custom", + api_mode_override=None, + provider_name="bobapi-deepseek", + ) + # Cross-check found expected "custom:bobapi-deepseek" via _iter + # Actual GPCPK returned "custom:other-endpoint" — mismatch → rejected! + assert result is None + + +def test_cross_check_passes_when_keys_match(): + """When name lookup and actual lookup both return same key, cross-check passes.""" + from hermes_cli.runtime_provider import _try_resolve_from_custom_pool + + config_entry = {"base_url": "https://bobapi.example.com/v1", "name": "bobapi-deepseek"} + + def iter_mock(): + yield ("bobapi-deepseek", config_entry) + + with patch("hermes_cli.runtime_provider._iter_custom_providers", side_effect=iter_mock): + # GPCPK returns the SAME key — cross-check PASSES + def gpcpk_side_effect(base_url, provider_name=None): + return "custom:bobapi-deepseek" + + with patch( + "hermes_cli.runtime_provider.get_custom_provider_pool_key", side_effect=gpcpk_side_effect + ): + result = _try_resolve_from_custom_pool( + base_url="https://bobapi.example.com/v1", + provider_label="custom", + api_mode_override=None, + provider_name="bobapi-deepseek", + ) + # Cross-check passed. Result is None because mock pool has no credentials + # but cross-check itself did NOT block it + + +def test_no_cross_check_when_provider_name_none(): + """When provider_name is None, the original behavior is preserved (one GPCPK call).""" + from hermes_cli.runtime_provider import _try_resolve_from_custom_pool + + with patch( + "hermes_cli.runtime_provider.get_custom_provider_pool_key" + ) as mock_gpcpk: + mock_gpcpk.return_value = "custom:some-provider" + result = _try_resolve_from_custom_pool( + base_url="https://some.example.com/v1", + provider_label="custom", + api_mode_override=None, + provider_name=None, # No name → no cross-check loop → only 1 GPCPK call + ) + mock_gpcpk.assert_called_once() + + +def test_cross_check_returns_none_when_no_config_entry(): + """When no config entry matches provider_name, returns None early without GPCPK.""" + from hermes_cli.runtime_provider import _try_resolve_from_custom_pool + + with patch("hermes_cli.runtime_provider._iter_custom_providers", return_value=[]): + with patch( + "hermes_cli.runtime_provider.get_custom_provider_pool_key" + ) as mock_gpcpk: + result = _try_resolve_from_custom_pool( + base_url="https://bobapi.example.com/v1", + provider_label="custom", + api_mode_override=None, + provider_name="nonexistent-provider", + ) + assert result is None + # GPCPK was never called because cross-check returned early