fix(bot-relay): Windows path SyntaxError in waiter + PATH-less delivery ENOENT

tests/tools/test_bot_relay_windows_paths.py

@@ -0,0 +1,147 @@
+"""Windows-path viability and venv CLI resolution for bot relay (#93590).
+
+Two failures on a Windows desktop install talking to a remote gateway:
+
+1. ``waiter_command`` embeds the reply path into generated ``python -c``
+   source with ``!r``. repr escapes each backslash, but the Windows
+   execution layer the waiter runs under folds ``\\`` back to ``\`` —
+   ``\\U`` in ``C:\\Users\\...`` then parses as a unicode escape and
+   SyntaxErrors the whole script. The raw-string prefix keeps the folded
+   single backslash a literal; POSIX paths contain no backslashes, so it
+   is a no-op there, and ``\\'`` inside a raw literal still cannot
+   terminate the string, so the injection defense from #93091's
+   python -c hardening is unchanged.
+
+2. ``local_delivery_command`` hardcoded ``"hermes"``, relying on PATH —
+   which service contexts (systemd units, desktop launchers, non-login
+   SSH shells) do not provide, so delivery died with ENOENT. It now
+   resolves the CLI next to this gateway's own interpreter (the venv
+   bin/Scripts sibling), falling back to the bare name. The #93091
+   turn-lock recognition in bot_mode_dm matches the CLI element by
+   basename so resolved absolute paths (and ``hermes.exe``) still take
+   the per-profile lock.
+"""
+
+import ast
+import shlex
+from pathlib import Path
+
+import tools.bot_mode_dm as bot_mode_dm
+import tools.bot_relay as bot_relay
+
+
+ENV = {"id": "d" * 32, "target_handle": "researcher", "target_connection": "ssh-vps"}
+
+
+def _waiter_code(root, env=None) -> str:
+    cmd = bot_relay.waiter_command(root, env or ENV)
+    parts = shlex.split(cmd)
+    return parts[parts.index("-c") + 1]
+
+
+def test_waiter_windows_path_compiles_after_backslash_folding():
+    """A Windows reply path must survive the execution layer folding the
+    repr-escaped double backslash back to a single one — the exact shape
+    that SyntaxErrored with ``\\U`` on #93590's reporter setup."""
+    code = _waiter_code("C:\\Users\\joshu\\.hermes")
+    assert "C:" in code  # sanity: the Windows path made it into the payload
+    folded = code.replace("\\\\", "\\")
+    # Raw literals: `p = r'C:\Users\joshu\...'` — no unicode-escape crash.
+    compile(folded, "<waiter>", "exec")
+
+
+def test_waiter_posix_path_and_label_values_roundtrip():
+    """On POSIX (backslash-free paths) the raw prefix changes nothing."""
+    root = Path("/tmp/hermes-home")
+    code = _waiter_code(root)
+    assigns = {
+        t.targets[0].id: t.value
+        for t in ast.parse(code).body
+        if isinstance(t, ast.Assign) and isinstance(t.targets[0], ast.Name)
+    }
+    expected = str(root / "bot_relay" / "replies" / f"{ENV['id']}.json")
+    assert assigns["p"].value == expected
+    assert assigns["label"].value == "@researcher on ssh-vps"
+    # The literals are raw-prefixed in the generated source.
+    assert "\np = r'" in code
+    assert "\nlabel = r'" in code
+
+
+def test_waiter_raw_prefix_keeps_injection_defense():
+    """Hostile roster fields must stay data under the raw prefix too."""
+    inj = {
+        "id": "e" * 32,
+        "target_handle": "researcher",
+        "target_connection": "x'); __import__('sys').exit(2); print('x",
+    }
+    code = _waiter_code(Path("/tmp/hermes-home"), inj)
+    compile(code, "<waiter>", "exec")
+    calls = [
+        n.func.id
+        for n in ast.walk(ast.parse(code))
+        if isinstance(n, ast.Call) and isinstance(n.func, ast.Name)
+    ]
+    # The generated waiter only calls str/print/compile-free builtins by
+    # name; the payload's __import__ must remain a string literal, not a
+    # live call — parse it back and confirm it stayed data.
+    assert "__import__" not in calls
+    assert "x'); __import__('sys').exit(2); print('x" in code
+
+
+def test_local_delivery_resolves_sibling_hermes(tmp_path, monkeypatch):
+    bin_dir = tmp_path / "venv" / "bin"
+    bin_dir.mkdir(parents=True)
+    sibling = bin_dir / "hermes"
+    sibling.touch()
+    sibling.chmod(0o755)
+    monkeypatch.setattr("sys.executable", str(bin_dir / "python"))
+
+    argv = bot_relay.local_delivery_command("ops", "query.json")
+    assert argv[0] == str(sibling)
+    assert argv[1:3] == ["-p", "ops"]
+    assert argv[argv.index("--query-file") + 1] == "query.json"
+
+
+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"))
+
+    argv = bot_relay.local_delivery_command("ops", "query.json")
+    assert argv[0] == "hermes"
+    assert argv[1:3] == ["-p", "ops"]
+
+
+def test_delivery_lock_recognizes_resolved_cli_paths(tmp_path, monkeypatch):
+    """The #93091 per-profile turn lock must keep matching delivery argvs
+    now that argv[0] may be a resolved absolute path (or hermes.exe)."""
+    acquired = []
+
+    class _Ctx:
+        def __enter__(self):
+            acquired.append("locked")
+            return self
+
+        def __exit__(self, *exc):
+            return False
+
+    monkeypatch.setattr(bot_relay, "acquire_turn_lock", lambda root, profile: _Ctx())
+    monkeypatch.setenv("HERMES_HOME", str(tmp_path))
+
+    with bot_mode_dm._delivery_lock(
+        [str(tmp_path / "venv" / "bin" / "hermes"), "-p", "ops", "chat"],
+        stdin_file=False,
+    ):
+        pass
+    with bot_mode_dm._delivery_lock(["hermes", "-p", "ops", "chat"], stdin_file=False):
+        pass
+    with bot_mode_dm._delivery_lock(
+        ["C:\\venv\\Scripts\\hermes.exe", "-p", "ops", "chat"], stdin_file=False
+    ):
+        pass
+    assert acquired == ["locked", "locked", "locked"]
+
+    # Unrelated argvs still bypass the lock entirely.
+    with bot_mode_dm._delivery_lock(["python", "-m", "whatever"], stdin_file=False):
+        pass
+    assert acquired == ["locked", "locked", "locked"]

