From 42a6d761d2dc7dc2b618c26ca10983896a5186de Mon Sep 17 00:00:00 2001 From: Teknium Date: Mon, 24 Aug 2026 00:11:47 -0700 Subject: [PATCH] fix(bot-relay): add shutil.which step to CLI resolution and pin utf-8 decoding on delivery subprocess MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Salvage hardening on top of #93601 (with #93597 covering the same core mechanisms) for #93590: - _hermes_cli(): after the venv-sibling check (hermes.exe on win32), try shutil.which('hermes') before the bare-name fallback, so environments with a PATH but no venv sibling resolve exactly what an interactive shell would. Platform test switched os.name -> sys.platform ('win32') per repo convention. - tui_gateway/methods_bot_relay.py deliver: pin encoding='utf-8', errors='replace' on both subprocess.run sites — without them the child's UTF-8 output is decoded with the locale codec (cp1252/GBK on Windows), mangling non-ASCII replies or raising on undecodable bytes. - Regression tests: shutil.which resolution step, bare-name fallback with which=None, and encoding-pin assertions in the deliver transport test. Refs #93590, #93597, #93601 --- tests/tools/test_bot_relay_windows_paths.py | 18 +++++++++++++++++- tests/tui_gateway/test_bot_relay_methods.py | 7 +++++++ tools/bot_relay.py | 16 ++++++++++++---- tui_gateway/methods_bot_relay.py | 4 ++++ 4 files changed, 40 insertions(+), 5 deletions(-) diff --git a/tests/tools/test_bot_relay_windows_paths.py b/tests/tools/test_bot_relay_windows_paths.py index 01064b313f..e6123a7371 100644 --- a/tests/tools/test_bot_relay_windows_paths.py +++ b/tests/tools/test_bot_relay_windows_paths.py @@ -1,4 +1,4 @@ -"""Windows-path viability and venv CLI resolution for bot relay (#93590). +r"""Windows-path viability and venv CLI resolution for bot relay (#93590). Two failures on a Windows desktop install talking to a remote gateway: @@ -102,10 +102,26 @@ def test_local_delivery_resolves_sibling_hermes(tmp_path, monkeypatch): assert argv[argv.index("--query-file") + 1] == "query.json" +def test_local_delivery_uses_shutil_which_when_no_sibling(tmp_path, monkeypatch): + """Without a venv sibling, a PATH hit (shutil.which) wins next — + interactive shells keep resolving exactly what they resolve today.""" + empty = tmp_path / "nowhere" + empty.mkdir(parents=True) + monkeypatch.setattr("sys.executable", str(empty / "python")) + which_hit = str(tmp_path / "usr-local-bin" / "hermes") + monkeypatch.setattr( + bot_relay.shutil, "which", lambda name: which_hit if name == "hermes" else None + ) + + argv = bot_relay.local_delivery_command("ops", "query.json") + assert argv[0] == which_hit + + def test_local_delivery_falls_back_to_bare_name(tmp_path, monkeypatch): empty = tmp_path / "nowhere" empty.mkdir(parents=True) monkeypatch.setattr("sys.executable", str(empty / "python")) + monkeypatch.setattr(bot_relay.shutil, "which", lambda name: None) argv = bot_relay.local_delivery_command("ops", "query.json") assert argv[0] == "hermes" diff --git a/tests/tui_gateway/test_bot_relay_methods.py b/tests/tui_gateway/test_bot_relay_methods.py index 7b70ec7b86..fb6d357744 100644 --- a/tests/tui_gateway/test_bot_relay_methods.py +++ b/tests/tui_gateway/test_bot_relay_methods.py @@ -70,6 +70,7 @@ def test_deliver_validates_profile_and_runs_transport(home, monkeypatch): def _fake_run(argv, **kwargs): calls["argv"] = argv + calls["kwargs"] = kwargs return _Proc() monkeypatch.setattr("subprocess.run", _fake_run) @@ -77,6 +78,12 @@ def test_deliver_validates_profile_and_runs_transport(home, monkeypatch): srv._methods["bot_relay.deliver"](1, {"profile": "ops", "message": "ping"}) ) assert out["reply"] == "pong from ops" + # Decoding is pinned (#93590 sibling defect): without encoding= the + # child's UTF-8 output is decoded with the locale codec — cp1252/GBK on + # Windows — mangling non-ASCII replies; errors="replace" keeps a bad + # byte from raising instead of delivering. + assert calls["kwargs"]["encoding"] == "utf-8" + assert calls["kwargs"]["errors"] == "replace" argv = calls["argv"] # argv[0] may be a resolved venv path (#93590) — match by basename. assert argv[1:3] == ["-p", "ops"] diff --git a/tools/bot_relay.py b/tools/bot_relay.py index 8abc53762e..08c6d3b044 100644 --- a/tools/bot_relay.py +++ b/tools/bot_relay.py @@ -38,6 +38,7 @@ import logging import os import re import shlex +import shutil import sys import tempfile import time @@ -542,12 +543,19 @@ def _hermes_cli() -> str: entrypoint. A bare ``"hermes"`` relies on PATH, which is exactly what service contexts (systemd units, desktop launchers, non-login SSH shells) do not provide, so delivery died with ENOENT there (#93590). - Falls back to the bare name when no sibling exists (e.g. running from - a source tree without an installed script), preserving PATH lookup. + When no sibling exists (e.g. running from a source tree without an + installed script), a ``shutil.which`` lookup runs next — it honors + whatever PATH the process does have — before falling back to the bare + name, preserving today's behavior for interactive shells. """ exe = Path(sys.executable or "") - sibling = exe.parent / ("hermes.exe" if os.name == "nt" else "hermes") - return str(sibling) if sibling.is_file() else "hermes" + sibling = exe.parent / ("hermes.exe" if sys.platform == "win32" else "hermes") + if sibling.is_file(): + return str(sibling) + found = shutil.which("hermes") + if found: + return found + return "hermes" def local_delivery_command(profile: str, query_file: str) -> list[str]: diff --git a/tui_gateway/methods_bot_relay.py b/tui_gateway/methods_bot_relay.py index 0e853f857f..b1baa40964 100644 --- a/tui_gateway/methods_bot_relay.py +++ b/tui_gateway/methods_bot_relay.py @@ -125,6 +125,8 @@ def _(rid, params: dict) -> dict: local_delivery_command(resolved, tmp), capture_output=True, text=True, + encoding="utf-8", + errors="replace", timeout=600, ) if proc.returncode != 0: @@ -147,6 +149,8 @@ def _(rid, params: dict) -> dict: local_delivery_command(resolved, tmp), capture_output=True, text=True, + encoding="utf-8", + errors="replace", timeout=600, ) finally: