Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions docs/porting-to-a-new-os.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 25 additions & 5 deletions src/automator/adapters/tmux_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +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 / decoding / timeout) 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
Expand Down Expand Up @@ -37,7 +39,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,

@augmentcode augmentcode Bot Jul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BaseTmuxBackend._run now accepts kw-only env, so any subclass override should accept env as well (even if unused) to avoid a future TypeError if callers start passing it; other locations where this applies: tests/test_multiplexer.py:263.

Severity: medium

Other Locations
  • tests/test_multiplexer.py:263

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in e5aa06a — the PsmuxStyle test double now accepts env=None. (No production caller passes env yet, so this was latent, but worth closing now.)

) -> 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
Expand All @@ -46,11 +58,19 @@ 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.
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], 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()}")
Expand Down
63 changes: 62 additions & 1 deletion tests/test_multiplexer.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"""

import json
import os
import subprocess

import pytest
Expand Down Expand Up @@ -259,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()
Expand All @@ -269,3 +270,63 @@ 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)
monkeypatch.setenv("TMUX", "/tmp/tmux-0/default,1234,0") # a nesting-guard var to scrub
before = dict(os.environ)

# 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
Loading