tests/tools/test_bot_retry_policy.py

@@ -76,11 +76,18 @@ def _deliver(params):
     return srv._methods["bot_relay.deliver"](1, params)
 
 
+def _is_hermes_cli(argv) -> bool:
+    """Match the delivery CLI by basename — local_delivery_command may
+    resolve the venv-relative hermes next to the interpreter (#93590)."""
+    name = str(argv[0]).rsplit("\\", 1)[-1].rsplit("/", 1)[-1]
+    return name in ("hermes", "hermes.exe")
+
+
 def _transport_calls(calls):
     """Only the Bot Chat transport spawns — a global subprocess.run patch also
     catches unrelated maintenance calls (git version probes on first server
     import), which must not count as delivery attempts."""
-    return [argv for argv in calls if argv and argv[0] == "hermes"]
+    return [argv for argv in calls if argv and _is_hermes_cli(argv)]
 
 
 class _Proc:
@@ -97,7 +104,7 @@ def test_deliver_retries_same_argv_on_transient_failure(home, monkeypatch):
 
     def _fake_run(argv, **kwargs):
         calls.append(list(argv))
-        if list(argv)[:1] != ["hermes"]:
+        if not _is_hermes_cli(list(argv)):
             return _Proc(0)
         if len(_transport_calls(calls)) == 1:
             return _Proc(1, stderr="Error code: 429 - rate limit exceeded")
@@ -119,7 +126,7 @@ def test_deliver_retries_once_on_context_overflow(home, monkeypatch):
 
     def _fake_run(argv, **kwargs):
         calls.append(list(argv))
-        if list(argv)[:1] != ["hermes"]:
+        if not _is_hermes_cli(list(argv)):
             return _Proc(0)
         if len(_transport_calls(calls)) == 1:
             return _Proc(1, stderr="This model's maximum context length is 200000 tokens")
@@ -139,7 +146,7 @@ def test_deliver_never_retries_auth_failure(home, monkeypatch):
 
     def _fake_run(argv, **kwargs):
         calls.append(list(argv))
-        if list(argv)[:1] != ["hermes"]:
+        if not _is_hermes_cli(list(argv)):
             return _Proc(0)
         return _Proc(1, stderr="Error code: 401 - Your API key is invalid")
 
@@ -156,7 +163,7 @@ def test_deliver_failure_carries_typed_reason(home, monkeypatch):
     monkeypatch.setattr(
         "subprocess.run",
         lambda argv, **k: _Proc(1, stderr="502 server error - overloaded")
-        if list(argv)[:1] == ["hermes"]
+        if _is_hermes_cli(list(argv))
         else _Proc(0),
     )
     out = _deliver({"profile": "ops", "message": "ping"})

tests/tools/test_bot_turn_lock.py

