From c3b0502297365d1e5246d036d20bdd9666455968 Mon Sep 17 00:00:00 2001 From: Ken Date: Thu, 28 May 2026 08:16:06 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20muxterm=20binary=20discovery=20=E2=80=94?= =?UTF-8?q?=20find=20local=20build=20before=20PATH=20search?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _find_muxterm_binary() now resolves in this order: 1. Explicit binary_path argument (unchanged) 2. MUXTERM_BINARY env var (new) 3. /muxterm/muxterm — the go build output for dev installs (new) 4. shutil.which('muxterm') — PATH fallback (unchanged) Before this fix the service logged 'muxterm binary not found; terminal feature disabled' on every restart because the binary is not on PATH and no explicit path was passed from main.py. Also fix stop_muxterm() to catch ProcessLookupError from terminate() when the process has already exited (was only caught in the wait_for block, not the terminate() call itself), which caused a noisy 'Application shutdown failed' traceback in the service journal. --- muxplex/muxterm.py | 41 ++++++++++++++++++++++++++++++----- muxplex/tests/test_muxterm.py | 16 ++++++++++---- 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/muxplex/muxterm.py b/muxplex/muxterm.py index eccac16..c49a8cc 100644 --- a/muxplex/muxterm.py +++ b/muxplex/muxterm.py @@ -16,6 +16,8 @@ Internal: import asyncio import logging +import os +import pathlib import shutil logger = logging.getLogger(__name__) @@ -35,16 +37,37 @@ _restart_task: asyncio.Task | None = None # type: ignore[type-arg] def _find_muxterm_binary(binary_path: str | None = None) -> str: """Locate the muxterm binary. - Checks explicit *binary_path* first, then falls back to - ``shutil.which('muxterm')``. Raises :exc:`FileNotFoundError` if - the binary cannot be found. + Resolution order: + 1. Explicit *binary_path* argument. + 2. ``MUXTERM_BINARY`` environment variable. + 3. ``/muxterm/muxterm`` — the build output for dev/local installs + (package lives at ``/muxplex/``, binary at + ``/muxterm/muxterm``). + 4. ``shutil.which('muxterm')`` — installed on PATH. + + Raises :exc:`FileNotFoundError` if none of the above resolves. """ if binary_path is not None: return binary_path + + env_path = os.environ.get("MUXTERM_BINARY") + if env_path: + candidate = pathlib.Path(env_path) + if candidate.is_file() and os.access(candidate, os.X_OK): + return str(candidate) + + # Dev/local-install: binary is built next to the Python package directory. + # muxterm.py lives in /muxplex/ + # muxterm binary is at /muxterm/muxterm + local_candidate = pathlib.Path(__file__).parent.parent / "muxterm" / "muxterm" + if local_candidate.is_file() and os.access(local_candidate, os.X_OK): + return str(local_candidate) + found = shutil.which("muxterm") if found is None: raise FileNotFoundError( - "muxterm binary not found on PATH; pass binary_path explicitly" + "muxterm binary not found; tried MUXTERM_BINARY env var, " + f"{local_candidate}, and PATH. Build it with `cd muxterm && go build .`" ) return found @@ -157,9 +180,15 @@ async def stop_muxterm() -> None: _restart_task = None if _muxterm_process is not None: - _muxterm_process.terminate() + try: + _muxterm_process.terminate() + except ProcessLookupError: + pass # already exited try: await asyncio.wait_for(_muxterm_process.wait(), timeout=5.0) except (asyncio.TimeoutError, ProcessLookupError): - _muxterm_process.kill() + try: + _muxterm_process.kill() + except ProcessLookupError: + pass _muxterm_process = None diff --git a/muxplex/tests/test_muxterm.py b/muxplex/tests/test_muxterm.py index a7b2dd7..d9aa1db 100644 --- a/muxplex/tests/test_muxterm.py +++ b/muxplex/tests/test_muxterm.py @@ -46,16 +46,24 @@ def test_find_muxterm_binary_explicit_path(tmp_path): def test_find_muxterm_binary_uses_which(): - """Falls back to shutil.which('muxterm') when no explicit path.""" - with patch("shutil.which", return_value="/usr/bin/muxterm") as mock_which: + """Falls back to shutil.which('muxterm') when no env var and no local binary.""" + with ( + patch.dict("os.environ", {"MUXTERM_BINARY": ""}), # suppress env override + patch("muxplex.muxterm.pathlib.Path.is_file", return_value=False), # no local build + patch("shutil.which", return_value="/usr/bin/muxterm") as mock_which, + ): result = _find_muxterm_binary() mock_which.assert_called_once_with("muxterm") assert result == "/usr/bin/muxterm" def test_find_muxterm_binary_raises_when_not_found(): - """Raises FileNotFoundError if muxterm is not found.""" - with patch("shutil.which", return_value=None): + """Raises FileNotFoundError if muxterm is not found anywhere.""" + with ( + patch.dict("os.environ", {"MUXTERM_BINARY": ""}), # suppress env override + patch("muxplex.muxterm.pathlib.Path.is_file", return_value=False), # no local build + patch("shutil.which", return_value=None), + ): with pytest.raises(FileNotFoundError): _find_muxterm_binary()