fix(bot-relay): add shutil.which step to CLI resolution and pin utf-8 decoding on delivery subprocess
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
This commit is contained in:
@@ -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"
|
||||
|
||||
@@ -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"]
|
||||
|
||||
@@ -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]:
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user