@@ -247,13 +247,17 @@ class _P:
 
 def test_local_delivery_command_never_reenters_the_lock():
     """The gateway deliver handler runs local_delivery_command ALREADY holding
-    the profile lock. That argv must stay a raw `hermes -p … chat` invocation:
+    the profile lock. That argv must stay a raw hermes CLI invocation:
     routing it through the --run-delivery wrapper would make the child hit
-    _delivery_lock (argv[0]=='hermes' and argv[1]=='-p'), burn the full wait
+    _delivery_lock (hermes CLI + '-p'), burn the full wait
     budget against its parent's flock, and fail every relay delivery with
-    target_busy."""
+    target_busy. argv[0] may be a resolved venv path (#93590) — the lock
+    matcher and this assertion both go by basename."""
+    from pathlib import Path
+
     argv = bot_relay.local_delivery_command("ops", "/tmp/q.txt")
-    assert argv[:3] == ["hermes", "-p", "ops"]
+    assert argv[1:3] == ["-p", "ops"]
+    assert Path(argv[0]).name in ("hermes", "hermes.exe")
     assert "--run-delivery" not in argv
     assert not any("bot_mode_dm" in part for part in argv)
 

tests/tui_gateway/test_bot_relay_methods.py

@@ -78,12 +78,14 @@ def _fake_run(argv, **kwargs):
     )
     assert out["reply"] == "pong from ops"
     argv = calls["argv"]
-    assert argv[:3] == ["hermes", "-p", "ops"]
+    # argv[0] may be a resolved venv path (#93590) — match by basename.
+    assert argv[1:3] == ["-p", "ops"]
+    assert argv[0].rsplit("\\", 1)[-1].rsplit("/", 1)[-1] in ("hermes", "hermes.exe")
     assert "Bot Chat" in argv and "--query-file" in argv
 
     # 'hermes' alias resolves to default
     _result(srv._methods["bot_relay.deliver"](2, {"profile": "hermes", "message": "x"}))
-    assert calls["argv"][:3] == ["hermes", "-p", "default"]
+    assert calls["argv"][1:3] == ["-p", "default"]
 
     # unknown profile refuses without spawning
     calls.clear()

tools/bot_mode_dm.py

@@ -527,7 +527,19 @@ def _delivery_lock(argv: list[str], *, stdin_file: bool):
     lock in ``tools.bot_relay``. Peer transports (stdin mode) run on the
     remote gateway; their turn is locked THERE by its own deliver path.
     """
-    if stdin_file or len(argv) < 3 or argv[0] != "hermes" or argv[1] != "-p":
+    # The CLI element is matched by basename: local_delivery_command now
+    # resolves the venv-relative hermes next to this gateway's interpreter
+    # (#93590 — service contexts lack PATH), so argv[0] may be an absolute
+    # path (and on Windows carries the .exe suffix). Split on both
+    # separators so the shape matches regardless of which platform built
+    # the argv.
+    cli = (argv[0] if argv else "").rsplit("\\", 1)[-1].rsplit("/", 1)[-1]
+    if (
+        stdin_file
+        or len(argv) < 3
+        or cli not in ("hermes", "hermes.exe")
+        or argv[1] != "-p"
+    ):
         return contextlib.nullcontext()
     from tools.bot_relay import acquire_turn_lock
 

tools/bot_relay.py

@@ -496,10 +496,18 @@ def waiter_command(root: Path | str, envelope: dict) -> str:
     )
     # Encode label with !r so roster fields cannot break out of the generated
     # python -c source (quotes, parens, or extra statements in connection_id).
+    # The raw-string prefix keeps Windows paths viable: repr escapes each
+    # backslash ("C:\\Users\\..."), but the Windows execution layer the
+    # waiter runs under folds "\\" back to "\", which turns "\U" into an
+    # invalid unicode escape and SyntaxErrors the whole script (#93590).
+    # With the r prefix the folded single backslash parses as a literal.
+    # POSIX paths contain no backslashes, so the prefix is a no-op there,
+    # and \' inside a raw literal still cannot terminate the string, so
+    # the injection defense above is unchanged.
     code = (
         "import json,os,sys,time\n"
-        f"p = {reply_path!r}\n"
-        f"label = {label!r}\n"
+        f"p = r{reply_path!r}\n"
+        f"label = r{label!r}\n"
         f"deadline = time.time() + {REPLY_WAIT_SECONDS}\n"
         "while time.time() < deadline:\n"
         "    if os.path.exists(p):\n"
@@ -526,10 +534,26 @@ def waiter_command(root: Path | str, envelope: dict) -> str:
 # ── delivery command (used by the deliver RPC on the TARGET gateway) ────────
 
 
+def _hermes_cli() -> str:
+    """Resolve the hermes CLI beside this gateway's own interpreter.
+
+    The deliver RPC runs on the target gateway, whose process is the venv
+    python — its bin/Scripts directory holds the matching ``hermes``
+    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.
+    """
+    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"
+
+
 def local_delivery_command(profile: str, query_file: str) -> list[str]:
     """argv that delivers a DM into ``profile``'s Bot Chat on THIS gateway."""
     return [
-        "hermes",
+        _hermes_cli(),
         "-p",
         profile,
         "chat",