From 96f948d8f25ddf288e778f2410e6f420df99eaa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Davor=20Raci=C4=87?= Date: Thu, 2 Jul 2026 23:36:16 +0200 Subject: [PATCH 1/3] refactor(adapters): let tmux _run carry output encoding and per-call env BaseTmuxBackend._run gains a class-level _ENCODING (default None) and an optional keyword-only env kwarg, both threaded into subprocess.run with defaults byte-identical to today's POSIX behavior. A native-Windows leaf can then force UTF-8 decoding and spawn with a scrubbed env without re-implementing the spawn primitive. The POSIX TmuxMultiplexer is unchanged at runtime (no encoding, no env passed); existing tmux tests pass unmodified, plus regression tests for the default path, a subclass setting _ENCODING, and a non-leaking custom env. Closes #40 --- src/automator/adapters/tmux_base.py | 27 +++++++++++--- tests/test_multiplexer.py | 56 +++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/src/automator/adapters/tmux_base.py b/src/automator/adapters/tmux_base.py index ce494728..a32a3ea7 100644 --- a/src/automator/adapters/tmux_base.py +++ b/src/automator/adapters/tmux_base.py @@ -5,7 +5,8 @@ :mod:`.tmux_backend`). The point of the split is that a tmux-*family* backend — an eventual native-Windows "psmux" — can subclass :class:`BaseTmuxBackend` and override only the single spawn primitive :meth:`BaseTmuxBackend._run` (to tweak -the binary / decoding / timeout) plus the few divergent methods (e.g. the +the binary or timeout — output decoding and per-call ``env`` are ``_run`` +parameters, not overrides) plus the few divergent methods (e.g. the parked-window trailer), **without editing** :mod:`.tmux_backend`. Every method that talks to tmux funnels through :meth:`BaseTmuxBackend._run`, the @@ -37,7 +38,17 @@ class BaseTmuxBackend(TerminalMultiplexer): """tmux-family backend: all argv construction and every contract method, with one overridable subprocess primitive (:meth:`_run`) every call funnels through.""" - def _run(self, argv: list[str], *, check: bool = True) -> subprocess.CompletedProcess[str]: + #: Output decoding for captured tmux text. ``None`` (POSIX) = locale default, + #: byte-identical to a bare ``text=True``; a Windows leaf sets ``"utf-8"``. + _ENCODING: str | None = None + + def _run( + self, + argv: list[str], + *, + check: bool = True, + env: dict[str, str] | None = None, + ) -> subprocess.CompletedProcess[str]: """The ONE place tmux is spawned. ``argv`` are the args after ``tmux``. With ``check=True`` a non-zero exit raises :class:`TmuxError` (the strict @@ -46,11 +57,17 @@ def _run(self, argv: list[str], *, check: bool = True) -> subprocess.CompletedPr A timeout / missing binary always propagates (``TimeoutExpired`` / ``OSError``) so callers' existing ``try/except`` still fires. - Subclasses (e.g. a native-Windows psmux) override this to tweak the binary, - decoding (``encoding="utf-8"``), or timeout — and nothing else. + ``env`` (keyword-only, default ``None`` → inherit the parent env) lets one + caller spawn with a scrubbed env without mutating this process's; decoding is + the :attr:`_ENCODING` class attr. A leaf sets those rather than overriding here. """ proc = subprocess.run( - ["tmux", *argv], capture_output=True, text=True, timeout=TMUX_TIMEOUT_S + ["tmux", *argv], + capture_output=True, + text=True, + encoding=self._ENCODING, + env=env, + timeout=TMUX_TIMEOUT_S, ) if check and proc.returncode != 0: raise TmuxError(f"tmux {' '.join(argv[:2])} failed: {proc.stderr.strip()}") diff --git a/tests/test_multiplexer.py b/tests/test_multiplexer.py index fbfe7ebd..1e4d7fe6 100644 --- a/tests/test_multiplexer.py +++ b/tests/test_multiplexer.py @@ -8,6 +8,7 @@ """ import json +import os import subprocess import pytest @@ -269,3 +270,58 @@ def _run(self, argv, *, check=True): mux.window_alive("s", "@1") # sentinel methods still degrade rather than leak the raw timeout assert mux.list_windows("s", ["window_id"]) == [] + + +# ---------------------------------------------- _run seam: encoding + env (#40) +# +# The spawn primitive carries the two knobs its docstring promises — output +# decoding (class attr _ENCODING) and a per-call env — both defaulting to today's +# POSIX behavior, so a native-Windows leaf overrides zero lines of spawn plumbing. + + +class _RecordRun: + """Stand-in for subprocess.run that records the kwargs of the one spawn.""" + + def __init__(self): + self.kwargs: dict = {} + + def __call__(self, argv, **kwargs): + self.kwargs = kwargs + return subprocess.CompletedProcess(argv, 0, stdout="", stderr="") + + +def test_run_posix_default_passes_no_encoding_and_no_env(monkeypatch): + rec = _RecordRun() + monkeypatch.setattr(tmux_base.subprocess, "run", rec) + + TmuxMultiplexer()._run(["list-windows"]) + + # byte-identical to today: locale-default decode (encoding=None ≡ bare text=True), + # inherit the parent env (env=None). + assert rec.kwargs["text"] is True + assert rec.kwargs["encoding"] is None + assert rec.kwargs["env"] is None + + +def test_run_subclass_encoding_reaches_subprocess(monkeypatch): + rec = _RecordRun() + monkeypatch.setattr(tmux_base.subprocess, "run", rec) + + class Utf8Backend(TmuxMultiplexer): + _ENCODING = "utf-8" # a Windows leaf forces UTF-8 without touching _run + + Utf8Backend()._run(["list-windows"]) + assert rec.kwargs["encoding"] == "utf-8" + + +def test_run_custom_env_is_forwarded_without_leaking(monkeypatch): + rec = _RecordRun() + monkeypatch.setattr(tmux_base.subprocess, "run", rec) + before = dict(os.environ) + + scrubbed = {"PATH": os.environ.get("PATH", "")} # e.g. nesting-guard vars dropped + TmuxMultiplexer()._run(["new-session"], env=scrubbed) + + assert rec.kwargs["env"] == scrubbed + # the scrubbed env is confined to the child spawn — this process's env is untouched + assert dict(os.environ) == before From e5aa06a1a05fd312100fd3ca6104dd192cb651d8 Mon Sep 17 00:00:00 2001 From: pbean Date: Thu, 2 Jul 2026 17:55:09 -0700 Subject: [PATCH 2/3] docs+test(adapters): sync porting doc and test double with the widened _run seam Review fixups for #41: - porting doc: decoding is the _ENCODING class attr and env a _run param; overriding _run itself is now only for binary/timeout - PsmuxStyle test double accepts the new env kwarg so it stays signature-compatible when a contract method starts forwarding env= - _run docstring: scrubbed envs must be built by removing vars from a copy of the parent env (Windows children need SystemRoot etc.) Co-Authored-By: Claude Fable 5 --- docs/porting-to-a-new-os.md | 8 +++++--- src/automator/adapters/tmux_base.py | 2 ++ tests/test_multiplexer.py | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/porting-to-a-new-os.md b/docs/porting-to-a-new-os.md index 09286e47..b0e11ac8 100644 --- a/docs/porting-to-a-new-os.md +++ b/docs/porting-to-a-new-os.md @@ -60,9 +60,11 @@ fallback, so POSIX behavior is unchanged. (The result is cached — see - **Extend `BaseTmuxBackend`** (`adapters/tmux_base.py`) for a **tmux-family** backend. `BaseTmuxBackend` holds every argv construction and routes every spawn - through one primitive, `_run(argv, *, check=...)`. A native-Windows "psmux" - that speaks a tmux-like CLI overrides only `_run()` (to tweak the binary, - decoding, or timeout) plus the few genuinely divergent methods (e.g. the + through one primitive, `_run(argv, *, check=..., env=...)`. A native-Windows + "psmux" that speaks a tmux-like CLI sets the `_ENCODING` class attribute for + output decoding (e.g. `"utf-8"`) and passes a per-call `env=` where needed — + overriding `_run()` itself only to tweak the binary or timeout — plus the few + genuinely divergent methods (e.g. the parked-window `sh -c` trailer in `new_parked_window`) — **without editing** `tmux_base.py` or its POSIX leaf `tmux_backend.py` (`TmuxMultiplexer`). - **Implement `TerminalMultiplexer` fresh** when the host has no tmux-shaped CLI diff --git a/src/automator/adapters/tmux_base.py b/src/automator/adapters/tmux_base.py index a32a3ea7..75ef8f54 100644 --- a/src/automator/adapters/tmux_base.py +++ b/src/automator/adapters/tmux_base.py @@ -60,6 +60,8 @@ def _run( ``env`` (keyword-only, default ``None`` → inherit the parent env) lets one caller spawn with a scrubbed env without mutating this process's; decoding is the :attr:`_ENCODING` class attr. A leaf sets those rather than overriding here. + Build a scrubbed env by copying the parent env and *removing* the offending + vars — not from scratch (on Windows the child needs ``SystemRoot`` etc.). """ proc = subprocess.run( ["tmux", *argv], diff --git a/tests/test_multiplexer.py b/tests/test_multiplexer.py index 1e4d7fe6..b02af628 100644 --- a/tests/test_multiplexer.py +++ b/tests/test_multiplexer.py @@ -260,7 +260,7 @@ def test_seam_honesty_holds_for_psmux_style_run_override(monkeypatch): monkeypatch.setattr(tmux_base.shutil, "which", lambda _name: "/usr/bin/tmux") class PsmuxStyle(TmuxMultiplexer): - def _run(self, argv, *, check=True): + def _run(self, argv, *, check=True, env=None): raise subprocess.TimeoutExpired(["tmux", *argv], 30) mux = PsmuxStyle() From 350589f0df0eaefa84665cd6ac81da0084ad69c5 Mon Sep 17 00:00:00 2001 From: pbean Date: Thu, 2 Jul 2026 18:13:37 -0700 Subject: [PATCH 3/3] docs+test(adapters): name the decoding knob accurately; model the safe env scrub MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review fixups (round 2) for #41: - module docstring: decoding is the _ENCODING class attribute, not a _run parameter — only env is; reword so nobody tries _run(..., encoding=...) - env-forwarding test: build the scrubbed env by copying the parent and removing the nesting-guard var, matching the _run docstring guidance, instead of rebuilding from scratch (unsafe template on Windows) Co-Authored-By: Claude Fable 5 --- src/automator/adapters/tmux_base.py | 5 +++-- tests/test_multiplexer.py | 7 ++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/automator/adapters/tmux_base.py b/src/automator/adapters/tmux_base.py index 75ef8f54..78bde1c7 100644 --- a/src/automator/adapters/tmux_base.py +++ b/src/automator/adapters/tmux_base.py @@ -5,8 +5,9 @@ :mod:`.tmux_backend`). The point of the split is that a tmux-*family* backend — an eventual native-Windows "psmux" — can subclass :class:`BaseTmuxBackend` and override only the single spawn primitive :meth:`BaseTmuxBackend._run` (to tweak -the binary or timeout — output decoding and per-call ``env`` are ``_run`` -parameters, not overrides) plus the few divergent methods (e.g. the +the binary or timeout — output decoding is the :attr:`BaseTmuxBackend._ENCODING` +class attribute and a scrubbed per-call ``env`` is a ``_run`` parameter, neither +an override) plus the few divergent methods (e.g. the parked-window trailer), **without editing** :mod:`.tmux_backend`. Every method that talks to tmux funnels through :meth:`BaseTmuxBackend._run`, the diff --git a/tests/test_multiplexer.py b/tests/test_multiplexer.py index b02af628..cfa00f64 100644 --- a/tests/test_multiplexer.py +++ b/tests/test_multiplexer.py @@ -317,11 +317,16 @@ class Utf8Backend(TmuxMultiplexer): def test_run_custom_env_is_forwarded_without_leaking(monkeypatch): rec = _RecordRun() monkeypatch.setattr(tmux_base.subprocess, "run", rec) + monkeypatch.setenv("TMUX", "/tmp/tmux-0/default,1234,0") # a nesting-guard var to scrub before = dict(os.environ) - scrubbed = {"PATH": os.environ.get("PATH", "")} # e.g. nesting-guard vars dropped + # per the _run docstring: copy the parent env and REMOVE the offending var — + # never rebuild from scratch (Windows children need SystemRoot etc.) + scrubbed = dict(os.environ) + del scrubbed["TMUX"] TmuxMultiplexer()._run(["new-session"], env=scrubbed) assert rec.kwargs["env"] == scrubbed + assert "TMUX" not in rec.kwargs["env"] # the scrubbed env is confined to the child spawn — this process's env is untouched assert dict(os.environ) == before